-
Notifications
You must be signed in to change notification settings - Fork 2k
Open
Labels
core[Component] This issue is related to the core interface and implementation[Component] This issue is related to the core interface and implementation
Description
Code duplication in callback system
Problem Description
The callback system has significant code duplication that makes maintenance difficult:
1. Duplicate canonical_*_callbacks
Methods
Six identical property methods across BaseAgent
and LlmAgent
(42 lines total):
canonical_before_agent_callbacks
canonical_after_agent_callbacks
canonical_before_model_callbacks
canonical_after_model_callbacks
canonical_before_tool_callbacks
canonical_after_tool_callbacks
Each method implements the same logic:
@property
def canonical_XXX_callbacks(self):
if not self.XXX_callback: return []
if isinstance(self.XXX_callback, list): return self.XXX_callback
return [self.XXX_callback]
2. Repeated Callback Execution Logic
Similar callback execution patterns repeated in ~100 lines across 4 locations:
base_agent.py
:__handle_before_agent_callback
,__handle_after_agent_callback
base_llm_flow.py
:_handle_before_model_callback
,_handle_after_model_callback
functions.py
: Tool callback execution (2 places)
Impact
- High maintenance burden: Any change to callback logic requires updating 6+ locations
- Risk of bugs: Easy to introduce inconsistencies when modifying callback behavior
- Poor developer experience: New contributors find it hard to understand the callback system
- Technical debt: 60+ TODO comments in codebase related to callback workarounds
Proposed Solution
Introduce a unified callback pipeline system with three core components:
CallbackPipeline[TInput, TOutput]
- Generic, type-safe callback executornormalize_callbacks()
- Single helper function to replace 6 duplicate methodsCallbackExecutor
- Unified integration of plugin + agent callbacks
Benefits
- Eliminate duplication: 42 lines → 6 lines (85% reduction)
- Single source of truth: All callback logic in one place
- Type safety: Generics instead of Union types
- Easy to extend: New callback types require minimal code
- Backward compatible: Zero breaking changes
Implementation Plan
See PR #[will-be-filled] for:
- Detailed design documentation
- Complete implementation with tests (24/24 passing)
- Phase-by-phase refactoring plan
Priority
Medium-High: This is technical debt cleanup that improves maintainability but doesn't block new features.
Related
- Related to general code quality improvements
- Addresses callback-related TODO comments throughout codebase
Metadata
Metadata
Assignees
Labels
core[Component] This issue is related to the core interface and implementation[Component] This issue is related to the core interface and implementation