From a215b78d99ea2f853e9bfdf07911f1f94df50ca6 Mon Sep 17 00:00:00 2001 From: "ZOU Yi (BD/SWD-WDE1)" Date: Mon, 14 Sep 2026 11:05:04 +0800 Subject: [PATCH] fix(sap): address review feedback on stream chunk validation Type the chunk payload as dict[str, object] and narrow choices with an isinstance guard before iterating, fix the in-place mutation suppression that claimed the loop never mutated, build the litellm Usage with model_validate instead of spreading a dict[str, Any], and make the usage regression test go through model_dump_json, the path that actually raised MockValSer. model_dump() self-heals via __getattr__, so the old test passed on unfixed code. Carry the same typing into to_openai_chunk and the payload literals it validates, so no basedpyright rule counts higher than before the change. --- litellm/llms/sap/chat/handler.py | 50 ++++++++++--------- .../llms/sap/chat/test_sap_chat_calls.py | 5 +- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/litellm/llms/sap/chat/handler.py b/litellm/llms/sap/chat/handler.py index 2867b64057b..d0be0086214 100755 --- a/litellm/llms/sap/chat/handler.py +++ b/litellm/llms/sap/chat/handler.py @@ -49,14 +49,16 @@ class _StreamParser: @staticmethod def _validate_chunk( - payload: dict, # mutable-ok: normalized in place (pops empty logprobs) before validation + payload: dict[str, object], # mutable-ok: normalized in place (pops empty logprobs) before validation ) -> OpenAIChatCompletionChunk: - for choice in payload.get("choices") or []: # mutable-ok: only iterated, never mutated - if isinstance(choice, dict) and not choice.get("logprobs"): - choice.pop("logprobs", None) + choices: Final = payload.get("choices") + if isinstance(choices, list): + for choice in choices: + if isinstance(choice, dict) and not choice.get("logprobs"): + choice.pop("logprobs", None) # mutable-ok: pops the logprobs key in-place before model_validate chunk = OpenAIChatCompletionChunk.model_validate(payload) if chunk.usage is not None: - chunk.usage = Usage(**chunk.usage.model_dump()) + chunk.usage = Usage.model_validate(chunk.usage.model_dump()) return chunk @staticmethod @@ -68,25 +70,24 @@ class _StreamParser: if not orc: return None - return _StreamParser._validate_chunk( - { - "id": orc.get("id") or evt.get("request_id") or "stream-chunk", - "object": orc.get("object") or "chat.completion.chunk", - "created": orc.get("created") or evt.get("created") or _now_ts(), - "model": orc.get("model") or "unknown", - "choices": [ - { - "index": c.get("index", 0), - "delta": c.get("delta") or {}, - "finish_reason": c.get("finish_reason"), - } - for c in (orc.get("choices") or []) - ], - } - ) + payload: Final[dict[str, object]] = { + "id": orc.get("id") or evt.get("request_id") or "stream-chunk", + "object": orc.get("object") or "chat.completion.chunk", + "created": orc.get("created") or evt.get("created") or _now_ts(), + "model": orc.get("model") or "unknown", + "choices": [ + { + "index": c.get("index", 0), + "delta": c.get("delta") or {}, + "finish_reason": c.get("finish_reason"), + } + for c in (orc.get("choices") or []) + ], + } + return _StreamParser._validate_chunk(payload) @staticmethod - def to_openai_chunk(event_obj: dict) -> OpenAIChatCompletionChunk | None: + def to_openai_chunk(event_obj: dict[str, object]) -> OpenAIChatCompletionChunk | None: """ Accepts: - {"final_result": } (IMPORTANT: this is just another chunk, NOT terminal) @@ -102,7 +103,10 @@ class _StreamParser: # FINAL RESULT IS *NOT* TERMINAL: treat it as the next chunk if "final_result" in event_obj: - fr: Final = event_obj["final_result"] or {} + final_result: Final = event_obj["final_result"] + if not isinstance(final_result, dict): + return None + fr: Final[dict[str, object]] = final_result # ensure it looks like an OpenAI chunk if "object" not in fr: fr["object"] = "chat.completion.chunk" diff --git a/tests/test_litellm/llms/sap/chat/test_sap_chat_calls.py b/tests/test_litellm/llms/sap/chat/test_sap_chat_calls.py index 170354dae7e..90b4b39582b 100644 --- a/tests/test_litellm/llms/sap/chat/test_sap_chat_calls.py +++ b/tests/test_litellm/llms/sap/chat/test_sap_chat_calls.py @@ -1,3 +1,4 @@ +import json import httpx from unittest.mock import patch, PropertyMock @@ -267,7 +268,7 @@ def test_validate_chunk_without_usage_keeps_none(): assert chunk.usage is None -def test_validated_usage_survives_nested_model_dump(): +def test_validated_usage_survives_nested_model_dump_json(): from litellm.llms.sap.chat.handler import _StreamParser from litellm.types.utils import ModelResponseStream @@ -276,7 +277,7 @@ def test_validated_usage_survives_nested_model_dump(): model_response = ModelResponseStream() setattr(model_response, "usage", chunk.usage) - dumped = model_response.model_dump() + dumped = json.loads(model_response.model_dump_json()) assert dumped["usage"]["total_tokens"] == 62528