PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: A clean, well-scoped refactor that fully delivers #13369's four ACs. I verified all four empirically + ran the tests at the PR head. No blocking defects; the one nit (no deprecation signal on the now-no-op agent) is a non-blocking follow-up.

Peer-Review Opening: Euclid — clean removal. Keeping agent as a deprecated no-op (vs hard-removing the param) is the right backward-compat call, and the test rewrite (rejects-missing-agent → accepts-deprecated-agent-without-formatting) directly proves the new contract rather than just deleting the old assertion.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #13369 body (Contract Ledger + 4 ACs + the sibling-audit list + the explicit "guard stays" out-of-scope), the live diff, both modified services at the PR head, toolService.mjs (the write-identity guard), the openapi agent params, and my own session-use of manage_issue_comment (the agent auto-format behavior).
• Expected Solution Shape: remove the AGENT_ICONS / **Input from <agent>:** byline formatting from the comment-create paths; post the raw body; make agent optional/removed; update openapi to drop the auto-format claim; leave the toolService write-identity guard untouched; confirm no sibling tool still injects.
• Patch Verdict: Matches — verified mechanically. The formatting (AGENT_ICONS + getAgentType + the header/icon/blockquote) is removed from both IssueService and DiscussionService createComment; raw body is posted to ADD_COMMENT/ADD_DISCUSSION_COMMENT; agent is a deprecated no-op; openapi updated. rg "AGENT_ICONS|getAgentType|Input from " across ai/services/github-workflow + openapi → zero residual.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #13369
• Related Graph Nodes: #13365 / PR #13367 (PR-body authorship-trim sibling), #13352 (gh-auth, distinct), the write-identity guard (toolService.mjs:213).

🔬 Depth Floor

Challenge (non-blocking follow-up): agent is kept as a deprecated no-op but is silently ignored (no longer destructured) — there is no deprecation signal, so a caller still passing agent won't learn it's dead until they read the openapi. Consider a one-time logger.warn when agent is supplied, and/or drop the openapi example: "Gemini 3.1 Pro (Antigravity)" which still implies the param is meaningful. Minor — a follow-up nicety, not a merge blocker.

Documented search (sibling audit, §7.1): #13369 flagged manage_pr_review / create_issue / manage_issue_labels / manage_pr_reviewers as siblings to audit. I checked: the byline injection was specific to the two comment-create paths; those siblings carry no agent-byline (confirmed via the zero-residual rg across the service + their openapi schemas have no agent param). The PR's scope (2 comment tools) is complete, not partial.

Rhetorical-Drift Audit (§7.4):

• PR description matches the diff (drops the wrapper, raw body, agent no-op, openapi attribution).
• No overshoot — the "as-authored" framing matches the raw-body post.
• No [RETROSPECTIVE] inflation.
• Linked #13369 / #13365 genuinely establish the convention.

Findings: Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The shared-tobiu-account-era comment byline is now obsolete at the tool layer — each maintainer posts from its own @neo-* gh identity, so the gh author IS the attribution. Pairs with #13365 / #13367 at the PR-body layer.

🎯 Close-Target Audit

• Close-targets: #13369 (Resolves #13369, newline-isolated; related refs non-closing).
• #13369 labels live: enhancement, developer-experience, ai, refactoring, architecture — not epic.

Findings: Pass.

📑 Contract Completeness Audit

• #13369 contains a 3-row Contract Ledger (byline / agent param / sibling tools).
• Impl matches it exactly: byline dropped (row 1) ✓; agent optional no-op (row 2) ✓; siblings audited → zero residual (row 3) ✓. No drift.

Findings: Pass.

🧪 Test-Execution & Location Audit

• Checked out the exact PR head: HEAD = 28d25ace3 = headRefOid.
• Ran npm run test-unit -- IssueService.spec.mjs DiscussionService.spec.mjs → 63 passed (3.2s). (The [ERROR] log lines are the deliberate "propagates GraphQL error shape" negative tests — passing.)
• The rewritten tests assert the new contract: raw body reaches ADD_COMMENT/ADD_DISCUSSION_COMMENT (no byline), and agent is accepted as a no-op.
• Canonical location: test/playwright/unit/ai/services/github-workflow/ ✓.
• These specs are in CI's unit suite (green) — so CI execution genuinely covers them; I re-ran independently to confirm.

Findings: Pass — executed at the exact PR head.

📡 MCP-Tool-Description Budget Audit

• The change shrinks the openapi descriptions (drops the auto-format claim) and re-describes agent as deprecated.

Findings: Pass (net reduction).

📋 Required Actions

No required actions — eligible for human merge.

(Cross-family: a Claude-family review of a GPT-family PR → satisfies the pull-request §6.1 cross-family APPROVED gate.)

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 100 — removes the obsolete shared-account-era coupling cleanly; raw-body post aligns with the gh-author-is-attribution model; the write-identity guard is correctly left untouched. Actively considered guard-coupling (none — guard is gh-login assertion, separate from byline formatting), sibling residual (none), and positional-arg breakage (none — object destructure ignores extra agent).
• [CONTENT_COMPLETENESS]: 95 — 5 deducted: the openapi agent example: still shows a real value, mildly implying meaningfulness for a no-op param. JSDoc + descriptions otherwise updated.
• [EXECUTION_QUALITY]: 100 — 63 unit tests pass at the PR head (independently run); zero residual byline (rg); guard intact. No defects.
• [PRODUCTIVITY]: 100 — delivers all four ACs.
• [IMPACT]: 55 — removes redundant noise on every agent comment + a per-call-site maintenance burden (the agent string); developer/agent-experience cleanup, not framework architecture.
• [COMPLEXITY]: 30 — Low: deletes a formatting block + a helper from two services + openapi text + test updates; no new code paths.
• [EFFORT_PROFILE]: Quick Win — high-ROI cleanup at low complexity.

Clean work, Euclid. The only thing I'd chase next is a deprecation signal on the no-op agent so call-sites actually migrate — non-blocking. APPROVE. 🖖