From 553d491cbc8b95a81d7f6c3f67e5c8bd046250a4 Mon Sep 17 00:00:00 2001 From: worldmozara Date: Wed, 15 Jul 2026 14:17:34 +0800 Subject: [PATCH] core/config+cli: empty models are bad; prompt model on provider switch Reject providers with no models into bad_providers and warn with reasons on chat/providers. When /provider changes and the current model is not listed, prompt for a new model (keep it when still supported). --- src/plyngent/cli/app.py | 32 ++++++++++++-- src/plyngent/cli/slash.py | 18 ++++++-- src/plyngent/config/store.py | 6 +++ tests/test_cli/test_repl_commands.py | 61 +++++++++++++++++++++++++++ tests/test_cli/test_selection.py | 18 +++++++- tests/test_config/plyngent-valid.toml | 6 +++ tests/test_config/test_config.py | 24 ++++++++++- 7 files changed, 155 insertions(+), 10 deletions(-) diff --git a/src/plyngent/cli/app.py b/src/plyngent/cli/app.py index fef5af6..911cf61 100644 --- a/src/plyngent/cli/app.py +++ b/src/plyngent/cli/app.py @@ -3,7 +3,7 @@ from __future__ import annotations import asyncio import sys from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, cast import click import msgspec @@ -29,6 +29,8 @@ from plyngent.runtime import ProviderNotSupportedError, create_client from plyngent.tools import set_workspace_root if TYPE_CHECKING: + from collections.abc import Mapping + from plyngent.config.store import ConfigStore _DEFAULT_DB_FILENAME = "chat.db" @@ -43,6 +45,29 @@ def _load_config(config_path: Path | None) -> ConfigStore: raise click.ClickException(msg) from exc +def _warn_bad_providers(bad: Mapping[str, object]) -> None: + """Surface ignored provider entries (parse errors, empty models, …).""" + if not bad: + return + names = ", ".join(sorted(bad.keys())) + click.secho( + f"warning: ignored bad providers ({len(bad)}): {names}", + fg="yellow", + err=True, + ) + for name in sorted(bad.keys()): + entry = bad[name] + reason = "invalid or incomplete entry" + if isinstance(entry, dict): + entry_map = cast("dict[str, object]", entry) + raw_reason = entry_map.get("_reason") + if isinstance(raw_reason, str) and raw_reason: + reason = raw_reason + elif "preset" not in entry_map and not any(k in entry_map for k in ("access_key_or_token", "url")): + reason = "not a provider table" + click.secho(f" - {name}: {reason}", fg="yellow", err=True) + + def _database_config(store: ConfigStore, *, quiet: bool = False) -> DatabaseConfig: raw = dict(store.database) # Prefer a durable file DB so sessions survive CLI restarts. @@ -163,8 +188,7 @@ async def _run_chat( # noqa: C901 — chat orchestration store = _load_config(config_path) if store.bad_providers and not quiet: - names = ", ".join(sorted(store.bad_providers.keys())) - click.secho(f"warning: ignored bad providers: {names}", fg="yellow", err=True) + _warn_bad_providers(store.bad_providers) _setup_workspace_and_hooks(store, workspace, interactive=interactive) confirm_destructive: bool | None = False if yes else None @@ -397,7 +421,7 @@ def providers_cmd(config_path: Path | None) -> None: models = ", ".join(sorted(provider.models.keys())) or "(none listed)" click.echo(f"{name}\tpreset={tag}\tmodels={models}") if store.bad_providers: - click.secho(f"bad: {', '.join(sorted(store.bad_providers.keys()))}", fg="yellow") + _warn_bad_providers(store.bad_providers) @main.group("config") diff --git a/src/plyngent/cli/slash.py b/src/plyngent/cli/slash.py index d94fdf6..fad42d0 100644 --- a/src/plyngent/cli/slash.py +++ b/src/plyngent/cli/slash.py @@ -526,11 +526,23 @@ def provider_cmd(state: ReplState, name: str | None) -> None: return try: pname, provider = select_provider(state.config.providers, preferred=name.strip()) + prev_model = state.model state.provider_name = pname state.provider = provider - # Keep model if still listed; else first model on the new provider. - if state.model not in provider.models and provider.models: - state.model = next(iter(sorted(provider.models.keys()))) + if prev_model in provider.models: + state.model = prev_model + else: + # Current model not on the new provider — pick one (prompt when interactive). + try: + state.model = select_model(provider, preferred=None, interactive=True) + except click.ClickException as exc: + click.echo(f"error: switched provider but model selection failed: {exc}") + return + if prev_model: + click.secho( + f"model {prev_model!r} is not available on {pname}; using {state.model!r}", + fg="yellow", + ) state.rebuild_client() _await(state.persist_llm_selection()) click.echo(f"switched provider to {pname} model={state.model}") diff --git a/src/plyngent/config/store.py b/src/plyngent/config/store.py index 86a1642..60a7ba1 100644 --- a/src/plyngent/config/store.py +++ b/src/plyngent/config/store.py @@ -70,6 +70,12 @@ def _parse_providers( bad_providers[name] = raw_entry continue + # Usable providers must list at least one model (DeepSeek seeds defaults + # when models is omitted; an explicit empty models={} is invalid). + if not provider.models: + bad_providers[name] = {**cast("dict[str, object]", raw_entry), "_reason": "no models"} + continue + providers[name] = provider return providers, bad_providers diff --git a/tests/test_cli/test_repl_commands.py b/tests/test_cli/test_repl_commands.py index 188f946..95b7d2e 100644 --- a/tests/test_cli/test_repl_commands.py +++ b/tests/test_cli/test_repl_commands.py @@ -190,6 +190,67 @@ async def test_model_switch_persists(state: ReplState) -> None: assert row.model == "m2" +async def test_provider_switch_prompts_when_model_missing( + state: ReplState, + monkeypatch: pytest.MonkeyPatch, +) -> None: + from plyngent.config.models import ModelConfig, OpenAICompatibleProvider, OpenAIProvider + + state.config.providers = { + "a": OpenAIProvider( + access_key_or_token="sk", + models={"shared": ModelConfig(), "only-a": ModelConfig()}, + ), + "b": OpenAICompatibleProvider( + access_key_or_token="sk", + url="https://x/v1", + models={"shared": ModelConfig(), "only-b": ModelConfig()}, + ), + } + state.provider_name = "a" + state.provider = state.config.providers["a"] + state.model = "only-a" + state.rebuild_client() + + # When switching to b, only-a is missing → select_model is invoked interactively. + monkeypatch.setattr( + "plyngent.cli.slash.select_model", + lambda provider, preferred=None, interactive=True: "only-b", + ) + assert await handle_slash(state, "/provider b") is True + assert state.provider_name == "b" + assert state.model == "only-b" + sid = state.session_id + assert sid is not None + row = await state.memory.get_session(sid) + assert row is not None + assert row.provider_name == "b" + assert row.model == "only-b" + + +async def test_provider_switch_keeps_shared_model(state: ReplState) -> None: + from plyngent.config.models import ModelConfig, OpenAICompatibleProvider, OpenAIProvider + + state.config.providers = { + "a": OpenAIProvider( + access_key_or_token="sk", + models={"shared": ModelConfig()}, + ), + "b": OpenAICompatibleProvider( + access_key_or_token="sk", + url="https://x/v1", + models={"shared": ModelConfig()}, + ), + } + state.provider_name = "a" + state.provider = state.config.providers["a"] + state.model = "shared" + state.rebuild_client() + assert await handle_slash(state, "/provider b") is True + assert state.provider_name == "b" + assert state.model == "shared" + + async def test_delete_slash_confirm( state: ReplState, capsys: pytest.CaptureFixture[str], diff --git a/tests/test_cli/test_selection.py b/tests/test_cli/test_selection.py index 30f9d44..7c7a181 100644 --- a/tests/test_cli/test_selection.py +++ b/tests/test_cli/test_selection.py @@ -52,9 +52,23 @@ def test_select_provider_interactive_choose() -> None: from tests.test_prompting import ScriptedBackend providers = { - "a": OpenAIProvider(access_key_or_token="sk"), - "b": OpenAICompatibleProvider(access_key_or_token="sk", url="https://x/v1"), + "a": OpenAIProvider(access_key_or_token="sk", models={"m": ModelConfig()}), + "b": OpenAICompatibleProvider( + access_key_or_token="sk", + url="https://x/v1", + models={"m": ModelConfig()}, + ), } with temporary_backend(ScriptedBackend(["2"])): name, _ = select_provider(providers) assert name == "b" + + +def test_select_model_when_preferred_missing_raises() -> None: + provider = OpenAICompatibleProvider( + access_key_or_token="sk", + url="https://x/v1", + models={"m1": ModelConfig()}, + ) + with pytest.raises(Exception, match="unknown model"): + _ = select_model(provider, preferred="nope") diff --git a/tests/test_config/plyngent-valid.toml b/tests/test_config/plyngent-valid.toml index 9f4dd74..4c006a0 100644 --- a/tests/test_config/plyngent-valid.toml +++ b/tests/test_config/plyngent-valid.toml @@ -6,6 +6,9 @@ url = ":memory:" preset = "openai" access_key_or_token = "sk-1145141919810" +[providers.test1.models] +"gpt-test" = { text = true } + [providers.test2] preset = "openai-compatible" url = "https://www.example.com/v1" @@ -18,6 +21,9 @@ access_key_or_token = "sk-1145141919810" preset = "anthropic" access_key_or_token = "sk-1145141919810" +[providers.test3.models] +"claude-test" = { text = true } + [providers.foo1] preset = "deepseek" access_key_or_token = "sk-1145141919810" diff --git a/tests/test_config/test_config.py b/tests/test_config/test_config.py index ca10e3a..15c8b1f 100644 --- a/tests/test_config/test_config.py +++ b/tests/test_config/test_config.py @@ -81,6 +81,23 @@ def test_read_bad_config() -> None: assert isinstance(config.bad_providers, Mapping) +def test_provider_with_empty_models_is_bad(tmp_path: Path) -> None: + path = tmp_path / "empty-models.toml" + _ = path.write_text( + """ +[providers.hollow] +preset = "openai-compatible" +url = "https://example.com/v1" +access_key_or_token = "sk-test" +models = {} +""", + encoding="utf-8", + ) + config = plyngent.config.load(path) + assert "hollow" not in config.providers + assert "hollow" in config.bad_providers + + def test_read_invalid_config() -> None: with pytest.raises(ConfigFormatError): _ = plyngent.config.load(Path(__file__).parent / "plyngent-invalid.toml") @@ -91,8 +108,13 @@ def test_write_new_config() -> None: file.unlink(missing_ok=True) config = plyngent.config.load(file) assert isinstance(config.providers, Mapping) + from plyngent.config import ModelConfig + config.providers = { - "foo1": OpenAIProvider(access_key_or_token="sk-00301212"), + "foo1": OpenAIProvider( + access_key_or_token="sk-00301212", + models={"gpt-test": ModelConfig()}, + ), "foo2": DeepseekProvider(access_key_or_token="sk-00301212"), } assert isinstance(config.providers, Mapping)