feat(engine): WriteTxn - validate schema + open each data table once per write

[ragnorc]

WriteTxn

A single mutate/load re-validated the schema contract ~4× and opened each touched data table 4× — redundant per-write I/O that scales with the write's internal structure, not its work. This threads a capture-once WriteTxn { branch, base } so a write validates the schema once and opens each data table at most once. Constant-factor/RTT win (3a already removed the depth-slope term).

Locked by two cost gates (write_cost.rs)

• write_validates_schema_contract_once — 3 read_text / 2 exists (was 12/9)
• keyed_insert_opens_table_at_most_once — data_open_count = 1 (was 4)

How

• Schema-once: capture txn once at entry (the single validation); the 4 sites collapse — entry check removed; open_for_mutation_on_branch + prepare_updates_for_commit source txn.base; commit_all uses fresh_snapshot_for_branch_unchecked (the OCC manifest re-read minus the schema re-validation, also dropping a spurious commit-graph exists probe — same Snapshot).
• Open-collapse 4→1: Add OpenAPI documentation endpoint and schema #1 accumulation open eliminated (node path discarded it; read the pinned entry.table_version); [codex] Fix CLI ergonomics and stream export output #2 staging open kept (the one open); Add support for null literals in query parameters #3 drift-guard reads live HEAD via a cheap latest_version_id() probe off the already-open staged handle (not a fresh open); Support multi-statement mutations (insert + edge in one query) #4 index build reuses the commit_staged handle threaded through CommittedMutation instead of reopening.
• Counter fix (review finding): open_count → data_open_count/internal_open_count classified by URI, so the gate measures data-table opens (not the __manifest/_graph_commits opens the publisher/commit-graph append make) and is actually reachable.
• Positional returns named (OpenedForMutation, CommittedMutation) + a debug_assert pinning the open-skip contract (a None handle is legal only on the non-strict-txn path).

When txn is None (branch merge, schema apply, tests) every threaded path is byte-identical. Strict ops keep live-HEAD + ensure_expected_version; the fork path and the commit-time OCC re-read stay fresh (correctness machinery).

Deferred to step 5

Session-aware write base opens — the one remaining open stays a HEAD open; the warm-session benefit is an object-store (S3) phenomenon invisible on local FS, so it earns its own write_cost_s3.rs gate in step 5, where WriteTxn becomes the non-optional publish carrier (PublishPlan unification).

Verification

Full engine suite + failpoints (59, recovery-sidecar lifecycle) + cargo test --workspace --locked all green. No new concurrency test needed: #2 stays a HEAD open (no pinned/session base introduced), so the publisher CAS + #3 live-HEAD probe fences are unchanged.

Stacked on merged #297 (optimize-vs-write race).

---

Note

Medium Risk
Changes the mutate/load write hot path (schema validation timing, skipped opens, pinned base) while relying on preserved strict-op and OCC guards; edge cardinality now depends on an extra live-HEAD open on a narrow path, with regression coverage but remaining TOCTOU called out for step 4.

Overview
RFC-013 step 3b adds a capture-once WriteTxn { branch, base }: mutate and load call open_write_txn after recovery heal (and drop the duplicate entry ensure_schema_state_valid), then thread Option<&WriteTxn> through staging and publish. Resolve sites reuse txn.base instead of re-running resolved_branch_target / snapshot_for_branch (which re-validated the schema contract). None keeps branch merge and other callers byte-identical.

Open collapse (4→1 on the hot path): open_for_mutation_on_branch returns OpenedForMutation; for non-strict Insert/Merge under a txn on the target branch it skips the accumulation Dataset::open and only pins expected_version from the manifest entry. Staging still does one reopen; commit drift uses latest_version_id() on the staged handle instead of another open; CommittedMutation.committed_handles lets publish-time index build reuse post-commit_staged handles instead of reopen_for_mutation.

OCC stays fresh: commit-time manifest re-read uses fresh_snapshot_for_branch_unchecked (manifest-only, no second schema check / commit-graph probe) when a txn is present. Strict Update/Delete, fork paths, and publisher CAS behavior are unchanged.

Integrity: when the edge open is skipped, non-default @card validation uses edge_cardinality_read_handle (live HEAD), plus a regression test for stale-handle concurrent commits. IO probes add data_open_count / internal_open_count; write_cost.rs gates schema-once reads and data_open_count <= 1 on keyed insert.

Developer handoff docs for RFC-013 and schema-apply recovery flake are added/linked.

^{Reviewed by Cursor Bugbot for commit c9a2a4b. Bugbot is set up for automated code reviews on this repo. Configure here.}

Greptile Summary

This PR implements RFC-013 step 3b: a capture-once WriteTxn { branch, base } that validates the schema contract exactly once per mutate/load call and collapses ~4 data-table opens per write down to 1 (on the non-strict main-branch insert path). All previously-flagged issues (cardinality stale-handle regression, feature-branch schema re-validation) are addressed or explicitly deferred to step 4 with regression tests and handoff documentation.

• Schema-once: open_write_txn captures a pinned base snapshot after the single ensure_schema_state_valid call; open_for_mutation_on_branch, prepare_updates_for_commit, and the OCC re-read in commit_all all consume it via fresh_snapshot_for_branch_unchecked (dropped commit-graph side-load) rather than re-resolving.
• Open-collapse (4→1): collapse Add OpenAPI documentation endpoint and schema #1 skips the accumulation open for non-strict Insert/Merge on the txn's own branch; collapse Add support for null literals in query parameters #3 replaces a fresh open_dataset_head_for_write drift-guard call with latest_version_id() on the already-open staged handle; collapse Support multi-statement mutations (insert + edge in one query) #4 threads post-commit_staged handles via CommittedMutation.committed_handles to reuse them for the index build.
• Cardinality fix + regression test: edge_cardinality_read_handle restores the live-HEAD read for non-default @card validation when collapse Add OpenAPI documentation endpoint and schema #1 skipped the accumulation open; cardinality_rejected_for_stale_handle_after_concurrent_edge_commit locks this deterministically.

Confidence Score: 5/5

The write pipeline change is well-guarded: strict ops (Update/Delete/Overwrite) are untouched by the open-collapse, the OCC re-read in commit_all still uses a fresh manifest snapshot, and the publisher CAS still fences against concurrent advances. The cardinality regression introduced by collapse #1 is correctly repaired and regression-tested.

The schema-once and open-collapse paths are gated conservatively — strict ops, the fork path, and no-txn callers all keep their prior behavior byte-identical. The two locked cost gates make regressions detectable in CI. The one flagged item is a defensive fallback arm documented as unreachable today and carries only forward-looking risk.

The defensive None arm in edge_cardinality_read_handle (exec/mutation.rs) warrants attention if future work makes txn.base not contain the edge table entry — it would open at a pinned manifest version rather than live HEAD, silently reintroducing the staleness the function was added to prevent.

Important Files Changed

Filename	Overview
crates/omnigraph/src/db/omnigraph.rs	Introduces WriteTxn struct and open_write_txn/fresh_snapshot_for_branch_unchecked; wires txn through open_for_mutation_on_branch and commit_updates_on_branch_with_expected. Logic is sound.
crates/omnigraph/src/db/omnigraph/table_ops.rs	Core collapse logic: adds OpenedForMutation struct, implements collapse #1 (non-strict txn early-return for main/active branch), and collapse #4 (post-commit handle reuse in prepare_updates_for_commit). Guards and edge cases (fork path, strict ops) are correctly kept out of the collapse paths.
crates/omnigraph/src/exec/mutation.rs	Threads WriteTxn through execute_named_mutation and all op sub-fns; adds edge_cardinality_read_handle to restore live-HEAD cardinality scan when collapse #1 skipped the accumulation open; gates cardinality validation on !is_default() (safe, matching the inner early-return in validate_edge_cardinality_with_pending). Strict op callers use .expect() for always-Some handles.
crates/omnigraph/src/exec/staging.rs	Refactors commit_all return type to CommittedMutation; replaces the collapse #3 fresh open_dataset_head_for_write drift-guard open with latest_version_id() on the already-open staged handle; carries committed_handles for collapse #4 index-build reuse. OCC correctness (fresh manifest re-read, strict CAS check) is preserved.
crates/omnigraph/src/loader/mod.rs	Replaces db.snapshot_for_branch(branch) with open_write_txn + txn.base; threads txn through per-table opens and commit_all. Overwrite path uses MutationOpKind::SchemaRewrite (strict), so collapse #1 never fires for it — correct. Branch already exists before open_write_txn is called.
crates/omnigraph/src/instrumentation.rs	Adds data_open_count/internal_open_count to QueryIoProbes; record_open classifies by URI last segment; INTERNAL_TABLE_DIRS covers the four system tables. open_dataset_tracked and open_table_dataset both call record_open.
crates/omnigraph/tests/write_cost.rs	Adds two locked cost gates: write_validates_schema_contract_once (3 read_text / 2 exists) and keyed_insert_opens_table_at_most_once (data_open_count <= 1). Gates are tight and independently verifiable.
crates/omnigraph/tests/validators.rs	Adds cardinality_rejected_for_stale_handle_after_concurrent_edge_commit — deterministic regression guard for the #298 cardinality fix: handle B commits a second edge for a @card(0..1) endpoint that handle A already saturated; validates that the live-HEAD reopen rejects the insert.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant C as Client
    participant M as mutate()/load()
    participant WT as open_write_txn
    participant OFM as open_for_mutation_on_branch
    participant S as MutationStaging
    participant CA as commit_all
    participant P as ManifestPublisher

    C->>M: mutate(branch, query, params)
    M->>M: heal_pending_recovery_sidecars()
    M->>WT: open_write_txn(branch)
    WT->>WT: resolved_branch_target → ensure_schema_state_valid (ONCE)
    WT-->>M: "WriteTxn { branch, base }"

    loop per table (Insert/Merge on txn branch)
        M->>OFM: "open_for_mutation_on_branch(branch, table, Insert, Some(&txn))"
        OFM->>OFM: ensure_schema_apply_not_locked
        OFM->>OFM: use txn.base entry (no re-validate)
        Note over OFM: Collapse #1: non-strict + txn → handle=None, expected_version=base.version
        OFM-->>M: "OpenedForMutation { handle: None, expected_version }"
        M->>S: ensure_path(table, full_path, table_branch, expected_version)
    end

    M->>S: stage_merge_insert / stage_append per table
    M->>CA: "commit_all(db, branch, ..., Some(&txn))"
    CA->>CA: acquire per-(table,branch) write queues
    CA->>CA: fresh_snapshot_for_branch_unchecked (OCC re-read, no schema check)
    CA->>CA: "Collapse #3: latest_version_id() drift check (no new Dataset::open)"
    CA->>CA: commit_staged per table → committed_handles map
    CA-->>M: "CommittedMutation { updates, expected_versions, committed_handles }"

    M->>P: "commit_updates_on_branch_with_expected(branch, updates, ..., Some(&txn), committed_handles)"
    P->>P: "prepare_updates_for_commit: Collapse #4 reuses committed_handles for index build"
    P->>P: fresh_snapshot_for_branch_unchecked (OCC re-read)
    P->>P: ManifestBatchPublisher::publish (CAS on expected_versions)
    P-->>C: commit version

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant C as Client
    participant M as mutate()/load()
    participant WT as open_write_txn
    participant OFM as open_for_mutation_on_branch
    participant S as MutationStaging
    participant CA as commit_all
    participant P as ManifestPublisher

    C->>M: mutate(branch, query, params)
    M->>M: heal_pending_recovery_sidecars()
    M->>WT: open_write_txn(branch)
    WT->>WT: resolved_branch_target → ensure_schema_state_valid (ONCE)
    WT-->>M: "WriteTxn { branch, base }"

    loop per table (Insert/Merge on txn branch)
        M->>OFM: "open_for_mutation_on_branch(branch, table, Insert, Some(&txn))"
        OFM->>OFM: ensure_schema_apply_not_locked
        OFM->>OFM: use txn.base entry (no re-validate)
        Note over OFM: Collapse #1: non-strict + txn → handle=None, expected_version=base.version
        OFM-->>M: "OpenedForMutation { handle: None, expected_version }"
        M->>S: ensure_path(table, full_path, table_branch, expected_version)
    end

    M->>S: stage_merge_insert / stage_append per table
    M->>CA: "commit_all(db, branch, ..., Some(&txn))"
    CA->>CA: acquire per-(table,branch) write queues
    CA->>CA: fresh_snapshot_for_branch_unchecked (OCC re-read, no schema check)
    CA->>CA: "Collapse #3: latest_version_id() drift check (no new Dataset::open)"
    CA->>CA: commit_staged per table → committed_handles map
    CA-->>M: "CommittedMutation { updates, expected_versions, committed_handles }"

    M->>P: "commit_updates_on_branch_with_expected(branch, updates, ..., Some(&txn), committed_handles)"
    P->>P: "prepare_updates_for_commit: Collapse #4 reuses committed_handles for index build"
    P->>P: fresh_snapshot_for_branch_unchecked (OCC re-read)
    P->>P: ManifestBatchPublisher::publish (CAS on expected_versions)
    P-->>C: commit version

Loading

Comments Outside Diff (1)

1. crates/omnigraph/src/exec/mutation.rs, line 820-843 (link)

   Schema re-validated on feature-branch path despite captured txn

   The fork-queue-guard check calls self.snapshot_for_branch(Some(active)).await?, which calls resolved_branch_target → ensure_schema_state_valid() — a full schema re-validation. This fires for every feature-branch mutate call (when requested.as_deref() is Some), so feature-branch inserts validate the schema at least twice despite the captured txn. The two cost gates (write_validates_schema_contract_once and keyed_insert_opens_table_at_most_once) only instrument main-branch inserts where requested.as_deref() is None, leaving this path uncovered.

   The guard's only use of this snapshot is reading table_branch to detect whether a fork is needed — a field that can't change between open_write_txn capture and this point without holding the write-queue guard that also serializes our path. Using &txn.base directly here would eliminate the redundant validation.

   

2. crates/omnigraph/src/db/omnigraph.rs, line 115-126 (link)

   _unchecked variant is pub(crate) without the caller precondition enforced

   The doc comment states "Callers with no WriteTxn MUST use the checked variant," but the function is pub(crate), so any code in the engine crate can call it without a WriteTxn in scope. The invariant is upheld by convention only. Consider a ZST witness token, a private submodule, or a stronger name like _without_schema_check to make the safety consequence visible at every call site.

   

_{Reviews (4): Last reviewed commit: "docs(engine): tighten 3b claims to match..." | Re-trigger Greptile}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 47707d1. Configure here.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 47707d166b

ℹ️ 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]

Revalidate edge cardinality against live head

When an edge type has non-default cardinality and another writer commits an edge after open_write_txn captures txn.base but before this insert validates cardinality, the None branch opens the pinned base snapshot instead of the live table. Because edge inserts remain non-strict and commit_all refreshes the expected version to allow rebasing, this write can then commit on top of the concurrent edge without ever counting it, allowing @card max bounds to be violated; keep the cardinality scan on a live head (or revalidate under the write queue) for this path.

Useful? React with 👍 / 👎.