From bc9f8d1c628bd51d2d0d447c03f4f0a3310ad951 Mon Sep 17 00:00:00 2001 From: Fini Date: Sat, 9 May 2026 20:59:40 +0800 Subject: [PATCH] =?UTF-8?q?feat(ai):=20aesthetic=20detector=20=E2=80=94=20?= =?UTF-8?q?mixed-sibling-padding=20(mirror=20cornerRadius=20rule)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why: continuation of the aesthetic detector series. Mirrors detectMixedSiblingCornerRadius (53435bf7) for the padding axis. Three cards with padding 16 / 16 / 20 looks ragged on canvas; the existing sibling-inconsistency detector covers cards-vs-cards but dedupes against cornerRadius and other props so the padding outlier sometimes drops. What: detectMixedSiblingPadding normalises padding values to a 4-tuple [top, right, bottom, left] before comparison, so padding: 16 → [16,16,16,16] padding: [12, 24] → [12,24,12,24] (CSS 2-tuple shorthand) padding: [16,16,16,16] → [16,16,16,16] all compare equal and don't trigger false positives. Modal value collapses back to a scalar when all four sides are equal so the suggested fix matches the model's preferred shorthand. Same 60% modal-majority threshold as the cornerRadius detector — 1-1-1 three-way splits are skipped because there's no canonical value to suggest. Same divider / spacer skip and same-type-and-role grouping. Wired through detectAllIssues + index.ts exports + the debug_validation_report MCP categories enum. 6 new tests cover: number shorthand outlier, number-vs-array equivalence, 2-tuple-vs-4-tuple equivalence, 1-1-1 split skip, mixed-role groups skipped, no-padding siblings excluded from modal. 57 / 57 diagnostics tests pass (was 51; +6). --- .../src/__tests__/diagnostics.test.ts | 90 ++++++++++++++++++ .../src/diagnostics/detectors.ts | 95 ++++++++++++++++++- .../pen-ai-skills/src/diagnostics/index.ts | 1 + .../pen-ai-skills/src/diagnostics/types.ts | 3 +- packages/pen-mcp/src/routes/debug-routes.ts | 1 + 5 files changed, 188 insertions(+), 2 deletions(-) diff --git a/packages/pen-ai-skills/src/__tests__/diagnostics.test.ts b/packages/pen-ai-skills/src/__tests__/diagnostics.test.ts index 7d5e78f4a..0f2d61e04 100644 --- a/packages/pen-ai-skills/src/__tests__/diagnostics.test.ts +++ b/packages/pen-ai-skills/src/__tests__/diagnostics.test.ts @@ -9,6 +9,7 @@ import { detectMixedSiblingCornerRadius, detectTextEffect, detectTextStroke, + detectMixedSiblingPadding, detectAllIssues, } from '../diagnostics/detectors'; import type { PenNode, PenDocument } from '@zseven-w/pen-types'; @@ -969,3 +970,92 @@ describe('detectTextStroke', () => { expect(detectTextStroke(root)).toHaveLength(0); }); }); + +describe('detectMixedSiblingPadding', () => { + it('flags an outlier when 2 of 3 cards share padding (number shorthand)', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { id: 'c1', type: 'frame', role: 'card', padding: 16, children: [] }, + { id: 'c2', type: 'frame', role: 'card', padding: 16, children: [] }, + { id: 'c3', type: 'frame', role: 'card', padding: 20, children: [] }, + ], + } as unknown as PenNode; + const issues = detectMixedSiblingPadding(root); + expect(issues).toHaveLength(1); + expect(issues[0].nodeId).toBe('c3'); + expect(issues[0].suggestedValue).toBe(16); // collapses to scalar when all 4 sides equal + }); + + it('treats number 16 and array [16,16,16,16] as equal (no false positive)', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { id: 'c1', type: 'frame', role: 'card', padding: 16, children: [] }, + { id: 'c2', type: 'frame', role: 'card', padding: [16, 16, 16, 16], children: [] }, + { id: 'c3', type: 'frame', role: 'card', padding: 16, children: [] }, + ], + } as unknown as PenNode; + expect(detectMixedSiblingPadding(root)).toHaveLength(0); + }); + + it('treats CSS-shorthand 2-tuple [v,h] equivalent to [v,h,v,h]', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { id: 'c1', type: 'frame', role: 'card', padding: [12, 24], children: [] }, + { id: 'c2', type: 'frame', role: 'card', padding: [12, 24, 12, 24], children: [] }, + { id: 'c3', type: 'frame', role: 'card', padding: [16, 16, 16, 16], children: [] }, + ], + } as unknown as PenNode; + const issues = detectMixedSiblingPadding(root); + expect(issues).toHaveLength(1); + expect(issues[0].nodeId).toBe('c3'); + // Modal is [12,24,12,24] — not all equal — suggested stays array + expect(issues[0].suggestedValue).toEqual([12, 24, 12, 24]); + }); + + it('does not flag a 1-1-1 three-way split (no canonical modal)', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { id: 'c1', type: 'frame', role: 'card', padding: 8, children: [] }, + { id: 'c2', type: 'frame', role: 'card', padding: 16, children: [] }, + { id: 'c3', type: 'frame', role: 'card', padding: 24, children: [] }, + ], + } as unknown as PenNode; + expect(detectMixedSiblingPadding(root)).toHaveLength(0); + }); + + it('does not flag siblings with mixed roles (card vs button)', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { id: 'c1', type: 'frame', role: 'card', padding: 16, children: [] }, + { id: 'c2', type: 'frame', role: 'card', padding: 16, children: [] }, + { id: 'b1', type: 'frame', role: 'button', padding: 8, children: [] }, + ], + } as unknown as PenNode; + expect(detectMixedSiblingPadding(root)).toHaveLength(0); + }); + + it('skips siblings with no padding prop (modal counts only set values)', () => { + const root = { + id: 'r', + type: 'frame', + children: [ + { id: 'c1', type: 'frame', role: 'card', padding: 16, children: [] }, + { id: 'c2', type: 'frame', role: 'card', padding: 16, children: [] }, + { id: 'c3', type: 'frame', role: 'card', children: [] }, // no padding + ], + } as unknown as PenNode; + // Only 2 cards have padding — no modal pair to compare against the + // unpadded one, so silent. + expect(detectMixedSiblingPadding(root)).toHaveLength(0); + }); +}); diff --git a/packages/pen-ai-skills/src/diagnostics/detectors.ts b/packages/pen-ai-skills/src/diagnostics/detectors.ts index 0485df1a6..aff928388 100644 --- a/packages/pen-ai-skills/src/diagnostics/detectors.ts +++ b/packages/pen-ai-skills/src/diagnostics/detectors.ts @@ -592,7 +592,99 @@ export function detectTextStroke(root: PenNode): Issue[] { } /** - * Run all 9 detectors and return the deduplicated combined issue list. + * Aesthetic detector: siblings (>= 3 of the same type+role) with + * inconsistent padding. Mirrors detectMixedSiblingCornerRadius — picks + * the modal value, flags the outliers. + * + * Padding is normalised to a 4-tuple [top, right, bottom, left] for + * comparison so e.g. `padding: 16` (shorthand) and `padding: [16,16,16,16]` + * (explicit) compare equal. Numbers, 2-tuples [v, h] (CSS shorthand) and + * 4-tuples are all accepted; anything else is treated as "no padding". + * + * Picks up "three cards with padding 16 / 16 / 20" — the kind of + * 4px-mismatch the model emits when copying examples from different + * sources. The existing sibling-inconsistency detector already covers + * cards-vs-cards via FRAME_STRICT_PROPS, BUT it dedupes against + * cornerRadius and other props so the padding outlier sometimes drops. + * This is a stricter equal-or-report check on padding alone. + */ +export function detectMixedSiblingPadding(root: PenNode): Issue[] { + const issues: Issue[] = []; + function normalise(p: unknown): string | null { + if (typeof p === 'number') return `[${p},${p},${p},${p}]`; + if (Array.isArray(p)) { + if (p.length === 2) return `[${p[0]},${p[1]},${p[0]},${p[1]}]`; + if (p.length === 4) return `[${p[0]},${p[1]},${p[2]},${p[3]}]`; + } + return null; + } + 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(); + const norms = siblings.map((s) => { + const p = (s as unknown as { padding?: unknown }).padding; + const n = normalise(p); + if (n) counts.set(n, (counts.get(n) ?? 0) + 1); + return n; + }); + if (counts.size < 2) continue; + let modal = ''; + let modalCount = 0; + for (const [val, count] of counts) { + if (count > modalCount) { + modal = val; + modalCount = count; + } + } + // Same 60% rule as cornerRadius — only flag when there's a + // clear majority. 1-1-1 splits get skipped (no canonical + // value to suggest). + if (modalCount / siblings.length < 0.6) continue; + siblings.forEach((s, i) => { + const norm = norms[i]; + if (norm !== null && norm !== modal) { + // Parse modal back to a value the store can apply. Modal + // is always [a,b,c,d] form. + const m = modal.match( + /\[(-?\d+(?:\.\d+)?),(-?\d+(?:\.\d+)?),(-?\d+(?:\.\d+)?),(-?\d+(?:\.\d+)?)\]/, + ); + if (!m) return; + const [, a, b, c, d] = m; + const tup = [Number(a), Number(b), Number(c), Number(d)]; + const allEqual = tup.every((n) => n === tup[0]); + const suggested: number | number[] = allEqual ? tup[0] : tup; + issues.push({ + nodeId: s.id, + category: 'mixed-sibling-padding', + severity: 'warning', + property: 'padding', + currentValue: (s as unknown as { padding?: unknown }).padding, + suggestedValue: suggested, + reason: `padding ${norm} doesn't match the ${modalCount} other sibling(s) at ${modal}`, + }); + } + }); + } + } + for (const c of node.children) walk(c); + } + walk(root); + return issues; +} + +/** + * Run all 10 detectors and return the deduplicated combined issue list. * Dedup key: `${nodeId}:${property}` (matches runPreValidationFixes). * On collision, the first issue wins (detector execution order below). */ @@ -607,6 +699,7 @@ export function detectAllIssues(root: PenNode, doc: PenDocument): Issue[] { ...detectMixedSiblingCornerRadius(root), ...detectTextEffect(root), ...detectTextStroke(root), + ...detectMixedSiblingPadding(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 0da21c88c..b26d40831 100644 --- a/packages/pen-ai-skills/src/diagnostics/index.ts +++ b/packages/pen-ai-skills/src/diagnostics/index.ts @@ -10,5 +10,6 @@ export { detectMixedSiblingCornerRadius, detectTextEffect, detectTextStroke, + detectMixedSiblingPadding, detectAllIssues, } from './detectors'; diff --git a/packages/pen-ai-skills/src/diagnostics/types.ts b/packages/pen-ai-skills/src/diagnostics/types.ts index fddcffa23..a9d81940e 100644 --- a/packages/pen-ai-skills/src/diagnostics/types.ts +++ b/packages/pen-ai-skills/src/diagnostics/types.ts @@ -9,7 +9,8 @@ export type IssueCategory = | 'text-corner-radius' | 'mixed-sibling-corner-radius' | 'text-effect' - | 'text-stroke'; + | 'text-stroke' + | 'mixed-sibling-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 9e146194b..3efe13746 100644 --- a/packages/pen-mcp/src/routes/debug-routes.ts +++ b/packages/pen-mcp/src/routes/debug-routes.ts @@ -44,6 +44,7 @@ export const DEBUG_TOOL_DEFINITIONS = [ 'mixed-sibling-corner-radius', 'text-effect', 'text-stroke', + 'mixed-sibling-padding', ], }, description: 'Filter to specific detector categories.',