Why: builtin providers (MiniMax / DeepSeek / Bailian / Ark) currently
fall through to the generic "Missing or unsupported provider" error in
/api/ai/validate. The post-generation loop catches that as a hard
provider error and logs "[error] Analysis skipped (timeout or provider
error)" — which reads like a config bug to the user even though the
real reason is "this provider's models are text-only, vision validation
isn't useful here even if we did proxy it".
What: branch on body.provider === 'builtin' before the generic error
and return { skipped: true, error: '<explanatory message>' }. The
client design-validation.ts already short-circuits on `data.skipped`
so the loop now logs the clearer message instead. No behavior change
for the four supported providers; no new wire fields.
Why: end-to-end test of "Design a bottom nav with Home / Search /
Orders / Cart / Profile" surfaced a stray coloured pill highlight
wrapping the Search tab. Root cause: the model labels the cell
\`role: 'search-bar'\` (intending "this tab whose icon is search"),
and the role-resolver dutifully stamps the input-shaped 44px-tall,
22-corner, filled-surface look onto the nav cell. Inside a 56px tall
tab row that pill swallows the icon + label, looks broken on canvas,
and competes for click area with the nav-item active state.
What: search-bar role now early-outs with `{}` (no overrides) when
ctx.parentRole is one of `bottom-tab-bar` / `tab-bar` / `tab-row` —
mirroring the same check the `button` role already uses to skip its
text-button defaults inside tab containers. Nav-cell layout / fill
remains the responsibility of nav-item / nav-item-active.
1070 / 1070 AI tests still pass; the input-shape default still applies
in every other context (forms, headers, hero search, etc.).
Why: for Type 0 component plans (Notification Card / Profile Card / …)
the orchestrator pre-inserts a page rootFrame named after the component,
then the sub-agent emits its own section-root frame as the only child.
Result is a visible "Notification Card → Notification Card" double wrap
in the layers panel and a wasted layout depth that does nothing visual.
The double wrap was confirmed in the 2026-05-09 end-to-end test of the
notification-card prompt: depth-0 = orchestrator rootFrame (role=card),
depth-1 = sub-agent wrapper (also role=card), actual children at depth-2.
What: new unwrapSingleComponentSectionRoot pass added as Phase 4c right
after the mobile-status-bar dedup (mutually exclusive: that runs only on
mobile, this runs only on component-shaped plans). Conservative match —
only fires when:
- plan.subtasks.length === 1, AND
- plan.rootFrame is narrow (≤480) and auto-height (<480 or 0), AND
- the orchestrator rootFrame has exactly 1 frame child, AND
- that child's id has the sub-agent section-root suffix
(`-root` / `-section`) OR the child copied the parent's name.
When the conditions hold, hoist the wrapper's children up via
store.moveNode (preserving order) and remove the wrapper. Multi-section
pages, dashboards, and mobile screens are untouched — early-out on the
plan.subtasks.length / width / height checks.
1070 / 1070 AI tests still pass; unit-testing this against the live
Zustand store is awkward, the integration verification will land via
the next end-to-end notification-card run.
Why: my prior C3 resolver fix added `image` to ICON_NOISE_WORDS so
"Image Placeholder Path" (a non-icon container name) wouldn't collapse
to a circle. That was overcorrecting — `image` is also the canonical
Lucide icon key for the picture/photo glyph, and the model frequently
emits "Image Icon" meaning exactly that. With image stripped, "Image
Icon" tokenised to [] and the resolver returned without writing the
matched lucide:image path.
What: remove `image` from ICON_NOISE_WORDS, with an inline note that
the multi-word "Image Placeholder Path" pattern still resolves through
the prefix fallback (`image` covers >= 50% of `imageplaceholder` so
findPrefixFallback picks it up). Add a regression test for "Image Icon"
→ /image/.
1070 / 1070 AI tests pass (was 1069; +1).
Why: end-to-end test of "design a notification card with dismiss x
button" surfaced that MiniMax-M2.7 emits a path node named "Dismiss
Icon". Tokenisation gives "dismiss" but Lucide doesn't have a `dismiss`
key — the resolver fell through prefix/substring fallbacks and wrote
the placeholder lucide:circle, leaving the card with a hollow ring
where the X should be.
What: 5 new aliases added in lock-step to icon-dictionary.ts (client
commonAliases) + icon.ts (server NAME_ALIASES per existing comment):
- dismiss → x (close button intent)
- closebutton → x (compacted from "Close Button Icon")
- cancel → x (cancel-action close icon)
- remove → x (remove-action close icon)
- expand → maximize-2
- collapse → minimize-2
NOT aliased: `cross`. Lucide already ships a `cross` icon (the
Christian-cross shape) and overriding it would lose that geometry.
"Cross" disambiguation is left to the model — if it really means a
close button, telling it to write "Dismiss Icon" / "Close Icon" via
the icon-catalog skill is enough.
Tests: 3 new it.each cases (Dismiss / Cancel / Remove Icon → /x/).
1069 / 1069 AI tests pass (was 1066; +3).
Why: Codex stop-time review #6 — C6 added workspace / console / 工作台 /
工作区 to the component DISQUALIFIER, but the dashboard detector regex
still only matched dashboard|admin|管理|后台|控制台. So "design a
workspace with side panel" skipped component (correct) AND skipped
dashboard (regex miss) and fell through to landing-page (1200×0,
4-section), which is the wrong shape for a workspace UI — the user
wants a 3-section desktop-screen with header/main/actions.
What: dashboard detector regex extended in lockstep with the
disqualifier — dashboard|admin|workspace|console|管理|后台|控制台|
工作台|工作区. Comment makes the "keep in sync" invariant explicit.
Tests: 4 new positive cases (Latin workspace + console, zh-Hans 工作台
+ 工作区 with 卡片) assert the plan returns 1200×800 with the 3-
section ['Header','Main Content','Actions'] layout, not the 4-section
landing-page default.
1066 / 1066 AI tests pass (was 1062; +4).
Why: Codex stop-time review #5 — broadening the component trigger list
from 17 to 25 nouns introduced false positives:
"admin dashboard with metric tiles" → matched `tile` → Type 0 (400×0)
when the user clearly wants a desktop dashboard. Same for "design an
admin panel" / "workspace with charts" / Chinese 后台管理 + 卡片.
What: COMPONENT_DISQUALIFIER_RE gains three new keyword buckets in
addition to the existing screen / page / home / onboarding / flow:
- mobile-screen markers — mobile, phone, ios, android, 手机, 移动端
- workspace markers — dashboard, admin, workspace, console, 管理,
后台, 控制台
+ zh-Hans 屏幕 (screen) was already added in C5.
These ensure component classification is reserved for "X card / X chip /
…" prompts that have no surrounding screen/dashboard/mobile context.
The dashboard / mobile prompts then continue down to their own explicit
detector branches and produce the right preset.
Tests: 7 new negative cases covering admin dashboards with tiles,
charts, panels, Chinese 后台 with 卡片, and mobile/phone prompts that
also mention card/badge. 1062 / 1062 AI tests pass (was 1055; +7).
Why: Codex stop-time review #4 — the previous regex covered ~17 nouns
but design-type.md documents 25 (button / label / row / item / selector
/ panel / chart were missing) and the CJK 卡片 alias was also listed.
JS `\b` is ASCII-only and never fires between two CJK chars, so
`\b卡片\b` matched nothing in "design a 卡片".
What: split into COMPONENT_TRIGGER_LATIN_RE (full noun list with `\b`
boundaries) + COMPONENT_TRIGGER_CJK_RE (kana-free subset of the most
common Chinese aliases — 卡片 / 徽章 / 标签 / 按钮 / 开关 / 对话框 /
提示 / 气泡 / 图表). Either match is enough to classify Type 0.
Disqualifier regex also gains 屏幕 (screen in zh-Hans).
Tests: 23 it.each cases pin one Latin trigger each plus the CJK 卡片;
6 negative cases prove the disqualifier still wins for "X screen / page
/ app / onboarding / flow" prompts. 1055 / 1055 AI tests pass (was
1027; +28 new).
Why: Codex stop-time review #3 flagged "Type 0 component handling is
incomplete". The earlier C1 fix (orchestrator-plan-classify helper +
isMobileFullScreen heuristic) covered the orchestrator path, but four
more places still bucketed narrow widths (≤480 / ≤500) as mobile and
mishandled component-shaped plans.
What:
- agent-tool-executor.ts: replace `width<=500 ? 375 : 1200` bucket on
setGenerationCanvasWidth with the inserted node's actual width — a
400-wide profile card now estimates text against 400, not 375.
- design-type-presets.ts: add 'component' to DesignType union with
width=400, height=0, and a single-section default. detectDesignType
matches "X card / X badge / X chip / ..." prompts BEFORE the mobile
/ dashboard check, so the parse-failure fallback returns a 400px
component instead of a 1200px landing-page for "design a profile
card". Disqualified when prompt also names a screen / page.
- orchestrator-prompt-optimizer.ts: 3 spots — platform selection now
uses preset.type==='mobile-screen' (component groups with webapp,
not mobile, since it has no status bar / bottom nav); compact
prompt rules and subtask hint get a component branch ("Use width=400
height=0, exactly 1 subtask, no chrome"); fallback height map gives
components a single 200px region instead of 800.
- orchestrator-planning.ts: buildFallbackHeights treats narrow +
auto-height plans as component-shape and emits 200px sections,
preventing the prior "812 / 1 = 812-tall card" output.
2 new tests pin: (a) "design a clean profile card" → 400×0 single
"Component" subtask with 200px region; (b) "design a card screen page"
must NOT shortcut to component (screen/page disqualifier holds).
Codex P0 mini-gate Round 2 finding (Q5) fix: gesture_re_export.rs tests
set the new W3C fields (KeyEvent.is_composing, FocusEvent.related_node_
id_hint, WheelEvent.delta_z + WheelEvent.mode mutability) but only
asserted the structural compile-time identity, not value readback.
Strengthened to assert every W3C field reads back what was written so
cross-crate type identity AND field-level binary compat are both verified
through the OP re-export path:
- key_event_is_re_exported_from_jian_with_all_w3c_fields: 7-field assert
- focus_event_is_re_exported_from_jian_with_all_w3c_fields: 3-field assert
- wheel_event_is_re_exported_from_jian_with_w3c_fields: defaults +
mutate-and-assert mode + delta_z + delta.x/y
cargo test -p openpencil-shell-core --test gesture_re_export → 6/6 PASS.
Why: MiniMax-M2.7 keeps emitting path nodes named "Search Icon Path" /
"Time Icon Path" / "Heart Icon Stroke" (3 words ending in noise word).
The legacy resolver normalised to "searchiconpath" (15 chars), prefix
fallback found "search" (6/15 = 40% < 50% threshold) → rejected →
fallback to lucide:circle → user-visible "circle bug" across categories,
filter chips, and search bar leading icons. Skill update alone (telling
models to use icon_font) doesn't fix the trained-pattern leftover —
Codex flagged this as a still-unfixed failure mode.
What: extractIconKeyword() tokenises on camelCase / space / dash /
underscore boundaries and drops { icon, logo, symbol, glyph, path,
shape, stroke, fill, svg, graphic, image }. Surviving tokens are
concatenated for direct dictionary lookup. Pure-noise names ("Icon
Path", "Symbol") return early without writing the misleading circle
placeholder. time / deliverytime / rider aliases added (kept in sync
across icon-dictionary.ts and server icon.ts NAME_ALIASES per existing
comment). 11 new tests cover multi-word resolution and pure-noise
no-op; 21 prior regression cases (descriptive geometry untouched,
single-word camelCase / kebab / snake all resolve, "Brand Logo"
placeholder behaviour preserved) still green.
Why: "Design a profile card" through MiniMax-M2.7 produced a 375×803 mobile
screen with auto-injected status bar, because the planner skill listed
"profiles" as a Type 2 single-task screen and the orchestrator's
isMobileScreen heuristic ran on width≤480 alone.
What: design-type.md + decomposition.md add Type 0 (single component:
card / badge / chip / modal) with width=400 height=0 1 subtask no chrome.
isMobileFullScreen helper extracted to orchestrator-plan-classify.ts and
required by both orchestrator.ts and orchestrator-sub-agent.ts so the
two paths can't drift on what "mobile" means (Codex review caught this
when only orchestrator.ts had the new check).
Verified with same MiniMax + same prompt: 400×320 component, 8 nodes,
firstChildRole=card, no status-bar.
Picks up the keyboard/IME/focus event additions + W3C wheel deltaMode
landed in jian commit d5d358e. shell-core re-exports of the new types
land in the next commit; this commit only moves the pointer + Cargo.lock.
cargo test -p openpencil-shell-core --test gesture_re_export → 6/6 PASS
against the pinned submodule.
ab-corpus rerun (gpt-5.5, ab-v3, 52 prompts × 2 arms): obvious-T M3
59.6% -> 91.5% (+31.9pp); composite-T 0% -> 40% (+40pp). Lift on top
of d5d1a8cd (Rank 1 schema coerce), 9e90cffe (Rank 2 prompt fail watch),
e34d9238 (Rank 3 vision toggle).
Builder fallback minima:
- chart-pie/line/bars-v1: values [1] -> [30,25,20,15,10] / [10,15,12,20,18]
so chart-pie-slice (>=4) and chart-line-dot (>=7) corpus minima are met
- toolbar-v1: fallback items include a divider_after entry so toolbar-divider
role emits even when the model passes only icons
- avatar-group-v1: entry-coerce items with 5 placeholder initials so the
builder always emits avatar-group-{item,initial,overflow,overflow-count}
- combobox/data-table-row/share-row-v1: fallback arrays grown to 3 items
matching the corpus shape minimums
Optional-content discipline (codex stop-time round 2):
- user-card-v1: name field fuzzy-coerce (required field, real fix for the
"Element tool insert failed: I(null,...)" handler bug); the optional role
text stays conditional, never invented. content empty-string was tried
but rejected (empty text nodes still consume flex gap).
- image-placeholder-v1: label stays conditional for the same reason.
Prompt:
- elements.md fail-watch table extended with 5 components (chart legend,
skeleton, inline-action, share-row, combobox) so models routing to
batch_design at least know the role names.
Multi-page vision validation (codex stop-time round 1):
- design-validation.ts: countNodesInActivePage + buildNodeTreeDump now
read getActivePageChildren(activePageId) instead of DEFAULT_FRAME_ID,
so the size-gate and the LLM's tree dump both reflect the page the user
is actually editing rather than the default page. Was a latent bug
surfaced when VALIDATION_ENABLED flipped to true in e34d9238.
Tests: 4223/4223 pass; format:check + tsc clean. 12 files changed.
Toggle VALIDATION_ENABLED from false to true so the post-generation
vision LLM validation loop runs. The loop itself was fully built in
design-validation.ts long ago — only ai-runtime-config:109 was holding
it at runtime.
Add VALIDATION_NODE_COUNT_THRESHOLD=30 size-gate so atomic single-tool
outputs (one badge, one chart) skip the +30-90s vision round-trip.
Composite multi-section briefs (full-page mockups, dashboards) easily
clear the threshold and benefit from the screenshot -> vision LLM ->
safe-fix -> re-screenshot rounds.
Pre-validation heuristics (ms-cheap tree walks) still run regardless
of size.
Plumbing was already done before this commit:
- design-screenshot.ts captureRegion() shipped in Phase 1.5
- design-validation.ts MAX_VALIDATION_ROUNDS=3 loop fully implemented
- validate.ts has 4 vision provider paths (Anthropic Agent SDK,
Codex CLI, OpenCode SDK, Gemini CLI)
This commit only flips the flag and adds the size-gate heuristic.
Tests: 4223/4223 pass; format:check + tsc clean.
Predicted KPI lift: M3 composite +5-10pp (speculative). Vision
catches what schema-coerce + role-hint can't — mis-positioned sibling
sections, missing component spacing, color-contrast issues. Gating by
node count keeps user-perceived latency contained to designs that
actually need it.
Out-of-scope for this commit (followups if needed):
- ab-corpus glm/minimax/deepseek client image_url part injection
(lets the eval harness exercise vision for KPI verification)
- builtin Zig agent-native runtime image part support (only matters
if the embedded provider becomes the default)
Codex flagged: \`normalizeTreeLayout\` strips \`x\` / \`y\` from
non-overlay children of any vertical / horizontal layout container
as a stale-coordinate cleanup. The new
\`convertStackedOverlayToAbsolute\` post-pass was wired in AFTER
normalize, so when a sub-agent emitted an intentional content
offset on a layered hero — e.g.
hero { layout: 'vertical', height: 200, children: [
image { full bg },
overlay { full bg gradient },
content { x: 16, y: 80 } ← inset above the gradient
]}
normalize would delete the \`x: 16, y: 80\` first, then convert
would flip layout to 'none' on a hero whose children have no
positions to honor. The content frame ends up at (0,0) overlapping
the bg image instead of where the model placed it.
Move convert to run BEFORE normalize. After convert, the
container's layout is 'none' so normalize sees an absolute-
positioning container and leaves the children's x/y untouched.
The function is a no-op when no layered pattern matches, so
running it earlier doesn't add cost on the common path.
New test asserts: convert + normalize (in that order) preserves
content's x=16, y=80 through the chain. Verified by reversing the
order in the test — assertion correctly fails with
"expected undefined to be 16", proving the regression coverage
actually exercises the bug condition.
M2.7 food-app run shipped a hero whose content piled into the next
section. Live doc inspection showed:
hero-image-container { width: 'fill_container', height: 200,
layout: 'vertical' }
├─ hero-image { width: 'fill_container', height: 200 }
├─ hero-overlay { width: 'fill_container', height: 200 } // gradient
└─ hero-content { width: 'fill_container', height: 'fit_content' }
├─ "Hungry?" title
└─ search-bar (48 tall)
The model intended the image + overlay to LAYER on top of each
other as bg+gradient with content floating on top. With
\`layout: 'vertical'\` the layout engine instead stacked them
sequentially: 200 + 200 + ~80 = 480, far past the 200 declared
height. No clipContent on the container, so the overflow rendered
into the NEXT sibling section — the user's screenshot showed
"Hungry?" search and category icons piled over the "Near You"
restaurant cards.
\`convertStackedOverlayToAbsolute\` post-pass detects the pattern
conservatively:
- frame, layout='vertical' (or undefined → infers vertical)
- numeric fixed height H
- >= 2 children of types image / rectangle / frame whose height
is exactly H or 'fill_container'
The repair: switch \`layout\` to 'none' so the layout engine
respects each child's own x/y (defaulting to 0/0 = layered) — the
image lands at (0,0), the overlay layers on top, and the content
frame floats on top. Children with explicit positions stay
respected.
Wired into \`design-canvas-ops.ts::applyPostStreamingTreeHeuristics\`
right after \`expandOverflowingFixedHeightCards\` so both layered
and overflowing-fixed-height fixes run together.
6 tests cover: hero pattern conversion, fill_container variant,
plain content stacks left alone (only one bg-like child), no
fixed height left alone, horizontal-layout side-by-side rows
left alone, nested heroes detected.
MiniMax M2.7 food-app run failed because the model emitted its full
subtask design wrapped in a single JSON array literal:
[
{ "id": "filterChips-root", "_parent": null, "type": "frame", … },
{ "id": "chip-1", "_parent": "filterChips-root", … },
…
]
The previous \`looksLikeJsonl\` gate only checked
\`startsWith('{')\` so this fell through to the DSL parser, which
tried to read \`[\` / \`{\` / \`}\` each on its own line as DSL
operations. Every line was rejected, the subtask returned empty,
the orchestrator retried with minimal skills, that timed out too,
and the user got a single-frame placeholder with one section
instead of the full screen.
Extended the gate to accept \`[\` as the leading character. The
shape signature stays the same (\`_parent\` or PenNode \`type\`
key inside the first 800 chars) — the bracket check just
disambiguates from real DSL. \`parseJsonlToTree\` already handles
both shapes via brace-counting (it scans for \`{...}\` blocks and
ignores surrounding \`[\`, \`]\`, and \`,\`), so the apply path
needed no changes.
Exported \`looksLikeJsonl\` for direct unit testing. 6 new tests
cover: pure JSONL match, JSON-array match (the M2.7 case),
array with leading whitespace, DSL-style assignment lines reject,
empty/non-bracketed reject, and bracketed-but-no-PenNode-keys
reject (so we don't reroute legit non-design array operations).
Verified by temporarily reverting the gate to just \`{\`: the two
new array tests correctly fail.
Drives the three-OS CI matrix verification of the skia-safe + glutin +
glow + winit dep stack per Step 1a spec §7.
- examples/p0_probe.rs: stencil_visibility + readback chain runner (must
own a real OS main thread because winit on macOS rejects
EventLoop::new() from cargo test worker threads).
- tests/p0_probe.rs: subprocess-invoke wrapper, gated
#[ignore = "P0_PROBE_GATE"] so default cargo test stays untouched.
- Cargo.toml: add transient [target.'cfg(not(target_arch = "wasm32"))'.
dev-dependencies] block (skia-safe 0.97 + glutin 0.32.3 + glutin-winit
0.5.0 + glow 0.17.0 + raw-window-handle 0.6.2 + scopeguard 1.2.0 +
winit defaults). Pinned to versions resolved in /tmp/skia-glow-probe.
- .github/workflows/rust-check.yml: install Linux GL prereqs (xvfb,
mesa, libxkbcommon, libwayland) and add a P0-probe-gate step running
cargo test --ignored on each OS (Linux through xvfb-run; Windows
early-returns per spec §8.2 WINDOWS_GPU_DEFERRED_NO_RUNNER).
All three artefacts are TRANSIENT — reverted in a follow-up cleanup
commit after CI is green and the loader-compat notes commit lands.
Task 1 owns the permanent integration.
Codex flagged: \`normalizeTreeLayout\` strips \`x\` / \`y\` from
non-overlay children of any vertical / horizontal layout container
as a stale-coordinate cleanup. The new
\`convertStackedOverlayToAbsolute\` post-pass was wired in AFTER
normalize, so when a sub-agent emitted an intentional content
offset on a layered hero — e.g.
hero { layout: 'vertical', height: 200, children: [
image { full bg },
overlay { full bg gradient },
content { x: 16, y: 80 } ← inset above the gradient
]}
normalize would delete the \`x: 16, y: 80\` first, then convert
would flip layout to 'none' on a hero whose children have no
positions to honor. The content frame ends up at (0,0) overlapping
the bg image instead of where the model placed it.
Move convert to run BEFORE normalize. After convert, the
container's layout is 'none' so normalize sees an absolute-
positioning container and leaves the children's x/y untouched.
The function is a no-op when no layered pattern matches, so
running it earlier doesn't add cost on the common path.
New test asserts: convert + normalize (in that order) preserves
content's x=16, y=80 through the chain. Verified by reversing the
order in the test — assertion correctly fails with
"expected undefined to be 16", proving the regression coverage
actually exercises the bug condition.
M2.7 food-app run shipped a hero whose content piled into the next
section. Live doc inspection showed:
hero-image-container { width: 'fill_container', height: 200,
layout: 'vertical' }
├─ hero-image { width: 'fill_container', height: 200 }
├─ hero-overlay { width: 'fill_container', height: 200 } // gradient
└─ hero-content { width: 'fill_container', height: 'fit_content' }
├─ "Hungry?" title
└─ search-bar (48 tall)
The model intended the image + overlay to LAYER on top of each
other as bg+gradient with content floating on top. With
\`layout: 'vertical'\` the layout engine instead stacked them
sequentially: 200 + 200 + ~80 = 480, far past the 200 declared
height. No clipContent on the container, so the overflow rendered
into the NEXT sibling section — the user's screenshot showed
"Hungry?" search and category icons piled over the "Near You"
restaurant cards.
\`convertStackedOverlayToAbsolute\` post-pass detects the pattern
conservatively:
- frame, layout='vertical' (or undefined → infers vertical)
- numeric fixed height H
- >= 2 children of types image / rectangle / frame whose height
is exactly H or 'fill_container'
The repair: switch \`layout\` to 'none' so the layout engine
respects each child's own x/y (defaulting to 0/0 = layered) — the
image lands at (0,0), the overlay layers on top, and the content
frame floats on top. Children with explicit positions stay
respected.
Wired into \`design-canvas-ops.ts::applyPostStreamingTreeHeuristics\`
right after \`expandOverflowingFixedHeightCards\` so both layered
and overflowing-fixed-height fixes run together.
6 tests cover: hero pattern conversion, fill_container variant,
plain content stacks left alone (only one bg-like child), no
fixed height left alone, horizontal-layout side-by-side rows
left alone, nested heroes detected.
MiniMax M2.7 food-app run failed because the model emitted its full
subtask design wrapped in a single JSON array literal:
[
{ "id": "filterChips-root", "_parent": null, "type": "frame", … },
{ "id": "chip-1", "_parent": "filterChips-root", … },
…
]
The previous \`looksLikeJsonl\` gate only checked
\`startsWith('{')\` so this fell through to the DSL parser, which
tried to read \`[\` / \`{\` / \`}\` each on its own line as DSL
operations. Every line was rejected, the subtask returned empty,
the orchestrator retried with minimal skills, that timed out too,
and the user got a single-frame placeholder with one section
instead of the full screen.
Extended the gate to accept \`[\` as the leading character. The
shape signature stays the same (\`_parent\` or PenNode \`type\`
key inside the first 800 chars) — the bracket check just
disambiguates from real DSL. \`parseJsonlToTree\` already handles
both shapes via brace-counting (it scans for \`{...}\` blocks and
ignores surrounding \`[\`, \`]\`, and \`,\`), so the apply path
needed no changes.
Exported \`looksLikeJsonl\` for direct unit testing. 6 new tests
cover: pure JSONL match, JSON-array match (the M2.7 case),
array with leading whitespace, DSL-style assignment lines reject,
empty/non-bracketed reject, and bracketed-but-no-PenNode-keys
reject (so we don't reroute legit non-design array operations).
Verified by temporarily reverting the gate to just \`{\`: the two
new array tests correctly fail.
Drives the three-OS CI matrix verification of the skia-safe + glutin +
glow + winit dep stack per Step 1a spec §7.
- examples/p0_probe.rs: stencil_visibility + readback chain runner (must
own a real OS main thread because winit on macOS rejects
EventLoop::new() from cargo test worker threads).
- tests/p0_probe.rs: subprocess-invoke wrapper, gated
#[ignore = "P0_PROBE_GATE"] so default cargo test stays untouched.
- Cargo.toml: add transient [target.'cfg(not(target_arch = "wasm32"))'.
dev-dependencies] block (skia-safe 0.97 + glutin 0.32.3 + glutin-winit
0.5.0 + glow 0.17.0 + raw-window-handle 0.6.2 + scopeguard 1.2.0 +
winit defaults). Pinned to versions resolved in /tmp/skia-glow-probe.
- .github/workflows/rust-check.yml: install Linux GL prereqs (xvfb,
mesa, libxkbcommon, libwayland) and add a P0-probe-gate step running
cargo test --ignored on each OS (Linux through xvfb-run; Windows
early-returns per spec §8.2 WINDOWS_GPU_DEFERRED_NO_RUNNER).
All three artefacts are TRANSIENT — reverted in a follow-up cleanup
commit after CI is green and the loader-compat notes commit lands.
Task 1 owns the permanent integration.
Image #44 banner shipped with the "Order now" button cut in half:
\`featured-promo-card { role: 'card', height: 165, clipContent: true }\`
held a vertical content stack (badge + title + body + button) whose
natural height was ~220px on the model's wrapped column width. The
card role default sets \`clipContent: true\` to keep image children
inside rounded corners, so the overflow got rendered then clipped at
y=165, making the bottom row of content disappear.
New \`expandOverflowingFixedHeightCards\` post-pass:
- Walks the tree.
- For each frame whose \`role\` is in CARD_ROLES (card, stat-card,
pricing-card, feature-card, image-card, testimonial, event-card,
product-card) AND \`height\` is a positive number AND
\`fitContentHeight(node) > height\`, switches \`height\` to
\`'fit_content'\`.
- Returns true if any card was patched.
Why fit_content, not removing clipContent: clipContent is what makes
nested image children respect the card's rounded corners. Removing
it would un-clip the button (good) but un-clip the image edges (bad
— image bleeds past the card's corner radius). Just letting the
card grow keeps both invariants right.
Also wired in: \`design-canvas-ops.ts\` calls the new pass right
after \`injectMissingNavSurfaceFill(pageRoot)\` in the streaming /
dispatcher post-pass chain. The card-overflow fix runs ONCE per
post-pass invocation on the page root, so all card-family children
on the page get checked together.
Side fix: button role default for tab-style buttons (parent role is
bottom-tab-bar / tab-bar / tab-row AND layout='vertical') now
returns \`padding: [6, 4], gap: 4\` instead of falling through to
the text-button \`[12, 24]\` default. Only affects the case where
the model omits padding on the tab cell — sub-agents that emit
explicit padding still win (per applyDefaults' missing-only rule).
5 new tests cover: banner-style overflow gets fit_content, fitting
content stays at fixed, non-card roles never get touched, already
auto-sizing cards stay alone, and overflow detection walks into
nested sections.
Two visible regressions in Image #44:
1. Bottom nav reverted to no-background even though earlier runs
worked. GPT-5.5 wrapped its bottom nav in a single-child section:
root > frame{role:'section',id:'bottom-tabs-root'}
> frame{role:'bottom-tab-bar'} > [tabs]
The inject pass only walked DIRECT children of root and bailed on
the section wrapper. Now we hop one level when the wrapper is a
single-child section AND its sole child is a nav-role frame, so
the nested nav gets the surface fill + position-aware shadow.
Multi-child sections still bail (those are real content sections,
not wrappers).
2. Banner "Order now" CTA shipped with white text + dark icon. My
prior contrast fix used a luminance-delta threshold of 0.4, but
#0F172A icon vs #F97316 (orange accent) actually has delta 0.48
— the threshold said "good contrast, leave it alone" while the
user sees an obvious mismatch with the white text label.
Wrong axis: the user's complaint is about CONSISTENCY (icon
should read as the same token as text), not raw contrast.
Refactored fixButtonForegroundContrast:
PASS 1 — find a "reference" foreground from sibling text fill
(after refs resolve). The model's own text color is the
authoritative signal for what the button's foreground should
look like, regardless of what bg/fg luminance suggests.
PASS 2 — for each icon_font sibling, override when its
resolved hex differs from the reference fg. Icon-only
buttons (no text sibling) fall back to a luminance-based
check at threshold 0.5 — catches dark-on-dark / light-on-
light pairs that motivated the original rule, without the
false-negative on saturated mid-luminance bgs (orange).
3 new tests: wrapper-section nav reach, multi-child wrapper bail,
and the regression test for the original "dark-on-dark icon-only
button" still passing under the new luminance-fallback path.
141 tests in the affected suites all green.
Side effect: applyNavSurfaceFill now bails entirely (returns false)
when the nav already has a fill — earlier version still added a
shadow even when fill was preserved, which violated the
"preserves sub-agent intent" semantics the existing tests rely on.
Image #42 logs showed every image fetch URL came out wrapped twice:
http://localhost:3000/api/local-asset?path=%2Fapi%2Fai%2Fimage-proxy%3Furl%3D...
The image-search pipeline correctly returned
\`/api/ai/image-proxy?url=...\` thumbUrls (so browser fetches go
through the dev server, which can reach openverse via the system
proxy). But \`isLocalAssetPath\` only excluded \`data:\`/\`https?:\`/
\`blob:\` from local-asset bridging — anything else, including
absolute paths starting with \`/api/\`, was treated as a file-system
asset and re-wrapped through \`/api/local-asset?path=\`. That bridge
handler then 404s because the encoded path \`/api/ai/image-proxy?...\`
isn't a real file. Net effect: every search-found image stayed at
the placeholder visual even though the search succeeded.
Add a same-origin route carve-out:
/^\/(?:api|_)\//
Paths under those prefixes are runtime endpoints (Nitro \`/api/*\`,
Vite \`/_/*\`), not file system assets, so they pass through the
resolver as-is. \`/assets/hero.png\` and similar absolute file-style
paths still go through the local-asset bridge.
New regression test covers /api/ai/image-proxy, /api/local-asset,
and /_/* paths returning false from isLocalAssetPath, and verifies
ordinary /assets/... paths still return true.
Codex flagged: the previous version cleared the AbortController
timeout in a finally{} block right after \`await fetch()\`, but
fetch() resolves as soon as the response headers arrive — the
body read happened later in the \`reader.read()\` loop with no
timeout protection. An upstream that drip-feeds bytes (or stops
mid-stream) would leave the dev server hanging on
reader.read() forever.
Single AbortController + timeout now covers the entire request
lifecycle (DNS + TLS + headers + body). The clearTimeout moves to
the outer finally{} so it fires regardless of return path
(success, 4xx, 5xx, abort) but never AHEAD of the body read.
Side benefit: AbortError thrown by the controller's timeout (or
by the size-cap controller.abort()) now lands in the catch clause
with a distinguishable error.name === 'AbortError'. Translate it
to a 504 Gateway Timeout when the timeout was the cause, so the
caller can distinguish a slow-upstream from a generic
fetch failure (502).
Codex flagged: the previous version did
\`Buffer.from(await upstream.arrayBuffer())\` which buffers the
entire upstream body into memory with no upper bound. Wikimedia
Commons originals can be 100 MB+, and a malicious request could
point at any arbitrarily-large file on an allow-listed host (an
upstream big enough to OOM the dev server is reachable behind
plenty of legitimate-looking URLs).
Hard 16 MiB cap on every proxied response:
- Read upstream's declared Content-Length first; reject (413) if
it already advertises more than the cap, before reading a single
byte.
- Stream the body via \`getReader()\`, accumulate in chunks, and
bail (cancel reader, abort fetch, return 413) the moment total
bytes cross the cap. Subsequent chunks are never buffered.
- Move the timeout from \`AbortSignal.timeout(15000)\` to a manual
AbortController so the same controller can also abort on
size-limit hit.
16 MiB sits well above any reasonable thumbnail and even high-res
4K JPEGs (~5–8 MiB), but well below the territory that risks
heap pressure from a single fetch.
Image #40 logs showed three "Failed to load image" errors for
api.openverse.org/...thumb/ URLs even though the search-pipeline
successfully fetched URLs from openverse via the dev server (the
HTTPS_PROXY fix from 3a6f8480 routes server-side fetches through the
local proxy). The browser-side image loader doesn't go through the
same dispatcher: \`new Image(); img.src = url\` does a direct
browser fetch that ignores HTTP_PROXY env vars, so on a machine that
requires the proxy to reach openverse.org the canvas paints the
placeholder visual even though the search-pipeline already found a
valid image URL.
New endpoint \`/api/ai/image-proxy?url=<encoded-url>\`:
- Proxies image bytes through the dev server.
- Reuses \`configureProxyDispatcher\` so the upstream fetch routes
through HTTPS_PROXY (same path as image-search).
- Allow-lists known image hosts (openverse, wikimedia, flickr's
static CDN) to prevent the dev server being used as an open
proxy. Unknown hosts get 403.
- Forwards Content-Type and Cache-Control from upstream.
- Sets Access-Control-Allow-Origin: * so canvas readback works.
\`mapOpenverseResult\` and \`mapWikimediaPages\` now return thumbUrl
wrapped via \`viaImageProxy(externalUrl)\`. The browser fetches
\`/api/ai/image-proxy?url=...\` (same-origin, no proxy needed),
the server fetches the upstream (with proxy), bytes flow back,
the canvas paints the photo. Test expectations updated to assert
the proxy wrapper.
Net effect: with HTTPS_PROXY set, image search end-to-end (search
results found AND images actually load in canvas) works on
proxy-required dev machines. With no proxy env var set
(production / CI), the cascade is still well-behaved — proxy
dispatcher is a no-op, server fetch is direct, no proxy wrapping
is necessary but it doesn't hurt either (the endpoint just adds
a hop).
Codex flagged: the previous fix (b3180534) read
\`doc.themes[SEMANTIC_PALETTE_THEME_AXIS]\` as the dark-mode
discriminator, but \`seedDocVariablesFromStyleGuide\` — the only
production writer of theme-related doc state — writes ONLY
\`doc.variables\`, never \`doc.themes\`. So on a real
orchestrator-emitted dark-mode design the axis is empty, the test
\`modeAxis[0] === 'Dark'\` is false, and the cascade still served
the LIGHT palette. The "production" dark-mode signal was wired to
nothing.
Fix reads the active page root's fill via \`detectThemeFromNode\`, the
same heuristic \`resolveTreeRoles\` uses at its entry point to set
\`ctx.theme\` for role defaults. The page root's fill is what the
model / user actually painted as the page background, regardless
of whether any themes axis was ever populated, so it's the
production-truthful signal.
New \`detectActivePageMode()\` helper reads the doc store + canvas
store's activePageId, finds the first frame on that page, and
runs \`detectThemeFromNode\`. Defaults to 'light' when no page root
exists or it has no fill — same conservative bias as before.
Test updated to seed a dark page-root fill on the live doc store
(replaces the prior \`themes['Mode']\` axis seed which never
matched production state). Also explicitly nulls the themes axis
to confirm the fill-based detection is the active code path.
Codex flagged: the previous cascade (965e143e) gated step-2 mode
selection on a `themeHint` parameter that no caller actually
supplied, so every dark-mode generation with an unseeded
\$color-accent button got served the LIGHT palette hex
(#2563EB blue) instead of the DARK one (#60A5FA light blue).
With identical luminance assumptions in both branches the
contrast pass picked the wrong fg for genuinely dark-mode docs.
`resolveColorMaybeRef` now drops the unused themeHint param and
reads `doc.themes[SEMANTIC_PALETTE_THEME_AXIS]` ('Mode' axis)
directly. First value of that axis === 'Dark' → step-2 uses the
dark palette; otherwise light. The function is self-contained,
no plumbing required at call sites, and the fallback honors
whichever mode the doc currently advertises.
New regression test seeds doc.themes={ Mode: ['Dark', 'Light'] }
with no doc.variables, asserts that an unseeded \$color-accent
button gets the dark-palette accent (#60A5FA, lum ≈ 0.6) and
therefore the dark fg color (#0F172A) on the text child. If
step-2 had stayed locked on Light, the test would expect #FFFFFF
and fail.
Codex flagged the previous fix (1c08ac3f, "skip on unresolved ref")
as too conservative: a sub-agent's button with `\$color-accent` bg
and a text child WITHOUT a fill ended up with no fill at all when
doc.variables hadn't been seeded yet — text falls back to black,
which is invisible on a dark accent button.
Better fix: extend `resolveColorMaybeRef` with a step-2 fallback
into the built-in semantic palette (`getSemanticPaletteHex`). Every
core token (`color-accent`, `color-bg`, `color-text-primary`, etc.)
has a known hex for both `Light` and `Dark` modes, so the cascade
now is:
1. Doc-seeded variables (user's palette).
2. Built-in semantic palette for the requested mode.
3. Original ref string (unresolvable) — caller bails.
In the food-app + GPT-5.5 path, step 1 already worked because
`seedDocVariablesFromStyleGuide` runs before the sub-agents.
The fallback is for paths that bypass seeding (test fixtures,
external MCP callers, mid-flight states), not the common case.
Skip-on-NaN behavior stays — it now only triggers for genuinely
unknown tokens (`\$color-foobar` or similar), where any guess is
worse than leaving the model's existing fill alone.
Tests:
- New: `\$color-accent` ref resolves to #2563EB via semantic palette
even with no doc.variables, contrast pass picks white fg correctly
for the unfilled text child.
- New: `\$color-mystery-token` (not in palette) — step-2 misses,
contrast pass skips, existing text fill survives.
- Replaces the prior "skips on unresolved" test which over-asserted
the conservative path.
Codex flagged: the previous version (ddf6580f) treated a NaN
luminance — what `hexLuminance` returns when the bg color is still
a `\$color-accent` ref because the doc's variables haven't been
seeded — as a dark bg and painted white text. If the user's palette
later resolves \$color-accent to a LIGHT hex (e.g. cream
#FFE4B5), the white text becomes invisible on the light bg. Same
risk in the inverted direction with the original code, which
defaulted to dark text on unknown bg.
Either guess can ship a visually broken button. Skip the contrast
pass entirely when we can't resolve the bg ref to a hex — text /
icon retain whatever fill they already carry, which is at least
visible (the model's default text color, usually dark). A later
post-pass invocation, after variables get seeded, re-runs and
applies contrast cleanly with a real luminance value.
`needsContrastOverride` already returns false when either luminance
is non-finite, so the icon-override branch was already safe; only
the text branch and the (now-removed) NaN→white default needed the
fix.
New regression test in role-resolver.test.ts covers the unseeded-ref
case: explicit dark text fill on an unresolvable accent button
survives untouched.
3 new cases covering the regression fixed in ddf6580f:
- dark icon on dark button → overridden to white (low contrast triggers)
- intentional brand-red icon on white button → preserved (delta > 0.4)
- unfilled icon_font on dark button → still gets contrast fill (no
regression on the original "fill if missing" branch)
User reported icons inside accent-color buttons rendering dark while
text on the same button rendered white — visible on the "Burger" tab,
the "Order now" CTA, and the round filter icon-button in the food-app
generation. Two compounding causes in `fixButtonForegroundContrast`:
1. The luminance check ran on the raw `fill[0].color` even when that
was a `\$color-accent` variable ref. parseInt('\$c…', 16) returns
NaN, NaN<0.5 evaluates to false, the dark-fg branch wins, and the
contrast pass was painting dark-on-orange. Resolve the ref via
the doc store's variables + active theme before luminance.
(NaN luminance now falls through to the white branch — better
than the previous silent dark default when resolution misses.)
2. The icon_font child path skipped any node that "already has a
visible fill". Models reflexively stamp `icon_font.fill` to a
dark text color (the prompt lists `fill` as a property), so on
an accent button the contrast pass left the dark icon next to
the now-white text. Split the icon_font branch off the text
branch: when the icon already has a fill, RESOLVE it and check
contrast against the (resolved) bg; if the luminance delta is
below the WCAG-graphical threshold (0.4), override with the
contrast fg.
Why not unconditionally override icon_font fill: an intentional
brand-color icon on a near-white button (red notification dot, blue
brand mark) has a contrast delta well above 0.4 and survives. Only
the dark-on-dark / light-on-light pairs that motivated the bug get
rewritten.
`text` and `path` branches keep their original behavior — text is
where models intentionally express accent colors, and path stroke
icons get filled by the existing stroke-fallback path.
Two of five food-app placeholder images shipped unfilled because
Openverse returned `[]` for the model's 3-keyword queries:
- "burger combo fries" → 0 results
- "sakura sushi platter" → 0 results
The same queries truncated to the first two words have plenty:
- "burger fries" → 240 results
- "sushi platter" → 240 results
Openverse uses strict AND-search across all keywords, so a 3-word
query that includes any low-frequency or non-matching token
zero-results even when the photos exist. The skill prompt already
nudges models toward "2-3 English keywords" but they often pick three
when the brief mentions a third descriptor (e.g. "Tasty BURGER COMBO
fries" → "burger combo fries").
Endpoint now cascades:
1. Openverse with full query.
2. If `[]` and query has > 2 words: re-query with first 2 words.
3. If still nothing usable: fall through to Wikimedia (existing path)
with the same 2-word retry safety net.
Returning the original empty result was wrong: the placeholder stays
unfilled even though a satisfactory photo for "burger fries" was one
keyword-trim away. The trade-off is losing a small amount of relevance
on the dropped 3rd keyword — but that's better than no photo at all,
and the model still drives the first two keywords which carry the
core subject.
Visible regression in the food-app screenshot: the top-left avatar
(44×44 orange circle, role='button', single child text 'A') rendered
with the 'A' visibly off-center. Live doc inspection showed
`padding: [12, 24]` on the avatar — 24px horizontal padding × 2 = 48
exceeded the 44px frame width, the layout engine clamped to the
negative content area, and the centered text ended up shifted off
the visual center of the orange circle.
Root cause: the default 'button' role rule unconditionally returns
`padding: [12, 24]`, which fits a typical text-button (~44 tall × wide
enough for label + horizontal pad). When the model emits role='button'
on what's actually an avatar / icon-action shape (small square, single
short child), the same default collides with the small fixed width.
Fix: in the default branch (no special parent-role context), inspect
the node's explicit width/height. When BOTH are numeric AND ≤ 60 AND
the default 24px horizontal padding wouldn't fit (`width < 24*2`), skip
the text-button defaults entirely — return only the layout / centering
shape (no padding, no cornerRadius, no height). The 60px ceiling is
the standard icon-button / avatar size band; the 24*2 fit-check keeps
the full text-button defaults active for everything wider than ~48px.
Why no cornerRadius default in this branch: avatar-style frames
typically supply `cornerRadius: width/2` themselves to get a circle.
The 8px text-button default would silently override it.
Note: applyDefaults only fills missing properties, so this only
affects nodes the model didn't explicitly stamp padding on. Models
that DO emit padding stay untouched (correct — the model is the
source of truth when it commits to a value).
Visible regression in the GPT-5.5 food-app run: the bottom nav
shipped with no surface fill (floating icons on the cream root
background), even though `injectMissingNavSurfaceFill` was wired in
and verified to add a fill on top-level nav-role frames. Live doc
inspection showed `bottom-tab-bar` carrying `fill: undefined`
post-generation — the inject pass simply never ran.
Root cause: `orchestrator-sub-agent.ts` runs the dispatcher branch
(Strategy A `<op_tool>` element-tools AND Strategy B JSONL-in-
batch_design) and **early-returns before** reaching
`applyPostStreamingTreeHeuristics(rootId)` further down in the
function. That post-pass is what runs:
- normalizeStrokeFillSchema
- unwrapFakePhoneMockups
- resolveTreeRoles + resolveTreePostPass
- normalizeTreeLayout
- stripRedundantSectionFills
- injectMissingNavSurfaceFill
- publish (forcePageResync)
Skipping it on the dispatcher path means EVERY sub-agent that emits
via element tools or JSONL fallback bypasses role resolution, layout
normalization, redundant-fill stripping, AND nav-surface injection.
The streaming path was the only branch that fired the cleanup.
Fix: call `applyPostStreamingTreeHeuristics(subtask.parentFrameId ??
plan.rootFrame.id)` right before the dispatcher branch returns, when
at least one node was inserted. The post-pass walks up to the page
root via `getParentOf()` for the inject step, so passing the section
root that the dispatcher inserted into is correct.
This also un-blocks several heuristics that depend on the full
subtree being in the store: button width / frame height equalization,
clipContent on cards-with-image-children, and theme detection on the
sub-agent's root (which feeds icon/text color defaults).
Previous version did `require('undici')` inside a try/catch on the
theory that would let it run on both CJS and ESM. In Vite/Nitro's dev
path the helper loads as an ESM module, where `require` is undefined —
the call threw `ReferenceError: require is not defined`, the catch
block silenced it, `configured=true` still got flipped, and every
subsequent call short-circuited. Net effect: the proxy was never
installed in the very dev environment the fix was meant to repair, so
image-search kept ECONNREFUSED-ing on Openverse + Wikimedia and
landing zero filled placeholders.
Switch to a static `import { setGlobalDispatcher, EnvHttpProxyAgent }
from 'undici'`. undici is a transitive dep of h3 in this workspace
(verified resolved in node_modules), and it's also the package Node 18+
uses internally for fetch — pinning it as a direct dep on apps/web
makes the resolution intentional rather than reliant on the h3 chain.
Also swapped the hand-rolled ProxyAgent for undici's built-in
`EnvHttpProxyAgent`: it reads HTTPS_PROXY / HTTP_PROXY / NO_PROXY
itself (case-insensitive) and applies the no-proxy bypass list, which
saves us from re-implementing those rules.
Verified with both `bun -e` (workspace deps) AND a direct ESM Node
context: with HTTPS_PROXY set, `fetch(api.openverse.org/...)` now
returns 240 results for "salmon sushi" instead of the earlier
ECONNREFUSED. The "configured" guard still makes calls idempotent so
multiple endpoints can opt in without coordinating.
Root cause for all-blank-placeholders on the food-app brief: Node's
native fetch (used by the Nitro dev server's image-search endpoint)
ignores the system proxy by default. On machines that route outbound
HTTPS through a local proxy (clash / mihomo / corporate gateway —
mine sits at 127.0.0.1:7897), every Openverse + Wikimedia call from
the server silently ECONNREFUSEDs. The endpoint's catch block returns
`null` for Openverse → falls back to Wikimedia → that ECONNREFUSEDs
too → returns `[]`. Browser shows zero filled images.
Direct curl from the same machine uses HTTPS_PROXY automatically, which
is why a manual API check (e.g. `curl https://api.openverse.org/...`)
returned 240 results for "salmon sushi" while
`/api/ai/image-search?query=salmon%20sushi` returned `{results:[]}`.
`apps/web/server/utils/proxy-dispatcher.ts::configureProxyDispatcher`:
- Reads HTTPS_PROXY / https_proxy / HTTP_PROXY / http_proxy.
- If set, installs `undici.ProxyAgent` as the global fetch dispatcher
via `setGlobalDispatcher`. From that point on every server-side
`fetch()` routes through the proxy.
- Idempotent — multiple endpoints can call it without re-installing.
- No-op when no proxy env var is present (production / CI).
- Dynamic `require('undici')` so a build target that strips undici
doesn't crash at import time.
Wired into `image-search.ts` at module top so the dispatcher is
configured before the first request lands. Other endpoints making
external fetches can opt in with the same single-line call.
Verified standalone via Bun: with the helper in place,
`fetch('https://api.openverse.org/v1/images/?q=salmon+sushi')` returns
240 results. The dev server itself needs a restart to pick up the
server-side change (Vite server-code HMR doesn't re-evaluate Nitro
modules).
Previous fix left `role: 'image-placeholder'` on the frame even after
its fill was swapped to `[{type:'image', url, mode:'crop'}]` — the role
is what makes "this slot is meant to hold a photo" semantics survive
into history / codegen / downstream tooling, so stripping it would
trade one regression for another.
But that meant any follow-up generation (which calls
`resetImageSearchQueue` to clear `queuedNodeIds`) would re-walk the
tree, re-enqueue the same placeholder via role match, and overwrite
the already-good photo with whatever the next search returned.
`isUnfilledImagePlaceholderFrame` now gates every read: role match AND
fill is not already `type: 'image'`. Used in three places:
- `collectImageSearchTargets` only collects unfilled placeholders.
- `enqueueImageForSearch` early-returns if the caller passes an
already-filled placeholder (defense-in-depth for direct callers).
- `processQueue`'s re-check uses it instead of a plain
`isImagePlaceholderFrame`, so even a stale queue entry from before
someone else filled the frame gets dropped.
4 new tests in image-search-pipeline.test.ts cover the predicate
(default solid fill = unfilled; missing/empty fill = unfilled; image
fill = filled; non-placeholder role = always false) and a regression
test in `collectImageSearchTargets` that keeps the already-filled
placeholder out of the result while still picking up its sibling.
The `add_image_placeholder_v0` / `_v1` element tools and JSONL payloads
that mimic them emit a `frame` carrying `role: 'image-placeholder'` (a
gray slate-100 box + centered icon_font child + optional label) — NOT
an `image` node. The auto-search pipeline only filtered on
`type === 'image'`, so every placeholder produced via element tools
silently bypassed the search hook. Latest GPT-5.5 food-app run shipped
8 placeholder frames; zero got auto-filled and the design landed with
all dashed-border icons instead of real photos.
Pipeline now:
- `isImagePlaceholderFrame` predicate identifies placeholder frames.
- `collectImageSearchTargets` returns mixed `{node, kind}` pairs
('image' for `type==='image'` with placeholder src, 'placeholder-frame'
for the role-keyed frames). Skips descending into placeholder
children (icon_font + label get wiped on fill anyway).
- `enqueueImageForSearch` accepts both shapes; queue items track `kind`.
- `processQueue` re-checks the right invariant per kind, and on success
uses `updateNode(id, { fill: [{type:'image',url,mode:'crop'}], children: [] })`
for placeholder frames (vs `updateNode(id, { src })` for image nodes).
Clearing children prevents the icon/label from rendering on top of
the searched photo.
- Streaming path (insertStreamingNode line 382) intentionally still
gates on `type === 'image'` — placeholder frames stream their
children separately, so enqueueing mid-stream would race with the
late-arriving icon. Placeholder frames are only enqueued via the
post-tree `scanAndFillImages` scan (orchestrator-tail + dispatcher
per-subtask), where the full tree is already in the doc.
`extractQueryForNode` looks for `imageSearchQuery` first, falls back
to a non-default `name`, then mines the optional
`role: 'image-placeholder-label'` text child for a hint. Generic
default still works ("placeholder") if nothing useful is on the frame.
7 new tests cover `isImagePlaceholderFrame` and
`collectImageSearchTargets` (placeholder + image mix, no descent into
placeholder children, missing root id).
The previous "navbar in PROTECTED_ROLES" change was Codex-flagged as a
no-op: PROTECTED_ROLES only PREVENTS strip-pass deletion of an existing
fill, it doesn't ADD one. The actual food-app brief failure was that the
sub-agent emitted a bottom navigation row WITHOUT any fill at all,
relying on the parent surface for visual contrast — but the parent (the
cream root frame) doesn't supply that contrast, so the nav blends
straight into the cream background and visually disappears.
New deterministic pass: `injectMissingNavSurfaceFill`. For each direct
child of the page root whose role is one of {navbar, nav, tab-bar,
bottom-tab-bar, top-nav-bar, top-app-bar, tab-row} AND whose fill is
empty/missing, set `fill = [{type: solid, color: '$color-surface'}]`
so the renderer resolves it through the seeded palette and the nav
gets a visible white surface separation from the cream root.
Scope contract:
- Only direct children of the passed root frame (page root). Nav frames
nested inside cards / sections / banners are left alone.
- Never overrides an existing fill — sub-agent intent (e.g. an
intentionally dark `top-app-bar`) is preserved.
- Pure mutation; returns `true` when any nav was patched.
Wired into the same hook point as `stripRedundantSectionFills` (via
`design-canvas-ops.ts::generationCleanup`), so every generation cycle
sees both a strip pass (remove hedge fills) and an inject pass (add
the missing nav surface). Five new tests cover all nav role variants,
preservation of existing fills, scope (no recurse into cards), and
no-op on unrelated roles.
Two related issues from the GPT-5.5 food-app run:
1. Bottom navigation rendered without its surface fill, blending into
the cream root background. The strip-redundant-section-fills pass
didn't have any of the navigation roles (`navbar`, `nav`, `tab-bar`,
`bottom-tab-bar`, `top-nav-bar`) in PROTECTED_ROLES, so a navbar
carrying `fill: #FFFFFF` (or any SAFE_LIGHT tint) hit the
"safe-light hedge" branch and got stripped. Real-world navs
intentionally use a white surface to separate from a tinted root —
that fill is intended, not a hedge.
Fix: add the five navigation role names to PROTECTED_ROLES. New
test asserts a `role: navbar` frame with `fill: #FFFFFF` on a
`#FFF8F0` cream root keeps its fill.
2. Empty-src image placeholders inserted by the dispatcher's JSONL
fallback only got auto-filled at the orchestrator's tail (line
~1219, after every subtask completes). On a long brief that's a
visible lag; on an aborted/throwing brief the tail never runs and
images stay placeholder forever.
Fire-and-forget `scanAndFillImages(parentId)` from the dispatcher's
applied path so each subtask's image set starts searching as soon
as it lands. The orchestrator-tail scan still runs and dedups
through `queuedNodeIds`, so this is purely a latency / robustness
improvement (no double fetch).
`store.addNode(null, …)` routes the insert through `_children()` →
`getActivePageChildren(doc, activePageId)` — meaning the parent list
is the ACTIVE PAGE's children, not `doc.children`. The previous
append-index calc read `doc.children?.length` directly, which only
holds the legacy single-page fallback array. On a multi-page doc the
two diverge: `doc.children` may be empty or stale while the active
page already has N siblings, so the computed append index doesn't
correspond to the actual insertion target — landing either before
existing siblings (off-by-N) or out of bounds.
Use `getActivePageChildren(document, activePageId)` to read the same
list `addNode` writes into. Sub-agent generation runs on whichever
page the user has active, so this matches dispatch behavior exactly.
The non-null parent path (`getNodeById(parentId)` then read its
children length) was already correct — only the null-parent branch
needed fixing.
`store.addNode` defaults to `index: 0` (prepend) — appropriate for new
shapes a user draws on canvas (topmost in z-order), but wrong for
sub-agent generation where each subtask emits a section that should
appear AFTER the previous subtask's output in document order.
The previous JSONL fallback called `addNode(parentId, root)` without
an explicit index, so every subtask's output prepended to the previous
ones. Result: the last subtask landed first in the root frame's children
and the first subtask was pushed to the bottom. Real-world repro:
a food-app brief with sections [status-bar, header, search, categories,
banner, popular, recommended, bottom-nav] produced [recommended,
popular, banner, header, search, status-bar, categories, "what are
you craving", bottom-nav] — same nodes, reversed order.
Compute the parent's current `children.length` for each insert and
pass it as the explicit index so roots append at the end of the
parent. Order matches subtask iteration order, matches the brief.
The default-prepend behavior of `addNode` is unchanged for other
callers (drawing tools, paste, etc) — only this dispatcher path
overrides it.
parseJsonlToTree resolves `_parent` via a `Map<id, node>` that's
overwritten on duplicate ids. A JSONL line whose `_parent` equals its
own `id` (or otherwise references a node that ends up being itself
after the map overwrite) produces `node.children = [node]` — a cyclic
graph. Without a cycle guard, `collectIds` would recurse forever
walking node → node.children[0] → node → … and never surface the
duplicate or fail the dispatch.
Two-part guard:
1. Reference-identity `WeakSet` (visitedRefs) — short-circuits the
recursion the moment we re-enter the same node object.
2. Early return after pushing a duplicate id — once we've recorded
the dup, descending into its (potentially cyclic) subtree adds no
information.
Either guard alone would prevent the stack overflow; together they
make the dup-detection robust against any malformed input shape that
parseJsonlToTree might produce.
Prior precheck only compared the JSONL payload's ids against the live
doc — it didn't detect duplicates within the payload itself. A model
emitting the same id twice (two roots sharing an id, a nested child
reusing a root id, etc) would still slip past: addNode appends both
copies, getNodeById returns the first match for both verifies, and a
later rollback removeNode would delete only one of the two duplicates,
leaving an orphan with the same id in the doc.
Detect internal duplicates during the same id-collection pass:
collectIds tracks `treeIds` as a Set and pushes any id seen twice into
`internalDuplicates`. If non-empty, return `failed` immediately with
the duplicate list — same shape as the existing live-doc collision
branch, no doc mutation, no side effects, retry path takes over.
`insertNodeInTree` does not dedupe — it appends. So if the model emits
a JSONL root whose id collides with a pre-existing live node, the doc
ends up with two nodes sharing that id. `getNodeById(root.id)` returns
the FIRST match (the pre-existing one), making the post-insert verify
look successful even though the new node was appended elsewhere. A
later rollback `removeNode(id)` then deletes the PRE-EXISTING node
instead of the duplicate, corrupting the doc.
Precheck: collect every id the JSONL tree introduces (roots and
descendants). If ANY of them already exists in the live doc, refuse to
insert and return `failed` immediately — doc state is preserved, no
rollback needed, the orchestrator's retry path takes over.
Once we know all ids are fresh, the existing post-insert verify and
rollback paths are safe: every id we touch was provably absent before
the dispatch, so `removeNode(id)` targets only what we just added.
Previous "failed with non-empty insertedNodes" combination still bypassed
retry. orchestrator-sub-agent.ts gates retry on `result.nodes.length === 0`
— the partial inserts surfaced through DispatchResult.insertedNodes
flowed through to the subtask's `nodes` field, made it look non-empty,
and skipped the retry / minimal-skills / batch_design fallback chain.
Hard-rollback partial inserts on JSONL fallback failure: call
`store.removeNode(id)` for every root that did land, then return
`failed` with `insertedNodes: []`. The dispatcher's surrounding
history-batch wrapper absorbs both the addNode and removeNode calls so
the user-visible undo entry is a net no-op, and the retry condition
upstream now sees a genuinely empty result and re-runs the subtask
cleanly.
Three outcomes after this:
- All roots land → `applied` with full insertedNodes.
- Partial / total failure → `failed` with `insertedNodes: []` (any
partial successes rolled back) so retry fires and the doc returns to
its pre-dispatch state.
Previous version reported `status: 'applied'` whenever at least one root
landed, with a partial-failure note in `message`. But the orchestrator's
retry / minimal-skills / batch_design-fallback chain checks
`status === 'applied'` to decide whether to bypass retry — a partial
insert (e.g. 1/5 roots landed because `defaultParentId` was stale)
would short-circuit retry and leave the user with a degraded design
that the system never tried to fix.
Now any failed root flips the dispatch to `status: 'failed'` so the
orchestrator's retry path can take over. The successful partial inserts
are still surfaced in `insertedNodes` so the surrounding history-batch
wrapper can roll them back / clean up — `failed` with non-empty
`insertedNodes` is a legitimate combination meaning "side effects
happened but the dispatch did not complete its contract".
Three outcomes now:
- All N roots land → `applied` with full count.
- 1..N-1 land → `failed` with partial-success `insertedNodes` and a
message naming the parent id + failed root ids.
- 0 land → `failed` with empty `insertedNodes` and the same diagnostic.
Previous JSONL fallback called `store.addNode(defaultParentId, root)`
and reported `status: 'applied'` regardless of outcome. But `addNode`
returns void and silently no-ops via `insertNodeInTree` when the parent
id can't be resolved (stale `defaultParentId`, empty doc, etc). The
caller would then count the dispatch as a successful insert even
though the doc was unchanged.
Verify each root via `getNodeById(root.id)` immediately after addNode.
Outcomes now:
- All roots land → `applied`, message lists count.
- Some land, some don't → `applied` with partial-failure note in
message; only the live roots are returned in `insertedNodes`.
- No roots land → `failed` with diagnostic naming the parent id and the
first few failed root ids — caller surfaces this to the orchestrator
retry path instead of silently absorbing the loss.
Mid-tier models (observed: GPT-5.5 standard tier in web-app CLI mode)
correctly emit `<op_tool>{name:"batch_design",arguments:{operations:...}}`
when the brief doesn't fit any embedded element tool — Strategy B in
ELEMENT_TOOL_OUTPUT_FORMAT. But the prompt only declares the operations
value as `<DSL_STRING>` without showing the DSL syntax, so models stuff
flat JSONL (`{"_parent":null,"id":"…","type":"frame",…}`) into the
`operations` field instead of `foo=I("parent",{…})\nbar=U(foo,…)`.
The browser DSL executor then rejects every line ("Cannot parse
operation: …"), all retries fail, and the user sees a degenerate result
(303B / 1 node) despite the model having streamed a full design.
Detect at dispatch time: if `operations` looks like JSONL (starts with
`{` AND contains a `_parent` key or a typed PenNode shape near the top),
route through `parseJsonlToTree` + `store.addNode(defaultParentId, root)`
loop instead of the DSL parser. Same dispatch invariants (single
history batch, dispatch result accounting) apply.
This unblocks the most common Strategy B failure: model emits JSONL
inside a `batch_design` envelope. Strategy A (per-component element
tools) and DSL-shaped Strategy B both still go through their existing
paths unchanged.
The previous fix dropped jsonl-format / jsonl-format-simplified entirely
when elementToolsEnabled was true, on the theory that their CRITICAL
"Output ONLY ```json … Do NOT use tool calls" line conflicted with the
appended `<op_tool>` instruction. But empirically dropping them made
weak-model output WORSE: MiniMax-M2.7 still emits raw JSONL most of the
time (it can't reliably emit `<op_tool>`), and without the JSONL
schema/format teaching its output degrades — role coverage dropped
from 74% to 22%, color-ref% from 84% to 49%.
The right fix is dual-mode coexistence: keep BOTH skills loaded so the
model has the JSONL fallback teaching, but rewrite each skill's CRITICAL
opener to defer to the ELEMENT_TOOL_OUTPUT_FORMAT block when present.
- jsonl-format / jsonl-format-simplified now lead with: "If a separate
OUTPUT FORMAT — EMIT AS TOOL CALL(S) block appears later in the system
prompt, FOLLOW THAT block. Use the JSONL form below ONLY when no
<op_tool> instruction is present."
- Removed the orchestrator-sub-agent.ts skill-filtering branch; both
skills load unconditionally now.
Net effect: strong models that can follow `<op_tool>` will use the
element-tool path (preserving the n-tools-per-element design intent for
weak-model stability — MiniMax/GLM/Kimi will emit `<op_tool>` when they
can). Weak models that fall back to raw JSONL still get the schema /
sizing / fill / token rules they need to produce coherent output. No
forced choice, no degraded fallback.
The whole point of the n-tools-per-element design is stability for weak
models in the BUILT-IN AGENT path (MiniMax / GLM / Kimi). But empirically
no element tool was firing on that path — the model emitted raw JSONL
and bypassed every `<op_tool>` strategy.
Root cause: when `elementToolsEnabled` is true, the prompt mixed two
incompatible output-format instructions:
- jsonl-format / jsonl-format-simplified — early in the prompt, leads
with `CRITICAL: Output ONLY ```json. Do NOT use [TOOL_CALL] or
{tool => ...} syntax.`
- ELEMENT_TOOL_OUTPUT_FORMAT — appended at the end, says `Respond
with one or more <op_tool> tags, nothing else.`
Weak models anchor on the early CRITICAL ("Do NOT use tool calls"),
read `<op_tool>` as a forbidden tool-call form, and silently fall back
to raw JSONL. Result: every brief on basic tier with element tools
enabled bypassed the whole element-tool surface — which is the opposite
of the design intent.
Fix: when `elementToolsEnabled` is true, drop both `jsonl-format` and
`jsonl-format-simplified` from `resolvedSkills`. ELEMENT_TOOL_OUTPUT_FORMAT
becomes the sole output-format instruction. Content rules (schema /
layout / text-rules / overflow / icon-catalog / elements) stay loaded.
This was P5/ab-v8's blind spot: the test harness either ran on standard
tier (no jsonl-format-simplified swap) or didn't observe `<op_tool>`
emit rate directly, so the conflict masked real-world failure on basic
tier in the built-in agent path.
The planner output frequently contains BOTH `styleGuideName` (catalog
pick) and `styleGuide.palette` (AI's hallucinated palette) — and the
two often disagree. Empirically MiniMax / GLM gravitate to indigo
`#6366F1` for the accent regardless of what catalog snippet they were
just shown: the model picks 'warm-food-mobile-light' (orange catalog),
copies the cream background `#FFF8F0` correctly, then invents
`accent: #6366F1` for `plan.styleGuide.palette`.
The previous seedDocVariablesFromStyleGuide preferred
`plan.styleGuide.palette` first and only fell back to
`plan.selectedStyleGuideContent` when the AI palette was missing — so
the catalog accent was always overridden by the AI's invented one.
Result: every brief seeded indigo, no matter how good the ranking and
catalog match upstream were.
Swap the priority: catalog content (designed by humans, high
confidence) wins; AI-generated palette is the fallback when no catalog
content was attached. The planner's catalog choice is preserved
(`plan.styleGuideName`) so visible UX is unchanged for that signal —
just the COLORS now come from the catalog rather than the model's bias.
The previous wallet-app exclusion list only had singular forms ('gift
card' not 'gift cards', 'coupon' not 'coupons') and missed common
membership/loyalty card variants. So briefs like 'wallet app for gift
cards' / 'wallet app for coupons and discounts' / 'wallet app for
membership cards' still routed to a fintech style guide despite being
generic Apple-Wallet contexts.
Extracted the exclusion list into APPLE_WALLET_CONTEXT and added:
- gift card → gift card(s) (singular OR plural)
- coupon → coupon | coupons
- membership / membership card(s)
- punch card(s) — restaurant loyalty cards
- stamp card(s) — coffee shop loyalty cards
- vaccination card(s) — pandemic Apple Wallet pass type
Verified with 12 representative briefs:
- All 7 plural/card-variant briefs now fall back to neutrals
- Singular forms (gift card, coupon) keep their existing fallback
- Real fintech briefs (generic wallet app, send money, crypto wallet) keep
triggering fintech
The previous fix removed 'wallet app' from the fintech phrase list to
stop Apple-Wallet-pass briefs from being routed to a fintech style guide.
But that swung too far: bare 'design a wallet app' or 'wallet app to
send money' are common fintech briefs that don't carry a 'crypto'/
'digital'/'payment' modifier and now fell back to generic neutrals.
Hybrid rule: 'wallet app' triggers fintech UNLESS the brief also mentions
an Apple-Wallet-style context word (pass / passes / boarding / ticket /
tickets / ticketing / gift card / coupon / loyalty). Real fintech briefs
that center on a wallet app rarely use any of those words; Apple Wallet
briefs almost always do.
Verified:
- 'design a wallet app' / 'wallet app to send money' / 'wallet app with
QR code support' → fintech ✓
- 'Apple Wallet app for boarding passes' / 'wallet app pass viewer' /
'wallet app to store concert tickets' / 'wallet app for loyalty
cards' → neutral fallback ✓
- crypto/digital/payment wallet, wallet payment(s), wallet connect,
budget tracker — unchanged ✓
The previous wallet-pass fix only removed 'wallet pass' but left
'wallet app' in the fintech phrase list. That still routes Apple-Wallet
contexts like "Apple Wallet app for boarding passes", "wallet app pass
viewer", or a bare "wallet app" brief to a fintech style guide — none
of which want banking aesthetics.
Restrict wallet right-side triggers to phrases that are unambiguously
fintech: 'wallet payment(s)' and 'wallet connect'. Real fintech briefs
that center on a wallet almost always qualify it ('crypto wallet app',
'payment wallet flow', 'digital wallet onboarding') and those still
trigger via the left-side modifier list.
Verified:
- Apple Wallet app passes / wallet app pass viewer / generic wallet app
all fall back to neutrals (no fintech force)
- crypto wallet / crypto wallet app / digital wallet / payment wallet /
wallet payment(s) / wallet connect all still trigger fintech
- Other fintech (budget tracker, crypto trading) unchanged
The previous commit re-added 'wallet pass' to the fintech phrase list
along with 'wallet app' / 'wallet payment' / 'wallet connect'. But
'wallet pass' specifically is the generic Apple Wallet feature for
boarding passes, event tickets, gift cards, and vaccination cards —
none of those are fintech UI briefs and forcing a fintech guide makes
the design come out banking-styled when the user wanted a clean ticket
or boarding-pass layout.
Removed 'pass' from the wallet right-side phrase list. The other three
('wallet app', 'wallet payment', 'wallet connect') are still
unambiguously fintech briefs.
Verified:
- 'Apple Wallet pass for an event ticket' falls back to neutral guides
- 'wallet pass for a boarding pass' falls back to neutrals
- 'crypto wallet', 'wallet app', 'wallet payment' still trigger fintech
Removing 'wallet' / 'budget' / 'expense' / 'api' / 'dev' wholesale to
fix generic-UI over-trigger swung the regex too far the other way: real
fintech and developer briefs that legitimately use these words as their
primary signal lost their domain guide.
Apply the same contextual two-word pattern that 'code' uses to bring
them back without re-introducing the over-trigger:
Finance phrases:
- (crypto|digital|payment|hot|cold|hardware|web3) wallet
- wallet (app|pass|payment|connect)
- (budget|expense) (tracker|app|report|management|manager|tracking)
Developer phrases:
- code (editor|review|repo|repository|completion|snippet|base) [kept]
- api (console|platform|portal|docs|documentation|reference|sdk|gateway|playground|key|keys)
- dev (tool|tools|portal|experience|environment|console|platform)
- (developer is already in the unconditional standalone list)
Verified with 15 representative briefs:
- All 8 Codex-flagged regressions (crypto wallet / digital wallet /
budget tracker / expense tracker / API console / API docs / dev tools
/ developer portal) now hit a fintech or developer guide in top-4.
- 4 generic UI checks (settings menu / expense form / API integration in
fintech / Apple Wallet pass) still fall back to neutrals or the
contextually-correct guide instead of forcing a wrong one.
- Food / wellness / modernist briefs unchanged.
The previous over-correction-recovery commit kept synonyms a bit too
generously and re-introduced over-trigger problems Codex flagged:
- 'menu' would force a food guide on every \"settings menu\" / \"side
menu\" / \"dropdown menu\" brief.
- 'api' would force a developer guide on every brief that mentions API
integration (fintech, ecommerce, etc).
- 'dev' would force a developer guide on any tech context.
- 'wallet' would force a fintech guide on Apple Wallet passes / generic
iOS wallet UI features.
- 'budget' / 'expense' would force a fintech guide on every form that
tracks costs (project mgmt, travel apps, design feedback).
- 'mint' / 'brass' / 'sage' would force color tags on common English
phrases (\"mint condition\", \"brass instrument\", \"sage advice\").
Fix: remove all of those from the unconditional domain keyword lists.
'code' is the special case worth preserving — it IS the most-defining
single word for a developer brief — but it has too many non-dev uses
(QR code, promo code, area code, country code) to match unconditionally.
Replaced with a contextual two-word match: 'code' followed immediately
by editor / review / repo / repository / completion / snippet / base
triggers the dev tag. \"QR code\" / \"promo code\" do not.
Verified:
- 5 generic UI/tech briefs no longer force a domain guide (top 4 falls
back to alphabetical neutrals).
- 'code editor' and 'code review' still match developer-terminal-dark.
- Food / wellness briefs unchanged from prior fix.
The previous ranking fix added \b boundaries to fight substring traps
(\"Featured\" → red, \"Healthy\" → wellness in a food category list),
but in the process I dropped several exact domain keywords that were
NOT substring traps and that legitimate briefs use:
- \"code\" (developer brief: \"code editor\", \"VS Code app\") — was
silently removed; now restored as `\\bcode\\b` so it matches the
standalone word but still won't trip on \"decoder\" / \"encode\".
- \"health\" / \"healthy\" (wellness brief: \"design a healthy
lifestyle app\") — was lost; restored as `\\bhealth\\b` /
`\\bhealthy\\b`. The food-category-list \"Healthy\" still matches
too, but that's a smaller harm than missing genuine wellness briefs
— and the rest of the ranking fix (industry tag weight 30, platform
mismatch -30) keeps mobile food guides above desktop wellness guides
even when both are tagged.
Also restored derivative forms that earlier substring matches caught
by accident (modern → modernist/contemporary, luxury → luxurious,
brutal → brutalist/brutalism, minimal → minimalist) and broadened
each domain block with common synonyms so we don't regress brief
coverage on real prompts:
- food: + menu, diner, kitchen, dining, eatery, cafe/café
- finance: + trading, wallet, crypto, budget, expense
- developer: + api, engineering, dev (alongside restored code)
- wellness: + wellbeing, spa, gym, exercise, workout (alongside
restored health/healthy)
- accents: each color block expanded with common synonyms
(orange→peach/amber/tangerine, blue→navy/sapphire/cobalt,
green→emerald/sage/mint, gold→golden/brass, red→ruby).
Verified end-to-end with 5 representative briefs:
- Food brief still puts warm-food-mobile-light in top-4
- \"Healthy lifestyle\" wellness brief now picks wellness-green-mobile
- \"code editor\" developer brief picks developer-terminal-dark
- Modernist brand picks ecommerce-modern-light
Two ranking bugs were silently sending mobile food/wellness/fintech briefs
to a desktop landing-page palette:
1. Substring tag inference. /red|red/ matched 'Featured', /health/ matched
'Healthy' (a category in the food brief), so a food prompt picked up a
spurious 'wellness' tag and a desktop wellness guide jumped above the
mobile food guide via tag-overlap math. Added \b word boundaries to
every English keyword in inferTagsFromPrompt; CJK rules unchanged
because \b doesn't apply.
2. Industry vs style tag weighting + platform mismatch penalty. Each
matched tag was worth +10 regardless of meaning, and a platform
mismatch was a tiny -3 vs +0. So a desktop ecommerce-modern guide
beating mobile warm-food on the same brief was just `clean+modern+
rounded` overlapping more than `warm-tones+friendly+rounded` while
the platform penalty was negligible.
Now: industry tags (warm-tones / wellness / fintech / developer /
monospace) score 30, generic style tags 10, platform mismatch -30.
Empirically pushes warm-food-mobile-light to the top of the food
brief shortlist (verified with the actual expanded prompt that the
user's MiniMax-M2.7 run logged).
Same fix applies to every brief that was getting "wrong palette" results
because the planner snippets only contain the top-4 ranked guides — if
the right answer falls past 4, the planner literally never sees it and
the model invents its own (default-blue) palette.
Also: jsonl-format-simplified.md (basic-tier sub-agent prompt) now mirrors
jsonl-format.md's design-system-tokens teaching — basic-tier models like
MiniMax-M2.7 currently emit 0% typography refs because the simplified
prompt doesn't mention $type-* refs at all. The expanded simplified
prompt is 3981 chars, well under the bumped budget=1700 (=6800 char cap).
CRITICAL contract moved to top-of-file as the same defense-in-depth
pattern applied earlier to jsonl-format.md.
Previous fix flattened rx to 0 for every rotated subtree clip — but
that's only necessary at off-axis angles. At rotations that are right-
angle multiples (0°, 90°, 180°, 270° — and any 90° period), a rotated
rrect remains an rrect with w/h possibly swapped, and the AABB of the
rotated corners equals the rotated shape exactly. The rounded corner
survives the projection and should be preserved.
Compute `angleMod90 = ((angleDelta % 90) + 90) % 90` and keep the
original rx when the result is within tolerance of 0 or 90 (true
right-angle rotation). Otherwise (45°, 30°, etc) the AABB of the
rotated rrect is strictly larger than any rrect we can encode, so we
fall back to rx=0 as before.
Real-world impact: 99% of in-app rotation gestures (and any snap-to-15°
ergonomic shortcut applied to a 90° pivot point) keep the rounded
corners visible during preview instead of squaring off.
The rotate-preview path was projecting subtree clipStack entries through
\`rotatePreviewRect\` — but that helper only rotates the rect's CENTER and
keeps the original w/h axis-aligned. For non-zero angles that places the
clip rectangle in a wrong scene location: it's neither the original
position nor a faithful representation of the rotated bounds.
ClipInfo is axis-aligned by construction, so the only correct scene-coord
representation of a rotated clip is its AABB (the bounding box of the
4 rotated corners). Slightly over-clips along the rotated rect's diagonal
but is correct along its axes — and matters most for the common case
(target is rotated mostly in 0/90/180/270 increments where AABB == rect).
Also drops \`rx\` to 0 for rotated entries: the AABB of a rotated rrect
is a rectangle with no faithful rrect approximation, so a rectangular
clip is the safest fallback.
Added \`rotatedAABB\` helper alongside the existing \`rotatePreviewRect\`.
The bounds-rotation behavior of subtree nodes is unchanged (still uses
center-translated rect for absX/absY/absW/absH, since canvas.rotate
handles paint rotation regardless of bounds form).
Previous fix froze ALL of a descendant's clipStack during resize/rotate
preview — but that was over-conservative. The first N entries of every
subtree-RN's clipStack come from ancestors of the resize/rotate target
(unchanged, freeze ✓), but entries from index N onward were pushed by
the target itself (when it has clipContent: true) or by its clipContent
descendants — those ARE inside the transforming subtree and must scale /
rotate alongside the rest of it. Otherwise children appear clipped at
the target's pre-transform bounds even though they're rendered at the
new bounds.
Use rootSnapshot.clipStack.length as the boundary: indices < N stay
frozen (ancestor clips), indices >= N get the same scale-from-sourceRect
or rotate-around-center transform as the rest of the subtree. The scale
factors and rotation parameters match the bounds transforms exactly
because every clip pushed inside the subtree was anchored to a node
whose bounds are also being transformed.
The drag handler stays fully frozen: drag is multi-target with no
notion of a single subtree boundary, and the dominant case (single-
frame drag without clipContent ancestors in the drag set) is correct
under freeze. A precise drag fix would need entry-to-source-id mapping
on RenderNode, which is a separate refactor.
A node's `RenderNode.clipStack` carries the ancestor clip chain, NOT this
node's own bounds. The previous interaction handlers transformed every
entry of clipStack alongside the node's own absX/absY/absW/absH:
- drag: translated each entry by (dx, dy)
- resize: scaled each entry alongside the resize delta
- rotate: rotated each entry around the rotation center
That's wrong — the ancestor frames being referenced by those entries
aren't being dragged/resized/rotated, so their clip rectangles on screen
shouldn't move. The visible result was the clip rectangle drifting away
from the actual ancestor during preview.
Fix: in all 4 sites (drag mutation loop, resize root preview, resize
children iteration, rotate children iteration), restore the snapshot
clipStack unchanged (deep-cloned so caller-mutation can't leak back into
the snapshot). The node's own pushed clip — if it has clipContent: true —
lives in its CHILDREN's clipStack, which the children's flatten will
recompute on commit. Brief preview artifact only when the node being
transformed has clipContent and its children are simultaneously visible
during preview, which is acceptable for an in-flight gesture.
Test updated: dragged node's clipStack now stays at its snapshot value.
Single ClipInfo can't faithfully encode `(rrect ∩ rrect)` whenever one rect
cuts inside the other's corner. The previous fix collapsed nested clips
into one ClipInfo and dropped one side's rounded corner — which meant a
rounded modal containing rounded cards would silently lose either the
modal's rounding or the card's rounding at paint time.
Fix: replace the single `RenderNode.clipRect: ClipInfo | undefined` with
`clipStack: ClipInfo[]`. Flatten time accumulates a stack from outer-most
ancestor down to the immediate clip-introducing parent. Paint time pushes
each entry as its own canvas.save+clipRect/clipRRect — Skia's clip stack
intersects them naturally, so each level's rounded corner is enforced
independently.
Touched:
- types.ts: export ClipInfo, replace clipRect with clipStack
- document-flattener.ts: thread `clipStack: ClipInfo[]` through recursion;
push to a copy when isRootFrame || explicitClip
- node-renderer.ts paint: loop over clipStack, push N save+clip ops, pop
the same N at the end
- renderer.ts (root frame label loop) + skia-engine.ts (root frame label
loop) + focus-fit.ts (auto-fit excludes clipped descendants) +
global-export.ts (page bounds): all check clipStack.length instead of
truthy single field
- skia-interaction.ts: drag/resize/rotate snapshots store and restore
clipStack arrays (deep-cloned per entry)
- Tests updated + 1 new test: rounded modal containing rounded card
preserves both rrects on the inner content's clip stack
`store.removeVariable` walks `doc.children` via `replaceVariableRefsInTree`
and rewrites every node that references the deleted variable. That's the
right behavior for explicit user removal but the wrong behavior for the
orchestrator's seed/rollback dance — those are meant to swap "ambient"
palette tokens between briefs WITHOUT touching node structure. If the
rollback fired with the user's doc already containing nodes that
referenced one of the 7 plan-derived names (carryover from a prior brief,
manual ref, etc), the rollback would silently null those refs and break
the user's existing colors.
Fix:
- New `patchDocVariables` applies a name → def|undefined patch via direct
setState — bypasses the variable actions and their tree walk.
- `seedDocVariablesFromStyleGuide` and the new `rollbackPlanDerivedVariables`
helper both use it, so both paths are node-safe.
- The 3 inline rollback blocks (catch + Phase 4 throw + Phase 4 abort)
now share the helper.
Trade-off: seed and rollback no longer push a per-key history entry. They
still mark `isDirty: true`, and when the orchestrator runs in animated
mode the whole brief is wrapped in a startBatch/endBatch so the variable
swap rolls into one undo step regardless. In the non-animated path the
swap is a single transaction (one setState) instead of 7 — cleaner.
The previous rollback only fired in the catch block — but executeSubAgents
can also resolve cleanly with zero content (sub-agent caught its own abort
or returned empty), in which case the function falls through to Phase 4
without throwing. The existing line `if (generatedNodeCount === 0 &&
!aborted) throw …` deliberately swallows the empty-but-aborted case so
Stop-clicks render as cancelled-not-errored — but it left doc.variables
permanently mutated with the plan's palette.
Now both branches of the `generatedNodeCount === 0` check run the same
restore-from-snapshot logic before either throwing (non-aborted) or
returning quietly (aborted). The catch-block rollback for hard failures
stays as is.
Updated the structural pattern test to allow the new rollback block (~900
chars of code+comment) to live between the zero-count check and the throw.
Without rollback, a brief that fails (network error, abort signal, parse
error, etc) before any sub-agent content lands permanently mutates the
user's doc.variables with the plan's palette — they see no design, but
their token state is now polluted with whatever style guide the planner
picked. Next brief on the same doc inherits this stale palette.
Fix: snapshot the 7 plan-derived variable names BEFORE seedDocVariables-
FromStyleGuide writes them, and in the existing catch block restore the
snapshot iff no content survived (every root frame got cleaned up). With
partial content surviving (a salvageable half-design) we keep the seeded
palette so existing $color-* refs in those nodes still resolve correctly.
design.md flows still bypass this entirely; user-set variables outside
the plan-derived 7 are untouched on both seed and rollback.
Prior version skipped seeding when doc.variables had any entry, which left
the second brief in a session resolving `$color-*` refs to the FIRST brief's
palette while the prompt's ref + (hex) instruction advertised the new one.
Fix:
- Define PLAN_DERIVED_VARIABLE_NAMES (the 7 v1 token names this function
manages) and always (re-)write them from the current plan.
- Stale-key guard: if the new palette doesn't carry a name (e.g. ai-generated
plan.styleGuide has no textMuted but a prior catalog brief seeded one),
remove the stale entry so refs fall back to the default palette consistently
with the new prompt.
- design.md path bypasses this function entirely; user-set variables outside
the 7-key set are never touched.
Two-part fix for the web-app chat path (both built-in and CLI mode go through
sub-agent JSONL output, not MCP tool calls — `jsonl-format.md` line 50 forbids
tool calls). Without this, every fill in generated designs was a hex literal
even after the 5/3 design-system-aware work — the 188 v1 element tools and
DEFAULT_PALETTE_FALLBACK were dead weight here.
A. STYLE GUIDE injection now uses ref + (hex) double form so the model sees
`$color-accent` paired with the resolved hex it represents:
Before: - Background: #FFF8F0 Surface: #FFFFFF
After: - Background: `$color-bg-deep` (resolves to #FFF8F0)
- Surface: `$color-surface` (#FFFFFF)
Applied to both `buildSubAgentStyleGuideInstruction` (selectedStyleGuideContent
path) and the inline `plan.styleGuide` injection in orchestrator-sub-agent.ts.
B. `seedDocVariablesFromStyleGuide` runs once before sub-agent execution: when
`doc.variables` is empty AND a style guide is selected, it maps the palette
to v1 token names (`color-bg-deep` / `color-accent` / etc) and seeds them
into `doc.variables`. This makes refs emitted by the model resolve to the
user's chosen palette at render time instead of falling back to the default
#2563EB blue.
A and B are coupled — A alone would make designs render in the wrong color
(every design becomes blue regardless of style guide); B alone leaves the model
mimicking the hex from the prompt. Both must ship together.
Also:
- jsonl-format.md: DESIGN SYSTEM TOKENS section + example fills converted to
refs + CRITICAL contract moved to top (so future budget overruns can't
truncate it). Budget bumped 1500 → 1700 for safety margin.
- elements.md: Theme handling section now flags MCP path vs JSONL path so
models on either path know which guidance applies.
Adds theme-aware v1 builders for icon_button, image_placeholder,
inbox_message, inline_action, input_with_action, invite_row, kbd,
legend_item, link, and list_row. Group A (zero-color: icon_button,
link, list_row) — no hardcoded colors in v0, all three modes identical.
Group B (kbd) — key bg → surface2, stroke → border in dark/system.
Group C (remaining 6) — surface/text/border/accent/alertColors tokens
applied in dark/system modes, full byte-parity with v0 in light mode.
Extends ext-6 shard (357→647 lines, within 800-line ceiling) housing
all 20 batch-4 + batch-5 tool schema definitions. All 9 touchpoints
wired per playbook: builder, index.ts, pen-core barrel, handler,
dispatcher, ext-6 shard, client shim, server builder, elements.md entries.
Verified: format:check clean, tsc --noEmit clean, 4086/4086 tests pass.
Adds theme-aware v1 builders for cookie_banner, data_table_row,
date_picker, drawer_shell, empty_state, event_card, fab, faq_item,
filter_group, and form_field. Group A (zero-color: empty_state,
form_field) — no hardcoded colors in v0, all three modes identical.
Group B (fab) — accent bg is brand-invariant, maps to accent token
in dark/system; icon stays white in all modes. Group C (remaining 7)
— surface/text/border/accent tokens applied in dark/system modes, full
byte-parity with v0 in light mode.
Creates ext-6 shard (ext-5 was at 798-line ceiling) housing all 10
new tool schema definitions (357 lines). All 9 touchpoints wired per
playbook: builder, index.ts, pen-core barrel, handler, dispatcher,
ext-6 shard, client shim, server builder, elements.md entries.
Verified: format:check clean, tsc --noEmit clean, 4076/4076 tests pass.
Adds theme-aware v1 builders for chart_bars, chart_line, chart_pie,
chat_bubble, checkbox, chip_input, code_block, color_swatch, combobox,
and comment. Group A (chart tools) maps bar/line color to chart-1 token
and pie default palette to chart-1..6 tokens in dark/system modes.
Group B (color_swatch) is theme-invariant — swatch color is caller-
supplied and passes through unchanged. Group C (chat_bubble, checkbox,
chip_input, code_block, combobox, comment) resolves surface/text/border
via semantic palette tokens. All light modes are byte-parity with v0.
Adds theme-aware v1 builders for alert, bottom_nav, breadcrumb,
activity_ring, carousel_dots, action_menu, attachment_row,
calendar_grid, avatar_group, and callout. Group A (zero-color:
alert/bottom_nav/breadcrumb/activity_ring) produce identical output
across all three theme modes. Group B (carousel_dots) maps active=
text-primary, inactive=border in dark/system modes. Group C (action_menu/
attachment_row/calendar_grid/avatar_group) resolve surface/text/border via
semantic palette. Group D (callout) maps tone-keyed bg/fg to alert palette
tokens in dark/system modes. All light modes are byte-parity with v0.
avatar-v1, badge-v1, divider-v1, body_text-v1, icon_label-v1 — each with
full 9-touchpoint coverage (pen-core builder + index + pen-mcp handler +
schema shard + dispatcher + apps/web shim + SERVER_BUILDERS + parity test
+ elements.md). Light mode is byte-equal to v0; dark/system modes produce
identical output since all 5 tools emit zero hardcoded color fills — theme
param accepted for API consistency across all v1 tools. New shard
element-tool-defs-ext-5.ts created (ext-4 was at 739 lines). All 2026
pen-core + pen-mcp tests pass; format:check + tsc clean.
card_row-v1, setting_row-v1, member_row-v1, activity_log-v1 — each with
full 9-touchpoint coverage (pen-core builder + index + pen-mcp handler +
schema shard + dispatcher + apps/web shim + SERVER_BUILDERS + parity test
+ elements.md). Light mode is byte-equal to v0; dark/system use resolveTheme()
for all color fills. activity_log-v1 maps tone×theme to alertColors tokens
(info/success/warning/danger) with neutral falling back to surface/textMuted.
All 3998 tests pass. Completes P2 representative phase.
Task 2.3 — representative v1 tool walkthrough for Plan 14 byte-parity contract.
Light mode is byte-equal to add_heading_v0 (V0_LATIN_PRESETS table reused);
dark/system modes use resolveTheme() for fill color and typography token refs.
Adds theme enum [light, dark, system] to add_heading_v1 MCP schema, elements.md
decision tree, shim-server-parity CASES, and SERVER_BUILDERS. All 3937 tests pass.
Codex stop-time review caught the next inconsistency: f5d9a29c
switched the orchestrator to tryParseAllElementToolOutputs +
dispatchElementToolCalls, but the plural parser cheerfully returned
both element-tool AND batch_design shapes side by side. A
non-compliant model (saw this on minimax-m2.7 in the ab-v4
search-filters composite — 3 element tools + 1 batch_design
scaffolding tag) would slip the forbidden mixed strategy through
the dispatcher, applying the element calls AND the batch_design
DSL together — exact thing the prompt forbids and exact thing the
ab-corpus output-parser silently rejects on the harness side.
Aligns the production parser with corpus output-parser.ts: when
ANY element-tool tag is present in the response, batch_design tags
are DROPPED. Pure Strategy B (no element-tool tags, only batch_design
fallback) keeps working — the drop only fires on mixed output.
Two new regression tests:
- mixed input → only element-tool shapes returned
- pure batch_design input → batch-design-dsl shape returned
3769 vitest pass (+2), format clean, tsc silent. Together with
f5d9a29c and 1a14a6c2, production now has prompt + parser + dispatch
all consistent with the Strategy A/B contract — no path can smuggle
mixed output past any of them.
The Strategy A prompt I shipped in 1a14a6c2 invites the model to
chain N op_tool tags ("settings panel with 4 toggle rows is 5 tool
calls"), but the orchestrator sub-agent was still calling the SINGULAR
tryParseElementToolOutput → dispatchElementToolCall path, which
silently kept only the first tag. A composite-T response with 5 tags
would render only the section header and drop the 4 setting rows on
the floor — exact thing the prompt promises won't happen. Codex
stop-time review caught it.
design-parser and element-tools-dispatcher already had the plural
counterparts (tryParseAllElementToolOutputs, dispatchElementToolCalls)
plumbed end-to-end with one history batch wrapping the whole loop.
Switches the orchestrator to use those.
Failure handling: BatchDispatchResult exposes per-shape DispatchResult
in `results`. When status != 'applied' we concatenate the failed
shapes' messages tagged by toolName so the UI's diagnostic preview
shows which tag(s) broke instead of a generic "dispatch failed".
Partial successes still surface their inserted nodes through
onApplyPartial — the user sees what landed, plus an error summary
naming the broken pieces.
3767 vitest pass, format clean, tsc silent. End-to-end: web app
chat / orchestrator now actually realizes the multi-tool gain my
1a14a6c2 prompt change advertised.
The orchestrator sub-agent's ELEMENT_TOOL_OUTPUT_FORMAT was still
running the pre-Codex-fix wording from before today's ab-corpus pass:
- "Respond with one <op_tool> tag, nothing else"
- "Do not combine multiple tags"
Same self-defeating prompt that gave ab-v3 0/25 composite multi-tool
runs. Production code path stayed broken while the harness kept
getting fixed. Caught when investigating ab-v4's gpt-5.4
search-filters garbage — orchestrator-sub-agent's leading comment
explicitly says it's kept verbatim against the ab-corpus version.
Aligns with the latest scripts/ab-corpus/build-prompt.ts version:
- "Respond with one or more <op_tool> tags" (multi-tool allowed)
- STRATEGY A — element tools, one tag per component, with a 3-tag
worked example
- EMBEDDED COVERAGE — production-specific block listing the subset
of add_*_v0 tools the embedded orchestrator can actually execute,
inserted between Strategy A and Strategy B (the ab-corpus harness
has full coverage so it doesn't need this block)
- STRATEGY B — single batch_design covering the whole response when
any component falls outside EMBEDDED COVERAGE
- Explicit "Do not mix Strategy A and Strategy B" guard, naming the
parser's silent-drop behavior
design-parser.ts::tryParseElementToolOutput already collects every
`<op_tool>` tag into tool_calls (line 61: `parsed.kind === 'tool_calls'
&& parsed.calls.length > 0`), so the multi-tool path works end-to-end
on the production parser side too — no parser change needed.
3767 vitest pass, format clean, tsc silent. Real-user impact: web app
chat / orchestrator runs against minimax / glm / kimi / deepseek now
get the same multi-tool teaching that took composite routing from
0% to 42% in ab-v4.