PR Review Summary

Status: Request Changes / Drop+Supersede

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Drop+Supersede
• Rationale: The implementation is test-green but the premise is now disproven by the live #13750 thread. Raw updatedAt recency is the wrong lever because the first failure is candidate-pool exclusion, and old meta issues also receive fresh updatedAt when the team discusses them. The replacement shape should union Current Release / Incident Focus candidates into the computed candidate pool and boost by current-focus membership, not by generic recency.

Peer-Review Opening: I am treating this as a premise correction, not a code-quality critique. The code is narrow and locally testable, but the public root-cause evidence moved after the draft opened.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #13750 body and latest comments, #13755 epic body/comments, PR #13793 body, changed-file list, current dev source around GoldenPathSynthesizer.synthesizeGoldenPath, the #13750 live correction comment, KB answer for Golden Path / Dream Pipeline / AiConfig authority, current CI state, and exact-head local checkout at 09c12e6f0d.
• Expected Solution Shape: Correct steering needs to repair the candidate set first: the computed path should rank over a union of semantic candidates plus Current Release / Incident Focus issue IDs, preserve non-actionable filters, and apply any additive boost from the current-focus/release/incident signal rather than generic updatedAt. It must not hardcode a one-off issue or let old meta chatter become fresh work merely because it was commented on today; tests should cover candidate-pool inclusion and non-actionable filtering.
• Patch Verdict: Contradicts the corrected expected shape. The diff adds a bounded recency function and config leaves, but it only runs after semanticIds are already selected from the top-20 Chroma result, so rank-199 incident issues still never enter scoring. The live #13750 correction also shows updatedAt boosts old meta that received same-day discussion.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #13750
• Related Graph Nodes: #13755, golden-path-ranking, current-release-incident-focus, candidate-pool-union

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge: The patch assumes ranking-time recency can fix the steering output, but the current failure happens before ranking for the incident node class: those nodes are absent from semanticIds. Even if a node enters the pool, updatedAt is not an intent signal because old meta can be updated by today’s discussion.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches what the diff substantiates (no overshoot)
• Anchor & Echo summaries: precise codebase terminology, no metaphor or source-code snapshot anchor (ticket/PR/lane/AC/cycle/line number) that overshoots durable intent
• [RETROSPECTIVE] tag: accurately characterizes what shipped (no inflation of architectural significance)
• Linked anchors: cited tickets/PRs actually establish the claimed pattern (no borrowed authority)

Findings: Rhetorical drift flagged. The PR says the mechanism counters the structural-weight bias that surfaces old meta over fresh release work, but the live V-B-A shows raw recency does not distinguish release/incident work from freshly discussed meta and does not fix candidate-pool exclusion.

🧠 Graph Ingestion Notes

• [KB_GAP]: N/A for code comprehension; the KB answer confirmed the existing semantic2 + structural1 authority shape but did not contain the live issue-specific correction.
• [TOOLING_GAP]: Memory Core query_summaries and query_raw_memories both failed during the prior-art sweep with Embedding write canary timed out after 5000ms; review authority fell back to live GitHub, KB, exact-head source, CI, and local tests.
• [RETROSPECTIVE]: This is a useful falsifier: Golden Path steering fixes must treat candidate-pool formation and intent/focus membership as first-class signals. Generic recency is not sufficient when meta threads are actively discussed.

🎯 Close-Target Audit

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

• Close-targets identified: #13750
• For each #N: confirmed not epic-labeled (or flagged as Required Action below)

Findings: Close target is not epic-labeled, but it is overclaimed. #13750 remains a broad PRIO-zero root with unresolved ingestion/ranking/stability proof, and this draft’s own Post-Merge Validation leaves tuning and follow-on slices open. A replacement PR should resolve a narrow leaf, not #13750 directly.

📑 Contract Completeness Audit

• Originating ticket (or parent epic) contains a Contract Ledger matrix
• Implemented PR diff matches the Contract Ledger exactly (no drift)

Findings: Missing ledger if this config-leaf approach were salvaged. The draft adds public config leaves (NEO_GOLDEN_PATH_RECENCY_WEIGHT, NEO_GOLDEN_PATH_RECENCY_HALF_LIFE_DAYS) against #13750, but #13750 is not a narrow contract-bearing leaf with a ledger for these knobs. This is secondary to the Drop+Supersede premise failure.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line (or N/A justified inline)
• Achieved evidence ≥ close-target required evidence, OR residuals are explicitly listed in the PR's ## Residual / Post-Merge Validation section
• If residuals exist: close-target issue body has the residuals annotated as [L<N>-deferred — operator handoff needed]
• Two-ceiling distinction: PR body distinguishes "shipped at L because sandbox ceiling" from "shipped at L because author didn't probe further"
• Evidence-class collapse check: review language does NOT promote L1/L2 evidence to L3/L4 framing without explicit sandbox-ceiling caveat

Findings: Evidence is adequate for the pure recency helper only, not for the close target. The PR body states L2 unit evidence and residual live tuning, but the live proof requirement for #13750 is steering quality: current incident/release work must enter and outrank old meta in the computed recommendation. The patch does not reach that proof target.

N/A Audits — 📡 🔗

N/A across listed dimensions: the PR does not touch MCP OpenAPI surfaces or skill/workflow integration substrate.

🧪 Test-Execution & Location Audit

• Branch checked out locally (e.g., via checkout_pull_request MCP tool or gh pr checkout)
• Canonical Location: New/moved test files placed correctly per unit-test.md (e.g., test/playwright/unit/ai/mcp/server/)
• If a test file changed: Ran the specific test file.
• If code changed: Verified if there are tests, or if new tests are needed.

Findings: Exact-head checkout at 09c12e6f0d completed. The changed test remains in the canonical test/playwright/unit/ai/services/graph/ area. GitHub CI is green. Local npm run test-unit -- test/playwright/unit/ai/services/graph/GoldenPathSynthesizer.spec.mjs initially failed under default parallel workers with import/setup races, then passed single-worker with --workers=1: 22/22.

📋 Required Actions

To proceed with merging, please address the following:

• Do not undraft or merge #13793 as-is. Close it as superseded or replace it with the corrected candidate-pool + current-focus-membership design from the #13750 thread.
• Retarget the replacement to a narrow leaf and remove Resolves #13750; #13750 should remain open until the live steering proof is satisfied.
• If the replacement still adds public ranking/config knobs, backfill a Contract Ledger on the leaf ticket covering env names, defaults, semantics, and rollback behavior.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 45 - The code follows local config/helper style, but the architecture now contradicts the corrected source-of-authority: candidate-pool formation must change before a ranking-time recency term can matter.
• [CONTENT_COMPLETENESS]: 55 - JSDoc and PR body are detailed for the original premise, but the close target overclaims #13750 and the config leaves lack a narrow contract ledger.
• [EXECUTION_QUALITY]: 85 - The pure helper is defensive and the focused test passes single-worker; 15 deducted because the tested mechanism does not cover the actual failure mode and the default parallel local run surfaced setup fragility.
• [PRODUCTIVITY]: 20 - The PR produces reusable code, but it should not advance the root lane now that the live V-B-A identifies generic recency as the wrong lever.
• [IMPACT]: 90 - Golden Path steering is the current swarm-routing control plane, so a wrong or right change here has high operational impact.
• [COMPLEXITY]: 55 - The diff is small, but it sits inside a high-blast ranking path with config semantics and live steering consequences.
• [EFFORT_PROFILE]: Architectural Pillar - The touched mechanism influences how the swarm selects work; even a small ranking term is pillar-level because it can redirect the team.

This should supersede cleanly: the replacement design already has a precise shape in #13750, and this draft usefully proved that recency alone is the wrong abstraction.