Repository navigation
🤖 fix: store post-compaction context on the compaction boundary row - #7693
ethanndickson wants to merge 28 commits into
Conversation
Post-compaction attachments (plan file, edited-file diffs, todo list, path skills) were injected only into the provider request. The next turn rebuilt the request without them, so the model saw its context change between turns. This caused many thinking-block drops. Now the first non-compaction turn after a compaction appends the attachments as a hidden synthetic row in chat.jsonl, after the user message, like the file-change notice rows. Later turns read the same row from history, so the request prefix stays stable. - Skip the row for compaction turns, prepared requests and when attachments are disabled. Reuse an existing row in the current window. - Delete the row on a context-exceeded retry (new HistoryService.deleteMessageUnderAdmission). - Never copy the row into a compaction's preserved tail. - Hide the row in the transcript. - Treat the row like the budget warning row for resume, task goal continuation, goal follower and revival scans. - Remove the periodic refresh every 5 turns. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh` • Cost: `$94.02`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh costs=94.02 -->
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (2)
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🟢 e2e stories (Haiku): 14 passed22 agent steps · 7 replayed from cache · 15 model calls · 46.3k tokens (24% cached) All 14 tests in 9 files
e2e 0.20.0 · 1m 9s · web · run artifacts 🟢 e2e Electron stories (Haiku): 3 passed3 agent steps · 3 model calls · 8.0k tokens (22% cached) All 3 tests in 3 files
e2e 0.20.0 · 18.2s · electron · run artifacts 🟢 e2e agent repros (Haiku): no tests selectedSpend: $0.00 of the $1.00 cap (proxy ledger) e2e 0.20.0 · 5.3s · 2 targets (web, phone) · run artifacts |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: a51fdcfed3
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a51fdcfed3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Place the attachments row before the turn's prompt in every provider request. - Append the row only while the history tail still matches the turn's read, so a compaction publication that lands meanwhile cannot leave a row from the older snapshot in the new window. - Count skills that the row carries as surfaced, so path-scoped skills get no second pointer.
|
@codex review Round 1 is addressed in b6d680e and 36832f5: the attachments row sits before the prompt in the provider payload, the row append and the row reuse both check that the history tail still matches the turn's read, and carried skills count as surfaced. Declined: "Track durable attachment rows after acknowledgment". After the acknowledgment the row is ordinary history that the model already acted on. Deleting it on a later overflow would change that conversation. Before this PR, later turns had no retry without attachments either. Overflow from later history goes through the context strategy recovery ( |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36832f51e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Restore the post-compaction attachments row when an edit cuts it, unless the edit also cut the row's context boundary. The snapshot was acknowledged, so no later turn writes it again. - Consume the pending snapshot and mark the context retry only after the row delete succeeds, so a failed delete leaves a row that the next turn can still reuse or delete. - Stop reading carried skills from the row text: plan text or a skill body can contain a skill tag and hide a real path-skill pointer. A carried skill can now get one extra pointer.
|
@codex review Round 2 is addressed in b8a5950:
|
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8a5950bbc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The post-compaction attachments row is a user-role row stored after its turn's prompt. Edits that cut a later boundary keep it, branch summaries and /refine skip it, rolling cuts and title context do not start a turn on it, request context goes before the prompt instead of before the row, and the default Continue resume stays muted when the row follows it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cee66e832d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The boundary drops the old window's post-compaction attachments row, and the attachment budget already counts the fresh row that replaces it. Cut selection and the staged-summary fit check now give that row zero weight, so a large row no longer forces a smaller or empty retained tail.
|
@codex review Round 4 fix is in 7484a75: rolling cut selection and the staged-summary fit check no longer count the old attachments row that the boundary drops. The security advisory about provider order was fixed in b6d680e ( |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7484a7586a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Pausing the Codex review loop for a human decision (AGENTS.md early stop case 2: the loop does not converge). Findings per round: 4, 3, 6, 1, 5. Round 5 grew again. Each round finds one more consumer of Open round 5 findings on 7484a75:
I left these threads open. Options: fix 1 to 4 and continue the loop, simplify the design so fewer consumers see the row, or stop here for human review. |
Compaction renders the post-compaction context once and stores it on the boundary row. Every request of the window inserts the same synthetic row after the boundary, so the prefix stays stable and Anthropic keeps the replayed thinking. Keeps the context-exceeded retry without the header, the report cutoff at wall-clock time, a 30s header build limit, the header fit for continuous compaction, and plan-file re-tracking after the post-compaction clear.
ethanndickson
changed the title
|
@codex review |
…ary row The context-exceeded retry now marks the boundary row with the first request sequence that leaves the header out. Later requests leave it out too, also after a restart, and replay-verify rebuilds the earlier requests with it. The header also stores the plan as it read. Turn start tracks the plan from it, so plan edits after the snapshot get a change notice, even before the next turn or after a restart. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh -->
expect.any(Number) inside toEqual is an unsafe any assignment for the type-aware ESLint rule. Check the plan fields with toMatchObject and the modified time with typeof. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh -->
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c3e21ee2f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…mal requests The continuous compaction builder now fits the summary and adds the header once. The swap sends that row, and finalize persists it as built. A pending follow-up that makes the row too big leaves it oversized; the history scanner recovers oversized compaction boundaries. The header keeps its plan baseline or is dropped with it. A plan in the header that nothing tracks would replay stale after the user edits the file. The header-omission retry arms only for requests that are not compaction requests. A compaction request trims its input to its budget, and the trim can drop the header row, so its overflow does not show that the header caused it. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh -->
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A context-exceeded retry leaves the post-compaction header out of later requests. The boundary row's contextUsage now drops the header's tokens at the same time, so the context meter and the next compaction check stop counting a header that requests no longer render. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh -->
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6a78d7e9d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
An edit or fork that removes every row a request without the header saw also removes the reason to leave the header out: the retained turns ran with it, and the request that overflowed is gone. Truncation now clears the omission mark on the latest boundary in that case and counts the header in the boundary's usage again. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh -->
|
@codex review 5675315 fixes the edit/fork finding: a cut before the header omission puts the header back. Accepted known limitations (please do not raise these again):
|
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5675315150
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This reverts commit 5675315. Rendering the header again after an edit or fork cut did not restore the state that the headerless retry had already changed: the plan-baseline cache keyed by the boundary ID and the discarded carryover. An edit or fork back before a header omission now keeps the header out, and the boundary state stays consistent. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh -->
A continuous boundary drops a header that does not fit its row, but its contextUsage still added the header estimate from before the build. The boundary now counts the header on the row: the journaled header for a prepared boundary, and the header that publication adds for the others. The unused attachmentTokens argument of buildContinuousCompactionRows is removed. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh -->
|
@codex review 42718e9 reverts 5675315: rendering a left-out header again after an edit or fork cut could not restore the plan-baseline cache or the discarded carryover. e53d3bf makes a continuous boundary count only the header that its row keeps. Accepted known limitations (please do not raise these again):
|
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e53d3bfec6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review No code change since e53d3bf. The discard-failure finding is declined with reasons in its thread. Accepted known limitations (please do not raise these again):
|
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e53d3bfec6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- A mid-stream prefix swap now checks the window again with the header it sends. The estimate that admitted the summary can lack that header when its build timed out. A swap that no longer fits falls back to the durable apply. - Turn start seeds the plan baseline from the boundary header only when the tracker has no newer state for the plan. A tool in the retained tail of a swap can record a newer state that the model already saw. _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh -->
|
@codex review 942e080 fixes the 2 findings on e53d3bf:
Accepted known limitations (please do not raise these again):
|
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 942e080f01
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const withContext: MuxMessage = { | ||
| ...boundary, | ||
| metadata: { ...boundary.metadata, postCompactionContext: context }, |
There was a problem hiding this comment.
When compaction upgrades a streamed placeholder before a late finalizer rewrites the same row, this newly stored field is lost: getCompactionMetadataToPreserve() preserves the boundary markers and summary metadata but not postCompactionContext. The next turn can therefore acknowledge the pending snapshot against a headerless boundary, dropping a successfully built plan/skills/diffs header. Preserve the header in the existing late-rewrite protection.
AGENTS.md reference: AGENTS.md:L114-L117
Useful? React with 👍 / 👎.
| const omitted: MuxMessage = { | ||
| ...boundary, | ||
| metadata: { | ||
| ...boundary.metadata, | ||
| postCompactionContext: { ...context, omittedFromRequestSequence: requestHistorySequence }, | ||
| }, |
There was a problem hiding this comment.
In continuous mode, an eager summary can be staged while this header is still present; if a later request hits context_exceeded, this rewrite removes the header without resetting the compactor. Because ContinuousCompactor.fingerprint() excludes postCompactionContext, the staged summary still passes isValid() and can later be applied even though it was generated from the carryover that the retry deliberately discarded. Include the header/omission state in the fingerprint or invalidate the staged summary when persisting this marker.
AGENTS.md reference: AGENTS.md:L114-L117
Useful? React with 👍 / 👎.
| if ( | ||
| this.planBaselineBoundaryId !== undefined && | ||
| (await this.contextController.compaction.peekPendingState()) === null | ||
| ) | ||
| return; |
There was a problem hiding this comment.
When the user edits a message from before the latest compaction, truncation can remove that boundary and expose an older plan-bearing boundary, but planBaselineBoundaryId remains defined and this fast path skips the history read because no new pending state exists. The tracker consequently retains the discarded branch's newer plan baseline, so the model keeps seeing the older header without receiving a diff for the current plan. This is separate from the reverted omitted-header restoration case: a normal edit changes the latest boundary ID without invalidating this cache; reset the cache and incompatible tracked plan state when such an edit truncates across a boundary.
Useful? React with 👍 / 👎.
I looked at why we keep losing thinking blocks mid-session, and 22 of the 43 drops came from post-compaction attachments (plan file, edited file diffs, todo list, path skills). We only injected them into the provider request, at a position and with content that changed between turns, so the prefix changed under the model and Anthropic dropped the replayed thinking.
My first attempt wrote the attachments as a separate synthetic row in
chat.jsonl, but five review rounds showed that a separate row touches every history consumer (the rolling cut, edits, forks, the timeline, retries).Instead, compaction now renders the attachments once and stores the text on the boundary row itself, as
postCompactionContextmetadata. Every request inserts that text as a synthetic user row right after the boundary, so every turn, step and compaction request in the window builds the same bytes. Forks, edits and truncation already keep the boundary row as is, so they keep the context for free. The field isn't inMuxMessageSchema, so the frontend never receives it.Continuous compaction builds the header when it prepares the swap and journals it on the boundary, so the persisted row matches the prefix that was already sent. The builder fits the summary and adds the header once, and finalize persists that row as is, even when a later pending follow-up makes it oversized (the history scanner recovers oversized boundaries). If the header would push the row over the history line limit, we drop it together with its plan baseline rather than cut the summary. The build is time-boxed at 30s, and a failure just publishes the boundary without a header. The context-exceeded retry from #1932 still exists for normal requests; it now marks the boundary row so later requests leave the header out, also after a restart. Compaction requests skip it, since their input trim can already drop the header and they have their own retry. The header also stores the plan as it read it, and turn start tracks the plan from there, so an external plan edit after the snapshot still gets a notice, even one made before the next turn or after a restart. We only track the plan when the header carries it, so a sidebar exclusion change after compaction doesn't change what gets tracked. Replay envelopes no longer store
postCompactionAttachmentsHash, as the header is inchat.jsonlnow. The retry's mark records the first request without the header, so replay-verify rebuilds the earlier requests with it.The header is a snapshot from compaction time, so it no longer refreshes every 5 turns, and a change to the sidebar exclusions applies from the next compaction. Tracking the plan from the header can report a plan edit twice, when the agent made it after the snapshot (continuous compaction runs mid-stream) or we already reported it before a restart, but it never misses one. Boundaries written before this change have no header, so the first window after an upgrade runs without the context until the next compaction. That includes a continuous-compaction journal an older build left behind for recovery. Remote runtimes still don't re-track the plan file, as the file tracker reads the host filesystem; that's the same on
main, and a runtime-backed tracker can be a follow-up.Two more follow-ups. The header's skill names list every skill in the snapshot, even one the 80k budget cut, so path activation won't point the model at a cut skill;
mainderives the names the same way. A fork copies the header as is, so it names the source workspace's plan path. Onmaina fork after compaction got no post-compaction context at all, and the copied summary already names that path; remapping it without breaking the replayed thinking can come later.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh