Make branch delete's dependency check manifest-only per surviving branch

crates/omnigraph/src/db/omnigraph.rs

@@ -2366,12 +2366,23 @@ impl Omnigraph {
             )));
         }
 
+        // Dependency detection reads ONLY each surviving branch's manifest
+        // `table_branch` entries, so take the manifest-only snapshot. A full
+        // `snapshot_of` resolve would additionally load the commit-lineage
+        // projection and re-read + re-validate the schema contract PER BRANCH —
+        // O(branches x history) I/O that made deletion time out on large
+        // graphs — and its per-foreign-branch schema validation could wedge
+        // deletion of an unrelated branch behind another branch's schema
+        // drift. This operation's own schema was already validated under the
+        // schema gate above.
         for other_branch in branches
             .iter()
             .filter(|candidate| candidate.as_str() != branch)
         {
             let snapshot = self
-                .snapshot_of(ReadTarget::branch(other_branch.as_str()))
+                .fresh_snapshot_for_branch_unchecked(
+                    Self::normalize_branch_name(other_branch)?.as_deref(),
+                )
                 .await?;
             if snapshot
                 .entries()

crates/omnigraph/tests/branch_control_cost.rs

@@ -0,0 +1,109 @@
+//! Cost-budget tests for native branch CONTROL operations (create/delete) on the
+//! shared `helpers::cost` harness — the branch-op sibling of `write_cost.rs`
+//! (mutations) and `merge_cost.rs` (merge).
+//!
+//! The gated term: `branch_delete` must prove no surviving branch still
+//! references the deleted branch's per-table forks. That dependency check reads
+//! ONLY each foreign branch's manifest `table_branch` entries, so its per-branch
+//! cost budget is ONE manifest-only snapshot read (`__manifest` open + state
+//! scan). The pre-fix implementation instead performed a full cold target
+//! resolve per foreign branch — manifest state scan PLUS commit-lineage scan
+//! plus a schema-contract re-read/re-validate — doubling the `__manifest` read
+//! slope and making deletion O(branches × history) on un-compacted graphs.
+//!
+//! Like the other cost tests, the body runs on a 64 MiB-stack thread: the
+//! debug-build engine futures plus the `cost_harness`/`measure` task-local
+//! layers overflow the default 2 MiB test stack.
+#![recursion_limit = "512"]
+
+mod helpers;
+
+use std::future::Future;
+
+use helpers::cost::{IoCounts, cost_harness, local_graph, measure};
+
+/// Run an async test body on a thread with a large stack (see module docs).
+fn on_big_stack<F>(body: impl FnOnce() -> F + Send + 'static)
+where
+    F: Future<Output = ()>,
+{
+    std::thread::Builder::new()
+        .stack_size(64 * 1024 * 1024)
+        .spawn(move || {
+            tokio::runtime::Builder::new_current_thread()
+                .enable_all()
+                .build()
+                .unwrap()
+                .block_on(body());
+        })
+        .unwrap()
+        .join()
+        .unwrap();
+}
+
+/// Per-foreign-branch budget for the delete dependency check: ONE manifest-only
+/// snapshot (one `__manifest` open + state scan) per surviving branch. On this
+/// fixture that is exactly 1 read iop and 1 internal open per branch; the full
+/// cold resolve (state scan + lineage scan per branch) measures exactly 2 of
+/// each, so a regression re-trips this gate. `SLOPE_SLACK` absorbs incidental
+/// constant-ish noise without admitting a second per-branch open.
+const DELETE_PER_BRANCH_BUDGET: u64 = 1;
+const SLOPE_SLACK: u64 = 2;
+
+/// `branch_delete`'s dependency check visits every surviving branch, so its
+/// `__manifest` reads necessarily scale with branch count — this gate bounds the
+/// SLOPE (reads per surviving branch), not the total. Sibling branches (created
+/// straight off main, never written) keep every other delete precondition
+/// identical across the sweep: no path-prefix children, no lineage descendants,
+/// and constant `__manifest` content (native branch create/delete emits no
+/// lineage rows), so the fixed per-delete overhead cancels in the delta and the
+/// slope isolates the per-branch dependency-check cost.
+#[test]
+fn branch_delete_manifest_reads_bounded_per_surviving_branch() {
+    on_big_stack(|| {
+        cost_harness(async {
+            let dir = tempfile::tempdir().unwrap();
+            let db = local_graph(&dir).await;
+
+            let mut curve: Vec<(u64, IoCounts)> = Vec::new();
+            let mut created = 0u64;
+            for n in [4u64, 12] {
+                while created < n {
+                    db.branch_create(&format!("sib_{created:02}"))
+                        .await
+                        .unwrap();
+                    created += 1;
+                }
+                let victim = format!("victim_{n}");
+                db.branch_create(&victim).await.unwrap();
+                let (res, io) = measure(db.branch_delete(&victim)).await;
+                res.unwrap();
+                eprintln!(
+                    "surviving branches={} (+main): DELETE manifest_reads={} data_reads={} \
+                     internal_open_count={}",
+                    n, io.manifest_reads, io.data_reads, io.internal_open_count
+                );
+                curve.push((n, io));
+            }
+
+            let (n_lo, n_hi) = (curve[0].0, curve[1].0);
+            let added_branches = n_hi - n_lo;
+            let budget = added_branches * DELETE_PER_BRANCH_BUDGET + SLOPE_SLACK;
+            let read_delta = curve[1]
+                .1
+                .manifest_reads
+                .saturating_sub(curve[0].1.manifest_reads);
+            let open_delta = curve[1]
+                .1
+                .internal_open_count
+                .saturating_sub(curve[0].1.internal_open_count);
+            assert!(
+                read_delta <= budget && open_delta <= budget,
+                "branch_delete __manifest cost grew {read_delta} reads / {open_delta} opens \
+                 across {added_branches} added surviving branches (budget {budget}) — the \
+                 dependency check must read one manifest-only snapshot per surviving branch, \
+                 not a full cold resolve (state + lineage scans + schema contract) per branch",
+            );
+        })
+    });
+}

docs/dev/testing.md

@@ -51,6 +51,7 @@ The engine's `tests/` is the principal coverage surface; most graph-shaped behav
 | `lance_version_columns.rs` | Per-row `_row_last_updated_at_version` behavior |
 | `validators.rs` | Schema constraint enforcement (enum, range, unique, cardinality) across JSONL load, mutation insert/update. ALL THREE write surfaces — mutation, bulk load, AND merge — route through the unified `crate::validate` evaluator (Δ-scoped, index-backed, reusing these leaf checks). Cross-version-uniqueness closure: `cross_version_unique_rejected_on_mutation_insert` + `reinsert_existing_key_is_upsert_not_unique_violation` (mutation path); `cross_version_unique_rejected_on_append_load` + `merge_load_reupsert_existing_key_is_not_unique_violation` (load path). Per-table `Overwrite`: `overwrite_load_validates_ri_against_new_image` (an edges-only overwrite still resolves RI against retained committed nodes) + `append_load_rejects_orphan_edge`. The evaluator's own unit tests live in `src/validate.rs` (`#[cfg(test)]`); its merge-conflict equivalence is pinned by `merge_truth_table.rs` (OrphanEdge) + `branching.rs` (Unique/Cardinality merge tests). Intra-batch duplicate-`@key` rejection on every load mode is pinned by `consistency.rs::loader_rejects_intra_batch_duplicate_keys`; the mutation-coalesce counterpart (insert+update / chained updates of one id are NOT a self-collision) by `writes.rs`. Non-String `@unique` columns probe committed state with a TYPED literal (not a stringified key): `cross_version_unique_rejected_on_date_column` + `noncolliding_write_to_date_unique_column_succeeds` (a `Date @unique` collision is a proper `@unique` violation, and a distinct value does not raise a Date32-vs-Utf8 coercion error). Cardinality is keyed by edge id, last-wins (matching commit's `dedupe_merge_batches_by_id`): `merge_load_edge_src_move_rechecks_vacated_src_cardinality` (a Merge-load moving an edge recounts the vacated src for `@card` min) + `merge_load_duplicate_edge_id_counts_once_per_card` (a dup edge id under two srcs in one batch counts once, no spurious max violation). Direct deletes capture the ids they remove (from the delete op's own scan) into the change-set's `deleted_ids`, so a delete emptying a src is validated: `mutation_delete_edge_below_card_min_rejected` (a `delete Edge` dropping a src below `@card` min is rejected, not silently committed). |
 | `merge_cost.rs` | Cost-budget tests for branch MERGE on the shared `helpers::cost` harness: `merge_validation_is_delta_scoped` (a 1-row-delta merge opens ≤3 data tables — Δ-scoped, not the whole catalog; was ~6 pre-#5) and `merge_manifest_cost_grows_with_history` (the cross-branch `__manifest` open amplification still grows with commit depth — a separate, not-yet-addressed term — while validation `data_open_count` stays flat) |
+| `branch_control_cost.rs` | Cost-budget tests for native branch CONTROL ops on the shared `helpers::cost` harness: `branch_delete_manifest_reads_bounded_per_surviving_branch` gates the SLOPE of `branch_delete`'s `__manifest` reads per surviving branch — the delete dependency check reads one manifest-only snapshot per foreign branch, never a full cold resolve (state + lineage scans + schema-contract re-read) per branch |
 | `policy_engine_chassis.rs` | Engine-layer Cedar enforcement (MR-722): allow + deny through every `_as` writer via the SDK directly — no HTTP — proving embedded and CLI callers hit the same gate as the server, with action × scope shapes matching `authorize_request` |
 | `maintenance.rs` | `ensure_indices`, `optimize` (compaction), `repair` (explicit uncovered-drift publish), and `cleanup` (version GC): empty/idempotent/no-op edges, policy validation, head preservation. EnsureIndices refuses uncovered drift before arming its identity-bearing v9 envelope and keeps untrainable Vector work pending. Cleanup pins exact keep-count behavior, lazy-branch retention, graph-wide fail-closed ordering, and refusal of uncovered main HEAD drift before GC. Optimize's bounded payload inside the v9 envelope publishes multiple productive data tables through one graph commit, emits no lineage/sidecar at steady state, skips uncovered drift, refuses pending recovery, and compacts blob-v2 tables. Repair previews/heals verified maintenance drift and requires `--force` for semantic drift |
 | `failpoints.rs` | Failure-injection coverage (gated on `failpoints` feature). RFC-022 includes deterministic post-stage/pre-effect races for mutation/load uniqueness and strict disjoint-head changes, plus the cross-handle post-effect `RecoveryRequired` → read-write-open rollback cell. Branch merge adds the captured-source advance cell; post-confirm target-winner compensation; mixed physical + pointer-only delta recovery with fixed commit id/actor/parents; both sidecar-before-first-ref and ambiguous-ref-create recovery; and an 8,193-delete between-chunk crash proving an `Armed` exact-transaction prefix is rolled back before the successful retry. Identity-bearing v9 SchemaApply is pinned by `schema_apply_phase_b_failure_recovered_on_next_open` (exact confirmed roll-forward with fixed commit id + initiating actor), `schema_apply_partial_table_effect_rolls_back_exactly` (Armed proper-prefix compensation), `schema_apply_recovery_reclaims_owned_add_type_target_and_retry_succeeds` (strict owned first-touch cleanup), `schema_apply_first_touch_foreign_winner_is_preserved_not_adopted` (foreign unregistered winner preservation), `schema_apply_post_effect_disjoint_winner_is_preserved` (winner-preserving compensation), `schema_apply_post_effect_same_table_winner_fails_closed` (buried-effect refusal), `schema_apply_recovers_partial_schema_promotion_after_commit_crash` (read-only refusal for both valid and corrupt intents in the torn manifest/schema window, followed by fixed-outcome completion of a partial source/IR/state promotion), and `schema_apply_live_query_waits_for_coherent_schema_publication` (same-handle publication wait plus pre-apply-handle query/export/whole-graph-index capture from the operation-local accepted catalog). Metadata-only before/after-staging and rollback-retry cells keep the empty-effect v9 boundary pinned. EnsureIndices v9 recovery retains both boundaries in `recovery_rolls_forward_ensure_indices_on_feature_branch`: the first residual rolls forward on the next read-write open, and a second roll-forward-eligible `EffectsConfirmed` residual under an unchanged captured token is completed by a same-handle retry before new planning. `ensure_indices_complete_armed_effects_roll_back` keeps the authority-clean complete-effect Armed rollback rule isolated, while `ensure_indices_entry_barrier_refuses_partial_armed_before_staging` leaves one of two table effects pending and proves the original `RecoveryRequired` wins before the remaining index can reach the post-stage failpoint. Its remaining cells are `ensure_indices_stage_btree_failure_leaves_existing_tables_writable` (after a clean entry barrier, expensive mixed-index staging remains outside the final authority/gates), `ensure_indices_first_touch_crash_before_ref_recovers_cleanly` (sidecar-before-ref no-effect recovery), `ensure_indices_mixed_first_touch_rollback_does_not_delete_moved_ref` (owned-effect rollback and sibling first-touch cleanup), and the no-work/no-sidecar failpoint cell; the recovery module separately pins existing + first-touch payload round-trip and identity-less-input refusal. Optimize's graph-wide identity-bearing v9 envelope is pinned by `optimize_phase_b_failure_recovered_on_next_open` (two-table roll-forward), `optimize_multi_table_partial_effect_rolls_back_under_one_v2_sidecar` (one shared sidecar, no partial visibility, compensation), `optimize_post_manifest_failure_finalizes_multi_table_v2_sidecar` (lost publish acknowledgement), and `optimize_excludes_pending_only_vector_table_from_v2_sidecar` (pending status cannot poison sibling recovery), plus its late-sidecar/main-gate/retry cells. Native controls are pinned by `native_branch_controls_reclassify_lost_acknowledgements` (matching create and absent-ref delete, with no version/lineage movement); `armed_first_touch_recovery_accepts_missing_target_ref` additionally forges and reclaims the clone-only/no-`BranchContents` table state. Legacy path overlap has both sides pinned: `armed_first_touch_recovery_defers_legacy_path_overlap_until_leaf_delete` permits open only for a proven no-effect intent, while `partial_first_touch_recovery_fails_closed_on_legacy_path_overlap` leaves one exact multi-table effect and verifies open fails closed until offline leaf cleanup lets rollback converge. Other control/recovery race cells include `first_touch_post_create_open_error_keeps_recovery_ownership`, `branch_delete_orphans_sidecar_armed_after_initial_barrier`, `branch_merge_fences_target_delete_recreate_aba`, `branch_merge_fences_concurrent_sync_on_same_handle`, `branch_merge_rejects_fresh_target_manifest_change_before_effects`, `branch_merge_rechecks_late_sidecar_after_table_gates`, `optimize_rechecks_late_schema_apply_sidecar_after_main_gate` (late zero-pin graph-global intent), `optimize_rechecks_late_disjoint_main_sidecar_after_main_gate` (table-disjoint intent sharing `graph_head:main`), `optimize_holds_main_gate_through_disjoint_table_effects` (post-relist branch-gate lifetime), `cleanup_rechecks_sidecars_under_gc_gates`, `full_recovery_rereads_sidecar_body_after_discovery`, `recovery_discovery_skips_sidecar_deleted_after_list` (an unrelated write succeeds after a listed sidecar is published/deleted), and `read_only_recovery_discovery_skips_sidecar_deleted_after_list` (read-only open succeeds against that same concurrent completion). The suite also includes the five per-writer effect → manifest-CAS recovery tests, write-entry in-process heal contract, storage-fault matrix, S3 recovery twin, and convergence-idempotent roll-forward regression. |