From 06dda09060fc47bca4e2b261f8bc330ac07cec7d Mon Sep 17 00:00:00 2001 From: Kenny Lin Date: Wed, 22 Jul 2026 15:55:44 -0400 Subject: [PATCH 01/16] address gamut-review --- .../agent-tools/skills/gamut-review/SKILL.md | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/packages/gamut/agent-tools/skills/gamut-review/SKILL.md b/packages/gamut/agent-tools/skills/gamut-review/SKILL.md index ae46627ba5..4707d4892c 100644 --- a/packages/gamut/agent-tools/skills/gamut-review/SKILL.md +++ b/packages/gamut/agent-tools/skills/gamut-review/SKILL.md @@ -73,9 +73,9 @@ Report each violation as `file:line`. --- -## Check 3b — SCSS/CSS module imports and className on Gamut components +## Check 3b — SCSS/CSS module imports, className, and inline styles on Gamut components -Gamut components are styled via the variance system (system props, `css()`, `variant()`, `states()` from `@codecademy/gamut-styles`). Importing SCSS/CSS modules and passing `className` to Gamut components bypasses this system entirely, breaks ColorMode token propagation, and prevents system props from composing correctly. +Gamut components are styled via the variance system (system props, `css()`, `variant()`, `states()` from `@codecademy/gamut-styles`). Importing SCSS/CSS modules, passing `className`, and passing an inline `style` prop to Gamut components all bypass this system, break ColorMode token propagation, and prevent system props from composing correctly. **Step 1 — SCSS/CSS module imports** @@ -112,7 +112,29 @@ Each match is an error. Report as `file:line `. Severity note: `className` is not always forbidden — some Gamut components accept it for integration with third-party tools (e.g. passing a class to an external drag-and-drop library). Downgrade to ⚠ warning only when the usage is clearly an integration seam, not styling. -Remediation: replace SCSS module rules with system props directly on the Gamut component — use semantic ColorMode tokens as values (`color="text"`, `bg="background"`, `borderColor="border-primary"`, etc.) rather than hardcoded hex or palette names; use `css()`, `variant()`, or `states()` from `@codecademy/gamut-styles` (with `styled` from `@emotion/styled`) for styles not expressible as system props; delete the SCSS file when all rules are migrated. +**Step 3 — inline `style` prop** + +Grep source files (`.ts`, `.tsx`, `.js`, `.jsx`) for an inline `style` prop on any JSX element, and specifically on the known Gamut component names from Step 2: + +``` +style=\{\{ +``` + +and + +``` +<(Box|FlexBox|Column|LayoutGrid|GridBox|Card|Text|Anchor|FillButton|StrokeButton|TextButton|CTAButton|IconButton|Toggle|List|ListRow|ListCol|Background|Disclosure)\b[^>]*\bstyle=\{ +``` + +Skip `node_modules`, `dist`. Each match is an error — an inline `style` object silently bypasses ColorMode and the variance system exactly like `className` does, and its values are almost always hardcoded hex or pixel literals rather than tokens. + +Before reporting a match, check the matched line and the line immediately above it for an `eslint-disable` / `eslint-disable-next-line` comment referencing a style-related rule (e.g. `react/forbid-component-props`, `react/forbid-dom-props`, or any bare `eslint-disable(-next-line)` directly above the match). If found, still report the match but annotate it `(eslint-disabled)` and downgrade to ⚠ warning — someone made a deliberate, reviewed exception, but the suppression may be stale or broader than intended, so surface it rather than silently skip it. + +Report as `file:line ` (or the bare JSX tag when not a known Gamut component). + +Severity note: like `className`, downgrade to ⚠ warning only for a clear third-party integration seam (e.g. a style object required by an external widget's API) — not for layout or color values that a Gamut component or system prop could express. + +Remediation: replace SCSS module rules and inline `style` objects with system props directly on the Gamut component — use semantic ColorMode tokens as values (`color="text"`, `bg="background"`, `borderColor="border-primary"`, etc.) rather than hardcoded hex, pixel literals, or inline objects; use `css()`, `variant()`, or `states()` from `@codecademy/gamut-styles` (with `styled` from `@emotion/styled`) for styles not expressible as system props; delete the SCSS file when all rules are migrated. Skill references: [`gamut-system-props`](../gamut-system-props/SKILL.md) · [`gamut-style-utilities`](../gamut-style-utilities/SKILL.md) · [`gamut-color-mode`](../gamut-color-mode/SKILL.md) @@ -358,13 +380,16 @@ Import patterns src/Thing.tsx:7 src/Other.tsx:12 -SCSS modules & className [→ gamut-system-props] [→ gamut-style-utilities] +SCSS modules, className & inline style [→ gamut-system-props] [→ gamut-style-utilities] [→ gamut-color-mode] ✗ SCSS/CSS imports 14 files — migrate to system props and css()/variant() src/components/Card/Card.scss src/components/Nav/Nav.scss (+ 12 more) ✗ className on Gamut components 9 occurrences src/components/Card/Card.tsx:14 src/components/Nav/Nav.tsx:7 + ✗ inline style on Gamut components 3 occurrences — use system props or css()/variant()/states() with semantic tokens + src/components/Hero/Hero.tsx:31 + src/components/Nav/Nav.tsx:19 (eslint-disabled) ⚠ Nested selectors [→ gamut-system-props] [→ gamut-style-utilities] ⚠ Tag selectors 3 occurrences — replace with system props or layout components (FlexBox, GridBox) @@ -395,4 +420,4 @@ Test setup [→ ga Icons: `✓` = pass, `⚠` = warning (recommended, not required), `✗` = error (required). `[→ skill-name]` annotations indicate which Gamut skill has remediation guidance for that category. -After printing the report, offer one sentence of prioritized next-step advice based on what was found. +After printing the report, offer one sentence of prioritized next-step advice based on what was found, then ask the user whether they'd like you to fix the findings — do not apply any remediation unprompted. From 998e6199d4ba12cdd39d978c5066f997d3ad025a Mon Sep 17 00:00:00 2001 From: Kenny Lin Date: Wed, 22 Jul 2026 16:15:48 -0400 Subject: [PATCH 02/16] update gamut-forms skill --- .../agent-tools/skills/gamut-forms/SKILL.md | 52 ++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/packages/gamut/agent-tools/skills/gamut-forms/SKILL.md b/packages/gamut/agent-tools/skills/gamut-forms/SKILL.md index 6ba8f77669..9686683b2f 100644 --- a/packages/gamut/agent-tools/skills/gamut-forms/SKILL.md +++ b/packages/gamut/agent-tools/skills/gamut-forms/SKILL.md @@ -22,7 +22,57 @@ For typical product forms, prefer `GridForm` (declarative `fields`, `LayoutGrid` `htmlFor` / `id` pairing — universal rule in [`accessibility.mdc`](../../rules/accessibility.mdc) (Form label association). Form-specific notes: - `FormGroupLabel` → control `id` (or stable `name` when that is your field’s id convention). -- Checkbox, Radio, Select: same pairing; checkbox/radio use the visually hidden input pattern from `@codecademy/gamut-styles` where applicable. +- Checkbox, Select: same pairing; checkbox uses the visually hidden input pattern from `@codecademy/gamut-styles` where applicable. +- Radio: see [`Radio` and `RadioGroup`](#radio-and-radiogroup) below — its label/group wiring differs from a single Checkbox/Select field. + +--- + +## `Radio` and `RadioGroup` + +`packages/gamut/src/Form/inputs/Radio.tsx` · `packages/gamut/src/Form/inputs/RadioGroup.tsx` · reference: `packages/styleguide/src/lib/Atoms/FormInputs/Radio/Radio.mdx` + +### No `variant`/`size` props — state is derived, not chosen + +`Radio` does not take a consumer-facing `variant` or `size` prop. Its visual state is derived internally from the booleans you already pass: + +- `error?: boolean` and `disabled?: boolean` (from `BaseInputProps`) feed a `conditionalStyleState(error, disabled)` helper that resolves to `'error' | 'disabled' | undefined`, which is then applied via `variant()` (from `@codecademy/gamut-styles`) on both the input and its label (`conditionalRadioInputStyles` / `conditionalRadioLabelStyles` in `Form/styles/Radio-styles.ts`). +- `error` wins over `disabled` when both are true — do not pass both expecting a combined visual; pick the one that matches the actual field state. +- There is no size scale — all Radio inputs render at a fixed 20×20 indicator. If a design calls for a different size, that's a design deviation to flag, not a prop to reach for. + +### Visually-hidden-input pattern + +The real `` (`RadioInput`) is styled with `screenReaderOnly` from `@codecademy/gamut-styles` — it stays in the accessibility tree and keeps native keyboard/focus/change behavior, but is visually hidden. The circle indicator the user sees is rendered on the sibling `RadioLabel`'s `::before` (outer ring) and `::after` (fill dot) pseudo-elements, driven by CSS sibling selectors keyed to the hidden input's `:checked`, `:hover`, and `:focus` state (`InputSelectors.CHECKED_AFTER`, `HOVER_FOCUS_BEFORE`, etc.). + +Do not rebuild this with a `className`/inline-`style` override or a separate decorative `` — that duplicates state the hidden input already owns and drifts out of sync on focus/hover/checked. If a design needs a different indicator look, change `Radio-styles.ts`, not the call site. + +### Group and label wiring + +`RadioGroup` clones its `Radio` children and injects three things you should _not_ also pass on each child: `name`, `onChange`, and `htmlFor` (built from `htmlForPrefix` + child index, e.g. `example-radio-0`). Wire it as: + +- `RadioGroup` gets `htmlForPrefix`, `name`, and (optionally) `onChange`/`disabled`/`selected`. +- Each `Radio` gets its own `label`, `value`, and optionally `infotip` — not `name` or `htmlFor`, those come from the parent. +- To label the group itself (not an individual option), wrap `RadioGroup` in `FormGroup` and pass `infotip` to `FormGroup`, not to a `Radio` — `FormGroup` links it via `aria-labelledby` to the group label automatically. Individual `Radio`s can still carry their own `infotip` for per-option context; the two are independent (see the worked example below). + +### Example — `RadioGroup` inside `FormGroup`, with mixed InfoTips + +```tsx + + + + + + +``` + +For an error state on the group, pass `error` to each `Radio` (there is no group-level `error` prop on `RadioGroup` itself) and surface the message the same way as any other field — via `FormGroup`'s `error` prop, not a second live region (see [Live regions](#live-regions-do-not-double-up)). --- From bc668172726ac244441050a2585eba6253608d85 Mon Sep 17 00:00:00 2001 From: Kenny Lin Date: Fri, 24 Jul 2026 16:23:29 -0400 Subject: [PATCH 03/16] update darkmode, and new gamut-modal + gamut-z-index skills --- packages/gamut/agent-tools/DESIGN.LXStudio.md | 111 ++++++----- packages/gamut/agent-tools/DESIGN.Percipio.md | 106 +++++----- .../skills/gamut-accessibility/SKILL.md | 8 +- .../skills/gamut-datalist/SKILL.md | 4 +- .../skills/gamut-datatable/SKILL.md | 4 +- .../agent-tools/skills/gamut-menu/SKILL.md | 4 +- .../agent-tools/skills/gamut-modal/SKILL.md | 186 ++++++++++++++++++ .../agent-tools/skills/gamut-review/SKILL.md | 2 +- .../agent-tools/skills/gamut-z-index/SKILL.md | 99 ++++++++++ 9 files changed, 412 insertions(+), 112 deletions(-) create mode 100644 packages/gamut/agent-tools/skills/gamut-modal/SKILL.md create mode 100644 packages/gamut/agent-tools/skills/gamut-z-index/SKILL.md diff --git a/packages/gamut/agent-tools/DESIGN.LXStudio.md b/packages/gamut/agent-tools/DESIGN.LXStudio.md index 2cdf190145..73d4af7c74 100644 --- a/packages/gamut/agent-tools/DESIGN.LXStudio.md +++ b/packages/gamut/agent-tools/DESIGN.LXStudio.md @@ -141,88 +141,97 @@ LX Studio communicates **modern professional craft** — clean, precise, and too **Design philosophy**: - Larger border radii (`sm`–`lg`) for a softer, more modern feel -- Brand blue (`sapphire` / `primary`) drives CTAs, buttons, and links +- Brand blue (`sapphire` / `primary`) drives CTAs, buttons, and links in **light mode** — in dark mode `primary` falls back to Core's `yellow-500`, not `sapphire` (see Semantic Color Aliases) - Soft shadows (`shadow-primary` → `navy-200`) - Skillsoft Text (`base`) and Skillsoft Sans (`accent`) for all UI typography +- Dark mode is supported but not bespoke — `lxStudioTheme` only overrides light-mode tokens (see `packages/gamut-styles/src/themes/lxStudio.ts`); dark mode falls back to Core's dark palette unmodified --- ## Themes -LX Studio uses a single Gamut theme — light mode only. +LX Studio uses a single Gamut theme with both light and dark modes. -| Theme | Use case | Base font | Dark mode | -| ------------- | -------------------------------------- | --------------------- | ---------- | -| **LX Studio** | Skillsoft LX Studio authoring platform | Skillsoft Text / Sans | light only | +| Theme | Use case | Base font | Dark mode | +| ------------- | -------------------------------------- | --------------------- | ----------------------------------------------------- | +| **LX Studio** | Skillsoft LX Studio authoring platform | Skillsoft Text / Sans | ✓ light + dark (dark inherited from Core, unmodified) | -The active theme is set at the app root via ``. +The active theme is set at the app root via ``. `lxStudioTheme` calls `.addColorModes('light', { light: {...} })` with **only** a `light` override map — there is no `dark` key in the theme definition, so every semantic token falls back to Core's `dark` values for that token when the app is in dark mode. This is different from `adminTheme`/`platformTheme`, which define their own explicit `dark` overrides on top of Core. --- ## Semantic Color Aliases -Use these token names when specifying colors. LX Studio is light mode only — there are no dark mode counterparts. +Use these token names when specifying colors — they work in both light and dark mode via ``. **Light** below is LX Studio's own value where it overrides Core (in `lxStudio.ts`); everywhere else, Light is simply inherited from Core. **Dark** is always Core's unmodified dark value for that token, since `lxStudioTheme` never defines a `dark` override map. ### Text -| Token | Resolves to | Use for | -| ---------------- | ----------- | --------------------------- | -| `text` | `navy-800` | Default body and UI text | -| `text-accent` | `navy-900` | Stronger emphasis text | -| `text-secondary` | `navy-600` | Supporting / secondary copy | -| `text-disabled` | `navy-500` | Disabled state labels | +Not overridden by LX Studio at all — these are Core's values in both modes. + +| Token | Light | Dark | Use for | +| ---------------- | ---------- | ----------- | --------------------------- | +| `text` | `navy-800` | `white` | Default body and UI text | +| `text-accent` | `navy-900` | `beige` | Stronger emphasis text | +| `text-secondary` | `navy-600` | `white-600` | Supporting / secondary copy | +| `text-disabled` | `navy-500` | `white-500` | Disabled state labels | ### Background -| Token | Resolves to | Use for | -| --------------------- | ------------------- | --------------------------------- | -| `background` | `white` | Default page/component background | -| `background-primary` | `lxStudioBgPrimary` | Slightly elevated surfaces | -| `background-contrast` | `white` | Maximum contrast surface | -| `background-selected` | `navy-100` | Selected row / item | -| `background-hover` | `navy-200` | Hover state overlay | -| `background-disabled` | `navy-200` | Disabled surface | -| `background-success` | `green-0` | Success state container | -| `background-warning` | `yellow-0` | Warning state container | -| `background-error` | `red-0` | Error state container | +| Token | Light | Dark | Use for | +| --------------------- | ------------------- | ------------ | --------------------------------- | +| `background` | `white` | `navy-800` | Default page/component background | +| `background-primary` | `lxStudioBgPrimary` | `navy-900` | Slightly elevated surfaces | +| `background-contrast` | `white` | `black` | Maximum contrast surface | +| `background-selected` | `navy-100` | `white-100` | Selected row / item | +| `background-hover` | `navy-200` | `white-200` | Hover state overlay | +| `background-disabled` | `navy-200` | `white-200` | Disabled surface | +| `background-success` | `green-0` | `green-900` | Success state container | +| `background-warning` | `yellow-0` | `yellow-900` | Warning state container | +| `background-error` | `red-0` | `red-900` | Error state container | + +`background-primary` is the only row LX Studio overrides in light mode (`lxStudioBgPrimary`); its dark value is Core's plain `navy-900`, not an LX-specific token. ### Interactive -| Token | Resolves to | Use for | -| ----------------- | ------------ | ------------------------------------ | -| `primary` | `sapphire` | Primary CTA, links, focus rings | -| `primary-hover` | `navy-800` | Hover state of primary interactive | -| `primary-inverse` | `yellow-500` | Primary on a colored background | -| `secondary` | `navy-800` | Secondary CTA, ghost buttons | -| `secondary-hover` | `navy-700` | Hover state of secondary interactive | -| `danger` | `red-500` | Destructive actions, error states | -| `danger-hover` | `red-600` | Hover on danger interactive | +| Token | Light | Dark | Use for | +| ----------------- | ------------ | ------------ | ------------------------------------ | +| `primary` | `sapphire` | `yellow-500` | Primary CTA, links, focus rings | +| `primary-hover` | `navy-800` | `yellow-400` | Hover state of primary interactive | +| `primary-inverse` | `yellow-500` | `hyper-500` | Primary on a colored background | +| `secondary` | `navy-800` | `white` | Secondary CTA, ghost buttons | +| `secondary-hover` | `navy-700` | `white-700` | Hover state of secondary interactive | +| `danger` | `red-500` | `red-300` | Destructive actions, error states | +| `danger-hover` | `red-600` | `red-400` | Hover on danger interactive | + +**`primary` does not stay LX Studio's brand blue in dark mode.** `sapphire`/`navy-800` are light-mode-only overrides; dark mode uses Core's `yellow-500`/`yellow-400` (the same values Core itself uses), since `lxStudioTheme` never defines a `dark` override. If a design calls for `sapphire` to persist in dark mode, that requires an actual theme change, not a documentation fix. ### Border -| Token | Resolves to | Use for | -| ------------------ | ----------- | ------------------------------- | -| `border-primary` | `navy-400` | Standard input and card borders | -| `border-secondary` | `navy-600` | Medium-weight borders | -| `border-tertiary` | `navy-800` | Strong structural borders | -| `border-disabled` | `navy-300` | Disabled input borders | +| Token | Light | Dark | Use for | +| ------------------ | ---------- | ----------- | ------------------------------- | +| `border-primary` | `navy-400` | `white` | Standard input and card borders | +| `border-secondary` | `navy-600` | `white-600` | Medium-weight borders | +| `border-tertiary` | `navy-800` | `white-300` | Strong structural borders | +| `border-disabled` | `navy-300` | `white-500` | Disabled input borders | -`border-primary` resolves to `navy-400` — mid-weight borders for inputs and cards. +`border-primary` resolves to `navy-400` in light mode — mid-weight borders for inputs and cards. `border-secondary` is the only row here LX Studio does not override; it's inherited from Core in both modes. ### Feedback -| Token | Resolves to | Use for | -| ------------------ | ----------------- | -------------------------------- | -| `feedback-error` | `red-600` | Error messages, validation | -| `feedback-success` | `lxStudioSuccess` | Success messages, confirmations | -| `feedback-warning` | `yellow-500` | Warning messages, caution states | +| Token | Light | Dark | Use for | +| ------------------ | ----------------- | ----------- | -------------------------------- | +| `feedback-error` | `red-600` | `red-300` | Error messages, validation | +| `feedback-success` | `lxStudioSuccess` | `green-400` | Success messages, confirmations | +| `feedback-warning` | `yellow-500` | `yellow-0` | Warning messages, caution states | + +`feedback-error` and `feedback-warning` are inherited from Core in light mode too (not LX-specific); `feedback-success` is LX Studio's own light override, but its dark value falls back to Core's `green-400`, not `lxStudioSuccess`. ### Shadow -| Token | Resolves to | -| ------------------ | ----------- | -| `shadow-primary` | `navy-200` | -| `shadow-secondary` | `navy-600` | +| Token | Light | Dark | +| ------------------ | ---------- | ----------- | +| `shadow-primary` | `navy-200` | `white` | +| `shadow-secondary` | `navy-600` | `white-600` | Use `shadow-primary` for standard elevated surfaces. @@ -380,8 +389,8 @@ Key patterns: ### Colors - **Do** use semantic color aliases (`primary`, `text`, `background`, etc.) — never hardcode hex values. -- **Do** use `primary` (resolves to palette `sapphire`) for buttons and links. -- **Don't** attempt dark mode — LX Studio is light only. +- **Do** use `primary` (resolves to palette `sapphire` in light mode) for buttons and links. +- **Don't** assume LX Studio has a bespoke dark palette — it inherits Core's dark values unmodified (see Semantic Color Aliases). `primary` in particular becomes Core's `yellow-500` in dark mode, not `sapphire` — verify visually rather than assuming brand color persists. - **Don't** use raw palette swatches for adaptive UI — use semantic aliases. ### Typography diff --git a/packages/gamut/agent-tools/DESIGN.Percipio.md b/packages/gamut/agent-tools/DESIGN.Percipio.md index 40af3e63d7..4f80578fa8 100644 --- a/packages/gamut/agent-tools/DESIGN.Percipio.md +++ b/packages/gamut/agent-tools/DESIGN.Percipio.md @@ -130,8 +130,8 @@ Percipio communicates **professional clarity** — clean, trustworthy, and enter **Design philosophy**: -- Light mode only — no dark mode support -- Brand blue (`sapphire` / `primary`) for buttons, links, and focus rings +- Dark mode is supported but not bespoke — `percipioTheme` only overrides light-mode tokens (see `packages/gamut-styles/src/themes/percipio.ts`); dark mode falls back to Core's dark palette unmodified (see Semantic Color Aliases below) +- Brand blue (`sapphire` / `primary`) for buttons, links, and focus rings in **light mode** — in dark mode `primary` resolves to Core's `yellow-500`, not `sapphire` - Text uses dedicated Percipio palette tokens (`percipioTextPrimary`, etc.) - Shadows are soft and minimal (`shadow-primary` → `navy-200`) - Title font weight is **500** via `fontWeight="title"` — use semantic weight, not literal `700` @@ -140,79 +140,83 @@ Percipio communicates **professional clarity** — clean, trustworthy, and enter ## Themes -Percipio uses a single Gamut theme — light mode only. +Percipio uses a single Gamut theme with both light and dark modes. -| Theme | Use case | Base font | Dark mode | -| ------------ | --------------------------- | --------------------- | ---------- | -| **Percipio** | Skillsoft Percipio platform | Skillsoft Text / Sans | light only | +| Theme | Use case | Base font | Dark mode | +| ------------ | --------------------------- | --------------------- | ----------------------------------------------------- | +| **Percipio** | Skillsoft Percipio platform | Skillsoft Text / Sans | ✓ light + dark (dark inherited from Core, unmodified) | -The active theme is set at the app root via ``. +The active theme is set at the app root via ``. `percipioTheme` calls `.addColorModes('light', { light: {...} })` with **only** a `light` override map — there is no `dark` key in the theme definition, so every semantic token falls back to Core's `dark` values for that token when the app is in dark mode. This is different from `adminTheme`/`platformTheme`, which define their own explicit `dark` overrides on top of Core. --- ## Semantic Color Aliases -Use these token names when specifying colors. Percipio is light mode only — there are no dark mode counterparts. +Use these token names when specifying colors — they work in both light and dark mode via ``. **Light** below is Percipio's own value (overridden in `percipio.ts` where noted); **Dark** is Core's unmodified dark value for that same token, since `percipioTheme` never defines a `dark` override map. A token whose Light column is a plain palette name (not a `percipio*` token) is itself inherited from Core in light mode too — Percipio didn't touch it in either mode. ### Text -| Token | Resolves to | Use for | -| ---------------- | ----------------------- | ------------------------------------------------ | -| `text` | `percipioTextPrimary` | Default body and UI text | -| `text-accent` | `percipioTextPrimary` | Emphasis text (same value as `text` in Percipio) | -| `text-secondary` | `percipioTextSecondary` | Supporting / secondary copy | -| `text-disabled` | `percipioTextDisabled` | Disabled state labels | +| Token | Light | Dark | Use for | +| ---------------- | ----------------------------------------------------- | ----------- | --------------------------- | +| `text` | `percipioTextPrimary` | `white` | Default body and UI text | +| `text-accent` | `percipioTextAccent` (same hex as `text` in Percipio) | `beige` | Stronger emphasis text | +| `text-secondary` | `percipioTextSecondary` | `white-600` | Supporting / secondary copy | +| `text-disabled` | `percipioTextDisabled` | `white-500` | Disabled state labels | ### Background -| Token | Resolves to | Use for | -| --------------------- | ------------------- | --------------------------------- | -| `background` | `white` | Default page/component background | -| `background-primary` | `percipioBgPrimary` | Slightly elevated surfaces | -| `background-selected` | `navy-100` | Selected row / item | -| `background-hover` | `navy-200` | Hover state overlay | -| `background-disabled` | `navy-200` | Disabled surface | -| `background-success` | `percipioBgSuccess` | Success state container | -| `background-warning` | `percipioBgWarning` | Warning state container | -| `background-error` | `percipioBgError` | Error state container | +| Token | Light | Dark | Use for | +| --------------------- | ------------------- | ------------ | --------------------------------- | +| `background` | `white` | `navy-800` | Default page/component background | +| `background-primary` | `percipioBgPrimary` | `navy-900` | Slightly elevated surfaces | +| `background-selected` | `navy-100` | `white-100` | Selected row / item | +| `background-hover` | `navy-200` | `white-200` | Hover state overlay | +| `background-disabled` | `navy-200` | `white-200` | Disabled surface | +| `background-success` | `percipioBgSuccess` | `green-900` | Success state container | +| `background-warning` | `percipioBgWarning` | `yellow-900` | Warning state container | +| `background-error` | `percipioBgError` | `red-900` | Error state container | + +Note the dark values for `background-primary`/`success`/`warning`/`error` are **Core's** dark tokens, not Percipio-specific — the `percipioBg*` tokens Percipio defines have no dark counterpart. ### Interactive -| Token | Resolves to | Use for | -| ----------------- | ------------------------------ | ------------------------------------ | -| `primary` | `sapphire` | Primary CTA, links, focus rings | -| `primary-hover` | `percipioActionPrimaryHover` | Hover state of primary interactive | -| `primary-inverse` | `white` | Primary on a colored background | -| `secondary` | `percipioActionSecondary` | Secondary CTA, ghost buttons | -| `secondary-hover` | `percipioActionSecondaryHover` | Hover state of secondary interactive | -| `danger` | `percipioDanger` | Destructive actions, error states | -| `danger-hover` | `percipioActionDangerHover` | Hover on danger interactive | +| Token | Light | Dark | Use for | +| ----------------- | ------------------------------ | ------------ | ------------------------------------ | +| `primary` | `sapphire` | `yellow-500` | Primary CTA, links, focus rings | +| `primary-hover` | `percipioActionPrimaryHover` | `yellow-400` | Hover state of primary interactive | +| `primary-inverse` | `white` | `hyper-500` | Primary on a colored background | +| `secondary` | `percipioActionSecondary` | `white` | Secondary CTA, ghost buttons | +| `secondary-hover` | `percipioActionSecondaryHover` | `white-700` | Hover state of secondary interactive | +| `danger` | `percipioDanger` | `red-300` | Destructive actions, error states | +| `danger-hover` | `percipioActionDangerHover` | `red-400` | Hover on danger interactive | + +**`primary` does not stay Percipio's brand blue in dark mode.** `sapphire` is a light-mode-only override; dark mode uses Core's `yellow-500` (the same value Core itself uses, since `percipioTheme` never defines a `dark` override). If a design calls for `sapphire` to persist in dark mode, that requires an actual theme change — adding a `dark` override to `percipioTheme` — not a documentation fix. Flag it rather than assuming the token already does this. ### Border -Percipio's border weights use a non-standard order: `primary` is mid-weight, `secondary` is very light, `tertiary` is the strongest (solid navy). Use them for their semantic intent, not their numeric rank. +Percipio's border weights use a non-standard order in light mode: `primary` is mid-weight, `secondary` is very light, `tertiary` is the strongest (solid navy). Use them for their semantic intent, not their numeric rank. Dark values are Core's unmodified dark borders. -| Token | Resolves to | Use for | -| ------------------ | ----------- | ----------------------------------- | -| `border-primary` | `navy-400` | Standard input and card borders | -| `border-secondary` | `navy-200` | Subtle dividers, section separators | -| `border-tertiary` | `navy-800` | Strong structural borders | -| `border-disabled` | `navy-300` | Disabled input borders | +| Token | Light | Dark | Use for | +| ------------------ | ---------- | ----------- | ----------------------------------- | +| `border-primary` | `navy-400` | `white` | Standard input and card borders | +| `border-secondary` | `navy-600` | `white-600` | Subtle dividers, section separators | +| `border-tertiary` | `navy-800` | `white-300` | Strong structural borders | +| `border-disabled` | `navy-300` | `white-500` | Disabled input borders | ### Feedback -| Token | Resolves to | Use for | -| ------------------ | ------------------------- | -------------------------------- | -| `feedback-error` | `percipioDanger` | Error messages, validation | -| `feedback-success` | `percipioFeedbackSuccess` | Success messages, confirmations | -| `feedback-warning` | `percipioFeedbackWarning` | Warning messages, caution states | +| Token | Light | Dark | Use for | +| ------------------ | ------------------------- | ----------- | -------------------------------- | +| `feedback-error` | `percipioDanger` | `red-300` | Error messages, validation | +| `feedback-success` | `percipioFeedbackSuccess` | `green-400` | Success messages, confirmations | +| `feedback-warning` | `percipioFeedbackWarning` | `yellow-0` | Warning messages, caution states | ### Shadow -| Token | Resolves to | -| ------------------ | ----------- | -| `shadow-primary` | `navy-200` | -| `shadow-secondary` | `navy-400` | +| Token | Light | Dark | +| ------------------ | ---------- | ----------- | +| `shadow-primary` | `navy-200` | `white` | +| `shadow-secondary` | `navy-400` | `white-600` | Use `shadow-primary` for standard elevated surfaces. @@ -378,8 +382,8 @@ Key patterns: ### Colors - **Do** use semantic color aliases (`primary`, `text`, `background`, etc.) — never hardcode hex values. -- **Do** use `primary` (resolves to palette `sapphire`) as the brand interactive color. -- **Don't** attempt dark mode — Percipio is light only. +- **Do** use `primary` (resolves to palette `sapphire` in light mode) as the brand interactive color. +- **Don't** assume Percipio has a bespoke dark palette — it inherits Core's dark values unmodified (see Semantic Color Aliases). `primary` in particular becomes Core's `yellow-500` in dark mode, not `sapphire` — verify visually rather than assuming brand color persists. ### Typography diff --git a/packages/gamut/agent-tools/skills/gamut-accessibility/SKILL.md b/packages/gamut/agent-tools/skills/gamut-accessibility/SKILL.md index b6db56191e..9711023fd7 100644 --- a/packages/gamut/agent-tools/skills/gamut-accessibility/SKILL.md +++ b/packages/gamut/agent-tools/skills/gamut-accessibility/SKILL.md @@ -1,6 +1,6 @@ --- name: gamut-accessibility -description: Use this skill when implementing accessibility for a specific Gamut component, building a custom overlay or composite widget, or auditing component usage against WCAG — complements the always-loaded `accessibility.mdc` with Gamut component-specific patterns. Form wiring lives in `gamut-forms`. +description: Use this skill when implementing accessibility for a specific Gamut component, building a custom overlay or composite widget, or auditing component usage against WCAG — complements the always-loaded `accessibility.mdc` with Gamut component-specific patterns. Form wiring lives in `gamut-forms`; Modal/Dialog composition detail lives in `gamut-modal`. --- # Gamut Accessibility @@ -33,8 +33,8 @@ Forms — `FormGroup`, `ConnectedForm` / `ConnectedFormGroup`, `GridForm`, field | ------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | `FillButton`, `TextButton`, `StrokeButton`, `CTAButton` | Render ` + + + + ); +}; diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/grading.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/grading.json new file mode 100644 index 0000000000..25b9cf0ee8 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/grading.json @@ -0,0 +1,25 @@ +{ + "expectations": [ + { + "text": "Flags this component as duplicating an existing Gamut component (bespoke dialog) -- role=\"dialog\" is set by hand with no Modal/Dialog import", + "passed": true, + "evidence": "\"CustomConfirm.tsx:27 role=\\\"dialog\\\" with no Modal/Dialog import from @codecademy/gamut -- ... strongly recommend Modal\"." + }, + { + "text": "Flags the manual document.addEventListener('keydown', ...) Escape-key handling as hand-rolled dismiss logic that Overlay/FocusTrap already provides", + "passed": true, + "evidence": "\"manual Escape-key dismiss ... with no Overlay/FocusTrap/PopoverContainer import -- Gamut's Modal/Overlay already implements Escape-to-close\"." + }, + { + "text": "Points remediation at gamut-modal and/or gamut-component-first", + "passed": true, + "evidence": "\"[→ gamut-component-first]\" cross-reference tag on the section header, plus explicit text pointer." + }, + { + "text": "Treats this as a warning/heuristic finding needing manual confirmation, not an unconditional hard error -- since it's pattern-matching, not a certainty", + "passed": false, + "evidence": "REAL RULE VIOLATION: the section header correctly says \"(heuristic -- confirm manually)\", but two of the five individual findings under it (the role=\"dialog\" finding and the manual-Escape-listener finding) are marked with a hard ✗ (error) icon, not ⚠ (warning). gamut-review's own Check 6 instructions explicitly state: \"Report all matches as ⚠ warning (never ✗ error)\". The report is internally inconsistent -- it gets the outside-click variant correctly marked ⚠ (with an explicit note explaining why), but fails to apply the same rule to the two more clear-cut findings. This is a genuine, verifiable failure to follow the skill's own stated instruction, not a matter of interpretation." + } + ], + "summary": { "passed": 3, "failed": 1, "total": 4, "pass_rate": 0.75 } +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/outputs/report.md b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/outputs/report.md new file mode 100644 index 0000000000..15d8604b3f --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/outputs/report.md @@ -0,0 +1,51 @@ +Gamut Review — /Users/kenny.lin/Documents/eng/codecademy-eng/gamut/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/fixture/CustomConfirm.tsx +══════════════════════════════════════════════════ + +Scope note: audit root is a single standalone file (fixture), not a project checkout. Checks that require project-level artifacts (package.json, DESIGN.md, app-root wiring, test files) are marked N/A below rather than scored, per the single-file audit instructions. + +DESIGN.md +— N/A no project root to resolve; this is a single-file audit, not a project checkout + +Dependencies +— N/A no package.json present at audit root (single-file audit) — skipped per instructions + +Setup +— N/A (reduced relevance) GamutProvider / ColorMode / Background / theme augmentation are app-root/project concerns; none appear in this file, which is expected for a single leaf component and is not itself a violation. +Note: the file does use TypeScript (typed props via `interface CustomConfirmProps`), so if/when this component is reviewed in its real project, Check 2's "Theme augmentation" row should still be verified there. + +Import patterns +✓ Deep dist imports none found +✓ Deep src imports none found +Note: the file contains no import from @codecademy/gamut, @codecademy/gamut-styles, or @codecademy/variance at all — see "Bespoke component duplication" below; this is the root cause of most findings in this report. + +SCSS modules, className & inline style [→ gamut-system-props] [→ gamut-style-utilities] [→ gamut-color-mode] +✓ SCSS/CSS imports none found +✓ className on Gamut components none found (no Gamut components imported/used) +✗ inline style on JSX elements 2 occurrences — hardcoded layout/color values on plain
s that should be a Gamut Modal/Overlay (or, at minimum, Box) using system props and semantic tokens +CustomConfirm.tsx:28
— backdrop; Gamut's Modal/Overlay provides this scrim for free +CustomConfirm.tsx:32
— dialog surface; hardcoded 'white' fill and pixel literals instead of a semantic bg/p/layout system props + +Nested selectors [→ gamut-system-props] [→ gamut-style-utilities] +✓ none found (no styled/Emotion template literals in this file) + +styled(GamutComponent) bypassing system props [→ gamut-system-props] [→ gamut-style-utilities] +✓ none found (styled is not imported/used) + +Hardcoded colors [→ gamut-color-mode] +✓ no hex literals (#RGB/#RRGGBB) found +⚠ CustomConfirm.tsx:28 'rgba(0,0,0,0.5)' → semantic: (n/a — scrim) | palette: — | note: not a hex literal so outside Check 4's strict discovery pattern, but the same class of problem as the inline-style finding above: a hand-set backdrop color that a Gamut Modal/Overlay would supply via its own scrim styling, no token needed +⚠ CustomConfirm.tsx:32 'white' → semantic: background | palette: white | note: literal CSS color keyword; if this stays a bespoke element, prefer bg="background" on a Box/Card rather than the keyword + +Test setup [→ gamut-testing] +— N/A this is a component file, not a test file; no **tests**/.test./.spec. files exist alongside it in this fixture + +Bespoke component duplication (heuristic — confirm manually) [→ gamut-component-first] +✗ CustomConfirm.tsx:27 role="dialog" with no Modal/Dialog import from @codecademy/gamut — this is a hand-rolled confirmation dialog; strongly recommend Modal (see decision table in gamut-component-first) +✗ CustomConfirm.tsx:14-21 manual Escape-key dismiss (e.key === 'Escape' + document.addEventListener('keydown', ...)) with no Overlay/FocusTrap/PopoverContainer import — Gamut's Modal/Overlay already implements Escape-to-close +⚠ CustomConfirm.tsx:29,33 manual outside-click dismiss (backdrop onClick={onClose} + inner content onClick={(e) => e.stopPropagation()}) — same class of hand-rolled dismiss logic as the Escape handler; Gamut's Modal/Overlay/PopoverContainer handles outside-click dismissal internally, so this doesn't need to be reimplemented (flagged as a warning since it's a variant of the literal addEventListener('click', ...) pattern the check greps for, but functionally identical) +⚠ CustomConfirm.tsx (whole file) filename CustomConfirm.tsx does not literally match the Check 6 Step 2 filename list (Modal, Dialog, Dropdown, Tooltip, Popover, Menu, Toast, Accordion, Tabs, Pagination, Avatar, Badge, Tag), so the filename-matching heuristic doesn't fire — but the component's actual behavior (role="dialog", overlay + centered surface, Escape/outside-click dismiss, confirm/cancel actions) is exactly a confirmation modal. Worth flagging manually alongside the Step 1/3 hits above rather than relying on filename alone. +⚠ CustomConfirm.tsx:36-37 plain ` / `` have no variant styling, no focus-visible treatment, no consistent sizing/padding, and no way to express a destructive/danger action. There's also no defined spacing between the two buttons (relying on default browser margins) versus the `columnGap: 16` token used by `ModalContainer`. + +### 13. No close ("X") button + +Every Gamut `Modal`/`Dialog` renders a dismiss `IconButton` (`MiniDeleteIcon`) by default (optionally hidden via `closeButtonProps.hidden`). `CustomConfirm` provides no equivalent, so the only way to dismiss it is clicking the scrim or hitting Escape — a smaller/less discoverable affordance than the rest of the system's dialogs. + +### 14. API shape diverges from the established `Dialog` contract + +Gamut's existing pattern for this exact scenario uses `confirmCta` / `cancelCta` (each `{ children, onClick, href? }`) and `onRequestClose`. `CustomConfirm` instead exposes `onConfirm` / `onClose` with no label flexibility (button text is hard-coded to "Cancel"/"Confirm"). This introduces a second, parallel confirm-dialog API that solves the same problem the system already solves, increasing cognitive overhead for engineers who know the real `Dialog` contract. + +## Suggested remediation + +Replace this component entirely with Gamut's `Dialog`: + +```tsx +import { Dialog } from '@codecademy/gamut'; + +; +``` + +This alone resolves items 1–13 by inheriting the system's token-based colors/spacing/radius, portal + z-index handling, focus trap, escape/outside-click handling, ARIA wiring, typography, and button variants for free. diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-bespoke-modal/without_skill/run-1/timing.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-bespoke-modal/without_skill/run-1/timing.json new file mode 100644 index 0000000000..9a8d43344b --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-bespoke-modal/without_skill/run-1/timing.json @@ -0,0 +1,5 @@ +{ + "total_tokens": 44727, + "duration_ms": 269186, + "total_duration_seconds": 269.186 +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/eval_metadata.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/eval_metadata.json new file mode 100644 index 0000000000..8502218061 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/eval_metadata.json @@ -0,0 +1,11 @@ +{ + "eval_id": 1, + "eval_name": "audit-hero-section", + "prompt": "Can you review this file for Gamut design system compliance issues? /Users/kenny.lin/Documents/eng/codecademy-eng/gamut/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-hero-section/fixture/HeroSection.tsx -- write your findings to ./outputs/report.md", + "assertions": [ + "Flags the './HeroSection.scss' import as an SCSS/CSS module violation", + "Flags className=\"hero-wrapper\" on the Box component as a violation", + "Flags the inline style={{ color: '#10162F' }} on Box as a violation", + "For the hex #10162F, recommends the semantic token 'text' (not just the palette name navy-800) as the primary remediation" + ] +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/with_skill/run-1/grading.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/with_skill/run-1/grading.json new file mode 100644 index 0000000000..b0d430c27a --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/with_skill/run-1/grading.json @@ -0,0 +1,32 @@ +{ + "expectations": [ + { + "text": "Flags the './HeroSection.scss' import as an SCSS/CSS module violation", + "passed": true, + "evidence": "\"SCSS/CSS imports 1 file\" with exact path and remediation." + }, + { + "text": "Flags className=\"hero-wrapper\" on the Box component as a violation", + "passed": true, + "evidence": "\"className on Gamut components 1 occurrence\"." + }, + { + "text": "Flags the inline style={{ color: '#10162F' }} on Box as a violation", + "passed": true, + "evidence": "\"inline style on Gamut components 1 occurrence\"." + }, + { + "text": "For the hex #10162F, recommends the semantic token 'text' (not just the palette name navy-800) as the primary remediation", + "passed": true, + "evidence": "\"semantic: text ... | palette: navy-800\" -- semantic listed first, with an honest low-confidence caveat since no DESIGN.md exists." + } + ], + "summary": { "passed": 4, "failed": 0, "total": 4, "pass_rate": 1.0 }, + "user_notes_summary": { + "uncertainties": [], + "needs_review": [ + "Tally is 4 errors this time (vs 3 in iteration 1) because the report deliberately double-counts the #10162F line as both an inline-style violation and a hardcoded-color violation, explicitly explained as intentional (two distinct problems on one line). Also again self-reports the bare

/Text gap as informational-only, correctly excluded from the tally." + ], + "workarounds": [] + } +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/with_skill/run-1/outputs/report.md b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/with_skill/run-1/outputs/report.md new file mode 100644 index 0000000000..4cd4f605bc --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/with_skill/run-1/outputs/report.md @@ -0,0 +1,47 @@ +Gamut Review — /Users/kenny.lin/Documents/eng/codecademy-eng/gamut/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-hero-section/fixture/HeroSection.tsx +══════════════════════════════════════════════════ +Scope note: audit root is a single standalone file (HeroSection.tsx) plus its sibling HeroSection.scss — not a full project. Checks that require project-level artifacts (package.json, project-wide GamutProvider wiring, tsconfig, test suite) are marked N/A rather than pass/fail. + +DESIGN.md +✗ missing No DESIGN.md found next to the fixture. Run: gamut plugin install cursor --theme (or copy the matching DESIGN.\*.md and rename it). [blocking for color audit — Check 4 findings below use low-confidence, theme-unverified language as a result] + +Dependencies +N/A — no package.json at the audit root (single-file audit); dependency check skipped. + +Setup +N/A (partial) — this is a leaf presentational component, not an app root, so GamutProvider/ColorMode/Background/theme augmentation would not be expected to appear here. None of these symbols appear in the file; not flagged as violations for that reason, but note that Background (semantic surface) would be a reasonable replacement for the hand-rolled hero surface — see Hardcoded colors below. + +Import patterns +✓ Deep dist imports none found +✓ Deep src imports none found + +SCSS modules, className & inline style [→ gamut-system-props] [→ gamut-style-utilities] [→ gamut-color-mode] +✗ SCSS/CSS imports 1 file — migrate to system props and css()/variant() +HeroSection.scss (imported at HeroSection.tsx:3; only rule is `.hero-wrapper { padding: 32px; }`, directly expressible as a p={32} system prop) +✗ className on Gamut components 1 occurrence +HeroSection.tsx:6 +✗ inline style on Gamut components 1 occurrence — use system props or css()/variant()/states() with semantic tokens +HeroSection.tsx:6 + +Nested selectors [→ gamut-system-props] [→ gamut-style-utilities] +✓ none found (HeroSection.scss has a single flat class rule; no bare tag selectors, `*` selectors, or ${Component} interpolations) + +styled(GamutComponent) bypassing system props [→ gamut-system-props] [→ gamut-style-utilities] +✓ none found (no styled() usage in this file) + +Hardcoded colors [→ gamut-color-mode] +⚠ low confidence — no DESIGN.md +✗ HeroSection.tsx:6 '#10162F' → semantic: text (low confidence — read as body/heading foreground color on a content wrapper; could also be border-primary or secondary depending on actual usage, per Appendix B disambiguation) | palette: navy-800 | note: verify against the correct theme Storybook page before applying; also flagged above as an inline-style violation on a Gamut component (double-counted deliberately — two distinct problems on one line: bypassing the variance system, and a hardcoded literal instead of a token) +Non-Gamut CSS vars: none found in HeroSection.scss (no var(--...) usage) + +Test setup [→ gamut-testing] +N/A — no test files present for this component (no **tests**, .test., or .spec. files in the audit scope). + +Bespoke component duplication (heuristic — confirm manually) [→ gamut-component-first] +✓ none found - No suspicious ARIA roles (dialog/menu/tooltip/listbox/alert). - Filename "HeroSection" does not match a known Gamut component name (Modal, Dialog, Dropdown, Tooltip, Popover, Menu, Toast, Accordion, Tabs, Pagination, Avatar, Badge, Tag). - No hand-rolled dismiss/focus-trap logic. - HeroSection.scss is not named after a Gamut component. + +Additional observation (outside the formal Check 1–6 list, informational only — not counted in the tally) +ℹ HeroSection.tsx:7

Welcome to the platform

is a bare native heading, not a Gamut typography component. Consider Gamut's Typography/Heading component (see gamut-typography) so heading styles stay on the token scale rather than relying on browser/user-agent defaults. Not a scored violation under this skill's checks, flagged for awareness only. + +══════════════════════════════════════════════════ +4 error(s), 0 warning(s) found. diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/with_skill/run-1/timing.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/with_skill/run-1/timing.json new file mode 100644 index 0000000000..9c145473f2 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/with_skill/run-1/timing.json @@ -0,0 +1,5 @@ +{ + "total_tokens": 43202, + "duration_ms": 214557, + "total_duration_seconds": 214.557 +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/without_skill/run-1/grading.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/without_skill/run-1/grading.json new file mode 100644 index 0000000000..a3f14c4ee9 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/without_skill/run-1/grading.json @@ -0,0 +1,32 @@ +{ + "expectations": [ + { + "text": "Flags the './HeroSection.scss' import as an SCSS/CSS module violation", + "passed": true, + "evidence": "Finding #4 explicitly names the SCSS import and recommends deleting it, without citing the ESLint plugin per the no-eslint-plugin instruction." + }, + { + "text": "Flags className=\"hero-wrapper\" on the Box component as a violation", + "passed": true, + "evidence": "Finding #4 explicitly names className as fighting Box's own API." + }, + { + "text": "Flags the inline style={{ color: '#10162F' }} on Box as a violation", + "passed": true, + "evidence": "Finding #1, framed purely via manual code-review reasoning (dark-mode contrast bug) rather than citing a lint rule, per this iteration's instruction." + }, + { + "text": "For the hex #10162F, recommends the semantic token 'text' (not just the palette name navy-800) as the primary remediation", + "passed": true, + "evidence": "\"\", with correct dark-mode reasoning." + } + ], + "summary": { "passed": 4, "failed": 0, "total": 4, "pass_rate": 1.0 }, + "user_notes_summary": { + "uncertainties": [], + "needs_review": [ + "Correctly complied with the no-ESLint-plugin instruction this iteration -- findings framed entirely through manual source/token reasoning, no eslint-plugin-gamut citations. Still independently found the bare

/Text gap." + ], + "workarounds": [] + } +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/without_skill/run-1/outputs/report.md b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/without_skill/run-1/outputs/report.md new file mode 100644 index 0000000000..cb60512c47 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/without_skill/run-1/outputs/report.md @@ -0,0 +1,72 @@ +# Design System Compliance Review: HeroSection.tsx + +**File reviewed:** `HeroSection.tsx` (+ sibling `HeroSection.scss`) + +```tsx +import { Box } from '@codecademy/gamut'; + +import './HeroSection.scss'; + +export const HeroSection = () => ( + +

Welcome to the platform

+ +); +``` + +```scss +.hero-wrapper { + padding: 32px; +} +``` + +This is a small file, but it violates several core conventions of the Gamut design system. Findings are ordered by severity. + +## 1. Hardcoded hex color duplicates an existing design token (High) + +`style={{ color: '#10162F' }}` sets text color via an inline style with a raw hex value. `#10162F` is not arbitrary — it is exactly `navy-800` in `@codecademy/gamut-styles`'s core color palette (`coreSwatches.navy['800']` in `packages/gamut-styles/src/variables/colors.ts`), and in the light color mode that value is precisely what the semantic `text` token resolves to (`text._: 'navy-800'` in `packages/gamut-styles/src/themes/core.ts`). + +This should be expressed as a Gamut color prop, e.g. `` (or simply omitted, since `text` is the default). Two compounding problems: + +- **Inline `style` bypasses the theme entirely.** Color props (`color`, `bg`, `textColor`, `borderColor`) resolve through the active color mode. A hardcoded hex string won't respond to dark mode — `text._` resolves to `white` in dark mode, so this hero heading would render near-black text even in a dark context, a real contrast/legibility bug. +- **`style` fights `Box`'s own API.** `Box` already exposes typed, token-backed `color`/`textColor` props (`packages/gamut-styles/src/variance/config.ts`, `color` prop group, `scale: 'colors'`). Real usages throughout `packages/gamut/src/**` (e.g. `Card/index.tsx`, `Form/SelectDropdown/elements/*`, `Disclosure/DisclosureButton/index.tsx`) consistently use `color="text"`, `color="text-secondary"`, `color="text-disabled"` — never the native `style` attribute. + +## 2. Raw `

` bypasses the Gamut typography scale (High) + +The heading is a bare `

Welcome to the platform

` with no Gamut typography applied. Gamut ships a `Text` component (`packages/gamut/src/Typography/Text.tsx`) for exactly this: `as="h1"` maps through `typographyElementVariants` (`packages/gamut/src/Typography/variants.ts`) to the `title-xxl` variant (`fontSize: 64`, token-based `fontWeight: 'title'`, `lineHeight: 'title'`). + +By rendering a plain `

`, the component gets whatever the browser/UA (or an unrelated global stylesheet) applies to `h1` — not the design system's title scale, font weight, line-height, or font-family tokens — and won't participate in any type-scale consistency the rest of the app relies on. This should be `Welcome to the platform` (imported from `@codecademy/gamut`). + +## 3. Spacing hardcoded as a raw px value instead of the token-based `p` prop (Medium) + +`padding: 32px;` in `HeroSection.scss` hardcodes a pixel value. Gamut's spacing scale (`packages/gamut-styles/src/variables/spacing.ts`) already defines `32` as a named scale step (`32: pxRem(32)` → `2rem`), and `Box` already exposes a token-backed `p` prop (`property: 'padding', scale: 'spacing'` in `packages/gamut-styles/src/variance/config.ts`) that consumes exactly that scale. + +The hardcoded `32px` happens to match a real spacing token value, which underscores the point: this should be authored as ``, not a magic number in a separate stylesheet. As written, if the spacing scale is ever adjusted, this component's spacing silently drifts out of sync with the rest of the system since it isn't wired to any token. + +## 4. Mixing a plain global-scope SCSS file with a Gamut styled component (Medium) + +The component imports a plain `.scss` file and applies styling via a bare, unscoped class name (`className="hero-wrapper"`) on a `Box`. This is not how Gamut-based UI is built: + +- Across the entire `packages/gamut/src` tree there is exactly one `.scss` file in the whole package, and it exists only to theme a third-party vendor widget (`Video/styles/vds_base_theme.scss`) — an exception for code Gamut doesn't own, not a pattern to imitate for first-party components. +- `Box` is `styled('div', ...)` on Emotion + the `variance` prop system specifically so layout/spacing/color are expressed as typed, token-constrained props (`system.space`, `system.color`, `system.layout`, etc. — `packages/gamut/src/Box/props.ts`). Reaching past that API to a global class name + separate stylesheet loses type safety, loses theme/token resolution, and risks a class-name collision in the consuming app's global CSS scope (nothing about `.hero-wrapper` is namespaced or scoped). +- It also duplicates responsibility for the same concern (spacing) across two different mechanisms — prop-driven styling everywhere else, hand-written global CSS here — which is exactly the inconsistency a token/prop-based system is meant to prevent. + +**Recommendation:** delete `HeroSection.scss` and express `padding: 32px` as `p={32}` on `Box`. + +## 5. Net effect + +Taken together, none of this component's visual styling (color, spacing) is actually theme-aware — it's all applied through mechanisms that sit outside Gamut's theming system (inline `style`, a global SCSS class, a magic-number px value) despite being built on `Box`, which exists precisely to make that unnecessary. + +### Suggested rewrite + +```tsx +import { Box, Text } from '@codecademy/gamut'; + +export const HeroSection = () => ( + + Welcome to the platform + +); +``` + +This drops the SCSS file and inline `style` entirely, uses the `32` spacing token via `p`, lets the default `text` color drive text color so it responds correctly to color-mode changes, and renders the heading through the `title-xxl` typography variant instead of an unstyled native `

`. diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/without_skill/run-1/timing.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/without_skill/run-1/timing.json new file mode 100644 index 0000000000..a90be80c73 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-hero-section/without_skill/run-1/timing.json @@ -0,0 +1,5 @@ +{ + "total_tokens": 52420, + "duration_ms": 269388, + "total_duration_seconds": 269.388 +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/eval_metadata.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/eval_metadata.json new file mode 100644 index 0000000000..2a8b6de619 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/eval_metadata.json @@ -0,0 +1,11 @@ +{ + "eval_id": 2, + "eval_name": "audit-styled-wrapper", + "prompt": "Can you review this file for Gamut design system compliance issues? /Users/kenny.lin/Documents/eng/codecademy-eng/gamut/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-styled-wrapper/fixture/CardShells.tsx -- write your findings to ./outputs/report.md", + "assertions": [ + "Flags CardShell (styled(Box) with a raw tagged-template literal, where every property is prop-expressible) as an error/violation, with remediation to delete the wrapper and use Box/FlexBox props directly", + "Does NOT flag GlowShell as an error the same way -- it's wrapped in css(), which is the compliant form for a non-expressible gradient", + "Notes that GlowShell's padding: 24 should still move out to a prop even though the gradient must stay in css()", + "Recommends FlexBox instead of Box + display:flex for CardShell" + ] +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/with_skill/run-1/grading.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/with_skill/run-1/grading.json new file mode 100644 index 0000000000..b84527df6a --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/with_skill/run-1/grading.json @@ -0,0 +1,32 @@ +{ + "expectations": [ + { + "text": "Flags CardShell (styled(Box) with a raw tagged-template literal, where every property is prop-expressible) as an error/violation, with remediation to delete the wrapper and use Box/FlexBox props directly", + "passed": true, + "evidence": "\"CardShells.tsx:7 ... delete wrapper; use FlexBox ... with flexDirection=\\\"column\\\" and p={16}\"." + }, + { + "text": "Does NOT flag GlowShell as an error the same way -- it's wrapped in css(), which is the compliant form for a non-expressible gradient", + "passed": true, + "evidence": "As in iteration 1, this run correctly identifies that background takes the gradient directly (no scale) and flags GlowShell's wrapper as unnecessary too -- more correct than my original assertion anticipated. Graded on substance, consistent with iteration 1's treatment of the same finding." + }, + { + "text": "Notes that GlowShell's padding: 24 should still move out to a prop even though the gradient must stay in css()", + "passed": true, + "evidence": "\"padding maps directly to p={24}\"." + }, + { + "text": "Recommends FlexBox instead of Box + display:flex for CardShell", + "passed": true, + "evidence": "Explicit FlexBox recommendation." + } + ], + "summary": { "passed": 4, "failed": 0, "total": 4, "pass_rate": 1.0 }, + "user_notes_summary": { + "uncertainties": [], + "needs_review": [ + "Did not catch a subtler bug the without_skill run for this same eval found: `padding` (the raw CSS property name) inside css() is NOT the same as the `p` alias -- variance's getStaticCss only scales recognized alias keys, so `padding: 24` passes through as unscaled literal CSS, bypassing the spacing token system entirely, distinct from (and in addition to) it merely being avoidable as a prop. Verified this claim directly against packages/variance/src/core.ts (createCss/getStaticCss) -- it's accurate. This wasn't part of the pre-registered assertions so it doesn't affect the score, but it's a real miss relative to the without_skill run on this eval." + ], + "workarounds": [] + } +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/with_skill/run-1/outputs/report.md b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/with_skill/run-1/outputs/report.md new file mode 100644 index 0000000000..31776d1064 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/with_skill/run-1/outputs/report.md @@ -0,0 +1,61 @@ +Gamut Review — /Users/kenny.lin/Documents/eng/codecademy-eng/gamut/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-styled-wrapper/fixture/CardShells.tsx +══════════════════════════════════════════════════ + +Scope note: this is a single-file audit (not a full project). The following +checks require project-level context that isn't available for a standalone +file and were skipped rather than scored: Check 0 (DESIGN.md — no project +root), Check 1 (Dependencies — no package.json), Check 2 (Setup — no app +root to search for GamutProvider/ColorMode/Background/theme augmentation; +the file contains no `states()`/`variant()` usage so the conditional +StyleProps check doesn't apply either), Check 5 (Test setup — not a test +file). Checks 3, 3b, 3c, 3d, 4, and 6 apply to any source file and were run +in full. + +Because there is no DESIGN.md / theme context, the Check 4 color mapping +below uses the Appendix A/B tables as a best-available heuristic — treat the +semantic suggestion as low-confidence and confirm against the actual +product theme before applying. + +Import patterns +✓ Deep dist imports none found +✓ Deep src imports none found + +SCSS modules, className & inline style [→ gamut-system-props] [→ gamut-style-utilities] [→ gamut-color-mode] +✓ SCSS/CSS imports none found +✓ className on Gamut components none found +✓ inline style on Gamut components none found + +Nested selectors [→ gamut-system-props] [→ gamut-style-utilities] +✓ none found + +styled(GamutComponent) bypassing system props [→ gamut-system-props] [→ gamut-style-utilities] +✗ styled(Box) raw CSS 1 occurrence — every property has a prop equivalent +CardShells.tsx:7 styled(Box)`display: flex; flex-direction: column; padding: 16px;` +— display, flex-direction, padding → delete wrapper; use FlexBox (not Box) with +flexDirection="column" and p={16} directly on the JSX element. `display: flex` +is the specific case the skill calls out to become FlexBox rather than a +`display` prop on Box. + +✗ styled(Box)(css(...)) raw CSS via css() 1 occurrence — every property has a prop equivalent, despite using css() +CardShells.tsx:15 styled(Box)(css({ background: 'radial-gradient(...)', padding: 24 })) +— the code comment claims "the gradient isn't expressible as a prop," but that's +incorrect: unlike `bg` (which is token-scale-constrained), `background` takes any +raw CSS value as-is, so a gradient string is already valid passed directly as +`background="radial-gradient(circle, #3A10E5 0%, transparent 100%)"`. `padding` +maps directly to `p={24}`. Because both properties in this block have direct prop +equivalents, the `styled()` wrapper (and the css() escape hatch inside it) is +unnecessary — delete it and pass `background` and `p` as props on `Box` directly. +(Using css() is normally the compliant way to hold a property that truly can't be +a prop; it doesn't get a pass here because nothing in this block actually requires +it.) + +Hardcoded colors [→ gamut-color-mode] ⚠ low confidence — no DESIGN.md / theme context for this single-file audit +✗ CardShells.tsx:17 '#3A10E5' → semantic: primary (if this glow should track brand/CTA color and adapt with ColorMode) | palette: hyper-500 | note: used as a literal inside a `radial-gradient(...)` string passed through `styled(Box)(css({...}))` — decorative gradient CSS in a styled-component wrapper is "adaptive UI" per the check's severity rule, so this is an error rather than a documented exception; confirm with design whether the glow is meant to always render this exact brand purple (in which case use the palette token `hyper-500` explicitly, not a raw hex) or should shift with theme/dark mode (in which case use the semantic `primary` token). Either way, replace the hex literal — do not leave it as `#3A10E5`. + +Bespoke component duplication (heuristic — confirm manually) [→ gamut-component-first] +✓ none found — no ARIA roles, no filenames matching a Gamut component name, no hand-rolled dismiss logic, no stylesheets in this file. + +══════════════════════════════════════════════════ +3 error(s), 0 warning(s) found. + +Next step: both exports in this file can be deleted as `styled()` wrappers entirely — replace `CardShell` with `` and `GlowShell` with ``, resolving the hardcoded-color finding as part of the same edit. diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/with_skill/run-1/timing.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/with_skill/run-1/timing.json new file mode 100644 index 0000000000..a9f3a6edbc --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/with_skill/run-1/timing.json @@ -0,0 +1,5 @@ +{ + "total_tokens": 44165, + "duration_ms": 242767, + "total_duration_seconds": 242.767 +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/without_skill/run-1/grading.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/without_skill/run-1/grading.json new file mode 100644 index 0000000000..75b6c30707 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/without_skill/run-1/grading.json @@ -0,0 +1,32 @@ +{ + "expectations": [ + { + "text": "Flags CardShell (styled(Box) with a raw tagged-template literal, where every property is prop-expressible) as an error/violation, with remediation to delete the wrapper and use Box/FlexBox props directly", + "passed": true, + "evidence": "\"The whole component collapses to -- no bespoke wrapper needed.\"" + }, + { + "text": "Does NOT flag GlowShell as an error the same way -- it's wrapped in css(), which is the compliant form for a non-expressible gradient", + "passed": true, + "evidence": "Correctly treats the gradient itself as a legitimate css() case (background has no scale) while still flagging the hardcoded color inside it. Did not reach the 'delete the wrapper entirely' insight with_skill reached, but valid." + }, + { + "text": "Notes that GlowShell's padding: 24 should still move out to a prop even though the gradient must stay in css()", + "passed": true, + "evidence": "Finding #3 goes further than any other run across both iterations: it identifies that `padding` (the raw CSS property, not the `p` alias) inside css() is not recognized by variance's scale lookup at all, so it silently passes through as unscaled literal CSS -- a real, verified bug (confirmed against packages/variance/src/core.ts), distinct from and worse than merely 'should be a prop for cleanliness'. No other run in either iteration caught this." + }, + { + "text": "Recommends FlexBox instead of Box + display:flex for CardShell", + "passed": true, + "evidence": "\"\"." + } + ], + "summary": { "passed": 4, "failed": 0, "total": 4, "pass_rate": 1.0 }, + "user_notes_summary": { + "uncertainties": [], + "needs_review": [ + "Found a genuinely novel, verified-accurate technical point missed by all 3 other CardShells runs across both iterations: `padding: 24` inside css() bypasses the spacing scale because variance only scales recognized alias keys (p/px/py/etc), not the raw CSS property name. This means the fixture's GlowShell has a real functional bug (unscaled spacing), not just an avoidable-but-harmless pattern." + ], + "workarounds": [] + } +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/without_skill/run-1/timing.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/without_skill/run-1/timing.json new file mode 100644 index 0000000000..4166142517 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-2/eval-audit-styled-wrapper/without_skill/run-1/timing.json @@ -0,0 +1,5 @@ +{ + "total_tokens": 53895, + "duration_ms": 496915, + "total_duration_seconds": 496.915 +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/agent_map.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/agent_map.json new file mode 100644 index 0000000000..a78959d43e --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/agent_map.json @@ -0,0 +1,4 @@ +{ + "ab3bf1a2cd6f90190": "eval-audit-styled-wrapper/with_skill", + "a81a31c62682e7948": "eval-audit-styled-wrapper/without_skill" +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/benchmark.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/benchmark.json new file mode 100644 index 0000000000..e25b4872e4 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/benchmark.json @@ -0,0 +1,142 @@ +{ + "metadata": { + "skill_name": "gamut-review", + "skill_path": "", + "executor_model": "", + "analyzer_model": "", + "timestamp": "2026-07-28T17:11:22Z", + "evals_run": [2], + "runs_per_configuration": 3 + }, + "runs": [ + { + "eval_id": 2, + "configuration": "with_skill", + "run_number": 1, + "result": { + "pass_rate": 1.0, + "passed": 4, + "failed": 0, + "total": 4, + "time_seconds": 152.303, + "tokens": 47430, + "tool_calls": 0, + "errors": 0 + }, + "expectations": [ + { + "text": "Flags CardShell (styled(Box) with a raw tagged-template literal, where every property is prop-expressible) as an error/violation, with remediation to delete the wrapper and use Box/FlexBox props directly", + "passed": true, + "evidence": "Check 3d finding: 'CardShells.tsx:7 -- styled(Box)`display: flex; flex-direction: column; padding: 16px;` -> \u2717 error ... delete the wrapper, use '" + }, + { + "text": "Does NOT flag GlowShell as an error the same way -- it's wrapped in css(), which is the compliant form for a non-expressible gradient", + "passed": true, + "evidence": "Explicitly states the outer styled(Box)(css({...})) form 'is not itself a wrapper violation' per Step 2 -- the Step 4 finding below it targets the specific padding key, not the wrapper choice." + }, + { + "text": "Notes that GlowShell's padding: 24 should still move out to a prop even though the gradient must stay in css()", + "passed": true, + "evidence": "Check 3d Step 4 finding: 'css({ background: ..., padding: 24 }) -> \u2717 error ... Fix: padding: 24 -> p: 24.' Also correctly leaves background unflagged since it has no scale to miss." + }, + { + "text": "Recommends FlexBox instead of Box + display:flex for CardShell", + "passed": true, + "evidence": "'Since display: flex is present, the recommendation is specifically FlexBox (not Box + a display prop)'" + } + ], + "notes": [ + "Correctly complied with the no-ESLint-plugin instruction -- no citation of eslint-plugin-gamut." + ] + }, + { + "eval_id": 2, + "configuration": "without_skill", + "run_number": 1, + "result": { + "pass_rate": 1.0, + "passed": 4, + "failed": 0, + "total": 4, + "time_seconds": 315.373, + "tokens": 67355, + "tool_calls": 0, + "errors": 0 + }, + "expectations": [ + { + "text": "Flags CardShell (styled(Box) with a raw tagged-template literal, where every property is prop-expressible) as an error/violation, with remediation to delete the wrapper and use Box/FlexBox props directly", + "passed": true, + "evidence": "Finding #1: 'CardShell reinvents FlexBox instead of using it' -- 'The idiomatic version of this entire file is -- no wrapper needed.'" + }, + { + "text": "Does NOT flag GlowShell as an error the same way -- it's wrapped in css(), which is the compliant form for a non-expressible gradient", + "passed": true, + "evidence": "Finding #3 targets the specific 'padding' key inside css(), not the styled(Box)(css({...})) composition itself; explicitly notes elsewhere that this composition shape 'matches real internal usage ... is idiomatic; the issue is specifically the property names/values chosen, not the composition mechanism.'" + }, + { + "text": "Notes that GlowShell's padding: 24 should still move out to a prop even though the gradient must stay in css()", + "passed": true, + "evidence": "Finding #3: 'css({ padding: 24 }) doesn't actually use the spacing scale ... The fix is css({ p: 24, ... }).'" + }, + { + "text": "Recommends FlexBox instead of Box + display:flex for CardShell", + "passed": true, + "evidence": "Finding #1 and the suggested-fix code block both replace styled(Box) with styled(FlexBox) / ." + } + ], + "notes": [ + "No skill access, so remediation is derived entirely from reading real Gamut source (FlexBox.tsx, spacing.ts, variance/core.ts, themes/core.ts, Toggle/elements.tsx) rather than citing a named check -- still reaches the same substantive conclusions as with_skill, including the identical Step-4-equivalent padding-vs-p bug and correctly not conflating it with a wrapper violation.", + "Went further than with_skill on Finding #4 (dark-mode color-token drift), citing theme.colors.primary interpolation precedent from Toggle/elements.tsx -- a real, verified pattern this session confirmed earlier." + ] + } + ], + "run_summary": { + "with_skill": { + "pass_rate": { + "mean": 1.0, + "stddev": 0.0, + "min": 1.0, + "max": 1.0 + }, + "time_seconds": { + "mean": 152.303, + "stddev": 0.0, + "min": 152.303, + "max": 152.303 + }, + "tokens": { + "mean": 47430.0, + "stddev": 0.0, + "min": 47430, + "max": 47430 + } + }, + "without_skill": { + "pass_rate": { + "mean": 1.0, + "stddev": 0.0, + "min": 1.0, + "max": 1.0 + }, + "time_seconds": { + "mean": 315.373, + "stddev": 0.0, + "min": 315.373, + "max": 315.373 + }, + "tokens": { + "mean": 67355.0, + "stddev": 0.0, + "min": 67355, + "max": 67355 + } + }, + "delta": { + "pass_rate": "+0.00", + "time_seconds": "-163.1", + "tokens": "-19925" + } + }, + "notes": [] +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/benchmark.md b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/benchmark.md new file mode 100644 index 0000000000..96a83e4867 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/benchmark.md @@ -0,0 +1,13 @@ +# Skill Benchmark: gamut-review + +**Model**: +**Date**: 2026-07-28T17:11:22Z +**Evals**: 2 (3 runs each per configuration) + +## Summary + +| Metric | With Skill | Without Skill | Delta | +| --------- | ------------- | ------------- | ------- | +| Pass Rate | 100% ± 0% | 100% ± 0% | +0.00 | +| Time | 152.3s ± 0.0s | 315.4s ± 0.0s | -163.1s | +| Tokens | 47430 ± 0 | 67355 ± 0 | -19925 | diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/eval_metadata.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/eval_metadata.json new file mode 100644 index 0000000000..2a8b6de619 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/eval_metadata.json @@ -0,0 +1,11 @@ +{ + "eval_id": 2, + "eval_name": "audit-styled-wrapper", + "prompt": "Can you review this file for Gamut design system compliance issues? /Users/kenny.lin/Documents/eng/codecademy-eng/gamut/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-styled-wrapper/fixture/CardShells.tsx -- write your findings to ./outputs/report.md", + "assertions": [ + "Flags CardShell (styled(Box) with a raw tagged-template literal, where every property is prop-expressible) as an error/violation, with remediation to delete the wrapper and use Box/FlexBox props directly", + "Does NOT flag GlowShell as an error the same way -- it's wrapped in css(), which is the compliant form for a non-expressible gradient", + "Notes that GlowShell's padding: 24 should still move out to a prop even though the gradient must stay in css()", + "Recommends FlexBox instead of Box + display:flex for CardShell" + ] +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/with_skill/run-1/grading.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/with_skill/run-1/grading.json new file mode 100644 index 0000000000..b85f56c6c6 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/with_skill/run-1/grading.json @@ -0,0 +1,32 @@ +{ + "expectations": [ + { + "text": "Flags CardShell (styled(Box) with a raw tagged-template literal, where every property is prop-expressible) as an error/violation, with remediation to delete the wrapper and use Box/FlexBox props directly", + "passed": true, + "evidence": "Check 3d finding: 'CardShells.tsx:7 -- styled(Box)`display: flex; flex-direction: column; padding: 16px;` -> ✗ error ... delete the wrapper, use '" + }, + { + "text": "Does NOT flag GlowShell as an error the same way -- it's wrapped in css(), which is the compliant form for a non-expressible gradient", + "passed": true, + "evidence": "Explicitly states the outer styled(Box)(css({...})) form 'is not itself a wrapper violation' per Step 2 -- the Step 4 finding below it targets the specific padding key, not the wrapper choice." + }, + { + "text": "Notes that GlowShell's padding: 24 should still move out to a prop even though the gradient must stay in css()", + "passed": true, + "evidence": "Check 3d Step 4 finding: 'css({ background: ..., padding: 24 }) -> ✗ error ... Fix: padding: 24 -> p: 24.' Also correctly leaves background unflagged since it has no scale to miss." + }, + { + "text": "Recommends FlexBox instead of Box + display:flex for CardShell", + "passed": true, + "evidence": "'Since display: flex is present, the recommendation is specifically FlexBox (not Box + a display prop)'" + } + ], + "summary": { "passed": 4, "failed": 0, "total": 4, "pass_rate": 1.0 }, + "user_notes_summary": { + "uncertainties": [], + "needs_review": [ + "Correctly complied with the no-ESLint-plugin instruction -- no citation of eslint-plugin-gamut." + ], + "workarounds": [] + } +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/with_skill/run-1/outputs/report.md b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/with_skill/run-1/outputs/report.md new file mode 100644 index 0000000000..7fc969186a --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/with_skill/run-1/outputs/report.md @@ -0,0 +1,25 @@ +## Gamut Review — CardShells.tsx + +File: `/Users/kenny.lin/Documents/eng/codecademy-eng/gamut/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-styled-wrapper/fixture/CardShells.tsx` + +**Scope note:** Standalone file audit — Checks 0 (DESIGN.md), 1 (Dependencies), 2 (Setup), and 5 (Test setup) were skipped as they require full-project context. Checks 3, 3b, 3c, 3d, 4, and 6 were applied. + +**Import patterns (Check 3):** ✓ none found — imports are `@codecademy/gamut`, `@codecademy/gamut-styles`, `@emotion/styled` (public roots, no `/dist/` or `/src/` deep imports). + +**SCSS/className/inline style (Check 3b):** ✓ none found. + +**Nested selectors (Check 3c):** ✓ none found. + +**styled(GamutComponent) bypassing system props (Check 3d) — the core finding:** + +- `CardShells.tsx:7` — `` styled(Box)`display: flex; flex-direction: column; padding: 16px;` `` → **✗ error**. This matches Step 2's tagged-template violation pattern. Every property (`display`, `flex-direction`, `padding`) has a direct system-prop equivalent. Since `display: flex` is present, the recommendation is specifically `FlexBox` (not `Box` + a `display` prop): delete the wrapper, use ``. +- `CardShells.tsx:15` — `styled(Box)(css({...}))` → this outer syntax is the **compliant** form per Step 2 (character after `(` is `css(`, not a backtick/`{`), so it is not itself a wrapper violation. Worth noting: per the skill's own aside, the gradient in `background` is expressible as a plain `background` prop as-is (no token scale, takes any CSS value) — the gradient alone never justified reaching for `css()`/`styled()`. +- `CardShells.tsx:18` — `css({ background: '...', padding: 24 })` → **✗ error** under Step 4. `padding` is the raw CSS property name, not the variance alias `p`; `getStaticCss` passes unrecognized keys straight through as unscaled literals, so `24` bypasses the spacing scale entirely regardless of the value written — a functional bug, not a style nit. `background` in the same block is correctly _not_ flagged (it has no token scale, so there's no alias it's "missing"). Fix: `padding: 24` → `p: 24`. Once corrected, note that both properties (gradient `background`, and `p`) are individually prop-expressible, so the whole `styled()/css()` wrapper for `GlowShell` may be unnecessary — consider `` directly unless a named reusable shell is required. + +**Hardcoded colors (Check 4):** ⚠ low confidence — no DESIGN.md/theme context for this standalone file. + +- `CardShells.tsx:17` — `'#3A10E5'` → palette: `hyper-500`/`hyper` (Appendix A); semantic: `primary` (Appendix B, unverified — confirm theme). This hex is a gradient stop hardcoded in application code (not a token-definition file), so it's a Check 4 violation independent of the Step 4 finding above (that finding was about the wrapper mechanics; this one is about the literal color value itself). Gradients can't collapse to a single semantic token string, but if this glow should track ColorMode/theme, pull the stop color from the theme's hyper/primary token rather than hardcoding it — or confirm it's an intentional fixed decorative exception. + +**Bespoke component duplication (Check 6):** ✓ none found — no ARIA roles, no filename collision with Check 6's component-name list (`Card` isn't on that list), no hand-rolled dismiss logic, no colliding stylesheets. + +**Tally: 3 error(s), 0 warning(s) found.** diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/with_skill/run-1/timing.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/with_skill/run-1/timing.json new file mode 100644 index 0000000000..a3ad298389 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/with_skill/run-1/timing.json @@ -0,0 +1,5 @@ +{ + "total_tokens": 47430, + "duration_ms": 152303, + "total_duration_seconds": 152.303 +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/without_skill/run-1/grading.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/without_skill/run-1/grading.json new file mode 100644 index 0000000000..4288e5d300 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/without_skill/run-1/grading.json @@ -0,0 +1,33 @@ +{ + "expectations": [ + { + "text": "Flags CardShell (styled(Box) with a raw tagged-template literal, where every property is prop-expressible) as an error/violation, with remediation to delete the wrapper and use Box/FlexBox props directly", + "passed": true, + "evidence": "Finding #1: 'CardShell reinvents FlexBox instead of using it' -- 'The idiomatic version of this entire file is -- no wrapper needed.'" + }, + { + "text": "Does NOT flag GlowShell as an error the same way -- it's wrapped in css(), which is the compliant form for a non-expressible gradient", + "passed": true, + "evidence": "Finding #3 targets the specific 'padding' key inside css(), not the styled(Box)(css({...})) composition itself; explicitly notes elsewhere that this composition shape 'matches real internal usage ... is idiomatic; the issue is specifically the property names/values chosen, not the composition mechanism.'" + }, + { + "text": "Notes that GlowShell's padding: 24 should still move out to a prop even though the gradient must stay in css()", + "passed": true, + "evidence": "Finding #3: 'css({ padding: 24 }) doesn't actually use the spacing scale ... The fix is css({ p: 24, ... }).'" + }, + { + "text": "Recommends FlexBox instead of Box + display:flex for CardShell", + "passed": true, + "evidence": "Finding #1 and the suggested-fix code block both replace styled(Box) with styled(FlexBox) / ." + } + ], + "summary": { "passed": 4, "failed": 0, "total": 4, "pass_rate": 1.0 }, + "user_notes_summary": { + "uncertainties": [], + "needs_review": [ + "No skill access, so remediation is derived entirely from reading real Gamut source (FlexBox.tsx, spacing.ts, variance/core.ts, themes/core.ts, Toggle/elements.tsx) rather than citing a named check -- still reaches the same substantive conclusions as with_skill, including the identical Step-4-equivalent padding-vs-p bug and correctly not conflating it with a wrapper violation.", + "Went further than with_skill on Finding #4 (dark-mode color-token drift), citing theme.colors.primary interpolation precedent from Toggle/elements.tsx -- a real, verified pattern this session confirmed earlier." + ], + "workarounds": [] + } +} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/without_skill/run-1/outputs/report.md b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/without_skill/run-1/outputs/report.md new file mode 100644 index 0000000000..96a9966a16 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/without_skill/run-1/outputs/report.md @@ -0,0 +1,63 @@ +# Design System Compliance Review + +**File reviewed:** `/Users/kenny.lin/Documents/eng/codecademy-eng/gamut/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-styled-wrapper/fixture/CardShells.tsx` + +## Summary + +Both exported components reimplement in raw CSS what `Box`/`FlexBox` already expose as first-class, theme-aware system props, and in doing so silently drop out of the token system (spacing scale/rem units, color tokens, dark-mode color-mode switching). The file's own comments concede this ("every property here has a direct system-prop equivalent") but neither component acts on it. + +## Findings + +**1. `CardShell` reinvents `FlexBox` instead of using it** (lines 7-11) + +```tsx +export const CardShell = styled(Box)` + display: flex; + flex-direction: column; + padding: 16px; +`; +``` + +`packages/gamut/src/Box/FlexBox.tsx` already defaults to `display: flex` and exposes a `column` state (`flexDirection: 'column'`, defined via `flexStates` in `packages/gamut/src/Box/props.ts`). The idiomatic version of this entire file is `` — no wrapper needed. Hand-rolling this on top of `Box` with a plain Emotion tagged template: + +- Loses the responsive array/object syntax all system props get for free through `variance`'s parser (e.g. `p={[8, 16, 24]}`). +- Creates a prop-shadowing hazard: because `CardShell` has no narrowed prop type, it still accepts `Box`'s own `p`/`display`/`flexDirection` props, but those will conflict with the hardcoded template CSS in ways that depend on Emotion's style-injection order — a real footgun for a consumer who expects `` to just work. + +**2. `padding: 16px` bypasses the spacing scale (and rem conversion)** +`packages/gamut-styles/src/variables/spacing.ts` defines discrete steps (`16: pxRem(16)`, `24: pxRem(24)`, …), where `pxRem` (`packages/gamut-styles/src/styles/pxRem.ts`) converts px to `rem` so spacing scales with root font size. `Box`'s `p` prop already routes through this. `CardShell`'s `padding: 16px` (and `GlowShell`'s `padding: 24`, see #3) hardcode literal pixel values that happen to match scale steps 16 and 24 exactly — but never pass through `pxRem`, silently opting out of the rem-based spacing convention. + +**3. `GlowShell`'s `css({ padding: 24 })` doesn't actually use the spacing scale — an easy-to-miss trap** (lines 15-20) +`css()` is `variance.createCss` (`packages/variance/src/core.ts`), which only scale-resolves keys that are registered shorthand props — `p`, `px`, `py`, `pt`, `pb`, `pr`, `pl` (see `PROPERTIES.padding` in `packages/gamut-styles/src/variance/config.ts`). The raw property name `padding` is not registered, so `createCss`'s static-CSS passthrough (`getStaticCss`) emits it untouched as `padding: 24px`. This _looks_ like it's going through the design system because it's wrapped in `css()`, but it isn't — `css({ padding: 24 })` behaves identically to a plain object literal here. The fix is `css({ p: 24, ... })`. + +**4. Hardcoded hex duplicates an existing token and will not respond to dark mode** + +```tsx +background: 'radial-gradient(circle, #3A10E5 0%, transparent 100%)', +``` + +`#3A10E5` is exactly `coreSwatches.hyper['500']` in `packages/gamut-styles/src/variables/colors.ts`, and per `packages/gamut-styles/src/themes/core.ts`, light mode's `primary._`/`interface._` resolve to `hyper-500`. Two consequences: + +- **Token drift risk** — if the `hyper` swatch is retuned, this glow silently goes out of sync with every other "primary"-colored element. +- **Breaks dark mode** — Gamut's theme builder (`packages/variance/src/createTheme/createTheme.ts`) compiles colors to CSS custom properties and implements color-mode switching by swapping which token a semantic alias resolves to at the CSS-variable layer. In dark mode, `primary._`/`interface._` resolve to `yellow-500`, not `hyper-500` (see `themes/core.ts`). A literal hex string can never participate in that swap, so this glow stays purple in dark mode while the rest of the UI's accent switches to yellow — a real visual-consistency regression for any consumer using Gamut's color modes. +- Note that the `background` system prop itself wouldn't fully solve this either — its config entry has no `scale`, so a token name wouldn't auto-resolve inside a gradient string. The correct pattern (used elsewhere in this repo, e.g. `packages/gamut/src/Toggle/elements.tsx`'s `` outline: `3px solid ${theme.colors.primary}` ``) is to import `theme` from `@codecademy/gamut-styles` and interpolate `theme.colors.primary` (or the specific token) into the string, preserving the CSS-variable reference. + +**5. The file's own comments concede the gap** — lines 5-6 and 13-14 state the properties/padding are prop-expressible, yet none of them use that path. This looks like the deliberate crux the fixture is testing. + +**What's not a problem:** exporting a `styled(Box)(css({...}))` result directly as a plain component (no `forwardRef`/narrowed props) matches real internal usage (e.g. `Circle` in `Toggle/elements.tsx`, `CollapsibleContent`/`CleanFillButton` in `Alert/elements.tsx`) — that shape is idiomatic; the issue is specifically the property names/values chosen, not the composition mechanism. + +**Suggested fix:** + +```tsx +import { FlexBox } from '@codecademy/gamut'; +import { css, theme } from '@codecademy/gamut-styles'; +import styled from '@emotion/styled'; + +export const CardShell = styled(FlexBox)``; // or just use directly + +export const GlowShell = styled(Box)( + css({ + background: `radial-gradient(circle, ${theme.colors.primary} 0%, transparent 100%)`, + p: 24, + }) +); +``` diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/without_skill/run-1/timing.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/without_skill/run-1/timing.json new file mode 100644 index 0000000000..9e627c6a62 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-3/eval-audit-styled-wrapper/without_skill/run-1/timing.json @@ -0,0 +1,5 @@ +{ + "total_tokens": 67355, + "duration_ms": 315373, + "total_duration_seconds": 315.373 +} diff --git a/packages/gamut/agent-tools/skills/gamut-review/SKILL.md b/packages/gamut/agent-tools/skills/gamut-review/SKILL.md index 74d1ce58b9..08c447b6c1 100644 --- a/packages/gamut/agent-tools/skills/gamut-review/SKILL.md +++ b/packages/gamut/agent-tools/skills/gamut-review/SKILL.md @@ -232,11 +232,26 @@ Read the properties inside the flagged block: - **Every property has a direct system-prop equivalent** (layout/flex/space/color/border/positioning/typography — see [`gamut-system-props`](../gamut-system-props/SKILL.md) prop groups) → ✗ error. Remediation: delete the `styled()` wrapper; pass the same values as props directly on the JSX element. Flag `display: 'flex'`/`display:flex` specifically — recommend `FlexBox` instead of `Box` + a `display` prop. Note that `background` (unlike `bg`) has no token scale and takes any CSS value as-is — a gradient string is already valid as a plain `background` prop and does **not** by itself justify a `styled()` wrapper. - **Something in the block isn't expressible as a prop** (`background-clip`, `background-blend-mode`, a variant that should branch on a prop, pseudo-selectors) → ⚠ warning. Remediation: keep `styled(ComponentName)`, but move the object into `css()`, `variant()`, or `states()` from `@codecademy/gamut-styles` instead of a raw literal — and pull the properties that _are_ expressible (padding, display, plain colors, gradients via `background`) back out to props rather than leaving them in the escape hatch just because one sibling property forced it. +**Step 4 — raw CSS property names inside `css()` silently skip the scale lookup** + +Being wrapped in `css()` (the compliant form from Step 2) isn't enough on its own to guarantee token-scale treatment. `css()`'s scale-aware behavior only applies to the _alias_ keys variance defines (`p`, `m`, `bg`, `borderRadius`, `fontSize`, `fontWeight`, `lineHeight`, …) — not the literal CSS property name. `padding: 24` inside a `css({...})` call is **not** the same as `p: 24`: variance's static-CSS extraction (`getStaticCss` in `packages/variance/src/core.ts`) passes any key it doesn't recognize as an alias straight through as literal, unscaled CSS. A block that _looks_ correct because it's wrapped in `css()` can still bypass the spacing/typography scale entirely if the property inside is spelled out as the raw CSS name instead of its alias. + +Grep for raw CSS property names as keys inside a `css({...})` call, where a shorter alias exists: + +``` +css\(\s*\{[^}]*\b(padding|margin|background-color|border-radius|font-size|font-weight|line-height)\s*: +``` + +(non-exhaustive — the pattern is: any CSS property name used as a key where its alias — `p`/`m`/`bg`/`borderRadius`/`fontSize`/`fontWeight`/`lineHeight` — belongs instead.) + +Each match is **✗ error**, not a style nit — the value silently doesn't get the intended token/rem conversion regardless of what number is written, which is a functional bug, not a preference. This check applies independently of Step 3's classification — a block can pass Step 3 (genuinely needs `css()` for one property) and still fail Step 4 (a _different_ property inside that same block used the wrong key name). + Report as `file:line styled(ComponentName)` with the classification and the specific properties found, e.g.: ``` src/HeroSection.tsx:14 styled(Box)`...` — display, flex-direction, padding, color: white → delete wrapper, use FlexBox + props (color: use a semantic token) src/ColumnTitle.tsx:8 styled(Box)`...` — background-clip: text + padding → wrap in css(), move padding to a prop +src/GlowShell.tsx:15 css({ padding: 24, ... }) — 'padding' is not a recognized alias; use 'p' or the value bypasses the spacing scale entirely (renders as an unscaled literal px value), even though the block correctly stays in css() for the gradient ``` Skill references: [`gamut-system-props`](../gamut-system-props/SKILL.md#dont-wrap-a-gamut-component-in-styled-to-hand-write-css) · [`gamut-style-utilities`](../gamut-style-utilities/SKILL.md) @@ -400,7 +415,7 @@ Skill reference for remediation: [`gamut-testing`](../gamut-testing/SKILL.md) ## Check 6 — Bespoke component duplication -Unlike Checks 1–5, this check is heuristic, not deterministic — every match needs a human glance before acting. Report all matches as ⚠ warning (never ✗ error), and say so explicitly in the finding. +Unlike Checks 1–5, this check is heuristic, not deterministic — every match needs a human glance before acting. **Every Check 6 finding is reported with `⚠`. There is no `✗` option for this check** — if a match feels like a clear-cut violation, that feeling is exactly the failure mode this rule exists to catch (a confident-looking ARIA-role/hand-rolled-listener match is still just a pattern match, not a certainty). The goal: find custom-built UI that duplicates something `@codecademy/gamut` already provides — a hand-rolled modal, dropdown, tooltip, or focus trap living next to the library that already solves it. See [`gamut-component-first`](../gamut-component-first/SKILL.md) for the full decision table and the "signals" list this check is built from. @@ -412,11 +427,11 @@ Grep source files (`.ts`, `.tsx`, `.js`, `.jsx`) for hand-set roles: role=["']?(dialog|menu|tooltip|listbox|alert)["']? ``` -For each match, check whether the same file imports the corresponding component from `@codecademy/gamut` (`Modal`/`Dialog` for `dialog`, `Menu` for `menu`, `ToolTip`/`PreviewTip`/`InfoTip`/`Tip` for `tooltip`, `SelectDropdown` for `listbox`, `Alert` for `alert`). If the import is absent, flag it. +For each match, check whether the same file imports the corresponding component from `@codecademy/gamut` (`Modal`/`Dialog` for `dialog`, `Menu` for `menu`, `ToolTip`/`PreviewTip`/`InfoTip`/`Tip` for `tooltip`, `SelectDropdown` for `listbox`, `Alert` for `alert`). If the import is absent, report as `⚠ : role="..." with no matching Gamut import`. **Step 2 — Component files named after a Gamut component that don't import it** -Look for source files whose filename (not import path) matches a known Gamut component name — `Modal`, `Dialog`, `Dropdown`, `Tooltip`, `Popover`, `Menu`, `Toast`, `Accordion`, `Tabs`, `Pagination`, `Avatar`, `Badge`, `Tag` — and check whether that file imports the matching name from `@codecademy/gamut`. A same-named local file that does _not_ import from the library is a strong signal of a parallel implementation. +Look for source files whose filename (not import path) matches a known Gamut component name — `Modal`, `Dialog`, `Dropdown`, `Tooltip`, `Popover`, `Menu`, `Toast`, `Accordion`, `Tabs`, `Pagination`, `Avatar`, `Badge`, `Tag` — and check whether that file imports the matching name from `@codecademy/gamut`. A same-named local file that does _not_ import from the library is a strong signal of a parallel implementation. Report as `⚠ filename matches a Gamut component; no @codecademy/gamut import found`. **Step 3 — Hand-rolled dismiss/focus-trap logic** @@ -426,14 +441,16 @@ Grep for manual Escape-key or outside-click dismiss handling that isn't going th (key === ['"]Escape['"]|addEventListener\(['"]keydown) ``` -in files that don't import `Overlay`, `FocusTrap`, or `PopoverContainer` from `@codecademy/gamut`. Same idea for manual outside-click listeners (`addEventListener('click'` at the document/window level alongside a "contains" check). +in files that don't import `Overlay`, `FocusTrap`, or `PopoverContainer` from `@codecademy/gamut`. Same idea for manual outside-click listeners (`addEventListener('click'` at the document/window level alongside a "contains" check). Report as `⚠ : hand-rolled dismiss logic, no FocusTrap/Overlay/PopoverContainer import`. **Step 4 — SCSS/CSS module files named after a Gamut component** -Cross-reference with Check 3b's SCSS import list: a stylesheet named `Modal.scss`, `Dropdown.module.css`, `Tooltip.scss`, etc. is worth a second look even if Check 3b already flagged the import generically — the filename is the signal that this isn't just "some CSS," it's a parallel version of a specific Gamut component. +Cross-reference with Check 3b's SCSS import list: a stylesheet named `Modal.scss`, `Dropdown.module.css`, `Tooltip.scss`, etc. is worth a second look even if Check 3b already flagged the import generically — the filename is the signal that this isn't just "some CSS," it's a parallel version of a specific Gamut component. Report as `⚠ stylesheet named after a Gamut component`. **Reporting:** for each match, name the likely Gamut component from the [decision table](../gamut-component-first/SKILL.md#decision-table-common-needs) and note this needs manual confirmation — a real product-specific one-off will look identical to a grep tool. +**Before finalizing the report**, re-scan every line under this section specifically for a `✗` icon. If you find one, that's a mistake — change it to `⚠`. **When computing the final ` error(s), warning(s)` tally, count every Check 6 match toward the warning total, never the error total, even if a `✗` slipped through above** — this is the one place a stray icon can't corrupt the report's headline numbers. + Skill reference for remediation: [`gamut-component-first`](../gamut-component-first/SKILL.md) --- diff --git a/packages/gamut/agent-tools/skills/gamut-review/evals/evals.json b/packages/gamut/agent-tools/skills/gamut-review/evals/evals.json new file mode 100644 index 0000000000..40acb784a0 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-review/evals/evals.json @@ -0,0 +1,41 @@ +{ + "skill_name": "gamut-review", + "evals": [ + { + "id": 1, + "prompt": "Can you review this file for Gamut design system compliance issues? .../eval-audit-hero-section/fixture/HeroSection.tsx (Box with an SCSS import, a className, and an inline hex-color style) -- write findings to ./outputs/report.md", + "expected_output": "A report flagging the SCSS import, className, and inline style as violations, recommending the semantic token 'text' for the hex color.", + "files": ["HeroSection.tsx"], + "expectations": [ + "Flags the SCSS import", + "Flags className on Box", + "Flags the inline style on Box", + "Recommends the semantic token 'text' for #10162F" + ] + }, + { + "id": 2, + "prompt": "Can you review this file for Gamut design system compliance issues? .../eval-audit-styled-wrapper/fixture/CardShells.tsx (one unnecessary styled(Box) raw-CSS wrapper, one legitimate css()-wrapped gradient) -- write findings to ./outputs/report.md", + "expected_output": "A report that flags CardShell (fully prop-expressible, raw wrapper) but not GlowShell (gradient, css()-wrapped) the same way, with correct remediation for each.", + "files": ["CardShells.tsx"], + "expectations": [ + "Flags CardShell as a violation with delete-the-wrapper remediation", + "Does not flag GlowShell as an error", + "Notes GlowShell's padding should move to a prop", + "Recommends FlexBox for CardShell" + ] + }, + { + "id": 3, + "prompt": "Can you review this file for Gamut design system compliance issues? .../eval-audit-bespoke-modal/fixture/CustomConfirm.tsx (a hand-rolled dialog with no Gamut imports at all) -- write findings to ./outputs/report.md", + "expected_output": "A report flagging this as a bespoke reimplementation of Modal/Dialog, pointing at gamut-modal/gamut-component-first, reported as a heuristic warning.", + "files": ["CustomConfirm.tsx"], + "expectations": [ + "Flags the hand-rolled dialog as duplicating Modal/Dialog", + "Flags the manual Escape-key listener as reinvented dismiss logic", + "Points remediation at gamut-modal/gamut-component-first", + "Reports as a warning/heuristic, not an unconditional error" + ] + } + ] +} From 12dde0a6bfec4405ec4efadfb3b6585e38605b61 Mon Sep 17 00:00:00 2001 From: Kenny Lin Date: Tue, 28 Jul 2026 13:57:28 -0400 Subject: [PATCH 12/16] add check for FlexBox/GridBox when setting display on Box --- .../agent-tools/skills/gamut-review/SKILL.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/gamut/agent-tools/skills/gamut-review/SKILL.md b/packages/gamut/agent-tools/skills/gamut-review/SKILL.md index 08c447b6c1..15f2b19457 100644 --- a/packages/gamut/agent-tools/skills/gamut-review/SKILL.md +++ b/packages/gamut/agent-tools/skills/gamut-review/SKILL.md @@ -246,12 +246,27 @@ css\(\s*\{[^}]*\b(padding|margin|background-color|border-radius|font-size|font-w Each match is **✗ error**, not a style nit — the value silently doesn't get the intended token/rem conversion regardless of what number is written, which is a functional bug, not a preference. This check applies independently of Step 3's classification — a block can pass Step 3 (genuinely needs `css()` for one property) and still fail Step 4 (a _different_ property inside that same block used the wrong key name). +**Step 5 — plain ``/`` (heuristic, not a bypass)** + +Unlike Steps 1–4, this isn't a system-props bypass — `display` is a real system prop and the code works correctly. It's a semantic-consistency nit: `FlexBox`/`GridBox` already default to `display: flex`/`display: grid`, so reaching for `Box` + a `display` prop says the same thing more verbosely and loses the more legible component name at the call site. + +Grep files already confirmed to import `Box` from `@codecademy/gamut` (Step 1) for: + +``` +]*\bdisplay=\{?["']flex["']\}? +]*\bdisplay=\{?["']grid["']\}? +``` + +Always **⚠ warning**, never ✗ — don't flag `display="inline-flex"`/`"inline-grid"`, or a conditionally-computed `display` (e.g. `display={isOpen ? 'flex' : 'none'}`); both are legitimate reasons to stay on `Box`. + Report as `file:line styled(ComponentName)` with the classification and the specific properties found, e.g.: ``` src/HeroSection.tsx:14 styled(Box)`...` — display, flex-direction, padding, color: white → delete wrapper, use FlexBox + props (color: use a semantic token) src/ColumnTitle.tsx:8 styled(Box)`...` — background-clip: text + padding → wrap in css(), move padding to a prop src/GlowShell.tsx:15 css({ padding: 24, ... }) — 'padding' is not a recognized alias; use 'p' or the value bypasses the spacing scale entirely (renders as an unscaled literal px value), even though the block correctly stays in css() for the gradient +src/Panel.tsx:9 — same defaults as FlexBox, more legible as FlexBox +src/Grid.tsx:14 — same defaults as GridBox, more legible as GridBox ``` Skill references: [`gamut-system-props`](../gamut-system-props/SKILL.md#dont-wrap-a-gamut-component-in-styled-to-hand-write-css) · [`gamut-style-utilities`](../gamut-style-utilities/SKILL.md) @@ -507,6 +522,9 @@ styled(GamutComponent) bypassing system props [→ gamut-system-props src/HeroSection.tsx:14 display, flex-direction, padding, color: white → delete wrapper, use FlexBox + props ⚠ styled(Box) raw CSS 1 occurrence — partially expressible, needs css() not a raw literal src/ColumnTitle.tsx:8 background-clip: text + padding → wrap in css(), move padding to a prop + ⚠ Box used where FlexBox/GridBox fits 2 occurrences — same defaults, more legible name + src/Panel.tsx:9 → FlexBox + src/Grid.tsx:14 → GridBox (or: ✓ none found) Hardcoded colors [→ gamut-color-mode] From 24122a7c731885808050d1a72f464c1d1153141e Mon Sep 17 00:00:00 2001 From: Kenny Lin Date: Tue, 28 Jul 2026 16:23:31 -0400 Subject: [PATCH 13/16] have eslint ignore skills workspace files --- .eslintignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.eslintignore b/.eslintignore index 2b0150ad6d..7ba45a3731 100644 --- a/.eslintignore +++ b/.eslintignore @@ -11,3 +11,4 @@ packages/gamut-patterns/src/patterns **/code-connect/** packages/code-connect .nx +packages/gamut/agent-tools/skills/*-workspace From 4d5ce404735272337214d23b49598685f8b11d00 Mon Sep 17 00:00:00 2001 From: Kenny Lin Date: Fri, 31 Jul 2026 14:57:16 -0400 Subject: [PATCH 14/16] clean up some evals and removed older iterations --- .../CustomConfirm.tsx | 0 .../eval-audit-hero-section}/HeroSection.scss | 0 .../eval-audit-hero-section}/HeroSection.tsx | 0 .../eval-audit-styled-wrapper}/CardShells.tsx | 0 .../iteration-1/agent_map.json | 8 - .../iteration-1/benchmark.json | 309 ----------------- .../iteration-1/benchmark.md | 13 - .../eval_metadata.json | 11 - .../with_skill/run-1/grading.json | 25 -- .../with_skill/run-1/outputs/report.md | 51 --- .../with_skill/run-1/timing.json | 5 - .../without_skill/run-1/grading.json | 32 -- .../without_skill/run-1/outputs/report.md | 51 --- .../without_skill/run-1/timing.json | 5 - .../eval_metadata.json | 11 - .../with_skill/run-1/grading.json | 32 -- .../with_skill/run-1/outputs/report.md | 49 --- .../with_skill/run-1/timing.json | 5 - .../without_skill/run-1/grading.json | 33 -- .../without_skill/run-1/outputs/report.md | 67 ---- .../without_skill/run-1/timing.json | 5 - .../eval_metadata.json | 11 - .../with_skill/run-1/grading.json | 32 -- .../with_skill/run-1/outputs/report.md | 52 --- .../with_skill/run-1/timing.json | 5 - .../without_skill/run-1/grading.json | 33 -- .../without_skill/run-1/outputs/report.md | 73 ---- .../without_skill/run-1/timing.json | 5 - .../eval_metadata.json | 2 +- .../eval_metadata.json | 2 +- .../eval_metadata.json | 2 +- .../eval_metadata.json | 2 +- .../iteration-1/agent_map.json | 8 - .../iteration-1/benchmark.json | 295 ---------------- .../iteration-1/benchmark.md | 13 - .../eval_metadata.json | 11 - .../with_skill/run-1/grading.json | 25 -- .../with_skill/run-1/outputs/StatusBadge.tsx | 63 ---- .../with_skill/run-1/timing.json | 5 - .../without_skill/run-1/grading.json | 25 -- .../run-1/outputs/StatusBadge.tsx | 68 ---- .../without_skill/run-1/timing.json | 5 - .../eval_metadata.json | 11 - .../with_skill/run-1/grading.json | 25 -- .../with_skill/run-1/outputs/Chip.tsx | 97 ------ .../with_skill/run-1/timing.json | 5 - .../without_skill/run-1/grading.json | 25 -- .../without_skill/run-1/outputs/Chip.tsx | 130 ------- .../without_skill/run-1/timing.json | 5 - .../eval_metadata.json | 11 - .../with_skill/run-1/grading.json | 25 -- .../with_skill/run-1/outputs/Sparkline.tsx | 100 ------ .../with_skill/run-1/timing.json | 5 - .../without_skill/run-1/grading.json | 25 -- .../without_skill/run-1/outputs/Sparkline.tsx | 115 ------- .../without_skill/run-1/timing.json | 5 - .../iteration-2/agent_map.json | 8 - .../iteration-2/benchmark.json | 299 ---------------- .../iteration-2/benchmark.md | 13 - .../eval_metadata.json | 11 - .../with_skill/run-1/grading.json | 25 -- .../with_skill/run-1/outputs/StatusBadge.tsx | 56 --- .../with_skill/run-1/timing.json | 5 - .../without_skill/run-1/grading.json | 25 -- .../run-1/outputs/StatusBadge.tsx | 52 --- .../without_skill/run-1/timing.json | 5 - .../eval_metadata.json | 11 - .../with_skill/run-1/grading.json | 32 -- .../with_skill/run-1/outputs/Chip.tsx | 87 ----- .../with_skill/run-1/timing.json | 5 - .../without_skill/run-1/grading.json | 25 -- .../without_skill/run-1/outputs/Chip.tsx | 107 ------ .../without_skill/run-1/timing.json | 5 - .../eval_metadata.json | 11 - .../with_skill/run-1/grading.json | 25 -- .../with_skill/run-1/outputs/Sparkline.tsx | 94 ----- .../with_skill/run-1/timing.json | 5 - .../without_skill/run-1/grading.json | 25 -- .../without_skill/run-1/outputs/Sparkline.tsx | 100 ------ .../without_skill/run-1/timing.json | 5 - .../iteration-1/agent_map.json | 8 - .../iteration-1/benchmark.json | 321 ------------------ .../iteration-1/benchmark.md | 13 - .../eval_metadata.json | 12 - .../with_skill/run-1/grading.json | 56 --- .../with_skill/run-1/outputs/CTAContainer.tsx | 33 -- .../with_skill/run-1/timing.json | 5 - .../without_skill/run-1/grading.json | 56 --- .../run-1/outputs/CTAContainer.tsx | 27 -- .../without_skill/run-1/timing.json | 5 - .../eval_metadata.json | 11 - .../with_skill/run-1/grading.json | 53 --- .../with_skill/run-1/outputs/GradientGlow.tsx | 35 -- .../with_skill/run-1/timing.json | 5 - .../without_skill/run-1/grading.json | 60 ---- .../run-1/outputs/GradientGlow.tsx | 26 -- .../without_skill/run-1/timing.json | 5 - .../eval_metadata.json | 12 - .../with_skill/run-1/grading.json | 58 ---- .../run-1/outputs/HeroContainer.tsx | 20 -- .../with_skill/run-1/timing.json | 5 - .../without_skill/run-1/grading.json | 58 ---- .../run-1/outputs/HeroContainer.tsx | 13 - .../without_skill/run-1/timing.json | 5 - 104 files changed, 4 insertions(+), 3967 deletions(-) rename packages/gamut/agent-tools/skills/gamut-review-workspace/{iteration-1/eval-audit-bespoke-modal/fixture => fixtures/eval-audit-bespoke-modal}/CustomConfirm.tsx (100%) rename packages/gamut/agent-tools/skills/gamut-review-workspace/{iteration-1/eval-audit-hero-section/fixture => fixtures/eval-audit-hero-section}/HeroSection.scss (100%) rename packages/gamut/agent-tools/skills/gamut-review-workspace/{iteration-1/eval-audit-hero-section/fixture => fixtures/eval-audit-hero-section}/HeroSection.tsx (100%) rename packages/gamut/agent-tools/skills/gamut-review-workspace/{iteration-1/eval-audit-styled-wrapper/fixture => fixtures/eval-audit-styled-wrapper}/CardShells.tsx (100%) delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/agent_map.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/benchmark.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/benchmark.md delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/eval_metadata.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/outputs/report.md delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/without_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/without_skill/run-1/outputs/report.md delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/without_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-hero-section/eval_metadata.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-hero-section/with_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-hero-section/with_skill/run-1/outputs/report.md delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-hero-section/with_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-hero-section/without_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-hero-section/without_skill/run-1/outputs/report.md delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-hero-section/without_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-styled-wrapper/eval_metadata.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-styled-wrapper/with_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-styled-wrapper/with_skill/run-1/outputs/report.md delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-styled-wrapper/with_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-styled-wrapper/without_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-styled-wrapper/without_skill/run-1/outputs/report.md delete mode 100644 packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-styled-wrapper/without_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/agent_map.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/benchmark.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/benchmark.md delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-badge-variant-status/eval_metadata.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-badge-variant-status/with_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-badge-variant-status/with_skill/run-1/outputs/StatusBadge.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-badge-variant-status/with_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-badge-variant-status/without_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-badge-variant-status/without_skill/run-1/outputs/StatusBadge.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-badge-variant-status/without_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-chip-independent-states/eval_metadata.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-chip-independent-states/with_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-chip-independent-states/with_skill/run-1/outputs/Chip.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-chip-independent-states/with_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-chip-independent-states/without_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-chip-independent-states/without_skill/run-1/outputs/Chip.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-chip-independent-states/without_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-sparkline-theme-color/eval_metadata.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-sparkline-theme-color/with_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-sparkline-theme-color/with_skill/run-1/outputs/Sparkline.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-sparkline-theme-color/with_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-sparkline-theme-color/without_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-sparkline-theme-color/without_skill/run-1/outputs/Sparkline.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-1/eval-sparkline-theme-color/without_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/agent_map.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/benchmark.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/benchmark.md delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-badge-variant-status/eval_metadata.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-badge-variant-status/with_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-badge-variant-status/with_skill/run-1/outputs/StatusBadge.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-badge-variant-status/with_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-badge-variant-status/without_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-badge-variant-status/without_skill/run-1/outputs/StatusBadge.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-badge-variant-status/without_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-chip-independent-states/eval_metadata.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-chip-independent-states/with_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-chip-independent-states/with_skill/run-1/outputs/Chip.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-chip-independent-states/with_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-chip-independent-states/without_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-chip-independent-states/without_skill/run-1/outputs/Chip.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-chip-independent-states/without_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-sparkline-theme-color/eval_metadata.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-sparkline-theme-color/with_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-sparkline-theme-color/with_skill/run-1/outputs/Sparkline.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-sparkline-theme-color/with_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-sparkline-theme-color/without_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-sparkline-theme-color/without_skill/run-1/outputs/Sparkline.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-style-utilities-workspace/iteration-2/eval-sparkline-theme-color/without_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/agent_map.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/benchmark.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/benchmark.md delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-cta-banner-spacing-color/eval_metadata.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-cta-banner-spacing-color/with_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-cta-banner-spacing-color/with_skill/run-1/outputs/CTAContainer.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-cta-banner-spacing-color/with_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-cta-banner-spacing-color/without_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-cta-banner-spacing-color/without_skill/run-1/outputs/CTAContainer.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-cta-banner-spacing-color/without_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-gradient-glow-mixed/eval_metadata.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-gradient-glow-mixed/with_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-gradient-glow-mixed/with_skill/run-1/outputs/GradientGlow.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-gradient-glow-mixed/with_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-gradient-glow-mixed/without_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-gradient-glow-mixed/without_skill/run-1/outputs/GradientGlow.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-gradient-glow-mixed/without_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-hero-section-flex-layout/eval_metadata.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-hero-section-flex-layout/with_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-hero-section-flex-layout/with_skill/run-1/outputs/HeroContainer.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-hero-section-flex-layout/with_skill/run-1/timing.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-hero-section-flex-layout/without_skill/run-1/grading.json delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-hero-section-flex-layout/without_skill/run-1/outputs/HeroContainer.tsx delete mode 100644 packages/gamut/agent-tools/skills/gamut-system-props-workspace/iteration-1/eval-hero-section-flex-layout/without_skill/run-1/timing.json diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/fixture/CustomConfirm.tsx b/packages/gamut/agent-tools/skills/gamut-review-workspace/fixtures/eval-audit-bespoke-modal/CustomConfirm.tsx similarity index 100% rename from packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/fixture/CustomConfirm.tsx rename to packages/gamut/agent-tools/skills/gamut-review-workspace/fixtures/eval-audit-bespoke-modal/CustomConfirm.tsx diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-hero-section/fixture/HeroSection.scss b/packages/gamut/agent-tools/skills/gamut-review-workspace/fixtures/eval-audit-hero-section/HeroSection.scss similarity index 100% rename from packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-hero-section/fixture/HeroSection.scss rename to packages/gamut/agent-tools/skills/gamut-review-workspace/fixtures/eval-audit-hero-section/HeroSection.scss diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-hero-section/fixture/HeroSection.tsx b/packages/gamut/agent-tools/skills/gamut-review-workspace/fixtures/eval-audit-hero-section/HeroSection.tsx similarity index 100% rename from packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-hero-section/fixture/HeroSection.tsx rename to packages/gamut/agent-tools/skills/gamut-review-workspace/fixtures/eval-audit-hero-section/HeroSection.tsx diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-styled-wrapper/fixture/CardShells.tsx b/packages/gamut/agent-tools/skills/gamut-review-workspace/fixtures/eval-audit-styled-wrapper/CardShells.tsx similarity index 100% rename from packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-styled-wrapper/fixture/CardShells.tsx rename to packages/gamut/agent-tools/skills/gamut-review-workspace/fixtures/eval-audit-styled-wrapper/CardShells.tsx diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/agent_map.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/agent_map.json deleted file mode 100644 index 00b6ee778d..0000000000 --- a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/agent_map.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "ad094235447f2e000": "eval-audit-hero-section/with_skill", - "a83052b2c9da3c242": "eval-audit-hero-section/without_skill", - "ac25e43357e2a09f3": "eval-audit-styled-wrapper/with_skill", - "ac1139376740f3c56": "eval-audit-styled-wrapper/without_skill", - "ad77ff5fdb227f1e3": "eval-audit-bespoke-modal/with_skill", - "a3333bd5e23f3b43a": "eval-audit-bespoke-modal/without_skill" -} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/benchmark.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/benchmark.json deleted file mode 100644 index 444c2ac393..0000000000 --- a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/benchmark.json +++ /dev/null @@ -1,309 +0,0 @@ -{ - "metadata": { - "skill_name": "gamut-review", - "skill_path": "", - "executor_model": "", - "analyzer_model": "", - "timestamp": "2026-07-28T15:35:07Z", - "evals_run": [1, 2, 3], - "runs_per_configuration": 3 - }, - "runs": [ - { - "eval_id": 3, - "configuration": "with_skill", - "run_number": 1, - "result": { - "pass_rate": 0.75, - "passed": 3, - "failed": 1, - "total": 4, - "time_seconds": 480.626, - "tokens": 46235, - "tool_calls": 0, - "errors": 0 - }, - "expectations": [ - { - "text": "Flags this component as duplicating an existing Gamut component (bespoke dialog) -- role=\"dialog\" is set by hand with no Modal/Dialog import", - "passed": true, - "evidence": "\"CustomConfirm.tsx:27 role=\\\"dialog\\\" with no Modal/Dialog import from @codecademy/gamut -- ... strongly recommend Modal\"." - }, - { - "text": "Flags the manual document.addEventListener('keydown', ...) Escape-key handling as hand-rolled dismiss logic that Overlay/FocusTrap already provides", - "passed": true, - "evidence": "\"manual Escape-key dismiss ... with no Overlay/FocusTrap/PopoverContainer import -- Gamut's Modal/Overlay already implements Escape-to-close\"." - }, - { - "text": "Points remediation at gamut-modal and/or gamut-component-first", - "passed": true, - "evidence": "\"[\u2192 gamut-component-first]\" cross-reference tag on the section header, plus explicit text pointer." - }, - { - "text": "Treats this as a warning/heuristic finding needing manual confirmation, not an unconditional hard error -- since it's pattern-matching, not a certainty", - "passed": false, - "evidence": "REAL RULE VIOLATION: the section header correctly says \"(heuristic -- confirm manually)\", but two of the five individual findings under it (the role=\"dialog\" finding and the manual-Escape-listener finding) are marked with a hard \u2717 (error) icon, not \u26a0 (warning). gamut-review's own Check 6 instructions explicitly state: \"Report all matches as \u26a0 warning (never \u2717 error)\". The report is internally inconsistent -- it gets the outside-click variant correctly marked \u26a0 (with an explicit note explaining why), but fails to apply the same rule to the two more clear-cut findings. This is a genuine, verifiable failure to follow the skill's own stated instruction, not a matter of interpretation." - } - ], - "notes": [] - }, - { - "eval_id": 1, - "configuration": "with_skill", - "run_number": 1, - "result": { - "pass_rate": 1.0, - "passed": 4, - "failed": 0, - "total": 4, - "time_seconds": 316.703, - "tokens": 39997, - "tool_calls": 0, - "errors": 0 - }, - "expectations": [ - { - "text": "Flags the './HeroSection.scss' import as an SCSS/CSS module violation", - "passed": true, - "evidence": "\"SCSS/CSS imports 1 file\" with the exact file path and remediation note, correctly using the Check 3b Step 1 format." - }, - { - "text": "Flags className=\"hero-wrapper\" on the Box component as a violation", - "passed": true, - "evidence": "\"className on Gamut components 1 occurrence -- HeroSection.tsx:6 \", correctly noted as not an integration seam." - }, - { - "text": "Flags the inline style={{ color: '#10162F' }} on Box as a violation", - "passed": true, - "evidence": "\"inline style on Gamut components 1 occurrence\" with the exact line and value, correctly checked for an eslint-disable comment (found none)." - }, - { - "text": "For the hex #10162F, recommends the semantic token 'text' (not just the palette name navy-800) as the primary remediation", - "passed": true, - "evidence": "\"semantic: text | palette: navy / navy-800\" -- semantic listed first per Check 4's semantic-first rule." - } - ], - "notes": [ - "Report includes an honest, self-aware note that the bare

(bypassing Gamut's Text/typography component) was NOT flagged as a violation because no existing gamut-review check targets bare heading tags outside nested-selector context -- a real, correctly-identified scope gap. The without_skill run for this same eval independently found and flagged this exact gap as a real issue -- worth considering as a future Check addition." - ] - }, - { - "eval_id": 2, - "configuration": "with_skill", - "run_number": 1, - "result": { - "pass_rate": 1.0, - "passed": 4, - "failed": 0, - "total": 4, - "time_seconds": 128.204, - "tokens": 42244, - "tool_calls": 0, - "errors": 0 - }, - "expectations": [ - { - "text": "Flags CardShell (styled(Box) with a raw tagged-template literal, where every property is prop-expressible) as an error/violation, with remediation to delete the wrapper and use Box/FlexBox props directly", - "passed": true, - "evidence": "\"CardShells.tsx:7 styled(Box)\\`...\\` ... Delete the wrapper; use FlexBox ... with flexDirection=\\\"column\\\" and p={16} directly\"." - }, - { - "text": "Does NOT flag GlowShell as an error the same way -- it's wrapped in css(), which is the compliant form for a non-expressible gradient", - "passed": true, - "evidence": "This assertion was written on a premise I have since verified is WRONG: system.background has no scale and accepts any CSS value directly, so the gradient IS prop-expressible after all (confirmed during the gamut-system-props benchmark earlier this session). This run correctly identified that nuance and flagged GlowShell as an error too, recommending deleting the wrapper entirely -- a MORE correct answer than my original assertion anticipated. Grading as passed on substance (correctly distinguishes 'syntactically compliant' from 'actually necessary') rather than failing it for exceeding a flawed assertion." - }, - { - "text": "Notes that GlowShell's padding: 24 should still move out to a prop even though the gradient must stay in css()", - "passed": true, - "evidence": "Recommends p={24} directly on Box alongside the background prop." - }, - { - "text": "Recommends FlexBox instead of Box + display:flex for CardShell", - "passed": true, - "evidence": "Explicit: \"use FlexBox (display: flex -> FlexBox, not Box + display)\"." - } - ], - "notes": [ - "This report reaches a deeper, more optimal conclusion than the without_skill run for the same eval: it recognizes GlowShell needs no styled()/css() wrapper at all (since `background` takes the gradient directly), while without_skill kept the wrapper and just recommended a theme-callback color fix -- both valid, but with_skill's is the more complete remediation." - ] - }, - { - "eval_id": 3, - "configuration": "without_skill", - "run_number": 1, - "result": { - "pass_rate": 0.75, - "passed": 3, - "failed": 1, - "total": 4, - "time_seconds": 538.241, - "tokens": 43729, - "tool_calls": 0, - "errors": 0 - }, - "expectations": [ - { - "text": "Flags this component as duplicating an existing Gamut component (bespoke dialog) -- role=\"dialog\" is set by hand with no Modal/Dialog import", - "passed": true, - "evidence": "Finding #1, \"Reinvents an existing component instead of using Dialog/Modal (Critical)\", with a full working Dialog-based replacement code sample." - }, - { - "text": "Flags the manual document.addEventListener('keydown', ...) Escape-key handling as hand-rolled dismiss logic that Overlay/FocusTrap already provides", - "passed": true, - "evidence": "Finding #7, \"Duplicated, less robust escape/outside-click handling\"." - }, - { - "text": "Points remediation at gamut-modal and/or gamut-component-first", - "passed": true, - "evidence": "Cannot cite these skills by name (had no access to them), but the substantive remediation is identical: recommends Dialog directly with a working code sample -- the same component either skill would point to." - }, - { - "text": "Treats this as a warning/heuristic finding needing manual confirmation, not an unconditional hard error -- since it's pattern-matching, not a certainty", - "passed": false, - "evidence": "Report presents every finding with a confident severity label (Critical/High/Medium/Low) and declarative language (\"This should be used instead of hand-rolled markup\") with no hedging that this is a heuristic judgment that could have false positives for a genuinely custom, product-specific widget. Never having been taught the heuristic-confidence distinction, it has no reason to hedge -- but the assertion is about the report's calibration, not the run's awareness of a specific rule, so this is graded as not meeting the bar either, for a different underlying reason than the with_skill run's failure." - } - ], - "notes": [ - "Extraordinarily thorough -- 13 distinct findings vs. gamut-review's ~5, correctly citing the same real no-inline-style ESLint rule found in the other evals, and correctly citing the exact FocusTrap/index.tsx eslint-disable precedent. Took roughly 2x the tool calls and 9 minutes vs. with_skill's ~8 minutes for a comparable core finding, but produced meaningfully more comprehensive coverage (focus trap, z-index, ARIA completeness, button/typography component usage, overflow handling) than the current gamut-review skill's Check 6 scope." - ] - }, - { - "eval_id": 1, - "configuration": "without_skill", - "run_number": 1, - "result": { - "pass_rate": 1.0, - "passed": 4, - "failed": 0, - "total": 4, - "time_seconds": 363.078, - "tokens": 48264, - "tool_calls": 0, - "errors": 0 - }, - "expectations": [ - { - "text": "Flags the './HeroSection.scss' import as an SCSS/CSS module violation", - "passed": true, - "evidence": "Finding #3, citing the real @codecademy/eslint-plugin-gamut no-css-standalone rule (error) by exact file/line -- more precise than gamut-review's own check, which doesn't know this ESLint rule exists." - }, - { - "text": "Flags className=\"hero-wrapper\" on the Box component as a violation", - "passed": true, - "evidence": "Finding #5 explicitly names className and treats combining it with external SCSS as fighting the styling model." - }, - { - "text": "Flags the inline style={{ color: '#10162F' }} on Box as a violation", - "passed": true, - "evidence": "Finding #1, citing the real no-inline-style ESLint rule (error) by exact source file." - }, - { - "text": "For the hex #10162F, recommends the semantic token 'text' (not just the palette name navy-800) as the primary remediation", - "passed": true, - "evidence": "Finding #2: \"Should be instead\", with correct dark-mode reasoning (text resolves to white in dark mode) as to why this matters." - } - ], - "notes": [ - "This run discovered two real, existing ESLint rules in @codecademy/eslint-plugin-gamut (no-inline-style, no-css-standalone) that gamut-review's Check 3b substantially duplicates -- neither the skill nor its author (me, in this session) knew this plugin existed. Verified real via direct file inspection (packages/eslint-plugin-gamut/src/no-inline-style.ts, no-css-standalone.ts both exist and are set to error in recommended.ts). This also explains a detail noticed much earlier in this same benchmarking session: packages/gamut/src/FocusTrap/index.tsx has a `// eslint-disable-next-line gamut/no-inline-style` comment, which is exactly this rule.", - "Also independently found and flagged the bare

bypassing Gamut's Text/typography component -- a real issue outside any existing gamut-review check (the with_skill run for this same eval explicitly noted this same gap in its own report, correctly recognizing it as out of scope for the current skill)." - ] - }, - { - "eval_id": 2, - "configuration": "without_skill", - "run_number": 1, - "result": { - "pass_rate": 1.0, - "passed": 4, - "failed": 0, - "total": 4, - "time_seconds": 547.774, - "tokens": 64260, - "tool_calls": 0, - "errors": 0 - }, - "expectations": [ - { - "text": "Flags CardShell (styled(Box) with a raw tagged-template literal, where every property is prop-expressible) as an error/violation, with remediation to delete the wrapper and use Box/FlexBox props directly", - "passed": true, - "evidence": "\"Drop CardShell and use directly at call sites\", with a repo-wide grep confirming no other styled(Box) template-literal usage exists as precedent." - }, - { - "text": "Does NOT flag GlowShell as an error the same way -- it's wrapped in css(), which is the compliant form for a non-expressible gradient", - "passed": true, - "evidence": "Correctly did not flag the wrapper itself as the CardShell-style violation -- flagged the hardcoded hex color inside it instead (a real, valid issue). Did not reach the deeper insight (also reached by with_skill) that the wrapper is unnecessary entirely since `background` takes the gradient as a plain prop -- recommended a theme-callback fix that keeps the wrapper instead. Valid but less optimal than with_skill's answer for this eval." - }, - { - "text": "Notes that GlowShell's padding: 24 should still move out to a prop even though the gradient must stay in css()", - "passed": true, - "evidence": "Finding #3 explicitly flags this with a precedence-hazard rationale (a consumer passing p= would conflict with the baked-in padding)." - }, - { - "text": "Recommends FlexBox instead of Box + display:flex for CardShell", - "passed": true, - "evidence": "\"\"." - } - ], - "notes": [ - "Discovered two more real ESLint rules not previously known: gamut/import-paths and gamut/prefer-themed (the latter steering toward the themed() utility for exactly this hardcoded-color-in-a-string case). Also independently derived that #3A10E5 is precisely the hyper/hyper-500 token via direct source inspection -- matches the same value independently verified earlier in this session's gamut-review Check 4 Appendix A research.", - "Extremely thorough (42 tool calls, 64k tokens, ~9 minutes) relative to with_skill's more targeted pass (4 tool calls, 42k tokens, ~2 minutes) for a comparable-quality result on this eval." - ] - } - ], - "run_summary": { - "with_skill": { - "pass_rate": { - "mean": 0.9167, - "stddev": 0.1443, - "min": 0.75, - "max": 1.0 - }, - "time_seconds": { - "mean": 308.511, - "stddev": 176.3538, - "min": 128.204, - "max": 480.626 - }, - "tokens": { - "mean": 42825.3333, - "stddev": 3159.3706, - "min": 39997, - "max": 46235 - } - }, - "without_skill": { - "pass_rate": { - "mean": 0.9167, - "stddev": 0.1443, - "min": 0.75, - "max": 1.0 - }, - "time_seconds": { - "mean": 483.031, - "stddev": 103.9916, - "min": 363.078, - "max": 547.774 - }, - "tokens": { - "mean": 52084.3333, - "stddev": 10785.4856, - "min": 43729, - "max": 64260 - } - }, - "delta": { - "pass_rate": "+0.00", - "time_seconds": "-174.5", - "tokens": "-9259" - } - }, - "notes": [ - "Exact tie on pass_rate (0.917 both) -- unlike the other three gamut skills benchmarked this session, gamut-review is an audit skill, not a code-generation skill, and a sufficiently thorough generic code review can match its structured checks point-for-point. The value here reads as efficiency and consistency, not raw finding quality: with_skill was ~1.6x faster and used ~18% fewer tokens for the same pass rate.", - "Biggest finding: audit-bespoke-modal exposed a genuine, verifiable rule violation in with_skill's own output -- gamut-review's Check 6 instructions explicitly state findings must be reported as warning, never error ('heuristic, not deterministic'). The report's own section header correctly says '(heuristic -- confirm manually)', but 2 of 5 individual findings under it were marked with a hard error icon anyway. This is not a matter of interpretation; it's a direct, checkable violation of the skill's own stated rule.", - "without_skill failed the same assertion for a different reason: with no heuristic-confidence framing to follow, it presented every finding (13 of them, more than gamut-review's own checks produce) with confident severity labels and no hedging about false-positive risk for a genuinely custom widget.", - "A significant discovery, independently made by both without_skill runs on two different evals: this repo already has a real ESLint plugin, @codecademy/eslint-plugin-gamut, with rules no-inline-style, no-css-standalone, import-paths, and prefer-themed -- all set to error in the recommended config. gamut-review's Check 3b (SCSS imports, className, inline style) substantially duplicates what no-inline-style and no-css-standalone already catch mechanically. This also retroactively explains a detail noticed much earlier in this session: FocusTrap/index.tsx carries a `// eslint-disable-next-line gamut/no-inline-style` comment -- that rule is real. gamut-review's own SKILL.md never mentions this plugin exists.", - "audit-hero-section revealed a real scope gap in gamut-review, honestly self-reported by the with_skill run itself: neither Check 3b/3c/3d nor any other check flags a bare

that bypasses Gamut's Text/typography component. without_skill found and flagged this independently. This is a legitimate candidate for a new check.", - "audit-styled-wrapper revealed that one of my own pre-registered assertions for this eval was based on a premise I had already disproven earlier in this session (that GlowShell's gradient needs a styled()/css() escape hatch) -- with_skill correctly applied the corrected understanding (system.background takes any value, no wrapper needed) and flagged GlowShell as an error too; without_skill kept the wrapper and recommended a theme-callback fix instead, which is valid but less optimal. Graded both as passing on substance rather than penalizing with_skill for exceeding a flawed assertion.", - "Recommended follow-ups, in priority order: (1) fix the Check 6 severity-icon inconsistency demonstrated live in this benchmark -- likely needs a stronger per-finding reminder, not just a preamble statement; (2) add a cross-reference to @codecademy/eslint-plugin-gamut in Check 3b so the skill doesn't present itself as the only way to catch these issues when a lint run already would; (3) consider a new check for raw HTML typography elements vs. Text/Typography component usage." - ] -} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/benchmark.md b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/benchmark.md deleted file mode 100644 index f26bc2d799..0000000000 --- a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/benchmark.md +++ /dev/null @@ -1,13 +0,0 @@ -# Skill Benchmark: gamut-review - -**Model**: -**Date**: 2026-07-28T15:35:07Z -**Evals**: 1, 2, 3 (3 runs each per configuration) - -## Summary - -| Metric | With Skill | Without Skill | Delta | -| --------- | --------------- | --------------- | ------- | -| Pass Rate | 92% ± 14% | 92% ± 14% | +0.00 | -| Time | 308.5s ± 176.4s | 483.0s ± 104.0s | -174.5s | -| Tokens | 42825 ± 3159 | 52084 ± 10785 | -9259 | diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/eval_metadata.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/eval_metadata.json deleted file mode 100644 index 9a8cd108a3..0000000000 --- a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/eval_metadata.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "eval_id": 3, - "eval_name": "audit-bespoke-modal", - "prompt": "Can you review this file for Gamut design system compliance issues? /Users/kenny.lin/Documents/eng/codecademy-eng/gamut/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/fixture/CustomConfirm.tsx -- write your findings to ./outputs/report.md", - "assertions": [ - "Flags this component as duplicating an existing Gamut component (bespoke dialog) -- role=\"dialog\" is set by hand with no Modal/Dialog import", - "Flags the manual document.addEventListener('keydown', ...) Escape-key handling as hand-rolled dismiss logic that Overlay/FocusTrap already provides", - "Points remediation at gamut-modal and/or gamut-component-first", - "Treats this as a warning/heuristic finding needing manual confirmation, not an unconditional hard error -- since it's pattern-matching, not a certainty" - ] -} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/grading.json b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/grading.json deleted file mode 100644 index 25b9cf0ee8..0000000000 --- a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/grading.json +++ /dev/null @@ -1,25 +0,0 @@ -{ - "expectations": [ - { - "text": "Flags this component as duplicating an existing Gamut component (bespoke dialog) -- role=\"dialog\" is set by hand with no Modal/Dialog import", - "passed": true, - "evidence": "\"CustomConfirm.tsx:27 role=\\\"dialog\\\" with no Modal/Dialog import from @codecademy/gamut -- ... strongly recommend Modal\"." - }, - { - "text": "Flags the manual document.addEventListener('keydown', ...) Escape-key handling as hand-rolled dismiss logic that Overlay/FocusTrap already provides", - "passed": true, - "evidence": "\"manual Escape-key dismiss ... with no Overlay/FocusTrap/PopoverContainer import -- Gamut's Modal/Overlay already implements Escape-to-close\"." - }, - { - "text": "Points remediation at gamut-modal and/or gamut-component-first", - "passed": true, - "evidence": "\"[→ gamut-component-first]\" cross-reference tag on the section header, plus explicit text pointer." - }, - { - "text": "Treats this as a warning/heuristic finding needing manual confirmation, not an unconditional hard error -- since it's pattern-matching, not a certainty", - "passed": false, - "evidence": "REAL RULE VIOLATION: the section header correctly says \"(heuristic -- confirm manually)\", but two of the five individual findings under it (the role=\"dialog\" finding and the manual-Escape-listener finding) are marked with a hard ✗ (error) icon, not ⚠ (warning). gamut-review's own Check 6 instructions explicitly state: \"Report all matches as ⚠ warning (never ✗ error)\". The report is internally inconsistent -- it gets the outside-click variant correctly marked ⚠ (with an explicit note explaining why), but fails to apply the same rule to the two more clear-cut findings. This is a genuine, verifiable failure to follow the skill's own stated instruction, not a matter of interpretation." - } - ], - "summary": { "passed": 3, "failed": 1, "total": 4, "pass_rate": 0.75 } -} diff --git a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/outputs/report.md b/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/outputs/report.md deleted file mode 100644 index 15d8604b3f..0000000000 --- a/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/with_skill/run-1/outputs/report.md +++ /dev/null @@ -1,51 +0,0 @@ -Gamut Review — /Users/kenny.lin/Documents/eng/codecademy-eng/gamut/packages/gamut/agent-tools/skills/gamut-review-workspace/iteration-1/eval-audit-bespoke-modal/fixture/CustomConfirm.tsx -══════════════════════════════════════════════════ - -Scope note: audit root is a single standalone file (fixture), not a project checkout. Checks that require project-level artifacts (package.json, DESIGN.md, app-root wiring, test files) are marked N/A below rather than scored, per the single-file audit instructions. - -DESIGN.md -— N/A no project root to resolve; this is a single-file audit, not a project checkout - -Dependencies -— N/A no package.json present at audit root (single-file audit) — skipped per instructions - -Setup -— N/A (reduced relevance) GamutProvider / ColorMode / Background / theme augmentation are app-root/project concerns; none appear in this file, which is expected for a single leaf component and is not itself a violation. -Note: the file does use TypeScript (typed props via `interface CustomConfirmProps`), so if/when this component is reviewed in its real project, Check 2's "Theme augmentation" row should still be verified there. - -Import patterns -✓ Deep dist imports none found -✓ Deep src imports none found -Note: the file contains no import from @codecademy/gamut, @codecademy/gamut-styles, or @codecademy/variance at all — see "Bespoke component duplication" below; this is the root cause of most findings in this report. - -SCSS modules, className & inline style [→ gamut-system-props] [→ gamut-style-utilities] [→ gamut-color-mode] -✓ SCSS/CSS imports none found -✓ className on Gamut components none found (no Gamut components imported/used) -✗ inline style on JSX elements 2 occurrences — hardcoded layout/color values on plain
s that should be a Gamut Modal/Overlay (or, at minimum, Box) using system props and semantic tokens -CustomConfirm.tsx:28
— backdrop; Gamut's Modal/Overlay provides this scrim for free -CustomConfirm.tsx:32
— dialog surface; hardcoded 'white' fill and pixel literals instead of a semantic bg/p/layout system props - -Nested selectors [→ gamut-system-props] [→ gamut-style-utilities] -✓ none found (no styled/Emotion template literals in this file) - -styled(GamutComponent) bypassing system props [→ gamut-system-props] [→ gamut-style-utilities] -✓ none found (styled is not imported/used) - -Hardcoded colors [→ gamut-color-mode] -✓ no hex literals (#RGB/#RRGGBB) found -⚠ CustomConfirm.tsx:28 'rgba(0,0,0,0.5)' → semantic: (n/a — scrim) | palette: — | note: not a hex literal so outside Check 4's strict discovery pattern, but the same class of problem as the inline-style finding above: a hand-set backdrop color that a Gamut Modal/Overlay would supply via its own scrim styling, no token needed -⚠ CustomConfirm.tsx:32 'white' → semantic: background | palette: white | note: literal CSS color keyword; if this stays a bespoke element, prefer bg="background" on a Box/Card rather than the keyword - -Test setup [→ gamut-testing] -— N/A this is a component file, not a test file; no **tests**/.test./.spec. files exist alongside it in this fixture - -Bespoke component duplication (heuristic — confirm manually) [→ gamut-component-first] -✗ CustomConfirm.tsx:27 role="dialog" with no Modal/Dialog import from @codecademy/gamut — this is a hand-rolled confirmation dialog; strongly recommend Modal (see decision table in gamut-component-first) -✗ CustomConfirm.tsx:14-21 manual Escape-key dismiss (e.key === 'Escape' + document.addEventListener('keydown', ...)) with no Overlay/FocusTrap/PopoverContainer import — Gamut's Modal/Overlay already implements Escape-to-close -⚠ CustomConfirm.tsx:29,33 manual outside-click dismiss (backdrop onClick={onClose} + inner content onClick={(e) => e.stopPropagation()}) — same class of hand-rolled dismiss logic as the Escape handler; Gamut's Modal/Overlay/PopoverContainer handles outside-click dismissal internally, so this doesn't need to be reimplemented (flagged as a warning since it's a variant of the literal addEventListener('click', ...) pattern the check greps for, but functionally identical) -⚠ CustomConfirm.tsx (whole file) filename CustomConfirm.tsx does not literally match the Check 6 Step 2 filename list (Modal, Dialog, Dropdown, Tooltip, Popover, Menu, Toast, Accordion, Tabs, Pagination, Avatar, Badge, Tag), so the filename-matching heuristic doesn't fire — but the component's actual behavior (role="dialog", overlay + centered surface, Escape/outside-click dismiss, confirm/cancel actions) is exactly a confirmation modal. Worth flagging manually alongside the Step 1/3 hits above rather than relying on filename alone. -⚠ CustomConfirm.tsx:36-37 plain