Review findings from greptile on this PR.
P1 — a profile-derived language refused by a *remote* worker never
reached the new branch. The worker's ValueError travels home as
RemoteJobFailed, a GatewayError caught ahead of the ValueError handler,
so the user got a retryable 503 offering "run it on this machine
instead" — which cannot help, because the same engine refuses the same
language wherever it runs. Both handlers now build the response through
one helper, so the local and remote paths cannot drift again. Only
language refusals change class there; a genuine worker failure keeps its
retryable 503 and its "run it here" offer.
P2 — a *generic* rejection was wrapped twice. `_language_rejection_or`
adds the engine remedy before the handler sees it, and the profile
message then quoted that wrapper, repeating both the engine-switch
advice and "Engine's own message:". The wrapper now keeps the engine's
own error reachable and the profile message quotes that, once. The
Kokoro path never showed this because its wording is self-describing and
skips the first wrapper, which is why the first cut's assertion passed.
Three regression tests, each failing before this commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A user on mlx-audio with the language picker on "Auto" got:
400: mlx-audio's Kokoro model doesn't support language='Persian'.
… Pick one of those, leave language as 'Auto', or switch to a
multilingual engine …
They had left it on Auto. The UI omits `language` entirely while its
picker reads "Auto" (useProfiles.js only appends a non-Auto value), and
#533 fills that gap from the selected voice profile. So Auto is exactly
how 'Persian' got there: the remedy the message leads with is the state
the user was already in, and nothing points at the profile that actually
supplied the language. Three generate attempts in their action log, a
detour through Settings, then the report.
Provenance was the missing fact, and only the request scope has it — the
engine adapters are handed a language with no idea who chose it. So
_resolve_profile_conditioning now reports whether it filled the language,
and /generate uses that to answer a refused profile language by naming
the profile and the remedies that exist: change the profile's language,
pick a supported one explicitly, or switch engine.
An explicitly requested language is untouched — the user really did pick
it, so blaming the profile would be a lie — and #533 still drives
generation whenever the engine can speak the profile's language.
Recognising the refusal needed one more thing: Kokoro's wording
("doesn't support language=…") matched none of #1257's signatures, so
the engine that issue was written for was the one engine its rewrite
never fired for. That wording is now recognised, but kept out of #1257's
rewrite path — it already names its engine and its languages, and
re-wrapping it only nests "Engine's own message:" twice.
No per-model language map: #1257 weighed that and chose engine-naming
over "a brittle map that goes stale on each engine update". This follows
the same principle — say where the language came from, don't enumerate.
docs/engines/mlx-audio.md repeated the same "leave language on Auto"
advice and is corrected here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Raises docstring coverage on the changed files past the threshold the PR
check enforces; the reasoning each test carried in a leading comment now
lives where the checker and a reader both find it.
resolve_kokoro_lang_code validates against mlx-audio's own LANG_CODES —
its docstring is explicit that the vendored table is the only source of
truth — but built the "Kokoro supports: …" list in the rejection message
from the hardcoded _KOKORO_ISO_BY_FULL_NAME instead.
That map exists to translate full names into Kokoro's codes, and it has
no entry that produces "b". British English is reachable as en-gb, and
the existing test asserts resolve("en-gb") == "b", yet the message never
mentioned it: the installed table lists nine languages and the message
named eight. Any language a later mlx-audio adds would be dropped the
same way, telling a user to switch engines when they need not.
Derives the labels from ALIASES and LANG_CODES instead, preferring the
full name a caller can actually pass and falling back to the table's own
description for codes no full name reaches.
The derivation takes the tables as arguments, so its tests run on every
platform. The resolution tests around it need the Apple-Silicon-only
package and skip on every CI runner, which is why a message drifting
from the table it documents went unnoticed.
Fixes#2156
Installing a gated model failed the fast download path with "401
Unauthorized" even with a valid token and the licence accepted, then fell
back to snapshot_download and logged a 401 that reads like the token or
the licence grant is at fault when it is neither (#2163).
`token_resolver.resolve()` returns a ResolvedToken record, not the bearer
string. Two call sites handed that record straight to consumers typed
`token: str | None`, and both fail silently rather than loudly:
- `_segmented_snapshot` passes it to HfApi, get_hf_file_metadata and our
own segmented_download. huggingface_hub's build_hf_headers ignores a
non-str token and falls back to its own ambient discovery, so a token
held only in VoiceStudio's Settings produces NO Authorization header
and every gated file 401s. segmented_download instead interpolates it
into `f"Bearer {token}"`, sending a malformed header that also inlines
the raw secret into the request.
- `_step_fetch_weights` passes it to snapshot_download, so gated engine
weights 401 the same way.
Every other resolve() caller already unwraps `.token`; these two were the
outliers. Both now unwrap once, at the seam.
The existing weights tests all stubbed resolve() to return None, so no
test ever exercised a resolved token — which is why this went unnoticed.
The new tests drive a real ResolvedToken through both seams and assert a
`str` reaches every consumer, plus an integration test that installs the
pyannote diarisation pipeline end to end: a weightless config_only repo
validates, both dependency repos are fetched, and every call carries the
bearer string.
Two catalogue invariants keep the rest of #2163 from returning by edit:
dependency repos must be revision-pinned (revision_for raises otherwise,
so an unpinned one ships an always-failing install), and a config_only
entry must declare config_required_files (without them the completeness
check can never pass and the error lists no files at all — the shape the
report hit on 0.5.2).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Palash Debnath