docs: load media examples from disk instead of inline base64#3108
Conversation
📚 Documentation preview
|
maxisbey
force-pushed
the
docs-media-path-examples
branch
2 times, most recently
from
2e0ab9a to
4d2f50e
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
The Image/Audio tutorials fed the helpers base64-decoded one-pixel placeholders, so the first thing a reader met was an opaque blob. Point them at logo.png / chime.wav beside the server instead, anchored to the script's own directory so hosts that launch stdio servers from an arbitrary working directory still resolve them. The page teaches path= first and keeps data= + format= for bytes that never touch disk. Tests fabricate the assets in tmp_path and repoint the module constants, so no binary lands in the repo.
maxisbey
force-pushed
the
docs-media-path-examples
branch
from
4d2f50e to
1565b87
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| @pytest.mark.usefixtures("logo_file") | ||
| async def test_image_result_has_no_structured_content_and_no_output_schema() -> None: | ||
| """tutorial001: media is content for the model, not data for the application.""" | ||
| async with Client(tutorial001.mcp) as client: |
There was a problem hiding this comment.
🟡 test_image_result_has_no_structured_content_and_no_output_schema asserts result.structured_content is None without first asserting not result.is_error — and now that this PR makes the logo tool fallible (Image(path=...) does file I/O), an error result also has structured_content=None, so the key assertion passes vacuously on the failure path. Add assert not result.is_error before the structured_content check, matching test_audio_return_becomes_an_audio_content_block added in this same PR.
Extended reasoning...
The bug. test_image_result_has_no_structured_content_and_no_output_schema (tests/docs_src/test_media.py:41-48) pins the doc claim "media is content for the model, not data for the application" by asserting tool.output_schema is None and result.structured_content is None. It never asserts not result.is_error. An error CallToolResult also has structured_content=None, so the structured-content assertion cannot distinguish the documented success behaviour from a failed tool call — it passes on both paths.
Why this PR introduced it. Before this PR, the logo tool returned inline bytes (Image(data=LOGO_PNG, format="png")) and could not fail. After it, the tool does file I/O: Image(path=LOGO_FILE) opens the file when the result is built, inside convert_result. Any exception there is caught by MCPServer._handle_call_tool (src/mcp/server/mcpserver/server.py:412-413), which returns CallToolResult(content=[TextContent(...)], is_error=True) with structured_content defaulting to None — it does not raise client-side. The PR touches this exact test (adding @pytest.mark.usefixtures("logo_file")), so the missing guard is squarely in scope.
Step-by-step proof of the vacuous pass. Suppose a future edit inlines Path(__file__).parent / "logo.png" in the tool body while leaving the LOGO_FILE constant in place (the logo_file fixture's monkeypatch.setattr(tutorial001, "LOGO_FILE", path) still succeeds silently — it patches a name nothing reads anymore):
- The test calls
client.call_tool("logo", {}). - The tool body evaluates
Image(path=...); building the result opens the missing file →FileNotFoundError. _handle_call_toolcatches it and returnsCallToolResult(is_error=True, structured_content=None).assert tool.output_schema is None— passes (unrelated to the call).assert result.structured_content is None— passes, because error results carry no structured content.- The test goes green while no longer proving the doc claim it exists to pin.
Why nothing else fully prevents it. The sibling test_image_return_becomes_an_image_content_block uses the same fixture and does assert not result.is_error plus exact content, so a fixture breakage would not be silent at the suite level — that is what keeps this at nit rather than normal. But this individual test's proof is still hollow, and the repo's own bar makes that a defect, not a style preference: .claude/skills/test-quality/SKILL.md's Hollow-proof check requires each claim to be proven by an assertion that cannot pass on an unrelated path, and AGENTS.md mandates conformance to that skill for test work.
The fix. One line, matching the pattern the same PR uses in test_audio_return_becomes_an_audio_content_block:
result = await client.call_tool("logo", {})
assert not result.is_error
assert result.structured_content is None1 participant
Rework the media docs examples to load their files from disk instead of embedding a base64 blob.
Motivation and Context
The image/audio tutorials on the media page opened with
LOGO_PNG = base64.b64decode("iVBORw0KGgo...")— the first thing a reader met was an opaque blob rather than the API being taught, and it read as if conjuring base64 by hand were part of the job.The examples now show the realistic pattern:
Image(path=...)/Audio(path=...)pointing atlogo.png/chime.wavbeside the server. The paths are anchored withPath(__file__).parentso the example keeps working when a host launches the stdio server from an arbitrary working directory (hosts generally do — a bare relative path would resolve against wherever that happens to be), and the page now says so explicitly. Thedata=+format=mode is still taught in "Bytes or a file", reframed as the mode for bytes that never touch disk (Pillow output, screenshots, ...).No binary asset is committed: the tests fabricate the files in
tmp_pathand repoint the tutorials' module-level path constants (the same pattern FastAPI's docs tests use for itsFileResponseexamples). The SDK never parses the contents, so opaque bytes suffice.Also fixed while here: the audio wire-format illustration showed the base64 of the now-deleted placeholder (its
RIFFsize field included), which no reader's own WAV would reproduce — truncated to the universal prefix.How Has This Been Tested?
tests/docs_src/test_media.pyupdated: fabricated assets intmp_path, exactImageContent/AudioContentround-trip assertions, plus a new test pinning thatpath=files are read when the result is built (which is what makes the fabricate-then-call pattern sound).mcp run+stdio_client): image and audio round-trip byte-for-byte with the files on disk; a missing file surfaces as a clean tool error naming the path; launching the server from a different working directory still resolves the files (the__file__anchoring at work).Breaking Changes
None — docs and tests only.
Types of changes
Checklist
Additional context
The old examples'
data=mode (with theformat=caveat) is still covered bytest_raw_data_without_a_format_falls_back_to_a_default_mime_typeand the!!! checkadmonition — nothing the page previously taught was dropped, the two construction modes just swapped billing.AI Disclaimer