Agentic Review: Introduce Review addon#34837
Open
yannbf wants to merge 52 commits into
Open
Conversation
New @storybook/addon-review-changes. An ADE agent pushes a review via the MCP addon; it is broadcast over the Storybook channel; this addon's page receives it (and requests a replay on mount for late/refreshed tabs) and renders the raw agent state — narrative, clusters, changed files, diff hunks, per-story metadata.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces ChangesReview Changes Addon Implementation
CSF Type-level Test Refinement
🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/build/entry-configs.ts (1)
4-4: ⚡ Quick winMaintain alphabetical ordering in imports and exports.
Both the import statement and the
buildEntriesproperty for@storybook/addon-review-changesare placed out of alphabetical order. They should appear afteronboardingand beforethemes(or beforepseudo-statesif sorting@storybook/addon-*packages separately).📝 Suggested reordering
Move the import from line 4 to after line 10 (after
onboardingConfig):// `@ts-ignore` import onboardingConfig from '../../../code/addons/onboarding/build-config'; +// `@ts-ignore` +import reviewChangesConfig from '../../../code/addons/review-changes/build-config'; // `@ts-ignore` import pseudoStatesConfig from '../../../code/addons/pseudo-states/build-config';Move the buildEntries property from line 94 to after line 97 (after
addon-onboarding):'`@storybook/addon-onboarding`': onboardingConfig, + '`@storybook/addon-review-changes`': reviewChangesConfig, 'storybook-addon-pseudo-states': pseudoStatesConfig,Remove the current lines 4 and 94.
Also applies to: 94-94
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/build/entry-configs.ts` at line 4, The import reviewChangesConfig is out of alphabetical order; move the import statement for reviewChangesConfig (currently importing '../../../code/addons/review-changes/build-config') to sit after onboardingConfig (i.e., after the onboarding import) so imports remain alphabetized; likewise find the buildEntries array where the `@storybook/addon-review-changes` entry is added and move that entry (the buildEntries property for `@storybook/addon-review-changes`) to appear after the addon-onboarding entry (or between onboarding and themes / before pseudo-states if you sort addon-* separately) so the buildEntries list remains alphabetically ordered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/addons/review-changes/package.json`:
- Line 3: Update the version of the `@storybook/addon-review-changes` package to
match the rest of the Storybook packages: change the "version" field in
package.json for the review-changes addon from "10.4.0-alpha.17" to
"10.5.0-alpha.0" and also update the corresponding entry in the auto-generated
versions list (look for the review-changes entry in
code/core/src/common/versions.ts) so both locations are consistent; if this
addon intentionally needs a different version, add a brief comment in
package.json (or a README) explaining the separate release requirement instead
of leaving mismatched version strings.
In `@code/addons/review-changes/src/components/ReviewChangesPage.tsx`:
- Around line 253-255: The event handler for EVENTS.APPLY_REVIEW_STATE currently
assumes the payload is a valid ReviewState and directly calls setState(next);
add a runtime guard in the useChannel callback to validate the shape (e.g.,
ensure next is an object, has a clusters array and each cluster has
sampleStoryIds array, etc.) and only call setState(next) when the guard passes;
if validation fails, ignore the payload (optionally log a warning) to prevent
downstream errors in components that access state.clusters or
cluster.sampleStoryIds. Use the existing symbols useChannel,
EVENTS.APPLY_REVIEW_STATE, setState and ReviewState to locate the handler and
implement the guard at that event boundary.
---
Nitpick comments:
In `@scripts/build/entry-configs.ts`:
- Line 4: The import reviewChangesConfig is out of alphabetical order; move the
import statement for reviewChangesConfig (currently importing
'../../../code/addons/review-changes/build-config') to sit after
onboardingConfig (i.e., after the onboarding import) so imports remain
alphabetized; likewise find the buildEntries array where the
`@storybook/addon-review-changes` entry is added and move that entry (the
buildEntries property for `@storybook/addon-review-changes`) to appear after the
addon-onboarding entry (or between onboarding and themes / before pseudo-states
if you sort addon-* separately) so the buildEntries list remains alphabetically
ordered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dd7cacb6-4f1f-41f2-85bd-dcf5d02ce4fa
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
code/.storybook/main.tscode/addons/review-changes/build-config.tscode/addons/review-changes/manager.jscode/addons/review-changes/package.jsoncode/addons/review-changes/project.jsoncode/addons/review-changes/src/components/ReviewChangesPage.tsxcode/addons/review-changes/src/components/ReviewChangesView.stories.tsxcode/addons/review-changes/src/constants.tscode/addons/review-changes/src/index.tscode/addons/review-changes/src/manager.tsxcode/addons/review-changes/src/review-state.tscode/addons/review-changes/tsconfig.jsoncode/core/src/common/versions.tsscripts/build/entry-configs.ts
yannbf
added
the
ci:normal
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 0 B | 53 KB | 🚨 +53 KB 🚨 |
| Dependency size | 0 B | 654 B | 🚨 +654 B 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 74 | 72 | 🎉 -2 🎉 |
| Self size | 20.40 MB | 20.42 MB | 🚨 +25 KB 🚨 |
| Dependency size | 36.65 MB | 36.11 MB | 🎉 -539 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 205 | 203 | 🎉 -2 🎉 |
| Self size | 908 KB | 908 KB | 0 B |
| Dependency size | 89.13 MB | 88.61 MB | 🎉 -513 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 198 | 196 | 🎉 -2 🎉 |
| Self size | 32 KB | 32 KB | 0 B |
| Dependency size | 87.62 MB | 87.10 MB | 🎉 -513 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 75 | 73 | 🎉 -2 🎉 |
| Self size | 1.08 MB | 1.08 MB | 🎉 -66 B 🎉 |
| Dependency size | 57.05 MB | 56.53 MB | 🎉 -513 KB 🎉 |
| Bundle Size Analyzer | node | node |
…nused files - Updated import path for ReviewChangesPage component. - Deleted unused ReviewChangesDetailsScreen, ReviewChangesPage, ReviewChangesScreen, and ReviewCollectionGrid components along with their associated stories. - Streamlined the codebase by removing obsolete files to enhance maintainability.
This reverts commit e3fcd45.
…handling and add stories - Added location search handling to ReviewChangesPage to parse collection and story parameters. - Introduced ReviewChangesPage stories to demonstrate various states and interactions. - Updated CollectionGrid and SummaryScreen components for improved naming consistency and functionality.
Review Changes: Implement review changes screen based on latest designs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/addons/review-changes/src/screens/SummaryScreen.tsx`:
- Line 324: The two SearchInput instances in SummaryScreen.tsx currently rely on
placeholder text for identification; add explicit accessible names by supplying
either an aria-label (e.g., aria-label="Search stories") or connecting a
visible/visually-hidden <label> via id/for to each SearchInput component
(reference the SearchInput elements in SummaryScreen.tsx for both occurrences).
Ensure the labels are unique and descriptive for each input (e.g., "Find
stories" and any other context-specific label) so assistive technologies receive
a programmatic name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 218101ef-b8c6-49d0-a5b9-9b3d9976a917
📒 Files selected for processing (12)
code/addons/review-changes/src/ReviewChangesPage.stories.tsxcode/addons/review-changes/src/ReviewChangesPage.tsxcode/addons/review-changes/src/components/CollectionGrid.stories.tsxcode/addons/review-changes/src/components/CollectionGrid.tsxcode/addons/review-changes/src/index.tscode/addons/review-changes/src/manager.tsxcode/addons/review-changes/src/review-navigation.tscode/addons/review-changes/src/review-state.tscode/addons/review-changes/src/screens/DetailsScreen.stories.tsxcode/addons/review-changes/src/screens/DetailsScreen.tsxcode/addons/review-changes/src/screens/SummaryScreen.stories.tsxcode/addons/review-changes/src/screens/SummaryScreen.tsx
✅ Files skipped from review due to trivial changes (1)
- code/addons/review-changes/src/review-navigation.ts
…w-design-iterations
… navigation logic, and enhance URL handling for review states. Add new ReviewPage component and associated stories for improved review flow.
…tate` to `request-review`. Update related references in the codebase for consistency.
… and TabButton styled components, and update layout for improved tab functionality and accessibility.
…ations UI: More iterations on review page
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
code/addons/review/src/preset.test.ts (1)
70-78: ⚡ Quick winAdd a regression case for unresolved-branch payload sanitization.
Please add a test where incoming
PUSH_REVIEWcontainsbranchNameandresolveBranchreturnsundefined; expected emitted payload should omitbranchName. This locks in the server-resolved contract and prevents spoofed branch display.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/addons/review/src/preset.test.ts` around lines 70 - 78, Add a new test that sends a PUSH_REVIEW containing branchName while mocking resolveBranch to return undefined, call experimental_serverChannel with that mock, fire EVENTS.PUSH_REVIEW with a sampleReview that includes branchName, and assert that the emitted payload for EVENTS.DISPLAY_REVIEW omits the branchName (i.e., emitted equals [{ event: EVENTS.DISPLAY_REVIEW, payload: <sampleReview without branchName> }]); use the same helpers in the file (createMockChannel, experimental_serverChannel, resolveBranch, sampleReview, emitted, EVENTS.DISPLAY_REVIEW) so the test verifies server-side sanitization when resolveBranch returns undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/addons/review/src/manager.tsx`:
- Around line 22-28: When booting outside the review route the code only handles
the 'restore' marker and leaves a lingering 'keep' value; update the block that
checks path.startsWith(REVIEW_CHANGES_URL) to read const val =
sessionStorage.getItem(RESTORE_NAV_SESSION_KEY) and then: if (val === 'restore')
call api.toggleNav(true), and if (val === 'restore' || val === 'keep') call
sessionStorage.removeItem(RESTORE_NAV_SESSION_KEY) so both markers are cleared;
refer to RESTORE_NAV_SESSION_KEY, REVIEW_CHANGES_URL, path, and api.toggleNav in
manager.tsx to locate and change the logic.
In `@code/addons/review/src/node/git-branch.test.ts`:
- Around line 3-52: The tests currently mock node:child_process by hoisting
mockExecFile and calling mockImplementation directly; change to use Vitest
spy-mocking and vi.mocked typing: mock 'node:child_process' with { spy: true }
so execFile is a spy, then obtain the typed mock via vi.mocked(importedExecFile)
(or vi.mocked(require('node:child_process').execFile)) in beforeEach, reset it
there, and move per-test mockImplementation setups into the shared beforeEach or
nested beforeEach blocks for each it; keep the test assertions and the
currentGitBranch import as-is but reference the execFile mock via vi.mocked
instead of the hoisted mockExecFile symbol.
In `@code/addons/review/src/preset.ts`:
- Around line 21-27: The function enrichWithBranch must prevent agent-supplied
branchName from persisting when local resolution fails: first create a sanitized
copy of the incoming ReviewState with any existing branchName removed (e.g.,
clone payload and delete branchName) then call resolveBranch(process.cwd()); if
a branchName is returned, return a new ReviewState that spreads the sanitized
payload and sets branchName, otherwise return the sanitized payload; update the
function enrichWithBranch and reference ReviewState and resolveBranch
accordingly.
---
Nitpick comments:
In `@code/addons/review/src/preset.test.ts`:
- Around line 70-78: Add a new test that sends a PUSH_REVIEW containing
branchName while mocking resolveBranch to return undefined, call
experimental_serverChannel with that mock, fire EVENTS.PUSH_REVIEW with a
sampleReview that includes branchName, and assert that the emitted payload for
EVENTS.DISPLAY_REVIEW omits the branchName (i.e., emitted equals [{ event:
EVENTS.DISPLAY_REVIEW, payload: <sampleReview without branchName> }]); use the
same helpers in the file (createMockChannel, experimental_serverChannel,
resolveBranch, sampleReview, emitted, EVENTS.DISPLAY_REVIEW) so the test
verifies server-side sanitization when resolveBranch returns undefined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a5aed8d5-a254-4ef1-b764-35a2d601ed0d
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (32)
code/.storybook/main.tscode/addons/review/README.mdcode/addons/review/build-config.tscode/addons/review/manager.jscode/addons/review/package.jsoncode/addons/review/preset.jscode/addons/review/project.jsoncode/addons/review/src/ReviewPage.stories.tsxcode/addons/review/src/ReviewPage.tscode/addons/review/src/components/CollectionGrid.stories.tsxcode/addons/review/src/components/CollectionGrid.tsxcode/addons/review/src/constants.tscode/addons/review/src/index.tscode/addons/review/src/manager.tsxcode/addons/review/src/node/git-branch.test.tscode/addons/review/src/node/git-branch.tscode/addons/review/src/preset.test.tscode/addons/review/src/preset.tscode/addons/review/src/prototype/ReviewFlowDemo.stories.tsxcode/addons/review/src/review-grouping.tscode/addons/review/src/review-navigation.tscode/addons/review/src/review-state.tscode/addons/review/src/screens/DetailsScreen.stories.tsxcode/addons/review/src/screens/DetailsScreen.tsxcode/addons/review/src/screens/SummaryScreen.stories.tsxcode/addons/review/src/screens/SummaryScreen.tsxcode/addons/review/tsconfig.jsoncode/addons/review/vitest.config.tscode/core/src/common/versions.tscode/core/src/manager/globals/exports.tscode/package.jsonscripts/build/entry-configs.ts
💤 Files with no reviewable changes (4)
- code/addons/review/tsconfig.json
- code/addons/review/manager.js
- code/addons/review/src/screens/SummaryScreen.stories.tsx
- code/addons/review/src/components/CollectionGrid.stories.tsx
✅ Files skipped from review due to trivial changes (10)
- code/addons/review/preset.js
- code/addons/review/project.json
- code/addons/review/README.md
- code/addons/review/src/constants.ts
- code/core/src/manager/globals/exports.ts
- scripts/build/entry-configs.ts
- code/addons/review/src/index.ts
- code/addons/review/src/review-state.ts
- code/addons/review/vitest.config.ts
- code/core/src/common/versions.ts
ghengeveld
changed the title
ghengeveld
changed the title
Contributor
|
Actionable comments posted: 0 |
- Introduced `ReviewViewState` context for managing review state across components. - Implemented `useReviewViewStore` hook to handle expanded collections and components. - Integrated `StandaloneReviewView` for consistent state management in review screens. - Added `useScrollRestore` hook to manage scroll position restoration during component updates. - Updated `ReviewPage`, `SummaryScreen`, and `CollectionGrid` to utilize the new state management and scroll restoration features.
- Added transform effects to the CollectionGrid for improved hover and focus visibility. - Updated SearchBox component to handle focus and blur events, revealing thumbnail info bars during search. - Integrated state management for search focus in SummaryScreen to enhance user experience while filtering.
The detail-route prefix encodes the detail kind, not the active tab.
Passing `activeTab` into buildReviewChangesDetailHref made the inactive
panel's links drop the collection index, breaking navigation. Hardcode
the prefix to the link's own kind ('collections' / 'components') so both
panels always produce valid hrefs.
Co-authored-by: Cursor <cursoragent@cursor.com>
ghengeveld
added
the
qa:needed
The detail header "Guide Page / Default" renders inside the aria-hidden
route-transition layer and "Guide Page" also appears as collection-cell
labels, so findByText('Guide Page') matched multiple elements. Assert on
the detail-only "2/3" nav button and branch footer instead.
Co-authored-by: Cursor <cursoragent@cursor.com>
- Fix failing Minimal story: drop stale branchName assertion (the branch is only rendered in the details footer, not the summary) - preset: strip agent-supplied branchName before server-side enrichment so an unresolvable local branch can't leave a spoofed branch (+ regression test) - manager: clear both 'restore' and 'keep' nav markers on non-review loads - git-branch.test: use vi.mocked() + beforeEach mock setup - Rename reviewChangesConfig import to reviewConfig - Set addon displayName to "Review"; refresh README to match the shipped UI
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
code/addons/review/src/screens/DetailsScreen.tsx (1)
164-166: 💤 Low valueConsider adding
ariaLabel={false}to the position counter button.Per the codebase convention, buttons with visible text content that serves as the accessible name should explicitly set
ariaLabel={false}to indicate this is intentional rather than an oversight.Suggested fix
- <Button variant="ghost" size="small" readOnly> + <Button variant="ghost" size="small" readOnly ariaLabel={false}> {storyIndex + 1}/{totalStories} </Button>Based on learnings: "set ariaLabel=false when the button's children already serve as the accessible name."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/addons/review/src/screens/DetailsScreen.tsx` around lines 164 - 166, The position counter Button in DetailsScreen (rendering "{storyIndex + 1}/{totalStories}") should explicitly opt out of automatic aria labeling; add the prop ariaLabel={false} to the Button component instance so its visible children are used as the accessible name (update the Button element that currently shows "{storyIndex + 1}/{totalStories}" to include ariaLabel={false}).code/addons/review/src/screens/SummaryScreen.tsx (2)
614-628: 💤 Low value
groupStoriesByComponentis called multiple times per render.When
stateis truthy,groupStoriesByComponent(state.collections)is called in the effect (line 624) and again in the render body (line 636). Consider memoizing the result to avoid redundant computation on every render.Also applies to: 636-636
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/addons/review/src/screens/SummaryScreen.tsx` around lines 614 - 628, The effect and render both call groupStoriesByComponent(state.collections), causing duplicate work; memoize the grouping once (e.g., create const grouped = useMemo(() => groupStoriesByComponent(state?.collections || []), [state?.collections]) and then replace calls in the useEffect and the render with grouped, and update setExpandedComponents to use new Set(grouped.map(g => g.componentId)); this keeps useEffect, setExpandedComponents, groupStoriesByComponent, and state.collections intact while avoiding repeated computation.
224-232: ⚡ Quick winHardcoded color values bypass theming.
CollectionLabel,CollectionRationale, andCollectionCountuse hardcoded hex colors (#2E3338,#5C6570) instead of theme tokens. These won't adapt to dark mode or custom themes.Suggested fix using theme tokens
const CollectionLabel = styled.strong({ minWidth: 0, overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap', fontWeight: 700, lineHeight: '20px', - color: '`#2E3338`', + color: theme.color.defaultText, });Similarly for
CollectionRationaleandCollectionCount, consider usingtheme.textMutedColorinstead of#5C6570.Also applies to: 261-264, 266-276
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/addons/review/src/screens/SummaryScreen.tsx` around lines 224 - 232, CollectionLabel, CollectionRationale, and CollectionCount use hardcoded hex colors which bypass theming and won't adapt to dark/custom themes; update their styled definitions to pull colors from the theme object (e.g., use theme.textColor or theme.textMutedColor) instead of '`#2E3338`' and '`#5C6570`', ensuring the styled callback receives the theme parameter (or uses the theme-aware styled utility already in project) so colors respond to dark mode/custom themes; modify CollectionLabel to use the primary text token (theme.textColor) and CollectionRationale/CollectionCount to use the muted text token (theme.textMutedColor).code/addons/review/src/preset.test.ts (1)
61-152: ⚡ Quick winMove
resolveBranchmock behavior setup intobeforeEach(or nesteddescribebeforeEach).
vi.fn().mockResolvedValue(...)is repeatedly configured insideit(...)blocks; this breaks the repo’s Vitest mock setup convention and makes scenarios harder to scan.As per coding guidelines: "Implement mock behaviors in
beforeEachblocks in Vitest tests" and "Avoid inline mock implementations within test cases in Vitest tests."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/addons/review/src/preset.test.ts` around lines 61 - 152, The test suite repeatedly creates resolveBranch mocks inside individual it blocks; move the resolveBranch vi.fn().mockResolvedValue(...) setup into a beforeEach (or a nested describe+beforeEach) so all tests reuse the mocked behavior and follow Vitest conventions—create the resolveBranch variable at top-level of the test file, assign it in beforeEach, and then pass it into experimental_serverChannel(...) in each test; for cases requiring different return values (e.g., resolved string vs undefined) override the resolveBranch.mockResolvedValue(...) inside the specific test or a nested describe's beforeEach to keep per-scenario setup organized while avoiding inline mock creation inside it blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/addons/review/src/review-navigation.ts`:
- Around line 25-27: The early-return when input is only whitespace currently
returns the original untrimmed value (return value;) which preserves
spaces/URL-encoded `%20`; change that to return the trimmed string instead
(return trimmed;) so whitespace-only IDs normalize to an empty string. Update
the conditional around trimmed in the function handling story IDs (the block
using the variables trimmed and value) to return trimmed rather than value.
- Line 106: Replace direct calls to decodeURIComponent in the detail parsing
with the safe decoder helper (e.g., safeDecodeURIComponent) to avoid runtime
exceptions from malformed URIs: change the expression that sets storyId
(currently using decodeURIComponent(storySegment) passed to
normalizeReviewStoryId) to use the safe decoder and do the same for the other
occurrence at the parse location around line 121; ensure you import or reference
the existing safe decoder function and preserve the conditional fallback to
undefined.
---
Nitpick comments:
In `@code/addons/review/src/preset.test.ts`:
- Around line 61-152: The test suite repeatedly creates resolveBranch mocks
inside individual it blocks; move the resolveBranch
vi.fn().mockResolvedValue(...) setup into a beforeEach (or a nested
describe+beforeEach) so all tests reuse the mocked behavior and follow Vitest
conventions—create the resolveBranch variable at top-level of the test file,
assign it in beforeEach, and then pass it into experimental_serverChannel(...)
in each test; for cases requiring different return values (e.g., resolved string
vs undefined) override the resolveBranch.mockResolvedValue(...) inside the
specific test or a nested describe's beforeEach to keep per-scenario setup
organized while avoiding inline mock creation inside it blocks.
In `@code/addons/review/src/screens/DetailsScreen.tsx`:
- Around line 164-166: The position counter Button in DetailsScreen (rendering
"{storyIndex + 1}/{totalStories}") should explicitly opt out of automatic aria
labeling; add the prop ariaLabel={false} to the Button component instance so its
visible children are used as the accessible name (update the Button element that
currently shows "{storyIndex + 1}/{totalStories}" to include ariaLabel={false}).
In `@code/addons/review/src/screens/SummaryScreen.tsx`:
- Around line 614-628: The effect and render both call
groupStoriesByComponent(state.collections), causing duplicate work; memoize the
grouping once (e.g., create const grouped = useMemo(() =>
groupStoriesByComponent(state?.collections || []), [state?.collections]) and
then replace calls in the useEffect and the render with grouped, and update
setExpandedComponents to use new Set(grouped.map(g => g.componentId)); this
keeps useEffect, setExpandedComponents, groupStoriesByComponent, and
state.collections intact while avoiding repeated computation.
- Around line 224-232: CollectionLabel, CollectionRationale, and CollectionCount
use hardcoded hex colors which bypass theming and won't adapt to dark/custom
themes; update their styled definitions to pull colors from the theme object
(e.g., use theme.textColor or theme.textMutedColor) instead of '`#2E3338`' and
'`#5C6570`', ensuring the styled callback receives the theme parameter (or uses
the theme-aware styled utility already in project) so colors respond to dark
mode/custom themes; modify CollectionLabel to use the primary text token
(theme.textColor) and CollectionRationale/CollectionCount to use the muted text
token (theme.textMutedColor).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 617854aa-461e-46ce-ae01-52b1a0121245
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (20)
code/.storybook/main.tscode/addons/review/README.mdcode/addons/review/package.jsoncode/addons/review/src/ReviewPage.stories.tsxcode/addons/review/src/ReviewPage.tscode/addons/review/src/components/CollectionGrid.tsxcode/addons/review/src/manager.tsxcode/addons/review/src/node/git-branch.test.tscode/addons/review/src/preset.test.tscode/addons/review/src/preset.tscode/addons/review/src/prototype/ReviewFlowDemo.stories.tsxcode/addons/review/src/review-navigation.tscode/addons/review/src/review-state.tscode/addons/review/src/screens/DetailsScreen.stories.tsxcode/addons/review/src/screens/DetailsScreen.tsxcode/addons/review/src/screens/SummaryScreen.stories.tsxcode/addons/review/src/screens/SummaryScreen.tsxcode/core/src/common/versions.tscode/package.jsonscripts/build/entry-configs.ts
✅ Files skipped from review due to trivial changes (2)
- code/core/src/common/versions.ts
- code/addons/review/README.md
🚧 Files skipped from review as they are similar to previous changes (12)
- scripts/build/entry-configs.ts
- code/addons/review/src/review-state.ts
- code/package.json
- code/addons/review/src/screens/DetailsScreen.stories.tsx
- code/.storybook/main.ts
- code/addons/review/src/preset.ts
- code/addons/review/src/manager.tsx
- code/addons/review/package.json
- code/addons/review/src/components/CollectionGrid.tsx
- code/addons/review/src/ReviewPage.stories.tsx
- code/addons/review/src/prototype/ReviewFlowDemo.stories.tsx
- code/addons/review/src/ReviewPage.ts
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new @storybook/addon-review addon that surfaces an agent-pushed “review changes” experience inside the Storybook manager. It adds a dedicated Review page (summary + detail views), server-side channel handling to cache/enrich the review payload, and supporting UI components/stories/tests, plus wires the addon into the repo build and local Storybook setup.
Changes:
- Add new
@storybook/addon-reviewpackage (manager page UI, navigation helpers, server preset, git-branch enrichment, and tests). - Wire the addon into build entry configs, workspace deps, and the local
code/Storybook config. - Add Storybook stories/interactive tests for the new UI components and screens.
Reviewed changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Adds workspace lock entries for the new addon package. |
| scripts/build/entry-configs.ts | Registers @storybook/addon-review build entry for packaging. |
| code/package.json | Adds @storybook/addon-review to code/ workspace dependencies. |
| code/core/src/csf/csf-factories.test.ts | Updates type-level tests (Object → object). |
| code/core/src/common/versions.ts | Updates generated versions map to include the new addon. |
| code/addons/review/vitest.config.ts | Adds Vitest config for the new addon package. |
| code/addons/review/tsconfig.json | Adds TS config for the new addon package. |
| code/addons/review/src/screens/SummaryScreen.tsx | Implements the review summary UI (tabs, search, grouping, expand/collapse). |
| code/addons/review/src/screens/SummaryScreen.stories.tsx | Adds stories + play tests for Summary screen states. |
| code/addons/review/src/screens/DetailsScreen.tsx | Implements the review detail UI with story iframe + prev/next navigation. |
| code/addons/review/src/screens/DetailsScreen.stories.tsx | Adds stories + play tests for Details screen behavior. |
| code/addons/review/src/ReviewPage.ts | Adds manager-page router integration + channel wiring to render summary/details. |
| code/addons/review/src/ReviewPage.stories.tsx | Adds stories + play tests for ReviewPage routing and channel interactions. |
| code/addons/review/src/review-state.ts | Defines the review payload shape used by the UI/preset. |
| code/addons/review/src/review-navigation.ts | Adds URL building + parsing helpers for review summary/detail routes. |
| code/addons/review/src/review-grouping.ts | Adds grouping helper for component-based clustering of stories. |
| code/addons/review/src/prototype/ReviewFlowDemo.stories.tsx | Prototype story to exercise end-to-end navigation in preview. |
| code/addons/review/src/preset.ts | Adds serverChannel preset to enrich/cache/broadcast review payloads. |
| code/addons/review/src/preset.test.ts | Adds unit tests for serverChannel caching/enrichment behavior. |
| code/addons/review/src/node/git-branch.ts | Implements best-effort git-branch resolution. |
| code/addons/review/src/node/git-branch.test.ts | Adds unit tests for git-branch resolution behavior. |
| code/addons/review/src/manager.tsx | Registers the Review page + auto-navigation behavior on review display. |
| code/addons/review/src/index.ts | Exposes addon constants + types. |
| code/addons/review/src/constants.ts | Defines addon IDs, route, session keys, and channel event names. |
| code/addons/review/src/components/CollectionGrid.tsx | Implements the story thumbnail grid with lazy iframe mounting and “Review all”. |
| code/addons/review/src/components/CollectionGrid.stories.tsx | Adds stories + play tests for grid layout behavior. |
| code/addons/review/README.md | Adds package README describing flow + channel contract. |
| code/addons/review/project.json | Adds Nx project definition for the addon package. |
| code/addons/review/preset.js | Adds ESM export shim for the built preset. |
| code/addons/review/package.json | Adds new package manifest/exports metadata for publishing/build. |
| code/addons/review/manager.js | Adds ESM export shim for the built manager entry. |
| code/addons/review/build-config.ts | Defines build entries for browser/node outputs. |
| code/.storybook/main.ts | Adds review addon stories directory + enables the addon in local Storybook. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- review-navigation: trim whitespace-only story ids, use safe URI
decoding at both detail-route sites, and reject trailing path segments
on the collections route
- ReviewPage / ReviewFlowDemo: guard non-Element click targets before
calling closest('a')
- SummaryScreen: use theme tokens for text colors and make the
collection/component header a real <button> with aria-expanded
- CollectionGrid: source the grid min width from GRID_MIN_CELL_WIDTH,
use theme tokens for the focus ring and info-bar colors, make the
info-bar background theme-aware, and drop its top border
Revert the summary header to a non-semantic clickable row (a pointer affordance only) and move the accessible toggle back to the chevron <Button>, which carries the aria-label and now aria-expanded state for assistive tech. This also restores the chevron's column alignment under the toolbar's expand/collapse-all button without the manual margin hack.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I did
This PR introduces a new
@storybook/addon-reviewaddon: an in-Storybook UI for reviewing agent-produced changes.Flow: an ADE agent pushes a review via the MCP addon (
@storybook/addon-mcp); the dev server enriches it with the current git branch, caches it, and broadcasts it over the Storybook channel; the addon's page receives it (and requests a replay on mount so late/refreshed tabs catch up) and renders it.The review experience has two views:
Also includes a responsive CollectionGrid with lazy iframe mounting, server-side branch resolution/caching/replay for late-joining tabs, navigation + grouping utilities, stories, and unit tests.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
yarn task --task sandbox --start-from auto --template react-vite/default-tsDocumentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This pull request has been released as version
0.0.0-pr-34837-sha-4d331ed6. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-34837-sha-4d331ed6 sandboxor in an existing project withnpx storybook@0.0.0-pr-34837-sha-4d331ed6 upgrade.More information
0.0.0-pr-34837-sha-4d331ed6yann/agentic-review-mcp-integration4d331ed61780403460)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=34837Summary by CodeRabbit