perf(cli): parallelize github/npm/llms-txt fetches in sync command#8
Merged
amondnet merged 6 commits intoApr 7, 2026
Merged
Conversation
Refs #2 `syncCmd` previously iterated `config.docs` strictly serially. With many github entries each requiring a tarball download + ls-remote round-trip (~3-5s each), total wall time grew linearly with entry count. This change: * Adds `runWithConcurrency` (src/concurrency.ts) — a tiny inline limiter that preserves input order, caps in-flight workers, and propagates errors to the caller (catch-and-continue is layered above). * Extracts `syncEntry` from the syncCmd loop. Same write order as before (saveDocs → addDocEntry → upsertLockEntry → removeDocs(old) → generateSkill); same log lines; never throws (failures returned as 'failed' status). * Partitions `config.docs` into a parallel-safe group (github / npm / llms-txt, concurrency=5) and a serial group (web — kept serial to remain polite to upstream documentation servers). * Exports `runSync(projectDir, options)` so the partition + concurrency behavior is testable without spawning a child process. The bare `syncCmd` definition is now a thin wrapper. * Guards `runMain(main)` behind an entry-point check so the module is importable from tests without triggering CLI execution. Concurrency safety audit (T-2): `addDocEntry` and `upsertLockEntry` both perform fully synchronous read-modify-write on the .ask files. There is no `await` between read and write, so Node's single-threaded event loop guarantees they execute atomically with respect to each other even when called from parallel workers. Tests: * test/concurrency.test.ts — 8 cases (empty / single / order preservation / limit=1 serial / bounded parallel / unbounded / capped at items.length / error propagation) * test/sync-partition.test.ts — 5 cases (mixed parallel+serial, cap at 5, catch-and-continue with one failure, empty config, all-web serial) Out of scope (per spec): generic readJson/writeJson helpers in io.ts, batching lock+config writes, --concurrency CLI flag.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Deploying ask-registry with
|
| Latest commit: |
823caae
|
| Status: | ⚡️ Build in progress... |
The previous `file://${process.argv[1]}` template-string comparison
broke on Windows (backslash paths produce `file://C:\path\…` which
never matches `file:///C:/path/…`) and on POSIX paths containing
spaces or unicode (no URL encoding). Use `pathToFileURL` instead.
Caught by /review:code-review (confidence 85).
This was referenced Apr 8, 2026
Merged
Merged
Merged
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
1 participant
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.
Closes #2
Summary
syncCmdpreviously iteratedconfig.docsstrictly serially, so wall time grew linearly with the number of github/npm entries. This change parallelizes network-bound work with bounded concurrency while preserving every existing safety invariant.runWithConcurrency(src/concurrency.ts) — tiny inline limiter, no new dependency. Preserves input order, caps in-flight workers, propagates errors to the caller.syncEntryextracted from thesyncCmdloop. Same write order (saveDocs → addDocEntry → upsertLockEntry → removeDocs(old) → generateSkill), same log lines, never throws (failures returned as'failed'status so a single bad entry can't abort the batch).config.docsinto a parallel-safe group (github,npm,llms-txt, concurrency=5) and a serial group (web— kept serial to remain polite to upstream documentation servers).runSync(projectDir, options)exported so the partition + concurrency behavior is testable without a child process. The baresyncCmddefinition is now a thin wrapper.runMainguarded behind an entry-point check so the module is importable from tests without triggering CLI execution.Concurrency safety audit (T-2)
addDocEntryandupsertLockEntryboth perform fully synchronous read-modify-write on the.ask/files (fs.readFileSync→ mutate →fs.writeFileSync). There is noawaitbetween read and write, so Node's single-threaded event loop guarantees they execute atomically with respect to each other even when called from parallel workers. Documented inline next tosyncEntry.Tests
test/concurrency.test.ts— 8 cases (empty / single / order preservation /limit=1strict serial / bounded parallel / unbounded / capped atitems.length/ error propagation)test/sync-partition.test.ts— 5 cases (mixed parallel+serial with overlap assertion, cap at 5 with 12 entries, catch-and-continue with one failing entry, empty config, all-web serial)All 13 new tests pass. Lint and build clean.
Out of scope (per spec)
readJson<T>/writeJson<T>helpers inio.ts--concurrencyCLI flag (hard-coded 5 for now)Test plan