perf(catalog): probe the next __manifest version by name instead of listing

[ragnorc]

What & why

This removes a listing of up to 1,000 keys (about 300 KB) from every warm read on a graph with a long history. A read on a warm graph handle checks whether its branch's __manifest moved before it reuses the coordinator it holds. That check called Lance's Dataset::latest_version_id, which lists _versions/. On a store with ordered listing (S3, GCS, Azure) Lance reads one page of up to 1,000 keys, so every branch past 1,000 commits paid a full page on every read. Below 1,000 attached versions, the listing also pages through the detached d*.manifest entries that unattached and unpublished commits leave in the same directory. The cost grew with history until it hit the page cap, and long-lived graphs sit at the cap.

The probe now asks for attached versions by name (ManifestNamingScheme::manifest_path, V1 or V2 paths). It HEADs the held version v and v + 1 concurrently, then v + 2 when v + 1 exists. While v exists, the first "not found" ends the history. So an unchanged branch costs two small HEADs in one round trip, and a branch one commit ahead costs three. An absent v falls back to the listing, and so do a gap of more than two versions, a detached handle and any error. The listing stays the authority. A removed __manifest answers "not found" for every name, so the listing still reports it as NotFound, and a write on a warm handle refuses before effects instead of recreating the directory. Lance's own probe_versions_upward (its S3 Express path) also checks the starting version first, and SlateDB ships the same next-version probe (its RFC 0032).

On a graph with a few dozen versions the listing is already one small page, so this change is roughly neutral there. A warm read still waits on one round trip, now a HEAD instead of a LIST. In the server's region that is about 20 ms saved per read; see Measurement.

Backing issue / RFC

• Implements part of the accepted Attribute the served read floor (~0.2 s server-side on a trivial read) and expose it as usage #752: item 3, "cut what the attribution shows". The listing is the largest remaining request of the served read floor that Attribute the served read floor (~0.2 s server-side on a trivial read) and expose it as usage #752 measured. Attribute the served read floor (~0.2 s server-side on a trivial read) and expose it as usage #752 stays open for its per-phase usage object.

Measurement

These are read-only timings of the probe's requests against a production graph on S3-compatible storage. Its main __manifest is at Lance version 3,334. Each cell is the median and range of 10 requests on one reused connection.

Request	From the server's container (same region as the bucket)	From a client outside the region
LIST _versions/ (1,000 keys, 308 KB)	40 ms (39–42)	623–679 ms (281–2,200)
HEAD of the next version (answered "not found")	19 ms (18–26)	176–183 ms (165–263)
HEAD of the held version	20 ms (19–21)	not measured

On a graph with 260 versions, the in-region LIST took 30 ms against the same 18–19 ms HEADs.

In the server's region, the probe saves about 20 ms per warm read: the two HEADs run concurrently, replacing one 40 ms LIST. A write probes twice, so it saves about 40 ms. The saving grows with the store's per-request latency and with the listing's size, up to the 1,000-key page. End-to-end hosted latency with this change is not measured.

Request cost, from the DST cost golden (crates/omnigraph-dst/cost_table.txt, whole suite, main → this PR):

	Before	After	Change
LIST	889	588	−301 (−34%)
GET/HEAD	2,985	3,306	+321 (+11%)
All calls	4,120	4,140	+20 (+0.5%)

• Single writes and harness reads list nothing now. That covers DeletePerson, UpdateV, EnsureIndices and the harness's _history and _close reads.
• Each operation trades listings for counted reads one for one. The table does not appear to record the probe's "not found" HEAD (inferred from the per-operation totals).
• Request spend falls 22–24%. GQT --measure's price model (LIST 5.0 µ$, GET or HEAD 0.4 µ$, S3 request pricing) makes a probe go from 5.0 µ$ to 0.8 µ$. Read-request spend across the suite falls from about 5,640 µ$ to 4,260–4,380 µ$.
• The table counts calls, not latency or bytes. Its graphs hold a few dozen versions, so it cannot show the cost of a 1,000-key page. The latency case rests on the timings above.

Why the first absent name is the latest

• Lance writes an attached version only with a put-if-absent of read_version + 1. The catalog's publish is an Overwrite with zero retries, and Lance's S3 commit handler uses the same conditional put.
• Nothing deletes a version of a live branch's __manifest. cleanup collects graph-table versions and retired branch trees, and storage: safely reclaim obsolete physical __manifest versions and files #644 (reclaiming old __manifest versions) is open. The code comment and the new docs/dev/lance.md fence row state the constraint storage: safely reclaim obsolete physical __manifest versions and files #644 must keep: every version above one a live handle holds stays.
• A retired branch whose tree was collected, or a removed root, has no held version, so the listing answers. The named-branch probe's identifier read also catches a delete and recreate, as before.
• Unattached and unpublished commits are detached versions, so they never occupy an attached name.
• The probe needs the store to show a completed PUT to a later HEAD, which is the guarantee the listing already relied on.

Checklist

• Change is focused (one logical change)
• Tests added/updated for behavior changes
• Public docs updated: docs/dev/lance.md fence row and changelog.d/catalog-freshness-probe-heads-next-version.performance.md
• Reviewed against docs/dev/invariants.md. The probe is a mutable-tip hint, and writers still revalidate through the version CAS. A removed __manifest still fails before effects. No new state or queue.

Local verification

• cargo test -p omnigraph-catalog --locked: 207 passed, including freshness_probe_asks_for_the_next_version_by_name. With latest_attached_version reduced to the old latest_version_id call, that test fails on its first assertion.
• RUST_MIN_STACK=16777216 cargo test --workspace --exclude omnigraph-gqt --exclude omnigraph-dst --locked --features omnigraph-engine/failpoints,omnigraph-cluster/failpoints --no-fail-fast: all passed. The first local run without RUST_MIN_STACK overflowed the stack in omnigraph-bench --lib and in engine integration. Both were re-run with CI's stack size: bench lib 406 passed, engine integration 616 passed with 8 ignored. That run includes lifecycle::write_preparation_manifest_read_failures_carry_before_effect_evidence, which failed on the first push of this PR.
• cd crates/omnigraph-dst && DST_REGEN_COSTS=1 cargo test --locked --test scenarios dst_bench_cost_count_golden, then cargo test --locked there: 53, 2 and 51 passed.
• cargo test -p omnigraph-gqt --locked --test runner_dispatch measure_counts: passes. The full --lib --test runner_dispatch run passes except concurrent_block_refusals, whose starved_wall fixture hits its wall-time budget under this machine's load. It passes alone, and it passed in CI on this PR.
• cargo fmt --all --check: ok.
• cargo clippy -p omnigraph-catalog --all-targets --locked -- -D warnings -W clippy::dbg_macro: ok.
• python3 scripts/check-docs.py: ok. typos on the changed files: ok.
• Not run locally: the GQT case corpus, the S3/RustFS and Azure suites, and the DST workflow's clippy graph.

Notes for reviewers

• The test. freshness_probe_asks_for_the_next_version_by_name runs on Lance's shared-memory:// store, whose listing is ordered and shared across handles, so it exercises the LIST that local-path stores never issue (they read_dir). It covers V1 and V2 paths:

  • unchanged: HEADs and no LIST;
  • one commit by another writer: HEADs and no LIST;
  • a gap of three versions: the listing answers;
  • a removed __manifest: the listing's NotFound.

  It extends the existing HistoryGets counter with LIST counts.

• Warm-read cost tests. warm_read_cost.rs now pins one recorded __manifest request on a warm read, the probe's HEAD of the held manifest version. On local stores the old listing was a read_dir the tracker never saw, which is why the count was zero. The HEAD of the next version answers "not found", and the tracker records no failed request. Lance's tracker reports a HEAD's range as the object's whole size, so the assertions name the request by path rather than by bytes.

• GQT measure test. measure_counts_schema_contract_requests_issue_817 allows a write step one more repeat read: the freshness probe's HEAD of the held __manifest version. A write probes when it captures its branch and again when it revalidates that capture. A read probes once.

• DST cost golden. cost_table.txt moves calls from l.list to l.get: about two small HEADs replace each _versions/ listing.

[ragnorc]

Recommendation: request changes. This is a COMMENT review because the authenticated account is the PR author.

This PR reduces the storage work before a query can reuse a warm graph handle. The old check lists manifest names. The new check asks whether the next one or two names exist. It avoids a large LIST response when the reader is current or one commit behind. It retains Lance's latest-version lookup for larger gaps, detached handles, and probe errors.

The normal path works in the focused tests. However, the new negative check also accepts missing manifest authority as a current cached head. That breaks the existing failure contract. See the inline P1. Fix this at the shared catalog probe, so reads, write capture, merges, and unpublished/unattached validation all retain the same rule. Do not suppress the lifecycle test's directory error: that error exposes effects after authority disappeared.

Design and tradeoffs. Attached publication uses Overwrite with zero retries and skips automatic cleanup (commit owner). I checked these assumptions against the lockfile's Lance 11.0.0 source, including strict overwrite, manifest naming, object-store HEAD, branch paths, and latest-version resolution. I also checked callers and branch retirement. The local dependency archives match Cargo.lock, and the inspected sources match those archives.

Lance's own upward probe first confirms that its starting version exists. This helper omits that positive witness. A missing successor proves no advance only while the underlying authority remains valid.

The change adds no persistent state, public API, or second commit authority. One private helper is preferable to separate optimizations in each caller. It removes repeated listing payloads, but adds a continuity requirement and a fixed probe budget. Caught-up readers pay one HEAD. One-commit lag pays two HEADs. Two or more commits of lag pay two HEADs plus the old lookup. This favors frequent reads and sparse writes. It can cost more under sustained writes or on a local filesystem, where Lance can read the directory directly. The PR's cloud timings are author measurements of individual requests. I did not reproduce them or measure served latency.

The lasting liability is retention: #644 cannot remove an intermediate version above a held reader without changing this probe. The documented fence helps, but future GC must solve that interaction by design. Five caller-specific versions of this optimization would multiply that obligation. Keep one owner and explicit tests for absence, advance, gaps, and storage errors.

I surveyed the other 32 open PRs. This change complements immutable table-handle reuse in #818 and the authority-cache work in #825. Group commit (#785) can change how often readers lag, so its cost results should include this fallback. Unattached-root benchmarks (#961) remain relevant because unpublished validation calls this probe. Planner, language, deployment, and documentation proposals do not remove its freshness obligation. Lance 13.0.0 still separates general latest-version lookup from the restricted successor probe. An upgrade alone does not discharge the continuity or existence checks.

Local validation. Production code came from these exact commits:

• Before: 47dfe7b4a00c1d6405bc076dc6f4f549ae6fc06c.
• Reviewed head: e3f1e1268447ddb4405f5804c72cf7ac9e4e2fa6.

Commands used Rust 1.97.1, RUST_MIN_STACK=16777216, RUSTC_WRAPPER=, the repository's Tokio configuration, and a reused build directory. The explicit build flags were RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable'.

Check	Before	Reviewed head
cargo test -p omnigraph-catalog --locked freshness_probe -- --nocapture	PR request-count test fails: (0,0,0) instead of (0,1,1). Temporary missing-authority test passes.	Request-count test passes. Temporary missing-authority test fails: returns Ok(ManifestIncarnation { version: 1, ... }).
cargo test -p omnigraph-engine --locked --test integration lifecycle::write_preparation_manifest_read_failures_carry_before_effect_evidence -- --exact --nocapture	1 passed	1 failed: restoring __manifest reports DirectoryNotEmpty, matching CI.
cargo test -p omnigraph-catalog --locked	Not run	207 passed
cargo test -p omnigraph-engine --locked --test integration -- warm_read_cost:: branching:: consistency:: point_in_time:: write_cost::	Not run	139 passed

For the comparison, I copied the PR's test additions to the base and declared the budget 2 inside the test on both commits. Production code was unchanged. I also ran this same temporary catalog test on both commits:

#[tokio::test]
async fn freshness_probe_refuses_missing_manifest_authority_review_962() {
    let dir = tempfile::tempdir().unwrap();
    let uri = dir.path().to_str().unwrap();
    let coordinator = ManifestCoordinator::init(uri, &build_test_catalog()).await.unwrap();
    let manifest = dir.path().join("__manifest");
    let held = dir.path().join("manifest-unavailable");
    std::fs::rename(&manifest, &held).unwrap();
    let result = coordinator.probe_latest_incarnation().await;
    std::fs::rename(&held, &manifest).unwrap();
    assert!(result.is_err(), "missing manifest authority must refuse, got {result:?}");
}

Please include this minimal regression, or its equivalent in the existing owner, with the fix. GQT cannot express removal of the internal authority directory or exact HEAD/LIST counts. The catalog and existing lifecycle Rust owners directly test those contracts. All temporary edits were restored. The checkout is clean at the reviewed head.

CI and limitations. The reviewed head passed ordinary, DST, and served GQT jobs, format, clippy, and storage-upgrade compatibility. The overall gate is red:

• Workspace test failure: the lifecycle regression above. CI stopped after 2,880 of 4,338 tests.
• DST failure: the cost golden needs a reviewed update after the fix. For example, AddFriend LISTs change 16→9 and _verify GETs change 1425→1443. The meter counts successful GETs after the call, so missing HEAD attempts are not a complete request-cost total.
• Fix Regression Gate: the gate interpreted the body as closing #752 and found no issue-named test. Name the existing Rust regression for the issue, or correct the gate's closure interpretation. This PR does not complete #752's per-phase attribution work.

A first local engine build ran out of disk space. After removing confirmed unused incremental Rust artifacts, the focused builds and comparisons completed. I did not run the full workspace, local DST suite, live S3/Azure tests, or a latency benchmark. No additional optional code changes are requested.

[ragnorc]

[P1] Preserve missing-authority refusal before reusing the cached head

Ok(false) also means that the whole __manifest tree is absent. In that case this returns the held version without proving that any authority still exists. The main-branch arm then returns the cached incarnation, so callers accept it as current and can proceed into durable work. This is observable now: lifecycle::write_preparation_manifest_read_failures_carry_before_effect_evidence passes at 47dfe7b4 and fails at e3f1e126 because operations recreate a nonempty __manifest directory while the original is unavailable. The smaller catalog reproduction in the review also changes from an error to Ok(version=1).

Preserve a positive authority/existence check at this shared probe before treating a missing successor as freshness. Cover missing authority across its read and write callers, and keep the existing before-effect contract. Fixing only the lifecycle caller or ignoring its restore error would leave the same cause in cached reads and merge/proposal validation.

[ragnorc]

Recommendation: approve 82ec066bc16428b956590bbae25bca944c52ce9d. The missing-authority P1 from my previous review is resolved. This is a COMMENT review because the authenticated account is the author. I found no remaining blocking defect and request no optional implementation change.

This PR reduces the storage work before a query can reuse a warm graph handle. Instead of listing manifest history, it checks the held version and the next version by name. A current reader avoids a potentially large LIST response. The revised probe also checks that its starting authority exists. Removing __manifest now causes refusal instead of allowing cached authority to reach write effects.

The fix belongs in the shared catalog probe. Both quick returns require a successful HEAD of the held version. An absent held version, a probe error, or a larger gap uses Lance's existing latest-version lookup. This restores the invariant for reads, write capture, merge capture, and captured proposal/unattached validation together. The named-branch identifier check still distinguishes branch lifetimes. No caller-specific exception hides the failure.

I checked the current guides, relevant upstream pages, adjacent callers, storage retirement, and the pinned Lance 11.0.0 implementation. Catalog publication uses strict Overwrite with zero retries and disables automatic cleanup. Attached versions remain contiguous under the supported storage contract. Lance's own upward probe also checks its starting version before accepting an absent successor.

The request tradeoff is concrete:

Reader position	Probe work before any refresh
Current	Two concurrent HEADs, no LIST
One version behind	Three HEADs across two rounds, no LIST
Two or more versions behind	Three HEADs, then the existing latest-version lookup

This favors frequent reads with little intervening publication and large remote version directories. It can increase cost for lagging readers or small local datasets. The successful-read meter omits absent HEAD attempts, so its GET total alone is not the request total. I did not reproduce the author's cloud timings or establish a served latency improvement.

The change adds one private decision rule, with no persistent state, public API, or second authority. The fix removes the need for caller-specific missing-authority guards. Its remaining liability is the continuity assumption: future physical manifest GC in #644 must preserve the required range or replace this probe. The documented fence and V1/V2 tests make that obligation visible. Five future callers should reuse this owner rather than copy the algorithm.

I also surveyed the other open PRs. Immutable table-handle reuse (#818) complements this check. Group commit (#785) changes the distribution of reader lag, so its measurements must include the fallback path. Live-branch schemas (#963) and graph diff/merge preview (#964/#966) still need fresh authority before fixing their snapshots. Native admission work (#893/#899) does not change this rule. Lance 13.0.0 still exposes general latest-version lookup separately from the restricted upward probe. A future upgrade needs a new compatibility check.

Regression proof. Before the fix: e3f1e1268447ddb4405f5804c72cf7ac9e4e2fa6. After: 82ec066bc16428b956590bbae25bca944c52ce9d.

• I reran the unchanged lifecycle regression on both commits with cargo test -j3 -p omnigraph-engine --locked --test integration lifecycle::write_preparation_manifest_read_failures_carry_before_effect_evidence -- --exact --nocapture. Before: one failure, because effects recreated the removed authority directory. After: one pass, including BeforeEffect/NotFound checks for nine write doors and unchanged branches/commits.
• I reused the same minimal temporary catalog probe from the previous review. It initializes one catalog, moves __manifest aside, probes, restores the directory, then requires an error. The recorded before-run (cargo test -p omnigraph-catalog --locked freshness_probe -- --nocapture) failed that assertion by returning Ok(version: 1). The new run passed. This direct refusal assertion establishes the behavioral failure independently of lifecycle teardown.
• The PR now includes the equivalent missing-authority assertion in its existing V1/V2 catalog test. No additional regression is needed. GQT cannot express removal of the internal authority directory or exact HEAD/LIST operations, so these Rust owners are appropriate.

Other local checks. cargo test -j3 -p omnigraph-catalog --locked --lib -- --nocapture: 208 passed (207 checked-in tests plus the temporary probe). cargo test -j3 -p omnigraph-engine --locked --test integration warm_read_cost:: -- --nocapture: 27 passed. cargo test -j3 -p omnigraph-gqt --locked --test runner_dispatch measure_counts_schema_contract_requests_issue_817 -- --exact --nocapture: one passed. Documentation, AGENTS links, formatting, and diff checks passed. All commands used Rust 1.97.1, RUST_MIN_STACK=16777216, RUSTC_WRAPPER=, CARGO_TARGET_DIR=/tmp/review-928/target, and RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable'. Temporary test edits were restored.

CI evidence and limits. CI belongs to this PR head but checked out merge commit 998317cf80c00f0ada532360e3369213f7b67dc8, which includes other main changes. It is separate evidence from the exact-head local runs. Workspace CI passed 4,342 tests, with 32 skipped. DST CI passed the cost golden. GQT CI passed 349 corpus cases and all 26 dispatch tests, including the changed schema-request count test. I did not run the full workspace, full DST suite, or live S3/Azure checks locally.

[ragnorc]

Recommendation: approve f2c65354979e7905b5971f6f340bf34ce2b69d19. This follow-up covers the merge of current main since my previous review. The freshness algorithm is unchanged. I found no blocking defect or optional implementation change to request. This is a COMMENT review because the authenticated account is the author.

This PR reduces storage work before a warm graph handle can be reused. The shared catalog probe checks the held manifest version and its successor by name. It accepts an absent successor only when the held version exists. A missing authority, storage error, detached handle, or larger gap uses the existing latest-version lookup. Named branches retain their lifetime check. The earlier missing-authority defect remains fixed for reads, write capture, merge capture, and captured validation through this common owner.

The merge preserves the relevant contracts. On a coordinator-cache hit, the code still probes freshness and removes a failed entry. Opening a cache miss outside the lock does not grant that entry publication authority; capture stays under the returned guard, and writes retain revalidation and version CAS. Historical alias binding receives the accepted catalog and live snapshot together. It can therefore compare physical table identity against the same accepted view.

I checked the repository guides, relevant upstream documentation, callers, retirement behavior, deployment boundaries, and pinned Lance 11.0.0 source. Catalog publication uses strict Overwrite with zero retries and disables automatic cleanup. Live manifest versions remain contiguous. Lance's own upward probe also establishes a starting version before accepting an absent successor.

The tradeoff remains: a current reader uses two concurrent HEADs and no LIST; a reader one version behind uses three HEADs across two rounds; a reader further behind uses three HEADs before the existing lookup. This favors frequent reads with little intervening publication and large remote histories. Lagging readers and small local histories can pay more requests. The successful-read counter excludes absent HEAD attempts, so it does not report the whole request cost.

The liability assessment is unchanged. This adds one private decision rule, with no persistent state or public API. The held-version check removes the need for separate missing-authority guards in each caller. Five future callers can reuse this owner. The remaining obligation is continuity: physical manifest GC in #644 must preserve the required range or replace this probe.

I surveyed the other open PRs. Immutable table-handle reuse (#818) complements this check. Group commit (#785) needs measurements across reader lag and fallback. Live-branch schemas (#963), diff/merge preview (#964/#966), and actor views (#951) must retain one accepted authority when fixing their read view. Lance 13.0.0 still separates general latest-version lookup from the restricted probe; its availability does not remove the need to recheck naming, commit, and cleanup contracts at a future dependency bump.

Fresh regression proof. Before: e3f1e1268447ddb4405f5804c72cf7ac9e4e2fa6. After: f2c65354979e7905b5971f6f340bf34ce2b69d19.

• The unchanged lifecycle regression failed before: write effects recreated the removed manifest directory, so restoration failed with DirectoryNotEmpty. It passed at the reviewed head, including refusal evidence and unchanged graph state.
• The same temporary catalog test ran on both commits. It creates one catalog, moves __manifest aside, probes, restores the directory, and requires an error. Before: one assertion failure, because the probe returned Ok(ManifestIncarnation { version: 1, ... }). After: one pass. This confirms the behavioral cause independently of lifecycle teardown.
• The PR already includes the equivalent missing-authority assertion for V1 and V2. GQT cannot remove an internal authority directory or assert exact HEAD/LIST operations. The existing Rust owners cover those contracts. No extra regression is requested. All temporary edits were restored.

On the before commit, I ran the lifecycle command below. I ran the direct probe command on both commits:

cargo test --locked -p omnigraph-engine --features failpoints --test integration -j 3 lifecycle::write_preparation_manifest_read_failures_carry_before_effect_evidence -- --exact --nocapture
cargo test --locked -p omnigraph-catalog --lib -j 3 tests::freshness_probe_refuses_missing_manifest_authority_review_962 -- --exact --nocapture

At the exact head, the same lifecycle case ran in this focused set, followed here by the catalog-suite command:

cargo test --locked -p omnigraph-engine --features failpoints --test integration -j 3 -- --nocapture warm_read_cost:: point_in_time:: lifecycle::write_preparation_manifest_read_failures_carry_before_effect_evidence
cargo test --locked -p omnigraph-catalog --lib -j 3 -- --nocapture

Local results: 46 engine tests passed: 27 warm-read, 18 historical-snapshot, and one lifecycle test. All 207 checked-in catalog tests passed. The temporary direct probe passed separately. Formatting, AGENTS links, and documentation checks passed. The documentation check first needed the missing release tag fetched into this checkout. Commands used Rust 1.97.1, RUST_MIN_STACK=16777216, RUSTC_WRAPPER=, CARGO_TARGET_DIR=/tmp/review-928/target, and RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable'.

CI and limits: Workspace CI passed 4,342 tests, with 32 skipped. It checked out 77ae96ddf29759baef7e0a0cb7ced9d509f1a2d3; I verified that its tree exactly equals this reviewed head. DST, ordinary GQT, and served GQT also report success. Those CI results are separate from the local runs. I did not rerun the full workspace or live S3/Azure tests locally, reproduce the author's cloud timings, or establish a served latency improvement.