split ui.zig into src/ui/* facade (plan #656, source #652) - #658
Conversation
dvui.button/labelNoFmt pins one face. Noto has no Arrows block so U+2191 tofus; fontSymbols() on the whole chip tofus Latin. Same split rich text already uses. Measure the arrow on the symbols face.
…wrap-to-clip in narrow canvas (review #654 L1+L9)
…f ↑ on narrow canvas (review #654 Minor L1)
Move paint helpers, state, metrics, and chrome out of the 1628-line ui.zig into 11 sibling files under ui/: state, metrics, scroll, kinds, chrome, toolrun, thinking, chip, status, skill, composer. ui.zig is now the public facade: BUILD_ID, onInit, onDeinit, frame. Import graph is acyclic (no state->paint cycles). No behavior, palette, protocol, or cap changes. Living docs path pointers updated. Plan: #656 Source: #652
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #658
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← plan/split-ui-facade · 16 files · mechanical ui.zig → src/ui/* facade split (plan #656)
Lenses run: L1 L3 L4 L5 L6 L8 L9 (skip: L2 — no API/workflows/secrets/bridge lengths; L7 — no host/config seams)
AGENTS.md read: yes · docs/feature-divide.md read: yes
Findings
| Sev | Lens | Finding | Break scenario | Refutation attempt | Confidence |
|---|---|---|---|---|---|
| Minor | L8 | ui/kinds.zig · lifecycleLabel resurrected (plan #656 DoD: do not reintroduce; deleted on main by #653). The extra bridge import exists only to serve this dead helper — plan import graph locked kinds.zig → palette, dvui only. |
Next agent treats the pub helper as product API and paints it on status-bar line 1, replacing the #651 idle rect_spinner. That is a pixel change this PR claimed not to make. |
“It’s unused pub, compile-clean, no pixels change today.” True for this HEAD — the trap is the API + the AGENTS.md wording below, not a current paint path. Still a DoD miss. |
high |
| Minor | L8 | AGENTS.md identity row drive-by: spinner → lifecycle. Canvas still paints rect_spinner.paint (ui.zig line 1). docs/feature-divide.md still says spinner. Living-docs plan said path pointers only. |
Agent “fixes” line 1 to match AGENTS.md (drop spinner, show lifecycleLabel). Same root as the row above. |
“Wording, not code.” Agents read AGENTS.md first; this PR is what taught them the wrong chrome. | high |
| Nit | L8 | In-tree pointers still say the moved symbols live in ui.zig: thinking_collapse.zig (“operator set lives in ui.zig” — now ui/state.zig), rich/kinds.zig (tool-run / skill paint), rich/toolrun.zig test comment (paintToolRun), transcript_split.zig (TOUCH_H now ui/metrics.zig), native/harness/README.md tree. |
Agent opens ui.zig for thinking-open maps or TOUCH_H, misses the owner, edits the facade or duplicates a constant. |
Listed living docs were updated; these comments were not in the plan’s doc table. Bounded. | high |
Residual risk
Moved bodies match main 1:1 (qualification + rich_toolrun alias + local scroll/chrome renames only). build-harness on this HEAD is green (test-rich + wasm), so the first-commit missing bridge import in thinking.zig is gone. Inner @src() in moved files still churns widget ids once on the new artifact (status pack, chip label) — expand maps stay integer-keyed and persist. test-rich still does not compile ui.zig / ui/*; the next missing import only fails on the wasm step (already happened once this PR). Nine vitest failures named in the PR body are the stale checked-out wasm, not this diff (no TS). No operator frame smoke of the PR artifact was exercised here.
Merge guidance
- PASS WITH NOTES: safe to merge from this attack.
- Before or in a follow-up: delete
lifecycleLabel+ thebridgeimport fromkinds.zig; revert AGENTS.mdlifecycle→spinner. Optional: retarget the staleui.zigcomments.
What was not attacked
Live DO runner, prod Gateway, Vercel preview of the PR wasm artifact, pixel-diff / operator smoke of composer hug · chip click · tool-run L0/L2 persist · thinking collapse on this build.
…d, update stale ui.zig comments
Summary
Split
native/harness/src/ui.zig(1628 lines) into a facade +src/ui/*siblings without changing pixels, protocol, or palette.What moved
ui/state.zigui/metrics.zigTOUCH_H,STATUS_BAR_H, composer caps, etc.)ui/scroll.zigisNearBottom,clampScrollToContent,scrollToBottomui/kinds.zigkindLabel,kindTextColor,kindFill,lifecycleLabelui/chrome.zigchrome*Font,composerIconFont,paintStatusChip,toolRunClipboardui/toolrun.zigpaintToolRun,isCommandLikeRun,detailUsesMonoui/thinking.zigpaintThinking,thinkingPreviewui/chip.zigpaintLastUserChipui/status.zigpaintStatusSlots,truncateStatusValue,truncateToWidthPx,utf8CharLen,statusPackMaxWidthui/skill.zigpaintSkillAttachedui/composer.zigclearPrompt,submitTextWhat stays
ui.zig= facade:BUILD_ID,onInit,onDeinit,frame()main.zigstill@import("ui.zig")— no changebuild.zigno change — relative@imports resolve automaticallyImport graph (acyclic)
No cycle:
thinking → chrome,toolrun → chrome,chromedoes not import either.chip → status,statusdoes not importchip.stateimports no paint file.Living docs
docs/feature-divide.md— Zig UI row: facade +ui/*.zigdocs/harness-limits.md—truncateToWidthPx→ui/status.zig; ScrollInfo →ui/state.zig;paintToolRun→ui/toolrun.zigdocs/agent-stream.md— Thinking paint →ui/thinking.zigAGENTS.md— stream-chrome + tool-run rows updatedTest results
npx tsc --noEmitnpx vitest runPlan
Post-merge
Needs
build-harnesson the merge commit to compile the Wasm artifact. Thetest-richstep will also verify the import graph compiles.