|
| 1 | +# Implementation Plan: Grouped CLI Commands |
| 2 | + |
| 3 | +**Priority**: P0 |
| 4 | +**Source Spec**: `../apcore-cli/docs/features/grouped-commands.md` |
| 5 | +**Module Paths**: `apcore_cli/cli.py`, `apcore_cli/__main__.py`, `apcore_cli/discovery.py`, `apcore_cli/output.py`, `apcore_cli/shell.py` |
| 6 | +**Dependencies**: Core Dispatcher (FE-01), Display Overlay (§5.13) |
| 7 | + |
| 8 | +--- |
| 9 | + |
| 10 | +## Tasks |
| 11 | + |
| 12 | +### Task 1: `_resolve_group()` — group resolution logic |
| 13 | +**Status**: pending |
| 14 | +**Type**: RED-GREEN-REFACTOR |
| 15 | + |
| 16 | +**RED** — Write failing tests in `tests/test_cli.py` (class `TestResolveGroup`): |
| 17 | +- `test_resolve_group_explicit_group`: descriptor with `display.cli.group = "admin"`, `display.cli.alias = "create"` → returns `("admin", "create")` |
| 18 | +- `test_resolve_group_explicit_group_no_alias`: descriptor with `display.cli.group = "admin"` but no alias → returns `("admin", module_id)` |
| 19 | +- `test_resolve_group_opt_out_empty_string`: descriptor with `display.cli.group = ""`, `display.cli.alias = "healthcheck"` → returns `(None, "healthcheck")` |
| 20 | +- `test_resolve_group_auto_from_alias_dot`: descriptor with `display.cli.alias = "user.list"`, no group → returns `("user", "list")` |
| 21 | +- `test_resolve_group_auto_from_module_id_dot`: module_id `"user.create"`, no display overlay → returns `("user", "create")` |
| 22 | +- `test_resolve_group_no_dot_top_level`: module_id `"standalone"`, no display overlay → returns `(None, "standalone")` |
| 23 | +- `test_resolve_group_multi_dot_first_only`: module_id `"a.b.c"`, no display overlay → returns `("a", "b.c")` |
| 24 | +- `test_resolve_group_empty_module_id_warns`: module_id `""` → returns `(None, "")`, WARNING logged |
| 25 | + |
| 26 | +**GREEN** — Implement `_resolve_group(module_id, descriptor)` as a static method on `GroupedModuleGroup`: |
| 27 | +1. Read `display = _get_display(descriptor)`, `cli_display = display.get("cli") or {}`. |
| 28 | +2. `explicit_group = cli_display.get("group")`. |
| 29 | +3. If `explicit_group` is non-empty string: return `(explicit_group, cli_display.get("alias") or module_id)`. |
| 30 | +4. If `explicit_group == ""`: return `(None, cli_display.get("alias") or module_id)`. |
| 31 | +5. `cli_name = cli_display.get("alias") or module_id`. |
| 32 | +6. If `"." in cli_name`: `group, _, cmd = cli_name.partition(".")` → return `(group, cmd)`. |
| 33 | +7. Else: return `(None, cli_name)`. |
| 34 | + |
| 35 | +**REFACTOR** — None expected. |
| 36 | + |
| 37 | +**Verification**: `pytest tests/test_cli.py::TestResolveGroup -v` |
| 38 | + |
| 39 | +--- |
| 40 | + |
| 41 | +### Task 2: `_build_group_map()` and `GroupedModuleGroup.__init__` |
| 42 | +**Status**: pending |
| 43 | +**Type**: RED-GREEN-REFACTOR |
| 44 | + |
| 45 | +**RED** — Write failing tests in `tests/test_cli.py` (class `TestBuildGroupMap`): |
| 46 | +- `test_build_group_map_three_groups`: Registry has `product.list`, `product.get`, `user.create`, `user.list`, `standalone` → `_group_map = {"product": 2 entries, "user": 2 entries}`, `_top_level_modules = {"standalone": 1 entry}` |
| 47 | +- `test_build_group_map_idempotent`: Call twice → second call is a no-op (check registry.list call count = 1) |
| 48 | +- `test_build_group_map_builtin_collision_warns`: Module `list.something` exists → WARNING logged about collision with built-in command `list` |
| 49 | +- `test_build_group_map_failure_allows_retry`: Registry raises on first call → `_group_map_built` stays False, second call retries |
| 50 | +- `test_build_group_map_with_display_overlay_group`: descriptor with `display.cli.group = "admin"`, `display.cli.alias = "create"` → grouped under "admin" with command name "create" |
| 51 | + |
| 52 | +**GREEN** — Implement: |
| 53 | +- `GroupedModuleGroup(LazyModuleGroup)` with `__init__` adding `_group_map`, `_top_level_modules`, `_group_cache`, `_group_map_built`. |
| 54 | +- `_build_group_map()`: |
| 55 | + 1. Guard: if `_group_map_built`, return. |
| 56 | + 2. Call `self._build_alias_map()` (parent method populates descriptor cache). |
| 57 | + 3. Iterate `self._registry.list()`, get descriptors from cache, call `_resolve_group`. |
| 58 | + 4. Partition into `_group_map` and `_top_level_modules`. |
| 59 | + 5. Check group name collisions with `BUILTIN_COMMANDS`, log warnings. |
| 60 | + 6. Set `_group_map_built = True` inside try block. |
| 61 | + |
| 62 | +**REFACTOR** — None expected. |
| 63 | + |
| 64 | +**Verification**: `pytest tests/test_cli.py::TestBuildGroupMap -v` |
| 65 | + |
| 66 | +--- |
| 67 | + |
| 68 | +### Task 3: `list_commands()` and `get_command()` overrides |
| 69 | +**Status**: pending |
| 70 | +**Type**: RED-GREEN-REFACTOR |
| 71 | + |
| 72 | +**RED** — Write failing tests in `tests/test_cli.py` (class `TestGroupedModuleGroupRouting`): |
| 73 | +- `test_list_commands_shows_groups_and_top_level`: With product (2 modules) + user (2 modules) + standalone → returns sorted `[completion, describe, exec, list, man, product, standalone, user]` |
| 74 | +- `test_get_command_returns_lazy_group`: `get_command(ctx, "product")` → returns a `click.Group` instance (the `_LazyGroup`) |
| 75 | +- `test_get_command_returns_top_level_command`: `get_command(ctx, "standalone")` → returns a `click.Command` (not a group) |
| 76 | +- `test_get_command_returns_builtin`: `get_command(ctx, "list")` → returns built-in list command (not a group named "list") |
| 77 | +- `test_get_command_unknown_returns_none`: `get_command(ctx, "nonexistent")` → returns `None` |
| 78 | +- `test_get_command_caches_lazy_group`: Two calls to `get_command(ctx, "product")` → same object |
| 79 | + |
| 80 | +**GREEN** — Override `list_commands()` and `get_command()` on `GroupedModuleGroup`: |
| 81 | +- `list_commands`: build group map, return sorted(builtins + group names (excluding collisions) + top-level module names). |
| 82 | +- `get_command`: check builtins → check group cache → check group map (create `_LazyGroup`) → check top-level modules → None. |
| 83 | + |
| 84 | +**REFACTOR** — None expected. |
| 85 | + |
| 86 | +**Verification**: `pytest tests/test_cli.py::TestGroupedModuleGroupRouting -v` |
| 87 | + |
| 88 | +--- |
| 89 | + |
| 90 | +### Task 4: `_LazyGroup` — nested group commands |
| 91 | +**Status**: pending |
| 92 | +**Type**: RED-GREEN-REFACTOR |
| 93 | + |
| 94 | +**RED** — Write failing tests in `tests/test_cli.py` (class `TestLazyGroup`): |
| 95 | +- `test_lazy_group_list_commands`: `_LazyGroup` with members `{"list": ..., "get": ..., "create": ...}` → `list_commands` returns `["create", "get", "list"]` |
| 96 | +- `test_lazy_group_get_command`: `get_command(ctx, "list")` → returns a `click.Command` built from the descriptor |
| 97 | +- `test_lazy_group_get_command_not_found`: `get_command(ctx, "nonexistent")` → returns `None` |
| 98 | +- `test_lazy_group_caches_commands`: Two calls to `get_command(ctx, "list")` → same object |
| 99 | +- `test_lazy_group_command_execution`: Via `CliRunner`, invoke `apcore-cli product list --category food` → executor called with correct module_id and args |
| 100 | + |
| 101 | +**GREEN** — Implement `_LazyGroup(click.Group)`: |
| 102 | +- `__init__`: store `members`, `executor`, `help_text_max_length`, init `_cmd_cache`. |
| 103 | +- `list_commands`: return `sorted(self._members.keys())`. |
| 104 | +- `get_command`: check cache → lookup in members → `build_module_command` → cache → return. |
| 105 | + |
| 106 | +**REFACTOR** — None expected. |
| 107 | + |
| 108 | +**Verification**: `pytest tests/test_cli.py::TestLazyGroup -v` |
| 109 | + |
| 110 | +--- |
| 111 | + |
| 112 | +### Task 5: `format_help()` — collapsed root help display |
| 113 | +**Status**: pending |
| 114 | +**Type**: RED-GREEN-REFACTOR |
| 115 | + |
| 116 | +**RED** — Write failing tests in `tests/test_cli.py` (class `TestGroupedHelpDisplay`): |
| 117 | +- `test_root_help_shows_groups_section`: Via `CliRunner --help`, output contains "Groups:" section header |
| 118 | +- `test_root_help_shows_group_with_count`: Output contains `product` with "(3 commands)" or similar count |
| 119 | +- `test_root_help_shows_top_level_modules`: Output contains "Modules:" section with standalone command |
| 120 | +- `test_root_help_shows_builtin_commands`: Output contains "Commands:" with exec, list, describe, etc. |
| 121 | +- `test_group_help_shows_commands`: Via `CliRunner`, `apcore-cli product --help` shows individual commands (list, get, create) |
| 122 | + |
| 123 | +**GREEN** — Override `format_help()` on `GroupedModuleGroup`: |
| 124 | +1. Build group map. |
| 125 | +2. Use Click's `HelpFormatter` to write sections: Options, Commands (builtins), Modules (top-level), Groups (with counts). |
| 126 | +3. `_LazyGroup` uses default Click help formatting (shows its commands normally). |
| 127 | + |
| 128 | +**REFACTOR** — None expected. |
| 129 | + |
| 130 | +**Verification**: `pytest tests/test_cli.py::TestGroupedHelpDisplay -v` |
| 131 | + |
| 132 | +--- |
| 133 | + |
| 134 | +### Task 6: Wire `GroupedModuleGroup` into `create_cli()` |
| 135 | +**Status**: pending |
| 136 | +**Type**: RED-GREEN-REFACTOR |
| 137 | + |
| 138 | +**RED** — Write failing tests in `tests/test_cli.py` (class `TestCreateCliGrouped`): |
| 139 | +- `test_create_cli_uses_grouped_module_group`: Call `create_cli(extensions_dir=...)` → returned group is instance of `GroupedModuleGroup` |
| 140 | + |
| 141 | +**GREEN** — Change `__main__.py`: |
| 142 | +- Import `GroupedModuleGroup` instead of (or in addition to) `LazyModuleGroup`. |
| 143 | +- Change `cls=LazyModuleGroup` → `cls=GroupedModuleGroup` in the `@click.group()` decorator. |
| 144 | + |
| 145 | +**REFACTOR** — None expected. |
| 146 | + |
| 147 | +**Verification**: `pytest tests/test_cli.py::TestCreateCliGrouped -v` |
| 148 | + |
| 149 | +--- |
| 150 | + |
| 151 | +### Task 7: Discovery `list --flat` and `describe group.command` |
| 152 | +**Status**: pending |
| 153 | +**Type**: RED-GREEN-REFACTOR |
| 154 | + |
| 155 | +**RED** — Write failing tests in `tests/test_discovery.py` (class `TestGroupedDiscovery`): |
| 156 | +- `test_list_flat_flag`: `apcore-cli list --flat` → output matches flat table (all modules, no grouping) |
| 157 | +- `test_list_default_grouped_display`: `apcore-cli list` with grouped modules → output shows group headers with commands underneath |
| 158 | +- `test_describe_group_dot_command`: `apcore-cli describe product.list` → resolves to the correct module, shows metadata |
| 159 | +- `test_describe_full_module_id`: `apcore-cli describe product.list_products.get` → works with canonical module_id |
| 160 | + |
| 161 | +**GREEN** — Modify `discovery.py`: |
| 162 | +- `list_cmd`: add `--flat` flag. When not flat, group modules by their display group and render grouped output. |
| 163 | +- `describe_cmd`: before `validate_module_id`, try to resolve `group.command` notation → scan registry for matching module_id. |
| 164 | + |
| 165 | +Modify `output.py`: |
| 166 | +- Add `format_grouped_module_list()` that renders modules grouped under section headers. |
| 167 | + |
| 168 | +**REFACTOR** — Ensure `format_module_list()` still works for `--flat` path. |
| 169 | + |
| 170 | +**Verification**: `pytest tests/test_discovery.py::TestGroupedDiscovery -v` |
| 171 | + |
| 172 | +--- |
| 173 | + |
| 174 | +### Task 8: Shell completion for nested groups |
| 175 | +**Status**: pending |
| 176 | +**Type**: RED-GREEN-REFACTOR |
| 177 | + |
| 178 | +**RED** — Write failing tests in `tests/test_shell.py` (class `TestGroupedCompletion`): |
| 179 | +- `test_bash_completion_includes_groups`: Generated bash completion for position 1 includes group names |
| 180 | +- `test_bash_completion_nested_commands`: At position 2 after a group name, completes with group's commands |
| 181 | +- `test_zsh_completion_includes_groups`: Generated zsh completion includes group names |
| 182 | +- `test_fish_completion_includes_groups`: Generated fish completion includes group names and nested subcommands |
| 183 | + |
| 184 | +**GREEN** — Modify `shell.py`: |
| 185 | +- Update `_generate_bash_completion`: position 1 completes with builtins + group names + top-level modules; position 2 after a group name completes with that group's commands. |
| 186 | +- Update `_generate_zsh_completion` and `_generate_fish_completion` similarly. |
| 187 | +- Accept `registry` parameter (or group instance) to get group/command lists dynamically. |
| 188 | + |
| 189 | +**REFACTOR** — Extract common group/command list generation. |
| 190 | + |
| 191 | +**Verification**: `pytest tests/test_shell.py::TestGroupedCompletion -v` |
| 192 | + |
| 193 | +--- |
| 194 | + |
| 195 | +### Task 9: Integration tests — end-to-end grouped invocation |
| 196 | +**Status**: pending |
| 197 | +**Type**: RED-GREEN-REFACTOR |
| 198 | + |
| 199 | +**RED** — Write failing tests in `tests/test_cli.py` (class `TestGroupedE2E`): |
| 200 | +- `test_grouped_invocation_product_get`: Via `CliRunner`, `apcore-cli product get --id 123` → executor called with correct module_id |
| 201 | +- `test_single_command_group_works`: `apcore-cli health check` → executor called with `health.check` module_id |
| 202 | +- `test_top_level_module_works`: `apcore-cli standalone --key val` → executor called with `standalone` module_id |
| 203 | +- `test_unknown_group_exits_2`: `apcore-cli nonexistent` → exit code 2 |
| 204 | +- `test_unknown_command_in_group_exits_2`: `apcore-cli product nonexistent` → exit code 2 |
| 205 | + |
| 206 | +**GREEN** — No new production code (this validates the full stack from tasks 1–8). |
| 207 | + |
| 208 | +**REFACTOR** — Fix any issues found during integration. |
| 209 | + |
| 210 | +**Verification**: `pytest tests/test_cli.py::TestGroupedE2E -v` |
| 211 | + |
| 212 | +--- |
| 213 | + |
| 214 | +## Implementation Order |
| 215 | + |
| 216 | +Execute tasks sequentially: 1 → 2 → 3 → 4 → 5 → 6 → 7 → 8 → 9 |
| 217 | + |
| 218 | +Tasks 1–4 are the core grouped commands engine (cli.py only). |
| 219 | +Task 5 is the help display. |
| 220 | +Task 6 wires it in. |
| 221 | +Tasks 7–8 update downstream features. |
| 222 | +Task 9 is the integration test sweep. |
| 223 | + |
| 224 | +## Files Modified |
| 225 | + |
| 226 | +| File | Tasks | Changes | |
| 227 | +|------|-------|---------| |
| 228 | +| `src/apcore_cli/cli.py` | 1–5 | Add `GroupedModuleGroup`, `_LazyGroup`, `_resolve_group`, `_build_group_map`, `format_help` | |
| 229 | +| `src/apcore_cli/__main__.py` | 6 | Change `cls=LazyModuleGroup` → `cls=GroupedModuleGroup` | |
| 230 | +| `src/apcore_cli/discovery.py` | 7 | Add `--flat` flag, `group.command` resolution in `describe` | |
| 231 | +| `src/apcore_cli/output.py` | 7 | Add `format_grouped_module_list()` | |
| 232 | +| `src/apcore_cli/shell.py` | 8 | Update completion generators for two-level groups | |
| 233 | +| `tests/test_cli.py` | 1–6, 9 | ~30 new tests across 7 test classes | |
| 234 | +| `tests/test_discovery.py` | 7 | ~4 new tests | |
| 235 | +| `tests/test_shell.py` | 8 | ~4 new tests | |
0 commit comments