Fix OpenAI o1 model parameter translation (resolves #8)#11
Merged
Conversation
- Introduced `translate_options/3` callback in the `ReqLLM.Provider` module to allow providers to modify option keys and values before API calls, addressing parameter name differences and model-specific restrictions. - Added `on_unsupported` option in the schema to manage unsupported parameter translations with configurable behaviors (warn, error, ignore). - Enhanced the `ReqLLM.Generation` module to utilize the new translation capabilities, ensuring proper handling of options during request preparation. - Implemented translation helper functions in the `ReqLLM.Provider.DSL` module for renaming and dropping options, along with combining warnings. - Added comprehensive test coverage for translation functionality in both the `OpenAI` provider and the generation pipeline, ensuring robust validation of option handling.
- Updated AGENTS.md to include the `translate_options/3` callback, detailing its role in handling model-specific parameter requirements. - Added a section in usage-rules.md explaining the automatic parameter translation feature, with examples demonstrating its implementation. - Revised README.md to highlight the automatic parameter translation for model-specific requirements, improving clarity for users.
- Add translate_options/3 callback to Provider behavior for model-specific parameter translation - Implement OpenAI o1 model parameter translation: max_tokens → max_completion_tokens and drop temperature - Fix encode_chat_body to use maybe_put for conditional parameters instead of hardcoding - Add comprehensive tests for translation functionality and integration - Update documentation to reflect new options translation capability - Add demo script proving Issue #8 resolution Resolves #8: OpenAI o1 models now work with standard ReqLLM parameters through automatic translation Amp-Thread-ID: https://ampcode.com/threads/T-f9ddd7ac-ace0-4db3-86fb-9aa00e33592a Co-authored-by: Amp <amp@ampcode.com>
Pull Request Test Coverage Report for Build 56683a4cb50c99c9f38265cfa4419f20280ad9f8-PR-11Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
There was a problem hiding this comment.
Pull Request Overview
This PR implements automatic parameter translation for OpenAI o1 models to resolve compatibility issues with ReqLLM's standard API parameters. The changes add a provider-level translation system that allows automatic conversion of standard parameters (like max_tokens) to model-specific requirements (like max_completion_tokens for o1 models).
- Added provider options translation system with
translate_options/3callback - Implemented OpenAI o1 model parameter translation with warning system
- Fixed request encoding to properly handle translated parameters
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/req_llm/provider.ex | Added optional translate_options/3 callback to Provider behavior |
| lib/req_llm/provider/dsl.ex | Added translation helper functions to DSL for providers |
| lib/req_llm/generation.ex | Integrated translation pipeline and added on_unsupported option |
| lib/req_llm/providers/openai.ex | Implemented o1 model translation and fixed encoding with maybe_put |
| test/req_llm/providers/openai_test.exs | Comprehensive unit tests for OpenAI translation functionality |
| test/req_llm/provider_dsl_translation_test.exs | Tests for DSL translation helper functions |
| test/req_llm/generation_translation_test.exs | Integration tests for translation pipeline |
| usage-rules.md | Added documentation for provider-specific parameter handling |
| README.md | Added automatic parameter translation to features list |
| AGENTS.md | Updated provider architecture documentation |
Comments suppressed due to low confidence (1)
lib/req_llm/providers/openai.ex:15
- [nitpick] Inconsistent trailing whitespace removed from line 15 but not from other comment lines in the same block. Consider reviewing lines 17 and maintaining consistent formatting.
# Option 1: Set directly in JidoKeys
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Comment on lines
+229
to
+235
| %{ | ||
| model: request.options[:model] || request.options[:id], | ||
| temperature: request.options[:temperature], | ||
| max_tokens: request.options[:max_tokens], | ||
| stream: request.options[:stream] | ||
| } | ||
| |> maybe_put(:temperature, request.options[:temperature]) | ||
| |> maybe_put(:max_tokens, request.options[:max_tokens]) | ||
| |> maybe_put(:max_completion_tokens, request.options[:max_completion_tokens]) |
There was a problem hiding this comment.
The encoding now handles both :max_tokens and :max_completion_tokens parameters, but this could lead to conflicts if both are present after translation. Consider adding validation to ensure only one is used, or document the precedence behavior.
| :ok | ||
|
|
||
| :error -> | ||
| if warnings == [], do: :ok, else: {:error, {:unsupported_options, warnings}} |
There was a problem hiding this comment.
The error tuple {:error, {:unsupported_options, warnings}} may not provide sufficient context for debugging. Consider using a more descriptive error structure or adding the original options to the error message.
Suggested change
| if warnings == [], do: :ok, else: {:error, {:unsupported_options, warnings}} | |
| if warnings == [], do: :ok, else: {:error, {:unsupported_options, warnings, opts}} |
- Implemented `translate_options/3` for OpenAI o3 models to rename `max_tokens` to `max_completion_tokens` and drop the unsupported `temperature` parameter. - This enhancement ensures compatibility with OpenAI o3 model requirements, improving the overall functionality of the provider. Resolves #8: OpenAI o3 models now correctly handle standard ReqLLM parameters through automatic translation.
- Introduced a new validation function `validate_mutex!/3` in the `ReqLLM.Provider.DSL` module to ensure that conflicting parameters are not used together, improving error handling for provider options. - Updated the OpenAI provider to dynamically determine the appropriate token parameter (`max_tokens` or `max_completion_tokens`) based on the model name, enhancing compatibility with different OpenAI models. This change improves the robustness of parameter handling and aligns with the recent enhancements for model-specific option translation.
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
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.
Overview
This PR resolves Issue #8 by implementing automatic parameter translation for OpenAI o1 models that require different parameter names than the standard ReqLLM API.
Problem Solved
OpenAI o1 models have unique parameter requirements:
max_completion_tokensinstead ofmax_tokenstemperatureparameter (must be omitted entirely)Previously, using standard ReqLLM parameters with o1 models would result in API errors.
Solution
1. Added Provider Options Translation System
ReqLLM.Providerbehavior with optionaltranslate_options/3callbackReqLLM.Provider.DSL2. Implemented OpenAI o1 Translation
max_tokens→max_completion_tokensfor o1 modelstemperatureparameter dropped with warning for o1 models3. Fixed Request Encoding Bug
encode_chat_bodyto usemaybe_putfor conditional parametersnullvalues from being included in API requestsKey Changes
translate_options/3callbackon_unsupportedoption and translation integrationDocumentation Updates
Verification
✅ Real API Testing: Confirmed fix works with actual OpenAI API calls
✅ All Tests Pass: No regressions, all existing functionality preserved
✅ Demo Script: Included working demonstration of the fix
✅ Backward Compatibility: Standard models continue to work unchanged
Usage Example
Resolves #8