[EnRaiha]

Summary

Rebased on main after #429. The earlier 11-commit series is replaced by three commits that cover what #429 leaves open. #424 and #425 are closed by #429, so this PR closes nothing.

#429 fixes a predicate UPDATE or DELETE inside BEGIN … COMMIT, a trigger body or a procedure body, and the fusion and search key naming. A predicate write outside a block still fails on main, and so do three smaller gaps.

#	Commit	Gap on main
1	fix(txn) refuse several writes of one sum balance before a route is chosen	Two writes of one request that move one materialized-sum balance settle against the same committed rows. The request is accepted, or fails with XX000 before it reaches a check. It is refused whole.
2	fix(txn) run a request in which a predicate write follows another write as one implicit transaction	A predicate UPDATE or DELETE over HTTP, NDJSON, WebSocket or native SQL outside a block, on a collection that spans vShards, answers XX000 (calvin bulk predicate write reached overlay staging without ollp_predicted_surrogates). It takes the recon coordinator that pgwire uses, and one scan per predicate write. A predicate that follows another write in the request runs as an implicit transaction, so it matches the rows the earlier statements wrote.
3	fix(search) refuse a hybrid score alias named doc_id	A hybrid hit names its row under doc_id. An alias of that name replaced the field, and every row answered an empty id. The planner refuses the alias.

How the commits were chosen

The tests of the earlier series were run on main after #429, with none of its source. Passing means #429 covers the fix, and the fix was dropped.

Area	On main
In-process: matched set of a staged predicate write, an empty set, commit parity of a block, transition guard (18 tests)	18 pass
Three-node: fusion naming of a hit in no edge, of a text-only hit, of a declared key, of a key equal to its surrogate, search keys for hybrid, subquery, vector-primary and bitemporal rows, a predicate write in a block on a follower (13 tests)	13 pass
Wire: _key score alias, fusion over HTTP and native SQL (39 tests)	39 pass
Wire: predicate writes outside a block, sum balance guard, doc_id alias	10 fail, the three gaps above

Test plan

Red means the test run on main without the commit.

Test	Red on main	With the commit
materialized_sum_cross_shard_http: predicate UPDATE and DELETE of a cross-shard sum source over HTTP, NDJSON and WebSocket RPC	XX000	pass
predicate_write_two_statements_http: two predicate updates of two collections in one request	XX000	pass
predicate_write_sequence_http: a third statement reads the first across two vShards; a predicate reads the point write before it across two vShards	XX000	pass
materialized_sum_same_request: two predicate updates of one balance, and a predicate beside a point update of it	XX000 before the guard is reached	refused, nothing applied
sql_hybrid_search::a_score_alias_named_doc_id_is_refused	rows answered with an empty id	refused

Run on this head: the wire filters for predicate writes, sum balances, fusion and hybrid search (42 and 16 pass). The full local mirror of the CI Test workflow runs now. I will post its counts here.

Known on main with rustc 1.96.1, not changed by this PR: an unused import in nodedb-sql (search_cells.rs:32) and two nonminimal_bool findings fail clippy with -D warnings. nodedb-vector search_allocations_do_not_scale_with_distance_count times out at 120 s on main too, run alone.

Tradeoffs

• A request with a predicate write after another write waits one sequencer epoch (default 20 ms), because it commits as a transaction.
• Two writes of one request that move one sum balance are refused, not settled statement by statement. Settling in sequence needs the planner to settle per statement.
• A request that mixes UPDATE and DELETE answers 500 (one statement reported two command verbs) on main too. This PR does not change it.

Evidence: https://github.com/EnRaiha/nodedb-fusion-evidence

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

[farhan-syah]

Request changes. The transaction path still permits silent omissions and rejection after a durable commit verdict.

The per-write prediction mapping and owner-supplied identities address real defects. Five correctness findings block merge.

• Transaction staging must capture its own matched rows atomically.
• Batched statements must observe preceding writes.
• Calvin must enforce transitions before voting commit.
• Identity reads must cover temporal storage.
• Identity metadata must remain separate from user-named score fields.

Rebuild this 26-commit series after correcting the implementation. Combine dependent changes with their regression tests into coherent commits. Keep the sound changes. Rewriting history alone resolves none of these findings.

Static review only, against f63110. No builds, tests, or formatters ran. Contributor test claims remain unchecked. Existing CI reports Static gates passed and Test Suite skipped.

[farhan-syah]

[P1] Capture the prediction from the staging scan

statement_time_prediction completes before stage_write dispatches. Both bulk staging handlers independently rescan the live predicate.

• A matching row inserted between those dispatches enters the staging scan.
• RETURNING and the transaction overlay include that row.
• The saved prediction excludes it, so Calvin omits it at COMMIT.

The static commit route performs no OLLP membership check that repairs this mismatch.

Return the matched surrogates from the staging handler and save that exact set. Add a deterministic interleaving test between prediction and staging.

[farhan-syah]

[P1] Enforce recomputed transitions before Calvin votes commit

This routes local predicate transactions through Calvin, which recomputes updates against current bodies. The new stage_bulk_update transition check only judges statement-time images. stage_calvin_bulk_update calls stage_apply_update without repeating transition enforcement.

• A collection allows A→B and A→C, but forbids C→B.
• One transaction stages A→B.
• Another session commits A→C before the first transaction commits.
• Calvin recomputes C→B and votes commit.
• Committed redo enforcement rejects the transition, and the scheduler halts with RedoInstallRefused.

Run transition enforcement against the current and recomputed bodies during Calvin staging, before voting. Add coverage for this interleaving.

[farhan-syah]

[P1] Predict later statements against preceding writes

Every recon scan reads committed state with txn_id: None, before any batch statement applies. Sequentially awaiting these scans does not expose preceding writes.

For an edge-bearing collection containing n=1, submit this HTTP SQL batch without joined triggers:

UPDATE t SET n = 2 WHERE n = 1;
UPDATE t SET n = 3 WHERE n = 2;

The implicit-transaction gate does not fire for this pair of ordinary bulk updates. Recon predicts [row] for the first statement and [] for the second. Calvin applies those fixed sets and reports success with n=2. The second update silently disappears.

Compute each prediction against the preceding statements' staged writes. Cover dependent predicates in one collection, alongside the existing independent two-collection case.

[farhan-syah]

[P2] Read temporal identities from the versioned table

sparse.get reads the ordinary document table. Bitemporal inserts write their bodies into DOCUMENTS_VERSIONED, so this helper returns None for existing temporal rows.

On a nonminting coordinator without the catalog binding, an unlinked fusion hit still reports its surrogate decimal. Hybrid and vector searches inherit the missing owner identity through this helper.

Reuse text_base_body, which already selects versioned_get_current for temporal collections. Add RF1 coverage with WITH (bitemporal=true) and query every cluster node.

[farhan-syah]

[P2] Separate owner identity from user-named score fields

The planner accepts _key as a hybrid score alias. For rrf_score(...) AS _key, this removal discards the user's score, including numeric values when no owner identity exists.

HybridSearchHit::write also emits duplicate _key fields when the score alias and owner identity share that name. flatten_hybrid_hits_to_relational_rows repeats the unconditional removal on gathered results.

Carry owner identity separately from fields whose names the query controls. Add direct and gathered hybrid coverage using _key as the score alias.

[EnRaiha]

Thanks for the review. All five findings hold. The fixes are pushed to this branch as ten commits on top of f63110de5 (head 6f6194eff). I have not rebuilt the series yet: the rebuild into coherent commits, with each fix beside its tests, follows once the three-node suites have run on the new head.

#	Finding	Commit
1	The matched set comes from the staging handler	46aa534fd, f444edfdd
2	The Calvin stage judges transitions before it votes	02afb9cb5
3	A later statement sees the writes before it	b3a6af4ea, ecdd4b647, 89ee5cb01
4	Identities read from the temporal table	898b1792e
5	Owner identity apart from score aliases	f428f34a6

1. BulkUpdate and BulkDelete staging return the surrogates they staged, in both reply shapes. The session saves that set. The separate pre-exec scan is gone.

On the interleaving test: I did not write one, and I think none can exist. The race needed two reads of the predicate with a gap between them, the prediction scan and the staging scan. There is now one read, the staging scan. The rows it writes to the overlay, the rows it returns under RETURNING, and the set it reports are the same list built in one pass, so no insert can land between "the rows staged" and "the rows saved". A test would have nothing to pause between. What I test instead is the equality that closes the race:

• test_stage_write_matched stages a bulk update and a bulk delete, with and without RETURNING, and checks that the reported set equals the surrogates the overlay holds, including the zero-row case.
• txn_calvin_empty_predicate checks that a statement that matched no row still saves its empty set. Dropping it made COMMIT fail with XX000.
• The existing block tests check that COMMIT submits the saved set through the Calvin stage.

There is no fake-dispatch test of stage_write, because the session store gives a unit test no way into a block. If you want the race covered by a pause point anyway, say where you would put it and I will add it.

2. stage_calvin_bulk_update now calls check_stored_update_transitions after it recomputes the SET. A violation is the stage's refusal, the vote is abort, and nothing reaches the redo, so the scheduler keeps serving. The point-update staging checks transitions the same way, so a rule violation in a block now fails at the statement and not at COMMIT.

3. A request of two or more predicate writes runs as one implicit transaction, so each statement reads the rows the one before it staged. The recon coordinator keeps one predicate write per submission and returns an internal error for a second. The guard for two writes that move one sum balance now runs ahead of the route, because recon no longer sees those requests. Tests: predicate_write_sequence_http, which includes your exact pair (WHERE n = 1, then WHERE n = 2, on an edge-bearing collection: n=2 and the second update dropped before, n=3 after) and a third statement that reads the first across two vShards (v2 before, v3 after) and two_predicate_updates_of_one_balance_are_refused_and_change_nothing (accepted before, refused after). A request that mixes UPDATE and DELETE answers 500 (one statement reported two command verbs) on main too; I left it.

4. stored_client_identity reads the body through text_base_body, which picks the versioned table for a bitemporal collection.

5. The hybrid hit carries the owner key as __nodedb_key. A score alias that starts with __nodedb_, or that is doc_id, is refused at plan time, so rrf_score(...) AS _key keeps the user's score. The same holds through a subquery over the hybrid search.

Run on the new head: wire filters for edge, predicate, materialized sum, transactions, triggers, procedures and graph RAG (469 pass), in-process txn (32 pass), library calvin and session:: (835 pass). The three-node dml_suite and common_suite and the clippy and static gates have not run on these ten commits yet.

[farhan-syah]

Request changes. Four previous findings are resolved. Sequential batch visibility remains incomplete.

Previous finding	Status	Checked implementation
Capture the prediction from the staging scan	Resolved	Staging reports its matched set, including empty sets. The session saves that response.
Enforce recomputed transitions before Calvin votes commit	Resolved	Calvin checks current-body transitions before staging the recomputed image.
Predict later statements against preceding writes	Partial	Multiple predicates use implicit staging. One predicate beside preceding point writes still bypasses it.
Read temporal identities from the versioned table	Resolved	Owner identity reads reuse text_base_body.
Separate owner identity from user-named score fields	Resolved	Reserved metadata and planner alias checks preserve _key scores.

I accept your reasoning about the removed race. Capturing matches inside staging removes the second predicate read. A forced interleaving test between two reads is no longer required. I inspected the replacement tests without executing them.

The rebuilt series contains 11 commits. Their grouping addresses the history request, but two publication cleanups remain:

• Remove Claude-Session trailers from all 11 commits. Session URLs do not belong in the public commit history.
• Update the PR description for this revision. It still reports 26 commits, superseded SHAs, the removed prediction scan, and the old _key metadata field.

Static review only, against c688c41ecb3e8d65a37f2b93eddedd33e61eb736. No builds, tests, or formatters ran. git diff --check passes. Existing CI reports Static gates passed and Test Suite skipped. Contributor runtime claims remain unchecked.

[farhan-syah]

[P1] Predict later statements against preceding writes

The original two-predicate example now takes implicit staging. This classifier still excludes batches containing one predicate and preceding point writes.

Use plain document collections t and u on different vShards, with declared id primary keys. Seed t.x and u.y with n=0, without triggers or materialized sums. Submit this HTTP SQL batch:

UPDATE t SET n = 1 WHERE id = 'x';
UPDATE t SET n = 2 WHERE n = 1;
UPDATE u SET n = 9 WHERE id = 'y';

• The task set contains PointUpdate, BulkUpdate, and PointUpdate, so this predicate count is one.
• Implicit staging does not fire. The strict multi-vShard route instead invokes try_multi_shard_predicate_recon.
• Recon scans n=1 before the first point update, producing an empty set.
• The point writes keep the submission nonempty, so Calvin stages t.x=1, an empty bulk update, then u.y=9.

The stored result is t.x.n=1, instead of 2. All three commands use UPDATE, avoiding the separate mixed-command response-fold limitation.

Preserve statement order for batches containing a predicate and preceding writes, regardless of those writes' plan variants. Add mixed point/predicate coverage across vShards.

[EnRaiha]

Thanks for the second pass. The series is now at ea23abef0, 11 commits, rebased on main 8b1318b80, and the earlier SHAs I quoted no longer exist on this branch.

Remaining finding (predicate beside preceding point writes). Reproduced as written: with your batch the stored result is t.x.n = 1. The classifier now sends a request to implicit staging when a predicate write follows any other write, not only when it follows another predicate. It ignores the balance-delta tasks of a settled sum, which follow the write that moved them. New test a_predicate_reads_the_point_write_before_it_across_two_vshards uses your two collections on different vShards: ["1"] before the change, ["2"] after. It is part of the run a request of several predicate writes as one implicit transaction commit.

Publication cleanups. The Claude-Session trailers are removed from all 11 commits. The PR description is rewritten for this revision: 11 commits, the staging-reported matched set, __nodedb_key, the current test table, and what is not run yet.

On this head I ran the wire filters for edge, predicate, materialized sum, transactions, triggers, procedures and graph RAG (471 pass), in-process txn (32 pass), and library calvin, session:: and txn_route (835 pass). The full suites and the three-node dml_suite and common_suite run next. I will post the counts here.

[farhan-syah]

Static re-review passes at ea23abef0d5d6316817ff2eed99243f2b10d0d4a. All five original findings are resolved in the reviewed code.

Previous finding	Status
Capture the prediction from the staging scan	Resolved
Enforce recomputed transitions before Calvin votes commit	Resolved
Predict later statements against preceding writes	Resolved
Read temporal identities from the versioned table	Resolved
Separate owner identity from user-named score fields	Resolved

The classifier now detects a predicate after preceding writes, including point updates. That routes the reported mixed batch through implicit staging. The new regression test uses both vShards and checks t.x.n=2 and u.y.n=9. I inspected the test without executing it.

The other four repairs remain unchanged since the previous review. The 11-commit series contains no Claude-Session trailers. The updated PR description records the current implementation and outstanding checks.

No new static findings. git diff --check passes. Existing CI reports Static gates passed and Test Suite skipped.

Full-suite and three-node results remain pending in your update. This records a clean static review, without merge approval or independent confirmation of runtime claims. No builds, tests, or formatters ran during this review.

[EnRaiha]

This branch is rebased on main after #429 and cut to three commits (fdbe1c4ac). The earlier 11 commits are gone from the branch; your static re-review passed on the previous head, so please read this one again.

#429 closes #424 and #425, and it replaces most of what this PR did. I ran the tests of the old series on main with none of its source. The in-process tests (18), the three-node tests (13) and the wire tests for the _key alias and fusion over HTTP and native SQL (39) all pass there, so I dropped those fixes: the matched set of a staged predicate write, the empty set, the commit of a block through Calvin, the transition check, the fusion and search key naming, and the log lines.

Ten wire tests still fail on main. They are the three commits that remain:

1. A predicate UPDATE or DELETE over HTTP, NDJSON, WebSocket or native SQL outside a block, on a collection that spans vShards, answers XX000 (calvin bulk predicate write reached overlay staging without ollp_predicted_surrogates). The fix routes it through the recon coordinator and runs a request in which a predicate follows another write as one implicit transaction. That includes your batch with point writes beside a predicate.
2. Two writes of one request that move one sum balance are refused whole.
3. A hybrid score alias named doc_id is refused. It replaced the field the hit names its row by, and every row answered an empty id.

The PR description lists the red and green runs. The title and description changed: this PR no longer closes anything.

If you prefer a new PR with a smaller history and this one closed, say so and I will open it from the same branch. I run the full local mirror of the Test workflow on this head now and will post the counts.

[EnRaiha]

Results of the local mirror of the Test workflow on the current head (fdbe1c4ac, x86_64, rustc 1.96.1, nextest 0.9.143, --all-features --cargo-profile ci --profile ci --no-fail-fast):

Step	Result
cargo fmt --all -- --check	pass
The ten scripts/ci gates, cargo deny check	pass
cargo clippy --workspace --all-targets --all-features --profile ci -D warnings	fails on nodedb-sql search_cells.rs:32, an unused import already in main (not touched here)
Workspace tests without nodedb-cluster-tests	22,783 of 22,784 pass, 3 skipped. One times out
nodedb-cluster-tests	515 of 515 pass

The timed-out test is nodedb-vector search_allocations_do_not_scale_with_distance_count: 120 s limit, twice, and the same on main 8b1318b80 and on this head. This PR changes no file of that crate.

Not run here: the three extra nodedb-client clippy commands, codeql, fuzz, the wasm32 check, and an ARM64 run. The workflow's Test Suite job stays skipped on the PR without the run-ci label, so these numbers are not an independent confirmation. Could a maintainer add the label?

The binary of this head also runs on a NodeDB node with a real data set: search and GRAPH RAG FUSION answer on both of its databases.

[EnRaiha]

The head is now eb0c9b25b. The tree is identical to fdbe1c4ac, which the numbers above are for. Only the history changed: the second commit did not pass cargo fmt --all -- --check on its own (a module line in the wire test list), and the fix sat in the third commit. Each of the three commits now passes cargo fmt, cargo check -p nodedb --tests and the tests it adds when checked out alone:

Commit	Tests it adds, run on the commit alone
refuse several writes of one sum balance	2 pass
run a request in which a predicate write follows another write as one implicit transaction	15 pass (the sequence, two-statement, sum-balance and cross-shard sum tests)
refuse a hybrid score alias named doc_id	16 pass

The three nodedb-client clippy commands of the workflow pass too.

[farhan-syah]

Request changes. The three-commit rebuild reintroduces a materialized-sum concurrency error on the new recon route.

Previous review area	Current status
Staged matched sets and transition enforcement	Superseded by merged #429. These changes are absent from this PR's diff.
Temporal and search identity repairs	Superseded by merged #429. The remaining doc_id alias guard has coverage.
Mixed point/predicate batch visibility	Retained. The implicit-transaction classifier still covers the reported batch.
Publication cleanup	Three commits, updated description, no session trailers.
Settlement read protection	Regressed. Recon receives no settlement read versions and constructs an empty OCC read set.

The inline finding blocks merge. Restore the settlement reads through every recon adapter and into the transaction builder. Add deterministic coverage for a concurrent source change between settlement and recon.

Current-head CI now supplies independent test evidence:

• Workspace: 22,779 passed, including one recovered flaky test. Three skipped.
• Cluster: 515 passed.
• Static gates, CodeQL and Gitleaks: passed.
• Lint & Check: failed during cargo deny. It reports RUSTSEC-2026-0378 and RUSTSEC-2026-0379 against wasmtime 49.0.3.

I checked that the dependency manifests, lockfile and deny.toml match the base. The dependency gate error is inherited, but CI remains red.

Static source review against eb0c9b25b2e95b9bfb95c93c402c02a7ba79e40a. I read existing CI logs without running builds or tests. git diff --check passes.

[farhan-syah]

[P1] Preserve settlement reads on the recon route

This route consumes the tasks without passing sum_target_reads, then returns before the normal Calvin dispatch uses those reads. dependent_recon.rs:405-410 builds the predicted transaction with &[] instead.

Cross-shard balances still carry deltas computed from source images during planning. Their protection requires the corresponding OCC read versions. The Data Plane's materialized_sum/divergence.rs:98-111 explicitly skips cross-shard bindings because it relies on those reads.

A concrete interleaving:

1. One source row has amount 10, and its cross-shard target balance is 10.
2. Planning a predicate SET amount = 1 produces a shipped delta of -9.
3. Another request commits source amount 20 and target balance 20 before recon.
4. Recon sees the same row identity and join key, so its prediction does not detect the amount change.
5. Calvin sets the source amount to 1 and adds the stale -9 delta, leaving balance 11.

This commits a balance unequal to the source sum. The added uncontended settlement tests do not exercise this window.

Thread settlement reads through recon into build_single_vshard_predicted_tx_class. Preserve those versions across retries, or refresh settlement and reads together. Add a deterministic interleaving test that rejects or correctly recomputes the stale settlement.

[EnRaiha]

@farhan-syah The settlement-reads finding is addressed on this branch. Five commits sit on top of eb0c9b25b, head cfe5152e4:

• 55fb68050 threads sum_target_reads through the SQL, native and pgwire OLLP recon adapters into build_single_vshard_predicted_tx_class. A stale read aborts with 40001.
• 29047a3d9 does the same for the pgwire implicit edge recon (pre_dispatch.rs). Its tasks are already settled, and the reads were in scope.
• a33b6df63 fixes TRUNCATE on the recon route. dispatch_truncate kept only the TRUNCATE task and passed no reads, so the settled ApplyBalanceDelta tasks were dropped and the balance stayed at its old total. Without the new route an edge-free TRUNCATE passed, so this PR would have introduced that regression. The edge-bearing case also fails on main.
• 468ac5340 and cfe5152e4 add the tests: TRUNCATE of a cross-shard sum source, and the settlement interleaving over HTTP, pgwire, native SQL and native bulk update.

Two questions:

1. is_edge_recon_plan includes CrdtOp::DocDelete, and dispatch_crdt_doc_deletes ignores the reads. I found no sum binding that can name a CRDT collection (planner/materialized_sum has no CRDT path). Is that an invariant? If yes, I add a debug_assert there. If a CRDT source can bind later, I thread the reads the same way as TRUNCATE.
2. I could not test a concurrent source change between settlement and submit for TRUNCATE. The recon::before_submit gate is not on that path, because edge_truncate.rs submits through submit_calvin_routed directly. May I add a truncate::before_submit gate to fail_gate.rs for this test, or do you prefer another hook?