Skip to content

feat: add extract tool for SerpApi Extract API calls - #76

Open
adarshdigievo wants to merge 3 commits into
mainfrom
feat/extract-tool
Open

adarshdigievo wants to merge 3 commits into
mainfrom
feat/extract-tool

Conversation

@adarshdigievo

Copy link
Copy Markdown
Member
  • Add a dedicated extract tool that calls the SerpApi Extract API
  • This is helpful for AI agents and MCP clients to discover the new functionality.

Tests done:

  • Verified MCP search and extract tools locally via Docker. Also tested map, crawl, and scrape modes of the Extract API with sample requests.

Copilot AI balanced review requested due to automatic review settings October 15, 2026 12:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Compact defaults conflict with disabled Markdown, and source-based launches lack the schema needed for Extract discovery and guided input.

2 open findings
What changed in this PR

Adds a discoverable extract MCP tool that reuses the existing SerpApi search pipeline.

Changes:

  • Adds extraction-specific defaults and tool metadata.
  • Documents scrape, map, and crawl usage.
  • Adds protocol coverage and registration checks.
File Description
tests/​test_server.py Checks Extract tool registration.
tests/​test_mcpb.py Checks Extract availability over stdio.
tests/​test_extract_protocol.py Tests defaults, validation, authentication, and responses.
src/​mcp_components/​tools.py Implements the Extract wrapper and defaults.
README.md Documents Extract parameters and examples.
mcpb/​manifest.json Lists Extract in the bundle manifest.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/mcp_components/tools.py Outdated
Comment thread src/mcp_components/tools.py

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Compact extraction can discard page content when Markdown is disabled using the string "false".

0 open findings

2 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Handle string false when selecting non-markdown text content

src/​mcp_components/​tools.py:324

With mode="compact" and params.include_markdown="false", this identity check selects text_content.markdown while forwarding the string unchanged to SerpApi. Markdown is disabled upstream, so the default selector filters out the page content instead of returning text_content. Recognize the API-style string as well as Boolean false, and add a regression case; the current tests cover only the Boolean value.

🧠 Review effort: Balanced

@strzibny

Copy link
Copy Markdown
  1. We need to ship extract schema:

The file engines/extract.json isn’t included. Copilot marked “Ship Extract schema for source-based startup” as resolved, but no file was added. I checked the live Playground: it does list extract (131 engines there, 118 committed), so builds that regenerate engines/ will pick it up. That covers Docker, the release bundle and the hosted server.

Running from source (uv run src/server.py) or building with mcpb/build.py --no-rebuild-engines won’t have it.
In those setups, serpapi://engines/extract is missing, but both the README and the tool description point to it.

  1. We return content twice by default:

Markdown is on by default, even in complete mode. With include_markdown: True, a complete scrape returns both the full text and the markdown, so the page content arrives roughly twice. That’s a lot of tokens for the default call ({"params": {"url": ...}}). Is that intended, or should the markdown default apply only in compact mode?

@adarshdigievo

Copy link
Copy Markdown
Member Author
  1. We need to ship extract schema:

The file engines/extract.json isn’t included. Copilot marked “Ship Extract schema for source-based startup” as resolved, but no file was added. I checked the live Playground: it does list extract (131 engines there, 118 committed), so builds that regenerate engines/ will pick it up. That covers Docker, the release bundle and the hosted server.

Running from source (uv run src/server.py) or building with mcpb/build.py --no-rebuild-engines won’t have it. In those setups, serpapi://engines/extract is missing, but both the README and the tool description point to it.

IMO, engine files in Git are adding a lot of noise. We usually have a few engine schemas updated every week. I planned to remove those entirely and generate them on the fly (during the build or first-run phase). Also, planning to set up an automated weekly job that refreshes the engine schema on our deployed server.

Our hosted MCP and built bundles are not affected because they get the latest engine schemas built in CI, as you noted.

Agree that this affects local runs using the `uv run command; I will fix the need to have static engine schemas as a follow-up PR.

@strzibny What do you think? Should I try the cleanup now or merge this and create a follow-up PR?

  1. We return content twice by default:

Markdown is on by default, even in complete mode. With include_markdown: True, a complete scrape returns both the full text and the markdown, so the page content arrives roughly twice. That’s a lot of tokens for the default call ({"params": {"url": ...}}). Is that intended, or should the markdown default apply only in compact mode?

Agree with the token usage part.
Since MCP will be used mostly by agents, I wanted the response to include the most helpful data for agents. text_content.full is unformatted plain text and doesn't convey the page structure well. This was the reasoning behind adding Markdown.

Removing it from being added by default in complete also sounds reasonable. What do you recommend?

@strzibny

Copy link
Copy Markdown

If you plan a follow-up, then it's fine. I don't think there is an easy answer for Markdown, but ideally I would like us to do the same default for search and scrape API. Maybe it makes sense that both would be Markdown by default, on the other hand this is not true for the server. But this might not be an issue per se, MCP could default to Markdown for it's obvious use by agents.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants