fix: apply benchmark-informed deep code review findings (2 critical, 6 warning, 5 info)

Benchmarked against top Python MCP server projects (PrefectHQ/fastmcp,
awslabs/mcp, Anthropic reference servers) and official MCP specification.

Critical:
- CR-01: Fix synonym substring matching false positives (word-boundary regex)
- CR-02: Catch all exceptions in tool handlers as ToolError (MCP spec compliance)

Warning:
- WR-01: Add max_length=500 to query/slug string inputs
- WR-02: Move version validation before DB writes in ingest_inventory
- WR-03: Add check_same_thread=False for concurrent async dispatch safety
- WR-05: Use inspect.signature for robust version extraction in observability
- WR-06: Clearer logging for partial (symbols-only) ingestion

Info:
- IN-01: Replace f-string logging with lazy %-style formatting
- IN-02: Add idempotentHint=True to ToolAnnotations
- IN-03: Add py.typed PEP 561 marker file
- IN-04: Fix inaccurate services/__init__.py docstring
- IN-05: Make AppContext service fields non-optional (enforces runtime invariant)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: ayhammouda

src/mcp_server_python_docs/__main__.py

@@ -238,8 +238,8 @@ def build_index(versions: str, skip_content: bool) -> None:
                             result.stderr[-2000:] if result.stderr else "(no output)",
                         )
                         logger.warning(
-                            "Symbols were ingested for %s but sections/examples "
-                            "will be missing. Smoke tests may fail.",
+                            "Version %s has SYMBOLS ONLY (sphinx-build failed). "
+                            "search_docs will work but get_docs will return empty pages.",
                             version,
                         )
                         any_version_succeeded = True

src/mcp_server_python_docs/app_context.py

@@ -9,12 +9,10 @@
 import sqlite3
 from dataclasses import dataclass, field
 from pathlib import Path
-from typing import TYPE_CHECKING
 
-if TYPE_CHECKING:
-    from mcp_server_python_docs.services.content import ContentService
-    from mcp_server_python_docs.services.search import SearchService
-    from mcp_server_python_docs.services.version import VersionService
+from mcp_server_python_docs.services.content import ContentService
+from mcp_server_python_docs.services.search import SearchService
+from mcp_server_python_docs.services.version import VersionService
 
 
 @dataclass
@@ -23,7 +21,7 @@ class AppContext:
 
     db: sqlite3.Connection
     index_path: Path
+    search_service: SearchService
+    content_service: ContentService
+    version_service: VersionService
     synonyms: dict[str, list[str]] = field(default_factory=dict)
-    search_service: SearchService | None = None
-    content_service: ContentService | None = None
-    version_service: VersionService | None = None

src/mcp_server_python_docs/ingestion/inventory.py

@@ -86,6 +86,14 @@ def ingest_inventory(
     5. Handle duplicates via priority ordering (INGR-I-05)
     6. Populate symbols_fts in same transaction (INGR-I-06)
     """
+    # Validate version format first, before any DB writes (WR-02)
+    import re
+
+    if not re.match(r"^\d+\.\d+$", version):
+        from mcp_server_python_docs.errors import IngestionError
+
+        raise IngestionError(f"Invalid version format: {version!r}")
+
     bootstrap_schema(conn)
 
     # Upsert doc_set for this version
@@ -111,19 +119,11 @@ def ingest_inventory(
     # Clear existing symbols for this doc_set (re-ingestion support)
     conn.execute("DELETE FROM symbols WHERE doc_set_id = ?", (doc_set_id,))
 
-    # Validate version format before URL construction (WR-04)
-    import re
-
-    if not re.match(r"^\d+\.\d+$", version):
-        from mcp_server_python_docs.errors import IngestionError
-
-        raise IngestionError(f"Invalid version format: {version!r}")
-
     # Download and parse objects.inv (INGR-I-01)
     url = f"https://docs.python.org/{version}/objects.inv"
-    logger.info(f"Downloading {url}...")
+    logger.info("Downloading %s...", url)
     inv = soi.Inventory(url=url)  # type: ignore[call-arg]  # sphobjinv lacks type stubs
-    logger.info(f"Downloaded {len(inv.objects)} inventory objects")
+    logger.info("Downloaded %d inventory objects", len(inv.objects))
 
     # Filter to Python domain objects and collect by qualified_name
     # for duplicate resolution (INGR-I-05)
@@ -174,5 +174,5 @@ def ingest_inventory(
     conn.execute("INSERT INTO symbols_fts(symbols_fts) VALUES('rebuild')")
     conn.commit()
 
-    logger.info(f"Ingested {count} symbols for Python {version}")
+    logger.info("Ingested %d symbols for Python %s", count, version)
     return count

src/mcp_server_python_docs/models.py

@@ -16,7 +16,8 @@ class SearchDocsInput(BaseModel):
     """Input parameters for search_docs tool."""
 
     query: str = Field(
-        description="Search query - Python symbol (asyncio.TaskGroup) or concept (parse json)"
+        max_length=500,
+        description="Search query - Python symbol (asyncio.TaskGroup) or concept (parse json)",
     )
     version: str | None = Field(
         default=None,
@@ -69,7 +70,10 @@ class SearchDocsResult(BaseModel):
 class GetDocsInput(BaseModel):
     """Input parameters for get_docs tool."""
 
-    slug: str = Field(description="Page slug (e.g. 'library/asyncio-task.html')")
+    slug: str = Field(
+        max_length=500,
+        description="Page slug (e.g. 'library/asyncio-task.html')",
+    )
     version: str | None = Field(
         default=None,
         description="Python version. Defaults to latest.",

src/mcp_server_python_docs/py.typed

@@ -86,20 +86,35 @@ def classify_query(
     return "fts"
 
 
+def _build_concept_patterns(
+    synonyms: dict[str, list[str]],
+) -> dict[str, re.Pattern[str]]:
+    """Pre-compile word-boundary regex patterns for multi-word synonym concepts."""
+    patterns: dict[str, re.Pattern[str]] = {}
+    for concept in synonyms:
+        if " " in concept:
+            patterns[concept] = re.compile(r"\b" + re.escape(concept) + r"\b")
+    return patterns
+
+
 def expand_synonyms(
     query: str,
     synonyms: dict[str, list[str]],
+    *,
+    _concept_patterns: dict[str, re.Pattern[str]] | None = None,
 ) -> set[str]:
     """Expand query using synonym table for concept search (RETR-05).
 
-    Checks each token and the full query against the synonym table.
-    Returns a set of terms including the original tokens plus any
-    expansion values.
+    Checks each token and multi-word phrases against the synonym table.
+    Single-word concepts use exact token matching. Multi-word concepts
+    use word-boundary regex to avoid substring false positives (CR-01).
 
     Args:
         query: User search query.
         synonyms: Mapping of concept -> list of expansion terms.
             Loaded from synonyms.yaml at startup.
+        _concept_patterns: Pre-compiled regex patterns for multi-word
+            concepts. Built lazily on first call if not provided.
 
     Returns:
         Set of terms (original + expansions). Empty set if query
@@ -112,15 +127,20 @@ def expand_synonyms(
     tokens = query.lower().split()
     expanded = set(tokens)
 
-    # Check individual tokens against synonym keys
+    # Single-word concepts: exact token match (no substring false positives)
     for token in tokens:
         if token in synonyms:
             expanded.update(synonyms[token])
 
-    # Check multi-word concepts (e.g., "http requests", "file io")
+    # Multi-word concepts: word-boundary regex match (CR-01 fix)
+    if _concept_patterns is None:
+        _concept_patterns = _build_concept_patterns(synonyms)
     query_lower = query.lower()
     for concept, expansions in synonyms.items():
-        if concept in query_lower:
+        if " " not in concept:
+            continue
+        pattern = _concept_patterns.get(concept)
+        if pattern and pattern.search(query_lower):
             expanded.update(expansions)
 
     return expanded
@@ -129,6 +149,8 @@ def expand_synonyms(
 def build_match_expression(
     query: str,
     synonyms: dict[str, list[str]],
+    *,
+    _concept_patterns: dict[str, re.Pattern[str]] | None = None,
 ) -> str:
     """Build a complete FTS5 MATCH expression with synonym expansion.
 
@@ -138,6 +160,7 @@ def build_match_expression(
     Args:
         query: User search query.
         synonyms: Synonym mapping for expansion.
+        _concept_patterns: Pre-compiled regex patterns for multi-word concepts.
 
     Returns:
         FTS5-safe MATCH expression string.
@@ -146,7 +169,7 @@ def build_match_expression(
     if not query:
         return '""'
 
-    expanded = expand_synonyms(query, synonyms)
+    expanded = expand_synonyms(query, synonyms, _concept_patterns=_concept_patterns)
     original_tokens = set(query.lower().split())
 
     # If expansion added new terms, OR-join all terms

src/mcp_server_python_docs/retrieval/query.py

@@ -72,7 +72,7 @@ async def app_lifespan(server: FastMCP) -> AsyncIterator[AppContext]:
     logger.info("Loaded %d synonym entries", len(synonyms))
 
     # Open read-only connection (STOR-06, STOR-07)
-    db = sqlite3.connect(f"file:{index_path}?mode=ro", uri=True)
+    db = sqlite3.connect(f"file:{index_path}?mode=ro", uri=True, check_same_thread=False)
     db.execute("PRAGMA journal_mode = WAL")
     db.execute("PRAGMA synchronous = NORMAL")
     db.execute("PRAGMA foreign_keys = ON")
@@ -113,6 +113,7 @@ async def app_lifespan(server: FastMCP) -> AsyncIterator[AppContext]:
 _TOOL_ANNOTATIONS = ToolAnnotations(
     readOnlyHint=True,
     destructiveHint=False,
+    idempotentHint=True,
     openWorldHint=False,
 )
 
@@ -141,6 +142,9 @@ def search_docs(
             return app_ctx.search_service.search(query, version, kind, max_results)
         except DocsServerError as e:
             raise ToolError(str(e))
+        except Exception as e:
+            logger.exception("Unexpected error in search_docs")
+            raise ToolError(f"Internal error: {type(e).__name__}")
 
     @mcp.tool(annotations=_TOOL_ANNOTATIONS)
     def get_docs(
@@ -160,6 +164,9 @@ def get_docs(
             )
         except DocsServerError as e:
             raise ToolError(str(e))
+        except Exception as e:
+            logger.exception("Unexpected error in get_docs")
+            raise ToolError(f"Internal error: {type(e).__name__}")
 
     @mcp.tool(annotations=_TOOL_ANNOTATIONS)
     def list_versions(
@@ -171,6 +178,9 @@ def list_versions(
             return app_ctx.version_service.list_versions()
         except DocsServerError as e:
             raise ToolError(str(e))
+        except Exception as e:
+            logger.exception("Unexpected error in list_versions")
+            raise ToolError(f"Internal error: {type(e).__name__}")
 
     # SRVR-07: _meta hint for get_docs tool.
     # FastMCP 1.27 does not expose a public API for setting _meta on tool

src/mcp_server_python_docs/server.py

@@ -2,6 +2,6 @@
 
 Services sit between FastMCP tool handlers and the retrieval/storage layers.
 Dependency rule: server -> services -> retrieval/storage.
-No service touches SQL directly (except through storage/retrieval functions).
+Services receive sqlite3.Connection via constructor and execute queries directly.
 No service imports MCP types.
 """

src/mcp_server_python_docs/services/__init__.py

@@ -10,6 +10,7 @@
 from __future__ import annotations
 
 import functools
+import inspect
 import sys
 import time
 from collections.abc import Callable
@@ -66,13 +67,14 @@ def wrapper(self: Any, *args: Any, **kwargs: Any) -> Any:
                 "latency_ms": round(elapsed_ms, 1),
             }
 
-            # Extract version from kwargs or positional args
-            version_val = kwargs.get("version")
-            if version_val is None and args:
-                # For search: (query, version, kind, max_results)
-                # For get_docs: (slug, version, anchor, ...)
-                if len(args) >= 2:
-                    version_val = args[1]
+            # Extract version by name via inspect (WR-05: avoids fragile positional indexing)
+            try:
+                sig = inspect.signature(fn)
+                bound = sig.bind(self, *args, **kwargs)
+                bound.apply_defaults()
+                version_val = bound.arguments.get("version")
+            except (TypeError, ValueError):
+                version_val = kwargs.get("version")
             fields["version"] = version_val or "default"
 
             # Extract result-specific fields

src/mcp_server_python_docs/services/observability.py

@@ -41,7 +41,7 @@ def get_readonly_connection(path: str | Path) -> sqlite3.Connection:
     Uses SQLite URI mode with ?mode=ro to prevent accidental writes.
     """
     path = Path(path)
-    conn = sqlite3.connect(f"file:{path}?mode=ro", uri=True)
+    conn = sqlite3.connect(f"file:{path}?mode=ro", uri=True, check_same_thread=False)
     _set_pragmas(conn)
     conn.row_factory = sqlite3.Row
     return conn