-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Explore manual summarization #3137
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds a manual conversation summarization feature through a new /compact command. The feature allows users to explicitly trigger summarization of their conversation history to reduce context size, similar to the automatic summarization that occurs during agentic workflows.
Changes:
- Implements a new
CompactIntentthat handles the/compactslash command for manual conversation summarization - Integrates the summarization logic from the existing agentic summarization system by reusing
SummarizedConversationHistoryPropsBuilderand related prompts - Registers the command as part of the agent participant in VS Code's chat interface
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/extension/intents/node/compactIntent.ts | New intent implementation that handles manual conversation summarization, reusing existing summarization infrastructure |
| src/extension/intents/node/allIntents.ts | Registers the CompactIntent in the intent registry |
| src/extension/common/constants.ts | Adds Compact intent constant and maps it to the Agent participant's commands |
| package.nls.json | Adds localized description for the compact command |
| package.json | Registers the compact command under the agent participant's available commands |
| return response.value; | ||
| } | ||
|
|
||
| private applySummaryToHistory(turns: readonly import('../../prompt/common/conversation').Turn[], summary: string, toolCallRoundId: string): void { |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import type annotation here is unusual and appears to be auto-generated or incomplete. The type should be imported at the top of the file along with other types from the conversation module. This inline import makes the code less readable and deviates from standard TypeScript practices.
| private applySummaryToHistory(turns: readonly import('../../prompt/common/conversation').Turn[], summary: string, toolCallRoundId: string): void { | ||
| for (const turn of [...turns].reverse()) { | ||
| const round = turn.rounds.find(r => r.id === toolCallRoundId); | ||
| if (round) { | ||
| round.summary = summary; | ||
| return; | ||
| } | ||
| } | ||
| } |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The applySummaryToHistory method mutates the rounds in the turns array, but the turns parameter is typed as readonly. While the implementation creates a new array with [...turns].reverse(), it still mutates the round.summary property of objects in the original turns array. This violates the readonly contract and could lead to unexpected side effects. Consider either removing the readonly modifier from the parameter type or creating deep copies of the turns to avoid mutation.
| for (const turn of [...turns].reverse()) { | ||
| const round = turn.rounds.find(r => r.id === toolCallRoundId); | ||
| if (round) { | ||
| round.summary = summary; | ||
| return; | ||
| } | ||
| } |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the target round is not found (e.g., if toolCallRoundId doesn't match any round in the history), the summary is silently discarded without any logging or error handling. This could make debugging difficult if the summarization appears to fail silently. Consider adding a log warning when the round is not found to help with troubleshooting.
| for (const turn of [...turns].reverse()) { | |
| const round = turn.rounds.find(r => r.id === toolCallRoundId); | |
| if (round) { | |
| round.summary = summary; | |
| return; | |
| } | |
| } | |
| let found = false; | |
| for (const turn of [...turns].reverse()) { | |
| const round = turn.rounds.find(r => r.id === toolCallRoundId); | |
| if (round) { | |
| round.summary = summary; | |
| found = true; | |
| break; | |
| } | |
| } | |
| if (!found) { | |
| this.logService.warn(`Failed to apply conversation summary: no round found with id ${toolCallRoundId}.`); | |
| } |
|
This looks like a separate implementation of summarization? Let's trigger the existing one |
|
Maybe you can get it to reuse more of The other idea I had was to go through the normal agent loop but trigger summarization here:
|
Ideally we want users to be able to trigger summarization themselves whenever they want to keep context down. This explores adding that feature. We can then add this to the context window widget.
@roblourens as the summarization expert I would appreciate your thoughts on this approach as I ideally would like this to be as close to the agentic summarization as possible and reuse as much code as possible