Skip to content

Commit 4a9684b

Browse files
authored
Close RFC-022 writer surface (#353)
* Close RFC-022 writer surface * Close writer guard gateway exclusions
1 parent 88edc0e commit 4a9684b

40 files changed

Lines changed: 2491 additions & 419 deletions

AGENTS.md

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,10 +254,10 @@ omnigraph policy explain --cluster ./company-brain --graph knowledge --actor act
254254
| Per-dataset versioning + time travel || `snapshot_at_version`, `entity_at`, snapshot-pinned reads across many tables |
255255
| Per-dataset branches || **Graph-level** refs are logically atomic through authoritative `__manifest` `BranchContents`; native create/delete crash gaps are classified and reclaimed under a single-writer-process boundary; live names are path-prefix-disjoint; data-table forks are lazy; system branches are filtered |
256256
| Atomic single-dataset commits | ✅ | **Multi-table publish via three layers**, NOT a single Lance primitive: (1) per-table Lance `commit_staged` for the data write, (2) `__manifest` row-level CAS via `ManifestBatchPublisher` for cross-table ordering, (3) the open-time recovery sweep for the residual gap between (1) and (2). All three layers ship; the five migrated writers (`MutationStaging::commit_all`, `schema_apply`, `branch_merge`, `ensure_indices`, `optimize_all_tables`) write a `__recovery/{ulid}.json` sidecar before Phase B and delete it after Phase C. Under their final schema → branch → table gates, mutation/load, SchemaApply, BranchMerge, and EnsureIndices prove every existing effect target still equals its manifest pin before arming; first-touch refs or datasets are created only after the sidecar is durable. The next `Omnigraph::open` (gated on `OpenMode::ReadWrite`) runs the sweep in `db/manifest/recovery.rs`: classify, decide all-or-nothing per sidecar, roll forward via single `ManifestBatchPublisher::publish` or roll back via `Dataset::restore` followed by a manifest publish of the restored version (so both directions converge to `manifest == HEAD` — no residual drift), and record an internal audit row in `_graph_commit_recoveries.lance`. Schema-v3 Mutation/Load, schema-v4 BranchMerge, schema-v7 SchemaApply, and schema-v8 EnsureIndices roll-forward preserve the interrupted writer's fixed commit lineage and actor. SchemaApply v7 captures native main authority + accepted schema identity, pre-mints exact existing-table `Overwrite` and strict read-version-zero AddType/RenameType create identities, and confirms the complete registration/update/tombstone delta only after every table effect and schema staging file is durable. `Armed` rolls back; `EffectsConfirmed` rolls forward only under the captured token. The fixed manifest outcome lands before schema staging promotion. Rollback reclaims an exact owned first-touch path; an unregistered foreign first-touch winner is preserved but never adopted, while foreign movement on a manifest-owned table or an owned effect buried by a same-table winner fails closed. Metadata-only applies use the same protocol with an empty effect set. Query, export, graph-index, and blob reads capture their snapshot + catalog under the same process-local schema gate, so they cannot observe the manifest-before-catalog window on the applying handle. Read-only open performs no recovery writes but refuses a fixed SchemaApply manifest outcome until the matching schema identity is live. The reader remains backward-compatible with schema-v5 bridge files under their original target-hash/Phase-C-confirmation and loose-classification semantics. EnsureIndices v8 captures exact branch/schema authority, fixed original/rollback lineage, one pre-minted mixed BTREE/FTS/vector CreateIndex transaction per table, the complete confirmed pointer delta, and exact first-touch ref identity; `Armed` rolls back and `EffectsConfirmed` rolls forward only under the captured token. Schema-v6 EnsureIndices files remain readable under their original loose classification and fixed-rollback semantics, never reinterpreted as v8 ownership. Other rollback and legacy recovery commits use `omnigraph:recovery`. BranchMerge v4 also owns first-touch refs, pre-mints each table's exact ordered data-transaction chain with zero transparent conflict retries, carries pointer-only updates in its complete confirmed manifest delta, and recognizes an interrupted compensation restore on restart. There is currently no public CLI query for the recovery-audit table, and ordinary commit history is not a complete recovery enumeration. The write entry points (`load_as`, `mutate_as`, `apply_schema_as`, `branch_merge_as`) and `refresh` additionally run an in-process roll-forward-only heal, serialized against same-process live writers through the shared root-scoped gate manager, so a long-lived server converges on its next write without restart; only rollback-eligible sidecars still defer to the next read-write open (a future background reconciler's goal). These gates are process-local: exact v7/v8 ownership prevents false adoption but does not make destructive recovery safe against a still-running writer in another process; multi-process recovery and live schema capture still need a distributed fence. Engine writes route through a sealed `TableStorage` trait (`db.storage()`) exposing only `stage_*` + `commit_staged` + reads. `stage_create_indices` batches every missing BTREE, FTS, and full-table vector artifact for one table into one staged CreateIndex transaction; `InlineCommitResidual` and `storage_inline_residual()` are removed, so the surface cannot couple writing artifacts with a HEAD advance — §1 holds by construction. `delete` migrated to the staged path in MR-A (`stage_delete` via Lance 7.0 `DeleteBuilder::execute_uncommitted`, [#6658](https://github.com/lance-format/lance/issues/6658)); the beta.21 one-segment full-table vector shape now stages through the exact EnsureIndices v8 adapter, while [#6666](https://github.com/lance-format/lance/issues/6666) remains relevant only to generic multi-segment exact publication; `LoadMode::Overwrite` uses Lance `Overwrite` staged transactions. |
257-
| Compaction (`compact_files`) + reindex (`optimize_indices`) | ✅ | `omnigraph optimize` orchestrates over all node/edge tables with bounded physical concurrency and one graph visibility envelope. Under schema → main → every accepted-catalog table gate it loads one operation-local catalog/snapshot, skips uncovered HEAD drift, and plans only productive tables. One schema-v2 `SidecarKind::Optimize` sidecar pins that complete set before any table HEAD movement; each productive task runs `compact_files`, Lance `optimize_indices` (incremental coverage merge, not retrain), and declared-missing index materialization, but no task publishes independently. After all tasks settle, one maintenance-class monotonic manifest CAS publishes every still-needed pointer plus one graph lineage commit; a pointer already at or beyond the achieved version is converged and omitted. Thus two changed tables become visible together, a no-work run creates no sidecar/lineage, and any post-arm error returns `RecoveryRequired`. Full v2 recovery rolls a complete set forward in one batch or compensates a partial set before visibility. Main remains held through final physical-only `__manifest` compaction. Optimize still has legacy loose provenance (no exact transaction/authority/fixed-lineage proof), so destructive recovery retains the documented single-writer-process boundary until the exact adapter slice. It **commits even with no compaction work if index coverage is stale**; reports untrainable vector-only work as pending without pinning it; **refuses on an unrecovered graph**; **skips uncovered HEAD > manifest drift** with `DriftNeedsRepair`; and **compacts blob-bearing tables**. |
257+
| Compaction (`compact_files`) + reindex (`optimize_indices`) | ✅ | `omnigraph optimize` orchestrates over all node/edge tables with bounded physical concurrency and one graph visibility envelope. Under schema → main → every accepted-catalog table gate it loads one operation-local catalog/snapshot, skips uncovered HEAD drift, and plans only productive tables. One bounded schema-v2 `SidecarKind::Optimize` sidecar pins that complete set before any table HEAD movement; each productive task runs `compact_files`, Lance `optimize_indices` (incremental coverage merge, not retrain), and declared-missing index materialization, but no task publishes independently. After all tasks settle, one maintenance-class monotonic manifest CAS publishes every still-needed pointer plus one graph lineage commit; a pointer already at or beyond the achieved version is converged and omitted. Thus two changed tables become visible together, a no-work run creates no sidecar/lineage, and any post-arm error returns `RecoveryRequired`. Full v2 recovery rolls a complete set forward in one batch or compensates a partial set before visibility. Main remains held through final physical-only `__manifest` compaction. The v2 classifier has no exact transaction/authority/fixed-lineage proof, so destructive recovery retains the documented single-writer-process boundary. Exact provenance is deferred until Lance exposes a stable public caller-controlled maintenance transaction API and OmniGraph has distributed recovery fencing. It **commits even with no compaction work if index coverage is stale**; reports untrainable vector-only work as pending without pinning it; **refuses on an unrecovered graph**; **skips uncovered HEAD > manifest drift** with `DriftNeedsRepair`; and **compacts blob-bearing tables**. |
258258
| Repair uncovered drift || `omnigraph repair` explicitly classifies uncovered table `HEAD > manifest` drift: verified maintenance drift (`ReserveFragments`/`Rewrite`) can be published with `--confirm`; suspicious or unverifiable drift requires `--force --confirm`. Sidecar-covered crash residuals still recover automatically on open. |
259259
| Cleanup (`cleanup_old_versions`) || `omnigraph cleanup` derives requested `--keep` / `--older-than` cutoffs from each table's available versions; Lance refs plus OmniGraph's live-lazy-branch and recovery floors may retain additional versions. It fails closed on unopenable pins, recovery intent, or uncovered main-table HEAD drift |
260-
| BTREE / inverted (FTS) / vector indexes || `@index`/`@key` declares intent; the physical index is derived state that never fails a logical op. Built per column through one chokepoint (`build_indices_on_dataset_for_catalog`, type-dispatched by `node_prop_index_kind`: enum + orderable scalar → BTREE, free-text String → FTS, Vector → vector); idempotent; lazy across branches. **Schema apply and mutation/load build no indexes inline**: the latter publish only their exact data effects, leaving physical intent pending. `ensure_indices` materializes declared-but-missing indexes through one staged mixed CreateIndex transaction per table under its exact schema-v8 authority/lineage/delta protocol, while continuing to report untrainable Vector columns as pending. `optimize` separately restores fragment coverage and remains on its legacy recovery adapter. |
260+
| BTREE / inverted (FTS) / vector indexes || `@index`/`@key` declares intent; the physical index is derived state that never fails a logical op. Built per column through one chokepoint (`build_indices_on_dataset_for_catalog`, type-dispatched by `node_prop_index_kind`: enum + orderable scalar → BTREE, free-text String → FTS, Vector → vector); idempotent; lazy across branches. **Schema apply and mutation/load build no indexes inline**: the latter publish only their exact data effects, leaving physical intent pending. `ensure_indices` materializes declared-but-missing indexes through one staged mixed CreateIndex transaction per table under its exact schema-v8 authority/lineage/delta protocol, while continuing to report untrainable Vector columns as pending. `optimize` separately restores fragment coverage through its bounded schema-v2 maintenance adapter. |
261261
| `merge_insert` upsert || `LoadMode::Merge`, mutation `update`/`insert`/`delete` lowering |
262262
| Vector search || `nearest()` query op; embedding pipeline (Gemini / OpenAI clients); `@embed` in schema |
263263
| Full-text search || `search/fuzzy/match_text/bm25` query ops |
@@ -277,6 +277,19 @@ omnigraph policy explain --cluster ./company-brain --graph knowledge --actor act
277277
| Local S3 testing || run RustFS/MinIO + the `AWS_*` env; see [docs/user/deployment.md](docs/user/deployment.md)*Testing against S3 locally* |
278278
| Agent skill || `skills/omnigraph` — operational playbook for driving Omnigraph; install with `npx skills add ModernRelay/omnigraph@omnigraph` |
279279

280+
The supported SDK graph-write set is closed primarily by Rust visibility: the
281+
raw storage, handle-cache, and coordinator modules are crate-private, while a
282+
public snapshot opens tables through a read-only facade whose scan builder
283+
executes reads without exposing Lance's raw `Scanner` or physical plan. The defense-in-depth registry in
284+
`crates/omnigraph/tests/forbidden_apis.rs` classifies every public async inherent
285+
`Omnigraph` method and loader convenience, every crate-visible async coordinator
286+
method, and exact per-file occurrences of the registered durable-call shapes,
287+
including recovery. It also rejects known direct Lance open/mutation shapes
288+
outside exact gateway files. A new supported writer or durable gateway must
289+
update that registry and its protocol coverage. The source scanner is not a
290+
Rust macro-expansion or alias-analysis engine; visibility is the structural
291+
boundary.
292+
280293
---
281294

282295
## Maintenance contract for agents

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/omnigraph-cli/tests/cli_schema_config.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
44
use std::fs;
55

6-
use lance::index::DatasetIndexExt;
76
use omnigraph::db::{Omnigraph, ReadTarget};
87
use serde_json::Value;
98
use tempfile::tempdir;

crates/omnigraph-server/tests/schema_routes.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use std::sync::Arc;
66

77
use axum::body::Body;
88
use axum::http::{Method, Request, StatusCode};
9-
use lance::index::DatasetIndexExt;
109
use omnigraph::db::{Omnigraph, ReadTarget};
1110
use omnigraph::loader::LoadMode;
1211
use omnigraph_server::api::{

crates/omnigraph/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ lance-namespace-impls = { workspace = true }
6262
lance-io = { git = "https://github.com/lance-format/lance", rev = "1aec14652dcbace23ac277fa8ced35000bea0c40", features = ["test-util"] }
6363
serial_test = "3"
6464
proptest = "1"
65+
syn = { version = "2", features = ["full", "visit"] }
6566
# benches/scenarios.rs only: wait4/rusage peak-RSS + setrlimit memory caps.
6667
libc = "0.2"
6768

crates/omnigraph/src/changes/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ impl ChangeFilter {
110110
/// 1. Manifest diff — skip unchanged sub-tables
111111
/// 2. Lineage check — same branch → version-column diff; different → ID-based diff
112112
/// 3. Row-level diff
113-
pub async fn diff_snapshots(
113+
pub(crate) async fn diff_snapshots(
114114
table_store: &TableStore,
115115
from: &Snapshot,
116116
to: &Snapshot,

crates/omnigraph/src/db/graph_coordinator.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ pub(crate) struct PublishedSnapshot {
9090
pub _snapshot_id: SnapshotId,
9191
}
9292

93-
pub struct GraphCoordinator {
93+
pub(crate) struct GraphCoordinator {
9494
root_uri: String,
9595
storage: Arc<dyn StorageAdapter>,
9696
manifest: ManifestCoordinator,
@@ -99,7 +99,7 @@ pub struct GraphCoordinator {
9999
}
100100

101101
impl GraphCoordinator {
102-
pub async fn init(
102+
pub(crate) async fn init(
103103
root_uri: &str,
104104
catalog: &Catalog,
105105
storage: Arc<dyn StorageAdapter>,
@@ -234,7 +234,7 @@ impl GraphCoordinator {
234234
})
235235
}
236236

237-
pub async fn branch_create(&mut self, name: &str) -> Result<()> {
237+
pub(crate) async fn branch_create(&mut self, name: &str) -> Result<()> {
238238
let branch = normalize_branch_name(name)?
239239
.ok_or_else(|| OmniError::manifest("cannot create branch 'main'".to_string()))?;
240240

@@ -245,7 +245,7 @@ impl GraphCoordinator {
245245
self.manifest.create_branch(&branch).await
246246
}
247247

248-
pub async fn branch_delete(&mut self, name: &str) -> Result<()> {
248+
pub(crate) async fn branch_delete(&mut self, name: &str) -> Result<()> {
249249
let branch = normalize_branch_name(name)?
250250
.ok_or_else(|| OmniError::manifest("cannot delete branch 'main'".to_string()))?;
251251
if self.current_branch() == Some(branch.as_str()) {

0 commit comments

Comments
 (0)