Skip to content

fix(dotAI): forward maxCompletionTokens to Azure OpenAI builder for gpt-5.x and o-series models#36608

Open
ihoffmann-dot wants to merge 2 commits into
mainfrom
dot-ai-azure-max-completion-tokens
Open

fix(dotAI): forward maxCompletionTokens to Azure OpenAI builder for gpt-5.x and o-series models#36608
ihoffmann-dot wants to merge 2 commits into
mainfrom
dot-ai-azure-max-completion-tokens

Conversation

@ihoffmann-dot

Copy link
Copy Markdown
Member

Summary

  • maxCompletionTokens was defined in ProviderConfig but never forwarded to the LangChain4J Azure builder, leaving gpt-5.x and o-series users unable to control output length
  • Fixed for both buildChatModel and buildStreamingChatModel
  • Also added missing braces to the existing maxTokens guard (dotCMS convention)

Test plan

  • Configure Azure OpenAI with a gpt-5.x deployment and set maxCompletionTokens in the providerConfig — verify it is respected
  • Configure with a gpt-4.x deployment and set maxTokens — verify it still works unchanged
  • Verify no 500 error from Azure when using gpt-5.x models with the new field

Closes #36607

🤖 Generated with Claude Code

@github-actions github-actions Bot added the Area : Backend PR changes Java/Maven backend code label Jul 16, 2026
@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ihoffmann-dot's task in 3m 43s —— View job


Code Review

  • Read the diff and full file
  • Compared Azure vs OpenAI strategy token-limit logic
  • Checked ProviderConfig field and test coverage
  • Posted review

The core change is correct: maxCompletionTokens was declared in ProviderConfig (line 121) but never forwarded to the Azure builders, so gpt-5.x / o-series users had no way to cap output length. Forwarding it in both buildChatModel and buildStreamingChatModel, gated by requiresCompletionTokens(config), is the right fix. The brace additions match the dotCMS convention.

New Issues

  • 🟡 Medium: AzureOpenAiModelProviderStrategy.java:34-35 (and 50-51) — maxTokens is silently dropped for gpt-5.x / o-series Azure deployments. The two guards are mutually exclusive on requiresCompletionTokens(config): for a gpt-5/o-series deployment, maxTokens is never applied (guard requires !requiresCompletionTokens), and maxCompletionTokens is applied only if that specific field is set. So a user who configures a gpt-5 deployment with only maxTokens (the pre-existing/intuitive field) gets no token limit forwarded at all — it's silently ignored with no log.

    Compare the sibling OpenAiModelProviderStrategy.applyTokenLimit (lines 90-105), which treats the two fields as interchangeable and routes to the correct builder param based on the model:

    final Integer tokens = config.maxCompletionTokens() != null
            ? config.maxCompletionTokens()
            : config.maxTokens();
    if (tokens == null) return;
    if (model.matches("o\\d+.*") || model.matches("gpt-([5-9]|\\d{2,}).*")) {
        maxCompletionTokensFn.accept(tokens);
    } else {
        maxTokensFn.accept(tokens);
    }

    The Azure strategy would be more robust and consistent if it applied the same fallback (use maxCompletionTokens ?? maxTokens, then route by model). As written, the two strategies behave differently for the same config, which is a footgun for anyone migrating a maxTokens-based config to a gpt-5 Azure deployment. Fix this →

  • 🟡 Medium: AzureOpenAiModelProviderStrategy.java:135-137requiresCompletionTokens keys off deploymentName, which on Azure is an arbitrary user-chosen string. For classic Azure OpenAI, the deployment name need not contain the model family (e.g. a gpt-5 deployment named chat-prod). In that case requiresCompletionTokens returns false, so maxCompletionTokens is not forwarded and maxTokens is — the exact 500 error this PR aims to prevent. This is pre-existing behavior (the maxTokens guard already used it), but the PR now depends on it for the new field too. Assumption: Azure deployment names are frequently not model-named. What to verify: whether your deployments are consistently named after the model, or whether detection should fall back to config.model() when the deployment name doesn't match.

Test gaps

  • 🟡 Medium: No unit test exercises buildChatModel/buildStreamingChatModel for the gpt-5/o-series branch. ProviderConfigTest covers config parsing but nothing asserts that maxCompletionTokens reaches the builder (or that maxTokens is correctly excluded). Given the field routing is the whole point of the fix, a small test would guard against regressions.

None of the above block merge — the primary fix is sound. The maxTokens fallback gap is the most worthwhile follow-up.

dot-ai-azure-max-completion-tokens

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

fix(dotAI): maxCompletionTokens not forwarded to Azure OpenAI builder for gpt-5.x and o-series models

1 participant