Repository navigation
feat(codex): add fast tier settings toggle - #367
Conversation
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 new feature allowing users to configure a 'fast' service tier for Codex requests via a dashboard setting. When enabled, the system will automatically apply a 'priority' service tier to Codex requests that do not specify one, optimizing performance. It also ensures consistent handling of the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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
|
![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 a new dashboard setting to enable a 'fast' service tier for Codex requests by default. The changes include a new UI component for the setting, an API route to manage it, and updates to the Codex executor to apply the setting. The implementation is solid, but I have a couple of suggestions to improve error handling in the new UI component and to enhance type safety in the new API route.
| const save = async (nextEnabled: boolean) => { | ||
| setEnabled(nextEnabled); | ||
| setSaving(true); | ||
| setStatus(""); | ||
|
|
||
| try { | ||
| const res = await fetch("/api/settings/codex-service-tier", { | ||
| method: "PUT", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ enabled: nextEnabled }), | ||
| }); | ||
|
|
||
| if (res.ok) { | ||
| setStatus("saved"); | ||
| setTimeout(() => setStatus(""), 2000); | ||
| } else { | ||
| setStatus("error"); | ||
| } | ||
| } catch { | ||
| setStatus("error"); | ||
| } finally { | ||
| setSaving(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The error handling in the save function can be improved:
- Silent Failure: The
statusis set to"error"on failure, but there is no corresponding UI element to display this error. This means the save operation fails silently for the user. - Inconsistent State: The optimistic UI update is not reverted on failure, leaving the toggle in a state that doesn't match the backend.
To fix the inconsistent state, you can revert the enabled state on failure. For the silent failure, you should add a UI element to display when status === 'error'. Here is a suggestion to fix the state inconsistency:
const save = async (nextEnabled: boolean) => {
setEnabled(nextEnabled);
setSaving(true);
setStatus("");
try {
const res = await fetch("/api/settings/codex-service-tier", {
method: "PUT",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ enabled: nextEnabled }),
});
if (res.ok) {
setStatus("saved");
setTimeout(() => setStatus(""), 2000);
} else {
setStatus("error");
setEnabled(!nextEnabled);
}
} catch {
setStatus("error");
setEnabled(!nextEnabled);
} finally {
setSaving(false);
}
};
| } | ||
| } | ||
|
|
||
| export async function PUT(request) { |
There was a problem hiding this comment.
For improved type safety and code clarity, the request parameter in the PUT handler should be typed. You can use the Request type from next/server.
You will need to update the import statement at the top of the file to include it:
import { NextResponse, type Request } from "next/server";| export async function PUT(request) { | |
| export async function PUT(request: Request) { |
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Existing Comments (2)The following issues were already flagged by previous reviewers and are NOT duplicated here:
Files Reviewed (8 files)
Note: The existing comments cover important issues including error handling in the UI component and type safety. The race condition issue reported above is a new finding that should be addressed. |
a23ec7f to
2dfc356
Compare
Add a default-off dashboard setting that injects Codex fast service tier only when the request did not already specify one. Also preserve service_tier through OpenAI-to-Responses translation and restore the setting at startup.
2dfc356 to
00188f7
Compare
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| } | ||
|
|
||
| const config = validation.data; | ||
| setDefaultFastServiceTierEnabled(config.enabled); |
There was a problem hiding this comment.
CRITICAL: Race condition - in-memory state updated before database
The in-memory state (setDefaultFastServiceTierEnabled) is updated on line 47 BEFORE the database is updated on line 48. If updateSettings throws an error, the in-memory state will be inconsistent with the persisted state.
Consider updating the database first, then updating in-memory state, or wrap both in a transaction with rollback capability.
Thanks @kfiramar! Codex fast-tier toggle merged 🎉 — default-off, full stack (UI tab + API + executor injection + translator passthrough + startup restore). 48 tests passing. Users can now enable flex tier in Dashboard → Settings → Codex Service Tier.
- PR diegosouzapw#368: gpt-5.4 in Codex model registry (cx/gpt-5.4, codex/gpt-5.4) - PR diegosouzapw#367: Codex fast tier toggle (default-off, full stack, 48 tests) - PR diegosouzapw#366: Codex quota policy 5h/weekly with auto-rotation - fix diegosouzapw#356: analytics charts show provider display names not raw IDs
Thanks @kfiramar! Codex fast-tier toggle merged 🎉 — default-off, full stack (UI tab + API + executor injection + translator passthrough + startup restore). 48 tests passing. Users can now enable flex tier in Dashboard → Settings → Codex Service Tier.
- PR diegosouzapw#368: gpt-5.4 in Codex model registry (cx/gpt-5.4, codex/gpt-5.4) - PR diegosouzapw#367: Codex fast tier toggle (default-off, full stack, 48 tests) - PR diegosouzapw#366: Codex quota policy 5h/weekly with auto-rotation - fix diegosouzapw#356: analytics charts show provider display names not raw IDs
Thanks @kfiramar! Codex fast-tier toggle merged 🎉 — default-off, full stack (UI tab + API + executor injection + translator passthrough + startup restore). 48 tests passing. Users can now enable flex tier in Dashboard → Settings → Codex Service Tier.
- PR diegosouzapw#368: gpt-5.4 in Codex model registry (cx/gpt-5.4, codex/gpt-5.4) - PR diegosouzapw#367: Codex fast tier toggle (default-off, full stack, 48 tests) - PR diegosouzapw#366: Codex quota policy 5h/weekly with auto-rotation - fix diegosouzapw#356: analytics charts show provider display names not raw IDs
2 participants
Summary
Testing