Skip to content

Commit 4e5393e

Browse files
fix: resolve type checking errors in over_saturation constraint
- Fix first_iteration -> first_token_iteration attribute name - Add type ignore for OverSaturationConstraint return type - Fix validated_kwargs type handling for stop_over_saturated parameter Signed-off-by: Alon Kellner <[email protected]>
1 parent 378751f commit 4e5393e

File tree

8 files changed

+204
-190
lines changed

8 files changed

+204
-190
lines changed

src/guidellm/scheduler/constraints/base.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@
1515
from guidellm.scheduler.schemas import SchedulerState, SchedulerUpdateAction
1616
from guidellm.schemas import RequestInfo, StandardBaseModel
1717
from guidellm.utils import InfoMixin
18-
from .protocols import Constraint, ConstraintInitializer, SerializableConstraintInitializer
18+
19+
from .protocols import (
20+
Constraint,
21+
)
1922

2023
__all__ = [
2124
"PydanticConstraintInitializer",
@@ -130,4 +133,3 @@ def __call__(
130133
"Cannot invoke unserializable constraint instance. "
131134
"This constraint was not properly serialized and cannot be executed."
132135
)
133-

src/guidellm/scheduler/constraints/factory.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from typing import Any
1212

1313
from guidellm.utils import InfoMixin, RegistryMixin
14+
1415
from .base import UnserializableConstraintInitializer
1516
from .protocols import (
1617
Constraint,
@@ -180,4 +181,3 @@ def resolve_constraints(
180181
resolved_constraints[key] = cls.create_constraint(key, val)
181182

182183
return resolved_constraints
183-

src/guidellm/scheduler/constraints/over_saturation.py

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,13 @@
1414

1515
from pydantic import Field
1616

17-
from guidellm.scheduler.schemas import SchedulerState, SchedulerUpdateAction
17+
from guidellm.scheduler.schemas import (
18+
SchedulerState,
19+
SchedulerUpdateAction,
20+
)
1821
from guidellm.schemas import RequestInfo
1922
from guidellm.settings import settings
23+
2024
from .base import PydanticConstraintInitializer
2125
from .factory import ConstraintsInitializerFactory
2226
from .protocols import Constraint
@@ -362,10 +366,10 @@ def __call__(
362366
elif (
363367
request_info.status == "completed"
364368
and request_info.timings
365-
and request_info.timings.first_iteration
369+
and request_info.timings.first_token_iteration
366370
):
367371
ttft = (
368-
request_info.timings.first_iteration
372+
request_info.timings.first_token_iteration
369373
- request_info.timings.request_start
370374
)
371375
self.over_saturation_detector.add_finished(
@@ -436,13 +440,13 @@ def create_constraint(self, **_kwargs) -> Constraint:
436440
minimum_duration=self.min_seconds,
437441
maximum_window_seconds=self.max_window_seconds,
438442
)
439-
return OverSaturationConstraint(
443+
return OverSaturationConstraint( # type: ignore[return-value]
440444
over_saturation_detector=over_saturation_detector,
441445
stop_over_saturated=self.stop_over_saturated,
442446
)
443447

444448
@classmethod
445-
def validated_kwargs(cls, stop_over_saturated: bool, **kwargs) -> dict[str, Any]:
449+
def validated_kwargs(cls, stop_over_saturated: bool | None = None, **kwargs) -> dict[str, Any]:
446450
"""
447451
Validate and process arguments for OverSaturationConstraint creation.
448452
@@ -451,8 +455,10 @@ def validated_kwargs(cls, stop_over_saturated: bool, **kwargs) -> dict[str, Any]
451455
:return: Validated dictionary with stop_over_saturated field
452456
"""
453457
aliases = ["stop_over_saturated", "stop_over_sat", "stop_osd"]
458+
result = stop_over_saturated if stop_over_saturated is not None else False
454459
for alias in aliases:
455-
stop_over_saturated = stop_over_saturated or kwargs.get(alias)
456-
457-
return {"stop_over_saturated": stop_over_saturated}
460+
alias_value = kwargs.get(alias)
461+
if alias_value is not None:
462+
result = bool(alias_value) or result
458463

464+
return {"stop_over_saturated": result}

src/guidellm/scheduler/constraints/protocols.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,3 @@ def create_constraint(self, **kwargs) -> Constraint:
8585
:param kwargs: Additional configuration parameters
8686
:return: Configured constraint evaluation function
8787
"""
88-

src/guidellm/scheduler/constraints/standard.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
from guidellm.schemas import RequestInfo, StandardBaseModel
2121
from guidellm.settings import settings
2222
from guidellm.utils import InfoMixin
23+
2324
from .base import PydanticConstraintInitializer
2425
from .factory import ConstraintsInitializerFactory
2526
from .protocols import Constraint
@@ -690,4 +691,3 @@ def __call__(
690691
stop_time=stop_time,
691692
),
692693
)
693-

tests/unit/scheduler/OVER_SATURATION_TEST_COVERAGE.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ This document outlines the comprehensive unit test coverage for the over-saturat
142142
- **Focus**: Basic initialization, core algorithms, critical paths
143143

144144
### Sanity Tests (`@pytest.mark.sanity`)
145-
- **Count**: 21 tests
145+
- **Count**: 21 tests
146146
- **Purpose**: Comprehensive validation of feature behavior
147147
- **Runtime**: 1-3 minutes total
148148
- **Focus**: Realistic scenarios, robustness, edge cases
@@ -156,22 +156,22 @@ This document outlines the comprehensive unit test coverage for the over-saturat
156156
-**Threshold detection**: TTFT violations and concurrent request tracking
157157
-**Statistical significance**: Margin of error and confidence testing
158158

159-
### Integration Coverage
159+
### Integration Coverage
160160
-**Detector ↔ Constraint**: Proper data flow and decision making
161161
-**Constraint ↔ Scheduler**: State integration and action generation
162162
-**Factory ↔ Initializer**: Proper constraint creation and configuration
163163
-**Timing ↔ Detection**: Accurate duration and timing calculations
164164

165165
### Robustness Coverage
166166
-**Empty data**: No crashes or false positives
167-
-**Malformed data**: Proper validation and error handling
167+
-**Malformed data**: Proper validation and error handling
168168
-**Extreme values**: Numerical stability maintained
169169
-**Memory management**: Bounded growth under stress
170170
-**Performance**: Efficiency maintained at scale
171171

172172
### Scenario Coverage
173173
-**Gradual degradation**: Detected correctly
174-
-**Sudden spikes**: Detected correctly
174+
-**Sudden spikes**: Detected correctly
175175
-**Stable performance**: No false positives
176176
-**Recovery patterns**: Proper handling
177177
-**Variable workloads**: Robust detection

tests/unit/scheduler/test_over_saturation.py

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,7 @@ def test_check_alert_requires_minimum_duration(self):
154154
@pytest.mark.sanity
155155
def test_check_alert_requires_minimum_window_size(self):
156156
"""Test that check_alert requires minimum window size."""
157-
detector = OverSaturationDetector(
158-
minimum_duration=0.0, minimum_window_size=10
159-
)
157+
detector = OverSaturationDetector(minimum_duration=0.0, minimum_window_size=10)
160158

161159
# Add few requests
162160
for i in range(5):
@@ -291,15 +289,11 @@ def test_constraint_stops_when_over_saturated(self, detector):
291289
# Simulate over-saturation by creating positive slopes
292290
# Add many started requests with increasing concurrent count
293291
for i in range(20):
294-
detector.add_started(
295-
{"concurrent_requests": i * 2, "duration": float(i)}
296-
)
292+
detector.add_started({"concurrent_requests": i * 2, "duration": float(i)})
297293

298294
# Add finished requests with increasing TTFT
299295
for i in range(20):
300-
detector.add_finished(
301-
{"ttft": 1.0 + i * 0.1, "duration": float(i) + 10.0}
302-
)
296+
detector.add_finished({"ttft": 1.0 + i * 0.1, "duration": float(i) + 10.0})
303297

304298
detector.update_duration(30.0)
305299
detector.check_alert() # Prime the slope checkers
@@ -363,7 +357,11 @@ class TestOverSaturationConstraintInitializer:
363357
params=[
364358
{"stop_over_saturated": True},
365359
{"stop_over_saturated": False},
366-
{"stop_over_saturated": True, "min_seconds": 10.0, "max_window_seconds": 60.0},
360+
{
361+
"stop_over_saturated": True,
362+
"min_seconds": 10.0,
363+
"max_window_seconds": 60.0,
364+
},
367365
]
368366
)
369367
def valid_instances(self, request):
@@ -619,4 +617,3 @@ def test_check_slope_requires_minimum_samples(self, slope_checker):
619617
slope_checker.add_data_point(3.0, 6.0)
620618
result = slope_checker.check_slope(3.0)
621619
# Might be True or False depending on confidence intervals
622-

0 commit comments

Comments
 (0)