fix(session): keep loop exit and part ordering robust to id wrap - #48472
Open
yixiao5428 wants to merge 1 commit into
Open
fix(session): keep loop exit and part ordering robust to id wrap#48472yixiao5428 wants to merge 1 commit into
yixiao5428 wants to merge 1 commit into
Conversation
Ascending ids encode only the low 36 timestamp bits, so they wrapped on 2026-08-14 and no longer order chronologically across the boundary. The terminal gate's parentID check never holds when the newest assistant was written by a pre-fix binary with a stale parent, so the loop issues one spurious request per run (issue anomalyco#42816, measured 31 of 251k sessions); fall back to comparing the assistant against the latest user by time.created with id as tiebreaker. Message parts were still ordered by raw part id, which reorders parts of a message whose ids straddle the wrap; order them by persisted creation time like the message pagination cursor already does. Document the encoding period on the id helpers. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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
1 participant
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.
Issue for this PR
Fixes #42816 — specifically the completion-gate fallback item. The part-ordering gap below is new (not in that issue). Items 1 and 2 of #42816 (revert cleanup ordering, localeCompare collation) are intentionally left to #42819.
Type of change
What does this PR do?
Two ordering gaps survive the August rollover fixes:
The completion gate has no chronological fallback. It requires
lastAssistant.parentID === lastUser.id. For histories written before the parentID gate landed — or with synthetic users carrying legacy short ids — the chronologically latest assistant stays parented to an older user, the equality never holds, and every run issues one spurious provider request. [BUG] Two ordering-correctness gaps remain after the ID-rollover fix #42816 measured 31 of 251k sessions in that shape. Add a fallback: the turn is also complete when the assistant precedes the latest user bytime.created(id tiebreak) —compareChronology, the same ordering the pagination cursor already uses. TheparentIDarm is unchanged, so well-formed histories behave identically.Message parts were still ordered by raw part id (
hydrate,parts()). Parts of one message whose ids straddle the wrap sort backwards. Order them by persistedtime_createdwith the id tiebreak, matching the message-level cursor. (Client stores still key parts by id — follow-up.)Two comment corrections in
id.ts: the encoding keeps the low 36 timestamp bits, so ids are not globally monotonic across the 2^36 ms period.Known tradeoff, stated openly: with the fallback, a run whose latest assistant predates a later concurrent user (assistant parented to the earlier user) exits before answering the newer user;
parentIDalone answers the newer user but never exits stale-parent histories. We chose exit — the same choice the issue reporter made.Related work: earlier attempts at this family — #39806 (full ordering rework), #42640 (time.created in
latest()/runLoop), #42684 (numeric compare inisAfter) — were closed unmerged, two of them on the description-compliance window rather than on merit; the message-level half they targeted has since landed via #40990. This PR covers the two gaps none of them reached, with two differences: both fixes are pinned to reproductions on current dev (one request on dev, zero here; wrong part order flipping to correct), and it deliberately stays out of the domains already owned by open work — revert cleanup/collation (#42819, #46974), TUI redo/child ordering (#42590, #42907, #43035), and the app store sort that #42816 raised as wanting a shared comparator — so nothing here competes with those.How did you verify your code works?
bun run typecheckinpackages/opencodebun test test/session/— 423 tests, 0 fail, including two new fixtures: one reproducing the spurious request across a pinned rollover boundary (one request on dev, zero here), one seeding parts across the wrap boundary (order flips from wrong to correct)Screenshots / recordings
N/A — no UI change.
Checklist