Resolve post-ordering sort keys once per row; renew a multi-store chain's idle window at each native commit

[kriszyp]

Two intermittent Unit tests: lmdb failures on main, first seen together at a6c24a542 (run 35279766894), traced to two product mechanisms and fixed with fails-on-base regression tests. Neither is caused by the merge at that head (harper#2661 changes one error-message string); no revert is warranted. Design note with the investigation, alternatives and review record: docs/design/sort-key-decoration-and-commit-phase-rearm.md.

Post-ordering re-read records per comparison after a GC. transformToOrderedSelect compared entries by dereferencing their records on every comparison. A cached entry holds its record only weakly, so a record collected between collection and comparison was re-read from the store on every comparison and once more at materialization (query.test.js "narrow constraint sorting on different property": 40 to 76 reads for a 20-row query against a bound of 25, reproduced locally with a forced gc()). The sort clauses are now resolved once per entry as it is collected, into one sidecar key array per clause, and a packed position array is sorted over those keys; the comparator never touches a record.

A multi-store commit cascade was exposed to the idle limit after its pre-commit phase. When the head's pre-commit work (a blob file write) finished, every link kept whatever remained of its idle window, and this.next.commit() is only reached after the head's asynchronous native commit. A monitor tick in that window poisoned the second link after the head had committed: a partial multi-store commit (txn-tracking.test.js "keeps every multi-store link alive", reproduced locally by deferring the head store's ifVersion 250 ms; head record present, secondary absent). Each transaction class now owns renewIdleTimeout() from its own engine's expiration (RocksDB, LMDB), and a link renews every remaining open, unpoisoned link (renewChainForNativeCommit()) the first time it hands writes of its own to the store, so every hop of the cascade starts with a full window and a stalled native commit stays bounded by one.

For the human reviewer

1. Requirement. The task asked to attribute and resolve the red head. Both failures are pre-existing intermittents (the query one hit main at 75e6f766e and a PR head before this; the monitor one hit a PR head at 07:03Z the same day), so the deliverable became two product fixes rather than a revert or a test tweak. Both are small and independently revertable; neither changes an API, a storage format or a default.
2. Eager later-clause resolution (overruled review suggestion). Every sort clause is resolved for every row at collection. The old comparator resolved clause N+1 only on a tie at clause N, twice per tie comparison with no bound in n; collection resolves it exactly n times. A resolver that cannot evaluate a returned row now fails the query at collection instead of failing only if that row happened to be tie-compared. Reversal is a contained change to collect. Say no if a later-clause resolver being called for every row is a cost you do not want to pay by default.
3. Per-hop re-arm vs a "sealed" chain state. A committing chain gets one fresh idle window per hop, renewed by each write-bearing link at its first native entry (!retries on LMDB, no retry-round options.transaction on RocksDB). A native commit that outlasts a full window can still leave an earlier store durable while a later link is reaped; that is the existing bound, unchanged, and this PR does not claim cross-store crash atomicity. The alternative, a state the monitor skips once commit() is entered, removes the bound entirely. Re-deciding reopens the Force-committing an over-time transaction leaves orphaned secondary-index entries (atomicity violation) #1407/Over-time transaction abort deletes the pre-saved deploy payload blob, leaving later deploys referencing a destroyed blob (regression from #1411, 5.1.21) #2062 interaction.
4. Renewal gate shape. A link whose own writes were all removed renews nothing; it also performs no native commit, and the write-bearing link renews at its own hop. Recorded because the design note states it only in passing.
5. stallNativeCommit replaces a live store method in two tests. The repository's rule names sinon/rewire; there is no product seam that defers a native commit deterministically. The helper restores itself on first call and in finally. A real slow commit may enqueue the handle synchronously before its promise stalls, so the fixture's timing is a model of production timing, not production timing.
6. Design note in docs/design/. Kept there because that folder already holds design notes (record-lock-ownership.md and siblings); it carries the alternatives and review rulings a later reader of these two functions will want. Move it to the PR if the folder should hold only forward-looking designs.
7. Declined comment nit. The one-line comment on renewIdleTimeout() stays: the LMDB override is textually identical and closes over a different module's expiration, and the round-1 ledger predicted a reader would take it for dead duplication.

Where to look hardest: the two gate lines that decide when a chain renews (DatabaseTransaction.commit before it closes the head for the native commit, and LMDBTransaction.commit before the optimistic write path), and collect/comparePositions in transformToOrderedSelect, whose ordering must match the old comparator for every clause shape (descending, next, resolver-backed attributes).

Changes

• resources/Table.ts: transformToOrderedSelect collects entries with once-resolved comparable keys per clause and sorts a position array; the stale comment that claimed the sort value was stored is replaced by the invariant.
• resources/DatabaseTransaction.ts: renewIdleTimeout() (RocksDB expiration) and renewChainForNativeCommit(); called before the head is closed for its native commit, on the first non-retry entry with own writes.
• resources/LMDBTransaction.ts: renewIdleTimeout() override over the LMDB expiration (also used by getReadTxn); the chain renewal before the optimistic write path, gated on !retries and own writes.
• docs/design/sort-key-decoration-and-commit-phase-rearm.md: investigation, mechanism proofs, four-axis alternatives for both fixes, planning-review and round-1 rulings.

Verification

End-to-end route: the regression tests run through the real table, store and transaction layers under both storage engines; the failing CI leg is the LMDB unit step, which is reproduced by the same tests under HARPER_STORAGE_ENGINE=lmdb.

• unitTests/resources/query.test.js: new test hands transformToOrderedSelect real WeakRef entries whose records are collected by global.gc() after the last entry is collected; asserts count, the exact two-clause order against the ordinary search, and that store reads stay within materialization. On base: 97 reads (LMDB) and 96 reads (RocksDB) for 20 rows; with the fix: passes on both engines, 5/5 repeated LMDB runs. The existing < 25 assertion is kept.
• unitTests/resources/txn-tracking.test.js: three new tests, sharing a stallNativeCommit helper and a third database. renewIdleTimeout on an LMDBTransaction re-arms to the LMDB expiration (base: method absent). The CI shape, with the window decayed to ≤150 ms and the head's native commit deferred 250 ms, asserts every link was armed to the full 400 ms budget at the handoff and both records committed (base, both engines: Transaction was aborted after exceeding the maximum open-transaction time from LMDBTransaction.commit, the CI trace). A three-link cascade with every native commit deferred 150 ms under a 200 ms window commits all three links (base: the same trace).
• Fails-on-base was run by building dist from the base versions of the three source files and running the four tests on both engines, then restoring and rebuilding.
• Full unit gates on the final head, all green: npm run test:unit:resources 2814 passing; HARPER_STORAGE_ENGINE=lmdb npm run test:unit:resources 2178 passing; npm run test:unit:main 5759 passing; npm run test:unit:apitests 201 passing. Integration suites were not run locally; CI runs them on this PR.
• npm run lint:required clean; prettier --check clean on every changed file.
• Not covered: no integration or crash-recovery test proves the per-hop durability boundary; the review CLI's no-restart-test self-check fired for that reason and the boundary is stated in decision 3.

Complexity: complicated

🤖 Generated with Claude Code

https://claude.ai/code/session_01CsUcZR9mwDwWZWyt3PEskN

Origin — the dispatch brief this PR was written from

Investigate and resolve the current harper main Unit Test regression at a6c24a5 (harper#2661). Do not assume #2661 caused it: determine whether the failures are reproducible and attributable, then implement the smallest justified correction or open a draft PR. If the regression is attributable to the single merge and no prompt targeted fix is clear, recommend reverting harper#2661; do not perform a revert without evidence.

Acceptance

Establish the failure mechanism with a focused reproduction or CI evidence; preserve intentional transaction timeout behavior; run relevant unit tests; open a public-safe draft PR only for a justified fix. Record whether #2661 is causal, a runner flake, or merely temporal, and state whether a revert is recommended. Do not edit or broaden the unrelated MQTT fix.

_{Dispatch: task main-red-kriszyp_harper_a6c24a542 · queued by ci-health-cron · ran by claude/fable/high · worker kzyp-xps-1}

_{Review-Coverage: authored=claude; ran=gemini,codex; adjudicated=domain; declined=cursor-grok,cursor-composer; rounds=2; full=1 @ fca46bf}

_{Human-Review-Need: 3 (decisions: eager-vs-lazy-later-clauses, native-commit-stall-fixture, renewal-gate-shape, per-hop-rearm-vs-sealed-state, design-note-in-repo) @ fca46bf}

[gemini-code-assist]

Code Review

This pull request introduces optimizations and bug fixes for in-memory sorting and transaction commit cascades. Specifically, it resolves sort keys once during collection into sidecar arrays to prevent GC-induced re-read storms during post-ordering, and implements idle timeout renewals (renewIdleTimeout and renewChainForNativeCommit) to ensure each hop in a multi-store commit cascade receives a fresh idle window. Comprehensive regression tests have been added to verify these behaviors. There are no review comments to address, and I have no additional feedback to provide.

[claude]

Reviewed; no blockers found.