Skip to content

Return structured tool output, not prose for the model to parse#16

Merged
gradill22 merged 1 commit into
mainfrom
feat/structured-tool-output
Jul 26, 2026
Merged

Return structured tool output, not prose for the model to parse#16
gradill22 merged 1 commit into
mainfrom
feat/structured-tool-output

Conversation

@gradill22

Copy link
Copy Markdown
Contributor

The problem

Every MCP tool flattened its result into a hand-formatted text blob. An agent polling a job got this:

Job job_abc [crawl] — status: running (12/50)
Created: 2026-01-01T09:14:22Z | Finished: —
## https://example.com/a  (depth 1)  ✓

To decide whether the job was done, a model had to pattern-match status: out of a sentence, split 12/50 on a slash, read as null, and interpret / as per-item success. The JS SDK already returns a typed BulkJob — this adapter was the only place throwing that structure away.

The change

Every tool now declares an outputSchema and returns structuredContent alongside the existing text block:

{ "kind": "crawl", "jobId": "job_abc", "status": "done",
  "total": 12, "completed": 12, "done": true, "truncated": false,
  "results": [
    { "url": "https://example.com/a", "depth": 1, "ok": true,  "markdown": "# A" },
    { "url": "https://example.com/b", "depth": 1, "ok": false, "error": "target_timeout" }
  ] }
  • One shared job schema covers bulk, crawl, get_job, and wait_for_jobget_job is polymorphic by design, so crawl-only fields (depth, truncated, truncatedReason) are optional and discriminated by kind.
  • jsonify() is load-bearing, not cosmetic. Date fields (createdAt, finishedAt, retrievedAt) and the SDK's computed ok/done getters would both fail output validation if the SDK object were passed through as-is.
  • Errors are unchanged. They stay text-only and are exempt from validation (McpServer.validateToolOutput returns early on isError).

The text block stays — and what that costs

The spec asks a tool returning structured content to also return a functionally equivalent unstructured copy, and dropping it would silently blank the tool on hosts that don't read structured output.

That duplication has a measured price. On an 8.5 KB article the markdown appears in both shapes:

bytes
text block 8,737
structuredContent 8,886
total on wire 2.02x the text-only baseline

Reviewer call: whether the model pays for both halves or just the structured one depends on the host, which I did not measure — I'm only claiming the wire cost. If the trade isn't worth it, cutting the text block is a one-line change in ok().

Verification

npm test11/11, typecheck clean, against main's @types/node 26.1.1.

The new suite stubs globalThis.fetch rather than the client methods, so each case drives the whole chain: canned API JSON → the real SDK parser (snake_case → camelCase, Date coercion, the computed getters) → jsonify → the MCP SDK's own output validation. A passing call is therefore itself proof of schema conformance.

I confirmed the tests detect the bug rather than trusting a green run — with the source reverted and the tests kept, 8 of 9 fail:

✖ every tool advertises an outputSchema
✖ extract returns typed structured content alongside the text block
✖ get_job returns job state as real JSON types, not prose
✖ crawl carries its crawl-only fields through the shared job schema
✖ search returns per-result status as a boolean
✖ get_usage returns numbers, not a formatted percentage line
   … 8 failed, 1 passed

The 9th passes either way by design: it guards the error path, asserting a failure still returns isError without tripping output validation.

test/structured-output.test.mjs is wired into npm test by explicit filename. smoke.mjs makes a live API call, so a directory glob would have pulled the network into CI.

Not verified

No run against a live WellMarked API or a real MCP host — every test here is hermetic. Worth exercising the remote connector once before release, since http.ts and index.ts share createServer() and the parity test only pins that the two agree.

Context

This came out of an audit for text/* responses across the API and website. Both were already 100% application/json (all API routes plus 75 website routes checked, zero non-JSON), so the "model parses JSON as text" problem lived here rather than in the HTTP layer.

🤖 Generated with Claude Code

Every tool flattened its result into a hand-formatted text blob, so an
agent asking for job state got

    Job abc [crawl] - status: running (12/50)
    Created: 2026-01-01T09:14:22Z | Finished: -

and had to pattern-match `status` out of a sentence, split `12/50` on a
slash, and read an em dash as null. The JS SDK already returns a typed
BulkJob; this layer was the only place throwing that structure away.

Each tool now declares an outputSchema and returns structuredContent
next to the existing text block. One shared job schema covers bulk,
crawl, get_job, and wait_for_job, since get_job is polymorphic by
design; crawl-only fields (depth, truncated, truncatedReason) are
optional and discriminated by `kind`.

jsonify() is load-bearing rather than cosmetic: Date fields
(createdAt, finishedAt, retrievedAt) and the SDK's computed ok/done
getters would both fail output validation if the SDK object were passed
through as-is.

The text block stays. The spec asks a tool returning structured content
to also return a functionally equivalent unstructured copy, and dropping
it would silently blank the tool on hosts that don't read structured
output. That costs payload: on an 8.5 KB article the markdown appears in
both shapes, measured at 2.02x the text-only baseline.

Errors stay text-only and are exempt from validation
(McpServer.validateToolOutput returns early on isError), so the error
path is unchanged.

Tests stub globalThis.fetch rather than the client methods, so each case
drives the whole chain: canned API JSON -> the real SDK parser -> jsonify
-> the MCP SDK's own output validation. A passing call is therefore proof
of schema conformance. Verified they detect the bug: with the source
reverted and the tests kept, 8 of 9 fail. The 9th passes either way by
design -- it guards the error path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@gradill22 gradill22 self-assigned this Jul 26, 2026
@gradill22

Copy link
Copy Markdown
Contributor Author

Verified against the local dev environment

Booted the API on :8000 against the dev Postgres + Redis, registered a throwaway account, then drove the real MCP server through an in-memory MCP client. The only thing faked was the hostname — globalThis.fetch rewrote api.wellmarked.iolocalhost:8000. Everything else was the real path:

MCP tool call → wellmarked SDK → real HTTP → FastAPI → real extraction
              → SDK parser → jsonify → MCP output validation → client

All 7 tools returned structuredContent, each validated by the MCP SDK against its declared outputSchema:

Tool Result Evidence it's real, not canned
get_usage plan:"free", limit:1000 as numbers
extract metadata.title:"Example Domain" from the live page; real ISO retrievedAt; tokensSaved:118
bulk 2 URLs, completed:2, done:true, per-item ok:true
crawl kind:"crawl", truncated:false, truncatedReason:null, results[].depth:0
get_job verified on both bulk-shaped and crawl-shaped jobs
wait_for_job crawl job, full results
search Wikipedia ok:true; Britannica ok:false / target_http_error

search is the clearest illustration of the change: a real mixed success/failure arrived as typed booleans rather than / glyphs for a model to interpret.

The error path was also confirmed organically rather than by construction — every rate-limit 429 encountered during testing returned isError:true, text-only, with no structuredContent, and never tripped output validation. That exercises the validateToolOutput early-return under real conditions, not just against a stub.

Unrelated issue found while testing

Not caused by this PR and not touched by it, but worth recording: the SDK's polymorphic getJob() on a crawl job issues GET /bulk/{id} then GET /crawl/{id} back-to-back with no spacing, and the API's own min-spacing limiter rejects the second:

GET /bulk/{id}  -> 200
GET /crawl/{id} -> 429  rate_limit_too_fast   Retry-After-Ms: 38

Client pacing of ~400ms between requests was needed to get crawl / get_job / wait_for_job through at all. Locally there is no network latency to space the two calls naturally, so how often this bites in production is unmeasured. Tracking it separately.

Not covered

No run against a real MCP host (Claude Desktop / Cursor) — the client here is the SDK's in-memory transport.

@gradill22
gradill22 merged commit 53a0127 into main Jul 26, 2026
3 checks passed
@gradill22
gradill22 deleted the feat/structured-tool-output branch July 26, 2026 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant