Skip to content

Update tool_loop_agent_runner.py #6725

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
LIghtJUNction wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update tool_loop_agent_runner.py #6725

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions astrbot/core/agent/runners/tool_loop_agent_runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,9 @@ async def reset(
# TODO: 2. after LLM output a tool call
self.context_config = ContextConfig(
# <=0 will never do compress
max_context_tokens=provider.provider_config.get("max_context_tokens", 0),
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,
Comment on lines +142 to +144
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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 = 15

These constants can then be used here, making the code easier to understand and modify in the future.

truncate_turns=self.truncate_turns,
llm_compress_instruction=self.llm_compress_instruction,
llm_compress_keep_recent=self.llm_compress_keep_recent,
Expand Down Expand Up @@ -606,6 +606,8 @@ async def step_until_done(
) -> T.AsyncGenerator[AgentResponse, None]:
"""Process steps until the agent is done."""
step_count = 0
# TODO:将max_step由30改为一个较小的值
max_step = min(max_step, 3)
Comment on lines +609 to +610
Copy link
Contributor

@sourcery-ai sourcery-ai bot Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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 = 3

Then, you could use max_step = min(max_step, MAX_TOOL_STEPS_CAP). This makes the purpose of the value clear and centralizes its definition.

while not self.done() and step_count < max_step:
step_count += 1
async for resp in self.step():
Expand Down
Loading