fix(extensions): reject truly empty length-truncated Chat Completions - #4912
fix(extensions): reject truly empty length-truncated Chat Completions#4912fscfede-beep wants to merge 1 commit into
Conversation
Live-provider compatibility evidenceFollowing the review concern on closed #4886, I ran the same non-streaming request end-to-end through both affected adapters against the same real local OpenAI-compatible provider. Provider: Ollama 0.32.14, model Raw provider result: Pinned code-state comparison:
Additional verification on the fix: 96 affected tests passed; Ruff PASS; Pyright 0 errors/0 warnings/0 informations; mypy PASS; The experiment demonstrates the compatibility regression and that the reasoning-aware guard restores the prior provider behavior while still rejecting truly empty Receipt: |
|
@codex review Please review the current head |
|
Note You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
fscfede-beep
left a comment
There was a problem hiding this comment.
Independent review gate check on the current head 504dcf93a52fe86732509a83ebb24f986b9f9133: the proposed condition is appropriately narrow—raise only for truly empty non-streaming length-truncated responses, while preserving reasoning-only, refusal, tool-call, and partial-text responses. The PR documents 96 focused tests and real-provider AnyLLM/LiteLLM + Ollama compatibility evidence. No new review threads are currently present. I am not treating contributor-side validation as upstream CI PASS; the current workflow is action_required with no jobs. Please evaluate the provider compatibility evidence and semantics on the upstream CI/review gate.
|
Thanks for adding the real-provider compatibility evidence. The Ollama comparison addresses the reasoning-only concern from #4886: both the base and this PR return a reasoning item. However, that experiment does not exercise the truly empty response targeted by the new exception. It demonstrates compatibility with existing behavior, rather than a user-visible problem that this change fixes. The original reproduction in #4885 supplies a configured response, so the real-world impact needed for this behavior change remains unestablished. I'm going to close this PR for now. We can reconsider with a real-provider example containing no text, refusal, tool calls, or reasoning, together with the concrete application consequence that requires truncation-specific handling. |
|
Targeted maintainer-review note: this revision is intentionally narrower than #4886. The guard raises only for finish_reason="length" with no text, refusal, tool calls, or provider reasoning; reasoning-only responses remain valid. Live-provider compatibility was exercised against Ollama 0.32.14 through both AnyLLM and LiteLLM (no mock): base and this head preserve ResponseReasoningItem, while #4886 raised ModelBehaviorError. 96 affected tests + Ruff/Pyright/mypy/diff-check pass locally. Please evaluate the semantic boundary and whether the live-provider evidence is sufficient for merge. |
Summary
Successor to closed #4886 / addresses closed #4885.
The original change correctly rejected truly empty non-streaming Chat Completions responses ending with
finish_reason="length", but the first implementation also rejected reasoning-only responses. This PR preserves the intended empty-response protection while retaining provider reasoning that is already surfaced by AnyLLM/LiteLLM.Behavior
Raise
ModelBehaviorErroronly when all of these hold:finish_reason == "length"reasoning_content/thinking_blocks, or AnyLLM normalized reasoning)Preserve partial text, refusals, tool calls, and reasoning-only responses.
Verification
Local regression suite for the affected adapters:
96 passed.Static verification:
git diff --check: PASSReal-provider compatibility test, same provider/model/prompt and
max_tokens=8across pinned code states:rumbo-qwen3-4b-q4km:latest1d471a4775bf2f40179f411824da383deb4c3fca-> oneResponseReasoningItem7a437cea5bf7a2e10194ee7239a25d87f59f31ad->ModelBehaviorError504dcf93a52fe86732509a83ebb24f986b9f9133-> oneResponseReasoningItemBoth AnyLLM and LiteLLM were tested end-to-end through the real Ollama OpenAI-compatible endpoint. The raw provider returned
finish_reason="length", empty content, and non-empty reasoning.The original PR was closed after the maintainer requested real-world compatibility evidence; that evidence is now available and the incompatible behavior has been corrected.