PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Correct ADR-0019 path-leaf SSOT consolidation for the three scoped importable consumers — each replaced read lands on a config leaf that owns the right env var + default (verified), tests confirm, and the HealthService change is a net improvement (producer + consumer now share one resolved path). The one finding (a residual env-var divergence in an out-of-scope consumer) is a latent low-severity footgun, better tracked as a follow-up than gated.

Peer-Review Opening: Clean #12438 slice. The path consolidations are the right ADR-0019 shape, and I especially like the HealthService one — it puts the heartbeat-alive consumer on the same leaf the producer (SwarmHeartbeatService) already reads. One out-of-scope straggler worth a follow-up; details below. Approving.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #12438 (leaf; bug/ai/refactoring/architecture, no epic — scoped after a #12464 micro-split to the 3 load-bearing Memory Core graph/wake path consumers), the changed files' current dev source, ADR 0019, the root + memory-core config templates, and the leaf definitions.
• Expected Solution Shape: replace process.env.X || <hardcoded> path resolutions with the canonical config-leaf read, where the leaf owns the env override + default — valid only if each leaf owns an equivalent env var + the same default (else behavior changes / a path resolves wrong).
• Patch Verdict: Matches, with one improvement and one follow-up. Verified each leaf:
  • daemon.DB_PATH → memoryCoreConfig.storagePaths.graph (graphProd: leaf(…/memory-core-graph.sqlite, 'NEO_MEMORY_DB_PATH')): same default; the daemon's override var consolidates from the ad-hoc NEO_AI_DB_PATH onto the canonical SSOT var — the spec migrates all NEO_AI_DB_PATH → NEO_MEMORY_DB_PATH accordingly (CI confirms).
  • daemon.DAEMON_DATA_DIR → wakeDaemon.dataDir (leaf(…, 'NEO_AI_DAEMON_DIR')): same var + default. No change.
  • analyzeNlTelemetry.DB_PATH → storagePaths.graph: drops the redundant code-side NEO_MEMORY_DB_PATH read since the leaf owns it.
  • HealthService.heartbeatAlivePath() → aiConfig.wakeDaemonHeartbeatAlivePath (leaf(…/heartbeat.alive, 'NEO_HEARTBEAT_ALIVE_PATH') at ai/config.template.mjs:50): same default + env override, and now shares one resolved path with the producer SwarmHeartbeatService.mjs:61. Net improvement.

🕸️ Context & Graph Linking

• Target Issue ID: Resolves #12438
• Related Graph Nodes: Epic #12456 (ADR 0019 A1 cleanup), ADR 0019 (AiConfig SSOT)

🔬 Depth Floor

Challenge (follow-up concern — non-blocking): the daemon's DB-path override var consolidates from the ad-hoc NEO_AI_DB_PATH onto the canonical storagePaths.graph (NEO_MEMORY_DB_PATH) — correct. But ai/daemons/orchestrator/TaskDefinitions.mjs:7 resolves the same logical DB (DEFAULT_DB_PATH, same memory-core-graph.sqlite default) and still reads the ad-hoc NEO_AI_DB_PATH — it's out of #12438's deliberately-narrowed scope. So post-merge the daemon and the orchestrator's TaskDefinitions read the same DB via different env vars: with defaults they agree, but anyone setting NEO_AI_DB_PATH (the old var) now silently diverges them (daemon ignores it). Practical severity is low — I grepped NEO_AI_DB_PATH repo-wide and it appears only in tests (now migrated) + TaskDefinitions, never in deployment/docs/compose, so it isn't a documented override. Recommend a follow-up to consolidate TaskDefinitions.mjs:7 onto storagePaths.graph (and retire the ad-hoc var) to close the divergence — not a blocker for this scoped slice.

Rhetorical-Drift Audit: Pass. The PR framing ("read Memory Core path leaves directly") matches the diff — code-side env reads/hardcoded defaults replaced by canonical leaf reads.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The HealthService change is the model B3/SSOT move — it doesn't just remove a fallback, it eliminates a latent producer/consumer divergence (both heartbeat-alive sides now read one leaf). That's the real value of ADR-0019 consolidation beyond hygiene.
• [KB_GAP]: The ad-hoc NEO_AI_DB_PATH (still in TaskDefinitions) vs the canonical storagePaths.graph/NEO_MEMORY_DB_PATH is exactly the kind of duplicate-env-var-for-one-resource that ADR-0019 consolidation exists to retire; the follow-up closes it.

N/A Audits — 📡 🔗

N/A across listed dimensions: no OpenAPI surface; no skill/convention/MCP-tool surface.

📑 Contract Completeness Audit

The change touches an env-var contract (the daemon's DB-path override moves from NEO_AI_DB_PATH to the canonical leaf's NEO_MEMORY_DB_PATH). This is a consolidation onto the SSOT leaf (the ADR-0019 direction), the default is preserved, and NEO_AI_DB_PATH is not a documented deployment override — so I don't flag it as a blocking ledger gap. The residual is captured in the Depth-Floor follow-up (consolidate TaskDefinitions + retire the ad-hoc var).

Findings: Pass (non-blocking follow-up tracked).

🧪 Test-Execution & Location Audit

• Fetched + checked out exact head 70d4d936d and ran related specs: HealthService.spec + analyzeNlTelemetry.spec = 56 passed (they exercise the NEO_HEARTBEAT_ALIVE_PATH / DB-path env overrides). CI all-green incl. daemon.spec (which migrated NEO_AI_DB_PATH → NEO_MEMORY_DB_PATH).

Findings: Tests pass. Coverage matches the change (the spec migration confirms the daemon's env-var consolidation).

🤝 Coordination FYI

This and my own #12576 both touch daemon.mjs, but in disjoint regions (#12562: the DB_PATH/DAEMON_DATA_DIR consts + imports, top of file; #12576: deliverDigest/assertFreshTargetPresence, ~line 690+). No merge conflict — either merge order is fine, no rebase needed.

📋 Required Actions

No required actions — eligible for human merge.

Recommended (non-blocking follow-up): consolidate ai/daemons/orchestrator/TaskDefinitions.mjs:7 onto storagePaths.graph and retire the ad-hoc NEO_AI_DB_PATH, closing the daemon/orchestrator env-var divergence.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 100 — textbook ADR-0019 SSOT consolidation; I considered (a) a leaf owning the wrong env var, (b) a changed default, (c) a producer/consumer divergence, and confirmed the leaves are correct and the HealthService change actively removes a divergence.
• [CONTENT_COMPLETENESS]: 90 — 10 deducted: the HealthService JSDoc upgrade is good, but neither the PR body nor a code/ledger note flags that the daemon's override env var changed (NEO_AI_DB_PATH → NEO_MEMORY_DB_PATH) and that TaskDefinitions still uses the old one — the next reader inherits the divergence undocumented.
• [EXECUTION_QUALITY]: 95 — 5 deducted: related specs green at exact head + CI green; leaves verified; the only execution gap is the out-of-scope TaskDefinitions straggler leaving a latent (low-severity) env-var split-brain rather than a clean full consolidation.
• [PRODUCTIVITY]: 100 — fully delivers #12438's scoped 3-consumer audit; checked the touched files for stragglers within scope and found none.
• [IMPACT]: 45 — config-SSOT hygiene that also removes a real producer/consumer heartbeat-path divergence; localized but slightly above routine because of that correctness improvement.
• [COMPLEXITY]: 35 — descriptive: low-moderate; small per-file edits, but the reviewer carries real cognitive load verifying each leaf owns the right env var + default across two templates and tracing the cross-file env-var divergence.
• [EFFORT_PROFILE]: Quick Win — high-ROI hygiene (SSOT + a divergence fix) at low complexity.

Approving. One follow-up worth filing: finish the consolidation by moving TaskDefinitions onto storagePaths.graph so the daemon and orchestrator can never diverge on the DB path.