mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-19 20:44:16 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Contributors wait for the full PR workflow before they can deliver fixes. > - Server shards still execute independent files in series, and a single Runner file serializes two long real-clock waits. > - Faster work outside the critical path does not prove a faster workflow. > - Overlap independent files while preserving isolation, test cases and real production deadlines. > - Compare complete baseline and candidate workflows, separating queue delay from execution cost. ## Linked Issues or Issue Description Refs #16429. Related duration refresh: #12577. **What existing behavior does this improve?** The elapsed time from PR CI creation to the last required check. **Current behavior** General server shards use one worker. One Runner transport file takes about 333 seconds on Linux, including over 100 seconds in two real-clock regressions. These are the longest verification lanes after #16429. **Proposed behavior** Run independent general-server files in at most two isolated forks. Move the two intact Runner deadline regressions into a separate file to overlap their waits in the existing pool. Run two chat partitions per existing job, each with its own temporary home and embedded PostgreSQL. Preserve serial execution within files and every assertion and timing boundary. ## What Changed - Qualify both serial and parallel forks with real worker processes, isolated homes/configuration, timers, globals and environment. - Require both parallel workers to arrive before either exits. - Use two isolated forks for sharded general-server tests; retain one worker with an explicit shared test database. Other test groups keep their existing concurrency. - Extract the lease-expiry and Pi cold-admission regressions verbatim into a separate Runner test file; retain real waits and process/lease assertions. - Split each sharded chat job into two independent invocations; preserve serial shared-database execution. Await both children before failing the parent. Prove disjoint coverage and failure propagation. - Update the paired Linux benchmark to compare a complete general-server shard with identical case inventories. - Document concurrency and deadline-test placement. No production behavior, workflow topology or runner count changes. - Allow 15 seconds for snapshot integration cases that cold-start multiple real TypeScript workers. On the repeat, one-worker cases took 2.5–3 seconds, so queued or sequential starts could exceed the default five seconds. Preserve all cleanup, queue-release and filesystem assertions; these cases do not enforce a production latency deadline. ## Verification All 48 jobs passed on scheduling head `5d718e18` in [attempt 1](https://github.com/paperclipai/paperclip/actions/runs/38763885288/attempts/1). The [bounded repeat](https://github.com/paperclipai/paperclip/actions/runs/38763885288/attempts/2) failed two existing snapshot-worker tests at their default five-second timeout. Its shorter duration is **not** counted as a successful performance sample. The latest head `3c894462` adds a targeted test-budget repair and [passes all 48 CI jobs](https://github.com/paperclipai/paperclip/actions/runs/38768433532) in **8m06s** (workflow creation to final required workflow job). This is **70 seconds / 12.6% shorter** than the 9m16s intermediate run, but the earlier 9m23s result remains part of the evidence: a stable average improvement is not established. Both branch-protection checks (`ci / verify` and `ci / e2e`) pass. The non-required Superagent Security Scan is still running; external PR scans are separate from this workflow duration. | Measurement | Before | After | What it establishes | | --- | ---: | ---: | --- | | General-server shard, paired on one Linux host | 430.94s | 292.99s | 32.0% faster; identical 1,047 case inventory | | Chat job 1 | 460s | 211s | Same full chat inventory spread across isolated partitions | | Chat job 2 | 408s | 274s | Serial cases within each child, separate PostgreSQL instances | | Chat job 3 | 408s | 216s | Total chat runner time fell from 1,276s to 701s | | Full PR workflow, first chat candidate | 9m16s | 9m23s | Not an end-to-end win in this pair: unchanged Build rose from 365s to 465s | | Full PR workflow, final repaired head | 9m16s | 8m06s | All 48 jobs pass; 70 seconds faster observed, not a controlled causal estimate | - Latest [paired benchmark](https://github.com/paperclipai/paperclip/actions/runs/38763885223) passes. Earlier paired server measurements were 33.3% and 37.4% faster. Unchanged fixture timing noise is not attributed to this PR. - All **1,337 chat case identities and outcomes match exactly** across [before-chat](https://github.com/paperclipai/paperclip/actions/runs/38760937988) and [after-chat](https://github.com/paperclipai/paperclip/actions/runs/38763885288/attempts/1), without duplicates or removals. Whole-suite records rose from 44,563 to 44,574: three new launcher checks and eight upstream warm-owner tests. Remaining name differences were generated fixture IDs/timestamps/PIDs, not removed tests. - The final run used `m7i-flex.xlarge` for Build (408s), with all chat jobs completing in 238s / 318s / 280s. It used the same actual merge base `afe5b341` as the first chat candidate. Optional profiling was disabled after the bounded measurement experiment; the full 48-job correctness workflow still ran. - The fleet used `m7i.xlarge` for the earlier Build and `m6a.xlarge` for the first chat candidate Build. Several unchanged compile/install/probe phases slowed together. This does not prove hardware alone caused the difference, and full-run attribution must account for runner/cache variance and the upstream base change. - Original full baseline: [all 48 jobs passed](https://github.com/paperclipai/paperclip/actions/runs/38753547876/attempts/2), 12m07s, including a server job that queued over five minutes. It is not a clean basis for attributing the whole elapsed-time reduction to this PR. An interrupted baseline and two misrouted candidate attempts were excluded and preserved; routing/trust rules were not weakened. - Local: 32 sharding/launcher checks pass, including disjoint coverage, shared-database fallback and a failed child that waits for its sibling. Real paired chat partitions passed 416 cases in 217.63s on independent embedded databases. - Both extracted Runner deadline regressions pass unchanged with real waits. An initial local attempt rejected a Node executable under the controller home; the identical binary in an owned non-home path passed without weakening that guard. - Real worker-isolation proof, actionlint and diff checks pass. Latest-head Greptile is **5/5**, with no review threads or actionable findings. - The eight snapshot-worker cases pass locally with a private `PAPERCLIP_HOME`. The first direct local invocation failed to open the default-home lock database under the sandbox; no production guard was changed. ## Risks Parallel files and paired chat databases may expose shared-state assumptions or increase peak memory. Cases stay serial within each invocation, and explicit shared databases keep the serial path. Complete server coverage and the full 48-job CI workflow pass. Check external required PR scans before merge; the full-workflow measurements are observational and do not establish a steady 12.6% speedup. Queueing, cache state and runner variance must be distinguished from execution improvements. ## Model Used OpenAI GPT-6 through Codex, with repository inspection, code execution and GitHub tools. The exact serving model suffix and context capacity are not exposed to 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 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 (48 jobs; both required branch-protection checks pass; non-required external security scan pending) - [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>
77 lines
4.9 KiB
JavaScript
77 lines
4.9 KiB
JavaScript
import assert from "node:assert/strict";
|
|
import { execFileSync, spawnSync } from "node:child_process";
|
|
import { mkdirSync, readdirSync, readFileSync, writeFileSync } from "node:fs";
|
|
import path from "node:path";
|
|
import { fileURLToPath } from "node:url";
|
|
|
|
const candidate = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..");
|
|
const baseline = path.resolve(process.argv[2] ?? "");
|
|
assert.ok(process.argv[2] && baseline !== candidate, 'Pass a separately installed unchanged baseline checkout');
|
|
const output = path.resolve(process.argv[3] ?? path.join(candidate, '.ci-benchmark'));
|
|
mkdirSync(output, { recursive: true });
|
|
const results = [];
|
|
const commands = [
|
|
['general-server', process.execPath, ['scripts/run-vitest-stable.mjs', '--mode', 'general', '--group', 'general-server-without-chat-or-native-runner', '--shard-index', '0', '--shard-count', '12']],
|
|
['fixtures', 'pnpm', ['exec', 'vitest', 'run', '--project', '@paperclipai/server',
|
|
'server/src/services/workspace-git-snapshot-streaming.test.ts', 'server/src/__tests__/workspace-runtime.test.ts',
|
|
'-t', 'streams.*all four real Git|commits fixed-port reservations']],
|
|
];
|
|
for (const [variant, cwd] of [['baseline', baseline], ['candidate', candidate]]) {
|
|
for (const [name, command, args] of commands) {
|
|
const profile = path.join(output, `${variant}-${name}`);
|
|
mkdirSync(profile, { recursive: true });
|
|
const started = performance.now();
|
|
const result = spawnSync(command, args, {
|
|
cwd, encoding: 'utf8', timeout: 15 * 60_000, maxBuffer: 64 * 1024 * 1024,
|
|
env: { ...process.env, GITHUB_WORKSPACE: cwd, NODE_ENV: 'test', PAPERCLIP_CI_PROFILE: 'true',
|
|
PAPERCLIP_CI_PROFILE_DIR: profile, PAPERCLIP_CI_PROFILE_JOB: `benchmark-${variant}-${name}` },
|
|
});
|
|
const wallMs = performance.now() - started;
|
|
writeFileSync(path.join(output, `${variant}-${name}.log`), `${result.stdout ?? ''}\n${result.stderr ?? ''}`);
|
|
const records = readdirSync(profile).filter((file) => file.endsWith('.ndjson'))
|
|
.flatMap((file) => readFileSync(path.join(profile, file), 'utf8').split('\n').filter(Boolean).map((line) => JSON.parse(line)));
|
|
results.push({ variant, name, wallMs, status: result.status,
|
|
sha: execFileSync('git', ['rev-parse', 'HEAD'], { cwd, encoding: 'utf8' }).trim(),
|
|
tests: records.filter((record) => record.kind === 'test' && record.status !== 'skipped')
|
|
.map((record) => [record.file, record.name, record.status]).sort(),
|
|
files: records.filter((record) => record.kind === 'file').map((record) => record.file).sort(),
|
|
});
|
|
writeFileSync(path.join(output, 'results.json'), JSON.stringify({ node: process.version, results }, null, 2));
|
|
assert.equal(result.status, 0, `${variant}/${name} failed: ${result.error ?? ''}; see ${output}`);
|
|
console.log(`${variant}/${name}: ${(wallMs / 1000).toFixed(2)}s`);
|
|
}
|
|
}
|
|
const rows = [];
|
|
for (const [name] of commands) {
|
|
const before = results.find((result) => result.variant === 'baseline' && result.name === name);
|
|
const after = results.find((result) => result.variant === 'candidate' && result.name === name);
|
|
assert.ok(before.tests.length > 0, `${name}: missing test evidence`);
|
|
assert.deepEqual(after.files, before.files, `${name}: file inventory changed`);
|
|
if (name === 'general-server') assert.deepEqual(after.tests, before.tests, 'general server case inventory changed');
|
|
else {
|
|
const contracts = (tests) => {
|
|
const found = new Set();
|
|
for (const [file, title, status] of tests) {
|
|
assert.equal(status, 'passed');
|
|
if (file.endsWith('/workspace-runtime.test.ts') && title.includes('commits fixed-port reservations')) found.add('database');
|
|
if (file.endsWith('/workspace-git-snapshot-streaming.test.ts')) {
|
|
if (title.includes('above 32 MiB')) found.add('large Git lanes');
|
|
if (/stag(?:es|ing)/.test(title)) found.add('bounded staging');
|
|
}
|
|
}
|
|
return [...found].sort();
|
|
};
|
|
for (const result of [before, after]) assert.deepEqual(contracts(result.tests),
|
|
['bounded staging', 'database', 'large Git lanes'], 'fixture contracts must all execute');
|
|
}
|
|
rows.push(`| ${name} | ${(before.wallMs / 1000).toFixed(2)}s | ${(after.wallMs / 1000).toFixed(2)}s | ${((1 - after.wallMs / before.wallMs) * 100).toFixed(1)}% | ${after.tests.length} |`);
|
|
}
|
|
const summary = ['| Work | Baseline | Candidate | Reduction | Executed cases |',
|
|
'|---|---:|---:|---:|---:|', ...rows,
|
|
'', 'Paired sequential runs on one host with Node ' + process.version + '. Setup, cache transfer, queue time and the full CI critical path are outside these command timings.',
|
|
].join('\n');
|
|
writeFileSync(path.join(output, 'summary.md'), summary + '\n');
|
|
console.log(summary);
|
|
const total = (variant) => results.filter((result) => result.variant === variant).reduce((sum, result) => sum + result.wallMs, 0);
|
|
assert.ok(total('candidate') < total('baseline'), 'No combined speedup measured; inspect the paired evidence before claiming an improvement');
|