[ragnorc]

What & why

omnigraph embed embedded type: <Type>\n<field>: <value> for each spec field, while a text nearest($v, $q) embeds $q verbatim. A record whose @embed source equals the query text therefore did not rank at distance zero: under the mock provider the vectors were unrelated, and under a real model the template was a systematic skew between stored and query vectors. This PR makes the offline pipeline embed exactly the @embed source value, which is what the query path already does.

Reproduction with the debug CLI and OMNIGRAPH_EMBEDDINGS_MOCK=1 (schema embedding: Vector(1536)? @embed("title"), rows "Fix login timeout" and "Paint the bikeshed", query nearest($i.embedding, "Fix login timeout")):

	login	shed
before	2.0027 (ranked second)	1.9704
after	0.0 (ranked first)	2.0363

Which side changes

The pipeline, not the query:

• @embed("source") declares vector = embed(source), with exactly one String source property (the parser refuses anything else; RFC 0015 states the derivation).
• A query string has no type or field to put into a template, and vectors that users supply themselves (which the docs allow) never carried one.
• The one intended difference between the two sides is the provider's retrieval role (Gemini's RETRIEVAL_DOCUMENT / RETRIEVAL_QUERY), which EmbedRole already carries.

The template dated from the initial public commit, with no stated reason.

Behavior changes in omnigraph embed

• The source value is embedded unchanged, including whitespace (the old path also trimmed it).
• A spec's fields must name exactly one property, the @embed source. Lists with none or several are refused when the spec or seed manifest is read, because no @embed declaration can describe them. The spec is validated by serde(try_from), so a resolved EmbedSpec cannot hold more than one field.
• A processed record whose source is absent, null or blank gets no vector. Before, it got a vector of type: <T> alone, a placeholder roughly equidistant from every query.
• Whatever such a record's vector property held is removed and counted in cleaned_records. That covers a stale vector on --reembed-all or --select (so a model switch cannot leave old-model vectors behind), and a null or [] placeholder in any mode ([] used to pass through and then fail the load).
• A source value that is not a string is refused with its line number.

changelog.d/embed-exact-source-text.breaking.md tells users to regenerate vectors written by earlier builds with --reembed-all and covers the one-field rule. .fixed.md describes the fix.

Backing issue / RFC

• Fixes an accepted issue: no issue exists yet for this defect; it was found while building a beads-style example client on OmniGraph. I can open one if a maintainer wants it to back the PR.
• Follows the derivation RFC 0015 declares for @embed (docs/rfcs/0015-ingest-embeddings.md, "Embeddings are derived state, keyed off @embed"), though RFC 0015 is a draft.
• Trivial fast-lane

Checklist

• Change is focused (one logical change)
• Tests added/updated for behavior changes
• Public docs updated: docs/user/search/embeddings.md (the text rule and the spec format), docs/dev/execution.md, skills/omnigraph/references/search.md
• Reviewed against docs/dev/invariants.md: invariant 8 (no placeholders: a record without source text no longer gets a type-name vector; a spec that cannot match @embed fails loudly). No write-path, publication or substrate change.

Local verification

• cargo test -p omnigraph-cli --locked --bin omnigraph embed: 9 passed (4 new). Baseline before the change: 6 passed.
• cargo test -p omnigraph-cli --locked --test cli_data embed: 5 passed, both before and after (the shared seed fixture's spec moved from fields: [slug, intent] to [intent]).
• Red check: with the template temporarily restored in embed_row, document_vector_is_the_query_vector_of_its_source_text fails (the two vectors differ). It passes without the template.
• cargo clippy -p omnigraph-cli -p omnigraph-engine --all-targets --locked -- -D warnings -W clippy::dbg_macro: clean.
• cargo fmt --all --check, python3 scripts/check-docs.py, bash scripts/check-agents-md.sh, typos on the changed files: clean. release_notes.py preview --working-tree renders both notes.
• The end-to-end reproduction above (embed, init, load, query), before and after.
• Not run locally: the canonical workspace graph and the GQT corpus; CI runs them.

Notes for reviewers

• Test tier: the GQT runner refuses @embed schemas because they resolve a provider from the process environment, so the regression test is the in-source unit test in crates/omnigraph-cli/src/embed.rs. It asserts that the vector embed_row writes equals EmbeddingClient::embed_query_text(source) under the mock provider, which ignores the retrieval role. The engine side, a string nearest embedding the raw query, is already pinned by crates/omnigraph/tests/search.rs::nearest_string_param_matches_explicit_vector_under_mock_embeddings.
• No issue_N name: no issue exists, so the tests carry no issue_N suffix and the body uses no closing keyword.
• Remaining gap (not in this PR): omnigraph embed still does not read the schema, so a spec that names the wrong source property or dimension is not caught. Deriving the spec from the schema's @embed declarations (and checking a recorded model= against the resolved provider, as the query path already does) is a separate follow-up.

[ragnorc]

Recommendation: request changes for the empty-vector case in the inline comment. The main source-text fix is sound.

This PR fixes a mismatch between stored embeddings and text searches. The old file command embedded a type name, field label, and trimmed value. A text query embedded the query string alone. The new command passes the source string unchanged to the shared embedding client. It also requires one source field and removes stale vectors when a selected record has no source text. Existing vectors need explicit regeneration; the default mode preserves them.

The general invariant is that the document input comes from the declared source property. The query caller and the shared client support this design. The change applies to every configured type and keeps meaningful whitespace. Query and document roles remain distinct for providers that use them. Equal vectors in the mock test do not establish equal vectors for every provider.

The design reduces long-term liability by removing the competing text template. The validated single-source type makes the supported input shape explicit. The new result enum records actual work for the summary; it adds no persistent state or public service API. This pattern can cover more types without per-type formatting rules. Its remaining flaw is the reuse of “needs embedding” as “safe to leave unchanged” in the no-source path. Those contracts differ for an empty array. Fix that boundary once rather than add another special-case rendering rule.

The tradeoff is a deliberate compatibility break for multi-field specs and previously generated vectors. Regeneration costs provider calls and a later load. The offline spec still duplicates source, target, and dimension from the schema, so operators must keep them aligned, as the updated guide explains. This PR remains a local file transformation. It adds no calls to the graph write path and changes no Lance storage format. I checked the relevant upstream index/vector documentation and the pinned Lance 11.0.0 implementation at ab6b5bbe46009ed78746b444df8db59a8bc5d842, including numeric nearest-query validation. The text mismatch belongs above that boundary.

Local validation used a clean isolated checkout, Rust 1.97.1, and a private reused build cache:

export RUSTUP_TOOLCHAIN=1.97.1
export CARGO_TARGET_DIR=/tmp/review-928/target
export RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable'
cargo test --locked -p omnigraph-cli --features omnigraph/failpoints,omnigraph-cluster/failpoints --bin omnigraph -j 3 embed -- --nocapture
cargo test --locked -p omnigraph-cli --features omnigraph/failpoints,omnigraph-cluster/failpoints --test cli_data -j 3 embed -- --nocapture
cargo test --locked -p omnigraph-cli --features omnigraph/failpoints,omnigraph-cluster/failpoints --bin omnigraph -j 3 embed::tests::document_vector_is_the_query_vector_of_its_source_text -- --exact --nocapture

• On head 542495623286928b552463aa4fece5dc8c48d536, the first two commands passed: 9 filtered unit tests and 5 filtered CLI tests. Four of the CLI tests cover embedding modes and record preservation.
• On parent 76ce6a0d9cc1c4e4ad693ee25c67c11e01d093ce, I temporarily placed the source/query equality regression in the existing unit-test owner. I adapted only the old private spec/call signature and omitted the new result-enum assertion. The third command ran one test and failed on the predicted vector mismatch for "Fix login timeout".
• Back on the clean reviewed head, the third command passed one test, including its whitespace-preservation case. This is behavioral failure before and success after, not a compile failure or an empty run.
• For the new defect, changing the stale target in a_record_without_source_text_keeps_no_vector to [] made that existing test fail: Unchanged versus Removed. The unchanged head binary also reproduced the full failure: one blank-source record with embedding: [] passed embed, then failed load with “expects 4 dimensions, got 0.” All temporary source/test edits were restored.

Documentation checks passed for 233 Markdown files and 54 AGENTS links; cargo fmt --all --check and git diff --check also passed.

A GQT test cannot cover this file command today. Its step set has no offline embedding operation, and the runner explicitly refuses @embed schemas and string nearest queries that need a provider. The existing Rust owner is the appropriate minimal regression here.

Exact-head CI passed the workspace, GQT, Clippy, format, and CI Gate checks. It was not fully green: RustFS failpoints stopped on a Docker registry authentication-endpoint timeout; Azure integration failed the deployment-status equality assertion in cluster_e2e_azure_live_apply_without_restart (in_progress: false versus true). These logs do not show a failure in the changed embedding path. I did not establish their root causes. Local checks used the mock provider; they make no claim about live-provider retrieval quality, ANN recall, or throughput.

[ragnorc]

Recommendation: request changes. The existing P2 empty-vector finding still reproduces on 99d09a4274a251347cefb82ef0b9b1e09bfa76ae. This follow-up checks the merge from main. The embedding implementation and its tests are unchanged from the previously reviewed 542495623286928b552463aa4fece5dc8c48d536. I found no additional blocking defect or optional improvement in the merge's effect on this feature.

This PR fixes inconsistent text inputs to embeddings. The old file command added type and field labels and trimmed the value. Text queries used the query string alone. The new command sends the source property unchanged through the shared client and requires one source field. This gives file-generated vectors the intended input. Existing vectors still need explicit regeneration; the default mode preserves them. The query caller and shared client retain separate query/document roles where a provider needs them.

The remaining defect violates the no-source output contract. The guard treats “needs embedding” as “safe to leave unchanged.” An empty array satisfies the first condition but is not a valid nullable vector. Missing, null, and blank sources reach this path in both fill-missing and explicit re-embedding. Clear the target at this shared boundary and derive the result from that cleanup. Extend the existing test with []; a separate workaround for each source shape would leave the cause intact.

The main design removes liability by deleting the competing text template. One validated source field and the shared client can serve five more types without five formatting rules. The result enum records work without adding persistent state. The tradeoffs remain: multi-field specs break, old vectors need regeneration and a later load, and the offline spec must stay aligned with the schema. Regeneration costs provider calls. This change remains a local file transformation, with no new service API or Lance format. I rechecked the unchanged Lance 11.0.0 pin (ab6b5bbe46009ed78746b444df8db59a8bc5d842) and numeric vector boundary against the upstream vector/index documentation. OmniGraph's strict loader rejects the bad array before storage.

Local validation on the exact new head used Rust 1.97.1 and a reused private build cache:

export RUSTUP_TOOLCHAIN=1.97.1
export CARGO_TARGET_DIR=/tmp/review-928/target
export RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable'
cargo test --locked -p omnigraph-cli --features omnigraph/failpoints,omnigraph-cluster/failpoints --bin omnigraph -j 3 embed -- --nocapture
cargo test --locked -p omnigraph-cli --features omnigraph/failpoints,omnigraph-cluster/failpoints --test cli_data -j 3 embed -- --nocapture
cargo test --locked -p omnigraph-cli --features omnigraph/failpoints,omnigraph-cluster/failpoints --bin omnigraph -j 3 embed::tests::document_vector_is_the_query_vector_of_its_source_text -- --exact --nocapture
cargo test --locked -p omnigraph-cli --features omnigraph/failpoints,omnigraph-cluster/failpoints --bin omnigraph -j 3 embed::tests::a_record_without_source_text_keeps_no_vector -- --exact --nocapture

• Clean head: 9 filtered unit tests and 5 filtered CLI tests passed. The source/query equality test also passed on its own, including preserved whitespace.
• Before-fix evidence is reused from the previous review: the third command on 76ce6a0d9cc1c4e4ad693ee25c67c11e01d093ce ran one transplanted test and failed on the predicted vector mismatch. Only its old private API calls were adapted. The regression and embedding code have not changed since that proof.
• Temporary minimal regression on the new head: replace [0.1, 0.2] with [] in the existing no-source test. The fourth command ran one test and failed with Unchanged versus Removed. The defect therefore has no after-fix pass yet. I restored the edit and reran the clean unit filter successfully.
• The clean head binary also reproduced the full user failure with one blank-source record: embed --input input.jsonl --output output.jsonl --spec spec.json --json exited 0 and kept embedding: []; load --direct --data output.jsonl --mode append <temporary-graph> exited 1 with “expects 4 dimensions, got 0.” The spec used one title source and dimension 4; the schema used Vector(4)? @embed("title"). Both used the mock provider.

GQT still cannot express this offline command. The current runner also refuses provider-dependent schemas. The existing Rust test is the minimal appropriate owner, including after the new served GQT runner arrived. Please include the empty-array regression with the fix and show its behavioral failure before and success after.

Exact-head CI Gate, workspace tests, ordinary/DST/served GQT, Clippy, and the storage integration checks now pass. Local documentation, AGENTS-link, format, and diff checks passed. Temporary source edits are restored. These checks use mock embeddings; they do not establish live-provider retrieval quality, ANN recall, or performance.

[ragnorc]

Recommendation: approve. The existing P2 empty-vector finding is fixed. I found no new blocking defect or optional code change. This is a COMMENT review because the authenticated account is the author.

Reviewed 6494c3766b6868339b515f60f9d06b1166f58639, including the fix since 99d09a4274a251347cefb82ef0b9b1e09bfa76ae.

The PR makes the offline embedding command use the declared source text unchanged. It removes the type/field template that differed from text-query input. The latest commit also removes any target value when a processed record has no source text. A blank-source record with embedding: [] previously passed embed and then failed load. It now leaves the command without that field and loads as a nullable vector. Existing vectors still require explicit regeneration to adopt the text fix.

The violated invariant was at the file-output boundary: a processed record without source text must not retain a derived value or invalid placeholder. “Needs embedding” describes input selection. It does not prove valid output. The new removal result supplies the cleanup decision directly. This covers missing, null and blank sources, with any target value, without a special branch for empty arrays. It also makes the cleanup count follow the actual removal.

I traced fill-missing, explicit re-embedding, selectors, type filters and clean mode. They share the existing selection and transformation owners. Default mode still preserves existing nonempty vectors. The revised guide correctly limits cleanup to processed records. Non-string sources remain errors. The shared client retains query/document roles for providers that distinguish them, so mock vector equality does not imply identical vectors from every provider.

The fix reduces liability: one map removal replaces a separate predicate and removal, and the existing regression expands in place. It adds no state, public API, migration or abstraction. Five more target shapes can use the same cleanup rule. The broader tradeoff remains explicit: multi-field specs break, old vectors need provider calls and a later load, and the offline spec must stay aligned with the schema's source, target and dimension.

Direction and other open PRs

I screened the scopes and changed files of all 29 open PRs. The relevant overlaps support this boundary:

• Search contracts #793 proposes schema-owned encoding recipes, including exact input, transforms and roles. Removing an implicit template fits that direction. The current offline spec remains a maintenance obligation until that broader contract is implemented.
• Plan acceptance #839 and planner diagnostics #815 govern query planning and retrieval policy. This fix stays in input preparation. The polymorphism draft #884 does not require another embedding path.
• Operator guides #892 touches the same two documentation files, but updates provider activation and bindings. Its changes and this PR's source-text/cleanup explanation should both remain. The storage, deployment, merge, lexical-search and test-corpus work adds no prerequisite for this local transformation.

I checked the repository guides, complete upstream index/vector pages, and the pinned Lance 11.0.0 source at ab6b5bbe46009ed78746b444df8db59a8bc5d842. Lance nearest validation receives numeric vectors. OmniGraph's strict loader rejects the empty array before storage. Newer Lance 14 beta search effort and auto-probing changes concern vector search. My assessment is that they do not remove the need for this source-text fix. No Lance upgrade, storage-format change or index workaround is needed here. I did not execute this PR against Lance 14.

Validation

The clean exact head passed nine tests in the unit embed filter and five in the CLI embed filter. Four of the latter cover embedding modes and record preservation.

export RUSTUP_TOOLCHAIN=1.97.1
export CARGO_TARGET_DIR=/tmp/review-928/target
export RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable'
cargo test --locked -p omnigraph-cli --features omnigraph/failpoints,omnigraph-cluster/failpoints --bin omnigraph -j 3 embed -- --nocapture
cargo test --locked -p omnigraph-cli --features omnigraph/failpoints,omnigraph-cluster/failpoints --test cli_data -j 3 embed -- --nocapture
cargo test --locked -p omnigraph-cli --features omnigraph/failpoints,omnigraph-cluster/failpoints --bin omnigraph -j 3 embed::tests::a_record_without_source_text_keeps_no_vector -- --exact --nocapture

For the third command, I copied only the head's expanded test into the same owner on parent 99d09a4274a251347cefb82ef0b9b1e09bfa76ae. One test ran and failed for { "slug": "i-1" } with target []: Unchanged versus Removed. On restored, clean head 6494c3766b6868339b515f60f9d06b1166f58639, the same test passed. No compile failure, skipped test or empty run is counted as the proof. All temporary test edits were restored.

The original source-text regression also passes in the current unit filter, including whitespace preservation. Its before-fix failure on 76ce6a0d9cc1c4e4ad693ee25c67c11e01d093ce is reused from the first review. That source-text implementation is unchanged by this follow-up.

I also ran the original one-record CLI example on the clean head:

omnigraph embed --input input.jsonl --output output.jsonl --spec spec.json --json
omnigraph init --direct --schema schema.pg graph.omni
omnigraph load --direct --data output.jsonl --mode append graph.omni

With the mock provider, title: " ", embedding: [], and Vector(4)? @embed("title"), all three commands exited 0. Embed reported one cleaned record, its output omitted embedding, and load accepted one entity.

GQT has no offline embedding step. Its runner also refuses provider-dependent schemas. The existing Rust owner is therefore the minimal appropriate regression.

Documentation checks passed for 238 Markdown files, along with AGENTS links, formatting and diff whitespace. Workspace CI passed 4,303 tests, with 32 skipped. Ordinary, served and DST GQT, Clippy and CI Gate are green. CI checked merge 3289fcb43d28790d43a1f32439902af1481ad40a, whose tree differs from the reviewed head. Those results are separate from the local exact-head tests. Live-provider retrieval quality, ANN recall and performance were not measured.