mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-02 02:07:25 +08:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Its PR CI runs the general-server vitest lane pinned to `maxWorkers=1` and sharded across 3 runners (introduced in #8360) > - Suites were assigned to shards round-robin by sorted file index, so shard test time was unbalanced: a recent PR run split 73s / 153s / 115s, and the heaviest shard made "General tests (server 2/3)" the slowest check in the whole workflow at 314s wall > - The slowest shard sets the lane's wall time, so unbalanced partitions waste the other two runners and stretch the PR critical path > - This pull request replaces the round-robin assignment with a deterministic longest-processing-time partition weighted by a checked-in per-suite duration manifest > - The benefit is near-even shard weights (projected 113s / 113s / 113s with the current manifest), taking roughly 40s off the PR critical path with no reduction in coverage ## Linked Issues or Issue Description - Refs #8360 (introduced the 3-way general-server sharding this PR rebalances) - No public issue exists. Problem: the general-server test lane's round-robin shard assignment ignores per-suite duration, so one shard can carry multiple 30s+ suites while another finishes in half the time; the slowest shard alone determines the check's wall time. ## What Changed - `scripts/general-server-shard.mjs` (new): manifest loader and deterministic LPT (longest-processing-time) partitioner; suites missing from the manifest get the median recorded weight, and a missing or malformed manifest degrades to uniform weights so the lane never fails on stale data - `scripts/general-server-shard-durations.json` (new): per-suite duration manifest sampled from a real PR run (240 suites); the `$comment` field documents how to regenerate it - `scripts/run-vitest-stable.mjs`: both shard-selection sites (run and `--dry-run`) now use the balanced partition instead of index round-robin - `scripts/__tests__/run-vitest-stable-shard.test.mjs`: 6 new tests covering skew-balance vs round-robin, determinism, median fallback for unlisted suites, malformed-manifest degradation, manifest coverage of the current suite set, and real-partition balance - `server/src/__tests__/heartbeat-issue-rewake-throttle.test.ts`: hardened the `afterEach` sweep — post-run bookkeeping (run-event records, follow-up wake scheduling) can still insert rows briefly after a run reaches a terminal status, and a late insert landing between the `agent_wakeup_requests` and `agents` deletes failed teardown with a foreign-key violation on the first CI attempt of this PR; the sweep now retries so a late background write cannot take down the shard - `release-verify.yml` shares the same runner script and inherits the balancing with no workflow change ## Verification - `node --test scripts/__tests__/run-vitest-stable-shard.test.mjs` — 9/9 pass (run against current master) - `npx vitest run src/__tests__/heartbeat-issue-rewake-throttle.test.ts` — 6/6 pass against embedded Postgres with the hardened teardown - `node --test scripts/__tests__/release-verify-workflow.test.mjs` — 2/2 pass - `node scripts/run-vitest-stable.mjs --dry-run` with each shard flag shows every suite assigned exactly once across the 3 shards, with projected weights ~113s each ## Risks - Low risk: partition changes which runner executes which suite, not what runs; a completeness test asserts every suite is assigned to exactly one shard - The duration manifest will drift as suites are added/changed; unlisted suites get the median weight and a coverage test flags when the manifest covers less than half the suite set, so drift degrades balance gracefully rather than breaking the lane > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Fable 5 (`claude-fable-5`, Anthropic), extended thinking enabled, agentic tool use (file edits, shell, test execution) via Claude Code ## 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 - [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: Claude (Paperclip SWE) <noreply@paperclip.ing>
76 lines
2.7 KiB
JavaScript
76 lines
2.7 KiB
JavaScript
import { readFileSync } from "node:fs";
|
|
|
|
// Fallback weight (ms) when the duration manifest is missing or empty.
|
|
const FALLBACK_SUITE_WEIGHT_MS = 1000;
|
|
|
|
// Loads the per-suite duration manifest produced from a real PR run (see the
|
|
// $comment field in scripts/general-server-shard-durations.json). Returns an
|
|
// empty map on any read/parse problem so sharding degrades to uniform weights
|
|
// instead of failing the test lane.
|
|
export function loadShardDurations(manifestPath) {
|
|
let parsed;
|
|
try {
|
|
parsed = JSON.parse(readFileSync(manifestPath, "utf8"));
|
|
} catch {
|
|
return {};
|
|
}
|
|
|
|
const durations = parsed?.durations;
|
|
if (!durations || typeof durations !== "object" || Array.isArray(durations)) {
|
|
return {};
|
|
}
|
|
|
|
const result = {};
|
|
for (const [file, ms] of Object.entries(durations)) {
|
|
if (typeof ms === "number" && Number.isFinite(ms) && ms >= 0) {
|
|
result[file] = ms;
|
|
}
|
|
}
|
|
return result;
|
|
}
|
|
|
|
// Weight assigned to suites absent from the manifest (new or renamed files).
|
|
// The median keeps one unknown suite from skewing a shard the way a mean
|
|
// dragged up by a few 30s+ suites would.
|
|
export function defaultSuiteWeight(durations) {
|
|
const values = Object.values(durations).sort((a, b) => a - b);
|
|
if (values.length === 0) {
|
|
return FALLBACK_SUITE_WEIGHT_MS;
|
|
}
|
|
const mid = Math.floor(values.length / 2);
|
|
return values.length % 2 === 1 ? values[mid] : (values[mid - 1] + values[mid]) / 2;
|
|
}
|
|
|
|
// Deterministic longest-processing-time partition: heaviest suite first, each
|
|
// assigned to the currently lightest shard. Ties break by file path and then
|
|
// by shard index, so every runner in the matrix computes the identical
|
|
// partition from the same checkout — that invariant is what makes the shards
|
|
// a complete, non-overlapping cover of the suite set.
|
|
export function partitionGeneralServerSuites(files, shardCount, durations = {}) {
|
|
const fallbackWeight = defaultSuiteWeight(durations);
|
|
const weighted = files
|
|
.map((file) => ({ file, weight: durations[file] ?? fallbackWeight }))
|
|
.sort((a, b) => b.weight - a.weight || a.file.localeCompare(b.file));
|
|
|
|
const shards = Array.from({ length: shardCount }, () => ({ files: [], totalWeight: 0 }));
|
|
for (const { file, weight } of weighted) {
|
|
let target = 0;
|
|
for (let index = 1; index < shards.length; index += 1) {
|
|
if (shards[index].totalWeight < shards[target].totalWeight) {
|
|
target = index;
|
|
}
|
|
}
|
|
shards[target].files.push(file);
|
|
shards[target].totalWeight += weight;
|
|
}
|
|
|
|
for (const shard of shards) {
|
|
shard.files.sort((a, b) => a.localeCompare(b));
|
|
}
|
|
return shards;
|
|
}
|
|
|
|
export function selectGeneralServerShard(files, shardIndex, shardCount, durations = {}) {
|
|
return partitionGeneralServerSuites(files, shardCount, durations)[shardIndex].files;
|
|
}
|