Skip to content

Reviewer: senior design-systems engineer (fast pass, ~17 tool calls). Scope: brand contracts, design tokens, component library, showcase, mbr site (current focus).

1. mBR site bypasses the token + Tailwind theme system entirely

Section titled “1. mBR site bypasses the token + Tailwind theme system entirely”

What: mBR has zero connection to the OKLCH token pipeline. It defines its own raw hex variables and a stubbed Tailwind config — the exact problem the design system exists to solve. Where:

  • sites/mbr/tailwind.config.mjs:1-8 — empty theme.extend: {}, no token wiring (compare to sites/template/tailwind.config.mjs:6-53 which maps primary, secondary, etc. to hsl(var(--*))).
  • sites/mbr/src/styles/global.css:5-11 — hardcoded --mbr-emerald: #004425;, --mbr-gold: #d5b038;, --mbr-rose: #e8627a; etc. (the same brand colors that are SSoT in src/brand/mbr/index.ts:14-17 and would be generated by pnpm colors).
  • sites/mbr/src/styles/global.css:49,133,138,147,153–368 — 30+ hardcoded ad-hoc hex values (#d6dbd3, #1b5c40, #2b3d33, #a62845, …) for borders, surfaces, accents. None come from a palette scale.
  • src/brand/mbr/ — no colors.oklch.json, no tokens.css (compare to src/brand/sd/ and src/brand/ts/ which both have these). Why: This is a design system repo whose entire value proposition is brand-contract → OKLCH palette → CSS vars → Tailwind theme → components. mBR opts out of every layer. Result: brand changes can’t be made in one place, dark mode is impossible without rewriting global.css, no perceptual-uniformity guarantees, no shared component theming works (e.g., Button.astro references bg-primary which is unmapped here). Fix: (a) Create src/brand/mbr/colors.oklch.json mirroring src/brand/sd/colors.oklch.json with the rose tertiary; (b) run pnpm colors from sites/mbr to generate themes; (c) replicate sites/template/tailwind.config.mjs in sites/mbr/tailwind.config.mjs; (d) replace --mbr-emerald with hsl(var(--primary)) etc.; (e) replace the 30+ ad-hoc hexes with primary-N / stone-N scale positions. This is the single highest-leverage fix in the repo.

2. mBR consumes Template components via deep relative path (workspace violation)

Section titled “2. mBR consumes Template components via deep relative path (workspace violation)”

What: sites/mbr/src/components/brand/MyBetterRatesEntity.astro:2 does import MyBetterRates from '../../../../template/src/components/brand/MyBetterRates.astro'. Where: sites/mbr/src/components/brand/MyBetterRatesEntity.astro:2. Why: Template is supposed to be the component library source of truth, but it’s a site, not a package. Cross-site reach with ../../../../ couples mBR’s build to Template’s internal file layout. If Template reorganizes, mBR breaks silently. The repo already has packages/components/ for exactly this purpose. Fix: Move brand entity components (MyBetterRates.astro, SmartDebt.astro, BrandLogo.astro) into packages/components/src/brand/ and import via the workspace package. Or, if Template must stay authoritative, expose it as a workspace package (@monorepo/template-components) instead of importing across site boundaries.

3. sites/template/src/components/ui/ is empty — components live in packages/components/src/ui/

Section titled “3. sites/template/src/components/ui/ is empty — components live in packages/components/src/ui/”

What: CLAUDE.md says “Component Conventions — Location: sites/template/src/components/ui/” and “Template is source of truth - build components there first, then reuse.” But ls sites/template/src/components/ui/ returns empty; all 22 UI components are in packages/components/src/ui/. Where: CLAUDE.md lines under ”## Component Conventions” vs. actual packages/components/src/ui/ (Accordion, Alert, Badge, Banner, … Textarea). Why: Docs and reality diverged. New contributors (or future Claude sessions) will create components in the wrong place per the documented convention, fragmenting the library. Fix: Either (a) update CLAUDE.md to say packages/components/src/ui/ is canonical (recommended — packages-first is the better architecture), or (b) consolidate back to Template. Don’t leave both stories live.

4. BrandLogo.astro has TS color logic but ignores dark for TS, and mBR has no light/dark colour rules at all

Section titled “4. BrandLogo.astro has TS color logic but ignores dark for TS, and mBR has no light/dark colour rules at all”

What: In sites/template/src/components/brand/BrandLogo.astro:

  • TS uses a hardcoded hsl(215 69% 45%) for logo-bg (lines 161, 180) — not a token. If TS primary anchor moves, this breaks silently.
  • mBR has only a .logo-text rule (line 232–234); no .logo-bg or .logo-secondary colour rule, no dark variant.
  • The brand-logo--dark modifier exists for sd and ts but not mbr (lines 168–181 only). Where: sites/template/src/components/brand/BrandLogo.astro:160-181, 231-234. Why: Brand-specific hardcoded fills defeat the whole “neutral-color SVG → semantic class → token” mapping the file’s docstring claims (lines 71–80). And mBR will render with inherited CSS from :global(.logo-bg) chains — fragile. Fix: Replace hsl(215 69% 45%) with hsl(var(--primary-8)) (after task 1, mBR/TS sites will have their own primary scales). Add explicit .brand-logo--mbr .logo-bg / .logo-secondary fills using tokens. Add .brand-logo--dark.brand-logo--mbr rules for parity.

5. Brand contract is shape-inconsistent across brands

Section titled “5. Brand contract is shape-inconsistent across brands”

What: Each src/brand/{sd,ts,mbr}/index.ts has slightly different shapes:

  • sd has 4 logo variants (circle, leafDot, leafSquare, leafRect), ts has 1 (circle), mbr has 2 (dollarDot, dollarDotS).
  • BrandLogo.astro Props.variant type ('leafDot' | 'leafSquare' | 'leafRect' | 'circle') is sd-specific. Passing variant="dollarDot" for mbr is a type error; the component silently ignores variant for mbr (line 59: brand === 'mbr' ? 'mbr-dollarDot' :).
  • All three brands have an unused colors.tertiary in the contract but no consumer reads it.
  • The tokens lazy-loader is “DISABLED” in comments on sd and ts (lines 7–8) — dead code in the contract. Where: src/brand/sd/index.ts, src/brand/ts/index.ts, src/brand/mbr/index.ts, BrandLogo.astro:24-30, 59. Why: Type-level contract drift. Adding a 4th brand requires editing 3 places (Props.brand, svgMap, viewBoxMap, ariaLabel conditional, plus per-brand CSS block). The contract isn’t load-bearing — it’s documentation. Fix: Make brand contract authoritative: each brand declares variants: { name, viewBox, svg }[] and ariaLabel; BrandLogo.astro becomes a generic dispatcher that iterates the contract. Removes the brand-specific if chains. Also: delete the disabled tokens: comments, or wire them up.

6. Test coverage is palette-math-only — nothing validates the token pipeline end-to-end

Section titled “6. Test coverage is palette-math-only — nothing validates the token pipeline end-to-end”

What: packages/design-tokens/tests/oklch-palette.test.mjs tests parseRgb, parseOklch, rgbToCmyk, palette generation math. Zero tests verify that:

  • colors.oklch.json produces valid light.css / dark.css
  • Semantic mappings ("7": ["light:primary"]) actually emit --primary: <hsl> in output
  • Tailwind config consumes the resulting vars
  • WCAG AA contrast holds for generated theme pairs (--primary vs --primary-foreground, etc.) Where: packages/design-tokens/tests/oklch-palette.test.mjs (only test file). Why: The math is correct but irrelevant if the wiring breaks. A regression in generate-color-system.mjs (which is a 800+ line script per the grep output) would not be caught. Fix: Add an integration test that runs generate-color-system.mjs against a fixture brand and asserts (a) presence of expected CSS vars, (b) contrast ratios via a quick culori calc, (c) every semanticMappings key has a matching emitted var.

7. Card.astro violates its own theming with hardcoded fallback #ececec

Section titled “7. Card.astro violates its own theming with hardcoded fallback #ececec”

What: packages/components/src/ui/Card.astro:26const bgColor = backgroundColor || (hasHeader ? "#ececec" : undefined); — a hardcoded surface color baked into the component. Where: packages/components/src/ui/Card.astro:26. Why: Defeats --card, --muted, --secondary tokens. Won’t adapt to dark mode. Why does Card need a hex when line 54 already uses hsl(var(--card))? Fix: Remove the #ececec default. If a tinted header card is desired, use hsl(var(--muted)) or a scale position (hsl(var(--stone-1))).

8. Showcase has no coverage for mBR’s actual page components

Section titled “8. Showcase has no coverage for mBR’s actual page components”

What: sites/template/src/components/showcase/ has MyBetterRatesShowcase.astro (brand entity / logo only). The mBR site uses RateTable, SurvivalMathVisual, WaitlistForm, AnnualGainSnapshot — none in the showcase, none in packages/components/. They’re site-local in sites/mbr/src/components/. Where: sites/mbr/src/components/{RateTable,SurvivalMathVisual,WaitlistForm,AnnualGainSnapshot}.astro vs. sites/template/src/components/showcase/MyBetterRatesShowcase.astro. Why: These are exactly the kind of domain-flavored components that benefit from showcase coverage (states, edge cases, dark mode). And if SDC/TS ever need a “survival math” or “annual gain” widget, there’s no shared version — they’ll re-invent it. Fix: For each: decide if it’s truly mBR-specific (then OK to leave) or if it’s a candidate for packages/components/src/widgets/ with brand-neutral theming via tokens. At minimum, add showcase entries so designers can review them.

9. Button has duplicate default + primary variant aliases

Section titled “9. Button has duplicate default + primary variant aliases”

packages/components/src/ui/Button.astro:25, 42-44variant: 'default' | 'primary' | ... where both map to identical styles. Pick one (shadcn convention is default). Aliases invite drift.

10. Button has stale <style> block using Tailwind-ring pseudo-syntax

Section titled “10. Button has stale <style> block using Tailwind-ring pseudo-syntax”

packages/components/src/ui/Button.astro:82-86ring: 2px; ring-color: ...; inside a <style> block is not valid CSS (these are Tailwind utility names, not CSS properties). The focus ring already works via the focus-visible:ring-2 utility on line 61, so this block is dead and confusing.

sites/sdc/src/ contains only pages/ and styles/. Either SDC is a pure pages-only site (fine — note it) or it’s incomplete. Worth clarifying its status given Template/mBR have rich components/ trees.

12. design-tokens.conservative.css / design-tokens.expressive.css — unclear purpose

Section titled “12. design-tokens.conservative.css / design-tokens.expressive.css — unclear purpose”

sites/template/src/styles/ has three parallel token CSS files. No README. Either document the variant strategy or consolidate.

13. MBRFavicon.astro exists alongside BrandLogo with brand-switch — favicon system is fragmented

Section titled “13. MBRFavicon.astro exists alongside BrandLogo with brand-switch — favicon system is fragmented”

sites/template/src/components/brand/ has three separate per-brand favicon components (SDFavicon.astro, TSFavicon.astro, MBRFavicon.astro) but a single unified BrandLogo.astro. Inconsistent. Generalize favicon to a BrandFavicon with brand prop, mirroring BrandLogo.

14. Empty plugins array + no @tailwindcss/forms / @tailwindcss/typography

Section titled “14. Empty plugins array + no @tailwindcss/forms / @tailwindcss/typography”

sites/template/tailwind.config.mjs:56, sites/mbr/tailwind.config.mjs:7. Form inputs (Input.astro, Textarea.astro, Select.astro) reinvent reset styles; @tailwindcss/forms would standardize this with one line.

15. docs/design/colors.md references SmartDebt anchors but the live anchor table at line 319 shows TalbotStevens primary (#0053B3, blue, hue 258°)

Section titled “15. docs/design/colors.md references SmartDebt anchors but the live anchor table at line 319 shows TalbotStevens primary (#0053B3, blue, hue 258°)”

The doc is generated and got overwritten with the TS palette last time pnpm colors ran. Either generate per-brand docs or note the brand context. Currently misleading.