dispatcher: retire Tier-4 legacy resolve_slot + add public SlotManager.state()#649
dispatcher: retire Tier-4 legacy resolve_slot + add public SlotManager.state()#649thinmintdev wants to merge 2 commits into
Conversation
…r.state() (closes #624) Remove the legacy path/name heuristics (proxy.py) from the Dispatcher's four-tier resolution order. Tiers 1-3 (registry → passthrough → cold-cache prefetch) now raise NoRouteFound directly on a miss; image-gen and embed models must have explicit registry bindings. Delete proxy.py and test_image_routing.py; update all SlotManager stubs in tests to expose .state() alongside the private ._current_state(). Add SlotManager.state(name) -> SlotState as a public API; switch both Dispatcher sites off the private _current_state() leak. Document the two orthogonal routing axes in ARCHITECTURE.md: Dispatcher = model-id→URL transport; route_for_request = capability→slot selection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thinmintdev
left a comment
There was a problem hiding this comment.
REVIEWER VERDICT: APPROVE — conditional on green CI (python checks pending; the rewritten dispatcher/integration tests are what validate this — certify on green). Sharp correctness read below; this is a clean Tier-1 refactor.
SPEC (#624): Deletes dispatcher/proxy.py (Tier-4 resolve_slot + LegacyResolutionFailed); adds public SlotManager.state(); 3-tier docstring rewrite; ARCHITECTURE two-axes note (transport vs capability). The proxy.py "do not delete until post-v0.2" caveat is now satisfied (v0.3 shipped; #624 sanctions it).
(a) No live importers — VERIFIED. Zero references to dispatcher.proxy/resolve_slot/LegacyResolutionFailed in src/ except the explanatory comment at router.py:548. proxy.py fully deleted, import removed from router.py.
(b) NoRouteFound preserves behavior — no silent route-drop. VERIFIED, and it is STRICTLY LOUDER than before. Old Tier-4 rule-7 silently routed any unmatched model → primary (a silent misroute: unknown/embed/image requests got misdelivered to the chat slot). New code raises NoRouteFound when Tiers 1-3 miss. So the deleted path WAS the silent drop; removing it eliminates the misroute. Two load-bearing confirmations:
- Capability axis intact:
omni_router/route_for_requestuntouched by this PR (only documented); live wiring inapi/__init__.py:1086-1115unchanged. - Image-gen unaffected:
/v1/images/generations(v1.py:995) callsget_provider("comfyui").infer()directly — it does NOT pass throughDispatcher.dispatch()tier resolution, so the deleted/images/*path-pins were redundant for the live flow. No image regression. - New operator-facing contract (note for merge-gate): bare-slot-name + path-pin addressing is gone — image/embed models must have explicit registry bindings. The rewritten
test_router.pyasserts exactly this (/embeddingsno-binding → NoRouteFound). The integration tests (test_serving_integration,test_composite_dispatch) confirm the runtime binding path — hence the on-green condition.
(c) SlotManager.state() delegation + coverage — VERIFIED. state() is a correct one-line delegate to _current_state (manager.py:317). Dispatcher now calls .state() EXCLUSIVELY (router.py:630, 665); every remaining _current_state caller is internal to manager.py — decoupling is complete. 3 new tests: offline-unknown, post-transition, agrees-with-private.
(d) Rewritten legacy-fallback tests assert correct new behavior — VERIFIED. The 3 router tests flip from resolution_path == "legacy_slot:*" to pytest.raises(NoRouteFound) + dispatch.no_route; the __cause__ is LegacyResolutionFailed assertion is removed; the legacy-only test_image_routing.py + the proxy non-regression test in test_v1_chat_slot_alias.py are deleted. Mock SlotManagers across 3 test files gained state() alongside _current_state. Coherent.
Merge-ready on green. ARCHITECTURE.md overlaps #647 — see merge-order flag in report.
|
Converted to draft — blocked: the audit premise (#624 “Tier-4 is legacy”) does not survive the test suite. Removing Tier-4 Retiring it requires migrating that resolution into tiers 1–3 first (a routing feature), not deleting it. The happy-path assertions are the spec and must stay 200 — do not weaken them. Holding for a maintainer decision on the migration path. |
Closes #624
Summary
dispatcher/proxy.pyand the legacy Tier-4 resolution block fromDispatcher.dispatch(). Tiers 1-3 (registry → passthrough → cold-cache prefetch) now raiseNoRouteFounddirectly when all miss. Image-gen and embed models must have explicit registry bindings (the seedhaloai_models.jsonalready has them).SlotManager.state(name) -> SlotStateas a public method; switch both Dispatcher call sites off the private_current_state()leak (router.py:669,704).ARCHITECTURE.md: Dispatcher = model-id→URL transport;route_for_request= capability→slot selection. Clarify thatomni_router/route_for_requestare untouched and load-bearing..state()matching the new public API.Files changed
src/hal0/dispatcher/proxy.pysrc/hal0/dispatcher/router.py_current_state→stateat both call sitessrc/hal0/slots/manager.pystate()method (delegates to_current_state)ARCHITECTURE.mdtests/dispatcher/test_image_routing.pyresolve_slotdirectly, now gonetests/dispatcher/test_router.pyNoRouteFoundassertions; addstate()to_FakeSlotManagertests/dispatcher/test_serving_integration.pystate()to_RecordingSlotManagertests/dispatcher/test_composite_dispatch.pystate()to_OfflineSlotManagertests/slots/test_manager.pySlotManager.state()tests/api/test_v1_chat_slot_alias.pyresolve_slotTest results
Rebase note
This branch is independent of #647 (voice provider retirement) in code — both touch
ARCHITECTURE.md, so a rebase conflict will land only there if #647 merges first. No dispatcher or manager.py overlap between the two branches.🤖 Generated with Claude Code