Skip to content

feat(harness): wide modal help-overlay table, wheel stays in panel, backdrop-close (plan #781) - #783

Merged
btipling merged 5 commits into
mainfrom
plan/help-overlay
Aug 22, 2026
Merged

feat(harness): wide modal help-overlay table, wheel stays in panel, backdrop-close (plan #781)#783
btipling merged 5 commits into
mainfrom
plan/help-overlay

Conversation

@btipling

Copy link
Copy Markdown
Owner

Plan #781 — wide modal help-overlay table · wheel stays in panel · backdrop-close

Implements plan #781 (source issue #780): the in-canvas help overlay was a tiny 460×320 absolute box that leaked scroll to the transcript and painted red duplicate-id outlines.

What changed

  • Modal floatingWindow subwindow (was an absolute dvui.box painted after the transcript). Being a modal subwindow, windowFor routes pointer + wheel to it before the earlier-built transcript scrollArea, so scrolling stays in the panel — the transcript's state.transcript_scroll never moves while open (Goal 2).
  • Backdrop click-outside closes (NEW at plan: harness help overlay — wide modal table, wheel stays in panel, no duplicate-id red outlines #781; explicit behavior once modal, per review) — paint scans an unhandled press/release outside the panel and the caller flips help_overlay_open off. Esc / Ctrl+//leader ? toggling is unchanged (a floatingWindow only routes mouse events, so keymap_dispatch at ui.zig:707 still sees the chords — test 2c documented in the plan).
  • Wide fill-band panel: HELP_OVERLAY_W/H (fixed 460×320) is retired for HELP_OVERLAY_W_FRACTION/H_FRACTION + MIN/FLOOR floors (human-approved cap change, 2026-08-22). Width-first; ~390 px stays on-canvas with no horizontal overflow and internal scroll.
  • Two-column table: fixed chord column (HELP_OVERLAY_CHORD_COL_W) + remaining-width help column, so every chord and every help string starts on a stable x (Goal 4).
  • Loop-unique id_extra on every looping widget (distinct bases per box/chord/help) — removes the duplicate-id red errorOutline (Goal 1).
  • Row order/de-dupe (prev_action per distinct action), KEY_TABLE row+help source, and WARM-muted context-off greying are unchanged. TEAL only; EMBER never.

Tests

help_overlay.test.zig extended past the #761 string pins to paint locks under the dvui testing backend:

  • wide band fills most of the band (≫ the old 460 cap) and is tall
  • two-column: chord and help left edges align per column
  • ~390 px band stays on-canvas with internal scroll
  • wheel over the panel scrolls the overlay's own list (Goal 2)
  • backdrop click-outside closes; click inside does not (2b)

Gates

All green locally: zig fmt --check · zig build test-rich (help_overlay 65/65) · zig build test-rich-invariants · zig build harness -Doptimize=Debug (full Wasm compile).

Ops / docs

  • Cloud ops: N/A — no Production mutate (in-canvas harness chrome only; no GHA added).
  • Living docs: docs/harness-limits.md (Keyboard row + new "Help overlay" section), AGENTS.md ownership row (metrics/caps names landed).

Refs #781 — implementation (not merged).

…ackdrop-close (plan #781)

help_overlay: convert the absolute post-paint box into a modal dvui.floatingWindow subwindow so pointer+wheel stay in the panel (never scroll the transcript), close on a backdrop click-outside (NEW), and fill most of the band (retire the fixed 460x320 HELP_OVERLAY_W/H cap).

Two-column table (fixed chord column + remaining-width help column) so every chord/help string starts on a stable x; loop-unique id_extra per widget kind removes the duplicate-id red outlines; row de-dupe + KEY_TABLE order + WARM-muted context-off rows unchanged.

metrics: HELP_OVERLAY_W/H retired for HELP_OVERLAY_W_FRACTION/H_FRACTION + MIN/FLOOR + HELP_OVERLAY_CHORD_COL_W (human-approved cap change 2026-08-22).

tests: help_overlay.test.zig extended past string pins to paint locks (wide panel, two-column x, ~390px on-canvas + internal scroll, wheel-isolation, backdrop-click close).

docs: harness-limits.md (Keyboard row + Help overlay section), AGENTS.md ownership row.

Gates: zig fmt --check; zig build test-rich (help_overlay 65/65); test-rich-invariants; zig build harness -Doptimize=Debug.
@btipling btipling added ui harness Harness / agent UI labels Aug 22, 2026
@vercel

vercel Bot commented Aug 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
invincible Ignored Ignored Aug 22, 2026 9:08pm

Request Review

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #783

Verdict: CONCERNS
Repo: btipling/invincible
Scope: mainplan/help-overlay · 6 files · in-canvas help overlay (modal floatingWindow, band-fraction size, two-column table, backdrop-close)
Lenses run: L1, L3, L6, L8, L9 (skip: L2 no trust-boundary/API/secret/workflow files; L4 no GHA/wasm-artifact paths; L5 no poll/history/body-size change; L7 no host/deploy pin; L6 DI/cost N/A — zig dvui_testing, not PGlite/createDbConnection)
AGENTS.md read: yes (docs/feature-divide.md also — overlay stays Wasm; no dual-chat)

Findings

Sev Lens Finding Break scenario Refutation attempt Confidence
Major L6 Goal 2 is not locked. help_overlay.test.zig · "wheel over the panel scrolls the overlay list, not the transcript" mounts only help.paint — no transcript scrollArea, no state.transcript_scroll. The test comment admits the transcript is not in the frame. The assertion is ctx_scroll.offset increased. Revert floatingWindow to the old absolute dvui.box (the #780 leak). Overlay list can still scroll; production state.transcript_scroll moves with the wheel. CI stays green because nothing in the test frame is the transcript. Defender: “offset moving proves the overlay captured the wheel; a modal windowFor cannot also deliver to the transcript.” That is only true if the production widget tree and paint order stay this shape. The test does not construct that tree, so it cannot fail the bug this PR exists to fix. high
Minor L6 Plan test 2c (Esc / Ctrl+/ / leader ? still reach keymap_dispatch while the modal is open) is not implemented. #781 named “modal wall swallows Esc” as a risk; the PR text treats “documented in the plan” as a substitute for a lock. Open overlay, press Esc. If a dvui bump (or a child) starts marking Esc handled, help_close never runs and the overlay sticks. No test in help_overlay.test.zig injects a key while paint is up. Existing keymap.zig cases never build the modal. Defender: traced vendored FloatingWindowWidget.processEventsAfter (pin 4f810ef) — it only handles tab next/prev; keymap_dispatch walks unhandled keys and ignores eventMatch / target_windowId. Current pin is probably fine. That is an untested invariant on the exact surface the plan required locking. high
Minor L1 help_overlay.paint · dvui.floatingWindow sets .resize = .none but never dragAreaSets an empty/header rect. Default drag_area is the full panel; processEventsAfter starts a middle-drag on an unhandled press inside. Next frame rect_store is recomputed from the band, so the panel snaps back. Open help. Click-drag on the title / padding (not a textLayout). Panel translates with the cursor for the rest of the frame, then jumps back to the centered band rect. Defender: children handle presses on rows, so drag only starts on chrome; snap-back bounds the displace. Still a visible grab/glitch on the new modal, and it fights any press-drag on unhandled interior pixels. high
Minor L6 Two-column x-lock only compares overlay-chord-0/1 and overlay-help-0/1. After prev_action de-dupe those are submit and queue_save — both rowChord = "Ctrl/Cmd+Enter". Goal 4’s hard case (/Esc vs the long chords) is not asserted. Shrink HELP_OVERLAY_CHORD_COL_W so "Ctrl/Cmd+Enter" grows past the min and "↑" does not. Help x diverges on later rows. Test still passes. Defender: min_size_content.w = HELP_OVERLAY_CHORD_COL_W (140) equalizes short chords. Likely true today; the test still does not lock the claim it titles. high
Minor L1 Help textLayout has no .expand = .horizontal / max width. Chord column reserves 140px. On a ~390 band: panel_w = min(max(0.92·390, 360), 390−32) = 358, minus padding/border/row pad → ~180px for help. horizontal = .none on ctx_scroll. Long copy ("Thinking default collapsed", "Send (enqueue when busy)") can clip instead of wrap. The ~390 test only checks the scroll-area widget rect stays on-canvas. Open help on a ~390-wide canvas. Long help strings clip at the panel edge; no h-scrollbar (repo no-h-scroll) and no wrap. Defender: textLayout may still wrap to the box-allocated width. Without expand, rectFor sizes to unwrapped min_size, so wrap is not guaranteed. Did not rasterize glyphs here — medium. medium
Nit L8 native/harness/build.zig help_overlay test block still says “no frame” / “paint is not invoked”. Tests now testing.init + settle + paint. Next agent skips adding a dvui_testing dep or misreads the target as pure. Comment-only; the import is already dvui_testing. high
Nit L1 pub var ctx_scroll is never reset when the overlay closes (ui.zig flips help_overlay_open; New/Clear does not zero the ScrollInfo). Scroll the list, Esc, reopen — list resumes mid-table, first rows off-viewport. Might be desirable memory; still surprising for a help cheatsheet. high

Residual risk

Did not run zig build test-rich in this workspace (no Zig 0.16 on PATH). Keyboard-through-modal is inferred from the vendored FloatingWindowWidget pin, not executed. First-frame floatingWindow hide (w=h=0) plus focusSubwindow is stock dvui; composer focused_widget_id on the base window should survive close via Window.end unused-subwindow fallback — not operator-checked. Backdrop close on press-or-release is unhandled-event scanning after fw.deinit; same-frame click-through to composer is probably prevented by mouse.floating_win tagging, but test 2b never mounts composer/queue-band to prove “underlying not steered.”

Merge guidance

  • CONCERNS: do not merge on the Goal 2 test as written. Mount a transcript (or a stand-in ScrollInfo painted before the overlay, same order as ui.zig) and assert its offset is unchanged when the wheel hits overlay-scroll-area. While there: add 2c (Esc / Ctrl+/ with the modal up), compare a short-chord row against a long-chord row, dragAreaSet(.{}) (or header-only), and give the help column a real remaining-width wrap. Minors/nits can ride with an explicit accept; the Goal 2 lock cannot.

What was not attacked

Live DO runner, prod Gateway, Wasm in-browser operator pass (duplicate-id console, real trackpad, ~390 device), zig build test-rich / test-rich-invariants / harness -Doptimize=Debug execution in this session, dvui versions other than pin 4f810ef1d695b55fb714a13536a930b9304d619e.

@btipling

btipling commented Aug 22, 2026

Copy link
Copy Markdown
Owner Author

address_pr_feedback #783 — living planning comment (round 3)

Review: adversarial round 3 → CONCERNS (head e600706). Round 2 was PASS WITH NOTES; round 3 re-attacked e600706 and found 3 Minor + 2 Nit in the in-canvas help overlay + its test. DoD fixed below in one pass (commit 0d71b84), pushed.

Items → status

# Sev Finding (round 3) Fix (this round) Done
1 Minor L6 "middle-drag" test could not fail the dragAreaSet(.{}) lock: enums.Button.pointer() is left/touch only, so an injected .middle press never reaches processEventsAfter's drag handler — CI stayed green with the empty drag area reverted Test now drives .left press alone on the panel's left padding ring; rename left-drag ... (Minor L1); comment records why .middle was vacuous
2 Minor L1 resetScroll() only called on help_close + backdrop-close; help_toggle, help_toggle_leader, and New/Clear (ui.zig:520) skipped it → reopen resumed mid-table after Ctrl+//leader-?/New help_toggle + help_toggle_leader now call help_overlay.resetScroll(); ui.zig New/Clear path now calls it too (single authoritative UI site; state.zig stays paint-file-free by policy)
3 Minor L1+L6 Help-column wrap set max_size_content.h = TOUCH_H-6 AND min_size_content.h = TOUCH_H-6; WidgetData.init/minSizeSetAndRefresh clamp min to max → wrapped lines clipped in a one-line well at ~390. 800px wrap test only proved .expand = .horizontal, not wrap-vs-clip Dropped the height cap: .max_size_content = .width(help_max_w) (width ceiling + unbounded height via MaxSize.width). New test: the SAME long help string measures taller at a ~390 band than its one-line height at 800 (robust cross-width proof of wrap-not-clip)
4 Nit L6 Test 2c never asserted a key event was present; empty event list vacuously satisfied "none handled" Frame now records any_key_present; test asserts both presence and unhandled
5 Nit L8 help_max_w hardcoded panel pad 12 / row pad 4 instead of the Options.padding literals; docs said backdrop-close is "NEW" Shared PANEL_PAD_X/ROW_PAD_X constants used by both padding and the wrap-ceiling arithmetic; docs/harness-limits.md aged "NEW", documents reset-to-top + wrap-not-clip

Key decisions / deviations

  • resetScroll at New/Clear: routed through the one authoritative ui.zig site (which already imports help_overlay) rather than state.zigstate.zig's documented policy is "imports NO other ui/* paint file (no cycles)". Same observable outcome (reopen after New/Clear resumes at top).
  • Goal 2 not re-opened: the transcript stand-in dual-offset wheel lock from a456b44 is intact and still fails a box regression — untouched this round.

Gates (green locally, harness-only — no TS gates apply)

zig fmt --check (all 4 touched zig) · zig build test-rich exit 0 (help_overlay 70/70, incl. new/renamed left-drag, presence, wrap-not-clip locks) · zig build test-rich-invariants · zig build harness -Doptimize=Debug (full Wasm compile).

State

Branch plan/help-overlay pushed e600706..0d71b84, tree clean, PR #783 OPEN (no merge). No deferrals — all 5 items addressed.

Next: re-run /adversarial_review on updated head 0d71b84 (implementer doesn't self-review); a clean PASS-with-NOTES is the merge-ready signal.

…#783 Major L6)

The Goal-2 wheel-isolation lock was vacuous: it mounted only help.paint, so a
wheel that leaked to the transcript (the #780 bug) could not fail it. Frame.paint
now builds a transcript scroll container with overflow rows BEFORE the overlay
modal (same order ui.zig paints after the transcript laid out), and the test
asserts BOTH infos: overlay ctx_scroll offset moves while the transcript
stand-in offset stays put — a regression to the absolute dvui.box now moves the
transcript offset and fails the test.

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #783 (round 2)

Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: mainplan/help-overlay · 6 files · 2 commits (a002060 + a456b44 transcript stand-in for Goal 2)
Lenses run: L1, L3, L6, L8, L9 (skip: L2 no trust-boundary/API/secret files; L4 no GHA/artifact path change — zig → harness.wasm green on HEAD; L5 no poll/history/body-size; L7 no host pin; L6 DI/cost N/A — existing dvui_testing backend, not PGlite)
AGENTS.md read: yes (docs/feature-divide.md — keymap/help stays in-canvas; no DOM cheatsheet)

Round 1 was CONCERNS (Major L6: wheel test mounted only help.paint, so a transcript leak could not fail). a456b44 addresses that. This pass re-attacks HEAD, including the follow-up.

Findings

Sev Lens Finding Break scenario Refutation attempt Confidence
Minor L6 Plan #781 test 2c is still absent: no paint+dispatch lock that Esc / Ctrl+/ / leader ? still reach keymap_dispatch while the modal is open. DoD and the plan risk table both named “modal wall swallows Esc.” A later dvui pin where FloatingWindowWidget.processEventsAfter handles more than Tab (or a first-frame focus sink marks Esc handled) ships a help overlay that cannot be dismissed from the keyboard. Click-outside still works; Busy Esc-wins is unproven in this tree. Defender: keymap_dispatch.dispatch still walks raw dvui.events() after overlay paint (ui.zig ~714). Vendored FloatingWindowWidget only consumes Tab next/prev, not Escape. keymap.zig already pins help_open → help_close. Residual is the combined modal+dispatch path, not the table. high
Nit L8 docs/harness-limits.md Help overlay still says backdrop-close is “NEW”; metrics comments cite “plan #781”. Living-docs rule was timeless / no process artifacts. Harmless for operators; next doc pass treats “NEW” as current. Caps/names themselves are correct. high
Nit L6 Wheel test never resets help.ctx_scroll (only Frame.transcript_scroll). File order currently paints-then-wheels once, so after > before holds. A later test above it that wheels, or shuffled Zig order, leaves the overlay at scrollMax; after > before fails green-to-flake, or a no-op wheel looks like a product regression. Sequential source order today. Cheap ctx_scroll = .{} in the test setup. medium

What the follow-up actually locked (L6)

Frame.paint now builds a transcript scrollArea with overflow rows before help.paint, matching ui.zig (transcript deinit/processEvents runs first). The Goal 2 test asserts both infos: overlay ctx_scroll moves and the stand-in offset stays put.

That is the real #780 leak: without a subwindow, windowFor is the base window, the earlier transcript consumes the wheel, overlay offset would not move / transcript offset would. A revert to the pre-#781 absolute dvui.box now fails CI. Round-1 Major L6 is closed.

(Not required for Goal 2: dropping .modal = true but keeping a floatingWindow would still pass the wheel test, because windowFor hits the overlay rect. Modal remains locked by the backdrop-click test, not by the wheel test.)

Residual risk

Did not run zig build test-rich here. GHA zig → harness.wasm is green on a456b44. Keyboard dismiss through the modal is source-plausible (see 2c refutation) but untested as a pair. Operator: no live /harness wheel/Esc. pub var ctx_scroll is not zeroed on close (reopen keeps last offset) — pre-existing shape, not a leak.

Merge guidance

  • PASS WITH NOTES: safe to merge from this attack. The Goal 2 lock is no longer vacuous.
  • Optional same-tree follow-up: a 2c paint+dispatch Esc/Ctrl+/ case, reset ctx_scroll in the wheel test, drop “NEW” from living docs. None of those should gate the merge.

What was not attacked

Live /harness wheel + Esc, test-rich / test-rich-invariants in this session, dvui versions other than pin 4f810ef, composer click-through on the same frame as a backdrop close (modal windowFor should deny it; not driven).

…agAreaSet, wrap, scroll reset (increments 2-7)

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #783 (round 3)

Verdict: CONCERNS
Repo: btipling/invincible
Scope: mainplan/help-overlay · 8 files · 3 commits (HEAD e600706 — keys-through-modal, dragAreaSet, wrap, scroll reset)
Lenses run: L1, L3, L6, L8, L9 (skip: L2 no trust-boundary/API/secret files; L4 no GHA/artifact path change — zig → harness.wasm green on HEAD; L5 no poll/history/body-size; L7 no host pin; L6 DI/cost N/A — existing dvui_testing backend, not PGlite)
AGENTS.md read: yes (docs/feature-divide.md — keymap/help stays in-canvas; no DOM cheatsheet)

Round 1 CONCERNS (vacuous Goal 2). Round 2 PASS WITH NOTES after the transcript stand-in. This pass re-attacks e600706.

Round-2 items

Round-2 Status on e600706
Minor L6 plan test 2c (Esc / Ctrl+/ through modal) Partially closed. New test asserts those key events stay unhandled after overlay processEventsAfter. Does not run keymap_dispatch or flip help_overlay_open. Necessary, not sufficient.
Nit L8 living-docs “NEW” / plan #781 Unchanged (docs/harness-limits.md still says “NEW — it is a modal”).
Nit L6 wheel test never resets ctx_scroll Closed for close-via-Esc / backdrop (resetScroll). Not closed for Ctrl+/ toggle, leader ? toggle, or New/Clear.

Goal 2 dual-offset wheel lock from a456b44 is intact.

Findings (HEAD)

Sev Lens Finding Break scenario Refutation attempt Confidence
Minor L6 “middle-drag” test cannot fail the dragAreaSet(.{}) lock. Vendored enums.Button.pointer() is left or touch only (enums.zig:136-138). FloatingWindowWidget.processEventsAfter starts a drag only inside if (me.button.pointer()). The test injects .middle press/release, which the handler never sees. before/after rects match with or without the empty drag area. A revert of fw.dragAreaSet(.{}) leaves CI green. An unhandled left press on the padding ring still one-frame-drags the panel (the snap-back the comment describes). Defender: production empty drag_area is the right fix; .contains on {} is false. The lock is just pointed at the wrong button (dragPart .middle ≠ mouse .middle). high
Minor L1 resetScroll() is only called from help_close and backdrop-close. help_toggle / help_toggle_leader flip help_overlay_open without it; New/Clear (ui.zig:520, state.zig:182) also skip it. Operator scrolls the table, hits Ctrl+/ (documented close), opens again → list is mid-table. Same after New/Clear in a live Wasm instance. The new invariant (“reopen resumes at the top”) does not hold on the primary toggle. Defender: Esc and click-outside are covered. Ctrl+/ is still the Keyboard-row toggle. Sticky offset is not data loss, but it is the bug they just named. high
Minor L1+L6 Help-column wrap sets .max_size_content.h = TOUCH_H - 6 and .min_size_content.h = TOUCH_H - 6. WidgetData.init / minSizeSetAndRefresh clamp min_size to max_sizeGet(), so a wrapped string cannot report a taller min — extra lines clip inside a one-line well. The new wrap test runs at 800×600, where help_max_w ≈ 564 and “Send (enqueue when busy)” fits one line; equal widths only prove .expand = .horizontal, not wrap-vs-clip. At ~390 (help_max_w ≈ 186) the long help strings wrap inside a 34 px cap and clip. The ~390 overflow test only checks the scroll-area vs win.w, not row height or visible wrap. Defender: break_lines default is true; .expand = .horizontal does force leftover width (the 800 px test). Ceiling help_max_w is fine. The height cap is what undoes wrap at the phone width the plan cares about. high
Nit L6 Test 2c never asserts a key event was present in dvui.events(), and never calls keymap_dispatch. An empty key list still yields any_key_handled == false. A pressKey / step mismatch would keep 2c green while Esc is swallowed elsewhere. pressKey does addEventKey down+up before step, so this frame’s events() should contain them. Overlay source still only handles Tab next/prev. Residual is the missing dispatch/close assertion. medium
Nit L8 help_max_w hardcodes panel pad 12 and row pad 4 instead of the same literals the Options.padding uses. Docs still say backdrop-close is “NEW”. Pad drift desyncs the wrap ceiling. “NEW” ages into the timeless row. Values currently match. high

Residual risk

Did not run zig build test-rich here. GHA zig → harness.wasm is green on e600706. keymap_dispatch → help_overlay import is acyclic. Operator: no live /harness wrap at 390, left-drag on padding, or Ctrl+/ reopen. pub var ctx_scroll still lives across a toggle-close.

Merge guidance

  • CONCERNS: do not merge on the wrap-height clip or the vacuous drag lock. Cheap fixes: drop max_size_content.h (keep width ceiling / expand); drive the drag test with .left (or press+motion that pointer() accepts); call resetScroll() whenever the overlay goes false (help_toggle, leader ?, New/Clear).
  • Do not re-open Goal 2. The transcript stand-in dual-offset lock still fails a box regression.
  • 2c as “keys unhandled” is acceptable if the three production misses above are fixed; adding a dispatch/close assertion is optional.

What was not attacked

Live /harness at 390, test-rich / test-rich-invariants in this session, dvui pins other than 4f810ef, first-frame floatingWindow hide vs composer focus.

…croll on toggle/New/Clear, wrap-height clip, shared pad constants (plan #781)

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #783 (round 4)

Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: mainplan/help-overlay · 8 files · 4 commits (HEAD 0d71b84 — left-drag lock, resetScroll on toggle/New/Clear, wrap-height, shared pad constants)
Lenses run: L1, L3, L6, L8, L9 (skip: L2 no trust-boundary/API/secret files; L4 no GHA/artifact path change — zig → harness.wasm green on HEAD; L5 no poll/history/body-size; L7 no host pin; L6 DI/cost N/A — existing dvui_testing backend, not PGlite)
AGENTS.md read: yes (docs/feature-divide.md — keymap/help stays in-canvas; no DOM cheatsheet)

Round 1 CONCERNS (vacuous Goal 2). Round 2 PASS WITH NOTES (transcript stand-in). Round 3 CONCERNS (wrong mouse button on drag lock, incomplete resetScroll, wrap height cap, 2c empty-list hole). This pass re-attacks 0d71b84.

Round-3 items

Round-3 Status on 0d71b84
Minor L6 drag test used .middle (Button.pointer() is left/touch only) Closed. Test now injects .left press/motion/release on the padding ring. Revert of dragAreaSet(.{}) would translate the panel this frame (12*PXCOL_EPS).
Minor L1 resetScroll skipped on Ctrl+/, leader ?, New/Clear Closed on the named paths: help_toggle / help_toggle_leader always reset; New/Clear n==0 block in ui.zig resets; Esc and backdrop still reset.
Minor L1+L6 wrap max_size_content.h = TOUCH_H-6 clipped; wrap test only at 800 px Closed. .max_size_content = .width(help_max_w) (MaxSize.h = max_float_safe). New test paints the same long string at 800 then at a 390 band and asserts narrow_h > one_line_h + PX.
Nit L6 2c never asserted a key event was present Closed. any_key_present is set from dvui.events() and asserted.
Nit L8 help_max_w hardcoded 12/4; docs said “NEW” Closed. PANEL_PAD_X / ROW_PAD_X shared with Options.padding. Docs dropped “NEW”; reopen-at-top + wrap-not-clip are stated.

Goal 2 dual-offset wheel lock from a456b44 is intact.

Findings (HEAD)

Sev Lens Finding Break scenario Refutation attempt Confidence
Nit L1 state.resetTranscriptScroll() (onInit) still sets help_overlay_open = false without help_overlay.resetScroll(). In-process re-init (comment at ui.zig:137 — wasm reload / host re-mount without module re-zero) leaves ctx_scroll mid-table; next open is not at the top. Named close paths (Esc, backdrop, toggle, New/Clear n==0) reset. A full Wasm instantiate would re-zero the pub var. Edge is in-process onInit only. high
Nit L8 help_overlay.zig drag comment still says “unhandled middle-drag press” after the lock was corrected to left/touch pointer(). Next reader re-introduces a .middle test. Production dragAreaSet(.{}) is button-agnostic (empty rect). Test name/body now say left. high

No surviving Major/Minor on wrap, drag, or the documented close paths.

Residual risk

Did not run zig build test-rich here. GHA zig → harness.wasm is green on 0d71b84. 2c still does not call keymap_dispatch / flip help_overlay_open (keys-unhandled is the actual modal-wall risk; dispatch remains a raw dvui.events() walk after paint). Wrap-height test reuses the 800×600 window and restyles the band to 390 — help_max_w is from paint’s band args, so wrap math matches production; a tiny fallback font that still fits the long string on one line at 186 px would false-fail, not false-pass. Operator: no live /harness wrap at 390 or Ctrl+/ reopen.

Merge guidance

  • PASS WITH NOTES: safe to merge from this attack. Round-3 wrap clip, vacuous drag lock, and toggle/New/Clear reset are actually closed.
  • Optional: resetScroll() inside resetTranscriptScroll / onInit; drop “middle-drag” from the comment. Neither should gate the merge.
  • Do not re-open Goal 2.

What was not attacked

Live /harness at 390, test-rich / test-rich-invariants in this session, dvui pins other than 4f810ef, first-frame floatingWindow hide vs composer focus.

@btipling

Copy link
Copy Markdown
Owner Author

address_pr_feedback #783 — round 4 (PASS WITH NOTES, 2 Nits) resolved

Adversarial round 4 on PR #783 returned PASS WITH NOTES (head 0d71b84) with two optional Nits, both addressed in commit 9d01d42 (simple direct-fix round — no design work, no planning comment).

# Sev Finding Fix Done
1 Nit L1 state.resetTranscriptScroll() (onInit, ui.zig:135) closes the overlay (help_overlay_open = false) but never help_overlay.resetScroll() — an in-process re-init (wasm reload / host re-mount) leaves ctx_scroll mid-table, so the next open resumes mid-list Added help_overlay.resetScroll() in ui.zig onInit (the authoritative re-init site); state.zig stays free of ui paint imports per policy
2 Nit L8 Stale help_overlay.zig drag comment still said "middle-drag press" after the drag lock was corrected to left/touch pointer() Reworded to "unhandled press on interior chrome" + recorded that the test drives a LEFT press (.middle can't reach the handler)

Gates (green locally, harness-only)

zig fmt --check (both touched) · zig build test-rich exit 0 · zig build test-rich-invariants · zig build harness -Doptimize=Debug (full Wasm compile).

State

Branch plan/help-overlay pushed 0d71b84..9d01d42, tree clean, PR #783 OPEN (no merge). No deferrals.

Next (implementer): re-run /adversarial_review on updated head 9d01d42 to confirm the PASS-with-NOTES holds — the clean re-review is the merge-ready signal.

@btipling
btipling merged commit 846e9c2 into main Aug 22, 2026
3 checks passed
@btipling
btipling deleted the plan/help-overlay branch August 22, 2026 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

harness Harness / agent UI ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant