[skill][NO-JIRA]improve-backpack-review-skill#4330
Conversation
|
Visit https://backpack.github.io/storybook-prs/4330 to see this build running in a browser. |
There was a problem hiding this comment.
Pull request overview
Upgrades the backpack-code-review-checklist Claude skill to v3.x by splitting embedded prompts into separate agent files, adding a learned-patterns workflow/agent, and updating the orchestrator spec for improved runtime via diff prefetch + parallel phases.
Changes:
- Restructures the skill into a multi-file architecture with dedicated prompts under
agents/. - Adds “learn mode” + Agent 6 to mine past PR feedback and surface recurring review patterns.
- Updates
SKILL.mdorchestration flow for performance (prefetch diff, parallel Phase 1/1.5, inline confidence scoring).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| .claude/skills/backpack-code-review-checklist/SKILL.md | Updates skill spec to v3.0.2, adds Phase 1.5 + Agent 6 integration, and documents performance optimizations. |
| .claude/skills/backpack-code-review-checklist/learn-mode.md | Defines the self-improvement workflow for mining merged PR feedback and proposing prompt updates. |
| .claude/skills/backpack-code-review-checklist/domain-knowledge.md | Adds Backpack token/component domain reference material for edge cases. |
| .claude/skills/backpack-code-review-checklist/agents/agent1-constitution.md | New Constitution/API compliance agent prompt file. |
| .claude/skills/backpack-code-review-checklist/agents/agent2-sass.md | New Sass/token compliance agent prompt file. |
| .claude/skills/backpack-code-review-checklist/agents/agent3-a11y.md | New accessibility/testing compliance agent prompt file. |
| .claude/skills/backpack-code-review-checklist/agents/agent4-history.md | New git history/hotspot analysis agent prompt file. |
| .claude/skills/backpack-code-review-checklist/agents/agent5-bugs.md | New bug scanning agent prompt file. |
| .claude/skills/backpack-code-review-checklist/agents/agent6-learned.md | New learned-patterns agent prompt file consuming Phase 1.5 mined comments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## Step 4: Package import investigation | ||
|
|
||
| For each `import X from '../../bpk-component-Y'`: | ||
| 1. Read `packages/bpk-component-Y/index.tsx` to see full export list | ||
| 2. Look for size/variant suffixes (Large, Small, OnDark, V2) | ||
| 3. Verify the imported variant matches context |
There was a problem hiding this comment.
This agent is scoped to SCSS-only diffs, but Step 4 asks it to inspect JS/TS imports (import X from '../../bpk-component-Y') and read index.tsx. With only .scss changes provided, the agent won’t see those imports and can’t reliably execute this step. Consider moving this investigation to the Constitution/API agent, or rephrasing Step 4 to cover SCSS-relevant @use dependencies instead.
| ## Step 4: Package import investigation | |
| For each `import X from '../../bpk-component-Y'`: | |
| 1. Read `packages/bpk-component-Y/index.tsx` to see full export list | |
| 2. Look for size/variant suffixes (Large, Small, OnDark, V2) | |
| 3. Verify the imported variant matches context | |
| ## Step 4: Sass dependency investigation | |
| For each changed SCSS file, inspect its `@use` statements: | |
| 1. Confirm it only uses allowed mixin/token modules (e.g. `../../bpk-mixins/tokens`, `../../bpk-mixins/utils`), not other components’ private SCSS entrypoints. | |
| 2. Check that any newly added `@use` is actually referenced in the changed lines (avoid obviously unused `@use`). | |
| 3. If the change introduces new visual behavior that should be tokenised or mixed-in, verify the appropriate `@use` dependency has been added (or flag its absence). |
| > [ | ||
| > { | ||
| > "rule": "One-sentence rule statement", | ||
| > "domain": "constitution|sass-tokens|a11y-testing|bug-scan|new", |
There was a problem hiding this comment.
In Step B, the domain options include history, but the JSON schema right below omits it (constitution|sass-tokens|a11y-testing|bug-scan|new). This will produce inconsistent outputs from the clustering step. Align the allowed domain values with the earlier list (either add history to the schema or remove it from the earlier list).
| > "domain": "constitution|sass-tokens|a11y-testing|bug-scan|new", | |
| > "domain": "constitution|sass-tokens|a11y-testing|history|bug-scan|new", |
| ```bash | ||
| gh pr view [NUMBER] \ | ||
| --repo Skyscanner/backpack \ | ||
| --json reviews,comments |
There was a problem hiding this comment.
gh pr view --json reviews,comments is unlikely to capture inline review comments left on diffs; it typically returns review summaries (reviews[].body) plus top-level PR comments. Since learn mode is supposed to mine reviewer comments, include the appropriate field for inline comments (e.g. reviewThreads/review comments) and extract thread comment bodies too, otherwise the learned rules will miss most of the useful feedback.
| --json reviews,comments | |
| --json reviews,comments,reviewThreads |
| **Diff pre-fetch (critical for performance):** Fetch the full diff once: | ||
| ```bash | ||
| gh pr diff [NUMBER] --repo Skyscanner/backpack # PR mode | ||
| git diff main...HEAD # local mode | ||
| ``` | ||
| Slice into scoped sections to embed directly in each agent's prompt: | ||
| - **SCSS slice** → Agent 2: `gh pr diff [N] --repo Skyscanner/backpack -- '*.scss'` | ||
| - **TSX/TS slice** → Agents 1, 3, 5: filter lines for `.ts`/`.tsx` files | ||
| - **Full diff** → Agents 4, 6 |
There was a problem hiding this comment.
Phase 1 claims the diff is pre-fetched once for performance, but the slicing instructions suggest additional gh pr diff calls (e.g. the SCSS slice). That undermines the main optimisation. Prefer slicing the already-fetched diff locally (or fetch once per glob and reuse) so the orchestrator doesn’t reintroduce multiple network calls.
| gh pr view [N] --repo Skyscanner/backpack --json reviews,comments | ||
| ``` | ||
| 4. Collect raw text from `reviews[].body` and `comments[].body`. Pass to Agent 6 as-is. |
There was a problem hiding this comment.
In Phase 1.5, gh pr view [N] --json reviews,comments likely misses inline review comments on code (the main source of recurring patterns). To make Agent 6 effective, include the JSON field that contains diff-thread comments (e.g. reviewThreads/review comments) and aggregate those bodies along with reviews[].body.
| gh pr view [N] --repo Skyscanner/backpack --json reviews,comments | |
| ``` | |
| 4. Collect raw text from `reviews[].body` and `comments[].body`. Pass to Agent 6 as-is. | |
| gh pr view [N] --repo Skyscanner/backpack --json reviews,comments,reviewThreads |
- Collect raw text from
reviews[].body,comments[].body, and inline diff-thread
comments (for examplereviewThreads[].comments[].body). Pass to Agent 6 as-is.
IrinaWei
added
patch
|
Visit https://backpack.github.io/storybook-prs/4330 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/4330 to see this build running in a browser. |
… redundant snapshot check - Remove Step 4 (snapshot currency) from agent3-a11y.md — snapshot staleness is caught by CI; agent reading .snap files added 65+ tool calls with no value - Replace permissive "use Read when needed" with diff-first analysis: only read files when the diff raises a specific unanswerable question - Remove snapshot check from SKILL.md Phase 5 for the same reason - Update autopost default: PR mode now posts inline comments by default; use "don't post" / BACKPACK_REVIEW_AUTOPOST=false to suppress - Fix autopost API call: use --input with JSON file instead of --field (gh api --field cannot accept JSON arrays) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Visit https://backpack.github.io/storybook-prs/4330 to see this build running in a browser. |
Summary
Major upgrade of the
backpack-code-review-checklistskill from v2.1.0 to v3.0.2.This PR restructures the skill into a multi-file architecture, adds a self-improving
learned-patterns agent, and applies performance optimisations that reduce review runtime
from ~16 minutes to ~10 minutes on PRs.
Changes
Architecture: Multi-file layout (v3.0.0)
Previously, all 5 agent prompts were embedded inline inside
SKILL.md. They are nowextracted into individual files under
agents/, with two new supporting files:agents/agent1-constitution.mdagents/agent2-sass.mdagents/agent3-a11y.mdagents/agent4-history.mdagents/agent5-bugs.mdagents/agent6-learned.mddomain-knowledge.mdlearn-mode.mdlearn)New: Phase 1.5 + Agent 6 — Learned Patterns (v3.0.0)
A new pre-review phase mines the 5 most recently merged PRs for each changed component,
collects raw reviewer comments, and passes them to the new Agent 6. Agent 6 clusters
historical comments into recurring patterns and cross-references them against the current
diff — surfacing issues that the static checklist would otherwise miss.
New rules learned from 40 recent PRs (v3.0.1)
9 rules were extracted via learn mode and embedded directly into agent prompts:
typeprop, no build artifacts in source, Ark-UI internal-usage restriction, unversioned
src/layout convention$bpk-spacing-icon-text)Performance optimisations (v3.0.2)
gh pr diffconfidence+confidence_explanationin JSON outputTest Plan
/backpack-code-review-checklist <PR URL>— verify review completes and output is well-formed.scsschanges — verify Agent 2 fires and receives SCSS-scoped diff/backpack-code-review-checklist learn— verify learn mode executes vialearn-mode.mdconfidenceandconfidence_explanationRemember to include the following changes:
[Clover-123][BpkButton] Updating the colourREADME.md(If you have created a new component)README.md