perf(mutate): stage independent tables at the loader's write concurrency

[memmmmike]

Closes #504.

Per the acceptance comment: the fix stays inside the existing mechanism. stage_all now delegates at stage_write_concurrency() (the loader's OMNIGRAPH_LOAD_CONCURRENCY knob, default 8) instead of a pinned 1. The resolver moves to exec::staging with a pure parse half; the loader's private copy is deleted and both call sites share it.

Scope kept, verified against source:

• Publication untouched — everything after staging still funnels through the single manifest CAS; commit_all still acquires sorted table gates.
• Ordering untouched — the stream is buffered (order-preserving), not buffer_unordered.
• Failure semantics untouched — the staging stream drained before surfacing the first error at width 1 too (buffered(1).collect::<Vec<Result<_>>>), so wider staging changes only how much of that pre-existing drain overlaps; residue remains reclaimable and never graph-visible.

Tests: a pure unit test on the parse rules, and a serial-vs-concurrent equivalence test on a multi-table mutation (Person + two Knows edges in one query) asserting affected counts, per-table row counts, the inserted row's values, and exact edge endpoint pairs agree between OMNIGRAPH_LOAD_CONCURRENCY=1 and =4 (#[serial]-guarded per the search.rs precedent). Full writes suite: 43/43.

Ad-hoc local measurement (16 node tables touched per mutation, MinIO behind a 25ms toxiproxy latency, 5 timed runs each after a warm-up): end-to-end mutate mean 5.80s at concurrency 1 vs 4.00s at 8. The remaining gap to the theoretical staging ratio is the untouched validate/publish path, which stays serial by design. Happy to re-run under any harness you prefer, per docs/dev/testing.md's benchmark guidance.

🤖 Generated with Claude Code

https://claude.ai/code/session_01E4S9Xpe9G7pLEv9JaoXUAb

Greptile Summary

The PR makes mutation staging use the loader’s configurable cross-table write concurrency and centralizes the resolver behind a scoped test override.

• Shares OMNIGRAPH_LOAD_CONCURRENCY between loader and mutation staging.
• Adds task-local test control plus serial-versus-concurrent equivalence and write-cost coverage.
• Documents concurrent staging and the unchanged single-manifest publication model.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
crates/omnigraph/src/exec/staging.rs	Centralizes staging-width resolution and delegates independent per-table staging at the configured concurrency.
crates/omnigraph/src/instrumentation.rs	Adds a task-local staging-concurrency override that avoids process-global environment mutation in tests.
crates/omnigraph/src/loader/mod.rs	Replaces the loader’s private concurrency resolver with the shared staging resolver.
crates/omnigraph/tests/write_cost.rs	Adds history-depth cost coverage for concurrent multi-table staging.
crates/omnigraph/tests/writes.rs	Adds end-to-end equivalence coverage comparing scoped serial and concurrent mutation staging.
docs/dev/writes.md	Documents the shared staging knob and the unchanged single-manifest publication path.

_{Reviews (2): Last reviewed commit: "review: scope the staging-width test sea..." | Re-trigger Greptile}

Context used:

• Context used - CLAUDE.md (source)
• Knowledge Base — OmniGraph write path

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3e7e14a184

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Add a history-depth cost gate for concurrent staging

This changes the mutation hot path for performance, but the added test only compares results on two fresh, shallow stores; it neither verifies that table stages actually overlap nor guards the write cost at realistic commit-history depth. A future change that restores width 1 or adds a history-dependent scan would therefore pass. Add or extend a helpers::cost test over a deep-history fixture that asserts the intended bounded staging behavior, as required by the repository's before-every-task testing checklist.

AGENTS.md reference: AGENTS.md:L7-L9

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Document that the load knob now controls mutations

When an operator sets OMNIGRAPH_LOAD_CONCURRENCY to tune bulk loads, this change also applies that value to every constructive mutation, but the only repository documentation for the variable still describes it under “Load modes” and says it applies to the loader's fragment-writing stage. This newly broadened configuration contract can unexpectedly change mutation resource usage and latency; update the operator/developer documentation to state the shared scope, or use a mutation-specific/shared setting with an explicit compatibility story.

AGENTS.md reference: AGENTS.md:L194-L194

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Avoid mutating the environment in the parallel test binary

Under the default parallel libtest harness, #[serial] only excludes other tests carrying the same serial lock; the other unannotated tests in writes.rs can still run while this code calls the Rust 2024 unsafe environment mutation APIs. That violates the APIs' requirement that no other thread concurrently reads or writes the process environment and can also make unrelated mutations observe concurrency 1 or 4. Exercise a test-only concurrency seam directly or isolate this check in a subprocess instead of changing process-global environment state.

Useful? React with 👍 / 👎.

[memmmmike]

Thanks both bots — all three points addressed in 16a1e87.

1. #[serial] doesn't isolate the env mutation (greptile + codex, agreed)

Right, and the reasoning goes further than flakiness: #[serial] only excludes other #[serial] tests, so EnvGuard raced every unannotated test in this binary, and set_var under a live multi-thread runtime violates setenv's thread-safety precondition regardless.

Rather than annotate the rest of the file, the width override now rides the seam this repo already established for exactly this problem — instrumentation::with_traversal_mode, whose doc comment spells out the rationale ("scope-bound … process-safe … removing the need for #[serial]"). Added with_stage_write_concurrency alongside it; stage_write_concurrency() consults the scoped override, then the env var, then the default. EnvGuard and #[serial] are both deleted, and no unsafe remains in the test.

This works because staging is explicitly off-queue (it runs on the caller's task; only the commit is queued), so the task-local reaches stage_all.

One thing I'd have missed without writing it down: a scoped override that silently fails to apply would make the equivalence test run at the default width on both sides and assert nothing. There's now a unit test pinning that the override reaches the resolver, that 0 falls back rather than pinning the width to zero, and that it's gone once the future resolves.

2. Document that the load knob now controls mutations (greptile + codex, agreed)

docs/dev/writes.md described it only inside the LoadMode::Overwrite paragraph. Updated in both relevant places: the stage_all bullet now states the width is shared with the loader and why cross-table staging has no shared state to race, and the Overwrite paragraph notes the same knob tunes ordinary multi-table mutations.

3. History-depth cost gate (codex P1, agreed)

Fair — the equivalence test proves sameness on two shallow stores and nothing about cost. Added multi_table_staging_is_flat_in_history to write_cost.rs, in that file's existing vocabulary (cost_harness / commit_many / assert_flat), asserting a two-table mutation's __manifest and data reads stay flat from depth 10 to depth 100. Measured on local FS: depth10 __manifest=11 / data=21, depth100 __manifest=10 / data=19 — flat within fixture noise, and a history-proportional regression is ~10x at depth 100, so it trips immediately. That's the guard against a future change making each stage re-resolve per-table state at depth: results would stay correct and the equivalence test would stay green while the write got quadratically slower.

On the other half of that P1 — asserting the stages overlap — I did not add a timing or peak-inflight assertion, deliberately. Every version I could write was either timing-based (flaky in CI) or required instrumenting the production staging loop to prove a scheduling property that .buffered(concurrency) already gives structurally. The cost gate plus the width-seam test cover the regression I actually care about; happy to add an overlap probe if you'd rather have it explicit.

Also: the original commit was never cargo fmt'd, and CI on this branch has never run (action_required), so the fmt job would have failed the moment you approved it. Fixed in the same commit — cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings, and the writes / write_cost / lib suites (43 / 13 / 297) are all green locally on 1.97.1.