From 19c00a5e21a31176dcf20f8a74b36cf7b9e2e6f2 Mon Sep 17 00:00:00 2001 From: anthonytwh Date: Tue, 7 Jul 2026 13:27:39 -0400 Subject: [PATCH 1/3] feat(models): add maxCompletionTokens for OpenAI OpenAI's `max_tokens` request parameter is deprecated for the Chat Completions API and is rejected outright by reasoning models (GPT-5 / o-series): 400 Bad Request: Unsupported parameter: 'max_tokens' is not supported with this model. Use 'max_completion_tokens' instead. kagent's OpenAIConfig only exposed maxTokens (-> max_tokens), so there was no way to cap output for those models. Add a sibling maxCompletionTokens field that maps to the OpenAI `max_completion_tokens` parameter. maxTokens is left intact and unchanged for backward compatibility and for OpenAI-compatible endpoints (baseUrl) that still expect max_tokens; the two are independent, so existing configs are unaffected. Wired end to end: - add MaxCompletionTokens to OpenAIConfig (v1alpha2, +kubebuilder Minimum=1) and regenerate the CRDs - carry it on the adk.OpenAI serialization struct - copy Spec.OpenAI.MaxCompletionTokens in the controller translator - emit max_completion_tokens in the Python ADK OpenAI client and the Go ADK OpenAI model client Adds Go translator + Go ADK tests and a Python client test. Signed-off-by: anthonytwh --- go/adk/pkg/agent/agent.go | 23 ++++++----- go/adk/pkg/models/openai.go | 21 +++++----- go/adk/pkg/models/openai_adk.go | 3 ++ go/adk/pkg/models/openai_adk_test.go | 10 +++++ go/api/adk/types.go | 21 +++++----- .../crd/bases/kagent.dev_modelconfigs.yaml | 13 +++++- go/api/v1alpha2/modelconfig_types.go | 12 +++++- .../translator/agent/adk_api_translator.go | 3 ++ .../agent/adk_api_translator_test.go | 41 +++++++++++++++++++ .../templates/kagent.dev_modelconfigs.yaml | 13 +++++- .../src/kagent/adk/models/_openai.py | 3 ++ .../kagent-adk/src/kagent/adk/types.py | 2 + .../tests/unittests/models/test_openai.py | 21 ++++++++++ 13 files changed, 152 insertions(+), 34 deletions(-) diff --git a/go/adk/pkg/agent/agent.go b/go/adk/pkg/agent/agent.go index fa9d633d14..e81ab00c80 100644 --- a/go/adk/pkg/agent/agent.go +++ b/go/adk/pkg/agent/agent.go @@ -191,17 +191,18 @@ func CreateLLM(ctx context.Context, m adk.Model, log logr.Logger) (adkmodel.LLM, switch m := m.(type) { case *adk.OpenAI: cfg := &models.OpenAIConfig{ - TransportConfig: transportConfigFromBase(m.BaseModel, m.Timeout), - Model: m.Model, - BaseUrl: m.BaseUrl, - FrequencyPenalty: m.FrequencyPenalty, - MaxTokens: m.MaxTokens, - N: m.N, - PresencePenalty: m.PresencePenalty, - ReasoningEffort: m.ReasoningEffort, - Seed: m.Seed, - Temperature: m.Temperature, - TopP: m.TopP, + TransportConfig: transportConfigFromBase(m.BaseModel, m.Timeout), + Model: m.Model, + BaseUrl: m.BaseUrl, + FrequencyPenalty: m.FrequencyPenalty, + MaxTokens: m.MaxTokens, + MaxCompletionTokens: m.MaxCompletionTokens, + N: m.N, + PresencePenalty: m.PresencePenalty, + ReasoningEffort: m.ReasoningEffort, + Seed: m.Seed, + Temperature: m.Temperature, + TopP: m.TopP, } return models.NewOpenAIModelWithLogger(cfg, log) diff --git a/go/adk/pkg/models/openai.go b/go/adk/pkg/models/openai.go index fc975cce00..a4665b3fb4 100644 --- a/go/adk/pkg/models/openai.go +++ b/go/adk/pkg/models/openai.go @@ -19,16 +19,17 @@ import ( // OpenAIConfig holds OpenAI configuration type OpenAIConfig struct { TransportConfig - Model string - BaseUrl string - FrequencyPenalty *float64 - MaxTokens *int - N *int - PresencePenalty *float64 - ReasoningEffort *string - Seed *int - Temperature *float64 - TopP *float64 + Model string + BaseUrl string + FrequencyPenalty *float64 + MaxTokens *int + MaxCompletionTokens *int + N *int + PresencePenalty *float64 + ReasoningEffort *string + Seed *int + Temperature *float64 + TopP *float64 } // AzureOpenAIConfig holds Azure OpenAI configuration diff --git a/go/adk/pkg/models/openai_adk.go b/go/adk/pkg/models/openai_adk.go index dd5b3e6d21..b30ca86f20 100644 --- a/go/adk/pkg/models/openai_adk.go +++ b/go/adk/pkg/models/openai_adk.go @@ -174,6 +174,9 @@ func applyOpenAIConfig(params *openai.ChatCompletionNewParams, cfg *OpenAIConfig if cfg.MaxTokens != nil { params.MaxTokens = openai.Int(int64(*cfg.MaxTokens)) } + if cfg.MaxCompletionTokens != nil { + params.MaxCompletionTokens = openai.Int(int64(*cfg.MaxCompletionTokens)) + } if cfg.TopP != nil { params.TopP = openai.Float(*cfg.TopP) } diff --git a/go/adk/pkg/models/openai_adk_test.go b/go/adk/pkg/models/openai_adk_test.go index 05de50f733..da17f62b06 100644 --- a/go/adk/pkg/models/openai_adk_test.go +++ b/go/adk/pkg/models/openai_adk_test.go @@ -248,6 +248,16 @@ func TestApplyOpenAIConfig(t *testing.T) { } }) + t.Run("config with max_completion_tokens", func(t *testing.T) { + n := 100 + cfg := &OpenAIConfig{MaxCompletionTokens: &n} + var params openai.ChatCompletionNewParams + applyOpenAIConfig(¶ms, cfg) + if !params.MaxCompletionTokens.Valid() || params.MaxCompletionTokens.Value != 100 { + t.Errorf("MaxCompletionTokens: Valid=%v, Value=%v, want (true, 100)", params.MaxCompletionTokens.Valid(), params.MaxCompletionTokens.Value) + } + }) + t.Run("config with reasoning_effort", func(t *testing.T) { effort := "medium" cfg := &OpenAIConfig{ReasoningEffort: &effort} diff --git a/go/api/adk/types.go b/go/api/adk/types.go index 502a1cde3a..549ec8ed51 100644 --- a/go/api/adk/types.go +++ b/go/api/adk/types.go @@ -79,16 +79,17 @@ type TokenExchangeConfig struct { type OpenAI struct { BaseModel - BaseUrl string `json:"base_url"` - FrequencyPenalty *float64 `json:"frequency_penalty,omitempty"` - MaxTokens *int `json:"max_tokens,omitempty"` - N *int `json:"n,omitempty"` - PresencePenalty *float64 `json:"presence_penalty,omitempty"` - ReasoningEffort *string `json:"reasoning_effort,omitempty"` - Seed *int `json:"seed,omitempty"` - Temperature *float64 `json:"temperature,omitempty"` - Timeout *int `json:"timeout,omitempty"` - TopP *float64 `json:"top_p,omitempty"` + BaseUrl string `json:"base_url"` + FrequencyPenalty *float64 `json:"frequency_penalty,omitempty"` + MaxTokens *int `json:"max_tokens,omitempty"` + MaxCompletionTokens *int `json:"max_completion_tokens,omitempty"` + N *int `json:"n,omitempty"` + PresencePenalty *float64 `json:"presence_penalty,omitempty"` + ReasoningEffort *string `json:"reasoning_effort,omitempty"` + Seed *int `json:"seed,omitempty"` + Temperature *float64 `json:"temperature,omitempty"` + Timeout *int `json:"timeout,omitempty"` + TopP *float64 `json:"top_p,omitempty"` // TokenExchange configures dynamic bearer token acquisition TokenExchange *TokenExchangeConfig `json:"token_exchange,omitempty"` diff --git a/go/api/config/crd/bases/kagent.dev_modelconfigs.yaml b/go/api/config/crd/bases/kagent.dev_modelconfigs.yaml index 4dfdc96bce..040e8fde05 100644 --- a/go/api/config/crd/bases/kagent.dev_modelconfigs.yaml +++ b/go/api/config/crd/bases/kagent.dev_modelconfigs.yaml @@ -594,8 +594,19 @@ spec: frequencyPenalty: description: Frequency penalty type: string + maxCompletionTokens: + description: |- + 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. + minimum: 1 + type: integer maxTokens: - description: Maximum tokens to generate + description: |- + 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. type: integer "n": description: N value diff --git a/go/api/v1alpha2/modelconfig_types.go b/go/api/v1alpha2/modelconfig_types.go index 4e0231fbd1..8ba7b86000 100644 --- a/go/api/v1alpha2/modelconfig_types.go +++ b/go/api/v1alpha2/modelconfig_types.go @@ -151,10 +151,20 @@ type OpenAIConfig struct { // +optional Temperature string `json:"temperature,omitempty"` - // Maximum tokens to generate + // 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. // +optional 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. + // +optional + // +kubebuilder:validation:Minimum=1 + MaxCompletionTokens int `json:"maxCompletionTokens,omitempty"` + // Top-p sampling parameter // +optional TopP string `json:"topP,omitempty"` diff --git a/go/core/internal/controller/translator/agent/adk_api_translator.go b/go/core/internal/controller/translator/agent/adk_api_translator.go index f5b1f18797..237c77195d 100644 --- a/go/core/internal/controller/translator/agent/adk_api_translator.go +++ b/go/core/internal/controller/translator/agent/adk_api_translator.go @@ -489,6 +489,9 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC if model.Spec.OpenAI.MaxTokens > 0 { openai.MaxTokens = &model.Spec.OpenAI.MaxTokens } + if model.Spec.OpenAI.MaxCompletionTokens > 0 { + openai.MaxCompletionTokens = &model.Spec.OpenAI.MaxCompletionTokens + } if model.Spec.OpenAI.Seed != nil { openai.Seed = model.Spec.OpenAI.Seed } diff --git a/go/core/internal/controller/translator/agent/adk_api_translator_test.go b/go/core/internal/controller/translator/agent/adk_api_translator_test.go index 8c4202f663..037c9422b8 100644 --- a/go/core/internal/controller/translator/agent/adk_api_translator_test.go +++ b/go/core/internal/controller/translator/agent/adk_api_translator_test.go @@ -470,6 +470,47 @@ func Test_AdkApiTranslator_AzureOpenAIParams(t *testing.T) { assert.Equal(t, &maxTokens, m.MaxTokens) } +func Test_AdkApiTranslator_OpenAIMaxCompletionTokens(t *testing.T) { + scheme := schemev1.Scheme + require.NoError(t, v1alpha2.AddToScheme(scheme)) + + maxCompletionTokens := 8192 + modelConfig := &v1alpha2.ModelConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "m", Namespace: "ns"}, + Spec: v1alpha2.ModelConfigSpec{ + Model: "gpt-5", + Provider: v1alpha2.ModelProviderOpenAI, + APIKeyPassthrough: true, + OpenAI: &v1alpha2.OpenAIConfig{ + MaxCompletionTokens: maxCompletionTokens, + }, + }, + } + agent := &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: "ns"}, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "x", + ModelConfig: "m", + }, + }, + } + + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns"}} + kubeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(ns, modelConfig, agent).Build() + trans := translator.NewAdkApiTranslator(kubeClient, types.NamespacedName{Namespace: "ns", Name: "m"}, nil, "", nil) + + outputs, err := translator.TranslateAgent(context.Background(), trans, agent) + require.NoError(t, err) + + m, ok := outputs.Config.Model.(*adk.OpenAI) + require.True(t, ok) + assert.Equal(t, &maxCompletionTokens, m.MaxCompletionTokens) + // maxTokens (deprecated) must not be set when only maxCompletionTokens is configured. + assert.Nil(t, m.MaxTokens) +} + func Test_AdkApiTranslator_ServiceAccountNameOverride(t *testing.T) { scheme := schemev1.Scheme require.NoError(t, v1alpha2.AddToScheme(scheme)) diff --git a/helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml b/helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml index 4dfdc96bce..040e8fde05 100644 --- a/helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml +++ b/helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml @@ -594,8 +594,19 @@ spec: frequencyPenalty: description: Frequency penalty type: string + maxCompletionTokens: + description: |- + 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. + minimum: 1 + type: integer maxTokens: - description: Maximum tokens to generate + description: |- + 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. type: integer "n": description: N value diff --git a/python/packages/kagent-adk/src/kagent/adk/models/_openai.py b/python/packages/kagent-adk/src/kagent/adk/models/_openai.py index ae6fd6df46..10e448d3b6 100644 --- a/python/packages/kagent-adk/src/kagent/adk/models/_openai.py +++ b/python/packages/kagent-adk/src/kagent/adk/models/_openai.py @@ -374,6 +374,7 @@ class BaseOpenAI(KAgentTLSMixin, BaseLlm): frequency_penalty: Optional[float] = None default_headers: Optional[dict[str, str]] = None max_tokens: Optional[int] = None + max_completion_tokens: Optional[int] = None n: Optional[int] = None presence_penalty: Optional[float] = None reasoning_effort: Optional[str] = None @@ -463,6 +464,8 @@ async def generate_content_async( kwargs["frequency_penalty"] = self.frequency_penalty if self.max_tokens: kwargs["max_tokens"] = self.max_tokens + if self.max_completion_tokens: + kwargs["max_completion_tokens"] = self.max_completion_tokens if self.n is not None: kwargs["n"] = self.n if self.presence_penalty is not None: diff --git a/python/packages/kagent-adk/src/kagent/adk/types.py b/python/packages/kagent-adk/src/kagent/adk/types.py index 766e7136b4..90ffca3c9c 100644 --- a/python/packages/kagent-adk/src/kagent/adk/types.py +++ b/python/packages/kagent-adk/src/kagent/adk/types.py @@ -269,6 +269,7 @@ class OpenAI(BaseLLM): base_url: str | None = None frequency_penalty: float | None = None max_tokens: int | None = None + max_completion_tokens: int | None = Field(default=None, ge=1) n: int | None = None presence_penalty: float | None = None reasoning_effort: str | None = None @@ -643,6 +644,7 @@ def _create_llm_from_model_config(model_config: ModelUnion): default_headers=extra_headers, frequency_penalty=model_config.frequency_penalty, max_tokens=model_config.max_tokens, + max_completion_tokens=model_config.max_completion_tokens, model=model_config.model, n=model_config.n, presence_penalty=model_config.presence_penalty, diff --git a/python/packages/kagent-adk/tests/unittests/models/test_openai.py b/python/packages/kagent-adk/tests/unittests/models/test_openai.py index 5dba0e52ac..56117fe0f0 100644 --- a/python/packages/kagent-adk/tests/unittests/models/test_openai.py +++ b/python/packages/kagent-adk/tests/unittests/models/test_openai.py @@ -389,6 +389,27 @@ async def mock_coro(*args, **kwargs): assert kwargs["max_tokens"] == 4096 +@pytest.mark.asyncio +async def test_generate_content_async_with_max_completion_tokens( + llm_request, generate_content_response, generate_llm_response +): + # Reasoning models (GPT-5 / o-series) reject max_tokens and require + # max_completion_tokens instead; verify it is forwarded verbatim. + openai_llm = OpenAI(model="gpt-5", max_completion_tokens=4096, type="openai", api_key="fake") + with mock.patch.object(openai_llm, "_client") as mock_client: + + async def mock_coro(*args, **kwargs): + return generate_content_response + + mock_client.chat.completions.create.return_value = mock_coro() + + _ = [resp async for resp in openai_llm.generate_content_async(llm_request, stream=False)] + mock_client.chat.completions.create.assert_called_once() + _, kwargs = mock_client.chat.completions.create.call_args + assert kwargs["max_completion_tokens"] == 4096 + assert "max_tokens" not in kwargs + + @pytest.mark.asyncio async def test_streaming_vs_non_streaming_equivalence( openai_llm, llm_request, generate_content_response, generate_streaming_content_response From 0b813ae5c94aa217b6a9575421963753a3d0da3d Mon Sep 17 00:00:00 2001 From: anthonytwh Date: Tue, 7 Jul 2026 15:21:10 -0400 Subject: [PATCH 2/3] fix(models): make OpenAI maxTokens and maxCompletionTokens mutually exclusive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback: if a ModelConfig set both maxTokens and maxCompletionTokens, the client sent both request parameters. They are mutually exclusive on the OpenAI API — reasoning models (GPT-5 / o-series) reject max_tokens, and some OpenAI-compatible endpoints only accept max_tokens — so sending both risks a hard 400 and makes migration brittle. Send exactly one at request-build time, with max_completion_tokens (the modern superset parameter) taking precedence when both are configured, in both the Python and Go ADK OpenAI clients. Single-field configs are unaffected. Adds precedence tests for both runtimes. Signed-off-by: anthonytwh --- go/adk/pkg/models/openai_adk.go | 10 +++++--- go/adk/pkg/models/openai_adk_test.go | 14 +++++++++++ .../src/kagent/adk/models/_openai.py | 9 ++++++-- .../tests/unittests/models/test_openai.py | 23 +++++++++++++++++++ 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/go/adk/pkg/models/openai_adk.go b/go/adk/pkg/models/openai_adk.go index b30ca86f20..79a4832e5d 100644 --- a/go/adk/pkg/models/openai_adk.go +++ b/go/adk/pkg/models/openai_adk.go @@ -171,11 +171,15 @@ func applyOpenAIConfig(params *openai.ChatCompletionNewParams, cfg *OpenAIConfig if cfg.Temperature != nil { params.Temperature = openai.Float(*cfg.Temperature) } - if cfg.MaxTokens != nil { - params.MaxTokens = openai.Int(int64(*cfg.MaxTokens)) - } + // max_tokens and max_completion_tokens are mutually exclusive on the OpenAI + // API: reasoning models (GPT-5 / o-series) reject max_tokens, while some + // OpenAI-compatible endpoints only accept max_tokens. Never send both; + // max_completion_tokens (the modern, superset parameter) takes precedence + // when both are configured. if cfg.MaxCompletionTokens != nil { params.MaxCompletionTokens = openai.Int(int64(*cfg.MaxCompletionTokens)) + } else if cfg.MaxTokens != nil { + params.MaxTokens = openai.Int(int64(*cfg.MaxTokens)) } if cfg.TopP != nil { params.TopP = openai.Float(*cfg.TopP) diff --git a/go/adk/pkg/models/openai_adk_test.go b/go/adk/pkg/models/openai_adk_test.go index da17f62b06..dde5959be8 100644 --- a/go/adk/pkg/models/openai_adk_test.go +++ b/go/adk/pkg/models/openai_adk_test.go @@ -258,6 +258,20 @@ func TestApplyOpenAIConfig(t *testing.T) { } }) + t.Run("both set prefers max_completion_tokens", func(t *testing.T) { + mt := 100 + mct := 200 + cfg := &OpenAIConfig{MaxTokens: &mt, MaxCompletionTokens: &mct} + var params openai.ChatCompletionNewParams + applyOpenAIConfig(¶ms, cfg) + if !params.MaxCompletionTokens.Valid() || params.MaxCompletionTokens.Value != 200 { + t.Errorf("MaxCompletionTokens: Valid=%v, Value=%v, want (true, 200)", params.MaxCompletionTokens.Valid(), params.MaxCompletionTokens.Value) + } + if params.MaxTokens.Valid() { + t.Errorf("MaxTokens should not be set when max_completion_tokens is present, got Value=%v", params.MaxTokens.Value) + } + }) + t.Run("config with reasoning_effort", func(t *testing.T) { effort := "medium" cfg := &OpenAIConfig{ReasoningEffort: &effort} diff --git a/python/packages/kagent-adk/src/kagent/adk/models/_openai.py b/python/packages/kagent-adk/src/kagent/adk/models/_openai.py index 10e448d3b6..9d26157775 100644 --- a/python/packages/kagent-adk/src/kagent/adk/models/_openai.py +++ b/python/packages/kagent-adk/src/kagent/adk/models/_openai.py @@ -462,10 +462,15 @@ async def generate_content_async( if self.frequency_penalty is not None: kwargs["frequency_penalty"] = self.frequency_penalty - if self.max_tokens: - kwargs["max_tokens"] = self.max_tokens + # max_tokens and max_completion_tokens are mutually exclusive on the + # OpenAI API: reasoning models (GPT-5 / o-series) reject max_tokens, + # while some OpenAI-compatible endpoints only accept max_tokens. Never + # send both; max_completion_tokens (the modern, superset parameter) + # takes precedence when both are configured. if self.max_completion_tokens: kwargs["max_completion_tokens"] = self.max_completion_tokens + elif self.max_tokens: + kwargs["max_tokens"] = self.max_tokens if self.n is not None: kwargs["n"] = self.n if self.presence_penalty is not None: diff --git a/python/packages/kagent-adk/tests/unittests/models/test_openai.py b/python/packages/kagent-adk/tests/unittests/models/test_openai.py index 56117fe0f0..90b2d5fa7d 100644 --- a/python/packages/kagent-adk/tests/unittests/models/test_openai.py +++ b/python/packages/kagent-adk/tests/unittests/models/test_openai.py @@ -410,6 +410,29 @@ async def mock_coro(*args, **kwargs): assert "max_tokens" not in kwargs +@pytest.mark.asyncio +async def test_max_tokens_and_max_completion_tokens_are_mutually_exclusive( + llm_request, generate_content_response, generate_llm_response +): + # If both are configured, only max_completion_tokens is sent (it takes + # precedence); sending both would 400 on reasoning models. + openai_llm = OpenAI( + model="gpt-5", max_tokens=1024, max_completion_tokens=4096, type="openai", api_key="fake" + ) + with mock.patch.object(openai_llm, "_client") as mock_client: + + async def mock_coro(*args, **kwargs): + return generate_content_response + + mock_client.chat.completions.create.return_value = mock_coro() + + _ = [resp async for resp in openai_llm.generate_content_async(llm_request, stream=False)] + mock_client.chat.completions.create.assert_called_once() + _, kwargs = mock_client.chat.completions.create.call_args + assert kwargs["max_completion_tokens"] == 4096 + assert "max_tokens" not in kwargs + + @pytest.mark.asyncio async def test_streaming_vs_non_streaming_equivalence( openai_llm, llm_request, generate_content_response, generate_streaming_content_response From 9283c159a9eda3511b607ee62960b23adebe8c11 Mon Sep 17 00:00:00 2001 From: anthonytwh Date: Wed, 8 Jul 2026 16:51:39 -0400 Subject: [PATCH 3/3] fix(models): enforce OpenAI maxTokens/maxCompletionTokens mutual exclusivity and minimum Add a type-level CEL validation rule rejecting ModelConfigs that set both maxTokens and maxCompletionTokens, and add the missing Minimum=1 marker to maxTokens so non-positive values are rejected at admission rather than silently ignored by the translator. Regenerate CRD bases and helm templates, and add an envtest-based CEL test covering both-set rejection, minimum enforcement, and single-field configs. Signed-off-by: anthonytwh --- .../crd/bases/kagent.dev_modelconfigs.yaml | 6 + go/api/v1alpha2/modelconfig_cel_test.go | 158 ++++++++++++++++++ go/api/v1alpha2/modelconfig_types.go | 5 + .../templates/kagent.dev_modelconfigs.yaml | 6 + 4 files changed, 175 insertions(+) create mode 100644 go/api/v1alpha2/modelconfig_cel_test.go diff --git a/go/api/config/crd/bases/kagent.dev_modelconfigs.yaml b/go/api/config/crd/bases/kagent.dev_modelconfigs.yaml index 040e8fde05..f660143f0f 100644 --- a/go/api/config/crd/bases/kagent.dev_modelconfigs.yaml +++ b/go/api/config/crd/bases/kagent.dev_modelconfigs.yaml @@ -600,6 +600,7 @@ spec: `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. minimum: 1 type: integer maxTokens: @@ -607,6 +608,8 @@ spec: 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. + minimum: 1 type: integer "n": description: N value @@ -663,6 +666,9 @@ spec: description: Top-p sampling parameter type: string type: object + x-kubernetes-validations: + - message: maxTokens and maxCompletionTokens are mutually exclusive + rule: '!(has(self.maxTokens) && has(self.maxCompletionTokens))' provider: default: OpenAI description: The provider of the model diff --git a/go/api/v1alpha2/modelconfig_cel_test.go b/go/api/v1alpha2/modelconfig_cel_test.go new file mode 100644 index 0000000000..574d9ce794 --- /dev/null +++ b/go/api/v1alpha2/modelconfig_cel_test.go @@ -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) + }) + } +} diff --git a/go/api/v1alpha2/modelconfig_types.go b/go/api/v1alpha2/modelconfig_types.go index 8ba7b86000..aa87c0b2fc 100644 --- a/go/api/v1alpha2/modelconfig_types.go +++ b/go/api/v1alpha2/modelconfig_types.go @@ -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 @@ -154,13 +156,16 @@ type OpenAIConfig struct { // 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"` diff --git a/helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml b/helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml index 040e8fde05..f660143f0f 100644 --- a/helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml +++ b/helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml @@ -600,6 +600,7 @@ spec: `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. minimum: 1 type: integer maxTokens: @@ -607,6 +608,8 @@ spec: 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. + minimum: 1 type: integer "n": description: N value @@ -663,6 +666,9 @@ spec: description: Top-p sampling parameter type: string type: object + x-kubernetes-validations: + - message: maxTokens and maxCompletionTokens are mutually exclusive + rule: '!(has(self.maxTokens) && has(self.maxCompletionTokens))' provider: default: OpenAI description: The provider of the model