From b719efd3e8b0968805c7983ffabb7b35e4d7ac03 Mon Sep 17 00:00:00 2001 From: Fini Date: Mon, 4 May 2026 23:22:52 +0800 Subject: [PATCH] fix(pen-core): strip safe-light fills on misrolled component-wrapper sections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Real repro from MiniMax-M2.7: sub-agent emits a section wrapper with the WRONG role applied — Search Bar(role=search-bar) > Search Input Container(role=input,fill=$color-surface). The outer "search-bar" frame is actually a section-level wrapper (its child carries the real atom), but its role is `search-bar` which is in PROTECTED_ROLES, so the strip pass treated it as the real atom and left its #F8FAFC hedge fill alone. Result: visible double-cream nesting against the cream root background. Detect this misroll: a frame whose role IS protected but ALSO contains a child carrying either the same role or another protected/structural role with its own solid fill is a wrapper, not the atom — its fill is eligible for the same safe-light/safe-dark hedge stripping that pure section frames get. Counter-case kept covered: a real `search-bar` atom whose children are just icons / placeholder text (no nested input/search-bar/card/etc with its own fill) keeps its fill — that fill is intentional, not a hedge. Two new tests: - M2.7 misrolled wrapper (search-bar > input + safe-light fill) — outer fill stripped, inner input fill preserved. - Real search-bar atom (no fill-bearing component children) — fill preserved. --- .../strip-redundant-section-fills.test.ts | 52 +++++++++++++++++++ .../layout/strip-redundant-section-fills.ts | 36 ++++++++++++- 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/packages/pen-core/src/__tests__/strip-redundant-section-fills.test.ts b/packages/pen-core/src/__tests__/strip-redundant-section-fills.test.ts index a3c7c42b9..22fddf076 100644 --- a/packages/pen-core/src/__tests__/strip-redundant-section-fills.test.ts +++ b/packages/pen-core/src/__tests__/strip-redundant-section-fills.test.ts @@ -287,6 +287,58 @@ describe('stripRedundantSectionFills', () => { expect((section as PenNode & { fill?: unknown }).fill).toBeUndefined(); }); + it('strips a misrolled section wrapper (search-bar role with a nested input child)', () => { + // Real repro: MiniMax-M2.7 emits Search Bar(role=search-bar) + // > Search Input Container(role=input,fill=$color-surface). The OUTER + // wrapper labeled 'search-bar' is actually a section, not the atom — + // its safe-light hedge fill should still be stripped because the inner + // child carries the real component fill. + const innerInput = frame({ + id: 'search-input', + name: 'Search Input Container', + role: 'input', + fill: solidFill('#FFFFFF'), + }); + const wrapper = frame({ + id: 'search-bar-wrapper', + name: 'Search Bar', + role: 'search-bar', + fill: solidFill('#F8FAFC'), // a safe-light hedge + children: [innerInput], + }); + const root = frame({ + id: 'root-frame', + fill: solidFill('#FFF8F0'), + children: [wrapper], + }); + const changed = stripRedundantSectionFills(root); + expect(changed).toBe(true); + expect((wrapper as PenNode & { fill?: unknown }).fill).toBeUndefined(); + // Inner input keeps its fill — it's the actual atom. + expect((innerInput as PenNode & { fill?: unknown }).fill).toEqual(solidFill('#FFFFFF')); + }); + + it('preserves a real search-bar atom with no inner-component children (not a wrapper)', () => { + // Counter-case: a search bar that is the actual atom (no nested input/ + // search-bar/etc child) — its fill is intentional and must be kept. + const realSearchBar = frame({ + id: 'real-search', + name: 'Search Bar', + role: 'search-bar', + fill: solidFill('#F1F5F9'), + children: [ + frame({ id: 'icon', type: 'frame' }), // no role / no fill + ], + }); + const root = frame({ + id: 'root', + fill: solidFill('#FFFFFF'), + children: [realSearchBar], + }); + stripRedundantSectionFills(root); + expect((realSearchBar as PenNode & { fill?: unknown }).fill).toEqual(solidFill('#F1F5F9')); + }); + it('reproduces the M2.7 health-tracker case', () => { // Direct repro of the actual failure: root #1a1a2e, six section roots // all hardcoded #0A0A0A, including one real card. The six section diff --git a/packages/pen-core/src/layout/strip-redundant-section-fills.ts b/packages/pen-core/src/layout/strip-redundant-section-fills.ts index e4119b8ee..270b4f215 100644 --- a/packages/pen-core/src/layout/strip-redundant-section-fills.ts +++ b/packages/pen-core/src/layout/strip-redundant-section-fills.ts @@ -97,13 +97,47 @@ const STRUCTURAL_ROLES = new Set([ function isSectionLevelFrame(node: PenNode): boolean { const role = (node as PenNode & { role?: string }).role; if (!role) return true; // unrolled section root - if (PROTECTED_ROLES.has(role)) return false; + if (PROTECTED_ROLES.has(role)) { + // A protected-role frame at section depth that ALSO contains a + // descendant of the SAME role (or a stand-alone fill-bearing child + // that visibly carries the component's true surface) is almost + // certainly a sub-agent-misrolled wrapper, NOT the real atom — e.g. + // sub-agent emits Search Bar(role=search-bar) > Search Input + // Container(role=input). The outer "search-bar" is just a section + // wrapper. Treat as section-level so its hedge fill can be stripped. + if (hasNestedFilledComponent(node, role)) return true; + return false; + } if (STRUCTURAL_ROLES.has(role)) return true; // Unknown role: be conservative, treat as protected so we don't clobber // future role additions. return false; } +/** + * True when a frame's children contain another node that carries either + * the same component role or a sibling component role with its own solid + * fill — the parent is then effectively a wrapper around the real atom. + */ +function hasNestedFilledComponent(node: PenNode, parentRole: string): boolean { + if (!('children' in node) || !Array.isArray(node.children)) return false; + for (const child of node.children) { + const childRole = (child as PenNode & { role?: string }).role; + if (!childRole) continue; + // Same role nested again ('search-bar' inside 'search-bar') — wrapper. + if (childRole === parentRole) return true; + // Component-bearing role with its own fill ('input' / 'card' / 'button' + // / 'badge' inside another component) — also a wrapper pattern. + if ( + (PROTECTED_ROLES.has(childRole) || STRUCTURAL_ROLES.has(childRole)) && + getFirstSolidColor(child) + ) { + return true; + } + } + return false; +} + /** * Hex tints that sub-agents reach for when they want a "safe dark" * background without knowing the real design background color. Any of