perf(ci): shard the general-server test lane across 3 runners (#8360)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Every PR runs the `PR` GitHub Actions workflow, whose `verify` gate fans out into parallel test lanes (general tests, serialized server route suites, build, typecheck) > - The `General tests (server)` lane had grown into the run's critical path: it executed all ~213 non-route server suites serially in a single job (~7.2m of test time), more than 2x any other job > - It runs serially because `server/vitest.config.ts` pins `maxWorkers: 1`, so server suites cannot parallelize within a single runner — the only lever is spreading them across runners > - This pull request shards that lane into 3 even partitions that run on separate runners, mirroring the 4-way sharding already used for the serialized route suites > - The benefit is the lane drops from ~7.7m to ~2.4m/shard, cutting overall PR wall time roughly in half (~8.5m → ~4.2m) ## Linked Issues or Issue Description No public GitHub issue exists for this work, so the underlying issue is described inline following the feature-request template. ### Problem or motivation PR CI wall time had crept back up to ~8.5m. On a recent fully-green run, the `General tests (server)` job took 7.72m — more than double any other job and the clear critical path. Of that, 7.23m was pure test execution (dependency install was a cached 0.27m). The job ran all server suites that are not route/authz tests (213 files) one after another, because the server vitest project pins `maxWorkers: 1`, making these suites inherently serial within a single runner. ### Proposed solution Shard the general-server lane across 3 parallel runners — the same technique the route/authz suites already use — so the suite set is split into even, deterministic partitions that run concurrently. Add a regression test that proves the shards always cover the full suite set with no gaps or overlap. ### Alternatives considered - **Raise `maxWorkers` for the server project** to parallelize within one runner — rejected: the server suites share process-level state (DB/port), which is exactly why `maxWorkers: 1` is pinned. - **Two shards instead of three** — would leave the lane at ~3.6m, still above the next bottleneck (Canary Dry Run, ~4.1m wouldn't be the gate). Three lands the lane comfortably below it. - **Do nothing / accept the slow lane** — rejected: it gates every PR. ### Roadmap alignment Developer-experience / CI tooling. Not core product roadmap work; does not overlap with planned features in `ROADMAP.md`. ## What Changed - `scripts/run-vitest-stable.mjs`: the `general-server` general-test group now accepts `--shard-index` / `--shard-count`. It enumerates the full server test set (the whole `server/src` tree, minus the route/authz suites that already run in their own serialized shards) and splits it deterministically by modulo. The non-sharded local invocation (`pnpm test:run:general --group general-server`) is unchanged. - `.github/workflows/pr.yml`: the `general_tests` matrix runs `general-server` as 3 parallel shards (1/3, 2/3, 3/3). Workspace groups are unchanged. The `verify` gate already aggregates the whole matrix result, so the required check name is unaffected. - `scripts/__tests__/run-vitest-stable-shard.test.mjs`: a `node:test` suite asserting the 3 shards form a complete, non-overlapping partition of the general-server set, that no route/authz suite leaks into it, and that shard flags are rejected for the parallel workspace groups. Wired into the `policy` job. ## Verification - New partition test passes locally: `node --test ./scripts/__tests__/run-vitest-stable-shard.test.mjs` (3/3). - Confirmed the 3 shards form a complete, non-overlapping partition of all 213 files (71/71/71). - Ran a live thin shard (3 real server suites, including one outside `__tests__`) — 23 tests passed, confirming positional-include execution works end to end. - This PR's own CI is the authoritative check: all three `General tests (server (n/3))` jobs went green on the prior run, collectively covering every suite the old single job ran. ## Risks - Low risk. No product code changes — only test orchestration and CI matrix. Shard partitioning is deterministic and is now covered by an automated test that fails if the partition ever develops a gap or overlap. Modulo-on-sorted-filenames balances duration reasonably, matching the approach already proven by the serialized route shards. ## Model Used - Claude (Anthropic), `claude-opus-4-8`, extended thinking + tool use (agentic coding via Paperclip). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots (N/A — no UI change) - [x] I have updated relevant documentation to reflect my changes (inline comments explain the sharding rationale) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (pending this PR's run) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending review) - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -6,6 +6,7 @@ import path from "node:path";
|
||||
|
||||
const repoRoot = process.cwd();
|
||||
const serverRoot = path.join(repoRoot, "server");
|
||||
const serverSrcDir = path.join(repoRoot, "server", "src");
|
||||
const serverTestsDir = path.join(repoRoot, "server", "src", "__tests__");
|
||||
const nonServerProjects = [
|
||||
"@paperclipai/shared",
|
||||
@@ -198,8 +199,13 @@ function parseCliOptions(argv) {
|
||||
fail("--shard-index and --shard-count must be provided together.");
|
||||
}
|
||||
|
||||
if (mode !== serializedModeName && shardIndex !== null) {
|
||||
fail("--shard-index/--shard-count are only valid with --mode serialized.");
|
||||
const shardAllowed =
|
||||
mode === serializedModeName ||
|
||||
(mode === generalModeName && group === generalServerGroupName);
|
||||
if (!shardAllowed && shardIndex !== null) {
|
||||
fail(
|
||||
"--shard-index/--shard-count are only valid with --mode serialized or --mode general --group general-server.",
|
||||
);
|
||||
}
|
||||
|
||||
if (group !== null && mode !== generalModeName) {
|
||||
@@ -210,17 +216,17 @@ function parseCliOptions(argv) {
|
||||
fail(`Unknown group "${group}". Expected one of: ${generalGroupNames.join(", ")}.`);
|
||||
}
|
||||
|
||||
if (mode === serializedModeName) {
|
||||
const resolvedShardCount = shardCount ?? 1;
|
||||
const resolvedShardIndex = shardIndex ?? 0;
|
||||
if (resolvedShardIndex >= resolvedShardCount) {
|
||||
fail(`--shard-index must be less than --shard-count. Received ${resolvedShardIndex} of ${resolvedShardCount}.`);
|
||||
if (shardIndex !== null) {
|
||||
if (shardIndex >= shardCount) {
|
||||
fail(`--shard-index must be less than --shard-count. Received ${shardIndex} of ${shardCount}.`);
|
||||
}
|
||||
}
|
||||
|
||||
if (mode === serializedModeName) {
|
||||
return {
|
||||
mode,
|
||||
shardIndex: resolvedShardIndex,
|
||||
shardCount: resolvedShardCount,
|
||||
shardIndex: shardIndex ?? 0,
|
||||
shardCount: shardCount ?? 1,
|
||||
group: null,
|
||||
dryRun,
|
||||
};
|
||||
@@ -228,8 +234,8 @@ function parseCliOptions(argv) {
|
||||
|
||||
return {
|
||||
mode,
|
||||
shardIndex: null,
|
||||
shardCount: null,
|
||||
shardIndex,
|
||||
shardCount,
|
||||
group,
|
||||
dryRun,
|
||||
};
|
||||
@@ -280,8 +286,31 @@ function runProjectGroup(projects, groupName) {
|
||||
}
|
||||
}
|
||||
|
||||
function runGeneralGroup(routeTests, groupName) {
|
||||
function runGeneralGroup(routeTests, groupName, shardIndex = null, shardCount = null) {
|
||||
if (groupName === generalServerGroupName) {
|
||||
if (shardCount !== null && shardCount > 1) {
|
||||
const shardFiles = generalServerTestFiles.filter(
|
||||
(_, index) => index % shardCount === shardIndex,
|
||||
);
|
||||
console.log(
|
||||
`\n[test:run] general-server shard ${shardIndex + 1}/${shardCount} running ${shardFiles.length} of ${generalServerTestFiles.length} suites`,
|
||||
);
|
||||
if (shardFiles.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
runVitest(
|
||||
[
|
||||
"--project",
|
||||
"@paperclipai/server",
|
||||
...serializedServerVitestArgs,
|
||||
...shardFiles,
|
||||
],
|
||||
`${groupName} shard ${shardIndex + 1}/${shardCount}`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const excludeRouteArgs = routeTests.flatMap((file) => ["--exclude", file.serverPath]);
|
||||
runVitest(
|
||||
[
|
||||
@@ -336,6 +365,17 @@ const routeTests = walk(serverTestsDir)
|
||||
}))
|
||||
.sort((a, b) => a.repoPath.localeCompare(b.repoPath));
|
||||
|
||||
// Every server test file that the general-server group is responsible for,
|
||||
// i.e. the whole server project minus the route/authz suites that run in the
|
||||
// dedicated serialized shards. Sharding this list across runners is what keeps
|
||||
// the general-server lane from becoming the PR critical path: the server vitest
|
||||
// config pins maxWorkers to 1, so the only way to parallelize is across jobs.
|
||||
const generalServerTestFiles = walk(serverSrcDir)
|
||||
.map((file) => toRepoPath(file))
|
||||
.filter((repoPath) => repoPath.endsWith(".test.ts"))
|
||||
.filter((repoPath) => !isRouteOrAuthzTest(repoPath))
|
||||
.sort((a, b) => a.localeCompare(b));
|
||||
|
||||
const options = parseCliOptions(process.argv.slice(2));
|
||||
if (options.dryRun) {
|
||||
const serializedSuites =
|
||||
@@ -352,6 +392,15 @@ if (options.dryRun) {
|
||||
availableGeneralGroups: generalGroupNames,
|
||||
serializedSuiteCount: routeTests.length,
|
||||
selectedSerializedSuites: serializedSuites.map((routeTest) => routeTest.repoPath),
|
||||
generalServerSuiteCount: generalServerTestFiles.length,
|
||||
selectedGeneralServerSuites:
|
||||
options.mode === generalModeName &&
|
||||
options.group === generalServerGroupName &&
|
||||
options.shardCount !== null
|
||||
? generalServerTestFiles.filter(
|
||||
(_, index) => index % options.shardCount === options.shardIndex,
|
||||
)
|
||||
: null,
|
||||
},
|
||||
null,
|
||||
2,
|
||||
@@ -362,7 +411,7 @@ if (options.dryRun) {
|
||||
|
||||
if (options.mode === generalModeName || options.mode === allModeName) {
|
||||
if (options.group) {
|
||||
runGeneralGroup(routeTests, options.group);
|
||||
runGeneralGroup(routeTests, options.group, options.shardIndex, options.shardCount);
|
||||
} else {
|
||||
runGeneralSuites(routeTests);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user