From 8bd4cbc3fde6400590da3b0710d417af88d97045 Mon Sep 17 00:00:00 2001 From: Etienne <45695613+etiennejouan@users.noreply.github.com> Date: Thu, 4 Jun 2026 19:27:25 +0200 Subject: [PATCH] fix(ai) - optim (#21233) 1. tool-registry.service.ts, Pass precomputed catalog to resolveSchemas() resolveSchemas() now accepts an optional precomputedCatalog parameter. Both getToolsByName() and getToolInfo() pass the catalog they already fetched, eliminating a redundant getCatalog() rebuild inside resolveSchemas(). 2. database-tool.provider.ts, Skip field lookup when schemas=false When building the catalog index (includeSchemas=false), getFlatFieldsFromFlatObjectMetadata() is no longer called for each of the 25 objects. The hasGroupByToolInputSchema() check is also skipped, group_by tools are always included in the index, with the real eligibility check deferred to learn_tools time. --> 100/150ms gain on learn/execute_tool execution --- .../build-mcp-server-instructions.util.ts | 8 ++-- .../providers/database-tool.provider.ts | 14 +++---- .../resolvers/tool-index.resolver.ts | 15 ++++--- .../services/tool-registry.service.ts | 42 +++++++++++-------- .../constants/chat-system-prompts.const.ts | 4 +- 5 files changed, 48 insertions(+), 35 deletions(-) diff --git a/packages/twenty-server/src/engine/api/mcp/utils/build-mcp-server-instructions.util.ts b/packages/twenty-server/src/engine/api/mcp/utils/build-mcp-server-instructions.util.ts index 7e3586072b..4449779631 100644 --- a/packages/twenty-server/src/engine/api/mcp/utils/build-mcp-server-instructions.util.ts +++ b/packages/twenty-server/src/engine/api/mcp/utils/build-mcp-server-instructions.util.ts @@ -44,6 +44,8 @@ export const buildMcpServerInstructions = ( ` Single record by id → find_one_{object}`, ` Analytics / grouped metrics → group_by_{objects} (COUNT, SUM, AVG, MIN, MAX)`, ` Multiple metrics → run parallel group_by calls, merge results`, + ` Per-record updates (different data per record) → find_many_{objects} first, then upsert_many_{objects} with each record's id (or other unique identifier) and new values`, + ` Bulk update (same data for all) → update_many_{objects}`, ` Non-CRUD task → use tool name from the list above + learn_tools for schema`, ``, `Execution rules:`, @@ -53,10 +55,10 @@ export const buildMcpServerInstructions = ( `Data efficiency:`, ` Default limit: 10. Only increase if user explicitly needs more.`, ` Always apply filters to narrow results — don't fetch all records of a type.`, - ` Use batch tools (create_many_*, update_many_*) instead of looping single-item calls.`, + ` Use batch tools (create_many_*, upsert_many_*, update_many_*) instead of looping single-item calls.`, ``, - `Destructive operations (update_many, delete):`, - ` 1. Run find_{objects} with the same filter — state record count to user`, + `Destructive operations (delete_one, delete_many):`, + ` 1. Run find_many_{objects} with the same filter — state record count to user`, ` 2. Wait for explicit confirmation before executing`, ``, `Twenty primitives:`, diff --git a/packages/twenty-server/src/engine/core-modules/tool-provider/providers/database-tool.provider.ts b/packages/twenty-server/src/engine/core-modules/tool-provider/providers/database-tool.provider.ts index 350d8dc1c1..6a487f3959 100644 --- a/packages/twenty-server/src/engine/core-modules/tool-provider/providers/database-tool.provider.ts +++ b/packages/twenty-server/src/engine/core-modules/tool-provider/providers/database-tool.provider.ts @@ -113,13 +113,11 @@ export class DatabaseToolProvider implements ToolProvider { continue; } - const objectMetadata = { - ...flatObject, - fields: getFlatFieldsFromFlatObjectMetadata( - flatObject, - flatFieldMetadataMaps, - ), - }; + const fields = includeSchemas + ? getFlatFieldsFromFlatObjectMetadata(flatObject, flatFieldMetadataMaps) + : []; + + const objectMetadata = { ...flatObject, fields }; const restrictedFields = permission.restrictedFields; const canBeManagedByAutomation = canObjectBeManagedByAutomation({ @@ -171,7 +169,9 @@ export class DatabaseToolProvider implements ToolProvider { const groupBySchema = shouldGenerateGroupBy ? generateGroupByToolInputSchema(objectMetadata, restrictedFields) : null; + const hasGroupBySchema = + !includeSchemas || groupBySchema !== null || hasGroupByToolInputSchema(objectMetadata, restrictedFields); diff --git a/packages/twenty-server/src/engine/core-modules/tool-provider/resolvers/tool-index.resolver.ts b/packages/twenty-server/src/engine/core-modules/tool-provider/resolvers/tool-index.resolver.ts index 1a28a89353..7b28c22112 100644 --- a/packages/twenty-server/src/engine/core-modules/tool-provider/resolvers/tool-index.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/tool-provider/resolvers/tool-index.resolver.ts @@ -84,12 +84,15 @@ export class ToolIndexResolver { return null; } - const schemas = await this.toolRegistryService.resolveSchemas([toolName], { - workspaceId: workspace.id, - roleId, - rolePermissionConfig: { unionOf: [roleId] }, - userId: user?.id, - userWorkspaceId, + const schemas = await this.toolRegistryService.resolveSchemas({ + toolNames: [toolName], + context: { + workspaceId: workspace.id, + roleId, + rolePermissionConfig: { unionOf: [roleId] }, + userId: user?.id, + userWorkspaceId, + }, }); return schemas.get(toolName) ?? null; diff --git a/packages/twenty-server/src/engine/core-modules/tool-provider/services/tool-registry.service.ts b/packages/twenty-server/src/engine/core-modules/tool-provider/services/tool-registry.service.ts index 79905ea6e4..099e057943 100644 --- a/packages/twenty-server/src/engine/core-modules/tool-provider/services/tool-registry.service.ts +++ b/packages/twenty-server/src/engine/core-modules/tool-provider/services/tool-registry.service.ts @@ -2,8 +2,8 @@ import { Inject, Injectable, Logger } from '@nestjs/common'; import { type ToolSet, jsonSchema } from 'ai'; -import { type ToolProvider } from 'src/engine/core-modules/tool-provider/interfaces/tool-provider.interface'; import { type ToolProviderContext } from 'src/engine/core-modules/tool-provider/interfaces/tool-provider-context.type'; +import { type ToolProvider } from 'src/engine/core-modules/tool-provider/interfaces/tool-provider.interface'; import { type ToolRetrievalOptions } from 'src/engine/core-modules/tool-provider/interfaces/tool-retrieval-options.type'; import { TOOL_PROVIDERS } from 'src/engine/core-modules/tool-provider/constants/tool-providers.token'; @@ -31,9 +31,6 @@ export class ToolRegistryService { private readonly toolExecutorService: ToolExecutorService, ) {} - // Returns ToolIndexEntry[] (lightweight, no schemas). - // Underlying data (metadata, permissions) is already cached by WorkspaceCacheService. - // Providers run in parallel since they are independent. async getCatalog(context: ToolProviderContext): Promise { const results = await Promise.all( this.providers.map(async (provider) => { @@ -50,16 +47,19 @@ export class ToolRegistryService { return results.flat(); } - // On-demand schema generation for specific tools - async resolveSchemas( - toolNames: string[], - context: ToolProviderContext, - ): Promise> { - const index = await this.getCatalog(context); + async resolveSchemas({ + toolNames, + context, + precomputedCatalog, + }: { + toolNames: string[]; + context: ToolProviderContext; + precomputedCatalog?: ToolIndexEntry[]; + }): Promise> { + const index = precomputedCatalog ?? (await this.getCatalog(context)); const nameSet = new Set(toolNames); const matchingEntries = index.filter((entry) => nameSet.has(entry.name)); - // Group matching entries by provider category const byCategory = new Map(); for (const entry of matchingEntries) { @@ -176,11 +176,15 @@ export class ToolRegistryService { ): Promise { const fullContext = this.buildContextFromToolContext(context); - const index = await this.getCatalog(fullContext); + const catalog = await this.getCatalog(fullContext); const nameSet = new Set(names); - const matchingEntries = index.filter((entry) => nameSet.has(entry.name)); + const matchingEntries = catalog.filter((entry) => nameSet.has(entry.name)); - const schemas = await this.resolveSchemas(names, fullContext); + const schemas = await this.resolveSchemas({ + toolNames: names, + context: fullContext, + precomputedCatalog: catalog, + }); const descriptors: ToolDescriptor[] = matchingEntries .filter((entry) => schemas.has(entry.name)) @@ -204,14 +208,18 @@ export class ToolRegistryService { > { const fullContext = this.buildContextFromToolContext(context); - const index = await this.getCatalog(fullContext); + const catalog = await this.getCatalog(fullContext); const nameSet = new Set(names); - const matchingEntries = index.filter((entry) => nameSet.has(entry.name)); + const matchingEntries = catalog.filter((entry) => nameSet.has(entry.name)); let schemas: Map | undefined; if (aspects.includes('schema')) { - schemas = await this.resolveSchemas(names, fullContext); + schemas = await this.resolveSchemas({ + toolNames: names, + context: fullContext, + precomputedCatalog: catalog, + }); } return matchingEntries.map((entry) => { diff --git a/packages/twenty-server/src/engine/metadata-modules/ai/ai-chat/constants/chat-system-prompts.const.ts b/packages/twenty-server/src/engine/metadata-modules/ai/ai-chat/constants/chat-system-prompts.const.ts index c83a23e2bb..7e0f2dfe8d 100644 --- a/packages/twenty-server/src/engine/metadata-modules/ai/ai-chat/constants/chat-system-prompts.const.ts +++ b/packages/twenty-server/src/engine/metadata-modules/ai/ai-chat/constants/chat-system-prompts.const.ts @@ -34,7 +34,7 @@ For simple CRUD operations (find/create/update/delete a record), you do NOT need - The \`http_request\` tool is ONLY for external third-party APIs (not for Twenty's own data) - If you need to look up a record by ID, use find_one_*; to search with filters, use find_many_* - For comparative/grouped analytics questions (by/per/top/most/least/average/total/ranking), use \`group_by_*\` instead of \`find_many_*\`; if multiple metrics are needed, run multiple \`group_by_*\` calls with the same dimensions and merge results. -- **update_many_* vs upsert_many_***: use \`update_many_*\` when ALL matched records get the SAME data (e.g. mark all as closed). Use \`upsert_many_*\` when each record has different data to set, or when some records may not exist yet (insert-or-update per record). +- **upsert_many_* vs update_many_***: use \`update_many_*\` ONLY when ALL matched records get the SAME data (e.g. mark all as closed). Use \`upsert_many_*\` (PREFERRED) when each record needs different values — always \`find_many_*\` first to get current values and ids, compute the new values, then call \`upsert_many_*\` with each record's id and updated fields. ## Data Efficiency @@ -42,7 +42,7 @@ For simple CRUD operations (find/create/update/delete a record), you do NOT need - Always apply filters to narrow results — don't fetch all records of a type. - Fetch one type of data at a time and check if you have what you need before fetching more. - Every record returned consumes context. Fetching too many records at once will cause failures. -- For multiple items of the same type, use batch tools (\`create_many_*\`, \`update_many_*\`, \`upsert_many_*\`, etc.) instead of looping single-item calls. +- For multiple items of the same type, use batch tools (\`create_many_*\`, \`upsert_many_*\`, \`update_many_*\`, etc.) instead of looping single-item calls. Prefer \`upsert_many_*\` over \`update_many_*\` for per-record updates. ## Tool Strategy