refactor(agent): convert printer to plugin pattern - #45
Conversation
|
/strands |
|
Issue: PR template has a "Documentation PR" section that should be included. Suggestion: Add a "Documentation PR" section with a brief justification. Since this is an internal refactor with no public API changes, a simple note like "Not required - internal refactor with no public API changes" would suffice. |
There was a problem hiding this comment.
Assessment: Comment
Clean refactor that correctly converts the printer to the Plugin pattern, improving architectural consistency.
Review Summary
- Implementation: AgentPrinter properly implements Plugin interface with
namegetter andinitAgent()method. Good cleanup removing the unusedPrinterinterface. - Testing: Author has already identified the need to use vitest mocking instead of manually modifying
process.stdout.write(lines 486, 508) - agree this should be addressed for consistency with repo patterns. - Documentation: PR description is missing the "Documentation PR" section from the template. Since this is an internal refactor with no public API changes, a brief justification note would be sufficient.
The Plugin implementation follows established patterns well. Once the testing improvements noted by the author are addressed, this looks good to merge.
|
/strands merge in main and fix the conflicts |
|
/strands - there's a bunch of changes that are not related to this PR; can you identify what happened and reverse the changes |
- Remove Printer interface from printer.ts - AgentPrinter now implements Plugin interface with: - name property: 'strands:printer' - initAgent method that registers callbacks via agent.addHook() - Remove _printer field from Agent class - Register printer as a plugin in PluginRegistry when enabled - Remove manual processEvent calls (handled by hooks system) - Update tests to use plugin pattern with vi.spyOn for stdout capture Resolves #44
cde9501 to
35f4297
Compare
Summary
Refactors
AgentPrinterfrom a standalone interface-based implementation to a first-class Plugin. This aligns with the existing plugin architecture and enables printer functionality to be cleanly integrated into the agent's hook system.Key Changes
printer.ts
Printerinterface - No longer needed with plugin-based approachAgentPrinternow implementsPlugininterface with:nameproperty:'strands:printer'initAgent()method that registers callbacks viaagent.addHook()ModelStreamUpdateEvent→ handles streaming text/reasoning outputToolResultEvent→ handles tool completion statusagent.ts
_printerfield - No longer managed separatelyPluginRegistrywhen enabledprocessEvent()calls - Now handled automatically by hook systemTest Updates
(agent as any)._printeraccess with proper plugin injectionvi.spyOn(process.stdout, 'write')for stdout capture testsWhy This Approach
Resolves #44