Input from Claude Opus 4.7 (Claude Code):

❋ # PR Review Summary

Status: Request Changes

The schema + service-extension shape mirrors mailboxPreview cleanly and the graceful error-handling is right. One Required Action: the explicit reason=mention default filter from Sub 4's AC + epic body Avoided Traps is missing — current code calls raw gh api notifications which returns all-reasons (high-noise per the epic's explicit rejection of that pattern).

---

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #10218 (Sub 4 of Epic #10214)
• Related Graph Nodes: Memory Core's mailboxPreview (the architectural sibling this mirrors), my epic-review on #10214 (commentId IC_kwDODSospM8AAAABAd-oaA)

---

🔬 Depth Floor

Challenge 1 (CRITICAL — RA below): The reason=mention default filter is the Sub 4 AC's explicit named requirement. Epic body §The Fix → Sub 4:

"Default filter: reason=mention only. GitHub fires notifications on any subscription-triggering activity (authoring, commenting, cross-ref, merging) — raw polling is high-noise. The mention filter is authoritative 'someone specifically tagged this agent.'"

And in §Avoided Traps:

"Raw GitHub notifications without filter — rejected. High noise (every merge, comment, cross-ref fires). reason=mention is the authoritative default."

The current diff calls gh api notifications without query parameters → GitHub's /notifications endpoint returns all-reasons including author, comment, subscribed, manual. This is the exact Avoided-Trap the epic explicitly named.

Fix: gh api 'notifications?reason=mention' (or via query param), with a documented opt-in expansion path for assign + review_requested (also explicitly named in epic body).

Challenge 2 (Suggestion — non-blocking): the schema's latest items don't surface reason in a filterable way. If the opt-in expansion lands later, consumers (agents reading the healthcheck) need to know which reason fired. Worth keeping the reason field in the response (already there ✓) and documenting which reasons populate by default. The schema description could explicitly state the default-filter contract.

Challenge 3 (Suggestion — non-blocking): No test added to HealthService.spec.mjs (or equivalent). The fail-soft on gh api error is good defensive coding; a unit test that mocks the execAsync call would lock that contract. Low priority since the path is straightforward.

Rhetorical-Drift Audit: PR description focuses on the mechanical shape; doesn't claim noise-filtering capability. The drift is in the IMPLEMENTATION-vs-AC gap (RA #1) rather than the prose.

Findings: One blocker, two suggestions.

---

🧠 Graph Ingestion Notes

• [KB_GAP]: N/A
• [TOOLING_GAP]: N/A
• [RETROSPECTIVE]: Schema-extension pattern mirrors mailboxPreview cleanly — exact pattern from epic body's "passive signal on every existing boot-time call, no new polling discipline required" framing. This compounds correctly with the existing healthcheck infrastructure.

---

🛂 Provenance Audit

Internal Origin: Epic #10214 brainstorm session ae546a40-... per parent. Clean Chain of Custody.

---

🎯 Close-Target Audit

• Close-targets identified: #10218
• #10218 confirmed not epic-labeled (sub of epic #10214)

Findings: Pass

---

📡 MCP-Tool-Description Budget Audit

• OpenAPI schema additions are structured properties: blocks, not bloated description: literals
• Single-line descriptions on each property
• No internal cross-refs / ticket numbers / Phase sequencing in descriptions
• No architectural narrative

Findings: Pass.

---

🔌 Wire-Format Compatibility Audit

• Healthcheck response schema is BACKWARD-COMPATIBLE — notificationPreview is optional (only present when authenticated)
• Existing consumers reading the healthcheck won't break
• Worth confirming (Suggestion): does mailboxPreview's shape inform downstream consumers' notificationPreview parsing expectations? The two should be structurally similar (count + array of recent items). Quick audit confirms they are — shape parity preserved.

Findings: Pass.

---

🔗 Cross-Skill Integration Audit

• No skill files touched
• No new MCP tool surface (extends existing healthcheck)
• No AGENTS.md / AGENTS_STARTUP.md changes needed (the healthcheck is already documented)

Findings: Pass.

---

📋 Required Actions

To proceed with merging:

• Apply the reason=mention default filter on the gh api notifications call. The epic body explicitly names this as Sub 4's default and lists "raw notifications without filter" as an Avoided Trap. Consider also documenting the opt-in expansion path (assign + review_requested) inline so future maintainers know the filter taxonomy. Implementation could be as light as gh api 'notifications?reason=mention' or via the query parameter shape gh api notifications -f reason=mention.

Suggestions (non-blocking):

• Schema description on notificationPreview could explicitly state the default-filter contract (e.g., "By default, includes only reason=mention notifications. Opt-in expansions for assign + review_requested may be added via configuration.").
• Add a HealthService.spec.mjs test mocking execAsync to lock the contract.

---

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 75 — 25 deducted: missing the explicit reason=mention default filter named in Sub 4's AC + Avoided Traps. Otherwise the schema shape and service-extension pattern align cleanly with the mailboxPreview substrate-reuse precedent.
• [CONTENT_COMPLETENESS]: 90 — 10 deducted: schema doesn't document the default-filter contract; opt-in expansion path not noted in code.
• [EXECUTION_QUALITY]: 90 — 10 deducted: fail-soft on gh api error is correct; no automated test for the new path.
• [PRODUCTIVITY]: 70 — 30 deducted: Sub 4's filter requirement is a load-bearing AC (epic body explicitly rejects raw polling). Without it, the implementation surfaces high-noise content; partial productivity. Once filter lands, this jumps back to 100.
• [IMPACT]: 60 — Substantive: enables passive boot-time GitHub signal on every healthcheck. Compounds with existing infrastructure cleanly. Once filter is correct.
• [COMPLEXITY]: 25 — Low: schema addition + ~20-line service extension, single file pair.
• [EFFORT_PROFILE]: Quick Win — Filter fix is one-line; the architecture is right.

Once RA #1 lands, this is a clean Quick Win merge.

Input from Claude Opus 4.7 (Claude Code):

PR Review Summary — Cycle 2

Status: Approved

The reason=mention filter is applied cleanly via client-side JS (notifications.filter(n => n.reason === 'mention')). Your note on the REST API not supporting ?reason=mention as a native query param is correct — /notifications only accepts participating, all, since, before, etc. Client-side filter is the architecturally-equivalent alternative; functional outcome is identical.

---

Cycle 1 Required Action Resolution

• RA #1 (reason=mention default filter): addressed via client-side JS filter on the notifications array. unreadCount now reflects only mention-reason count; latest slice draws from filtered subset. Behaviorally equivalent to a server-side query-param filter.

---

Depth Floor (Cycle 2)

I actively looked for: (a) regression on the unfiltered-notifications path (none), (b) opt-in expansion path for assign + review_requested (deferrable per epic body's "may be added as second tier"), (c) schema description drift after the filter (the unreadCount description still says "Total number of unread notifications" — technically inaccurate now since it counts only mentions; minor doc cleanup, non-blocking), (d) error-path coverage (existing graceful try/catch survives unchanged).

Rhetorical-Drift Audit: Pass. Cycle 1 RA fully resolved at substrate.

Findings: One minor schema-description drift; non-blocking.

---

Graph Ingestion Notes (Cycle 2 deltas)

• [KB_GAP]: /notifications doesn't accept reason= as a query param. Client-side filter is the canonical workaround per this PR's empirical anchor. Worth a one-line note in github-workflow service-area docs.
• [RETROSPECTIVE]: Cycle 1 → Cycle 2 turnaround: ~7 minutes. Filter discipline named in Sub 4's AC + parent epic's Avoided Traps; my Cycle 1 caught it; Cycle 2 fixed cleanly. Cross-family review value-prop demonstrated again.

---

Required Actions

No required actions — eligible for human merge.

Optional polish (non-blocking):

• Schema description on unreadCount could read "Number of unread mention-reason notifications" to match filtered semantics.
• notificationPreview.description could note the default reason=mention filter contract for future API consumers.

---

Evaluation Metrics (Cycle 2 deltas vs Cycle 1)

• [ARCH_ALIGNMENT]: 75 → 95 — 5 deducted: schema description drift (minor); filter applied at correct layer.
• [CONTENT_COMPLETENESS]: 90 → 90 (unchanged).
• [EXECUTION_QUALITY]: 90 → 95 — 5 deducted: still no automated test; acceptable for gh api subprocess scope.
• [PRODUCTIVITY]: 70 → 100 — Sub 4's filter requirement satisfied.
• [IMPACT]: 60 (unchanged).
• [COMPLEXITY]: 25 (unchanged).
• [EFFORT_PROFILE]: Quick Win.

Cross-family mandate satisfied. Eligible for @tobiu's human-merge call.