Repository navigation
feat(OMN-15156): thread optional backend_id pin through LocalDelegationDispatchPort.dispatch() - #1897
Conversation
…onDispatchPort.dispatch() dispatch() previously resolved the initial backend purely from task_type + tier_order via _resolve_initial_backend, with no caller-supplied pin. Add an optional backend_id kwarg (default None = byte-identical prior behavior) threaded dispatch() -> _resolve_initial_backend -> resolve_delegation_backend's existing backend_id kwarg (the same targeted-resolution path the LLM-judge already uses, OMN-13470). When a pin is supplied, tier_order/first_eligible_tier is bypassed entirely for the initial attempt; an unresolvable pin fails loudly via resolve_delegation_backend's existing RuntimeError (never a silent fallback). Escalation is unchanged: a transport failure on the pinned backend still excludes its whole tier via the existing tier_for_backend/next_eligible_tier loop, not just that backend -- documented in the PR body per the plan's P0 readback requirement, not redesigned here. RuntimeDelegationDispatchPort was read but not mirrored: its dispatch() does not call _resolve_initial_backend/resolve_delegation_backend at all (backend resolution happens downstream, off a ModelDelegationRequest with no backend_id field) -- adding an unhonored parameter there would be a silent no-op. Plan: docs/plans/2026-07-26-steel-node-dispatch-integration-plan.md (P0, hostile finding #4). Epic OMN-15154.
📝 WalkthroughWalkthroughLocal delegation dispatch now accepts an optional ChangesLocal delegation backend pinning
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
| Rule | Description |
|---|---|
| ARCH-TOPIC-001 | No hardcoded Kafka topic strings in handler code |
| ARCH-DI-001 | No event_bus=None bypass |
| ARCH-DI-002 | No direct Handler instantiation outside workflow_runner/adapters |
| ARCH-DI-003 | No reinvented DI containers |
| ARCH-TOPIC-002 | contract.yaml topics follow `onex.{cmd |
ARCH-DI-002: Direct Handler instantiation (warning)\n\n```\nsrc/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py:372: self._effect_handler = effect_handler or HandlerLlmDelegationCall()
src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py:373: self._projection_handler = projection_handler or HandlerProjectionDelegation()
src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py:393: self._judge = judge if judge is not None else HandlerJudgeAdequacy()\n```\n\nHandler classes should be wired only in workflow_runner.py or adapter modules. Direct instantiation elsewhere bypasses the contract system.\n\n
Static architectural lint — no model inference (OMN-14176).
✅ Hostile Reviewer — PASSEDBlocking findings (critical/error): 0 Gate semantics
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524) |
Summary
Threads an optional caller-supplied
backend_idpin throughLocalDelegationDispatchPort.dispatch()->_resolve_initial_backend->resolve_delegation_backend's pre-existingbackend_idkwarg.backend_iddefaults to
None, which preserves the exact pre-existing cheapest-firsttask_type+tier_orderresolution — a regression test proves thisbyte-for-byte.
Ticket: OMN-15156 (P0, epic OMN-15154). Governing plan:
docs/plans/2026-07-26-steel-node-dispatch-integration-plan.md(P0, hostilefinding #4 —
dispatch()had no caller-supplied backend pin; this ticket addsit).
Signature diffs
LocalDelegationDispatchPort(src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py):No other call sites were touched —
HandlerDelegateSkill.handle()/ProtocolDelegationDispatchPortdo not passbackend_idin this ticket(narrowly scoped to the port-level chain per the ticket description); a
non-
Nonepin is reachable today only via a directdispatch(..., backend_id=...)call (e.g. steel's futureLlmBusDelegationClient, or afuture CLI flag — out of scope here).
RuntimeDelegationDispatchPortwas read but NOT mirrored. Itsdispatch()shares the outer call signature (same kwargs, same
ProtocolDelegationDispatchPortduck-typed surface in
handler_delegate_skill.py), but it does not call_resolve_initial_backendorresolve_delegation_backendat all — it builds aModelDelegationRequest(frozen,extra="forbid", nobackend_idfield) andpublishes it to the bus; backend resolution happens downstream, off-repo-file,
in whatever consumes that command topic. Adding a
backend_idkwarg to thatport's
dispatch()today would be an unhonored parameter — a silent no-op thatlooks wired but isn't. Threading a pin onto the bus path is a separate, larger
change (new
ModelDelegationRequestfield + downstream consumer support) andis out of this ticket's scope.
Escalation-interaction note (verbatim from the code, P0 readback requirement)
This was verified by reading the escalation branch in
dispatch()(unchangedby this PR): on transport failure it computes
current_tier = tier_for_backend(backend.backend_id) or backend.tier,excluded_tiers.add(current_tier), then calls_resolve_next_backend(...),which re-resolves via
next_eligible_tier+backend_id_for_tier— thetier-order ladder, never back to the original pin. If the pinned backend's
tier cannot be classified by
tier_for_backend(an off-ladder backend_id),backend.tier(the resolved backend's own declared tier, e.g.localforlocal-coder-mlx) is used instead, so the exclusion is still tier-scoped, notun-scoped.
Tests
tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_backend_id_pin_omn15156.py(3 new tests, all passing locally):
test_pinned_backend_id_selects_exactly_that_backend_and_bypasses_tier_order— pin resolves the exact backend;
first_eligible_tieris monkeypatched toraise if invoked at all, proving the pin bypasses tier_order selection
entirely rather than merely agreeing with it.
test_backend_id_none_preserves_existing_tier_based_resolution— regression:both an explicit
backend_id=Noneand a caller that omits the (new,optional) kwarg entirely still drive the pre-existing
first_eligible_tier->backend_id_for_tier->resolve_delegation_backendchain, byte-identical to pre-OMN-15156 behavior.
test_unknown_backend_id_pin_fails_loudly_not_silently— uses the REALresolve_delegation_backend(onlyload_bifrost_backendsis patched to asmall in-memory list) against an unresolvable pin:
RuntimeErrorpropagatesout of
dispatch()verbatim, and the effect handler is never invoked (provesfail-closed, not a silent fallback to a different backend).
Verification
uv run ruff check/uv run ruff format --check— clean.uv run mypy src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py --strict— clean (also re-run via the pre-push hook's full strict pass — clean).tests/unit/nodes/node_delegate_skill_orchestrator/,tests/unit/delegation/,tests/integration/node_delegate_skill_orchestrator/):728 passed, 2 skipped, 1 pre-existing unrelated failure
(
test_local_dispatch_fork_safety_omn13842.py::test_dispatch_returns_typed_receipt_through_process_boundary— reproduced identically on unmodified
devviagit stash; a flakyunreachable.invalidDNS/timeout test, not touched by this diff).uv run pre-commit run --files ...): all 33applicable hooks passed;
topic-naming-lintinitially reported "Failed"only because
OMNIBASE_INFRA_PATHwasn't set in this shell (an env-pathresolution gap on this Mac,
pass_filenames: falseunrelated to the diffcontent) — re-ran with the var set and it passed clean; confirmed via
git commit's own pre-commit run (which correctly resolved the siblingpath) that this hook is green on the real commit.
git push's pre-push hook auto-escalated to the governed impacted-testselector's full local suite (
tests/nodes/ tests/unit/ --ignore=tests/integration -m 'not kafka', 5943 collected) becauseport_local_delegation_dispatch.pyis a widely-imported shared module — narrowing wasn't provably safe. Result:
5933 passed, 9 skipped, 1 xfailed, 0 failed (312.53s); push allowed.
port-signature-scoped (one new optional kwarg threaded through two methods)
.200because the pre-pushhook itself already ran the full local suite as part of the push (see
above) — that superseded the need for a second
.200full-suite run forthis small a diff.
Receipt / CI gates
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Evidence-Source: OCC#4918
Evidence-Ticket: OMN-15156
Evidence-Commit: d79b5a97be56334cfb56b645af7eb9e62ccd6e98