Skip to content

fix(compaction): snapshot history once during checkpoint capture - #1588

Open
rlaope wants to merge 4 commits into
code-yeongyu:mainfrom
rlaope:fix/compaction-checkpoint-history-snapshot
Open

rlaope wants to merge 4 commits into
code-yeongyu:mainfrom
rlaope:fix/compaction-checkpoint-history-snapshot

Conversation

@rlaope

@rlaope rlaope commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Snapshot getEntries() once before the newest-first agent-name scan in compaction checkpoint capture.
  • Preserve full-history selection (including entries outside the active branch), agentName/agent precedence, and null/empty-name behavior.
  • Add deterministic regression coverage using a real persisted SessionManager after compaction trims its resident mirror. The history-loader instrumentation delegates to the real file reader.

Why

For a persisted session whose resident mirror has been trimmed, each getEntries() synchronously reloads the complete history. The previous loop called it for initialization and again for each examined entry, repeatedly reading and parsing the same transcript. One snapshot removes that repeated work without changing session persistence or compaction policy.

Verification

  • Failing first: all 6 new cases failed with expected 32 to be 1 full-history loads; after the fix, 6 passed (6).
  • bun run --cwd packages/coding-agent test test/compaction test/session-manager test/compaction-checkpoint-oneshot.test.ts test/suite/compaction-checkpoint-history.test.ts --maxWorkers=4: 91 files, 720 tests passed.
  • bun run build: passed all six build phases.
  • bun run check: passed. Its formatter touched four pre-existing unrelated formatting issues; those edits were restored to keep this PR scoped. Changed-file Biome, tsc --noEmit, and git diff --check also passed on the final tree. LSP was unavailable, so verification used the compiler directly.
  • Standalone script importing the built exported captureAgentCheckpoint: three synthetic 32-entry compacted sessions each performed one full-history load, with correct missing-marker, legacy-agent, and field-precedence results. Empty history returned null; tools, thinking level, and model were preserved.
  • Real CLI mock-provider turn: passed with a localhost-only request and expected response.
  • Independent reviewer: APPROVE / High confidence; independently reproduced the six-case red/green transition and ran the existing legacy checkpoint tests (5 passed).

Pre-existing validation failures

The broader mock-loop.mjs --self-test --api openai-completions reports 13/22 passed both on this change and on unchanged base ef382f1aa. The same 9 failures concern complete/truncated text-tool-protocol recovery (single request instead of two, missing final marker/replayed tool output, visible leaked protocol, and missing recovery guidance). Normal CLI round-trip and credential-isolation checks pass. Those unrelated failures are not modified here.

No dependency, lockfile, or release changes are included.


Summary by cubic

Snapshots the session history once before the newest-first agent-name scan in compaction checkpoint capture, so each capture no longer reloads and parses the full transcript repeatedly.

  • Removes redundant disk reads for persisted sessions with trimmed resident mirrors without changing agent selection behavior.
  • Adds deterministic regression tests covering the new single-load path and records the fix in the changelog and compaction policy docs.
  • Aligns test fixtures with current contracts: the question-prompt routing test now covers both resolved outcomes, and a stale fixture was dropped.

Written for commit d6df622. Summary will update on new commits.

Review in cubic

@rlaope

rlaope commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Pushed CI repair in merge commit b4c67c9 (ordinary merge of main; no rebase/force-push). The compaction snapshot fix is unchanged.

Root causes from failed run 34581390968, both reproduced locally:

  • Static TS2353: upstream's goal test fixture still supplied removed liveWakeSources. Removed that obsolete fixture property.
  • RPC deadline: the routing test expected question_incomplete for a nonempty partial answer, but fix(tui): make ask-user questions advance and submit predictably #1576 intentionally made partial answers resolve. The test now rejects an empty submission, then exercises both partial-answer and comment-only resolution across three actual IPC clients. Listeners register before sending; payloads/unanswered IDs are asserted. No sleeps, skipped tests, or timeout increases.

Validation: 119 focused tests passed; after a fresh full build, 665 RPC/compaction tests across 89 files passed; bun run check, final no-emit compiler and focused Biome passed; real CLI RPC smoke 4/4; isolated mock-provider CLI turn passed; built checkpoint entry point preserved single-history-load behavior in all four scenarios. Independent read-only reviewer approved. Unrelated formatter-only changes were restored. LSP was unavailable, so the compiler supplied type diagnostics.

Current required CI is 34583356893. Monitoring it to completion; this comment is not a green-CI claim.

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.

2 participants