mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-22 16:51:28 +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>
198 lines
12 KiB
Markdown
198 lines
12 KiB
Markdown
# Profile the complete PR suite
|
|
|
|
Add the `profile-ci` label to a PR authored by a CI CODEOWNER. Adding the label starts a fresh `PR` workflow
|
|
run. Further pushes while the label remains also run with profiling enabled.
|
|
The run is named **CI profile · PR #number**. It runs all lanes of the main PR
|
|
suite, including middle-of-stack and release-notes-only PRs that normally skip
|
|
most lanes.
|
|
|
|
After the run finishes, open its Summary and follow **Open the Honeycomb
|
|
waterfall**. The separate **CI profile** workflow exports the trace after all
|
|
jobs finish. Its summary has the exact time range, job/span counts, and coverage
|
|
warnings. Keep the label on the PR and its head unchanged until export finishes.
|
|
Remove the label to stop profiling subsequent pushes. To profile the same
|
|
revision again, remove and re-add the label or rerun all jobs.
|
|
|
|
The PR author, original workflow actor, and rerun actor must all be individual
|
|
users in the trusted `master` branch's `.github/**` CODEOWNERS rule. GitHub user
|
|
IDs are resolved from that rule; PR edits to CODEOWNERS cannot grant access.
|
|
A label on an outside contributor's PR does not enable profiling, even when a
|
|
maintainer applies it. The PR still runs normal CI and needs no Honeycomb
|
|
access. Failed identity lookups disable profiling.
|
|
|
|
Ordinary PRs do not write timing artifacts or export traces. The exporter job is
|
|
skipped for ordinary run names. Adding another label does not run the CI matrix.
|
|
|
|
Profiling preserves the existing trusted AWS runner routing. Eligible PR jobs
|
|
use `paperclip-public-pr-x64`; the routing gate uses `paperclip-ci-routing-x64`.
|
|
The existing fail-closed GitHub-hosted fallback still applies if AWS routing
|
|
cannot be authorized. Only the separate post-run Honeycomb exporter uses
|
|
`ubuntu-latest` directly; it is outside the measured PR suite.
|
|
|
|
## Read the waterfall
|
|
|
|
The workflow root measures elapsed time from run creation to the last job's
|
|
completion. For reruns, it starts at that attempt's `run_started_at` instead of
|
|
the original creation time. Every job is a child on the same wall clock, with
|
|
its steps and available test details beneath it. All spans use a sample rate of
|
|
one. There is no sampling within the selected attempt. For a compact Gantt view,
|
|
open a job row's menu and choose **Collapse spans at this depth**. Expand an
|
|
individual job to inspect its steps and test files.
|
|
|
|
The expandable hierarchy is **workflow → job → step → test filename → tests**.
|
|
Vitest, Playwright, and Node file rows use repository-relative filenames, also
|
|
available in `test.file`. Separate files retain their own children even when
|
|
individual test names repeat across files.
|
|
Honeycomb shows only the 32,000 spans closest to the root in a very large trace.
|
|
All file rows are near the root, but the chart can omit some individual test
|
|
rows. Use **View events** to query all recorded test spans. Test names are
|
|
limited to 256 characters in the trace; file paths remain intact.
|
|
|
|
- Compare the jobs that finish last (`ci.finishes_last`) with other overlapping
|
|
work. A long job that finishes early is not necessarily delaying CI.
|
|
- Expand setup, dependency installation, cache restore, Rust compilation, builds,
|
|
and tests to find repeated or slow work.
|
|
- `ci.execution_ms` measures a job's execution. `ci.start_offset_ms` shows when
|
|
it started relative to the whole run. Do not add parallel job durations to
|
|
calculate elapsed time.
|
|
- Waiting spans cover job creation to runner start. Dependency waits can occur
|
|
before GitHub creates a job, so they appear as gaps from the workflow start.
|
|
GitHub does not expose a separate ready timestamp; do not interpret every gap
|
|
as runner queue time or claim this is a computed dependency critical path.
|
|
|
|
GitHub job/step timestamps have second precision. Vitest and Playwright record
|
|
test timestamps at the runner. Node's test reporter records completion receipt
|
|
time and duration (`test.clock=reporter-receipt`); buffered IPC delivery can shift
|
|
those small test spans. Vitest file spans include its test/hook execution and
|
|
carry collection/setup durations as fields. Playwright file spans cover the
|
|
first through last test attempt in that file. Rust and standalone script checks
|
|
retain their full job/step timings; stable libtest does not expose per-test wall
|
|
timestamps. The trace is CI timing data, not application tracing.
|
|
|
|
Scope includes all main PR lanes: policy, typecheck/release registry, general and
|
|
serialized tests, Runner checks, Docker context checks, build, canary dry run,
|
|
browser E2E, and aggregate checks. Separate Storybook, image, deployment, release,
|
|
and live-provider eval workflows are outside this trace.
|
|
|
|
## Failure, coverage, and reruns
|
|
|
|
The exporter uses GitHub's attempt-specific job API and paginates jobs and
|
|
artifacts. It includes failed and cancelled jobs and uploads available test
|
|
timings with `always()`. A force-cancelled or failed setup job may leave no
|
|
artifact. Coverage warnings and `ci.profile_complete=false` make missing
|
|
artifacts, interrupted reporters, rejected data, and absent test details visible.
|
|
Test launchers write an expected-invocation record before starting each Vitest,
|
|
Playwright, or Node process. Each reporter must write matching start/end records,
|
|
so a missing reporter cannot hide behind another successful invocation in the
|
|
same job. Direct package scripts can start a reporter without the launcher;
|
|
their start/end pair proves collection. Compare `ci.expected_invocations` with
|
|
`ci.reported_invocations`.
|
|
Failed or cancelled workflows are conservatively incomplete because a failed
|
|
command can prevent later tests or steps from running.
|
|
|
|
Each attempt gets a different trace ID. Rerunning failed jobs alone produces an
|
|
attempt trace for those jobs; it does not replay completed jobs. Reruns are marked
|
|
`ci.partial_rerun=true` conservatively. Use a fresh label-triggered run for an
|
|
unambiguous full-suite baseline. GitHub may carry earlier successful jobs into
|
|
the new attempt's job inventory. Jobs completed before the new attempt started
|
|
are excluded and counted in `ci.carried_job_count`.
|
|
|
|
The exporter sends bounded batches with pauses and retries only events that
|
|
Honeycomb rejects temporarily. If export still fails, inspect **CI profile**
|
|
before rerunning it. A partial HTTP export may already have sent some spans;
|
|
reruns use the same IDs but the Events API does not promise deduplication. Timing artifacts
|
|
remain available for 14 days. Invalid/oversized artifacts stop export instead of
|
|
silently truncating the suite. No logs, environment dumps, error messages, stack
|
|
traces, or credentials are included in the exported field allowlist.
|
|
|
|
## Operator setup and activation
|
|
|
|
Use the existing Honeycomb `paperclip` team, `test` environment, and a separate
|
|
service dataset named `paperclip-ci`. Create GitHub environment `ci-profiling` with **Selected branches and tags**:
|
|
allow only the branch `master`, with no PR-ref or tag patterns. Store environment
|
|
secret `HONEYCOMB_CI_API_KEY` with a dedicated ingest-only key for the Honeycomb
|
|
`test` environment. Do not create a repository- or organization-level secret of
|
|
that name: the environment restriction prevents in-repository PR workflows from
|
|
requesting the key. Tests still execute the PR branch; only the separate exporter
|
|
runs from `master`. It needs event
|
|
ingest permission; dataset creation permission is needed only if the dataset
|
|
has not been created yet. Do not store the environment configuration key in CI.
|
|
Create the repository label `profile-ci` if it does not exist.
|
|
|
|
`ci-profile.yml` runs from the trusted default-branch revision. It checks out only
|
|
the exporter, installs no dependencies, and parses PR artifacts as bounded JSON
|
|
without extracting or executing their contents. The PR jobs never receive the
|
|
Honeycomb key. The exporter independently verifies CI CODEOWNER identities,
|
|
the current label, repository, and exact tested PR revision before reading
|
|
artifacts or sending data. Its credential is sent only in an HTTPS header to
|
|
`api.honeycomb.io`; redirects are rejected. Exported fields are allowlisted.
|
|
`CODEOWNERS` also covers `scripts/ci-profile/**`, so changes to the privileged
|
|
exporter require infrastructure-owner review.
|
|
|
|
The new trusted workflow and routing definitions must merge to master before
|
|
the label-triggered journey is active. A PR uses `pr-trusted.yml@master` and
|
|
`ci-routing.yml@master`; do not change those trust references just to test this
|
|
feature before merge.
|
|
|
|
## Local qualification
|
|
|
|
```sh
|
|
node --test .github/scripts/tests/ci-profile.test.mjs .github/scripts/tests/ci-routing.test.mjs
|
|
node --test scripts/ci-profile/reporters.test.mjs
|
|
actionlint .github/workflows/{pr,pr-trusted,ci-routing,ci-profile}.yml
|
|
node scripts/ci-profile/export.mjs --run-id=RUN_ID --attempt=1 --historical --dry-run --output=/tmp/ci-trace.json
|
|
```
|
|
|
|
`--historical` is local-only and permits a deliberate export of an existing
|
|
unlabeled run. It has complete GitHub job/step timing but cannot reconstruct
|
|
test details that were never recorded. Omit `--dry-run` and supply
|
|
`HONEYCOMB_CI_API_KEY` to send the selected baseline to Honeycomb. It is labeled
|
|
as historical/incomplete test coverage; it is not proof of the label-triggered
|
|
workflow.
|
|
|
|
References: [Honeycomb trace fields](https://docs.honeycomb.io/get-started/best-practices/organizing-data/),
|
|
[Events API](https://docs.honeycomb.io/api/events/create-events),
|
|
[large trace display limit](https://docs.honeycomb.io/troubleshoot/common-issues/data-in-honeycomb/#your-very-large-trace-displays-only-32000-spans),
|
|
[GitHub workflow_run](https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#workflow_run).
|
|
|
|
## Qualify reductions in CI work
|
|
|
|
For changes to the stable test launcher, dependency setup, or the large Git and
|
|
readiness fixtures, `profile-ci` also runs **CI work benchmark**. It uses one
|
|
GitHub-hosted runner without secrets or AWS access. It compares the unchanged
|
|
PR base and candidate sequentially. It records complete command times, test
|
|
inventories, revisions, and failure logs in the `ci-work-benchmark` artifact.
|
|
The complete general-server shard must retain the same cases. The fixture comparison retains
|
|
the database contract and splits large Git scanning from bounded staging.
|
|
These command timings exclude queue time and setup; use the full Honeycomb
|
|
waterfall to assess the complete CI critical path.
|
|
|
|
The benchmark also exercises cold dependency setup and a real materialized
|
|
cache restore after removing the installed directories. Its two qualification
|
|
entries are scoped to the PR merge ref and a separate checkout path. They cannot
|
|
populate master's caches. Normal PR lanes restore only; the exact canonical
|
|
master push in release verification is the normal cache writer.
|
|
|
|
Materialized dependency keys include every tracked package manifest, lockfile,
|
|
workspace configuration, patches, repository scripts, Node version/ABI, platform,
|
|
architecture, libc, pnpm version, store location, and physical checkout path.
|
|
There are no prefix restores. An input change takes the normal install path.
|
|
A stale lockfile uses the existing resolution fallback but cannot publish an
|
|
entry. An additional workspace install/prepare hook disables materialized reuse
|
|
until its inputs are modeled. Shared/SDK build outputs have a separate key that also includes both
|
|
packages' source/configuration and the build verifier. The existing verifier
|
|
checks source and output fingerprints and rebuilds incomplete or changed output.
|
|
Tests and their results are never cached or omitted by this mechanism.
|
|
|
|
Serialized shards share one Vitest controller with `forks`, `isolate`, one
|
|
worker, and file parallelism disabled. Each file gets a separate process and
|
|
test home/configuration. The isolation regression runs files in both orders and
|
|
checks process IDs, environment, globals, fake timers, and filesystem state.
|
|
|
|
The readiness regression uses a real one-second PostgreSQL idle timeout and
|
|
verifies that the backend connection changes while the parent write can commit.
|
|
Its configured readiness window stays long. The Git regression keeps all four
|
|
filename lanes above 32 MiB, while the staging case transfers and deletes 4,000
|
|
files whose filename list exceeds 2 MiB. It still checks every staged file,
|
|
unusual names, snapshot ownership, cleanup, and bounded command lengths.
|