[ScriptedAlchemy]

Latest exact-head review repair — 8631a633 (Refs #4152)

The bounded row_chunks carry introduced at 407e488 missed valid literal chunk IDs: stored_chunk_key writes canonical IDs as SQLite BLOB and literal IDs as TEXT, but the scanner decoded every key as Vec<u8> and rebound its cursor as BLOB. The new source preserves the exact mixed SQLite storage class via rusqlite::types::Value in the read, seek and update. The on-disk production regression now inserts and confirms 6,000 TEXT + 6,000 BLOB keys before exercising mixed shifts, cancellation/deadline rollback and reopen durability. The obsolete inline test import and time-relative spill-control comment are corrected. Previous code fails when shifted TEXT keys are encountered.

A disposable SQLite 3.46.1 probe verified the same mixed-class keyset ordering, 11,980 point remaps, rollback and integrity_check=ok; this is not Rust/Bazel verification. Exact-head CI 42742985758 is queued, and review 5835936616 is COMMENTED, not merge approval. No CI or protections were bypassed. Leave PR unmerged and issue OPEN until current-head nonzero Bazel tests pass and remaining original issue acceptance is proven on master.

Focused SQLite TEMP spill repair — Refs #4152

Root causes

Incremental lexical carry used three costly statement forms on a long-lived FILE-backed SQLite builder:

1. A full CREATE TEMP TABLE ... AS SELECT copy of surviving clone exact postings. DROP leaves allocated unlinked TEMP pager space when TEMP auto-vacuum is off.
2. The first replacement performed bulk composite-primary-key UPDATEs. SQLite can create an unlinked statement-local ephemeral index that vanishes after each statement; PRAGMA temp.page_count = 0 did not detect it. Stock SQLite 3.46.1 on disk measured 4,739,004 peak unlinked bytes on the 48,000-posting fixture.
3. The existing two-phase INTEGER PRIMARY KEY remap for row_blocks and clone_occurrences has the same issue: the second bulk restoration UPDATE can allocate an unlinked ephemeral key set. The independent FILE-backed 16,000-key probe observed about 1.5 MB of transient storage with the old SQL, zero observed with point updates.

The row_chunks non-key document-ID UPDATE was separately tested using its actual positive-ID production transform on up to 120,000 rows. It produced no observed unlinked spill; the exploratory change to it was reverted in 2eed865. Do not conflate a synthetic negative-ID UPDATE with the production statement.

Source repair

Earlier 2eed865 preserved preceding contributions and uses bounded (4,096-key), primary-key-ordered scans with exact per-row updates for clone exact postings and INTEGER PRIMARY KEY carry. Moved rowids stage through disjoint negative keys, then restore. Existing ShiftV1 mapping, one-row update guards, cancellation/deadline checkpoints, transactional rollback, and unchanged schema are retained. No TEMP reset or modification to allocator, global writer policy, CI, or unrelated issue owners.

Behavioral verification

Published Rust regression coverage uses real on-disk SQLite with FILE-backed TEMP, small caches, and 48,000 clone postings or 16,000 rowid keys. It checks shrinking/expanding multi-range remaps against independent expected keys, genuine collision pressure, interrupted staging, rollback and commit across database reopen, independent data survival, and main/TEMP integrity. Linux subprocess tests sample actual unlinked etilqs_* descriptors during statements and have an old-bulk-SQL positive spill control. The rowid positive control executes both phases because the second phase causes the transient spill.

Independently executed Python SQLite 3.46.1 differential probes passed rowid and chunk positive/negative range cases and disk reopen/integrity checks, and measured the rowid/clone transient spill mechanism. These are not passing Rust/Bazel results; the published Rust regressions have not yet been executed here.

Merge and issue hold

Exact-head hosted Bazel CI #42723821065 was queued when inspected; no current-head format, Clippy, Linux Rust, Windows or native macOS green is claimed. Local Bazel is unavailable; Cargo was never used. Require nonzero Bazel tests, all applicable gates and independent review before merge. This is partial only: original in-memory dirty-page corruption causality, current-workload sorter lifetime/footprint bounds and retained/cleared projection ownership tables are not fully accepted on master.

Keep #4152 open and use Refs #4152, not Fixes. Related #4794 (table sizes), #4805 (store recovery), and all other owner branches are untouched. Mixed DRAFT #4776 and Kev/decision-model PRs must stay DRAFT and never merge/auto-merge; release #4763 is read-only. No production/infra/tags/credentials, delegation, CI weakening or scheduling changes.

[changeset-bot]

⚠️ No Changeset found

Latest commit: f91b68a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[devin-ai-integration]

Note

Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[ScriptedAlchemy]

Independent code/behavior review at exact head 4a9ff682 (Refs #4152).

I checked the complete changed file against parent adf02208 and the existing carry invariants. clone_exact_postings is a WITHOUT ROWID table keyed by (class, normalization_revision, digest, occurrence_ordinal); both deleted-range filtering and ShiftV1::sql_image come from the same source ranges. Reusing the proven two-phase shift_rowid_keys negative namespace moves surviving composite keys without a positive-key collision, keeps the transaction and rollback behavior intact, and removes the only CREATE TEMP TABLE carried_clone_exact materialization. Trigger authorization is already disabled only for this controlled incremental-carry transaction; no new bypass or TEMP reset is added.

Independent disposable stock SQLite 3.46.1 differential test: 48,000 real keyed postings, both a net shrinking and a net expanding two-range shift, 47,964 rows equal old vs new; live main/TEMP integrity both ok. The removed path's dropped TEMP copy retained 545 pages with temp_store=FILE, whereas the new two-phase update retained 0. The added Rust behavioral regression invokes the production helper with the same shape, asserts every surviving key, live integrity, rollback, commit, file mode, and a positive old-style TEMP spill control, rather than merely checking the absence of a table.

This is a focused source fix for one remaining unlinked TEMP producer, not a complete issue closure or proof of the original in-memory page-overwrite cause. No local Bazel executable is available. Hosted exact-head Bazel/format/Clippy tests are queued; this review is COMMENTED, not an approval to merge. Preserve #4776 and Kev/decision drafts and the independent #4794 owner lane.

[ScriptedAlchemy]

Review findings addressed — Refs #4152

Reviewed the published delta at 9eb32c4394f4461c2e303f159fa1d518af679fbb against concurrent parent e96a95c1ef4ec3c4211884661a55e21a156ef2a1. The peer's bounded keyset scans, exact composite-key UPDATEs, negative-key staging, checked mapping and one-row update guards are preserved. This follow-up was published using force=false with that exact expected old head, not by overwriting the concurrent change.

1. Hidden spill / previous evidence correction. The initial shift_rowid_keys bulk UPDATEs allocate a statement-local ephemeral key index (observed OpenEphemeral), even though PRAGMA temp.page_count is zero after the statement returns. An independent real on-disk SQLite 3.46.1 probe sampled /proc/self/fd during SQLite progress callbacks: the 48,000-row repeated-digest fixture reached 4,739,004 bytes of unlinked SQLite storage under the old bulk remap, versus 0 observed bytes for bounded exact-key updates. Both returned the same rows and reopened intact. This corrects the blind spot in my earlier zero-TEMP-page review; a final page count alone does not prove absence of transient spill. The concurrent e96a95c implements the bounded update; this commit adds an isolated Linux behavioral regression with the actual bulk helper as a positive spill control and live sampling around the actual replacement helper. No source-shape/opcode-string gate substitutes for behavior.

2. Cancellation was missing from the bounded loops. Both passes could execute all keyed writes without consulting CodeIndexExecutionControlV1. The production caller now passes its existing control; the helper checks it before every window and row update. The regression interrupts after a completed staging window and again during restoration, requires a typed interruption and a partially negative key set, drops the transaction, reopens the disk database, and compares every original key. A successful retry is also checked. The predecessor has no such checks and does not satisfy the cancellation assertion.

3. Fixture validity and cost. Consecutive ordinals now share the same digest/prefix, so a naive positive remap demonstrably fails a UNIQUE constraint rather than accidentally avoiding collisions. Each family crosses the 4,096-key boundary. Both rollback and commit are verified through fresh disk connections, preserving the independent row and integrity checks. Fixture insertion uses a single transaction instead of 48,000 separate on-disk commits. Existing shrink/expand and explicit TEMP-copy positive controls remain, now in the adjacent test module.

Executed here: disposable SQLite 3.46.1 probes passed both mixed-range cases (47,964 exact survivors each), two interrupted-phase rollbacks, successful retry/reopen, integrity checks, and 120 deterministic randomized mixed-range/prefix/window cases. These are SQL/behavior probes, not executed Rust/Bazel tests. No local Bazel executable is available. Exact-head hosted run 42720828288 is queued; format, Clippy, bundled SQLite Rust tests and applicable native coverage are unverified. COMMENT only, not merge approval.

Only the clone-remap source and its test module changed. This remains partial Refs #4152: no claim about original dirty-page corruption causality or all production sorters. Leave the issue open and keep #4776/Kev holds, #4763, other owners' branches and schedules unchanged.

[ScriptedAlchemy]

Independent review of exact head 9eb32c4394f4 — Refs #4152, partial fix only.

I found and reproduced a second distinct spill mechanism in the original replacement: removing CREATE TEMP TABLE avoided the persistent TEMP b-tree, but shift_rowid_keys still did two full-table PK-changing UPDATE statements. SQLite can materialize their statement-local key index in an unlinked etilqs_* file, which PRAGMA temp.page_count does not count. This is a real physical-storage defect, not a missing assertion. On a disposable on-disk SQLite 3.46.1 database of 48,000 WITHOUT ROWID clone postings, temp_store=FILE, and two independent multi-range remaps (shrink/expand), I measured 3,620,300 open-deleted bytes for the bulk UPDATE versus 0 for the bounded keyed update, while both reported 0 TEMP pager pages. Both paths preserved exactly 47,964 identical keys, passed live integrity checks, committed and reopened durably.

The focused published correction e96a95c1 reads bounded primary-key windows and updates each exact composite key through the two-phase negative ordinal namespace, preserving transaction rollback. I inspected the subsequent exact-head 9eb32c43 change: it carries execution-control checkpoints through both phases and adds cancellation/rollback, adjacent-key collision, durable reopen, and isolated Linux live-FD positive-control regressions. The latter now tests the actual physical spill that the original 0-page test missed. No overlapping owner PRs need changes.

Validation gate: the earlier 4a9ff682 CI failed its Bazel rustfmt check; current exact-head CI run 42720828288 is queued, not a pass. No local Bazel/Rust toolchain is present; the differential probe is SQLite, not a Rust test. This is COMMENTED review, not merge approval. Leave #4152 open and protected DRAFT #4776/Kev branches untouched.

[ScriptedAlchemy]

Independent issue-owner source and SQLite review at exact PR head 2eed865ff2defb40e05d9c368689e3f55cf32b62 — Refs #4152, not merge approval.

Newly measured remaining spill: the existing shift_rowid_keys helper bulk-updates INTEGER PRIMARY KEY values in row_blocks and clone_occurrences. On on-disk SQLite 3.46.1 with FILE TEMP, 16,000 rowid keys and an 8-page cache, the second bulk UPDATE (WHERE first_document < 0) allocated 1,529,300 bytes of live unlinked etilqs storage, even though TEMP page_count was zero afterward. This is the same invisible statement-local allocation mechanism corrected earlier for the composite clone-postings key. The old helper could therefore still spill after PR #4853's first fix.

The new source keeps two-phase disjoint-negative-key collision protection while using bounded 4,096-key primary-key seeks and individual keyed updates, each with a cancellation/deadline checkpoint and a one-row guard. It affects only the two integer-rowid carry callers. The on-disk Rust regression added to the PR tests both actual table shapes, shrink/expand mapping, true positive-key collisions, rollback of partial stages, commit/reopen integrity, surviving unrelated rows and Linux live-FD positive/negative controls. Its positive control deliberately exercises both bulk phases because a first-phase-only control would be vacuous for the measured spill.

I also checked the nearby WITHOUT ROWID row_chunks path, whose production UPDATE only changes a non-key positive document ID. Independent exact-statement SQLite FILE probes through 120,000 keys observed zero unlinked bytes, unlike an artificial negative-ID update. The exploratory point-update rewrite to that unrelated path was therefore removed in commit 2eed865; its original source behavior is preserved. This was a correction of an overbroad change, not a claimed row_chunks storage fix.

Independent Python SQLite mapped shrink and expansion cases successfully and reopened their durable files with integrity ok. These probes are not Rust/Bazel test passes. Exact-head CI #42723821065 is queued. Hold the merge until nonzero Bazel Rust tests, rustfmt, Clippy and relevant checks pass. Issue #4152 remains open for original memory-corruption causality, remaining projection TEMP ownership, and current-master workload bounds. Do not move work out of protected DRAFT #4776 or disturb other issue-owner branches.

[ScriptedAlchemy]

Independent corrective review of exact PR #4853 head 21b2fd47a3616bf8436d6cd02f447130500e48ca — Refs #4152; COMMENTED, not approval.

Hosted Bazel failure repaired: exact preceding head 2eed865 failed the mandatory //:rustfmt_check under CI #42723821065, with ten concrete hunks across the remap helper and its adjacent tests. The follow-up commit applies those reported formatter changes; there is no CI relaxation.

Additional pre-existing test compile blocker fixed: clones/tests.rs called the now-bounded shift_rowid_keys helper with its obsolete 4-argument signature (new signature requires execution control), and that helper no longer performs the bulk SQL needed for the positive spill control. The regression now executes the actual original two-phase composite-key UPDATE SQL directly on the disposable clone table, then runs the production bounded remap separately. I independently executed the restored SQL on stock SQLite 3.46.1, a real FILE-TEMP on-disk table with 48,000 rows and small caches. It preserved all 48,000 shifted rows (ordinals 2–16001), observed 4,739,004 bytes of live unlinked SQLite spill during execution, and still reported zero TEMP pager pages afterward. This makes the physical positive control nonvacuous and is not a Rust/Bazel pass.

The branch was advanced from exact old head 2eed865 with expected_sha and force=false; concurrent remap commits and unrelated owner branches are preserved. Current exact-head CI #42730056362 is queued; the new Rust regression/Clippy/Linux matrix are not verified. The separate reviewer observation about pre-existing unbounded row_chunks cancellation remains open and is not silently marked resolved. Do not merge until tests and substantive acceptance pass. #4152 remains open; protected DRAFT #4776/Kev and release #4763 untouched.

[ScriptedAlchemy]

Source/behavior review of current PR head 407e488ca02e490598a29db75cb702c9794ca683 — Refs #4152, partial; COMMENTED, not merge approval.

Finding addressed: carry_row_chunks still used one uninterruptible UPDATE row_chunks SET document_id = CASE ... WHERE document_id >= first across every surviving chunk. The prior #4853 point-key refactor correctly avoided bulk primary-key spill for row_blocks and clone_occurrences, but left this distinct non-primary-key update without any execution-control checkpoint inside the statement. Cancellation or deadline could therefore hold a long-lived builder transaction until the whole update completed. This was already raised in the open review thread.

Published fix: 45d908d changes only the existing row_chunks path to keyset windows ordered by the actual stable chunk_id BLOB PRIMARY KEY, with a checkpoint before each window and exact keyed update, checked row counts and checked ShiftV1 position mapping. It skips identity mappings to avoid needless writes after net-zero ranges. 407e488 strengthens the behavioral reference: expected positions are computed independently from concrete replacement ranges instead of reusing the production ShiftV1::map calculation. Both changes were exact-old-head, nonforce fast-forwards, preserving peer commits.

Independent disposable SQL probe: on stock on-disk SQLite with the same WITHOUT ROWID chunk schema, 12,000 rows, FILE-backed TEMP and 8-page cache, EXPLAIN QUERY PLAN for the bounded cursor selected SEARCH row_chunks USING PRIMARY KEY (chunk_id>?). After 4,100 scanned rows, the partial transaction had modified 4,100 rows, was neither the original nor complete result, and ROLLBACK restored all 12,000. An uninterrupted remap committed 11,987 expected survivors (11,980 updates), reopened exactly, with integrity_check=ok. This proves the SQL mechanism, not the new Rust/Bazel tests.

Committed Rust regression: both cancellation and deadline stop mid-carry with partially written rows, then roll back and reopen; a successful run preserves every independently mapped chunk and unrelated data through commit, reopen, and main/TEMP integrity checks. It would fail on the previous monolithic SQL because no code path in that statement observes CodeIndexExecutionControlV1.

Hold: current-head CI is queued; no current-head Rust/Bazel, rustfmt, Clippy, or native pass is claimed. The original in-memory dirty-page corruption cause, active-sorter lifetime bounds, and protected mixed DRAFT #4776 ownership tables remain unproven. This commit does not claim row_chunks itself caused a physical spill: independent earlier probes of its original non-key UPDATE saw none up to 120k rows. Keep #4152 open, PR unmerged, release #4763 read-only and all protected/other-owner branches untouched. No Cargo, delegation, or CI weakening.

[ScriptedAlchemy]

Refs #4152 — source and behavioral review at exact head 8631a6331935e5e997b24f679f4d7fbb2f5213be (COMMENTED, not merge approval).

Confirmed blocker and repair. The prior carry_row_chunks bounded scan assumed chunk_id: Vec<u8> and always sought via Value::Blob. But the production stored_chunk_key writes canonical SHA-256 chunk keys as BLOB and valid literal IDs as TEXT. A mixed artifact with a shifted literal chunk therefore fails column conversion before it can complete the carry. This is an actual product-path regression, not merely a stylistic observation. The new source uses rusqlite::types::Value for both the fetched key and pagination cursor; the exact same SQLite storage class is bound to seeks and checked updates. No schema or identity normalization change.

Nonvacuous regression: 12,000 on-disk row_chunks entries now contain exactly 6,000 BLOB and 6,000 TEXT keys in the same primary-key table, asserting those counts before running the real production carry, both cancellation and deadline interruption with partially written rollback/reopen, mixed shrink/expand expected positions, main/TEMP integrity and committed reopen. The prior Vec<u8> implementation fails on this fixture's TEXT keys. Review-only cleanups remove an inline atomic import and a time-relative comment.

Independent disposable SQLite 3.46.1 test with 6,000 BLOB + 6,000 TEXT keys confirmed SQLite preserves both classes, 3 mixed keyset windows and 11,980 exact point remaps (without deletions in this isolated probe), correct values, live integrity ok, and full rollback. That probe is not a compiled Rust/Bazel result.

Exact-head CI run 42742985758 is queued, so new-head rustfmt, Clippy, nonzero Rust/Bazel, and native coverage are not proven. Keep PR unmerged and #4152 open: original live in-connection memory-corruption causality and residual spill bounds remain outside this partial patch. The mixed protected #4776 and all Kev/decision-model drafts remain held, and #4794 is owned by its independent lane.

[ScriptedAlchemy]

Refs #4152 — independent root-cause review of the new payload-restoration fix at exact head f835a9e2b5117e83cad20681e7b24df1a52255fb (COMMENTED, not approval).

Additional, distinct spill source proven: renumber_clone_payloads still staged changed clone_body_payloads.ordinal keys individually, but restored every negative INTEGER PRIMARY KEY in one bulk UPDATE ... SET ordinal=-ordinal-1 WHERE ordinal<0. SQLite materializes a statement-local key set outside the TEMP pager. A disposable on-disk SQLite 3.46.1 positive-control probe with 16,000 real 256-byte payloads and reversed ordinal assignments observed 5,088,100 bytes of live, unlinked etilqs_* allocation while the old bulk restore executed; temp.page_count would miss this transient file. The source-equivalent bounded 4,096-key seek/point-update path observed 0 such bytes. Both yielded identical 16,000 payloads, survived close/reopen and passed PRAGMA integrity_check=ok. This is SQLite mechanism verification, not Rust/Bazel verification.

Published focused repair: replaced only the payload bulk restoration with primary-key-ordered bounded scans, exact one-row updates, and cancellation/deadline checkpoints before each window and row. Kept the existing negative-key collision isolation, transaction scope, byte payloads, and parent/occurrence numbering semantics. Also added checkpoints to the old-key staging loop so a partially applied transaction remains interruptible.

Committed falsifiable Rust regressions: an on-disk 16,000-payload first-use reversal forces every old ordinal to move; a naive positive-key rekey must violate a real unique constraint. The production renumber_clone_payloads path is tested for interrupted partial restoration and rollback, full digest/payload integrity, occurrence repointing, independent-row survival, commit/reopen, and main/TEMP integrity. A Linux child-process test observes actual /proc/self/fd deleted SQLite files during the bulk-positive-control statement and requires no observed spill during the bounded production path. No test expectations or CI policy were weakened.

No local Bazel executable or checkout is mounted. Current-head hosted CI #42751477632 is queued; no Rust test pass or merge approval is claimed. Preserve independent #4794/#4805 and held mixed DRAFT #4776. Original in-connection memory corruption and residual workload-sorter bounds remain unresolved; #4152 stays open.

[ScriptedAlchemy]

Refs #4152 — exact-head review of published CI repairs at 18cc1a415202df9f4870318918a68f3e5f9df3fc (COMMENT, not merge approval).

The preceding exact-head Bazel workflow 42751477632 reports two actionable failures:

• Repository gates rustfmt_check_2024 demanded nine specific hunks, all in increment/clones/tests.rs.
• Strict Clippy rejected the now-unused ShiftV1::sql_image and six unhandled rusqlite::Connection::progress_handler Results (four in clones/tests.rs, two in increment/tests.rs). Ignoring handler installation could make physical-spill test observations vacuous.

The focused f91b68a deletes the retired private SQL API after its callers were migrated, applies the nine formatter changes, and checks all handler install/teardown Results with .unwrap() in tests (no bypass/suppression). 18cc1a4 aligns those chains with the repository's established Rustfmt layout. I inspected the exact one-parent, three-file and subsequent two-file diffs and re-read current master policy/remaining TableSizes and rebuild TEMP ownership areas.

Both commits were appended via guarded force=false, expected_sha fast-forwards to the existing issue PR. No other owner branch changed. New exact-head CI 42762915407 is queued; no passing Rust/Bazel/Clippy result for this head. The old fleet job was still in progress. Keep unmerged until nonzero tests and required gates pass; issue remains open for in-connection corruption causality and current-workload spill bounds.

[ScriptedAlchemy]

Refs #4152 — exact-head Bazel/behavior review on 18cc1a415202df9f4870318918a68f3e5f9df3fc (COMMENTED, not merge approval).

Re-read the current root AGENTS.md and complete source paths for incremental clone exact postings, rowid blocks/occurrences, TEXT/BLOB chunk IDs, and payload renumbering. The bounded keyset updates, negative-key collision isolation, cancellation checkpoints, and transaction boundaries remain intact; no new source defect was confirmed by this read. All five existing inline threads are resolved.

New exact-head verification: CI #42762915407 ran actual Linux Bazel //crates/tracedecay-query:unit_test and PASSED in 7.8s; repository/rustfmt gates, Clippy, Feature gates, Dashboard, change scope and benchmark self-tests are green. The Linux fleet's only failed target was //crates/tracedecay-global-db:unit_test, TIMEOUT at 300.9s, while 182 other Bazel targets passed. Its 500-test output includes registered::workflow_schema_tests::published_artifact_ceiling::background_upgrade_is_atomic_at_every_cancellation_point running over 60s; the log does not establish the precise cause of the suite timeout. This crate does not depend on tracedecay-query, and this PR changes only four tracedecay-query paths, so do not make a speculative #4152 change to the separately owned global-DB suite. The Linux fallback remains queued, Windows in progress, ordinary macOS skipped.

This is a new current-head CI merge blocker, not a confirmed SQLite TEMP data-corruption recurrence. No Cargo/local Bazel, re-run, merge, branch rewrite, or CI weakening. Keep PR and #4152 open until applicable checks and full issue acceptance are established on master; leave protected DRAFT #4776 and model PRs untouched.