Guard state.db replay after compressed context anchors - #4695
franksong2702 wants to merge 1 commit into
Conversation
|
| Filename | Overview |
|---|---|
| api/models.py | Adds compression-anchor boundary detection and fail-closed state.db replay guard for compacted context. Logic is sound; a minor text-extractor inconsistency in _context_messages_include_compression_marker could theoretically suppress fail-closed behavior for non-standard content shapes. |
| tests/test_issue1217_transcript_compaction.py | Adds five regression tests covering verified anchor, unverifiable anchor, missing anchor key, ISO-format anchor timestamp, and missing ts. Coverage is thorough for the documented failure modes. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[reconciled_state_db_messages_for_session] --> B{prefer_context\nand context_messages?}
B -- No --> G[state_db_delta_after_context\nfull state_messages]
B -- Yes --> C{context_messages include\ncompaction marker?}
C -- No --> G
C -- Yes --> D{compression_anchor_message_key\npresent?}
D -- No --> E[Return context_messages only\nfail-closed]
D -- Yes --> F{_state_db_anchor_index\nfinds match?}
F -- No --> E
F -- Yes --> H[Slice state_messages\nto post-anchor rows]
H --> I[state_db_delta_after_context\npost-anchor state_messages]
G --> J[merge_session_messages_append_only]
I --> J
%%{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"}}}%%
flowchart TD
A[reconciled_state_db_messages_for_session] --> B{prefer_context\nand context_messages?}
B -- No --> G[state_db_delta_after_context\nfull state_messages]
B -- Yes --> C{context_messages include\ncompaction marker?}
C -- No --> G
C -- Yes --> D{compression_anchor_message_key\npresent?}
D -- No --> E[Return context_messages only\nfail-closed]
D -- Yes --> F{_state_db_anchor_index\nfinds match?}
F -- No --> E
F -- Yes --> H[Slice state_messages\nto post-anchor rows]
H --> I[state_db_delta_after_context\npost-anchor state_messages]
G --> J[merge_session_messages_append_only]
I --> J
Reviews (5): Last reviewed commit: "fix(compression): guard state replay aft..." | Re-trigger Greptile
| ### Fixed | ||
|
|
||
| - **Compressed model context no longer replays stale state.db history when the compression anchor is incomplete.** When a session has compacted `context_messages`, WebUI now only resumes state.db rows after a timestamp-verified compression anchor; missing or mismatched anchors fail closed to the compacted context instead of appending the old uncompressed transcript back into the next model prompt. Fixes #4249. |
There was a problem hiding this comment.
CHANGELOG.md edited directly in a contributor PR
This repo's policy is that CHANGELOG.md is maintained exclusively by the release process via release: vX.Y.Z commits authored by the release agent — individual contributor PRs should not touch it directly. Please revert these lines; the release agent will pick up the entry from the PR description / commit messages when cutting the next release.
Rule Used: Do not flag missing CHANGELOG.md updates on indivi... (source)
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!
147049f to
44440de
Compare
nesquena-hermes
left a comment
There was a problem hiding this comment.
Thanks @franksong2702 — this is the right boundary to fix (#4249's stale post-compression context replay), and the timestamp-verified-anchor approach is sound. The gate reproduced one CORE gap that has to close before it ships, plus a follow-up under-coverage:
CORE (blocker, reproduced): the no-anchor case does not fail closed
reconciled_state_db_messages_for_session guards the new anchor logic with if anchor_key: (models.py:5278). When a compressed context_messages session has no compression_anchor_message_key (absent / None), the guard is skipped entirely and execution still falls through to state_db_delta_after_context() — which replays the stale uncompressed transcript into model-facing context. That's the exact failure mode the PR targets, left open for the no-anchor path.
Reproduced (compressed context + anchor_key=None):
context_messages=[
{"role":"user","content":"[context compaction] summary of earlier work"},
{"role":"user","content":"new question"},
]
compression_anchor_message_key=None
state=[{"role":"user","content":"old question","ts":1.0},
{"role":"assistant","content":"old answer","ts":2.0},
{"role":"user","content":"new question","ts":3.0}]
# result → ['[context compaction] summary…', 'new question', 'old question', 'old answer'] ← STALE LEAKFix: when using_context_messages AND the context is a compressed context (detect via api.compression_anchor.is_context_compression_marker over local_messages), require a truthy, timestamp-verifiable anchor — if it's missing or unverifiable, return list(local_messages) before state_db_delta_after_context(). Keep the existing delta behavior only for ordinary (non-compressed) context_messages so you don't over-suppress fresh turns on normal sessions. Please add a regression test for compressed-context + compression_anchor_message_key=None asserting no stale state.db rows return.
Follow-up (not a blocker, but it defeats the fix for the target sessions)
The anchor timestamp match appears to handle only numeric epoch-second timestamps. The continuation/LCM sessions #4249 is actually about carry ISO-8601 timestamps, so the anchor delta path silently no-ops for exactly those sessions (falls through to old behavior — no stale leak, but no fix either). Worth ISO-aware timestamp parsing + a regression test + a debug log when an anchor is present but can't be verified, so this doesn't look fixed while quietly doing nothing on the real-world case.
Everything else checks out: the visible-transcript path is unaffected (double-gated on prefer_context + using_context_messages), no perf regression, and the timestamp-match false-positive risk is acceptably bounded (role + attachment-count + normalized-text). Happy to re-review as soon as the no-anchor compressed-context case fails closed.
44440de to
bdebd52
Compare
|
Updated the PR to close the requested gap:
Local verification: |
bdebd52 to
548a49f
Compare
|
Re-reviewed at head CORE gap (no-anchor compressed context) — closed ✅The previous head guarded the anchor logic with if using_context_messages:
compressed_context = _context_messages_include_compression_marker(local_messages)
anchor_key = getattr(session, "compression_anchor_message_key", None)
if compressed_context:
if not anchor_key:
... return list(local_messages) # no anchor → fail closed
anchor_index = _state_db_anchor_index(state_messages, anchor_key)
if anchor_index is None:
... return list(local_messages) # unverifiable → fail closed
state_messages = list(state_messages or [])[anchor_index + 1 :]
state_messages = state_db_delta_after_context(local_messages, state_messages)The reproduced leak ( ISO-8601 anchor timestamps — closed ✅
return datetime.datetime.fromisoformat(str(value).strip().replace("Z", "+00:00")).timestamp()
One non-blocking nit (optional polish)
Nothing here blocks merge. The fail-closed semantics are correct, the visible-transcript path stays untouched (double-gated on |
Release v0.51.589 — Release UV (guard state.db replay after compressed anchors, #4695)
|
Shipped in v0.51.589 (Release UV) 🚀 — thank you @franksong2702. This guards the state.db replay path behind a timestamp-verified compression anchor exactly as your fix did: compressed Gate before ship (all three legs, run on your latest head
One non-blocking, latent-not-live follow-up was noted (unifying the compression-marker prefix re-check with the card-content extractor so the guarantee never depends on card-content shape) — it doesn't fire today since the canonical card is built as a plain string. Filing that as a small separate hardening; your code shipped as reviewed. Thanks again for the clean, well-tested fix on a delicate path. |
Thinking Path
reconciled_state_db_messages_for_session(..., prefer_context=True): oncecontext_messagesis compacted, content-fingerprint alignment against the uncompressed state.db can fall back to returning old transcript rows.context_messageswith no anchor key still fell through to the old state.db delta path. This update fails closed for compacted context when the anchor is missing or unverifiable.What Changed
api/models.py.Ztimestamps from real sessions.session.context_messagesare used withprefer_context=True, state.db rows are sliced only after a verified role/text/attachment/timestamp anchor match.context_messagesonly.context_messages, so mirrored Session Arc Summary contexts can still admit fresh follow-up rows.ts: None, missing anchor keys, and ISO timestamp anchors.Why It Matters
This prevents the post-compression next turn from silently rebuilding model context with the old uncompressed transcript. For long sessions, that is the failure mode behind the apparent "compression did nothing" token/cache behavior described in #4249.
Contract Routing
Task type: runtime/model-context bugfix
Touched areas:
api/models.pystate.db reconciliation, compression anchor handling, transcript compaction testsRelevant public docs:
AGENTS.mdCONTRIBUTING.mddocs/CONTRACTS.mddocs/rfcs/webui-run-state-consistency-contract.mdScope boundaries: model-facing context reconstruction only; no UI rendering or visible transcript behavior changes.
Evidence needed before claiming done: regression tests prove stale state.db rows are not replayed after compacted context when the anchor is missing or incomplete, and timestamped anchors still allow fresh post-anchor rows.
Verification
./scripts/test.sh tests/test_issue1217_transcript_compaction.py -q-> 31 passedgit diff --check origin/masterRisks / Follow-ups
CHANGELOG.mdentry in this contributor PR; the release process owns changelog entries.Fixes #4249.
Model Used
AI-assisted implementation and review.
@openai-codex:gpt-5.3-codex-spark(openai-codex) athttp://127.0.0.1:8787/session/ad0e0f643364.