Repository navigation
feat(llm,rooms): model-affinity scheduler and full join-queue wiring (Phase 16) - #25
Merged
Merged
Conversation
…(Phase 16) Two pieces: the model-swap-aware LLM scheduler DECISIONS.md #13 had flagged for this phase (ModelAffinityScheduler, bounded per-base_url admission control), and the join queue's real seat/room wiring that docs/rooms.md and docs/capacity-and-queueing.md had documented as unbuilt since Phase 7 -- room-seat offers with anti-steal protection, global concurrent-game queueing with self-promotion, and frontend UI for queue position/ETA and accepting an offered seat. Found and fixed two real bugs along the way: a pre-existing Redis TTL race in JoinQueue's offer reclaim (never caught before because no test exercised a real wall-clock wait), and an advisory-lock leak in RoomService.start() that deadlocked a second session trying to acquire the same lock after the first raised without releasing it. Claude-Session: https://claude.ai/code/session_01RdYr89Z6wPoS79MWe7VAkC
python-reviewer (2 HIGH, confirmed with a live repro): - ModelAffinityScheduler.release() woke a waiter unconditionally, even when the released model still had other in-flight calls occupying its slot -- the woken waiter then self-admitted with no capacity re-check, letting the active set exceed max_loaded. Now only wakes when a model's count actually reaches zero, and acquire() re-checks capacity under the lock after being woken instead of blindly admitting. - force_start()'s human->agent seat backfill was staged, not committed, alongside start()'s own final commit -- but start() can now enqueue-and-roll-back on CapacityExceededError instead of only ever raising outright, and _promote_next_queued_room retries that exact room from a different session later. An uncommitted backfill was invisible to that retry, silently and permanently orphaning the room in the global queue. Now committed immediately in force_start(). _promote_next_queued_room also logs a suppressed retry failure instead of swallowing it silently. Also fixed: a real fairness window where leave()/_reclaim_expired_offers() wrote a freed seat to "open" before offering it to the queue, letting a concurrent non-queued joiner win it out from under the queue (MEDIUM); a `status` local shadowing the imported `fastapi.status` module (LOW). react-reviewer: missing aria-live on the queue/offer status panels in JoinByCode and the room detail page -- a screen reader user got no announcement when a seat became available (HIGH, accessibility). Claude-Session: https://claude.ai/code/session_01RdYr89Z6wPoS79MWe7VAkC
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two pieces of Phase 16 ("Scale and polish"), both scoped with the owner before building:
ModelAffinityScheduler, bounded per-base_urladmission control so LLM calls sharing one Ollama backend don't all fire at once and thrash model swaps. Wired transparently intoModelRegistry.client_for()via a new_ScheduledClientwrapper."offered"seat status), global concurrent-game queueing with self-promotion when a slot frees, and frontend UI for queue position/ETA and accepting an offered seat.Two real bugs found and fixed by the tests written for this feature (not assumed correct):
JoinQueue's offer reclaim (the offer key's own automatic expiry raced sweep()'s explicit reclaim for the same deadline) — never caught before because no existing test exercised a real wall-clock wait.RoomService.start()left a Postgres advisory lock held after raising, which deadlocked a second session trying to acquire the same lock — a genuine ~90s hang under a realtimeoutguard, not just a slow test.Left for later, explicitly (not silently): no standalone leader-elected sweeper task, no
seat_offer/queue_updatepush events — both were Phase 7's original plan. Built instead: opportunistic reclaim (a join retry or a queue-status poll both sweep as a side effect) and REST polling. Documented as a real, bounded gap indocs/capacity-and-queueing.md.Test plan
uv run pytest tests— full suite green (new tests for the scheduler, the queue wiring, and both bugs above)uv run prek run --all-filesclean (ruff, mypy, ty, biome, zizmor, SDK generation)bun run buildandbun run lintclean