feat(tui): collapsible groups in the tasks pane - #527
Conversation
The pane grew a second group when background tasks landed in it. With a few agents and a few jobs it fills the strip, and the half you are not watching crowds out the half you are. Space folds the selected group. A folded heading shows the count, so collapsing never hides how much is behind it. Two things that would otherwise make this feel broken: The pane is sized from the collapsed row count, so folding actually frees space. Sizing from the full list would leave the strip the same height and buy nothing. The selection skips folded rows, and collapsing the group that holds the selection moves it to the first row still visible. Otherwise the selection sits on something off-screen and the pane looks unresponsive.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ede3a5d791
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.collapsed_groups.push(source); | ||
| let visible = super::tasks::visible_indices(&self.tasks, &self.collapsed_groups); | ||
| self.tasks_selected = visible.first().copied().unwrap_or(0); |
There was a problem hiding this comment.
When two source groups are present, collapsing the selected group immediately moves tasks_selected into the other group, while tasks_select only traverses visible task rows. The collapsed heading can therefore never be selected again, so pressing Space cannot unfold that group without first collapsing every remaining visible group and relying on the hidden fallback index. Keep group headings in the selection model, or otherwise retain an explicit selection for the collapsed heading.
Useful? React with 👍 / 👎.
| if group_folded { | ||
| continue; |
There was a problem hiding this comment.
On a narrow terminal where the pane overflows, folding a source that precedes the selected source removes its tasks from task_ends, but tasks_selected remains an absolute index into app.tasks. The overflow code later calls task_ends.get(app.tasks_selected) and falls back to row zero, so moving down through the remaining group can scroll the selected marker off-screen instead of windowing around it. Store rendered row ends by original task index or translate the selection to its visible ordinal.
Useful? React with 👍 / 👎.
Folding a group moved the selection into the other group, and Up/Down only walked unfolded rows, so the folded group's heading could never be reached again to unfold it. A collapsed group now keeps exactly one selectable row — its heading — the marker renders there, and Enter on it unfolds instead of opening a hidden row's output. The overflow window keyed rendered-row ends by absolute task index, so a folded group ahead of the selection shifted the lookup and scrolled the selected task off screen. Rows are anchored to their original task index now, and a visible folded heading accounts for the rows behind it rather than double-counting them in "+n more".
|
@codex review — both P2 findings on ede3a5d are fixed in b6efd9a. Keep collapsed group headings selectable (app.rs): a collapsed group now contributes exactly one selectable row — its first task index, which the heading stands in for ( Preserve task indices when omitting folded rows (render.rs): rendered-row ends are no longer a dense vec indexed by absolute task index. Each row is a Regression tests, each verified to fail against the previous behaviour: Full gate green: |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6efd9a8ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // it accounts for every task behind it. | ||
| anchors.push(RowAnchor { | ||
| task: idx, | ||
| end: lines.len() - 1, |
There was a problem hiding this comment.
When the pane has only one content row plus the overflow indicator (for example, a narrow 14-row terminal) and the selected folded group is not the first group, this anchor points at the heading itself, but the windowing below computes sel_start = sel_end - 1 under the assumption that every anchor ends on a task headline. The resulting window shows the preceding blank/task line instead of the selected heading, so the ❯ marker disappears and the user must unfold the group blindly. Record the anchor's actual start or distinguish single-line folded headings when selecting the window offset.
Useful? React with 👍 / 👎.
A folded group heading is a single line, but the overflow window derived the marker row as `end - 1` — the shape of a two-line status+headline task row. When the selected folded group was not the first one, that stepped onto the blank separator above the heading, so on a short pane the selected group scrolled off screen and had to be unfolded blind. Each row now records the line its marker sits on rather than deriving it, so short-pane windowing keeps a selected folded heading visible at every pane height.
|
@codex review — P2 "Keep later folded headings visible in short panes" fixed in 2fea1c3. Confirmed the bug first: with 12 agent rows expanded and a folded Root cause was the assumption that every anchor is a two-line status+headline pair, so the marker row could be derived as
Regression tests: Full gate green: |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
… collapse work #519 (model checklist) and #520 (palette routing) landed on the same pane. Four conflict sites, resolved as: - app.rs: additive on both sides — `collapsed_groups` alongside `todos` and `conversation_epoch`. - pane sizing: `pane_rows_with_todos` now also takes the collapsed groups, so one computation accounts for both the checklist and folded rows. Keeps main's saturating `u16::try_from` — a model-authored checklist has no length limit and `as u16` would wrap a huge plan to a one-row request. - group heading: keeps the fold marker, the `(n)` count and the selection caret, with main's `palette().muted` instead of the hardcoded colour #520 removed. - overflow accounting: keeps main's footer-earns-its-row logic and reads the row anchors, so a surviving folded heading still accounts for its whole group instead of double-reporting those rows in "+n more". Checklist rows keep their own separate count.
|
@codex review — merged main (843651a). #519 (model checklist) and #520 (palette routing) landed on this same pane; four conflict sites, all in the tasks pane:
New test for the interaction: Snapshot invariants verified after the merge: Full gate green: One unrelated note: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 843651aac6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // not a space the user is trying to type. | ||
| (m, KeyCode::Char(' ')) | ||
| if m.is_empty() | ||
| && app.tasks_visible() |
There was a problem hiding this comment.
When a TodoUpdate makes the pane visible without creating any task rows, this guard still captures Space, but toggle_selected_group() immediately returns because there is no selected group. The keystroke is therefore silently discarded instead of reaching insert_char, unlike the arrow and Enter bindings that deliberately use tasks_nav_active() to avoid swallowing keys in checklist-only panes; use the same predicate here.
Useful? React with 👍 / 👎.
A `TodoUpdate` makes the tasks pane visible without creating any task rows. The fold binding still claimed Space there, and `toggle_selected_group` returned immediately with no group selected, so the keystroke was dropped instead of reaching the composer. Gate it on `tasks_nav_active` like the arrow and Enter bindings already do. Also restores the Enter binding's comment, which the fold arm had been inserted into the middle of.
|
@codex review — P2 "Let checklist-only panes pass Space to the composer" fixed in fb5d0ea. Correct catch, and it is a merge-interaction bug: #519 made a Also restored the Enter binding's comment: my fold arm had been inserted between its first line and the rest, so "Retained prompts win the empty Enter: after an aborted turn the" was left dangling above the Space arm and the Enter arm started mid-sentence. The fold arm now sits above that comment with its own. Regression test Full gate green: |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Three conflicts, all the same shape: this branch and main each added a method or test at the same anchor, with both sides ending mid-item so they shared one closing brace. Unioning naively fused two items into one; each side is closed explicitly instead. - app.rs: snap_selection_to_selectable (here) and captured_cards (#526) - render.rs: the folded-group frame test and #526's drill-in test - tasks.rs: the collapse row-count tests and #526's capture tests mask_version intact (2 refs), goldens still read agent-code x.y.z, no regeneration needed. 739 bin tests pass.
|
@codex review — merged main (which absorbed #507, #522, #526). Three conflicts, all the same shape: this branch and main each added a method or test at the same anchor with both sides ending mid-item, so they shared one closing brace — each side is now closed explicitly rather than fused. Kept: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5afe32d10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| #[test] | ||
| fn a_folded_group_hides_its_rows_and_shows_its_size() { |
There was a problem hiding this comment.
Inserting this function immediately after the pre-existing #[test] makes that attribute apply to the new folding test, while normal_mode_is_visible_in_the_prompt below is now an unannotated helper and is silently omitted from the test suite because the CLI crate allows dead code. Add a separate #[test] before the original function so its vi-mode prompt rendering coverage continues to run.
Useful? React with 👍 / 👎.
The main merge inserted the folding test directly after an existing `#[test]`, so that attribute bound to the new function and `normal_mode_is_visible_in_the_prompt` (vi-mode prompt rendering) became an unannotated helper that no longer ran. The crate root allows dead code for its public API surface, so nothing warned. `copy_reports_no_assistant_when_empty` in app.rs was dead the same way and already dead on main; both pass now that they run. The pane modules' test modules opt back in with `#![deny(dead_code)]`, which turns a lost `#[test]` into a compile error instead of a test that silently stops running.
|
@codex review — P2 "Restore the displaced prompt-mode test annotation" fixed in b726dc4. Correct and well spotted. The merge dropped the folding test in directly after an existing Two follow-ons from auditing the same class:
Bin test count goes 739 → 741, matching the two restored tests. Full gate green: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b726dc499c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (m, KeyCode::Char(' ')) | ||
| if m.is_empty() | ||
| && app.tasks_nav_active() | ||
| && !app.show_queue_pane | ||
| && app.input.is_empty() => |
There was a problem hiding this comment.
When vi editing is enabled and the empty composer is in Normal mode, the earlier bare-character dispatch consumes KeyCode::Char(' '), calls the no-op vi_normal_key(' '), and returns before this arm. As a result, the documented fold/unfold key does nothing for vi-mode users even though arrows and Enter still control the pane; route this contextual Space action before normal-mode character handling or explicitly let it fall through under these pane conditions.
Useful? React with 👍 / 👎.
Vi normal mode owns bare characters, so its dispatch arm consumed
Space, called the no-op `vi_normal_key(' ')` and returned before the
fold binding. Vi users got a documented key that did nothing while
their arrows and Enter still drove the pane.
Space now falls through on an empty composer, as Backspace and Enter
already do — as a motion it moves right, which does nothing on an empty
line. Both dispatch sites ask one shared `space_folds_group` predicate
so the conditions cannot drift apart.
|
@codex review — P2 "Let Space reach folding in vi normal mode" fixed in b4e2682. Right again, and the third interaction this branch has had with a PR that landed underneath it (#519 checklist, #522 vi mode, #526 captured cards). Vi normal mode owns bare characters, so its arm consumed Fixed the way that arm already handles the same problem for Backspace and Enter: fall through on an empty composer. As a vi motion Space moves right, which does nothing on an empty line, so normal mode gives up nothing real. Both dispatch sites now ask a single Regression tests: Bin tests 741 → 743. Full gate green: |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Closes the remainder of D4-22. #512 unified agents and background jobs into one pane with group headings; this adds the collapsible part.
The pane grew a second group when background tasks landed in it. With a few agents and a few jobs it fills the strip, and the half you are not watching crowds out the half you are.
Spacefolds the selected group:A folded heading shows its count, so collapsing never hides how much is behind it.
Two things that would otherwise make this feel broken
The pane is sized from the collapsed row count.
pane_rows_collapsedreplaces the sizing input, so folding actually frees vertical space. Sizing from the full list would leave the strip exactly as tall and the feature would do nothing visible —a_collapsed_group_costs_only_its_headingasserts the height genuinely drops, by exactly the folded rows.The selection skips folded rows.
tasks_selectwalksvisible_indices, andtoggle_selected_groupmoves the selection to the first still-visible row when you collapse the group it was in. Without that the selection sits on something off-screen and the pane looks unresponsive to arrow keys — which is indistinguishable from a hang.Key choice
Spacerather than a chord: this branch only runs with an empty composer, so it is not a space the user is trying to type. Same guard shape the existing pane keys use, and it yields to the queue pane exactly as they do.Verification
a_collapsed_group_costs_only_its_heading,pane_rows_matches_the_uncollapsed_count— sizingfolded_rows_are_not_selectable,folding_everything_leaves_nothing_selectable— selection domaincollapsing_moves_a_selection_off_a_hidden_row,selection_steps_over_a_folded_group,unfolding_restores_the_group— selection behavioura_folded_group_hides_its_rows_and_shows_its_size— renders throughTestBackendand asserts the row text disappears, the▸ agents (1)marker appears, and the other group is untouched652 bin tests pass;
clippy --all-targets -- -D warningsandfmt --checkclean.