Repository navigation
fix(core): expose model reasoning capability - #55100
zhengkaics wants to merge 2 commits into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
This works for models that come from the models.dev catalog. I ran serve from the base and from this branch and called GET /api/model. On the base, none of the 12 models has capabilities.reasoning. With this change, all 12 report reasoning: true. The core models tests, the app global-sync tests and the schema/core/app type checks pass. I couldn't check the desktop picker itself; the mapper is covered by the unit test.
Public API: Model.Capabilities (the /api/model response and the client's generated ModelCapabilities type) gains an optional reasoning?: boolean. This is backwards compatible: it's an optional addition, and existing callers and configs keep working.
Concerns:
packages/protocol/openapi.jsonwasn't regenerated. Runningbun run generateinpackages/protocoladdsreasoningtoModel.Capabilities. Ondev, the protocolcheck:generatedalready fails for an unrelatedencodingfield, but this change adds its own stale part.- Models defined in a user's provider config can't set
reasoningyet, so they keep showing "No reasoning". See the inline comment.
| const fallback: Capabilities = base ?? Capabilities.default() | ||
| return { | ||
| tools: config.tools ?? fallback.tools, | ||
| reasoning: config.reasoning ?? fallback.reasoning, |
There was a problem hiding this comment.
config.reasoning is always undefined here. Config.Model.Capabilities in packages/schema/src/config/provider.ts builds its fields by hand with mapFields and only includes tools, input and output, so a user can't set capabilities.reasoning for a model in their provider config. Custom and openai-compatible models get Capabilities.default(), which has no reasoning, so they still show "No reasoning" (#51846, #50608). Could you add reasoning: optional(fields.reasoning) to that config schema? You could also map the v1 reasoning flag in migrateModel (packages/core/src/v1/config/migrate.ts), which currently builds capabilities from only tool_call and modalities.
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
|
There's one more place the same flag gets dropped, which this PR does not touch: the v1 → v2 config migration.
Reported in #51846. Minimal repro config (v1 syntax, custom openai-compatible provider): {
"provider": {
"localtest": {
"npm": "@ai-sdk/openai-compatible",
"options": { "baseURL": "http://127.0.0.1:9999/v1", "apiKey": "sk-local-test" },
"models": {
"glm-flash": { "name": "GLM Flash", "reasoning": true },
"plain-model": { "name": "Plain Model", "tool_call": true }
}
}
}
}Both versions of
The change is two lines: const capabilities =
- info.tool_call !== undefined || info.modalities?.input !== undefined || info.modalities?.output !== undefined
+ info.tool_call !== undefined ||
+ info.modalities?.input !== undefined ||
+ info.modalities?.output !== undefined ||
+ info.reasoning !== undefined
? {
tools: info.tool_call ?? defaults.tools,
+ ...(info.reasoning === undefined ? {} : { reasoning: info.reasoning }),
input: info.modalities?.input ?? defaults.input,
output: info.modalities?.output ?? defaults.output,
}
: undefinedAdding I verified this on a source build of I can open a stacked PR on top of this one, or if you'd rather keep it in one place feel free to fold it in — I have the commit and tests ready either way. Happy either way. |
|
Both points addressed in
Added tests: v1 migration with a reasoning-only model, and a config overlay declaring |
|
Thanks — folded in rather than stacking, so there is one place to track it.
Tests added: reasoning-only model through |
|
Tests look right to me — the toEqual on the undeclared model is stricter than what I had, and the silent case is a good addition. Same for the overlay test: asserting the inherited tools/input/output survive when only reasoning is declared is exactly the mergeCapabilities semantic I was worried about. On openapi.json: keep the Provider.Encoding hunks. They're a generation lag for a schema that already exists on dev, not part of this fix — dropping them would fail check:generated again and leave dangling $refs. Worth one line in the description so reviewers don't wonder where it came from. |
|
Added the fallback from the inventory analysis in #50257 (thanks @wangzishi): when the catalog has no New tests: variants fallback, reasoningField fallback, and explicit catalog |
|
Heads-up on sequencing: #55853 (in flight) introduces a rich The two sit at different layers and should compose:
Happy to rebase #55100 on top of #55853 once it lands — e.g. keep the boolean on |
|
Same root cause, plus one data point for the fallback path. Environment: OpenCode desktop 2.0.30 (Electron, 1. The shipped bundle still reads a field the mapper never sets. From s=()=>e.model.capabilities?e.model.capabilities.reasoning?t.t(`model.tooltip.reasoning.allowed`):t.t(`model.tooltip.reasoning.none`):e.model.reason…Worth noting the fallback branch ( 2. Nothing in the served inventory carries the signal either. On 2.0.30,
Top-level 3. A case the fallback alone covers — and a good test shape. "compatibility": { "reasoningField": "reasoning_content" },
"variants": [ { "id": "low", "settings": { "reasoningEffort": "low" } }, … "high", "max" ]i.e. the flag is absent (not reasoning: model.capabilities.reasoning ?? (model.compatibility?.reasoningField !== undefined || model.variants.length > 0)this resolves to Not filing a separate issue — #50257 already tracks it and this thread has the fix. |
|
#56084 was closed as a duplicate of this PR. I found one fallback edge case: When the catalog reasoning flag is absent, please consider: reasoning:
model.capabilities.reasoning ??
(model.compatibility?.reasoningField !== undefined ||
model.variants.some((variant) => variant.id !== "none")),I tested the adapter before and after the change (3 expected failures before; 5/5 passing after). The added regression cases cover no variants, The minimal implementation and in-repo regression test are available in #56084, based on current |
|
@zhengkaics I opened a stacked PR directly against your It is only 2 files / +30 -1 lines and keeps your explicit catalog-flag precedence. The original adapter fails the The PR targets your fork because the OpenCode bot correctly closed #56084 as overlapping with #55100. Your fork has Issues disabled, so its inherited |
029af99 to
fa54330
Compare
|
Rebased
Re-verified on this head: typecheck passes and the app/core suites are green (only the 8 macOS shell-env failures, identical on |
|
Independent confirmation on 2.0.32, reported in #50257 and cross-posted on #53520: |
|
Independent verification on
The config path works end to end, so custom providers no longer depend on a models.dev entry. Also, the earlier review note about Fixes #50257 for both catalog and config-defined models. |
fa54330 to
adb929a
Compare
|
Rebased onto current On the new head Thanks @Wit-Wang for the independent end-to-end verification of the config path — that matches what I see locally (catalog and config-defined models both report |
There was a problem hiding this comment.
Your new commits fix my earlier comment: a model's capabilities.reasoning in the provider config now reaches the model, and the v1 reasoning flag is migrated. I checked with serve from the base and from this branch. On the base, no model in /api/model reports reasoning. On this branch, catalog models do, and so does a custom model configured with reasoning: { supported: true }. The core config, provider and models tests, the app global-sync tests, both check:generated scripts and the schema/core/app type checks pass.
Public API: Model.Capabilities (the /api/model response, openapi.json and the client's generated ModelCapabilities type) gains an optional reasoning?: boolean. This is backwards compatible. The v1 config migration now turns reasoning: true/false into capabilities.reasoning: { supported }, a field that already exists in the v2 config.
Concern: the new fallback in the app mapper marks custom models as reasoning-capable when they aren't (see the inline comment). Also, the new comments in utils.ts and its test are in Chinese; the rest of the codebase uses English.
packages/app/src/runtime/server/global-sync/utils.ts:87: This fallback reports reasoning for models that don't support it. Core gives custom openai-compatible models default effort variants (low/medium/high) even when nothing in the config mentions reasoning. In my run, a bare custom modelplainhadcapabilities.reasoningunset with those variants, so this line would map it totrue. Since core now fills inreasoningfrom both the catalog and the config, I'd keepmodel.capabilities.reasoning ?? falsehere. If unflagged models need a better answer, core is the place to work it out, not the client. That would also let you drop most of the 6 extra test fixtures added for this fallback.
Port of anomalyco#50283 to the V2 line (the original PR was closed by the V1→V2 dev promotion sweep, not by review). Fixes anomalyco#50257. The models.dev catalog's reasoning flag is still dropped on current dev, so the desktop model picker reports "No reasoning" for every model — 160 models / 0 with capabilities.reasoning on 2.0.20, still reproducible on 2.0.26. - schema: Model.Capabilities gains an optional reasoning field - core: models.dev materialization and mergeCapabilities thread it through, so provider config overlays no longer drop it - app: model mapper derived capabilities.reasoning from the catalog instead of hardcoding false - client: regenerated API types via bun run generate - tests: app mapper coverage + models.dev fixtures
Follow-up to the port, now layered on top of the rich Config.Model.Reasoning that landed in anomalyco#55853: - capabilities: mergeCapabilities accepts the rich config reasoning object and derives the runtime reasoning flag from its `supported` value, so a config-declared capability survives into Model.Info - v1 migration: migrateModel keeps a declared reasoning flag by translating it to { supported } and includes reasoning-only models in the capabilities guard - protocol/client: regenerate generated outputs (Model.Capabilities.reasoning) - tests: v1 migration + config overlay coverage Co-authored-by: hril3 <hril3@users.noreply.github.com>
adb929a to
5bdee8f
Compare
|
Addressed the review on
Rebased onto current On the new head |
|
Superseded by #60457. Thanks for the review and for landing the app-side fix. |
Issue for this PR
Closes #50257
Closes #53520
(V2 port of #50283, which the V1→V2 promotion sweep closed without review.)
Type of change
What does this PR do?
The desktop model picker reports "No reasoning" for every model because the models.dev catalog's
reasoningflag is dropped. On 2.0.20: 160 models, none withcapabilities.reasoning; still reproducible on 2.0.26.packages/schema/src/model.ts:Model.Capabilitiesgains an optionalreasoningpackages/core/src/models-dev.ts: materializereasoningfrom the catalogpackages/core/src/model.ts:mergeCapabilitiespreserves it across provider config overlayspackages/app/src/runtime/server/global-sync/utils.ts: the app mapper derives the flag instead of hardcodingfalsepackages/client: regenerate API types (bun run generate)true, unflagged →false) + models.dev fixturesHow did you verify your code works?
bun turbo typecheck: 36/36 greenglobal-synctests: 9/9; core model tests (model, models, model-resolver, model-plugins, variant): 91/91false, notundefinedref=dev(matches the recon in desktop: model picker shows "No reasoning" for all V2 models — capabilities.reasoning is never set #50257)Screenshots / recordings
Not applicable — tooltip/label only, no visual change beyond the label itself.
Checklist
Added by the opencode bot (AI-generated).
Fixes #50557
Fixes #51846
Related: #50608
Public API: adds optional
reasoningto Model.Capabilities in /api/model, openapi.json and the client's ModelCapabilities type; v1reasoningnow migrates tocapabilities.reasoning(backwards compatible)