Skip to content

Commit 716d1eb

Browse files
authored
Merge pull request #64 from smith6jt-cop/claude/gifted-ptolemy-3GIL6
feat(mcp): migrate server to FastMCP with structured tool output
2 parents 180fbb5 + f4e24c4 commit 716d1eb

13 files changed

Lines changed: 1751 additions & 899 deletions

File tree

.github/workflows/mcp-eval.yml

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
name: MCP Eval
2+
3+
# Evaluates the KINTSUGI MCP tool surface.
4+
# * Structural pass (token cost, schema quality, tool coverage): runs on every
5+
# PR as a fast, deterministic gate (no API key, --strict fails on regressions
6+
# such as a documented-but-unregistered tool).
7+
# * Agentic tool-selection pass: runs on manual dispatch and weekly schedule
8+
# only, and only when an ANTHROPIC_API_KEY secret is configured (it spends
9+
# API tokens and is nondeterministic, so it never runs on PRs).
10+
on:
11+
pull_request:
12+
branches: [main, develop]
13+
paths:
14+
- "src/kintsugi/mcp/**"
15+
- "evals/mcp/**"
16+
- ".github/workflows/mcp-eval.yml"
17+
workflow_dispatch:
18+
inputs:
19+
model:
20+
description: "Model for the agentic tool-selection eval"
21+
default: "claude-sonnet-4-6"
22+
schedule:
23+
- cron: "0 6 * * 1" # Mondays 06:00 UTC
24+
25+
concurrency:
26+
group: ${{ github.workflow }}-${{ github.ref }}
27+
cancel-in-progress: true
28+
29+
jobs:
30+
mcp-eval:
31+
name: MCP tool-surface eval
32+
runs-on: ubuntu-latest
33+
steps:
34+
- uses: actions/checkout@v4
35+
36+
- name: Set up Python
37+
uses: actions/setup-python@v5
38+
with:
39+
python-version: "3.12"
40+
41+
- name: Install system dependencies
42+
run: |
43+
sudo apt-get update
44+
sudo apt-get install -y libvips-dev
45+
46+
- name: Install Python dependencies
47+
run: |
48+
python -m pip install --upgrade pip
49+
pip install -e ".[dev]" anthropic
50+
51+
- name: Run MCP evaluation
52+
env:
53+
ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
54+
EVAL_MODEL: ${{ github.event.inputs.model || 'claude-sonnet-4-6' }}
55+
run: |
56+
if [ -n "$ANTHROPIC_API_KEY" ] && [ "${{ github.event_name }}" != "pull_request" ]; then
57+
python evals/mcp/harness.py --agentic --model "$EVAL_MODEL" --strict --json mcp_eval_report.json
58+
else
59+
if [ "${{ github.event_name }}" != "pull_request" ]; then
60+
echo "::notice::ANTHROPIC_API_KEY secret not set — running the structural eval only."
61+
fi
62+
python evals/mcp/harness.py --strict --json mcp_eval_report.json
63+
fi
64+
65+
- name: Write job summary
66+
if: always()
67+
run: |
68+
python - <<'PY' >> "$GITHUB_STEP_SUMMARY"
69+
import json, pathlib
70+
p = pathlib.Path("mcp_eval_report.json")
71+
if not p.exists():
72+
print("No eval report produced.")
73+
raise SystemExit(0)
74+
r = json.loads(p.read_text())
75+
s = r["structural"]
76+
print("## MCP tool-surface eval\n")
77+
print(f"- Tools: **{s['tool_count']}** (output schemas: "
78+
f"{s['tools_with_output_schema']}/{s['tool_count']})")
79+
print(f"- Tool-definition tokens (est.): **~{s['total_definition_tokens_est']:,}**")
80+
print(f"- Params missing description: {s['params_missing_description_count']}")
81+
gaps = s["task_coverage_gaps"]
82+
print(f"- Task coverage gaps: {gaps if gaps else 'none'}")
83+
a = r.get("agentic")
84+
if a:
85+
print(f"\n### Agentic tool-selection: **{a['passed']}/{a['total']} = "
86+
f"{a['accuracy']:.0%}** (model `{a['model']}`)\n")
87+
fails = [x for x in a["results"] if not x["pass"]]
88+
if fails:
89+
print("| task | chose | expected |")
90+
print("|------|-------|----------|")
91+
for x in fails:
92+
print(f"| {x['task']} | {x['chosen']} | {', '.join(x['expected'])} |")
93+
else:
94+
print("\n_Agentic pass skipped (PR event or no ANTHROPIC_API_KEY secret)._")
95+
PY
96+
97+
- name: Upload report
98+
if: always()
99+
uses: actions/upload-artifact@v4
100+
with:
101+
name: mcp-eval-report
102+
path: mcp_eval_report.json
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"name": "mcp-fastmcp-structured-output",
3+
"version": "1.0.0",
4+
"description": "Migrate a Python MCP server to FastMCP with structured tool output, preserve curated schemas, and evaluate the tool surface. Trigger when: (1) migrating kintsugi.mcp or any low-level mcp.server.Server to FastMCP, (2) adding structured output / outputSchema to MCP tools, (3) an MCP tool result fails output validation with 'None is not of type ...', (4) preserving per-parameter descriptions or enums under FastMCP's auto schemas, (5) building an MCP tool-selection evaluation harness or wiring it into CI.",
5+
"author": {
6+
"name": "KINTSUGI Team"
7+
},
8+
"skills": "./skills",
9+
"repository": "https://github.com/smith6jt-cop/Skills_Registry"
10+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Publishing this skill to the Skills Registry
2+
3+
This is a drafted `/retrospective` skill, preserved in the KINTSUGI repo because
4+
the `Skills_Registry` is a **separate** repository
5+
(`github.com/smith6jt-cop/Skills_Registry`). It already passed the registry's
6+
validator locally (`OK mcp-fastmcp-structured-output`).
7+
8+
To publish it:
9+
10+
```bash
11+
# from the KINTSUGI repo root
12+
git submodule update --init Skills_Registry
13+
cp -r docs/skills/mcp-fastmcp-structured-output Skills_Registry/plugins/kintsugi/
14+
15+
cd Skills_Registry
16+
git checkout main && git pull # leave the detached submodule HEAD
17+
python scripts/generate_marketplace.py # adds it to marketplace.json
18+
python scripts/validate_plugins.py # expect: OK mcp-fastmcp-structured-output
19+
git add plugins/kintsugi/mcp-fastmcp-structured-output marketplace.json
20+
git commit -m "feat(kintsugi): add mcp-fastmcp-structured-output skill"
21+
git push
22+
```
23+
24+
Optionally, bump the submodule pointer in KINTSUGI afterward:
25+
26+
```bash
27+
cd .. # back to KINTSUGI root
28+
git add Skills_Registry
29+
git commit -m "chore: bump Skills_Registry (add mcp-fastmcp-structured-output)"
30+
```
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
---
2+
name: mcp-fastmcp-structured-output
3+
description: "Migrate a low-level MCP server to FastMCP with structured output, preserve curated input schemas, and evaluate the tool surface. Trigger: FastMCP migration, adding outputSchema/structuredContent, 'None is not of type' output-validation errors, building MCP tool-selection evals."
4+
author: KINTSUGI Team
5+
date: 2026-06-22
6+
---
7+
8+
# MCP Server: FastMCP Migration + Structured Output + Evaluation
9+
10+
## Experiment Overview
11+
| Item | Details |
12+
|------|---------|
13+
| **Date** | 2026-06-22 |
14+
| **Goal** | Migrate `kintsugi.mcp` from low-level `mcp.server.Server` to FastMCP, deliver structured tool output, and stand up an evaluation/improvement workflow |
15+
| **Environment** | `mcp==1.27.2` (floor `>=1.10`), Python 3.11/3.12, FastMCP (`mcp.server.fastmcp`) |
16+
| **Status** | Success |
17+
18+
## Context
19+
The MCP server hand-wrote a `Tool(...)` list + an `elif` dispatch in `call_tool`,
20+
returned only `TextContent` (a JSON string), and exposed no output schemas. Goal:
21+
move to FastMCP so each tool returns machine-readable `structuredContent`, on a
22+
foundation that makes later resources/prompts/elicitation cheap.
23+
24+
## What Worked (verified approach)
25+
26+
1. **Floor `mcp>=1.10.0,<2`.** Structured output (`Tool.outputSchema`,
27+
`CallToolResult.structuredContent`, FastMCP `structured_output`) and
28+
elicitation **do not exist below 1.10** (the SDK release after spec 2025-06-18).
29+
30+
2. **Register existing handlers; keep them FastMCP-agnostic.** A declarative
31+
registry (`tool_specs.py`: name → module/attr + description + input schema)
32+
plus, in `server.py`:
33+
```python
34+
server = FastMCP("kintsugi")
35+
handler = getattr(module, spec["attr"]) # plain async dict-returning fn
36+
server.add_tool(handler, name=spec["name"],
37+
description=spec["description"], structured_output=True)
38+
```
39+
Handlers stay plain `async` functions returning `dict[str, Any]`, so the unit
40+
tests keep calling them directly.
41+
42+
3. **Returns: `dict[str, Any]` + `structured_output=True`.** FastMCP then emits
43+
the dict as `structuredContent` **and** a JSON text block, for both the
44+
success and the in-band `{"error": ...}` branches — no per-tool models needed.
45+
46+
4. **Preserve curated input schemas (descriptions + enums) by overriding** the
47+
advertised schema after registration; validation still runs from the handler
48+
signature:
49+
```python
50+
# FastMCP's signature-derived schema drops per-param descriptions + enums.
51+
server._tool_manager.get_tool(name).parameters = curated_input_schema # wrap in try/except
52+
```
53+
54+
5. **Evaluation harness** (`evals/mcp/`): a structural pass (tool-definition token
55+
cost, schema-quality checks, confusable-group flags, task coverage) + an
56+
agentic pass (`tool_choice="auto"`, score the model's first `tool_use`).
57+
Tool **selection** needs only the schemas — **no image data or handler
58+
execution** — so it runs with just an API key. Wire agentic into CI behind an
59+
`ANTHROPIC_API_KEY` secret (manual + scheduled); gate PRs with the structural
60+
`--strict` pass. The harness immediately found `analyze_weighted_subtraction`
61+
documented + implemented but **never registered**.
62+
63+
## What Failed (do not repeat)
64+
65+
| Attempt | Result |
66+
|---------|--------|
67+
| Strict `TypedDict` returns (`total=False`) to get rich per-field outputSchema | **FAILED.** FastMCP materializes absent keys as `None`, then validates against the non-nullable field type → `Output validation error: None is not of type 'string'`, `structuredContent=None`, `isError=True` on **both** success and error returns |
68+
| `Optional`-field `TypedDict`s (to dodge the above) | Validates, but **pollutes every result** with `null` keys for absent fields (noisy/misleading) → use plain `dict[str, Any]` instead |
69+
| Rely on docstrings for per-parameter descriptions under FastMCP | **FAILED.** FastMCP 1.27 does **not** parse docstring args; descriptions are dropped. Use `Annotated[T, Field(description=...)]` or override `.parameters` |
70+
| Raise the `mcp` floor only in `pyproject.toml` extras | **Incomplete.** `src/kintsugi/deps.py` `OPTIONAL_GROUPS` is a parallel source of truth used by `kintsugi install`/`check` — update both (`claude` *and* `dev`) |
71+
72+
## Verification
73+
- `pytest tests/test_mcp_tools.py tests/test_mcp_path_safety.py` — direct-call
74+
tests stay green (handlers unchanged); add an in-memory client test
75+
(`mcp.shared.memory.create_connected_server_and_client_session`) asserting
76+
`outputSchema` present + `structuredContent` populated + text block + error
77+
branch clean.
78+
- `python evals/mcp/harness.py --strict` — structural gate (fails on coverage
79+
gaps / missing descriptions).
80+
- Preserve the `create_server()` `ImportError` contract by simulating
81+
`MCP_AVAILABLE = False` (don't swallow the import in a bare `except`).
82+
83+
## Key Files (reference implementation)
84+
- `src/kintsugi/mcp/server.py` — FastMCP wiring, `_advertise_schema` override
85+
- `src/kintsugi/mcp/tool_specs.py` — declarative registry (mcp-free, also feeds `kintsugi mcp tools`)
86+
- `evals/mcp/{tasks.py,harness.py,README.md}` — evaluation harness
87+
- `.github/workflows/mcp-eval.yml` — keyed agentic eval + structural PR gate

evals/mcp/README.md

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# KINTSUGI MCP — Evaluation Harness
2+
3+
Measures the quality of the KINTSUGI MCP **tool surface** with realistic tasks
4+
instead of intuition, following the `mcp-builder` philosophy. Use it to drive and
5+
verify MCP improvements (tool consolidation, description quality, resources/prompts).
6+
7+
## Why
8+
9+
The server exposes 27 tools to Claude. Two questions decide whether that surface
10+
is *good*:
11+
12+
1. **Is it cheap and clear?** (structural) — how many tokens do the tool
13+
definitions cost, and are any tools/params under-described or confusable?
14+
2. **Does the model use it correctly?** (agentic) — given a realistic request,
15+
does Claude pick the right tool?
16+
17+
## Run it
18+
19+
```bash
20+
# Structural only — no network, no API key. Runs anywhere mcp is installed.
21+
python evals/mcp/harness.py
22+
23+
# Agentic tool-selection eval — needs an API key + the anthropic SDK.
24+
pip install anthropic
25+
ANTHROPIC_API_KEY=sk-... python evals/mcp/harness.py --agentic --model claude-sonnet-4-6
26+
27+
# Save the full report (structural + agentic) as JSON.
28+
python evals/mcp/harness.py --agentic --json mcp_eval_report.json
29+
```
30+
31+
The agentic mode measures **tool selection only** — it records the model's first
32+
`tool_use` per task and never executes the handler — so it needs **no image data
33+
or project**, just the live tool schemas.
34+
35+
## Files
36+
37+
| File | Purpose |
38+
|------|---------|
39+
| `tasks.py` | 15 realistic natural-language tasks with `expected` acceptable tool(s). Includes deliberate confusable pairs (`denoise` vs `denoise_advanced`; the three background-removal tools; `assess_quality` vs `compute_snr`). |
40+
| `harness.py` | Builds the live FastMCP server, runs the structural metrics, and (opt-in) the agentic tool-selection loop. |
41+
42+
Add a task by appending an `EvalTask` to `TASKS`; the structural pass will verify
43+
its `expected` tools exist on the server.
44+
45+
## Findings (baseline, structural pass)
46+
47+
Run against the post-migration server:
48+
49+
- **27 tools**, **27/27 carry an output schema** (structured output is live).
50+
- **~3.9k tokens** of tool definitions injected on every call (~144/tool) — modest;
51+
no urgent token pressure (compare: GitHub's MCP server ≈ 55k for 43 tools).
52+
- **0 params missing a description**, **0 tools with weak descriptions** — the
53+
curated input schemas held up.
54+
- **First eval-driven fix:** the harness caught `analyze_weighted_subtraction`
55+
documented in `CLAUDE.md` and implemented, but **never registered**. Now wired up.
56+
57+
### Open items the eval points at (future PRs)
58+
59+
- **Confusable groups** worth either consolidating or giving sharper
60+
"Use this when …" descriptions, to be confirmed by the agentic pass:
61+
- background removal: `subtract_blank` / `gaussian_subtract` / `estimate_background`
62+
- denoising: `denoise` / `denoise_advanced`
63+
- parameter suggestion: `suggest_parameters` / `suggest_with_learning` / `get_learned_parameters`
64+
- **Resources/prompts** (deferred P1 items) — once added, extend the harness with
65+
resource-read and prompt tasks.
66+
67+
Run the agentic pass with a key to turn the "confusable groups" hypotheses into
68+
measured tool-selection accuracy, then act on the tools that get mis-selected.

0 commit comments

Comments
 (0)