Repository navigation
fix(cli): surface 429 rate limit cool-down and countdown in loading indicator - #30012
urielefrenvirtusa wants to merge 10 commits into
Conversation
…ndicator - Add isRateLimitRetry helper and active countdown ticker to useLoadingIndicator - Surface 429 rate limit cool-downs immediately on Attempt 1 in default low error verbosity - Propagate optional HTTP status code on RetryAttemptPayload from core
|
📊 PR Size: size/L
|
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 enhances the terminal user experience during rate-limited API requests by surfacing explicit cool-down notices and a real-time countdown ticker. By improving error propagation and detection, the CLI now provides immediate feedback instead of appearing to hang during initial retry attempts, while maintaining backward compatibility for other network error types. 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 the 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 counterproductive. 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. Footnotes
|
|
/Gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a cool-down countdown UX for 429 rate limit retries by extracting the HTTP error status in core clients and implementing a countdown timer in the CLI's loading indicator hook, accompanied by a new test suite. The review feedback correctly identifies a bug where the countdown timer resets and jumps when paused or resumed due to the effect resetting state on inactivity. To resolve this, the reviewer suggests a simplified state adjustment pattern that preserves the countdown state when paused, along with a corresponding test improvement to properly validate the pause/resume behavior.
There was a problem hiding this comment.
Code Review
This pull request introduces a countdown and cool-down UX for 429 rate limit retries in the loading indicator. It adds status code tracking to retry events across the core clients and implements a countdown timer in the useLoadingIndicator hook, along with comprehensive unit tests. The review feedback correctly identifies a bug where pausing updates resets the countdown timer, as well as a React rule violation caused by reading a ref during rendering. It is recommended to refactor the hook to use derived state to safely track the previous retry status and preserve the countdown state when paused.
- Refactor useLoadingIndicator countdown to derive state when retryStatus changes - Prevent countdown resetting to initial delay on pause or resume - Remove prevRetryStatusRef to avoid reading refs during render - Update Case 6 test to verify countdown freezing and resuming after running
|
/Gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a cool-down countdown UX for 429 rate limit retries. It adds a status field to the RetryAttemptPayload across the core LLM clients to propagate HTTP status codes, implements a countdown timer and rate-limit detection in the useLoadingIndicator hook, and adds comprehensive unit tests. The review feedback suggests optimizing the countdown useEffect by using destructured primitive properties of retryStatus as stable dependencies to avoid unnecessary interval resets. Additionally, it recommends aligning the rate-limit attempt numbering with standard retries by using attempt + 1 and safely handling optional properties with default values.
…ttempt numbering - Destructure attempt, delayMs, and model as primitive dependencies for countdown interval - Align rate-limit attempt numbering with standard retries by using attempt + 1 - Use null-safe property access with default fallbacks for retryStatus
|
/Gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a cool-down countdown UX for 429 rate limit retries in the CLI loading indicator. It adds a status field to the RetryAttemptPayload across the core LLM clients to capture HTTP error statuses, implements rate-limit detection and countdown logic in the useLoadingIndicator hook, and adds comprehensive unit tests. The review feedback suggests improving the robustness of the isSameRetry comparison by including status and error fields, and optimizing performance in the terminal environment by only running the countdown interval when a rate limit is actually active to prevent unnecessary re-renders.
…omparison - Include status and error in isSameRetry comparison - Restrict countdown interval to rate limit retries only to avoid unnecessary renders - Remove duplicate isRateLimit definition
|
/Gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a 429 Rate Limit cool-down countdown UX to the CLI loading indicator. It updates the core clients (BaseLlmClient, GeminiClient, and GeminiChat) to propagate the HTTP error status code within the RetryAttemptPayload, allowing the useLoadingIndicator hook to detect rate limits and run a countdown timer. Comprehensive tests are also added to verify this behavior. Feedback on the changes highlights a redundant attempt === 0 guard check in the loading indicator hook's effect, and a bug in the test cleanup where vi.runOnlyPendingTimers is referenced but not invoked.
…rs in test - Remove redundant attempt === 0 check from useEffect guard - Invoke vi.runOnlyPendingTimers() inside act in test afterEach cleanup before restoring real timers
|
/Gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for tracking and displaying a cool-down countdown when encountering HTTP 429 rate limit errors. It updates the core LLM clients (BaseLlmClient, GeminiClient, and GeminiChat) to extract and include the HTTP status code in retry attempt payloads. In the CLI UI, the useLoadingIndicator hook is updated to detect rate limits, manage a countdown timer, and display a user-friendly cooling-down message. Comprehensive tests are also added to verify this countdown behavior under various scenarios. The review feedback correctly identifies a potential background resource leak in the countdown useEffect within useLoadingIndicator.ts, where the interval is not cleared once the countdown reaches its minimum value, and provides a clear suggestion to resolve it.
- Guard countdown interval with countdownSeconds === null || countdownSeconds <= 1 - Include countdownSeconds in dependency array so interval terminates cleanly when countdown completes
|
/Gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a cool-down countdown UX for 429 Rate Limit retries. It adds an isRateLimitRetry helper and countdown logic to the useLoadingIndicator hook in packages/cli, updating the UI to display a remaining seconds countdown during rate limit backoffs. To support this, the RetryAttemptPayload in packages/core has been updated to include the HTTP status code retrieved via getErrorStatus(error). Comprehensive unit tests have also been added to verify the countdown behavior under various scenarios. No review comments were provided, and the implementation looks solid, so I have no feedback to provide.
Summary
Improves the terminal user experience during HTTP 429 (Rate Limit / Quota Exceeded) events by immediately surfacing a cool-down notice on Attempt 1 and rendering an active, second-by-second visual countdown ticker in the loading indicator.
Details
lowerror verbosity mode, generic retries are suppressed on Attempt 1 until later retries. For rate limits (429status code,QUOTA_EXCEEDED,RESOURCE_EXHAUSTED), this suppression caused perceived "silent hangs". We now detect rate limit errors viaisRateLimitRetry()and surface them immediately on Attempt 1.useLoadingIndicatornow manages an active 1-second countdown interval whileisTimerActiveis true, decrementing remaining seconds (delayMs / 1000down to1s) to give continuous visual feedback:Rate limit reached. Cooling down... Retrying in 12s (Attempt 1/3)RetryAttemptPayloadinpackages/corewith optionalstatus?: number, and forwardedstatus: getErrorStatus(error)fromgeminiChat.ts,baseLlmClient.ts, andclient.ts.eslint-disablerules introduced.Related Issues
Fixes #29357
How to Validate
npm test -w @google/gemini-cli -- src/ui/hooks/useLoadingIndicator_429.test.tsxnpm test -w @google/gemini-cli -- src/ui/hooks/useLoadingIndicator.test.tsx src/ui/components/LoadingIndicator.test.tsx src/ui/components/StatusRow.test.tsxnpm test -w @google/gemini-cli-core -- src/utils/retry.test.ts src/core/geminiChat_network_retry.test.tsPre-Merge Checklist