[ragnorc]

Stacked on #966 (graph diff, branch review and merge dry run). Review that first; this PR's own change is the last commit.

This PR takes the three cheap fixes from a measured investigation of graph diff cost. The deeper bottlenecks it found are filed as issues (linked below).

Changes

1. A diff side that cannot be a commit id is never looked up as one. Every graph commit id is minted canonical, either a ULID or an hb1 address (is_valid_graph_commit_id). Classification now skips the commit lookup for any other spelling. Before this, a side like main took the lookup's miss path, which checks out and scans every live branch's __manifest.

Measured under DST: a one-change diff between two branch names cost about 22 requests per live branch. With 32 idle branches that was 795 requests, of which 23 read the table (about 7 s under the S3-like model). The new cost gate shows the same diff reading 5 __manifest objects with 1 idle branch and with 9. Without the gate it read 15 and 31.

A malformed hb1. spelling is now a typed ReadTargetNotFound instead of an internal error from the id parser.

2. One resolution serves authorization and the read. /diff used to classify each side once to authorize it (diff_read_target_branch), and the engine classified it again to read. That doubled the cost, and the two resolutions could disagree if a branch was created or deleted in between.

Omnigraph::prepare_diff_page now resolves the request once and reports the branch each two-point side reads (PreparedDiffPage::side_branches). The server authorizes those branches, then run_diff_page reads. diff_page is the one-call form for embedded callers. diff_read_target_branch is removed. A continuation still classifies each spelled side for authorization and reads the commits its token pins, as before.

3. The CLI asks for full pages when it follows every page. Every change page restarts its comparison: it re-plans, re-resolves its endpoints and re-sorts the remaining keys. Measured on a release build, a 100k-change diff over 1M rows took 19.5 s at the 1,000-change default and 7.3 s at 8,192.

commit changes, changes poll, diff and branch diff now ask for 8,192 changes per page when they paginate themselves, unless --limit is given. A single --page-token fetch keeps the 1,000 default, and so do the HTTP routes. The --limit help text no longer claims the server clamps it: an over-ceiling limit is refused.

Tests

• New changes_cost::graph_diff_side_resolution_is_flat_in_live_branches: a branch-name diff with 1 and 9 idle branches, __manifest reads gated flat. It fails with the spelling gate removed (15 → 31 reads) and passes with it (5 → 5).
• changes::graph_diff_refuses_an_ambiguous_or_unknown_side now also covers:
  • a malformed hb1. side, which is a typed not-found;
  • prepare_diff_page reporting ("main", "main") and (<commit>, "feature") as the side branches.
• Rerun locally with RUST_MIN_STACK=16777216, all passing:
  • engine changes:: and changes_cost:: (65), and forbidden_apis (35; the new public methods are registered as read-only);
  • server auth_policy::, data_routes:: and openapi:: (224): the 404 parity for forbidden sides is unchanged;
  • CLI parity_matrix:: and cli_data:: (119);
  • the served GQT conformance for the four graph_diff_* cases;
  • workspace clippy, cargo fmt --check, check-docs.py and typos.

Not in this PR (filed as issues)

• performance: graph diff and branch review read both full tables unless the endpoints are one transaction apart #969: a diff reads both full tables of every changed type unless its endpoints are one transaction apart.
• performance: every change page restarts its comparison (re-plan, re-resolve endpoints, re-sort keys) #970: every change page restarts its comparison (re-plan, re-resolve endpoints, re-sort keys). This PR's CLI page size is the interim mitigation.
• performance: ordered-cursor hydration re-reads data pages and file metadata for every chunk #971: ordered-cursor hydration re-reads data pages and file metadata for every chunk. This is 76–90% of a merge dry run's requests, and real merges pay it too.
• performance: a commit lookup that misses the current branch checks out every live branch's __manifest #972: a commit lookup that misses the current branch still checks out every live branch's __manifest (about 4 requests per branch for commit-id sides).

[cursor]

Review of 81ac9d1a — recommendation: request changes (1 blocking finding)

This review is posted as a comment because this account cannot set a formal review state. Please read it as "changes requested".

What this PR does, and the impact

• Why: a graph diff paid for lookups it did not need. A branch-name side took the commit lookup's miss path, which checks out every live branch's __manifest. /diff also resolved each side twice, once for authorization and once for the read. The CLI followed pages of 1,000 changes, and each page restarts its comparison.
• What and how: the commit lookup is skipped for any spelling is_valid_graph_commit_id refuses. prepare_diff_page / run_diff_page give /diff one resolution for authorization and for the read. The CLI asks for 8,192 changes per page when it follows every page.
• Impact: a diff between two branch names reads a flat number of __manifest objects (5 with 1 idle branch and with 9). That part is correct and well guarded. But the new order in /diff changes what a forbidden side reveals. Details below.

Blocking

1. /diff is an existence oracle for a forbidden from side (handlers.rs line 4134, engine side at omnigraph.rs line 2951). Both sides are now resolved before either is authorized. from=feature&to=no-such-branch names no-such-branch when feature exists, and names from when it does not. On main the route answered 'feature' names no branch and no commit in both cases. The same order opens a forbidden commit's snapshot before the policy check. The fix and a regression test are in the inline comment.

Should fix in the same change

2. The BranchNotFound side guess names the wrong side (handlers.rs line 4129). For a commit on a deleted branch, from=<commit>&to=main answers 'main' names no branch and no commit, which is false. Map the error where the side is known. Per-side authorization in the engine removes this arm.

Optional

3. Docs wording and changelog: each page restarts its comparison; it does not resume it. The changelog also omits diff and branch diff.
4. Merge note for #990: fix(policy)!: refuse branch names that are not canonical #990 takes sides as written. The new trim() copies then follow a different rule from the classifier.

Checked and correct

• Spelling gate premise. is_valid_graph_commit_id is the one rule for public commit ids. Storage upgrade refuses a non-canonical id (omnigraph-catalog/src/history.rs, canonical_ulid → legacy_error). Normal open serves only stamps 14–16. So no stored commit has a spelling the gate skips. Lance branch names allow every character of a ULID or an hb1 address, and such a branch name still gets both lookups. So AmbiguousReadTarget still works. The malformed hb1. case now gives a typed 404, not a 500. This is an improvement.
• CLI page size. Only the follow-every-page loops change; --page-token passes limit as given. 8,192 is also the ceiling in v0.12.0 and v0.13.0, so a new CLI against an older server is safe. The 4 MiB default byte budget still bounds each page.
• Cost gate. It is a real guard. With the spelling gate disabled locally, it fails at 15 → 31 __manifest reads. With the gate, it passes at 5 → 5.

Tradeoffs and liability

• Spelling gate: low liability. One function decides what a commit id can look like, and minting, upgrade and this lookup all use it. A future id format must be added there first, which is already true today.
• Two-phase public API: medium liability as written. prepare_diff_page + run_diff_page move the rule "authorize each side before you read or resolve more" out of the engine. Each transport must now apply it: the HTTP route, the GQ branch diff statement, and the MCP read tools in feat(server): schema and ad-hoc read tools on /mcp, metadata at the advertised path #993/feat(server): /mcp for every credential source, read parity, resources and opt-in writes #994. After five more transports, five call sites must get the order right, and finding 1 shows that this is easy to get wrong. A read predicate passed into the engine keeps the rule in one place, and keeps this PR's cost win.
• CLI ceiling coupling: low. The CLI sends the server's compile-time ceiling. If a later server lowers it, followed pages fail with 413 rather than degrade. This is acceptable today.
• Not in scope, still open: a commit-id side still pays the per-branch fanout on a miss (performance: a commit lookup that misses the current branch checks out every live branch's __manifest #972). A continuation reclassifies both sides on every page, so it pays that fanout once per page. The larger CLI pages reduce that count. No Lance API changes here; the fanout is OmniGraph's record_in_live_branches, not Lance branch behavior.

Validation

Clean detached worktree at 81ac9d1a464b0fbfb4f339fe1c8c017f70b88de6, merge base ff80af2fbca81ceb396faae8f1d58931442e6c01, RUST_MIN_STACK=16777216.

Command	Result
engine --test integration changes:: changes_cost::	64 passed, 0 failed, 1 ignored (an existing repair ignore)
changes_cost::graph_diff_side_resolution_is_flat_in_live_branches --nocapture	manifest_reads 5 → 5 (1 → 9 idle branches)
same test, with the spelling gate disabled locally (reverted)	fails: 15 → 31
server --test integration auth_policy:: data_routes:: openapi::	224 passed, 0 failed
temporary /diff probe test (not pushed), run at head and at merge base	results in the inline comments
cargo fmt --all --check	pass
check-docs.py (pinned venv), typos 1.50.1, check-agents-md.sh	pass (269 Markdown files)
CI on 81ac9d1a	all checks green, including Test Workspace, Lint (clippy), GQ Logic Tests and CI Gate

Limitations: I did not run CLI parity_matrix:: / cli_data::, served GQT, forbidden_apis or workspace clippy locally; I relied on CI for those. I did not run the S3 or Azure suites. I did not measure the 19.5 s → 7.3 s CLI timing.