Conversation
… settings
Add two new provider-level settings for claude/claude-auth type providers:
- anthropicMaxTokensPreference: Override request max_tokens (1-64000)
- anthropicThinkingBudgetPreference: Override thinking.budget_tokens (1024-32000)
Key behaviors:
- When thinking budget is set, thinking.type is forced to 'enabled'
- Runtime clamping: budget_tokens clamped to max_tokens-1 if needed
- UI shows these fields only for claude/claude-auth providers
- Values stored as JSON strings ('inherit' or numeric like '32000')
Changes:
- Core override logic with audit trail (src/lib/anthropic/provider-overrides.ts)
- 36 unit tests covering all scenarios
- Database migration for new varchar columns
- Type system updates (Provider, ProviderDisplay, Create/Update interfaces)
- Form UI with validation and 'Max Out' quick-fill button
- i18n translations for 5 languages (en, zh-CN, zh-TW, ja, ru)
- Proxy pipeline integration after Codex overrides
📝 WalkthroughWalkthrough为 Anthropic（Claude）提供商增加参数级覆写与思考预算整流：数据库迁移、类型与验证、后端覆写与审计、代理整流与重试、前端表单与多语言文案、快照与测试等按迁移顺序的变更。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
Summary of ChangesHello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the control over Anthropic API requests by allowing administrators to define global override preferences for Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
github-actions
bot
added
enhancement
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces provider-level override settings for Anthropic's max_tokens and thinking.budget_tokens, which is a valuable feature for fine-tuning provider behavior. The implementation is comprehensive, covering database schema changes, backend logic with auditing, extensive unit tests, and a well-designed UI. Overall, the code quality is high.
I have identified a high-severity issue in the runtime clamping logic that could lead to invalid API requests and a medium-severity issue with the validation logic for updates. Addressing these points will enhance the feature's robustness and prevent potential runtime errors.
| if (thinkingBudget !== null) { | ||
| ensureCloned(); | ||
| const existingThinking = isPlainObject(output.thinking) ? output.thinking : {}; | ||
| let budgetTokens = thinkingBudget; | ||
| const currentMaxTokens = typeof output.max_tokens === "number" ? output.max_tokens : null; | ||
| if (currentMaxTokens !== null && budgetTokens >= currentMaxTokens) { | ||
| budgetTokens = currentMaxTokens - 1; | ||
| } | ||
| const nextThinking: Record<string, unknown> = { | ||
| ...existingThinking, | ||
| type: "enabled", | ||
| budget_tokens: budgetTokens, | ||
| }; | ||
| output.thinking = nextThinking; | ||
| } |
There was a problem hiding this comment.
The current clamping logic for thinking.budget_tokens can result in an invalid value being sent to the Anthropic API. According to the documentation and your own validation schema, thinking.budget_tokens must be at least 1024. However, if max_tokens is less than 1025 (e.g., 1000 from a client request) and a thinkingBudget override is active (e.g., 1024), the clamping logic budgetTokens = currentMaxTokens - 1 will set budget_tokens to 999. This is an invalid value and will cause the API request to fail.
To prevent sending invalid requests, I suggest modifying the logic to only apply the thinking override if the final clamped value is valid (i.e., >= 1024).
ensureCloned();
const existingThinking = isPlainObject(output.thinking) ? output.thinking : {};
let budgetTokens = thinkingBudget;
const currentMaxTokens = typeof output.max_tokens === "number" ? output.max_tokens : null;
if (currentMaxTokens !== null && budgetTokens >= currentMaxTokens) {
budgetTokens = currentMaxTokens - 1;
}
// Per Anthropic docs, budget_tokens must be at least 1024.
// Only apply the override if the final value is valid to avoid sending a request that is guaranteed to fail.
if (budgetTokens >= 1024) {
const nextThinking: Record<string, unknown> = {
...existingThinking,
type: "enabled",
budget_tokens: budgetTokens,
};
output.thinking = nextThinking;
}| .superRefine((data, ctx) => { | ||
| const maxTokens = data.anthropic_max_tokens_preference; | ||
| const budget = data.anthropic_thinking_budget_preference; | ||
| if (maxTokens && maxTokens !== "inherit" && budget && budget !== "inherit") { | ||
| const maxTokensNum = Number.parseInt(maxTokens, 10); | ||
| const budgetNum = Number.parseInt(budget, 10); | ||
| if (budgetNum >= maxTokensNum) { | ||
| ctx.addIssue({ | ||
| code: z.ZodIssueCode.custom, | ||
| message: "thinking.budget_tokens must be less than max_tokens", | ||
| path: ["anthropic_thinking_budget_preference"], | ||
| }); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
The superRefine validation on UpdateProviderSchema to ensure thinking.budget_tokens is less than max_tokens is not fully effective. Since UpdateProviderSchema fields are optional, this check only runs if both anthropic_max_tokens_preference and anthropic_thinking_budget_preference are included in the same update request. If a user updates only one of these fields, the validation will not trigger, potentially allowing an invalid state to be saved (e.g., setting a budget that is higher than the already-persisted max_tokens).
While the runtime clamping provides a safeguard, it's better to prevent such configurations at the form submission level. I recommend moving this cross-field validation logic into the editProvider server action, where you can fetch the existing provider data, merge it with the update payload, and then perform the validation on the complete entity before saving.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -0,0 +1,3 @@ | |||
| ALTER TABLE "notification_target_bindings" ALTER COLUMN "schedule_timezone" DROP DEFAULT;--> statement-breakpoint | |||
There was a problem hiding this comment.
Unrelated change to notification_target_bindings.schedule_timezone included in migration. Check if this was intentional or should be in a separate migration.
Prompt To Fix With AI
This is a comment left during a code review.
Path: drizzle/0060_bored_gertrude_yorkes.sql
Line: 1:1
Comment:
Unrelated change to `notification_target_bindings.schedule_timezone` included in migration. Check if this was intentional or should be in a separate migration.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx:
- Around line 549-550: The hardcoded visible strings in the SelectItem
components (e.g., the "Custom" label for value="custom" in routing-section.tsx
and the similar items at the other mentioned lines) must be replaced with i18n
keys and translations; update the JSX in the RoutingSection component (the
SelectItem children) to call the project's translation helper (e.g.,
useTranslations/t function) with a new key like
settings.providers.routing.custom instead of the literal "Custom", and add
corresponding entries for zh-CN, zh-TW, en, ja, ru in the locale
JSON/translation files; ensure you update both occurrences referenced (around
value="custom" and the other SelectItem at lines ~614-615) so all user-facing
options use the new i18n keys.
In `@src/lib/anthropic/provider-overrides.ts`:
- Around line 61-75: The clamp logic for budgetTokens can push it below
Anthropic's minimum (1024); after computing budgetTokens (in the block that uses
normalizeNumericPreference/provider.anthropicThinkingBudgetPreference and
currentMaxTokens), add a check: if budgetTokens < 1024 then do not set
output.thinking (i.e., skip the ensureCloned()/assignment to
nextThinking/output.thinking) so the thinking override is not applied; otherwise
proceed to build nextThinking and assign it as currently done.
🧹 Nitpick comments (4)
src/types/provider.ts (1)
33-37: 明确 null 与 "inherit" 的语义差异类型允许 null 与 "inherit"，但未说明 null 的含义；建议补充注释或统一语义，避免 API/DB/UI 行为含糊。
Based on learnings In TypeScript interfaces, explicitly document and enforce distinct meanings for null and undefined. Example: for numeric limits like limitTotalUsd, use 'number | null | undefined' when null signifies explicitly unlimited (e.g., matches DB schema or special UI logic) and undefined signifies 'inherit default'. This pattern should be consistently reflected in type definitions across related fields to preserve semantic clarity between database constraints and UI behavior.
Also applies to: 137-139
src/lib/validation/schemas.ts (3)
28-56: 建议将魔法数字提取为常量Anthropic 相关的限制值（1, 64000, 1024, 32000）直接硬编码在 schema 中。参考
PROVIDER_LIMITS的模式，建议将这些限制提取到@/lib/constants/provider.constants.ts中，便于维护和复用。♻️ 建议的重构方案
在
provider.constants.ts中添加：export const ANTHROPIC_LIMITS = { MAX_TOKENS: { MIN: 1, MAX: 64000 }, THINKING_BUDGET: { MIN: 1024, MAX: 32000 }, } as const;然后在 schema 中引用这些常量，保持与其他限制配置的一致性。
34-41: 错误消息语言不一致新增的 Anthropic schema 使用英文错误消息，而文件中其他验证消息（如 Line 62 的 "用户名不能为空"）使用中文。建议保持一致性，统一使用中文或在 UI 层通过 i18n 映射。
764-779: 补充 UI 提示，说明两个字段的约束关系和可能的自动调整行为当前验证逻辑确实存在边界情况：如果仅通过 API 部分更新其中一个字段，schema 无法验证新值与已存在值的组合。不过实际上，UI 表单始终同时提交两个字段（lines 210-211），所以 schema 的 superRefine 验证能正常工作。
建议在 UI 层补充说明：
- 在 thinking budget 字段的 Tooltip（目前在 line 581-582 处为空）中添加提示文字，说明
budget < max_tokens的约束- 说明如果提交的值不满足约束会发生什么（是否自动调整）
这样用户在直接调用 API 进行部分更新时，也能了解到这个隐含约束。
...app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx
Outdated
Show resolved
Hide resolved
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| let budgetTokens = thinkingBudget; | ||
| const currentMaxTokens = typeof output.max_tokens === "number" ? output.max_tokens : null; | ||
| if (currentMaxTokens !== null && budgetTokens >= currentMaxTokens) { | ||
| budgetTokens = currentMaxTokens - 1; |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] thinking.budget_tokens can be clamped below 1024
File: src/lib/anthropic/provider-overrides.ts:68
Why this is a problem: applyAnthropicProviderOverrides clamps budget_tokens to max_tokens - 1. If the client request has max_tokens <= 1024, this produces budget_tokens < 1024 while also forcing thinking.type = "enabled", which is very likely to create an invalid Anthropic request (and break requests when this override is enabled). This also violates the intended minimum enforced in validation (thinking.budget_tokens >= 1024).
Concrete example: max_tokens = 100, provider override thinkingBudget = 10240 => budget_tokens becomes 99.
Suggested fix:
const MIN_THINKING_BUDGET_TOKENS = 1024;
// ...
const currentMaxTokens = typeof output.max_tokens === "number" ? output.max_tokens : null;
if (currentMaxTokens !== null && budgetTokens >= currentMaxTokens) {
budgetTokens = currentMaxTokens - 1;
if (budgetTokens < MIN_THINKING_BUDGET_TOKENS) {
// max_tokens too small to safely enable thinking; skip the thinking override
return output;
}
}Add a unit test covering the max_tokens <= 1024 clamping case to lock this behavior.
| <SelectItem value="inherit"> | ||
| {t("sections.routing.anthropicOverrides.maxTokens.options.inherit")} | ||
| </SelectItem> | ||
| <SelectItem value="custom">Custom</SelectItem> |
There was a problem hiding this comment.
[MEDIUM] [STANDARD-VIOLATION] Hardcoded UI label "Custom" bypasses i18n
File: src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx:549 (also at :614)
Why this is a problem: Project rule: "i18n Required - All user-facing strings must use i18n (5 languages supported). Never hardcode display text". The literal Custom is rendered in the provider form, so it will never be translated.
Suggested fix:
<SelectItem value="custom">
{t("sections.routing.anthropicOverrides.maxTokens.options.custom")}
</SelectItem>Apply the same change for thinkingBudget, and add the corresponding options.custom keys to messages/*/settings/providers/form/sections.json.
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: XL
- Lines changed: 4,585 (4,404 additions + 181 deletions)
- Files changed: 21
Note on size: While this is an XL PR, the changes are well-organized and follow a clear pattern. The bulk comes from comprehensive test coverage (36 tests) and i18n translations (5 languages × multiple keys). The core logic is concise and focused.
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean (proper logging in all catch blocks)
- Type safety - Clean (comprehensive Zod schemas with cross-field validation)
- Documentation accuracy - Good (clear comments explaining behavior)
- Test coverage - Excellent (36 unit tests, 1786 total tests passing)
- Code clarity - Good (follows existing patterns, clear separation of concerns)
Key Strengths
-
Comprehensive Testing: 36 unit tests covering all scenarios including edge cases (clamping logic, provider type filtering, audit trail)
-
Layered Validation: Three-tier validation approach prevents invalid configurations:
- UI validation (HTML min/max)
- Schema validation (prevents misconfigured providers)
- Runtime clamping (handles dynamic edge cases)
-
Proper Error Handling: All async operations have proper .catch() blocks with contextual logging
-
Type Safety: Strong Zod schemas with cross-field validation ensuring
budget_tokens < max_tokens -
Immutability: Core logic properly clones request object before modification
-
i18n Compliance: All user-facing strings properly internationalized for 5 languages
Migration Note
The migration 0060_bored_gertrude_yorkes.sql includes ALTER TABLE "notification_target_bindings" ALTER COLUMN "schedule_timezone" DROP DEFAULT which fixes a schema drift issue (previous migration snapshot had a default value that wasn't in the schema definition). This is a legitimate schema sync fix generated by Drizzle.
Automated review by Claude AI
There was a problem hiding this comment.
Code Review Summary
This PR introduces provider-level Anthropic parameter overrides and touches DB migrations, validation, proxy forwarding, admin UI, and i18n. Given the XL size (4585 lines across 21 files), the change-set is difficult to validate as a single unit; consider splitting to reduce merge risk and ease regression review.
PR Size: XL
- Lines changed: 4585
- Files changed: 21
- Suggested split (XL):
- PR 1: Core override logic + proxy integration + unit tests (
src/lib/anthropic/*,src/app/v1/_lib/proxy/forwarder.ts,tests/unit/proxy/*) - PR 2: Provider schema/repo/types + migrations (
src/lib/validation/schemas.ts,src/types/provider.ts,src/repository/*,src/drizzle/schema.ts,drizzle/*) - PR 3: Admin UI + i18n (
src/app/[locale]/settings/providers/**,messages/*)
- PR 1: Core override logic + proxy integration + unit tests (
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 1 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 1 | 0 |
| Tests | 0 | 0 | 1 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
(none)
High Priority Issues (Should Fix)
- [HIGH] [LOGIC-BUG]
thinking.budget_tokenscan be clamped below 1024 when clientmax_tokens <= 1024, which likely produces invalid Anthropic requests (seesrc/lib/anthropic/provider-overrides.ts:68). - [TEST-EDGE-CASE] Add coverage for the
max_tokens <= 1024clamping/skip behavior intests/unit/proxy/anthropic-provider-overrides.test.ts. - [MEDIUM] [STANDARD-VIOLATION] Hardcoded UI label
Custombypasses i18n (seesrc/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx:549).
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
- Labeled PR
#689assize/XL. - Posted inline review comments on:
src/lib/anthropic/provider-overrides.ts:68— potential invalidthinking.budget_tokenswhen clamped below 1024.src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx:549— hardcoded UI labelCustombypasses i18n (also occurs at:614).
- Submitted the required “Code Review Summary” review comment on PR
#689.
…rrors Add reactive rectifier that automatically fixes Anthropic API errors when thinking.budget_tokens is below the minimum threshold of 1024. Trigger: Error message 'thinking.enabled.budget_tokens: Input should be greater than or equal to 1024' Rectification: - Set thinking.type = 'enabled' - Set thinking.budget_tokens = 32000 (maximum) - Set max_tokens = 64000 if missing or < 32001 Features: - System setting toggle: enableThinkingBudgetRectifier (default: true) - Follows same pattern as thinking-signature-rectifier - Retries once on same provider after rectification - Full audit trail via ThinkingBudgetRectifierSpecialSetting Changes: - Core rectifier logic with detect/rectify functions (22 unit tests) - Forwarder integration in retry loop (section 2.6) - System settings full chain (schema/types/cache/repo/actions/validation) - UI toggle in system settings form - i18n translations for 5 languages (en, zh-CN, zh-TW, ja, ru) - Database migration for new column
- Fix budget_tokens clamping below API minimum (1024) - Skip thinking override when clamped value would be invalid - Replace hardcoded "Custom" with i18n keys (5 languages) - Add unit tests for 1024 boundary behavior
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
…errides feat(provider): add Anthropic max_tokens and thinking budget override settings
1 participant
Summary
Add two new provider-level settings for
claudeandclaude-authtype providers that allow overriding Anthropic API request parameters:anthropicMaxTokensPreference: Overridemax_tokensfield (range: 1-64,000 or "inherit")anthropicThinkingBudgetPreference: Overridethinking.budget_tokensfield (range: 1,024-32,000 or "inherit")Key Behaviors
thinking.typeis automatically forced to"enabled"budget_tokens >= max_tokens, it's clamped tomax_tokens - 1"inherit"or numeric like"32000")Changes
Core Logic
src/lib/anthropic/provider-overrides.ts- Override logic with audit trail supportTests
tests/unit/proxy/anthropic-provider-overrides.test.ts- 36 unit tests covering all scenariosDatabase
drizzle/0060_bored_gertrude_yorkes.sql- Migration adding two varchar(20) columnsType System & Repository
src/types/provider.ts- Added types to Provider, ProviderDisplay, Create/Update interfacessrc/drizzle/schema.ts- Added database columnssrc/repository/provider.ts+transformers.ts- Field mappingsUI & Forms
routing-section.tsx- New UI section with toggle, input, validationprovider-form-context.tsx+provider-form-types.ts- Form state managementprovider-form/index.tsx- Form submission handlingi18n
Proxy Integration
src/app/v1/_lib/proxy/forwarder.ts- Integrated into proxy pipeline after Codex overridesVerification
Greptile Overview
Greptile Summary
Adds two provider-level override settings for Anthropic claude/claude-auth providers that allow administrators to force-override
max_tokens(1-64,000) andthinking.budget_tokens(1,024-32,000) API parameters.Key Changes
provider-overrides.tsimplements override application with runtime clamping (budget < max_tokens), minimum budget validation (>= 1024), and comprehensive audit trail supportthinking-budget-rectifier.tsthat detects budget_tokens < 1024 API errors and automatically sets maximum values (32000/64000) with one retry attemptenable_thinking_budget_rectifierboolean to system_settingsAnthropicMaxTokensPreferenceandAnthropicThinkingBudgetPreferencetypes throughout Provider interfaces (Provider, ProviderDisplay, Create/Update)Test Coverage
Notable Patterns
thinking.typeautomatically forced to "enabled"Confidence Score: 5/5
Important Files Changed
Sequence Diagram
sequenceDiagram participant Client participant ProxyForwarder participant ProviderOverrides participant Validation participant ThinkingRectifier participant AnthropicAPI Client->>ProxyForwarder: POST /v1/messages Note over ProxyForwarder: Load provider config ProxyForwarder->>Validation: Validate request Validation-->>ProxyForwarder: Valid alt Provider is claude/claude-auth ProxyForwarder->>ProviderOverrides: applyAnthropicProviderOverridesWithAudit() Note over ProviderOverrides: Check preference settings alt Preferences set (not "inherit") ProviderOverrides->>ProviderOverrides: Override parameters Note over ProviderOverrides: Clamp budget to limit minus one Note over ProviderOverrides: Skip if budget below minimum ProviderOverrides->>ProviderOverrides: Build audit trail end ProviderOverrides-->>ProxyForwarder: Modified request with audit ProxyForwarder->>ProxyForwarder: Store audit in special settings end ProxyForwarder->>AnthropicAPI: Forward request alt Error: budget validation failure AnthropicAPI-->>ProxyForwarder: 400 error (budget constraint) ProxyForwarder->>ThinkingRectifier: detectThinkingBudgetRectifierTrigger() ThinkingRectifier-->>ProxyForwarder: Trigger detected alt First retry ProxyForwarder->>ThinkingRectifier: rectifyThinkingBudget() Note over ThinkingRectifier: Set budget to maximum Note over ThinkingRectifier: Increase limit if needed ThinkingRectifier-->>ProxyForwarder: Rectified request ProxyForwarder->>ProxyForwarder: Store rectifier audit ProxyForwarder->>AnthropicAPI: Retry with rectified request end end AnthropicAPI-->>ProxyForwarder: Success response ProxyForwarder-->>Client: Return response