Skip to content

Commit 5496a8c

Browse files
committed
Add mutual exclusivity validation for maxTokens and maxCompletionTokens in OpenAI ChatOptions
- Add SLF4J logger to OpenAiChatOptions class for validation warnings - Implement 'last-set-wins' validation in builder methods for maxTokens() and maxCompletionTokens() - Add javadoc with model-specific usage guidance: * maxTokens: Use for non-reasoning models (gpt-4o, gpt-3.5-turbo) * maxCompletionTokens: Required for reasoning models (o1, o3, o4-mini series) - Add 8 unit tests covering mutual exclusivity scenarios: * Validation when setting conflicting parameters * Null value handling (no validation triggered) * Individual parameter setting * Direct setter behavior (no validation enforced) - Fix existing unit tests that set both parameters simultaneously - Update OpenAI documentation with detailed usage patterns and examples - Add model compatibility table and builder validation examples This ensures OpenAI API compatibility by preventing simultaneous use of mutually exclusive token limit parameters, matching the robust validation already implemented for Azure OpenAI integration. Signed-off-by: Mark Pollack <[email protected]>
1 parent 987faf9 commit 5496a8c

File tree

3 files changed

+393
-7
lines changed

3 files changed

+393
-7
lines changed

models/spring-ai-openai/src/main/java/org/springframework/ai/openai/OpenAiChatOptions.java

Lines changed: 85 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@
3030
import com.fasterxml.jackson.annotation.JsonInclude.Include;
3131
import com.fasterxml.jackson.annotation.JsonProperty;
3232

33+
import org.slf4j.Logger;
34+
import org.slf4j.LoggerFactory;
35+
3336
import org.springframework.ai.model.ModelOptionsUtils;
3437
import org.springframework.ai.model.tool.ToolCallingChatOptions;
3538
import org.springframework.ai.openai.api.OpenAiApi;
@@ -55,6 +58,8 @@
5558
@JsonInclude(Include.NON_NULL)
5659
public class OpenAiChatOptions implements ToolCallingChatOptions {
5760

61+
private static final Logger logger = LoggerFactory.getLogger(OpenAiChatOptions.class);
62+
5863
// @formatter:off
5964
/**
6065
* ID of the model to use.
@@ -84,13 +89,31 @@ public class OpenAiChatOptions implements ToolCallingChatOptions {
8489
*/
8590
private @JsonProperty("top_logprobs") Integer topLogprobs;
8691
/**
87-
* The maximum number of tokens to generate in the chat completion. The total length of input
88-
* tokens and generated tokens is limited by the model's context length.
92+
* The maximum number of tokens to generate in the chat completion.
93+
* The total length of input tokens and generated tokens is limited by the model's context length.
94+
*
95+
* <p><strong>Model-specific usage:</strong></p>
96+
* <ul>
97+
* <li><strong>Use for non-reasoning models</strong> (e.g., gpt-4o, gpt-3.5-turbo)</li>
98+
* <li><strong>Cannot be used with reasoning models</strong> (e.g., o1, o3, o4-mini series)</li>
99+
* </ul>
100+
*
101+
* <p><strong>Mutual exclusivity:</strong> This parameter cannot be used together with
102+
* {@link #maxCompletionTokens}. Setting both will result in an API error.</p>
89103
*/
90104
private @JsonProperty("max_tokens") Integer maxTokens;
91105
/**
92106
* An upper bound for the number of tokens that can be generated for a completion,
93107
* including visible output tokens and reasoning tokens.
108+
*
109+
* <p><strong>Model-specific usage:</strong></p>
110+
* <ul>
111+
* <li><strong>Required for reasoning models</strong> (e.g., o1, o3, o4-mini series)</li>
112+
* <li><strong>Cannot be used with non-reasoning models</strong> (e.g., gpt-4o, gpt-3.5-turbo)</li>
113+
* </ul>
114+
*
115+
* <p><strong>Mutual exclusivity:</strong> This parameter cannot be used together with
116+
* {@link #maxTokens}. Setting both will result in an API error.</p>
94117
*/
95118
private @JsonProperty("max_completion_tokens") Integer maxCompletionTokens;
96119
/**
@@ -678,12 +701,72 @@ public Builder topLogprobs(Integer topLogprobs) {
678701
return this;
679702
}
680703

704+
/**
705+
* Sets the maximum number of tokens to generate in the chat completion. The total
706+
* length of input tokens and generated tokens is limited by the model's context
707+
* length.
708+
*
709+
* <p>
710+
* <strong>Model-specific usage:</strong>
711+
* </p>
712+
* <ul>
713+
* <li><strong>Use for non-reasoning models</strong> (e.g., gpt-4o,
714+
* gpt-3.5-turbo)</li>
715+
* <li><strong>Cannot be used with reasoning models</strong> (e.g., o1, o3,
716+
* o4-mini series)</li>
717+
* </ul>
718+
*
719+
* <p>
720+
* <strong>Mutual exclusivity:</strong> This parameter cannot be used together
721+
* with {@link #maxCompletionTokens(Integer)}. If both are set, the last one set
722+
* will be used and the other will be cleared with a warning.
723+
* </p>
724+
* @param maxTokens the maximum number of tokens to generate, or null to unset
725+
* @return this builder instance
726+
*/
681727
public Builder maxTokens(Integer maxTokens) {
728+
if (maxTokens != null && this.options.maxCompletionTokens != null) {
729+
logger
730+
.warn("Both maxTokens and maxCompletionTokens are set. OpenAI API does not support setting both parameters simultaneously. "
731+
+ "The previously set maxCompletionTokens ({}) will be cleared and maxTokens ({}) will be used.",
732+
this.options.maxCompletionTokens, maxTokens);
733+
this.options.maxCompletionTokens = null;
734+
}
682735
this.options.maxTokens = maxTokens;
683736
return this;
684737
}
685738

739+
/**
740+
* Sets an upper bound for the number of tokens that can be generated for a
741+
* completion, including visible output tokens and reasoning tokens.
742+
*
743+
* <p>
744+
* <strong>Model-specific usage:</strong>
745+
* </p>
746+
* <ul>
747+
* <li><strong>Required for reasoning models</strong> (e.g., o1, o3, o4-mini
748+
* series)</li>
749+
* <li><strong>Cannot be used with non-reasoning models</strong> (e.g., gpt-4o,
750+
* gpt-3.5-turbo)</li>
751+
* </ul>
752+
*
753+
* <p>
754+
* <strong>Mutual exclusivity:</strong> This parameter cannot be used together
755+
* with {@link #maxTokens(Integer)}. If both are set, the last one set will be
756+
* used and the other will be cleared with a warning.
757+
* </p>
758+
* @param maxCompletionTokens the maximum number of completion tokens to generate,
759+
* or null to unset
760+
* @return this builder instance
761+
*/
686762
public Builder maxCompletionTokens(Integer maxCompletionTokens) {
763+
if (maxCompletionTokens != null && this.options.maxTokens != null) {
764+
logger
765+
.warn("Both maxTokens and maxCompletionTokens are set. OpenAI API does not support setting both parameters simultaneously. "
766+
+ "The previously set maxTokens ({}) will be cleared and maxCompletionTokens ({}) will be used.",
767+
this.options.maxTokens, maxCompletionTokens);
768+
this.options.maxTokens = null;
769+
}
687770
this.options.maxCompletionTokens = maxCompletionTokens;
688771
return this;
689772
}

models/spring-ai-openai/src/test/java/org/springframework/ai/openai/OpenAiChatOptionsTests.java

Lines changed: 250 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ void testBuilderWithAllFields() {
9191
"streamOptions", "seed", "stop", "temperature", "topP", "tools", "toolChoice", "user",
9292
"parallelToolCalls", "store", "metadata", "reasoningEffort", "internalToolExecutionEnabled",
9393
"httpHeaders", "toolContext")
94-
.containsExactly("test-model", 0.5, logitBias, true, 5, 100, 50, 2, outputModalities, outputAudio, 0.8,
94+
.containsExactly("test-model", 0.5, logitBias, true, 5, null, 50, 2, outputModalities, outputAudio, 0.8,
9595
responseFormat, streamOptions, 12345, stopSequences, 0.7, 0.9, tools, toolChoice, "test-user", true,
9696
false, metadata, "medium", false, Map.of("header1", "value1"), toolContext);
9797

@@ -120,8 +120,8 @@ void testCopy() {
120120
.logitBias(logitBias)
121121
.logprobs(true)
122122
.topLogprobs(5)
123-
.maxTokens(100)
124-
.maxCompletionTokens(50)
123+
.maxCompletionTokens(50) // Only set maxCompletionTokens to avoid validation
124+
// conflict
125125
.N(2)
126126
.outputModalities(outputModalities)
127127
.outputAudio(outputAudio)
@@ -280,4 +280,251 @@ void testFromOptions_webSearchOptions() {
280280
assertThat(target.getWebSearchOptions().userLocation().approximate().timezone()).isEqualTo("UTC+8");
281281
}
282282

283+
@Test
284+
void testEqualsAndHashCode() {
285+
OpenAiChatOptions options1 = OpenAiChatOptions.builder()
286+
.model("test-model")
287+
.temperature(0.7)
288+
.maxTokens(100)
289+
.build();
290+
291+
OpenAiChatOptions options2 = OpenAiChatOptions.builder()
292+
.model("test-model")
293+
.temperature(0.7)
294+
.maxTokens(100)
295+
.build();
296+
297+
OpenAiChatOptions options3 = OpenAiChatOptions.builder()
298+
.model("different-model")
299+
.temperature(0.7)
300+
.maxTokens(100)
301+
.build();
302+
303+
// Test equals
304+
assertThat(options1).isEqualTo(options2);
305+
assertThat(options1).isNotEqualTo(options3);
306+
assertThat(options1).isNotEqualTo(null);
307+
assertThat(options1).isEqualTo(options1);
308+
309+
// Test hashCode
310+
assertThat(options1.hashCode()).isEqualTo(options2.hashCode());
311+
assertThat(options1.hashCode()).isNotEqualTo(options3.hashCode());
312+
}
313+
314+
@Test
315+
void testBuilderWithNullValues() {
316+
OpenAiChatOptions options = OpenAiChatOptions.builder()
317+
.temperature(null)
318+
.logitBias(null)
319+
.stop(null)
320+
.tools(null)
321+
.metadata(null)
322+
.build();
323+
324+
assertThat(options.getModel()).isNull();
325+
assertThat(options.getTemperature()).isNull();
326+
assertThat(options.getLogitBias()).isNull();
327+
assertThat(options.getStop()).isNull();
328+
assertThat(options.getTools()).isNull();
329+
assertThat(options.getMetadata()).isNull();
330+
}
331+
332+
@Test
333+
void testBuilderChaining() {
334+
OpenAiChatOptions.Builder builder = OpenAiChatOptions.builder();
335+
336+
OpenAiChatOptions.Builder result = builder.model("test-model").temperature(0.7).maxTokens(100);
337+
338+
assertThat(result).isSameAs(builder);
339+
340+
OpenAiChatOptions options = result.build();
341+
assertThat(options.getModel()).isEqualTo("test-model");
342+
assertThat(options.getTemperature()).isEqualTo(0.7);
343+
assertThat(options.getMaxTokens()).isEqualTo(100);
344+
}
345+
346+
@Test
347+
void testNullAndEmptyCollections() {
348+
OpenAiChatOptions options = new OpenAiChatOptions();
349+
350+
// Test setting null collections
351+
options.setLogitBias(null);
352+
options.setStop(null);
353+
options.setTools(null);
354+
options.setMetadata(null);
355+
options.setOutputModalities(null);
356+
357+
assertThat(options.getLogitBias()).isNull();
358+
assertThat(options.getStop()).isNull();
359+
assertThat(options.getTools()).isNull();
360+
assertThat(options.getMetadata()).isNull();
361+
assertThat(options.getOutputModalities()).isNull();
362+
363+
// Test setting empty collections
364+
options.setLogitBias(new HashMap<>());
365+
options.setStop(new ArrayList<>());
366+
options.setTools(new ArrayList<>());
367+
options.setMetadata(new HashMap<>());
368+
options.setOutputModalities(new ArrayList<>());
369+
370+
assertThat(options.getLogitBias()).isEmpty();
371+
assertThat(options.getStop()).isEmpty();
372+
assertThat(options.getTools()).isEmpty();
373+
assertThat(options.getMetadata()).isEmpty();
374+
assertThat(options.getOutputModalities()).isEmpty();
375+
}
376+
377+
@Test
378+
void testStreamUsageStreamOptionsInteraction() {
379+
OpenAiChatOptions options = new OpenAiChatOptions();
380+
381+
// Initially false
382+
assertThat(options.getStreamUsage()).isFalse();
383+
assertThat(options.getStreamOptions()).isNull();
384+
385+
// Setting streamUsage to true should set streamOptions
386+
options.setStreamUsage(true);
387+
assertThat(options.getStreamUsage()).isTrue();
388+
assertThat(options.getStreamOptions()).isEqualTo(StreamOptions.INCLUDE_USAGE);
389+
390+
// Setting streamUsage to false should clear streamOptions
391+
options.setStreamUsage(false);
392+
assertThat(options.getStreamUsage()).isFalse();
393+
assertThat(options.getStreamOptions()).isNull();
394+
395+
// Setting streamOptions directly should update streamUsage
396+
options.setStreamOptions(StreamOptions.INCLUDE_USAGE);
397+
assertThat(options.getStreamUsage()).isTrue();
398+
assertThat(options.getStreamOptions()).isEqualTo(StreamOptions.INCLUDE_USAGE);
399+
400+
// Setting streamOptions to null should set streamUsage to false
401+
options.setStreamOptions(null);
402+
assertThat(options.getStreamUsage()).isFalse();
403+
assertThat(options.getStreamOptions()).isNull();
404+
}
405+
406+
@Test
407+
void testStopSequencesAlias() {
408+
OpenAiChatOptions options = new OpenAiChatOptions();
409+
List<String> stopSequences = List.of("stop1", "stop2");
410+
411+
// Setting stopSequences should also set stop
412+
options.setStopSequences(stopSequences);
413+
assertThat(options.getStopSequences()).isEqualTo(stopSequences);
414+
assertThat(options.getStop()).isEqualTo(stopSequences);
415+
416+
// Setting stop should also update stopSequences
417+
List<String> newStop = List.of("stop3", "stop4");
418+
options.setStop(newStop);
419+
assertThat(options.getStop()).isEqualTo(newStop);
420+
assertThat(options.getStopSequences()).isEqualTo(newStop);
421+
}
422+
423+
@Test
424+
void testFromOptionsWithWebSearchOptionsNull() {
425+
OpenAiChatOptions source = OpenAiChatOptions.builder()
426+
.model("test-model")
427+
.temperature(0.7)
428+
.webSearchOptions(null)
429+
.build();
430+
431+
OpenAiChatOptions result = OpenAiChatOptions.fromOptions(source);
432+
assertThat(result.getModel()).isEqualTo("test-model");
433+
assertThat(result.getTemperature()).isEqualTo(0.7);
434+
assertThat(result.getWebSearchOptions()).isNull();
435+
}
436+
437+
@Test
438+
void testCopyChangeIndependence() {
439+
OpenAiChatOptions original = OpenAiChatOptions.builder().model("original-model").temperature(0.5).build();
440+
441+
OpenAiChatOptions copied = original.copy();
442+
443+
// Modify original
444+
original.setModel("modified-model");
445+
original.setTemperature(0.9);
446+
447+
// Verify copy is unchanged
448+
assertThat(copied.getModel()).isEqualTo("original-model");
449+
assertThat(copied.getTemperature()).isEqualTo(0.5);
450+
}
451+
452+
@Test
453+
void testMaxTokensMutualExclusivityValidation() {
454+
// Test that setting maxTokens clears maxCompletionTokens
455+
OpenAiChatOptions options = OpenAiChatOptions.builder()
456+
.maxCompletionTokens(100)
457+
.maxTokens(50) // This should clear maxCompletionTokens
458+
.build();
459+
460+
assertThat(options.getMaxTokens()).isEqualTo(50);
461+
assertThat(options.getMaxCompletionTokens()).isNull();
462+
}
463+
464+
@Test
465+
void testMaxCompletionTokensMutualExclusivityValidation() {
466+
// Test that setting maxCompletionTokens clears maxTokens
467+
OpenAiChatOptions options = OpenAiChatOptions.builder()
468+
.maxTokens(50)
469+
.maxCompletionTokens(100) // This should clear maxTokens
470+
.build();
471+
472+
assertThat(options.getMaxTokens()).isNull();
473+
assertThat(options.getMaxCompletionTokens()).isEqualTo(100);
474+
}
475+
476+
@Test
477+
void testMaxTokensWithNullDoesNotClearMaxCompletionTokens() {
478+
// Test that setting maxTokens to null doesn't trigger validation
479+
OpenAiChatOptions options = OpenAiChatOptions.builder()
480+
.maxCompletionTokens(100)
481+
.maxTokens(null) // This should not clear maxCompletionTokens
482+
.build();
483+
484+
assertThat(options.getMaxTokens()).isNull();
485+
assertThat(options.getMaxCompletionTokens()).isEqualTo(100);
486+
}
487+
488+
@Test
489+
void testMaxCompletionTokensWithNullDoesNotClearMaxTokens() {
490+
// Test that setting maxCompletionTokens to null doesn't trigger validation
491+
OpenAiChatOptions options = OpenAiChatOptions.builder()
492+
.maxTokens(50)
493+
.maxCompletionTokens(null) // This should not clear maxTokens
494+
.build();
495+
496+
assertThat(options.getMaxTokens()).isEqualTo(50);
497+
assertThat(options.getMaxCompletionTokens()).isNull();
498+
}
499+
500+
@Test
501+
void testBuilderCanSetOnlyMaxTokens() {
502+
// Test that we can set only maxTokens without issues
503+
OpenAiChatOptions options = OpenAiChatOptions.builder().maxTokens(100).build();
504+
505+
assertThat(options.getMaxTokens()).isEqualTo(100);
506+
assertThat(options.getMaxCompletionTokens()).isNull();
507+
}
508+
509+
@Test
510+
void testBuilderCanSetOnlyMaxCompletionTokens() {
511+
// Test that we can set only maxCompletionTokens without issues
512+
OpenAiChatOptions options = OpenAiChatOptions.builder().maxCompletionTokens(150).build();
513+
514+
assertThat(options.getMaxTokens()).isNull();
515+
assertThat(options.getMaxCompletionTokens()).isEqualTo(150);
516+
}
517+
518+
@Test
519+
void testSettersMutualExclusivityNotEnforced() {
520+
// Test that direct setters do NOT enforce mutual exclusivity (only builder does)
521+
OpenAiChatOptions options = new OpenAiChatOptions();
522+
options.setMaxTokens(50);
523+
options.setMaxCompletionTokens(100);
524+
525+
// Both should be set when using setters directly
526+
assertThat(options.getMaxTokens()).isEqualTo(50);
527+
assertThat(options.getMaxCompletionTokens()).isEqualTo(100);
528+
}
529+
283530
}

0 commit comments

Comments
 (0)