ci: park perf-nightly to dispatch and stop the coverage-gate cascade double-red (#1781 A3, A5) - #1822
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks: no changes in the largest emitted chunks. |
|
The workflow change is semantically sound, but two readiness items remain. First, the cascade fix has only green-path evidence: per the repo’s planted-red rule, temporarily force |
a435241 to
f9c62b9
Compare
|
Both items addressed. Planted-red witness for the cascade fix — pushed a temporary commit Cost claim — PR body rewritten: no dollar figures; the defensible saving is ~22 min/night of macOS runner occupancy (Actions minutes are free on this public repo). Current head is the clean |
|
Re-reviewed #1822: the prior evidence/body blockers are resolved. Planted run 32145339142 shows |
…double-red (#1781 A3, A5) A3: perf-nightly writes a report and compares nothing, so it structurally cannot catch a regression. iOS wall-clock medians swing up to +122% night-to-night at n=5 (a comparator would print noise), no doc/issue reads the report, and the iOS job holds a macOS runner ~22min nightly. Parked to workflow_dispatch following the #1781 A1 pattern (replays-manual.yml); it declares no gate-manifest check, so no declarations.ts change is needed. `pnpm perf` / scripts/perf are untouched. A5: the "Enforce changed-line coverage gate" step ran `if: always()`, so when the preceding "Run coverage" step failed, lcov.info was never written and this step failed too with "no lcov report" -- a cascade double-red, not a coverage verdict. 16 of the last 17 red instances (60d) were this cascade; the step now runs only when Run coverage succeeded.
f9c62b9 to
1c2466d
Compare
|
Rebased onto main ( |
|
Re-reviewed exact 1c2466d: clean and merge-ready. The rebase resolves the inherited Layering failures without changing the two-workflow scope. Planted run 32145339142 proves coverage failure skips enforcement; the clean head executes enforcement successfully. The public-repo runner-occupancy claim remains accurate, and all exact-head gates and device smoke lanes are green. |
thymikee
added
the
ready-for-human
|
1 participant
What / why
Two small, decided CI changes from #1781.
A3 — park
perf-nightly.ymlto dispatch-onlyperf-nightly.ymlwrites a JSON/markdown perf report and compares nothing (scripts/perf/report.tsjust serializes the run; greppingscripts/perf+ the workflow forbaseline|threshold|regress|compare|deltafinds zero implementation, only a comment stating the intent). It therefore structurally cannot report a regression — a run only turns red on a harness crash.Evidence (60-day run history + artifact diff):
type+122%,snapshot (deep)+74%,open (relaunch→root)+60%,boot device+39%. A fixed-threshold comparator would mostly print noise without multi-night smoothing.grep -rniE "perf-nightly|scripts/perf" docs/ CONTEXT.md AGENTS.md README.md→ no hits;gh issue list --search "perf-nightly"→ only Prune the test lanes that don't pay for themselves, and add the checks we're missing #1781 itself).Implemented exactly the #1781 A1 parking pattern (
replays-manual.yml): dropped theschedule:trigger, keptworkflow_dispatch, added a short header comment naming the parking reason and the un-park condition.pnpm perfandscripts/perfare untouched — still runnable on demand.scripts/gate/declarations.tsunchanged. perf-nightly declares nopnpm gate <name>check of its own —pnpm perfisn't a registered gate, and the onlygate:reference in the file (swift-runner-ios, used to build the shared XCTest runner) is already declared and owned by other lanes (ios.yml,xctest-nightly.yml,conformance-differential.yml).pnpm check:gate-manifestreports the same47 checks / 33 lanesbefore and after this change, so nothing needed aMANUAL_ONLY_OWNERSentry.Docs: the only
perf-nightlymention outside the workflow itself isdocs/agents/contract-projection-output-economy-spike.md:159, a dated investigation closeout describing a past one-off dispatch attempt — not living documentation asserting perf-nightly runs on a schedule. Left as-is; rewriting a historical log to match a later architecture change would misrepresent what happened at the time.A5 — stop the changed-line coverage gate's cascade double-red
In
ci.yml'scoveragejob, "Enforce changed-line coverage gate" ranif: always() && github.event_name == 'pull_request'. When the preceding "Run coverage" step (unit-ci, the vitest-coverage run) failed,coverage/lcov.infowas never written, and the Enforce step then failed too with "no lcov report" — a second, redundant red from the same job, not a coverage verdict.Retrospective (60-day window, 3259 CI runs): 17 runs had the Enforce step fail. 16/17 were this lcov-missing cascade (Run coverage already failed/crashed). Only 1/17 was a genuine, isolated block: run 32104571856 / PR #1804 ("feat(ai-sdk): add agent-device/ai-sdk tool set"), 6.90% changed-line coverage on a large new file — correctly caught, PR was fixed and merged.
Fix: gave "Run coverage" an explicit
id: run-coverageand changed the Enforce step's condition tosteps.run-coverage.outcome == 'success' && github.event_name == 'pull_request'. The checker script (scripts/coverage-changed/run.ts) is unchanged.Four lines — both lanes, as they stand after this change
perf-nightly (parked, dispatch-only):
pnpm perf/ scripts/perf still cost nothing extra when run on demand.changed-line coverage gate (still gating, cascade removed):
vitest.config.ts) would hide — 1 confirmed instance in 60 days.coverage-waiverlabel doesn't exist in the repo).Test plan
pnpm format:check,pnpm lint,pnpm typecheck— cleanpnpm check:gate-manifest—47 checks wired across 33 lanes, manual-only: replay-android, replay-ios, replay-ios-device(unchanged before/after)pnpm check:gate-manifest:test— 37/37 passingyaml.safe_load) for both changed workflow filescoveragejob's Enforce step behavior is unaffected on the (expected) green path