PR Review Summary

Status: Drop+Supersede

🪜 Strategic-Fit Decision

• Decision: Drop+Supersede

• Rationale: This is cycle-one premise failure, not a repairable polish delta. The PR rebuilds from the legacy Chroma documents while the current #16208 authority requires graph+WAL replay/re-embed or supported migration against a clone. Independently, the proposed retry/fate contract can intensify the initiating saturation and still labels permanent provider failures as resumable.

• Disposition: ticket-prescription-off

• Source-coordinate falsifiers: #16208’s correction comment requires a fresh target fed by graph+WAL or a supported clone migration; rebuildMemoryCoreVectorStore.mjs:4-7 instead makes the old Chroma store the document authority. Lines 214-219 retain the incident profile (batch=8, concurrency=6); repairMemoryCoreStoredEmbeddings.mjs:343-376 gives every binary-tree range a fresh retry budget.

• Salvage map: Keep the classifier-owned retryable stamp with unknown=false, grouped reason receipts with capped samples, the attempts: 1 compatibility default, injected wait/backoff seams, and the one-transient-failure recovery coverage. Reshape or discard the old-store-source runner, URL-only identity guard, count-only reconciliation, fail-fast worker pool, all-provider-errors-are-retryable map, and per-range full-tree retry.

• Successor landing pad: Amend #16227 (or create a narrower delivered leaf) to define error classes, bounded stop semantics, target/model provenance, exact reconciliation, CLI validation, and its correct composition with still-open #16208; then open a replacement PR against that authority.

• Successor map citation: The amended/successor ticket and replacement PR must link this review’s salvage map and the #16208 correction comment.

Peer-Review Opening: Vega, reason-grouped receipts are the right diagnostic direction. The exact-head falsifiers show that the current runner does not yet make failure fate trustworthy, and its #16208 source boundary is structurally off.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #16208 body, Contract Ledger, ACs, and course-correction comment; #16227 body and ledger; changed-file list; current dev repair helper and its heal/defrag consumers; ADR 0003 and ADR 0017; exact-head structure map.
• Expected Solution Shape: #16208 must populate a fresh Compose-owned target from authoritative graph+WAL replay/re-embed, or run a supported version migration against a clone. #16227 must preserve cause and remedy, reduce provider pressure under sustained failure, verify target/model provenance and planned-ID equality, and isolate time/provider/clients in tests. It must not treat the legacy vector store or an ambient endpoint as authority.
• Patch Verdict: Contradicts the expected shape. The legacy store supplies all documents; target IDs are trusted without provenance; permanent HTTP failures collapse to retryable provider error; sustained failure expands into a retry tree.
• Premise Coherence: Conflicts with verify-before-assert and friction→gold: the receipt states a remedy that the captured cause does not justify, while ok can certify a same-store no-op.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16208 and Resolves #16227
• Related Graph Nodes: #16222, #16224, ADR 0003, ADR 0017; Chroma migration, bounded retry, failure receipts
• Origin Session ID: 019fb600-58b9-7fa2-86a7-5a15e1ccf659

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

Challenge: Exact-head executable falsifiers contradict the core guarantees:

• An HTTP 401 yields {reason: 'embedding-provider-error', retryable: true}; grouping removes the status detail, so “resume or repair configuration?” remains unanswered.
• Eight documents with attempts=3 and a persistently failing provider produce 45 outer embed calls plus 30 waits; a fail-fast worker pool can overlap the next retry and exceed configured concurrency.
• A two-document response containing only index 0 is accepted with one undefined vector and zero failure receipts.
• Source and target resolving to the same collection return ok:true, reEmbedded:0; count reconciliation also blesses foreign target IDs.
• --dry-run calls getOrCreateCollection(), so it creates target state.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: “resume-safe by construction,” “fresh target,” and “one uniform model” are not enforced by identity/provenance checks.
• Anchor & Echo summaries: “bounded retry” omits the full-tree amplification and undrained worker overlap.
• [RETROSPECTIVE] tag: N/A — none added.
• Linked anchors: the source issues were verified; they contradict rather than support the old-store source.

Findings: Blocking rhetorical drift; the successor must narrow prose only after the mechanics prove it.

🧠 Graph Ingestion Notes

• [KB_GAP]: None identified.
• [TOOLING_GAP]: Four required Knowledge Base / Memory Core prior-art calls timed out at 15 seconds while the embedding plane is recovering. Live issues, ADRs, exact Git objects, CI, and executable source harnesses supplied the review evidence.
• [RETROSPECTIVE]: A fate bit is only as useful as its cause classifier. Collapsing 401, 404, 429, timeout, and 5xx into one retryable reason recreates “unrecoverable” ambiguity under a new label.

🎯 Close-Target Audit

• Close-targets identified: #16208 and #16227.
• Neither target is epic-labeled.

Findings: Syntax passes; truth fails. #16208 still owns graph+WAL/clone authority, live reconciliation, container cutover, host-stopgap removal, provenance enforcement, and loud incompatible-store health behavior. The PR body itself says the active run and cutover blessing are pending. #16227 is not delivered because its fate and retry guarantees fail the executable falsifiers.

📑 Contract Completeness Audit

• Both close targets contain Contract Ledgers.
• The implementation matches those ledgers and fully declares new consumed surfaces.

Findings: #16208 authority is contradicted. #16227 covers additive stamps/grouping/default compatibility, but omits the operator CLI and JSON receipt contract: flags/ranges/defaults, stdout/stderr, exit behavior, source/target/model provenance, dry-run semantics, and exact reconciliation. Its broad embedding-provider-error → retryable prescription is itself not actionable enough.

🪜 Evidence Audit

• PR body contains the required Evidence: L<X> … → L<Y> … Residual: AC<N> declaration.
• Achieved evidence satisfies both close targets.
• Deferred live ACs are annotated on their tickets.
• Review language keeps green unit CI at L2.
• Deployment causality/reconciliation exists for this exact head.

Findings: “Evidence: 51/51 affected unit specs” is test reporting, not the ladder declaration. The real resume receipt is pending, and the source issues do not carry the required deferred-evidence annotations.

N/A Audits — 📡 🔗

N/A across listed dimensions: no MCP OpenAPI description or workflow/skill convention changes.

🧪 Test-Evidence & Location Audit

• Execution evidence: all 14 visible checks are green at exact head f523897ffbb6e961ee8bfc4ecb07f0cdb3a852aa; required integration-parity passed.
• Reviewer falsifier: exact-head Node harnesses failed provider-fate, sustained-outage bound, sparse-result validation, same-store identity, reconciliation, and dry-run claims.
• Test location: both specs are in the canonical test/playwright/unit/ai/ tree and use Neo setup imports.

Findings: Existing tests cover one transient recovery and happy-path resume, but miss persistent outage, undrained concurrency, permanent HTTP classes, sparse/duplicate provider indexes, semantic source/target identity, contaminated targets, and true no-write dry-run.

📋 Required Actions

To proceed with merging, please address the following:

• Close #16228 unmerged and supersede it from amended authority: keep #16208 open; amend #16227 or create a narrower leaf with the complete retry/fate/CLI/provenance/reconciliation contract; cite the salvage map above; then open a replacement PR whose runner follows #16208’s graph+WAL-or-supported-clone boundary and whose exact-head falsifiers prove bounded pressure plus actionable failure causes.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 22 - the maintenance placement is reasonable, but the old-store authority and unverified resume boundary contradict the governing migration shape.
• [CONTENT_COMPLETENESS]: 48 - detailed prose and JSDoc are present, but the CLI/receipt contract, evidence declaration, and several mechanical claims are incomplete or false.
• [EXECUTION_QUALITY]: 20 - exact-head harnesses show false success, malformed-result acceptance, retry amplification, concurrency escape, and dry-run mutation.
• [PRODUCTIVITY]: 28 - grouped receipts are useful salvage, but the primary clarity and safe-rebuild goals are not achieved.
• [IMPACT]: 96 - this runner determines whether the only Memory Core vector corpus can be rebuilt and safely cut over.
• [COMPLEXITY]: 92 - data provenance, concurrent inference, retry classification, resumability, and destructive migration evidence interact.
• [EFFORT_PROFILE]: Architectural Pillar - this is load-bearing data migration and recovery infrastructure.

The successor should preserve the compact reason histogram, but only after cause, remedy, and target provenance are mechanically true.

[review-budget-managed]

• outcome: terminal-drop-supersede
• ordinary-limit: 2
• activation-issue: 15257
• activation-pr: 15307
• activated-at: 2026-07-16T20:54:31Z