Support audio input in Studio chat on Apple Silicon by Lyxot · Pull Request #7699 · unslothai/unsloth · GitHub
Skip to content

Support audio input in Studio chat on Apple Silicon - #7699

Merged
danielhanchen merged 6 commits into
unslothai:mainfrom
Lyxot:feat/studio-mlx-audio-inference
Aug 3, 2026
Merged

Support audio input in Studio chat on Apple Silicon#7699
danielhanchen merged 6 commits into
unslothai:mainfrom
Lyxot:feat/studio-mlx-audio-inference

Conversation

@Lyxot

@Lyxot Lyxot commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

Studio chat on Apple Silicon could not accept audio input. The MLX inference backend reported no audio capability, had no audio generation path, and its command loop had no handler for the audio-input command, so a Mac user running Gemma 3n, Gemma 4 or MiniCPM-o saw no attach control and such a request would never have been answered. The transformers backend has served audio input for these models all along, so this was a Mac-only gap.

Separately, Phi-4-multimodal failed to load at all, with Model type phi4mm not supported — a message about the wrong thing, since the model is multimodal and mlx-vlm implements it.

What changed

Audio input for MLX omni models. Whether a checkpoint can accept audio is asked of unsloth_zoo, which answers by observing whether audio content changes what the processor produces. That is a per-checkpoint question rather than a per-family one: an export can keep its audio modules and report a sample rate while its processor quietly drops the audio, and only asking the processor reveals it. Two things stay in this backend because they belong to it rather than to the checkpoint — uploads arrive at the rate the chat route decodes to, and prompts are rendered through mlx-vlm's registry, where some families accept an audio count and silently drop it. The rendered prompt is what the capability call probes with, so a family whose marker only its own template emits is judged on the real thing.

Generation hands the waveform to mlx-vlm's streaming API alongside the rendered prompt, decodes greedily with the same default prompts as the transformers backend so both transcribe alike, and yields incremental deltas because the audio route forwards each chunk verbatim, unlike the text and vision paths whose consumers diff cumulative snapshots. Only the current user turn may caption the audio, so an audio-only turn takes the transcription default instead of borrowing text from earlier history.

Capability travels the whole chain rather than stopping at the backend: the worker's load result and the load response both prefer the post-load answer over pre-load config detection, and the frontend refreshes an existing catalog entry instead of skipping it. The UI can therefore neither advertise audio for a checkpoint that would refuse it nor hide it for one that works.

Phi-4-multimodal routing. Nothing in its config marks it as a vision model: it keeps its image settings under embd_layer, declares itself ForCausalLM, and exposes none of the keys the detector looks for, so it was routed to the text loader, which has no implementation for it. It joins the curated list of remote-code vision types alongside Phi-3-vision, which is the same shape. It then loads as the vision model it is, and its audio input works too.

图片

Risks and tradeoffs

  • Depends on feat(mlx): audio-input training for VLM SFT, behind a per-family gate unsloth-zoo#966, which adds the public audio_input_capability and audio_extractor_sampling_rate used here, and which also teaches the audio path to feed processors that take (samples, rate) pairs — the contract Phi-4-multimodal requires. This should land after that PR, and the unsloth_zoo floor in pyproject.toml will need to move to the release containing it.
  • Capability detection is an optional probe and must never fail a load, so the dependency import, the call and every access to its result run inside one guard. A build with a different signature degrades to "no audio" rather than taking the load down.
  • phi4mm is added to a detection set shared by all backends, so it is now treated as a vision model on the CUDA and GGUF paths too. That is correct — it is one — but it is a wider surface than the MLX work.
  • The audio route is audio-only by design, matching the transformers backend: an image sent alongside audio is dropped by the route before it reaches this code.
  • TTS is untouched. Omni audio-input models are classified so that the chat route's text-to-speech redirect cannot fire for them.

Validation

Real checkpoints on Apple Silicon:

model audio capability result
mlx-community/gemma-3n-E2B-it-4bit advertised streams a transcription
unsloth/gemma-4-E4B-it-UD-MLX-4bit advertised streams a transcription
mlx-community/MiniCPM-o-4_5-4bit advertised streams a transcription
microsoft/Phi-4-multimodal-instruct advertised loads and transcribes real speech
mlx-community/Qwen2.5-VL-3B-Instruct-4bit none image-only model, unchanged
mlx-community/Qwen3-0.6B-4bit none text model, unchanged

Coverage also spans a full inference worker subprocess run, from load through streamed tokens to completion, and a load request returning audio capability for Gemma 3n and none for a text control.

python -m pytest studio/backend/tests/test_mlx_inference_backend.py studio/backend/tests/test_safetensors_capability_advertise.py studio/backend/tests/test_orchestrator_unload_cancel.py -q
# 156 passed

Lyxot added 2 commits August 1, 2026 01:09
Studio chat on Apple Silicon could not accept audio input. The MLX inference backend reported no audio capability, had no audio generation path, and its command loop had no handler for the audio-input command, so users of Gemma 3n, Gemma 4 and MiniCPM-o on a Mac saw no attach control and such a request would never have been answered. The transformers backend has served audio input for these models all along.

Whether a checkpoint can accept audio is now asked of unsloth_zoo, which answers it by observing whether audio content changes what the processor produces. That is a per-checkpoint question rather than a per-family one: an export can keep its audio modules and report a sample rate while its processor quietly drops the audio, and only asking the processor reveals it. Two things stay in this backend because they belong to it rather than to the checkpoint: uploads arrive at the rate the chat route decodes to, and prompts are rendered through mlx-vlm's registry, where some families accept an audio count and silently drop it. The rendered prompt is what the capability call probes with, so a family whose marker only its own template emits is judged on the real thing.

Generation hands the waveform to mlx-vlm's streaming API alongside the rendered prompt, decodes greedily and uses the same default prompts as the transformers backend so both transcribe alike, and yields incremental deltas because the audio route forwards each chunk verbatim, unlike the text and vision paths whose consumers diff cumulative snapshots. Only the current user turn may caption the audio, so an audio-only turn takes the transcription default instead of borrowing text from earlier history.

Capability travels the whole chain instead of stopping at the backend: the worker's load result and the load response both prefer the post-load answer over pre-load config detection, and the frontend refreshes an existing catalog entry rather than skipping it, so the UI can neither advertise audio for a checkpoint that would refuse it nor hide it for one that works. The new command branch reuses the drain discipline of ordinary generation, so a queued audio request cannot erase an unload's cancellation.

Capability detection is an optional probe and must never fail a load, so the dependency import, the call and every access to its result run inside one guard; a build with a different signature degrades to "no audio" rather than taking the load down.

Validated on Apple Silicon with real checkpoints: Gemma 3n, Gemma 4 and MiniCPM-o classify as audio-capable and stream transcriptions, while an image-only VLM and a text model report no audio capability. Coverage spans a full inference worker subprocess run from load through streamed tokens to completion, and a load request returning audio capability for Gemma 3n and none for a text control.
Phi-4-multimodal was sent to the text loader, which has no implementation for it, so loading failed with "Model type phi4mm not supported" — a message about the wrong thing, since the model is multimodal and mlx-vlm implements it.

Nothing in its config says so. It keeps its image settings under embd_layer, declares itself ForCausalLM, and exposes none of the keys that mark a vision model, so every structural signal misses it. The curated list of remote-code vision types exists for exactly that case, alongside Phi-3-vision, which is the same shape.

With this it loads as the vision model it is, and its audio input works too: unsloth_zoo feeds processors that take (samples, rate) pairs the payload they accept, which is what this processor requires.
@Lyxot
Lyxot requested a review from danielhanchen as a code owner July 31, 2026 17:11
@Lyxot

Lyxot commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b7669c9fd8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread studio/backend/core/inference/mlx_inference.py
@Lyxot

Lyxot commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b7669c9fd8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

)

logger.info("MLX audio-input generating: prompt_len=%d", len(prompt))
with self._generation_lock:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor adapter selection for MLX audio requests

When an audio-capable LoRA is used in Base-vs-LoRA compare mode, the frontend sends use_adapter=false for the base side, but the audio route does not forward that field and this generation block never enters _temporary_mlx_adapter_state. Consequently both sides run the loaded adapter and can produce identical adapter-backed transcriptions; propagate the request's adapter selection through the audio worker protocol and apply it for the duration of this stream.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in 57d2502. The scenario is wire-reachable: chat-adapter.ts puts audio_base64 and use_adapter in the same request body, and resolveUseAdapter returns false for the base side of a compare pair. The audio branch of the route never read payload.use_adapter, orchestrator.generate_audio_input_response had no such parameter, and worker._handle_generate_audio_input never forwarded one, so both sides ran the loaded adapter.

Plumbed through all five layers:

  • routes/inference.py passes use_adapter = payload.use_adapter on the audio call, as the text branches already do.
  • orchestrator.generate_audio_input_response / _generate_audio_input_inner take it and set cmd["use_adapter"] only when it is not None, so the key stays absent for requests that do not select an adapter.
  • worker._handle_generate_audio_input forwards it only when present, matching the generate branch, so a backend without the parameter keeps working.
  • MLX: the stream is now wrapped in with self._generation_lock, _temporary_mlx_adapter_state(self._model, use_adapter), held for the whole stream rather than just the prompt build.
  • transformers: _apply_adapter_state(use_adapter) inside generate_fn under the generation lock, which is a no-op for None. That path had the same gap and was worth closing at the same time so the two backends behave identically.

Two regression tests cover it: one asserts the MLX backend enters the adapter context with the requested selection, one asserts the worker only puts use_adapter on the backend call when the command carries it. Both fail with the fix reverted.

…ake, and honor use_adapter on audio input

Three fixes on top of the MLX audio-input support.

1. The post-load mirror was stripping TTS and Whisper classifications on
   Apple Silicon. `_classify_mlx_audio_type` returned None for three
   different situations: "probed and this model has no audio input", "not a
   vision model, so never probed", and "the probe could not run at all". The
   worker mirrors the backend entry over the pre-load ModelConfig, so all
   three read as an authoritative negative and a snac/dac/bicodec/csm TTS
   checkpoint or a Whisper checkpoint came out of the load classified as a
   plain text model. On a Mac that turns the chat route's TTS redirect off
   and streams raw <custom_token_...> codec text into the conversation, and
   it turns off the "Whisper models require audio input" guard.

   The probe now takes the pre-load `audio_type` and only ever speaks for
   "audio_vlm". Anything it has no standing to judge is carried through
   untouched, and each silent early return now logs why. This also covers
   the case Codex flagged: on the currently released unsloth_zoo the
   capability helpers do not exist yet, the import raises inside the guard,
   and previously that missing dependency silently downgraded the model.
   Now it leaves the pre-load answer alone.

   Also switched the entry's `has_audio_input` to the existing
   `is_audio_input_type` helper instead of a second copy of the tuple.

2. Audio input ignored the request's adapter selection. Base-vs-LoRA compare
   sends `audio_base64` and `use_adapter` in the same body, but the audio
   route dropped `use_adapter`, so both sides of the comparison ran the
   loaded adapter. Plumbed it route -> orchestrator -> worker -> backend, and
   applied it for the duration of the stream: the MLX backend wraps the
   generation in `_temporary_mlx_adapter_state`, the transformers backend
   calls `_apply_adapter_state` under the generation lock. The worker only
   forwards the key when it is present, matching the text path, so the
   backend signatures stay compatible with callers that do not send it.

3. `test_autoload_records_backend_loaded_model_identity` was asserting on
   source literals the frontend hunk removed when it routed the summary
   through `syncModelCapabilities`. Updated the assertion to the new call.

Tests: `tests/studio` 2144 passed; `studio/backend/tests` failure set
unchanged against the pristine head. The new regression tests fail without
the corresponding fix and pass with it.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 57d2502d07

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +741 to +743
"is_audio": _audio_type is not None and _audio_type != "audio_vlm",
"audio_type": _audio_type,
"has_audio_input": is_audio_input_type(_audio_type),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Do not advertise audio modes absent from the MLX worker

When an Apple Silicon user loads an MLX-compatible TTS checkpoint such as a SNAC/CSM model, preserving _audio_type here sets is_audio=true, which makes the chat route call InferenceOrchestrator.generate_audio_response; that sends a generate_audio command, but the MLX fast-path command loop has no such branch and MLXInferenceBackend has no TTS implementation, so the request waits for the full 120-second timeout. The same mirroring advertises Whisper even though the MLX backend lacks generate_whisper_response, producing an immediate generation error. Either implement these MLX handlers or only publish the newly implemented audio_vlm capability.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch on the outcome, and fixed in e783e83, though the cause is not where you place it.

Verified the mechanism first. The MLX command loop handles generate, generate_audio_input, share_object, load, unload, cancel, reset, status and shutdown, and has no terminal else, so a generate_audio command was dropped without a reply. generate_audio_response then sat on deadline = time.monotonic() + 120.0 before raising Timeout waiting for audio generation (120s). Whisper is different: it rides the generate_audio_input command, which the loop does handle, and fails at backend.generate_whisper_response with a raw AttributeError. Both confirmed.

Where I disagree is the attribution. On main, _handle_load takes is_audio / audio_type / has_audio_input straight from the pre-load ModelConfig with no mirroring, so a snac checkpoint on Apple Silicon is already is_audio=true and already hits that 120s timeout today. Preserving the classification restores main's behavior; it does not create this. The alternative you suggest, publishing only audio_vlm, is what the mirror did before 57d2502, and it is worse than the timeout: the TTS redirect stops firing, so mlx-lm serves the checkpoint as a plain text model and streams raw <custom_token_...> codec text into the conversation, and the TTS panel disappears. Trading a loud failure for silent wrong output is not an improvement.

So I took your first option, scoped to what this PR can honestly own. The MLX loop now answers generate_audio immediately with "Text-to-speech is not supported on the MLX backend yet", an unhandled command type falls into the same terminal else the GPU loop has always had, and the whisper branch is behind a hasattr check with a plain reason instead of an AttributeError. Both replies carry the request_id, which matters: _direct_reader routes by it while a dispatcher thread is alive, so an unaddressed error would be discarded and the caller would wait out the deadline regardless.

Implementing real MLX TTS and ASR is the right end state, but it is a separate piece of work, not a precondition for shipping audio input.

danielhanchen and others added 2 commits August 2, 2026 16:05
…g them

Follow-up to the classification fix. Keeping a TTS or Whisper classification
on Apple Silicon is correct, and matches main, but it leaves the user facing
the pre-existing gap that MLXInferenceBackend has no TTS and no ASR
implementation. Inference dispatch is by device rather than by modality
(`_hw.DEVICE == DeviceType.MLX`), so a snac/dac/bicodec/csm checkpoint or a
Whisper checkpoint does reach the MLX worker.

Two of those failures were badly behaved:

- `generate_audio` had no branch in the MLX command loop and no terminal
  else, so the command was dropped in silence and
  `generate_audio_response` sat on its 120s deadline before raising
  "Timeout waiting for audio generation". It now answers immediately with a
  reason, and an unhandled command type falls into the same terminal else
  the GPU loop already has. Both replies carry the request_id, without which
  the direct-reader mailbox discards them while a dispatcher thread is
  running and the caller waits out the deadline anyway.

- Whisper reached `backend.generate_whisper_response` unguarded, so the user
  got a raw AttributeError naming an internal method. It is now a hasattr
  check with a plain reason. The guard is backend-agnostic, so it costs the
  transformers path nothing.

Tests: five new cases drive the real MLX command loop with its init
short-circuited and assert every command produces exactly one addressed
reply. All five fail with the three guards removed. Full backend suite
failure set unchanged against the pristine head (19); `tests/studio` 2144
passed.
danielhanchen added a commit to shimmyshimmer/unsloth-staging-4 that referenced this pull request Aug 2, 2026
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 2a619c737e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Collapse the multi-line explanations on the audio adapter forwarding, the
MLX audio classification probe, and the MLX command loop branches down to
one or two lines each. Comments only, no behaviour change.
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

@danielhanchen
danielhanchen merged commit 2419ff1 into unslothai:main Aug 3, 2026
47 of 50 checks passed
danielhanchen added a commit that referenced this pull request Aug 3, 2026
…xt one (#7768)

* Give the auto-load harness the adapter's new import, and catch the next one

main is red again: tests/studio/test_chat_autoload_failure_gate.py fails three
scenarios on every PR since #7699, unrelated to what those PRs change.

The harness slices autoLoadSmallestModel verbatim out of chat-adapter.ts and
supplies stubs for everything it imports. #7699 replaced an inline setModels
block in that region with a call to syncModelCapabilities, imported from
../hooks/use-chat-model-runtime, and the harness has no such stub. So the call is
a bare ReferenceError, the retry loop around it catches that and scores it as a
failed load, and a healthy model looks broken:

  assert _loaded_paths(out) == [GEMMA_REPO]
  AssertionError: assert ['unsloth/Qwen3.5-4B-MTP-GGUF', ...] == ['unsloth/gemma-...']

Nothing in the failure says what is actually missing, which is the worse half of
this. So the harness now checks, before running anything, that every name
chat-adapter.ts imports and the sliced region calls is defined here, and names
the ones that are not.

That check found two more of the same waiting: prepareHfTokenForUse and
fetchGgufStagedMetadata. Both sit behind a GPU-selection branch no scenario
currently takes, so they are unreached rather than broken, but they would have
become this same bug on the first scenario that turned a GPU option on. Stubbed
too.

Bisected: 8b8eba6 passes, 2419ff1 (#7699) fails.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Match the real metadata shape, and flag a name that is used but never called

Three defects in the previous commit, found reviewing my own change.

The fetchGgufStagedMetadata stub returned is_diffusion. The call site reads
.isDiffusion, and chat-api.ts returns camelCase throughout, so the stub yielded
undefined and fed it into reconcilePersistedGpuIds and the load payload. That is
exactly the silent misbehaviour the comment above it claimed to be preventing,
which is a good argument for checking a stub against the call site rather than
against the name.

The guard only looked for a name followed by a paren, so it could not see an
import used any other way. useChatRuntimeStore, toast and GPU_LAYERS_AUTO are all
used in this region without one, and each is the same ReferenceError waiting to
happen; they pass today only because the preamble already defines them. It now
flags any mention, with comments stripped first so a name discussed in prose does
not count, and `import { type Foo }` specifiers dropped since they are erased at
runtime and can never be a ReferenceError.

The definition check rejected export-prefixed declarations, which is how half of
this preamble is already written, so the natural fix to a future failure would
itself have been refused. It accepts export and class now.

Checked that removing a bare-identifier stub is caught, which the previous
version missed entirely, and that an export-form definition is accepted.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Read default, namespace and mixed import forms in the guard

The guard only parsed `import { ... } from`, so a default or namespace binding
never entered the set and its missing stub would pass the check. chat-adapter.ts
has none today, which is why this was latent rather than broken, but a guard that
silently declines to look is the failure mode it exists to prevent.

Adding those turned up a third gap: the braced pattern anchored on `^import\s+\{`,
so the mixed form `import def, { named } from` had its braces skipped as well.

Verified against all of them at once: default, namespace, mixed, plain braced and
multi-line braced are captured; `import type { T }` is still ignored, since type
specifiers are erased at runtime and can never be a ReferenceError.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
danielhanchen added a commit that referenced this pull request Aug 19, 2026
…t guard it (#9189)

* Repair the two chat auto-load suites that #9173 left red on main

#9173 added a vision-projector fallback to the auto-load success toast. Both
of the suites that read that code broke on it, and both were green on the
commit immediately before (54b6ca4).

72 tests, in two different shapes:

1. tests/studio/test_chat_autoload_failure_gate.py, 71 failures. The harness
   slices autoLoadSmallestModel verbatim out of chat-adapter.ts and runs it, so
   anything the slice references must exist in PREAMBLE. mmprojFallbackMessage
   did not, which is a bare ReferenceError inside the retry loop; the loop
   catches it and scores it as a failed load, so every scenario fails as a
   wrong-model assertion. The file's own guard caught this and named the symbol,
   which is what it was written for after #7699 did the same thing.

   Stubbed as a function of the reason rather than a copy of the real
   three-message record. The value only reaches `options.description`, and these
   scenarios assert on which model loaded, never on toast copy, so copying
   user-facing strings in here would give them a second home to drift from.

2. tests/studio/test_model_picker_contracts.py, 1 failure. It asserted the
   literal `description: cpuFallbackReason`, and the mmproj branch went in front
   of it. The property held; only the spelling moved. Its own comment records
   this happening once already, when the CPU-fallback branch first appeared, so
   it now pins the property: the description varies on both fallback reasons and
   still has an undefined arm for the ordinary path.

   Scoped to the description EXPRESSION, not the whole helper. `cpuFallbackReason`
   is also a parameter name in the signature above, so a substring test over the
   block stays green with the CPU branch deleted outright -- the first cut of this
   check did exactly that, and mutation caught it.

Three mutations verified red: the description no longer driven by any fallback
reason, the CPU branch dropped, and the mmproj branch dropped.

4223 passed, 4 skipped.

* Say both fallbacks when both fire, not just the projector one

Both load paths wrote the toast description as

  mmprojFallbackReason ? mmprojMessage : cpuFallbackReason ? cpuMessage : undefined

so whenever both reasons are set the CPU-fallback sentence is dropped. The user
is told "loaded without vision" and never told the model is running on the CPU,
which reads as a deliberate, explained degradation rather than an unaccelerated
session.

The combination is reachable. On a CPU-fallback replay llama_cpp.py preserves
_cpu_fallback_reason (it clears it only when not _replaying_cpu_fallback) and
resets _mmproj_fallback_reason so the projector can fail again inside that same
launch. A low VRAM machine whose Vulkan backend crashed is exactly where the
projector then falls back too.

loadFallbackNotice() in mmproj-fallback.ts is now the single composition of the
title suffix, the description and the degraded flag, and both call sites delegate
to it. CPU_FALLBACK_MESSAGE moves there as well, so the two paths cannot describe
the same condition differently again.

Tests: four combined-case cases in mmproj-fallback.test.ts, verified red against
the shipped nested ternary. test_model_picker_contracts.py now asserts the call
sites delegate and pass both reasons rather than matching the old inline ternary,
and test_chat_autoload_failure_gate.py's stub mirrors the composition so a call
site dropping a reason stays detectable.

* Repin the CPU-fallback toast test to behaviour, and unbreak the queued-capabilities test

Two frontend suites were red.

auto-load-cpu-fallback-toast.test.ts matched the warn-vs-success choice and the
message text as substrings of showAutoLoadSuccess. Both moved into
loadFallbackNotice, which is the single definition the explicit-load path now
shares, so the match went stale. Matching the inline form again would go red on a
refactor that changes nothing a user can see, and would stay green if only one of
the two load paths kept the behaviour. It now calls loadFallbackNotice and asserts
the verdict, and separately asserts the call site delegates to it.

queued-model-capabilities.test.ts was red on main before this branch. #9173 added
`import { isTextOnlyMmprojFallback } from "./mmproj-fallback"` to
image-input-support.ts, which the test imports statically. Extensionless is the
right form -- 2314 of the 2367 relative imports under src/ are written that way,
and vite and tsconfig's "bundler" mode resolve them -- but the bare node loader
does not, and a static import resolves before any registration can run. The test
now registers the bundler resolver and imports dynamically, which is what
mmproj-fallback.test.ts already does for the same module.

Both files reformatted by biome; regex literals hoisted out of the test bodies
for useTopLevelRegex.

Full frontend suite: 3734 pass, 0 fail. typecheck clean. The 4 biome errors in
chat-adapter.ts and use-chat-model-runtime.ts are byte-identical on origin/main.

* Order the split-axis abort against the mmproj strip, not against its argument

test_tensor_split_abort_raises_early_to_layer_fallback has been red on main
since #9173, which renamed the text-only strip's argument from _last_spawn_cmd
to _vision_gpu_cmd. That rename is right: the strip should read the vision GPU
command rather than whatever was spawned last, and #9173 refreshes
_last_spawn_cmd from the result immediately after. The test was pinned to the
old argument name, so a rename with no behavioural content took it down.

The failure also misreported itself. `assert raise_idx < src.find(needle)` reads
as an ordering check but is two claims at once, and when the landmark is gone it
fails with "assert 249423 < -1" -- which says the ordering broke, when what
happened is that the landmark moved. Each landmark is now required to exist
before it is ordered, and says so.

The strip is matched on the call rather than on what is passed to it. What this
test is about is that the abort raises BEFORE the projector is discarded (#6659);
which command the strip reads from is that code's own business.

Checked both ways: removing the strip call from load_model goes red with a
message naming the missing landmark, and renaming the argument again stays green.

* Name the endpoints when the heavy-thread harness catches a stray request

The harness records every /api/ URL issued during a measured action, then keeps
only the count, so the failure reads

  let 2 /api/ requests reach the network during the measured actions

and stops there. It says an interaction paid for a round trip without saying
which one, and the reader has to bisect the frontend to learn what the harness
already knew and discarded.

It now reports the endpoints. Deduplicated and capped at eight, because the case
this instrument exists to catch is a request issued once per message, which would
otherwise print hundreds of copies of one line.

This is why it surfaced now: the step has not run on main since #9173, whose unit
test break fails earlier in the same job and short-circuits it. It was last green
at 54b6ca4. With the unit tests repaired on this branch the job reaches the
step again, and the first thing it needed to say was the one thing it did not.

* Skip the playwright harness tests on the module they need, not the package

Five tests guard themselves with `pytest.importorskip("playwright")` and then
import a harness that does `from playwright.sync_api import Page`. On the Repo
tests (CPU) runner the top-level name resolves as a namespace directory with no
sync_api inside it, so the guard passes and the import dies with

  ImportError: cannot import name 'Page' from 'playwright.sync_api' (unknown location)

A skip condition reported as a failure, on every branch, for as long as that
runner stays that way. It is red on #9202 and #9213 too, neither of which touches
any of this.

One of the two files already said what the guard was really for: "importing a
harness pulls in playwright.sync_api". It now checks that.

* Stop the fork-count store asking the server about threads it has never seen

Two fixes, both found by the heavy-thread smoke once it could name what it caught.

The smoke reported "let 2 /api/ requests reach the network during the measured
actions" and, with the endpoints now printed, they were

  POST /api/chat/threads/__LOCALID_lsQbsDZ/forks

A `__LOCALID_` thread has no server record, so that request can only 404, and
getThreadForkCounts already maps 404 to the empty map the entry starts as. It is
a round trip whose answer is known before it is sent.

Not a rounding error. A new chat is in exactly that state, and this store
refreshes on CHAT_HISTORY_UPDATED_EVENT, which fires once per streaming chunk, so
the first reply in a new chat paid one useless request per debounce window for as
long as it streamed. #8992 added the store to stop the chat getting slower as a
thread fills; excluding threads the server has never seen is the same intent.
thread-ids.ts already had the predicate.

Two tests: a local thread must not fetch on subscribe or on a burst of history
events, and a saved thread on screen beside it must still refresh -- the guard has
to be per thread, not a global off switch. They import the real
isAssistantLocalThreadId rather than restating the prefix, so the rule under test
cannot drift from the app's. Both go red with the guard removed.

Second fix, same job: the playwright skip guard. The previous commit moved it from
"playwright" to "playwright.sync_api" and it still failed, because sync_api
resolves as a namespace package on that runner too. Only the symbol the harnesses
import distinguishes a usable install, so the guard now checks for Page the way
the harness does. Verified both ways against a Page-less sync_api: it skips, and
it still proceeds when Page is there.

Frontend suite 3789 pass, typecheck clean, tests/studio 4237 pass.

* Bound and retry the Playwright browser install so a stall is not a silent 30 minutes

This step stalls. Three times in one day it sat in apt's download loop until the
job's 30-minute timeout killed it, while the sibling shards finished the whole job
in 4 to 9 minutes. Twice on #9202 and once on #9189, always the same step.

The cost is out of proportion to the cause. GitHub scores a job timeout as
"cancelled" rather than a failure, prints no reason, and skips every step after
it, so the chat shard reported nothing about the chat surface for what both times
turned out to be an infrastructure hiccup that cleared on a plain re-run of the
same commit.

A per-attempt `timeout` turns the stall into a failure instead of a silent wait,
and the retry is what actually recovers. The healthy time is about 2 minutes, so 8
per attempt is 4x headroom and a merely slow mirror will not trip it.
timeout-minutes bounds the pair in case `timeout` is outlived by an unkillable
child. Same reasoning, and the same wording, as the bounded prime-hf step in
studio-mac-ui-smoke.yml.

Both install steps in this file, since ui-smoke and ui-indicator run the identical
command. Left alone on mac and windows: neither passes --with-deps, so neither has
the apt phase this is about, and neither has been observed to stall.

* Make the Playwright install retry able to actually recover

The bound added in the previous commit worked: the stall became an 8m37s step
FAILURE with a complete log instead of a silent 30-minute cancellation, and the
log named the cause on the first try.

It also showed the retry could not work. playwright shells out to apt-get as root,
so terminating the python parent leaves that child alive holding the lock, and
attempt 2 died two seconds later with

  E: Could not get lock /var/lib/dpkg/lock-frontend. It is held by process 4578 (apt-get)

A retry that cannot succeed is worse than no retry: it buries the real reason
under a second, different failure. Attempt 2 now waits for the lock to clear, up
to two minutes, and only then takes it -- the holder is our own orphan and the
runner is a throwaway.

Two smaller things the same log exposed. --kill-after was missing, so a process
that ignores SIGTERM would have been waited on forever inside the step bound. And
the warning said "did not finish within 8 minutes" about a two-second exit, which
sends the next reader looking for a stall that never happened; it now separates
timeout's 124/137 from playwright refusing outright, and prints the status.

timeout-minutes 18 to 22 to cover two 8-minute attempts plus the lock wait, still
inside the job's 30.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants