Skip to content

fix: address bugbot review issues from codex service tier PR #871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ding113 merged 1 commit into from
Mar 6, 2026
Merged

fix: address bugbot review issues from codex service tier PR #871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions messages/ru/dashboard.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@
"multiplier": "Множитель поставщика",
"totalCost": "Общая стоимость",
"fast": "fast",
"fastPriority": "Priority service tier (fast mode)",
"fastPriority": "Приоритетный уровень обслуживания (режим fast)",
"context1m": "1M контекст",
"context1mPricing": "Вход 2x >200k, Выход 1.5x >200k"
},
Expand Down Expand Up @@ -377,7 +377,7 @@
"multiplier": "Множитель поставщика",
"totalCost": "Общая стоимость",
"fast": "fast",
"fastPriority": "Priority service tier (fast mode)",
"fastPriority": "Приоритетный уровень обслуживания (режим fast)",
"context1m": "1M Контекст",
"context1mPricing": "Вход >200k 2x, Выход >200k 1.5x"
}
Expand Down
4 changes: 2 additions & 2 deletions messages/zh-CN/dashboard.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@
"multiplier": "供应商倍率",
"totalCost": "总费用",
"fast": "fast",
"fastPriority": "Priority service tier（fast 模式）",
"fastPriority": "优先服务等级（fast 模式）",
"context1m": "1M 上下文",
"context1mPricing": "输入 2x >200k, 输出 1.5x >200k"
},
Expand Down Expand Up @@ -377,7 +377,7 @@
"multiplier": "供应商倍率",
"totalCost": "总费用",
"fast": "fast",
"fastPriority": "Priority service tier（fast 模式）",
"fastPriority": "优先服务等级（fast 模式）",
"context1m": "1M 上下文",
"context1mPricing": "输入 >200k 2倍, 输出 >200k 1.5倍"
}
Expand Down
4 changes: 2 additions & 2 deletions messages/zh-TW/dashboard.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@
"multiplier": "供應商倍率",
"totalCost": "總費用",
"fast": "fast",
"fastPriority": "Priority service tier（fast 模式）",
"fastPriority": "優先服務層級（fast 模式）",
"context1m": "1M 上下文",
"context1mPricing": "輸入 2x >200k, 輸出 1.5x >200k"
},
Expand Down Expand Up @@ -377,7 +377,7 @@
"multiplier": "供應商倍率",
"totalCost": "總費用",
"fast": "fast",
"fastPriority": "Priority service tier（fast 模式）",
"fastPriority": "優先服務層級（fast 模式）",
"context1m": "1M 上下文長度",
"context1mPricing": "輸入 >200k 2倍, 輸出 >200k 1.5倍"
}
Expand Down
4 changes: 4 additions & 0 deletions src/actions/providers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2166,6 +2166,7 @@ export interface BatchUpdateProvidersParams {
allowed_models?: string[] | null;
allowed_clients?: string[];
blocked_clients?: string[];
codex_service_tier_preference?: CodexServiceTierPreference | null;
anthropic_thinking_budget_preference?: AnthropicThinkingBudgetPreference | null;
anthropic_adaptive_thinking?: AnthropicAdaptiveThinkingConfig | null;
};
Expand Down Expand Up @@ -2220,6 +2221,9 @@ export async function batchUpdateProviders(
if (updates.blocked_clients !== undefined) {
repositoryUpdates.blockedClients = updates.blocked_clients;
}
if (updates.codex_service_tier_preference !== undefined) {
repositoryUpdates.codexServiceTierPreference = updates.codex_service_tier_preference;
}
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Mar 6, 2026

Choose a reason for hiding this comment

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

[HIGH] [TEST-MISSING-CRITICAL] Missing unit test for new codex_service_tier_preference batch mapping (src/actions/providers.ts:2224-2226)\n\nEvidence:\n- Added mapping:\n if (updates.codex_service_tier_preference !== undefined) {\n repositoryUpdates.codexServiceTierPreference = updates.codex_service_tier_preference;\n }\n- Project rule (CLAUDE.md): 2. **Test Coverage** - All new features must have unit test coverage of at least 80%\n\nWhy this is a problem: This PR fixes a previously-dropped field; without a regression test, a future refactor can reintroduce the same silent drop and the batch edit UI will appear to succeed while not applying the service tier change.\n\nSuggested fix (add to tests/unit/actions/providers-batch-field-mapping.test.ts):\n\nts\nit("should map codex_service_tier_preference correctly", async () => {\n const { batchUpdateProviders } = await import("@/actions/providers");\n const result = await batchUpdateProviders({\n providerIds: [1],\n updates: { codex_service_tier_preference: "priority" },\n });\n\n expect(result.ok).toBe(true);\n expect(updateProvidersBatchMock).toHaveBeenCalledWith([1], {\n codexServiceTierPreference: "priority",\n });\n});\n

if (updates.anthropic_thinking_budget_preference !== undefined) {
repositoryUpdates.anthropicThinkingBudgetPreference =
updates.anthropic_thinking_budget_preference;
Expand Down
6 changes: 5 additions & 1 deletion .../[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,11 @@ export function RoutingSection() {
disabled={state.ui.isPending}
>
<SelectTrigger className="w-full">
<SelectValue placeholder="inherit" />
<SelectValue
placeholder={t(
"sections.routing.codexOverrides.serviceTier.options.inherit"
)}
/>
</SelectTrigger>
<SelectContent>
{["inherit", "auto", "default", "flex", "priority"].map((val) => (
Expand Down
3 changes: 3 additions & 0 deletions src/lib/provider-patch-contract.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,9 @@ function applyPatchField<T>(
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 };
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Mar 6, 2026

Choose a reason for hiding this comment

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

[HIGH] [TEST-MISSING-CRITICAL] Missing regression test for clear-mode codex_service_tier_preference (src/lib/provider-patch-contract.ts:884-886)\n\nEvidence:\n- New clear-mode branch:\n case "codex_service_tier_preference":\n updates.codex_service_tier_preference = "inherit";\n return { ok: true, data: undefined };\n- Project rule (CLAUDE.md): 2. **Test Coverage** - All new features must have unit test coverage of at least 80%\n\nWhy this is a problem: This PR changes batch clear behavior for service tier. Without a targeted unit test, the clear path can regress (e.g., missing switch case) and batch edit will fail to clear the override.\n\nSuggested fix (extend tests/unit/lib/provider-patch-contract.test.ts):\n\nts\nit("builds apply updates for codex_service_tier_preference clear", () => {\n const normalized = normalizeProviderBatchPatchDraft({\n codex_service_tier_preference: { clear: true },\n });\n\n expect(normalized.ok).toBe(true);\n if (!normalized.ok) return;\n\n const updates = buildProviderBatchApplyUpdates(normalized.data);\n expect(updates.ok).toBe(true);\n if (!updates.ok) return;\n\n expect(updates.data.codex_service_tier_preference).toBe("inherit");\n});\n

case "anthropic_max_tokens_preference":
updates.anthropic_max_tokens_preference = "inherit";
return { ok: true, data: undefined };
Expand Down
4 changes: 2 additions & 2 deletions src/types/provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ export interface Provider {
codexReasoningSummaryPreference: CodexReasoningSummaryPreference | null;
codexTextVerbosityPreference: CodexTextVerbosityPreference | null;
codexParallelToolCallsPreference: CodexParallelToolCallsPreference | null;
codexServiceTierPreference?: CodexServiceTierPreference | null;
codexServiceTierPreference: CodexServiceTierPreference | null;

// Anthropic (Messages API) parameter overrides (only for claude/claude-auth providers)
anthropicMaxTokensPreference: AnthropicMaxTokensPreference | null;
Expand Down Expand Up @@ -459,7 +459,7 @@ export interface ProviderDisplay {
codexReasoningSummaryPreference: CodexReasoningSummaryPreference | null;
codexTextVerbosityPreference: CodexTextVerbosityPreference | null;
codexParallelToolCallsPreference: CodexParallelToolCallsPreference | null;
codexServiceTierPreference?: CodexServiceTierPreference | null;
codexServiceTierPreference: CodexServiceTierPreference | null;
anthropicMaxTokensPreference: AnthropicMaxTokensPreference | null;
anthropicThinkingBudgetPreference: AnthropicThinkingBudgetPreference | null;
anthropicAdaptiveThinking: AnthropicAdaptiveThinkingConfig | null;
Expand Down
Loading