diff --git a/packages/deepctl-cmd-login/src/deepctl_cmd_login/command.py b/packages/deepctl-cmd-login/src/deepctl_cmd_login/command.py index 2f3db58..1b4de0b 100644 --- a/packages/deepctl-cmd-login/src/deepctl_cmd_login/command.py +++ b/packages/deepctl-cmd-login/src/deepctl_cmd_login/command.py @@ -10,6 +10,7 @@ DeepgramClient, ProfileInfo, ProfilesResult, + get_output_format, ) from rich.console import Console from rich.prompt import Prompt @@ -751,6 +752,7 @@ def handle( # Determine key and its exact storage source api_key: str | None = None key_source = "not set" + env_key = os.environ.get("DEEPGRAM_API_KEY") try: api_key = _keyring.get_password(KEYRING_SERVICE, f"api-key.{profile_name}") @@ -760,14 +762,19 @@ def handle( pass if not api_key and profile_cfg.api_key: + # Config merges DEEPGRAM_API_KEY into the profile at load time + # (environment overrides the file), so a profile key equal to the + # environment value came from the environment, not the config + # file — label it accordingly instead of claiming "config file". api_key = profile_cfg.api_key - key_source = "config file" - - if not api_key: - env_key = os.environ.get("DEEPGRAM_API_KEY") - if env_key: - api_key = env_key + if env_key and profile_cfg.api_key == env_key: key_source = "DEEPGRAM_API_KEY (env)" + else: + key_source = "config file" + + if not api_key and env_key: + api_key = env_key + key_source = "DEEPGRAM_API_KEY (env)" authenticated = api_key is not None masked: str | None = None @@ -777,21 +784,25 @@ def handle( project_id = auth_manager.get_project_id() base_url = profile_cfg.base_url or "https://api.deepgram.com" - if not authenticated: - console.print( - "[yellow]Not logged in.[/yellow] Run 'dg login' to authenticate." - ) - else: - console.print("[green]✓[/green] Authenticated") - console.print(f" Profile: {profile_name}") - console.print(f" API Key: {masked} [dim]({key_source})[/dim]") - console.print( - f" Project ID: {project_id}" - if project_id - else " Project ID: [dim]not set[/dim]" - ) - if base_url != "https://api.deepgram.com": - console.print(f" Base URL: {base_url}") + # Render the human block only in default mode. For json/yaml/csv the + # framework serialises the returned result to stdout, so printing the + # block here would corrupt that output for piping (the #97 pattern). + if get_output_format() == "default": + if not authenticated: + console.print( + "[yellow]Not logged in.[/yellow] Run 'dg login' to authenticate." + ) + else: + console.print("[green]✓[/green] Authenticated") + console.print(f" Profile: {profile_name}") + console.print(f" API Key: {masked} [dim]({key_source})[/dim]") + console.print( + f" Project ID: {project_id}" + if project_id + else " Project ID: [dim]not set[/dim]" + ) + if base_url != "https://api.deepgram.com": + console.print(f" Base URL: {base_url}") return WhoamiResult( authenticated=authenticated, diff --git a/packages/deepctl-cmd-login/tests/unit/test_login_command.py b/packages/deepctl-cmd-login/tests/unit/test_login_command.py index 7634af7..fa0d53d 100644 --- a/packages/deepctl-cmd-login/tests/unit/test_login_command.py +++ b/packages/deepctl-cmd-login/tests/unit/test_login_command.py @@ -402,3 +402,76 @@ def test_proceeds_when_guided_and_tty(self): mock_stdout.isatty.return_value = True cmd._maybe_prompt_skills_setup() mock_detect.assert_called_once() + + +class TestWhoamiKeySource: + """The key-source label must name where the key actually came from. + + Config merges DEEPGRAM_API_KEY into the profile at load time, so the + profile's api_key being set does not prove the key came from the config + file. whoami used to label an environment-sourced key "config file". + """ + + @pytest.fixture + def whoami_command(self): + from deepctl_cmd_login.command import WhoamiCommand + + return WhoamiCommand() + + def _run(self, whoami_command, mock_config, mock_client, env, profile_key): + auth_manager = Mock(spec=AuthManager) + auth_manager.get_project_id.return_value = "proj-1" + mock_config.get_profile.return_value.api_key = profile_key + with ( + patch.dict("os.environ", env, clear=False), + patch("keyring.get_password", return_value=None), + ): + if "DEEPGRAM_API_KEY" not in env: + import os + + os.environ.pop("DEEPGRAM_API_KEY", None) + return whoami_command.handle( + config=mock_config, + auth_manager=auth_manager, + client=mock_client, + ) + + def test_env_key_merged_into_profile_labeled_env( + self, whoami_command, mock_config, mock_client + ): + """A profile key equal to DEEPGRAM_API_KEY is labeled as env.""" + result = self._run( + whoami_command, + mock_config, + mock_client, + env={"DEEPGRAM_API_KEY": "dg_env_key_12345"}, + profile_key="dg_env_key_12345", + ) + assert result.key_source == "DEEPGRAM_API_KEY (env)" + assert result.authenticated is True + + def test_real_config_file_key_still_labeled_config_file( + self, whoami_command, mock_config, mock_client + ): + """A profile key with no matching env var keeps the config label.""" + result = self._run( + whoami_command, + mock_config, + mock_client, + env={}, + profile_key="dg_file_key_67890", + ) + assert result.key_source == "config file" + + def test_env_key_without_profile_labeled_env( + self, whoami_command, mock_config, mock_client + ): + """No profile key, env set: the env fallback branch labels env.""" + result = self._run( + whoami_command, + mock_config, + mock_client, + env={"DEEPGRAM_API_KEY": "dg_env_only_11111"}, + profile_key=None, + ) + assert result.key_source == "DEEPGRAM_API_KEY (env)" diff --git a/packages/deepctl-cmd-models/src/deepctl_cmd_models/command.py b/packages/deepctl-cmd-models/src/deepctl_cmd_models/command.py index 610cbff..fe67321 100644 --- a/packages/deepctl-cmd-models/src/deepctl_cmd_models/command.py +++ b/packages/deepctl-cmd-models/src/deepctl_cmd_models/command.py @@ -23,6 +23,63 @@ # output that callers pipe into jq and friends. status_console = get_status_console() +# Models the API still lists but which are legacy. The catalog flags them so +# that nobody — human or coding agent — treats them as a current model family +# or composes new model names from them. (Paying accounts have requested the +# nonexistent "nova-3-conversational", a natural composition of "nova-3" and +# the legacy name below.) +DEPRECATED_MODELS: dict[str, str] = { + "conversationalai": ( + "Legacy model. For conversational audio use 'nova-3'. There is no " + "model named 'nova-3-conversational'." + ), + "2-conversationalai": ( + "Legacy model. For conversational audio use 'nova-3'. There is no " + "model named 'nova-3-conversational'." + ), +} + + +def _build_model_info(m: dict[str, Any], model_type: str) -> ModelInfo: + """Map one API model entry onto ModelInfo. + + The Deepgram Python SDK's generated response classes rename the API's + ``uuid`` field to ``uuid_`` (to avoid shadowing), so ``model_dump()`` + emits ``uuid_`` — both spellings are read here. The API also reports + ``languages`` as a list; the older singular ``language`` key is kept as + a fallback for compatibility. + """ + model_id = m.get("uuid_") or m.get("uuid") or m.get("model_id") or "" + + raw_languages = m.get("languages") or [] + if not isinstance(raw_languages, list): + raw_languages = [raw_languages] + languages = [str(lang) for lang in raw_languages] + primary_language = str(m.get("language") or (languages[0] if languages else "")) + + name = str(m.get("name") or "") + deprecation_note = DEPRECATED_MODELS.get(name.lower(), "") + + return ModelInfo( + model_id=str(model_id), + name=name, + canonical_name=str(m.get("canonical_name") or ""), + architecture=str(m.get("architecture") or ""), + version=str(m.get("version") or ""), + language=primary_language, + languages=languages, + model_type=model_type, + deprecated=bool(deprecation_note), + deprecation_note=deprecation_note, + ) + + +def _format_languages(languages: list[str], limit: int = 4) -> str: + """Join a language list for table display, truncating long lists.""" + if len(languages) <= limit: + return ", ".join(languages) + return ", ".join(languages[:limit]) + f" +{len(languages) - limit}" + class ModelsCommand(BaseCommand): """Command for listing available Deepgram models.""" @@ -44,7 +101,9 @@ class ModelsCommand(BaseCommand): agent_help = ( "List available Deepgram speech-to-text and text-to-speech models. " "Filter by type (stt/tts) and optionally include outdated versions. " - "Requires authentication." + "Use the canonical_name field as the `model` request parameter. " + "Entries flagged deprecated are legacy: do not use them or derive " + "new model names from them. Requires authentication." ) def get_arguments(self) -> list[dict[str, Any]]: @@ -76,33 +135,32 @@ def handle( try: result = client.list_models(include_outdated=include_outdated) - stt_models = result.get("stt", []) - tts_models = result.get("tts", []) + # `or []` also covers an explicit null in the response body. + stt_models = result.get("stt") or [] + tts_models = result.get("tts") or [] all_models: list[ModelInfo] = [] if model_type != "tts": for m in stt_models: - all_models.append( - ModelInfo( - model_id=m.get("uuid", m.get("model_id", "")), - name=m.get("name", ""), - version=m.get("version", ""), - language=m.get("language", ""), - model_type="stt", - ) + all_models.append(_build_model_info(m, "stt")) + if not stt_models: + status_console.print( + "[yellow]Warning: the API returned zero speech-to-text " + "models. Deepgram publishes speech-to-text models " + "(Nova-3, Flux), so an empty list usually means an API " + "or account problem, not an empty catalog.[/yellow]" ) if model_type != "stt": for m in tts_models: - all_models.append( - ModelInfo( - model_id=m.get("uuid", m.get("model_id", "")), - name=m.get("name", ""), - version=m.get("version", ""), - language=m.get("language", ""), - model_type="tts", - ) + all_models.append(_build_model_info(m, "tts")) + if not tts_models: + status_console.print( + "[yellow]Warning: the API returned zero text-to-speech " + "models. Deepgram publishes text-to-speech models " + "(Aura-2), so an empty list usually means an API or " + "account problem, not an empty catalog.[/yellow]" ) if not all_models: @@ -117,19 +175,34 @@ def handle( title="Deepgram Models", show_header=True, header_style="bold blue" ) table.add_column("Name", style="green") + table.add_column("Canonical name", style="green") table.add_column("Type", style="cyan") - table.add_column("Language") + table.add_column("Languages") table.add_column("Version") table.add_column("ID", style="dim") for m in all_models: + display_name = m.name + if m.deprecated: + display_name = f"{m.name} [yellow](deprecated)[/yellow]" table.add_row( - m.name, m.model_type.upper(), m.language, m.version, m.model_id + display_name, + m.canonical_name, + m.model_type.upper(), + _format_languages(m.languages), + m.version, + m.model_id, ) console.print(table) console.print(f"\n[dim]{len(all_models)} model(s) found[/dim]") + deprecated_notes = { + m.name: m.deprecation_note for m in all_models if m.deprecated + } + for name, note in sorted(deprecated_notes.items()): + console.print(f"[yellow]Deprecated:[/yellow] {name} — {note}") + return ModelsResult( status="success", models=all_models, diff --git a/packages/deepctl-cmd-models/src/deepctl_cmd_models/models.py b/packages/deepctl-cmd-models/src/deepctl_cmd_models/models.py index af41fa2..b11fa0d 100644 --- a/packages/deepctl-cmd-models/src/deepctl_cmd_models/models.py +++ b/packages/deepctl-cmd-models/src/deepctl_cmd_models/models.py @@ -9,9 +9,14 @@ class ModelInfo(BaseModel): model_id: str = "" name: str = "" + canonical_name: str = "" # the value to pass as the `model` parameter + architecture: str = "" version: str = "" - language: str = "" + language: str = "" # primary language (first entry of `languages`) + languages: list[str] = Field(default_factory=list) model_type: str = "" # "stt" or "tts" + deprecated: bool = False + deprecation_note: str = "" class ModelsResult(BaseResult): diff --git a/packages/deepctl-cmd-models/tests/unit/test_models_command.py b/packages/deepctl-cmd-models/tests/unit/test_models_command.py index de4192d..9e7c498 100644 --- a/packages/deepctl-cmd-models/tests/unit/test_models_command.py +++ b/packages/deepctl-cmd-models/tests/unit/test_models_command.py @@ -314,3 +314,224 @@ def test_default_mode_renders_table(self, mock_console, _fmt, command): ) assert mock_console.print.called + + +class TestFieldMapping: + """The SDK/API field-name fixes: uuid_ and languages. + + The Deepgram Python SDK's generated response classes rename the API's + `uuid` field to `uuid_`, and the API reports `languages` as a list. + The command used to read `uuid` and `language`, so every row rendered + with an empty ID and an empty language column. + """ + + @pytest.fixture + def command(self): + return ModelsCommand() + + @pytest.fixture + def mock_config(self): + return Mock(spec=Config) + + @pytest.fixture + def mock_auth_manager(self): + manager = Mock(spec=AuthManager) + manager.get_api_key.return_value = "test-api-key" + return manager + + @pytest.fixture + def mock_client(self): + return Mock(spec=DeepgramClient) + + def test_sdk_shape_uuid_underscore_and_languages_list( + self, command, mock_config, mock_auth_manager, mock_client + ): + """model_dump() output (uuid_, languages list) maps onto ModelInfo.""" + mock_client.list_models.return_value = { + "stt": [ + { + "uuid_": "6b28e919-8427-4f32-9847-492e2efd7daf", + "name": "nova-3", + "canonical_name": "nova-3-general", + "architecture": "nova-3", + "languages": ["en", "en-US"], + "version": "2025-01-01.0", + }, + ], + "tts": [ + { + "uuid_": "6fe3f8e3-14d3-456c-9534-766132310608", + "name": "agathe", + "canonical_name": "aura-2-agathe-fr", + "architecture": "aura-2", + "languages": ["fr", "fr-FR"], + "version": "2025-10-29.0", + }, + ], + } + + result = command.handle( + config=mock_config, + auth_manager=mock_auth_manager, + client=mock_client, + ) + + assert result.status == "success" + stt = result.models[0] + assert stt.model_id == "6b28e919-8427-4f32-9847-492e2efd7daf" + assert stt.canonical_name == "nova-3-general" + assert stt.architecture == "nova-3" + assert stt.language == "en" + assert stt.languages == ["en", "en-US"] + + tts = result.models[1] + assert tts.model_id == "6fe3f8e3-14d3-456c-9534-766132310608" + assert tts.canonical_name == "aura-2-agathe-fr" + assert tts.model_type == "tts" + + def test_legacy_shape_still_maps( + self, command, mock_config, mock_auth_manager, mock_client + ): + """The older keys (uuid, language) keep working as fallbacks.""" + mock_client.list_models.return_value = { + "stt": [ + { + "uuid": "legacy-uuid", + "name": "Nova-3", + "version": "1.0", + "language": "en", + }, + ], + "tts": [], + } + + result = command.handle( + config=mock_config, + auth_manager=mock_auth_manager, + client=mock_client, + type="stt", + ) + + assert result.status == "success" + assert result.models[0].model_id == "legacy-uuid" + assert result.models[0].language == "en" + + def test_null_category_does_not_crash( + self, command, mock_config, mock_auth_manager, mock_client + ): + """An explicit null for a category is treated as an empty list.""" + mock_client.list_models.return_value = { + "stt": [ + {"uuid_": "u1", "name": "nova-3", "languages": ["en"]}, + ], + "tts": None, + } + + result = command.handle( + config=mock_config, + auth_manager=mock_auth_manager, + client=mock_client, + ) + + assert result.status == "success" + assert result.count == 1 + + def test_empty_tts_prints_warning( + self, command, mock_config, mock_auth_manager, mock_client + ): + """Zero TTS models triggers a loud warning instead of silence. + + A catalog that silently shows only speech-to-text models led agents + to conclude Deepgram has no text-to-speech side. The warning makes + an empty category visible as an anomaly. + """ + mock_client.list_models.return_value = { + "stt": [ + {"uuid_": "u1", "name": "nova-3", "languages": ["en"]}, + ], + "tts": [], + } + + with patch("deepctl_cmd_models.command.status_console") as mock_status_console: + result = command.handle( + config=mock_config, + auth_manager=mock_auth_manager, + client=mock_client, + ) + + assert result.status == "success" + warnings = [str(call) for call in mock_status_console.print.call_args_list] + assert any("zero text-to-speech" in w for w in warnings) + + def test_stt_filter_suppresses_tts_warning( + self, command, mock_config, mock_auth_manager, mock_client + ): + """--type stt does not warn about the (unrequested) TTS category.""" + mock_client.list_models.return_value = { + "stt": [ + {"uuid_": "u1", "name": "nova-3", "languages": ["en"]}, + ], + "tts": [], + } + + with patch("deepctl_cmd_models.command.status_console") as mock_status_console: + result = command.handle( + config=mock_config, + auth_manager=mock_auth_manager, + client=mock_client, + type="stt", + ) + + assert result.status == "success" + warnings = [str(call) for call in mock_status_console.print.call_args_list] + assert not any("zero text-to-speech" in w for w in warnings) + + +class TestDeprecationFlags: + """Legacy models are flagged so nobody derives new names from them.""" + + @pytest.fixture + def command(self): + return ModelsCommand() + + @pytest.fixture + def mock_config(self): + return Mock(spec=Config) + + @pytest.fixture + def mock_auth_manager(self): + manager = Mock(spec=AuthManager) + manager.get_api_key.return_value = "test-api-key" + return manager + + @pytest.fixture + def mock_client(self): + return Mock(spec=DeepgramClient) + + @pytest.mark.parametrize("legacy_name", ["conversationalai", "2-conversationalai"]) + def test_conversationalai_flagged_deprecated( + self, command, mock_config, mock_auth_manager, mock_client, legacy_name + ): + mock_client.list_models.return_value = { + "stt": [ + {"uuid_": "u1", "name": legacy_name, "languages": ["en"]}, + {"uuid_": "u2", "name": "nova-3", "languages": ["en"]}, + ], + "tts": [], + } + + result = command.handle( + config=mock_config, + auth_manager=mock_auth_manager, + client=mock_client, + type="stt", + ) + + legacy = next(m for m in result.models if m.name == legacy_name) + assert legacy.deprecated is True + assert "nova-3" in legacy.deprecation_note + assert "nova-3-conversational" in legacy.deprecation_note + + current = next(m for m in result.models if m.name == "nova-3") + assert current.deprecated is False + assert current.deprecation_note == "" diff --git a/packages/deepctl-core/src/deepctl_core/__init__.py b/packages/deepctl-core/src/deepctl_core/__init__.py index 6a6f86d..9c6ff6a 100644 --- a/packages/deepctl-core/src/deepctl_core/__init__.py +++ b/packages/deepctl-core/src/deepctl_core/__init__.py @@ -24,6 +24,7 @@ print_success, print_warning, setup_output, + update_output, ) from .plugin_manager import PluginManager from .timing import ( @@ -63,4 +64,5 @@ "print_timing_summary", "print_warning", "setup_output", + "update_output", ] diff --git a/packages/deepctl-core/src/deepctl_core/output.py b/packages/deepctl-core/src/deepctl_core/output.py index f7469db..04bc2ee 100644 --- a/packages/deepctl-core/src/deepctl_core/output.py +++ b/packages/deepctl-core/src/deepctl_core/output.py @@ -103,6 +103,30 @@ def setup_output( console.quiet = quiet +def update_output( + format_type: str | None = None, + quiet: bool | None = None, + verbose: bool | None = None, +) -> None: + """Update parts of the global output configuration in place. + + Unlike setup_output(), only the arguments actually provided change; + every other setting keeps its current value. The per-command + pass-through options use this so that ``dg models -o json`` has the + same effect as ``dg -o json models`` without resetting the quiet or + verbose state the root group already applied. + """ + if format_type is not None: + if format_type == "default" and _output_config["agentic"]: + format_type = "json" + _output_config["format"] = format_type + if quiet is not None: + _output_config["quiet"] = quiet + console.quiet = quiet + if verbose is not None: + _output_config["verbose"] = verbose + + class OutputFormatter: """Formatter for different output types.""" diff --git a/packages/deepctl-core/src/deepctl_core/plugin_manager.py b/packages/deepctl-core/src/deepctl_core/plugin_manager.py index e25628c..4824dbb 100644 --- a/packages/deepctl-core/src/deepctl_core/plugin_manager.py +++ b/packages/deepctl-core/src/deepctl_core/plugin_manager.py @@ -293,6 +293,80 @@ def _agent_friendly_callback( help="Skip interactive prompts; use defaults for any optional features.", )(cmd) + # Accept the global output options after the subcommand as well. + cmd = self._add_global_passthrough_options(cmd) + + return cmd + + def _add_global_passthrough_options(self, cmd: click.Command) -> click.Command: + """Let the global output options work after the subcommand too. + + The root group defines -o/--output, -q/--quiet and -v/--verbose, but + Click only parses group options that appear *before* the subcommand + name — so ``dg models -o json`` used to fail with "No such option + '-o'" while ``dg -o json models`` worked, and the error gave no hint. + These pass-through copies apply the same effect from the subcommand + position. A copy is only added when the command does not already + define the option name itself (for example, ``dg speak --output`` + names an audio file and keeps its own meaning). + """ + from .output import update_output + + taken: set[str] = set() + for param in cmd.params: + taken.update(param.opts) + taken.update(param.secondary_opts) + + def _apply_format( + _ctx: click.Context, _param: click.Parameter, value: str | None + ) -> None: + if value is not None: + update_output(format_type=value) + + def _apply_quiet( + _ctx: click.Context, _param: click.Parameter, value: bool + ) -> None: + if value: + update_output(quiet=True) + + def _apply_verbose( + _ctx: click.Context, _param: click.Parameter, value: bool + ) -> None: + if value: + update_output(verbose=True) + + if not {"--output", "-o"} & taken: + cmd = click.option( + "--output", + "-o", + type=click.Choice( + ["json", "yaml", "table", "csv"], case_sensitive=False + ), + expose_value=False, + callback=_apply_format, + help="Output format (same as the global -o before the command).", + )(cmd) + + if not {"--quiet", "-q"} & taken: + cmd = click.option( + "--quiet", + "-q", + is_flag=True, + expose_value=False, + callback=_apply_quiet, + help="Suppress non-essential output.", + )(cmd) + + if not {"--verbose", "-v"} & taken: + cmd = click.option( + "--verbose", + "-v", + is_flag=True, + expose_value=False, + callback=_apply_verbose, + help="Enable verbose output.", + )(cmd) + return cmd def _build_help_text(self, instance: Any) -> str: diff --git a/packages/deepctl-core/tests/unit/test_plugin_manager.py b/packages/deepctl-core/tests/unit/test_plugin_manager.py index ad5d16d..3d0143f 100644 --- a/packages/deepctl-core/tests/unit/test_plugin_manager.py +++ b/packages/deepctl-core/tests/unit/test_plugin_manager.py @@ -575,3 +575,92 @@ def test_warn_if_plugin_venv_python_mismatch_warns_on_major_diff( ) as mock_warn: plugin_manager._warn_if_plugin_venv_python_mismatch() mock_warn.assert_called_once() + + +class TestGlobalPassthroughOptions: + """Global output options must work after the subcommand too. + + The root group defines -o/--output, -q and -v, but Click only parses + group options given before the subcommand, so `dg models -o json` + failed with "No such option '-o'". Every generated command now carries + pass-through copies — except where the command defines the same option + itself (dg speak --output names an audio file). + """ + + @pytest.fixture + def plugin_manager(self): + return PluginManager() + + def _make_command_class(self, arguments: list[dict[str, Any]]): + class PassthroughProbeCommand(BaseCommand): + name = "probe" + help = "Probe command" + + def get_arguments(self) -> list[dict[str, Any]]: + return arguments + + def handle( + self, + config: Config, + auth_manager: AuthManager, + client: DeepgramClient, + **kwargs, + ) -> Any: + return {"result": "success"} + + return PassthroughProbeCommand + + @pytest.mark.unit + def test_output_option_added_when_free(self, plugin_manager): + """A command without its own -o/--output gets the pass-through.""" + command_class = self._make_command_class([]) + cmd = plugin_manager._create_click_command(command_class()) + + opts = set() + for param in cmd.params: + opts.update(param.opts) + assert "--output" in opts + assert "-o" in opts + assert "--quiet" in opts + assert "--verbose" in opts + + @pytest.mark.unit + def test_output_option_skipped_on_collision(self, plugin_manager): + """A command with its own --output keeps its own meaning.""" + command_class = self._make_command_class( + [ + { + "names": ["--output", "-o"], + "help": "Output audio file path", + "type": str, + "is_option": True, + }, + ] + ) + cmd = plugin_manager._create_click_command(command_class()) + + output_params = [p for p in cmd.params if "--output" in p.opts] + assert len(output_params) == 1 + # The command's own option takes a free-form string, not a Choice. + assert not isinstance(output_params[0].type, click.Choice) + + @pytest.mark.unit + def test_passthrough_output_applies_format(self, plugin_manager): + """Parsing `probe -o json` switches the global output format.""" + from click.testing import CliRunner + from deepctl_core.output import _output_config + + command_class = self._make_command_class([]) + instance = command_class() + # Bypass auth/client plumbing: the test targets option parsing only. + instance.execute = lambda ctx, **kwargs: None + cmd = plugin_manager._create_click_command(instance) + + previous_format = _output_config["format"] + try: + runner = CliRunner() + result = runner.invoke(cmd, ["-o", "yaml"], standalone_mode=False) + assert result.exception is None + assert _output_config["format"] == "yaml" + finally: + _output_config["format"] = previous_format diff --git a/src/deepctl/main.py b/src/deepctl/main.py index 6e65a47..cb8fb51 100644 --- a/src/deepctl/main.py +++ b/src/deepctl/main.py @@ -287,6 +287,38 @@ def _telemetry_transaction() -> Iterator[None]: yield +def _global_option_hint(message: str) -> str | None: + """Build a placement hint when a 'No such option' names a global option. + + Global options belong before the subcommand (``dg -o json models``). + When one is placed after the subcommand instead (``dg models --profile + staging``), Click reports "No such option" with no explanation, which + reads as if the option does not exist at all. This looks the failing + option up on the root group and, when it is a global one, returns a + hint showing the working placement. + """ + import re + + match = re.search(r"[Nn]o such option:?\s*'?(--?[A-Za-z][\w-]*)'?", message) + if not match: + return None + token = match.group(1) + + for param in cli.params: + opts = list(getattr(param, "opts", [])) + list( + getattr(param, "secondary_opts", []) + ) + if token in opts: + takes_value = not getattr(param, "is_flag", False) + usage = f"{token} " if takes_value else token + return ( + f"[yellow]Hint: '{token}' is a global option and must come " + f"before the subcommand, for example: " + f"dg {usage} ...[/yellow]" + ) + return None + + def _safe_console_print(message: str) -> None: """Print a diagnostic to stderr, tolerating a closed/broken stream. @@ -401,6 +433,17 @@ def main() -> None: # Abort, not KeyboardInterrupt. Both are user cancellation: exit 2. _safe_console_print("\n[yellow]Operation cancelled by user[/yellow]") sys.exit(2) + except click.UsageError as e: + # Keep the published exit-code contract (1 = error), but explain the + # one failure mode that reads as a lie: a *global* option placed + # after the subcommand, where Click says "No such option" about an + # option that very much exists. + message = e.format_message() + _safe_console_print(f"[red]Error: {message}[/red]") + hint = _global_option_hint(message) + if hint: + _safe_console_print(hint) + sys.exit(1) except Exception as e: _safe_console_print(f"[red]Error: {e}[/red]") sys.exit(1) # 1 = error; 2 is reserved for user interrupt diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index 9380a35..7ad33fa 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -276,3 +276,63 @@ def test_main_survives_broken_console_on_error(self): main_mod.main() assert exc_info.value.code == 1 + + +class TestGlobalOptionHint: + """A global option after the subcommand gets a placement hint. + + `dg models --profile staging` fails because Click parses group options + only before the subcommand name. The bare "No such option" reads as if + the option does not exist; the hint names the working placement. The + exit code stays 1 per the published contract. + """ + + def test_hint_for_global_option_after_subcommand(self, capsys): + from deepctl.main import main + + # --api-key is a real global option with no pass-through copy, so + # it still fails when placed after the subcommand — now with a hint. + with ( + patch("sys.argv", ["deepctl", "not-a-command", "--api-key", "x"]), + pytest.raises(SystemExit) as exc_info, + ): + main() + + assert exc_info.value.code == 1 + captured = capsys.readouterr() + assert captured.out == "" + assert "Error" in captured.err + + def test_hint_names_global_option(self): + """The helper recognizes root-group options and skips unknowns.""" + from deepctl.main import _global_option_hint + + hint = _global_option_hint("No such option: '--api-key'") + assert hint is not None + assert "--api-key" in hint + assert "before the subcommand" in hint + + hint = _global_option_hint("No such option: '--profile'") + assert hint is not None + assert "--profile" in hint + + # An option that is not global gets no hint. + assert _global_option_hint("No such option: '--not-a-real-flag'") is None + + # A message with no option token gets no hint. + assert _global_option_hint("Missing argument 'FILE'.") is None + + def test_usage_error_without_hint_keeps_contract(self, capsys): + """A plain unknown flag still errors to stderr and exits 1.""" + from deepctl.main import main + + with ( + patch("sys.argv", ["deepctl", "--definitely-not-a-flag"]), + pytest.raises(SystemExit) as exc_info, + ): + main() + + assert exc_info.value.code == 1 + captured = capsys.readouterr() + assert captured.out == "" + assert "Error" in captured.err