Skip to content

build(storybook): typecheck stories + export coverage guard - #482

Merged
Astro-Han merged 7 commits into
mainfrom
opencode/storybook-drift-guards
Jul 3, 2026
Merged

build(storybook): typecheck stories + export coverage guard#482
Astro-Han merged 7 commits into
mainfrom
opencode/storybook-drift-guards

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Add two drift-prevention guards so Storybook stories stay in sync with production components: type-check stories with tsc, and a contract test asserting a curated set of primitive components has JSX usage in at least one story.

Why

Story files were excluded from the @maka/ui tsc build and never type-checked. build-storybook uses Vite, which strips types without checking them. This meant a component could change its props and the story would silently keep using the old API. There was also no guard against a curated component losing its story coverage.

Scope

Changed:

  • apps/desktop/tsconfig.storybook.json (new) — a dedicated tsconfig that includes only story/config entries (.storybook, stories, packages/ui/stories, src/global.d.ts) with paths mapping for workspace packages. Narrow enough that main-process type errors do not surface; broad enough to cover .storybook/preview.tsx.
  • apps/desktop/package.json — added typecheck:stories script; wired storybook tsconfig into the existing typecheck script.
  • apps/desktop/src/main/__tests__/storybook-baseline-contract.test.ts — new contract test: scans all story files and asserts each curated primitive component has JSX tag usage (<Name) in at least one story. Named "curated baseline" to be honest that it is not an exhaustive export check; typecheck:stories is the primary drift guard.
  • packages/ui/stories/toggle.stories.tsx — fixed real drift: toggleMultiple -> multiple (Base UI renamed the prop; story still used the old name).
  • apps/desktop/stories/settings/settings-pages.stories.tsx — fixed real drift: UsageSummary fixture fields updated to match the current interface; mergeSettings replaces shallow spread + type assertion so the story bridge mock matches production IPC behavior.
  • apps/desktop/stories/onboarding.stories.tsx — fixed type errors: stop mutating readonly AppSettings fields via spread-based fixture construction.
  • apps/desktop/stories/maka-bridge.tsx — fixed type errors: narrow the window cast so delete-on-optional and MakaGlobal assignability errors go away.

Not included:

  • eslint-plugin-storybook (project has no eslint infrastructure; would require introducing eslint + flat config, out of scope).
  • @storybook/addon-vitest (requires Playwright chromium in CI; deferred to a future PR).
  • @storybook/addon-a11y (deferred; recommend adopting with test: 'todo' first).
  • Visual regression tooling (deferred; project has screenshot harness for the Electron app).
  • Exhaustive export coverage (every export * component). The curated baseline covers 24 primitive components; ChoiceCard, InputGroup, Item, SettingsSelect, SettingsSwitch are public exports without standalone stories but are out of scope for this PR.

Verification

  • npm run -w @maka/desktop typecheck passes (includes the new storybook tsconfig).
  • npm run -w @maka/desktop build-storybook passes.
  • npm run -w @maka/desktop test passes (1712 tests green).
  • Verified tsconfig isolation: a type error in .storybook/preview.tsx turns typecheck:stories red; a type error in src/main/main.ts does not.
  • Verified export coverage test: deleting toolbar.stories.tsx turns the test red (Toolbar missing); replacing <Toolbar with a comment-only reference also turns it red (JSX match required, not word boundary).

User-facing impact

None. Storybook tooling and test-only changes; no runtime or build output changes. The story fixture fixes (toggle, settings, onboarding) correct stale mock data but do not affect the rendered app.

Reviewer notes

  • The typecheck caught three real drift issues on first run: toggleMultiple -> multiple (Base UI prop rename), UsageSummary fields (interface changed), readonly AppSettings mutation. These were silent because Vite strips types.
  • The export coverage test uses a curated allowlist of 24 primitive component names and matches JSX tags (<Name), not word boundaries, so comments and unused imports do not count as coverage. Component names match actual JSX tags (DialogRoot, TabsRoot, SelectRoot) rather than export names.
  • tsconfig.storybook.json uses paths to resolve @maka/ui, @maka/core, etc. to source entry points, so tsc follows imports without including entire package source trees.
  • mergeSettings is used in the settings story bridge mock to match the real IPC handler's deep-merge behavior, replacing a shallow spread + as assertion that could return settings with undefined nested fields.

Add tsconfig.storybook.json and wire typecheck:stories into the
desktop typecheck script so tsc --noEmit covers story files.
Story files were previously excluded from the @maka/ui tsc build
and never type-checked: build-storybook uses Vite which strips
types without checking. This meant a component could change its
props and the story would silently keep using the old API.
Fix three drift issues caught by the new typecheck:
- toggle.stories.tsx: toggleMultiple -> multiple (Base UI renamed
the prop; the story still used the old name)
- settings-pages.stories.tsx: UsageSummary fixture fields updated
to match the current interface (range/fromMs/toMs/requestCount/
errorCount -> totalRequests/totalCostUsd/totalTokens/etc)
- onboarding.stories.tsx: stop mutating readonly AppSettings fields
via spread-based fixture construction
- maka-bridge.tsx: narrow the window cast so delete-on-optional
and MakaGlobal assignability errors go away
Add a contract test that scans all story files and asserts each
public UI component name (Button, Badge, Input, ..., Toolbar,
ToastProvider) appears in at least one story. Catches the 'new
component, no story' drift class without manual grep audits.
Verified red/green: deleting toolbar.stories.tsx turns the test red
(Toolbar missing), restoring it turns green.
…gs story
Replace { ...createDefaultSettings(), ...patch } as ReturnType<...>
with mergeSettings(createDefaultSettings(), patch) in the settings
story bridge mock. The real IPC handler uses mergeSettings for deep
merge and normalization of nested settings (webSearch, localMemory,
botChat). The shallow spread + type assertion could return settings
with undefined nested fields, diverging from production behavior.
The previous tsconfig included entire package source trees
(packages/ui/src, packages/core/src, etc), causing typecheck:stories
to surface errors in main-process files that stories never import.
It also missed .storybook/ (main.ts, preview.tsx).
Narrow include to: .storybook, stories, src/global.d.ts, and
packages/ui/stories. Add paths mapping so @maka/ui, @maka/core, etc
resolve to source entry points without including full source trees.
Verified: a type error in .storybook/preview.tsx turns
typecheck:stories red; a type error in src/main/main.ts does not.
Review feedback: the test claimed 'every public UI component export'
but used a hand-written allowlist and \bName\b regex that matched
comments and unused imports as 'coverage'.
- Rename to 'covers curated primitive components with stories' and
add a message clarifying this is a curated baseline, not an
exhaustive export check; typecheck:stories is the primary guard.
- Match '<Name' (JSX tag) instead of \bName\b so only real JSX
usage counts as coverage, not comments or import-only references.
- Fix component names to match actual JSX tags: Dialog -> DialogRoot,
Tabs -> TabsRoot, Select -> SelectRoot (stories use the Root
variants).
@Astro-Han
Astro-Hanforce-pushed the opencode/storybook-drift-guards branch from 631ad9b to d95b53cCompareJuly 3, 2026 12:17
When no maka property existed before the bridge was installed,
cleanup set it to undefined instead of deleting it, leaving a
visible 'maka' key with value undefined for subsequent bridgeless
stories. Record hadPrevious and delete the key when it was absent.
The <Name regex is a text scan, not a JSX AST check: comments or
strings containing the tag pattern would be false positives. Rename
the test and message to 'textual smoke check' so the promise
matches the implementation.
@Astro-Han
Astro-Han merged commit 2bff307 into mainJul 3, 2026
Astro-Han added a commit that referenced this pull request Jul 3, 2026
5 CSS files (daily-review, health-center, permission-center,
settings/connection, tool-stream) took main's format (@layer
removal, indentation) and re-applied foreground alias replacement
(40/50→muted-foreground, 60/70/80→foreground-secondary).
@Astro-Han
Astro-Han deleted the opencode/storybook-drift-guards branch July 14, 2026 05:05
Sign up for freeto 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

@Astro-Han