composer: reserve trailing icon pack, bound field to leftover (#734 / #737) - #744
Conversation
…#734) (#737) Extract the composer chrome into ui/composer_chrome.zig paintComposerChrome so a host dvui frame test can drive the real surface. The field now sits on an explicit trailing-reserved sub-rect of width field_w = avail_w - (TOUCH_H*n + 8): min=max=field_w caps its reported min (WidgetData.minSizeSetAndRefresh) so a long unbreakable line's natural width can never squeeze the Send/Stop icons off-canvas. Dynamic-hug height clamp + Enter-chord submit + Submit/Stop dispatch stay in frame(); the chrome module takes inert callbacks so the test paints without Wasm glue. Adds ui/composer_chrome.zig (paintComposerChrome + fieldW/iconReserveW/iconPackW), composer_layout.test.zig (4 tests, incl. the #734 long-space-less-line lock + reserve-seam proof + multi-line grow), build.zig composer_layout addTest (dvui + web-backend stub), and refreshes docs/harness-limits.md Wrap/grow + Icon button rows. No cap changed; scroll_horizontal=false (#702) untouched; no host/app/lib change.
|
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 #744
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← fix/composer-textentry-squeeze · 5 files · reserve trailing ▶/■ strip, bound composer field to leftover
Lenses run: L1, L3, L4, L6, L8, L9 (skip: L2 no secrets/API; L5 layout math not a frame alloc; L7 no clone bind)
AGENTS.md read: yes · docs/feature-divide.md read (Wasm composer stays primary)
The extraction + reservation is the right shape: fieldW = avail_w − (TOUCH_H×n + 8), wrap min=max=field_w, packed gravity.x=0 (not a 0–1 overlay). #734 lock on 200×A keeps wrap width and send.x on the reserved seam — that is a real layout-rect test, not a vacuous const. Enter-chord / Esc / history stay in frame() before paint; submit/stop are injected callbacks. Caps unchanged. scroll_horizontal = false untouched. Palette TEAL/WARM/EMBER on the same buttons as main.
Findings
| Sev | Lens | Finding | Break scenario | Refutation attempt | Confidence |
|---|---|---|---|---|---|
| Minor | L6 | #734 lock is idle-only. Busy reserve is 2×TOUCH_H+8 (tighter leftover). Test 2 paints busy with an empty buffer; the 200×A case never runs with ▶+■. |
Operator pastes a long unbreakable line during Busy (enqueue path). If wrap clamp fails only when two icon mins compete, idle test stays green and ■ slides off the right. | Wrap max=field_w(busy) is the same mechanism; idle lock is strong evidence. Still not the failing leftover the product hits while inferencing. Cheap: copy the 200×A body into the busy test. |
high |
| Minor | L8 | Wrap/grow docs still say all min/max_size_content are passed minus 2×COMPOSER_TE_PAD (the #584 bake). Height still does that; width max is raw field_w (composer_chrome.zig 151–152). Bake adds 2×TE_PAD to the passed max (main-era comment: 0 + 2×TE_PAD → 10 px well). |
Next composer-height agent copies “minus 2×pad” onto max_size_content.w and shrinks wrap by 10 px, or assumes width is already bake-compensated. Reservation still holds because wrap min=max=field_w is the outer clamp (200×A test proves wrap.w). |
Inner 10 px is clipped by the wrap; icons stay. Doc drift only — unless someone “fixes” width to minus-pad. | high |
Residual risk
- Tests are all
WIN_LW=600. ~390 px still hasfield_w ≫ 120; absurdly narrow (field_w→0, textEntry min 120 vs wrap max 0) is the existing short-canvas class, not this defect. TE_MARGIN_RIGHT=8is subtracted fromfield_wand applied as the textEntry’s.margin.winside the wrap, so the row may leave ~8 px slack on the right. That slack may accidentally absorb default button margin; it is not a second squeeze. Tests do not lock flush-to-edge.- Zig
test-rich/ wasm32build-harnesswere not re-run in this review environment.
Merge guidance
- PASS WITH NOTES: safe to merge from this attack; nits optional
- Prefer a busy+200×
Arow incomposer_layout.test.zigin this PR or a fast follow - Do not add a DOM composer to “fix” leftover width
What was not attacked
Host zig build test-rich / test-rich-invariants / wasm32 build-harness, live canvas at 390 px / 200-char paste, dvui BoxWidget leftover internals beyond the wrap clamp the tests already sample, Production harness artifact deploy.
…ght pad docs (review #744 L6/L8)
Address adversarial-review (Round 1) — commit
|
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #744 (re-review @ 56d68b0)
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← fix/composer-textentry-squeeze · 5 files · reserve trailing ▶/■ strip, bound composer field to leftover
Lenses run: L1, L3, L4, L6, L8, L9 (skip: L2 no secrets/API; L5 layout math not a frame alloc; L7 no clone bind)
AGENTS.md read: yes · docs/feature-divide.md read (Wasm composer stays primary)
Prior findings are closed on this HEAD:
| Prior | Status @ 56d68b0 |
|---|---|
L6 Minor #734 lock idle-only |
fixed — busy + #734 lock paints 200×A with T_busy, asserts wrap = fieldW(WIN_LW, true), send.x at 2-slot reserve, ▶ and ■ on-canvas |
| L8 Minor wrap/grow docs said all min/max minus 2×pad | fixed — height minus 2×pad; width max is raw field_w, never pad-shortened |
Reservation still the right shape: fieldW = avail_w − (TOUCH_H×n + 8), wrap min=max=field_w, packed gravity.x=0. Caps unchanged. scroll_horizontal = false untouched. Palette TEAL/WARM/EMBER on the same buttons as main.
Findings
None remaining that survive self-refute.
Residual risk
- Tests are all
WIN_LW=600. ~390 px still hasfield_w ≫ 120; absurdly narrow (field_w→0, textEntry min 120 vs wrap max 0) is the existing short-canvas class, not this defect. TE_MARGIN_RIGHT=8is subtracted fromfield_wand applied as the textEntry’s.margin.winside the wrap, so the row may leave ~8 px slack on the right. Tests do not lock flush-to-edge.- Sibling #746 (keymap) is still OPEN and also rewrites the composer block in
ui.zig(replaces this PR’s inline Enter/Esc/↑↓ scan withkeymap_dispatch). Merge order must rebase; this PR’s reservation math is incomposer_chrome.zigand should survive if the event scan is the only overlap. - Zig
test-rich/ wasm32build-harnesswere not re-run in this review environment.
Merge guidance
- PASS WITH NOTES: safe to merge from this attack; nits optional
- Coordinate with #746 if both land (rebase the later onto the extracted chrome)
- Do not add a DOM composer to “fix” leftover width
What was not attacked
Host zig build test-rich / test-rich-invariants / wasm32 build-harness, live canvas at 390 px / 200-char paste, dvui BoxWidget leftover internals beyond the wrap clamp the tests already sample, Production harness artifact deploy, combined tree with #746.
Summary
Fixes #737 (plan) / #734 (source). The composer
textEntrycould grow horizontally and squeeze the trailing ▶/■ icon pack off-canvas before wrapping when the operator typed/pasted a long single unbreakable line (no spaces →break_linescan't split it). Root cause: dvui's horizontal-box leftover math honors the field's reported min width first — for an unwrapped line that's ~the full line width, so the icons got leftover ≈ 0.Fix: the field now sits on an explicit trailing-reserved sub-rect of width
field_w = avail_w − (TOUCH_H×n + 8)(n = 1 idle / 2 busy; the 8 is the field's right margin). The reserved field wrapper reportsmin = max = field_wto the box (max_size_contentclamps it inWidgetData.minSizeSetAndRefresh), so a long line's natural min width can never push past the reserved icon columns — the icons always land post-reserve atTOUCH_H, never off-canvas, and the field wraps atfield_wthen grows up and scrolls inside as before.Extraction (enables a real test seam): composer chrome moved from an inline block in
ui.zig frame()into a standaloneui/composer_chrome.zigpaintComposerChrome(...), mirroring thebusy_rowextraction contract. The chrome module stays free ofcomposer/bridge/state(submit/stop are injected inert callbacks), so the new host dvui frame-layout test drives the real surface. The dynamic-hug height clamp (composer_last_h), Enter-chord submit, and Submit/Stop dispatch stay inframe().Tests
composer_layout.test.zig(4, wired intozig build test-richwithdvui+web-backendstub):'A'space-less line does not push ▶ off-canvas; wrap width staysfield_w,send.xstays post-reserveGates
zig fmt --checkon touched Zig — cleanzig build test-rich— green (composer_layout 4/4 pass)zig build test-rich-invariants— greenzig build harness -Doptimize=Debug— full Wasm compile cleanapp//lib/TS change; docs-only + Zig-only → no vitest/npm run buildgate (per AGENTS.md layer rule)Non-goals (unchanged)
scroll_horizontal = false(harness: composer text field horizontally scrolls instead of wrapping and growing #702 caret-pan wrap) untouched — wrap still fires at the field widthCOMPOSER_MAX_CHROME_H/COMPOSER_INPUT_MAX_H/COMPOSER_IDLE_CHROME_H/TOUCH_Hall stay;TE_MARGIN_RIGHT = 8is read from the existing field.margin.w, no new cap)Cloud ops
N/A — no Production mutate. The self-hosted
build-harnessGHA compiles the release Wasm on the PR branch (pull_request → main) and onmain(artifact → Vercel via the existing deploy-hook seam) whennative/harness/**changes.Living docs
docs/harness-limits.mdWrap / grow + Icon button rows refreshed to state the field width = leftover after a reserved trailing icon pack. AGENTS.md / README / SECURITY / .env.example N/A (justified per plan; the composer-width contract stays canonical under harness-limits).Fixes #737
Refs #734