PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

• Decision: Approve
• Rationale: Merge-safe at the exact head — the streaming restructure is behavior-preserving where it counts (counts, envelopes, guards, explicit-vector path), the memory bound is proven by both a deadlock-style falsifier and disposable-process RSS controls, and the intake narrowing (#15693 owns the durable resume contract) keeps this leaf cleanly data-plane-only.

Peer-Review Opening: Emmy, the 20k restore went from ~922 MiB to 228 MiB peak RSS and the orchestrator's 1 GiB envelope breathes again — and you got there without touching a single importer contract. The gated-EOF witness is one of the best falsifier designs I've reviewed this week.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: the PR body (incl. the RSS control table and Evolution note), both importer diffs, both spec diffs, and my earlier peer-role analysis of the same lane (the #15643 audit — restore paths pass explicit vectors, so the fix axis is importer memory, not re-embedding).
• Expected Solution Shape: flush during async JSONL iteration — never materialize the full file before the first store write; merge-mode existence checks and missing-row writes scoped inside one 250-row batch; replace mode streams the same bound; counts, envelopes, guards, and the explicit-vector contract unchanged; no cursor/checkpoint/scheduler creep (that's #15693's contract).
• Patch Verdict: Matches. Both importers accumulate into a batch array that is detached before each awaited write (rows become collectible immediately), flushBatch fires per bound and once at EOF, merge keeps its preserve-live semantics with a batch-local existence Set, and reEmbed moved from whole-array strip to per-chunk strip with no semantic change. The gated-EOF spec proves row 500 reaches upsert() before EOF is even readable — a whole-file materializer deadlocks on the gate by construction.
• Premise Coherence: coheres with verify-before-assert — the memory claim is measured (5k/20k disposable-child controls against recorded pre-change numbers), not asserted; and with the intake discipline — the unowned durable-resume surface was narrowed out before code.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15692
• Related Graph Nodes: #15691 (embedding-compatibility preflight), #15693 (orchestrator-governed restore-delta-merge), #15695 (restore-stall reproduction + pre-change RSS anchors), ADR-0027 (recovery actuator authority)

🔬 Depth Floor

Challenge (non-blocking): one subtle semantic shift worth naming for the record — an intra-file duplicate id (same id in two different batches of a corrupt backup) previously failed at add() (counted failed); now batch 2's existence check sees batch 1's freshly-added row and skips it as existing (counted skipped). The new behavior is arguably the truer preserve-live idempotency, but the edge's count semantics changed — if any consumer ever reconciles failed-vs-skipped on corrupt inputs, it will read a different number. Secondary: the RSS controls are disposable-child measurements (honest L2); a bounded-heap harness spec asserting peak RSS would turn the regression into a permanent CI gate, and the PMV corroboration on the next off-host restore is the right place to confirm the shape in production.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The detach-before-await pattern is the correct way to stream with backpressure in async iterators: flush into a local, null the reference, then await the store — rows are collectible while the write is in flight, and the loop's natural pause is the backpressure. Also the gated-stream falsifier: the strongest way to prove "no full materialization" is to gate EOF behind the first store write and watch the importer flush before the stream ends.

🎯 Close-Target Audit

• Close-targets identified: Resolves #15692 (leaf, newline-isolated); Related: #15691, #15693, #15695 all non-closing
• #15692 confirmed not epic-labeled

Findings: Pass

📑 Contract Completeness Audit

• Importer call signatures, return envelopes, preserve-live counts, explicit-vector behavior, and destructive-operation guards are unchanged (verified against the diff; the JSDoc additions describe the bound without altering the public contract).
• Batch invariants hold at the boundaries: KB flushes at exactly 500 (+final), MC existence/add pairs stay inside each 250-row window, replace streams 250-row upserts (250/250/1 pinned in spec).

Findings: Pass

🪜 Evidence Audit

• PR body contains an Evidence: declaration: L2 (native importer/restore specs plus disposable 5k/20k child-process measurements with strict collection seams) → L2 required (bounded row/ID retention, truthful counts, zero new provider authority). No residuals claimed — consistent with the leaf's deterministic coverage.
• The RSS table is honest measurement against the recorded pre-change controls in #15695 (921.5 → 228.0 MiB MC, 934.2 → 226.5 MiB KB at 20k); the PMV is correctly labeled corroboration, not a close blocker.

Findings: Pass

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI green at 46650f66d3 (no non-SUCCESS, none pending) + 18 focused importer specs + 35 restore specs + the RSS controls.
• Reviewer falsifier: N/A — the gated-EOF deadlock witness and the per-batch call-order assertions (get → add → get → add → get → add with max-250 payloads) are precisely the falsifiers I would have written.
• Test location: both specs live with their owning importers; the global createReadStream patch is serial-scoped and restored in finally.

Findings: Pass

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 95 — streaming lives inside the two importers that already owned the write path; no new authority, no checkpoint/scheduler creep; the narrowing kept the leaf data-plane-only.
• [CONTENT_COMPLETENESS]: 93 — both importers' JSDoc now states the bound explicitly; the PR body's Evolution section documents the intake narrowing honestly. 7 deducted: the intra-file duplicate-id count shift (fail → skip) is not named anywhere.
• [EXECUTION_QUALITY]: 93 — the deadlock-style EOF gate is a real falsifier, not a shape-check; call-order and payload-max assertions pin the bound on both modes; full CI green with the restore suites intact.
• [PRODUCTIVITY]: 95 — the measured outcome (~4× RSS reduction at 20k) closes the cloud-restore dead-end class this audit chain started with; no contract grew.
• [IMPACT]: 75 — restores at realistic memory scale now fit the orchestrator envelope; unblocks the #15693 orchestrated path on a bounded foundation.
• [COMPLEXITY]: 45 — two importer loops restructured + two focused spec additions; moderate reader load, all in the right places.
• [EFFORT_PROFILE]: Quick Win — large memory win for a contained, well-evidenced diff.

The gated-EOF witness — deadlock unless row 500 flushes before the stream ends — is how you prove a negative without a single expect(memory). Beautiful. 🌈