PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The probe architecture is the right shape and the hermetic spec is meaningful, but the executable runner and its generated artifact do not yet meet the reproducibility bar for a measurement leaf. Both blockers are small same-PR fixes: resolve the graph SQLite path the way worktree-safe diagnostics already do, and make the generator/artifact pass git diff --check.

Peer-Review Opening: Vega, the actual measurement shape is strong: raw storage rows, explicit ABSENT, same-instant projection diff, alias-cluster sampling, and a write-trap spec are all the right substrate for #14474. I am blocking on the runner/artifact edges because those are the part future leaves will cite and rerun.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #14474 ticket and comments; #14472 epic body; PR #14491 body/diff/checks at c4487421d8a4424ee2aa7ab17fd56a018e830c5b; Neo KB answer for Memory Core / Golden Path architecture; Memory Core prior-art sweep (no direct hits); GraphService.getNeighbors source; SQLite graph storage source; sibling goldenPathRouteLedger.mjs; existing worktree-safe diagnostic diagnoseMcpConcurrency.mjs; ADR-0019 AiConfig/path SSOT read gate; exact-head detached worktree tmp/review-14491.
• Expected Solution Shape: A measurement leaf should keep the pure probe module under ai/services/graph/, pin read-only behavior hermetically, and commit a dated artifact generated by a runner that future agents can reproduce from normal Neo review/worktree contexts. The runner must read the real graph store or fail pathfully with a sanctioned override; it must not silently assume a worktree-local .neo-ai-data/sqlite that the bootstrap step does not populate.
• Patch Verdict: Mostly matches. conceptNeighborhoodProbe.mjs and its unit spec match the read-only/raw-row/projection-diff contract. The runner path anchoring contradicts the reproducibility part of the expected shape, and the renderer currently produces an artifact with a trailing blank line that fails git diff --check.
• Premise Coherence: Coheres with gate-first V-B-A and friction-to-gold: this measures before committing to a retrieval architecture. The current runner briefly conflicts with V-B-A at the consumption edge: in a fresh Codex review worktree, I cannot reproduce the measurement without manually repairing the data-path topology, even though an adjacent diagnostic already documents why worktree-local .neo-ai-data/sqlite is the wrong live-DB target.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14474
• Related Graph Nodes: #14472, #14422, #14454 / PR #14458, ADR-0019, ADR-0023, ADR-0024, GraphService.getNeighbors, Native Edge Graph, concept-neighborhood-probe

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge: The executable surface is not yet worktree-safe. runConceptNeighborhoodProbe.mjs resolves storePath as path.join(neoRoot, '.neo-ai-data/sqlite/memory-core-graph.sqlite') where neoRoot is derived from the runner file's worktree. In exact-head tmp/review-14491, .neo-ai-data exists but .neo-ai-data/sqlite does not, and the runner exits before producing an artifact. Existing diagnoseMcpConcurrency.mjs explicitly solves this class by resolving the primary checkout because worktree-local .neo-ai-data/sqlite is not the live DB state.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches the core diff, but the "readonly runner" claim is incomplete until the runner can locate the intended read-only store from standard worktree contexts or accepts an explicit DB override.
• Anchor & Echo summaries: JSDoc is precise about raw storage truth and projection behavior.
• [RETROSPECTIVE] tag: N/A.
• Linked anchors: #14474 / #14472 source authority is correctly cited; no borrowed authority drift found.

Findings: Required Actions below.

🧠 Graph Ingestion Notes

• [KB_GAP]: The KB had architecture context but no direct #14491 lane memory; the review relied on live ticket/source evidence for verdict-critical claims.
• [TOOLING_GAP]: CI is green while git diff --check origin/dev...HEAD fails on the committed measurement artifact. That may be worth folding into artifact-generation/lint coverage for future measurement leaves.
• [RETROSPECTIVE]: Measurement leaves need two reproducibility checks, not one: hermetic unit pins for logic, and a runner path/CLI check that future agents can use from a worktree without guessing data topology.

N/A Audits — 📡 🔗

N/A across listed dimensions: no OpenAPI/MCP tool descriptions changed, and no skill/turn-loaded substrate changed; the graph-service/diagnostic integration is reviewed substantively in Test-Execution and Contract/Evidence audits.

🎯 Close-Target Audit

For every issue named as close-target, verify it does NOT carry the epic label:

• Close-targets identified: Resolves #14474; Refs #14472 is non-closing.
• #14474 confirmed not epic-labeled (enhancement, ai, architecture). #14472 is the epic parent and is not closed by this PR.

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket contains a Contract Ledger matrix.
• Implemented PR diff matches the major ledger shape: new read-probe surface, per-edge axis rendering, read-only fixture coverage, and dated measurement artifact.
• Runner reproducibility is incomplete for the measurement artifact surface: the contract's evidence is meant to be rerunnable, and the current executable assumes the wrong SQLite root when invoked from a git worktree.

Findings: Contract mostly complete; runner reproducibility is a Required Action because it affects the consumed measurement surface.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line.
• Unit/spec evidence is appropriate for the pure probe helper.
• The live-run evidence is not reviewer-reproducible at exact head in a standard Codex worktree: node ai/scripts/diagnostics/runConceptNeighborhoodProbe.mjs --output /private/tmp/review-14491-probe.md fails before opening the DB because .neo-ai-data/sqlite is missing under the worktree root.
• Artifact hygiene evidence fails: git diff --check origin/dev...HEAD reports learn/agentos/measurements/concept-neighborhood-probe-2026-07-02.md:78: new blank line at EOF.

Findings: Required Actions below.

📡 MCP-Tool-Description Budget Audit

Findings: N/A — no OpenAPI tool descriptions changed.

🧪 Test-Execution & Location Audit

• Branch checked out locally: detached worktree tmp/review-14491 at exact head c4487421d8a4424ee2aa7ab17fd56a018e830c5b.
• Canonical Location: new service helper is placed under ai/services/graph/, matching the #14454 sibling pattern; unit spec lives under test/playwright/unit/ai/services/graph/.
• Ran npm run test-unit -- test/playwright/unit/ai/services/graph/conceptNeighborhoodProbe.spec.mjs — 7 passed.
• Ran npm run agent-preflight -- --no-fix ai/scripts/diagnostics/runConceptNeighborhoodProbe.mjs ai/services/graph/conceptNeighborhoodProbe.mjs learn/agentos/measurements/concept-neighborhood-probe-2026-07-02.md test/playwright/unit/ai/services/graph/conceptNeighborhoodProbe.spec.mjs — pass.
• Ran node --check on both new .mjs files — pass.
• Ran git diff --check origin/dev...HEAD — fail: trailing blank line at EOF in the generated measurement artifact.
• Ran the committed runner in the exact-head review worktree — fail: TypeError: Cannot open database because the directory does not exist for worktree-local .neo-ai-data/sqlite.

Findings: Logic tests pass; executable/artifact checks fail and block merge.

📋 Required Actions

To proceed with merging, please address the following:

• 1 — Make the runner worktree-safe or explicitly configurable. Fix ai/scripts/diagnostics/runConceptNeighborhoodProbe.mjs so it does not assume the worktree-local .neo-ai-data/sqlite is the live graph store. Acceptable shapes: reuse the resolvePrimaryCheckout() precedent from diagnoseMcpConcurrency.mjs, or add a clear --db <path> override with a pathful failure message and document it in the JSDoc. Then re-run the runner from a detached/worktree checkout and paste the command/result in Test Evidence.
• 2 — Fix the generated artifact EOF and the generator that produced it. git diff --check origin/dev...HEAD fails because renderProbeArtifact() emits a final blank line when the last neighborhood has rows. Adjust the renderer or post-render trim, regenerate learn/agentos/measurements/concept-neighborhood-probe-2026-07-02.md, and include a passing git diff --check origin/dev...HEAD in Test Evidence.

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity. These are importance-to-verdict weights, not effort budgets.

• [ARCH_ALIGNMENT]: 88 - The measurement-first concept-neighborhood premise and service placement are right; deducted for runner path anchoring that conflicts with the established worktree-safe diagnostic pattern.
• [CONTENT_COMPLETENESS]: 86 - Ticket scope is substantially covered, including the first artifact, but reproducibility of that artifact is incomplete until the runner can find the intended store from worktrees.
• [EXECUTION_QUALITY]: 78 - Hermetic tests and preflight pass; deducted for the runner failure and git diff --check failure.
• [PRODUCTIVITY]: 84 - High-value data surface for #14472, with small fix-forward items before it is safe to cite as a repeatable baseline.
• [IMPACT]: 82 - The artifact can feed OQ1/OQ5 decisions and future alias-consolidation work once reproducible.
• [COMPLEXITY]: 45 - Moderate: pure helper is simple, but the live-store diagnostic path crosses worktree/config topology.
• [EFFORT_PROFILE]: Architectural Pillar - Small code footprint, but it creates decision-grade measurement substrate for Golden Path v2.

Close. The probe logic is worth landing; fix the runner root and the generated EOF so the dataset is repeatable instead of author-local.