From 63ef515c5aed89667a434aa1925ea832a2a91398 Mon Sep 17 00:00:00 2001 From: Gabriel Costa Date: Sun, 21 Jun 2026 21:45:53 -0700 Subject: [PATCH 1/6] feat(paper-search): add SerpAPI and SearchAPI provider support Add a 'provider' config field to the Google Scholar paper search tool so developers can choose between Serper (default), SerpAPI, or SearchAPI as the backend. All three query Google Scholar and return results normalized to the same shape (title, year, snippet, link, publication info, citations), so the agent-facing tool behavior is identical regardless of provider. Changes: - paper_search.py: add PaperSearchProvider StrEnum, provider dispatch in search(), _search_serpapi/_search_searchapi clients, _normalize_serpapi/ _normalize_searchapi mappers, _extract_year regex for publication strings, _parse_year_range shared helper - register.py: add provider/serpapi_api_key/searchapi_api_key config fields, _PROVIDER_KEY_INFO dispatch table, _resolve_api_key helper, provider-aware stub error messages - pyproject.toml: bump 1.0.0 -> 1.1.0, fix httpx -> aiohttp dep (code already used aiohttp), update description - tests: 21 -> 58 tests covering provider init, year extraction, normalization, fetch payload construction, pagination (incl. SearchAPI 1-based page), provider dispatch, and end-to-end search per provider - docs: update source README, root README API keys table, deploy/.env.example with all three providers Fully backward compatible: provider defaults to 'serper', serper_api_key field and SERPER_API_KEY env var path unchanged, existing configs work without modification. Signed-off-by: Gabriel Costa --- README.md | 15 +- deploy/.env.example | 8 +- sources/google_scholar_paper_search/README.md | 66 ++- .../pyproject.toml | 6 +- .../src/paper_search.py | 408 ++++++++++++--- .../src/register.py | 76 ++- .../tests/conftest.py | 101 +++- .../tests/test_paper_search.py | 463 ++++++++++++++++++ uv.lock | 6 +- 9 files changed, 1017 insertions(+), 132 deletions(-) diff --git a/README.md b/README.md index c77490406..89cefaf9f 100644 --- a/README.md +++ b/README.md @@ -106,7 +106,7 @@ This project is for: **Optional requirements:** - Tavily API key (for web search functionality) -- Serper API key (for academic paper search functionality) +- A paper search API key for one of the supported providers: Serper (`SERPER_API_KEY`), SerpAPI (`SERPAPI_API_KEY`), or SearchAPI (`SEARCHAPI_API_KEY`) > **Note:** Configure at least one data source (Tavily web search, Serper search tool, or knowledge layer) to enable research functionality. @@ -206,10 +206,17 @@ uv pip install -e "./sources/knowledge_layer[llamaindex,foundational_rag]" 2. Navigate to your dashboard 3. Generate an API key -#### Obtain a Serper API Key +#### Obtain a Paper Search API Key -1. Sign in to [Serper](https://serper.dev/) -2. Generate an API key from your dashboard +Paper search supports three interchangeable providers. Set the `provider` field on the `paper_search` function in your workflow config (defaults to `serper`): + +| Provider | Environment Variable | Sign-up | +|----------|----------------------|---------| +| Serper (default) | `SERPER_API_KEY` | [serper.dev](https://serper.dev/) | +| SerpAPI | `SERPAPI_API_KEY` | [serpapi.com](https://serpapi.com/) | +| SearchAPI | `SEARCHAPI_API_KEY` | [searchapi.io](https://www.searchapi.io/) | + +See [sources/google_scholar_paper_search/README.md](sources/google_scholar_paper_search/README.md) for configuration details. ### Set Up Environment Variables diff --git a/deploy/.env.example b/deploy/.env.example index 03b5d7130..89a91e4b1 100644 --- a/deploy/.env.example +++ b/deploy/.env.example @@ -20,8 +20,12 @@ NVIDIA_API_KEY= # Web search (Required) TAVILY_API_KEY= -# Paper search (Optional) -# SERPER_API_KEY= # to enable, set API key and update the relevant config in configs/ directory +# Paper search (Optional — choose one provider) +# The provider is selected in your workflow config via the `provider` field +# on the paper_search function (defaults to 'serper'). +# SERPER_API_KEY= # provider: serper — https://serper.dev/ +# SERPAPI_API_KEY= # provider: serpapi — https://serpapi.com/ +# SEARCHAPI_API_KEY= # provider: searchapi — https://www.searchapi.io/ # ----------------------------------------------------------------------------- diff --git a/sources/google_scholar_paper_search/README.md b/sources/google_scholar_paper_search/README.md index 92a46c794..360b8f653 100644 --- a/sources/google_scholar_paper_search/README.md +++ b/sources/google_scholar_paper_search/README.md @@ -1,17 +1,29 @@ # Google Scholar Paper Search -A NeMo Agent Toolkit function that searches for academic papers using Google Scholar through the Serper API. +A NeMo Agent Toolkit function that searches for academic papers using Google Scholar. You can choose between three backend providers: + +| Provider | `_type` value | Env var | Sign-up | +|----------|---------------|---------|---------| +| **Serper** (default) | `serper` | `SERPER_API_KEY` | [serper.dev](https://serper.dev/) | +| **SerpAPI** | `serpapi` | `SERPAPI_API_KEY` | [serpapi.com](https://serpapi.com/) | +| **SearchAPI** | `searchapi` | `SEARCHAPI_API_KEY` | [searchapi.io](https://www.searchapi.io/) | + +All three query Google Scholar and return the same normalized result shape (title, year, snippet, link, publication info, citations), so the agent-facing tool behavior is identical regardless of provider. ## Prerequisites -Before using this function, you need a Serper API key: +You need an API key for **one** of the supported providers. The default is Serper. -1. Go to [serper.dev](https://serper.dev/) and create an account -2. Generate an API key from your dashboard -3. Add the API key to your `deploy/.env` file in the project root: +1. Create an account at your chosen provider (see table above) +2. Generate an API key from the dashboard +3. Add the key to your `deploy/.env` file in the project root, for example: ```bash SERPER_API_KEY="your-serper-api-key" +# OR +SERPAPI_API_KEY="your-serpapi-api-key" +# OR +SEARCHAPI_API_KEY="your-searchapi-api-key" ``` Alternatively, you can provide the API key directly in the configuration file (see below). @@ -34,24 +46,48 @@ nat info components -t function | grep paper_search ### Adding the Function -Add the `paper_search` function to the `functions` section of your workflow configuration file: +Add the `paper_search` function to the `functions` section of your workflow configuration file. Use the `provider` field to select the backend (defaults to `serper`): ```yaml functions: paper_search_tool: _type: paper_search + provider: serper # 'serper' (default), 'serpapi', or 'searchapi' max_results: 10 timeout: 30 serper_api_key: ${SERPER_API_KEY} ``` +To use SerpAPI instead: + +```yaml +functions: + paper_search_tool: + _type: paper_search + provider: serpapi + serpapi_api_key: ${SERPAPI_API_KEY} +``` + +To use SearchAPI: + +```yaml +functions: + paper_search_tool: + _type: paper_search + provider: searchapi + searchapi_api_key: ${SEARCHAPI_API_KEY} +``` + #### Configuration Options | Parameter | Type | Default | Description | |-----------|------|---------|-------------| +| `provider` | string | `serper` | Backend provider: `serper`, `serpapi`, or `searchapi` | | `max_results` | integer | 10 | Maximum number of search results to return (capped at 50) | | `timeout` | integer | 30 | Timeout in seconds for search requests | -| `serper_api_key` | string | None | Serper API key (can also be set through the `SERPER_API_KEY` environment variable) | +| `serper_api_key` | string | None | Serper API key (required when `provider: serper`; also read from `SERPER_API_KEY` env var) | +| `serpapi_api_key` | string | None | SerpAPI key (required when `provider: serpapi`; also read from `SERPAPI_API_KEY` env var) | +| `searchapi_api_key` | string | None | SearchAPI key (required when `provider: searchapi`; also read from `SEARCHAPI_API_KEY` env var) | ### Adding as a Tool to an Agent @@ -74,7 +110,7 @@ functions: ### Complete Example -Here is a complete configuration example showing how to integrate the paper search tool: +Here is a complete configuration example showing how to integrate the paper search tool with SerpAPI as the provider: ```yaml llms: @@ -87,8 +123,9 @@ llms: functions: paper_search_tool: _type: paper_search + provider: serpapi max_results: 10 - serper_api_key: ${SERPER_API_KEY} + serpapi_api_key: ${SERPAPI_API_KEY} web_search_tool: _type: tavily_internet_search @@ -115,7 +152,6 @@ The paper search function accepts the following arguments when called by an agen |----------|------|----------|-------------| | `query` | string | Yes | The search query for finding academic papers | | `year` | string | No | Year or year range filter (for example, "2023" or "2020-2023") | -| `source` | string | No | Search source (defaults to "serper") | ## Troubleshooting @@ -125,8 +161,9 @@ The paper search function accepts the following arguments when called by an agen If you see an error about the API key not being found: -- Verify the `SERPER_API_KEY` environment variable is set correctly -- Alternatively, ensure the `serper_api_key` is specified in the configuration file +- Verify the correct environment variable is set for your chosen `provider` (`SERPER_API_KEY`, `SERPAPI_API_KEY`, or `SEARCHAPI_API_KEY`) +- Alternatively, ensure the matching key field is specified in the configuration file +- Make sure the `provider` field matches the key you provided **Request timeout** @@ -141,11 +178,11 @@ If no papers are found: - Try a broader search query - Remove year filters to expand the search range -- Verify your Serper API key has available quota +- Verify your provider API key has available quota ## Disabling Paper Search -If you don't have a Serper API key or don't need paper search functionality, you can disable it by removing the tool from your configuration: +If you don't have a provider API key or don't need paper search functionality, you can disable it by removing the tool from your configuration: ### Remove from Configuration @@ -156,6 +193,7 @@ functions: # Remove or comment out this section # paper_search_tool: # _type: paper_search + # provider: serper # max_results: 5 # serper_api_key: ${SERPER_API_KEY} ``` diff --git a/sources/google_scholar_paper_search/pyproject.toml b/sources/google_scholar_paper_search/pyproject.toml index 5505df22b..2920306e8 100644 --- a/sources/google_scholar_paper_search/pyproject.toml +++ b/sources/google_scholar_paper_search/pyproject.toml @@ -23,13 +23,13 @@ package-dir = {"google_scholar_paper_search" = "src"} [project] name = "google-scholar-paper-search" -version = "1.0.0" -description = "NAT-based Google Scholar paper search tool using Serper (Google Scholar)" +version = "1.1.0" +description = "NAT-based Google Scholar paper search tool supporting Serper, SerpAPI, and SearchAPI backends" readme = "README.md" requires-python = ">=3.11,<3.14" license = {text = "Apache-2.0"} dependencies = [ - "httpx>=0.24.0", + "aiohttp>=3.8.0", "pydantic>=2.0.0", ] diff --git a/sources/google_scholar_paper_search/src/paper_search.py b/sources/google_scholar_paper_search/src/paper_search.py index 0a4e4d3c6..3af37820f 100644 --- a/sources/google_scholar_paper_search/src/paper_search.py +++ b/sources/google_scholar_paper_search/src/paper_search.py @@ -12,14 +12,18 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -"""Paper search tool using Serper (Google Scholar). +"""Paper search tool using Google Scholar via Serper, SerpAPI, or SearchAPI. -This module contains the NAT-independent PaperSearchTool class. +This module contains the NAT-independent PaperSearchTool class. The provider +is selected at construction time; each provider's raw response is normalized +into a common shape consumed by ``format_results``. """ import asyncio import logging import math +import re +from enum import StrEnum from typing import Any import aiohttp @@ -27,42 +31,188 @@ logger = logging.getLogger(__name__) SERPER_API_URL = "https://google.serper.dev/scholar" +SERPAPI_API_URL = "https://serpapi.com/search" +SEARCHAPI_API_URL = "https://www.searchapi.io/api/v1/search" + +# Matches a 4-digit year (19xx or 20xx) inside a publication summary string. +_YEAR_RE = re.compile(r"\b(19\d{2}|20\d{2})\b") + + +class PaperSearchProvider(StrEnum): + """Supported Google Scholar search providers.""" + + SERPER = "serper" + SERPAPI = "serpapi" + SEARCHAPI = "searchapi" class PaperSearchTool: - """ - Paper search tool for academic papers using Google Scholar (Serper). + """Paper search tool for academic papers using Google Scholar. + + The ``provider`` argument selects which backend API to use. All providers + return results normalized to the same shape so ``format_results`` and the + rest of the pipeline are provider-agnostic. This class is NAT-independent and receives all dependencies via constructor. Example: + >>> # Serper (default, backward compatible) + >>> tool = PaperSearchTool(serper_api_key="your-key") + >>> + >>> # SerpAPI + >>> tool = PaperSearchTool( + ... provider="serpapi", + ... serpapi_api_key="your-key", + ... ) + >>> + >>> # SearchAPI >>> tool = PaperSearchTool( - ... serper_api_key="your-api-key", - ... timeout=30, - ... max_results=10, + ... provider="searchapi", + ... searchapi_api_key="your-key", ... ) >>> result = await tool.search("machine learning transformers") """ def __init__( self, - serper_api_key: str, + serper_api_key: str | None = None, *, + provider: str | PaperSearchProvider = PaperSearchProvider.SERPER, + serpapi_api_key: str | None = None, + searchapi_api_key: str | None = None, timeout: int = 30, max_results: int = 10, ) -> None: - """ - Initialize the paper search tool. + """Initialize the paper search tool. Args: - serper_api_key: API key for Serper (Google Scholar). + serper_api_key: API key for Serper. Kept as the first positional + argument for backward compatibility; required when + ``provider="serper"``. + provider: Which backend to use — ``"serper"``, ``"serpapi"``, or + ``"searchapi"``. Defaults to ``"serper"``. + serpapi_api_key: API key for SerpAPI (required when provider is serpapi). + searchapi_api_key: API key for SearchAPI (required when provider is searchapi). timeout: Timeout in seconds for search requests (default 30). max_results: Maximum number of search results to return (default 10). """ + self.provider = PaperSearchProvider(provider) self.serper_api_key = serper_api_key + self.serpapi_api_key = serpapi_api_key + self.searchapi_api_key = searchapi_api_key self.timeout = timeout self.max_results = max_results + # ── Public API ── + async def search( + self, + query: str, + year: str | None = None, + ) -> str: + """Search for peer-reviewed academic papers and scientific publications. + + This method returns papers from Google Scholar with citations, abstracts, + and links for research queries requiring authoritative, scholarly sources + including: scientific concepts, algorithms, methodologies, technical + foundations, theoretical frameworks, empirical studies, and peer-reviewed + evidence. + + Args: + query: The search query string. + year: Optional year or year range (e.g., "2023" or "2020-2023"). + + Returns: + Formatted string with search results. + """ + if not query: + return "Error: 'query' argument is required" + + if year is not None and not isinstance(year, str): + year = str(year) + + logger.info(f"Paper search ({self.provider.value}) for: {query}") + + try: + if self.provider is PaperSearchProvider.SERPER: + results = await self._search_serper(query, year, self.max_results) + elif self.provider is PaperSearchProvider.SERPAPI: + results = await self._search_serpapi(query, year, self.max_results) + elif self.provider is PaperSearchProvider.SEARCHAPI: + results = await self._search_searchapi(query, year, self.max_results) + else: # pragma: no cover - exhausted by enum + raise ValueError(f"Unsupported provider: {self.provider}") + return self.format_results(results) + + except TimeoutError: + logger.error(f"Paper search timed out for query: {query}") + return f"Paper search timed out after {self.timeout}s for query: {query}" + except Exception as e: + logger.error(f"Paper search failed: {e}") + return f"Paper search failed: {str(e)}" + + @staticmethod + def format_results(results: list[dict[str, Any]]) -> str: + """Format normalized Google Scholar results. + + Expects each dict to have the keys: ``title``, ``year``, ``snippet``, + ``link``, ``publicationInfo``, ``citedBy``. All provider responses are + normalized to this shape before reaching here. + """ + if not results: + return "No papers found via Google Scholar." + + formatted_papers = [] + for i, paper in enumerate(results, 1): + title = paper.get("title", "Unknown Title") + year = paper.get("year", "Unknown Year") + snippet = paper.get("snippet", "") + link = paper.get("link", "") + pub_info = paper.get("publicationInfo", "") + citations = paper.get("citedBy", 0) + + paper_str = ( + f"{i}. **{title}** ({year})\n" + f" - **Publication**: {pub_info}\n" + f" - **Citations**: {citations}\n" + f" - **Snippet**: {snippet}\n" + f" - **Link**: {link}" + ) + formatted_papers.append(paper_str) + + return "\n\n".join(formatted_papers) + + # ── Shared helpers ── + @staticmethod + def _parse_year_range(year: str | None) -> tuple[str | None, str | None]: + """Parse a year argument into a ``(start_year, end_year)`` tuple. + + Accepts a single year (``"2023"``) or a range (``"2020-2023"``, + ``"-2023"``, ``"2020-"``). + """ + if not year: + return None, None + if "-" in year: + parts = year.split("-") + if len(parts) == 2: + start_year = parts[0] if parts[0] else None + end_year = parts[1] if parts[1] else None + return start_year, end_year + return year, year + + @staticmethod + def _extract_year(publication_info: Any) -> str: + """Extract a 4-digit year from a publication summary string. + + SerpAPI and SearchAPI embed the year inside the publication summary + (e.g. ``"JL Harper - ..., 1977 - cabdirect.org"``). Serper returns + a clean ``year`` field, so this is only used for the other two. + """ + if not isinstance(publication_info, str): + return "Unknown Year" + match = _YEAR_RE.search(publication_info) + return match.group(1) if match else "Unknown Year" + + # ── Serper (POST, X-API-KEY header) ── async def _fetch_serper_page( self, query: str, @@ -71,10 +221,9 @@ async def _fetch_serper_page( start_year: str | None, end_year: str | None, ) -> dict[str, Any]: - """Fetch a single page from Serper.""" payload: dict[str, Any] = { "q": query, - "num": min(num, 20), # API limit per request + "num": min(num, 20), "start": offset, } @@ -84,7 +233,7 @@ async def _fetch_serper_page( payload["as_yhi"] = end_year headers = { - "X-API-KEY": self.serper_api_key, + "X-API-KEY": self.serper_api_key or "", "Content-Type": "application/json", } @@ -106,23 +255,16 @@ async def _search_serper( year: str | None = None, limit: int = 10, ) -> list[dict[str, Any]]: - """Perform search using Serper (Google Scholar).""" - start_year = None - end_year = None - if year: - if "-" in year: - parts = year.split("-") - if len(parts) == 2: - start_year = parts[0] if parts[0] else None - end_year = parts[1] if parts[1] else None - else: - start_year = year - end_year = year - - # Limit max results for Serper (cost control and API limits) + """Perform search using Serper (Google Scholar). + + Serper already returns the normalized shape (``title``, ``year``, + ``snippet``, ``link``, ``publicationInfo``, ``citedBy``), so results + are returned as-is. + """ + start_year, end_year = self._parse_year_range(year) + limit = min(limit, 50) - # Calculate pagination page_size = 10 # Serper default/typical total_pages = math.ceil(limit / page_size) @@ -151,68 +293,174 @@ async def _search_serper( return all_papers[:limit] - @staticmethod - def format_results(results: list[dict[str, Any]]) -> str: - """Format Serper (Google Scholar) results.""" - if not results: - return "No papers found via Google Scholar." + # ── SerpAPI (GET, api_key query param, start offset) ── + async def _fetch_serpapi_page( + self, + query: str, + num: int, + offset: int, + start_year: str | None, + end_year: str | None, + ) -> dict[str, Any]: + params: dict[str, Any] = { + "engine": "google_scholar", + "q": query, + "num": min(num, 20), + "start": offset, + "api_key": self.serpapi_api_key or "", + } - formatted_papers = [] - for i, paper in enumerate(results, 1): - title = paper.get("title", "Unknown Title") - year = paper.get("year", "Unknown Year") - snippet = paper.get("snippet", "") - link = paper.get("link", "") - pub_info = paper.get("publicationInfo", "") - citations = paper.get("citedBy", 0) + if start_year: + params["as_ylo"] = start_year + if end_year: + params["as_yhi"] = end_year - paper_str = ( - f"{i}. **{title}** ({year})\n" - f" - **Publication**: {pub_info}\n" - f" - **Citations**: {citations}\n" - f" - **Snippet**: {snippet}\n" - f" - **Link**: {link}" + async with aiohttp.ClientSession() as session: + async with session.get( + SERPAPI_API_URL, + params=params, + timeout=aiohttp.ClientTimeout(total=self.timeout), + ) as response: + if response.status != 200: + text = await response.text() + raise Exception(f"SerpAPI error: {response.status} - {text}") + return await response.json() + + async def _search_serpapi( + self, + query: str, + year: str | None = None, + limit: int = 10, + ) -> list[dict[str, Any]]: + start_year, end_year = self._parse_year_range(year) + limit = min(limit, 50) + + page_size = 10 + total_pages = math.ceil(limit / page_size) + + tasks = [] + for page in range(total_pages): + current_limit = min(page_size, limit - (page * page_size)) + if current_limit <= 0: + break + tasks.append( + self._fetch_serpapi_page( + query, + current_limit, + page * page_size, + start_year, + end_year, + ) ) - formatted_papers.append(paper_str) - return "\n\n".join(formatted_papers) + page_results = await asyncio.gather(*tasks) + raw = [] + for result in page_results: + raw.extend(result.get("organic_results", [])) + return self._normalize_serpapi(raw)[:limit] - async def search( + @staticmethod + def _normalize_serpapi(results: list[dict[str, Any]]) -> list[dict[str, Any]]: + normalized = [] + for paper in results: + pub_info = paper.get("publication_info", {}) or {} + summary = pub_info.get("summary", "") + inline_links = paper.get("inline_links", {}) or {} + cited_by = inline_links.get("cited_by", {}) or {} + normalized.append( + { + "title": paper.get("title", "Unknown Title"), + "year": PaperSearchTool._extract_year(summary), + "snippet": paper.get("snippet", ""), + "link": paper.get("link", ""), + "publicationInfo": summary, + "citedBy": cited_by.get("total", 0), + } + ) + return normalized + + # ── SearchAPI (GET, api_key query param, 1-based page) ── + async def _fetch_searchapi_page( self, query: str, - year: str | None = None, - ) -> str: - """ - Search for peer-reviewed academic papers and scientific publications. + num: int, + page: int, + start_year: str | None, + end_year: str | None, + ) -> dict[str, Any]: + params: dict[str, Any] = { + "engine": "google_scholar", + "q": query, + "num": min(num, 20), + "page": page, + "api_key": self.searchapi_api_key or "", + } - This method returns papers from Google Scholar with citations, abstracts, - and links for research queries requiring authoritative, scholarly sources - including: scientific concepts, algorithms, methodologies, technical - foundations, theoretical frameworks, empirical studies, and peer-reviewed - evidence. + if start_year: + params["as_ylo"] = start_year + if end_year: + params["as_yhi"] = end_year - Args: - query: The search query string. - year: Optional year or year range (e.g., "2023" or "2020-2023"). + async with aiohttp.ClientSession() as session: + async with session.get( + SEARCHAPI_API_URL, + params=params, + timeout=aiohttp.ClientTimeout(total=self.timeout), + ) as response: + if response.status != 200: + text = await response.text() + raise Exception(f"SearchAPI error: {response.status} - {text}") + return await response.json() - Returns: - Formatted string with search results. - """ - if not query: - return "Error: 'query' argument is required" + async def _search_searchapi( + self, + query: str, + year: str | None = None, + limit: int = 10, + ) -> list[dict[str, Any]]: + start_year, end_year = self._parse_year_range(year) + limit = min(limit, 50) - if year is not None and not isinstance(year, str): - year = str(year) + page_size = 10 + total_pages = math.ceil(limit / page_size) - logger.info(f"Paper search (serper) for: {query}") + tasks = [] + for page_idx in range(total_pages): + current_limit = min(page_size, limit - (page_idx * page_size)) + if current_limit <= 0: + break + # SearchAPI pages are 1-based + tasks.append( + self._fetch_searchapi_page( + query, + current_limit, + page_idx + 1, + start_year, + end_year, + ) + ) - try: - results = await self._search_serper(query, year, self.max_results) - return self.format_results(results) + page_results = await asyncio.gather(*tasks) + raw = [] + for result in page_results: + raw.extend(result.get("organic_results", [])) + return self._normalize_searchapi(raw)[:limit] - except TimeoutError: - logger.error(f"Paper search timed out for query: {query}") - return f"Paper search timed out after {self.timeout}s for query: {query}" - except Exception as e: - logger.error(f"Paper search failed: {e}") - return f"Paper search failed: {str(e)}" + @staticmethod + def _normalize_searchapi(results: list[dict[str, Any]]) -> list[dict[str, Any]]: + normalized = [] + for paper in results: + pub_info = paper.get("publication", "") or "" + inline_links = paper.get("inline_links", {}) or {} + cited_by = inline_links.get("cited_by", {}) or {} + normalized.append( + { + "title": paper.get("title", "Unknown Title"), + "year": PaperSearchTool._extract_year(pub_info), + "snippet": paper.get("snippet", ""), + "link": paper.get("link", ""), + "publicationInfo": pub_info, + "citedBy": cited_by.get("total", 0), + } + ) + return normalized diff --git a/sources/google_scholar_paper_search/src/register.py b/sources/google_scholar_paper_search/src/register.py index 20c6dcf8d..f1cd16a82 100644 --- a/sources/google_scholar_paper_search/src/register.py +++ b/sources/google_scholar_paper_search/src/register.py @@ -27,22 +27,26 @@ from nat.cli.register_workflow import register_function from nat.data_models.function import FunctionBaseConfig +from .paper_search import PaperSearchProvider from .paper_search import PaperSearchTool logger = logging.getLogger(__name__) -# Track if we've already warned about missing API key to avoid duplicate warnings _missing_key_warned = False class PaperSearchToolConfig(FunctionBaseConfig, name="paper_search"): - """ - Configuration for the paper search tool. + """Configuration for the paper search tool. - Tool that searches for academic papers using Google Scholar (Serper). - Requires a SERPER_API_KEY environment variable or config. + Tool that searches for academic papers using Google Scholar. The + ``provider`` field selects the backend API: ``serper`` (default), + ``serpapi``, or ``searchapi``. Each provider requires its own API key. """ + provider: PaperSearchProvider = Field( + default=PaperSearchProvider.SERPER, + description="Google Scholar backend: 'serper', 'serpapi', or 'searchapi'", + ) timeout: int = Field( default=30, description="Timeout in seconds for the search requests", @@ -53,41 +57,63 @@ class PaperSearchToolConfig(FunctionBaseConfig, name="paper_search"): ) serper_api_key: SecretStr | None = Field( default=None, - description="The API key for Serper (Google Scholar)", + description="API key for Serper (required when provider='serper')", + ) + serpapi_api_key: SecretStr | None = Field( + default=None, + description="API key for SerpAPI (required when provider='serpapi')", + ) + searchapi_api_key: SecretStr | None = Field( + default=None, + description="API key for SearchAPI (required when provider='searchapi')", ) +# Maps each provider to (env var name, config attr name, sign-up URL) +_PROVIDER_KEY_INFO = { + PaperSearchProvider.SERPER: ("SERPER_API_KEY", "serper_api_key", "https://serper.dev/"), + PaperSearchProvider.SERPAPI: ("SERPAPI_API_KEY", "serpapi_api_key", "https://serpapi.com/"), + PaperSearchProvider.SEARCHAPI: ("SEARCHAPI_API_KEY", "searchapi_api_key", "https://www.searchapi.io/"), +} + + +def _resolve_api_key(provider: PaperSearchProvider, tool_config: PaperSearchToolConfig) -> str | None: + env_var, config_attr, _ = _PROVIDER_KEY_INFO[provider] + config_value = getattr(tool_config, config_attr) + if not os.environ.get(env_var) and config_value: + os.environ[env_var] = config_value.get_secret_value() + return os.environ.get(env_var) + + @register_function(config_type=PaperSearchToolConfig) async def paper_search(tool_config: PaperSearchToolConfig, builder: Builder): - """Register paper search tool using Google Scholar (Serper).""" - # Set environment variable if provided in config - if not os.environ.get("SERPER_API_KEY") and tool_config.serper_api_key: - os.environ["SERPER_API_KEY"] = tool_config.serper_api_key.get_secret_value() - - serper_api_key = os.environ.get("SERPER_API_KEY") + provider = tool_config.provider + api_key = _resolve_api_key(provider, tool_config) - if not serper_api_key: - # Log warning only once to avoid duplicate warnings if tool is registered multiple times + if not api_key: + env_var, _, signup_url = _PROVIDER_KEY_INFO[provider] global _missing_key_warned if not _missing_key_warned: logger.warning( - "SERPER_API_KEY not found. The paper search tool will be registered but will " - "return an error when called. To enable: set SERPER_API_KEY in your environment, " - ".env file, or specify the API key in your workflow config (SERPER_API_KEY)." + "%s not found for provider '%s'. The paper search tool will be registered but " + "will return an error when called. To enable: set %s in your environment, .env " + "file, or specify the API key in your workflow config.", + env_var, + provider.value, + env_var, ) _missing_key_warned = True - # Yield a stub function that returns an error message async def _paper_search_stub( query: str = Field(..., validation_alias=AliasChoices("query", "question")), year: str | None = None, ) -> str: - """Paper search tool (unavailable - missing SERPER_API_KEY).""" return ( - "Error: Paper search is unavailable because SERPER_API_KEY is not set.\n" + f"Error: Paper search is unavailable because {env_var} is not set " + f"(provider='{provider.value}').\n" "To enable this tool:\n" - "1. Get an API key from https://serper.dev/\n" - "2. Set the API key in your environment or .env file\n" + f"1. Get an API key from {signup_url}\n" + f"2. Set the API key in your environment or .env file as {env_var}\n" " (alternatively, specify the API key in your workflow config)\n" "3. Restart the application" ) @@ -98,9 +124,11 @@ async def _paper_search_stub( ) return - # Create the NAT-independent tool instance tool = PaperSearchTool( - serper_api_key=serper_api_key, + provider=provider, + serper_api_key=tool_config.serper_api_key.get_secret_value() if tool_config.serper_api_key else None, + serpapi_api_key=tool_config.serpapi_api_key.get_secret_value() if tool_config.serpapi_api_key else None, + searchapi_api_key=tool_config.searchapi_api_key.get_secret_value() if tool_config.searchapi_api_key else None, timeout=tool_config.timeout, max_results=tool_config.max_results, ) diff --git a/sources/google_scholar_paper_search/tests/conftest.py b/sources/google_scholar_paper_search/tests/conftest.py index 33d6d3c71..7b225d84b 100644 --- a/sources/google_scholar_paper_search/tests/conftest.py +++ b/sources/google_scholar_paper_search/tests/conftest.py @@ -21,7 +21,7 @@ @pytest.fixture def paper_search_tool(): - """Create a PaperSearchTool instance for testing.""" + """Create a PaperSearchTool instance (Serper default) for testing.""" return PaperSearchTool( serper_api_key="test-api-key", timeout=30, @@ -29,6 +29,28 @@ def paper_search_tool(): ) +@pytest.fixture +def serpapi_tool(): + """Create a PaperSearchTool instance configured for SerpAPI.""" + return PaperSearchTool( + provider="serpapi", + serpapi_api_key="test-serpapi-key", + timeout=30, + max_results=10, + ) + + +@pytest.fixture +def searchapi_tool(): + """Create a PaperSearchTool instance configured for SearchAPI.""" + return PaperSearchTool( + provider="searchapi", + searchapi_api_key="test-searchapi-key", + timeout=30, + max_results=10, + ) + + @pytest.fixture def sample_serper_response(): """Sample Serper API response for testing.""" @@ -54,9 +76,84 @@ def sample_serper_response(): } +@pytest.fixture +def sample_serpapi_response(): + """Sample SerpAPI response (shape matches serpapi.com docs).""" + return { + "search_metadata": {"status": "Success"}, + "organic_results": [ + { + "position": 0, + "title": "Attention Is All You Need", + "result_id": "abc123", + "link": "https://arxiv.org/abs/1706.03762", + "snippet": "The dominant sequence transduction models...", + "publication_info": { + "summary": "A Vaswani, N Shazeer, N Parmar... - Advances in neural information..., 2017 - arxiv.org" + }, + "inline_links": { + "cited_by": { + "total": 50000, + "link": "https://scholar.google.com/scholar?cites=123", + }, + }, + }, + { + "position": 1, + "title": "BERT: Pre-training of Deep Bidirectional Transformers", + "link": "https://arxiv.org/abs/1810.04805", + "snippet": "We introduce a new language model...", + "publication_info": { + "summary": "J Devlin, MW Chang, K Lee, K Toutanova - arXiv preprint, 2018 - arxiv.org" + }, + "inline_links": { + "cited_by": {"total": 40000}, + }, + }, + ], + } + + +@pytest.fixture +def sample_searchapi_response(): + """Sample SearchAPI response (shape matches searchapi.io docs).""" + return { + "search_metadata": {"status": "Success"}, + "organic_results": [ + { + "position": 1, + "title": "Attention Is All You Need", + "data_cid": "abc123", + "link": "https://arxiv.org/abs/1706.03762", + "publication": ( + "A Vaswani, N Shazeer, N Parmar... - Advances in neural information..., 2017 - arxiv.org" + ), + "snippet": "The dominant sequence transduction models...", + "inline_links": { + "cited_by": { + "cites_id": "123", + "total": 50000, + "link": "https://scholar.google.com/scholar?cites=123", + } + }, + }, + { + "position": 2, + "title": "BERT: Pre-training of Deep Bidirectional Transformers", + "link": "https://arxiv.org/abs/1810.04805", + "publication": "J Devlin, MW Chang, K Lee, K Toutanova - 2018 - arxiv.org", + "snippet": "We introduce a new language model...", + "inline_links": { + "cited_by": {"total": 40000}, + }, + }, + ], + } + + @pytest.fixture def sample_papers(): - """Sample paper data for format testing.""" + """Sample normalized paper data for format testing.""" return [ { "title": "Test Paper 1", diff --git a/sources/google_scholar_paper_search/tests/test_paper_search.py b/sources/google_scholar_paper_search/tests/test_paper_search.py index 75a60e8ae..a9475dfb3 100644 --- a/sources/google_scholar_paper_search/tests/test_paper_search.py +++ b/sources/google_scholar_paper_search/tests/test_paper_search.py @@ -20,6 +20,7 @@ from unittest.mock import patch import pytest +from google_scholar_paper_search.paper_search import PaperSearchProvider from google_scholar_paper_search.paper_search import PaperSearchTool @@ -58,6 +59,32 @@ def test_init_with_custom_max_results(self): assert tool.max_results == 50 + def test_init_defaults_to_serper_provider(self): + """Test that provider defaults to serper.""" + tool = PaperSearchTool(serper_api_key="test-key") + + assert tool.provider is PaperSearchProvider.SERPER + + def test_init_with_serpapi_provider(self): + """Test initialization with serpapi provider.""" + tool = PaperSearchTool(provider="serpapi", serpapi_api_key="serpapi-key") + + assert tool.provider is PaperSearchProvider.SERPAPI + assert tool.serpapi_api_key == "serpapi-key" + + def test_init_with_searchapi_provider(self): + """Test initialization with searchapi provider.""" + tool = PaperSearchTool(provider="searchapi", searchapi_api_key="searchapi-key") + + assert tool.provider is PaperSearchProvider.SEARCHAPI + assert tool.searchapi_api_key == "searchapi-key" + + def test_init_provider_from_enum(self): + """Test that provider accepts the enum directly.""" + tool = PaperSearchTool(provider=PaperSearchProvider.SERPAPI, serpapi_api_key="key") + + assert tool.provider is PaperSearchProvider.SERPAPI + class TestFormatResults: """Tests for format_results static method.""" @@ -402,3 +429,439 @@ async def test_fetch_no_year_params_when_none(self, paper_search_tool): payload = call_kwargs["json"] assert "as_ylo" not in payload assert "as_yhi" not in payload + + +class TestExtractYear: + """Tests for _extract_year static method.""" + + def test_extract_year_from_summary(self): + """Test extracting year from a publication summary string.""" + summary = "JL Harper - Population biology of plants., 1977 - cabdirect.org" + assert PaperSearchTool._extract_year(summary) == "1977" # noqa: SLF001 + + def test_extract_year_multiple_numbers(self): + """Test that first valid year is extracted.""" + summary = "A Author - Journal published 2020, vol 123 - host.com" + assert PaperSearchTool._extract_year(summary) == "2020" # noqa: SLF001 + + def test_extract_year_no_year(self): + """Test returns Unknown Year when no year is present.""" + assert PaperSearchTool._extract_year("no year here") == "Unknown Year" # noqa: SLF001 + + def test_extract_year_empty_string(self): + """Test returns Unknown Year for empty string.""" + assert PaperSearchTool._extract_year("") == "Unknown Year" # noqa: SLF001 + + def test_extract_year_non_string(self): + """Test returns Unknown Year for non-string input.""" + assert PaperSearchTool._extract_year(None) == "Unknown Year" # noqa: SLF001 + assert PaperSearchTool._extract_year(123) == "Unknown Year" # noqa: SLF001 + + def test_extract_year_rejects_1800s(self): + """Test that years before 1900 are not matched.""" + assert PaperSearchTool._extract_year("published in 1899") == "Unknown Year" # noqa: SLF001 + + def test_extract_year_21st_century(self): + """Test that 21st century years are matched.""" + assert PaperSearchTool._extract_year("2023 - arxiv.org") == "2023" # noqa: SLF001 + + +class TestNormalizeSerpapi: + """Tests for _normalize_serpapi static method.""" + + def test_normalize_full_result(self, sample_serpapi_response): + """Test normalizing a complete SerpAPI result.""" + raw = sample_serpapi_response["organic_results"] + normalized = PaperSearchTool._normalize_serpapi(raw) # noqa: SLF001 + + assert len(normalized) == 2 + first = normalized[0] + assert first["title"] == "Attention Is All You Need" + assert first["year"] == "2017" + assert first["link"] == "https://arxiv.org/abs/1706.03762" + assert first["snippet"] == "The dominant sequence transduction models..." + assert "2017" in first["publicationInfo"] + assert first["citedBy"] == 50000 + + def test_normalize_missing_fields(self): + """Test normalizing results with missing fields uses defaults.""" + raw = [{"title": "Only Title"}] + normalized = PaperSearchTool._normalize_serpapi(raw) # noqa: SLF001 + + assert normalized[0]["title"] == "Only Title" + assert normalized[0]["year"] == "Unknown Year" + assert normalized[0]["snippet"] == "" + assert normalized[0]["link"] == "" + assert normalized[0]["publicationInfo"] == "" + assert normalized[0]["citedBy"] == 0 + + def test_normalize_empty_list(self): + """Test normalizing an empty list.""" + assert PaperSearchTool._normalize_serpapi([]) == [] # noqa: SLF001 + + +class TestNormalizeSearchapi: + """Tests for _normalize_searchapi static method.""" + + def test_normalize_full_result(self, sample_searchapi_response): + """Test normalizing a complete SearchAPI result.""" + raw = sample_searchapi_response["organic_results"] + normalized = PaperSearchTool._normalize_searchapi(raw) # noqa: SLF001 + + assert len(normalized) == 2 + first = normalized[0] + assert first["title"] == "Attention Is All You Need" + assert first["year"] == "2017" + assert first["link"] == "https://arxiv.org/abs/1706.03762" + assert first["snippet"] == "The dominant sequence transduction models..." + assert "2017" in first["publicationInfo"] + assert first["citedBy"] == 50000 + + def test_normalize_missing_fields(self): + """Test normalizing results with missing fields uses defaults.""" + raw = [{"title": "Only Title"}] + normalized = PaperSearchTool._normalize_searchapi(raw) # noqa: SLF001 + + assert normalized[0]["title"] == "Only Title" + assert normalized[0]["year"] == "Unknown Year" + assert normalized[0]["citedBy"] == 0 + + def test_normalize_empty_list(self): + """Test normalizing an empty list.""" + assert PaperSearchTool._normalize_searchapi([]) == [] # noqa: SLF001 + + +class TestSearchSerpapi: + """Tests for _search_serpapi internal method.""" + + @pytest.mark.asyncio + async def test_year_parsing_single_year(self, serpapi_tool): + """Test year parsing passes start/end to fetch.""" + with patch.object( + serpapi_tool, + "_fetch_serpapi_page", + new_callable=AsyncMock, + return_value={"organic_results": []}, + ) as mock_fetch: + await serpapi_tool._search_serpapi( # noqa: SLF001 + "query", year="2023", limit=10 + ) + + mock_fetch.assert_called_once() + call_args = mock_fetch.call_args + assert call_args[0][3] == "2023" # start_year + assert call_args[0][4] == "2023" # end_year + + @pytest.mark.asyncio + async def test_pagination_multiple_pages(self, serpapi_tool): + """Test pagination for results requiring multiple pages.""" + with patch.object( + serpapi_tool, + "_fetch_serpapi_page", + new_callable=AsyncMock, + return_value={"organic_results": [{"title": "Paper"}]}, + ) as mock_fetch: + await serpapi_tool._search_serpapi( # noqa: SLF001 + "query", limit=25 + ) + + assert mock_fetch.call_count == 3 + + @pytest.mark.asyncio + async def test_normalizes_results(self, serpapi_tool, sample_serpapi_response): + """Test that raw SerpAPI results are normalized.""" + with patch.object( + serpapi_tool, + "_fetch_serpapi_page", + new_callable=AsyncMock, + return_value=sample_serpapi_response, + ): + result = await serpapi_tool._search_serpapi( # noqa: SLF001 + "query", limit=10 + ) + + assert len(result) == 2 + assert result[0]["title"] == "Attention Is All You Need" + assert result[0]["citedBy"] == 50000 + + +class TestSearchSearchapi: + """Tests for _search_searchapi internal method.""" + + @pytest.mark.asyncio + async def test_year_parsing_single_year(self, searchapi_tool): + """Test year parsing passes start/end to fetch.""" + with patch.object( + searchapi_tool, + "_fetch_searchapi_page", + new_callable=AsyncMock, + return_value={"organic_results": []}, + ) as mock_fetch: + await searchapi_tool._search_searchapi( # noqa: SLF001 + "query", year="2023", limit=10 + ) + + mock_fetch.assert_called_once() + call_args = mock_fetch.call_args + assert call_args[0][3] == "2023" # start_year + assert call_args[0][4] == "2023" # end_year + + @pytest.mark.asyncio + async def test_pagination_multiple_pages(self, searchapi_tool): + """Test pagination for results requiring multiple pages.""" + with patch.object( + searchapi_tool, + "_fetch_searchapi_page", + new_callable=AsyncMock, + return_value={"organic_results": [{"title": "Paper"}]}, + ) as mock_fetch: + await searchapi_tool._search_searchapi( # noqa: SLF001 + "query", limit=25 + ) + + assert mock_fetch.call_count == 3 + + @pytest.mark.asyncio + async def test_page_is_one_based(self, searchapi_tool): + """Test that SearchAPI page numbers start at 1, not 0.""" + with patch.object( + searchapi_tool, + "_fetch_searchapi_page", + new_callable=AsyncMock, + return_value={"organic_results": []}, + ) as mock_fetch: + await searchapi_tool._search_searchapi( # noqa: SLF001 + "query", limit=25 + ) + + page_args = [call.args[2] for call in mock_fetch.call_args_list] + assert page_args == [1, 2, 3] + + @pytest.mark.asyncio + async def test_normalizes_results(self, searchapi_tool, sample_searchapi_response): + """Test that raw SearchAPI results are normalized.""" + with patch.object( + searchapi_tool, + "_fetch_searchapi_page", + new_callable=AsyncMock, + return_value=sample_searchapi_response, + ): + result = await searchapi_tool._search_searchapi( # noqa: SLF001 + "query", limit=10 + ) + + assert len(result) == 2 + assert result[0]["title"] == "Attention Is All You Need" + assert result[0]["citedBy"] == 50000 + + +class TestFetchSerpapiPage: + """Tests for _fetch_serpapi_page internal method.""" + + @pytest.mark.asyncio + async def test_fetch_builds_correct_params(self, serpapi_tool): + """Test that fetch builds correct GET params.""" + mock_response = MagicMock() + mock_response.status = 200 + mock_response.json = AsyncMock(return_value={"organic_results": []}) + + mock_session = MagicMock() + mock_context = MagicMock( + __aenter__=AsyncMock(return_value=mock_response), + __aexit__=AsyncMock(), + ) + mock_session.get = MagicMock(return_value=mock_context) + + with patch("aiohttp.ClientSession") as mock_client: + mock_client.return_value.__aenter__ = AsyncMock(return_value=mock_session) + mock_client.return_value.__aexit__ = AsyncMock() + + await serpapi_tool._fetch_serpapi_page( # noqa: SLF001 + query="test query", + num=10, + offset=0, + start_year="2020", + end_year="2023", + ) + + mock_session.get.assert_called_once() + call_kwargs = mock_session.get.call_args[1] + params = call_kwargs["params"] + + assert params["engine"] == "google_scholar" + assert params["q"] == "test query" + assert params["num"] == 10 + assert params["start"] == 0 + assert params["as_ylo"] == "2020" + assert params["as_yhi"] == "2023" + assert params["api_key"] == "test-serpapi-key" + + @pytest.mark.asyncio + async def test_fetch_no_year_params_when_none(self, serpapi_tool): + """Test that year params are not included when None.""" + mock_response = MagicMock() + mock_response.status = 200 + mock_response.json = AsyncMock(return_value={"organic_results": []}) + + mock_session = MagicMock() + mock_context = MagicMock( + __aenter__=AsyncMock(return_value=mock_response), + __aexit__=AsyncMock(), + ) + mock_session.get = MagicMock(return_value=mock_context) + + with patch("aiohttp.ClientSession") as mock_client: + mock_client.return_value.__aenter__ = AsyncMock(return_value=mock_session) + mock_client.return_value.__aexit__ = AsyncMock() + + await serpapi_tool._fetch_serpapi_page( # noqa: SLF001 + query="test", + num=10, + offset=0, + start_year=None, + end_year=None, + ) + + call_kwargs = mock_session.get.call_args[1] + params = call_kwargs["params"] + assert "as_ylo" not in params + assert "as_yhi" not in params + + +class TestFetchSearchapiPage: + """Tests for _fetch_searchapi_page internal method.""" + + @pytest.mark.asyncio + async def test_fetch_builds_correct_params(self, searchapi_tool): + """Test that fetch builds correct GET params with 1-based page.""" + mock_response = MagicMock() + mock_response.status = 200 + mock_response.json = AsyncMock(return_value={"organic_results": []}) + + mock_session = MagicMock() + mock_context = MagicMock( + __aenter__=AsyncMock(return_value=mock_response), + __aexit__=AsyncMock(), + ) + mock_session.get = MagicMock(return_value=mock_context) + + with patch("aiohttp.ClientSession") as mock_client: + mock_client.return_value.__aenter__ = AsyncMock(return_value=mock_session) + mock_client.return_value.__aexit__ = AsyncMock() + + await searchapi_tool._fetch_searchapi_page( # noqa: SLF001 + query="test query", + num=10, + page=2, + start_year="2020", + end_year="2023", + ) + + mock_session.get.assert_called_once() + call_kwargs = mock_session.get.call_args[1] + params = call_kwargs["params"] + + assert params["engine"] == "google_scholar" + assert params["q"] == "test query" + assert params["num"] == 10 + assert params["page"] == 2 + assert params["as_ylo"] == "2020" + assert params["as_yhi"] == "2023" + assert params["api_key"] == "test-searchapi-key" + + +class TestProviderDispatch: + """Tests for provider-based dispatch in search().""" + + @pytest.mark.asyncio + async def test_search_dispatches_to_serper(self, paper_search_tool): + """Test that search() dispatches to _search_serper by default.""" + with patch.object( + paper_search_tool, + "_search_serper", + new_callable=AsyncMock, + return_value=[], + ) as mock_serper: + await paper_search_tool.search("test query") + + mock_serper.assert_called_once() + + @pytest.mark.asyncio + async def test_search_dispatches_to_serpapi(self, serpapi_tool): + """Test that search() dispatches to _search_serpapi.""" + with ( + patch.object( + serpapi_tool, + "_search_serpapi", + new_callable=AsyncMock, + return_value=[], + ) as mock_serpapi, + patch.object( + serpapi_tool, + "_search_serper", + new_callable=AsyncMock, + ) as mock_serper, + ): + await serpapi_tool.search("test query") + + mock_serpapi.assert_called_once() + mock_serper.assert_not_called() + + @pytest.mark.asyncio + async def test_search_dispatches_to_searchapi(self, searchapi_tool): + """Test that search() dispatches to _search_searchapi.""" + with patch.object( + searchapi_tool, + "_search_searchapi", + new_callable=AsyncMock, + return_value=[], + ) as mock_searchapi: + await searchapi_tool.search("test query") + + mock_searchapi.assert_called_once() + + @pytest.mark.asyncio + async def test_search_serpapi_success(self, serpapi_tool, sample_serpapi_response): + """Test successful search via SerpAPI produces formatted output.""" + with patch.object( + serpapi_tool, + "_search_serpapi", + new_callable=AsyncMock, + return_value=PaperSearchTool._normalize_serpapi( # noqa: SLF001 + sample_serpapi_response["organic_results"] + ), + ): + result = await serpapi_tool.search("transformers") + + assert "Attention Is All You Need" in result + assert "BERT" in result + + @pytest.mark.asyncio + async def test_search_searchapi_success(self, searchapi_tool, sample_searchapi_response): + """Test successful search via SearchAPI produces formatted output.""" + with patch.object( + searchapi_tool, + "_search_searchapi", + new_callable=AsyncMock, + return_value=PaperSearchTool._normalize_searchapi( # noqa: SLF001 + sample_searchapi_response["organic_results"] + ), + ): + result = await searchapi_tool.search("transformers") + + assert "Attention Is All You Need" in result + assert "BERT" in result + + @pytest.mark.asyncio + async def test_search_serpapi_handles_error(self, serpapi_tool): + """Test that search() handles errors for SerpAPI provider.""" + with patch.object( + serpapi_tool, + "_search_serpapi", + new_callable=AsyncMock, + side_effect=Exception("SerpAPI Error"), + ): + result = await serpapi_tool.search("test query") + + assert "Paper search failed" in result + assert "SerpAPI Error" in result diff --git a/uv.lock b/uv.lock index 75724f38d..4ea8b3111 100644 --- a/uv.lock +++ b/uv.lock @@ -1769,16 +1769,16 @@ wheels = [ [[package]] name = "google-scholar-paper-search" -version = "1.0.0" +version = "1.1.0" source = { editable = "sources/google_scholar_paper_search" } dependencies = [ - { name = "httpx" }, + { name = "aiohttp" }, { name = "pydantic" }, ] [package.metadata] requires-dist = [ - { name = "httpx", specifier = ">=0.24.0" }, + { name = "aiohttp", specifier = ">=3.8.0" }, { name = "pydantic", specifier = ">=2.0.0" }, ] From fffc73286390dae256ee15faca5d689de2841414 Mon Sep 17 00:00:00 2001 From: Gabriel Costa Date: Sun, 21 Jun 2026 22:34:01 -0700 Subject: [PATCH 2/6] fix(paper-search): address CodeRabbit review feedback Sanitize error handling and fix key resolution per review: - paper_search.py: remove raw response body from provider exceptions (Serper/SerpAPI/SearchAPI) to prevent API key leakage via error messages; top-level search() handler now returns a generic message without echoing exception text - register.py: _resolve_api_key no longer persists SecretStr values to os.environ (avoids cross-workflow secret leakage); tool constructor now uses the resolved api_key instead of reading config fields, which fixes a bug where env-only setups would pass None to the provider - tests: update error-handling assertions to verify sanitized output (assert raw exception text is NOT present) - README: fix table header from '_type value' to 'provider value' Signed-off-by: Gabriel Costa --- sources/google_scholar_paper_search/README.md | 2 +- .../src/paper_search.py | 15 ++++++--------- .../google_scholar_paper_search/src/register.py | 15 +++++++++------ .../tests/test_paper_search.py | 4 ++-- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/sources/google_scholar_paper_search/README.md b/sources/google_scholar_paper_search/README.md index 360b8f653..c9757efe8 100644 --- a/sources/google_scholar_paper_search/README.md +++ b/sources/google_scholar_paper_search/README.md @@ -2,7 +2,7 @@ A NeMo Agent Toolkit function that searches for academic papers using Google Scholar. You can choose between three backend providers: -| Provider | `_type` value | Env var | Sign-up | +| Provider | `provider` value | Env var | Sign-up | |----------|---------------|---------|---------| | **Serper** (default) | `serper` | `SERPER_API_KEY` | [serper.dev](https://serper.dev/) | | **SerpAPI** | `serpapi` | `SERPAPI_API_KEY` | [serpapi.com](https://serpapi.com/) | diff --git a/sources/google_scholar_paper_search/src/paper_search.py b/sources/google_scholar_paper_search/src/paper_search.py index 3af37820f..1c4d8f9dd 100644 --- a/sources/google_scholar_paper_search/src/paper_search.py +++ b/sources/google_scholar_paper_search/src/paper_search.py @@ -146,9 +146,9 @@ async def search( except TimeoutError: logger.error(f"Paper search timed out for query: {query}") return f"Paper search timed out after {self.timeout}s for query: {query}" - except Exception as e: - logger.error(f"Paper search failed: {e}") - return f"Paper search failed: {str(e)}" + except Exception: + logger.exception("Paper search failed for provider '%s'", self.provider.value) + return f"Paper search failed: unable to fetch results from {self.provider.value}." @staticmethod def format_results(results: list[dict[str, Any]]) -> str: @@ -245,8 +245,7 @@ async def _fetch_serper_page( timeout=aiohttp.ClientTimeout(total=self.timeout), ) as response: if response.status != 200: - text = await response.text() - raise Exception(f"Serper API error: {response.status} - {text}") + raise RuntimeError(f"Serper API error: HTTP {response.status}") return await response.json() async def _search_serper( @@ -322,8 +321,7 @@ async def _fetch_serpapi_page( timeout=aiohttp.ClientTimeout(total=self.timeout), ) as response: if response.status != 200: - text = await response.text() - raise Exception(f"SerpAPI error: {response.status} - {text}") + raise RuntimeError(f"SerpAPI error: HTTP {response.status}") return await response.json() async def _search_serpapi( @@ -408,8 +406,7 @@ async def _fetch_searchapi_page( timeout=aiohttp.ClientTimeout(total=self.timeout), ) as response: if response.status != 200: - text = await response.text() - raise Exception(f"SearchAPI error: {response.status} - {text}") + raise RuntimeError(f"SearchAPI error: HTTP {response.status}") return await response.json() async def _search_searchapi( diff --git a/sources/google_scholar_paper_search/src/register.py b/sources/google_scholar_paper_search/src/register.py index f1cd16a82..a82cb94fc 100644 --- a/sources/google_scholar_paper_search/src/register.py +++ b/sources/google_scholar_paper_search/src/register.py @@ -79,10 +79,13 @@ class PaperSearchToolConfig(FunctionBaseConfig, name="paper_search"): def _resolve_api_key(provider: PaperSearchProvider, tool_config: PaperSearchToolConfig) -> str | None: env_var, config_attr, _ = _PROVIDER_KEY_INFO[provider] + env_value = os.environ.get(env_var) + if env_value: + return env_value config_value = getattr(tool_config, config_attr) - if not os.environ.get(env_var) and config_value: - os.environ[env_var] = config_value.get_secret_value() - return os.environ.get(env_var) + if config_value: + return config_value.get_secret_value() + return None @register_function(config_type=PaperSearchToolConfig) @@ -126,9 +129,9 @@ async def _paper_search_stub( tool = PaperSearchTool( provider=provider, - serper_api_key=tool_config.serper_api_key.get_secret_value() if tool_config.serper_api_key else None, - serpapi_api_key=tool_config.serpapi_api_key.get_secret_value() if tool_config.serpapi_api_key else None, - searchapi_api_key=tool_config.searchapi_api_key.get_secret_value() if tool_config.searchapi_api_key else None, + serper_api_key=api_key if provider is PaperSearchProvider.SERPER else None, + serpapi_api_key=api_key if provider is PaperSearchProvider.SERPAPI else None, + searchapi_api_key=api_key if provider is PaperSearchProvider.SEARCHAPI else None, timeout=tool_config.timeout, max_results=tool_config.max_results, ) diff --git a/sources/google_scholar_paper_search/tests/test_paper_search.py b/sources/google_scholar_paper_search/tests/test_paper_search.py index a9475dfb3..88a2d0974 100644 --- a/sources/google_scholar_paper_search/tests/test_paper_search.py +++ b/sources/google_scholar_paper_search/tests/test_paper_search.py @@ -189,7 +189,7 @@ async def test_search_general_exception(self, paper_search_tool): result = await paper_search_tool.search("test query") assert "Paper search failed" in result - assert "API Error" in result + assert "API Error" not in result @pytest.mark.asyncio async def test_search_with_integer_year(self, paper_search_tool, sample_serper_response): @@ -864,4 +864,4 @@ async def test_search_serpapi_handles_error(self, serpapi_tool): result = await serpapi_tool.search("test query") assert "Paper search failed" in result - assert "SerpAPI Error" in result + assert "SerpAPI Error" not in result From b1d95de6ca7dccf28cedf669cd3d1a782d17b9c1 Mon Sep 17 00:00:00 2001 From: Gabriel Costa Date: Sun, 21 Jun 2026 22:56:36 -0700 Subject: [PATCH 3/6] fix(paper-search): address CodeRabbit follow-up nits - register.py: align stub year type to str | int | None to match the real _paper_search signature, so integer years aren't rejected by schema validation before the graceful unavailable message - paper_search.py: use logger.error instead of logger.exception to avoid logging tracebacks that could include the request URL (which carries the api_key query param for SerpAPI/SearchAPI) - tests: strengthen test_search_dispatches_to_searchapi to assert _search_serper and _search_serpapi are not called, matching the SerpAPI dispatch test pattern Signed-off-by: Gabriel Costa --- .../src/paper_search.py | 2 +- .../src/register.py | 2 +- .../tests/test_paper_search.py | 26 ++++++++++++++----- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/sources/google_scholar_paper_search/src/paper_search.py b/sources/google_scholar_paper_search/src/paper_search.py index 1c4d8f9dd..07e29d541 100644 --- a/sources/google_scholar_paper_search/src/paper_search.py +++ b/sources/google_scholar_paper_search/src/paper_search.py @@ -147,7 +147,7 @@ async def search( logger.error(f"Paper search timed out for query: {query}") return f"Paper search timed out after {self.timeout}s for query: {query}" except Exception: - logger.exception("Paper search failed for provider '%s'", self.provider.value) + logger.error("Paper search failed for provider '%s'", self.provider.value) return f"Paper search failed: unable to fetch results from {self.provider.value}." @staticmethod diff --git a/sources/google_scholar_paper_search/src/register.py b/sources/google_scholar_paper_search/src/register.py index a82cb94fc..cb3e68484 100644 --- a/sources/google_scholar_paper_search/src/register.py +++ b/sources/google_scholar_paper_search/src/register.py @@ -109,7 +109,7 @@ async def paper_search(tool_config: PaperSearchToolConfig, builder: Builder): async def _paper_search_stub( query: str = Field(..., validation_alias=AliasChoices("query", "question")), - year: str | None = None, + year: str | int | None = None, ) -> str: return ( f"Error: Paper search is unavailable because {env_var} is not set " diff --git a/sources/google_scholar_paper_search/tests/test_paper_search.py b/sources/google_scholar_paper_search/tests/test_paper_search.py index 88a2d0974..bd09948c7 100644 --- a/sources/google_scholar_paper_search/tests/test_paper_search.py +++ b/sources/google_scholar_paper_search/tests/test_paper_search.py @@ -810,15 +810,29 @@ async def test_search_dispatches_to_serpapi(self, serpapi_tool): @pytest.mark.asyncio async def test_search_dispatches_to_searchapi(self, searchapi_tool): """Test that search() dispatches to _search_searchapi.""" - with patch.object( - searchapi_tool, - "_search_searchapi", - new_callable=AsyncMock, - return_value=[], - ) as mock_searchapi: + with ( + patch.object( + searchapi_tool, + "_search_searchapi", + new_callable=AsyncMock, + return_value=[], + ) as mock_searchapi, + patch.object( + searchapi_tool, + "_search_serper", + new_callable=AsyncMock, + ) as mock_serper, + patch.object( + searchapi_tool, + "_search_serpapi", + new_callable=AsyncMock, + ) as mock_serpapi, + ): await searchapi_tool.search("test query") mock_searchapi.assert_called_once() + mock_serper.assert_not_called() + mock_serpapi.assert_not_called() @pytest.mark.asyncio async def test_search_serpapi_success(self, serpapi_tool, sample_serpapi_response): From 047538f218983a5133b8489380bffae75d8e7716 Mon Sep 17 00:00:00 2001 From: Gabriel Costa Date: Tue, 23 Jun 2026 18:31:47 -0700 Subject: [PATCH 4/6] fix(paper-search): allowlist placeholder api keys for detect-secrets CI Mark the new SerpAPI/SearchAPI placeholder and test-fixture api-key strings with inline '# pragma: allowlist secret' comments (the repo's established convention, 29 existing usages incl. docs/source/extending/adding-a-tool.md). Refresh .secrets.baseline line numbers shifted by this PR. None of the flagged strings are real secrets. Signed-off-by: Gabriel Costa --- .secrets.baseline | 19 +++++-------------- sources/google_scholar_paper_search/README.md | 4 ++-- .../src/paper_search.py | 6 +++--- .../tests/conftest.py | 4 ++-- .../tests/test_paper_search.py | 8 ++++---- 5 files changed, 16 insertions(+), 25 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 0e6f9882d..d3dee49c8 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -133,7 +133,7 @@ "filename": "README.md", "hashed_secret": "73140b88094aaf220a03532196b27b58a03c9b09", "is_verified": false, - "line_number": 307 + "line_number": 314 } ], "deploy/.env.example": [ @@ -142,7 +142,7 @@ "filename": "deploy/.env.example", "hashed_secret": "9d4e1e23bd5b727046a9e3b4b7db57bd8d6ee684", "is_verified": false, - "line_number": 30 + "line_number": 34 } ], "deploy/compose/README.md": [ @@ -205,16 +205,7 @@ "filename": "sources/google_scholar_paper_search/README.md", "hashed_secret": "42110b77eaf9886a18dbddaa8c08ac7d2a1ac997", "is_verified": false, - "line_number": 14 - } - ], - "sources/google_scholar_paper_search/src/paper_search.py": [ - { - "type": "Secret Keyword", - "filename": "sources/google_scholar_paper_search/src/paper_search.py", - "hashed_secret": "11fa7c37d697f30e6aee828b4426a10f83ab2380", - "is_verified": false, - "line_number": 40 + "line_number": 22 } ], "sources/google_scholar_paper_search/tests/conftest.py": [ @@ -232,7 +223,7 @@ "filename": "sources/google_scholar_paper_search/tests/test_paper_search.py", "hashed_secret": "3acfb2c2b433c0ea7ff107e33df91b18e52f960f", "is_verified": false, - "line_number": 33 + "line_number": 34 } ], "sources/knowledge_layer/KNOWLEDGE-LAYER-SETUP.md": [ @@ -290,5 +281,5 @@ } ] }, - "generated_at": "2026-05-22T20:01:44Z" + "generated_at": "2026-06-24T01:29:07Z" } diff --git a/sources/google_scholar_paper_search/README.md b/sources/google_scholar_paper_search/README.md index c9757efe8..f9abb8cbb 100644 --- a/sources/google_scholar_paper_search/README.md +++ b/sources/google_scholar_paper_search/README.md @@ -21,9 +21,9 @@ You need an API key for **one** of the supported providers. The default is Serpe ```bash SERPER_API_KEY="your-serper-api-key" # OR -SERPAPI_API_KEY="your-serpapi-api-key" +SERPAPI_API_KEY="your-serpapi-api-key" # pragma: allowlist secret # OR -SEARCHAPI_API_KEY="your-searchapi-api-key" +SEARCHAPI_API_KEY="your-searchapi-api-key" # pragma: allowlist secret ``` Alternatively, you can provide the API key directly in the configuration file (see below). diff --git a/sources/google_scholar_paper_search/src/paper_search.py b/sources/google_scholar_paper_search/src/paper_search.py index 07e29d541..550fc3137 100644 --- a/sources/google_scholar_paper_search/src/paper_search.py +++ b/sources/google_scholar_paper_search/src/paper_search.py @@ -57,18 +57,18 @@ class PaperSearchTool: Example: >>> # Serper (default, backward compatible) - >>> tool = PaperSearchTool(serper_api_key="your-key") + >>> tool = PaperSearchTool(serper_api_key="your-key") # pragma: allowlist secret >>> >>> # SerpAPI >>> tool = PaperSearchTool( ... provider="serpapi", - ... serpapi_api_key="your-key", + ... serpapi_api_key="your-key", # pragma: allowlist secret ... ) >>> >>> # SearchAPI >>> tool = PaperSearchTool( ... provider="searchapi", - ... searchapi_api_key="your-key", + ... searchapi_api_key="your-key", # pragma: allowlist secret ... ) >>> result = await tool.search("machine learning transformers") """ diff --git a/sources/google_scholar_paper_search/tests/conftest.py b/sources/google_scholar_paper_search/tests/conftest.py index 7b225d84b..943c0060a 100644 --- a/sources/google_scholar_paper_search/tests/conftest.py +++ b/sources/google_scholar_paper_search/tests/conftest.py @@ -34,7 +34,7 @@ def serpapi_tool(): """Create a PaperSearchTool instance configured for SerpAPI.""" return PaperSearchTool( provider="serpapi", - serpapi_api_key="test-serpapi-key", + serpapi_api_key="test-serpapi-key", # pragma: allowlist secret timeout=30, max_results=10, ) @@ -45,7 +45,7 @@ def searchapi_tool(): """Create a PaperSearchTool instance configured for SearchAPI.""" return PaperSearchTool( provider="searchapi", - searchapi_api_key="test-searchapi-key", + searchapi_api_key="test-searchapi-key", # pragma: allowlist secret timeout=30, max_results=10, ) diff --git a/sources/google_scholar_paper_search/tests/test_paper_search.py b/sources/google_scholar_paper_search/tests/test_paper_search.py index bd09948c7..2e012b9fc 100644 --- a/sources/google_scholar_paper_search/tests/test_paper_search.py +++ b/sources/google_scholar_paper_search/tests/test_paper_search.py @@ -70,14 +70,14 @@ def test_init_with_serpapi_provider(self): tool = PaperSearchTool(provider="serpapi", serpapi_api_key="serpapi-key") assert tool.provider is PaperSearchProvider.SERPAPI - assert tool.serpapi_api_key == "serpapi-key" + assert tool.serpapi_api_key == "serpapi-key" # pragma: allowlist secret def test_init_with_searchapi_provider(self): """Test initialization with searchapi provider.""" tool = PaperSearchTool(provider="searchapi", searchapi_api_key="searchapi-key") assert tool.provider is PaperSearchProvider.SEARCHAPI - assert tool.searchapi_api_key == "searchapi-key" + assert tool.searchapi_api_key == "searchapi-key" # pragma: allowlist secret def test_init_provider_from_enum(self): """Test that provider accepts the enum directly.""" @@ -694,7 +694,7 @@ async def test_fetch_builds_correct_params(self, serpapi_tool): assert params["start"] == 0 assert params["as_ylo"] == "2020" assert params["as_yhi"] == "2023" - assert params["api_key"] == "test-serpapi-key" + assert params["api_key"] == "test-serpapi-key" # pragma: allowlist secret @pytest.mark.asyncio async def test_fetch_no_year_params_when_none(self, serpapi_tool): @@ -767,7 +767,7 @@ async def test_fetch_builds_correct_params(self, searchapi_tool): assert params["page"] == 2 assert params["as_ylo"] == "2020" assert params["as_yhi"] == "2023" - assert params["api_key"] == "test-searchapi-key" + assert params["api_key"] == "test-searchapi-key" # pragma: allowlist secret class TestProviderDispatch: From b078d1f168e31e1dcadfc1d011881b72dc322326 Mon Sep 17 00:00:00 2001 From: Gabriel Costa Date: Thu, 2 Jul 2026 09:59:00 -0700 Subject: [PATCH 5/6] fix(paper-search): correct year parsing and add stub description Extract the publication year as the final year token before the source suffix instead of the first 19xx/20xx match, so date ranges and pre-1900 papers no longer yield a wrong or Unknown year. Widen the year window to 1500-2099 and reject arXiv-id leading digits via a negative lookahead. Give the missing-API-key stub an explicit provider-aware description so FunctionInfo.description is non-None in the graceful-degradation path. Adds year-parsing tests (date ranges, arXiv ids, pre-1900) and a registration-level test asserting the stub description. Signed-off-by: Gabriel Costa --- .../src/paper_search.py | 24 ++++++--- .../src/register.py | 7 ++- .../tests/test_paper_search.py | 53 +++++++++++++++++-- 3 files changed, 72 insertions(+), 12 deletions(-) diff --git a/sources/google_scholar_paper_search/src/paper_search.py b/sources/google_scholar_paper_search/src/paper_search.py index 550fc3137..f1dbb9ebd 100644 --- a/sources/google_scholar_paper_search/src/paper_search.py +++ b/sources/google_scholar_paper_search/src/paper_search.py @@ -34,8 +34,11 @@ SERPAPI_API_URL = "https://serpapi.com/search" SEARCHAPI_API_URL = "https://www.searchapi.io/api/v1/search" -# Matches a 4-digit year (19xx or 20xx) inside a publication summary string. -_YEAR_RE = re.compile(r"\b(19\d{2}|20\d{2})\b") +# Matches a 4-digit publication year (1500-2099). The window is wide enough to +# cover pre-1900 scholarly works while excluding arXiv-style identifiers, and +# the negative lookahead prevents matching the leading digits of an id such as +# ``1910.12345``. +_YEAR_RE = re.compile(r"\b(1[5-9]\d{2}|20\d{2})\b(?!\.\d)") class PaperSearchProvider(StrEnum): @@ -201,16 +204,23 @@ def _parse_year_range(year: str | None) -> tuple[str | None, str | None]: @staticmethod def _extract_year(publication_info: Any) -> str: - """Extract a 4-digit year from a publication summary string. + """Extract the publication year from a publication summary string. SerpAPI and SearchAPI embed the year inside the publication summary - (e.g. ``"JL Harper - ..., 1977 - cabdirect.org"``). Serper returns - a clean ``year`` field, so this is only used for the other two. + (e.g. ``"JL Harper - ..., 1977 - cabdirect.org"``). The publication + year is the final year token before the trailing source suffix, not the + first one: ``"... (1919-1933 …, 1926 - JSTOR"`` yields ``1926``, not + ``1919``. Serper returns a clean ``year`` field, so this is only used + for the other two providers. """ if not isinstance(publication_info, str): return "Unknown Year" - match = _YEAR_RE.search(publication_info) - return match.group(1) if match else "Unknown Year" + # Drop the trailing source/host suffix (" - cabdirect.org") so its + # tokens (e.g. arXiv identifiers) can't be mistaken for a year, then + # take the final year-shaped token in the remaining summary. + summary = publication_info.rsplit(" - ", 1)[0] + matches = _YEAR_RE.findall(summary) + return matches[-1] if matches else "Unknown Year" # ── Serper (POST, X-API-KEY header) ── async def _fetch_serper_page( diff --git a/sources/google_scholar_paper_search/src/register.py b/sources/google_scholar_paper_search/src/register.py index cb3e68484..ff76016b9 100644 --- a/sources/google_scholar_paper_search/src/register.py +++ b/sources/google_scholar_paper_search/src/register.py @@ -123,7 +123,12 @@ async def _paper_search_stub( yield FunctionInfo.from_fn( _paper_search_stub, - description=_paper_search_stub.__doc__, + description=( + f"Search for academic papers and peer-reviewed scientific publications " + f"on Google Scholar via the {provider.value} backend. This tool is " + f"registered in a degraded state because {env_var} is not configured; " + f"calling it returns setup instructions instead of search results." + ), ) return diff --git a/sources/google_scholar_paper_search/tests/test_paper_search.py b/sources/google_scholar_paper_search/tests/test_paper_search.py index 2e012b9fc..83fec0a26 100644 --- a/sources/google_scholar_paper_search/tests/test_paper_search.py +++ b/sources/google_scholar_paper_search/tests/test_paper_search.py @@ -19,6 +19,7 @@ from unittest.mock import MagicMock from unittest.mock import patch +import google_scholar_paper_search.register as register_module import pytest from google_scholar_paper_search.paper_search import PaperSearchProvider from google_scholar_paper_search.paper_search import PaperSearchTool @@ -440,7 +441,7 @@ def test_extract_year_from_summary(self): assert PaperSearchTool._extract_year(summary) == "1977" # noqa: SLF001 def test_extract_year_multiple_numbers(self): - """Test that first valid year is extracted.""" + """Test that the year before the source suffix is extracted.""" summary = "A Author - Journal published 2020, vol 123 - host.com" assert PaperSearchTool._extract_year(summary) == "2020" # noqa: SLF001 @@ -457,9 +458,25 @@ def test_extract_year_non_string(self): assert PaperSearchTool._extract_year(None) == "Unknown Year" # noqa: SLF001 assert PaperSearchTool._extract_year(123) == "Unknown Year" # noqa: SLF001 - def test_extract_year_rejects_1800s(self): - """Test that years before 1900 are not matched.""" - assert PaperSearchTool._extract_year("published in 1899") == "Unknown Year" # noqa: SLF001 + def test_extract_year_supports_pre_1900(self): + """Pre-1900 publication years are extracted, not forced to Unknown.""" + summary = "J Smith - An early treatise on chemistry, 1859 - jstor.org" + assert PaperSearchTool._extract_year(summary) == "1859" # noqa: SLF001 + + def test_extract_year_date_range_picks_publication_year(self): + """Final year before the source suffix wins, not a range start.""" + summary = "Science Progress in the Twentieth Century (1919-1933 \u2026, 1926 - JSTOR" + assert PaperSearchTool._extract_year(summary) == "1926" # noqa: SLF001 + + def test_extract_year_ignores_arxiv_identifier(self): + """An arXiv id prefix is not mistaken for the publication year.""" + summary = "Vaswani et al., Attention Is All You Need, arXiv:1706.03762, 2017 - arxiv.org" + assert PaperSearchTool._extract_year(summary) == "2017" # noqa: SLF001 + + def test_extract_year_arxiv_id_only_is_unknown(self): + """A summary carrying only an arXiv id (no year) yields Unknown Year.""" + summary = "Some Author - A preprint, arXiv:1910.12345 - arxiv.org" + assert PaperSearchTool._extract_year(summary) == "Unknown Year" # noqa: SLF001 def test_extract_year_21st_century(self): """Test that 21st century years are matched.""" @@ -879,3 +896,31 @@ async def test_search_serpapi_handles_error(self, serpapi_tool): assert "Paper search failed" in result assert "SerpAPI Error" not in result + + +class TestRegisterMissingKeyStub: + """Registration-level tests for the missing-API-key stub function.""" + + @pytest.mark.parametrize( + ("provider", "env_var"), + [ + (PaperSearchProvider.SERPER, "SERPER_API_KEY"), + (PaperSearchProvider.SERPAPI, "SERPAPI_API_KEY"), + (PaperSearchProvider.SEARCHAPI, "SEARCHAPI_API_KEY"), + ], + ) + @pytest.mark.asyncio + async def test_stub_description_is_meaningful(self, provider, env_var, monkeypatch): + """Missing-key FunctionInfo exposes a non-None, provider-aware description.""" + monkeypatch.delenv(env_var, raising=False) + # Reset the module-level warn guard so prior runs cannot alter behavior. + monkeypatch.setattr(register_module, "_missing_key_warned", False) + + tool_config = register_module.PaperSearchToolConfig(provider=provider) + + async with register_module.paper_search(tool_config, MagicMock()) as fn: + description = fn.description + + assert description is not None + assert provider.value in description + assert env_var in description From cde2c3fe6af7a76cc4266dea86ab08570a3e71bd Mon Sep 17 00:00:00 2001 From: Gabriel Costa Date: Sun, 5 Jul 2026 09:49:51 -0700 Subject: [PATCH 6/6] fix(paper-search): sanitize timeout path and short-circuit missing key The TimeoutError handler no longer logs or returns the raw query; it reports only the timeout duration and provider, so an input query is not echoed into logs or tool output on a timeout. Direct PaperSearchTool construction now short-circuits when the selected provider's API key is missing, returning a provider-named unavailable message instead of dispatching an upstream call with an empty credential. This matches the register stub's graceful-degradation path. Adds a regression assertion that the query is absent from the timeout result, a parametrized short-circuit test across all three providers, and a no-dispatch test confirming _search_* is never reached. Signed-off-by: Gabriel Costa --- .../src/paper_search.py | 24 ++++++++++++-- .../tests/test_paper_search.py | 32 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/sources/google_scholar_paper_search/src/paper_search.py b/sources/google_scholar_paper_search/src/paper_search.py index f1dbb9ebd..d8231a21c 100644 --- a/sources/google_scholar_paper_search/src/paper_search.py +++ b/sources/google_scholar_paper_search/src/paper_search.py @@ -106,6 +106,16 @@ def __init__( self.timeout = timeout self.max_results = max_results + def _selected_api_key(self) -> str | None: + """Return the API key for the active provider, or ``None`` if unset.""" + if self.provider is PaperSearchProvider.SERPER: + return self.serper_api_key + if self.provider is PaperSearchProvider.SERPAPI: + return self.serpapi_api_key + if self.provider is PaperSearchProvider.SEARCHAPI: + return self.searchapi_api_key + return None # pragma: no cover - exhausted by enum + # ── Public API ── async def search( self, @@ -133,6 +143,16 @@ async def search( if year is not None and not isinstance(year, str): year = str(year) + if not self._selected_api_key(): + logger.warning( + "Paper search unavailable: no API key configured for provider '%s'", + self.provider.value, + ) + return ( + f"Error: Paper search is unavailable because no API key is configured " + f"for provider '{self.provider.value}'." + ) + logger.info(f"Paper search ({self.provider.value}) for: {query}") try: @@ -147,8 +167,8 @@ async def search( return self.format_results(results) except TimeoutError: - logger.error(f"Paper search timed out for query: {query}") - return f"Paper search timed out after {self.timeout}s for query: {query}" + logger.error("Paper search timed out after %ss for provider '%s'", self.timeout, self.provider.value) + return f"Paper search timed out after {self.timeout}s. Try again or narrow the query." except Exception: logger.error("Paper search failed for provider '%s'", self.provider.value) return f"Paper search failed: unable to fetch results from {self.provider.value}." diff --git a/sources/google_scholar_paper_search/tests/test_paper_search.py b/sources/google_scholar_paper_search/tests/test_paper_search.py index 83fec0a26..d44569627 100644 --- a/sources/google_scholar_paper_search/tests/test_paper_search.py +++ b/sources/google_scholar_paper_search/tests/test_paper_search.py @@ -177,6 +177,7 @@ async def test_search_timeout_error(self, paper_search_tool): assert "Paper search timed out" in result assert "30s" in result + assert "test query" not in result @pytest.mark.asyncio async def test_search_general_exception(self, paper_search_tool): @@ -201,6 +202,37 @@ async def test_search_with_integer_year(self, paper_search_tool, sample_serper_r mock_search.assert_called_once_with("transformers", "2023", 10) + @pytest.mark.parametrize( + "provider", + [ + PaperSearchProvider.SERPER, + PaperSearchProvider.SERPAPI, + PaperSearchProvider.SEARCHAPI, + ], + ) + @pytest.mark.asyncio + async def test_search_short_circuits_when_provider_key_missing(self, provider): + """Direct construction with the selected provider's key missing short-circuits.""" + tool = PaperSearchTool(provider=provider) + result = await tool.search("transformers") + + assert "unavailable" in result.lower() + assert provider.value in result + + @pytest.mark.asyncio + async def test_search_missing_key_does_not_dispatch(self, paper_search_tool): + """A missing provider key short-circuits before any provider call.""" + paper_search_tool.serper_api_key = None + with patch.object( + paper_search_tool, + "_search_serper", + new_callable=AsyncMock, + ) as mock_search: + result = await paper_search_tool.search("transformers") + + assert "unavailable" in result.lower() + mock_search.assert_not_called() + class TestSearchSerper: """Tests for _search_serper internal method."""