[tsapeta]

Why

The AI review summary comment shows only the coordinator's overall summary. Each reviewer agent's own view of the change is lost, and the comment doesn't say which agents ran or why one didn't.

Change

• Each reviewer pass can return an optional summary (one or two sentences). The parser fails soft like the trace: it flattens the value to one line, cuts it at 280 characters, and drops a malformed value but keeps the findings.
• The engine keeps up to two distinct pass summaries per agent (a chunked agent runs once per chunk) and joins them. No model rewrites them, and the coordinator never sees them.
• buildAgentSummaries makes one row per roster agent. Agents that ran come first, in run order with the cross-cutting pass last. Agents that didn't run follow, marked paths (the paths filter skipped them) or router (the router didn't select them).
• An 8 KB budget keeps the rows. Over the budget, the last rows lose their text but keep their place.
• The budget is per scope, so in the aggregate comment the sum across scopes is not bounded. When the body is over the size limit, renderAggregateMarkdown first leaves out every scope's summaries, from the visible rows and the state, with a one-line note in each scope. Only then does it trim findings, so summaries never cost a finding.
• attachAgentSummaries attaches the list as an engine-owned agentSummaries field after the decision and strips any coordinator-written value, the same way as reviewTrace.
• The full review is stored in the comment's hidden state, so /dismiss re-renders keep the section.
• The comment shows a collapsed "🔎 Reviewer summaries (N)" section under the overall summary (and under each scope's summary in the aggregate comment), with one line per agent. The terminal reporter prints the rows too.
• Summaries are model text that untrusted PR content can steer, so plainModelLine strips formatting tags, escapes every <, collapses whitespace, and puts a zero-width space after every @ that could start a mention. That includes an @ after a backtick, since a backtick can close a code span or have no partner. A local review run by this repo's own reviewer setup found that backtick case in an earlier version, and it's now fixed and tested.
• One known cosmetic limit: a < inside a code span shows as <, the same as the existing short rationale excerpt. The escape stays because a code span could otherwise carry a forged state marker.
• The templates and this repo's own shared.md now ask for the summary field. README, LLP 0002 and LLP 0005 document it.

Test Plan

bun test (803 pass), typecheck, lint, fmt:check and llp:check all pass, and so do ecr ref-check and ecr verify-config on this repo's own setup.

The new tests fail before the change and pass after. They cover:

• the parser summary field and its fail-soft behavior
• the merge, roster rows and byte budget
• the coordinator boundary strip
• the early-return path listing agents skipped by paths
• the collapsed section, sanitizing, the backtick case and the state round trip
• the aggregate comment, and summaries left out before any finding when the body is too large

I also ran ecr review locally on this branch with the repo's own reviewers (consistency, correctness and security on Claude Opus 5.5). All three returned a summary, and the rendered comment shows the section as designed.

[github-actions]

🤖 AI code review

Decision: Ready for human review

Overall PR risk: Medium. This change modifies existing behavior by adding per-agent summaries to the reviewer output schema and rendering paths. The affected surfaces include renderMarkdown, renderAggregateMarkdown, CoordinatorOutput state, and the terminal reporter — the change touches every rendered review comment. Model text now reaches the visible comment through plainModelLine escaping, and the size fallback re-renders without summaries; summaries never reach the decision path, and old state parses safely because the field is optional.

🔗 Review setup:

• This PR changes code the reviewer prompts cite (src, src/tests, src/core/render.ts, templates). Check that the guidance quoting it is still correct.

No findings.

---

This review is advisory — it never blocks a merge and never auto-approves.

[brentvatne]

Review notes from a multi-model review (Codex, Grok and Claude, each finding then re-checked against the code). Fixes are in cd22aec and 58eff53, pushed to this branch.

1. Mentions get past plainModelLine (holds). The rule only broke @ after a non-word character, and GitHub looks for mentions in rendered text after decoding character references. So these stayed live: _@octocat_ (renders as <em>@octocat</em>), @octocat, @octocat, @octocat, and @octocat (the reference spells a login character, not the @). Not tested against live GitHub.

• Fix: escape the & of every character reference, and break each @ that doesn't follow an ASCII letter or digit. Emails stay whole; a && b and R&D are untouched. feat: let an agent mention configured people when a change needs a human review #90 inherits this, since the filter is still the one function.

2. No size check on the single comment (holds, reproduced). The 8 KB budget is on raw JSON, but escaping and the base64 state copy multiply it: 40 <-heavy summaries at 7.7 KB of JSON added 38K characters, and 12 findings went from 45.5K to 83.7K. Over GitHub's limit the post fails and ci.ts replaces the comment with the "failed to run" notice, losing a finished review.

• Fix: renderMarkdown re-renders without summaries over MAX_COMMENT_CHARS, with the same note the aggregate uses.

3. An agent named constructor (or toString, __proto__) crashes the run (holds). buildAgentSummaries runs outside the per-pass try, so a prototype-named agent fails the whole review, even with no summary. A cross-cutting.md agent shares the cross-file pass's bucket and was listed twice.

• Fix: the parts are a Map, and each agent is listed once. Rejecting reserved or prototype ids in the loader is a separate follow-up, since that predates this PR.

4. Low (fixed): parts are now picked in diff order, not pass-completion order, so the same PR gives the same summary; clipping no longer leaves half an emoji; an unknown notRun reason reads "not run" instead of _undefined_.

Not changed: summaries left out for size stay gone on /dismiss and cache reuse (documented in LLP 0005). Model text, summaries included, can still carry links and images, but that predates this PR.

Checks: typecheck, bun test, lint, format and llp:check pass. Every new test fails with its fix reverted.

[brentvatne]

Follow-up: the AI review here is right. My broader @ rule (cd22aec) captured the character before each @, so the second @ of @@login was never matched, and @​@login still notified. Fixed in ce95adc with a lookbehind, (?<![A-Za-z0-9])@, plus a test for @@x and @@@y. The test fails with the old regex.