From d435fc53a79f10258b2fe86e1d51d0833567ae0f Mon Sep 17 00:00:00 2001 From: Fini Date: Sun, 10 May 2026 21:37:11 +0800 Subject: [PATCH] Merge branch 'v0.8.0' of github.com:ZSeven-W/openpencil into v0.8.0 --- .../orchestrator-plan-classify.test.ts | 81 +++++ .../services/ai/orchestrator-plan-classify.ts | 43 ++- apps/web/src/services/ai/orchestrator.ts | 10 +- bun.lock | 1 + packages/agent-native | 2 +- packages/pen-ai-skills/package.json | 1 + .../skills/phases/generation/layout.md | 7 + .../src/__tests__/detectors-spacing.test.ts | 87 ++++- .../__tests__/detectors-typography.test.ts | 297 ++++++++++++++++++ .../src/diagnostics/color-utils.ts | 64 ++++ .../src/diagnostics/detectors-spacing.ts | 88 ++++++ .../src/diagnostics/detectors-typography.ts | 213 +++++++++++++ .../src/diagnostics/detectors.ts | 70 +---- .../pen-ai-skills/src/diagnostics/index.ts | 4 +- .../pen-ai-skills/src/diagnostics/types.ts | 4 +- packages/pen-mcp/src/routes/debug-routes.ts | 2 + scripts/ab-corpus/check-stacked-padding.ts | 110 +++++++ scripts/ab-corpus/inspect-contrast-hits.ts | 56 ++++ scripts/ab-corpus/inspect-issue-category.ts | 59 ++++ scripts/ab-corpus/inspect-shape.ts | 76 +++++ scripts/ab-corpus/replay-detectors.ts | 113 +++++++ 21 files changed, 1317 insertions(+), 71 deletions(-) create mode 100644 apps/web/src/services/ai/__tests__/orchestrator-plan-classify.test.ts create mode 100644 packages/pen-ai-skills/src/__tests__/detectors-typography.test.ts create mode 100644 packages/pen-ai-skills/src/diagnostics/color-utils.ts create mode 100644 packages/pen-ai-skills/src/diagnostics/detectors-typography.ts create mode 100644 scripts/ab-corpus/check-stacked-padding.ts create mode 100644 scripts/ab-corpus/inspect-contrast-hits.ts create mode 100644 scripts/ab-corpus/inspect-issue-category.ts create mode 100644 scripts/ab-corpus/inspect-shape.ts create mode 100644 scripts/ab-corpus/replay-detectors.ts diff --git a/apps/web/src/services/ai/__tests__/orchestrator-plan-classify.test.ts b/apps/web/src/services/ai/__tests__/orchestrator-plan-classify.test.ts new file mode 100644 index 000000000..91a371936 --- /dev/null +++ b/apps/web/src/services/ai/__tests__/orchestrator-plan-classify.test.ts @@ -0,0 +1,81 @@ +import { describe, it, expect } from 'vitest'; +import { isMobileFullScreen } from '../orchestrator-plan-classify'; +import type { OrchestratorPlan } from '../ai-types'; + +const plan = (width: number, height: number, subtaskCount: number): OrchestratorPlan => + ({ + rootFrame: { id: 'page', name: 'Page', width, height, layout: 'vertical' }, + subtasks: Array.from({ length: subtaskCount }, (_, i) => ({ + id: `s-${i}`, + label: `Section ${i}`, + elements: [], + region: { width, height: 100 }, + idPrefix: '', + parentFrameId: null, + })), + }) as unknown as OrchestratorPlan; + +describe('isMobileFullScreen', () => { + it('true for narrow + tall (canonical mobile page)', () => { + expect(isMobileFullScreen(plan(375, 812, 5))).toBe(true); + }); + + it('false for wide root (desktop / landing page)', () => { + expect(isMobileFullScreen(plan(1200, 800, 4))).toBe(false); + }); + + it('false for narrow + tiny height + single subtask (Type 0 component)', () => { + expect(isMobileFullScreen(plan(375, 200, 1))).toBe(false); + }); + + // 2026-05-10 user report — DeepSeek "Bistro" food app generated with + // status bar missing. Plan came back with width=375, height=0 because + // the LLM emitted a non-numeric height that asNonNegativeNumber + // rejected → parser fell back to landing-page preset's rootHeight=0. + // Six structural subtasks (Header / Hero / Search / Categories / + // Specials / BottomNav) make it clearly a mobile page, not a Type 0 + // component. The subtask-count discriminator catches this. + + it('true for narrow + height=0 with 2+ subtasks (height lost in plan coercion)', () => { + expect(isMobileFullScreen(plan(375, 0, 6))).toBe(true); + }); + + it('true for narrow + height=0 with exactly 2 subtasks', () => { + expect(isMobileFullScreen(plan(375, 0, 2))).toBe(true); + }); + + it('false for narrow + height=0 with single subtask (canonical Type 0)', () => { + expect(isMobileFullScreen(plan(375, 0, 1))).toBe(false); + }); + + it('honors width boundary at 480 (exclusive)', () => { + expect(isMobileFullScreen(plan(480, 812, 5))).toBe(true); + expect(isMobileFullScreen(plan(481, 812, 5))).toBe(false); + }); + + // 2026-05-10 Codex stop-hook — orchestrator strips status-bar subtasks + // mid-pipeline, then sub-agents re-classify the mutated plan and would + // get a different answer (smaller subtask count). The memo pins the + // first-call result so all downstream consumers agree. + + it('memoizes per plan — subtask mutation does not flip classification', () => { + const p = plan(375, 0, 2); // narrow + height-0 + 2 subtasks → mobile + expect(isMobileFullScreen(p)).toBe(true); + // Simulate the orchestrator stripping status-bar subtask: + p.subtasks.pop(); + expect(p.subtasks.length).toBe(1); + // Second call sees post-strip plan but returns cached pre-strip result. + expect(isMobileFullScreen(p)).toBe(true); + }); + + it('different plan instances are classified independently (no cross-pollination)', () => { + const p1 = plan(375, 0, 2); // mobile + const p2 = plan(375, 0, 1); // Type 0 component + expect(isMobileFullScreen(p1)).toBe(true); + expect(isMobileFullScreen(p2)).toBe(false); + // Mutating p1 doesn't change p2's cached result. + p1.subtasks.pop(); + expect(isMobileFullScreen(p1)).toBe(true); + expect(isMobileFullScreen(p2)).toBe(false); + }); +}); diff --git a/apps/web/src/services/ai/orchestrator-plan-classify.ts b/apps/web/src/services/ai/orchestrator-plan-classify.ts index f478efc0c..6002a2533 100644 --- a/apps/web/src/services/ai/orchestrator-plan-classify.ts +++ b/apps/web/src/services/ai/orchestrator-plan-classify.ts @@ -1,5 +1,22 @@ import type { OrchestratorPlan } from './ai-types'; +/** + * Memoize the classification on a per-plan basis so call sites that hit + * the helper AFTER the orchestrator strips the status-bar subtask still + * see the original answer. The strip is a real mutation + * (`plan.subtasks = plan.subtasks.filter(...)`), so the second classify + * has a smaller subtask count than the first. Without this memo, a plan + * that came in as [status-bar, content] (2 items, height=0 from LLM + * emitting a non-numeric height) flips from "mobile" → "not-mobile" + * across the strip, and downstream consumers (sub-agent prompt + * builders, status-bar injection branches) disagree about chrome + * handling. Codex stop-hook 2026-05-10 caught this. + * + * WeakMap keeps the memo out of the plan's type surface and lets it + * vanish naturally when the plan goes out of scope. + */ +const memo = new WeakMap(); + /** * A plan represents a full mobile screen only when the root frame is narrow * AND tall. Narrow + auto-height (or small fixed height) is a Type 0 component @@ -14,6 +31,28 @@ import type { OrchestratorPlan } from './ai-types'; * 2026-05-09). */ export function isMobileFullScreen(plan: OrchestratorPlan): boolean { - if (plan.rootFrame.width > 480) return false; - return plan.rootFrame.height >= 480; + const cached = memo.get(plan); + if (cached !== undefined) return cached; + const result = computeIsMobileFullScreen(plan); + memo.set(plan, result); + return result; +} + +function computeIsMobileFullScreen(plan: OrchestratorPlan): boolean { + if (plan.rootFrame.width > 480) return false; + if (plan.rootFrame.height >= 480) return true; + // Width-narrow + height-zero/auto: distinguish a single Type 0 + // component from a mobile page whose declared height got lost in + // plan coercion. The LLM frequently emits a non-numeric height + // ("fit_content" / "auto") which asNonNegativeNumber rejects → the + // parser falls back to the landing-page preset's rootHeight=0 → + // we'd misclassify as Type 0 and skip status-bar injection. + // + // 2026-05-10 user reported a DeepSeek "Bistro" mobile design that + // arrived with a 375-wide root and missing iOS status bar; the live + // tree had 6+ section frames so it was structurally a multi-section + // mobile page, not a single card. A subtask-count discriminator + // catches this without re-relaxing the height check for genuine + // Type 0 components (badge / single card / modal — always 1 subtask). + return plan.subtasks.length >= 2; } diff --git a/apps/web/src/services/ai/orchestrator.ts b/apps/web/src/services/ai/orchestrator.ts index ea94acbbd..cb077df99 100644 --- a/apps/web/src/services/ai/orchestrator.ts +++ b/apps/web/src/services/ai/orchestrator.ts @@ -835,7 +835,15 @@ export async function executeOrchestration( useHistoryStore.getState().startBatch(useDocumentStore.getState().document); } - const isMobile = isMobileFullScreen(plan); + // Reuse the L742 classification — `isMobileScreen` was computed BEFORE + // the status-bar subtask got stripped from `plan.subtasks`. Re-running + // `isMobileFullScreen(plan)` here would see the post-strip count, and + // a plan that originally had [status-bar, content] (2 items, fits the + // narrow + multi-subtask fallback added 2026-05-10 for height-loss + // cases) would drop to 1 item and misclassify as Type 0 → no status + // bar injection. Codex stop-hook 2026-05-10 caught this; the in-place + // strip + re-classify pattern is the fragile bit. + const isMobile = isMobileScreen; const useDashboardColumns = shouldUseDashboardColumns(request.prompt, plan); const defaultFill: FrameNode['fill'] = (plan.rootFrame.fill as FrameNode['fill']) ?? [ { type: 'solid', color: plan.styleGuide?.palette?.background ?? '#FFFFFF' }, diff --git a/bun.lock b/bun.lock index 10a50b200..59cc48044 100644 --- a/bun.lock +++ b/bun.lock @@ -130,6 +130,7 @@ "name": "@zseven-w/pen-ai-skills", "version": "0.8.0", "dependencies": { + "@zseven-w/pen-core": "workspace:*", "@zseven-w/pen-types": "workspace:*", "gray-matter": "^4.0.3", "js-yaml": "^4.1.1", diff --git a/packages/agent-native b/packages/agent-native index e1f90cab9..22f20e42c 160000 --- a/packages/agent-native +++ b/packages/agent-native @@ -1 +1 @@ -Subproject commit e1f90cab9658e6c215b48bccfc3489412c8788a5 +Subproject commit 22f20e42cdfbfba7623693ce3f8b17d59fac8ea7 diff --git a/packages/pen-ai-skills/package.json b/packages/pen-ai-skills/package.json index d858a12c1..02df3d0a2 100644 --- a/packages/pen-ai-skills/package.json +++ b/packages/pen-ai-skills/package.json @@ -40,6 +40,7 @@ } }, "dependencies": { + "@zseven-w/pen-core": "workspace:*", "@zseven-w/pen-types": "workspace:*", "gray-matter": "^4.0.3", "js-yaml": "^4.1.1" diff --git a/packages/pen-ai-skills/skills/phases/generation/layout.md b/packages/pen-ai-skills/skills/phases/generation/layout.md index dce37d6c6..87643c8f7 100644 --- a/packages/pen-ai-skills/skills/phases/generation/layout.md +++ b/packages/pen-ai-skills/skills/phases/generation/layout.md @@ -102,3 +102,10 @@ AESTHETIC HYGIENE — keep these silent (never emit, the post-pass also strips t - INNER LAYOUT FRAMES (sections, wrappers, header / body containers inside a card) DO NOT need fill, stroke, OR shadow. They inherit from the page / card surface. Only opt into a fill / border / shadow on the OUTER card, button, badge, chip — NEVER on the wrapper that holds it. +- PAGE GUTTER GOES ON ONE LAYER, NOT BOTH. Pick: either the root frame carries horizontal padding + (e.g. `padding: [0,16]` on root) and direct child sections use horizontal padding 0, OR the root + carries 0 horizontal padding and each section sets its own. Stacking both produces a doubled + inset (root 16 + section 24 = 40px gutter on a 375px page → only 295px content). Default + convention: PUT HORIZONTAL PADDING ON THE ROOT, sections set vertical padding only. Hero / + banner / image-bleed sections then sit edge-to-edge by simply NOT adding horizontal padding (the + root's gutter shows through them naturally). diff --git a/packages/pen-ai-skills/src/__tests__/detectors-spacing.test.ts b/packages/pen-ai-skills/src/__tests__/detectors-spacing.test.ts index 081db2ab8..5a0260745 100644 --- a/packages/pen-ai-skills/src/__tests__/detectors-spacing.test.ts +++ b/packages/pen-ai-skills/src/__tests__/detectors-spacing.test.ts @@ -1,6 +1,9 @@ import { describe, it, expect } from 'vitest'; import type { PenNode } from '@zseven-w/pen-types'; -import { detectEdgeSectionPadding } from '../diagnostics/detectors-spacing'; +import { + detectEdgeSectionPadding, + detectStackedHorizontalPadding, +} from '../diagnostics/detectors-spacing'; // 2026-05-09 user report — image 6: "Categories" mobile section had its // chip text glued to the screen edge because both the page root and the @@ -241,3 +244,85 @@ describe('detectEdgeSectionPadding', () => { expect(issues[0].nodeId).toBe('page'); }); }); + +// 2026-05-10 user report — DeepSeek "Bistro" mobile food app shipped with +// root padding [0,16,0,16] AND a "Today's Specials" section padding [0,24], +// effective 40px gutter on a 375px page. Looked pinched / "too much +// padding". detectStackedHorizontalPadding catches the page-vs-section +// double-pad pattern (info-only — section can legitimately want larger +// inset for emphasis, so detect-only and let user/agent decide). + +describe('detectStackedHorizontalPadding', () => { + it('flags section whose horizontal padding stacks with mobile root', () => { + const root = mobileRoot( + [section('specials', [text('t1')], { padding: [0, 24] }), section('news', [text('t2')])], + [0, 16, 0, 16], + ); + const issues = detectStackedHorizontalPadding(root); + expect(issues).toHaveLength(1); + expect(issues[0].nodeId).toBe('specials'); + expect(issues[0].category).toBe('stacked-horizontal-padding'); + expect(issues[0].severity).toBe('info'); + expect(issues[0].reason).toMatch(/stacks with root/); + }); + + it('flags multiple offending sections in one root', () => { + const root = mobileRoot( + [ + section('a', [text('t1')], { padding: [0, 24] }), + section('b', [text('t2')], { padding: 16 }), + section('c', [text('t3')]), + ], + [0, 16, 0, 16], + ); + const issues = detectStackedHorizontalPadding(root); + expect(issues).toHaveLength(2); + expect(new Set(issues.map((i) => i.nodeId))).toEqual(new Set(['a', 'b'])); + }); + + it('does NOT flag when only root has horizontal padding (the goal state)', () => { + const root = mobileRoot( + [section('a', [text('t1')]), section('b', [text('t2')])], + [0, 16, 0, 16], + ); + expect(detectStackedHorizontalPadding(root)).toHaveLength(0); + }); + + it('does NOT flag when root has 0 horizontal padding (sections own gutter is the only one)', () => { + const root = mobileRoot([ + section('a', [text('t1')], { padding: [0, 24] }), + section('b', [text('t2')], { padding: [0, 24] }), + ]); + expect(detectStackedHorizontalPadding(root)).toHaveLength(0); + }); + + it('does NOT flag full-bleed roles (top-nav / hero / banner) even when their padding stacks', () => { + const root = mobileRoot( + [ + section('topnav', [text('Title')], { role: 'top-nav', padding: [0, 24] }), + section('hero', [text('h')], { role: 'hero', padding: [0, 24] }), + ], + [0, 16, 0, 16], + ); + expect(detectStackedHorizontalPadding(root)).toHaveLength(0); + }); + + it('does NOT flag desktop-shaped roots (component-internal padding stacking is legitimate)', () => { + // The detector is page-shape-only: a 1200px desktop card with a chip + // having internal padding is a totally normal nested-component pattern, + // not the page-gutter doubling we care about. + const root = { + id: 'page', + type: 'frame', + width: 1200, + height: 800, + layout: 'vertical', + padding: [0, 24, 0, 24], + children: [ + section('a', [text('t1')], { padding: [0, 16] }), + section('b', [text('t2')], { padding: [0, 16] }), + ], + } as unknown as PenNode; + expect(detectStackedHorizontalPadding(root)).toHaveLength(0); + }); +}); diff --git a/packages/pen-ai-skills/src/__tests__/detectors-typography.test.ts b/packages/pen-ai-skills/src/__tests__/detectors-typography.test.ts new file mode 100644 index 000000000..d4f638535 --- /dev/null +++ b/packages/pen-ai-skills/src/__tests__/detectors-typography.test.ts @@ -0,0 +1,297 @@ +import { describe, it, expect } from 'vitest'; +import type { PenNode, PenDocument } from '@zseven-w/pen-types'; +import { detectTextBgContrast } from '../diagnostics/detectors-typography'; + +// 2026-05-09 user reported black-card-with-white-text mixed into a cream +// page — the white-on-cream paths read as "muddy" and the AA check is the +// systematic way to flag it. Auto-fix is intentionally omitted; the right +// replacement depends on the design system + theme so this stays detect-only. + +const text = ( + id: string, + fill: unknown, + fontSize: number = 16, + fontWeight?: number | string, +): PenNode => + ({ + id, + type: 'text', + content: 'sample', + fontSize, + fontWeight, + fill, + }) as unknown as PenNode; + +const frame = (id: string, children: PenNode[], fill?: unknown): PenNode => + ({ + id, + type: 'frame', + layout: 'vertical', + fill, + children, + }) as unknown as PenNode; + +const solid = (color: string) => [{ type: 'solid' as const, color }]; + +const emptyDoc: PenDocument = { + children: [], + variables: undefined, + themes: undefined, +} as unknown as PenDocument; + +const docWithVars = (vars: Record, themes?: Record) => + ({ + children: [], + variables: vars, + themes, + }) as unknown as PenDocument; + +describe('detectTextBgContrast', () => { + it('does NOT flag black text on white background (ratio 21:1)', () => { + const root = frame('page', [text('t1', solid('#000000'))], solid('#FFFFFF')); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(0); + }); + + it('flags very-low-contrast gray on white (ratio ~2.1 < default 2.5)', () => { + // 2026-05-10 calibration — was #888888 (ratio 3.95) when threshold was + // WCAG AA 4.5; corpus replay showed the strict threshold flagging + // industry-standard caption patterns. Lowered to 2.5 normal / 2.0 large + // so only genuinely-broken contrast trips. #B0B0B0 is the lighter + // boundary still failing 2.5. + const root = frame('page', [text('t1', solid('#B0B0B0'))], solid('#FFFFFF')); + const issues = detectTextBgContrast(root, emptyDoc); + expect(issues).toHaveLength(1); + expect(issues[0].nodeId).toBe('t1'); + expect(issues[0].category).toBe('text-bg-contrast'); + expect(issues[0].severity).toBe('info'); + expect(issues[0].suggestedValue).toBeNull(); + expect(issues[0].reason).toMatch(/below 2\.5:1/); + }); + + it('does NOT flag Tailwind slate-400 captions on white (ratio ~2.56 — intentional tertiary text)', () => { + // The 2026-05-08 corpus replay was 43% noise because WCAG-AA strict + // flagged this pattern. New threshold tolerates it. + const root = frame('page', [text('t1', solid('#94A3B8'))], solid('#FFFFFF')); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(0); + }); + + it('does NOT flag Tailwind blue-600 chips on blue-100 (ratio ~4.24 — chip pattern)', () => { + const root = frame('page', [text('t1', solid('#2563EB'))], solid('#DBEAFE')); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(0); + }); + + it('flags white text on white bg (ratio 1.0 — invisible)', () => { + const root = frame('page', [text('t1', solid('#FFFFFF'))], solid('#FFFFFF')); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(1); + }); + + it('does NOT flag white text on dark cream-styled card (the user-reported good case)', () => { + // Cream page with a dark card; white text on the dark card should pass. + const root = frame( + 'page', + [frame('card', [text('t1', solid('#FFFFFF'))], solid('#1A1A1A'))], + solid('#FFF8E7'), // cream + ); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(0); + }); + + it('flags white text on cream page (the user-reported bad case)', () => { + // The same white text but with no dark card wrapping — sits directly + // on the cream page background; ratio ~1.10, fails AA hard. + const root = frame('page', [text('t1', solid('#FFFFFF'))], solid('#FFF8E7')); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(1); + }); + + it('uses the LARGE-text threshold (2.0) for fontSize >= 24', () => { + // #B0B0B0 on white = ratio ~2.13 — fails normal 2.5 but passes large 2.0 + const root = frame('page', [text('t1', solid('#B0B0B0'), 32)], solid('#FFFFFF')); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(0); + }); + + it('uses the LARGE-text threshold (2.0) for fontSize >= 19 + bold weight', () => { + const root = frame('page', [text('t1', solid('#B0B0B0'), 20, 700)], solid('#FFFFFF')); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(0); + }); + + it('still flags >=19px non-bold text (large rule needs 700+ weight)', () => { + // Non-bold large text uses the NORMAL threshold (2.5); 2.13 < 2.5 → flag. + const root = frame('page', [text('t1', solid('#B0B0B0'), 20, 400)], solid('#FFFFFF')); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(1); + }); + + it('honors caller-supplied opts.normalThreshold to enforce stricter audits', () => { + // 2.56:1 (slate-400) is silenced by default 2.5 but should re-fire when + // a stricter audit asks for WCAG-AA 4.5. + const root = frame('page', [text('t1', solid('#94A3B8'))], solid('#FFFFFF')); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(0); + expect(detectTextBgContrast(root, emptyDoc, { normalThreshold: 4.5 })).toHaveLength(1); + }); + + it('walks ancestor chain to find first non-transparent bg', () => { + // Outer page has cream; inner section has no fill (transparent); + // the text's effective bg should still resolve to cream. + const root = frame( + 'page', + [ + frame('section', [text('t1', solid('#FFF8E7'))]), // section has no fill + ], + solid('#FFF8E7'), + ); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(1); + }); + + it('defaults to white bg when no ancestor has any fill', () => { + // No page fill at all — detector falls back to the canvas default + // (white). White text on white = invisible. + const root = frame('page', [text('t1', solid('#FFFFFF'))]); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(1); + }); + + it('resolves $variable refs through doc.variables / theme', () => { + const doc = docWithVars({ + 'color-text': { type: 'color', value: '#B0B0B0' }, + 'color-bg': { type: 'color', value: '#FFFFFF' }, + }); + const root = frame('page', [text('t1', solid('$color-text'))], solid('$color-bg')); + const issues = detectTextBgContrast(root, doc); + expect(issues).toHaveLength(1); + expect(issues[0].reason).toMatch(/text=#B0B0B0 on bg=#FFFFFF/); + }); + + it('skips text whose color ref does not resolve (no false positive)', () => { + const doc = docWithVars({}); // no variables, ref will not resolve + const root = frame('page', [text('t1', solid('$nonexistent'))], solid('#FFFFFF')); + expect(detectTextBgContrast(root, doc)).toHaveLength(0); + }); + + it('skips text with no fill array (renderer default applies)', () => { + const root = frame('page', [text('t1', undefined)], solid('#FFFFFF')); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(0); + }); + + it('approximates gradient bg by first stop', () => { + const gradientBg = [ + { + type: 'linear_gradient' as const, + stops: [ + { color: '#FFFFFF', offset: 0 }, + { color: '#000000', offset: 1 }, + ], + }, + ]; + // First stop is white → light bg; white text fails. + const root = frame('page', [text('t1', solid('#FFFFFF'))], gradientBg); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(1); + }); + + // 2026-05-10 Codex stop-hook review caught: a wrapper with opacity=0 + // or #RRGGBBAA alpha=00 was being treated as the bg color, masking + // contrast failures whose real bg lives further up the ancestor chain. + + it('skips wrapper fill with opacity=0 and uses real bg further up', () => { + const transparentFill = [{ type: 'solid' as const, color: '#FFFFFF', opacity: 0 }]; + // Page is cream; transparent white wrapper between page and text. + // Without the fix the detector would think bg=#FFFFFF (the wrapper) + // and pass cream-text-on-white. With the fix it walks past the + // invisible wrapper and flags cream-text-on-cream. + const root = frame( + 'page', + [frame('wrap', [text('t1', solid('#FFF8E7'))], transparentFill)], + solid('#FFF8E7'), + ); + const issues = detectTextBgContrast(root, emptyDoc); + expect(issues).toHaveLength(1); + expect(issues[0].reason).toMatch(/bg=#FFF8E7/); + }); + + it('skips wrapper fill with 8-hex alpha 00 and uses real bg further up', () => { + const transparentFill = [{ type: 'solid' as const, color: '#FFFFFF00' }]; + const root = frame( + 'page', + [frame('wrap', [text('t1', solid('#FFF8E7'))], transparentFill)], + solid('#FFF8E7'), + ); + const issues = detectTextBgContrast(root, emptyDoc); + expect(issues).toHaveLength(1); + expect(issues[0].reason).toMatch(/bg=#FFF8E7/); + }); + + // 2026-05-10 Codex round 4 — align the walk prune with pen-core's + // canonical isNodeVisible (visible/enabled, NOT opacity). Renderer + // treats opacity as paint alpha; pruning on opacity=0 would diverge + // from what the renderer walks, so a contrast detector should match. + // opacity=0 wrappers still get bypassed at the bg-resolution layer + // because alpha-0 fills paint nothing visible. + + it('walks into opacity=0 subtree but uses real bg from behind the invisible wrapper', () => { + const wrap: PenNode = { + id: 'wrap', + type: 'frame', + layout: 'vertical', + fill: solid('#FFFFFF'), + opacity: 0, + children: [text('t1', solid('#FFF8E7'))], + } as unknown as PenNode; + const root = frame('page', [wrap], solid('#FFF8E7')); + const issues = detectTextBgContrast(root, emptyDoc); + expect(issues).toHaveLength(1); + expect(issues[0].reason).toMatch(/bg=#FFF8E7/); + }); + + it('does NOT flag text inside a visible=false subtree (canonical hidden)', () => { + const wrap: PenNode = { + id: 'wrap', + type: 'frame', + layout: 'vertical', + fill: solid('#FFFFFF'), + visible: false, + children: [text('t1', solid('#FFF8E7'))], + } as unknown as PenNode; + const root = frame('page', [wrap], solid('#FFF8E7')); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(0); + }); + + it('does NOT flag text inside an enabled=false subtree (canonical hidden)', () => { + const wrap: PenNode = { + id: 'wrap', + type: 'frame', + layout: 'vertical', + fill: solid('#FFFFFF'), + enabled: false, + children: [text('t1', solid('#FFF8E7'))], + } as unknown as PenNode; + const root = frame('page', [wrap], solid('#FFF8E7')); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(0); + }); + + it('still treats opacity=0.5 as visible (only opacity=0 is the alpha-0 sentinel)', () => { + // We don't try to math a 50% wash against the layer below; that is + // outside the detector's scope. The fill stays as the bg. + const halfFill = [{ type: 'solid' as const, color: '#000000', opacity: 0.5 }]; + const root = frame('page', [text('t1', solid('#FFFFFF'))], halfFill); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(0); + }); + + it('does NOT skip 8-hex with alpha 80 (semi-transparent stays opaque)', () => { + // #00000080 is 50% black — out of scope for the detector, but it is + // NOT the same as alpha=00. Treat it as an opaque-enough bg. + const semi = [{ type: 'solid' as const, color: '#00000080' }]; + const root = frame('page', [text('t1', solid('#FFFFFF'))], semi); + expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(0); + }); + + it('flags multiple offending texts in one pass', () => { + const root = frame( + 'page', + [ + text('good', solid('#000000')), + text('bad-1', solid('#FFFFFF')), + text('bad-2', solid('#EEEEEE')), + ], + solid('#FFFFFF'), + ); + const issues = detectTextBgContrast(root, emptyDoc); + expect(issues).toHaveLength(2); + expect(new Set(issues.map((i) => i.nodeId))).toEqual(new Set(['bad-1', 'bad-2'])); + }); +}); diff --git a/packages/pen-ai-skills/src/diagnostics/color-utils.ts b/packages/pen-ai-skills/src/diagnostics/color-utils.ts new file mode 100644 index 000000000..240dfb1b6 --- /dev/null +++ b/packages/pen-ai-skills/src/diagnostics/color-utils.ts @@ -0,0 +1,64 @@ +/** + * Color helpers shared across diagnostic detectors. + * + * Extracted from detectors.ts in 2026-05-10 when the second contrast-aware + * detector (detectTextBgContrast) needed the same WCAG luminance math the + * invisible-container detector already had inline. Kept narrow on purpose — + * the only export surface is what detectors actually call. + */ + +/** + * Parse `#rgb` / `#rrggbb` / `#rrggbbaa` to `{r, g, b}` (alpha is dropped). + * Returns null on parse failure or non-string input. + */ +export function parseHexColor(s: unknown): { r: number; g: number; b: number } | null { + if (typeof s !== 'string') return null; + const m = s.trim().match(/^#([0-9a-fA-F]{3,8})$/); + if (!m) return null; + let hex = m[1]; + if (hex.length === 3) { + hex = hex + .split('') + .map((c) => c + c) + .join(''); + } + if (hex.length !== 6 && hex.length !== 8) return null; + const r = parseInt(hex.slice(0, 2), 16); + const g = parseInt(hex.slice(2, 4), 16); + const b = parseInt(hex.slice(4, 6), 16); + if (Number.isNaN(r) || Number.isNaN(g) || Number.isNaN(b)) return null; + return { r, g, b }; +} + +/** WCAG 2.x relative luminance for sRGB. Returns 0.0–1.0. */ +export function relativeLuminance(c: { r: number; g: number; b: number }): number { + const lin = (v: number): number => { + const s = v / 255; + return s <= 0.03928 ? s / 12.92 : Math.pow((s + 0.055) / 1.055, 2.4); + }; + return 0.2126 * lin(c.r) + 0.7152 * lin(c.g) + 0.0722 * lin(c.b); +} + +/** + * Compare two color strings via WCAG relative-luminance contrast ratio. + * Returns 1.0 for identical colors, growing toward 21.0 as they diverge, + * or Infinity if either color cannot be parsed (e.g. unresolved variable + * refs — caller should resolve refs upstream). + * + * Why WCAG ratio rather than max RGB channel diff: the human eye is much + * more sensitive to small tonal differences on dark backgrounds than light + * ones (Weber–Fechner / dark adaptation). Channel-diff would give false + * positives in dark themes; ratio is luminance-based and matches the metric + * WCAG / Stark / Figma report. + */ +export function colorContrast(a: string, b: string): number { + if (a === b) return 1; + const pa = parseHexColor(a); + const pb = parseHexColor(b); + if (!pa || !pb) return Infinity; + const lumA = relativeLuminance(pa); + const lumB = relativeLuminance(pb); + const lighter = Math.max(lumA, lumB); + const darker = Math.min(lumA, lumB); + return (lighter + 0.05) / (darker + 0.05); +} diff --git a/packages/pen-ai-skills/src/diagnostics/detectors-spacing.ts b/packages/pen-ai-skills/src/diagnostics/detectors-spacing.ts index 148fe80bc..6433184e1 100644 --- a/packages/pen-ai-skills/src/diagnostics/detectors-spacing.ts +++ b/packages/pen-ai-skills/src/diagnostics/detectors-spacing.ts @@ -31,6 +31,17 @@ function getPaddingLeft(node: PenNode): number { return 0; } +function getPaddingRight(node: PenNode): number { + const p = (node as unknown as { padding?: unknown }).padding; + if (typeof p === 'number') return p; + if (Array.isArray(p)) { + if (p.length === 4) return Number(p[1] ?? 0); + if (p.length === 2) return Number(p[1] ?? 0); + if (p.length === 1) return Number(p[0] ?? 0); + } + return 0; +} + function hasTextOrIconDescendant(node: PenNode): boolean { if (node.type === 'text') return true; if ((node.type as string) === 'icon_font') return true; @@ -162,3 +173,80 @@ export function detectEdgeSectionPadding(root: PenNode): Issue[] { } } } + +/** + * Aesthetic detector: page root and a direct content section both apply + * horizontal padding, producing a doubled inset. + * + * 2026-05-10 user-reported "Bistro" mobile design hit this: root carried + * `padding: [0, 16, 0, 16]` and `Today's Specials` section carried + * `padding: [0, 24]`. Effective gutter = 16 + 24 = 40px on a 375px page, + * leaving only 295px for content. Read as "too much padding" / pinched. + * + * The skill prompt now teaches "page gutter goes on ONE layer, not both" + * (see layout.md AESTHETIC HYGIENE). This detector is the cheap + * post-pass guard for when the LLM ignores the rule. + * + * Severity is INFO (detect-only). Auto-fix is tempting (zero out section + * horizontal padding, keep root) but a section may legitimately want a + * larger inset for visual emphasis (a hero card with deeper gutter than + * surrounding sections). Surface in the audit panel; let the user/agent + * decide. The skill rule covers the preventive side. + * + * Page-shape filter mirrors detectEdgeSectionPadding to avoid firing on + * components-with-internal-padding (chip → status-badge stacking is + * legitimate; the doubled-gutter problem is unique to mobile pages). + */ +export function detectStackedHorizontalPadding(root: PenNode): Issue[] { + const issues: Issue[] = []; + walk(root); + return issues; + + function walk(node: PenNode): void { + const width = (node as unknown as { width?: unknown }).width; + const height = (node as unknown as { height?: unknown }).height; + const looksLikeMobilePage = + node.type === 'frame' && + typeof width === 'number' && + width >= 320 && + width <= 480 && + typeof height === 'number' && + height >= 568 && + height >= width * 1.5; + + if ( + looksLikeMobilePage && + 'children' in node && + Array.isArray(node.children) && + node.children.length >= 2 + ) { + const rootL = getPaddingLeft(node); + const rootR = getPaddingRight(node); + if (rootL > 0 || rootR > 0) { + for (const child of (node as { children: PenNode[] }).children) { + if (child.type !== 'frame') continue; + const role = ((child as { role?: string }).role ?? '').toLowerCase(); + if (FULL_BLEED_ROLES.has(role)) continue; + const childL = getPaddingLeft(child); + const childR = getPaddingRight(child); + if (childL === 0 && childR === 0) continue; + // Section's horizontal padding stacks with root's. Flag the section + // (the offender — root is the established gutter holder). + issues.push({ + nodeId: child.id, + category: 'stacked-horizontal-padding', + severity: 'info', + property: 'padding', + currentValue: (child as unknown as { padding?: unknown }).padding, + suggestedValue: null, + reason: `section h-padding [${childL}/${childR}] stacks with root h-padding [${rootL}/${rootR}] — combined gutter ${rootL + childL}/${rootR + childR}`, + }); + } + } + } + + if ('children' in node && Array.isArray(node.children)) { + for (const c of node.children) walk(c); + } + } +} diff --git a/packages/pen-ai-skills/src/diagnostics/detectors-typography.ts b/packages/pen-ai-skills/src/diagnostics/detectors-typography.ts new file mode 100644 index 000000000..5ae5dfe1e --- /dev/null +++ b/packages/pen-ai-skills/src/diagnostics/detectors-typography.ts @@ -0,0 +1,213 @@ +import type { PenNode, PenDocument } from '@zseven-w/pen-types'; +import { resolveColorRef, getDefaultTheme, isNodeVisible } from '@zseven-w/pen-core'; +import type { Issue } from './types'; +import { colorContrast } from './color-utils'; + +/** + * Default contrast thresholds — looser than WCAG 2.x AA on purpose. + * + * 2026-05-10 calibration against `2026-05-08-rank4-gpt55` corpus (104 real + * GPT-5.5 dashboard outputs, 95 applied successfully): + * + * - WCAG-AA strict (4.5/3.0) → 41 hits, ~43% of designs flagged. + * Most hits were industry-standard Tailwind palettes used as + * intentional tertiary text (slate-400 captions on white = 2.56:1, + * blue-600 chips on blue-100 = 4.24:1, emerald-500 deltas on white + * = 2.54:1). These pass design review at Linear / Vercel / Notion / + * GitHub and feel like false positives to the user. + * - 2.5/2.0 → expected ~5 hits. Catches the user's reported pain + * (white-on-cream = 1.10:1, white-on-white = 1.0:1, white-on-near- + * white) without flagging muted-caption patterns. + * + * This detector reports physical readability, not WCAG compliance. + * Callers can override via opts.normalThreshold / opts.largeThreshold + * if they want stricter audits without re-implementing the walk. + */ +const DEFAULT_NORMAL_THRESHOLD = 2.5; +const DEFAULT_LARGE_THRESHOLD = 2.0; + +export interface DetectTextBgContrastOptions { + /** Contrast ratio below which normal-size text is flagged. Default 2.5. */ + normalThreshold?: number; + /** Contrast ratio below which large text (>=24px or >=19px bold) is flagged. Default 2.0. */ + largeThreshold?: number; +} + +/** + * Pull the first solid color out of a fill array. Gradients get reduced to + * their first stop as a coarse approximation — text on a gradient bg is + * already a hard contrast call, and reading the gradient midpoint is + * outside this detector's scope. Returns null when there's no usable + * solid color (image fills, missing fills, transparent overlays). + * + * Effectively-transparent fills are skipped so the ancestor walk + * continues past invisible wrappers to the real visible bg: + * - `fill.opacity === 0` + * - 8-hex color with alpha = 00 (e.g. `#FFFFFF00`) + * + * Without these guards, a wrapper like + * `frame{ fill: [{type:'solid', color:'#FFFFFF', opacity: 0}], children: [text] }` + * sitting on a cream page would make detectTextBgContrast treat the + * wrapper itself as a white bg and report a healthy contrast ratio, + * silently masking the cream-on-cream failure underneath. + */ +function firstSolidColor(fills: unknown): string | null { + if (!Array.isArray(fills) || fills.length === 0) return null; + for (const fill of fills) { + if (!fill || typeof fill !== 'object') continue; + const f = fill as { + type?: string; + color?: string; + stops?: Array<{ color?: string }>; + opacity?: number; + }; + if (typeof f.opacity === 'number' && f.opacity === 0) continue; + if (f.type === 'solid' && typeof f.color === 'string') { + // 8-hex with alpha 00 = fully transparent solid — same as opacity=0. + // Format is `#RRGGBBAA` (9 chars including the hash). + if (f.color.length === 9 && f.color.slice(7).toLowerCase() === '00') continue; + return f.color; + } + if ( + (f.type === 'linear_gradient' || f.type === 'radial_gradient') && + Array.isArray(f.stops) && + f.stops.length > 0 && + typeof f.stops[0].color === 'string' + ) { + return f.stops[0].color; + } + } + return null; +} + +/** + * WCAG 2.x large-text rule: >= 18pt OR >= 14pt bold (weight >= 700). + * Web maps 18pt ≈ 24px and 14pt ≈ 18.66px → use 24/19 in pixel terms. + */ +function isLargeText(node: PenNode): boolean { + const t = node as PenNode & { fontSize?: number; fontWeight?: number | string }; + if (typeof t.fontSize !== 'number') return false; + if (t.fontSize >= 24) return true; + const weight = + typeof t.fontWeight === 'number' + ? t.fontWeight + : typeof t.fontWeight === 'string' + ? parseInt(t.fontWeight, 10) + : NaN; + if (t.fontSize >= 19 && Number.isFinite(weight) && weight >= 700) return true; + return false; +} + +/** + * Aesthetic detector: text whose color contrast against its rendered + * background falls below WCAG AA threshold (4.5:1 for normal text, + * 3.0:1 for large text). + * + * Background determination walks the parent chain and picks the first + * solid fill (or gradient first-stop) it finds. Missing fills are + * "transparent" — the walk continues to the next ancestor; if the chain + * exhausts with no color, the detector defaults to white (#FFFFFF), the + * canvas bg in OpenPencil's renderer. + * + * Both text and background colors are resolved through the document's + * variable table and active theme so `$color-text-primary` etc. compare + * correctly. Unresolvable refs (typo'd token names, runtime-only refs) + * skip the check rather than guess. + * + * Severity is INFO (detect-only, never auto-fix). The 2026-05-09 user + * feedback was explicit that auto-replacing brand colors via "nearest + * token" heuristics was unsafe — this detector surfaces issues into the + * audit panel and chat status line so the user / agent can decide, + * without silently rewriting their fills. + */ +export function detectTextBgContrast( + root: PenNode, + doc: PenDocument, + opts: DetectTextBgContrastOptions = {}, +): Issue[] { + const issues: Issue[] = []; + const variables = doc.variables ?? {}; + const theme = getDefaultTheme(doc.themes); + const normalThreshold = opts.normalThreshold ?? DEFAULT_NORMAL_THRESHOLD; + const largeThreshold = opts.largeThreshold ?? DEFAULT_LARGE_THRESHOLD; + + walk(root, []); + return issues; + + function walk(node: PenNode, ancestors: PenNode[]): void { + // Align prune with the canonical render-time visibility check + // (pen-core's isNodeVisible — `visible !== false && enabled !== false`). + // 2026-05-10 Codex round 4 caught a divergence: my earlier prune also + // checked `opacity === 0`, but the renderer treats opacity as a paint + // alpha, not a "skip rendering" flag — opacity=0 nodes still get + // walked + laid out, just painted with alpha 0. Pruning on opacity=0 + // would disagree with what users actually see when they probe the + // tree elsewhere. Stay aligned with the shared helper instead. + // + // Side note: opacity=0 nodes still get caught upstream — they paint + // nothing, so ancestorBgColor() further down still skips them when + // resolving the real bg. The text inside an opacity=0 wrapper IS + // walked, but its contrast is computed against whatever sits BEHIND + // the invisible wrapper, which is the user-visible truth. + if (!isNodeVisible(node)) return; + if (node.type === 'text') { + checkText(node, ancestors); + } + if ('children' in node && Array.isArray(node.children)) { + const nextAncestors = ancestors.concat(node); + for (const c of node.children) walk(c, nextAncestors); + } + } + + function ancestorBgColor(ancestors: PenNode[]): string { + // Walk closest-first; skip transparent/imageless ancestors. Default + // to white when the chain runs out — matches OpenPencil's canvas + // default and avoids the "everything fails" report when a doc has + // no explicit page fill. + for (let i = ancestors.length - 1; i >= 0; i--) { + // Node-level opacity=0 / visible=false hides the WHOLE wrapper + // including its fill. The 2026-05-10 Codex review caught this as + // a separate path from fill-level opacity (the inner `firstSolid + // Color` guard) — both have to skip or a node-opacity-0 wrapper + // masks the real bg further up. + const ancestor = ancestors[i] as PenNode & { opacity?: unknown; visible?: unknown }; + if (ancestor.opacity === 0) continue; + if (ancestor.visible === false) continue; + const fill = (ancestor as unknown as { fill?: unknown }).fill; + const raw = firstSolidColor(fill); + if (!raw) continue; + const resolved = resolveColorRef(raw, variables, theme); + if (typeof resolved === 'string') return resolved; + } + return '#FFFFFF'; + } + + function checkText(node: PenNode, ancestors: PenNode[]): void { + const textFill = (node as unknown as { fill?: unknown }).fill; + const rawText = firstSolidColor(textFill); + if (!rawText) return; // no fill or non-solid — renderer default applies, skip + const textColor = resolveColorRef(rawText, variables, theme); + if (typeof textColor !== 'string') return; // unresolvable ref + + const bgColor = ancestorBgColor(ancestors); + const ratio = colorContrast(textColor, bgColor); + if (!Number.isFinite(ratio)) return; // either color failed to parse + + const threshold = isLargeText(node) ? largeThreshold : normalThreshold; + if (ratio >= threshold) return; + + issues.push({ + nodeId: node.id, + category: 'text-bg-contrast', + // Detect-only — auto-replacing the fill with a "nearest brand + // token" is the path the 2026-05-09 review explicitly rejected. + severity: 'info', + property: 'fill', + currentValue: textFill ?? null, + // No suggestedValue: the right replacement depends on the design + // system + theme + intent, which only the user/agent can decide. + suggestedValue: null, + reason: `text/bg contrast ${ratio.toFixed(2)}:1 below ${threshold}:1 (text=${textColor} on bg=${bgColor})`, + }); + } +} diff --git a/packages/pen-ai-skills/src/diagnostics/detectors.ts b/packages/pen-ai-skills/src/diagnostics/detectors.ts index 0570b25ac..c0a009941 100644 --- a/packages/pen-ai-skills/src/diagnostics/detectors.ts +++ b/packages/pen-ai-skills/src/diagnostics/detectors.ts @@ -1,6 +1,8 @@ import type { PenNode, PenDocument } from '@zseven-w/pen-types'; import type { Issue } from './types'; -import { detectEdgeSectionPadding } from './detectors-spacing'; +import { detectEdgeSectionPadding, detectStackedHorizontalPadding } from './detectors-spacing'; +import { detectTextBgContrast } from './detectors-typography'; +import { colorContrast, parseHexColor, relativeLuminance } from './color-utils'; /** Extract the first fill color from a node (raw, including variable refs) */ function getFirstFillColor(node: PenNode): string | null { @@ -10,68 +12,6 @@ function getFirstFillColor(node: PenNode): string | null { return null; } -/** - * Compare two color strings via WCAG relative-luminance contrast ratio. - * Returns 1.0 for identical colors, growing toward 21.0 as they diverge, - * or Infinity if either color cannot be parsed (e.g. variable refs). - * - * Why WCAG contrast ratio rather than max RGB channel diff: - * the human eye is much more sensitive to small tonal differences on - * dark backgrounds than light ones (Weber–Fechner / dark adaptation). - * A 9-unit RGB diff means very different things at different lightness: - * - * #FAFAFA vs #F1F1F1 (light, RGB diff 9): contrast ratio ≈ 1.07 → invisible - * #111111 vs #1A1A1A (dark, RGB diff 9): contrast ratio ≈ 1.18 → distinguishable - * - * Channel-diff treats them identically and produces false positives on - * dark theme cards. Contrast ratio is luminance-based, perceptually - * uniform-ish, and gives the right answer in both regimes. It also - * matches the metric WCAG and design tools (Stark, Figma) use. - * - * Used by detectInvisibleContainers to catch cases where fill colors are - * visually nearly identical but not strictly equal. - */ -function colorContrast(a: string, b: string): number { - if (a === b) return 1; - const pa = parseHexColor(a); - const pb = parseHexColor(b); - if (!pa || !pb) return Infinity; - const lumA = relativeLuminance(pa); - const lumB = relativeLuminance(pb); - const lighter = Math.max(lumA, lumB); - const darker = Math.min(lumA, lumB); - return (lighter + 0.05) / (darker + 0.05); -} - -/** WCAG 2.x relative luminance for sRGB. Returns 0.0–1.0. */ -function relativeLuminance(c: { r: number; g: number; b: number }): number { - const lin = (v: number): number => { - const s = v / 255; - return s <= 0.03928 ? s / 12.92 : Math.pow((s + 0.055) / 1.055, 2.4); - }; - return 0.2126 * lin(c.r) + 0.7152 * lin(c.g) + 0.0722 * lin(c.b); -} - -/** Parse #rgb / #rrggbb / #rrggbbaa to {r,g,b}. Returns null on parse failure. */ -function parseHexColor(s: string): { r: number; g: number; b: number } | null { - if (typeof s !== 'string') return null; - const m = s.trim().match(/^#([0-9a-fA-F]{3,8})$/); - if (!m) return null; - let hex = m[1]; - if (hex.length === 3) { - hex = hex - .split('') - .map((c) => c + c) - .join(''); - } - if (hex.length !== 6 && hex.length !== 8) return null; - const r = parseInt(hex.slice(0, 2), 16); - const g = parseInt(hex.slice(2, 4), 16); - const b = parseInt(hex.slice(4, 6), 16); - if (Number.isNaN(r) || Number.isNaN(g) || Number.isNaN(b)) return null; - return { r, g, b }; -} - /** Check if a node already has a visible stroke */ function hasStroke(node: PenNode): boolean { if (!('stroke' in node)) return false; @@ -751,7 +691,7 @@ export function detectExcessiveFrameEffects(root: PenNode): Issue[] { } /** - * Run all 12 detectors and return the deduplicated combined issue list. + * Run all 14 detectors and return the deduplicated combined issue list. * Dedup key: `${nodeId}:${property}` (matches runPreValidationFixes). * On collision, the first issue wins (detector execution order below). */ @@ -769,6 +709,8 @@ export function detectAllIssues(root: PenNode, doc: PenDocument): Issue[] { ...detectMixedSiblingPadding(root), ...detectExcessiveFrameEffects(root), ...detectEdgeSectionPadding(root), + ...detectStackedHorizontalPadding(root), + ...detectTextBgContrast(root, doc), ]; const seen = new Set(); const unique: Issue[] = []; diff --git a/packages/pen-ai-skills/src/diagnostics/index.ts b/packages/pen-ai-skills/src/diagnostics/index.ts index bbfafbfe1..342fa32e8 100644 --- a/packages/pen-ai-skills/src/diagnostics/index.ts +++ b/packages/pen-ai-skills/src/diagnostics/index.ts @@ -14,4 +14,6 @@ export { detectExcessiveFrameEffects, detectAllIssues, } from './detectors'; -export { detectEdgeSectionPadding } from './detectors-spacing'; +export { detectEdgeSectionPadding, detectStackedHorizontalPadding } from './detectors-spacing'; +export { detectTextBgContrast } from './detectors-typography'; +export { colorContrast, parseHexColor, relativeLuminance } from './color-utils'; diff --git a/packages/pen-ai-skills/src/diagnostics/types.ts b/packages/pen-ai-skills/src/diagnostics/types.ts index 41dea235e..475174c3b 100644 --- a/packages/pen-ai-skills/src/diagnostics/types.ts +++ b/packages/pen-ai-skills/src/diagnostics/types.ts @@ -12,7 +12,9 @@ export type IssueCategory = | 'text-stroke' | 'mixed-sibling-padding' | 'excessive-frame-effects' - | 'edge-section-padding'; + | 'edge-section-padding' + | 'text-bg-contrast' + | 'stacked-horizontal-padding'; export interface Issue { /** Node id where the issue was detected */ diff --git a/packages/pen-mcp/src/routes/debug-routes.ts b/packages/pen-mcp/src/routes/debug-routes.ts index 320f04b4e..fdf03a466 100644 --- a/packages/pen-mcp/src/routes/debug-routes.ts +++ b/packages/pen-mcp/src/routes/debug-routes.ts @@ -47,6 +47,8 @@ export const DEBUG_TOOL_DEFINITIONS = [ 'mixed-sibling-padding', 'excessive-frame-effects', 'edge-section-padding', + 'text-bg-contrast', + 'stacked-horizontal-padding', ], }, description: 'Filter to specific detector categories.', diff --git a/scripts/ab-corpus/check-stacked-padding.ts b/scripts/ab-corpus/check-stacked-padding.ts new file mode 100644 index 000000000..65015e647 --- /dev/null +++ b/scripts/ab-corpus/check-stacked-padding.ts @@ -0,0 +1,110 @@ +/** + * Empirical check: how common is "root has horizontal padding AND a + * direct child also has horizontal padding" in real LLM output? + * + * Drives the cost-benefit on adding a `detect-stacked-horizontal- + * padding` detector. The 2026-05-10 user-reported "Bistro" Mobile + * design hit this pattern — root [0,16,0,16] + section [0,24] = 40px + * effective gutter, which read as "too much padding". Need to know + * if it's a one-off or a recurring AI-output shape before adding a + * detector. + */ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { parseModelOutput } from '@zseven-w/pen-ai-skills'; +import { applyToFreshDoc } from './apply'; + +interface JsonlRow { + promptId: string; + category: string; + difficulty: string; + variant: string; + rawOutput: string; +} + +function getHorizontalPadding(p: unknown): { left: number; right: number } { + if (typeof p === 'number') return { left: p, right: p }; + if (Array.isArray(p)) { + if (p.length === 4) return { left: Number(p[3] ?? 0), right: Number(p[1] ?? 0) }; + if (p.length === 2) return { left: Number(p[1] ?? 0), right: Number(p[1] ?? 0) }; + if (p.length === 1) return { left: Number(p[0] ?? 0), right: Number(p[0] ?? 0) }; + } + return { left: 0, right: 0 }; +} + +const FULL_BLEED_ROLES = new Set([ + 'hero', + 'banner', + 'cover', + 'header', + 'top-nav', + 'bottom-nav', + 'status-bar', + 'tab-bar', + 'tabbar', + 'navbar', +]); + +async function main(): Promise { + const runId = process.argv[2]; + if (!runId) { + console.error('usage: bun run scripts/ab-corpus/check-stacked-padding.ts '); + process.exit(1); + } + const path = join(import.meta.dir, 'runs', runId, 'scores.jsonl'); + const rows: JsonlRow[] = readFileSync(path, 'utf-8') + .split('\n') + .filter(Boolean) + .map((l) => JSON.parse(l)); + + let applied = 0; + let stacked = 0; + const examples: string[] = []; + + for (const r of rows) { + const parsed = parseModelOutput(r.rawOutput); + if (parsed.kind === 'garbage') continue; + const result = await applyToFreshDoc(parsed); + if (!result.ok || !result.doc) continue; + applied++; + const root = result.doc.children?.[0] as + | (Record & { padding?: unknown; children?: unknown[] }) + | undefined; + if (!root) continue; + const rootP = getHorizontalPadding(root.padding); + if (rootP.left === 0 && rootP.right === 0) continue; + if (!Array.isArray(root.children)) continue; + const offending: string[] = []; + for (const child of root.children as Array< + Record & { id?: string; role?: string; padding?: unknown } + >) { + if (child?.type !== 'frame') continue; + const role = String(child.role ?? '').toLowerCase(); + if (FULL_BLEED_ROLES.has(role)) continue; + const p = getHorizontalPadding(child.padding); + if (p.left > 0 || p.right > 0) offending.push(String(child.id ?? '?')); + } + if (offending.length > 0) { + stacked++; + if (examples.length < 8) { + examples.push( + ` ${r.promptId}/${r.variant} root=${rootP.left}+${rootP.right} offenders=${offending.length} (${offending.slice(0, 3).join(',')})`, + ); + } + } + } + + console.log(`applied: ${applied}`); + console.log( + `stacked horizontal padding (root H>0 AND ≥1 child section H>0): ${stacked} (${((stacked / applied) * 100).toFixed(1)}%)`, + ); + if (examples.length > 0) { + console.log('\nexamples:'); + for (const e of examples) console.log(e); + } +} + +main().catch((e) => { + console.error(e); + process.exit(1); +}); diff --git a/scripts/ab-corpus/inspect-contrast-hits.ts b/scripts/ab-corpus/inspect-contrast-hits.ts new file mode 100644 index 000000000..ae5410148 --- /dev/null +++ b/scripts/ab-corpus/inspect-contrast-hits.ts @@ -0,0 +1,56 @@ +/** + * For a given run, list every text-bg-contrast hit with the failing + * text color, resolved bg color, and ratio. Helps eyeball whether the + * detector is catching real contrast violations or over-firing because + * the apply-to-fresh-doc harness has no page-level fill. + */ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { detectTextBgContrast, parseModelOutput, type Issue } from '@zseven-w/pen-ai-skills'; +import { applyToFreshDoc } from './apply'; + +interface JsonlRow { + promptId: string; + category: string; + difficulty: string; + variant: string; + rawOutput: string; +} + +async function main(): Promise { + const runId = process.argv[2]; + const limit = Number(process.argv[3] ?? 50); + if (!runId) { + console.error('usage: bun run scripts/ab-corpus/inspect-contrast-hits.ts [limit]'); + process.exit(1); + } + const path = join(import.meta.dir, 'runs', runId, 'scores.jsonl'); + const lines = readFileSync(path, 'utf-8').split('\n').filter(Boolean); + const rows: JsonlRow[] = lines.map((l) => JSON.parse(l)); + + let shown = 0; + for (const r of rows) { + if (shown >= limit) break; + const parsed = parseModelOutput(r.rawOutput); + if (parsed.kind === 'garbage') continue; + const result = await applyToFreshDoc(parsed); + if (!result.ok || !result.doc) continue; + const root = result.doc.children?.[0] ?? null; + if (!root) continue; + const issues: Issue[] = detectTextBgContrast(root, result.doc); + if (issues.length === 0) continue; + console.log( + `\n=== ${r.promptId} [${r.category}/${r.difficulty}/${r.variant}] (${issues.length} hits) ===`, + ); + for (const issue of issues) { + // reason looks like: "text/bg contrast 3.05:1 below WCAG AA 4.5:1 (text=#... on bg=#...)" + console.log(` ${issue.nodeId.padEnd(50)} ${issue.reason}`); + } + shown++; + } +} + +main().catch((err) => { + console.error(err); + process.exit(1); +}); diff --git a/scripts/ab-corpus/inspect-issue-category.ts b/scripts/ab-corpus/inspect-issue-category.ts new file mode 100644 index 000000000..222a7b2b4 --- /dev/null +++ b/scripts/ab-corpus/inspect-issue-category.ts @@ -0,0 +1,59 @@ +/** + * Print every issue of a specific category across an entire run, with the + * row id, node id, and reason. Generic version of inspect-contrast-hits + * — used for spot-checking any single detector's true-positive vs + * false-positive rate against a real corpus. + */ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { + detectAllIssues, + parseModelOutput, + type Issue, + type IssueCategory, +} from '@zseven-w/pen-ai-skills'; +import { applyToFreshDoc } from './apply'; + +interface JsonlRow { + promptId: string; + category: string; + difficulty: string; + variant: string; + rawOutput: string; +} + +async function main(): Promise { + const runId = process.argv[2]; + const wantCat = process.argv[3] as IssueCategory | undefined; + if (!runId || !wantCat) { + console.error('usage: bun run scripts/ab-corpus/inspect-issue-category.ts '); + process.exit(1); + } + const path = join(import.meta.dir, 'runs', runId, 'scores.jsonl'); + const rows: JsonlRow[] = readFileSync(path, 'utf-8') + .split('\n') + .filter(Boolean) + .map((l) => JSON.parse(l)); + + for (const r of rows) { + const parsed = parseModelOutput(r.rawOutput); + if (parsed.kind === 'garbage') continue; + const result = await applyToFreshDoc(parsed); + if (!result.ok || !result.doc) continue; + const root = result.doc.children?.[0] ?? null; + if (!root) continue; + const issues: Issue[] = detectAllIssues(root, result.doc).filter((i) => i.category === wantCat); + if (issues.length === 0) continue; + console.log( + `\n=== ${r.promptId} [${r.category}/${r.difficulty}/${r.variant}] (${issues.length} hits) ===`, + ); + for (const issue of issues) { + console.log(` ${issue.nodeId.padEnd(50)} ${issue.reason}`); + } + } +} + +main().catch((e) => { + console.error(e); + process.exit(1); +}); diff --git a/scripts/ab-corpus/inspect-shape.ts b/scripts/ab-corpus/inspect-shape.ts new file mode 100644 index 000000000..3b28cf534 --- /dev/null +++ b/scripts/ab-corpus/inspect-shape.ts @@ -0,0 +1,76 @@ +/** + * Dump the doc shape (root width / height / layout / direct-children + * roles) for every applied row in a category. Used 2026-05-10 to figure + * out why detectEdgeSectionPadding scored 0 hits on the 220-row mobile + * subset of `2026-05-03-ab-v8-v1-default` — needed to see whether the + * model is producing page-shaped roots at all or just fragments that + * don't look like a mobile page. + */ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { parseModelOutput } from '@zseven-w/pen-ai-skills'; +import { applyToFreshDoc } from './apply'; + +interface JsonlRow { + promptId: string; + category: string; + difficulty: string; + variant: string; + rawOutput: string; +} + +async function main(): Promise { + const runId = process.argv[2]; + const wantCategory = process.argv[3] ?? 'mobile'; + if (!runId) { + console.error('usage: bun run scripts/ab-corpus/inspect-shape.ts [category]'); + process.exit(1); + } + const path = join(import.meta.dir, 'runs', runId, 'scores.jsonl'); + const rows: JsonlRow[] = readFileSync(path, 'utf-8') + .split('\n') + .filter(Boolean) + .map((l) => JSON.parse(l)); + + const buckets = new Map(); + for (const r of rows) { + if (r.category !== wantCategory) continue; + const parsed = parseModelOutput(r.rawOutput); + if (parsed.kind === 'garbage') continue; + const result = await applyToFreshDoc(parsed); + if (!result.ok || !result.doc) continue; + const root = result.doc.children?.[0] as + | (Record & { width?: unknown; height?: unknown; children?: unknown[] }) + | undefined; + if (!root) continue; + const w = root.width; + const h = root.height; + const childCount = Array.isArray(root.children) ? root.children.length : 0; + const wKey = + typeof w === 'number' + ? w >= 320 && w <= 480 + ? 'mobile-w' + : w > 480 && w <= 800 + ? 'tablet-w' + : w > 800 + ? 'desktop-w' + : 'tiny-w' + : `non-numeric:${typeof w}`; + const hKey = typeof h === 'number' ? (h >= 568 ? 'tall' : 'short') : `non-numeric:${typeof h}`; + const ratioKey = + typeof w === 'number' && typeof h === 'number' && w > 0 && h / w >= 1.5 + ? 'aspect-ok' + : 'aspect-low'; + const key = `${wKey} / ${hKey} / ${ratioKey} / children=${childCount}`; + buckets.set(key, (buckets.get(key) ?? 0) + 1); + } + + console.log(`shapes for category=${wantCategory}:`); + const sorted = Array.from(buckets.entries()).sort((a, b) => b[1] - a[1]); + for (const [k, v] of sorted) console.log(` ${String(v).padStart(4)} ${k}`); +} + +main().catch((e) => { + console.error(e); + process.exit(1); +}); diff --git a/scripts/ab-corpus/replay-detectors.ts b/scripts/ab-corpus/replay-detectors.ts new file mode 100644 index 000000000..b1b6eed48 --- /dev/null +++ b/scripts/ab-corpus/replay-detectors.ts @@ -0,0 +1,113 @@ +/** + * Replay an existing scores.jsonl through the CURRENT detector set. + * + * 2026-05-10 — used to validate the post-2026-05-08 detector additions + * (detectEdgeSectionPadding + detectTextBgContrast) against real GPT-5.5 + * output without burning fresh API tokens. Reads each row's rawOutput, + * re-applies it to a fresh doc, runs detectAllIssues with today's 13 + * detectors, and reports per-category counts so we can spot detectors + * that now over-fire (false-positive prone) or stay silent. + * + * Usage: bun run scripts/ab-corpus/replay-detectors.ts + */ +import { readFileSync, existsSync } from 'node:fs'; +import { join } from 'node:path'; +import { detectAllIssues, parseModelOutput, type Issue } from '@zseven-w/pen-ai-skills'; +import { applyToFreshDoc } from './apply'; + +interface JsonlRow { + promptId: string; + category: string; + difficulty: string; + model: string; + variant: string; + rawOutput: string; + issues: Issue[]; +} + +async function main(): Promise { + const runId = process.argv[2]; + if (!runId) { + console.error('usage: bun run scripts/ab-corpus/replay-detectors.ts '); + process.exit(1); + } + const path = join(import.meta.dir, 'runs', runId, 'scores.jsonl'); + if (!existsSync(path)) { + console.error(`scores.jsonl not found: ${path}`); + process.exit(1); + } + + const lines = readFileSync(path, 'utf-8').split('\n').filter(Boolean); + const rows: JsonlRow[] = lines.map((l) => JSON.parse(l)); + console.log(`replay: ${rows.length} rows from ${runId}`); + + let appliedOk = 0; + let appliedErr = 0; + const oldByCategory = new Map(); + const newByCategory = new Map(); + const sampleByCategory = new Map(); + + for (const r of rows) { + for (const issue of r.issues ?? []) { + oldByCategory.set(issue.category, (oldByCategory.get(issue.category) ?? 0) + 1); + } + } + + for (const r of rows) { + const parsed = parseModelOutput(r.rawOutput); + if (parsed.kind === 'garbage') { + appliedErr++; + continue; + } + const result = await applyToFreshDoc(parsed); + if (!result.ok || !result.doc) { + appliedErr++; + continue; + } + appliedOk++; + + const root = result.doc.children?.[0] ?? null; + if (!root) continue; + const issues = detectAllIssues(root, result.doc); + for (const issue of issues) { + newByCategory.set(issue.category, (newByCategory.get(issue.category) ?? 0) + 1); + const samples = sampleByCategory.get(issue.category) ?? []; + if (samples.length < 3) { + samples.push(r); + sampleByCategory.set(issue.category, samples); + } + } + } + + console.log(`\napplied: ok=${appliedOk} err=${appliedErr}`); + const allCategories = new Set([...oldByCategory.keys(), ...newByCategory.keys()]); + console.log('\nper-category counts (old → new):'); + console.log('category old new delta'); + console.log('--------------------------------- ----- ----- -------'); + const sorted = Array.from(allCategories).sort(); + for (const cat of sorted) { + const o = oldByCategory.get(cat) ?? 0; + const n = newByCategory.get(cat) ?? 0; + const delta = n - o; + const sign = delta > 0 ? '+' : ''; + console.log( + `${cat.padEnd(33)} ${String(o).padStart(5)} ${String(n).padStart(5)} ${sign}${String(delta).padStart(5)}`, + ); + } + + const newCats = Array.from(newByCategory.keys()).filter((c) => !oldByCategory.has(c)); + if (newCats.length > 0) { + console.log('\nfirst-time-firing detectors (worth spot-checking):'); + for (const cat of newCats) { + console.log(`\n ${cat} (${newByCategory.get(cat)} hits)`); + for (const sample of sampleByCategory.get(cat) ?? []) { + console.log(` - ${sample.promptId} [${sample.category}/${sample.difficulty}]`); + } + } + } +} + +main().catch((err) => { + console.error(err); + process.exit(1); +});