One table for the dashboard's keys, and an issue side that can answer #99

Merged

a73x merged 1 commit from worktree-agent-a5f9477bbb514c130 into main

2026-08-14 06:22:20 +00:00

a73x commented

2026-08-14 06:17:47 +00:00

(Migrated from git.a73x.sh)

Replaces 57 KeyCode:: arms across src/tui/state.rs and src/tui/events.rs with one binding table in src/tui/keys.rs: context -> key -> action, dispatch by lookup, footer and a new ? overlay generated from it.

The table

Context is the (ViewMode, Pane) pair that already governed dispatch, with ListMode folded in where it genuinely changes what a key means — nine contexts: IssueList, IssueDetail, PatchList, PatchSummary, PatchDetail, EventHistory, EventDetail, ReviewVerdict, Help. Each entry is keys (aliases: j/Down), action, label, whether the footer advertises it, and an optional partner so j/k and [/] print as one item with the key names taken from the table rather than typed into a string. Labels that depend on state (a:show all vs a:closed, C:close vs C:reopen) are fn(&App) -> &'static str.

A key the context does not bind returns None and the reader is told: z does nothing on Issues — list — press ? for this pane's keys. That is the whole difference from the old structure, where "not bound" and "bound but gated out by pane == Pane::Detail" both reached the same silent _ => KeyAction::Continue.

Invariant tests, both shown to bite

no_context_binds_a_key_twice — planted c -> OpenEventHistory beside c -> CommentOnIssue, watched it fail with IssueList binds c twice: CommentOnIssue and OpenEventHistory (the live bug, in the test's own words), removed it. The generated-footer test caught the same plant independently.

every_context_is_reachable — made Context::surface never return PatchSummary, watched it fail with no state produces PatchSummary, so its bindings are unreachable, restored it.

Also: every context has a way out, every pair has its partner, every binding is labelled, Context::ALL matches an exhaustive ordinal match.

`d7158619`

c comments on the issue under the cursor, from either issue pane, through the same $EDITOR suspend the patch side uses and against issue::comment — the same call git-collab issue comment makes.

Event History moves to e. Comment has the stronger claim on c because the dashboard is a review surface by decision; e was unbound in every context (only Ctrl-E was taken, different context and different modifier) and matches the pane's own name.

ViewMode::CommitList/CommitDetail and KeyAction::OpenCommitBrowser renamed to EventHistory/EventDetail/OpenEventHistory.

Triage decision: close/reopen yes, labels no

C closes with a reason composed in $EDITOR (empty buffer abandons it — the DAG is append-only), and reopens a closed issue. Closing is the terminal move of the review loop: you read, you comment, and the conclusion is "done" or "wrong". Withholding it means quitting the dashboard to run one command, which is the complaint that produced this issue.

Labels are different in kind. A label is free text from an open set, so its input is a completion problem — the dashboard has no completion and no list of existing labels on screen. Hand-typing them from a modal line is how a repository ends up with bug, Bug and bgu. Labels are organisation, not review, and the CLI and web UI have the surface for it.

Found on the way

The a filter offered "closed" and "all" for issues it could never show: closing archives the ref and the dashboard loaded with state::list_issues, which excludes archived — so two of three filter settings were structurally empty, and C-reopen would have been unreachable. issue list -a and the web UI have always used the _with_archived listers. The dashboard now does too; visible_issues does the narrowing, so the default view is unchanged.

Two of the new pty tests waited on needles a cell-diffing redraw can never emit ((all) — the ( does not change when the word after it does, so it is never re-emitted) and burned the 20s pty timeout in silence while still passing. Both now wait on text drawn into blank cells. tui_review_test.rs went 20.7s -> 3.1s; no pre-existing test was doing this.

Verification

Full suite green (54 binaries). cargo clippy --all-targets silent. tui_review_test run 4x after the timeout fix: 23/23 each, 3.13/3.17/3.17/3.13s.

Not covered: nobody has driven this in a human terminal. Everything here is asserted through TestBackend or the pty harness. In particular the ? overlay's placement is only checked as text-on-screen, not as a layout a person would call readable; the popup is sized from the widest key and widest label and clipped to the pane, and at very small terminal sizes it will clip rather than reflow. Shift handling assumes the terminal folds Shift into the character (true for the crossterm defaults here, not verified under the kitty keyboard protocol). Key release events are still not filtered, so a terminal that reports them would now produce an "does nothing" message per release — unchanged in kind from before, but newly visible.

Fixes 02d3eb34. Also fixes d7158619.

Migrated from git-collab patche 3ba51232757b5c5f6dc21e587d6f7f4d7376bbc7.

Original author: a73x dev@a73x.sh.

Fixes #93

Revision 1: 618f79e2eef0f6c940c39b87060d73f5f64bcd02; base 969a798355aeb5b0569464ab0659cfbabeb924ba

Revision 2: f3d6c51a310971e38d750cc70c5cae3cfbb1bfdd; base ae48e4fea7a079a1424d7dc6d2c5da88e024f21b

Rebased onto main@ae48e4f3 (which dropped tests/server_migrate_test.rs). No conflicts, no code changes. Re-verified: full suite green (53 binaries, 0 failures), clippy --all-targets silent, tui_review_test 3x at 23/23 and 3.17s each.

Replaces 57 `KeyCode::` arms across `src/tui/state.rs` and `src/tui/events.rs` with one binding table in `src/tui/keys.rs`: context -> key -> action, dispatch by lookup, footer and a new `?` overlay generated from it. ## The table Context is the (`ViewMode`, `Pane`) pair that already governed dispatch, with `ListMode` folded in where it genuinely changes what a key means — nine contexts: IssueList, IssueDetail, PatchList, PatchSummary, PatchDetail, EventHistory, EventDetail, ReviewVerdict, Help. Each entry is keys (aliases: `j`/Down), action, label, whether the footer advertises it, and an optional partner so `j/k` and `[/]` print as one item with the key names taken from the table rather than typed into a string. Labels that depend on state (`a:show all` vs `a:closed`, `C:close` vs `C:reopen`) are `fn(&App) -> &'static str`. A key the context does not bind returns `None` and the reader is told: `z does nothing on Issues — list — press ? for this pane's keys.` That is the whole difference from the old structure, where "not bound" and "bound but gated out by `pane == Pane::Detail`" both reached the same silent `_ => KeyAction::Continue`. ## Invariant tests, both shown to bite `no_context_binds_a_key_twice` — planted `c -> OpenEventHistory` beside `c -> CommentOnIssue`, watched it fail with `IssueList binds c twice: CommentOnIssue and OpenEventHistory` (the live bug, in the test's own words), removed it. The generated-footer test caught the same plant independently. `every_context_is_reachable` — made `Context::surface` never return `PatchSummary`, watched it fail with `no state produces PatchSummary, so its bindings are unreachable`, restored it. Also: every context has a way out, every pair has its partner, every binding is labelled, `Context::ALL` matches an exhaustive ordinal match. ## d7158619 `c` comments on the issue under the cursor, from **either** issue pane, through the same `$EDITOR` suspend the patch side uses and against `issue::comment` — the same call `git-collab issue comment` makes. Event History moves to `e`. Comment has the stronger claim on `c` because the dashboard is a review surface by decision; `e` was unbound in every context (only `Ctrl-E` was taken, different context and different modifier) and matches the pane's own name. `ViewMode::CommitList`/`CommitDetail` and `KeyAction::OpenCommitBrowser` renamed to `EventHistory`/`EventDetail`/`OpenEventHistory`. ## Triage decision: close/reopen yes, labels no `C` closes with a reason composed in `$EDITOR` (empty buffer abandons it — the DAG is append-only), and reopens a closed issue. Closing is the terminal move of the review loop: you read, you comment, and the conclusion is "done" or "wrong". Withholding it means quitting the dashboard to run one command, which is the complaint that produced this issue. Labels are different in kind. A label is free text from an open set, so its input is a completion problem — the dashboard has no completion and no list of existing labels on screen. Hand-typing them from a modal line is how a repository ends up with `bug`, `Bug` and `bgu`. Labels are organisation, not review, and the CLI and web UI have the surface for it. ## Found on the way The `a` filter offered "closed" and "all" for issues it could never show: closing archives the ref and the dashboard loaded with `state::list_issues`, which excludes archived — so two of three filter settings were structurally empty, and `C`-reopen would have been unreachable. `issue list -a` and the web UI have always used the `_with_archived` listers. The dashboard now does too; `visible_issues` does the narrowing, so the default view is unchanged. Two of the new pty tests waited on needles a cell-diffing redraw can never emit (`(all)` — the `(` does not change when the word after it does, so it is never re-emitted) and burned the 20s pty timeout in silence while still passing. Both now wait on text drawn into blank cells. `tui_review_test.rs` went 20.7s -> 3.1s; no pre-existing test was doing this. ## Verification Full suite green (54 binaries). `cargo clippy --all-targets` silent. `tui_review_test` run 4x after the timeout fix: 23/23 each, 3.13/3.17/3.17/3.13s. **Not covered:** nobody has driven this in a human terminal. Everything here is asserted through TestBackend or the pty harness. In particular the `?` overlay's *placement* is only checked as text-on-screen, not as a layout a person would call readable; the popup is sized from the widest key and widest label and clipped to the pane, and at very small terminal sizes it will clip rather than reflow. Shift handling assumes the terminal folds Shift into the character (true for the crossterm defaults here, not verified under the kitty keyboard protocol). Key *release* events are still not filtered, so a terminal that reports them would now produce an "does nothing" message per release — unchanged in kind from before, but newly visible. Fixes 02d3eb34. Also fixes d7158619. --- Migrated from git-collab patche `3ba51232757b5c5f6dc21e587d6f7f4d7376bbc7`. Original author: a73x <dev@a73x.sh>. Fixes #93 Revision 1: `618f79e2eef0f6c940c39b87060d73f5f64bcd02`; base `969a798355aeb5b0569464ab0659cfbabeb924ba` Revision 2: `f3d6c51a310971e38d750cc70c5cae3cfbb1bfdd`; base `ae48e4fea7a079a1424d7dc6d2c5da88e024f21b` Rebased onto main@ae48e4f3 (which dropped tests/server_migrate_test.rs). No conflicts, no code changes. Re-verified: full suite green (53 binaries, 0 failures), clippy --all-targets silent, tui_review_test 3x at 23/23 and 3.17s each.