Prevent repeated foreground summarization after failure#4953
Merged
Conversation
When foreground summarization fails (e.g. summary too large), the outcome metadata is recorded on the turn. On subsequent BudgetExceededErrors within the same turn, renderWithSummarization checks for this metadata and skips the expensive LLM summarization call, falling back directly to a no-cache-breakpoints render. This prevents the user from seeing repeated 'Compacting...' progress indicators and avoids wasting LLM calls on summarization that already failed. - Add retry guard in renderWithSummarization checking turn metadata - Record failure metadata (source: foreground, outcome: errorKind) - Add 4 tests covering failure metadata, success metadata, round.summary not set on failure, and retry guard contract
Contributor
There was a problem hiding this comment.
Pull request overview
Prevents repeated (and likely futile) foreground conversation summarization attempts within the same turn after an initial summarization failure, reducing redundant LLM calls and repeated “Compacting conversation...” UX.
Changes:
- Add an early retry guard in
renderWithSummarizationto skip foreground summarization if the latest turn already recorded a prior foreground failure. - Fall back directly to a non-cache-breakpoints render when the guard triggers.
- Add new summarization-related unit tests (though several do not currently validate the behaviors their names/descriptions imply).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/extension/intents/node/agentIntent.ts | Adds retry guard based on turn SummarizedConversationHistoryMetadata to avoid repeated foreground summarization after failure. |
| src/extension/prompts/node/agent/test/summarization.spec.tsx | Adds tests around summarization success/failure metadata, but some assertions are currently ineffective/misleading. |
Comments suppressed due to low confidence (2)
src/extension/prompts/node/agent/test/summarization.spec.tsx:496
- This test asserts
round.summarystays undefined after a failed summarization, but in this test you never applySummarizedConversationHistoryMetadataback ontotoolCallRounds(unlike the existingagentPromptToStringhelper which does). As written,round.summarywill remain undefined regardless of whether summarization succeeded, so the test doesn’t validate the intended behavior.
test('failed summarization does not set round.summary', async () => {
chatResponse[0] = 'x'.repeat(500_000);
const instaService = accessor.get(IInstantiationService);
const endpoint = instaService.createInstance(MockEndpoint, undefined);
const toolCallRounds = [
new ToolCallRound('ok', [createEditFileToolCall(1)]),
new ToolCallRound('ok 2', [createEditFileToolCall(2)]),
new ToolCallRound('ok 3', [createEditFileToolCall(3)]),
];
const turn = new Turn('turnId', { type: 'user', message: 'hello' });
const testConversation = new Conversation('sessionId', [turn]);
const promptContext: IBuildPromptContext = {
chatVariables: new ChatVariablesCollection([{ id: 'vscode.file', name: 'file', value: fileTsUri }]),
history: [],
query: 'edit this file',
toolCallRounds,
toolCallResults: createEditFileToolResult(1, 2, 3),
tools,
conversation: testConversation,
};
const customizations = await PromptRegistry.resolveAllCustomizations(instaService, endpoint);
const props: AgentPromptProps = {
priority: 1,
endpoint,
location: ChatLocation.Panel,
promptContext,
enableCacheBreakpoints: true,
triggerSummarize: true,
customizations,
};
const renderer = PromptRenderer.create(instaService, endpoint, AgentPrompt, props);
await renderer.render();
// None of the rounds should have summary set since summarization failed
for (const round of toolCallRounds) {
expect(round.summary).toBeUndefined();
}
});
src/extension/prompts/node/agent/test/summarization.spec.tsx:545
- The “failure metadata on turn prevents repeated foreground summarization attempts” test re-implements the retry-guard boolean locally instead of exercising the actual behavior in
agentIntent.ts(e.g., verifying that a second budget-exceeded path does not invoke the summarization fetcher again). This makes the test prone to false confidence: it will keep passing even if the production guard regresses or is removed.
test('failure metadata on turn prevents repeated foreground summarization attempts', async () => {
// This test verifies the contract that agentIntent.ts relies on:
// after a foreground summarization failure, setting SummarizedConversationHistoryMetadata
// with outcome !== 'success' on the turn causes the retry guard to skip summarization.
const turn = new Turn('turnId', { type: 'user', message: 'hello' });
// Simulate what agentIntent.ts does after a failed foreground summarization
turn.setMetadata(new SummarizedConversationHistoryMetadata(
'', // no toolCallRoundId for failures
'', // no summary text for failures
{
model: 'test-model',
source: 'foreground',
outcome: 'budgetExceeded',
contextLengthBefore: 100_000,
},
));
// Verify the retry guard condition from renderWithSummarization matches
const previousForegroundSummary = turn.getMetadata(SummarizedConversationHistoryMetadata);
expect(previousForegroundSummary).toBeDefined();
expect(previousForegroundSummary!.source).toBe('foreground');
expect(previousForegroundSummary!.outcome).toBe('budgetExceeded');
expect(previousForegroundSummary!.outcome).not.toBe('success');
// The guard condition: source === 'foreground' && outcome && outcome !== 'success'
const shouldSkip = previousForegroundSummary!.source === 'foreground'
&& !!previousForegroundSummary!.outcome
&& previousForegroundSummary!.outcome !== 'success';
expect(shouldSkip).toBe(true);
// Also verify that successful summarization does NOT trigger the skip guard
turn.setMetadata(new SummarizedConversationHistoryMetadata(
'roundId',
'summary text',
{
model: 'test-model',
source: 'foreground',
outcome: 'success',
},
));
const successMeta = turn.getMetadata(SummarizedConversationHistoryMetadata);
const shouldSkipAfterSuccess = successMeta!.source === 'foreground'
&& !!successMeta!.outcome
&& successMeta!.outcome !== 'success';
expect(shouldSkipAfterSuccess).toBe(false);
});
- Extract renderWithoutSummarization() helper to deduplicate fallback render logic between skip-path and failure-path - Add triggerSummarizeSkipped telemetry event + GenAiMetrics counter when the retry guard activates - Add comment documenting getLatestTurn() assumption - Extract createSummarizationTestContext() helper to DRY test setup - Fix test assertions: failed summarization throws from renderer (fallback lives in agentIntent.ts, not PromptRenderer)
deepak1556
approved these changes
Apr 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: microsoft/vscode#299810