diff --git a/benchmarks/adapters/python_docs_mcp_adapter.py b/benchmarks/adapters/python_docs_mcp_adapter.py new file mode 100644 index 0000000..9969855 --- /dev/null +++ b/benchmarks/adapters/python_docs_mcp_adapter.py @@ -0,0 +1,259 @@ +"""Offline stdio adapter for python-docs-mcp-server itself (issue #86). + +Runs the product's own MCP server as a benchmark system under test: spawns +it as a local subprocess over stdio, issues the documented retrieval flow +(``search_docs`` -> ``get_docs``) for one corpus question, and returns the +retrieved content plus the raw tool call/response payloads for transcript +recording. + +This adapter is intentionally **not** a +``benchmarks.adapters.base.ProviderAdapter`` subclass: that contract models +an LLM provider call (prompt in, generated answer + token usage out). This +adapter models a documentation *retrieval* tool call instead -- there is no +LLM in this offline work package. Pairing the retrieved context with an LLM +adapter (``benchmarks.adapters.openai_adapter`` / ``google_adapter``, issue +#73) is reserved for a maintainer-run live phase; see the confirmed +composition decision on issue #86 (also PLAN.md Amendment 2026-07-08). + +Offline guarantee (roadmap principle 2.2, preserved at runtime): + +- This module never performs network I/O itself. +- The spawned server process always runs with + ``PYTHON_DOCS_MCP_DISABLE_AUTO_INDEX=1`` so a missing index can never + trigger the server's own auto-index-build fallback + (``mcp_server_python_docs.server._auto_build_symbol_index``), which does + fetch network resources. +- A missing index is instead detected *before* the subprocess is even + spawned (see :meth:`PythonDocsMcpAdapter.run`) and reported as a + ``tool_failure``, never a crash. +- ``search_docs`` and ``get_docs`` themselves only ever read the local + SQLite index; neither performs network I/O (see + ``mcp_server_python_docs.services.search`` / ``services.content``). +""" + +from __future__ import annotations + +import asyncio +import os +import sys +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Protocol + +from benchmarks.runner import BenchmarkCellFailure + +#: Environment variable the server checks to skip its own auto-index-build +#: fallback (see ``mcp_server_python_docs.server._auto_build_symbol_index``). +#: Always set for benchmark-spawned servers so a missing index can never +#: trigger a network-touching auto-build. +DISABLE_AUTO_INDEX_ENV = "PYTHON_DOCS_MCP_DISABLE_AUTO_INDEX" + +#: Default per-cell budget for the full search_docs -> get_docs round trip. +DEFAULT_TIMEOUT_SECONDS = 30.0 + +#: Default number of search_docs hits requested; only the top hit is +#: followed up with get_docs (see module docstring "documented retrieval +#: flow"). +DEFAULT_MAX_RESULTS = 3 + + +@dataclass(frozen=True) +class ToolCallRecord: + """One raw MCP tool call/response pair, recorded verbatim for audit.""" + + tool: str + arguments: dict[str, Any] + result: dict[str, Any] | None + is_error: bool + + +@dataclass(frozen=True) +class PythonDocsMcpResult: + """Structured output of one search_docs -> get_docs cell run.""" + + answer: str + tool_calls: list[ToolCallRecord] + + +class _CallToolResult(Protocol): + """Structural subset of ``mcp.types.CallToolResult`` this module needs. + + Naming this as a ``Protocol`` (rather than importing the concrete SDK + type at module scope for the type hint) lets unit tests hand in a + lightweight stand-in without constructing a full MCP session. + """ + + content: list[Any] + structuredContent: dict[str, Any] | None + isError: bool + + +class _CallToolSession(Protocol): + """Structural subset of ``mcp.ClientSession`` this module needs.""" + + async def call_tool( + self, name: str, arguments: dict[str, Any] | None = None + ) -> _CallToolResult: + """Invoke one MCP tool by name and return its raw call result.""" + raise NotImplementedError + + +def _default_command() -> list[str]: + return [sys.executable, "-m", "mcp_server_python_docs", "serve"] + + +def _default_index_path() -> Path: + from mcp_server_python_docs.storage.db import get_index_path + + return get_index_path() + + +def _first_text(result: _CallToolResult) -> str | None: + for block in result.content: + text = getattr(block, "text", None) + if isinstance(text, str): + return text + return None + + +def _tool_result_payload(result: _CallToolResult) -> dict[str, Any] | None: + """Extract a JSON-safe raw payload from a tool call result for the transcript. + + Prefers ``structuredContent`` (FastMCP auto-generates this from the + tool's Pydantic return model). Falls back to the first text content + block (e.g. on an error result, where ``structuredContent`` is + ``None``) so the transcript still records something useful for audit. + """ + if result.structuredContent is not None: + return result.structuredContent + text = _first_text(result) + return {"text": text} if text is not None else None + + +async def _run_retrieval_flow( + session: _CallToolSession, + prompt: str, + *, + max_results: int = DEFAULT_MAX_RESULTS, +) -> PythonDocsMcpResult: + """Run the documented search_docs -> get_docs flow for one prompt. + + Pure orchestration logic against a duck-typed ``session`` (only + ``call_tool`` is used) so it can be unit tested with a fake/stubbed + transport -- no real server spawn required. Raises + ``BenchmarkCellFailure`` (category ``tool_failure``) when either tool + call itself reports an error. + """ + tool_calls: list[ToolCallRecord] = [] + + search_args: dict[str, Any] = {"query": prompt, "max_results": max_results} + search_result = await session.call_tool("search_docs", search_args) + search_payload = _tool_result_payload(search_result) + tool_calls.append( + ToolCallRecord( + tool="search_docs", + arguments=search_args, + result=search_payload, + is_error=bool(search_result.isError), + ) + ) + if search_result.isError: + raise BenchmarkCellFailure( + "tool_failure", + f"search_docs returned an error: {_first_text(search_result)}", + ) + + hits = (search_payload or {}).get("hits") or [] + if not hits: + return PythonDocsMcpResult(answer="", tool_calls=tool_calls) + + top_hit = hits[0] + get_args: dict[str, Any] = {"slug": top_hit.get("slug"), "version": top_hit.get("version")} + if top_hit.get("anchor"): + get_args["anchor"] = top_hit["anchor"] + get_result = await session.call_tool("get_docs", get_args) + get_payload = _tool_result_payload(get_result) + tool_calls.append( + ToolCallRecord( + tool="get_docs", + arguments=get_args, + result=get_payload, + is_error=bool(get_result.isError), + ) + ) + if get_result.isError: + raise BenchmarkCellFailure( + "tool_failure", + f"get_docs returned an error: {_first_text(get_result)}", + ) + + answer = str((get_payload or {}).get("content", "")) + return PythonDocsMcpResult(answer=answer, tool_calls=tool_calls) + + +class PythonDocsMcpAdapter: + """Offline stdio adapter: python-docs-mcp-server as a retrieval tool. + + ``run()`` is the synchronous entry point the benchmark runner's + dispatch registry calls per cell (see + ``benchmarks.runner._python_docs_mcp_stdio_answer``). + """ + + def __init__( + self, + *, + command: list[str] | None = None, + index_path: Path | None = None, + timeout_seconds: float = DEFAULT_TIMEOUT_SECONDS, + max_results: int = DEFAULT_MAX_RESULTS, + ) -> None: + self._command = command if command is not None else _default_command() + self._index_path = index_path + self._timeout_seconds = timeout_seconds + self._max_results = max_results + + def run(self, prompt: str) -> PythonDocsMcpResult: + """Run one search_docs -> get_docs cell. + + Raises ``BenchmarkCellFailure`` with category ``tool_failure`` + (missing index or a tool-level error), ``timeout`` (the round trip + exceeded ``timeout_seconds``), or ``mcp_protocol_crash`` (the stdio + transport/session itself failed) -- never a bare exception, so the + runner's existing except clause for adapter dispatch (see + ``benchmarks.runner._execute_cell``) can catch this uniformly. + """ + index_path = self._index_path if self._index_path is not None else _default_index_path() + if not index_path.exists(): + raise BenchmarkCellFailure( + "tool_failure", + f"no local index found at {index_path}; run 'python-docs-mcp-server " + "build-index --versions ' before running this benchmark adapter " + "(dry-run validates without needing one)", + ) + try: + return asyncio.run(asyncio.wait_for(self._run_async(prompt), self._timeout_seconds)) + except BenchmarkCellFailure: + raise + except TimeoutError as exc: + raise BenchmarkCellFailure( + "timeout", + f"python-docs-mcp-server did not complete search_docs -> get_docs within " + f"{self._timeout_seconds}s", + ) from exc + except Exception as exc: # noqa: BLE001 - failures are benchmark artifacts + raise BenchmarkCellFailure("mcp_protocol_crash", str(exc)) from exc + + async def _run_async(self, prompt: str) -> PythonDocsMcpResult: + # Imported lazily so importing this module never requires the `mcp` + # client submodules unless an actual run is attempted. + from mcp import ClientSession, StdioServerParameters + from mcp.client.stdio import stdio_client + + env = {**os.environ, DISABLE_AUTO_INDEX_ENV: "1"} + params = StdioServerParameters( + command=self._command[0], args=list(self._command[1:]), env=env + ) + async with stdio_client(params) as (read, write): + async with ClientSession(read, write) as session: + await session.initialize() + return await _run_retrieval_flow(session, prompt, max_results=self._max_results) diff --git a/benchmarks/model_matrix.py b/benchmarks/model_matrix.py index d1d8306..fbd498d 100644 --- a/benchmarks/model_matrix.py +++ b/benchmarks/model_matrix.py @@ -6,13 +6,17 @@ existing competitor manifest consumed by ``benchmarks.runner``. This module intentionally does not execute anything against -``benchmarks.runner``. Wiring model-matrix cells into the runner's per-cell -execution path (so a manifest competitor can be paired with a specific model -family end to end) is deferred to a follow-up issue once the maintainer -confirms how that composition should affect the artifact shapes already -covered by ``tests/benchmarks/test_runner.py`` -- this package only defines -the matrix, the cross-multiplication contract, and mocked provider adapters -(see ``benchmarks.adapters``). +``benchmarks.runner``. The maintainer confirmed (issue #86, 2026-07-08) how +tool x model composition should affect the artifact shapes already covered +by ``tests/benchmarks/test_runner.py``: cell composition stays competitor x +question (no cell-shape or artifact-layout change); a manifest instead +enumerates one entry per tool x model pairing, and this module gains a +manifest<->matrix validator (:func:`validate_manifest_against_matrix`) so a +manifest entry's declared provider/model pairing can be checked against +this file before a run. Wiring model-matrix cells directly into the +runner's per-cell execution path remains out of scope. This package +defines the matrix, the cross-multiplication contract, the manifest<->matrix +validator, and mocked provider adapters (see ``benchmarks.adapters``). """ from __future__ import annotations @@ -23,7 +27,7 @@ import yaml -from benchmarks.runner import BenchmarkValidationError +from benchmarks.runner import BenchmarkValidationError, Competitor #: Label for tokens counted by the benchmark's own methodology tokenizer #: (Claude token counting after client-side rewrap, per @@ -193,3 +197,44 @@ def tool_model_cells( for competitor_id in competitor_ids for family in model_families ] + + +def validate_manifest_against_matrix( + competitors: list[Competitor], matrix: ModelMatrix +) -> None: + """Validate a competitor manifest's tool x model pairings against the matrix. + + Per the confirmed composition decision (issue #86, 2026-07-08): the + competitor manifest enumerates one entry per tool x model pairing, so a + manifest entry that declares both a ``provider`` and a ``model`` field + (the pairing this validator checks) must correspond to a + ``docs/benchmarks/model-matrix.yml`` entry with that exact + ``(provider, model_id)`` combination. Raises + ``BenchmarkValidationError`` naming the offending competitor id and + pairing when no match is found. + + Manifest entries that omit ``provider`` or ``model`` (e.g. the + model-agnostic no-MCP baseline) are not a tool x model pairing and are + skipped -- this validator only checks pairings a manifest actually + declares. + + This function is not invoked automatically by ``benchmarks.runner``; + callers (a CLI, a maintainer script, or a test) run it explicitly + before a benchmark run. See this module's docstring for why the + runner's per-cell dispatch does not consume the model matrix directly. + """ + known_pairs = {(family.provider, family.model_id) for family in matrix.model_families} + for competitor in competitors: + provider = competitor.raw.get("provider") + model = competitor.raw.get("model") + has_provider = isinstance(provider, str) and bool(provider) + has_model = isinstance(model, str) and bool(model) + if not (has_provider and has_model): + continue + if (provider, model) not in known_pairs: + raise BenchmarkValidationError( + f"competitor {competitor.id!r} declares provider/model pairing " + f"{provider}/{model} that is not present in the model matrix; add a " + "matching model_families entry to docs/benchmarks/model-matrix.yml or " + "fix the manifest" + ) diff --git a/benchmarks/report.py b/benchmarks/report.py index 86f25f9..3941777 100644 --- a/benchmarks/report.py +++ b/benchmarks/report.py @@ -23,12 +23,19 @@ files are missing, so a README-ready block can never be produced from an incomplete run. -Known gap inherited from issue #73/#74 (do not fix here): the model matrix -is not yet wired into ``benchmarks.runner._execute_cell`` dispatch, so a -given run's ``tool_model_key`` values come from the competitor manifest, not -from ``docs/benchmarks/model-matrix.yml``. This module treats the model -matrix as descriptive context (the benchmark's target model axis) and never -assumes an observed ``tool_model_key`` corresponds to a matrix entry. +Confirmed design decision (issue #86, 2026-07-08), narrowing the prior +issue #73/#74 known gap: cell composition stays competitor x question, not +competitor x model x question, so the model matrix is intentionally not +wired into ``benchmarks.runner._execute_cell`` dispatch -- a given run's +``tool_model_key`` values come from the competitor manifest (one manifest +entry per tool x model pairing), not from an automatic per-cell lookup +against ``docs/benchmarks/model-matrix.yml``. Issue #86 adds +``benchmarks.model_matrix.validate_manifest_against_matrix`` so a +manifest's declared provider/model pairings can be checked against this +matrix before a run, but that check is not invoked automatically by +``run_benchmark``. This module treats the model matrix as descriptive +context (the benchmark's target model axis) and never assumes an observed +``tool_model_key`` corresponds to a matrix entry. """ from __future__ import annotations @@ -452,11 +459,13 @@ def _render_report( lines.append("") lines.append( "Defines the benchmark's target model axis (`docs/benchmarks/model-matrix.yml`, " - f"token label `{matrix.methodology_token_label}`). **Known gap (inherited, not fixed " - "by this report):** the model matrix is not yet wired into the runner's per-cell " - "dispatch (tracked as a follow-up to issue #73). The `tool_model_key` values observed " - "below come from the competitor manifest, not necessarily from a matching entry in " - "this matrix." + f"token label `{matrix.methodology_token_label}`). **Design note (issue #86, " + "confirmed 2026-07-08):** cell composition stays competitor x question, so this " + "matrix is intentionally not wired into the runner's per-cell dispatch -- " + "`benchmarks.model_matrix.validate_manifest_against_matrix` can check a manifest's " + "declared provider/model pairings against this matrix before a run, but the " + "`tool_model_key` values observed below still come from the competitor manifest, " + "not from an automatic lookup against this matrix." ) lines.append("") lines.extend( @@ -662,8 +671,8 @@ def _render_report( if not any_aggregate: lines.append( "No cross-model aggregation applies to this run: every competitor present exactly " - "one `tool_model_key` (the model matrix is not yet wired into the runner's " - "dispatch; see 'Model / Client Matrix' above)." + "one `tool_model_key` (cell composition stays competitor x question by design; " + "see 'Model / Client Matrix' above)." ) lines.append("") diff --git a/benchmarks/runner.py b/benchmarks/runner.py index af8b719..2da4879 100644 --- a/benchmarks/runner.py +++ b/benchmarks/runner.py @@ -1,7 +1,16 @@ """Benchmark runner artifact plumbing. -This module intentionally contains only local fake/baseline execution. Real -provider adapters and report generation are follow-up work packages. +Dispatches each competitor/question cell to an adapter looked up by the +competitor manifest's ``adapter`` id (see ``_ADAPTER_DISPATCH``): the local +fake/baseline adapter (issue #72), or the offline stdio adapter that runs +python-docs-mcp-server itself as a system under test (issue #86, see +``benchmarks.adapters.python_docs_mcp_adapter``). Live LLM provider +adapters (``benchmarks.adapters.openai_adapter`` / ``google_adapter``, +issue #73) and report generation (issue #74/#90) live in separate modules. +Wiring a manifest's model-matrix pairing into this module's per-cell +dispatch remains out of scope -- cell composition stays competitor x +question (see the ``benchmarks.model_matrix`` module docstring for the +confirmed composition decision). """ from __future__ import annotations @@ -12,6 +21,7 @@ import subprocess import sys import time +from collections.abc import Callable from dataclasses import dataclass from datetime import UTC, datetime from pathlib import Path @@ -20,7 +30,6 @@ import yaml _SAFE_ID = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.-]*$") -_EXECUTABLE_ADAPTERS = {"fake", "no-mcp-baseline", "no_mcp_baseline"} class BenchmarkValidationError(ValueError): @@ -221,9 +230,12 @@ def _execute_cell(cell: BenchmarkCell) -> dict[str, Any]: status = "succeeded" error: dict[str, str] | None = None answer = "" + tool_calls: list[dict[str, Any]] | None = None try: - answer = _fake_provider_answer(cell) + dispatched = _dispatch_adapter(cell) + answer = dispatched.answer + tool_calls = dispatched.tool_calls except BenchmarkCellFailure as exc: status = "failed" error = {"category": exc.category, "type": type(exc).__name__, "message": str(exc)} @@ -244,6 +256,10 @@ def _execute_cell(cell: BenchmarkCell) -> dict[str, Any]: "completed_at": completed_at, "messages": [{"role": "user", "content": cell.question.prompt}], "answer": answer, + # Raw tool call/response payloads for adapters that make them + # available (e.g. the python-docs-mcp-stdio adapter, issue #86). + # None for adapters that do not record raw payloads. + "tool_calls": tool_calls, "error": error, "external_provider_calls": False, } @@ -284,20 +300,77 @@ def _execute_cell(cell: BenchmarkCell) -> dict[str, Any]: } -def _fake_provider_answer(cell: BenchmarkCell) -> str: - adapter = cell.competitor.adapter - if adapter not in _EXECUTABLE_ADAPTERS: - raise BenchmarkCellFailure( - "tool_failure", f"adapter {adapter!r} is not implemented in issue #72 runner" - ) +@dataclass(frozen=True) +class _CellDispatchResult: + """Return value of one adapter dispatch call (see ``_ADAPTER_DISPATCH``). + + ``tool_calls`` is optional raw tool call/response metadata (e.g. the + python-docs-mcp-stdio adapter's ``search_docs``/``get_docs`` payloads, + issue #86) merged into the cell's transcript record. Adapters with + nothing extra to record (the fake/baseline adapter) leave it ``None``. + """ + + answer: str + tool_calls: list[dict[str, Any]] | None = None + + +def _fake_adapter_answer(cell: BenchmarkCell) -> _CellDispatchResult: + """The mocked-plumbing fake/no-mcp-baseline adapter (issue #72).""" forced_failure = cell.competitor.raw.get("force_failure") if forced_failure: category = _forced_failure_category(forced_failure) raise BenchmarkCellFailure(category, f"forced fake provider {category}") answer = cell.competitor.raw.get("fake_answer") if isinstance(answer, str): - return answer - return f"[fake:{cell.competitor.id}] {cell.question.prompt}" + return _CellDispatchResult(answer=answer) + return _CellDispatchResult(answer=f"[fake:{cell.competitor.id}] {cell.question.prompt}") + + +def _python_docs_mcp_stdio_answer(cell: BenchmarkCell) -> _CellDispatchResult: + """Dispatch to the offline python-docs-mcp-server stdio adapter (issue #86). + + Imported lazily to avoid a module-level import cycle: adapter modules + import ``BenchmarkCellFailure`` from this module, so this module cannot + import from ``benchmarks.adapters`` at module scope. + """ + from benchmarks.adapters.python_docs_mcp_adapter import PythonDocsMcpAdapter + + result = PythonDocsMcpAdapter().run(cell.question.prompt) + return _CellDispatchResult( + answer=result.answer, + tool_calls=[ + { + "tool": call.tool, + "arguments": call.arguments, + "result": call.result, + "is_error": call.is_error, + } + for call in result.tool_calls + ], + ) + + +#: Adapter-id -> dispatch function registry (issue #86), replacing the +#: prior hardcoded ``_EXECUTABLE_ADAPTERS`` allowlist membership check. +#: Adding a new dispatchable adapter means adding one entry here; an +#: unrecognized adapter id still fails cleanly with a ``tool_failure`` +#: (see ``_dispatch_adapter``), matching the prior allowlist's behavior. +_ADAPTER_DISPATCH: dict[str, Callable[[BenchmarkCell], _CellDispatchResult]] = { + "fake": _fake_adapter_answer, + "no-mcp-baseline": _fake_adapter_answer, + "no_mcp_baseline": _fake_adapter_answer, + "python-docs-mcp-stdio": _python_docs_mcp_stdio_answer, +} + + +def _dispatch_adapter(cell: BenchmarkCell) -> _CellDispatchResult: + adapter = cell.competitor.adapter + handler = _ADAPTER_DISPATCH.get(adapter) + if handler is None: + raise BenchmarkCellFailure( + "tool_failure", f"adapter {adapter!r} is not implemented in this runner" + ) + return handler(cell) def _forced_failure_category(value: object) -> str: diff --git a/docs/benchmarks/PUBLIC-BENCHMARK-METHODOLOGY.md b/docs/benchmarks/PUBLIC-BENCHMARK-METHODOLOGY.md index 7265e40..fce0fab 100644 --- a/docs/benchmarks/PUBLIC-BENCHMARK-METHODOLOGY.md +++ b/docs/benchmarks/PUBLIC-BENCHMARK-METHODOLOGY.md @@ -1,7 +1,6 @@ # Public Benchmark Methodology -**Status:** Draft methodology for issue -[#63](https://github.com/ayhammouda/python-docs-mcp-server/issues/63). +**Status:** Accepted (blessed 2026-07-08; see PLAN-2026-07-08-benchmark-plumbing.md Amendment 1) Do not publish comparative claims until the harness has produced reproducible data from this methodology. diff --git a/tests/benchmarks/test_model_matrix.py b/tests/benchmarks/test_model_matrix.py index 8523738..72d64b8 100644 --- a/tests/benchmarks/test_model_matrix.py +++ b/tests/benchmarks/test_model_matrix.py @@ -11,8 +11,9 @@ ModelFamily, load_model_matrix, tool_model_cells, + validate_manifest_against_matrix, ) -from benchmarks.runner import BenchmarkValidationError +from benchmarks.runner import BenchmarkValidationError, Competitor REPO_ROOT = Path(__file__).resolve().parents[2] MODEL_MATRIX_PATH = REPO_ROOT / "docs" / "benchmarks" / "model-matrix.yml" @@ -205,3 +206,66 @@ def test_tool_model_cells_cross_multiplies_without_averaging() -> None: assert ("python-docs-mcp-server", "google-test") in cells assert ("no-mcp-baseline", "openai-test") in cells assert ("no-mcp-baseline", "google-test") in cells + + +# --- validate_manifest_against_matrix (issue #86) -------------------------- +# +# Confirmed composition decision: the competitor manifest enumerates one +# entry per tool x model pairing; a manifest entry whose declared +# provider/model pair is absent from the model matrix must fail validation +# with a clean BenchmarkValidationError. + + +def test_validate_manifest_against_matrix_accepts_a_known_pairing(tmp_path: Path) -> None: + matrix = load_model_matrix(_matrix_yaml(tmp_path / "matrix.yml")) + competitors = [ + Competitor( + id="python-docs-mcp-server", + adapter="python-docs-mcp-stdio", + raw={"provider": "openai", "model": "gpt-4o-mini"}, + ) + ] + + validate_manifest_against_matrix(competitors, matrix) # must not raise + + +def test_validate_manifest_against_matrix_rejects_an_unknown_pairing(tmp_path: Path) -> None: + matrix = load_model_matrix(_matrix_yaml(tmp_path / "matrix.yml")) + competitors = [ + Competitor( + id="python-docs-mcp-server", + adapter="python-docs-mcp-stdio", + raw={"provider": "openai", "model": "gpt-4o-does-not-exist"}, + ) + ] + + with pytest.raises(BenchmarkValidationError) as exc_info: + validate_manifest_against_matrix(competitors, matrix) + + assert "python-docs-mcp-server" in str(exc_info.value) + assert "openai/gpt-4o-does-not-exist" in str(exc_info.value) + + +def test_validate_manifest_against_matrix_skips_entries_without_a_full_pairing( + tmp_path: Path, +) -> None: + matrix = load_model_matrix(_matrix_yaml(tmp_path / "matrix.yml")) + competitors = [ + Competitor(id="no-mcp-baseline", adapter="no-mcp-baseline", raw={}), + Competitor(id="partial", adapter="fake", raw={"provider": "openai"}), + ] + + validate_manifest_against_matrix(competitors, matrix) # must not raise: no full pairing + + +def test_validate_manifest_against_matrix_against_the_real_model_matrix_file() -> None: + matrix = load_model_matrix(MODEL_MATRIX_PATH) + competitors = [ + Competitor( + id="python-docs-mcp-server", + adapter="python-docs-mcp-stdio", + raw={"provider": "openai", "model": "gpt-4o-mini"}, + ) + ] + + validate_manifest_against_matrix(competitors, matrix) # must not raise diff --git a/tests/benchmarks/test_python_docs_mcp_adapter.py b/tests/benchmarks/test_python_docs_mcp_adapter.py new file mode 100644 index 0000000..e4e9e3a --- /dev/null +++ b/tests/benchmarks/test_python_docs_mcp_adapter.py @@ -0,0 +1,310 @@ +"""Tests for the offline stdio python-docs-mcp-server adapter (issue #86). + +All tests here drive the adapter against a fake/stubbed MCP transport (a +scripted session double) -- no real server subprocess is spawned. The one +exception is the optional integration test at the bottom, which spawns the +real server only when a local index already exists at the default cache +path, and is skipped otherwise (per the issue's acceptance criteria). +""" + +from __future__ import annotations + +import asyncio +import socket +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any + +import pytest + +from benchmarks.adapters.python_docs_mcp_adapter import ( + PythonDocsMcpAdapter, + PythonDocsMcpResult, + _default_index_path, + _run_retrieval_flow, + _tool_result_payload, +) +from benchmarks.runner import BenchmarkCellFailure + + +@dataclass +class _FakeContentBlock: + text: str + + +@dataclass +class _FakeCallToolResult: + content: list[Any] = field(default_factory=list) + structuredContent: dict[str, Any] | None = None + isError: bool = False + + +class _ScriptedSession: + """A minimal fake MCP session returning one scripted result per tool. + + No real transport, no subprocess, no network -- this is the + "fake/stubbed MCP transport" the issue's acceptance criteria requires + for unit tests of the search_docs -> get_docs flow. + """ + + def __init__(self, responses: dict[str, _FakeCallToolResult]) -> None: + self._responses = responses + self.calls: list[tuple[str, dict[str, Any]]] = [] + + async def call_tool( + self, name: str, arguments: dict[str, Any] | None = None + ) -> _FakeCallToolResult: + self.calls.append((name, arguments or {})) + return self._responses[name] + + +def _search_result(hits: list[dict[str, Any]]) -> _FakeCallToolResult: + return _FakeCallToolResult( + content=[_FakeContentBlock(text="ignored")], + structuredContent={"hits": hits, "note": None}, + isError=False, + ) + + +def _get_result(content: str, **extra: Any) -> _FakeCallToolResult: + payload = { + "content": content, + "slug": "lib/m.html", + "title": "t", + "version": "3.13", + **extra, + } + return _FakeCallToolResult( + content=[_FakeContentBlock(text="ignored")], + structuredContent=payload, + isError=False, + ) + + +def _error_result(message: str) -> _FakeCallToolResult: + return _FakeCallToolResult( + content=[_FakeContentBlock(text=message)], + structuredContent=None, + isError=True, + ) + + +# --- _run_retrieval_flow: pure orchestration against a fake session -------- + + +async def test_retrieval_flow_calls_search_then_get_docs_and_records_raw_payloads() -> None: + hit = {"slug": "lib/asyncio-task.html", "anchor": "asyncio.TaskGroup", "version": "3.13"} + session = _ScriptedSession( + { + "search_docs": _search_result([hit]), + "get_docs": _get_result("TaskGroup docs content"), + } + ) + + result = await _run_retrieval_flow(session, "asyncio.TaskGroup", max_results=3) + + assert result.answer == "TaskGroup docs content" + assert [call.tool for call in result.tool_calls] == ["search_docs", "get_docs"] + assert session.calls[0] == ("search_docs", {"query": "asyncio.TaskGroup", "max_results": 3}) + assert session.calls[1] == ( + "get_docs", + {"slug": "lib/asyncio-task.html", "version": "3.13", "anchor": "asyncio.TaskGroup"}, + ) + assert result.tool_calls[0].result == {"hits": [hit], "note": None} + assert result.tool_calls[0].is_error is False + assert result.tool_calls[1].is_error is False + + +async def test_retrieval_flow_returns_empty_answer_when_search_has_no_hits() -> None: + session = _ScriptedSession({"search_docs": _search_result([])}) + + result = await _run_retrieval_flow(session, "no such symbol") + + assert result.answer == "" + assert len(result.tool_calls) == 1 + assert result.tool_calls[0].tool == "search_docs" + + +async def test_retrieval_flow_omits_anchor_argument_when_hit_has_no_anchor() -> None: + hit = {"slug": "lib/page.html", "anchor": None, "version": "3.13"} + session = _ScriptedSession( + {"search_docs": _search_result([hit]), "get_docs": _get_result("page content")} + ) + + await _run_retrieval_flow(session, "anything") + + assert session.calls[1] == ("get_docs", {"slug": "lib/page.html", "version": "3.13"}) + + +async def test_retrieval_flow_raises_tool_failure_when_search_docs_errors() -> None: + session = _ScriptedSession({"search_docs": _error_result("boom")}) + + with pytest.raises(BenchmarkCellFailure) as exc_info: + await _run_retrieval_flow(session, "anything") + + assert exc_info.value.category == "tool_failure" + assert "boom" in str(exc_info.value) + + +async def test_retrieval_flow_raises_tool_failure_when_get_docs_errors() -> None: + hit = {"slug": "lib/missing.html", "anchor": None, "version": "3.13"} + session = _ScriptedSession( + { + "search_docs": _search_result([hit]), + "get_docs": _error_result("Page not found"), + } + ) + + with pytest.raises(BenchmarkCellFailure) as exc_info: + await _run_retrieval_flow(session, "anything") + + assert exc_info.value.category == "tool_failure" + assert "Page not found" in str(exc_info.value) + + +# --- _tool_result_payload --------------------------------------------------- + + +def test_tool_result_payload_prefers_structured_content() -> None: + result = _get_result("x") + + assert _tool_result_payload(result) == result.structuredContent + + +def test_tool_result_payload_falls_back_to_first_text_block_on_error() -> None: + result = _error_result("Error executing tool get_docs: not found") + + assert _tool_result_payload(result) == {"text": "Error executing tool get_docs: not found"} + + +# --- PythonDocsMcpAdapter.run(): missing index -> tool_failure, no spawn --- + + +def test_run_fails_with_tool_failure_when_index_is_missing_and_never_spawns( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + def _fail_if_called(*args: Any, **kwargs: Any) -> Any: + raise AssertionError("asyncio.run must not be called when the index is missing") + + monkeypatch.setattr("benchmarks.adapters.python_docs_mcp_adapter.asyncio.run", _fail_if_called) + adapter = PythonDocsMcpAdapter(index_path=tmp_path / "does-not-exist.db") + + with pytest.raises(BenchmarkCellFailure) as exc_info: + adapter.run("anything") + + assert exc_info.value.category == "tool_failure" + assert "no local index found" in str(exc_info.value) + + +def test_run_missing_index_check_never_opens_a_socket( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + def _fail_if_called(*args: Any, **kwargs: Any) -> Any: + raise AssertionError("no socket should be opened when the index is missing") + + monkeypatch.setattr(socket, "socket", _fail_if_called) + adapter = PythonDocsMcpAdapter(index_path=tmp_path / "does-not-exist.db") + + with pytest.raises(BenchmarkCellFailure): + adapter.run("anything") + + +def test_run_uses_default_index_path_when_none_is_configured( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.setattr( + "benchmarks.adapters.python_docs_mcp_adapter._default_index_path", + lambda: tmp_path / "default-missing.db", + ) + adapter = PythonDocsMcpAdapter() + + with pytest.raises(BenchmarkCellFailure) as exc_info: + adapter.run("anything") + + assert "default-missing.db" in str(exc_info.value) + + +# --- PythonDocsMcpAdapter.run(): failure-category mapping ------------------ + + +def test_run_maps_a_slow_call_to_a_timeout_failure( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + index_path = tmp_path / "index.db" + index_path.write_text("existence is all `run()` checks before spawning") + + async def _slow_run_async(self: PythonDocsMcpAdapter, prompt: str) -> PythonDocsMcpResult: + await asyncio.sleep(1) + return PythonDocsMcpResult(answer="too late", tool_calls=[]) + + monkeypatch.setattr(PythonDocsMcpAdapter, "_run_async", _slow_run_async) + adapter = PythonDocsMcpAdapter(index_path=index_path, timeout_seconds=0.01) + + with pytest.raises(BenchmarkCellFailure) as exc_info: + adapter.run("anything") + + assert exc_info.value.category == "timeout" + + +def test_run_maps_an_unexpected_transport_error_to_mcp_protocol_crash( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + index_path = tmp_path / "index.db" + index_path.write_text("existence is all `run()` checks before spawning") + + async def _broken_run_async(self: PythonDocsMcpAdapter, prompt: str) -> PythonDocsMcpResult: + raise RuntimeError("stdio pipe closed unexpectedly") + + monkeypatch.setattr(PythonDocsMcpAdapter, "_run_async", _broken_run_async) + adapter = PythonDocsMcpAdapter(index_path=index_path) + + with pytest.raises(BenchmarkCellFailure) as exc_info: + adapter.run("anything") + + assert exc_info.value.category == "mcp_protocol_crash" + assert "stdio pipe closed unexpectedly" in str(exc_info.value) + + +def test_run_propagates_benchmark_cell_failure_from_run_async_unchanged( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + index_path = tmp_path / "index.db" + index_path.write_text("existence is all `run()` checks before spawning") + + async def _tool_level_failure(self: PythonDocsMcpAdapter, prompt: str) -> PythonDocsMcpResult: + raise BenchmarkCellFailure("tool_failure", "search_docs returned an error: boom") + + monkeypatch.setattr(PythonDocsMcpAdapter, "_run_async", _tool_level_failure) + adapter = PythonDocsMcpAdapter(index_path=index_path) + + with pytest.raises(BenchmarkCellFailure) as exc_info: + adapter.run("anything") + + assert exc_info.value.category == "tool_failure" + assert "boom" in str(exc_info.value) + + +# --- Optional integration test: real server spawn, skipped without an index + + +@pytest.mark.skipif( + not _default_index_path().exists(), + reason="requires a locally built python-docs-mcp-server index", +) +def test_real_server_round_trip_when_a_local_index_exists() -> None: + adapter = PythonDocsMcpAdapter(timeout_seconds=60) + + try: + result = adapter.run("os.path.join") + except BenchmarkCellFailure as exc: + # A locally built but symbol-only index (no content ingestion) is a + # legitimate dev state; get_docs then fails with a recorded + # tool_failure rather than a crash. Any of the three documented + # categories is an acceptable outcome for this environment-specific + # smoke test -- an unhandled exception is not. + assert exc.category in {"tool_failure", "timeout", "mcp_protocol_crash"} + return + + assert isinstance(result.answer, str) + assert result.tool_calls + assert result.tool_calls[0].tool == "search_docs" diff --git a/tests/benchmarks/test_runner.py b/tests/benchmarks/test_runner.py index ae00991..5ae8d98 100644 --- a/tests/benchmarks/test_runner.py +++ b/tests/benchmarks/test_runner.py @@ -376,3 +376,123 @@ def test_cli_dry_run_outputs_summary(tmp_path: Path) -> None: assert summary["dry_run"] is True assert summary["planned_cells"] == 1 assert (out_dir / "run-summary.json").is_file() + + +# --- python-docs-mcp-stdio adapter dispatch (issue #86) --------------------- +# +# These tests exercise the runner's dispatch-registry wiring only, via a +# fake product-adapter class monkeypatched over the real one -- no real MCP +# server subprocess is spawned. See tests/benchmarks/test_python_docs_mcp_adapter.py +# for the adapter's own unit tests against a fake/stubbed MCP transport. + + +class _FakeProductAdapter: + """Stands in for ``PythonDocsMcpAdapter`` to test dispatch wiring only.""" + + def __init__(self, *args: Any, **kwargs: Any) -> None: + pass + + def run(self, prompt: str) -> Any: + from benchmarks.adapters.python_docs_mcp_adapter import ( + PythonDocsMcpResult, + ToolCallRecord, + ) + + return PythonDocsMcpResult( + answer=f"fake-retrieved-content for {prompt}", + tool_calls=[ + ToolCallRecord( + tool="search_docs", + arguments={"query": prompt, "max_results": 3}, + result={"hits": [{"slug": "lib/x.html", "anchor": "a1"}]}, + is_error=False, + ), + ToolCallRecord( + tool="get_docs", + arguments={"slug": "lib/x.html", "anchor": "a1"}, + result={"content": f"fake-retrieved-content for {prompt}"}, + is_error=False, + ), + ], + ) + + +def test_python_docs_mcp_stdio_adapter_is_dispatchable_and_records_tool_calls( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr( + "benchmarks.adapters.python_docs_mcp_adapter.PythonDocsMcpAdapter", + _FakeProductAdapter, + ) + out_dir = tmp_path / "results" / "product-adapter" + + summary = run_benchmark( + BenchmarkConfig( + corpus_path=_corpus(tmp_path), + manifest_path=_manifest( + tmp_path, + [{"id": "python-docs-mcp-server", "adapter": "python-docs-mcp-stdio"}], + ), + out_dir=out_dir, + run_id="product-adapter", + ) + ) + + assert summary["succeeded_cells"] == 1 + transcript = json.loads( + (out_dir / "transcripts" / "python-docs-mcp-server" / "q001.json").read_text( + encoding="utf-8" + ) + ) + assert transcript["status"] == "succeeded" + assert transcript["answer"].startswith("fake-retrieved-content") + assert transcript["tool_calls"][0]["tool"] == "search_docs" + assert transcript["tool_calls"][1]["tool"] == "get_docs" + assert transcript["external_provider_calls"] is False + + +def test_dry_run_validates_python_docs_mcp_stdio_manifest_without_an_index( + tmp_path: Path, +) -> None: + # dry-run must validate and plan cells for the new adapter without ever + # touching a local index or spawning a subprocess (issue #86). + out_dir = tmp_path / "results" / "product-adapter-dry-run" + + summary = run_benchmark( + BenchmarkConfig( + corpus_path=_corpus(tmp_path), + manifest_path=_manifest( + tmp_path, + [{"id": "python-docs-mcp-server", "adapter": "python-docs-mcp-stdio"}], + ), + out_dir=out_dir, + run_id="product-adapter-dry-run", + dry_run=True, + ) + ) + + assert summary["dry_run"] is True + assert summary["planned_cells"] == 1 + assert not (out_dir / "transcripts").exists() + + +def test_unrecognized_adapter_id_fails_cleanly_without_crashing(tmp_path: Path) -> None: + out_dir = tmp_path / "results" / "unknown-adapter" + + summary = run_benchmark( + BenchmarkConfig( + corpus_path=_corpus(tmp_path), + manifest_path=_manifest( + tmp_path, [{"id": "mystery", "adapter": "totally-unknown-adapter"}] + ), + out_dir=out_dir, + run_id="unknown-adapter", + ) + ) + + assert summary["failed_cells"] == 1 + failure = json.loads( + (out_dir / "failures" / "mystery" / "q001.json").read_text(encoding="utf-8") + ) + assert failure["error"]["category"] == "tool_failure" + assert "not implemented in this runner" in failure["error"]["message"]