Skip to content

Collapse specialist findings in review panel verdict - #454

Merged
stbenjam merged 2 commits into
mainfrom
collapse-review-panel-specialists
Jul 26, 2026
Merged

Collapse specialist findings in review panel verdict#454
stbenjam merged 2 commits into
mainfrom
collapse-review-panel-specialists

Conversation

@not-stbenjam

Copy link
Copy Markdown
Collaborator

Summary

  • Wraps each specialist's findings in a <details> block in the review panel verdict template
  • Keeps synthesis, required actions, and optional follow-ups expanded (the actionable parts)
  • Reduces verbosity of the PR comment — readers can expand individual specialists on demand

Ref: #451 (comment)

Test plan

  • Trigger a review panel on a test PR and verify specialist findings render collapsed
  • Verify synthesis and required actions are still visible without expanding

🤖 Generated with Claude Code

Wrap each specialist's findings in a <details> block so the PR
comment is less verbose. The synthesis, required actions, and
optional follow-ups remain expanded since those are actionable.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@not-stbenjam
not-stbenjam requested a review from stbenjam as a code owner July 26, 2026 01:47
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@not-stbenjam, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d693e888-6961-4fef-a753-08a8309a6b43

📥 Commits

Reviewing files that changed from the base of the PR and between 81a95ef and fe60b43.

⛔ Files ignored due to path filters (2)
  • .agents/skills/skillsaw-review-panel/verdict-template.md is excluded by !.agents/**
  • .claude/skills/skillsaw-review-panel/verdict-template.md is excluded by !.claude/**
📒 Files selected for processing (2)
  • .apm/skills/skillsaw-review-panel/verdict-template.md
  • .skillsaw.yaml
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch collapse-review-panel-specialists

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Collapse specialist findings in review panel verdict template

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Wrap each specialist findings section in collapsible / blocks.
• Keep synthesis and action sections expanded for immediate readability.
• Update duplicated verdict templates across agent/APM/Claude integration paths.
Diagram

graph TD
  A["Review panel runner"] --> B["Verdict template(s)"] --> C["GitHub PR comment"]
  B --> D["Specialist Findings (<details>)"] --> C
  B --> E["Panel Synthesis"] --> C
  B --> F["Required Actions / Follow-ups"] --> C
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Single around all Specialist Findings
  • ➕ Even shorter default comment view
  • ➕ Simpler template markup (one wrapper)
  • ➖ Harder to expand only one specialist
  • ➖ Readers must scan a large block after expanding
2. Collapse based on content length (conditional rendering)
  • ➕ Avoids collapsing short/important findings
  • ➕ Adapts to verbosity automatically
  • ➖ Requires logic in the renderer (not just template changes)
  • ➖ More moving parts and edge cases to test

Recommendation: Per-specialist / blocks are the best tradeoff: they reduce noise while preserving quick access to specific reviewers. The alternatives either reduce navigability (single block) or require non-trivial renderer logic (conditional collapsing) for marginal UX gains.

Files changed (4) +74 / -20

Enhancement (3) +72 / -18
verdict-template.mdCollapse each specialist findings section via <details>/<summary> +24/-6

Collapse each specialist findings section via <details>/<summary>

• Replaces bold specialist headings with per-specialist <details> blocks and <summary> titles. This makes specialist findings collapsed by default while leaving downstream sections (synthesis/actions) unaffected.

.agents/skills/skillsaw-review-panel/verdict-template.md

verdict-template.mdMirror specialist findings collapsing in APM verdict template +24/-6

Mirror specialist findings collapsing in APM verdict template

• Applies the same per-specialist <details>/<summary> structure to the APM copy of the verdict template. Keeps placeholders unchanged, only adjusting presentation.

.apm/skills/skillsaw-review-panel/verdict-template.md

verdict-template.mdMirror specialist findings collapsing in Claude verdict template +24/-6

Mirror specialist findings collapsing in Claude verdict template

• Updates the Claude integration’s verdict template to wrap each specialist findings placeholder in a collapsible <details> block. Ensures consistent PR comment formatting across template variants.

.claude/skills/skillsaw-review-panel/verdict-template.md

Documentation (1) +2 / -2
.skillsaw-card.svgUpdate generated report card metrics +2/-2

Update generated report card metrics

• Updates the embedded violation density and content token counts in the SVG report card. Likely reflects regenerated project stats after template changes.

.skillsaw-card.svg

@qodo-code-review

qodo-code-review Bot commented Jul 26, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 136 rules
✅ Skills: 5 invoked
  skillsaw-pr-review
  skillsaw-issue-solver
  skillsaw-pr-followup
  skillsaw-create-plugin
  skillsaw-review-panel
✅ REVIEW.md

Grey Divider


Remediation recommended

1. Unverified card regen ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
This PR updates the committed .skillsaw-card.svg metrics (violation density / content tokens) even
though the functional change is in the review-panel verdict template. Since .skillsaw-card.svg is
a generated artifact from skillsaw badge --large, the SVG should be regenerated and verified to
match the tool output to avoid drifting/misleading published stats.
Code

.skillsaw-card.svg[R19-20]

+    <text x="24" y="76"><tspan class="label">Violation density</tspan><tspan x="180" class="value"><tspan data-testid="density">0.10</tspan> per 10k tokens</tspan></text>
+    <text x="24" y="97"><tspan class="label">Content tokens</tspan><tspan x="180" class="value">~<tspan data-testid="tokens">41,872</tspan></tspan></text>
Relevance

⭐⭐⭐ High

Repo docs already emphasize committing and regenerating the report card; reviewers accept “re-verify
generated/snapshotted artifacts” reminders.

PR-#400
PR-#442

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
.skillsaw-card.svg is documented and implemented as a generated artifact produced by `skillsaw
badge --large`; therefore, changes to its numeric fields should come from regenerating via the CLI
and verifying the diff matches the generator output.

docs/ci.md[138-176]
src/skillsaw/cli/_badge.py[13-15]
src/skillsaw/cli/_badge.py[100-121]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The repo-committed `.skillsaw-card.svg` (a generated report card) changed its displayed metrics in this PR. Because this file is generated by the CLI, it should be updated only by rerunning the generator and committing the exact output, otherwise the README-embedded stats can drift from what `skillsaw badge --large` would currently produce.

## Issue Context
`docs/ci.md` documents `.skillsaw-card.svg` as an artifact produced by `skillsaw badge --large`, and the CLI implementation writes this file when `--large` is enabled.

## Fix Focus Areas
- .skillsaw-card.svg[19-20]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread .skillsaw-card.svg Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6e728251b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread .skillsaw-card.svg Outdated
<text x="24" y="76"><tspan class="label">Violation density</tspan><tspan x="180" class="value"><tspan data-testid="density">0.14</tspan> per 10k tokens</tspan></text>
<text x="24" y="97"><tspan class="label">Content tokens</tspan><tspan x="180" class="value">~<tspan data-testid="tokens">28,082</tspan></tspan></text>
<text x="24" y="76"><tspan class="label">Violation density</tspan><tspan x="180" class="value"><tspan data-testid="density">0.10</tspan> per 10k tokens</tspan></text>
<text x="24" y="97"><tspan class="label">Content tokens</tspan><tspan x="180" class="value">~<tspan data-testid="tokens">41,872</tspan></tspan></text>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Regenerate the report card from a clean checkout

The committed token count jumps from 28,082 to 41,872 even though the only newly lintable source is the .apm verdict template, which grew by just 324 bytes; generated .agents and .claude copies are excluded by .skillsaw.yaml, and token estimation is approximately one token per four body characters. This indicates the card was generated from a polluted environment or additional local plugin content, so the clean make update CI job will rewrite this line (and the derived density above it) and fail its cleanliness check. Regenerate the artifact in the same clean dependency environment used by CI.

AGENTS.md reference: AGENTS.md:L20-L20

Useful? React with 👍 / 👎.

Comment on lines +20 to +21
<details>
<summary><strong>Architecture Reviewer</strong></summary>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add fixture-backed integration coverage for collapsed verdicts

This introduces new review-panel behavior without adding the repository-required integration fixture or test. Add coverage that renders a representative verdict and verifies all six specialist sections have balanced <details> blocks while synthesis, required actions, and optional follow-ups remain outside them; otherwise template-generation drift or malformed nesting can silently break the posted PR comment.

AGENTS.md reference: AGENTS.md:L32-L34

Useful? React with 👍 / 👎.

Worktrees contain duplicate skill files that inflate the token
count when linting locally but are absent in CI, causing
verify-update to fail.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@stbenjam
stbenjam enabled auto-merge (squash) July 26, 2026 01:53
@stbenjam
stbenjam merged commit d29ab13 into main Jul 26, 2026
19 checks passed
@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.54%. Comparing base (81a95ef) to head (fe60b43).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #454   +/-   ##
=======================================
  Coverage   93.54%   93.54%           
=======================================
  Files         145      145           
  Lines       11418    11418           
=======================================
  Hits        10681    10681           
  Misses        737      737           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@stbenjam
stbenjam deleted the collapse-review-panel-specialists branch July 27, 2026 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants