Skip to content

feat: trpc-agent框架web工具支持tavily#199

Merged
raychen911 merged 1 commit into
trpc-group:mainfrom
weimch:feature/weimch_dev
Jul 17, 2026
Merged

feat: trpc-agent框架web工具支持tavily#199
raychen911 merged 1 commit into
trpc-group:mainfrom
weimch:feature/weimch_dev

Conversation

@weimch

@weimch weimch commented Jul 16, 2026

Copy link
Copy Markdown
Contributor
  • 接入tavily到websearch及webfetch工具
  • 补充单测及文档

@CongkeChen

Copy link
Copy Markdown
Contributor

AI Code Review

审查完成。我先核对关键调用链(SSRF、缓存、provider 分发、payload 构造与解析),下面给出结论。

发现的问题

⚠️ Warning

  • trpc_agent_sdk/tools/_websearch_tool.py:597-609:649include_images 双来源不一致,可能静默产生额外费用
    • 请求体里 include_images 先取自调用参数(:603),随后被 tavily_extra_params 整体 update 覆盖(:609);但解析阶段用的是局部变量 include_images:649,来自 args,默认 False)。若调用方在 tavily_extra_params 里固化了 include_images: True(文档把该字段列为可透传项),每次请求都会让 Tavily 返回图片(更慢、更贵、响应更大),但代码因 if include_images: 为假而把图片全部丢弃,用户拿不到也察觉不到原因。建议统一以「最终生效的 include_images」驱动解析,例如 effective_include_images = include_images or bool(self._tavily_extra_params.get("include_images")),并据此判断解析。

💡 Suggestion

  • trpc_agent_sdk/tools/_webfetch_tool.py:623-628:Tavily Extract 路径硬编码 include_images: False 后再 update(extra_params),但该工具只返回 raw_content/content 文本,从不解析图片。即便用户通过 tavily_extra_params={"include_images": True} 传入,图片数据也会被丢弃。建议要么不在 Extract payload 里默认塞该字段,要么在文档/参数说明里明确该参数对 WebFetchTool 无效,避免误导。

总结

整体改动结构清晰,SSRF 校验、域名策略、缓存与凭据回退在 Tavily 路径上基本对齐了既有 direct/google 路径,测试覆盖也较充分;未发现阻塞级问题。主要风险是 include_images 在透传参数与解析逻辑间的口径不一致,可能导致额外开销与预期落空,建议合并修复。

测试建议

  • 建议补一条用例:WebSearchTool(provider="tavily", tavily_extra_params={"include_images": True}) 且调用时不传 include_images,断言返回结果中 images 是否符合预期(当前会为空),以固定该口径的预期行为。

@weimch
weimch force-pushed the feature/weimch_dev branch from bac52f7 to 6ccb485 Compare July 17, 2026 02:27
@CongkeChen

Copy link
Copy Markdown
Contributor

AI Code Review

发现的问题

🚨 Critical

(无)

⚠️ Warning

  • trpc_agent_sdk/tools/_websearch_tool.py:347:构造期对非法 providerValueError 属于破坏性变更

    • 旧实现允许 WebSearchTool(provider="bing") 正常构造(旧测试 test_invalid_provider_accepted_by_construction 明确固化了这一行为,运行时再降级为 Google 错误提示)。新实现改为构造期 raise ValueError,任何依赖旧行为(如从配置动态传入 provider 字符串)的下游在构造阶段就会抛异常而非运行期兜底。建议在 release notes 中显式标注此 breaking change,或对未知 provider 退回旧行为以保持兼容。
  • trpc_agent_sdk/tools/_webfetch_tool.py:621tavily 模式 SSRF 仅校验用户传入的首跳 URL,无法覆盖 Tavily 服务端跟随的重定向

    • direct 模式对每一跳重定向都做 _check_ssrf,文档也声称 "tavily 模式同样会在调用 Extract 前校验目标主机"。但实际 _extract_with_tavily 只对原始 url 调一次 _check_ssrf,Tavily 服务端后续的重定向解析无法被框架拦截。这是 Tavily 服务端抓取的固有限制,建议在文档中明确 "tavily 模式仅校验首跳、不保证逐跳 SSRF",避免给使用者留下与 direct 等价的错觉。
  • trpc_agent_sdk/tools/_webfetch_tool.py:643:Tavily HTTP 状态错误未映射为文档承诺的 TAVILY_EXTRACT_ERROR

    • response.raise_for_status() 抛出的 HTTPStatusError 会被 _fetch_and_buildexcept httpx.HTTPError 捕获,统一变成 HTTP_ERROR,而非文档列为 tavily 专属错误的 TAVILY_EXTRACT_ERROR(该码只在响应体 failed_results/无 results 时出现)。下游 LLM 若按 TAVILY_EXTRACT_ERROR 做 tavily 专属降级判断会漏掉 4xx/5xx 场景。建议在 _extract_with_tavily 内捕获 HTTPStatusError 并返回 TAVILY_EXTRACT_ERROR,或修正文档措辞。

💡 Suggestion

  • trpc_agent_sdk/tools/_websearch_tool.py:686summary 无条件使用响应中的 answer 字段,与请求 payload 的 include_answer 解耦。happy-path 测试在 include_answer=False 时仍断言 summary 含 answer 内容,掩盖了这一不一致;建议仅在确实请求了 answer 时回填 summary,或补充说明。

总结

整体风险可控,未发现阻塞级问题;Tavily provider 的接入逻辑、SSRF/凭据校验、去重与测试覆盖基本完整。主要需关注 WebSearchTool 对非法 provider 改为构造期抛异常带来的兼容性变更,以及 tavily 模式下 SSRF/错误码与文档表述的两处偏差。

测试建议

  • 补充 WebSearchTool(provider="bing") 构造抛异常是否与外部配置注入兼容的回归说明(若确认 breaking,需在 changelog 标注)。
  • 补充 WebFetchTool tavily 模式下 raise_for_status()(如 401/500)返回错误码的断言,明确是 HTTP_ERROR 还是 TAVILY_EXTRACT_ERROR

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.77612% with 7 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@eac046f). Learn more about missing BASE report.

Files with missing lines Patch % Lines
trpc_agent_sdk/tools/_websearch_tool.py 95.55556% 4 Missing ⚠️
trpc_agent_sdk/tools/_webfetch_tool.py 93.18182% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main        #199   +/-   ##
==========================================
  Coverage        ?   87.90237%           
==========================================
  Files           ?         479           
  Lines           ?       44984           
  Branches        ?           0           
==========================================
  Hits            ?       39542           
  Misses          ?        5442           
  Partials        ?           0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@weimch
weimch force-pushed the feature/weimch_dev branch 2 times, most recently from 563f7e8 to 262cac5 Compare July 17, 2026 03:44
@CongkeChen

Copy link
Copy Markdown
Contributor

AI Code Review

我已读完 pr.diff 全部变更(文档、两个工具的源码、两个测试文件),并对照了仓库中 _webfetch_tool.py / _websearch_tool.py 的相关上下文(SSRF 校验、域名过滤、helper 函数、重定向循环)。下面是审查结论。

发现的问题

⚠️ Warning

  • trpc_agent_sdk/tools/_websearch_tool.py:506trpc_agent_sdk/tools/_websearch_tool.py:658include_images 解析与 include_answer 处理不对称,固化的 tavily_extra_params 会被静默丢弃
    • include_answerpayload.update(self._tavily_extra_params) 之后用 bool(payload.get("include_answer")) 重新计算,因此通过 tavily_extra_params 打开 answer 时能正确回填 summary;但 include_images 始终用调用参数派生的局部变量(include_images = bool(args.get("include_images", False))),未从最终 payload 重算。若开发者通过 tavily_extra_params={"include_images": True} 固化开启图片,payload 会带上该标志、Tavily 会返回图片并计费,但 if include_images: 分支不进入,images 返回空列表——产生静默数据丢失与无谓开销。建议与 include_answer 一致,从最终 payload 重算 include_images 再决定是否解析图片。

💡 Suggestion

  • trpc_agent_sdk/tools/_webfetch_tool.py:625trpc_agent_sdk/tools/_websearch_tool.py:615payload.update(self._tavily_extra_params) 在内置键之后执行,可覆盖安全相关字段
    • _extract_with_tavily 中,urls 先经 _check_ssrf(url) 校验,随后才 payload.update(extra_params);若 tavily_extra_paramsurls/extract_depth 等键会覆盖已校验值(websearch 侧同理可覆盖 query/max_results/include_domains)。extra_params 属构造期可信配置,不构成实际 SSRF 绕过,但属于易踩的维护性隐患。建议先 update(extra_params) 再强制写入框架管控的键(urls/query/include_domains 等),使 SSRF 校验的对象与实际请求体一致。

总结

整体变更质量较高:Tavily provider 的凭证缺失、SSRF 前置校验、HTTP/抽取失败映射、域名映射与去重、图片去重均有测试覆盖,且对既有 WebSearchResult / FetchResult schema 为非破坏性扩展。未发现 Critical 级别问题;存在 1 处 Warning(include_images 解析不对称导致固化配置下图片被静默丢弃),建议修复。

测试建议

  • 补充一条用例:WebSearchTool(provider="tavily", tavily_extra_params={"include_images": True}) 且调用参数不传 include_images,断言返回的 images 非空(当前会失败,可锁定上述 Warning)。

@CongkeChen

Copy link
Copy Markdown
Contributor

AI Code Review

我已经掌握了所需的所有上下文。让我来写这份审查报告。

发现的问题

🚨 Critical

  • trpc_agent_sdk/tools/_websearch_tool.py:519-520:未知 provider 静默走 Google 分支,导致凭据/计费错配
    • 构造器对非 duckduckgo/google/tavily 的 provider 只记 warning,运行时 else 分支会落到 _search_google,并用 GOOGLE_CSE_API_KEY 兜底拼请求。若调用方拼错 provider 名(如 "Tavily" 大写、"tavily_search"),工具不会报错,而是用 Google 凭据去打 Google 接口、或直接返回 "not configured",与用户预期完全不符,排障困难。注释还把 "runtime falls through to the Google search path" 当作设计,但这并非有意兜底而是漏处理。建议未知 provider 在 _run_async_impl 返回明确的 INVALID_PROVIDER 结构化错误,而不是隐式走 Google。
    • 对比 WebFetchTool 对未知 provider 直接 raise ValueError_webfetch_tool.py:444),两工具行为不一致,且 WebSearchTool 这条路径测试还断言 t._base_url == google...test_websearch_tool.py:154)把缺陷固化成了预期。

⚠️ Warning

  • trpc_agent_sdk/tools/_webfetch_tool.py:614-691:tavily 路径的 httpx.HTTPError(非 status)未被结构化捕获,且绕过了 _fetch_and_build 的异常映射

    • _extract_with_tavilyexcept httpx.HTTPStatusError,连接超时/网络错误等 httpx.HTTPError 子类会直接抛出。虽然外层 _fetch_and_buildexcept httpx.HTTPError 兜底映射为 HTTP_ERROR,但 _extract_with_tavily 内部用了 client.post(...).raise_for_status(),非 status 类异常会跳过 finally 中的 aclose(实际 finally 仍执行,OK)——主要问题是错误码风格不统一:tavily 的 4xx/5xx 走 TAVILY_EXTRACT_ERROR,而连接类错误走外层 HTTP_ERROR,且后者丢失了 status_code。建议在 tavily 分支内统一捕获 httpx.HTTPError 并映射为 TAVILY_EXTRACT_ERROR,或至少在文档/错误码约定中说明。
  • trpc_agent_sdk/tools/_webfetch_tool.py:623:tavily 模式 SSRF 校验在缺凭据返回之后,且无域名白/黑名单二次校验

    • _extract_with_tavily_check_ssrf 之前已 return TAVILY_NOT_CONFIGURED,这没问题;但 _run_async_impl 的工具级 _is_blocked_url:541)在进入 _fetch_and_build 前已做过,OK。真正风险是:tavily 模式只对"首跳 URL"做 SSRF,文档已坦承无法拦截 Tavily 端重定向——这一点应在 block_private_network 的 docstring 里更显眼提示(目前只在 Args 里一行),否则部署方易误以为等价于 direct 的逐跳防护。建议在 tavily 构造时若 block_private_network=True 额外 log 一次提醒。
  • trpc_agent_sdk/tools/_websearch_tool.py:656-682:图片 URL 仅做"能否解析出域名"过滤,未应用 allowed/blocked_domains 策略

    • include_images=true 时返回的图片 URL 只用 _extract_domain_from_url(url) == "" 过滤空 host,未经过 _is_blocked(...) 域名白/黑名单。当 LLM 传入 allowed_domains 限定可信站点时,图片仍可能来自任意第三方域名(含追踪/广告 CDN),与"域名策略"语义不一致,也可能把不可信图片 URL 直接交给下游渲染。建议图片候选同样走 _is_blocked(image_url, allowed, blocked)
  • trpc_agent_sdk/tools/_websearch_tool.py:603-618tavily_extra_params 可覆盖 include_domains/exclude_domains/max_results,导致调用方域名策略被静默绕过

    • payload.update(self._tavily_extra_params) 在写入 include_domains/exclude_domains 之后执行,若用户在 tavily_extra_params 里设了 include_domains,会覆盖本次调用 LLM 传入的 allowed_domains 映射结果,使白名单失效;max_results 同理可被改大超过 count clamp。建议把 tavily_extra_params 的合并放到受控字段之前,或对安全相关字段(include_domains/exclude_domains/max_results/query)做"调用方值优先、禁止 extra_params 覆盖"的保护。
  • tests/tools/test_websearch_tool.py:1121-1154test_happy_path 期望 payload 含 include_raw_content,但源码 payload 同样写死该字段——两者一致,但未覆盖 include_answertavily_extra_params 与默认值合并冲突时的实际生效值

    • include_answer 最终值取自 payload.get("include_answer")(合并后),逻辑正确且有 test_tavily_extra_params_are_merged 覆盖;但缺少一条"调用方未传 include_images、但 tavily_extra_params 设 include_images=True"时 payload 最终值的断言,存在合并顺序回归盲区。建议补一条 extra_params 覆盖布尔默认值的测试。
  • trpc_agent_sdk/tools/_webfetch_tool.py:677:tavily 成功路径 status_code 固定写 200,丢失 Tavily 实际响应码

    • 即使 Tavily 返回 2xx 以外的成功语义(或未来返回 201),_extract_with_tavily 硬编码 status_code=200/status_text="OK",而 direct 路径用真实 response.status_code。下游若用 status_code 做监控/告警区分会失真。建议用 response.status_code

💡 Suggestion

  • trpc_agent_sdk/tools/_websearch_tool.py:584-693_search_tavily_search_google/_search_duckduckgo 在去重、截断、错误映射上逻辑高度相似,且图片解析分支较长。可将"结果遍历 + 去重 + 截断 + 截断到 n"抽成共用helper,减少三处重复,便于后续新增 provider 时统一行为。

  • CHANGELOG.md:本 PR 新增 Tavily provider(WebFetch + WebSearch)属 Features,但 CHANGELOG 未更新。按仓库惯例(顶部 ## [1.1.12] ... Features 段)建议补一条,避免发版遗漏。

总结

新增 Tavily provider 整体结构清晰、测试覆盖较全,无安全阻塞问题;主要风险集中在 WebSearchTool 对未知 provider 的静默 Google 兜底(Critical,建议修复),以及 tavily 路径 extra_params 可覆盖域名安全字段、图片未走域名策略、HTTP 错误映射不一致等稳定性/兼容性 Warning。

测试建议

  • 补一条用例:provider="tavily"tavily_extra_params={"include_domains": ["evil.com"]} + 调用参数 allowed_domains=["python.org"],断言最终 payload 与返回结果以 allowed_domains 为准(覆盖 _websearch_tool.py:615 的合并顺序回归)。
  • 补一条用例:include_images=true + allowed_domains=["python.org"],断言返回图片仅来自 python.org(验证图片候选也走域名策略,修复后应通过)。

lang = (args.get("lang") or self._lang or "").strip() or None
include_images = bool(args.get("include_images", False))

try:

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.

未知 provider 静默走 Google 分支,导致凭据/计费错配

构造器对非 duckduckgo/google/tavily 的 provider 只记 warning,运行时 else 分支会落到 _search_google 并用 GOOGLE_CSE_API_KEY 兜底。调用方拼错 provider 名(如 "Tavily"、"tavily_search")不会报错,而用 Google 凭据打 Google 接口或返回 not configured,排障困难。建议未知 provider 在 _run_async_impl 返回明确的 INVALID_PROVIDER 结构化错误,而非隐式走 Google。与 WebFetchTool 对未知 provider 直接 raise ValueError 的行为不一致。

- 接入tavily到websearch及webfetch工具
- 补充单测及文档
@weimch
weimch force-pushed the feature/weimch_dev branch from 262cac5 to c867ac8 Compare July 17, 2026 03:56
@CongkeChen

Copy link
Copy Markdown
Contributor

AI Code Review

审查完成。基于 pr.diff(新增 Tavily provider 给 WebFetchTool / WebSearchTool)并结合仓库中 _webfetch_tool.py / _websearch_tool.py 的相关上下文,结论如下。

发现的问题

⚠️ Warning

  • trpc_agent_sdk/tools/_websearch_tool.py:647-673:Tavily 图片结果未受域名白/黑名单约束

    • _search_tavilyresults 调用了 _is_blocked(url, allowed, blocked) 做客户端二次过滤,但 images 收集循环只做 _extract_domain_from_url(...) == "" 的空域名检查,未调用 _is_blocked。当 LLM 通过 allowed_domains 把检索限定到可信站点时,Tavily 仍可能返回其它域的图片 URL 并透传给 LLM,违反工具级域名策略。建议在加入 images 前对 image_url 同样执行 _is_blocked(image_url, allowed, blocked) 过滤。
  • tests/tools/test_websearch_tool.py:948-980test_happy_path_posts_payload_and_parses_results 断言与文档契约矛盾

    • 该用例同时断言 payload["include_answer"] is False(第 979-980 行)和 "free-threaded" in res["summary"](第 966 行),但 _TAVILY_RESPONSE 固定带 answer 字段。文档明确“默认发送 include_answer=false,可通过 tavily_extra_params 开启”,即真实 Tavily 在 include_answer=false 时不会返回 answer。当前 mock 形状在生产中不可复现,无法防护“summary 在 include_answer=false 时被错误填充/读取”的回归。建议让该用例的 mock 不带 answer、并断言 summary == "",把 answer 填充的验证集中到 test_include_answer_fills_summary

💡 Suggestion

  • trpc_agent_sdk/tools/_webfetch_tool.py:628trpc_agent_sdk/tools/_websearch_tool.py:609payload.update(self._tavily_extra_params) 会覆盖受保护字段
    • tavily_extra_paramsurls/query/max_results/include_domains 等键之后合并,构造方误传这些键会静默覆盖已做过 SSRF/域名校验的值(例如 WebFetch 中 {"urls": [...]} 替换掉校验过的首跳 URL)。虽属构造期可信输入、非 LLM 可控,仍建议对保留键做保护性过滤或在文档中显式提示不要覆盖这些字段。

总结

整体改动结构清晰、错误路径与 SSRF/域名策略基本对齐,未发现阻塞级问题。主要风险是 Tavily 图片结果绕过域名策略、以及一处 mock 形状与真实 API 契约不一致的测试,建议合并前修复上述两条 Warning。

测试建议

  • 补充用例:allowed_domains 生效时,断言返回的 images 中不含非白名单域的 URL。
  • 补充用例:include_answer=False(默认)且 Tavily 响应不含 answer 时,summary 为空字符串。

@raychen911
raychen911 merged commit 5d44031 into trpc-group:main Jul 17, 2026
8 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