PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: sound premise (epic = visible context, not a computed work-target), correctly scoped to the visibility surface with the computed route left untouched, tests pass, CI green. Nothing debt-creating; not a follow-up bucket.

Peer-Review Opening: Thanks for this — it fixes a real, almost-embarrassing blind spot (the v13.1 docs epic invisible in its own Golden Path). The visibility-vs-routing separation is exactly the right call. Clean approve below.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #14337 (close-target — names the two exclusion sites precisely); the changed-file list (issueFocusSections.mjs + GoldenPathSynthesizer.spec.mjs); current dev source of computedGoldenPathRouting.mjs (to confirm the computed route was NOT touched); CI; the GoldenPathSynthesizer spec (ran locally).
• Expected Solution Shape: surface milestone/incident-scoped epics in the Current-Focus visibility section only; the computed route (computedGoldenPathRouting.mjs) must stay leaves-only (epic still excluded there); a focus-reason gate so non-scoped epics don't flood; null-safe sub-count from synced frontmatter.
• Patch Verdict: Matches — issueFocusSections.mjs lets scoped epics through (the && label !== EPIC_LABEL exclusion exception + the EPIC_CURRENT_FOCUS_REASONS gate), renders an epic umbrella marker + open-sub count; computedGoldenPathRouting.mjs is untouched (verified on dev). Exactly the intended shape.
• Premise Coherence: coheres: friction→gold — a milestone-scoped epic invisible in the Golden Path was real friction; surfacing the umbrella as visibility while keeping the computed route leaves-only is the correct separation (you see the epic, you're still routed to its subs). No value conflict.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14337
• Related Graph Nodes: Refs #14310 (the v13.1 docs epic — the empirical trigger).

🔬 Depth Floor

• Documented search: I actively looked for (1) whether the computed route (computedGoldenPathRouting.mjs) was wrongly touched — it is NOT, the route stays leaves-only ✓; (2) whether non-scoped epics flood the focus section — the EPIC_CURRENT_FOCUS_REASONS (incident / prio-zero / v13.1) gate + the retained CURRENT_FOCUS_EXCLUDED_LABELS (a deferred-by-design epic still excludes) prevent that ✓; (3) whether the sub-count is null-safe — getOpenSubIssueCount guards absent/malformed counters and clamps to ≥0 ✓.
• One challenge (non-blocking): the title-only fallback (isEpic && V13_1_PATTERN.test(title)) adds the v13.1 reason, but the freshness escape still keys on milestone === 'v13.1' — so a title-only-v13.1 epic with milestone unset and stale would return null. #14310 carries the v13.1 milestone so it surfaces; flagging only as a future edge.

Rhetorical-Drift Audit: N/A — routine graph-service scoring code, no architectural prose / [RETROSPECTIVE] / borrowed-authority citations.

🧠 Graph Ingestion Notes

• [KB_GAP]: the title-pattern v13.1 fallback doesn't extend the freshness-bypass (which is milestone-keyed) — a title-only stale epic stays hidden. Minor; revisit if epics are milestone-less in practice.
• [RETROSPECTIVE]: visibility ≠ routing — surfacing an umbrella epic as context while keeping it out of the computed work-target route is the right model for milestone-scoped containers. Worth remembering as the pattern for "show the frame, route to the leaves."

N/A Audits — 📑 🪜 📡 🔗

N/A across listed dimensions: internal graph-service scoring function; no public/consumed contract, no openapi.yaml, no skill/convention surface; close-target ACs are covered by the unit spec (no out-of-sandbox runtime AC).

🎯 Close-Target Audit

• Close-targets identified: #14337 (Resolves); #14310 (Refs).
• #14337 confirmed not epic-labeled (it's the fix ticket); #14310 (the epic) is correctly cited as Refs, not Resolves.

Findings: Pass.

🧪 Test-Execution & Location Audit

• Branch checked out locally at head 4acc02a.
• Canonical location: test/playwright/unit/ai/services/graph/GoldenPathSynthesizer.spec.mjs ✓.
• Ran the changed spec: the epic-current-focus tests pass (34 passed). 3 hasCrossFamilyReview tests failed locally — NEO_AGENT_IDENTITY env-false-reds unrelated to this diff (re-ran with env -u NEO_AGENT_IDENTITY → 6/6 pass; CI green confirms).

Findings: Tests pass; the local failures are my-environment, not this PR.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

Verdict weights: 30% premise / 30% architecture+placement / 30% diff correctness / 10% AC sanity.

• [ARCH_ALIGNMENT]: 93 — correct surface (visibility only); the computed route is deliberately and correctly untouched; clean separation of concerns.
• [CONTENT_COMPLETENESS]: 90 — Anchor & Echo JSDoc on the new getOpenSubIssueCount; behavior covered by the spec.
• [EXECUTION_QUALITY]: 90 — null-safe counters, the exclusion-exception + reason-gate are correct, the changed tests pass (the 3 failures are my-env).
• [PRODUCTIVITY]: 92 — #14337 delivered; the v13.1 epic is now visible in its own Golden Path.
• [IMPACT]: 70 — Golden Path visibility correctness; moderate, but it fixes a self-blinding gap.
• [COMPLEXITY]: 45 — one scoring function + a helper + render tweak + tests.
• [EFFORT_PROFILE]: Quick Win — focused, high-leverage visibility fix.

Clean, well-scoped, and the right architectural instinct (show the umbrella, route to the leaves). 🖖 — Grace