feat(ai): detect stacked horizontal padding (page-vs-section gutter)
14th pre-validation detector + a preventive skill rule. User-reported 2026-05-10 "Bistro" mobile food app shipped with root padding [0,16,0,16] AND a "Today's Specials" section padding [0,24]. Effective gutter = 40px on a 375px page → only 295px of usable content width. Reads as "too much padding" / pinched. Two pieces: 1. layout.md AESTHETIC HYGIENE block now teaches "page gutter goes on ONE layer, not both" — pick root horizontal padding OR per-section horizontal padding, not both. Default convention: root carries the gutter, sections set vertical-only padding. Hero / banner / image-bleed sections then sit edge-to-edge by simply NOT adding horizontal padding (root's gutter shows through). Preventive teaching at prompt time. 2. detectStackedHorizontalPadding (info-only, detect-only). Walks every mobile-shaped root (width 320–480 + tall + multi-child), compares root horizontal padding against each direct child's horizontal padding; flags the section as the offender when both are > 0. Page-shape filter mirrors detectEdgeSectionPadding so the legitimate component-internal padding stacking pattern (chip → badge → icon, etc.) doesn't trip it. Severity is INFO because a section may legitimately want a deeper inset for visual emphasis — let the user/agent decide via audit panel. Side-quest: scripts/ab-corpus/check-stacked-padding.ts ships with this commit so the next stacked-padding-style detector calibration can survey corpus frequency without rebuilding the harness.
This commit is contained in:
parent
160ab4c86c
commit
20dbbf227a
|
|
@ -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).
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
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';
|
||||
|
||||
|
|
@ -691,7 +691,7 @@ export function detectExcessiveFrameEffects(root: PenNode): Issue[] {
|
|||
}
|
||||
|
||||
/**
|
||||
* Run all 13 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).
|
||||
*/
|
||||
|
|
@ -709,6 +709,7 @@ export function detectAllIssues(root: PenNode, doc: PenDocument): Issue[] {
|
|||
...detectMixedSiblingPadding(root),
|
||||
...detectExcessiveFrameEffects(root),
|
||||
...detectEdgeSectionPadding(root),
|
||||
...detectStackedHorizontalPadding(root),
|
||||
...detectTextBgContrast(root, doc),
|
||||
];
|
||||
const seen = new Set<string>();
|
||||
|
|
|
|||
|
|
@ -14,6 +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';
|
||||
|
|
|
|||
|
|
@ -13,7 +13,8 @@ export type IssueCategory =
|
|||
| 'mixed-sibling-padding'
|
||||
| 'excessive-frame-effects'
|
||||
| 'edge-section-padding'
|
||||
| 'text-bg-contrast';
|
||||
| 'text-bg-contrast'
|
||||
| 'stacked-horizontal-padding';
|
||||
|
||||
export interface Issue {
|
||||
/** Node id where the issue was detected */
|
||||
|
|
|
|||
|
|
@ -48,6 +48,7 @@ export const DEBUG_TOOL_DEFINITIONS = [
|
|||
'excessive-frame-effects',
|
||||
'edge-section-padding',
|
||||
'text-bg-contrast',
|
||||
'stacked-horizontal-padding',
|
||||
],
|
||||
},
|
||||
description: 'Filter to specific detector categories.',
|
||||
|
|
|
|||
110
scripts/ab-corpus/check-stacked-padding.ts
Normal file
110
scripts/ab-corpus/check-stacked-padding.ts
Normal file
|
|
@ -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<void> {
|
||||
const runId = process.argv[2];
|
||||
if (!runId) {
|
||||
console.error('usage: bun run scripts/ab-corpus/check-stacked-padding.ts <run-id>');
|
||||
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<string, unknown> & { 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<string, unknown> & { 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);
|
||||
});
|
||||
Loading…
Reference in a new issue