feat: multi-column sort for ResourceTable#52
Conversation
- Track sort state via _sort_columns (list of (col_idx, reverse) tuples) - Click column header: set as primary sort ascending; click again to toggle - Shift+Click: add secondary/tertiary sort (max 3 columns) - Sort indicators ▲/▼ displayed in column headers; multi-sort shows rank (▲1, ▲2) - Stable multi-key sort using reversed-order stable sort trick - Sort resets when resource type changes (different columns); maintained on refresh - Add _compute_next_sort() as testable pure-logic helper - Add 15 new tests covering sort logic, multi-column, edge cases, and UI behavior Co-authored-by: Tanbir Ahmed <iamtanbirahmed@users.noreply.github.com>
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Imports and event handlers src/gantry/widgets.py |
Adds Tuple and rich.text.Text imports. Implements on_mouse_down to record Shift state and on_data_table_header_selected to compute/update _sort_columns, toggle directions, handle Shift+click for secondary sorts, and update header labels with ▲/▼ and sort rank. |
Label decorators and type coercion src/gantry/widgets.py |
Adds helpers to build sort-decorated header Text objects showing direction and rank, and type-aware coercion functions used by the sorter. |
Column rebuild and row caching src/gantry/widgets.py |
Refactors populate_resources to produce stable column keys and labels (including indicator text), preserve _sort_columns only when the incoming columns list is unchanged, cache all row values in _all_rows, and defer actual row rendering to _apply_filter. |
Stable multi-key sorter and filter integration src/gantry/widgets.py |
Adds _sort_items to perform stable multi-key ascending/descending sorts across _sort_columns. Modifies _apply_filter to always compute visible rows from the search term, call _sort_items, clear existing rows, and re-add rows in sorted order. |
Test suite for sorting behavior tests/test_app.py |
New unit and integration tests validate initial sort state, _sort_items behavior (no-op when empty, single/multi-column ordering, case-insensitive compare), _compute_next_sort transitions (primary setup, toggles, shift additions, max three columns), and integration scenarios for sort state reset/preservation and sorted row rendering. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'feat: multi-column sort for ResourceTable' accurately summarizes the main change: implementing multi-column sorting functionality for the ResourceTable widget. |
| Linked Issues check | ✅ Passed | The PR implementation satisfies all primary coding requirements from issue #49: clickable headers, ascending/descending toggle, Shift+Click for secondary/tertiary sorts (up to 3 columns), visual indicators (▲/▼) with rank numbers, and stable multi-column sorting logic. |
| Out of Scope Changes check | ✅ Passed | All changes are directly related to implementing multi-column sort functionality for ResourceTable, including the widget implementation, supporting helper methods, test coverage, and state management—nothing appears out of scope. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 96.55% which is sufficient. The required threshold is 80.00%. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
claude/issue-49-20260512-2214
Tip
💬 Introducing Slack Agent: The best way for teams to turn conversations into code.
Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
- Generate code and open pull requests
- Plan features and break down work
- Investigate incidents and troubleshoot customer tickets together
- Automate recurring tasks and respond to alerts with triggers
- Summarize progress and report instantly
Built for teams:
- Shared memory across your entire org—no repeating context
- Per-thread sandboxes to safely plan and execute work
- Governance built-in—scoped access, auditability, and budget controls
One agent for your entire SDLC. Right inside Slack.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/gantry/widgets.py`:
- Around line 97-119: Duplicate label-building logic in _update_column_labels
and populate_resources should be extracted into a single helper (e.g.,
_build_column_label or _column_label_for_index) that accepts the column index
and base label and returns the final Text label; move computation of sort_map,
multi, indicator, and suffix into that helper and have both
_update_column_labels and populate_resources call it to set column.label (and
preserve calling self.refresh() in _update_column_labels). Update references to
self._sort_columns, self._columns, and the column objects inside the new helper
so it can be used from both locations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 671985b1-fdee-4ad6-be49-5a6ffe67ca15
📒 Files selected for processing (2)
src/gantry/widgets.pytests/test_app.py
|
@claude, address these comments |
|
Claude finished @iamtanbirahmed's task in 1m 20s —— View job Addressing CodeRabbit Review Comments
Addressed the CodeRabbit refactoring suggestion by extracting a |
…ate sort label logic Co-authored-by: Tanbir Ahmed <iamtanbirahmed@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/gantry/widgets.py`:
- Line 155: populate_resources currently coerces all values to strings
(row_values = [str(resource.get(k, "")) for k in column_keys]) which destroys
numeric/date types and breaks _sort_items' comparison; instead preserve raw
typed values when building each row (keep a parallel raw_values list or store
values without str() in populate_resources for the same column_keys) and pass
those raw values into _sort_items; update _sort_items to perform type-aware
comparisons (handle numbers and datetimes first, then fallback to
case-insensitive string comparison, and handle None consistently) so sorting
respects numeric/date semantics while still using stringified display values for
rendering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d1ee09be-c1b2-432c-85e0-05d4609cc2bd
📒 Files selected for processing (1)
src/gantry/widgets.py
|
@claude address these comments |
|
Claude finished @iamtanbirahmed's task in 3m 28s —— View job Addressing CodeRabbit Review Comments
What was fixed: CodeRabbit flagged that Changes made in
|
…ting
Previously all resource values were stringified at storage time, causing
numeric columns (e.g., restarts: 10) to sort lexicographically ("10" < "2").
- Add _coerce_sort_value returning a (type_tag, value) tuple so numbers,
ISO datetimes, and strings each sort correctly and cross-type comparisons
never raise TypeError
- Store raw (un-stringified) values in _all_rows; stringify only at render
time via add_row(*[str(v) for v in row_values])
- Update _sort_items to use _coerce_sort_value instead of str().lower()
Co-authored-by: Tanbir Ahmed <iamtanbirahmed@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/gantry/widgets.py`:
- Around line 55-57: The current shift state stored in self._shift_held (set in
on_mouse_down) can become stale or be missing if header selection happens
without a prior mouse event; update on_data_table_header_selected to defensively
read the event modifier (e.g., shift = getattr(event, "shift", False)) and then,
after handling the header selection logic that uses self._shift_held, reset
self._shift_held = False so subsequent interactions aren’t affected by stale
state; reference the methods on_mouse_down and on_data_table_header_selected and
the attribute _shift_held when making this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 129d8640-6cbb-4385-95d6-86388794ee27
📒 Files selected for processing (1)
src/gantry/widgets.py
|
Claude finished @iamtanbirahmed's task in 50s —— View job Addressing CodeRabbit Review Comments
What was fixed: CodeRabbit flagged that Change in |
Ensures subsequent header clicks aren't incorrectly treated as Shift+Click if on_mouse_down doesn't fire first (e.g., keyboard navigation). Co-authored-by: Tanbir Ahmed <iamtanbirahmed@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/gantry/widgets.py`:
- Line 11: The module imports rich.text.Text at top-level (see import of Text in
src/gantry/widgets.py), but rich is not declared in pyproject.toml dependencies;
add "rich" (compatible version, e.g. same major as current transitively used by
textual) to the [project]/dependencies in pyproject.toml so the package declares
a direct dependency rather than relying on a transitive dependency; update the
version specifier to match your compatibility policy and run dependency tooling
(poetry/pip) to refresh lockfiles.
- Around line 70-77: The current shift-click branch appends a new sort tuple to
self._sort_columns and when len > 3 uses pop(0), which discards the primary
sort; change the overflow removal to drop the least-significant pre-existing key
instead of the primary by popping the second-to-last entry after the append
(e.g. replace self._sort_columns.pop(0) with self._sort_columns.pop(-2)) so
primary remains at index 0; adjust only the overflow branch in the shift
handling around existing_pos/col_idx/_sort_columns.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1e0fba24-2420-4fb0-896c-1fa24cf3b9fb
📒 Files selected for processing (1)
src/gantry/widgets.py
| from textual.message import Message | ||
| from textual.events import Key | ||
| from textual.css.query import NoMatches | ||
| from rich.text import Text |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm rich is not declared as a direct dependency in pyproject.toml
fd -t f 'pyproject.toml' --max-depth 3 --exec cat {}
echo "---"
fd -t f 'pyproject.toml' --max-depth 3 --exec rg -n '\brich\b' {} \;Repository: iamtanbirahmed/gantry
Length of output: 468
Declare rich as a direct dependency.
rich.text.Text is imported and used at module level, but rich is not listed in dependencies within pyproject.toml. While rich is available transitively through textual, relying on this is fragile—a future Textual release could drop or replace it. Add rich to pyproject.toml's dependencies.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gantry/widgets.py` at line 11, The module imports rich.text.Text at
top-level (see import of Text in src/gantry/widgets.py), but rich is not
declared in pyproject.toml dependencies; add "rich" (compatible version, e.g.
same major as current transitively used by textual) to the
[project]/dependencies in pyproject.toml so the package declares a direct
dependency rather than relying on a transitive dependency; update the version
specifier to match your compatibility policy and run dependency tooling
(poetry/pip) to refresh lockfiles.
| if shift: | ||
| if existing_pos is not None: | ||
| idx, rev = self._sort_columns[existing_pos] | ||
| self._sort_columns[existing_pos] = (idx, not rev) | ||
| else: | ||
| self._sort_columns.append((col_idx, False)) | ||
| if len(self._sort_columns) > 3: | ||
| self._sort_columns.pop(0) |
There was a problem hiding this comment.
Shift+Click overflow silently demotes the primary sort column.
When _sort_columns already has 3 entries and the user shift-clicks a new column, pop(0) discards the primary sort key, promoting what was the secondary to primary. This is surprising UX — the user's most-significant sort silently disappears while they were trying to add a tertiary. Most multi-sort UIs either ignore the addition past the cap or drop the least significant key (last secondary), preserving the user's primary intent.
♻️ Proposed fix — drop the least-significant sort instead of the primary
if shift:
if existing_pos is not None:
idx, rev = self._sort_columns[existing_pos]
self._sort_columns[existing_pos] = (idx, not rev)
else:
- self._sort_columns.append((col_idx, False))
- if len(self._sort_columns) > 3:
- self._sort_columns.pop(0)
+ if len(self._sort_columns) >= 3:
+ # At cap: drop the least-significant key, keep primary intent.
+ self._sort_columns.pop()
+ self._sort_columns.append((col_idx, False))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gantry/widgets.py` around lines 70 - 77, The current shift-click branch
appends a new sort tuple to self._sort_columns and when len > 3 uses pop(0),
which discards the primary sort; change the overflow removal to drop the
least-significant pre-existing key instead of the primary by popping the
second-to-last entry after the append (e.g. replace self._sort_columns.pop(0)
with self._sort_columns.pop(-2)) so primary remains at index 0; adjust only the
overflow branch in the shift handling around existing_pos/col_idx/_sort_columns.
|
Claude finished @iamtanbirahmed's task in 6m 12s —— View job • Addressing CodeRabbit Review CommentsThe PR was closed without merging, so I implemented the full multi-column sort feature on a new branch with all four CodeRabbit fixes applied from the start.
What was implemented
|
1 participant
Implements multi-column sort for the resource table as described in issue #49.
Changes:
Closes #49
Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests