feat: add IconTheme type for light/dark mode icon support#3163
feat: add IconTheme type for light/dark mode icon support#3163gfortaine wants to merge 1 commit intoPrefectHQ:mainfrom
Conversation
Test Failure AnalysisSummary: Type checker () errors because from (v1.x) doesn't support the Root Cause: The PR adds The type checker correctly identifies 4 instances where
Suggested Solution: The Option 1: Remove
Option 2: Mark tests as xfail/skip until MCP SDK v2+
Detailed AnalysisType Checker ErrorsAll 4 errors are from the Verification# Check Icon model fields
$ uv run python -c "from mcp.types import Icon; print(Icon.model_fields.keys())"
dict_keys(['src', 'mimeType', 'sizes'])Code Added in PRThe PR correctly adds the # src/fastmcp/utilities/types.py
try:
from mcp.types import IconTheme
except ImportError:
IconTheme: TypeAlias = Literal["light", "dark"] # type: ignore[no-redef]But the tests assume Related Files
|
WalkthroughAdds an 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/fastmcp/__init__.py (1)
21-21: Consider whetherIconThemewarrants top-level export.The existing top-level exports (
FastMCP,Client,Context) are core abstractions.IconThemeis a narrow auxiliary type—users who need it could import fromfastmcp.utilities.typesdirectly. That said, the PR objective explicitly requests this, and the doc examples usefrom fastmcp import IconTheme, so this is a deliberate choice.Just flagging for intentionality per the project's re-export guideline. If the maintainer is comfortable with it, no change needed. Based on learnings: "only re-export to
fastmcp.*for the most fundamental types."Also applies to: 35-35
docs/servers/icons.mdx (1)
67-89: Consider simplifying the example by passing string literals directly.The intermediate variables
light_themeanddark_themeexist only to showcase the type annotation but add verbosity. A simpler example that passestheme="light"directly (with a note aboutIconThemefor type checking) would be more idiomatic and easier to copy-paste.
Add IconTheme type alias and document the theme field on Icon for parity with the MCP Python SDK and Go SDK. This allows servers to specify whether an icon is designed for a light or dark background, enabling clients to select the appropriate icon based on their UI theme. Changes: - Add IconTheme = Literal['light', 'dark'] to fastmcp.utilities.types with forward-compatible import from mcp.types when available (v2+) - Export IconTheme from fastmcp package - Document theme field and theme variants in icon docs - Add tests for icon theme support Closes jlowin#3162
1e67eab to
7af0676
Compare
Description
Add
IconThemetype alias and document thethemefield onIconfor parity with the MCP Python SDK and Go SDK. This allows servers to specify whether an icon is designed for a light or dark background, enabling clients to select the appropriate icon based on their UI theme.This complements the existing icon support (added in v2.13.0), adding theme-aware icon variants that MCP clients like VS Code and GitHub Desktop can use to match their UI appearance.
Changes
src/fastmcp/utilities/types.py: AddIconTheme = Literal["light", "dark"]with a forward-compatible import frommcp.types(for when MCP SDK v2+ is available)src/fastmcp/__init__.py: ExportIconThemefrom thefastmcppackagedocs/servers/icons.mdx&docs/v2/servers/icons.mdx: Document thethemefield and add a "Theme Variants" section with usage examplestests/utilities/test_inspect.py: Add 4 tests for icon theme supportMCP Python SDK Reference
The MCP Python SDK added this in modelcontextprotocol/python-sdk#1978:
Go SDK Reference
Use Case
MCP servers like GitHub MCP Server provide both light and dark variants of their icons. The
IconThemetype enables type-safe theme specification:Implementation Note
Since FastMCP currently pins to
mcp>=1.24.0,<2.0and thethemefield was added to the MCP SDK'smain(v2 development), this PR definesIconThemelocally with a forward-compatibletry/exceptimport. When MCP SDK v2 is available, FastMCP will automatically use the upstream type. TheIconmodel already accepts extra fields viaextra='allow', so thethemefield works at runtime today.Generated using GitHub Copilot.
Contributors Checklist
Review Checklist