mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
perf(tools): read tool metadata instead of the registry on client paths (#6155)
* perf(tools): read tool metadata instead of the registry on client paths Cuts the last four edges that pulled `@/tools/registry` into the workspace shell. Every workspace route drops ~4,700 modules: route before after /w (canvas) 6,592 1,908 -71% /logs 6,227 1,543 -75% /tables 5,903 1,217 -79% /files 5,996 1,310 -78% workspace layout 5,751 1,063 -82% Dev cold compile of the canvas, n=3, cache cleared between runs: before 32.3s / 31.4s / 30.1s RSS 9.0-12.5 GB after 22.4s / 22.2s / 21.6s RSS 7.8-9.2 GB That lands where the `dev:minimal` escape hatch measured (20.0s / 6.7 GB) without its downside — `dev:minimal` swaps in curated registries that drop ~250 services, whereas this keeps every tool working. Rewired: - `block-outputs` -> `getToolOutputsMetadata` (needed `outputs`) - `serializer` -> `getToolParams` (needed `params`) - `validation` -> `hasToolId` (needed existence only) - `tools/params` -> `getToolMetadata` (needed `params`, `oauth`, `name`) `tools/params.ts` was the stubborn one: `mcp-dynamic-args.tsx` imports only `formatParameterLabel` from it, so the whole registry rode in behind a string helper — the same shape as the `mergeToolParameters` edge cut earlier. Adds a third generated artifact, `tool-ids.ts` (~110 KB). Resolution needs only the key set, so `@/tools/metadata` and `@/tools/metadata-outputs` both resolve through it and stay independent of each other, and an existence check costs ~110 KB instead of ~4 MB. Behaviour preservation was the risk here: `getTool` resolves an unversioned name onto its newest version, and a plain key lookup would have silently reported 246 versioned tools as missing. `resolveToolId` is reproduced against the id set and differentially tested — 4,404 probes (every id, every stripped base name, and an unknown) comparing old vs new resolution and existence: 0 mismatches. `ToolWithParameters.toolConfig` and `SubBlocksForToolInput.toolConfig` narrow from `ToolConfig` to `ToolMetadata`. The only external reader is `tool-input.tsx`, which uses `.name`. * docs(tools): point the boundary skill at the three metadata modules The skill still routed `hasToolMetadata` and `getToolIds` to `@/tools/metadata`, but this PR moved id resolution into `@/tools/tool-ids`. Left as-is it would send the next caller to the 4 MB module for an existence check that costs 110 KB — the exact mistake the skill exists to prevent. Also records the two properties a caller can silently get wrong: lookups guard with `Object.hasOwn` (a bare bracket lookup returns inherited prototype members), and they resolve unversioned names (246 tools are versioned, and a plain lookup reports them missing rather than crashing). * fix(tools): cut the settings-route registry edge and fix serializer test mocks Two findings from review, both real. The settings route still reached the registry: settings/[section]/page.tsx -> settings.tsx -> (dynamic import) ee/access-control/components/access-control.tsx -> group-detail.tsx -> tools/utils.ts -> tools/registry.ts It reads `getTool(id)?.name` — metadata — so it moves to `getToolMetadata`. The earlier audit missed it because it walked only from the canvas route, and the edge hides behind a dynamic `import()` that a static walk skips. Serializer tests mocked the wrong module. `Serializer` now reads params via `getToolParams` from `@/tools/metadata`, but the tests still only mocked `@/tools/utils`, so they controlled nothing and passed because the real generated artifacts happen to agree with the fixtures. Adds `toolsMetadataMock` to `@sim/testing/mocks`, backed by the same `mockToolConfigs` as `toolsUtilsMock` so a test mocking both sees one consistent tool universe, and mocks it in the three serializer suites. Verified the mock is now load-bearing: pointing it at a sentinel param makes the three user-only-required validation tests fail, and restoring it returns all 110 serializer tests to green. Before this they passed either way. * fix(tools): freeze the tool id array handed out by getToolIds `getToolIds()` returned the module's internal array by reference, so a caller doing `getToolIds().sort()` would reorder it in place and silently corrupt every later lookup — the in-place-mutation footgun `.claude/rules/sim-react-performance.md` calls out. Frozen rather than copied: the array is consumed in loops, so copying would allocate on every call. Freezing makes the mutation throw instead of corrupt, and `[...getToolIds()].sort()` still works. Return type is now `readonly string[]`, so the mistake is a compile error rather than a runtime surprise. No caller mutates it today; this is closing the hole, not fixing a live bug. * test(tools): enforce that the two tool-id resolvers never diverge `resolveToolId` now exists twice on purpose — `@/tools/utils` resolves against the live registry (so a tool added before regeneration still resolves at runtime), `@/tools/tool-ids` against the generated id list (so client code resolves without importing 4,300 tools). Nothing structurally kept them in step; a change to versioning logic in one would silently drift from the other. `tool-metadata:check` now asserts they agree across every id, every stripped base name, and an unknown — 4,404 probes — and only after the staleness check passes, so a missing regeneration reports as staleness rather than as drift. Verified it fails: breaking resolution for `gmail*` exits 1; restoring it passes. It cannot live in a vitest suite. `vitest.setup.ts` globally mocks `@/tools/registry` to an empty map, so `getTool` resolves nothing there — a parity test written as a spec passes or fails for the wrong reason. Both facts are recorded where the code is. Both resolvers stay exported. An earlier pass here un-exported the `@/tools/utils` one as dead; `tools/utils.server.ts` imports it through a multi-line import that a grep missed, and `tsc` caught it. Its doc now says which resolver a caller should reach for instead of leaving two identically-named functions unexplained.
This commit is contained in:
@@ -17,9 +17,14 @@
|
||||
* a single consumer — keeping it separate means callers that only need `params`
|
||||
* or an id check don't pay for it:
|
||||
*
|
||||
* tools/generated/tool-ids.ts every registered tool id
|
||||
* tools/generated/tool-metadata.ts id -> { name, description, version, params, oauth }
|
||||
* tools/generated/tool-outputs.ts id -> outputs
|
||||
*
|
||||
* Ids are their own artifact because resolving a possibly-unversioned tool name
|
||||
* needs only the key set, and an existence check needs nothing more — so those
|
||||
* callers load ~100 KB instead of ~4 MB.
|
||||
*
|
||||
* Each artifact holds its data as one JSON string parsed at runtime — see
|
||||
* `serialize()` for why an imported `.json` or an object literal is not viable
|
||||
* at this size.
|
||||
@@ -32,11 +37,14 @@ import { mkdir, readFile, writeFile } from 'node:fs/promises'
|
||||
import { dirname, resolve } from 'node:path'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { tools } from '../apps/sim/tools/registry'
|
||||
import { hasToolId } from '../apps/sim/tools/tool-ids'
|
||||
import type { ToolConfig } from '../apps/sim/tools/types'
|
||||
import { getTool } from '../apps/sim/tools/utils'
|
||||
|
||||
const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url))
|
||||
const ROOT = resolve(SCRIPT_DIR, '..')
|
||||
const GENERATED_DIR = resolve(ROOT, 'apps/sim/tools/generated')
|
||||
const IDS_PATH = resolve(GENERATED_DIR, 'tool-ids.ts')
|
||||
const METADATA_PATH = resolve(GENERATED_DIR, 'tool-metadata.ts')
|
||||
const OUTPUTS_PATH = resolve(GENERATED_DIR, 'tool-outputs.ts')
|
||||
|
||||
@@ -141,6 +149,12 @@ function build(registry: ToolRecord) {
|
||||
}
|
||||
|
||||
return {
|
||||
ids: serializeValue(
|
||||
Object.keys(metadata),
|
||||
'toolIds',
|
||||
'/** Every registered tool id, including versioned variants. */',
|
||||
'string[]'
|
||||
),
|
||||
metadata: serialize(
|
||||
metadata,
|
||||
'toolMetadata',
|
||||
@@ -192,12 +206,16 @@ function toJsStringLiteral(json: string): string {
|
||||
* generated artifact nothing reads by eye and CI verifies wholesale.
|
||||
*/
|
||||
function serialize(entries: Record<string, unknown>, exportName: string, doc: string): string {
|
||||
const literal = toJsStringLiteral(JSON.stringify(entries))
|
||||
return serializeValue(entries, exportName, doc, 'Record<string, unknown>')
|
||||
}
|
||||
|
||||
function serializeValue(value: unknown, exportName: string, doc: string, type: string): string {
|
||||
const literal = toJsStringLiteral(JSON.stringify(value))
|
||||
return `// Generated by scripts/sync-tool-metadata.ts — do not edit.
|
||||
// Regenerate with: bun run tool-metadata:generate
|
||||
|
||||
${doc}
|
||||
const ${exportName}: Record<string, unknown> = JSON.parse(
|
||||
const ${exportName}: ${type} = JSON.parse(
|
||||
${literal}
|
||||
)
|
||||
|
||||
@@ -205,24 +223,57 @@ export default ${exportName}
|
||||
`
|
||||
}
|
||||
|
||||
/**
|
||||
* Asserts the two tool-id resolvers agree.
|
||||
*
|
||||
* `@/tools/utils` resolves against the live registry; `@/tools/tool-ids` against
|
||||
* the generated id list. Both exist deliberately — the registry-backed one keeps
|
||||
* a newly-added tool resolvable before regeneration, the list-backed one lets
|
||||
* client code resolve without importing 4,300 tools. Nothing structurally keeps
|
||||
* the two in step, so it is checked here rather than left to trust.
|
||||
*
|
||||
* Runs only after the staleness check passes, since a stale id list would
|
||||
* otherwise report a divergence that is really just a missing regeneration. It
|
||||
* cannot live in a vitest suite: `vitest.setup.ts` globally mocks
|
||||
* `@/tools/registry` to an empty map, so `getTool` resolves nothing there.
|
||||
*/
|
||||
function assertResolverParity() {
|
||||
const ids = Object.keys(tools)
|
||||
const probes = new Set([...ids, ...ids.map((id) => id.replace(/_v\d+$/, '')), '__not_a_tool__'])
|
||||
const divergent: string[] = []
|
||||
for (const probe of probes) {
|
||||
if (Boolean(getTool(probe)) !== hasToolId(probe)) divergent.push(probe)
|
||||
}
|
||||
if (divergent.length > 0) {
|
||||
throw new Error(
|
||||
`Tool id resolvers disagree on ${divergent.length} of ${probes.size} names ` +
|
||||
`(e.g. ${divergent.slice(0, 5).join(', ')}).\n` +
|
||||
'resolveToolId in apps/sim/tools/utils.ts and apps/sim/tools/tool-ids.ts have drifted.'
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
async function main() {
|
||||
const checkOnly = process.argv.includes('--check')
|
||||
const { metadata, outputs, toolCount } = build(tools as ToolRecord)
|
||||
const { ids, metadata, outputs, toolCount } = build(tools as ToolRecord)
|
||||
|
||||
if (checkOnly) {
|
||||
const [existingMetadata, existingOutputs] = await Promise.all([
|
||||
const [existingIds, existingMetadata, existingOutputs] = await Promise.all([
|
||||
readFile(IDS_PATH, 'utf8').catch(() => null),
|
||||
readFile(METADATA_PATH, 'utf8').catch(() => null),
|
||||
readFile(OUTPUTS_PATH, 'utf8').catch(() => null),
|
||||
])
|
||||
if (existingMetadata !== metadata || existingOutputs !== outputs) {
|
||||
if (existingIds !== ids || existingMetadata !== metadata || existingOutputs !== outputs) {
|
||||
throw new Error('Generated tool metadata is stale. Run: bun run tool-metadata:generate')
|
||||
}
|
||||
console.log(`✓ tool metadata in sync (${toolCount} tools)`)
|
||||
assertResolverParity()
|
||||
console.log(`✓ tool metadata in sync (${toolCount} tools), resolvers agree`)
|
||||
return
|
||||
}
|
||||
|
||||
await mkdir(GENERATED_DIR, { recursive: true })
|
||||
await Promise.all([
|
||||
writeFile(IDS_PATH, ids, 'utf8'),
|
||||
writeFile(METADATA_PATH, metadata, 'utf8'),
|
||||
writeFile(OUTPUTS_PATH, outputs, 'utf8'),
|
||||
])
|
||||
|
||||
Reference in New Issue
Block a user