mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
Nicky LeachandClaude Opus 5.5
8ed712f00a
## 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 > - PR CI runs the general server tests in 12 shards, and each shard runs files in parallel on 4 workers > - `server/src/services/workspace-recovery.test.ts` takes about 699 seconds in CI by itself. A file runs whole on one worker, so shard 2/12 takes 11.5 minutes (median) when the other shards take about 7 minutes > - A different shard count cannot fix this, because the time of one file sets the minimum time of its shard > - This pull request splits the file into seven feature files with a shared fixture > - The benefit is that the shards can balance again. This removes about 2 minutes from the median PR CI time and about 5 minutes from the p90 ## Linked Issues or Issue Description **What existing behavior does this improve?** The time of the general server test shards in PR CI. **Subsystem affected** Server tests: workspace recovery (`server/src/services/workspace-recovery*.test.ts`) and the general-server shard weights. **Current behavior** One file holds 460 tests and takes about 699 seconds in CI. Its shard is the slowest in most PR runs. **Proposed behavior** Seven files of about 60 to 140 seconds each. The shard partitioner places them on different shards. **Reason and benefit** The general server shards finish close together. The PR CI critical path becomes shorter. **Breaking changes** None. No product code changes. ## What Changed - Split `server/src/services/workspace-recovery.test.ts` into seven files by feature area: - `retry-copyback` (76 tests); - `preparation` (80); - `preparation-proof` (53); - `preparation-dispatch` (25); - `preparation-gates` (43); - `warm-handoff` (36); - `admission` (147). - Add `server/src/services/workspace-recovery.test-fixture.ts` with the shared hooks, `seed`, helpers and an embedded Postgres per file. - Test bodies, hooks and helpers are moved without change. - Four lines that assigned the shared `db`, `root` or `connectionString` now call `replaceFixtureState(...)`. - `scripts/general-server-shard-durations.json`: replace the old key with the seven new files. The weights divide the current CI cost of the old file (636 seconds, master runs 42296943853 and 42294279748) in proportion to measured local time. A `splitEstimates` entry records the method, as for earlier splits. - Update the file references in `doc/task-recovery.md` and `maps/feature-map/recovery.md`. ## Verification - `vitest list`, before and after: 136 declarations each, with identical sorted names. At run time, 460 tests before and after. - Each new file passes when run alone: 76, 80, 53, 25, 43, 36 and 147 tests. - All seven files run together with `--maxWorkers=1`: 460/460 pass. - A timing check of the original file: - `afterEach` took 4.7 seconds in total and `beforeAll` 4.1 seconds, out of about 1,000 seconds, so the teardown is not the cost; - most of the time is in three `it.each` blocks. - Focused `tsc --noEmit` on the eight new files: clean. - `node --test scripts/__tests__/run-vitest-stable-shard.test.mjs`: 39/39 pass. - `node scripts/check-maps.mjs`: aligned. - With the new weights, the 12-shard partition puts the seven files on seven different shards. ## Risks - The CI time of each new file is an estimate from local runs. If `workspace-recovery-preparation.test.ts` takes more than about 150 seconds in CI, the next split point is the "keeps ancestor Retry held" block. - Each file starts its own embedded Postgres, which adds a few seconds of setup per file. - The optional external-database environment variables keep their old behavior but were not tested. CI does not set them. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use and code execution. A Claude sub-agent made the split, and the parent session reviewed it and corrected the shard weights. ## 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>