mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-20 18:16:30 +03:00
Add mechanical pre-scan for typeset and layout (#345)
* Add mechanical pre-scan for typeset and layout commands. Introduce --scope filtering, layout/type rule scopes, DESIGN.md font-size validation, and pre-scan steps in the skill references so agents run detect before LLM judgment. Fixes #149 Co-authored-by: Cursor <cursoragent@cursor.com> * Add isolated sub-agent orchestration for typeset and layout pre-scans. Run the mechanical detector and visual assessment in parallel sub-agents so deterministic findings cannot anchor LLM judgment, matching the critique pattern Paul requested on PR #345. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix: reject bare --scope so detect never scans unscoped by mistake. When --scope had no value, the CLI dropped the flag and ran a full scan instead of failing, which could silently use the wrong rule set during typeset/layout pre-scans. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix: require both typeset and layout assessments in sub-agents. Close a loophole where agents ran only the mechanical pre-scan inline by interpreting "running both" as permitting one inline assessment. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Abdul Wahab <abdulwahab@Abduls-MacBook-Pro-2.local> Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
co-authored by
Abdul Wahab
Cursor
parent
c11cc7b58c
commit
f40e2f8f0a
@@ -15,6 +15,7 @@ import {
|
||||
isAllowedColorRaw,
|
||||
isAllowedFont,
|
||||
isAllowedRadiusRaw,
|
||||
isAllowedFontSizeRaw,
|
||||
loadDesignSystemForCwd,
|
||||
normalizeDesignSystem,
|
||||
} from '../cli/engine/design-system.mjs';
|
||||
@@ -31,8 +32,9 @@ function sampleDesignSystem() {
|
||||
return normalizeDesignSystem({
|
||||
frontmatter: {
|
||||
typography: {
|
||||
display: { fontFamily: 'Avenir Next, Georgia, serif' },
|
||||
body: { fontFamily: 'IBM Plex Sans, Arial, sans-serif' },
|
||||
display: { fontFamily: 'Avenir Next, Georgia, serif', fontSize: 'clamp(2.5rem, 6vw, 4rem)' },
|
||||
body: { fontFamily: 'IBM Plex Sans, Arial, sans-serif', fontSize: '16px' },
|
||||
label: { fontFamily: 'IBM Plex Sans, Arial, sans-serif', fontSize: '0.875rem' },
|
||||
},
|
||||
colors: {
|
||||
ink: '#241f1a',
|
||||
@@ -96,6 +98,15 @@ describe('normalizeDesignSystem()', () => {
|
||||
assert.equal(isAllowedRadiusRaw('100px', designSystem), true);
|
||||
assert.equal(isAllowedRadiusRaw('9999px', designSystem), true);
|
||||
assert.equal(isAllowedRadiusRaw('18px', designSystem), false);
|
||||
|
||||
assert.equal(isAllowedFontSizeRaw('16px', designSystem), true);
|
||||
assert.equal(isAllowedFontSizeRaw('1rem', designSystem), true);
|
||||
assert.equal(isAllowedFontSizeRaw('0.875rem', designSystem), true);
|
||||
assert.equal(isAllowedFontSizeRaw('14px', designSystem), true);
|
||||
assert.equal(isAllowedFontSizeRaw('12.5px', designSystem), false);
|
||||
assert.equal(isAllowedFontSizeRaw('1.2em', designSystem), true);
|
||||
assert.equal(isAllowedFontSizeRaw('clamp(1rem, 2vw, 2rem)', designSystem), true);
|
||||
assert.equal(isAllowedFontSizeRaw('var(--text-body)', designSystem), true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -220,9 +231,50 @@ scale.style.cssText = 'font-family:' + MONO + '; font-size: 10px;';
|
||||
|
||||
assert.deepEqual(
|
||||
findings.map((item) => item.ignoreValue),
|
||||
['#ff00aa', 'Poppins', '#cc00ff'],
|
||||
['10px', '#ff00aa', 'Poppins', '#cc00ff'],
|
||||
);
|
||||
});
|
||||
|
||||
it('reports literal font sizes outside the DESIGN.md type ramp', () => {
|
||||
const designSystem = sampleDesignSystem();
|
||||
const source = `.off-ramp {
|
||||
font-size: 12.5px;
|
||||
}
|
||||
const label = { fontSize: "11px" };
|
||||
const badge = { className: "text-[10px]" };
|
||||
/* font-size: 9px; */
|
||||
.on-ramp {
|
||||
font-size: 1rem;
|
||||
}
|
||||
`;
|
||||
const findings = checkSourceDesignSystem(source, '/tmp/sizes.css', { designSystem });
|
||||
const fontSizeFindings = findings.filter((item) => item.antipattern === 'design-system-font-size');
|
||||
|
||||
assert.equal(fontSizeFindings.length, 3);
|
||||
assert.deepEqual(
|
||||
fontSizeFindings.map((item) => item.ignoreValue),
|
||||
['12.5px', '11px', '10px'],
|
||||
);
|
||||
assert.deepEqual(
|
||||
fontSizeFindings.map((item) => item.line),
|
||||
[2, 4, 5],
|
||||
);
|
||||
});
|
||||
|
||||
it('abstains on font-size checks when DESIGN.md has no literal ramp steps', () => {
|
||||
const designSystem = normalizeDesignSystem({
|
||||
frontmatter: {
|
||||
typography: {
|
||||
display: { fontFamily: 'Avenir Next, Georgia, serif', fontSize: 'clamp(2.5rem, 6vw, 4rem)' },
|
||||
body: { fontFamily: 'IBM Plex Sans, Arial, sans-serif', fontSize: 'clamp(1rem, 2vw, 1.125rem)' },
|
||||
},
|
||||
},
|
||||
});
|
||||
assert.equal(designSystem.hasFontSizes, false);
|
||||
|
||||
const findings = checkSourceDesignSystem('.bad { font-size: 12.5px; }', '/tmp/clamp-only.css', { designSystem });
|
||||
assert.equal(findings.some((item) => item.antipattern === 'design-system-font-size'), false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('collectStaticDesignSystemFindings()', () => {
|
||||
|
||||
@@ -276,8 +276,9 @@ describe('detectHtml — static HTML/CSS fixtures', () => {
|
||||
const designSystem = normalizeDesignSystem({
|
||||
frontmatter: {
|
||||
typography: {
|
||||
display: { fontFamily: 'Avenir Next, Georgia, serif' },
|
||||
body: { fontFamily: 'IBM Plex Sans, Arial, sans-serif' },
|
||||
display: { fontFamily: 'Avenir Next, Georgia, serif', fontSize: 'clamp(2.5rem, 6vw, 4rem)' },
|
||||
body: { fontFamily: 'IBM Plex Sans, Arial, sans-serif', fontSize: '16px' },
|
||||
label: { fontFamily: 'IBM Plex Sans, Arial, sans-serif', fontSize: '14px' },
|
||||
},
|
||||
colors: {
|
||||
ink: '#241f1a',
|
||||
@@ -315,6 +316,13 @@ describe('detectHtml — static HTML/CSS fixtures', () => {
|
||||
designFindings.some((r) => r.antipattern === 'design-system-font' && /Google Fonts: Poppins/.test(r.snippet || '')),
|
||||
'expected source-level Google Fonts usage in HTML to be flagged',
|
||||
);
|
||||
assert.ok(
|
||||
designFindings.some((r) => r.antipattern === 'design-system-font-size' && /12\.5px/.test(r.snippet || '')),
|
||||
'expected off-ramp literal font-size to be flagged',
|
||||
);
|
||||
assert.doesNotMatch(snippets, /1rem is off/, 'documented rem step must pass');
|
||||
assert.doesNotMatch(snippets, /1\.2em is off/, 'relative em sizes are abstained on');
|
||||
assert.doesNotMatch(snippets, /16px is off|14px is off/, 'on-ramp sizes must pass');
|
||||
assert.doesNotMatch(snippets, /Undocumented color #ff00aa/, 'source and computed color findings should not duplicate');
|
||||
assert.doesNotMatch(snippets, /font-family: Poppins/, 'source and computed font findings should not duplicate');
|
||||
assert.doesNotMatch(snippets, /border-radius: 18px is outside/, 'source and computed radius findings should not duplicate');
|
||||
@@ -341,6 +349,8 @@ describe('detectHtml — static HTML/CSS fixtures', () => {
|
||||
}
|
||||
for (const label of [
|
||||
'Pass Display Font',
|
||||
'Pass Rem Font Size',
|
||||
'Pass Relative Font Size',
|
||||
'Pass Generic Font',
|
||||
'Pass Token Color',
|
||||
'Pass Alpha Color',
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
buildImportGraph, resolveImport,
|
||||
detectFrameworkConfig, isPortListening, FRAMEWORK_CONFIGS,
|
||||
} from '../cli/engine/detect-antipatterns.mjs';
|
||||
import { filterByScopes } from '../cli/engine/registry/antipatterns.mjs';
|
||||
import {
|
||||
checkElementTextOverflowDOM,
|
||||
checkPageTypography,
|
||||
@@ -1054,6 +1055,79 @@ rounded:
|
||||
}
|
||||
});
|
||||
|
||||
test('filterByScopes keeps only findings for the requested design domain', () => {
|
||||
const findings = [
|
||||
{ antipattern: 'flat-type-hierarchy' },
|
||||
{ antipattern: 'nested-cards' },
|
||||
{ antipattern: 'line-length' },
|
||||
];
|
||||
|
||||
expect(filterByScopes(findings, ['type']).map((f) => f.antipattern)).toEqual([
|
||||
'flat-type-hierarchy',
|
||||
'line-length',
|
||||
]);
|
||||
expect(filterByScopes(findings, ['layout']).map((f) => f.antipattern)).toEqual([
|
||||
'nested-cards',
|
||||
'line-length',
|
||||
]);
|
||||
expect(filterByScopes(findings, [])).toEqual(findings);
|
||||
});
|
||||
|
||||
test('--scope filters CLI output to a design domain', () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-cli-scope-'));
|
||||
try {
|
||||
fs.writeFileSync(path.join(dir, 'DESIGN.md'), `---
|
||||
typography:
|
||||
body:
|
||||
fontFamily: "IBM Plex Sans, Arial, sans-serif"
|
||||
fontSize: "16px"
|
||||
colors:
|
||||
ink: "#241f1a"
|
||||
paper: "#f7f4ee"
|
||||
---
|
||||
|
||||
# Design System
|
||||
`);
|
||||
fs.writeFileSync(path.join(dir, 'index.css'), `
|
||||
.bad {
|
||||
font-family: "IBM Plex Sans", Arial, sans-serif;
|
||||
font-size: 12.5px;
|
||||
color: #ff00aa;
|
||||
}
|
||||
`);
|
||||
|
||||
const full = runIn(dir, '--json', 'index.css');
|
||||
expect(full.code).toBe(2);
|
||||
const fullIds = JSON.parse(full.stdout).map((finding) => finding.antipattern);
|
||||
expect(fullIds).toContain('design-system-font-size');
|
||||
expect(fullIds).toContain('design-system-color');
|
||||
|
||||
const typeOnly = runIn(dir, '--json', '--scope', 'type', 'index.css');
|
||||
const typeIds = JSON.parse(typeOnly.stdout).map((finding) => finding.antipattern);
|
||||
expect(typeIds).toContain('design-system-font-size');
|
||||
expect(typeIds.some((id) => id === 'design-system-color')).toBe(false);
|
||||
|
||||
const badScope = runIn(dir, '--scope', 'bogus', 'index.css');
|
||||
expect(badScope.code).toBe(1);
|
||||
expect(badScope.stderr).toContain('Valid scopes:');
|
||||
|
||||
// A bare --scope must fail instead of silently scanning unscoped.
|
||||
const missingTrailing = runIn(dir, 'index.css', '--scope');
|
||||
expect(missingTrailing.code).toBe(1);
|
||||
expect(missingTrailing.stderr).toContain('--scope requires a value');
|
||||
|
||||
const missingBeforeFlag = runIn(dir, '--scope', '--json', 'index.css');
|
||||
expect(missingBeforeFlag.code).toBe(1);
|
||||
expect(missingBeforeFlag.stderr).toContain('--scope requires a value');
|
||||
|
||||
const emptyInline = runIn(dir, '--scope=', 'index.css');
|
||||
expect(emptyInline.code).toBe(1);
|
||||
expect(emptyInline.stderr).toContain('--scope requires a value');
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('detector designSystem.enabled=false disables CLI design-system rules', () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-cli-design-disabled-'));
|
||||
try {
|
||||
|
||||
@@ -38,8 +38,11 @@
|
||||
.flag-background { background-color: rgb(20, 180, 220); }
|
||||
.flag-border { border-color: #00a982; }
|
||||
.flag-radius { border-radius: 18px; }
|
||||
.flag-font-size { font-size: 12.5px; }
|
||||
|
||||
.pass-display-font { font-family: "Avenir Next", Georgia, serif; }
|
||||
.pass-rem-font-size { font-size: 1rem; }
|
||||
.pass-relative-font-size { font-size: 1.2em; }
|
||||
.pass-generic-font { font-family: ui-sans-serif, system-ui, sans-serif; }
|
||||
.pass-token-color { color: var(--brand-accent); }
|
||||
.pass-alpha-color { color: rgba(184, 66, 46, 0.45); }
|
||||
@@ -61,9 +64,12 @@
|
||||
<div class="case flag-background">Flag Background Cyan</div>
|
||||
<div class="case flag-border">Flag Border Teal</div>
|
||||
<div class="case flag-radius">Flag Radius Eighteen</div>
|
||||
<div class="case flag-font-size">Flag Font Size Twelve Point Five</div>
|
||||
<div class="case" data-font-source="https://fonts.googleapis.com/css2?family=Poppins:wght@400&display=swap">Flag Google Font Source</div>
|
||||
|
||||
<div class="case pass-display-font">Pass Display Font</div>
|
||||
<div class="case pass-rem-font-size">Pass Rem Font Size</div>
|
||||
<div class="case pass-relative-font-size">Pass Relative Font Size</div>
|
||||
<div class="case pass-generic-font">Pass Generic Font</div>
|
||||
<div class="case pass-token-color">Pass Token Color</div>
|
||||
<div class="case pass-alpha-color">Pass Alpha Color</div>
|
||||
|
||||
Reference in New Issue
Block a user