Add checkpoints v1.1 support to entire explain#1311
Merged
Conversation
Entire-Checkpoint: 8600a8f63481
computermode
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the v1.1 committed-checkpoint read path so entire explain only reads from the local v1.1 custom ref (refs/entire/checkpoints/v1.1) after it has been successfully synced to the v1 metadata branch tip, otherwise falling back to v1 (including the existing origin/... fallback).
Changes:
- Introduces a
NewCommittedReadStorefactory that syncs/validates the v1.1 custom ref before binding committed reads to it, otherwise falling back to the v1 store. - Updates
GitStoreto track a configurable committed-read ref (reads can target v1.1; writes remain pinned to v1). - Updates
entire explainread and--generatereload behavior to respect the committed-read store selection, with regression tests for fallback/sync scenarios.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/strategy/v1_custom_ref_mirror.go | Updates docs to reflect that entire explain may read the v1.1 custom ref after a trusted sync. |
| cmd/entire/cli/paths/paths.go | Clarifies semantics of the v1.1 metadata custom ref (sync-then-read, fallback to v1). |
| cmd/entire/cli/explain.go | Switches committed reads to NewCommittedReadStore; ensures --generate writes via v1 store then rebuilds read store for reload. |
| cmd/entire/cli/explain_test.go | Adds regression test ensuring v1.1 mode reload sees summaries written via v1 store and ref resync occurs. |
| cmd/entire/cli/checkpoint/store.go | Adds committedReadRef to GitStore, defaulting to v1, with a constructor to override for read-only v1.1 mode. |
| cmd/entire/cli/checkpoint/committed.go | Resolves committed reads (tree + author lookup) against committedReadRef, preserving origin fallback only for v1. |
| cmd/entire/cli/checkpoint/committed_read_store.go | New: sync-then-read logic for v1.1 custom ref with safe fallback to v1. |
| cmd/entire/cli/checkpoint/committed_read_store_test.go | New: tests for seeding/advancing/divergence/write-failure and v1 fallback behavior (including remote-only metadata). |
When checkpoints_version is 1.1, NewCommittedReadStore now always binds committed reads to the v1.1 custom ref instead of falling back to the v1 store on sync failure/divergence. The read path stays on v1.1 so it is genuinely exercised rather than masked by a v1 fallback. To keep `entire explain` working on fresh clones (where v1 exists only as origin/entire/checkpoints/v1), the read-time sync now seeds the v1.1 ref from the origin remote-tracking tip when no local v1 branch exists, then reads v1.1. Sync stays best-effort; failures are logged, never gating the store choice. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: dee7c9fa1ebb
Trim verbose doc/inline comments in committed_read_store.go to the essentials, and factor shared test scaffolding (newTestRepo, writeV1Checkpoint, enableV11, setRef, ref-name helpers) so each test states only what it exercises. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: 12f59cd44f0f
Address Copilot review: syncV1CustomRefForRead treated any repo.Reference error on the v1.1 custom ref as "missing" and seeded/overwrote it, which could mask unexpected storer errors. Seed only on plumbing.ErrReferenceNotFound; on any other read error, log and leave the ref untouched (read it as-is). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: 80dd84520632
Collapse the 14 read-store test functions into 4: merge the two ref-getter checks, table the seven syncV1CustomRefForRead cases, and table the v1.1 read-behavior cases (found vs not-found, no v1 fallback). Trim the doc comments on NewCommittedReadStore and syncV1CustomRefForRead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: 19a4f72d6c69
pfleidi
reviewed
Jun 1, 2026
pfleidi
reviewed
Jun 1, 2026
pfleidi
previously approved these changes
Jun 1, 2026
Contributor
pfleidi
left a comment
pfleidi
left a comment
There was a problem hiding this comment.
I had two comments but only one question about the concrete changes to this PR. Looks great, otherwise!
Per review, NewCommittedReadStore is now a pure ref selector with no side effects; the v1.1 sync moves to an explicit SyncCommittedReadRef step that the explain read paths call before constructing the store. Also tighten the related doc comments and the V11Reads test header. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: 13a62c2d76d0
pfleidi
approved these changes
Jun 1, 2026
computermode
added a commit
that referenced
this pull request
Jun 1, 2026
Introduce a single value object that names the committed-metadata ref
topology for the active checkpoints_version, instead of re-deriving it
inline from settings.MirrorsToV1CustomRef plus the paths constants at
every read/write/mirror site.
CommittedRefs has three named refs:
- Primary: source of truth, written first and pushed/fetched
- Read: ref committed reads resolve against
- Mirror: advanced to Primary after writes, kept current before reads;
empty when there is no mirror (local-only, never pushed)
Invariant (holds in both rollout phases): Read == Mirror when a mirror
exists, else Read == Primary.
Two entry points:
- ResolveCommittedRefs(ctx): resolves from disk settings.
- ResolveCommittedRefsFromSettings(s): resolves from an already-loaded
EntireSettings, honoring an injected object (the path attach uses)
rather than disk. A nil settings object resolves to v1-branch-only.
Today Primary is always the v1 branch; opting into checkpoints_version
"1.1" sets Read and Mirror to the local-only v1.1 custom ref. Behavior
is unchanged — this is the seam that lets a future rollout phase flip
the write order (v1.1 as Primary) by changing the resolver alone instead
of editing every call site.
Addresses @pfleidi's review comment on PR #1311.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: f2ebba6141ef
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://entire.io/gh/entireio/cli/trails/465
Summary
Adds a v1.1 committed-checkpoint read path so
entire explainreads fromrefs/entire/checkpoints/v1.1whencheckpoints_versionis1.1. When v1.1 is enabled, reads bind to the v1.1 custom ref and never fall back to v1 — the v1.1 path is genuinely exercised rather than masked by a v1 fallback.Details
GitStoregains a configurablecommittedReadReffor reads only. Writes and read-modify-write paths always target the v1 branch (the durable source of truth) and are mirrored to v1.1 separately by the write-time mirror.NewCommittedReadStorealways binds reads to the v1.1 ref when v1.1 is enabled. Before returning, it best-effort sync-then-reads: advances the local-only v1.1 ref up to the v1 tip (seed when missing, advance when an ancestor, no-op when equal, leave a diverged ref as-is). Sync failures are logged, never gating the store choice — there is no v1 store fallback.origin/entire/checkpoints/v1when no local branch exists, soentire explainstill works on a fresh clone whose v1 metadata is remote-only — without reading through a v1 store.entire explain --generatewrites summaries through the v1 store (source of truth), then rebuilds the read store so v1.1 mode reloads the newly written summary after the next sync.Validation
go test ./cmd/entire/cli/checkpointgo test ./cmd/entire/cli -run Explainmise run lintRegression coverage: sync rules (seed, seed-from-origin, advance, equal, diverged, write-failure, no-v1-tip) and read-store behavior (ref selection, read equivalence with v1, remote-only metadata, and no-v1-fallback when the custom ref is unavailable or diverged).