-
Notifications
You must be signed in to change notification settings - Fork 843
fix(model): apply thinkingBudget to OpenAI-compatible API request #1727
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,6 +81,13 @@ public void applyOptions( | |
| request.setReasoningEffort(reasoningEffort); | ||
| } | ||
|
|
||
| // Apply thinking budget | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [praise] Clean, minimal addition that follows the established apply-options pattern exactly. The null-guard and |
||
| Integer thinkingBudget = | ||
| getOptionOrDefault(options, defaultOptions, GenerateOptions::getThinkingBudget); | ||
| if (thinkingBudget != null) { | ||
| request.setThinkingBudget(thinkingBudget); | ||
| } | ||
|
|
||
| // Apply top_p | ||
| Double topP = getOptionOrDefault(options, defaultOptions, GenerateOptions::getTopP); | ||
| if (topP != null) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,6 +128,17 @@ public class OpenAIRequest { | |
| @JsonProperty("reasoning_effort") | ||
| private String reasoningEffort; | ||
|
|
||
| /** | ||
| * Maximum tokens allocated for the model's internal thinking/reasoning process. | ||
| * Supported by OpenAI-compatible providers that expose a thinking budget parameter | ||
| * (e.g., Alibaba Cloud Bailian/DashScope Qwen3 series via {@code thinking_budget}). | ||
| * When set, the model is prevented from spending more than this many tokens on | ||
| * reasoning, leaving the remainder of {@code max_completion_tokens} for the | ||
| * visible response. | ||
| */ | ||
| @JsonProperty("thinking_budget") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [praise] Good Javadoc — citing a concrete provider (DashScope Qwen3) gives future maintainers the context they need to understand why this non-standard field exists on an OpenAI-compatible DTO.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [praise] Good Javadoc — citing a concrete provider (DashScope Qwen3) gives future maintainers the context they need to understand why this non-standard field exists on an OpenAI-compatible DTO. |
||
| private Integer thinkingBudget; | ||
|
|
||
| /** | ||
| * Controls whether to allow parallel tool calls. | ||
| * Set to false to disable parallel tool calling. | ||
|
|
@@ -345,6 +356,14 @@ public void setReasoningEffort(String reasoningEffort) { | |
| this.reasoningEffort = reasoningEffort; | ||
| } | ||
|
|
||
| public Integer getThinkingBudget() { | ||
| return thinkingBudget; | ||
| } | ||
|
|
||
| public void setThinkingBudget(Integer thinkingBudget) { | ||
| this.thinkingBudget = thinkingBudget; | ||
| } | ||
|
|
||
| public Boolean getParallelToolCalls() { | ||
| return parallelToolCalls; | ||
| } | ||
|
|
@@ -554,6 +573,11 @@ public Builder reasoningEffort(String reasoningEffort) { | |
| return this; | ||
| } | ||
|
|
||
| public Builder thinkingBudget(Integer thinkingBudget) { | ||
| request.setThinkingBudget(thinkingBudget); | ||
| return this; | ||
| } | ||
|
|
||
| public Builder parallelToolCalls(Boolean parallelToolCalls) { | ||
| request.setParallelToolCalls(parallelToolCalls); | ||
| return this; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -379,6 +379,64 @@ void testApplyToolsWithNullStrict() { | |
| } | ||
| } | ||
|
|
||
| @Nested | ||
| @DisplayName("thinkingBudget Tests") | ||
| class ThinkingBudgetTests { | ||
|
|
||
| @Test | ||
| @DisplayName("Should apply thinkingBudget from GenerateOptions") | ||
| void testApplyThinkingBudget() { | ||
| OpenAIRequest request = | ||
| OpenAIRequest.builder().model("qwen3.7-max").messages(List.of()).build(); | ||
|
|
||
| GenerateOptions options = GenerateOptions.builder().thinkingBudget(4096).build(); | ||
|
|
||
| formatter.applyOptions(request, options, null); | ||
|
|
||
| assertEquals(4096, request.getThinkingBudget()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Should apply thinkingBudget from defaultOptions when options is null") | ||
| void testApplyThinkingBudgetFromDefault() { | ||
| OpenAIRequest request = | ||
| OpenAIRequest.builder().model("qwen3.7-max").messages(List.of()).build(); | ||
|
|
||
| GenerateOptions defaultOptions = GenerateOptions.builder().thinkingBudget(2048).build(); | ||
|
|
||
| formatter.applyOptions(request, null, defaultOptions); | ||
|
|
||
| assertEquals(2048, request.getThinkingBudget()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Options thinkingBudget should override defaultOptions") | ||
| void testThinkingBudgetOptionsOverridesDefault() { | ||
| OpenAIRequest request = | ||
| OpenAIRequest.builder().model("qwen3.7-max").messages(List.of()).build(); | ||
|
|
||
| GenerateOptions defaultOptions = GenerateOptions.builder().thinkingBudget(1000).build(); | ||
| GenerateOptions options = GenerateOptions.builder().thinkingBudget(8000).build(); | ||
|
|
||
| formatter.applyOptions(request, options, defaultOptions); | ||
|
|
||
| assertEquals(8000, request.getThinkingBudget()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Should not set thinkingBudget when not specified") | ||
| void testThinkingBudgetNotSetWhenAbsent() { | ||
| OpenAIRequest request = | ||
| OpenAIRequest.builder().model("gpt-4o").messages(List.of()).build(); | ||
|
|
||
| GenerateOptions options = GenerateOptions.builder().temperature(0.7).build(); | ||
|
|
||
| formatter.applyOptions(request, options, null); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [praise] Solid test coverage for a single-field addition. The four cases (direct set, default fallback, override, absent) mirror the pattern used by other option test groups and give confidence that
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [praise] Solid test coverage for a single-field addition. The four cases (direct set, default fallback, override, absent) mirror the pattern used by other option test groups and give confidence that |
||
|
|
||
| assertNull(request.getThinkingBudget()); | ||
| } | ||
| } | ||
|
|
||
| @Nested | ||
| @DisplayName("applyAdditionalBodyParams Tests") | ||
| class ApplyAdditionalBodyParamsTests { | ||
|
|
||
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.
[praise] Clean, minimal addition that follows the established apply-options pattern exactly. The null-guard and
getOptionOrDefaultusage are consistent with every other field in this method — easy to review and low risk.