release #5564: mint stable per-message ids for large-session fork alignment - #5595
Conversation
WebUI keeps two parallel arrays per session: messages (display transcript) and context_messages (what's sent to the model). In large/compacted sessions they diverge. Forking / in-place truncation copies a prefix of both and relies on truncate_context_for_display_keep() to translate a display keep-index into the matching context index — an aligner that prefers a stable per-row `id`. But the model-context rows carried neither id nor timestamp (a 989-session live scan found 0 with any id), so alignment fell back to fragile content-signature matching that goes ambiguous on repeated tool calls / empty assistant turns. This closes the DATA gap (the alignment logic itself already prefers `id` and handles the compacted case via the matcher, shipped in #5563): mint a monotonic, session-unique integer `id` on the per-turn result rows AFTER the context restore and BEFORE both arrays are built, so the display and model-context copies of a logical row share the same id. - api/streaming.py: new _assign_stable_message_ids(); _restore_reasoning_metadata carries `id` forward across turns (as it already does timestamp); wired into all three streaming commit sites (main turn, retry, self-heal). - api/routes.py: same mint on the runs/MoA _handle_chat_sync commit path. - api/gateway_chat.py: mints ids on the two new rows of a gateway turn. - The `id` is stripped before the provider API call (not in _API_SAFE_MSG_KEYS), same treatment as timestamp — nothing new reaches the provider. session_ops.py alignment was already delivered by #5563 (id-preferring matcher + compacted-case fall-through), so no change is needed there now; tests updated to assert the post-#5563 behavior (id-bearing = exact cut; id-less = errs toward under-keeping, never the old raw-index mis-cut). Co-authored-by: b3nw <b3nw@users.noreply.github.com>
… merge Codex/Opus gate finding: in eager session-save mode the current user turn is checkpointed into s.messages before the agent runs (no id yet); after _assign_stable_message_ids stamps the returned result row, the display merge keeps the durable eager checkpoint and skips the stamped result — leaving the display row id-less while its context twin carries the minted id, silently defeating id-based fork/truncate alignment for eager-mode users. The merge now copies the minted id onto the kept checkpoint when it lacks one (guarded so it can't overwrite or duplicate). Opus verified cross-array id pairing holds across the real 3-turn transform pipeline.
…boundary path) Codex round-3 gate finding: _dedupe_replayed_context_messages deep-copies the stale-user repaired boundary row (streaming.py:4501) into the context array. When the mint ran AFTER dedupe, that deep-copied context row was no longer the shared result dict, so it stayed id-less while the display copy got the minted id — re-opening the cross-array id divergence for the stale-repair path. Fix: move _assign_stable_message_ids() to run immediately after _restore_reasoning_metadata and BEFORE _dedupe_replayed_context_messages at all four commit sites (streaming main / in-band self-heal / except-path self-heal + routes runs/MoA), so the shared result rows are stamped before any deep-copy — both arrays inherit the id.
…gnment + CHANGELOG
|
| Filename | Overview |
|---|---|
| api/streaming.py | Adds _assign_stable_message_ids and wires it at three commit sites (streaming main, two except-path self-heals); extends _restore_reasoning_metadata to carry ids forward; patches _merge_display_messages_after_agent_result for the eager-checkpoint id gap. All four call sites correctly order mint before dedupe. |
| api/gateway_chat.py | Adds id minting for gateway-path turns; minting failure is silently swallowed with only a DEBUG-level log, which could hide regressions. |
| api/routes.py | Wires _assign_stable_message_ids into _handle_chat_sync (runs/MoA sync path), correctly placed before _dedupe_replayed_context_messages. |
| tests/test_context_message_stable_ids.py | New test file with good coverage of the minting helper, id carry-forward, and aligner resolution. The shared-id integration test inverts the production call order (dedupe before mint), passing only because prev_context is empty — it doesn't validate the stale-user boundary invariant that motivated moving the mint call. |
| tests/test_issue1217_transcript_compaction.py | Updated existing compaction test to tolerate the new id field via _no_id projection; adds assertions that brand-new turn rows carry unique integer ids. |
| CHANGELOG.md | Release changelog entry added by the release process, as expected for this repo. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Agent
participant streaming as streaming.py
participant ids as _assign_stable_message_ids
participant dedupe as _dedupe_replayed_context_messages
participant display as _merge_display_messages
participant aligner as truncate_context_for_display_keep
Agent->>streaming: result_messages (no id)
streaming->>streaming: _restore_reasoning_metadata() carry existing id forward
streaming->>ids: mint ids on result_messages
note over ids: seed = max(existing ids), new rows get seed+1, seed+2...
ids-->>streaming: result_messages now have ids
streaming->>dedupe: _dedupe_replayed_context_messages() deep-copies happen AFTER mint
dedupe-->>streaming: context array (shared id)
streaming->>display: _merge_display_messages() eager checkpoint carry id to kept row
display-->>streaming: display array (same id)
note over streaming: s.context_messages and s.messages share identical id per logical row
aligner->>aligner: fork/truncate match by id, no ambiguity on large sessions
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Agent
participant streaming as streaming.py
participant ids as _assign_stable_message_ids
participant dedupe as _dedupe_replayed_context_messages
participant display as _merge_display_messages
participant aligner as truncate_context_for_display_keep
Agent->>streaming: result_messages (no id)
streaming->>streaming: _restore_reasoning_metadata() carry existing id forward
streaming->>ids: mint ids on result_messages
note over ids: seed = max(existing ids), new rows get seed+1, seed+2...
ids-->>streaming: result_messages now have ids
streaming->>dedupe: _dedupe_replayed_context_messages() deep-copies happen AFTER mint
dedupe-->>streaming: context array (shared id)
streaming->>display: _merge_display_messages() eager checkpoint carry id to kept row
display-->>streaming: display array (same id)
note over streaming: s.context_messages and s.messages share identical id per logical row
aligner->>aligner: fork/truncate match by id, no ambiguity on large sessions
Reviews (1): Last reviewed commit: "release #5564: mint stable per-message i..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # --- context path (mirrors streaming.py commit site) --- | ||
| next_ctx = _restore_reasoning_metadata(prev_context, result_messages) | ||
| next_ctx = _dedupe_replayed_context_messages(prev_context, next_ctx, "hello") | ||
| _assign_stable_message_ids(result_messages, prev_display, prev_context) | ||
| context_out = _deduplicate_context_messages(next_ctx) |
There was a problem hiding this comment.
Lines 93–94 call _dedupe_replayed_context_messages before _assign_stable_message_ids, but the real streaming commit site (and all three other call sites) do the opposite — mint first, then dedupe — precisely because _dedupe_replayed_context_messages can deep-copy a stale-user boundary row out of result_messages before the mint runs, leaving that copy id-less (Round 2 of the gate). The test comment says it "mirrors streaming.py commit site" but the order is reversed. The test still passes because prev_context = [], which makes dedupe a no-op, so the deep-copy hazard never fires. A follow-up test with a non-empty prev_context containing a stale-user boundary row would expose the gap, and this test as written gives no regression protection for that path.
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!
| try: | ||
| from api.streaming import _assign_stable_message_ids | ||
|
|
||
| _assign_stable_message_ids( | ||
| [user_msg, assistant_msg], | ||
| previous_context, | ||
| list(getattr(s, "messages", None) or []), | ||
| ) | ||
| except Exception: | ||
| logger.debug("Failed to stamp stable ids on gateway turn rows", exc_info=True) |
There was a problem hiding this comment.
The except Exception block swallows any error from _assign_stable_message_ids and logs only at DEBUG level. In production, DEBUG is typically disabled, so a minting failure here (e.g., a future refactor that moves or renames _assign_stable_message_ids) would leave gateway-path turns silently id-less — reverting them to the pre-fix content-signature matching with no visible signal. The fallback is intentional and correct, but bumping the log level to WARNING would make regressions detectable without adding any noise to the happy path.
1 participant
Release: #5564 — mint stable per-message ids so large-session forks align
Ships @b3nw's root-cause fix for fork/truncate context misalignment, rebuilt + reconciled on current
masterand taken through a 3-round convergent gate. Nathan approved shipping on the clean re-gate.The bug
WebUI keeps two arrays per session — the visible transcript and the trimmed model context. Forking/truncating a large (compacted) session translates a display cut-index into the matching context cut-index via a per-message identity — an aligner that prefers a stable
id(shipped in #5563). But the model-context rows carried no id (a 989-session scan found zero), so on large sessions full of look-alike rows the aligner went ambiguous and could cut mid-turn, leaving the fork's context ending on a danglingtool_usethat the provider API rejects.The fix
_assign_stable_message_ids()mints a monotonic, session-unique integeridon the per-turn result rows after context-restore and before both arrays are built, so the display and model-context copies of a logical row share the id. Wired into all commit sites (streaming main / in-band self-heal / except-path self-heal, runs/MoA sync, gateway)._restore_reasoning_metadatacarries the id forward across turns. The id is stripped before the provider call (not in_API_SAFE_MSG_KEYS), same astimestamp.Reconciliation with #5563 (shipped since this PR was authored)
#5563 already delivered the aligner half (id-preferring matcher + removed the compacted raw-slice short-circuit), so #5564's
session_ops.pychange was redundant and was dropped; this PR is now purely the id-minting. Tests updated to assert post-#5563 behavior.Gate (3 rounds, converged)
_dedupe_replayed_context_messagesdeep-copies the boundary row before the mint ran). Root-fixed by moving the mint to run before dedupe at all four commit sites — so the shared row is stamped before any deep-copy._message_identitykeys on role+content+tool, neverid).Attribution: original author @b3nw (
Co-authored-bytrailer preserved).Closes #5564