Fall back to v1 checkpoint transcripts for v2 reads#1223
Conversation
There was a problem hiding this comment.
Pull request overview
This PR centralizes the selection of committed checkpoint readers behind a new shared factory (NewCommittedReader / committedCheckpointReadMode) and introduces a DualCheckpointReader that, when v2 dual-read mode is active, transparently falls back from v2 to v1 raw transcript artifacts (e.g., when only v2 compact transcripts exist but v2 /full/* is missing). Compact transcript reads remain v2-only, with callers routed back through the raw-content path when compact metadata is unavailable.
Changes:
- Add
CommittedReadMode,NewCommittedReader,DualCheckpointReader,ReadCommittedCheckpoint,ReadLatestSessionContent, andReadRawSessionLogForCheckpointhelpers incheckpoint/committed_reader_resolve.go, plus a newcmd/entire/cli/checkpoint_reader.gofactory used across callers. - Refactor
explain.go/explain_export.go/resume.go/rewind.go/review_helpers.go/review_context.go/manual_commit_rewind.goto use the new factory and reader, removing duplicated v1/v2 selection logic and replacing direct*V2GitStoretype assertions with capability interfaces (v2MainContentReader,compactTranscriptReader). - Add tests for dual-read fallback (compact-only v2 → v1 raw), mode selection, and list merging.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/checkpoint/committed_reader_resolve.go | Introduces CommittedReadMode, DualCheckpointReader, and shared read helpers; reworks ResolveCommittedReaderForCheckpoint/ResolveRawSessionLogForCheckpoint to route through them. |
| cmd/entire/cli/checkpoint/committed_reader_resolve_test.go | Adds coverage for mode selection, dual reader fallback to v1 raw by session ID, list merging, and v2-only no-fallback behavior. |
| cmd/entire/cli/checkpoint_reader.go | New CLI-side factory that builds v1/v2 stores plus a CommittedListReader per the resolved read mode. |
| cmd/entire/cli/explain.go | Switches to the shared reader/factory; replaces *V2GitStore type assertions with v2MainContentReader; removes local listCommittedForExplain / readLatestSessionContentForExplain helpers in favor of shared variants. |
| cmd/entire/cli/explain_export.go | Uses the shared reader and a compactTranscriptReader capability interface; adds raw fallback when compact transcript is missing; consolidates metadata-read switch. |
| cmd/entire/cli/explain_test.go / explain_export_test.go | Adds tests for v1 raw fallback when v2 /full/* is missing and updates listCommittedForExplain tests to use NewCommittedReader. |
| cmd/entire/cli/resume.go | Replaces inline v2/v1 GetSessionLog logic with the shared reader; falls back to LookupSessionLog only when the repo can't be opened. |
| cmd/entire/cli/resume_test.go | Adds a fake agent and a regression test for v1 fallback on resume when v2 has only a compact transcript. |
| cmd/entire/cli/rewind.go | Uses shared reader + ReadRawSessionLogForCheckpoint for session transcript restore. |
| cmd/entire/cli/review_context.go | Switches review-context lookup to shared reader; treats ErrCheckpointNotFound from v2 metadata/prompts as fall-through to ReadSessionContent. |
| cmd/entire/cli/review_helpers.go | Uses shared reader for HEAD review-checkpoint detection; drops bespoke v2URL/store wiring. |
| cmd/entire/cli/strategy/manual_commit_rewind.go | Removes local committedReader interface and inline v2-first logic in favor of cpkg.NewCommittedReader/ReadCommittedCheckpoint; tightens v2-only-mode error handling. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| content, err = readV2ContentFromMain(ctx, mainReader, fullCheckpointID, summary) | ||
| } else { | ||
| content, err = readLatestSessionContentForExplain(ctx, resolvedReader, fullCheckpointID, summary) | ||
| content, err = checkpoint.ReadLatestSessionContent(ctx, resolvedReader, fullCheckpointID, summary) |
There was a problem hiding this comment.
Post-generate reload fails for v1-only checkpoints in dual mode
Medium Severity
In dual-read mode, DualCheckpointReader satisfies the v2MainContentReader interface, so isCheckpointsV2 is always true. After --generate succeeds, the reload path unconditionally calls readV2ContentFromMain, which delegates to DualCheckpointReader.ReadSessionMetadataAndPrompts — a v2-only method with no v1 fallback. For a v1-only checkpoint, this returns ErrCheckpointNotFound and the reload fatally errors instead of falling through to ReadLatestSessionContent.
The initial load in loadCheckpointForExplain correctly handles this with an ErrCheckpointNotFound guard (lines 737–742), but the post-generate reload lacks the same fallback.
Reviewed by Cursor Bugbot for commit 8099048. Configure here.
Flatten the nested nil-handling in NewCommittedReader's Dual branch into a flat switch. Use early returns in readFallbackSessionContent so the v1-by-session-ID path reads top-to-bottom. Reuse the summary already resolved by ResolveCommittedReaderForCheckpoint instead of re-reading it through ReadRawSessionLogForCheckpoint. Entire-Checkpoint: 824b5f21a156
|
Bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 0854f2b. Configure here.
…com:entireio/cli into fix/checkpoints-v2-full-transcript-fallback
Adds a doc comment on the helper explaining why prefer-main attempts the v2 /main ref first (cheaper, /full/* refs may not be fetched) and why ErrCheckpointNotFound still falls through (v1-only checkpoints in dual-read mode). Wraps the ReadLatestSessionContent error to satisfy wrapcheck. Entire-Checkpoint: 7a21ab090a27
Entire-Checkpoint: e55b124d2b57
https://entire.io/gh/entireio/cli/trails/384
What
DualCheckpointReaderfor v2 dual-read mode so missing v2 full transcript artifacts fall back to v1 raw artifacts.Verification
mise run buildmise run lintmise run test:ci:coremise run test:ci:integration:shard -- bE2E canary was left to CI because this branch does not change e2e prompts or agent-facing canary behavior.
Note
Medium Risk
Changes the checkpoint read path across
explain,resume, and rewind/strategy flows, including new dual-store fallback behavior that can affect which transcript bytes are shown or restored when v2 artifacts are missing. Risk is mitigated by explicit read modes, context-cancellation handling, and expanded tests covering fallback and list merging.Overview
Centralizes committed checkpoint reading behind a new reader factory and dual-reader mode. Adds
CommittedReadMode,CommittedListReader, andNewCommittedReader, plus aDualCheckpointReaderthat tries v2 first and falls back to v1 for summaries/session metadata and especially raw transcript reads when v2/full/*artifacts are missing.Refactors CLI commands to use the shared reader selection and consistent helpers. Introduces
newCommittedCheckpointReader(chooses v1/v2/dual based on settings) and updatesexplain, export/JSON/transcript streaming, review context,resume,rewind, and manual-commit restore paths to useReadCommittedCheckpoint/ReadLatestSessionContentand the dual-reader’s fallback semantics.Hardens export/display fallbacks and coverage. Export now falls back from compact to raw transcripts on
ErrCheckpointNotFound/ErrNoTranscript, list merging moves into the dual reader, and new tests validate v2→v1 transcript fallback by session ID, transcript-offset preservation, and “v2-only” mode not falling back.Reviewed by Cursor Bugbot for commit 0854f2b. Configure here.