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
This commit is contained in:
+5
-3
@@ -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:`,
|
||||
|
||||
+7
-7
@@ -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);
|
||||
|
||||
|
||||
+9
-6
@@ -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;
|
||||
|
||||
+25
-17
@@ -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<ToolIndexEntry[]> {
|
||||
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<Map<string, object>> {
|
||||
const index = await this.getCatalog(context);
|
||||
async resolveSchemas({
|
||||
toolNames,
|
||||
context,
|
||||
precomputedCatalog,
|
||||
}: {
|
||||
toolNames: string[];
|
||||
context: ToolProviderContext;
|
||||
precomputedCatalog?: ToolIndexEntry[];
|
||||
}): Promise<Map<string, object>> {
|
||||
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<string, ToolIndexEntry[]>();
|
||||
|
||||
for (const entry of matchingEntries) {
|
||||
@@ -176,11 +176,15 @@ export class ToolRegistryService {
|
||||
): Promise<ToolSet> {
|
||||
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<string, object> | 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) => {
|
||||
|
||||
+2
-2
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user