Repository navigation
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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. Comment |
|
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. This pull request was already a draft. Its draft status will be preserved after every issue above is resolved. |
e6ce9b6 to
e6a463f
Compare
Ingwannu
left a comment
There was a problem hiding this comment.
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.
e6a463f to
e6ccd76
Compare
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>
|
Thank you, @tsruu! Your change landed on On top of it, #7028 rebased onto the current Closing this in favor of #7028. Thanks again for the careful work on the spend admission side. |
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
minMaxOutputTokensto theazure/azure-openaiadapter. WithminMaxOutputTokens: 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.tsand review of the pre-dispatch spend-accounting changes.Verification
Refreshed in response to the maintainer review. Final validated head:
e6ccd76aeae34bd7a78dd2f31fd6cf4d40f5cffb, rebased cleanly onto currentdevcommit6da2774369aa32552e318e333902499683d053bc(version 2.85.0).git range-diffconfirms 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:
bun run typecheck: passed.bun run privacy:scan: passed.bun run structure:check: passed.git diff --check upstream/dev: passed.cd docs-site,bun install --frozen-lockfile, thenbun run build: passed; 569 pages built and 79,372 internal links checked, with no dependency/lockfile changes.Broader validation / local resource exception:
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
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.