Skip to content

Perf: compile all discovered agent entrypoints in one esbuild build - #814

Open
AmaadMartin wants to merge 6 commits into
mainfrom
feat/batch-agent-loader-esbuild-builds
Open

Perf: compile all discovered agent entrypoints in one esbuild build#814
AmaadMartin wants to merge 6 commits into
mainfrom
feat/batch-agent-loader-esbuild-builds

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):
    N/A
  2. Or, if no issue exists, describe the change:
    Problem: AgentLoader discovery runs one independent esbuild.build() per discovered entrypoint. Every build re-resolves and re-parses the same dependency graph. The plugin that rewrites __dirname closes over one file path, which is what makes a shared build impossible.

Solution: The loader now discovers every entrypoint first, then compiles the whole batch with one esbuild.build() per output module format (one build in the common case, two for a directory that mixes CommonJS and ESM). replaceDirnamePlugin takes a map of entry directories instead of one path. Each agent keeps its own temp directory and output file, so the emitted bundles do not change. The esbuild concern moves to a new dev/src/utils/agent_compiler.ts, because it grew to about 200 lines inside a module whose job is loading agents.

Measured effect (esbuild 0.25.12, node v22.22.2, Linux, warm, 9 repetitions in a fresh process each, fixture tests/integration/app_loader/discovery):

phase before after
compile, 4 entrypoints min 708 / median 751 ms min 546 / median 573 ms
import the 4 bundles median 2263 ms median 2256 ms
listAgents() end to end, 4 entrypoints min 2823 / median 2862 ms min 2686 / median 2721 ms
listAgents() end to end, 1 entrypoint min 1057 / median 1072 ms min 1073 / median 1085 ms

The compile phase gets about 24% faster, but it is only about a quarter of listAgents(). End to end that is about 140 ms of 2.86 s, or about 5%. The dominant cost is importing four separate ~5.7 MB bundles, and this change does not touch it. The single-entrypoint case is about 13 ms slower (about 1%), which is the cost of the extra scratch directory and the rename.

The four bundles are byte-identical before and after (same SHA-256 for all four).

Known behaviour change: a file deleted between the directory scan and the build used to be skipped, because the stat in load() raised AgentFileLoadingError. It now fails the batch, because compilation happens before that stat. This is a development-time race and the file watcher re-runs discovery.

Disclosed test edits: about a dozen existing test bodies changed. dev/test/utils/agent_loader_test.ts mocks esbuild.build and asserts the option object, so it has to change with the implementation. Every case is preserved. The list:

  • outfile assertions became outdir + outExtension + {in, out} entry points, and the twelve copy-pasted mock bodies became one mockEsbuildBuild helper.
  • createTempDir now returns a fresh directory per call, because the loader calls it twice per load.
  • 'uses js loader for non-ts files', 'returns js loader for mts files' and 'returns js loader for cts files' collapsed into one it.each over the three extensions. All three cases still run, under the same three names.
  • 'throws if rootAgent is not found' used rejects.toThrow(<full message>), which needs the compiled path up front; the path is now a fresh temp directory, so the test captures the rejection with .catch and asserts toBeInstanceOf(Error) plus the same full message.
  • 'does not preload agents again if already preloaded' spied on the deleted private loadAgentFromFile; it now asserts the observable thing, that a second preloadAgents() issues no further build.
  • The six replaceDirnamePlugin cases moved to dev/test/utils/agent_compiler_test.ts with the code they cover.

No test was deleted, skipped or weakened.

No new suppressions: replaceDirnamePlugin's setup now takes Pick<esbuild.PluginBuild, 'onLoad'>, the only hook it uses, so the test stub passes {onLoad: vi.fn()} directly. That deletes the six as unknown as esbuild.PluginBuild casts the old plugin tests carried. compileEntrypoints returns an array in input order rather than a Map, which removes the non-null assertion at its call sites. The diff also drops the @ts-ignore on the esbuild-shim-plugin import: the package ships index.d.ts, so the directive was dead.

Collision check: gh pr list --repo AmaadMartin/adk-js --state open --limit 400 plus gh pr diff --name-only on every open PR touching dev/src/utils/agent_loader.ts (#264, #285, #457, #480, #511, #633, #653, #664, #674, #703, #725, #726, #731, #755, #793). None batches the builds. This PR branches from main and leaves the {filter: /.*/} and getDirFiles lines alone.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

npx vitest run --project unit:dev dev/test/utils/agent_loader_test.ts dev/test/utils/agent_compiler_test.ts  # 41 passed
npx vitest run --project integration tests/integration/app_loader/app_loader_test.ts    # 6 passed
npx vitest run --project integration tests/integration/agent_loader/agent_dirname_test.ts  # 3 passed
npm run lint && npm run format:check && npm run build                          # clean

tests/integration/app_loader/app_loader_test.ts is unchanged. It passed on the first revision of this branch and on CI. npm run ts:check reports 287 pre-existing errors on this branch and 287 on main, none of them in the files this PR touches. npm run docs:check passes.

Ten mutations prove the new tests fail against broken code:

mutation failing test message
plugin closes over the first entry again gives each batched entry its own directory expected undefined to be 'js'
groupByModuleType returns one group builds cjs and esm entrypoints in separate builds expected "spy" to be called 2 times, but got 1 times
groupByModuleType returns one group per entry compiles every discovered entrypoint in one build expected "spy" to be called 1 times, but got 3 times
drop the catch cleanup removes every temp directory when the build fails promise resolved "undefined" instead of rejecting
share one output directory gives every entrypoint its own output file and directory expected 1 to be 3
drop removeFolder(buildDir) removes the batch scratch directory after a successful preload promise resolved "undefined" instead of rejecting
drop the shouldCompile guard skips the build when compile and bundle are both off expected "spy" to not be called at all, but actually been called 1 times
drop the empty-batch guard creates no build for a directory with no entrypoint expected [ { …(2) } ] to deeply equal []
swallow a non-loading error propagates an import error that is not an agent loading error promise resolved "[]" instead of rejecting
hand every agent the first artifact (compiled[0]) lists all agents, and 9 more ENOENT: no such file or directory, unlink '.../agent-HA7it6/agent1.cjs'

Every line and branch this PR adds to dev/src/utils/agent_loader.ts and dev/src/utils/agent_compiler.ts is covered, except four helpers moved verbatim out of agent_loader.ts (getFileModuleType, getTypeFromPackageJson, linkProjectNodeModules, getProjectNodeModulesDir). Their coverage is unchanged; the move is byte-identical.

Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

cd tests/integration/app_loader/discovery && npm install && cd -
npm run build
node dev/dist/esm/cli_entrypoint.js api_server tests/integration/app_loader/discovery --port 8123
curl http://localhost:8123/list-apps

Returns ["service_alpha","service_beta","standalone_agent","standalone_app"], the same four names as before.

adk run on a single agent file exercises the batch-of-one path:

cd <dir with one agent file>
node <repo>/dev/dist/esm/cli_entrypoint.js run standalone_agent.ts

Prints Running agent standalone_agent_name, type exit to exit.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.

Amaad Martin added 6 commits August 8, 2026 08:49
AgentLoader discovery ran one independent esbuild.build() per entrypoint,
so every build re-resolved and re-parsed the same dependency graph. The
loader now discovers first, then compiles the whole batch with one build
per output module format.

Each agent keeps its own temp directory and output file, so the emitted
bundles are unchanged. The replace-dirname plugin takes a map of entry
directories instead of one closed-over path, which is what made a batched
build impossible before.
The esbuild cluster in agent_loader.ts grew to about 200 lines and no
longer belongs in a module whose job is loading agents. It now lives in
dev/src/utils/agent_compiler.ts, together with the module-type and
node_modules helpers only it uses, and the plugin tests move with it.

compileEntrypoints returns an array in input order instead of a Map, so
callers index it and no longer need a non-null assertion. The plugin's
setup() takes Pick<PluginBuild, 'onLoad'>, the only hook it uses, so a
test stub needs no cast. The batch scratch directory prefix is now
adk_agent_build, which no longer prefix-matches adk_agent_loader.
esbuild-shim-plugin ships index.d.ts, so the suppression silenced
nothing. tsc reports the equivalent @ts-expect-error as unused.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant