feat(tui): hotbar Alt+1-8 discoverability + decision-card key disambiguation

[Hmbown]

Why this exists

The hotbar Alt+1-8 discoverability work shipped in the 0.8.66 local test candidate (the binary built for testing) and #3731 explicitly describes it as the "v0.8.66 local candidate" state — but it never had its own PR, so when the 7 fix PRs + #3756 merged, this slice was left behind and is not on main. This PR lands it so the release matches what was tested.

What's here

• Sidebar hotbar panel exposes the chord: title Hotbar (Alt+1-8), cells Alt1:label (not bare digits that look like raw number keys), hover Slot N (Alt+N): ....
• Empty config (hotbar = []) omits the panel and reclaims sidebar height.
• /hotbar help explains Alt+1–Alt+8 and hotbar = [].
• Number-key disambiguation: bare digits drive decision-card selection (modifiers rejected); Alt+digit hotbar dispatch is suppressed while a decision card is open.
• KEYBINDINGS.md documents Alt-1..8 (Option+number on macOS) and why F-keys/Cmd-number aren't the primary chords; QA matrix covers the decision-card state.

Validation

Built + tested against current main (f6a19c25d): cargo build -p codewhale-tui ✓; hotbar tests 63 passed; decision_card tests 2 passed. All 6 files were untouched by the 0.8.66 merges, so this is a clean addition.

Scope decision (your call)

My read is this belongs in 0.8.66 (it's in the tested candidate). If you'd rather ship the basic discoverability in 0.8.67 with the rest of #3731 (customization, terminal QA), just say so and I'll re-milestone/hold. This PR does not close #3731 — it's the discoverability slice; the lane stays open for 0.8.67.

[claude]

Claude finished @Hmbown's task in 3m 27s —— View job

---

Code Review

• Gather context and read changed files
• Review hotbar.rs changes
• Review sidebar.rs changes
• Review ui.rs changes
• Review tests.rs changes
• Review docs changes
• Post findings

---

Summary: Clean, focused slice. The logic is correct, the guard placement is right, and the extraction refactors improve testability. Two minor observations below — neither is blocking.

---

Priority 1 — Correctness

decision_card_number_from_key guard is correct and in the right branch.
The decision card's number handler runs in an early continue-guarded branch (line ~3578–3617), so it never reaches the hotbar dispatch path. The decision_card.is_some() guard in hotbar_slot_from_key (line ~5118) closes the reverse — Alt+digit during an active card doesn't fire the hotbar. These two guards are independent and non-overlapping. ✓

Modifier check on decision card numbers is sound.
!key.modifiers.is_empty() rejects Alt+digit, Ctrl+digit, and any modifier combo, leaving only unmodified bare digits for the card. Matches the stated contract ("bare digits only"). ✓

Arithmetic in decision_card_number_from_key is correct.
(c as u8 - b'1' + 1) as usize for '1'..='9' produces 1..=9, matching select_number's 1-based contract (select_number does n - 1 internally to get the zero-indexed slot). ✓ (The formula is equivalent to c as u8 - b'0' — minor readability note, not a bug, and it was carried from the original code.)

---

Priority 3 — Reuse / simplification

resolve_hotbar_bindings is called twice per render when hotbar is enabled.
hotbar_panel_enabled (line 261) calls resolved_hotbar_bindings to check emptiness. If the panel is enabled, hotbar_panel_slots (line 266) calls it again. For a TUI render cycle this is harmless, but the resolution work is duplicated. A straightforward fix would be to compute once and pass &[HotbarBinding] into both:

// in render_sidebar / render_hotbar_panel:
let bindings = resolved_hotbar_bindings(app, config);
if !bindings.is_empty() { ... render with &bindings ... }

Not blocking — TUI render latency is not a concern at this scale.

---

Priority 4 — Tests

Narrow-width cell rendering is no longer explicitly exercised.
The hotbar_panel_lines_keep_two_fixed_rows_and_hover_status test moved from content_width = 20 to content_width = 32. At the minimum sidebar width (24 px, minus 4 for borders/padding → 20 content chars), each of the 4 cells is only 4 chars wide. "Alt1:" is 5 chars, so at width 4 the label is completely clipped and the "*" active marker is never visible — cells reduce to bare "Alt1", "Alt2", etc.

The old test at width 20 caught this case with text[0].contains("1:vo") and text[0].contains("4:*a"). The new assertion text[0].contains("Alt4:*") requires 6 visible chars per cell, which needs content_width ≥ 27. The minimum-width fallback (bare accelerator, no marker, no label) is currently untested. Fix this →

New tests are well-targeted:

• sidebar_hotbar_render_smoke_omits_panel_when_empty_config — smoke-tests the layout guard for hotbar = []. ✓
• hotbar_alt_digit_is_blocked_while_decision_card_is_active — directly verifies the new guard. ✓
• decision_card_numeric_shortcuts_accept_bare_digits_only — covers Alt/Ctrl rejection. ✓
• Renaming the old omnibus test and splitting it into "absent config" vs "explicit config" variants improves isolation. ✓

---

Notable good choices

• Extracting resolved_hotbar_bindings as a shared helper eliminates the prior duplicate known_action_ids construction in hotbar_panel_slots. Clean.
• Extracting decision_card_number_from_key as a pure function makes it trivially unit-testable, and the test covers both the pass case and the modifier-rejection cases. Good refactor pattern.
• hotbar_panel_enabled short-circuits split_sidebar_hotbar_area entirely for empty config — sidebar height reclaim happens at the layout level, not just in the render guard.
• Panel title "Hotbar (Alt+1-8)" vs cell text "Alt1:" (no plus) vs hover "Alt+N" is intentional width compression. The inconsistency is fine given space constraints.

---

Verdict: Ready to merge. The narrow-width test gap is a mild coverage note, not a correctness issue — behavior at minimum width is still correct (cells clip gracefully to bare accelerators), it's just not asserted.

[claude]

Claude finished @Hmbown's task in 2m 23s —— View job

---

Code Review (re-sync)

• Gather context and read changed files
• Review the PR diff
• Verify correctness, tests, and documentation
• Post findings

---

Summary: Same feature slice as the previous review, now merged with current main. All correctness findings from that review still hold. Two new observations below, one of which is new; the two mild gaps from the prior review remain unchanged.

---

Priority 1 — Correctness ✓

Guard composition is correct.
hotbar_panel_enabled(app, config) && !is_hotbar_disabled(config) — both checks are sound. hotbar_panel_enabled catches the "resolved bindings are empty" path (including hotbar = []); is_hotbar_disabled is the pre-existing semantic guard for the explicit disabled state. They don't contradict each other.

decision_card_number_from_key extract is clean.
The refactor moves a KeyCode::Char('1'..='9') match into a pure function, the continue after the card block is untouched, and the || app.decision_card.is_some() guard in hotbar_slot_from_key closes the reverse direction. No execution-order change.

---

Priority 3 — Reuse / simplification (new observation)

&& !is_hotbar_disabled(config) is redundant at the layout level.
hotbar_panel_enabled calls resolved_hotbar_bindings, which returns empty whenever hotbar = []. So is_hotbar_disabled can never flip the result to true when hotbar_panel_enabled already returned false. The only case is_hotbar_disabled would matter is when bindings are non-empty but the explicit-disable flag is set — which can't happen because empty-array config produces zero bindings.

Net: the layout line is strictly controlled by hotbar_panel_enabled alone. Not a bug, just a dead branch that adds noise. Could simplify to:

let (main_area, hotbar_area) = split_sidebar_hotbar_area(area, hotbar_panel_enabled(app, config));

resolved_hotbar_bindings is called twice per render (first in hotbar_panel_enabled, then in hotbar_panel_slots). Noted in the prior review; still present. Harmless at TUI scale, minor cleanup opportunity.

---

Priority 4 — Tests

Narrow-width coverage gap is unchanged. Test bumped from content_width = 20 to content_width = 32; the minimum-width clipping path (4-char cells → bare "Alt1", no label, no marker) is still unasserted. Behavior is correct, just untested. Add a test →

New tests are well-targeted:

• decision_card_numeric_shortcuts_accept_bare_digits_only — covers modifier rejection (None, Alt, Ctrl). ✓
• hotbar_alt_digit_is_blocked_while_decision_card_is_active — directly exercises the new guard. ✓
• sidebar_hotbar_render_smoke_omits_panel_when_empty_config — smoke-tests the layout guard for hotbar = []. ✓
• Split of the old omnibus slot test into "absent config" and "explicit config" variants improves isolation. ✓

---

Docs ✓

KEYBINDINGS.md — note about why F-keys/Cmd-number aren't primary chords is clear and accurate. The update from "not yet user-configurable" to calling out [[hotbar]] as already configurable is correct.

hotbar-qa-matrix.md — decision-card row added to the manual QA checklist and test reference updated to name the new test. Accurate.

---

Verdict: Ready to merge. The redundant is_hotbar_disabled guard and double resolved_hotbar_bindings call are cosmetic; the narrow-width test gap is a mild coverage note with no correctness impact. All 6 files are clean additions against current main.

[Hmbown]

Closing in favor of #3788, which landed first and solves the Hotbar chord discoverability better.

When I opened this, main lacked any chord display, so I ported the 0.8.66 test-candidate slice. But #3788 ("make Hotbar introduced, configurable, and easy to disable") already:

• puts ⌥+1-8 / alt+1-8 in the panel title (platform-aware via key_hint::alt_prefix()), and
• deliberately avoided a per-cell prefix because at minimum sidebar width the 4-column cells are only ~4 chars wide and a prefix clips the labels.

That's the same narrow-width clipping risk this PR's per-cell Alt1: approach would have reintroduced — so #3788's design supersedes this one, and merging this would clobber #3788's sidebar/ui changes. Right call to take #3788.

One small, genuinely-separate item this branch also carried that main still lacks: the decision-card vs hotbar number-key disambiguation (decision_card_number_from_key + suppressing Alt+digit while a decision card is open). It's minor (the decision-card path already returns before hotbar dispatch, so it's belt-and-suspenders) — happy to send it as a tiny standalone PR if you want it; otherwise it can ride the 0.8.67 hotbar lane (#3731).