perf(imports): break the github/client <-> db/repositories cycle taxing every test import#8548
Merged
Merged
Conversation
…ng every test import The cycle cost ~1.1s of cold import under vitest and nearly every test file paid it, for trivially small reasons: repositories pulled the whole @loopover/engine barrel (~420ms) for one parser (now an additive ./parse-pull-request-target-key subpath export); gittensor/api pulled all of github/client for the PRODUCT_USER_AGENT constant (now a leaf module, re-exported from client so its other importers are unchanged); and client statically pulled repositories for two call sites, both now lazy await import() at the call site -- the idiom processors.ts already uses, module-cached after the first call. Cold-import cost under vitest: github/client 1155ms -> 37ms, gittensor/api 1119ms -> 11ms, db/repositories 1106ms -> 771ms, queue/processors 2677ms -> 1978ms. Back-to-back A/B on the identical tree: aggregate suite import time 795s -> 715s. Compounds the TestD1Database template-clone fix; also simply removes a real dependency cycle.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | c322a24 | Commit Preview URL Branch Preview URL |
Jul 24 2026, 07:29 PM |
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8548 +/- ##
==========================================
+ Coverage 89.58% 93.67% +4.08%
==========================================
Files 97 697 +600
Lines 22706 56845 +34139
Branches 3872 20164 +16292
==========================================
+ Hits 20341 53248 +32907
- Misses 2187 2613 +426
- Partials 178 984 +806
Flags with carried forward coverage won't be shown. Click here to find out more.
|
CI's validate-tests failed 3 rag-index.test.ts tests at the 15s per-test timeout: the MAX_CHUNKS_PER_REPO cap tests build up to 4000+25 real chunk rows to prove capping BEHAVIOR, which is fine locally but borderline-to-over CI's timeout on a shared runner. rag.ts gains maxChunksPerRepo()/ setMaxChunksPerRepoForTest() -- the same ...ForTest override convention as clearInstallationTokenCacheForTest -- and rag-index.ts's three cap-read sites switch from the raw constant to the getter; production is unaffected. The test file runs under a 24-row cap.
Contributor
|
Important 🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪 🔍 LoopOver is reviewing…AI analysis is in progress. This comment will update when the review is complete. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟪 Reviewing |
This was referenced Jul 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to the TestD1Database template-clone fix: with schema replay gone, the suite's next-largest fixed cost is module import time (~505s aggregate per full run). Profiling found it concentrates in one place: the
github/client ↔ db/repositoriesdependency cycle, which cost ~1.1s of cold import under vitest and taxed nearly every test file that touches the app — and it existed for trivially small reasons:db/repositoriesimported the whole@loopover/enginebarrel (~420ms of the cycle) for exactly one tiny parser. → Engine gains a./parse-pull-request-target-keysubpath export (additive; the barrel still exports it), and repositories imports the subpath.gittensor/api(imported bydb/repositories) importedgithub/clientfor ONE constant,PRODUCT_USER_AGENT. → The constant moves to a new leaf modulesrc/github/user-agent.ts;clientimports and re-exports it, so its ~8 other importers are unchanged.github/clientstatically importeddb/repositoriesfor two call sites (resolveRepoActionMode's freeze read; the suppressed-write audit hook). → Both become lazyawait import(...)at the call site — the exact idiomprocessors.tsalready uses — module-cached after first call.Measured cold-import cost under vitest (same machine):
github/clientgittensor/apidb/repositoriesqueue/processorsdb/repositories' remaining ~770ms is its own 8.3k lines + drizzle schema — reducible only by splitting the file itself (a #4013-style sequence of its own, deliberately out of scope here).Behavior is unchanged: no cycle member had import-time side effects the other depended on, both lazy call sites are in already-async functions, and killing a real import cycle is a layering improvement in its own right.
Suite-level effect, measured with a back-to-back A/B on the identical tree (stash → baseline → pop → treatment, same machine conditions): aggregate import time 795s → 715s (−80s, −10%); wall time within noise on a loaded 10-core machine but the import saving concentrates in exactly the files CI's single
validate-testsrunner grinds through. The honest framing: this is a compounding follow-up to the template-clone fix (the big win), not a second 3× — plus it kills a real dependency cycle, which is worth having at any speed.Validation
npm run typecheckclean;npm run test:coverage(full, unsharded): 21,473 passed / 0 failed on the treatment run (the baseline run of the same A/B had 2 unrelated pre-existing flakes).resolveRepoActionModeacross the actuation suites; the audit hook via github-app's dry-run suppressed-write tests).