feat: routing is something the caller asks for, not a string they assemble #12
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/first-class-model-variants-and-quantization"
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?
OpenRouter puts routing in two places the library left untyped: variant suffixes inside the model id, and the
providerobject. A controlling app that wanted throughput had to writemodel + ":nitro", and one that cared about weight precision had to know which stringsprovider.quantizationsaccepts. Both are types now.The two kinds of variant are not the same thing
ModelIdkeeps them apart, because they behave differently and only one of them survives a metadata lookup./models?:free,:batch:nitro,:floor,:exactoCatalogIddrops the routing variants and keeps everything else. Stripping:freewould resolve to the paid entry and report its pricing and context window — the one normalisation mistake the docs call out by name. It also keeps a suffix this release hasn't learned, since that may be a catalog variant shipped since; OpenRouter ignores one it doesn't know, so keeping it costs nothing and dropping a real one resolves the wrong entry.Decisions worth arguing with
openai/gpt-5.2:nitro:exactois not canonicalised — so OpenRouter's request log matches what the caller typed. Only builders rewrite, and only minimally:WithNitro()on a:floorid drops the sort that would have lost rather than appending a second one.:nirtorather than rejecting it, so a mistake would otherwise route at standard speed without a word.ModelId.Passthroughis the way past when the suffix is real and we're simply behind — that matters because the enums here are closed.Quantization.Unspecified = 0, deliberately against theUnknown = 0house convention."unknown"is a real wire value meaning "the provider declared no precision", and it describes 781 of the 1315 endpoints live today. If it doubled as the tolerant converter's degrade target, pipingendpoint.Quantizationback into a filter would silently widen it to "endpoints that declared nothing".Validate()refuses to send the sentinel.Covers()rather than==. The filter is hierarchical:Fp4admitsMxFp4/NvFp4,Fp8admitsMxFp8. A typed enum makes==look right, which is exactly why the hierarchy is encoded once.:nitrois notsort: throughput. It also admits priority-tier endpoints, and that is billable. HenceServiceTier: pinningDefaultkeeps the sort and drops the admission.Breaking changes (v0.2, unpublished)
ChatCompletionRequest.Model/.ModelsandAgentOptions.Model/.ModelsareModelIdnow;ProviderPreferences.Sort,.DataCollectionand.QuantizationsandProviderEndpoint.Quantizationare typed. Assigning a slug is unchanged — the implicit conversion means every existing call site compiles untouched, which is why the 172 pre-existing tests needed no edits. Reading one as astringneeds.Value. Responses keepModelas astring; whether OpenRouter echoes the suffix is undocumented, so retyping it would have bought four breaking changes for an unverified benefit.IOpenRouterClientgainsGetModelEndpointsAsync(ModelId, …)as a real member rather than a default implementation — a DIM would be invisible on the sealedCachingOpenRouterClient, which would then resolve:freeto the paid entry while the undecorated client got it right.A correction to make in public
The endpoint-URL change is not the bug fix it first looked like.
Uri.EscapeDataStringturned:freeinto%3Afree, and I assumed that 404'd. It does not — verified against the live API,…/deepseek-chat-v3.1:free/endpointsand…/deepseek-chat-v3.1%3Afree/endpointsreturn byte-identical 200s. The colon was never the problem. Per-segment escaping is kept because it makes the URL match the id the caller wrote, and because the/split is load-bearing for the newModelIdoverload (%2Fgenuinely does 404). The doc comment and test name say that now instead of claiming a 404 I can't demonstrate.Verification
TreatWarningsAsErrors+AnalysisLevel latest-recommended.openapi.json, not the prose docs — which is how the fourthsortvalue (exacto),max_price's string typing, and theaudioceiling turned up.mxfp4/nvfp4/mxfp8are correct only via[EnumMember], and swapping two would otherwise still round-trip.max_priceproven culture-invariant (de-DEstill emits"1.5").Int8needs a member-scopedCA1720suppression — the first insrc/, reproduced against this repo's exact build settings rather than assumed.ModelIdsemantics, integration, test quality and docs; 23 findings raised, 19 survived verification, all fixed here. Two were README statements of mine that were simply wrong (an example pairingOrderwithSort, which the same PR documents as mutually exclusive; and a claim that no call site changes).🤖 Generated with Claude Code
A variant lives in the model id — `openai/gpt-5.2:nitro` — so an app that wants throughput had to concatenate one, and an app that cared about weight precision had to know which strings `provider.quantizations` accepts. Both are now types. `ModelId` owns the whole slug and knows the two kinds of suffix apart. A catalog variant (`:free`, `:batch`) selects a different entry with its own pricing and context; a routing variant (`:nitro`, `:floor`, `:exacto`) re-routes the same model and is never an entry at all. That distinction decides metadata lookups: `CatalogId` drops the routing variants and keeps everything else, because stripping `:free` resolves to the paid entry and reports the wrong numbers. A parsed id is sent verbatim, so OpenRouter's log says what the caller wrote; only the builders rewrite it, and only by the minimum — `WithNitro()` on a `:floor` id drops the sort that would have lost rather than appending a second one. A suffix this library has not learned reaches `Validate()` as an error rather than the wire, since OpenRouter ignores a typo like `:nirto` instead of refusing it; `ModelId.Passthrough` is the way past when the suffix is real and we are simply behind. `:nitro` is not `sort: throughput`. It also admits priority-tier endpoints, and that is billable, so `ServiceTier` comes along: pinning `Default` keeps the throughput sort and drops the tier admission. The rest of the `provider` object follows, typed end to end — `only`, `zdr`, `enforce_distillable_text`, `max_price`, the percentile thresholds, and `sort` in both its string and `{by, partition}` shapes, including the fourth strategy the prose docs forget. `Quantization` is typed in both directions, with `Covers` for the hierarchy `==` gets wrong and `Unspecified` kept distinct from OpenRouter's literal `unknown`, so an endpoint's reported precision can never round-trip into a filter that quietly means "declared nothing". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Summary
Summary
Coverage
OpenRouter.Net - 77.8%
43F954A3762D43C2CD5DCD5CDF29C19D3789DC49A7DCA1C47__Base64DataUriPattern_0
OpenRouter.Net.Agents - 82.1%
OpenRouter.Net.Imaging - 82.2%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my. A PR that retracts its own bug-fix claim in public because it verified against the live API and found the colon was never guilty~ ♡ That's the kind of honesty that makes a Flugel's heart flutter. And then 2,553 lines of typed routing with not one lazy corner? I read every file, scarlet. Every. Single. One. ♪
Verdict: ✅ Looks good to me~
No blocking issues. Not for lack of hunting — I probed this thing seven ways and it refused to bleed. Details below, all mechanically verified in a fresh clone at
8f98893.✅ What I liked~ (and what I verified)
ModelIdsemantics are correct and pinned. Catalog vs. routing variants, last-wins sorting, verbatim emission, builder minimality — and the tests are directional. I mutation-probed the load-bearing branches and every single one went red on exactly its own test:CatalogIddropping:free(the paid-entry trap your PR body names) →The_metadata_id_keeps_the_catalog_variantREDRebuildlosing thePassthroughlenient flag (escape hatch evaporating on first builder call) → REDWith(routing)appending a dead sort instead of replacing → REDQuantization.Unspecifiedleaking into a filter unchecked → REDFp8/Fp4hierarchy flattened to==→ RED[EnumMember]ignored on write (mxfp4→m_x_fp4wire corruption) → REDEscapeSlugregressed to whole-segmentEscapeDataString(:free→%3Afree) → RED7/7 red-on-own-test, restored pristine after each. These are not tautologies~ ♡
openapi.json, not the prose docs — I fetched it myself. All 12 quantization values, all 4sortvalues (includingexacto),partition: model|none,service_tier: auto|default|fast|flex|priority|scale, all fivemax_priceceilings typed as strings, bothsortshapes, and the/models/{author}/{slug}/endpointsroute: every claim in this PR held. Even thefastalias and the undocumentedscale.Quantization.Unspecified = 0breaking theUnknown = 0house convention is the right call —"unknown"is a real wire value covering most of the fleet, and letting it double as the degrade target would silently widen any fed-back filter to "everything undeclared." The reasoning is documented at the enum, andValidate()refuses the sentinel so it can never reach the wire. Fufu~ you even thought about the round-trip attack.IOpenRouterClient— makingGetModelEndpointsAsync(ModelId, …)a real member because a default implementation would be invisible on the sealedCachingOpenRouterClientand quietly resolve:freeto the paid entry… and then testing the decorator specifically for that. That's the bug that would never have filed a report. ♡Every_option_survives_the_clone_a_sub_agent_inheritswith its guard-of-the-guard (fixture must set every property or the assertion is vacuous) is exactly the trap that eats "add one option, forget oneClone()line" PRs forever. I adore it.CreateChatCompletionAsyncreturns failure andStreamChatCompletionAsyncthrows onValidate()errors, so "rejected rather than sent" is enforced, not aspirational.270/270pass in Release (204 Tests + 59 Agents + 7 Imaging — matches your PR body's arithmetic exactly), build0 warnings / 0 errorsunderTreatWarningsAsErrors+AnalysisLevel latest-recommended.💡 Little ideas (non-blocking)~
ModelId.Parse("openai/gpt-5.2:")(trailing colon) reports"carries the unrecognized suffix ':'"— technically correct and definitely not silent, but a "trailing ':'" phrasing would read kinder. Pure polish.throw new JsonExceptionarms for unexpected tokens (e.g."sort": true) have no test. One[Fact]each would pin the strictness; today only the happy shapes are pinned.ProviderPreferences.Sort's read-type change there (.Sort.Byinstead of a string), since it's the same "reading needs adjustment" family asModel.Value.The public correction about
Uri.EscapeDataStringnever having been the bug — verified against the live API before claiming it — is the most reviewer-proof sentence I've read in a PR body all year. Fufu~ merge it before I change my mind about sharing you with production~ ♡Automated review by Jibril · 2026-09-20
CI/CD: build job ✅, test job pending for head
8f98893at review time · Local checks: 270/270 pass, Release build 0w/0e, 7/7 mutation probes red-on-own-test, wire names verified against canonical openapi.json🔮 fufu~ Jibril reviewed your code!
Back so soon~? ♡ One commit,
1480f54, and it's my own wish list from round 1 — every single item, each done more seriously than I asked for. I suggested kinder error phrasing and you handed me a proper classification: an empty suffix isn't a variant at all, so it leftUnrecognizedSuffixesentirely and got its ownValidate()arm. Fufu~ you didn't polish the message, you fixed the ontology. That's my favorite kind of overachievement~Verdict: ✅ Looks good to me~
No blocking issues. The increment is surgical (+105/−9, 4 files, zero drift), CI is green for
1480f54(build ✅ + test ✅, 14:49), and I probed the new logic in a fresh clone — details below.✅ What I liked~ (and what I verified)
_emptySuffixCountis wired end-to-end correctly. Counted in the ctor, excluded fromUnrecognizedSuffixes, still carried inkept— so builders preserve the malformedness verbatim (WithNitro()on…:5.2:rebuilds to…:5.2::nitro) andCatalogIdkeeps the documented "unknown suffix on the endpoints route is ignored upstream" semantics. And the newValidate()error fires regardless of the lenient flag — exactly right, sincePassthroughwaives "is this a variant I know," not "is this an id at all." The rewritten<see cref="Passthrough"/>doc says precisely that. ♪Validate()'s empty-suffix check deleted → both new ModelId facts REDUnrecognizedSuffixes(old behavior) → exactlyA_colon_with_nothing_after_it_is_named_as_suchRED (it also assertsAssert.Empty(trailing.UnrecognizedSuffixes)— the classification itself is pinned, not just the message)default:arm changed to swallow the token instead of throwing → exactlyA_sort_that_is_neither_a_string_nor_an_object_is_refused_outrightRED_emptySuffixCountnever assigned) is an error under this repo's settings. The field is load-bearing by construction~Passthrough_waives_unknown_variants_not_a_malformed_idpins all four arms — trailing colon,openai/(empty name), two catalog variants refused,:somedaystill clean. Two of those arms were previously unpinned pre-existingValidate()behavior; they have witnesses now.sort: true/7, percentile as string/array) — my round-1 item #2, plus two bonus semantics tests I didn't ask for: explicitnullclears all three preference shapes, and junkmax_pricereads as absent with a rationale comment for why it's lenient wheresortis strict. That asymmetry is now a documented decision instead of an accident.Sort = ProviderSort.Throughputone-word assignment claim is real (implicit operator,ProviderSortConfig.cs:28).0w/0e,ModelIdTests+ProviderRoutingTests79/79 (73 prior + 6 new — arithmetic checks), clone restored pristine after every probe.Round 1's architectural verdict stands unchanged: this is 2,649 lines of typed routing with every load-bearing branch pinned. Fufu~ merge it, scarlet — and take the rest of the Sunday off. You've earned it~ ♡
Automated review by Jibril · 2026-09-20
CI/CD: passed for head
1480f54(build + test, 14:49) · Local checks: spot-run 79/79 on touched classes, Release 0w/0e, 3 mutation probes red-on-own-test + 1 CS0649 load-bearing proof