Agentic Review: Baseline comparison

code/addons/review/package.json

@@ -42,6 +42,7 @@
     "!src/**/*"
   ],
   "devDependencies": {
+    "http-proxy-middleware": "^4.0.0",
     "react": "^18.2.0",
     "react-dom": "^18.2.0",
     "typescript": "^5.9.3"

code/addons/review/src/ReviewPage.stories.tsx

@@ -1,12 +1,18 @@
-import { expect, fn, userEvent, within } from 'storybook/test';
+import { expect, fn, userEvent, waitFor, within } from 'storybook/test';
 
-import { ManagerContext, type API, type State } from 'storybook/manager-api';
+import {
+  ManagerContext,
+  type API,
+  type State,
+  internal_fullStatusStore,
+} from 'storybook/manager-api';
 import { MemoryRouter } from 'storybook/internal/router';
 
 import preview from '../../../.storybook/preview.tsx';
 import { EVENTS, RESTORE_NAV_SESSION_KEY } from './constants.ts';
 import type { ReviewState } from './review-state.ts';
 import { ReviewPage } from './ReviewPage.ts';
+import { sessionStore } from './session-store.ts';
 
 type EventListener = (payload?: unknown) => void;
 
@@ -64,6 +70,10 @@ const reviewState: ReviewState = {
   title: 'Manager settings polish',
   description: 'Updated settings views and spacing.',
   branchName: 'feat/review-page',
+  // Drives the baseline-index fetch (keyed on createdAt) for "New" detection.
+  // Anchored to "now" so the "Created … ago" label stays small and stable
+  // instead of computing minutes against a fixed past timestamp.
+  createdAt: Date.now(),
   collections: [
     {
       title: 'Settings',
@@ -77,6 +87,39 @@ const reviewState: ReviewState = {
   ],
 };
 
+// Baseline index served via the dev-server proxy. Intentionally omits
+// `manager-settings-checklist--default` so it reads as a newly added story,
+// while guidepage/aboutscreen already exist in the baseline.
+const baselineIndex = {
+  v: 5,
+  entries: {
+    'manager-settings-guidepage--default': {
+      type: 'story',
+      id: 'manager-settings-guidepage--default',
+      title: 'Manager/Settings/Guide Page',
+      name: 'Default',
+    },
+    'manager-settings-aboutscreen--default': {
+      type: 'story',
+      id: 'manager-settings-aboutscreen--default',
+      title: 'Manager/Settings/About Screen',
+      name: 'Default',
+    },
+  },
+};
+
+const originalFetch = globalThis.fetch;
+const fetchMock = fn(async (input: RequestInfo | URL): Promise<Response> => {
+  const url = typeof input === 'string' ? input : input.toString();
+  if (url.includes('/__review-baseline/index.json')) {
+    return new Response(JSON.stringify(baselineIndex), {
+      status: 200,
+      headers: { 'Content-Type': 'application/json' },
+    });
+  }
+  return new Response(null, { status: 404 });
+});
+
 const applyReviewState = () => {
   expect(onMock).toHaveBeenCalledWith(EVENTS.DISPLAY_REVIEW, expect.any(Function));
   emitMock(EVENTS.DISPLAY_REVIEW, reviewState);
@@ -105,7 +148,15 @@ const meta = preview.meta({
     offMock.mockReset();
     emitMock.mockReset();
     toggleNavMock.mockReset();
-    sessionStorage.removeItem(RESTORE_NAV_SESSION_KEY);
+    fetchMock.mockClear();
+    sessionStore.remove(RESTORE_NAV_SESSION_KEY);
+    // Reset change-detection statuses so a story marking one "new" doesn't leak
+    // into stories that assert the absence of the badge.
+    internal_fullStatusStore.unset();
+    globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch;
+    return () => {
+      globalThis.fetch = originalFetch;
+    };
   },
 });
 
@@ -145,6 +196,84 @@ export const Details = meta.story({
     applyReviewState();
 
     await expect(await canvas.findByRole('button', { name: '2/3' })).toBeInTheDocument();
-    await expect(await canvas.findByText('Latest on feat/review-page')).toBeInTheDocument();
+    await expect(
+      await canvas.findByTitle('Latest manager-settings-guidepage--default')
+    ).toBeInTheDocument();
+    // guidepage exists in the baseline index, so it must not be flagged "New".
+    await expect(canvas.queryByText('New')).not.toBeInTheDocument();
+  },
+});
+
+export const DetailsFocusesTitle = meta.story({
+  parameters: {
+    routerInitialEntries: ['/?path=/review/collections/0/manager-settings-guidepage--default'],
+  },
+  play: async ({ canvasElement }) => {
+    const canvas = within(canvasElement);
+    await expect(emitMock).toHaveBeenCalledWith(EVENTS.REQUEST_REVIEW);
+
+    applyReviewState();
+
+    // Opening a detail moves focus to its heading so users are oriented by what
+    // they opened. The summary's h1 is inert/aria-hidden, so the detail h2 is
+    // the only heading the accessibility tree exposes.
+    const heading = await canvas.findByRole('heading', { level: 2 });
+    await waitFor(() => expect(heading).toHaveFocus());
+
+    // The summary stays mounted behind the detail screen…
+    const summaryHeading = canvas.getByText('Manager settings polish');
+    expect(summaryHeading).toBeInTheDocument();
+    // …but is inert and hidden from assistive tech so focus can't reach it.
+    const inertWrapper = summaryHeading.closest('[inert]');
+    expect(inertWrapper).not.toBeNull();
+    expect(inertWrapper).toHaveAttribute('aria-hidden', 'true');
+  },
+});
+
+export const DetailsNewStory = meta.story({
+  parameters: {
+    routerInitialEntries: ['/?path=/review/collections/0/manager-settings-checklist--default'],
+  },
+  play: async ({ canvasElement }) => {
+    const canvas = within(canvasElement);
+    await expect(emitMock).toHaveBeenCalledWith(EVENTS.REQUEST_REVIEW);
+
+    applyReviewState();
+
+    await expect(
+      await canvas.findByTitle('Latest manager-settings-checklist--default')
+    ).toBeInTheDocument();
+    // checklist is absent from the baseline index, so it is newly added.
+    await expect(await canvas.findByText('New')).toBeInTheDocument();
+  },
+});
+
+export const DetailsChangeDetectedNew = meta.story({
+  parameters: {
+    routerInitialEntries: ['/?path=/review/collections/0/manager-settings-guidepage--default'],
+  },
+  play: async ({ canvasElement }) => {
+    const canvas = within(canvasElement);
+    // guidepage exists in the baseline index, so the baseline check alone would
+    // not flag it. Mark it new via the change-detection status store instead.
+    internal_fullStatusStore.set([
+      {
+        storyId: 'manager-settings-guidepage--default',
+        typeId: 'storybook/change-detection',
+        value: 'status-value:new',
+        title: 'Change Detection',
+        description: '',
+      },
+    ]);
+
+    await expect(emitMock).toHaveBeenCalledWith(EVENTS.REQUEST_REVIEW);
+
+    applyReviewState();
+
+    await expect(
+      await canvas.findByTitle('Latest manager-settings-guidepage--default')
+    ).toBeInTheDocument();
+    // Flagged "New" by change-detection despite existing in the baseline.
+    await expect(await canvas.findByText('New')).toBeInTheDocument();
   },
 });

code/addons/review/src/ReviewPage.ts

@@ -1,7 +1,13 @@
 import React, { type FC, type ReactNode, useEffect, useMemo, useRef, useState } from 'react';
 
-import { useChannel, useStorybookApi, useStorybookState } from 'storybook/manager-api';
+import {
+  experimental_useStatusStore,
+  useChannel,
+  useStorybookApi,
+  useStorybookState,
+} from 'storybook/manager-api';
 import { Location, type RenderData, useNavigate } from 'storybook/internal/router';
+import type { StatusesByStoryIdAndTypeId } from 'storybook/internal/types';
 
 import type { StoryInfo } from './components/CollectionGrid.tsx';
 import { EVENTS, RESTORE_NAV_SESSION_KEY, REVIEW_CHANGES_URL } from './constants.ts';
@@ -15,6 +21,7 @@ import {
   type ReviewTab,
 } from './review-navigation.ts';
 import type { ReviewState } from './review-state.ts';
+import { sessionStore } from './session-store.ts';
 import { DetailsScreen } from './screens/DetailsScreen.tsx';
 import { SummaryScreen } from './screens/SummaryScreen.tsx';
 
@@ -27,8 +34,19 @@ export const ReviewPage: FC = () =>
       search: location.search ?? '',
     })) as unknown as ReactNode);
 
+// Served through the dev-server proxy declared in `preset.ts`, pointing at the
+// baseline Storybook. Used to detect stories that don't exist in the baseline.
+const BASELINE_INDEX_URL = '/__review-baseline/index.json';
+
+// Change-detection status value marking a story as newly added.
+const NEW_STATUS_VALUE = 'status-value:new';
+
 const ReviewPageContent: FC<{ search: string }> = ({ search }) => {
   const [state, setState] = useState<ReviewState | null>(null);
+  // Story IDs present in the baseline Storybook's index. `null` means the
+  // baseline is unresolved or unavailable (no fetch yet, network/proxy error,
+  // or an unparseable index) — in which case no "New" badge is shown.
+  const [baselineStoryIds, setBaselineStoryIds] = useState<Set<string> | null>(null);
 
   const api = useStorybookApi();
   const { index } = useStorybookState();
@@ -52,6 +70,36 @@ const ReviewPageContent: FC<{ search: string }> = ({ search }) => {
     emit(EVENTS.REQUEST_REVIEW);
   }, [emit]);
 
+  // Resolve which stories exist in the baseline so newly added stories can be
+  // flagged. Keyed on `createdAt`: a freshly pushed review re-fetches the
+  // baseline index. Any non-OK outcome leaves the set `null` (no badge).
+  const reviewCreatedAt = state?.createdAt;
+  useEffect(() => {
+    if (reviewCreatedAt === undefined) {
+      return undefined;
+    }
+    let cancelled = false;
+    setBaselineStoryIds(null);
+    fetch(BASELINE_INDEX_URL)
+      .then((response) => (response.ok ? response.json() : null))
+      .then((data: { entries?: Record<string, unknown>; stories?: Record<string, unknown> }) => {
+        if (cancelled || !data) {
+          return;
+        }
+        const entries = data.entries ?? data.stories;
+        if (!entries || typeof entries !== 'object') {
+          return;
+        }
+        setBaselineStoryIds(new Set(Object.keys(entries)));
+      })
+      .catch(() => {
+        // Baseline unavailable — leave `null` so no "New" badge is shown.
+      });
+    return () => {
+      cancelled = true;
+    };
+  }, [reviewCreatedAt]);
+
   const activeTab = parseReviewChangesActiveTab(search);
   const detailLocation = parseReviewChangesDetailLocation(search);
 
@@ -75,16 +123,16 @@ const ReviewPageContent: FC<{ search: string }> = ({ search }) => {
   // router handles as an SPA transition). A user who keeps the sidebar
   // collapsed by choice ('keep') is left untouched.
   useEffect(() => {
-    if (sessionStorage.getItem(RESTORE_NAV_SESSION_KEY) === null) {
-      sessionStorage.setItem(RESTORE_NAV_SESSION_KEY, api.getIsNavShown() ? 'restore' : 'keep');
+    if (sessionStore.read(RESTORE_NAV_SESSION_KEY) === null) {
+      sessionStore.write(RESTORE_NAV_SESSION_KEY, api.getIsNavShown() ? 'restore' : 'keep');
     }
     api.toggleNav(false);
 
     return () => {
-      if (sessionStorage.getItem(RESTORE_NAV_SESSION_KEY) === 'restore') {
+      if (sessionStore.read(RESTORE_NAV_SESSION_KEY) === 'restore') {
         api.toggleNav(true);
       }
-      sessionStorage.removeItem(RESTORE_NAV_SESSION_KEY);
+      sessionStore.remove(RESTORE_NAV_SESSION_KEY);
     };
   }, [api]);
 
@@ -110,6 +158,20 @@ const ReviewPageContent: FC<{ search: string }> = ({ search }) => {
     return info;
   }, [index, state]);
 
+  // Stories the change-detection mechanism flagged as newly added. This is an
+  // independent "New" signal from the baseline-index check: a story can be new
+  // here even when a baseline exists (e.g. added on this branch).
+  const allStatuses = experimental_useStatusStore() as StatusesByStoryIdAndTypeId;
+  const changeDetectedNewStoryIds = useMemo(() => {
+    const ids = new Set<string>();
+    for (const [storyId, statusesByType] of Object.entries(allStatuses)) {
+      if (Object.values(statusesByType).some((status) => status.value === NEW_STATUS_VALUE)) {
+        ids.add(storyId);
+      }
+    }
+    return ids;
+  }, [allStatuses]);
+
   // SPA navigation: imperatively attach a click listener to the container so
   // left-clicks on in-page review links push history and swap the iframe URL
   // without a manager reload (no flash). Real hrefs are kept for accessibility
@@ -118,6 +180,7 @@ const ReviewPageContent: FC<{ search: string }> = ({ search }) => {
   // interactions` doesn't flag the delegation root — the div isn't itself
   // interactive, it's just catching bubbled clicks from real <a> elements.
   const containerRef = useRef<HTMLDivElement>(null);
+  const summaryWrapperRef = useRef<HTMLDivElement>(null);
   useEffect(() => {
     const container = containerRef.current;
     if (!container) {
@@ -186,13 +249,21 @@ const ReviewPageContent: FC<{ search: string }> = ({ search }) => {
       const nextStoryId = detailStoryIds[nextStoryIndex];
       const currentStoryId = detailStoryIds[currentStoryIndex];
       const currentStoryInfo = storyInfo[currentStoryId];
+      // A story is "New" if change-detection flagged it (regardless of any
+      // baseline), or once the baseline index has resolved and confirms the
+      // story is absent. While the baseline is unresolved/unavailable (`null`)
+      // it contributes nothing — only the change-detection signal applies.
+      const isNew =
+        changeDetectedNewStoryIds.has(currentStoryId) ||
+        (baselineStoryIds !== null && !baselineStoryIds.has(currentStoryId));
       detailScreen = React.createElement(DetailsScreen, {
         title: detailTitle,
         storyId: currentStoryId,
         storyIndex: currentStoryIndex,
         totalStories,
         componentTitle: currentStoryInfo?.title,
         storyName: currentStoryInfo?.name,
+        isNew,
         backHref: buildReviewChangesSummaryHref(activeTab),
         previousHref: buildReviewChangesDetailHref(
           detailLocation.kind === 'collection'
@@ -220,13 +291,23 @@ const ReviewPageContent: FC<{ search: string }> = ({ search }) => {
               },
           activeTab
         ),
-        branchName: state.branchName,
       });
     }
   }
 
   const hasDetailScreen = detailScreen !== null;
 
+  // While the detail screen is open the summary stays mounted behind it, but
+  // must drop out of the tab order and the accessibility tree so keyboard and
+  // screen-reader focus can't reach it. React 18 doesn't serialize a boolean
+  // `inert` prop to the DOM, so toggle the property imperatively.
+  useEffect(() => {
+    const node = summaryWrapperRef.current;
+    if (node) {
+      node.inert = hasDetailScreen;
+    }
+  }, [hasDetailScreen]);
+
   return React.createElement(
     'div',
     { ref: containerRef, style: { display: 'contents' } },
@@ -236,8 +317,8 @@ const ReviewPageContent: FC<{ search: string }> = ({ search }) => {
       React.createElement(
         'div',
         {
+          ref: summaryWrapperRef,
           'aria-hidden': hasDetailScreen || undefined,
-          inert: hasDetailScreen || undefined,
           style: hasDetailScreen ? { pointerEvents: 'none' } : undefined,
         },
         React.createElement(SummaryScreen, {

code/addons/review/src/constants.ts

@@ -7,6 +7,11 @@ export const REVIEW_CHANGES_URL = '/review/';
 // full-reload navigations between review screens. Value: 'restore' | 'keep'.
 export const RESTORE_NAV_SESSION_KEY = `${ADDON_ID}/restore-nav`;
 
+// sessionStorage key remembering the detail screen's preview layout so it
+// persists as the user moves between the detail and summary screens (and
+// across the full-reload navigations between them). Value: '1up' | '2up'.
+export const PREVIEW_MODE_SESSION_KEY = `${ADDON_ID}/preview-mode`;
+
 // `@storybook/addon-mcp` display-review tool call emits this event with the raw agent payload.
 const PUSH_REVIEW = `${ADDON_ID}/push-review`;
 // Display agent review in the UI