Add mcp-agent cloud compatibility to basic slack agent example #498
Conversation
Main changes:
@app.tool
async def analyze_stock(company_name: str = "Apple") -> str:
"""Analyze a stock and generate a comprehensive financial report."""
- Uses @app.tool decorator to expose as MCP tool
- Function-based with parameters
- Returns string results
# os.makedirs(OUTPUT_DIR, exist_ok=True) # COMMENTED OUT
# context = app.context # Not needed when filesystem is commented out
- File operations commented out
- No filesystem server configuration
- Returns text output directly
server_names=["fetch"] # All agents use fetch only
- Only uses fetch server
- No Google Search dependency
- No filesystem server => commented out
Output Handling:
return f"✅ Stock Analysis Complete for {company_name}!\n\n" + "="*60 + "\n" + result
- Returns formatted string with results
- No file verification
Error Handling
return f"💥 Error during workflow execution: {str(e)}"
- Returns error as string
WalkthroughReplaces the previous stock analyzer with an orchestrated workflow using an Orchestrator and AugmentedLLM. Adds an async tool analyze_stock that orchestrates research, quality evaluation, analysis, and report writing, returning a string. Elevates quality threshold to EXCELLENT, updates MCP app name, and introduces a simple CLI entry that prints the final report. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI (main)
participant App as MCPApp (unified_stock_analyzer)
participant Tool as tool analyze_stock(company_name)
participant Orc as Orchestrator
participant LLM as OpenAIAugmentedLLM
participant RA as research_agent
participant QC as research_quality_controller
participant EV as research_evaluator
participant FA as analyst_agent
participant RW as report_writer
User->>CLI: Provide company name
CLI->>App: Initialize & invoke tool
App->>Tool: analyze_stock(company_name)
Tool->>Orc: orchestrate(company_name)
Orc->>RA: Gather data
RA-->>Orc: Research results
Orc->>QC: Evaluate quality (min EXCELLENT)
QC-->>Orc: Pass/Fail rating
alt Quality sufficient
Orc->>EV: Assess and summarize quality
EV-->>Orc: Quality assessment
Orc->>FA: Perform financial analysis
FA-->>Orc: Analysis insights
Orc->>RW: Format institutional report
RW-->>Orc: Final report (string)
Orc-->>Tool: Return report
Tool-->>App: Report string
App-->>CLI: Report string
CLI-->>User: Print report
else Quality insufficient
note over Orc,RA: Retry/augment research as configured
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| output_file = f"{COMPANY_NAME.lower().replace(' ', '_')}_report_{timestamp}.md" | ||
| output_path = os.path.join(OUTPUT_DIR, output_file) | ||
| # output_file = f"{company_name.lower().replace(' ', '_')}_report_{timestamp}.md" | ||
| output_path = os.path.join(OUTPUT_DIR, f"{company_name.lower().replace(' ', '_')}_report_{timestamp}.md") |
There was a problem hiding this comment.
Variable OUTPUT_DIR is used but never defined in the new code structure. The original OUTPUT_DIR definition was removed when converting from main() to @app.tool function, but line 38 still references it. This will cause a NameError at runtime. Either define OUTPUT_DIR as a constant or remove this line entirely since file operations are commented out.
| output_path = os.path.join(OUTPUT_DIR, f"{company_name.lower().replace(' ', '_')}_report_{timestamp}.md") | |
| # output_path = os.path.join(OUTPUT_DIR, f"{company_name.lower().replace(' ', '_')}_report_{timestamp}.md") |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| logger = analyzer_app.logger | ||
| # Access the running app context directly | ||
| # context = app.context # Not needed when filesystem is commented out | ||
| logger = app.logger |
There was a problem hiding this comment.
Accessing app.logger outside of the app context will fail. The analyze_stock function is decorated with @app.tool but tries to access app.logger directly without being inside an async with app.run() context. This will cause an AttributeError or return None/undefined logger. The logger should be accessed through the app context when the tool is actually executed.
| logger = app.logger | |
| # Logger will be accessed within functions when needed |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| - Include source URLs for verification | ||
| - Note data timestamps/dates | ||
| - If any section is missing data, explicitly state what couldn't be found. """, | ||
| server_names=["fetch"], |
There was a problem hiding this comment.
Server configuration mismatch: research_agent uses server_names=["fetch"] but its instructions contain multiple references to Google Search functionality (lines 63, 68, 73, 78 with search queries like 'Search: "{company_name} stock price today current"'). The agent expects to perform Google searches but only has access to the fetch server, which will cause tool execution failures when it tries to search.
| server_names=["fetch"], | |
| server_names=["fetch", "search"], |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
![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
🧹 Nitpick comments (8)
examples/usecases/mcp_financial_analyzer/main.py (8)
22-26: Remove dead file-output code and unused constants
OUTPUT_DIR,COMPANY_NAME,MAX_ITERATIONS,timestamp, andoutput_pathare unused after switching to pure string output.Apply:
@@ -# Configuration values -OUTPUT_DIR = "company_reports" -COMPANY_NAME = "Apple" if len(sys.argv) <= 1 else sys.argv[1] -MAX_ITERATIONS = 3 +# Configuration values (filesystem disabled; keep config minimal) +MODEL_NAME = os.getenv("MCP_OPENAI_MODEL", "gpt-4o") @@ - timestamp = datetime.now().strftime("%Y%m%d_%H%M%S") - # output_file = f"{company_name.lower().replace(' ', '_')}_report_{timestamp}.md" - output_path = os.path.join(OUTPUT_DIR, f"{company_name.lower().replace(' ', '_')}_report_{timestamp}.md") + # (no file output variables needed)Also applies to: 36-39
44-50: Delete commented-out filesystem server codeThese comments are now noise and can mislead future readers.
Apply:
- # Configure filesystem server to use current directory (COMMENTED OUT - NOW PRINTING RESULTS) - # if "filesystem" in context.config.mcp.servers: - # context.config.mcp.servers["filesystem"].args.extend([os.getcwd()]) - # logger.info("Filesystem server configured") - # else: - # logger.warning("Filesystem server not configured - report saving may fail")
31-34: Sanitize user-provided company name to reduce prompt-injection surface
company_nameis interpolated into multiple prompts as instructions. Sanitize/limit length to prevent adversarial input shaping agent behavior.Apply:
@@ async def analyze_stock(company_name: str = "Apple") -> str: """Analyze a stock and generate a comprehensive financial report.""" + # Sanitize input to reduce prompt-injection and template breakage + import re + company_name = (re.sub(r'[^A-Za-z0-9 .,&\-()/#:+%]', '', company_name) or "Apple")[:120].strip()
248-257: Hard-coded “EST” can be wrong; use timezone-aware timestampUse
zoneinfoto render the local New York market time correctly (EST/EDT).Apply:
@@ -from datetime import datetime +from datetime import datetime +from zoneinfo import ZoneInfo @@ - **Report Date:** {datetime.now().strftime('%B %d, %Y at %I:%M %p EST')} + **Report Date:** {datetime.now(ZoneInfo('America/New_York')).strftime('%B %d, %Y at %I:%M %p %Z')}
185-191: Make quality threshold and refinement budget configurable
min_rating=EXCELLENTwith defaultmax_refinements=3can be costly/slow. Parameterize via env to tune for cloud costs/latency.Apply:
@@ - research_quality_controller = EvaluatorOptimizerLLM( + research_quality_controller = EvaluatorOptimizerLLM( optimizer=research_agent, evaluator=research_evaluator, llm_factory=OpenAIAugmentedLLM, - min_rating=QualityRating.EXCELLENT, + min_rating=QualityRating[os.getenv("MCP_QUALITY_MIN_RATING", "EXCELLENT")], + max_refinements=int(os.getenv("MCP_RESEARCH_MAX_REFINEMENTS", "3")), )
409-411: Parameterize the model via env varAvoid hard-coding the model name; makes cloud deployment configurable.
Apply:
- result = await orchestrator.generate_str( - message=task, request_params=RequestParams(model="gpt-4o") - ) + result = await orchestrator.generate_str( + message=task, request_params=RequestParams(model=os.getenv("MCP_OPENAI_MODEL", "gpt-4o")) + )
424-426: Don’t leak exception details to end usersReturn a generic error and log the stack trace server-side.
Apply:
- except Exception as e: - logger.error(f"Error during workflow execution: {str(e)}") - return f"💥 Error during workflow execution: {str(e)}" + except Exception: + logger.exception("Error during workflow execution") + return "💥 Error during workflow execution. Please try again later."
357-363: Cap report length to control token usage/cost1200–1800 words may exceed practical budgets. Consider 800–1200 words or make this configurable via env.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/usecases/mcp_financial_analyzer/main.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/usecases/mcp_financial_analyzer/main.py (3)
src/mcp_agent/app.py (3)
tool(717-769)logger(193-210)run(397-420)src/mcp_agent/workflows/evaluator_optimizer/evaluator_optimizer.py (2)
EvaluatorOptimizerLLM(49-478)QualityRating(25-31)src/mcp_agent/workflows/orchestrator/orchestrator.py (1)
Orchestrator(116-716)
🔇 Additional comments (1)
examples/usecases/mcp_financial_analyzer/main.py (1)
372-385: Orchestrator wiring looks solidPassing
EvaluatorOptimizerLLMdirectly inavailable_agentsand usingplan_type="full"is consistent with the Orchestrator API.
| @app.tool | ||
| async def analyze_stock(company_name: str = "Apple") -> str: | ||
| """Analyze a stock and generate a comprehensive financial report.""" | ||
| # Create output directory and set up file paths (COMMENTED OUT - NOW PRINTING RESULTS) |
There was a problem hiding this comment.
🧩 Analysis chain
MCP tool exposure + string output is the right move for MCP Cloud
Registering analyze_stock as an MCP tool that returns a string (no filesystem writes) aligns with cloud constraints.
Please confirm the @app.tool decorator supports async functions in your version. To verify:
🏁 Script executed:
#!/bin/bash
# Check decorator docs/impl to ensure async functions are supported
rg -n -C3 "def tool\(" src/mcp_agent/app.py
rg -n -C3 "_create_workflow_from_function" src/mcp_agent/app.pyLength of output: 885
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/mcp_agent/app.py"
echo "=== Showing lines 500-840 of $file ==="
nl -ba "$file" | sed -n '500,840p' || true
echo
echo "=== Search for async/await/inspect helpers in src ==="
rg -n --hidden --no-ignore -S "async def|inspect.iscoroutinefunction|inspect.isawaitable|isawaitable" src || true
echo
echo "=== Show surrounding lines where `def tool(` and `_create_workflow_from_function` were matched ==="
rg -n -C8 --hidden --no-ignore "def tool\(|def _create_workflow_from_function\(" src/mcp_agent/app.py || trueLength of output: 57554
@app.tool is synchronous — make the tool function sync or register it as an async-capable workflow
src/mcp_agent/app.py defines the tool decorator as a synchronous tool (see def tool at src/mcp_agent/app.py ~717); examples/usecases/mcp_financial_analyzer/main.py currently registers async def analyze_stock — convert it to a regular def analyze_stock(...) -> str (remove async) or register it via a workflow/adapter that supports async.
🤖 Prompt for AI Agents
In examples/usecases/mcp_financial_analyzer/main.py around lines 31 to 34, the
function analyze_stock is declared async but app.tool is a synchronous tool
decorator; change analyze_stock to a synchronous function by removing the async
keyword (def analyze_stock(company_name: str = "Apple") -> str), ensure any
awaited calls inside are converted to synchronous equivalents or run via
asyncio.run/loop if necessary, and return a plain str; alternatively, if you
need true async behavior, register the function through the async-capable
workflow/adapter API instead of using @app.tool so the decorator and function
capabilities match.
| # Research agent: Collects data using Google Search | ||
| research_agent = Agent( | ||
| name="search_finder", | ||
| instruction=f"""You are a comprehensive financial data collector for {company_name}. | ||
| Your job is to gather ALL required financial information using Google Search and fetch tools. | ||
| **REQUIRED DATA TO COLLECT:** |
There was a problem hiding this comment.
🧩 Analysis chain
Prompt still requires “Google Search” while only ‘fetch’ is allowed
With server_names=["fetch"] and the PR goal to drop Google Search, instructing the agent to use “Google Search” will lead to failed tool calls or brittle scraping of search pages.
Apply minimally:
- Your job is to gather ALL required financial information using Google Search and fetch tools.
+ Your job is to gather ALL required financial information using the HTTP fetch tool only (no search engine APIs).
+ Prefer reputable sources such as SEC EDGAR, the company's Investor Relations site, Yahoo Finance, Nasdaq, Reuters, and WSJ. Include direct source URLs and timestamps.
@@
- Search: "{company_name} stock price today current"
- Search: "{company_name} trading volume market data"
+ Find and fetch: a real-time/last close quote page (e.g., Yahoo Finance) showing price, % change, volume, 52-week range
@@
- Search: "{company_name} latest quarterly earnings results"
- Search: "{company_name} earnings vs estimates beat miss"
+ Find and fetch: latest quarterly earnings release and consensus summary (company IR/press release; Yahoo Finance earnings page)
@@
- Search: "{company_name} financial news latest week"
- Search: "{company_name} analyst ratings upgrade downgrade"
+ Find and fetch: 3–5 recent news items from credible outlets with dates and sources; include any recent analyst actions
@@
- Search: "{company_name} PE ratio market cap financial metrics"
+ Find and fetch: key valuation metrics from a reliable quote/overview pageEnsure the MCP Cloud environment has a configured ‘fetch’ server. Quick check:
Also applies to: 63-80
🏁 Script executed:
#!/bin/bash
# Look for MCP server configuration mentioning 'fetch'
rg -n -C2 '"servers"\s*:\s*\{' -g '!**/node_modules/**' -S
rg -n -C2 '"fetch"' -SLength of output: 52426
Replace "Google Search" with fetch-only instructions in the financial research agent prompt
examples/usecases/mcp_financial_analyzer/main.py — lines 53-60 (also applies to 63-80). Verified the repo already configures a 'fetch' server (e.g., src/mcp_agent/data/templates/basic_agent.py, src/mcp_agent/cli/commands/config.py, tests reference "fetch"); update the prompt to use the HTTP fetch tool only.
- Your job is to gather ALL required financial information using Google Search and fetch tools.
+ Your job is to gather ALL required financial information using the HTTP fetch tool only (no search engine APIs).
+ Prefer reputable sources such as SEC EDGAR, the company's Investor Relations site, Yahoo Finance, Nasdaq, Reuters, and WSJ. Include direct source URLs and timestamps.
@@
- Search: "{company_name} stock price today current"
- Search: "{company_name} trading volume market data"
+ Find and fetch: a real-time/last close quote page (e.g., Yahoo Finance) showing price, % change, volume, 52-week range
@@
- Search: "{company_name} latest quarterly earnings results"
- Search: "{company_name} earnings vs estimates beat miss"
+ Find and fetch: latest quarterly earnings release and consensus summary (company IR/press release; Yahoo Finance earnings page)
@@
- Search: "{company_name} financial news latest week"
- Search: "{company_name} analyst ratings upgrade downgrade"
+ Find and fetch: 3–5 recent news items from credible outlets with dates and sources; include any recent analyst actions
@@
- Search: "{company_name} PE ratio market cap financial metrics"
+ Find and fetch: key valuation metrics from a reliable quote/overview page📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Research agent: Collects data using Google Search | |
| research_agent = Agent( | |
| name="search_finder", | |
| instruction=f"""You are a comprehensive financial data collector for {company_name}. | |
| Your job is to gather ALL required financial information using Google Search and fetch tools. | |
| **REQUIRED DATA TO COLLECT:** | |
| # Research agent: Collects data using Google Search | |
| research_agent = Agent( | |
| name="search_finder", | |
| instruction=f"""You are a comprehensive financial data collector for {company_name}. | |
| Your job is to gather ALL required financial information using the HTTP fetch tool only (no search engine APIs). | |
| Prefer reputable sources such as SEC EDGAR, the company's Investor Relations site, Yahoo Finance, Nasdaq, Reuters, and WSJ. Include direct source URLs and timestamps. | |
| **REQUIRED DATA TO COLLECT:** |
🤖 Prompt for AI Agents
In examples/usecases/mcp_financial_analyzer/main.py around lines 53-60 (and
similarly lines 63-80), the research agent prompt still instructs "Google
Search" — change the prompt to instruct the agent to use only the configured
HTTP fetch tool (fetch server) instead of Google Search; reword the instruction
to explicitly say use the fetch tool/HTTP endpoints to retrieve pages, follow
links, and collect the required financial data, and remove any references to
Google Search so the prompt aligns with the repo's fetch-only configuration.
1 participant
Main changes:
Added @app.tool
async def analyze_stock(company_name: str = "Apple") -> str:
"""Analyze a stock and generate a comprehensive financial report."""
os.makedirs(OUTPUT_DIR, exist_ok=True) # COMMENTED OUT
context = app.context # Not needed when filesystem is commented out
server_names=["fetch"] # All agents use fetch only
Output Handling:
return f"✅ Stock Analysis Complete for {company_name}!\n\n" + "="*60 + "\n" + result
Error Handling
return f"💥 Error during workflow execution: {str(e)}"
Summary by CodeRabbit
New Features
Improvements
Refactor