feat(ui): D.0 foundation for Live 12 restyle — primitives + Storybook - #64
Conversation
PR 1 of 16 in the UI consistency overhaul (plan at /root/.claude/plans/can-we-plan-a-fancy-dove.md). Lays the foundation for migrating consumers; no existing component is touched in this PR. Design tokens (apps/ui/src/index.css): - Append @theme block with device-chrome shadows, gradients (title-strip, device-face, rail), LED palette, focus rings, semantic spacing scale, type-scale lock, signal-cable colors. - Add @layer components block with .device-rack (+ status modifiers), .device-rack__title-strip / __body / __rail, .led-indicator (+ pulse keyframes), .signal-cable (+ animated variant), .signal-arrow, .parameter-display, .tabular-mono. Primitive library (apps/ui/src/components/ui/): - cn.ts (twMerge + clsx), variants.ts (Tone/Status/Size), index.ts barrel. - Button (cva variants: primary/secondary/ghost/danger/link × sm/md/lg, iconOnly, ledIndicator). - Pill (tone × variant × size with compoundVariants; leadingDot; drops the legacy info/violet/blue/teal tones). - Panel (rack/surface/ghost/inset × padding × tone — composes the new .device-rack chrome). - SectionHeader (consumes getTextRoleClassName from utils/displayText and propagates data-text-role to the heading element — keeps the brittle assertions in tests/services/analysisResultsUi.test.ts green). - LedIndicator (status × size; optional label). - Tooltip (Radix-backed, preserves the existing `text` prop API as a drop-in for components/Tooltip.tsx). - Checkbox (Radix-backed, visually a Live device on/off pad). Storybook + Chromatic: - .storybook/main.ts uses @storybook/react-vite and reapplies the Tailwind v4 plugin via viteFinal so @theme tokens resolve. - .storybook/preview.tsx imports src/index.css, wires the dark background palette, and mounts TooltipProvider once. - Stories cover every primitive with variant matrices. - package.json scripts: storybook, build-storybook, chromatic (intentionally outside `npm run verify`). - .github/workflows/chromatic.yml runs visual regression on PRs touching apps/ui; non-blocking (exit-zero-on-changes); requires CHROMATIC_PROJECT_TOKEN secret. Dependencies: - runtime: class-variance-authority, tailwind-merge, clsx, @radix-ui/react-{tooltip,dialog,popover,checkbox,slider,scroll-area} - dev: storybook (v10), @storybook/react-vite, @storybook/addon-a11y, chromatic Pre-flight decisions locked per plan §1: native selects (not Radix Select), Radix Checkbox (not Switch), ParameterKnob deferred, Chromatic free tier for visual regression. Verified: - npm run lint: clean - npm run test:unit: 605/605 passing - npm run build: clean (89.5 KB CSS, 14.5 KB gzipped) - npm run build-storybook: clean; new tokens + utility classes confirmed present in storybook-static CSS output. - npm run test:smoke: not run locally (Playwright browser download blocked by sandbox network policy); existing components untouched so smoke tests should pass in CI. https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
The Chromatic check on PR #64 failed in 11s with non-zero exit from chromaui/action because the CHROMATIC_PROJECT_TOKEN secret has not been configured yet. Add a precheck step that detects the missing token and skips the publish (emitting a notice annotation explaining how to enable it) so the workflow stays green until the user wires up the Chromatic project. https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
slittycode
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES (posted as COMMENT — GitHub blocks self-review)
Summary
Pure UI/infra foundation PR — design tokens, 7 CVA-based primitives, Storybook + Chromatic wiring. Zero backend or Phase 1/2/3 data-flow changes; the non-goals list is accurate and respected. Tests: 605/605 passing, TypeScript clean. The infra (Storybook pipeline, Tailwind v4 viteFinal workaround, CI workflow) is well-structured. There are three bugs that should be fixed before D.1 builds on top of this foundation.
Findings
Should fix
SectionHeader.tsx:83 — 'accent' tone renders as an unlit LED (every default header is broken)
The mapping ledTone === 'neutral' ? 'idle' : (ledTone as 'active') is a type cast, not a value transform. When ledTone is 'accent' (the default), the runtime string 'accent' lands on LedIndicator's status prop. LedIndicator has no led-indicator--accent CSS class — CVA silently drops unrecognised variants — so the LED renders as the idle gray dot. Every SectionHeader without an explicit ledTone prop ships with a gray LED instead of the accent orange. Fix:
ledTone === 'neutral' ? 'idle'
: ledTone === 'accent' ? 'active'
: ledTone as 'success' | 'warning' | 'error'Checkbox.tsx:49+58 — indeterminate state renders with checked styling
isChecked = checked ?? defaultChecked ?? false. When checked='indeterminate' (a valid Radix CheckedState), Boolean('indeterminate') is true, so checkboxVariants applies the checked border/glow/shadow. The Controlled story explicitly uses useState<boolean | 'indeterminate'>, so this is a reachable case. Fix: checked === true rather than Boolean(isChecked), or guard indeterminate explicitly.
Worth considering
Panel.tsx:20-26 — device-rack--* tone classes emitted for non-rack variants
The device-rack--active/success/warning/error rules only have effect under a .device-rack parent. On variant="surface" or "ghost" with an active tone, the class is appended but inert. The API implies tone works across variants; it only does for rack. Compound variants would constrain this and make the intent explicit.
Button.tsx:94-95 — redundant double branch simplifies to {children}
{!iconOnly && children}
{iconOnly && children}Mutually exclusive, together always render children. The iconOnly flag correctly affects padding via compound variants; it just doesn't suppress text children as someone reading this might expect. Not a functional bug, but will confuse the next reader.
Test results
605 / 605 passing. 0 failures. TypeScript clean.
No Vitest coverage for the new components — but in this codebase's node-only Vitest environment, pure React components can't be rendered without jsdom. Storybook + Chromatic handles visual correctness. Acceptable for layout primitives.
Phase boundary check
Clean. No changes to src/services/, src/types/, analyze.py, server.py, analysis_runtime.py, or any Phase 1/2/3 data contract. The diff does exactly what the description says.
Generated by Claude Code
Four issues flagged in the review on PR #64. 1. SectionHeader.tsx: the default ledTone='accent' silently rendered as an unlit gray LED. The previous `ledTone === 'neutral' ? 'idle' : (ledTone as 'active')` was a type cast, not a value transform, so the runtime string 'accent' was passed to LedIndicator.status which has no matching CSS class. Extract a toneToLedStatus() helper that maps Tone -> LED status explicitly. 2. Checkbox.tsx: Boolean(isChecked) coerced 'indeterminate' to true, so the indeterminate state shipped with the lit accent border. Use a strict `=== true` check for the lit style and render a horizontal bar instead of a checkmark when indeterminate. 3. Panel.tsx: device-rack--* tone classes were emitted for all variants but only have CSS effect under .device-rack. Move tone styling into compoundVariants so non-rack variants (surface, ghost, inset) get a border-tint instead of an inert class. 4. Button.tsx: drop the mutually-exclusive `{!iconOnly && children}{iconOnly && children}` pair — together they always render children, and the implication that iconOnly suppresses text is misleading. Verified: lint clean, 605/605 unit tests passing, build clean. https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
PR 14 of 16 in the UI consistency overhaul. Final cleanup pass. Removes three CSS utility classes from index.css: - .ableton-panel - .ableton-device - .ableton-header All three were retired by D.* migrations — every former consumer now uses the canonical primitives from components/ui/ (Panel, DeviceRack, SectionHeader). `grep -rn` against apps/ui/src/ confirms zero remaining usages. .ableton-shell and .ableton-toolbar STAY because App.tsx:952,956 still consumes them for the outer app chrome (the load-bearing section-grid shell). A focused follow-up could migrate those two to DeviceRack / a future AppShell primitive; out of scope here. Deferred (preserved as-is): - components/MeasurementPrimitives.tsx cannot be fully deleted yet — 5 consumers still depend on timeline/lane primitives (LaneContainer, LaneRow, StatsBar, TimeRuler) that the plan explicitly preserved. The shipped primitives (StatusBadge, DeltaBadge, MetricBar, MetricBarRow, OutlinePillButton, TokenBadgeList, StyledDataTable, AccentMetricCard) also still have consumers in MeasurementDashboard and AnalysisResults sections that weren't migrated (StructureLanes, HarmonyLanes, AnalysisResults remaining sections). Keep the file intact. - components/Tooltip.tsx stays as a re-export shim; the file isn't causing any drift and external imports may still target it. Verified: lint clean, 628/628 unit tests passing, build clean. D.* migrations summary (all merged or open at time of this PR): - D.0 Foundation (#64) ✓ - D.1 Primitives complete (#65) ✓ - D.2 SamplePlayback (#66) ✓ - D.3 SignalChain marquee (#67) ✓ - D.4 FileUpload + Input Source (#69) ✓ - D.5a Results header + metric strip (#71) ✓ - D.5b Mix Chain (#73) ✓ - D.5c Patches (#74) ✓ - D.5d Section headers (#75) ✓ - D.6a Dashboard structure (#76) ✓ - D.6b Dashboard sections (#77) ✓ - D.6c Dashboard headers (#78) ✓ - D.7 SessionMusicianPanel + MixDoctorPanel (#79) — open - D.8 Signal Monitor + SpectrogramViewer drift (#80) — open - D.9 DenseDawConcept drift (#81) — open - D.10 Cleanup (this PR) ✓ https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
…82) PR 14 of 16 in the UI consistency overhaul. Final cleanup pass. Removes three CSS utility classes from index.css: - .ableton-panel - .ableton-device - .ableton-header All three were retired by D.* migrations — every former consumer now uses the canonical primitives from components/ui/ (Panel, DeviceRack, SectionHeader). `grep -rn` against apps/ui/src/ confirms zero remaining usages. .ableton-shell and .ableton-toolbar STAY because App.tsx:952,956 still consumes them for the outer app chrome (the load-bearing section-grid shell). A focused follow-up could migrate those two to DeviceRack / a future AppShell primitive; out of scope here. Deferred (preserved as-is): - components/MeasurementPrimitives.tsx cannot be fully deleted yet — 5 consumers still depend on timeline/lane primitives (LaneContainer, LaneRow, StatsBar, TimeRuler) that the plan explicitly preserved. The shipped primitives (StatusBadge, DeltaBadge, MetricBar, MetricBarRow, OutlinePillButton, TokenBadgeList, StyledDataTable, AccentMetricCard) also still have consumers in MeasurementDashboard and AnalysisResults sections that weren't migrated (StructureLanes, HarmonyLanes, AnalysisResults remaining sections). Keep the file intact. - components/Tooltip.tsx stays as a re-export shim; the file isn't causing any drift and external imports may still target it. Verified: lint clean, 628/628 unit tests passing, build clean. D.* migrations summary (all merged or open at time of this PR): - D.0 Foundation (#64) ✓ - D.1 Primitives complete (#65) ✓ - D.2 SamplePlayback (#66) ✓ - D.3 SignalChain marquee (#67) ✓ - D.4 FileUpload + Input Source (#69) ✓ - D.5a Results header + metric strip (#71) ✓ - D.5b Mix Chain (#73) ✓ - D.5c Patches (#74) ✓ - D.5d Section headers (#75) ✓ - D.6a Dashboard structure (#76) ✓ - D.6b Dashboard sections (#77) ✓ - D.6c Dashboard headers (#78) ✓ - D.7 SessionMusicianPanel + MixDoctorPanel (#79) — open - D.8 Signal Monitor + SpectrogramViewer drift (#80) — open - D.9 DenseDawConcept drift (#81) — open - D.10 Cleanup (this PR) ✓ https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe Co-authored-by: Claude <noreply@anthropic.com>
Summary
PR 1 of 16 in the UI consistency overhaul / Live 12 restyle stack. Lays the foundation for migrating consumers — no existing component is touched in this PR.
Design tokens (
apps/ui/src/index.css)@themeblock: device-chrome shadows (--shadow-device-{face,rack,active,success,error}), gradients (--gradient-{title-strip,title-strip-active,device-face,rail-bg}), LED palette (--color-led-*), focus rings, semantic spacing scale (--space-device-*,--space-title-strip-h,--space-rail-h), type-scale lock (--text-{meta,eyebrow,body-sm,body,value,value-lg,value-xl}), signal-cable colors.@layer componentsblock:.device-rack(+ status modifiers),.device-rack__{title-strip,body,rail},.led-indicator(+ pulse keyframes),.signal-cable(+ animated variant),.signal-arrow,.parameter-display,.tabular-mono.Primitive library (
apps/ui/src/components/ui/)cn.ts,variants.ts,index.tsTone/Status/Sizetypes + barrelButtoniconOnly,leadingIcon,ledIndicatorPill/BadgeleadingDot. Drops the legacyinfo/violet/blue/tealtones — call-site migration happens in D.5/D.6Panelrackcomposes the new.device-rackchromeSectionHeadergetTextRoleClassNamefromutils/displayTextand propagatesdata-text-roleto the heading — keeps the brittledata-text-roleassertions intests/services/analysisResultsUi.test.ts:465-474green when promoted in D.5LedIndicatorTooltiptextprop API as a drop-in forcomponents/Tooltip.tsxCheckboxEach primitive ships with a
.stories.tsxcovering default + variant matrix.Storybook + Chromatic
.storybook/main.tsuses@storybook/react-viteand reapplies the Tailwind v4 plugin viaviteFinal(documented gotcha — the standalone Storybook Vite pipeline does not inherit the app's plugin chain)..storybook/preview.tsximportssrc/index.css, wires the dark background palette, mountsTooltipProvideronce at the decorator level.package.jsonscripts:storybook,build-storybook,chromatic. Intentionally outsidenpm run verify— Storybook is design review, not a regression gate..github/workflows/chromatic.ymlruns visual regression on PRs touchingapps/ui. Non-blocking (exit-zero-on-changes). Requires theCHROMATIC_PROJECT_TOKENrepository secret to be configured before the first run.Dependencies
class-variance-authority,tailwind-merge,clsx,@radix-ui/react-{tooltip,dialog,popover,checkbox,slider,scroll-area}storybook@^10,@storybook/react-vite,@storybook/addon-a11y,chromaticPre-flight decisions (locked per plan §1)
<select>+ restyle viaappearance-none. No Radix Select primitive. (Only 2 selects in scope —App.tsx:984-998,1191-1204.)<Checkbox>, not Switch. Selection metaphor, not feature-flag metaphor.<ParameterKnob>: deferred to a follow-up PR. Not shipped here.Non-goals (untouched)
All
apps/backend/code;Phase1Result/AnalysisRunSnapshot/Phase2Resulttypes; everyapps/ui/src/services/*.ts;analyzeAudio()/monitorAnalysisRun()orchestration; chain-of-custody invariant (CitationBlock,CitationHeadline,phase2Validator.tsviolations); ports; env vars; every existingdata-testid.Test plan
npm run lint— cleannpm run test:unit— 605/605 passingnpm run build— clean (89.5 KB CSS, 14.5 KB gzipped)npm run build-storybook— clean; new tokens + utility classes verified present instorybook-static/assets/*.cssnpm run test:smoke— not run locally (Playwright browser download blocked by sandbox network policy); existing components untouched so smoke tests should pass in CInpm run storybook— boot locally and confirm tokens resolve, gradients render, focus rings appear, LED pulse animation runsCHROMATIC_PROJECT_TOKENsecret so the Chromatic workflow can baseline the initial storiesUp next (PR 2)
D.1 Primitives complete —
DeviceRack,MetricTile,MetricBar,MetricBarRow,SignalChain,ChainSeparator,EmptyState,TimeReadout,DataTable+ stories. Migratecomponents/Tooltip.tsxto re-export fromui/Tooltip.tsx. PR description will include the fullOldExport → NewPrimitivemapping table forMeasurementPrimitives.tsx.https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
Generated by Claude Code