[kriszyp]

⊙ Problem

Record history showed fields from the future. For a record that only received patches after some point, history reads (getHistoryOfRecord, getHistory, read_audit_log, subscription replay) put a field first written by a later patch into every earlier snapshot. A counter created by a later add showed 0 instead of being absent or null. A later put then made the leaked field vanish from the same snapshot.

🤔 Is this fix wanted at this size?

As specified, yes. It is the remaining plain-field and counter part of #1413, confined to resources/crdt.ts. It merges cleanly with the open #3504, which reworks the eviction path in the same file.

💡 Solution

• A field a later patch wrote keeps its reversed value only when that reversal is exact. Otherwise it comes from a forward rebuild, and is left out if a complete rebuild lacks it.
• Integer counters that reverse to a nonzero integer skip a rebuild.
• Over incomplete history (pruned, corrupt or body-less entries, an invalidate's stub, out-of-order merges), unsettled fields keep their old best-effort value.

⚖️ Alternatives

• Return the forward rebuild for the whole record: it would also replace fields no later patch touched, whose live value is already exact.
• Store pre-patch values in audit entries: changes the replicated audit format, and cannot fix entries already in the log.
• Rebuild every counter from history: one recent counter snapshot would read the record's whole history (2 → n+2 audit reads).

🤔 Is the remaining forward-walk cost acceptable?

A counter that reverses to 0, or has a fractional delta, still walks back to its last put: n+2 audit reads, about 12–15 ms at 1,000 patches (RocksDB, loaded box). Plain fields already pay this on main. A depth cap would make old history best effort instead. Follow-up: #3569.

🔧 Changes

• resources/crdt.ts: applyReverse sends plain sets and inexact adds to the rebuild. reconstructForward reports incomplete history through a partial out-object: a corrupt entry, a merged branch, or an entry with no readable body, which also restarts the replay. An invalidate ends the partial rebuild's history: its stub holds only projected and indexed fields. getRecordAtTime rebuilds across a newer patch with no readable body or a newer invalidate (a stub cannot be reversed from), skips older adds under a newer plain set (subtracting from the set's value invented one, or threw on BigInt(2.5)), rebuilds across a newer patch with no readable body and removes fields a complete rebuild lacks. add.reverse no longer throws on a bigint delta folded into a number field. ⚠️ Look hardest: the completeness decision: every way history can be incomplete must reach it, or the delete claims a field did not exist.
• resources/DESIGN.md: the exactness rule and its limits.

🤔 Should a newer invalidate force a rebuild from history?

A snapshot older than an invalidate is now rebuilt from history, as across a put, not reversed from the stub. With pruned history that snapshot is absent instead of partial. Follow-up: #3614.

🤔 Should a coerced counter keep its original type?

A numeric string or boolean that a later add coerced comes back as its number ('3' reads as 3). Recovering the type means rebuilding every counter from history. Changing this later is local to the trust check.

✅ Verification

• 28 new or changed tests in crdt.test.js (mock store), auditLog.test.js (real tables on RocksDB and LMDB, through getHistoryOfRecord, getHistory and readAuditLog) and subscriptionSuperseded.test.js. 18 fail on an origin/main build on both engines. The other 10 each fail when their guard is removed. At bae3b06, crdt, auditLog and subscriptionSuperseded pass on both engines (RocksDB 198, LMDB 195).
• test:unit:resources: RocksDB 4919 passing, LMDB 3730 passing. The 1 and 3 failures also fail on the base build (socket?.off, setReplicationConfirmation).
• Merged with Keep a record's audit history readable after it is evicted #3504's head: the three suites pass on both engines.
• test:unit:main and test:integration:all did not finish locally (overloaded box); CI runs them.

New tests

• Mock store: plain field omitted; number and bigint counters omitted; null restored; a counter already 0 kept; record that began with a patch; delete bound; pruned base keeps unsettled values; writes before a body-less entry not trusted; fractional delta; unsafe-integer total; out-of-order merged branch; newer body-less patch; integer counter reads only two entries; body-less and corrupt base entries; bigint delta on a number field; prototype-named field; null rebuild after a delete; invalidate in range (absence, older writes, base-less add); newer invalidate; older add under a newer plain set.
• Real tables: plain field and counter omitted, also after a later put; randomAccessFields table; null restored; pruned base.
• Subscription replay with includeSuperseded.

PR QA: Does record history stay free of future fields when two nodes patch the same record concurrently? — queued @ c776194, qa-pr-harper-3578-c776194

Docs: none needed. History now matches its documented meaning.

Refs #1413

_{🤖 Generated by Claude Opus 5.5 (Claude Code dev-agent); posted via @kriszyp.}

Release target: main — fixes history reconstruction on main; no release-line customer need recorded
Related PRs: #3504 overlaps (same file; clean trial merge), 2 others independent
Complexity: complicated

Deferred findings

• 6148b771cf8b67ff minor performance — resources/crdt.ts — add-only counters with a zero or fractional reversed result force a forward audit walk per getRecordAtTime — filed Record history rebuilds each patch entry from scratch, so listing a record's history costs n² audit reads #3569
• b98f1ab64e1fc0d4 minor data-integrity — resources/crdt.ts — out-of-order write merged after the timestamp is in the live image but never reversed — filed getRecordAtTime misses out-of-order writes merged after the timestamp, and skips the fill on a history with no put or patch #3570
• de4d2ec8c10da23a minor data-integrity — resources/crdt.ts — whole-record rebuild treats a corrupt sentinel as start of history and a bodiless put as an empty base — filed Whole-record history rebuild treats a corrupt or body-less audit entry as complete history #3565

Late Grok findings

5 surviving (highest major) — Grok reviewed 24d708e

• major — resources/crdt.ts:237 — When a patch's body can't be read (a bodyless mint, the warned path at resources/auditStore.ts:1373-1386), only partial.truncated is set. The fields from before that patch stay on priorRecord, and the fill at resources/crdt.ts:367 copies them over the correct reversed value. Example: put `{c
• major — resources/crdt.ts:137 — applyReverse now adds every key to unknowns, including keys changed only by CRDT add operations; main put only plain-set keys there. So a history read that crosses only add patches now runs reconstructForward back to the nearest put on every read. getRecordAtTime runs once per partial audi
• minor — resources/crdt.ts:363 — When reconstructForward returns null over complete history (for example the entry at auditTime is an invalidate and the one before it is a delete), the if (priorRecord) guard skips the new delete for absent keys. The reversed best-effort values stay on the returned record when they should
• minor — resources/crdt.ts:206 — The check that treats an entry read with no type as truncated history only runs when partial is set. A whole-record rebuild still treats a corrupt entry as the start of history. This was deferred earlier under whole-record-rebuild-corrupt-entry-as-history-start.
• nit — resources/crdt.ts:134 — New comments repeat one rationale in three places (:134-136, :236, :348-351) and narrate the code (:206, :238-239).

Not standing: 1 dropped-factually-wrong. Grok ran 23m (23m from launch to report); the fold adjudication (cursor-grok@24d708e74dce-7b158545, claude) took 1m.

Late Grok findings

6 surviving (highest major) — Grok reviewed b4ee076

• major — resources/crdt.ts:361 — The fix only partly restores the fast path. An add whose reversed result is 0, or any add with a fractional delta (resources/crdt.ts:144-146), still sends getRecordAtTime on a forward audit walk back to the record's base. That function runs once per audit entry during history scans (`resources/a
• minor — resources/crdt.ts:361 — The new trust gate only checks the final reversed value. It never checks the stored value before reversal or the delta. Two ways to get a wrong snapshot:
• minor — resources/crdt.ts:378 — The new delete for keys missing from a complete rebuild can drop a field that an out-of-order replicated patch wrote on an additionalAuditRefs branch. The forward walk follows a single previous-link chain (resources/crdt.ts:228-230, resources/crdt.ts:258-277), so it never collects a write that
• minor — resources/crdt.ts:330 — A patch newer than the requested time with no readable body reverses as a no-op. The keys it wrote stay out of unknowns, so their newer values leak into the snapshot. This change handles unreadable bodies in the forward replay (resources/crdt.ts:240-245) but not here in the reverse walk. Already
• nit — resources/crdt.ts:134 — Added comments:
• nit — resources/crdt.ts:378 — When the rebuild returns null, the absent-key delete is skipped. The author declined to change this (a null rebuild means there is no put or patch in range to prove the key was absent). Deferred.

Not standing: 1 dropped-factually-wrong, 1 accepted-known. Grok ran 23m (23m from launch to report); the fold adjudication (cursor-grok@b4ee076b7eba-7c26db1d, claude) took 2m.

Origin — the dispatch brief this PR was written from

Residue of #1413 in getRecordAtTime (resources/crdt.ts): the unknowns fill only overwrites when Object.hasOwn(priorRecord, key), so a plain field absent at the target time keeps its future value; add.reverse subtracts unconditionally, so a counter first established by an add becomes 0 instead of absent (or null when seeded null). Open PR #3504 rewrites reconstructForward in the same file: base on it if it has merged, otherwise coordinate. applyReverse adds add keys to unknowns; reconstructForward reports whether it hit a pruned link (expose the internal truncation); on a complete reconstruction delete keys absent from priorRecord; on a truncated one keep the best-effort value.

Acceptance

Tests on rocksdb and lmdb: a plain field first written after t2 is absent at t2; a counter established by a later add is absent at t2 and restores null if seeded null; a pruned base keeps the best-effort value. Show each new test failing on origin/main sources (forced rebuild) before the fix.

_{Dispatch: task harper-history-reverse-absent-fields · queued by finding-triage/wave-40 · ran by claude/opus/xhigh · worker kzyp-xps-1}

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

_{Review-Attention: deep ~25m (critical: crdt.ts; decisions: delete-absent-on-complete-history, trust-nonzero-integer-reversal, invalidate-policy-split, newer-invalidate-routes-to-rebuild) @ bae3b06}

[gemini-code-assist]

Code Review

This pull request refactors historical record reconstruction (getRecordAtTime and reconstructForward) to accurately handle fields first written by newer patches, ensuring that fields are omitted or restored to their correct historical values (such as null or 0) based on exact integer arithmetic reversals. It also improves handling of incomplete history, pruned base puts, and corrupt entries, backed by comprehensive unit tests. The feedback suggests that when encountering a corrupt entry with an undefined type during a rebuild, the code should log a warning and skip the entry rather than breaking the loop, to prevent aborting commits or wedging subscriptions.