TUI usability improvements by daniel-thom · Pull Request #294 · NatLabRockies/torc

daniel-thom · 2026-04-29T20:11:39Z

No description provided.

Press 'm' to cycle Peak Memory sort (desc → asc → off) and 'p' to cycle Peak CPU % sort. Column headers show ↓ / ↑ indicators when active. Also renames the Results column from "Avg CPU %" to "Peak CPU %" since it has always read peak_cpu_percent, not avg_cpu_percent. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

… UX, scoped help - Jobs tab sorting via 1/2/3 (cycles ID/Name/Status: desc → asc → off) with arrow indicators in column headers - g / G jump to top / bottom of the focused table (Workflows or any Detail pane) - Preserve selection across workflow refresh and Jobs / Results sort cycles instead of snapping back to row 0 - Show active filter ("filter: column~value") in every table title in magenta when a filter is set - '=' applies a filter using the selected row's primary column value (Workflows → User, Jobs / Results / Scheduled Nodes → Status, Events → Event Type, Files → Name, Compute Nodes → Hostname) - Scope the ? help popup to the active pane / detail tab via HelpContext, showing only the relevant section plus globals Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR improves TUI usability by adding context-aware help, visible filter state in table titles, quick navigation, and table sorting for Jobs/Results.

Changes:

Add HelpContext and render context-specific help content in the help popup.
Show active filter state in table titles and add = shortcut to filter by the currently selected row.
Add jump-to-top/bottom keys (g/G) and add sorting controls + indicators for Jobs (1/2/3) and Results (m/p).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
src/tui/ui.rs	Adds help-context mapping, filter suffix display in titles, and sort indicators in headers.
src/tui/components.rs	Reworks help popup to show context-specific keybindings via `HelpContext`.
src/tui/app.rs	Adds sort state + sorting/selection restore logic, jump-to-top/bottom, and filter-by-row behavior.
src/tui.rs	Wires new keybindings for jumping, filter-by-row, and sorting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- refresh_workflows: clamp stale selection index after refresh and clear selection on empty list, instead of leaving an out-of-bounds index - restore_results_selection / restore_jobs_selection: explicitly select None when the list becomes empty so a stale index can't be used by later actions - JobsSort id ordering: keep None last in both directions to match the None-last convention used by the Results sort and the status sort - JobsSort name ordering: use sort_by_cached_key so we only allocate the lowercased key once per row instead of per comparison Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- filter_by_current_row: don't mutate filter_target before validating that column_name resolves; restore the prior target on the early return path so a bad mapping can't leave the app inconsistent - ResultsSort PeakCpu*: use f64::total_cmp instead of partial_cmp + unwrap_or(Equal). The latter violates the strict-weak-ordering required by sort_by when NaN is present and can scramble unrelated rows - help_context_for: while a popup is open, focus is Focus::Popup and the prior focus is stashed in previous_focus. Read previous_focus so the help popup keeps showing the pane-specific section instead of collapsing to the generic Other view Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Previously c always cancelled the selected job in the Jobs detail tab, which meant a filter applied to the Jobs tab couldn't be cleared without leaving the tab. Now c uniformly clears the active filter on whatever pane has focus, and C cancels the selected job when on the Jobs detail tab (and otherwise cancels the selected workflow, matching its existing role). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

- Add g/G jump-to-top/bottom and PgUp/PgDn rows to Basic Navigation - Note that ? help is now context-aware (scoped to current pane / tab) - Update Job Management table: cancel is now C (was c) and add row for 1/2/3 column sort - New "Filtering and Sorting" section covering f / = / c keys, the filter-suffix shown in table titles, and the Jobs / Results sort shortcuts (with the sticky-selection behavior) - New "Results Tab" section listing the columns including the renamed Peak CPU % column Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Pressing 'e' from either the Workflows pane or any Detail tab switches to the Events detail tab for the highlighted workflow and opens its live Server-Sent Events stream. Saves the 4-keystroke "Enter, Tab, Tab, Tab" walk to reach Events from the default Summary landing tab. The shortcut is special-cased rather than generalizing to "a letter per tab" because Events has the unique side effect of starting an SSE connection, and a generic scheme would conflict with existing keys (r=refresh, s=submit, f=filter, ...). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-29T22:05:38Z

+    pub fn cycle_results_sort_peak_memory(&mut self) {
+        let prev_id = self.selected_result_id();
+        self.results_sort = self.results_sort.cycle_peak_memory();
+        self.apply_results_sort();
+        self.restore_results_selection(prev_id);
+    }
+
+    pub fn cycle_results_sort_peak_cpu(&mut self) {
+        let prev_id = self.selected_result_id();
+        self.results_sort = self.results_sort.cycle_peak_cpu();
+        self.apply_results_sort();


ResultsSort::None currently leaves the vector in whatever order it had from the previous sort, so cycling to “off” doesn’t actually turn sorting off (it only hides the indicator). This conflicts with the documented/help behavior of cycling ↓ ↑ off. Consider making the None state actively restore the natural (unsorted) order for the current view, e.g., by rebuilding self.results from the canonical source (results_all + current filter) when sort becomes None, then re-applying selection restoration.

Suggested change

pub fn cycle_results_sort_peak_memory(&mut self) {

let prev_id = self.selected_result_id();

self.results_sort = self.results_sort.cycle_peak_memory();

self.apply_results_sort();

self.restore_results_selection(prev_id);

}

pub fn cycle_results_sort_peak_cpu(&mut self) {

let prev_id = self.selected_result_id();

self.results_sort = self.results_sort.cycle_peak_cpu();

self.apply_results_sort();

fn restore_unsorted_results_order(&mut self) {

let mut visible_counts: HashMap<Option<i64>, usize> = HashMap::new();

for result in &self.results {

*visible_counts.entry(result.id).or_insert(0) += 1;

}

self.results = self

.results_all

.iter()

.filter_map(|result| {

let remaining = visible_counts.get_mut(&result.id)?;

if *remaining == 0 {

return None;

}

*remaining -= 1;

Some(result.clone())

})

.collect();

}

pub fn cycle_results_sort_peak_memory(&mut self) {

let prev_id = self.selected_result_id();

self.results_sort = self.results_sort.cycle_peak_memory();

if matches!(self.results_sort, ResultsSort::None) {

self.restore_unsorted_results_order();

} else {

self.apply_results_sort();

}

self.restore_results_selection(prev_id);

}

pub fn cycle_results_sort_peak_cpu(&mut self) {

let prev_id = self.selected_result_id();

self.results_sort = self.results_sort.cycle_peak_cpu();

if matches!(self.results_sort, ResultsSort::None) {

self.restore_unsorted_results_order();

} else {

self.apply_results_sort();

}

Copilot · 2026-04-29T22:05:39Z

+    /// Sort `self.jobs` in-place based on `self.jobs_sort`. Stable for None.
+    pub fn apply_jobs_sort(&mut self) {
+        match self.jobs_sort {
+            JobsSort::None => {}
+            // Sort missing IDs last in either direction (server-assigned IDs
+            // should always be present, but match the None-last convention
+            // used by the Results / Status sorts for consistency).
+            JobsSort::IdDesc => self
+                .jobs
+                .sort_by_key(|j| (j.id.is_none(), std::cmp::Reverse(j.id.unwrap_or(i64::MIN)))),
+            JobsSort::IdAsc => self
+                .jobs
+                .sort_by_key(|j| (j.id.is_none(), j.id.unwrap_or(i64::MAX))),
+            // Cache lowercased keys so we don't allocate on every comparison.
+            JobsSort::NameDesc => self
+                .jobs
+                .sort_by_cached_key(|j| std::cmp::Reverse(j.name.to_lowercase())),
+            JobsSort::NameAsc => self.jobs.sort_by_cached_key(|j| j.name.to_lowercase()),
+            JobsSort::StatusDesc => self.jobs.sort_by(|a, b| {
+                let ka = a.status.map(|s| s as u8).unwrap_or(u8::MAX);
+                let kb = b.status.map(|s| s as u8).unwrap_or(u8::MAX);
+                kb.cmp(&ka)
+            }),
+            JobsSort::StatusAsc => self.jobs.sort_by(|a, b| {
+                let ka = a.status.map(|s| s as u8).unwrap_or(u8::MAX);
+                let kb = b.status.map(|s| s as u8).unwrap_or(u8::MAX);
+                ka.cmp(&kb)
+            }),
+        }
+    }
+
+    pub fn cycle_jobs_sort_id(&mut self) {
+        let prev_id = self.selected_job_id();
+        self.jobs_sort = self.jobs_sort.cycle_id();
+        self.apply_jobs_sort();
+        self.restore_jobs_selection(prev_id);
+    }
+
+    pub fn cycle_jobs_sort_name(&mut self) {
+        let prev_id = self.selected_job_id();
+        self.jobs_sort = self.jobs_sort.cycle_name();
+        self.apply_jobs_sort();
+        self.restore_jobs_selection(prev_id);
+    }
+
+    pub fn cycle_jobs_sort_status(&mut self) {
+        let prev_id = self.selected_job_id();
+        self.jobs_sort = self.jobs_sort.cycle_status();
+        self.apply_jobs_sort();
+        self.restore_jobs_selection(prev_id);
+    }


Same issue as Results sorting: when JobsSort::None is selected, the table remains in the previously-sorted order, so cycling to “off” doesn’t actually revert to the natural order (it only removes the arrow indicator). To match the UI/docs (cycles ↓ ↑ off), consider restoring the unsorted order when switching to None (e.g., rebuild self.jobs from jobs_all + current filter, then re-apply selection restoration). Also, the doc comment Stable for None is misleading in the current behavior—either restore natural order for None or update the comment/help/docs to reflect that None means “keep current order.”

daniel-thom and others added 2 commits April 29, 2026 13:15

daniel-thom requested a review from Copilot April 29, 2026 20:11

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Comment thread src/tui/app.rs Outdated

Comment thread src/tui/app.rs Outdated

Comment thread src/tui/app.rs Outdated

Comment thread src/tui/app.rs

Comment thread src/tui/app.rs

Copilot started reviewing on behalf of daniel-thom April 29, 2026 20:40 View session

daniel-thom requested a review from Copilot April 29, 2026 20:50

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Comment thread src/tui/app.rs

Comment thread src/tui/app.rs

Comment thread src/tui/ui.rs

daniel-thom requested a review from Copilot April 29, 2026 21:11

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Comment thread src/tui/app.rs

Comment thread src/tui/app.rs

Comment thread src/tui/app.rs

Comment thread src/tui/components.rs Outdated

Copilot started reviewing on behalf of daniel-thom April 29, 2026 21:22 View session

Copilot started reviewing on behalf of daniel-thom April 29, 2026 21:31 View session

daniel-thom and others added 2 commits April 29, 2026 15:40

daniel-thom requested a review from Copilot April 29, 2026 21:57

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Copilot started reviewing on behalf of daniel-thom April 29, 2026 22:08 View session

daniel-thom merged commit faafd02 into main Apr 29, 2026
13 checks passed

daniel-thom deleted the feat/tui-usability-improvements branch April 29, 2026 22:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TUI usability improvements#294

TUI usability improvements#294
daniel-thom merged 7 commits intomainfrom
feat/tui-usability-improvements

daniel-thom commented Apr 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    pub fn cycle_results_sort_peak_memory(&mut self) {
-        let prev_id = self.selected_result_id();
-        self.results_sort = self.results_sort.cycle_peak_memory();
-        self.apply_results_sort();
-        self.restore_results_selection(prev_id);
-    }
-    pub fn cycle_results_sort_peak_cpu(&mut self) {
-        let prev_id = self.selected_result_id();
-        self.results_sort = self.results_sort.cycle_peak_cpu();
-        self.apply_results_sort();
+    fn restore_unsorted_results_order(&mut self) {
+        let mut visible_counts: HashMap<Option<i64>, usize> = HashMap::new();
+        for result in &self.results {
+            *visible_counts.entry(result.id).or_insert(0) += 1;
+        }
+        self.results = self
+            .results_all
+            .iter()
+            .filter_map(|result| {
+                let remaining = visible_counts.get_mut(&result.id)?;
+                if *remaining == 0 {
+                    return None;
+                }
+                *remaining -= 1;
+                Some(result.clone())
+            })
+            .collect();
+    }
+    pub fn cycle_results_sort_peak_memory(&mut self) {
+        let prev_id = self.selected_result_id();
+        self.results_sort = self.results_sort.cycle_peak_memory();
+        if matches!(self.results_sort, ResultsSort::None) {
+            self.restore_unsorted_results_order();
+        } else {
+            self.apply_results_sort();
+        }
+        self.restore_results_selection(prev_id);
+    }
+    pub fn cycle_results_sort_peak_cpu(&mut self) {
+        let prev_id = self.selected_result_id();
+        self.results_sort = self.results_sort.cycle_peak_cpu();
+        if matches!(self.results_sort, ResultsSort::None) {
+            self.restore_unsorted_results_order();
+        } else {
+            self.apply_results_sort();
+        }

Conversation

daniel-thom commented Apr 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants