From 44e4f6e4b055692b088b47738cabe732dab9098f Mon Sep 17 00:00:00 2001 From: Arvin Xu Date: Tue, 17 Mar 2026 02:01:05 +0800 Subject: [PATCH] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20perf:=20optimize=20tool=20?= =?UTF-8?q?system=20prompt=20=E2=80=94=20remove=20duplicate=20APIs,=20simp?= =?UTF-8?q?lify=20XML=20tags=20(#13041)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 💄 style: remove platform-specific Spotlight reference from searchLocalFiles Replace "using Spotlight (macOS) or native search" with "using native search" since the actual search implementation is platform-dependent and the LLM doesn't need to know the specific backend. Fixes LOBE-5778 Co-Authored-By: Claude Opus 4.6 (1M context) * ⚡️ perf: remove duplicate API descriptions from tool system prompt API identifiers and descriptions are already in the tools schema passed via the API tools parameter. Repeating them in the system prompt wastes tokens. Now only tools with systemRole (usage instructions) are injected. Also rename XML tags: plugins→tools, collection→tool, collection.instructions→tool.instructions Co-Authored-By: Claude Opus 4.6 (1M context) * 💄 style: inject tool description when no systemRole instead of skipping Tools without systemRole now show their description as children. Tools with systemRole use wrapper as before. Co-Authored-By: Claude Opus 4.6 (1M context) * 💄 style: always emit tag, fallback to "no description" Co-Authored-By: Claude Opus 4.6 (1M context) * update tools * fix --------- Co-authored-by: Claude Opus 4.6 (1M context) --- .../src/systemRole.desktop.ts | 2 +- .../src/systemRole.ts | 2 +- .../messages/__tests__/MessagesEngine.test.ts | 2 +- .../src/providers/ToolSystemRole.ts | 1 + .../__tests__/ToolSystemRoleProvider.test.ts | 38 +++++++- .../prompts/src/prompts/plugin/index.test.ts | 15 +-- packages/prompts/src/prompts/plugin/index.ts | 9 +- .../prompts/src/prompts/plugin/tools.test.ts | 95 ++++++++----------- packages/prompts/src/prompts/plugin/tools.ts | 15 ++- 9 files changed, 94 insertions(+), 85 deletions(-) diff --git a/packages/builtin-tool-local-system/src/systemRole.desktop.ts b/packages/builtin-tool-local-system/src/systemRole.desktop.ts index e61caa6a9e..91a2890c3a 100644 --- a/packages/builtin-tool-local-system/src/systemRole.desktop.ts +++ b/packages/builtin-tool-local-system/src/systemRole.desktop.ts @@ -34,7 +34,7 @@ You have access to a set of tools to interact with the user's local file system: 9. **killCommand**: Terminate a running background shell command by its ID. **Search & Find:** -10. **searchLocalFiles**: Searches for files based on keywords and other criteria using Spotlight (macOS) or native search. Use this tool to find files if the user is unsure about the exact path. +10. **searchLocalFiles**: Searches for files based on keywords and other criteria using native search. Use this tool to find files if the user is unsure about the exact path. 11. **grepContent**: Search for content within files using regex patterns. Supports various output modes, filtering, and context lines. 12. **globLocalFiles**: Find files matching glob patterns (e.g., "**/*.js", "*.{ts,tsx}"). diff --git a/packages/builtin-tool-local-system/src/systemRole.ts b/packages/builtin-tool-local-system/src/systemRole.ts index 77618ae2b8..ddc6bcc642 100644 --- a/packages/builtin-tool-local-system/src/systemRole.ts +++ b/packages/builtin-tool-local-system/src/systemRole.ts @@ -23,7 +23,7 @@ You have access to a set of tools to interact with the user's local file system: 9. **killCommand**: Terminate a running background shell command by its ID. **Search & Find:** -10. **searchLocalFiles**: Searches for files based on keywords and other criteria using Spotlight (macOS) or native search. Use this tool to find files if the user is unsure about the exact path. +10. **searchLocalFiles**: Searches for files based on keywords and other criteria using native search. Use this tool to find files if the user is unsure about the exact path. 11. **grepContent**: Search for content within files using regex patterns. Supports various output modes, filtering, and context lines. 12. **globLocalFiles**: Find files matching glob patterns (e.g., "**/*.js", "*.{ts,tsx}"). diff --git a/packages/context-engine/src/engine/messages/__tests__/MessagesEngine.test.ts b/packages/context-engine/src/engine/messages/__tests__/MessagesEngine.test.ts index ddb2057fdb..f4adf10f7b 100644 --- a/packages/context-engine/src/engine/messages/__tests__/MessagesEngine.test.ts +++ b/packages/context-engine/src/engine/messages/__tests__/MessagesEngine.test.ts @@ -369,7 +369,7 @@ describe('MessagesEngine', () => { // Should inject tool system role when manifests are provided const systemMessage = result.messages.find((msg) => msg.role === 'system'); expect(systemMessage).toBeDefined(); - expect(systemMessage!.content).toContain('tool1'); + expect(systemMessage!.content).toContain('Tool 1'); }); it('should skip tool system role provider when no tools', async () => { diff --git a/packages/context-engine/src/providers/ToolSystemRole.ts b/packages/context-engine/src/providers/ToolSystemRole.ts index 364e1d3d7a..43414d709f 100644 --- a/packages/context-engine/src/providers/ToolSystemRole.ts +++ b/packages/context-engine/src/providers/ToolSystemRole.ts @@ -107,6 +107,7 @@ export class ToolSystemRoleProvider extends BaseProvider { name: this.toolNameResolver.generate(manifest.identifier, api.name, manifest.type), }), ), + description: manifest.meta?.description, identifier: manifest.identifier, name: manifest.meta?.title || manifest.identifier, systemRole: manifest.systemRole, diff --git a/packages/context-engine/src/providers/__tests__/ToolSystemRoleProvider.test.ts b/packages/context-engine/src/providers/__tests__/ToolSystemRoleProvider.test.ts index 7eee4e3e24..9ea38f412b 100644 --- a/packages/context-engine/src/providers/__tests__/ToolSystemRoleProvider.test.ts +++ b/packages/context-engine/src/providers/__tests__/ToolSystemRoleProvider.test.ts @@ -16,11 +16,12 @@ const createMockManifests = (identifiers: string[]): LobeToolManifest[] => identifier: id, api: [{ name: 'action', description: `${id} action`, parameters: {} }], meta: { title: id }, + systemRole: `Instructions for ${id}`, type: 'default' as const, })); describe('ToolSystemRoleProvider', () => { - it('should inject tool system role when manifests are provided and FC is supported', async () => { + it('should inject tool system role when manifests with systemRole are provided and FC is supported', async () => { const mockIsCanUseFC = () => true; const manifests = createMockManifests(['calculator', 'weather']); @@ -36,11 +37,12 @@ describe('ToolSystemRoleProvider', () => { const ctx = createContext(messages); const result = await provider.process(ctx); - // Should have system message with tool system role + // Should have system message with tool instructions const systemMessage = result.messages.find((msg) => msg.role === 'system'); expect(systemMessage).toBeDefined(); - expect(systemMessage!.content).toContain('calculator'); - expect(systemMessage!.content).toContain('weather'); + expect(systemMessage!.content).toContain(''); + expect(systemMessage!.content).toContain(''); + expect(systemMessage!.content).toContain('Instructions for calculator'); // Should update metadata expect(result.metadata.toolSystemRole).toBeDefined(); @@ -48,6 +50,34 @@ describe('ToolSystemRoleProvider', () => { expect(result.metadata.toolSystemRole!.supportsFunctionCall).toBe(true); }); + it('should skip injection when manifests have apis but no systemRole', async () => { + const mockIsCanUseFC = () => true; + const manifests: LobeToolManifest[] = [ + { + identifier: 'no-instructions', + api: [{ name: 'action', description: 'some action', parameters: {} }], + meta: { title: 'No Instructions' }, + type: 'default', + }, + ]; + + const provider = new ToolSystemRoleProvider({ + manifests, + model: 'gpt-4', + provider: 'openai', + isCanUseFC: mockIsCanUseFC, + }); + + const messages = [{ id: 'u1', role: 'user', content: 'Do something' }]; + const ctx = createContext(messages); + const result = await provider.process(ctx); + + // Has apis → still injects tool info even without systemRole + const systemMessage = result.messages.find((msg) => msg.role === 'system'); + expect(systemMessage).toBeDefined(); + expect(systemMessage!.content).toContain('No Instructions'); + }); + it('should merge tool system role with existing system message', async () => { const mockIsCanUseFC = () => true; const manifests = createMockManifests(['calculator']); diff --git a/packages/prompts/src/prompts/plugin/index.test.ts b/packages/prompts/src/prompts/plugin/index.test.ts index de410c0561..d5678ceaaf 100644 --- a/packages/prompts/src/prompts/plugin/index.test.ts +++ b/packages/prompts/src/prompts/plugin/index.test.ts @@ -18,12 +18,9 @@ describe('pluginPrompts', () => { }, ]; - const expected = ` - - -API 1 - -`; + const expected = ` +no description +`; expect(pluginPrompts({ tools })).toBe(expected); }); @@ -31,10 +28,6 @@ describe('pluginPrompts', () => { it('should generate plugin prompts without tools', () => { const tools: Tool[] = []; - const expected = ` - -`; - - expect(pluginPrompts({ tools })).toBe(expected); + expect(pluginPrompts({ tools })).toBe(''); }); }); diff --git a/packages/prompts/src/prompts/plugin/index.ts b/packages/prompts/src/prompts/plugin/index.ts index a31fff1fa2..a47256512b 100644 --- a/packages/prompts/src/prompts/plugin/index.ts +++ b/packages/prompts/src/prompts/plugin/index.ts @@ -2,11 +2,12 @@ import type { Tool } from './tools'; import { toolsPrompts } from './tools'; export const pluginPrompts = ({ tools }: { tools: Tool[] }) => { - const prompt = ` -${toolsPrompts(tools)} -`; + const content = toolsPrompts(tools); + if (!content) return ''; - return prompt.trim(); + return ` +${content} +`; }; export { type API, apiPrompt, type Tool, toolPrompt, toolsPrompts } from './tools'; diff --git a/packages/prompts/src/prompts/plugin/tools.test.ts b/packages/prompts/src/prompts/plugin/tools.test.ts index 1db0e91d06..28a770a7a4 100644 --- a/packages/prompts/src/prompts/plugin/tools.test.ts +++ b/packages/prompts/src/prompts/plugin/tools.test.ts @@ -4,104 +4,83 @@ import type { Tool } from './tools'; import { apiPrompt, toolPrompt, toolsPrompts } from './tools'; describe('Prompt Generation Utils', () => { - // 测试 apiPrompt 函数 describe('apiPrompt', () => { it('should generate correct api prompt', () => { - const api = { - name: 'testApi', - desc: 'Test API Description', - }; - + const api = { name: 'testApi', desc: 'Test API Description' }; expect(apiPrompt(api)).toBe(`Test API Description`); }); }); - // 测试 toolPrompt 函数 describe('toolPrompt', () => { - it('should generate tool prompt with system role', () => { + it('should use tool.instructions when systemRole is present', () => { const tool: Tool = { name: 'testTool', identifier: 'test-id', - systemRole: 'Test System Role', - apis: [ - { - name: 'api1', - desc: 'API 1 Description', - }, - ], + description: 'Short desc', + systemRole: 'Detailed instructions', + apis: [{ name: 'api1', desc: 'API 1' }], }; - const expected = ` -Test System Role -API 1 Description -`; - - expect(toolPrompt(tool)).toBe(expected); + expect(toolPrompt(tool)).toBe(` +Detailed instructions +`); }); - it('should generate tool prompt without system role', () => { + it('should use description as children when no systemRole', () => { const tool: Tool = { name: 'testTool', identifier: 'test-id', - apis: [ - { - name: 'api1', - desc: 'API 1 Description', - }, - ], + description: 'A useful tool for testing', + apis: [{ name: 'api1', desc: 'API 1' }], }; - const expected = ` + expect(toolPrompt(tool)).toBe(`A useful tool for testing`); + }); -API 1 Description -`; + it('should fallback to "no description" when no systemRole and no description', () => { + const tool: Tool = { + name: 'testTool', + identifier: 'test-id', + apis: [{ name: 'api1', desc: 'API 1' }], + }; - expect(toolPrompt(tool)).toBe(expected); + expect(toolPrompt(tool)).toBe('no description'); }); }); - // 测试 toolsPrompts 函数 describe('toolsPrompts', () => { - it('should generate tools prompts with multiple tools', () => { + it('should include tools with systemRole and description', () => { const tools: Tool[] = [ { name: 'tool1', identifier: 'id1', - apis: [ - { - name: 'api1', - desc: 'API 1', - }, - ], + systemRole: 'Instructions for tool1', + apis: [{ name: 'api1', desc: 'API 1' }], }, { name: 'tool2', identifier: 'id2', - apis: [ - { - name: 'api2', - desc: 'API 2', - }, - ], + description: 'Tool 2 description', + apis: [{ name: 'api2', desc: 'API 2' }], + }, + { + name: 'tool3', + identifier: 'id3', + apis: [{ name: 'api3', desc: 'API 3' }], }, ]; - const expected = ` - -API 1 - - - -API 2 -`; + const expected = ` +Instructions for tool1 + +Tool 2 description +no description`; expect(toolsPrompts(tools)).toBe(expected); }); - it('should generate tools prompts with empty tools array', () => { - const tools: Tool[] = []; - - expect(toolsPrompts(tools)).toBe(''); + it('should return empty for empty tools array', () => { + expect(toolsPrompts([])).toBe(''); }); }); }); diff --git a/packages/prompts/src/prompts/plugin/tools.ts b/packages/prompts/src/prompts/plugin/tools.ts index be4258006a..751bf7b509 100644 --- a/packages/prompts/src/prompts/plugin/tools.ts +++ b/packages/prompts/src/prompts/plugin/tools.ts @@ -4,6 +4,7 @@ export interface API { } export interface Tool { apis: API[]; + description?: string; identifier: string; name?: string; systemRole?: string; @@ -11,11 +12,15 @@ export interface Tool { export const apiPrompt = (api: API) => `${api.desc}`; -export const toolPrompt = (tool: Tool) => - ` -${tool.systemRole ? `${tool.systemRole}` : ''} -${tool.apis.map((api) => apiPrompt(api)).join('\n')} -`; +export const toolPrompt = (tool: Tool) => { + if (tool.systemRole) { + return ` +${tool.systemRole} +`; + } + + return `${tool.description || 'no description'}`; +}; export const toolsPrompts = (tools: Tool[]) => { const hasTools = tools.length > 0;