fix(llm): LM Studio translation failed on a placeholder model name (#1332) (#1359)

* fix(llm): LM Studio translation failed on a placeholder model name (#1332)

Reported as a clean A/B: translation works through Ollama and fails
through LM Studio on the same machine. The difference is one line in the
provider table. LM Studio shipped `local-model` as its default_model,
which is a placeholder, not a model id — LM Studio serves whatever the
user has loaded and 404s a name it does not know. Ollamas default is
`llama3.1`, a real name people actually pull, so the identical code path
worked there.

No name we ship can be right, because the answer depends on what the user
loaded. So ask the server: resolve_model now discovers from /v1/models for
providers whose default is a placeholder, positioned BELOW any explicit
env or stored setting so it can never override a deliberate choice, and
above the default so a server that is down leaves the caller where it was.

Cached per provider — translation resolves the model per segment and a
round-trip each time would trade a broken setup for a slow one — and
dropped whenever a base_url or model edit could invalidate it, since a
stale id would make the users change look like it did nothing.

Also: a 404 from a LOCAL provider is almost never a wrong URL, because the
request reached the server. The generic "check the model name and Base URL
path" sends the user to audit a URL that works, so a local 404 now names
the models that ARE loaded, or says the server has none.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(llm): bound the discovery cache both ways; do not over-claim a 404

Three review findings, all valid, all about the cache being permanent in
one direction or absent in the other.

- A failed probe was not remembered, so a stopped LM Studio cost a 5s
  timeout on EVERY translated segment — a 200-segment dub would spend
  1000s discovering nothing, worse than the bug being fixed. Remembered
  for 30s: short enough that starting the server recovers in seconds
  rather than needing a restart.
- A successful discovery was cached forever, so swapping the loaded model
  inside LM Studio 404d every translation until an app restart. Now a 300s
  TTL, plus an immediate invalidation when a local 404 proves the cached
  name is one the server rejects.
- _local_models collapsed a FAILED listing into [], which let the error say
  "reports no loaded models" about a lookup that never happened — a
  confident wrong diagnosis replacing a vague right one. None vs [] are
  now distinct, and the generic 404 text stands when nothing was
  established.

The cache therefore cannot be a dict[str, str]: "no entry" and "we looked
and there was nothing" have to be distinguishable for the negative case to
be cacheable at all.

Five tests, three failing before this change. They age the cache entry
rather than patching time.monotonic — that name is the stdlib`s, shared
with sqlite and logging, and freezing it breaks the settings store
underneath the test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Palash Debnath
2026-08-04 14:48:56 +05:30
committed by GitHub
co-authored by Claude Opus 5
parent c03e3f2525
commit 1a4b95890a
4 changed files with 421 additions and 6 deletions
+1
View File
@@ -48,6 +48,7 @@ The bundled TTS model package (`pyproject.toml`) is versioned independently.
- Linux AppImage: recording failed with "No microphone found" on hosts whose GStreamer is newer than the build runner's, even with a verified-healthy audio stack — your own GStreamer now takes precedence, and the plugin cache is app-private so it can neither be confused by nor corrupt the one other apps use — thanks @Kakuzen93! (#1333)
- Linux AppImage: that GStreamer preference actually takes effect — the check guarding it could never pass, so it had been silently doing nothing. (#1333)
- A TTS job abandoned for exceeding its compute budget now records where it was actually stuck, so a hang stops being reported as a machine that is merely too slow. (#1338, #1329, #1348)
- Translation through LM Studio works. The built-in model name was the placeholder `local-model`, which LM Studio rejects because it serves whatever you have loaded — OmniVoice now asks it, and a 404 from a local server names the models that ARE loaded instead of telling you to check a URL that was fine — thanks @biga73! (#1332)
### Docs
+51 -2
View File
@@ -446,14 +446,63 @@ def test_llm_provider(provider_id: str):
"latency_ms": int((_time.monotonic() - t0) * 1000),
}
except Exception as e: # noqa: BLE001 — surface a clean, scrubbed error to the UI
kind = _classify_llm_error(e)
detail = _scrub_llm_detail(e, api_key)
# A 404 from a LOCAL server is almost never a wrong URL — the request
# reached it — it is a model name the server does not have loaded. The
# generic "check the model name and Base URL path" sends the user to
# audit a URL that works. Ask what IS loaded and say so (#1332).
if kind == "not_found" and p.local:
available = _local_models(base_url, api_key)
asked = llm_providers.resolve_model(p)
if available:
detail = (
f"{p.display_name} is running, but has no model named "
f"{asked!r}. Loaded right now: {', '.join(available[:10])}"
+ ("" if len(available) > 10 else "")
+ ". Pick one in the Model field above."
)
# The cached discovery, if any, produced a name this server
# rejects — most likely the user swapped the loaded model
# inside the local app. Drop it so the next attempt re-asks
# rather than repeating the same 404 until the TTL expires.
llm_providers.forget_discovered_models(p.id)
elif available == []:
detail = (
f"{p.display_name} is running, but reports no loaded models, "
f"so {asked!r} cannot be served. Load a model in "
f"{p.display_name} first, then test again."
)
llm_providers.forget_discovered_models(p.id)
# available is None: the model listing itself failed, so nothing
# here is established. Keep the generic 404 text rather than
# inventing a diagnosis the lookup did not support.
return {
"ok": False,
"kind": _classify_llm_error(e),
"detail": _scrub_llm_detail(e, api_key),
"kind": kind,
"detail": detail,
"latency_ms": int((_time.monotonic() - t0) * 1000),
}
def _local_models(base_url: str, api_key: str):
"""Model ids a local OpenAI-compatible server currently serves.
``None`` when the listing itself failed, ``[]`` when it succeeded and the
server has nothing loaded. The distinction is load-bearing: collapsing both
to ``[]`` let the caller state "reports no loaded models" on a lookup that
never happened, which is a confident wrong diagnosis in place of a vague
right one (CodeRabbit). Only used to sharpen an error message, so it must
never raise a second error on top of the first.
"""
try:
from openai import OpenAI
client = OpenAI(api_key=api_key, base_url=base_url, max_retries=0)
return sorted(m.id for m in client.models.list(timeout=5))
except Exception: # noqa: BLE001
return None
@router.get("/llm-providers/{provider_id}/models")
def list_llm_provider_models(provider_id: str):
"""List model ids the provider's key can access (OpenAI-compat /models).
+125 -4
View File
@@ -26,6 +26,7 @@ from __future__ import annotations
import logging
import os
import time
from dataclasses import dataclass
from typing import Optional
@@ -55,6 +56,10 @@ class Provider:
# the pre-registry behaviour where a lone TRANSLATE_BASE_URL was usable
# keyless.
key_optional: bool = False
# True when ``default_model`` is a placeholder rather than a model this
# provider will accept. LM Studio serves whatever the user has loaded, so
# there is no name we can ship that is right — see resolve_model.
model_is_placeholder: bool = False
needs_account: bool = False # Cloudflare: base_url needs an account id
account_env: Optional[str] = None
signup_url: str = ""
@@ -143,11 +148,16 @@ _PROVIDERS: tuple[Provider, ...] = (
base_url_env="OLLAMA_BASE_URL", model_env="OLLAMA_MODEL",
signup_url="https://ollama.com",
notes="Fully offline. Run `ollama pull llama3.1` first."),
# `local-model` is a placeholder, NOT a model id — LM Studio serves
# whatever the user has loaded and rejects a name it does not know, which
# is why translation failed here while Ollama (whose default `llama3.1` is
# a real name people actually pull) worked on the same machine (#1332).
# resolve_model asks the server instead of shipping a guess.
Provider("lmstudio", "LM Studio (local)", "http://localhost:1234/v1",
"local-model", local=True,
"local-model", local=True, model_is_placeholder=True,
base_url_env="LMSTUDIO_BASE_URL", model_env="LMSTUDIO_MODEL",
signup_url="https://lmstudio.ai",
notes="Fully offline. Start the LM Studio local server."),
notes="Fully offline. Start the LM Studio local server and load a model."),
Provider("custom", "Custom (OpenAI-compatible)", "", "",
key_envs=("TRANSLATE_API_KEY",), base_url_env="TRANSLATE_BASE_URL",
model_env="TRANSLATE_MODEL", key_optional=True,
@@ -205,13 +215,120 @@ def resolve_base_url(p: Provider, *, substitute: bool = True) -> str:
return val or ""
#: How long a discovered model id is trusted. Bounded rather than permanent
#: because the user can swap the loaded model inside LM Studio without touching
#: OmniVoice at all — an unbounded cache would keep sending the unloaded name
#: and 404 every translation until a restart (greptile).
DISCOVERY_TTL_S = 300.0
#: How long a FAILED probe is remembered. Without this, a server that is
#: stopped costs a 5s timeout on *every translated segment* — a 200-segment dub
#: would spend 1000s discovering nothing, which is worse than the bug being
#: fixed (greptile / CodeRabbit). Short, so starting the server recovers within
#: seconds rather than needing a restart.
DISCOVERY_FAILURE_TTL_S = 30.0
#: provider id → (model id or None, monotonic expiry). ``None`` is a remembered
#: failure, which is why this cannot be a plain ``dict[str, str]``: "no entry"
#: and "we looked and there was nothing" have to be distinguishable or the
#: negative case cannot be cached at all.
_DISCOVERED_MODEL: dict[str, tuple[Optional[str], float]] = {}
def forget_discovered_models(pid: Optional[str] = None) -> None:
"""Drop the discovery cache (all providers, or one).
Called whenever the user changes a provider's model or base URL: keeping a
model discovered from the previous server would silently ignore the edit,
which is a worse failure than the one this whole path exists to fix. Also
called when a request is rejected for an unknown model, so a swap made
inside the local app self-heals on the next attempt rather than at the next
TTL expiry.
"""
if pid is None:
_DISCOVERED_MODEL.clear()
else:
_DISCOVERED_MODEL.pop(pid, None)
def _cached_discovery(pid: str) -> tuple[bool, Optional[str]]:
"""``(hit, value)``. ``hit`` is False once the entry has expired, so a
remembered failure (value ``None``) is still a hit until it ages out."""
entry = _DISCOVERED_MODEL.get(pid)
if entry is None:
return False, None
value, expires_at = entry
if time.monotonic() >= expires_at:
_DISCOVERED_MODEL.pop(pid, None)
return False, None
return True, value
def discover_model(p: Provider) -> Optional[str]:
"""Ask an OpenAI-compatible server which model it is actually serving.
Only used when we would otherwise send a placeholder. Never raises: a
server that is down or does not implement ``/v1/models`` leaves the caller
with the placeholder, which is exactly where it was before.
Both outcomes are cached success for :data:`DISCOVERY_TTL_S`, failure for
:data:`DISCOVERY_FAILURE_TTL_S` because this runs once per translated
segment, so an uncached failure costs a 5s timeout per segment.
"""
hit, cached = _cached_discovery(p.id)
if hit:
return cached
base_url = resolve_base_url(p)
if not base_url:
return None
try:
from openai import OpenAI
# max_retries=0 + a short timeout: this runs in the request path, and a
# local server that is not running must fail fast rather than add the
# SDK's retry ladder to a translation the user is waiting on.
client = OpenAI(api_key=resolve_api_key(p) or "local",
base_url=base_url, max_retries=0)
ids = [m.id for m in client.models.list(timeout=5)]
except Exception as e: # noqa: BLE001 — discovery is best-effort by design
logger.debug("model discovery failed for %s: %s", p.id, e)
_DISCOVERED_MODEL[p.id] = (None, time.monotonic() + DISCOVERY_FAILURE_TTL_S)
return None
if not ids:
_DISCOVERED_MODEL[p.id] = (None, time.monotonic() + DISCOVERY_FAILURE_TTL_S)
return None
# Deterministic rather than "whatever the server listed first", so two runs
# on the same machine pick the same model and a bug report is reproducible.
chosen = sorted(ids)[0]
if len(ids) > 1:
logger.info(
"%s has %d models loaded and no model is set in Settings; using %r. "
"Pick one in Settings → LLM Providers to choose deliberately.",
p.display_name, len(ids), chosen,
)
_DISCOVERED_MODEL[p.id] = (chosen, time.monotonic() + DISCOVERY_TTL_S)
return chosen
def resolve_model(p: Provider) -> str:
"""Env override → stored override → discovered → default.
Discovery sits between the user's choice and the built-in default so it can
never override an explicit setting, and only runs for providers whose
default is a placeholder everyone else keeps a pure, offline resolution.
"""
from services import settings_store
return (
explicit = (
(p.model_env and os.environ.get(p.model_env))
or settings_store.get_text(_MODEL_KEY + p.id)
or p.default_model
)
if explicit:
return explicit
if p.model_is_placeholder:
discovered = discover_model(p)
if discovered:
return discovered
return p.default_model
def resolve_api_key(p: Provider) -> Optional[str]:
@@ -329,6 +446,10 @@ def save_overrides(pid: str, *, base_url: Optional[str] = None,
settings_store.set_text(_BASE_URL_KEY + pid, "" if bu == p.default_base_url else bu)
if model is not None:
settings_store.set_text(_MODEL_KEY + pid, model.strip())
# Any base_url or model edit can invalidate a discovered id — a stale one
# would make the user's change look like it did nothing.
if base_url is not None or model is not None:
forget_discovered_models(pid)
if account_id is not None:
settings_store.set_text(f"llm.account.{pid}", account_id.strip())
+244
View File
@@ -0,0 +1,244 @@
"""Translation must work on LM Studio, not just on Ollama (#1332).
A user reported that translation works through Ollama and fails through LM
Studio on the same machine a clean A/B that rules out the surrounding
pipeline. The cause is in the provider table: LM Studio's `default_model` was
the string ``"local-model"``, which is a placeholder and not a model id. LM
Studio serves whatever the user has loaded and rejects a name it does not know,
so every request 404s. Ollama's default is ``llama3.1`` — a real name that
people actually pull so the same code path worked there.
There is no name we could ship that would be right, because the answer depends
on what the user loaded. So the fix asks the server, and these tests pin the
parts that would silently regress: that discovery never overrides a user's
explicit choice, that it stays off for providers with real defaults, and that a
server which is down or empty leaves the caller no worse off.
"""
import importlib
import os
import sys
import pytest
os.environ.setdefault("OMNIVOICE_MODEL", "test")
os.environ.setdefault("OMNIVOICE_DISABLE_FILE_LOG", "1")
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "backend"))
@pytest.fixture
def llm():
"""Resolve at call time — sibling suites reload/purge ``services.*``."""
mod = importlib.import_module("services.llm_providers")
mod.forget_discovered_models()
yield mod
mod.forget_discovered_models()
@pytest.fixture(autouse=True)
def _no_env_or_store(monkeypatch, llm):
"""A clean slate: no env pins, and a settings store that answers empty.
Without this the developer's own configuration decides the result, which is
how a test like this passes on one machine and fails on another.
"""
for var in ("LMSTUDIO_MODEL", "LMSTUDIO_BASE_URL", "OLLAMA_MODEL"):
monkeypatch.delenv(var, raising=False)
import services.settings_store as store
monkeypatch.setattr(store, "get_text", lambda *a, **k: "")
def _expire(llm, pid):
"""Age a cache entry past its TTL without touching the clock.
``time.monotonic`` is the stdlib's, shared with sqlite and logging, so
freezing it breaks the settings store underneath the test rewriting the
stored expiry tests the same branch without that blast radius.
"""
value, _ = llm._DISCOVERED_MODEL[pid]
llm._DISCOVERED_MODEL[pid] = (value, 0.0)
def _fake_openai(monkeypatch, llm, ids, raises=None):
"""Stub the OpenAI SDK's models.list for the discovery path."""
calls = {"n": 0}
class _Models:
def list(self, timeout=None):
calls["n"] += 1
if raises:
raise raises
return [type("M", (), {"id": i})() for i in ids]
class _Client:
def __init__(self, **kw):
self.models = _Models()
fake = type("openai", (), {"OpenAI": _Client})
monkeypatch.setitem(sys.modules, "openai", fake)
return calls
def test_placeholder_is_replaced_by_a_real_loaded_model(monkeypatch, llm):
"""The reported bug: without this, `local-model` goes on the wire."""
_fake_openai(monkeypatch, llm, ["qwen2.5-7b-instruct"])
p = llm.get_provider("lmstudio")
assert llm.resolve_model(p) == "qwen2.5-7b-instruct"
def test_an_explicit_model_always_wins(monkeypatch, llm):
"""Discovery sits BELOW the user's choice. A setting that gets silently
replaced by whatever the server happens to serve is a worse bug than the
one being fixed."""
import services.settings_store as store
monkeypatch.setattr(store, "get_text",
lambda key, *a, **k: "my-pick" if "model" in key else "")
calls = _fake_openai(monkeypatch, llm, ["something-else"])
p = llm.get_provider("lmstudio")
assert llm.resolve_model(p) == "my-pick"
assert calls["n"] == 0, "discovery ran despite an explicit model being set"
def test_env_pin_also_wins(monkeypatch, llm):
monkeypatch.setenv("LMSTUDIO_MODEL", "from-env")
calls = _fake_openai(monkeypatch, llm, ["something-else"])
p = llm.get_provider("lmstudio")
assert llm.resolve_model(p) == "from-env"
assert calls["n"] == 0
def test_providers_with_a_real_default_never_probe(monkeypatch, llm):
"""Ollama's default is a real model name, and every cloud provider's is
too. Probing them would add a network round-trip to resolve a value that
was already correct and for a cloud provider, on every call."""
calls = _fake_openai(monkeypatch, llm, ["irrelevant"])
for pid, expected in (("ollama", "llama3.1"), ("openai", "gpt-4o-mini")):
assert llm.resolve_model(llm.get_provider(pid)) == expected
assert calls["n"] == 0
def test_server_down_falls_back_to_the_default(monkeypatch, llm):
"""Discovery is best-effort: a local server that is not running must leave
the caller exactly where it was, not raise inside a translation."""
_fake_openai(monkeypatch, llm, [], raises=ConnectionError("refused"))
p = llm.get_provider("lmstudio")
assert llm.resolve_model(p) == "local-model"
def test_server_with_no_models_falls_back(monkeypatch, llm):
"""LM Studio running with nothing loaded returns an empty list. Picking
from it would be an IndexError in the request path."""
_fake_openai(monkeypatch, llm, [])
assert llm.resolve_model(llm.get_provider("lmstudio")) == "local-model"
def test_choice_is_deterministic_across_runs(monkeypatch, llm):
"""Several models loaded: two runs on one machine must pick the same one,
or a bug report stops being reproducible."""
_fake_openai(monkeypatch, llm, ["zeta", "alpha", "mid"])
p = llm.get_provider("lmstudio")
first = llm.resolve_model(p)
llm.forget_discovered_models()
_fake_openai(monkeypatch, llm, ["mid", "zeta", "alpha"]) # different order
assert llm.resolve_model(p) == first == "alpha"
def test_discovery_is_cached(monkeypatch, llm):
"""Translation resolves the model per segment; a round-trip each time would
turn a working setup into a slow one."""
calls = _fake_openai(monkeypatch, llm, ["qwen2.5-7b-instruct"])
p = llm.get_provider("lmstudio")
for _ in range(5):
llm.resolve_model(p)
assert calls["n"] == 1
def test_saving_a_base_url_drops_the_cache(monkeypatch, llm):
"""Pointing at a different server must not keep serving the old server's
model that would make the user's edit look like it did nothing."""
_fake_openai(monkeypatch, llm, ["first-server-model"])
p = llm.get_provider("lmstudio")
assert llm.resolve_model(p) == "first-server-model"
import services.settings_store as store
monkeypatch.setattr(store, "set_text", lambda *a, **k: None)
llm.save_overrides("lmstudio", base_url="http://localhost:9999/v1")
_fake_openai(monkeypatch, llm, ["second-server-model"])
assert llm.resolve_model(p) == "second-server-model"
def test_lmstudio_is_the_only_placeholder_today():
"""A guard on the flag itself: marking a provider as a placeholder makes
every resolve for it hit the network, so it should be a deliberate act."""
import services.llm_providers as mod
flagged = {p.id for p in mod.all_providers() if p.model_is_placeholder}
assert flagged == {"lmstudio"}, (
f"model_is_placeholder changed to {flagged}. Each one adds a network "
f"probe to model resolution — intended?"
)
def test_a_failed_probe_is_not_retried_per_segment(monkeypatch, llm):
"""A stopped server must cost one 5s timeout, not one per segment.
Translation resolves the model for every segment it renders, so an uncached
failure turns a 200-segment dub into 1000 seconds of discovering nothing
worse than the bug this whole path fixes (greptile / CodeRabbit).
"""
calls = _fake_openai(monkeypatch, llm, [], raises=ConnectionError("refused"))
p = llm.get_provider("lmstudio")
for _ in range(20):
assert llm.resolve_model(p) == "local-model"
assert calls["n"] == 1, (
f"probed {calls['n']} times against a server that is down; each one is "
f"a 5s timeout in the request path"
)
def test_an_empty_server_is_also_remembered(monkeypatch, llm):
"""Running-but-nothing-loaded is just as expensive to re-probe as down."""
calls = _fake_openai(monkeypatch, llm, [])
p = llm.get_provider("lmstudio")
for _ in range(10):
llm.resolve_model(p)
assert calls["n"] == 1
def test_a_remembered_failure_expires(monkeypatch, llm):
"""...but it must expire, or starting the server would need an app restart."""
_fake_openai(monkeypatch, llm, [], raises=ConnectionError("refused"))
p = llm.get_provider("lmstudio")
assert llm.resolve_model(p) == "local-model"
# Age the entry rather than patching time.monotonic: that name is the
# stdlib's, shared with sqlite and logging, and freezing it breaks the
# settings store underneath the test.
_expire(llm, "lmstudio")
_fake_openai(monkeypatch, llm, ["now-its-up"])
assert llm.resolve_model(p) == "now-its-up", (
"a failure was remembered forever; the user starts LM Studio and "
"nothing works until they restart OmniVoice"
)
def test_a_discovered_model_expires(monkeypatch, llm):
"""The user can swap the loaded model inside LM Studio without touching
OmniVoice. An unbounded cache keeps sending the unloaded name and 404s
every translation until a restart (greptile)."""
_fake_openai(monkeypatch, llm, ["first-model"])
p = llm.get_provider("lmstudio")
assert llm.resolve_model(p) == "first-model"
_expire(llm, "lmstudio")
_fake_openai(monkeypatch, llm, ["swapped-model"])
assert llm.resolve_model(p) == "swapped-model"
def test_a_fresh_discovery_is_still_reused_within_its_ttl(monkeypatch, llm):
"""The TTL must not defeat the caching it bounds."""
calls = _fake_openai(monkeypatch, llm, ["m"])
p = llm.get_provider("lmstudio")
llm.resolve_model(p)
llm.resolve_model(p)
assert calls["n"] == 1