diff --git a/packages/pen-ai-skills/src/__tests__/diagnostics.test.ts b/packages/pen-ai-skills/src/__tests__/diagnostics.test.ts index 72dfbab60..717f7f30b 100644 --- a/packages/pen-ai-skills/src/__tests__/diagnostics.test.ts +++ b/packages/pen-ai-skills/src/__tests__/diagnostics.test.ts @@ -4,6 +4,9 @@ import { detectEmptyPaths, detectTextExplicitHeights, detectSiblingInconsistencies, + detectUnexpectedRotation, + detectTextCornerRadius, + detectMixedSiblingCornerRadius, detectAllIssues, } from '../diagnostics/detectors'; import type { PenNode, PenDocument } from '@zseven-w/pen-types'; @@ -629,3 +632,203 @@ describe('detectAllIssues', () => { expect(issues).toHaveLength(0); }); }); + +describe('detectUnexpectedRotation', () => { + it('flags a frame with non-axis-aligned rotation', () => { + const root = { + id: 'r', + type: 'frame', + rotation: 12, + children: [], + } as unknown as PenNode; + const issues = detectUnexpectedRotation(root); + expect(issues).toHaveLength(1); + expect(issues[0].category).toBe('unexpected-rotation'); + expect(issues[0].nodeId).toBe('r'); + expect(issues[0].suggestedValue).toBe(0); + }); + + it('does not flag rotation=0 (default)', () => { + const root = { id: 'r', type: 'frame', rotation: 0, children: [] } as unknown as PenNode; + expect(detectUnexpectedRotation(root)).toHaveLength(0); + }); + + it('does not flag missing rotation prop', () => { + const root = { id: 'r', type: 'frame', children: [] } as unknown as PenNode; + expect(detectUnexpectedRotation(root)).toHaveLength(0); + }); + + it('does not flag axis-aligned rotation (90/180/270 — intentional vertical text / grid)', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { id: 'a', type: 'frame', rotation: 90, children: [] } as unknown as PenNode, + { id: 'b', type: 'frame', rotation: 180, children: [] } as unknown as PenNode, + { id: 'c', type: 'frame', rotation: -90, children: [] } as unknown as PenNode, + ], + } as unknown as PenNode; + expect(detectUnexpectedRotation(root)).toHaveLength(0); + }); + + it('does NOT flag path / line / polygon / image (decorative geometry often rotated)', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { id: 'p', type: 'path', rotation: 30 } as unknown as PenNode, + { id: 'l', type: 'line', rotation: 45 } as unknown as PenNode, + { id: 'g', type: 'polygon', rotation: 17 } as unknown as PenNode, + { id: 'i', type: 'image', rotation: 12 } as unknown as PenNode, + ], + } as unknown as PenNode; + expect(detectUnexpectedRotation(root)).toHaveLength(0); + }); + + it('flags nested frame with rotation', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { + id: 'wrapper', + type: 'frame', + children: [{ id: 'tilted', type: 'text', rotation: 7 } as unknown as PenNode], + } as unknown as PenNode, + ], + } as unknown as PenNode; + const issues = detectUnexpectedRotation(root); + expect(issues).toHaveLength(1); + expect(issues[0].nodeId).toBe('tilted'); + }); +}); + +describe('detectTextCornerRadius', () => { + it('flags text node with cornerRadius > 0', () => { + const root = { + id: 'r', + type: 'frame', + children: [{ id: 't', type: 'text', cornerRadius: 8, content: 'hi' } as unknown as PenNode], + } as unknown as PenNode; + const issues = detectTextCornerRadius(root); + expect(issues).toHaveLength(1); + expect(issues[0].category).toBe('text-corner-radius'); + expect(issues[0].nodeId).toBe('t'); + expect(issues[0].suggestedValue).toBeUndefined(); + }); + + it('does not flag text without cornerRadius', () => { + const root = { + id: 'r', + type: 'frame', + children: [{ id: 't', type: 'text', content: 'hi' } as unknown as PenNode], + } as unknown as PenNode; + expect(detectTextCornerRadius(root)).toHaveLength(0); + }); + + it('does not flag text with cornerRadius=0', () => { + const root = { + id: 'r', + type: 'frame', + children: [{ id: 't', type: 'text', cornerRadius: 0, content: 'hi' } as unknown as PenNode], + } as unknown as PenNode; + expect(detectTextCornerRadius(root)).toHaveLength(0); + }); + + it('does not flag frame nodes with cornerRadius (only text)', () => { + const root = { + id: 'r', + type: 'frame', + cornerRadius: 12, + children: [], + } as unknown as PenNode; + expect(detectTextCornerRadius(root)).toHaveLength(0); + }); +}); + +describe('detectMixedSiblingCornerRadius', () => { + it('flags an outlier when 2 of 3 same-role cards share cornerRadius', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { id: 'c1', type: 'frame', role: 'card', cornerRadius: 8, children: [] }, + { id: 'c2', type: 'frame', role: 'card', cornerRadius: 8, children: [] }, + { id: 'c3', type: 'frame', role: 'card', cornerRadius: 12, children: [] }, + ], + } as unknown as PenNode; + const issues = detectMixedSiblingCornerRadius(root); + expect(issues).toHaveLength(1); + expect(issues[0].nodeId).toBe('c3'); + expect(issues[0].suggestedValue).toBe(8); + }); + + it('does not flag when all siblings share cornerRadius', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { id: 'c1', type: 'frame', role: 'card', cornerRadius: 8, children: [] }, + { id: 'c2', type: 'frame', role: 'card', cornerRadius: 8, children: [] }, + { id: 'c3', type: 'frame', role: 'card', cornerRadius: 8, children: [] }, + ], + } as unknown as PenNode; + expect(detectMixedSiblingCornerRadius(root)).toHaveLength(0); + }); + + it('does not flag a 1-1-1 three-way split (no canonical modal value)', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { id: 'c1', type: 'frame', role: 'card', cornerRadius: 4, children: [] }, + { id: 'c2', type: 'frame', role: 'card', cornerRadius: 8, children: [] }, + { id: 'c3', type: 'frame', role: 'card', cornerRadius: 12, children: [] }, + ], + } as unknown as PenNode; + expect(detectMixedSiblingCornerRadius(root)).toHaveLength(0); + }); + + it('does not flag siblings with mixed roles (card vs button — different design tier)', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { id: 'c1', type: 'frame', role: 'card', cornerRadius: 8, children: [] }, + { id: 'c2', type: 'frame', role: 'card', cornerRadius: 8, children: [] }, + { id: 'b1', type: 'frame', role: 'button', cornerRadius: 4, children: [] }, + ], + } as unknown as PenNode; + expect(detectMixedSiblingCornerRadius(root)).toHaveLength(0); + }); + + it('does not flag fewer than 3 siblings', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { id: 'c1', type: 'frame', role: 'card', cornerRadius: 8, children: [] }, + { id: 'c2', type: 'frame', role: 'card', cornerRadius: 12, children: [] }, + ], + } as unknown as PenNode; + expect(detectMixedSiblingCornerRadius(root)).toHaveLength(0); + }); + + it('skips dividers and spacers when grouping (they should not count)', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { id: 'c1', type: 'frame', role: 'card', cornerRadius: 8, children: [] }, + { id: 'd1', type: 'frame', role: 'divider', cornerRadius: 0, children: [] }, + { id: 'c2', type: 'frame', role: 'card', cornerRadius: 8, children: [] }, + { id: 's1', type: 'frame', role: 'spacer', cornerRadius: 0, children: [] }, + { id: 'c3', type: 'frame', role: 'card', cornerRadius: 12, children: [] }, + ], + } as unknown as PenNode; + const issues = detectMixedSiblingCornerRadius(root); + // Only c1/c2/c3 are grouped (3 cards), modal=8, c3 is the outlier + expect(issues).toHaveLength(1); + expect(issues[0].nodeId).toBe('c3'); + }); +}); diff --git a/packages/pen-ai-skills/src/diagnostics/detectors.ts b/packages/pen-ai-skills/src/diagnostics/detectors.ts index 1f8a52a01..8d67e91bb 100644 --- a/packages/pen-ai-skills/src/diagnostics/detectors.ts +++ b/packages/pen-ai-skills/src/diagnostics/detectors.ts @@ -371,7 +371,151 @@ export function detectSiblingInconsistencies(root: PenNode): Issue[] { } /** - * Run all 4 detectors and return the deduplicated combined issue list. + * Aesthetic detector: rotation on a UI-bearing node. + * + * Rationale: AI-generated layouts almost never want rotation on a text / + * frame / shape node. When a model emits `rotation: 12` on a frame the + * card visibly tilts — usually an artifact of a misread Figma export or + * a model copying decorative geometry into UI flow. Skip path / line / + * polygon nodes (legitimate decorative geometry frequently has + * rotation), skip exactly-0 and missing values, and skip rotation that + * is a multiple of 90 (intentional vertical text or grid rotation). + */ +export function detectUnexpectedRotation(root: PenNode): Issue[] { + const issues: Issue[] = []; + const SKIP_TYPES = new Set(['path', 'line', 'polygon', 'image']); + function walk(node: PenNode): void { + const rec = node as unknown as Record; + const rotation = typeof rec.rotation === 'number' ? rec.rotation : 0; + if ( + !SKIP_TYPES.has(node.type) && + rotation !== 0 && + Math.abs(rotation % 90) > 0.5 // 90/180/270 rotations are usually intentional + ) { + issues.push({ + nodeId: node.id, + category: 'unexpected-rotation', + severity: 'warning', + property: 'rotation', + currentValue: rotation, + suggestedValue: 0, + reason: `${node.type} node has rotation=${rotation}; UI nodes are rarely tilted on purpose`, + }); + } + if ('children' in node && Array.isArray(node.children)) { + for (const c of node.children) walk(c); + } + } + walk(root); + return issues; +} + +/** + * Aesthetic detector: text node with cornerRadius. + * + * Rationale: cornerRadius on a text node has no rendering effect (text + * isn't drawn into a clipped rectangle), but it's a common AI hallucination + * — the model copies a generic "card-like" prop set onto a text run. The + * stale prop survives in the doc, confuses downstream code generators + * that round-trip the schema, and burns LLM context on subsequent + * batch_get calls. Suggest fixing to undefined (remove). + */ +export function detectTextCornerRadius(root: PenNode): Issue[] { + const issues: Issue[] = []; + function walk(node: PenNode): void { + if (node.type === 'text') { + const cr = (node as unknown as { cornerRadius?: unknown }).cornerRadius; + const num = typeof cr === 'number' ? cr : null; + if (num !== null && num > 0) { + issues.push({ + nodeId: node.id, + category: 'text-corner-radius', + severity: 'warning', + property: 'cornerRadius', + currentValue: num, + suggestedValue: undefined, + reason: 'text nodes have no clip path — cornerRadius is silently dropped at render time', + }); + } + } + if ('children' in node && Array.isArray(node.children)) { + for (const c of node.children) walk(c); + } + } + walk(root); + return issues; +} + +/** + * Aesthetic detector: siblings (>= 3 of the same type+role) with + * inconsistent cornerRadius. Picks the modal value, flags the outliers. + * + * The existing `sibling-inconsistency` detector already catches this for + * frame nodes via FRAME_STRICT_PROPS, BUT only when the modal-vs-outlier + * delta is >= 50% of the modal value. For cornerRadius specifically the + * threshold misses tiny but visually obvious mismatches (8 vs 12 across + * three cards reads as ragged on canvas). This detector uses a strict + * "equal or report" check on cornerRadius alone. + */ +export function detectMixedSiblingCornerRadius(root: PenNode): Issue[] { + const issues: Issue[] = []; + function walk(node: PenNode): void { + if (!('children' in node) || !Array.isArray(node.children)) return; + if (node.children.length >= 3) { + const groups = new Map(); + for (const c of node.children) { + const role = ((c as { role?: string }).role ?? '').toLowerCase() || '__none__'; + if (role === 'divider' || role === 'spacer') continue; + const key = `${c.type}:${role}`; + if (!groups.has(key)) groups.set(key, []); + groups.get(key)!.push(c); + } + for (const [, siblings] of groups) { + if (siblings.length < 3) continue; + const counts = new Map(); + for (const s of siblings) { + const cr = (s as unknown as { cornerRadius?: unknown }).cornerRadius; + const num = typeof cr === 'number' ? cr : 0; + counts.set(num, (counts.get(num) ?? 0) + 1); + } + if (counts.size < 2) continue; + let modal = 0; + let modalCount = 0; + for (const [val, count] of counts) { + if (count > modalCount) { + modal = val; + modalCount = count; + } + } + // Only flag if the modal is a clear majority (>= 60%) so we + // don't report a 1-1-1 three-way split as "outliers" — there's + // no canonical value to suggest in that case. + if (modalCount / siblings.length < 0.6) continue; + for (const s of siblings) { + const cr = (s as unknown as { cornerRadius?: unknown }).cornerRadius; + const num = typeof cr === 'number' ? cr : 0; + if (num !== modal) { + issues.push({ + nodeId: s.id, + category: 'mixed-sibling-corner-radius', + severity: 'warning', + property: 'cornerRadius', + currentValue: num, + suggestedValue: modal, + reason: `cornerRadius ${num} doesn't match the ${modalCount} other ${node.children.length - 1} sibling(s) at ${modal}`, + }); + } + } + } + } + for (const c of node.children) walk(c); + } + walk(root); + return issues; +} + +/** + * Run all 7 detectors and return the deduplicated combined issue list. * Dedup key: `${nodeId}:${property}` (matches runPreValidationFixes). * On collision, the first issue wins (detector execution order below). */ @@ -381,6 +525,9 @@ export function detectAllIssues(root: PenNode, doc: PenDocument): Issue[] { ...detectEmptyPaths(root), ...detectTextExplicitHeights(root), ...detectSiblingInconsistencies(root), + ...detectUnexpectedRotation(root), + ...detectTextCornerRadius(root), + ...detectMixedSiblingCornerRadius(root), ]; 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 220cab4ed..8b56824ef 100644 --- a/packages/pen-ai-skills/src/diagnostics/index.ts +++ b/packages/pen-ai-skills/src/diagnostics/index.ts @@ -5,5 +5,8 @@ export { detectEmptyPaths, detectTextExplicitHeights, detectSiblingInconsistencies, + detectUnexpectedRotation, + detectTextCornerRadius, + detectMixedSiblingCornerRadius, detectAllIssues, } from './detectors'; diff --git a/packages/pen-ai-skills/src/diagnostics/types.ts b/packages/pen-ai-skills/src/diagnostics/types.ts index bc0974b80..61d19a9cc 100644 --- a/packages/pen-ai-skills/src/diagnostics/types.ts +++ b/packages/pen-ai-skills/src/diagnostics/types.ts @@ -4,7 +4,10 @@ export type IssueCategory = | 'invisible-container' | 'empty-path' | 'text-explicit-height' - | 'sibling-inconsistency'; + | 'sibling-inconsistency' + | 'unexpected-rotation' + | 'text-corner-radius' + | 'mixed-sibling-corner-radius'; 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 18595b046..0a031766e 100644 --- a/packages/pen-mcp/src/routes/debug-routes.ts +++ b/packages/pen-mcp/src/routes/debug-routes.ts @@ -39,6 +39,9 @@ export const DEBUG_TOOL_DEFINITIONS = [ 'empty-path', 'text-explicit-height', 'sibling-inconsistency', + 'unexpected-rotation', + 'text-corner-radius', + 'mixed-sibling-corner-radius', ], }, description: 'Filter to specific detector categories.',