From 82f9ace7a5f257a58b4f6309f939e93e9558cc0a Mon Sep 17 00:00:00 2001 From: Lawrence Lane Date: Fri, 10 Apr 2026 17:12:30 -0400 Subject: [PATCH 1/4] Fix lazy commands not propagating function signature defaults to argparse function_to_schema() tracked which params had defaults but never stored the actual values. _add_arguments_from_schema() only read defaults via inspect.signature() for eager commands. This caused lazy command parameters to resolve to None instead of their declared defaults. Store default values in the JSON schema and fall back to schema defaults when the function signature is unavailable. Co-Authored-By: Claude Opus 4.6 --- src/milo/commands.py | 4 ++- src/milo/schema.py | 2 ++ tests/test_lazy.py | 60 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/src/milo/commands.py b/src/milo/commands.py index ee48d33..9107d4e 100644 --- a/src/milo/commands.py +++ b/src/milo/commands.py @@ -725,9 +725,11 @@ def _add_arguments_from_schema( else: kwargs["type"] = str - # Set default from signature if available + # Set default from signature or schema if param and param.default is not inspect.Parameter.empty and json_type != "boolean": kwargs["default"] = param.default + elif "default" not in kwargs and "default" in param_schema: + kwargs["default"] = param_schema["default"] # Required vs optional if param_name in required_set and json_type != "boolean": diff --git a/src/milo/schema.py b/src/milo/schema.py index 7310397..dc447e1 100644 --- a/src/milo/schema.py +++ b/src/milo/schema.py @@ -161,6 +161,8 @@ def function_to_schema(func: Callable[..., Any]) -> dict[str, Any]: properties[name] = prop has_default = param.default is not inspect.Parameter.empty + if has_default: + prop["default"] = param.default if not has_default and not is_optional: required.append(name) diff --git a/tests/test_lazy.py b/tests/test_lazy.py index 5f599b9..6d5b9ad 100644 --- a/tests/test_lazy.py +++ b/tests/test_lazy.py @@ -296,6 +296,66 @@ def test_lazy_in_group_run(self): assert result == 30 +# --------------------------------------------------------------------------- +# Lazy commands: default value propagation +# --------------------------------------------------------------------------- + + +class TestLazyDefaults: + def test_lazy_command_uses_signature_defaults(self): + """Lazy commands should use function defaults when args are omitted.""" + cli = CLI(name="app") + cli.lazy_command( + "add", + "_lazy_handlers:add", + description="Add numbers", + ) + # b has default=0 in the handler; omitting --b should use 0, not None + result = cli.run(["add", "--a", "5"]) + assert result == 5 + + def test_lazy_command_precomputed_schema_with_defaults(self): + """Pre-computed schemas with 'default' fields should propagate.""" + cli = CLI(name="app") + cli.lazy_command( + "add", + "_lazy_handlers:add", + description="Add numbers", + schema={ + "type": "object", + "properties": { + "a": {"type": "integer"}, + "b": {"type": "integer", "default": 0}, + }, + "required": ["a"], + }, + ) + result = cli.run(["add", "--a", "5"]) + assert result == 5 + + def test_lazy_command_bool_default_false(self): + """Boolean defaults should work for lazy commands.""" + cli = CLI(name="app") + cli.lazy_command( + "greet", + "_lazy_handlers:greet", + description="Say hello", + ) + result = cli.run(["greet", "--name", "World"]) + assert result == "Hello, World!" + + def test_lazy_command_bool_default_override(self): + """Boolean flags should be overridable for lazy commands.""" + cli = CLI(name="app") + cli.lazy_command( + "greet", + "_lazy_handlers:greet", + description="Say hello", + ) + result = cli.run(["greet", "--name", "World", "--loud"]) + assert result == "HELLO, WORLD!" + + # --------------------------------------------------------------------------- # MCP with lazy commands # --------------------------------------------------------------------------- From 7e11f2718c90174aa85b5a71690699cd9fca5ea0 Mon Sep 17 00:00:00 2001 From: Lawrence Lane Date: Fri, 10 Apr 2026 21:30:56 -0400 Subject: [PATCH 2/4] Add JSON serialization guard, boolean schema fallback, and display_result option Three improvements on top of the lazy defaults fix: 1. schema.py: Only store JSON-safe types (str, int, float, bool, None, list, dict) as defaults in schema. Non-serializable defaults (Path, Enum) are omitted, preventing json.dumps() crashes in MCP mode. 2. commands.py: Boolean params now read schema "default" as fallback before defaulting to False, closing the gap where pre-computed schemas with "default": true were silently ignored. 3. CommandDef/LazyCommandDef: Add display_result option. When False, plain- format stdout output is suppressed while --format json and --output still work. Commands can return structured data without dumping ugly dicts in plain mode. Co-Authored-By: Claude Opus 4.6 --- src/milo/_command_defs.py | 8 ++++ src/milo/commands.py | 39 ++++++++++++------ src/milo/groups.py | 4 ++ src/milo/schema.py | 2 +- tests/test_lazy.py | 85 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 13 deletions(-) diff --git a/src/milo/_command_defs.py b/src/milo/_command_defs.py index cae1c31..94b9626 100644 --- a/src/milo/_command_defs.py +++ b/src/milo/_command_defs.py @@ -59,6 +59,8 @@ class CommandDef: """If non-empty, prompt for confirmation before running.""" annotations: dict[str, Any] = field(default_factory=dict) """MCP tool annotations (readOnlyHint, destructiveHint, etc.).""" + display_result: bool = True + """If False, suppress plain-format output (return value still available for --format json).""" class LazyCommandDef: @@ -80,6 +82,7 @@ class LazyCommandDef: "annotations", "confirm", "description", + "display_result", "examples", "hidden", "import_path", @@ -100,6 +103,7 @@ def __init__( examples: tuple[dict[str, Any], ...] | list[dict[str, Any]] = (), confirm: str = "", annotations: dict[str, Any] | None = None, + display_result: bool = True, ) -> None: self.name = name self.description = description @@ -110,6 +114,7 @@ def __init__( self.examples = tuple(examples) self.confirm = confirm self.annotations = annotations or {} + self.display_result = display_result self._schema = schema self._resolved: CommandDef | None = None self._lock = threading.Lock() @@ -159,6 +164,7 @@ def resolve(self) -> CommandDef: examples=self.examples, confirm=self.confirm, annotations=self.annotations, + display_result=self.display_result, ) return self._resolved @@ -185,6 +191,7 @@ def _make_command_def( examples: tuple[dict[str, Any], ...] = (), confirm: str = "", annotations: dict[str, Any] | None = None, + display_result: bool = True, ) -> CommandDef: """Build a CommandDef from a function and decorator kwargs.""" from milo.schema import function_to_schema @@ -204,6 +211,7 @@ def _make_command_def( examples=examples, confirm=confirm, annotations=annotations or {}, + display_result=display_result, ) diff --git a/src/milo/commands.py b/src/milo/commands.py index 9107d4e..9b553d6 100644 --- a/src/milo/commands.py +++ b/src/milo/commands.py @@ -220,6 +220,7 @@ def command( examples: tuple[dict[str, Any], ...] | list[dict[str, Any]] = (), confirm: str = "", annotations: dict[str, Any] | None = None, + display_result: bool = True, ) -> Callable: """Register a function as a CLI command. @@ -232,6 +233,8 @@ def command( confirm: If set, prompt user with this message before executing. annotations: MCP tool annotations (readOnlyHint, destructiveHint, idempotentHint, openWorldHint). + display_result: If False, suppress plain-format output while still + returning data for ``--format json`` or ``--output``. """ def decorator(func: Callable[..., Any]) -> Callable[..., Any]: @@ -245,6 +248,7 @@ def decorator(func: Callable[..., Any]) -> Callable[..., Any]: examples=tuple(examples), confirm=confirm, annotations=annotations, + display_result=display_result, ) self._commands[name] = cmd for alias in aliases: @@ -267,11 +271,16 @@ def lazy_command( examples: tuple[dict[str, Any], ...] | list[dict[str, Any]] = (), confirm: str = "", annotations: dict[str, Any] | None = None, + display_result: bool = True, ) -> LazyCommandDef: """Register a lazy-loaded command. The handler module is not imported until the command is invoked. This keeps CLI startup fast for large command sets. + + When providing a pre-computed *schema*, include ``"default"`` fields + in properties for optional parameters so argparse receives the correct + defaults without importing the handler module. """ cmd = LazyCommandDef( name=name, @@ -284,6 +293,7 @@ def lazy_command( examples=examples, confirm=confirm, annotations=annotations, + display_result=display_result, ) self._commands[name] = cmd for alias in aliases: @@ -704,11 +714,12 @@ def _add_arguments_from_schema( # Determine type json_type = param_schema.get("type", "string") if json_type == "boolean": - default = ( - param.default - if param and param.default is not inspect.Parameter.empty - else False - ) + if param and param.default is not inspect.Parameter.empty: + default = param.default + elif "default" in param_schema: + default = param_schema["default"] + else: + default = False kwargs["action"] = "store_true" kwargs["default"] = default elif json_type == "integer": @@ -881,13 +892,17 @@ def run(self, argv: list[str] | None = None) -> Any: ctx.error(f"after_command hook failed: {type(exc).__name__}: {exc}") # Format and output (to file or stdout) - output_file = ctx.output_file - if output_file: - formatted = format_output(result, fmt=fmt) - with open(output_file, "w") as f: - f.write(formatted + "\n") - else: - write_output(result, fmt=fmt) + # When display_result=False, suppress plain-format stdout output but + # still honor explicit --format or --output requests. + suppress = not cmd.display_result and fmt == "plain" and not ctx.output_file + if not suppress: + output_file = ctx.output_file + if output_file: + formatted = format_output(result, fmt=fmt) + with open(output_file, "w") as f: + f.write(formatted + "\n") + else: + write_output(result, fmt=fmt) return result diff --git a/src/milo/groups.py b/src/milo/groups.py index 6aad7f3..76e648f 100644 --- a/src/milo/groups.py +++ b/src/milo/groups.py @@ -67,6 +67,7 @@ def command( hidden: bool = False, examples: tuple[dict[str, Any], ...] | list[dict[str, Any]] = (), confirm: str = "", + display_result: bool = True, ) -> Callable: """Register a function as a command within this group.""" from milo.commands import _make_command_def @@ -81,6 +82,7 @@ def decorator(func: Callable[..., Any]) -> Callable[..., Any]: hidden=hidden, examples=tuple(examples), confirm=confirm, + display_result=display_result, ) self._commands[name] = cmd for alias in aliases: @@ -100,6 +102,7 @@ def lazy_command( aliases: tuple[str, ...] | list[str] = (), tags: tuple[str, ...] | list[str] = (), hidden: bool = False, + display_result: bool = True, ) -> Any: """Register a lazy-loaded command within this group. @@ -115,6 +118,7 @@ def lazy_command( aliases=aliases, tags=tags, hidden=hidden, + display_result=display_result, ) self._commands[name] = cmd for alias in aliases: diff --git a/src/milo/schema.py b/src/milo/schema.py index dc447e1..82edaac 100644 --- a/src/milo/schema.py +++ b/src/milo/schema.py @@ -161,7 +161,7 @@ def function_to_schema(func: Callable[..., Any]) -> dict[str, Any]: properties[name] = prop has_default = param.default is not inspect.Parameter.empty - if has_default: + if has_default and isinstance(param.default, (str, int, float, bool, type(None), list, dict)): prop["default"] = param.default if not has_default and not is_optional: required.append(name) diff --git a/tests/test_lazy.py b/tests/test_lazy.py index 6d5b9ad..745eb9d 100644 --- a/tests/test_lazy.py +++ b/tests/test_lazy.py @@ -355,6 +355,91 @@ def test_lazy_command_bool_default_override(self): result = cli.run(["greet", "--name", "World", "--loud"]) assert result == "HELLO, WORLD!" + def test_schema_defaults_are_json_serializable(self): + """function_to_schema() should only store JSON-safe defaults.""" + import json + + from milo.schema import function_to_schema + + def handler(name: str, count: int = 5, flag: bool = True) -> str: + return "" + + schema = function_to_schema(handler) + # Should not raise + json.dumps(schema) + assert schema["properties"]["count"]["default"] == 5 + assert schema["properties"]["flag"]["default"] is True + + def test_schema_omits_non_serializable_defaults(self): + """Non-JSON-serializable defaults should be omitted from schema.""" + import json + from pathlib import Path + + from milo.schema import function_to_schema + + def handler(output: Path = Path(".")) -> str: + return "" + + schema = function_to_schema(handler) + # Should not raise + json.dumps(schema) + # Path default should NOT be in the schema + assert "default" not in schema["properties"]["output"] + + +# --------------------------------------------------------------------------- +# display_result suppression +# --------------------------------------------------------------------------- + + +class TestDisplayResult: + def test_display_result_false_suppresses_plain(self): + """display_result=False suppresses plain stdout output.""" + cli = CLI(name="app") + + @cli.command("info", display_result=False) + def info() -> dict: + return {"status": "ok", "count": 42} + + result = cli.invoke(["info"]) + assert result.output == "" + assert result.result == {"status": "ok", "count": 42} + + def test_display_result_false_allows_json(self): + """display_result=False still outputs with --format json.""" + cli = CLI(name="app") + + @cli.command("info", display_result=False) + def info() -> dict: + return {"status": "ok"} + + result = cli.invoke(["info", "--format", "json"]) + assert '"status"' in result.output + + def test_display_result_true_default(self): + """By default, display_result=True and output is shown.""" + cli = CLI(name="app") + + @cli.command("info") + def info() -> str: + return "hello" + + result = cli.invoke(["info"]) + assert "hello" in result.output + + def test_lazy_display_result_false(self): + """Lazy commands support display_result=False.""" + cli = CLI(name="app") + cli.lazy_command( + "add", + "_lazy_handlers:add", + description="Add numbers", + display_result=False, + ) + result = cli.invoke(["add", "--a", "3", "--b", "7"]) + assert result.output == "" + assert result.result == 10 + # --------------------------------------------------------------------------- # MCP with lazy commands From ce2294f31a674f672f6d83934ecfd6b3c5f6e491 Mon Sep 17 00:00:00 2001 From: Lawrence Lane Date: Fri, 10 Apr 2026 21:32:19 -0400 Subject: [PATCH 3/4] Update lazyapp example schemas with defaults and add Enum serialization test Sprint 2 cleanup: add "default" fields to pre-computed schemas in the lazyapp example so they match handler signatures. Extend serialization test to cover Enum defaults. Co-Authored-By: Claude Opus 4.6 --- examples/lazyapp/app.py | 8 ++++---- tests/test_lazy.py | 10 ++++++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/examples/lazyapp/app.py b/examples/lazyapp/app.py index 68b30b9..3d1ff34 100644 --- a/examples/lazyapp/app.py +++ b/examples/lazyapp/app.py @@ -49,7 +49,7 @@ def status() -> dict: "type": "object", "properties": { "target": {"type": "string"}, - "dry_run": {"type": "boolean"}, + "dry_run": {"type": "boolean", "default": False}, }, "required": ["target"], }, @@ -63,7 +63,7 @@ def status() -> dict: "type": "object", "properties": { "target": {"type": "string"}, - "steps": {"type": "integer"}, + "steps": {"type": "integer", "default": 1}, }, "required": ["target"], }, @@ -77,8 +77,8 @@ def status() -> dict: schema={ "type": "object", "properties": { - "target": {"type": "string"}, - "lines": {"type": "integer"}, + "target": {"type": "string", "default": "production"}, + "lines": {"type": "integer", "default": 20}, }, }, ) diff --git a/tests/test_lazy.py b/tests/test_lazy.py index 745eb9d..f721967 100644 --- a/tests/test_lazy.py +++ b/tests/test_lazy.py @@ -373,18 +373,24 @@ def handler(name: str, count: int = 5, flag: bool = True) -> str: def test_schema_omits_non_serializable_defaults(self): """Non-JSON-serializable defaults should be omitted from schema.""" import json + from enum import Enum from pathlib import Path from milo.schema import function_to_schema - def handler(output: Path = Path(".")) -> str: + class Color(Enum): + RED = "red" + BLUE = "blue" + + def handler(output: Path = Path("."), color: Color = Color.RED) -> str: return "" schema = function_to_schema(handler) # Should not raise json.dumps(schema) - # Path default should NOT be in the schema + # Non-serializable defaults should NOT be in the schema assert "default" not in schema["properties"]["output"] + assert "default" not in schema["properties"]["color"] # --------------------------------------------------------------------------- From d929be6cfac3152ee3549604b52e0aec66e396c8 Mon Sep 17 00:00:00 2001 From: Lawrence Lane Date: Fri, 10 Apr 2026 22:26:29 -0400 Subject: [PATCH 4/4] Fix ruff formatting and add changelog fragment Co-Authored-By: Claude Opus 4.6 --- changelog.d/+lazy-cmd-defaults.fixed.md | 1 + src/milo/schema.py | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 changelog.d/+lazy-cmd-defaults.fixed.md diff --git a/changelog.d/+lazy-cmd-defaults.fixed.md b/changelog.d/+lazy-cmd-defaults.fixed.md new file mode 100644 index 0000000..12d75f9 --- /dev/null +++ b/changelog.d/+lazy-cmd-defaults.fixed.md @@ -0,0 +1 @@ +Lazy commands now propagate function signature defaults to argparse. Schema defaults are JSON-safe, boolean schema defaults are respected, and a new `display_result=False` option suppresses plain-format output while preserving `--format json`. diff --git a/src/milo/schema.py b/src/milo/schema.py index 82edaac..3bc8bc0 100644 --- a/src/milo/schema.py +++ b/src/milo/schema.py @@ -161,7 +161,9 @@ def function_to_schema(func: Callable[..., Any]) -> dict[str, Any]: properties[name] = prop has_default = param.default is not inspect.Parameter.empty - if has_default and isinstance(param.default, (str, int, float, bool, type(None), list, dict)): + if has_default and isinstance( + param.default, (str, int, float, bool, type(None), list, dict) + ): prop["default"] = param.default if not has_default and not is_optional: required.append(name)