Skip to content

Feature: LlmAgent支持配置最大轮数/LLM次数/工具调用次数 - #286

Merged
raychen911 merged 1 commit into
mainfrom
feature/agentloop_can_exit
Aug 6, 2026
Merged

Feature: LlmAgent支持配置最大轮数/LLM次数/工具调用次数#286
raychen911 merged 1 commit into
mainfrom
feature/agentloop_can_exit

Conversation

@weimch

@weimch weimch commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

--story=136739020 LlmAgent支持配置最大轮数/LLM次数/工具调用次数

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

目前没有任何针对 max_turnsmax_tool_callsagent_limitsRunLimitException 的测试。现在我来撰写审查结论。

发现的问题

⚠️ Warning

  • trpc_agent_sdk/agents/core/_tools_processor.py:222-232:并行工具批量触发 RunLimitException 后,仅 cancel 未完成任务,但 function_response_events 可能已被部分任务写入 None/错误事件,且后续 if function_response_events: 合并逻辑在 raise 之前不会执行(raise 在 try 内),行为正确;但 cancel 后的 gather(..., return_exceptions=True) 会把 CancelledError 作为异常吞掉,不影响结果。真正风险在于:__invoke_tools 在工具内部抛 RunLimitException 时会被 line 152 的 except RunLimitException: raise 正确透传,但同一批中其他已完成任务已 append 结果事件到共享列表——由于 raise 后直接退出,这些事件被丢弃,符合"整批不执行"语义。建议补一条并行批量超限的测试以锁定该语义。

    • 实际更需关注的是:MAX_TOOL_CALLS 计数在 _llm_agent.py:566increment=len(collected_tool_calls) 一次性累加整批并 raise,但并行执行时若任务内部再触发其他 limit(如子 agent 的 max_llm_calls),多个 RunLimitException 会通过 gather 同时抛出,except RunLimitException 只捕获第一个,其余作为 CancelledError 被吞。建议确认这是预期,并补充并发超限场景测试。
  • tests/configs/test_run_config.py 及全仓测试目录:新增的 max_turnsmax_tool_callsagent_limitsAgentRunLimitsRunLimitExceptionraise_if_limit 等核心逻辑没有任何单元测试覆盖。raise_if_limit 的边界(configured_value<=0 跳过、observed>configured 触发、increment<0ValueError)、_get_run_limit 的 per-agent 继承与 0 显式禁用语义、以及 _reset_run_limit_observed 的隔离性,均属高风险且无回归保护。建议至少补 InvocationContext.raise_if_limit / _get_run_limit 的纯逻辑测试(不依赖 LLM),不依赖外部模型的 limit 触发集成测试。

    • 仓库内已有的 examples/llmagent_with_limit/run_agent.py 是需真实 LLM key 的示例,不计入测试覆盖。
  • trpc_agent_sdk/server/a2a/executor/_a2a_agent_executor.py:271-284:当 run_config_factory/run_config 配置了 server-owned RunConfig 时,用 model_copy(update={"agent_run_config": {...}}) 合并,但合并后丢失了 request 自身的顶层 limit 字段(如请求 RunConfig 若带 max_turns 会被整体替换为 configured_run_config 的值)。当前 convert_a2a_request_to_run_args 仅构造 agent_run_config、不设 limit,所以现状无 bug;但该合并是 shallow 替换整个 RunConfig,未来若请求侧也带 limit 会静默丢失。建议显式合并 limit 字段或在注释中明确"server-owned 覆盖请求 limit"的契约。

  • trpc_agent_sdk/configs/_agent_run_limits.py:29AgentRunLimits.max_llm_callsField(..., ge=0, lt=sys.maxsize),而 RunConfig.max_llm_callsfield_validator==sys.maxsize 时抛错、<=0 时打 warning(不抛错)。两者对边界值(如负数、sys.maxsize)的处理不一致:AgentRunLimits 会直接 422 校验失败,RunConfig 则宽容。建议统一两处的校验语义,避免 per-agent override 与 top-level 行为不一致引发困惑。

💡 Suggestion

  • trpc_agent_sdk/agents/_llm_agent.py:474:503raise_if_limit(MAX_TURNS) / raise_if_limit(MAX_LLM_CALLS) 计数器递增与实际"执行"之间存在窗口——计数在调用前预扣,若后续 build_request 返回 error_event 直接 return,该 turn/llm call 实际未发生但仍被计数。对低配额场景会略微提前触顶。可考虑在确认实际发起后再递增,或在文档注明"预扣"语义。

总结

整体为新增 run-limit 能力,框架各层 except RunLimitException: raise 的透传链路完整,Runner/AG-UI/A2A 三条出口的清理与事件下发处理合理,未发现阻塞级缺陷。主要风险集中在缺少对新 limit 逻辑的单元测试以及若干边界/合并语义不一致,建议合并前补齐 raise_if_limit/_get_run_limit 的纯逻辑测试。

测试建议

  • 补充 InvocationContext.raise_if_limit_get_run_limit 的纯逻辑单测:覆盖 configured<=0 跳过、observed>configured 抛错且 observed_value/configured_value 正确、agent_limits 继承与 0 显式禁用、increment<0ValueError_reset_run_limit_observed 在子 agent 调用间计数隔离。
  • 补充并行工具批量超 max_tool_calls 时"整批不执行"的测试,以及子 agent(AgentTool/run_subagent_streaming)独立计数不串扰父 agent 的测试。

@weimch
weimch force-pushed the feature/agentloop_can_exit branch from b93def9 to 52e7f47 Compare August 6, 2026 03:03
@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

get_text exists on Event. Good.

I have enough. Let me finalize the review.

One more: the create_exception_status_event docstring says "terminal failed" but final=True default — fine.

Now compile findings.

Let me reconsider the severity of the gather change. Given __invoke_tools catches all Exception subclasses and converts to error events, the only way a task raises is a BaseException subclass (Cancel/GeneratorExit/KeyboardInterrupt) or a bug. For GeneratorExit (stream closed by consumer), the old TaskGroup would cancel siblings; the new code lets them run. This could lead to background tool execution continuing after the client disconnected — a real but edge resource leak. Warning level is appropriate.

Also, the new code on RunLimitException does cancel siblings — but RunLimitException can't actually be raised inside __invoke_tools (raise_if_limit for tool calls is at the llm_agent level before execution). So the except RunLimitException branch in tools_processor is effectively dead for the parallel path, while the regression for other BaseExceptions is real. This strengthens the warning: the change adds complexity for a case that can't occur, while removing sibling cancellation for cases that can.

Let me write the review.## 发现的问题

🚨 Critical

未发现必须修复的阻塞性问题。

⚠️ Warning

  • trpc_agent_sdk/agents/core/_tools_processor.py:217-233:并行工具执行由 asyncio.TaskGroup 改为 asyncio.gather,丢失了对兄弟任务的取消语义。

    • 原实现中任一任务抛错时 TaskGroup 会取消其余运行中的任务;新代码仅对 RunLimitException 做取消,其它异常(如 GeneratorExit/CancelledError__invoke_toolsexcept Exception 无法捕获这些 BaseException 子类)经 gather 抛出后,未完成的兄弟任务不会被取消,会作为孤儿任务继续执行(产生网络调用、写入 function_response_events 等副作用)。而 RunLimitException 实际不会在此处触发——raise_if_limit(MAX_TOOL_CALLS)_llm_agent.py:569 调用、早于工具执行。建议在通用异常分支也显式取消剩余任务,或继续使用 TaskGroup 并通过解包 ExceptionGroup 保留 RunLimitException 透传。
    ...
    try:
        await asyncio.gather(*tasks)
    except RunLimitException:
        for task in tasks:
            if not task.done():
                task.cancel()
        ...
  • trpc_agent_sdk/configs/_run_config.py:30(对应 max_llm_calls 默认 500):本次 PR 为原本无限制的 Runner.run_async() 引入了默认生效的 max_llm_calls=500,对存量调用方是隐式的行为变更。

    • 此前 origin/main 上未强制任何 LLM 调用上限;合入后任何未显式配置的调用在第 501 次 LLM 调用时会抛 RunLimitException。这是有意设计且文档已说明,但对长会话/循环型 Agent 的存量用户属于兼容性风险,建议在升级说明中明确提示,或评估是否将默认值改为 0(不限制)以保持向后兼容。
  • trpc_agent_sdk/runners.py:577-585RunLimitException 分支调用 cleanup_incomplete_function_callsupdate_session,但未像 RunCancelledException 分支那样通过 trace_cancellation 记录终止 trace。

    • 该分支设置了 trace_error_type/trace_error_message,最终 finally 中的 trace_runner 会带上错误信息(runner_trace_recorded 仍为 False),可以覆盖;但取消分支会显式 runner_trace_recorded = True 并补发 AgentCancelledEvent。RunLimit 路径既不补发终止事件、也未对 temp_text_parts 做收尾,流式消费者可能拿不到本次调用的最终状态。建议确认是否需要对齐取消分支的事件收尾,或在文档/示例中说明调用方需自行捕获 RunLimitException 处理。

💡 Suggestion

  • trpc_agent_sdk/context/_invocation_context.py:341-369raise_if_limitconfigured_value <= 0(禁用)时仍会累加 _observed_run_limits 计数。当前无害,但禁用场景下无谓写入私有状态,可在禁用时提前 return 以明确意图并避免长期累积。

总结

整体风险中等:run-limit 功能的核心计数、异常透传、trace/指标改造逻辑自洽,测试也同步更新。主要问题集中在并行工具执行改用 gather 后丢失兄弟任务取消语义,以及默认 max_llm_calls=500 对存量调用方的隐式兼容性影响,建议合入前处理。

测试建议

  • 补充并行工具执行(parallel_tool_calls=True)下单个工具抛 GeneratorExit/CancelledError 时,其余兄弟任务被取消、无孤儿任务残留的测试。
  • 补充 max_tool_calls 批量超限(LLM 一次返回多个工具调用)时整批不执行的集成测试,覆盖 _llm_agent.py:569 的提前抛出路径。

@weimch
weimch force-pushed the feature/agentloop_can_exit branch from 52e7f47 to 920d0db Compare August 6, 2026 03:27
--story=136739020 LlmAgent支持配置最大轮数/LLM次数/工具调用次数
@weimch
weimch force-pushed the feature/agentloop_can_exit branch from 920d0db to eade0a9 Compare August 6, 2026 03:28
@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

现在让我来撰写最终的审查结论。

发现的问题

🚨 Critical

  • examples/llmagent_with_limit/run_agent.py:216-218:示例代码硬编码了真实的 Langfuse 凭证
    • public_keysecret_key 直接以字面量写入并随仓库提交,secret_key 是真实可用的密钥(sk-lf-...),泄露后任何人都能向该 Langfuse 实例(http://9.135.94.249:3000)上报/读取数据。示例应改为从环境变量读取(与同目录 .envTRPC_AGENT_API_KEY 一致的做法),并应立即吊销已泄露的 key。
      LangfuseConfig(
          public_key="pk-lf-d56fef7a-a925-44eb-8203-885b00221913",
          secret_key="sk-lf-94bd4402-c7c9-40d7-a0f0-0bc80a3c4e24",
          host="http://9.135.94.249:3000",
      )

⚠️ Warning

  • trpc_agent_sdk/context/_invocation_context.py:341-371raise_if_limit / _get_run_limit 核心限制逻辑缺少单元测试

    • 本次新增的计数、阈值比较、agent_limits 继承与 0 禁用语义(observed_value > configured_valueNone 继承顶层、0 关闭限制)均无任何测试覆盖;tests/ 中仅有 RunLimitException/RunLimitType 的导入校验。该逻辑直接决定框架何时中断 Agent 运行,属于高风险路径。建议补 tests/context/test_invocation_context.py 用例:max_iterations=1 触发 observed=2agent_limits 覆盖与继承、0 禁用、负增量抛 ValueError 等场景。
  • trpc_agent_sdk/agents/_llm_agent.py:569-572MAX_TOOL_CALLS 检查点位于并行工具执行之前,使并行取消分支实际不可达

    • ctx.raise_if_limit(MAX_TOOL_CALLS, increment=len(collected_tool_calls))execute_tools_async 之前执行,超限时会在任何工具运行前抛出;因此 _tools_processor.py:228-233 中针对并行任务的 RunLimitException 取消逻辑在该限制下不会触发。该取消分支属于无效防御代码,建议确认是否有其他来源的 RunLimitException(当前 diff 中无),否则可删除以减少维护负担,或在注释中说明保留意图。

💡 Suggestion

  • trpc_agent_sdk/server/a2a/executor/_a2a_agent_executor.py:277-285:服务端 run_config 合并会静默丢弃请求方传入的 max_llm_calls/max_iterations/max_tool_calls/agent_limits
    • 当配置了 run_config/run_config_factory 时,合并只保留 agent_run_config(请求 metadata),其余字段全部取自服务端配置,客户端传入的限制被忽略。这与 docstring “server-owned configuration” 描述一致,但行为对调用方不透明;建议在 docstring 中明确“客户端限制字段将被服务端配置覆盖”,避免误用。

总结

核心限制机制(raise_if_limit 计数与继承语义、各执行边界的 RunLimitException 透传、runner/ag-ui/a2a 的异常处理与清理)实现完整且一致;主要阻塞问题是示例中硬编码了真实的 Langfuse 密钥,必须修复并吊销。次要注意点是核心限制逻辑缺乏单元测试。

测试建议

  • InvocationContext.raise_if_limit / _get_run_limit 补充单元测试:覆盖 max_iterations=1observed=2 触发)、agent_limits 覆盖与 None 继承顶层、0 禁用、increment<0ValueError,以及 RunLimitExceptionerror_code/get_custom_metadata 字段。
  • 补一条端到端测试:同一 session 第一次触发 RunLimitException、第二次用解除限制的 RunConfig 继续运行,验证会话未被关闭且未残留未完成的 function_call(覆盖 runners.py:580-587 的清理路径)。


langfuse_opentelemetry_setup(
LangfuseConfig(
public_key="pk-lf-d56fef7a-a925-44eb-8203-885b00221913",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

示例硬编码真实 Langfuse 凭证

public_key 与 secret_key 以字面量写入并随仓库提交,secret_key 为真实可用密钥,泄露后任何人都能向该 Langfuse 实例上报或读取数据。应改为从环境变量读取,并立即吊销已泄露的 key。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

基于我对 pr.diff 及相关仓库上下文的分析,以下是我的审查结论。

发现的问题

⚠️ Warning

  • trpc_agent_sdk/agents/core/_tools_processor.py:226-233:并行工具执行从 TaskGroup 改为 asyncio.gather 后,丢失了对非 RunLimitException 异常的兄弟任务取消

    • except RunLimitException 分支会取消并回收未完成任务,但当一个并行工具抛出普通异常(如 ValueError)时,gather 直接抛出而不会取消其他仍在运行的任务,这些任务会变成孤儿任务继续执行,并可能继续向共享的 function_response_events 列表写入,造成资源泄漏与潜在数据竞争。原 TaskGroup 会在首个异常时取消所有兄弟任务。建议在 except RunLimitException 之外再补一个 except BaseException(或 except Exception)分支,同样执行取消 + gather(..., return_exceptions=True) 后再 raise
    • try:
          await asyncio.gather(*tasks)
      except RunLimitException:
          ...
          raise
      # 普通异常会直接抛出,未取消其他 task
  • tests/:新增的运行次数限制核心逻辑缺少单元测试

    • raise_if_limit 的计数累加、0 表示禁用、批量超限前置拒绝、agent_limitsagent.name 的精确匹配与继承、以及 _create_invocation_context_reset_run_limit_observed 借助 model_copy 实现的“每次调用独立计数”隔离等高风险控制流,均无对应测试(现有测试只覆盖了 RunLimitException/RunLimitType 的构造与公开 API)。建议补充针对 InvocationContext.raise_if_limit 的直接单测,覆盖禁用、单次/批量、继承与按 agent 覆盖、跨调用重置等路径。

💡 Suggestion

  • trpc_agent_sdk/configs/_agent_run_limits.py:29max_llm_calls 的校验与 RunConfig 不一致
    • AgentRunLimitsField(..., ge=0, lt=sys.maxsize) 直接拒绝 sys.maxsize,而 RunConfig.max_llm_callsfield_validator 仅拒绝恰好等于 sys.maxsize 的值并对 <=0 发告警;两者对边界值的处理不一致,长期维护易产生混淆。建议统一为同一种校验方式,或让 AgentRunLimits 复用同一校验逻辑。

总结

整体实现合理:限制计数逻辑、0 禁用语义、批量工具调用前置拒绝、异常在各执行边界(agent/llm/tool/graph/sub-agent/filter)的透传与会话清理均与文档一致,未发现阻塞级问题。主要风险是并行工具执行路径在普通异常下不再取消兄弟任务(建议修复),以及核心限制逻辑缺少单元测试覆盖。

测试建议

  • InvocationContext.raise_if_limit 补充单测:configured_value=0 不触发、单次递增恰好等于限制时不触发、超出时抛出且 observed_value 正确、increment<0ValueError
  • 补充并行工具执行(parallel_tool_calls=True)下,单个工具抛普通异常时兄弟任务被取消/不泄漏的回归测试。

@raychen911
raychen911 merged commit 859ac4e into main Aug 6, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants