Repository navigation
fix(gateway): keep max_tokens above thinking budget - #3501
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WalkthroughThe Anthropic request builder now raises insufficient ChangesAnthropic thinking budget handling
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/actions/src/prepare-request-body.spec.ts (1)
3839-3845: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the new
as anyassertions.Use
toMatchObjectto assertthinkingandmax_tokenswithout bypassing TypeScript.
packages/actions/src/prepare-request-body.spec.ts#L3839-L3845: Removeas anyand assert both fields withtoMatchObject.packages/actions/src/prepare-request-body.spec.ts#L3867-L3869: Removeas anyand assertmax_tokenswithtoMatchObject.Proposed fix
- )) as any; - - expect(requestBody.thinking).toEqual({ - type: "enabled", - budget_tokens: 1024, - }); - expect(requestBody.max_tokens).toBe(2024); + )); + + expect(requestBody).toMatchObject({ + thinking: { type: "enabled", budget_tokens: 1024 }, + max_tokens: 2024, + }); ... - )) as any; - - expect(requestBody.max_tokens).toBe(8000); + )); + + expect(requestBody).toMatchObject({ max_tokens: 8000 });As per coding guidelines, “do not use
anyoras anyunless absolutely necessary.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/actions/src/prepare-request-body.spec.ts` around lines 3839 - 3845, Remove the `as any` assertions in `packages/actions/src/prepare-request-body.spec.ts` at lines 3839-3845 and 3867-3869. Update the first expectation to use `toMatchObject` for both `thinking` and `max_tokens`, and the second to use `toMatchObject` for `max_tokens`, preserving the asserted values without bypassing TypeScript.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/actions/src/prepare-request-body.spec.ts`:
- Around line 3839-3845: Remove the `as any` assertions in
`packages/actions/src/prepare-request-body.spec.ts` at lines 3839-3845 and
3867-3869. Update the first expectation to use `toMatchObject` for both
`thinking` and `max_tokens`, and the second to use `toMatchObject` for
`max_tokens`, preserving the asserted values without bypassing TypeScript.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 81fda677-7ef5-4ab2-87db-20d6b547d868
📒 Files selected for processing (2)
packages/actions/src/prepare-request-body.spec.tspackages/actions/src/prepare-request-body.ts
Problem
Production support chat was failing with:
The support-chat route (
apps/api/src/routes/public-chat-support.ts) calls the gateway withmodel: "auto"andmaxOutputTokens: 1024. When auto-routing lands on a reasoning-capable Anthropic model it setsreasoning_effort = "low", which maps to a thinking budget of exactly 1024 — Anthropic's minimum. The directanthropic/vertex-anthropicbranch ofprepareRequestBodythen forwarded the caller'smax_tokensverbatim, so the upstream request hadmax_tokens == thinking.budget_tokensand Anthropic 400s.The
aws-bedrockbranch already guarded against this; the direct Anthropic branch did not, despite its comment claiming to ("Set max_tokens, ensuring it's higher than thinking budget"). It only enforced the floor on the fallback value used when the caller omittedmax_tokens.Approach
Mirror the bedrock logic: when non-adaptive thinking is enabled, raise
max_tokenstothinking.budget_tokens + 1000if the caller-supplied value doesn't clear it.Raising
max_tokensis the only possible fix — the budget can't be lowered to fit, since 1024 is Anthropic's floor forbudget_tokens, so anymax_tokens <= 1024is unsatisfiable without bumping it.Adaptive models (Opus 4.7+) are untouched: they use
thinking: { type: "adaptive" }with nobudget_tokens, so the constraint doesn't apply.Verification
prepare-request-body.spec.tscover both the raise (1024 → 2024 atreasoning_effort: "low") and the no-op case (an already-sufficient 8000 stays 8000).packages/actionsunit suite: 581 passed.pnpm formatand fullpnpm buildclean.🤖 Generated with Claude Code
Summary by CodeRabbit