fix(agent/tools): port AkShare to ToolBase so it works as an Agent tool - #16417
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization 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)
📝 WalkthroughWalkthrough
ChangesAkShare ToolBase Migration
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant AkShare
participant akshare
participant ToolOutput
Agent->>AkShare: _invoke(query=...)
AkShare->>AkShare: check_if_canceled()
AkShare->>akshare: stock_news_em(symbol)
akshare-->>AkShare: news DataFrame
AkShare->>ToolOutput: formalized_content / _ERROR
AkShare-->>Agent: formatted string
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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.
Actionable comments posted: 4
🧹 Nitpick comments (1)
test/unit_test/agent/component/test_akshare.py (1)
30-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for
_invokeoutput.These tests cover
AkShareParam, but the PR also fixes execution via_invoke. Mockakshare.stock_news_em(...), invoke the tool, and assert thatformalized_contentis set so the removed_run/be_outputpath cannot regress unnoticed.🤖 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 `@test/unit_test/agent/component/test_akshare.py` around lines 30 - 43, Add a regression test for AkShareParam._invoke to verify the execution path, not just meta and validation. Mock akshare.stock_news_em, call _invoke on the AkShare tool, and assert the returned result includes formalized_content so the removed _run/be_output flow cannot regress. Use the existing test_akshare.py coverage style and keep the test focused on the _invoke method’s output contract.
🤖 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.
Inline comments:
In `@agent/tools/akshare.py`:
- Around line 60-78: The AkShare invocation in _invoke still depends only on the
timeout decorator, which does not enforce a real deadline in normal mode, so the
external call can hang. Update the _invoke flow around ak.stock_news_em in the
AkShare tool to use a true transport/request timeout if the library supports
one, or change the timeout wrapper so it enforces execution deadlines even when
ENABLE_TIMEOUT_ASSERTION is not set. Keep the fix localized to AkShare._invoke
and the timeout usage around the ak.stock_news_em call.
- Around line 71-98: The AkShare retry loop in the processing method should not
pause after the final failed attempt; update the retry handling around the
for-loop in the AkShare tool so the sleep only happens when another retry
remains. Use the existing max_retries logic and the try/except block in the
method that calls ak.stock_news_em to gate
time.sleep(self._param.delay_after_error), while preserving cancellation checks
and the last_e capture for the eventual failure path.
- Around line 83-89: The HTML emitted in akshare.py is interpolating external
feed values directly in the item formatting, so sanitize them before building
formalized_content. Update the news-item rendering in the loop over
df.iterrows() to escape the title/content/source text and validate or restrict
the 新闻链接 value before placing it in the anchor href, keeping the existing
formatting in the same aggregation path.
- Around line 30-41: The ToolMeta definition in the AkShare tool metadata is
incomplete because it only sets name, description, and parameters, but omits
displayName and displayDescription. Update the metadata in the akshare tool
class so the self.meta object fully satisfies the ToolMeta contract and direct
consumers of param.meta can render it correctly. Keep the existing
akshare_stock_news identity and parameter schema, and add the missing display
fields alongside the current metadata keys.
---
Nitpick comments:
In `@test/unit_test/agent/component/test_akshare.py`:
- Around line 30-43: Add a regression test for AkShareParam._invoke to verify
the execution path, not just meta and validation. Mock akshare.stock_news_em,
call _invoke on the AkShare tool, and assert the returned result includes
formalized_content so the removed _run/be_output flow cannot regress. Use the
existing test_akshare.py coverage style and keep the test focused on the _invoke
method’s output contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3fb3a12f-11f8-4794-8f49-e1611ac078bb
📒 Files selected for processing (2)
agent/tools/akshare.pytest/unit_test/agent/component/test_akshare.py
| self.meta: ToolMeta = { | ||
| "name": "akshare_stock_news", | ||
| "description": "AkShare retrieves the latest news articles for a given Chinese A-share stock from East Money (东方财富).", | ||
| "parameters": { | ||
| "query": { | ||
| "type": "string", | ||
| "description": "The stock symbol/code to fetch news for, e.g. '600519'.", | ||
| "default": "{sys.query}", | ||
| "required": True, | ||
| } | ||
| }, | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Complete the ToolMeta contract.
ToolMeta includes displayName and displayDescription, but this metadata omits both. get_meta() currently works because it only reads a subset, but consumers that read param.meta directly can break or render incomplete descriptors.
Proposed fix
self.meta: ToolMeta = {
"name": "akshare_stock_news",
+ "displayName": "AkShare Stock News",
"description": "AkShare retrieves the latest news articles for a given Chinese A-share stock from East Money (东方财富).",
+ "displayDescription": "Retrieve latest East Money news for a Chinese A-share stock.",
"parameters": {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self.meta: ToolMeta = { | |
| "name": "akshare_stock_news", | |
| "description": "AkShare retrieves the latest news articles for a given Chinese A-share stock from East Money (东方财富).", | |
| "parameters": { | |
| "query": { | |
| "type": "string", | |
| "description": "The stock symbol/code to fetch news for, e.g. '600519'.", | |
| "default": "{sys.query}", | |
| "required": True, | |
| } | |
| }, | |
| } | |
| self.meta: ToolMeta = { | |
| "name": "akshare_stock_news", | |
| "displayName": "AkShare Stock News", | |
| "description": "AkShare retrieves the latest news articles for a given Chinese A-share stock from East Money (东方财富).", | |
| "displayDescription": "Retrieve latest East Money news for a Chinese A-share stock.", | |
| "parameters": { | |
| "query": { | |
| "type": "string", | |
| "description": "The stock symbol/code to fetch news for, e.g. '600519'.", | |
| "default": "{sys.query}", | |
| "required": True, | |
| } | |
| }, | |
| } |
🤖 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 `@agent/tools/akshare.py` around lines 30 - 41, The ToolMeta definition in the
AkShare tool metadata is incomplete because it only sets name, description, and
parameters, but omits displayName and displayDescription. Update the metadata in
the akshare tool class so the self.meta object fully satisfies the ToolMeta
contract and direct consumers of param.meta can render it correctly. Keep the
existing akshare_stock_news identity and parameter schema, and add the missing
display fields alongside the current metadata keys.
| for _ in range(self._param.max_retries + 1): | ||
| if self.check_if_canceled("AkShare processing"): | ||
| return | ||
|
|
||
| try: | ||
| import akshare as ak | ||
|
|
||
| df = ak.stock_news_em(symbol=symbol).head(self._param.top_n) | ||
|
|
||
| if self.check_if_canceled("AkShare processing"): | ||
| return | ||
|
|
||
| items = [ | ||
| '<a href="{}">{}</a>\n 新闻内容: {} \n发布时间:{} \n文章来源: {}'.format( | ||
| i["新闻链接"], i["新闻标题"], i["新闻内容"], i["发布时间"], i["文章来源"] | ||
| ) | ||
| for _, i in df.iterrows() | ||
| ] | ||
| res = "\n\n".join(items) | ||
| self.set_output("formalized_content", res) | ||
| return res | ||
| except Exception as e: | ||
| if self.check_if_canceled("AkShare processing"): | ||
| return | ||
|
|
||
| last_e = e | ||
| logging.exception(f"AkShare error: {e}") | ||
| time.sleep(self._param.delay_after_error) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Skip the delay after the final retry fails.
The loop sleeps even after the last failed attempt, delaying error reporting without another retry to wait for.
Proposed fix
- for _ in range(self._param.max_retries + 1):
+ for attempt in range(self._param.max_retries + 1):
if self.check_if_canceled("AkShare processing"):
return
@@
last_e = e
logging.exception(f"AkShare error: {e}")
- time.sleep(self._param.delay_after_error)
+ if attempt < self._param.max_retries:
+ time.sleep(self._param.delay_after_error)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for _ in range(self._param.max_retries + 1): | |
| if self.check_if_canceled("AkShare processing"): | |
| return | |
| try: | |
| import akshare as ak | |
| df = ak.stock_news_em(symbol=symbol).head(self._param.top_n) | |
| if self.check_if_canceled("AkShare processing"): | |
| return | |
| items = [ | |
| '<a href="{}">{}</a>\n 新闻内容: {} \n发布时间:{} \n文章来源: {}'.format( | |
| i["新闻链接"], i["新闻标题"], i["新闻内容"], i["发布时间"], i["文章来源"] | |
| ) | |
| for _, i in df.iterrows() | |
| ] | |
| res = "\n\n".join(items) | |
| self.set_output("formalized_content", res) | |
| return res | |
| except Exception as e: | |
| if self.check_if_canceled("AkShare processing"): | |
| return | |
| last_e = e | |
| logging.exception(f"AkShare error: {e}") | |
| time.sleep(self._param.delay_after_error) | |
| for attempt in range(self._param.max_retries + 1): | |
| if self.check_if_canceled("AkShare processing"): | |
| return | |
| try: | |
| import akshare as ak | |
| df = ak.stock_news_em(symbol=symbol).head(self._param.top_n) | |
| if self.check_if_canceled("AkShare processing"): | |
| return | |
| items = [ | |
| '<a href="{}">{}</a>\n 新闻内容: {} \n发布时间:{} \n文章来源: {}'.format( | |
| i["新闻链接"], i["新闻标题"], i["新闻内容"], i["发布时间"], i["文章来源"] | |
| ) | |
| for _, i in df.iterrows() | |
| ] | |
| res = "\n\n".join(items) | |
| self.set_output("formalized_content", res) | |
| return res | |
| except Exception as e: | |
| if self.check_if_canceled("AkShare processing"): | |
| return | |
| last_e = e | |
| logging.exception(f"AkShare error: {e}") | |
| if attempt < self._param.max_retries: | |
| time.sleep(self._param.delay_after_error) |
🤖 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 `@agent/tools/akshare.py` around lines 71 - 98, The AkShare retry loop in the
processing method should not pause after the final failed attempt; update the
retry handling around the for-loop in the AkShare tool so the sleep only happens
when another retry remains. Use the existing max_retries logic and the
try/except block in the method that calls ak.stock_news_em to gate
time.sleep(self._param.delay_after_error), while preserving cancellation checks
and the last_e capture for the eventual failure path.
| items = [ | ||
| '<a href="{}">{}</a>\n 新闻内容: {} \n发布时间:{} \n文章来源: {}'.format( | ||
| i["新闻链接"], i["新闻标题"], i["新闻内容"], i["发布时间"], i["文章来源"] | ||
| ) | ||
| for _, i in df.iterrows() | ||
| ] | ||
| res = "\n\n".join(items) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Escape external feed values before emitting HTML.
News title/content/source/link values come from an external provider and are interpolated directly into an HTML string. Escape text fields and restrict links before writing formalized_content.
Proposed fix
import logging
import os
import time
from abc import ABC
+from html import escape
+from urllib.parse import urlparse
from agent.tools.base import ToolMeta, ToolParamBase, ToolBase
from common.connection_utils import timeout+ def safe_href(value):
+ href = str(value or "")
+ parsed = urlparse(href)
+ if parsed.scheme not in {"http", "https"}:
+ return "#"
+ return escape(href, quote=True)
+
items = [
'<a href="{}">{}</a>\n 新闻内容: {} \n发布时间:{} \n文章来源: {}'.format(
- i["新闻链接"], i["新闻标题"], i["新闻内容"], i["发布时间"], i["文章来源"]
+ safe_href(i["新闻链接"]),
+ escape(str(i["新闻标题"]), quote=True),
+ escape(str(i["新闻内容"]), quote=True),
+ escape(str(i["发布时间"]), quote=True),
+ escape(str(i["文章来源"]), quote=True),
)
for _, i in df.iterrows()
]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| items = [ | |
| '<a href="{}">{}</a>\n 新闻内容: {} \n发布时间:{} \n文章来源: {}'.format( | |
| i["新闻链接"], i["新闻标题"], i["新闻内容"], i["发布时间"], i["文章来源"] | |
| ) | |
| for _, i in df.iterrows() | |
| ] | |
| res = "\n\n".join(items) | |
| def safe_href(value): | |
| href = str(value or "") | |
| parsed = urlparse(href) | |
| if parsed.scheme not in {"http", "https"}: | |
| return "#" | |
| return escape(href, quote=True) | |
| items = [ | |
| '<a href="{}">{}</a>\n 新闻内容: {} \n发布时间:{} \n文章来源: {}'.format( | |
| safe_href(i["新闻链接"]), | |
| escape(str(i["新闻标题"]), quote=True), | |
| escape(str(i["新闻内容"]), quote=True), | |
| escape(str(i["发布时间"]), quote=True), | |
| escape(str(i["文章来源"]), quote=True), | |
| ) | |
| for _, i in df.iterrows() | |
| ] | |
| res = "\n\n".join(items) |
🤖 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 `@agent/tools/akshare.py` around lines 83 - 89, The HTML emitted in akshare.py
is interpolating external feed values directly in the item formatting, so
sanitize them before building formalized_content. Update the news-item rendering
in the loop over df.iterrows() to escape the title/content/source text and
validate or restrict the 新闻链接 value before placing it in the anchor href,
keeping the existing formatting in the same aggregation path.
yuzhichang
left a comment
There was a problem hiding this comment.
Thanks for the ToolBase port. I re-reviewed the current head and the important regression is fixed: AkShare now has ToolMeta, a working _invoke path, and no longer depends on the removed legacy _run / be_output flow.
I did not find a merge-blocking bug in the runtime code itself. The main gap I still see is test coverage: the new tests only validate param construction and metadata. Please add one focused regression test that mocks akshare.stock_news_em(...), calls _invoke(query=...), and asserts the restored execution path returns content and updates formalized_content. That would make this much safer to merge.
yuzhichang
left a comment
There was a problem hiding this comment.
I re-reviewed the branch locally. The important agent-runtime regression is fixed: AkShare now exposes ToolMeta, implements _invoke, and no longer depends on the removed legacy _run / be_output flow. I did not find a merge-blocking correctness issue in the implementation itself.
The main remaining gap is coverage for the restored execution path. Please add a focused regression test that mocks akshare.stock_news_em(...), calls _invoke(query=...), and asserts the returned content plus the formalized_content output. Right now the tests only validate param construction/metadata, so they would not catch a future break in the runtime path.
Address review feedback on infiniflow#16417: the tests only covered param construction/metadata. Add a focused regression test that mocks akshare.stock_news_em(...), calls _invoke(query=...), and asserts the returned content and the formalized_content output (and that top_n is applied); plus an empty-query short-circuit that does not call akshare.
|
Thanks @yuzhichang — added the With a mocked
Full file (6 tests) passes locally. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@test/unit_test/agent/component/test_akshare.py`:
- Around line 95-99: Strengthen test_invoke_empty_query_returns_empty so it
verifies the short-circuit in _make_tool()/_invoke() does not call
akshare.stock_news_em at all when query is empty. Add a mock/spy around the
akshare call used by the tool and assert it is never invoked, while still
checking the returned empty string and formalized_content output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 86ec5894-796b-4b98-baa8-31d2982b64c3
📒 Files selected for processing (1)
test/unit_test/agent/component/test_akshare.py
| def test_invoke_empty_query_returns_empty(): | ||
| # Empty query short-circuits without calling akshare. | ||
| tool, out = _make_tool() | ||
| assert tool._invoke(query="") == "" | ||
| assert out.get("formalized_content") == "" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that empty queries never reach akshare.
This test only checks the returned/output values. A regression that still calls akshare.stock_news_em("") before returning "" would slip through, even though "no akshare call" is part of the behavior this PR claims to lock down.
Proposed test hardening
-def test_invoke_empty_query_returns_empty():
+def test_invoke_empty_query_returns_empty(monkeypatch):
# Empty query short-circuits without calling akshare.
+ pytest.importorskip("akshare")
+ import akshare
+
+ monkeypatch.setattr(
+ akshare,
+ "stock_news_em",
+ lambda *args, **kwargs: pytest.fail("stock_news_em should not be called for empty queries"),
+ )
+
tool, out = _make_tool()
assert tool._invoke(query="") == ""
assert out.get("formalized_content") == ""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_invoke_empty_query_returns_empty(): | |
| # Empty query short-circuits without calling akshare. | |
| tool, out = _make_tool() | |
| assert tool._invoke(query="") == "" | |
| assert out.get("formalized_content") == "" | |
| def test_invoke_empty_query_returns_empty(monkeypatch): | |
| # Empty query short-circuits without calling akshare. | |
| pytest.importorskip("akshare") | |
| import akshare | |
| monkeypatch.setattr( | |
| akshare, | |
| "stock_news_em", | |
| lambda *args, **kwargs: pytest.fail("stock_news_em should not be called for empty queries"), | |
| ) | |
| tool, out = _make_tool() | |
| assert tool._invoke(query="") == "" | |
| assert out.get("formalized_content") == "" |
🤖 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 `@test/unit_test/agent/component/test_akshare.py` around lines 95 - 99,
Strengthen test_invoke_empty_query_returns_empty so it verifies the
short-circuit in _make_tool()/_invoke() does not call akshare.stock_news_em at
all when query is empty. Add a mock/spy around the akshare call used by the tool
and assert it is never invoked, while still checking the returned empty string
and formalized_content output.
78db4e9 to
f86a0e7
Compare
AkShare was never ported to the modern ToolBase/_invoke interface during
the agent module redesign and was still written against the removed
legacy _run/be_output API, leaving it non-functional:
- AkShare extended ComponentBase (not ToolBase) and AkShareParam defined
no `meta`, so it had no get_meta(). agent_with_tools builds each tool's
descriptor via cpn.get_meta(), so adding AkShare to an Agent raised
AttributeError: 'AkShare' object has no attribute 'get_meta'.
- invoke() dispatches to _invoke, but AkShare only implemented the legacy
_run, so _invoke fell through to ComponentBase._invoke ->
NotImplementedError. _run also called the removed be_output.
Port AkShareParam to ToolParamBase with a ToolMeta exposing a required
`query` parameter (the stock symbol, default {sys.query}), and rewrite the
component with _invoke/set_output("formalized_content", ...) while keeping
top_n. Add regression tests covering param construction, validation, and
the tool descriptor.
Closes infiniflow#16416
Address review feedback on infiniflow#16417: the tests only covered param construction/metadata. Add a focused regression test that mocks akshare.stock_news_em(...), calls _invoke(query=...), and asserts the returned content and the formalized_content output (and that top_n is applied); plus an empty-query short-circuit that does not call akshare.
8466af7 to
618b364
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16417 +/- ##
=======================================
Coverage 93.16% 93.16%
=======================================
Files 10 10
Lines 717 717
Branches 118 118
=======================================
Hits 668 668
Misses 29 29
Partials 20 20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ol (infiniflow#16417) ### What problem does this PR solve? Closes infiniflow#16416. The **AkShare** agent tool (`agent/tools/akshare.py`) was never ported to the modern `ToolBase`/`_invoke` interface during the agent module redesign and was still written against the removed legacy `_run`/`be_output` API, so it was non-functional: 1. **Adding it to an Agent raised `AttributeError`.** `AkShare` extended `ComponentBase` (not `ToolBase`) and `AkShareParam` defined no `meta`, so it had no `get_meta()`. `agent/component/agent_with_tools.py` builds each tool's function descriptor via `cpn.get_meta()`, so constructing an Agent that includes the AkShare tool raised `AttributeError: 'AkShare' object has no attribute 'get_meta'`. 2. **It could never run.** `invoke()` dispatches to `self._invoke`, but `AkShare` only implemented the legacy `_run`, so `_invoke` fell through to `ComponentBase._invoke` → `NotImplementedError`. `_run` also called `be_output(...)`, which no longer exists on the base classes. ### Type of change - [x] Bug Fix (non-breaking change which fixes an issue) ### Changes - Port `AkShareParam` to `ToolParamBase` with a `ToolMeta` (defined before `super().__init__()`, matching `ArXivParam`/`TavilyExtractParam`) exposing a required `query` parameter — the stock symbol to look up, default `{sys.query}`. `query` matches the `{sys.query}` convention shared by the other tools. - Rewrite the component with `_invoke`/`set_output("formalized_content", ...)` (errors surfaced via `_ERROR`), keeping `top_n` and importing `akshare` lazily. - Add regression tests (`test/unit_test/agent/component/test_akshare.py`) covering param construction, validation, and the tool descriptor. Same class of defect as infiniflow#16329 (DeepL) and infiniflow#16414 (Crawler). Backend-only; no frontend changes. --------- Co-authored-by: Zhichang Yu <yuzhichang@gmail.com>
…ol (infiniflow#16417) ### What problem does this PR solve? Closes infiniflow#16416. The **AkShare** agent tool (`agent/tools/akshare.py`) was never ported to the modern `ToolBase`/`_invoke` interface during the agent module redesign and was still written against the removed legacy `_run`/`be_output` API, so it was non-functional: 1. **Adding it to an Agent raised `AttributeError`.** `AkShare` extended `ComponentBase` (not `ToolBase`) and `AkShareParam` defined no `meta`, so it had no `get_meta()`. `agent/component/agent_with_tools.py` builds each tool's function descriptor via `cpn.get_meta()`, so constructing an Agent that includes the AkShare tool raised `AttributeError: 'AkShare' object has no attribute 'get_meta'`. 2. **It could never run.** `invoke()` dispatches to `self._invoke`, but `AkShare` only implemented the legacy `_run`, so `_invoke` fell through to `ComponentBase._invoke` → `NotImplementedError`. `_run` also called `be_output(...)`, which no longer exists on the base classes. ### Type of change - [x] Bug Fix (non-breaking change which fixes an issue) ### Changes - Port `AkShareParam` to `ToolParamBase` with a `ToolMeta` (defined before `super().__init__()`, matching `ArXivParam`/`TavilyExtractParam`) exposing a required `query` parameter — the stock symbol to look up, default `{sys.query}`. `query` matches the `{sys.query}` convention shared by the other tools. - Rewrite the component with `_invoke`/`set_output("formalized_content", ...)` (errors surfaced via `_ERROR`), keeping `top_n` and importing `akshare` lazily. - Add regression tests (`test/unit_test/agent/component/test_akshare.py`) covering param construction, validation, and the tool descriptor. Same class of defect as infiniflow#16329 (DeepL) and infiniflow#16414 (Crawler). Backend-only; no frontend changes. --------- Co-authored-by: Zhichang Yu <yuzhichang@gmail.com>
…16692) ### Summary Port the **QWeather** agent tool to the modern `ToolBase` / `_invoke` interface. It was still written against the removed legacy `ComponentBase` / `_run` / `be_output` API, so it was non-functional as an Agent tool — adding it to an Agent raised `AttributeError` because it had no `get_meta()`. This is the same defect that was fixed for the AkShare tool in #16417. **Changes** - `QWeatherParam` now extends `ToolParamBase` with a `meta` exposing a `query` (location) parameter, and adds `get_input_form()`. Existing config (`web_apikey`, `lang`, `type`, `user_type`, `time_period`) is preserved. - `QWeather` now extends `ToolBase` and implements `_invoke(**kwargs)` with the standard retry loop, cancellation checks, `set_output("formalized_content", ...)`, and `thoughts()`. The weather / indices / air-quality branches and the API error-code messages are kept. - Added `test/unit_test/agent/component/test_qweather.py` covering the restored `meta`, param validation, the weather-now and multi-day and indices branches, the empty-query short-circuit, and the location-lookup error message. **Testing** - `ruff check agent/tools/qweather.py test/unit_test/agent/component/test_qweather.py` — clean - `ruff format --check` — clean - `pytest test/unit_test/agent/component/test_qweather.py`
What problem does this PR solve?
Closes #16416.
The AkShare agent tool (
agent/tools/akshare.py) was never ported to the modernToolBase/_invokeinterface during the agent module redesign and was still written against the removed legacy_run/be_outputAPI, so it was non-functional:AttributeError.AkShareextendedComponentBase(notToolBase) andAkShareParamdefined nometa, so it had noget_meta().agent/component/agent_with_tools.pybuilds each tool's function descriptor viacpn.get_meta(), so constructing an Agent that includes the AkShare tool raisedAttributeError: 'AkShare' object has no attribute 'get_meta'.invoke()dispatches toself._invoke, butAkShareonly implemented the legacy_run, so_invokefell through toComponentBase._invoke→NotImplementedError._runalso calledbe_output(...), which no longer exists on the base classes.Type of change
Changes
AkShareParamtoToolParamBasewith aToolMeta(defined beforesuper().__init__(), matchingArXivParam/TavilyExtractParam) exposing a requiredqueryparameter — the stock symbol to look up, default{sys.query}.querymatches the{sys.query}convention shared by the other tools._invoke/set_output("formalized_content", ...)(errors surfaced via_ERROR), keepingtop_nand importingaksharelazily.test/unit_test/agent/component/test_akshare.py) covering param construction, validation, and the tool descriptor.Same class of defect as #16329 (DeepL) and #16414 (Crawler). Backend-only; no frontend changes.