feat: centralize maxAttempts configuration via ExperimentFlags#24761
feat: centralize maxAttempts configuration via ExperimentFlags#24761
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 centralizes the management of retry attempt limits and network request timeouts. By shifting these values from hardcoded constants to a configuration-driven approach using 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. Footnotes
|
|
Size Change: +336 B (0%) Total Size: 34 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request introduces dynamic configuration for request timeouts and maximum retry attempts by integrating experiment flags into the core configuration system. It refactors the initialization sequence to ensure global fetch timeouts are updated before client instantiation and standardizes retry logic across BaseLlmClient, GeminiChat, and WebFetchTool to use centralized settings. Feedback was provided regarding the removal of the cap on maximum retry attempts, which may require validation to prevent potential resource exhaustion from excessive retry loops.
🚨 Action Required: Eval Regressions DetectedModel: The following trustworthy evaluations passed on
The check passed or was cleared for 59 other trustworthy evaluations. 🛠️ Troubleshooting & Fix InstructionsFailure 1: should perform a case-sensitive search1. Ask Gemini CLI to fix it (Recommended)Copy and paste this prompt to the agent: 2. Reproduce LocallyRun the following command to see the failure trajectory: GEMINI_MODEL=gemini-3-flash-preview npm run test:all_evals -- evals/grep_search_functionality.eval.ts --testNamePattern="should perform a case-sensitive search"Failure 2: Agent routes global and project preferences to memory1. Ask Gemini CLI to fix it (Recommended)Copy and paste this prompt to the agent: 2. Reproduce LocallyRun the following command to see the failure trajectory: GEMINI_MODEL=gemini-3-flash-preview npm run test:all_evals -- evals/save_memory.eval.ts --testNamePattern="Agent routes global and project preferences to memory"3. Manual FixSee the Fixing Guide for detailed troubleshooting steps. This PR modifies files that affect the model's behavior (prompts, tools, or instructions).
This is an automated guidance message triggered by steering logic signatures. |
85052ac to
a2da774
Compare
Replaces hardcoded fetch timeouts in `undici`'s Agent and ProxyAgent with newly defined experiment flags `DEFAULT_REQUEST_TIMEOUT` and `DEFAULT_TOTAL_REQUEST_TIMEOUT`. This allows for remote configuration of request timeouts. - Added `DEFAULT_REQUEST_TIMEOUT` and `DEFAULT_TOTAL_REQUEST_TIMEOUT` to `ExperimentFlags`. - Updated `fetch.ts` to use `DEFAULT_REQUEST_TIMEOUT` for headers and body timeouts. - Added unit tests to verify `setGlobalProxy` correctly applies the timeout flags.
This commit centralizes the retry attempt limits to be driven by the `ExperimentFlags.MAX_ATTEMPTS` flag or the user configuration, rather than being hardcoded throughout the codebase. The retry logic in `baseLlmClient`, `geminiChat`, `client`, and `web-fetch` has been updated to retrieve the `maxAttempts` setting directly from `Config`. It also addresses the removal of the previous 10-attempt cap in the Config initialization to allow tests simulating high retry limits to pass successfully.
a2da774 to
ea3c8f4
Compare
🛑 Action Required: Evaluation ApprovalSteering changes have been detected in this PR. To prevent regressions, a maintainer must approve the evaluation run before this PR can be merged. Maintainers:
Once approved, the evaluation results will be posted here automatically. |
Summary
This PR centralizes the retry attempt limits and fetch timeouts to be driven by
ExperimentFlagsor user configuration, removing hardcoded values across the codebase.Details
baseLlmClient,geminiChat,client, andweb-fetchto retrievemaxAttemptsfromConfig.getMaxAttempts().MAX_ATTEMPTSandDEFAULT_REQUEST_TIMEOUTtoExperimentFlags.Configinitialization to support testing with higher limits.getRequestTimeoutMsto retrieve and apply fetch timeouts from experiment flags.fetch.tsto support dynamic timeout updates viaupdateGlobalFetchTimeouts.settingsSchema.tsand regeneratedsettings.mdto reflect the removal of the 10-attempt limit onmaxAttempts.Related Issues
Fixes #24016
How to Validate
npm test -w @google/gemini-cli-coreto verify retry and config logic.npm test -w @google/gemini-cli-core -- src/utils/fetch.test.tsto verify timeout updates.maxAttemptsin user config or experiment flags correctly impacts retry behavior inBaseLlmClientandGeminiChat.Pre-Merge Checklist