mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-06-21 08:47:13 -07:00
refactor(core,cli): remove strategic_intent from update_topic tool
Removing the strategic_intent parameter to streamline narration and avoid redundancy. Updates tool schema, state management, UI rendering, and prompt instructions while maintaining summary field utility.
This commit is contained in:
@@ -10,7 +10,7 @@ import { ToolGroupMessage } from './ToolGroupMessage.js';
|
||||
import {
|
||||
UPDATE_TOPIC_TOOL_NAME,
|
||||
TOPIC_PARAM_TITLE,
|
||||
TOPIC_PARAM_STRATEGIC_INTENT,
|
||||
TOPIC_PARAM_SUMMARY,
|
||||
makeFakeConfig,
|
||||
CoreToolCallStatus,
|
||||
ApprovalMode,
|
||||
@@ -264,7 +264,6 @@ describe('<ToolGroupMessage />', () => {
|
||||
name: UPDATE_TOPIC_TOOL_NAME,
|
||||
args: {
|
||||
[TOPIC_PARAM_TITLE]: 'Testing Topic',
|
||||
[TOPIC_PARAM_STRATEGIC_INTENT]: 'This is the description',
|
||||
},
|
||||
}),
|
||||
];
|
||||
@@ -280,19 +279,18 @@ describe('<ToolGroupMessage />', () => {
|
||||
|
||||
const output = lastFrame();
|
||||
expect(output).toContain('Testing Topic');
|
||||
expect(output).toContain('— This is the description');
|
||||
expect(output).toMatchSnapshot('update_topic_tool');
|
||||
unmount();
|
||||
});
|
||||
|
||||
it('renders update_topic tool call with summary instead of strategic_intent', async () => {
|
||||
it('renders update_topic tool call with summary', async () => {
|
||||
const toolCalls = [
|
||||
createToolCall({
|
||||
callId: 'topic-tool-summary',
|
||||
name: UPDATE_TOPIC_TOOL_NAME,
|
||||
args: {
|
||||
[TOPIC_PARAM_TITLE]: 'Testing Topic',
|
||||
summary: 'This is the summary',
|
||||
[TOPIC_PARAM_SUMMARY]: 'This is the summary',
|
||||
},
|
||||
}),
|
||||
];
|
||||
@@ -319,7 +317,7 @@ describe('<ToolGroupMessage />', () => {
|
||||
name: UPDATE_TOPIC_TOOL_NAME,
|
||||
args: {
|
||||
[TOPIC_PARAM_TITLE]: 'Testing Topic',
|
||||
[TOPIC_PARAM_STRATEGIC_INTENT]: 'This is the description',
|
||||
[TOPIC_PARAM_SUMMARY]: 'This is the summary',
|
||||
},
|
||||
}),
|
||||
createToolCall({
|
||||
|
||||
@@ -11,7 +11,6 @@ import {
|
||||
UPDATE_TOPIC_DISPLAY_NAME,
|
||||
TOPIC_PARAM_TITLE,
|
||||
TOPIC_PARAM_SUMMARY,
|
||||
TOPIC_PARAM_STRATEGIC_INTENT,
|
||||
} from '@google/gemini-cli-core';
|
||||
import type { IndividualToolCallDisplay } from '../../types.js';
|
||||
import { theme } from '../../semantic-colors.js';
|
||||
@@ -26,16 +25,22 @@ export const isTopicTool = (name: string): boolean =>
|
||||
export const TopicMessage: React.FC<TopicMessageProps> = ({ args }) => {
|
||||
const rawTitle = args?.[TOPIC_PARAM_TITLE];
|
||||
const title = typeof rawTitle === 'string' ? rawTitle : undefined;
|
||||
const rawIntent =
|
||||
args?.[TOPIC_PARAM_STRATEGIC_INTENT] || args?.[TOPIC_PARAM_SUMMARY];
|
||||
const intent = typeof rawIntent === 'string' ? rawIntent : undefined;
|
||||
const rawSummary = args?.[TOPIC_PARAM_SUMMARY];
|
||||
const summary = typeof rawSummary === 'string' ? rawSummary : undefined;
|
||||
|
||||
// Use summary as a fallback subtitle if title is present,
|
||||
// or use it as the main text if title is missing.
|
||||
return (
|
||||
<Box flexDirection="row" marginLeft={2}>
|
||||
<Text color={theme.text.primary} bold>
|
||||
{title || 'Topic'}
|
||||
</Text>
|
||||
{intent && <Text color={theme.text.secondary}> — {intent}</Text>}
|
||||
{summary && (
|
||||
<Text color={theme.text.secondary} wrap="truncate-end">
|
||||
{' — '}
|
||||
{summary.length > 80 ? `${summary.substring(0, 80)}...` : summary}
|
||||
</Text>
|
||||
)}
|
||||
</Box>
|
||||
);
|
||||
};
|
||||
|
||||
+2
-2
@@ -77,7 +77,7 @@ exports[`<ToolGroupMessage /> > Golden Snapshots > renders header when scrolled
|
||||
`;
|
||||
|
||||
exports[`<ToolGroupMessage /> > Golden Snapshots > renders mixed tool calls including update_topic 1`] = `
|
||||
" Testing Topic — This is the description
|
||||
" Testing Topic — This is the summary
|
||||
╭──────────────────────────────────────────────────────────────────────────╮
|
||||
│ ✓ read_file Read a file │
|
||||
│ │
|
||||
@@ -141,7 +141,7 @@ exports[`<ToolGroupMessage /> > Golden Snapshots > renders two tool groups where
|
||||
`;
|
||||
|
||||
exports[`<ToolGroupMessage /> > Golden Snapshots > renders update_topic tool call using TopicMessage > update_topic_tool 1`] = `
|
||||
" Testing Topic — This is the description
|
||||
" Testing Topic
|
||||
"
|
||||
`;
|
||||
|
||||
|
||||
@@ -5,30 +5,22 @@
|
||||
*/
|
||||
|
||||
/**
|
||||
* Manages the current active topic title and tactical intent for a session.
|
||||
* Manages the current active topic title for a session.
|
||||
* Hosted within the Config instance for session-scoping.
|
||||
*/
|
||||
export class TopicState {
|
||||
private activeTopicTitle?: string;
|
||||
private activeIntent?: string;
|
||||
|
||||
/**
|
||||
* Sanitizes and sets the topic title and/or intent.
|
||||
* Sanitizes and sets the topic title.
|
||||
* @returns true if the input was valid and set, false otherwise.
|
||||
*/
|
||||
setTopic(title?: string, intent?: string): boolean {
|
||||
setTopic(title?: string): boolean {
|
||||
const sanitizedTitle = title?.trim().replace(/[\r\n]+/g, ' ');
|
||||
const sanitizedIntent = intent?.trim().replace(/[\r\n]+/g, ' ');
|
||||
|
||||
if (!sanitizedTitle && !sanitizedIntent) return false;
|
||||
if (!sanitizedTitle) return false;
|
||||
|
||||
if (sanitizedTitle) {
|
||||
this.activeTopicTitle = sanitizedTitle;
|
||||
}
|
||||
|
||||
if (sanitizedIntent) {
|
||||
this.activeIntent = sanitizedIntent;
|
||||
}
|
||||
this.activeTopicTitle = sanitizedTitle;
|
||||
|
||||
return true;
|
||||
}
|
||||
@@ -37,12 +29,7 @@ export class TopicState {
|
||||
return this.activeTopicTitle;
|
||||
}
|
||||
|
||||
getIntent(): string | undefined {
|
||||
return this.activeIntent;
|
||||
}
|
||||
|
||||
reset(): void {
|
||||
this.activeTopicTitle = undefined;
|
||||
this.activeIntent = undefined;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -131,4 +131,3 @@ export const UPDATE_TOPIC_TOOL_NAME = 'update_topic';
|
||||
export const UPDATE_TOPIC_DISPLAY_NAME = 'Update Topic Context';
|
||||
export const TOPIC_PARAM_TITLE = 'title';
|
||||
export const TOPIC_PARAM_SUMMARY = 'summary';
|
||||
export const TOPIC_PARAM_STRATEGIC_INTENT = 'strategic_intent';
|
||||
|
||||
@@ -96,7 +96,6 @@ export {
|
||||
SKILL_PARAM_NAME,
|
||||
TOPIC_PARAM_TITLE,
|
||||
TOPIC_PARAM_SUMMARY,
|
||||
TOPIC_PARAM_STRATEGIC_INTENT,
|
||||
} from './base-declarations.js';
|
||||
|
||||
// Re-export sets for compatibility
|
||||
|
||||
@@ -27,7 +27,6 @@ import {
|
||||
UPDATE_TOPIC_TOOL_NAME,
|
||||
TOPIC_PARAM_TITLE,
|
||||
TOPIC_PARAM_SUMMARY,
|
||||
TOPIC_PARAM_STRATEGIC_INTENT,
|
||||
} from './base-declarations.js';
|
||||
|
||||
/**
|
||||
@@ -216,7 +215,7 @@ export function getUpdateTopicDeclaration(): FunctionDeclaration {
|
||||
return {
|
||||
name: UPDATE_TOPIC_TOOL_NAME,
|
||||
description:
|
||||
'Manages your narrative flow. Include `title` and `summary` only when starting a new Chapter (logical phase) or shifting strategic intent.',
|
||||
'Manages your narrative flow. Include `title` and `summary` only when starting a new Chapter (logical phase).',
|
||||
parametersJsonSchema: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
@@ -227,15 +226,10 @@ export function getUpdateTopicDeclaration(): FunctionDeclaration {
|
||||
[TOPIC_PARAM_SUMMARY]: {
|
||||
type: 'string',
|
||||
description:
|
||||
'(OPTIONAL) A detailed summary (5-10 sentences) covering both the work completed in the previous topic and the strategic intent of the new topic. This is required when transitioning between topics to maintain continuity.',
|
||||
},
|
||||
[TOPIC_PARAM_STRATEGIC_INTENT]: {
|
||||
type: 'string',
|
||||
description:
|
||||
'A mandatory one-sentence statement of your immediate intent.',
|
||||
'(OPTIONAL) A detailed summary (5-10 sentences) covering both the work completed in the previous topic and the high-level goals of the new topic. This is required when transitioning between topics to maintain continuity.',
|
||||
},
|
||||
},
|
||||
required: [TOPIC_PARAM_STRATEGIC_INTENT],
|
||||
required: [],
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -79,7 +79,6 @@ import {
|
||||
UPDATE_TOPIC_DISPLAY_NAME,
|
||||
TOPIC_PARAM_TITLE,
|
||||
TOPIC_PARAM_SUMMARY,
|
||||
TOPIC_PARAM_STRATEGIC_INTENT,
|
||||
} from './definitions/coreTools.js';
|
||||
|
||||
export {
|
||||
@@ -157,7 +156,6 @@ export {
|
||||
SKILL_PARAM_NAME,
|
||||
TOPIC_PARAM_TITLE,
|
||||
TOPIC_PARAM_SUMMARY,
|
||||
TOPIC_PARAM_STRATEGIC_INTENT,
|
||||
};
|
||||
|
||||
export const EDIT_TOOL_NAMES = new Set([EDIT_TOOL_NAME, WRITE_FILE_TOOL_NAME]);
|
||||
|
||||
@@ -13,7 +13,6 @@ import {
|
||||
UPDATE_TOPIC_TOOL_NAME,
|
||||
TOPIC_PARAM_TITLE,
|
||||
TOPIC_PARAM_SUMMARY,
|
||||
TOPIC_PARAM_STRATEGIC_INTENT,
|
||||
} from './definitions/base-declarations.js';
|
||||
import type { Config } from '../config/config.js';
|
||||
|
||||
@@ -24,36 +23,31 @@ describe('TopicState', () => {
|
||||
state = new TopicState();
|
||||
});
|
||||
|
||||
it('should store and retrieve topic title and intent', () => {
|
||||
it('should store and retrieve topic title', () => {
|
||||
expect(state.getTopic()).toBeUndefined();
|
||||
expect(state.getIntent()).toBeUndefined();
|
||||
const success = state.setTopic('Test Topic', 'Test Intent');
|
||||
const success = state.setTopic('Test Topic');
|
||||
expect(success).toBe(true);
|
||||
expect(state.getTopic()).toBe('Test Topic');
|
||||
expect(state.getIntent()).toBe('Test Intent');
|
||||
});
|
||||
|
||||
it('should sanitize newlines and carriage returns', () => {
|
||||
state.setTopic('Topic\nWith\r\nLines', 'Intent\nWith\r\nLines');
|
||||
state.setTopic('Topic\nWith\r\nLines');
|
||||
expect(state.getTopic()).toBe('Topic With Lines');
|
||||
expect(state.getIntent()).toBe('Intent With Lines');
|
||||
});
|
||||
|
||||
it('should trim whitespace', () => {
|
||||
state.setTopic(' Spaced Topic ', ' Spaced Intent ');
|
||||
state.setTopic(' Spaced Topic ');
|
||||
expect(state.getTopic()).toBe('Spaced Topic');
|
||||
expect(state.getIntent()).toBe('Spaced Intent');
|
||||
});
|
||||
|
||||
it('should reject empty or whitespace-only inputs', () => {
|
||||
expect(state.setTopic('', '')).toBe(false);
|
||||
expect(state.setTopic('')).toBe(false);
|
||||
});
|
||||
|
||||
it('should reset topic and intent', () => {
|
||||
state.setTopic('Test Topic', 'Test Intent');
|
||||
it('should reset topic', () => {
|
||||
state.setTopic('Test Topic');
|
||||
state.reset();
|
||||
expect(state.getTopic()).toBeUndefined();
|
||||
expect(state.getIntent()).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -76,11 +70,10 @@ describe('UpdateTopicTool', () => {
|
||||
expect(tool.displayName).toBe('Update Topic Context');
|
||||
});
|
||||
|
||||
it('should update TopicState and include strategic intent on execute', async () => {
|
||||
it('should update TopicState on execute', async () => {
|
||||
const invocation = tool.build({
|
||||
[TOPIC_PARAM_TITLE]: 'New Chapter',
|
||||
[TOPIC_PARAM_SUMMARY]: 'The goal is to implement X. Previously we did Y.',
|
||||
[TOPIC_PARAM_STRATEGIC_INTENT]: 'Initial Move',
|
||||
});
|
||||
const result = await invocation.execute(new AbortController().signal);
|
||||
|
||||
@@ -88,47 +81,20 @@ describe('UpdateTopicTool', () => {
|
||||
expect(result.llmContent).toContain(
|
||||
'Topic summary: The goal is to implement X. Previously we did Y.',
|
||||
);
|
||||
expect(result.llmContent).toContain('Strategic Intent: Initial Move');
|
||||
expect(mockConfig.topicState.getTopic()).toBe('New Chapter');
|
||||
expect(mockConfig.topicState.getIntent()).toBe('Initial Move');
|
||||
expect(result.returnDisplay).toContain('## 📂 Topic: **New Chapter**');
|
||||
expect(result.returnDisplay).toContain('**Summary:**');
|
||||
expect(result.returnDisplay).toContain(
|
||||
'> [!STRATEGY]\n> **Intent:** Initial Move',
|
||||
);
|
||||
});
|
||||
|
||||
it('should render only intent for tactical updates (same topic)', async () => {
|
||||
it('should render only topic title for same topic updates', async () => {
|
||||
mockConfig.topicState.setTopic('New Chapter');
|
||||
|
||||
const invocation = tool.build({
|
||||
[TOPIC_PARAM_TITLE]: 'New Chapter',
|
||||
[TOPIC_PARAM_STRATEGIC_INTENT]: 'Subsequent Move',
|
||||
});
|
||||
const result = await invocation.execute(new AbortController().signal);
|
||||
|
||||
expect(result.returnDisplay).not.toContain('## 📂 Topic:');
|
||||
expect(result.returnDisplay).toBe(
|
||||
'> [!STRATEGY]\n> **Intent:** Subsequent Move',
|
||||
);
|
||||
expect(result.llmContent).toBe('Strategic Intent: Subsequent Move');
|
||||
});
|
||||
|
||||
it('should return error if strategic_intent is missing', async () => {
|
||||
try {
|
||||
tool.build({
|
||||
[TOPIC_PARAM_TITLE]: 'Title',
|
||||
});
|
||||
expect.fail('Should have thrown validation error');
|
||||
} catch (e: unknown) {
|
||||
if (e instanceof Error) {
|
||||
expect(e.message).toContain(
|
||||
"must have required property 'strategic_intent'",
|
||||
);
|
||||
} else {
|
||||
expect.fail('Expected Error instance');
|
||||
}
|
||||
}
|
||||
expect(mockConfig.topicState.getTopic()).toBeUndefined();
|
||||
expect(result.returnDisplay).toBe('## 📂 Topic: **New Chapter**');
|
||||
expect(result.llmContent).toBe('Current topic: "New Chapter"');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -9,7 +9,6 @@ import {
|
||||
UPDATE_TOPIC_DISPLAY_NAME,
|
||||
TOPIC_PARAM_TITLE,
|
||||
TOPIC_PARAM_SUMMARY,
|
||||
TOPIC_PARAM_STRATEGIC_INTENT,
|
||||
} from './definitions/coreTools.js';
|
||||
import {
|
||||
BaseDeclarativeTool,
|
||||
@@ -25,7 +24,6 @@ import type { Config } from '../config/config.js';
|
||||
interface UpdateTopicParams {
|
||||
[TOPIC_PARAM_TITLE]?: string;
|
||||
[TOPIC_PARAM_SUMMARY]?: string;
|
||||
[TOPIC_PARAM_STRATEGIC_INTENT]?: string;
|
||||
}
|
||||
|
||||
class UpdateTopicInvocation extends BaseToolInvocation<
|
||||
@@ -43,17 +41,15 @@ class UpdateTopicInvocation extends BaseToolInvocation<
|
||||
|
||||
getDescription(): string {
|
||||
const title = this.params[TOPIC_PARAM_TITLE];
|
||||
const intent = this.params[TOPIC_PARAM_STRATEGIC_INTENT];
|
||||
if (title) {
|
||||
return `Update topic to: "${title}"`;
|
||||
}
|
||||
return `Update tactical intent: "${intent || '...'}"`;
|
||||
return 'Update Topic';
|
||||
}
|
||||
|
||||
async execute(): Promise<ToolResult> {
|
||||
const title = this.params[TOPIC_PARAM_TITLE];
|
||||
const summary = this.params[TOPIC_PARAM_SUMMARY];
|
||||
const strategicIntent = this.params[TOPIC_PARAM_STRATEGIC_INTENT];
|
||||
|
||||
const activeTopic = this.config.topicState.getTopic();
|
||||
const isNewTopic = !!(
|
||||
@@ -62,14 +58,12 @@ class UpdateTopicInvocation extends BaseToolInvocation<
|
||||
title.trim() !== activeTopic
|
||||
);
|
||||
|
||||
this.config.topicState.setTopic(title, strategicIntent);
|
||||
this.config.topicState.setTopic(title);
|
||||
|
||||
const currentTopic = this.config.topicState.getTopic() || '...';
|
||||
const currentIntent =
|
||||
strategicIntent || this.config.topicState.getIntent() || '...';
|
||||
|
||||
debugLogger.log(
|
||||
`[TopicTool] Update: Topic="${currentTopic}", Intent="${currentIntent}", isNew=${isNewTopic}`,
|
||||
`[TopicTool] Update: Topic="${currentTopic}", isNew=${isNewTopic}`,
|
||||
);
|
||||
|
||||
let llmContent = '';
|
||||
@@ -79,15 +73,10 @@ class UpdateTopicInvocation extends BaseToolInvocation<
|
||||
// Handle New Topic Header & Summary
|
||||
llmContent = `Current topic: "${currentTopic}"\nTopic summary: ${summary || '...'}`;
|
||||
returnDisplay = `## 📂 Topic: **${currentTopic}**\n\n**Summary:**\n${summary || '...'}`;
|
||||
|
||||
if (strategicIntent && strategicIntent.trim()) {
|
||||
llmContent += `\n\nStrategic Intent: ${strategicIntent.trim()}`;
|
||||
returnDisplay += `\n\n> [!STRATEGY]\n> **Intent:** ${strategicIntent.trim()}`;
|
||||
}
|
||||
} else {
|
||||
// Tactical update only
|
||||
llmContent = `Strategic Intent: ${currentIntent}`;
|
||||
returnDisplay = `> [!STRATEGY]\n> **Intent:** ${currentIntent}`;
|
||||
// Fallback display if not a new topic (though mandate suggests only for new topics)
|
||||
llmContent = `Current topic: "${currentTopic}"`;
|
||||
returnDisplay = `## 📂 Topic: **${currentTopic}**`;
|
||||
}
|
||||
|
||||
return {
|
||||
@@ -98,7 +87,7 @@ class UpdateTopicInvocation extends BaseToolInvocation<
|
||||
}
|
||||
|
||||
/**
|
||||
* Tool to update semantic topic context and tactical intent for UI grouping and model focus.
|
||||
* Tool to update semantic topic context for UI grouping and model focus.
|
||||
*/
|
||||
export class UpdateTopicTool extends BaseDeclarativeTool<
|
||||
UpdateTopicParams,
|
||||
|
||||
Reference in New Issue
Block a user