From 7a94e7ee4a226e1920a27bc31c90df63cf325430 Mon Sep 17 00:00:00 2001 From: Lawrence Lane Date: Fri, 10 Apr 2026 16:02:39 -0400 Subject: [PATCH 1/3] Fix group help and refactor command resolution with discriminated types Two bugs: (1) bare group invocation (`myapp config`) showed "Unknown command" instead of group help, (2) help output showed raw `_command` positional arg instead of listing subcommands by name. Introduces ResolvedCommand/ResolvedGroup/ResolvedNothing union type so `_resolve_command_from_args` distinguishes commands, groups, and misses. Group and root help now render from CLI/Group registries instead of extracting from argparse internals. Suppresses noisy defaults (False, 0, empty string) in help template and tightens whitespace. Co-Authored-By: Claude Opus 4.6 --- src/milo/commands.py | 137 +++++++++++++++++++++++++++------- src/milo/groups.py | 32 ++++++++ src/milo/help.py | 2 + src/milo/templates/help.kida | 33 +++++++-- tests/test_groups.py | 139 +++++++++++++++++++++++++++++++++++ 5 files changed, 310 insertions(+), 33 deletions(-) diff --git a/src/milo/commands.py b/src/milo/commands.py index 4201868..5469599 100644 --- a/src/milo/commands.py +++ b/src/milo/commands.py @@ -9,6 +9,7 @@ import os import sys from collections.abc import Callable +from dataclasses import dataclass from typing import TYPE_CHECKING, Any, NoReturn from milo._command_defs import ( @@ -41,6 +42,38 @@ ] +# --------------------------------------------------------------------------- +# Resolve result types — discriminated union for command resolution +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True, slots=True) +class ResolvedCommand: + """A command was found and should be dispatched.""" + + command: CommandDef | LazyCommandDef + fmt: str + + +@dataclass(frozen=True, slots=True) +class ResolvedGroup: + """A group was invoked without a subcommand — show its help.""" + + group: Group + fmt: str + + +@dataclass(frozen=True, slots=True) +class ResolvedNothing: + """No command or group matched — may offer did-you-mean.""" + + attempted: str | None + fmt: str + + +ResolveResult = ResolvedCommand | ResolvedGroup | ResolvedNothing + + class _MiloArgumentParser(argparse.ArgumentParser): """ArgumentParser subclass that provides did-you-mean suggestions.""" @@ -620,7 +653,7 @@ def _add_commands_to_subparsers( "--format", choices=["plain", "json", "table"], default="plain", - help="Output format (default: plain)", + help="Output format", ) def _add_groups_to_subparsers( @@ -747,20 +780,27 @@ def run(self, argv: list[str] | None = None) -> Any: ctx = self._build_context(args) # Resolve command from args (may be nested in groups) - found, fmt = self._resolve_command_from_args(args) - if not found: - # Did-you-mean suggestion for typos - cmd_name = getattr(args, "_command", None) - if cmd_name: - suggestion = self.suggest_command(cmd_name) + result = self._resolve_command_from_args(args) + + if isinstance(result, ResolvedGroup): + result.group.format_help(self.name) + return None + + if isinstance(result, ResolvedNothing): + if result.attempted: + suggestion = self.suggest_command(result.attempted) if suggestion: sys.stderr.write( - f"Unknown command: {cmd_name!r}. Did you mean {suggestion!r}?\n" + f"Unknown command: {result.attempted!r}. " + f"Did you mean {suggestion!r}?\n" ) return None - parser.print_help() + self._format_root_help() return None + found = result.command + fmt = result.fmt + # Resolve lazy commands cmd = found.resolve() if isinstance(found, LazyCommandDef) else found @@ -914,30 +954,79 @@ def _build_context(self, args: argparse.Namespace) -> Context: globals=user_globals, ) - def _resolve_command_from_args( - self, args: argparse.Namespace - ) -> tuple[CommandDef | LazyCommandDef | None, str]: - """Walk the parsed args to find the leaf command.""" + def _format_root_help(self) -> None: + """Render root help from command/group registries.""" + from milo.help import HelpState + from milo.templates import get_env + + commands = tuple( + [ + {"name": cmd.name, "help": getattr(cmd, "description", "")} + for cmd in self._commands.values() + if not getattr(cmd, "hidden", False) + ] + + [ + {"name": g.name, "help": g.description} + for g in self._groups.values() + if not g.hidden + ] + ) + + # Built-in options + builtin_opts = [ + {"flags": "-h, --help", "help": "Show this help message and exit"}, + {"flags": "-v, --verbose", "help": "Increase verbosity (-v verbose, -vv debug)"}, + {"flags": "-q, --quiet", "help": "Suppress non-error output"}, + {"flags": "--no-color", "help": "Disable color output"}, + {"flags": "-n, --dry-run", "help": "Show what would happen without making changes"}, + {"flags": "-o, --output-file", "metavar": "FILE", "help": "Write output to FILE instead of stdout"}, + ] + + if self.version: + builtin_opts.insert(0, {"flags": "--version", "help": f"Show version ({self.version})"}) + + # User-defined global options + for opt in self._global_options: + flags = f"--{opt.name.replace('_', '-')}" + if opt.short: + flags = f"{opt.short}, {flags}" + entry: dict[str, Any] = {"flags": flags, "help": opt.description} + if opt.default and not opt.is_flag: + entry["default"] = str(opt.default) + builtin_opts.append(entry) + + state = HelpState( + prog=self.name, + description=self.description, + commands=commands, + options=tuple(builtin_opts), + ) + env = get_env() + template = env.get_template("help.kida") + sys.stdout.write(template.render(state=state) + "\n") + sys.stdout.flush() + + def _resolve_command_from_args(self, args: argparse.Namespace) -> ResolveResult: + """Walk the parsed args to find the leaf command, group, or nothing.""" fmt = getattr(args, "format", "plain") - # Check top-level command cmd_name = getattr(args, "_command", None) if not cmd_name: - return None, fmt + return ResolvedNothing(attempted=None, fmt=fmt) - # Is it a direct command? + # Direct command? cmd = self.get_command(cmd_name) if cmd: - return cmd, fmt + return ResolvedCommand(command=cmd, fmt=fmt) - # Is it a group? Walk into it. + # Group? Walk into it. group = self._groups.get(cmd_name) if group is None: resolved = self._group_alias_map.get(cmd_name) if resolved: group = self._groups.get(resolved) if group is None: - return None, fmt + return ResolvedNothing(attempted=cmd_name, fmt=fmt) return self._resolve_group_command(group, args, fmt) @@ -946,23 +1035,21 @@ def _resolve_group_command( group: Group, args: argparse.Namespace, fmt: str, - ) -> tuple[CommandDef | None, str]: + ) -> ResolveResult: """Recursively resolve a command within a group from parsed args.""" sub_name = getattr(args, f"_command_{group.name}", None) if not sub_name: - return None, fmt + return ResolvedGroup(group=group, fmt=fmt) - # Check if it's a command in this group cmd = group.get_command(sub_name) if cmd: - return cmd, fmt + return ResolvedCommand(command=cmd, fmt=fmt) - # Check if it's a nested sub-group sub_group = group.get_group(sub_name) if sub_group: return self._resolve_group_command(sub_group, args, fmt) - return None, fmt + return ResolvedNothing(attempted=sub_name, fmt=fmt) def call(self, command_name: str, **kwargs: Any) -> Any: """Programmatically call a command by name or dotted path. diff --git a/src/milo/groups.py b/src/milo/groups.py index 5558ae5..094aba6 100644 --- a/src/milo/groups.py +++ b/src/milo/groups.py @@ -181,6 +181,38 @@ def to_def(self) -> GroupDef: hidden=self.hidden, ) + def format_help(self, prog_prefix: str = "") -> str: + """Render help from this group's command/group registries. + + Returns the rendered help string. Does not print. + """ + import sys + + from milo.help import HelpState + from milo.templates import get_env + + prog = f"{prog_prefix} {self.name}".strip() if prog_prefix else self.name + commands = tuple( + [ + {"name": cmd.name, "help": getattr(cmd, "description", "")} + for cmd in self._commands.values() + if not getattr(cmd, "hidden", False) + ] + + [ + {"name": g.name, "help": g.description} + for g in self._groups.values() + if not g.hidden + ] + ) + + state = HelpState(prog=prog, description=self.description, commands=commands) + env = get_env() + template = env.get_template("help.kida") + output = template.render(state=state) + sys.stdout.write(output + "\n") + sys.stdout.flush() + return output + def walk_commands(self, prefix: str = ""): """Yield (dotted_path, CommandDef) for all commands in this tree.""" path_prefix = f"{prefix}{self.name}." if prefix else f"{self.name}." diff --git a/src/milo/help.py b/src/milo/help.py index 6abc7e5..5cd114b 100644 --- a/src/milo/help.py +++ b/src/milo/help.py @@ -17,6 +17,8 @@ class HelpState: usage: str = "" groups: tuple[dict[str, Any], ...] = () examples: tuple[dict[str, Any], ...] = () + commands: tuple[dict[str, Any], ...] = () + options: tuple[dict[str, Any], ...] = () class HelpRenderer(argparse.HelpFormatter): diff --git a/src/milo/templates/help.kida b/src/milo/templates/help.kida index c3d40c1..1785f5d 100644 --- a/src/milo/templates/help.kida +++ b/src/milo/templates/help.kida @@ -1,13 +1,30 @@ {{ state.prog | bold }}{% if state.description %} {{ "-" | dim }} {{ state.description }}{% endif %} +{%- if state.commands %} + +{{ "commands" | yellow | bold }}: +{%- for cmd in state.commands %} + {{ cmd.name | cyan }} {{ cmd.help }} +{%- endfor %} +{%- endif %} +{%- for group in state.groups %} -{% for group in state.groups %} {{ group.title | yellow | bold }}: -{% for action in group.actions %} {% if action.option_strings %}{{ action.option_strings | join(", ") | green }}{% else %}{{ action.dest | cyan }}{% endif %}{% if action.metavar %} {{ action.metavar | dim }}{% endif %} {{ action.help }}{% if action.default is not none and action.default != "==SUPPRESS==" %} {{ ("(default: " ~ action.default ~ ")") | dim }}{% endif %} -{% endfor %} -{% endfor %} -{% if state.examples %} +{%- for action in group.actions %} + {% if action.option_strings %}{{ action.option_strings | join(", ") | green }}{% if action.metavar %} {{ action.metavar | dim }}{% endif %} {{ action.help }}{% if action.default is not none and action.default != "==SUPPRESS==" and action.default is not false and action.default != 0 and action.default != "" %} {{ ("(default: " ~ action.default ~ ")") | dim }}{% endif %}{% else %}{{ action.dest | cyan }}{% if action.metavar %} {{ action.metavar | dim }}{% endif %} {{ action.help }}{% if action.default is not none and action.default != "==SUPPRESS==" and action.default is not false and action.default != 0 and action.default != "" %} {{ ("(default: " ~ action.default ~ ")") | dim }}{% endif %}{% endif %} +{%- endfor %} +{%- endfor %} +{%- if state.options %} + +{{ "options" | yellow | bold }}: +{%- for opt in state.options %} + {{ opt.flags | green }}{% if opt.metavar %} {{ opt.metavar | dim }}{% endif %} {{ opt.help }}{% if opt.default %} {{ ("(default: " ~ opt.default ~ ")") | dim }}{% endif %} +{%- endfor %} +{%- endif %} +{%- if state.examples %} + {{ "examples" | yellow | bold }}: -{% for ex in state.examples %} {{ ("$ " ~ ex.command) | green }}{% if ex.description %} +{%- for ex in state.examples %} + {{ ("$ " ~ ex.command) | green }}{% if ex.description %} {{ ex.description | dim }}{% endif %} -{% endfor %} -{% endif %} +{%- endfor %} +{%- endif %} diff --git a/tests/test_groups.py b/tests/test_groups.py index 9d9dadc..176e978 100644 --- a/tests/test_groups.py +++ b/tests/test_groups.py @@ -327,3 +327,142 @@ def test_top_level_commands_still_shown(self): txt = generate_llms_txt(cli) assert "## Commands" in txt assert "**init**" in txt + + +# --------------------------------------------------------------------------- +# ResolveResult types and dispatch +# --------------------------------------------------------------------------- + + +class TestResolveResult: + """Verify _resolve_command_from_args returns correct discriminated types.""" + + def test_resolves_command(self): + from milo.commands import ResolvedCommand + + cli = _make_cli_with_groups() + parser = cli.build_parser() + args = parser.parse_args(["init", "--name", "x"]) + result = cli._resolve_command_from_args(args) + assert isinstance(result, ResolvedCommand) + assert result.command.name == "init" + + def test_resolves_group(self): + from milo.commands import ResolvedGroup + + cli = _make_cli_with_groups() + parser = cli.build_parser() + args = parser.parse_args(["site"]) + result = cli._resolve_command_from_args(args) + assert isinstance(result, ResolvedGroup) + assert result.group.name == "site" + + def test_resolves_nested_group(self): + from milo.commands import ResolvedGroup + + cli = _make_cli_with_groups() + parser = cli.build_parser() + args = parser.parse_args(["site", "config"]) + result = cli._resolve_command_from_args(args) + assert isinstance(result, ResolvedGroup) + assert result.group.name == "config" + + def test_resolves_nested_command(self): + from milo.commands import ResolvedCommand + + cli = _make_cli_with_groups() + parser = cli.build_parser() + args = parser.parse_args(["site", "build"]) + result = cli._resolve_command_from_args(args) + assert isinstance(result, ResolvedCommand) + assert result.command.name == "build" + + def test_resolves_nothing_no_args(self): + from milo.commands import ResolvedNothing + + cli = _make_cli_with_groups() + parser = cli.build_parser() + args = parser.parse_args([]) + result = cli._resolve_command_from_args(args) + assert isinstance(result, ResolvedNothing) + assert result.attempted is None + + def test_resolves_nothing_preserves_fmt(self): + from milo.commands import ResolvedNothing + + cli = _make_cli_with_groups() + parser = cli.build_parser() + args = parser.parse_args([]) + result = cli._resolve_command_from_args(args) + assert isinstance(result, ResolvedNothing) + assert result.fmt == "plain" + + +# --------------------------------------------------------------------------- +# Group bare invocation and help rendering +# --------------------------------------------------------------------------- + + +class TestGroupBareInvocation: + def test_group_bare_shows_help_not_error(self): + cli = _make_cli_with_groups() + result = cli.invoke(["site"]) + assert result.exit_code == 0 + assert result.stderr == "" + assert "build" in result.output + assert "serve" in result.output + + def test_nested_group_bare_shows_help(self): + cli = _make_cli_with_groups() + result = cli.invoke(["site", "config"]) + assert result.exit_code == 0 + assert result.stderr == "" + assert "show" in result.output + assert "validate" in result.output + + def test_unknown_command_still_suggests(self): + cli = _make_cli_with_groups() + result = cli.invoke(["siet"]) + assert result.exit_code == 2 + assert "Did you mean" in result.stderr + + def test_root_no_args_shows_help(self): + cli = _make_cli_with_groups() + result = cli.invoke([]) + assert result.exit_code == 0 + assert "bengal" in result.output + + +class TestHelpSubcommandListing: + def test_root_help_lists_commands_by_name(self): + cli = _make_cli_with_groups() + result = cli.invoke([]) + assert "init" in result.output + assert "site" in result.output + assert "_command" not in result.output + + def test_group_help_lists_subcommands_by_name(self): + cli = _make_cli_with_groups() + result = cli.invoke(["site"]) + assert "build" in result.output + assert "serve" in result.output + assert "_command_site" not in result.output + + def test_nested_group_help_lists_subcommands(self): + cli = _make_cli_with_groups() + result = cli.invoke(["site", "config"]) + assert "show" in result.output + assert "validate" in result.output + assert "_command_config" not in result.output + + def test_root_help_shows_options(self): + cli = _make_cli_with_groups() + result = cli.invoke([]) + assert "--verbose" in result.output + assert "--help" in result.output + + def test_group_help_no_argparse_internals(self): + """Group help should use registry data, not argparse positional args.""" + cli = _make_cli_with_groups() + result = cli.invoke(["site"]) + assert "positional arguments" not in result.output From 43144fc48d64d63aa68d3414ffb56ed8c723826f Mon Sep 17 00:00:00 2001 From: Lawrence Lane Date: Fri, 10 Apr 2026 16:06:06 -0400 Subject: [PATCH 2/3] Fix ruff formatting and add changelog fragment Co-Authored-By: Claude Opus 4.6 --- changelog.d/+group-help-fix.fixed.md | 1 + src/milo/commands.py | 9 ++++++--- 2 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 changelog.d/+group-help-fix.fixed.md diff --git a/changelog.d/+group-help-fix.fixed.md b/changelog.d/+group-help-fix.fixed.md new file mode 100644 index 0000000..ddc90e4 --- /dev/null +++ b/changelog.d/+group-help-fix.fixed.md @@ -0,0 +1 @@ +Fixed group bare invocation showing "Unknown command" instead of group help, and help output now lists subcommands by name instead of raw argparse internals. diff --git a/src/milo/commands.py b/src/milo/commands.py index 5469599..4d8f47a 100644 --- a/src/milo/commands.py +++ b/src/milo/commands.py @@ -791,8 +791,7 @@ def run(self, argv: list[str] | None = None) -> Any: suggestion = self.suggest_command(result.attempted) if suggestion: sys.stderr.write( - f"Unknown command: {result.attempted!r}. " - f"Did you mean {suggestion!r}?\n" + f"Unknown command: {result.attempted!r}. Did you mean {suggestion!r}?\n" ) return None self._format_root_help() @@ -979,7 +978,11 @@ def _format_root_help(self) -> None: {"flags": "-q, --quiet", "help": "Suppress non-error output"}, {"flags": "--no-color", "help": "Disable color output"}, {"flags": "-n, --dry-run", "help": "Show what would happen without making changes"}, - {"flags": "-o, --output-file", "metavar": "FILE", "help": "Write output to FILE instead of stdout"}, + { + "flags": "-o, --output-file", + "metavar": "FILE", + "help": "Write output to FILE instead of stdout", + }, ] if self.version: From f5cd7b660c5c112c575558e6a86a1868cbd3bf42 Mon Sep 17 00:00:00 2001 From: Lawrence Lane Date: Fri, 10 Apr 2026 16:21:48 -0400 Subject: [PATCH 3/3] Address PR review: nested prog prefix, parser-derived options, docstring fix - Add prog field to ResolvedGroup and accumulate full path through group resolution so nested groups render correct prog prefix (e.g. "bengal site config") - Replace hardcoded built-in options in _format_root_help() with parser-derived options so new flags like --llms-txt, --mcp are never missed - Fix Group.format_help() docstring to accurately reflect stdout writing Co-Authored-By: Claude Opus 4.6 --- src/milo/commands.py | 54 ++++++++++++++++++-------------------------- src/milo/groups.py | 2 +- 2 files changed, 23 insertions(+), 33 deletions(-) diff --git a/src/milo/commands.py b/src/milo/commands.py index 4d8f47a..ee48d33 100644 --- a/src/milo/commands.py +++ b/src/milo/commands.py @@ -61,6 +61,7 @@ class ResolvedGroup: group: Group fmt: str + prog: str = "" @dataclass(frozen=True, slots=True) @@ -743,6 +744,7 @@ def _add_arguments_from_schema( def run(self, argv: list[str] | None = None) -> Any: """Parse args and dispatch to the appropriate command.""" parser = self.build_parser() + self._parser = parser args = parser.parse_args(argv) # --completions mode @@ -783,7 +785,7 @@ def run(self, argv: list[str] | None = None) -> Any: result = self._resolve_command_from_args(args) if isinstance(result, ResolvedGroup): - result.group.format_help(self.name) + result.group.format_help(result.prog) return None if isinstance(result, ResolvedNothing): @@ -954,7 +956,7 @@ def _build_context(self, args: argparse.Namespace) -> Context: ) def _format_root_help(self) -> None: - """Render root help from command/group registries.""" + """Render root help from command/group registries and the actual parser.""" from milo.help import HelpState from milo.templates import get_env @@ -971,38 +973,24 @@ def _format_root_help(self) -> None: ] ) - # Built-in options - builtin_opts = [ - {"flags": "-h, --help", "help": "Show this help message and exit"}, - {"flags": "-v, --verbose", "help": "Increase verbosity (-v verbose, -vv debug)"}, - {"flags": "-q, --quiet", "help": "Suppress non-error output"}, - {"flags": "--no-color", "help": "Disable color output"}, - {"flags": "-n, --dry-run", "help": "Show what would happen without making changes"}, - { - "flags": "-o, --output-file", - "metavar": "FILE", - "help": "Write output to FILE instead of stdout", - }, - ] - - if self.version: - builtin_opts.insert(0, {"flags": "--version", "help": f"Show version ({self.version})"}) - - # User-defined global options - for opt in self._global_options: - flags = f"--{opt.name.replace('_', '-')}" - if opt.short: - flags = f"{opt.short}, {flags}" - entry: dict[str, Any] = {"flags": flags, "help": opt.description} - if opt.default and not opt.is_flag: - entry["default"] = str(opt.default) - builtin_opts.append(entry) + # Derive options from the actual parser so new flags are never missed + options: list[dict[str, Any]] = [] + for action in self._parser._actions: + if isinstance(action, argparse._SubParsersAction): + continue + flags = ", ".join(action.option_strings) if action.option_strings else "" + if not flags: + continue + entry: dict[str, Any] = {"flags": flags, "help": action.help or ""} + if action.metavar: + entry["metavar"] = action.metavar + options.append(entry) state = HelpState( prog=self.name, description=self.description, commands=commands, - options=tuple(builtin_opts), + options=tuple(options), ) env = get_env() template = env.get_template("help.kida") @@ -1031,18 +1019,20 @@ def _resolve_command_from_args(self, args: argparse.Namespace) -> ResolveResult: if group is None: return ResolvedNothing(attempted=cmd_name, fmt=fmt) - return self._resolve_group_command(group, args, fmt) + return self._resolve_group_command(group, args, fmt, prog=self.name) def _resolve_group_command( self, group: Group, args: argparse.Namespace, fmt: str, + prog: str = "", ) -> ResolveResult: """Recursively resolve a command within a group from parsed args.""" + group_prog = f"{prog} {group.name}".strip() sub_name = getattr(args, f"_command_{group.name}", None) if not sub_name: - return ResolvedGroup(group=group, fmt=fmt) + return ResolvedGroup(group=group, fmt=fmt, prog=group_prog) cmd = group.get_command(sub_name) if cmd: @@ -1050,7 +1040,7 @@ def _resolve_group_command( sub_group = group.get_group(sub_name) if sub_group: - return self._resolve_group_command(sub_group, args, fmt) + return self._resolve_group_command(sub_group, args, fmt, prog=group_prog) return ResolvedNothing(attempted=sub_name, fmt=fmt) diff --git a/src/milo/groups.py b/src/milo/groups.py index 094aba6..6aad7f3 100644 --- a/src/milo/groups.py +++ b/src/milo/groups.py @@ -184,7 +184,7 @@ def to_def(self) -> GroupDef: def format_help(self, prog_prefix: str = "") -> str: """Render help from this group's command/group registries. - Returns the rendered help string. Does not print. + Writes rendered help to stdout and returns the output string. """ import sys