Codex stop-hook #2: with the hidden IME textarea now focused (R1
fix in fe994c4b), every keystroke routes through it — including
arrow keys the user is pressing to navigate the IME's candidate
picker. The window-level keydown listener still fires on these
keystrokes, and `DropdownState::apply_key` was mutating selection
+ opening the menu behind the IME panel. Surfaced as: "focused
IME textarea lets composing keys mutate dropdown state."
Spec §2.4 says: "Widgets that consume keys directly should
usually skip dispatch when is_composing == true and let the
ImeEvent path handle the composition instead." The plumbing for
`is_composing` already runs through Phase C2.2 (W3C
`KeyboardEvent.isComposing` → C1 `map_keyboard_parts(...,
is_composing)` → `KeyEvent.is_composing` → C2.1
`DropdownState::apply_key`); we just weren't honoring the bit on
the consuming side.
Fix: add `event.is_composing` to the early-return condition in
`DropdownState::apply_key`. ArrowDown/Up/Enter/Escape during a
composition no-op now; the IME's candidate picker keeps the
keystroke and the dropdown stays put.
`TextInputState::apply_ime` is unaffected — it already only
processes ImeEvent, never KeyEvent, so composing-key bleed-
through was never a concern there.
Test: `dropdown_apply_key_ignores_keys_during_ime_composition`
asserts both ArrowDown (would advance + open) and Enter (would
close) are no-ops when `is_composing` is true. Test count
20 → 21.
Verification:
- `cargo test -p openpencil-shell-core --test widgets_static` —
21/21 passing
- `cargo check -p openpencil-shell-core --target
wasm32-unknown-unknown` — green
- `cargo build -p openpencil-shell-web --target
wasm32-unknown-unknown --features skia --release` — green
- `bash tools/check-wasm-bundle.sh` — PASS:
- 0 env.* imports
- 622 156 bytes gzip = 59% of 1 MiB ceiling
Phase D may extend this guard pattern to other widgets that gain
key handling (Tree typeahead, etc.); the spec §2.4 is_composing
contract becomes a per-widget invariant.
Codex stop-hook surfaced after the Phase C gate GO: window-level
composition listeners were processing IME activity from ANYWHERE on
the page (URL bar, devtools search, any other editable element)
as if it were directed at the inspector's TextInput state. Without
an owned editable target, the IME wiring was technically reachable
end-to-end but semantically incorrect.
Fix: create a hidden `<textarea>` in `mount()`, append to
`document.body`, programmatically focus it, and register the 3
composition listeners on it instead of `window`. The textarea is:
- styled `position:fixed; left:-9999px; top:0; width:1px;
height:1px; opacity:0; pointer-events:none;` so it does not
visually intrude
- `aria-hidden="true"` so screen readers ignore it
- `tabindex="-1"` so Tab-traversal skips it
`focus()` is best-effort (returns Err if the document is not yet
visible — e.g. background tab); the page user gives focus on the
first interaction. Once focused the textarea owns IME composition
contexts; only compositions targeted at it reach the inspector.
Keyboard listeners stay on `window` — Cmd+S / Tab / arrow shortcuts
should fire regardless of which element has focus, and the
stop-hook concern was specifically about IME, not keyboard.
Plumbing changes:
- `WebShell` gains `ime_target: web_sys::HtmlElement` field. Drop
calls `self.ime_target.remove()` after unregistering listeners
so leaving the page does not leave an orphan node + the browser
does not ship dead composition state into the next document.
- `add_listener` is now generic over the target type
`T: Clone + Into<EventTarget>`; the helper clones into an owned
EventTarget once for the registration call + Listener cleanup
storage. Call sites pass `&win_target` (T = EventTarget) for
keyboard listeners and `&ime_textarea` (T = HtmlElement) for IME
listeners without an explicit cast.
- The partial-registration unwind path also calls
`ime_textarea.remove()` so a failed mount does not leave an
orphan node behind (Phase C gate Round 1 BLOCK fix from earlier
is preserved + extended).
Verification:
- `cargo build -p openpencil-shell-web --target wasm32-unknown-
unknown --features skia --release` — green
- `cargo check -p openpencil-shell-web --target wasm32-unknown-
unknown --no-default-features --features web` — green
(compile guard)
- `bash tools/check-wasm-bundle.sh` — PASS:
- 0 env.* imports
- 622 149 bytes gzip = 59% of 1 MiB ceiling (+1 KiB vs C-gate
R5 — hidden-textarea creation + remove paths cost ~1 KiB of
web-sys glue)
- `cargo test -p openpencil-shell-web --test dom_event_mapping`
— 25/25 (mappers are pure; this fix is mount-side glue and
doesn't touch the mappers)
- `bash tools/check-widget-boundary.sh` — PASS
Phase D will land focus management for arbitrary widget chrome
focus + the Reflect-based getTargetRanges() lookup that the IME
selection pipeline still owes; for Phase C / Step 1b the hidden
textarea is the correct simplest scope.
Two fixes surfaced by the Phase C gate review iterations:
# C-gate R1 BLOCK: partial listener registration not exception-safe
Five `add_listener?` calls in `mount()` were sequenced via the
question-mark operator. If registration #K returned `Err`, the
K-1 already-landed listeners would never be unregistered:
WebShell never reaches `Ok(WebShell { ... })` so its `Drop`
never runs, and the partial `listeners` Vec drops without
calling `remove_event_listener_with_callback` first — the
browser-held DOM callbacks then outlive their Closures, which
is dangling-listener UB at the wasm-bindgen boundary.
Fix: wrap all 5 registrations in an inner closure that consumes
`&mut Vec<Listener>` and returns `Result<(), JsValue>`. On Err
we drain the partial vec and unregister each listener with the
same body Drop uses, then return the error. Comment cites the
codex finding inline.
# C-gate R1 CONCERN + R3 BLOCK: incomplete named-key coverage
`map_key_value` covered Enter/Escape/Tab/Space/Backspace/Delete/
Arrow{Up,Down,Left,Right} only. jian's `NamedKey` enum has 31
variants total — Home, End, PageUp, PageDown, F1..F12, Shift,
Control, Alt, Meta, and CapsLock were all falling through to
`Unidentified(<key>)`.
`map_key_code` similarly missed Home, End, PageUp, PageDown, and
the 8 modifier physical codes (ShiftLeft / ShiftRight / etc).
Fix: extended both tables. After the fix, every NamedKey jian
exposes round-trips through map_key_value, and every
non-Unknown KeyCode variant round-trips through map_key_code
(F1-F12 stay Unknown because jian's KeyCode does NOT have F-key
variants — NamedKey covers them; comment in keyboard.rs
explains).
# Tests added (3 new round-trip tests, 22 → 25)
- `keyboard_navigation_keys_mapped` — Home / End / PageUp /
PageDown / CapsLock all map to Named(<variant>), not
Unidentified.
- `keyboard_function_keys_mapped` — F1..F12 round-trip via a
for-loop.
- `keyboard_modifier_keys_mapped_to_named_values` — Shift /
Control / Alt / Meta with both Left and Right `code` variants
assert both Named(<modifier>) on `key` and the matching
KeyCode::{Mod}{Left,Right} on `code`.
CapsLock specifically was the Round 3 NO-GO discovery — the
only NamedKey variant the Round 1 fix missed; Round 4 added it
+ the round-trip tests above.
Verification:
- `cargo test -p openpencil-shell-web --test dom_event_mapping` —
25/25 passing
- `cargo build -p openpencil-shell-web --target
wasm32-unknown-unknown --features skia --release` — green
- `bash tools/check-wasm-bundle.sh` — PASS:
- 0 env.* imports
- 621 151 bytes gzip = 59% of 1 MiB ceiling (+0.6 KiB vs
pre-fix; new key table entries are tiny)
- `bash tools/check-widget-boundary.sh` — PASS
Codex Phase C gate review: 5 rounds (gate-level, not commit-level).
Round 1 BLOCK + CONCERN, Round 2 sandbox-only BLOCK, Round 3
NO-GO (CapsLock + missing test coverage), Round 4 sandbox-only
BLOCK (source verdict clean), Round 5 GO with Phase B5 R2
precedent applied for sandbox-only build evidence.
Lands the DOM-side glue that drives Phase C2.1's apply_ime /
apply_key paths from real browser events. Inspector text input now
accepts CJK IME composition (compositionstart/update/end) and the
dropdown responds to keyboard navigation (Arrow/Enter/Escape).
Architecture:
- WebShell restructured: was `{ backend, host }`, now
`{ inner: Rc<RefCell<Inner>>, listeners: Vec<Listener> }` where
`Inner { backend, host }`. Each browser closure needs `'static`
ownership (wasm-bindgen Closure::new requirement); cloning the
Rc per closure and borrow_mut'ing on dispatch is the canonical
pattern. The `inner` field on WebShell anchors the original
ownership so the Rc isn't dropped before Drop removes listeners
(`#[allow(dead_code)]` on the field — all reads go through the
closure-captured clones).
- `Listener` struct stores
`Closure<dyn FnMut(JsValue)>` uniformly across event types; each
handler body uses runtime-checked `dyn_into::<SpecificEvent>()`
(not `unchecked_into`) so a mismatched synthetic event from
same-page JS silently skips the handler instead of producing a
wrong-type reference (codex C2.2 R1 CONCERN-3 fix).
- `add_listener<E, F>` helper registers a listener via
`EventTarget::add_event_listener_with_callback` and pushes the
Closure into the listener vec for lifetime anchoring.
- `Inner::repaint()` extracted from the old
`WebShell::paint_inspector` — every closure body calls
`inner.borrow_mut(); inner.host.apply_*(...); inner.repaint()`.
Returns Result for present errors but closure bodies use
`let _ = inner.repaint()` since closure must be infallible;
errors still surface through console_error_panic_hook on panic
paths.
- `modifiers_from_keyboard` builds Jian Modifiers from W3C
KeyboardEvent.{shiftKey, ctrlKey, altKey, metaKey}; metaKey →
CMD per spec §2.4 ("Cmd on macOS / Win key on Windows / Super
on Linux").
- Drop impl: drains listener vec, calls
`remove_event_listener_with_callback` BEFORE Closure drops so
wasm-bindgen sees a valid registration to unregister. Best-
effort — if target detached from DOM the call is a no-op.
5 listeners registered on `window` (rationale comment at the
decision point: keyboard/composition events on canvas only fire
with focus + tabindex; smoke HTML doesn't set tabindex; window-
level always fires for the static inspector demo. Phase D+ widget
chrome may parameterize the target — codex C2.2 R1 NIT-8 fix):
- `keydown` → `KeyEvent { state: Pressed }` →
`host.apply_key(...)`
- `keyup` → `KeyEvent { state: Released }` (no-op in apply_key
per C2.1)
- `compositionstart` → `host.apply_ime(&composition_start())`
- `compositionupdate` → `host.apply_ime(&composition_update(
evt.data().unwrap_or_default(), None))`. Selection currently
passes `None`; W3C `getTargetRanges()` requires `Reflect::get`
+ a manual Function call (web-sys 0.3.94 doesn't expose the
getter). Phase D's DOM mirror already needs Reflect for
accesskit::TextSelection mapping; folding the IME selection
path into that work is cleaner than adding Reflect just here.
- `compositionend` → `host.apply_ime(&composition_end(...))`
Plan-vs-implementation deviations (deliberate, all kept narrow):
- Listeners on `window` instead of canvas (focus+tabindex
avoidance — see decision-point comment).
- `compositionupdate` selection skipped pending Phase D
Reflect adapter.
- Pointer / wheel / focus listeners NOT registered yet — those
mappers exist in event/*.rs but no widget consumes them today
(Tree click→selected wiring is Phase D). Listener helper +
Drop pattern apply unchanged when they land.
- `dyn_into` (runtime-checked) instead of plan-body's
`unchecked_into` for type safety.
Verification:
- `cargo build -p openpencil-shell-web --target
wasm32-unknown-unknown --features skia --release` — green
- `cargo check -p openpencil-shell-web --target
wasm32-unknown-unknown --no-default-features --features web` —
green (compile guard)
- `bash tools/check-wasm-bundle.sh` — PASS:
- 0 env.* imports
- 620 548 bytes gzip = 59% of 1 MiB ceiling (+5 KiB vs C2.1 —
wasm-bindgen Closure infrastructure for 5 listeners; ~1 KiB
each gzipped)
- `bash tools/check-widget-boundary.sh` — PASS
Codex iterate review: 2 rounds → GO. Round 1 1 CONCERN
(unchecked_into trust) + 2 NITs (redundant guard, missing
rationale comment); Round 2 GO with 1 doc-comment NIT (stale
unchecked_into prose); both fixed in this commit.
Lands the data-flow piece of plan C2: shell-core widgets gain pure
state-mutation methods (`TextInputState::apply_ime`,
`DropdownState::apply_key`) that the WidgetHost forwards to from the
two new `// glue:` marked methods. Phase C2.2 will land the browser
closure registration that drives these methods from real DOM
events.
shell-core (widgets stay platform-agnostic per spec §1.4):
- `TextInputState::apply_ime(&ImeEvent)` — CompositionStart clears
preedit, CompositionUpdate replaces preedit with `event.text`
(selection deferred to Phase D DOM mirror), CompositionEnd
appends the commit text to value and clears preedit.
- `DropdownState::apply_key(&KeyEvent, option_count)` — ArrowDown
advances + opens (saturates at last option, no wrap), ArrowUp
retreats with `saturating_sub` + opens, Enter / Escape close
without mutating selection. Skips on Released or empty options.
shell-web (glue file widget_host.rs, both methods carry `// glue:`
markers per spec §1.4 boundary check):
- `WidgetHost::apply_ime(&ImeEvent)` — forwards to text_input.state
- `WidgetHost::apply_key(&KeyEvent)` — forwards to dropdown.state
with `dropdown.options.len()` for the option count
Tests (`tests/widgets_static.rs`, +10 → 20 total):
- text_input apply_ime: Start clears preedit (value untouched);
Update replaces preedit (value untouched); End commits +
clears; double-Start without End each clears (codex C2.1 R1
CONCERN-1 — pathological host state machine)
- dropdown apply_key: ArrowDown advances + saturates; ArrowUp
retreats + saturating_sub; Enter / Escape close + Escape
preserves selection (codex C2.1 R1 CONCERN-2); Released = no-op;
zero options = no-op; unrelated NamedKey (Tab) = no-op
- Helper `keydown(named)` / `keyup(named)` build minimal KeyEvents;
apply_key reads only `key` + `state` so the harness's KeyCode
field is intentionally Unknown(String::new())
Plan-vs-implementation deviations (deliberate):
- WidgetHost forwarding methods carry `// glue:` markers. F3 in
the boundary script's exemption set covered `fn paint(...)`;
the same exemption applies here because these methods import +
invoke `openpencil_shell_core::{ImeEvent, KeyEvent}` at the
signature line. Verified `bash tools/check-widget-boundary.sh`
still PASS.
- Codex C2.1 R1 CONCERN-3 (WidgetHost forwarding has no direct
test coverage) deferred to C2.2 — the browser closure
registration there exercises the forwarding end-to-end via
real DOM events, which is more meaningful than mocking
WidgetHost in shell-web tests with read-only accessors that
would only exist for testing.
Verification:
- `cargo test -p openpencil-shell-core --test widgets_static` —
20/20 passing
- `cargo check -p openpencil-shell-core --target
wasm32-unknown-unknown` — green (shell-core stays wasm32-clean
per spec §1.2)
- `bash tools/check-widget-boundary.sh` — PASS
Codex iterate review: 2 rounds → GO.
Lands the four pure W3C → Jian gesture mappers that Phase C2's
browser listeners will consume:
- `event:⌨️:map_keyboard_parts(key, code, location, repeat,
pressed, modifiers, is_composing) -> KeyEvent` — W3C
KeyboardEvent.key/code/location string lookups produce KeyValue
(Char / Named / Unidentified) + KeyCode enum + KeyLocation enum.
Phase C1 covers KeyA-Z, Digit0-9, Enter / Escape / Tab / Space /
Backspace / Delete / arrows; Home/End/PageUp/PageDown/F-keys/
modifier physical codes (ShiftLeft etc) extend the table in C2
(codex C1 NIT-2 deferred).
- `event::ime::{composition_start, composition_update, composition_end}`
+ `utf16_selection_to_utf8` helper. The helper walks
`text.char_indices()` accumulating `len_utf16()` to remap UTF-16
code-unit offsets (what `CompositionEvent.getTargetRanges()`
hands us) to UTF-8 byte offsets (what jian-core's `ImeKind::
CompositionUpdate { selection: Range<usize> }` requires per spec
§2.4). Mis-ordered range returns None; out-of-range bounds clamp
to text.len(). CJK (你好), surrogate pairs (🙂), mixed-encoding
(aé), zero-length selections, and empty text are all covered.
- `event::pointer::map_wheel(position, dx, dy, dz, mode, mods,
timestamp: Instant) -> WheelEvent` — flips W3C deltaY sign so
widget code reads Jian-internal positive-up. Phase C1
intentionally takes `timestamp` as a parameter rather than
calling `Instant::now()` internally; `std::time::Instant::now()`
panics on wasm32-unknown-unknown ("time not implemented on this
platform") and the C2 listener will fill the timestamp from a
polyfill (web_time::Instant). Made the mapper pure to keep the
panic locus in the listener glue, where the polyfill lives.
- `event::focus::map_focus(gained, node_id_hint,
related_node_id_hint) -> FocusEvent` — pure pass-through; the
W3C target → WidgetId correlation work lives in C2 alongside
the DOM mirror id registry (Phase D groundwork).
Tests (`tests/dom_event_mapping.rs`, 22 tests, all native):
- keyboard: 5 tests + 1 empty-string-key fallback test (codex C1
NIT-3) — key/code/location preservation, named key, location
decoding, is_composing propagation, multi-codepoint &
empty-string Unidentified fallback
- ime: 8 tests — start/update/end shapes, UTF-16→UTF-8 remap for
CJK / surrogate pair / mixed encoding / zero-length / no-selection
/ mis-ordered range / out-of-range clamp / empty text
- wheel: 4 tests — Y sign flip, X no-flip, mode decoding, deltaZ
passthrough
- focus: 2 tests — gained=true with both hints, blur with no
related target
Plumbing:
- shell-web grows a direct path-with-version `jian-core` dep.
shell-core re-exports `gesture::*` but not `geometry::*`, and
the wheel mapper builds `WheelEvent.position` from
`jian_core::geometry::Point::new(...)`. Same path-with-version
pattern as shell-core's own jian-core dep.
- `pub mod event;` is NOT cfg-gated to skia — the mappers are pure
and useful on the wasm32-clean stub baseline too.
Plan-vs-implementation deviations (deliberate, all kept narrow):
- `map_keyboard_parts` adds `is_composing: bool` parameter (jian-core
KeyEvent struct REQUIRES the field per spec §2.4).
- `map_focus` adds `related_node_id_hint: Option<u64>` parameter
(jian-core FocusEvent struct field; plan body missed it).
- `map_wheel` takes `timestamp: Instant` parameter instead of
calling `Instant::now()` internally (avoids wasm32-unknown-unknown
runtime panic; pure mapper).
- `event/pointer.rs` covers ONLY wheel; full PointerEvent mapping
(kind / phase / buttons / pressure) lives in C2 alongside listener
registration since that's where the W3C PointerEvent surface meets
the runtime context.
Verification:
- `cargo test -p openpencil-shell-web --test dom_event_mapping` —
22/22 passing
- `cargo check -p openpencil-shell-web --target
wasm32-unknown-unknown --no-default-features --features web` —
green (compile guard)
- `EMSDK=$HOME/.emsdk bash tools/check-wasm-bundle.sh` — PASS
- 0 env.* imports
- 615 616 bytes gzip = 58% of 1 MiB ceiling (no growth — event
modules dead-code-eliminated when not called)
- `bash tools/check-widget-boundary.sh` — PASS (event/ doesn't
violate F1-F4)
Codex iterate review: 1 round → GO with 1 deferred CONCERN
(Instant source for C2 — already documented inline) + 3 NITs
(deltaX comment wording, additional KeyCode entries, extra
edge-case tests). NIT-1 and NIT-3 fixed in this commit; NIT-2
deferred to C2.
Enforces the Step 1b §1.4 widget boundary invariant: widget logic
(Widget impls + layout/paint/access_node methods) lives in
crates/openpencil-shell-core/src/widgets/; shell-web's only
widget-touching file is `widget_host.rs` and even there the only
widget-method signature allowed is the `// glue:` marked paint
dispatcher.
Forward checks (no widget logic in shell-web/src/):
- F1: `impl(<...>)?[[:space:]]+(ns::)*Widget[[:space:]]+for[[:space:]]`
anywhere under shell-web/src/. Allows generic params + arbitrary
namespace depth so `impl<T> shell_core::widgets::Widget for X` is
caught. No `// glue:` exemption — Widget impls have no place in
shell-web period.
- F2: `fn[[:space:]]+(layout|access_node)\(` anywhere under
shell-web/src/. No exemption.
- F3: `fn[[:space:]]+paint\(` under shell-web/src/, EXCEPT lines in
widget_host.rs that ALSO carry `// glue:`. Tight exemption — the
marker only blesses one specific signature, not arbitrary tagged
lines.
- F4: any line under shell-web/src/ mentioning both
`openpencil_shell_core` AND `widgets`, except widget_host.rs.
Catches direct + grouped `use` forms (e.g. `use
openpencil_shell_core::{widgets::TreeWidget};`) plus path
expressions. Multi-line braced `use` is out of scope (single-line
policy in this crate).
Reverse check (shell-core/src/widgets/ has all four impls):
- R1: For each of {tree, prop_row, dropdown, text_input}, the file
must exist AND, after stripping `//` line comments, must contain
a live `impl Widget for X`. Block comments out of scope (line
comments only in this directory).
CI integration:
- New "Verify Step 1b widget boundary (spec §1.4)" step in
.github/workflows/rust-check.yml right after the existing
"Verify Jian boundary invariants" step, gated to Linux runner
(matches the jian-boundaries pattern).
- Added `tools/check-jian-boundaries.sh` and
`tools/check-widget-boundary.sh` to the rust-check.yml push +
pull_request path filters so PRs editing only the checker still
trigger CI.
7-test regression matrix (positive + 6 negative cases):
- positive (real codebase) → PASS
- generic `impl<T> Widget for X` injected → FAIL F1
- direct `use openpencil_shell_core::widgets` outside host → FAIL F4
- `// glue:` tag on `impl Widget for X` line → FAIL F1 (exemption
doesn't save it; only `fn paint` lines are exempted)
- shell-core file replaced with `// stub` → FAIL R1
- grouped `use openpencil_shell_core::{widgets::TreeWidget};` → FAIL F4
- grouped `use openpencil_shell_core::{widgets};` → FAIL F4
- shell-core file body replaced with `// impl Widget for X { ... }` → FAIL R1
Codex iterate review: 5 rounds → GO. Round 1 BLOCK (greedy
WidgetHost match), R2 BLOCK + 3 CONCERN (calls/imports unchecked,
generic impls, broad exemption, filename-only count), R3 BLOCK +
CONCERN (grouped imports, commented-out impls), R4 2 NITs
(documentation parity), R5 GO clean.
Replaces the Phase A red-rect demo with the Step 1b inspector
composition: WebShell now owns a `WidgetHost` and `mount()` paints
the four shell-core widgets (Tree / PropertyRow / Dropdown /
TextInput) into a 280-px column on a white-cleared canvas.
What's added:
- `crates/openpencil-shell-web/src/widget_host.rs` — the only file
in shell-web that calls into `openpencil_shell_core::widgets::*`,
per spec §1.4 boundary. Module doc anchors the invariant; the
paint signature carries the `// glue:` marker that the Phase B4
boundary check script (`tools/check-widget-boundary.sh`) will
grep for.
- `WidgetHost { tree, width, dropdown, text_input }` owns one of
each kind; `WidgetHost::new()` populates them from the B2 sample
/ new constructors. `Default` forwards to `new()`.
- `WidgetHost::paint(&self, backend, available_width)` builds a
`LayoutCx`, iterates a `[&dyn Widget; 4]` array, places each at
x=16 with 12-px vertical gaps. Reborrows backend each iteration
(`&mut *backend`) so subsequent iterations don't fail
borrow-check on the moved `&mut WebBackend`.
shell-web/src/lib.rs:
- Adds `mod widget_host;` cfg-gated to the `skia` feature.
- WebShell gains `host: WidgetHost`.
- Renames `paint_phase_a` → `paint_inspector`. Body clears the
canvas to white, dispatches via `self.host.paint(...)`, then
surfaces `take_present_error()` as JsValue exception. Same
panic-safe + canvas-type-check + present-error-propagation
pattern as Phase A C-hard.2 (codex Phase A gate review approved).
- The stub mount entry (no skia feature) is unchanged — the
kickoff §1.2 wasm32-clean compile guard CI still uses it.
Plan-vs-implementation deviations (deliberate):
- Plan B3 step 2 simplifies `mount()` in a way that drops the
panic hook + canvas-type-check + present-error propagation.
Preserved all three because the codex Phase A gate review
explicitly approved them as "panic-safe mount". Plan body's
`mount` block is treated as historical sketch.
- Plan body's `let mut cx = PaintCx { backend };` would move the
reference and fail borrow-check on the second iteration.
Changed to explicit reborrow `&mut *backend`. This is the
plan's intent, just with the borrow-checker subtlety made
explicit.
Verification:
- `cargo build -p openpencil-shell-web --target
wasm32-unknown-unknown --features skia --release` — green
- `cargo check -p openpencil-shell-web --target
wasm32-unknown-unknown --no-default-features --features web` —
green (compile guard)
- `bash tools/check-wasm-bundle.sh` — PASS
- 0 env.* imports (bundle still LinkError-free)
- 615 764 bytes gzip = 58% of 1 MiB ceiling
- +2 KiB vs Phase A C-hard.2 (~613 KiB) — widget code is small
- `cargo check -p openpencil-shell-native` — green (no regression)
Codex iterate review: 1 round → GO with 2 NITs (script name
singular vs plural, stale "Phase A red-rect" phrase) — both
fixed in this commit.
Lands the four Step 1b inspector widgets in shell-core (per spec
§1.4 — widget logic lives here so shell-native + shell-web reuse
it; only the RenderBackend impl + DOM event mapping + accesskit
DOM mirror are platform-owned).
Widgets:
- `widgets::TreeWidget` (Role::Tree, label "Layers") — sample
3-item tree (Frame / Title / Button) with selection-aware
blue-row paint and depth-indented labels. WidgetIds 100-103.
- `widgets::PropertyRow` (Role::Group, label "{label} {value}")
— single-row label/value pair. PropertyRow uses Role::Group
rather than Role::GenericContainer so the row label survives
ARIA filtering on the way to VoiceOver / NVDA (codex B2 R1
CONCERN). WidgetIds 200-299.
- `widgets::Dropdown` (Role::ComboBox, label "Blend") — sample
blend-mode picker with 3 options. Phase B static slice does
NOT yet pop a menu when `state.open == true`; Phase C wires
click + keyboard handling. WidgetIds 300-399.
- `widgets::TextInput` (Role::TextInput) — single-line input
with CJK IME preview. Paints `state.preedit` (in-progress
composition) when present, else `state.value`; non-empty
preedit also draws an 80px underline. Phase C lands
compositionstart / update / end → state mutation in shell-web.
WidgetIds 400-499.
State separation:
- `DropdownState { selected, open }` and `TextInputState { value,
preedit }` live as their own structs so Phase C event handlers
can swap them without taking ownership of the surrounding
widget. `TextInputState::default()` returns the empty state.
Tests (`tests/widgets_static.rs`):
- `four_inspector_widgets_paint_static_content` — paints all
four widgets through one RecordingBackend, asserts
≥5 fills / ≥3 strokes / ≥7 text dispatches (each tightened
vs the plan sketch to actually catch per-widget regressions).
- Per-widget role + label assertions (Tree / Group / ComboBox /
TextInput).
- `text_input_paints_preedit_underline_when_composing` —
drives `state.preedit = "你好"`, paints, asserts the IME
branch emits 1 fill + 2 strokes (border + underline) + 1
text run.
- `dropdown_state_independent_state_struct` +
`text_input_state_default_is_empty` — verify state structs
are independent + default-constructible.
Plan-vs-implementation deviations (deliberate):
- `WidgetId::new(N)` instead of the plan's `WidgetId(N)` tuple
literal so the sample/new constructors exercise the B1
debug_assert non-zero check. Tuple stays public for pattern
matching + `const` contexts.
- WidgetId range conventions per widget kind (Tree=100s /
PropertyRow=200s / Dropdown=300s / TextInput=400s) added as
doc-only comments. Real Phase C host will allocate from a
counter; the conventions just keep the B-phase fixtures
predictable.
- Per-widget paint counts in `four_inspector_widgets_…` test
tightened to ≥5/≥3/≥7 (plan sketch had ≥4/≥3/≥4 which
wouldn't catch a regression in Tree's selection-row fill).
Verification:
- `cargo test -p openpencil-shell-core` — 10/10 passing
- `cargo check -p openpencil-shell-core --target
wasm32-unknown-unknown` — green (shell-core stays
wasm32-clean per spec §1.2 — no platform deps creeping in)
- `cargo check -p openpencil-shell-native` — green
Codex iterate review: 2 rounds → GO.
Adds the widget facade that B2 inspector widgets and Phase C event
handling will plug into. Logic-bearing widget code lives in
shell-core (per spec §1.4); shell-native + shell-web only own their
RenderBackend impls + DOM event mapping + accesskit DOM mirror.
What's added:
- `widgets::Widget` trait with `id` / `layout` / `paint(&self,...)` /
`access_node` methods. Phase B widgets are static — `paint` is
`&self`, mutable per-widget state lives in `*State` structs that
B2 lands. Phase C will extend the trait with a `&mut self` event
method for input handling.
- `widgets::WidgetId(pub u64)` plus a `pub const ROOT_WIDGET_ID =
WidgetId(0)` and a `WidgetId::new(id)` constructor with
`debug_assert!(id != 0)`. The tuple constructor stays public so
pattern matching + `const` contexts keep working; `::new` is the
conventional path that surfaces the root-id reservation in debug
builds. (Codex B1 R1 NIT-7 — make the convention compiler-visible
before Phase C tree routing lands.)
- `widgets::PaintCx<'a> { backend: &'a mut dyn RenderBackend }` and
`widgets::LayoutCx { available_width, dpi }` — frame-scoped paint
context + layout-time context. The `&mut dyn` indirection lets
shell-native + shell-web reuse the widget code without
monomorphising over the concrete backend.
- `widgets::LayoutBox { rect: Rect }` with `Debug + Clone + Copy +
PartialEq` derives.
- A `rect(x, y, w, h)` constructor convenience used by tests + B2.
Test harness (`tests/widgets_static.rs`):
- `RecordingBackend` impl `RenderBackend` counting each call.
- `paint_cx_dispatches_through_dyn_backend` — verifies fill_rect /
stroke_rect / save / translate / clip_rect / restore all dispatch
via `&mut dyn RenderBackend`.
- `widget_trait_dispatches_layout_and_paint` — minimal `StubWidget`
proves the trait shape compiles; asserts layout result, paint
dispatch count, `WidgetId::new(7)` round-trip, `ROOT_WIDGET_ID.0
== 0`, and `access_node().role() == Role::GenericContainer`. Real
semantic roles (TreeItem / EditableText / etc) land with B2.
Plumbing:
- `accesskit = "0.24"` added to shell-core deps to match shell-web's
pin (the version compatible with shell-native's accesskit_winit
Step 1a usage). Codex B1 R1 Q3 flagged that shell-native does not
yet pull accesskit; this is acknowledged as a Phase C tracked item
— verify the same version when DOM mirror / native a11y wires up.
- `Rect` now derives `PartialEq` so `LayoutBox` can use the same
derive. `Eq` is intentionally NOT derived (Vec2 carries floats);
comment in render_backend.rs explains.
Plan-vs-implementation deviations (deliberate, all kept narrow):
- Plan B1 step 2 declares `pub mod {dropdown, prop_row, text_input,
tree};` + re-exports inside widgets/mod.rs. Omitted here because
those modules don't exist until B2; declaring them now would
break the B1 standalone build. Top-block plan mini-patch
convention applies (override sketches in body).
- Plan didn't enumerate the accesskit dep + `Rect: PartialEq`
deltas — added with rationale comments.
Verification:
- `cargo test -p openpencil-shell-core` — green
- `cargo check -p openpencil-shell-core --target
wasm32-unknown-unknown` — green (shell-core stays wasm32-clean
per spec §1.2)
- `cargo check -p openpencil-shell-native` — green (no regression)
Codex iterate review: 4 rounds → GO. Round 1 CONCERN (3 items),
Round 2 CONCERN (1 stale comment), Round 3 CONCERN (comment vs
test body mismatch), Round 4 GO clean. Q3 (accesskit_winit
alignment) carries to Phase C as informational.
Implements the Phase A A4 NIT-1 fix from codex Phase A gate review
(verdict GO, 2 NITs flagged): the local equivalent of the Step 1b §6
+ §7.1 CI gate, runnable by developers before submitting Phase A-E
PRs. The full CI workflow remains DEFERRED in
`.github/workflows/rust-release.yml` until brew emscripten install +
EMSDK + .wasm.a → .a symlink + wasm-bindgen + wasm-opt are
automated; until then this script is the authoritative gate.
What it enforces (matches spec §6 + §7.1):
- EMSDK env var present (build-time-only emsdk libcxx headers +
wasm-aware clang)
- cargo build → wasm-bindgen → wasm-opt -Oz pipeline succeeds
- 0 env.* imports in the post-bindgen bundle (any leak = LinkError
at load time = regression)
- gzip size ≤ STEP1B_SHELL_WASM_GZIP_LIMIT_BYTES (default 1 MiB)
Verified on the C-hard.2 bundle: 613 006 bytes gzip (58% of 1 MiB
ceiling), 0 env.* imports, exit 0.
Step 1b Phase A — completes A3 (local) + A4 (codex verdict GO).
Why: Codex stop-time review 2026-05-10 — clipCardImageCorners was
slotted between unwrapFakePhoneMockups and resolveTreeRoles in the
post-streaming pipeline. Cards that don't carry an explicit
cornerRadius from the sub-agent get one filled in by role-resolver
(e.g. role='card' → default cornerRadius=12). Running clip before role
defaults silently skipped every default-radius card — match policy
required scalar cornerRadius > 0 and saw cornerRadius=undefined at
that point. Net effect: the bug stayed for the most common case where
the model wrote role='card' without a numeric radius.
What: move the clipCardImageCorners call to the freshRoot block right
after resolveTreeRoles + resolveTreePostPass. By that point the
role-resolver has populated defaults, so a card without an explicit
radius now correctly hits the predicate. Reuse the existing
\`freshRoot\` reference (re-fetched after updateNode mutations earlier
in the pipeline) so we don't double-fetch from the store.
No new tests — the existing 9 unit tests already pin the predicate;
the bug was placement-only. 1448 / 1448 pen-core tests, 1110 / 1110
AI service tests still pass.
Why: 2026-05-10 user report — "你看不到任何真正的生产级设计工具的希望"
called out three persistent visible issues with the food-app design:
1. Search bar shows a "weird inner rounded border" (image 7) — the
wrapper around address+search section sets fill + stroke +
cornerRadius together. strip-redundant-section-fills only cleared
the fill; the leftover stroke + cornerRadius kept drawing the
visible inner pill, which has been there for a long time.
2. Card image has all 4 corners rounded (image 8) — Taco Fiesta /
Bella Italia cards show the food image with bottom corners
rounded too, leaving them visually "ragged" against the title
text below that's flush with the card surface.
(The third — Categories padding — is a layout/intent question; left
for later since it can't be auto-fixed safely without knowing whether
the design wants edge-to-edge content.)
What — two coordinated fixes:
Fix#1: strip-redundant-section-fills now removes stroke and
cornerRadius alongside the fill. A misroll wrapper sets all three
together to "look like a card"; the fill gets stripped on detection
but the leftover chrome kept drawing a phantom card outline. The
three travel together so they should be cleared together. New test
pins the search-bar wrapper case.
Fix#2: new clipCardImageCorners pass in pen-core. When a card-shape
parent (scalar cornerRadius > 0, 2+ children, first child is an
image or canonical image-placeholder, image has its own scalar
cornerRadius) is detected, set parent.clipContent = true and remove
the image's scalar cornerRadius. The card's own corner clip then
cleanly handles the image — top corners round with the card, bottom
corners flush against the title below. 9 unit tests pin the
conservative match policy (silent on standalone image, title-first
card, array-form cornerRadius, cornerRadius:0, no image
cornerRadius, nested cards, existing clipContent).
Wired into applyPostStreamingTreeHeuristics right after
unwrapFakePhoneMockups. 1448 / 1448 pen-core tests pass (was 1438; +10);
1110 / 1110 AI service tests still pass.
Why: 2026-05-10 user report — "Mexican" badge in the food-app screenshot
landed with a "带尖的背景阴影" (pointy / spiked background shadow). The
underlying cause is the model emitting effects with positive spread,
which "bleeds" the shadow color outward and creates a visible bloom /
halo around the badge that doesn't match real product UI shadows.
Real UI shadows are tight: blur 4-16, spread 0, near-black low-alpha.
Existing detectors only handle text effects (text-effect from 7aef1b14);
frame-level effects had no aesthetic gate.
What: detectExcessiveFrameEffects flags a frame node iff ANY of:
- blur > 40 (glow / halo signature; modal-shell scrim uses exactly
40 so the threshold is strict-greater to keep that legitimate use
untouched — verified by detectors-builder-clean.test.ts)
- any effect carries spread > 0 (bleeding outward = the "spiked
shadow" the user called out)
- 3+ stacked effects on one frame (typical UI uses 0-2)
Suggested fix is to remove the effects array; the user / agent can
re-add a proper subtle shadow afterwards if intentional.
Wired through detectAllIssues + index.ts public exports + the
debug_validation_report MCP categories enum. Skips text nodes (those
go through detectTextEffect with a stricter zero-tolerance rule).
7 new tests cover: positive on spread > 0 / blur > 40 / 3+ stacked,
negative on typical subtle shadow / no effects / blur exactly 40
(modal-shell legit) / text node (different detector). 241 / 241
pen-ai-skills tests pass (was 234; +7). 1110 / 1110 AI service tests
pass (unchanged — production builders pre-clean).
Why: 2026-05-10 user report — the food-app "Featured" block landed
with a black background on a cream page. The wrapper had role='card'
AND held 3 restaurant cards each with role='card'. The existing
strip-redundant-section-fills pass treats role='card' as PROTECTED
(cards legitimately own their fills) so the black hedge fill survived.
Visible result: a giant black band between Categories and Popular Near
You that doesn't fit the cream page bg — exactly the "莫名其妙的背景颜色"
issue the user called out.
Root cause: the existing wrapper detection (hasNestedFilledComponent)
only fires for ATOMIC roles (search-bar / button / input / badge /
chip / tag / pill). Container-role wrappers around same-role children
were never matched, so a card-of-cards misroll kept its fill.
What: new hasMultipleSameRoleChildren predicate. Treats a frame as a
section-level wrapper (eligible for safe-dark / safe-light fill
stripping) when ALL of:
- frame role is in CONTAINER_PROTECTED_ROLES (card / banner /
pricing-card / feature-card / image-card / testimonial /
metric-card / gallery-item / phone-mockup)
- frame has ≥ 2 children with the SAME role
- existing safe-hex / root-match check still applies
Net effect: the Featured wrapper's #000000 fill is now stripped, the
section inherits the cream page bg as intended, and the 3 inner
restaurant cards keep their own white surface fills (each was
role='card' but only 1 child of the same role per card, so the
predicate is silent on them).
3 new it() cases pin: Featured-block misroll (3 cards inside) gets
stripped, single-same-role-child stays untouched, banner-wrapping-
banners pattern also strips. 25 / 25 strip tests pass (was 22; +3).
1438 / 1438 pen-core tests pass overall.
Why: Codex stop-time review 2026-05-10 caught a content-erasure bug.
The b9b4126b heuristic accepted "0 or 1 children" with no type check,
so a hero frame named "Hero" with a single CTA button child matched.
Then the queue's update path (line 388-391 of processQueue) clears
\`children: []\` when the photo lands — destroying the legitimate CTA
button under it. Same risk for banners with sale text, covers with
nested frames, etc.
What: tighten the children clause in isImageAreaFrameByHeuristic. A
heuristic frame is now accepted only when:
- children is undefined / not an array, OR
- children.length === 0, OR
- children.length === 1 AND children[0].type === 'icon_font'
(the typical "broken image" placeholder hint).
Anything else — single non-icon child OR multiple children — means the
frame holds real content and the children:[] erase step would damage
the design. Such frames are now rejected by the heuristic and stay
unchanged on canvas.
3 new it() cases pin the rejection: hero+button, banner+text, and
cover+nested-frame all return false. Existing single-icon-child case
still passes. 38 / 38 tests in image-search-pipeline.test.ts (was 37;
+1; the rejection case adds 1 new it). 1110 / 1110 AI tests pass.
Why: Codex stop-time review #N (2026-05-10) caught the real bug — my
b9b4126b / 810c2f7a chain detected heuristic image-area frames at
collectImageSearchTargets and enqueued them at enqueueImageForSearch,
but the queue processor's still-needs-fill re-check (line 350-358 of
processQueue) called isUnfilledImagePlaceholderFrame which strictly
requires role='image-placeholder'. Heuristic frames have no role, so
the re-check returned false and the queue silently dropped them
before issuing the fetch — net effect was zero photos for the food-app
card scenario the heuristic was supposed to fix.
What: extract a new pure-function predicate
\`isFramePlaceholderStillUnfilled(node)\` that accepts a node iff it is
EITHER a canonical unfilled placeholder (role-based) OR a heuristic
match (name-based). Queue processor calls this single helper instead
of the strict canonical-only check. The helper is also exported and
test-covered separately so a future regression on this code path
fails loudly.
6 new it() cases pin: positive on canonical + heuristic, negative on
already-filled (canonical AND heuristic), null/undefined, and
unrelated frame names. 31 → 37 tests in image-search-pipeline.test.ts;
1109 / 1109 AI service tests pass overall (was 1103; +6).
Why: 810c2f7a added the parent-walk query mining for heuristic image-
area frames but landed without unit coverage of the two new helpers.
Without it, a future edit to GENERIC_PLACEHOLDER_NAMES or the layout-
word filter could silently regress query quality (e.g. start emitting
"Wrapper" or "Section" as queries) and the food-app card photos would
go back to looking generic.
What: export the two helpers and add 5 it() cases covering:
- explicit imageSearchQuery wins over name
- generic literal "Image" + parent "Bella Italia" → returns "Bella Italia"
- skip layout words ("Card Wrapper") in the parent walk; accept the
next semantic ancestor ("Margherita Pizza")
- 4-hop layout-only chain returns null (maxHops bound)
- fall back to non-generic-but-image-themed name when parent walk
yields nothing ("My Custom Photo")
31 / 31 tests in image-search-pipeline.test.ts pass (was 26; +5).
1103 / 1103 AI service tests pass (was 1098; +5).
Why: b9b4126b's collectImageSearchTargets returns heuristic-matched
frames (named "Image" / "Photo" / "Cover" without the canonical
role) but two downstream code paths quietly dropped them:
1. enqueueImageForSearch only accepted type==='image' or
isUnfilledImagePlaceholderFrame, so the heuristic frames got past
collect but never reached the queue.
2. extractQueryForNode would have returned the literal name "Image" or
"Photo" — useless to the photo search API. The user's "Bella Italia"
restaurant card never gets a relevant photo because the placeholder
frame's name says nothing about the restaurant.
What:
- enqueueImageForSearch grows a third branch: isImageAreaFrameByHeuristic
→ kind: 'placeholder-frame'. Same kind so the rest of the pipeline
treats it identically to a canonical placeholder.
- extractQueryForNode learns to skip "generic" placeholder names
(Image / Photo / Cover / Hero / Thumbnail / Banner / Poster + a few
variants) and walk up to the nearest semantic parent frame name
("Bella Italia" / "Margherita Pizza" / "Sushi House" — whatever the
enclosing card was named). Bounded to 3 hops. Filters layout words
(Card / Wrapper / Container / Section / Frame / Root / Page / Stack /
Row / Column / Content) so we don't end up searching for "Card".
- A new helper findParentSemanticName builds a parent map from the
live document on demand. Cheap for typical designs (< few hundred
nodes); avoids threading parent through every collect / enqueue call
site.
Net effect: a model-emitted plain "Image" frame inside a "Bella Italia"
card now searches for "Bella Italia" instead of literally "Image". The
existing isImageAreaFrameByHeuristic test coverage protects the entry
condition; 1098 / 1098 AI service tests still pass.
Why: 2026-05-09 user report — the food-app card design landed with
empty colored rectangles where restaurant photos should be (Bella
Italia / Green Bowl / Margherita Pizza). Root cause: the model emitted
plain frames named "Image" / "Photo" / "Cover" with a solid fill as
the card-top image area, instead of using add_image_placeholder_v1
(which sets role: 'image-placeholder'). The auto-search pipeline
strict-checks role and missed all of them, so scanAndFillImages found
nothing to search and the cards stayed solid-colored.
What: new isImageAreaFrameByHeuristic(node) supplements the strict
role check. Conservative match — only fires when ALL of:
- frame node WITHOUT role='image-placeholder' (strict path handles
that)
- name matches /\\b(image|photo|cover|hero|thumbnail|thumb|picture|
banner|poster)\\b/i
- width >= 80 AND height >= 60 (filters tiny color swatches)
- exactly one solid (non-image) fill (gradient = decorative, image
= already filled, both skip)
- 0 or 1 children (an icon child is OK, multi-child = real layout)
collectImageSearchTargets walks both the strict and heuristic paths
and produces the same kind: 'placeholder-frame' target either way, so
the existing query / aspect / search code-path runs unchanged.
10 new tests cover positive matches (Image / Photo / Cover / Hero /
Thumbnail / Banner / Poster), negative for canonical placeholder
(double-counting prevented), unrelated names (Card / Wrapper),
already-filled image fills, gradients, content-rich frames, single-
icon-child acceptance, undersize frames, and non-numeric dimensions.
26 / 26 tests in image-search-pipeline.test.ts pass (was 16; +10).
1098 / 1098 AI service tests pass (unchanged).
Why: f1923ff1 added coerceNavTabIcon to bottom-nav-v1 and noted that
sidebar-nav-v1 should adopt the same helper. Without it, a sidebar
nav with \`{ label: 'Profile', icon: 'profile' }\` would still render
a placeholder circle (resolver doesn't know "profile" is a known
wrong-glyph alias for "user") instead of the lucide:user glyph.
What: import coerceNavTabIcon and apply it in buildItemV1 before
stamping iconFontName onto the Icon child node. Same convention
single-sourced. Existing 1435 tests still pass — no test relied on
the prior pass-through behavior for known wrong-glyph names.
Why: end-to-end test of "Design a bottom nav with Home / Search /
Orders / Cart / Profile" with MiniMax-M2.7 surfaced that the model
emits \`{ title: 'Cart', icon: 'shopping-bag' }\` for the Cart tab
~half the time. Both icons exist in lucide but they are different
glyphs — bag is for carrying, cart has wheels for checkout. The
icon-catalog skill update (19ca1c66) fixed it for the planning side
but not for the builder's runtime input — direct \`add_bottom_nav_v1\`
calls still pass through whatever icon the model picks.
What: new \`coerceNavTabIcon(title, icon, builder)\` helper in
coerce-params.ts. Maintains a small Title→canonical-lucide-name map
(Cart→shopping-cart, Profile→user, Home→house, etc., with Chinese
labels) AND a per-canonical KNOWN_WRONG_ALTS list so the swap only
fires when the emitted icon is one of the known wrong-glyph choices
for that title:
Cart + shopping-bag → shopping-cart (warn)
Cart + package → shopping-cart (warn)
Cart + rocket → rocket (pass-through)
Cart + shopping-cart → shopping-cart (silent)
Custom + anything → anything (silent)
The pass-through rule keeps user / model intentional custom choices
intact. Warnings flow through the existing coerce-params sink so
orchestrators can surface them.
bottom-nav-v1.ts now calls coerceNavTabIcon before stamping
iconFontName onto the Tab frame. Sidebar-nav-v1 + similar nav
builders can adopt the same helper later without re-implementing the
map.
10 new unit + integration tests cover: positive swaps for cart /
profile / notifications / Chinese 购物车, pass-through for
custom titles, case-insensitivity, and the builder integration
that the emitted Tab tree carries the canonical iconFontName.
1098 / 1098 tests pass overall (1080 AI + 10 new + 8 elsewhere).
Why: 3abdfcf9 added integration tests for 4 of the 6 new aesthetic
detectors (rotation / text-cornerRadius / text-stroke / mixed-sibling-
cornerRadius). text-effect (7aef1b14) and mixed-sibling-padding
(ad025c95) landed without integration coverage of the
detect → applyFixes → store mutation chain.
What: 2 more it() cases mirroring the existing pattern:
- text-effect: text node with shadow effects → effects cleared
- mixed-sibling-padding: 3 cards with padding 16/16/20 → outlier
rewritten to scalar 16 (collapsed from the 4-tuple modal because
all sides are equal — locks in the scalar-collapse code path)
13 / 13 tests in design-pre-validation.test.ts pass (was 11; +2).
1088 / 1088 AI tests overall (was 1086; +2).
Why: even after the icon-catalog skill rewrite (19ca1c66) said "ALWAYS
USE icon_font, NEVER path NODES", end-to-end testing with MiniMax-M2.7
shows the model still emits ~half its icons as \`path\` nodes (Bell
Icon / Cart Icon etc.) because the per-subtask CRITICAL LAYOUT
CONSTRAINTS prompt — which the model treats as the prompt of record
— never restated the icon convention. The skill prompt is upstream
context the model can drift away from; the per-subtask block is the
last thing the model reads before generating, so it carries weight.
What: append a one-line ICONS rule directly to CRITICAL LAYOUT
CONSTRAINTS in orchestrator-sub-agent.ts. Restates the icon_font shape
inline (\`{"type":"icon_font","iconFontName":"<lucide-name>",…}\`) and
calls out the failure mode by name (resolver guess + placeholder
circle) so the model sees both the right pattern and the consequence
of the wrong one.
The path-with-iconic-name fallback path keeps working — this is purely
preventive guidance, the resolver tokenize fix (ef6f7ed3) still
catches the leftover cases. 1086 / 1086 AI tests still pass.
Why: the 4 new aesthetic detectors (rotation / text-cornerRadius /
text-stroke / mixed-sibling-cornerRadius) have unit tests against the
pure detect functions in pen-ai-skills, but the chain through the live
Zustand store + runPreValidationFixes had no end-to-end coverage.
Without it, a future refactor that subtly breaks the apply step (e.g.
suggestedValue:undefined not clearing the prop) would slip through.
What: 4 new integration tests in design-pre-validation.test.ts using
the existing makeDoc / loadDocument fixtures. Each builds a doc with
exactly one known aesthetic issue, runs runPreValidationFixes, and
asserts the store mutation took effect:
- rotation:12 on a frame → reset to 0
- cornerRadius:8 on a text node → cleared (undefined)
- stroke on a text node → cleared (undefined)
- mixed cornerRadius across 3 sibling cards (8 / 8 / 12) → outlier
rewritten to modal 8 (note: handled by the older
sibling-inconsistency detector; mixed-sibling-corner-radius is
the dedupe-loser here since sibling-inconsistency runs first
with the same {nodeId, property} key — both produce the right fix)
11 / 11 tests in design-pre-validation.test.ts pass (was 7; +4).
1086 / 1086 AI tests overall (was 1082; +4).
Why: C9 (b1ffa1c5) removed `image` from the resolver noise list to make
"Image Icon" resolve to lucide:image. The doc comment claimed "Image
Placeholder Path" still resolves via prefix fallback, but the math is
wrong — `image` is 5 chars, `imageplaceholder` is 16 chars, 5/16 = 31%
which is below the 50% FALLBACK_MIN_RATIO. So "Image Placeholder Path"
genuinely no longer hits the resolver fallback path. That's actually
correct (no icon marker word, resolver returns early), and the original
food-app circles came from the model emitting circle path-data
directly, not from the resolver fallback. But the photo / camera
aliases were also implicit dependencies that deserve explicit coverage
to lock in the behavior.
What: 2 new tests verify photo / camera resolve correctly through the
already-existing alias chain (`photo: _IMAGE` in BUILTIN_ICONS, lucide
camera native). 1082 / 1082 AI tests pass (was 1080; +2).
Why: the 6 aesthetic detectors added in 53435bf7 / 7aef1b14 / cd1e4325 /
ad025c95 catch problems AFTER the model emits them. Telling the model
upfront — in the always-loaded layout skill — prevents the same patterns
in the first place. Cheaper than running a corrective post-pass on
every generation, and the model produces cleaner output that doesn't
trip the detectors at all.
What: AESTHETIC HYGIENE block appended to layout.md (priority 10, base,
loaded for every generation). 4 rules each backed by a corresponding
detector:
- Text never gets cornerRadius / stroke / effects / rotation. Mirrors
detectTextCornerRadius / detectTextStroke / detectTextEffect.
- Rotation on UI frames is almost always wrong. Mirrors
detectUnexpectedRotation (with the same 90/180/270 + path/line/polygon
/image escape hatches).
- Same-role siblings must share cornerRadius AND padding. Mirrors
detectMixedSiblingCornerRadius / detectMixedSiblingPadding.
- Inner layout frames (sections, wrappers) inherit from page/card —
only opt into fill/stroke/shadow on the outer card/button/badge/chip.
Mirrors the existing invisible-container detector.
Phrased as a "keep these silent" pre-condition since the post-pass
also strips them. 1080/1080 AI tests + 234/234 pen-ai-skills tests
still pass.
Why: when the pre-validation pass auto-fixes issues, the chat panel
just says "Pre-checks: fixed 5 issues" — generic and uninformative.
The user can't tell whether 5 invisible-container fixes happened
(structural, mostly safe), 5 unexpected-rotation fixes (aesthetic,
worth reviewing), or 5 mixed-sibling-padding fixes (consistency, worth
reviewing). With 10 detector categories now (4 original + 6 aesthetic
added in 53435bf7 / 7aef1b14 / cd1e4325 / ad025c95), the per-category
visibility starts to matter.
What:
- runPreValidationFixesDetailed() returns { total, byCategory } where
byCategory is a per-category count of APPLIED fixes (excludes the
info-severity skips and the protected-status-bar skip).
- runPreValidationFixes() kept as a thin wrapper returning .total so
no caller needs to change.
- design-validation.ts now uses the detailed result and formats the
breakdown as e.g. "fixed 5 (3 text-effect, 2 unexpected-rotation)"
in the chat panel — sorted by count descending so the dominant
category surfaces first. Both the no-vision-validation path and the
size-gated skip path show the breakdown when it exists.
Falls back to the legacy "fixed N issues" format when byCategory is
empty (defensive — should never happen if total > 0). 1080 / 1080 AI
tests still pass — the new return shape is additive and the wrapper
preserves the integer contract.
Why: continuation of the aesthetic detector series. Mirrors
detectMixedSiblingCornerRadius (53435bf7) for the padding axis. Three
cards with padding 16 / 16 / 20 looks ragged on canvas; the existing
sibling-inconsistency detector covers cards-vs-cards but dedupes
against cornerRadius and other props so the padding outlier
sometimes drops.
What: detectMixedSiblingPadding normalises padding values to a
4-tuple [top, right, bottom, left] before comparison, so
padding: 16 → [16,16,16,16]
padding: [12, 24] → [12,24,12,24] (CSS 2-tuple shorthand)
padding: [16,16,16,16] → [16,16,16,16]
all compare equal and don't trigger false positives. Modal value
collapses back to a scalar when all four sides are equal so the
suggested fix matches the model's preferred shorthand.
Same 60% modal-majority threshold as the cornerRadius detector —
1-1-1 three-way splits are skipped because there's no canonical value
to suggest. Same divider / spacer skip and same-type-and-role grouping.
Wired through detectAllIssues + index.ts exports + the
debug_validation_report MCP categories enum.
6 new tests cover: number shorthand outlier, number-vs-array
equivalence, 2-tuple-vs-4-tuple equivalence, 1-1-1 split skip,
mixed-role groups skipped, no-padding siblings excluded from modal.
57 / 57 diagnostics tests pass (was 51; +6).
Why: continuation of the aesthetic detector series. Outlined text on a
UI label is almost always an AI mistake — Lucide / SF / Material icons
get stroked, but body / heading / label text is filled. The model
occasionally copies a generic "give it a stroke" instruction onto text
nodes; on canvas the result reads as double-rendered glyphs. The
existing sibling-inconsistency detector doesn't catch this because
text stroke is rarely a sibling-by-sibling outlier — it's emitted
across the whole tree at once.
What:
- detectTextStroke added with the same shape as the other text-only
aesthetic detectors (text node + property check + warning severity +
suggestedValue undefined).
- Skips stroke.thickness === 0 (some model JSON keeps an empty stroke
object as a placeholder; flagging that would be noise).
- Wired through detectAllIssues + index.ts public exports + the
debug_validation_report MCP tool's categories enum.
Tests: 4 new positive + negative cases (text with stroke, text without
stroke, text with thickness=0 placeholder, frame with stroke). 51 / 51
diagnostics tests pass (was 47; +4); 228 / 228 pen-ai-skills overall.
Why: continuation of the aesthetic detector family added in 53435bf7.
The model frequently sprinkles \`effects: [{type:'shadow', …}]\` onto
body / label / caption text. On canvas the type goes fuzzy and reads
"AI-designed". Real product UIs use text shadows extremely sparingly
(hero overlays on photos, a few brand elements). Detection is cheap
(walk + isArray check) and the suggested fix (remove effects array)
is safe — text shadow on UI labels is almost never intentional.
What:
- detectTextEffect added to packages/pen-ai-skills/diagnostics with the
same shape as the prior 3 (warning severity, suggestedValue undefined,
reason string for logs).
- Wired through detectAllIssues + index.ts public exports + the
debug_validation_report MCP tool's categories enum.
Tests: 5 new it() cases covering positive (shadow / blur on text),
negative (text without effects, empty effects array, frame with
effects), and tree-walk (multiple text effects in nested frames).
47 / 47 diagnostics tests pass (was 42; +5).
Why: user reports the validation pipeline lacks "aesthetic standards"
— it accepts misalignment, unwanted corner radius, and other visual
issues as "normal". Existing detectors are pure code-quality (invisible
container / empty path / text height / sibling inconsistency); they
don't catch design-system violations the user can see at a glance.
Vision validation does, but it only runs on Anthropic / Codex /
OpenCode / Gemini providers and only above 30 nodes — leaving a long
tail of small-design / builtin-provider runs with no aesthetic check
at all. Adding cheap pure-function detectors closes that gap with no
upstream provider dependency.
What: 3 new pure detectors in pen-ai-skills/diagnostics:
- detectUnexpectedRotation — flags non-axis-aligned rotation on
UI-bearing nodes (frame / text / shape). Skips path / line /
polygon / image (legitimate decorative geometry frequently
rotated), skips multiples of 90° (intentional vertical text /
grid). Catches the "tilted card" hallucination cleanly.
- detectTextCornerRadius — flags text nodes with cornerRadius > 0.
Text isn't drawn into a clipped rectangle so the prop is silently
dropped at render time, but it survives in the doc and burns
LLM context on subsequent batch_get calls. Suggested fix: remove.
- detectMixedSiblingCornerRadius — stricter than the existing
sibling-inconsistency check on cornerRadius alone. Flags outliers
when 2+ of 3 same-type-and-role siblings share a value and one
differs (e.g. three cards with cornerRadius 8 / 8 / 12 reads as
ragged on canvas). Skips 1-1-1 three-way splits (no canonical
modal) and divider / spacer nodes (visual primitives).
All three are wired through detectAllIssues + the index.ts public
exports + the debug_validation_report MCP tool's `categories` enum so
the user / agent can opt-in or filter via `op debug_validation_report
--categories unexpected-rotation`.
35 new tests cover the load-bearing positive + negative cases for each
detector. 219/219 pen-ai-skills tests pass (was 184; +35). 1080/1080
AI service tests still pass.
Why: dd8eb0eb's unwrap pass (Type 0 single-component section root
hoist) was integration-tested via the live Playwright run but had no
unit coverage. The integration test won't catch regressions when
someone tightens the heuristics — and the load-bearing "do nothing"
guards (multi-section, mobile screen, desktop, 0/N children, non-frame
child) are exactly where a careless edit would silently flatten a
multi-page design.
What: split the helper into two — a pure predicate
shouldUnwrapSingleComponentSectionRoot(plan, root) returning bool, and
the existing unwrapSingleComponentSectionRoot(rootNodes, plan) which
calls the predicate then mutates the store. Predicate is exported.
10 new tests cover:
- 3 positive: wrapper id ends -root / wrapper id ends -section /
wrapper name copies parent name
- 7 negative load-bearing guards: multi-section plan, mobile screen
(height >= 480), desktop (width > 480), root with 0 children, root
with multi children, wrapper with no children, wrapper with
unrelated id+name, wrapper is non-frame (text / icon)
1080 / 1080 AI tests pass (was 1070; +10).
Why: every time the vision validation loop returned skipped:true the
chat panel logged the same hardcoded "(timeout or provider error)"
string regardless of the actual cause — provider mismatch, HTTP error,
upstream config issue. Now that the server (validate.ts) returns
explicit skip reasons (e.g. "Vision validation is not supported for
builtin providers"), the UI should surface them so the user can fix
the right thing instead of guessing it's a timeout.
What: ValidationResult gains an optional `skippedReason` field.
validateDesignScreenshot fills it from response.json's `error` (or
the HTTP status text on a non-OK response) and propagates it through
the loop. The chat-panel status line now reads
"[error] Analysis skipped (<reason>)" with the server-provided
message clipped to 120 chars; falls back to the legacy string when no
reason is present.
1070 / 1070 AI tests still pass; no test depended on the literal
"timeout or provider error" string.
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).
The pre-commit hook runs `bun run format` (prettier via oxfmt) on
every staged commit; without a prettierignore entry the hook keeps
reformatting the upstream-original Cargo.toml / Cargo.toml.orig
files inside vendor/skia-safe-op/{skia-bindings,skia-safe}/ to use
2-space indent + flow-style author arrays — drift against the
crates.io tarball form that we want to preserve so the diff against
upstream rust-skia stays minimal and auditable.
Same treatment vendor/agent/ and vendor/jian/ already get.
Updates the rust-multiplatform + rust-release workflows for the
post-C-hard.2 reality where the wasm32-unknown-unknown bundle IS
runtime-loadable locally but the CI side still needs more
automation before it can publish a release artifact.
rust-multiplatform.yml:
- add `vendor/skia-safe-op/**` to push + pull_request path
filters so changes inside the fork actually trigger CI
- rename the wasm-web job → "wasm32-unknown-unknown / openpencil-
shell-web (compile guard)" to make explicit that this is the
--no-default-features --features web stub-mount baseline, not
the real render bundle
- drop the artifact upload from this job: the stub .wasm has no
skia and would mislead downstream consumers
rust-release.yml:
- delete the standalone `wasm` job for now and update the comment
to a DEFERRED block listing the 6 CI-side automation steps
still missing (brew emscripten install, EMSDK env var,
.wasm.a → .a symlink hack, wasm-bindgen + wasm-opt, browser
smoke). Re-add the job once the pipeline lands
- update the workflow header copy so it stops claiming to build
the WASM bundle alongside desktop binaries
- drop `wasm` from the release-draft `needs:` list
This is an explicit deferral, NOT a silent drop — every removed
piece is annotated with the work item it is waiting on.
Step 1b §3.2 P0.5B Run path, sub-phase C-hard CI follow-up.
Lights up the Phase A WebShell on the C-hard pipeline (vendor/skia-
safe-op + crates/wasm-libc-shim, wired in the previous two commits)
so `cargo build --target wasm32-unknown-unknown --features skia`
followed by `wasm-bindgen --target web` produces a browser-loadable
ES module with 0 env.* imports.
What's added:
- WebBackend (src/backend/mod.rs): impl RenderBackend over a
skia-safe raster N32_PREMUL surface; presents each frame to the
host <canvas> via image_snapshot → read_pixels → ImageData →
put_image_data. end_frame surfaces present errors via
last_present_error / take_present_error so a stale failure does
not leak into a subsequent successful frame
- skia_wasm.rs: thin make_raster_surface helper so swapping in a
GPU GrContext (Phase A round 2) is a self-contained change
- mount(canvas_id) entry: locates the host <canvas>, builds a
WebBackend, paints the Phase A red-rect demo synchronously,
propagates any present error as a JsValue exception
- smoke/step-1b.html: manual smoke harness that mounts the shell
and surfaces a structured diagnostic (with regression-mode
LinkError messaging + rebuild instructions) if loading fails
- extern crate wasm_libc_shim as _; in lib.rs to keep the shim's
no_mangle symbols from being dead-code-eliminated
- .gitignore for wasm-bindgen pkg/ output
Cargo.toml feature wiring:
- default = ["web"] keeps the kickoff §1.2 wasm32-clean compile
guard CI green (stub mount, no skia)
- skia = ["dep:skia-safe", "wasm-libc-shim"] opts into the real
WebBackend + raster paint loop; the shim dep is target-gated
so only wasm32-unknown-unknown actually pulls it in
- wasm-bindgen = "=0.2.117" pinned (last release that compiles
on Rust 1.85; bump alongside the toolchain in a future commit)
Verified end-to-end:
- `cargo build … --features skia --release` green
- `wasm-bindgen --target web` produces ../pkg/*.{js,_bg.wasm}
- WebAssembly.Module.imports() returns 22 imports, all from
./openpencil_shell_web_bg.js; 0 env.* imports
- post `wasm-opt -Oz`: 1542 KiB raw / 599 KiB gzip — within
spec §6 ceiling (≤ 1024 KiB gzip)
- the kickoff §1.2 wasm32-clean compile guard still passes
(`cargo check … --no-default-features --features web`)
Browser-side manual smoke (Phase E) is still TODO; the bundle is
structurally LinkError-free but a human still needs to confirm the
red rect actually paints in Safari / Chrome / Firefox before the
sub-phase can be marked complete.
Step 1b §3.2 P0.5B Run path, sub-phase C-hard.2.
Provides the libc / libcxx / libm symbols that the wasm32-unknown-
unknown skia static archive (built via vendor/skia-safe-op) imports
at link time but wasm-bindgen does not synthesize. With this crate
linked in, the post-bindgen bundle has 0 env.* imports and is
runtime-loadable as a vanilla browser ES module.
Categories implemented (~83 symbols, dedup against the actual
import list):
- allocator: malloc / free / calloc / realloc / malloc_usable_size
via dlmalloc-rs + a 16-byte size header per allocation so the
GlobalAlloc::dealloc layout contract is preserved on free and
realloc copies min(old_size, new_size) on grow
- libm: asinh / acosh / atanh / nextafterf / remainder via libm
- libc string: memchr / wmemchr / strcmp / strcpy / strtoull
hand-rolled byte-wise
- libc stdio: snprintf / vsnprintf / vfprintf as C-side variadic
stubs (stdio_stub.c, compiled by cc) that route into a Rust
extern wasm_libc_shim_stdio_panic before returning, so any
actual invocation surfaces a named panic via console_error_
panic_hook instead of a silent empty success
- libc misc: abort (panics with diagnostic) + __errno_location
(single-mut-static — single-threaded wasm only)
- C++ ABI: __cxa_atexit (no-op), __cxa_guard_acquire / release,
__cxa_pure_virtual (panics)
- operator new / delete: _Znwm / _Znam / _ZdlPv* / _ZdaPv*
forwarding to malloc / free; _Znwm(0) routes through
malloc(1) per C++ standard (operator new must return a
non-null pointer)
- threads: sem_init / sem_destroy / sem_post / sem_wait no-op
- libcxx string / locale / iostream / shared_weak_count /
to_string: ~23 panic stubs via the libcxx_stub! macro that
panic with the symbol name; these are linker-pulled by
templated code that the skia raster + custom_empty fontmgr
pipeline does not exercise at runtime, so a panic = regression
signal
Build-time gating:
- active only on wasm32-unknown-unknown via cfg(all(target_arch
= "wasm32", target_os = "unknown")); native builds link an
empty crate so the symbols do not collide with the host libc
- compile_error! on target_feature = "atomics" because the
static-mut errno + non-atomic __cxa guard impls would race
under wasm threads — the path forward is real TLS errno +
atomic guard variables in a follow-up sub-phase
Step 1b §3.2 P0.5B Run path, sub-phase C-hard.2.
Wires the workspace at vendor/skia-safe-op (committed in the previous
commit) via [patch.crates-io] so every consumer of skia-safe /
skia-bindings — both the wasm32-unknown-unknown shell-web bundle and
the macOS / Linux / Windows shell-native desktop binary — resolves
through the fork on every target.
[patch.crates-io] is workspace-global, NOT target-scoped; cargo does
not natively support per-target patches, so this is the accepted
blast radius. The fork is byte-identical to upstream rust-skia 0.97.0
except for the new `wasm_unknown` platform module + its single new
dispatch arm; native builds resolve to the same upstream platform
modules they did before. Verified `cargo check -p
openpencil-shell-native` builds through the fork unchanged.
Trade-off: upstream rust-skia patches no longer flow until we
re-vendor; Cargo.lock records `path` sources for skia-bindings /
skia-safe rather than `registry+...`. The full rationale block is
inline in Cargo.toml.
The Cargo.lock delta also pins js-sys 0.3.97 → 0.3.94 / web-sys
0.3.97 → 0.3.94 — this is the transitive consequence of pinning
wasm-bindgen = "=0.2.117" on shell-web (last 0.2.x release that
compiles on the workspace's Rust 1.85 toolchain; 0.2.120+ requires
1.86). Documented in shell-web/Cargo.toml.
Step 1b §3.2 P0.5B Run path, sub-phase C-hard.1.
Vendors rust-skia 0.97.0 (from crates.io tarballs) into
vendor/skia-safe-op/{skia-bindings,skia-safe} and adds a single new
platform module — build_support/platform/wasm_unknown.rs — that
recompiles Skia C++ directly into the wasm32-unknown-unknown ABI:
- reuses emsdk's libc / libcxx headers via -isystem
- drops C++ exceptions + RTTI (-fno-exceptions -fno-rtti) so the
resulting .o files do not import emscripten's exception runtime
- forces clang's target via --target=wasm32-unknown-unknown
- sets CC_/CXX_/AR_wasm32_unknown_unknown so the cc-crate FFI shim
also picks up emsdk's bundled clang (host clang lacks wasm32)
- on every gn_args invocation overwrites skia/bin/activate-emsdk
with a no-op python stub so Skia's GN does not try to bootstrap
a parallel emsdk install (we use emsdk's clang directly)
Why a fork: wasm-bindgen --target=web on wasm32-unknown-emscripten
emits emscripten library glue, not a browser-loadable ES module,
which is incompatible with our distribution model. Compiling Skia
to wasm32-unknown-unknown unblocks the browser ES module path; the
remaining libc/libcxx/libm gap is filled by crates/wasm-libc-shim
in a follow-up commit.
The Skia C++ source tree under skia-bindings/skia/ (~770 MB, 30k+
files) is gitignored — it is fetched at build time by
binary_cache::download. Only the Rust source from the upstream
0.97.0 crate tarballs and the new wasm_unknown platform module are
committed here.
Step 1b §3.2 P0.5B Run path, sub-phase C-hard.1.
Kayshen-X