mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Cloud waits for source verification before deploying a new image. > - The slowest server verification job spends about ten minutes running tests. > - Each job uses one test worker to preserve test isolation. > - This pull request distributes those suites across ten standard hosted runners. > - The benefit is a shorter verification path with the same test coverage. ## Linked Issues or Issue Description **Current behavior** In [readiness run 34572340764](https://github.com/paperclipai/paperclip/actions/runs/34572340764), the slowest server job ran for 638 seconds. Test execution used 594 seconds. This held readiness behind the image job. **Proposed behavior** Use ten general server jobs in the reusable release verification workflow. Keep the three chat jobs and every existing prerequisite. The complete partition test verifies that no server suite is omitted or duplicated. **Reason and benefit** Reduce merge-to-deployable time on the existing runner type. The next longest prerequisite was Runner verification at 526 seconds, so the initial expected total gain is about two minutes rather than a halving of readiness time. Measure actual queue and execution time before claiming a result. Related: #13198 introduced the separate chat lane. #12577 refreshes duration estimates; this change leaves that manifest alone. ## What Changed - Increase the general server matrix from five jobs to ten. - Verify the ten-way partition covers the complete server suite when combined with the chat lane. - Document runner demand and the unchanged local and PR grouping. ## Verification - `node --test scripts/__tests__/release-verify-workflow.test.mjs scripts/__tests__/run-vitest-stable-shard.test.mjs`: 29 passed. - `actionlint .github/workflows/release-verify.yml`: passed. - Full local `pnpm -r typecheck` and `pnpm build`: passed. - All latest-head GitHub CI checks passed, including the complete Linux test partition, build, typecheck, and browser gates. Greptile: 5/5 with zero open findings. - [Ten-shard timing probe](https://github.com/paperclipai/paperclip/actions/runs/34606772388): all 16 jobs passed; slowest server job 6m 23s versus 10m 38s in the earlier five-shard sample. This compares the server lane, not total readiness, and is not a controlled same-source A/B. - The full local `pnpm test:run` is also running. It has reproduced previously observed macOS-only failures in unchanged skill-cache and native-session suites; the corresponding Linux CI suites passed. Final local results will be attached separately. No affected-workflow test failed. ## Risks Five additional concurrent jobs per release verification run increase runner demand and repeated setup work. Queueing can offset the gain. Test workers, timeouts, permissions, and readiness requirements stay unchanged. Revert the matrix and its partition test to restore the previous split. ## Model Used OpenAI GPT-6 / Codex, with reasoning, tool use, and code execution. The exact serving model identifier and context-window size are not exposed by this session. ## 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 the affected workflow tests locally and they pass; full-suite macOS limitations are disclosed above - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
319 lines
14 KiB
JavaScript
319 lines
14 KiB
JavaScript
import assert from "node:assert/strict";
|
|
import { spawnSync } from "node:child_process";
|
|
import path from "node:path";
|
|
import { fileURLToPath } from "node:url";
|
|
import test from "node:test";
|
|
|
|
import {
|
|
defaultSuiteWeight,
|
|
loadShardDurations,
|
|
partitionGeneralServerSuites,
|
|
} from "../general-server-shard.mjs";
|
|
|
|
import { assertSelectedTests, partitionTestLines } from "../test-line-shard.mjs";
|
|
|
|
const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", "..");
|
|
const script = path.join(repoRoot, "scripts", "run-vitest-stable.mjs");
|
|
const durationsManifest = path.join(repoRoot, "scripts", "general-server-shard-durations.json");
|
|
const serializedDurationsManifest = path.join(
|
|
repoRoot,
|
|
"scripts",
|
|
"serialized-shard-durations.json",
|
|
);
|
|
|
|
function dryRun(args) {
|
|
const result = spawnSync(process.execPath, [script, ...args, "--dry-run"], {
|
|
cwd: repoRoot,
|
|
encoding: "utf8",
|
|
});
|
|
return result;
|
|
}
|
|
|
|
function dryRunJson(args) {
|
|
const result = dryRun(args);
|
|
assert.equal(result.status, 0, `expected success for ${args.join(" ")}: ${result.stderr}`);
|
|
return JSON.parse(result.stdout);
|
|
}
|
|
|
|
const SHARD_COUNT = 5;
|
|
const SERIALIZED_SHARD_COUNT = 5;
|
|
|
|
|
|
test("the serialized shards form a complete, non-overlapping partition", () => {
|
|
const shards = Array.from({ length: SERIALIZED_SHARD_COUNT }, (_, index) =>
|
|
dryRunJson(["--mode", "serialized", "--shard-index", String(index), "--shard-count", String(SERIALIZED_SHARD_COUNT)]),
|
|
);
|
|
|
|
const total = shards[0].serializedSuiteCount;
|
|
const selected = shards.flatMap((shard) => shard.selectedSerializedSuites);
|
|
assert.equal(selected.length, total, "every serialized suite must be selected exactly once");
|
|
assert.equal(new Set(selected).size, total, "serialized shards must not overlap");
|
|
});
|
|
|
|
test("the general-server shards form a complete, non-overlapping partition", () => {
|
|
const shards = Array.from({ length: SHARD_COUNT }, (_, index) =>
|
|
dryRunJson(["--mode", "general", "--group", "general-server", "--shard-index", String(index), "--shard-count", String(SHARD_COUNT)]),
|
|
);
|
|
|
|
const total = shards[0].generalServerSuiteCount;
|
|
assert.ok(total > 0, "expected a non-empty general-server suite set");
|
|
|
|
const seen = new Set();
|
|
let selectedTotal = 0;
|
|
for (const shard of shards) {
|
|
assert.equal(shard.generalServerSuiteCount, total, "suite count must be stable across shards");
|
|
for (const file of shard.selectedGeneralServerSuites) {
|
|
assert.ok(!seen.has(file), `suite assigned to more than one shard: ${file}`);
|
|
seen.add(file);
|
|
selectedTotal += 1;
|
|
}
|
|
}
|
|
|
|
// Every suite runs exactly once: union covers the whole set with no overlap.
|
|
assert.equal(selectedTotal, total, "every suite must be selected exactly once");
|
|
assert.equal(seen.size, total, "union of shards must cover the whole suite set");
|
|
});
|
|
|
|
test("a route/authz suite never leaks into the general-server shards", () => {
|
|
const shard = dryRunJson(["--mode", "general", "--group", "general-server", "--shard-index", "0", "--shard-count", SHARD_COUNT.toString()]);
|
|
for (const file of shard.selectedGeneralServerSuites) {
|
|
assert.ok(
|
|
!/[^/]*(?:route|routes|authz)[^/]*\.test\.ts$/.test(file),
|
|
`route/authz suite must stay in the serialized lane, not general-server: ${file}`,
|
|
);
|
|
}
|
|
});
|
|
|
|
test("shard flags are rejected for the workspaces-b group", () => {
|
|
const result = dryRun(["--mode", "general", "--group", "general-workspaces-b", "--shard-index", "0", "--shard-count", "3"]);
|
|
assert.notEqual(result.status, 0, "workspaces-b must not accept shard flags");
|
|
});
|
|
|
|
test("workspaces-a shards map to Vitest native --shard slices over a stable project list", () => {
|
|
const shards = [0, 1].map((index) =>
|
|
dryRunJson([
|
|
"--mode", "general", "--group", "general-workspaces-a",
|
|
"--shard-index", String(index), "--shard-count", "2",
|
|
]),
|
|
);
|
|
|
|
assert.deepEqual(
|
|
shards.map((shard) => shard.workspacesVitestShard),
|
|
["1/2", "2/2"],
|
|
"each matrix job must pass its own --shard slice to vitest",
|
|
);
|
|
// Vitest's --shard partitions each project's file list deterministically, so
|
|
// an identical project list across jobs is what guarantees complete,
|
|
// non-overlapping coverage of the lane.
|
|
assert.deepEqual(shards[0].workspaceProjects, shards[1].workspaceProjects);
|
|
assert.ok(shards[0].workspaceProjects.length > 0, "workspaces-a must run at least one project");
|
|
|
|
const unsharded = dryRunJson(["--mode", "general", "--group", "general-workspaces-a"]);
|
|
assert.deepEqual(
|
|
unsharded.workspaceProjects,
|
|
shards[0].workspaceProjects,
|
|
"sharding must not change which projects the lane covers",
|
|
);
|
|
assert.equal(unsharded.workspacesVitestShard, null);
|
|
});
|
|
|
|
test("duration-aware partition balances skewed weights better than round-robin", () => {
|
|
// Round-robin puts all three heavy suites on shard 0 (indexes 0, 3, 6).
|
|
const files = ["a", "b", "c", "d", "e", "f", "g", "h", "i"];
|
|
const durations = { a: 30000, d: 30000, g: 30000, b: 100, c: 100, e: 100, f: 100, h: 100, i: 100 };
|
|
|
|
const shards = partitionGeneralServerSuites(files, 3, durations);
|
|
const totals = shards.map((shard) => shard.totalWeight);
|
|
const maxTotal = Math.max(...totals);
|
|
const minTotal = Math.min(...totals);
|
|
assert.ok(
|
|
maxTotal - minTotal <= 200,
|
|
`expected near-even shard weights, got ${totals.join(", ")}`,
|
|
);
|
|
assert.equal(
|
|
shards.flatMap((shard) => shard.files).sort().join(","),
|
|
files.join(","),
|
|
"partition must cover every file exactly once",
|
|
);
|
|
});
|
|
|
|
test("the partition is deterministic for identical inputs", () => {
|
|
const files = Array.from({ length: 50 }, (_, index) => `suite-${index}.test.ts`);
|
|
const durations = Object.fromEntries(files.map((file, index) => [file, (index * 37) % 5000]));
|
|
|
|
const first = partitionGeneralServerSuites(files, 3, durations);
|
|
const second = partitionGeneralServerSuites(files, 3, durations);
|
|
assert.deepEqual(first, second, "same inputs must always produce the same partition");
|
|
});
|
|
|
|
test("suites missing from the manifest get the median weight", () => {
|
|
assert.equal(defaultSuiteWeight({ a: 100, b: 300, c: 900 }), 300);
|
|
assert.equal(defaultSuiteWeight({ a: 100, b: 300, c: 500, d: 900 }), 400);
|
|
assert.equal(defaultSuiteWeight({}), 1000, "empty manifest falls back to a fixed weight");
|
|
});
|
|
|
|
test("a missing or malformed manifest degrades to uniform weights", () => {
|
|
assert.deepEqual(loadShardDurations(path.join(repoRoot, "scripts", "no-such-manifest.json")), {});
|
|
|
|
const files = ["a", "b", "c", "d"];
|
|
const shards = partitionGeneralServerSuites(files, 2, {});
|
|
assert.equal(shards[0].files.length + shards[1].files.length, files.length);
|
|
assert.equal(Math.abs(shards[0].files.length - shards[1].files.length), 0);
|
|
});
|
|
|
|
test("the checked-in manifest loads and covers most of the current suite set", () => {
|
|
const durations = loadShardDurations(durationsManifest);
|
|
assert.ok(Object.keys(durations).length > 0, "manifest must parse to a non-empty duration map");
|
|
|
|
const shard = dryRunJson(["--mode", "general", "--group", "general-server", "--shard-index", "0", "--shard-count", "1"]);
|
|
const currentFiles = shard.selectedGeneralServerSuites;
|
|
const known = currentFiles.filter((file) => durations[file] !== undefined).length;
|
|
assert.ok(
|
|
known / currentFiles.length >= 0.5,
|
|
`manifest is stale: only ${known} of ${currentFiles.length} suites have recorded durations — regenerate it from a recent PR run (see the manifest's $comment)`,
|
|
);
|
|
});
|
|
|
|
test("the measured chat integration cohort does not share a general-server shard", () => {
|
|
const chatSuite = "server/src/__tests__/chat-channels.integration.test.ts";
|
|
const durations = loadShardDurations(durationsManifest);
|
|
assert.ok(
|
|
Number.isFinite(durations[chatSuite]),
|
|
"the full chat cohort must have a measured duration, not the median fallback",
|
|
);
|
|
const unsharded = dryRunJson([
|
|
"--mode",
|
|
"general",
|
|
"--group",
|
|
"general-server",
|
|
"--shard-index",
|
|
"0",
|
|
"--shard-count",
|
|
"1",
|
|
]);
|
|
const shards = partitionGeneralServerSuites(
|
|
unsharded.selectedGeneralServerSuites,
|
|
SHARD_COUNT,
|
|
durations,
|
|
);
|
|
const chatShards = shards.filter((shard) => shard.files.includes(chatSuite));
|
|
assert.equal(
|
|
chatShards.length,
|
|
1,
|
|
"the full chat cohort must run exactly once",
|
|
);
|
|
assert.deepEqual(
|
|
chatShards[0].files,
|
|
[chatSuite],
|
|
"its measured cost must reserve one existing shard without other suites",
|
|
);
|
|
assert.deepEqual(
|
|
shards.flatMap((shard) => shard.files).sort(),
|
|
[...unsharded.selectedGeneralServerSuites].sort(),
|
|
"duration balancing must not omit or duplicate any general-server suite",
|
|
);
|
|
});
|
|
|
|
test("the checked-in serialized manifest loads and covers most of the current suite set", () => {
|
|
const durations = loadShardDurations(serializedDurationsManifest);
|
|
assert.ok(Object.keys(durations).length > 0, "manifest must parse to a non-empty duration map");
|
|
|
|
const shard = dryRunJson(["--mode", "serialized", "--shard-index", "0", "--shard-count", "1"]);
|
|
const currentFiles = shard.selectedSerializedSuites;
|
|
const known = currentFiles.filter((file) => durations[file] !== undefined).length;
|
|
assert.ok(
|
|
known / currentFiles.length >= 0.5,
|
|
`manifest is stale: only ${known} of ${currentFiles.length} suites have recorded durations — regenerate it from a recent PR run (see the manifest's $comment)`,
|
|
);
|
|
});
|
|
|
|
test("the real serialized shard partition is duration-balanced", () => {
|
|
const durations = loadShardDurations(serializedDurationsManifest);
|
|
const fallback = defaultSuiteWeight(durations);
|
|
const shards = Array.from({ length: SERIALIZED_SHARD_COUNT }, (_, index) =>
|
|
dryRunJson(["--mode", "serialized", "--shard-index", String(index), "--shard-count", String(SERIALIZED_SHARD_COUNT)]),
|
|
);
|
|
|
|
const totals = shards.map((shard) =>
|
|
shard.selectedSerializedSuites.reduce((sum, file) => sum + (durations[file] ?? fallback), 0),
|
|
);
|
|
const maxTotal = Math.max(...totals);
|
|
const minTotal = Math.min(...totals);
|
|
// LPT keeps the spread within the heaviest single suite; use that as the bound.
|
|
const heaviest = Math.max(...Object.values(durations));
|
|
assert.ok(
|
|
maxTotal - minTotal <= heaviest,
|
|
`serialized shard weight spread ${maxTotal - minTotal}ms exceeds heaviest suite ${heaviest}ms: ${totals.join(", ")}`,
|
|
);
|
|
});
|
|
|
|
test("the real shard partition is duration-balanced", () => {
|
|
const durations = loadShardDurations(durationsManifest);
|
|
const fallback = defaultSuiteWeight(durations);
|
|
const shards = Array.from({ length: SHARD_COUNT }, (_, index) =>
|
|
dryRunJson(["--mode", "general", "--group", "general-server", "--shard-index", String(index), "--shard-count", String(SHARD_COUNT)]),
|
|
);
|
|
|
|
const totals = shards.map((shard) =>
|
|
shard.selectedGeneralServerSuites.reduce((sum, file) => sum + (durations[file] ?? fallback), 0),
|
|
);
|
|
const maxTotal = Math.max(...totals);
|
|
const minTotal = Math.min(...totals);
|
|
// LPT keeps the spread within the heaviest single suite; use that as the bound.
|
|
const heaviest = Math.max(...Object.values(durations));
|
|
assert.ok(
|
|
maxTotal - minTotal <= heaviest,
|
|
`shard weight spread ${maxTotal - minTotal}ms exceeds heaviest suite ${heaviest}ms: ${totals.join(", ")}`,
|
|
);
|
|
});
|
|
|
|
|
|
test("release server shards plus the dedicated chat file cover the original server group exactly", () => {
|
|
const full = dryRunJson(["--mode", "general", "--group", "general-server", "--shard-index", "0", "--shard-count", "1"]);
|
|
const shards = Array.from({ length: 10 }, (_, index) => dryRunJson([
|
|
"--mode", "general", "--group", "general-server-without-chat",
|
|
"--shard-index", String(index), "--shard-count", "10",
|
|
]));
|
|
const files = shards.flatMap((shard) => shard.selectedGeneralServerSuites);
|
|
const chat = "server/src/__tests__/chat-channels.integration.test.ts";
|
|
assert.ok(!files.includes(chat));
|
|
assert.deepEqual([...files, chat].sort(), full.selectedGeneralServerSuites.sort());
|
|
assert.equal(new Set(files).size, files.length);
|
|
const defaultRun = dryRunJson([]);
|
|
assert.ok(defaultRun.generalServerSuiteCount === full.generalServerSuiteCount);
|
|
});
|
|
|
|
const lineShardFile = path.join(repoRoot, "server/src/__tests__/chat-channels.integration.test.ts");
|
|
const caseAt = (line, name) => ({ name, file: lineShardFile, projectName: "@paperclipai/server", location: { line, column: 3 } });
|
|
|
|
test("test-line shards cover nested and parameterized cases exactly once without splitting a source line", () => {
|
|
const cases = [caseAt(10, "suite > nested > first"), caseAt(10, "suite > nested > second"),
|
|
caseAt(20, "same name"), caseAt(30, "same name"), caseAt(40, "last"), caseAt(50, "new case")];
|
|
const shards = partitionTestLines(cases, 3, lineShardFile);
|
|
assert.deepEqual(shards.map((shard) => shard.tests.length), [2, 2, 2]);
|
|
assert.equal(shards.filter((shard) => shard.lines.includes(10)).length, 1);
|
|
assert.equal(shards.find((shard) => shard.lines.includes(10)).tests.length, 2);
|
|
assert.equal(shards.flatMap((shard) => shard.lines).length, 5);
|
|
assert.deepEqual(shards.flatMap((shard) => shard.tests).sort((a, b) => a.location.line - b.location.line), cases);
|
|
assert.deepEqual(partitionTestLines([...cases].reverse(), 3, lineShardFile).map((shard) => shard.lines), shards.map((shard) => shard.lines));
|
|
});
|
|
|
|
test("line-shard collection rejects empty, foreign, or unlocated tests and invalid shard counts", () => {
|
|
const good = caseAt(10, "valid");
|
|
for (const input of [[], null, [{ ...good, file: "/another.test.ts" }], [{ ...good, projectName: "wrong" }],
|
|
[{ ...good, location: undefined }], [{ ...good, location: { line: 0 } }], [{ ...good, name: "" }]]) {
|
|
assert.throws(() => partitionTestLines(input, 1, lineShardFile));
|
|
}
|
|
for (const count of [0, -1, 1.5, Infinity, 2]) assert.throws(() => partitionTestLines([good], count, lineShardFile));
|
|
});
|
|
|
|
test("filtered collection must match the exact assigned case identities, including duplicates", () => {
|
|
const expected = [caseAt(10, "same"), caseAt(10, "same"), caseAt(20, "nested > case")];
|
|
assertSelectedTests(expected, [...expected].reverse(), lineShardFile);
|
|
for (const actual of [expected.slice(1), [...expected, caseAt(30, "extra")],
|
|
[expected[0], expected[1], caseAt(20, "renamed")],
|
|
[expected[0], expected[1], caseAt(21, "nested > case")]]) {
|
|
assert.throws(() => assertSelectedTests(expected, actual, lineShardFile));
|
|
}
|
|
});
|