fix: Reconnect previous_response_id to maintain conversation context across loop iterations #131
Conversation
…across loop iterations 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:
|
👋 Greetings, Airbyte Team Member!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/1760131223-fix-response-id-context", "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/1760131223-fix-response-id-context#egg=airbyte-connector-builder-mcp' --helpPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
📝 WalkthroughWalkthroughAdded explanatory comments in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (4)
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: 0
🧹 Nitpick comments (1)
connector_builder_agents/src/run.py (1)
271-273: Add defensive error handling for response_id access.While the code guards against empty
raw_responses, it doesn't handle the case whereraw_responses[-1]might not have aresponse_idattribute or where theresponse_idmight be invalid. This could lead toAttributeErrorat runtime.Apply this diff to add defensive error handling:
-prev_response_id = ( - run_result.raw_responses[-1].response_id if run_result.raw_responses else None -) +prev_response_id = None +if run_result.raw_responses: + try: + prev_response_id = run_result.raw_responses[-1].response_id + except AttributeError: + update_progress_log( + "⚠️ Warning: Could not extract response_id from last response", session_state + )Alternatively, if you're confident that
response_idalways exists on raw responses, consider adding a comment explaining this assumption:+# Note: raw_responses[-1].response_id is guaranteed to exist when raw_responses is non-empty prev_response_id = ( run_result.raw_responses[-1].response_id if run_result.raw_responses else None )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
connector_builder_agents/src/run.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
connector_builder_agents/src/run.py (1)
connector_builder_agents/src/tools.py (2)
is_complete(53-55)is_complete(179-181)
⏰ 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). (2)
- GitHub Check: Pytest (Fast)
- GitHub Check: Run Evals (Single Connector)
🔇 Additional comments (2)
connector_builder_agents/src/run.py (2)
260-260: LGTM! Initialization is appropriate.Initializing
prev_response_idtoNonebefore the loop is correct, ensuring the firstRunner.runcall starts a fresh conversation thread.
268-268: Ensure duplicate response IDs are prevented
No validation or duplicate checks exist aroundprevious_response_idin connector_builder_agents/src/run.py:271–273; verify that passing this ID won’t reintroduce errors or add the necessary checks.
… previous_response_id Co-Authored-By: AJ Steers <[email protected]>
1 participant
fix: Reconnect previous_response_id to maintain conversation context across loop iterations
Summary
Uncomments the
previous_response_idcontext tracking in the manager-developer orchestration loop. Previously, each loop iteration was starting a fresh conversation instead of continuing from the previous context, leading to agent identity confusion and coordination failures.Root cause identified: The commented-out
previous_response_idlines (260, 268, 271) meant eachRunner.run()call created a new conversation thread rather than continuing the existing one, causing agents to lose context about their role and previous work.Review & Testing Checklist for Human
Notes
This change addresses the core context bleeding issue identified in the investigation. The fix is simple but affects critical orchestration infrastructure, so thorough testing is essential before merge.
Link to Devin run: https://app.devin.ai/sessions/6acb99e3b34a4918af907764ca9b55c3
Requested by: @aaronsteers
Summary by CodeRabbit