mirror of
https://github.com/NCBM/plyngent.git
synced 2026-07-23 05:55:16 +08:00
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).
This commit is contained in:
+28
-4
@@ -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")
|
||||
|
||||
@@ -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}")
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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],
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user