Skip to content

test(inference): pin the streaming request-slot lifecycle of the steer endpoints - #214

Open
Alexander230 wants to merge 1 commit into
hijohnnylin:mainfrom
Alexander230:fix/38-streaming-request-lock
Open

Alexander230 wants to merge 1 commit into
hijohnnylin:mainfrom
Alexander230:fix/38-streaming-request-lock

Conversation

@Alexander230

@Alexander230 Alexander230 commented Jul 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Rescoped after #237. The request-lock change this PR originally carried is superseded upstream: with_request_lock admits the handler, and the SSE generators take their own non-exclusive slot via stream_lock for the stream's lifetime. What was still missing is a test that pins that lifecycle, so this PR is now tests only — no production code changes.

apps/inference/tests/unit/test_steer_stream_slot.py drives /steer/completion and /steer/completion-chat with a stub eager backend (no model, no GPU) and checks that:

  • the handler's admission slot is released before the stream starts, and the generator's own acquire yields the first frame promptly — an admission slot held across the stream would deadlock the single mutex a non-vLLM pod runs with;
  • the slot is held while streaming and released when the stream is exhausted and when it is closed early (client gone), on both endpoints' generators;
  • a non-streaming request generates under the handler's slot and takes no second one.

Verified to bite: with stream_lock mutated to never take a slot, 5 of the 6 cases fail; with the non-stream path made to take a stream slot, the deadlock is reported as a TimeoutError rather than a hang. ruff and pyright clean.

Refs #38 (closed as not planned).

🤖 Generated with Claude Code

@vercel

vercel Bot commented Jul 15, 2026

Copy link
Copy Markdown

@Alexander230 is attempting to deploy a commit to the Neuronpedia Team on Vercel.

A member of the Team first needs to authorize it.

@Alexander230

Alexander230 commented Jul 15, 2026 •

Copy link
Copy Markdown
Contributor Author

CI status note, to save reviewer time triaging red checks — none of the failures on this PR are introduced by it:

  • Vercel — outside-PR deploy-authorization gate; this PR doesn't touch the webapp.
  • Inference - Tests: the workflow-repair commit in this PR (originally split out as Make inference CI runnable: drop the impossible 3.10 matrix leg, scope pytest to tests/ #215, consolidated here) removes the impossible 3.10 matrix leg and scopes pytest to tests/, so once approved the checks execute for real. The first still-red steps are pre-existing app-wide findings, byte-identical to main in the same environment (see table).
  • Codecov / inference-coverage fails on two pre-existing tests/unit/test_sae_manager.py cases (MockSAE.to() missing 'dtype' — sae-lens mock drift), red on main since February. This PR's tests pass in that job (tests/unit/test_request_lock.py ....).

Verification from a fork rehearsal of the exact workflow (Alexander230#1, runs 29406654972 / 29406654966, Python 3.11 + 3.12 on clean runners):

CI step (app-wide) main base this PR delta
ruff check . 2935 2934 −1
ruff format --check . 212 files 211 −1
pyright . 3723 3723 0
pytest tests 2 unit + 5 integration failures same failures +4 new passing tests

(The 5 integration failures are tokenizer/numeric drift, identical on main in the same environment.)

@Alexander230 Alexander230 changed the title Fix request-lock coverage for streaming steer completions (#38) Fix request-lock coverage for streaming steer completions + make inference CI runnable (#38) Jul 15, 2026
@Alexander230
Alexander230 force-pushed the fix/38-streaming-request-lock branch from 4a7406b to 6163a7a Compare September 11, 2026 17:35
@Alexander230 Alexander230 changed the title Fix request-lock coverage for streaming steer completions + make inference CI runnable (#38) test(inference): pin the streaming request-slot lifecycle of the steer endpoints Sep 11, 2026
…r endpoints

/steer/completion and /steer/completion-chat release the admission slot when the
handler returns and take a slot of their own inside the SSE generator (stream_lock)
for the stream's lifetime. Nothing covered that ordering. These tests drive both
endpoints with a stub eager backend and check that the handler's slot is free before
the stream starts, that the generator holds one while streaming, that it is released
on exhaustion and on early close, and that a non-streaming request generates under
the handler's slot without taking a second one (which would deadlock the single
mutex a non-vLLM pod runs with).

Refs hijohnnylin#38.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Alexander230
Alexander230 force-pushed the fix/38-streaming-request-lock branch from 6163a7a to 16d1170 Compare October 9, 2026 10:44

This branch has not been deployed

No deployments
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