From 3403889528376a23fa22cad1ba64cfa494d280da Mon Sep 17 00:00:00 2001 From: Tim Pietrusky Date: Thu, 23 Apr 2026 11:28:50 +0200 Subject: [PATCH] fix: address review comments on responses/messages handlers and lmcache guard - engine.py: drop UnboundLocalError-prone isinstance(response, ...) checks in except blocks of _handle_responses_request and _handle_messages_request; emit SSE-shaped error frames mid-stream instead of raw dicts; add missing blank line between handlers. - engine_args.py: restructure LMCache HMA guard so the warning branch is actually reachable when user explicitly sets disable_hybrid_kv_cache_manager=False, and correct the inverted message (HMA must be disabled = True). - requirements.txt: drop stray whitespace in transformers version specifier. --- builder/requirements.txt | 2 +- src/engine.py | 41 +++++++++++++++++----------------------- src/engine_args.py | 19 +++++++++++-------- 3 files changed, 29 insertions(+), 33 deletions(-) diff --git a/builder/requirements.txt b/builder/requirements.txt index 1d7e047..d03f3a2 100644 --- a/builder/requirements.txt +++ b/builder/requirements.txt @@ -9,7 +9,7 @@ typing-extensions>=4.8.0 pydantic pydantic-settings hf-transfer -transformers>= 4.57.0,< 5 +transformers>=4.57.0,<5 bitsandbytes>=0.45.0 kernels torch-c-dlpack-ext diff --git a/src/engine.py b/src/engine.py index c60dc3b..8c99326 100644 --- a/src/engine.py +++ b/src/engine.py @@ -410,13 +410,10 @@ class OpenAIvLLMEngine(vLLMEngine): extra={"request_id": request_id}, exc_info=True ) - if isinstance(response, ErrorResponse): - yield response.model_dump() - else: - yield create_error_response( - "Internal server error during response generation", - err_type="InternalServerError" - ).model_dump() + yield create_error_response( + "Internal server error during response generation", + err_type="InternalServerError" + ).model_dump() return if isinstance(response, (ErrorResponse, ResponsesResponse)): @@ -436,10 +433,12 @@ class OpenAIvLLMEngine(vLLMEngine): extra={"request_id": request_id}, exc_info=True ) - yield create_error_response( + error_payload = create_error_response( "Streaming response failed", err_type="InternalServerError" - ).model_dump() + ).model_dump_json() + yield f"event: error\ndata: {error_payload}\n\n" + async def _handle_messages_request(self, openai_request: JobInput): request_id = getattr(openai_request, "request_id", "unknown") @@ -470,19 +469,12 @@ class OpenAIvLLMEngine(vLLMEngine): extra={"request_id": request_id}, exc_info=True ) - if isinstance(response, ErrorResponse): - error_type = getattr(response, "type", "internal_error") - error_message = getattr(response, "message", str(e)[:200]) - yield AnthropicErrorResponse( - error=AnthropicError(type=error_type, message=error_message) - ).model_dump() - else: - yield AnthropicErrorResponse( - error=AnthropicError( - type="internal_error", - message="Failed to generate messages" - ) - ).model_dump() + yield AnthropicErrorResponse( + error=AnthropicError( + type="internal_error", + message="Failed to generate messages" + ) + ).model_dump() return if isinstance(response, ErrorResponse): @@ -507,9 +499,10 @@ class OpenAIvLLMEngine(vLLMEngine): extra={"request_id": request_id}, exc_info=True ) - yield AnthropicErrorResponse( + error_payload = AnthropicErrorResponse( error=AnthropicError( type="internal_error", message="Error while streaming messages" ) - ).model_dump() + ).model_dump_json() + yield f"event: error\ndata: {error_payload}\n\n" diff --git a/src/engine_args.py b/src/engine_args.py index c810d59..8ed132c 100644 --- a/src/engine_args.py +++ b/src/engine_args.py @@ -442,14 +442,17 @@ def get_engine_args(): ) lmcache_detected = lmcache_via_offload or lmcache_via_transfer - if lmcache_detected and not args.get("disable_hybrid_kv_cache_manager"): - args["disable_hybrid_kv_cache_manager"] = True - logging.info("LMCache detected: automatically setting disable_hybrid_kv_cache_manager=True") - elif lmcache_detected and args.get("disable_hybrid_kv_cache_manager") is False: - logging.warning( - "LMCache configuration detected but disabled: " - "disable_hybrid_kv_cache_manager must be False when using LMCache" - ) + if lmcache_detected: + current = args.get("disable_hybrid_kv_cache_manager") + if current is False: + logging.warning( + "disable_hybrid_kv_cache_manager=False conflicts with LMCache; " + "overriding to True (HMA must be disabled when using LMCache)" + ) + args["disable_hybrid_kv_cache_manager"] = True + elif current is None: + args["disable_hybrid_kv_cache_manager"] = True + logging.info("LMCache detected: automatically setting disable_hybrid_kv_cache_manager=True") except Exception as e: logging.error( "Failed to check LMCache configuration: %s",