Skip to content

Commit 90d910e

Browse files
committed
mvoe validation logic to RequestHandlers
Signed-off-by: Uri Shaket <ushaket@redhat.com>
1 parent c74cfbd commit 90d910e

4 files changed

Lines changed: 105 additions & 196 deletions

File tree

src/guidellm/backends/openai/request_handlers.py

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,14 +219,16 @@ def compile_non_streaming(
219219
text = choice.get("text", "")
220220
input_metrics, output_metrics = self.extract_metrics(usage, text)
221221

222-
return GenerationResponse(
222+
compiled = GenerationResponse(
223223
request_id=request.request_id,
224224
request_args=arguments.model_dump_json(),
225225
response_id=response.get("id"), # use vLLM ID if available
226226
text=text,
227227
input_metrics=input_metrics,
228228
output_metrics=output_metrics,
229229
)
230+
self._validate_compiled_response(compiled)
231+
return compiled
230232

231233
def add_streaming_line(self, line: str) -> int | None:
232234
"""
@@ -269,14 +271,26 @@ def compile_streaming(
269271
text = "".join(self.streaming_texts)
270272
input_metrics, output_metrics = self.extract_metrics(self.streaming_usage, text)
271273

272-
return GenerationResponse(
274+
compiled = GenerationResponse(
273275
request_id=request.request_id,
274276
request_args=arguments.model_dump_json(),
275277
response_id=self.streaming_response_id, # use vLLM ID if available
276278
text=text,
277279
input_metrics=input_metrics,
278280
output_metrics=output_metrics,
279281
)
282+
self._validate_compiled_response(compiled)
283+
return compiled
284+
285+
def _validate_compiled_response(self, response: GenerationResponse) -> None:
286+
"""Raise when endpoint produced a terminal payload with no usable output."""
287+
has_text = bool(response.text and response.text.strip())
288+
output_tokens = response.output_metrics.total_tokens or 0
289+
if not has_text and output_tokens <= 0:
290+
raise ValueError(
291+
"[UNUSABLE_BACKEND_RESPONSE] backend resolved without a usable "
292+
"terminal response payload"
293+
)
280294

281295
def extract_line_data(self, line: str) -> dict[str, Any] | None:
282296
"""
@@ -493,14 +507,16 @@ def compile_non_streaming(
493507
text = choice.get("message", {}).get("content", "")
494508
input_metrics, output_metrics = self.extract_metrics(usage, text)
495509

496-
return GenerationResponse(
510+
compiled = GenerationResponse(
497511
request_id=request.request_id,
498512
request_args=arguments.model_dump_json(),
499513
response_id=response.get("id"), # use vLLM ID if available
500514
text=text,
501515
input_metrics=input_metrics,
502516
output_metrics=output_metrics,
503517
)
518+
self._validate_compiled_response(compiled)
519+
return compiled
504520

505521
def add_streaming_line(self, line: str) -> int | None:
506522
"""
@@ -543,14 +559,16 @@ def compile_streaming(
543559
text = "".join(self.streaming_texts)
544560
input_metrics, output_metrics = self.extract_metrics(self.streaming_usage, text)
545561

546-
return GenerationResponse(
562+
compiled = GenerationResponse(
547563
request_id=request.request_id,
548564
request_args=arguments.model_dump_json(),
549565
response_id=self.streaming_response_id, # use vLLM ID if available
550566
text=text,
551567
input_metrics=input_metrics,
552568
output_metrics=output_metrics,
553569
)
570+
self._validate_compiled_response(compiled)
571+
return compiled
554572

555573

556574
@OpenAIRequestHandlerFactory.register(

src/guidellm/scheduler/worker.py

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
ResponseT,
3838
)
3939
from guidellm.scheduler.strategies import SchedulingStrategy
40-
from guidellm.schemas import GenerationResponse, RequestInfo
40+
from guidellm.schemas import RequestInfo
4141
from guidellm.utils import (
4242
InterProcessMessaging,
4343
wait_for_sync_barrier,
@@ -371,14 +371,7 @@ async def _process_next_request(self, target_start: float):
371371
raise RuntimeError("Received invalid request info from backend")
372372

373373
request_info.timings.resolve_end = time.time()
374-
if not self._has_usable_terminal_response(response):
375-
request_info.error = (
376-
"[UNUSABLE_BACKEND_RESPONSE] backend resolved without a usable "
377-
"terminal response payload"
378-
)
379-
self._send_update("errored", None, request, request_info)
380-
else:
381-
self._send_update("completed", response, request, request_info)
374+
self._send_update("completed", response, request, request_info)
382375

383376
response = request = request_info = None
384377
except asyncio.CancelledError:
@@ -402,21 +395,6 @@ async def _process_next_request(self, target_start: float):
402395
if request_info is not None:
403396
self.strategy.request_completed(request_info)
404397

405-
def _has_usable_terminal_response(self, response: ResponseT | None) -> bool:
406-
"""
407-
Return whether backend resolve produced a usable terminal response.
408-
409-
A missing terminal response indicates the backend transport resolved but did
410-
not provide result payload data that can be aggregated.
411-
"""
412-
413-
if isinstance(response, GenerationResponse):
414-
has_text = bool(response.text and response.text.strip())
415-
output_tokens = response.output_metrics.total_tokens or 0
416-
return has_text or output_tokens > 0
417-
418-
return bool(response)
419-
420398
async def _dequeue_next_request(
421399
self, target_start: float
422400
) -> tuple[RequestT, RequestInfo]:

tests/unit/backends/openai/test_request_handlers.py

Lines changed: 81 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -300,9 +300,6 @@ def test_format_ignore_eos(self, valid_instances):
300300
10,
301301
5,
302302
),
303-
({"choices": [{"text": ""}], "usage": {}}, "", None, None),
304-
({"choices": [], "usage": {}}, "", None, None),
305-
({}, "", None, None),
306303
],
307304
)
308305
def test_non_streaming(
@@ -363,7 +360,6 @@ def test_non_streaming(
363360
None,
364361
None,
365362
),
366-
(["", "data: [DONE]"], "", None, None),
367363
],
368364
)
369365
def test_streaming(
@@ -397,6 +393,47 @@ def test_streaming(
397393
assert response.output_metrics.text_words == len(expected_text.split())
398394
assert response.output_metrics.text_characters == len(expected_text)
399395

396+
@pytest.mark.regression
397+
@pytest.mark.parametrize(
398+
"response",
399+
[
400+
{"choices": [{"text": ""}], "usage": {}},
401+
{"choices": [], "usage": {}},
402+
{},
403+
],
404+
)
405+
def test_non_streaming_raises_for_unusable_terminal_payload(
406+
self, valid_instances, generation_request, response
407+
):
408+
"""Test unusable non-streaming text response raises.
409+
410+
### WRITTEN BY AI ###
411+
"""
412+
instance = valid_instances
413+
arguments = instance.format(generation_request)
414+
415+
with pytest.raises(ValueError, match="UNUSABLE_BACKEND_RESPONSE"):
416+
instance.compile_non_streaming(generation_request, arguments, response)
417+
418+
@pytest.mark.regression
419+
def test_streaming_raises_for_unusable_terminal_payload(
420+
self, valid_instances, generation_request
421+
):
422+
"""Test unusable streaming text response raises.
423+
424+
### WRITTEN BY AI ###
425+
"""
426+
instance = valid_instances
427+
arguments = instance.format(generation_request)
428+
429+
for line in ["", "data: [DONE]"]:
430+
result = instance.add_streaming_line(line)
431+
if result is None:
432+
break
433+
434+
with pytest.raises(ValueError, match="UNUSABLE_BACKEND_RESPONSE"):
435+
instance.compile_streaming(generation_request, arguments)
436+
400437
@pytest.mark.smoke
401438
@pytest.mark.parametrize(
402439
("line", "expected_output"),
@@ -818,18 +855,6 @@ def test_format_multimodal(self, valid_instances):
818855
10,
819856
5,
820857
),
821-
(
822-
{"choices": [{"message": {"content": ""}}], "usage": {}},
823-
"",
824-
None,
825-
None,
826-
),
827-
(
828-
{"choices": [], "usage": {}},
829-
"",
830-
None,
831-
None,
832-
),
833858
],
834859
)
835860
def test_non_streaming(
@@ -883,12 +908,6 @@ def test_non_streaming(
883908
None,
884909
None,
885910
),
886-
(
887-
["", "data: [DONE]"],
888-
"",
889-
None,
890-
None,
891-
),
892911
],
893912
)
894913
def test_streaming(
@@ -920,6 +939,46 @@ def test_streaming(
920939
assert response.input_metrics.text_tokens == expected_input_tokens
921940
assert response.output_metrics.text_tokens == expected_output_tokens
922941

942+
@pytest.mark.regression
943+
@pytest.mark.parametrize(
944+
"response",
945+
[
946+
{"choices": [{"message": {"content": ""}}], "usage": {}},
947+
{"choices": [], "usage": {}},
948+
],
949+
)
950+
def test_non_streaming_raises_for_unusable_terminal_payload(
951+
self, valid_instances, generation_request, response
952+
):
953+
"""Test unusable non-streaming chat response raises.
954+
955+
### WRITTEN BY AI ###
956+
"""
957+
instance = valid_instances
958+
arguments = instance.format(generation_request)
959+
960+
with pytest.raises(ValueError, match="UNUSABLE_BACKEND_RESPONSE"):
961+
instance.compile_non_streaming(generation_request, arguments, response)
962+
963+
@pytest.mark.regression
964+
def test_streaming_raises_for_unusable_terminal_payload(
965+
self, valid_instances, generation_request
966+
):
967+
"""Test unusable streaming chat response raises.
968+
969+
### WRITTEN BY AI ###
970+
"""
971+
instance = valid_instances
972+
arguments = instance.format(generation_request)
973+
974+
for line in ["", "data: [DONE]"]:
975+
result = instance.add_streaming_line(line)
976+
if result is None:
977+
break
978+
979+
with pytest.raises(ValueError, match="UNUSABLE_BACKEND_RESPONSE"):
980+
instance.compile_streaming(generation_request, arguments)
981+
923982

924983
class TestAudioRequestHandler:
925984
"""Test cases for AudioRequestHandler.

0 commit comments

Comments
 (0)