Skip to content

feat(cli): blob put and blob clear - #928

Merged
ragnorc merged 40 commits into
ModernRelay:mainfrom
ragnorc:claude/blob-put-cli
Oct 11, 2026
Merged

ragnorc merged 40 commits into
ModernRelay:mainfrom
ragnorc:claude/blob-put-cli

Conversation

@ragnorc

@ragnorc ragnorc commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

RFC 0033 step 3C, the last step of Phase 3: CLI blob put and blob clear, over the engine writes (3A, #908) and the HTTP verbs (3B, #922).

The engine writes (#908) and the HTTP verbs (#922) are on main; this diff is the CLI step alone. Step 3-pre landed on main with the write_max_bytes setting (#933). The plan is RFC 0033 §6 and §12.4 (#900).

Commands

omnigraph blob put   ENTITY TYPE ID PROPERTY [--branch] [--file PATH] [--if-match TAG] [--json]
omnigraph blob clear ENTITY TYPE ID PROPERTY [--branch] [--if-match TAG] [--json]
  • Input: put reads --file or stdin, never both, and refuses an interactive stdin rather than waiting on it.
  • Bound: the value must fit write_max_bytes: the process's OMNIGRAPH_WRITE_MAX_BYTES for --store, the server's when served. blob put takes no --set.
  • Refused before the graph is addressed: input over 32 MiB (the setting's maximum), refused without reading past one more byte and reported with the engine's resource (Blob write payload bytes), and a malformed --if-match. Neither arm starts; JSON output reports execution: not_started.
  • One parser and one receipt: --if-match uses the API types' parse_blob_if_match, the parser the server applies to If-Match. Both arms print the receipt blob_write_output builds, as JSON or as key/value text.
  • Embedded arm (--store): calls put_blob_at_as / clear_blob_at_as. --as names the actor. A precondition failure is rendered through engine_error_output, so its --json body is the server's.
  • Served arm: sends one PUT (raw octet-stream body) or DELETE with the raw If-Match value. The client has no redirects and no retries, and --as is refused, as for every served write.
  • Outcomes: blob put and blob clear join the data-write outcome classification. A served 412 whose only detail is blob_precondition_failure (code conflict) is a verified request precondition, so a single-request command exits 4, as a graph-commit precondition does. An embedded mismatch exits 1, as for mutate --if-commit: it opened storage for writing. blob_precondition_failure now counts as a refusal detail and maps to the refresh action.

Tests

Owner Coverage
cli_data.rs::blob_put_and_clear_write_receipts_and_refuse_bad_input_before_scope put from --file with --if-match and --as: the receipt and the commit carry the actor, the receipt's ETag equals stat's, and get returns the bytes. put from stdin on an edge, with human output. A stale tag exits 1 with blob_precondition_failure and refresh. clear with *, then a no-op clear with commit: null. With OMNIGRAPH_WRITE_MAX_BYTES=4093, a 4,094-byte --store put is refused as Blob write payload bytes (limit 4,093) and the cell keeps its value. A malformed --if-match and a 32 MiB + 1 file are refused against a store that does not exist, and the store is never created.
parity_matrix.rs::parity_blob_put_and_clear_share_receipts_and_refusals The embedded and served arms write independent byte copies (never the hard-linked read twin). Put, clear and no-op clear receipts are equal once commit ids and the ETag are normalized; the actor matches on both arms. A stale precondition gives the same error and detail, with exit 1 embedded and 4 served. The oversize refusal is identical on both arms.
system_remote.rs::remote_blob_write_delivery_loss_never_replays_committed_effect Through the fault proxy, for put (a 2 MiB raw body) and clear, under disconnect, truncation and a gateway 504, and with the caller abandoned while the proxy withholds the reply it consumed (observed, not waited out, as the merge journey does since #936; about 9 s): the CLI fails, the server holds exactly the receipt the proxy saw, published once, and the proxy saw one data submission.

The proxy now carries a raw request body up to 33 MiB (JSON bodies keep their 1 MiB bound) and breaks delivery of Blob write receipts as it does for merges. run_both passes --as on the direct arm for blob put and blob clear, as for other writes. Test Workspace requires the delivery-loss journey by name, beside the merge journey, so a skipped or filtered run is not green.

Docs: blobs.md, the CLI guide and reference, and the developer Blob and testing guides. Changelog: a new fragment, and the 3A fragment's "the CLI does not expose them yet" is replaced.

ragnorc added 11 commits October 8, 2026 15:47
Every keyed write checked one 32 MiB ceiling on a batch's whole Arrow
memory, which includes a managed Blob value's bytes, so a value of
exactly 32 MiB never fit beside its row. KeyedBytes splits that count into
payload (each managed value's logical length) and framing (the rest),
each under its own 32 MiB ceiling, and every keyed check uses it:
staging, keyed-write preparation, the update carry scan, branch merge
buffering and proven-insert chunking, and the compatibility loader's
pre-decode forecast. The halves sum to the old count. A payload refusal
names its resource with 'Blob payload bytes' in place of 'bytes'.

Two Lance guards pin what the Blob write plan relies on: a whole-row
merge-update keeps the row's stable row id, and merge-insert refuses an
external reference outside the dataset's bases.
Session::put_blob_at_as and clear_blob_at_as replace or clear one Blob value
of an existing node or edge, addressed by exact id like a Blob read. A cell
write is a Mutation-protocol write: it stages one upsert of the carried row
and runs the Mutation tail, now split into commit_staged_mutation and
publish_committed_mutation, which mutate uses too.

The target's old payload is never read; the row's other cells are carried as
an update carries them and share the 32 MiB payload allowance with the new
value, which is bounded at 32 MiB inclusive before anything is captured. An
optional precondition (an ETag set, or any existing value) is evaluated
against the attempt's base and re-evaluated when a competing commit forces a
re-prepare; a change of branch incarnation or accepted schema between
attempts fails closed. The receipt's ETag is read from the detached commit
before publication.
PUT and DELETE /graphs/{graph_id}/blob replace or clear one Blob value of
an existing node or edge through the engine cell writes, each as one graph
commit under `change`.

PUT is a raw ingress route like /load/ndjson: it authorizes, checks the
media type (415) and a declared length (413) before polling the body, then
collects at most 32 MiB under the body deadline (408) into one buffer the
engine adopts, and shrinks its ingress reservation to the received size.
Both verbs take a branch, refuse snapshot and unknown parameters, run as
owned writes, and answer the exact receipt. If-Match is parsed by a shared
parser in the API types; a failed one is a 412 with the new
blob_precondition_failure detail and the current ETag header.
The compatibility loader's pre-decode forecast charged every `base64:`
string as Blob payload, so a declared String whose text starts with
`base64:` used the payload ceiling: a 24 MiB Blob beside such a 12 MiB
String was refused at 33 MiB although the batch check admits it. The
forecast now takes the type's Blob properties, as the Arrow check does:
only a Blob property's `base64:` value is payload; every other value is
framing.
# Conflicts:
#	crates/omnigraph-server/src/blob_transport.rs
…ions

The public vocabulary guard classifies every graph-storage term in the
served OpenAPI. The five new descriptions now use the public entity
vocabulary, so no inventory exception is needed.
`omnigraph blob put` stores the raw bytes of --file or stdin in one Blob cell
and `omnigraph blob clear` nulls it, on a --store graph through the engine
writes or on a served one through PUT and DELETE /blob. Both print the
receipt the server returns. A malformed --if-match, input over 32 MiB and an
interactive stdin are refused before the graph is addressed. --as names the
actor of a --store write. A served Blob 412 is a verified request
precondition and exits 4; a write is sent once and never retried.

The lost-delivery fault proxy now carries a raw put body past its JSON
bound and breaks delivery of a put's or clear's receipt.

@ragnorc ragnorc left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation: hold merge until the stack's existing regression-test and CI requirements are complete. I found no new blocking correctness defect in the CLI increment, 6c6b4554..f0aa6012, and have no optional change to request. The minimal GQT regression requested on #903 remains a prerequisite. The focused results below do not replace the required workspace checks for this head.

The head moved during review. I inspected the merge from current main, including both conflict resolutions, and repeated the focused checks on the new head. The CLI increment is unchanged.

This PR lets an operator replace one existing node or edge Blob from a file or stdin, or clear it. A put stores raw managed bytes and returns its exact commit and ETag. A clear sets a nullable cell to null; clearing an already-null cell returns commit: null. A stale --if-match refuses the write. This makes individual binary edits available without preparing a base64 load or mutation.

The design keeps the rules with their existing owners:

  • Input admission uses the engine limit and shared If-Match parser before graph resolution. The client dispatch uses the engine writes or one HTTP request. Both use the shared receipt mapping.
  • The engine path handles node and edge cells through normal mutation staging. It checks the captured base, omits the target's old payload when carrying the row, and publishes once. The receipt's ETag is resolved before publication from the version that will be published. The CLI adds no alternative commit path.
  • The common HTTP client disables redirects and retries. The Blob precondition classification only promotes the matching typed 412 to a verified refusal. Lost delivery retains an unknown outcome.

The tradeoff fits bounded, individual Blob edits. Raw input avoids base64 encoding, but put buffers up to 32 MiB of input data. Pinned Lance 11.0.0 rejects direct Blob-v2 updates, so the engine still rewrites the row. Other Blob cells can require reads and share the payload budget; carried external cells require the existing policy. Large-object streaming and measured throughput are outside this evidence.

Liability assessment: the two public verbs, flags, output contract, and tests add maintenance cost. Reusing the parser, receipt, outcome tracker, and mutation publisher keeps that cost contained. No new persisted state or retry controller is added. Five similar commands would grow command dispatch and tests without multiplying sources of write authority. The change implements the general cell-write contract through its existing owner; it does not depend on a special-case repair of stored data.

Local validation used a clean isolated checkout at f0aa6012b16d341af8923550550673a2f0735f95, Rust 1.97.1, and CLI/server binaries built at that commit. Each row below ran one test, with zero failures and zero ignored tests:

Target Exact test filter Result
cli_data blob_put_and_clear_write_receipts_and_refuse_bad_input_before_scope Passed, 19.11 s
parity_matrix parity_blob_put_and_clear_share_receipts_and_refusals Passed, 35.16 s
system_remote remote_blob_write_delivery_loss_never_replays_committed_effect Passed, 46.28 s

Command for each target/filter:

cargo test --locked -p omnigraph-cli \
  --features omnigraph/failpoints,omnigraph-cluster/failpoints \
  --test TARGET -j 3 TEST_FILTER -- --exact --nocapture --test-threads=1

The environment selected Rust 1.97.1, a private build cache, and RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable'. The server fixture used the exact-head server binary. The lost-response test covers four faults for both put and clear, after the proxy receives the server's receipt. It checks the stored bytes/null, exact commit, one publication, and one data submission. It does not prove every cancellation point during storage work.

Formatting, documentation (235 Markdown files), AGENTS links, and diff checks passed. The macOS linker emitted a compact-unwind size warning. The CLI/file/stdin and HTTP-fault contracts cannot be expressed by GQT's current step set, so the existing Rust owners are appropriate. This new-command test run is not a fail-before/pass-after bug-fix proof; the parent does not expose these commands. The inherited payload fix retains its separate GQT requirement above.

Exact-head CI reports successful GQT ordinary/DST/clippy runs, workspace clippy, RustFS S3 default/failpoint suites, Azurite, storage-upgrade compatibility, server AWS-feature tests, and the separate DST suite. Test Workspace was still running at the final check. These are CI results, separate from the local checks above. I did not run the full workspace, live S3/Azure, or performance tests locally. No source or test edits were retained.

@ragnorc ragnorc left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation: accept this CI follow-up, but hold the stack until the compact GQT regression requested on #903 lands. The earlier pending-CI concern is resolved. I found no new blocking correctness defect or optional change in f0aa6012..b14909bd. The existing GQT request still applies. The author’s generated-parameter plan avoids committing a large base64 literal. This review adds no duplicate inline finding.

Reviewed head: b14909bded0918b482e08a990c38bd6c77de8e2b.

This PR lets an operator put raw file/stdin bytes into one existing node or edge Blob cell, or clear a nullable cell. It returns the exact commit and managed-value ETag. The CLI uses the existing engine or HTTP write path. A lost response leaves an unknown outcome and must not cause an automatic resubmission.

The new workflow entry makes the existing delivery-loss journey mandatory. The shared JUnit checker requires a passed result. Removing, filtering out, or ignoring the named test cannot leave this gate green. This addresses the general missing-test problem at the CI owner. It adds one maintained test name and no runtime state, API, or new checker. Five similar requirements would extend one list. Renaming a required test now requires updating that list, which is the intended tradeoff.

I also inspected the merged changes relevant to Blob storage. The cell-write path remains unchanged: it carries other columns, skips the target’s old payload, stages the row, and publishes once. The receipt is resolved from the staged version before publication. The schema-evolution integration keeps field identity and catalog column order. Null descriptors no longer demand a nonexistent data file. I checked these assumptions against the pinned Lance 11.0.0 implementation and the upstream documentation, including the registry archive checksum against Cargo.lock for the source inspected.

The workload tradeoff is unchanged: these are bounded individual edits. Raw bytes avoid base64 encoding, but input is buffered and Lance still rewrites the row. Other Blob cells can require reads and share the payload budget. The two CLI verbs add an interface and test obligation. Reusing the existing publisher, receipt, precondition parser, and HTTP outcome handling limits that liability. There is no new retry controller or second source of write authority.

Local validation used a clean isolated checkout, Rust 1.97.1, and binaries built at the reviewed head:

  • The real CLI/server/proxy journey passed 1 test, 0 failures, 0 ignored, in 42.29 s. It covers put and clear under disconnect, truncated response, gateway timeout, and caller timeout. Each scenario checks one submission, one publication, the commit receipt, and stored bytes or null.
  • With synthetic JUnit reports, the owner list from parent 1c3e80071b9f9d94b2b5a8023dd437e2495e30af accepted an absent or skipped Blob test. The current list rejected both, rejected failure, and accepted a passed test. I used the existing checker with --passed NAME for every name extracted from each workflow revision. This is a CI configuration proof. GQT cannot express JUnit ownership or HTTP delivery faults, so the existing Python and Rust owners fit these contracts.
  • The workspace-owner, CI-cell, CI-gate, and merge-group checker self-tests passed. Action-pin and diff checks passed.

Exact local journey command:

env 'CARGO_BIN_EXE_omnigraph-server=/tmp/review-928/target/debug/omnigraph-server' \
  RUSTUP_TOOLCHAIN=1.97.1 CARGO_TARGET_DIR=/tmp/review-928/target \
  RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable' \
  cargo test --locked -p omnigraph-cli \
  --features omnigraph/failpoints,omnigraph-cluster/failpoints \
  --test system_remote -j 3 \
  remote_blob_write_delivery_loss_never_replays_committed_effect \
  -- --exact --nocapture --test-threads=1

The inherited payload fix also passed the same temporary GQT boundary case at this head: 1 selected execution passed in 8.23 s. The retained before-fix run at b8b631ee8f813e557805baf33efe869011f48275 failed at mutation step 9. It charged 33,556,272 bytes against the 33,554,432-byte limit for an exactly 32 MiB Blob plus row framing. This was a behavioral failure. I reused that earlier failure evidence and reran the identical fixture at the current head. The fixture SHA-256 is fb499d12646ca7dd0f58dddc909d8fa8a0ab508da8036511908c0bdbc0457892.

The before command was:

cargo run -p omnigraph-gqt --locked --features omnigraph/failpoints \
  --bin omnigraph-gqt -- /tmp/review-903-followup/blob_limit.gqt \
  --target omnigraph-engine --storage local-filesystem \
  --artifacts /tmp/review-903-followup/gqt-before-artifacts

The current-head command was the same with -j 3 before --, and --artifacts /tmp/review-928-followup/gqt-artifacts. Both used Rust 1.97.1 and the RUSTFLAGS above, with private build caches. This selects one ordinary local-filesystem execution. It does not select DST or served execution. The temporary boundary step extends the existing Blob-update case; its large literal is proof material, not a proposed repository fixture. Please retain the planned compact generated case on #903.

CI Test Workspace passed 4,088 tests, with 17 skipped. The Blob journey passed and the checker confirmed all 20 required owners. CI checked merge commit fd76a51f7856a5aa61f5b3fe96f5373a48c41ef4. Its tree, a03584b637db84bfe0902997ff604e9baa0ccd3b, equals the reviewed head’s tree. The required gates, ordinary/served/DST GQT, storage-upgrade checks, RustFS, and Azurite are green. These CI results are separate from the focused local results above.

Limits: the delivery faults occur after the proxy receives a successful server receipt. They do not prove every cancellation point during storage work. I did not run the full workspace, live cloud, or performance tests locally. The linker emitted the existing compact-unwind size warning. No source or test edits were retained.

Main's write_max_bytes setting landed the payload/row accounting split this
branch inherited from its accounting prerequisite, so the merge keeps main's
accounting and drops the prerequisite's. The Blob put now checks its value
against the session's write_max_bytes and stages under that operation's
WriteBudget. The prerequisite's two Lance surface guards, which pin facts the
put relies on, come along unchanged.
The engine put now honors the session's write_max_bytes. The route's
pre-read bound stays BLOB_WRITE_MAX_BYTES, now the setting's maximum, and
the server's process write_max_bytes bounds the value after the body is
read; data_routes pins that a lowered process value refuses a PUT with 413.
A put honors write_max_bytes: the CLI keeps its 32 MiB pre-read refusal,
and the process's OMNIGRAPH_WRITE_MAX_BYTES bounds a --store put's value;
cli_data pins that a lowered value refuses the put.
# Conflicts:
#	crates/omnigraph/tests/detached_commit_matrix.rs
# Conflicts:
#	changelog.d/blob-cell-put-clear.added.md

@ragnorc ragnorc left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation: approve. The earlier GQT prerequisite is resolved in this head. The superseded #903 fix now comes from #933, with small permanent boundary cases. I found no blocking defect or optional change to request in this CLI follow-up.

Reviewed head: c3fef0648d0eb1868da7ceb19851cdb08f9d4b40.

This PR lets an operator replace one existing node or edge Blob cell from a file or stdin, or clear a nullable cell. A successful write returns its exact graph commit and value metadata. A stale --if-match refuses the write. Clearing an already-null cell returns no new commit. This avoids preparing base64 input for an individual binary edit.

The new follow-up changes the delivery-loss test. It waits until the proxy holds the successful server receipt, checks that the CLI is still waiting, then stops it. The shared proxy helper detects the closed connection. A timer remains only as a failure bound. This observes the required ordering directly and keeps the test in the regular CI gate. It tests caller abandonment after publication, not expiry of a production deadline.

The runtime design still has one write owner. The CLI admits input before scope resolution. The client calls the engine or sends one HTTP write. The shared HTTP client disables retries and redirects. The engine carries sibling columns, omits the target's old payload, stages the replacement, resolves its ETag, and publishes once.

The inherited boundary fix addresses the general cause. Typed Blob payloads and row data have separate accounts, and one captured WriteBudget reaches the Blob operation's staging path. A payload can reach its allowance without row metadata making that boundary impossible. The permanent boundary case lowers the allowance to 4,093 bytes. Adjacent cases cover carried payloads and edge loads.

Tradeoffs and liability: the workload is bounded, individual cell edits. The CLI buffers input up to 32 MiB. The effective process or server write_max_bytes can be smaller. Sibling Blob values can require reads and share the payload allowance. Lance 11.0.0 still refuses direct Blob-v2 updates, so this path rewrites the row. I checked the pinned implementation at ab6b5bbe46009ed78746b444df8db59a8bc5d842, its merge-insert constraints, and the relevant upstream documentation. Separate row and payload allowances are representation limits, not a combined heap cap.

The two CLI verbs add an interface and test obligation. Reusing the precondition parser, receipt mapping, outcome handling and publisher contains that cost. The new test helper replaces repeated wait loops without adding production state. Five similar commands can use these same owners instead of adding separate retry or publication rules. No persisted format or second source of write authority is added.

Local checks used a clean isolated checkout and exact-head CLI/server binaries. All six selected Rust tests passed, with none ignored: the Blob CLI input/receipt test, the embedded/remote parity test, and four delivery-loss tests covering Blob, merge-verb, merge-statement and deployment paths. The Blob journey checks both put and clear under disconnect, truncated reply, gateway timeout and caller abandonment. It checks one submission, one publication, stored bytes/null and the exact receipt.

Commands used this environment:

export RUSTUP_TOOLCHAIN=1.97.1
export CARGO_TARGET_DIR=/tmp/review-928/target
export RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable'

For each Rust row below, I ran cargo test --locked -p omnigraph-cli --features omnigraph/failpoints,omnigraph-cluster/failpoints -j 3 --test TARGET FILTER -- --nocapture --test-threads=1, with --exact for the single-test filters. The server override was CARGO_BIN_EXE_omnigraph-server=/tmp/review-928/target/debug/omnigraph-server.

Target Filter Result
system_remote delivery_loss 4 passed
cli_data blob_put_and_clear_write_receipts_and_refuse_bad_input_before_scope 1 passed
parity_matrix parity_blob_put_and_clear_share_receipts_and_refusals 1 passed

Four GQT executions passed locally on omnigraph-engine/local-filesystem: the three committed cases write_max_bytes.gqt, write_max_bytes_update_carried_blob.gqt, load_write_max_bytes_edge_blob_value.gqt, and the same temporary default-boundary fixture used for the earlier before-fix proof. This selects ordinary engine execution, not served or DST execution.

cargo run --locked -p omnigraph-gqt --features omnigraph/failpoints \
  --bin omnigraph-gqt -j 3 -- \
  crates/omnigraph-gqt/cases/write_max_bytes.gqt \
  crates/omnigraph-gqt/cases/write_max_bytes_update_carried_blob.gqt \
  crates/omnigraph-gqt/cases/load_write_max_bytes_edge_blob_value.gqt \
  /tmp/review-933/default_boundary.gqt \
  --target omnigraph-engine --storage local-filesystem \
  --artifacts /tmp/review-928-20261011/gqt-artifacts

The identical default-boundary fixture has one node type, two fields, one insert and a query checking its key. Its SHA-256 is c3d5b93e8ddde0057bc144b16722ffec426160b42a8ed255994b1536050f344a. I reused the recorded before-fix run at 7a3bddf544640135a39cdec7952872b3ce0e3918: it ran the insert and failed because 33,555,360 row bytes exceeded 33,554,432. It passed at this exact head. The comparison needs a 32 MiB value because the old revision has no configurable allowance. The committed cases use the smaller equivalent boundary.

The before command was /tmp/review-928/target/debug/omnigraph-gqt /tmp/review-933/default_boundary.gqt --target omnigraph-engine --storage local-filesystem --artifacts /tmp/review-933/default-before. That binary was built from the before commit with the same toolchain and cargo build --locked -p omnigraph-gqt --features omnigraph/failpoints --bin omnigraph-gqt -j 3. This is a behavioral failure before and success after, not a build or setup failure.

The CLI/file/stdin and HTTP-fault contracts cannot be expressed by GQT. Their existing Rust owners fit those boundaries. These new-command and test-ordering checks are not themselves before/after bug-fix proofs.

CI Test Workspace passed 4,264 tests, with 29 skipped. The Blob delivery test passed; the owner checker confirmed all 20 required names. CI checked cfefe0800e4b96b05ed3662adcc2951c18b82daa; its tree bdd532f3f67ec63e0bb77afae365e5d2693c27a7 equals this reviewed head's tree. The current CI Gate, ordinary/served/DST GQT, RustFS, Azurite and Clippy checks passed. These CI results are separate from the local checks.

Local documentation checks passed for 244 Markdown files and 54 AGENTS links. The owner-checker self-test, format and diff checks passed. No source or test edits are retained. I did not measure throughput, peak memory, live cloud behavior, or cancellation during storage work before a receipt exists. The macOS linker emitted its existing compact-unwind size warning.

@ragnorc ragnorc left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation: approve the implementation at e610d1dfc5c45e26782c61c678c233b859c500ad. Required CI must finish successfully before merge. No new blocking defect or optional change to request. The earlier GQT prerequisite remains satisfied. This is a COMMENT review because the authenticated account owns the PR.

This PR lets an operator replace one existing node or edge Blob cell from a file or stdin, or clear a nullable cell. It returns the exact graph commit and value metadata. A stale --if-match refuses the write; clearing an already-null cell makes no commit. Raw input avoids preparing a base64 mutation for an individual binary edit.

The follow-up retains the CLI implementation while adopting main’s shared integration-test layout. I inspected the merge resolutions. The Blob delivery test keeps its assertions and proxy, moves to integration/system_remote.rs, and uses crate::support. The required CI owner now uses its complete module-qualified name. The test remains required; a skipped or filtered execution cannot satisfy that owner. Documentation matches the new locations. The final import-order commit changes no runtime behavior.

The integration still has one write owner:

  • CLI admission parses the precondition and bounds input before resolving the graph. The client calls the engine or sends one HTTP data request. The shared client disables retries and redirects. Only the matching typed 412 becomes a verified Blob precondition refusal.
  • The engine explicitly selects ordinary publication. The new unpublished-commit support in main therefore does not change Blob write visibility. The shared publisher retains its exact graph-head precondition.
  • Main's full-text analyzer change now passes through the common staging gateway. If it adds an untrained declaration segment, staging records the final detached version and transaction identity. Blob writes resolve their ETag from that same final version before publication. They do not add a separate index or publication path.

I checked these interactions against the current guides, callers, and pinned Lance 11.0.0 source at ab6b5bbe46009ed78746b444df8db59a8bc5d842. I reused the prior complete storage-documentation audit and reread the complete Blob, FTS-format, and tokenizer pages. The pinned implementation still rejects direct Blob-v2 updates. Its untrained index metadata has empty coverage, and its flat text matcher obtains the analyzer from the segment. Newer features described in upstream documentation were not assumed to exist in this pin.

Tradeoffs and liability: this design serves bounded individual edits. The CLI buffers up to 32 MiB; the engine or server can apply a smaller write_max_bytes. Rewriting the row can also read sibling Blob values, which share the payload allowance. Separate row and payload accounts remove the framing-overhead defect at the storage owner; they are not a combined heap limit. The two verbs add an interface and test obligation. Reusing the parser, receipts, outcome tracking, staging, and publisher contains that cost. Five similar commands can use those same owners without adding retry controllers or competing write authority. I found no reason to add another abstraction.

Fresh local validation used the clean isolated head and CLI/server binaries built from it. Each Rust row ran one test, with zero failures and zero ignored tests:

Target Exact filter Result
integration cli_data::blob_put_and_clear_write_receipts_and_refuse_bad_input_before_scope Passed, 7.85s
integration parity_matrix::parity_blob_put_and_clear_share_receipts_and_refusals Passed, 17.20s
integration system_remote::remote_blob_write_delivery_loss_never_replays_committed_effect Passed, 8.84s

For each row:

cargo test --locked -p omnigraph-cli \
  --features omnigraph/failpoints,omnigraph-cluster/failpoints -j3 \
  --test TARGET FILTER -- --exact --nocapture --test-threads=1

Environment: RUSTUP_TOOLCHAIN=1.97.1, CARGO_TARGET_DIR=/tmp/review-928/target, RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable', and CARGO_BIN_EXE_omnigraph-server=/tmp/review-928/target/debug/omnigraph-server. The binaries were built with cargo build --locked -p omnigraph-cli -p omnigraph-server --features omnigraph/failpoints,omnigraph-cluster/failpoints -j3.

The delivery test checks put and clear under disconnect, truncation, gateway 504, and caller abandonment after the proxy holds the successful receipt. It verifies one submission, one publication, the exact receipt, and stored bytes/null. GQT cannot express CLI file/stdin or HTTP delivery faults, so these existing Rust owners are appropriate. These new-command tests are not fail-before/pass-after bug-fix proofs.

All four selected GQT executions passed on omnigraph-engine/local-filesystem:

cargo run --locked -p omnigraph-gqt --features omnigraph/failpoints \
  --bin omnigraph-gqt -j3 -- \
  crates/omnigraph-gqt/cases/write_max_bytes.gqt \
  crates/omnigraph-gqt/cases/write_max_bytes_update_carried_blob.gqt \
  crates/omnigraph-gqt/cases/load_write_max_bytes_edge_blob_value.gqt \
  /tmp/review-933/default_boundary.gqt \
  --target omnigraph-engine --storage local-filesystem \
  --artifacts /tmp/review-928-2207/head-e610/gqt-artifacts

For the inherited boundary fix, the identical default_boundary.gqt has one node type, one insert, and a key query. Its SHA-256 is c3d5b93e8ddde0057bc144b16722ffec426160b42a8ed255994b1536050f344a. The retained before run at 7a3bddf544640135a39cdec7952872b3ce0e3918 failed at the insert: 33,555,360 charged row bytes exceeded 33,554,432. It compiled and executed successfully before that behavioral failure. At this exact head it passes in 7.94s. The old revision lacks a configurable allowance, so this comparison uses 32 MiB; the permanent cases retain the small 4,093-byte equivalent boundary.

The before command was /tmp/review-928/target/debug/omnigraph-gqt /tmp/review-933/default_boundary.gqt --target omnigraph-engine --storage local-filesystem --artifacts /tmp/review-933/default-before, using the binary built at that before commit with cargo build --locked -p omnigraph-gqt --features omnigraph/failpoints --bin omnigraph-gqt -j3 and the same build environment. This is retained failure evidence with a fresh exact-head success.

The current workspace CI run targets this head and was still running at the final check. The storage-upgrade qualification has passed. The local results above are exact-head evidence; they do not replace those gates. Earlier workspace CI passed 4,300 tests with 32 skipped at cd65a1fe8c244e0d8d68e95daa900a0cee6ca2ee, before the latest test relocation. I do not count that earlier run as CI proof for this head.

Local documentation checks passed for 249 Markdown files, along with 54 AGENTS links, formatting, diff checks, action pins, and the workspace-owner, CI-cell, CI-gate, and storage-upgrade checker self-tests. The CI-cell checker finds all 68 required test names, including the relocated Blob owner. No source or test edits remain. I did not run the full workspace or live cloud suites locally, measure throughput or peak memory, or test every cancellation point before publication. The macOS linker emitted its existing compact-unwind size warning.

@ragnorc
ragnorc added this pull request to the merge queue Oct 11, 2026
Merged via the queue into ModernRelay:main with commit 680fdfe Oct 11, 2026
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant