feat(agents): Replace openai-agents with pydantic-ai implementation #139
Conversation
- Migrated from openai-agents to pydantic-ai framework - Replaced Agent class from agents with Agent from pydantic_ai - Updated agent creation to use pydantic-ai patterns - Replaced handoff mechanism with tool-based delegation - Updated manager agent to delegate to developer using @agent.tool decorator - Replaced Runner.run with Agent.run methods - Replaced session-based management with message_history tracking - Updated MCP server integration to use pydantic_ai.mcp.MCPServerStdio - Removed function_tool decorators (pydantic-ai tools are plain functions) - Updated dependencies in pyproject.toml to use pydantic-ai and pydantic-ai-slim - Preserved manager-developer architecture and 3-phase workflow - All tests passing (98 passed, 2 skipped, 1 xfailed) Co-Authored-By: AJ Steers <[email protected]>
Original prompt from AJ Steers |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
📝 WalkthroughWalkthroughReplaces OpenAI-specific agents and handoff models with pydantic_ai Agent-based tooling and RunContext integration, moves MCP server wiring into toolsets, refactors tools to factory patterns with RunContext logging, simplifies run/session orchestration via SessionState/message_history and iterative agent.run loops, and updates defaults/dependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Manager as Manager Agent
participant Dev as Developer Agent
participant MCP as MCP Servers
Note right of Manager #eef: Manager toolset includes delegate_to_developer + MCP servers
User->>Manager: submit assignment (title, description)
Manager->>Manager: plan / decide delegation
Manager->>Manager: delegate_to_developer(task) --bound tool-->
Manager->>Dev: developer_agent.run(ctx, task)
activate Dev
Dev->>MCP: call MCP tools (fs/browser/connector)
MCP-->>Dev: tool results
Dev->>Manager: report_back_to_manager(status, summary) --bound tool-->
deactivate Dev
Manager->>Manager: update SessionState / message_history
alt incomplete
Manager->>Dev: delegate_to_developer(...)
else complete
Manager-->>User: final outputs/status
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
👋 Welcome to the Airbyte Connector Builder MCP!Thank you for your contribution! Here are some helpful tips and reminders for your convenience. Testing This Branch via MCPTo test the changes in this specific branch with an MCP client like Claude Desktop, use the following configuration: {
"mcpServers": {
"connector-builder-mcp-dev": {
"command": "uvx",
"args": ["--from", "git+https://github.com/airbytehq/connector-builder-mcp.git@devin/1760288645-replace-openai-agents-with-pydantic-ai", "connector-builder-mcp"]
}
}
}Testing This Branch via CLIYou can test this version of the MCP Server using the following CLI snippet: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/connector-builder-mcp.git@devin/1760288645-replace-openai-agents-with-pydantic-ai#egg=airbyte-connector-builder-mcp' --helpPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
AI Builder EvaluationsAI builder evaluations run automatically under the following conditions:
A set of standardized evaluations also run on a schedule (Mon/Wed/Fri at midnight UTC) and can be manually triggered via workflow dispatch. Helpful ResourcesIf you have any questions, feel free to ask in the PR comments or join our Slack community. |
PyTest Results (Full)0 tests 0 ✅ 0s ⏱️ Results for commit a0386f7. ♻️ This comment has been updated with latest results. |
- Imported duckduckgo_search from pydantic_ai.tools - Added to developer agent's tools list - Replaces the previous WebSearchTool from openai-agents - Addresses web search capability checklist item from PR description Co-Authored-By: AJ Steers <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
30-31: Remove duplicated dev dependency.
poethepoetis listed twice. Keep the newer pin only.- "poethepoet>=0.29.0", "poethepoet>=0.37.0",
🧹 Nitpick comments (9)
pyproject.toml (1)
69-69: Align mypy target python with runtime.
requires-pythonis ">=3.11" but mypy is set to "3.10". Set to 3.11 to avoid mismatches in type inference.Suggested:
[tool.mypy] python_version = "3.11"connector_builder_agents/src/run.py (4)
28-31: Use a collision‑free session id.
int(time.time())can collide under concurrency. Prefer UUID.-def generate_session_id() -> str: - """Generate a unique session ID based on current timestamp.""" - return f"unified-mcp-session-{int(time.time())}" +import uuid + +def generate_session_id() -> str: + """Generate a unique session ID.""" + return f"unified-mcp-session-{uuid.uuid4()}"
98-105: Avoid CWD‑relative prompt path; use the module constant.This will fail if run outside repo root. Reuse
ROOT_PROMPT_FILE_STR.- prompt_file = Path("./prompts") / "root-prompt.md" - prompt = prompt_file.read_text(encoding="utf-8") + "\n\n" - prompt += instructions + from .constants import ROOT_PROMPT_FILE_STR + prompt = ROOT_PROMPT_FILE_STR + "\n\n" + instructions
200-234: Add safety limits and trim message history to prevent unbounded growth.
- Loop can run indefinitely if agents never mark completion.
message_historygrows without bounds.- while not is_complete(session_state): + from .constants import MAX_CONNECTOR_BUILD_STEPS + MAX_HISTORY = 200 # local cap; consider promoting to constants + while not is_complete(session_state) and iteration_count < MAX_CONNECTOR_BUILD_STEPS: @@ - session_state.message_history.extend(run_result.new_messages()) + session_state.message_history.extend(run_result.new_messages()) + if len(session_state.message_history) > MAX_HISTORY: + session_state.message_history = session_state.message_history[-MAX_HISTORY:] @@ - return all_run_results + if not is_complete(session_state): + update_progress_log("⏹ Reached max iterations; stopping.", session_state) + return all_run_results
236-241: Avoidsys.exitin run_manager_developer_build.Return cleanly so callers control process termination.
- except KeyboardInterrupt: + except KeyboardInterrupt: update_progress_log("\n🛑 Build terminated (ctrl+c input received).", session_state) - sys.exit(0) + return all_run_results if 'all_run_results' in locals() else []connector_builder_agents/src/agents.py (2)
106-121: Handle errors from delegated developer runs.If the developer toolchain fails (tool error/timeout), log via
log_tool_failureand propagate a concise message to the manager to decide next steps.- result = await developer_agent.run( + try: + result = await developer_agent.run( assignment_description, message_history=ctx.deps.message_history, deps=ctx.deps, ) + except Exception as e: + update_progress_log( + f"🛠️ Delegation failed for '{assignment_title}': {e}", ctx.deps + ) + return f"Delegation failed: {e}"
118-120: Cap shared message history to avoid ballooning.Same concern as in run.py; trim after extending.
- ctx.deps.message_history.extend(result.new_messages()) + ctx.deps.message_history.extend(result.new_messages()) + if len(ctx.deps.message_history) > 200: + ctx.deps.message_history = ctx.deps.message_history[-200:]connector_builder_agents/src/tools.py (2)
22-31: Avoid mutable default formessage_history.Use
Field(default_factory=list)and drop manual kwargs handling.- message_history: list = [] + message_history: list = Field(default_factory=list) @@ - message_history = kwargs.get("message_history", []) @@ super().__init__( workspace_dir=workspace_dir, execution_log_file=execution_log_file, - message_history=message_history, + # Let Pydantic default_factory create a fresh list per instance start_time=start_time, **kwargs, )Also applies to: 40-44
58-73: Duplicate connector‑builder MCP servers; reuse a single instance.Two identical
MCPServerStdio("uv", ["run","airbyte-connector-builder-mcp"])are spawned, contradicting the “reuse instances” docstring and wasting resources.-connector_builder_dev = MCP_CONNECTOR_BUILDER_FOR_DEVELOPER() -connector_builder_manager = MCP_CONNECTOR_BUILDER_FOR_MANAGER() +connector_builder = MCP_CONNECTOR_BUILDER_FOR_DEVELOPER() @@ - all_servers = [ + all_servers = [ # MCP_PLAYWRIGHT_WEB_BROWSER(), - connector_builder_dev, - connector_builder_manager, + connector_builder, filesystem_server, ] @@ - manager_servers = [ - connector_builder_manager, + manager_servers = [ + connector_builder, filesystem_server, ] @@ - developer_servers = [ + developer_servers = [ # MCP_PLAYWRIGHT_WEB_BROWSER(), - connector_builder_dev, + connector_builder, filesystem_server, ]Or drop the separate lambdas and define a single
MCP_CONNECTOR_BUILDERfactory.Also applies to: 106-127
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
connector_builder_agents/src/agents.py(3 hunks)connector_builder_agents/src/constants.py(1 hunks)connector_builder_agents/src/guidance.py(0 hunks)connector_builder_agents/src/run.py(5 hunks)connector_builder_agents/src/tools.py(5 hunks)pyproject.toml(1 hunks)
💤 Files with no reviewable changes (1)
- connector_builder_agents/src/guidance.py
🧰 Additional context used
🧬 Code graph analysis (2)
connector_builder_agents/src/run.py (3)
connector_builder_agents/src/_util.py (1)
get_secrets_dotenv(29-88)connector_builder_agents/src/agents.py (2)
create_developer_agent(23-51)create_manager_agent(54-151)connector_builder_agents/src/tools.py (4)
SessionState(17-50)update_progress_log(144-168)is_complete(48-50)is_complete(132-134)
connector_builder_agents/src/agents.py (2)
connector_builder_agents/src/tools.py (5)
SessionState(17-50)create_log_progress_milestone_from_developer_tool(284-291)create_log_problem_encountered_by_developer_tool(264-271)create_log_tool_failure_tool(220-251)update_progress_log(144-168)connector_builder_agents/src/guidance.py (2)
get_default_developer_prompt(76-93)get_default_manager_prompt(58-73)
🪛 GitHub Actions: Dependency Analysis
pyproject.toml
[error] 1-1: DEP002 'pydantic-ai' defined as a dependency but not used in the codebase.
[error] 1-1: DEP002 'pydantic-ai-slim' defined as a dependency but not used in the codebase.
⏰ 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). (3)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
🔇 Additional comments (2)
connector_builder_agents/src/agents.py (1)
4-6: Verify availability ofduckduckgo_searchtool.Ensure
duckduckgo_searchworks without extra install. If it requires an extra, add it to project deps or the newagentsextra.If needed, add:
[project.optional-dependencies] agents = [ "pydantic-ai[duckduckgo]", # or the specific package providing duckduckgo search ]connector_builder_agents/src/constants.py (1)
16-18: Clarify model ID format for pydantic-aiAgent shorthand supports provider-prefixed IDs (e.g.
Agent("openai:gpt-4o")), but direct OpenAI model instantiation (e.g.OpenAIChatModel) expects the raw name ("gpt-4o"). EnsureDEFAULT_DEVELOPER_MODELandDEFAULT_MANAGER_MODELare only used withAgent; otherwise strip theopenai:prefix.
- Corrected import path from pydantic_ai.common_tools.duckduckgo - Changed to function call: duckduckgo_search_tool() - Added [duckduckgo] optional group to pydantic-ai-slim dependency - Added emoji as direct dependency (used in tools.py) - Excluded connector_builder_agents/src/evals/ from Deptry scanning - Fixes Deptry false positives for pydantic-ai package usage Co-Authored-By: AJ Steers <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
connector_builder_agents/src/agents.py (1)
120-120: Consider type-safe result data handling.The conversion
str(result.data)assumesresult.datais present and meaningful. Ifresult.dataisNoneor an unexpected type, the conversion succeeds silently but may return unhelpful values like"None".For better type safety, consider explicit handling:
- return str(result.data) + if result.data is None: + return "Task completed (no data returned)" + return str(result.data)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
connector_builder_agents/src/agents.py(3 hunks)pyproject.toml(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
connector_builder_agents/src/agents.py (2)
connector_builder_agents/src/tools.py (5)
SessionState(17-50)create_log_progress_milestone_from_developer_tool(284-291)create_log_problem_encountered_by_developer_tool(264-271)create_log_tool_failure_tool(220-251)update_progress_log(144-168)connector_builder_agents/src/guidance.py (2)
get_default_developer_prompt(76-93)get_default_manager_prompt(58-73)
⏰ 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). (3)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (Fast)
🔇 Additional comments (4)
connector_builder_agents/src/agents.py (4)
4-5: LGTM: Clean migration to pydantic-ai framework.The imports correctly replace the openai-agents framework with pydantic-ai's Agent and RunContext, and adopt the built-in duckduckgo_search_tool.
23-51: LGTM: Developer agent correctly configured.The developer agent is properly initialized with pydantic-ai's Agent class, includes appropriate tools (progress logging, problem reporting, and web search), and correctly integrates MCP servers via the toolsets pattern.
54-85: LGTM: Manager agent properly initialized.The manager agent configuration follows the same pydantic-ai patterns as the developer agent, with appropriate tools for orchestration and MCP server integration.
122-149: LGTM: Well-structured progress reporting mechanism.The
report_back_to_managertool provides a clean interface for the developer agent to communicate status back to the manager with structured flags (is_full_success,is_partial_success,is_blocked) and detailed updates.
|
/build-connector
|
The evals command was failing with ModuleNotFoundError for pydantic_ai because the connector_builder_agents subdirectory has its own separate package configuration that still referenced openai-agents. - Replaced openai-agents>=0.3.3 with pydantic-ai>=0.0.14,<1.0 - Added pydantic-ai-slim[openai,duckduckgo]>=0.0.14,<1.0 - Removed mcp-agent (was transitive dependency of openai-agents) - Removed openinference-instrumentation-openai-agents (openai-agents specific) - Updated connector_builder_agents/uv.lock via uv sync Fixes: /build-connector evals failure Co-Authored-By: AJ Steers <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
connector_builder_agents/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
connector_builder_agents/pyproject.toml(1 hunks)
⏰ 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). (3)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
The pydantic-ai AgentRunResult object uses 'output' attribute, not 'data'. This was causing the evals CI check to fail with: AttributeError: 'AgentRunResult' object has no attribute 'data' Fixes the delegate_to_developer function in agents.py. Co-Authored-By: AJ Steers <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
connector_builder_agents/src/agents.py (2)
106-110: Add error handling for developer agent invocation.The
developer_agent.run()call lacks error handling. If the developer agent encounters an exception, it will propagate uncaught, potentially causing the manager to fail without proper logging or recovery.The previous review comment on this issue remains valid and unresolved. Consider wrapping the invocation in a try-except block as suggested in the earlier review.
118-118: Limit message history growth.The
ctx.deps.message_history.extend(result.new_messages())call allows unbounded growth. In long-running sessions with many delegations, this could lead to excessive memory consumption and performance degradation.The previous review comment on this issue remains valid and unresolved. Consider implementing a mechanism to prune old messages or set a maximum history size as suggested in the earlier review.
🧹 Nitpick comments (1)
connector_builder_agents/src/agents.py (1)
122-150: Consider enhancing the return value.The tool returns a static string
"Status reported to manager"regardless of the status content. While functional, a more informative return value could include a summary of the reported status.For example:
- return "Status reported to manager" + return f"Reported to manager: {short_status} (full_success={is_full_success}, blocked={is_blocked})"This would provide better context in the message history and agent logs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
connector_builder_agents/src/agents.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
connector_builder_agents/src/agents.py (2)
connector_builder_agents/src/tools.py (5)
SessionState(17-50)create_log_progress_milestone_from_developer_tool(284-291)create_log_problem_encountered_by_developer_tool(264-271)create_log_tool_failure_tool(220-251)update_progress_log(144-168)connector_builder_agents/src/guidance.py (2)
get_default_developer_prompt(76-93)get_default_manager_prompt(58-73)
🔇 Additional comments (4)
connector_builder_agents/src/agents.py (4)
4-5: LGTM!The import changes correctly reflect the migration from openai-agents to pydantic-ai framework.
23-51: LGTM!The developer agent setup correctly uses pydantic-ai patterns:
- Proper return type annotation with
Agent- Correct constructor parameters including
deps_type- MCP servers attached via
toolsets.append()- Web search tool updated to pydantic-ai's
duckduckgo_search_tool()
54-85: LGTM!The manager agent setup follows the same correct pydantic-ai patterns as the developer agent, with proper type annotations and MCP server attachment.
114-114: Confirm.outputis correct
Pydantic-AI’s RunResult object exposes its output via the.outputattribute, not.data.
Found three additional locations in run.py that were using result.data instead of result.output: - Line 143 in run_interactive_build function - Line 224 in run_manager_developer_build function - Line 231 in run_manager_developer_build function This completes the migration from openai-agents to pydantic-ai. Co-Authored-By: AJ Steers <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
connector_builder_agents/src/run.py (1)
145-145: Previous critical issue not addressed: blocking I/O and process termination.The blocking
input()call at Line 145 andsys.exit(0)at Line 154 were flagged in the previous review but remain unresolved.
- Line 145:
input()blocks the async event loop, preventing concurrent operations.- Line 154:
sys.exit(0)terminates the entire process ungracefully, preventing cleanup and making the function non-reusable.Apply this diff to address both issues:
+import asyncio + async def run_interactive_build( prompt: str, model: str, session_id: str, ) -> None: """Run the agent using interactive mode with conversation loop.""" workspace_dir = get_workspace_dir(session_id) session_state = create_session_state(workspace_dir) all_mcp_servers, _, _ = create_session_mcp_servers(session_state) agent = Agent( model, name="MCP Connector Builder", deps_type=SessionState, system_prompt=( "You are a helpful assistant with access to MCP tools for building Airbyte connectors." ), ) for mcp_server in all_mcp_servers: agent.toolsets.append(mcp_server) input_prompt: str = prompt while True: update_progress_log("\n⚙️ AI Agent is working...", session_state) try: result = await agent.run( input_prompt, message_history=session_state.message_history, deps=session_state, ) session_state.message_history.extend(result.new_messages()) update_progress_log(f"\n🤖 AI Agent: {result.output}", session_state) - input_prompt = input("\n👤 You: ") + loop = asyncio.get_event_loop() + input_prompt = await loop.run_in_executor(None, input, "\n👤 You: ") if input_prompt.lower() in {"exit", "quit"}: update_progress_log("☑️ Ending conversation...", session_state) break except KeyboardInterrupt: update_progress_log( "\n🛑 Conversation terminated (ctrl+c input received).", session_state ) - sys.exit(0) + return None return NoneAlso applies to: 150-154
🧹 Nitpick comments (1)
connector_builder_agents/src/run.py (1)
50-50: Consider preserving type information in return signatures.The return types have been loosened from
list[RunResult]tolist(Line 165) and fromlist[RunResult] | Nonetolist | None(Line 50). This removes valuable type information and reduces type safety.If
RunResultis no longer available due to the framework migration, consider defining a replacement type or using a more specific type hint.If pydantic-ai provides a result type, use it:
+from pydantic_ai import RunResult # or equivalent + -) -> list | None: +) -> list[RunResult] | None:-) -> list: +) -> list[RunResult]:Also applies to: 165-165
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
connector_builder_agents/src/run.py(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
connector_builder_agents/src/run.py (3)
connector_builder_agents/src/_util.py (1)
get_secrets_dotenv(29-88)connector_builder_agents/src/agents.py (2)
create_developer_agent(23-51)create_manager_agent(54-151)connector_builder_agents/src/tools.py (4)
SessionState(17-50)update_progress_log(144-168)is_complete(48-50)is_complete(132-134)
…_output The pydantic-ai AgentRunResult object uses 'output' attribute, not 'final_output'. This was causing the evals CI check to fail with: AttributeError: 'AgentRunResult' object has no attribute 'final_output' Completes the migration from openai-agents to pydantic-ai by fixing the last remaining incompatible attribute access in the evals code. Co-Authored-By: AJ Steers <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
connector_builder_agents/src/evals/task.py (1)
54-54: Rename dictionary key to match attribute. The code now usesfinal_result.outputbut the result dict still uses"final_output"—consider renaming it to"output"for consistency.
The pydantic-ai Agent class expects MCP servers to be passed via the 'toolsets' parameter during construction, not appended afterward. Updated both create_developer_agent and create_manager_agent to follow the correct pattern from the reference implementation in agentic-connector-builder-webapp. Fixes issue reported by @aaronsteers in PR #139. Co-Authored-By: AJ Steers <[email protected]>
|
Fixed! ✅ The MCP servers are now properly passed via the Changes made:
Reference: Verification: Commit: d733a08 |
|
/build-connector
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
pyproject.toml (1)
76-81: Deptry misconfiguration: DEP001 errors (missing deps) still fail CI.The errors show DEP001 for pydantic_ai and emoji, but config ignores DEP003. Either exclude the agents tree from deptry, or declare deps in an optional extra.
Minimal fix (exclude entire agents dir so deptry doesn’t scan it):
[tool.deptry] -extend_exclude = [ - ".*connector_builder_agents/src/evals/.*", -] - -[tool.deptry.per_rule_ignores] -DEP003 = ["pydantic_ai", "emoji"] # These are used in the connector builder agents project. +extend_exclude = [ + ".*connector_builder_agents/.*", + ".*connector_builder_agents/src/evals/.*", +]Alternative (preferred structural fix) — keep scanning, but move agent-only deps to an optional extra and/or include the agents package in the wheel. See earlier reviewer note; re-posting for visibility.
Run deptry locally after the change to confirm it passes.
connector_builder_agents/src/tools.py (1)
114-125: env={} wipes subprocess environment; preserve parent env.Passing env={} replaces all inherited vars (e.g., credentials), causing unexpected failures. Use os.environ.copy() (and merge extras if needed) or omit env.
Apply this diff:
+import os @@ return FilteredMCPServerStdio( @@ - env={}, + env=os.environ.copy(), @@ return FilteredMCPServerStdio( @@ - env={}, + env=os.environ.copy(), @@ MCP_PLAYWRIGHT_WEB_BROWSER = lambda: MCPServerStdio( # noqa: E731 @@ - env={}, + env=os.environ.copy(), @@ return MCPServerStdio( @@ - env={}, + env=os.environ.copy(),Also applies to: 132-153, 156-165, 169-178, 4-10
connector_builder_agents/src/run.py (2)
131-156: Don’t block the event loop or terminate the process in a library function.Replace blocking input() with asyncio.to_thread and return gracefully on Ctrl+C.
+import asyncio @@ input_prompt: str = prompt while True: update_progress_log("\n⚙️ AI Agent is working...", session_state) try: result = await agent.run( input_prompt, message_history=session_state.message_history, deps=session_state, ) session_state.message_history.extend(result.new_messages()) update_progress_log(f"\n🤖 AI Agent: {result.output}", session_state) - input_prompt = input("\n👤 You: ") + input_prompt = await asyncio.to_thread(input, "\n👤 You: ") if input_prompt.lower() in {"exit", "quit"}: update_progress_log("☑️ Ending conversation...", session_state) break except KeyboardInterrupt: update_progress_log( "\n🛑 Conversation terminated (ctrl+c input received).", session_state ) - sys.exit(0) + return NoneAlso applies to: 4-12
236-241: Avoid sys.exit in run_manager_developer_build; return gracefully.Let callers clean up instead of terminating the process.
except KeyboardInterrupt: update_progress_log("\n🛑 Build terminated (ctrl+c input received).", session_state) - sys.exit(0) + return []
🧹 Nitpick comments (4)
connector_builder_agents/src/tools.py (2)
74-91: Optional: log failures around call_tool() for better triage.Wrap call_tool() in try/except to log exceptions, then re-raise.
- result = await call_tool(tool_name, tool_args, None) + try: + result = await call_tool(tool_name, tool_args, None) + except Exception as ex: + update_progress_log( + f"❌ [{agent_name}] MCP Tool '{tool_name}' failed: {ex}", + session_state, + ) + raise
121-124: Make MCP timeouts configurable.Long operations may exceed 60–180s. Allow override via env and raise defaults.
- timeout=180, + timeout=int(os.environ.get("AB_MCP_TIMEOUT_SEC", "300")), @@ - timeout=60, + timeout=int(os.environ.get("AB_MCP_TIMEOUT_SEC", "300")), @@ - timeout=60, + timeout=int(os.environ.get("AB_MCP_TIMEOUT_SEC", "300")), @@ - timeout=60, + timeout=int(os.environ.get("AB_MCP_TIMEOUT_SEC", "300")),Also applies to: 140-143, 162-164, 176-178
connector_builder_agents/src/run.py (2)
29-33: Optional: improve session id uniqueness.Using seconds can collide. Prefer uuid4.
-from pathlib import Path +from pathlib import Path +import uuid @@ def generate_session_id() -> str: """Generate a unique session ID based on current timestamp.""" - return f"unified-mcp-session-{int(time.time())}" + return f"unified-mcp-session-{uuid.uuid4()}"
100-107: Verify ROOT_PROMPT_FILE_PATH existence at runtime.Add a sanity check to fail early with a clear message if missing.
- prompt_file = ROOT_PROMPT_FILE_PATH + prompt_file = ROOT_PROMPT_FILE_PATH + if not prompt_file.exists(): + raise FileNotFoundError(f"ROOT_PROMPT_FILE_PATH not found: {prompt_file}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
connector_builder_agents/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
connector_builder_agents/pyproject.toml(1 hunks)connector_builder_agents/src/guidance.py(1 hunks)connector_builder_agents/src/phases.py(0 hunks)connector_builder_agents/src/run.py(6 hunks)connector_builder_agents/src/tools.py(7 hunks)connector_builder_mcp/server.py(1 hunks)pyproject.toml(1 hunks)
💤 Files with no reviewable changes (1)
- connector_builder_agents/src/phases.py
🚧 Files skipped from review as they are similar to previous changes (1)
- connector_builder_agents/pyproject.toml
🧰 Additional context used
🧬 Code graph analysis (1)
connector_builder_agents/src/run.py (3)
connector_builder_agents/src/_util.py (1)
get_secrets_dotenv(29-88)connector_builder_agents/src/agents.py (2)
create_developer_agent(23-47)create_manager_agent(50-145)connector_builder_agents/src/tools.py (6)
SessionState(20-53)create_session_mcp_servers(181-213)update_progress_log(228-252)create_session_state(56-58)is_complete(51-53)is_complete(216-218)
🪛 GitHub Actions: Dependency Analysis
connector_builder_agents/src/run.py
[error] 8-8: DEP001 'pydantic_ai' imported but missing from the dependency definitions
connector_builder_agents/src/tools.py
[error] 11-11: DEP001 'emoji' imported but missing from the dependency definitions
[error] 14-14: DEP001 'pydantic_ai' imported but missing from the dependency definitions
[error] 15-15: DEP001 'pydantic_ai' imported but missing from the dependency definitions
⏰ 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). (3)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
🔇 Additional comments (3)
connector_builder_mcp/server.py (1)
24-28: LGTM: quieter startup banner + clear stderr separators.Suppressing FastMCP banner and adding start/stop separators improves logs without changing behavior. Good to ship.
Also applies to: 34-36
connector_builder_agents/src/guidance.py (1)
49-50: LGTM: clarified “last resort” failure guidance.Text change improves clarity without behavioral impact.
connector_builder_agents/src/tools.py (1)
100-108: Verify exclusion semantics for FilteredMCPServerStdio.Substring filtering may hide similarly named tools unintentionally. Confirm intended behavior; consider exact-name matching or prefix-only matching if safer.
Also applies to: 142-153
…ctory - Change extend_exclude to cover all of connector_builder_agents/ - Change DEP003 to DEP001 to match actual error type from CI - Fixes Deptry CI failure by properly excluding the separate subproject The connector_builder_agents directory has its own pyproject.toml with its own dependencies (pydantic-ai-slim, emoji), so it should not be analyzed against the root project's dependency definitions. Co-Authored-By: AJ Steers <[email protected]>
…' of github.com:airbytehq/connector-builder-mcp into devin/1760288645-replace-openai-agents-with-pydantic-ai
|
/build-connector
✳️ Show/Hide Summary OutputEvaluation Run SummaryExperiment: RXhwZXJpbWVudDoxMDQ= Run Date: 2025-10-22 19:09 UTC
Experiment Stats
Per-Connector ResultsComparing to prior experiment: RXhwZXJpbWVudDo3MQ==
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
connector_builder_agents/src/tools.py (1)
116-127: env={} strips inherited env; merge with os.environ (likely cause of the evals failure).Passing env={} replaces the subprocess environment, dropping PATH/credentials. Inherit and optionally extend instead.
Apply this diff:
@@ -import json +import json +import os @@ - env={}, + env=os.environ.copy(), @@ - env={}, + env=os.environ.copy(), @@ - env={}, + env=os.environ.copy(), @@ - env={}, + env=os.environ.copy(),If you need to augment env, merge extras: env = os.environ.copy() | extra.
Also applies to: 134-155, 158-166, 171-180
🧹 Nitpick comments (4)
connector_builder_agents/src/tools.py (4)
33-34: Simplify message_history handling; rely on the Field default (and kwargs override).Redundant local message_history variable and explicit pass are unnecessary.
Apply this diff:
- message_history = kwargs.get("message_history", []) @@ super().__init__( workspace_dir=workspace_dir, execution_log_file=execution_log_file, - message_history=message_history, start_time=start_time, **kwargs, )Also applies to: 46-46
102-109: Substring filtering can hide unintended tools.Using any(excluded in name ...) may exclude tools whose names merely contain the token. Prefer exact match or anchored patterns.
Example:
excluded = set(self.excluded_tools) return {n: t for n, t in all_tools.items() if n not in excluded} # or use fnmatch for explicit wildcards
12-14: Import Field from pydantic public API.Use from pydantic import Field to avoid relying on internal module paths.
Apply this diff:
-from pydantic import BaseModel -from pydantic.fields import Field +from pydantic import BaseModel, Field
241-255: Optional: make logging writes safer and emoji parsing more robust.
- File writes can interleave under concurrency; add a simple file lock.
- Emojis can be multi-codepoint; current 2-char check is partial.
If contention is possible, consider a lock around file writes (e.g., fasteners/portalocker), or serialize updates via a single task. For emoji parsing, accept the current approach or switch to a lib that parses grapheme clusters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
connector_builder_agents/src/tools.py(7 hunks)
🔇 Additional comments (2)
connector_builder_agents/src/tools.py (2)
25-25: Good fix: avoid mutable default.Using Field(default_factory=list) for message_history is correct.
61-95: Remove the suggestion to pass ctx as the third argument to call_tool; the current code is correct.The review comment's core ctx-passing suggestion is incorrect. Per the pydantic-ai API,
CallToolFuncaccepts(name: str, args: dict[str, Any], metadata: dict[str, Any] | None = None), notctx. The third argument should be metadata (e.g.{'deps': ctx.deps}), not the RunContext object. The current code correctly passesNone.The redaction suggestion for sensitive args before logging is reasonable and independent of this error.
Likely an incorrect or invalid review comment.
aaronsteers
left a comment
aaronsteers
left a comment
There was a problem hiding this comment.
✅ Approved! Nice work, @pnilan
pnilan
deleted the
devin/1760288645-replace-openai-agents-with-pydantic-ai
branch
Replace openai-agents with pydantic-ai implementation
Summary
Migrated the connector builder MCP from
openai-agentstopydantic-ai, updating the agent architecture, tool delegation mechanisms, and dependencies across both the main package and theconnector_builder_agentssubpackage.Key changes:
openai-agents.Agentwithpydantic_ai.Agenthandoff()classes to tool-based delegation usingRunContextWebSearchTool()toduckduckgo_search_tool()from pydantic-aimessage_historytoSessionStatefor pydantic-ai's message managementmcp_serversparameter totoolsets.append()for attaching MCP serverspyproject.tomlfiles (root andconnector_builder_agents/) to use pydantic-ai packagesReview & Testing Checklist for Human
/build-connectorcommand to confirm evals work with pydantic-ai (initial failure was fixed by adding pydantic-ai to connector_builder_agents/pyproject.toml)Notes
evals/subdirectory to prevent false positivesLink to Devin run: https://app.devin.ai/sessions/0011f26c17d1458b81190e5bcfe5e0f7
Requested by: @aaronsteers
Summary by CodeRabbit
New Features
Improvements
Refactor
Chores