-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix & refactor: add supportsTemperature and Responses API flags #6969
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Extend ModelInfo schema with supportsTemperature and usesResponsesApi capabilities to control request param inclusion and API selection. Refactor OpenAiNativeHandler to generically handle Responses API models instead of hardcoded families, normalizing IDs and gating temperature, verbosity, and max token params via getModelParams. Update GlamaHandler, LiteLLMHandler, UnboundHandler, and XAIHandler to use getModelParams for capability-aware temperature/max token handling. Enhance tests to cover Responses API flows, conversation continuity, and temperature stripping for unsupported models, replacing SSE mocks with SDK responses.create where applicable.
Extend ModelInfo schema with supportsTemperature and usesResponsesApi capabilities to control request param inclusion and API selection. Refactor OpenAiNativeHandler to generically handle Responses API models instead of hardcoded families, normalizing IDs and gating temperature, verbosity, and max token params via getModelParams. Update GlamaHandler, LiteLLMHandler, UnboundHandler, and XAIHandler to use getModelParams for capability-aware temperature/max token handling. Enhance tests to cover Responses API flows, conversation continuity, and temperature stripping for unsupported models, replacing SSE mocks with SDK responses.create where applicable. test(openai-native.spec): add fallback for unexpected call count Adds explicit error throwing in mock implementations when callCount does not match expected cases. Ensures tests fail with clear messages on unexpected execution paths, improving debuggability and test reliability.
… hotfix/responses-api-gpt-5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! I've reviewed the changes and have some suggestions for improvement. The implementation looks solid overall - good work on extending the ModelInfo schema with capability flags and refactoring the handlers to use these for parameter management.
Suggestions:
-
Temperature default handling - In openai-native.ts, the logic for defaultTemperature seems backwards. When supportsTemperature is true, you're passing undefined, but when false you're passing OPENAI_NATIVE_DEFAULT_TEMPERATURE. Should this be reversed?
-
Missing documentation - The new supportsTemperature and usesResponsesApi fields would benefit from more detailed JSDoc comments explaining their purpose and implications.
-
Unused import - GPT5_DEFAULT_TEMPERATURE is still imported in openai-native.ts but was removed from the exports.
-
Magic number - Consider extracting the 1.0 temperature value in model-params.ts as a named constant for hybrid reasoning models.
-
Comment style - The Q&A format comment about GPT-5 temperature could be more concise.
Overall, this is a well-structured refactor that improves the codebase's maintainability. The test coverage is comprehensive.
Reverses the conditional assignment to defaultTemperature so it sets the default when supportsTemperature is true, and undefined otherwise. Prevents models without temperature support from receiving an unintended default value.
Hey @snipeship, Thank you for your contribution! I opened this PR which implements the responses API for all models. I'll close this PR in favor of this on #7067. Again, we appreciate your contributions, and I look forward to seeing more of them! |
Extend ModelInfo schema with supportsTemperature and usesResponsesApi capabilities to control request param inclusion and API selection.
Refactor OpenAiNativeHandler to generically handle Responses API models instead of hardcoded families, normalizing IDs and gating temperature, verbosity, and max token params via getModelParams.
Update GlamaHandler, LiteLLMHandler, UnboundHandler, and XAIHandler to use getModelParams for capability-aware temperature/max token handling.
Enhance tests to cover Responses API flows, conversation continuity, and temperature stripping for unsupported models, replacing SSE mocks with SDK responses.create where applicable.
Related GitHub Issue
Status: Open.
#6965
Closes: #6965
Roo Code Task Context (Optional)
Description
Fixes some slop as well for better maintaineability
Test Procedure
Run GPT-5, o3, and other providers
Pre-Submission Checklist
Documentation Updates
Additional Notes
Get in Touch
Important
Add
supportsTemperature
andusesResponsesApi
flags to ModelInfo schema and refactor handlers to use these capabilities for parameter management.supportsTemperature
andusesResponsesApi
flags tomodelInfoSchema
inmodel.ts
.OpenAiNativeHandler
to handle Responses API models generically, usinggetModelParams
for parameter gating.GlamaHandler
,LiteLLMHandler
,UnboundHandler
, andXAIHandler
to usegetModelParams
for temperature and max token handling.openai-native.spec.ts
to cover Responses API flows and temperature stripping for unsupported models.unbound.spec.ts
to test temperature handling foropenai/o3-mini
.model-params.spec.ts
for verbosity and reasoning settings.This description was created by
for 301731a. You can customize this summary. It will automatically update as commits are pushed.