SemianiakaVY/openpencil
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

feat(ai): aesthetic detector — mixed-sibling-padding (mirror cornerRadius rule)

Browse source 
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).
This commit is contained in:
Fini 2026-05-09 20:59:40 +08:00
parent 3eacc8c9ac
commit bc9f8d1c62
5 changed files with 188 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

90
packages/pen-ai-skills/src/__tests__/diagnostics.test.ts
View file

@ -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);
});
});

95
packages/pen-ai-skills/src/diagnostics/detectors.ts
View file

@ -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<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<string, number>();
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<string>();
const unique: Issue[] = [];

1
packages/pen-ai-skills/src/diagnostics/index.ts
View file

@ -10,5 +10,6 @@ export {
detectMixedSiblingCornerRadius,
detectTextEffect,
detectTextStroke,
detectMixedSiblingPadding,
detectAllIssues,
} from './detectors';

3
packages/pen-ai-skills/src/diagnostics/types.ts
View file

@ -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 */

1
packages/pen-mcp/src/routes/debug-routes.ts
View file

@ -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.',