feat: add reasoningEffort field support in OpenAIChatFormatter#444
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds direct field support for the reasoningEffort parameter in OpenAIChatFormatter, completing the implementation for OpenAI's thinking block/reasoning mode (Issue #98). While the response parsing was already implemented, this change adds the ability to set reasoningEffort directly through GenerateOptions.builder().reasoningEffort("high") instead of only through additionalBodyParam().
Key Changes:
- Added direct
reasoningEffortfield handling inOpenAIChatFormatter.applyOptions()using the same pattern as other generation parameters - Added 3 comprehensive unit tests covering direct field usage, default options merging, and default-only scenarios
- Implementation follows the existing code patterns and integrates seamlessly with the existing
additionalBodyParammechanism
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
OpenAIChatFormatter.java |
Added 6 lines of code to extract and apply the reasoningEffort field from GenerateOptions using the standard getOptionOrDefault() pattern |
OpenAIChatFormatterTest.java |
Added 3 new unit tests to verify the new functionality works correctly with direct field usage, default options, and null options scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Test | ||
| @DisplayName("Should apply reasoningEffort from GenerateOptions field") | ||
| void testApplyReasoningEffortFromField() { | ||
| OpenAIRequest request = OpenAIRequest.builder().model("o1").messages(List.of()).build(); | ||
|
|
||
| GenerateOptions options = GenerateOptions.builder().reasoningEffort("high").build(); | ||
|
|
||
| formatter.applyOptions(request, options, null); | ||
|
|
||
| assertEquals("high", request.getReasoningEffort()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Should apply reasoningEffort with default options") | ||
| void testApplyReasoningEffortWithDefault() { | ||
| OpenAIRequest request = OpenAIRequest.builder().model("o1").messages(List.of()).build(); | ||
|
|
||
| GenerateOptions defaultOptions = | ||
| GenerateOptions.builder().reasoningEffort("low").build(); | ||
| GenerateOptions options = GenerateOptions.builder().reasoningEffort("high").build(); | ||
|
|
||
| formatter.applyOptions(request, options, defaultOptions); | ||
|
|
||
| // Options should override defaultOptions | ||
| assertEquals("high", request.getReasoningEffort()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Should apply reasoningEffort from default when options is null") | ||
| void testApplyReasoningEffortFromDefaultOnly() { | ||
| OpenAIRequest request = OpenAIRequest.builder().model("o1").messages(List.of()).build(); | ||
|
|
||
| GenerateOptions defaultOptions = | ||
| GenerateOptions.builder().reasoningEffort("medium").build(); | ||
|
|
||
| formatter.applyOptions(request, null, defaultOptions); | ||
|
|
||
| assertEquals("medium", request.getReasoningEffort()); | ||
| } |
There was a problem hiding this comment.
Consider adding a test case that validates the behavior when both the direct reasoningEffort field and the additionalBodyParam("reasoning_effort", ...) are set. Based on the implementation in OpenAIChatFormatter.applyOptions(), the additionalBodyParam is applied after the direct field (lines 120-122 with comment "must be last to allow overriding"), which means it will override the direct field value. This behavior should be explicitly tested to ensure it's intentional and to document this precedence for future maintainers.
| @@ -73,6 +73,12 @@ public void applyOptions( | |||
| if (temperature != null) { | |||
| request.setTemperature(temperature); | |||
| } | |||
There was a problem hiding this comment.
Add a blank line before the "Apply reasoning effort" comment to maintain consistency with the formatting of other parameter applications in this method. All other parameter sections (temperature, top_p, frequency penalty, etc.) have a blank line separating them from the previous section.
| } | |
| } |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
2 participants
AgentScope-Java Version
0.9.0-SNAPSHOT
Description
Background
Issue #98 requested support for OpenAI's thinking block/reasoning mode. While the response parsing was already implemented, the request building was missing direct handling of the
reasoningEffortfield fromGenerateOptions.Changes
reasoningEffortparameter handling inOpenAIChatFormatter.applyOptions()testApplyReasoningEffortFromField- test direct field usagetestApplyReasoningEffortWithDefault- test with default optionstestApplyReasoningEffortFromDefaultOnly- test default-only scenarioHow to Test