Skip to content

Commit 430859c

Browse files
committed
ci(test-core): wire the shard timing-drift check above the attestation pair
Runs partition-test-shards.mjs --check-drift over .turbo/runs/*.json on every shard, with no if: and no continue-on-error, so a red past 1.5x measured/ predicted withholds the attestation; past 1.3x it annotates. Restates the comments the wiring made stale: the drift block's CLI figures, WHY SIX (the CLI is the heaviest indivisible suite on the refreshed dataset), the 45-minute timeout's revert condition against measured shard wall time, and the idle slice-leg prose. Claude-Session: https://claude.ai/code/session_01VF48aw8RPG6wzDnMgp6rtw Co-authored-by: Claude <noreply@anthropic.com>
1 parent 094d962 commit 430859c

1 file changed

Lines changed: 118 additions & 72 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 118 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -412,12 +412,17 @@ jobs:
412412
# shard, because every shard gets its own runner.
413413
#
414414
# SIX specifically, and not four or five: sharding is BY PACKAGE, so no
415-
# shard can finish faster than its single heaviest package.
416-
# `@objectstack/spec` is that package — at five shards or fewer the
417-
# partitioner must co-schedule it with others, and at six it very nearly
418-
# fills a bin on its own. Six is therefore the smallest count that
419-
# isolates the heaviest indivisible suite; past six, spec's shard cannot
420-
# improve, only the others can.
415+
# shard can finish faster than its single heaviest package. On the
416+
# current dataset (scripts/test-shard-timings.json as #21826 refreshed it,
417+
# provenance run 37262126122) that package is `@objectstack/cli` at
418+
# 1702.69s, ahead of `@objectstack/spec` at 1134.86s. At five shards or
419+
# fewer the partitioner must co-schedule it with others (heaviest bin
420+
# 1957s at five); at six it fills bin 1 alone and the other five sit at
421+
# 1615-1617s. Six is therefore the smallest count that isolates the
422+
# heaviest indivisible suite; past six, the CLI's shard cannot improve,
423+
# only the others can. (spec held that place until #21826 measured the
424+
# CLI whole: the dataset before it recorded the CLI at 733.33s, summed
425+
# from two file-level slices, and #21487 had already retired the slicing.)
421426
#
422427
# ⚠ THE ARGUMENT ABOVE USED TO BE MADE IN TEST-FILE COUNTS (bins
423428
# 415/389/389/389/389/388, spec carrying 415 of ~2360 files). It is now
@@ -428,21 +433,24 @@ jobs:
428433
# measurement; only the units it is argued in changed. Weights now come
429434
# from scripts/test-shard-timings.json — see partition-test-shards.mjs.
430435
#
431-
# AND WHY NOT MORE THAN SIX, measured on that dataset (#10472 asked for
432-
# 6 -> 8 to be considered). The floor is the heaviest single package, and
433-
# it does not move when shards are added, while the mean falls with every
434-
# shard — so max/mean, which is what the acceptance bound is written in,
435-
# gets WORSE past the point where the floor becomes the max:
436+
# AND WHY NOT MORE THAN SIX (#10472 asked for 6 -> 8 to be considered).
437+
# The floor is the heaviest single package, and it does not move when
438+
# shards are added, while the mean falls with every shard — so max/mean,
439+
# which is what the acceptance bound is written in, gets WORSE past the
440+
# point where the floor becomes the max. Re-derived on the current dataset
441+
# with the partitioner's own partition() and balanceOf():
436442
#
437-
# 6 shards -> max 650s mean 649s ratio 1.00x
438-
# 7 shards -> max 571s mean 556s ratio 1.03x
439-
# 8 shards -> max 571s mean 487s ratio 1.17x
440-
# 10 shards -> max 571s mean 389s ratio 1.46x
443+
# 6 shards -> max 1703s mean 1630s ratio 1.04x
444+
# 7 shards -> max 1703s mean 1397s ratio 1.22x
445+
# 8 shards -> max 1703s mean 1223s ratio 1.39x (past the 1.3x bound)
446+
# 10 shards -> max 1703s mean 978s ratio 1.74x
441447
#
442-
# Eight shards would buy ~79s off the critical path for two more runners'
443-
# fixed overhead and a worse balance ratio. The partitioner's self-test
444-
# pins this arithmetic so the next person gets the answer from a failing
445-
# assertion rather than from a CI run.
448+
# Past six the critical path does not move at all — the CLI's 1703s is
449+
# the maximum at every count — so more shards buy only more runners'
450+
# fixed overhead and a worse ratio. The partitioner's self-test pins the
451+
# half of this that can fail: at SHARD_COUNT the heaviest package must
452+
# sit within 1.3x the mean, and when it stops fitting, slicing it below
453+
# package granularity is the remedy that failure names.
446454
#
447455
# ⚠ THE COST, stated because it is real: per-shard fixed overhead
448456
# (checkout + pnpm/Turbo cache restore + install, ~60s measured on run
@@ -465,31 +473,33 @@ jobs:
465473
if: ${{ !cancelled() && (needs.filter.outputs.core != 'false' || needs.filter.outputs.crosspkg != 'false') }}
466474
runs-on: ubuntu-latest
467475
# Backstop only — the stall guard on the test steps is the primary
468-
# detector for a #4250-style hang and fires well before this. 30 min is
469-
# ~5× a normal sharded run (~4-6 min), with margin for a cold Turbo cache;
470-
# the old 45 left a hung job "running" for half an hour past any plausible
471-
# healthy finish.
476+
# detector for a #4250-style hang and fires well before this. 30 min was
477+
# chosen as ~5× a normal sharded run when a normal run was ~4-6 min, with
478+
# margin for a cold Turbo cache; the old 45 left a hung job "running" for
479+
# half an hour past any plausible healthy finish.
472480
#
473-
# ⚠ TEMPORARY RAISE, 30 -> 45 (#16173). Shard 5/6 is being killed at the
474-
# 30-minute wall — 12+ observations at 30:16-30:21 with every other job in
475-
# every one of those runs green — so the paragraph above describes the
476-
# value this line must RETURN to, not the value it currently carries.
481+
# ⚠ RAISED 30 -> 45 (#16173), and the raise is still load-bearing. It was
482+
# made because shard 5/6 was being killed at the 30-minute wall (12+
483+
# observations at 30:16-30:21 with every other job green; the green band
484+
# topped out at 28:56, a ~1 minute margin), and so the tail could be
485+
# measured uncensored before anyone re-derived
486+
# `scripts/test-shard-timings.json`. Both are done: the dataset was
487+
# refreshed (#20388, then #21826) and the split now runs the whole CLI
488+
# alone on shard 1/6 (#21487).
477489
#
478-
# Two reasons, both #16173's:
479-
# 1. Unblock. The only uncensored shard-5 readings are 25:32 and 27:36,
480-
# and shard 6/6 has been seen at 28:56 — a ~1 minute margin at the top
481-
# of the observed green band against a 30:00 wall.
482-
# 2. Un-censor. Every tail observation is a reading of THIS number, not
483-
# of the shard: the true duration is >= 30:16, unbounded above. So the
484-
# shard timings cannot be re-derived from CI history while the wall
485-
# stands here. Raising it produces the durations #16173 needs before
486-
# anyone touches `scripts/test-shard-timings.json`.
487-
#
488-
# ⛔ REVERT CONDITION, explicit: back to `30` once #16173 lands its shard
489-
# rebalance. This value is temporary and carries no other expiry — a raise
490-
# with no revert condition written beside it becomes permanent by
491-
# forgetting. The backstop stays loose only for that window; the stall
492-
# guard named above remains the primary hang detector throughout.
490+
# ⛔ REVERT CONDITION, restated against measured wall time, not a card:
491+
# back to `30` only when the slowest shard's JOB wall time stays at or
492+
# under 24 minutes (80% of 30, so the margin the old wall lacked is
493+
# there) on every scheduled run for a week. It does not today. On the 16
494+
# main runs after the #21826 refresh (37413386379 to 37467882762), shard
495+
# 1/6 — the CLI alone — read 6m14s-35m43s of job wall time, over 30
496+
# minutes on 9 of them (34m39s run 37415122516, 35m43s run 37453598388,
497+
# 34m08s run 37460325624), so the condition as first written ("once
498+
# #16173 lands") would now kill that shard; and its slowest reading is
499+
# already 79% of this 45. Read the `Test Core (1/6)` job's started and
500+
# completed times before touching this value. It carries no other expiry
501+
# — a raise with no revert condition beside it becomes permanent by
502+
# forgetting — and the stall guard stays the primary hang detector.
493503
timeout-minutes: 45
494504
permissions:
495505
contents: read
@@ -697,8 +707,11 @@ jobs:
697707
# and `pnpm check:stall-guard-headroom` both read this step, so it keeps
698708
# its own `--stall-minutes` and its own headroom row.
699709
#
700-
# A shard with no slice runs zero iterations here; every shard still
701-
# reaches the step, so its name is a stable site for those two gates.
710+
# A shard with no slice runs zero iterations here, and since #21487
711+
# emptied FILE_SHARDED_PACKAGES that is every shard: no package is sliced
712+
# today, so the paragraphs above describe the mechanism a slice would
713+
# use, not anything this job currently runs. Every shard still reaches
714+
# the step, so its name is a stable site for those two gates.
702715
- name: Build the sliced package's dependency closure
703716
env:
704717
NODE_OPTIONS: --report-on-signal --report-signal=SIGUSR2 --report-directory=${{ runner.temp }}/stall-reports
@@ -805,7 +818,7 @@ jobs:
805818
# ⛔ A failing leg STOPS the remaining ones, the same way turbo stops
806819
# scheduling on the first failure inside one run. Carrying on would add
807820
# a second full suite to a job that is already red and already inside a
808-
# 30-minute wall — turning an informative red into a killed job with no
821+
# `timeout-minutes` wall — turning an informative red into a killed job with no
809822
# attestation at all, which is the #16173 failure mode itself.
810823
#
811824
# `test test:repo` (#16466): six heavy packages split their suite into
@@ -815,8 +828,11 @@ jobs:
815828
# and is a no-op for every package without a `test:repo` script. The
816829
# per-shard Turbo cache restored above (main-seeded) is what carries
817830
# an unmoved `PKG#test` across a change outside the package; the
818-
# completeness guard below reads both tasks' summaries. The cli slice
819-
# leg stays `test`-only: cli is not split.
831+
# completeness guard below reads both tasks' summaries. A slice leg
832+
# stays `test`-only: the CLI, the one package whose slice wiring is
833+
# in the tree, is not split. No package is sliced today
834+
# (FILE_SHARDED_PACKAGES is empty since #21487), so every shard runs
835+
# the whole-package leg alone and the slice branch below is idle.
820836
STATUS=0
821837
LOGS=""
822838
for LEG in __whole__ $SLICES; do
@@ -956,30 +972,60 @@ jobs:
956972
if-no-files-found: ignore
957973
retention-days: 1
958974

959-
# ⛔ THE DRIFT STEP IS DELIBERATELY NOT WIRED HERE YET (#16173).
960-
#
961-
# partition-test-shards.mjs carries a fully-tested `--check-drift` mode —
962-
# it reads the summary uploaded above back and reds when a shard's MEASURED
963-
# test total outruns its PREDICTED one past MAX_MEASURED_OVER_PREDICTED. The
964-
# code, its self-tests and its ablation are all on this branch; only this
965-
# invocation waits, and the wait is a SEQUENCING decision, not an oversight.
966-
#
967-
# Why: scripts/test-shard-timings.json is still stale for @objectstack/cli
968-
# (458.15s recorded, 1231.52s measured), so wiring the step today would red
969-
# the shard carrying a CLI slice on every single PR — a true reading, but one
970-
# that blocks everything until the dataset is refreshed. The file-level split
971-
# above already removes the urgent hazard on its own, taking the worst shard
972-
# from ~1445s (80% of this job's 30-minute wall) to ~1059s (59%) with the
973-
# dataset untouched, because the CLI is halved across two runners instead of
974-
# falling on one.
975-
#
976-
# ⇒ The refresh and this step land TOGETHER in the follow-up, in that order.
977-
# The refresh recipe is in PR #16220's body. When it lands, restore a step
978-
# here that runs `--check-drift` over `.turbo/runs/*.json` with
979-
# `--label "Test Core (${{ matrix.shard }}/6)"`, with NO `if:` and NO
980-
# `continue-on-error` (the point is the red), placed ABOVE the attestation
981-
# pair for the #6082 reason documented on the upload above — so a drift red
982-
# also withholds the attestation, which is the fail-closed direction.
975+
# ── Predicted vs measured: this shard's timing drift (#16173, #16465) ──
976+
#
977+
# partition-test-shards.mjs `--check-drift` reads back the summary the
978+
# step above uploads and compares this shard's MEASURED test total with
979+
# what the split PREDICTED for the same packages. One ratio,
980+
# measured/predicted, and four verdicts on it:
981+
#
982+
# OK at or under 1.3x (WARN_MEASURED_OVER_PREDICTED)
983+
# WARN past 1.3x, at or under 1.5x: a `::warning::` annotation
984+
# naming the shard and its heaviest overshoot; green
985+
# DRIFT past 1.5x (MAX_MEASURED_OVER_PREDICTED): red
986+
# NOT MEASURED no executed test task carried a dataset entry; green
987+
#
988+
# ONE red rule and a warning under it, on the same ratio: triage ruled
989+
# the in-tree 1.5x the red and the card's 1.3x a warning only, and took no
990+
# second red on wall time against `timeout-minutes`. Both constants, their
991+
# measured basis and a self-test battery each live in the script.
992+
#
993+
# EXECUTED WINDOWS ONLY. Measurements come from samplesFromSummary() in
994+
# measure-test-shard-timings.mjs, which drops cache HITs and failed
995+
# suites, and the prediction is summed over the same executed packages.
996+
# A replay's near-zero window would read as a shard far FASTER than
997+
# predicted and vouch for a rotted dataset, so a shard whose packages all
998+
# replayed is NOT MEASURED, never "fast".
999+
#
1000+
# NO `if:` and NO `continue-on-error` (the red is the point), and ABOVE
1001+
# the attestation pair for the #6082 reason documented on the upload
1002+
# above: a failed unguarded step skips that pair, so a drift red also
1003+
# withholds this shard's attestation, the fail-closed direction. A shard
1004+
# with no packages runs no turbo and writes no summary; that is NOT
1005+
# MEASURED here rather than a usage error from the script.
1006+
#
1007+
# The dataset it reads is scripts/test-shard-timings.json as #21826
1008+
# refreshed it (provenance run 37262126122): the whole CLI at 1702.69s,
1009+
# alone on shard 1/6 since FILE_SHARDED_PACKAGES went empty (#21487).
1010+
# The second sample taken before wiring this, executed windows only, from
1011+
# the `Test Core` timing tables of the main runs after that refresh:
1012+
# shard 1/6 read 0.61x-1.05x on all eight scheduled runs (37416453417 to
1013+
# 37467795959), and no package on any shard read 1.5x, the highest being
1014+
# @objectstack/spec at 1.39x-1.45x on the three runs that executed it
1015+
# (37433381795, 37453598388, 37467882762). spec carries 70% of shard
1016+
# 2/6's prediction, so that shard is the one nearest the warning. Those
1017+
# tables print ten packages, so shards 2-6 were bounded there rather
1018+
# than read whole; this step reads them whole. Per-shard detail: #16465.
1019+
- name: Check this shard's timing drift
1020+
run: |
1021+
shopt -s nullglob
1022+
SUMMARIES=(.turbo/runs/*.json)
1023+
if [ "${#SUMMARIES[@]}" -eq 0 ]; then
1024+
echo "shard-timing-drift: NOT MEASURED -- Test Core (${{ matrix.shard }}/6) wrote no turbo run summary (no packages on this shard)."
1025+
exit 0
1026+
fi
1027+
node scripts/partition-test-shards.mjs --check-drift "${SUMMARIES[@]}" \
1028+
--label "Test Core (${{ matrix.shard }}/6)"
9831029
9841030
# Runs even when the suite failed — that is when it earns its keep. It
9851031
# answers TWO questions about a red suite, and needs both to be able to
@@ -1062,7 +1108,7 @@ jobs:
10621108
# slowTestThreshold (which this repo never sets — it picks
10631109
# the colour, not the presence). So the stream is enough
10641110
# and no machine-readable reporter is added to the shard's
1065-
# 30-minute wall. The script's header carries the
1111+
# `timeout-minutes` wall. The script's header carries the
10661112
# measurement and its control.
10671113
# per PACKAGE `.turbo/runs/*.json`, the same `--summarize` output the
10681114
# step above already uploads — turbo's own execution

0 commit comments

Comments
 (0)