Merge pull request #481 from ModernRelay/cursor/merge-complexity-investigation-a080

Author: ragnorc

AGENTS.md

@@ -104,6 +104,8 @@ Full diagram and concurrency model: [docs/dev/architecture.md](docs/dev/architec
 | Transactions and atomicity (per-query atomic; branches as multi-query transactions) | [docs/user/branching/transactions.md](docs/user/branching/transactions.md) |
 | Direct-publish write path (staging, D2, recovery sidecars; the former Run state machine) | [docs/dev/writes.md](docs/dev/writes.md) |
 | Three-way merge and conflict kinds | [docs/dev/merge.md](docs/dev/merge.md) |
+| Branch-merge complexity / timeout diagnosis (OmniGraph + Lance) | [docs/dev/merge-complexity.md](docs/dev/merge-complexity.md) |
+| Merge latency L1–L3 implementation plan | [docs/dev/merge-l1-l3-plan.md](docs/dev/merge-l1-l3-plan.md) |
 | Diff / change feed (`diff_between`, `diff_commits`) | [docs/user/branching/changes.md](docs/user/branching/changes.md) |
 | Query execution, mutation execution, bulk loader, `load` vs `ingest` | [docs/dev/execution.md](docs/dev/execution.md) |
 | `optimize` (compaction) and `cleanup` (version GC) | [docs/user/operations/maintenance.md](docs/user/operations/maintenance.md) |
@@ -273,7 +275,7 @@ omnigraph policy explain --cluster ./company-brain --graph knowledge --actor act
 | Repair uncovered drift | — | `omnigraph repair` explicitly classifies uncovered table `HEAD > manifest` drift: verified maintenance drift (`ReserveFragments`/`Rewrite`) can be published with `--confirm`; suspicious or unverifiable drift requires `--force --confirm`. Sidecar-covered crash residuals still recover automatically on open. |
 | Cleanup (`cleanup_old_versions`) | ✅ | `omnigraph cleanup` derives requested `--keep` / `--older-than` cutoffs from each table's available versions; Lance refs plus OmniGraph's live-lazy-branch and recovery floors may retain additional versions. It fails closed on unopenable pins, recovery intent, or uncovered main-table HEAD drift |
 | BTREE / inverted (FTS) / vector indexes | ✅ | `@index`/`@key` declares intent; physical indexes are derived state and never fail a logical operation. One type-dispatched chokepoint builds BTREE, FTS, or vector indexes idempotently and lazily across branches. Schema apply and mutation/load publish only logical effects. `ensure_indices` first runs the roll-forward-only recovery barrier, then materializes declared-but-missing artifacts through one staged mixed CreateIndex transaction under ordinary recovery-v9 authority; untrainable vector columns remain pending. |
-| Strict insert / upsert ingestion | ✅ transaction conflict filters + uncommitted fragment staging | Internal schema v6 owns the explicit logical mode. Strict insert and upsert use the sealed exact-`id`, forced-v2 MergeInsert adapter; Mutation/Load remains one transaction per table, capped before arm at 8,192 rows / 32 MiB. BranchMerge's proven all-new route accepts only a complete certificate chain plus final source/target native-incarnation checks; missing or malformed proof falls back to the general ordered diff. Raw Lance writers are outside the supported graph-writer topology. |
+| Strict insert / upsert ingestion | ✅ transaction conflict filters + uncommitted fragment staging | Internal schema v6 owns the explicit logical mode. Strict insert exact-probes its pinned parent, then stages a join-free exact-`id` filtered insertion-only Update; upsert uses the sealed exact-`id`, forced-v2 MergeInsert adapter. Mutation/Load remains one transaction per table, capped before arm at 8,192 rows / 32 MiB. BranchMerge's proven all-new route accepts only a complete certificate chain plus final source/target native-incarnation checks; ordered adopt fallback routes new rows through join-free StrictInsert and changed rows through a sealed update-only (`UpdateAll` + `DoNothing`) arm that fails closed unless every classified id updates. Raw Lance writers are outside the supported graph-writer topology. |
 | Bounded graph-batch ingestion | — | Raw graph-level NDJSON is parsed at a strict logical envelope and committed through the shared ordinary Load transaction. One request produces one graph commit and is acknowledged only after manifest visibility. The public shape names logical node/edge declarations, never physical datasets. There is no MemWAL, token ledger, lifecycle, hidden stream metadata, or stream-specific recovery path. |
 | Vector search | ✅ | `nearest()` query op; embedding pipeline (Gemini / OpenAI clients); `@embed` in schema |
 | Full-text search | ✅ | `search/fuzzy/match_text/bm25` query ops |

crates/omnigraph/src/db/omnigraph.rs

@@ -2991,14 +2991,6 @@ impl Omnigraph {
         .await
     }
 
-    pub(crate) async fn build_indices_on_dataset(
-        &self,
-        table_key: &str,
-        ds: &mut SnapshotHandle,
-    ) -> Result<Vec<PendingIndex>> {
-        table_ops::build_indices_on_dataset(self, table_key, ds).await
-    }
-
     // Used only by in-tree tests (`#[cfg(test)]`); the runtime path now
     // uses `commit_updates_on_branch_with_expected` exclusively.
     #[cfg(test)]

crates/omnigraph/src/exec/merge.rs

@@ -2464,33 +2464,19 @@ async fn publish_rewritten_merge_table(
         )));
     }
 
-    // Failpoint: crash after the Phase 2 delete commit, before the index build.
+    // Failpoint: crash after the Phase 2 delete commit, before confirmation.
     // Models a partial Phase B on the three-way path — constructive rows +
     // deletes are on Lance HEAD but the achieved-version intent has not been
-    // recorded, so recovery must roll BACK (the index is reconciler-owned derived
-    // state, but the merge itself never reached its commit boundary). See
+    // recorded, so recovery must roll BACK. See
     // tests/failpoints.rs::branch_merge_rewrite_partial_after_delete_rolls_back.
     crate::failpoints::maybe_fail(
-        crate::failpoints::names::BRANCH_MERGE_REWRITE_AFTER_DELETE_PRE_INDEX,
+        crate::failpoints::names::BRANCH_MERGE_REWRITE_AFTER_DELETE_PRE_CONFIRM,
     )?;
 
-    // Phase 3: rebuild indices.
-    //
-    // `build_indices_on_dataset` stages every missing BTREE/FTS/vector artifact
-    // into one table-level `CreateIndex` tail transaction. This rebuildable
-    // derived-state tail is not part of the merge's logical pre-minted data
-    // chain; Armed v9 recovery accepts it only after that complete exact chain
-    // and discards it with a rollback.
-    let row_count = target_db
-        .storage()
-        .table_state(&full_path, &current_ds)
-        .await?
-        .row_count;
-    if row_count > 0 {
-        target_db
-            .build_indices_on_dataset(table_key, &mut current_ds)
-            .await?;
-    }
+    // Index coverage is reconciler-owned derived state. As on the adopt path,
+    // publish the logical merge without waiting for BTREE / FTS / vector work;
+    // `ensure_indices` / `optimize` converges coverage later while reads remain
+    // correct over uncovered fragments.
     let final_state = target_db
         .storage()
         .table_state(&full_path, &current_ds)
@@ -2825,19 +2811,19 @@ async fn publish_adopted_delta(
         crate::failpoints::names::BRANCH_MERGE_ADOPT_AFTER_APPEND_PRE_UPSERT,
     )?;
 
-    // Phase 1b: upsert the CHANGED rows. The fenced merge join is
-    // bounded to the genuinely-changed set, not the whole delta. It runs against
-    // the committed view that already includes the inserts; the changed ids are
-    // disjoint from the inserted ids (each id is classified into exactly one of
-    // new / changed / deleted / unchanged in the single ordered walk), so the
-    // join never collides with an appended row. Every logical data step uses
-    // the next identity in the exact transaction chain armed before Phase B.
+    // Phase 1b: update the CHANGED rows. Classification proved these ids present
+    // in the target-equals-base image; the sealed update-only adapter forbids
+    // insertion and fails closed if final staging cannot update every id. The
+    // changed ids are disjoint from the inserted ids (each id is classified into
+    // exactly one of new / changed / deleted / unchanged in the single ordered
+    // walk). Every logical data step uses the next identity in the exact
+    // transaction chain armed before Phase B.
     if let Some(upsert_table) = &delta.upserts {
         current_ds = commit_staged_keyed_chunks(
             target_db,
             table_key,
             upsert_table,
-            KeyedWriteSemantics::Upsert,
+            KeyedWriteSemantics::KnownPresentUpdate,
             current_ds,
             planned_transactions,
             &mut planned_index,

crates/omnigraph/src/failpoints.rs

@@ -83,8 +83,8 @@ pub mod names {
     /// Every merge table effect is complete, but the sidecar is still in its
     /// pre-confirmation shape.
     pub const BRANCH_MERGE_POST_EFFECTS_PRE_CONFIRM: &str = "branch_merge.post_effects_pre_confirm";
-    pub const BRANCH_MERGE_REWRITE_AFTER_DELETE_PRE_INDEX: &str =
-        "branch_merge.rewrite_after_delete_pre_index";
+    pub const BRANCH_MERGE_REWRITE_AFTER_DELETE_PRE_CONFIRM: &str =
+        "branch_merge.rewrite_after_delete_pre_confirm";
     pub const BRANCH_MERGE_REWRITE_AFTER_MERGE_PRE_DELETE: &str =
         "branch_merge.rewrite_after_merge_pre_delete";
     pub const CLASSIFY_FRESH_READ: &str = "classify.fresh_read";

crates/omnigraph/src/instrumentation.rs

@@ -239,6 +239,10 @@ pub struct MergeWriteProbes {
     pub stage_append_rows: Arc<AtomicU64>,
     pub stage_merge_insert_calls: Arc<AtomicU64>,
     pub stage_merge_insert_rows: Arc<AtomicU64>,
+    /// Update-only keyed stages whose ids were proven present by merge
+    /// classification. Kept separate from insertion-capable Upsert.
+    pub stage_known_present_update_calls: Arc<AtomicU64>,
+    pub stage_known_present_update_rows: Arc<AtomicU64>,
     /// Strict-insert transactions that write new fragments directly and carry
     /// Lance's inserted-row key filter without running a target merge join.
     pub stage_fenced_insert_calls: Arc<AtomicU64>,
@@ -295,6 +299,13 @@ impl MergeWriteProbes {
     pub fn stage_merge_insert_rows(&self) -> u64 {
         self.stage_merge_insert_rows.load(Ordering::Relaxed)
     }
+    pub fn stage_known_present_update_calls(&self) -> u64 {
+        self.stage_known_present_update_calls
+            .load(Ordering::Relaxed)
+    }
+    pub fn stage_known_present_update_rows(&self) -> u64 {
+        self.stage_known_present_update_rows.load(Ordering::Relaxed)
+    }
     pub fn stage_fenced_insert_calls(&self) -> u64 {
         self.stage_fenced_insert_calls.load(Ordering::Relaxed)
     }
@@ -420,6 +431,17 @@ pub(crate) fn record_stage_merge_insert(rows: u64) {
     });
 }
 
+/// Record one update-only keyed stage whose ids were proven present by merge
+/// classification. No-op when no test or benchmark probe is installed.
+pub(crate) fn record_stage_known_present_update(rows: u64) {
+    let _ = MERGE_WRITE_PROBES.try_with(|p| {
+        p.stage_known_present_update_calls
+            .fetch_add(1, Ordering::Relaxed);
+        p.stage_known_present_update_rows
+            .fetch_add(rows, Ordering::Relaxed);
+    });
+}
+
 /// Record one join-free, filter-bearing strict insert of `rows` rows against
 /// the active probes. This is distinct from `stage_merge_insert`: both commit
 /// a fenced Lance `Operation::Update`, but only the latter runs a target join.

crates/omnigraph/src/storage_layer.rs

@@ -143,6 +143,10 @@ pub enum KeyedWriteSemantics {
     StrictInsert,
     /// Insert new ids and replace the full row for ids already present.
     Upsert,
+    /// Replace full rows only for ids already proven present by a coherent
+    /// merge classification. Missing ids are an internal read-set change,
+    /// never inserts.
+    KnownPresentUpdate,
 }
 
 /// One exact chunk admitted to the join-free strict-insert adapter by the
@@ -607,9 +611,11 @@ pub trait TableStorage: sealed::Sealed + Send + Sync + Debug {
 
     /// Stage one RFC-023 fenced keyed write from an in-memory batch.
     ///
-    /// This production adapter accepts only the graph `id` key, checks
-    /// that the target dataset declares exactly that unenforced primary key,
-    /// and forces Lance's filter-bearing non-index merge route.
+    /// This production adapter accepts only the graph `id` key and checks that
+    /// the target dataset declares exactly that unenforced primary key.
+    /// Insertion-capable Upsert forces Lance's filter-bearing non-index route;
+    /// known-present update-only staging may use the indexed route because
+    /// `DoNothing` makes insertion structurally unreachable.
     async fn stage_keyed_write(
         &self,
         snapshot: SnapshotHandle,