Repository navigation
Commit 6abd265
ci: a per-package suite-duration ratchet — Test Core reds a suite over its measured ceiling (#22480)
Part of #16468
Clause-②: no
The per-package suite-duration ratchet. `Test Core` now grades every
executed package's test suite against a measured ceiling and goes red
when one runs past it, naming the package and its slowest test files.
The ceilings are generated by the shard-timings refresh into
`scripts/test-shard-timings.json`, and a ceiling rises only by a ruling.
This PR is `Part of` the card because no seat can write the first
ceiling table (see "Bootstrap" below). The first scheduled run of
`.github/workflows/shard-timings-refresh.yml` after this merges writes
it. Until that refresh PR merges, the check prints NOT MEASURED on every
run, with a warning annotation. What finishes the card: that refresh PR
merged, and a `Test Core` run on `main` printing `suite-ceiling: OK`
over graded packages. The held-versus-refreshed question below should
also be answered before the second refresh.
Authority (maintainer, quoted in the card body): 「同意你的建议,你负责执行派发所有可行的优化」
## The ruling (comment 6079579826, quoted verbatim)
> **A — the ceiling is the package's slowest executed run in the refresh
window × 1.25; a provisional package is not red.** The ratchet's three
ruled clauses stand: the ceilings are a generated table beside the
timings file, refreshed with it, 25% headroom; the check runs after the
shard's tests, reads the turbo summary, and reds a package over its
ceiling naming the package and its slowest files; a ceiling rises only
by a ruling. "The refreshed measurement" is read as the window maximum,
so the window's own variation never trips the brake, and a pull request
goes red only when a suite runs 25% slower than its slowest measured
run. A package in the dataset's `provisional` list prints "no ceiling:
provisional" and is not red; only a package absent from the dataset is
red under clause 3, the new-package shape. Build order: the dev first
measures A's false-red rate over the 21-run window (expected 0; if not
0, stop and report, do not force it), then builds the check; the CLI's
three pull-request-time slices (#22075, PR #22415) are summed across
shards or each compared to a third of the ceiling, an implementation
detail the dev reports. ⛔ Not taken: B (31 packages already over the
line with no code change; a brake that is red by default is learned
around, the #16465 round-55 lesson), C (no red at the pull request,
which is the card's core), D (no brake).
## What changed
- **`scripts/measure-test-shard-timings.mjs` (the generator).**
- New `ceilings` field, mapping each package to seconds. A package's
ceiling is its slowest executed run in the refresh window ×
`CEILING_HEADROOM` (1.25), rounded up to the hundredth. For a
file-sliced package, the "run" is the per-run sum of its slices.
- New `uncapped` field, naming each package that gets no ceiling and
why. `provisional` means fewer than 3 executed runs. `carried` means a
cache HIT re-confirmed the package but no run in the window executed it.
- `provenance.ceilingHeadroom` records the headroom.
- The medians in `packages` are unchanged, and so is every reader of
them (the partitioner and the drift step).
- The spread table in the refresh PR body gains a `ceiling s` column.
- **`scripts/check-test-suite-ceilings.mjs` (the check, new).**
- Reads the per-shard timing captures that `report-test-timings.mjs
--capture` already writes, and sums a sliced package's parts across
shards.
- Grades each executed package:
- over its ceiling: red, naming the package and its 5 slowest files;
- absent from the dataset: red;
- `provisional`: prints `no ceiling: provisional`, not red;
- `carried`: prints `no ceiling: carried`, not red;
- a sliced package missing a part: NOT MEASURED for that package.
- If the dataset has no `ceilings` field, the whole run reads NOT
MEASURED and the step exits 0.
- `RULED_CEILING_RAISES` is the ruled-raises record. Each entry is a
package, its raised seconds and the ruling's comment URL. An entry
without a well-formed ruling URL is refused, and the step goes red.
- **`.github/workflows/ci.yml`.**
- `Check the per-package suite-duration ceilings` is a new last step of
the `Test Core` aggregator job, `if: always()`, with no
`continue-on-error`.
- The aggregator's comment block now names it as the one step there that
does red the job.
- Not changed: the job and step names, `needs:`, the matrix, the shard
`timeout-minutes` (45), the drift step and its constants (1.5× red, 1.3×
warning), the seven required contexts, and #22075's split
(`FILE_SHARDED_PACKAGES`, the density cap, `planShards()`).
- **`.github/workflows/lint.yml`.** New step `Suite-duration ceiling
check self-test` (wiring for `check-self-test-wired`).
- **`.github/workflows/shard-timings-refresh.yml`.**
- The PR body gets a ceiling line. It says whether this refresh writes
the FIRST table, or holds the existing one and adds new packages. It
also names the uncapped packages, and any package that ran over its held
ceiling in the window.
- The header records that a refresh never moves a ceiling.
- No change to selection, download, the generate loop or the write step.
## The table: a new field in the dataset, not a new file
The claim left this choice to me. I chose a field, for three reasons:
- **The pairing is structural.** One generator call writes the weights
and the ceilings into one file, and the refresh lane's existing `cmp`,
`git add` and PR body cover both. Nothing can refresh one without the
other.
- **"No table yet" is a property of the file.** An absent `ceilings` key
means a dataset written before this generator version. The dataset
already uses the same pattern for `provisional`. After the first refresh
the key is always present, so deleting it would mean hand-editing a
generated file.
- **The ruling's "beside the timings file" holds as "beside the
weights".** `packages` is untouched, and so is every reader of it.
## Bootstrap: no seat can write the first table (assumption 1,
confirmed)
- I confirmed it once, on artifact 11612571468 of run 37921147590. `GET
/actions/artifacts/{id}/zip` answered `302` to
`productionresultssa3.blob.core.windows.net`. The container's egress
then answered `CONNECT tunnel failed, response 403`, and the proxy
logged `connect_rejected ... 403` for that host.
- Job logs redirect to the same host, and MCP
`download_workflow_run_artifact` returns the same blob URL.
- So the table is generated only by the refresh lane on a hosted runner.
No ceiling is typed by hand, and none is seeded from PR #22368's body.
- The bootstrap path:
1. This PR merges. The check reads NOT MEASURED, with a warning
annotation, on every run.
2. The next refresh (scheduled Monday 05:30 UTC, or a manual dispatch by
the maintainer) writes `ceilings`. It opens its PR with the "FIRST
suite-duration ceiling table" paragraph and the ceiling column.
3. That PR merges, and the check grades from the next run on.
- This PR's own `pull_request` run of the refresh lane (it touches the
generator) is a dry run on real artifacts, so its step summary shows the
table a scheduled run would write. Nothing is pushed from it.
## A's false-red rate (build order, before the check was built)
**Over the 21-run window: 0, by construction.** Every reading in the
window is at most the window maximum, and the ceiling is the window
maximum × 1.25.
**On the runs after the window** (assumption 2), the measurement that
can say something:
- **Runs:** all 13 hourly `schedule` runs of `ci.yml` after the window's
newest run (37850732030). That is 37857021445 through 37921147590; 12 of
them ran after the dataset refresh landed (`040184752c`).
- **Readings:** each run's `Test Core` timing table, read from the
aggregator job log (13 MCP `get_job_logs` reads). It lists the 10
slowest executed packages with turbo's own window. The same function
reads those windows for the dataset, for the check, and for the PR
#22368 spread table.
- **Ceilings:** window maximum × 1.25, from PR #22368's spread table.
The table was used as the measurement input only, not as table data.
- **Density per run, by commit ancestry.** Round 2 of #22075
(`c64130bafa`, reverted at `806b03e2ae`) is in the commits of
37897571511 and 37902633826. Every negative ancestry check has a control
leg (`040184752c` is an ancestor of every commit, exit 0).
| runs | executed package-runs | read directly | over A's ceiling |
bounded under by the 10th-slowest reading | provisional | not measured
(below the top-10 cut) |
|---|--:|--:|--:|--:|--:|--:|
| 11 at the dataset's density | 497 | 108 | **0** | 46 | 4 | 339 |
| 2 under round 2's density (37897571511, 37902633826) | 128 | 20 |
**2** | 11 | 2 | 95 |
- **The two exceedances** are both on 37902633826 (`081e6a09d4`), the
one run in which round 2's split executed every package:
- `@objectstack/service-automation`: 567.21 s against 497.55 s (1.14×);
- `@objectstack/metadata-protocol`: 769.55 s against 717.28 s (1.07×).
- On the 11 runs at the dataset's density those two packages read at
most 420.65 s and 529.41 s.
- Round 2 packed ~2641 s of predicted windows onto three shards (1.49×
the density cap), so these readings are the split's packing, not an
unchanged tree. I read them as the brake firing on a pull request that
made suites slower, not as noise, and so I did not stop.
- **Near misses at the normal density:**
- `@objectstack/plugin-approvals` read 218.90 s against 222.47 s (0.98)
on 37869710053. That is 1.23× its window maximum, within a day of the
window.
- `@objectstack/plugin-auth` read 522.15 s against 537.76 s (0.97) on
37886662177.
- So A's 25% over the window maximum has thin margin on some packages.
The week after the first table lands is when this gets read on graded
runs.
- **The coverage gap.** The logged table prints only the top 10
packages, so 339 small-package readings could not be bounded. The check
prints every executed package's reading, ceiling and ratio in its log,
which makes the next measurement of this kind complete.
## The CLI's slices: summed across shards (assumption 3)
- The three slice windows are summed, and the sum is graded against the
whole package's ceiling. That is the quantity the ceiling was measured
in: the generator records a sliced package as the per-run sum of its
slices, and takes the slowest of those sums.
- Comparing each slice to a third of the ceiling was rejected on PR
#22456's measured skew, 0.81 / 1.17 / 1.02 of an even third. With the
CLI ceiling at 1844.58 × 1.25 = 2305.73 s, the heaviest slice would red
at a whole-suite 1970.7 s. That is 6.8% over the window maximum instead
of 25%.
- vitest cuts the file list by a hash of each path. A new or renamed
file can move a slow file into another slice while the suite gets no
slower.
- Summing needs all three shards, so the check runs in the `Test Core`
aggregator, which already downloads every shard's capture. The ruling's
"after the shard's tests" is met as "after all shards' tests".
- A set missing a part is NOT MEASURED for that package. A part is never
graded as the whole.
## Density (assumption 4)
- The ceilings assume the packing density the dataset's own split runs
at. The live split (PR #22456) holds that density under its density cap:
no shard carries more predicted windows than the densest bin of the
whole-package split, 1772.65 s today.
- A package's window is contended wall clock, so it grows with its
shard's density. Round 2 measured 1.5–1.9×, and the table above shows
two packages over A's ceiling on that run.
- A future split that raises the density will red this check on packages
it did not touch. That red is right: the split made those suites slower.
The remedy is the split, not a raise.
- If a split that raises density is ever wanted on purpose, its PR
should carry the ruled raises with it.
## Slowest files (assumption 5)
These come from the per-file capture of #16454 that already runs in each
shard, the vitest module lines in each capture's `files`. The check
imports no reader of its own for them: it reads the captures the
aggregator already downloads.
## A decision this PR makes, flagged for a ruling: a refresh HOLDS a
ceiling
The ruled clauses meet head-on at the second refresh:
- "the ceiling is the package's slowest executed run in the refresh
window × 1.25";
- "refreshed with it";
- "a ceiling rises only by a ruling".
This PR **holds**: a refresh writes a ceiling only for a package that
has none (new, or newly non-provisional), and never raises or lowers an
existing one. Raising follows the ruling. Not lowering is my call, for
one reason: a quieter window is variance, not a faster suite, and a
ceiling ratcheted down on variance is red on the next ordinary run. The
plugin-approvals reading above is that variance, 1.23× its window
maximum within a day.
The alternatives are a one-line change in `suiteCeilings()`:
- shrink-only (lower on a quieter window, never raise), which is the
line ratchets' rule;
- free-follow (recompute every refresh, so ceilings rise with growth).
The first table is identical under all three, so this only needs
answering before the second refresh. The open question is in my report
on the card.
## A consequence of clause 3 the card should know
- A package absent from the dataset is red. A refresh measures only what
runs on `main`, and a package whose PR is red never reaches `main`.
- So a new package can never get a ceiling by waiting for "the next
refresh". Its first ceiling comes from a ruled raise in
`RULED_CEILING_RAISES`, which the check honours for a package absent
from the dataset.
- Renaming a package has the same effect.
## Gates (head `caffe8376d`, after one `origin/main` merge)
- **Derivation.** `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` on `caffe8376d` derived 66 commands: 34
`pnpm`, 32 direct `node`, over 5 paths against the merge base
`f66c440de`. That is the same list as before the merge.
- **Runs.** All 66 ran on `caffe8376d`, each with its exit code captured
before any pipe. The 22-minute `pnpm check:pm-dispatch-gates` battery
ran in the background, wrote its exit code to a file, and I waited on it
in the foreground: exit 0, `dispatch-gates self-test: 2011 cases pass`.
- **Reconciliation.** `--ran`: `Run reconciliation — 66 derived, 66 run,
0 NOT-MEASURED, 0 UNRUN`, then `✓ dispatch-gates --ran: 66 derived
famil(ies) accounted for — 66 run, 0 NOT-MEASURED (a DERIVED zero — all
66 recorded an exit code and none of them is 3)`. All 66 exited 0. They
include:
- `node scripts/check-test-suite-ceilings.mjs --self-test`: `self-test
OK (35 cases across 7 batteries)`;
- `node scripts/measure-test-shard-timings.mjs --self-test`: `self-test
OK`. Its new battery `suite-duration ceilings (#16468)` has 14 cases,
and the roster floor goes 3 to 4;
- `node scripts/partition-test-shards.mjs --self-test`: `bins
1772.63/1771.02/1772.63/1770.95/1771.09/1772.46s within the 1772.65s
density cap, file-level slices: @objectstack/cli x3`. The split is
unchanged;
- `node scripts/ci/select-shard-timings-run.mjs --self-test`, which
drives the refresh lane's real generate step with the new generator;
- `check-self-test-wired`, `check-self-test-workflow-commands`,
`check:required-contexts`, `check:shard-attestation`,
`check-aggregator-roster`, `check-step-collectors`,
`check:stall-guard-budget`, `check:stall-guard-headroom` and
`check:nul-bytes`.
- **Before the merge, at `8ab0fb8f0a`.** The same battery went red, 1 of
2011 cases, on the import described in the acceptance notes. The check
was changed at `68ca4e88d7`, and the rerun above is green.
- **Lint, a declared narrowing, at `caffe8376d`.** `eslint
--no-inline-config --format json` over the two changed scripts reported
2 files, 0 errors and 0 warnings.
- The population comes from eslint's own config: `isPathIgnored` is
false for both scripts and true for `ci.yml`, and workflow YAML is not
an eslint input.
- The computed config for both scripts has no `parserOptions.project`
and no `projectService`. So no type-aware rule runs, and this diff
cannot move an untouched file's verdict.
- The full `pnpm lint` is CI's.
## Ablations (the code committed first; each through `node
scripts/ablation-replace.mjs`, restore proven)
Each mutation was made at head `68ca4e88d7`. Each was confirmed on disk
by the tool (anchor 1 → 0, blob changed) before the self-test ran. Each
was restored with blob == HEAD and an empty `git diff HEAD`. These are
plain node scripts, with no build or `dist/` step.
1. **Over-ceiling red removed.**
- Mutation: `row.status = seconds > row.ceiling ? 'over' : 'ok';` became
`row.status = 'ok';` (blob `1f01cb9c9543` → `40c38e79e976`).
- Result: RED, `Error: over: exit 0, verdict OK`. Restored to
`1f01cb9c9543`.
2. **Provisional exemption removed.**
- Mutation: `if (inDataset && table.provisional.has(name)) {` became `if
(false) {` (blob → `e34689295a71`).
- Result: RED, `Error: provisional: exit 0, uncapped`. Restored.
- The direction is worth knowing. With the exemption gone, a provisional
package that is `uncapped` falls to the uncapped row ("no ceiling:
provisional"), still not red. A provisional package that still holds a
ceiling turns red.
- Measured with `--hold` on a one-off fixture (a provisional package at
9999 s holding a 12 s ceiling): restored code gave `NOT MEASURED 0`, the
mutated code gave `OVER 1`. Restored, blob == HEAD.
3. **The ruled reading (window maximum, not median) removed from the
generator.**
- Mutation: `Math.max(...values)` became `median(values)` in
`ceilingOf()` (blob `4c08f3d8463b` → `44bda0d56fe4`).
- Result: RED, the spread row read `25.00` (median × 1.25) instead of
`37.50`. Restored.
4. **The no-table reading removed.**
- Mutation: the `if (!Object.hasOwn(dataset, 'ceilings')) return {
table: null };` line deleted (blob → `94844d5c01e1`).
- Result: RED. A pre-ceilings dataset becomes a refusal, `` `ceilings`
is undefined ``, instead of NOT MEASURED. Restored.
5. **Slice completeness removed.**
- Mutation: `complete: row.sliceCount === null || parts.length ===
row.sliceCount` became `complete: true` (blob → `4eb8e8b191c1`).
- Result: RED, `Error: slices: an incomplete set read over / OVER`.
Restored.
## Acceptance notes
- **`check:pm-dispatch-gates` caught a real interaction.** The first
draft of the check imported `mergeCaptures` from
`report-test-timings.mjs`. That module is a gate file only through its
own self-test, so a gate family importing it stops inheriting its
declared populations in the dispatch-gates derivation. The battery's
promotion invariant went red on exactly that (1 of 2011 cases). The
check now folds the captures itself (`foldCaptures`): the same sum and
completeness mark, and an unknown capture schema is named and left out.
The turbo-summary reader is still the shared `samplesFromSummary`, which
runs on the shard when the capture is taken.
- **A stale comment, noted rather than edited.**
`report-test-timings.mjs`'s header says `samplesFromSummary` "reads the
`test` task only". Since the `test:repo` fold it sums both tasks. That
file is outside this card's surface. Comment only; noted, not filed.
- **`skip-changeset`:** root `scripts/` and workflows publish nothing.
---
_Generated by [Claude
Code](https://claude.ai/code/session_0115N1oNnQS5WqofZ2DzaT3q)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 7895671 commit 6abd265
5 files changed
Lines changed: 1234 additions & 14 deletions
File tree
- .github/workflows
- scripts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1391 | 1391 | | |
1392 | 1392 | | |
1393 | 1393 | | |
1394 | | - | |
| 1394 | + | |
| 1395 | + | |
| 1396 | + | |
| 1397 | + | |
1395 | 1398 | | |
1396 | 1399 | | |
1397 | 1400 | | |
| |||
1447 | 1450 | | |
1448 | 1451 | | |
1449 | 1452 | | |
| 1453 | + | |
| 1454 | + | |
| 1455 | + | |
| 1456 | + | |
| 1457 | + | |
| 1458 | + | |
| 1459 | + | |
| 1460 | + | |
| 1461 | + | |
| 1462 | + | |
| 1463 | + | |
| 1464 | + | |
| 1465 | + | |
| 1466 | + | |
| 1467 | + | |
| 1468 | + | |
| 1469 | + | |
| 1470 | + | |
| 1471 | + | |
| 1472 | + | |
| 1473 | + | |
| 1474 | + | |
| 1475 | + | |
| 1476 | + | |
| 1477 | + | |
| 1478 | + | |
| 1479 | + | |
| 1480 | + | |
| 1481 | + | |
| 1482 | + | |
| 1483 | + | |
| 1484 | + | |
1450 | 1485 | | |
1451 | 1486 | | |
1452 | 1487 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4369 | 4369 | | |
4370 | 4370 | | |
4371 | 4371 | | |
| 4372 | + | |
| 4373 | + | |
| 4374 | + | |
| 4375 | + | |
| 4376 | + | |
| 4377 | + | |
| 4378 | + | |
| 4379 | + | |
| 4380 | + | |
| 4381 | + | |
4372 | 4382 | | |
4373 | 4383 | | |
4374 | 4384 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
36 | 46 | | |
37 | 47 | | |
38 | 48 | | |
| |||
634 | 644 | | |
635 | 645 | | |
636 | 646 | | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
637 | 694 | | |
638 | 695 | | |
639 | 696 | | |
| |||
675 | 732 | | |
676 | 733 | | |
677 | 734 | | |
678 | | - | |
| 735 | + | |
679 | 736 | | |
680 | 737 | | |
681 | 738 | | |
| 739 | + | |
| 740 | + | |
682 | 741 | | |
683 | 742 | | |
684 | 743 | | |
| |||
0 commit comments