Skip to content

fix(session): ignore failed compaction summaries when reordering - #42283

Open
codeAnqiang-ma wants to merge 1 commit into
anomalyco:devfrom
codeAnqiang-ma:compaction-summary-guard
Open

fix(session): ignore failed compaction summaries when reordering#42283
codeAnqiang-ma wants to merge 1 commit into
anomalyco:devfrom
codeAnqiang-ma:compaction-summary-guard

Conversation

@codeAnqiang-ma

Copy link
Copy Markdown

Issue for this PR

Closes #42282

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

filterCompacted decides "this compaction is complete" twice, with two different tests. The truncation loop requires summary && finish && !error; the reordering branch underneath only checks summary. A compaction whose summary turn failed lands between them: the loop keeps the full history, then the reorder returns [compaction..summary] + [tail..compaction] + [after summary] and drops everything before tail_start_id. Since the summary failed there is nothing to replace that history, and the errored assistant message is dropped by toModelMessagesEffect anyway. tail_start_id is persisted even on failure, so the loss repeats every turn.

This makes the reordering branch use the same test as the loop. Successful compactions are unaffected — the reorder still fires, this only stops it firing on a summary the loop already considers incomplete.

Only the guard is changed. Tightening the tail_start_id write in compaction.ts would stop new sessions from getting into this state, but sessions that already have the marker persisted still need this guard to recover, so it seemed like the right single place to fix.

How did you verify your code works?

Added keeps history before tail_start_id when the summary failed directly beside the existing success-path test retains original tail when compaction stores tail_start_id, reusing the same addUser / addAssistant / addCompactionPart helpers with an errored summary instead of a finished one.

  • On dev it fails: expected all 8 messages, received 6 — the first turn's user and assistant messages are missing and the compaction pair is hoisted to the front
  • With the fix it passes and history stays chronological

From packages/opencode:

  • bun test test/session/messages-pagination.test.ts — 52 pass, 0 fail
  • bun test test/session/message-v2.test.ts — 39 pass, 0 fail
  • bun test test/session/compaction.test.ts — 54 pass, 1 skip, 0 fail
  • bun test test/session/prompt.test.ts — 57 pass, 1 skip, 0 fail
  • bun test test/session/revert-compact.test.ts — 8 pass, 0 fail
  • bun test test/session/snapshot-tool-race.test.ts — 1 pass, 0 fail
  • bun typecheck, and the pre-push bun turbo typecheck — 30/30 packages
  • Prettier clean; oxlint warning count unchanged from baseline on both files

I also drove the whole path end to end against the real SessionCompaction.process with persistence — three turns, then an auto-compaction whose summary request returns a non-retryable 401 — and confirmed the next turn sees 3 of 5 messages before the change and all 5 after. That one is slow and provider-shaped so I left it out of the suite; happy to add it if you'd like it committed.

Did not run the rest of packages/opencode.

Screenshots / recordings

N/A — not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Disclosure: found and fixed with AI assistance; I reviewed and verified every claim locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failed compaction silently drops pre-tail history from the model context

1 participant