feat: add codex service tier provider overrides#870
Conversation
📝 WalkthroughWalkthrough该PR为提供者模型添加了Codex服务等级偏好字段(codexServiceTierPreference),包括数据库架构、类型定义、UI表单、批量补丁处理和仪表板日志显示等多个层面的传播。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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, 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 introduces a significant enhancement by allowing users to explicitly control the Highlights
Changelog
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
|
🧪 测试结果
总体结果: ✅ 所有测试通过 |
| @@ -452,6 +459,7 @@ export interface ProviderDisplay { | |||
| codexReasoningSummaryPreference: CodexReasoningSummaryPreference | null; | |||
| codexTextVerbosityPreference: CodexTextVerbosityPreference | null; | |||
| codexParallelToolCallsPreference: CodexParallelToolCallsPreference | null; | |||
| codexServiceTierPreference?: CodexServiceTierPreference | null; | |||
There was a problem hiding this comment.
Inconsistent optional modifier on codexServiceTierPreference
The codexServiceTierPreference field is marked optional (?) in both the Provider (line 375) and ProviderDisplay (line 462) interfaces, while all sibling codex preference fields are required-but-nullable:
codexReasoningEffortPreference: CodexReasoningEffortPreference | null;codexReasoningSummaryPreference: CodexReasoningSummaryPreference | null;codexTextVerbosityPreference: CodexTextVerbosityPreference | null;codexParallelToolCallsPreference: CodexParallelToolCallsPreference | null;
Since the transformer (src/repository/_shared/transformers.ts line 131) always provides this field via ?? null, the optional marker is misleading and weakens TypeScript's type-safety guarantee. For consistency and clarity, both occurrences should be changed to required-but-nullable form.
| codexServiceTierPreference: CodexServiceTierPreference | null; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/types/provider.ts
Line: 375-462
Comment:
**Inconsistent optional modifier on `codexServiceTierPreference`**
The `codexServiceTierPreference` field is marked optional (`?`) in both the `Provider` (line 375) and `ProviderDisplay` (line 462) interfaces, while all sibling codex preference fields are required-but-nullable:
- `codexReasoningEffortPreference: CodexReasoningEffortPreference | null;`
- `codexReasoningSummaryPreference: CodexReasoningSummaryPreference | null;`
- `codexTextVerbosityPreference: CodexTextVerbosityPreference | null;`
- `codexParallelToolCallsPreference: CodexParallelToolCallsPreference | null;`
Since the transformer (`src/repository/_shared/transformers.ts` line 131) always provides this field via `?? null`, the optional marker is misleading and weakens TypeScript's type-safety guarantee. For consistency and clarity, both occurrences should be changed to required-but-nullable form.
```suggestion
codexServiceTierPreference: CodexServiceTierPreference | null;
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive support for service_tier overrides for Codex providers, integrating changes across the database schema, repository, actions, UI forms, and logging components, including Zod validation schemas, server actions for CRUD and batch operations, and UI enhancements. The implementation includes appropriate authorization checks for administrative actions, and no security vulnerabilities were identified in the modified code. My review includes a couple of suggestions for the dashboard log components to improve code reuse and readability by memoizing a computed value and considering a shared component for duplicated UI elements.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsx (1)
629-662:⚠️ Potential issue | 🟠 Major
fast徽标不应该依赖costUsd。这里把 badge/tooltip 放在
log.costUsd != null分支里了,所以优先 tier 已命中但未计费/计费失败的日志只会显示-,不会显示fast。PR 目标是“effectiveservice_tier为priority时显示 badge”,建议把优先 tier 的展示条件从费用文案里拆出来;usage-logs-table.tsx的同段逻辑也需要同样调整。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/logs/_components/virtualized-logs-table.tsx around lines 629 - 662, The fast (priority) badge and its tooltip are incorrectly nested under the cost branch (log.costUsd != null) so priority logs without cost don't show the badge; update the rendering in virtualized-logs-table.tsx to render hasPriorityServiceTierSpecialSetting(log.specialSettings) independently of the log.costUsd check (move the Badge and its TooltipContent out of the costUsd conditional), and apply the same change in usage-logs-table.tsx so the priority badge and its tooltip appear whenever hasPriorityServiceTierSpecialSetting(...) is true regardless of costUsd.
🧹 Nitpick comments (2)
tests/unit/lib/provider-patch-contract.test.ts (1)
7-35: 把clear -> inherit这条路径也补进测试。现在两个用例都只覆盖了
{ set: "priority" }。codex_service_tier_preference在批量 patch 里还有 clear 语义,如果后续把 clear 映射成null/undefined,这组测试拦不住回归。建议再补一条clear断言,把归一化和 apply updates 两步一起锁住。As per coding guidelines "All new features must have unit test coverage of at least 80%".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/lib/provider-patch-contract.test.ts` around lines 7 - 35, Add test cases to cover the "clear -> inherit" path for codex_service_tier_preference in the existing suite: call normalizeProviderBatchPatchDraft with codex_service_tier_preference: { clear: true } (or the project’s clear representation) and assert result.ok and that normalized.data.codex_service_tier_preference equals { mode: "inherit", value: undefined } (or the expected inherit shape), then pass that normalized data into buildProviderBatchApplyUpdates and assert updates.ok and updates.data.codex_service_tier_preference equals the expected inherited value (e.g., "inherit" or null per implementation); place these assertions alongside the existing tests so both normalization (normalizeProviderBatchPatchDraft) and application (buildProviderBatchApplyUpdates) behaviors for clear->inherit are locked in.tests/unit/dashboard-logs-virtualized-special-settings-ui.test.tsx (1)
180-193: 再补一条costUsd: null的回归用例。当前只覆盖了“有费用时显示
fast”,但需求关注的是 effectiveservice_tier。建议再补一条costUsd: null仍显示fast的断言,把这类回归锁住。As per coding guidelines "All new features must have unit test coverage of at least 80%".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/dashboard-logs-virtualized-special-settings-ui.test.tsx` around lines 180 - 193, Add a new unit test alongside the existing "fast badge" test in tests/unit/dashboard-logs-virtualized-special-settings-ui.test.tsx that verifies a record with costUsd: null still renders the "fast" badge: create a test (e.g., "renders fast badge when costUsd is null") that renders <VirtualizedLogsTable filters={{}} autoRefreshEnabled={false} /> (using the same helpers renderWithIntl, flushMicrotasks, waitForText), ensure the mock log data returned includes a service_tier indicating "fast" while costUsd is explicitly null, await the "Loaded 1 records" text, and assert container.textContent contains "fast" before unmounting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@messages/ru/dashboard.json`:
- Around line 286-287: The "fastPriority" tooltip text is still in English;
update the Russian locale entry for the "fastPriority" key in
messages/ru/dashboard.json to a Russian translation while keeping fixed terms
like "fast" and "priority" (if preferred) in Latin, and ensure the corresponding
tooltip at the other occurrence (keys around the second occurrence referenced,
lines ~379-380) is translated the same way into Russian to avoid mixed-language
UI strings.
In `@messages/ru/settings/providers/form/sections.json`:
- Around line 193-203: The "serviceTier" block contains mixed English; translate
the "label" and "help" strings fully into Russian while preserving any API enum
tokens and internal keys (keep the property name serviceTier and option keys
auto, default, flex, inherit, priority unchanged); update the "help" to remove
English fragments and explain behavior in Russian (mention что priority —
быстрый режим, inherit — не переопределять, unsupported tiers могут вызвать
upstream-ошибку) and translate each option display value to Russian (e.g., "auto
(по умолчанию проекта)", "default (стандартный)", "flex (best-effort)" →
translate best-effort to Russian, "inherit" → "Не переопределять (следовать
клиенту)", "priority" → "priority (быстрый)"), ensuring all user-facing text in
serviceTier.label, serviceTier.help, and serviceTier.options.* is Russian.
In `@messages/zh-TW/dashboard.json`:
- Around line 286-287: Translate the value for the "fastPriority" key into full
Traditional Chinese instead of leaving the main phrase in English; keep the
"fast" term if needed but rewrite the parenthetical explanation into complete
Traditional Chinese (e.g., change "Priority service tier(fast 模式)" to a full
zh-TW string like "優先服務等級(fast 模式)" or similar natural phrasing). Update every
occurrence of the "fastPriority" key (including the other occurrence noted in
the file) to use the same Traditional Chinese translation for consistency.
In `@src/lib/codex/provider-overrides.ts`:
- Around line 126-136: The current hit calculation wrongly treats a client-sent
service_tier:"priority" as a provider override because it checks
beforeServiceTier === "priority"; update the logic so hit only reflects actual
provider-provided overrides (i.e., remove the beforeServiceTier === "priority"
condition and rely on serviceTier !== null for provider-side service tier
overrides alongside the other provider-derived prefs: parallelToolCalls,
reasoningEffort, reasoningSummary, textVerbosity). If you still need to record
an “effective priority” coming from the request, add a separate
flag/special-setting (e.g., hasEffectivePrioritySpecialSetting) that is set when
beforeServiceTier === "priority" && serviceTier === null instead of mixing it
into the provider override hit.
In `@src/lib/provider-patch-contract.ts`:
- Line 103: The clear path for codex_service_tier_preference is missing: update
applyPatchField to handle the clear branch for the symbol
codex_service_tier_preference (and any other listed preference symbols) so that
when patch.normalized is { clear: true } it falls back to the same "inherit" (or
existing empty-value semantic) used by the other preference fields; mirror the
clear-case logic used for the other preference keys in
buildProviderBatchApplyUpdates/applyPatchField so bulk-editing a clear override
returns the inherited value instead of "clear mode is not supported for this
field".
In `@src/repository/provider.ts`:
- Line 1035: The batch update flow is dropping codexServiceTierPreference
because the action layer (BatchUpdateProvidersParams["updates"] /
repositoryUpdates in src/actions/providers.ts) does not map
codexServiceTierPreference to the DB payload key codex_service_tier_preference;
update the mapping where repositoryUpdates and BatchProviderUpdates are
constructed (the code that builds updates passed into updateProvidersBatch() and
BatchProviderUpdates) to include codexServiceTierPreference ->
codex_service_tier_preference, and mirror this fix for the other occurrence
noted (the similar mapping block around the other BatchProviderUpdates usage) so
the UI/patch layer’s batch service-tier changes are forwarded into
updateProvidersBatch().
In `@src/types/provider.ts`:
- Line 375: Change the declaration of codexServiceTierPreference from optional
to required-but-nullable in both Provider and ProviderDisplay (i.e., remove the
"?" so it reads codexServiceTierPreference: CodexServiceTierPreference | null)
to match the normalization in toProvider() and the other Codex preference
fields; update the two places where it’s declared (Provider and ProviderDisplay)
so the type expresses "present or null" rather than optional, ensuring
consistency with toProvider() which uses "?? null".
---
Outside diff comments:
In `@src/app/`[locale]/dashboard/logs/_components/virtualized-logs-table.tsx:
- Around line 629-662: The fast (priority) badge and its tooltip are incorrectly
nested under the cost branch (log.costUsd != null) so priority logs without cost
don't show the badge; update the rendering in virtualized-logs-table.tsx to
render hasPriorityServiceTierSpecialSetting(log.specialSettings) independently
of the log.costUsd check (move the Badge and its TooltipContent out of the
costUsd conditional), and apply the same change in usage-logs-table.tsx so the
priority badge and its tooltip appear whenever
hasPriorityServiceTierSpecialSetting(...) is true regardless of costUsd.
---
Nitpick comments:
In `@tests/unit/dashboard-logs-virtualized-special-settings-ui.test.tsx`:
- Around line 180-193: Add a new unit test alongside the existing "fast badge"
test in tests/unit/dashboard-logs-virtualized-special-settings-ui.test.tsx that
verifies a record with costUsd: null still renders the "fast" badge: create a
test (e.g., "renders fast badge when costUsd is null") that renders
<VirtualizedLogsTable filters={{}} autoRefreshEnabled={false} /> (using the same
helpers renderWithIntl, flushMicrotasks, waitForText), ensure the mock log data
returned includes a service_tier indicating "fast" while costUsd is explicitly
null, await the "Loaded 1 records" text, and assert container.textContent
contains "fast" before unmounting.
In `@tests/unit/lib/provider-patch-contract.test.ts`:
- Around line 7-35: Add test cases to cover the "clear -> inherit" path for
codex_service_tier_preference in the existing suite: call
normalizeProviderBatchPatchDraft with codex_service_tier_preference: { clear:
true } (or the project’s clear representation) and assert result.ok and that
normalized.data.codex_service_tier_preference equals { mode: "inherit", value:
undefined } (or the expected inherit shape), then pass that normalized data into
buildProviderBatchApplyUpdates and assert updates.ok and
updates.data.codex_service_tier_preference equals the expected inherited value
(e.g., "inherit" or null per implementation); place these assertions alongside
the existing tests so both normalization (normalizeProviderBatchPatchDraft) and
application (buildProviderBatchApplyUpdates) behaviors for clear->inherit are
locked in.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 855c9e44-59d7-4388-aa3f-dd222d6c81b1
📒 Files selected for processing (35)
drizzle/0078_remarkable_lionheart.sqldrizzle/meta/0078_snapshot.jsondrizzle/meta/_journal.jsonmessages/en/dashboard.jsonmessages/en/settings/providers/form/sections.jsonmessages/ja/dashboard.jsonmessages/ja/settings/providers/form/sections.jsonmessages/ru/dashboard.jsonmessages/ru/settings/providers/form/sections.jsonmessages/zh-CN/dashboard.jsonmessages/zh-CN/settings/providers/form/sections.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/settings/providers/form/sections.jsonsrc/actions/providers.tssrc/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsxsrc/app/[locale]/settings/providers/_components/batch-edit/build-patch-draft.tssrc/app/[locale]/settings/providers/_components/forms/provider-form/index.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-context.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-types.tssrc/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsxsrc/drizzle/schema.tssrc/lib/codex/provider-overrides.tssrc/lib/provider-patch-contract.tssrc/lib/utils/special-settings.tssrc/lib/validation/schemas.tssrc/repository/_shared/transformers.tssrc/repository/provider.tssrc/types/provider.tstests/integration/usage-ledger.test.tstests/unit/dashboard-logs-virtualized-special-settings-ui.test.tsxtests/unit/lib/provider-patch-contract.test.tstests/unit/proxy/codex-provider-overrides.test.tstests/unit/settings/providers/build-patch-draft.test.ts
| "fast": "fast", | ||
| "fastPriority": "Priority service tier (fast mode)", |
There was a problem hiding this comment.
请把 fastPriority 的说明文案翻译成俄语。
这里新增的是俄语语言包,但 tooltip/说明仍是完整英文,最终会直接出现在日志详情里。建议保留 fast / priority 这类固定术语,其余说明改成俄语,避免界面语言混杂。
Also applies to: 379-380
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/ru/dashboard.json` around lines 286 - 287, The "fastPriority"
tooltip text is still in English; update the Russian locale entry for the
"fastPriority" key in messages/ru/dashboard.json to a Russian translation while
keeping fixed terms like "fast" and "priority" (if preferred) in Latin, and
ensure the corresponding tooltip at the other occurrence (keys around the second
occurrence referenced, lines ~379-380) is translated the same way into Russian
to avoid mixed-language UI strings.
| "serviceTier": { | ||
| "help": "Управляет OpenAI service_tier для запросов Codex (Responses API). Значение priority соответствует fast mode. Режим \"следовать клиенту\" не переписывает service_tier, остальные значения принудительно его переопределяют. Неподдерживаемый tier может вызвать ошибку upstream.", | ||
| "label": "Переопределение Service Tier", | ||
| "options": { | ||
| "auto": "auto (по умолчанию проекта)", | ||
| "default": "default (стандартный)", | ||
| "flex": "flex (best-effort)", | ||
| "inherit": "Не переопределять (следовать клиенту)", | ||
| "priority": "priority (fast)" | ||
| } | ||
| }, |
There was a problem hiding this comment.
请把这组俄语表单文案完整本地化。
这里新增的是直接面向终端用户的设置项,但 label 仍然显示 Service Tier,帮助文案里也混入了较多未翻译的英文说明,界面会出现明显的语言混杂。建议保留必须与 API 对齐的固定枚举值(如 auto、priority、service_tier),其余说明改成完整俄语。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/ru/settings/providers/form/sections.json` around lines 193 - 203,
The "serviceTier" block contains mixed English; translate the "label" and "help"
strings fully into Russian while preserving any API enum tokens and internal
keys (keep the property name serviceTier and option keys auto, default, flex,
inherit, priority unchanged); update the "help" to remove English fragments and
explain behavior in Russian (mention что priority — быстрый режим, inherit — не
переопределять, unsupported tiers могут вызвать upstream-ошибку) and translate
each option display value to Russian (e.g., "auto (по умолчанию проекта)",
"default (стандартный)", "flex (best-effort)" → translate best-effort to
Russian, "inherit" → "Не переопределять (следовать клиенту)", "priority" →
"priority (быстрый)"), ensuring all user-facing text in serviceTier.label,
serviceTier.help, and serviceTier.options.* is Russian.
| "fast": "fast", | ||
| "fastPriority": "Priority service tier(fast 模式)", |
There was a problem hiding this comment.
请把 fastPriority 的说明文案翻译成繁体中文。
当前 fastPriority 仍然是英文主文案,只在括号里加了中文说明,最终显示出来会很突兀。建议保留 fast 这类固定术语,其余解释改为完整繁中。
Also applies to: 379-380
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/zh-TW/dashboard.json` around lines 286 - 287, Translate the value
for the "fastPriority" key into full Traditional Chinese instead of leaving the
main phrase in English; keep the "fast" term if needed but rewrite the
parenthetical explanation into complete Traditional Chinese (e.g., change
"Priority service tier(fast 模式)" to a full zh-TW string like "優先服務等級(fast 模式)"
or similar natural phrasing). Update every occurrence of the "fastPriority" key
(including the other occurrence noted in the file) to use the same Traditional
Chinese translation for consistency.
| const serviceTier = normalizeStringPreference(provider.codexServiceTierPreference); | ||
|
|
||
| const beforeServiceTier = toAuditValue(request.service_tier); | ||
|
|
||
| const hit = | ||
| parallelToolCalls !== null || | ||
| reasoningEffort !== null || | ||
| reasoningSummary !== null || | ||
| textVerbosity !== null; | ||
| textVerbosity !== null || | ||
| serviceTier !== null || | ||
| beforeServiceTier === "priority"; |
There was a problem hiding this comment.
不要把客户端自带的 priority 记成 provider override。
beforeServiceTier === "priority" 会在 provider 仍是 "inherit" 时也生成 provider_parameter_override 审计;而 hasPriorityServiceTierSpecialSetting() 只检查 after === "priority",不看 changed。结果就是客户端自己传的 service_tier: "priority" 也会被后续统计/展示当成 provider override 命中。要么把 hit 限定为真正发生了 provider override,要么为 “effective priority” 单独建一种 special-setting。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/codex/provider-overrides.ts` around lines 126 - 136, The current hit
calculation wrongly treats a client-sent service_tier:"priority" as a provider
override because it checks beforeServiceTier === "priority"; update the logic so
hit only reflects actual provider-provided overrides (i.e., remove the
beforeServiceTier === "priority" condition and rely on serviceTier !== null for
provider-side service tier overrides alongside the other provider-derived prefs:
parallelToolCalls, reasoningEffort, reasoningSummary, textVerbosity). If you
still need to record an “effective priority” coming from the request, add a
separate flag/special-setting (e.g., hasEffectivePrioritySpecialSetting) that is
set when beforeServiceTier === "priority" && serviceTier === null instead of
mixing it into the provider override hit.
| codex_reasoning_summary_preference: true, | ||
| codex_text_verbosity_preference: true, | ||
| codex_parallel_tool_calls_preference: true, | ||
| codex_service_tier_preference: true, |
There was a problem hiding this comment.
clear 路径还没闭环。
codex_service_tier_preference 被声明成 clearable,归一化也会接受 { clear: true },但 applyPatchField() 的 clear 分支没有对应 case,所以 buildProviderBatchApplyUpdates() 最终会返回 clear mode is not supported for this field。批量编辑里清空这个 override 会直接失败。请补一个 clear 分支,并和同文件其他 preference 字段保持一致地回落到 "inherit"(或按你们既有的空值语义处理)。
建议补齐 clear 分支
case "codex_parallel_tool_calls_preference":
updates.codex_parallel_tool_calls_preference = "inherit";
return { ok: true, data: undefined };
+ case "codex_service_tier_preference":
+ updates.codex_service_tier_preference = "inherit";
+ return { ok: true, data: undefined };
case "anthropic_max_tokens_preference":Also applies to: 492-496, 627-627, 747-750, 956-956, 1022-1022
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/provider-patch-contract.ts` at line 103, The clear path for
codex_service_tier_preference is missing: update applyPatchField to handle the
clear branch for the symbol codex_service_tier_preference (and any other listed
preference symbols) so that when patch.normalized is { clear: true } it falls
back to the same "inherit" (or existing empty-value semantic) used by the other
preference fields; mirror the clear-case logic used for the other preference
keys in buildProviderBatchApplyUpdates/applyPatchField so bulk-editing a clear
override returns the inherited value instead of "clear mode is not supported for
this field".
| codexReasoningSummaryPreference?: string | null; | ||
| codexTextVerbosityPreference?: string | null; | ||
| codexParallelToolCallsPreference?: string | null; | ||
| codexServiceTierPreference?: string | null; |
There was a problem hiding this comment.
批量更新入口还会静默丢掉这个字段。
这里虽然给 BatchProviderUpdates 和 updateProvidersBatch() 加了 codexServiceTierPreference,但 src/actions/providers.ts:2157-2231 里的 BatchUpdateProvidersParams["updates"] / repositoryUpdates 仍然没有映射 codex_service_tier_preference。结果是 UI/patch 层提交的批量 service tier 修改到不了这段仓储代码,PR 目标里的 batch patch flow 仍然是不通的。
建议同时补齐 action 层映射
export interface BatchUpdateProvidersParams {
providerIds: number[];
updates: {
...
+ codex_service_tier_preference?: CodexServiceTierPreference | null;
};
}
...
+if (updates.codex_service_tier_preference !== undefined) {
+ repositoryUpdates.codexServiceTierPreference =
+ updates.codex_service_tier_preference;
+}Also applies to: 1142-1144
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/repository/provider.ts` at line 1035, The batch update flow is dropping
codexServiceTierPreference because the action layer
(BatchUpdateProvidersParams["updates"] / repositoryUpdates in
src/actions/providers.ts) does not map codexServiceTierPreference to the DB
payload key codex_service_tier_preference; update the mapping where
repositoryUpdates and BatchProviderUpdates are constructed (the code that builds
updates passed into updateProvidersBatch() and BatchProviderUpdates) to include
codexServiceTierPreference -> codex_service_tier_preference, and mirror this fix
for the other occurrence noted (the similar mapping block around the other
BatchProviderUpdates usage) so the UI/patch layer’s batch service-tier changes
are forwarded into updateProvidersBatch().
| codexReasoningSummaryPreference: CodexReasoningSummaryPreference | null; | ||
| codexTextVerbosityPreference: CodexTextVerbosityPreference | null; | ||
| codexParallelToolCallsPreference: CodexParallelToolCallsPreference | null; | ||
| codexServiceTierPreference?: CodexServiceTierPreference | null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -C3 'codexServiceTierPreference|codexReasoningEffort|codexReasoningSummary|codexTextVerbosity|codexParallelToolCalls' \
src/types/provider.ts src/repository/_shared/transformers.ts | head -100Repository: ding113/claude-code-hub
Length of output: 3333
🏁 Script executed:
sed -n '370,380p' src/types/provider.ts
sed -n '457,467p' src/types/provider.ts
sed -n '125,140p' src/repository/_shared/transformers.tsRepository: ding113/claude-code-hub
Length of output: 2622
Provider 和 ProviderDisplay 中的 codexServiceTierPreference 应该移除可选标记。
这个字段在两处都声明为 codexServiceTierPreference?: CodexServiceTierPreference | null,但所有相邻的 Codex 偏好字段(codexReasoningEffortPreference、codexReasoningSummaryPreference 等)都是非可选的。同时 src/repository/_shared/transformers.ts 的 toProvider() 中对所有这些字段都用 ?? null 统一归一化处理,说明字段在运行时应该总是存在。可选标记混淆了"字段缺失"和"显式设为 null"的语义,削弱类型安全性。
改为非可选,只保留 | null 表达"未设置"。
建议修改
- codexServiceTierPreference?: CodexServiceTierPreference | null;
+ codexServiceTierPreference: CodexServiceTierPreference | null;(同时修改 ProviderDisplay 类中第 462 行)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| codexServiceTierPreference?: CodexServiceTierPreference | null; | |
| codexServiceTierPreference: CodexServiceTierPreference | null; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/types/provider.ts` at line 375, Change the declaration of
codexServiceTierPreference from optional to required-but-nullable in both
Provider and ProviderDisplay (i.e., remove the "?" so it reads
codexServiceTierPreference: CodexServiceTierPreference | null) to match the
normalization in toProvider() and the other Codex preference fields; update the
two places where it’s declared (Provider and ProviderDisplay) so the type
expresses "present or null" rather than optional, ensuring consistency with
toProvider() which uses "?? null".
| > | ||
| <SelectTrigger className="w-full"> | ||
| <SelectValue placeholder="inherit" /> | ||
| </SelectTrigger> |
There was a problem hiding this comment.
[MEDIUM] [STANDARD-VIOLATION] Hardcoded Select placeholder bypasses i18n
Evidence: src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx:673 adds <SelectValue placeholder="inherit" />.
Why this is a problem: CLAUDE.md requires: i18n Required - All user-facing strings must use i18n (5 languages supported). Never hardcode display text. A hardcoded placeholder can surface untranslated UI text.
Suggested fix:
<SelectValue placeholder={t("sections.routing.codexOverrides.serviceTier.options.inherit")} />There was a problem hiding this comment.
Code Review Summary
This PR is XL largely due to generated Drizzle snapshot changes, but it also introduces new Codex service tier override behavior across validation, provider patching, proxy audit, and dashboard UI. The main actionable issue found in the diff is a new hardcoded placeholder string in the provider form that bypasses i18n.
PR Size: XL
- Lines changed: 4422
- Files changed: 35
- Split suggestion (recommended):
- PR1: DB migration +
drizzle/meta/*snapshot updates - PR2: Codex override/audit logic + unit tests
- PR3: Dashboard/provider-form UI + i18n message updates
- PR1: DB migration +
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 1 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None.
High Priority Issues (Should Fix)
- [MEDIUM] [STANDARD-VIOLATION] Hardcoded Select placeholder bypasses i18n (
src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx:673) (Confidence: 80)
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.
- PR
#870labeled assize/XL. - Inline review comment posted on
src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx:673([MEDIUM][STANDARD-VIOLATION]): hardcodedplaceholder="inherit"in new Codex service tier select; suggested switching placeholder to an i18n key. - Summary review submitted via
gh pr review(only the issue above met the confidence threshold).
Summary
Adds provider-level
service_tieroverride support for OpenAI Codex API calls, allowing administrators to configure default service tier preferences (auto/flex/priority) per provider.Changes
Core Functionality
codex_service_tier_preferencecolumn toproviderstable (migration 0078)service_tierpreference with validationUI Enhancements
codexServiceTierPreferencevia batch edit operationsservice_tierispriorityInternationalization
Added i18n strings for all 5 supported languages:
Testing
Patch Contract Updates
Extended
provider-patch-contract.tswith new special field handling forcodexServiceTierPreferenceincluding:Technical Details
Service Tier Values
auto- Default behaviorflex- Flex processingpriority- Priority processing (shown as "fast" in UI)Validation
The service tier preference is validated against OpenAI Codex API specifications with proper type checking and enum validation.
Validation Performed
bun run db:generate- Database generation successfulbun run build- Production build successfulbun run lint- Biome linting passedbun run lint:fix- Auto-fix appliedbun run typecheck- TypeScript type checking passedbun run test- All tests passingDescription enhanced with additional technical details and validation checklist
Greptile Summary
This PR adds provider-level
service_tieroverride support for the Codex (Responses API) integration, following the exact same pattern as the existingcodexReasoningEffortPreference,codexTextVerbosityPreference, andcodexParallelToolCallsPreferenceoverrides. Changes span the full stack: a non-breaking DB migration, Zod schema validation, TypeScript types, Drizzle schema, repository queries, server actions, batch patch contract, provider form UI (with i18n across 5 locales), and an orange "fast" badge in dashboard logs whenservice_tier: "priority"is in effect.Key design decisions:
applyCodexProviderOverridesWithAuditintentionally fires ahitaudit record (withchanged: false) even when the client itself sendsservice_tier: "priority"and no provider override is configured. This surfaces the "fast" badge in the logs for client-initiated priority requests, not just provider-forced ones — this behavior is explicitly tested.hasPriorityServiceTierSpecialSettinghelper checkschange.after === "priority", covering both the override case and the pass-through case above.codexServiceTierPreferenceis declared as optional (?) in theProviderandProviderDisplayinterfaces while every other codex preference field is required-but-nullable. This does not cause a runtime error (the transformer always provides the field), but it weakens TypeScript's guarantee and is inconsistent with sibling fields.Confidence Score: 4/5
codexServiceTierPreferenceis marked optional (?) in theProviderandProviderDisplayinterfaces unlike all sibling fields — a cosmetic/type-safety concern that doesn't break runtime behavior.codexServiceTierPreferenceshould be non-optional (CodexServiceTierPreference | null) in bothProviderandProviderDisplayto match all other codex preference fields.Sequence Diagram
sequenceDiagram participant Client participant Proxy as Proxy / applyCodexProviderOverridesWithAudit participant DB as providers table participant UI as Dashboard Logs UI Client->>Proxy: Request with service_tier (any) or "priority" Proxy->>DB: Load provider.codexServiceTierPreference DB-->>Proxy: "inherit" | "auto" | "default" | "flex" | "priority" | null alt Provider override is set (not inherit/null) Proxy->>Proxy: Override request.service_tier with provider value Proxy->>Proxy: Audit: hit=true, changed=(before≠after) else Client sent service_tier="priority" (no provider override) Proxy->>Proxy: Audit: hit=true, changed=false (pass-through recorded) else No override, no priority from client Proxy->>Proxy: No audit record created end Proxy-->>Client: Response Proxy->>DB: Persist special_settings audit (ProviderParameterOverride) UI->>DB: Fetch usage logs with specialSettings DB-->>UI: UsageLogRow with specialSettings[] UI->>UI: hasPriorityServiceTierSpecialSetting(specialSettings) UI-->>UI: Render orange "fast" badge if after==="priority"Last reviewed commit: 8f48cd0