[ragnorc]

An outside review of the tree found four gaps that are documentation, not product: nothing about encryption at rest, R2 and GCS support claimed but never tested, and no statement of when an embedding provider is called or whether it can be switched off.

Changes

• New docs/user/operations/data-protection.md. OmniGraph does no client-side or field-level encryption. Encryption at rest comes from the store (S3 bucket default SSE-S3/SSE-KMS, Azure Storage default, an encrypted volume). The server speaks plain HTTP behind a TLS proxy. Text goes to the embedding provider only. Deletes and drops leave values in history until cleanup. It is linked from the user index and deployment.md. deployment.md stays at the 350-line cap, so the page holds the content.
• storage.md. gs:// is refused and has no native GCS support. A store must enforce If-None-Match: * and If-Match on PUT. CI tests s3:// against RustFS. R2 and GCS through its S3 API are not qualified.
• embeddings.md, new "Network egress" section. Only a text nearest query and omnigraph embed call a provider. A served graph without embedding_provider resolves the provider from the process environment, which defaults to OpenRouter. No setting turns providers off, and mock returns vectors that are not comparable with real ones.
• README. The capability row and quickstart no longer list R2/GCS as supported. They point at the conditional-write requirement instead.

Verified against code

• Per-request SSE settings are refused on CLI targets: CapturedStorageBinding::validate_environment refuses any recognized non-auth S3 option that is not in the captured map, and the capture never adds encryption keys.
• gs:// is refused by the storage URI parser, and Lance is built with aws and azure only.
• The S3 binding sets ConditionalPut=etag. The cluster's lock and control objects use If-Match, and Lance commits use put-if-absent.
• Embedding calls exist only in engine/search.rs (query text) and omnigraph-cli/src/embed.rs. settings.rs leaves embedding: None without a profile, and EmbeddingResolver then calls EmbeddingClient::from_env().

Classification

Trivial fast-lane (doc corrections). No behavior change.

Testing

bash scripts/check-agents-md.sh, python3 scripts/check-docs.py, typos.

[ragnorc]

Recommendation: request changes. Do not merge until the three sentences below match the code.

Why

An outside review found four gaps in the deployment docs. This pull request adds one data-protection page and corrects the storage and embedding pages. Operators need those limits before they choose a store or turn embeddings on.

What

The new page is the right owner for encryption, transit, egress, and erasure. The storage page correctly says OmniGraph refuses gs://. It correctly says CI qualifies s3:// only against RustFS. The embedding page correctly says a provider is called only for a text nearest and for omnigraph embed. A missing API key fails before any HTTP request on the environment path.

Three sentences still disagree with the code. Those three are blocking.

Blocking

1. Encryption environment variables. Named CLI targets refuse AWS_SERVER_SIDE_ENCRYPTION and AZURE_STORAGE_ENCRYPTION_KEY. A server process and a direct s3:// or az:// open do not use that captured binding. Lance 13.0.0 merges the process environment into the store. Those variables are then sent as request headers. The page must describe both paths. Bucket default encryption still applies when the variables are unset.

2. Mock model rule. A text nearest is refused when the active model id differs from the model recorded by @embed. OMNIGRAPH_EMBED_MODEL is only the fallback for a mock client built from the environment. A cluster profile kind: mock with model: sets the active model. The same page already states the correct rule under Schema annotation. Keep that one rule.

3. External services. omnigraph cluster push uploads the cluster config files, including schema, query, and policy text, to POST /v1/clusters/{id}/config. Stored graph rows are not in that body. The embedding path sends query text and the source values of omnigraph embed. The sentence "the only external service" is too broad.

Optional

• The README capability cell still says data never leaves the store. This pull request edited that cell. Query text leaves when a text nearest runs. Point that cell at the data-protection page.
• The setup step still names MinIO next to RustFS. The storage page says only RustFS is qualified in CI. Use the same limit in the README.
• The erasure summary says cleanup removes old values. Bytes of a dropped property stay in the current data files until optimize. The linked schema procedure already says this. Name optimize in the summary.
• Conditional writes: If-None-Match: * protects creation of an object that must not already exist. If-Match protects replacement of a control object. Local disk does not implement If-Match. Put the header names on S3-compatible stores.
• An empty cluster API-key environment variable is present. std::env::var returns that empty string, and the cluster path sends it. from_env treats an empty key as missing.
• Add a one-line changelog.d note. The release-note rule asks for one on a user-visible documentation change. The observable contract did not change, so this note is optional.
• Rebase onto current main (277478d84cf19159f43cb21f202b49a044f0185e). Open pull requests docs: refocus README on introduction and getting started #800 and feat(cli)!: derive omnigraph embed vectors from the schema's @embed declarations #952 edit the same README and embeddings pages. Main now mentions OmniGraph Cloud private preview. This page should say it describes the engine you run. A later traces change can add another external service. The absolute "only service" sentence has no place to record that.

Tradeoff and liability

The new page adds no API, no state, and no second runtime authority. It removes the false claim that Cloudflare R2, Google Cloud Storage, and gs:// are supported. That reduces operator liability.

It also adds a second prose copy of the embedding model rule, and that copy is already wrong. Five more pages that restate the same rule will drift the same way. Keep one rule in docs/user/search/embeddings.md and link to it.

The encryption paragraph adds liability if it ships as written. An operator can set AWS_SERVER_SIDE_ENCRYPTION on the server and believe the variable is unused. The server sends it.

Validation

Reviewed commit 0b95fb23a19a578f8fb5c5fa100fb033f9805551. Pull request base 7fd9cc9037c634e2f3284f1ede55a85a6f7b28e8. origin/main is 277478d84cf19159f43cb21f202b49a044f0185e.

Evidence is code inspection, the pinned Lance 13.0.0 sources, object_store 0.14.2, and the Lance object-store guide. I did not run a Rust test, RustFS, live S3, or an embedding HTTP call. I did not use CI on this commit as evidence.

Local check, on this commit: python scripts/check-docs.py reported Documentation OK (361 Markdown files checked).

This pull request does not change rows, errors, types, or storage behavior. A GQT regression test does not apply.

Inline comments

docs/user/operations/data-protection.md lines 13–17:

Named CLI targets and unbound opens do not treat these variables the same way.

CapturedStorageBinding::capture copies only authentication keys from the environment (crates/omnigraph-storage/src/binding.rs). validate_environment then refuses any other recognized provider option that is still set. AWS_SERVER_SIDE_ENCRYPTION is one of those options, so a named CLI target fails closed. The same refusal applies to AZURE_STORAGE_ENCRYPTION_KEY on a captured Azure target.

A server and a direct open use binding: None (ClusterStore::for_storage_root in crates/omnigraph-cluster/src/store.rs). LanceAccessContext::with_storage then uses the default Lance registry. Lance 13.0.0 AwsStoreProvider::new_store calls StorageOptions::with_env_s3, which copies every recognized AWS_* key that is not already set. The S3 builder sends aws_server_side_encryption as a request header. CanonicalAzureRoot::from_env keeps every recognized AZURE_* key, including AZURE_STORAGE_ENCRYPTION_KEY. object_store 0.14 sends that value as the customer-provided AES-256 key header.

Please state both paths. Bucket default encryption still applies when these variables are unset. On a captured CLI target the variables are refused. On a server or a direct open they are sent.

docs/user/operations/data-protection.md line 29:

omnigraph cluster push posts the captured config files to POST /v1/clusters/{id}/config (crates/omnigraph-cli/src/managed/lifecycle.rs). The body includes cluster.yaml and the schema, query, and policy files it references (crates/omnigraph-cli/src/managed/lifecycle/capture.rs). Stored graph rows are not in that body.

The embedding provider receives the query text of a text nearest, and the source values of omnigraph embed.

Please name cluster push in this paragraph, and keep the limit that stored rows stay in the store.

docs/user/search/embeddings.md lines 79–80:

resolve_nearest_query_vec compares client.config().model with the model recorded by @embed (crates/omnigraph/src/engine/search.rs). It does not read OMNIGRAPH_EMBED_MODEL itself.

EmbeddingConfig::mock uses OMNIGRAPH_EMBED_MODEL only when no model was supplied. from_parts for provider mock keeps an explicit model (crates/omnigraph/src/embedding.rs). A cluster profile kind: mock with model: cluster-mock therefore resolves to cluster-mock while OMNIGRAPH_EMBED_MODEL is unset. cluster_boot_injects_embedding_provider_config covers that case.

The Schema annotation section below already states the rule: the active provider must resolve to the recorded model id. Delete this sentence, or make it match that rule.