ci: guard Storybook rendering and dead CSS; cut the catalog to one story per surface - #1783
Merged
Conversation
Astro-Han
changed the title
Astro-Han
force-pushed
the
ci/storybook-guardrails
branch
2 times, most recently
from
August 1, 2026 10:34
13af652 to
47cb569
Compare
check-dead-css was red on main: `.astryx-collapsible-trigger` is rendered by Astryx's own Collapsible and targeted by chat-message.css to give reasoning and tool disclosures one trigger dialect (#1768), but it never appears as a className literal, so the scanner counted it dead. It joins the astryx-button / astryx-badge entries already in DYNAMIC_STYLE_HOOKS. The regression stayed invisible because the guard only ran under `check:release`. It now runs in the typecheck job alongside knip, which is the same class of source-scanning governance. Storybook had no CI coverage of rendering at all. Story typechecking already rides along in `typecheck` via tsconfig.storybook.json, but the static build and the 51-check render/play smoke ran only when someone remembered them locally. Both now run at the end of the e2e job, next to the alignment audit. That job drives Electron through `_electron.launch`, which uses Electron's own binary and never downloads a browser; the smoke calls `chromium.launch()`, so Chromium is installed explicitly. Locally the build takes 7s and the smoke 17s cold, which is noise next to the Electron suite. The e2e route also had to widen. It keys on direct workspace changes so backend work does not drag the renderer suites along, and that stays. But .storybook/preview.tsx reads THEME_PALETTES straight out of packages/core, so a core change could break the Storybook build while routing away from the only job that builds it.
Eleven stories documented something other than this product. Five were straight Astryx pass-throughs. Button, Badge and Astryx Atoms render re-exports from packages/ui/src/index.ts; Menu and Dialog import @astryxdesign/core/DropdownMenu, /Dialog and /Layout directly. Astryx publishes its own Storybook for all of them, so ours could only ever be a staler copy. Six were catalogues of the token layer #1565 is deleting. Typography, Elevation, Layering, Palette Matrix and Spacing were last touched before the first Astryx commit; Design System/Tokens is newer but documents the same set. All of them hand-render --shadow-minimal, --border-strong and --font-sans through inline styles, under a namespace that presents them as current truth. Rewriting them against Astryx tokens now would mean writing them twice, and once the token contract is settled the useful artifact is one Maka/Astryx integration story that can act as an oracle for the migration — not six generic catalogues that duplicate upstream. Kept: Primitives/Toast wraps Maka's own toast.tsx (the toast.confirm() queue) and product-smoke-manifest.json pins primitives-toast--confirm-queued; Primitives/StatTile covers a Maka primitive. Design System/Icons, Animation Catalog and Interaction States also reference legacy variables, but what they document — the Maka icon set, Maka motion tokens, interaction states — outlives the token rename, so they move with it rather than out. Storybook still builds and the smoke still passes its 51 render/play checks.
…onvention FIDELITY.md described `story-annotation-contract.test.ts` as the thing that checks every product story carries a `// Real path:` sentence, and spent a paragraph on how it derives its file list and refuses to skip exports it cannot classify. #1724 deleted that test with the rest of the source-scanning contract suite. A document that promises enforcement nobody runs is worse than one that admits there is none — it tells reviewers the check already happened. The convention itself is worth keeping, so it now says plainly that it is a review convention and that it decays like one. It had already decayed: AstryxNativeConversation was the one product story with no annotation. It is reachable — a long session accumulates reasoning, tool calls and prose, and an image can sit staged in the composer — but it stacks those states deliberately, which FIDELITY.md requires a story to say out loud. Its comment now says both. Every `Product/*` story carries the sentence again; the remaining gaps are all `Primitives/*` and `Design System/*`, which the document exempts.
The header said tailwind-theme.css stays out "until Tailwind leaves", which has it backwards. That file's own header calls it a Tailwind Bridge: it maps Astryx tokens onto Tailwind v4 @theme variables so utilities resolve against the theme. It is only useful while Tailwind is present, and the slice in question deletes Tailwind — so it should never be imported at all. Anyone reading the old note while planning that slice would go looking for an import that must not happen. Also corrects the slice number: Tailwind removal is Slice 13, not 12, and #1565 asks that "PR N" never be used as a synonym for a slice.
…tate stories The earlier cut used the wrong test. It asked "is this Maka's own?" and kept whatever answered yes. The question that matters is whether Astryx already owns the domain — if it does, a Maka catalogue of the same thing is a parallel vocabulary being documented as the contract, which is exactly what the six deleted token stories were. Animation Catalog failed that test. Its DurationScale and EasingScale presented --duration-quick/base/emphasized/large and the four --ease-* tokens as the motion scale, while the Astryx theme already ships --duration-fast/medium/slow. Both are gone. What remains is the spinner and the streaming shimmer — load-bearing, not decorative, and they outlive the token rename — so the file is now Design System/Functional Motion, which is what it actually shows. Interaction States failed it too, in the part nobody flagged: ButtonStates, NeutralButtonStates and SolidButtonStates are state matrices for Astryx's Button, the same component whose Primitives/Button story this branch already deleted. ListRowStates stays, and it is the reason the file stays: it compares hover and focus across two DIFFERENT row components — Astryx's side-nav item and its list item — as the sidebar composes them. session-list-panel.stories.tsx owns SessionListPanel's own states; nothing else covers the seam between the two. The file is now Design System/Composite Row States. Design System/* is left holding Icons and Functional Motion — the Maka icon set, the brand mark, and two functional animations — plus that one cross-component seam. Nothing in it catalogues a token scale any more.
The previous commit had FIDELITY.md admit that nothing checks the `// Real path:` annotations. That was honest but it was the wrong repair: presence of the sentence is mechanically decidable, and this branch had just proved review does not hold that line — chat-surface.stories.tsx reached thirteen stories with twelve annotations and nobody noticed until a script counted them. So the convention now splits along what a machine can decide. Existence is checked by scripts/check-story-annotations.mjs in CI; truth stays with the reviewer, where it has to stay — a schema is satisfied by a plausible lie just as easily. This is not a revival of story-annotation-contract.test.ts. #1724 was right to delete 149 tests that read source text and charged every refactor a rewrite of its own guards, and it explicitly kept the fast scripts/check-*.mjs commands for non-cosmetic invariants. This is one of those: ~110 lines, no build, runs beside check-dead-css in the typecheck job. It keeps the one property of the retired test that mattered: it fails on any top-level export it cannot classify rather than skipping it, because a guard that ignores what it cannot parse passes *because* it did not understand. It also fails if .storybook/main.ts stops loading a directory it scans, so a story tree cannot drift out of coverage. Eight tests cover both, including the case that caught a real bug while writing this: both story roots end in `stories`, so the config check originally matched only the last path segment and stayed green when either root was removed.
The smoke this branch wired into CI verified 12 stories out of 141. The manifest is curated on purpose — its 51 checks assert layout geometry across three viewports and both colour schemes, which is expensive and only worth paying for where layout actually varies. But that left ~130 stories verified by nothing: build-storybook bundles a story without ever mounting it, so a render that throws, a play function that rejects, or a console error all ship green. "Storybook can no longer rot unobserved" was not true as written. The catalog pass renders every unmanifested story once at wide/light and asks one question: does it mount and finish its play function without errors. It reuses smokeStory, so it already catches storyThrewException, playFunctionThrewException, unhandledErrorsWhilePlaying, console errors and empty roots. Four pages run concurrently: 124 renders in ~35s, against 217s serial. The manifest jobs stay serial and unbaselined — they measure geometry, which is why they pin a viewport. The first run found seven already-broken stories, which is the argument for the pass. Each is a play function whose selector went stale during the Astryx migration with nothing executing it — model-picker looks for the exact placeholder '搜索模型' while shared-ui-copy.ts now reads '搜索模型…'; ask-user-question wants an accessible name user-question-prompt no longer exposes; tool-activity predates the #1768 disclosure dialect. They span four components and each needs that component's current DOM to fix, which is a different change from wiring up the guard. So they go in storybook-catalog-baseline.json, keyed by story id with the reason — the same shape as check-dead-css-baseline.json. Unlisted failures fail the build. A listed story that starts passing ALSO fails, demanding its removal; without that a baseline quietly turns into a permanent exemption, and the list stops being a to-do.
The catalog pass found seven stories whose play functions had gone stale during
the Astryx migration. The previous commit recorded them in a baseline so the
guard could land without also fixing four components. Deleting them is the
better answer: a story that has not verified anything since the migration is not
coverage waiting to be restored, and the states they claimed to cover are either
already covered elsewhere or were asserting behaviour rather than appearance.
model-picker Search / NoResults filtering the catalog; the empty result is
EmptyCatalog's job
tool-activity DisclosureInteraction aria-expanded, computed display, hit-area
width and chevron centring — geometry that
audit-alignment.mjs and e2e own
ask-user-question OtherAnswerSelected typing into the free-text answer
settings-pages WebSearch/Voice ×3 provider round-trips reaching a status text
Three more went with them under the same rule — a story earns its place only if
someone opens it to make a visual judgement that no other story supports:
model-picker Pending Default with the control disabled
tool-activity CopyFeedback same fixture as ErrorsAndPermissionDenied
plus a transient label
tool-activity FileDiffAndWebSearch denseMixedResultItems already contains all
three of its items, expanded
So storybook-catalog-baseline.json and the reconcile step are gone with them.
Without known-broken entries the catalog pass is just "every story renders", and
a future breakage has two honest options — fix it or delete it — rather than a
third that defers both.
136 stories → 126; the smoke now runs 51 manifest checks and 114 catalog renders
in ~64s with nothing exempted.
126 stories down to 58. The rule: a story earns its place only if someone opens
it to make a visual judgement no other story supports. In practice that leaves,
per surface, the densest reachable state — plus an empty state where the layout
genuinely differs, plus whatever product-smoke-manifest.json pins.
What went:
state enumerations SearchModal's Loading / NoResults / Error / Blocked next
to Results; onboarding's four gate variants of one hero;
provider-settings' Loading / LoadError / Empty
behaviour, not paint CommandPaletteKeyboardFocusedSelection, RowActions,
RowMenuOpen — interaction the E2E suite owns
states contained by Usage and UsageEmpty under UsageRequestsPopulated;
a denser sibling Memory under MemoryPopulated; app-shell's PlanModeActive
and SwarmModeActive; tool-activity's StatusOverview,
TerminalAndLiveOutput and SubagentAndExplore, all inside
denseMixedResultItems
duplicated elsewhere app-shell's MarkdownCore against markdown.stories.tsx
Every story the manifest pins survives, including the eight module-hubs entries
and primitives-toast--confirm-queued.
Deleting stories orphans their fixtures, and tsconfig.storybook.json does not
set noUnusedLocals, so nothing would have reported them. Compiling once with the
flag on found fourteen — bridges, settings fixtures, voice capture helpers — and
they go too.
Two guards caught mistakes made while doing this, which is the argument for
having wired them up: validateCoverageManifest rejected a build after a regex
with a shared name prefix took ExtensionsSkillsInstalled out along with
ExtensionsSkills, and check-story-annotations rejected the next one after a
cleanup pass ate the `// Real path:` line above Models.
The catalog pass is now 47 renders instead of 114, and the whole smoke runs in
34s instead of 64s.
Three of the cuts failed the retention rule they were made under — "keep the state no other story supports" — and an independent review of the branch caught all three. Palette Matrix was dropped as a token catalog Astryx now owns. It is not one: THEME_PALETTES is a product feature, the palettes a user picks in 设置 → 外观, and the catalog pass renders wide/light/default only. Ten of the eleven had no oracle at all. The file's header now records why the other five token stories still went, since "Astryx already owns this domain" was the wrong reason for all of them — maka-tokens.css is still the single authority and dies with Slice 13; a table of var names is simply low review value with a short life. Permission Center kept the collapsed state and cut the expanded one, which is backwards: the capability layers grid and guidance block only exist once diagnostics are expanded, and the deleted story's own comment says that is where the last overflow bug was hiding. Everything the collapsed story showed is still on screen in the expanded one, so this stays one story. VoicePermissionDenied comes back because the surviving Voice story is idle and the page's only error surface had no coverage anywhere — the E2E suite checks fields and persistence, not capture outcomes. It comes back without the 120-line MediaRecorder decorator the old story carried: permissionSnapshot already reports microphone `denied` on darwin and runCaptureSmoke returns on that snapshot before it ever reaches getUserMedia, so the real path needs no browser mocking. The catalog pass then failed the restored story immediately, for the same reason the originals rotted: the result lives in one of six `[role="status"]` regions and `querySelector` had picked an empty one. Also drops two doc comments the prune orphaned, which now described stories that no longer exist.
The guard claimed to fail on anything it could not classify. Review found five
inputs where it did not, each now covered by a test:
- `.storybook/main.ts` loads `*.stories.@(ts|tsx)`; the scanner only opened
`.stories.tsx`, so a `.ts` story was never read at all.
- `export { Story }` and `export async function Story()` reach the browser as
stories and matched nothing, so they passed in silence — the exact failure
the "fail on what you cannot parse" rule exists to prevent.
- `export const X: Story =` wrapping onto the next line was reported as a
malformed export. A guard that reddens on formatting teaches people to
ignore it.
- The title regex took the first `title:` in the file, not meta's. A fixture
literal above meta decided the file's namespace, and a `Design System/…` one
exempted every story in it.
- An empty `// Real path:` counted as an annotation.
checkStorybookRoots claimed to catch a root added to main.ts but not scanned
here; it only checked that the two known roots were still present. It now
compares both directions, and the test covers the addition case and the
accept-the-real-config case — the latter previously asserted `[] deepEqual []`
against a function that returns undefined, so it verified nothing.
FIDELITY.md now also says what the check is: a convention guard that must fail
in seconds inside typecheck, approximating a story set only Storybook's index
truly knows. Every gap above was closed by widening what it fails on, never by
widening what it skips.
`scripts/**` only sets scriptMode, so a change to the smoke runner was verified by nothing but the run it never triggered — the one file most able to make the guard silently stop guarding.
Astro-Han
force-pushed
the
ci/storybook-guardrails
branch
from
August 1, 2026 14:14
47cb569 to
2ab7931
Compare
Astro-Han
added a commit
that referenced
this pull request
Aug 1, 2026
Plan reminders held five of the manifest's surfaces while every neighbouring page held one, and two of the five paid for it without showing anything new. The attention state is a single blocked row, which is exactly the kind of thing a scanning page only proves it surfaces when the row has healthy neighbours — so it joins the configured list, the way ExtensionsMcpConfigured already pairs one healthy server with one failed one. Keep-awake renders nothing at all: plan-reminder-panel.test.tsx asserts the enabled page stays visually identical and keeps the state inside the settings menu, so the story smoked pixels identical to the configured one and its play function re-tested a prop the unit test already covers. Also drop the "~130 stories" figure from the catalog-pass comment; the count has been wrong since #1783 cut it, and a live number there earns nothing.
Astro-Han
added a commit
that referenced
this pull request
Aug 1, 2026
Plan reminders held five of the manifest's surfaces while every neighbouring page held one, and two of the five paid for it without showing anything new. The attention state is a single blocked row, which is exactly the kind of thing a scanning page only proves it surfaces when the row has healthy neighbours — so it joins the configured list, the way ExtensionsMcpConfigured already pairs one healthy server with one failed one. Keep-awake renders nothing at all: plan-reminder-panel.test.tsx asserts the enabled page stays visually identical and keeps the state inside the settings menu, so the story smoked pixels identical to the configured one and its play function re-tested a prop the unit test already covers. Also drop the "~130 stories" figure from the catalog-pass comment; the count has been wrong since #1783 cut it, and a live number there earns nothing.
Astro-Han
added a commit
that referenced
this pull request
Aug 1, 2026
Plan reminders held five of the manifest's surfaces while every neighbouring page held one, and two of the five paid for it without showing anything new. The attention state is a single blocked row, which is exactly the kind of thing a scanning page only proves it surfaces when the row has healthy neighbours — so it joins the configured list, the way ExtensionsMcpConfigured already pairs one healthy server with one failed one. Keep-awake renders nothing at all: plan-reminder-panel.test.tsx asserts the enabled page stays visually identical and keeps the state inside the settings menu, so the story smoked pixels identical to the configured one and its play function re-tested a prop the unit test already covers. Also drop the "~130 stories" figure from the catalog-pass comment; the count has been wrong since #1783 cut it, and a live number there earns nothing.
Astro-Han
added a commit
that referenced
this pull request
Aug 1, 2026
Plan reminders held five of the manifest's surfaces while every neighbouring page held one, and two of the five paid for it without showing anything new. The attention state is a single blocked row, which is exactly the kind of thing a scanning page only proves it surfaces when the row has healthy neighbours — so it joins the configured list, the way ExtensionsMcpConfigured already pairs one healthy server with one failed one. Keep-awake renders nothing at all: plan-reminder-panel.test.tsx asserts the enabled page stays visually identical and keeps the state inside the settings menu, so the story smoked pixels identical to the configured one and its play function re-tested a prop the unit test already covers. Also drop the "~130 stories" figure from the catalog-pass comment; the count has been wrong since #1783 cut it, and a live number there earns nothing.
Astro-Han
added a commit
that referenced
this pull request
Aug 1, 2026
…scaffolding (#1849) * refactor(stories): derive bot brand icons from BOT_BRAND The catalog kept its own list of the eight IM channels, so the story was satisfied by any subset of the registry BotBrandLogo actually reads — a newly supported channel would simply never render. Read the keys instead, matching how LucideIcons already tracks the icon barrel, and drop the empty omit-list and the always-true component guard it filtered through. * refactor(stories): drop the ask-user-question review scaffold The story built its own two-column board with a hand-written fake transcript and 60 lines of story-only CSS, then hardcoded 760px and 390px chat columns. The prompt already caps itself at --maka-chat-measure and its production slot is display:contents, so those widths were the story's invention rather than the product's — the retyped chain FIDELITY.md warns about, and the transcript above it was a screen no user reaches. Mount the prompt the way its sibling sandbox-boundary-prompt story does, and give the manifest the narrow column back as compact and floor viewports, where it is the real thing instead of a fixed div. * refactor(stories): fold the plan-reminder variants into one list Plan reminders held five of the manifest's surfaces while every neighbouring page held one, and two of the five paid for it without showing anything new. The attention state is a single blocked row, which is exactly the kind of thing a scanning page only proves it surfaces when the row has healthy neighbours — so it joins the configured list, the way ExtensionsMcpConfigured already pairs one healthy server with one failed one. Keep-awake renders nothing at all: plan-reminder-panel.test.tsx asserts the enabled page stays visually identical and keeps the state inside the settings menu, so the story smoked pixels identical to the configured one and its play function re-tested a prop the unit test already covers. Also drop the "~130 stories" figure from the catalog-pass comment; the count has been wrong since #1783 cut it, and a live number there earns nothing. * refactor(stories): host the chat transcript in the real shell Product/Chat Surface mounted ChatView and Composer inside a 960x760 centred card with a border and a radius — a frame the product does not have. In the app the transcript lives in the full-height maka-panel-detail pane that app-shell.stories.tsx already composes from the real AstryxAppShell, so every geometry comparison against the old story was measuring its own scaffolding. Two of its six stories were also the shell stories' states in that wrong frame: StreamingResponse against StreamingTurn, and the two empty chats against EmptyHome, which is the only empty home the app renders. Move the states the shell did not have — the reasoning-and-tool-call transcript and the session context layer — onto ComposedShell, and delete the parallel file with its second copy of the session, message and composer fixtures. Widening the session overlay to Partial<SessionSummary> keeps the one-active-session rule while letting a story set the name and labels the context layer reads. SessionContextNarrow is not carried over: it faked a narrow column with a 360px div, and the manifest now renders the same story at compact and floor. * test: assert the keep-awake state on its settings menu item The panel renders identically whether keep-awake is on or off, so the settings checkbox is the only thing carrying the state — and the existing test only checked that the trigger exists. Deleting the story that opened the menu therefore left aria-checked unverified. Astryx renders the menu in static markup, so the contract lands in the same suite rather than needing a browser. * refactor(stories): derive the shell's chat state from production helpers Review found the migrated stories still short of what this PR claimed. The detail pane retyped AppShellDetailPanel's classes and dropped the two wrappers below it, so .mainColumn never applied and the composer measured its own box; the composer omitted onModelChange, which renders ChatModelSwitcher disabled; and NativeConversation lost the footer actions the old file hardcoded. Mount the real AppShellDetailPanel with the production wrappers, and derive what the renderer derives: deriveAppShellTurnViewModel for footer actions, deriveBranchBanner and deriveSessionRevisionNavigation for lineage. Deriving is the point — SessionContextLayer previously hand-wrote a banner and a revision counter for a session catalog containing neither a parent nor a revision family, and asserted fromAbortedTurn, which app-shell.tsx documents it deliberately never passes. The story now supplies the relatives and gets the banner only if the real rules still produce one. A story that renders no lineage while claiming to show it is the failure this should catch, so sessionContext gets a surface check for the breadcrumbs, the revision counter and horizontal overflow; removing the fixture makes it fail at all three viewports. Excluding id from the session overlay closes the one field that would have desynchronized the sidebar from the chat rather than projecting onto it. * fix(stories): make the shell fixtures states the app can actually produce Second review round found the lineage fixture from the first one was itself impossible. The store's isValidRevisionLineage accepts the five revision fields together or not at all and rejects any index below 2; the fixture set revisionRootSessionId and revisionIndex alone, and made the root its own revision. reviseBeforeTurn shows the real shape: the root carries no revision fields and each revision carries all five, keeping the branch lineage it was revised from. The family is now built that way, and the sidebar runs through projectRevisionLinkedSessionTree, because production collapses a family to one row and the story was about to show three. WaitingForPermission forced disabled: true, but app-shell.tsx never passes disabled to ChatComposerRegion — the textarea stays usable while a tool waits, so the story was inventing a locked input. Restore the no-active-session composer as NewChatComposer. Deleting EmptyChat took the only coverage of Composer's other branch with it: without an active session it renders NewChatModelPicker instead of ChatModelSwitcher and drops the thinking selector. EmptyHome's comment claimed that state differed only in the sidebar, which was wrong. The ask-user-question decorator now reproduces the two wrappers around the composer slot. The prompt's root carries the composer class and .mainColumn zeroes its top padding, so the bare canvas rendered it 8px low; measured at 0px in the built story afterwards. Guards, all three verified by making them fail: - sessionContext joins REQUIRED_PRODUCT_SURFACES, so deleting its manifest entry now fails the validator instead of silently retiring its check. - The check reads every OverflowList copy rather than the first, which is the hidden measurement clone, and asserts goal and revision are visible or in the overflow menu — not merely present in the DOM. - The countdown spacing loop ran zero times if countdowns disappeared; it now requires one when a scheduled row exists. - check-story-annotations skipped `export` followed by a newline, so a story in that shape passed unread while the check reported success. * test(ui): cover every Host origin marker where the assertion runs #1853 asserted the automation-origin marker's line height inside a story `play` function. Play functions never execute in this Storybook — no test addon is configured, and a deliberately throwing `play` still ships green through scripts/storybook-visual-smoke.mjs — so that contract never ran. Keep the automation-triggered turn as visual coverage by folding it into the transcript scaffold, and move the contract into the seam that runs: the existing origin-presentation test, widened from Goal alone to all three Host origins, each of which is a separate branch with its own copy. * docs(stories): record that play functions do not run, and stop restating the checker Two edits to the fidelity convention: - Add the rule the commit before this one discovered: `play` is inert here, so behavioural and computed-style contracts belong in a packages/ui test or the smoke script's checks, not in a story. - Cut three paragraphs that re-derive check-story-annotations.mjs's design — its fail-closed rationale, #1724's history, and why a source scanner only approximates the story set. All of it already lives in that script's header, which is where someone editing the guard will read it. The doc keeps the rule authors need: annotate every story, and write it as `export const Name: Story = …`. - Drop the trailing `# Sandbox boundary prompt` section. It described one story's two variants, in an H1 that broke the document's structure, and those variants carry the same description as their own `// Real path:`.
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
1 participant
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.
Summary
Storybook is where pixel work happens in this repo, and nothing in CI checked that it still renders.
check-dead-csswas already red onmainand nobody could see it, because it lives only undercheck:release. And the catalog had grown to 136 stories, of which the smoke verified 12.Three guards now run in CI, in the jobs that already own their class of work — no new jobs — and the catalog is down to 63 stories, every one of them rendered on every renderer change.
Guards
typecheckjob, next to knip..astryx-collapsible-triggerjoinsDYNAMIC_STYLE_HOOKS: Astryx's ownCollapsiblerenders it andchat-message.csstargets it (refactor(ui): align chat chrome with Astryx message composition #1768), but it never appears as aclassNameliteral, so the scanner counted it dead.typecheckjob. Newscripts/check-story-annotations.mjschecks that everyProduct/*story carries a// Real path:sentence. Existence is mechanical; truth stays with the reviewer.e2ejob, next to the alignment audit. That job drives Electron through_electron.launch, which never downloads a browser, so Chromium is installed explicitly for the smoke'schromium.launch().The
e2eroute now also fires on directpackages/corechanges —.storybook/preview.tsxreadsTHEME_PALETTESstraight out ofpackages/core/src/settings.ts.The smoke had to grow to be worth wiring up. It verified 12 of 141 stories: the manifest is curated on purpose, since its 51 checks assert layout geometry across three viewports and both colour schemes. But
build-storybookbundles a story without ever mounting it, so everything else shipped green regardless. A catalog pass now renders every unmanifested story once at wide/light, reusingsmokeStoryso it already catchesstoryThrewException,playFunctionThrewException, console errors and empty roots.Its first run found seven stories that had been broken since the Astryx migration — play functions whose selectors went stale with nothing executing them.
model-pickerlooked for the exact placeholder搜索模型whileshared-ui-copy.ts:114now reads搜索模型…;ask-user-questionwanted an accessible nameuser-question-prompt.tsxno longer exposes;tool-activitypredated the #1768 disclosure dialect. They are deleted, not fixed: a story that has verified nothing since the migration is not coverage waiting to be restored.Then the catalog itself. 126 → 58, on one rule: a story earns its place only if someone opens it to make a visual judgement no other story supports. Per surface that leaves the densest reachable state, an empty state where the layout genuinely differs, and everything
product-smoke-manifest.jsonpins.CommandPaletteKeyboardFocusedSelection,RowActions,RowMenuOpen— the E2E suite owns interactionUsage/UsageEmptyunderUsageRequestsPopulated; tool-activity's three fixtures, all insidedenseMixedResultItemsMarkdownCoreagainstmarkdown.stories.tsxDeleting stories orphans their fixtures, and
tsconfig.storybook.jsondoes not setnoUnusedLocals, so nothing would have reported them. Compiling once with the flag on found fourteen; they go too.Also deleted, on the rule that a Maka catalogue of something Astryx already owns documents a parallel vocabulary as if it were the contract: five straight Astryx pass-through stories (Button, Badge, Astryx Atoms, Menu, Dialog); six catalogues of the token layer Slice 13 will delete (Typography, Elevation, Layering, Palette Matrix, Spacing, Design System/Tokens);
Animation Catalog'sDurationScale/EasingScale, since the Astryx theme already ships--duration-fast/medium/slow; andInteraction States' three Astryx Button matrices. The two survivors were renamed to what they actually show —Design System/Functional Motion(spinner, streaming shimmer) andDesign System/Composite Row States(hover and focus across two different row components, which nothing else covers).Docs.
FIDELITY.mdclaimed a test enforced the annotations; #1724 deleted it. It now describes the split that exists: existence by script, truth by reviewer.styles.csssaid Astryx'stailwind-theme.csslands "until Tailwind leaves" — that file's own header calls it a Tailwind Bridge, so it should never be imported at all. Slice 12 → Slice 13 corrected.One unrelated commit is needed for this branch to be green at all: #1778 removed the last caller of
readSessionsIpcSourcebut kept the export, soknip --workspace apps/desktophas been failing onmain.Refs #1565
Review round
Rebase note.
#1825deletedscripts/storybook-visual-smoke.test.mjsand made the runner's exports private, on the grounds that fake-unit coverage of a presentation harness tests the mock. That deletion is honoured here rather than re-litigated: therunJobstest added in this round is dropped and the export reverted. The real protection for that runner is the CI routing above, which makes a change to it re-run the 102 live renders it drives — which is the direction#1825is going in, not against it.#1795'sChatAttachmentChipsis likewise kept: it renders the four non-image attachment kinds inside a sent turn, a different branch fromImageThumbnails.Two independent reviews of the whole branch (one general agent, one Codex) converged on four defects. All four are fixed in the three commits on top.
Three cuts failed the rule they were made under.
THEME_PALETTESis the palette set a user picks in 设置 → 外观, the catalog pass renders wide/light/defaultonly, and no other story — including the Appearance page's own — shows the other ten. They had no oracle at all. The five remaining token stories still go, but for the honest reason:maka-tokens.cssis still the single authority and dies with Slice 13, so a table of var names is low review value with a short life, not a domain Astryx has already taken over.settings.spec.tschecks fields and persistence, not capture outcomes. It returns without the 120-lineMediaRecorderdecorator the original carried:permissionSnapshotalready reports microphonedeniedon darwin andrunCaptureSmokereturns on that snapshot before reachinggetUserMedia, so the real path needs no browser mocking at all. The catalog pass then failed the restored story on its first run, for exactly the reason the originals rotted — the result sits in one of six[role="status"]regions andquerySelectorhad picked an empty one.Pushed back on two:
ComposerPermissionModes(permission-mode-menu.tsxis explicit that the modes carry no distinct tone, andpermission-mode-surface.spec.tscovers the labels and states in the real app) andComposerPendingAndDisabled(the disabled half is already in app-shell'sWaitingForPermission; the unique remainder is a button label swap, which is not worth ~50 lines of restored scaffolding).The annotation guard did not hold the line it claimed. Five inputs passed or failed wrongly, each now covered by a test:
.stories.tsfiles were never opened thoughmain.tsloads them;export { Story }andexport async function Story()matched nothing and passed in silence; a wrappedexport const X: Story =was reported as malformed; the title regex took the file's firsttitle:rather than meta's, so a fixture literal above meta could exempt every story below it; an empty// Real path:counted.checkStorybookRootsclaimed to catch an added root and only checked for removed ones — it now checks both, and its happy-path test previously asserted[] deepEqual []against a function returningundefined.FIDELITY.mdno longer overclaims: this is a convention guard that must fail in seconds insidetypecheck, approximating a story set only Storybook's index truly knows. Every gap was closed by widening what it fails on.The smoke runner could stop guarding silently.
scripts/**sets onlyscriptMode, so editingstorybook-visual-smoke.mjsran its unit tests and never the run it drives — the one file most able to make the guard stop guarding was the one change the guard never saw. The e2e job's own scripts now route to it.Deferred, with reasons:
validateCoverageManifestmarks only 5 of 12 surfaces required (pre-existing manifest policy, not this change); a story rendering a non-empty but meaningless wrapper still passes the catalog pass (a documented limit of "does it mount", which is all the pass claims).Verification
npm run lint/format:checknpm run build/npm run typechecknpx knip(apps/desktop, packages/ui)main)node scripts/check-dead-css.mjs --checkmain)node scripts/check-story-annotations.mjsnode --test scripts/…build-storybooksmoke:storybookRun 30692460144 is green on an earlier revision of this branch, including the three new e2e steps (Chromium install 21s, Build Storybook 19s, smoke 32s). The catalog pass landed after it; locally the smoke went from 17s to 34s, so expect the e2e job to grow by roughly 90s total. Run 30695067250 is green on the full catalog cut. The branch has since been rebased onto latest
mainagain (30+ commits, including #1795 and thetest: collapseseries); CI on the new head is pending.Both new guards were verified negatively as well as positively, and both then caught real mistakes made while cutting the catalog:
validateCoverageManifestrejected a build after a regex with a shared name prefix removedExtensionsSkillsInstalledalong withExtensionsSkills.check-story-annotationsrejected the next one after a cleanup pass ate the// Real path:line aboveModels.Not run: the visual-contract and hit-test executables, which are deliberately not wired into CI.
Review focus
e2ejob rather than its own.e2eis becoming a slightly inaccurate name for "renderer verification". Splitting is cheap later if wall-clock becomes the constraint.