-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[review.agent] Wrap review summary and inline comments in collapsible <details> blocks #35226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -137,7 +137,46 @@ Before posting inline comments, validate **both**: | |||||
|
|
||||||
| **Cap inline comments at 30** (the safe-output limit). If more than 30 findings, post the 30 most severe inline and include the rest in the review summary body. | ||||||
|
|
||||||
| The review body must include: | ||||||
| #### Collapsible formatting (required) | ||||||
|
|
||||||
| Wrap **both** the review summary body and every inline comment body in `<details>` blocks so reviewers see one-line headers in the timeline by default and only expand what's relevant. This keeps low-severity findings out of the way without filtering them. | ||||||
|
|
||||||
| **Review summary body** (passed to `submit_pull_request_review`) — must follow this exact structure: | ||||||
|
|
||||||
| ```html | ||||||
| ## 🤖 Expert Code Review | ||||||
|
|
||||||
| <details> | ||||||
| <summary>{verdictDot} <strong>{verdictLabel}</strong> — <a href="https://github.com/{owner}/{repo}/commit/{commitFull}"><code>{commitSha7}</code></a> · <strong>{prTitle}</strong></summary> | ||||||
|
|
||||||
| [full review body: methodology, overall assessment, findings table, discarded findings, test coverage, summary] | ||||||
|
|
||||||
| </details> | ||||||
| ``` | ||||||
|
|
||||||
| Where: | ||||||
| - `{verdictDot}` / `{verdictLabel}` is one of: `🟢 LGTM` (no 🔴/🟡 findings), `🟡 Changes Suggested` (any 🟡 MODERATE), `🔴 Changes Required` (any 🔴 CRITICAL), `🟠 Discussion Needed` (architectural concerns flagged). | ||||||
| - `{commitSha7}` is the 7-character short SHA of the head commit being reviewed; `{commitFull}` is the full SHA used in the link. | ||||||
| - `{prTitle}` is the PR title at the time of review. | ||||||
| - Use `<details open>` (open by default) when the verdict is 🔴 or 🟡 so reviewers see findings without an extra click. Use `<details>` (closed) for 🟢 LGTM. | ||||||
|
||||||
| - Use `<details open>` (open by default) when the verdict is 🔴 or 🟡 so reviewers see findings without an extra click. Use `<details>` (closed) for 🟢 LGTM. | |
| - Use `<details open>` (open by default) when the verdict is 🔴, 🟡, or 🟠 so reviewers see findings or architectural discussion without an extra click. Use `<details>` (closed) for 🟢 LGTM. |
Copilot
AI
Apr 29, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inline comment summaries hardcode "({N}/3 reviewers)", but Step 2 explicitly allows a 2-reviewer fallback when only 2 sub-agents complete successfully. In that scenario the inline header would be inaccurate; consider making this "({N}/{totalReviewers} reviewers)" or omitting the denominator when fewer than 3 reviewers contributed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The instructions say the review summary body “must follow this exact structure”, but the next bullets require switching between
<details>and<details open>based on the verdict. Consider rewording to explicitly allow that single conditional variation (e.g., “exact structure, with<details>open/closed depending on verdict”) so the orchestrator doesn’t treat<details open>as violating the required template.