fix(agent): purge les api_base LLM obsoletes (Mistral/Cohere /v2)
Un api_base stocke (ex. https://api.mistral.ai/v2) ecrasait l'URL par defaut corrigee et faisait echouer le test de connexion. Desormais: normalisation a l'ecriture (une base egale au defaut n'est pas stockee), possibilite de vider le champ (api_base='' vs None), et migration 14 qui efface les bases obsoletes/redondantes dans user_llm_keys et llm_config. Tests de non-regression ajoutes.
This commit is contained in:
@@ -547,3 +547,44 @@ def _migration_v511_wiki_v512_templates(conn: sqlite3.Connection) -> None:
|
||||
conn.execute(
|
||||
"CREATE INDEX IF NOT EXISTS idx_pgt_creator ON page_global_templates(created_by)"
|
||||
)
|
||||
|
||||
|
||||
@register(14, "v5.13.0: fix stale LLM provider api_base values")
|
||||
def _migration_fix_llm_api_bases(conn: sqlite3.Connection) -> None:
|
||||
"""Clear `api_base` values that freeze a provider URL to a wrong/default value.
|
||||
|
||||
The Agent "Test connection" uses the per-user (or global) stored `api_base`
|
||||
when present, so an old/incorrect value (e.g. Mistral `…/v2`, Cohere
|
||||
`…/v2`, Google native `/v1beta`) keeps failing even after `PROVIDERS` is
|
||||
corrected. Two kinds of rows are reset to the provider default:
|
||||
|
||||
* known-wrong legacy bases from earlier releases;
|
||||
* a stored base identical to the current provider default (a no-op
|
||||
override that would block future default changes).
|
||||
"""
|
||||
from app.services.llm_client import PROVIDERS
|
||||
|
||||
legacy: dict[str, set[str]] = {
|
||||
"mistral": {"https://api.mistral.ai/v2"},
|
||||
"cohere": {"https://api.cohere.com/v2", "https://api.cohere.com/v1",
|
||||
"https://api.cohere.ai/v2"},
|
||||
"google": {"https://generativelanguage.googleapis.com/v1beta"},
|
||||
"perplexity": {"https://api.perplexity.ai/v1"},
|
||||
"chutes": {"https://api.chutes.ai/v1"},
|
||||
"sensenova": {"https://token.sensenova.cn/v1"},
|
||||
"ltx": {"https://api.ltx.io/v1"},
|
||||
"memtensor": {"https://memos.memtensor.cn/api/openmem/v1"},
|
||||
}
|
||||
for provider, (base, _) in PROVIDERS.items():
|
||||
if base:
|
||||
legacy.setdefault(provider, set()).update({base, base.rstrip("/")})
|
||||
|
||||
for provider, bases in legacy.items():
|
||||
variants = {b for b in bases if b}
|
||||
variants |= {b.rstrip("/") for b in bases if b}
|
||||
for table in ("user_llm_keys", "llm_config"):
|
||||
for value in variants:
|
||||
conn.execute(
|
||||
f"UPDATE {table} SET api_base='' WHERE provider=? AND api_base=?",
|
||||
(provider, value),
|
||||
)
|
||||
|
||||
@@ -810,11 +810,13 @@ async def save_llm_key(request: Request, llm_provider: str):
|
||||
if provider not in PROVIDERS:
|
||||
raise HTTPException(status_code=400, detail=f"Provider inconnu: {provider}")
|
||||
body = await request.json() if request.headers.get("content-type") else {}
|
||||
api_base_raw = body.get("api_base")
|
||||
raw = upsert_user_llm_key(
|
||||
user_id,
|
||||
provider,
|
||||
api_key=(body.get("api_key") or "").strip(),
|
||||
api_base=(body.get("api_base") or "").strip(),
|
||||
# None = keep the stored base, "" = reset to the provider default.
|
||||
api_base=api_base_raw.strip() if isinstance(api_base_raw, str) else None,
|
||||
default_model=(body.get("default_model") or "").strip(),
|
||||
models=body.get("models"),
|
||||
)
|
||||
|
||||
@@ -101,8 +101,12 @@ def set_llm_config(*, provider: str | None = None, model: str | None = None,
|
||||
cfg["verified"] = 0
|
||||
cfg["verified_model"] = ""
|
||||
cfg["last_error"] = ""
|
||||
if api_base is not None and api_base.strip():
|
||||
cfg["api_base"] = api_base.strip()
|
||||
if api_base is not None:
|
||||
# Normalise so a base equal to the provider default is stored as "use
|
||||
# default" — a later correction of PROVIDERS then applies automatically.
|
||||
cfg["api_base"] = _normalize_api_base(
|
||||
provider or cfg.get("provider") or "offline", api_base
|
||||
)
|
||||
if clear_keys:
|
||||
cfg["api_key"] = ""
|
||||
cfg["api_base"] = ""
|
||||
@@ -222,18 +226,44 @@ def list_user_llm_keys(user_id: int) -> list[dict]:
|
||||
return [_mask_key(r) for r in rows]
|
||||
|
||||
|
||||
def _default_api_base(provider: str) -> str:
|
||||
"""The OpenAI-compatible base URL the app uses by default for a provider."""
|
||||
base = (PROVIDERS.get(provider.lower()) or (None, None))[0]
|
||||
return (base or "").rstrip("/")
|
||||
|
||||
|
||||
def _normalize_api_base(provider: str, api_base: str) -> str:
|
||||
"""Drop an api_base that just repeats the provider default.
|
||||
|
||||
Storing the default as a per-user override freezes it: a later correction
|
||||
of the provider URL (e.g. Cohere `/v2` → `/compatibility/v1`) would never
|
||||
apply. An empty value means "use the provider default".
|
||||
"""
|
||||
value = (api_base or "").strip()
|
||||
if value.rstrip("/") == _default_api_base(provider):
|
||||
return ""
|
||||
return value
|
||||
|
||||
|
||||
def upsert_user_llm_key(user_id: int, provider: str, *, api_key: str = "",
|
||||
api_base: str = "", default_model: str = "",
|
||||
api_base: str | None = None, default_model: str = "",
|
||||
models: list[str] | None = None) -> dict:
|
||||
"""Upsert a user's provider key. Empty api_key keeps the existing one
|
||||
(allows saving model/base without re-typing the key). Saving a *different*
|
||||
key resets the `verified` flag so the provider must pass a test again."""
|
||||
key resets the `verified` flag so the provider must pass a test again.
|
||||
|
||||
``api_base`` uses ``None`` to mean "keep the stored value" and an empty
|
||||
string to explicitly reset it to the provider default.
|
||||
"""
|
||||
from app.db import get_conn
|
||||
|
||||
provider = provider.lower()
|
||||
existing = get_user_llm_key(user_id, provider)
|
||||
new_key = api_key if api_key else (existing.get("api_key", "") if existing else "")
|
||||
new_base = api_base if api_base else (existing.get("api_base", "") if existing else "")
|
||||
if api_base is None:
|
||||
new_base = (existing.get("api_base", "") if existing else "")
|
||||
else:
|
||||
new_base = _normalize_api_base(provider, api_base)
|
||||
new_model = default_model if default_model else (existing.get("default_model", "") if existing else "")
|
||||
new_models = models if models is not None else (
|
||||
json.loads(existing["models_json"]) if existing and existing.get("models_json") else []
|
||||
|
||||
@@ -106,6 +106,50 @@ def test_llm_client_openai_compatible_bases(client):
|
||||
assert PROVIDERS["sensenova"][0].endswith("/compatible-mode/v1")
|
||||
|
||||
|
||||
def test_user_llm_key_api_base_normalization(client):
|
||||
"""A stored api_base equal to the provider default is not persisted as an
|
||||
override, so a corrected provider URL always takes effect."""
|
||||
from app.services.llm_config import get_user_llm_key, upsert_user_llm_key
|
||||
|
||||
uid = _admin_id()
|
||||
# default base → stored as empty (no override)
|
||||
upsert_user_llm_key(uid, "mistral", api_key="sk-test",
|
||||
api_base="https://api.mistral.ai/v1")
|
||||
assert get_user_llm_key(uid, "mistral")["api_base"] == ""
|
||||
# custom base → kept
|
||||
upsert_user_llm_key(uid, "mistral", api_base="https://proxy.example.com/v1")
|
||||
assert get_user_llm_key(uid, "mistral")["api_base"] == "https://proxy.example.com/v1"
|
||||
# api_base="" resets the override, api_base=None keeps it
|
||||
upsert_user_llm_key(uid, "mistral", api_base="")
|
||||
assert get_user_llm_key(uid, "mistral")["api_base"] == ""
|
||||
upsert_user_llm_key(uid, "mistral", api_base="https://proxy.example.com/v1")
|
||||
upsert_user_llm_key(uid, "mistral", default_model="mistral-large-latest")
|
||||
assert get_user_llm_key(uid, "mistral")["api_base"] == "https://proxy.example.com/v1"
|
||||
|
||||
|
||||
def test_migration_clears_stale_llm_api_bases(client):
|
||||
"""Migration 14 clears legacy/incorrect api_base values so the corrected
|
||||
provider default applies (e.g. Mistral `/v2` → `/v1`)."""
|
||||
from app.db import get_conn
|
||||
from app.migrations import _migration_fix_llm_api_bases
|
||||
from app.services.llm_config import get_user_llm_key, upsert_user_llm_key
|
||||
|
||||
uid = _admin_id()
|
||||
upsert_user_llm_key(uid, "mistral", api_key="sk-test")
|
||||
with get_conn() as conn:
|
||||
conn.execute(
|
||||
"UPDATE user_llm_keys SET api_base='https://api.mistral.ai/v2' "
|
||||
"WHERE user_id=? AND provider='mistral'",
|
||||
(uid,),
|
||||
)
|
||||
conn.commit()
|
||||
assert get_user_llm_key(uid, "mistral")["api_base"] == "https://api.mistral.ai/v2"
|
||||
with get_conn() as conn:
|
||||
_migration_fix_llm_api_bases(conn)
|
||||
conn.commit()
|
||||
assert get_user_llm_key(uid, "mistral")["api_base"] == ""
|
||||
|
||||
|
||||
def test_llm_client_offline_no_credentials(client):
|
||||
from app.services.llm_client import LLMClient
|
||||
llm = LLMClient(provider="openai", api_key="")
|
||||
|
||||
Reference in New Issue
Block a user