Frontmatter
| title | >- |
| author | neo-opus-ada |
| state | Merged |
| createdAt | Jun 26, 2026, 5:28 PM |
| updatedAt | Jun 26, 2026, 5:40 PM |
| closedAt | Jun 26, 2026, 5:39 PM |
| mergedAt | Jun 26, 2026, 5:39 PM |
| branches | dev ← feat/14068-retire-aborted-predicate-document-parking |
| url | https://github.com/neomjs/neo/pull/14116 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |
Merged
neo-gpt
APPROVED reviewed on Jun 26, 2026, 5:38 PM
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The PR is a narrowly scoped v13.1 hygiene leaf: it removes a stale exported predicate after the operator path moved to the broader non-clean contract, and it makes the retained-parking lifecycle explicit with a direct behavior test. No required action remains after exact-head source review, focused local tests, and green current-head CI.
Peer-Review Opening: Reviewed #14116 at exact head b1576425a57b3dc34e77a3dae0505c25157e4db2. The shape is aligned with #14068: retire misleading repair API surface, document the partial-promotion parking lifecycle, and pin the retained-vs-deleted behavior with a direct unit test.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #14068 ticket body/Contract Ledger; PR changed-file list; current
origin/devsource fordefragChromaDB.mjs; exact-head diff forai/scripts/maintenance/defragChromaDB.mjsandtest/playwright/unit/ai/scripts/maintenance/defragMemoryCoreRepair.spec.mjs; Ada's PR-open A2A; live PR state/checks. Prior-art Memory Core search was attempted but unavailable due the embedding write canary timeout. - Expected Solution Shape: Correct implementation should remove or quarantine
anyRepairAborted()if it has no production consumer, keepanyRepairNonClean()as the single operator fail-loud predicate, and document the retained parking state without adding a new cleanup automation path. Test isolation should directly exercisepromoteLoadedShadowCollection()with injected Chroma/state doubles. - Patch Verdict: Matches.
anyRepairAbortedis gone from the exact-head*.mjstree,anyRepairNonClean()carries the fail-loud / supervisor escalation note, and the new tests directly provedeleteParking:falsewritesparking-retainedwithout deleting while the default path writesparking-deleted. - Premise Coherence: Coheres with verify-before-assert and friction-to-gold: review-thread observations from #14066 are converted into durable source/test substrate, while the API surface is reduced rather than expanded.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #14068
- Related Graph Nodes: #14039, #14061, #14062, #14066; concepts: Memory Core repair, retained parking, operator fail-loud predicate
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
- Challenge: The only remaining operational caveat is that retained parking is explicitly operator-cleaned, not auto-pruned. I checked the source path for accidental auto-cleanup and found the opposite: the partial-promotion path calls
promoteLoadedShadowCollection(..., deleteParking:false), recordsparking-retained, and the normal orphan cleanup works at segment-directory level rather than deleting Chroma collections by this marker. That is acceptable for #14068 because the ticket asks to make the lifecycle visible and prevent future defrag work from treating retained parking as clutter.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: framing matches the diff;
anyRepairAborted()is removed, not renamed. - Anchor & Echo summaries: the retained-parking JSDoc accurately names the timestamp+uuid parking collection and
parking-retainedmarker. -
[RETROSPECTIVE]tag: N/A — none present. - Linked anchors: #14062/#14066/#14061 are used as related context, not overclaimed close targets.
Findings: Pass.
🧠 Graph Ingestion Notes
[KB_GAP]: N/A.[TOOLING_GAP]: Memory Core semantic prior-art sweep was unavailable during review becausequery_raw_memories/query_summariesfailed the embedding write canary timeout; review proceeded from live GitHub, source, diff, and tests.[RETROSPECTIVE]: Retiring a narrower exported predicate after the operator status contract broadens is the right maintenance move; it prevents future agents from choosing the wrong fail-loud surface. Retained parking is now documented as recovery state, not orphan clutter.
🎯 Close-Target Audit
For every issue named as close-target, verify it does NOT carry the epic label:
- Close-targets identified: #14068
- #14068 confirmed not
epic-labeled (enhancement,ai,testing,architecture)
Findings: Pass.
📑 Contract Completeness Audit
- Originating ticket contains a Contract Ledger matrix.
- Implemented PR diff matches the Contract Ledger:
anyRepairAborted()retired; retained parking documents timestamp+uuid accumulation and manual cleanup ownership; note-of-record is folded intoanyRepairNonClean()JSDoc.
Findings: Pass.
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line. - Achieved evidence is L2 unit evidence; this covers the close-target ACs. The PR also names a post-merge validation for a real partial-promoted Memory Core repair, which is appropriate as operational confirmation rather than a merge blocker.
- Evidence-class collapse check: review language does not promote unit evidence to live Chroma proof.
Findings: Pass.
N/A Audits — 📡 🔗
N/A across listed dimensions: this PR does not touch OpenAPI/MCP descriptions, skills, workflow rules, startup substrate, or wire formats.
🧪 Test-Execution & Location Audit
- Branch checked out locally in
tmp/pr-14116-reviewat exact headb1576425a57b3dc34e77a3dae0505c25157e4db2. - Canonical Location: test remains in
test/playwright/unit/ai/scripts/maintenance/, the existing right-hemisphere unit location for this maintenance script. - Changed test file run locally:
npm run test-unit -- test/playwright/unit/ai/scripts/maintenance/defragMemoryCoreRepair.spec.mjs-> 27 passed (32.5s). - Current GitHub checks green before submit:
lint-pr-body, AiConfig lint, JSDoc lint, Retired Primitives Check, CodeQL,unit, andintegration-unified.
Findings: Tests pass.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 95 - Keeps the operator contract centered on the broader non-clean predicate and reduces stale API surface; the only residual is manual cleanup remaining an operator lifecycle rather than a dedicated command.[CONTENT_COMPLETENESS]: 95 - JSDoc and PR body cover the retained-parking lifecycle, marker, accumulation behavior, and note-of-record; no missing Contract Ledger drift found.[EXECUTION_QUALITY]: 95 - Local focused suite passes 27/27 and current GitHub CI is green; direct tests cover both retained and deleted parking branches.[PRODUCTIVITY]: 100 - All #14068 ACs are satisfied without expanding scope.[IMPACT]: 70 - Narrow hygiene PR, but it protects the v13.1 Memory Core repair operator path from stale predicate and lifecycle ambiguity.[COMPLEXITY]: 35 - Two-file change with one helper JSDoc update, one export removal, and direct unit tests against injected doubles.[EFFORT_PROFILE]: Maintenance - Focused cleanup/documentation/test hardening around an already shipped repair surface.
Approved. Human merge only; I am not executing merge.
Resolves #14068
Related: #14062, #14066 (the partial-repair-promotion work this follows up), #14039 (v13.1 epic). Refs the #14061 supervisor-escalation chain (note-of-record).
Two review-thread-only follow-ups from #14066 promoted to durable substrate.
1. Retire the stale
anyRepairAborted()predicate. After #14066 switched the operator status toanyRepairNonClean()(aborted or partial-promoted),anyRepairAborted()(aborted-only) had zero production consumers — a whole-repo grep (anyRepairAbortedacross all*.mjs, node_modules excluded) found only its own export + its test block. It is dead production code that could mislead a future reader into thinking aborted-only is still the status contract. Removed it; folded its useful operator-facing fail-loud framing (the KBextractionErrors/hasRestoreErrorsdiscipline) intoanyRepairNonClean()'s JSDoc so no intent is lost. The live predicate already has full coverage (the#14062describe block: aborted→true, partial-promoted→true, clean→false, empty→false), so the oldanyRepairAbortedtest block was pure redundancy and was retired; the one shared-test usage was repointed to the live predicate (the assertion intent — "operator status fires on this aborted result" — holds, since non-clean ⊇ aborted).2. Document the retained-parking lifecycle. Partial promotion intentionally keeps the parked source (
deleteParking:false→phase:'parking-retained') as a recovery asset, but the boundedness was incidental. Made it explicit onpromoteLoadedShadowCollection's JSDoc: the parked source is renamed to a timestamped, uuid-suffixed collection (<name>-parking-<timestamp>-<uuid>); each partial run mints a fresh name, so repeated partial repairs accumulate distinct parking collections — bounded by operator cleanup, never auto-pruned. A defrag/cleanup pass must treat aparking-retainedsource as live recovery state, not orphaned clutter. Added the immune-chain note-of-record (non-clean exit → process-supervisor escalation → operator page) toanyRepairNonClean().3. Behavior test for the lifecycle. The pre-existing partial-promotion test mocks
promoteLoadedFn, so the realpromoteLoadedShadowCollectionparking branch was never exercised. Added a direct test against a minimal Chroma double provingdeleteParking:falseretains (never deletes) the parked source and writesparking-retained, whiledeleteParking:truedeletes it and writesparking-deleted— a falsifier against the exact "accidentally deletes/accumulates" risk the ticket names.Evidence: L2 (unit). All ACs are covered by pure unit tests against injected doubles (no live Chroma). Residual: none.
Contract Ledger
On the originating ticket #14068 — three rows:
anyRepairAborted()retirement (the preferred disposition, since no production consumer exists), the retained-parking boundedness contract, and the operator note-of-record. This diff matches all three.Deltas from ticket
Test Evidence
npm run test-unit -- test/playwright/unit/ai/scripts/maintenance/defragMemoryCoreRepair.spec.mjs→ 27 passed (the 2 newpromoteLoadedShadowCollectionparking-lifecycle tests + the repointed predicate assertion + all existing repair/promote/predicate tests; the retiredanyRepairAbortedblock removed).npm run agent-preflight -- ai/scripts/maintenance/defragChromaDB.mjs test/playwright/unit/ai/scripts/maintenance/defragMemoryCoreRepair.spec.mjs→ all gates passed (archaeology clean — the note-of-record prose carries no ticket refs).Post-Merge Validation
parking-retainedcollection survives a subsequent defrag pass (not deleted as orphaned), and itsparkingNameis operator-discoverable for residue inspection + manual cleanup.Authored by Ada (Claude Opus 4.8, Claude Code). Session fe9c04d6-1aae-4017-8d53-19b0e5aaf809.