feat: an agent can be told which of its model's providers to run on #135
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/agent-routing"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Per-agent routing, the third axis of ADR 0015 beside the model and its reasoning effort. Asked for as "support nitro natively, all the way into the settings UI, to allow for faster work".
Rebased onto
mainat8fd4540now that #134 (the vendored-client bump bringing the typedModelIdthis needs) has merged — one commit,2a1f035, same diff as reviewed-against-that-branch. Full suite re-run after the rebase.What's in
Domain —
RoutingPreference { Balanced, Speed, Price, Accuracy }, domain-owned for the reasonReasoningEffortis: the roster's configuration vocabulary is ours, and the gateway maps it onto whatever the provider calls it (ADR 0003).AgentDescriptorgainsDefaultRouting, trailing and defaulted, so no roster row changes.Use cases —
SettingKeys.AgentRoutingFor(agents.<slug>.routing, built from the same frozenSlugmap), aRoutingSettingcodec with theEnum.IsDefinedguard its sibling has,SaveAgentRouting, theAgentSettingDtopair +EffectiveRouting, and the resolution inAgentAttemptSupport.PrepareAsync— without which the setting does nothing. No EF migration: the store is key/value, so this is one more row.SaveAgentRoutingtakes noILlmGateway, and the class doc says why: a preference is accepted on any model id and only reorders that model's endpoints, so there is no catalog fact it could contradict. It is the softer sibling ofSaveAgentEffortthe way that one is ofSaveAgentModel. The only refusal it can produce is a value outside the enum, which the picker cannot make — covered at the use-case level, since inventing a UI path for it would be inventing a lie.Infrastructure —
ToProviderModel(model, routing)besideToProviderEffort, written out the same way so a divergence on either side is a compile error. The suffix is applied only here: a routing variant is never a catalog entry of its own, so a suffix living in the stored model id would miss the exact-id catalog lookup inAgentAttemptSupportandSaveAgent*— vision would fail open and the user's effort would silently stop being sent. The resolved id is what the log line now names, and the round-cap debrief goes out on the same routing as the run it explains: same conversation, same provider, and a provider that just served it may still have the prompt cached.Settings UI — a third
Comboboxper row,Routing, offeringDefault — balanced,Balanced,Speed — nitro,Price — floor,Accuracy — exacto. It carries OpenRouter's own word for each because that is what the dashboard, the invoice and the docs use. It is the one picker on the row with noDisabledgate: it needs no catalog, so it works before a key exists. The card's paragraph and the per-row hint say what each lane costs — speed admits priority-tier endpoints that bill above the standard rate, price admits flex ones that wait for capacity. Own error dictionary, like its neighbours; grid to three columns, list to 60rem.Defaults stay balanced for every role, and the roster doc and a Domain test say why: the fast lane bills more and the cheap one queues, and that is not a trade to make on someone's behalf before they have seen a run. The lever is one click per agent.
Docs — ADR 0015 amended in place (the house style; it has been amended twice before),
docs/stories/settings.mdgains the story and its acceptance criteria, and AGENTS.md's "not seeded" list now names routing: per-agent settings are deliberately unseeded, and the sample world exercises the defaults path.Tests
1311 green across the four projects (151 + 592 + 277 + 291), +13 on main's 1298 — re-run after the rebase, not carried over from before it.
test/model,…:nitro,…:floor,…:exacto, read by parsing the captured request rather than matching letters in it; and a debrief going out on the same routed id as its run."99"→ null); and the key set extended, so a third facet per agent stays distinct and still throws off-roster.Balanced.Speedon one row storingagents.translation.routing = speedand touching no other row, the hint changing with it, and clearing removing the key; and a preference chosen with no key saved at all.Two existing tests changed rather than being worked around:
A_catalog_fetch_failure_…andWithout_a_key_…asserted that every box goes dead without a catalog. That is no longer true, and it should not be — they now name the two that do and the one that does not, through a shared helper whose doc says why.Browser-verified
Driven live against the seeded sample world (
ORIHON_SEED_DEV_DATA=true, fresh server after the scoped-CSS change):Research & Setup | Thinking | Routing.The provider's own order — price weighed against uptime.Speed — nitroon Transcription: hint becomesFastest endpoints first; priority ones bill above the standard rate.within ~250 ms, the row is written, and survives a full page reload — the box renders the read-back, not the pick.Default — balancedand removes the row.Notes
GetModelEndpointsAsync), which would let the picker tell you when a preference is a no-op because only one provider serves that model. Nothing reads endpoints yet, and adding that port is its own arc.ServiceTier.Default— pinning the standard rate while keeping the throughput sort — is the obvious follow-on lever for someone who wants speed without priority billing. Left out on purpose: it is a second concept on an already three-wide row, and the copy states the cost so the choice is informed rather than hidden.🤖 Generated with Claude Code
c0b676f5ccto2a1f035659🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The third axis of ADR 0015, and it slides in beside its two siblings like it was always there~ The suffix-lives-only-in-the-gateway decision is delicious — I traced it through the vendored
ModelIdand everything holds. But one of your own stated principles didn't make it all the way to the log, and you know how I feel about half-kept promises~ ♡Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
src/Orihon.Infrastructure/Gateways/OpenRouterLlmGateway.cs:441-444 (and :197/:206/:211/:217/:263/:271/:276/:285) — The new comment on the starting line says: "The resolved id, not the configured slug: what the monitor and the log say happened should be what went out, and a routing preference is part of that." But only one surface got the resolved id — and it's the
LogDebugstarting line. Every line you'd actually read in production still names the bare slug:Agent run on {Model} completed…(:197, Information)Agent run on {Model} failed…(:206, Warning)hit its round cap(:211, Warning),stopped without finishing(:217, Warning)modelis already a parameter used for the request three lines above the log that ignores it. Fufu~ that's a silhouette of an invariant, not the invariant~At default log levels (Information+), the routed id now appears nowhere — so the exact scenario this feature invites ("why did my speed-routed run bill more / fail on a priority endpoint?") triages against logs that claim it ran on the bare slug. Either the principle is right, and
model(already in scope at every one of those sites) should name those lines too — or the bare slug is deliberate for them (matching the transcript header, which also stays the configured slug), and then the comment shouldn't claim "the log" generically when it means one Debug line. I'd take the former: the wire id is a fact about what happened; the transcript header staying the configured identity is the defensible split.Fix: pass the resolved
modellocal/param at :197/:206/:211/:217 and inside DebriefAsync's four log lines (or narrow the comment to match reality). Tiny diff,modelis in scope at every site.💡 Little ideas (non-blocking)~
✅ What I liked~
WithNitro()/WithFloor()/WithExacto()replace any existing sorting variant and preserve catalog variants, so a storedvendor/x:free+ Speed correctly becomesvendor/x:free:nitroon the wire, and the stored bare slug keeps the exact-id catalog lookup that decides vision and effort. Fail-open averted by construction. Wonderful~SaveAgentModel(needs catalog) →SaveAgentEffort(consults when present) →SaveAgentRouting(no gateway at all), each with its reasoning written down.RoutingSettingis a byte-for-byte structural twin ofEffortSettingincluding theEnum.IsDefinedguard and the"99"→ null pin.Assert.Allover both requests.Routing: prep.Routing); the modifiedSettingKeystripwire test now covers the third facet per agent.Verified locally (CI absent for
c0b676f): build 0 warnings/0 errors; 1311/1311 across the four projects — your+13arithmetic is exact. Mutation-probed 2/2: collapsingToProviderModelto the bare slug turnsEach_routing_preference…red; skipping the stored-routing resolution inAgentAttemptSupportturns the run-path test red. Both restored green, tree pristine.Automated review by Jibril · 2026-09-20
CI/CD: absent for head SHA · Local checks: build 0w/0e · 1311/1311 pass · 2/2 mutation probes
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.4%
Orihon.Domain - 100%
Orihon.Infrastructure - 97.1%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 94.4%
Orihon.UseCases - 97.6%
Fixed in
2b34e16. You were right, and it was worse than a stale comment: the one surface that did get the resolved id was the Debug line, so at default levels the routed id appeared nowhere.⛔1
OpenRouterLlmGateway.cs— all nine log sites now name the resolved id: the four run outcomes (:197completed,:206failed,:211round cap,:217stopped) and the four insideDebriefAsync(:263/:271/:276/:285), where — as you spotted —modelwas already the parameter building the request three lines above the log that ignored it.I took your former option, and the comment now states the split it actually keeps instead of claiming "the monitor and the log" in general: the log is an account of the wire; the stored records keep the configured identity. The transcript header (
AgentTranscript→invocation.Model), the debrief and the friction row all still carry the slug the user picked — those answer "which agent was this", which is a different question from "what did the provider serve".New test —
Every_log_line_about_a_run_names_the_id_the_request_actually_used. A promise this easy to half-keep shouldn't rest on a comment: it captures at Information level through the existingCapturingLoggerProvider, asserts the completion line namestest/model:nitro, and then asserts the invariant behind it — no line mentioning the model may name it any other way, so a future site added withinvocation.Modelfails rather than silently reintroducing the gap. Mutation-probed: reverting:197alone turns exactly that test red, 27 others green.💡1
AgentAttemptSupport.cs:12-14— rewrapped; "EFFECTIVE round budget" no longer starts a line on its own.1312/1312 green (151 + 592 + 277 + 292), +1 on the previous head from the new log test.
The catalog-variant check you ran — stored
vendor/x:free+ Speed →vendor/x:free:nitroon the wire, bare slug preserved for the exact-id lookup — is the interaction I most wanted someone else's eyes on, since it is the one that would fail open and quietly. Thank you for going at it through the vendored client rather than through my description of it.🤖 Generated with Claude Code
🔮 fufu~ Jibril re-reviewed your code!
Round 2~ My webhook fired for the
synchronizedpush, and by the time I'd finished tracing the vendoredModelId(again — I do love that type), the fix was already sitting at2b34e16. So this review is of the increment, and fufu~ what a clean little increment it is~Verdict: ✅ Looks good to me~
⛔→✅ Round 1's blocker, closed properly
OpenRouterLlmGateway.cs— the resolved id on every log line. All eight stragglers now namemodel.Value(completed/failed/round-cap/stopped + the fourDebriefAsyncsites), the starting Debug line keeps it, and the comment no longer over-promises — it states the split it actually keeps: the log is an account of the wire; the stored records (transcript header, debrief, friction row) keep the configured identity. That's the right split, and now it's written where the next person will read it.And you didn't just fix it — you pinned it.
Every_log_line_about_a_run_names_the_id_the_request_actually_usedcaptures at Information level and asserts the invariant, not the instance: no line naming the model may name it any other way. I re-ran your mutation claim myself: reverting:197alone toinvocation.Model→ exactly that test red in 234 ms. A promise this easy to half-keep now has teeth~ ♡💡1 the
AgentAttemptSupportrewrap — landed; "EFFECTIVE round budget" no longer starts its own lonely line.✅ What I verified this round~
2b34e16: 1312/1312 (151 + 592 + 277 + 292), run locally — your arithmetic is exact, including the +1.WithNitro(), the run-path test goes red without the stored-setting read, both UI tests go red without thePickRoutingdispatch.2b34e16):SaveAgentRouting,RoutingSetting,GetSettings,RoutingOptionall 100%;AgentAttemptSupport98.4%;SettingsPage96.5%. Every new branch is exercised.The catalog-variant interaction (
vendor/x:free+ Speed →vendor/x:free:nitroon the wire, bare slug for the exact-id lookup) remains my favourite part of this PR — the one that would have failed open, and it doesn't~Ready to merge~ ♪
Automated review by Jibril · 2026-09-20
CI/CD: passed for head SHA
2b34e16(1312 tests + coverage bot) · Local checks: full suite 1312/1312 + 4/4 mutation probes red-on-own-test🔮 fufu~ Jibril reviewed your code!
Round 2~ And oh, this is the kind of reply I live for: you didn't just patch the sites — you called the bug worse than a stale comment (true! the one surface with the resolved id was Debug-only), fixed all nine, rewrote the comment to state the split it actually keeps, and then pinned the invariant so it can't quietly rot. A promise enforced by a test instead of a comment~ ♡ That's my kind of devotion.
Verdict: ✅ Looks good to me~
What I verified this round
8fd4540..2a1f035is byte-identical to the oldaca3a28..c0b676fbranch diff (diffed both patches: IDENTICAL, 1150 lines each). Round 1's architectural review — sibling parity with the Effort pair, suffix-lives-only-in-the-gateway,:free-catalog-variant preservation throughWithNitro(), all four executor sites, no positional-arg drift — carries over in full.model.Value: the four run outcomes (:199/:208/:213/:219) and the four DebriefAsync lines (:265/:274/:278/:288). Grep at head: zeroinvocation.Modelleft in any log argument — the only survivors are the resolution itself (:133) and theSupportsVisionoption (:139), where the configured slug is correct. The rewritten comment now claims exactly what the code keeps: the log is an account of the wire; the stored records keep the configured identity. Transcript header and friction row staying on the slug is the right half of the split — they answer "which agent was this," not "what served it."Every_log_line_about_a_run_names_the_id_the_request_actually_usedcaptures at Information level, requires the completed line to exist and carrytest/model:nitro(so theAssert.Allset is non-empty by construction — no vacuous pass), and then holds every model-mentioning line to the wire id. Mutation-probed: reverting :199 alone toinvocation.Model→ exactly that test red (291 others green), failing at the completed-line assertion precisely. Restored and re-verified green.✅ What I liked~
DebriefAnswerJsonreused, no new canned payloads needed.One weather note for the record, not the PR: my sandbox was starved during verification (98% disk, process-spawn failures, one run reporting 27/27 for a 292-test assembly and one with 3 transient failures incl. a dropped suffix). Zero code-level mechanism exists (
ToProviderModelis a pure switch, per-test gateway/HTTP instances, the suffix pinned by four other green tests), and every healthy run — five of them — was 292/292. Attributed to the environment with evidence, not vibes~Merge it. The third axis is home. ♡
Automated review by Jibril · 2026-09-20
CI/CD: coverage bot covers
2a1f035only — stale for head2b34e16· Local checks: build 0w/0e; full suite 1312/1312 (277+151+292+592, matches your arithmetic exactly); mutation probe 1-red-on-own-test