Feat/split store interface#1474
Conversation
Replace the combined checkpoint Store interface with committed, temporary, and optional author capabilities. This keeps the current GitStore implementation while making the committed storage surface independent from shadow-branch operations. Entire-Checkpoint: b6e36ed89559
Expose committed and temporary checkpoint storage through separate interfaces from the Stores facade. Call sites now depend on committed readers and writers or temporary shadow-branch storage instead of the concrete GitStore. Entire-Checkpoint: bb51dce048b6
There was a problem hiding this comment.
Pull request overview
This PR refactors the checkpoint storage API by splitting the previously unified GitStore surface into capability-based interfaces for committed checkpoint operations vs. temporary (shadow-branch) operations, and updates strategy + CLI call sites to use the narrower interfaces.
Changes:
- Introduces
checkpoint.CommittedStore/checkpoint.TemporaryStorecapability interfaces and updatescheckpoint.Open()to return a facade (Stores) exposing both. - Updates manual-commit strategy paths (save/rewind/condense) to use
Stores.Temporary()for shadow-branch operations and committed interfaces for v1 metadata reads/writes. - Updates CLI features (explain/resume/attach/attribution) and tests to consume the new interfaces and new committed-reading helpers.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/strategy/manual_commit.go | Adds getCheckpointStores helper and narrows getCheckpointStore to CommittedStore. |
| cmd/entire/cli/strategy/manual_commit_test.go | Updates committed-read test flow to use ReadCommittedCheckpoint + ReadLatestSessionContent. |
| cmd/entire/cli/strategy/manual_commit_rewind.go | Switches rewind listing to use TemporaryStore from Stores.Temporary(). |
| cmd/entire/cli/strategy/manual_commit_git.go | Switches shadow-branch writes to TemporaryStore from Stores.Temporary(). |
| cmd/entire/cli/strategy/manual_commit_condensation.go | Updates doc comment to reference CommittedStore committed writes. |
| cmd/entire/cli/resume.go | Narrows helper to accept checkpoint.CommittedListReader instead of *GitStore. |
| cmd/entire/cli/explain.go | Uses CommittedStore in lookup and type-asserts optional author lookup via AuthorReader; narrows temp helpers to TemporaryStore. |
| cmd/entire/cli/checkpoint/store.go | Updates compile-time interface assertions for GitStore capabilities. |
| cmd/entire/cli/checkpoint/open.go | Updates Stores to expose Primary CommittedStore + Temporary() TemporaryStore. |
| cmd/entire/cli/checkpoint/committed.go | Updates LookupSessionLog to use ReadRawSessionLogForCheckpoint helper on a committed reader. |
| cmd/entire/cli/checkpoint/committed_reader_resolve.go | Introduces committed capability interfaces (CommittedWriter, CommittedStore) and optional AuthorReader. |
| cmd/entire/cli/checkpoint/checkpoint.go | Replaces prior combined store interface with TemporaryStore capability interface. |
| cmd/entire/cli/attribution.go | Narrows attribution resolver store dependency to a capability interface instead of *GitStore. |
| cmd/entire/cli/attach.go | Narrows attach store opening to return CommittedStore interface. |
SaveStep, SaveTaskStep, and GetRewindPoints each repeated the getCheckpointStores + Temporary() pair. A getTemporaryStore accessor mirrors getCheckpointStore so each call site is a single line, with getCheckpointStores remaining the shared open-with-blob-fetcher wrapper. Entire-Checkpoint: 684fdbdfe046
|
Bugbot run |
Keep empty-session checkpoints distinct from missing checkpoints when reading latest session content. Also narrow resume checkpoint metadata resolution to the methods it actually uses so the split store interfaces stay precise. Entire-Checkpoint: 8e67ab6c5252
Exercise the concrete GitStore latest-session reader in the redaction failure regression test. This keeps the test checking that dropped transcripts surface ErrNoTranscript after condensation still writes checkpoint metadata. Entire-Checkpoint: 02eff406bed3
Keep attribution resolution dependent only on the committed summary and session metadata methods it uses. Add a focused test with a reader that omits unused session-content methods so the local interface stays precise. Entire-Checkpoint: 136650505f9c
|
Bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 772908f. Configure here.
Return ErrCheckpointNotFound for empty committed summaries in the split reader helper. This keeps the interface split behavior-neutral and updates the regression test to cover the pre-split contract. Entire-Checkpoint: b7a703224f22
Introduce session and checkpoint document interfaces over the existing git committed storage layout. The new GitStore adapters preserve the old on-disk behavior while exposing SessionRef-based reads and option-based backfills for later caller migration. Entire-Checkpoint: 51f5a0ee3b11
Move committed checkpoint callers to the split checkpoint and session store surface. GitStore keeps the low-level v1 methods internally, but production callers now depend on ReadCheckpoint, ReadSession, WriteSession, UpdateSession, and UpdateCheckpoint. Entire-Checkpoint: 7319e37bab43
|
Bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 887b239. Configure here.
Route legacy summary updates through the session update API and reuse the committed checkpoint reader from attribution code. This keeps committed read/write behavior centralized without changing the public call sites. Entire-Checkpoint: 854458fe9b0a
Rename the committed checkpoint metadata store interface to avoid the generic Store name. Also fast-path indexed session refs so callers that already know a session index do not re-read the checkpoint summary. Entire-Checkpoint: 7ecf2c5cef5a
https://entire.io/gh/entireio/cli/trails/614
This draft pull request was opened by Entire after CI was requested for the linked trail. Feel free to edit the title or body — the link above is what keeps the trail and PR connected.
Summary
Split checkpoint storage interfaces so committed checkpoint metadata and git-only shadow-branch storage no longer share one fat
Storesurface.checkpoint.Storeinterface withCommittedStore,CommittedWriter,TemporaryStore, and optionalAuthorReader.checkpoint.Open/StoressoPrimaryexposes committed storage andTemporary()exposes shadow-branch storage, while the current git backend still backs both.explainnow reads checkpoint authors only when the committed store supportsAuthorReader.No behavior change intended.
Note
Medium Risk
Wide refactor across checkpoint persistence and every major read/write path; behavior is intended to be unchanged but regressions in attach, resume, condensation, or attribution are plausible if ref resolution or not-found handling diverges subtly.
Overview
Replaces the single fat
checkpoint.Storewith layered interfaces:TemporaryStorefor shadow-branch work, andCommittedStore(session + checkpoint read/write) forentire/checkpoints/v1.checkpoint.Opennow typesPrimaryasCommittedStoreandTemporary()asTemporaryStore, still backed by the sameGitStore.Adds
committed_domain.gowithSessionRef(LatestSessionRef,SessionIndexRef,SessionIDRef),ReadOption/WriteOption, and façade methodsReadCheckpoint,ReadSession,WriteSession,UpdateSession,UpdateCheckpoint. Production code is migrated offWriteCommitted,ReadCommitted,ListCommitted,UpdateCommitted, and per-index metadata helpers to these entry points.CLI and strategy paths (attach, attribution, explain, resume, rewind, dispatch, manual-commit hooks/condensation) depend on smaller interfaces (
committedCheckpointReader, optionalAuthorReader) and treat missing checkpoints viaErrCheckpointNotFoundwhere summaries used to be nil. Tests and bench fixtures follow the new API.Reviewed by Cursor Bugbot for commit 887b239. Configure here.