feat(models): add maxCompletionTokens for OpenAI #2175
There are no files selected for viewing
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| /* | ||
| Copyright 2025. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package v1alpha2 | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| corev1 "k8s.io/api/core/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/runtime" | ||
| ctrl_client "sigs.k8s.io/controller-runtime/pkg/client" | ||
| "sigs.k8s.io/controller-runtime/pkg/envtest" | ||
| ) | ||
|
|
||
| // TestOpenAIConfigCELValidation pins the OpenAIConfig admission rules against a | ||
| // real kube-apiserver loaded with the shipped CRDs: | ||
| // - maxTokens and maxCompletionTokens are mutually exclusive (type-level | ||
| // XValidation), because native OpenAI reasoning models reject max_tokens | ||
| // while some OpenAI-compatible endpoints reject max_completion_tokens, so | ||
| // sending both risks hard 400s. | ||
| // - both fields carry Minimum=1, so a non-positive value is rejected at | ||
| // admission rather than silently ignored by the translator. | ||
| func TestOpenAIConfigCELValidation(t *testing.T) { | ||
| testEnv := &envtest.Environment{ | ||
| BinaryAssetsDirectory: envtestAssetsDir(t), | ||
| CRDDirectoryPaths: []string{crdBasesDir(t)}, | ||
| ErrorIfCRDPathMissing: true, | ||
| } | ||
| cfg, err := testEnv.Start() | ||
| require.NoError(t, err) | ||
| t.Cleanup(func() { _ = testEnv.Stop() }) | ||
|
|
||
| scheme := runtime.NewScheme() | ||
| require.NoError(t, corev1.AddToScheme(scheme)) | ||
| require.NoError(t, AddToScheme(scheme)) | ||
| cl, err := ctrl_client.New(cfg, ctrl_client.Options{Scheme: scheme}) | ||
| require.NoError(t, err) | ||
|
|
||
| ctx := context.Background() | ||
| const ns = "openai-cel" | ||
| require.NoError(t, cl.Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}})) | ||
|
|
||
| cases := []struct { | ||
| name string | ||
| build func() ctrl_client.Object | ||
| wantReject string // substring in admission error; empty means accept | ||
| }{ | ||
| { | ||
| name: "both maxTokens and maxCompletionTokens rejected", | ||
| build: func() ctrl_client.Object { | ||
| return &ModelConfig{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: "mc-both-token-caps", Namespace: ns}, | ||
| Spec: ModelConfigSpec{ | ||
| Model: "gpt-4", | ||
| Provider: ModelProviderOpenAI, | ||
| OpenAI: &OpenAIConfig{MaxTokens: 1000, MaxCompletionTokens: 1000}, | ||
| }, | ||
| } | ||
| }, | ||
| wantReject: "maxTokens and maxCompletionTokens are mutually exclusive", | ||
| }, | ||
| { | ||
| name: "maxTokens below minimum rejected", | ||
| build: func() ctrl_client.Object { | ||
| return &ModelConfig{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: "mc-maxtokens-neg", Namespace: ns}, | ||
| Spec: ModelConfigSpec{ | ||
| Model: "gpt-4", | ||
| Provider: ModelProviderOpenAI, | ||
| OpenAI: &OpenAIConfig{MaxTokens: -1}, | ||
| }, | ||
| } | ||
| }, | ||
| wantReject: "maxTokens", | ||
| }, | ||
| { | ||
| name: "maxCompletionTokens below minimum rejected", | ||
| build: func() ctrl_client.Object { | ||
| return &ModelConfig{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: "mc-maxcompletion-neg", Namespace: ns}, | ||
| Spec: ModelConfigSpec{ | ||
| Model: "gpt-4", | ||
| Provider: ModelProviderOpenAI, | ||
| OpenAI: &OpenAIConfig{MaxCompletionTokens: -1}, | ||
| }, | ||
| } | ||
| }, | ||
| wantReject: "maxCompletionTokens", | ||
| }, | ||
| { | ||
| name: "only maxTokens accepted", | ||
| build: func() ctrl_client.Object { | ||
| return &ModelConfig{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: "mc-maxtokens-only", Namespace: ns}, | ||
| Spec: ModelConfigSpec{ | ||
| Model: "gpt-4", | ||
| Provider: ModelProviderOpenAI, | ||
| OpenAI: &OpenAIConfig{MaxTokens: 1000}, | ||
| }, | ||
| } | ||
| }, | ||
| }, | ||
| { | ||
| name: "only maxCompletionTokens accepted", | ||
| build: func() ctrl_client.Object { | ||
| return &ModelConfig{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: "mc-maxcompletion-only", Namespace: ns}, | ||
| Spec: ModelConfigSpec{ | ||
| Model: "gpt-5", | ||
| Provider: ModelProviderOpenAI, | ||
| OpenAI: &OpenAIConfig{MaxCompletionTokens: 1000}, | ||
| }, | ||
| } | ||
| }, | ||
| }, | ||
| { | ||
| name: "neither token cap accepted", | ||
| build: func() ctrl_client.Object { | ||
| return &ModelConfig{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: "mc-no-token-caps", Namespace: ns}, | ||
| Spec: ModelConfigSpec{ | ||
| Model: "gpt-4", | ||
| Provider: ModelProviderOpenAI, | ||
| OpenAI: &OpenAIConfig{}, | ||
| }, | ||
| } | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, c := range cases { | ||
| t.Run(c.name, func(t *testing.T) { | ||
| err := cl.Create(ctx, c.build()) | ||
| if c.wantReject == "" { | ||
| require.NoError(t, err) | ||
| return | ||
| } | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), c.wantReject) | ||
| }) | ||
| } | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -138,6 +138,8 @@ type TokenExchangeConfig struct { | |
| } | ||
|
|
||
| // OpenAIConfig contains OpenAI-specific configuration options | ||
| // | ||
| // +kubebuilder:validation:XValidation:message="maxTokens and maxCompletionTokens are mutually exclusive",rule="!(has(self.maxTokens) && has(self.maxCompletionTokens))" | ||
| type OpenAIConfig struct { | ||
| // Base URL for the OpenAI API (overrides default) | ||
| // +optional | ||
|
|
@@ -151,10 +153,23 @@ type OpenAIConfig struct { | |
| // +optional | ||
| Temperature string `json:"temperature,omitempty"` | ||
|
|
||
| // Maximum tokens to generate. Sent as the OpenAI `max_tokens` request | ||
| // parameter, which is deprecated and rejected by reasoning models | ||
| // (GPT-5 / o-series). For those models set maxCompletionTokens instead. | ||
| // Mutually exclusive with maxCompletionTokens. | ||
| // +optional | ||
| // +kubebuilder:validation:Minimum=1 | ||
| MaxTokens int `json:"maxTokens,omitempty"` | ||
|
|
||
| // Maximum completion tokens to generate. Sent as the OpenAI | ||
| // `max_completion_tokens` request parameter (an upper bound on visible | ||
| // output plus reasoning tokens). This is the parameter reasoning models | ||
| // (GPT-5 / o-series) require in place of the deprecated maxTokens. | ||
| // Mutually exclusive with maxTokens. | ||
| // +optional | ||
| // +kubebuilder:validation:Minimum=1 | ||
| MaxCompletionTokens int `json:"maxCompletionTokens,omitempty"` | ||
|
Contributor
There was a problem hiding this comment. We should add a validation rule that only one of
Author
There was a problem hiding this comment. Done in 9283c15. Added a type-level CEL rule on So a ModelConfig that sets both is now rejected at admission (matching the existing XValidation pattern used for the TLSConfig rules in this file). The runtime-level precedence guard in the Python/Go clients stays as defense-in-depth for OpenAI-compatible paths that don't go through CRD admission. Added an envtest-based CEL test ( |
||
|
|
||
| // Top-p sampling parameter | ||
| // +optional | ||
| TopP string `json:"topP,omitempty"` | ||
|
|
||
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
maxTokens has no +kubebuilder:validation:Minimum=1 marker but maxCompletionTokens below does..
value like maxTokens: 0 or maxTokens: -1 passes crd validation silently.. and is then ignored by the translator
adding the same minimum marker here would make the two fields consistent in my oprinio
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.
Good catch — fixed in 9283c15.
maxTokensnow carries+kubebuilder:validation:Minimum=1too, somaxTokens: 0/-1is rejected at admission instead of silently ignored by the translator. The regenerated CRD bases/helm templates reflectminimum: 1on both fields, and the new CEL test asserts a negative value on either field is rejected.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.
why do you post llm outputs here instead of writing your own sentences? i think everyone should maintain healthy communication as a sign of respect for the person they are interacting with..