anti-patterns.md

  1---
  2name: anti-patterns
  3description: Use when adding, modifying, or debugging an anti-pattern detection rule in this repo. Walks through the TDD recipe, the rule schema, all five plug-in points, jsdom constraints, the cross-validation step against the impeccable skill, and the post-implementation checklist. Trigger this for any work touching src/detect-antipatterns.mjs, tests/fixtures/antipatterns/, or extension/detector/.
  4tools: Read, Edit, Write, Glob, Grep, Bash, mcp__claude-in-chrome__navigate, mcp__claude-in-chrome__javascript_tool, mcp__claude-in-chrome__tabs_context_mcp, mcp__claude-in-chrome__tabs_create_mcp
  5---
  6
  7# Anti-Pattern Engine Maintenance
  8
  9This agent handles every step of adding or modifying an anti-pattern detection rule in the impeccable repo. The rule engine is wired into many places and a single source-of-truth design ties them together. Skip any step at your peril — the build's cross-validator will fail loudly if drift slips in.
 10
 11## The five things that need to stay in sync
 12
 13When you add a rule, all of these update or get regenerated:
 14
 15| Where | What | How it stays in sync |
 16|---|---|---|
 17| `src/detect-antipatterns.mjs` `ANTIPATTERNS` | Rule metadata (id, category, name, description, skillSection, skillGuideline) and the detection logic (`checkXxx`) | **Hand-edited.** Source of truth. |
 18| `src/detect-antipatterns-browser.js` | Browser-bundled engine for the public site overlay | Generated by `bun run build:browser` |
 19| `extension/detector/detect.js` | Browser-bundled engine for the Chrome extension | Generated by `bun run build:extension` |
 20| `extension/detector/antipatterns.json` | Rule list (id, name, category, description) for the extension's devtools panel — drives rule toggles UI | Generated by `bun run build:extension` |
 21| `public/js/generated/counts.js` | `DETECTION_COUNT` integer for homepage display | Generated by `bun run build` |
 22| `source/skills/impeccable/SKILL.md` | Per-rule **DON'T** line in the right `### Section` — taught to users via the impeccable skill | **Hand-edited.** Validator catches drift. |
 23
 24The CLI (`bin/cli.js`) imports `ANTIPATTERNS` directly from `src/detect-antipatterns.mjs` — no separate sync needed.
 25
 26## Rule schema
 27
 28Each entry in the `ANTIPATTERNS` array (around src/detect-antipatterns.mjs:77) looks like this:
 29
 30```js
 31{
 32  id: 'icon-tile-stack',                          // kebab-case, unique, stable
 33  category: 'slop',                                // 'slop' or 'quality' (see below)
 34  name: 'Icon tile stacked above heading',        // human-readable, used in extension UI
 35  description:                                     // 1–2 sentences. Used in CLI output, extension tooltips, web overlay labels
 36    'A small rounded-square icon container above a heading is the universal AI feature-card template — every generator outputs this exact shape. Try a side-by-side icon and heading, or let the icon sit in flow without its own container.',
 37  skillSection: 'Typography',                      // OPTIONAL but strongly recommended. Must be one of the parser's allowed sections (see below)
 38  skillGuideline: 'large icons with rounded corners above every heading',  // OPTIONAL but strongly recommended. Substring that must appear in some **DON'T**: line of the named section in source/skills/impeccable/SKILL.md
 39}
 40```
 41
 42### Categories
 43
 44- **`slop`** = "AI tells". Patterns that scream *AI generated this*. Things like purple gradients, gradient text, dark glow accents, thick side borders, icon-tile-stacks. Flagging these is about taste and freshness, not correctness.
 45- **`quality`** = real design or accessibility issues regardless of who wrote the code. WCAG contrast, line length, padding, line height, justified text, skipped headings, etc.
 46
 47If you're not sure, ask: *"would a human designer who's careful and tasteful still ship this?"* If no, it's `quality`. If they would (because it works fine, it just looks templated), it's `slop`.
 48
 49### `skillSection` allowed values
 50
 51These are the section names the `readPatterns()` parser at scripts/lib/utils.js:221 understands. Use **exactly** these strings (note the parser normalizes `Color & Theme``Color & Contrast`):
 52
 53```
 54Typography
 55Color & Contrast
 56Layout & Space
 57Visual Details
 58Motion
 59Interaction
 60Responsive
 61UX Writing
 62```
 63
 64### `skillGuideline` substring
 65
 66A 3–6 word substring that appears verbatim in some `**DON'T**:` line of the named section in `source/skills/impeccable/SKILL.md`. The build validator checks this with `String.includes()`. Pick a substring that's:
 67
 68- Short enough that benign rewordings of the DON'T won't break it
 69- Specific enough that it can't accidentally collide with an unrelated DON'T
 70
 71Examples: `'AI color palette'`, `'large icons with rounded corners above every heading'`, `'WCAG AA contrast'`.
 72
 73If a rule genuinely doesn't deserve a skill DON'T (rare — only the most niche a11y-only rules), omit both `skillSection` and `skillGuideline`. The validator skips rules without `skillGuideline`.
 74
 75## The TDD recipe (always do it in this order)
 76
 77This order is non-negotiable. Fixture and failing test before implementation. The full suite must run between the rule going in and you committing.
 78
 79### 1. Write the fixture (two-column convention)
 80
 81A single HTML file at `tests/fixtures/antipatterns/{rule-id}.html` with two columns: left = should-flag, right = should-pass. Each test case carries a unique heading text so the test can match snippets back to expectations.
 82
 83Convention skeleton:
 84
 85```html
 86<!DOCTYPE html>
 87<html>
 88<head>
 89  <style>
 90    .grid { display: grid; grid-template-columns: 1fr 1fr; gap: 32px; max-width: 960px; margin: 0 auto; padding: 24px; }
 91    .col h2 { font-size: 14px; text-transform: uppercase; }
 92    /* ... per-case styles with EXPLICIT pixel dimensions (jsdom can't lay out) ... */
 93  </style>
 94</head>
 95<body>
 96  <div class="grid">
 97    <div class="col" data-col="flag">
 98      <h2>Should flag</h2>
 99      <!-- 4–6 cases that should be flagged, each with a unique <h3> text -->
100    </div>
101    <div class="col" data-col="pass">
102      <h2>Should pass</h2>
103      <!-- 5–8 cases that should NOT be flagged: cover every false-positive shape you can think of -->
104    </div>
105  </div>
106  <script src="/js/detect-antipatterns-browser.js"></script>
107</body>
108</html>
109```
110
111The script tag at the bottom is critical — it lets you load the fixture in the browser via `http://localhost:3000/fixtures/antipatterns/{rule-id}.html` (served by `server/index.js:62` route for `/fixtures/*`).
112
113**Should-pass cases must cover the false-positive shapes you can think of in advance.** A good fixture has 5+ pass cases. The icon-tile-stack fixture covers: round avatar, wide thumbnail, side-by-side, no-icon, too-tiny, too-huge.
114
115### 2. Write the failing test
116
117Add to `tests/detect-antipatterns-fixtures.test.mjs` in its own `describe` block. Use the snippet-substring matching pattern — the test parses heading text out of each finding's snippet and asserts membership against expected lists:
118
119```js
120describe('detectHtml — {rule-id}', () => {
121  const SHOULD_FLAG = ['Heading One', 'Heading Two', /* ... */];
122  const SHOULD_PASS = ['Pass Heading One', /* ... */];
123
124  it('{rule-id}: flags only the should-flag column', async () => {
125    const f = await detectHtml(path.join(FIXTURES, '{rule-id}.html'));
126    const flagged = new Set();
127    for (const r of f) {
128      if (r.antipattern !== '{rule-id}') continue;
129      const m = (r.snippet || '').match(/"([^"]+)"/);
130      if (m) flagged.add(m[1]);
131    }
132    for (const text of SHOULD_FLAG) {
133      assert.ok(flagged.has(text), `expected "${text}" to be flagged`);
134    }
135    for (const text of SHOULD_PASS) {
136      assert.ok(!flagged.has(text), `"${text}" should NOT be flagged`);
137    }
138  });
139});
140```
141
142For this to work, the rule's snippet **must include the heading text in quotes**. See "Snippet conventions" below.
143
144Run `node --test tests/detect-antipatterns-fixtures.test.mjs` and **watch it fail**. If it doesn't fail, your test is wrong.
145
146### 3. Add the rule definition
147
148Add a new entry to the `ANTIPATTERNS` array in `src/detect-antipatterns.mjs`. Place it in the right category section (slop or quality). Fill in all fields including `skillSection` and `skillGuideline`.
149
150### 4. Implement the pure check function
151
152Add a `checkXxx(opts)` function alongside the others (`checkColors`, `checkBorders`, `checkMotion`, `checkGlow`, `checkIconTile`, etc.). The pure function takes a plain options object — no DOM access — and returns an array of `{ id, snippet }`. This makes it testable and reusable across the browser/Node adapters.
153
154Example shape (see `checkIconTile` in src/detect-antipatterns.mjs for a real one):
155
156```js
157function checkXxx(opts) {
158  const { tag, /* whatever fields the rule needs */ } = opts;
159  if (SAFE_TAGS.has(tag)) return [];
160  // ... your detection logic ...
161  if (matches) {
162    return [{ id: 'rule-id', snippet: `... "${headingText}"` }];
163  }
164  return [];
165}
166```
167
168### 5. Add the two adapters
169
170Two adapters wrap the pure function with environment-specific input gathering:
171
172- **`checkElementXxxDOM(el)`** — for the browser. Uses `getComputedStyle(el)` and `el.getBoundingClientRect()`.
173- **`checkElementXxx(el, tag, window)`** — for jsdom (Node). Uses `window.getComputedStyle(el)` and **must read explicit pixel dimensions from `parseFloat(style.width)`** instead of bounding rects, because **jsdom does not lay out**`getBoundingClientRect()` returns 0×0 for everything.
174
175If your rule needs vertical positioning info (e.g. "icon must be above heading"), that check is browser-only — gate it behind `if (headingTop && siblingBottom)` so the Node path skips it. The structural checks alone (sizes, sibling identity, classes) are enough for the fixture.
176
177### 6. Wire into both element-iteration loops
178
179Two loops iterate every element on the page. You need to add your DOM-adapter call to **both**:
180
181- **Browser loop** at src/detect-antipatterns.mjs:1837 (`for (const el of document.querySelectorAll('*'))` with the `findings` spread). Add a line like:
182  ```js
183  ...checkElementXxxDOM(el).map(f => ({ type: f.id, detail: f.snippet })),
184  ```
185- **Node (jsdom) loop** at src/detect-antipatterns.mjs:2058 (in `detectHtml`). Add a block like:
186  ```js
187  for (const f of checkElementXxx(el, tag, window)) {
188    findings.push(finding(f.id, filePath, f.snippet));
189  }
190  ```
191
192Forgetting one of these is the most common mistake — the test passes but the live page doesn't show anything (or vice versa).
193
194### 7. Add the SKILL.md DON'T line if the rule doesn't reuse an existing one
195
196Open `source/skills/impeccable/SKILL.md`, find the right `### Section`, and add a **DON'T** line that contains your `skillGuideline` substring verbatim. Match the style of existing DON'Ts (terse, prescriptive, 1–2 sentences max).
197
198If your rule reuses an existing DON'T (e.g. multiple engine rules can map to the same skill guidance, like `side-tab` and `border-accent-on-rounded` both pointing to `'thick colored border on one side'`), no skill edit is needed.
199
200### 8. Run the build (this regenerates everything and validates)
201
202```bash
203bun run build && bun run build:browser && bun run build:extension
204```
205
206This regenerates:
207- `src/detect-antipatterns-browser.js` (public-site detector)
208- `extension/detector/detect.js` (extension detector)
209- `extension/detector/antipatterns.json` (extension rule list, includes description)
210- `public/js/generated/counts.js` (DETECTION_COUNT)
211
212And validates:
213- Cross-checks every rule with `skillGuideline` against `source/skills/impeccable/SKILL.md` via `validateAntipatternRules()` in scripts/build.js. **Build fails if drift exists.**
214
215### 9. Run the test suite
216
217```bash
218bun run test
219```
220
221166 unit tests + N fixture tests, including your new one. All should be green.
222
223### 10. Verify on a live page in the browser
224
225Don't skip this. The jsdom path uses `parseFloat(style.width)` and the browser path uses `getBoundingClientRect()` — they can disagree. The fixture test catches one path; manual browser verification catches the other.
226
227```
228http://localhost:3000/fixtures/antipatterns/{rule-id}.html
229http://localhost:3000/antipattern-examples/{your-example}.html  (if relevant)
230http://localhost:3000/                                           (no false positives on real pages)
231```
232
233Use the chrome MCP tools (`mcp__claude-in-chrome__navigate` + `mcp__claude-in-chrome__javascript_tool`) to inject `window.impeccableScan()` and read `.impeccable-overlay` / `.impeccable-label` from the DOM to verify. Don't try to screenshot — the overlays are decorative; read them programmatically.
234
235## Snippet conventions
236
237The fixture-test convention extracts the heading text from a finding's snippet using regex `/"([^"]+)"/` — so **wrap the identifying heading text in straight double quotes** in your snippet. Examples:
238
239- `'80x80px icon tile above h3 "Lightning Fast"'`
240- `'4.5:1 (need 4.5:1) — text #808080 on #3b82f6'`  ← uses element identifiers instead, since this rule isn't anchored to a heading
241
242If your rule isn't naturally anchored to a heading, pick another stable identifier (a class name, the parent element's text, etc.) and document the test pattern in the test itself.
243
244## jsdom constraints (the most common gotcha)
245
246- **No layout.** `getBoundingClientRect()` returns `0×0` always. Read `parseFloat(style.width)` and `parseFloat(style.height)` instead — jsdom does honor explicit pixel widths in `<style>` and inline styles.
247- **`background:` shorthand isn't decomposed.** `style.backgroundColor` and `style.backgroundImage` may be empty even when `style="background: ..."` is set. The existing `resolveBackground()` and `resolveGradientStops()` helpers (src/detect-antipatterns.mjs:631 and src/detect-antipatterns.mjs:670) handle this — use them.
248- **Computed colors are normalized in real browsers, not in jsdom.** A browser returns `rgb(59, 130, 246)`; jsdom may return the original hex. The `parseGradientColors()` helper handles both.
249- **No SAFE_TAGS skipping for parent walks.** When walking ancestors, you don't get the `SAFE_TAGS` filter the main loop applies — be explicit.
250
251## Where to find concrete example rules to learn from
252
253- Simplest border check: **`side-tab`** — `checkBorders()` at src/detect-antipatterns.mjs:312
254- Color/contrast with gradient handling: **`low-contrast`** — `checkColors()` at src/detect-antipatterns.mjs:339
255- Element-relationship check (siblings): **`icon-tile-stack`** — `checkIconTile()` at src/detect-antipatterns.mjs:425
256- Page-level / cross-element: **`flat-type-hierarchy`** — `checkPageTypography()` at src/detect-antipatterns.mjs:1080
257- Motion/animation: **`bounce-easing`** — `checkMotion()` at src/detect-antipatterns.mjs:425
258
259## Pre-commit checklist
260
261Before you commit a new rule, all of these MUST be true. The first three are non-negotiable — if any is missing, the engine, extension, public site, and skill will silently drift apart.
262
263- [ ] Test passes: `bun run test` is green
264- [ ] Build passes: `bun run build && bun run build:browser && bun run build:extension` is green (validator says `✓ Validated N/N anti-pattern rules`)
265- [ ] Live verification: rule fires on a real page and produces zero false positives on the homepage `http://localhost:3000/`
266- [ ] Both element loops were updated (browser DOM at line ~1846 + Node jsdom at line ~2058)
267- [ ] Rule has a corresponding SKILL.md DON'T (or explicitly omitted `skillGuideline`)
268- [ ] Snippet format matches the test's extraction regex
269- [ ] Fixture covers ≥4 should-flag and ≥5 should-pass cases
270- [ ] Commit only the relevant files — `git status` will show many unrelated stale skill builds; do not stage them
271
272## Things that have bitten previous sessions
273
274- **Forgot to run `bun run build:extension`** — extension JSON went stale, missing the new rule. Symptom: extension panel doesn't show toggle for new rule. Fix: always run all three build commands.
275- **Forgot to update both loops** — test passed in jsdom but live browser was silent (or vice versa). Fix: grep for an existing rule's adapter call and copy its placement.
276- **Used a `skillGuideline` substring that doesn't appear in SKILL.md** — validator fails. Fix: the substring must appear verbatim in some `**DON'T**:` line of the named section.
277- **Used the wrong `skillSection` name** — `Color & Theme` vs `Color & Contrast` (parser normalizes the former to the latter, so use `Color & Contrast`).
278- **Wrote the fixture without explicit pixel dimensions** — jsdom returned 0×0 and the rule never matched. Fix: always set `width: Npx; height: Npx` in CSS for fixture elements, or use inline style attributes.