[ragnorc]

Part of #972. A commit that only a branch's __manifest buffers (every commit of an unmerged branch) was found by listing every live branch ref and checking out branch manifests one after another, newest first, on every lookup. A diff side spelled as such a commit looks it up twice, and a commit's own changes up to three times. On a hosted graph with nine live branches, GET /commits/{id}/changes on a one-change branch commit took 13–14 s and a diff of a branch head against itself 13.6 s (measurements in the #972 comment). OmniGraph Studio reads a branch's commits one after another for history and review, so it pays this on every read.

A graph handle's HistoryCache now remembers, per history block, the branch a lookup found a buffered commit on. One native branch writes a block, so the block's other commits stay buffered there until they are released or merged. resolve_commit, commit_state and published_record ask that branch first, before __history and the scan of every live branch: one __manifest open and one branch scan. The guard is that the hint only chooses where to look first. The record still comes from that branch's own __manifest, read the way the scan reads it. A branch that no longer holds the commit, whether released, merged, deleted or recreated under a new native name, drops the hint and the lookup runs as before. The incarnation check that answers 404 for a deleted branch's commit is unchanged. The cache holds at most 4,096 block names per handle, and the new ManifestCoordinator::record_in_hinted_branch is registered as a read-only coordinator surface.

warm_read_cost::by_id_read_of_a_block_found_before_scans_only_its_branch reads two commits of one block from a handle on main with four newer branches. The first lookup scans six branches and the second one; with the hint disabled the second also scans six. The graph as of the hint-found commit reads its rows correctly, and after the branch is deleted the commit still resolves through __history. The neighbouring lookup-order test keeps its counts. The catalog and engine suites pass with the 16 MiB stack CI sets (composite_flow_multi_branch_sequential_merges overflows the default debug stack on main as well), and clippy passes on the pinned 1.97.1 toolchain.

For review: the decision to challenge is answering from the hinted branch's copy of a record before __history. The scan already returns whichever branch holds the commit first, in an order that depends on the handle's own branch, so this relies on nothing the scan did not already rely on. The first lookup of each block still scans every branch; #972's durable address is the fix for that, and this PR leaves it open. I could not run the GQT corpus locally: on this machine under load, every debug case that failed exceeded its 10 s wall-time budget, none with an assertion failure, and one that timed out under load (branch_create_at_a_commit) passed in 8 s alone; CI's GQT job is the check for the corpus. Not measured on the hosted graph.

[ragnorc]

Revise this change before merge.

Why. A by-id read that misses the current branch scans every live __manifest branch. A history list and a review do that read once per commit. The cost grows with the number of live branches.

What. The handle remembers the native branch where it last found a commit of a history block. The next read of a commit in that block opens that branch first. Then it reads __history. Then it scans the live branches.

How. HistoryCache stores the block id and the native name. None means main. record_in_live_branches records the home when a scan finds the commit. resolve_commit, commit_state, and published_record call record_in_hinted_branch before __history.

Impact. The second and later commits of a known block open one branch and scan one branch. The first lookup of a block still scans every live branch. Issue 972 stays open. On the current code a warm handle can also fail a read that __history can answer, and the merge into current main does not compile.

Blocking

1. A missing branch tree stops the lookup

record_in_hinted_branch treats only RefNotFound as a miss. Lance 13.0.0 checkout_branch returns NotFound when the branch tree has no _versions directory. That return is Error::NotFound at lance-table 13.0.0 src/io/commit.rs:653.

Branch delete copies the commits that branch wrote into __history and leaves the tree. Later cleanup removes __manifest/tree/{native}. The maintenance test cleanup_reclaims_retirement_archive_after_the_age_window shows that removal when older_than is Duration::ZERO after the age window.

The next get_commit, snapshot read, or branch-create-from-commit on that handle returns the storage error. The hint stays. Later reads of that block fail the same way. __history can still serve the commit. capture_commit_start_source uses published_record, so branch create from that commit fails on the same path.

The new cost test does not catch this. After delete, checkout of the retired tree still succeeds. The test says the commit is found in __history past a stale hint. That holds only while the tree remains.

Fix the miss in record_in_hinted_branch. Treat NotFound the same as RefNotFound. Forget the home. Return Ok(None). The caller then reads __history and the live branches. A commit that exists only on the missing tree still fails when the later scan meets the same error.

Add a regression in warm_read_cost or the maintenance owner. Learn the hint. Delete the branch. Reclaim the tree the way the age-window test does. get_commit must still return the commit. A .gqt case cannot express a warm handle plus cleanup.

2. The merge into current main does not compile

CI run 40477076158 is a pull_request run of head b5916cbc. It builds merge commit 194ead86 (merge of b5916cbc into 790174c).

Lint, Test Workspace, and Storage Upgrade fail with queries overflow the depth limit in omnigraph-bench. The note names omnigraph-bench.rs:564 and a depth increase of 130.

Azurite fails the same way in omnigraph-cluster test failpoints. The note names async fn body of offline_fixture() and a depth increase of 130.

CI Gate fails because those jobs fail. Main push run 40440899362 at 790174c succeeded. Bench sources match between the merge base and that main commit. The new unboxed async read is what the merge adds inside futures that already meet the compiler depth limit.

refresh already boxes refresh_inner for this hazard. The comment there names the engine stack-depth hazard and keeps that layout out of every caller frame. Box the hinted read the same way. Box it inside record_in_hinted_branch, or at both awaits in record_held_elsewhere and resolve_commit. Do not raise recursion_limit.

Optional

HistoryCache::reset clears settled commits, history objects, and cold registry rows. It does not clear homes.

refresh_inner calls reset when the manifest image changes at the held version. The comment says nothing held from the replaced root describes the new one. The block homes stay.

The hint is read against the current root, so it does not return stale commit bytes. After the NotFound miss is fixed, a stale name is one failed checkout and then the normal lookup. Clear homes in reset as well. One function then owns the rule that this cache does not describe a replaced root. Five more maps that forget reset is the liability to avoid.

Tradeoff and liability

The change removes a scan of every live branch on the second and later commits of a block this handle already found. That matches a history walk on one branch.

The first lookup of a block still scans every live branch. This pull request does not close issue 972. A durable commit-to-branch address remains the general fix. No other open pull request owns this lookup. Pull request 818 caches a different fact, the version a write committed.

The change adds a process-local hint. The hint is not a second source of truth when a miss continues into __history and the live-branch scan. Today a NotFound checkout does not continue. The hint can then fail a read that __history can answer. That liability is on the error path, not on record identity.

HistoryCache already owns the hint state. record_in_hinted_branch must own the miss policy. reset must own invalidation. Those three owners are enough.

Reading the branch buffer before __history keeps the same record when both copies exist. Catalog test block_placement_preserves_fork_and_parent_records_through_two_releases keeps the fork archive and the parent archive equal. The commit row is stored as written. tables_at uses the earliest table replacement. I checked this order. The records stay equal.

A fork stored as the home of a block the writer later extends causes one miss and then a full scan. The result stays correct.

Validation

Reviewed head b5916cbcf754cf6a3a1c9a133dcd87e8698abcb6. Merge base da1c46368c33876d61615add0ceb6778f9ce2b58. Toolchain rustc 1.97.1.

Same command on both commits, from isolated checkouts, with CARGO_TARGET_DIR=/workspace/target:

cargo test --locked -p omnigraph-engine --test integration -- warm_read_cost::by_id_read_of_a_block_found_before_scans_only_its_branch warm_read_cost::temp_commit_stays_readable_after_hinted_branch_tree_is_reclaimed warm_read_cost::by_id_read_of_a_commit_held_elsewhere_scans_the_likely_branches_first -- --test-threads=1 --nocapture

The third name is a temporary test. It is not in the pull request. I removed it after the runs.

Head result. The cost test passed. The neighbor test by_id_read_of_a_commit_held_elsewhere_scans_the_likely_branches_first passed. The temporary test failed at get_commit after removal of every entry under __manifest/tree. The error was Storage(NotFound) for tree/task.../_versions, from lance-table-13.0.0/src/io/commit.rs:653. Asserts before that removal passed, including equal commit records and people_as_of on the second commit. Result: 2 passed, 1 failed, 640 filtered out.

Merge-base result. The temporary test passed. The cost test failed with left: (1, 6) and right: (1, 1) on the second commit of the block. The second read still scans six branches. Commit equality asserts before that line passed. The neighbor test passed. Result: 2 passed, 1 failed.

The scan count is the performance difference. The lost read after tree removal is the behavior regression. It fails on the head and passes on the merge base.

CI run 40477076158 on merge 194ead86: clippy, Test Workspace, Storage Upgrade, and Azurite fail at compile time. Those jobs stop before tests, so they do not show query behavior. GQT and the DST pinned suite passed on that run. I did not rerun them locally.

Lance inspection. Dataset::checkout_branch uses checkout_by_ref, then find_branch on tree/{name}/, then resolve_latest_location. An empty or missing _versions listing returns Error::NotFound.

Limitations

I did not rerun workspace clippy, the full workspace test graph, GQT, or DST on this machine. I did not measure hosted-graph wall time. The local proof is the open count and the scan count.

The local tree removal was remove_dir_all of every entry under __manifest/tree. It was not a full cleanup(older_than: ZERO) after a backdated retirement. The maintenance test is the evidence that cleanup removes that tree. The observed error is the missing _versions listing that checkout returns when that directory is gone.

Temporary edits were restored. I did not push.

Recommendation: revise.

Inline comment, crates/omnigraph-catalog/src/lib.rs line 2441, side RIGHT:

checkout_branch returns NotFound, not only RefNotFound, when the branch tree has no _versions directory. Lance 13.0.0 builds that as Error::NotFound at lance-table src/io/commit.rs:653.

Branch delete leaves the tree. Cleanup later removes __manifest/tree/{native} (cleanup_reclaims_retirement_archive_after_the_age_window, older_than: Duration::ZERO after the age window). This arm then returns the storage error and keeps the hint. __history can still serve the commit. get_commit, commit_state, and published_record all stop here. capture_commit_start_source stops here too.

Treat NotFound as the RefNotFound arm. Forget the home and return Ok(None).

Local proof on b5916cbc. Two commits on branch task, a second handle reads both ids, branch_delete("task"), then removal of every entry under __manifest/tree. The next get_commit failed with Storage(NotFound) for tree/task.../_versions at commit.rs:653. The same steps passed on merge base da1c463, which has no hint. The new cost test does not remove the tree, so checkout still succeeds after delete.

Inline comment, crates/omnigraph-catalog/src/lib.rs line 2421, side RIGHT:

This unboxed async read is inlined into callers that already meet the compiler depth limit.

CI run 40477076158 builds merge 194ead86 (b5916cbc into 790174c). Lint, Test Workspace, and Storage Upgrade fail with queries overflow the depth limit in omnigraph-bench.rs:564 (depth +130). Azurite fails the same way in omnigraph-cluster test failpoints (offline_fixture, depth +130). Main push run 40440899362 at 790174c succeeded.

Box the await the way refresh boxes refresh_inner in this file. The comment on refresh_inner names the stack-depth hazard. Box it here, or at both awaits in record_held_elsewhere and resolve_commit. Do not raise recursion_limit.

[ragnorc]

Review of head b5916cbc (base da1c4636; main has moved on).

Why, what, how, impact

• Why: A commit that only a branch's __manifest holds is found by a scan of every live branch. Tools that read a branch's commits one by one pay that scan on each read (#972).
• What: A handle now remembers which branch held a history block's commit. The next lookup for a commit of that block reads that branch first.
• How: HistoryCache.homes maps block to branch. ManifestCoordinator::record_in_hinted_branch is tried before __history in resolve_commit and in the new record_held_elsewhere. A miss or RefNotFound drops the hint.
• Impact: The second and later commits of a block cost one open and one scan, not one scan per live branch.

Blocking findings

None. The hint only picks where to look first. The record still comes from a branch's own __manifest, as the scan reads it. Records are immutable by commit id, so answering before __history gives the same record. commit_state still runs the incarnation check after the lookup. I found no input that gives a wrong answer.

Optional findings

1. The test does not prove that the hint is dropped (warm_read_cost.rs, new test). After branch_delete("task") it checks only the result. The commit would also resolve with no forget_block_home call, because the later steps fall back to __history. Please assert the open and scan counts of that lookup (one wasted open, then __history) and of a second lookup (no wasted open). Add a released or merged case too. The PR text claims these cases but no test covers them.
2. The hint can flip back and forth. The scan stores a hint, and a miss removes it. If a block has commits both still buffered and already in __history, alternate lookups of those commits pay one extra open and scan each time. I did not check whether a block can be in that mixed state, so this is unverified. If it cannot happen, a one-line comment saying so would remove the doubt.
3. Eviction clears all hints. At 4,096 blocks, remember_block_home clears the whole map. A workload that cycles through more than 4,096 blocks then gets no benefit and does extra work. This is probably rare. A bound that evicts one entry, or a comment that names the assumption, would be safer.
4. A hint error fails the lookup. An error other than RefNotFound (for example a storage error on checkout_branch) returns Err before __history is tried. The old order could have answered from __history. The scan already behaves this way, so this is consistent. Still, the PR text says "the hint only chooses where to look first". Treating a hint error as a miss would match that text. Hiding storage errors has a cost too, so I leave this to you.
5. The test hard-codes (1, 6) scan counts. They depend on scan order (own branch, main, newest branch first). The neighbouring lookup-order test has the same coupling, so this is acceptable. Keep the two tests in step if the order changes.

Tradeoffs and software liability

• Added: one cache field with a manual size bound, three small methods, one read-only coordinator surface, and one more step in the lookup order. About 190 lines in total, most of them the test.
• Removed: nothing. This is a cache on top of a scan that stays. Five similar changes would give five hints with five invalidation rules.
• Better owner: #972's durable address removes the scan and the hint together. This PR is a stop-gap with a clear exit, and the PR text says so. I accept it on that basis. Please link the removal to #972 in the code comment.
• Single source of truth: homes copies facts that live in branch __manifest files. It cannot hold a wrong record, because the record is always re-read. It can only cost an extra read.
• Future direction: I did not see a conflict with the open RFC PRs that touch history or branch bases. #1235 and #1234 change how branch bases follow a copied root. The hint stores native branch names, not paths, so I expect it to survive them. This is unverified.

Validation

• Local, new test on the PR head b5916cbc: CARGO_TARGET_DIR=<shared target> RUST_MIN_STACK=16777216 cargo test -p omnigraph-engine --test integration by_id_read_of_a_block_found_before passed (1 passed, 641 filtered out).
• Local, same test before the fix: I restored the production files of crates/omnigraph-catalog and crates/omnigraph/src to base da1c4636 and kept the PR's test. It failed at warm_read_cost.rs:2443 with left: (1, 6), right: (1, 1). The cause is the predicted one: the second lookup scanned all six branches. I restored all edits afterwards.
• Local, neighbouring test on the PR head: by_id_read_of_a_commit_held_elsewhere_scans_the_likely_branches_first passed.
• GQT: The contract is storage I/O counts, which GQT cannot express. warm_read_cost.rs is the right owner, so I asked for no GQT case.
• CI: The run on this head has red checks (Lint (clippy), Test Workspace, Storage Upgrade Compatibility, Azurite, CI Gate). The logs I read show queries overflow the depth limit while compiling omnigraph-bench, which this diff does not touch. The run dates from 2026-10-27, and the branch is behind main. Please rebase and rerun before merge. I did not run the full suite, clippy or GQT locally.
• Not verified: the 13–14 s to 1 scan improvement on a hosted graph. The PR states it was not measured there, and I did not measure it either.