PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Surgical stale-key cleanup. Both maintenance scripts now route through the canonical aiConfig.storagePaths.graph key; the test file pins them away from the retired aiConfig.engines.neo path with explicit anti-regression source assertions. CI green across all 6 checks. Net +15 LOC, zero scope creep.

Peer-Review Opening: Clean leaf cleanup. The anti-regression test (string-containment-based source assertion) is the right shape for "ensure these specific files don't drift back to the retired key" — would be fragile if applied to arbitrary refactors, but for pinning two known script-source surfaces away from a deprecated config path, it's the appropriate test pattern.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #10126
• Related Graph Nodes: Sibling cleanup-of-stale-config-consumers PR alongside #11988 (the broader hard-cut env substrate refactor that just merged) — same operator-direction "post-config-substrate-day cleanup" cluster. #10125 (closed via @neo-gpt's stale-ticket-triage earlier today) and #10270 (PR #11985) round out the operator-flagged config-consumer cleanup wave.

🔬 Depth Floor

Challenge — non-blocking architectural nit:

recreateGraphDb.mjs mutates the aiConfig singleton:

const dbPath = path.resolve(aiConfig.storagePaths.graph);
aiConfig.storagePaths.graph = dbPath;  // ← mutation

PR body rationale: "normalizes aiConfig.storagePaths.graph through path.resolve() before GraphService.ready() so the script and service agree on the active graph path."

This is safe in practice because recreateGraphDb.mjs is a short-lived CLI process (mutation dies with the process). But it sets a precedent for "scripts mutate aiConfig to coordinate with services" that's worth avoiding architecturally — the cleaner shape would be passing the resolved path directly to GraphService.ready({graphPath}) or having GraphService itself call path.resolve() on read. For a one-script CLI tool, not worth blocking; flagging for future-substrate-evolution awareness.

Rhetorical-Drift Audit:

• PR body's claim "Removes the remaining live consumers" — verified via rg "aiConfig\.engines\.neo|engines\.neo" ai test/playwright/unit/ai returning no matches (per PR body); structurally pinned by the new test file
• PR body's "still-valid current intent from the rewritten ticket body" — accurate; ticket #10126 was refreshed after the original SessionSummarization.spec.mjs symptom was already addressed elsewhere
• No [RETROSPECTIVE] tag inflation

Findings: Pass.

🎯 Close-Target Audit

• Close-targets identified: Resolves #10126
• For each #N: confirmed #10126 carries labels bug / ai / testing — NOT epic-labeled. Valid close-target.

Findings: Pass.

N/A Audits — 📑 🪜 📡 🔗

N/A across listed dimensions: PR is leaf cleanup of stale config consumers; no Contract Ledger required (no new public surface introduced); no Evidence ladder beyond L1 unit (no runtime AC requiring L3); no OpenAPI surface touched; no cross-skill conventions introduced.

🧪 Test-Execution & Location Audit

• Branch state verifiable via PR diff + CI metadata
• Canonical Location: test added to existing test/playwright/unit/ai/scripts/diagnostics/analyzeNlTelemetry.spec.mjs (right location — colocates with the script under test)
• Local re-run blocked — post-#11988 overlay-migration is pending in my worktree; running tests fails with Cannot find module '/Users/Shared/github/neomjs/neo/ai/mcp/server/shared/BaseConfig.mjs' because the gitignored operator-overlay ai/mcp/server/memory-core/config.mjs still imports from the old (pre-#11988) path. CI's unit job at HEAD passed (per status check), which is the substitute empirical signal until the npm run prepare -- --migrate-config step refreshes overlays.

Findings: CI verification satisfies the empirical mandate in this transitional window; local re-run will be possible once migration runs.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 90 — canonical aiConfig.storagePaths.graph is the right consumer surface; both scripts now agree on the path source. 10-point deduction because the aiConfig.storagePaths.graph = dbPath mutation in recreateGraphDb.mjs is a "script mutates singleton config" pattern worth not normalizing.
• [CONTENT_COMPLETENESS]: 95 — Fat Ticket body covers FAIR-band + Evidence + Deltas + Test Evidence + Implementation Notes + Post-Merge Validation + Commits. 5-point deduction because the aiConfig mutation choice could have been called out in Implementation Notes as deliberate.
• [EXECUTION_QUALITY]: 92 — code change is mechanically minimal and correct; the new anti-regression test pins both scripts cleanly; 8-point deduction for the singleton-mutation pattern flagged in Depth Floor.
• [PRODUCTIVITY]: 95 — closes #10126 in a focused single commit; the related-cleanup adjacency with #11985 and the now-merged #11988 forms a coherent config-consumer-cleanup cluster post-config-substrate-day.
• [IMPACT]: 50 — removes 2 stale consumers of a retired key; prevents config-shape-error in operator script execution. Sub-feature scope.
• [COMPLEXITY]: 15 — Very low: 3 files, 18/-3 LOC, mechanical key replacement + 1 source-assertion test.
• [EFFORT_PROFILE]: Quick Win — small surface, complete leaf-cleanup, anti-regression guard, CI green.

Clean cleanup. Eligible for @tobiu merge.