Conversation
dosubot
bot
added
the
size:XS
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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 refines the 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
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Changing
max_context_tokensdefault from0(no compression) to4096and forcingenforce_max_turnsto15when-1is passed introduces new implicit behavior; consider driving these values from configuration or explicit defaults instead of hardcoding to avoid surprising callers. - The unconditional
max_step = min(max_step, 3)silently overrides the caller’s requestedmax_step; if this cap is needed, consider exposing it as a configuration option or clarifying it in the function’s contract rather than hardcoding it. - The inline TODO about changing
max_stepfrom 30 to a smaller value is now effectively implemented with3; either remove or rephrase the TODO to reflect the current behavior and any remaining work.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Changing `max_context_tokens` default from `0` (no compression) to `4096` and forcing `enforce_max_turns` to `15` when `-1` is passed introduces new implicit behavior; consider driving these values from configuration or explicit defaults instead of hardcoding to avoid surprising callers.
- The unconditional `max_step = min(max_step, 3)` silently overrides the caller’s requested `max_step`; if this cap is needed, consider exposing it as a configuration option or clarifying it in the function’s contract rather than hardcoding it.
- The inline TODO about changing `max_step` from 30 to a smaller value is now effectively implemented with `3`; either remove or rephrase the TODO to reflect the current behavior and any remaining work.
## Individual Comments
### Comment 1
<location path="astrbot/core/agent/runners/tool_loop_agent_runner.py" line_range="609-610" />
<code_context>
) -> T.AsyncGenerator[AgentResponse, None]:
"""Process steps until the agent is done."""
step_count = 0
+ # TODO:将max_step由30改为一个较小的值
+ max_step = min(max_step, 3)
while not self.done() and step_count < max_step:
step_count += 1
</code_context>
<issue_to_address>
**issue (bug_risk):** Hard-capping max_step to 3 may break callers and can fail when max_step is None or non-positive.
Rebinding `max_step` to `min(max_step, 3)` silently imposes a global cap and ignores caller-provided values, which can break longer-running agents. It will also raise or change behavior if `max_step` can be `None` or `<= 0` (often used to mean “no limit”). Instead, either apply this cap only at specific call sites that need it, use a separate internal/debug variable guarded by a flag, or validate/normalize `max_step` so `min` is only applied when it’s a positive int.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # TODO:将max_step由30改为一个较小的值 | ||
| max_step = min(max_step, 3) |
There was a problem hiding this comment.
issue (bug_risk): Hard-capping max_step to 3 may break callers and can fail when max_step is None or non-positive.
Rebinding max_step to min(max_step, 3) silently imposes a global cap and ignores caller-provided values, which can break longer-running agents. It will also raise or change behavior if max_step can be None or <= 0 (often used to mean “no limit”). Instead, either apply this cap only at specific call sites that need it, use a separate internal/debug variable guarded by a flag, or validate/normalize max_step so min is only applied when it’s a positive int.
dosubot
bot
added
the
area:provider
|
Related Documentation 1 document(s) may need updating based on files changed in this PR: AstrBotTeam's Space pr4697的改动View Suggested Changes@@ -273,6 +273,8 @@
- 子代理执行时，`max_steps` 参数从系统配置 `provider_settings.max_agent_step` 读取（默认值为 30）
- 用户可通过桌面应用 → 配置文件 → 普通配置 → AI 配置页面，统一设置主代理和子代理的工具调用轮数上限
- 子代理在执行长自动化任务时不再因硬编码上限而提前终止，而是遵循用户配置的上限值
+
+> **重要变更**：`max_agent_step` 配置现已被限制为最大 3 步，无论用户配置的值为多少。该限制通过 `step_until_done()` 方法中的 `max_step = min(max_step, 3)` 实现。此限制是临时措施，未来可能调整为其他较小值。
##### 参考图片自动传递（PR #5579）
@@ -494,6 +496,12 @@
#### 逻辑改进
工具注册和配置加载逻辑已优化，确保子代理配置的正确性和工具的动态注册。FunctionTool 新增 `is_background_task` 属性，支持异步后台任务。
+
+**上下文压缩配置**：工具循环 Agent Runner 的上下文管理配置已更新：
+
+- **`max_context_tokens`**：默认值从 0（禁用压缩）更改为 4096，意味着当上下文超过 4096 个 token 时，默认启用上下文压缩
+- **`enforce_max_turns`**：当配置为 -1 时，默认值为 15 个对话轮次，超过此轮数后强制执行压缩
+- 这些默认值通过 `ContextConfig` 在 `tool_loop_agent_runner.py` 的 `reset()` 方法中设置
#### MCP 客户端初始化（PR #5993）
Note: You must be authenticated to accept/decline updates. How did I do? Any feedback? |
![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 sensible defaults to improve agent behavior by preventing excessively long-running or resource-intensive executions. It sets default values for max_context_tokens and enforce_max_turns to enable context management by default, and caps the maximum number of tool loop steps. My feedback focuses on improving maintainability by replacing magic numbers with named constants. The provided rule regarding asyncio event loops does not apply to these comments, so they remain unchanged.
| max_context_tokens=provider.provider_config.get("max_context_tokens", 4096), | ||
| # enforce max turns before compression | ||
| enforce_max_turns=self.enforce_max_turns, | ||
| enforce_max_turns=self.enforce_max_turns if self.enforce_max_turns != -1 else 15, |
There was a problem hiding this comment.
The values 4096 and 15 are used as magic numbers for default configuration values. To improve code clarity and maintainability, it's recommended to define these as named constants at the module level. For example:
DEFAULT_MAX_CONTEXT_TOKENS = 4096
DEFAULT_ENFORCE_MAX_TURNS = 15These constants can then be used here, making the code easier to understand and modify in the future.
| """Process steps until the agent is done.""" | ||
| step_count = 0 | ||
| # TODO:将max_step由30改为一个较小的值 | ||
| max_step = min(max_step, 3) |
There was a problem hiding this comment.
The value 3 is hardcoded here to cap the maximum number of agent steps. Using a magic number like this can make the code harder to understand and maintain. It would be better to define it as a named constant at the module level, for instance:
MAX_TOOL_STEPS_CAP = 3Then, you could use max_step = min(max_step, MAX_TOOL_STEPS_CAP). This makes the purpose of the value clear and centralizes its definition.
1 participant
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能，已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试，并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库，或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Adjust tool loop agent defaults for context handling and step iteration limits.
Enhancements: