feat: grouped collapsible sidebar with full K8s resource taxonomy#3
Merged
Conversation
Implements ResourceSidebar in widgets.py with 5 collapsible groups (Workloads, Service, Config & Storage, Cluster, CRDs), ResourceSelected message, and 6 new passing tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ion, clean imports Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…if with tables - Replace bare ListView sidebar with ResourceSidebar widget in ClusterScreen - Add _FETCH_FNS, _COLUMN_DEFS, _DESCRIBE_TYPE_MAP dispatch tables - Remove _RESOURCE_TYPES class attribute - Add on_resource_sidebar_resource_selected handler; remove on_list_view_highlighted - Fix describe_resource to use _DESCRIBE_TYPE_MAP (fixes Config Maps → ConfigMap) - Add _ready guard + call_after_refresh in ResourceSidebar to suppress mount-time highlights - Add focus_first_item() to ResourceSidebar; update ClusterScreen panel nav to use it - Fix _on_key to be async to avoid coroutine-not-awaited warning - Update tests: replace _RESOURCE_TYPES check with _FETCH_FNS/_COLUMN_DEFS, update navigation expectations to match new group structure Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Skip Fix 1 (_ready guard removal): Textual fires Highlighted on every ListView during mount across all 5 Collapsible groups, so removing the guard causes CRDs to win as the final resource type. The _ready guard remains as the correct defensive pattern for this widget structure. Applied fixes: same-type debounce in on_resource_sidebar_resource_selected, remove conflicting width:20 from #resource-type-sidebar CSS (widget CSS width:24 now wins), rename test_cluster_screen_has_sidebar to test_cluster_screen_has_dispatch_tables. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove leftover #resource-type-sidebar CSS blocks from ClusterScreen now handled by ResourceSidebar widget CSS. Add test verifying stub resource types set connection_status to "Not implemented". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe pull request introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ResourceSidebar
participant ClusterScreen
participant ResourceTable
User->>ResourceSidebar: Highlights resource type
ResourceSidebar->>ResourceSidebar: Emit ResourceSelected(resource_type, implemented)
ResourceSidebar->>ClusterScreen: Post ResourceSelected message
alt Resource Implemented
ClusterScreen->>ClusterScreen: Set current_resource_type
ClusterScreen->>ClusterScreen: Resolve fetch function via _FETCH_FNS
ClusterScreen->>ClusterScreen: Fetch resources for type
ClusterScreen->>ClusterScreen: Apply column defs from _COLUMN_DEFS
ClusterScreen->>ResourceTable: Update with fetched data & columns
ResourceTable->>User: Display populated table
else Resource Not Implemented
ClusterScreen->>ClusterScreen: Set current_resource_type
ClusterScreen->>ClusterScreen: Set connection_status to "Not implemented"
ClusterScreen->>ResourceTable: Clear table
ResourceTable->>User: Show empty table with stub status
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/gantry/widgets.py (1)
356-393: Make the sidebar taxonomy immutable.
GROUPSis shared class state, so any accidental mutation affects everyResourceSidebarinstance. Use immutable tuples and aClassVarannotation to avoid global sidebar drift.♻️ Proposed hardening
-from typing import Any, Dict, List, Optional, Callable +from typing import Any, Callable, ClassVar, Dict, List, Optional class ResourceSidebar(Widget): """Grouped, collapsible sidebar for Kubernetes resource type selection.""" - GROUPS: list[tuple[str, list[tuple[str, bool]]]] = [ - ("Workloads", [ + GROUPS: ClassVar[tuple[tuple[str, tuple[tuple[str, bool], ...]], ...]] = ( + ("Workloads", ( ("Pods", True), ("Deployments", True), ("Daemon Sets", False), ("Stateful Sets", False), ("Replica Sets", False), ("Replication Controllers", False), ("Jobs", False), ("Cron Jobs", False), - ]), - ("Service", [ + )), + ("Service", ( ("Services", True), ("Ingresses", False), ("Ingress Classes", False), - ]), + )), ... - ] + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gantry/widgets.py` around lines 356 - 393, GROUPS is currently a mutable list shared across ResourceSidebar instances; change it to an immutable structure and mark it as class-level only by converting the list-of-lists to nested tuples and annotating GROUPS as a ClassVar[tuple[tuple[str, tuple[tuple[str, bool], ...]], ...]] (or equivalent typing) so it cannot be mutated per-instance. Locate the GROUPS symbol in src/gantry/widgets.py and replace the top-level list and inner lists with tuples, update the type annotation to use ClassVar and tuple types, and ensure any code that iterates GROUPS still works with tuples (no runtime logic changes).src/gantry/screens.py (1)
278-295: Use the requiredRESOURCE_COLUMNSdictionary name.The new column definitions live in
_COLUMN_DEFS, but the project convention expectsRESOURCE_COLUMNSonClusterScreen. Rename the map and update the lookup to match that contract. As per coding guidelines,src/gantry/screens.py: Define resource columns with column definitions inRESOURCE_COLUMNSdictionaries on ClusterScreen.♻️ Proposed rename
- _COLUMN_DEFS = { + RESOURCE_COLUMNS = { @@ - col_def = self._COLUMN_DEFS.get(resource_type) + col_def = self.RESOURCE_COLUMNS.get(resource_type)Also applies to: 461-466
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gantry/screens.py` around lines 278 - 295, Rename the private map _COLUMN_DEFS to RESOURCE_COLUMNS and update any lookups to use RESOURCE_COLUMNS on ClusterScreen; specifically change the dictionary name defined near the top (currently _COLUMN_DEFS) to RESOURCE_COLUMNS and adjust any code in ClusterScreen that reads column definitions (references to _COLUMN_DEFS, e.g., where ClusterScreen accesses pod/service/deployment/configmap columns) to reference RESOURCE_COLUMNS instead so the class follows the required contract; also apply the same rename/update for the second occurrence noted around the later block (the other _COLUMN_DEFS usage at lines ~461-466).tests/test_app.py (1)
346-419: Move component-specific tests into mirrored test modules.These
ResourceSidebarandClusterScreentests maketests/test_app.pycover widget and screen internals. Split them into corresponding modules such astests/test_widgets.pyandtests/test_screens.py, leaving app-shell tests here. As per coding guidelines,tests/test_*.py: Test structure should mirror source files with corresponding test modules for each major component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_app.py` around lines 346 - 419, These tests mix app-shell checks with component internals; move ResourceSidebar-related tests (test_resource_sidebar_instantiates, test_resource_sidebar_has_five_groups, test_resource_sidebar_pods_implemented, test_resource_sidebar_daemon_sets_stub, test_resource_sidebar_resource_selected_message, test_resource_sidebar_stub_resource_selected_message, and test_stub_resource_shows_not_implemented which exercise ResourceSidebar behavior) into a widget-focused test module and move ClusterScreen-specific tests (test_cluster_screen_mounts_resource_sidebar, test_cluster_screen_no_legacy_sidebar and any tests that directly assert ClusterScreen.query_one or on_resource_sidebar_resource_selected behavior) into a screen-focused test module that mirrors the source structure; leave only high-level app-shell integration tests in the current app test file. Ensure imports are updated to reference ResourceSidebar and ClusterScreen in their new modules and that pytest.mark.asyncio contexts are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/gantry/screens.py`:
- Around line 428-430: When _FETCH_FNS lacks a fetch function (fetch_fn is
None), short-circuit _refresh_resources to centrally handle the
unsupported-resource case: set the resource connection state to an explicit
"Unsupported" (instead of letting downstream handlers or the worker
finally-block flip it back to "Connected"), clear _resource_data so no stale
rows remain, and return without triggering the duplicate refresh via
watch_current_resource_type(). Also update the worker's unsupported-resource
path to avoid unconditionally setting "Connected" in its finally block—only set
"Connected" on successful fetch—so that current_resource_type,
watch_current_resource_type, _refresh_resources, _FETCH_FNS, and _resource_data
consistently reflect unsupported resources.
In `@src/gantry/widgets.py`:
- Around line 472-524: The sidebar currently focuses only the first ListView
(focus_first_item) and _on_key doesn't let keyboard users traverse group
ListViews; modify _on_key (and optionally focus_first_item) to allow moving
focus between per-group ListViews when a user reaches a list boundary or presses
Tab/Shift-Tab or left/right arrows: use the ordered keys from
self._list_view_items (the dict/iterable populated from GROUPS) to find the
current list_view id (event.list_view.id), compute the next/previous lv id, call
self.query_one(f"#{next_lv_id}", ListView).focus() and stop the event; also when
handling Up/Down within ListView (e.g., in on_list_view_highlighted or the
ListView key handler) detect idx is 0 and Up was pressed to move to previous
group, or idx == len(items)-1 and Down was pressed to move to next group, and
move focus accordingly so keyboard users can traverse Services, ConfigMaps,
Cluster resources and CRDs.
---
Nitpick comments:
In `@src/gantry/screens.py`:
- Around line 278-295: Rename the private map _COLUMN_DEFS to RESOURCE_COLUMNS
and update any lookups to use RESOURCE_COLUMNS on ClusterScreen; specifically
change the dictionary name defined near the top (currently _COLUMN_DEFS) to
RESOURCE_COLUMNS and adjust any code in ClusterScreen that reads column
definitions (references to _COLUMN_DEFS, e.g., where ClusterScreen accesses
pod/service/deployment/configmap columns) to reference RESOURCE_COLUMNS instead
so the class follows the required contract; also apply the same rename/update
for the second occurrence noted around the later block (the other _COLUMN_DEFS
usage at lines ~461-466).
In `@src/gantry/widgets.py`:
- Around line 356-393: GROUPS is currently a mutable list shared across
ResourceSidebar instances; change it to an immutable structure and mark it as
class-level only by converting the list-of-lists to nested tuples and annotating
GROUPS as a ClassVar[tuple[tuple[str, tuple[tuple[str, bool], ...]], ...]] (or
equivalent typing) so it cannot be mutated per-instance. Locate the GROUPS
symbol in src/gantry/widgets.py and replace the top-level list and inner lists
with tuples, update the type annotation to use ClassVar and tuple types, and
ensure any code that iterates GROUPS still works with tuples (no runtime logic
changes).
In `@tests/test_app.py`:
- Around line 346-419: These tests mix app-shell checks with component
internals; move ResourceSidebar-related tests
(test_resource_sidebar_instantiates, test_resource_sidebar_has_five_groups,
test_resource_sidebar_pods_implemented, test_resource_sidebar_daemon_sets_stub,
test_resource_sidebar_resource_selected_message,
test_resource_sidebar_stub_resource_selected_message, and
test_stub_resource_shows_not_implemented which exercise ResourceSidebar
behavior) into a widget-focused test module and move ClusterScreen-specific
tests (test_cluster_screen_mounts_resource_sidebar,
test_cluster_screen_no_legacy_sidebar and any tests that directly assert
ClusterScreen.query_one or on_resource_sidebar_resource_selected behavior) into
a screen-focused test module that mirrors the source structure; leave only
high-level app-shell integration tests in the current app test file. Ensure
imports are updated to reference ResourceSidebar and ClusterScreen in their new
modules and that pytest.mark.asyncio contexts are preserved.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e5af2a87-e05b-4357-bdd3-99df37d8da50
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
pyproject.tomlsrc/gantry/screens.pysrc/gantry/widgets.pytests/test_app.py
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.
Summary
ListViewsidebar withResourceSidebarwidget: 5 collapsible groups (Workloads, Service, Config & Storage, Cluster, CRDs) covering 25 resource typesClusterScreenwith_FETCH_FNS,_COLUMN_DEFS, and_DESCRIBE_TYPE_MAPlookup tablesTest Plan
ResourceSidebarunit tests: instantiation, 5 groups, implemented/stub flags,ResourceSelectedmessageClusterScreenmountsResourceSidebar(not bareListView)#resource-type-sidebar ListViewselector remains🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
Chores