feat(ai): aesthetic detectors — rotation / text-cornerRadius / mixed-sibling-cornerRadius
Why: user reports the validation pipeline lacks "aesthetic standards"
— it accepts misalignment, unwanted corner radius, and other visual
issues as "normal". Existing detectors are pure code-quality (invisible
container / empty path / text height / sibling inconsistency); they
don't catch design-system violations the user can see at a glance.
Vision validation does, but it only runs on Anthropic / Codex /
OpenCode / Gemini providers and only above 30 nodes — leaving a long
tail of small-design / builtin-provider runs with no aesthetic check
at all. Adding cheap pure-function detectors closes that gap with no
upstream provider dependency.
What: 3 new pure detectors in pen-ai-skills/diagnostics:
- detectUnexpectedRotation — flags non-axis-aligned rotation on
UI-bearing nodes (frame / text / shape). Skips path / line /
polygon / image (legitimate decorative geometry frequently
rotated), skips multiples of 90° (intentional vertical text /
grid). Catches the "tilted card" hallucination cleanly.
- detectTextCornerRadius — flags text nodes with cornerRadius > 0.
Text isn't drawn into a clipped rectangle so the prop is silently
dropped at render time, but it survives in the doc and burns
LLM context on subsequent batch_get calls. Suggested fix: remove.
- detectMixedSiblingCornerRadius — stricter than the existing
sibling-inconsistency check on cornerRadius alone. Flags outliers
when 2+ of 3 same-type-and-role siblings share a value and one
differs (e.g. three cards with cornerRadius 8 / 8 / 12 reads as
ragged on canvas). Skips 1-1-1 three-way splits (no canonical
modal) and divider / spacer nodes (visual primitives).
All three are wired through detectAllIssues + the index.ts public
exports + the debug_validation_report MCP tool's `categories` enum so
the user / agent can opt-in or filter via `op debug_validation_report
--categories unexpected-rotation`.
35 new tests cover the load-bearing positive + negative cases for each
detector. 219/219 pen-ai-skills tests pass (was 184; +35). 1080/1080
AI service tests still pass.
This commit is contained in:
parent
8f35361518
commit
2faf79b5d1
|
|
@ -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');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>;
|
||||
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<string, PenNode[]>();
|
||||
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<number, number>();
|
||||
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<string>();
|
||||
const unique: Issue[] = [];
|
||||
|
|
|
|||
|
|
@ -5,5 +5,8 @@ export {
|
|||
detectEmptyPaths,
|
||||
detectTextExplicitHeights,
|
||||
detectSiblingInconsistencies,
|
||||
detectUnexpectedRotation,
|
||||
detectTextCornerRadius,
|
||||
detectMixedSiblingCornerRadius,
|
||||
detectAllIssues,
|
||||
} from './detectors';
|
||||
|
|
|
|||
|
|
@ -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 */
|
||||
|
|
|
|||
|
|
@ -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.',
|
||||
|
|
|
|||
Loading…
Reference in a new issue