fix(responses): complete chat bridge reasoning lifecycle - #32777
Conversation
Greptile SummaryThis PR fixes the chat-completions → Responses API streaming bridge for providers that emit reasoning deltas before text deltas, adding the missing lifecycle events (
Confidence Score: 4/5Safe to merge for the targeted clients that track stream state by item_id; the output_index hardcoding is a pre-existing gap that this PR does not worsen. The core lifecycle changes are correct and well-tested. The output_index stays 0 for all output items regardless of order, which is non-conformant with the spec but unlikely to break Vercel AI SDK or OpenCode if they resolve events by item_id. The new tests do not assert output_index, so the gap is not caught by CI. streaming_iterator.py — output_index=0 hardcoding in the message-item OutputItemAddedEvent (line 793) and in create_content_part_added_event (line 435) when a reasoning item already occupied index 0.
|
| Filename | Overview |
|---|---|
| litellm/responses/litellm_completion_transformation/streaming_iterator.py | Fixes the reasoning → text lifecycle in the chat-completions bridge; message output item and content-part events still hardcode output_index=0 when reasoning precedes text. |
| tests/test_litellm/responses/litellm_completion_transformation/test_litellm_completion_responses.py | Adds two new mocked unit tests for reasoning lifecycle; missing output_index assertions leave a spec-compliance gap. |
Comments Outside Diff (1)
-
litellm/responses/litellm_completion_transformation/streaming_iterator.py, line 791-793 (link)output_indexstays 0 when a message item follows a reasoning itemWhen the stream produces reasoning first and text second, both
OUTPUT_ITEM_ADDEDevents are emitted withoutput_index=0— the reasoning item at line 756 and the message item here. Per the Responses API spec, distinct top-level output items must carry sequential indices (reasoning → 0, message → 1). Clients that build an output array keyed byoutput_indexwould overwrite the reasoning entry with the message entry.create_content_part_added_event()(line 435) also hardcodesoutput_index=0, so the downstreamCONTENT_PART_ADDEDevent has the same conflict. A minimal fix would be to track a running output index (e.g.self._next_output_index) and increment it each time a new output item is emitted.
Reviews (1): Last reviewed commit: "test(responses): cover reasoning stream ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if not self._sent_reasoning_summary_part_added_event: | ||
| self._sent_reasoning_summary_part_added_event = True |
There was a problem hiding this comment.
_sent_reasoning_summary_part_added_event is always in sync with sent_output_item_added_event
Both flags are set to True in the same block — the guard at line 749 (if self.sent_output_item_added_event: return) ensures the code only reaches line 768 when sent_output_item_added_event was previously False. The two flags therefore always transition together, making _sent_reasoning_summary_part_added_event functionally redundant. Consider replacing it with a reuse of sent_output_item_added_event, or document a concrete future scenario where they could diverge.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| def test_reasoning_then_text_emits_message_output_item(self): | ||
| """A text delta after reasoning must open a message item before output_text.delta.""" | ||
| from litellm.types.llms.openai import ResponsesAPIStreamEvents | ||
|
|
||
| iterator = self._make_iterator() | ||
|
|
||
| iterator._ensure_output_item_for_chunk(self._make_reasoning_chunk()) | ||
| iterator._pending_response_events.clear() | ||
| iterator._ensure_output_item_for_chunk(self._make_text_chunk()) | ||
|
|
||
| events = iterator._pending_response_events | ||
| assert len(events) == 2 | ||
| assert events[0].type == ResponsesAPIStreamEvents.OUTPUT_ITEM_ADDED | ||
| assert events[0].item.type == "message" | ||
| assert events[1].type == ResponsesAPIStreamEvents.CONTENT_PART_ADDED | ||
| assert events[1].item_id == events[0].item.id |
There was a problem hiding this comment.
output_index not asserted for the message item that follows reasoning
The test verifies that the correct event types are emitted and that item_id linkage is consistent, but it does not assert the output_index value on either the OUTPUT_ITEM_ADDED or the CONTENT_PART_ADDED event. If the spec requires the message item to use output_index=1 (because the reasoning item occupied index 0), this gap would let a wrong value go undetected. Adding assert events[0].output_index == 1 and assert events[1].output_index == 1 would make the spec expectation explicit and guard against future regressions.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
9eae219 to
4a033e0
Compare
…itellm_/revert-pr-34649-9f1755
The template tests hardcoded the model names the presets happened to ship with, so editing autorouter_presets.json to name newer models turned every preset red in the fixtures and hung six waitFor calls
…itellm_gate_owns_basedpyright_heap
…preset_test_fixtures fix(ui): derive auto-router preset tests from the bundled preset JSON
…9-9f1755 revert: "test(e2e): vendor API strategy coverage across endpoints" (BerriAI#34649)
…ider-dep-bump-5feb4a chore(deps): bump grpc and golang.org/x modules in the terraform provider
…klin-058a96 test(e2e): skip view-backed global spend probes pending LIT-5211
…pyright_heap fix(lint): move the basedpyright heap flag into the type check gate
chore(ci): promote internal staging to main
…endpoint-audit-36c1c5 feat(ui): add role capability gating, migrate Tool Policies route
…71b7b2 refactor(ui): inject the fetch client's base url instead of reading it at import
The stale-types failure already writes the regenerated schema.d.ts to the working tree, and staging it cannot introduce a new failure: the file is listed in .prettierignore and the eslint config ignores, so no lint pass sees it, and gen:api derives it purely from the Python proxy code, so a second regeneration is a no-op. The only reason left to re-run is when other checks also failed, so say exactly that in the script message and CLAUDE.md instead of prescribing an unconditional re-run.
For a backend-only commit, staging the regenerated schema.d.ts newly satisfies the ui file triggers, so the folder-wide dashboard lint budgets run locally for the first time and CI's frontend-lint job (budgets plus knip) activates on the PR. Those can only fail from pre-existing dashboard-tree state, never from the regenerated file, but the guidance should say so instead of implying a re-run is always redundant.
They were taking too long
chore: remove unused .flake8 config and flake8 dev dependency
…_types_no_rerun chore: stop advising pre-commit and bootstrap
…rriAI#35831) A three-segment token presented while `general_settings.enable_jwt_auth` is unset is never treated as JWT-shaped, so it falls through to the virtual-key path and is rejected for not starting with 'sk-'. That reads as a missing key in the verification table and sends the operator off to inspect virtual keys, when the real cause is one missing config line. The rejection now names `enable_jwt_auth`, appended to the existing text so the Prometheus invalid-key filter and the admin UI keep matching what they match today. The hint claims only that the key is JWT-shaped. Segment count cannot tell a JWT from any other dotted credential, so asserting the key IS a JWT would swap one confident misdiagnosis for a narrower one. The enterprise gate on that same path raised a bare `ValueError`, which the terminal handler turns into a 401. Every sibling enterprise gate answers 403, and a 401 tells the client to retry with a better credential, which no credential can satisfy while the install is unlicensed. It now raises a 403 `ProxyException` like the SSO gate does.
) * feat(auto-router): make reminder marker pair configurable Some harnesses inject internal context using their own marker pair instead of Claude Code's <system-reminder>/</system-reminder> convention, and some send it as a separate follow-up user message rather than inline with the ask. Both cases fall out of the same root cause: the router's marker-matching is hardcoded, so foreign markers never strip to empty and the reminder-only turn wins "newest human ask" selection instead of being skipped. Add an optional reminder_markers field to ComplexityRouterConfig so operators can override the (open, close) pair via proxy config, with the existing skip-when-empty selection logic handling both cases once the markers match. * test(auto-router): drop unsolicited comments from the reminder-markers regression test Per Greptile review on BerriAI#35874: no comments unless explicitly requested.
The dashboard pins engines node >=24.14.1 with engine-strict, so make bootstrap dies with EBADENGINE on any shell whose default node is older. Wrap the npm install in scripts/with_dashboard_node.sh: it execs the command as-is when node already meets the floor, otherwise activates the .nvmrc version via nvm or fnm, and fails fast with install instructions when neither manager exists
…floor fix(bootstrap): switch to the dashboard node floor via nvm or fnm
… connected (BerriAI#36041) * warn at startup when a proxy-wide budget is set but no DB is connected litellm.max_budget is only enforced via DB-loaded global spend, so a DB-less proxy silently ignores it. Log a one-time startup warning. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(proxy): inject max_budget into DB-less budget warning Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(proxy): cover DB-less budget warning startup call Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(proxy): pin DB-less budget warning call site Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(proxy): stabilize budget warning call-site pin Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: tin <tin@berri.ai> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
BerriAI#35866) * fix(proxy): promote caller metadata trace fields into litellm_metadata Routes in LITELLM_METADATA_ROUTES keep the caller's metadata as a provider passthrough field and track proxy state in litellm_metadata, which is the dict the logging integrations read. The caller's trace_id, session_id, trace_user_id and trace_metadata therefore never reached any callback on /v1/responses, /v1/messages, /v1/batches or /v1/files, and mask_input / mask_output were dropped with them so a caller asking for redaction had their prompt logged in full. Promote an explicit allow-list of those fields from the requester_metadata snapshot into litellm_metadata, never overwriting a value already set so header-derived ids keep precedence. Trace-mutation controls (existing_trace_id, update_trace_keys) and trace_public are deliberately excluded: langfuse applies them to an arbitrary caller-chosen trace with no ownership check. tags is excluded because per-tag budget enforcement runs earlier, at auth time. This covers providers with a native Responses API config. Providers reaching /v1/responses through the chat-completions bridge need the companion change to get_litellm_params. * ci: retrigger workflows
…ider-sync-0-4-0 feat(terraform): sync provider 0.3.0 from the mirror and cut 0.4.0
…I#36110) The shared `timeout` guardrail param already parsed into LitellmParams, but the Zscaler initializer never forwarded it and _send_request hardcoded a 5 second constant, so a configured value was silently ignored and slow scans failed with `Timeout passed=5` regardless of config. Forward litellm_params.timeout through to the HTTP call, keep 5 seconds as the default, fall back to it for non-positive values, and declare the field on the config model so the dashboard renders it.
…erriAI#36105) get_litellm_params returned metadata=None whenever only litellm_metadata was supplied, which overwrote the fallback function_setup had already applied and left litellm_params["metadata"] empty. On the /v1/responses completion-transformation bridge, used by every provider without a native Responses API config, and on /v1/messages, that discarded the caller's trace fields a second time after the proxy had promoted them. Resolve metadata to a copy of litellm_metadata when metadata is empty, guarding on isinstance because the proxy leaves an unparseable litellm_metadata string in place and a null metadata would otherwise suppress the backfill and break the merge. update_from_kwargs copies rather than aliases for the same reason: on these routes it is handed the caller's provider-bound dict and would otherwise write user_api_key_auth into it.
…sts (BerriAI#36121) * fix(proxy): re-assert the authenticated identity on passthrough requests The passthrough merges the client's litellm_metadata into the request metadata and then re-asserts only user_api_key and the parent span. Every other identity field the spend and budget pipeline reads stays whatever the request body set, so a body carrying user_api_key_user_id, user_api_key_team_id, user_api_key_org_id or user_api_key_end_user_id charges that user, team, org or end user instead of the caller. Re-assert the whole sanitized identity after the merge, so the client's copy of any of those fields is overwritten by the authenticated key's own values. * test(passthrough): assert no authenticated identity field is client settable The existing regression names seven fields; the re-assertion covers every field get_sanitized_user_information_from_key returns, which is twenty today. Derive the set from the helper so a field added to StandardLoggingUserAPIKeyMetadata is covered without touching the test. Two of the twenty were not covered before, including user_api_key_hash, which is distinct from user_api_key and was client settable.
…n-bump-b6f5a2 chore: bump litellm-enterprise 0.1.53 -> 0.1.54, litellm-proxy-extras 0.4.83 -> 0.4.84
…nnon-ffc974 test(router): assert the auto-router max_input_chars kwarg
Closes GHSA-5p4m-2wfm-xmqj (CVSS 7.5), flagged by osv-scan against ui/litellm-dashboard/package-lock.json. js-yaml is pinned by an exact npm override, so the override and the lock move together. Dev-only dependency: js-yaml reaches the tree through eslintrc, knip and @redocly/openapi-core, none of which ship in the built dashboard. 4.3.1 published 2026-07-31, clear of the 3-day min-release-age cooldown.
Closes GHSA-6hr6-w5qg-qmwg (CVSS 5.3), the second finding from the same osv-scan run as the js-yaml bump. Bundled here so the scan goes green in one merge instead of two PRs that each stay red on the other's finding. Re-derived with `uv lock --upgrade-package h2` rather than taking the Dependabot lock wholesale: that keeps the diff to the two packages that actually move (h2, plus hpack 4.2.0 which h2 4.4.1 requires) and leaves the `exclude-newer` snapshot a real timestamp. h2 4.4.1 published 2026-08-03, hpack 4.2.0 on 2026-06-23 — both clear of the 3-day exclude-newer window.
…erge-ven7h6 fix(managed_files): skip unparseable rows when listing managed files
…client (BerriAI#35978) create_a2a_client took the raw client off a process-wide cached handler and called headers.update() on it, then leaned on folding the header set into the cache key (through the unrelated disable_aiohttp_transport field) to keep one caller's credentials away from the next. Per-caller headers now ride with each request through the a2a SDK's call context, and the agent card fetch gets them through resolver_http_kwargs, so the shared client is never written to and its cache key no longer varies by header set. Since the proxy puts a fresh trace id in every request's headers, that key previously changed on every call, giving each request its own httpx client and flushing the 200-entry client cache that every other provider shares. All A2A callers on one timeout now reuse a single pooled client. Sharing that client also means sharing its httpx cookie jar, which httpx fills from every Set-Cookie and replays on any later request to a matching domain, so one agent's session cookie would arrive at another agent on the same host. The pooled client now carries a cookie policy that stores and sends nothing, which neither litellm nor the a2a SDK relies on: the SDK's auth interceptor skips cookie-borne API keys outright.
build(deps): bump h2 to 4.4.1 and js-yaml to 4.3.1
chore: promote staging to main
4a033e0 to
960c35c
Compare
|
|
Credit PR BerriAI#32777 for identifying the missing lifecycle event
14 participants
Summary
Fixes the chat-completions -> Responses API streaming bridge when a provider emits reasoning deltas followed by text deltas.
Strict Responses API clients such as Vercel AI SDK / OpenCode track stream state by
item_id,summary_index, andcontent_index. The bridge currently can emit reasoning/text deltas without the lifecycle events those clients need to register the corresponding parts.Problem
For reasoning-capable chat-completions providers, the bridge can produce invalid Responses streams:
response.reasoning_summary_text.deltawithout a matchingresponse.reasoning_summary_part.addedThese shapes surface as strict-client errors like:
reasoning part <id>:0 not foundtext part <id> not foundChanges
response.reasoning_summary_part.addedbefore reasoning deltasresponse.reasoning_summary_text.deltasummary_index=0on reasoning deltasRelation to other PRs
This is separate from #32519, which guards terminal
choices: []chunks. That fix is complementary but not included here.Related prior art:
content_part.addedfor message items/responsesstreams/responsespassthroughThis PR targets the chat-completions -> Responses bridge path.
Tests