Skip to content

ci(storybook): report docs index changes on PRs - #5257

Merged
sauldom102 merged 1 commit into
mainfrom
claude/storybook-docs-ci-detection-75a4f5
Aug 26, 2026
Merged

ci(storybook): report docs index changes on PRs#5257
sauldom102 merged 1 commit into
mainfrom
claude/storybook-docs-ci-detection-75a4f5

Conversation

@sauldom102

@sauldom102 sauldom102 commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Description

Storybook's sidebar is assembled from a curated directory allowlist in .storybook/main.ts plus per-entry tags, which means a docs page can stop being exposed without anything turning red — move a component to a directory that isn't listed, drop autodocs, or add no-sidebar, and the page is simply gone. This snapshots the Storybook index on both sides of a PR and comments the difference, so losing a page is something a reviewer sees rather than something someone notices weeks later.

Type of change

  • Other: CI check (non-blocking, reports via PR comment)

What gets flagged, and how it reads

Every example below is real rendered output, produced by running this check against actual commits in this repo — not mock-ups. Each is labelled with the comparison that generated it.

1. A docs page disappeared

From #5095 at its pre-fix commit (9a6ac810, before fix(F0Select): restore canonical docs route) against its merge-base. That PR added tags: ["!autodocs"] plus an unattached MDX (<Meta title="Select/Overview" />), which moved the canonical Select docs page to a sub-route:

## ⚠️ Storybook pages lost (2)

🗑️ 2 removed · ➕ 7 added · ✏️ 43 updated

### 🗑️ Removed — gone from Storybook (2)

- `src/components/F0Select/__stories__/F0Select.stories.tsx`
  - [Components/Select](…/?path=/docs/components-select--documentation) — **docs page gone while its stories remain**: `autodocs` was probably dropped (or `!autodocs` added)
  - [Components/Select › Snapshot](…/?path=/story/components-select--snapshot) — was `story`

### ➕ Added (7)

1 new docs page(s) and 6 new story/stories.

- `src/components/F0Select/__stories__/F0Select.mdx`
  - [Components/Select/Overview](…/?path=/docs/components-select-overview--documentation)

The diagnosis on that first bullet is the literal root cause, and the misplaced replacement sits a few lines below it under Added — which is what makes the wrong-route conclusion quick to reach.

2. A page is still indexed but no longer reachable

From main against main~150. These stories gained no-sidebar, so their tests kept passing while no human could find them:

### 🙈 No longer in the sidebar (5)

Still indexed — so tests keep running — but not reachable by a human.

- [Patterns/Navigation/Sidebar/ChatList › Cascade Loading](…) — gained the `no-sidebar` tag · `src/patterns/Navigation/Sidebar/Chats/index.stories.tsx`
- [Patterns/Navigation/Sidebar/ChatList › Empty](…) — gained the `no-sidebar` tag · `src/patterns/Navigation/Sidebar/Chats/index.stories.tsx`
- [Patterns/Navigation/Sidebar/ChatList › Live Updates](…) — gained the `no-sidebar` tag · `src/patterns/Navigation/Sidebar/Chats/index.stories.tsx`

3. A page survives but something else generates it now

From #5095 at its current head (d9c11db7, after the fix) against its merge-base. The page's URL is intact, so an id-only diff sees nothing — only the source file changed:

### ♻️ Docs source replaced (1)

The page kept its URL, but something else generates it now. Attaching an `.mdx` file to a
component **silently takes over** its auto-generated `autodocs` page: the props table and the
story previews that page used to show are gone unless the new file renders them itself. Check
that nothing documented on `main` was dropped.

- [Components/Select](…/?path=/docs/components-select--documentation) — auto-generated → hand-written MDX
  - was `src/components/F0Select/__stories__/F0Select.stories.tsx`
  - now `src/components/F0Select/__stories__/F0Select.mdx`

This one is worth reading closely, because it is a live example of real content loss: that MDX renders 3 story previews where autodocs rendered 45, and it has no <Description />, so the component's description no longer appears. See the known limitation below.

4. Informational categories, collapsed by default

A directory rename moves every story in it, so these group by file pair rather than listing 16 findings. Same mechanism on both sides, URLs unchanged, so it does not raise the warning heading. From main against main~150:

<details><summary>🚚 Source file moved — 16 entries across 2 file(s)</summary>

- `src/patterns/ResourceHeader/index.stories.tsx` → `src/patterns/F0ResourceHeader/index.stories.tsx`
  - [Patterns/Resource header › Company Header](…)
  - [Patterns/Resource header › Default](…)

✏️ Updated, 🏷️ Maturity changed and 🔇 Added but not in the sidebar are collapsed the same way.

5. Nothing lost

The state this PR itself is in — see the live comment below:

## ✅ Storybook docs — no pages lost

Every page reachable on `main` is still reachable here.

Reported on every run rather than staying silent, so a previous ⚠️ resolves back to ✅ once fixed instead of leaving a stale warning.

The one-line summary

The counts line is always complete even when a section's list is capped:

🗑️ 10 removed · 🙈 5 hidden · ♻️ 2 source replaced · 🚚 16 source moved · ➕ 106 added · ✏️ 422 updated

Known limitation

Case 3 tells a reviewer where to look, not how much was lost. The index records that a page exists and which file backs it, never which stories that file renders — so the check cannot currently say "3 previews where autodocs rendered 45". Closing that means parsing the MDX for <Canvas of={…}> / <Stories /> and comparing against the stories the autodocs page covered. Deliberately left out of this PR; #5095 is a ready-made regression test for it.

The check also verifies a page is indexed and reachable, not that it renders. A page whose MDX throws at runtime would pass.

Implementation details

  • ci: add Storybook Docs Index workflow — snapshot the index for the PR and for main in parallel, diff them, comment the result

    Why this is cheap enough to run on every PR

    storybook index only runs the indexers — no Vite, no bundling, and (verified) no f0-core build. A full snapshot of ~2,750 entries takes about 6 seconds, so the job cost is essentially pnpm install. This does not build Storybook. Measured in CI on this PR: 1m8s and 1m15s for the two snapshot legs in parallel, 1m16s for the diff.

  • feat: add check-docs-index.ts — normalizes storybook index output into a snapshot (identity, sidebar visibility, source-file hash) and classifies the diff

  • feat: report a page that is still indexed but no longer reaches the sidebar

    Why this needs its own category

    An entry reaches the sidebar only if it is tagged dev and not tagged no-sidebar (this repo's filter, applied in .storybook/manager.ts). Losing either leaves the entry in the index — so its tests keep passing and nothing looks broken — while no human can find the page. Presence alone can't detect this, so visibility is tracked per entry.

  • feat: detect a docs page whose source was replaced rather than edited

    The case an id-only diff cannot see

    A docs page id is a single slot that two mechanisms compete for: autodocs generates the page, and an .mdx attached to the same component replaces it. Adding an MDX file therefore silently overwrites the auto-generated page — same id, same title, same URL, entirely different content. Nothing is added and nothing is removed, so a diff keyed on ids sees no change at all. Comparing each entry's importPath across both sides is what catches it.

    A same-kind relocation (ResourceHeaderF0ResourceHeader) is classified separately and does not raise the warning.

  • feat: name a dropped autodocs tag as the likely cause when a docs page vanishes

    How the cause is inferred

    Verified against the real index by temporarily setting !autodocs on F0ActionBar: the --documentation entry disappears from the index outright, so it already surfaces as a removal. What the entry can't say is why — a deleted component takes its stories with it, whereas a dropped tag leaves them behind. That second shape is flagged explicitly, and it fired correctly on feat(F0Select): add inline variant #5095, whose root cause was exactly an autodocs!autodocs change.

  • fix: compare against the merge commit's first parent, not the branch point

    Why

    Both sides derive from the PR merge commit, so the PR is synced with main before either snapshot is taken. Snapshotting the branch as-is would report every page that landed on main after the branch point as removed. Same approach as the Public API Surface check.

  • fix: normalize both sides with the same copy of the script

    Why

    The base leg restores check-docs-index.ts from the merge commit before running. Otherwise the base side would use whatever version exists on main — or none at all, on this PR — and any change to the tool would masquerade as a change to the docs. Confirmed working in CI on this PR, which is the bootstrap case.

  • ci: retarget the comment's links at the PR's Storybook once Chromatic publishes one

    How, and why not a computed permalink

    This check finishes in about a minute; a Chromatic build takes several. So the comment is posted immediately against the public Storybook, and chromatic.yml rewrites every link in place once a build for the PR exists. The comment records its own link base in an HTML comment, so nothing has to be carried between workflow runs — no artifacts, no workflow_run plumbing.

    Computing the Chromatic permalink from the branch name was rejected: branch slugs are truncated at 37 characters and collapse repeated dashes, and this repo's branch names exceed that. The published URL also turns out to be <appId>-<buildHash>.chromatic.com, a different shape from the documented branch permalink — so a computed URL would have pointed somewhere else entirely. The URL comes from the Chromatic action's own output instead.

    Best-effort by design. If Chromatic wins the race, the comment keeps its public links and self-corrects on the next push.

  • chore: add check:docs-index script for running the comparison locally

Non-blocking on purpose

Losing a page does not fail the gate. Deleting a page is often deliberate, and a check that blocked merges on it would be routinely overridden until it was ignored. It reports through a PR comment and a workflow warning annotation.

Verification

  • 38 unit tests covering each classification, the comment rendering, and the retargeting contract
  • Validated against real indexes rather than fixtures: HEAD vs main correctly reports no changes; main vs main~150 surfaces real losses, hidden pages and overwrites; and feat(F0Select): add inline variant #5095's pre-fix commit reproduces the exact regression it was opened to fix
  • The Chromatic rewrite script was extracted from the YAML and run against real rendered output: all links retargeted, no public links left, and idempotent on re-run, missing comment, and older comment formats. Confirmed in CI — Retargeted docs comment links at https://66a7a8d7d124220c363457cc-dzcelszubf.chromatic.com
  • Because this PR touches packages/react, its own check runs on it — the comment below is this tooling reporting on itself

🤖 Generated with Claude Code

Storybook's sidebar comes from a curated directory allowlist in
.storybook/main.ts plus per-entry tags, so a docs page can go missing
without anything turning red: move a component to an unlisted directory,
drop `autodocs`, or add `no-sidebar`, and the page is simply gone.

Snapshot the Storybook index on both sides of a PR and comment the diff.
`storybook index` only runs the indexers -- no Vite, no bundling, no core
build -- so a snapshot takes ~6s and this can run on every PR.

Two failure modes get named explicitly:

- A page still indexed but no longer reaching the sidebar (it lost `dev`
  or gained `no-sidebar`). Tests keep passing; nobody can find the page.
- A docs page id is one slot that two mechanisms compete for. Attaching
  an .mdx to a component silently replaces its autodocs page: same id,
  same title, same URL, different content, props table and story
  previews gone. Nothing is added or removed, so an id-only diff sees no
  change at all -- comparing each entry's importPath is what catches it.

Comparison runs against the PR merge commit's first parent, so pages
that landed on main after the branch point are not reported as removed.
Both sides are normalized by the same copy of the script, restored onto
the base checkout, so a change to the tool cannot masquerade as a change
to the docs.

Non-blocking: deleting a page is often deliberate, and a gate that
blocked merges on it would be routinely overridden until ignored. It
reports via a PR comment and a workflow warning annotation instead.

The check finishes long before a Chromatic build exists, so links start
out pointing at the public Storybook and chromatic.yml rewrites them in
place once a build for the PR is published -- the comment records its own
link base, so nothing has to be carried between workflow runs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sauldom102
sauldom102 requested a review from a team as a code owner August 26, 2026 10:00
@github-actions github-actions Bot added ci react Changes affect packages/react labels Aug 26, 2026
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

🔍 Review policy: Code change

Default rule: any other change needs one approval from f0-devs (rule 4).

Required approvals

Team Why Status
@factorialco/f0-devs Every code change needs a dev approval ✅ approved by @developerdanx
How this was decided
  • PRs touching only sds/ modules require their owners and nothing else.
  • Otherwise, docs-only changes (*.md, *.mdx, *.stories.tsx, anything in __stories__/) → one f0-general approval.
  • Otherwise, feat: titles → one f0-devs and one f0-designers approval. Not a feature? Fix the title prefix.
  • Anything else → one f0-devs approval.
  • Add the needs-design-review label to also request a design approval on any PR.
  • Creating a new sds/ module (new package.yml) additionally requires an f0-general approval.

Policy source: ownership/review-policy.ts · Team members: ownership/teams.yml

@github-actions

Copy link
Copy Markdown
Contributor

✅ No New Circular Dependencies

No new circular dependencies detected. Current count: 0

@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

✅ Storybook docs — no pages lost

Every page reachable on main is still reachable here.

Links point at this PR's Storybook build — browse the full Storybook.

Snapshot of the Storybook index (docs pages + stories) compared against main. Non-blocking.

@github-actions

Copy link
Copy Markdown
Contributor

📦 Alpha Package Version Published

Use pnpm i github:factorialco/f0#npm/alpha-pr-5257 to install the package

Use pnpm i github:factorialco/f0#6317335b3e059eb330873ad3fa5103f310f4630a to install this specific commit

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Visual review for your branch is published 🔍

Here are the links to:

@github-actions

Copy link
Copy Markdown
Contributor

♿ Accessibility (axe) — components changed in this PR

✅ No a11y issues in the stories this PR changed.

Scope: only stories in the files/component folders this PR changed. It can't yet flag downstream ripple from shared-code/token changes, or diff against main (planned: base-vs-head delta).

@github-actions

Copy link
Copy Markdown
Contributor

✅ No breaking public API changes

No public exports were removed, renamed, or had existing props/types changed in a breaking way compared to main.

Comparing f0, experimental and ai against main. Adding components, types, or optional props is safe. This check is non-blocking.

⚠️ Could not analyze component-status (no-base) — a build may have failed; results may be incomplete.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report for packages/react

Status Category Percentage Covered / Total
🔵 Lines 68.86% 28991 / 42099
🔵 Statements 67.82% 30669 / 45218
🔵 Functions 61.66% 6914 / 11212
🔵 Branches 61.47% 21687 / 35278
File CoverageNo changed files found.
Generated in workflow #17508 for commit 222870e by the Vitest Coverage Report Action

@sauldom102
sauldom102 merged commit 9e3a979 into main Aug 26, 2026
57 checks passed
@sauldom102
sauldom102 deleted the claude/storybook-docs-ci-detection-75a4f5 branch August 26, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci react Changes affect packages/react

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants