-
Notifications
You must be signed in to change notification settings - Fork 769
docs: fix import paths for Workflow and WorkflowResult #525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughDocs updated to change example imports of Workflow and WorkflowResult from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
docs/advanced/monitoring.mdx (1)
1596-1596
: Fix encoding artifact in cost alert titleSame artifact as above; clean the string.
- title: '=� LLM Cost Alert' + title: 'LLM Cost Alert'
🧹 Nitpick comments (1)
docs/advanced/composition.mdx (1)
40-40
: Optional: prefer timezone-aware timestampsSince the examples use timestamps (e.g.,
datetime.utcnow().isoformat()
later), consider importingtimezone
and usingdatetime.now(timezone.utc).isoformat()
for clarity.-from datetime import datetime +from datetime import datetime, timezoneThen, where applicable:
datetime.now(timezone.utc).isoformat()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/advanced/composition.mdx
(1 hunks)docs/advanced/monitoring.mdx
(4 hunks)docs/advanced/temporal.mdx
(2 hunks)docs/cloud/getting-started.mdx
(1 hunks)docs/concepts/workflows.mdx
(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: rholinshead
PR: lastmile-ai/mcp-agent#414
File: src/mcp_agent/logging/logger.py:18-19
Timestamp: 2025-09-05T14:31:48.139Z
Learning: In the mcp-agent logging module (src/mcp_agent/logging/logger.py), temporalio should be imported lazily with try/except ImportError to avoid making it a hard dependency. Use temporalio.workflow.in_workflow() instead of isinstance checks on internal classes like _WorkflowInstanceImpl.
📚 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:
docs/cloud/getting-started.mdx
docs/advanced/monitoring.mdx
docs/advanced/temporal.mdx
docs/advanced/composition.mdx
📚 Learning: 2025-09-05T14:31:48.139Z
Learnt from: rholinshead
PR: lastmile-ai/mcp-agent#414
File: src/mcp_agent/logging/logger.py:18-19
Timestamp: 2025-09-05T14:31:48.139Z
Learning: In the mcp-agent logging module (src/mcp_agent/logging/logger.py), temporalio should be imported lazily with try/except ImportError to avoid making it a hard dependency. Use temporalio.workflow.in_workflow() instead of isinstance checks on internal classes like _WorkflowInstanceImpl.
Applied to files:
docs/advanced/monitoring.mdx
docs/advanced/temporal.mdx
docs/advanced/composition.mdx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: update_release_draft
🔇 Additional comments (6)
docs/advanced/temporal.mdx (2)
82-82
: Consistent public import sourceUpdated import to
mcp_agent.executor.workflow
matches the rest of the PR; looks correct.
487-487
: Second import update is also correctMatches usage in the code block below; no further changes needed.
docs/advanced/composition.mdx (1)
37-37
: Import path update is correct
Workflow
/WorkflowResult
reference updated to the canonical module; usage below type-checks in the examples.docs/advanced/monitoring.mdx (1)
158-158
: Import path fix matches PR goalThe updated import aligns with the rest of the docs changes.
docs/concepts/workflows.mdx (1)
27-27
: Import update is correctMatches the new public module for
Workflow
andWorkflowResult
.docs/cloud/getting-started.mdx (1)
133-133
: Approve import path update
No remainingfrom mcp_agent.workflows import Workflow
occurrences found across Python and documentation files; changes are approved.
slack_configs: | ||
- channel: '#critical-alerts' | ||
title: '=¨ CRITICAL: MCP Agent Alert' | ||
title: '=� CRITICAL: MCP Agent Alert' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove stray non-ASCII/control characters in alert title
The title
contains unexpected characters (=�
). This will render poorly in docs.
- title: '=� CRITICAL: MCP Agent Alert'
+ title: 'CRITICAL: MCP Agent Alert'
📝 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.
title: '=� CRITICAL: MCP Agent Alert' | |
title: 'CRITICAL: MCP Agent Alert' |
🤖 Prompt for AI Agents
In docs/advanced/monitoring.mdx around line 1463, the alert title contains stray
non-ASCII/control characters ("=�") that will render poorly; remove those
characters and ensure the title contains only valid ASCII or intended Unicode
characters (e.g., change `title: '=� CRITICAL: MCP Agent Alert'` to `title:
'CRITICAL: MCP Agent Alert'` or the correct localized text), then save the file
and run a quick docs build or linter to verify no other control characters
remain.
await handler(alert) | ||
|
||
print(f"=¨ ALERT: {alert.severity.value.upper()} - {alert.message}") | ||
print(f"=� ALERT: {alert.severity.value.upper()} - {alert.message}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix console print artifact
The printed prefix includes a stray character (=�
). Use a plain ASCII prefix.
- print(f"=� ALERT: {alert.severity.value.upper()} - {alert.message}")
+ print(f"ALERT: {alert.severity.value.upper()} - {alert.message}")
📝 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.
print(f"=� ALERT: {alert.severity.value.upper()} - {alert.message}") | |
print(f"ALERT: {alert.severity.value.upper()} - {alert.message}") |
🤖 Prompt for AI Agents
In docs/advanced/monitoring.mdx around line 1636 the printed alert prefix
contains a stray non-ASCII character ("=�"); replace it with a plain ASCII
prefix (for example "=== ALERT:" or "ALERT:") so the print statement uses only
standard ASCII characters and reads clearly.
Summary by CodeRabbit