[kriszyp]

⊙ Problem

Deleting and recreating {a: null} stores the row but loses its null index entry. Reloading a deleted sourcedFrom cache row has the same failure, so null queries miss it.

🤔 Recreated nulls must remain queryable

A deleted row recreated with an explicit null should be queryable like a newly inserted row. Matching guards at the shared comparison and named regression tests enforce this for normal writes and cache fills.

💡 Solution

In existing-record guard, treat a null or undefined existing record as absent when diffing index values, matching the new-record guard. Resolver handling is unchanged.

Framing-Verdict: chosen-approach-sound

🤔 Existing index damage has no automatic repair

This fixes future writes. To repair already-missing entries and pre-fix phantom null entries, drop and re-add the affected attribute’s index to trigger backfill. With a retained tombstone, writes no longer incidentally remove legacy phantom null entries. We accept losing that cleanup: absent records contribute no index values, and no migration is included.

⚖️ Alternatives

• Caller normalization repeats the rule across eight callers.
• Changing tombstones changes deletion-version storage and its consumers.
• Rebuilding alone allows the next reload to lose the entry again.

🔧 Changes

Product and architecture tour

Null records and null fields

Which existing values contribute to the index diff?

Existing record	Previous indexed value	Fixed indexed value
Tombstone null	null	undefined (absent)
{a: null}	null	null
{a: [null, 'x']}	[null, 'x']	[null, 'x']

✅ Verification

PR QA: Do null indexes survive source 404 recovery and restart? — queued @ c589412, qa-pr-harper-3921-c589412

• Four raw-index and REST regressions fail on forced-rebuilt base ee7f9b613 and pass on both engines: scalar/array null reinserts and audited/unaudited REST DELETE → GET cache reloads. Raw index checks and REST null queries verify committed results.

• RocksDB resource CI partition: 6,403 passing, 235 pending. Full LMDB resources: 4,873 passing, 1,319 pending. Core CI selection: 5,141 passing, 194 pending.

• Build, lint, full formatting, design-doc and exact-partition checks pass. Existing full-product TTL/index oracles pass on both engines; new HTTP cases use a superuser fixture.

• CI: Windows integration 5/6 hit a 60-second fresh-install startup timeout. Resource O–Z hit txn.state ENOENT; isolated case passes, CI retry pending.

Docs: not needed — restores existing null-index semantics. Optional comment/recovery documentation is tracked in legacy recovery, including the nit still labeled NOT FILED below.

🤔 Broad local gates did not finish green

Component timing failure reproduces on rebuilt base. Server units hit one setup timeout; that file passes alone. Monolithic unit runs timed out. Full integration timed out after TTL, branch-lifecycle, early-hints, query, put-resolution and upload failures; causes remain unclassified. Shared-host port contention also occurred. Clean CI is required before promotion.

_{🤖 Generated by OpenAI Codex; posted via @kriszyp.}

Release target: v5.4 — current main resolver/stub implementation; the underlying bug exists on v5.3, but no backport is requested
Related PRs: #2155 overlaps (same existing-value expression; partial scan: one large PR unverified), 1 others independent
Complexity: medium

Deferred findings

• 88850e047354714d major data-integrity — resources/Table.ts — Legacy phantom null entries lose incidental cleanup — filed Validate recovery for existing missing and phantom null index entries #3918
• 62472ee5f516e956 nit test-coverage — unitTests/resources/indexOrphanStagingOrder.test.js — Existing transaction-ordering fixture lacks explicit null values — filed Add explicit null coverage to same-transaction index ordering fixture #3919
• 86df0bcc81306afe nit documentation — resources/Table.ts — Adjacent explanation describes only the removal-side guard — NOT FILED

🤔 defer this PR's major data-integrity finding 88850e047354714d (Legacy phantom null entries lose incidental cleanup) to #3918 instead of fixing it here?

Origin — the dispatch brief this PR was written from

updateIndices() in resources/Table.ts (~8804) computes existingValue = existingRecord && (resolver ? resolver(existingRecord) : existingRecord[key]); for a tombstone existingRecord is null so existingValue is null, and a new explicit null value then hits if (value === existingValue && !isIndexing) continue; and the [null, id] index entry is never written. The new-record side (~8803) already has the record == null ? undefined guard from #1896. Treat a tombstone/null existing record as undefined on the existing side too. Affected paths: plain table insert-null/delete/re-insert or upsert; sourcedFrom cache reload after REST/ops DELETE or origin 404 (getFromSource ~9702 passes existingEntry?.value); expired entry then origin 404 then back; audit:false cache with a Resource-subclass source; write-through DELETE then PUT {a:null}. Effect: ops search_by_conditions equals null, REST attr==null and SQL v2 IS NULL miss the row until a later non-null write.

Acceptance

Regression tests for the plain-table leg and the sourcedFrom cache leg (REST DELETE then GET reload) on rocksdb and lmdb, each shown failing on origin/main sources (forced rebuild) before the fix; existing index unit tests stay green.

_{Dispatch: task harper-index-null-over-tombstone · queued by finding-triage/wave-45 · ran by codex/gpt-6.1-sol/high · worker kzyp-xps-1}

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

_{Review-Attention: study ~11m (critical: Table.ts; decisions: shared-tombstone-normalization, legacy-repair-placement, deferred-88850e04) @ c589412}

[gemini-code-assist]

Code Review

This pull request updates the existing value resolution logic in resources/Table.ts to check if existingRecord is null or undefined using loose equality, returning undefined if so. This ensures correct behavior when handling tombstones. Additionally, a new test suite has been added in unitTests/resources/indexNullOverTombstone.test.js to verify indexing of null values over tombstones. There are no review comments, and I have no feedback to provide.