From 00068e9c5152bdbb6a3c3b8e8694eeed2cd2824e Mon Sep 17 00:00:00 2001 From: Abhijit Balaji Date: Mon, 30 Mar 2026 11:39:50 -0700 Subject: [PATCH] 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. --- .../messages/ToolGroupMessage.test.tsx | 10 ++-- .../ui/components/messages/TopicMessage.tsx | 15 +++-- .../ToolGroupMessage.test.tsx.snap | 4 +- packages/core/src/config/topicState.ts | 23 ++------ .../tools/definitions/base-declarations.ts | 1 - .../core/src/tools/definitions/coreTools.ts | 1 - .../dynamic-declaration-helpers.ts | 12 +--- packages/core/src/tools/tool-names.ts | 2 - packages/core/src/tools/topicTool.test.ts | 56 ++++--------------- packages/core/src/tools/topicTool.ts | 25 +++------ 10 files changed, 42 insertions(+), 107 deletions(-) diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx index e31c32899f..47b41dceb8 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx @@ -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('', () => { name: UPDATE_TOPIC_TOOL_NAME, args: { [TOPIC_PARAM_TITLE]: 'Testing Topic', - [TOPIC_PARAM_STRATEGIC_INTENT]: 'This is the description', }, }), ]; @@ -280,19 +279,18 @@ describe('', () => { 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('', () => { 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({ diff --git a/packages/cli/src/ui/components/messages/TopicMessage.tsx b/packages/cli/src/ui/components/messages/TopicMessage.tsx index 810628606d..4fd8663ba1 100644 --- a/packages/cli/src/ui/components/messages/TopicMessage.tsx +++ b/packages/cli/src/ui/components/messages/TopicMessage.tsx @@ -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 = ({ 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 ( {title || 'Topic'} - {intent && — {intent}} + {summary && ( + + {' — '} + {summary.length > 80 ? `${summary.substring(0, 80)}...` : summary} + + )} ); }; diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap index a3af0178a5..61ffb78568 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap @@ -77,7 +77,7 @@ exports[` > Golden Snapshots > renders header when scrolled `; exports[` > 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[` > Golden Snapshots > renders two tool groups where `; exports[` > Golden Snapshots > renders update_topic tool call using TopicMessage > update_topic_tool 1`] = ` -" Testing Topic — This is the description +" Testing Topic " `; diff --git a/packages/core/src/config/topicState.ts b/packages/core/src/config/topicState.ts index ee9a50af4f..3d8b5a0326 100644 --- a/packages/core/src/config/topicState.ts +++ b/packages/core/src/config/topicState.ts @@ -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; } } diff --git a/packages/core/src/tools/definitions/base-declarations.ts b/packages/core/src/tools/definitions/base-declarations.ts index 08b14ce6cb..408f3b9432 100644 --- a/packages/core/src/tools/definitions/base-declarations.ts +++ b/packages/core/src/tools/definitions/base-declarations.ts @@ -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'; diff --git a/packages/core/src/tools/definitions/coreTools.ts b/packages/core/src/tools/definitions/coreTools.ts index f642d2709f..d0ca7195d9 100644 --- a/packages/core/src/tools/definitions/coreTools.ts +++ b/packages/core/src/tools/definitions/coreTools.ts @@ -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 diff --git a/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts b/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts index 59b1bf7479..56e21313e3 100644 --- a/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts +++ b/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts @@ -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: [], }, }; } diff --git a/packages/core/src/tools/tool-names.ts b/packages/core/src/tools/tool-names.ts index 935c1834e7..325364f689 100644 --- a/packages/core/src/tools/tool-names.ts +++ b/packages/core/src/tools/tool-names.ts @@ -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]); diff --git a/packages/core/src/tools/topicTool.test.ts b/packages/core/src/tools/topicTool.test.ts index 25d2730e8c..5d309655e5 100644 --- a/packages/core/src/tools/topicTool.test.ts +++ b/packages/core/src/tools/topicTool.test.ts @@ -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"'); }); }); diff --git a/packages/core/src/tools/topicTool.ts b/packages/core/src/tools/topicTool.ts index 91d1b5abc5..276907e14a 100644 --- a/packages/core/src/tools/topicTool.ts +++ b/packages/core/src/tools/topicTool.ts @@ -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 { 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,