feat(harness): single keymap + leader + help overlay for shortcuts (plan #741) - #746
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Implemented — ready for adversarial reviewImplementation of plan #741 is complete and pushed. Summary of the 4 commits on
Verified
No TS/DOM/backend changes → no vitest/typecheck gate applies (Zig-only PR). Next step: adversarial-review on this head. |
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #746
Verdict: REQUEST CHANGES
Repo: btipling/invincible
Scope: main ← plan/harness-keymap · 11 files · single keymap table + leader + help overlay (#741)
Lenses run: L1, L3, L5, L6, L8, L9 (skip: L2 no secrets/API; L4 no workflow; L7 no clone bind)
AGENTS.md read: yes · docs/feature-divide.md read (Wasm owns chords; DOM adds none)
Architecture is the right shape: pure keymap.match, one dvui.events() walk before textEntry, reserved deny-list never marked handled, request_submit after te.deinit, queue handleEscape/submitChord gone. Caps are new (KEYMAP_MAX 64, LEADER_WINDOW_MS 800, HELP_OVERLAY_*) — none raised. dvui.timer(..., micros) × LEADER_WINDOW_MS*1000 is 800 ms. Palette TEAL + WARM-muted grey (not EMBER). Feature-divide holds.
The leader contract the last commit wrote (e780b8c: swallow everything except ? / Esc / re-arm) is broken at the dispatcher, and tests cannot see it.
Findings
| Sev | Lens | Finding | Break scenario | Refutation attempt | Confidence |
|---|---|---|---|---|---|
| Major | L1 | keymap_dispatch.fromDvui(...) orelse continue punches a hole in the leader swallow. Unmapped keys never reach keymap.match, are not marked handled, and do not disarm. Key only lists a subset of letters (c v x a z s f t n w r l p i j) plus a few specials — no b d e g h k m o q u y, digits, Backspace, = , , , etc. Pure tests only feed the enum (leader + unmatched printable uses .a, which is mapped). |
Ctrl+Shift+Space, then h (or 1, or Backspace). h inserts into the composer; leader stays armed. Next mapped letter (l in “help”) swallows and disarms — prompt contains he, overlay may still open on a later ? in the same 800 ms. The #741 “never lands in the prompt” promise is false for most of the keyboard. |
Defender: “unknown keys pass through (Goal 3).” Goal 3 is the reserved deny-list, which already runs inside match. Goal 2 / last-commit swallow is “unmatched key while leader pending → handled + disarm.” continue skips both. Fix: if fromDvui is null and state.leader_armed, mark handled + disarm; else pass through. |
high |
| Minor | L1 | Help overlay silently no-ops when the leftover transcript width is < HELP_OVERLAY_MIN_W (300). paint(pane_w, …, avail.w - pane_w, …) — open rail is 220 px. At ~390: leftover 170. help_overlay_open flips; nothing paints. PR body: “Mobile ~390px: overlay still paints.” |
Open the left rail on a ~390 canvas, hit Ctrl+/. State is open, Esc still closes, operator sees no panel. | Closed rail (40) leaves 350 ≥ 300, so the happy path works. Lower MIN_W or size against avail.w (full band) instead of leftover. |
high |
| Minor | L8 | Overlay is not the table. rowLabel switches on action, so history_older + history_older_in both render “Older message / ↑” (duplicate row). row.help is unused. KEY_TABLE precedence (help_close before leader_cancel) is dead while leader_pending because match() short-circuits to Esc → leader_cancel before the table — overlay-open + leader armed needs two Escapes; docs say help wins. |
Operator opens help, then hits the leader, then Esc. Overlay stays. A later agent “fixes” overlay copy from row.help and still ships the duplicate ↑ row. |
Last commit made the short-circuit explicit; queue-edit Esc also loses to leader (safer). Duplicate is paint-only. | high |
| Nit | L9 | Overlay chord labels ↑/↓ use addText on .theme(.mono). Harness-limits already say Noto has no Arrows block (chip jump hint uses addTextMixed). May tofu in the cheatsheet of all places. |
Open help — arrows are .notdef. |
ASCII Ctrl/Cmd+/ rows are fine. Same class as #732. |
medium |
Residual risk
LEADER_TIMER_ID = 0x7410_0000_0000_0001vs widgetId.extendId— assumed collision-safe, not proven.- Overlay
ctx_scrollis a module-levelvar; reopen keeps the old scroll offset. - Dispatcher has no host test (only pure
keymap.zig). The Major above is exactly the seamtest-richdoes not compile. - Zig
test-rich/ wasm32build-harnesswere not re-run in this review environment.
Merge guidance
- Do not merge until leader-pending unmapped keys are swallowed (or the dispatcher tests a non-enum key and the overlay is visible at ~390 with the rail open).
- Do not add a DOM
windowkeydown / React cheatsheet to paper over the overlay min-width. - Do not silently change
LEADER_WINDOW_MS/KEYMAP_MAX.
What was not attacked
Host zig build test-rich / wasm32 build-harness, live canvas (Ctrl+/ , leader+?, Esc vs menus), dvui timer clock vs rAF, Production harness artifact deploy, #742 leader+t (not in this PR).
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #746 (verdict correction)
Verdict: CONCERNS
Repo: btipling/invincible
Scope: main ← plan/harness-keymap · 11 files · single keymap table + leader + help overlay (#741)
Lenses run: L1, L3, L5, L6, L8, L9 (skip: L2 no secrets/API; L4 no workflow; L7 no clone bind)
AGENTS.md read: yes · docs/feature-divide.md read (Wasm owns chords; DOM adds none)
Correction: the previous comment used REQUEST CHANGES, which is not a skill verdict. Allowed: BLOCK | CONCERNS | PASS WITH NOTES | INCOMPLETE. One Major + no Blocker → CONCERNS. Findings unchanged from the attack @ e780b8c.
Architecture is the right shape: pure keymap.match, one dvui.events() walk before textEntry, reserved deny-list never marked handled, request_submit after te.deinit, queue handleEscape/submitChord gone. Caps are new (KEYMAP_MAX 64, LEADER_WINDOW_MS 800, HELP_OVERLAY_*) — none raised. dvui.timer(..., micros) × LEADER_WINDOW_MS*1000 is 800 ms. Palette TEAL + WARM-muted grey (not EMBER). Feature-divide holds.
The leader contract the last commit wrote (e780b8c: swallow everything except ? / Esc / re-arm) is broken at the dispatcher, and tests cannot see it.
Findings
| Sev | Lens | Finding | Break scenario | Refutation attempt | Confidence |
|---|---|---|---|---|---|
| Major | L1 | keymap_dispatch.fromDvui(...) orelse continue punches a hole in the leader swallow. Unmapped keys never reach keymap.match, are not marked handled, and do not disarm. Key only lists a subset of letters (c v x a z s f t n w r l p i j) plus a few specials — no b d e g h k m o q u y, digits, Backspace, = , , , etc. Pure tests only feed the enum (leader + unmatched printable uses .a, which is mapped). |
Ctrl+Shift+Space, then h (or 1, or Backspace). h inserts into the composer; leader stays armed. Next mapped letter (l in “help”) swallows and disarms — prompt contains he, overlay may still open on a later ? in the same 800 ms. The #741 “never lands in the prompt” promise is false for most of the keyboard. |
Defender: “unknown keys pass through (Goal 3).” Goal 3 is the reserved deny-list, which already runs inside match. Goal 2 / last-commit swallow is “unmatched key while leader pending → handled + disarm.” continue skips both. Fix: if fromDvui is null and state.leader_armed, mark handled + disarm; else pass through. |
high |
| Minor | L1 | Help overlay silently no-ops when the leftover transcript width is < HELP_OVERLAY_MIN_W (300). paint(pane_w, …, avail.w - pane_w, …) — open rail is 220 px. At ~390: leftover 170. help_overlay_open flips; nothing paints. PR body: “Mobile ~390px: overlay still paints.” |
Open the left rail on a ~390 canvas, hit Ctrl+/. State is open, Esc still closes, operator sees no panel. | Closed rail (40) leaves 350 ≥ 300, so the happy path works. Lower MIN_W or size against avail.w (full band) instead of leftover. |
high |
| Minor | L8 | Overlay is not the table. rowLabel switches on action, so history_older + history_older_in both render “Older message / ↑” (duplicate row). row.help is unused. KEY_TABLE precedence (help_close before leader_cancel) is dead while leader_pending because match() short-circuits to Esc → leader_cancel before the table — overlay-open + leader armed needs two Escapes; docs say help wins. |
Operator opens help, then hits the leader, then Esc. Overlay stays. A later agent “fixes” overlay copy from row.help and still ships the duplicate ↑ row. |
Last commit made the short-circuit explicit; queue-edit Esc also loses to leader (safer). Duplicate is paint-only. | high |
| Nit | L9 | Overlay chord labels ↑/↓ use addText on .theme(.mono). Harness-limits already say Noto has no Arrows block (chip jump hint uses addTextMixed). May tofu in the cheatsheet of all places. |
Open help — arrows are .notdef. |
ASCII Ctrl/Cmd+/ rows are fine. Same class as #732. |
medium |
Residual risk
LEADER_TIMER_ID = 0x7410_0000_0000_0001vs widgetId.extendId— assumed collision-safe, not proven.- Overlay
ctx_scrollis a module-levelvar; reopen keeps the old scroll offset. - Dispatcher has no host test (only pure
keymap.zig). The Major above is exactly the seamtest-richdoes not compile. - Zig
test-rich/ wasm32build-harnesswere not re-run in this review environment.
Merge guidance
- CONCERNS: merge only with explicit accept of the Major (leader-pending unmapped keys insert). Prefer fixing
fromDvuinull +leader_armed→ swallow/disarm before merge. - Do not add a DOM
windowkeydown / React cheatsheet to paper over the overlay min-width. - Do not silently change
LEADER_WINDOW_MS/KEYMAP_MAX.
What was not attacked
Host zig build test-rich / wasm32 build-harness, live canvas (Ctrl+/ , leader+?, Esc vs menus), dvui timer clock vs rAF, Production harness artifact deploy, #742 leader+t (not in this PR).
Plan to address feedback on PR #746 (Status: DONE)Last updated: 2026-08-21 Feedback items (most complex → least)
#1 — Major L1: leader window lets unmapped keys insert + stay armed (🟢 DONE)
#2 — Minor L1: overlay no-ops on narrow canvas with rail open (🟢 DONE)
#3 — Minor L8: overlay is not the table (🟢 DONE)
#4 — Nit L9: overlay arrow glyphs may tofu (🟢 DONE)
Round 2 — mergeable-state reconciliation (single-file conflict vs #744)Not adversarial review feedback — the merge fast-fail. The 4 review items above were DONE and PASS'd at head
Progress log (append-only)
Merge decisionPASS WITH NOTES: the 4 round-1 items are done and the merge-state conflict is reconciled. Head changed ( |
…erlay fixes (review #746) Address PR #746 adversarial-review CONCERNS (1 Major + 2 Minor + 1 Nit): - Major L1: fromDvui no longer drops unrecognized non-modifier keys via `orelse continue`. New keymap.Key.unknown receives every unmapped printable (letters outside the subset, digits, punctuation, Backspace, f1-f4/f6+, nav), so keymap.match swallows+disarms it while a leader is pending (never lands in the prompt). Pure modifier/lock keys (shift/control/alt/command, menu, locks, dvui unknown) still return null -> pass through, so releasing/re-pressing Shift for the `?` command never disarms the leader. Host-tested in the pure keymap module (the seam the review said test-rich does not cover). - Minor L1: help overlay sized against the FULL band (avail.w) instead of the leftover transcript width, so an open rail on a ~390px canvas still paints (avail.w=390 >= HELP_OVERLAY_MIN_W 300). - Minor L8: overlay now iterates KEY_TABLE and presents each distinct action once (history_older + history_older_in no longer duplicate the Old-up row), help copy comes from row.help (overlay IS the table), and leader+Esc closes help first (help wins, one Esc) per docs/harness-limits L61. - Nit L9: chord labels (↑/↓ arrows) painted via addTextMixed so the DejaVu symbols face renders them (Noto/Vera mono have no Arrows block; same tofu class as #732). Zig-only: test-rich 88/88 (keymap 27->36) + invariants + harness Debug wasm + fmt clean. No bridge exports changed (no new inv_*).
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #746 (re-review @ 95d956e)
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← plan/harness-keymap · 11 files · single keymap table + leader + help overlay (#741)
Lenses run: L1, L3, L5, L6, L8, L9 (skip: L2 no secrets/API; L4 no workflow; L7 no clone bind)
AGENTS.md read: yes · docs/feature-divide.md read (Wasm owns chords; DOM adds none)
Prior findings are closed on this HEAD:
| Prior | Status @ 95d956e |
|---|---|
L1 Major fromDvui(...) orelse continue drops unmapped keys |
fixed — unmapped non-modifiers map to keymap.Key.unknown; match(.unknown) while leader pending is .swallow_leader; tests lock .unknown down/repeat/up |
| L1 Minor overlay no-op at ~390 with rail open | fixed — help_overlay.paint(0, scroll_y, avail.w, scroll_h, …) sizes against the full band (390 ≥ MIN_W 300) |
| L8 Minor overlay ≠ table / duplicate ↑ / two Esc | fixed — row.help is the copy; consecutive same-action rows collapsed; leader+Esc with help open → help_close (and runAction disarms) |
L9 Nit ↑/↓ tofu |
fixed — mixed_text.addTextMixed on chord labels |
Architecture still holds: one dvui.events() walk before textEntry, reserved deny-list never marked handled, request_submit after te.deinit. Caps unchanged. Pure modifiers still null so Shift for ? does not kill the window.
Findings
None remaining that survive self-refute.
Residual risk
fromDvuistill lives inkeymap_dispatch.zig(dvui-backed, not intest-rich). A laterelse => nullwould reopen the Major without a red host test; onlykeymap.match(.unknown)is locked.- dvui’s own
.unknownkey still returnsnull(pass through, never disarms). Exotic scancodes dvui does not name can still insert during the window. - Overlay dedup is consecutive
row.actiononly. Inserting a row betweenhistory_older/history_older_inwould paint ↑ twice again. LEADER_TIMER_ID = 0x7410_0000_0000_0001vs widgetId.extendId— assumed collision-safe.- Overlay
ctx_scrollis a module-levelvar; reopen keeps the old offset. - 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
- Do not add a DOM
windowkeydown / React cheatsheet - Do not silently change
LEADER_WINDOW_MS/KEYMAP_MAX
What was not attacked
Host zig build test-rich / wasm32 build-harness, live canvas (Ctrl+/ , leader+?/h/1, Esc vs menus), dvui timer clock vs rAF, Production harness artifact deploy, #742 leader+t (not in this PR).
# Conflicts: # native/harness/src/ui.zig
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #746 (re-review @ 160c8d3, merge of main)
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← plan/harness-keymap · 11 files (+ merge of #744 chrome + later main) · keymap + leader + help overlay (#741)
Lenses run: L1, L3, L4, L5, L6, L8, L9 (skip: L2 no secrets/API; L7 no clone bind)
AGENTS.md read: yes · docs/feature-divide.md read (Wasm owns chords; DOM adds none)
This HEAD is 95d956e (prior PASS WITH NOTES) merged with origin/main (ff9a1e8, #744 composer chrome extraction). Attack focused on the ui.zig conflict resolution.
Merge wiring is the right shape:
keymap_dispatch.dispatchstill runs beforecomposer_chrome.paintComposerChrome(the extracted textEntry).request_submitis consumed after chrome returnsres.typed.- No leftover
for (dvui.events())inui.zig/queue_band/composer_chrome. #734reservation (field_w, wrap min=max) is intact incomposer_chrome.zig;composer_layoutandkeymapboth still register intest-rich.- Prior CONCERNS stay closed:
Key.unknownswallow, overlayavail.w, help-wins Esc,addTextMixedarrows. - Docs still have both the Keyboard & focus table and the width-vs-height pad / reserved-strip rows.
Findings
None remaining that survive self-refute.
Candidate “dispatch is after queue_band.paint” does not survive: the queue editor is multiline = false, and the removed submitChord already ran after that textEntry. Composer (the widget that inserts \n on Enter, modifier ignored) is still built after dispatch.
Residual risk
fromDvuistill lives inkeymap_dispatch.zig(not intest-rich). A laterelse => nullreopens the leader-swallow Major without a red host test.- dvui’s own
.unknownkey still returnsnull(pass through during the leader window). - Overlay dedup is consecutive
row.actiononly. LEADER_TIMER_IDvs widgetId.extendId— assumed collision-safe.- Overlay
ctx_scrollpersists across close/reopen. - Queue Ctrl+Enter save still depends on the single-line textEntry not marking the chord handled before dispatch (same order as the old inline scan).
- Zig
test-rich/ wasm32build-harnesswere not re-run in this review environment (merge is the kind of change that can fail those).
Merge guidance
- PASS WITH NOTES: safe to merge from this attack; nits optional
- Do not add a DOM
windowkeydown / React cheatsheet - Do not silently change
LEADER_WINDOW_MS/KEYMAP_MAX - Do not re-inline the composer textEntry in
ui.zig(breaks #744 and this dispatch-before-TE contract)
What was not attacked
Host zig build test-rich / wasm32 build-harness on the merge commit, live canvas (Ctrl+/ , leader+h, queue Ctrl+Enter save), dvui timer clock vs rAF, Production harness artifact deploy, #742 leader+t.
Summary
Implements plan #741 — harness keyboard shortcuts (keymap, leader, help overlay). Ships a single keymap table in Wasm, a browser-safe reserved-browser deny-list, a Ctrl+Shift+Space leader with an in-canvas help overlay, and migrates all existing chords (Enter send/enqueue, ↑↓ history, Escape busy-cancel / queue-edit, Ctrl/Cmd+Enter queue-save) onto the table.
Fixes #741
What changed (all harness + docs; no DOM, no TS, no backend)
native/harness/src/keymap.zig(new) — the single chord table (ACTIONrows with chord +whencontext), the reserved-browser deny-list (fail-closed: a reserved chord is never marked handled, even if a row overlaps), thematch()pure dispatcher, and the leader machine. +26 unit tests covering every shipped row, reserved deny, and leader arm/swallow/timeout.native/harness/src/ui/keymap_dispatch.zig(new) — the one per-frame walk ofdvui.events()(before the composer textEntry is built). Marks handled, runs product seams via aHandlers{history}struct (avoids an import cycle withui.zig), and drives the leader viadvui.timerexpiry (no host I/O, no new exports).native/harness/src/ui/help_overlay.zig(new) — TEAL in-canvas panel over the transcript band listing the live keymap; context-off rows grey (WARM-muted, not EMBER). No GPA/alloc, no host I/O.native/harness/src/ui.zig— the inline composer event loop (composer_submit, busy-Escape, ↑↓ history) is replaced by the singlekeymap_dispatch.dispatch(...)call; submit is requested viastate.request_submitand consumed aftertextEntry.deinit; help overlay painted over the transcript band; ring-clear (New/Clear/hydrate) closes the overlay + disarms the leader.native/harness/src/ui/queue_band.zig— deletedhandleEscapeandsubmitChord(both inlinefor (dvui.events())scans);queue_save/cancel_queue_editnow route through the dispatcher via a new pubcancelEditFromUi+ existingsaveEdit. Queue paint reads no keys.native/harness/src/ui/state.zig— new flagshelp_overlay_open,leader_armed,request_submit(+ reset on clear/hydrate).native/harness/src/ui/metrics.zig—HELP_OVERLAY_*(NEW cap constants; helpful, generous, no wire).native/harness/build.zig— registerskeymapintest-rich.docs/harness-limits.md(Keyboard & focus → live keymap table),docs/feature-divide.md+AGENTS.md(keymap + dispatcher + help overlay are harness-owned; DOM adds no shortcut UI).Caps table
KEYMAP_MAXnative/harness/src/keymap.zigLEADER_WINDOW_MS?/a letter, short enough not to trap typingkeymap.zigHELP_OVERLAY_*ui/metrics.zigNo existing cap is raised or lowered. No Function/Blob/Redis wire.
Verification
zig build test-rich— green (all host unit tests incl. newkeymapsuite)zig build test-rich-invariants— greenzig build harness -Doptimize=Debug— green (full wasm32-freestanding compile)zig fmt --checkon all touched .zig — cleanfor (dvui.events())key loop outside the dispatcher (verified via grep);ui.zig+queue_band.handleEscape+queue_band.submitChordinline scans all gone.build-harnessCI (release wasm32-freestanding on the self-hosted runner) will be the release gate — watching on this PR. Local Debug compile already proves the Wasm compiles.Notes for review
composercontext is proxied as!queue_editing(dvui focus is frame-global; the composer is the only text input). Matches the old inline gating exactly.