CSR arm: an undirected hop chains csr(edge).neighbors with
csc(edge).neighbors under the existing visited/seen_dst gates, so
both-direction pairs and self-loops dedup for free (set semantics),
including per-hop in variable-length BFS. Indexed arm: endpoint_probes
returns both (src,dst) and (dst,src) orientations for Both; the per-hop
scan runs once per orientation into one neighbor_map, deduped by the
existing per-source seen_dst sets. Bulk anti-join: "no edge in EITHER
direction" (csr || csc has_neighbors).
Also fixes a lowering gap the anti-join test caught red: negation
inners are typechecked into a discarded context clone, so the
ResolvedTraversal direction lookup silently fell back to Out inside
not{} — undirectedness now travels on the AST node itself (the syntax
is the source of truth), immune to context-propagation gaps.
Tests: traversal.rs (out ∪ in vs the directional miss, both-ways dedup,
var-length from an incoming-only node, undirected anti-join vs
directional), traversal_indexed.rs both_modes equivalence, and the
proptest arm-equivalence property widened with <knows>{1,3}.
iss-gq-undirected-traversal, the Expand-internal design (no plan-level
Union; unblocked from iss-744/579): the grammar gains an undirected_edge
alternative in the traversal rule (angle brackets are collision-free —
comparisons live in the structurally separate filter production), the
AST Traversal carries `undirected`, Direction gains `Both`, and
typecheck resolves undirected patterns to Both after the new T22 rule:
undirected requires a same-endpoint-type edge (an asymmetric edge is
well-typed in at most one orientation, so the form is rejected with
guidance to use the directional pattern). Lowering passes Both through;
the reverse-expand orientation flip is a no-op for a symmetric
traversal. Bounds ({min,max}) and not{} compose unchanged.
Direction has no serde derives and IR never crosses a wire — no
compatibility surface. Parser/typecheck tests cover the bare, bounded,
inside-not forms, Both resolution on Knows (Person->Person), and the
T22 rejection on WorksAt (Person->Company).
Both review bots caught it: the new fns were inserted between run_once's
cfg(unix) attribute and the fn itself, leaving append_result, git_sha,
and run_once ungated (non-unix name-resolution break, again). All three
gated; results_path keeps the displaced attribute.
The harness's JSON lines were vanishing into terminal scrollback — the
numbers that justify decisions (the 6.9GB merge crash, the prefilter
recall pair) had no system of record, and the v7 baselines are needed
for before/after comparison across the Lance 9 bump. Every run now
appends its record, stamped with a unix ts and the current git sha, to
--out <path> / OMNIGRAPH_BENCH_RESULTS / benches/results.jsonl
(gitignored — host-specific, self-describing via host+params).
Greptile follow-up on the previous cfg(unix) fix: ungated scenario fns
still referenced the unix-only helpers module, failing name resolution
on non-unix hosts (cfg_attr(allow) suppresses warnings, not resolution).
Every item is now cfg(unix)-gated; the non-unix build compiles to the
stub main alone.
Greptile follow-up: the scenario ignored the flag and ran the full query
loop, so a baseline/delta pair measured ~0 instead of the queries'
memory cost. Baseline now exits after seeding + optimize, matching
merge-all-changed's shape.
Greptile review follow-ups: wait4 now retries on EINTR (a delivered
signal previously panicked the harness mid-run via the reaped-pid
assert); a failed setrlimit warns on inherited stderr instead of
silently running the child uncapped while the parent JSON records the
requested cap; and the Unix-only syscalls are cfg(unix)-gated with an
inert Windows stub main so cargo bench compiles everywhere.
The dedicated cost/perf instrument testing.md's write_cost_s3 note has been
promising: one cold, stateful macro-run per scenario in a fresh subprocess
(self-respawn via current_exe), reaped with libc::wait4 so ru_maxrss gives
kernel-exact peak RSS with no sampling; results are JSON lines and there are
deliberately no assertions — a decision instrument, never a CI gate.
Criterion is deliberately not used: statistics over warm in-process
iterations is the wrong model for multi-second stateful scenarios, it
measures no memory, and an OOM under --memory-cap-mb (setrlimit; enforced on
Linux, best-effort on macOS) is a data point that needs crash isolation.
Two scenarios ship with the skeleton:
- merge-all-changed: an embedding table whose branch changed EVERY row's
vector, merged three-way into a diverged main — the changed-delta
concat + hash-join cost of branch_merge. --baseline re-runs the identical
workload minus the merge so the peak-RSS delta isolates it (smoke,
20k rows x 256 dims: ~72 MB merge contribution on a 20.5 MB raw delta,
~3.5x).
- nearest-prefilter: selectivity-s filtered nearest() where matching rows sit
far from the query point — quantifies the post-filter ANN recall deficit
(smoke, 20k rows, s=0.05, k=10: 1000 matching rows exist, 0 returned);
becomes the prefilter latency comparison unchanged once the fix lands.
Greptile follow-up: the zero-norm guard computed the norm and then
normalize_vector recomputed it (a wasted pass at 1536+ dims), with the
inner zero guard unreachable by construction. Normalize in place with
the validated norm; normalize_vector stays for its other caller (the
mock provider).
The mock flag's precedence over OMNIGRAPH_EMBED_PROVIDER is deliberate
and stays (pinned by from_env_mock_flag_wins) — but the override was
silent, and mock vectors are indistinguishable from real ones (correct
dimension, unit norm, deterministic hash), so a leaked test env var
silently poisoned persisted embeds (CLI) and query-time nearest()
(server, for graphs without a bound cluster embedding profile). Emit a
tracing::warn naming the overridden provider; document the precedence
in the embeddings env-var table.
Closes iss-embeddings-mock-override-silent.
Replace the FixedSizeList arm's as_f64().unwrap_or(0.0) coercion with a
loud per-row OmniError::manifest in the loader's own style (property
name included, mutation path's "elements must be numeric" phrasing).
The red test turns green; the whole validators suite stays green.
Closes iss-loader-vector-element-coercion.
iss-loader-vector-element-coercion: the loader's FixedSizeList arm does
val.as_f64().unwrap_or(0.0) — a null element (what json! produces for a
non-finite float) or a string element silently zeroes the vector while
the mutation path rejects the same input loudly. No loader vector
validation had any test coverage; this also pins the existing dimension
check.
Fails as predicted against current code: the null-element load succeeds
(unwrap_err panics on Ok) instead of erroring.
Validate every component is_finite() (naming the offending index) and
reject zero-norm vectors before normalizing, via the existing
Err(String) channel — callers already map it through EmbedCallError
{retryable:false} into OmniError::manifest_internal, so no new error
plumbing. The red tests turn green; docs state the finite/non-zero
contract in embeddings.md.
Closes iss-embedding-nan-validation.
iss-embedding-nan-validation: validate_and_normalize_embedding checks
only dimension. The normalize guard (norm > f32::EPSILON) inverts on NaN
— normalization is skipped and the raw NaN vector returns Ok; an Inf
component normalizes to 0s + NaN; an all-zero vector (no direction,
undefined under cosine/ANN) passes. All were unpinned.
Fails as predicted against current code: unwrap_err() panics on the NaN
case — the poisoned vector is returned Ok.
Lance's scanner defaults to prefilter=false: a filter riding the same
scanner as nearest()/bm25() is applied AFTER the ANN/FTS top-k, so
`limit k` meant top-k of the whole table and a selective predicate
silently starved results (the deny-list's silent-partial-result shape;
measured by the nearest-prefilter bench scenario: 20k rows, s=0.05,
k=10 -> 1000 matching rows exist, 0 returned). Set prefilter(true)
whenever a structured filter is pushed to the scanner: one flag governs
both the vector and FTS sources, plain scans ignore it, and it
re-enables scalar-index acceleration for the predicate under nearest.
The red test turns green: filtered nearest now returns the top-k of
MATCHING rows. Docs state the filters-before-search contract explicitly
(docs/user/search/index.md).
Closes iss-nearest-postfilter-starves-results.
iss-nearest-postfilter-starves-results: a scalar match predicate combined
with nearest returns the post-filtered remainder of the GLOBAL ANN top-k,
not the top-k of matching rows. Fixture: 3 matching docs sit far from the
query vector while 3 non-matching docs occupy the global top-3 — the query
returns 0 rows despite limit 3 and 3 matches existing.
Fails as predicted against current code:
assertion left == right failed: left: 0, right: 3
Greptile review follow-up: a property renamed and widened in one
migration emits RenameProperty followed by ExtendEnum carrying the
post-rename name. That ordering/naming contract is now stated on the
ExtendEnum variant's doc and pinned by
plan_orders_rename_before_widening_and_names_the_new_property, so a
sequential step consumer (or a future apply-ordering change) can't
silently break it.
Same-PR docs for the ExtendEnum step: the supported-steps list in the
schema migration section (with the narrowing/rename/String-conversion
carve-outs and the order-insensitivity note), the OG-MF-106 lint-table
row narrowed to what it still covers, and the schema_apply.rs test-map
row extended with the two new tests.
The widening is planner-verified (superset, same shape), so every
committed row is already valid under the wider set: the apply arm joins
the AddConstraint metadata-only family — accepted-catalog update, no
table work, no version bump — and the unified validator accepts the new
variants on all three write surfaces immediately.
Tests: enum_widening_apply_is_metadata_only_and_accepts_new_variant
(no table-version bump; new variant accepted, original still accepted,
out-of-set still rejected) and enum_narrowing_apply_is_refused
(OG-MF-106 surfaced, graph left writable).
Adding variants to an enum property previously planned as an unsupported
property-type change (OG-MF-106): enum values live inside PropType and
the planner compared whole types. Special-case the one safe shape — same
scalar/list/nullability and the desired value set a strict superset of
the accepted one — into a new ExtendEnum step carrying the added values.
Narrowing, variant renames, and enum<->String conversions still plan as
OG-MF-106; a pure reorder was already a no-op (the schema IR sorts +
dedups enum values).
Planner matrix pinned in in-source tests (widening, reorder-as-no-change,
narrowing, rename, widen+nullability-flip, enum<->String both ways); CLI
plan rendering gains the ExtendEnum arm.
Devin review follow-up on the opener unification: StagedTableNamespace's
open_head opens a DATA table (its table_uri is the per-table physical
path), so it belongs on the table_wrapper/data_open_count bucket, not
the manifest one. Test-only impact (the namespace module is #[cfg(test)]
since RFC-013 step 3a), but the classification should be right where the
cost vocabulary is defined.
Greptile review follow-up on the session-threading commit: the
Arc::new(Session::default()) boilerplate at every test TableStore
construction collapses into helpers::test_session() (and a local twin in
staged_writes.rs, which is primitive-level and deliberately does not
include the helpers module) — a future session-constructor change is a
single-point edit.
Thread the graph's one Lance Session (previously read-path-only via
ReadCaches) into TableStore, so open_dataset_head, the branch ops, and
open_at_entry all attach it through the unified opener — the read path's
handle cache and the write side now warm the same Lance metadata/index
caches. diff_snapshots takes the graph's &TableStore instead of
constructing its own session-less store from a root uri.
Measured effect (write_cost.rs): the local data-table SCAN term — the
merge-insert/RI scan that re-read O(depth) immutable fragment metadata
per write — collapses from growing-with-depth to flat (depth 10: 1 read,
depth 100: 1 read). The opener/scan-split gate is re-pinned to the new
behavior and renamed
(data_table_reads_split_into_flat_opener_and_scan_flat_with_session):
a red there now means a write-side open dropped the session. The
Snapshot-without-caches fallback stays session-less by design (a
detached snapshot has no graph to share a session with); the S3
acceptance of the opener term remains owed to write_cost_s3 per the
RFC-013 handoff.
testing.md's backend-split note updated in the same change (the "local
scan grows with depth" claim is now stale).
The method was a delegating twin of open_dataset_head that discarded its
table_key argument ("retained for signature stability") — a dead
parameter every caller still had to supply, and a second name for the
same open policy on the sealed TableStorage surface. Remove it from the
trait, the TableStore impl, and all call sites, which also lets the two
needs_index_work_* helpers drop their now-unused table_key parameters.
Purely mechanical; no behavior change.
Unify the two instrumented openers (open_dataset_tracked latest-only,
open_table_dataset pinned-only) into a single
instrumentation::open_dataset whose version resolution (Latest | At(v))
is an explicit typed parameter, and route every production Dataset open
in the engine through it — including the previously-raw opens in the
branch ops (delete/list/force_delete_branch), manifest branch delete,
version-metadata resolution, recovery classification/restore, the
recovery-audit table, and schema-apply version GC (whose
forbidden-api-allow sentinel is now unnecessary and removed).
One chokepoint means three properties hold uniformly instead of
per-path: record_open feeds the cost probes on every open (the raw
sites were invisible to the gates), the per-query IO wrapper is
attached with the right class (manifest vs table) everywhere, and the
shared per-graph Session parameter exists on every path — threading it
into the write-side owners lands separately.
Behavior-preserving: every call site keeps its existing version
resolution, session argument, and wrapper class; in-source tests keep
raw opens (out of scope for the chokepoint by design).
Greptile review on #324: the lineage_projection row said the legacy
datasets 'hold ZERO commit rows' but the test asserts the directories are
never created — a strictly stronger invariant; say that. And
write_cost_s3.rs's module doc still claimed it runs in the rustfs CI job,
contradicting the (correct) new table row — align it with reality (run on
demand, cost gate).
The engine integration-test table drifted: failpoint_names_guard.rs,
lineage_projection.rs, merge_fast_forward.rs, scalar_indexes.rs, and
write_cost_s3.rs existed with no row (write_cost_s3 had prose mentions
only), and the file count said 28 where there are 36. testing.md is
@-imported into every agent session as the test-coverage map, so missing
rows defeat its "check what already covers it" contract. Row texts are
derived from each file's own doc comments; the write_cost_s3 row records
its run-on-demand (not every-merge CI) status per the existing
Cost-budget-tests note.
Greptile review on #323: the timeout path (server alive but never healthy)
panicked without the captured stderr — the harder failure mode was less
diagnosable than the port-race one. Read the log after kill+wait so the
final flush is captured. Hoist the io imports to the file's use block, and
reference the #327 design-fix follow-up (server-side :0 binding) at the
retry stop-gap.
spawn_server_process picked a port by binding :0 and DROPPING the
listener before the spawned omnigraph-server rebound it — a TOCTOU
window in which any concurrently-spawning test steals the port. The
loser exits "address in use", but stderr was nulled, so the test
panicked with a bare "server exited before becoming healthy: exit
status: 1" on whichever parity/system test lost the race that run
(~1-in-2 parallel runs locally; 12/12 pass serially).
Fix, two halves:
- Retry the spawn on a fresh port (up to 5 attempts) when the child
exits before passing the health check. A lost race becomes invisible;
a deterministic startup failure still panics loudly after the retries.
- Capture stderr to a temp file (not a pipe, which a chatty healthy
server would fill and block on; not null) and include it in the panic
and per-retry messages, so genuine startup failures carry the server's
own error text.
StdCommand isn't Clone and a reused command would trip clap's
duplicate --bind rejection, so retries respawn via clone_command
(program/args/envs/cwd).
Verified: parity_matrix 3x parallel clean (previously flaked ~every
other run), full omnigraph-cli suite green.
The generic export/import recipe never said what v0.8.0 actually changes.
Add the release-specific section: the internal-schema v4 on-disk deltas
(lineage rows in __manifest, the two retired commit-graph datasets, the
both-direction version gate) and the unified stricter validation with a
branch-based pre-flight recipe, plus the three ways to read the format
version.
Same-PR documentation for the vendored pin: a dated patch-pin stanza in
lance.md's alignment history (mechanism, pinning tests, removal
condition, and the adjacent v8-only #7251 finding that gates iss-986 on
the 7→8 bump), a Known Gaps entry in invariants.md so the pin cannot be
forgotten, and the writes.rs row in testing.md now names the row-id
overlap regression pair.
iss-merge-rowid-overlap-corrupts-filtered-reads / lance#7444: an
update-style merge_insert over a merge-written fragment legally reuses the
updated rows' stable row ids (row-id-lineage spec: updates preserve
_rowid) while the superseded fragment keeps its full sequence plus a
deletion vector. A later delete leaves the overlapping id range sparsely
tiled, and lance-table 7.0.0's RowIdIndex::new asserted dense tiling —
failing every filtered read that builds the id→address map ("Wrong range"
debug assert; "all columns in a record batch must have the same length"
or a silently-wrong batch in release).
The upstream fix (lance#7480, merged 2026-07-01) landed hours AFTER
v8.0.0 was cut, so no release ≤ 8.0.0 carries it. Consume it now as a
vendored pin: vendor/lance-table is the pristine published 7.0.0 source
plus ONLY the #7480 rowids/index.rs hunk (drop the false tiling assert;
hard-error on the true invariant — one live id claimed by two fragments)
and upstream's regression unit test, wired via [patch.crates-io]. The fix
is read-side only, so already-written graphs become readable as-is — no
data repair.
Removal condition (see vendor/lance-table/README.omnigraph.md): drop the
vendor dir + patch entry at the first Lance bump whose lance-table ships
lance#7480 (9.0.0, or a backported 8.0.1). The surface guard
filtered_scan_tolerates_merge_update_row_id_overlap keeps that honest in
both directions.
Turns the previous commit's red tests green. Full workspace gate passes
(cargo test --workspace --locked --no-fail-fast, 68 suites).
iss-merge-rowid-overlap-corrupts-filtered-reads / lance#7444: an
update-style merge over a merge-written fragment reuses the updated rows'
stable row ids (spec-legal overlap); a later delete leaves the overlapping
range sparsely tiled and unpatched lance-table 7.0.0's RowIdIndex::new
fails every filtered read that builds the id→address map.
Two boundaries:
- writes.rs::filtered_read_after_merge_update_and_delete_keeps_row_ids_consistent
(engine walk: merge-load → same-key merge-load → delete → keyed point
lookups), plus the green isolation control
filtered_read_after_append_and_delete_is_consistent.
- lance_surface_guards.rs::filtered_scan_tolerates_merge_update_row_id_overlap
(faithful transcription of lance#7444's minimal repro at the raw Lance
API; pins the surface so a future bump that regresses lance#7480 turns
red).
Both fail on unpatched Lance 7.0.0 with the predicted symptom
(rowids/index.rs:50 "Wrong range" via the take path); green arrives with
the vendored lance-table patch in the next commit.
* ci: shard the RustFS S3 integration job across parallel runners
The RustFS S3 Integration job chronically hit its 75-minute timeout (e.g. on
the v0.8.0 release run) and got cancelled. Root cause is compile time, not test
time: the S3 tests each run in seconds (the write_cost_s3 step took 0.2m once the
engine was built), but the job ran six serial `cargo test` steps across four
crates plus a `--features failpoints` rebuild, and on a cold cache (any Cargo.lock
change, e.g. a release version bump) every suite must recompile the omnigraph-engine
+ Lance/DataFusion tree, summing to ~75m.
Split the suites into a `strategy.matrix.shard` (engine / server / cluster / cli /
failpoints), one suite per shard on its own runner with a per-shard rust-cache key
and `fail-fast: false`. Wall-clock becomes the slowest single shard (~40m cold,
~25m warm) instead of the sum. Bundling suites would not help — each crate adds its
own unique-dep compile on top of the shared substrate — so each gets its own shard;
the failpoints shard is isolated because its distinct feature set recompiles the
engine tree. Timeout lowered 75 -> 50 (headroom over the worst cold shard).
The job is renamed `RustFS S3 Integration (<shard>)`; it is not a required check,
so branch protection is unaffected. Docs updated in docs/dev/ci.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* ci: drop the write_cost_s3 cost gate from the correctness job
The RustFS integration job is a correctness gate. write_cost_s3 is a
deterministic IO-count COST gate (RFC-013 step-3a data-table opener, flat
across commit depth) — a performance contract, not a correctness test.
Cost/perf contracts belong on a dedicated harness with a stable runner and
their own cadence, not on the every-merge correctness path. Remove the step
from the engine shard; a comment + testing.md record how to run it on demand
and note it's pending a dedicated cost harness. The local write_cost.rs
opener/scan-split guard still runs every-PR, so the split stays covered; only
the S3 acceptance of the opener term moves off the correctness path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(release): trim v0.8.0 notes, drop internal RFC references
Shorten the v0.8.0 release notes to a scannable highlights + upgrade format and
remove internal RFC-track references (RFC-013 Phase 7, step 3a, etc.), which are
not public-inspectable and don't belong in OSS-facing release notes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(release): widen rebuild note to any pre-v0.8.0 graph
Greptile P1: the version gate blocks any pre-v4 graph (v0.4.x-v0.7.x), not just
v0.7.x. The trim over-narrowed it and left older-release users without a signal.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(release): cover #314 stricter validation + #316 linux-arm64 in v0.8.0 notes
Two changes landed after the version-bump commit (#313) that wrote the v0.8.0
release notes, so they were undocumented:
- #314 unified constraint validation across the loader, mutation, and merge
surfaces. Its behavior changes are all stricter (enum enforced on merge,
cross-version @unique rejection, precise within-batch vs across-batch dup-key
semantics, per-table overwrite validation) and are user-visible, so they get a
dedicated section.
- #316 added linux-arm64 (aarch64) as a first-class prebuilt target + Homebrew
bottle; note the new platform.
Also mention the stricter validation in the release intro. Docs-only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(release): scope cross-version uniqueness to the write's visible state
Greptile P2: the prior wording implied a live-head @unique guarantee, but #314's
check is snapshot-scoped (probes the write's pinned base view via the
@key/@unique BTREEs), so a concurrent writer committing the same value after the
base was opened is not caught — matching the 'full cross-version uniqueness is
still a gap' note in docs/dev/invariants.md. Qualify with 'visible to that write'.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The test seeded the full fixture (test.jsonl, which carries Knows/WorksAt edges)
then fault-injected a per-table Overwrite of node:Person down to a single row.
Overwrite RI validation now correctly rejects that: dropping the seeded Persons
strands the retained edges, so the load failed with "src 'Alice' not found in
Person" during validation — before it reached the post-finalize failpoint the
test drives, so the recovery roll-forward was never exercised.
Seed Persons only (no edges) so the overwrite is a clean single-table
roll-forward, which is what the test asserts (RolledForward{node:Person}). The
overwrite still removes the seeded Persons, so F1's overwrite-removed-ids path is
still exercised — just without an orphan.
Feature-gated: this binary runs under `--features failpoints`, a separate CI
step from the workspace test run, so it's invisible to `cargo test -p
omnigraph-engine` and only the post-merge main CI exercises it.
The seeded knowledge graph (test.jsonl) carries Knows/WorksAt edges over its
Persons, so the policy test's per-table `--mode overwrite` of a lone Person
stranded those retained edges against the new node image. Overwrite RI
validation now correctly rejects that as an OrphanEdge, so the allowed-actor
load failed with "src 'Alice' not found in Person".
Replace the edge tables in the same overwrite (Knows LoadPolicy->LoadPolicy,
WorksAt LoadPolicy->Acme — Acme is a retained Company) so the load commits
cleanly. The test's subject — engine-layer policy enforcement on a destructive
overwrite — is unchanged; the loaded data was always incidental.
Surfaced only on main: cargo test --workspace fail-fasts at the first failing
binary, and the cli system suite runs only under the full workspace gate, not
-p omnigraph-engine.
* 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.
* ci: add linux-arm64 (aarch64) prebuilt release target
Build an omnigraph-linux-arm64 archive in both the tagged-release and
edge-release matrices on the ubuntu-24.04-arm runner, and teach the
install script to map Linux/aarch64 to the new asset. Update the install
and CI docs to list the new platform.
Previously aarch64 Linux hit the install-script arch guard
("no prebuilt binary is available for Linux/aarch64") and could only
build from source; it is now a first-class prebuilt target.
* ci: emit a linux-arm64 bottle in the Homebrew formula
The formula generator only resolved macos-arm64 and linux-x86_64 and
emitted `on_linux { on_intel }`, so `brew install` on Linux/aarch64 had
no URL/sha and failed even though the release now ships an
omnigraph-linux-arm64 archive. Resolve that asset's digest and add an
`on_arm` block under `on_linux` so the documented Homebrew path matches
the new prebuilt target.
* perf(engine): scope the CSR topology index to traversed edges, reuse it cross-branch
The in-memory CSR graph index was built over every edge type in the catalog and
cache-keyed by the resolved snapshot id, so a single-edge join
(`$x identifiesPerson $p`) full-scanned every edge table in the graph (the
40-60s / 428s-first-traversal hang), and a lazy-fork branch cold-rebuilt main's
index. Two cuts close that:
- Scope (A2): build only the edge types the query traverses
(`referenced_edge_types` over Expand/AntiJoin, exhaustive match), not the whole
catalog. Threaded through GraphIndexHandle -> RuntimeCache; cache-keyed on the
scoped set.
- Cross-branch reuse (A1): key RuntimeCache by each edge table's physical identity
(table_key, version, table_branch, e_tag) instead of the snapshot id, so a
lazy-fork branch whose edge tables physically are main's reuses main's built
index. Local-FS (e_tag None) falls back to refresh-invalidation.
Adds graph_build_count/graph_edges_built probes for the cost tests.
* test(engine): cost tests for scoped + cross-branch-reused topology index
fresh_branch_traversal_reuses_main_graph_index (A1: a lazy-fork branch reuses
main's cached CSR index, 0 rebuilds) and single_edge_query_builds_only_referenced_edge
(A2: a one-edge query builds only that edge, not the whole catalog), via the
graph_build_count/graph_edges_built probes. Forced CSR mode, #[serial]. Updates the
recreated-branch incarnation test comment for the physical-identity key.
* docs(engine): topology-index scoping + physical-identity cache key
Document the scoped CSR build and the physical-identity (e_tag) graph-index cache
key with its local-FS refresh-invalidation fallback across invariants, testing,
execution, and architecture docs.
* fix(test): move CSR-forced topology cost tests to the all-serial binary
The two topology-build cost tests force OMNIGRAPH_TRAVERSAL_MODE via process-
global env mutation, which query.rs reads. In warm_read_cost.rs (a mixed
serial/non-serial binary) a concurrent non-serial traversal test could race the
env write (UB under Rust 2024's unsafe set_var contract) and be forced onto CSR.
Move them to traversal_indexed.rs — the dedicated all-serial binary with no
non-serial env reader (its documented-safe home) — and add a ModeGuard RAII
helper so a panic mid-test cannot leak the override. Addresses a PR review (P2).
* fix(engine): include edge endpoints in the graph-index cache key
The A1 physical-identity key omitted the edge's (from_type, to_type). GraphIndex
keys its TypeIndexes by those endpoint names and execute_expand_csr looks them up
by the current catalog's names, so a schema repoint of an edge type that leaves
the edge table's physical identity unchanged would reuse a stale index built with
the old endpoint namespace and fail with "no type index for <new type>". The old
snapshot_id (carrying the manifest version) masked this; dropping it exposed it.
Adding the endpoints to the key rebuilds on a repoint while preserving lazy-fork
cross-branch reuse (same endpoints -> same key). Addresses a PR review (P1).
* test(engine): scoped with_traversal_mode seam + e_tag graph-index coverage
Replace the process-global OMNIGRAPH_TRAVERSAL_MODE env-mutation test hack (which
forced #[serial] + dedicated all-serial binaries and was triplicated as ModeGuard
+ set_mode/clear_mode) with one general abstraction: a task-local
`with_traversal_mode` seam mirroring `with_query_io_probes`. It is scope-bound
(leak-free even on panic) and process-safe (never touches shared state), so a
forced-mode test cannot affect a concurrent test in the same binary.
`traversal_indexed_override` consults the seam first, then the env var (which
stays the documented ops escape hatch).
- Migrate traversal_indexed.rs, proptest_equivalence.rs, and the two topology cost
tests (moved back to warm_read_cost.rs) to the seam; drop all ModeGuard /
set_mode / clear_mode / #[serial] / per-file column0 helpers.
- Consolidate the duplicated first-column extractors into one shared
`helpers::first_column_sorted`.
- Add `s3_storage.rs::s3_fresh_branch_traversal_reuses_main_graph_index_with_etags`:
the CSR cache-key cross-branch reuse path on a REAL per-table e_tag (None on
local FS, so local tests can't reach it). Confirmed empirically that RustFS — the
CI S3 backend — surfaces ETags into version_metadata.e_tag(). CI path filter now
triggers the rustfs job on runtime_cache/graph_index changes.
* docs: state cluster apply is storage-direct, not server-routed
`cluster apply` reaches the object store directly — the `__cluster/` ledger
and each graph's Lance datasets — never through a running omnigraph-server,
so the host that runs it needs storage credentials. The rationale (declarative
control plane, not a runtime mutation API) was documented in cluster-axioms.md
§3/§4, and the out-of-band/direct-storage fact was stated for the maintenance
verbs and init/load, but never spelled out for apply itself.
- docs/user/clusters/index.md: add a day-2 note making apply's storage-direct
execution and credential requirement explicit, linking the why to axioms 3/4.
- skills/omnigraph/SKILL.md: extend the "init/load write storage directly
(bypassing the server)" line to include cluster apply, with the same reasoning.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs: disambiguate the §5 cross-reference in cluster apply note
The trailing (§5) sat right after the cluster-axioms.md §3/§4 citation, so a
reader could read §5 as referring to cluster-axioms.md (whose §5 covers locked
state) rather than this guide's §5. Make it an explicit same-page forward
reference. Addresses Greptile P2 on #306.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs: don't claim the server is read-only against storage
The "server only reads from it" wording was wrong: the data plane serves HTTP
writes (mutate/load/branch) that go through the server to the graph datasets,
so omnigraph-server is not read-only against object storage. The hazard is an
operator granting the server read-only S3 creds and breaking runtime writes.
Scope the read-only claim to cluster (control-plane) state at boot, and state
that data-plane writes still need read-write storage access. Addresses Greptile
P-level finding on #306.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Ragnor Comerford <ragnor.comerford@gmail.com>
* docs(dev): write-latency roadmap (validated cost model + layered fix)
Records the validated 6-LIST warm-write cost model, the two root causes
(un-GC'd _versions/; re-resolving latest by listing), and the layered fix
(GC + capture-once reuse), plus how commit-graph-table retirement feeds in.
Linked from docs/dev/index.md next to the RFC-013 docs.
* feat(engine)!: strand storage versioning — one internal-schema version, no in-place migration
Set MIN_SUPPORTED == CURRENT == 4: this binary reads exactly one `__manifest`
internal-schema version and refuses any older graph on open with a
rebuild-via-export/import message, instead of migrating it in place. Storage
format changes become a deliberate cutover, not a permanently-carried in-place
migration — the pre-release "complexity must be earned" contract.
Delete the entire in-place migration apparatus and everything that existed only
to support it: the `migrate_vN` arms + dispatcher + stamp-bump helpers + the
schema-version-floor tripwire; `migrate_on_open` (both open modes now refuse);
the legacy `_graph_commits.lance` readers + the v3 test fixtures + migration
tests + `migration.v3_to_v4.*` failpoints + the two surface guards that pinned
Lance variants only the migration matched on; and `state::merge_lineage_rows`.
Keep `read_stamp` / `stamp_current_version` / `set_stamp` /
`refuse_if_stamp_unsupported` — the seam a future one-shot converter plugs into.
`load_commit_cache_for_branch` now reads the `__manifest` projection
unconditionally (sub-v4 graphs are refused at open). Adds
`sub_current_graph_is_refused_on_open_with_rebuild_hint`.
The commit-graph TABLES are still created/used as branch-ref ledgers — their
retirement (CommitGraph -> pure `__manifest` projection) is the next commit.
BREAKING CHANGE: a graph created by omnigraph <= 0.7.2 (internal schema v3) is
refused on open. Rebuild it: `omnigraph export` with the old release, then
`omnigraph init` + `omnigraph load` with this one. Data, vectors, and blobs are
preserved; commit history and branches are not.
* feat(engine)!: retire `_graph_commits.lance` / `_graph_commit_actors.lance` — CommitGraph is a pure `__manifest` projection
Since RFC-013 Phase 7, graph lineage lives in `__manifest` (`graph_commit` /
`graph_head` rows) and branch authority is `__manifest` (branch create forks it
first). The two commit-graph datasets were vestigial: `_graph_commit_actors.lance`
was never written or read; `_graph_commits.lance` carried zero commit rows and
only mirrored the manifest's branch refs (a deny-list "parallel copy"). Retire
both.
- `CommitGraph` collapses to a pure projection: drops its Lance dataset handles
(`dataset`/`actor_dataset`) and all branch methods; `open`/`open_at_branch`/
`refresh`/`init` open NO dataset, building the cache from
`ManifestCoordinator::read_graph_lineage_at`. Removes ~1.4s of cold-open
dataset opens.
- `graph_coordinator`: `commit_graph` is now non-`Option` (always a valid
projection). `branch_create`/`branch_delete` go through `ManifestCoordinator`
only — a single atomic op, replacing the two-step manifest-fork +
commit-graph-fork + rollback. Deleted `create_commit_graph_branch`,
`reclaim_commit_graph_branch`, `ensure_commit_graph_initialized`, and every
`storage.exists(_graph_commits.lance)` gate.
- `optimize`: dropped `reconcile_commit_graph_orphans` and the two tables from
the internal-table compaction set (now `__manifest` only).
- `instrumentation`: `INTERNAL_TABLE_DIRS` no longer lists the two tables.
- Fresh graphs create neither table; `lineage_projection.rs` now asserts both
`.lance` dirs are absent. Deleted the obsolete commit-graph-branch-race
failpoint tests + their failpoint names, and updated the `maintenance`
optimize tests (one internal table, not three).
Review-pass fixes folded in:
- Removed two stale `omnigraph.rs` in-source tests the prior run missed (a
disk-full link failure masked them): one asserting `open` probes
`_graph_commits.lance` (the exists-gate this commit removes) — it was masked
earlier by a disk-full link failure.
- Corrected src comments referencing deleted code (`migrate_v3_to_v4`,
`append_commit`/`append_merge_commit`, the three-internal-table list,
the `_graph_commits` reconcile owner) in publisher/recovery/optimize/recovery_audit.
- Narrowed `set_stamp_for_test` to `cfg(test)` (its only caller is the refusal
test) — removes a dead-code warning in the failpoints build.
Branch create/delete atomicity improves (single atomic `__manifest` op). No
behavior change for reads or branches.
Follow-up (separate commit): the now-always-0 `IoCounts::commit_graph_reads` test
counter + its `IOTracker`, threaded through ~11 cost-test files.
* feat: surface the internal-schema (storage-format) version to operators
After stranding storage versioning (a sub-v4 graph is refused on open), operators
could only discover the storage-format version by hitting a refusal. Surface it:
- `omnigraph version` prints an `internal-schema <N>` line (the binary's CURRENT
storage-format version).
- `omnigraph snapshot` includes `internal_schema_version` — the GRAPH's per-branch
on-disk stamp, read via the new `Omnigraph::internal_schema_version_of`.
- `GET /healthz` includes `internal_schema_version` (server-scoped: the binary's
CURRENT, alongside `version`/`source_version`).
Wire: re-expose `INTERNAL_MANIFEST_SCHEMA_VERSION` as `pub` on `db::manifest`;
add `internal_schema_version: u32` to `SnapshotOutput` + `HealthOutput`;
`snapshot_payload` takes the per-graph version (the `Snapshot` does not carry it),
threaded through the embedded CLI + server snapshot callers. `openapi.json`
regenerated (two added int32 properties). Extends the existing healthz / snapshot /
version tests.
* docs(engine): gate internal-schema version at the graph level; record the per-branch read gap
PR reviewers flagged that the open path validates only main's internal-schema stamp, so a branch read could decode a branch stamped outside this binary's range. The stamp is a graph-wide storage-format property (the upgrade path is a whole-graph export/import), so with one binary version every branch is always CURRENT; divergence needs concurrent multi-version writers, an unsupported topology already in one-winner-CAS territory. Gating per-branch would add a second __manifest open per non-main branch read to defend a state we do not support, unearned complexity that regresses the warm-read budget.
Keep the graph-level gate, document it at the code site (refuse_if_internal_schema_unsupported), and record the read-only residual hole as a known gap in invariants.md to close only when multi-version write topologies become supported. Also clarify the sub-floor rebuild message to say "export with the older omnigraph binary that created it."
No behavior change: HEAD already gated at the graph level.
* test(cost): remove the dead commit_graph_reads IO counter
Phase B retired _graph_commits.lance / _graph_commit_actors.lance, so no commit-graph dataset is opened and the commit_graph IOTracker term is structurally always 0. Remove IoCounts::commit_graph_reads, its total_reads() term, the commit_graph IOTracker in OpProbes, and the now-dead commit_graph_wrapper field on QueryIoProbes (it had no accessor — nothing ever attached it). Drop the 7 trivially-true assert_eq!(commit_graph_reads, 0) checks in warm_read_cost.rs and the debug-print refs in write_cost{,_s3}.rs.
Lineage and actor rows now live in __manifest (RFC-013 Phase 7), so the internal_table_scans_are_flat_in_history gate folds into the single manifest_reads flat-assertion — the manifest scan already covers them. Harness-only; no production runtime impact.
* docs: align with the commit-graph retirement + strand storage versioning
Update the always-loaded and user-facing docs to match the landed state: graph lineage lives in __manifest, the _graph_commits.lance / _graph_commit_actors.lance tables are retired, and storage is strict-single-version (no in-place migration — a sub-CURRENT graph is refused with an export/import rebuild).
Fixed stale claims in invariants.md (the migration/atomicity known-gap entry, the Truth Matrix branch-delete row, the read-path/optimize internal-table scope), lance.md (the migrate_v1_to_v2 PK bullet now reflects init-time set; removed the two deleted v3->v4 migration surface guards), testing.md (dropped the deleted migration failpoint tests; manifest-only internal-table term), writes.md (rewrote the Migration-code section to the strand model), storage.md / maintenance.md / constants.md (retired tables out of the layout, internal-table compaction scope, and the constants cheat-sheet), and AGENTS.md. Marked the retirement DONE in the RFC-013 handoff/roadmap and banner-noted the historical RFC analysis.
Added docs/user/operations/upgrade.md (the export/import rebuild recipe) and docs/dev/versioning.md (the four-axis compatibility policy: release lockstep / wire additive / storage strict-single-version / Lance pinned), cross-linked from the audience indexes and the AGENTS.md topic map, and rewrote the in-progress v0.8.0 release note for the strand model + version surfacing. check-agents-md.sh passes (65 links, 62 docs).
* test(manifest): cover the v3-refusal→export/import rebuild cycle and branch stamp inheritance
Two coverage additions from PR review (P1):
(a) sub_current_graph_is_refused_then_rebuilt_via_export_import — the full operator narrative in one flow: load → export → a sub-CURRENT graph (stamp rewound below CURRENT) is refused with the export nudge → fresh init + load(export) → data present and the rebuilt graph opens. The refusal is stamp-only (read before any data), so a stamp-rewound graph is a faithful stand-in for a real older-release graph without a second binary; vector/blob fidelity stays covered by tests/export.rs.
(b) branch_inherits_main_internal_schema_stamp — proves a branch cannot diverge from main's stamp under single-binary operation (create_branch forks main's __manifest, the publisher does not re-stamp), which is why the graph-level (main-only) stamp gate is sufficient for supported inputs. A divergent branch stamp needs concurrent multi-version writers, the unsupported topology recorded as a known gap.
* test(engine): pin zero-row cascade delete must not drift an edge table (red)
A delete <Node> cascades a delete_where into every incident edge type. The
inline delete_where (Dataset::delete) advances Lance HEAD even when zero edges
match, but the cascade records the new version only if deleted_rows > 0 — so a
node with no incident edges leaves edge:Knows HEAD>manifest drift, which trips
the next strict write's ExpectedVersionMismatch and repair refuses it.
Red today: edge:Knows manifest=v5, Lance HEAD=v6. Goes green when delete moves
to the staged two-phase path (iss-950, Lance 7.0 DeleteBuilder::execute_uncommitted),
where a 0-row delete commits no Lance version and the deleted_rows>0 gate becomes
correct by construction.
* fix(engine): a zero-row delete must not advance Lance HEAD
Lance's Dataset::delete commits a new version even when the predicate matches
nothing (build_transaction always emits Operation::Delete), so a node delete
that cascades a delete_where into an incident edge type with no matching edges
advanced that edge table's Lance HEAD while the cascade skipped record_inline
(gated on deleted_rows > 0) — leaving HEAD>manifest drift that wedged the next
strict write and that repair refused as suspicious/unverifiable.
Use Lance 7.0's two-phase DeleteBuilder::execute_uncommitted to read
num_deleted_rows before committing: a no-match delete now advances nothing (no
version, no drift) and the existing deleted_rows>0 gate is correct by
construction. Non-zero deletes commit the staged transaction with
skip_auto_cleanup + affected_rows (parity with the prior inline path).
First step of the staged-delete migration (iss-950); turns the
node_delete_with_no_incident_edges_leaves_no_edge_table_drift regression green.
* feat(engine): stage_delete two-phase primitive (MR-A step 0)
Add TableStore::stage_delete (Lance 7.0 DeleteBuilder::execute_uncommitted),
the two-phase analogue of stage_merge_insert: writes deletion files without
advancing Lance HEAD, returns Option<StagedWrite> (None on 0 rows = true no-op),
carrying the deletion-vector updated_fragments as new_fragments and the
superseded originals as removed_fragment_ids so combine_committed_with_staged
makes the deletion visible to in-query reads.
No affected_rows is threaded: like stage_merge_insert's Operation::Update commit,
the staged delete relies on OmniGraph's per-table write queue + manifest CAS, not
Lance's per-dataset conflict resolver (commit_staged is a single attempt).
Flip the two residual guards to the staged path: staged_writes.rs now asserts
stage_delete does NOT advance HEAD and that a staged delete is read-your-writes
visible (the deletion-vector RYW proof D2 retirement depends on); the
lance_surface_guards delete guard pins execute_uncommitted's UncommittedDelete.
No behavior change yet (callers still use delete_where); Step 1 wires them.
* feat(engine): TableStorage::stage_delete + migrate merge delete path (MR-A step 1a)
Add stage_delete/Option<StagedHandle> to the TableStorage trait (delegates to
TableStore::stage_delete). Migrate the two branch_merge delete sites
(three-way RewriteMerged + adopt delta) from the inline delete_where residual to
stage_delete + commit_staged — identical in shape to the stage_merge_insert +
commit_staged pair above each. HEAD still advances within the merge sequence
(via commit_staged), under the unchanged SidecarKind::BranchMerge Phase-B
confirmation; the _pre_delete/_pre_index failpoints fire by position, unchanged.
merge_truth_table, branching, composite_flow green.
* feat(engine): migrate all delete sites to staged path, retire inline delete (MR-A step 1b/1c)
Routes every delete through the staged write path so delete never advances
Lance HEAD inline — the last inline-commit residual on the mutation path is
gone. `MutationStaging` now accumulates delete predicates (`record_delete`)
alongside pending write batches; at end-of-query `stage_all` combines a
table's predicates into one `(p1) OR (p2) …` `stage_delete` (a deletion-vector
transaction, no HEAD advance) and `commit_all` commits it through the same
`commit_staged` path as inserts/updates. Deletes are now ordinary staged
entries: one sidecar pin at `expected + 1`, no inline special-casing.
Migrated callers (all 5): the 3 mutation.rs sites (delete-node, cascade,
delete-edge) and the 2 merge.rs sites (already on stage_delete in step 1a).
`affected_edges`/`affected` move from post-inline-commit `deleted_rows` to a
committed `count_rows` at record time — exact under D₂, bounded by the cascade
working set. A predicate matching zero rows stages nothing (the staged
equivalent of the old "skip record_inline on 0 deleted rows"), so the zero-row
edge-table drift class stays closed by construction.
Retired scaffolding now that no caller remains:
- `MutationStaging.inline_committed` + `record_inline` → `delete_predicates` +
`record_delete`; `StagedMutation.inline_committed`/`paths` fields and all the
`commit_all` inline handling (queue keys, sidecar pins with the
`record_inline` table_version special-case, the inline recheck loop).
- `open_table_for_mutation`'s post-inline-commit reopen branch (deletes no
longer advance HEAD mid-query, so a second touch reopens at the pinned
version like any write).
- `InlineCommitResidual::delete_where` + its `TableStore` impl, the orphaned
`TableStore::delete_where`, and `DeleteState`. `InlineCommitResidual` now
carries only `create_vector_index` (Lance #6666 still open).
D₂ stays for now: staged-delete read-your-writes doesn't yet compose into the
pending accumulator (insert-then-delete on one table), so mixed
insert/update/delete in one query is still rejected at parse time. Retiring D₂
is step 2. Doc comments updated to match across exec/, storage_layer, db/.
Tests (all green): writes, consistency, validators, end_to_end, composite_flow,
merge_truth_table, maintenance, recovery, staged_writes, forbidden_apis,
lance_surface_guards, changes, point_in_time (286), plus failpoints (63).
* docs: delete is a staged write, not an inline-commit residual (MR-A step 1)
Update the docs that described `delete` as the inline-commit residual now that
MR-A routes it through `stage_delete`. Always-loaded surfaces (AGENTS.md rule
4 / capability matrix, invariants.md Invariant 4 / truth matrix / known gaps)
plus the dev write-path docs (writes.md, execution.md incl. its mutation
sequence diagram, architecture.md) now state: deletes accumulate as predicates
and stage like inserts/updates, no inline HEAD advance; `InlineCommitResidual`
carries only `create_vector_index` (Lance #6666). The parse-time D₂ rule is
documented as retained — not because delete inline-commits, but because
staged-delete read-your-writes is not yet wired into the pending accumulator
(MR-A step 2). lance.md's 7.0 audit note marked MR-A as landed.
* docs: D₂ is a deliberate boundary, not temporary scaffolding (MR-A close-out)
After MR-A staged the delete path, D₂ (a mutation query is insert/update-only
OR delete-only) was left framed as temporary — "until Lance ships two-phase
delete" / "retire in step 2". Lance shipped that and we used it for the
inline-commit fix; D₂'s original justification is gone. It now stands for a
different, permanent reason: keeping a query to one kind keeps its
read-your-writes unambiguous and each table to one version per query. Retiring
it would buy single-commit mixed atomicity (cheap workaround: split, or a
branch) at the cost of an in-query delete view, pending pruning, edge
id-resolution, and two-commit-per-table ordering in the hot mutation path —
complexity not worth earning. Decision: keep D₂ as a deliberate boundary.
Reframes the now-stale wording everywhere, no logic change:
- The D₂ parse-time error message no longer promises "this restriction lifts
when Lance exposes a two-phase delete API"; it states the boundary and points
to a branch+merge for one atomic commit.
- `enforce_no_mixed_destructive_constructive` doc, AGENTS.md, invariants.md
(Invariant 4 / truth matrix / removed from the known-gaps), writes.md,
architecture.md, lance.md, and the user mutations doc (which wrongly said
deletes "commit through a different path" — both stage now).
- Swept remaining stale `delete_where` mentions left from the Step-1 migration:
the merge.rs "swap when upstream ships" comments (already swapped), the
forbidden_apis / table_ops residual notes, the staged_writes vector-index
guard doc (was "same as stage_delete's absence" — stage_delete now exists),
and test comments/assert messages in recovery/maintenance/writes/failpoints.
Genuinely-historical records (dated Lance audit, rfc-013, bug-case-fix) left.
Verified: engine builds warning-free; check-agents-md OK; writes/maintenance/
recovery/staged_writes/forbidden_apis all green. Closes MR-A.
* test(engine): overlapping delete predicates must not double-count affected_* (red)
Reproduces a reporting regression from the staged-delete migration flagged in
PR #308 review. Because deletes now stage (instead of inline-committing), two
delete statements in one query both scan the same unchanged committed snapshot;
counting each predicate independently over-reports `affected_*` when they
overlap. The old inline path committed each delete before the next ran, so it
counted distinct.
`delete Person where name = "Alice"` then `delete Person where age > 29` over
the standard fixture (Alice 30, Charlie 35) removes 2 distinct nodes and 3
distinct edges, but the buggy per-statement counting returns 3 nodes / 6 edges.
RED at this commit (asserts left=3, right=2).
* fix(engine): dedup overlapping delete predicates when counting affected_*
Count each delete statement against the committed snapshot MINUS the predicates
a prior delete statement on the same table already recorded:
`(pred) AND NOT ((prior1) OR (prior2) …)`. Summed over statements this is
inclusion-exclusion — `Σ |pₙ \ (p₁ ∪ …)| = |p₁ ∪ p₂ ∪ …|` — exactly the distinct
count the combined `(p1) OR (p2)` staged delete removes. Works for nodes and
edges alike with no edge identity needed; the node ID scan uses the same
exclusion so a later statement also doesn't re-cascade already-deleted nodes.
The ORIGINAL predicate is still what gets recorded (the staged delete removes
the union); only the count uses the exclusion. The common single-delete path is
unchanged (`prior` empty → filter is just the base predicate).
New helper `dedup_delete_filter` + `MutationStaging::recorded_delete_predicates`.
Turns the red regression test green (2 nodes / 3 edges); writes (33),
end_to_end, validators, maintenance, recovery, composite_flow, merge_truth_table,
consistency, changes, and failpoints (63) all stay green.
* test(engine): delete dedup must not drop NULL-column rows (red)
Follow-up to the overlapping-delete fix flagged in PR #308 review (Greptile P1):
the `(base) AND NOT (prior)` exclusion breaks under SQL three-valued logic. If a
prior delete predicate references a NULLable column, a later statement's
matching row whose column is NULL makes `prior` evaluate to UNKNOWN, `NOT
UNKNOWN` is UNKNOWN, and the row is filtered out of the scan — even though the
prior delete never matched it. That drops it from `deleted_ids`, skipping its
cascade (orphaned edges) or, if it is the only match, leaving the node
undeleted. A data bug, not just a miscount.
Data: Charlie(age 35), Zoe(age NULL); Knows Zoe→Charlie. `delete Person where
age > 30` then `delete Person where name = "Zoe"`. Under the buggy `NOT`, Zoe's
scan `(name='Zoe') AND NOT (age>30)` is UNKNOWN → Zoe survives. RED at this
commit (Person count left=1, right=0).
* fix(engine): NULL-safe delete dedup — exclude only definitely-matched prior rows
Change `dedup_delete_filter` from `(base) AND NOT (prior)` to
`(base) AND ((prior) IS NOT TRUE)`. `IS NOT TRUE` keeps both FALSE and UNKNOWN
rows, so a prior predicate that evaluates to SQL UNKNOWN (a NULL in a referenced
column) no longer drops a row this statement legitimately matches — only rows a
prior predicate matched as definitely TRUE are excluded from the count/scan. The
distinct-count semantics are unchanged for non-NULL data.
Turns the red NULL-dedup test green (Zoe deleted, her edge cascaded), and the
overlapping-dedup + writes/end_to_end/validators/maintenance/recovery/
composite_flow/consistency suites stay green.
* docs(engine): note dedup_delete_filter's load-bearing dependency on D₂
Self-review follow-up: the overlapping-delete dedup assumes the committed
snapshot is invariant across a query's statements, which holds only because D₂
forbids mixing writes with deletes (so a delete-touched table has no pending
writes). Make that dependency explicit at the function so a future D₂ relaxation
is forced to revisit the dedup. Comment-only.
* Preserve staged write commit metadata
