feat: extend ReactAgent interface to support full runtime parameters (#362) - #367
Conversation
TDD step 1: Write failing tests for verbose, debug, output_logger, on_event, dry_run, and fix_coordinator parameters on ReactAgent. All tests fail with TypeError since the constructor doesn't accept these parameters yet.
Add 6 new constructor params to ReactAgent: dry_run, verbose, on_event, debug, output_logger, fix_coordinator. Implement _verbose_print, _setup_debug_log, _debug_log methods. Enhance _emit to call on_event callback. Add dry_run interception in _execute_tool_with_lint for write tools. All 58 tests pass.
Remove the dry_run+react ValueError guard (now supported). Pass all 6 runtime params (dry_run, verbose, on_event, debug, output_logger, fix_coordinator) to ReactAgent constructor. Update integration test to verify dry_run is forwarded instead of rejected.
WalkthroughAdds runtime observability and control to ReactAgent: constructor now accepts dry_run, verbose, on_event, debug, output_logger, fix_coordinator; runtime wiring passes these through for engine="react"; ReactAgent gains debug/verbose logging, event hooks, dry-run behavior for write tools, and accompanying tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Runtime as Runtime
participant ReactAgent as ReactAgent
participant LLM as LLM
participant Tool as Tool
participant OutputLog as OutputLogger
participant EventPub as on_event
Runtime->>ReactAgent: instantiate(params: dry_run, verbose, debug, output_logger, on_event, ...)
ReactAgent->>LLM: prompt / get action
LLM-->>ReactAgent: action (tool, args)
ReactAgent->>ReactAgent: _verbose_print / _debug_log / _emit("tool.dispatched")
alt tool is write & dry_run == true
ReactAgent-->>Runtime: return ToolResult(stub, skipped=true)
else
ReactAgent->>Tool: execute(tool,args)
Tool-->>ReactAgent: result / error
end
ReactAgent->>LLM: provide tool result (and lint if applicable)
ReactAgent->>OutputLog: write(streamed status) (if provided)
ReactAgent->>EventPub: emit phase/tool events (if provided)
ReactAgent-->>Runtime: final AgentStatus (including failure count)
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 |
Extend
|
Code Review: ReactAgent Runtime Parameters (PR #367)SummaryThis PR successfully implements full runtime parameter support for ReactAgent, achieving feature parity with the plan-based Agent. The implementation is well-structured, thoroughly tested, and follows the project's architectural patterns. ✅ Strengths1. Excellent Test Coverage
2. Clean Architecture
3. Smart Dry-Run Implementation
4. Documentation Quality
🔍 Observations & Suggestions1. Tool Classification Completeness (Minor)Location: _WRITE_TOOLS = {"edit_file", "create_file", "run_command"}
_READ_TOOLS = {"read_file", "list_files", "search_codebase", "run_tests"}Question: Are these tool lists exhaustive? If new tools are added to
Not a blocker, but worth considering for maintainability. 2.
|
- Increment _failure_count on tool errors and gate failures so debug log verbosity ramp works as intended (flagged by coderabbit + claude) - Flip dry-run check to `not in _READ_TOOLS` so unknown tools are blocked by default (fail-safe) - Add sync-maintenance comment on _WRITE_TOOLS/_READ_TOOLS - Add 2 tests: unknown tool blocked in dry-run, failure_count increments
Follow-up Review ✅I see a follow-up commit has been added since my initial review. The implementation continues to look solid and production-ready. Key Strengths Maintained:
All Prior Observations Addressed:The implementation handles the points raised in my previous review well:
Test Results:
Final Recommendation: ✅ APPROVED FOR MERGEThis PR successfully achieves its goal of bringing ReactAgent to full feature parity with the plan-based Agent. The code is well-tested, follows project conventions, and maintains the headless core architecture. Great work on this implementation! 🚀 |
Summary
Implements #362: Extends ReactAgent to accept all 7 runtime parameters that the plan-based Agent supports, enabling full feature parity when using
--engine react.dry_run,verbose,on_event,debug,output_logger,fix_coordinatorparameters to ReactAgent constructor_verbose_print()for stdout + output logger streaming (cf work follow)_setup_debug_log()and_debug_log()for file-based debug logging_emit()withon_eventcallback support (exception-safe)_execute_tool_with_lint()— write tools return stub results, read tools execute normallyfix_coordinatoras accepted for interface compatibility (no-op in ReAct architecture)runtime.pyto pass all parameters whenengine="react"and removed the dry_run+react ValueError guardAcceptance Criteria
dry_run,verbose,on_event,debug,output_logger,event_publisher,fix_coordinatorparametersengine="react"Test Plan
Implementation Notes
event_publisherwas already supported (from feat: phase-based event emission for ReactAgent progress reporting #364) — no changes needed_execute_tool_with_lint) rather than modifyingtools.py, keeping the change containedfix_coordinatoris stored but not used — ReAct loop doesn't have step-based execution that would benefit from global fix coordinationCloses #362
Summary by CodeRabbit
New Features
Improvements
Tests