-
Notifications
You must be signed in to change notification settings - Fork 5
feat: phase-based event emission for ReactAgent progress reporting #364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… model Add AgentPhase class with phase constants (EXPLORING, PLANNING, CREATING, EDITING, TESTING, FIXING, VERIFYING) for ReactAgent progress reporting. Enhance ProgressEvent with three new optional fields: tool_name, file_path, and iteration. Update the computed data property to include these fields for SSE format compatibility.
Add _TOOL_PHASE_MAP for mapping tool names to AgentPhase constants. Add event_publisher param to __init__ and _emit_progress() method. Emit EXPLORING before context loading, PLANNING before system prompt construction, phase-appropriate events before each tool call in the react loop, VERIFYING before gates.run(), and FIXING during verification retry tool calls.
Pass event_publisher from execute_agent() to ReactAgent constructor, enabling phase-based progress events during react engine execution. Update the NOTE comment to remove event_publisher from unsupported list.
Add TestToolPhaseMap tests verifying all 7 tool->phase mappings and unknown tool default behavior. Add TestPhaseEmissionEdgeCases with tests for publisher exceptions not crashing the agent, multiple tool calls in a single iteration, run_command mapping to TESTING, and correct task_id on all events.
ReactAgent now publishes CompletionEvent on success and ErrorEvent on failure to the EventPublisher, then calls complete_task_sync() to send the END_OF_STREAM sentinel. Without this, SSE subscribers would hang until timeout. Also filters existing phase tests to ProgressEvent instances so they don't break when CompletionEvent/ErrorEvent are in the events list.
WalkthroughAdded AgentPhase constants class defining seven progress phases (EXPLORING, PLANNING, CREATING, EDITING, TESTING, FIXING, VERIFYING). Extended ProgressEvent with optional fields: tool_name, file_path, and iteration. Integrated event publishing into ReactAgent to emit progress events throughout the agent's execution lifecycle. Changes
Sequence DiagramsequenceDiagram
actor Client
participant Runtime
participant ReactAgent
participant EventPublisher
participant Tools
Client->>Runtime: execute_agent(task)
Runtime->>ReactAgent: __init__(event_publisher)
ReactAgent->>EventPublisher: emit ProgressEvent(EXPLORING)
ReactAgent->>EventPublisher: emit ProgressEvent(PLANNING)
loop ReAct Loop - Each Iteration
ReactAgent->>Tools: dispatch tool
EventPublisher-->>Client: emit ProgressEvent(phase, tool_name, file_path, iteration)
Tools-->>ReactAgent: result
alt Tool execution failed
ReactAgent->>EventPublisher: emit ProgressEvent(FIXING)
ReactAgent->>Tools: dispatch edit tool
EventPublisher-->>Client: emit ProgressEvent(EDITING, iteration)
end
end
rect rgba(100, 200, 150, 0.5)
ReactAgent->>EventPublisher: emit ProgressEvent(VERIFYING)
ReactAgent->>EventPublisher: emit CompletionEvent(task_id)
end
EventPublisher-->>Client: stream complete
ReactAgent-->>Runtime: return result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Comment |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ✏️ Tip: You can disable in-progress messages and the fortune message in your review settings. Tip You can customize the high-level summary generated by CodeRabbit.Configure the ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
2 similar comments
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ✏️ Tip: You can disable in-progress messages and the fortune message in your review settings. Tip You can customize the high-level summary generated by CodeRabbit.Configure the ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ✏️ Tip: You can disable in-progress messages and the fortune message in your review settings. Tip You can customize the high-level summary generated by CodeRabbit.Configure the ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Add phase-based progress events and terminal stream events to
|
5 similar comments
Add phase-based progress events and terminal stream events to
|
Add phase-based progress events and terminal stream events to
|
Add phase-based progress events and terminal stream events to
|
Add phase-based progress events and terminal stream events to
|
Add phase-based progress events and terminal stream events to
|
Add phase-based progress and terminal event emission to
|
Moves complete_task_sync() into finally blocks in _emit_stream_completion and _emit_stream_error to prevent SSE stream leaks if publish_sync throws.
Code Review: Phase-Based Event Emission for ReactAgentThis PR successfully implements phase-based progress reporting for the ReactAgent, enabling meaningful execution visibility through ✅ Strengths1. Architecture & Design
2. Event Emission Strategy
3. Robustness
4. Test Coverage
🔍 Code Quality Observations1. class AgentPhase:
EXPLORING = "exploring"
PLANNING = "planning"
# ...This works but consider using from enum import StrEnum
class AgentPhase(StrEnum):
EXPLORING = "exploring"
PLANNING = "planning"
# ...Impact: Low (current approach works fine, but enum would provide stricter typing) 2. Tool-to-Phase Mapping Completeness Suggestion: Add a comment or doc noting that unmapped tools default to EXPLORING, or add logging when the default is used. 3. Stream Completion Timing
This is correct, but the 4. Type Annotations def _emit_progress(
self,
phase: str,
*,
message: str | None = None, # ✅ Modern syntax
# ...
)🎯 Alignment with CLAUDE.md Guidelines✅ Core-first architecture: No FastAPI imports in 🔒 Security & PerformanceSecurity: ✅ No security concerns. Event payloads contain task data (file paths, tool names) which is appropriate for authenticated SSE streams. Performance: ✅ Minimal overhead. Events are only published when 📋 Acceptance Criteria Verification
🎉 SummaryThis is production-ready code that successfully closes #349. The implementation is thoughtful, well-tested, and maintains CodeFRAME's architectural principles. The minor suggestions above are for future enhancement and don't block merging. Recommendation: ✅ Approve and merge Great work on the comprehensive test coverage and the critical fix in commit 6 ensuring stream closure via |
Summary
Implements #349: Adds phase-based event emission to the ReactAgent so
cf work followand SSE streaming show meaningful execution progress.AgentPhaseconstants (EXPLORING, PLANNING, CREATING, EDITING, TESTING, FIXING, VERIFYING) mapped to specific tool callsProgressEventmodel withtool_name,file_path, anditerationfieldsProgressEventat each phase transition and tool dispatchCompletionEvent/ErrorEventand closes SSE streams viacomplete_task_sync()EventPublisherfrom runtime into ReactAgent constructorAcceptance Criteria
events.pybefore/after tool callsstreaming.pySSE infrastructurecf work followcommandTest Plan
test_react_agent.py(22 new, 16 original, 4 stream completion)ruff checkclean on all modified filescomplete_task_sync) identified and fixedFiles Changed
codeframe/core/models.pyAgentPhaseconstants,ProgressEventenhancementcodeframe/core/react_agent.pycodeframe/core/runtime.pyevent_publishertoReactAgenttests/core/test_react_agent.pyCloses #349
Summary by CodeRabbit