diff --git a/.changeset/layout-checks-in-done-and-preview.md b/.changeset/layout-checks-in-done-and-preview.md new file mode 100644 index 000000000..5055c796e --- /dev/null +++ b/.changeset/layout-checks-in-done-and-preview.md @@ -0,0 +1,6 @@ +--- +"@open-codesign/core": patch +"@open-codesign/desktop": patch +--- + +Check layout and accessibility in `preview` and `done`. After a page loads, a deterministic measurement looks for a page that scrolls sideways, text that its box clips, text with too little contrast (WCAG AA, 4.5:1, or 3:1 for large text), tap targets under 24 by 24 px that have another target next to them, and controls with no accessible name. It measures at the current width and again at a 390 px phone width. `preview` lists up to 20 findings in `layoutIssues`; `done` shows them under "Non-blocking warnings", and when one is present it lets the agent fix it and check it with `preview` instead of telling it to stop. Findings never change `ok` or the `done` status, so a false positive cannot cost a repair round. `done:verify:v1` now returns `warnings` and a `schemaVersion` next to `errors`. diff --git a/apps/desktop/src/main/done-verify.ts b/apps/desktop/src/main/done-verify.ts index fa316ce49..e46b2cc0f 100644 --- a/apps/desktop/src/main/done-verify.ts +++ b/apps/desktop/src/main/done-verify.ts @@ -19,10 +19,11 @@ import { mkdtemp, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath, pathToFileURL, URL } from 'node:url'; -import type { DoneError, DoneRuntimeVerifier } from '@open-codesign/core'; +import type { DoneError, DoneRuntimeResult, DoneRuntimeVerifier } from '@open-codesign/core'; import { findSystemChrome } from '@open-codesign/exporters'; import { buildSrcdoc } from '@open-codesign/runtime'; import type { Browser, ConsoleMessage, HTTPRequest, Page } from 'puppeteer-core'; +import { inspectLayout, layoutFailureWarning, layoutWarnings } from './layout-probe'; import { boundedPreview } from './preview-interactions'; import { buildWorkspacePreviewDocument, @@ -181,12 +182,13 @@ async function verifyWithSystemChrome( html: string, workspaceRoot?: string, signal?: AbortSignal, -): Promise { +): Promise { signal?.throwIfAborted(); const executablePath = await findSystemChrome(); const puppeteer = (await import('puppeteer-core')).default; const userDataDir = await mkdtemp(join(tmpdir(), 'codesign-done-chrome-')); const errors: DoneError[] = []; + const warnings: DoneError[] = []; const seen = new Set(); let browser: Browser | null = null; let page: Page | null = null; @@ -245,6 +247,13 @@ async function verifyWithSystemChrome( signal, ); await boundedPreview(settleAfterLoad(), SETTLE_AFTER_LOAD_MS + 100, signal); + // Layout findings are advice: a failed measurement is reported, never turned into an error. + try { + warnings.push(...layoutWarnings(await inspectLayout(page, signal))); + } catch (err) { + signal?.throwIfAborted(); + warnings.push(layoutFailureWarning(err)); + } } catch (err) { signal?.throwIfAborted(); pushUniqueError( @@ -259,11 +268,11 @@ async function verifyWithSystemChrome( await rm(userDataDir, { recursive: true, force: true }); } - return errors; + return { errors, warnings }; } export function makeRuntimeVerifier(options?: { workspaceRoot: string }): DoneRuntimeVerifier { - return async (artifactSource, context): Promise => { + return async (artifactSource, context): Promise => { context?.signal?.throwIfAborted(); const workspaceRoot = options ? resolve(options.workspaceRoot) : undefined; const srcdoc = workspaceRoot diff --git a/apps/desktop/src/main/done-verify.workspace.test.ts b/apps/desktop/src/main/done-verify.workspace.test.ts index 831c82ded..cbd8937b2 100644 --- a/apps/desktop/src/main/done-verify.workspace.test.ts +++ b/apps/desktop/src/main/done-verify.workspace.test.ts @@ -40,7 +40,10 @@ describe.skipIf(!chromeAvailable)('done workspace assets in system Chrome', () = const prefix = path.startsWith('screens/') ? '../' : ''; const source = sourceFor(`${prefix}logo.svg`, `${prefix}art/poster.svg`); await writeFile(join(workspace, path), source); - expect(await makeRuntimeVerifier({ workspaceRoot: workspace })(source, { path })).toEqual([]); + expect(await makeRuntimeVerifier({ workspaceRoot: workspace })(source, { path })).toEqual({ + errors: [], + warnings: [], + }); const preview = await runPreview({ workspaceRoot: workspace, path, vision: false }); expect(preview.ok).toBe(true); expect(preview.assetErrors).toEqual([]); @@ -49,7 +52,7 @@ describe.skipIf(!chromeAvailable)('done workspace assets in system Chrome', () = it('reports a missing relative asset with its URL', async () => { const source = sourceFor('logo.svg', 'missing.svg'); await writeFile(join(workspace, 'App.jsx'), source); - const errors = await makeRuntimeVerifier({ workspaceRoot: workspace })(source, { + const { errors } = await makeRuntimeVerifier({ workspaceRoot: workspace })(source, { path: 'App.jsx', }); expect(errors.some((error) => error.message.includes('missing.svg'))).toBe(true); @@ -60,7 +63,7 @@ describe.skipIf(!chromeAvailable)('done workspace assets in system Chrome', () = await writeFile(outside, svg); const source = sourceFor('logo.svg', pathToFileURL(outside).href); await writeFile(join(workspace, 'App.jsx'), source); - const errors = await makeRuntimeVerifier({ workspaceRoot: workspace })(source, { + const { errors } = await makeRuntimeVerifier({ workspaceRoot: workspace })(source, { path: 'App.jsx', }); expect( diff --git a/apps/desktop/src/main/harness-document.test.ts b/apps/desktop/src/main/harness-document.test.ts index 612719a4e..aa9d9f932 100644 --- a/apps/desktop/src/main/harness-document.test.ts +++ b/apps/desktop/src/main/harness-document.test.ts @@ -21,39 +21,46 @@ const browser = vi.hoisted(() => ({ vi.mock('@open-codesign/exporters', () => ({ findSystemChrome: async () => 'synthetic-chrome' })); vi.mock('./logger', () => ({ getLogger: () => ({ warn: vi.fn(), error: vi.fn() }) })); -vi.mock('puppeteer-core', () => ({ - default: { - launch: async () => { - const handlers = new Map void>(); - const page = { - setViewport: async () => {}, - setRequestInterception: async () => {}, - evaluateOnNewDocument: async () => {}, - on: (event: string, handler: (arg: unknown) => void) => handlers.set(event, handler), - goto: async (url: string) => { - const request: FakeRequest = { - url: () => url, - isNavigationRequest: () => true, - resourceType: () => 'document', - respond: vi.fn(async () => {}), - continue: vi.fn(async () => {}), - abort: vi.fn(async () => {}), - }; - browser.navigations.push({ - url, - existedOnDisk: existsSync(fileURLToPath(url)), - request, - }); - handlers.get('request')?.(request); - await vi.waitFor(() => expect(request.respond).toHaveBeenCalled()); - }, - evaluate: async () => ({ nodes: 1, width: 1280, height: 800 }), - close: async () => {}, - }; - return { newPage: async () => page, close: async () => {} }; +vi.mock('puppeteer-core', async () => { + const { samples } = await import('./layout-test-fixtures'); + return { + default: { + launch: async () => { + const handlers = new Map void>(); + const page = { + viewport: () => ({ width: 1280, height: 800 }), + setViewport: async () => {}, + setRequestInterception: async () => {}, + evaluateOnNewDocument: async () => {}, + on: (event: string, handler: (arg: unknown) => void) => handlers.set(event, handler), + goto: async (url: string) => { + const request: FakeRequest = { + url: () => url, + isNavigationRequest: () => true, + resourceType: () => 'document', + respond: vi.fn(async () => {}), + continue: vi.fn(async () => {}), + abort: vi.fn(async () => {}), + }; + browser.navigations.push({ + url, + existedOnDisk: existsSync(fileURLToPath(url)), + request, + }); + handlers.get('request')?.(request); + await vi.waitFor(() => expect(request.respond).toHaveBeenCalled()); + }, + evaluate: async (fn: { name: string }) => + fn.name === 'collectLayoutSamples' + ? samples({ viewportWidth: 1280, documentWidth: 1280 }) + : { nodes: 1, width: 1280, height: 800 }, + close: async () => {}, + }; + return { newPage: async () => page, close: async () => {} }; + }, }, - }, -})); + }; +}); const workspace = mkdtempSync(join(tmpdir(), 'codesign-harness-doc-')); @@ -74,12 +81,12 @@ function onlyNavigation() { describe('harness documents for sandboxed system Chrome (#455)', () => { it('serves the done verifier document from memory instead of a shared temp file', async () => { - const errors = await makeRuntimeVerifier()('
ok
', { + const result = await makeRuntimeVerifier()('
ok
', { path: 'index.html', }); const navigation = onlyNavigation(); - expect(errors).toEqual([]); + expect(result).toEqual({ errors: [], warnings: [] }); expect(navigation.url).toMatch(/^file:.*verify\.html$/); expect(navigation.existedOnDisk).toBe(false); expect(navigation.request.continue).not.toHaveBeenCalled(); diff --git a/apps/desktop/src/main/ipc/generate.ts b/apps/desktop/src/main/ipc/generate.ts index dfc71f566..484569733 100644 --- a/apps/desktop/src/main/ipc/generate.ts +++ b/apps/desktop/src/main/ipc/generate.ts @@ -1093,8 +1093,10 @@ export function registerGenerateIpc({ db, getMainWindow }: RegisterGenerateIpcDe ) { throw new CodesignError('done:verify:v1 expects { artifact: string }', 'IPC_BAD_INPUT'); } - const errors = await sharedRuntimeVerifier((raw as { artifact: string }).artifact); - return { errors }; + const { errors, warnings } = await sharedRuntimeVerifier( + (raw as { artifact: string }).artifact, + ); + return { schemaVersion: 1, errors, warnings }; }); ipcMain.handle('codesign:v1:generate', async (_e, raw: unknown) => { diff --git a/apps/desktop/src/main/ipc/generate.workspace-rename.test.ts b/apps/desktop/src/main/ipc/generate.workspace-rename.test.ts index b9b8cc21b..3d4000a9e 100644 --- a/apps/desktop/src/main/ipc/generate.workspace-rename.test.ts +++ b/apps/desktop/src/main/ipc/generate.workspace-rename.test.ts @@ -184,7 +184,7 @@ vi.mock('../memory-ipc', () => ({ })); vi.mock('../done-verify', () => ({ - makeRuntimeVerifier: vi.fn(() => async () => []), + makeRuntimeVerifier: vi.fn(() => async () => ({ errors: [], warnings: [] })), })); vi.mock('../preview-runtime', () => ({ @@ -420,6 +420,26 @@ describe('generate IPC workspace rename coordination', () => { expect(vi.mocked(generateViaAgent)).toHaveBeenCalledOnce(); }); + it('returns verifier errors and warnings from done:verify:v1 with a schema version', async () => { + const error = { message: 'ReferenceError: x is not defined', source: 'pageerror' }; + const warning = { + message: 'The page is 132 px wider than a 390 px viewport', + source: 'layout', + }; + vi.mocked(makeRuntimeVerifier).mockReturnValueOnce(async () => ({ + errors: [error], + warnings: [warning], + })); + registerGenerateIpc({ db: initTestDb(), getMainWindow: () => null }); + + await expect( + getHandler('done:verify:v1')(null, { artifact: 'function App(){return null;}' }), + ).resolves.toEqual({ schemaVersion: 1, errors: [error], warnings: [warning] }); + await expect(getHandler('done:verify:v1')(null, { artifact: 42 })).rejects.toThrow( + 'done:verify:v1 expects { artifact: string }', + ); + }); + it('persists preparation failures and validates recovery cursors', async () => { vi.mocked(preparePromptContext).mockRejectedValueOnce(new Error('Reference unavailable')); const db = initTestDb(); diff --git a/apps/desktop/src/main/ipc/tool-log.test.ts b/apps/desktop/src/main/ipc/tool-log.test.ts index 1d027ea39..4f9610ebe 100644 --- a/apps/desktop/src/main/ipc/tool-log.test.ts +++ b/apps/desktop/src/main/ipc/tool-log.test.ts @@ -84,7 +84,7 @@ describe('done diagnostic durability', () => { it('keeps warnings visible without changing successful completion to failure', () => { const result = { - content: [{ type: 'text', text: 'ok\nNon-blocking design metadata warnings' }], + content: [{ type: 'text', text: 'ok\nNon-blocking warnings' }], details: { status: 'ok', path: 'App.jsx', diff --git a/apps/desktop/src/main/layout-analysis.test.ts b/apps/desktop/src/main/layout-analysis.test.ts new file mode 100644 index 000000000..fa23624e1 --- /dev/null +++ b/apps/desktop/src/main/layout-analysis.test.ts @@ -0,0 +1,393 @@ +import type { LayoutIssue } from '@open-codesign/core'; +import { describe, expect, it } from 'vitest'; +import { + analyzeLayoutSamples, + compositeOver, + contrastRatio, + measureTextContrast, + relativeLuminance, + selectIssues, + toHex, +} from './layout-analysis'; +import { OVERFLOW_TOLERANCE_PX, type Rgba, type TextGroup } from './layout-samples'; +import { samples } from './layout-test-fixtures'; + +const WHITE: Rgba = [255, 255, 255, 1]; +const group = (overrides: Partial = {}): TextGroup => ({ + selector: 'p.note', + count: 1, + foreground: [119, 119, 119, 1], + layers: [], + large: false, + ...overrides, +}); + +describe('colour maths', () => { + it('computes the relative luminance of black, white and a grey', () => { + expect(relativeLuminance([0, 0, 0])).toBe(0); + expect(relativeLuminance([255, 255, 255])).toBeCloseTo(1, 10); + expect(relativeLuminance([119, 119, 119])).toBeCloseTo(0.1845, 4); + }); + + it('computes WCAG contrast ratios the way the standard lists them', () => { + expect(contrastRatio([0, 0, 0], [255, 255, 255])).toBeCloseTo(21, 10); + expect(contrastRatio([255, 255, 255], [0, 0, 0])).toBeCloseTo(21, 10); + expect(contrastRatio([119, 119, 119], [255, 255, 255])).toBeCloseTo(4.48, 2); + expect(contrastRatio([118, 118, 118], [255, 255, 255])).toBeCloseTo(4.54, 2); + expect(contrastRatio([10, 20, 30], [10, 20, 30])).toBe(1); + }); + + it('lays a translucent colour over an opaque one', () => { + expect(compositeOver([0, 0, 0, 0.5], [255, 255, 255])).toEqual([127.5, 127.5, 127.5]); + expect(compositeOver([10, 20, 30, 1], [200, 200, 200])).toEqual([10, 20, 30]); + expect(compositeOver([10, 20, 30, 0], [200, 200, 200])).toEqual([200, 200, 200]); + }); + + it('writes colours as hex, rounding and keeping them in range', () => { + expect(toHex([127.5, 0, 255])).toBe('#8000ff'); + expect(toHex([-4, 300, 1.2])).toBe('#00ff01'); + }); +}); + +describe('measureTextContrast', () => { + it('measures text on white when no background is set', () => { + const { ratio, text, backdrop } = measureTextContrast({ + foreground: [119, 119, 119, 1], + layers: [], + }); + + expect(backdrop).toEqual([255, 255, 255]); + expect(text).toEqual([119, 119, 119]); + expect(ratio).toBeCloseTo(4.48, 2); + }); + + it('lays the backgrounds over white from the outermost to the innermost', () => { + // The innermost is half-transparent black over an opaque blue. + const { backdrop } = measureTextContrast({ + foreground: WHITE, + layers: [ + [0, 0, 0, 0.5], + [0, 0, 255, 1], + ], + }); + + expect(backdrop).toEqual([0, 0, 127.5]); + }); + + it('lays translucent text over the backdrop it sits on', () => { + const { text, backdrop } = measureTextContrast({ + foreground: [0, 0, 0, 0.5], + layers: [[255, 255, 255, 1]], + }); + + expect(backdrop).toEqual([255, 255, 255]); + expect(text).toEqual([127.5, 127.5, 127.5]); + }); +}); + +describe('analyzeLayoutSamples: overflow and clipped text', () => { + it('reports a page that fits, or is off by less than the tolerance, as clean', () => { + expect(analyzeLayoutSamples(samples())).toEqual([]); + expect(analyzeLayoutSamples(samples({ documentWidth: 390 + OVERFLOW_TOLERANCE_PX }))).toEqual( + [], + ); + }); + + it('names each element that makes the page scroll sideways, with the numbers', () => { + const issues = analyzeLayoutSamples( + samples({ + documentWidth: 1412, + overflowing: [ + { selector: 'table#prices', right: 1412 }, + { selector: 'main > div.hero', right: 960 }, + ], + }), + ); + + expect(issues).toEqual([ + { + check: 'horizontal-overflow', + viewportWidth: 390, + selector: 'table#prices', + message: + 'table#prices reaches 1,412 px, past the 390 px viewport edge, so the page scrolls sideways.', + measured: 1412, + limit: 390, + }, + { + check: 'horizontal-overflow', + viewportWidth: 390, + selector: 'main > div.hero', + message: + 'main > div.hero reaches 960 px, past the 390 px viewport edge, so the page scrolls sideways.', + measured: 960, + limit: 390, + }, + ]); + }); + + it('still reports the overflow when no single element was identified', () => { + expect(analyzeLayoutSamples(samples({ documentWidth: 520 }))).toEqual([ + { + check: 'horizontal-overflow', + viewportWidth: 390, + selector: 'html', + message: + 'The page is 520 px wide at a 390 px viewport, so it scrolls sideways; no single element was identified.', + measured: 520, + limit: 390, + }, + ]); + }); + + it('does not report overflow the page hides, because it cannot scroll sideways', () => { + expect( + analyzeLayoutSamples( + samples({ + documentWidth: 900, + rootClipsX: true, + overflowing: [{ selector: 'div.wide', right: 900 }], + }), + ), + ).toEqual([]); + }); + + it('reports text a box clips on the side that sticks out', () => { + const issues = analyzeLayoutSamples( + samples({ + viewportWidth: 1280, + documentWidth: 1280, + clipped: [ + { selector: 'div#card', side: 'right', textEdge: 340, boxEdge: 300 }, + { selector: 'p.teaser', side: 'bottom', textEdge: 190, boxEdge: 120 }, + { selector: 'span.badge', side: 'left', textEdge: 20, boxEdge: 40 }, + { selector: 'h2.title', side: 'top', textEdge: 8, boxEdge: 30 }, + ], + }), + ); + + expect(issues).toEqual([ + { + check: 'clipped-text', + viewportWidth: 1280, + selector: 'div#card', + message: + 'div#card hides part of its text: the text reaches 340 px but the box ends at 300 px (viewport 1,280 px).', + measured: 340, + limit: 300, + }, + { + check: 'clipped-text', + viewportWidth: 1280, + selector: 'p.teaser', + message: + 'p.teaser hides part of its text: the text reaches 190 px from the top but the box ends at 120 px (viewport 1,280 px).', + measured: 190, + limit: 120, + }, + { + check: 'clipped-text', + viewportWidth: 1280, + selector: 'span.badge', + message: + 'span.badge hides part of its text: the text starts at 20 px but the box starts at 40 px (viewport 1,280 px).', + measured: 20, + limit: 40, + }, + { + check: 'clipped-text', + viewportWidth: 1280, + selector: 'h2.title', + message: + 'h2.title hides part of its text: the text starts at 8 px from the top but the box starts at 30 px (viewport 1,280 px).', + measured: 8, + limit: 30, + }, + ]); + }); +}); + +describe('analyzeLayoutSamples: contrast', () => { + const contrastOf = (...groups: TextGroup[]) => + analyzeLayoutSamples(samples({ textGroups: groups })).filter( + (issue) => issue.check === 'low-contrast', + ); + + it('reports text just under 4.5:1 and names both colours', () => { + const [issue] = contrastOf(group()); + + expect(issue).toEqual({ + check: 'low-contrast', + viewportWidth: 390, + selector: 'p.note', + message: 'p.note has low text contrast: 4.47:1 for #777777 on #ffffff, but text needs 4.5:1.', + measured: 4.47, + limit: 4.5, + }); + }); + + it('never rounds a failing ratio up to the threshold', () => { + // #777 is 4.478:1; rounded to two decimals it would read 4.48, still short of 4.5. + const [issue] = contrastOf(group()); + + expect(issue?.measured).toBe(4.47); + expect(issue?.measured).toBeLessThan(4.5); + }); + + it('accepts text at or above 4.5:1', () => { + expect(contrastOf(group({ foreground: [118, 118, 118, 1] }))).toEqual([]); + expect(contrastOf(group({ foreground: [0, 0, 0, 1] }))).toEqual([]); + }); + + it('asks only 3:1 of large text', () => { + // #949494 is 3.03:1 on white: enough for large text, not for body text. + const grey: Rgba = [148, 148, 148, 1]; + + expect(contrastOf(group({ foreground: grey, large: true }))).toEqual([]); + const [issue] = contrastOf(group({ foreground: grey })); + expect(issue?.limit).toBe(4.5); + const [largeIssue] = contrastOf(group({ foreground: [170, 170, 170, 1], large: true })); + expect(largeIssue?.limit).toBe(3); + expect(largeIssue?.message).toContain('large text needs 3:1'); + }); + + it('measures against the backgrounds laid over white', () => { + const [issue] = contrastOf( + group({ foreground: WHITE, layers: [[0, 0, 0, 0.5]], selector: 'h2.badge' }), + ); + + expect(issue?.message).toBe( + 'h2.badge has low text contrast: 3.97:1 for #ffffff on #808080, but text needs 4.5:1.', + ); + }); + + it('says how many text elements share a failing pair', () => { + const [issue] = contrastOf(group({ count: 7 })); + + expect(issue?.message).toMatch(/ 7 text elements use this pair\.$/); + expect(contrastOf(group({ count: 1 }))[0]?.message).not.toContain('use this pair'); + }); + + it('lists the worst pair first and every failing pair', () => { + const issues = contrastOf( + group({ selector: 'a', foreground: [150, 150, 150, 1] }), + group({ selector: 'b', foreground: [200, 200, 200, 1] }), + group({ selector: 'c', foreground: [119, 119, 119, 1] }), + ); + + expect(issues.map((issue) => issue.selector)).toEqual(['b', 'a', 'c']); + }); +}); + +describe('analyzeLayoutSamples: small targets and names', () => { + it('reports each small target with its size', () => { + const issues = analyzeLayoutSamples( + samples({ smallTargets: [{ selector: 'button.close', width: 16, height: 20 }] }), + ); + + expect(issues).toEqual([ + { + check: 'small-target', + viewportWidth: 390, + selector: 'button.close', + message: + 'button.close is 16 × 20 px; a tap target should be at least 24 × 24 px, or have that much room around it.', + measured: 16, + limit: 24, + }, + ]); + }); + + it('reports each control with no name, without numbers', () => { + const issues = analyzeLayoutSamples( + samples({ + unnamedControls: [ + { selector: 'button.menu', kind: 'button' }, + { selector: 'div.toggle', kind: 'div role=switch' }, + ], + }), + ); + + expect(issues).toEqual([ + { + check: 'missing-name', + viewportWidth: 390, + selector: 'button.menu', + message: + 'button.menu (button) has no accessible name: it has no text, aria-label, aria-labelledby, label or title.', + }, + { + check: 'missing-name', + viewportWidth: 390, + selector: 'div.toggle', + message: + 'div.toggle (div role=switch) has no accessible name: it has no text, aria-label, aria-labelledby, label or title.', + }, + ]); + expect(issues[0]).not.toHaveProperty('measured'); + }); + + it('lists the checks in one fixed order', () => { + const issues = analyzeLayoutSamples( + samples({ + documentWidth: 600, + overflowing: [{ selector: 'div.wide', right: 600 }], + clipped: [{ selector: 'div#card', side: 'right', textEdge: 340, boxEdge: 300 }], + textGroups: [group()], + smallTargets: [{ selector: 'button.close', width: 16, height: 16 }], + unnamedControls: [{ selector: 'button.menu', kind: 'button' }], + }), + ); + + expect(issues.map((issue) => issue.check)).toEqual([ + 'horizontal-overflow', + 'clipped-text', + 'low-contrast', + 'small-target', + 'missing-name', + ]); + }); +}); + +describe('selectIssues', () => { + const issue = (check: LayoutIssue['check'], index: number): LayoutIssue => ({ + check, + viewportWidth: 390, + selector: `${check}-${index}`, + message: `${check} ${index}`, + }); + + it('keeps everything when it fits the budget', () => { + const issues = [issue('low-contrast', 1), issue('missing-name', 1)]; + + expect(selectIssues(issues, 5)).toBe(issues); + expect(selectIssues(issues, 2)).toBe(issues); + }); + + it('takes one from each check in turn and keeps the original order', () => { + const issues = [ + ...[1, 2, 3, 4, 5].map((index) => issue('horizontal-overflow', index)), + ...[1, 2, 3, 4, 5].map((index) => issue('low-contrast', index)), + issue('missing-name', 1), + ]; + + expect(selectIssues(issues, 4).map((item) => item.selector)).toEqual([ + 'horizontal-overflow-1', + 'horizontal-overflow-2', + 'low-contrast-1', + 'missing-name-1', + ]); + }); + + it('still shows a rare check when one check has far more findings than the budget', () => { + const issues = [ + ...Array.from({ length: 40 }, (_, index) => issue('small-target', index)), + issue('low-contrast', 1), + ]; + + const kept = selectIssues(issues, 20); + + expect(kept).toHaveLength(20); + expect(kept.some((item) => item.check === 'low-contrast')).toBe(true); + }); +}); diff --git a/apps/desktop/src/main/layout-analysis.ts b/apps/desktop/src/main/layout-analysis.ts new file mode 100644 index 000000000..6c6346715 --- /dev/null +++ b/apps/desktop/src/main/layout-analysis.ts @@ -0,0 +1,210 @@ +/** + * The decisions behind the layout checks, made on the plain data the page-side + * function collects, so every threshold is testable without a browser. + */ + +import type { LayoutIssue } from '@open-codesign/core'; +import { + type ClippedText, + LAYOUT_PROBE_LIMITS, + type LayoutSamples, + MIN_CONTRAST_RATIO, + MIN_LARGE_TEXT_CONTRAST_RATIO, + MIN_TARGET_SIZE_PX, + type Rgba, + type TextGroup, +} from './layout-samples'; + +const formatPx = (value: number) => `${value.toLocaleString('en-US')} px`; + +type Rgb = [number, number, number]; + +/** A colour with some transparency, laid over an opaque one. */ +export function compositeOver(top: Rgba, base: Rgb): Rgb { + const [red, green, blue, alpha] = top; + return [ + red * alpha + base[0] * (1 - alpha), + green * alpha + base[1] * (1 - alpha), + blue * alpha + base[2] * (1 - alpha), + ]; +} + +/** WCAG relative luminance of an sRGB colour. */ +export function relativeLuminance([red, green, blue]: Rgb): number { + const linear = (channel: number) => { + const value = channel / 255; + return value <= 0.04045 ? value / 12.92 : ((value + 0.055) / 1.055) ** 2.4; + }; + return 0.2126 * linear(red) + 0.7152 * linear(green) + 0.0722 * linear(blue); +} + +/** WCAG contrast ratio, from 1 to 21. */ +export function contrastRatio(first: Rgb, second: Rgb): number { + const a = relativeLuminance(first); + const b = relativeLuminance(second); + return (Math.max(a, b) + 0.05) / (Math.min(a, b) + 0.05); +} + +export function toHex([red, green, blue]: Rgb): string { + const part = (channel: number) => + Math.round(Math.min(255, Math.max(0, channel))) + .toString(16) + .padStart(2, '0'); + return `#${part(red)}${part(green)}${part(blue)}`; +} + +/** + * The backgrounds behind the text are laid over white, the colour of a page that + * sets none, from the outermost to the innermost; then the text over the result. + */ +export function measureTextContrast(group: Pick): { + ratio: number; + text: Rgb; + backdrop: Rgb; +} { + let backdrop: Rgb = [255, 255, 255]; + for (let index = group.layers.length - 1; index >= 0; index -= 1) { + const layer = group.layers[index]; + if (layer) backdrop = compositeOver(layer, backdrop); + } + const text = compositeOver(group.foreground, backdrop); + return { ratio: contrastRatio(text, backdrop), text, backdrop }; +} + +function overflowIssues(samples: LayoutSamples, tolerance: number): LayoutIssue[] { + const { viewportWidth, documentWidth } = samples; + if (samples.rootClipsX || documentWidth - viewportWidth <= tolerance) return []; + if (samples.overflowing.length === 0) { + return [ + { + check: 'horizontal-overflow', + viewportWidth, + selector: 'html', + message: `The page is ${formatPx(documentWidth)} wide at a ${formatPx(viewportWidth)} viewport, so it scrolls sideways; no single element was identified.`, + measured: documentWidth, + limit: viewportWidth, + }, + ]; + } + return samples.overflowing.map((element) => ({ + check: 'horizontal-overflow', + viewportWidth, + selector: element.selector, + message: `${element.selector} reaches ${formatPx(element.right)}, past the ${formatPx(viewportWidth)} viewport edge, so the page scrolls sideways.`, + measured: element.right, + limit: viewportWidth, + })); +} + +function clippedEdges({ side, textEdge, boxEdge }: ClippedText): string { + const text = formatPx(textEdge); + const box = formatPx(boxEdge); + switch (side) { + case 'right': + return `the text reaches ${text} but the box ends at ${box}`; + case 'bottom': + return `the text reaches ${text} from the top but the box ends at ${box}`; + case 'left': + return `the text starts at ${text} but the box starts at ${box}`; + case 'top': + return `the text starts at ${text} from the top but the box starts at ${box}`; + } +} + +function clippedIssues(samples: LayoutSamples): LayoutIssue[] { + const { viewportWidth } = samples; + return samples.clipped.map((text) => ({ + check: 'clipped-text', + viewportWidth, + selector: text.selector, + message: `${text.selector} hides part of its text: ${clippedEdges(text)} (viewport ${formatPx(viewportWidth)}).`, + measured: text.textEdge, + limit: text.boxEdge, + })); +} + +function contrastIssues(samples: LayoutSamples): LayoutIssue[] { + const failing = samples.textGroups + .map((group) => { + const { ratio, text, backdrop } = measureTextContrast(group); + return { group, ratio, text, backdrop }; + }) + .filter( + ({ group, ratio }) => + ratio < (group.large ? MIN_LARGE_TEXT_CONTRAST_RATIO : MIN_CONTRAST_RATIO), + ) + .sort((a, b) => a.ratio - b.ratio); + + return failing.map(({ group, ratio, text, backdrop }) => { + const needed = group.large ? MIN_LARGE_TEXT_CONTRAST_RATIO : MIN_CONTRAST_RATIO; + // WCAG does not round up: 4.499 does not pass as 4.5. + const shown = Math.floor(ratio * 100) / 100; + const shared = group.count > 1 ? ` ${group.count} text elements use this pair.` : ''; + return { + check: 'low-contrast', + viewportWidth: samples.viewportWidth, + selector: group.selector, + message: `${group.selector} has low text contrast: ${shown}:1 for ${toHex(text)} on ${toHex(backdrop)}, but ${group.large ? 'large text' : 'text'} needs ${needed}:1.${shared}`, + measured: shown, + limit: needed, + }; + }); +} + +function targetIssues(samples: LayoutSamples): LayoutIssue[] { + return samples.smallTargets.map((target) => ({ + check: 'small-target', + viewportWidth: samples.viewportWidth, + selector: target.selector, + message: `${target.selector} is ${target.width} × ${target.height} px; a tap target should be at least ${MIN_TARGET_SIZE_PX} × ${MIN_TARGET_SIZE_PX} px, or have that much room around it.`, + measured: Math.min(target.width, target.height), + limit: MIN_TARGET_SIZE_PX, + })); +} + +function nameIssues(samples: LayoutSamples): LayoutIssue[] { + return samples.unnamedControls.map((control) => ({ + check: 'missing-name', + viewportWidth: samples.viewportWidth, + selector: control.selector, + message: `${control.selector} (${control.kind}) has no accessible name: it has no text, aria-label, aria-labelledby, label or title.`, + })); +} + +/** Turns one measurement into findings, in the order the checks are listed in the docs. */ +export function analyzeLayoutSamples( + samples: LayoutSamples, + limits: Pick = LAYOUT_PROBE_LIMITS, +): LayoutIssue[] { + return [ + ...overflowIssues(samples, limits.overflowTolerance), + ...clippedIssues(samples), + ...contrastIssues(samples), + ...targetIssues(samples), + ...nameIssues(samples), + ]; +} + +/** + * Keeps at most `max` findings. When there are more, it takes one from each + * check in turn, so a page with fifty tiny buttons still shows its contrast + * problem, and keeps the original order. + */ +export function selectIssues(issues: LayoutIssue[], max: number): LayoutIssue[] { + if (issues.length <= max) return issues; + const queues = new Map(); + for (const issue of issues) { + const queue = queues.get(issue.check); + if (queue) queue.push(issue); + else queues.set(issue.check, [issue]); + } + const picked = new Set(); + while (picked.size < max) { + for (const queue of queues.values()) { + const next = queue.shift(); + if (next) picked.add(next); + if (picked.size === max) break; + } + } + return issues.filter((issue) => picked.has(issue)); +} diff --git a/apps/desktop/src/main/layout-browser-kit.ts b/apps/desktop/src/main/layout-browser-kit.ts new file mode 100644 index 000000000..eff3adafa --- /dev/null +++ b/apps/desktop/src/main/layout-browser-kit.ts @@ -0,0 +1,63 @@ +import { randomUUID } from 'node:crypto'; +import { mkdirSync, rmSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { findSystemChrome } from '@open-codesign/exporters'; +import puppeteer, { type Browser, type Page } from 'puppeteer-core'; +import { afterAll, afterEach, beforeAll, beforeEach } from 'vitest'; + +/** The system Chrome the browser tests drive, or null when there is none and they skip. */ +export const chrome = await findSystemChrome().catch(() => null); + +export const DESKTOP = { width: 1280, height: 800 } as const; + +/** + * One browser for the tests in a `describe`, and a fresh page at desktop size + * for each of them, with every request aborted so the page is only what the test sets. + */ +export function useChromePage(name: string): { + page: () => Page; + mount: (markup: string) => Promise; +} { + let browser: Browser; + let page: Page; + let profile: string; + + beforeAll(async () => { + if (!chrome) throw new Error('System Chrome unavailable'); + profile = resolve(`.${name}-${randomUUID()}`); + mkdirSync(profile); + browser = await puppeteer.launch({ + executablePath: chrome, + headless: true, + userDataDir: profile, + args: ['--disable-background-networking'], + }); + }, 30_000); + + afterAll(async () => { + try { + await browser?.close(); + } finally { + if (profile) rmSync(profile, { recursive: true, force: true }); + } + }, 30_000); + + beforeEach(async () => { + page = await browser.newPage(); + await page.setViewport(DESKTOP); + await page.setRequestInterception(true); + page.on('request', (request) => void request.abort()); + }); + + afterEach(async () => { + await page?.close(); + }); + + return { + page: () => page, + mount: (markup) => + page.setContent( + `${markup}`, + ), + }; +} diff --git a/apps/desktop/src/main/layout-probe.accessibility.browser.test.ts b/apps/desktop/src/main/layout-probe.accessibility.browser.test.ts new file mode 100644 index 000000000..21b4b5206 --- /dev/null +++ b/apps/desktop/src/main/layout-probe.accessibility.browser.test.ts @@ -0,0 +1,232 @@ +import { describe, expect, it } from 'vitest'; +import { chrome, DESKTOP, useChromePage } from './layout-browser-kit'; +import { inspectLayout } from './layout-probe'; +import { PHONE_VIEWPORT } from './layout-samples'; + +describe.skipIf(!chrome)('tap targets and control names in system Chrome', () => { + const { page, mount } = useChromePage('layout-accessibility'); + + const inspect = async (markup: string) => { + await mount(markup); + return inspectLayout(page()); + }; + const targetsIn = async (markup: string) => + (await inspect(markup)).issues.filter((issue) => issue.check === 'small-target'); + const namesIn = async (markup: string) => + (await inspect(markup)).issues.filter((issue) => issue.check === 'missing-name'); + + const square = (size: number, extra = '') => + `width:${size}px;height:${size}px;padding:0;border:0;${extra}`; + + describe('tap targets', () => { + // A small target is only a problem when another target is within 24 px of it (WCAG 2.2 SC 2.5.8). + const neighbour = ``; + // Wide enough to sit under a link in a sentence. + const bar = ``; + + it('reports a small button next to another target at phone width, with its size', async () => { + const issues = await targetsIn( + `
${neighbour}
`, + ); + + expect(issues).toEqual([ + { + check: 'small-target', + viewportWidth: PHONE_VIEWPORT.width, + selector: 'button#close', + message: + 'button#close is 16 × 16 px; a tap target should be at least 24 × 24 px, or have that much room around it.', + measured: 16, + limit: 24, + }, + ]); + }); + + it('accepts a target of 24 px, and reports one that is short in only one direction', async () => { + expect( + await targetsIn( + `
${neighbour}
`, + ), + ).toEqual([]); + const issues = await targetsIn( + `
${neighbour}
`, + ); + expect(issues.map((issue) => [issue.selector, issue.measured])).toEqual([ + ['button#wide', 20], + ]); + }); + + it('passes a lone small target, because nothing else is within 24 px of it', async () => { + expect(await targetsIn(``)).toEqual( + [], + ); + }); + + it('does not ask a link inside a sentence to be 24 px', async () => { + expect( + await targetsIn( + `

Read the terms before you start.

${bar}`, + ), + ).toEqual([]); + const issues = await targetsIn( + `
Chip${neighbour}
`, + ); + expect(issues.map((issue) => issue.selector)).toEqual(['a#chip']); + }); + + it('lets a small target pass when no other target is within the 24 px square around it', async () => { + const spaced = `
+ +
`; + const touching = `
+ +
`; + + expect(await targetsIn(spaced)).toEqual([]); + expect((await targetsIn(touching)).map((issue) => issue.selector)).toEqual([ + 'button#a', + 'button#b', + ]); + }); + + it.each([ + ['disabled', ``], + ['not rendered', ``], + ['invisible', ``], + ['transparent', ``], + [ + 'hidden from assistive technology', + ``, + ], + [ + 'a visually hidden input whose label is the target', + '', + ], + ])('skips a control that is %s, even with a target right next to it', async (_label, markup) => { + expect(await targetsIn(`
${markup}${neighbour}
`)).toEqual([]); + }); + + it('names the smallest first, at most five, and says how many more there were', async () => { + const buttons = [17, 12, 15, 10, 16, 14, 13, 11] + .map( + (size) => ``, + ) + .join(''); + + const { issues, notes } = await inspect(`
${buttons}
`); + + expect( + issues.filter((issue) => issue.check === 'small-target').map((issue) => issue.measured), + ).toEqual([10, 11, 12, 13, 14]); + expect(notes).toEqual(['3 more findings past the per-check limits were not listed.']); + }); + + it('says so when the page has more controls than it looks at', async () => { + const buttons = Array.from( + { length: 401 }, + () => ``, + ); + + const { notes } = await inspect(buttons.join('')); + + expect(notes).toEqual([ + 'Layout checks did not cover the whole page (more than 5,000 elements or 400 controls); the rest was not measured.', + ]); + }); + + it('measures at phone width only, so a small target is reported once', async () => { + const { issues } = await inspect( + `
${neighbour}
`, + ); + + expect(issues.map((issue) => issue.viewportWidth)).toEqual([PHONE_VIEWPORT.width]); + }); + }); + + describe('control names', () => { + it('reports an icon button that has no text, with the tag in the message', async () => { + const issues = await namesIn( + '', + ); + + expect(issues).toEqual([ + { + check: 'missing-name', + viewportWidth: DESKTOP.width, + selector: 'button#menu', + message: + 'button#menu (button) has no accessible name: it has no text, aria-label, aria-labelledby, label or title.', + }, + ]); + }); + + it.each([ + ['text', ''], + ['aria-label', ''], + [ + 'aria-labelledby', + 'Search', + ], + ['title', ''], + ['a label for the input', ''], + ['a wrapping label', ''], + ['the alt of an image in a link', 'Home'], + [ + 'the title of an svg in a button', + '', + ], + ['the default of a submit input', ''], + ['the value of a button input', ''], + ['text in a role=button element', '
Open
'], + ])('accepts a control named by %s', async (_label, markup) => { + expect(await namesIn(markup)).toEqual([]); + }); + + it.each([ + ['an empty button', '', 'button'], + ['a link with an empty alt image', '', 'a'], + ['an input with only a placeholder', '', 'input'], + ['an empty link', '', 'a'], + ['a select with no label', '', 'select'], + [ + 'a role=switch element with no text', + '
', + 'div role=switch', + ], + ])('reports %s', async (_label, markup, kind) => { + const issues = await namesIn(markup); + + expect(issues).toHaveLength(1); + expect(issues[0]?.message).toContain(`(${kind})`); + }); + + it('skips controls that are hidden from the user and from assistive technology', async () => { + expect( + await namesIn( + ` + + + `, + ), + ).toEqual([]); + }); + + it('names at most five controls, and says how many more there were', async () => { + const { issues, notes } = await inspect( + Array.from( + { length: 8 }, + (_, index) => ``, + ).join(''), + ); + + expect(issues.filter((issue) => issue.check === 'missing-name')).toHaveLength(5); + expect(notes).toEqual(['3 more findings past the per-check limits were not listed.']); + }); + + it('checks at the current width only, so a finding is not repeated at phone width', async () => { + const { issues } = await inspect(''); + + expect(issues.map((issue) => issue.viewportWidth)).toEqual([DESKTOP.width]); + }); + }); +}); diff --git a/apps/desktop/src/main/layout-probe.browser.test.ts b/apps/desktop/src/main/layout-probe.browser.test.ts new file mode 100644 index 000000000..3e533b48a --- /dev/null +++ b/apps/desktop/src/main/layout-probe.browser.test.ts @@ -0,0 +1,231 @@ +import { describe, expect, it } from 'vitest'; +import { chrome, DESKTOP, useChromePage } from './layout-browser-kit'; +import { inspectLayout } from './layout-probe'; +import { MAX_PROBE_ELEMENTS, PHONE_VIEWPORT } from './layout-samples'; + +const LONG_TEXT = 'Long text that wraps over several lines and is cut off by its box. '.repeat(6); + +describe.skipIf(!chrome)('layout checks in system Chrome', () => { + const { page: pageOf, mount: mountMarkup } = useChromePage('layout-probe-browser'); + const mount = (body: string, head = '') => + mountMarkup(`${head ? `` : ''}${body}`); + + describe('horizontal overflow', () => { + it('names the element that scrolls the page sideways, at phone width only', async () => { + await mount( + '
PlanPrice
', + ); + + const { issues, notes } = await inspectLayout(pageOf()); + + expect(notes).toEqual([]); + expect(issues).toHaveLength(1); + expect(issues[0]).toMatchObject({ + check: 'horizontal-overflow', + viewportWidth: PHONE_VIEWPORT.width, + selector: 'table#prices', + measured: 800, + limit: PHONE_VIEWPORT.width, + }); + expect(issues[0]?.message).toContain('table#prices reaches 800 px'); + }); + + it('puts the viewport back after measuring', async () => { + await mount('
Hello
'); + + await inspectLayout(pageOf()); + + expect(pageOf().viewport()).toMatchObject(DESKTOP); + }); + + it('reports nothing for a page that reflows to fit', async () => { + await mount( + '
One
Two
Three
', + '.row{display:flex;flex-wrap:wrap}.card{width:200px;height:80px}', + ); + + expect((await inspectLayout(pageOf())).issues).toEqual([]); + }); + + it('does not blame content inside a container that scrolls on its own', async () => { + await mount( + '
Wide strip
', + ); + + expect((await inspectLayout(pageOf())).issues).toEqual([]); + }); + + it('names the element that really overflows, not a wider one inside a scroller', async () => { + await mount( + `
Wide strip
+ `, + ); + + const { issues } = await inspectLayout(pageOf()); + + expect(issues.map((issue) => issue.selector)).toEqual(['div#banner']); + }); + + it('does not report overflow the page hides from the user', async () => { + await mount('
Wide
', 'body{overflow-x:hidden}'); + + expect((await inspectLayout(pageOf())).issues).toEqual([]); + }); + + it('ignores a fixed-position element, which cannot scroll the page', async () => { + await mount( + `
+ `, + ); + + const { issues } = await inspectLayout(pageOf()); + + expect(issues.map((issue) => issue.selector)).toEqual(['div#banner']); + }); + + it('names at most three elements and keeps the widest first', async () => { + const boxes = [500, 900, 600, 700, 800] + .map((width) => `
`) + .join(''); + await mount(`
${boxes}
`); + + const { issues } = await inspectLayout(pageOf()); + + expect(issues.map((issue) => issue.measured)).toEqual([900, 800, 700]); + }); + }); + + describe('clipped text', () => { + const clippedAt = async (markup: string, head = '') => { + await mount(markup, head); + return (await inspectLayout(pageOf())).issues.filter( + (issue) => issue.check === 'clipped-text', + ); + }; + + it('reports text that a fixed-height box cuts off, with a path to the element', async () => { + const issues = await clippedAt( + `
${LONG_TEXT}
`, + ); + + expect(issues.length).toBeGreaterThan(0); + expect(issues[0]).toMatchObject({ + check: 'clipped-text', + viewportWidth: DESKTOP.width, + selector: 'section > div.note.highlighted', + }); + expect(issues[0]?.measured).toBeGreaterThan(issues[0]?.limit ?? Number.POSITIVE_INFINITY); + }); + + it.each([ + [ + 'right', + `
${LONG_TEXT}
`, + 'the text reaches', + 1, + ], + [ + 'bottom', + `
${LONG_TEXT}
`, + 'from the top but the box ends at', + 1, + ], + [ + 'left', + '
Indented off the left edge
', + 'the text starts at', + -1, + ], + [ + 'left, when the text also sticks out of the right by less', + `
${'x'.repeat(50)}
`, + 'the text starts at', + -1, + ], + [ + 'top', + '
Pushed above the top edge
', + 'from the top but the box starts at', + -1, + ], + ])('names the %s side when text sticks out of it', async (_side, markup, wording, direction) => { + const [issue] = await clippedAt(markup); + + expect(issue?.message).toContain(wording); + expect(Math.sign((issue?.measured ?? 0) - (issue?.limit ?? 0))).toBe(direction); + }); + + it('ends the path at an element with an id and otherwise keeps three levels', async () => { + const issues = await clippedAt( + `

${LONG_TEXT}

`, + ); + + expect(issues[0]?.selector).toBe('p#deep'); + const unnamed = await clippedAt( + `

${LONG_TEXT}

`, + ); + expect(unnamed[0]?.selector).toBe('… > div > div > p'); + }); + + it('reports text that sticks out by more than 2 px and not by 2 px', async () => { + const indented = (px: number) => + `
Indented
`; + + expect(await clippedAt(indented(2))).toEqual([]); + expect(await clippedAt(indented(3))).toHaveLength(2); + }); + + it('leaves text alone that fits its box', async () => { + expect( + await clippedAt('
Short
'), + ).toEqual([]); + }); + + it.each([ + [ + 'an ellipsis', + `
${LONG_TEXT}
`, + ], + [ + 'a line clamp', + `

${LONG_TEXT}

`, + ], + ['a collapsed panel', '
Answer behind a toggle
'], + [ + 'a running animation', + `
${LONG_TEXT}
`, + ], + ])('treats %s as intended', async (_label, markup) => { + expect( + await clippedAt( + markup, + '@keyframes slide{from{transform:translateX(0)}to{transform:translateX(-100px)}}', + ), + ).toEqual([]); + }); + + it('names at most five clipped texts per viewport', async () => { + const boxes = Array.from( + { length: 8 }, + (_, index) => + `
${LONG_TEXT}
`, + ).join(''); + + const issues = await clippedAt(boxes); + + // Five at each of the two widths. + expect(issues).toHaveLength(10); + expect(issues.filter((issue) => issue.viewportWidth === DESKTOP.width)).toHaveLength(5); + }); + }); + + it('says so when the page has more elements than it measures', async () => { + await mount(Array.from({ length: MAX_PROBE_ELEMENTS + 100 }, () => '').join('')); + + const { notes } = await inspectLayout(pageOf()); + + expect(notes).toEqual([ + 'Layout checks did not cover the whole page (more than 5,000 elements or 400 controls); the rest was not measured.', + ]); + }); +}); diff --git a/apps/desktop/src/main/layout-probe.contrast.browser.test.ts b/apps/desktop/src/main/layout-probe.contrast.browser.test.ts new file mode 100644 index 000000000..e1463c38f --- /dev/null +++ b/apps/desktop/src/main/layout-probe.contrast.browser.test.ts @@ -0,0 +1,160 @@ +import { describe, expect, it } from 'vitest'; +import { chrome, DESKTOP, useChromePage } from './layout-browser-kit'; +import { inspectLayout } from './layout-probe'; + +describe.skipIf(!chrome)('contrast checks in system Chrome', () => { + const { page, mount } = useChromePage('layout-contrast'); + + const contrastOf = async (markup: string) => { + await mount(markup); + const { issues, notes } = await inspectLayout(page()); + return { issues: issues.filter((issue) => issue.check === 'low-contrast'), notes }; + }; + + describe('text and background', () => { + it('reports grey text on white that is just under 4.5:1, with both colours', async () => { + const { issues, notes } = await contrastOf('

Fine print

'); + + expect(notes).toEqual([]); + expect(issues).toEqual([ + { + check: 'low-contrast', + viewportWidth: DESKTOP.width, + selector: 'p#note', + message: + 'p#note has low text contrast: 4.47:1 for #777777 on #ffffff, but text needs 4.5:1.', + measured: 4.47, + limit: 4.5, + }, + ]); + }); + + it('accepts text at 4.5:1 and above, and the browser default of black on white', async () => { + expect( + (await contrastOf('

Readable

Default colours

')).issues, + ).toEqual([]); + }); + + it('asks only 3:1 of text that is 24 px, or 18.66 px and bold', async () => { + const grey = 'color:#949494'; + + expect((await contrastOf(`

Large

`)).issues).toEqual( + [], + ); + expect( + (await contrastOf(`

Bold

`)).issues, + ).toEqual([]); + expect( + (await contrastOf(`

Not bold

`)).issues, + ).toHaveLength(1); + expect((await contrastOf(`

Small

`)).issues).toHaveLength(1); + }); + + it('takes the background from the nearest ancestor that sets one', async () => { + const { issues } = await contrastOf( + '

Dim on dark

', + ); + + expect(issues).toHaveLength(1); + expect(issues[0]?.message).toContain('p#dim has low text contrast'); + expect(issues[0]?.message).toContain('#444444 on #222222'); + }); + + it('lays a translucent background over what is behind it', async () => { + const { issues } = await contrastOf( + '

New

', + ); + + expect(issues[0]?.message).toBe( + 'p#badge has low text contrast: 3.97:1 for #ffffff on #808080, but text needs 4.5:1.', + ); + }); + + it('lays translucent text over its background', async () => { + const { issues } = await contrastOf( + '

Half-transparent text

', + ); + + expect(issues[0]?.message).toContain('#808080 on #ffffff'); + }); + + it('reads colours in spaces other than sRGB', async () => { + const { issues } = await contrastOf( + `

oklch

+

color-mix

`, + ); + + expect(issues.map((issue) => issue.selector).sort()).toEqual(['p#a', 'p#b']); + expect(issues.find((issue) => issue.selector === 'p#b')?.message).toContain('#999999'); + }); + }); + + describe('what is reported once, and what is not measured', () => { + it('reports one finding for text elements that share a colour pair, and says how many', async () => { + const { issues } = await contrastOf( + Array.from( + { length: 5 }, + (_, index) => `

Hint

`, + ).join(''), + ); + + expect(issues).toHaveLength(1); + expect(issues[0]?.message).toMatch(/5 text elements use this pair\.$/); + }); + + it('lists the worst pair first', async () => { + const { issues } = await contrastOf( + `

Mild

Bad

`, + ); + + expect(issues.map((issue) => issue.selector)).toEqual(['p#bad', 'p#mild']); + }); + + it('does not guess on an image or gradient, and counts what it skipped', async () => { + const { issues, notes } = await contrastOf( + '

Over a gradient

', + ); + + expect(issues).toEqual([]); + expect(notes).toEqual([ + 'Contrast was not checked for 1 text element on a background image or gradient, or partly transparent.', + ]); + }); + + it('does not measure partly transparent elements', async () => { + const { issues, notes } = await contrastOf( + '

Faded

Faded too

', + ); + + expect(issues).toEqual([]); + expect(notes).toEqual([ + 'Contrast was not checked for 2 text elements on a background image or gradient, or partly transparent.', + ]); + }); + + it('ignores text nobody can see', async () => { + const { issues, notes } = await contrastOf( + `

Hidden

+

Not rendered

+

`, + ); + + expect(issues).toEqual([]); + expect(notes).toEqual([]); + }); + + it('measures the element that holds the text, not a container whose children have their own', async () => { + const { issues } = await contrastOf( + '
Black text
', + ); + + expect(issues).toEqual([]); + }); + + it('measures only at the current width, so a finding is not repeated at phone width', async () => { + const { issues } = await contrastOf('

Fine print

'); + + expect(issues.map((issue) => issue.viewportWidth)).toEqual([DESKTOP.width]); + }); + }); +}); diff --git a/apps/desktop/src/main/layout-probe.lifecycle.test.ts b/apps/desktop/src/main/layout-probe.lifecycle.test.ts new file mode 100644 index 000000000..5c405f1e1 --- /dev/null +++ b/apps/desktop/src/main/layout-probe.lifecycle.test.ts @@ -0,0 +1,102 @@ +import type { Page } from 'puppeteer-core'; +import { afterEach, expect, it, vi } from 'vitest'; +import { collectLayoutSamples, inspectLayout } from './layout-probe'; +import { LAYOUT_BUDGET_MS } from './layout-samples'; +import { samples } from './layout-test-fixtures'; + +afterEach(() => vi.useRealTimers()); + +it('restores the preview viewport before returning a layout timeout', async () => { + vi.useFakeTimers(); + const desktop = { width: 1280, height: 800 }; + let viewport = desktop; + let measurements = 0; + let finishPhone!: () => void; + const page = { + viewport: () => viewport, + setViewport: async (value: typeof desktop) => { + viewport = value; + }, + evaluate: async (fn: unknown) => { + if (fn !== collectLayoutSamples) return undefined; + measurements += 1; + if (measurements === 2) + await new Promise((resolve) => { + finishPhone = resolve; + }); + return samples({ viewportWidth: viewport.width, documentWidth: viewport.width }); + }, + } as unknown as Page; + const outcome = inspectLayout(page).catch((error) => error as Error); + await vi.advanceTimersByTimeAsync(LAYOUT_BUDGET_MS + 1); + const error = await outcome; + if (!(error instanceof Error)) throw new Error('Expected a layout timeout'); + expect(error.message).toContain('timed out'); + const widthWhenCallerResumes = viewport.width; + finishPhone(); + await vi.advanceTimersByTimeAsync(0); + expect(viewport.width).toBe(desktop.width); + expect(widthWhenCallerResumes).toBe(desktop.width); +}); + +it('does not start measuring or resize after an already cancelled call', async () => { + const controller = new AbortController(); + controller.abort(); + const viewport = vi.fn(); + const evaluate = vi.fn(); + await expect( + inspectLayout({ viewport, evaluate } as unknown as Page, controller.signal), + ).rejects.toThrow('Preview cancelled'); + expect(viewport).not.toHaveBeenCalled(); + expect(evaluate).not.toHaveBeenCalled(); +}); + +it('does not resize later when the desktop measurement outlives its budget', async () => { + vi.useFakeTimers(); + const setViewport = vi.fn(); + let finishDesktop!: (value: ReturnType) => void; + const page = { + viewport: () => ({ width: 1280, height: 800 }), + setViewport, + evaluate: () => + new Promise>((resolve) => { + finishDesktop = resolve; + }), + } as unknown as Page; + const outcome = inspectLayout(page).catch((error) => error as Error); + await vi.advanceTimersByTimeAsync(LAYOUT_BUDGET_MS + 1); + expect(await outcome).toBeInstanceOf(Error); + finishDesktop(samples()); + await vi.advanceTimersByTimeAsync(0); + expect(setViewport).not.toHaveBeenCalled(); +}); + +it('restores the viewport before returning cancellation during the phone measurement', async () => { + const controller = new AbortController(); + const desktop = { width: 1280, height: 800 }; + let viewport = desktop; + let measurements = 0; + let finishPhone!: (value: ReturnType) => void; + const page = { + viewport: () => viewport, + setViewport: async (value: typeof desktop) => { + viewport = value; + }, + evaluate: async (fn: unknown) => { + if (fn !== collectLayoutSamples) return undefined; + measurements += 1; + if (measurements === 2) { + controller.abort(); + return new Promise>((resolve) => { + finishPhone = resolve; + }); + } + return samples({ viewportWidth: viewport.width }); + }, + } as unknown as Page; + await expect(inspectLayout(page, controller.signal)).rejects.toThrow('Preview cancelled'); + expect(viewport).toEqual(desktop); + finishPhone(samples()); + await Promise.resolve(); + expect(viewport).toEqual(desktop); +}); diff --git a/apps/desktop/src/main/layout-probe.test.ts b/apps/desktop/src/main/layout-probe.test.ts new file mode 100644 index 000000000..60383a390 --- /dev/null +++ b/apps/desktop/src/main/layout-probe.test.ts @@ -0,0 +1,249 @@ +import { LAYOUT_WARNING_SOURCE, type LayoutIssue, MAX_LAYOUT_ISSUES } from '@open-codesign/core'; +import type { Page } from 'puppeteer-core'; +import { describe, expect, it } from 'vitest'; +import { + collectLayoutSamples, + inspectLayout, + layoutFailureWarning, + layoutWarnings, +} from './layout-probe'; +import { + ALL_CHECKS, + LAYOUT_PROBE_LIMITS, + type LayoutProbeLimits, + type LayoutSamples, + PHONE_VIEWPORT, +} from './layout-samples'; +import { samples } from './layout-test-fixtures'; + +describe('layout warnings', () => { + it('shows each finding and note as a layout warning', () => { + const issue: LayoutIssue = { + check: 'horizontal-overflow', + viewportWidth: 390, + selector: 'html', + message: 'The page scrolls sideways.', + measured: 520, + limit: 390, + }; + + expect(layoutWarnings({ issues: [issue], notes: ['Not the whole page.'] })).toEqual([ + { source: LAYOUT_WARNING_SOURCE, message: 'The page scrolls sideways.' }, + { source: LAYOUT_WARNING_SOURCE, message: 'Not the whole page.' }, + ]); + expect(layoutWarnings({ issues: [], notes: [] })).toEqual([]); + }); + + it('says why the checks did not run', () => { + expect(layoutFailureWarning(new Error('Preview operation timed out'))).toEqual({ + source: LAYOUT_WARNING_SOURCE, + message: 'Layout checks did not run: Preview operation timed out', + }); + expect(layoutFailureWarning('boom').message).toBe('Layout checks did not run: boom'); + }); +}); + +interface FakePage { + page: Page; + viewports: Array<{ width: number; height: number }>; + /** The limits the page-side function was called with, one per measurement. */ + calls: LayoutProbeLimits[]; +} + +/** A page whose measurements come from a script, one per call of the collector. */ +function fakePage( + original: { width: number; height: number } | null, + measurements: Array, +): FakePage { + const viewports: Array<{ width: number; height: number }> = []; + const calls: LayoutProbeLimits[] = []; + const queue = [...measurements]; + const page = { + viewport: () => original, + setViewport: async (viewport: { width: number; height: number }) => { + viewports.push(viewport); + }, + evaluate: async (fn: unknown, limits: LayoutProbeLimits) => { + if (fn !== collectLayoutSamples) return undefined; + calls.push(limits); + const next = queue.shift(); + if (next === undefined) throw new Error('Unexpected extra measurement'); + if (next instanceof Error) throw next; + return next; + }, + }; + return { page: page as unknown as Page, viewports, calls }; +} + +const desktop = { width: 1280, height: 800 }; +const atDesktop = (overrides: Partial = {}) => + samples({ viewportWidth: 1280, documentWidth: 1280, ...overrides }); +const overflowAtPhone = samples({ + documentWidth: 700, + overflowing: [{ selector: 'table#prices', right: 700 }], +}); + +describe('inspectLayout', () => { + it('measures at the current width and at phone width, then restores the viewport', async () => { + const { page, viewports } = fakePage(desktop, [atDesktop(), overflowAtPhone]); + + const inspection = await inspectLayout(page); + + expect(inspection.issues.map((issue) => [issue.check, issue.viewportWidth])).toEqual([ + ['horizontal-overflow', 390], + ]); + expect(inspection.notes).toEqual([]); + expect(viewports).toEqual([{ ...desktop, ...PHONE_VIEWPORT }, desktop]); + }); + + it('runs contrast and names at the current width, and tap targets at phone width', async () => { + const { page, calls } = fakePage(desktop, [atDesktop(), samples()]); + + await inspectLayout(page); + + expect(calls.map((call) => call.checks)).toEqual([ + { overflow: true, clipped: true, contrast: true, targets: false, names: true }, + { overflow: true, clipped: true, contrast: false, targets: true, names: false }, + ]); + expect(calls.every((call) => call.maxElements === LAYOUT_PROBE_LIMITS.maxElements)).toBe(true); + }); + + it('runs every check in one pass on a page that is already phone width', async () => { + const { page, viewports, calls } = fakePage({ width: PHONE_VIEWPORT.width, height: 700 }, [ + overflowAtPhone, + ]); + + const inspection = await inspectLayout(page); + + expect(inspection.issues).toHaveLength(1); + expect(viewports).toEqual([]); + expect(calls.map((call) => call.checks)).toEqual([ALL_CHECKS]); + }); + + it('restores the viewport and rejects when the phone measurement fails', async () => { + const { page, viewports } = fakePage(desktop, [ + atDesktop(), + new Error('Execution context was destroyed'), + ]); + + await expect(inspectLayout(page)).rejects.toThrow('Execution context was destroyed'); + + expect(viewports.at(-1)).toEqual(desktop); + }); + + it('rejects when the page has no viewport to measure', async () => { + const { page } = fakePage(null, []); + + await expect(inspectLayout(page)).rejects.toThrow('The page has no viewport to measure'); + }); + + it('stops when the run is cancelled', async () => { + const controller = new AbortController(); + controller.abort(); + const { page } = fakePage(desktop, [samples(), samples()]); + + await expect(inspectLayout(page, controller.signal)).rejects.toThrow('Preview cancelled'); + }); + + describe('when there is more than the budget', () => { + const clipped = (count: number) => + Array.from({ length: count }, (_, index) => ({ + selector: `p.item-${index}`, + side: 'right' as const, + textEdge: 400, + boxEdge: 300, + })); + + it('lists the budget, one check at a time, and says how many there were', async () => { + const { page } = fakePage(desktop, [ + atDesktop({ + clipped: clipped(5), + textGroups: Array.from({ length: 30 }, (_, index) => ({ + selector: `p.low-${index}`, + count: 1, + foreground: [200 - index, 200 - index, 200 - index, 1], + layers: [], + large: false, + })), + }), + samples({ unnamedControls: [{ selector: 'button.menu', kind: 'button' }] }), + ]); + + const inspection = await inspectLayout(page); + + expect(inspection.issues).toHaveLength(MAX_LAYOUT_ISSUES); + expect(new Set(inspection.issues.map((issue) => issue.check))).toEqual( + new Set(['clipped-text', 'low-contrast', 'missing-name']), + ); + expect(inspection.notes).toEqual([ + `Layout checks found 36 issues; ${MAX_LAYOUT_ISSUES} are listed, taken from each check in turn.`, + ]); + }); + + it('says how many findings went past the per-check limits', async () => { + const { page } = fakePage(desktop, [atDesktop({ omitted: 3 }), samples({ omitted: 1 })]); + + const inspection = await inspectLayout(page); + + expect(inspection.notes).toEqual([ + '4 more findings past the per-check limits were not listed.', + ]); + const single = await inspectLayout( + fakePage(desktop, [atDesktop({ omitted: 1 }), samples()]).page, + ); + expect(single.notes).toEqual(['1 more finding past the per-check limits was not listed.']); + }); + }); + + describe('notes about the measurement', () => { + it('says when the page was only measured in part', async () => { + const { page } = fakePage(desktop, [atDesktop({ truncated: true }), samples()]); + + const inspection = await inspectLayout(page); + + expect(inspection.issues).toEqual([]); + expect(inspection.notes).toEqual([ + 'Layout checks did not cover the whole page (more than 5,000 elements or 400 controls); the rest was not measured.', + ]); + }); + + it('says how many text elements were not measured for contrast, and why', async () => { + const many = fakePage(desktop, [atDesktop({ unmeasuredText: 12 }), samples()]); + const one = fakePage(desktop, [atDesktop({ unmeasuredText: 1 }), samples()]); + + expect((await inspectLayout(many.page)).notes).toEqual([ + 'Contrast was not checked for 12 text elements on a background image or gradient, or partly transparent.', + ]); + expect((await inspectLayout(one.page)).notes).toEqual([ + 'Contrast was not checked for 1 text element on a background image or gradient, or partly transparent.', + ]); + }); + + it('says how many colour combinations were past the limit', async () => { + const { page } = fakePage(desktop, [atDesktop({ droppedTextGroups: 3 }), samples()]); + + expect((await inspectLayout(page)).notes).toEqual([ + '3 colour combinations beyond the first 200 were not checked for contrast.', + ]); + }); + }); + + it('passes the documented limits to the page', () => { + expect(LAYOUT_PROBE_LIMITS).toMatchObject({ + maxElements: 5000, + overflowTolerance: 1, + clipTolerance: 2, + maxOverflowElements: 3, + maxClippedTexts: 5, + largeTextPx: 24, + largeBoldTextPx: 18.66, + boldWeight: 700, + maxTextGroups: 200, + minMeasuredOpacity: 0.99, + minTargetSizePx: 24, + maxTargets: 400, + maxSmallTargets: 5, + maxUnnamedControls: 5, + }); + }); +}); diff --git a/apps/desktop/src/main/layout-probe.ts b/apps/desktop/src/main/layout-probe.ts new file mode 100644 index 000000000..f02fb5945 --- /dev/null +++ b/apps/desktop/src/main/layout-probe.ts @@ -0,0 +1,548 @@ +/** + * Deterministic layout and accessibility checks for the pages `preview` and + * `done` already open in Chrome. The page-side function only collects raw + * measurements; the decisions (what counts as overflow, low contrast, a small + * target) are made on that data in `layout-analysis.ts`, so they can be tested + * without a browser. + * + * Findings are advice. They never change `ok` or the `done` status, because a + * heuristic can be wrong and a false positive must not cost a repair round. + */ + +import { + type DoneError, + LAYOUT_WARNING_SOURCE, + type LayoutIssue, + MAX_LAYOUT_ISSUES, +} from '@open-codesign/core'; +import type { Page } from 'puppeteer-core'; +import { analyzeLayoutSamples, selectIssues } from './layout-analysis'; +import { + ALL_CHECKS, + LAYOUT_BUDGET_MS, + LAYOUT_PROBE_LIMITS, + LAYOUT_RESTORE_BUDGET_MS, + type LayoutProbeChecks, + type LayoutProbeLimits, + type LayoutSamples, + PHONE_VIEWPORT, + type Rgba, + type TextGroup, +} from './layout-samples'; +import { boundedPreview } from './preview-interactions'; + +// collectLayoutSamples is serialized into the page. Node's tsconfig has no DOM +// types, so these describe only the browser globals it touches. +interface PageStyle { + display: string; + position: string; + visibility: string; + opacity: string; + overflowX: string; + overflowY: string; + textOverflow: string; + webkitLineClamp?: string; + animationName: string; + color: string; + backgroundColor: string; + backgroundImage: string; + fontSize: string; + fontWeight: string; +} +interface PageRect { + left: number; + right: number; + top: number; + bottom: number; + width: number; + height: number; +} +interface PageNode { + nodeType: number; + textContent: string | null; +} +interface PageElement extends PageNode { + tagName: string; + id: string; + classList: ArrayLike; + parentElement: PageElement | null; + children: ArrayLike; + childNodes: ArrayLike; + disabled?: boolean; + value?: string; + alt?: string; + labels?: ArrayLike; + getBoundingClientRect(): PageRect; + querySelectorAll(selector: string): ArrayLike; + matches(selector: string): boolean; + closest(selector: string): PageElement | null; + getAttribute(name: string): string | null; +} +interface PageCanvasContext { + globalCompositeOperation: string; + fillStyle: string; + fillRect(x: number, y: number, width: number, height: number): void; + getImageData(x: number, y: number, width: number, height: number): { data: ArrayLike }; +} +interface PageCanvas { + width: number; + height: number; + getContext(type: '2d', options: { willReadFrequently: boolean }): PageCanvasContext | null; +} +declare const document: { + documentElement: PageElement & { clientWidth: number; scrollWidth: number }; + body: PageElement | null; + createRange(): { selectNodeContents(node: PageNode): void; getBoundingClientRect(): PageRect }; + createElement(tag: 'canvas'): PageCanvas; + getElementById(id: string): PageElement | null; +}; +declare const Node: { TEXT_NODE: number }; +declare function getComputedStyle(element: PageElement): PageStyle; +declare function requestAnimationFrame(callback: () => void): number; + +/** + * Runs inside the page: plain DOM, `getComputedStyle`, `getBoundingClientRect` + * and a 1 × 1 canvas to turn any CSS colour into sRGB. It must stay + * self-contained, because Puppeteer serializes it to source. + */ +export function collectLayoutSamples(limits: LayoutProbeLimits): LayoutSamples { + const { checks } = limits; + const root = document.documentElement; + const body = document.body; + const clipsX = (value: string) => value === 'hidden' || value === 'clip'; + const rootOverflowX = getComputedStyle(root).overflowX; + const bodyOverflowX = body ? getComputedStyle(body).overflowX : 'visible'; + const rootClipsX = + clipsX(rootOverflowX) || (rootOverflowX === 'visible' && clipsX(bodyOverflowX)); + const viewportWidth = root.clientWidth; + const documentWidth = root.scrollWidth; + const samples: LayoutSamples = { + viewportWidth, + documentWidth, + rootClipsX, + overflowing: [], + clipped: [], + textGroups: [], + unmeasuredText: 0, + droppedTextGroups: 0, + omitted: 0, + smallTargets: [], + unnamedControls: [], + truncated: false, + }; + if (!body) return samples; + + const selectorOf = (element: PageElement): string => { + const parts: string[] = []; + let node: PageElement | null = element; + while (node && node !== body && parts.length < 3) { + let part = node.tagName.toLowerCase(); + if (node.id) { + parts.unshift(`${part}#${node.id}`); + node = null; + break; + } + const classes = Array.from(node.classList).slice(0, 2); + if (classes.length > 0) part += `.${classes.join('.')}`; + parts.unshift(part); + node = node.parentElement; + } + const path = parts.join(' > '); + return (node === body || node === null ? path : `… > ${path}`).slice(0, 120); + }; + const hasText = (node: PageNode) => (node.textContent ?? '').trim() !== ''; + const ownTextNodes = (element: PageElement) => + Array.from(element.childNodes).filter( + (child) => child.nodeType === Node.TEXT_NODE && hasText(child), + ); + + // Any CSS colour, as sRGB: computed colours can be in other spaces (oklch, color-mix). + const colors = new Map(); + const canvas = checks.contrast ? document.createElement('canvas') : null; + if (canvas) { + canvas.width = 1; + canvas.height = 1; + } + const context = canvas ? canvas.getContext('2d', { willReadFrequently: true }) : null; + if (checks.contrast && !context) throw new Error('A 2D canvas is not available to read colours'); + // Computed sRGB colours are `rgb(r, g, b)` or `rgba(r, g, b, a)`, which are read exactly; + // a canvas rounds alpha to 8 bits, so it is used only for the other spaces. + const legacyColor = /^rgba?\((\d+), (\d+), (\d+)(?:, ([\d.]+))?\)$/; + const toRgba = (value: string): Rgba => { + const known = colors.get(value); + if (known) return known; + const parts = legacyColor.exec(value); + if (parts) { + const exact: Rgba = [ + Number(parts[1]), + Number(parts[2]), + Number(parts[3]), + parts[4] === undefined ? 1 : Number(parts[4]), + ]; + colors.set(value, exact); + return exact; + } + if (!context) throw new Error('A 2D canvas is not available to read colours'); + context.globalCompositeOperation = 'copy'; + context.fillStyle = value; + context.fillRect(0, 0, 1, 1); + const data = context.getImageData(0, 0, 1, 1).data; + const color: Rgba = [ + data[0] ?? 0, + data[1] ?? 0, + data[2] ?? 0, + Math.round(((data[3] ?? 0) / 255) * 1000) / 1000, + ]; + colors.set(value, color); + return color; + }; + + const sideways = + checks.overflow && documentWidth - viewportWidth > limits.overflowTolerance && !rootClipsX; + const extending = new Set(); + const textGroups = new Map(); + const targets: Array<{ element: PageElement; box: PageRect }> = []; + const controlSelector = + 'a[href], button, input:not([type="hidden"]), select, textarea, summary, [role="button"], [role="link"], [role="tab"], [role="checkbox"], [role="radio"], [role="switch"], [role="menuitem"], [role="textbox"], [role="combobox"], [role="slider"]'; + const elements = body.querySelectorAll('*'); + const visited = Math.min(elements.length, limits.maxElements); + samples.truncated = elements.length > limits.maxElements; + + const clippedByAncestor = (element: PageElement): boolean => { + for (let node = element.parentElement; node && node !== body; node = node.parentElement) { + if (getComputedStyle(node).overflowX !== 'visible') return true; + } + return false; + }; + + const textOf = (node: PageNode | null) => (node?.textContent ?? '').replace(/\s+/g, ' ').trim(); + const accessibleName = (element: PageElement): string => { + const labelledBy = (element.getAttribute('aria-labelledby') ?? '') + .split(/\s+/) + .filter(Boolean) + .map((id) => textOf(document.getElementById(id))) + .join(' ') + .trim(); + if (labelledBy) return labelledBy; + const label = (element.getAttribute('aria-label') ?? '').trim(); + if (label) return label; + const tag = element.tagName.toLowerCase(); + const type = (element.getAttribute('type') ?? '').toLowerCase(); + if (tag === 'input') { + if (type === 'submit' || type === 'reset') return (element.value ?? '').trim() || type; + if (type === 'button') return (element.value ?? '').trim(); + if (type === 'image') return (element.alt ?? '').trim(); + } + const labelText = Array.from(element.labels ?? []) + .map((node) => textOf(node)) + .join(' ') + .trim(); + if (labelText) return labelText; + if (tag !== 'input' && tag !== 'select' && tag !== 'textarea') { + const text = textOf(element); + if (text) return text; + } + for (const inner of Array.from(element.querySelectorAll('img[alt], svg title, [aria-label]'))) { + const innerName = + (inner.getAttribute('alt') ?? '').trim() || + (inner.getAttribute('aria-label') ?? '').trim() || + (inner.tagName.toLowerCase() === 'title' ? textOf(inner) : ''); + if (innerName) return innerName; + } + return (element.getAttribute('title') ?? '').trim(); + }; + + for (let index = 0; index < visited; index += 1) { + const element = elements[index]; + if (!element) continue; + const style = getComputedStyle(element); + if (style.display === 'none') continue; + const box = element.getBoundingClientRect(); + const shown = style.visibility === 'visible'; + + if (sideways && style.position !== 'fixed' && box.width > 0) { + if (box.right > viewportWidth + limits.overflowTolerance && !clippedByAncestor(element)) { + extending.add(element); + } + } + + if (checks.clipped) { + const clipsText = clipsX(style.overflowX) || clipsX(style.overflowY); + const intentional = + style.textOverflow === 'ellipsis' || + (style.webkitLineClamp !== undefined && style.webkitLineClamp !== 'none') || + style.animationName !== 'none'; + if (clipsText && !intentional && box.width >= 1 && box.height >= 1) { + for (const child of ownTextNodes(element)) { + const range = document.createRange(); + range.selectNodeContents(child); + const text = range.getBoundingClientRect(); + if (text.width < 1 || text.height < 1) continue; + const sides = [ + { side: 'right', beyond: text.right - box.right, text: text.right, box: box.right }, + { + side: 'bottom', + beyond: text.bottom - box.bottom, + text: text.bottom, + box: box.bottom, + }, + { side: 'left', beyond: box.left - text.left, text: text.left, box: box.left }, + { side: 'top', beyond: box.top - text.top, text: text.top, box: box.top }, + ] as const; + const worst = sides.reduce((most, next) => (next.beyond > most.beyond ? next : most)); + if (worst.beyond > limits.clipTolerance) { + if (samples.clipped.length < limits.maxClippedTexts) { + samples.clipped.push({ + selector: selectorOf(element), + side: worst.side, + textEdge: Math.round(worst.text), + boxEdge: Math.round(worst.box), + }); + } else { + samples.omitted += 1; + } + break; + } + } + } + } + + if (checks.contrast && shown && box.width >= 1 && box.height >= 1) { + if (ownTextNodes(element).length > 0) { + let opacity = 1; + for (let node: PageElement | null = element; node; node = node.parentElement) { + opacity *= Number(getComputedStyle(node).opacity); + } + // The backgrounds behind the text: its own ancestors, up to the first opaque one. + const layers: Rgba[] = []; + let pictured = false; + for (let node: PageElement | null = element; node; node = node.parentElement) { + const nodeStyle = getComputedStyle(node); + if (nodeStyle.backgroundImage !== 'none') { + pictured = true; + break; + } + const layer = toRgba(nodeStyle.backgroundColor); + if (layer[3] > 0) { + layers.push(layer); + if (layer[3] === 1) break; + } + } + if (pictured || opacity < limits.minMeasuredOpacity) { + samples.unmeasuredText += 1; + } else { + const size = Number.parseFloat(style.fontSize); + const large = + size >= limits.largeTextPx || + (size >= limits.largeBoldTextPx && + Number.parseInt(style.fontWeight, 10) >= limits.boldWeight); + const foreground = toRgba(style.color); + const key = `${foreground.join(',')}|${layers.map((layer) => layer.join(',')).join(';')}|${large}`; + const group = textGroups.get(key); + if (group) { + group.count += 1; + } else if (textGroups.size < limits.maxTextGroups) { + textGroups.set(key, { + selector: selectorOf(element), + count: 1, + foreground, + layers, + large, + }); + } else { + samples.droppedTextGroups += 1; + } + } + } + } + + if ((checks.targets || checks.names) && shown && element.matches(controlSelector)) { + if (!element.closest('[aria-hidden="true"]')) { + if (checks.names && accessibleName(element) === '') { + if (samples.unnamedControls.length < limits.maxUnnamedControls) { + const role = element.getAttribute('role'); + samples.unnamedControls.push({ + selector: selectorOf(element), + kind: `${element.tagName.toLowerCase()}${role ? ` role=${role}` : ''}`, + }); + } else { + samples.omitted += 1; + } + } + // A control with no size, or hidden by opacity, is a visually hidden input whose label is the target. + const measurable = box.width >= 2 && box.height >= 2 && style.opacity !== '0'; + if ( + checks.targets && + measurable && + element.disabled !== true && + style.display !== 'inline' + ) { + if (targets.length < limits.maxTargets) targets.push({ element, box }); + else samples.truncated = true; + } + } + } + } + + // Name the outermost elements that cross the edge: along each chain, the first box + // that goes past it is where the overflow starts. + const outermost: Array<{ selector: string; right: number }> = []; + for (const element of extending) { + if (element.parentElement && extending.has(element.parentElement)) continue; + outermost.push({ + selector: selectorOf(element), + right: Math.round(element.getBoundingClientRect().right), + }); + } + outermost.sort((a, b) => b.right - a.right); + samples.overflowing = outermost.slice(0, limits.maxOverflowElements); + samples.omitted += Math.max(0, outermost.length - limits.maxOverflowElements); + + samples.textGroups = Array.from(textGroups.values()); + + // A small target passes when the square around its centre touches no other target. + const half = limits.minTargetSizePx / 2; + const small: Array<{ selector: string; width: number; height: number }> = []; + for (const { element, box } of targets) { + if (box.width >= limits.minTargetSizePx && box.height >= limits.minTargetSizePx) continue; + const centerX = (box.left + box.right) / 2; + const centerY = (box.top + box.bottom) / 2; + const crowded = targets.some( + (other) => + other.element !== element && + other.box.left < centerX + half && + other.box.right > centerX - half && + other.box.top < centerY + half && + other.box.bottom > centerY - half, + ); + if (crowded) { + small.push({ + selector: selectorOf(element), + width: Math.round(box.width), + height: Math.round(box.height), + }); + } + } + small.sort((a, b) => Math.min(a.width, a.height) - Math.min(b.width, b.height)); + samples.smallTargets = small.slice(0, limits.maxSmallTargets); + samples.omitted += Math.max(0, small.length - limits.maxSmallTargets); + return samples; +} + +export interface LayoutInspection { + issues: LayoutIssue[]; + /** Things the agent should know about the measurement itself, such as a cap that was hit. */ + notes: string[]; +} + +/** Waits two frames so a resized page has laid out again. */ +async function settleLayout(page: Page): Promise { + await page.evaluate( + () => + new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + }), + ); +} + +async function measure(page: Page, checks: LayoutProbeChecks): Promise { + return page.evaluate(collectLayoutSamples, { ...LAYOUT_PROBE_LIMITS, checks }); +} + +/** + * Measures the open page at its current viewport and at phone width, then puts + * the viewport back. A failure rejects, so the caller reports it instead of + * silently reporting a clean page. + */ +export async function inspectLayout(page: Page, signal?: AbortSignal): Promise { + return runInspection(page, signal); +} + +/** Contrast and names do not depend on width, so the phone pass skips them; tap targets matter there. */ +const CURRENT_WIDTH_CHECKS: LayoutProbeChecks = { ...ALL_CHECKS, targets: false }; +const PHONE_WIDTH_CHECKS: LayoutProbeChecks = { + overflow: true, + clipped: true, + contrast: false, + targets: true, + names: false, +}; + +async function runInspection(page: Page, signal?: AbortSignal): Promise { + const deadline = Date.now() + LAYOUT_BUDGET_MS; + // Bound each await so a timed-out operation cannot resume the inspection and + // resize a page after its caller has started collecting preview evidence. + const withinBudget = (operation: () => Promise): Promise => { + if (signal?.aborted) return Promise.reject(new Error('Preview cancelled')); + const remaining = deadline - Date.now(); + if (remaining <= 0) return Promise.reject(new Error('Preview operation timed out')); + return boundedPreview(operation(), remaining, signal); + }; + if (signal?.aborted) throw new Error('Preview cancelled'); + const original = page.viewport(); + if (original === null) throw new Error('The page has no viewport to measure'); + const alreadyPhone = original.width === PHONE_VIEWPORT.width; + const issues: LayoutIssue[] = []; + const measured: LayoutSamples[] = []; + const take = (samples: LayoutSamples) => { + measured.push(samples); + issues.push(...analyzeLayoutSamples(samples)); + }; + + take(await withinBudget(() => measure(page, alreadyPhone ? ALL_CHECKS : CURRENT_WIDTH_CHECKS))); + if (!alreadyPhone) { + try { + await withinBudget(() => page.setViewport({ ...original, ...PHONE_VIEWPORT })); + await withinBudget(() => settleLayout(page)); + take(await withinBudget(() => measure(page, PHONE_WIDTH_CHECKS))); + } finally { + // Cleanup must finish before the caller resumes, even after cancellation + // or exhaustion of the measurement budget, but must itself stay bounded. + await boundedPreview(page.setViewport(original), LAYOUT_RESTORE_BUDGET_MS); + } + } + + const notes: string[] = []; + if (measured.some((samples) => samples.truncated)) { + notes.push( + 'Layout checks did not cover the whole page (more than 5,000 elements or 400 controls); the rest was not measured.', + ); + } + const unmeasured = measured.reduce((sum, samples) => sum + samples.unmeasuredText, 0); + if (unmeasured > 0) { + notes.push( + `Contrast was not checked for ${unmeasured} text element${unmeasured === 1 ? '' : 's'} on a background image or gradient, or partly transparent.`, + ); + } + const dropped = measured.reduce((sum, samples) => sum + samples.droppedTextGroups, 0); + if (dropped > 0) { + notes.push( + `${dropped} colour combination${dropped === 1 ? '' : 's'} beyond the first 200 were not checked for contrast.`, + ); + } + const omitted = measured.reduce((sum, samples) => sum + samples.omitted, 0); + if (omitted > 0) { + notes.push( + `${omitted} more finding${omitted === 1 ? '' : 's'} past the per-check limits ${omitted === 1 ? 'was' : 'were'} not listed.`, + ); + } + if (issues.length > MAX_LAYOUT_ISSUES) { + notes.push( + `Layout checks found ${issues.length} issues; ${MAX_LAYOUT_ISSUES} are listed, taken from each check in turn.`, + ); + } + return { issues: selectIssues(issues, MAX_LAYOUT_ISSUES), notes }; +} + +/** What `done` shows for a measurement: each finding and note as a non-blocking warning. */ +export function layoutWarnings(inspection: LayoutInspection): DoneError[] { + return [...inspection.issues.map((issue) => issue.message), ...inspection.notes].map( + (message) => ({ message, source: LAYOUT_WARNING_SOURCE }), + ); +} + +/** What `done` shows when the measurement itself failed. */ +export function layoutFailureWarning(error: unknown): DoneError { + const reason = error instanceof Error ? error.message : String(error); + return { message: `Layout checks did not run: ${reason}`, source: LAYOUT_WARNING_SOURCE }; +} diff --git a/apps/desktop/src/main/layout-samples.ts b/apps/desktop/src/main/layout-samples.ts new file mode 100644 index 000000000..f4a0579f4 --- /dev/null +++ b/apps/desktop/src/main/layout-samples.ts @@ -0,0 +1,163 @@ +/** + * What the page-side layout function measures, and the named limits that bound + * it. Plain data and constants only: the function runs in the page, the + * decisions are made on this data in `layout-analysis.ts`. + */ + +/** The narrow viewport every page is also measured at. */ +export const PHONE_VIEWPORT = { width: 390, height: 844 } as const; +/** Most elements one measurement visits; a larger page is measured only in part. */ +export const MAX_PROBE_ELEMENTS = 5000; +/** Time allowed for all measurements together. */ +export const LAYOUT_BUDGET_MS = 5000; +/** Cleanup has its own budget so a cancelled inspection can restore the preview viewport. */ +export const LAYOUT_RESTORE_BUDGET_MS = 1000; + +/** Sub-pixel rounding is not overflow. */ +export const OVERFLOW_TOLERANCE_PX = 1; +/** Text may stick out of a clipping box by this much before it counts as clipped. */ +export const CLIP_TOLERANCE_PX = 2; +/** Elements named per viewport when the page scrolls sideways. */ +export const MAX_OVERFLOW_ELEMENTS = 3; +/** Clipped texts named per viewport. */ +export const MAX_CLIPPED_TEXTS = 5; + +/** WCAG 2.x level AA contrast: 4.5:1 for text, 3:1 for large text. */ +export const MIN_CONTRAST_RATIO = 4.5; +export const MIN_LARGE_TEXT_CONTRAST_RATIO = 3; +/** Large text is at least 24 px, or at least 18.66 px and bold (WCAG's 18 pt and 14 pt bold). */ +export const LARGE_TEXT_PX = 24; +export const LARGE_BOLD_TEXT_PX = 18.66; +export const BOLD_WEIGHT = 700; +/** Distinct colour and size combinations kept per measurement; more are counted, not checked. */ +export const MAX_TEXT_GROUPS = 200; +/** Text this transparent, or sitting in something this transparent, is not measured. */ +export const MIN_MEASURED_OPACITY = 0.99; + +/** WCAG 2.2 SC 2.5.8: a pointer target is at least 24 by 24 CSS px, or has room around it. */ +export const MIN_TARGET_SIZE_PX = 24; +/** Targets looked at per measurement; the spacing exception compares each with the others. */ +export const MAX_TARGETS = 400; +/** Small targets named per viewport, smallest first. */ +export const MAX_SMALL_TARGETS = 5; + +/** Controls without a name named per measurement. */ +export const MAX_UNNAMED_CONTROLS = 5; + +/** Red, green and blue as 0 to 255 and alpha as 0 to 1, in sRGB. */ +export type Rgba = [number, number, number, number]; + +export interface OverflowingElement { + selector: string; + /** Right edge of the element, in CSS px from the left of the viewport. */ + right: number; +} + +/** The side of a clipping box that its text sticks out of the most. */ +export type ClippedSide = 'right' | 'bottom' | 'left' | 'top'; + +export interface ClippedText { + selector: string; + side: ClippedSide; + /** The text's edge on that side, in CSS px from the top left of the viewport. */ + textEdge: number; + /** The box's edge on that side, in the same px. */ + boxEdge: number; +} + +/** Text elements that share a colour, the backgrounds behind it and a size class. */ +export interface TextGroup { + selector: string; + /** How many text elements share this combination. */ + count: number; + foreground: Rgba; + /** The backgrounds behind the text, innermost first, up to and including the first opaque one. */ + layers: Rgba[]; + large: boolean; +} + +export interface SmallTarget { + selector: string; + width: number; + height: number; +} + +export interface UnnamedControl { + selector: string; + /** The tag, plus the role when there is one, for the message. */ + kind: string; +} + +/** What the page-side function returns. Plain data, so it survives serialization. */ +export interface LayoutSamples { + viewportWidth: number; + documentWidth: number; + /** The page cannot scroll sideways, because the root or body hides horizontal overflow. */ + rootClipsX: boolean; + overflowing: OverflowingElement[]; + clipped: ClippedText[]; + textGroups: TextGroup[]; + /** Text elements on a background image or gradient, or partly transparent, that were not measured. */ + unmeasuredText: number; + /** Colour combinations past `maxTextGroups` that were not measured. */ + droppedTextGroups: number; + /** Findings past a per-check limit (overflowing elements, clipped texts, small targets, unnamed controls). */ + omitted: number; + smallTargets: SmallTarget[]; + unnamedControls: UnnamedControl[]; + /** The element limit was reached, so the rest of the page was not measured. */ + truncated: boolean; +} + +/** Which checks one measurement runs. Contrast and names do not depend on width; tap targets matter at phone width. */ +export interface LayoutProbeChecks { + overflow: boolean; + clipped: boolean; + contrast: boolean; + targets: boolean; + names: boolean; +} + +export interface LayoutProbeLimits { + checks: LayoutProbeChecks; + maxElements: number; + overflowTolerance: number; + clipTolerance: number; + maxOverflowElements: number; + maxClippedTexts: number; + largeTextPx: number; + largeBoldTextPx: number; + boldWeight: number; + maxTextGroups: number; + minMeasuredOpacity: number; + minTargetSizePx: number; + maxTargets: number; + maxSmallTargets: number; + maxUnnamedControls: number; +} + +export const ALL_CHECKS: LayoutProbeChecks = { + overflow: true, + clipped: true, + contrast: true, + targets: true, + names: true, +}; + +export const LAYOUT_PROBE_LIMITS: LayoutProbeLimits = { + checks: ALL_CHECKS, + maxElements: MAX_PROBE_ELEMENTS, + overflowTolerance: OVERFLOW_TOLERANCE_PX, + clipTolerance: CLIP_TOLERANCE_PX, + maxOverflowElements: MAX_OVERFLOW_ELEMENTS, + maxClippedTexts: MAX_CLIPPED_TEXTS, + largeTextPx: LARGE_TEXT_PX, + largeBoldTextPx: LARGE_BOLD_TEXT_PX, + boldWeight: BOLD_WEIGHT, + maxTextGroups: MAX_TEXT_GROUPS, + minMeasuredOpacity: MIN_MEASURED_OPACITY, + minTargetSizePx: MIN_TARGET_SIZE_PX, + maxTargets: MAX_TARGETS, + maxSmallTargets: MAX_SMALL_TARGETS, + maxUnnamedControls: MAX_UNNAMED_CONTROLS, +}; diff --git a/apps/desktop/src/main/layout-test-fixtures.ts b/apps/desktop/src/main/layout-test-fixtures.ts new file mode 100644 index 000000000..3abf009f8 --- /dev/null +++ b/apps/desktop/src/main/layout-test-fixtures.ts @@ -0,0 +1,20 @@ +import type { LayoutSamples } from './layout-samples'; + +/** A measurement of a page with nothing to report, with fields overridden by a test. */ +export function samples(overrides: Partial = {}): LayoutSamples { + return { + viewportWidth: 390, + documentWidth: 390, + rootClipsX: false, + overflowing: [], + clipped: [], + textGroups: [], + unmeasuredText: 0, + droppedTextGroups: 0, + omitted: 0, + smallTargets: [], + unnamedControls: [], + truncated: false, + ...overrides, + }; +} diff --git a/apps/desktop/src/main/layout-wiring.browser.test.ts b/apps/desktop/src/main/layout-wiring.browser.test.ts new file mode 100644 index 000000000..d30f1d851 --- /dev/null +++ b/apps/desktop/src/main/layout-wiring.browser.test.ts @@ -0,0 +1,79 @@ +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { LAYOUT_WARNING_SOURCE, trimPreviewResult } from '@open-codesign/core'; +import { findSystemChrome } from '@open-codesign/exporters'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { makeRuntimeVerifier } from './done-verify'; +import { PHONE_VIEWPORT } from './layout-samples'; +import { runPreview } from './preview-runtime'; + +vi.mock('./logger', () => ({ getLogger: () => ({ warn: vi.fn(), error: vi.fn() }) })); + +const chromeAvailable = await findSystemChrome().then( + () => true, + () => false, +); + +const jsx = (style: string) => + `function App(){return
Quarterly report
;} ReactDOM.createRoot(document.getElementById('root')).render();`; +const FITS = jsx('{ padding: 16 }'); +const TOO_WIDE = jsx('{ width: 900, padding: 16 }'); + +describe.skipIf(!chromeAvailable)('layout checks reach done and preview', () => { + let workspace: string; + beforeEach(async () => { + workspace = await mkdtemp(join(tmpdir(), 'codesign-layout-wiring-')); + }); + afterEach(async () => { + await rm(workspace, { recursive: true, force: true }); + }); + + it('done lists a page that scrolls sideways at phone width as a warning, not an error', async () => { + const { errors, warnings } = await makeRuntimeVerifier({ workspaceRoot: workspace })(TOO_WIDE, { + path: 'App.jsx', + }); + + expect(errors).toEqual([]); + expect(warnings).toHaveLength(1); + expect(warnings[0]?.source).toBe(LAYOUT_WARNING_SOURCE); + expect(warnings[0]?.message).toMatch( + new RegExp(`past the ${PHONE_VIEWPORT.width} px viewport edge`), + ); + }, 60_000); + + it('done reports no warning for a page that fits at both widths', async () => { + const result = await makeRuntimeVerifier({ workspaceRoot: workspace })(FITS, { + path: 'App.jsx', + }); + + expect(result).toEqual({ errors: [], warnings: [] }); + }, 60_000); + + it('preview returns the same finding as layoutIssues and still reports ok', async () => { + await writeFile(join(workspace, 'App.jsx'), TOO_WIDE); + + const preview = await runPreview({ workspaceRoot: workspace, path: 'App.jsx', vision: false }); + + expect(preview.ok).toBe(true); + expect(preview.layoutIssues).toHaveLength(1); + expect(preview.layoutIssues?.[0]).toMatchObject({ + check: 'horizontal-overflow', + viewportWidth: PHONE_VIEWPORT.width, + }); + expect(trimPreviewResult(preview).layoutIssues).toEqual(preview.layoutIssues); + }, 60_000); + + it('preview leaves layoutIssues out for a page that fits, and measures a vision preview too', async () => { + await writeFile(join(workspace, 'App.jsx'), FITS); + + const text = await runPreview({ workspaceRoot: workspace, path: 'App.jsx', vision: false }); + const vision = await runPreview({ workspaceRoot: workspace, path: 'App.jsx', vision: true }); + + expect(text.layoutIssues).toBeUndefined(); + expect(text.warnings).toBeUndefined(); + expect(vision.layoutIssues).toBeUndefined(); + expect(vision.screenshot).toMatch(/^data:image\/png;base64,/); + expect(vision.metrics.width).toBe(1280); + }, 60_000); +}); diff --git a/apps/desktop/src/main/layout-wiring.failure.browser.test.ts b/apps/desktop/src/main/layout-wiring.failure.browser.test.ts new file mode 100644 index 000000000..00cacbac0 --- /dev/null +++ b/apps/desktop/src/main/layout-wiring.failure.browser.test.ts @@ -0,0 +1,111 @@ +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { LAYOUT_WARNING_SOURCE } from '@open-codesign/core'; +import { findSystemChrome } from '@open-codesign/exporters'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { makeRuntimeVerifier } from './done-verify'; +import { inspectLayout } from './layout-probe'; +import { runPreview } from './preview-runtime'; + +vi.mock('./logger', () => ({ getLogger: () => ({ warn: vi.fn(), error: vi.fn() }) })); +vi.mock('./layout-probe', async (importOriginal) => ({ + ...(await importOriginal()), + inspectLayout: vi.fn(), +})); + +const chromeAvailable = await findSystemChrome().then( + () => true, + () => false, +); +const SOURCE = `function App(){return
Hello
;} ReactDOM.createRoot(document.getElementById('root')).render();`; + +describe.skipIf(!chromeAvailable)('a layout measurement that fails or is cancelled', () => { + let workspace: string; + beforeEach(async () => { + workspace = await mkdtemp(join(tmpdir(), 'codesign-layout-failure-')); + await writeFile(join(workspace, 'App.jsx'), SOURCE); + }); + afterEach(async () => { + vi.mocked(inspectLayout).mockReset(); + await rm(workspace, { recursive: true, force: true }); + }); + + it('is reported by done as a warning and does not fail the run', async () => { + vi.mocked(inspectLayout).mockRejectedValueOnce(new Error('Execution context was destroyed')); + + const result = await makeRuntimeVerifier({ workspaceRoot: workspace })(SOURCE, { + path: 'App.jsx', + }); + + expect(result).toEqual({ + errors: [], + warnings: [ + { + source: LAYOUT_WARNING_SOURCE, + message: 'Layout checks did not run: Execution context was destroyed', + }, + ], + }); + }, 60_000); + + it('is reported by preview as a warning and does not fail the preview', async () => { + vi.mocked(inspectLayout).mockRejectedValueOnce(new Error('Execution context was destroyed')); + + const preview = await runPreview({ workspaceRoot: workspace, path: 'App.jsx', vision: false }); + + expect(preview.ok).toBe(true); + expect(preview.layoutIssues).toBeUndefined(); + expect(preview.warnings).toEqual([ + 'Layout checks did not run: Execution context was destroyed', + ]); + }, 60_000); + + it('carries the notes of a partial measurement into the preview warnings', async () => { + vi.mocked(inspectLayout).mockResolvedValueOnce({ + issues: [], + notes: ['Layout checks stopped after 5,000 elements; the rest of the page was not measured.'], + }); + + const preview = await runPreview({ workspaceRoot: workspace, path: 'App.jsx', vision: false }); + + expect(preview.ok).toBe(true); + expect(preview.warnings).toEqual([ + 'Layout checks stopped after 5,000 elements; the rest of the page was not measured.', + ]); + }, 60_000); + + it('still stops the done verifier when the run was cancelled', async () => { + const controller = new AbortController(); + vi.mocked(inspectLayout).mockImplementationOnce(async () => { + controller.abort(new Error('Stopped')); + throw new Error('Preview cancelled'); + }); + + await expect( + makeRuntimeVerifier({ workspaceRoot: workspace })(SOURCE, { + path: 'App.jsx', + signal: controller.signal, + }), + ).rejects.toThrow('Stopped'); + }, 60_000); + + it('still stops the preview when the run was cancelled', async () => { + const controller = new AbortController(); + vi.mocked(inspectLayout).mockImplementationOnce(async () => { + controller.abort(new Error('Stopped')); + throw new Error('Preview cancelled'); + }); + + const preview = await runPreview({ + workspaceRoot: workspace, + path: 'App.jsx', + vision: false, + signal: controller.signal, + }); + + expect(preview.ok).toBe(false); + expect(preview.reason).toBe('Preview cancelled'); + expect(preview.warnings).toBeUndefined(); + }, 60_000); +}); diff --git a/apps/desktop/src/main/preview-runtime.ts b/apps/desktop/src/main/preview-runtime.ts index caaef532d..891244b83 100644 --- a/apps/desktop/src/main/preview-runtime.ts +++ b/apps/desktop/src/main/preview-runtime.ts @@ -27,6 +27,7 @@ import { resolveArtifactSourceReferencePath, } from '@open-codesign/runtime'; import type { Browser, ConsoleMessage, HTTPRequest, HTTPResponse, Page } from 'puppeteer-core'; +import { inspectLayout, layoutFailureWarning } from './layout-probe'; import { getLogger } from './logger'; import { boundedPreview, collectVisibleEvidence, runPreviewSteps } from './preview-interactions'; import { resolveSafeWorkspaceChildPath } from './workspace-reader'; @@ -253,6 +254,15 @@ export async function runPreview(opts: RunPreviewOptions): Promise 0) result.layoutIssues = inspection.issues; + if (inspection.notes.length > 0) result.warnings = inspection.notes; + } catch (err) { + result.warnings = [layoutFailureWarning(err).message]; + } + if (opts.vision) { const png = await boundedPreview( page.screenshot({ type: 'png', encoding: 'base64' }), @@ -333,12 +343,9 @@ export async function runPreview(opts: RunPreviewOptions): Promise, doneVerify: (artifact: string) => ipcRenderer.invoke('done:verify:v1', { artifact }) as Promise<{ + schemaVersion: 1; errors: Array<{ message: string; source?: string; lineno?: number }>; + warnings: Array<{ message: string; source?: string; lineno?: number }>; }>, generate: (payload: { prompt: string; diff --git a/packages/core/src/agent.design-loop.test.ts b/packages/core/src/agent.design-loop.test.ts new file mode 100644 index 000000000..c8cdb0c0a --- /dev/null +++ b/packages/core/src/agent.design-loop.test.ts @@ -0,0 +1,412 @@ +import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { Agent, AgentMessage, AgentOptions, StreamFn } from '@mariozechner/pi-agent-core'; +import { type AssistantMessage, createAssistantMessageEventStream } from '@mariozechner/pi-ai'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { generateViaAgent } from './agent'; +import type { GenerateInput } from './index'; +import type { DoneDetails, DoneRuntimeResult, DoneRuntimeVerifier } from './tools/done'; +import type { LayoutIssue, PreviewResult, RunPreviewFn } from './tools/preview'; +import type { TextEditorFsCallbacks } from './tools/text-editor'; + +const native = vi.hoisted(() => ({ stream: vi.fn(), agents: [] as Agent[] })); + +// Substitute only pi's streaming boundary; Agent scheduling, tools and recovery are real. +vi.mock('@mariozechner/pi-agent-core', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + Agent: class extends actual.Agent { + constructor(options: AgentOptions) { + super({ ...options, streamFn: native.stream }); + native.agents.push(this); + } + }, + }; +}); + +const DESIGN = `--- +version: alpha +name: Test design +colors: + primary: "#111111" +typography: + body: + fontFamily: Inter + fontSize: 16px + fontWeight: 400 +rounded: + sm: 4px +spacing: + sm: 8px +--- + +## Overview + +An isolated integration fixture. +`; +const BROKEN = `function App() { return

{price}

; } +ReactDOM.createRoot(document.getElementById('root')).render();`; +const FIXED = BROKEN.replace('{price}', 'Pricing'); +const input: GenerateInput = { + prompt: 'Build a pricing page', + history: [], + model: { provider: 'openai', modelId: 'gpt-5' }, + apiKey: 'synthetic-only', + systemPrompt: 'Use the design tools, then summarize.', +}; + +type Call = Extract; +const call = (name: string, args: Record, id = name): Call => ({ + type: 'toolCall', + name, + arguments: args, + id, +}); +const todos = () => call('set_todos', { items: [{ text: 'Complete design', checked: false }] }); +const create = (path: string, text: string) => + call( + 'str_replace_based_edit_tool', + { command: 'create', path, file_text: text }, + `create-${path}`, + ); +const preview = (id = 'preview') => call('preview', { path: 'App.jsx' }, id); +const done = (id = 'done') => call('done', { path: 'App.jsx' }, id); +const fix = (oldStr: string, newStr: string, id = 'fix') => + call( + 'str_replace_based_edit_tool', + { command: 'str_replace', path: 'App.jsx', old_str: oldStr, new_str: newStr }, + id, + ); + +function response(model: Parameters[0], content: AssistantMessage['content']) { + const stopReason: 'toolUse' | 'stop' = content.some((part) => part.type === 'toolCall') + ? 'toolUse' + : 'stop'; + const message: AssistantMessage = { + role: 'assistant', + api: model.api, + provider: model.provider, + model: model.id, + content, + timestamp: Date.now(), + stopReason, + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + }; + const stream = createAssistantMessageEventStream(); + stream.push({ type: 'done', reason: stopReason, message }); + return stream; +} + +/** What the model was shown before each of its turns, so a test can read the tool results it saw. */ +let requests: AgentMessage[][]; +function script(batches: Array) { + requests = []; + native.stream.mockImplementation((model, context) => { + requests.push([...context.messages]); + const batch = batches.shift(); + if (batch === undefined) throw new Error('Unexpected extra provider call'); + return response(model, typeof batch === 'string' ? [{ type: 'text', text: batch }] : batch); + }); +} + +/** The text of the tool results with this id that the model had seen by its n-th turn (0-based). */ +function toolResultText(turn: number, toolCallId: string): string { + const message = requests[turn]?.find( + (item) => item.role === 'toolResult' && item.toolCallId === toolCallId, + ); + if (!message || message.role !== 'toolResult') + throw new Error(`No ${toolCallId} result at turn ${turn}`); + return message.content.map((part) => (part.type === 'text' ? part.text : '')).join(''); +} + +const overflow: LayoutIssue = { + check: 'horizontal-overflow', + viewportWidth: 390, + selector: 'table#prices', + message: + 'table#prices reaches 800 px, past the 390 px viewport edge, so the page scrolls sideways.', + measured: 800, + limit: 390, +}; +const layoutWarning = { source: 'layout', message: overflow.message }; +const runtimeError = { + source: 'console.error', + message: 'ReferenceError: price is not defined', + lineno: 1, +}; +const clean: DoneRuntimeResult = { errors: [], warnings: [] }; + +function previewResult(overrides: Partial = {}): PreviewResult { + return { + ok: true, + consoleErrors: [], + assetErrors: [], + metrics: { nodes: 4, width: 1280, height: 800, loadMs: 1 }, + ...overrides, + }; +} + +let root: string; +let fs: TextEditorFsCallbacks; +let writes: string[]; +beforeEach(() => { + native.stream.mockReset(); + native.agents.length = 0; + root = mkdtempSync(join(tmpdir(), 'codesign-design-loop-')); + writes = []; + fs = { + view(path) { + if (!readdirSync(root).includes(path)) return null; + const content = readFileSync(join(root, path), 'utf8'); + return { content, numLines: content.split('\n').length }; + }, + listDir: () => readdirSync(root), + create(path, content) { + writeFileSync(join(root, path), content); + writes.push(path); + return { path }; + }, + strReplace(path, oldStr, newStr) { + const current = readFileSync(join(root, path), 'utf8'); + if (!current.includes(oldStr)) throw new Error('Missing exact edit target'); + writeFileSync(join(root, path), current.replace(oldStr, newStr)); + writes.push(path); + return { path }; + }, + insert() { + throw new Error('Unexpected insert'); + }, + }; +}); +afterEach(() => rmSync(root, { recursive: true, force: true })); + +/** Runs the production agent with stubbed browser executors, recording every `done` result. */ +function run(options: { verify: DoneRuntimeVerifier; runPreview?: RunPreviewFn }) { + const checks: DoneDetails[] = []; + const result = generateViaAgent( + { ...input, runPreview: options.runPreview ?? (async () => previewResult()) }, + { + fs, + runtimeVerify: options.verify, + onEvent: (event) => { + if (event.type === 'tool_execution_end' && event.toolName === 'done') { + checks.push(event.result.details as DoneDetails); + } + }, + }, + ); + return { result, checks }; +} + +describe('the design loop with the production agent and a scripted model', () => { + it('writes, previews, hears a runtime error from done, fixes it and finishes', async () => { + const verify = vi + .fn() + .mockResolvedValueOnce({ errors: [runtimeError], warnings: [] }) + .mockResolvedValueOnce(clean); + script([ + [todos()], + [create('DESIGN.md', DESIGN), create('App.jsx', BROKEN)], + [preview()], + [done('done-1')], + [fix('{price}', 'Pricing')], + [preview('preview-2')], + [done('done-2')], + 'Fixed the heading.', + ]); + + const { result, checks } = run({ verify }); + const finished = await result; + + expect(checks.map((check) => check.status)).toEqual(['has_errors', 'ok']); + expect(verify.mock.calls.map(([source]) => source.includes('Pricing'))).toEqual([false, true]); + expect(toolResultText(4, 'done-1')).toMatch( + /^has_errors\n- console\.error: ReferenceError: price is not defined \(line 1\)/, + ); + expect(readFileSync(join(root, 'App.jsx'), 'utf8')).toBe(FIXED); + expect(finished.resourceState?.lastDone?.status).toBe('ok'); + expect(finished.artifacts).toHaveLength(1); + }); + + describe('layout and accessibility advice', () => { + it('lists a layout warning without failing done, and tells the agent it may fix one without calling done again', async () => { + const verify = vi + .fn() + .mockResolvedValue({ errors: [], warnings: [layoutWarning] }); + script([ + [todos()], + [create('DESIGN.md', DESIGN), create('App.jsx', FIXED)], + [preview()], + [done()], + 'Done, with one layout note.', + ]); + + const { result, checks } = run({ verify }); + const finished = await result; + + expect(checks).toHaveLength(1); + expect(checks[0]).toMatchObject({ status: 'ok', errors: [], warnings: [layoutWarning] }); + const text = toolResultText(4, 'done'); + expect(text).toContain('Non-blocking warnings:'); + expect(text).toContain(`- layout: ${layoutWarning.message}`); + expect(text).toContain('advice, not errors'); + expect(text).not.toContain('STOP.'); + expect(finished.resourceState?.lastDone?.status).toBe('ok'); + }); + + it('lets the agent fix the warning and check it with preview, then stop without a second done', async () => { + const verify = vi + .fn() + .mockResolvedValue({ errors: [], warnings: [layoutWarning] }); + const runPreview = vi + .fn() + .mockResolvedValueOnce(previewResult({ layoutIssues: [overflow] })) + .mockResolvedValueOnce(previewResult()); + script([ + [todos()], + [create('DESIGN.md', DESIGN), create('App.jsx', FIXED)], + [preview('preview-1')], + [done()], + [fix('Pricing', 'Plans')], + [preview('preview-2')], + 'Fixed the layout note.', + ]); + + const { result, checks } = run({ verify, runPreview }); + await result; + + expect(checks).toHaveLength(1); + expect(verify).toHaveBeenCalledTimes(1); + expect(runPreview).toHaveBeenCalledTimes(2); + expect(readFileSync(join(root, 'App.jsx'), 'utf8')).toContain('Plans'); + }); + + it('shows the model the layout issues of a preview, which stays ok', async () => { + const runPreview = vi + .fn() + .mockResolvedValue(previewResult({ layoutIssues: [overflow] })); + script([ + [todos()], + [create('DESIGN.md', DESIGN), create('App.jsx', FIXED)], + [preview()], + [done()], + 'Done.', + ]); + + const { result } = run({ verify: async () => clean, runPreview }); + await result; + + const text = toolResultText(3, 'preview'); + expect(text).toContain( + 'preview ok: 4 nodes, 0 console errors, 0 asset errors, 1 layout issue', + ); + expect(text).toContain('"layoutIssues"'); + expect(text).toContain('table#prices'); + }); + + it('reports layout warnings next to the runtime errors that fail done', async () => { + const verify = vi + .fn() + .mockResolvedValueOnce({ errors: [runtimeError], warnings: [layoutWarning] }) + .mockResolvedValueOnce(clean); + script([ + [todos()], + [create('DESIGN.md', DESIGN), create('App.jsx', BROKEN)], + [preview()], + [done('done-1')], + [fix('{price}', 'Pricing')], + [done('done-2')], + 'Fixed.', + ]); + + const { result, checks } = run({ verify }); + await result; + + expect(checks.map((check) => check.status)).toEqual(['has_errors', 'ok']); + expect(checks[0]?.warnings).toEqual([layoutWarning]); + const text = toolResultText(4, 'done-1'); + expect(text.startsWith('has_errors')).toBe(true); + expect(text).toContain('Non-blocking warnings:'); + }); + + it('does not count warnings as repair rounds, however often done is called', async () => { + const verify = vi + .fn() + .mockResolvedValue({ errors: [], warnings: [layoutWarning] }); + script([ + [todos()], + [create('DESIGN.md', DESIGN), create('App.jsx', FIXED)], + [preview()], + [done('done-1')], + [done('done-2')], + [done('done-3')], + [done('done-4')], + [done('done-5')], + 'Done.', + ]); + + const { result, checks } = run({ verify }); + const finished = await result; + + expect(checks.map((check) => check.status)).toEqual(['ok', 'ok', 'ok', 'ok', 'ok']); + expect(finished.resourceState?.lastDone?.status).toBe('ok'); + }); + + it('still stops after three done error rounds, whatever the warnings say', async () => { + const verify = vi + .fn() + .mockResolvedValue({ errors: [runtimeError], warnings: [layoutWarning] }); + script([ + [todos()], + [create('DESIGN.md', DESIGN), create('App.jsx', BROKEN)], + [preview()], + [done('done-1')], + [done('done-2')], + [done('done-3')], + ]); + + const { result, checks } = run({ verify }); + + await expect(result).rejects.toMatchObject({ code: 'GENERATION_INCOMPLETE' }); + expect(checks.map((check) => check.status)).toEqual([ + 'has_errors', + 'has_errors', + 'has_errors', + ]); + expect(readFileSync(join(root, 'App.jsx'), 'utf8')).toBe(BROKEN); + }); + + it('treats a layout measurement that did not run as a warning, not a failure', async () => { + const notRun = { + source: 'layout', + message: 'Layout checks did not run: Execution context was destroyed', + }; + const verify = vi + .fn() + .mockResolvedValue({ errors: [], warnings: [notRun] }); + script([ + [todos()], + [create('DESIGN.md', DESIGN), create('App.jsx', FIXED)], + [preview()], + [done()], + 'Done.', + ]); + + const { result, checks } = run({ verify }); + const finished = await result; + + expect(checks[0]?.status).toBe('ok'); + expect(toolResultText(4, 'done')).toContain(`- layout: ${notRun.message}`); + expect(finished.resourceState?.lastDone?.status).toBe('ok'); + }); + }); +}); diff --git a/packages/core/src/agent.native-lifecycle.test.ts b/packages/core/src/agent.native-lifecycle.test.ts index 5790e52be..b89635ef1 100644 --- a/packages/core/src/agent.native-lifecycle.test.ts +++ b/packages/core/src/agent.native-lifecycle.test.ts @@ -222,7 +222,7 @@ describe('production generateViaAgent with installed pi', () => { async (source: string, context?: { path: string; signal?: AbortSignal }) => { expect(context?.signal).toBeInstanceOf(AbortSignal); expect(source).toBe(readFileSync(join(root, 'App.jsx'), 'utf8')); - return []; + return { errors: [], warnings: [] }; }, ); const checks: DoneDetails[] = []; @@ -438,7 +438,10 @@ describe('production generateViaAgent with installed pi', () => { const controller = new AbortController(); const active = new ActiveRunMessages('design', 'run', () => {}, controller.signal); const ended: AgentMessage[][] = []; - const verify = vi.fn(async () => [{ message: 'Broken runtime', source: 'runtime' }]); + const verify = vi.fn(async () => ({ + errors: [{ message: 'Broken runtime', source: 'runtime' }], + warnings: [], + })); script([ [todos()], ...Array.from({ length: 5 }, (_, n) => [ @@ -637,7 +640,10 @@ describe('production generateViaAgent with installed pi', () => { it('leaves repeated done calls to the done repair limit', async () => { seed(); - const verify = vi.fn(async () => [{ message: 'Broken runtime', source: 'runtime' }]); + const verify = vi.fn(async () => ({ + errors: [{ message: 'Broken runtime', source: 'runtime' }], + warnings: [], + })); script([[todos()], ...Array.from({ length: 5 }, () => [done()]), 'Must not complete']); await expect(generateViaAgent(input, { fs, runtimeVerify: verify })).rejects.toMatchObject({ code: 'GENERATION_INCOMPLETE', diff --git a/packages/core/src/agent.test.ts b/packages/core/src/agent.test.ts index 4628ddf3b..5ccc26e1d 100644 --- a/packages/core/src/agent.test.ts +++ b/packages/core/src/agent.test.ts @@ -1228,8 +1228,8 @@ describe('generateViaAgent()', () => { }; const runtimeVerify = vi .fn() - .mockResolvedValueOnce([{ message: 'Temporary render error' }]) - .mockResolvedValueOnce([]); + .mockResolvedValueOnce({ errors: [{ message: 'Temporary render error' }], warnings: [] }) + .mockResolvedValueOnce({ errors: [], warnings: [] }); const result = await generateViaAgent( { prompt: 'design a meditation app', diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 3eeeee862..459b6a70d 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -119,7 +119,9 @@ export { export { type DoneDetails, type DoneError, + type DoneRuntimeResult, type DoneRuntimeVerifier, + LAYOUT_WARNING_SOURCE, makeDoneTool, } from './tools/done.js'; export { @@ -137,6 +139,8 @@ export { type WorkspaceInspection, } from './tools/inspect-workspace.js'; export { + type LayoutIssue, + MAX_LAYOUT_ISSUES, makePreviewTool, type PreviewInput, type PreviewResult, diff --git a/packages/core/src/tools/done.test.ts b/packages/core/src/tools/done.test.ts index e790d3b8f..1acf6da52 100644 --- a/packages/core/src/tools/done.test.ts +++ b/packages/core/src/tools/done.test.ts @@ -72,7 +72,7 @@ components: `, ), }); - const runtime = vi.fn(async () => []); + const runtime = vi.fn(async () => ({ errors: [], warnings: [] })); const result = await makeDoneTool(fs, runtime).execute('extensions', { path }); expect(result.details.status).toBe('ok'); expect(result.details.errors).toEqual([]); @@ -88,12 +88,84 @@ components: } expect(result.content).toEqual([ expect.objectContaining({ - text: expect.stringContaining('Non-blocking design metadata warnings'), + text: expect.stringContaining('Non-blocking warnings'), }), ]); expect(runtime).toHaveBeenCalledTimes(path === 'App.jsx' ? 1 : 0); }); + describe('runtime warnings', () => { + const layoutWarning = { + source: 'layout', + message: 'The page is 132 px wider than a 390 px viewport (body > main.hero)', + }; + + it('lists layout warnings without failing the run, and lets the agent fix one without calling done again', async () => { + const fs = makeFs({ 'App.jsx': VALID_TASK_APP, 'DESIGN.md': VALID_DESIGN_MD }); + const runtime = vi.fn(async () => ({ errors: [], warnings: [layoutWarning] })); + + const result = await makeDoneTool(fs, runtime).execute('layout', { path: 'App.jsx' }); + + expect(result.details.status).toBe('ok'); + expect(result.details.errors).toEqual([]); + expect(result.details.warnings).toEqual([layoutWarning]); + const text = result.content[0]?.type === 'text' ? result.content[0].text : ''; + expect(text).toContain('Non-blocking warnings:'); + expect(text).toContain(`- layout: ${layoutWarning.message}`); + expect(text).toContain('advice, not errors'); + expect(text).toContain('do not call `done` again'); + expect(text).not.toContain('STOP.'); + }); + + it('keeps the stop instruction when no layout warning is present', async () => { + const fs = makeFs({ + 'App.jsx': VALID_TASK_APP, + 'DESIGN.md': VALID_DESIGN_MD.replace( + '\n---\n', + '\ncomponents:\n card:\n borderRadius: 16px\n---\n', + ), + }); + const runtime = vi.fn(async () => ({ errors: [], warnings: [] })); + + const result = await makeDoneTool(fs, runtime).execute('metadata', { path: 'App.jsx' }); + + const text = result.content[0]?.type === 'text' ? result.content[0].text : ''; + expect(result.details.status).toBe('ok'); + expect(text).toContain('STOP.'); + expect(text).not.toContain('advice, not errors'); + }); + + it('reports layout warnings next to runtime errors', async () => { + const fs = makeFs({ 'App.jsx': VALID_TASK_APP, 'DESIGN.md': VALID_DESIGN_MD }); + const error = { source: 'runtime', message: 'ReferenceError: missingTask is not defined' }; + const runtime = vi.fn(async () => ({ errors: [error], warnings: [layoutWarning] })); + + const result = await makeDoneTool(fs, runtime).execute('both', { path: 'App.jsx' }); + + expect(result.details.status).toBe('has_errors'); + expect(result.details.errors).toEqual([error]); + expect(result.details.warnings).toEqual([layoutWarning]); + const text = result.content[0]?.type === 'text' ? result.content[0].text : ''; + expect(text.startsWith('has_errors')).toBe(true); + expect(text).toContain(`- layout: ${layoutWarning.message}`); + }); + + it('does not report warnings when the verifier failed to run', async () => { + const fs = makeFs({ 'App.jsx': VALID_TASK_APP, 'DESIGN.md': VALID_DESIGN_MD }); + const runtime = vi.fn(async () => { + throw new Error('Chrome is not installed'); + }); + + const result = await makeDoneTool(fs, runtime).execute('failed', { path: 'App.jsx' }); + + expect(result.details.status).toBe('has_errors'); + expect(result.details.errors).toEqual([ + { source: 'runtime', message: 'Runtime verifier failed: Chrome is not installed' }, + ]); + expect(result.details.warnings).toBeUndefined(); + }); + }); + it.each([ 'App.jsx', 'DESIGN.md', @@ -105,7 +177,10 @@ components: .replace('sm: 4px', 'sm: .5rem') .replace('spacing:', 'shadows:\n card: "0 2px 8px #000000"\nspacing:'), }); - const result = await makeDoneTool(fs, async () => []).execute('top-level-extensions', { path }); + const result = await makeDoneTool(fs, async () => ({ errors: [], warnings: [] })).execute( + 'top-level-extensions', + { path }, + ); expect(result.details.status).toBe('ok'); expect(result.details.errors).toEqual([]); expect(result.details.warnings).toEqual([ @@ -125,9 +200,10 @@ components: '\nshadows:\n card: "0 2px 8px #000000"\n---\n', ), }); - const runtime = vi.fn(async () => [ - { source: 'runtime', message: 'ReferenceError: missingTask is not defined' }, - ]); + const runtime = vi.fn(async () => ({ + errors: [{ source: 'runtime', message: 'ReferenceError: missingTask is not defined' }], + warnings: [], + })); const result = await makeDoneTool(fs, runtime).execute('runtime', { path: 'App.jsx' }); expect(result.details.status).toBe('has_errors'); expect(result.details.errors).toEqual([ @@ -298,13 +374,16 @@ ReactDOM.createRoot(document.getElementById('root')).render();`, const fs = makeFs({ 'index.html': '

Hi

', }); - const runtimeVerify = vi.fn(async () => [ - { - message: 'ReferenceError: TWEAK_DEFAULT is not defined', - source: 'console.error', - lineno: 12, - }, - ]); + const runtimeVerify = vi.fn(async () => ({ + errors: [ + { + message: 'ReferenceError: TWEAK_DEFAULT is not defined', + source: 'console.error', + lineno: 12, + }, + ], + warnings: [], + })); const tool = makeDoneTool(fs, runtimeVerify); const res = await tool.execute('id5', { summary: 'shipped' }); expect(runtimeVerify).toHaveBeenCalledOnce(); @@ -321,7 +400,7 @@ ReactDOM.createRoot(document.getElementById('root')).render();`, const fs = makeFs({ 'index.html': '

Hi

', }); - const runtimeVerify = vi.fn(async () => []); + const runtimeVerify = vi.fn(async () => ({ errors: [], warnings: [] })); const tool = makeDoneTool(fs, runtimeVerify); const res = await tool.execute('id6', {}); expect(res.details.status).toBe('ok'); @@ -333,12 +412,15 @@ ReactDOM.createRoot(document.getElementById('root')).render();`, const fs = makeFs({ 'design-brief.md': '# Design brief\n\nA concise handoff document.', }); - const runtimeVerify = vi.fn(async () => [ - { - message: 'This verifier should only run for renderable sources.', - source: 'runtime', - }, - ]); + const runtimeVerify = vi.fn(async () => ({ + errors: [ + { + message: 'This verifier should only run for renderable sources.', + source: 'runtime', + }, + ], + warnings: [], + })); const tool = makeDoneTool(fs, runtimeVerify, { requireDesignMd: true }); const res = await tool.execute('id-markdown-done', { path: 'design-brief.md' }); expect(runtimeVerify).not.toHaveBeenCalled(); diff --git a/packages/core/src/tools/done.ts b/packages/core/src/tools/done.ts index 7e838f0d0..c4867b6a7 100644 --- a/packages/core/src/tools/done.ts +++ b/packages/core/src/tools/done.ts @@ -26,6 +26,27 @@ const DoneParams = Type.Object({ const DESIGN_MD_ENTRY = 'DESIGN.md'; +/** `source` of the advisory warnings the host's layout checks report. */ +export const LAYOUT_WARNING_SOURCE = 'layout'; + +const STOP_LINES = [ + 'STOP. The design is verified. Your only remaining action is a short', + '2–3 sentence natural-language summary of the design decisions — no', + 'code, no fenced blocks, no `` tags, no file re-emission.', + 'Do NOT call `done` again. Do NOT call any other tool. The host', + 'extracts the artifact from the virtual filesystem automatically;', + "anything else you emit is wasted tokens and pollutes the user's chat.", +]; + +/** Layout warnings are advice, so the agent may fix one it agrees with, but not by looping on `done`. */ +const LAYOUT_ADVICE_LINES = [ + 'The design is verified. The layout and accessibility warnings below are advice, not errors.', + 'If you agree one is a real defect, fix it with one edit and check it with', + '`preview`; do not call `done` again. Otherwise your only remaining action', + 'is a short 2–3 sentence natural-language summary of the design decisions —', + 'no code, no fenced blocks, no `` tags, no file re-emission.', +]; + export interface DoneError { message: string; source?: string; @@ -117,7 +138,7 @@ function designMdWorkspaceFindings( function formatWarnings(warnings: DoneError[]): string { return warnings.length === 0 ? '' - : `\n\nNon-blocking design metadata warnings:\n${warnings + : `\n\nNon-blocking warnings:\n${warnings .map((warning) => `- ${warning.source}: ${warning.message}`) .join('\n')}`; } @@ -134,13 +155,20 @@ function requiredDesignMdErrors(fs: TextEditorFsCallbacks, activePath: string): ]; } +/** What the host observed when it ran the artifact: console and load `errors` + * that make `done` fail, and advisory `warnings` that never do. */ +export interface DoneRuntimeResult { + errors: DoneError[]; + warnings: DoneError[]; +} + /** Host-injected runtime verifier. Receives the raw artifact source (the - * agent's JSX module, NOT a fully-built srcdoc) and returns any console / - * load errors observed when the host actually executed it. */ + * agent's JSX module, NOT a fully-built srcdoc) and returns what the host + * observed when it actually executed it. */ export type DoneRuntimeVerifier = ( artifactSource: string, context?: { path: string; signal?: AbortSignal }, -) => Promise; +) => Promise; const VOID_ELEMENTS = new Set([ 'area', @@ -528,14 +556,16 @@ export function makeDoneTool( ...(opts.requireDesignMd ? requiredDesignMdErrors(fs, path) : []), ...designFindings.errors, ]; + const warnings: DoneError[] = [...designFindings.warnings]; if (runtimeVerify && isRenderableDesignSourcePath(path)) { try { - const runtimeErrors = await runtimeVerify(file.content, { + const runtime = await runtimeVerify(file.content, { path, ...(signal ? { signal } : {}), }); signal?.throwIfAborted(); - errors.push(...runtimeErrors); + errors.push(...runtime.errors); + warnings.push(...runtime.warnings); } catch (err) { signal?.throwIfAborted(); errors.push({ @@ -549,9 +579,10 @@ export function makeDoneTool( status, path, errors, - ...(designFindings.warnings.length > 0 ? { warnings: designFindings.warnings } : {}), + ...(warnings.length > 0 ? { warnings } : {}), ...(params.summary !== undefined ? { summary: params.summary } : {}), }; + const hasLayoutAdvice = warnings.some((warning) => warning.source === LAYOUT_WARNING_SOURCE); const text = status === 'ok' ? [ @@ -559,16 +590,11 @@ export function makeDoneTool( ? 'ok — no syntactic or runtime issues detected.' : 'ok — no syntactic issues detected. (Runtime verification not configured in this host.)', '', - 'STOP. The design is verified. Your only remaining action is a short', - '2–3 sentence natural-language summary of the design decisions — no', - 'code, no fenced blocks, no `` tags, no file re-emission.', - 'Do NOT call `done` again. Do NOT call any other tool. The host', - 'extracts the artifact from the virtual filesystem automatically;', - "anything else you emit is wasted tokens and pollutes the user's chat.", + ...(hasLayoutAdvice ? LAYOUT_ADVICE_LINES : STOP_LINES), ].join('\n') : `has_errors\n${errors.map((e) => `- ${e.source ? `${e.source}: ` : ''}${e.message}${e.lineno ? ` (line ${e.lineno})` : ''}`).join('\n')}`; return { - content: [{ type: 'text', text: text + formatWarnings(designFindings.warnings) }], + content: [{ type: 'text', text: text + formatWarnings(warnings) }], details, }; }, diff --git a/packages/core/src/tools/preview.test.ts b/packages/core/src/tools/preview.test.ts index 9eaab3264..20df600eb 100644 --- a/packages/core/src/tools/preview.test.ts +++ b/packages/core/src/tools/preview.test.ts @@ -7,9 +7,12 @@ import { import { CodesignError } from '@open-codesign/shared'; import { describe, expect, it, vi } from 'vitest'; import { + type LayoutIssue, MAX_ASSET_ERRORS, MAX_CONSOLE_ENTRIES, + MAX_LAYOUT_ISSUES, MAX_PREVIEW_STEPS, + MAX_PREVIEW_WARNINGS, makePreviewTool, type PreviewInput, type PreviewResult, @@ -347,6 +350,79 @@ describe('makePreviewTool', () => { expect(JSON.stringify(result.content)).toContain('isolated browser terminated'); }); + describe('layout issues', () => { + const issue = (index: number): LayoutIssue => ({ + check: 'horizontal-overflow', + viewportWidth: 390, + selector: `body > div:nth-of-type(${index})`, + message: `Element ${index} extends past the right edge`, + measured: 420, + limit: 390, + }); + + it('returns layout issues to the agent without failing the preview', async () => { + const runPreview = vi.fn().mockResolvedValue(cannedResult({ layoutIssues: [issue(1)] })); + + const result = await makePreviewTool(runPreview).execute('call', { path: 'App.jsx' }); + + expect(result.details.ok).toBe(true); + expect(result.details.layoutIssues).toEqual([issue(1)]); + expect(result.content[0]).toMatchObject({ text: expect.stringContaining('preview ok') }); + expect(JSON.stringify(result.content)).toContain('Element 1 extends past the right edge'); + }); + + it('counts layout issues in the summary line, and only when there are some', async () => { + const summaryOf = async (layoutIssues?: LayoutIssue[]) => { + const runPreview = vi + .fn() + .mockResolvedValue(cannedResult(layoutIssues ? { layoutIssues } : {})); + const result = await makePreviewTool(runPreview).execute('call', { path: 'App.jsx' }); + const first = result.content[0]; + return first?.type === 'text' ? first.text : ''; + }; + + expect(await summaryOf([issue(1)])).toBe( + 'preview ok: 42 nodes, 0 console errors, 0 asset errors, 1 layout issue', + ); + expect(await summaryOf([issue(1), issue(2)])).toBe( + 'preview ok: 42 nodes, 0 console errors, 0 asset errors, 2 layout issues', + ); + expect(await summaryOf()).toBe('preview ok: 42 nodes, 0 console errors, 0 asset errors'); + }); + + it('caps layout issues to the documented budget', async () => { + const issues = Array.from({ length: MAX_LAYOUT_ISSUES + 10 }, (_, index) => issue(index)); + const runPreview = vi.fn().mockResolvedValue(cannedResult({ layoutIssues: issues })); + + const result = await makePreviewTool(runPreview).execute('call', { path: 'App.jsx' }); + + expect(MAX_LAYOUT_ISSUES).toBe(20); + expect(result.details.layoutIssues).toEqual(issues.slice(0, MAX_LAYOUT_ISSUES)); + }); + + it('caps warnings to the documented budget', async () => { + const warnings = Array.from( + { length: MAX_PREVIEW_WARNINGS + 1 }, + (_, index) => `Note ${index}`, + ); + const runPreview = vi.fn().mockResolvedValue(cannedResult({ warnings })); + + const result = await makePreviewTool(runPreview).execute('call', { path: 'App.jsx' }); + + expect(MAX_PREVIEW_WARNINGS).toBe(5); + expect(result.details.warnings).toEqual(warnings.slice(0, MAX_PREVIEW_WARNINGS)); + }); + + it('leaves the field out when the host did not measure the layout', async () => { + const runPreview = vi.fn().mockResolvedValue(cannedResult()); + + const result = await makePreviewTool(runPreview).execute('call', { path: 'App.jsx' }); + + expect(result.details).not.toHaveProperty('layoutIssues'); + expect(result.content).toHaveLength(1); + }); + }); + it('propagates select steps and accepts empty and maximum-length option values', async () => { const input: PreviewInput = { path: 'App.jsx', diff --git a/packages/core/src/tools/preview.ts b/packages/core/src/tools/preview.ts index efa2e7c02..44fba6724 100644 --- a/packages/core/src/tools/preview.ts +++ b/packages/core/src/tools/preview.ts @@ -123,6 +123,31 @@ export const AssetError = Type.Object({ type: Type.Optional(Type.String()), }); +/** Warnings a preview result carries; the ones past it are cut. */ +export const MAX_PREVIEW_WARNINGS = 5; + +/** Layout and accessibility findings are advice: they never change `ok` or the `done` status. */ +export const MAX_LAYOUT_ISSUES = 20; + +export const LayoutIssue = Type.Object({ + check: Type.Union([ + Type.Literal('horizontal-overflow'), + Type.Literal('clipped-text'), + Type.Literal('low-contrast'), + Type.Literal('small-target'), + Type.Literal('missing-name'), + ]), + /** Width of the viewport the page was measured at. */ + viewportWidth: Type.Integer(), + /** A short CSS-style path to the element, for the agent to find it in the source. */ + selector: Type.String(), + message: Type.String(), + /** What was measured and the limit it was compared with (px, or a contrast ratio), when the check is a number. */ + measured: Type.Optional(Type.Number()), + limit: Type.Optional(Type.Number()), +}); +export type LayoutIssue = Static; + export const PreviewResult = Type.Object({ ok: Type.Boolean(), /** Set only when capabilities.vision === true. */ @@ -138,7 +163,8 @@ export const PreviewResult = Type.Object({ loadMs: Type.Number(), }), reason: Type.Optional(Type.String()), - warnings: Type.Optional(Type.Array(Type.String(), { maxItems: 5 })), + warnings: Type.Optional(Type.Array(Type.String(), { maxItems: MAX_PREVIEW_WARNINGS })), + layoutIssues: Type.Optional(Type.Array(LayoutIssue, { maxItems: MAX_LAYOUT_ISSUES })), visibleText: Type.Optional(Type.String()), steps: Type.Optional( Type.Array( @@ -163,6 +189,12 @@ export function trimPreviewResult(result: PreviewResult): PreviewResult { ...result, consoleErrors: result.consoleErrors.slice(0, MAX_CONSOLE_ENTRIES), assetErrors: result.assetErrors.slice(0, MAX_ASSET_ERRORS), + ...(result.warnings !== undefined + ? { warnings: result.warnings.slice(0, MAX_PREVIEW_WARNINGS) } + : {}), + ...(result.layoutIssues !== undefined + ? { layoutIssues: result.layoutIssues.slice(0, MAX_LAYOUT_ISSUES) } + : {}), }; } @@ -170,6 +202,7 @@ function previewContent(summary: string, result: PreviewResult): Array = [{ type: 'text', text: summary }]; if ( result.warnings !== undefined || + result.layoutIssues !== undefined || result.steps !== undefined || result.domOutline !== undefined || result.visibleText !== undefined @@ -179,6 +212,7 @@ function previewContent(summary: string, result: PreviewResult): Array 0 + ? `, ${layoutCount} layout ${layoutCount === 1 ? 'issue' : 'issues'}` + : ''; const summary = result.ok - ? `preview ok: ${result.metrics.nodes} nodes, ${result.consoleErrors.length} console errors, ${result.assetErrors.length} asset errors` + ? `preview ok: ${result.metrics.nodes} nodes, ${result.consoleErrors.length} console errors, ${result.assetErrors.length} asset errors${layout}` : `preview failed${result.reason ? `: ${result.reason}` : ''} (${result.consoleErrors.length} console errors, ${result.assetErrors.length} asset errors)`; return { content: previewContent(summary, result),