From 62b24bda76bd599f4837c988677646a7cfda7478 Mon Sep 17 00:00:00 2001 From: Fini Date: Mon, 6 Apr 2026 19:44:28 +0800 Subject: [PATCH] fix(ai): normalize sub-agent stroke/fill schema violations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dumping the latest M2.7 retry showed that the activity rings and heart-rate chart line were all invisible, even though the AI had generated reasonable geometry. The root cause was three overlapping schema violations that the renderer silently dropped: 1. `stroke` as an array — the AI wraps it in [] as if it were `fill`. `stroke.thickness` is then undefined, canvas gives up, and the stroke is never drawn. Every activity ring had this. 2. Fill-shaped stroke — the inner value is { type: 'solid', color } instead of { thickness, fill }. `strokeWidth` is written as a top-level node field in CSS/SVG style rather than as `stroke.thickness`. This is what the heart-rate chart line had. 3. CSS keyword colors in fill — `fill: [{type: 'solid', color: 'none'}]` or `'transparent'`. Neither is valid PenFill; CanvasKit ignores them or falls back to an unspecified default. The 8-digit transparent hex `#00000000` is fine and is explicitly kept. Add a new pen-core pass `normalizeStrokeFillSchema` that recursively repairs all three in place: - Unwrap array-wrapped strokes. - Migrate fill-shaped stroke objects to proper PenStroke, pulling any top-level `strokeWidth` into `stroke.thickness` (default 2 if missing), moving the color into `stroke.fill[0]`, and deleting the stray strokeWidth field. - Drop `{color: "none"}` and `{color: "transparent"}` fill entries from both `fill` and `stroke.fill`. If the resulting stroke has no fill left, remove the whole stroke. Covered by 13 pen-core unit tests including two end-to-end repros of the M2.7 activity-ring and heart-rate-chart-line failures, plus defensive tests for valid hex fills, 8-digit transparent hex, object-shaped (non-array) strokes, and recursion into children. Wired in at the start of applyPostStreamingTreeHeuristics (streaming path) and sanitizeNodesForInsert/sanitizeNodesForUpsert (batch path), so every apply route sees a clean schema before role resolution and layout passes touch anything. --- apps/web/src/canvas/canvas-layout-engine.ts | 1 + apps/web/src/services/ai/design-canvas-ops.ts | 15 ++ .../normalize-stroke-fill-schema.test.ts | 218 ++++++++++++++++++ packages/pen-core/src/index.ts | 1 + .../normalize/normalize-stroke-fill-schema.ts | 162 +++++++++++++ 5 files changed, 397 insertions(+) create mode 100644 packages/pen-core/src/__tests__/normalize-stroke-fill-schema.test.ts create mode 100644 packages/pen-core/src/normalize/normalize-stroke-fill-schema.ts diff --git a/apps/web/src/canvas/canvas-layout-engine.ts b/apps/web/src/canvas/canvas-layout-engine.ts index ecc807cc..5e29cfc1 100644 --- a/apps/web/src/canvas/canvas-layout-engine.ts +++ b/apps/web/src/canvas/canvas-layout-engine.ts @@ -15,4 +15,5 @@ export { normalizeTreeLayout, unwrapFakePhoneMockups, stripRedundantSectionFills, + normalizeStrokeFillSchema, } from '@zseven-w/pen-core'; diff --git a/apps/web/src/services/ai/design-canvas-ops.ts b/apps/web/src/services/ai/design-canvas-ops.ts index c6108df7..94d14e29 100644 --- a/apps/web/src/services/ai/design-canvas-ops.ts +++ b/apps/web/src/services/ai/design-canvas-ops.ts @@ -18,6 +18,7 @@ import { normalizeTreeLayout, unwrapFakePhoneMockups, stripRedundantSectionFills, + normalizeStrokeFillSchema, } from '@/canvas/canvas-layout-engine'; import { forcePageResync } from '@/canvas/canvas-sync-utils'; import { @@ -626,6 +627,12 @@ export function applyPostStreamingTreeHeuristics(rootNodeId: string): void { if (!rootNode || rootNode.type !== 'frame') return; if (!Array.isArray(rootNode.children) || rootNode.children.length === 0) return; + // Schema-level normalization runs first: unwrap array-wrapped strokes, + // migrate fill-shaped stroke objects to proper PenStroke, drop illegal + // "none"/"transparent" CSS keyword fills. Sub-agents break these + // constraints constantly and downstream passes assume valid shapes. + normalizeStrokeFillSchema(rootNode); + // Earliest pass: strip fake phone mockup wrappers that weaker sub-agents // generate when they misread the prompt's phone mockup guidance. Must run // BEFORE resolveTreeRoles, otherwise the role resolver may write defaults @@ -882,6 +889,10 @@ function sanitizeNodesForInsert(nodes: PenNode[], existingIds: Set): Pen const cloned = nodes.map((n) => deepCloneNode(n)); for (const node of cloned) { + // Schema normalization first so later passes see valid stroke/fill + // shapes (unwrap stroke arrays, migrate fill-shaped strokes, drop + // CSS-keyword fill colors). + normalizeStrokeFillSchema(node); // Strip fake phone mockup wrappers BEFORE role resolution so role // defaults aren't wasted on a wrapper we're about to discard. unwrapFakePhoneMockups(node); @@ -912,6 +923,10 @@ function sanitizeNodesForUpsert(nodes: PenNode[]): PenNode[] { const cloned = nodes.map((n) => deepCloneNode(n)); for (const node of cloned) { + // Schema normalization first so later passes see valid stroke/fill + // shapes (unwrap stroke arrays, migrate fill-shaped strokes, drop + // CSS-keyword fill colors). + normalizeStrokeFillSchema(node); // Strip fake phone mockup wrappers BEFORE role resolution so role // defaults aren't wasted on a wrapper we're about to discard. unwrapFakePhoneMockups(node); diff --git a/packages/pen-core/src/__tests__/normalize-stroke-fill-schema.test.ts b/packages/pen-core/src/__tests__/normalize-stroke-fill-schema.test.ts new file mode 100644 index 00000000..bff3e8f6 --- /dev/null +++ b/packages/pen-core/src/__tests__/normalize-stroke-fill-schema.test.ts @@ -0,0 +1,218 @@ +import { describe, it, expect } from 'vitest'; +import type { PenNode } from '@zseven-w/pen-types'; +import { normalizeStrokeFillSchema } from '../normalize/normalize-stroke-fill-schema'; + +const path = (props: Partial = {}): PenNode => + ({ + id: 'p', + type: 'path', + d: 'M0 0 L100 0', + width: 100, + height: 50, + ...props, + }) as PenNode; + +const ellipse = (props: Partial = {}): PenNode => + ({ + id: 'e', + type: 'ellipse', + width: 100, + height: 100, + ...props, + }) as PenNode; + +const frame = (props: Partial & { children?: PenNode[] } = {}): PenNode => + ({ + id: 'f', + type: 'frame', + width: 200, + height: 200, + ...props, + }) as PenNode; + +const validFill = [{ type: 'solid' as const, color: '#C4F82A' }]; + +describe('normalizeStrokeFillSchema — stroke array unwrap', () => { + it('unwraps a stroke that is an array of one proper PenStroke object', () => { + const node = ellipse({ + stroke: [ + { thickness: 12, fill: validFill }, + ] as unknown as PenNode['stroke'], + }); + normalizeStrokeFillSchema(node); + const rec = node as unknown as { stroke?: { thickness?: number; fill?: unknown } }; + expect(Array.isArray(rec.stroke)).toBe(false); + expect(rec.stroke?.thickness).toBe(12); + expect(rec.stroke?.fill).toEqual(validFill); + }); + + it('leaves a stroke that is already a proper object alone', () => { + const node = ellipse({ + stroke: { thickness: 4, fill: validFill } as PenNode['stroke'], + }); + normalizeStrokeFillSchema(node); + const rec = node as unknown as { stroke?: { thickness?: number; fill?: unknown } }; + expect(rec.stroke?.thickness).toBe(4); + }); + + it('converts a fill-shaped stroke entry into a proper PenStroke', () => { + // Real M2.7 failure: stroke is an array and the inner object has + // {type, color} (fill shape) instead of {thickness, fill}. strokeWidth + // lives as a top-level node field. + const node = path({ + stroke: [{ type: 'solid', color: '#C4F82A' }] as unknown as PenNode['stroke'], + strokeWidth: 2.5, + } as Partial & { strokeWidth?: number }); + normalizeStrokeFillSchema(node); + const rec = node as unknown as { + stroke?: { thickness?: number; fill?: Array<{ color?: string }> }; + strokeWidth?: number; + }; + expect(rec.stroke?.thickness).toBe(2.5); + expect(rec.stroke?.fill?.[0]?.color).toBe('#C4F82A'); + // Stray strokeWidth field is cleaned up after migration + expect(rec.strokeWidth).toBeUndefined(); + }); + + it('uses a default thickness when neither strokeWidth nor thickness is present', () => { + const node = path({ + stroke: [{ type: 'solid', color: '#111' }] as unknown as PenNode['stroke'], + }); + normalizeStrokeFillSchema(node); + const rec = node as unknown as { stroke?: { thickness?: number; fill?: unknown } }; + expect(rec.stroke?.thickness).toBeGreaterThan(0); + expect(rec.stroke?.fill).toBeDefined(); + }); + + it('handles an object-shaped stroke with a fill-shaped inner value', () => { + // Some sub-agents emit { stroke: { type: "solid", color: "#fff" } } + // (object, not array) — still a fill shape. Same recovery rules apply. + const node = path({ + stroke: { type: 'solid', color: '#FFFFFF' } as unknown as PenNode['stroke'], + strokeWidth: 3, + } as Partial & { strokeWidth?: number }); + normalizeStrokeFillSchema(node); + const rec = node as unknown as { + stroke?: { thickness?: number; fill?: Array<{ color?: string }> }; + }; + expect(rec.stroke?.thickness).toBe(3); + expect(rec.stroke?.fill?.[0]?.color).toBe('#FFFFFF'); + }); +}); + +describe('normalizeStrokeFillSchema — illegal fill color drops', () => { + it('drops fills whose color is "none"', () => { + const node = path({ + fill: [{ type: 'solid', color: 'none' }] as unknown as PenNode['fill'], + }); + normalizeStrokeFillSchema(node); + const rec = node as unknown as { fill?: unknown }; + // Either absent or empty array is acceptable — the important thing + // is that no { color: "none" } entry remains to confuse the renderer. + const f = rec.fill as unknown[] | undefined; + expect(f === undefined || (Array.isArray(f) && f.length === 0)).toBe(true); + }); + + it('drops fills whose color is "transparent"', () => { + const node = path({ + fill: [{ type: 'solid', color: 'transparent' }] as unknown as PenNode['fill'], + }); + normalizeStrokeFillSchema(node); + const rec = node as unknown as { fill?: unknown }; + const f = rec.fill as unknown[] | undefined; + expect(f === undefined || (Array.isArray(f) && f.length === 0)).toBe(true); + }); + + it('keeps legitimate hex fills alone', () => { + const node = path({ + fill: [{ type: 'solid', color: '#C4F82A' }] as unknown as PenNode['fill'], + }); + normalizeStrokeFillSchema(node); + const rec = node as unknown as { fill?: Array<{ color?: string }> }; + expect(rec.fill?.[0]?.color).toBe('#C4F82A'); + }); + + it('keeps 8-digit transparent hex (#00000000) alone', () => { + // The 8-digit hex IS a valid color string (alpha channel). Only the + // CSS keywords "none" and "transparent" are the unsupported forms. + const node = ellipse({ + fill: [{ type: 'solid', color: '#00000000' }] as unknown as PenNode['fill'], + }); + normalizeStrokeFillSchema(node); + const rec = node as unknown as { fill?: Array<{ color?: string }> }; + expect(rec.fill?.[0]?.color).toBe('#00000000'); + }); + + it('also strips illegal colors from stroke.fill arrays', () => { + const node = path({ + stroke: { thickness: 2, fill: [{ type: 'solid', color: 'none' }] } as unknown as PenNode['stroke'], + }); + normalizeStrokeFillSchema(node); + const rec = node as unknown as { stroke?: { thickness?: number; fill?: unknown[] } }; + // Whole stroke becomes either unset, or stroke.fill is empty — either + // way the renderer will not try to use "none". + if (rec.stroke && Array.isArray(rec.stroke.fill)) { + expect(rec.stroke.fill.length).toBe(0); + } + }); +}); + +describe('normalizeStrokeFillSchema — recursion', () => { + it('recurses into children', () => { + const inner = ellipse({ + id: 'inner', + stroke: [{ thickness: 8, fill: validFill }] as unknown as PenNode['stroke'], + }); + const root = frame({ id: 'root', children: [inner] }); + normalizeStrokeFillSchema(root); + const rec = inner as unknown as { stroke?: { thickness?: number } }; + expect(Array.isArray((inner as unknown as { stroke?: unknown }).stroke)).toBe(false); + expect(rec.stroke?.thickness).toBe(8); + }); + + it('reproduces the M2.7 activity rings case end-to-end', () => { + const ring = ellipse({ + id: 'ring', + name: 'Steps Circle', + width: 100, + height: 100, + fill: [{ type: 'solid', color: '#00000000' }] as unknown as PenNode['fill'], + stroke: [ + { thickness: 12, fill: [{ type: 'solid', color: '#C4F82A' }] }, + ] as unknown as PenNode['stroke'], + }); + normalizeStrokeFillSchema(ring); + const rec = ring as unknown as { + fill?: Array<{ color?: string }>; + stroke?: { thickness?: number; fill?: Array<{ color?: string }> }; + }; + // Stroke is now a proper object with the original thickness and color + expect(rec.stroke?.thickness).toBe(12); + expect(rec.stroke?.fill?.[0]?.color).toBe('#C4F82A'); + // Transparent 8-digit hex fill is preserved (it's not a CSS keyword) + expect(rec.fill?.[0]?.color).toBe('#00000000'); + }); + + it('reproduces the M2.7 heart-rate chart line case end-to-end', () => { + const line = path({ + id: 'line', + name: 'Chart Line', + d: 'M0 50 L100 20', + fill: [{ type: 'solid', color: 'none' }] as unknown as PenNode['fill'], + stroke: [{ type: 'solid', color: '#C4F82A' }] as unknown as PenNode['stroke'], + strokeWidth: 2.5, + } as Partial & { strokeWidth?: number }); + normalizeStrokeFillSchema(line); + const rec = line as unknown as { + fill?: unknown[]; + stroke?: { thickness?: number; fill?: Array<{ color?: string }> }; + strokeWidth?: number; + }; + expect(rec.stroke?.thickness).toBe(2.5); + expect(rec.stroke?.fill?.[0]?.color).toBe('#C4F82A'); + // "none" fill is gone + const f = rec.fill; + expect(f === undefined || (Array.isArray(f) && f.length === 0)).toBe(true); + expect(rec.strokeWidth).toBeUndefined(); + }); +}); diff --git a/packages/pen-core/src/index.ts b/packages/pen-core/src/index.ts index 17ac5657..f0bebc48 100644 --- a/packages/pen-core/src/index.ts +++ b/packages/pen-core/src/index.ts @@ -59,6 +59,7 @@ export { export { normalizeTreeLayout } from './layout/normalize-tree.js'; export { unwrapFakePhoneMockups } from './layout/unwrap-fake-phone-mockup.js'; export { stripRedundantSectionFills } from './layout/strip-redundant-section-fills.js'; +export { normalizeStrokeFillSchema } from './normalize/normalize-stroke-fill-schema.js'; // Text measurement export { diff --git a/packages/pen-core/src/normalize/normalize-stroke-fill-schema.ts b/packages/pen-core/src/normalize/normalize-stroke-fill-schema.ts new file mode 100644 index 00000000..7ced97c1 --- /dev/null +++ b/packages/pen-core/src/normalize/normalize-stroke-fill-schema.ts @@ -0,0 +1,162 @@ +import type { PenNode, PenFill, PenStroke, SolidFill } from '@zseven-w/pen-types'; + +/** + * Normalize stroke/fill schema violations commonly emitted by AI sub-agents + * (MiniMax M2.7, GLM, Kimi) that don't strictly follow the PenNode types. + * + * Three classes of schema violation are repaired in place, recursively + * across the tree: + * + * 1. `stroke` as an array of one entry — AI wraps the stroke object in an + * array as if it were `fill` (which IS an array). We unwrap the first + * element and continue normalizing it. + * + * 2. Stroke value shaped like a `SolidFill` ({ type, color }) instead of + * a `PenStroke` ({ thickness, fill }). We migrate the inner `color` + * into a proper `stroke.fill[0]`, pull the `strokeWidth` top-level + * field (the CSS/SVG-style spelling that many models emit) into + * `stroke.thickness`, and delete the stray `strokeWidth`. If neither + * a thickness nor a strokeWidth is present, default to 2 so the + * stroke actually draws something. + * + * 3. Fill entries with illegal CSS-keyword colors (`"none"`, `"transparent"`) + * are dropped. The 8-digit transparent hex (`"#00000000"`) is valid + * and kept. The same rule applies to any `stroke.fill[]` entries. + * + * Returns nothing — the tree is mutated in place, matching the other + * pen-core normalize passes. Callers that rely on Zustand publish + * semantics should route the result through `forcePageResync()` the same + * way they already do for other mutating post-streaming passes. + */ +export function normalizeStrokeFillSchema(node: PenNode): void { + normalizeNodeStroke(node); + normalizeNodeFill(node); + + if ('children' in node && Array.isArray(node.children)) { + for (const child of node.children) { + normalizeStrokeFillSchema(child); + } + } +} + +// --------------------------------------------------------------------------- +// Stroke normalization +// --------------------------------------------------------------------------- + +interface MaybeStrokeHolder { + stroke?: unknown; + strokeWidth?: unknown; +} + +function normalizeNodeStroke(node: PenNode): void { + const rec = node as unknown as MaybeStrokeHolder; + const rawStroke = rec.stroke; + if (rawStroke === undefined || rawStroke === null) return; + + // (1) Unwrap `stroke: [ ... ]` by taking the first element. + let stroke: unknown = rawStroke; + if (Array.isArray(stroke)) { + stroke = stroke.length > 0 ? stroke[0] : undefined; + } + if (!stroke || typeof stroke !== 'object') { + delete rec.stroke; + return; + } + + // (2) Detect the fill-shape-as-stroke pattern and migrate it. + const maybeFillShape = stroke as { + type?: unknown; + color?: unknown; + thickness?: unknown; + fill?: unknown; + }; + const looksLikeFillShape = + typeof maybeFillShape.type === 'string' && + typeof maybeFillShape.color === 'string' && + maybeFillShape.thickness === undefined && + maybeFillShape.fill === undefined; + + if (looksLikeFillShape) { + const thickness = readThickness(rec); + rec.stroke = { + thickness, + fill: [ + { + type: 'solid', + color: maybeFillShape.color as string, + } as SolidFill, + ], + } as PenStroke; + delete rec.strokeWidth; + // Now clean illegal color inside the migrated stroke.fill + stripIllegalColorsFromStrokeFill(node); + return; + } + + // Otherwise we have something that looks like a real PenStroke — fix + // missing thickness, clean up illegal colors, and persist any + // strokeWidth field that survived as a top-level property. + const strokeObj = stroke as Partial & { [k: string]: unknown }; + if (strokeObj.thickness === undefined || strokeObj.thickness === null) { + const width = readThickness(rec); + (strokeObj as { thickness?: number }).thickness = width; + } + rec.stroke = strokeObj as PenStroke; + delete rec.strokeWidth; + stripIllegalColorsFromStrokeFill(node); + + // If after cleanup the stroke has no fill at all, drop the whole stroke. + const cleaned = rec.stroke as PenStroke | undefined; + if (cleaned && (!cleaned.fill || cleaned.fill.length === 0)) { + delete rec.stroke; + } +} + +function readThickness(rec: MaybeStrokeHolder): number { + const raw = rec.strokeWidth; + if (typeof raw === 'number' && raw > 0) return raw; + if (typeof raw === 'string') { + const n = parseFloat(raw); + if (Number.isFinite(n) && n > 0) return n; + } + return 2; +} + +function stripIllegalColorsFromStrokeFill(node: PenNode): void { + const rec = node as unknown as { stroke?: { fill?: unknown } }; + const stroke = rec.stroke; + if (!stroke || typeof stroke !== 'object') return; + const fillArr = stroke.fill; + if (!Array.isArray(fillArr)) return; + (stroke as { fill?: PenFill[] }).fill = fillArr.filter( + (f) => isLegalFillEntry(f), + ) as PenFill[]; +} + +// --------------------------------------------------------------------------- +// Fill normalization +// --------------------------------------------------------------------------- + +function normalizeNodeFill(node: PenNode): void { + const rec = node as unknown as { fill?: unknown }; + const raw = rec.fill; + if (!raw) return; + if (!Array.isArray(raw)) return; + const cleaned = raw.filter((f) => isLegalFillEntry(f)); + if (cleaned.length === 0) { + delete rec.fill; + } else { + rec.fill = cleaned as PenFill[]; + } +} + +/** Reject fill entries whose color is an unsupported CSS keyword. */ +function isLegalFillEntry(entry: unknown): boolean { + if (!entry || typeof entry !== 'object') return false; + const e = entry as { type?: unknown; color?: unknown }; + if (e.type === 'solid' && typeof e.color === 'string') { + const c = e.color.trim().toLowerCase(); + if (c === 'none' || c === 'transparent') return false; + } + return true; +}