diff --git a/.claude/skills/critique/scripts/detect-antipatterns.mjs b/.claude/skills/critique/scripts/detect-antipatterns.mjs index d67f493c8..e7bd6479c 100644 --- a/.claude/skills/critique/scripts/detect-antipatterns.mjs +++ b/.claude/skills/critique/scripts/detect-antipatterns.mjs @@ -79,6 +79,13 @@ const ANTIPATTERNS = [ }, ]; +/** Check if content looks like a full page (not a component/partial) */ +function isFullPage(content) { + // Strip HTML comments before checking — they might mention / in prose + const stripped = content.replace(//g, ''); + return /]|]/i.test(stripped); +} + function getAP(id) { return ANTIPATTERNS.find(a => a.id === id); } @@ -289,9 +296,11 @@ async function detectHtml(filePath) { } } - // Page-level typography checks - for (const f of checkPageTypography(document, window)) { - findings.push(finding(f.id, filePath, f.snippet)); + // Page-level typography checks (only for full pages, not partials) + if (isFullPage(html)) { + for (const f of checkPageTypography(document, window)) { + findings.push(finding(f.id, filePath, f.snippet)); + } } window.close(); @@ -540,8 +549,11 @@ function detectText(content, filePath) { } } - for (const analyzer of REGEX_ANALYZERS) { - findings.push(...analyzer(content, filePath)); + // Page-level analyzers only run on full pages + if (isFullPage(content)) { + for (const analyzer of REGEX_ANALYZERS) { + findings.push(...analyzer(content, filePath)); + } } return findings; @@ -712,7 +724,7 @@ if (isMainModule) main(); export { ANTIPATTERNS, SAFE_TAGS, OVERUSED_FONTS, GENERIC_FONTS, - checkElementBorders, checkPageTypography, isNeutralColor, + checkElementBorders, checkPageTypography, isNeutralColor, isFullPage, detectHtml, detectUrl, detectText, walkDir, formatFindings, SCANNABLE_EXTENSIONS, SKIP_DIRS, }; diff --git a/source/skills/critique/scripts/detect-antipatterns.mjs b/source/skills/critique/scripts/detect-antipatterns.mjs index d67f493c8..e7bd6479c 100644 --- a/source/skills/critique/scripts/detect-antipatterns.mjs +++ b/source/skills/critique/scripts/detect-antipatterns.mjs @@ -79,6 +79,13 @@ const ANTIPATTERNS = [ }, ]; +/** Check if content looks like a full page (not a component/partial) */ +function isFullPage(content) { + // Strip HTML comments before checking — they might mention / in prose + const stripped = content.replace(//g, ''); + return /]|]/i.test(stripped); +} + function getAP(id) { return ANTIPATTERNS.find(a => a.id === id); } @@ -289,9 +296,11 @@ async function detectHtml(filePath) { } } - // Page-level typography checks - for (const f of checkPageTypography(document, window)) { - findings.push(finding(f.id, filePath, f.snippet)); + // Page-level typography checks (only for full pages, not partials) + if (isFullPage(html)) { + for (const f of checkPageTypography(document, window)) { + findings.push(finding(f.id, filePath, f.snippet)); + } } window.close(); @@ -540,8 +549,11 @@ function detectText(content, filePath) { } } - for (const analyzer of REGEX_ANALYZERS) { - findings.push(...analyzer(content, filePath)); + // Page-level analyzers only run on full pages + if (isFullPage(content)) { + for (const analyzer of REGEX_ANALYZERS) { + findings.push(...analyzer(content, filePath)); + } } return findings; @@ -712,7 +724,7 @@ if (isMainModule) main(); export { ANTIPATTERNS, SAFE_TAGS, OVERUSED_FONTS, GENERIC_FONTS, - checkElementBorders, checkPageTypography, isNeutralColor, + checkElementBorders, checkPageTypography, isNeutralColor, isFullPage, detectHtml, detectUrl, detectText, walkDir, formatFindings, SCANNABLE_EXTENSIONS, SKIP_DIRS, }; diff --git a/tests/detect-antipatterns.test.js b/tests/detect-antipatterns.test.js index c55cafbfa..cacba15ce 100644 --- a/tests/detect-antipatterns.test.js +++ b/tests/detect-antipatterns.test.js @@ -3,7 +3,7 @@ import fs from 'fs'; import path from 'path'; import { spawnSync } from 'child_process'; import { - ANTIPATTERNS, checkElementBorders, isNeutralColor, + ANTIPATTERNS, checkElementBorders, isNeutralColor, isFullPage, detectHtml, detectText, walkDir, SCANNABLE_EXTENSIONS, } from '../source/skills/critique/scripts/detect-antipatterns.mjs'; @@ -145,12 +145,14 @@ describe('detectText — overused fonts', () => { describe('detectText — flat type hierarchy', () => { test('flags sizes too close together', () => { - const f = detectText('h1{font-size:18px}h2{font-size:16px}h3{font-size:15px}p{font-size:14px}.s{font-size:13px}', 'test.css'); + const page = ''; + const f = detectText(page, 'test.html'); expect(f.some(r => r.antipattern === 'flat-type-hierarchy')).toBe(true); }); test('passes good hierarchy', () => { - const f = detectText('h1{font-size:48px}h2{font-size:32px}p{font-size:16px}.s{font-size:12px}', 'test.css'); + const page = ''; + const f = detectText(page, 'test.html'); expect(f.filter(r => r.antipattern === 'flat-type-hierarchy')).toHaveLength(0); }); }); @@ -192,6 +194,14 @@ describe('detectHtml — jsdom', () => { expect(cleanFindings).toHaveLength(0); }); + test('partial-component: flags borders, skips page-level', async () => { + const f = await detectHtml(path.join(FIXTURES, 'partial-component.html')); + expect(f.some(r => r.antipattern === 'side-tab')).toBe(true); + expect(f.filter(r => r.antipattern === 'flat-type-hierarchy')).toHaveLength(0); + expect(f.filter(r => r.antipattern === 'single-font')).toHaveLength(0); + expect(f.filter(r => r.antipattern === 'overused-font')).toHaveLength(0); + }); + test('legitimate-borders has minimal false positives', async () => { const f = await detectHtml(path.join(FIXTURES, 'legitimate-borders.html')); const borderFindings = f.filter(r => r.antipattern === 'side-tab' || r.antipattern === 'border-accent-on-rounded'); @@ -212,6 +222,47 @@ describe('detectHtml — jsdom', () => { }); }); +// --------------------------------------------------------------------------- +// Full page vs partial detection +// --------------------------------------------------------------------------- + +describe('isFullPage', () => { + test('detects DOCTYPE', () => expect(isFullPage('')).toBe(true)); + test('detects ', () => expect(isFullPage('')).toBe(true)); + test('detects ', () => expect(isFullPage('')).toBe(true)); + test('rejects component/partial', () => expect(isFullPage('
content
')).toBe(false)); + test('rejects JSX', () => expect(isFullPage('export default function Card() { return
hi
}')).toBe(false)); +}); + +describe('partials skip page-level checks', () => { + test('regex: partial with flat hierarchy is not flagged', () => { + const partial = '
text
\n
text
\n
text
'; + const f = detectText(partial, 'card.tsx'); + expect(f.filter(r => r.antipattern === 'flat-type-hierarchy')).toHaveLength(0); + }); + + test('regex: partial with single overused font is not flagged for single-font', () => { + const partial = `
text
\n`.repeat(25); + const f = detectText(partial, 'card.tsx'); + expect(f.filter(r => r.antipattern === 'single-font')).toHaveLength(0); + }); + + test('regex: partial still flags border anti-patterns', () => { + const partial = '
card
'; + const f = detectText(partial, 'card.tsx'); + expect(f.some(r => r.antipattern === 'side-tab')).toBe(true); + }); + + test('regex: full page with flat hierarchy IS flagged', () => { + const page = '\n' + + '

h1

\n

h2

\n' + + '

p

\ns\n' + + 'sm\n'; + const f = detectText(page, 'index.html'); + expect(f.some(r => r.antipattern === 'flat-type-hierarchy')).toBe(true); + }); +}); + // --------------------------------------------------------------------------- // ANTIPATTERNS registry // --------------------------------------------------------------------------- diff --git a/tests/fixtures/antipatterns/partial-component.html b/tests/fixtures/antipatterns/partial-component.html new file mode 100644 index 000000000..03324748b --- /dev/null +++ b/tests/fixtures/antipatterns/partial-component.html @@ -0,0 +1,11 @@ + + + + +
+
+

Card Title

+

Card description with close font sizes.

+
+
+