mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 16:02:36 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Contributors wait for PR CI before merge, so CI wall-clock time sets the pace of work > - CI test shards are balanced with per-file durations that a weekly workflow refreshes > - The weekly refresh has failed since 2026-11-02, and the e2e weights have not been measured since 2026-09-15. Today e2e shards 3 and 6 take 7.0–7.7 minutes of tests, and shard 1 takes 4.3 minutes > - A single file can also grow until it sets the time of its shard, and nothing tells us about it > - This pull request repairs the refresh, extends it to the e2e and Runner package lanes, and warns about test files over 180 seconds > - The benefit is balanced shards that stay balanced, and early notice of oversized test files ## Linked Issues or Issue Description **What existing behavior does this improve?** The weekly refresh of the test-duration weights that balance CI shards. **Subsystem affected** CI: `scripts/ci/refresh-test-durations.mjs`, `.github/workflows/ci-test-durations.yml`, and the Vitest and Playwright CI reporters. **Current behavior** - The 2026-11-02 refresh (run 41408033686) failed. `gh api …/logs` now refuses output that contains terminal escape sequences. - Most successful `cloud-readiness.yml` master runs are admission-only and have no server jobs. The script took the newest 2 runs, so it would fail with "No server timing jobs found" on most weeks. - The e2e weights (`scripts/e2e-shard-durations.json`) have no refresh and are from 2026-09-15. - No signal tells us when one test file becomes too slow. **Proposed behavior** The refresh runs again, selects master runs that ran the server jobs, refreshes the e2e and Runner package-lane weights from merged PRs, and lists oversized files. CI writes a warning for each test file over 180 seconds. **Reason and benefit** Unbalanced shards add minutes to every PR. The current e2e weights put 472 seconds on the slowest shard. The new weights put about 300 seconds on each shard. **Breaking changes** None. Weights change only shard placement, never which tests run. ## What Changed - Add `scripts/ci/gh-api.mjs`. It always passes `--allow-escape-sequences`, retries without the flag when an older `gh` does not know it, and strips ANSI codes from logs. - `scripts/ci/refresh-test-durations.mjs`: - scans up to 50 master runs and uses the 2 newest that ran the server jobs; - adds the `--dry-run`, `--pr-body=<path>` and `--lanes=server,e2e,runner` options; - validates every lane before it writes a file. If a merged-PR lane has fewer than 2 valid runs, it keeps the current weights and writes a warning. - Add `scripts/ci/pr-run-durations.mjs`. It uses the final passing `pr.yml` run of each merged PR, and it validates: - the PR is merged into `master`; - the merge commit is in the checkout; - the run's head SHA is the PR's final head SHA; - every shard of the run succeeded. It parses Playwright per-spec times and Runner per-file times. - Add `scripts/ci/oversized-test-files.mjs` (Vitest) and `scripts/ci/oversized-test-files-playwright.mjs` (Playwright). In GitHub Actions they write a `::warning` for each test file over `PAPERCLIP_CI_OVERSIZED_TEST_FILE_SECONDS` (default 180). They never fail a job. The root and Runner Vitest configs and the e2e Playwright config load them. - `scripts/ci/test-timing-reporter.mjs`: move the per-file duration calculation into an exported `moduleDurationMs()` helper. The behavior does not change. - `.github/workflows/ci-test-durations.yml` and `.github/ci-duration-pr.md`: - the bot PR can also update the e2e and Runner manifests; - the PR body lists oversized files; - the verify step also runs the new tests and the e2e shard tests. - Refresh `scripts/e2e-shard-durations.json`: 56 of 56 specs measured (before: 30), from merged PRs #19613, #19191 and #19176. The file now uses the shared schema with `sources` and `coverage`. ## Verification - `node --test scripts/ci/test-selection-pr-timings.test.mjs scripts/ci/test-selection-timings.test.mjs scripts/__tests__/e2e-shard.test.mjs`: 32/32 pass. These include the e2e shard balance check with the new weights. - `node scripts/ci/refresh-test-durations.mjs --dry-run` against real GitHub data, all lanes: | Manifest | Measured | Sources | | --- | --- | --- | | `general-server-shard-durations.json` | 1488/1534 files | master runs 42296943853, 42294279748 | | `serialized-shard-durations.json` | 167/167 files | the same two runs | | `e2e-shard-durations.json` | 56/56 specs | PR runs 42296374365, 42297700476, 42296994179 | | Runner `vitest-lane-durations.json` | 310/311 files | the same three PR runs | - e2e placement over 8 shards, with the new medians as the real costs: - with the old weights, the slowest shard has 472 seconds; - with the new weights, every shard has about 300 seconds. - The dry run lists 12 files over 180 seconds. The largest is `server/src/services/workspace-recovery.test.ts`, at 636 seconds. - In this PR, only the e2e weight file changes. The weekly run refreshes the server weights. The Runner manifest is added by #19646. ## Risks - The `--allow-escape-sequences` path ran only in unit tests. The local `gh` did not support the flag, so the real dry run used the fallback. The next weekly run is the first real test of the flag. - The changes to the bot-PR shell steps did not run in Actions yet. - The e2e weights come from 3 PR runs. Real shard times vary more than the estimate. - This PR and #19637 both edit the `reporters` lists in `vitest.config.ts` and `packages/paperclip-runner/vitest.config.ts`. The second PR to merge needs a small merge resolution. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use and code execution. A Claude sub-agent wrote the change, and the parent session reviewed it. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
256 lines
15 KiB
JavaScript
256 lines
15 KiB
JavaScript
import assert from "node:assert/strict";
|
|
import { execFileSync } from "node:child_process";
|
|
import { existsSync, readFileSync, writeFileSync } from "node:fs";
|
|
import path from "node:path";
|
|
import { fileURLToPath } from "node:url";
|
|
import { stripVTControlCharacters } from "node:util";
|
|
import { listE2eSpecs } from "../e2e-shard.mjs";
|
|
import { ghApi } from "./gh-api.mjs";
|
|
import { oversizedThresholdEnv, oversizedThresholdMs } from "./oversized-test-files.mjs";
|
|
import { collectMergedPrSamples, e2eManifest, e2eShardJob, isMergedIntoHead, parseE2eLog, parseRunnerLog,
|
|
runnerInventory, runnerManifest, runnerPackageJob } from "./pr-run-durations.mjs";
|
|
import { timingPrefix } from "./test-timing-reporter.mjs";
|
|
|
|
const repository = "paperclipai/paperclip";
|
|
const api = `/repos/${repository}`;
|
|
const github = (endpoint, json = true) => ghApi(endpoint, { json });
|
|
const masterComment = "Generated by node scripts/ci/refresh-test-durations.mjs from successful protected-master Cloud readiness runs. Median per-file cost includes tests, hooks, collection, setup and environment preparation when the timing reporter is present; older logs bootstrap test/hook durations only. Missing files retain their prior weight or the partitioner's median fallback. Timing changes only partition work; they never authorize skipping a test.";
|
|
const e2eComment = "Generated by node scripts/ci/refresh-test-durations.mjs from successful .github/workflows/pr.yml runs of PRs merged into master, each pinned to the PR's final head SHA. Per spec: the sum of gaps between consecutive Playwright result lines in 'Run e2e tests' (one worker), so fixture and navigation time count; tests located in a *.shared.ts file are attributed to the shard's matching spec. Median across runs. scripts/e2e-shard.mjs uses these weights only to place specs on shards; they never change which specs run. Missing specs retain their prior weight or the median fallback.";
|
|
const runnerComment = "Generated by node scripts/ci/refresh-test-durations.mjs from successful .github/workflows/pr.yml runs of PRs merged into master, each pinned to the PR's final head SHA. Per file: the Vitest file duration printed by the Verify Paperclip Runner package job (tests and hooks; collection and setup are not included). Median across runs. Weights only place files on lanes; they never change which tests run. Missing files retain their prior weight or the median fallback.";
|
|
|
|
export function validateRun(run) {
|
|
assert.equal(run.repository?.full_name, repository);
|
|
assert.equal(run.repository?.id, 1170821064);
|
|
assert.equal(run.head_repository?.id, 1170821064);
|
|
assert.equal(run.head_branch, "master");
|
|
assert.equal(run.event, "push");
|
|
assert.equal(run.path, ".github/workflows/cloud-readiness.yml");
|
|
assert.equal(run.status, "completed");
|
|
assert.equal(run.conclusion, "success");
|
|
assert.match(run.head_sha, /^[a-f0-9]{40}$/);
|
|
assert.ok(Number.isSafeInteger(run.id) && run.id > 0);
|
|
assert.ok(Number.isSafeInteger(run.run_attempt) && run.run_attempt > 0);
|
|
}
|
|
|
|
export function parseTimings(log, inventory) {
|
|
const fallback = new Map(), measured = new Map();
|
|
for (const raw of log.split("\n")) {
|
|
const line = stripVTControlCharacters(raw);
|
|
const offset = line.indexOf(timingPrefix);
|
|
if (offset >= 0) {
|
|
const record = JSON.parse(line.slice(offset + timingPrefix.length));
|
|
assert.equal(record.version, 1);
|
|
assert.equal(typeof record.file, "string");
|
|
assert.match(record.file, /^server\/(src|scripts)\/.+\.test\.[cm]?[jt]s$/);
|
|
assert.ok(!record.file.split("/").some(part => part === ".." || part === "." || part === ""));
|
|
assert.ok(!record.file.includes("\\"));
|
|
assert.ok(Number.isFinite(record.durationMs) && record.durationMs >= 0 && record.durationMs <= 3_600_000);
|
|
// A trusted source run can predate a deletion or a lane reassignment.
|
|
if (!inventory.has(record.file)) continue;
|
|
measured.set(record.file, Math.max(measured.get(record.file) ?? 0, record.durationMs));
|
|
continue;
|
|
}
|
|
// Bootstrap from pre-reporter runs. These values omit import/setup cost;
|
|
// structured measurements take precedence as soon as master emits them.
|
|
const match = line.match(/@paperclipai\/server\s+([^\s]+\.test\.[cm]?[jt]s)\s+\(\d+ tests?[^)]*\)\s+(\d+)ms\s*$/);
|
|
if (!match) continue;
|
|
const file = `server/${match[1]}`, ms = Number(match[2]);
|
|
if (inventory.has(file) && ms <= 3_600_000) fallback.set(file, Math.max(fallback.get(file) ?? 0, ms));
|
|
}
|
|
return new Map([...fallback].map(([file, ms]) => [file, { ms, complete: false }])
|
|
.concat([...measured].map(([file, ms]) => [file, { ms, complete: true }])));
|
|
}
|
|
|
|
export function updatedDurations(previous, inventory, samples, sources, { comment = masterComment } = {}) {
|
|
const measured = [...inventory].filter(file => samples.some(sample => sample.has(file)));
|
|
assert.ok(measured.length >= inventory.size * 0.9, `Incomplete timing coverage: ${measured.length}/${inventory.size}`);
|
|
const durations = {};
|
|
let complete = 0;
|
|
for (const file of [...inventory].sort()) {
|
|
const values = samples.flatMap(sample => sample.has(file) ? [sample.get(file)] : []);
|
|
const full = values.filter(value => value.complete);
|
|
if (full.length) complete++;
|
|
const ms = (full.length ? full : values).map(value => value.ms).sort((a, b) => a - b);
|
|
if (!ms.length) {
|
|
if (Number.isFinite(previous.durations[file]) && previous.durations[file] >= 0) durations[file] = previous.durations[file];
|
|
continue;
|
|
}
|
|
const middle = Math.floor(ms.length / 2);
|
|
// A small floor prevents clock resolution from giving a suite zero cost.
|
|
durations[file] = Math.max(100, Math.ceil(ms.length % 2 ? ms[middle] : (ms[middle - 1] + ms[middle]) / 2));
|
|
}
|
|
return {
|
|
$comment: comment,
|
|
unit: "ms", sources, coverage: { measured: measured.length, withSetup: complete, inventory: inventory.size }, durations,
|
|
};
|
|
}
|
|
|
|
// Files over the per-file budget, slowest first, across every refreshed manifest.
|
|
export function oversizedFiles(manifests, thresholdMs) {
|
|
return manifests.flatMap(({ data }) => Object.entries(data.durations))
|
|
.filter(([, ms]) => ms > thresholdMs).sort((a, b) => b[1] - a[1] || a[0].localeCompare(b[0]));
|
|
}
|
|
|
|
export const oversizedPlaceholder = "<!-- oversized-test-files -->";
|
|
|
|
export function renderPrBody(template, oversized, thresholdMs) {
|
|
assert.ok(template.includes(oversizedPlaceholder), "PR body template lacks the oversized-file placeholder");
|
|
const seconds = Math.round(thresholdMs / 1000);
|
|
const lines = oversized.length
|
|
? [`- ${oversized.length} test file(s) exceed the ${seconds}s per-file budget (\`${oversizedThresholdEnv}\`). A shard cannot split a file, so each one bounds its lane's wall-clock time. Consider splitting:`,
|
|
...oversized.map(([file, ms]) => ` - \`${file}\`: ${Math.round(ms / 1000)}s`)]
|
|
: [`- No measured test file exceeds the ${seconds}s per-file budget (\`${oversizedThresholdEnv}\`).`];
|
|
return template.replace(oversizedPlaceholder, lines.join("\n"));
|
|
}
|
|
|
|
function summarizeChange(file, previous, next) {
|
|
const before = previous?.durations ?? {}, after = next.durations;
|
|
const changes = Object.keys(after).map(key => [key, before[key], after[key]])
|
|
.filter(([, old, value]) => old !== value)
|
|
.sort((a, b) => Math.abs(b[2] - (b[1] ?? 0)) - Math.abs(a[2] - (a[1] ?? 0)));
|
|
const removed = Object.keys(before).filter(key => !(key in after));
|
|
console.log(`${file}: ${next.coverage.measured}/${next.coverage.inventory} measured; ${next.coverage.withSetup} include setup; `
|
|
+ `${changes.length} changed, ${removed.length} removed; sources ${next.sources.map(source => source.pull ? `#${source.pull}/${source.run}` : source.run).join(", ")}`);
|
|
for (const [key, old, value] of changes.slice(0, 12)) console.log(` ${key}: ${old ?? "(new)"} -> ${value}`);
|
|
}
|
|
|
|
export function parseArgs(argv) {
|
|
const options = { runIds: [], dryRun: false, prBody: null, lanes: new Set(["server", "e2e", "runner"]) };
|
|
for (const arg of argv) {
|
|
if (arg === "--dry-run") options.dryRun = true;
|
|
else if (arg.startsWith("--pr-body=")) options.prBody = arg.slice("--pr-body=".length);
|
|
else if (arg.startsWith("--lanes=")) {
|
|
options.lanes = new Set(arg.slice("--lanes=".length).split(","));
|
|
for (const lane of options.lanes) assert.ok(["server", "e2e", "runner"].includes(lane), `Unknown lane ${lane}`);
|
|
} else {
|
|
assert.match(arg, /^\d+$/, `Unexpected argument ${arg}`);
|
|
options.runIds.push(arg);
|
|
}
|
|
}
|
|
return options;
|
|
}
|
|
|
|
export const serverLane = name => /General tests \(server \(\d+\/\d+\)\)$/.test(name) ? "general"
|
|
: /Serialized tests \(\d+\/\d+\)$/.test(name) ? "serialized" : null;
|
|
|
|
const readManifest = file => existsSync(file) ? JSON.parse(readFileSync(file, "utf8")) : { durations: {} };
|
|
|
|
async function refreshMasterLanes(runIds) {
|
|
const inventory = JSON.parse(execFileSync(process.execPath, ["scripts/run-vitest-stable.mjs", "--mode", "general",
|
|
"--group", "general-server", "--shard-index", "0", "--shard-count", "1", "--dry-run"], {
|
|
encoding: "utf8", env: { ...process.env, GITHUB_WORKFLOW: "PR", PAPERCLIP_CI_TEST_SELECTION: "" },
|
|
}));
|
|
const lanes = {
|
|
general: { files: new Set(inventory.selectedGeneralServerSuites), path: "scripts/general-server-shard-durations.json", samples: [] },
|
|
serialized: { files: new Set(inventory.selectedSerializedSuites), path: "scripts/serialized-shard-durations.json", samples: [] },
|
|
};
|
|
const runJobs = async run => {
|
|
const jobs = [];
|
|
for (let page = 1; ; page++) {
|
|
const result = await github(`${api}/actions/runs/${run.id}/attempts/${run.run_attempt}/jobs?per_page=100&page=${page}`);
|
|
jobs.push(...result.jobs);
|
|
if (result.jobs.length < 100) break;
|
|
}
|
|
return jobs;
|
|
};
|
|
const candidates = [];
|
|
if (runIds.length) {
|
|
assert.ok(runIds.length <= 3, "Expected one to three successful master runs");
|
|
for (const id of runIds) {
|
|
assert.match(String(id), /^\d+$/);
|
|
const run = await github(`${api}/actions/runs/${id}`);
|
|
validateRun(run);
|
|
execFileSync("git", ["merge-base", "--is-ancestor", run.head_sha, "HEAD"]);
|
|
candidates.push({ run, jobs: await runJobs(run) });
|
|
}
|
|
} else {
|
|
// Most master pushes only run Cloud admission and skip verification, and a
|
|
// newer push can land after this checkout. Use the two newest runs this
|
|
// checkout contains that actually ran the server test jobs.
|
|
const listed = await github(`${api}/actions/workflows/cloud-readiness.yml/runs?branch=master&event=push&status=success&per_page=50`);
|
|
for (const run of listed.workflow_runs) {
|
|
if (candidates.length === 2) break;
|
|
if (!isMergedIntoHead(run.head_sha)) continue;
|
|
validateRun(run);
|
|
const jobs = await runJobs(run);
|
|
if (jobs.some(job => serverLane(job.name))) candidates.push({ run, jobs });
|
|
}
|
|
}
|
|
assert.ok(candidates.length > 0, "Expected one to three successful master runs");
|
|
const sources = [];
|
|
for (const { run, jobs } of candidates) {
|
|
const samples = { general: new Map(), serialized: new Map() };
|
|
const selected = jobs.map(job => ({ job, lane: serverLane(job.name) })).filter(item => item.lane);
|
|
assert.ok(selected.length, "No server timing jobs found");
|
|
// Keep API traffic and log buffers bounded. Logs are parsed data, never code.
|
|
for (let start = 0; start < selected.length; start += 3) {
|
|
await Promise.all(selected.slice(start, start + 3).map(async ({ job, lane }) => {
|
|
assert.equal(job.run_id, run.id);
|
|
assert.equal(job.conclusion, "success");
|
|
const parsed = parseTimings(await github(`${api}/actions/jobs/${job.id}/logs`, false), lanes[lane].files);
|
|
for (const [file, value] of parsed) {
|
|
const previous = samples[lane].get(file);
|
|
if (!previous || (value.complete && !previous.complete)
|
|
|| (value.complete === previous.complete && value.ms > previous.ms)) samples[lane].set(file, value);
|
|
}
|
|
}));
|
|
}
|
|
for (const lane of Object.keys(lanes)) lanes[lane].samples.push(samples[lane]);
|
|
sources.push({ run: run.id, attempt: run.run_attempt, sha: run.head_sha, url: run.html_url });
|
|
}
|
|
return Object.values(lanes).map(lane => {
|
|
const previous = readManifest(lane.path);
|
|
return { path: lane.path, previous, data: updatedDurations(previous, lane.files, lane.samples, sources) };
|
|
});
|
|
}
|
|
|
|
// Fewer than two valid runs leaves a manifest unchanged with a visible warning;
|
|
// one run is too noisy to replace measured weights.
|
|
async function refreshPrLanes(names) {
|
|
if (!names.length) return [];
|
|
const definitions = {
|
|
e2e: () => ({ classify: e2eShardJob, parse: parseE2eLog, inventory: new Set(listE2eSpecs()), path: e2eManifest, comment: e2eComment }),
|
|
runner: () => ({ classify: runnerPackageJob, parse: parseRunnerLog, inventory: runnerInventory(), path: runnerManifest, comment: runnerComment }),
|
|
};
|
|
const lanes = names.map(name => ({ name, ...definitions[name]() }));
|
|
const collected = await collectMergedPrSamples({ gh: github, lanes });
|
|
const updates = [];
|
|
for (const lane of lanes) {
|
|
const { samples, sources } = collected[lane.name];
|
|
if (samples.length < 2) {
|
|
console.log(`::warning::${lane.path}: only ${samples.length} valid merged-PR run(s); keeping the current weights`);
|
|
continue;
|
|
}
|
|
const previous = readManifest(lane.path);
|
|
updates.push({ path: lane.path, previous,
|
|
data: updatedDurations(previous, lane.inventory, samples, sources, { comment: lane.comment }) });
|
|
}
|
|
return updates;
|
|
}
|
|
|
|
export async function refreshDurations(argv = []) {
|
|
const options = parseArgs(argv.map(String));
|
|
const updates = [
|
|
...(options.lanes.has("server") ? await refreshMasterLanes(options.runIds) : []),
|
|
...await refreshPrLanes(["e2e", "runner"].filter(lane => options.lanes.has(lane))),
|
|
];
|
|
// Validate every lane before replacing any checked-in manifest.
|
|
for (const { path: file, previous, data } of updates) {
|
|
summarizeChange(file, previous, data);
|
|
if (!options.dryRun) writeFileSync(file, JSON.stringify(data, null, 2) + "\n");
|
|
}
|
|
const thresholdMs = oversizedThresholdMs();
|
|
const oversized = oversizedFiles(updates, thresholdMs);
|
|
console.log(`${oversized.length} measured test file(s) exceed ${Math.round(thresholdMs / 1000)}s`);
|
|
for (const [file, ms] of oversized) console.log(` ${file}: ${Math.round(ms / 1000)}s`);
|
|
if (options.prBody) {
|
|
writeFileSync(options.prBody, renderPrBody(readFileSync(".github/ci-duration-pr.md", "utf8"), oversized, thresholdMs));
|
|
}
|
|
if (options.dryRun) console.log("Dry run: no manifest was written");
|
|
return updates;
|
|
}
|
|
|
|
if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) {
|
|
refreshDurations(process.argv.slice(2)).catch(error => { console.error(error); process.exitCode = 1; });
|
|
}
|