feat(core): strip unsupported sampling params via capability registry#18622
Draft
intojhanurag wants to merge 1 commit into
Draft
feat(core): strip unsupported sampling params via capability registry#18622intojhanurag wants to merge 1 commit into
intojhanurag wants to merge 1 commit into
Conversation
|
@intojhanurag is attempting to deploy a commit to the Mastra Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: 236c487 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds temperature capability tracking to provider registry generation and lookup, then uses it in the model router to strip unsupported ChangesTemperature Capability Registry and Router Stripping
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
PR triageLinked issue check skipped for core contributor @intojhanurag. PR complexity score
Applied label: Changed test gateChanged Test Gate is pending. The |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
packages/core/src/llm/model/router-custom-provider.test.ts (1)
214-301: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a custom-provider pass-through case.
All new assertions use registry-backed OpenAI IDs. The router only strips when capability lookup returns
false; unknown/custom providers returnundefinedand should keeptemperature/topP/topK. A dedicated test here would lock that behavior down.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/llm/model/router-custom-provider.test.ts` around lines 214 - 301, The new temperature-stripping tests in router-custom-provider.test.ts only cover registry-backed OpenAI IDs, but they miss the custom-provider path where capability lookup returns undefined. Add a dedicated test around Agent.generate and/or Agent.stream that uses a custom provider/model id and verifies modelSettings like temperature, topP, and topK are preserved in the spyGenerate/spyStream payload when the capability registry has no entry. Keep the focus on the router behavior in the Agent flow and the createOpenAICompatible/mock model setup so the pass-through case is locked down.packages/core/src/llm/model/provider-registry.test.ts (1)
44-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover nested-model fallback in the new temperature suite.
modelSupportsTemperature()now rides on the generic capability lookup, but this suite only exercises exact model IDs. Add one variant/dated model ID that should resolve through the base temperature capability entry so regressions in the fallback path are caught here too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/llm/model/provider-registry.test.ts` around lines 44 - 64, The new temperature tests for modelSupportsTemperature() only cover exact IDs, so they miss the nested-model fallback path in the generic capability lookup. Add a case in provider-registry.test.ts using a variant or dated model ID that should resolve through the base temperature capability entry, and assert modelSupportsTemperature() returns true to protect the fallback behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/twelve-candles-love.md:
- Line 5: Add a brief public-facing usage example for the new `@mastra/core`
sampling-parameter stripping behavior, and rewrite the changelog entry to
emphasize the developer outcome rather than models.dev or registry internals. In
the changeset entry, reference the user-visible behavior of unsupported
temperature/topP/topK being removed automatically before dispatch, and keep the
example and description centered on how consumers benefit from the feature.
In `@packages/core/scripts/generate-providers.ts`:
- Around line 20-39: Update writeRegistryFiles so capability file generation is
triggered when either attachmentCapabilities or temperatureCapabilities has
entries, not only when attachmentCapabilities is non-empty. Use the existing
writeRegistryFiles flow in generate-providers.ts and the capability output logic
that writes under capabilities/ to ensure temperature-only runs still emit the
registry files consumed by modelSupportsTemperature().
In `@packages/core/src/llm/model/gateways/models-dev.ts`:
- Line 114: The capability maps are accumulating stale entries because
fetchProviders() rebuilds providerConfigs but does not reset
temperatureCapabilities or attachmentCapabilities before repopulating them.
Update the logic in fetchProviders(), getTemperatureCapabilities(), and
fetchProvidersFromGateways() so both maps are cleared/reset alongside
providerConfigs before new data is loaded, ensuring removed providers/models or
capability changes are not retained across syncs.
In `@packages/core/src/llm/model/provider-registry.ts`:
- Around line 436-446: The capability-file contract is out of sync with what
writeRegistryFiles() emits because ProviderCapabilityFile still requires
attachment even when only temperature is written. Update ProviderCapabilityFile
in provider-registry.ts so attachment is optional and the
parser/loadProviderCapabilityFile path can represent temperature-only files
without relying on a cast. Keep CapabilityDimension and providerCapCaches
aligned with the revised schema so the writer and reader use the same contract.
In `@packages/core/src/llm/model/registry-generator.ts`:
- Line 257: The write path in writeRegistryFiles() is still controlled only by
attachment capability data, so temperature-only inputs never trigger
regeneration of the capabilities/ directory and stale registry files can remain.
Update the gating logic around writeRegistryFiles() and the capability file
generation path so temperatureCapabilities alone is enough to write/refresh the
temperature registry entries, and ensure any removal/cleanup logic also runs
when only temperature support changes. Use the existing writeRegistryFiles() and
modelSupportsTemperature() flow to locate the affected branch.
---
Nitpick comments:
In `@packages/core/src/llm/model/provider-registry.test.ts`:
- Around line 44-64: The new temperature tests for modelSupportsTemperature()
only cover exact IDs, so they miss the nested-model fallback path in the generic
capability lookup. Add a case in provider-registry.test.ts using a variant or
dated model ID that should resolve through the base temperature capability
entry, and assert modelSupportsTemperature() returns true to protect the
fallback behavior.
In `@packages/core/src/llm/model/router-custom-provider.test.ts`:
- Around line 214-301: The new temperature-stripping tests in
router-custom-provider.test.ts only cover registry-backed OpenAI IDs, but they
miss the custom-provider path where capability lookup returns undefined. Add a
dedicated test around Agent.generate and/or Agent.stream that uses a custom
provider/model id and verifies modelSettings like temperature, topP, and topK
are preserved in the spyGenerate/spyStream payload when the capability registry
has no entry. Keep the focus on the router behavior in the Agent flow and the
createOpenAICompatible/mock model setup so the pass-through case is locked down.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f9a5958c-85b6-40f4-abf2-9164128574c3
📒 Files selected for processing (13)
.changeset/twelve-candles-love.mdpackages/core/scripts/generate-providers.tspackages/core/src/llm/index.tspackages/core/src/llm/model/capabilities/anthropic.jsonpackages/core/src/llm/model/capabilities/openai.jsonpackages/core/src/llm/model/gateways/base.tspackages/core/src/llm/model/gateways/models-dev.tspackages/core/src/llm/model/index.tspackages/core/src/llm/model/provider-registry.test.tspackages/core/src/llm/model/provider-registry.tspackages/core/src/llm/model/registry-generator.tspackages/core/src/llm/model/router-custom-provider.test.tspackages/core/src/llm/model/router.ts
intojhanurag
force-pushed
the
feat/temperature-capability-registry
branch
2 times, most recently
from
1bfa6f4 to
3bc544f
Compare
…gistry Some models (claude-opus-4-7, gpt-5-pro) reject temperature/topP/topK with a 400. The model router now checks the capability registry before dispatching and silently strips unsupported sampling parameters. Extends the existing attachment-only capability system to a generic multi-dimension registry. The temperature dimension is populated from models.dev data and auto-updated by the CI regeneration workflow. Closes mastra-ai#16247
intojhanurag
force-pushed
the
feat/temperature-capability-registry
branch
from
3bc544f to
236c487
Compare
Contributor
Author
Known limitations (out of scope for this PR)Custom proxy aliases: If a user configures a custom model alias like Value-conditional rules: Some models reject temperature only under specific conditions (e.g., GPT-5 rejects Both could be addressed later by adding a user-facing capability override API (e.g., |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Extends the model capability registry to include a
temperaturedimension so the model router can proactively strip unsupported sampling parameters (temperature,topP,topK) before dispatching requests. Models likeclaude-opus-4-7andgpt-5-prothat reporttemperature: falseon models.dev now have these parameters silently removed, preventing 400 errors with zero user configuration.CapabilityDimension) that reads bothattachmentandtemperaturefrom per-provider JSON filesmodelSupportsTemperature()public lookup function following the same pattern asmodelSupportsAttachments()stripUnsupportedSamplingParams()inModelRouterLanguageModelthat checks the registry before everydoGenerateanddoStreamcallModelsDevGateway.fetchProviders()to collecttemperaturecapability data from models.dev alongside attachmentsregistry-generatorandgenerate-providersscript to write merged capability files with both dimensionsanthropic.jsonandopenai.jsoncapability files sourced from the live models.dev APIRelated Issue(s)
Fixes #16247
Type of Change
Checklist
ELI5
Some AI models only accept certain “settings” for how creative the response should be. This PR teaches the router to look up which settings each model supports and automatically remove unsupported ones (like temperature) so requests don’t fail.
Summary
temperaturedimension alongside existingattachmentsupport.attachmentandtemperature.ModelRouterLanguageModelto strip unsupported sampling parameters (temperature,topP, andtopKfor streaming) before callingdoGenerate/doStream, based onmodelSupportsTemperature().ModelsDevGateway.fetchProviders()to collect temperature capability data from models.dev and write it into the generated registries.modelSupportsTemperature()(and exported it from core) alongside the existing attachment capability lookup.anthropic.jsonandopenai.jsontemperature capability lists (and adjusted attachment allowlists), and added tests to verify the router’s parameter-stripping behavior.