Stub the implicit STT load in the disconnect-cancellation test by danielhanchen · Pull Request #9031 · unslothai/unsloth · GitHub
Skip to content

Stub the implicit STT load in the disconnect-cancellation test - #9031

Merged
danielhanchen merged 2 commits into
mainfrom
fix-stt-cancel-test-stubs-load
Aug 17, 2026
Merged

Stub the implicit STT load in the disconnect-cancellation test#9031
danielhanchen merged 2 commits into
mainfrom
fix-stt-cancel-test-stubs-load

Conversation

@danielhanchen

@danielhanchen danielhanchen commented Aug 16, 2026

Copy link
Copy Markdown
Member

test_disconnected_raw_transcription_cancels_its_sidecar passes in its file and fails on its own, on any machine that has not downloaded an STT model:

tests/test_stt_transcription_cancellation.py:190
    assert raised.value.status_code == 499
E   assert 409 == 499
E    +  where 409 = HTTPException(status_code=409, detail="STT model 'small' is not
       downloaded. Download it in Settings, then Voice, before loading it.").status_code

Cause

_transcribe_audio_result does an implicit load through the registry before it ever reaches the sidecar:

load_stt, _ = _stt_lifecycle()
await asyncio.to_thread(load_stt, model, serving_engine, cancel_event)

and the registry refuses unless a complete snapshot is on disk:

snapshot_path = _find_complete_cached_snapshot(model_id)
if snapshot_path is None:
    raise SttModelNotDownloadedError(...)

The test stubs _stt_sidecar_for and _resolve_serving_stt_engine, but not the load. So the disconnect it exists to test is never reached. It gets there only when an earlier test in the file has left an engine resident, or when the host happens to have the model cached.

The sibling test directly above it already stubs _stt_lifecycle for exactly this reason:

monkeypatch.setattr(inference_route, "_stt_sidecar_for", lambda _engine: sidecar)
monkeypatch.setattr(inference_route, "_stt_lifecycle", lambda: (load, None))

Fix

Do the same. Cancellation, not download state, is the subject of this test.

Verification

Forcing the no-snapshot condition (_find_complete_cached_snapshot returning None, which is what a runner that has never downloaded an STT model looks like):

result
main, this test alone 1 failed (409 == 499)
main, whole file 9 passed
this PR, this test alone 9 passed
this PR, on a host that does have the model cached 9 passed

The gap between rows one and two is the order dependency.

Which PR, and which side is wrong

Both this test and the implicit registry load it trips over came from the same PR, #7984 (d20db3f1f, 2026-08-11). The test was incomplete from birth, and the sibling test directly above it, added by that same PR, does stub _stt_lifecycle.

Worth flagging: #7984 is also the PR behind the stt_unload 500 in #9013, and behind the two test_stt_ggml_sidecar.py unload tests that fail standalone on main. Three defects in one area, each invisible because a serial run happened to leave the right state behind.

Robustification

The stub now records what it was asked to load, and the test asserts it. Otherwise, if the route ever stops loading through the registry, the stub silently becomes dead code and the test stays green for a path it no longer covers. Mutating the route to skip the load:

AssertionError: the transcribe path did not load through the registry: []

Found while measuring whether the backend suite can run under pytest-xdist. --dist load splits the file across workers, so the test lands on a worker where nothing loaded an engine first and it fails. The dependency is on main today though, which is why this is a standalone fix.

test_disconnected_raw_transcription_cancels_its_sidecar passes in its file and
fails on its own, on any machine that has not downloaded an STT model:

  assert raised.value.status_code == 499
  E assert 409 == 499
  E  + where 409 = HTTPException(status_code=409, detail="STT model 'small' is
       not downloaded. Download it in Settings, then Voice, before loading it.")

Introduced by #7984 (d20db3f, 2026-08-11), which added this test AND the
implicit registry load in _transcribe_audio_result it fails on. The test was
incomplete from birth: it stubs _stt_sidecar_for and _resolve_serving_stt_engine
but not the load, so the disconnect it exists to check is only reached when an
earlier test left an engine resident or the host has the model cached.

The sibling test directly above it, added by the same PR, does stub
_stt_lifecycle. This does the same. Cancellation, not download state, is the
subject.

Worth noting that #7984 is the same PR behind the stt_unload 500 (#9013) and the
two test_stt_ggml_sidecar unload tests that fail standalone: three defects in one
area, each invisible because a serial run happened to leave the right state
behind.

Robustification: the stub now records what it was asked to load, and the test
asserts it. Otherwise, if the route ever stops loading through the registry, the
stub becomes dead code and the test stays green for a path it no longer covers.
Checked by mutating the route to skip the load:

  the transcribe path did not load through the registry: []

Verified by forcing the no-snapshot condition:

  main, test alone:  1 failed (409 == 499)
  main, whole file:  9 passed   <- the order dependency
  fixed, no snapshot: 9 passed
  fixed, cached host: 9 passed
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: 2a2e1d1616

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@danielhanchen

Copy link
Copy Markdown
Member Author

@danielhanchen
danielhanchen merged commit ba1e956 into main Aug 17, 2026
33 of 35 checks passed
@danielhanchen
danielhanchen deleted the fix-stt-cancel-test-stubs-load branch August 17, 2026 04:46
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.

1 participant