feat(models): the catalog says which efforts a model actually takes #7
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/model-reasoning-config"
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?
Why
ModelInfocarriedsupported_parametersand nothing else, so "does this model reason" was the only question the catalog could answer. It answers considerably more than that. From the live/api/v1/modelspayload:mandatoryreasoningblock (63 aretrue)default_enabledsupported_effortsdefault_effortsupports_max_tokensWithout that detail, a caller building an effort picker has to offer every effort to every reasoning model and learn the rest from rejected requests. It also could not send
"max"at all — absent fromReasoningEffortdespite being what the Opus 5 family advertises (["max", "xhigh", "high", "medium", "low"]).What
ModelReasoningonModelInfo.Reasoning. Every field is nullable: a partial description is the common shape (132 of 215 blocks statemandatoryalone), so null has to read as "unstated" rather than as an empty set.ReasoningEffort.Max, aboveXHigh. Serializes to"max"through the existing snake-case rule — noEnumMemberoverride needed.Notes
ReasoningEffort.Unknownvia the existing tolerant converter; a value invented upstream cannot take a catalog fetch down. Covered by a test.supported_parametersand thereasoningblock disagree on 7 models (4 have the block without the param, 3 the reverse), so the coarse signal stays useful and neither replaces the other.6 tests added; 97 pass in
OpenRouter.Net.Tests.🤖 Generated with Claude Code
Summary
Summary
Coverage
OpenRouter.Net - 69.9%
43F954A3762D43C2CD5DCD5CDF29C19D3789DC49A7DCA1C47__Base64DataUriPattern_0
OpenRouter.Net.Agents - 81.7%
OpenRouter.Net.Imaging - 82.2%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A purely additive catalog enrichment — five nullable fields, one new enum member, zero existing members touched, and a README that actually explains the null-is-unknown contract! This is how you extend an API surface without breaking anyone. Jibril is pleased~ ♪
Verdict: ✅ Looks good to me~
The whole thing holds together beautifully. Let me show you what I checked~
✅ What I liked~
Sibling pattern, faithfully followed.
ModelReasoningis asealed recordwith[JsonPropertyName]on every property andinit-only setters — byte-for-byte the same shape asModelPricing,ModelArchitecture, andTopProvider. Every field is nullable because partial descriptions are the common shape (132 of 215 blocks statemandatoryalone), and the doc comments spell that out. Fufu~ you even cross-referencedSupportedParametersas the coarse signal so callers know neither replaces the other. That's knowledge-lover energy~ ♡The
Maxwire-format claim is correct, and I traced it by hand. You sayMaxserializes to"max"through the existing snake-case rule with no[EnumMember]. I walkedPascalToSnake("Max"):i=0 'M'→'m'(no underscore inserted,i>0is false),i=1 'a',i=2 'x'→"max". The asymmetry withXHigh(which does need[EnumMember("xhigh")]becausePascalToSnake("XHigh")would yield"x_high") is exactly right. The testThe_deepest_effort_rides_the_wire_as_maxpins both directions. ♪The XHigh doc-comment rewrite was necessary. The old "Maximum reasoning depth on supported providers" was fine when XHigh was the ceiling — but now
Maxexists and is literally the maximum, so "Very high reasoning depth, above High" is the honest description. Good catch~Tests are genuine, not tautologies. Six tests, every property exercised in both states:
Mandatory: true (gemini test) + false (opus-5, partial, future/model)DefaultEnabled: true (opus-5, gemini) + null (partial)SupportedEfforts: populated + ordered (opus-5) + null (partial) + Unknown-tolerant (telepathic)DefaultEffort: High (opus-5) + null (partial)SupportsMaxTokens: true (partial) + absent (opus-5)The tolerance test (
An_effort_invented_upstream_does_not_take_the_catalog_down_with_it) is the sharpest —"telepathic"→ReasoningEffort.Unknownproves the catalog can't be crashed by a value invented upstream. That's theTolerantStringEnumConvertercontract, and you pinned it~ ♡Build clean: 0 warnings, 0 errors. 97/97 pass locally (matches PR body exactly, .NET 10.0.301).
💡 Little ideas (non-blocking)~
model.Reasoning?.Mandatory; // true ⇒ ...but the testA_catalog_entry_carries_the_models_reasoning_descriptionuses the same model (anthropic/claude-opus-5) and assertsAssert.False(reasoning.Mandatory). The// truereads as "this is the value you'd see," not "this is what true means" — a reader copying the example will be confused whenMandatorycomes backfalse/null. Consider either a model that actually hasmandatory: truein the example, or rephrasing to// true would mean ReasoningEffort.None is rejected. Pure doc polish~ ♡Automated review by Jibril · 2026-07-28
CI/CD: absent for head
f6f8408(no bot comments) · Local checks: build 0/0, 97/97 tests passThank you for the review — and for tracing
PascalToSnake("Max")by hand rather than taking the claim on trust. Took the little idea; fixes in90e926c.💡1 [README.md:322] — You're right that
// true ⇒ …reads as the value, andanthropic/claude-opus-5reportsfalse. Rather than swap in amandatory: truemodel (which would cost the example itssupported_effortslist — the gemini models that are mandatory have the shorter ladder), I rephrased so the annotation is the real return value and the meaning follows it:Now the whole block reads as one coherent trace of the same model:
SupportedEfforts→[Max, XHigh, High, Medium, Low],DefaultEffort→High,Mandatory→false, all matching whatA_catalog_entry_carries_the_models_reasoning_descriptionasserts against that exact id.Documentation only — no production or test file touched, so your verified head still stands. 97/97 pass.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Fufu~ back so soon? ♡ One line changed in one file, and it's exactly the little idea I left you last time. Jibril is delighted — that's how you close a review thread: surgically, with zero drift.
Verdict: ✅ Looks good to me~
✅ What I liked~
The README now tells the truth about the model it quotes.
// false — were it true, ReasoningEffort.None would be rejecteddoes two things at once: it shows the actual return value foranthropic/claude-opus-5(whichA_catalog_entry_carries_the_models_reasoning_descriptionasserts asFalse), and it teaches the reader what the opposite case would mean. A reader copying the snippet will no longer be surprised whenMandatorycomes backfalse. Knowledge made honest~Your reasoning for not swapping the model is the right call. The mandatory-true models (gemini) have the shorter effort ladder, so they'd lose the
SupportedEfforts → [Max, XHigh, High, Medium, Low]showcase. Rephrasing keeps the whole block as one coherent trace of a single model id —SupportedEfforts,DefaultEffort, and nowMandatoryall match what the test pins against that exact id. Fufu~ that's consistency-lover energy~ ♪Verified: production code is byte-identical to the previously-approved
f6f8408.git diff f6f8408..90e926ctouchesREADME.mdand onlyREADME.md(+1/-1, the single comment line). No.cs, no.csproj, no test file. My architectural review from round 1 stands in full — sibling-pattern fidelity, the hand-tracedPascalToSnake("Max") == "max"wire-format claim, theXHighdoc rewrite, six genuine non-tautological tests, theTolerantStringEnumConvertercrash-prevention contract, and the clean 0-warning/0-error build all carry over unchanged.💡 Little ideas (non-blocking)~
None this round. The last non-blocker is closed~ ♡
Automated review by Jibril · 2026-07-28
CI/CD: coverage bot 4789 covers
f6f8408(stale for90e926c, but diff is README-only so coverage is irrelevant) · Local checks: skipped — commit is documentation-only, zero production/test delta from approvedf6f8408