Repository navigation
Code review found 4 potential issues
Found 4 candidates, confirmed 4. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 3 |
| 🟣 Pre-existing | 1 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/mcp/server/mcpserver/prompts/base.py:205 |
[quality] Rewritten render() conversion loop re-adds 'else: # pragma: no cover' on the JSON-dump fallback — the only |
| 🟡 Nit | src/mcp/server/mcpserver/utilities/func_metadata.py:246 |
[quality] nit: in-code docs for the new content rule omit 'Sequence': the 'structured_output' docstring bullet (lines 24 |
| 🟡 Nit | docs/servers/structured-output.md:217 |
[quality] nit: stale claim "There is one way to end up unstructured without asking for it" now contradicts the new "Cont |
| 🟣 Pre-existing | src/mcp/server/mcpserver/prompts/base.py:207 |
Pre-existing, surfaced by this rewrite: a prompt function returning the wire type mcp_types.PromptMessage (the type GetP |
Annotations
Check warning on line 205 in src/mcp/server/mcpserver/prompts/base.py
claude / Claude Code Review
[quality] Rewritten render() conversion loop re-adds `else: # pragma: no cover` on the JSON-dump fallback — the only pragma line the AGENTS.md diff audit flags — even though the branch is trivially testable and the PR presents this fallback as retain
[quality] Rewritten render() conversion loop re-adds `else: # pragma: no cover` on the JSON-dump fallback — the only pragma line the AGENTS.md diff audit flags — even though the branch is trivially testable and the PR presents this fallback as retained, documented behavior.
Check warning on line 246 in src/mcp/server/mcpserver/utilities/func_metadata.py
claude / Claude Code Review
[quality] nit: in-code docs for the new content rule omit `Sequence`: the `structured_output` docstring bullet (lines 244-246, "bare or inside a list, tuple or union") and `_returns_content`'s own docstring (lines 42-43, "items of a list/tuple") both unde
[quality] nit: in-code docs for the new content rule omit `Sequence`: the `structured_output` docstring bullet (lines 244-246, "bare or inside a list, tuple or union") and `_returns_content`'s own docstring (lines 42-43, "items of a list/tuple") both understate `_CONTENT_SEQUENCE_ORIGINS = (list, tuple, Sequence)` on line 38. The review round fixed only docs/servers/structured-output.md (which now correctly says "list, tuple or Sequence"); the docstrings shipped in this same diff were not swept.
Check warning on line 217 in docs/servers/structured-output.md
claude / Claude Code Review
[quality] nit: stale claim "There is one way to end up unstructured without asking for it" now contradicts the new "Content blocks and media" section added two paragraphs above on the same page, which introduces a second default opt-out path (content-bloc
[quality] nit: stale claim "There is one way to end up unstructured without asking for it" now contradicts the new "Content blocks and media" section added two paragraphs above on the same page, which introduces a second default opt-out path (content-block/Image/Audio return annotations derive no schema). The PR updated the Recap bullet at line 247 to list both opt-outs ("Content blocks, `Image` and `Audio` opt out by default; a class without type hints opts out silently") but left the section o
Check notice on line 207 in src/mcp/server/mcpserver/prompts/base.py
claude / Claude Code Review
Pre-existing, surfaced by this rewrite: a prompt function returning the wire type mcp_types.PromptMessage (the type GetPromptResult actually carries, and what lowlevel-server prompt handlers return) falls through the rewritten conversion loop to the JSON-
Pre-existing, surfaced by this rewrite: a prompt function returning the wire type mcp_types.PromptMessage (the type GetPromptResult actually carries, and what lowlevel-server prompt handlers return) falls through the rewritten conversion loop to the JSON-dump fallback: it is not a mcpserver Message, not a dict, and not in the new `str | ContentBlock | Image | Audio` arm, so `pydantic_core.to_json(msg)` stringifies it and `Message(role="user", content=...)` hardcodes the role. The loop was just w