Skip to content

feat(models): add maxCompletionTokens for OpenAI #2175

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.

Sign up for GitHub

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

Open
anthonytwh wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat(models): add maxCompletionTokens for OpenAI #2175

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
19c00a5
feat(models): add maxCompletionTokens for OpenAI
anthonytwh Jul 7, 2026
0b813ae
fix(models): make OpenAI maxTokens and maxCompletionTokens mutually e…
anthonytwh Jul 7, 2026
9283c15
fix(models): enforce OpenAI maxTokens/maxCompletionTokens mutual excl…
anthonytwh Jul 8, 2026
88680c8
Merge branch 'main' into wire-openai-max-completion-tokens
EItanya Jul 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions go/adk/pkg/agent/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
21 changes: 11 additions & 10 deletions go/adk/pkg/models/openai.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions go/adk/pkg/models/openai_adk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
10 changes: 10 additions & 0 deletions go/adk/pkg/models/openai_adk_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(&params, 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}
Expand Down
21 changes: 11 additions & 10 deletions go/api/adk/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
13 changes: 12 additions & 1 deletion go/api/config/crd/bases/kagent.dev_modelconfigs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 11 additions & 1 deletion go/api/v1alpha2/modelconfig_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

@mesutoezdil mesutoezdil Jul 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@anthonytwh anthonytwh Jul 8, 2026

Copy link
Copy Markdown
Author

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. maxTokens now carries +kubebuilder:validation:Minimum=1 too, so maxTokens: 0/-1 is rejected at admission instead of silently ignored by the translator. The regenerated CRD bases/helm templates reflect minimum: 1 on both fields, and the new CEL test asserts a negative value on either field is rejected.

@mesutoezdil mesutoezdil Jul 8, 2026

Copy link
Copy Markdown
Contributor

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. maxTokens now carries +kubebuilder:validation:Minimum=1 too, so maxTokens: 0/-1 is rejected at admission instead of silently ignored by the translator. The regenerated CRD bases/helm templates reflect minimum: 1 on both fields, and the new CEL test asserts a negative value on either field is rejected.

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..

@anthonytwh anthonytwh Jul 9, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is automatic, no disrespect here.. Thanks for your comment on the PR.


// 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"`

@supreme-gg-gg supreme-gg-gg Jul 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a validation rule that only one of MaxCompletionTokens or MaxTokens is provided (if I understood properly from the description they should be mutually exclusive)?

@anthonytwh anthonytwh Jul 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9283c15. Added a type-level CEL rule on OpenAIConfig:

+kubebuilder:validation:XValidation:message="maxTokens and maxCompletionTokens are mutually exclusive",rule="!(has(self.maxTokens) && has(self.maxCompletionTokens))"

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 (modelconfig_cel_test.go) covering both-set rejection plus the single-field and neither-field cases.


// Top-p sampling parameter
// +optional
TopP string `json:"topP,omitempty"`
Expand Down
3 changes: 3 additions & 0 deletions go/core/internal/controller/translator/agent/adk_api_translator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
41 changes: 41 additions & 0 deletions go/core/internal/controller/translator/agent/adk_api_translator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
13 changes: 12 additions & 1 deletion helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions python/packages/kagent-adk/src/kagent/adk/models/_openai.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions python/packages/kagent-adk/src/kagent/adk/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
21 changes: 21 additions & 0 deletions python/packages/kagent-adk/tests/unittests/models/test_openai.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading