fix(core): align maxRetries semantics#909
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough该 PR 统一了重试计数语义:在平台客户端和聊天模型中,将 Changes重试机制边界与计数语义统一
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Code Review
This pull request refactors the retry logic across the platform client and chat model to use a clearer distinction between retries and attempts, and updates the schema to allow a minimum of 0 retries. The reviewer identified an issue in ChatLunaChatModel where the new calculation (this._options.maxRetries ?? 1) + 1 inadvertently changes the default number of attempts from 1 to 2 when maxRetries is undefined. The reviewer suggested changing this to (this._options.maxRetries ?? 0) + 1 in two places to preserve the original default behavior.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/llm-core/platform/model.ts (1)
235-235: 💤 Low value注意
maxAttempts计算逻辑的重复。
maxAttempts的计算公式Math.max(1, (this._options.maxRetries ?? 1) + 1)在流式(Line 235)和非流式(Line 633)路径中重复出现。虽然按照代码规范应优先内联而非提取为函数,但这种重复可能在将来需要调整逻辑时(例如更改默认值)导致遗漏同步修改的风险。另外,模型层的 fallback 默认值
?? 1(表示 1 次重试 = 2 次总尝试)与配置层的默认值 5(现在表示 5 次重试 = 6 次总尝试)不一致。虽然实际运行时maxRetries应该总是从配置传入,这个 fallback 不太会被触发,但如果确实触发,会产生与配置默认值不同的行为。Also applies to: 633-633
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/llm-core/platform/model.ts` at line 235, The repeated maxAttempts calculation using Math.max(1, (this._options.maxRetries ?? 1) + 1) (seen where maxAttempts is declared and again later) should be consolidated into a single helper and the fallback default aligned with the configuration default; add a small private utility (e.g., computeMaxAttempts or normalizeMaxRetries) that reads this._options.maxRetries, applies the proper fallback (use the config default 5 instead of 1) and returns Math.max(1, retries + 1), then replace the inline occurrences of maxAttempts with calls to that helper so both the stream path and non-stream path use the same logic and default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/core/src/llm-core/platform/model.ts`:
- Line 235: The repeated maxAttempts calculation using Math.max(1,
(this._options.maxRetries ?? 1) + 1) (seen where maxAttempts is declared and
again later) should be consolidated into a single helper and the fallback
default aligned with the configuration default; add a small private utility
(e.g., computeMaxAttempts or normalizeMaxRetries) that reads
this._options.maxRetries, applies the proper fallback (use the config default 5
instead of 1) and returns Math.max(1, retries + 1), then replace the inline
occurrences of maxAttempts with calls to that helper so both the stream path and
non-stream path use the same logic and default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f3261163-b79b-4e19-90fa-755d50a1ecc7
📒 Files selected for processing (3)
packages/core/src/llm-core/platform/client.tspackages/core/src/llm-core/platform/model.tspackages/core/src/services/chat.ts
摘要
maxRetries实现,使其遵循已有配置语义:模型请求失败后的最大重试次数。maxRetries次。maxRetries: 0,用于显式关闭重试。验证
yarn fast-build core