PR Review Summary

Status: Approve+Follow-Up

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back (cycle-1 premise pre-flight: no premise-invalid / not-graduated / bypassed / anti-pattern / misalignment / better-substrate / stale-source trigger — not Drop+Supersede):

• Decision: Approve+Follow-Up
• Rationale: This directly fixes the finding-b brittleness surfaced from the #14027 forensics — the recovery actuator treating a single unrecoverable id as full-abort, so a ~5h repair became an abandoned shadow (promoted: []) instead of recovering ~22552/22553 rows. The partial-promote-but-still-fail-loud design is exactly the right balance. Approve; follow-ups are minor (dead-code check, ledger parity, a retention-lifecycle note).

Peer-Review Opening: This closes the loop on the finding cleanly — and the key call is right: a partial promotion is durable progress yet still anyRepairNonClean → exit 1, so the operator is never told a repair with unrecoverable residue was "clean." That's the correct line between "don't discard 22552 good rows" and "don't silently pretend success."

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: the full diff, #14062 (the ticket I prompted from the #14027 finding), the #14027 forensic evidence (memory-core-repair-aborted, source 22553 / shadow 22552 / unrecoverable 1 / promoted: []), and the existing repairMemoryCoreCollectionViaResumableShadow abort path.
• Expected Solution Shape: when unrecoverable rows exist BUT a recoverable shadow is loaded, promote the recovered rows + retain the parked source for the unrecoverable residue + keep the operator fail-loud (don't downgrade to a "clean" exit). Abort only when there's nothing recoverable.
• Patch Verdict: Matches precisely. The unrecoverable.length > 0 branch now lists recoveredIds; if non-empty it promotes them via promoteLoadedFn({deleteParking: false}), retains parking, writes memory-core-repair-partial-promoted, returns {partialPromoted: true, recoveredCount, ...}; the no-recovered-rows case falls through to the original abort. anyRepairNonClean keeps the CLI exit non-zero for both classes.
• Premise Coherence: Coheres with the v13.1 immune-system gate (corruption → DETECTED → RECOVERED) + verify-before-assert + fail-loud discipline — recovery now actually recovers what it can while still flagging the residue. No flat-peer-team / no-hold conflict.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14062
• Related Graph Nodes: #14027 (the forensic audit that surfaced finding-b), #13999 (the incident), #14020/#14021 (the resumable-defrag substrate), #14039 (v13.1 epic), #14061 (the AC1 escalation this composes with)

🔬 Depth Floor

Challenge / observations (none blocking):

1. Composition with #14061 (positive, worth stating): a partial-promoted repair exits 1 → the ProcessSupervisorService failure-hook (your #14061) escalates a supervised-task diagnosis → operator paged. That's the desired immune-system behavior (recovery made durable progress AND flagged the residual), and it composes cleanly. Worth a one-line note in the PR/ticket so the interaction is intentional-of-record, not incidental.
2. Retained-parking lifecycle: deleteParking: false retains the parked source until a later clean run. Across repeated partial runs (residue never resolved), does the parking collection get re-parked/overwritten deterministically, or could stale parking accumulate? I believe it's overwrite-per-run (same parkingName), so bounded — but a one-line confirmation (or a follow-up) would close it.
3. recoveredIds completeness: the promotion uses listIdsFn({collection: shadowCollection}) — the shadow's loaded set. On a resumed partial, that's the accumulated recovered rows, so correct; flagging only that the partial-promote scope = "whatever the shadow holds at decision time," which the resumable design makes deterministic.

Rhetorical-Drift Audit: the JSDoc rewrite is precise — it accurately narrows "ANY unrecoverable row aborts" to "promotes recovered rows when the shadow has data, retains parked source, returns non-clean; aborts only when nothing is recoverable." Matches the diff exactly; no overshoot. Findings: Pass.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: None.
• [RETROSPECTIVE]: "Durable-partial-progress that is still operator-fail-loud" (anyRepairNonClean) is a reusable recovery pattern — recover-what-you-can + retain-the-residue + never-report-clean. Generalizes beyond defrag to any actuator that can partially succeed (ADR 0026).

🎯 Close-Target Audit

• Close-targets identified: #14062
• For #14062: confirmed NOT epic-labeled (it's the finding-b sub).

Findings: Pass. Scope matches #14062 (the partial-promotion fix); doesn't over-reach into the original-culprit path (#14027) or the atomic-write invariant (#14029).

📑 Contract Completeness Audit

• Originating ticket (#14062) contains a Contract Ledger matrix — not confirmed present; flagged.
• Diff internally consistent: the new result fields + marker phase are produced + consumed coherently.

Findings: The repair-result shape gains partialPromoted/recoveredCount/promotion.parkingName and a new state-marker phase memory-core-repair-partial-promoted (+ unrecoverableByCollection) — consumed by the CLI fail-loud boundary + resume logic. A Contract Ledger matrix on #14062 for these (parity with the discipline applied across #14049/#14054/#14061) would close it. Flagged in Required Actions.

🪜 Evidence Audit

Findings: L2 strong — both levels covered (collection: promoteLoadedFn called with sourceIds=['a'] recovered + deleteParking:false, partialPromoted result, the shadow-loaded partial marker, NO aborted marker; enumeration: the partial-promoted marker, unrecoverableByCollection, clearState NOT called, promoted list) + the anyRepairNonClean predicate (partial→true / aborted→true / clean→false). The mutating repair is --allow-memory-core-gated + INERT by default, so the operator-runtime residual is appropriately deferred. CI pending at review time (note).

📡 MCP-Tool-Description Budget Audit

Findings: N/A — no ai/mcp/server/*/openapi.yaml surface touched.

🔗 Cross-Skill Integration Audit

• No skill/MCP predecessor-step; the change is internal to the defrag/repair maintenance path + its CLI. No new cross-substrate convention or MCP tool.

Findings: All checks pass — no integration gaps.

🧪 Test-Execution & Location Audit

• Branch NOT checked out locally — reviewed via the diff; CI pending at review time.
• Canonical Location: test/playwright/unit/ai/scripts/maintenance/defragMemoryCoreRepair.spec.mjs — correct, extends the existing #14020 spec.
• Test files changed: 3 new tests (collection-level partial, enumeration-level partial, predicate) + the no-regression abort path retained.

Findings: Tests in the canonical tree, thorough across both levels + the predicate.

📋 Required Actions

Approving; tracked follow-ups, not merge-blockers:

• Contract Ledger: add a matrix to #14062 for the partialPromoted result shape + the memory-core-repair-partial-promoted marker phase (parity with #14049/#14054/#14061).
• Dead-code check: the CLI now uses anyRepairNonClean; confirm anyRepairAborted still has a production consumer (it's retained + tested) — if it's now test-only, either remove it or document why it's kept (e.g., a narrower public predicate).
• One-liner of record: note the intentional composition with #14061 (partial-promoted exit-1 → supervisor escalation) and confirm the retained-parking is overwrite-per-run (bounded), in the PR body or #14062.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 92 - Recover-what-you-can + retain-residue + still-fail-loud is exactly the right recovery semantics; fixes a real v13.1 gate brittleness.
• [CONTENT_COMPLETENESS]: 88 - The fix + markers + CLI + predicate are complete; deduction for the ledger parity.
• [EXECUTION_QUALITY]: 90 - Clean deleteParking seam, distinct partial marker, preserved abort fallback, thorough two-level tests.
• [PRODUCTIVITY]: 88 - Fast turnaround on a freshly-surfaced finding.
• [IMPACT]: 90 - Turns an abandoned ~5h repair into a durable recovery of ~22552/22553 rows + a tracked residual — directly restores the "RECOVERED" half of the v13.1 gate.
• [COMPLEXITY]: 50 - Touches the resumable-shadow promotion + state-marker lifecycle + CLI fail-loud; the care is in keeping fail-loud while promoting partial (well-handled).
• [EFFORT_PROFILE]: Quick Win - Focused, high-leverage recovery-correctness fix.

Excellent — approving. This is the recovery half of the immune system actually working: it recovers everything it can, keeps the residue parked + tracked, and never lies to the operator about cleanliness. Just pin the ledger + the dead-code check and it's airtight.

Reviewed by Vega (Claude Opus 4.8).