omnigraph/docs/dev/testing.md
Ragnor Comerford 0dce7c8d18
feat(engine): unify constraint validation across all write surfaces (#314)
* feat(engine): unify constraint validation across all write surfaces

Constraint enforcement (value/range/check, enum, uniqueness, edge
referential integrity, cardinality) was implemented three times — once
each in the bulk loader, the mutation executor, and the branch-merge
path — and had drifted: merge validated @range/@check but not enum, and
neither the mutation nor the load path enforced cross-version uniqueness
against already-committed rows.

Introduce one catalog-derived evaluator (`crate::validate`) that all
three surfaces route through. It is delta-scoped (checks only the change
set, not the whole graph) and index-backed (probes committed state
through the @key/@unique/src/dst BTREEs instead of full-scanning every
catalog table), reusing the existing leaf checks
(validate_value_constraints, validate_enum_constraints,
composite_unique_key) so the surfaces cannot drift again. A one-row-delta
merge now opens ~3 data tables instead of ~6+, and validation cost is
flat in graph size rather than O(V+E).

Behavior changes (all stricter, none relaxed):
- Enum constraints are now enforced on the merge path (was a gap).
- A write or load whose @unique value collides with an already-committed
  different row is now rejected (cross-version uniqueness); re-upserting
  an existing @key still upserts.
- Uniqueness distinguishes a duplicate key WITHIN one input batch (two
  distinct records -> rejected, e.g. a bulk load listing a @key twice)
  from the SAME id reappearing ACROSS batches (ordered supersession of
  one logical row -> coalesced, e.g. a mutation insert-then-update).
- Overwrite loads validate per-table: a touched table's committed view is
  its replacement image (empty), but a table absent from the batch keeps
  its committed rows, so an edges-only overwrite still resolves
  referential integrity against retained nodes.

Remove the per-surface validation orchestration the evaluator supersedes,
and the now-orphaned version-pinned dataset opener from the sealed
storage trait (reads route through the snapshot path). Docs (invariants,
testing) updated; full engine suite green.

* test(engine): pin orphan-edge validation on adopt-by-pointer merge

Regression for a gap in the unified merge validation: when a table is
adopted by pointer switch (AdoptSourceState) — source on main, target on a
branch — build_merge_changeset skips it, so referential integrity is never
checked for it. Merging main into a branch that deleted a node while main
added an edge to that node silently publishes the orphan edge.

This test merges main -> feature where feature deleted Bob and main added
Knows Alice->Bob, and asserts an OrphanEdge conflict. Red against HEAD
(merge returns Merged); turns green with the AdoptSourceState validation fix.

* fix(engine): validate adopt-by-pointer merge tables (AdoptSourceState)

The unified merge validator skipped any table classified AdoptSourceState
(a pointer switch / fork), so referential integrity, uniqueness, and
cardinality were never checked for it. Merging main into a branch that
deleted a node while main added an edge to that node silently published the
orphan edge — the prior full-scan validation caught it.

Root cause: classify_adopt keyed AdoptSourceState on the publish mechanism
("does it advance Lance HEAD") and returned before computing any delta, and
build_merge_changeset then skipped the table. Fix decouples the validation
input from the publish mechanism: classify_adopt now always computes the
source-vs-target delta (base == target on this path, so it is the right
validation delta) and carries it as AdoptSourceState { validation_delta };
build_merge_changeset validates it exactly like AdoptWithDelta. The publish
stays a pointer/fork (delta ignored) and remains excluded from recovery
pins, so publish/recovery semantics are unchanged — only validation is
restored. Closes the class: no publish optimization can bypass validation.

Turns the orphan-edge regression test green.

* test(engine): pin typed committed-uniqueness probe on non-String columns

The cross-version @unique check pushes a committed-state filter built from
the stringified key. On a non-String @unique column (e.g. Date) this compares
a Date32 column to a Utf8 literal — and the stringified key is the raw day
count, so the probe raises "Cannot cast string '20633' to Date32" for ANY
second write to the table (colliding or not).

Two regressions: a colliding Date value must surface a proper "@unique
violation" (not a coercion error), and a non-colliding write must succeed.
Both red against HEAD; green with the typed-literal probe fix.

* fix(engine): build committed uniqueness probe from typed column values

The cross-version @unique check pushed a Lance filter built with a
stringified key (lit(String)) against the real, typed column. On a
non-String @unique column this compared a Date32/numeric/bool column to a
Utf8 literal: a coercion error on Date/Bool (failing every write to the
table) or a silent miss on Float. For Date the stringified key was even the
raw day count, so the literal could never parse.

unique_holders now takes typed ScalarValues, built at the call site via
ScalarValue::try_from_array(group_column, row), so the pushed-down predicate
compares like-typed for any scalar @unique. The in-memory intra-delta dedup
keeps the stringified key (a type-agnostic equality grouping, unaffected).

Turns the Date @unique cross-version regression tests green.

* test(engine): pin id-keyed cardinality on merge-load edge moves/dups

Two cardinality drifts between validation and what commit persists:

- Move (B): a Merge-load that moves an edge to a new src only recounts the
  new src, so vacating a src and dropping it below @card min is missed —
  moving Alice's only WorksAt to Bob silently succeeds under @card(1..).
- Dup (A): a Merge-load batch listing one edge id under two srcs counts it
  under both, but commit dedupes by id (last-wins). Alice gets a phantom
  second edge and a spurious "has 2 edges (max 1)" violation under @card(0..1).

Both red against HEAD; green with the id-keyed last-wins cardinality model.

* fix(engine): key merge/load cardinality by edge id, last-wins

@card validation diverged from what commit persists in two ways: (1) it only
recounted the new src of a delta edge, so a Merge-load that moves an edge to a
new src never rechecked the vacated src and missed a drop below @card min; (2)
it counted raw delta rows, so the same edge id under two srcs in one batch was
counted under both, while commit dedupes by id (last-wins) — a phantom edge
and a spurious max violation.

evaluate_cardinality now coalesces the delta by edge id (last-wins, matching
dedupe_merge_batches_by_id) and builds the affected-src set from both the new
src of each delta edge AND the old committed src of each changed/deleted edge
id; a committed edge is dropped from its src when the delta deletes or
re-places it. The validated edge set per src now equals the committed image.

Turns the edge-move and duplicate-id cardinality regression tests green.

* docs(rfcs): add RFC 0001 — branch merge by fragment adoption

Proposed design for the by-design fix to merge cost/OOM: adopt the source
branch's Lance fragments by reference (base_paths) instead of re-materializing
rows, with a re-home reconciler + branch-delete reference guard closing the
dangling-reference lifecycle, and a reachability-complete cleanup sweep. Grounded
in the public Lance 7.0.0 multi-base APIs and the prior art (Delta shallow/deep
clone, Iceberg/lakeFS reachability GC). Status: Proposed.

* test(engine): pin @card validation on direct edge delete

Deletes stage as predicates, not constructive batches, so a delete-only
mutation produces an empty change-set and validate_changeset no-ops — a
`delete WorksAt where from = X` that removes a source's only edge commits
below @card(1..), while the merge path (which carries deleted_ids) rejects it.

Red against HEAD (the delete commits); green once the delete path resolves
its predicates into the validation change-set.

* fix(engine): validate edge cardinality on delete via resolved predicates

A delete-only mutation produced an empty change-set (deletes stage as
predicates, not constructive batches), so validate_changeset no-op'd and a
`delete Edge` that dropped a source below @card min committed silently — while
the merge path, which carries deleted_ids, rejects it.

validate_staged_mutation now resolves each staged delete predicate against the
live committed table (CommittedState::deleted_ids_matching, a SQL-filter scan
projecting id) and folds the matched ids into the change-set's deleted_ids for
that table. The existing evaluator then recounts the srcs a delete empties
(@card min) and sees removed rows for RI/node-delete — the same faithful
change-set the merge path already builds, so validation matches what commits.
Covers direct edge deletes, node deletes, and node-delete edge cascades
uniformly (all are staged predicates).

Turns the direct-edge-delete @card regression test green.

* refactor(engine): capture deleted ids at delete time, drop validation re-scan

The delete-cardinality fix resolved staged delete predicates a second time at
validation. Instead, capture the removed ids during the delete op's own scan:
execute_delete_edge and the node-delete edge cascade now scan id (not
count_rows), record the ids via MutationStaging::record_deleted_ids, and
to_changeset() folds them into the change-set's deleted_ids. validate_staged_
mutation reverts to plain to_changeset(); CommittedState::deleted_ids_matching
and scan_filtered_sql are removed.

Behavior-preserving (the @card-on-delete test stays green) and strictly fewer
scans — one scan at delete time replaces count-here + resolve-at-validation.
Node deletes already scanned their ids; this reuses that via a shared
ids_from_batches helper. Full engine suite green; workspace builds clean.

* test(engine): pin overwrite-removal RI + coalesced-unique final image

Two reviewer findings, both red against HEAD:

- F1 (High): overwriting a node table removes nodes without expressing them as
  deleted_ids, so a retained edge in a non-overwritten table that references a
  removed node is published as an orphan (edge-RI path-b never runs).
  overwrite_node_removal_rejects_retained_orphan_edge.

- F2 (Medium): evaluate_unique accumulates superseded keys across batches, so a
  mutation that frees a @unique value (Alice.email temp -> final) and reuses it
  (insert Carol.email = temp) false-rejects a valid final image.
  chained_unique_update_then_reuse_freed_value_is_not_a_violation.

* fix(engine): validate overwrite removals (orphan edges, emptied srcs)

An Overwrite load replaces each touched table, but to_changeset() only recorded
the new batch, never the committed rows the overwrite removes. So overwriting
node:Person to drop Bob while a retained edge:Knows(Alice->Bob) referenced him
published an orphan edge unchecked — edge-RI path-b is gated on the node's
deleted_ids, which were empty.

The loader now computes per overwritten table the removed ids (committed ids in
the pinned base minus the replacement batch's ids, via validate::
overwrite_removed_ids) and folds them into the change-set's deleted_ids. The
evaluator then runs RI path-b and cardinality against them — the same faithful
change-set the merge path builds. Overwrite is per-table, so a table absent from
the batch is untouched; a removed node referenced by a retained edge is now a
loud OrphanEdge.

Updates two tests that asserted the old silent-orphan behavior to
self-consistent overwrites (per-table Overwrite can't drop edge endpoints
without also overwriting the edge tables): end_to_end::overwrite_replaces_data
and writes::load_overwrite_with_bad_edge_reference_unblocks_next_load. The
orphan-rejection case itself is pinned by the new validators test.

* fix(engine): evaluate @unique against the coalesced final delta image

evaluate_unique iterated the raw delta batches and accumulated every key it saw
into one cross-batch map, so a coalesced write that frees then reuses a @unique
value within a query — update a row's email to 'temp', update the same row to
'final', insert a new row with 'temp' — false-rejected: 'temp' lingered in the
seen-set from the superseded first write though it no longer holds in the final
image that commits.

Restructure to validate the final coalesced image — the bytes that actually
publish:
- Pass 1 coalesces the delta by id (last-wins) into each id's final key, and
  flags genuine within-ONE-batch duplicates (two distinct input records — the
  bulk-load contract) before coalescing, so an unordered load batch with a real
  dup still rejects.
- Pass 2 checks two distinct final ids holding the same key.
- Pass 3 does the committed cross-version lookup, excluding the delta's own ids.

Entries are sorted by id before the cross-row/committed passes so violation
order never depends on HashMap iteration. Coalescing first also drops the
redundant committed probes a superseded key used to issue.

Pinned by the chained-update red test; preserves intra-batch dup rejection
(consistency::loader_rejects_intra_batch_duplicate_keys) and cross-version
uniqueness (validators).

* style(engine): drop trailing blank line at staging.rs EOF

Left by a block-delete in an earlier refactor; flagged by git diff --check.

* docs(engine): refresh validate.rs module doc to current consumers

The module doc still said the merge path was the only consumer and the write
path a later, mechanical migration, and listed cardinality as a later
increment. Mutation and bulk load have since migrated onto the evaluator and
cardinality ships — correct both so the doc reflects that all three write
surfaces route through one evaluator.
2026-06-30 14:06:49 +02:00

30 KiB
Raw Blame History

Testing

This file is the always-on map of the test surface. Consult it before every task so you know what tests already cover the area you're about to change, what helpers to reuse, and where a new test belongs. The architectural invariant for boundary-matched tests lives in docs/dev/invariants.md.

Where tests live, per crate

Crate Path Style
omnigraph (engine) crates/omnigraph/tests/ Integration tests (28 files), fixture-driven, share tests/helpers/mod.rs
omnigraph-cli crates/omnigraph-cli/tests/ Per-area suites (post-modularization): cli_cluster.rs (cluster command surface + operator-actor cascade), cli_cluster_e2e.rs (spawned-binary lifecycle compositions — lost-state re-import recovery, out-of-band drift, graph-root destruction, multi-graph mixed-disposition convergence), cli_data.rs (load/read/change/branch/commit/export/snapshot/policy/embed/maintenance + operator format cascade), cli_schema_config.rs (init/config, schema plan/apply), cli_queries.rs, parity_matrix.rs (RFC-009 Phase 1: the embedded-vs-remote referee — every forked verb run against both arms with matched Cedar policy and the same actor, scrubbed-JSON + exit-code equality; divergences are pinned in its KNOWN_DIVERGENCES ledger, never silently repaired), system_local.rs (full-cycle cluster lifecycle with a spawned --cluster server, applied-policy enforcement over HTTP, keyed-credential auth, operator aliases), system_remote.rs; share tests/support/mod.rs (hermetic OMNIGRAPH_HOME by default)
omnigraph-cluster mostly in-source #[cfg(test)] mod tests; tests/failpoints.rs (feature-gated); tests/s3_cluster.rs (bucket-gated full lifecycle on object storage) Cluster config parser, local JSON state diff, state CAS/lock handling/recovery, read-only validate/plan/status plus explicit refresh/import graph observations, config-only apply (content-addressed payload publish, disposition gating, composite-digest convergence, idempotent re-apply), catalog payload verification (status read-only, refresh drift + self-heal), failpoint crash-mid-apply / CAS-race coverage, Stage 4A graph creation (create executor, recovery sidecars + sweep rows, create crash windows), Stage 4B schema apply (migration previews in plan, schema executor, schema-apply sweep classification, schema crash windows), Stage 4C gated deletes (digest-bound approvals, delete executor + tombstones, delete sweep rows, delete crash windows), and 5A policy binding metadata (applies_to in the applied revision, binding-change diffing + convergence, pre-5A backfill), and the 5B serving-snapshot read API (converged read, refusal rows)
omnigraph-server crates/omnigraph-server/tests/ Per-area suites (post-modularization): auth_policy.rs, data_routes.rs, schema_routes.rs, stored_queries.rs, multi_graph.rs (cluster-mode boot — converged serving, policy binding wiring, boot refusals — + the concurrent branch-ops matrix), boot_settings.rs (mode inference, PolicySource), s3.rs (bucket-gated: single-graph serving + config-free --cluster s3:// boot), openapi.rs (OpenAPI drift / regeneration); share tests/support/mod.rs
omnigraph-compiler mostly in-source #[cfg(test)] mod tests Parser, type-checker, IR lowering, lint

The engine's tests/ is the principal coverage surface; most graph-shaped behavior is exercised there.

Engine integration tests (crates/omnigraph/tests/)

File Covers
end_to_end.rs Full init → load → query/mutate flow
branching.rs Branch create / list / delete, lazy fork
merge_truth_table.rs Merge-pair truth table (MR-786): all 9×9 (left_op, right_op) cells from {noop, addNode, removeNode, addEdge, removeEdge, setProperty, dropProperty, addLabel, removeLabel}. Adding a new op to OpVariant forces a compile error in build_case until the new row + column are dispositioned. 36 executable cells run through real branch_merge with a structured oracle (MergeOutcome / MergeConflictKind + graph-state assert); 45 cells involving dropProperty/addLabel/removeLabel are recorded as Unsupported until the mutation grammar grows.
writes.rs Direct-publish writes: cancellation, non-strict insert/merge rebase under the per-table queue, strict stale-write conflicts, multi-statement atomicity, MR-794 staged-write rewire (D₂ rejection, insert+update coalesce, multi-append coalesce, partial-failure recovery, load RI/cardinality recovery)
staged_writes.rs TableStore staged-write primitives (stage_append, stage_merge_insert, commit_staged, scan_with_staged, count_rows_with_staged) — primitive-level only; engine code uses the in-memory MutationStaging accumulator instead
forbidden_apis.rs Defense-in-depth source-walk guard: engine code (exec/, db/omnigraph/, loader/, changes/) must not reach around the sealed storage trait to Lance inline-commit APIs, nor open datasets directly (Dataset::open / DatasetBuilder::from_uri/from_namespace) — reads route through Snapshot::open and the held-handle cache; // forbidden-api-allow: <reason> sentinel exempts reviewed lines
lance_surface_guards.rs Pins the Lance API surfaces omnigraph depends on (named runtime + compile-only guards; see lance.md) — the first smoke check on any Lance version bump; e.g. compact_files_still_fails_on_blob_columns turns red when the upstream blob-compaction fix lands
warm_read_cost.rs Cost-budget tests for the warm read path (query-latency work), measured at the object-store boundary with Lance IOTracker (the LanceDB IO-counted pattern): a warm same-branch read does 0 manifest opens, 1 version probe, validates the schema once (Fix 1 / finding A / Fix 2 at commit-history depth); stale same-branch reads perform exactly 2 probes and refresh manifest-only; recreated non-main branches with the same Lance version refresh by incarnation; recreated branch-owned table handles are distinguished by table e_tag or refresh-time cache clearing; recreated traversal topology is protected by per-edge-table e_tag in the graph-index cache key or refresh-time cache clearing; a warm repeat read does 0 table opens via the held-handle cache and a write re-opens only the changed table at its new version/e_tag (Fix 3/6A). Also the CSR topology-build cost guards: fresh_branch_traversal_reuses_main_graph_index (A1 — a lazy-fork branch reuses main's cached CSR index, 0 rebuilds via graph_build_count) and single_edge_query_builds_only_referenced_edge (A2 — a one-edge query builds only that edge via graph_edges_built); both force CSR via the scoped with_traversal_mode seam, so they need no #[serial]. See "Cost-budget tests" below.
write_cost.rs Cost-budget tests for the WRITE path (RFC-013), the latency twin of warm_read_cost.rs on the shared helpers::cost harness (measure/IoCounts/assert_flat/local_graph). Runs on local FS; gates the internal-table term (__manifest scans flat in commit-history depth, lineage rows included — internal_table_scans_are_flat_in_history, now green every-PR since RFC-013 step 2 brought the internal tables into optimize; the test compacts at each depth before measuring) plus green every-PR guards (single-insert data_writes bounded, a per-write read-op ceiling that fails the moment a round-trip is added, and a measure_with_staged fitness assert that a keyed insert routes through stage_merge_insert once with no stage_append/vector-index build). The data-table opener term is S3-only — see write_cost_s3.rs and the backend-split note in "Cost-budget tests" below
helpers/cost.rs The shared cost-budget harness (not a test): IoCounts/StagedCounts (counts by table class), measure/measure_with_staged (the one place the with_query_io_probes + MergeWriteProbes task-local + IOTracker wiring lives; reads per-op deltas via lance's incremental_stats(), the upstream per-request idiom from rust/lance/src/dataset/tests/dataset_io.rs), cost_harness/GraphIoMeter (installs ONE __manifest IOTracker for a whole test body so the graph opens under it and manifest_reads is ground truth — every read regardless of handle age, the warm-coordinator freshness probe included — closing the blind spot where a per-op tracker installed at measure time cannot see a long-lived handle's reads; outside cost_harness, measure falls back to fresh per-op tracking, so write_cost_s3.rs is unaffected), last_manifest_reads() (the manifest read log for assert_io_eq!-style failure diagnostics), assert_flat(curve, select, slack, what), and store-agnostic local_graph/s3_graph fixtures. warm_read_cost.rs, write_cost.rs, and write_cost_s3.rs all consume it so a cost test body is written once and reads in one vocabulary
lifecycle.rs Graph lifecycle, schema state
point_in_time.rs Snapshots, time travel (snapshot_at_version, entity_at)
changes.rs diff_between / diff_commits
consistency.rs Cross-table snapshot isolation, atomic publish
schema_apply.rs Migration plan + apply, schema-apply lock; index materialization deferred to the reconciler (iss-848): apply_schema_defers_vector_index_on_empty_table (an empty-table Vector @index never aborts the apply) and index_only_constraint_apply_touches_no_table_data (adding an @index is metadata-only — no table-version bump)
search.rs FTS / vector / hybrid (bm25, nearest, rrf)
traversal.rs Expand, variable-length hops, anti-join (CSR path — OMNIGRAPH_TRAVERSAL_MODE unset)
traversal_indexed.rs BTREE-indexed Expand (execute_expand_indexed) forced via the scoped with_traversal_mode seam (not the env var), asserted semantically equal to the CSR path. No #[serial] needed — the seam is scope-bound and process-safe. (The CSR topology-build cost guards — fresh_branch_traversal_reuses_main_graph_index (A1, graph_build_count) and single_edge_query_builds_only_referenced_edge (A2, graph_edges_built) — live in warm_read_cost.rs.)
proptest_equivalence.rs Property-based query-correctness invariants over generated graphs (shared key alphabet forces cross-type id collisions, cycles, self-loops) — pins Expand-mode equivalence so a future fork divergence fails loudly instead of silently; #[serial]
ordering.rs ORDER BY contract: descending, multi-key precedence, deterministic key-column tie-break (total order, so ORDER … LIMIT is deterministic), NULL placement (nulls_first = !descending)
literal_filters.rs Execution goldens for non-string/non-integer scalar literal filters (F64/F32/Bool/Date/DateTime) across both the in-memory comparison arm and the Lance-pushdown arm
aggregation.rs count, sum, avg, min, max
export.rs NDJSON streaming export filters
s3_storage.rs S3-backed graph (skipped unless OMNIGRAPH_S3_TEST_BUCKET is set). Includes s3_fresh_branch_traversal_reuses_main_graph_index_with_etags — the CSR topology cache-key test on a real per-table e_tag (None on local FS, so warm_read_cost.rs can't reach this path); forces CSR via the scoped with_traversal_mode seam
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)
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 optimize (compaction), repair (explicit uncovered-drift publish), and cleanup (version GC): empty/idempotent/no-op edges, policy validation, head preservation; optimize publishes its own compaction (optimize_publishes_compaction_to_manifest_so_schema_apply_succeeds), skips pre-existing uncovered drift (optimize_skips_preexisting_manifest_head_drift), and refuses to run while a __recovery sidecar is pending (optimize_defers_when_recovery_sidecar_is_pending); repair previews/heals verified maintenance drift, refuses raw semantic drift without --force, and forced repair publishes only by explicit operator choice; the index reconciler (iss-848): index_build_tolerates_null_vector_rows (an untrainable Vector column defers instead of aborting the build, sibling indexes still build) and optimize_materializes_index_declared_but_unbuilt (optimize creates a declared-but-deferred index)
failpoints.rs Failure-injection coverage (gated on failpoints feature). Includes the five per-writer Phase B → recovery integration tests (recovery_rolls_forward_after_finalize_publisher_failure, schema_apply_phase_b_failure_recovered_on_next_open, branch_merge_phase_b_failure_recovered_on_next_open, ensure_indices_phase_b_failure_recovered_on_next_open, optimize_phase_b_failure_recovered_on_next_open) and the write-entry in-process heal contract (the four *_after_finalize_publisher_failure_heals_without_reopen tests — load, mutation, schema apply, branch merge: a follow-up write on the same handle rolls a sidecar-covered residual forward without reopen/refresh) and the storage-fault matrix for the sidecar lifecycle (recovery.sidecar_{write,delete,list} / recovery.record_audit failpoints: Phase A put failure aborts with zero drift, Phase D delete failure is swallowed and healed by the next write, list failures are loud at heal and open, audit-append failures are retried to exactly one audit row; plus the bucket-gated s3_load_recovers_after_publisher_failure_without_reopen). And the convergence-idempotent roll-forward regression (open_sweep_roll_forward_converges_when_manifest_advances_concurrently: two concurrent open-sweeps race one sidecar at the recovery.before_roll_forward_publish rendezvous; the CAS loser must converge, not fail the open — iss-schema-apply-reopen-recovery-race).
recovery.rs Open-time recovery sweep — sidecar I/O, classifier dispatch (NoMovement / RolledPastExpected / UnexpectedAtP1 / UnexpectedMultistep / InvariantViolation), all-or-nothing decision, roll-forward via ManifestBatchPublisher::publish, roll-back via Dataset::restore, audit row in _graph_commit_recoveries.lance, OpenMode::ReadOnly skip path
composite_flow.rs Compositional/narrative end-to-end stories — multi-step flows that compose mechanics covered by other test files. Catches integration regressions where individual operations all pass their unit tests but their composition breaks (sequential merges, post-merge main writes, time-travel through merge DAG, reopen consistency over multi-merge histories, post-optimize and post-cleanup strict writes).

Fixtures

crates/omnigraph/tests/fixtures/ holds the canonical schema (.pg), seed data (.jsonl), and queries (.gq) shared across tests. Reuse these before inventing new ones — the helpers harness already knows how to load them.

Test helpers

  • Enginecrates/omnigraph/tests/helpers/mod.rs: init_and_load() (bootstrap a temp graph + load standard fixture), snapshot_main(), snapshot_branch(), query/mutation runners, row collection and counting. Use these instead of hand-rolling.
  • CLIcrates/omnigraph-cli/tests/support/mod.rs: Command-style wrapper for invoking omnigraph, server-process spawning, fixture resolution, output assertion helpers.
  • Server — no shared helpers; server tests call the Omnigraph engine API directly and exercise endpoints over the wire.

Note: the storage adapter has an in-memory backend (ObjectStorageAdapter::in_memory(), full contract including true conditional updates) used by the adapter contract tests in storage.rs. It covers only the text-object layer (sidecars, schema staging, cluster state) — Lance datasets bypass the adapter, so engine integration tests still use tempfile::tempdir(). An in-memory Lance substrate remains an architectural ask — keep it explicit in docs/dev/invariants.md under known gaps.

Failpoints (fault injection)

  • Cargo feature: failpoints = ["dep:fail", "fail/failpoints"] in crates/omnigraph/Cargo.toml; the cluster's failpoints feature additionally enables omnigraph/failpoints (crates/omnigraph-cluster/Cargo.toml), so the shared test guard is available to cluster tests.
  • Wrappers: crates/omnigraph/src/failpoints.rs and crates/omnigraph-cluster/src/failpoints.rs each expose maybe_fail("name") (per-crate error type). The test-side config guard ScopedFailPoint (new for action strings, with_callback for callbacks; RAII Drop removes the point) lives once in the engine and is reused by both test binaries.
  • Names are compile-checked. Every failpoint name is a pub const in omnigraph::failpoints::names (engine) / omnigraph_cluster::failpoints::names (cluster). Call sites and tests reference the constant, never a bare literal — a typo is a compile error, not a silently-never-firing point. Add a new failpoint by adding its const first.
  • Call sites are inserted at sensitive transaction boundaries (branch create, graph publish commit, the recovery sweep's classify→roll-forward-publish window, cluster apply's payload→state-write window, etc.).
  • Serialize and rendezvous, never sleep. The fail registry is process-global, so every failpoint test carries #[serial] (serial_test). For concurrent tests, use helpers::failpoint::Rendezvous (tests/helpers/failpoint.rs): park_first(name) parks the first thread to hit the point until release(), and wait_until_reached().await blocks on that condition (it doubles as a fired-assertion). Do not coordinate threads with fixed sleeps.
  • Activated tests: crates/omnigraph/tests/failpoints.rs and crates/omnigraph-cluster/tests/failpoints.rs (integration binaries, never in-source — the fail registry is process-global). Run with cargo test -p omnigraph-engine --features failpoints --test failpoints / cargo test -p omnigraph-cluster --features failpoints --test failpoints.

RustFS / S3 integration

CI runs these S3-backed tests against a containerized RustFS server (.github/workflows/ci.ymlrustfs_integration job):

  • cargo test -p omnigraph-engine --test s3_storage (lifecycle/branching + the e_tag-present CSR topology cache-key reuse test — the path local FS can't reach since its e_tag is None)
  • cargo test -p omnigraph-engine --test write_cost_s3 (RFC-013 step 3a's data-table opener cost gate — flat across commit depth on S3; the term local FS can't reproduce)
  • cargo test -p omnigraph-server --test s3 (single-graph serving + config-free --cluster s3:// boot)
  • cargo test -p omnigraph-cluster --test s3_cluster (full control-plane lifecycle on the bucket)
  • cargo test -p omnigraph-cli --test system_local local_cli_s3_end_to_end_init_load_read_flow
  • cargo test -p omnigraph-engine --features failpoints --test failpoints s3_ (recovery-sidecar lifecycle on a real bucket)

Locally, set OMNIGRAPH_S3_TEST_BUCKET (and the usual AWS_* vars including AWS_ENDPOINT_URL_S3 for non-AWS) before running. Without those, S3 tests skip gracefully.

System e2e requirements and suppression

The CLI system tests (system_local.rs) spawn the workspace-built omnigraph and omnigraph-server binaries (cargo provides paths via CARGO_BIN_EXE_*), bind ephemeral localhost ports, and use local-FS temp dirs — no external services, no env vars required; they run in the default cargo test --workspace. The comprehensive cluster lifecycle e2es (multi-server-restart flows) honor an opt-out for constrained sandboxes: set OMNIGRAPH_SKIP_SYSTEM_E2E=1 to skip them with a logged message (the same graceful-skip pattern as the S3 gate). Cargo-native filtering also works: cargo test --test system_local -- --skip local_cluster.

OpenAPI drift

crates/omnigraph-server/tests/openapi.rs regenerates openapi.json and diffs against the checked-in copy. CI auto-commits the regeneration on same-repository PRs and otherwise runs in strict-check mode (env: OMNIGRAPH_UPDATE_OPENAPI).

Examples & benches

  • crates/omnigraph/examples/bench_expand.rs — runnable example (not part of CI).
  • No benches/ directories. Add benches/ per crate when you ship a perf-driven change, and include the motivating workload with the optimization.

Coverage tooling — what's missing

There is no coverage tooling in the repository today: no tarpaulin.toml, no codecov.yml, no coverage CI step. If you want to know whether your change is covered, the answer comes from reading and running the relevant integration tests, not from a tool.

If introducing coverage tooling is in scope for your task, the natural first step is cargo-llvm-cov wired into a separate CI job, and a per-crate threshold rather than a global one.

First principle: check what already covers it

Before writing any new test, check whether an existing test already covers the case. The cost of duplicating coverage is high: more code to read, more places to keep in sync when behavior changes, and more drift when one copy lags. The cost of extending an existing test is usually one extra assertion or one extra fixture row.

How to check:

  1. Map the change to an area — use the engine integration-test table above (branching.rs, writes.rs, search.rs, etc.). The filename usually names the area.
  2. Open the file and skim every test fn name. Test fn names are the index — read them all, not just the first few.
  3. Grep for the symbol or path you're changing. rg <FunctionName> or rg <enum_variant> across all tests/ directories surfaces existing coverage you might miss.
  4. Decide one of three outcomes, in this order of preference:
    • Existing test already asserts the new behavior → no new test needed; this PR is a refactor or no-op behaviorally. Confirm by running the existing test against the change.
    • Existing test covers the area but not your caseadd an assertion or a fixture row to the existing test, don't write a new function with init_and_load() again.
    • No existing coverage in any test file → only then write a new test; put it in the file that owns the area, or open a new file only if the area itself is new.

Three duplicated init_and_load() → run_query → assert_eq blocks where one parameterized test would do is the most common form of test rot in this repository. Don't add to it.

Before-every-task checklist

When you pick up any change, walk through this:

  1. Find existing coverage (per the principle above). Don't just look at the first test file by name — grep for the symbol you're touching across every crate's tests/.
  2. Run those tests locally before editing. cargo test --workspace --locked for the broad pass; -p <crate> --test <file> for a focused loop. Confirm a clean baseline.
  3. Decide extend-vs-new explicitly. If you can extend an existing test (assertion, fixture row, parameterization), do that. Only add a new test fn or new file if no existing one owns the area.
  4. Reuse the helpers. init_and_load(), fixture files, the CLI support harness — re-use them. Don't bootstrap a fresh graph by hand if a helper exists.
  5. Mind the boundary. Per docs/dev/invariants.md, test at the layer the change lives at — planner-level changes deserve planner-level tests, not just end-to-end.
  6. For substrate-touching changes (Lance behavior), reach for failpoints or fixture-driven scenarios, not stubbed-out mocks.
  7. For server / API changes, confirm the OpenAPI regeneration happens in openapi.rs and that the diff lands in openapi.json.
  8. Verify your change makes an existing test fail before it makes the new one pass. If you can break the code without breaking a test, your coverage gap is the problem to fix first.
  9. Bound hot-path cost at history depth. If the change touches a read, write, or open path, add or extend a test that asserts a bounded cost (e.g. a warm same-branch read performs zero Dataset::open, or a per-write read-op count flat across commit depth) against a fixture with realistic commit-history depth, not just realistic row counts. Reuse the shared helpers::cost harness (measure/IoCounts/assert_flat) — don't hand-roll IOTracker wiring. Cost that scales with history is invisible on a shallow fixture and only bites in production. See "Cost-budget tests" below.

Cost-budget tests: bound hot-path cost at history depth

Correctness bugs fail loudly in tests; cost-scaling bugs pass every test and degrade silently in production. The engine read path historically had no cost assertion, and fixtures carry shallow commit history, so an O(commits)-per-query cost stayed green in CI and only surfaced on a long-lived graph (read snapshot resolution re-scanned the internal manifest and commit-graph tables on every query, and those tables were never compacted). Guard against the class:

  • Assert a cost budget, not just a result. For a read/open path, assert the number of Dataset::open calls (or object-store ops) a warm query performs, and that it does not grow with commit count. The reference is LanceDB's IO-counted tests, which assert a cached read costs 0-1 IO and carry a named regression test against "a list call on every subsequent query."
  • Test at history depth. Build a fixture with many commits (not many rows) and assert warm-read cost is flat across depths. A shallow fixture cannot catch an O(commits) cost.
  • Use the shared harness, and gate each term on the backend where it manifests. helpers::cost (measure/IoCounts/assert_flat/local_graph/s3_graph) is the one place the IOTracker/task-local plumbing lives — consume it, don't duplicate it. The write path has two distinct depth terms that split cleanly across backends, and conflating them is a real trap (the local data-table read count grows with depth too, but for a different reason — the merge-insert/RI scan reading O(depth) fragments, reduced by compaction, not by the opener): (1) the internal-table scan term (__manifest fragment scans, lineage rows included) reproduces on any backend including local FS, so write_cost.rs gates it on local every-PR; (2) the data-table opener term (latest-version resolution) is a per-object-store-RPC phenomenon — local-FS resolves latest with one cheap read_dir regardless of the opener used, so the namespace-vs-direct difference is invisible on local and only shows on a real object store (per-version GETs), gated by the bucket-gated write_cost_s3.rs. Same harness, different fixture; each term asserted where it actually appears.
  • Count on the handle that does the reads, not just the one a measured op opens. Lance's IO-counted tests attach the IOTracker to the (warm, cached) dataset and read incremental_stats() per request — the tracker MUST be on the handle performing the reads, or warm-handle reads escape. A per-op tracker installed at measure time cannot see reads on a long-lived handle opened earlier (the warm coordinator's __manifest handle, reused across writes), so such reads were silently undercounted. Wrap a depth-swept body in cost_harness so the manifest tracker is installed before the graph opens and manifest_reads is ground truth (handle-age-irrelevant). The version_probes counter is the freshness-probe call count; ground truth additionally reveals that a write's probe does ~3 object-store RPCs (a read's probe is a 0-IO cache hit). manifest_reads_capture_warm_probe is the guard that this stays true.
  • This is the testing companion to invariant 15 in docs/dev/invariants.md (hot-path cost is bounded by work, not history).

When in doubt, re-read docs/dev/invariants.md — quality gates apply to every change.