Skip to content

Agentic Review [3]: Add /review/ generic route and rename review namespace - #35220

Merged
yannbf merged 1 commit into
yann/agentic-review-mcp-integrationfrom
review-core/03-route-and-rename
Jun 19, 2026
Merged

yannbf merged 1 commit into
yann/agentic-review-mcp-integrationfrom
review-core/03-route-and-rename

Conversation

@yannbf

@yannbf yannbf commented Jun 18, 2026 •

Copy link
Copy Markdown
Member

What I did

Two changes that promote review from an addon-routed page to a core-routed, core-namespaced feature. PR3 of a 5-PR stack, telescoping on PR2 (#35219) → PR1 (#35218) → #34837.

1. /review/ as a generic core route (M6 / Q6)

  • Deleted resolvePathQueryParam from router/utils.ts and reverted the router's Location to plain queryFromLocation.
  • That helper was a review-specific band-aid: it hardcoded /story/, /docs/, /settings/ prefixes to disambiguate a duplicate path= query param. PR1 (Agentic Review [1]: Standalone review core quality fixes #35218) removed the root cause — customQueryParams no longer carries a stale path, and buildNavigationUrl filters path — so a duplicate path= is unproducible and the guard is dead code.
  • /review/ is now recognized by the same generic viewMode-prefix mechanism as /story/, /docs/, /settings/ (there is no central route registry; parsePath derives viewMode: 'review' for free, and the Layout already treats any non-story/docs viewMode as a full-screen page). The route is owned by the router, not a review special case.

2. Rename the namespace storybook/addon-review → storybook/review (Q7 / Q1)

  • Status type id REVIEW_STATUS_TYPE_ID, the channel events (PUSH_REVIEW/DISPLAY_REVIEW/REQUEST_REVIEW/REVIEW_STALE/DISMISS_REVIEW/REVIEW_DISMISSED), the page/toolbar ids, and the sessionStorage keys all move under storybook/review/*.
  • Updated the two in-core clones (review-mode.ts, ReviewWidget.tsx) and the test literal so the contract stays consistent across every emitter/listener.
  • The package name @storybook/addon-review is intentionally unchanged — it's distinct from the namespace string.

Stack position

Base: review-core/02-delete-compare (PR2, #35219).

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

For a maintainer:

  1. Run a sandbox or cd code && yarn storybook:ui.
  2. Push a review (via the updated @storybook/addon-mcp / a producer emitting the new storybook/review/* namespace — see coordination note). The review summary should load at /review/.
  3. Navigate from the summary into a story and back. Expected: URL params stay clean (no duplicate path=, no canvas blanking), the reviewing sidebar status still appears, and prev/next + dismiss still work.
  4. Confirm the reviewing status filter behaves under the renamed type id storybook/review.
  5. (Optional) Confirm a review pushed with the old storybook/addon-review/* namespace no longer displays — that breakage is expected and intentional.

Documentation

  • Add or update documentation reflecting your changes (addon README channel-contract section already references src/constants.ts, which now carries the new namespace; no literal to update)
  • MIGRATION — N/A, the contract is unreleased (see note)

Checklist for Maintainers

  • ci:normal
  • qa:needed
  • One changelog label: maintenance

🤖 AI note (Claude Code), not written by Yann — coordination required before merge:

Breaking namespace rename (handoff Cost #2). Renaming storybook/addon-review/* → storybook/review/* breaks any producer still emitting the old event names — notably the external @storybook/addon-mcp display-review tool, which is out of this repo. This PR must land in lockstep with the addon-mcp change. Old canary producers (0.0.0-pr-35110-*) will break. Blast radius is low today (review is new/unreleased).

Decisions I made:

  • The in-repo reference producer scripts/display-review.ts carries no namespace literal — it drives the flow through an MCP tools/call (name: 'display-review'), so there was nothing to rename there. The real coordination point is addon-mcp, flagged above. (The PRD anticipated editing the script; it turned out to be a no-op.)
  • I did not add an inert core reviewPageAddon page object yet. There is no route registry to add to, and a render: () => null core page would be dead code until the mount is internalized. The explicit core page registration + moving the persistentRender mount into core is PR4, where it becomes load-bearing. This PR makes the router generic; PR4 makes the render core-owned.
  • I marked BREAKING CHANGE as not applicable (kept maintenance) because the contract is unreleased per the stack README. Flip to BREAKING CHANGE if the team treats the canary contract as published.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Migrated the review integration to the core-owned storybook/review namespace, keeping review event handling and persisted review-mode state consistent.
    • Updated manager routing to stop using the legacy path query resolution and instead derive location path inputs directly, which can change match behavior based on query parameters.
    • Refreshed exported router-related globals to align with the updated routing utilities.
  • Tests
    • Updated review status and router utility tests for the new namespace and removed coverage for the dropped helper.

@yannbf yannbf added maintenance User-facing maintenance tasks ci:normal Run our default set of CI jobs (choose this for most PRs). qa:needed Pull Requests that will need manual QA prior to release. labels Jun 18, 2026
const location = R.useLocation();
const { path: pathFromQuery, singleStory } = queryFromLocation(location);
const path = resolvePathQueryParam(location.search, pathFromQuery);
const { path = '', singleStory } = queryFromLocation(location);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 AI note (Claude Code), not written by Yann: This revert (back to plain queryFromLocation) is safe only because PR1 (#35218) single-sourced customQueryParams so a duplicate path= can no longer be emitted (getCustomQueryParams omits path; buildNavigationUrl filters it). If PR1 is reverted or only partially lands, the deleted resolvePathQueryParam guard must come back. That's the one cross-PR coupling to keep in mind when rebasing the stack.

@yannbf yannbf changed the title Agentic Review: /review/ as a generic route + rename namespace to storybook/review Core: Add /review/ as a generic route and rename review namespace Jun 18, 2026
@yannbf
yannbf force-pushed the review-core/02-delete-compare branch from fa89f91 to 0ef089d Compare June 19, 2026 09:13
@yannbf
yannbf force-pushed the review-core/03-route-and-rename branch from ab9a58e to bf62f59 Compare June 19, 2026 09:13
@yannbf
yannbf force-pushed the review-core/02-delete-compare branch from 0ef089d to 9beb182 Compare June 19, 2026 09:23
@yannbf
yannbf force-pushed the review-core/03-route-and-rename branch from bf62f59 to 4946ed5 Compare June 19, 2026 09:23
@yannbf yannbf changed the title Core: Add /review/ as a generic route and rename review namespace PR3: Add /review/ generic route and rename review namespace Jun 19, 2026
@yannbf
yannbf marked this pull request as ready for review June 19, 2026 13:27
@coderabbitai

coderabbitai Bot commented Jun 19, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8dbbeaf1-65fa-4648-8f63-6c0b1b7d49b9

📥 Commits

Reviewing files that changed from the base of the PR and between 037a9fb and 0ac0627.

📒 Files selected for processing (10)
  • code/addons/review/src/constants.ts
  • code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx
  • code/core/src/manager/components/sidebar/ReviewWidget.tsx
  • code/core/src/manager/components/sidebar/review-mode.ts
  • code/core/src/manager/globals/exports.ts
  • code/core/src/manager/utils/status.test.ts
  • code/core/src/router/router.tsx
  • code/core/src/router/utils.test.ts
  • code/core/src/router/utils.ts
  • code/core/src/shared/status-store/index.ts
💤 Files with no reviewable changes (2)
  • code/core/src/manager/globals/exports.ts
  • code/core/src/router/utils.ts
✅ Files skipped from review due to trivial changes (2)
  • code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx
  • code/core/src/manager/components/sidebar/review-mode.ts
🚧 Files skipped from review as they are similar to previous changes (6)
  • code/addons/review/src/constants.ts
  • code/core/src/manager/utils/status.test.ts
  • code/core/src/manager/components/sidebar/ReviewWidget.tsx
  • code/core/src/router/utils.test.ts
  • code/core/src/shared/status-store/index.ts
  • code/core/src/router/router.tsx

📝 Walkthrough

Walkthrough

Two independent changes: the review addon namespace string is renamed from storybook/addon-review to storybook/review across the addon constants, shared status store, manager sidebar components (channel events, session keys), and a related test. Separately, the resolvePathQueryParam router utility is removed from utils.ts, its call site in router.tsx replaced with a direct queryFromLocation destructure, and it is removed from the globals export map and its tests.

Changes

Review Addon ID Rename

Layer / File(s) Summary
Canonical review ID and status type constants
code/addons/review/src/constants.ts, code/core/src/shared/status-store/index.ts
ADDON_ID updated to 'storybook/review' with an expanded namespace comment; REVIEW_STATUS_TYPE_ID updated to match in the shared status store.
Manager sidebar channel events and session keys
code/core/src/manager/components/sidebar/ReviewWidget.tsx, code/core/src/manager/components/sidebar/review-mode.ts, code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx
REVIEW_ADDON_ID updated to storybook/review in the widget, review-mode session key module, and stories, propagating the rename to derived channel event names and sessionStorage key prefix.
Status test constant import
code/core/src/manager/utils/status.test.ts
Hard-coded 'storybook/addon-review' string in the sidebar status test replaced with an imported REVIEW_STATUS_TYPE_ID constant.

Remove resolvePathQueryParam from Router

Layer / File(s) Summary
Delete utility, update Location, remove from exports and tests
code/core/src/router/utils.ts, code/core/src/router/router.tsx, code/core/src/manager/globals/exports.ts, code/core/src/router/utils.test.ts
resolvePathQueryParam is deleted from utils.ts; router.tsx now reads path directly from queryFromLocation(location); the symbol is removed from the internal router globals export map; the corresponding test block and import are removed.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

  • storybookjs/storybook#35110: Directly overlaps with the review namespace rename and router/status-id plumbing that introduced REVIEW_STATUS_TYPE_ID and resolvePathQueryParam-driven router path handling.
  • storybookjs/storybook#35218: Both PRs touch the review status identifiers in code/core/src/shared/status-store/index.ts—this PR changes REVIEW_STATUS_TYPE_ID's value to 'storybook/review', while the related PR introduces NON_AGGREGATED_STATUS_TYPE_IDS that includes REVIEW_STATUS_TYPE_ID.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

@ghengeveld ghengeveld self-assigned this Jun 19, 2026
@yannbf yannbf changed the title PR3: Add /review/ generic route and rename review namespace Agentic Review [3]: Add /review/ generic route and rename review namespace Jun 19, 2026
@yannbf
yannbf force-pushed the review-core/02-delete-compare branch from 9da9ee9 to f26f113 Compare June 19, 2026 14:50
@yannbf
yannbf force-pushed the review-core/03-route-and-rename branch from 91e8783 to 037a9fb Compare June 19, 2026 14:50
…ce to storybook/review

- M6: delete the review-specific `resolvePathQueryParam` band-aid and revert the
  router's Location to plain `queryFromLocation`. PR1 made a duplicate `path=`
  query param unproducible (customQueryParams no longer carries `path`), so the
  guard is dead. `/review/` is now recognized by the same generic viewMode-prefix
  mechanism as /story//docs//settings/ instead of a hardcoded route allowlist.
- Q7/Q1: rename the review namespace `storybook/addon-review` → `storybook/review`
  for the status type id (REVIEW_STATUS_TYPE_ID), the channel events, the page/
  toolbar ids, and the sessionStorage keys, plus the two core clones
  (review-mode.ts, ReviewWidget.tsx). Core now owns the `storybook/review/*`
  ingest contract.

The package name `@storybook/addon-review` is unchanged (distinct from the
namespace). The external `@storybook/addon-mcp` producer must emit the new
namespace in lockstep — see PR description.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@yannbf
yannbf force-pushed the review-core/02-delete-compare branch from f26f113 to 867db42 Compare June 19, 2026 15:14
@yannbf
yannbf force-pushed the review-core/03-route-and-rename branch from 037a9fb to 0ac0627 Compare June 19, 2026 15:14
@storybook-app-bot

Copy link
Copy Markdown
Contributor

Package Benchmarks

Commit: 0ac0627, ran on 19 June 2026 at 15:38:18 UTC

The following packages have significant changes to their size or dependencies:

storybook

Before After Difference
Dependency count 72 72 0
Self size 21.21 MB 21.26 MB 🚨 +50 KB 🚨
Dependency size 36.42 MB 36.42 MB 0 B
Bundle Size Analyzer Link Link

@storybook/angular-vite

Before After Difference
Dependency count 0 31 🚨 +31 🚨
Self size 0 B 25.14 MB 🚨 +25.14 MB 🚨
Dependency size 0 B 19.84 MB 🚨 +19.84 MB 🚨
Bundle Size Analyzer Link Link

@storybook/cli

Before After Difference
Dependency count 204 204 0
Self size 802 KB 821 KB 🚨 +19 KB 🚨
Dependency size 91.56 MB 91.62 MB 🚨 +59 KB 🚨
Bundle Size Analyzer Link Link

@storybook/codemod

Before After Difference
Dependency count 197 197 0
Self size 32 KB 32 KB 🚨 +36 B 🚨
Dependency size 90.05 MB 90.10 MB 🚨 +50 KB 🚨
Bundle Size Analyzer Link Link

create-storybook

Before After Difference
Dependency count 73 73 0
Self size 1.08 MB 1.09 MB 🚨 +10 KB 🚨
Dependency size 57.64 MB 57.69 MB 🚨 +50 KB 🚨
Bundle Size Analyzer node node

Base automatically changed from review-core/02-delete-compare to yann/agentic-review-mcp-integration June 19, 2026 16:02
@yannbf
yannbf merged commit ff365ba into yann/agentic-review-mcp-integration Jun 19, 2026
142 of 155 checks passed
@yannbf
yannbf deleted the review-core/03-route-and-rename branch June 19, 2026 16:02
@Sidnioulz Sidnioulz added qa:success Pull Requests that were successfully QA'ed by the release team. and removed qa:needed Pull Requests that will need manual QA prior to release. labels Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:normal Run our default set of CI jobs (choose this for most PRs). maintenance User-facing maintenance tasks qa:success Pull Requests that were successfully QA'ed by the release team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants