Repository navigation
feat(chat_models): record the response id and chain calls with use_previous_response_id - #349
Conversation
447967c to
1de8ee1
Compare
Akshay Dongare (Akshay-Dongare)
left a comment
There was a problem hiding this comment.
Thanks, the id on every path of both classes and the 1.102.0 floor are right: the new id assertions fail on litellm 1.101.0 and pass on 1.102.0. use_previous_response_id needs these before I can merge.
Blocking
- Chained turns lose the system prompt. litellm's Responses bridge sends a leading system message with str content as
instructions, andinstructionsare not carried over byprevious_response_id(see theinstructionsdocstring on openai'sResponseCreateParams). The trim in_last_messages_with_response_id(litellm.py:131) removes it, so from turn 2 on the model runs without its system prompt. ChatOpenAI gets away with the same trim only because it sends system messages as stored input items. Keep every message in the trimmed prefix that_convert_message_to_dictturns into rolesystemwith str content (SystemMessageandChatMessage(role="system")), ahead of the kept turns. List content already goes as a stored input item, so leave that out. - Only chain on
resp_ids. This PR puts an id on every reply, so a Chat Completions reply in the history (chatcmpl-..., or''from an OpenAI-compatible stream) becomesprevious_response_id, and everything before it is dropped. Match ChatOpenAI: chain only whenisinstance(response_id, str) and response_id.startswith("resp_"), otherwise keep walking back. - Refuse Router groups that don't share a response store. Two deployments with different
api_base, two Azure regions, or OpenAI plus Azure all pass the Responses check in_route_to_responses_api(litellm_router.py:295), and a chained call can land on a deployment that never stored the response: 12 to 15 of 20 chained calls in an offline repro. litellm'sresponses_api_deployment_checkcan't help, since the bridge returns the decoded id. RaiseValueErrorbefore any request._group_endpointwith_reasoning_endpointalready answers "one endpoint and set of credentials?", but evaluate it on params withoutprevious_response_id: that key isn't in_NON_ROUTING_PARAMS, so with it present every call reads as unknown reach and even a single deployment would be refused.
Also needed
-
Refusals name the wrong flag. With only
use_previous_response_idset, errors readuse_responses_api=True, but ...(litellm.py:1610,litellm_router.py:307,315,323,336). Name the flag the caller set and a fix that keeps chaining; the Router's "without it, ... still reaches the Responses API" hint means giving chaining up. -
The flag per call, via
bind()or inmodel_kwargsis not consumed. It is never popped, so it lands in the Chat Completions request body and nothing is trimmed. Pop it in_merge_call_paramsand raise, saying to set it on the model (ChatOpenAI reads the field only). Restore the deleted sentence thatuse_responses_apiis read from the call,model_kwargsor the field, since it's still true, and scope the new sentence (litellm.py:1636) to the field. -
A caller-supplied
previous_response_iddoesn't match the trim. A per-call id is sent while the history is trimmed at the history's id, and amodel_kwargsid loses to the history's. Pick one rule for all sources: a caller id turns the trim off, or the history's id always wins. -
Refuse fallbacks when chaining on
ChatLiteLLM. Withfallbacksinmodel_kwargs, the fallback model gets only the trimmed turn plus aprevious_response_idit can't use. The Router already refuses this through_unknown_reach. -
Set the stream id once. Drop the four
setdefault("id", ...)blocks and their comment (litellm.py:2002,2106,litellm_router.py:583,696). litellm keeps one id per stream, so the comment doesn't hold, and after a Router mid-stream fallback the per-chunk ids merge intochatcmpl-PRIMARYresp_BACKUP.... Put the id on thefinish_reasonchunk so a fallback's id wins, record it only when truthy, and give the reply id a different name from the request'sresponse_id. -
Chained replies drop their encrypted reasoning items.
previous_response_idmakes_reasoning_endpointreturnNone, so_keep_reasoning_itemsdiscards them. Leave out only the id the connector set when computing it; adding the key to_NON_ROUTING_PARAMSwould start re-sending items for callers who pass their own id with full history. -
Add
use_previous_response_idto_identifying_paramsnext touse_responses_api(litellm.py:2384). Today an LLM cache serves a flag-off reply to a flag-on call with the same history. -
Docs. The field docstring says it "always routes the call through the Responses API" (
litellm.py:1446); onChatLiteLLMRouterit checks the deployments instead, so say so there asuse_responses_apidoes. Also state: system messages are re-sent, onlyresp_ids chain, chaining needs stored responses (store=Falsebreaks it), and Router groups across stores are refused. -
Keep the 1.101.0 streamed-cost reason in the
pyproject.tomlcomment beside the new one; it still holds. -
Tests, through
serve_http, on both classes:- chaining on
streamandastream(today, removing the trim from all four stream methods leaves the suite green); payload["instructions"]on a chained turn, includingChatMessage(role="system");- a
chatcmpl-reply after aresp_one, an empty-string id, a history with no id, and the flag off; - the two-store Router refusal, plus a single-deployment group that still chains;
- refusal messages that name
use_previous_response_id; - a stream whose chunks carry two ids, merging to exactly one of them.
test_stream_keeps_a_responses_api_idmocks one hand-built chunk, so it never merges anything and passes on 1.101.0. Send it through the bridge. - chaining on
-
Retitle to
feat(chat_models): record the response id and chain calls with use_previous_response_id. The title becomes the changelog line, and chat model features are scopedchat_models.
|
I’ve updated the Responses API chaining implementation in both |
…s it (#353) Adds a **Sort imports** workflow that a maintainer runs before merging: give it a PR number and it sorts that PR's imports with ruff (`I001`) and commits the fix to the PR branch, forks included. Start it from the Actions tab or with `gh workflow run sort-imports.yml -f pr=<number>`. The built-in Actions token cannot push to a fork, and every outside contribution in the last 100 PRs came from one, so the push uses a project-owned GitHub App. A third-party app like autofix.ci would hold a write token for every branch except `main`, and triggering on every PR would let a stranger start a job holding one. Here only someone with write access can start the job, and the App key lives in an environment only `main` can use. The job reads the PR's head commit as plain files and runs `main`'s locked ruff over them, so no PR code executes. The token is minted only when there is a fix, with Contents write alone, and a plain push means a branch that moved meanwhile rejects it. Closed PRs, deleted forks and forks without maintainer edits are refused. **One-time setup before the first run:** 1. Create a GitHub App with repository permission Contents: Read and write and no webhook, and install it on this repository only. 2. Create a `sort-imports` environment limited to `main`, with the App's client ID as variable `SORT_IMPORTS_CLIENT_ID` and a private key as secret `SORT_IMPORTS_PRIVATE_KEY`. **Verified.** A dispatch workflow only runs from `main`, so each step's script ran locally, verbatim from the YAML: - the lookup resolves fork PR #349's head, refuses closed, deleted-fork and edits-off PRs, and keeps a branch name full of shell metacharacters as data - an unsorted import committed on #349's head comes back byte-identical; a clean head is left alone - a dry-run push fast-forwards onto this branch as the bot, and is rejected once the branch has moved - actionlint and zizmor, auditor persona with online audits, report nothing An App installed only upstream can push to fork branches: autofix.ci's does on mermaid-js/mermaid#8297 and mermaid-js/mermaid#8276.
|
Hi Akshay Dongare (@Akshay-Dongare), could you go through the PR whenever possible and let me know if there are any other changes to be made? |
Fixes #305
Summary
Adds full Responses API response-id chaining for
ChatLiteLLMandChatLiteLLMRouter: Responses API replies now expose LiteLLM’s upstreamresp_...id inAIMessage.response_metadata["id"]for sync, async, streaming, and non-streaming calls. Addsuse_previous_response_id=True, which finds the most recent response id in message history, sends it asprevious_response_id, omits already server-stored history, and forces the request through LiteLLM’s Responses API route; also raises the LiteLLM minimum to1.102.0, the first version that returns the upstream id.Verification
python -m compileall -q langchain_litellm tests/unit_tests/test_litellm.py tests/unit_tests/test_router.pygit diff --checkAdded focused unit coverage for direct and Router chaining, response-id propagation, and streamed response ids.