[review.agent] Wrap review summary and inline comments in collapsible <details> blocks#35226
[review.agent] Wrap review summary and inline comments in collapsible <details> blocks#35226kubaflo wants to merge 1 commit into
Conversation
… <details> blocks Updates the Expert Code Review orchestration to require a consistent collapsible format for both the review summary and individual inline comment bodies, matching the look-and-feel of the AI summary comment. Summary body: - ## 🤖 Expert Code Review heading (real h2, like the AI overview) - <details> with verdict dot + label, short commit SHA link, PR title - Open by default for 🔴/🟡 verdicts; closed for 🟢 LGTM - No timestamp or finding-count tally in the summary line Inline comments: - <details> with severity dot + label + one-line headline + consensus - Always closed by default Reduces conversation-timeline noise on PRs with many MINOR findings without filtering any content - everything is one click away. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35226Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35226" |
There was a problem hiding this comment.
Pull request overview
Updates the Expert Code Review shared workflow instructions to format both the top-level review body and each inline review comment using collapsible <details> blocks, reducing timeline noise while keeping all content available on demand.
Changes:
- Adds required
<details>/<summary>templates for thesubmit_pull_request_reviewsummary body. - Adds required
<details>/<summary>templates for eachcreate_pull_request_review_commentinline comment body. - Introduces verdict/severity “header line” conventions (emoji + label + metadata) for quick scanning.
| **Review summary body** (passed to `submit_pull_request_review`) — must follow this exact structure: | ||
|
|
There was a problem hiding this comment.
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.
| - `{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. |
There was a problem hiding this comment.
The “open by default” guidance only covers 🔴 and 🟡 verdicts, but the template also defines 🟠 “Discussion Needed”. That case is currently unspecified and may end up closed by default; consider stating whether 🟠 should also use <details open> (likely) to avoid hiding important architectural/discussion notes.
| - 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. |
| <summary>{severityDot} <strong>{severityLabel}</strong> — {one-line headline} ({N}/3 reviewers)</summary> | ||
|
|
There was a problem hiding this comment.
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.
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Summary
Wraps the Expert Code Review workflow's output (review summary body + every inline comment body) in collapsible
<details>blocks so reviewers see a one-line header in the timeline by default and only expand what's relevant. Reduces noise on PRs with many MINOR findings without filtering any content — everything is one click away.This addresses the readability problem visible on PRs like #33007, where 4 inline comments + a long summary scroll past every conversation-tab visit.
What changed
.github/workflows/shared/review-shared.mdStep 4 — orchestrator instructions now require:Review summary body (
submit_pull_request_review):{verdictDot}/{verdictLabel}: 🟢 LGTM | 🟡 Changes Suggested | 🔴 Changes Required | 🟠 Discussion Needed<details open>for 🔴/🟡 (so findings are visible without a click);<details>(closed) for 🟢Inline comments (each
create_pull_request_review_comment):Always closed by default — the headline carries enough info to decide whether to expand.
Preview
A live preview is posted on closed PR #35190 — see review #4196553207. The two inline comments under it have additionally been minimized via GraphQL
minimizeComment(classifier: OUTDATED)to demonstrate the optional "Show minimized comment" UX in the Files Changed tab (not part of this PR — would require a follow-up post-step in the workflow).Notes
review-shared.md(after the frontmatter---). Per gh-aw, only frontmatter changes affect the lock-file hash, so nogh aw compileis required.How to test
/reviewon the PR after enabling slash-command from this branch.)## 🤖 Expert Code Reviewheading + a single collapsed<details>line with verdict, commit short SHA, and PR title.<details>thread.A safe target is a small recently-merged PR — re-running on a closed PR is fine and produces a comparable preview comment without affecting open work.