Skip to content

refactor(storybook): rationalize sidebar taxonomy and prune variants#1395

Open
danyelf wants to merge 3 commits into
mainfrom
worktree-rationalize-storyobok
Open

refactor(storybook): rationalize sidebar taxonomy and prune variants#1395
danyelf wants to merge 3 commits into
mainfrom
worktree-rationalize-storyobok

Conversation

@danyelf
Copy link
Copy Markdown
Contributor

@danyelf danyelf commented May 22, 2026

PR checklist

  • Ensure you have added or ran the appropriate tests for your PR.
  • DCO signed

What type of PR is this?

refactor — storybook-only; no product code or user-facing behavior changes.

What this PR does / why we need it:

Storybook had grown to 16 top-level folders and ~428 stories, with two competing taxonomies (Visualizations/* vs top-level Schema/, Editor/, Lineage/), singleton folders for App/Editor/Schema/Summary/Timeline/Data, and a large variant explosion across primitives (EmptyState had 17, Toaster had 15, SquareIcon had 13 — most just demonstrating prop values already covered by argType controls). Finding features in the sidebar was hard.

This PR reorganizes the sidebar into 4 top-level buckets and prunes variant-explosion files down to a Default (with controls) plus the compositions that actually demonstrate something controls can't show.

Final sidebar shape:

Checks/      CheckList, CheckDetail, OutdatedCheckIndicator,
             TimelineEvent, ChangeSummary, EnvInfo
Diffs/
  Charts/    TopKBarChart
  Results/   Schema, RowCount, Profile, Query, ValueDiff,
             Histogram, TopK, SqlEditor, DataGrid
Lineage/
  LineageCanvas/   (LineageCanvas, CllExperience, ScaleTests/*)
  LineageLegend
  NodeView/        (NodeView, LineageTab)
Primitives/  (all formerly-split DiffPrimitives folded back in)

Notable prunes (16 → 3 pattern, with controls + compositions):

Component Before After
SquareIcon 13 3
DataTypeIcon 12 2 (kept AllCategories table)
ScreenshotBox 9 2
Split 12 4
DropdownValuesInput 16 3
ExternalLinkConfirmDialog 13 5 (3 are play tests)
MarkdownContent 18 2
EmptyState 17 5
Toaster 15 3
DiffText 16 3
RunStatusBadge 12 3
TimelineEvent 18 15 (constrained by visual.ts + .test.tsx)
EnvInfo 10 7

Other cleanup:

  • Dropped tags: ["autodocs"] from every story file. The autodocs page duplicated the canvas and slowed Vite cold-start.
  • Deleted HistogramDiffForm + TopKDiffForm stories — broken (require a LineageGraphContext the storybook mocks don't satisfy).
  • Renamed HistogramResultView.stories.tsxHistogramDiffResultView.stories.tsx (file now matches the exported component).
  • Rewrote the docs-page smoke loop in verify-ui-stories.visual.ts (would have broken when autodocs was removed) to load story IDs directly.

Net: 428 → 244 stories.

Which issue(s) this PR fixes: N/A

Special notes for your reviewer:

  • TimelineEvent kept 15 stories (not the originally-proposed 9) because 7 of them are pinned by TimelineEvent.visual.ts (Playwright visual regression) and 6 more are consumed by TimelineEvent.test.tsx via composeStories() as unit-test fixtures.
  • The 12 errors that surface under pnpm --filter @datarecce/storybook exec tsc are MUI-augmentation drift, not regressions — they exist on main and are fixed by a possible follow-up tsconfig PR. Canonical cd js && pnpm type:check passes clean.

Does this PR introduce a user-facing change?:

NONE

image

danyelf and others added 2 commits May 21, 2026 18:07
Reorganize storybook into 4 top-level buckets and prune variant
explosions across primitive components.

Sidebar shape:
- Diffs/{Charts, Results}/<check-type> (Forms bucket removed)
- Lineage/LineageCanvas/{CllExperience, ScaleTests}/...
- Lineage/NodeView/{NodeView, LineageTab}
- Checks/* absorbs former App, Editor, Summary, Timeline singletons
- Primitives/* (DiffPrimitives split reverted; merged back)

Story prunes (rely on argType controls instead of per-variant stories):
- SquareIcon 13->3, DataTypeIcon 12->2 (keep AllCategories table)
- ScreenshotBox 9->2, Split 12->4, DropdownValuesInput 16->3
- ExternalLinkConfirmDialog 13->5, MarkdownContent 18->2
- EmptyState 17->5, Toaster 15->3
- DiffText 16->3, RunStatusBadge 12->3
- TimelineEvent 18->9 (constrained by visual.ts regression test IDs)
- EnvInfo 10->7

Drop tags: ["autodocs"] from all stories. The per-component Docs page
duplicated the canvas, amplified Vite cold-start cost, and the
docs-loading smoke loop in verify-ui-stories.visual.ts is replaced by
direct story-ID smoke tests.

Delete HistogramDiffForm + TopKDiffForm stories (broken in storybook —
require a LineageGraphContext that the mocks don't fully satisfy).
Rename HistogramResultView -> HistogramDiffResultView (filename now
matches the exported component).

Net: 428 -> 244 stories.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Danyel Fisher <danyel@gmail.com>
The earlier prune dropped DescriptionChanged, NameChanged, PresetApplied,
CommentFromOtherUser, ActorWithoutFullname, and ActorWithoutName, but
TimelineEvent.test.tsx imports them via composeStories() as test
fixtures. Restoring the six story exports so the unit tests compile and
keep their behavioral coverage of event-type rendering and actor
fallbacks.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Danyel Fisher <danyel@gmail.com>
@danyelf danyelf requested a review from gcko May 22, 2026 20:46
@danyelf danyelf self-assigned this May 22, 2026
@danyelf
Copy link
Copy Markdown
Contributor Author

danyelf commented May 22, 2026

Responses to a self-review:

  1. HistogramDiffForm + TopKDiffForm deletion (the only real signal). Both are production components, exported from @datarecce/ui/components and registered in run/registry.ts:29,42,188. The PR justifies deletion as "the storybook mocks don't satisfy LineageGraphContext." ... Deleting working coverage for two production form components on a "probably broken" hunch is the wrong direction.

The page did not render -- it showed an internal error that the LineageGraphContext mock wasn't working. I'd add ot that the forms themselves are barely load-bearing; they're not the pieces I'm most worried about inspecting.

  1. verify-ui-stories.visual.ts docs-page loop removed. The pre-PR loop iterated 14 UI components and asserted no console/page errors on /docs/.... The replacement (5
    hand-picked smoke tests) loses passive error detection on 9 components. Commit 22b472c originally added this loop on purpose. If tags:["autodocs"] removal makes the
    docs URL meaningless, fine — but the loop could have been adapted to iframe URLs instead of deleted. Either restore it (rewritten for iframes) or note the deliberate
    coverage drop in the PR body.

Because we've had many broken stories in the test without actually turning this red, I'm not sure that it was doing what we wanted it to. I'm ok restoring it (and the autodocs), but I'd like to do so with intent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant