This repository was archived by the owner on Oct 14, 2026. It is now read-only.
Repository navigation
Agent-eval: expand to the 15-eval line, and fix the review-skip failure modes it uncovered (#324) - #325
Merged
Conversation
- Blanket workflow assertions on all visual-change evals: expectStoryTestsRanAndPassed (run-story-tests called + last run green) and expectStoryDiscoveryBeforeReview, per the Validation Workflow and story-ID mapping instructions; 802 also gains the §6a.2 completeness assertion. - New evals: 808 shared-infra fallback, 810 fix-failing-tests, 811 fix-a11y-violations, 812 first-story-empty-project, 813 monorepo-leaf-create-component, plus the plugin-only lifecycle block 820 init / 821 upgrade-from-sb9 / 822 upgrade-from-stable. - New minimal vite-app and monorepo templates; pinStorybookPackages and local-build detection now cover workspace package.json files, and fixtures can opt out of pinning with evals.pinStorybook: false. - Experiment lists route lifecycle evals to plugin experiments only; README documents the full-run cost estimate against the $75 budget.
✅ Deploy Preview for storybook-mcp-self-host-example canceled.
|
🦋 Changeset detectedLatest commit: 97b5111 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #325 +/- ##
==========================================
+ Coverage 79.29% 79.46% +0.17%
==========================================
Files 50 50
Lines 2053 2070 +17
Branches 595 606 +11
==========================================
+ Hits 1628 1645 +17
Misses 220 220
Partials 205 205 ☔ View full report in Codecov by Harness. |
…08's validation assertion to the plugin path Real QA runs surfaced that addon-vitest cannot start Vitest without a root vitest.config.ts exposing the storybook test project — 812 and the monorepo leaf now re-export their vitest.storybook.config.ts, matching the reshaped template. 808's run-story-tests assertion is gated to the plugin integration (MCP path loses the Validation Workflow to the server-instruction truncation PR #320 addresses); accepted known failure tracked in #317.
Bundle ReportChanges will increase total bundle size by 2.4kB (0.93%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: @storybook/addon-mcp-esmAssets Changed:
Files in
|
…rden Codex transcript capture Extends the local agent-eval patch so the failure classifier can run on a direct Anthropic API key (saving Vercel AI Gateway credits, with ANTHROPIC_BASE_URL normalization) and so Codex runs fall back to the saved ~/.codex session transcript when newer Codex builds stop echoing the full JSONL to stdout. Lockfiles update the recorded patch hash.
thermo-nuclear subagent review: - start-storybook-mcp.mjs now lives once in lib/mcp and is injected into any sandbox whose package.json references it (was triplicated verbatim). - The Playwright Amazon Linux package list becomes a shared 'playwright-chromium' preset referenced from every eval-template.json. - expectStorybookBoots polls /index.json (a real build artifact) instead of /. - 811 gains anti-deletion guards (icon-only story must survive the fix). Codex (gpt-5.5, xhigh) review: - expectStoryTestsRanAndPassed takes a covering option: the final green run-story-tests result must cover the changed component's stories, so a pre-change test run can't satisfy the Validation Workflow assertion. Validated against every passing 2026-07-02 QA transcript. - 811 additionally requires the button-name violation to have been observed in some test run before its absence counts as a fix. - 821/822 require storybook AND @storybook/react-vite at >= 10.4.6 (the stable at authoring time), so under-upgrades like 10.0.0 don't pass. - 808's conditional fallback requirement is kept deliberately: the 2026-07-02 cc-mcp QA run legitimately covered both consumers via get-changed-stories alone (module-graph related stories), matching spec §4; documented inline. 813 documents two accepted known failures inline (Claude plugin sometimes sources instructions from 'storybook ai setup' and verifies with raw vitest in the monorepo leaf); assertions gated to the passing integrations, per the code-comment policy for known failures. 808's MCP-path validation gating is documented the same way.
kasperpeulen
force-pushed
the
kasper/eval-coverage-324
branch
from
July 2, 2026 18:37
87c9dde to
eb3aaf9
Compare
kasperpeulen
added
ci:eval
A few covered lines in the package unit tests are timing-dependent, so identical source can lose a couple of hits between uploads; PR #325 failed codecov/project at -0.10% with zero coverable lines in its diff. Real regressions are far larger than this threshold.
Member
Author
|
First labeled CI eval run (28613334522, full 15-eval line):
All codex failures are genuine single-run agent-behavior misses (the same evals pass in other samples), not fixture or harness bugs — they stay strict. The To take this PR green: add credits to the storybookjs Vercel AI Gateway, then re-run the Everything else is green on the head: build, lint, format, typecheck, tests, publint, codecov patch + project. |
…#35359 storybook ai --help run from the monorepo root cannot load the leaf config and hides every runtime workflow command — the only listed command is setup, so agents (Claude and Codex alike) derail into setup + raw vitest and never discover display-review. The deterministic checks (component created, launch config, preview browser) stay active; swap test.todo back to test once the CLI lists its runtime commands without a resolvable config.
…rete triggers in the stories skill Mode A (agents end visual work at preview URLs instead of the review; seen on the Codex MCP path in evals 804/812): the review-enabled after-change instruction now feeds get-changed-stories into display-review instead of ending at preview-stories, preview-stories is explicitly a mid-loop tool, the 'or you skipped it' escape is gone (non-visual changes say so plainly instead of listing links), and get-changed-stories results append a 'publish the review now' hint when review is enabled — tool results are never truncated, so the nudge lands at the decision point. Validated locally: 6/6 codex-mcp samples of 804+812 now publish the review, with preview-stories gone from every workflow chain (baseline 2/4). Mode B (the stories skill does not trigger on shared-token edits; eval 808): the skill description now leads with concrete nouns (components, stories, styles, CSS, themes, colors, design tokens) instead of the file-type-based 'frontend component change', identical in both plugins and 321 bytes (limit 350). Honest caveat: measured trigger rate on token-edit prompts stayed ~2/3 across three wordings (9 samples) — the residual is Codex-side skill-triggering reliability, not wording; 808 stays strict as the canary.
…vals expectSkillInvoked now covers Codex too: engaging a skill there means reading its SKILL.md, which the transcript records as a shell command (codex skill directories drop the storybook- prefix, so storybook-init maps to skills/init). Added as a named assertion to 801-804, 808, 812, and 813, so a Mode B trigger miss fails as 'invokes the stories skill' instead of surfacing indirectly as missing workflow calls — and the trigger rate becomes readable straight from the vitest output. Replayed against all 32 passing plugin transcripts on file: zero would-flips. 805 deliberately has no such assertion: a pure rename should not trigger the skill.
The spec's '## 👀 Review your changes' heading is an example, not a literal: a browse request has no changes, and codex sensibly titled the section '## ReviewCard States' (CI run 28623099303) while doing the whole flow correctly. The heading check now requires a review-ish heading; the load-bearing assertions (AI-curated disclaimer, review link as the very last line, no story preview URLs) are unchanged.
The codex-mcp 806 failure in CI run 28650660373's predecessor showed the failure shape: the agent resolved story IDs, announced it would publish a review, then claimed display-review "isn't exposed in this session" without attempting the call — and the preview-stories description's hedged "when the display-review tool is available" clause sanctioned raw preview links as the fallback for exactly that false belief. When review is enabled, preview-stories now states display-review's availability as fact in its description and appends a recovery nudge to results that resolved URLs, contradicting the missing-tool belief one step before the final message. When review is disabled the description drops all display-review mentions — the tool is not registered there. Validated locally on codex-mcp: 806 (browse request needing the review) and 805 (non-visual refactor asserting no review) both pass. Also rewords the shell-parse walk-back comments per CodeRabbit: env is not matched by SHELL_BINARY_PATTERN, and separate-value flags like bash -O extglob -c are a documented accepted miss.
Comment on lines
+30
to
+32
| return `Use this tool to get one or more Storybook preview URLs. | ||
| Include each returned preview URL in your final user-facing response so users can open them directly — unless you're also publishing a curated review via display-review, in which case link the review page instead of listing individual URLs. | ||
| When the user asked to see or browse existing stories or components (e.g. "show me all the Button variants") and the display-review tool is available, publish a curated review with display-review (passing changedFiles: []) instead of answering with raw preview links; use this tool for verifying your own changes or sharing a specific story on request.`; | ||
| The display-review tool is available in this session. When the user asked to see or browse existing stories or components (e.g. "show me all the Button variants"), publish a curated review with display-review (passing changedFiles: []) instead of answering with raw preview links; use this tool for verifying your own changes or sharing a specific story on request.`; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/addon-mcp/src/tools/preview-stories.test.ts (1)
822-824: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor:
anyin generic type param weakens type safety.
callToolOn(targetServer: McpServer<any, AddonContext>, ...)usesanyfor the first generic parameter; consider using a concrete/inferred type matchingcreateReviewEnabledServer's return type or the outerserver's type for stronger typing.🤖 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 `@packages/addon-mcp/src/tools/preview-stories.test.ts` around lines 822 - 824, The helper callToolOn currently weakens type safety by using any in McpServer<any, AddonContext>; update it to use the concrete server type returned by createReviewEnabledServer or the existing server variable’s inferred type, and keep the targetServer parameter aligned with that type so the generic argument is fully typed.
🤖 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.
Nitpick comments:
In `@packages/addon-mcp/src/tools/preview-stories.test.ts`:
- Around line 822-824: The helper callToolOn currently weakens type safety by
using any in McpServer<any, AddonContext>; update it to use the concrete server
type returned by createReviewEnabledServer or the existing server variable’s
inferred type, and keep the targetServer parameter aligned with that type so the
generic argument is fully typed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 938abcaf-da46-4236-9e8c-76eb171e799f
📒 Files selected for processing (5)
.changeset/eval-review-exit-ramp.mdagent-eval/lib/shell-parse.tspackages/addon-mcp/src/tools/preview-stories.test.tspackages/addon-mcp/src/tools/preview-stories.tspackages/addon-mcp/src/tools/tool-registry.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/eval-review-exit-ramp.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/addon-mcp/src/tools/tool-registry.ts
- agent-eval/lib/shell-parse.ts
The internal storybook runs with changeDetection enabled, so the tools listing now carries the assertive review-enabled description and the preview-stories result appends the display-review recovery nudge. Missed in the previous commit because the e2e suite locally ran against a stale addon-mcp build.
The recurring cc-plugin discovery-skip flake (801/802/804 across three sweeps: Opus constructing story IDs from file names and publishing a review with zero discovery calls) traced to a stale instruction: the storyLinkingWorkflow line in get-storybook-story-instructions still had the pre-Mode-A wording — get-changed-stories feeding preview-stories — and the story-ID discipline (never invent IDs) lives only in the MCP server instructions, which plugin agents never receive. Their sole workflow channel simultaneously demanded a review and never said where its story IDs must come from. With review enabled, the story-linking workflow now discovers changed stories, mandates that story IDs come from discovery tools, and feeds them into display-review, with preview-stories reserved for iteration. The non-review branches keep their existing wording. Validated locally on cc-plugin-opus-high: 801, 802, and 804 all pass.
Plugin agents do receive workflow guidance from the storybook ai CLI help text as well — this tool is not their only channel, just the one that still carried the stale discovery-into-previews routing.
kasperpeulen
removed
the
ci:extra-evals
huang-julien
approved these changes
Jul 3, 2026
Drop the 'include each returned preview URL in your final response' default from the review-enabled branch — it framed URL-listing as the baseline and the review as the exception, the same inverted default the final-links guidance removed. With review enabled the tool is strictly mid-loop: iterate on a story or hand out an explicitly requested link; visual work and browse requests end with display-review. The review-off description is unchanged.
storybook ai --help embeds the same server instructions that the MCP handshake delivers (metadata.instructions from buildStorybookAiMetadata, rendered by the CLI as '# Storybook workflow instructions'), so plugin agents do see them — the earlier comments claiming otherwise were wrong. The actual failure: this tool, billed as the source of truth for story work, contradicted those instructions by routing discovery into previews, and agents resolved the conflict by hand-deriving story IDs.
Closed
kasperpeulen
added a commit
that referenced
this pull request
Jul 3, 2026
Resolutions: keep main's sharpened review-ON wording and its two new review-on tests; drop the re-aimed graph-fallback test they supersede; the review-OFF path stays the legacy released text. The dead review-off arm in the review-on builder branch is removed — that branch is only reachable with the flag on now.
This was referenced Jul 3, 2026
Closed
Closed
This branch had an error being deployed
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Implements #324, plus product fixes for the failure modes the new evals uncovered.
1. The eval line (issue #324)
Workflow-step hardening on every visual-change eval (801–804 + new 808/812/813):
expectStoryTestsRanAndPassed({ covering })—run-story-testswas called, the final result is green (## Passing Stories, no failing/unhandled sections), and covers the changed component's stories (so a pre-change run can't satisfy it). Parses the raw transcript across all four agent/integration paths (parseWorkflowToolResults, unit-tested).expectStoryDiscoveryBeforeReview()— story IDs must come fromget-changed-stories/get-stories-by-component, called before the review.expectSkillInvoked('stories')— the plugin path must engage the skill (Claude:Skilltool; Codex:SKILL.mdread). Replayed against all 32 passing plugin transcripts: zero flips.8 new evals (joining the 7 existing 80x evals = 15 total; numbering: 808–813 continue the workflow line, 82x is the lifecycle block):
button-name+color-contrast; deterministic — violation must be observed, story must survive, colors must stay)vite-appmonorepostorybook-initskill; lifecycle outcome + boot check on/index.json)vite-appstorybook+@storybook/react-vite)vite-appvite-appHarness: two new minimal templates (
vite-app,monorepo); workspace-aware Storybook pinning + local-build detection;evals.pinStorybook: falseopt-out for the upgrade fixtures;start-storybook-mcp.mjsdeduplicated intolib/mcp(injected wherever referenced); shared"playwright-chromium"system-package preset; lifecycle evals routed to plugin experiments only; default CI scope unchanged (801 smoke),EVAL_EXTRA_EVALS=1= 54 runs ≈ $30–45 (budget $75, documented in the README).2. What the evals uncovered — four review-skip modes
storybook ai setupin monorepos (813)storybook ai --helpfrom the repo root can't load the leaf config and hides all workflow commands —setupis the only one listedtest.todopinned to itrun-story-testsa11y output; not in this PRpreview-stories' description hedged "…and the display-review tool is available", sanctioning raw links for exactly that false belief; the browse path had no result-level nudge at allget-storybook-story-instructions— billed as the source of truth for story work — still had the pre-Mode-A story-linking line (discovery →preview-stories), contradicting the server instructions thatstorybook ai --helpembeds; agents resolved the conflict by hand-deriving IDsAccepted known failures are documented as code comments above the relaxed assertion (no tracking issue), incl. 808's MCP-path validation gating (instruction truncation, #320's territory).
One harness bug also fell out of transcript analysis: the plugin-path shell parser treated any
-ctoken as abash -cwrapper, so a compound command containinghead -c 800swallowed a genuineget-changed-storiescall (802 false negative). Fixed with a shell-binary walk-back + regression tests; the failing CI transcript replays green.3. Product fixes in this PR
Mode A —
display-reviewis now the single ending for visual work (@storybook/addon-mcp, changeset included):preview-stories;preview-storiesis explicitly a mid-loop tool ("never as the ending of visual work; the review is the ending");get-changed-storiesresults append a "publish the review now" next-step hint when review is enabled — tool results reach the model in full, unlike server instructions (Claude Code cuts those at 2,048 chars; on this PR's head the Validation Workflow section starts at char 7,742 of 9,110, measured); the hint is front-loaded anyway since host output caps can cut long tails.Validated: 6/6 local codex-mcp samples of 804+812 post-fix, with
preview-storiesgone from every workflow chain (baseline 2/4); then 804+812 green again in the full CI run.Mode B — stories-skill trigger reworded (both plugins, identical, 321/350 bytes): concrete nouns frontloaded ("components, stories, styles, CSS, themes, colors, or design tokens — anything that changes how the UI looks"). Honest measurement: trigger rate on token edits stayed ~2/3 across three wordings (9 samples) — the residual is codex-side skill-consultation reliability, not wording. Side win: 805 (pure rename) now consistently and correctly does not trigger.
Mode E —
preview-storiesno longer offers the review exit ramp (@storybook/addon-mcp): with review enabled its description states display-review's availability as fact instead of "when available" (no conditional left for a hallucinated absence to hide behind), and results that resolved URLs append a recovery nudge routing finished visual work and browse requests back to display-review — contradicting the missing-tool belief one step before the final message. With review disabled the description drops all display-review mentions (the tool isn't registered there). Validated: local codex-mcp 806 (needs the review) and 805 (must not review) both pass.Mode F — plugin story-linking routes discovery into the review (
@storybook/addon-mcp): theget-storybook-story-instructionsworkflow line — which agents treat as the source of truth for story work, alongside the server instructions embedded instorybook ai --help— now discovers changed stories, mandates that story IDs come from discovery tools (never file names, export names, or memory), and feeds them into display-review, with preview-stories reserved for iteration. Validated: local cc-plugin-opus runs of exactly the three flaked evals (801, 802, 804) all pass.4. Where the line stands (CI evidence)
Two full 54/54 sweeps landed on this branch on 2026-07-03: 28652912445 (head
1f7ad7c) and 28655158136 (head8aa84bb, all four experiments 100%, zero classifier findings — the run embedded below). Every failure in the intervening sweeps was hand-verified against the raw transcript before acting: one harness bug (the-cparser, fixed), the rest genuine agent behavior that became Modes E/F above and was product-fixed rather than re-rolled. All other checks (build, lint, typecheck, tests incl. e2e snapshots, publint, codecov patch+project, preview releases) are green on the head.5. Verification & reviews
pnpm --dir agent-eval run typecheck/eval:dry(all four routing modes) /oxfmt --check/ 404 unit tests across agent-eval + addon-mcp ✅gpt-5.5@ xhigh (gpt-5.5-codexunavailable on both auth paths; 5 findings — 4 fixed, 1 rejected with transcript evidence, documented inline in 808).get-changed-storiesand URL-resolvingpreview-storiesresults (not zero-result paths);expectSkillInvokedasserts the mechanism (skill engagement) rather than the outcome; the 10.4.6 upgrade floors are dated constants (commented);lib/test-utils.tsis at 970 lines — split before it grows past 1k.Agent eval results
3/3 evals passed · 1.8M tokens · $2.19 (workflow run)
Playground: https://storybook-evals-4gips90y0-storybookjs.vercel.app
Results by experiment
cc-mcp-opus-high(2026-07-03T11-36-52.728Z)801-create-component-no-launch-configcodex-mcp-gpt-5.5-medium(2026-07-03T11-36-52.729Z)801-create-component-no-launch-configcodex-plugin-gpt-5.5-medium(2026-07-03T11-36-52.730Z)801-create-component-no-launch-configCost is estimated from model token usage at provider list prices (Vercel AI Gateway adds no markup) and excludes Vercel Sandbox compute.
Summary by CodeRabbit