Skip to content

fix(google_search): AsyncGoogleSearch.run drops k, so every successful search raises TypeError - #372

Open
Anai-Guo wants to merge 1 commit into
InternLM:v1.0.0from
Anai-Guo:fix-async-google-search-k
Open

Anai-Guo wants to merge 1 commit into
InternLM:v1.0.0from
Anai-Guo:fix-async-google-search-k

Conversation

@Anai-Guo

Copy link
Copy Markdown

The bug

AsyncGoogleSearch does not define _parse_results; it inherits it from GoogleSearch:

MRO: ['AsyncGoogleSearch', 'AsyncActionMixin', 'GoogleSearch', 'BaseAction', 'object']
owner    : GoogleSearch._parse_results
signature: (self, results: dict, k: int) -> Union[str, List[str]]

The synchronous path passes both arguments, the async path passes only one:

call site code
GoogleSearch.run, google_search.py:72 parsed_res = self._parse_results(response, k)
AsyncGoogleSearch.run, google_search.py:199 parsed_res = self._parse_results(response)

k has no default, so every successful (HTTP 200) AsyncGoogleSearch call raises TypeError instead of returning results. The status_code == 200 branch is the only branch that reaches this line, so the async action never succeeds — the error paths (-1, other codes) are the only ones that return normally.

AsyncArxivSearch, the repo's other AsyncActionMixin search subclass, forwards its parameters correctly (return super().get_arxiv_article_information(query)), and all 16 self._parse_response(...) call sites in web_browser.py match their 1-argument definitions. AsyncGoogleSearch.run is the only mismatched site in lagent/.

The fix

One line — pass k, exactly as the synchronous sibling does.

Verification

Ran on the real module (loaded from lagent/actions/google_search.py, with _search stubbed to return (200, {...}) so no network/API key is needed), before and after the patch:

=== UNPATCHED (v1.0.0 @ faaf42b) ===
  TypeError raised out of AsyncGoogleSearch.run():
  GoogleSearch._parse_results() missing 1 required positional argument: 'k'

=== PATCHED ===
  state=0 (SUCCESS) errmsg=None
  result=[{'type': 'text', 'content': "['snippet-0', 'snippet-1', 'snippet-2']"}]

The patched run was driven with k=3 and returns exactly 3 snippets, so k is honoured rather than merely accepted.

Also replayed both call sites through inspect.Signature.bind against the real inherited signature:

sync  line 72 : bind OK
async line 199: bind FAILS: missing a required argument: 'k'

Base branch

Targeting v1.0.0 since that is where recent work has been merging (#365, #366, #367). The same line is identical on main — happy to retarget or open a companion PR if you would rather take it there.

🤖 Generated with Claude Code

AsyncGoogleSearch inherits GoogleSearch._parse_results(self, results, k),
but AsyncGoogleSearch.run called it with only `results`, so every HTTP 200
raised TypeError instead of returning the parsed snippets. The synchronous
GoogleSearch.run already passes k; this makes the async path match.
@Anai-Guo

Copy link
Copy Markdown
Author

Note on CI: the docs/readthedocs.org:lagent-cn check fails here, but that is pre-existing on the v1.0.0 base, not caused by this change — #368 (also based on v1.0.0, and touching no docs) shows the identical failure, while #370 and #371 (based on main) both pass the same check. This PR only edits lagent/actions/google_search.py and adds no docs.

🤖 Generated with Claude Code

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.

1 participant