mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-09 01:35:18 +02:00
docs: split user and developer docs (#93)
This commit is contained in:
parent
e8d49559c4
commit
60eee78465
39 changed files with 499 additions and 445 deletions
311
docs/dev/architecture.md
Normal file
311
docs/dev/architecture.md
Normal file
|
|
@ -0,0 +1,311 @@
|
|||
# Architecture
|
||||
|
||||
OmniGraph is a typed property-graph engine built as a coordination layer over many Lance datasets, with Git-style branches and commits across the whole graph, multi-modal querying (vector + FTS + BM25 + RRF + graph traversal) in one runtime, an HTTP server with Cedar policy, and a CLI driven by a single `omnigraph.yaml`.
|
||||
|
||||
## Reading guide
|
||||
|
||||
Three views, increasing zoom:
|
||||
|
||||
1. **System context** — what OmniGraph is and what it touches.
|
||||
2. **Layer view** — the eight-layer stack inside one OmniGraph process.
|
||||
3. **Component zoom-ins** — what's inside each layer.
|
||||
|
||||
For runtime flows (read query, mutation), see [`docs/dev/execution.md`](execution.md). For the on-disk layout of a repo, see [`docs/user/storage.md`](../user/storage.md).
|
||||
|
||||
L1 (orange in the diagrams) is what we inherit from Lance; L2 (blue) is what OmniGraph adds. The L1/L2 framing is also called out in prose at the bottom of this doc.
|
||||
|
||||
## System context
|
||||
|
||||
```mermaid
|
||||
flowchart LR
|
||||
classDef external fill:#fef3e8,stroke:#c46900,color:#000
|
||||
classDef omnigraph fill:#e8f4fd,stroke:#1e6aa8,color:#000
|
||||
classDef store fill:#f0f0f0,stroke:#555,color:#000
|
||||
|
||||
cli[CLI users]:::external
|
||||
http[HTTP clients<br/>and SDKs]:::external
|
||||
agents[Agents]:::external
|
||||
embed[Embedding providers<br/>OpenAI / Gemini]:::external
|
||||
|
||||
og[OmniGraph<br/>kernel]:::omnigraph
|
||||
|
||||
cedar[Cedar policy<br/>engine]:::external
|
||||
s3[Object store<br/>local FS / S3 / RustFS]:::store
|
||||
|
||||
cli --> og
|
||||
http --> og
|
||||
agents --> og
|
||||
og --> embed
|
||||
og --> cedar
|
||||
og --> s3
|
||||
```
|
||||
|
||||
OmniGraph runs as a single process (one binary, multiple crates). External dependencies are the embedding APIs (called during ingest and at query-time normalization), Cedar (called for every privileged action), and an object store (everything OmniGraph persists lands here).
|
||||
|
||||
## Layer view
|
||||
|
||||
Inside the OmniGraph process, work flows through these layers:
|
||||
|
||||
```mermaid
|
||||
flowchart TB
|
||||
classDef l2 fill:#e8f4fd,stroke:#1e6aa8,color:#000
|
||||
classDef l1 fill:#fef3e8,stroke:#c46900,color:#000
|
||||
|
||||
subgraph CLIs[CLI and HTTP server]
|
||||
cli[omnigraph CLI]:::l2
|
||||
srv[omnigraph-server<br/>Axum + Cedar]:::l2
|
||||
end
|
||||
|
||||
subgraph compiler[omnigraph-compiler]
|
||||
front[parse → AST → typecheck → catalog → IR]:::l2
|
||||
end
|
||||
|
||||
subgraph engine[omnigraph engine]
|
||||
plan[exec query and mutation]:::l2
|
||||
gi[graph index CSR/CSC<br/>RuntimeCache LRU 8]:::l2
|
||||
coord[coordinator<br/>ManifestRepo · CommitGraph]:::l2
|
||||
end
|
||||
|
||||
subgraph storage[storage trait — wraps Lance]
|
||||
ts[table_store · storage.rs<br/>direct lance::Dataset today]:::l2
|
||||
end
|
||||
|
||||
subgraph lance_layer[Lance 4.x — substrate]
|
||||
lance[per-dataset versions, fragments<br/>BTREE · Inverted FTS · IVF/HNSW vector<br/>merge_insert · compact_files · cleanup_old_versions]:::l1
|
||||
end
|
||||
|
||||
subgraph object_store[Object store]
|
||||
os[local FS · S3 · RustFS · MinIO]:::l1
|
||||
end
|
||||
|
||||
CLIs -- "string + params" --> compiler
|
||||
compiler -- IROp --> engine
|
||||
engine -- "scan / write request" --> storage
|
||||
storage -- "Stream of RecordBatch" --> engine
|
||||
storage -- "Lance API calls" --> lance_layer
|
||||
lance_layer -- bytes --> object_store
|
||||
```
|
||||
|
||||
The storage seam is partly aspirational. `TableStorage` exists as the sealed staged-write trait, but capability/stat surfaces and full call-site migration are still roadmap. The diagram shows the intended boundary.
|
||||
|
||||
## Component zoom-ins
|
||||
|
||||
### Compiler — `omnigraph-compiler`
|
||||
|
||||
```mermaid
|
||||
flowchart LR
|
||||
classDef l2 fill:#e8f4fd,stroke:#1e6aa8,color:#000
|
||||
|
||||
src[".gq source"]:::l2
|
||||
p[parser Pest<br/>query.pest · schema.pest]:::l2
|
||||
ast[AST<br/>QueryDecl · Mutation · Schema]:::l2
|
||||
cat[catalog<br/>NodeType · EdgeType · Interface]:::l2
|
||||
tc[typecheck<br/>typecheck_query]:::l2
|
||||
low[lower<br/>lower_query]:::l2
|
||||
ir[IROp pipeline<br/>NodeScan · Expand · Filter · AntiJoin]:::l2
|
||||
|
||||
src --> p --> ast --> tc
|
||||
cat --> tc
|
||||
tc --> low --> ir
|
||||
```
|
||||
|
||||
The compiler crate has zero Lance dependency. It owns the schema language, the query language, and the AST → IR lowering.
|
||||
|
||||
Code paths:
|
||||
|
||||
- Parser: `crates/omnigraph-compiler/src/query/parser.rs`, `crates/omnigraph-compiler/src/query/query.pest`
|
||||
- Typecheck: `crates/omnigraph-compiler/src/query/typecheck.rs:83` (`typecheck_query`)
|
||||
- Lower: `crates/omnigraph-compiler/src/ir/lower.rs:11` (`lower_query`)
|
||||
- Catalog: `crates/omnigraph-compiler/src/catalog/`
|
||||
|
||||
### Engine — `omnigraph` crate
|
||||
|
||||
```mermaid
|
||||
flowchart TB
|
||||
classDef l2 fill:#e8f4fd,stroke:#1e6aa8,color:#000
|
||||
|
||||
subgraph exec[exec module]
|
||||
eq[query · execute_query<br/>query.rs:347]:::l2
|
||||
em[mutation · mutate<br/>mutation.rs:511]:::l2
|
||||
ld[loader · ingest<br/>loader/mod.rs:74]:::l2
|
||||
end
|
||||
|
||||
subgraph state[graph state]
|
||||
coord[GraphCoordinator]:::l2
|
||||
mr[ManifestRepo<br/>db/manifest.rs]:::l2
|
||||
cg[CommitGraph<br/>_graph_commits.lance]:::l2
|
||||
stg[MutationStaging<br/>per-query in-memory accumulator<br/>exec/staging.rs]:::l2
|
||||
end
|
||||
|
||||
subgraph idx[graph index]
|
||||
gi[GraphIndex<br/>CSR/CSC built per query]:::l2
|
||||
rc[RuntimeCache LRU=8]:::l2
|
||||
end
|
||||
|
||||
subgraph io[Lance I/O]
|
||||
ts[table_store]:::l2
|
||||
st[storage adapter<br/>storage.rs]:::l2
|
||||
end
|
||||
|
||||
eq --> gi
|
||||
eq --> ts
|
||||
em --> stg
|
||||
em --> ts
|
||||
ld --> stg
|
||||
ld --> ts
|
||||
eq --> mr
|
||||
em --> mr
|
||||
coord --> mr
|
||||
coord --> cg
|
||||
ts --> st
|
||||
```
|
||||
|
||||
The engine binds the compiler IR to Lance. It owns multi-dataset coordination, the graph topology index, the per-query staging accumulator, and the snapshot/manifest read path.
|
||||
|
||||
Code paths:
|
||||
|
||||
- Read entry: `Omnigraph::query` at `crates/omnigraph/src/exec/query.rs:7`
|
||||
- Mutation entry: `Omnigraph::mutate` at `crates/omnigraph/src/exec/mutation.rs:511`
|
||||
- Manifest commit: `ManifestRepo::commit` at `crates/omnigraph/src/db/manifest.rs:280`
|
||||
- Graph index: `crates/omnigraph/src/graph_index/`
|
||||
- Loader: `Omnigraph::ingest` at `crates/omnigraph/src/loader/mod.rs:74`
|
||||
|
||||
### Mutation atomicity — in-memory accumulator (MR-794)
|
||||
|
||||
Inserts and updates inside `mutate_as` and the bulk loader's
|
||||
Append/Merge modes go through `MutationStaging`
|
||||
([`crates/omnigraph/src/exec/staging.rs`](../../crates/omnigraph/src/exec/staging.rs)),
|
||||
a per-query in-memory accumulator. No Lance HEAD advance happens during
|
||||
op execution; one `stage_*` + `commit_staged` per touched table runs
|
||||
at end-of-query, then the publisher commits the manifest atomically.
|
||||
|
||||
```
|
||||
op-1 (insert/update) → push RecordBatch → MutationStaging.pending[table]
|
||||
op-2 (insert/update) → read committed via Lance + pending via DataFusion
|
||||
MemTable (read-your-writes) → push batch
|
||||
op-N → push batch
|
||||
─── end of query ───────────────────────────────────────
|
||||
finalize: per pending table:
|
||||
concat batches → stage_append OR stage_merge_insert → commit_staged
|
||||
publisher: ManifestBatchPublisher::publish (one cross-table CAS)
|
||||
```
|
||||
|
||||
A failed op leaves Lance HEAD untouched on the staged tables: the next
|
||||
mutation proceeds normally with no drift to reconcile. Concrete
|
||||
contracts:
|
||||
|
||||
- `D₂` parse-time rule: a query is either insert/update-only or
|
||||
delete-only. Mixed → reject. Deletes still inline-commit (Lance
|
||||
4.0.0 has no public two-phase delete); D₂ keeps the inline path safe.
|
||||
- `LoadMode::Overwrite` keeps the inline-commit path
|
||||
(truncate-then-append doesn't fit the staged shape; overwrite has no
|
||||
in-flight read-your-writes requirement).
|
||||
- Read sites consume `TableStore::scan_with_pending`, which Lance-scans
|
||||
the committed snapshot at the captured `expected_version` and unions
|
||||
with a DataFusion `MemTable` over the pending batches.
|
||||
|
||||
This pattern realizes read-your-writes within a multi-statement mutation
|
||||
and keeps failure scope bounded for inserts/updates by construction at
|
||||
the writer layer. See [docs/dev/invariants.md](invariants.md) and
|
||||
[docs/dev/runs.md](runs.md) for the publisher CAS contract this builds on.
|
||||
|
||||
### Storage trait — today vs. roadmap
|
||||
|
||||
```mermaid
|
||||
flowchart LR
|
||||
classDef now fill:#e8f4fd,stroke:#1e6aa8,color:#000
|
||||
classDef future fill:#fff,stroke:#888,stroke-dasharray:5 5,color:#444
|
||||
|
||||
subgraph today[Today]
|
||||
d1[table_store<br/>opens lance::Dataset directly]:::now
|
||||
d2[storage.rs<br/>S3 / file URI plumbing]:::now
|
||||
end
|
||||
|
||||
subgraph roadmap[Roadmap - storage capabilities]
|
||||
t[trait Dataset<br/>schema · stats · placement<br/>capabilities · scan · write]:::future
|
||||
impl1[LanceStorage]:::future
|
||||
impl2[future test impl]:::future
|
||||
end
|
||||
|
||||
today -.-> roadmap
|
||||
t --> impl1
|
||||
t --> impl2
|
||||
```
|
||||
|
||||
The staged-write trait exists today as `TableStorage`, implemented by `TableStore`. Full engine migration plus capability and statistics surfaces remain roadmap, so the planner cannot yet reason about all pushdown opportunities through a documented trait surface.
|
||||
|
||||
### Index lifecycle — today vs. roadmap
|
||||
|
||||
```mermaid
|
||||
flowchart LR
|
||||
classDef now fill:#e8f4fd,stroke:#1e6aa8,color:#000
|
||||
classDef future fill:#fff,stroke:#888,stroke-dasharray:5 5,color:#444
|
||||
|
||||
subgraph today[Today]
|
||||
ei[ensure_indices<br/>omnigraph.rs:445]:::now
|
||||
manual[called manually<br/>or from optimize]:::now
|
||||
end
|
||||
|
||||
subgraph roadmap[Roadmap - manifest reconciler]
|
||||
rec[Reconciler<br/>observes manifest]:::future
|
||||
diff[coverage diff<br/>fragments − fragment_bitmap]:::future
|
||||
wp[worker pool<br/>builds index segments]:::future
|
||||
end
|
||||
|
||||
manual --> ei
|
||||
today -.-> roadmap
|
||||
rec --> diff --> wp
|
||||
```
|
||||
|
||||
Today, indexes are built explicitly via `ensure_indices`. Reads degrade gracefully when index coverage is partial — Lance's scanner unions indexed and scan paths automatically. The roadmap reconciler observes manifest state and converges coverage in the background.
|
||||
|
||||
### Server / CLI
|
||||
|
||||
```mermaid
|
||||
flowchart LR
|
||||
classDef l2 fill:#e8f4fd,stroke:#1e6aa8,color:#000
|
||||
|
||||
cli[omnigraph CLI<br/>command families]:::l2
|
||||
srv_in[Axum HTTP<br/>REST + OpenAPI]:::l2
|
||||
auth[Bearer auth<br/>SHA-256 hashed tokens]:::l2
|
||||
pol[Cedar policy gate<br/>per request]:::l2
|
||||
wl[WorkloadController<br/>per-actor admission]:::l2
|
||||
eng[engine API<br/>Arc<Omnigraph>]:::l2
|
||||
wq[WriteQueueManager<br/>per-(table, branch)]:::l2
|
||||
|
||||
cli -.-> eng
|
||||
srv_in --> auth --> pol --> wl --> eng
|
||||
eng --> wq
|
||||
```
|
||||
|
||||
The server applies Cedar policy at the HTTP boundary today. The roadmap, called out in [docs/dev/invariants.md](invariants.md) as a known gap, is to push policy into the planner as predicates. After Cedar, mutating handlers go through `WorkloadController` (per-actor admission cap + byte budget; PR 2 / MR-686) before reaching the engine. The engine itself holds an `Arc<WriteQueueManager>` so concurrent mutations on the same `(table, branch)` serialize at the queue, while disjoint keys run in parallel — see [docs/user/server.md](../user/server.md) "Per-actor admission control" and [docs/dev/runs.md](runs.md). The CLI bypasses the HTTP layer (and admission) and calls the engine API directly.
|
||||
|
||||
Code paths:
|
||||
|
||||
- Server entry: `crates/omnigraph-server/src/lib.rs`
|
||||
- Auth: `crates/omnigraph-server/src/auth.rs`
|
||||
- Policy: `crates/omnigraph-server/src/policy.rs`
|
||||
- CLI: `crates/omnigraph-cli/src/main.rs`
|
||||
|
||||
## L1 / L2 framing
|
||||
|
||||
Throughout the docs, capabilities are split into:
|
||||
|
||||
- **L1 — Inherited from Lance**: what OmniGraph gets "for free" by sitting on top of the Lance dataset format (columnar Arrow storage, per-dataset versions and branches, index types, `merge_insert`, `compact_files` / `cleanup_old_versions`).
|
||||
- **L2 — Added by OmniGraph**: typing (schema language), graph semantics, multi-dataset coordination via `__manifest`, graph-level branches and commits, the `.gq` query language and IR, the topology index, the HTTP server, Cedar policy, the CLI.
|
||||
|
||||
## Concurrency model
|
||||
|
||||
- **MVCC**: every Lance write bumps a per-dataset version; the OmniGraph manifest version coordinates which sub-table versions are visible together.
|
||||
- **Snapshot isolation**: a query holds one `Snapshot` for its lifetime; concurrent writes don't leak in.
|
||||
- **Cross-branch isolation**: copy-on-write means readers and writers on different branches don't block each other.
|
||||
- **Per-query staging**: `mutate_as` and `load` (Append/Merge) accumulate insert/update batches in an in-memory `MutationStaging`; one `stage_*` + `commit_staged` per touched table runs at end-of-query, then the publisher commits the manifest atomically. A mid-query failure leaves Lance HEAD untouched on staged tables. (MR-794; pre-v0.4.0 used a `__run__<id>` staging branch + Run state machine, removed in MR-771.)
|
||||
- **Schema-apply lock**: `__schema_apply_lock__` system branch serializes schema migrations.
|
||||
- **Fail-points** (`failpoints` cargo feature): `failpoints::maybe_fail("operation.step")?` in `branch_create`, publish, etc., for deterministic failure injection in tests.
|
||||
|
||||
## Workspace crates
|
||||
|
||||
- `omnigraph-compiler` — schema and query grammars, catalog, IR, lowering, type checker, lint, migration planner, OpenAI-style embedding client.
|
||||
- `omnigraph` (engine, published as `omnigraph-engine` on crates.io since v0.2.2) — the Lance-backed runtime: manifest, commit graph, snapshot, exec (incl. per-query `MutationStaging` accumulator), merge, loader, Gemini embedding client.
|
||||
- `omnigraph-cli` — the `omnigraph` binary.
|
||||
- `omnigraph-server` — the `omnigraph-server` binary (Axum HTTP server).
|
||||
83
docs/dev/branch-protection.md
Normal file
83
docs/dev/branch-protection.md
Normal file
|
|
@ -0,0 +1,83 @@
|
|||
# Branch protection on `main`
|
||||
|
||||
`main` is gated by a declarative branch-protection policy. The source of truth is `.github/branch-protection.json`; the apply mechanism is `scripts/apply-branch-protection.sh`. Re-running the script with a changed JSON is idempotent.
|
||||
|
||||
This page explains what the policy says and how to change it.
|
||||
|
||||
## Current policy
|
||||
|
||||
| Setting | Value | Why |
|
||||
|---|---|---|
|
||||
| **Required status checks (strict)** | `Classify Changes`, `Check AGENTS.md Links`, `Test Workspace`, `Test omnigraph-server --features aws`, `CODEOWNERS / drift`, `CODEOWNERS / noedit` | Every PR must pass workspace tests, AGENTS.md link integrity, and the CODEOWNERS hygiene checks. `strict: true` requires the branch to be up-to-date with `main` before merge. |
|
||||
| **Required approving reviews** | `1` | At least one reviewer. With a 2-person team, going higher would block all merges when one person is unavailable. |
|
||||
| **Require code-owner reviews** | `true` | The reviewer must be a code owner per `.github/CODEOWNERS`. This is what makes the codeowners chassis enforced. |
|
||||
| **Dismiss stale reviews on new commits** | `true` | A push after approval invalidates the prior review. Prevents the "approve, then sneak in unreviewed changes" pattern. |
|
||||
| **Require linear history** | `true` | No merge commits — squash or rebase only. Matches recent practice. |
|
||||
| **Disallow force pushes** | `true` | No history rewrites on `main`. |
|
||||
| **Disallow branch deletions** | `true` | `main` cannot be deleted. |
|
||||
| **Required conversation resolution** | `true` | All review comment threads must be resolved before merge. |
|
||||
| **Enforce on admins** | `true` | Even repo admins go through the gates. The point is no bypasses. |
|
||||
| **Required signed commits** | not yet | Not enabled. Would lock out maintainers until everyone enrolls GPG/SSH commit signing. Tracked as a follow-up. |
|
||||
|
||||
## How to apply
|
||||
|
||||
Run from the repo root:
|
||||
|
||||
```bash
|
||||
./scripts/apply-branch-protection.sh
|
||||
```
|
||||
|
||||
The script reads `.github/branch-protection.json`, strips the human-readable `_comment` field (the GitHub API rejects unknown keys), and PUTs to `repos/ModernRelay/omnigraph/branches/main/protection`.
|
||||
|
||||
Requires `gh` authenticated with a token that has admin permissions on the repo.
|
||||
|
||||
To preview without applying:
|
||||
|
||||
```bash
|
||||
DRY_RUN=1 ./scripts/apply-branch-protection.sh
|
||||
```
|
||||
|
||||
## How to change the policy
|
||||
|
||||
1. Edit `.github/branch-protection.json`.
|
||||
2. Open a PR. The JSON change goes through normal review.
|
||||
3. After the PR merges, an admin runs `./scripts/apply-branch-protection.sh` to push the new policy to GitHub.
|
||||
|
||||
The script is **not run automatically** by CI. Branch-protection changes are admin actions that should be applied deliberately — a CI-driven automatic apply would mean any merged PR could rewrite protection rules, which defeats the purpose. The script's existence makes the apply reproducible; the admin's manual invocation is the audit point.
|
||||
|
||||
## How to read the current GitHub state
|
||||
|
||||
```bash
|
||||
gh api repos/ModernRelay/omnigraph/branches/main/protection
|
||||
```
|
||||
|
||||
Outputs the live policy. Compare against `.github/branch-protection.json` to detect drift.
|
||||
|
||||
## Why declared as code
|
||||
|
||||
- **Audit trail**: `git log .github/branch-protection.json` shows every change with a reviewable diff and a merge commit.
|
||||
- **Disaster recovery**: if branch protection is accidentally removed or weakened via the UI, the JSON is the canonical recovery point.
|
||||
- **Consistency**: pairs with `.github/codeowners-roles.yml` (the CODEOWNERS source of truth). Repo policy lives in the repo.
|
||||
|
||||
## What this gates
|
||||
|
||||
After branch protection is applied, every PR targeting `main` must:
|
||||
|
||||
1. Pass all listed status checks.
|
||||
2. Be up-to-date with `main` (rebase or merge-from-main).
|
||||
3. Have at least one approving review from a code owner for the touched paths.
|
||||
4. Have all review conversations resolved.
|
||||
5. Be squash- or rebase-merged (no merge commits).
|
||||
|
||||
Even repo admins are subject to these rules.
|
||||
|
||||
## Subsequent hardening (not in this PR)
|
||||
|
||||
The branch-protection policy is the foundation. Future hardening adds:
|
||||
|
||||
- **Required signed commits** (`required_signatures: true`) — once maintainers enroll GPG/SSH signing.
|
||||
- **Tag protection** for `v*` tags via `repos/.../tags/protection`.
|
||||
- **Required reviewers from specific teams** for high-leverage paths (e.g., `docs/dev/invariants.md`) via CODEOWNERS tier expansion + the N-unique-approvers CI workaround.
|
||||
- **More required CI checks**: `cargo deny`, `cargo audit`, `cargo fmt --check`, `cargo clippy -D warnings`, CodeQL, secret scanning, schema-lint (MR-946).
|
||||
|
||||
See the hardening playbook for the full plan.
|
||||
10
docs/dev/ci.md
Normal file
10
docs/dev/ci.md
Normal file
|
|
@ -0,0 +1,10 @@
|
|||
# CI / Release Workflows
|
||||
|
||||
`.github/workflows/`:
|
||||
|
||||
- **ci.yml**: text-only changes skip; otherwise `cargo test --workspace --locked` on ubuntu-latest with protobuf compiler. OpenAPI-drift check that auto-commits the regenerated `openapi.json` for same-repo PRs. Also runs the AGENTS.md cross-link integrity check (`scripts/check-agents-md.sh`).
|
||||
- **AWS feature build job**: `cargo build/test -p omnigraph-server --features aws` on ubuntu-latest.
|
||||
- **RustFS S3 integration**: spins up RustFS in Docker, runs `s3_storage`, `server_opens_s3_repo_directly_and_serves_snapshot_and_read`, and `local_cli_s3_end_to_end_init_load_read_flow`.
|
||||
- **release-edge.yml**: on every push to main, retags `edge`, builds Linux/macOS-Intel/macOS-arm64 archives + sha256, publishes a rolling prerelease.
|
||||
- **release.yml**: on `v*` tags, builds the 3-platform matrix and updates the Homebrew tap (`scripts/update-homebrew-formula.sh`) by pushing the regenerated formula to `ModernRelay/homebrew-tap`.
|
||||
- **package.yml**: manual ECR image build; emits two image tags per commit (`<sha>`, `<sha>-aws`) via CodeBuild.
|
||||
37
docs/dev/codeowners.md
Normal file
37
docs/dev/codeowners.md
Normal file
|
|
@ -0,0 +1,37 @@
|
|||
# Code ownership
|
||||
|
||||
`.github/CODEOWNERS` is **generated** — not hand-edited. The source of truth is `.github/codeowners-roles.yml`, expanded by `.github/scripts/render-codeowners.py`. CI rejects drift between the two and rejects direct edits to `CODEOWNERS` that don't accompany a yml change.
|
||||
|
||||
This setup gives every role change a reviewable PR and a permanent in-repo audit trail (`git log .github/codeowners-roles.yml`).
|
||||
|
||||
## Current roles
|
||||
|
||||
| Role | Members | Scope |
|
||||
|---|---|---|
|
||||
| `engineering` | `@aaltshuler` | All code under `crates/**`, repo infrastructure, default for unmapped paths |
|
||||
| `docs` | `@aaltshuler`, `@ragnorc` | `docs/**`, README.md, AGENTS.md, CLAUDE.md, SECURITY.md |
|
||||
|
||||
GitHub treats multiple owners in a CODEOWNERS line as **"any one of them satisfies the review requirement"**. For docs, either named member can approve. To require N distinct approvers on a specific path, layer a CI check on top (not currently configured).
|
||||
|
||||
## How to change role membership or path mappings
|
||||
|
||||
1. Edit `.github/codeowners-roles.yml`.
|
||||
2. Run `python3 .github/scripts/render-codeowners.py` (requires PyYAML; `pip install pyyaml`).
|
||||
3. Commit both files in the same PR.
|
||||
|
||||
CI fails the PR if:
|
||||
- `CODEOWNERS` was edited without a corresponding yml change, or
|
||||
- The yml was changed but the rendered `CODEOWNERS` doesn't match.
|
||||
|
||||
## How to add a new role
|
||||
|
||||
1. Add a new entry to `roles:` in the yml with a `description` and `members` list.
|
||||
2. Reference the role from `paths:` (or `default:`).
|
||||
3. Regenerate + commit as above.
|
||||
|
||||
## Why a generator, not direct CODEOWNERS edits?
|
||||
|
||||
- **Audit trail**: `git log .github/codeowners-roles.yml` is the canonical record of every role change. The rendered `CODEOWNERS` is a derived artifact.
|
||||
- **Roles are first-class**: paths reference roles, not raw handles. Renaming a person or rotating a role updates one place, not every path.
|
||||
- **Future extension**: scheduled rotation (weekly on-call, quarterly leads) plugs into the same yml without changing the path mappings. Not enabled today.
|
||||
- **Consistency with the product**: omnigraph itself enforces auditable Cedar policy. The repo's code-owner policy follows the same "policy as reviewed code" pattern.
|
||||
180
docs/dev/execution.md
Normal file
180
docs/dev/execution.md
Normal file
|
|
@ -0,0 +1,180 @@
|
|||
# Query Execution, Mutations, and Loading
|
||||
|
||||
## Query execution (`exec/query.rs`)
|
||||
|
||||
Pipeline:
|
||||
|
||||
1. Parse + typecheck via `omnigraph-compiler`.
|
||||
2. Lower to IR.
|
||||
3. If `Expand` or `AntiJoin` is present, build (or fetch from `RuntimeCache`) a `GraphIndex`.
|
||||
4. Run `execute_query` against the snapshot.
|
||||
|
||||
### Read flow — sequence
|
||||
|
||||
```mermaid
|
||||
sequenceDiagram
|
||||
autonumber
|
||||
participant client as Client
|
||||
participant og as Omnigraph::query<br/>(query.rs:7)
|
||||
participant cmp as omnigraph-compiler
|
||||
participant exec as execute_query<br/>(query.rs:347)
|
||||
participant gi as GraphIndex<br/>(RuntimeCache)
|
||||
participant ts as table_store
|
||||
participant lance as Lance scanner
|
||||
|
||||
client->>og: query(target, source, name, params)
|
||||
og->>og: ensure_schema_state_valid()<br/>resolve target → snapshot
|
||||
og->>cmp: parse + typecheck_query (typecheck.rs:83)
|
||||
cmp-->>og: CheckedQuery
|
||||
og->>cmp: lower_query (lower.rs:11)
|
||||
cmp-->>og: QueryIR (pipeline of IROp)
|
||||
og->>exec: extract_search_mode + dispatch (query.rs:110)
|
||||
exec->>gi: build / fetch GraphIndex<br/>(if Expand or AntiJoin)
|
||||
gi-->>exec: CSR / CSC topology
|
||||
loop for each IROp in pipeline
|
||||
exec->>ts: scan with predicate / SIP
|
||||
ts->>lance: filter · nearest · full_text_search
|
||||
lance-->>ts: Stream of RecordBatch
|
||||
ts-->>exec: RecordBatch stream
|
||||
exec->>exec: factorize · expand · fuse · project
|
||||
end
|
||||
exec-->>og: QueryResult (RecordBatches)
|
||||
og-->>client: serialized result
|
||||
```
|
||||
|
||||
**Code paths:**
|
||||
|
||||
- Entry: `Omnigraph::query` at `crates/omnigraph/src/exec/query.rs:7`
|
||||
- Search-mode extraction: `extract_search_mode` at `crates/omnigraph/src/exec/query.rs:110`
|
||||
- Pipeline runner: `execute_query` at `crates/omnigraph/src/exec/query.rs:347`
|
||||
- RRF fan-out: `execute_rrf_query` at `crates/omnigraph/src/exec/query.rs:393`
|
||||
- Per-source-row BFS: `execute_expand` at `crates/omnigraph/src/exec/query.rs:675`
|
||||
- Lance scan + pushdown: `execute_node_scan` at `crates/omnigraph/src/exec/query.rs:1027`
|
||||
- Filter → SQL pushdown: `build_lance_filter` at `crates/omnigraph/src/exec/query.rs:1158`
|
||||
|
||||
### Multi-modal search modes (`SearchMode`)
|
||||
|
||||
The executor recognizes three modes that may be combined in a single query:
|
||||
|
||||
- **`nearest`** — vector ANN (uses Lance vector index; `LIMIT` required).
|
||||
- **`bm25`** — BM25 over an inverted index.
|
||||
- **`rrf`** — Reciprocal Rank Fusion of two rankings, with k (default 60).
|
||||
|
||||
Hybrid example: `order { rrf(nearest($d.embedding, $q), bm25($d.body, $q_text)) desc } limit 20`.
|
||||
|
||||
### Joins / set operations
|
||||
|
||||
- Joins are implicit: MATCH bindings + traversals are implemented as scans + CSR/CSC lookups.
|
||||
- `not { … }` lowers to an `AntiJoin` over the inner pipeline.
|
||||
|
||||
### Scoped reads
|
||||
|
||||
- `query(target, source, name, params)` — at any branch or snapshot.
|
||||
- `run_query_at(version, …)` — direct historical query at a manifest version.
|
||||
|
||||
### Concurrency
|
||||
|
||||
- Snapshot isolation per query: all reads inside a query use the same `Snapshot`.
|
||||
- Readers and writers on different branches don't block each other.
|
||||
|
||||
## Mutation execution (`exec/mutation.rs`)
|
||||
|
||||
Resolves expression values to literals, converts to typed Arrow arrays (`literal_to_typed_array(lit, DataType, num_rows)`), then writes via Lance's two-phase distributed-write API at end-of-query:
|
||||
|
||||
- `insert` (no `@key`, edges) → accumulate into `MutationStaging.pending` (Append mode); finalize calls `stage_append` once per touched table.
|
||||
- `insert` (`@key` node) → accumulate into `pending` (Merge mode); finalize calls `stage_merge_insert` once per touched table.
|
||||
- `update` → scan committed via Lance + pending via DataFusion `MemTable` (read-your-writes), apply assignments, accumulate into `pending` (Merge mode).
|
||||
- `delete` → still inline-commits via `delete_where` (Lance 4.0.0 has no public two-phase delete); recorded into `MutationStaging.inline_committed`.
|
||||
|
||||
**D₂ parse-time rule.** A single mutation query is either insert/update-only or delete-only. Mixed → reject before any I/O. The check fires in `enforce_no_mixed_destructive_constructive(&ir)` inside `execute_named_mutation`.
|
||||
|
||||
Multi-statement mutations are atomic at the publisher commit boundary: every insert/update batch lives in memory until end-of-query, then exactly one `stage_*` + `commit_staged` runs per touched table, then `ManifestBatchPublisher::publish` commits the manifest atomically with per-table `expected_table_versions` CAS.
|
||||
|
||||
### Mutation flow — sequence
|
||||
|
||||
```mermaid
|
||||
sequenceDiagram
|
||||
autonumber
|
||||
participant client as Client
|
||||
participant og as Omnigraph::mutate_as<br/>(mutation.rs)
|
||||
participant cmp as omnigraph-compiler
|
||||
participant stg as MutationStaging<br/>(exec/staging.rs)
|
||||
participant ts as table_store
|
||||
participant lance as Lance dataset
|
||||
participant pub as ManifestBatchPublisher
|
||||
|
||||
client->>og: mutate_as(branch, source, name, params, actor_id)
|
||||
og->>cmp: parse + typecheck + lower_mutation_query
|
||||
cmp-->>og: MutationIR
|
||||
og->>og: enforce_no_mixed_destructive_constructive (D₂)
|
||||
loop for each mutation op
|
||||
og->>og: resolve literals + build batch
|
||||
alt insert / update (accumulate)
|
||||
og->>ts: open dataset @ pre-write version (first touch)
|
||||
og->>stg: ensure_path + append_batch (PendingMode)
|
||||
opt update — scan committed + pending
|
||||
og->>ts: scan_with_pending (Lance + DataFusion MemTable union)
|
||||
ts-->>og: matched batches
|
||||
end
|
||||
else delete (inline-commit, D₂ keeps separate)
|
||||
og->>ts: delete_where (advances Lance HEAD)
|
||||
og->>stg: record_inline (SubTableUpdate)
|
||||
end
|
||||
end
|
||||
og->>stg: finalize(db, branch)
|
||||
loop per pending table
|
||||
stg->>ts: stage_append OR stage_merge_insert (one per table)
|
||||
ts-->>stg: StagedWrite (transaction + fragments)
|
||||
stg->>ts: commit_staged (advances Lance HEAD)
|
||||
ts-->>stg: new Dataset
|
||||
end
|
||||
stg-->>og: (updates: Vec<SubTableUpdate>, expected_versions)
|
||||
og->>pub: commit_updates_on_branch_with_expected
|
||||
pub->>pub: publisher CAS (cross-table OCC on __manifest)
|
||||
pub-->>og: new manifest version
|
||||
og-->>client: MutationResult
|
||||
```
|
||||
|
||||
**Code paths:**
|
||||
|
||||
- Entry: `Omnigraph::mutate_as` at `crates/omnigraph/src/exec/mutation.rs`
|
||||
- Per-mutation orchestration: `mutate_with_current_actor` at `crates/omnigraph/src/exec/mutation.rs`
|
||||
- D₂ check: `enforce_no_mixed_destructive_constructive` (in the same file)
|
||||
- Per-op execution: `execute_insert`, `execute_update`, `execute_delete_node`, `execute_delete_edge`
|
||||
- Pending-aware reads: `TableStore::scan_with_pending` / `count_rows_with_pending` at `crates/omnigraph/src/table_store.rs`
|
||||
- Edge cardinality with pending: `validate_edge_cardinality_with_pending` at `crates/omnigraph/src/exec/mutation.rs`
|
||||
- Per-query accumulator: `crates/omnigraph/src/exec/staging.rs` (`MutationStaging`, `PendingTable`, `PendingMode`, `finalize`)
|
||||
- End-of-query Lance commit: `TableStore::stage_append`, `stage_merge_insert`, `commit_staged` at `crates/omnigraph/src/table_store.rs`
|
||||
- Manifest commit primitive: `commit_updates_on_branch_with_expected` at `crates/omnigraph/src/db/omnigraph/table_ops.rs`
|
||||
|
||||
Atomicity guarantee for multi-statement mutations: a mid-query failure leaves Lance HEAD untouched on staged tables (no inline commit happened during op execution), so the next mutation proceeds normally with no `ExpectedVersionMismatch`. The publisher CAS at the very end either succeeds (manifest advances atomically across all touched sub-tables) or fails with a typed `ManifestConflictDetails::ExpectedVersionMismatch` (no partial publish). See [docs/dev/invariants.md](invariants.md) and [docs/dev/runs.md](runs.md).
|
||||
|
||||
## Bulk loader (`loader/mod.rs`)
|
||||
|
||||
- **JSONL only** in v1, with two record shapes:
|
||||
- Node: `{"type":"NodeType", "data":{…}}`
|
||||
- Edge: `{"edge":"EdgeType", "from":"src_id", "to":"dst_id", "data":{…}}`
|
||||
- Lines starting with `//` are treated as comments.
|
||||
- Schema validation on every row (typecheck, required props, blob base64 decoding).
|
||||
- Edge endpoint resolution by node `@key`.
|
||||
|
||||
## Load modes (`LoadMode`)
|
||||
|
||||
| Mode | Semantics | Path (post-MR-794) |
|
||||
|---|---|---|
|
||||
| `Overwrite` | Replace all data in the target tables on the branch | Inline-commit per type, then publisher CAS at end-of-load. Truncate-then-append doesn't fit the staged shape; documented residual. |
|
||||
| `Append` | Strict insert; duplicates error | In-memory `MutationStaging` accumulator; one `stage_append` + `commit_staged` per touched table at end-of-load; publisher CAS. |
|
||||
| `Merge` | Upsert by `id` (`merge_insert`) | Same accumulator; one `stage_merge_insert` per touched table at end-of-load (Merge mode dedupes by `id`, last-write-wins); publisher CAS. |
|
||||
|
||||
For Append/Merge, a mid-load failure (RI / cardinality violation, validation error) leaves Lance HEAD untouched on the staged tables — the next load on the same tables proceeds normally with no `ExpectedVersionMismatch`. For Overwrite, a mid-load failure can still leave Lance HEAD on a partially-truncated table; the next overwrite replaces it.
|
||||
|
||||
## `load` vs `ingest`
|
||||
|
||||
- `load(branch, data, mode)` — direct load to a branch (single publisher commit per call).
|
||||
- `ingest(branch, from, data, mode)` — branch-creating wrapper: if `branch` doesn't exist, fork it from `from` (default `main`) via `branch_create_from`, then call `load(branch, data, mode)`.
|
||||
- Returns `IngestResult { branch, base_branch, branch_created, mode, tables[] }`.
|
||||
- `ingest_as(actor_id)` records the actor on the resulting commit.
|
||||
|
||||
## Embeddings during load
|
||||
|
||||
If a node type has `@embed` properties, the loader calls the engine embedding client (Gemini, RETRIEVAL_DOCUMENT) per row to populate the vector column. See [embeddings.md](../user/embeddings.md).
|
||||
58
docs/dev/index.md
Normal file
58
docs/dev/index.md
Normal file
|
|
@ -0,0 +1,58 @@
|
|||
# Developer Docs
|
||||
|
||||
**Audience:** contributors, maintainers, and coding agents
|
||||
|
||||
This is the contributor-facing entry point. These docs explain architecture,
|
||||
invariants, implementation contracts, test ownership, and upstream Lance
|
||||
constraints. User-facing behavior should still be documented through
|
||||
[docs/user/index.md](../user/index.md) and the relevant public reference docs.
|
||||
|
||||
## Required For Every Non-Trivial Change
|
||||
|
||||
| Need | Read |
|
||||
|---|---|
|
||||
| Architectural rules, known gaps, deny-list | [invariants.md](invariants.md) |
|
||||
| Upstream Lance source-of-truth index | [lance.md](lance.md) |
|
||||
| Existing test coverage and test placement | [testing.md](testing.md) |
|
||||
|
||||
## Architecture And Storage
|
||||
|
||||
| Area | Read |
|
||||
|---|---|
|
||||
| System structure, L1/L2 framing, component diagrams | [architecture.md](architecture.md) |
|
||||
| On-disk layout, manifest schema, URI behavior | [storage.md](../user/storage.md) |
|
||||
| Direct-publish writes, D2, staged writes, recovery sidecars | [runs.md](runs.md) |
|
||||
| Query execution, mutation execution, loader flow | [execution.md](execution.md) |
|
||||
| Index lifecycle and graph topology indexes | [indexes.md](../user/indexes.md) |
|
||||
| Branch and commit internals | [branches-commits.md](../user/branches-commits.md) |
|
||||
| Three-way merge implementation and conflicts | [merge.md](merge.md) |
|
||||
| Diff/change-feed implementation | [changes.md](../user/changes.md) |
|
||||
| Branch protection policy | [branch-protection.md](branch-protection.md) |
|
||||
| CODEOWNERS source of truth | [codeowners.md](codeowners.md) |
|
||||
|
||||
## Language, Runtime, And Boundaries
|
||||
|
||||
| Area | Read |
|
||||
|---|---|
|
||||
| Schema grammar, catalog, migration planner | [schema-language.md](../user/schema-language.md) |
|
||||
| Query grammar, IR, lints, mutation restrictions | [query-language.md](../user/query-language.md) |
|
||||
| Embedding client and `@embed` integration | [embeddings.md](../user/embeddings.md) |
|
||||
| Cedar policy surface and server gating | [policy.md](../user/policy.md) |
|
||||
| Server auth, OpenAPI, endpoint handlers | [server.md](../user/server.md) |
|
||||
| Error taxonomy and serialization | [errors.md](../user/errors.md) |
|
||||
| Constants and tunables | [constants.md](../user/constants.md) |
|
||||
| Transaction model public contract | [transactions.md](../user/transactions.md) |
|
||||
|
||||
## Project Operations
|
||||
|
||||
| Area | Read |
|
||||
|---|---|
|
||||
| CI and release workflows | [ci.md](ci.md) |
|
||||
| Install and deployment packaging | [install.md](../user/install.md), [deployment.md](../user/deployment.md) |
|
||||
| Release history | [releases/](../releases/) |
|
||||
|
||||
## Boundary
|
||||
|
||||
Developer docs may mention implementation details, stale gaps, upstream Lance
|
||||
blockers, and review rules. User docs should not require that context unless
|
||||
the detail changes the public contract.
|
||||
206
docs/dev/invariants.md
Normal file
206
docs/dev/invariants.md
Normal file
|
|
@ -0,0 +1,206 @@
|
|||
# Architectural Invariants
|
||||
|
||||
**Type:** standing review checklist
|
||||
**Status:** living document
|
||||
**Audience:** anyone proposing, reviewing, or implementing an OmniGraph change
|
||||
|
||||
This file is intentionally short. It records the rules that should be in
|
||||
working memory for every non-trivial change. Detailed mechanics live in the
|
||||
area docs linked below.
|
||||
|
||||
Use it this way:
|
||||
|
||||
- Review the change against **Hard Invariants** and the **Deny-list**.
|
||||
- If code and docs disagree, either fix the code or add/update a **Known Gap**.
|
||||
- Keep implementation ledgers, roadmap detail, and historical MR notes in the
|
||||
per-area docs. This file is the filter, not the encyclopedia.
|
||||
|
||||
## Hard Invariants
|
||||
|
||||
1. **Respect the substrate.** Lance owns columnar storage, per-dataset
|
||||
versioning, fragments, branches, compaction, cleanup, and index primitives.
|
||||
DataFusion should own relational execution where it fits. Do not add custom
|
||||
WALs, transaction managers, buffer pools, page formats, or local clones of
|
||||
substrate behavior. Read [lance.md](lance.md) before guessing.
|
||||
|
||||
2. **Graph visibility is manifest-atomic.** Lance commits are per dataset.
|
||||
OmniGraph's graph-level atomicity comes from publishing one manifest update
|
||||
for the whole graph, guarded by expected table versions and sidecar recovery.
|
||||
No write path may make a subset of touched node/edge tables visible as a
|
||||
graph commit.
|
||||
|
||||
3. **A query reads one snapshot.** Query execution captures a manifest snapshot
|
||||
for its lifetime. Do not re-read branch head mid-query to discover newer
|
||||
table versions.
|
||||
|
||||
4. **Mutations publish at one boundary.** A `mutate_as` or `load` operation
|
||||
accumulates constructive writes, commits each touched table at the end, then
|
||||
publishes one manifest update. Do not commit per statement. Delete-only
|
||||
queries are the documented inline residual; the parse-time D2 rule prevents
|
||||
mixing deletes with insert/update until Lance exposes two-phase delete.
|
||||
Read [runs.md](runs.md) and [execution.md](execution.md).
|
||||
|
||||
5. **Recovery is part of the commit protocol.** Writers that can advance Lance
|
||||
HEAD before manifest publish must write `__recovery/{ulid}.json` sidecars.
|
||||
`Omnigraph::open` in read-write mode runs the all-or-nothing sweep, and
|
||||
`refresh` runs roll-forward-only recovery for long-lived processes. Do not
|
||||
add a new writer kind without sidecar coverage or an explicit proof that no
|
||||
Lance HEAD can move before manifest publish.
|
||||
|
||||
6. **Strong consistency is the default.** Reads are snapshot-isolated, writes
|
||||
are durable before acknowledgement, and branch reads observe the current
|
||||
committed graph state. Any eventual-consistency mode must be explicit,
|
||||
read-only, auditable, and non-default.
|
||||
|
||||
7. **Indexes are derived state.** Reads must see the correct result for the
|
||||
branch they read even when index coverage is partial. Expensive index work
|
||||
should converge from manifest state instead of extending the critical write
|
||||
path. Scalar staged index builds and vector inline residuals are documented
|
||||
in [runs.md](runs.md) and [indexes.md](../user/indexes.md).
|
||||
|
||||
8. **Schema identity survives renames.** Accepted schema identity must remain
|
||||
stable across type and property renames. Rename support belongs in migration
|
||||
planning, not in "drop and recreate" behavior. See the known gap below.
|
||||
|
||||
9. **Schema/data integrity failures are loud.** Type errors, required-field
|
||||
misses, invalid edge endpoints, cardinality violations, and unsupported
|
||||
mixed mutation modes fail before a graph commit is published. The system must
|
||||
not invent placeholder nodes or silently weaken integrity.
|
||||
|
||||
10. **Query semantics are first-class IR concepts.** Search modes, mutations,
|
||||
polymorphism, traversal, retrieval scores, imports, and policy predicates
|
||||
belong in typed AST/IR/planner structures. Do not smuggle semantics through
|
||||
strings, side tables, global state, or transport-specific flags.
|
||||
|
||||
11. **Transport/auth stay at the boundary.** Kernel crates should not depend on
|
||||
HTTP, OpenAPI, bearer-token parsing, or future transport protocols. The
|
||||
server resolves bearer tokens to actors; clients cannot set actor identity
|
||||
directly.
|
||||
|
||||
12. **Bearer-token plaintext is not retained.** Server startup hashes bearer
|
||||
tokens, authentication uses constant-time comparison, and request handling
|
||||
carries only the resolved actor identity and hash-derived match state.
|
||||
|
||||
13. **Operational failures are bounded and observable.** Timeout, memory, OOM,
|
||||
partial result, recovery, and conflict paths must fail loudly or degrade in
|
||||
a documented way. If a metric affects plan choice or operator behavior, it
|
||||
must be exposed through the relevant trait or observability surface.
|
||||
|
||||
14. **Tests match the boundary being changed.** Prefer extending the existing
|
||||
test that owns the area. Planner changes need planner-level coverage,
|
||||
storage changes need storage/recovery coverage, and end-to-end tests are not
|
||||
a substitute for missing lower-level assertions. Read [testing.md](testing.md)
|
||||
before adding tests.
|
||||
|
||||
## Current Truth Matrix
|
||||
|
||||
| Area | Current state | Source |
|
||||
|---|---|---|
|
||||
| Multi-table commit | Manifest CAS plus recovery sidecars; not a single Lance primitive | [runs.md](runs.md), [architecture.md](architecture.md) |
|
||||
| Constructive mutations | In-memory `MutationStaging`, one end-of-query table commit per touched table, then one manifest publish | [runs.md](runs.md), [execution.md](execution.md) |
|
||||
| Deletes | Inline-commit residual; delete-only queries allowed, mixed insert/update/delete rejected by D2 | [query-language.md](../user/query-language.md), [runs.md](runs.md) |
|
||||
| Schema validation | Type checks, required fields, defaults, edge endpoint checks, and edge cardinality are enforced on write paths | [schema-language.md](../user/schema-language.md), [execution.md](execution.md) |
|
||||
| Unique constraints | Intra-batch and write-path checks exist; full cross-version uniqueness is still a gap | [schema-language.md](../user/schema-language.md) |
|
||||
| Storage trait | `TableStorage` exists as the sealed staged-write surface; full call-site migration and capability/stat surfaces are incomplete | [runs.md](runs.md), [architecture.md](architecture.md) |
|
||||
| Index lifecycle | `ensure_indices` is explicit today; reconciler-based convergence is roadmap | [indexes.md](../user/indexes.md), [maintenance.md](../user/maintenance.md) |
|
||||
| Traversal IDs | Runtime still builds `TypeIndex`; Lance stable row-id based graph IDs are roadmap | [architecture.md](architecture.md), [query-language.md](../user/query-language.md) |
|
||||
| Auth | Bearer token hashing and server-side actor resolution are implemented at the HTTP boundary | [server.md](../user/server.md), [policy.md](../user/policy.md) |
|
||||
| Tests | Tempdir-backed Lance tests are the current substrate; there is no `MemStorage` test backend | [testing.md](testing.md) |
|
||||
|
||||
## Known Gaps
|
||||
|
||||
Do not hide these behind invariant wording. Either move them forward or keep
|
||||
them explicit.
|
||||
|
||||
- **Rename-stable schema identity:** the invariant is that accepted IDs survive
|
||||
renames. The current compiler still derives type IDs from `kind:name`; this
|
||||
must be fixed before relying on renamed IDs across accepted schemas.
|
||||
- **Storage abstraction:** `TableStorage` is present, sealed, and canonical for
|
||||
staged writes, but older inherent `TableStore` call sites and inline residuals
|
||||
remain. New write paths should use the staged shape unless a documented Lance
|
||||
blocker applies.
|
||||
- **Deletes and vector indexes:** `delete_where` and vector index creation still
|
||||
advance Lance HEAD inline because the required public Lance APIs are missing.
|
||||
Keep D2 and recovery coverage in place until those residuals are removed.
|
||||
- **Planner capability/stat surfaces:** cost-aware planning, complete
|
||||
capability advertisement, and explain-with-cost are roadmap. Do not describe
|
||||
them as implemented.
|
||||
- **Traversal execution:** current multi-hop execution still uses `TypeIndex`,
|
||||
ad-hoc ID filtering, and eager materialization in places. Stable row IDs, SIP,
|
||||
and factorization are target patterns, not current fact.
|
||||
- **Retrieval ranks:** hybrid search works, but rank/score are not yet carried
|
||||
everywhere as ordinary columns through the plan.
|
||||
- **Policy pushdown and `Source`:** Cedar enforcement is at the HTTP boundary
|
||||
today, and imports are still loader-shaped. Planner predicates and a unified
|
||||
`Source` operator are roadmap.
|
||||
- **Resource bounds:** some operations still lack enforced per-query memory or
|
||||
time budgets. New long-running work should add explicit bounds rather than
|
||||
widening the gap.
|
||||
|
||||
## Deny-list
|
||||
|
||||
If a proposal fits one of these, the burden is on the proposer to prove why the
|
||||
case is exceptional.
|
||||
|
||||
- Custom WAL, transaction manager, buffer pool, page format, or storage engine.
|
||||
- Per-table graph publishing outside the manifest publisher.
|
||||
- Re-reading current branch head during a query instead of using the captured
|
||||
snapshot.
|
||||
- New write paths that can advance Lance HEAD before manifest publish without a
|
||||
recovery sidecar.
|
||||
- Cross-query `BEGIN`/`COMMIT` transactions in the OSS engine. Use branches and
|
||||
merges for multi-query workflows.
|
||||
- Acknowledging writes before durable Lance and manifest persistence.
|
||||
- Silent fallback to eventual consistency, partial results, or dropped rows.
|
||||
- State that drifts from Lance or the manifest when it can be derived.
|
||||
- Job queues for manifest-derivable state where a reconciler is the right shape.
|
||||
- Synchronous inline vector/FTS index rebuilds on the query commit path, except
|
||||
for documented Lance API residuals.
|
||||
- Side-channels for query semantics: hidden globals, magic strings, transport
|
||||
flags, or out-of-band metadata.
|
||||
- Cost-blind plan choice when statistics are available or required.
|
||||
- Hidden statistics for behavior that affects planning or operator choice.
|
||||
- Hash-map iteration order in result ordering, plan choice, or migration output.
|
||||
- String-flattened SQL/filter generation when a structured pushdown API is
|
||||
available.
|
||||
- Eager multi-hop cross-product materialization when factorization fits.
|
||||
- Ad-hoc `IN`-list filtering where SIP or another structured selectivity path
|
||||
fits.
|
||||
- Discarding retrieval score/rank before fusion or projection decisions.
|
||||
- Auto-creating placeholder nodes for orphan edges.
|
||||
- Wire-protocol-specific code in compiler or engine crates.
|
||||
- Cloud-only correctness fixes or forks of the OSS engine for correctness.
|
||||
- Mutating immutable substrate state in place, including Lance fragments or
|
||||
index segments.
|
||||
- Shipping observable behavior as if it were not part of the contract. Output
|
||||
ordering, error text, timestamp precision, defaults, and latency profiles all
|
||||
become dependencies once exposed.
|
||||
|
||||
## Review Checklist
|
||||
|
||||
Use this as yes/no/NA for any non-trivial design or PR:
|
||||
|
||||
- Does it respect Lance/DataFusion instead of rebuilding them?
|
||||
- Does it preserve manifest-atomic graph visibility?
|
||||
- Does every query keep one snapshot for its lifetime?
|
||||
- Do mutations publish once at the commit boundary?
|
||||
- Can every Lance-HEAD-before-manifest gap recover all-or-nothing?
|
||||
- Are schema and edge integrity checks strict by default?
|
||||
- Are query semantics represented in AST/IR/planner structures?
|
||||
- Are transport, auth, and policy boundaries preserved?
|
||||
- Are failures bounded, typed, and observable?
|
||||
- Are result ordering and plan choices deterministic within a snapshot?
|
||||
- Are stats/capabilities exposed when behavior depends on them?
|
||||
- Are existing known gaps left no worse and documented if touched?
|
||||
- Does the test live at the same boundary as the change?
|
||||
- Does the change avoid every deny-list pattern, or justify the exception?
|
||||
|
||||
## Maintenance Policy
|
||||
|
||||
Update this file when an invariant changes, a known gap opens or closes, or a
|
||||
new review anti-pattern deserves deny-list treatment. Prefer stable headings
|
||||
over numbered sections so other docs can link here without churn.
|
||||
|
||||
Removing or relaxing a hard invariant requires the same review process as code.
|
||||
Adding a known gap is acceptable when it makes reality explicit; leaving stale
|
||||
claims is not.
|
||||
168
docs/dev/lance.md
Normal file
168
docs/dev/lance.md
Normal file
|
|
@ -0,0 +1,168 @@
|
|||
# Lance Docs Index (for OmniGraph agents)
|
||||
|
||||
OmniGraph sits on top of Lance. Many problems — index lifecycle, branching, transactions, fragments, compaction, vector/FTS internals — are answered upstream in Lance's docs, not in this repo.
|
||||
|
||||
This file is the curated entry point. **When you hit a Lance-shaped problem, find the matching topic below and fetch the listed URL(s) before guessing.** Don't grep our codebase for behavior that is documented authoritatively in Lance.
|
||||
|
||||
Base URL: `https://lance.org`. **Fetch the FULL page content, not summaries** — use `npx mdrip <url>` (or `npx mdrip --max-chars 200000 <url>` for very long pages). Tools that summarize pages (like Claude's `WebFetch`) routinely drop load-bearing details — defaults, `pub(crate)` blockers, sub-specs hidden behind navigation hubs. If `npx mdrip` is unavailable, fall back to `curl <url> | pandoc -f html -t markdown` or paste the rendered page text manually; **never act on a summarized fetch alone**. Keep this index curated to relevant material — the upstream sitemap has hundreds of URLs (notably the Namespace REST API model surface, Spark/Trino/Databricks integrations) that we don't use.
|
||||
|
||||
> **Substrate boundary check.** Before fetching, recall [docs/dev/invariants.md](invariants.md): if Lance already does the thing, we don't reimplement it. The most common reason to read these docs is to confirm a substrate behavior, not to learn what to clone.
|
||||
|
||||
## Quick-start (read these once per project)
|
||||
|
||||
| Read when | URL |
|
||||
|---|---|
|
||||
| Onboarding to Lance — concepts in 10 min | https://lance.org/quickstart/ |
|
||||
| Onboarding to vector search | https://lance.org/quickstart/vector-search/ |
|
||||
| Onboarding to full-text search | https://lance.org/quickstart/full-text-search/ |
|
||||
| Onboarding to versioning / time travel | https://lance.org/quickstart/versioning/ |
|
||||
| Lance's own AGENTS.md (its agent guide) | https://lance.org/format/AGENTS/ |
|
||||
|
||||
## By problem domain
|
||||
|
||||
### Storage format & file layout
|
||||
|
||||
Touching `db/manifest`, fragment lifecycle, dataset reconstruction, or anything that reads/writes raw Lance state.
|
||||
|
||||
| Topic | URL |
|
||||
|---|---|
|
||||
| Lance file format overview | https://lance.org/format/ |
|
||||
| File-level format spec | https://lance.org/format/file/ |
|
||||
| File encoding | https://lance.org/format/file/encoding/ |
|
||||
| File-level versioning | https://lance.org/format/file/versioning/ |
|
||||
| Table layout (fragments, manifest) | https://lance.org/format/table/layout/ |
|
||||
| Table schema metadata | https://lance.org/format/table/schema/ |
|
||||
| Table-level versioning | https://lance.org/format/table/versioning/ |
|
||||
| Transactions (commit semantics, conflict types) | https://lance.org/format/table/transaction/ |
|
||||
| MemWAL (durability story) | https://lance.org/format/table/mem_wal/ |
|
||||
| Row-ID lineage (stable row IDs) | https://lance.org/format/table/row_id_lineage/ |
|
||||
| Branches & tags (Lance native) | https://lance.org/format/table/branch_tag/ |
|
||||
|
||||
### Branching / tags / time travel
|
||||
|
||||
Touching graph-level branches, snapshots, run isolation, the commit graph.
|
||||
|
||||
| Topic | URL |
|
||||
|---|---|
|
||||
| Branch & tag format | https://lance.org/format/table/branch_tag/ |
|
||||
| Tags & branches operational guide | https://lance.org/guide/tags_and_branches/ |
|
||||
| Versioning quick-start | https://lance.org/quickstart/versioning/ |
|
||||
| Table-level versioning spec | https://lance.org/format/table/versioning/ |
|
||||
|
||||
### Indexes
|
||||
|
||||
Adding/changing index types, fixing coverage, debugging FTS or vector recall, designing the reconciler.
|
||||
|
||||
| Topic | URL |
|
||||
|---|---|
|
||||
| Index spec overview | https://lance.org/format/table/index/ |
|
||||
| BTREE scalar index | https://lance.org/format/table/index/scalar/btree/ |
|
||||
| Bitmap scalar index | https://lance.org/format/table/index/scalar/bitmap/ |
|
||||
| Bloom-filter scalar index | https://lance.org/format/table/index/scalar/bloom_filter/ |
|
||||
| Label-list scalar index | https://lance.org/format/table/index/scalar/label_list/ |
|
||||
| Zone-map scalar index | https://lance.org/format/table/index/scalar/zonemap/ |
|
||||
| R-Tree scalar index (spatial) | https://lance.org/format/table/index/scalar/rtree/ |
|
||||
| Full-text search (FTS) index | https://lance.org/format/table/index/scalar/fts/ |
|
||||
| N-gram scalar index | https://lance.org/format/table/index/scalar/ngram/ |
|
||||
| Vector index | https://lance.org/format/table/index/vector/ |
|
||||
| Fragment-reuse system index | https://lance.org/format/table/index/system/frag_reuse/ |
|
||||
| MemWAL system index | https://lance.org/format/table/index/system/mem_wal/ |
|
||||
| HNSW Rust example | https://lance.org/examples/rust/hnsw/ |
|
||||
| Distributed indexing | https://lance.org/guide/distributed_indexing/ |
|
||||
| Tokenizer (FTS, n-gram) | https://lance.org/guide/tokenizer/ |
|
||||
|
||||
### Reads & writes
|
||||
|
||||
Touching the bulk loader, mutation execution, `merge_insert`, `WriteMode` selection.
|
||||
|
||||
| Topic | URL |
|
||||
|---|---|
|
||||
| Read-and-write guide | https://lance.org/guide/read_and_write/ |
|
||||
| Distributed write | https://lance.org/guide/distributed_write/ |
|
||||
| Rust example: write & read a dataset | https://lance.org/examples/rust/write_read_dataset/ |
|
||||
|
||||
### Schema evolution
|
||||
|
||||
Touching `apply_schema`, the migration planner, additive evolution.
|
||||
|
||||
| Topic | URL |
|
||||
|---|---|
|
||||
| Data-evolution guide | https://lance.org/guide/data_evolution/ |
|
||||
| Migration guide | https://lance.org/guide/migration/ |
|
||||
|
||||
### Object store / S3
|
||||
|
||||
Touching `storage.rs`, S3-compatible backends (RustFS, MinIO), env vars.
|
||||
|
||||
| Topic | URL |
|
||||
|---|---|
|
||||
| Object-store guide | https://lance.org/guide/object_store/ |
|
||||
|
||||
### Data types
|
||||
|
||||
Touching schema-language scalar mappings, blob columns, JSON, list columns.
|
||||
|
||||
| Topic | URL |
|
||||
|---|---|
|
||||
| Data types overview | https://lance.org/guide/data_types/ |
|
||||
| Arrays / list types | https://lance.org/guide/arrays/ |
|
||||
| Blobs (LargeBinary) | https://lance.org/guide/blob/ |
|
||||
| JSON | https://lance.org/guide/json/ |
|
||||
|
||||
### Performance & tuning
|
||||
|
||||
Optimizing scans, fragment counts, cache behavior, memory pool sizing.
|
||||
|
||||
| Topic | URL |
|
||||
|---|---|
|
||||
| Performance guide | https://lance.org/guide/performance/ |
|
||||
|
||||
### Compaction & cleanup
|
||||
|
||||
Touching `omnigraph optimize` / `cleanup`, the underlying `compact_files` / `cleanup_old_versions`.
|
||||
|
||||
| Topic | URL |
|
||||
|---|---|
|
||||
| Read-and-write guide (covers `compact_files`, `cleanup_old_versions`) | https://lance.org/guide/read_and_write/ |
|
||||
| Performance (compaction tradeoffs) | https://lance.org/guide/performance/ |
|
||||
| Fragment-reuse index | https://lance.org/format/table/index/system/frag_reuse/ |
|
||||
|
||||
### DataFusion integration
|
||||
|
||||
The runtime substrate that may carry our query execution. See [docs/dev/invariants.md](invariants.md): we don't rebuild relational machinery.
|
||||
|
||||
| Topic | URL |
|
||||
|---|---|
|
||||
| DataFusion integration | https://lance.org/integrations/datafusion/ |
|
||||
|
||||
### SDK reference
|
||||
|
||||
Looking up a specific Rust API (signature, return type, error variant).
|
||||
|
||||
| Topic | URL |
|
||||
|---|---|
|
||||
| SDK docs landing | https://lance.org/sdk_docs/ |
|
||||
|
||||
## What's not in this index (and why)
|
||||
|
||||
- **Namespace REST API model surface** (`/format/namespace/client/operations/models/...`) — hundreds of REST schema docs for the Lance Namespace catalog API. Omnigraph does not run a Lance Namespace server, so these are not reachable from our problem space.
|
||||
- **Spark / Trino / Databricks / Dataproc / Hive / Glue / Polaris / Iceberg / Unity / OneLake / Gravitino integrations** — not part of OmniGraph's deployment surface.
|
||||
- **Python / TF / PyTorch / Hugging Face / Ray integrations** — OmniGraph is Rust-only; Python notebooks aren't relevant.
|
||||
- **Community / governance / release / voting / PMC pages** — meta, not technical.
|
||||
|
||||
If a future need pulls one of these into scope, add a row to the matching domain section above and link it from `AGENTS.md`'s topic index.
|
||||
|
||||
## Maintenance
|
||||
|
||||
When Lance ships a major release that changes any of the above (file format bump, new index type, transaction semantics change, new branching primitive), refresh this index in the same change as the omnigraph upgrade. Stale Lance pointers are worse than no pointers.
|
||||
|
||||
### Last alignment audit: 2026-05-02 (Lance 4.0.1 upstream; omnigraph pinned at 4.0.0)
|
||||
|
||||
A full read-through of every index page above was performed in the MR-793 cycle. Findings (no code changes required for PR #70):
|
||||
|
||||
- The MemWAL "three sub-pages" (Overview / Details / Implementation) turned out to be **anchor sections on the single existing page** at `https://lance.org/format/table/mem_wal/` — not separate URLs. Fetched in full via `npx mdrip`. Findings: MemWAL is opt-in (requires an unenforced primary key + explicit shard config; omnigraph doesn't use it), operates intra-table (LSM-tree for streaming writes into one Lance table), and does NOT overlap with MR-847's cross-table manifest-vs-Lance-HEAD recovery problem. MR-847's design is unaffected.
|
||||
- The distributed-indexing guide names Python APIs (`commit_existing_index_segments`, `merge_existing_index_segments`); the Rust analogues exist via `CreateIndexBuilder::execute_uncommitted` for scalar indices but **`build_index_metadata_from_segments` is `pub(crate)`** and blocks vector-index two-phase commits from outside the lance crate. Filed [lance-format/lance#6666](https://github.com/lance-format/lance/issues/6666) as a companion to [#6658](https://github.com/lance-format/lance/issues/6658).
|
||||
- "Stable Row ID for Index" is documented as **experimental** in lance-4.0.x. Our datasets enable stable row IDs at the dataset level (`WriteParams::enable_stable_row_ids = true`); confirming whether our created indices opt into stable-row-id mode is a follow-up worth doing before MR-848 (index reconciler) lands.
|
||||
- Fragment Reuse Index (FRI) is documented as one of three compaction strategies. omnigraph currently uses option 2 (immediate index rewrite at compaction time, via `omnigraph optimize`'s post-compaction rebuild). Adopting FRI is the explicit option for compaction-friendly index updates; relevant to MR-848.
|
||||
|
||||
Bump this date stanza on the next alignment pass.
|
||||
30
docs/dev/merge.md
Normal file
30
docs/dev/merge.md
Normal file
|
|
@ -0,0 +1,30 @@
|
|||
# Merging (three-way) and Conflicts
|
||||
|
||||
`exec/merge.rs`.
|
||||
|
||||
## Strategy
|
||||
|
||||
Ordered, row-by-row cursor merge:
|
||||
|
||||
- `OrderedTableCursor` scans each table sorted by `id` and supports peek/pop matching.
|
||||
- `StagedTableWriter` buffers `MERGE_STAGE_BATCH_ROWS = 8192` rows into a temp Lance dataset (`OMNIGRAPH_MERGE_STAGING_DIR`).
|
||||
- The merge runs per sub-table; results are published as one atomic manifest update.
|
||||
|
||||
## Outcome enum
|
||||
|
||||
`MergeOutcome { AlreadyUpToDate | FastForward | Merged }`
|
||||
|
||||
## Conflict types (`error.rs`)
|
||||
|
||||
```
|
||||
MergeConflictKind:
|
||||
DivergentInsert // same id inserted on both branches
|
||||
DivergentUpdate // updated differently on both branches
|
||||
DeleteVsUpdate // one side deletes, other updates
|
||||
OrphanEdge // edge references a node deleted by the other side
|
||||
UniqueViolation
|
||||
CardinalityViolation
|
||||
ValueConstraintViolation
|
||||
```
|
||||
|
||||
Returned as `OmniError::MergeConflicts(Vec<MergeConflict { table_key, row_id?, kind, message }>)`. The HTTP server surfaces this as a 409 with structured `merge_conflicts[]` (top 3 + "+N more").
|
||||
277
docs/dev/runs.md
Normal file
277
docs/dev/runs.md
Normal file
|
|
@ -0,0 +1,277 @@
|
|||
# Runs — REMOVED (MR-771)
|
||||
|
||||
The Run state machine and `__run__<id>` staging branches were removed in
|
||||
MR-771. `mutate_as` and `load` now write **directly to the target table**
|
||||
and call `ManifestBatchPublisher::publish` once at the end with
|
||||
`expected_table_versions` (the per-table manifest versions captured before
|
||||
the first write). Cross-table OCC is enforced inside the publisher; the
|
||||
publisher's row-level CAS on `__manifest` is the single fence.
|
||||
|
||||
## What this means in practice
|
||||
|
||||
- No `RunRecord`, no `_graph_runs.lance`, no `_graph_run_actors.lance`.
|
||||
- No `omnigraph run *` CLI subcommands and no `/runs/*` HTTP endpoints.
|
||||
- No `__run__<id>` staging branches. (Legacy on-disk artifacts from
|
||||
pre-MR-771 repos are inert; MR-770 sweeps them in production.)
|
||||
- Cancelled mutation futures leave **no graph-level state** — only orphaned
|
||||
Lance fragments, which the existing `omnigraph cleanup` pipe reclaims.
|
||||
|
||||
## Read-your-writes within a multi-statement mutation
|
||||
|
||||
A `.gq` query with multiple ops (e.g. `insert Person … insert Knows …`)
|
||||
must observe earlier ops' writes when validating later ops (referential
|
||||
integrity, edge cardinality). After MR-794 step 2+ this is implemented
|
||||
via an in-memory `MutationStaging` accumulator in
|
||||
[`crates/omnigraph/src/exec/staging.rs`](../../crates/omnigraph/src/exec/staging.rs),
|
||||
shared by both `mutate_as` and the bulk loader:
|
||||
|
||||
- On the first touch of each table, the pre-write manifest version is
|
||||
captured into `expected_versions[table_key]` (the publisher's CAS
|
||||
fence at end-of-query).
|
||||
- Each insert/update op pushes a `RecordBatch` into the per-table
|
||||
pending accumulator. Lance HEAD does **not** advance during op
|
||||
execution.
|
||||
- Read sites (validation, predicate matching for `update`) consume
|
||||
`TableStore::scan_with_pending`, which scans committed via Lance
|
||||
and applies the same SQL filter to the pending batches via DataFusion
|
||||
`MemTable`. Same-query writes are visible to subsequent reads.
|
||||
- At end-of-query, `MutationStaging::finalize` issues exactly one
|
||||
`stage_*` + `commit_staged` per touched table (concatenating
|
||||
accumulated batches; merge-mode dedupes by `id`, last-write-wins),
|
||||
and the publisher publishes the manifest atomically across all
|
||||
touched sub-tables. Cross-table conflicts surface as
|
||||
`ManifestConflictDetails::ExpectedVersionMismatch`.
|
||||
- **Deletes still inline-commit.** Lance's `Dataset::delete` is not
|
||||
exposed as a two-phase op in 4.0.0; deletes go through `delete_where`
|
||||
immediately and record their post-write state in
|
||||
`MutationStaging.inline_committed`. The parse-time D₂ rule (below)
|
||||
prevents inserts/updates from coexisting with deletes in one query,
|
||||
so the inline path is safe for delete-only mutations.
|
||||
|
||||
This upholds the manifest-atomic mutation and read-your-writes invariants
|
||||
tracked in [docs/dev/invariants.md](invariants.md).
|
||||
|
||||
### D₂ — parse-time mixed-mode rejection
|
||||
|
||||
A single mutation query is either insert/update-only or delete-only.
|
||||
Mixed → rejected at parse time with a clear error directing the user to
|
||||
split the query. Reason: mixing creates ordering hazards
|
||||
(insert→delete on the same row would silently no-op because the staged
|
||||
insert isn't visible to delete; cascading deletes of just-inserted
|
||||
edges break referential integrity). Until Lance exposes a two-phase
|
||||
delete API, the parse-time rejection keeps both paths atomic and
|
||||
correct. Tracked: MR-793, plus a Lance-upstream ticket.
|
||||
|
||||
### MR-793 status (storage trait two-phase invariant) — partial
|
||||
|
||||
MR-793 hoists the staged-write pattern into a `TableStorage` trait
|
||||
surface with sealed-trait enforcement and opaque `SnapshotHandle` /
|
||||
`StagedHandle` types — see `crates/omnigraph/src/storage_layer.rs`.
|
||||
The trait is the canonical surface for new engine code; existing call
|
||||
sites still use the inherent `TableStore` methods (mechanical migration
|
||||
deferred to a follow-up cycle — tracked).
|
||||
|
||||
Three writers have been migrated onto staged primitives:
|
||||
|
||||
* **`ensure_indices`** (`db/omnigraph/table_ops.rs::build_indices_on_dataset_for_catalog`)
|
||||
— scalar indices (BTree, Inverted) now use `stage_create_*_index` +
|
||||
`commit_staged`. Vector indices stay inline (residual — Lance
|
||||
`build_index_metadata_from_segments` is `pub(crate)` in 4.0.0;
|
||||
companion ticket to lance-format/lance#6658 needed).
|
||||
* **`branch_merge::publish_rewritten_merge_table`**
|
||||
(`exec/merge.rs`) — merge_insert now uses `stage_merge_insert` +
|
||||
`commit_staged`. Deletes stay inline (Lance #6658 residual).
|
||||
* **`schema_apply` rewritten_tables** (`db/omnigraph/schema_apply.rs`)
|
||||
— non-empty rewrites use `stage_overwrite` + `commit_staged`.
|
||||
Empty-batch rewrites stay inline (Lance `InsertBuilder::execute_uncommitted`
|
||||
rejects empty data; the empty case is rare and bounded by the
|
||||
schema-apply lock branch).
|
||||
|
||||
A defense-in-depth integration test (`tests/forbidden_apis.rs`) walks
|
||||
engine source and fails if non-allow-listed code calls Lance's
|
||||
inline-commit APIs directly. The trait surface itself is the primary
|
||||
enforcement (sealed + only-callable-via-trait once call sites land);
|
||||
the grep test catches type-system bypass attempts.
|
||||
|
||||
The "finalize → publisher residual" described below applies equally to
|
||||
the migrated writers — Lance has no multi-dataset atomic commit
|
||||
primitive, so the per-table commit_staged → manifest publish gap is
|
||||
the same drift class. Closing it requires either upstream Lance
|
||||
multi-dataset commit OR the omnigraph-side recovery-on-open reconciler
|
||||
described in `.context/mr-793-design.md` §15 (deferred to MR-795).
|
||||
|
||||
### Inline-commit method residuals on `TableStorage` (MR-793 acceptance §1 option b)
|
||||
|
||||
MR-793's acceptance criterion §1 ("`TableStore` public API has no method that performs a manifest commit as a side effect of writing") is met **per-method** by enumerating every inline-commit method that remains on the trait surface, naming why it cannot yet be removed, and keeping the residual comment at every call site:
|
||||
|
||||
| Method on `TableStore` | Inline-commit reason | Closes when |
|
||||
|---|---|---|
|
||||
| `delete_where` | `DeleteJob` is `pub(crate)` in lance-4.0.0 — no public two-phase delete API | [lance-format/lance#6658](https://github.com/lance-format/lance/issues/6658) lands and `stage_delete` joins the trait |
|
||||
| `create_vector_index` | Vector indices take Lance's "segment commit path"; the helper `build_index_metadata_from_segments` is `pub(crate)` | [lance-format/lance#6666](https://github.com/lance-format/lance/issues/6666) lands and `stage_create_vector_index` joins the trait |
|
||||
| `append_batch` | Legacy inherent method; some engine call sites haven't migrated to `stage_append + commit_staged` yet | MR-793 Phase 1b (call-site conversion) + Phase 9 (demote to `pub(crate)`) |
|
||||
| `merge_insert_batch` / `merge_insert_batches` | Legacy inherent method | Same — Phase 1b + Phase 9 |
|
||||
| `overwrite_batch` | Legacy inherent method | Same — Phase 1b + Phase 9 |
|
||||
| `create_btree_index` (inherent) | Legacy inherent method (the migrated callers use `stage_create_btree_index` + `commit_staged`; the inherent stays for tests / un-migrated paths) | Same — Phase 1b + Phase 9 |
|
||||
| `create_inverted_index` (inherent) | Same | Same — Phase 1b + Phase 9 + index-class split (MR-848) |
|
||||
| `truncate_table` (inherent on `TableStore`) | Used by `overwrite_batch` internally | Phase 9 |
|
||||
|
||||
After **lance#6658 + lance#6666 ship + MR-793 Phase 1b + MR-793 Phase 9 all complete**, the trait surface exposes only staged-write primitives + `commit_staged`. Until then this matrix names every residual explicitly, every call site carries a one-line residual comment, and no engine code outside `table_store.rs` is permitted to reach the inline-commit Lance APIs (enforced by the `tests/forbidden_apis.rs` guard).
|
||||
|
||||
### `LoadMode::Overwrite` residual
|
||||
|
||||
The bulk loader's Append and Merge modes use the staged-write path
|
||||
described above. `LoadMode::Overwrite` keeps the legacy inline-commit
|
||||
path: truncate-then-append doesn't fit the staged shape cleanly in
|
||||
Lance 4.0.0, and overwrite has no in-flight read-your-writes
|
||||
requirement (the prior data is being wiped). A mid-overwrite failure
|
||||
can leave Lance HEAD on a partially-truncated table; the next overwrite
|
||||
will replace it. Operator-driven (rare in agent workloads); document
|
||||
permanently until Lance exposes `Operation::Overwrite { fragments }` as
|
||||
a two-phase op.
|
||||
|
||||
### Open-time recovery sweep
|
||||
|
||||
The staged-write rewire eliminates one drift class **by construction at
|
||||
the writer layer**: an op that fails before pushing to the in-memory
|
||||
accumulator (validation errors, missing endpoints, parse-time D₂
|
||||
rejection) leaves Lance HEAD untouched on every staged table. This is
|
||||
the case the `partial_failure_leaves_target_queryable_and_unblocks_next_mutation`
|
||||
test pins.
|
||||
|
||||
A second, narrower drift class — the **finalize → publisher window** —
|
||||
is closed across one open cycle by the open-time recovery sweep:
|
||||
|
||||
`MutationStaging::finalize` runs `stage_*` + `commit_staged` per touched
|
||||
table sequentially, then the publisher commits the manifest. Lance has
|
||||
no multi-dataset atomic commit, so the per-table `commit_staged` calls
|
||||
are independent operations: if commit_staged on table N+1 fails *after*
|
||||
commit_staged on tables 1..N succeeded, or if the publisher's CAS
|
||||
pre-check rejects *after* every commit_staged succeeded, tables 1..N
|
||||
are left at `Lance HEAD = manifest_pinned + 1`.
|
||||
|
||||
**Recovery protocol** (lifecycle of every staged-write writer —
|
||||
`MutationStaging::finalize`, `schema_apply::apply_schema_with_lock`,
|
||||
`branch_merge_on_current_target`, `ensure_indices_for_branch`):
|
||||
|
||||
1. **Phase A**: writer writes a sidecar JSON to
|
||||
`__recovery/{ulid}.json` BEFORE its first `commit_staged`. The
|
||||
sidecar names every `(table_key, table_path, expected_version,
|
||||
post_commit_pin)` it intends to commit + the writer kind +
|
||||
actor_id.
|
||||
2. **Phase B**: writer's per-table `commit_staged` loop runs.
|
||||
3. **Phase C**: publisher commits the manifest.
|
||||
4. **Phase D**: writer deletes the sidecar.
|
||||
|
||||
> **Phase letter convention.** Throughout the recovery code, log
|
||||
> messages, failpoint names (e.g. `branch_merge.post_phase_b_pre_manifest_commit`),
|
||||
> and the per-writer integration tests, "Phase A/B/C/D" refers
|
||||
> exclusively to the four-step lifecycle above. The per-table
|
||||
> staged-write contract (`stage_*` then `commit_staged`, two steps)
|
||||
> is referred to by those API verbs — never by phase letters — so a
|
||||
> reader of `recovery.rs`, `failpoints.rs`, or this document only
|
||||
> encounters phase letters in the per-writer context.
|
||||
|
||||
A failure between Phase A and Phase D leaves the sidecar on disk. The
|
||||
next `Omnigraph::open` (gated on `OpenMode::ReadWrite`) runs the
|
||||
recovery sweep in `crates/omnigraph/src/db/manifest/recovery.rs`:
|
||||
|
||||
- For each sidecar in `__recovery/`, compare every named table's
|
||||
Lance HEAD to the manifest pin. Classify per the all-or-nothing
|
||||
decision tree (RolledPastExpected / NoMovement / UnexpectedAtP1 /
|
||||
UnexpectedMultistep / InvariantViolation).
|
||||
- If any table is `InvariantViolation` (Lance HEAD < manifest pinned —
|
||||
should be impossible), **abort** with a loud error and leave the
|
||||
sidecar on disk for operator review.
|
||||
- Otherwise, if every table is `RolledPastExpected`, **roll forward**:
|
||||
a single `ManifestBatchPublisher::publish` call extends every pin
|
||||
atomically. `SchemaApply` sidecars are eligible only when schema-state
|
||||
recovery promoted the matching staging files in the same recovery pass;
|
||||
otherwise full open-time recovery rolls them back and refresh-time
|
||||
recovery leaves them for the next read-write open.
|
||||
- Otherwise **roll back**: per-table `Dataset::restore` to the
|
||||
manifest-pinned table version for that branch. Rollback records the
|
||||
actual restore target in the audit row's `to_version`.
|
||||
- After a successful roll-forward or roll-back, an audit row is
|
||||
recorded — `_graph_commits.lance` carries
|
||||
a commit tagged `actor_id = "omnigraph:recovery"`, and a sibling
|
||||
`_graph_commit_recoveries.lance` row carries `recovery_kind`,
|
||||
`recovery_for_actor` (the original sidecar's actor), `operation_id`,
|
||||
per-table outcomes. Operators run `omnigraph commit list --filter
|
||||
actor=omnigraph:recovery` to find recoveries.
|
||||
- Sidecar deleted as the final step.
|
||||
|
||||
Triggers for the residual: transient Lance write errors during finalize
|
||||
(object-store retry budget exhaustion, disk full); persistent publisher
|
||||
contention exceeding `PUBLISHER_RETRY_BUDGET = 5` retries.
|
||||
|
||||
**Long-running servers**: `Omnigraph::refresh` runs roll-forward-only
|
||||
recovery in-process — the common Phase B → Phase C residual closes
|
||||
without a restart. The next mutation on the same handle (after refresh)
|
||||
no longer surfaces `ExpectedVersionMismatch` for the failed table.
|
||||
Sidecars that would require a `Dataset::restore` (mixed / unexpected
|
||||
state) are deferred to the next `OpenMode::ReadWrite` open: restore is
|
||||
unsafe under concurrency because Lance's `check_restore_txn` accepts
|
||||
the restore against in-flight Append/Update/Delete commits and
|
||||
silently orphans them (pinned by
|
||||
`tests/staged_writes.rs::lance_restore_loses_to_concurrent_append_via_orphaning`).
|
||||
Continuous in-process recovery for the rollback path is the goal of a
|
||||
future background reconciler with per-(table, branch) writer-queue
|
||||
acquisition.
|
||||
|
||||
The publisher-CAS contract is unchanged: a *concurrent writer* that
|
||||
advances any of our touched tables between snapshot capture and
|
||||
publisher commit produces exactly one winner. The residual above is
|
||||
about *our* abandoned commits in the failure path, not about
|
||||
concurrency races.
|
||||
|
||||
## Conflict shape
|
||||
|
||||
Concurrent writers to the same `(table, branch)` produce exactly one
|
||||
success and one failure. The losing writer's error is
|
||||
`OmniError::Manifest` with kind `Conflict` and details
|
||||
`ManifestConflictDetails::ExpectedVersionMismatch { table_key, expected,
|
||||
actual }`. The HTTP server maps this to **409 Conflict** with body
|
||||
`{"error": "...", "code": "conflict", "manifest_conflict": { "table_key":
|
||||
"...", "expected": N, "actual": M }}` — see [docs/user/server.md](../user/server.md).
|
||||
|
||||
## Audit
|
||||
|
||||
`actor_id` lands in `_graph_commits.lance` via `record_graph_commit` (no
|
||||
intermediate run record). Audit history is queried via `omnigraph commit
|
||||
list`.
|
||||
|
||||
## Migration code
|
||||
|
||||
`db/manifest/migrations.rs` does not change. Active deletion of
|
||||
`_graph_runs.lance` belongs in MR-770 (the production sweep) — this PR
|
||||
stops *creating* run state but does not destroy legacy bytes on disk.
|
||||
|
||||
## Mid-query partial failure: closed by MR-794
|
||||
|
||||
The pre-MR-794 design had a known limitation: a multi-statement `.gq`
|
||||
mutation where op-N inline-committed a Lance fragment and op-N+1 then
|
||||
failed left the touched table at `Lance HEAD = manifest_version + 1`,
|
||||
blocking the next mutation with `ExpectedVersionMismatch`.
|
||||
|
||||
MR-794 (step 1 + step 2+) closed this for inserts/updates **by
|
||||
construction at the writer layer**: insert and update batches accumulate
|
||||
in memory; no Lance HEAD advance happens during op execution; one
|
||||
`stage_*` + `commit_staged` per touched table runs at end-of-query, and
|
||||
only after every op succeeded. A failed op leaves Lance HEAD untouched
|
||||
on the staged tables, so the next mutation proceeds normally with no
|
||||
drift to reconcile.
|
||||
|
||||
The cancellation case (future drop mid-mutation) inherits the same
|
||||
guarantee — the in-memory accumulator evaporates with the dropped task
|
||||
and no Lance write was ever issued.
|
||||
|
||||
For delete-touching mutations the legacy inline-commit shape is
|
||||
preserved (Lance has no public two-phase delete in 4.0.0) — the same
|
||||
narrow window remains. The parse-time D₂ rule prevents inserts/updates
|
||||
from coexisting with deletes in one query, so a pure-delete failure
|
||||
cannot drift any staged-table state. If a delete-only multi-table
|
||||
mutation fails mid-cascade, the same workaround as before applies
|
||||
(retry; rely on `omnigraph cleanup` once a later successful commit
|
||||
moves HEAD past the orphan version). Closing this requires Lance to
|
||||
expose `DeleteJob::execute_uncommitted`; tracked in MR-793 and a
|
||||
Lance-upstream ticket.
|
||||
115
docs/dev/testing.md
Normal file
115
docs/dev/testing.md
Normal file
|
|
@ -0,0 +1,115 @@
|
|||
# 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](invariants.md).
|
||||
|
||||
## Where tests live, per crate
|
||||
|
||||
| Crate | Path | Style |
|
||||
|---|---|---|
|
||||
| `omnigraph` (engine) | `crates/omnigraph/tests/` | Integration tests (21 files), fixture-driven, share `tests/helpers/mod.rs` |
|
||||
| `omnigraph-cli` | `crates/omnigraph-cli/tests/` | `cli.rs` (unit-ish), `system_local.rs`, `system_remote.rs`, share `tests/support/mod.rs` |
|
||||
| `omnigraph-server` | `crates/omnigraph-server/tests/` | `server.rs` (HTTP-level), `openapi.rs` (OpenAPI drift / regeneration) |
|
||||
| `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. |
|
||||
| `runs.rs` | Direct-publish writes: cancellation, concurrent-writer CAS, 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 |
|
||||
| `lifecycle.rs` | Repo 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 |
|
||||
| `search.rs` | FTS / vector / hybrid (`bm25`, `nearest`, `rrf`) |
|
||||
| `traversal.rs` | `Expand`, variable-length hops, anti-join |
|
||||
| `aggregation.rs` | `count`, `sum`, `avg`, `min`, `max` |
|
||||
| `export.rs` | NDJSON streaming export filters |
|
||||
| `s3_storage.rs` | S3-backed repo (skipped unless `OMNIGRAPH_S3_TEST_BUCKET` is set) |
|
||||
| `lance_version_columns.rs` | Per-row `_row_last_updated_at_version` behavior |
|
||||
| `validators.rs` | Schema constraint enforcement (enum, range, unique, cardinality) across JSONL, insert, update paths |
|
||||
| `maintenance.rs` | `optimize` (compaction) + `cleanup` (version GC): empty/idempotent/no-op edges, policy validation, head preservation |
|
||||
| `failpoints.rs` | Failure-injection coverage (gated on `failpoints` feature). Includes the four 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`). |
|
||||
| `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). |
|
||||
|
||||
## 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
|
||||
|
||||
- **Engine** — `crates/omnigraph/tests/helpers/mod.rs`: `init_and_load()` (bootstrap a temp repo + load standard fixture), `snapshot_main()`, `snapshot_branch()`, query/mutation runners, row collection and counting. Use these instead of hand-rolling.
|
||||
- **CLI** — `crates/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: there is **no `MemStorage` or in-memory backend** today. Tests use `tempfile::tempdir()` for local FS. If you find yourself needing one for layer isolation, that's an architectural ask — keep it explicit in [docs/dev/invariants.md](invariants.md) under known gaps.
|
||||
|
||||
## Failpoints (fault injection)
|
||||
|
||||
- Cargo feature: `failpoints = ["dep:fail", "fail/failpoints"]` (in `crates/omnigraph/Cargo.toml`).
|
||||
- Wrapper: `crates/omnigraph/src/failpoints.rs` exposes `maybe_fail("name")` and `ScopedFailPoint` for tests.
|
||||
- Call sites are inserted at sensitive transaction boundaries (branch create, graph publish commit, etc.).
|
||||
- Activated tests: `crates/omnigraph/tests/failpoints.rs`. Run with `cargo test -p omnigraph-engine --features failpoints --test failpoints`.
|
||||
|
||||
## RustFS / S3 integration
|
||||
|
||||
CI runs three S3-backed tests against a containerized RustFS server (`.github/workflows/ci.yml` → `rustfs_integration` job):
|
||||
|
||||
- `cargo test -p omnigraph-engine --test s3_storage`
|
||||
- `cargo test -p omnigraph-server --test server server_opens_s3_repo_directly_and_serves_snapshot_and_read`
|
||||
- `cargo test -p omnigraph-cli --test system_local local_cli_s3_end_to_end_init_load_read_flow`
|
||||
|
||||
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.
|
||||
|
||||
## 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-repo 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 repo 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`, `runs.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 case* → **add 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 repo. 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 repo by hand if a helper exists.
|
||||
5. **Mind the boundary.** Per [docs/dev/invariants.md](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.
|
||||
|
||||
When in doubt, re-read [docs/dev/invariants.md](invariants.md) — quality gates apply to every change.
|
||||
Loading…
Add table
Add a link
Reference in a new issue