Skip to content

fix(azure): support an opt-in minimum output-token budget - #7012

Closed
tsruu wants to merge 1 commit into
lidge-jun:devfrom
tsruu:fix/azure-model-validation
Closed

tsruu wants to merge 1 commit into
lidge-jun:devfrom
tsruu:fix/azure-model-validation

Conversation

@tsruu

@tsruu tsruu commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Claude Desktop/Claude Code model-switch validation can send a tiny output budget that an Azure Responses deployment rejects with HTTP 400 (max_output_tokens: integer below minimum value), even though normal conversations with that deployment work.

Add optional provider setting minMaxOutputTokens to the azure / azure-openai adapter. With minMaxOutputTokens: 16, explicit positive integer budgets below 16 become 16. Unconfigured providers, omitted or invalid request budgets, and larger budgets retain existing behavior. The minimum is deployment-specific; enabling this setting authorizes raising all smaller positive request caps on that provider and can increase output usage.

The adapter copies only the changed request envelope before the existing Responses builder, preserving caller-owned input and normal serialization. Spend admission uses the same effective-cap calculation before dispatch, including each Combo target's prepaid booking, so a caller cap of 1 with a floor of 16 cannot pass a pool ceiling of 8. Disk and management validation reject invalid configured floors, and the numeric setting is exposed through the provider editor.

Related: #6696 proposes the same setting for openai-chat. This Azure implementation overlaps with its shared type, validation and field declarations; these need reconciling before both changes merge. Azure leaves an omitted budget omitted; reconciling the declarations must preserve that contract rather than acquire Chat's omitted-budget fallback. Maintainer guidance on a separate PR versus a follow-up to #6696 is welcome. The unset default also preserves the caller-cap contract from #5753.

Maintainer security review is requested for the non-secret provider field-policy addition in src/server/auth-cors.ts and review of the pre-dispatch spend-accounting changes.

Verification

Refreshed in response to the maintainer review. Final validated head: e6ccd76aeae34bd7a78dd2f31fd6cf4d40f5cffb, rebased cleanly onto current dev commit 6da2774369aa32552e318e333902499683d053bc (version 2.85.0). git range-diff confirms the contribution patch is unchanged by the rebase. Related #6696 is still open and has not landed in this base.

Tests use the current repository-required Bun 1.4.3. The runtime was installed in an ignored scratch directory and selected explicitly:

export PATH="$PWD/.tmp/test-runtime/node_modules/@oven/bun-darwin-aarch64/bin:$PATH"
export OCX_TEST_RUNNER_BUN="$PWD/.tmp/test-runtime/node_modules/@oven/bun-darwin-aarch64/bin/bun"
bun scripts/test.ts --parallel=1 \
  tests/providers/azure-adapter.test.ts \
  tests/providers/model-rename-migration.test.ts \
  tests/providers/provider-config-validation.test.ts \
  tests/ci-workflows/file-size-ratchet.test.ts \
  tests/responses/responses-spend-anchor.test.ts \
  tests/responses/responses-first-send-spend-policy.test.ts \
  tests/responses/responses-spend-ledger-wiring.test.ts \
  tests/responses/responses-spend-capacity-guard.test.ts \
  tests/providers/provider-model-discovery-contract.test.ts \
  tests/lab/core-lab-boundary.test.ts
  • 253 passed, 0 failed, 1,398 assertions across 10 files in 4.21 seconds, run serially through the repository's isolated wrapper. Covers Azure caps and omissions, caller immutability, auth/payload behavior, disk/management validation and editor exposure, rename handling, file-size limits, spend admission and fallback recomputation, provider discovery, and the core optional-subsystem boundary.
  • Nine added production-handler regressions cover both Azure aliases, direct/Combo admission, and fallback cap recomputation. The initial eight admission cases failed before the accounting fix and passed afterward; the current-head result is the 253-test run above.
  • bun run typecheck: passed.
  • bun run privacy:scan: passed.
  • bun run structure:check: passed.
  • git diff --check upstream/dev: passed.
  • Documentation: cd docs-site, bun install --frozen-lockfile, then bun run build: passed; 569 pages built and 79,372 internal links checked, with no dependency/lockfile changes.

Broader validation / local resource exception:

OCX_TEST_MAIN_TIMEOUT_MS=180000 bun run test:changed --parallel=1

The current Bun 1.4.3 isolated changed-file run did not complete within the configured 180-second local limit and was terminated by the repository wrapper (exit 124). It selected 1,807 of 2,267 test files but emitted no final completion/test-count summary, so no broad-suite pass is claimed. The captured output contained no test-failure markers, but an interrupted run is not correctness evidence. Full local execution was not attempted: the current import-connected selection covers most of the repository, and serial bounded execution exceeded the local budget. The passing current-head focused regressions and static/docs checks above provide scoped local evidence. Completion of import-connected/full and cross-platform coverage remains for hosted CI, whose current-head workflows require maintainer approval.

No live Azure request was sent from this checkout. The reported deployment failure motivates the fix; mocked adapter and production-handler tests verify request construction and pre-dispatch admission. Local results are author-run evidence, not independent security-review or hosted CI receipts.

The PR stays Draft pending current-head hosted validation, required independent boundary/security review, maintainer sponsorship for src/server/auth-cors.ts, and resolution of any automated feedback. No workflow authorization, sponsorship, review approval, or merge was performed.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the intake: hygiene-blocked Deterministic PR hygiene checks failed label Oct 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Deterministic hygiene checks failed.

  • unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/server/auth-cors.ts.

@github-actions github-actions Bot added the bug Something isn't working label Oct 12, 2026
@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • hygiene: unsponsored_surface.

What to do

  • Fix unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/server/auth-cors.ts.
  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.
@tsruu Tick the boxes once required local validation has passed with commands, results, and any full-suite exception documented, your branch is on the latest dev commit, and every correct Codex and CodeRabbit finding is resolved.

@tsruu
tsruu force-pushed the fix/azure-model-validation branch from e6ce9b6 to e6a463f Compare October 12, 2026 21:28

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Scoped review at e6a463fd32216a213c55789f30cce13d3e8901d1.

The inspected Azure builder copies only the changed envelope and raises smaller explicit positive integer caps, leaving omitted/invalid caps untouched. The shared effectiveAzureOutputBudget also reaches final-route normalization and Combo prepayment; the added production-handler fixtures assert refusal before any send below the floor, actual wire-time reservation and recomputation for the next target. This correctly treats the floor as operator-authorized extra output, rather than advertising a free validation-only exception.

No independent executable receipt in this pass: foreign Cargo/rustc builds occupy the host and local tests must remain serial/bounded. The 183-pass and 45-pass retry results are author-reported; the 180-second changed-graph timeout is not a pass. Current executable CI 38310888783 is action_required, and hygiene still reports the auth-cors field-policy sponsorship hold.

Please refresh against current dev 3bb7a56 (the branch is 24 commits behind) and qualify with current testRunnerBun 1.4.3, not the older 1.4.0 pin stated in Verification. Reconcile the shared minMaxOutputTokens type/validators/rename-field/DTO declarations with #6696 if both proceed; Azure's explicit-budget-only contract must not silently acquire Chat's omitted-budget fallback. Keep Draft pending current-head validation and required independent boundary review. No scan, sponsorship, workflow authorization, approval or merge was performed.

@tsruu
tsruu force-pushed the fix/azure-model-validation branch from e6a463f to e6ccd76 Compare October 13, 2026 06:29
lidge-jun added a commit that referenced this pull request Oct 13, 2026
Some Azure Responses deployments reject Claude Code's tiny model-switch validation budget. Add opt-in provider setting minMaxOutputTokens for the azure / azure-openai adapter: explicit positive integer max_output_tokens below the floor are raised to it; unset providers, omitted or invalid budgets, and larger budgets are unchanged. Spend admission reserves the same effective cap at final route normalization and for each Combo target, and Combo restores the caller's ceiling for targets without a floor.

Based on #7012 by @tsruu. Supersedes #6696 by @ismell0992-afk.

Co-authored-by: tsruu <183051730+tsruu@users.noreply.github.com>
Co-authored-by: ismell0992-afk <255305877+ismell0992-afk@users.noreply.github.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Thank you, @tsruu! Your change landed on dev through #7028 (squash d3d17d559a) with your commit preserved and a Co-authored-by trailer crediting you.

On top of it, #7028 rebased onto the current dev and added one small fix: Combo prepayment now keeps the caller's established output ceiling for targets that have no floor, which a spend contract test added after this PR was written caught. #6696 proposed the same minMaxOutputTokens floor for openai-chat; we kept your narrower design (opt-in, explicit caps only, omitted budget stays omitted) as the single one.

Closing this in favor of #7028. Thanks again for the careful work on the spend admission side.

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

Labels

bug Something isn't working intake: hygiene-blocked Deterministic PR hygiene checks failed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants