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.
