feat(config): remappable thread, submit, result, and cleanup modal keys - #51
Merged
Conversation
The submit-review modal rendered one unscrollable Paragraph in a 72%x72% box, so a review batch taller than the modal was clipped with no scroll and no indicator — in the one surface whose job is confirming exactly what will be sent. The body now scrolls: Up/Down by a line, PageUp/PageDown by a real viewport, added to SUBMIT_FORGE_KEYS so the help overlay and footer stay in sync. Printable characters still fall through to the summary field, which is why the scroll keys are the arrow/page keys and not j/k. The offset is a Cell clamped to the content at render time (the help overlay's model) and reset on every fresh open, and when content is clipped the modal spends one row top and bottom on a marker naming how many lines are hidden in that direction. A blocked request-changes confirm jumps to the bottom so its hint can't land off-screen. Lines are pre-wrapped rather than handed to Paragraph's Wrap, so the count the scroll math clamps against is the row count the terminal really shows — otherwise a long batch's last rows stay unreachable and the hidden-line count understates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWX3hBxudUxZooeph9ngEB
Replies in the submit-review modal previewed as `thread <id>: <body>` — a raw numeric id is meaningless at confirm time. Resolve each reply's target from the fetched thread overlay instead: root author and anchor (`path:line`, or `path (file-level)` once outdated), same conventions the thread overlay itself uses. Falls back to the id form only when the thread has dropped out of the overlay (e.g. a failed refresh). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWX3hBxudUxZooeph9ngEB
The submit modal's summary was a single-line push/pop field, so the substantive body of a forge review was capped at one line with no cursor motion or word-delete. `Ctrl-e` now hands it to the Compose editor, seeded with the text so far; saving returns to the modal with the whole body, cancelling leaves it as it was. The field shows the summary's first line plus a dim count of the lines it keeps off screen, and once the summary is multi-line the in-modal push/pop gestures step aside for `Ctrl-e` rather than silently mutate a line the reviewer can't see. Direct typing of a one-line summary is unchanged, as is Enter to confirm. Compose's two-mode `thread_id: Option<u64>` discriminant becomes a three-variant `ComposeKind`, so no combination of flags can describe a compose that is an annotation and a summary at once. The change is folded in here rather than split out because the third variant is the feature. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWX3hBxudUxZooeph9ngEB
A stopped submit left everything in one transient status line: honest about the counts, silent about which comments actually landed, so the only way to find out was to open the PR in a browser. The sequence now records what it set out to send (`SubmitAttempt`) into its report before it starts writing. Joined against the published and draft lists, that makes each item's fate knowable — published, pending draft, or never reached — rather than inferred from a diff of local state. A run that stops opens a read-only modal grouping every item under its outcome, named exactly as the submit preview named it (including the humanized "to <author> @ <path>:<line>" reply labels), with the review itself leading the not-sent group when the verdict never landed and the diagnostic underneath. The one-line status is unchanged and still set in both cases; a submit that publishes everything opens no modal. Keys come from a new SUBMIT_RESULT_KEYS table: Enter/Esc/q dismiss, U reopens the submit modal to retry the remainder, and j/k/arrows plus the page keys scroll (no summary field here, so the letter keys are free, unlike the submit modal). Long lists scroll with the render-time clamp the help overlay uses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWX3hBxudUxZooeph9ngEB
Adds a cursor and per-entry checkbox to the finished-review cleanup modal (X on the Pull Requests tab): j/k/arrows move the highlight, Space toggles the highlighted entry (all checked by default), and confirm deletes only the selected subset. Zero-selected Enter is a no-op; deselected entries keep their unpublished-work warnings visible since the warning belongs to the entry, not to the deletion. New CleanupReviewsAction rows (MoveDown/MoveUp/Toggle) go through the shared CLEANUP_REVIEWS_KEYS table so the footer strip and ? help stay in sync automatically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWX3hBxudUxZooeph9ngEB
The Review launcher already honored `[keys.review-launcher]`, but the four PR-flow modals were pinned to their compiled-in tables, against the repo's "keymap is data" convention. Each gains a bijective action-name pair and routes through the same `apply_modal_overrides` merge, so `[keys.thread-view]`, `[keys.submit-forge]`, `[keys.submit-result]`, and `[keys.cleanup-reviews]` get identical grammar, replace/unbind/collision semantics, and warning surface. The `?` overlay and footer strip already read the effective tables, so remaps reach both with no extra wiring. Submit-forge is free-text: its table is consulted before the char-insert fallback, so binding a control action to a bare printable key takes that character away from summary typing. Allowed rather than rejected — the same trade every other free-text mode here already accepts — and documented in the example config instead of encoded as a special-case validation rule. The example config's doc-drift test now reads each mode's expected action set off its default table rather than a hand-kept parallel list; that list had gone stale in step with the doc it polices, which is how the launcher's `toggle-all-commits` and `cleanup-finished-reviews` went undocumented. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWX3hBxudUxZooeph9ngEB
Carries both chains: keeps this branch's remappable-table wiring and doc comments, re-adds main's pr_description mode (field + static clone), and documents the launcher's refresh and pr-details keys in the example config — the self-maintaining drift test demanded them once the picker chain's actions landed alongside this branch's test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWX3hBxudUxZooeph9ngEB
Keeps this branch's remappable doc comments for the cleanup table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWX3hBxudUxZooeph9ngEB
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sixth and final PR of the submit/modals chain (stacked on #50). The launcher honored
[keys.review-launcher]remapping while the thread-view, submit, submit-result, and cleanup modals were hardcoded — a gap in the "keymap is data" convention.Changes
[keys.thread-view],[keys.submit-forge],[keys.submit-result],[keys.cleanup-reviews]now flow through the exact same config path as the launcher: same grammar, replace/unbind/collision semantics,InvalidValuewarning surface, and kebab-case naming. No config → behavior identical to today (pinned by tests).?help reflect remapped keys automatically (verified end-to-end config → footer).docs/example-config.tomlwith advice to preferctrl-/named keys — and pinned by a test so the resolve-before-fallthrough ordering can't silently flip.docs/example-config.toml: four new fully-annotated blocks; launcher block gains its missingtoggle-all-commits/cleanup-finished-reviewsentries. The doc-drift test now derives each block's expected action set from the mode's default table instead of a hand-kept list — the hand-list going stale is exactly how the launcher entries went missing. (Heads-up: when the picker chain'srefreshaction merges alongside this, that self-maintaining test will fail untilrefreshis documented — a deliberate forcing function.)Tests
7 added + 4 extended: per-section remap round-trips (key drives action, displaced default dead, label moves), table-driven invalid-value parity, config→footer end-to-end, the printable-shadow ordering pin, and the self-maintaining doc-drift coverage (mutation-verified).
Tracking: ENG-178 (Linear). All four gates pass.
🤖 Generated with Claude Code
https://claude.ai/code/session_01AWX3hBxudUxZooeph9ngEB