diff --git a/quest/m0/nightly/shipping-size.md b/quest/m0/nightly/shipping-size.md index 41469845a53..f828ce9d01c 100644 --- a/quest/m0/nightly/shipping-size.md +++ b/quest/m0/nightly/shipping-size.md @@ -12,28 +12,68 @@ Decided 2026-11-03 (maintainer, /quest-plan of the follow-ups from [moq-dev/moq#7667](https://github.com/moq-dev/moq/pull/7667)): milestone m0, a child of the Nightly epic. -The scheduled run of 2026-11-03 -([job](https://github.com/moq-dev/moq/actions/runs/41656613559/job/126039954216)) -failed on growth since the previous run: - -- `js/net` raw went from 444,692 to 467,348 bytes. -- `js/watch` also grew past the threshold. -- All four moq-relay archives grew to about 10.2–11.3 MB, from about - 9.5–10.3 MB. - -Bisect each artifact between the two runs' commits. Likely suspects are the -recent bigint group IDs ([moq-dev/moq#7681](https://github.com/moq-dev/moq/pull/7681)) -for js/net, and any new default feature or dependency for moq-relay. -`cargo bloat` and the bundle's metafile name the code that grew. - -If the threshold compares only against the previous run, one intended jump -fails every night until the baseline moves. Check how the job sets its -baseline, and give it a documented way to accept a reviewed growth. -[Size audit](/quest/m2/audit-size.md) owns the release-time view of these -sizes. +The [2026-11-03 shipping-size job](https://github.com/moq-dev/moq/actions/runs/41656613559/job/126039954216) +failed nine daily comparisons between +[dde50c699f](https://github.com/moq-dev/moq/commit/dde50c699f69507ced243754c2f700a6184d1576) +and [801fd9a2f2](https://github.com/moq-dev/moq/commit/801fd9a2f2f5c88371fa833d805620d97d372465). +[Investigation PR](https://github.com/moq-dev/moq/pull/7813) records every failing +measurement and its attribution evidence: + +- Rebuilt both JS endpoints and all 18 intervening JS/lockfile commits. All 21 + retained endpoint measurements reproduce exactly. Net raw grows 22,656 bytes; + watch raw grows 30,479. Subscription ranges contribute 17,090 and 17,111 + respectively, and stats/feedback adds 6,787 watch bytes. The remaining deltas, + including every failing compression format, are attributed in the PR. +- Bigint group IDs are excluded as a cause: their merge happened after the + failed Nightly commit. +- All four downloaded relay archives and executable code sections grew. Native + x86_64 Linux symbol attribution identifies new rind and rind-rustls alongside + transport's retained AWS-LC. The PR separates local diagnostic builds from + the actual shipping archive sizes. Native per-commit attribution on macOS, + aarch64 Linux, and Windows MSVC remains unfinished; do not label the Linux + measurements as those targets' results. +- The daily baseline already advances after a threshold failure. The failed + run retained its current point, and there was no thirty-day anchor yet. + `sh/size/README.md` documents acceptance using retained history, and tests + cover both daily advancement and the surviving thirty-day guard. A step over + 10% can still fail until a post-jump point becomes the thirty-day anchor; + recording acceptance alone does not waive that check. Summaries now identify + the current and failing baseline commits. + +Decided 2026-11-04 (maintainer, on the investigation PR): + +- ✅ Accept the exactly attributed JS growth (`js/net` and `js/watch`, + +5.02–5.28%) as the cost of the retained features and correctness fixes. + The PR's per-commit table is the acceptance record; the retained November 3 + point is the new daily baseline. +- Require further JS reduction while retaining those behaviors. + +- ✅ Keep the 5% daily and 10% thirty-day guards unchanged. +- Raise or exempt the guards for this step. + +Relay archive acceptance stays open until native per-commit attribution for +macOS, aarch64 Linux, and Windows MSVC is supplied, or the maintainer +explicitly accepts the evidence limit. Keep the retained history intact. +[Size audit](/quest/m2/audit-size.md) owns the release-time view. Public API: none. Wire: none. +## Waiting + +- moq-dev/moq: native per-commit relay archive attribution for macOS, aarch64 + Linux, and Windows MSVC. Windows needs a matching build host; none of the + listed SSH hosts runs Windows. macOS needs the pinned Nix toolchain rather + than kixtop's ambient Rust. Supply before/after archive measurements for the + responsible commits rather than substituting Linux `cargo bloat` estimates, + then put relay acceptance to the maintainer. +- moq-dev/moq: inspect the next scheduled Nightly shipping-size job on main, + its measured revision and retained baseline, for passing JS daily + comparisons and, when an anchor exists, thirty-day comparisons. If a + thirty-day comparison still fails, reduction or further acceptance work + remains, potentially until the post-jump point is thirty days old. No manual + Nightly dispatch; retain this quest until that evidence and the relay + attribution are settled. + ## Related - [Size audit](/quest/m2/audit-size.md) - pre-cut size investigation and the post-release refresh diff --git a/sh/size/README.md b/sh/size/README.md index fde6b67aba1..020fe580b56 100644 --- a/sh/size/README.md +++ b/sh/size/README.md @@ -6,6 +6,11 @@ and real browser consumer bundles (raw, gzip and Brotli). `just gh size-report OUTPUT [--history HISTORY]` compares the report with its previous report (5% growth) and the newest report at least 30 days old (10% growth). +The summary names the measured commit and, for each regression, the baseline's +date and commit. Download those runs' `nightly-size-history` artifacts for +`sizes.json` and JS metafiles before rebuilding: the release sizes are actual +uploaded archives, and a local build on another platform is not a replacement. + Nightly passes its own run, so the sizes are the shipping artifacts for every release target, Linux `x86_64` and `aarch64` included, without rebuilding them. Any retained run works locally too. Each release job's `Build` step durations are @@ -19,8 +24,30 @@ and missing release artifacts are refused. A new artifact starts its own series, named in the summary and compared from its next report, while the existing series keep their history. A retained artifact that goes missing, including by rename, is refused: review the change, then start a fresh history by reporting -without one. A refused report writes no history, so Nightly keeps loading the -retained one; reset it by deleting every `nightly-size-history` artifact: +without one. + +A threshold failure **does write history** before returning failure. Nightly +uploads that history even when the check fails, then the next run uses it as +its daily baseline. An intended jump therefore fails the **daily comparison** +once against the previous point, rather than every night against the last +passing point. + +The thirty-day comparison still applies. A step over 10% can keep Nightly red +until a post-jump point becomes the anchor, thirty days after that point was +recorded. Recording acceptance does not waive this comparison. To accept a +measured daily step, record the causing commits, before/after byte counts, and +reason in the implementation PR, and let the next scheduled run compare against +the retained point. Keep the thresholds unchanged. If the thirty-day comparison +still fails, reduction or further acceptance work remains; daily advancement +alone does not resolve it. Confirm every applicable comparison, the measured +revision, and its baselines before declaring the regression resolved. + +A report refused for invalid input or missing artifacts writes no history, so +Nightly keeps loading the retained one. Only a reviewed restart of **all** +artifact series calls for a fresh history. Reporting locally without `--history` +creates one; Nightly's existing full reset deletes every retained history +artifact (including diagnostic metafiles), and does not belong in routine growth +acceptance: ```sh gh api --paginate 'repos/{owner}/{repo}/actions/artifacts?name=nightly-size-history' \ diff --git a/sh/size/report.py b/sh/size/report.py index 19b3da88562..853579db433 100644 --- a/sh/size/report.py +++ b/sh/size/report.py @@ -4,9 +4,18 @@ import argparse import datetime as dt import json +import re from pathlib import Path +def source(row): + """Name the measured revision, keeping older reports without revision metadata readable.""" + revision = row.get("revision") + if revision is None: + return row["date"] + return f"{row['date']} [{revision[:10]}](https://github.com/moq-dev/moq/commit/{revision})" + + def compare(current, history): """Return regressions; a new artifact starts its series, but a missing one is refused.""" @@ -14,6 +23,10 @@ def validate(row): if row.get("version") != 1 or not row.get("bytes"): raise ValueError("expected a nonempty version 1 size report") date = dt.date.fromisoformat(row["date"]) + if "revision" in row and ( + not isinstance(row["revision"], str) or not re.fullmatch(r"[0-9a-f]{40}", row["revision"]) + ): + raise ValueError("expected a full Git commit SHA for revision") for name, size in row["bytes"].items(): if not isinstance(size, int) or isinstance(size, bool) or size <= 0: raise ValueError(f"invalid byte count for {name}: {size}") @@ -36,7 +49,7 @@ def validate(row): if anchors: comparisons.append(("30 days", anchors[-1], 0.10)) return [ - f"{name}: {size} > {row['bytes'][name]} bytes + {limit:.0%} ({label})" + f"{name}: {size} > {row['bytes'][name]} bytes + {limit:.0%} ({label}: {source(row)})" for label, row, limit in comparisons for name, size in current["bytes"].items() if name in row["bytes"] and size > row["bytes"][name] * (1 + limit) @@ -67,7 +80,7 @@ def main(): history = json.loads(args.history.read_text()) if args.history else [] failures = compare(current, history) (args.directory / "history.json").write_text(json.dumps(retain(current, history), indent=2) + "\n") - summary = "| Artifact | Bytes |\n|---|---:|\n" + summary = f"Measured {source(current)}.\n\n| Artifact | Bytes |\n|---|---:|\n" summary += "".join(f"| {name} | {size:,} |\n" for name, size in sorted(current["bytes"].items())) if failures: summary += "\n" + "\n".join(failures) + "\n" diff --git a/sh/size/report.test.py b/sh/size/report.test.py index 418fde7893f..625273197d6 100644 --- a/sh/size/report.test.py +++ b/sh/size/report.test.py @@ -1,5 +1,9 @@ #!/usr/bin/env python3 import importlib.util +import json +import subprocess +import sys +import tempfile import unittest from pathlib import Path @@ -25,6 +29,21 @@ def test_slow_drift(self): self.assertEqual(len(failures), 1) self.assertIn("30 days", failures[0]) + def test_large_step_fails_monthly_until_it_becomes_the_anchor(self): + history = [row("2026-09-10"), row("2026-10-09")] + for date, previous, monthly in [ + ("2026-10-10", True, True), + ("2026-10-11", False, True), + ("2026-11-08", False, True), + ("2026-11-09", False, False), + ]: + current = row(date, 112) + failures = report.compare(current, history) + with self.subTest(date=date): + self.assertEqual(any("previous:" in failure for failure in failures), previous) + self.assertEqual(any("30 days:" in failure for failure in failures), monthly) + history = report.retain(current, history) + def test_refuses_missing_and_invalid_artifacts(self): for invalid in [0, -1, None, True, float("nan")]: with self.assertRaises(ValueError): @@ -80,5 +99,50 @@ def test_validates_first_report_date(self): with self.assertRaises(ValueError): report.compare(row("invalid"), []) + def test_refuses_invalid_revision_metadata(self): + for revision in [None, 123, "main", "a" * 39, "a" * 40 + ")"]: + current = row("2026-10-10") | {"revision": revision} + with self.subTest(revision=revision), self.assertRaisesRegex(ValueError, "commit SHA"): + report.compare(current, []) + + def test_failed_growth_advances_daily_history_but_keeps_monthly_anchor(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + history = root / "previous.json" + history.write_text(json.dumps([row("2026-09-10"), row("2026-10-09")])) + + def run(current): + (root / "sizes.json").write_text(json.dumps(current)) + result = subprocess.run( + [sys.executable, str(Path(report.__file__)), str(root), "--history", str(history)], + capture_output=True, + text=True, + ) + retained = json.loads((root / "history.json").read_text()) + history.write_text(json.dumps(retained)) + return result, retained + + previous = row("2026-10-10", 106) | {"revision": "a" * 40} + failed, retained = run(previous) + self.assertEqual(failed.returncode, 1, failed.stderr) + self.assertEqual(retained[-1], previous) + + # A stable next report compares against the failed point, not the last success. + current = row("2026-10-11", 106) | {"revision": "b" * 40} + passed, _ = run(current) + self.assertEqual(passed.returncode, 0, passed.stderr) + self.assertIn(report.source(current), passed.stdout) + + # The thirty-day guard still catches drift despite the advanced daily baseline. + monthly, _ = run(row("2026-10-12", 111)) + self.assertEqual(monthly.returncode, 1, monthly.stderr) + self.assertIn("30 days: 2026-09-10", monthly.stdout) + self.assertNotIn("(previous:", monthly.stdout) + + def test_failure_names_the_measured_baseline_revision(self): + baseline = row("2026-10-09") | {"revision": "a" * 40} + failures = report.compare(row("2026-10-10", 106), [baseline]) + self.assertIn(report.source(baseline), failures[0]) + unittest.main()