fix: allow TITO session rollback to the empty checkpoint - #1826
Merged
Conversation
Shi-Dong
requested review from
fzyzcjy,
guapisolo,
jybsuper,
maocheng23 and
yueming-yuan
as code owners
July 27, 2026 16:47
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Merged
4 tasks
Shi-Dong
changed the base branch from
main
to
refactor/session-generated-checkpoints
July 27, 2026 23:08
A retry of the very first turn has no assistant message inside the matched prefix, so the rollback path rejected it outright and every later request on that session returned 400. Any harness whose HTTP client re-sends a request poisons the session on turn 1 this way — the OpenAI SDK retries 5xx, 408, 409, 429 and transport errors on its own, below LiteLLM — and the whole trajectory then scores reward 0. Treat "no assistant in the matched prefix" as a rollback to the empty checkpoint rather than an error: session state resets and the caller re-renders the prompt from scratch, so turn 1 regenerates the same way any later turn does. MAX_ASSISTANT_ROLLBACK_STEPS still bounds the discard, so a session that has stored more than one assistant is still rejected and left untouched.
guapisolo
force-pushed
the
shi/tito-rollback-to-empty-checkpoint
branch
from
July 28, 2026 07:13
0aff58e to
8a301c1
Compare
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A TITO session permanently rejects every request once a client re-sends the very first turn. The session becomes unusable and the whole trajectory scores reward 0.
The failure
_try_detect_and_rollback_to_assistant_checkpointrolls a session back to the last assistant message inside the prefix that the incoming request still shares with the stored history. Retrying the first turn is the one case where that prefix cannot contain an assistant:[user, assistant₁][user]match_lenis 1 andstored[0]is a user message, so there was no rollback target and the request was rejected with:From that point the session is poisoned. The stored history still holds an assistant the client does not know about, so every subsequent request diverges the same way and 400s again. In the agentic run that surfaced this, 1446 of 1504
/sessions/*/v1/chat/completionscalls returned 400 and every trial ended with reward 0.What re-sends the first turn
Tracing one session end-to-end in that run gives an exact account. Its access log is one
200, then nine400s — and the200is on a different connection (source port46994) from every one of the400s (all on47040):200and stores[user, assistant₁].max_retriesre-sends the request byte-for-byte on a fresh connection, which is whymatch_len == 1. It gets the400.BadRequestErroris not retryable, so it propagates up into two nested tenacity decorators — harbor'sLiteLLM.call(3 attempts) and terminus-2's_query_llm(3 attempts, no backoff). Those 3 × 3 = 9 attempts are the nine400s.The counts across the whole run match that structure exactly: 164 distinct sessions, each with precisely nine
400s. That uniformity is what rules out flakiness — this is a deterministic retry cascade, not an intermittent one. The200plus nine400s also means ten HTTP requests against nine tenacity slots, which is the extra SDK-level re-send in step 3.The trial artifacts corroborate step 2 from the agent's side:
trajectory.jsoncontains a single step withsource: "user"and no assistant at all,final_metricsreportstotal_completion_tokens: 0, andtrial.loglogsUnknown Error in LLM interaction: ... 400 ...exactly three times — once per_query_llmattempt. The agent never saw a usable completion, even though the server had produced and stored one.So the trigger is a client-side re-send of a request the server considered successful. Why the client fails to read that first response is a separate defect, still under investigation, and not fixed here. What is fixed here is that the re-send is unrecoverable.
Ruled out so far, for whoever picks that up:
_DROP_RESPONSE_HEADERSalready stripscontent-length/transfer-encoding/content-encoding, and the deployed build has it.finish_reason == "length"would makeLiteLLM.callraiseOutputLengthExceededError, which is a genuine verbatim-re-send path (and one that_query_llm's tenacity retries, since it only excludesContextLengthExceededError). But no trial artifact or server log contains any truncation signal, so it is not what fired here.stream, andLiteLLM.callraises onCustomStreamWrapper, so the plain-JSON branch is the one in use.5xx. Zero timeouts anywhere, and no5xxprecedes any rollout400.Two corrections to an earlier revision of this description, which got the trigger wrong:
5xxcaused an SDK retry. It did not. TheRetrying request to /chat/completions in 0.409547 secondsline and the500s I quoted came from a synthetic probe I ran myself at 06:51:56, sixteen minutes after the failure, against an engine that had already crashed — not from the rollouts. No5xxand no timeout precedes any rollout400.response_length_exceeded_policyisabortin this run (HARBOR_RESPONSE_LENGTH_POLICY=abort), so theregeneratepath was not involved either.That second path is still worth closing, though:
response_length_exceeded_policy="regenerate"re-sends the history without the truncated assistant turn and relies on exactly this rollback to discard it (the comment in harbor'sterminus_2.pysays so). A response-length overflow on turn 1 walks into the same hole by design.The fix
Treat "no assistant in the matched prefix" as a rollback to the empty checkpoint instead of an error. Session state resets, and
prepare_pretokenizedre-renders the prompt from scratch — exactly what it already does for a genuine first turn. Turn 1 now regenerates the same way any later turn does.That required moving the rollback check in
prepare_pretokenizedabove the "nothing stored yet" early return, because the rollback itself is what empties the session.MAX_ASSISTANT_ROLLBACK_STEPSstill bounds the discard, so this only succeeds while exactly one assistant has been stored. A session holding two or more stored assistants is still rejected, with state left untouched.The docs line describing the append-only contract is updated to match.
Scope
This is a robustness fix, not a root-cause fix. A session has to survive a duplicate turn-1 request whatever produced it — at-least-once delivery is normal HTTP client behaviour, and today the first such retry on a session is unrecoverable and costs the entire trajectory.
The guard is also not a recent regression: it arrived together with
_try_detect_and_rollback_to_assistant_checkpointin #783 (2026-03-25), andMAX_ASSISTANT_ROLLBACK_STEPSin #857 (2026-04-02). The hole has been latent since March; it only became fatal once a harness started retrying turn 1.Test plan
tests/fast/router/test_linear_trajectory.py— 42 passed. Replaced the test that asserted the old rejection, and added coverage for a verbatim first-turn re-send (regenerates and commits cleanly) plus the >1-assistant case still raising with state intact.tests/fast/router tests/fast/utils— 2585 passed, 1 skipped. No regressions.black,isortandruff checkclean.rollout/raw_reward.