SEP-1577: Sampling with tools #2551
Conversation
- Add tools and result_type parameters to ctx.sample() - Update OpenAI handler for tool content types - Client advertises sampling.tools capability by default - Collect tool results into single message with list content
marvin-context-protocol
bot
added
feature
Test Failure AnalysisSummary: All CI jobs are failing during dependency installation because the project requires Root Cause: This PR updates As noted in the PR description, this PR depends on modelcontextprotocol/python-sdk#1722 being merged and released first. That upstream PR is currently still open and unmerged. Suggested Solution: These test failures are expected and documented in the PR description. No action is needed at this time. The PR should remain in draft or marked as blocked until:
Once the dependency is available, the tests should pass without any code changes to this PR. Detailed AnalysisError Message from All JobsAffected JobsAll 4 CI jobs failed at the
Dependency Chain
Related Files
🤖 This analysis was automatically generated and updated. The failure is expected per the PR description. Edit: Updated analysis for workflow run 19951617050 (2025-12-05 03:24 UTC) |
WalkthroughThis pull request introduces comprehensive tool-assisted sampling support to FastMCP. The changes include documentation for both client and server sampling workflows with tools, new example scripts demonstrating structured outputs and tool-enabled sampling, updates to the OpenAI sampling handler to convert and process MCP tools, a new Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/fastmcp/experimental/sampling/handlers/openai.py (1)
51-86: Fix temperature=0.0 being dropped due to falsy evaluationThe
temperature=params.temperature or NOT_GIVENpattern on line 81 incorrectly treats0.0as falsy, sendingNOT_GIVENto the OpenAI API instead of the valid temperature value. Sincetemperatureis typed asfloat | None, an explicit0.0is distinct fromNoneand should be preserved.Change to:
- temperature=params.temperature or NOT_GIVEN, + temperature=( + params.temperature + if params.temperature is not None + else NOT_GIVEN + ),This preserves valid zero values while still defaulting
NonetoNOT_GIVEN. (Note:stopSequenceson line 83 has the same pattern and should also be reviewed.)
🧹 Nitpick comments (9)
src/fastmcp/server/server.py (1)
42-45: SDK-prefixed MCP types and list_ handlers look consistent*The new SDK* aliases and
_list_{tools,resources,resource_templates,prompts}_mcpmethods correctly map internal components tomcp.typesSDK objects using theto_mcp_*helpers and the.key/uriTemplateconventions, which preserves prefixing and filtering semantics across mounted servers and middleware. The extraContextlayer in both the*_mcpand*_middlewarefunctions is slightly redundant but not harmful; you could later simplify by having only the middleware layer own the request context, mirroring_call_tool_middleware, if you want to reduce nesting.Also applies to: 646-662, 730-746, 819-835, 913-928
docs/servers/sampling.mdx (1)
418-440: Clarify pseudo-code in the custom loop exampleThe
custom_agentexample uses placeholders likesome_conditionanddifferent_toolthat make the snippet non-runnable as-is. Consider adding a short comment (e.g.,# pseudo-code: define some_condition/different_tool as needed) so users don’t mistake it for a copy‑paste runnable example, given the repo’s usual “runnable examples” convention.examples/advanced_sampling/tool_use.py (1)
72-85: EnsureCallToolResult.datais populated for this exampleThe example prints
result.data["summary"]/["sources_used"]/["confidence"]. This relies onClient.call_toolsuccessfully parsing structured output intoCallToolResult.datausing the tool’soutput_schema. Please double‑check that theresearchtool is registered with an appropriateoutput_schema(or auto‑derived one) so thatdatais notNone; otherwise these subscripts will raise at runtime and you may need to read fromresult.structured_contentinstead.examples/advanced_sampling/structured_output.py (1)
47-65: Confirm structured data reachesCallToolResult.dataAs in the tool-use example, this script prints
result.data["sentiment"],["confidence"], and["keywords"]. Please verify that the tool’s structured output is being exposed viaCallToolResult.data(through a properoutput_schema), otherwisedatamay beNoneand these lines will fail. If needed, you could fall back toresult.structured_contentor wrap the output schema explicitly on the server side.src/fastmcp/client/client.py (1)
317-331: set_sampling_callback mirrors init behavior; watch for capability overridesThe updated
set_sampling_callbackcorrectly updates both the sampling callback andsampling_capabilities, defaulting to tools-enabled sampling when no explicit capabilities are provided, consistent with__init__. One thing to be aware of: callingset_sampling_callbackwithoutsampling_capabilitiesafter constructing a client with custom capabilities will overwrite them with the default tools-capability. If you expect users to swap handlers while preserving custom capabilities, consider documenting this or acceptingsampling_capabilities=...in those call sites.src/fastmcp/experimental/sampling/handlers/openai.py (1)
324-338: Tool choice mapping currently supports mode-only semantics
_convert_tool_choice_to_openaimaps MCPToolChoice.modevalues"auto","required", and"none"directly to OpenAI’s accepted values, defaulting to"auto"for anything unknown. If MCP’sToolChoiceever gains a “specific tool only” mode, this would need to be extended (e.g., to return{"type": "function", "function": {"name": ...}}). For now, with mode-only semantics, this is fine.src/fastmcp/server/context.py (3)
634-642: Clarify type identity check withstr.The condition
result_type is not struses identity comparison, which works here because you're checking if the type itself is thestrclass. However, this won't catch subclasses or type aliases. If that's intentional, consider adding a comment for clarity; otherwise, you might want a more robust check.
1003-1018: Bareexcept Exceptionis acceptable here but consider logging.The broad exception catches are justified since Pydantic validation and user-provided tool functions can raise various exception types. However, silently catching errors without logging may make debugging difficult.
Consider adding debug logging before returning error results:
except Exception as e: + logger.debug(f"Tool '{tool_use.name}' failed: {e}") tool_results.append( ToolResultContent(Also applies to: 1046-1054
1166-1184: Inconsistent type checks forresponse_type.The checks at lines 1168-1184 use
isinstance(response_type, dict)andisinstance(response_type, list), butresponse_typeis declared astype[T] | list[str] | dict[str, dict[str, str]] | None. When it's adictorlistvalue (not a type), these runtime checks make sense—but the logic is hard to follow and could be simplified.Consider extracting this into a helper or adding comments explaining the multi-select/single-select distinction.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
pyproject.tomlis excluded by none and included by nonetests/client/test_sampling.pyis excluded by none and included by nonetests/server/middleware/test_tool_injection.pyis excluded by none and included by nonetests/server/sampling/__init__.pyis excluded by none and included by nonetests/server/sampling/test_sampling_tool.pyis excluded by none and included by nonetests/server/test_context.pyis excluded by none and included by noneuv.lockis excluded by!**/*.lockand included by none
📒 Files selected for processing (18)
docs/clients/sampling.mdx(2 hunks)docs/servers/sampling.mdx(11 hunks)examples/advanced_sampling/README.md(1 hunks)examples/advanced_sampling/structured_output.py(1 hunks)examples/advanced_sampling/tool_use.py(1 hunks)src/fastmcp/client/client.py(3 hunks)src/fastmcp/client/transports.py(1 hunks)src/fastmcp/experimental/sampling/handlers/openai.py(5 hunks)src/fastmcp/prompts/prompt.py(2 hunks)src/fastmcp/resources/resource.py(2 hunks)src/fastmcp/resources/template.py(3 hunks)src/fastmcp/server/context.py(13 hunks)src/fastmcp/server/proxy.py(1 hunks)src/fastmcp/server/sampling/__init__.py(1 hunks)src/fastmcp/server/sampling/handler.py(2 hunks)src/fastmcp/server/sampling/sampling_tool.py(1 hunks)src/fastmcp/server/server.py(5 hunks)src/fastmcp/tools/tool.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/.cursor/rules/mintlify.mdc)
docs/**/*.mdx: Use clear, direct language appropriate for technical audiences
Write in second person ('you') for instructions and procedures in MDX documentation
Use active voice over passive voice in MDX technical documentation
Employ present tense for current states and future tense for outcomes in MDX documentation
Maintain consistent terminology throughout all MDX documentation
Keep sentences concise while providing necessary context in MDX documentation
Use parallel structure in lists, headings, and procedures in MDX documentation
Lead with the most important information using inverted pyramid structure in MDX documentation
Use progressive disclosure in MDX documentation: present basic concepts before advanced ones
Break complex procedures into numbered steps in MDX documentation
Include prerequisites and context before instructions in MDX documentation
Provide expected outcomes for each major step in MDX documentation
End sections with next steps or related information in MDX documentation
Use descriptive, keyword-rich headings for navigation and SEO in MDX documentation
Focus on user goals and outcomes rather than system features in MDX documentation
Anticipate common questions and address them proactively in MDX documentation
Include troubleshooting for likely failure points in MDX documentation
Provide multiple pathways (beginner vs advanced) but offer an opinionated path to avoid overwhelming users in MDX documentation
Always include complete, runnable code examples that users can copy and execute in MDX documentation
Show proper error handling and edge case management in MDX code examples
Use realistic data instead of placeholder values in MDX code examples
Include expected outputs and results for verification in MDX code examples
Test all code examples thoroughly before publishing in MDX documentation
Specify language and include filename when relevant in MDX code examples
Add explanatory comments for complex logic in MDX code examples
Document all API...
Files:
docs/servers/sampling.mdxdocs/clients/sampling.mdx
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bareexcept- be specific with exception types
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns even if shorter
Follow existing patterns and maintain consistency in code organization and style
Files:
src/fastmcp/resources/resource.pysrc/fastmcp/server/sampling/handler.pysrc/fastmcp/resources/template.pysrc/fastmcp/prompts/prompt.pysrc/fastmcp/server/server.pysrc/fastmcp/client/client.pysrc/fastmcp/tools/tool.pysrc/fastmcp/client/transports.pysrc/fastmcp/server/context.pysrc/fastmcp/server/sampling/__init__.pysrc/fastmcp/experimental/sampling/handlers/openai.pysrc/fastmcp/server/proxy.pysrc/fastmcp/server/sampling/sampling_tool.py
🧠 Learnings (3)
📚 Learning: 2025-11-26T21:51:44.174Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2025-11-26T21:51:44.174Z
Learning: Note that Resources and Resource Templates are distinct objects but both handled by ResourceManager, requiring coordinated updates when changes affect either object type
Applied to files:
src/fastmcp/resources/template.py
📚 Learning: 2025-11-26T21:51:44.174Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2025-11-26T21:51:44.174Z
Learning: When implementing changes that affect MCP object types (Tools, Resources, Resource Templates, or Prompts), such as adding tags or importing, ensure changes are adopted, applied, and tested on all four object types
Applied to files:
src/fastmcp/resources/template.pysrc/fastmcp/server/server.py
📚 Learning: 2025-11-26T21:51:44.174Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2025-11-26T21:51:44.174Z
Learning: Review and update related Manager classes (ToolManager, ResourceManager, PromptManager) when modifying MCP object definitions
Applied to files:
src/fastmcp/prompts/prompt.pysrc/fastmcp/server/server.pysrc/fastmcp/tools/tool.py
🧬 Code graph analysis (6)
src/fastmcp/resources/template.py (1)
src/fastmcp/server/proxy.py (1)
from_mcp_template(366-382)
examples/advanced_sampling/tool_use.py (3)
src/fastmcp/server/context.py (2)
fastmcp(190-195)Context(138-1246)src/fastmcp/client/client.py (2)
Client(107-1023)call_tool(939-1015)src/fastmcp/experimental/sampling/handlers/openai.py (1)
OpenAISamplingHandler(46-395)
src/fastmcp/server/server.py (3)
src/fastmcp/prompts/prompt.py (1)
Prompt(64-153)src/fastmcp/resources/resource.py (1)
Resource(33-155)src/fastmcp/tools/tool.py (1)
Tool(116-258)
src/fastmcp/client/client.py (1)
src/fastmcp/client/sampling.py (1)
create_sampling_callback(31-56)
src/fastmcp/server/context.py (1)
src/fastmcp/server/sampling/sampling_tool.py (4)
SamplingTool(18-138)from_mcp_tool(79-102)_to_sdk_tool(66-76)run(49-64)
src/fastmcp/experimental/sampling/handlers/openai.py (2)
src/fastmcp/tools/tool.py (1)
Tool(116-258)src/fastmcp/server/server.py (4)
name(327-328)tool(1320-1334)tool(1337-1351)tool(1353-1488)
🪛 LanguageTool
docs/servers/sampling.mdx
[style] ~410-~410: Try using a synonym here to strengthen your writing.
Context: ...ling iteration and returns immediately, giving you access to result.history so you can c...
(GIVE_PROVIDE)
🪛 Ruff (0.14.7)
src/fastmcp/server/context.py
630-632: Avoid specifying long messages outside the exception class
(TRY003)
686-689: Avoid specifying long messages outside the exception class
(TRY003)
785-788: Avoid specifying long messages outside the exception class
(TRY003)
797-800: Avoid specifying long messages outside the exception class
(TRY003)
852-854: Avoid specifying long messages outside the exception class
(TRY003)
1003-1003: Do not catch blind exception: Exception
(BLE001)
1046-1046: Do not catch blind exception: Exception
(BLE001)
src/fastmcp/experimental/sampling/handlers/openai.py
270-270: Avoid specifying long messages outside the exception class
(TRY003)
345-345: Avoid specifying long messages outside the exception class
(TRY003)
388-388: Avoid specifying long messages outside the exception class
(TRY003)
src/fastmcp/server/sampling/sampling_tool.py
92-95: Avoid specifying long messages outside the exception class
(TRY003)
131-131: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (25)
src/fastmcp/server/proxy.py (1)
558-571: LGTM! Sampling handler updated to new SamplingResult API.The change from capturing raw content to using
result.textaligns with the new SamplingResult container introduced in this PR. The fallback to an empty string is appropriate for handling cases where text may be None.docs/clients/sampling.mdx (1)
178-211: Verify that code examples have been tested.The code examples demonstrating tool-enabled sampling are comprehensive and well-structured. Please confirm that these examples have been tested and are executable as written.
Based on coding guidelines, MDX documentation requires that "all code examples are syntactically correct and executable."
src/fastmcp/prompts/prompt.py (1)
12-13: Prompt → SDKPrompt migration is cleanUsing
SDKPrompt/SDKPromptArgumentand updating theto_mcp_promptsignature keeps behavior identical while aligning with the new SDK-prefixed type naming used by the server. No functional issues spotted.Also applies to: 87-103
docs/servers/sampling.mdx (1)
13-404: Sampling, tools, and structured-output docs align with the implementationThe page clearly explains
ctx.sample,SamplingResult,result_type, tool loops,tool_choice, and OpenAI handlers, and the examples are consistent with the new client/server APIs (includingsampling_handler_behaviorand tool-capability advertising). This should be very helpful for users wiring up agentic sampling flows.Also applies to: 451-568, 579-581
examples/advanced_sampling/tool_use.py (1)
25-69: Tool-using research workflow example is consistent with sampling APIsThe sampling tools,
ResearchReportmodel, andresearchtool correctly demonstratectx.samplewithtools=[...],result_type=ResearchReport, andmax_iterations, matching the documented behavior. Returningresult.result.model_dump()from the tool is a reasonable choice for exposing structured output to clients.src/fastmcp/resources/template.py (1)
12-13: ResourceTemplate ↔ SDKResourceTemplate conversions look correctSwitching to
SDKResourceTemplateand updatingto_mcp_template/from_mcp_templatekeeps the existing field mapping (name,uriTemplate,mimeType,icons,annotations,_meta) intact while aligning with the rest of the SDK-prefixed MCP types. No behavioral regression evident.Also applies to: 193-225
examples/advanced_sampling/structured_output.py (1)
24-45: Structured-output sentiment tool example matchesresult_typebehaviorThe
SentimentAnalysismodel andanalyze_sentimenttool nicely illustrate usingctx.sample(..., result_type=SentimentAnalysis)and returningresult.result.model_dump()so the client can consume a plain dict. This is consistent with the structured-output semantics described in the docs.src/fastmcp/client/client.py (1)
215-241: Client now cleanly advertises sampling capabilities when a handler is setAdding the
sampling_capabilitiesparameter and wiring_session_kwargs["sampling_capabilities"]whensampling_handleris provided ensures clients advertisesampling.toolsby default (viaSamplingCapability(tools=SamplingToolsCapability())), matching the docs about tool-enabled sampling. Passing an explicitsampling_capabilitieslets advanced users fine-tune this. The new argument is backward compatible and well-scoped.Also applies to: 278-289
src/fastmcp/experimental/sampling/handlers/openai.py (4)
21-36: OpenAI imports and type wiring for tools are correctThe additional imports for
CreateMessageResultWithTools,StopReason,Tool*content types, and OpenAI tool-related params (ChatCompletionToolParam,ChatCompletionMessageToolCallParam,ChatCompletionToolChoiceOptionParam,FunctionDefinition, etc.) match how they are used later in the handler. This keeps the module’s public surface aligned with MCP’s sampling/tool abstractions.
119-271: Conversion of SamplingMessages to OpenAI messages covers text, tool use, and tool resultsThe extended
_convert_to_openai_messageshandles:
- Raw string prompts,
- Text-only
SamplingMessageinstances,- Lists of
TextContent,ToolUseContent, andToolResultContent(from prior tool-enabled iterations),- Standalone
ToolUseContentandToolResultContent.It builds the right OpenAI message types (
user,assistantwithtool_calls, andtoolmessages) and raises a clear error for unsupported content types. This should allow multi-iteration tool loops to round-trip correctly between MCP and OpenAI.
302-322: _convert_tools_to_openai correctly maps MCP tools to OpenAI function toolsMapping each MCP
Tool’sname,description, andinputSchemato an OpenAIChatCompletionToolParamwith aFunctionDefinitionlooks correct, and the defensivetype="object"default for parameters is helpful. Just ensuretool.inputSchemais always a mapping (as produced by the SDK) sodict(tool.inputSchema)is safe.
340-395: CreateMessageResultWithTools construction mirrors OpenAI tool-call responses
_chat_completion_to_result_with_tools:
- Validates there is at least one choice,
- Maps OpenAI
finish_reasonto MCPStopReason(tool_calls→"toolUse",stop→"endTurn",length→"maxTokens"),- Builds a
contentlist containing aTextContent(if present) and oneToolUseContentper tool call (parsing JSON arguments),- Raises if there is no content at all.
This should interoperate cleanly with the MCP sampling/tool loop machinery.
src/fastmcp/server/context.py (7)
79-99: Well-designed generic result container.The
SamplingResult[ResultT]dataclass cleanly encapsulates text, typed result, and history. The docstring is clear and explains the purpose of each field.
512-559: Overloads correctly enable type inference.The two overloads properly distinguish between structured (
result_typespecified) and text-only sampling, allowing callers to benefit from static type checking on the return type.
1056-1063: Type ignore is acceptable for SDK compatibility.The
# type: ignore[arg-type]is necessary due to the SDK's type definitions. This is a pragmatic workaround for passing tool results back in the message flow.
1343-1358: Clean text extraction helper.The
_extract_text_from_contentfunction correctly handles both single blocks and lists, using duck typing to find text content. The early return on first text found is appropriate.
1318-1340: Well-structured synthetic tool creation.The
_create_final_response_toolfunction properly derives the JSON schema from the result type and creates a passthrough function for the tool. Schema compression withprune_titles=Trueis a good choice for reducing payload size.
678-689: Good capability validation before tool usage.Properly checks that the client advertises
sampling.toolscapability before attempting tool-enabled sampling. The error message clearly explains what's required.
274-288: Return type annotations updated for SDK consistency.The return types for
list_resourcesandlist_promptsnow useSDKResourceandSDKPromptaliases, aligning with the broader SDK type standardization in this PR.src/fastmcp/server/sampling/sampling_tool.py (6)
18-48: Well-designed SamplingTool class.The class structure is clean with proper Pydantic configuration for the callable field. The docstring with examples demonstrates the intended usage patterns effectively.
49-64: Correct handling of sync and async tool functions.The
run()method properly defaultsNonearguments to an empty dict and handles both synchronous and asynchronous functions with theisawaitablecheck.
66-76: Clean SDK conversion method.The
_to_sdk_tool()method provides a straightforward conversion to the MCP SDK's Tool type. The underscore prefix appropriately marks it as internal.
78-102: Proper validation when converting from FastMCP Tool.The
from_mcp_tool()method correctly validates that the source tool has anfnattribute before conversion. The error message clearly explains that only FunctionTools can be converted.
141-187: Flexible decorator with proper overloads.The
sampling_tooldecorator correctly supports both bare usage (@sampling_tool) and parameterized usage (@sampling_tool(name="...")). The overloads enable proper type inference in both cases.Note that the decorated function is replaced by a
SamplingToolinstance, which is the intended behavior for use withcontext.sample(tools=[...]).
133-138: No issue found. The code correctly usesparsed.fnrather than the originalfnparameter.ParsedFunction.from_function()intentionally normalizes the function to handle callable classes (extracting__call__) and staticmethods (extracting__func__). Using the modifiedparsed.fnis required for proper execution, as demonstrated by therun()method's direct invocation viaself.fn(**arguments). This pattern is consistently applied across the codebase (e.g.,FunctionTool.from_function()uses the same approach). The design is sound.
| if iteration >= max_iterations: | ||
| # Return what we have with None text (tool use without completion) | ||
| return SamplingResult( | ||
| text=None, | ||
| result=cast(ResultT, None), | ||
| history=current_messages, | ||
| ) |
There was a problem hiding this comment.
Returning None as ResultT violates the type contract when result_type is specified.
When max_iterations is exhausted without a final response, the code returns result=cast(ResultT, None). If ResultT is a non-optional structured type (e.g., MyModel), this creates a type-unsafe state where callers expect a valid object but receive None.
Consider either:
- Raising an exception when iterations are exhausted without a valid result
- Changing the return type to
SamplingResult[ResultT | None]to make this explicit
# Check if we've hit max iterations
if iteration >= max_iterations:
- # Return what we have with None text (tool use without completion)
- return SamplingResult(
- text=None,
- result=cast(ResultT, None),
- history=current_messages,
- )
+ raise RuntimeError(
+ f"Sampling loop exhausted after {max_iterations} iterations "
+ "without receiving a final response from the LLM."
+ )🤖 Prompt for AI Agents
In src/fastmcp/server/context.py around lines 819 to 825, returning
result=cast(ResultT, None) breaks the type contract when ResultT is
non-optional; instead, change the behavior to raise a clear exception (e.g.,
IterationExhaustedError or RuntimeError) when max_iterations is reached without
a final result, and update the function signature/type hints and any
callers/tests that expect a SamplingResult with a possibly-None result to handle
the raised exception; ensure the raised exception includes context (iteration
count and any relevant state) so callers can handle or surface it.
| # Check if we've hit max iterations | ||
| if iteration >= max_iterations: | ||
| # Return what we have with None text (tool use without completion) | ||
| return SamplingResult( | ||
| text=None, | ||
| result=cast(ResultT, None), | ||
| history=current_messages, | ||
| ) |
There was a problem hiding this comment.
Same None result issue as in fallback handler.
This suffers from the same type contract violation when max_iterations is exhausted. Apply consistent fix here as well.
🤖 Prompt for AI Agents
In src/fastmcp/server/context.py around lines 914 to 921, the code returns a
SamplingResult with result=cast(ResultT, None) when max_iterations is reached,
which violates the ResultT contract; fix it to match the fallback handler
change: do not return None as the result — either raise the same
MaxIterationsExceededError used in the fallback handler or construct and return
a SamplingResult whose result is the explicit failure object used elsewhere
(e.g., a Failure/Err result instance) and keep text=None and
history=current_messages; ensure you import or create the same error/failure
type used by the fallback handler so behavior is consistent.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/fastmcp/experimental/sampling/handlers/openai.py (4)
154-216: Consider simplifying the list content handling logic.This block is quite complex with multiple nested conditionals and mixed responsibilities (collecting items, creating tool messages inline, then creating assistant messages). The logic:
- Iterates to collect tool_calls and text_parts
- Immediately creates tool messages for ToolResultContent during iteration
- After iteration, conditionally creates assistant/user messages
This mixed approach makes the flow hard to follow and maintain. Consider refactoring to separate concerns—perhaps first categorize all content items, then construct messages in a second pass.
Additionally, Line 192 concatenates text parts with newlines (
"\n".join(text_parts)). Verify this is the intended behavior, as it may relate to the past review comment about concatenation.
274-274: Make the error message more descriptive.The current error message
f"Unsupported content type: {type(content)}"is generic. Consider specifying which content types are supported (e.g., "Supported types: TextContent, ToolUseContent, ToolResultContent, or list").Note: Static analysis flagged this as a long message outside the exception class (TRY003). If following strict style guidelines, consider defining a custom exception or moving to a constant.
376-379: Consider more explicit JSON parsing error handling.When
json.loads(tool_call.function.arguments)fails, the code silently falls back to an empty dict. This could mask issues with malformed tool call arguments from the LLM. Consider logging the error or raising a more specific exception with context about which tool call failed to parse.Example:
try: arguments = json.loads(tool_call.function.arguments) -except json.JSONDecodeError: - arguments = {} +except json.JSONDecodeError as e: + raise ValueError( + f"Failed to parse arguments for tool '{tool_call.function.name}': {e}" + ) from e
349-349: Optional: Address static analysis hints about error messages.Static analysis (Ruff TRY003) suggests avoiding long messages directly in exception constructors. If following strict style guidelines, consider:
- Defining custom exception classes, or
- Moving messages to constants
This is a minor style concern and can be deferred.
Also applies to: 392-392
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/fastmcp/experimental/sampling/handlers/openai.py(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bareexcept- be specific with exception types
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns even if shorter
Follow existing patterns and maintain consistency in code organization and style
Files:
src/fastmcp/experimental/sampling/handlers/openai.py
🧬 Code graph analysis (1)
src/fastmcp/experimental/sampling/handlers/openai.py (1)
src/fastmcp/tools/tool.py (1)
Tool(116-258)
🪛 Ruff (0.14.7)
src/fastmcp/experimental/sampling/handlers/openai.py
274-274: Avoid specifying long messages outside the exception class
(TRY003)
349-349: Avoid specifying long messages outside the exception class
(TRY003)
392-392: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (4)
src/fastmcp/experimental/sampling/handlers/openai.py (4)
1-43: LGTM!The imports are well-organized and include all necessary types for tool-enabled sampling flows. The addition of
jsonfor argument parsing and the comprehensive OpenAI tool-related types are appropriate.
52-91: LGTM!The method signature correctly reflects the conditional return type based on tool usage. The tool and tool_choice conversion logic is properly guarded with presence checks before invoking the converters.
186-186: Potential type mismatch with empty string fallback.The code uses
content_parts if content_parts else "", which may not match the expected type forChatCompletionToolMessageParam. The OpenAI API expectscontentto be a string or a list of content parts. Verify that passing an empty string whencontent_partsis empty is the correct behavior, or consider using a more explicit empty list or a descriptive string like"(no content)".This same pattern appears on Line 251.
306-326: LGTM!The tool conversion logic is clean and handles the edge case of missing "type" fields in the input schema. The use of
tool.description or ""provides a sensible default.
| @staticmethod | ||
| def _convert_tool_choice_to_openai( | ||
| tool_choice: ToolChoice, | ||
| ) -> ChatCompletionToolChoiceOptionParam: | ||
| """Convert MCP tool_choice to OpenAI format.""" | ||
| if tool_choice.mode == "auto": | ||
| return "auto" | ||
| elif tool_choice.mode == "required": | ||
| return "required" | ||
| elif tool_choice.mode == "none": | ||
| return "none" | ||
| else: | ||
| # Unknown mode, default to auto | ||
| return "auto" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find ToolChoice definition
rg -nP 'class ToolChoice|ToolChoice\s*=' --type=py -A 10Repository: jlowin/fastmcp
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Check imports in the openai.py file
head -50 src/fastmcp/experimental/sampling/handlers/openai.py | grep -E 'import|from'Repository: jlowin/fastmcp
Length of output: 686
🏁 Script executed:
#!/bin/bash
# Read the complete imports section more carefully
head -100 src/fastmcp/experimental/sampling/handlers/openai.pyRepository: jlowin/fastmcp
Length of output: 3654
🏁 Script executed:
#!/bin/bash
# Search for ToolChoice usage and definition in the codebase
rg 'ToolChoice' --type=py -B 2 -A 5 | head -100Repository: jlowin/fastmcp
Length of output: 6944
🏁 Script executed:
#!/bin/bash
# Look at the full _convert_tool_choice_to_openai method implementation
sed -n '328,341p' src/fastmcp/experimental/sampling/handlers/openai.pyRepository: jlowin/fastmcp
Length of output: 541
🏁 Script executed:
#!/bin/bash
# Search for ToolChoice type definition - it's from mcp.types
# Let's check if we can find documentation or see how it's used
rg 'ToolChoice\(' --type=py -B 2 -A 2 | grep -A 3 'name='Repository: jlowin/fastmcp
Length of output: 676
🏁 Script executed:
#!/bin/bash
# Check how ChatCompletionToolChoiceOptionParam is used elsewhere in the code
rg 'ChatCompletionToolChoiceOptionParam' --type=py -B 3 -A 3Repository: jlowin/fastmcp
Length of output: 2039
🌐 Web query:
OpenAI API ChatCompletionToolChoiceOptionParam tool_choice specific tool selection
💡 Result:
Summary — what tool_choice accepts and how to force a specific tool
-
Allowed values
- "none" — model will not call any tools. [1]
- "auto" — model may choose to call zero, one, or multiple tools (default when tools are provided). [1][2]
- "required" — model must call one or more tools. [1][2]
- Object forcing a specific function/tool — e.g. {"type":"function","function":{"name":"my_function"}} — forces the model to call that named tool. The forced tool must appear in the request's tools list. [1][2][3]
-
Notes
- If tools is empty, tool_choice defaults to "none"; if tools is non-empty it defaults to "auto". [1][2]
- The model can call multiple tools in one response unless you disable parallel_tool_calls (set parallel_tool_calls: false) to restrict to at most one. [2]
- Client libraries / wrappers commonly validate that a forced function name exists in the tools list before sending. [3]
Sources: OpenAI Cookbook examples and docs on function/tool calling, community documentation and library behavior. [1][4][2][3]
References
- OpenAI Cookbook — How to call functions with chat models. [2]
- OpenAI Cookbook example: Using tool_required for customer service. [3]
- LangChain / client validation examples showing forced-function must be in tools list. [8]
- Community summaries/docs describing tool_choice behavior (tool_choice options and parallel_tool_calls). [4]
🏁 Script executed:
#!/bin/bash
# Search for ToolChoice type definition or related documentation
rg -i 'toolchoice|tool.*choice' --type=py --type=md | grep -i 'mode\|enum\|literal' | head -20Repository: jlowin/fastmcp
Length of output: 936
🏁 Script executed:
#!/bin/bash
# Look for ToolChoice usage with name in test files or elsewhere
rg 'ToolChoice\(' --type=py -A 1 | grep -B 1 'name'Repository: jlowin/fastmcp
Length of output: 418
🏁 Script executed:
#!/bin/bash
# Check the full method context including type hints and surrounding code
sed -n '320,350p' src/fastmcp/experimental/sampling/handlers/openai.pyRepository: jlowin/fastmcp
Length of output: 1095
🏁 Script executed:
#!/bin/bash
# Verify there are no other places handling ToolChoice that might provide context
rg '_convert_tool_choice' --type=py -B 5 -A 15Repository: jlowin/fastmcp
Length of output: 3825
Handle specific tool selection and raise errors for invalid modes.
The method ignores the name field of ToolChoice. When a specific tool must be forced (e.g., ToolChoice(mode="required", name="final_response")), the conversion should return the OpenAI object format {"type": "function", "function": {"name": "..."}} instead of just the string. Additionally, replace the silent fallback to "auto" on line 340 with a ValueError to catch configuration errors explicitly rather than masking them.
🤖 Prompt for AI Agents
In src/fastmcp/experimental/sampling/handlers/openai.py around lines 328 to 341,
the converter currently ignores ToolChoice.name and silently falls back to
"auto" for unknown modes; change it so that when tool_choice.mode == "required"
and a name is provided you return the OpenAI object form {"type": "function",
"function": {"name": "<name>"}} (validate that name is non-empty and raise
ValueError if missing), keep "auto" and "none" string returns for the explicit
"auto" and "none" modes, and replace the silent fallback with raising a
ValueError for any unrecognized mode so misconfiguration is surfaced.
Functions passed to ctx.sample(tools=[...]) are now auto-converted via SamplingTool.from_function(). Users can still use that method directly for custom name/description overrides.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/fastmcp/server/context.py (2)
821-827: Type contract violation whenmax_iterationsexhausted (previously flagged).Returning
result=cast(ResultT, None)when iterations are exhausted without a final response violates the type contract. Callers expectingResultT(e.g., a Pydantic model) will receiveNone, causing downstreamAttributeErrors.The previous review suggested raising an exception, which seems appropriate here:
# Check if we've hit max iterations if iteration >= max_iterations: - # Return what we have with None text (tool use without completion) - return SamplingResult( - text=None, - result=cast(ResultT, None), - history=current_messages, - ) + raise RuntimeError( + f"Sampling loop exhausted after {max_iterations} iterations " + "without receiving a final response from the LLM." + )
916-923: SameNoneresult issue in_sample_with_client(previously flagged).This location has the identical type contract violation as
_sample_with_fallback_handler. Apply the same fix for consistency.
🧹 Nitpick comments (5)
examples/advanced_sampling/tool_use.py (1)
78-81: Minor output formatting inconsistency.Line 81 formats
confidencewith a%suffix, butResearchReport.confidenceis defined as a plainfloat(line 48). If it's meant to be 0.0-1.0, consider multiplying by 100 or clarifying the expected range in the model's docstring.- print(f" Confidence: {result.data['confidence']}%") + print(f" Confidence: {result.data['confidence'] * 100:.1f}%")docs/servers/sampling.mdx (2)
425-447: Placeholdersome_conditionin custom loop example.The
some_conditionplaceholder is undefined, which will cause aNameErrorif users copy-paste directly. Consider using a realistic condition or adding a comment clarifying it's illustrative.# Custom logic: check conditions, change tools, etc. - if some_condition: + if "weather" in str(result.history): # Example condition tools = [different_tool]Or add a clarifying comment:
# Custom logic: check conditions, change tools, etc. # Replace `some_condition` with your actual termination logic if some_condition:
432-446: Missing termination condition inwhile Trueloop example.The custom loop example lacks a break condition other than
result.text, which could lead to an infinite loop if the LLM keeps calling tools. Consider adding a max iteration guard or documenting this caveat.async def custom_agent(question: str, ctx: Context) -> str: """Agent with custom loop logic.""" history = [] tools = [search, get_time] + max_loops = 20 # Prevent infinite loops - while True: + for _ in range(max_loops): result = await ctx.sample( messages=history or question, tools=tools, max_iterations=1, ) history = result.history # Custom logic: check conditions, change tools, etc. if some_condition: tools = [different_tool] # When we get a text response, we're done if result.text: return result.text + + return "Max iterations reached without final response"src/fastmcp/server/sampling/sampling_tool.py (1)
52-67: Consider handling unexpected argument errors inrun().If the underlying function receives arguments that don't match its signature (e.g., extra keys from LLM hallucination), this will raise a
TypeError. While the caller (_execute_tool_calls) catches exceptions, providing a more descriptive error here could aid debugging.async def run(self, arguments: dict[str, Any] | None = None) -> Any: """Execute the tool with the given arguments.""" if arguments is None: arguments = {} - result = self.fn(**arguments) + try: + result = self.fn(**arguments) + except TypeError as e: + raise TypeError( + f"Failed to call tool '{self.name}' with arguments {arguments}: {e}" + ) from e if inspect.isawaitable(result): result = await result return resultsrc/fastmcp/server/context.py (1)
1345-1360:_extract_text_from_contentuses duck typing.Using
hasattr(block, "text")works but is less type-safe than checkingisinstance(block, TextContent). Consider the explicit check for clarity and IDE support.def _extract_text_from_content( content: SamplingMessageContentBlock | list[SamplingMessageContentBlock], ) -> str | None: if isinstance(content, list): for block in content: - if hasattr(block, "text"): + if isinstance(block, TextContent): return block.text return None - elif hasattr(content, "text"): + elif isinstance(content, TextContent): return content.text return None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
tests/client/test_sampling.pyis excluded by none and included by nonetests/server/sampling/test_sampling_tool.pyis excluded by none and included by none
📒 Files selected for processing (5)
docs/servers/sampling.mdx(11 hunks)examples/advanced_sampling/tool_use.py(1 hunks)src/fastmcp/server/context.py(12 hunks)src/fastmcp/server/sampling/__init__.py(1 hunks)src/fastmcp/server/sampling/sampling_tool.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/fastmcp/server/sampling/init.py
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bareexcept- be specific with exception types
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns even if shorter
Follow existing patterns and maintain consistency in code organization and style
Files:
src/fastmcp/server/sampling/sampling_tool.pysrc/fastmcp/server/context.py
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/.cursor/rules/mintlify.mdc)
docs/**/*.mdx: Use clear, direct language appropriate for technical audiences
Write in second person ('you') for instructions and procedures in MDX documentation
Use active voice over passive voice in MDX technical documentation
Employ present tense for current states and future tense for outcomes in MDX documentation
Maintain consistent terminology throughout all MDX documentation
Keep sentences concise while providing necessary context in MDX documentation
Use parallel structure in lists, headings, and procedures in MDX documentation
Lead with the most important information using inverted pyramid structure in MDX documentation
Use progressive disclosure in MDX documentation: present basic concepts before advanced ones
Break complex procedures into numbered steps in MDX documentation
Include prerequisites and context before instructions in MDX documentation
Provide expected outcomes for each major step in MDX documentation
End sections with next steps or related information in MDX documentation
Use descriptive, keyword-rich headings for navigation and SEO in MDX documentation
Focus on user goals and outcomes rather than system features in MDX documentation
Anticipate common questions and address them proactively in MDX documentation
Include troubleshooting for likely failure points in MDX documentation
Provide multiple pathways (beginner vs advanced) but offer an opinionated path to avoid overwhelming users in MDX documentation
Always include complete, runnable code examples that users can copy and execute in MDX documentation
Show proper error handling and edge case management in MDX code examples
Use realistic data instead of placeholder values in MDX code examples
Include expected outputs and results for verification in MDX code examples
Test all code examples thoroughly before publishing in MDX documentation
Specify language and include filename when relevant in MDX code examples
Add explanatory comments for complex logic in MDX code examples
Document all API...
Files:
docs/servers/sampling.mdx
🧬 Code graph analysis (3)
src/fastmcp/server/sampling/sampling_tool.py (1)
src/fastmcp/tools/tool.py (2)
Tool(116-258)ParsedFunction(394-535)
src/fastmcp/server/context.py (1)
src/fastmcp/server/sampling/sampling_tool.py (5)
SamplingTool(18-141)from_mcp_tool(82-105)from_function(108-141)_to_sdk_tool(69-79)run(52-67)
examples/advanced_sampling/tool_use.py (3)
src/fastmcp/server/context.py (2)
fastmcp(190-195)Context(138-1248)src/fastmcp/client/client.py (2)
Client(107-1023)call_tool(939-1015)src/fastmcp/experimental/sampling/handlers/openai.py (1)
OpenAISamplingHandler(46-399)
🪛 LanguageTool
docs/servers/sampling.mdx
[style] ~417-~417: Try using a synonym here to strengthen your writing.
Context: ...ling iteration and returns immediately, giving you access to result.history so you can c...
(GIVE_PROVIDE)
🪛 Ruff (0.14.7)
src/fastmcp/server/sampling/sampling_tool.py
95-98: Avoid specifying long messages outside the exception class
(TRY003)
134-134: Avoid specifying long messages outside the exception class
(TRY003)
src/fastmcp/server/context.py
632-634: Avoid specifying long messages outside the exception class
(TRY003)
688-691: Avoid specifying long messages outside the exception class
(TRY003)
787-790: Avoid specifying long messages outside the exception class
(TRY003)
799-802: Avoid specifying long messages outside the exception class
(TRY003)
854-856: Avoid specifying long messages outside the exception class
(TRY003)
1005-1005: Do not catch blind exception: Exception
(BLE001)
1048-1048: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (8)
examples/advanced_sampling/tool_use.py (1)
1-85: Well-structured example demonstrating tool-enabled sampling.The example clearly shows:
- Defining sampling tools (
search_web,get_word_count)- Using
result_typefor structured output (ResearchReport)- Executing the agentic workflow via
ctx.sample()with toolsThe code follows the coding guidelines with proper type annotations and is runnable end-to-end.
docs/servers/sampling.mdx (1)
1-586: Comprehensive documentation for the new sampling features.The documentation effectively covers:
- Basic usage with clear examples
- Structured output with
result_type- Tool-enabled sampling with automatic loop handling
- Custom loop building for advanced use cases
- Fallback handler integration
Well-structured with progressive disclosure, proper component usage, and actionable code examples.
src/fastmcp/server/sampling/sampling_tool.py (1)
1-141: Clean implementation of SamplingTool.The class design is solid:
- Pydantic BaseModel with
arbitrary_types_allowedfor thefnfield- Proper async/sync handling in
run()- Multiple construction paths (
from_function,from_mcp_tool)- Lambda name validation prevents ambiguous tool names
src/fastmcp/server/context.py (5)
750-763:effective_tool_choicelogic handles last-iteration finalization well.The code correctly forces
final_responsewhenresult_typeis specified, or forcesmode="none"to get a text response on the last iteration. This prevents infinite loops and ensures a usable result.
1005-1020: BareExceptioncatch is acceptable here for LLM feedback loop.Catching
Exceptionwhen validatingfinal_responseinput allows the error to be sent back to the LLM for retry, which is the intended behavior for agentic workflows. The static analysis hint (BLE001) can be safely ignored in this context.
1038-1056: Tool execution error handling is robust.Catching exceptions during tool execution and returning them as
ToolResultContentwithisError=Trueallows the LLM to handle failures gracefully. This is appropriate for an agentic loop.
636-644: System prompt hint injection for structured output is clear.When
result_typeis specified, appending "Call final_response when you have completed the task." to the system prompt effectively guides the LLM. Consider making this hint configurable in future iterations if users need different phrasing.
79-98: Well-designedSamplingResultdataclass.The generic
SamplingResult[ResultT]cleanly separates:
.textfor string representation.resultfor typed result.historyfor conversation continuationThe docstring clearly explains each field's purpose.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/fastmcp/server/context.py (1)
836-842: Address the past review concern: Type-unsafeNonereturn.As flagged in the previous review, returning
cast(ResultT, None)whenmax_iterationsis exhausted violates the type contract whenresult_typeis a non-optional structured type. This creates a runtime hazard where callers expect a valid object but receiveNone.The previous review suggested raising an exception (e.g.,
RuntimeErroror a dedicatedMaxIterationsExceededError) instead of returningNone. This would make the failure explicit and type-safe.Apply a similar fix to both occurrences (here and at lines 931-938):
# Check if we've hit max iterations if iteration >= max_iterations: - # Return what we have with None text (tool use without completion) - return SamplingResult( - text=None, - result=cast(ResultT, None), - history=current_messages, - ) + raise RuntimeError( + f"Sampling exhausted {max_iterations} iterations without a final response. " + "Consider increasing max_iterations or simplifying the task." + )
🧹 Nitpick comments (1)
src/fastmcp/server/context.py (1)
647-649: Optional: Consider extracting error message to a constant.The error message is clear and functional. For strict adherence to Ruff's TRY003 guideline, you could extract it to a module-level constant, but this is a low-priority style improvement.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pyproject.tomlis excluded by none and included by noneuv.lockis excluded by!**/*.lockand included by none
📒 Files selected for processing (7)
src/fastmcp/client/client.py(3 hunks)src/fastmcp/client/transports.py(1 hunks)src/fastmcp/prompts/prompt.py(2 hunks)src/fastmcp/resources/resource.py(2 hunks)src/fastmcp/resources/template.py(3 hunks)src/fastmcp/server/context.py(12 hunks)src/fastmcp/server/server.py(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/fastmcp/client/client.py
- src/fastmcp/resources/template.py
- src/fastmcp/client/transports.py
- src/fastmcp/server/server.py
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bareexcept- be specific with exception types
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns even if shorter
Follow existing patterns and maintain consistency in code organization and style
Files:
src/fastmcp/prompts/prompt.pysrc/fastmcp/server/context.pysrc/fastmcp/resources/resource.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: jlowin
Repo: jlowin/fastmcp PR: 0
File: :0-0
Timestamp: 2025-12-01T15:48:05.095Z
Learning: PR #2505 in fastmcp adds NEW functionality to get_access_token(): it now first checks request.scope["user"] for the token (which never existed before), then falls back to _sdk_get_access_token() (the only thing the original code did). This is not a reversal of order but entirely new functionality to fix stale token issues.
📚 Learning: 2025-11-26T21:51:44.174Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2025-11-26T21:51:44.174Z
Learning: Review and update related Manager classes (ToolManager, ResourceManager, PromptManager) when modifying MCP object definitions
Applied to files:
src/fastmcp/prompts/prompt.py
🧬 Code graph analysis (1)
src/fastmcp/server/context.py (2)
src/fastmcp/tools/tool.py (6)
Tool(122-282)from_function(206-236)from_function(289-351)from_function(420-548)run(238-248)run(353-393)src/fastmcp/server/sampling/sampling_tool.py (5)
SamplingTool(18-141)from_mcp_tool(82-105)from_function(108-141)_to_sdk_tool(69-79)run(52-67)
🪛 Ruff (0.14.7)
src/fastmcp/server/context.py
647-649: Avoid specifying long messages outside the exception class
(TRY003)
703-706: Avoid specifying long messages outside the exception class
(TRY003)
802-805: Avoid specifying long messages outside the exception class
(TRY003)
814-817: Avoid specifying long messages outside the exception class
(TRY003)
869-871: Avoid specifying long messages outside the exception class
(TRY003)
1020-1020: Do not catch blind exception: Exception
(BLE001)
1063-1063: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (9)
src/fastmcp/server/context.py (7)
71-99: LGTM: Well-designed type definitions and result container.The new
ResultTTypeVar withdefault=strmaintains backward compatibility, and theSamplingResultgeneric dataclass provides a clean interface for both text and structured responses with full history tracking.
527-617: LGTM: Well-structured overloads and comprehensive documentation.The overloads correctly distinguish between typed (
result_typespecified) and text-only responses, with clear parameter documentation covering the new tool-enabled sampling workflow.
1020-1035: Verify: Broad exception handling in tool execution is intentional.The code catches
Exceptionbroadly when executing tools (lines 1020, 1063). While Ruff flags this as BLE001, catching broad exceptions in tool execution contexts is often appropriate to:
- Prevent individual tool failures from crashing the entire sampling loop
- Return actionable error messages to the LLM for retry/recovery
Confirm this defensive pattern is intentional for robustness. If specific exception types are expected (e.g.,
ValidationError,ToolExecutionError), consider catching those explicitly first, with a final broad catch as a safety net.Also applies to: 1063-1071
1184-1201: LGTM: Proper handling of enum response types.The new logic correctly handles dict-based single-select and list-based multi-select enum responses with direct value extraction from
result.content, maintaining type safety with appropriate casts.
1335-1375: LGTM: Well-designed helper functions.Both
_create_final_response_tooland_extract_text_from_contentare cleanly implemented:
- The synthetic final_response tool properly uses the type adapter and compressed schema
- Text extraction defensively handles both single blocks and lists with duck typing
- Validation is correctly deferred to the caller for final_response
964-1082: LGTM: Robust tool execution flow.The
_execute_tool_callsmethod correctly:
- Handles both synthetic
final_responseand regular tools- Batches tool results into a single user message per MCP conventions
- Returns early with a structured result when
final_responsesucceeds- Converts tool errors into
ToolResultContentwithisError=Truefor LLM recovery- Continues the loop on validation failures, allowing the LLM to retry
765-779: Good: Last-iteration forcing ensures termination.The code correctly forces completion on the final iteration:
- Forces
final_responsetool whenresult_typeis specified- Forces text-only response (
tool_choice=none) when only tools are usedThis makes the max_iterations limit meaningful. However, this relies on fixing the type-unsafe
Nonereturn at lines 836-842 and 931-938 (flagged separately) for scenarios where forcing still doesn't produce a valid response.Also applies to: 894-908
src/fastmcp/prompts/prompt.py (1)
12-13: LGTM: Clean SDK type alias adoption.The rename from
MCP*toSDK*type aliases is straightforward and aligns with the repository-wide naming convention update. The changes maintain functional equivalence while improving naming consistency.Also applies to: 97-108
src/fastmcp/resources/resource.py (1)
11-11: LGTM: Clean SDK type alias adoption.The rename from
MCPResourcetoSDKResourceis straightforward and aligns with the repository-wide naming convention update. The docstring is correctly updated to reflect the new return type.Also applies to: 135-149
Sampling with tools (SEP-1577) allows servers to pass tools to
ctx.sample()for agentic workflows where the LLM can call helper functions during generation.Basic Sampling
Request text generation from the client's LLM and return the response. This is essentially unchanged in SEP-1577, and still supported as before, though the new
.resultattribute may be preferred to the previous.text(though.textis still available, and will always have a string representation of the result).Structured Output
Pass
result_typeto get a validated Pydantic model, dataclass, or basic type) instead of raw text:Sampling with Tools
Pass
toolsto let the LLM call functions during generation. Plain functions are auto-converted to tools. The server automatically executes tool calls and feeds results back to the LLM in a loop of up tomax_iterationslength. Atool_choiceparameter is supported to permit automatic, constrained, or no tool selection.Agentic Workflow with Structured Output
Combine tools and structured output for agentic workflows that produce validated results:
Advanced Loop Control
By default, when max_iterations is reached, the final iteration forces the LLM to produce a result (text or structured output). Setting max_iterations=1 disables this behavior and returns the raw response, letting you build custom control loops. This is considered very advanced (and is a more low-level wrapper over the SDK, though it still performs automatic tool processing and execution on users' behalf).
SDK Dependency
This PR requires modelcontextprotocol/python-sdk#1722 to be merged and released. Tests will fail until then.