feat(rcache): closure-union index reads behind index_source="closure" by bryan-minimal · Pull Request #995 · gominimal/minimal

bryan-minimal · 2026-07-28T16:25:33Z

Phase 2 of #870, per the design comment: read the bounded per-commit closure snapshots (<commit>.closure.shisha, ~53 KB and the signed object) instead of the full-index byte-copy (~3.1 MB and growing linearly — the ratio is 50× and widening).

What

mfile — IndexSourceMode::Closure (index_source = "closure" / MINIMAL_INDEX_SOURCE=closure) resolves the required upstream closure plus one closure per pinned git sideload, in declaration order (CacheConfig::CommitClosures { upstream, sideloads }). Directory and unpinned links have no remote presence and contribute nothing; an unpinned upstream is a loud error.
rcache — the reader fetches each closure through the existing immutable-snapshot path (same 68-byte wire format, whole-record strictness, local cache forever) and unions them into one index (IndexFile::merge). Asymmetric strictness: the upstream closure is required (it is the signed catalog — missing is SnapshotMissing naming the object, never a fallback to root), while sideload closures are best-effort — sideload repos are typically not covered by build infrastructure and publish no index, so a missing one is logged and skipped and its specs resolve locally, exactly as they do today. No publishing requirement is imposed on sideload authors. Overlapping entries across links must agree; divergence merges last-wins and logs a warning (an equivocation-shaped signal).
mctx — no changes; the config arm is handled inside new_any_configured.

Rollout

Mirrors phase 1 exactly: closure is the opt-in testing lever; auto is untouched (still byte-copy → root) until the mode is proven in fleet testing, then folding closure in as the first link of the auto chain is a follow-up. Kill-switch (root) and pinned unchanged.

Sideload note: sideloaded projects work in this mode with zero requirements on the sideload repos — published closures contribute to the union, unpublished ones are skipped with an info log. The only degradation for an unpublished sideload is cache misses on its specs (local builds), which is their behavior today anyway.

Testing

mfile: closure resolution with upstream + pinned sideload + dir sideload + unpinned sideload (order and skipping pinned), no-pins error, env spelling parse.
rcache: two-link union over real HTTP transport (both links' entries resolve through one index); missing upstream errors naming the object even with a valid root index present; missing sideload is skipped and upstream entries still resolve; IndexFile::merge conflict counting (identical overlap ≠ conflict, changed value = conflict, other wins).
Deliberately not included: signature verification ([cache.verify]) — separable follow-up per the design comment; require remains gated on the signing-side decisions.

🤖 Generated with Claude Code

Note

Add closure-union index reads behind `index_source="closure"` in rcache

Adds a new IndexSourceMode::Closure variant and CacheConfig::CommitClosures config, parsed from the string "closure" in mfile, carrying upstream and sideload closure object paths per commit.
Adds IndexFile.merge in rcache to union two indices, with incoming values winning on overlap and a conflict count returned.
Implements RemoteCache::new_any_closure_union in remote.rs: fetches the upstream closure snapshot (hard failure if missing), merges available sideload closures (skips missing ones with an info log), and warns on conflicting overlapping entries.
Risk: missing upstream closure snapshot is a hard error (SnapshotMissing) with no fallback to root.

^{Macroscope summarized 75a52ec.}

Phase 2 of the per-commit index work: read the bounded per-commit closure snapshots (~40x smaller than the byte-copy, and the signed object) instead of the full-index copy. - mfile: IndexSourceMode::Closure resolves one closure object per pinned git chain link (upstream first, then sideloads); dir and unpinned links contribute nothing; no pinned links is an error. - rcache: the reader fetches each link's closure through the same immutable-snapshot path (whole-record check, cached forever) and unions them into one index. Strict by design: a missing closure is SnapshotMissing naming that link's object, never a fallback. Overlapping entries must agree; divergence logs a warning. - Rollout mirrors phase 1: "closure" is the opt-in testing mode (file setting or MINIMAL_INDEX_SOURCE); auto is unchanged until the mode is proven. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai · 2026-07-28T16:25:43Z

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 26 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 22c2b354-345d-4e36-945b-99abaab04acd

📥 Commits

Reviewing files that changed from the base of the PR and between c1d466c and 75a52ec.

📒 Files selected for processing (3)

crates/mfile/src/lib.rs
crates/rcache/src/index_file.rs
crates/rcache/src/remote.rs

_{Comment @coderabbitai help to get the list of available commands.}

…ired Sideload repos are typically not covered by build infrastructure and publish no index; requiring a closure from them would make closure mode unusable for exactly the projects sideloads exist for. A missing sideload closure is now logged and skipped (its specs resolve locally, as they always have); the upstream closure remains required — it is the signed catalog. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

twitchyliquid64 · 2026-07-29T00:05:25Z

    Root,
+    /// The union of the per-commit *closure* snapshots of every pinned chain
+    /// link (upstream + sideloads); a missing closure is an error.
+    Closure,


If Closure just ignores sideloads isnt it the same as Pinned ?

Not the same on either axis: sideloads aren't ignored — each one's closure is fetched and unioned (conflict-checked); only a sideload that publishes no closure is skipped, and its specs resolve locally (sideload repos aren't covered by build infra, so requiring one would break exactly the users this mode serves). And the objects differ: pinned reads <commit>.shisha (byte copy of the whole root index); closure reads <commit>.closure.shisha (the bounded per-commit catalog — the object that gets signed). The enum doc said "missing closure is an error" for all links — stale, fixed in 75a52ec. Context: #870.

…t-effort asymmetry

bryan-minimal requested review from norrietaylor and twitchyliquid64 July 28, 2026 19:00

twitchyliquid64 reviewed Jul 29, 2026

View reviewed changes

docs(mfile): state the closure-mode upstream-required vs sideload-bes…

75a52ec

…t-effort asymmetry

bryan-minimal force-pushed the feat/closure-index-reads branch from 77c1374 to 75a52ec Compare July 29, 2026 16:37

bryan-minimal requested a review from twitchyliquid64 July 29, 2026 17:11

twitchyliquid64 approved these changes Jul 29, 2026

View reviewed changes

bryan-minimal merged commit c4ff314 into main Jul 29, 2026
29 checks passed

bryan-minimal deleted the feat/closure-index-reads branch July 29, 2026 17:50

bryan-minimal mentioned this pull request Jul 30, 2026

minimal sbom + blueprint: session/task SBOM (CycloneDX 1.5), extended to a CycloneDX 2.0 Blueprint #700

Open

5 tasks

Navigation Menu

Uh oh!

feat(rcache): closure-union index reads behind index_source="closure" - #995

feat(rcache): closure-union index reads behind index_source="closure"#995
bryan-minimal merged 3 commits into
mainfrom
feat/closure-index-reads

bryan-minimal commented Jul 28, 2026 •

edited by macroscopeapp Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

Review limit reached

Uh oh!

twitchyliquid64 Jul 29, 2026

Uh oh!

bryan-minimal Jul 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

bryan-minimal commented Jul 28, 2026 • edited by macroscopeapp Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Rollout

Testing

Add closure-union index reads behind index_source="closure" in rcache

Uh oh!

coderabbitai Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

twitchyliquid64 Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

bryan-minimal Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bryan-minimal commented Jul 28, 2026 •

edited by macroscopeapp Bot

Loading

Add closure-union index reads behind `index_source="closure"` in rcache

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading