feat: Add SummarizingConversationManager#524
feat: Add SummarizingConversationManager#524jsamuel1 wants to merge 1 commit intostrands-agents:mainfrom
Conversation
Implements summarizing conversation manager that preserves context by summarizing older messages instead of simply trimming them. Features: - Configurable summary ratio (0.1-0.8, default 0.3) - Configurable recent message preservation (default 10) - Two summarization paths: direct model call or dedicated agent - Tool pair boundary detection to maintain conversation validity - Async hook integration with AfterModelCallEvent Closes strands-agents#279
github-actions
bot
added
the
strands-running
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| private readonly _preserveRecentMessages: number | ||
| private readonly _summarizationAgent?: Agent | ||
| private readonly _summarizationSystemPrompt?: string | ||
| private _summaryMessage?: Message |
There was a problem hiding this comment.
Issue: Unused private field
_summaryMessage is stored but never read or exposed. The Python SDK uses this field for session persistence via restore_from_session() and get_state() methods.
Suggestion: Either remove this field if session persistence isn't needed, or implement the session persistence methods to maintain API parity with the Python SDK.
| * | ||
| * @throws ContextWindowOverflowError If no valid split point can be found. | ||
| */ | ||
| private adjustSplitPointForToolPairs(messages: Message[], splitPoint: number): number { |
There was a problem hiding this comment.
Issue: Duplicated logic with SlidingWindowConversationManager
This tool pair boundary detection logic is nearly identical to the logic in SlidingWindowConversationManager.reduceContext() (lines 131-159). Consider extracting this to a shared utility function in a common location (e.g., a utils.ts file in this directory).
Suggestion: Create a shared function like findValidSplitPoint(messages: Message[], initialSplitPoint: number): number that both conversation managers can use.
| try { | ||
| summarizationAgent.messages.splice(0, summarizationAgent.messages.length, ...messages) | ||
| const result = await summarizationAgent.invoke('Please summarize this conversation.') | ||
| return new Message({ |
There was a problem hiding this comment.
Issue: Summary message role may be confusing
The summary is returned as a user role message, which may be unexpected since it's system-generated content. Consider documenting this design decision or using a different approach.
The Python SDK does the same thing ({"role": "user"}), so this is consistent, but it might be worth adding a comment explaining why the summary is injected as a user message.
| @@ -0,0 +1,198 @@ | |||
| { | |||
There was a problem hiding this comment.
Issue: Unrelated file included in PR
This LSP configuration file is unrelated to the SummarizingConversationManager feature and should be removed from this PR. It appears to be IDE-specific configuration that was accidentally committed.
|
Assessment: Request Changes This PR introduces a well-implemented SummarizingConversationManager that closely follows the Python SDK design. The core functionality is solid with good test coverage. Review Categories
The implementation is well-structured and the tests are comprehensive. Good work aligning with the Python SDK design! |
github-actions
bot
removed
the
strands-running
|
@jsamuel1 Love to get this. Any update? |
2 participants
Implements summarizing conversation manager that preserves context by summarizing older messages instead of simply trimming them.
Features
Testing
Closes #279