Repository navigation
feat(auto-combo): add nadir router strategy for prompt-aware model choice - #13051
doramirdor wants to merge 4 commits into
Conversation
|
This is careful, well-engineered work — the fail-open guarantees, the outbound-fetch guard, and the minimal-egress design (last user turn only, capped, no history/tools) all check out reading the code. Given you've disclosed maintaining Nadir yourself, I want to flag this for the repo owner as a policy decision rather than a pure code review: this would be the first vendor-authored SaaS routing integration shipped as a core |
diegosouzapw
changed the title
nadir router strategy for prompt-aware model choicenadir router strategy for prompt-aware model choice
|
Thanks for the careful read. Env gap closed in 993501d. Both files, not just The same commit changes how the strategy reads them, which is the part worth your eye. They were read indirectly: const ENV_API_KEY = "OMNIROUTE_NADIR_API_KEY";
// ...
readString(process.env[ENV_API_KEY]);
The entry text describes the behavior rather than the intent: the key is optional (a keyless call goes out on Nadir's anonymous per-IP tier, which is not usable behind a shared gateway), and Checks: On the policy question: that is the right way to treat it, and I would rather it be decided explicitly than slip through as "just another strategy". Happy to leave this parked for however long the owner call takes, and I am not going to press it. If the answer is "not in core", the plugin seam ( |
|
Checking in, and to be clear: nothing here needs a code change from you. Since your last comment, #13056 merged today, thanks for taking that one through the train. On this PR the one concrete gap you named is closed in So the only thing outstanding is the policy question you raised, whether a vendor-authored SaaS routing integration belongs in core as a If it is a yes, say the word and I will rebase. The branch is 518 commits behind |
993501d to
073d00b
Compare
|
Rebased onto the current Two conflicts, both places where your tip and my branch appended to the same block:
Nothing else was reformatted, and no other file needed touching. Re-verified after the rebase:
One red that is not mine: Still no action needed from you on the code. The policy question from my last comment is the only open item. |
|
Flagging the one red check here before it reads as mine: It did not run on my previous head at all, so the rebase surfaced it rather than the change causing it. My head reports 3: A clean worktree of the untouched tip ( So my head's failures are a strict subset of the base's, and this PR touches none of those three files. Repro, symlinking Same story as |
diegosouzapw
added
the
deferred-v3.8.52
|
Re-homed to |
…oice
Every RouterStrategy ranks the candidates by their own telemetry; none of
them reads the request. The `nadir` strategy sends the last user turn and
the pool's model ids to Nadir's decision API (POST /v1/bucket) and routes
to the model it selects, leaving the connection choice for that model to
the rules strategy so quota, health and cost still decide which account.
- RouterStrategy gains an optional selectAsync; selectWithStrategyAsync
prefers it and runs every sync strategy unchanged. The auto path awaits
it (resolveAutoStrategy already runs in an async function).
- RoutingContext carries the raw messages and the combo's config.nadir
block ({ apiKey, baseUrl, timeoutMs }; env fallbacks
OMNIROUTE_NADIR_API_KEY / OMNIROUTE_NADIR_BASE_URL).
- Fail-open: timeout (2 s default), non-2xx, bad JSON, missing prompt or a
selection outside the pool resolve to the rules decision; a 30 s cooldown
after a failure bounds the cost of an outage to one timeout per window.
- Egress is the last user message (capped at 16k chars), the model ids and
a source tag; the call goes through safeOutboundFetch with the provider
outbound URL guard.
- Dashboard option, Zod schema entry, MCP enum value and docs.
`OMNIROUTE_NADIR_API_KEY` and `OMNIROUTE_NADIR_BASE_URL` are read by `nadirStrategy.ts` but were missing from `.env.example` and `docs/reference/ENVIRONMENT.md`, which both declare themselves a complete contract over every variable the runtime reads. `check:env-doc-sync` stayed green on the gap because the strategy read the two through string constants (`process.env[ENV_API_KEY]`), and the checker's scanner only matches literal `process.env.NAME` member access. Reading them directly instead puts both inside the gate: its code-var count goes 598 -> 600 and the contract is still reported in sync, so the entries can no longer drift away unnoticed. Both entries land in section 6 (Tool & Routing Policies) next to the other combo/routing knobs, and say what the code does: the key is optional, a keyless call lands on the anonymous per-IP tier, and the base URL is only needed for a self-hosted Nadir.
073d00b to
adf038a
Compare
|
Thanks for re-homing this to Two notes on the current head
The code side is done from my end. The open question is still the policy call on a vendor-authored strategy in core. Is that something you can decide in the v3.8.52 cycle? Happy to rebase onto the tip whenever you want it on the train. |
diegosouzapw
removed
the
deferred-v3.8.52
diegosouzapw
changed the title
nadir router strategy for prompt-aware model choicenadir router strategy for prompt-aware model choice
This branch has not been deployed
2 participants
Summary
Adds a
nadirauto-router strategy: the firstRouterStrategythat reads the request, not only the candidates.Every existing strategy (
rules,score,cost,latency,sla-aware,lkgp) ranks the pool by its own telemetry. The only difficulty signal in the engine (complexityRouter.ts) is a keyword heuristic feeding two low-weight scoring factors, so nothing today picks the model tier a prompt needs.nadirsends the last user turn plus the pool's model ids to Nadir's decision API (POST /v1/bucket) and routes to the model Nadir selects from that menu (simple→ cheapest capable,complex→ frontier). The connection serving that model is still chosen byrules, so quota, health and cost keep deciding which account; Nadir only decides which model.Opt-in via
config.routerStrategy: "nadir", configured withconfig.nadir: { apiKey, baseUrl, timeoutMs }(env fallbacksOMNIROUTE_NADIR_API_KEY/OMNIROUTE_NADIR_BASE_URL;baseUrlonly matters for a self-hosted Nadir).Design constraints, all covered by tests:
rulesdecision, with the reason prefixedNadirStrategy: fallback (…). Never a 5xx.source: "omniroute"tag leave the box. No system prompt, history, tools or headers. The outbound call goes throughsafeOutboundFetchwith the provider outbound URL guard.strategyreadsnadirin routing events only when Nadir actually made the choice; fallbacks report the fallback's name.Mechanics:
RouterStrategygains an optionalselectAsync. NewselectWithStrategyAsyncprefers it and runs every existing sync strategy unchanged;resolveAutoStrategy.tsawaits it (it already runs inside an async function).selectWithStrategyis untouched.RoutingContextcarries the rawmessagesand the combo'snadirblock, parsed inparseAutoConfig.AUTO_ROUTING_STRATEGY_VALUES(MCPomniroute_set_routing_strategyenum),ROUTER_STRATEGY_OPTIONS(combo builder dropdown) andcomboRuntimeConfigSchema(nadirblock,.strict()) learn the new value.Disclosure: I maintain Nadir. The strategy is opt-in and inert unless a combo selects it; nothing else in the routing path changes.
Related Issues
Validation
Change type: routing (Contribution Golden Path → Routing).
npm run lint(eslint on every touched file, zero findings)release/v3.8.51tipba597b6)Commands run locally:
test:combo:matrixon this branch: 15 pass / 12 fail. The same command on the untouchedba597b6fails the identical 12 cases (priority fallback, round-robin, least-used, random, strict-random, p2c, reset-aware, headroom, lkgp, DRR fairness, weighted, fusion): every one of them dispatches toclaude/claude-3-5-sonnet-20241022, which the model-lifecycle guard now rejects (Model "claude/claude-3-5-sonnet-20241022" was shut down on 2025-10-28 and cannot be routed automatically, HTTP 410), so theclaudetarget never answers. The twoautocases, the only ones that reach the changed code, pass on both. The fixture model id needs a refresh, which I left out of this PR to keep it scoped; happy to open a separate one.Live contract check (keyless, zero cost):
POST https://api.getnadir.com/v1/bucketwith a 6-modelmenureturnsselected_modelverbatim from the menu, plusbucketandconfidence; the strategy binds only to those three fields.Tests Added Or Updated
tests/unit/router-strategy-nadir.test.ts(new, 13 tests): happy path incl. request shape (URL,X-API-Key,menu,prompt, timeout), connection choice delegated torules, confidence clamp, OPEN-breaker exclusion, every fail-open path (unknown model, empty response, non-2xx, thrown error, bad JSON), no-network on textless requests, failure cooldown with a fake clock, base URL normalization (/v1suffix, invalid scheme), env fallbacks, prompt cap, syncselect()fallback,selectWithStrategyAsyncdispatch for async and sync strategies, helper units.tests/unit/router-strategies.test.ts:listStrategiesexpectation now includesnadir.Coverage Notes
open-sse/services/autoCombo/nadirStrategy.tsis new and exercised end to end by the new test file through an injected transport (no global fetch mocking); the only untested lines are the default transport's two lazy imports ofsafeOutboundFetch/getProviderOutboundGuard. The 10-line additions torouterStrategy.ts,autoConfig.tsandresolveAutoStrategy.tsare covered by the same tests plus the existing combo matrix. No touched file loses coverage.Reviewer Notes
await selectWithStrategyAsync(...)inresolveAutoStrategy.ts; for every strategy other thannadirit resolves synchronously to the sameselect()result as before.config.nadir.apiKeylives in combo config likesla*andjudgeModeldo; operators who prefer to keep it out of the DB can useOMNIROUTE_NADIR_API_KEYinstead.