✨ Add an MCP server that enables LLM agents to fetch aigverse documentation#338
Open
marcelwa wants to merge 34 commits into
Open
✨ Add an MCP server that enables LLM agents to fetch aigverse documentation#338marcelwa wants to merge 34 commits into
aigverse documentation#338marcelwa wants to merge 34 commits into
Conversation
Add an MCP (Model Context Protocol) server that gives LLM-based agents on-demand access to the aigverse documentation and Python API reference hosted on ReadTheDocs. Tools: - get_documentation: fetch guide/tutorial pages by slug - lookup_api_symbol: per-symbol API reference lookup - search_documentation: keyword search via RTD search API Resource: - aigverse://pages: JSON listing of all available documentation pages The server is packaged as the aigverse[mcp] extra with a aigverse-mcp-server entry point script. Dependencies: fastmcp, httpx, beautifulsoup4, markdownify
- Introduced a new command line option `--run-mcp-integration` in `conftest.py` to control the execution of MCP integration tests. - Updated the `pytest_collection_modifyitems` function to skip MCP integration tests unless the new option is specified. - Created `test_mcp_server.py` to include various tests for the MCP server, including entry-point checks, module import tests, and live documentation integration tests. - Added dependencies for `pytest-console-scripts`, `beautifulsoup4`, `fastmcp`, and `httpx` in `uv.lock` for testing purposes.
marcelwa
added
enhancement
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an optional MCP (Model Context Protocol) server and CLI to serve Aigverse documentation and API reference to LLM clients; updates docs, Sphinx config, package metadata/entrypoint, pytest hooks, and test coverage for the new MCP functionality and docs links. Changes
Sequence DiagramsequenceDiagram
participant Client as LLM Client
participant MCP as Aigverse MCP Server
participant HTTP as HTTP Fetcher (httpx/urllib)
participant RTD as ReadTheDocs
participant Parser as HTML → Markdown Parser
Client->>MCP: get_documentation(slug)
MCP->>MCP: normalize slug, build RTD URL
MCP->>HTTP: fetch page (httpx → fallback urllib)
HTTP->>RTD: GET https://aigverse.readthedocs.io/...
RTD-->>HTTP: HTML
HTTP-->>MCP: HTML content
MCP->>Parser: extract main article / convert to Markdown
Parser-->>MCP: Markdown text
MCP-->>Client: documentation text
Client->>MCP: lookup_api_symbol(symbol)
MCP->>MCP: infer candidate API index pages
MCP->>HTTP: fetch candidate API pages
HTTP->>RTD: GET API page(s)
RTD-->>HTTP: HTML
HTTP-->>MCP: HTML content
MCP->>Parser: extract symbol fragment (HTML → Markdown)
Parser-->>MCP: Markdown fragment
MCP-->>Client: symbol reference
Client->>MCP: search_documentation(query)
MCP->>HTTP: call ReadTheDocs v3 search API
HTTP->>RTD: GET /api/v3/search/
RTD-->>HTTP: JSON results
HTTP-->>MCP: search hits + snippets
MCP-->>Client: search results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
python/aigverse/mcp/server.py (1)
88-88: Module-levelhttpx.Clientis never explicitly closed.The client will be garbage-collected on process exit, which is acceptable for a server process, but consider using
atexitto close it cleanly or wrapping calls in a context manager for better resource hygiene.♻️ Optional: register cleanup with atexit
+import atexit + _client = httpx.Client(follow_redirects=True, timeout=30.0) +atexit.register(_client.close)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/aigverse/mcp/server.py` at line 88, The module-level httpx.Client instance named _client is never closed; register a cleanup to call _client.close() (for example via atexit.register) or refactor call sites that use _client to create and use a context-managed client so the connection pool is closed cleanly; locate the module-level variable _client and either add an atexit.register(lambda: _client.close()) or change functions that use _client to use httpx.Client() as a context manager and remove the persistent _client.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Around line 48-49: The console script aigverse-mcp-server points to
aigverse.mcp:main but optional MCP dependencies (fastmcp, httpx, bs4,
markdownify) are imported at module scope and will make the wrapper crash for
users who didn't install the [mcp] extra; fix by moving those optional imports
into the main() function (or into helper functions called by main()), catch
ImportError and print a clear message explaining the missing extras and how to
install them (e.g., "install aigverse[mcp]"), and ensure module-level code no
longer imports these names so the entry-point can be created safely without the
extra dependencies.
In `@python/aigverse/mcp/server.py`:
- Around line 323-327: The code currently only catches httpx.HTTPStatusError
around the _client.get(...) / resp.raise_for_status() block, so network-level
failures (timeouts, DNS, connection errors) bubble up; change the except to
catch the broader httpx.HTTPError (or add an additional except httpx.HTTPError)
and return a user-friendly message including the exception details, referencing
the _client.get call and resp.raise_for_status usage so you update the
try/except that surrounds those calls.
- Around line 237-244: The try/except around the call to _fetch_page(url) only
catches httpx.HTTPStatusError and therefore misses connection, timeout, and
other transport errors; change the exception handling to catch httpx.HTTPError
(the broader base class) so connection/timeout failures are handled, and in the
error return include either exc.response.status_code when exc.response is
present or the exception text (str(exc)) to provide a friendly but informative
message from the _fetch_page call.
- Around line 281-284: The try/except around calling _fetch_page(url) only
catches httpx.HTTPStatusError and therefore lets connection/timeout failures
escape; update the exception handling to also catch httpx.RequestError (which
covers ConnectError and TimeoutException) so the loop continues to the next
submodule on network errors, and optionally log the caught exception with
context (e.g., mentioning url and the exception) before continuing; look for the
_fetch_page call and the surrounding try/except in server.py to implement this
change.
- Line 9: Replace the spurious spaced entry-point string "aigverse - mcp -
server" with the correct hyphenated token "aigverse-mcp-server" wherever it
appears (e.g., top-of-file docstring or entry_points definition) so the CLI
entry point is valid; search for the exact incorrect string and update it to
"aigverse-mcp-server" in the module (server.py) or packaging metadata.
- Around line 159-163: The initial regex used in the anchor lookup (soup.find
with id=re.compile(re.escape(symbol), re.IGNORECASE)) is too broad and matches
substrings inside larger identifiers; update the matching logic in server.py so
the first attempt only matches whole path segments (e.g., the symbol as a full
segment bounded by start/end or dots) or alternatively swap the order and try
the precise suffix match (the existing rf"\.{re.escape(symbol)}$" pattern)
before the broad match; adjust the regex used in the soup.find calls around the
anchor variable (and the symbol variable) to enforce segment boundaries rather
than substring matches.
---
Nitpick comments:
In `@python/aigverse/mcp/server.py`:
- Line 88: The module-level httpx.Client instance named _client is never closed;
register a cleanup to call _client.close() (for example via atexit.register) or
refactor call sites that use _client to create and use a context-managed client
so the connection pool is closed cleanly; locate the module-level variable
_client and either add an atexit.register(lambda: _client.close()) or change
functions that use _client to use httpx.Client() as a context manager and remove
the persistent _client.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 48e2856e-e7a4-415c-b70f-c90b8ab72029
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
README.mddocs/index.mddocs/mcp.mdpyproject.tomlpython/aigverse/mcp/__init__.pypython/aigverse/mcp/__main__.pypython/aigverse/mcp/server.pytest/conftest.pytest/mcp/test_mcp_server.py
Comment on lines
+48
to
+49
| [project.scripts] | ||
| aigverse-mcp-server = "aigverse.mcp:main" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Python packaging (PEP 621), are [project.scripts] console scripts installed unconditionally, or can they be conditional on optional extras? What are recommended patterns for optional CLIs that depend on extra dependencies?
💡 Result:
[project.scripts] (PEP 621 / the PyPA pyproject.toml spec) maps directly to console_scripts entry points: if the distribution is installed, the installer creates the wrappers for those scripts—there’s no standardized way to make a [project.scripts] entry exist only when an extra is selected. [1]
While the entry-points file format allows an entry point to be annotated with extras (e.g. cmd = pkg.mod:main [cli]), the entry-points specification explicitly says this is “no longer recommended” and that consumers may ignore it (notably, pip’s model differs from the old setuptools/egg model). [2] In practice, pip has historically installed the console script wrapper even if the extra isn’t selected (see pip issue #9726). [3]
Recommended patterns for “optional CLIs”
-
Always install the command, but gate functionality at runtime
- Keep the console script always present.
- In the CLI entry function, import optional deps lazily and, if missing, print a clear error like: “Install with
yourpkg[cli]”. - Put the real dependencies in
[project.optional-dependencies].cli = [...](extras are designed to add dependencies). [4]
-
Split into two distributions (most robust UX)
- Publish:
yourpkg= core library (no CLI deps)yourpkg-cli= small package that depends onyourpkg+ CLI deps, and defines the console script
- Users who don’t want a CLI never see the command; users who do get a clean install story.
- Publish:
-
Plugin/subcommand architecture via entry points
- Ship a small “base” CLI command.
- Optional features live in separate extra packages that add entry points in a plugin group (so installing the extra package truly adds new functionality/commands).
Sources: PEP 621 on [project.scripts] → console_scripts [1]; Entry points spec on extras being not recommended / ignorable [2]; pip issue showing extras-annotated console scripts still installed [3]; dependency specifiers on extras adding dependencies [4].
🏁 Script executed:
#!/bin/bash
# Find the mcp module
find . -type f -name "mcp.py" -o -path "*/mcp/__init__.py" | head -20Repository: marcelwa/aigverse
Length of output: 94
🏁 Script executed:
cat -n ./python/aigverse/mcp/__init__.pyRepository: marcelwa/aigverse
Length of output: 297
🏁 Script executed:
find . -name "server.py" -path "*/mcp/*" | head -5Repository: marcelwa/aigverse
Length of output: 92
🏁 Script executed:
cat -n ./python/aigverse/mcp/server.pyRepository: marcelwa/aigverse
Length of output: 14594
Console script aigverse-mcp-server will fail at runtime if MCP dependencies are not installed.
[project.scripts] entries are unconditional — users installing aigverse without the [mcp] extra will have the console script wrapper created but it will crash on invocation due to missing fastmcp, httpx, bs4, and markdownify (lines 26–29 import them at module level).
Recommended solutions:
- Move optional imports inside
main()with a clear error message if dependencies are missing. - Split into a separate
aigverse-clidistribution that depends onaigverse + [mcp]extras. - Use an entry-point plugin architecture where the CLI is optional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pyproject.toml` around lines 48 - 49, The console script aigverse-mcp-server
points to aigverse.mcp:main but optional MCP dependencies (fastmcp, httpx, bs4,
markdownify) are imported at module scope and will make the wrapper crash for
users who didn't install the [mcp] extra; fix by moving those optional imports
into the main() function (or into helper functions called by main()), catch
ImportError and print a clear message explaining the missing extras and how to
install them (e.g., "install aigverse[mcp]"), and ensure module-level code no
longer imports these names so the entry-point can be created safely without the
extra dependencies.
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/mcp/test_mcp_server.py (2)
245-251: Remove unused fixture parameter.The
networks_api_htmlfixture is passed but unused (suppressed with# noqa: ARG002). Sincelookup_api_symbolmakes its own HTTP request internally, this parameter provides no caching benefit and can be removed.🧹 Remove unused parameter
- def test_lookup_api_symbol_live(self, networks_api_html) -> None: # noqa: ARG002 + def test_lookup_api_symbol_live(self) -> None: """Public symbol lookup should find a known live API symbol.""" from aigverse.mcp.server import lookup_api_symbol section = lookup_api_symbol("aigverse.networks.Aig") assert "not found" not in section.lower() assert "Aig" in section🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/mcp/test_mcp_server.py` around lines 245 - 251, The test function test_lookup_api_symbol_live is accepting an unused fixture networks_api_html (suppressed with # noqa: ARG002) but lookup_api_symbol does its own HTTP request, so remove the unused fixture: update the test_lookup_api_symbol_live signature to def test_lookup_api_symbol_live() -> None (drop networks_api_html and the noqa), leaving the body that imports and calls lookup_api_symbol unchanged; ensure no other references to networks_api_html remain in this test.
154-160: Consider simplifying the nav exclusion assertion.Line 160's assertion
"nav" not in md.split("Title")[0]is convoluted. If_extract_articlecorrectly extracts only the<article>content, "nav" shouldn't appear inmdat all. A simpler check would be more direct:💡 Suggested simplification
def test_extracts_article_role_main(self, extract) -> None: """Content inside ``<article role='main'>`` is extracted.""" html = "<html><body><nav>nav</nav><article role='main'><h1>Title</h1><p>Body</p></article></body></html>" md = extract(html) assert "Title" in md assert "Body" in md - assert "nav" not in md.split("Title")[0] # nav should not precede title + assert "nav" not in md # nav element should be excluded from article extraction🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/mcp/test_mcp_server.py` around lines 154 - 160, The assertion in test_extracts_article_role_main is overly complex; simplify the nav exclusion by asserting that "nav" does not appear anywhere in the extracted markdown returned by extract (the helper that wraps _extract_article), i.e., replace the current md.split("Title")[0] check with a direct assertion like assert "nav" not in md to more clearly validate that _extract_article removed the navigation content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/mcp/test_mcp_server.py`:
- Around line 245-251: The test function test_lookup_api_symbol_live is
accepting an unused fixture networks_api_html (suppressed with # noqa: ARG002)
but lookup_api_symbol does its own HTTP request, so remove the unused fixture:
update the test_lookup_api_symbol_live signature to def
test_lookup_api_symbol_live() -> None (drop networks_api_html and the noqa),
leaving the body that imports and calls lookup_api_symbol unchanged; ensure no
other references to networks_api_html remain in this test.
- Around line 154-160: The assertion in test_extracts_article_role_main is
overly complex; simplify the nav exclusion by asserting that "nav" does not
appear anywhere in the extracted markdown returned by extract (the helper that
wraps _extract_article), i.e., replace the current md.split("Title")[0] check
with a direct assertion like assert "nav" not in md to more clearly validate
that _extract_article removed the navigation content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c48c7278-8c52-4423-9ae0-1c0ed2b081b5
📒 Files selected for processing (1)
test/mcp/test_mcp_server.py
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/mcp/test_mcp_server.py (1)
244-250: Unused fixture parameter.The
networks_api_htmlfixture is declared as a parameter but not used in the test body. The# noqa: ARG002comment suppresses the linter warning, but consider either:
- Removing the parameter if it serves no purpose, or
- Using the pre-fetched HTML by extracting the symbol from it directly (similar to
test_extract_symbol_from_live_api_page)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/mcp/test_mcp_server.py` around lines 244 - 250, The test test_lookup_api_symbol_live declares the networks_api_html fixture but doesn't use it; either remove the unused parameter from the test signature or use the fixture to drive the assertion: if removing, delete networks_api_html from test_lookup_api_symbol_live to eliminate the unused-argument suppression; if using, parse the provided networks_api_html to extract the expected symbol name (similar to test_extract_symbol_from_live_api_page) and pass that symbol to lookup_api_symbol (or assert the returned section contains the extracted symbol), referencing the test function test_lookup_api_symbol_live and the lookup_api_symbol call to locate where to change the signature or add parsing logic.python/aigverse/mcp/cli.py (1)
12-16: Consider catchingImportErrorinstead ofModuleNotFoundError.The MCP optional dependencies (
httpx,beautifulsoup4,markdownify,fastmcp) may have their own transitive dependencies. If a transitive dependency fails to import (e.g., corrupt install or missing native extension), Python raisesImportErrorrather thanModuleNotFoundError. SinceModuleNotFoundErroris a subclass ofImportError, catching the parent class handles both cases and provides a friendlier error message.Proposed fix
try: from aigverse.mcp.server import main as server_main - except ModuleNotFoundError as exc: + except ImportError as exc: msg = 'The MCP server requires optional dependencies. Install them with: pip install "aigverse[mcp]"' raise SystemExit(msg) from exc🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/aigverse/mcp/cli.py` around lines 12 - 16, Change the exception handler around the import of aigverse.mcp.server (the block that imports server_main) to catch ImportError instead of ModuleNotFoundError so transitive dependency import failures are also caught; update the except clause to "except ImportError as exc" and keep the existing SystemExit message and chaining (raise SystemExit(msg) from exc).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@python/aigverse/mcp/cli.py`:
- Around line 12-16: Change the exception handler around the import of
aigverse.mcp.server (the block that imports server_main) to catch ImportError
instead of ModuleNotFoundError so transitive dependency import failures are also
caught; update the except clause to "except ImportError as exc" and keep the
existing SystemExit message and chaining (raise SystemExit(msg) from exc).
In `@test/mcp/test_mcp_server.py`:
- Around line 244-250: The test test_lookup_api_symbol_live declares the
networks_api_html fixture but doesn't use it; either remove the unused parameter
from the test signature or use the fixture to drive the assertion: if removing,
delete networks_api_html from test_lookup_api_symbol_live to eliminate the
unused-argument suppression; if using, parse the provided networks_api_html to
extract the expected symbol name (similar to
test_extract_symbol_from_live_api_page) and pass that symbol to
lookup_api_symbol (or assert the returned section contains the extracted
symbol), referencing the test function test_lookup_api_symbol_live and the
lookup_api_symbol call to locate where to change the signature or add parsing
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 15bef27b-e2fc-4321-b6ba-ee7ee3a4db48
📒 Files selected for processing (9)
README.mddocs/conf.pydocs/mcp.mdpyproject.tomlpython/aigverse/mcp/__init__.pypython/aigverse/mcp/__main__.pypython/aigverse/mcp/cli.pypython/aigverse/mcp/server.pytest/mcp/test_mcp_server.py
✅ Files skipped from review due to trivial changes (3)
- docs/conf.py
- README.md
- python/aigverse/mcp/server.py
🚧 Files skipped from review as they are similar to previous changes (3)
- python/aigverse/mcp/main.py
- python/aigverse/mcp/init.py
- pyproject.toml
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/mcp/test_mcp_server.py (2)
29-33: Don’t codify success for unknown CLI flags without confirming contract.Line 32 currently expects
returncode == 0for--no-such-flag. That’s non-idiomatic for CLIs and can hide argument-validation regressions; please verify intended behavior and assert failure if unknown flags should be rejected.Suggested assertion shape
def test_invalid_flag_does_not_crash(self, script_runner) -> None: """An unrecognised flag should not crash the entry point.""" result = script_runner.run(["aigverse-mcp-server", "--no-such-flag"]) - assert result.returncode == 0 - assert "Starting MCP server 'aigverse'" in result.stderr + assert result.returncode != 0 + assert "--no-such-flag" in (result.stderr + result.stdout)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/mcp/test_mcp_server.py` around lines 29 - 33, The test test_invalid_flag_does_not_crash currently asserts result.returncode == 0 for an unknown CLI flag; confirm the intended CLI contract for aigverse-mcp-server (whether unknown flags should be accepted or cause an error) and update the test accordingly: if unknown flags should be rejected, change the assertion to expect a non-zero return code and assert an appropriate error message appears in result.stderr; if they should be ignored, keep the zero return code but also assert presence of a help/acknowledgement message to make intent explicit. Locate and modify test_invalid_flag_does_not_crash in test_mcp_server.py to reflect the agreed contract.
244-244: Remove the unused fixture parameter intest_lookup_api_symbol_live.Line 244 adds
networks_api_htmlonly to suppressARG002, but the value is unused and adds unnecessary fixture work/noise. Prefer dropping the parameter and the inline suppression.Small cleanup
- def test_lookup_api_symbol_live(self, networks_api_html) -> None: # noqa: ARG002 + def test_lookup_api_symbol_live(self) -> None:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/mcp/test_mcp_server.py` at line 244, The test function test_lookup_api_symbol_live currently declares an unused fixture parameter networks_api_html and suppresses ARG002; remove the unused parameter from the function signature and delete the inline "# noqa: ARG002" so the test reads def test_lookup_api_symbol_live(self) -> None:, ensuring no other references to networks_api_html remain in that function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/mcp/test_mcp_server.py`:
- Around line 29-33: The test test_invalid_flag_does_not_crash currently asserts
result.returncode == 0 for an unknown CLI flag; confirm the intended CLI
contract for aigverse-mcp-server (whether unknown flags should be accepted or
cause an error) and update the test accordingly: if unknown flags should be
rejected, change the assertion to expect a non-zero return code and assert an
appropriate error message appears in result.stderr; if they should be ignored,
keep the zero return code but also assert presence of a help/acknowledgement
message to make intent explicit. Locate and modify
test_invalid_flag_does_not_crash in test_mcp_server.py to reflect the agreed
contract.
- Line 244: The test function test_lookup_api_symbol_live currently declares an
unused fixture parameter networks_api_html and suppresses ARG002; remove the
unused parameter from the function signature and delete the inline "# noqa:
ARG002" so the test reads def test_lookup_api_symbol_live(self) -> None:,
ensuring no other references to networks_api_html remain in that function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9edb3611-0a89-4aad-be4e-ea666712dc20
📒 Files selected for processing (2)
python/aigverse/mcp/server.pytest/mcp/test_mcp_server.py
✅ Files skipped from review due to trivial changes (1)
- python/aigverse/mcp/server.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@python/aigverse/mcp/server.py`:
- Line 27: Import urljoin from urllib.parse and use it when assembling the
ReadTheDocs hit URL instead of concatenating paths; e.g., replace the manual
join that produced "https://...//en/latest/..." with
urljoin(f"{parsed.scheme}://{parsed.netloc}", path) where path is the returned
hit["path"]. Also update highlight extraction in the search result processing to
read block["highlights"]["content"] (a list) rather than calling .get() on
block["content"]; aggregate those strings into your result highlights (e.g.,
extend a highlights list with block["highlights"]["content"] for each block).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: caadad0e-fa8b-450a-a7dd-45afc86e1312
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
python/aigverse/mcp/server.pytest/mcp/test_mcp_server.py
Co-authored-by: Copilot <copilot@github.com>
…n depending on the configuration Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
fastmcp's @mcp.tool() decorator wraps functions in a FunctionTool object. ty 0.0.24 does not resolve FunctionTool.__call__, so every direct call to a tool in the test suite is flagged as call-non-callable. These calls are valid at runtime. Suppress the diagnostics with targeted ty: ignore comments until fastmcp ships proper __call__ type annotations.
An adversarial agent could supply a path-traversal slug such as '../../other-project/page' to fetch arbitrary ReadTheDocs pages outside the aigverse documentation. Add an explicit allowlist guard that rejects any slug that is not a known guide page or an api/aigverse/* path.
Bare noqa suppressions give auditors no context for why the Bandit S310 (audit-url-open) rule is safe to skip. Add an inline note on each suppressed line explaining that the URL is validated to HTTPS-only by the caller before the urllib fallback path is reached.
The module-level httpx.Client holds a connection pool that was never explicitly closed, causing resource leaks in long-running server processes. Add a _server_lifespan async context manager and pass it to FastMCP so the client is closed cleanly on shutdown. asyncio.to_thread is used to avoid blocking the event loop on the synchronous httpx.Client.close() call.
…requests When lookup_api_symbol receives an unqualified symbol name it scans up to seven submodule pages. Stable-docs pages are immutable for a given release, so fetching the same URL repeatedly across calls is wasteful. Introduce _fetch_page_cached (backed by functools.lru_cache) and use it for stable-version lookups. Latest-docs remain uncached since they may change during active development.
Previously search_documentation returned a JSON array on success but
plain strings for errors and empty results, forcing callers to guess
the shape. Now all paths return a JSON string:
- Success: JSON array of result objects [{title, url, highlights}, ...]
- Empty results: {"results": [], "message": "No results found for '...'"}
- HTTP errors: {"error": "Search failed: HTTP <status>"}
- Other errors: {"error": "Search failed: <message>"}
…i_symbol The dict.fromkeys(...).values() idiom is not immediately obvious to readers unfamiliar with the pattern. Add a brief inline note explaining that it is used to deduplicate while preserving insertion order.
fastmcp 2.x FunctionTool does not implement __call__; the underlying function must be accessed via the .fn attribute. Replace all direct tool calls in the test suite with .fn() and remove the now-redundant ty: ignore[call-non-callable] comments. Also adds TestLookupApiSymbol with four unit tests (no network): - Short symbol name found via stable-docs cache path - Fully qualified name narrows fetch to the correct submodule - Missing symbol returns a human-readable error containing the name - Invalid version string is rejected before any network call
The log message 'Starting MCP server \'aigverse\'' is an internal FastMCP implementation detail that can change with library upgrades. Asserting on exit code alone is sufficient to verify the entry point starts up and handles flags without crashing.
Rename _url to url and assert its value in FakeClient.get() so that any accidental change to the ReadTheDocs search endpoint will be caught immediately by the unit test rather than silently going unnoticed.
Verifies that when the ReadTheDocs API returns an empty result list the tool returns a JSON envelope containing an empty 'results' array and a human-readable 'message' key, consistent with the JSON-envelope contract introduced in the earlier search consistency fix.
…Article Replace the position-based 'nav not in md.split(Title)[0]' assertion with a unique sentinel value that can appear nowhere else in the output. This makes the assertion independent of Markdown serialisation order and immune to converter-specific whitespace or heading formatting changes.
The search_documentation tool accepts a max_results parameter that controls how many results are returned (default 5). Add this to the Tools reference table so users and LLM agents are aware of the option.
fastmcp 2.x wrapped @mcp.tool()-decorated functions in a FunctionTool object that did not implement __call__, breaking both direct test calls and ty's type-checking. fastmcp 3.0.0 changed the decorator to return the original function transparently, so the tools are plain callables: ty is satisfied without any suppressions and tests call them directly. Changes: - Raise fastmcp pin from '>=2.0.0,<3.0.0' to '>=3.0.0' in both the project optional-dependency and the dev dependency-group - Update uv.lock to fastmcp 3.2.4 - Remove the .fn() indirection added as a 2.x workaround in all 11 call sites across the test suite, reverting to plain direct calls - Fix FakeEmptyClient.get() to accept keyword arguments via **_kwargs so the positional _params rename no longer conflicts with the server's params= keyword call
Co-authored-by: Copilot <copilot@github.com>
MCP clients that accept JSON config (e.g. Claude Desktop) require the command and its arguments to be specified as separate fields. The previous docs only showed the bare 'aigverse-mcp-server' form and mentioned uvx solely as a prose tip, leaving users to guess the correct JSON structure. - Claude Desktop tab: add a second JSON block with the uvx form, explaining that command and args must be split when using JSON config - Generic (stdio) tab: add the equivalent JSON snippet alongside the bash command form - Tip: extend to explicitly state the command/args decomposition
Claude Code (the claude CLI) is the primary Anthropic coding agent and the most likely client for aigverse users. Add it as the first tab in the Client Configuration section, documenting: - claude mcp add (the idiomatic registration command) - uvx fallback for environments without aigverse-mcp-server on PATH - --scope project to share via .mcp.json in version control - The resulting JSON config in both direct and uvx forms The command/args JSON format is verified against the official MCP quickstart (Claude Desktop) and Claude Code docs: stdio servers use 'command' + 'args', the same shape used in .mcp.json and claude_desktop_config.json.
The integration tests appeared 'stuck' for two reasons: 1. ReadTheDocs blocks httpx (TLS fingerprint / bot detection) with 403 on every request, so _fetch_page always falls back to urllib. The stable API index page (297 KB) takes ~30 s via urllib; latest-docs pages take a similar ~30 s each. With three such fetches the suite exceeded any reasonable timeout. 2. The networks_api_html class fixture called _fetch_page directly, which does NOT populate the lru_cache used by _fetch_page_cached. So test_lookup_api_symbol_live triggered a redundant 30 s download of the same 297 KB page that the fixture had already fetched. Fix: switch the fixture to call _fetch_page_cached so that the lru_cache is warm when lookup_api_symbol runs. This cuts one redundant 30 s round- trip from the suite. Also add a session-scoped autouse fixture (_teardown_mcp_http_resources) that closes the module-level httpx.Client and clears the lru_cache after all tests finish. Without this, _server_lifespan (which normally handles cleanup) is never invoked because tests call tool functions directly and never start the MCP server process.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add an MCP server that enables LLM agents to fetch token-efficient
aigversedocumentation.Checklist:
Summary by CodeRabbit
New Features
Documentation
Tests
Chores