fix: standardize worker agent task dict interface - #267
Conversation
WalkthroughWorker agents (FrontendWorkerAgent, TestWorkerAgent) now accept task inputs as dictionaries (Dict[str, Any]) instead of Task objects; LeadAgent now forwards full task data via Changes
Sequence Diagram(s)(omitted — changes are refactor/signature updates without new multi-component control flow warranting a diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used📓 Path-based instructions (1)codeframe/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (3)📓 Common learnings📚 Learning: 2026-01-11T23:33:31.895ZApplied to files:
📚 Learning: 2025-12-17T19:21:30.131ZApplied to files:
🧬 Code graph analysis (4)codeframe/agents/frontend_worker_agent.py (1)
codeframe/agents/test_worker_agent.py (1)
tests/agents/test_test_worker_agent.py (4)
tests/agents/test_frontend_worker_agent.py (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)
🔇 Additional comments (12)
Comment |
Standardize worker agents to accept task dictionaries by updating
|
PR Review: Standardize Worker Agent Task Dict InterfaceSummaryThis PR fixes an interface mismatch where LeadAgent passed minimal task dicts (only id/title/description) to FrontendWorkerAgent and TestWorkerAgent, while those agents expected Task objects. The fix standardizes all worker agents to accept input. ✅ Strengths1. Fixes Real Bug
2. Consistent with Existing Patterns
3. Maintains Backward Compatibility
4. Good Test Coverage
|
![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 (5)
codeframe/agents/test_worker_agent.py (1)
196-203: Redundant dict construction —taskis already a dict.Since
taskis now aDict[str, Any]input, reconstructingtask_dictwith the same fields is unnecessary. You can passtaskdirectly or use a slice.♻️ Simplify by passing task directly
- # Task is already a dict, extract fields for git_workflow - task_dict = { - "id": task_id, - "project_id": task.get("project_id"), - "task_number": task.get("task_number", ""), - "title": task_title, - "description": task_description, - } - - commit_sha = self.git_workflow.commit_task_changes( - task=task_dict, files_modified=files_modified, agent_id=self.agent_id - ) + commit_sha = self.git_workflow.commit_task_changes( + task=task, files_modified=files_modified, agent_id=self.agent_id + )tests/agents/test_frontend_worker_auto_commit.py (1)
11-11: Unused import:Taskis no longer used in this file.After migrating to dict-based tasks, the
Taskclass is not instantiated or referenced. Consider removing the unused import.♻️ Remove unused import
-from codeframe.core.models import Task, AgentMaturity +from codeframe.core.models import AgentMaturitytests/agents/test_frontend_worker_agent.py (1)
342-360: Minor inconsistency: Missing optional fields compared to sample_task fixture.This task dict is missing several fields present in the
sample_taskfixture (e.g.,depends_on,can_parallelize,requires_mcp,estimated_tokens,actual_tokens). While this works becauseFrontendWorkerAgent.execute_taskuses.get()for optional fields, consider using consistent field sets across test fixtures for maintainability.codeframe/agents/frontend_worker_agent.py (2)
91-117: Consider defensive handling for missing required dict keys.The code extracts
task["id"],task["title"], andtask["description"]using direct dictionary access (lines 103-105), which will raiseKeyErrorif any key is missing. While callers (LeadAgent viatask.to_dict()) should always provide these, consider either:
- Using
.get()with appropriate defaults/error handling, or- Adding explicit validation with clear error messages
This would provide better debugging when integration issues arise.
♻️ Optional defensive validation
# Extract commonly used fields from task dict + required_fields = ["id", "title", "description"] + missing = [f for f in required_fields if f not in task] + if missing: + raise ValueError(f"Task dict missing required fields: {missing}") + task_id = task["id"] task_title = task["title"] task_description = task["description"]
185-192: Redundant task_dict construction.The
taskparameter is already a dictionary. This code creates a newtask_dictwith a subset of fields, buttaskitself could be passed directly togit_workflow.commit_task_changesif that method accepts the full dict. If the subset is intentional (to avoid passing unnecessary fields), this is fine, but consider documenting why.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
codeframe/agents/frontend_worker_agent.pycodeframe/agents/lead_agent.pycodeframe/agents/test_worker_agent.pytests/agents/test_frontend_worker_agent.pytests/agents/test_frontend_worker_auto_commit.pytests/agents/test_test_worker_agent.pytests/agents/test_test_worker_auto_commit.pytests/agents/test_worker_task_dict_interface.py
🧰 Additional context used
📓 Path-based instructions (1)
codeframe/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
codeframe/**/*.py: Use Python 3.11+ for backend development
Use FastAPI framework for backend API development
Use SQLite with async support (aiosqlite) for database operations
Use tiktoken for token counting
Run linting with ruff check .
Import and use domain-specific repositories from codeframe/persistence/repositories/ for data access instead of direct database calls
Files:
codeframe/agents/lead_agent.pycodeframe/agents/test_worker_agent.pycodeframe/agents/frontend_worker_agent.py
🧠 Learnings (1)
📚 Learning: 2025-12-17T19:21:30.131Z
Learnt from: frankbria
Repo: frankbria/codeframe PR: 128
File: tests/agents/test_bottleneck_detection.py:486-500
Timestamp: 2025-12-17T19:21:30.131Z
Learning: In tests/agents/*.py, when testing bottleneck detection logic, ensure that tests exercising detect_bottlenecks are async and mock _get_agent_workload to return a value below AGENT_OVERLOAD_THRESHOLD (5) while providing a non-empty tasks list to prevent early return. This guarantees the code path for low workload is exercised and behavior under threshold is verified.
Applied to files:
tests/agents/test_test_worker_auto_commit.pytests/agents/test_test_worker_agent.pytests/agents/test_worker_task_dict_interface.pytests/agents/test_frontend_worker_agent.pytests/agents/test_frontend_worker_auto_commit.py
🧬 Code graph analysis (4)
codeframe/agents/lead_agent.py (1)
codeframe/core/models.py (5)
to_dict(88-105)to_dict(197-210)to_dict(249-253)to_dict(284-305)to_dict(631-640)
codeframe/agents/test_worker_agent.py (2)
codeframe/core/models.py (4)
project_id(234-235)Task(257-305)id(230-231)title(242-243)codeframe/persistence/database.py (1)
update_task_commit_sha(426-428)
tests/agents/test_worker_task_dict_interface.py (1)
codeframe/core/models.py (2)
title(242-243)id(230-231)
codeframe/agents/frontend_worker_agent.py (1)
codeframe/core/models.py (4)
project_id(234-235)Task(257-305)id(230-231)title(242-243)
⏰ 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: opencode-review
- GitHub Check: E2E Smoke Tests (Chromium)
- GitHub Check: Macroscope - Correctness Check
🔇 Additional comments (21)
codeframe/agents/test_worker_agent.py (2)
101-127: Clean interface migration with proper Task reconstruction.The dict-based interface aligns with BackendWorkerAgent and ReviewWorkerAgent. The internal Task object construction for
self.current_taskpreserves blocker workflow compatibility while accepting the standardized dict input.
429-446: LGTM — consistent dict-based signature for linting method.The
_run_and_check_lintingmethod signature update aligns with the parentexecute_taskchange. Task ID extraction is handled correctly.codeframe/agents/lead_agent.py (1)
2201-2203: Good fix —task.to_dict()provides complete task data.Using
task.to_dict()instead of a minimal dict ensures downstream workers receive all required fields (project_id,task_number, etc.) that they previously expected from Task objects.tests/agents/test_test_worker_agent.py (2)
30-50: LGTM — fixture properly mirrorsTask.to_dict()output.The dict-based fixture includes all required fields (
project_id,task_number,issue_id, etc.) that the updatedexecute_taskmethod expects. This aligns with howLeadAgentnow passes task data.
401-401: Correct update to dict key access.Using
sample_task["id"]is consistent with the fixture change from Task object to dict.tests/agents/test_test_worker_auto_commit.py (2)
60-79: Test fixture updated correctly to dict format.The dict includes all fields matching
Task.to_dict()output. One minor note:depends_on: Nonediffers from the Task model's default of"", but the implementation handles both gracefully via.get()and conditional checks.
130-149: Consistent dict structure across test cases.Both test fixtures follow the same complete dict structure, ensuring consistent coverage of the dict-based interface.
tests/agents/test_frontend_worker_auto_commit.py (2)
61-80: LGTM — comprehensive dict-based test fixture.The fixture includes all fields that
Task.to_dict()produces, properly testing the updatedexecute_taskinterface.
123-142: Consistent fixture pattern maintained.Both test cases use the same complete dict structure, ensuring thorough coverage of the dict-based interface.
tests/agents/test_frontend_worker_agent.py (3)
44-63: LGTM! Well-structured task dictionary fixture.The
sample_taskfixture now correctly returns a dictionary matching theTask.to_dict()output format, which aligns with the PR objective of standardizing worker agents to acceptDict[str, Any]input. All essential fields are present includingproject_id,task_number, and other fields that were previously missing when LeadAgent passed minimal dicts.
375-387: Acceptable for error handling test.The reduced field set is appropriate here since the test focuses on error handling when
_create_component_filesraises an exception, not on field access patterns.
445-457: Acceptable for file conflict test.Similar to the error handling test, this reduced field set is appropriate since the test focuses on the "file already exists" error scenario.
tests/agents/test_worker_task_dict_interface.py (5)
1-13: Well-documented test module with clear purpose.The docstring clearly explains the test objectives and the interface mismatch being addressed. This provides excellent context for future maintainers.
25-141: Comprehensive integration test for LeadAgent task dict creation.This test effectively validates the critical fix: ensuring
_assign_and_execute_taskpasses complete task dictionaries with all required fields (project_id,task_number, etc.) instead of minimal dicts. The assertions on lines 118-140 provide strong contract verification.One consideration: The test accesses the private method
_assign_and_execute_taskdirectly. This is acceptable for verifying internal behavior changes, but be aware that refactoring the private method may break this test.
143-231: Good coverage for FrontendWorkerAgent dict interface.The tests verify that
execute_taskaccepts dict input without raisingAttributeErrorfor property access patterns (e.g.,task.idvstask["id"]). The mock for_generate_react_componentappropriately isolates the interface contract test from LLM dependencies.
233-331: Good coverage for TestWorkerAgent dict interface.The nested mocking of both
_generate_pytest_testsand_execute_and_correct_testsappropriately isolates the dict interface test. The test structure mirrors the FrontendWorkerAgent tests for consistency.
334-376: No issues found. The Task class is a standard dataclass with all required fields defined with appropriate defaults. The test correctly instantiates Task using keyword arguments, and theto_dict()method properly converts the status enum to its string value as expected by the assertions.codeframe/agents/frontend_worker_agent.py (4)
107-117: Good approach: Creating Task object for backward compatibility.Constructing a
Taskobject forself.current_taskmaintains compatibility with the blocker creation workflow that depends onself.current_task.idandself.current_task.project_id. This is a sound design choice that allows incremental migration without breaking existing functionality.
228-230: Good: Renamed exception variable to avoid shadowing.Renaming the inner exception to
broadcast_errprevents shadowing the outereexception variable, improving code clarity.
455-472: Consistent signature update for _run_and_check_linting.The internal method signature correctly updated to accept
Dict[str, Any], andtask_idis extracted at the start of the method. This maintains consistency withexecute_task.
584-713: Blocker methods still rely on self.current_task (Task object).The
create_blocker,wait_for_blocker_resolution, andcreate_blocker_and_waitmethods accessself.current_task.project_idandself.current_task.id. This works becauseexecute_tasknow constructs aTaskobject and assigns it toself.current_task(lines 108-117).However, if these blocker methods are ever called outside of an
execute_taskcontext (or ifcurrent_taskis not set), the code will fail. The current design is acceptable given the flow, but consider documenting this dependency.
Fix interface mismatch between LeadAgent and worker agents where: - LeadAgent was passing minimal dict with only id/title/description - FrontendWorkerAgent and TestWorkerAgent expected Task objects Changes: - LeadAgent now uses task.to_dict() for complete task data - FrontendWorkerAgent.execute_task accepts Dict[str, Any] - TestWorkerAgent.execute_task accepts Dict[str, Any] - Updated all tests to use dict input instead of Task objects This fixes AttributeError when workers accessed task.project_id, task.task_number, etc. for git workflow operations.
638714a to
4831820
Compare
PR Review: Standardize Worker Agent Task Dict InterfaceOverall Assessment: ✅ Approve with Minor SuggestionsThis PR correctly fixes an interface mismatch where FrontendWorkerAgent and TestWorkerAgent couldn't access critical task fields (like ✅ Strengths1. Fixes Real Bug
2. Backward Compatibility Maintained
3. Good Test Coverage
|
|
PR review posted: #267 (comment) Summary: Approve with minor suggestions ✅ The fix correctly addresses the interface mismatch - LeadAgent now passes complete task dicts via ✅ Backward compatibility is maintained by creating Task objects internally for ✅ Good test coverage (6 new tests, 467 total passing). Minor suggestions for future cleanup:
The PR is ready to merge. |
1 participant
Summary
project_idandtask_numberDict[str, Any]input, matching the existing pattern in BackendWorkerAgent and ReviewWorkerAgenttask.to_dict()for complete task data instead of manually creating a minimal dictChanges
codeframe/agents/lead_agent.pytask.to_dict()instead of minimal dictcodeframe/agents/frontend_worker_agent.pyDict[str, Any]instead ofTaskcodeframe/agents/test_worker_agent.pyDict[str, Any]instead ofTasktests/agents/test_worker_task_dict_interface.pytests/agents/test_*_worker*.pyTest plan
Summary by CodeRabbit
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.