-
Notifications
You must be signed in to change notification settings - Fork 6.5k
Feat/serpex tool integration #20141
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.
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
Feat/serpex tool integration #20141
Conversation
- Add SerpexToolSpec for web search via SERPEX API - Support multiple search engines (Google, Bing, DuckDuckGo, Brave, Yahoo, Yandex) - Include time range filtering (day, week, month, year) - Add comprehensive unit tests with mocked API responses - Add integration tests for real API verification - Include documentation and usage examples - Format with black and pass ruff linting - API endpoint: https://api.serpex.dev/api/search - All tests passing (8 passed, 2 skipped)
4036c75 to
c2c8903
Compare
llama-index-integrations/tools/llama-index-tools-serpex/tests/test_serpex.py
Fixed
Show fixed
Hide fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks overall good, I just have some doubts about the implementation and some things I would change (see comments below 👇)
| results_list = data.get("results", []) | ||
|
|
||
| if not results_list: | ||
| return [Document(text="No search results found.")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would return an empty string, this could be misleading if someone just checked len(result) > 0 before returning to the user
|
|
||
| except requests.exceptions.RequestException as e: | ||
| error_msg = f"Error calling SERPEX API: {str(e)}" | ||
| return [Document(text=error_msg)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would raise the error and not return a list with one document (could be misleading for the same reason as above)
| # Add metadata | ||
| metadata = data.get("metadata", {}) | ||
| if metadata: | ||
| num_results = metadata.get("number_of_results", 0) | ||
| response_time = metadata.get("response_time", 0) | ||
| results_text += ( | ||
| f"\n\n(Found {num_results} results in {response_time}ms)" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't we just add these in the document metadata?
| results_text = f"Search results for '{query}':\n\n" | ||
| results_text += "\n".join(formatted_results) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a reason why we return a single document with all the results instead of multiple documents?
|
|
||
|
|
||
| # Integration test (requires real API key) | ||
| @pytest.mark.skip(reason="Requires real SERPEX API key") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put skipif here and verify the absence of the key before skipping? In CI here on GitHub there is no difference, but if one would want to test locally, this would be skipped whether or not the key is available in the environment
|
@AstraBert Thanks for the suggestions! I've made the changes you mentioned. Could you review it now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
llama-index-integrations/tools/llama-index-tools-serpex/tests/test_serpex.py
Fixed
Show fixed
Hide fixed
Head branch was pushed to by a user without write access
SERPEX Search Tool Integration - PR Description
Description
This PR adds SERPEX integration as a search tool for LlamaIndex, enabling AI agents and RAG applications to access real-time web search results from multiple search engines.
SERPEX provides a simple, fast API for web search results with support for Google, Bing, DuckDuckGo, Brave, Yahoo, and Yandex search engines.
Motivation
Changes Made
New Package:
llama-index-tools-serpexllama-index-integrations/tools/llama-index-tools-serpex/SerpexToolSpecclass for easy integrationFeatures:
Testing:
Code Quality:
blackruff(all checks passing)Documentation:
examples/serpex_example.pyFixes
N/A - This is a new feature addition.
New Package?
llama-index-tools-serpextool.llamahubsection inpyproject.tomlDetails:
llama-index-tools-serpexVersion Bump?
pyproject.tomlType of Change
How Has This Been Tested?
Test Results
Manual Verification
SerpexToolSpec()Checklist
Code Review
Documentation
examples/serpex_example.pyTesting
Formatting & Linting
uv run make format; uv run make lintto appease the lint godsLlamaHub Compliance
Files Changed
New Files
Statistics
Installation
After merge, users can install via:
Usage Example
API Details
Endpoint:
https://api.serpex.dev/api/searchMethod: GET
Parameters:
q(required): Search queryengine(optional): Search engine (auto, google, bing, duckduckgo, brave, yahoo, yandex)category(optional): Search category (web)time_range(optional): Time filter (all, day, week, month, year)Get API Key: https://serpex.dev/dashboard
Dependencies
llama-index-core>=0.14.0requests>=2.31.0No new external dependencies beyond what's already used in LlamaIndex.
Related Issues
N/A - This is a new feature.
Additional Notes
CC
@run-llama/maintainers
Thank you for reviewing this contribution! 🎉