Commit graph

568 commits

Author SHA1 Message Date
Andrew Altshuler
7540c86fa0
ci(publish): add omnigraph-api-types + omnigraph-cluster to the crates.io publish order (#260)
The publish-crates workflow's list predated two workspace crates: the shared
wire-DTO crate `omnigraph-api-types` (RFC-009) and `omnigraph-cluster`. On the
v0.7.0 tag it published compiler/policy/engine, then failed on `omnigraph-server`
("no matching package named `omnigraph-api-types`") because that dependency was
never published.

Insert both in dependency order (after omnigraph-engine, before
omnigraph-server). The workflow is idempotent (per-crate version check), so a
re-dispatch for v0.7.0 skips the three already-published crates and finishes
api-types → cluster → server → cli.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-16 05:23:55 +03:00
Andrew Altshuler
80ef9964fe
docs(releases): finalize v0.7.0 notes (#259)
Reconcile the v0.7.0 release notes with what 0.7.0 actually ships. The draft was
mid-cycle; two facts changed and a late-cycle arc was missing.

- omnigraph.yaml is REMOVED (not deprecated): drop the deprecation-window
  framing (config migrate, OMNIGRAPH_NO_LEGACY_CONFIG, OMNIGRAPH_SUPPRESS_YAML_
  DEPRECATION); the two-surface config (cluster.yaml + ~/.omnigraph/config.yaml)
  is the only config.
- Cluster-only server: the server boots only from --cluster; no single-graph
  flat-route / positional-URI / omnigraph.yaml-graphs boot. Deprecated-route Link
  headers are the sibling-relative form (<load>, not </load>).
- Add the RFC-011 tail: defaults.store, profile list/show, schema-apply refusal
  (CLI signpost + server 409), read-only aliases, the any/served/direct/control/
  local capability vocabulary, removed legacy data-plane addressing.
- New "Engine & substrate" section: Lance 6→7, indexed traversal, scalar-index/
  query-latency, index-materialization-deferred, recovery liveness, branch-fork
  self-heal, composite @unique.
- New "Embeddings (RFC-012)" section + breaking bullet: provider-independent
  client (OpenRouter default), @embed same-space validation, the default-provider
  flip (OMNIGRAPH_EMBED_PROVIDER=gemini for Gemini-direct; OMNIGRAPH_GEMINI_BASE_URL
  dropped).
- Upgrade notes: replace the false "omnigraph.yaml keeps working / config migrate"
  guidance with the manual cluster.yaml + operator-config path; add server
  --cluster and embeddings notes.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-16 05:04:49 +03:00
Andrew Altshuler
51852fdbed
Merge pull request #248 from ModernRelay/ragnorc/shaping-config-integration
RFC-012: embedding provider independence + remove dead client
2026-06-16 04:38:50 +03:00
aaltshuler
e33041c8b7 Fix aws server startup config test 2026-06-16 04:23:07 +03:00
aaltshuler
4f8c71fa23 Merge remote-tracking branch 'origin/main' into ragnorc/shaping-config-integration
# Conflicts:
#	crates/omnigraph-cluster/src/lib.rs
#	crates/omnigraph-cluster/src/serve.rs
#	crates/omnigraph-server/src/lib.rs
#	crates/omnigraph-server/src/settings.rs
#	docs/user/clusters/config.md
2026-06-16 04:13:00 +03:00
aaltshuler
16e4a833c0 Wire cluster embedding providers 2026-06-16 04:02:08 +03:00
Andrew Altshuler
b5658dc696
[codex] fix RFC-011 follow-up regressions (#258)
* fix rfc-011 follow-up regressions

* test(cli): remove served schema-apply tests obsoleted by the cluster 409

This PR disables server-side schema apply for cluster-backed serving (409 →
`omnigraph cluster apply`). Two system_local tests still drove *served* schema
apply against a spawned `--cluster` server and asserted the pre-409 behavior, so
they failed under `cargo test --workspace`:

- `local_cli_schema_apply_enforces_engine_layer_policy` — expected a per-actor
  policy `denied`/allow on the served route; the route now 409s for everyone
  before policy runs.
- `local_cli_schema_apply_rejects_stored_query_breakage_before_publish` —
  expected a served apply to reject a stored-query breakage; the route now 409s
  before any apply.

Both exercise a path the PR intentionally removed. Their surviving coverage:
the 409 itself is pinned by `schema_routes::schema_apply_route_refuses_cluster_backed_server_mode`
(asserts 409 + no mutation); stored-query-breakage-before-publish stays covered
by `schema_routes::schema_apply_route_rejects_stored_query_breakage_before_publish`
(single-mode); engine-layer schema_apply Cedar enforcement stays covered by
`policy_engine_chassis`. Remove the obsolete served versions.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(server): report the cluster-backed schema-apply 409 after the Cedar gate

The 409 ("schema apply is disabled for cluster-backed serving") fired at the top
of `server_schema_apply`, before `authorize_request`. An authenticated-but-
unauthorized actor therefore learned the server is cluster-backed (409) instead
of getting a normal 403 — leaking topology before authorization, against the
same posture that keeps `GET /graphs` default-deny.

Move the 409 below the Cedar gate so the route reports 401 → 403 → 409: an
unauthorized actor gets 403, and only an actor authorized for `schema_apply`
sees the actionable "use `omnigraph cluster apply`" 409. (An open/unauthenticated
server still 409s, as it has no topology to protect.)

Regression: `schema_apply_route_cluster_backed_denies_unauthorized_actor_before_409`
(POLICY_YAML grants no schema_apply → act-ragnor gets 403, not 409). Addresses the
bot-review finding on #258.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-16 03:11:43 +03:00
aaltshuler
8d2128438e fix(cli): quote @embed annotation values in schema show so they round-trip
`render_annotations` emitted `@embed` values unquoted — `@embed(title,
model=openai/text-embedding-3-large)`. The parser stores values via
`decode_string_literal` (quotes stripped) and `annotation_kwarg` requires a
quoted `literal`, so the rendered output did not re-parse: a `model` containing
`/` or `-` is not a valid bare token. `schema show` therefore produced schema
text that `schema apply`/lint would reject.

Re-quote the positional value and every kwarg value as string literals, so
`schema show` reproduces `@embed("title", model="openai/text-embedding-3-large")`
and round-trips. Regression: `render_annotations_quotes_values_so_embed_round_trips`
parses the rendered form back through the schema grammar.

Addresses the bot-review finding on #248.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-16 01:25:27 +03:00
aaltshuler
1498ed821e fix(embedding): from_parts honors an explicit model for the mock provider
`EmbeddingConfig::from_parts(Some("mock"), _, Some(model), _)` short-circuited to
`Self::mock()` and silently dropped the provided `model`, falling back to
`OMNIGRAPH_EMBED_MODEL`. A cluster `embeddings` profile that sets
`provider: mock, model: X` (RFC-012 Phase 5) would therefore not resolve to X —
the query-time same-space check (`exec/query.rs`) compares the recorded model
against the resolved one, so a dropped model makes that check fire on the wrong
value (a spurious mismatch, or env-dependent). Mock vectors are model-independent
so this is not silent cross-space ranking, but it breaks the contract `from_parts`
documents ("model defaults exactly as from_env does") and the cluster path that
Phase 5 wiring will feed it.

Honor an explicit `model` for mock; fall back to `mock()`'s env-based model only
when none is supplied. Regression: `from_parts_mock_honors_an_explicit_model`
(red before this change).

Addresses the bot-review finding on #248.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-16 01:14:01 +03:00
Andrew Altshuler
9513b076d2
docs(cli): fix the --help capability legend (config removed, profile added) (#256)
The `local` line of the `--help` "COMMANDS BY CAPABILITY" legend was stale: it
still listed `config` (the `config migrate` group was removed with the
omnigraph.yaml excision) and omitted `profile` (added by RFC-011 D8). Update the
list to `embed, login, logout, profile, version`.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-15 23:44:23 +03:00
Andrew Altshuler
7c092d3206
feat(cli): add read-only profile list / profile show (RFC-011 D8) (#255)
Inspect the per-operator `~/.omnigraph/config.yaml` scope profiles without
running anything:

- `profile list [--json]` — every profile with its binding (server/cluster/store)
  and default graph; marks the `$OMNIGRAPH_PROFILE`-active one. A malformed
  (zero/two-scope) profile is reported as `invalid: <reason>`, not a hard failure.
- `profile show [<name>] [--json]` — one profile's resolved scope: binding kind +
  target, the resolved endpoint (a server's URL / a cluster's root / the store
  URI), default graph, and output format. With no name, shows the active
  (`$OMNIGRAPH_PROFILE`) profile, else the flat operator defaults.

Both are `local` (Session plane) — they read operator config only, take no
addressing flags. Display reads `OperatorProfile::binding()` + the same
`servers`/`clusters` lookups the scope resolver uses (not `resolve_scope`, which
is capability-gated and can't render all three binding kinds at once), so it is
honest about what a profile binds.

Also: RFC-011 bookkeeping (Status → Accepted; D8 shipped, D11 gated on RFC #219,
D5 deferred) and drop the stale "legacy config actor (RFC-008 window)" comment in
operator.rs (the legacy actor is gone).

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-15 23:33:01 +03:00
Ragnor Comerford
6a2dfa7325
fix: self-heal manifest-unreferenced branch forks (stop wedged branches) (#231)
* chore: correct stale global-lock comments

The global Arc<RwLock<Omnigraph>> that once serialized every server write was
removed — the server holds the engine as a lockless Arc<Omnigraph> and write
methods are &self, so the per-(table_key, branch) write queues are now the
actual write-serialization mechanism (in-process only).

Correct comments that still claimed the global lock is 'still in place' /
'today', or framed the queues as MR-686 scaffolding: write_queue.rs module doc,
exec/merge.rs, db/omnigraph/schema_apply.rs, db/manifest/recovery.rs, and the
bench_concurrent_http.rs example (which also wrongly stated mutate_as is
&mut self). workload.rs is left as-is — its 'previous global RwLock' wording is
accurate history.

* test: regression for self-healing a manifest-unreferenced fork

An interrupted first-write fork (create_branch succeeded, the manifest publish
did not) leaves a fully-formed Lance branch ref the manifest never references.
The branch stays a valid manifest branch, so cleanup's reconciler never
reclaims it, and today the next write to that table wedges with 'incomplete
prior delete; run cleanup'.

Forge that exact residue (a live 'feature' branch + a directly-created
'feature' ref on the Person table the manifest doesn't reference) and assert
the next load AND mutate self-heal. Deterministic and local — no S3 or timing,
since the forge IS the post-crash state. Adds a shared node_table_uri helper.

This commit is RED: it reproduces the bug and fails against the unfixed engine
with the predicted symptom. The fix follows in the next commit.

* fix: self-heal manifest-unreferenced branch forks

The first write to a table on a branch lazily forks it via Lance create_branch,
a durable two-phase op that advances Lance state BEFORE the atomic manifest
publish. If the writer dies or its request future is cancelled between the fork
and the publish, the branch ref is fully formed but the manifest never
references it. The next write re-enters the fork path, create_branch collides,
and the engine wedged with 'orphaned table state ... incomplete prior delete;
run cleanup' — which cleanup could not even fix, because the branch is still a
live manifest branch. This hit load, mutate, ingest, and the merge fork path
(one shared engine chokepoint), so a routine deploy restart or client
disconnect could wedge a branch.

Fix: treat the per-table fork ref as derived state of the manifest. fork_branch_
from_state returns a typed ForkOutcome instead of a human 'incomplete prior
delete' error; on RefAlreadyExists the db layer reclaims the manifest-
unreferenced fork (force_delete_branch + re-fork, exactly once) and proceeds.
A live committed fork is still routed to a retryable conflict before the fork
path, so concurrent first-writes stay correct.

Reclaim is only safe if no in-process writer can be mid-fork, so the write
entry points (load, mutate) acquire the per-(table, branch) write queues for
all touched tables up front — before the fork, held through the publish — when
forking a non-main branch. commit_all accepts these pre-held guards instead of
re-acquiring (the queue is non-re-entrant). The merge fork path already holds
the queue and self-heals through the shared wrapper. Cross-process in-flight
forks remain the documented one-winner-CAS gap.

Mechanical prep folded in: mutation IR lowering is hoisted so the touched-table
set is known before execution; commit_all gains the held_guards parameter.

Flips recreate_over_orphaned_fork_before_cleanup_is_actionable to assert
self-heal; fork_collision_with_live_concurrent_fork_is_retryable still holds.
Docs: writes.md cancelled-future note, invariants.md cross-process known gap.

* fix(cleanup): reconcile per-table manifest-unreferenced forks

reconcile_orphaned_branches keyed orphans on the branch NAME (absent from the
manifest), so it only reclaimed forks from a fully-deleted branch. A fork left
on a still-live branch by an interrupted first-write was never reclaimed — the
backstop the handoff expected cleanup to provide did not cover that case.

Broaden it to a per-table authority test: a Lance branch B on table T is an
orphan iff B is not a live manifest branch (delete-leftover) OR the manifest's
branch-B snapshot does not place T on B (interrupted first-write). Per-branch
snapshots are resolved once and cached across tables. Legitimately-forked
tables, main, and internal/system branches are never reclaimed; children are
dropped before parents to avoid Lance's referenced-parent RefConflict. The
commit-graph half stays whole-branch (per-table doesn't apply there).

This is the guaranteed-convergence backstop to the write-path self-heal: it
reclaims any fork the write path never revisits, and is what Lance's own
create_branch docstring asks embedders to provide for zombie/orphan refs.

* fix: reclaim self-validates against fresh manifest authority

The fork reclaim force-deletes a Lance branch ref, gated on the caller's proof
that the manifest does not place the table on the branch. But the first-write
path obtains that proof via snapshot_for_branch, which returns the coordinator's
CACHED snapshot when the handle is bound to the branch (an embedded handle on
the branch, or branch_merge's target swap). If that snapshot is stale and a
concurrent writer already published a legitimate fork, the reclaim would
force-delete it and re-fork from source, stranding the manifest at a version the
recreated ref no longer has.

Make the destructive primitive own its safety precondition: re-derive it from a
FRESH manifest read (fresh_snapshot_for_branch, which bypasses the cache)
immediately before force-deleting. If fresh authority shows the table is on the
branch, refuse with a retryable conflict instead of destroying a valid fork.
Correct for any caller regardless of snapshot staleness. Also stop branching on
Lance's exact RefConflict prose (loosened match; typed-variant is the durable
follow-up). Addresses PR review (Codex P1, Greptile P2).

* fix: cover delete-cascade edges in up-front fork-queue acquisition

A node delete cascades to every edge table touching that node (execute_delete_
node), forking those edge tables during execution. But touched_table_keys
derived the up-front fork-queue set from the IR ops alone (just node:Type), so a
branch delete that forks node + cascade edges held only the node queue —
commit_all then saw cascade-edge keys it had no guard for.

The touched set is a pure function of (IR ops + catalog), so compute the
COMPLETE set: op types plus, for delete-node ops, the cascade edges derived the
same way the executor derives them (from_type/to_type match). Pre-computed now
equals actual by construction.

Also promote commit_all's held-guard coverage check out of debug_assert into an
all-builds check that fails the write with a typed manifest_internal error: a
load-bearing serialization invariant must fail loudly+safely in release, not
silently proceed unguarded if a future execution path ever touches a table
outside the pre-computed set.

Adds branch_cascade_delete_forks_node_and_edges_under_held_queues, which drives
the cascade path on a branch (the gap the existing insert/load tests missed).
Addresses PR review (Cursor medium, Greptile P2).

* fix(cleanup): serialize fork reclaim against in-process live writers

The broadened per-table reconciler force_delete'd an orphan candidate on a LIVE
branch without holding the per-(table, branch) write queue. An in-process
first-write fork in its fork->publish window holds that queue and has not yet
advanced the manifest, so it looks exactly like an origin-2 orphan — concurrent
cleanup could delete the ref the writer still holds and is about to publish.
(The old branch-name-based reconciler did not have this race: a deleted branch
cannot have a live first-write.)

Bring the reconciler under the same invariant the write-path reclaim already
obeys: never force_delete a fork ref without holding the (table, branch) write
queue AND confirming, under it, from a fresh read, that the ref is still
manifest-unreferenced. Acquire one key at a time (no lock-order inversion vs
multi-table acquire_many writers); if the writer published meanwhile, the fresh
re-check sees the table on the branch and skips. Cross-process writers remain
the documented one-winner-CAS gap. Addresses PR review (Cursor high).

* fix: classify create_branch failure by ref existence, not by failure

fork_branch_from_state mapped ANY create_branch failure to RefAlreadyExists,
routing transient I/O / version / Lance-internal errors into the destructive
reclaim path and masking the real error as a retryable conflict.

Branch on the actual fact instead: on create_branch failure, check whether the
ref exists (list_branches). Only a genuinely pre-existing ref — a fully-formed
manifest-unreferenced fork — is a reclaim candidate; any other failure
propagates with fidelity. We deliberately do NOT force-delete on a not-found-ref
failure: it is indistinguishable from a transient error on a fresh create, and
force-deleting there is the overreach the fresh-authority guard already removed.
A phase-1-only Lance zombie (rarer; create_branch interrupted mid its two
internal phases) surfaces as the propagated error for manual reclaim.
Addresses PR review (Cursor medium).

* fix(cleanup): skip (not delete) on a transient re-check error for a live branch

The reconcile pre-delete re-check treated ANY fresh_snapshot error as 'still an
orphan' and proceeded to force_delete. A transient manifest read failure on a
LIVE branch could therefore destroy a fork the manifest still considers
legitimate — inconsistent with the write-path reclaim (aborts on the same error)
and the candidate scan (skips on snapshot failure).

Distinguish the two origins under the queue: a branch absent from the manifest
authority (origin 1) is a confirmed orphan and is deleted without a fresh read
(no live writer can hold a deleted branch's queue); a LIVE branch (origin 2)
gets the fresh re-check and, on a transient read error, is SKIPPED — never
destroyed on ambiguity — converging on a later cleanup. Same don't-destroy-on-
ambiguous-error principle as the create_branch failure classification.
Addresses PR review (Cursor medium).

* fix(cleanup): unify fork-ref reclaim on fresh authority under the queue

Consolidates the reconcile/reclaim hardening from PR review (the earlier per-site
commits were collapsed when reconciling with the main merge). Both destructive
fork-ref sites — the write-path reclaim and the cleanup reconciler — now share
one classifier, classify_fork_ref -> ForkRefStatus { Legitimate, Orphan,
Indeterminate }, evaluated from FRESH manifest authority under the held
(table, branch) write queue. A fork ref is destroyed ONLY on a confirmed Orphan;
a Legitimate (concurrent writer published a real fork) or Indeterminate
(transient read) status is never destroyed — the write path maps it to a
retryable conflict, cleanup maps it to skip. This closes, by construction:

- reclaim trusting a possibly-cached caller proof (Codex P1);
- reconcile racing an in-process live fork without the queue (Cursor);
- delete-on-transient-error in the re-check (Cursor/Greptile);
- origin-1 trusting a stale live_branches capture for a created-since branch
  (Cursor/Greptile P1).

Having one classifier removes the duplication that let the two sites drift.
ForkOutcome is made pub to match the sealed trait method returning it. Verified
green on Lance 7.0.0 (full engine suite + 48/48 failpoints).

* test(cleanup): pin classify_fork_ref decision (Legitimate / Orphan / ghost)

Both fork-ref reclaim sites (write-path reclaim + cleanup reconciler) route
their destroy/skip decision through classify_fork_ref, but it had no direct
test — reverting the fresh-authority logic was not test-detectable. Add a
deterministic in-source unit test that forges each state and asserts the status:
a manifest-placed fork -> Legitimate (never destroyed); a ref the manifest does
not place on the branch -> Orphan; a ref for a branch absent from the manifest
-> Orphan (ghost reclaim preserved). This makes the core fresh-authority
decision behind every reclaim fix revert-detectable in one place.

(The Indeterminate arm — transient read on a live branch -> skip — needs an
injected read failure and is left to the failpoints suite; the cross-process
cleanup-vs-writer and cached-snapshot reclaim races are the documented
one-winner-CAS gap, not reachable same-process bugs, so they are not faked here.)

* test(cleanup): pin the Indeterminate (transient re-check) reclaim arm

Closes the last untested classify_fork_ref arm. Adds a 'classify.fresh_read'
failpoint (no-op without the failpoints feature) that simulates a transient
failure of the fresh-authority read, and a failpoints test driving it through
cleanup: a genuine origin-2 orphan on a LIVE branch whose fresh re-check fails
classifies as Indeterminate, so the reconciler SKIPS it (never destroys on an
inconclusive read) and reclaims it on the next run once the read succeeds.

This makes the don't-destroy-on-ambiguity rule revert-detectable end-to-end.
The only paths now left untested are the cross-process cleanup-vs-writer and
reclaim-vs-publish races — the documented one-winner-CAS gap (cleanup is
&mut self / CLI-only, so no reachable same-process race), not faked here.

* test(server): avoid stale schema apply route handle

* fix(cleanup): report indeterminate fork authority clearly
2026-06-15 22:17:25 +02:00
Andrew Altshuler
d2340f19e9
feat(cli)!: schema apply refuses a cluster-managed graph (RFC-011 D10) (#253)
`omnigraph schema apply` against a cluster-managed graph's storage root bypassed
the cluster ledger/recovery/approvals. Mirror `init`'s refusal: on the embedded
(direct-store) path, if the resolved URI is inside a cluster
(`cluster_root_for_graph_uri`), bail and point at `cluster apply`. The served
(`--server`) path is unaffected — it addresses a server, not a storage root.
`schema plan`/`show` (read-only) are untouched.

Two e2e tests injected "out-of-band drift" via this exact CLI path; since the
CLI now refuses it, they inject drift via a direct engine `apply_schema` against
the storage root instead — a faithful control-plane bypass, which is what
out-of-band drift is. New regression:
`schema_apply_refuses_a_cluster_managed_graph_and_signposts_cluster_apply`.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-15 23:11:42 +03:00
Ragnor Comerford
7d412988b5
feat(cluster): per-graph embedding profile config (RFC-012 Phase 5)
Engine: factor the alias->(provider, base_url, model, key-envs) defaults into a shared provider_profile, used by both from_env and a new pub from_parts(provider, base_url, model, api_key) so the cluster path applies identical defaults (DRY).

Cluster: GraphConfig gains an optional per-graph embeddings: profile (EmbeddingProfile { provider?, base_url?, model?, api_key }). EmbeddingProfile::resolve() resolves the api_key from a ${NAME} env ref (resolve_secret_ref rejects inline values and unset vars) and builds an engine EmbeddingConfig via from_parts.

Parse + resolve + validate only; wiring the profile through the applied cluster state into the served handle (the config-free s3:// boot reads applied state, not cluster.yaml) follows. 21 engine + 4 cluster tests pass.
2026-06-15 22:03:10 +02:00
Ragnor Comerford
3de93ae7ab
feat(engine): inject embedding config into the handle (RFC-012 Phase 5)
The Omnigraph handle gains an optional embedding_config (Arc<EmbeddingConfig>) injected via with_embedding_config(), mirroring with_policy(). The query path now threads an EmbeddingResolver (bundling the reuse OnceCell + the optional injected config) instead of the bare cell; its lazy resolve() builds the client from the injected config when present, else EmbeddingClient::from_env(). Laziness preserved (a graph that never embeds needs no key). This is the engine half of cluster per-graph embedding wiring; the cluster config + serving injection follow. New search test proves the injected config is used (from_env would error with no keys).

Self-contained: no cluster dependency. 25 search tests pass.
2026-06-15 21:50:55 +02:00
Ragnor Comerford
ffb4a2c9ab
fix(embedding): openai-alias api key + deadline scope (PR review)
openai-alias api key (Cursor High): key resolution was dispatched on the Provider enum, which is OpenAiCompatible for both openai and openai-compatible, so OMNIGRAPH_EMBED_PROVIDER=openai (base api.openai.com) could send OPENROUTER_API_KEY and 401. Fix: the alias match now yields the ordered key-env list too, so base, model and key are all alias-derived in one place. openai takes only OPENAI_API_KEY (errors loudly if absent); openai-compatible/unset prefer OPENROUTER_API_KEY then OPENAI_API_KEY. Closes the class, not the instance.

deadline scope (Cursor Medium): the deadline bounds every embed call (query and document), which is correct, but the name said query-only. Renamed OMNIGRAPH_EMBED_QUERY_DEADLINE_MS -> OMNIGRAPH_EMBED_DEADLINE_MS (field query_deadline_ms -> deadline_ms) and updated the doc wording so the name matches the behavior. Two new alias-key tests; 20 embedding unit tests pass.

Resolved earlier in 30377c4: openai-alias base URL, single embed-model source, stale @embed ingest docs. Declined: RFC Status (the lifecycle keeps it Proposed while the PR is open).
2026-06-15 21:44:31 +02:00
Andrew Altshuler
4601e5f4bf
feat!: delete the legacy OmnigraphConfig + config migrate; finish the omnigraph.yaml docs sweep (#252)
* refactor(cli): own ReadOutputFormat/TableCellLayout in the CLI

The two output-presentation enums lived in `omnigraph-server::config` and were
re-exported for the CLI, even though the server never used them. Move both
definitions into `omnigraph-cli/src/read_format.rs` (where the renderer already
lives) and drop them from the server's public re-export. This is a step toward
deleting the legacy `omnigraph-server::config` module entirely — a CLI
presentation concern has no business in the server crate.

No behavior change. The server keeps private copies in `config.rs` only for the
soon-to-be-deleted legacy `CliDefaults`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(cli)!: remove the `config migrate` command and migrate.rs

`config migrate` was the last CLI consumer of the legacy `omnigraph.yaml`
(`OmnigraphConfig` + `load_config`). With the excision complete there is no
legacy file to split, so the whole `omnigraph config` command group is removed
along with `migrate.rs`. The `OmnigraphConfig` type, `load_config`, and the
deprecation machinery are deleted next.

- Remove `Command::Config` / `ConfigCommand` from the clap surface and the
  dispatch arm; drop `mod migrate;` and the now-unused `load_config` import.
- Drop the `Command::Config` arms in `planes.rs`.
- Delete the `config_migrate_splits_legacy_config` integration test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(server)!: delete the legacy OmnigraphConfig type and load_config

With `config migrate` gone, nothing loads `omnigraph.yaml` anymore. Delete the
entire `omnigraph-server::config` module: the `OmnigraphConfig` type and its
sub-structs (`ProjectConfig`, `TargetConfig`, `CliDefaults`, `ServerDefaults`,
`AuthDefaults`, `QueryDefaults`, `AliasConfig`, `AliasCommand`, `PolicySettings`,
`QueryEntry`, `McpSettings`), `load_config`, and the RFC-008 deprecation
machinery (`OMNIGRAPH_CONFIG`, `OMNIGRAPH_NO_LEGACY_CONFIG`,
`OMNIGRAPH_SUPPRESS_YAML_DEPRECATION`, the deprecation map + warner).

- `QueryRegistry::load` (the only `OmnigraphConfig`/`QueryEntry` consumer; its
  only caller was its own test) is removed — server boot and the CLI both build
  registries via `QueryRegistry::from_specs`.
- `graph_resource_id_for_selection` (CLI-only) moves into the CLI
  (`helpers.rs`), with its unit test; the server no longer exports it.
- Drop the already-dead `format_registry_load_errors` helper (config-adjacent).

No behavior change — every deleted item was unreachable after the excision.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs: purge the legacy omnigraph.yaml surface from the docs

Finish the RFC-011 excision in the docs: the CLI no longer reads omnigraph.yaml
and the server boots cluster-only, so every doc that described the legacy file
as a live config is now wrong.

- AGENTS.md: rewrite the HTTP-server line to cluster-only boot (drop the
  single-graph/flat-route and omnigraph.yaml-boot framing); rewrite the CLI
  two-surface-config passage (drop `config migrate`, the deprecation env vars,
  and "Never extend omnigraph.yaml"); fix the topic table + capability rows.
- cli/reference.md: delete the entire "omnigraph.yaml schema (legacy combined
  file)" section and the `config migrate` row; re-home the `policy` row, the
  bearer-token chain, the actor/format/param-precedence references, and the
  `--config` mentions to the operator config + `--cluster`.
- cli/index.md: rewrite the multi-graph-server + add-graph paragraphs to
  cluster (`--cluster` + `cluster apply`); fix the policy examples to
  `--cluster`; replace the `## Config` omnigraph.yaml example with the
  operator/cluster two-surface model.
- operations/policy.md: rewrite per-graph-vs-server-level policy to the cluster
  `policies:`/`applies_to` model; re-home the actor + CLI tooling sections.
- clusters/config.md, clusters/index.md, deployment.md: server boots from the
  cluster only; per-operator facts come from ~/.omnigraph/config.yaml.
- architecture.md, testing.md: drop the stale omnigraph.yaml / deleted-test
  references.

RFCs, design specs, and prior release notes are left as historical records.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-15 22:31:29 +03:00
Ragnor Comerford
70ed848b9d
docs(embeddings): @embed model arg + same-space validation (RFC-012 Phase 3-4)
Document the optional @embed model kwarg, the query-time same-space rejection, model-string strictness, and the loud schema-apply refusal on model change. Mark RFC-012 phases 1-4 implemented.
2026-06-15 21:09:35 +02:00
Ragnor Comerford
0a34f9011b
feat(engine): same-space validation for @embed model (RFC-012 Phase 4)
resolve_nearest_query_vec rejects a nearest($v, string) query with a typed error when the property recorded a model (via @embed) that differs from the resolved query embedder's model, closing the silent cross-space ranking. An @embed without a recorded model keeps working with no check. EmbeddingConfig::mock() honors OMNIGRAPH_EMBED_MODEL so the check is exercisable under mock. Two new search tests.
2026-06-15 21:09:35 +02:00
Ragnor Comerford
1a06150c33
feat(compiler): record @embed model in the catalog (RFC-012 Phase 3)
NodeType.embed_sources becomes HashMap<String, EmbedSource { source, model }>, populated from the @embed source arg + model kwarg; it round-trips through build_catalog_from_ir (the engine's IR-load path), so the recorded model reaches query execution. The migration planner already rejects any @embed change as UnsupportedChange, so changing a recorded model is a loud schema-apply refusal for free. New catalog test.
2026-06-15 21:09:34 +02:00
Ragnor Comerford
74476f7f51
feat(compiler): @embed model kwarg in grammar/AST/parser (RFC-012 Phase 3)
Annotations gain optional comma-separated key=value kwargs. Annotation keeps value (existing consumers unchanged) and adds kwargs: BTreeMap with serde(default, skip_serializing_if) so empty kwargs are omitted and existing schemas' IR JSON/hash stay byte-identical. The parser rejects any @embed kwarg other than model. render_annotations shows kwargs. 3 new parser tests.
2026-06-15 21:09:15 +02:00
Andrew Altshuler
0bee746a31
feat(cli)!: excise omnigraph.yaml from the CLI; policy/queries tooling reads --cluster (#251)
The server already dropped omnigraph.yaml (cluster-only boot). This removes the
CLI's last use of the legacy `OmnigraphConfig`: graphs are addressed only via
`--store`/`--server`/`--cluster`/`--profile`/operator defaults, and actor,
output format, and bearer credentials come from `~/.omnigraph/config.yaml`.
After this change no CLI command reads `omnigraph.yaml` except `config migrate`.

Resolvers (helpers.rs): drop every legacy fallback —
- `resolve_actor` → `--as` > `operator.actor` (no `cli.actor`);
- `resolve_read_format` → `--json`/`--format` > alias > `defaults.output`;
- `resolve_branch`/`resolve_read_target` → `--branch` > alias > "main";
- `resolve_uri`/`resolve_cli_graph` → scope path only; an absent address is a
  loud error;
- `resolve_remote_bearer_token` → operator keyed chain + `OMNIGRAPH_BEARER_TOKEN`.
`GraphClient::resolve`/`resolve_with_policy` drop the `&OmnigraphConfig` param;
direct-store access carries no Cedar policy (policy lives in the cluster/server).

Flags (cli.rs): remove `--config` from every data/query command; it stays only
on `cluster *` (the cluster dir) and `config migrate` (the legacy path).

Re-home control-plane tooling to `--cluster` (RFC-011):
- `policy validate|test|explain` source the Cedar bundle from the cluster's
  applied policies; `--graph` picks a graph's bundle; `policy test` takes
  `--tests <file>`;
- `queries list|validate` source the registry + schemas from the cluster
  serving snapshot; `--graph` scopes to one graph;
- `lint` requires `--schema` (offline) or a direct/cluster graph target;
- `schema plan`/`lint` route their graph-target through the shared direct-scope
  resolver so `--store`/`--profile`/`defaults.store` addressing works.

Tests migrate from `omnigraph.yaml` fixtures to `--store`/operator-config/
`--cluster` (converged-cluster fixtures); the now-impossible command-path
RFC-008 tests are deleted (`config migrate` coverage kept). The
`OmnigraphConfig` type, `load_config`/deprecation machinery, and `config
migrate` are removed in a follow-up.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-15 21:48:39 +03:00
Andrew Altshuler
8b01c6e547
feat(server)!: cluster-only server — remove single-graph serving (RFC-011) (#250)
omnigraph-server boots only from --cluster; all HTTP is /graphs/<id>/…; flat single-graph routes and the omnigraph.yaml server boot are removed. GraphRouting/ServerConfigMode collapse to multi-only; openapi.json regenerated to the nested shape; ~100 server route tests migrated; parity/system_local boot from a converged cluster. Gate green (1410 tests).
2026-06-15 20:17:25 +03:00
Ragnor Comerford
b183db078f
Index materialization is derived state: defer off the write path, reconcile via optimize (iss-848) (#246)
* test(engine): reproduce empty-table Vector @index aborting schema apply

A Vector (IVF) index trains k-means centroids over the column, so Lance
cannot build it on 0 vectors ("Creating empty vector indices with
train=False is not yet implemented"). schema apply reconciles a table's
whole index set whenever any @index on it changes, so adding an unrelated
scalar @index materializes the dormant empty vector index and aborts the
entire migration (all-or-nothing).

This regression test inits a 0-row Doc with a Vector @index, adds a scalar
@index, and asserts the apply succeeds (then loads one embedded row and
asserts the deferred index materializes). It fails today at the apply step
with the vector-index abort; the fix lands in the next commit.

Refs dev-graph iss-empty-vector-index-schema-apply, iss-848.

* fix(engine): defer Vector @index on an empty table instead of aborting schema apply

build_indices_on_dataset_for_catalog materialized a declared Vector @index
unconditionally. On a 0-row table Lance cannot train the IVF index
("Creating empty vector indices with train=False is not yet implemented"),
so any later migration that touches the table (e.g. adding an unrelated
scalar @index, which reconciles the table's whole index set) aborted the
entire migration on the dormant vector index — all-or-nothing.

Guard the vector arm with a row-count check, matching the guard
ensure_indices_for_branch and the branch-merge rebuild already use: an
untrainable column becomes a pending index that a later ensure_indices /
optimize materializes once the table has rows. Reads stay correct meanwhile
(vector search degrades to a brute-force scan).

Stop-gap: the residual rows-present-but-vectors-null window and the full
decoupling (intent recorded at apply, an idempotent coverage reconciler)
are dev-graph iss-848. Turns the green half of the regression test added in
the previous commit.

Refs dev-graph iss-empty-vector-index-schema-apply, iss-848, iss-687.

* docs(invariants): record the logical-contract-over-physical-state principle

The bug class behind the empty-table vector-index abort (and the schema-apply
vs optimize version drift) is one shape: a physical operation allowed to fail
a logical one. Several hard invariants (2, 5, 7, 13) and deny-list items are
already instances of this, but the unifying rule was never written down.

Add it to docs/dev/invariants.md as a "Governing principle" section above the
hard invariants, naming which invariants and deny-list items instantiate it
and the smell to watch for (a logical operation gated on a physical fact).
Add a one-line always-on rule (7) in AGENTS.md so it stays in working memory,
with the qualifier that genuine logical conflicts still fail loudly — the
licence to lag covers physical convergence, not correctness.

Audience-neutral: no private ticket refs. check-agents-md.sh passes.

* test(engine): index build must tolerate rows with null vectors (load-before-embed)

Loading rows whose vector column is null into a `Vector @index` table fails
today: build_indices (reached via the loader's prepare_updates_for_commit)
calls create_vector_index, and Lance's IVF KMeans errors "cannot train 1
centroids with 0 vectors". The same abort hits ensure_indices/optimize/schema
apply/merge, since they all funnel through build_indices_on_dataset_for_catalog.

This test loads two null-embedding rows and calls ensure_indices; it must not
abort (the untrainable vector column is deferred, sibling indexes still build).
Fails today at the load step; fixed in the next commit.

Refs dev-graph iss-848, iss-empty-vector-index-schema-apply.

* fix(engine): defer unbuildable index columns instead of aborting the write path

build_indices_on_dataset_for_catalog is the chokepoint every write path funnels
through (load/mutate via prepare_updates_for_commit, schema apply, ensure_indices,
optimize, branch merge). Its vector arm called create_vector_index
unconditionally, so a column with no trainable vectors yet — an empty table, or
rows loaded before `embed` populates them — aborted the whole operation with
Lance's IVF KMeans error.

Fault-isolate the vector build: on failure, record the column as a PendingIndex
(table, column, reason), log it, and continue building the sibling indexes; a
later ensure_indices/optimize materializes it once the column is trainable, and
reads use brute-force meanwhile. Manifest/CAS/IO errors at the publish boundary
still propagate. Isolating at the single chokepoint realizes the governing
principle (physical index state never fails a logical operation) for every write
path, and supersedes the earlier symptomatic count_rows==0 stop-gap (removed) —
closing the residual rows-present-but-vectors-null window it left open.

Surfacing pending index status rather than failing is the database norm
(Postgres indisvalid, LanceDB list_indices). ensure_indices and the build_indices
wrappers now return Vec<PendingIndex>; optimize surfaces it in a later commit.

Refs dev-graph iss-848, iss-951 (vector index stays inline-commit until lance#6666).

* test(engine): index-only schema apply must not touch table data

Adding an @index to an existing column should be a pure metadata change once
index materialization moves to the reconciler (iss-848): the apply records the
intent in the catalog/IR but builds nothing inline, so the table's manifest
version is unchanged. Today the indexed_tables block builds the index inline
and bumps the version (4 -> 5). Fixed in the next commit.

Refs dev-graph iss-848.

* fix(engine): schema apply records index intent only; index-only apply is metadata

Schema apply no longer builds indexes inline. The four build_indices calls
(added/renamed/rewritten/index-only tables) are removed; the @index/@key intent
is already persisted in the catalog/IR the apply writes, and the physical index
is materialized off the critical path by ensure_indices/optimize (iss-848).

Concretely:
- AddConstraint (an @index addition — every other added constraint plans as
  UnsupportedChange) becomes a pure metadata step alongside the metadata-only
  steps: it touches no table data, so the table version is unchanged.
- added/renamed/rewritten tables still write their data; only the trailing
  index build is gone. The rewritten table's coverage is restored later by
  optimize_indices.
- recovery_pins drops index-only tables (they no longer advance Lance HEAD) and
  keeps rewritten tables; their post_commit_pin = expected+1 is now exact (one
  rewrite commit), strengthening recovery classification.
- the now-orphaned Omnigraph::build_indices_on_dataset_for_catalog wrapper is
  removed.

A migration can no longer abort on an index build, for any index type at any
cardinality. Turns the green half of index_only_constraint_apply_touches_no_table_data.

Refs dev-graph iss-848.

* test(engine): optimize must converge a declared-but-unbuilt index

After iss-848, adding an @index post-data is a metadata-only apply that defers
the physical build, so the column is declared-indexed but unbuilt (reads scan).
`optimize` — the operator's cron reconciler — must materialize it. Today optimize
only maintains coverage of EXISTING indexes (optimize_indices) and never creates
missing ones, so the rank BTREE stays Degraded after optimize. Fixed next commit.

Refs dev-graph iss-848.

* fix(engine): optimize materializes declared-but-unbuilt indexes (the reconciler)

`omnigraph optimize` is the operator's cron reconciler. It already compacts and
folds new fragments into EXISTING indexes (optimize_indices); now it also builds
declared-but-missing indexes, so the indexes schema apply / load defer (iss-848)
converge on the next optimize.

Done inside optimize_one_table (not by composing the all-tables ensure_indices,
which is drift-blind and would re-publish the uncovered HEAD>manifest drift that
optimize deliberately skips): after the per-table drift/blob skips and under the
queue + Optimize sidecar already held, a needs_index_create gate (reusing
needs_index_work_node/edge — "declared index missing AND row_count > 0", so empty
tables stay no-ops) admits index-only work, and Phase B builds the missing index
over the just-compacted layout via the build chokepoint. An untrainable vector
column fault-isolates into the new TableOptimizeStats.pending_indexes (the
list_indices/indisvalid analog operators read), not a failure. committed now
reflects index commits, so the existing post-publish cache invalidation covers
them. LanceDB's optimize only maintains existing indexes; creating
declared-but-missing ones is the L2 behavior omnigraph's declarative @index needs.

Turns the green half of optimize_materializes_index_declared_but_unbuilt.

Refs dev-graph iss-848.

* docs: index materialization is deferred to the reconciler (iss-848)

Update the index-lifecycle docs to reflect the new contract: @index/@key
declares intent and the physical index is derived state that never fails a
logical operation. Schema apply builds nothing (records intent only);
load/mutate build inline through one chokepoint that defers an untrainable
Vector column as pending; optimize/ensure_indices is the reconciler that
creates declared-but-missing indexes and maintains coverage, reporting
still-pending columns.

Touches: dev/invariants.md (truth-matrix Index-lifecycle row), AGENTS.md
(capability matrix), user/search/indexes.md (L2 orchestration), user/operations/
maintenance.md (optimize reconciler bullet), dev/testing.md (new tests).

* test(server): schema_apply_route_can_add_index reflects deferred index build

iss-848 made schema apply record @index intent without building the physical
index inline. The route test asserted the index count increased after apply;
on an empty graph it now stays unchanged (the build is deferred to
ensure_indices/optimize). Assert the new contract: apply succeeds and the
physical index count is unchanged.

* fix(engine): precheck vector trainability — don't pin or swallow (PR review)

Two issues Cursor Bugbot caught in the chokepoint fault-isolation:

1. (HIGH) Pending vector pins roll back siblings. needs_index_work_node counted
   a missing vector index as work whenever the table had rows, so a column with
   no trainable vectors got pinned in the EnsureIndices recovery sidecar — but
   the build deferred it (zero commit). On a crash before manifest publish the
   classifier sees NoMovement and the all-or-nothing decision (recovery.rs
   decide()) rolls back the WHOLE sidecar, undoing a sibling table's committed
   index work.
2. (MED) Vector build swallowed fatal errors. The match arm converted every
   create_vector_index error into a deferred PendingIndex, hiding genuine
   I/O/manifest/Lance failures as "pending".

Fix both with one trainability precheck (vector_column_trainable: >=1 non-null
vector, the ivf_flat(1) minimum) used identically by needs_index_work_node and
the build arm: an untrainable column is never counted as work (so never pinned —
no zero-commit pin) and never attempted (so it can't fail); only a trainable
column is built, and then any error PROPAGATES (stays fatal). The deferred
column is still recorded as a PendingIndex with a clear reason.

Refs dev-graph iss-848.

* feat(cli): surface pending index column + reason in optimize output (PR review)

Codex (P2): pending_indexes was documented as visible in `optimize --json` but
the CLI projection never emitted it — operators would lose the only signal that
optimize has deferred index work. Greptile (P2): the stat dropped the reason, so
operators saw which column was stuck, not why.

Carry the reason: TableOptimizeStats.pending_indexes is now Vec<PendingIndex>
(column + reason), and `omnigraph optimize --json` emits {column, reason} per
pending index; human output prints a "↳ index pending on '<col>': <reason>" line.

Refs dev-graph iss-848.

* test: align CLI index-add test with deferred build; cover post-rename reconcile

- schema_apply_json_adds_index_for_existing_property (cli_schema_config.rs): the
  CLI analog of the server test — asserted the index count grew after apply;
  under iss-848 the apply defers the build, so the count is unchanged on an
  empty graph. Assert the deferred contract. (The only full-suite failure.)
- optimize_materializes_index_after_type_rename (maintenance.rs, new): covers
  the gap Greptile flagged — a RenameType writes the renamed table with rows but
  no indexes (inline build removed in Commit B); assert the rank index is
  Degraded post-rename and Indexed after optimize reconciles it.

Refs dev-graph iss-848.

* test(engine): in-source apply tests reflect deferred index materialization

The two db::omnigraph in-source unit tests asserted the old "schema apply builds
/ preserves indexes inline" behavior (the only remaining full-suite failures):

- test_apply_schema_defers_index_then_reconciler_builds_it (was
  test_apply_schema_adds_index_for_existing_property): apply records the @index
  intent but builds nothing; assert the BTREE on `age` is absent after apply and
  present after ensure_indices. (Uses `age`, unindexed in TEST_SCHEMA — `name
  @key` is already FTS-indexed at seed.)
- test_apply_schema_rewrite_defers_index_then_reconciler_restores (was
  test_apply_schema_rewrite_preserves_existing_indices): an AddProperty rewrite
  no longer rebuilds indexes inline; assert ensure_indices restores id BTREE +
  name FTS after the rewrite.

Verified by grep that these + the server/CLI tests are the complete set of
"apply builds an index" assertions; all other index-presence tests run after
load/ensure_indices/primitives, which still build.

Refs dev-graph iss-848.

* fix(engine): optimize always reports pending indexes, not only on create-work (PR review)

Cursor Bugbot (MED): pending_indexes was filled only when needs_index_create was
true, but the vector trainability precheck makes needs_index_work_node exclude an
untrainable Vector column. So a table whose sole missing index is untrainable, but
which optimize still compacts or reindexes, returned an empty pending_indexes —
contradicting the documented operator contract for deferred columns.

Run the (idempotent) build chokepoint unconditionally once past the no-op gate,
rather than gating it on needs_index_create. It skips existing indexes, builds
any buildable missing one, and reports an untrainable column as pending whether
the table entered for compaction, reindex, or index creation. needs_index_create
still gates the no-op decision (so an index-only table still enters the path).

Refs dev-graph iss-848.

* test(engine): reframe staged-BTREE-failure failpoint onto the reconciler path

ensure_indices_stage_btree_failure_leaves_existing_tables_writable fired
`ensure_indices.post_stage_pre_commit_btree` and expected `apply_schema` (adding
a type) to fail mid-BTREE-build. iss-848 removed apply's inline index build, so
that apply now succeeds and the test's unwrap_err panicked — it exercised a
removed code path.

Reframe onto where BTREE builds happen now: seed Person, add an `@index` on
`age` (apply records intent, defers the build), then `ensure_indices` builds the
deferred BTREE and the failpoint fires between stage and commit. Person's HEAD
is unchanged (no drift) and its EnsureIndices sidecar pins NoMovement; a write to
a different, unpinned table (Company) is unaffected (mutations/loads heal
roll-forward and proceed, unlike optimize/repair which refuse on a pending
sidecar). Preserves the original coverage (staged-index stage failure leaves
other tables writable, no drift) in the new architecture.

Refs dev-graph iss-848.

* feat(server): converge deferred indexes promptly after schema apply (iss-848)

Schema apply records @index intent but defers the physical build. On a
long-lived server, spawn a detached best-effort ensure_indices after a
successful apply so the indexes converge promptly instead of waiting for the
operator's next optimize. Fire-and-forget: it never blocks or fails the apply
response, and a failure is logged (the index still converges on the next
optimize). Guarded on result.applied. The CLI is one-shot, so it has no
equivalent; its convergence path is the optimize cadence.

handle.engine is already an Arc, so the spawn takes an owned clone. Convergence
itself is covered by the engine ensure_indices/optimize tests; the existing
empty-graph schema-apply route tests confirm the response is unaffected (the
spawn is a read-only no-op on an empty table).

Refs dev-graph iss-848.

* docs(maintenance): list pending_indexes in optimize per-table stats (consistency)
2026-06-15 18:48:43 +02:00
Ragnor Comerford
30377c453b
fix(embedding): address PR review feedback (RFC-012 Phase 2)
openai-alias host (Cursor): OMNIGRAPH_EMBED_PROVIDER=openai now defaults its base URL to https://api.openai.com/v1 (model text-embedding-3-large), while openai-compatible/unset keep the OpenRouter gateway default. The default is derived from the alias rather than the Provider enum, so an operator's stated intent can no longer be silently routed to OpenRouter; an explicit OMNIGRAPH_EMBED_BASE_URL still wins. New test from_env_openai_alias_uses_openai_host_not_openrouter.

single model source of truth (Cursor): remove the EmbedSpec.model field. The provider config is authoritative for the model, so a spec can no longer declare a model that is silently ignored while the API uses another (the wrong-space-vectors footgun); the embed summary reports the model actually resolved. Correct by construction rather than a truthful-echo patch.

stale @embed docs (Codex): docs/user/schema/index.md and docs/dev/execution.md still claimed @embed embeds at ingest; corrected to the real contract (catalog annotation; vectors supplied or pre-filled by 'omnigraph embed'). Also documented the openai-vs-OpenRouter base default in embeddings.md.

Greptile's RFC-status note is declined: the repo lifecycle keeps an RFC Status: Proposed while its PR is open and flips to Accepted on merge.
2026-06-15 18:37:34 +02:00
Ragnor Comerford
e7361dce49
docs(embeddings): provider-independent config surface (RFC-012 Phase 2)
Rewrite docs/user/search/embeddings.md for the resolved Provider model: the provider table, the OMNIGRAPH_EMBED_* env surface (default OpenRouter), the deadline/observability/reuse behavior, and a no-back-compat migration note for gemini-preview graphs.
2026-06-15 17:28:03 +02:00
Ragnor Comerford
532631bca0
perf(engine): reuse the embedding client across queries (RFC-012 Phase 2)
Hold the resolved EmbeddingClient in an Arc<tokio::sync::OnceCell> on the Omnigraph handle, built lazily on the first nearest($v, "string") that needs embedding (so a graph that never embeds needs no provider key) and reused by every later query — dropping the per-query EmbeddingClient::from_env() rebuild and keeping the provider connection pool warm. The cell is threaded through execute_query -> extract_search_mode/extract_sub_search_mode -> resolve_nearest_query_vec via a pub(crate) embedding_cell() accessor (the field is module-private). Covered by the string-nearest paths in tests/search.rs (direct, literal, RRF).
2026-06-15 17:28:02 +02:00
Ragnor Comerford
b999ae3753
feat(engine)!: provider-independent embedding client (RFC-012 Phase 2)
Replace the Gemini-only EmbeddingClient with one resolved EmbeddingConfig { provider, model, base_url, api_key } behind a sealed Provider enum (OpenAiCompatible | Gemini | Mock). OpenAiCompatible (POST {base}/embeddings, bearer, {model, input, dimensions}) covers OpenRouter — the new default gateway — OpenAI direct, and self-hosted endpoints; Gemini keeps its RETRIEVAL_QUERY/RETRIEVAL_DOCUMENT task types; Mock is offline/deterministic. EmbedRole replaces the task-type string.

from_env() resolves provider via OMNIGRAPH_EMBED_PROVIDER (default openai-compatible), base/model via OMNIGRAPH_EMBED_BASE_URL/_MODEL, and the api key from OPENROUTER_API_KEY/OPENAI_API_KEY or GEMINI_API_KEY. BREAKING (pre-release, no back-compat): the default provider is now OpenRouter, OMNIGRAPH_GEMINI_BASE_URL is dropped, and Gemini-only users set OMNIGRAPH_EMBED_PROVIDER=gemini.

Folds in RFC-012 Phase 1 NFR floor: a total-operation OMNIGRAPH_EMBED_QUERY_DEADLINE_MS deadline (default 60s; 0=unbounded) bounds the ~121s worst case, and tracing spans (target omnigraph::embedding) record provider/model/dim/attempt/elapsed/outcome. The offline 'omnigraph embed' CLI follows the resolved provider (its hardcoded gemini-only bail removed). 17 engine embedding unit tests, 4 CLI embed tests, and the search integration suite (22) pass.

Cross-query client reuse and the docs refresh land in follow-up commits.
2026-06-15 17:27:49 +02:00
Ragnor Comerford
6f3e0e3157
Correct 'merge' to 'merged' in README.md
Fix grammatical error in README.md regarding merging changes.
2026-06-15 17:22:28 +02:00
Andrew Altshuler
625ae7c208
feat(cli): defaults.store — a zero-flag local default scope (RFC-011) (#249)
Operator config gains defaults.store (a file:///s3:// graph storage URI), the local-dev counterpart of defaults.server + default_graph. Mutually exclusive with defaults.server, and a store cannot carry default_graph (both refused at load). The zero-flag local default that survives the upcoming removal of omnigraph.yaml's cli.graph. Additive, non-breaking.
2026-06-15 17:23:46 +03:00
Ragnor Comerford
21ada33e0a
Enhance README with additional details on Omnigraph
Expanded description of Omnigraph's functionality.
2026-06-15 16:00:27 +02:00
Andrew Altshuler
9ef5f90991
feat(cli)!: query/mutate invoke stored queries by name; server kind-assert (RFC-011 D3) (#247)
omnigraph query <name> / mutate <name> invoke a stored query by name from the served catalog (served-only). The verb asserts kind via a new expect_mutation on POST /queries/{name} (400 on mismatch). -e/--query + --store is the ad-hoc lane; the positional selects within the source (replacing --name). The bare positional graph URI, --uri, and --name are removed from query/mutate.
2026-06-15 16:52:58 +03:00
Ragnor Comerford
7c916f5b98
refactor(compiler): remove dead OpenAI embedding client (RFC-012 Phase 1)
The compiler-side EmbeddingClient (OpenAI/`text-embedding-3-small`) was pub(crate), #![allow(dead_code)], and had zero callers anywhere in the workspace; the live nearest("string") path and the offline `omnigraph embed` CLI both use the engine Gemini client. It carried the only NANOGRAPH_* env vars (vestigial 'nanograph os' naming) and was the sole user of reqwest+tokio in omnigraph-compiler — dropping them removes an HTTP client and async runtime from a crate that advertises 'Zero Lance dependency' (invariant 11).

Rewrites docs/user/search/embeddings.md to the single-client reality and corrects the false 'engine embeds @embed at ingest' claim. Verified: build green, 238 compiler tests pass, `rg NANOGRAPH_` empty.
2026-06-15 15:07:54 +02:00
Ragnor Comerford
5b83e6b56d
docs(rfc): add RFC-012 provider-independent embedding configuration
Records the design for one resolved EmbeddingConfig behind a sealed Provider enum (OpenAiCompatible primary, covering OpenRouter as the default gateway, OpenAI direct, and self-hosted endpoints; native Gemini for task-type asymmetry; Mock). Identity recorded in the schema IR with query-time same-space validation, NFR floor (deadline, observability, single normalize), and a 5-phase rollout. Corrects the prior handoff's misdiagnosis (the OpenAI client it blamed was dead code).

Links from docs/dev/index.md.
2026-06-15 15:07:38 +02:00
Andrew Altshuler
1bc0ea6b51
feat(cli): no-default-graph errors list candidate graphs (RFC-011 D7) (#245)
When a server/cluster scope resolves with no --graph and no default_graph, the CLI auto-uses a sole graph (cluster) or errors listing the candidate graph ids (cluster catalog; multi-graph server via best-effort GET /graphs), never a silent pick. GraphClient::resolve becomes async; flat/single-graph servers and happy paths are unaffected.
2026-06-15 15:48:29 +03:00
Andrew Altshuler
b395757e21
feat(cli): alias subcommand; remove --alias flag (RFC-011 D4) (#244)
Operator aliases move from the --alias flag on query/mutate to a dedicated 'omnigraph alias <name> [args]' subcommand, so an alias can never shadow or be shadowed by a built-in verb. Unknown name errors listing defined aliases. Removes the legacy alias machinery from query/mutate (net -156 lines); legacy omnigraph.yaml aliases lose their CLI entry point.
2026-06-15 15:23:03 +03:00
Andrew Altshuler
2ed05d2cb1
feat(cli): --quiet/--yes globals; echo resolved write target; gate non-local destructive writes (#243)
RFC-011 Decision 9. Writes echo their resolved target + access path to stderr (suppress with --quiet). Destructive writes (cleanup, overwrite load, branch delete) against a non-local scope require consent: --yes, a TTY prompt, or a hard refusal for non-TTY/--json runs. Local file:// writes unaffected.
2026-06-15 14:35:55 +03:00
Andrew Altshuler
a09045028f
feat(cli)!: unify graph selection under --graph; --cluster is a global scope; remove --cluster-graph (#241)
RFC-011: --graph is the single graph selector across server and cluster scopes; --cluster becomes a global scope primitive; --cluster-graph removed. Maintenance dispatch unified through resolve_scope. Wrong-address guard validates each scope flag against the verb it can consume.
2026-06-15 14:30:58 +03:00
Ragnor Comerford
c3d7639377
test(engine): pin Lance 7 immutable-PK behavior + sharpen native-namespace alignment notes (#240)
* test(engine): pin Lance 7 immutable-PK behavior + sharpen native-namespace alignment notes

Follow-up polish to the Lance 7.0.0 alignment (the immutable-PK migration fix
and the realigned native-namespace surface test). Two precision nits, no
behavior change:

1. Pin the upstream behavior we now depend on. Lance 7 makes the unenforced PK
   immutable once set (`lance::dataset::transaction`): re-applying the reserved
   `lance-schema:unenforced-primary-key` key — even with the same value — errors
   "cannot be changed once set". That is exactly what broke `migrate_v1_to_v2`'s
   crash-idempotency and forced its field-guard. Add
   `lance_surface_guards.rs::unenforced_primary_key_is_immutable_once_set` so a
   future Lance bump that relaxes immutability turns red, prompting re-evaluation
   of the migration guard. (Matches the "first smoke check on a Lance bump"
   discipline in docs/dev/lance.md.)

2. Clarify that the native `DirectoryNamespace` decoupling is contingent on
   omnigraph's legacy boolean PK key, not an unconditional v7 property: with the
   position key the native namespace would still read the manifest. omnigraph
   keeps the boolean key deliberately — Lance honors it permanently (maps to PK
   position 0) and one uniform on-disk format beats a new-vs-old split, since
   existing graphs can't be re-keyed under the same immutability rule. Updated
   the test comment and the lance.md stanza; also corrected the stale `is_empty()`
   description of the migration guard (it now matches on the specific PK field).

* test(engine): make the immutable-PK guard's red-bar diagnostic fire in every change-shape

Review follow-up: the guard's re-set assertion chained `.unwrap().await.unwrap_err()`,
which only surfaces the actionable "Lance no longer rejects re-setting the unenforced
PK" message when immutability is enforced on the async commit path and still returns an
error. Two other change-shapes would panic generically instead, defeating the guard's
purpose:

- if Lance moves the check to the sync validation stage, the first `.unwrap()` panics
  with a bare "unwrap() on Err";
- if Lance relaxes immutability so the re-set succeeds, `.unwrap_err()` panics with a
  bare "unwrap_err() on Ok".

Normalize the sync `.update()` result and the async `.await` into one `Result` and assert
on it, so the diagnostic fires whichever stage enforces (or relaxes) the rule.
2026-06-15 11:33:25 +02:00
Andrew Altshuler
9a607abdec
fix(engine): v1→v2 migration verifies the unenforced PK is object_id, not just non-empty (#239)
Greptile follow-up (#236): `migrate_v1_to_v2` guarded the field-set with
`unenforced_primary_key().is_empty()`, which skips the set whenever *any* field is
the PK — including the (corrupt/unexpected) case where a field other than
`object_id` carries it. That would silently leave merge-insert row-level CAS keyed
on the wrong column, and Lance 7 forbids changing the PK afterward.

Match on the specific PK field instead: `["object_id"]` is the idempotent
crash-recovery no-op, `[]` sets it (the genuine pre-v0.4.0 first migration), and any
other PK refuses loudly. Defensive — Lance won't let a fresh graph reach the error
branch — but correct by construction. The idempotent re-entry path stays covered by
test_publish_migrates_pre_stamp_manifest_to_current_version (28 manifest tests green).

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-15 04:34:02 +03:00
Andrew Altshuler
bc2a989a7b
feat(cli)!: remove legacy data-plane addressing (--target, positional http→remote, --as-on-served) (#238)
* feat(cli): --server accepts a literal URL (RFC-011 Decision 2)

`resolve_server_flag` now treats a `--server` value containing `://` as a literal
base URL (trailing slash trimmed; `--graph` appends `/graphs/<id>`), bypassing the
operator-config `servers:` registry; a bare name still resolves through the
registry. This is the replacement the upcoming `--uri http(s)://` deprecation
points at, and a small ergonomic win on its own (`--server https://host` with no
config entry). Token resolution for a literal-URL server falls to the legacy
OMNIGRAPH_BEARER_TOKEN chain, same as a positional URL today.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(cli): address the parity-matrix arms with global --store/--server flags

Prep for removing the positional-http→remote dispatch. The parity harness
addressed both arms with a positional graph right after the verb
(`omnigraph <verb> <addr> <args…>`), which only parses for top-level verbs —
for nested subcommands (`schema show`, `branch list`, …) the address landed in
the subcommand slot and BOTH arms failed identically, so the test passed
vacuously (matching exit codes, never comparing output).

Address both arms with the global flags instead — local `--store <graph>`
(embedded), remote `--server <url>` (served) — appended after the verb + args,
valid regardless of nesting. The previously-vacuous nested-verb parity checks
now actually compare embedded vs remote (and pass — parity holds), and the
remote arm no longer relies on the positional-URL dispatch that's about to be
removed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(cli)!: --as on a served write is a hard error (was a silent no-op)

A served write resolves the actor server-side from the bearer token, so `--as`
could never set identity there — it was silently ignored. It now errors (in the
remote write factory, before any HTTP call), pointing the user at removing `--as`
or writing directly with `--store`. Reads don't carry `--as`, so this is
write-path only. BREAKING for any script that passed `--as` to a remote write
(it was a no-op, so behavior is unchanged except the now-explicit error).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(cli)!: a positional/--uri http(s):// URL no longer dispatches to a server

Remote graphs must be addressed with `--server <url>` (or a named server / a
profile binding one). A positional or `--uri` `http(s)://` URL on a data verb now
errors instead of silently routing to the remote HTTP client — the scheme no
longer carries transport semantics. The discriminator is `via_server`: a remote
URL produced by a server scope is fine; a remote URL from a positional/`--uri`
source is rejected (`reject_positional_remote` in both GraphClient factories).

Storage verbs are unaffected — they already reject remote URIs through
`resolve_local_graph` with the existing "direct (storage-native)" error.

Migrated the gh-host keyed-credential system test to `--server <url>` (the literal
URL still prefix-matches the operator server for token resolution). BREAKING:
scripts addressing a server by a bare URL must switch to `--server <url>`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(cli)!: remove the --target flag (use --store / --profile / --server)

Removes the legacy named-graph flag and threads its parameter out of the whole
resolver chain. `--target` resolved a graph name through `omnigraph.yaml`'s
`graphs:` map; its replacements (`--store <uri>`, `--profile <name>`,
`--server <name>`) all ship.

- Drops the 22 `target` clap fields + the `--cluster` exclusion that named it.
- Threads `target`/`cli_target` out of `resolve_uri`/`resolve_cli_graph`/
  `resolve_local_graph`/`resolve_local_uri`/`resolve_storage_uri`/
  `resolve_remote_bearer_token`/`apply_server_flag`/`execute_query_lint`/
  `resolve_selected_graph`/`resolve_registry_selection_for_list`/
  `execute_queries_{validate,list}`, the two `GraphClient` factories, and
  `ScopeFlags`/`ResolvedScope`.
- Keeps the shared `OmnigraphConfig::resolve_target_uri` 3-arg (server boot uses
  it); the CLI passes None for the explicit-target arm. The `cli.graph` default
  (omnigraph.yaml bare-command fallback) is unchanged — its removal belongs to
  the omnigraph.yaml excision.
- Operator/file aliases that bind a `graph` name still work: the name is now
  resolved to a URI inline (a positional URI wins).
- Error messages and `--graph`/`--server`/`--store` help text no longer name
  `--target`; the queries-list selection hint points at `cli.graph`.

BREAKING. Tests updated (named-target resolution rewritten onto `cli.graph`;
positional-URI tests unchanged). Full omnigraph-cli suite green (228).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(cli): drop --target and positional-http addressing; --as-on-served is an error

Update the user docs for the legacy data-plane addressing removals:
- the CLI `--target` flag is gone — address graphs with a positional URI,
  `--store`, `--profile`, or `--server <name|url>`;
- a positional `http(s)://` URI no longer dispatches to a server (use `--server`);
- `--as` on a served write is now rejected (was a silent no-op).

Touches cli/reference.md (addressing intro, capability table, error examples,
scopes), cli/index.md (the remote-read example → --server), operations/maintenance
+ policy, and the cluster docs' data-plane load guidance. The server's own
`--target` boot flag is unchanged (server.md untouched). Also fixes a pre-existing
broken maintenance link in search/indexes.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(cli): --store is loudly exclusive with a positional URI / --server; test graphs→Served

Address two Greptile findings on the RFC-011 slices:
- Slice A (P1): `--store` combined with a positional URI silently dropped the URI
  (`scope.rs` did `store.or(uri)`); `--store` + `--server` errored with a
  misleading "positional URI" message. Now both combinations fail loudly with a
  declared `--store is exclusive with a positional URI and --server` error.
- Slice B (P2): the `command_capability` unit test never exercised the one
  Data→Served refinement (`graphs`); added the assertion so deleting that guard
  can't pass silently.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-15 04:29:16 +03:00
Andrew Altshuler
7eeced3e88
feat(cli): RFC-011 Slice B — capability vocabulary (any/served/direct/control/local) (#237)
* feat(cli): RFC-011 Slice B — capability vocabulary (any/served/direct/control/local)

User-facing CLI errors and --help now speak a single "capability" vocabulary —
what a command needs — instead of the internal four-plane jargon. Behavior is
unchanged: the --server/--graph allow set is identical (the served-graph
capabilities `any` ∪ `served` = the old `Data` plane, since `graphs` was already
allowed). Only error text and the --help legend change.

- planes.rs: add `Capability { Any, Served, Direct, Control, Local }` derived from
  the existing exhaustive `command_plane` classifier (which stays as the drift
  guard) plus the one Data→Served refinement (`graphs`). `guard_addressing` now
  allows `--server`/`--graph` on `{Any, Served}` and rejects elsewhere with a
  capability-worded message. The mapping reflects *current* behavior (`queries
  list` → Local, `queries validate` → Direct); it converges to the RFC end-state
  table when later slices re-route those verbs.
- scope.rs: `resolve_scope` takes `Capability` instead of `Plane`, so the whole
  addressing path speaks one vocabulary; call sites in client.rs (Any) and the 3
  maintenance verbs in main.rs (Direct) updated.
- helpers.rs: the storage-direct remote rejection reworded to "direct
  (storage-native) command".
- cli.rs: the --help legend is now "COMMANDS BY CAPABILITY".
- Tests: the 5 assertions pinning the old plane text updated; added planes.rs unit
  tests proving the allow set is exactly {Any, Served} (behavior-preservation),
  the per-verb mapping, and distinct capability phrases.

Full omnigraph-cli suite: 225 green (222 + 3 new), zero behavior-test changes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(cli): capability vocabulary in the CLI reference + maintenance addressing

Rename the reference's "Command planes" section to "Command capabilities"
(any/served/direct/control/local), reword the error examples, and update the
maintenance doc's addressing note + its section cross-link to match Slice B.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-15 03:02:07 +03:00
Andrew Altshuler
a4d08a4184
feat(cli): RFC-011 Slice A — additive scope/profile addressing (#235)
Some checks failed
CI / Classify Changes (push) Has been cancelled
CI / Check AGENTS.md Links (push) Has been cancelled
CI / Container Entrypoint (push) Has been cancelled
Release Edge / Prepare edge release (push) Has been cancelled
CI / Test Workspace (push) Has been cancelled
CI / Test omnigraph-server --features aws (push) Has been cancelled
CI / RustFS S3 Integration (push) Has been cancelled
Release Edge / Build edge omnigraph-linux-x86_64 (push) Has been cancelled
Release Edge / Build edge omnigraph-macos-arm64 (push) Has been cancelled
Release Edge / Build edge omnigraph-windows-x86_64 (push) Has been cancelled
Release Edge / Smoke Windows installer (push) Has been cancelled
* feat(cli): RFC-011 Slice A — operator-config scope structs (profiles/clusters/defaults)

Additive operator-config surface for the RFC-011 scope model. No behavior
change yet — these structs are parsed but not consumed until the scope
resolver lands.

- OperatorConfig gains `profiles:` (name → OperatorProfile) and `clusters:`
  (name → OperatorCluster { root }) — the latter the only place a storage
  root appears in operator config (RFC-011 storage-root rule).
- OperatorDefaults gains `server` and `default_graph` (the flat-default scope).
- OperatorProfile binds one of {server, cluster, store} + default_graph;
  `binding()` validates exactly-one on use and returns a ScopeBinding.
- Accessors profile()/cluster_root()/default_server()/default_graph();
  unknown-key warnings extended to the new blocks (forward-compat preserved —
  old configs still load, new keys are no longer "unknown").

Tests: parse profiles/clusters/scope-defaults, binding rejects zero/multiple
entities, unknown keys in a profile warn.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(cli): RFC-011 Slice A — scope resolver + --profile/--store, wired (additive)

Translate the new scope inputs into the existing addressing tuple, in front of
the unchanged resolvers. Purely additive: an explicit address
(--uri/--target/--server/--store) passes straight through, so every existing
invocation is byte-for-byte unchanged.

- scope.rs: resolve_scope() with the RFC-011 precedence (explicit > --profile /
  OMNIGRAPH_PROFILE > flat defaults.server), producing the effective
  (server, graph, uri, target) for data verbs and (cluster, cluster_graph) for
  maintenance. Plane×scope capability check (server scope rejected on a
  maintenance verb; cluster scope rejected on a data verb; store rejects --graph)
  fires only on the new paths. 9 unit tests.
- cli.rs: global --profile <NAME> and --store <URI>. (--graph keeps
  requires=server for now; profile/default graph comes from default_graph —
  profile+--graph override is deferred to the --cluster-graph rework.)
- client.rs: the two GraphClient factories call resolve_scope (Plane::Data) up
  front; the explicit branch reproduces today's behavior exactly.
- main.rs: the 15 data call sites forward --profile/--store; the 3 maintenance
  verbs consult the scope (Plane::Storage) only when no explicit per-command
  address is given, so cluster-binding profiles and --store reach
  optimize/repair/cleanup.

Verified: the full omnigraph-cli suite (221 tests) stays green untouched.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test+docs(cli): RFC-011 Slice A — end-to-end scope test + reference docs

- cli_data.rs: prove --store and a --profile store binding drive a read
  identically to the legacy positional URI (the additive-coexistence contract),
  end to end against a local graph (no server needed).
- cli/reference.md: document profiles/clusters/defaults.server/default_graph,
  the --profile/--store flags, and a "Scopes & profiles" section; note the model
  coexists with legacy addressing (nothing removed yet).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-15 02:37:55 +03:00
Andrew Altshuler
ceb37dd4cb
fix(engine): close the 2 Lance 7.0.0 alignment failures (immutable PK + native namespace) (#236)
* fix(engine): make the v1→v2 manifest migration idempotent under Lance 7's immutable unenforced primary key

Lance 7 (dataset/transaction.rs) makes the unenforced primary key immutable
once set: any write touching the reserved `lance-schema:unenforced-primary-key`
field metadata after the PK is set errors "cannot be changed once set" — even
re-applying the same value. `migrate_v1_to_v2` previously relied on the old Lance
6 idempotency (re-applying the annotation was a no-op-ish bump), which it needs
for crash-recovery: a v1 graph that crashes after the field-set but before the
stamp bump re-enters the migration with the PK already present.

Under Lance 7 that re-entry now errors, so a real pre-v0.4.0 graph crashing in
that window could never complete its migration. Guard the field-set with
`schema().unenforced_primary_key().is_empty()` so a genuine first-set still runs
but a re-set is skipped — restoring crash-idempotency by construction. (Fresh
graphs bake the PK into manifest_schema() at init and never run this migration.)

The existing test_publish_migrates_pre_stamp_manifest_to_current_version is the
regression guard: red under Lance 7 before this change, green after.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(engine): realign the native-namespace surface guard to Lance 7 (TableNotFound)

`test_directory_namespace_direct_publish_cannot_replace_native_omnigraph_write_path`
pokes Lance's NATIVE DirectoryNamespace (not omnigraph's production write path,
which is the manifest merge_insert publisher) to document that it cannot replace
omnigraph's authority.

Lance 7's DirectoryNamespace routes list/describe/create_table_version through
`check_table_status`, which now reports an omnigraph-manifest-tracked table as
absent — so all three return TableNotFound for `node:Person` (observed). The
native namespace is now fully decoupled from omnigraph's manifest: it cannot
enumerate, inspect, or publish over omnigraph's tables. This strengthens the
guard's thesis. Realigned the assertions to the v7 behavior and kept the
authority check (omnigraph's refresh ignores the direct append; row_count stays
0). Test-only; no production impact.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(lance): document the 2 runtime behavior changes in the 7.0.0 alignment stanza

The #229 stanza verified a clean engine *build* but not the test suite, and
claimed "no Lance API surface omnigraph uses changed." Two runtime behaviors did,
caught only by the full test suite:

- the unenforced primary key is immutable once set in v7 (transaction.rs) — broke
  the v1→v2 manifest migration's crash-idempotency; fixed by an is-set guard;
- the native DirectoryNamespace returns TableNotFound for omnigraph
  manifest-tracked tables (dir.rs) — test-only; the surface guard was realigned.

Corrects the over-broad "no surface changed" claim, adds both findings, and notes
the lesson: a clean build is not a clean alignment — run cargo test --workspace
before declaring a Lance bump done.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-15 02:37:38 +03:00
Ragnor Comerford
67baf615d9
build(deps): bump Lance 6.0.1 → 7.0.0 (correct-by-design substrate alignment) (#229)
* build(deps): bump Lance 6.0.1 → 7.0.0 (object_store 0.13.2, roaring 0.11.4)

Arrow stays 58 and DataFusion stays 53 (no change). The only transitive bump
is object_store 0.12.5 → 0.13.2. 141 upstream commits reviewed; no fixes lost
(the 6.0.x release-branch backports are all forward-ported into 7.0.0).

- object_store 0.13 moved get/put/head/rename/delete behind a new ObjectStoreExt
  trait (list/list_with_delimiter/put_opts stay on the core trait). Add
  `use object_store::ObjectStoreExt` in storage.rs and db/manifest/namespace.rs;
  no call-site changes. Mirrors Lance's own migration in PR #6672.
- roaring pinned to 0.11.4 (cargo update -p roaring --precise 0.11.4). Lance
  7.0.0's UpdatedFragmentOffsets newtype (lance#6650) derives Eq over
  HashMap<u64, RoaringBitmap>, which needs RoaringBitmap: Eq, added in roaring
  0.11.4; the loose `roaring = "0.11"` constraint otherwise resolves 0.11.3 and
  lance itself fails to compile.
- lance#6774: merge-insert INSERT rows now stamp _row_created_at_version with the
  commit version (was a fallback of 1). Flip the lance_version_columns assertion
  to `== v2` and correct the changes/mod.rs rationale comment. Production
  change-detection keys on _row_last_updated_at_version + ID membership, so its
  logic is unaffected.

Refs lance#6650, lance#6774, lance#6672.

* fix(storage): pin WriteParams::auto_cleanup = None (lance#6755 default flip)

lance#6755 flipped the WriteParams::auto_cleanup default from on (a full cleanup
pass every 20th commit) to None. On 6.0.1 the on-by-default hook could silently
GC versions that __manifest pins for snapshots/time-travel. OmniGraph owns
cleanup explicitly (optimize.rs::cleanup_all_tables) and never set auto_cleanup,
so it was relying on a default that is both wrong for our snapshot model and now
changed upstream.

Pin auto_cleanup: None explicitly at all 11 production WriteParams sites
(table_store ×6, commit_graph ×2, recovery_audit ×1, manifest/graph ×2 — the
__manifest + sub-table Create paths). Removes the dependency on a default-flag
value and locks in the snapshot-safe behavior regardless of future upstream
re-flips.

Refs lance#6755.

* test(lance): pin BTREE range-boundary correctness (lance#6796)

lance#6796 (issue #6792) fixed a BTREE scalar-index range-query bound
inclusiveness bug: `x <= hi AND x > lo` returned the wrong boundary row.
Add lance_surface_guards.rs::btree_range_query_boundary_is_correct, which
reproduces the exact #6792 shape (5 rows + an explicit BTREE drives the index
path even on tiny data) and pins the corrected inclusive-<= / exclusive->
semantics. It turns red if a future Lance regression reintroduces the bug.

OmniGraph today builds BTREE only on string @key columns and queries them by
equality/IN, so its current patterns do not hit this; the guard protects any
future BTREE-range path (BTREE-on-properties, range-on-key).

Refs lance#6796.

* docs(dev): align Lance docs + invariants to 7.0.0

- docs/dev/lance.md: new 2026-06-14 alignment stanza for the 6.0.1 → 7.0.0 bump
  (object_store ObjectStoreExt move, roaring 0.11.4, #6774/#6796/#6755 behavior,
  #6658 shipped → MR-A unblocked but separate, #6666 + blob compaction still
  open); prior 6.0.1 stanza demoted to historical.
- AGENTS.md: storage substrate 6.x → 7.x (line + architecture diagram).
- docs/dev/invariants.md: deletes/vector known gap updated — the staged
  two-phase delete API (lance#6658) now exists and MR-A is unblocked, but
  delete_where stays inline and D2 stays in place until the migration lands;
  create_vector_index still gated on lance#6666.

* fix(storage): skip Lance auto-cleanup on commit paths for legacy datasets

Addresses PR #229 review (Codex P1). `WriteParams::auto_cleanup` is create-time
config with no effect on existing datasets (Lance write.rs docs), so the previous
`auto_cleanup: None` change alone did NOT protect graphs created before the v7
bump: 6.0.1 defaulted auto_cleanup ON, leaving `lance.auto_cleanup.*` config on
those datasets, and Lance's per-commit hook (io/commit.rs: `if
!commit_config.skip_auto_cleanup`) fires off that stored config — so omnigraph's
own writes would GC versions the __manifest pins for snapshots/time-travel.

Skip the hook on every commit path, covering new and legacy datasets alike:
- commit_staged: CommitBuilder::with_skip_auto_cleanup(true) — the staged data path.
- __manifest publisher: MergeInsertBuilder::skip_auto_cleanup(true).
- all 11 WriteParams: skip_auto_cleanup: true (direct Dataset::write/append paths;
  auto_cleanup: None retained so new datasets store no cleanup config at all).

Tests:
- lance_surface_guards::skip_auto_cleanup_suppresses_version_gc — substrate:
  negative control (config GCs v1 without skip) + with-skip survival.
- staged_writes::commit_staged_skips_auto_cleanup_so_pinned_versions_survive —
  omnigraph usage: commit_staged on a legacy-config dataset preserves the pinned
  create version.

Refs lance#6755.

* test(lance): assert created_at-preserved + updated_at-bumped on merge_insert UPDATE

Addresses PR #229 review follow-up. `lance_merge_insert_update_preserves_created_at_version`
documented (in a comment) that a merge_insert UPDATE preserves created_at and
bumps updated_at, but only asserted the value change — leaving the change-feed
invariant unguarded. Add the two missing assertions:

- bob created_at == v1 (preserved across UPDATE; what the test name promises;
  lance#6774 only changed INSERT-row stamping).
- bob updated_at == v2 (bumped to the commit version) — the invariant
  OmniGraph's insert/update classification relies on (changes/mod.rs keys on
  _row_last_updated_at_version). A regression here would silently drop updates
  from the diff/change feed.
2026-06-14 20:42:24 +02:00
Ragnor Comerford
7963499995
fix(cli): unify remote URL builder, fix branch delete //branches 404 (#230)
* test(cli): reproduce branch-delete //branches 404 (failing)

Regression test for the `branch delete` 404 over a multi-graph
`--server`/`--graph` target: the composed URL must be `<base>/branches/<name>`
with no empty `//` segment. Fails against the current `remote_branch_url`,
which appends a trailing slash before extending path segments and so emits
`…/graphs/p9-os//branches/tmpbranch`. The next commit fixes it.

  left:  "http://host/graphs/p9-os//branches/tmpbranch"
  right: "http://host/graphs/p9-os/branches/tmpbranch"

* fix(cli): unify remote URL builder, close the //branches 404 class

Correct-by-design fix for the failing test in the previous commit. The bug was
not specific to `branch delete`: URL assembly was scattered across a
string-concat `remote_url`, a url-crate `remote_branch_url`, and several
`format!` interpolations that left dynamic path/query components un-encoded
(commit id in the path, branch in the query string). `branch delete` was the
instance that surfaced because it is the only verb that puts a dynamic value in
the path.

Replace both helpers with one `remote_url(base, segments, query)` that every
remote call routes through. Callers pass structured segments and query pairs,
so trailing-slash normalization (pop_if_empty) and per-segment / per-value
percent-encoding live in one place. A stray `//` or an un-encoded dynamic
component is no longer representable, closing the whole class rather than the
reported instance.

Migrates the previous commit's failing test to the new builder and adds the
single-graph, trailing-slash, slash-in-name, commit-id-path, and query-value
cases (the last two cover the previously latent siblings). All 16 callsites
migrated; `remote_branch_url` removed.
2026-06-14 20:37:12 +02:00
Andrew Altshuler
6fa04efc76
docs(rfc): RFC-011 — rename "context" → "profile" (#233)
"context" (borrowed from kubectl) collides with agent/LLM "context". Rename the
named-operator-defaults concept to "profile" (AWS-CLI precedent: --profile,
OMNIGRAPH_PROFILE, profiles:). Doc-only — the concept was unshipped. "scope" (the
resolved addressing target a profile populates) is unchanged.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-14 21:23:39 +03:00
Andrew Altshuler
35e87ab882
docs(rfc): RFC-011 — CLI refactoring (one addressing & config model) (#228)
* docs(rfc): RFC-011 — CLI refactoring (one addressing & config model)

A maintainer-internal RFC (Status: Proposed) for the post-omnigraph.yaml CLI:
one ontology (store/server/cluster; cluster vs operator config; catalog; context;
capability); addressing = scope + --graph with the access path *derived*; served
is the default front door and direct storage is privileged (admin/break-glass);
stateless per command; definitions named, payloads passed. Includes the full
end-state command taxonomy (by capability), a current-state appendix, migration,
invariants check, and the resolved Decisions (with two deferred).

Completes the config/CLI lineage RFC-007 → RFC-008 → RFC-009 → RFC-010.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(rfc): RFC-011 — address Greptile review (4 doc fixes)

- P1: end-state taxonomy `schema apply` annotation said "Open Q10" — now points
  at the resolved Decision 10 (cluster graphs via cluster apply).
- P1: add the `alias <name>` verb (Decision 4) to the end-state taxonomy's local
  section — it was claimed "full command set" but omitted.
- P2: Decision 11's bulk-data-plane reference now carries the "PR #219, not yet
  merged" caveat (matches the Relationship section).
- P2: footnote now states the `check`→`lint` argv-shim is removed (its end-state
  disposition was unspecified).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-14 20:21:23 +03:00
Ragnor Comerford
1bed998052
fix(engine): scalar index coverage + filter literal coercion (query latency) (#216)
* fix(engine): lower date/datetime filter literals as typed Arrow scalars

`literal_to_expr` lowered `Date`/`DateTime` query literals as Utf8 strings,
relying on DataFusion implicit casts. Against a physical `Date32`/`Date64`
column that can coerce the column side (`CAST(col AS Utf8)`), which defeats a
scalar BTREE and degrades the scan to a full filtered read. Lower to typed
`Date32`/`Date64` scalars instead (reusing the loader's
`parse_date32_literal`/`parse_date64_literal`, already used by the in-memory
comparison arm), so the predicate stays a direct column comparison and the
index is used. Malformed literals fall back to the Utf8 string so pushdown
behavior never regresses.

Tests: unit goldens asserting the lowered literal is typed (red before, green
after) + inline-binding pushdown equality in literal_filters confirming the
epoch conversion selects the right rows.

* fix(engine): build scalar BTREE for enum and orderable-scalar @index columns

`build_indices_on_dataset_for_catalog` only handled `String` (-> FTS) and
`Vector` (-> vector). Enums are physically `String`, so an enum `@index`
column (e.g. `status`) got an FTS inverted index, which Lance never consults
for `=`; and `DateTime`/`Date`/numeric/`Bool` `@index` columns fell through
and built nothing. Both meant equality/range filters degraded to full scans
with `indices_loaded=0`.

Dispatch index kind by property type via a shared `node_prop_index_kind`:
enum + orderable scalar -> BTREE, free-text String -> FTS, Vector -> vector,
list/Blob -> none. The helper is shared by the builder and
`needs_index_work_node` so they cannot drift — the latter decides recovery-
sidecar pinning, and under-reporting would leave a HEAD-advancing index build
uncovered (invariant 5).

Tests: scalar_indexes.rs asserts enum/DateTime/numeric @index columns report
`IndexCoverage::Indexed` while free-text String/un-annotated columns stay
`Degraded` (negative control). Docs: docs/user/indexes.md.

* feat(engine): reindex in optimize to keep index coverage current

A scalar/FTS/vector index only covers the fragments it was built over. Rows
appended after the build (e.g. `ingest --mode merge`, whose commit does not
rebuild an existing index) are scanned unindexed, and `compact_files` rewrites
fragments out of coverage. Nothing folded them back in, so coverage decayed as
the graph grew — even the id/src/dst BTREEs that power traversal.

`optimize_one_table` now runs Lance `optimize_indices` after `compact_files`
(incremental merge, not retrain — the same compact->optimize_indices sequence
LanceDB's `optimize()` uses) and enters the publish path on compaction work OR
stale index coverage (new `TableStore::has_unindexed_fragments`, reusing the
fragment_bitmap logic). `optimize_indices` is a committing call with no
uncommitted variant in lance-6.0.1, so it is an inline-commit residual covered
by the existing `SidecarKind::Optimize` recovery sidecar spanning both ops.
Blob-bearing tables are still skipped (the Lance blob-compaction bug is
compaction-specific; reindex-for-blob deferred as a noted follow-up).

Tests: maintenance.rs asserts an appended fragment is uncovered before and
covered after optimize, and idempotency holds (second pass is a no-op).
lance_surface_guards pins the `optimize_indices` signature and its incremental-
coverage behavior. The existing optimize Phase-B recovery failpoint now also
exercises a crash after reindex. Docs: maintenance.md, writes.md, invariants.md,
lance.md, AGENTS.md.

* fix(engine): coerce pushdown filter literals to the column type

Filter literals were pushed to Lance in their natural Arrow type (every integer
Int64, every float Float64). Against a narrower indexed column DataFusion widens
to the literal's type and casts the COLUMN (`CAST(n32 AS Int64)`), which defeats
the scalar BTREE and degrades to a full filtered read. A physical-plan probe
confirms it: an Int32 column filtered by an i32 literal uses `ScalarIndexQuery`;
by an i64 literal it does not.

Thread the scan's `arrow_schema` through `build_lance_filter_expr` ->
`ir_filter_to_expr` and coerce each literal operand to the opposite column's
exact Arrow type, reusing `projection::literal_to_array` + `arrow_cast` (the same
path the in-memory arm uses, so the two arms agree). Coercion never demotes a
filter to None: on failure it falls back to the natural literal, because a node
scan has no in-memory fallback for inline filters.

Supersedes the date-specific change in e4ef67b (PR1): the probe shows dates were
never index-defeated — temporal coercion casts the LITERAL, not the column — so
PR1's index-use rationale was wrong though harmless. The generic coercion
subsumes it; `literal_to_expr`'s date arms revert to the natural Utf8 fallback,
and its unit tests now assert the live coerced path.

Tests: surface guard `scalar_index_use_requires_matched_literal_type` pins the
substrate behavior (matched -> index, widened -> column-cast full scan); unit
tests cover Int32/UInt32/Float32 coercion, range op, reversed operand order, and
the natural fallback; `literal_filters` adds an I32 column with equality + range
and an F32 pushdown case.

* fix(engine): only coerce filter literals when the cast is lossless

The literal coercion in f064121 narrowed unconditionally. typecheck permits
numeric cross-type comparisons (`types_compatible`), so an out-of-domain literal
reaches `literal_to_typed_expr` and casts lossily: a fractional float vs an
integer column truncates (`{ count: 2.7 }` -> `count = 2`, wrongly matching the
count=2 row) and an out-of-range integer overflows to null (`count < 3e9` on I32
-> `count < NULL` -> empty). Both silently change results, and a node scan has no
in-memory fallback for inline filters.

Add a lossless guard for integer targets: round-trip the cast back to the natural
type and, on mismatch, return None so the caller keeps the natural literal
(correct via DataFusion coercion; the index is just unused for that out-of-domain
predicate). Float targets stay coerced -- narrowing F64 -> F32 is the column's own
precision domain, not a value error.

Resolves the two valid review findings on PR #216 (Codex float truncation, Greptile
out-of-range). Tests: unit cases for fractional/out-of-range fallback vs
whole-float/in-range coerce vs F32 exemption; e2e `{ count: 2.7 }` returns no rows.
2026-06-14 16:31:19 +02:00
Andrew Altshuler
77dffdae92
docs(user): de-dev polish — strip internal scaffolding from user docs (Phase 3a) (#226)
Remove developer-only scaffolding that leaked into the public user/operator
docs, while preserving every user-facing behavior, command, flag, endpoint,
constant, and env var. No behavior changes.

Removed across 18 files:
- internal ticket / sequencing refs (MR-NNN, RFC-NNN, "Phase N");
- source-code paths (crates/**/*.rs, *.pest) and internal struct/function
  dumps (e.g. the QueryIR / GraphCommit / SchemaMigrationPlan Rust types,
  internal fn names like fork_branch_from_state, optimize_all_tables);
- Lance-internal blocker prose (upstream issue numbers, blob-decode cause,
  sidecar Phase-B/C mechanics) — keeping the user-visible behavior (e.g.
  "optimize skips Blob-column tables; reads/writes unaffected");
- pre-v0.4.0 Run-state-machine archaeology.

Internal IR/lowering/recovery-internals sections were either trimmed to a
brief user-facing note (e.g. "Traversal execution", "interrupted writes
recover automatically; recovery commits are recorded under actor
omnigraph:recovery") or removed.

Kept: all language syntax, lint codes, Cedar actions/scopes, endpoints,
error taxonomy, every constant and env var (verified none dropped from the
constants cheat-sheet), and the operator-facing explanations of on-disk
artifacts. Residual "legacy" mentions are all user-facing (the deprecated
omnigraph.yaml, the legacy token chain, old command names).

Verified: zero internal-scaffolding leaks (MR/RFC/Phase/.rs/.pest = 0) across
docs/user; zero broken links; check-agents-md.sh green.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-14 14:39:25 +03:00