[kriszyp]

Depends-on: HarperFast/harper#3322

⊙ Problem

After count-based placement changes, an excluded peer can retain stale indexes and residency. Base copy and GET_RECORD also treat stored stubs as complete records, while failed remote reads can appear absent.

🤔 Keep the current placement contract

Yes to correcting these outcomes without changing relocation-on-read. A stored stub must stay partial across every transport boundary.

💡 Solution

Send current invalidations to every excluded record-based peer. Copy stubs as invalidations with residency IDs to peers that advertise residencyStubCopy, refuse them in GET_RECORD, and try the next listed resident. Absence requires every non-self resident to answer; a disconnected resident leaves the existing read error. The core companion supplies read-error propagation and correct stub indexes.

⚖️ Alternatives

Adopted the planning review's complete-residency absence check and preservation of id-derived first-answer behavior. Rejected a new no-relocate query API because the task keeps current relocation semantics.

Adopted a capability gate instead of a preparatory receiver release (🤔 below). No new "not resident" GET_RECORD answer (🤔 empty-response protocol).

Framing-Verdict: chosen-approach-sound

🔧 Changes

Product and architecture tour

Copying an illustrative stored stub

replicationConnection.ts:7189

entry = {
  key: 'r1', value: { id: 'r1', name: 'updated' },
  metadataFlags: INVALIDATED, residencyId: 7
}
stubAsInvalidation = isStub && peerCapabilitiesLearned && peerSupportsResidencyStubCopy(peerCapabilities)
copyType = stubAsInvalidation ? 'invalidate' : 'put'
createAuditEntry({ type: copyType, residencyId: entry.residencyId, … })

Product and architecture tour

From stale stub to full read

replication/replicationConnection.ts repairs the excluded-peer send branch, copied-row type/residency flag, and GET_RECORD handling. Healthy retrieval also needs current table structures/schema; exact-sized reads and the table name on every request come from #1086, which this branch is based on. Review the stored-to-audit residency flag conversion most closely.

replication/replicator.ts continues after an empty stored-stub response. It keeps id-derived lookup behavior and returns absence only when all listed remote holders answered successfully.

Retrieval outcomes

An empty response advances to the next listed holder; uncontacted holders prevent absence.

flowchart LR
    S[Local stub] --> G[GET_RECORD from listed resident]
    G --> F[Full row returned]
    G --> E[Not held: try next resident]
    E --> A[All answered empty: absence]
    E --> U[Uncontacted resident: read error]

Loading

The separate core pointer commit brings in the companion. replication/DESIGN.md records the transport invariants and DESIGN.md links them.

🤔 Keep the existing empty-response protocol

An empty GET_RECORD response represents both a missing row and a refused stub. As requested, try the other named residents and return absence only when all answer empty. This preserves eventual consistency; it does not arbitrate concurrent deletes or add tombstones. Errors returned by a still-connected resident retain the existing fatal behavior.

Known limitation: a stub list can be stale, for example one written while excluded peers still skipped invalidations. If every holder it names now holds only a stub, each answers "not held" and the read returns a false 404, even though other nodes hold the record. A distinct "not resident" answer is being decided separately and is not part of this PR.

🤔 Send schema with the cached structure frame

#1086 caches the GET_RECORD structure frame per table id, keyed on table identity and durable dictionary counts. This PR adds the table's attributes and schemaDefined to that frame, as the subscription stream already does, and re-announces when the table's schemaVersion or schemaDefined changes. The receiver therefore gets current schema without a frame on every read.

🤔 Keep subscription decoder IDs consistent

Blob repair (from #1086) uses the peer's announced table ID for both its request and response decoder. Until a usable decoder arrives, it fails locally rather than guessing an ID that could replace another streamed table's decoder. After reconnect, an idle table may have no decoder until its next replicated write, so repair_blob_data can fail on every peer for that table in the meantime. This is an explicit availability tradeoff pending a separate table-ID discovery or retrieval channel; do not infer that reconnect alone makes repair usable. Subscribed senders also refuse mismatched IDs.

🤔 Send the invalidation when placement changes

This adds an invalidation to peers excluded from both residency lists when placement changes. Unchanged-placement writes already sent it: previousResidencyId is recorded only when the list changes. The extra projection/frame is required to keep their indexes and holder list current; skipping it is the reported bug. No throughput improvement is claimed.

🤔 Copy stubs as invalidations only to peers that advertise support

A receiver without core's equal-version guard would demote its full row on a copied stub invalidate. The copy now sends that only to peers advertising the new residencyStubCopy key. Older or not-yet-learned peers get main's exact frame, never a skip. So older peers keep today's stub-copy behavior until they upgrade. Excluded-peer invalidations and the empty GET_RECORD answer stay ungated; older nodes already handle both. The key is advertised by Pro, not derived from core; Pro always ships its pinned core.

🤔 Leave retrieval lifetime policy unchanged

GET_RECORD still has no request deadline, and an already-retired session can register a waiter after its close sweep. Those existing requester-side issues need separate timeout/admission policy. The responder reads synchronously, so its table checks hold through the send; this does not solve requester lifetime.

🤔 Relocation remains outside this fix

Full-query materialization uses existing relocation-on-read, as the task requires. Without default placement, that path can mark other peers' stubs complete; on RocksDB it can also fail its audit append after returning the read. These pre-existing relocation defects remain for #436/#980. Operations API main-thread loading and non-indexed predicates/sorts also remain incomplete. Streamed collection errors can follow HTTP 200 headers.

✅ Verification

• Rebased onto main c9ab56b9 with Add static-sharding coverage and repair cold remote reads #1086. The GET_RECORD responder combines both PRs: Add static-sharding coverage and repair cold remote reads #1086's synchronous exact-sized read and per-table structure cache, plus this PR's schema fields and stub refusal. RocksDB and LMDB: wire tests 28/28, residencyReads 5/5, Add static-sharding coverage and repair cold remote reads #1086's staticSharding 8/8. Full Pro units: 1,949 pass, 2 pending.
• The same suites pass with core set to harper#3322 merged into harper main 07568b1e. That is the state sync_core produces once the companion merges.
• Schema re-announce keys on schemaVersion, because schema reloads edit attributes in place. Its new test fails with an array-reference key.
• Forced-build A/B: all 5 four-node tests fail on base and pass on RocksDB and LMDB with the fixes.
• Wire tests: 10 failures on base, 12/12 pass on each engine after the fix; missing-row and id-derived responses are unchanged controls.
• Core companion: 24 base failures on RocksDB and 23 on LMDB; 30/30 and 29/29 pass respectively (one HNSW skip) after the fix.
• Capability gate: older and unlearned peers get bytes equal to main's frame, and the key resolves absent as unsupported; both cases fail with the gate reverted (RocksDB and LMDB). Pro units 1,955 pass. No mixed-version cluster test: the core guard would keep it green.
• Pro typecheck, required lint, and formatting pass. Full Pro integration at a9949f85 (before the rebase) passed: 262 tests, 15 skipped, zero failures.

Regression coverage

A/B method: swap only the changed production sources to the named base revisions, rebuild core with npm run build (fresh TypeScript program), and Pro with npx tsc --build tsconfig.json --force; run the same tests, restore sources, rebuild, and repeat. Fixtures and assertions stay identical between runs.

integrationTests/cluster/residencyReads.test.mjs covers writer-plus-one placement, full reads from non-residents, HTTP collection loading, a peer excluded from old and new residency, retry after not-held, successful absence, and all/partly unreachable residents. The shared fixture's schema.graphql adds an indexed field; resources.js inspects raw local state and creates stale lists for retry checks without a read relocating the probe.

unitTests/replication/residencyWire.test.mjs drives the real WebSocket handler and copy loop over real stores, decodes the transmitted copy entries and full response, and checks repeated requests carry their table name. Loader tests capture the registered real source and mock transport replies to assert exact contacted-peer order, including successful absence and an unreachable holder. Three additional cases fail on the previously reviewed build: peer-ID request/response decoding, missing-decoder refusal, and mismatched-ID refusal on a subscription socket. Base sources: Pro 8383c5f7 (both changed replication files identical to branch base 1dd2f982), core 1fd591ba2. Every base run rebuilt dist. Temporary node directories are removed by teardownHarper; each node is registered for cleanup before fixture copying. Copy correctness uses the real wire loop, as the task permits, without adding a new restart contract.

Planning verdict: Framing-Verdict: chosen-approach-sound after adopting the capability gate.

Per-test rebuilt-base proof (RocksDB / LMDB)

Passes on base are the unchanged missing-row and id-derived controls.

Test	Base	Branch
handles retry after the first resident answers not held	fail / fail	pass / pass
handles absent after the first resident answers not held	fail / fail	pass / pass
handles unreachable after the first resident answers not held	fail / fail	pass / pass
handles id-derived after the first resident answers not held	pass / pass	pass / pass
names the table on repeated requests so each can resolve the live table	fail / fail	pass / pass
uses the subscription peer table id for blob repair requests and response decoding	fail / fail	pass / pass
refuses blob repair before the subscription has a usable table decoder	fail / fail	pass / pass
rejects mismatched table ids on subscription sockets before announcing a decoder	fail / fail	pass / pass
answers GET_RECORD for stub without promoting partial data	fail / fail	pass / pass
answers GET_RECORD for missing without promoting partial data	pass / pass	pass / pass
answers GET_RECORD for full without promoting partial data	fail / fail	pass / pass
copies a resident-targeted stub as an invalidation with its residency id	fail / fail	pass / pass

Refs #436. No documentation companion: fixes existing semantics; owning DESIGN notes are updated.

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

Release target: v5.4 — matches #436.

Related PRs: #940 overlaps; #1086 overlaps; #1101 overlaps; #1103 overlaps; #554 independent; #1100 independent
Complexity: complicated

Deferred findings

• cf00bc41b2cfacbc major correctness — core/resources/Table.ts — Non-indexed query filters and sorts operate on stub projections — NOT FILED
• 2fdfc4ef7a5b5e94 major correctness — replication/DESIGN.md — Non-indexed query filters and sorts operate on stub projections — NOT FILED
• 54cdaaaf983bff99 minor correctness — replication/replicator.ts — Self-only residency cannot produce absence — NOT FILED
• d88d81b1de038285 minor reliability — replication/replicator.ts — Connected-resident errors prevent fallback to healthy residents — NOT FILED
• b3fb5dc73f752e37 minor data-integrity — replication/replicationConnection.ts — Base-copy expiration metadata is lost — NOT FILED
• 46ae0b5775bb2027 minor reliability — replication/replicationConnection.ts — Unknown GET_RECORD response IDs disconnect the link — NOT FILED
• 75f27cf9ccbf2652 nit maintainability — replication/replicationConnection.ts — Copy wrapper flags and wire flags have different representations — NOT FILED
• c9ec81d5c278f251 nit performance — replication/replicationConnection.ts — stub GET_RECORD reads structure counts and may announce before refusing — NOT FILED

_{Dispatch: task hp-residency-read-correctness · queued by unknown · ran by codex/gpt-6-astra/xhigh · worker kzyp-xps-1}

_{Review-Coverage: authored=claude; ran=codex,cursor-grok; pending=cursor-grok; adjudicated=domain; blocked=gemini(timeout); declined=cursor-composer,cursor-kimi,cursor-muse,muse; rounds=13; full=5 @ 940a578}

_{Review-Attention: deep ~35m (critical: protocolCapabilities.ts, replicationConnection.ts +1; decisions: excluded-peer-refresh-every-write, capability-hardcoded-in-pro, source-error-semantics, collection-loads-through-relocate, do-less-alternative; raised: degraded review) @ 940a578}

[gemini-code-assist]

Code Review

This pull request implements send-side residency to ensure that partial records (stubs) are never sent, served, or stored as complete records during replication copy or retrieval paths. Key changes include updating replicationConnection.ts to handle invalidation copies, prevent stubs from being returned in GET_RECORD responses, and resolve peer table IDs for blob repairs. Additionally, fallback logic in replicator.ts was updated to handle cases where residents are unreachable, and comprehensive integration and unit tests were added. Feedback on the changes suggests removing a redundant optional chaining operator on tables in replicationConnection.ts since a prior guard already guarantees its definition.

[gemini-code-assist]

The optional chaining operator on tables is redundant here because the prior guard if (!table || ...) already guarantees that table is defined, which in turn guarantees that tables is defined and not nullish. To adhere to the rule of avoiding blind optional chaining when prior guards guarantee definition, use direct property access instead.

Suggested change

	tables?.[message[4]] !== table ||
	tables[message[4]] !== table ||

References

1. Do not blindly add optional chaining (?.) based on static analysis suggestions if prior guards or early returns already guarantee the object is defined.

[kriszyp]

Codex: removed the redundant optional access in 54cbd46. I checked all assignments to the captured table map: after the earlier table guard succeeds, none clears that map, so direct access preserves the post-await identity check.