mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-10 14:10:37 -07:00
fix(plan): keep approved plan during chat compression (#21284)
This commit is contained in:
@@ -177,4 +177,41 @@ describe('PromptProvider', () => {
|
||||
expect(prompt).toContain('/tmp/project-temp/plans/');
|
||||
});
|
||||
});
|
||||
|
||||
describe('getCompressionPrompt', () => {
|
||||
it('should include plan preservation instructions when an approved plan path is provided', () => {
|
||||
const planPath = '/path/to/plan.md';
|
||||
(
|
||||
mockConfig.getApprovedPlanPath as ReturnType<typeof vi.fn>
|
||||
).mockReturnValue(planPath);
|
||||
|
||||
const provider = new PromptProvider();
|
||||
const prompt = provider.getCompressionPrompt(mockConfig);
|
||||
|
||||
expect(prompt).toContain('### APPROVED PLAN PRESERVATION');
|
||||
expect(prompt).toContain(planPath);
|
||||
|
||||
// Verify it's BEFORE the structure example
|
||||
const structureMarker = 'The structure MUST be as follows:';
|
||||
const planPreservationMarker = '### APPROVED PLAN PRESERVATION';
|
||||
|
||||
const structureIndex = prompt.indexOf(structureMarker);
|
||||
const planPreservationIndex = prompt.indexOf(planPreservationMarker);
|
||||
|
||||
expect(planPreservationIndex).toBeGreaterThan(-1);
|
||||
expect(structureIndex).toBeGreaterThan(-1);
|
||||
expect(planPreservationIndex).toBeLessThan(structureIndex);
|
||||
});
|
||||
|
||||
it('should NOT include plan preservation instructions when no approved plan path is provided', () => {
|
||||
(
|
||||
mockConfig.getApprovedPlanPath as ReturnType<typeof vi.fn>
|
||||
).mockReturnValue(undefined);
|
||||
|
||||
const provider = new PromptProvider();
|
||||
const prompt = provider.getCompressionPrompt(mockConfig);
|
||||
|
||||
expect(prompt).not.toContain('### APPROVED PLAN PRESERVATION');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -234,7 +234,7 @@ export class PromptProvider {
|
||||
);
|
||||
const isModernModel = supportsModernFeatures(desiredModel);
|
||||
const activeSnippets = isModernModel ? snippets : legacySnippets;
|
||||
return activeSnippets.getCompressionPrompt();
|
||||
return activeSnippets.getCompressionPrompt(config.getApprovedPlanPath());
|
||||
}
|
||||
|
||||
private withSection<T>(
|
||||
|
||||
@@ -734,7 +734,17 @@ function formatToolName(name: string): string {
|
||||
/**
|
||||
* Provides the system prompt for history compression.
|
||||
*/
|
||||
export function getCompressionPrompt(): string {
|
||||
export function getCompressionPrompt(approvedPlanPath?: string): string {
|
||||
const planPreservation = approvedPlanPath
|
||||
? `
|
||||
|
||||
### APPROVED PLAN PRESERVATION
|
||||
An approved implementation plan exists at ${approvedPlanPath}. You MUST preserve the following in your snapshot:
|
||||
- The plan's file path in <key_knowledge>
|
||||
- Completion status of each plan step in <task_state> (mark as [DONE], [IN PROGRESS], or [TODO])
|
||||
- Any user feedback or modifications to the plan in <active_constraints>`
|
||||
: '';
|
||||
|
||||
return `
|
||||
You are a specialized system component responsible for distilling chat history into a structured XML <state_snapshot>.
|
||||
|
||||
@@ -750,7 +760,7 @@ When the conversation history grows too large, you will be invoked to distill th
|
||||
|
||||
First, you will think through the entire history in a private <scratchpad>. Review the user's overall goal, the agent's actions, tool outputs, file modifications, and any unresolved questions. Identify every piece of information for future actions.
|
||||
|
||||
After your reasoning is complete, generate the final <state_snapshot> XML object. Be incredibly dense with information. Omit any irrelevant conversational filler.
|
||||
After your reasoning is complete, generate the final <state_snapshot> XML object. Be incredibly dense with information. Omit any irrelevant conversational filler.${planPreservation}
|
||||
|
||||
The structure MUST be as follows:
|
||||
|
||||
|
||||
@@ -10,7 +10,7 @@ import {
|
||||
findCompressSplitPoint,
|
||||
modelStringToModelConfigAlias,
|
||||
} from './chatCompressionService.js';
|
||||
import type { Content, GenerateContentResponse } from '@google/genai';
|
||||
import type { Content, GenerateContentResponse, Part } from '@google/genai';
|
||||
import { CompressionStatus } from '../core/turn.js';
|
||||
import type { BaseLlmClient } from '../core/baseLlmClient.js';
|
||||
import type { GeminiChat } from '../core/geminiChat.js';
|
||||
@@ -189,6 +189,7 @@ describe('ChatCompressionService', () => {
|
||||
storage: {
|
||||
getProjectTempDir: vi.fn().mockReturnValue(testTempDir),
|
||||
},
|
||||
getApprovedPlanPath: vi.fn().mockReturnValue('/path/to/plan.md'),
|
||||
} as unknown as Config;
|
||||
|
||||
vi.mocked(getInitialChatHistory).mockImplementation(
|
||||
@@ -355,6 +356,63 @@ describe('ChatCompressionService', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('should include the approved plan path in the system instruction', async () => {
|
||||
const planPath = '/custom/plan/path.md';
|
||||
vi.mocked(mockConfig.getApprovedPlanPath).mockReturnValue(planPath);
|
||||
vi.mocked(mockConfig.getActiveModel).mockReturnValue(
|
||||
'gemini-3.1-pro-preview',
|
||||
);
|
||||
|
||||
const history: Content[] = [
|
||||
{ role: 'user', parts: [{ text: 'msg1' }] },
|
||||
{ role: 'model', parts: [{ text: 'msg2' }] },
|
||||
];
|
||||
vi.mocked(mockChat.getHistory).mockReturnValue(history);
|
||||
vi.mocked(mockChat.getLastPromptTokenCount).mockReturnValue(600000);
|
||||
|
||||
await service.compress(
|
||||
mockChat,
|
||||
mockPromptId,
|
||||
false,
|
||||
mockModel,
|
||||
mockConfig,
|
||||
false,
|
||||
);
|
||||
|
||||
const firstCallText = (
|
||||
vi.mocked(mockConfig.getBaseLlmClient().generateContent).mock.calls[0][0]
|
||||
.systemInstruction as Part
|
||||
).text;
|
||||
expect(firstCallText).toContain('### APPROVED PLAN PRESERVATION');
|
||||
expect(firstCallText).toContain(planPath);
|
||||
});
|
||||
|
||||
it('should not include the approved plan section if no approved plan path exists', async () => {
|
||||
vi.mocked(mockConfig.getApprovedPlanPath).mockReturnValue(undefined);
|
||||
|
||||
const history: Content[] = [
|
||||
{ role: 'user', parts: [{ text: 'msg1' }] },
|
||||
{ role: 'model', parts: [{ text: 'msg2' }] },
|
||||
];
|
||||
vi.mocked(mockChat.getHistory).mockReturnValue(history);
|
||||
vi.mocked(mockChat.getLastPromptTokenCount).mockReturnValue(600000);
|
||||
|
||||
await service.compress(
|
||||
mockChat,
|
||||
mockPromptId,
|
||||
false,
|
||||
mockModel,
|
||||
mockConfig,
|
||||
false,
|
||||
);
|
||||
|
||||
const firstCallText = (
|
||||
vi.mocked(mockConfig.getBaseLlmClient().generateContent).mock.calls[0][0]
|
||||
.systemInstruction as Part
|
||||
).text;
|
||||
expect(firstCallText).not.toContain('### APPROVED PLAN PRESERVATION');
|
||||
});
|
||||
|
||||
it('should force compress even if under threshold', async () => {
|
||||
const history: Content[] = [
|
||||
{ role: 'user', parts: [{ text: 'msg1' }] },
|
||||
|
||||
Reference in New Issue
Block a user