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/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/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 ee48d33..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": @@ -725,9 +736,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": @@ -879,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 7310397..3bc8bc0 100644 --- a/src/milo/schema.py +++ b/src/milo/schema.py @@ -161,6 +161,10 @@ 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) + ): + 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..f721967 100644 --- a/tests/test_lazy.py +++ b/tests/test_lazy.py @@ -296,6 +296,157 @@ 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!" + + 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 enum import Enum + from pathlib import Path + + from milo.schema import function_to_schema + + 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) + # Non-serializable defaults should NOT be in the schema + assert "default" not in schema["properties"]["output"] + assert "default" not in schema["properties"]["color"] + + +# --------------------------------------------------------------------------- +# 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 # ---------------------------------------------------------------------------