feat(agents): SDK Migration Phase 3 - Agent Pattern Migration - #34
Conversation
Phase 3 implements the hybrid agent pattern that bridges YAML-defined agents with the Claude Agent SDK's execution model, while preserving CodeFRAME's unique features (maturity levels, project scoping, context management). New Components: - SubagentGenerator: Generates SDK-compatible markdown from YAML definitions - Tool mapping (file_operations -> Read/Write, test_runner -> Bash, etc.) - Maturity-specific capabilities (D1-D4) - Output to .claude/agents/ directory - 37 tests (100% pass) - HybridWorkerAgent: SDK execution with CodeFRAME coordination - execute_task(): Context-aware SDK execution - execute_with_streaming(): Real-time UI updates - Token tracking via MetricsTracker - Session ID management for resume capability - 33 tests (100% pass) Updated Components: - AgentPoolManager: SDK mode support - use_sdk flag for SDK vs traditional mode - _create_hybrid_agent() for SDK agents - Per-agent SDK override capability - Pool status includes is_hybrid and session_id - 29 tests (100% pass) - LeadAgent: SDK coordination - use_sdk and project_root parameters - _sdk_sessions tracking throughout execution - _get_sdk_sessions() for pool-wide session gathering - Session persistence for conversation resume - 10 tests (100% pass) Generated Artifacts: - .claude/agents/*.md: SDK-compatible subagent markdown files Testing: - 109 new tests total, all passing - pytest-asyncio configuration added - Ruff linting clean Phase 3 Tasks Complete: - Task 3.1: SubagentGenerator ✅ - Task 3.2: HybridWorkerAgent ✅ - Task 3.3: AgentPoolManager SDK support ✅ - Task 3.4: LeadAgent SDK coordination ✅
WalkthroughAdds SDK-backed hybrid agents and orchestration: new HybridWorkerAgent, SubagentGenerator, AgentPoolManager branching for SDK vs traditional agents, LeadAgent SDK session tracking and persistence, multiple agent-spec documents, async test support, and extensive tests for hybrid flows and generation. Changes
Sequence Diagram(s)sequenceDiagram
participant LeadAgent
participant AgentPoolManager
participant HybridWorkerAgent
participant SDKClient
participant Database
participant Context
LeadAgent->>AgentPoolManager: create_agent(agent_type, use_sdk=True)
AgentPoolManager->>AgentPoolManager: decide hybrid vs traditional
alt hybrid
AgentPoolManager->>HybridWorkerAgent: instantiate with sdk_client & session_id
AgentPoolManager->>AgentPoolManager: pool[id] = {is_hybrid: true, session_id}
else traditional
AgentPoolManager->>AgentPoolManager: instantiate traditional worker
end
LeadAgent->>HybridWorkerAgent: execute_task(task)
HybridWorkerAgent->>Context: load HOT/WARM context
Context-->>HybridWorkerAgent: context items
HybridWorkerAgent->>HybridWorkerAgent: build prompt
HybridWorkerAgent->>SDKClient: call SDK with prompt
SDKClient-->>HybridWorkerAgent: response (content, usage, session_id)
HybridWorkerAgent->>HybridWorkerAgent: extract changed files, summarize
HybridWorkerAgent->>Database: record token usage & persist result
HybridWorkerAgent->>Context: save result/context items
HybridWorkerAgent-->>LeadAgent: result + session_id
LeadAgent->>LeadAgent: update _sdk_sessions[agent_id] = session_id
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Code Review - PR #34: SDK Migration Phase 3 - Agent Pattern MigrationOverviewThis PR implements Phase 3 of the Claude Agent SDK migration, introducing a hybrid agent pattern that bridges YAML-defined agents with the SDK's execution model. The implementation is well-structured and comprehensive, with 109 new tests and strong documentation. ✅ Strengths1. Excellent Architecture & Design
2. Comprehensive Testing
3. Strong Documentation
4. Code Quality
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
codeframe/agents/lead_agent.py (1)
1063-1091: Remove debug print statements before merge.There are extensive debug print statements throughout the coordination loop that should be removed or converted to proper logging. These will clutter production output.
- print(f"\n🚀 DEBUG: start_multi_agent_execution ENTERED (timeout={timeout})") try: - print("🚀 DEBUG: Creating asyncio.timeout context...") async with asyncio.timeout(timeout): - print("🚀 DEBUG: Inside timeout context, calling _execute_coordination_loop...") return await self._execute_coordination_loop(max_retries, max_concurrent)Similar debug prints appear throughout
_execute_coordination_loop(lines 1079-1282) and_all_tasks_complete(lines 1540-1572).
🧹 Nitpick comments (15)
codeframe/agents/hybrid_worker.py (1)
311-341: Address the hardcoded model name.The token tracking implementation is solid with proper error handling that doesn't fail task execution. However, line 333 hardcodes the model name with a TODO comment.
Apply this diff to use the model name from the SDK client:
- model_name="claude-sonnet-4-20250514", # TODO: Get from SDK client + model_name=getattr(self.sdk_client, "model", "claude-sonnet-4-20250514"),This ensures the recorded model matches the actual SDK client configuration while providing a safe fallback.
claudedocs/SESSION.md (3)
350-353: Fix markdown formatting for table.The table on line 353 should be surrounded by blank lines per markdown conventions (markdownlint MD058).
Add blank lines before and after the table:
### Task Status + | Task | Status | Notes | |------|--------|-------| | 3.1 Subagent Generator | ✅ Complete | 37 tests, 8 agents generated | +
437-437: Fix markdown formatting for Phase Overview table.The table on line 437 should be surrounded by blank lines per markdown conventions (markdownlint MD058).
Add blank lines before and after the table:
### Phase Overview + | Phase | Goal | Agents/Skills | Complexity | Risk | |-------|------|---------------|------------|------| | **1** | Analysis & Planning | `Explore`, `claude-agent-sdk` skill | 4-6 hrs | Low | +
456-456: Fix markdown formatting for Key Deliverables table.The table on line 456 should be surrounded by blank lines per markdown conventions (markdownlint MD058).
Add blank lines before and after the table:
### Key Deliverables + | Task | File | Lines | |------|------|-------| | 3.1 | `codeframe/agents/subagent_generator.py` | ~150 | +tests/agents/test_agent_pool_manager.py (1)
395-421: Verify session_id assertion completeness.The test at line 411-421 verifies
session_idis present in status but doesn't assert its expected value. Consider adding an explicit assertion for the expected value to ensure the field contains the correct data.assert agent_id in status assert "session_id" in status[agent_id] + assert status[agent_id]["session_id"] is None # Traditional agents have no session_id.claude/agents/backend-architect.md (1)
1-45: Consider adding output format section for consistency.This agent specification is functional but less comprehensive than
backend-worker.md. For consistency across agent definitions, consider adding:
- An output format section (JSON structure like other agents)
- Context awareness guidelines
- Integration points section
The tools list intentionally excludes
Write, which makes sense for an architecture review/advisory role.tests/agents/test_subagent_generator.py (1)
538-576: Consider skipping integration test by default.The integration test at line 541-565 depends on the existence of a real definitions directory. While it properly uses
pytest.skipwhen the directory doesn't exist, consider marking it with@pytest.mark.integrationto allow selective test execution.+ @pytest.mark.integration def test_with_real_definitions_dir(self, temp_output_dir):tests/agents/test_hybrid_worker.py (1)
415-429: Consider consolidating redundant session tests.Lines 415-420 set session_id and verify the attribute in a sync test, while lines 422-429 do the same verification in an async test. The sync test (415-420) doesn't provide additional value since it just checks attribute assignment. Consider removing it or consolidating into the async test.
codeframe/agents/agent_pool_manager.py (4)
138-142: Remove or convert debug print statements to logging.Multiple
print()statements throughout this file (Lines 138-142, 151, 154, 163, 196-210, 448-487) appear to be development artifacts. These should either be removed before merging or converted tologger.debug()for consistency with the existing logging pattern.- print( - f"\n🏭 DEBUG: create_agent called with agent_type={agent_type}, use_sdk={create_hybrid}" - ) + logger.debug( + f"create_agent called with agent_type={agent_type}, use_sdk={create_hybrid}" + ) with self.lock: - print("🏭 DEBUG: Acquired lock") + logger.debug("Acquired lock")
434-448: Add return type annotation for consistency.The method lacks a return type annotation. Consider adding a union type or base class return type for better type safety.
- def _create_traditional_agent(self, agent_id: str, agent_type: str): + def _create_traditional_agent( + self, agent_id: str, agent_type: str + ) -> BackendWorkerAgent | FrontendWorkerAgent | TestWorkerAgent | ReviewWorkerAgent:
450-488: Consider unifying agent type normalization.
_create_hybrid_agentusesagent_type.split("-")[0](Line 396) while this method uses explicit compound checks. This inconsistency could lead to maintenance burden. Consider extracting a shared normalization helper.# At class level or module level def _normalize_agent_type(agent_type: str) -> str: """Normalize agent type to base form (e.g., 'backend-worker' -> 'backend').""" return agent_type.split("-")[0]
419-432: Hardcoded maturity level may limit flexibility.The
AgentMaturity.D2default is reasonable, but consider making this configurable viacreate_agent()or pool initialization for workflows that need different maturity levels per agent. Based on learnings, the maturity levels D1-D4 are a key feature of the agent system.codeframe/agents/subagent_generator.py (3)
372-372: Minor type annotation style inconsistency.Line 372 uses
set[str](Python 3.9+ builtin syntax) while the rest of the file usesList[str],Dict[str, Any]from the typing module. Both are valid in Python 3.11+, but consistency is preferred.- sdk_tools: set[str] = set() + sdk_tools: Set[str] = set()And add
Setto the typing imports on Line 51:-from typing import Dict, List, Optional, Any +from typing import Any, Dict, List, Optional, Set
162-179: Good error isolation, consider exposing load failures.The error handling correctly isolates individual file failures. For observability, consider tracking failed loads and exposing them via a property or return value, so callers can detect partial load scenarios.
425-428: Consider thread safety for reload operation.If
SubagentGeneratormay be accessed concurrently,reload_definitions()could cause race conditions with other methods reading_raw_definitions. Consider adding a lock if concurrent access is expected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.claude/agents/backend-architect.md(1 hunks).claude/agents/backend-worker.md(1 hunks).claude/agents/code-reviewer.md(1 hunks).claude/agents/frontend-specialist.md(1 hunks).claude/agents/frontend-worker.md(1 hunks).claude/agents/test-engineer.md(1 hunks).claude/agents/test-worker.md(1 hunks)claudedocs/SESSION.md(1 hunks)codeframe/agents/agent_pool_manager.py(9 hunks)codeframe/agents/hybrid_worker.py(1 hunks)codeframe/agents/lead_agent.py(6 hunks)codeframe/agents/subagent_generator.py(1 hunks)pytest.ini(1 hunks)tests/agents/test_agent_pool_manager.py(1 hunks)tests/agents/test_hybrid_worker.py(1 hunks)tests/agents/test_lead_agent_session.py(1 hunks)tests/agents/test_subagent_generator.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
codeframe/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
codeframe/**/*.py: Use Python 3.11+ with async/await pattern, type hints, and comprehensive tests for backend code
Use ruff for linting Python code
Use async/await pattern for promises in Python async code
Use FastAPI for backend API implementation
Use SQLite with async support (aiosqlite) for database operations
Use WebSocket for real-time updates between backend and frontend
API endpoints should accept project_id query parameter for multi-project support
Files:
codeframe/agents/lead_agent.pycodeframe/agents/agent_pool_manager.pycodeframe/agents/subagent_generator.pycodeframe/agents/hybrid_worker.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Run all tests with pytest
Maintain 88%+ test coverage for Sprint 10 components and 100% pass rate
Files:
tests/agents/test_agent_pool_manager.pytests/agents/test_subagent_generator.pytests/agents/test_lead_agent_session.pytests/agents/test_hybrid_worker.py
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Documentation files must be sized to fit in a single agent context window (spec.md ~200-400 lines, plan.md ~300-600 lines, tasks.md ~400-800 lines)
Files:
claudedocs/SESSION.md
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:37.203Z
Learning: Implement Lead Agent for orchestration and Worker Agents for specialization (Backend, Frontend, Test, Review) with maturity levels D1-D4
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to tests/**/*.py : Run all tests with pytest
Applied to files:
pytest.ini
📚 Learning: 2025-11-25T19:08:54.154Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T19:08:54.154Z
Learning: Applies to {README.md,CODEFRAME_SPEC.md,CHANGELOG.md,SPRINTS.md,CLAUDE.md,AGENTS.md,TESTING.md,CONTRIBUTING.md} : Root-level documentation must include: README.md (project intro), CODEFRAME_SPEC.md (architecture, ~800 lines), CHANGELOG.md (user-facing changes), SPRINTS.md (timeline index), CLAUDE.md (coding standards), AGENTS.md (navigation guide), TESTING.md (test standards), and CONTRIBUTING.md (contribution guidelines)
Applied to files:
.claude/agents/backend-architect.md.claude/agents/backend-worker.md.claude/agents/code-reviewer.md.claude/agents/frontend-worker.md.claude/agents/frontend-specialist.md.claude/agents/test-engineer.md.claude/agents/test-worker.md
📚 Learning: 2025-11-25T19:08:37.203Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:37.203Z
Learning: Implement Lead Agent for orchestration and Worker Agents for specialization (Backend, Frontend, Test, Review) with maturity levels D1-D4
Applied to files:
.claude/agents/backend-architect.md.claude/agents/backend-worker.md.claude/agents/frontend-worker.mdcodeframe/agents/lead_agent.py.claude/agents/test-worker.mdtests/agents/test_lead_agent_session.pyclaudedocs/SESSION.md
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to tests/test_*worker_agent.py : Implement Worker Agent tests for async patterns
Applied to files:
.claude/agents/backend-worker.md.claude/agents/test-worker.mdtests/agents/test_agent_pool_manager.pytests/agents/test_lead_agent_session.pytests/agents/test_hybrid_worker.py
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to codeframe/**/*.py : Use Python 3.11+ with async/await pattern, type hints, and comprehensive tests for backend code
Applied to files:
.claude/agents/backend-worker.md
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to codeframe/agents/worker_agent.py : Block task completion when quality gates fail (test failures, type errors, coverage <85%, critical review issues)
Applied to files:
.claude/agents/backend-worker.md.claude/agents/test-worker.mdtests/agents/test_hybrid_worker.py
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to web-ui/src/**/*.{ts,tsx} : Use TypeScript 5.3+ with React, strict mode, and maintain 85%+ test coverage for frontend code
Applied to files:
.claude/agents/frontend-worker.md.claude/agents/frontend-specialist.md
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to codeframe/agents/worker_agent.py : Each agent maintains independent context scoped by (project_id, agent_id) tuple
Applied to files:
codeframe/agents/lead_agent.pycodeframe/agents/agent_pool_manager.pycodeframe/agents/hybrid_worker.py
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to codeframe/lib/quality_gates.py : Implement quality gates as multi-stage pre-completion checks: tests → type → coverage → review
Applied to files:
tests/agents/test_hybrid_worker.py
🧬 Code graph analysis (3)
tests/agents/test_subagent_generator.py (1)
codeframe/agents/subagent_generator.py (8)
SubagentGenerator(106-428)list_available_types(401-407)get_raw_definition(409-423)reload_definitions(425-428)generate_agent(203-242)generate_all(181-201)_map_tools_to_sdk(363-385)_get_maturity_config(339-361)
tests/agents/test_lead_agent_session.py (3)
codeframe/agents/lead_agent.py (3)
LeadAgent(27-1835)_get_sdk_sessions(1810-1835)on_session_end(1783-1808)codeframe/agents/agent_pool_manager.py (1)
get_agent_status(302-323)codeframe/core/session_manager.py (2)
SessionManager(9-91)load_session(69-83)
tests/agents/test_hybrid_worker.py (3)
codeframe/core/models.py (1)
ContextItemType(285-292)codeframe/agents/worker_agent.py (3)
WorkerAgent(7-442)load_context(163-197)save_context_item(131-161)codeframe/lib/metrics_tracker.py (1)
record_token_usage(130-207)
🪛 markdownlint-cli2 (0.18.1)
.claude/agents/test-worker.md
145-145: Strong style
Expected: underscore; Actual: asterisk
(MD050, strong-style)
145-145: Strong style
Expected: underscore; Actual: asterisk
(MD050, strong-style)
146-146: Strong style
Expected: underscore; Actual: asterisk
(MD050, strong-style)
146-146: Strong style
Expected: underscore; Actual: asterisk
(MD050, strong-style)
147-147: Strong style
Expected: underscore; Actual: asterisk
(MD050, strong-style)
147-147: Strong style
Expected: underscore; Actual: asterisk
(MD050, strong-style)
148-148: Strong style
Expected: underscore; Actual: asterisk
(MD050, strong-style)
148-148: Strong style
Expected: underscore; Actual: asterisk
(MD050, strong-style)
149-149: Strong style
Expected: underscore; Actual: asterisk
(MD050, strong-style)
149-149: Strong style
Expected: underscore; Actual: asterisk
(MD050, strong-style)
150-150: Strong style
Expected: underscore; Actual: asterisk
(MD050, strong-style)
150-150: Strong style
Expected: underscore; Actual: asterisk
(MD050, strong-style)
claudedocs/SESSION.md
350-350: Multiple headings with the same content
(MD024, no-duplicate-heading)
353-353: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
437-437: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
456-456: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🔇 Additional comments (35)
pytest.ini (1)
15-16: LGTM! Proper asyncio configuration for Phase 3 async tests.The
asyncio_mode = autoconfiguration correctly enables pytest-asyncio to automatically detect and execute async test functions introduced in the new test suites (test_hybrid_worker.py, test_lead_agent_session.py, test_subagent_generator.py)..claude/agents/code-reviewer.md (1)
1-54: LGTM! Comprehensive code review specialist documentation.The documentation provides well-structured guidance covering security (OWASP Top 10), performance, architecture, best practices, and technical debt assessment. The D2 maturity level with 3 correction attempts and escalation strategy aligns with the Phase 3 agent pattern migration objectives.
.claude/agents/frontend-specialist.md (1)
1-45: LGTM! Frontend specialist documentation aligns with project standards.The documentation properly covers modern frontend frameworks, accessibility (WCAG), performance optimization (Core Web Vitals), and TypeScript - all consistent with the coding guidelines requiring TypeScript 5.3+ with React and 85%+ frontend test coverage.
.claude/agents/frontend-worker.md (1)
1-109: LGTM! Comprehensive frontend worker agent documentation with excellent detail.The documentation provides thorough guidance on React/TypeScript development with proper emphasis on:
- Accessibility compliance (WCAG 2.1 AA with 4.5:1 contrast ratio)
- Test-driven development with multiple test types (unit, integration, a11y, visual regression)
- Modern React patterns (hooks, Context API, Suspense/lazy, React.memo)
- Strict TypeScript with no implicit any
- Integration with project infrastructure (database, codebase_index, Playwright, WebSocket)
The JSON output format and D2 maturity level with error recovery strategy align well with the Phase 3 hybrid agent architecture.
codeframe/agents/hybrid_worker.py (5)
82-124: LGTM! Proper initialization with comprehensive parameter support.The constructor properly initializes all required components with appropriate type hints and defaults. The
db: Anytyping is acceptable here as it maintains compatibility with the parent WorkerAgent class. Session ID support enables conversation resume capability as intended for Phase 3.
126-208: LGTM! Well-structured async execution with proper error handling.The
execute_taskmethod correctly implements the documented 6-step flow:
- Context loading (HOT + WARM tiers)
- Prompt building with context
- SDK execution with exception handling
- Result persistence to context
- Token usage tracking
- Flash save threshold check
Error handling properly catches exceptions and returns a consistent response structure. Token tracking and session ID propagation support the Phase 3 objectives.
210-309: LGTM! Well-designed helper methods with appropriate guardrails.The helper methods implement sensible strategies:
_build_execution_prompt: Limits warm context to 20 items and truncates long items to 500 chars to prevent prompt bloat_summarize_result: Extracts first paragraph or truncates to 500 chars for efficient context storage_extract_changed_files: Uses regex patterns to extract file paths from various common formatsThese implementations balance completeness with token efficiency as required for SDK integration.
402-416: LGTM! Clean session information getter.The method provides comprehensive session metadata including agent identification, maturity level, SDK session ID, and definition name. This supports the Phase 3 session tracking and coordination requirements.
343-400: Inconsistency is intentional design, not a bug.The different method signatures are correct by design:
send_message([{"role": "user", "content": prompt}])accepts a conversation list to maintain interface compatibility withAnthropicProvider, but internally extracts just the last message's content (line 89 in sdk_client.py)send_message_streaming(prompt)accepts a prompt string directly as a specialized streaming variantBoth methods ultimately call the SDK's
query(prompt=..., options=self._options)function. No changes needed..claude/agents/test-engineer.md (1)
1-47: LGTM! Test engineer documentation aligns with project testing standards.The documentation properly covers test automation with pytest, TDD methodology, and comprehensive test strategies (unit, integration, E2E, performance). The >80% coverage requirement aligns well with the coding guidelines requiring 88%+ test coverage for Sprint 10 components and 100% pass rate.
tests/agents/test_lead_agent_session.py (1)
335-509: LGTM! Comprehensive test coverage for SDK coordination features.The new test class
TestLeadAgentSDKCoordinationprovides excellent coverage of Phase 3 SDK integration:Initialization tests (lines 356-373): Verify
use_sdkandproject_rootparameters are properly storedSession tracking tests (lines 375-465): Comprehensive coverage of
_get_sdk_sessionsincluding:
- Empty state initialization
- Gathering session IDs from hybrid agents
- Filtering out traditional (non-hybrid) agents
- Excluding agents without session_id
- Graceful error handling
Persistence test (lines 466-489): Verifies end-to-end SDK session persistence via
on_session_endIntegration test (lines 491-509): Confirms SDK parameters propagate to AgentPoolManager
All tests use appropriate mocking, clear assertions, and cover edge cases. This aligns with the coding guidelines requiring comprehensive test coverage.
.claude/agents/backend-worker.md (1)
1-80: LGTM! Well-structured agent specification.The Backend Worker documentation comprehensively defines the agent's role, capabilities, output format, and integration points. The maturity level D2 and TDD workflow align with the project's agent architecture patterns. Based on learnings, this follows the expected Worker Agent specialization model.
.claude/agents/test-worker.md (1)
1-150: LGTM! Comprehensive test worker specification.The Test Worker documentation provides excellent guidance on test pyramid strategy (70/20/10 split), TDD workflow, and framework-specific patterns for both Python/Pytest and JavaScript/Jest. The accessibility testing section and mocking strategy align with quality standards.
The static analysis warnings about markdown strong-style (asterisk vs underscore) can be safely ignored - the file uses consistent asterisk style throughout the Integration Points section.
tests/agents/test_agent_pool_manager.py (2)
281-326: LGTM! Good SDK mode initialization tests.The test coverage for SDK mode initialization is comprehensive, verifying:
- Default
use_sdk=Falsebehavior- SDK availability gating when
use_sdk=True- Custom model and working directory propagation
328-392: LGTM! Thorough hybrid agent creation tests.Good coverage of hybrid vs traditional agent creation paths:
- Verifies SDK client and HybridWorkerAgent instantiation when SDK enabled
- Confirms traditional BackendWorkerAgent creation when SDK disabled
- Tests per-call
use_sdkoverride allowing SDK agents in non-SDK poolsThe mocking strategy correctly patches
SDK_AVAILABLE,SDKClientWrapper, andHybridWorkerAgent.tests/agents/test_subagent_generator.py (4)
1-123: LGTM! Well-designed test fixtures.The test fixtures provide realistic YAML definitions that exercise key features:
- Multi-line descriptions
- Maturity progression with D1-D4 levels
- Tool mappings
- Error recovery and integration points
The use of
tempfile.TemporaryDirectoryensures proper test isolation.
130-231: LGTM! Comprehensive definition loading and markdown generation tests.Good coverage of:
- YAML file loading on init and reload
- Graceful handling of missing definitions directory
- KeyError for unknown agent names
- File creation, naming sanitization, and content validation
322-394: LGTM! Thorough tool mapping tests.Excellent coverage of the YAML→SDK tool mapping logic:
- Individual tool mappings (file_operations→Read/Write, codebase_index→Glob/Grep, etc.)
- Deduplication of multiple tools mapping to same SDK tool
- Alphabetical sorting
- Warning logging for unknown tools
- Default tools (Read, Glob) always included
448-530: LGTM! Good edge case coverage.The edge case tests properly handle:
- Empty capabilities and tools lists
- Multiline description truncation in frontmatter
- Special characters in agent names for filename sanitization
tests/agents/test_hybrid_worker.py (5)
23-89: LGTM! Well-designed test fixtures.The mock fixtures properly set up:
- Database with context methods returning appropriate defaults
- SDK client with async
send_messagereturning realistic response structure- Streaming mock using async generator pattern
- Sample task with realistic fields
147-209: LGTM! Thorough task execution tests.Good coverage of:
- Successful execution with status/content/usage verification
- Current task tracking during execution
- Context loading from HOT/WARM tiers
- Context saving with task result
- File extraction from response content
- Graceful SDK error handling returning failed status
The async patterns follow the coding guidelines for Python 3.11+ with async/await.
262-293: LGTM! Important resilience test.The test at line 283-293 verifies that token tracking failures don't break task execution - this is critical for production resilience. The test correctly validates that a DB error during metrics recording still results in a
completedstatus.
509-568: LGTM! Comprehensive integration test.The full execution flow test properly validates:
- Context loading (HOT/WARM tiers)
- SDK client invocation
- Session ID propagation in result
- Metrics tracking and context manager integration
The test correctly uses multiple context managers for mocking.
243-255: The mock integration is correct as-is.The test properly patches
ContextManagerat the module level, and sinceshould_flash_save()(inherited fromWorkerAgent) dynamically imports and instantiatesContextManager(db=self.db)on line 128 ofworker_agent.py, the patched class is correctly intercepted. Themock_context_mgr_class.return_value = mock_context_mgrsetup ensures the instantiation returns the mocked instance, and the assertion verifies the method is called as expected duringexecute_task().codeframe/agents/lead_agent.py (5)
45-58: LGTM! Clean SDK parameter additions.The new
use_sdkandproject_rootparameters are well-documented with appropriate defaults. The docstring updates clearly explain their purpose for SDK execution mode.
80-99: LGTM! Proper SDK state initialization.The SDK mode flag and session tracking are correctly initialized:
use_sdkstored for later reference_project_root_overrideallows custom project root_sdk_sessionsdict for tracking hybrid agent sessionscwdparameter correctly propagated toAgentPoolManager
1373-1377: LGTM! SDK session tracking during task execution.Good implementation capturing session IDs from hybrid agents during task assignment. The defensive
getattrwith defaultNonehandles both hybrid and traditional agents safely.
1790-1806: LGTM! SDK sessions included in session state.The session state correctly includes
sdk_sessionsfor conversation resume capability. The log message helpfully indicates the session count.
1810-1835: LGTM! Robust SDK session gathering.The
_get_sdk_sessionsmethod safely gathers session IDs:
- Only includes hybrid agents with valid session IDs
- Exception handling prevents failures from breaking session save
- Clear logging of gathered session count
codeframe/agents/agent_pool_manager.py (2)
46-54: LGTM! Good defensive pattern for SDK availability.The try/except pattern with a module-level flag provides clean fallback behavior when the SDK client isn't available.
97-100: LGTM! Good defensive API key handling.The fallback to
ANTHROPIC_API_KEYenvironment variable and theuse_sdk and SDK_AVAILABLEguard ensure robust initialization.codeframe/agents/subagent_generator.py (4)
56-94: LGTM! Comprehensive tool mapping.The
YAML_TO_SDK_TOOL_MAPPINGprovides clear abstraction between CodeFRAME's YAML tool names and SDK concrete tools, with sensible defaults for internal-only tools.
268-278: LGTM! Flexible error recovery parsing.The dual-format handling (dict or list of dicts) provides good flexibility for different YAML definition structures while maintaining a sensible default.
381-385: LGTM! Sensible base tool inclusion.Always including
ReadandGlobensures agents have minimum required capabilities regardless of YAML configuration.
1-45: Excellent module documentation.The docstring provides clear format specification, usage examples, and context for the hybrid YAML-to-SDK approach. This significantly aids maintainability.
CRITICAL fixes: - Remove debug print statements from lead_agent.py (lines 1063-1572) - Remove debug print statements from agent_pool_manager.py (lines 138-488) - Fix YAML frontmatter tools format in subagent_generator.py (line 285) * Changed from Python list repr to valid YAML: [Tool1, Tool2] SHOULD_FIX improvements: - Fix hardcoded model name in hybrid_worker.py * Now uses getattr(self.sdk_client, "model", "claude-sonnet-4-20250514") - Add type annotation to _create_traditional_agent method * Returns: Union[BackendWorkerAgent, FrontendWorkerAgent, TestWorkerAgent, ReviewWorkerAgent] - Add assertion for session_id value in test_agent_pool_manager.py * Validates session_id is None for traditional agents - Fix type annotation style inconsistency in subagent_generator.py * Changed set[str] to Set[str] for consistency Additional fixes: - Remove unused variable 'unblocked' in lead_agent.py (ruff F841) Test results: - All 92 tests passing (100% pass rate) - All ruff linting checks passed Addresses CodeRabbit review comments on PR #34
Code Review: SDK Migration Phase 3 - Agent Pattern MigrationSummaryThis PR implements a well-designed hybrid agent pattern that bridges YAML-defined agents with the Claude Agent SDK. The implementation demonstrates strong software engineering practices with comprehensive testing (109 new tests, 100% pass rate), clean architecture, and thoughtful backward compatibility. ✅ StrengthsArchitecture & Design
Code Quality
Security
|
| Metric | Value | Status |
|---|---|---|
| New Tests | 109 | ✅ Excellent |
| Pass Rate | 100% | ✅ Perfect |
| Files Changed | 17 | ✅ Reasonable |
| Lines Added | 3,257 | |
| Lines Deleted | 117 | ✅ Good cleanup |
🏆 Overall Assessment
Grade: A- (Excellent)
This PR represents high-quality software engineering with:
- ✅ Well-designed architecture
- ✅ Comprehensive testing
- ✅ Thoughtful backward compatibility
- ✅ Clear documentation
- ✅ Security best practices
The identified issues are minor and mostly stylistic. The hybrid agent pattern is a clever solution that enables gradual migration while preserving CodeFRAME's unique features.
Recommendation: APPROVE with optional improvements
The code is production-ready. The suggested improvements can be addressed in follow-up PRs without blocking this merge.
Next Steps
After merge, consider:
- Phase 4: Workflow Migration (Task → Plan → Execute coordination)
- Add monitoring/alerting for SDK fallback scenarios
- Performance profiling of context loading in production
- Documentation update for SDK migration guide
Great work on this phase! 🎉
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
codeframe/agents/subagent_generator.py (1)
280-288: LGTM! YAML tools format issue has been addressed.The tools are now formatted as a valid YAML array using
"[" + ", ".join(sdk_tools) + "]", which produces[Read, Write, Bash]format. This addresses the previous review concern about Python list repr being invalid YAML.
🧹 Nitpick comments (4)
tests/agents/test_agent_pool_manager.py (1)
395-422: Consider adding a test for hybrid agent's session_id in status.The tests verify
is_hybridandsession_idfields for traditional agents. Consider adding a test case that verifiessession_idis correctly populated for hybrid agents inget_agent_status():@patch("codeframe.agents.agent_pool_manager.SDK_AVAILABLE", True) @patch("codeframe.agents.agent_pool_manager.SDKClientWrapper") @patch("codeframe.agents.agent_pool_manager.HybridWorkerAgent") def test_get_agent_status_includes_session_id_for_hybrid( self, mock_hybrid_class, mock_sdk_class, mock_db, mock_ws_manager ): """Test get_agent_status includes session_id for hybrid agents.""" mock_sdk_class.return_value = Mock() mock_hybrid_class.return_value = Mock(session_id="sdk-session-123") manager = AgentPoolManager( project_id=1, db=mock_db, ws_manager=mock_ws_manager, use_sdk=True ) agent_id = manager.create_agent("backend") status = manager.get_agent_status() assert status[agent_id]["session_id"] == "sdk-session-123"codeframe/agents/lead_agent.py (1)
80-99:_project_root_overrideappears unused.Line 82 stores
project_rootasself._project_root_override, but this attribute is never referenced elsewhere in the class. Theproject_rootvalue is already passed toAgentPoolManagerviacwd=project_rootat line 93.Either remove the unused attribute or document its intended purpose:
# Store SDK mode flag self.use_sdk = use_sdk - self._project_root_override = project_root # Multi-agent coordination (Sprint 4) with SDK support (Phase 3)codeframe/agents/subagent_generator.py (1)
234-236: Consider more robust filename sanitization.The current sanitization only handles spaces and underscores. Agent names with special characters (e.g.,
Backend/APIorTest@Unit) could produce invalid filenames:# Generate filename (sanitize agent name) - safe_name = agent_name.lower().replace(" ", "-").replace("_", "-") + import re + safe_name = re.sub(r'[^a-z0-9-]', '-', agent_name.lower()) + safe_name = re.sub(r'-+', '-', safe_name).strip('-') # Collapse multiple dashes output_path = self.output_dir / f"{safe_name}.md"codeframe/agents/hybrid_worker.py (1)
228-234: Consider making context limits configurable.The hard-coded limits (20 warm context items, 500 char truncation) work well but could be class constants or constructor parameters for flexibility:
class HybridWorkerAgent(WorkerAgent): MAX_WARM_CONTEXT_ITEMS = 20 MAX_CONTEXT_ITEM_LENGTH = 500 # ...This is a minor improvement for maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
codeframe/agents/agent_pool_manager.py(9 hunks)codeframe/agents/hybrid_worker.py(1 hunks)codeframe/agents/lead_agent.py(7 hunks)codeframe/agents/subagent_generator.py(1 hunks)tests/agents/test_agent_pool_manager.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Run all tests with pytest
Maintain 88%+ test coverage for Sprint 10 components and 100% pass rate
Files:
tests/agents/test_agent_pool_manager.py
codeframe/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
codeframe/**/*.py: Use Python 3.11+ with async/await pattern, type hints, and comprehensive tests for backend code
Use ruff for linting Python code
Use async/await pattern for promises in Python async code
Use FastAPI for backend API implementation
Use SQLite with async support (aiosqlite) for database operations
Use WebSocket for real-time updates between backend and frontend
API endpoints should accept project_id query parameter for multi-project support
Files:
codeframe/agents/hybrid_worker.pycodeframe/agents/agent_pool_manager.pycodeframe/agents/lead_agent.pycodeframe/agents/subagent_generator.py
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:37.203Z
Learning: Implement Lead Agent for orchestration and Worker Agents for specialization (Backend, Frontend, Test, Review) with maturity levels D1-D4
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to tests/test_*worker_agent.py : Implement Worker Agent tests for async patterns
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to tests/test_*worker_agent.py : Implement Worker Agent tests for async patterns
Applied to files:
tests/agents/test_agent_pool_manager.pycodeframe/agents/hybrid_worker.py
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to codeframe/agents/worker_agent.py : Each agent maintains independent context scoped by (project_id, agent_id) tuple
Applied to files:
codeframe/agents/hybrid_worker.pycodeframe/agents/agent_pool_manager.pycodeframe/agents/lead_agent.py
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to codeframe/agents/worker_agent.py : Implement flash save mechanism to archive COLD tier items when context approaches token limit (80% of 180k = 144k tokens)
Applied to files:
codeframe/agents/hybrid_worker.py
📚 Learning: 2025-11-25T19:08:37.203Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:37.203Z
Learning: Implement Lead Agent for orchestration and Worker Agents for specialization (Backend, Frontend, Test, Review) with maturity levels D1-D4
Applied to files:
codeframe/agents/lead_agent.py
🧬 Code graph analysis (2)
tests/agents/test_agent_pool_manager.py (1)
codeframe/agents/agent_pool_manager.py (3)
AgentPoolManager(56-473)create_agent(120-175)get_agent_status(288-309)
codeframe/agents/agent_pool_manager.py (4)
codeframe/agents/hybrid_worker.py (1)
HybridWorkerAgent(57-416)codeframe/core/models.py (1)
AgentMaturity(21-27)codeframe/providers/sdk_client.py (1)
SDKClientWrapper(25-125)codeframe/agents/definition_loader.py (1)
create_agent(299-331)
⏰ 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: Backend Unit Tests
- GitHub Check: Frontend Unit Tests
- GitHub Check: claude-review
🔇 Additional comments (14)
tests/agents/test_agent_pool_manager.py (2)
288-306: LGTM! SDK mode initialization tests are comprehensive.Tests correctly verify that
use_sdkdefaults toFalseand is gated bySDK_AVAILABLE. The test at line 299 importsSDK_AVAILABLEfrom the module under test to validate the conjunction logic.
328-392: LGTM! Hybrid agent creation tests cover key scenarios.Tests correctly validate:
- Hybrid agent creation when
use_sdk=Truewith SDK available- Traditional agent creation when
use_sdk=False- Per-agent
use_sdkoverride functionalityThe mock setup properly provides
session_idattribute for hybrid agents.codeframe/agents/lead_agent.py (3)
1325-1329: LGTM! SDK session tracking during task execution.The session tracking correctly extracts
session_idfrom hybrid agents and stores it for later persistence. The use ofgetattrwithNonefallback safely handles traditional agents.
1733-1749: LGTM! Session persistence with SDK sessions.The
on_session_endmethod correctly gathers SDK sessions and includes them in the persisted state for resume capability.
1753-1778: LGTM! Robust SDK session gathering.The
_get_sdk_sessionsmethod correctly:
- Filters for hybrid agents with valid session IDs
- Uses defensive exception handling to prevent failures from breaking session persistence
- Logs warnings for debugging while gracefully degrading
codeframe/agents/agent_pool_manager.py (4)
46-53: LGTM! SDK availability check with graceful fallback.The try/except pattern for optional SDK dependency is appropriate, with a warning log for debugging when SDK is unavailable.
97-100: LGTM! SDK mode initialization with sensible defaults.The initialization correctly:
- Falls back to environment variable for
api_key- Gates
use_sdkon actual SDK availability- Defaults
cwdto current working directory
135-156: Verify silent fallback behavior for per-calluse_sdk=Truewhen SDK unavailable.When a caller passes
use_sdk=Truetocreate_agent()butSDK_AVAILABLEisFalse, the code silently creates a traditional agent instead. Consider whether a warning log would help users understand why their override didn't take effect:# Determine whether to use SDK for this agent create_hybrid = use_sdk if use_sdk is not None else self.use_sdk + # Warn if SDK was requested but unavailable + if create_hybrid and not SDK_AVAILABLE: + logger.warning( + f"SDK requested for agent {agent_type} but SDK unavailable - " + "falling back to traditional agent" + ) + with self.lock:
362-418: LGTM! Hybrid agent creation with appropriate configurations.The method correctly:
- Maps agent types to specialized system prompts
- Assigns appropriate SDK tool sets per agent type
- Normalizes agent type names for consistent lookup
- Defaults to D2 maturity (coaching) which suits transitional agents
codeframe/agents/subagent_generator.py (2)
365-387: LGTM! Tool mapping with sensible defaults.The
_map_tools_to_sdkmethod correctly:
- Maps YAML tool names to SDK equivalents
- Deduplicates using a set
- Always includes
ReadandGlobfor basic agent functionality- Logs warnings for unmapped tools without failing
138-160: LGTM! Flexible definition loading with proper YAML parsing.The loading logic correctly:
- Handles both
.yamland.ymlextensions- Supports a
customsubdirectory for extensibility- Uses
yaml.safe_loadto prevent code execution vulnerabilitiescodeframe/agents/hybrid_worker.py (3)
82-124: LGTM! Clean initialization extending WorkerAgent.The initialization properly:
- Calls parent constructor with required parameters
- Stores SDK-specific attributes (
sdk_client,session_id)- Leaves
definitionasNonefor AgentFactory to populate if using YAML definitions- Logs initialization state for debugging
126-208: LGTM! Comprehensive task execution with proper error handling.The
execute_taskmethod correctly:
- Loads context from tiered memory (HOT + WARM)
- Builds contextual prompt
- Handles SDK errors gracefully, returning failure status
- Persists result summary to context
- Records token metrics (with non-blocking error handling)
- Triggers flash save when threshold exceeded
Good defensive coding throughout.
284-309: LGTM! Heuristic file extraction with reasonable patterns.The regex patterns cover common phrases used when describing file modifications. The approach appropriately:
- Uses multiple patterns for broader coverage
- Deduplicates with a set
- Returns sorted results for consistency
As a heuristic, it may have edge cases but is a pragmatic solution.
| async def execute_with_streaming(self, task: Task): | ||
| """Execute task with streaming response. | ||
|
|
||
| Similar to execute_task but yields intermediate results for | ||
| real-time UI updates. This is an async generator. | ||
|
|
||
| Args: | ||
| task: Task to execute | ||
|
|
||
| Yields: | ||
| dict with keys: | ||
| - status: "streaming" for intermediate, "completed" for final | ||
| - content_chunk: Partial content (for streaming chunks) | ||
| - content: Full content (for final result) | ||
| - task_id: Task ID | ||
| - files_changed: List of modified files (final only) | ||
| - session_id: SDK session ID (final only) | ||
| """ | ||
| self.current_task = task | ||
|
|
||
| # Load context | ||
| hot_context = await self.load_context(tier=ContextTier.HOT) | ||
| warm_context = await self.load_context(tier=ContextTier.WARM) | ||
|
|
||
| # Build prompt | ||
| prompt = self._build_execution_prompt(task, hot_context, warm_context) | ||
|
|
||
| # Stream execution | ||
| full_content = [] | ||
|
|
||
| async for message in self.sdk_client.send_message_streaming(prompt): | ||
| if hasattr(message, "content"): | ||
| content_chunk = str(message.content) | ||
| full_content.append(content_chunk) | ||
| yield { | ||
| "status": "streaming", | ||
| "content_chunk": content_chunk, | ||
| "task_id": task.id, | ||
| } | ||
|
|
||
| # Combine and yield final result | ||
| final_content = "".join(full_content) | ||
|
|
||
| # Save to context | ||
| result_summary = self._summarize_result(final_content) | ||
| await self.save_context_item( | ||
| item_type=ContextItemType.TASK, | ||
| content=f"Task {task.id} result: {result_summary}", | ||
| ) | ||
|
|
||
| # Yield final result (async generators can't use return with value) | ||
| yield { | ||
| "status": "completed", | ||
| "content": final_content, | ||
| "output": final_content, | ||
| "files_changed": self._extract_changed_files(final_content), | ||
| "session_id": self.session_id, | ||
| } |
There was a problem hiding this comment.
Missing error handling and token tracking in streaming execution.
Unlike execute_task, the execute_with_streaming method lacks:
- Try/except around SDK call for error handling
- Token usage recording via
_record_token_usage - Flash save threshold check
Consider adding parity with execute_task:
+ try:
async for message in self.sdk_client.send_message_streaming(prompt):
if hasattr(message, "content"):
content_chunk = str(message.content)
full_content.append(content_chunk)
yield {
"status": "streaming",
"content_chunk": content_chunk,
"task_id": task.id,
}
+ except Exception as e:
+ logger.error(f"Streaming execution failed for task {task.id}: {e}")
+ yield {
+ "status": "failed",
+ "content": str(e),
+ "task_id": task.id,
+ }
+ returnCommittable suggestion skipped: line range outside the PR's diff.
Add codebase_index parameter to AgentPoolManager.__init__ and pass it to BackendWorkerAgent instead of None. This fixes AttributeError when BackendWorkerAgent.gather_context() calls codebase_index.search_pattern().
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
codeframe/agents/agent_pool_manager.py (2)
365-421: Hybrid agent construction is solid; consider centralizing prompts/tool setsThe normalization of
agent_typeto abase_type, the tailoredsystem_promptsandtool_setsper role, and the encapsulation of SDK wiring viaSDKClientWrapper+HybridWorkerAgentare all clean and composable.To avoid drift with the YAML/SubagentGenerator definitions, consider pulling
system_promptsandtool_setsfrom a shared config or helper module used both here and by the generator, so backend/frontend/test/review capabilities stay in sync over time.
423-470: Traditional agent mapping looks correct; optionally fail fast on missingcodebase_indexMapping:
- backend/backend-worker →
BackendWorkerAgent(project_id, db, codebase_index=self.codebase_index, ...)- frontend/frontend-specialist →
FrontendWorkerAgent(...)- test/test-engineer →
TestWorkerAgent(...)- review/review-worker →
ReviewWorkerAgent(...)correctly aligns with the summarized constructor signatures, and passing
self.codebase_indexnow addresses the earlier bug whereNonewas always passed toBackendWorkerAgent. This is a good fix relative to the past review comment.If
BackendWorkerAgent.gather_context()still assumes a non-Nonecodebase_index, you might want to fail fast when it’s missing to avoid a laterAttributeError:- if agent_type == "backend" or agent_type == "backend-worker": - return BackendWorkerAgent( + if agent_type == "backend" or agent_type == "backend-worker": + if self.codebase_index is None: + raise RuntimeError( + "BackendWorkerAgent requires a non-None codebase_index; " + "pass a CodebaseIndex into AgentPoolManager(codebase_index=...)." + ) + return BackendWorkerAgent( project_id=self.project_id, db=self.db, codebase_index=self.codebase_index, provider="anthropic", api_key=self.api_key, )This keeps misconfiguration failures localized to agent creation instead of surfacing deep in backend processing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
codeframe/agents/agent_pool_manager.py(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
codeframe/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
codeframe/**/*.py: Use Python 3.11+ with async/await pattern, type hints, and comprehensive tests for backend code
Use ruff for linting Python code
Use async/await pattern for promises in Python async code
Use FastAPI for backend API implementation
Use SQLite with async support (aiosqlite) for database operations
Use WebSocket for real-time updates between backend and frontend
API endpoints should accept project_id query parameter for multi-project support
Files:
codeframe/agents/agent_pool_manager.py
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:37.203Z
Learning: Implement Lead Agent for orchestration and Worker Agents for specialization (Backend, Frontend, Test, Review) with maturity levels D1-D4
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to tests/test_*worker_agent.py : Implement Worker Agent tests for async patterns
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to codeframe/agents/worker_agent.py : Each agent maintains independent context scoped by (project_id, agent_id) tuple
Applied to files:
codeframe/agents/agent_pool_manager.py
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to codeframe/agents/worker_agent.py : Block task completion when quality gates fail (test failures, type errors, coverage <85%, critical review issues)
Applied to files:
codeframe/agents/agent_pool_manager.py
📚 Learning: 2025-11-25T19:08:37.203Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:37.203Z
Learning: Implement Lead Agent for orchestration and Worker Agents for specialization (Backend, Frontend, Test, Review) with maturity levels D1-D4
Applied to files:
codeframe/agents/agent_pool_manager.py
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to codeframe/agents/review_agent.py : Review Agent analysis performance target: <30s per file
Applied to files:
codeframe/agents/agent_pool_manager.py
🧬 Code graph analysis (1)
codeframe/agents/agent_pool_manager.py (7)
codeframe/agents/backend_worker_agent.py (1)
BackendWorkerAgent(35-1329)codeframe/agents/frontend_worker_agent.py (1)
FrontendWorkerAgent(22-866)codeframe/agents/test_worker_agent.py (1)
TestWorkerAgent(25-1023)codeframe/agents/review_worker_agent.py (1)
ReviewWorkerAgent(21-332)codeframe/agents/hybrid_worker.py (1)
HybridWorkerAgent(57-416)codeframe/core/models.py (1)
AgentMaturity(21-27)codeframe/providers/sdk_client.py (1)
SDKClientWrapper(25-125)
⏰ 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: Frontend Unit Tests
- GitHub Check: Backend Unit Tests
- GitHub Check: claude-review
🔇 Additional comments (3)
codeframe/agents/agent_pool_manager.py (3)
69-104: SDK feature flagging and initialization look sound
use_sdkbeing ANDed withSDK_AVAILABLE, the env-basedapi_keydefault, and the explicit warning when SDK is requested but unavailable give a clear, safe initialization story. The logging of the effectiveuse_sdkmode is also helpful for debugging pool configuration.Also applies to: 114-121
123-160: Hybrid vs traditional branching and pool metadata are well-structuredPer-call
use_sdkoverride (create_hybrid) layered on top of the pool-level default, the guarded_create_hybrid_agentcall (create_hybrid and SDK_AVAILABLE), and theis_hybridflag stored inagent_poolcollectively give a clean, predictable decision path. The split into_create_hybrid_agent/_create_traditional_agentalso improves readability and future extensibility.Also applies to: 169-171, 173-178
169-171: Makesession_idin status reflect the live agent instanceRight now
session_idis snapshotted once at creation time and then read back from the dict inget_agent_status. ForHybridWorkerAgent,session_idis expected to be updated after SDK calls, so status will likely return a stale value (oftenNone).Use the live instance attribute when building the status payload so callers see the current session:
@@ - for agent_id, agent_info in self.agent_pool.items(): - status[agent_id] = { + for agent_id, agent_info in self.agent_pool.items(): + instance = agent_info.get("instance") + status[agent_id] = { @@ - "blocked_by": agent_info.get("blocked_by"), - "is_hybrid": agent_info.get("is_hybrid", False), - "session_id": agent_info.get("session_id"), + "blocked_by": agent_info.get("blocked_by"), + "is_hybrid": agent_info.get("is_hybrid", False), + "session_id": getattr(instance, "session_id", agent_info.get("session_id")), }You can keep the stored
session_idin the pool for now for backwards compatibility; this change only affects the reported status.Also applies to: 301-310
⛔ Skipped due to learnings
Learnt from: CR Repo: frankbria/codeframe PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-25T19:08:20.036Z Learning: Applies to codeframe/agents/worker_agent.py : Each agent maintains independent context scoped by (project_id, agent_id) tuple
1 participant
Summary
Phase 3 of the Claude Agent SDK Migration implements the hybrid agent pattern that bridges YAML-defined agents with the SDK's execution model while preserving CodeFRAME's unique features.
New Components
codeframe/agents/subagent_generator.py): Generates SDK-compatible markdown from YAML agent definitions with tool mapping and maturity-specific capabilitiescodeframe/agents/hybrid_worker.py): SDK execution with CodeFRAME coordination, context management, and token tracking.claude/agents/*.md): 7 SDK-compatible markdown files for backend, frontend, test, and review agentsUpdated Components
use_sdkfeature flag, hybrid agent creation, per-agent SDK overrideKey Features
use_sdk=Falseby default)Test plan
Files Changed
New Files (11):
codeframe/agents/subagent_generator.py(380+ lines)codeframe/agents/hybrid_worker.py(420 lines)tests/agents/test_subagent_generator.pytests/agents/test_hybrid_worker.py.claude/agents/backend-worker.md.claude/agents/backend-architect.md.claude/agents/frontend-worker.md.claude/agents/frontend-specialist.md.claude/agents/test-worker.md.claude/agents/test-engineer.md.claude/agents/code-reviewer.mdModified Files (6):
codeframe/agents/agent_pool_manager.py(+180 lines)codeframe/agents/lead_agent.py(+55 lines)tests/agents/test_agent_pool_manager.py(+141 lines)tests/agents/test_lead_agent_session.py(+183 lines)pytest.ini(+3 lines - asyncio config)claudedocs/SESSION.md(session documentation)Related Issues
Part of SDK Migration Implementation Plan (Phase 3 of 5)
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.