Skip to content

Conversation

@rholinshead
Copy link
Member

@rholinshead rholinshead commented Oct 28, 2025

Summary

With the latest mcp sdk update, we can now set meta for tools via the decorator (still pending the same for resource decorator but this example doesn't need any meta for it); this allows us to clean up much of the code in the chatgpt app example which was just for overriding the tool/resource implementation.

Testing

  • Tested locally with uv run to see correct resource and tool
  • Tested against latest prod
Screenshot 2025-10-30 at 4 22 07 PM

Summary by CodeRabbit

  • Refactor

    • Modernized tool and resource integration to use decorator-based API definitions, simplifying the underlying infrastructure while maintaining existing functionality.
  • Chores

    • Updated widget resource versioning with refreshed metadata.

@coderabbitai
Copy link

coderabbitai bot commented Oct 28, 2025

Walkthrough

The PR migrates two instances of a ChatGPT example application from a manual MCP server handler pattern to modern OpenAI Apps SDK decorator-based annotations. Internal functions (_list_tools, _list_resources, _handle_read_resource, _call_tool_request) are replaced with @app.tool and @mcp.resource decorated functions, and the widget template URI is updated with a new timestamp.

Changes

Cohort / File(s) Summary
MCP Server Handler Refactoring
examples/cloud/chatgpt_app/main.py, src/mcp_agent/data/examples/cloud/chatgpt_app/main.py
Replaced manual MCP server routing functions with decorator-based tool/resource definitions: added @app.tool decorated flip_coin() returning structured output dict, added @mcp.resource decorated get_widget_html() serving widget HTML, removed _list_tools(), _list_resources(), _list_resource_templates(), _handle_read_resource(), _call_tool_request(), simplified _tool_meta() to consolidate widget metadata under "openai.com/widget" key, updated widget template_uri timestamp from 10-22-2025-15-48 to 10-27-2025-16-34, adjusted imports for FastMCP and Starlette integration.

Sequence Diagram

sequenceDiagram
    participant Client
    participant App as OpenAI App<br/>(Decorated)
    participant Tool as flip_coin()<br/>@app.tool
    participant Resource as get_widget_html()<br/>@mcp.resource
    
    rect rgb(200, 220, 240)
    Note over Client,App: New Decorator-Based Approach
    end
    
    Client->>App: Request tool list
    App->>App: Scan @app.tool decorators
    App-->>Client: Tool metadata (flip_coin)
    
    Client->>App: Execute flip_coin tool
    App->>Tool: Invoke decorated function
    Tool->>Tool: Generate random result
    Tool-->>App: Return {flipResult: ...}
    App-->>Client: Structured output
    
    Client->>App: Request widget resource
    App->>Resource: Invoke @mcp.resource
    Resource->>Resource: Serve WIDGET.html
    Resource-->>App: HTML string
    App-->>Client: Widget content
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify decorator annotations (@app.tool, @mcp.resource) are syntactically correct and properly configured with metadata (name, description, structured_output, etc.)
  • Confirm flip_coin() structured output format matches OpenAI Apps SDK expectations
  • Validate _tool_meta() returns widget descriptor under correct JSON keys ("openai.com/widget", "openai/outputTemplate")
  • Check whether both modified files are production examples or if one is test/fixture data; ensure consistency is intentional
  • Review widget template_uri timestamp change to confirm it reflects actual build artifact

Possibly related PRs

Suggested reviewers

  • saqadri
  • andrew-lastmile

Poem

🐰 A rabbit hops with decorator cheer,
No more manual routes to fear!
@app.tool and @mcp.resource shine,
Simple, clean, and oh-so-divine,
Flip a coin with elegance true,
The new way gleams in morning dew! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Update chatgpt app cloud example with latest mcp sdk updates" is directly related to the primary changes in the pull request. The changeset focuses on modernizing the example code by migrating from manual MCP server handler functions to a decorator-based API approach (@app.tool and @mcp.resource decorators), which represents the adoption of latest SDK capabilities. The title accurately conveys that this is an SDK-driven update to the example, though it could be more specific about the decorator-based migration. The title is clear, avoids vague terms like "misc updates" or "stuff," and provides sufficient information for a developer scanning the commit history to understand this is about updating the example with new SDK features.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/chatgpt-app-update

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rholinshead rholinshead marked this pull request as ready for review October 30, 2025 20:22
Copy link
Collaborator

@saqadri saqadri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it! love the deleted lines

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
examples/cloud/chatgpt_app/main.py (1)

135-138: Mirror the structured output schema fix

This copy shares the same Dict[str, str] annotation while structured_output=True. Please apply the TypedDict (or model) adjustment suggested in the src/... version so the schema names flipResult.

🧹 Nitpick comments (1)
src/mcp_agent/data/examples/cloud/chatgpt_app/main.py (1)

135-138: Clarify structured output schema

structured_output=True derives the JSON schema from the return annotation, and Dict[str, str] describes an arbitrary string-keyed map. The UI won’t know that flipResult is the sole field, so callers lose validation and documentation. Introduce a dedicated TypedDict (or BaseModel) so the schema explicitly surfaces flipResult.

+class FlipCoinResult(TypedDict):
+    flipResult: str
+
-async def flip_coin() -> Dict[str, str]:
+async def flip_coin() -> FlipCoinResult:

Add from typing import TypedDict to the imports near the top.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1aa967b and 2885141.

📒 Files selected for processing (2)
  • examples/cloud/chatgpt_app/main.py (3 hunks)
  • src/mcp_agent/data/examples/cloud/chatgpt_app/main.py (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-22T18:59:49.368Z
Learnt from: CR
PR: lastmile-ai/mcp-agent#0
File: examples/usecases/reliable_conversation/CLAUDE.md:0-0
Timestamp: 2025-07-22T18:59:49.368Z
Learning: Applies to examples/usecases/reliable_conversation/examples/reliable_conversation/src/**/*.py : Use mcp-agent's Agent abstraction for ALL LLM interactions, including quality evaluation, to ensure consistent tool access, logging, and error handling.

Applied to files:

  • src/mcp_agent/data/examples/cloud/chatgpt_app/main.py
🧬 Code graph analysis (1)
src/mcp_agent/data/examples/cloud/chatgpt_app/main.py (2)
src/mcp_agent/core/context.py (2)
  • mcp (154-155)
  • fastmcp (158-173)
src/mcp_agent/server/app_server.py (1)
  • app (280-282)

@rholinshead rholinshead merged commit cb09074 into main Oct 30, 2025
8 checks passed
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.

3 participants