Skip to content

perf(executors): lazy-load the executor registry — defer class imports + construction to first use (#11220) - #11421

Merged
diegosouzapw merged 4 commits into
diegosouzapw:release/v3.8.51from
oyi77:perf/lazy-executor-registry
Aug 25, 2026
Merged

diegosouzapw merged 4 commits into
diegosouzapw:release/v3.8.51from
oyi77:perf/lazy-executor-registry

Conversation

@oyi77

@oyi77 oyi77 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements the on-demand executor loading proposed in #11220 (the lazy PROVIDERS Proxy facet already exists upstream in open-sse/config/constants.ts:74; this PR covers the remaining two facets: the executor barrel and per-use construction).

Measured problem (isolated DATA_DIR, tsx/esm)

Phase Before After
Barrel import (all ~100 executors) 1295–1923 ms, ~65 MB heap 712–832 ms, ~45 MB
Marginal cost of the ~100 unused executors at boot ~0.7–1.2 s + ~35 MB ~0
First use of one executor — +120–150 ms, once

The barrel previously pulled every executor module and constructed every instance at boot even for deployments using a handful of providers.

Design

  • executors/index.ts: the declarative alias table stays byte-stable — same keys, same order, same ctor args — but each value is now a deferred loader (() => import("./x.ts").then(m => new m.XExecutor(...))). Bundlers split these into on-demand chunks.
  • executors/registry.ts: new registerLazyExecutor / loadRegisteredExecutor. Aliases are declared EAGERLY so hasSpecializedExecutor() and listExecutorAliases() remain synchronous and include lazy aliases; the instance materializes once on first use and caches into the same registry map a static registration would have used. Concurrent first uses share one in-flight load.
  • getExecutor() is now async; all production call sites await it: chatCore proxy resolver (direct/cliproxyapi/dario/fallback legs), video generation (Veo free), compression judge + eval clients (resolved per call inside complete(), keeping the sync factory signatures), quotaAutoPing deps type, anthropic OAuth validation.
  • The cliproxy wrapper ExecutorLike types drop their [key: string]: unknown index signatures so BaseExecutor satisfies them structurally under the newly-explicit return type.

Contract preserved

The golden lock (tests/unit/executor-map-golden.test.ts) passes with a byte-identical snapshot: keys, classes, provider identities and dispatch guards unchanged. Each alias still gets its OWN instance (aliases never share), matching previous semantics.

Test impact

24 unit suites adapted mechanically to the async seam: await getExecutor(...), async callbacks, union narrowing on the Response | { response } execute result, and class imports moved from the barrel to their module files. Guard tests converted from assert.throws to await assert.rejects with identical status/message assertions.

Verification:

  • golden locks (executor-map / g13-combo-chatcore / provider-translate-path): 8/8 pass
  • updated executor suites: 244/244 + 152/152 pass
  • executor-web-cookie-sweep SIGABRT failure is PRE-EXISTING — reproduced identically on the stashed clean base
  • pre-commit lint-staged suppression drift also reproduced on clean base (documented in commit)

Base: release/v3.8.51 (3192eb88d).

@oyi77
oyi77 requested a review from diegosouzapw as a code owner August 24, 2026 18:04
@oyi77

oyi77 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

CI triage + real fix pushed (514a0fd)

Genuine regression — FIXED in this push

Round 1 of the test migration only covered suites importing from executors/index.ts directly. Suites importing getExecutor through other paths (open-sse root re-export, dynamic imports inside tests) still called it synchronously → Promise instanceof X assertion failures (e.g. providers-yuanbao-web). A repo-wide sweep found 35 more suites; all adapted mechanically in 514a0fd. Locally verified green: providers-yuanbao-web, ninerouter-executor, provider-request-failure-pipeline, provider-limits-accesstoken-fallback-on-refresh-failure, executor-registry, chatcore-executor-proxy, poe-api-executor-regression, web-cookie-providers-new — 113/113 pass.

Remaining red checks after that fix

Check Verdict Evidence
No new ESLint warnings / Fast Quality Gates Pre-existing on base "suppressions left that do not occur anymore" reproduced verbatim on stashed clean release/v3.8.51. Green PRs (#11428/#11429/#11434) target release/v3.8.50 with different suppression state.
dast-smoke Does not reproduce from this diff Fuzz 500s hit DELETE /api/keys/{id}, OPTIONS /api/keys, OPTIONS /api/auth/logout; probed locally against this branch: 204/401/204 (documented statuses). Unrelated to executor resolution.
Any residual unit-shard failures Flakes ollama-local-embedding-2824 (rmSync ENOTEMPTY race in its own after-hook) and token-health-check-kimi (jitter timing) both fail at most 1/8500 tests per shard, in files untouched here, and pass locally.

@diegosouzapw

Copy link
Copy Markdown
Owner

Solid piece of engineering — the eager-alias/lazy-instance registry design preserves the golden snapshot and we confirmed check:cycles and the golden locks pass on your branch. Two mechanical blockers, both reproduced locally: (1) check:known-symbols hard-fails because it parses the literal name const executors = { which you renamed to lazyExecutors — please update scripts/check/check-known-symbols.ts (or keep the name); (2) dropping the eslint-disable-next-line in EditConnectionModal.tsx introduces a new set-state-in-effect error and is your only conflict with the base — restoring the comment resolves both problems at once.

oyi77 added 3 commits August 25, 2026 14:34
…s + construction to first use (diegosouzapw#11220)

The executor barrel statically imported ~100 executor modules and
constructed every instance at module load. Measured cold cost on top of
the minimal set: ~0.7–1.2s boot time and ~35MB heap, paid by every
deployment regardless of which providers it uses.

Now:
- executors/index.ts keeps the declarative alias table byte-stable (same
  keys, same order, same ctor args — pinned by the golden lock) but each
  value is a deferred loader using dynamic import; bundlers emit
  on-demand chunks
- registry.ts gains registerLazyExecutor/loadRegisteredExecutor: aliases
  are declared eagerly so hasSpecializedExecutor() and
  listExecutorAliases() stay synchronous, instances materialize once on
  first use and cache into the same registry map
- getExecutor() becomes async; production call sites (chatCore proxy
  resolver, video generation, compression judge/eval clients,
  quotaAutoPing deps, anthropic OAuth validation) await it
- cliproxy wrapper ExecutorLike types drop their index signatures so
  BaseExecutor satisfies them structurally

Measured after (isolated DATA_DIR): barrel boot 712-832ms / ~45MB with
first-use materialization of an executor costing +120-150ms once.

Test impact: 24 unit suites adapted mechanically to the async seam
(await + union narrowing on the Response | {response} execute result);
class imports moved from the barrel to executor module files. The
web-cookie sweep SIGABRT failure is pre-existing (reproduced identically
on the clean base).

Commit gate note: husky lint-staged fails with 'suppressions left that
do not occur anymore' — reproduced identically on a stashed clean tree
(22 baseline problems), independent of this change.
… (round 2)

Round 1 only covered suites importing from executors/index.ts directly;
suites importing getExecutor through other paths (open-sse root
re-export, dynamic imports inside tests) still called it synchronously,
producing Promise-vs-instance assertion failures in CI (e.g.
providers-yuanbao-web 'instanceof YuanbaoWebExecutor' false).

Sweep method: repo-wide scan for un-awaited getExecutor call sites,
adapted mechanically (await + async callbacks + Response|{response}
union narrowing); assert.rejects guards with promise-returning
callbacks verified correct as-is.

Locally verified green: providers-yuanbao-web, ninerouter-executor,
provider-request-failure-pipeline, provider-limits-accesstoken-fallback,
executor-registry, chatcore-executor-proxy, poe-api-executor-regression,
web-cookie-providers-new (113/113).
… tests

Without wrapping parens, .execute bound to the Promise returned by
getExecutor(), not the resolved Executor instance, causing:
  TypeError: getExecutor(...).execute is not a function

14 sites across 3 test files fixed:
  command-code-maxtokens-negative-5166.test.ts: 1
  command-code-user-array-5166.test.ts: 4
  command-code-vision.test.ts: 9

All 17 tests pass (Node test runner).
@oyi77
oyi77 force-pushed the perf/lazy-executor-registry branch from 70fbad9 to a9237b2 Compare August 25, 2026 07:34
…notation; restore dropped merge comment in EditConnectionModal
@oyi77

oyi77 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Both blockers are fixed in 11db30065 (pushed to this branch):

1. check-known-symbols literal — took your update-the-checker option: extractExecutorAliases now matches "const lazyExecutors" instead of "const executors = {". Deliberately without the = { suffix, because commit eb144cd (#11220) left a type annotation between name and initializer (const lazyExecutors: Record<string, () => Promise<BaseExecutor>> = {), so the old full-string match could never hit again. Verified: npx tsx scripts/check/check-known-symbols.ts now gets past extraction and reaches the alias-validation stage; its remaining output is the pre-existing missing-executor set that also fails on a clean base — untouched by this branch.

2. EditConnectionModal.tsx — the dropped eslint-disable-next-line comment is restored verbatim, which was also our only conflict with the base, so both problems resolved together as you said.

On the current CI reds (Docs Gates / unit shards / Vitest / ESLint-suppressions gate): same upstream-tip drift we've been tracking — reproduced at the current merge ref with none of this branch's content: check:docs-counts fails STRICT on README/AGENTS/llm.txt still saying "159 migrations" while code has 160, and tests/unit/agent-card-route.test.ts + providers-constants-split.test.ts fail at the tip itself. They'll clear from every open PR whenever the counts bump lands.

@diegosouzapw
diegosouzapw merged commit 7cfabfc into diegosouzapw:release/v3.8.51 Aug 25, 2026
7 of 16 checks passed
diegosouzapw pushed a commit that referenced this pull request Aug 26, 2026
…ovider API access (#11455)

Validated in a combined sub-batch worktree off release/v3.8.51 tip. This PR's branch also carried ~88 already-merged commits from a stale rebase (phantom-diff); cherry-picked only the genuine value commit. That commit's own test file (command-code-executor.test.ts) predated #11421's async getExecutor() change and had 7 test failures from unresolved-Promise call sites (`.execute()` on a still-pending getExecutor() Promise, and in one case on execute() itself not being awaited) — fixed all 7 call sites to properly await both async calls, verified 16/16 pass, and pushed both the cherry-pick and the fix to this branch.
- Focused tests: command-code-executor.test.ts (16/16) + provider-validation-specialty.test.ts — 140/140 combined, part of sub-batch's full run
- typecheck:core, file-size, changelog-integrity, complexity, cognitive-complexity — all OK
- Full-repo lint: 228 pre-existing dashboard react-hooks/* findings, unrelated to this diff

Thanks for tracing the Go-plan 403 to the v3.8.50 endpoint migration and building a clean fallback that preserves the new Provider-tier path while restoring CLI compatibility for Go plan.
diegosouzapw pushed a commit that referenced this pull request Aug 26, 2026
…#11421) (#11582)

Merged via /merge-batch (lote 2026-08-26, v3.8.51). Boarded no worktree combinado junto com outras ~30 PRs; validação única: typecheck/complexity/cognitive-complexity/changelog-integrity verdes, file-size rebaseado onde necessário (crescimento legítimo), lint com os mesmos 228 achados pré-existentes confirmados via sonda contra o tip puro (não introduzidos por este lote), e ~370 testes focados (unit + vitest) passando. Obrigado pela contribuição.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…s + construction to first use (diegosouzapw#11220) (diegosouzapw#11421)

Validated in a combined 3-PR batch worktree off release/v3.8.51 tip (a sibling PR from the same author, diegosouzapw#11495, was held out — a typecheck error in zai-web.ts only reproduced with this PR + diegosouzapw#11495 boarded together, and cleared without diegosouzapw#11495; isolated this PR alone confirmed clean on its own too, so the interaction belonged to diegosouzapw#11495's side — see its comment).
- Golden lock: executor-map-golden.test.ts — passes byte-identical (same keys, classes, provider identities, dispatch guards)
- Focused tests part of batch's 94/94 node:test run
- typecheck:core, file-size, changelog-integrity, complexity, cognitive-complexity — all OK
- Full-repo lint: 228 pre-existing dashboard react-hooks/* findings, unrelated to this diff

Thanks for the measured, careful methodology here — the golden-lock contract plus the isolated DATA_DIR benchmarking make this an easy PR to trust despite the wide surface (72 files).
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…ovider API access (diegosouzapw#11455)

Validated in a combined sub-batch worktree off release/v3.8.51 tip. This PR's branch also carried ~88 already-merged commits from a stale rebase (phantom-diff); cherry-picked only the genuine value commit. That commit's own test file (command-code-executor.test.ts) predated diegosouzapw#11421's async getExecutor() change and had 7 test failures from unresolved-Promise call sites (`.execute()` on a still-pending getExecutor() Promise, and in one case on execute() itself not being awaited) — fixed all 7 call sites to properly await both async calls, verified 16/16 pass, and pushed both the cherry-pick and the fix to this branch.
- Focused tests: command-code-executor.test.ts (16/16) + provider-validation-specialty.test.ts — 140/140 combined, part of sub-batch's full run
- typecheck:core, file-size, changelog-integrity, complexity, cognitive-complexity — all OK
- Full-repo lint: 228 pre-existing dashboard react-hooks/* findings, unrelated to this diff

Thanks for tracing the Go-plan 403 to the v3.8.50 endpoint migration and building a clean fallback that preserves the new Provider-tier path while restoring CLI compatibility for Go plan.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…diegosouzapw#11421) (diegosouzapw#11582)

Merged via /merge-batch (lote 2026-08-26, v3.8.51). Boarded no worktree combinado junto com outras ~30 PRs; validação única: typecheck/complexity/cognitive-complexity/changelog-integrity verdes, file-size rebaseado onde necessário (crescimento legítimo), lint com os mesmos 228 achados pré-existentes confirmados via sonda contra o tip puro (não introduzidos por este lote), e ~370 testes focados (unit + vitest) passando. Obrigado pela contribuição.
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.

2 participants