Repository navigation
feat(OMN-14018): per-run backend pin on context-ROI runner + generation-consumer honor path (ROI tier-flip enrichment) - #1636
Conversation
…on consumer honors it The context-ROI battery only ever routed to the contract-declared local tier, so context_roi_scores held a single tier (488 local rows, 100% success). To capture genuine per-tier graded outcomes (a real sub-floor cheap_cloud cohort for the demonstrable escalation-skip flip, OMN-14011 Path 1) a run must be pinnable to a chosen backend. Runner side: - ModelContextRoiRunRequest.target_endpoint (optional, default empty). - HandlerContextRoiRunner threads it onto every generation command as forced_endpoint_ref (only when non-empty; empty preserves the command shape). - run_context_roi_battery.py --target-endpoint flag → target_endpoint. Generation-consumer side (so the pin is REAL, not an inert field): - ModelNodeGenerationRequest.forced_endpoint_ref (optional, default empty). - handle() pins the STARTING route to the forced backend's tier via _forced_starting_route, which resolves the concrete model+endpoint through the SAME routing authority (delta with min_tier_name) the escalation path already uses. Fail-safe: an unresolvable ref keeps the contract-declared route (never crashes). Empty ref = byte-identical to prior behavior. Tests (5 new, all through the real routing authority — no mocked routing): - runner threads/omits forced_endpoint_ref by target_endpoint presence; - forced cloud pin makes generation attempt #1 post to the CLOUD tier's model+endpoint (vs contract-local); empty preserves local; bad ref falls back to local without crashing. 510 pass across gen-consumer + runner + scripts. - ruff/mypy --strict clean; check-no-faked-boundary + leaked-literals gates pass. NOTE (honest scope): this lands the enrichment MECHANISM. The live sub-floor capture (≥5 real cheap_cloud rows) + observed escalation-skip A/B is gated on OMN-14001/#1629 merged AND the stability-lane runtime redeployed with this code + OMNIDASH_ANALYTICS_DB_URL set (the ticket's own Blocked-by). No synthetic rows were authored. Refs OMN-14018, OMN-14011.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 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_context_roi_runner/handlers/handler_context_roi_runner.py:285: result = HandlerContextPackBuilder().handle(request)
src/omnimarket/nodes/node_generation_consumer/handlers/handler_generation_consumer.py:1112: self._effect = HandlerLlmOpenaiCompatible(transport=_Transport())\n```\n\nHandler classes should be wired only in workflow_runner.py or adapter modules. Direct instantiation elsewhere bypasses the contract system.\n\n
Powered by node_pr_review_bot
✅ PR Review Bot — CLEANFindings: 0
Verdict semantics
Powered by node_pr_review_bot — Qwen3-Coder-30B reviewer + DeepSeek-R1 judge (OMN-7963) |
…te-coverage gate)
Touching node_context_roi_runner activates the contract-state-coverage gate for
it; the two declared terminal output states were asserted only by substring, so
the gate saw them uncovered. Assert the full literal topic names
(onex.evt.omnimarket.context-roi-run-{completed,failed}.v1) in the two emit tests
-- a stronger assertion (exact topic, not substring) that covers both states.
…erlay-countable capture)
_forced_starting_route left endpoint_ref="" (mirroring escalated routes), so the
attempt + run-level benchmark recorded the bare TIER NAME (e.g. 'cheap_cloud') as
endpoint_class. The context_roi_scores row's endpoint_ref = that endpoint_class,
and the ROI overlay maps a row to a tier via tier_for_backend(endpoint_ref) which
resolves a BACKEND id ('cloud-glm' -> cheap_cloud) but returns None for a bare
tier name -> every captured per-tier row would be silently DROPPED, yielding zero
usable sub-floor rows for the OMN-14011 Path-1 flip.
Fix: carry the forced backend id on endpoint_ref. Routing is unaffected —
_call_llm posts to endpoint_url when authority_resolved is True (endpoint_ref is
attribution-only there). Tests now assert the attempt AND run-level benchmark
record the backend id as endpoint_class.
Refs OMN-14018, OMN-14011.
|
| Verdict | Meaning | Blocks merge? |
|---|---|---|
passed |
No critical/error findings | No |
blocked |
CRITICAL or ERROR findings found | Yes |
degraded |
All models unavailable (infra) | No (pilot) |
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)
Merge commits are not allowed on this repository
…-backend-override
…-backend-override # Conflicts: # tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_rejected_content_logged_omn14004.py
1 participant
Evidence-Source: OCC#3652
OMN-14018 — per-run backend pin on the context-ROI runner (enables the demonstrable tier-FLIP)
Design authority:
docs/plans/2026-07-06-OMN-14011-roi-enrichment-design.md§6. Parent OMN-14011.Problem
context_roi_scoresis fed only by the context-ROI battery, which always routes to thecontract-declared local tier — so the table is single-tier (488 local rows, 100% success) and
nothing is below the 0.5 ROI floor. Path 1 (the honest escalation-skip flip) needs a real
sub-floor
cheap_cloudcohort, which requires running the battery pinned to a genuinely-failingcloud backend.
Change — the enrichment MECHANISM (real routing, real grading, no fakery)
Runner side
ModelContextRoiRunRequest.target_endpoint(optional, default empty).HandlerContextRoiRunnerthreads it onto every generation command asforced_endpoint_ref(onlywhen non-empty; empty leaves the command shape byte-identical).
run_context_roi_battery.py --target-endpoint→target_endpoint.Generation-consumer side (so the pin is REAL, not an inert field)
ModelNodeGenerationRequest.forced_endpoint_ref(optional, default empty).handle()pins the STARTING route to the forced backend's tier via_forced_starting_route, whichresolves the concrete model + endpoint through the same routing authority (
deltawithmin_tier_name) the escalation path already uses. Fail-safe: an unresolvable ref keeps thecontract-declared route (never crashes). Empty ref = byte-identical to prior behavior.
Evidence
forced_endpoint_refbytarget_endpointpresence;cloud-gemini-flashpin makes generation attempt feat(merge-sweep): add failure history tracking and Track A-resolve [OMN-7573] #1 post to the CLOUD tier's model +endpoint (vs the contract-local start); empty preserves local; a bad ref falls back to local
without crashing.
uv run pytest tests/unit/nodes/node_generation_consumer/ tests/nodes/node_context_roi_runner/ tests/unit/scripts/→ 510 passed.mypy --strictclean; full pre-commit clean (incl.check-no-faked-boundaryOMN-13502 +leaked-literals OMN-10580).
Honest scope — what is NOT in this PR
This lands the mechanism. The live sub-floor capture (≥5 real graded
cheap_cloudrows withsuccess_rate < 0.5) and the observed escalation-skip A/B on the stability runtime are gated onthe ticket's own Blocked-by: OMN-14001/#1629 merged AND the stability lane redeployed with this
code +
OMNIDASH_ANALYTICS_DB_URLset (a Maconex delegatecannot reach.201Postgres — LAN grant).No synthetic rows were authored; when #1629 lands and the lane is redeployed, the battery can be run
pinned to
cloud-glm/cloud-gemini-flashto capture the real cohort and observe the flip.Refs OMN-14018, OMN-14011.
Evidence-Ticket: OMN-14018