From eafb2e39e26d6960500bc0f8ccd26b6391a2dd43 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sun, 19 Jul 2026 18:33:31 +0200 Subject: [PATCH 1/2] =?UTF-8?q?perf(469):=20format=20gate=20uses=20`dotnet?= =?UTF-8?q?=20format=20whitespace=20--folder`=20(~480s=20=E2=86=92=20~0.5s?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The blocking `format` CI job and the Husky pre-commit hook verified changed .cs files with `dotnet format ErsatzTV.sln --no-restore --verify-no-changes --include `. `--include` only narrows *which* files are checked, never what gets loaded: the full recipe loaded the ~10-project MSBuild workspace and built a Roslyn compilation per project before checking a single line (~480s locally, whole-solution). Switch both to `dotnet format whitespace . --folder --verify-no-changes --include `, which treats the tree as a plain folder of files, skips MSBuild/Roslyn entirely (~0.5s), and needs no `dotnet restore` (NuGet-cache + Restore steps removed). Coverage is unchanged: folder mode reads .editorconfig and enforces exactly the gate's purpose — whitespace + charset (BOM). Proven non-vacuous (error WHITESPACE on a trailing-space line, error CHARSET on a prepended BOM, exit 0 clean). The full gate never enforced the style/analyzer pass either — a warning-severity naming violation passes the full solution format (exit 0) — and the analyzers that must block (NU1904, S3981) are enforced at compile via WarningsAsErrors, not by this job. Docs: ci-cd.md Formatting section + the obsolete #406 memory note; decisions.md. fixes #469 Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitea/workflows/docker-build.yml | 44 +++++++++++++++---------------- .husky/pre-commit | 14 +++++----- docs/ci-cd.md | 37 +++++++++++++++++++++----- docs/decisions.md | 30 +++++++++++++++++++++ 4 files changed, 89 insertions(+), 36 deletions(-) diff --git a/.gitea/workflows/docker-build.yml b/.gitea/workflows/docker-build.yml index f98fe7a70..72cd66acf 100644 --- a/.gitea/workflows/docker-build.yml +++ b/.gitea/workflows/docker-build.yml @@ -885,16 +885,26 @@ jobs: echo "Generated API artifacts are in sync." # Formatting-as-you-touch gate (ersatztv#311): verify the .cs files THIS PR changed conform to - # .editorconfig (style + charset=utf-8, i.e. no UTF-8 BOM). Scoped to changed files so it enforces - # "normalize a legacy file when you touch it" WITHOUT a big-bang reformat of the ~2500 pre-existing - # BOM files. A PR that touches no .cs skips the expensive steps and passes trivially (always reports - # a status, so it is safe as a required check). + # .editorconfig whitespace + charset=utf-8 (i.e. no UTF-8 BOM). Scoped to changed files so it + # enforces "normalize a legacy file when you touch it" WITHOUT a big-bang reformat of the ~2500 + # pre-existing BOM files. A PR that touches no .cs skips the check and passes trivially (always + # reports a status, so it is safe as a required check). + # + # ersatztv#469: uses `dotnet format whitespace . --folder`, NOT the full `dotnet format `. + # `--folder` treats the tree as a plain folder of files and skips the MSBuild/Roslyn workspace load + # + per-project compilation that dominated the old recipe (~8 min locally on a whole-solution run) — + # `--include` only ever narrowed *which* files were checked, never what got loaded. Folder mode + # reads .editorconfig and still flags WHITESPACE (indent/EOL/trailing/final-newline) and CHARSET + # (BOM) violations — exactly what this gate exists to catch — in ~0.5s with no `dotnet restore`. + # What it drops is the style/analyzer pass (naming/`var`/qualification), which this gate never + # meaningfully enforced: those .editorconfig rules are :suggestion/:none severity. Full rationale + + # non-vacuity evidence: docs/ci-cd.md → Formatting; docs/decisions.md. format: name: Formatting (changed .cs conform to .editorconfig) - # Was on the `small` lane (ersatztv#390) to dodge a ~29 min queue; reverted to `ubuntu-latest` - # in ersatztv#406 — `dotnet format` needs the .NET SDK and real memory, so it does not belong - # in a lane sized for seconds-long shell jobs. See the api-docs job above for the full - # rationale; server-management#604 grew this lane so the queue it was dodging is gone. + # Folder-mode whitespace is now a seconds-long, low-memory job (no Roslyn workspace, unlike the + # 3.95 GiB full `dotnet format` measured in #406), so it no longer needs the memory headroom that + # kept it on `ubuntu-latest`. Left here to avoid re-touching the lane/memory-cap accounting; a + # move to a lighter lane is a server-management capacity call (#604). runs-on: ubuntu-latest container: image: 192.168.1.95:3000/timothy/ersatztv-ci:07048b8 @@ -924,26 +934,14 @@ jobs: echo "No .cs change -> skipping format verify (job passes)." fi - - name: Cache NuGet packages - if: steps.detect.outputs.cs_changed == 'true' - uses: actions/cache@v4 - with: - path: ~/.nuget/packages - key: nuget-${{ runner.os }}-${{ hashFiles('Directory.Packages.props', 'global.json') }} - restore-keys: nuget-${{ runner.os }}- - - - name: Restore - if: steps.detect.outputs.cs_changed == 'true' - run: dotnet restore - - name: Verify formatting of changed .cs files if: steps.detect.outputs.cs_changed == 'true' shell: bash run: | mapfile -t files < /tmp/changed-cs.txt - echo "Verifying ${#files[@]} changed .cs file(s) against .editorconfig..." - if ! dotnet format ErsatzTV.sln --no-restore --verify-no-changes --include "${files[@]}"; then - echo "::error::One or more .cs files this PR touches don't conform to .editorconfig (formatting or a UTF-8 BOM). Run 'dotnet format ErsatzTV.sln --include ' and commit the result in THIS PR — the fix-as-you-touch convention (docs/contributing.md §7; ersatztv#311). Legacy files you did NOT touch are unaffected." + echo "Verifying ${#files[@]} changed .cs file(s) against .editorconfig (whitespace + charset)..." + if ! dotnet format whitespace . --folder --verify-no-changes --include "${files[@]}"; then + echo "::error::One or more .cs files this PR touches don't conform to .editorconfig (whitespace or a UTF-8 BOM). Run 'dotnet format whitespace . --folder --include ' (or the full 'dotnet format ErsatzTV.sln --include ') and commit the result in THIS PR — the fix-as-you-touch convention (docs/contributing.md §7; ersatztv#311). Legacy files you did NOT touch are unaffected." exit 1 fi echo "All changed .cs files conform to .editorconfig." diff --git a/.husky/pre-commit b/.husky/pre-commit index c5d5c66aa..8abf993b8 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -11,15 +11,17 @@ if [ -n "$root_png" ]; then exit 1 fi -# dotnet format on staged .cs files (repo root). Scoped to the staged files so we -# don't pay the full-tree cost; skip entirely when no .cs is staged (avoids the -# ~20-40s sln load for web-only commits). +# dotnet format on staged .cs files (repo root). Uses `whitespace . --folder` — same recipe as +# the CI `format` job (ersatztv#469): folder mode checks .editorconfig whitespace + charset (BOM) +# without the MSBuild/Roslyn workspace load, so it runs in ~0.5s instead of the old ~20-40s sln +# load. Keeping this identical to CI avoids a local hook that blocks on rules CI no longer enforces. +# Skip entirely when no .cs is staged (avoids any cost for web-only commits). cs_files=$(git diff --cached --name-only --diff-filter=ACM -- '*.cs') if [ -n "$cs_files" ]; then - echo "husky - dotnet format (verify) on staged .cs files" + echo "husky - dotnet format (whitespace verify) on staged .cs files" # shellcheck disable=SC2086 - dotnet format ErsatzTV.sln --verify-no-changes --include $cs_files || { - echo "husky - dotnet format found issues in staged .cs files; run 'dotnet format ErsatzTV.sln --include ' to fix" + dotnet format whitespace . --folder --verify-no-changes --include $cs_files || { + echo "husky - dotnet format found whitespace/BOM issues in staged .cs files; run 'dotnet format whitespace . --folder --include ' to fix" exit 1 } fi diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 2668621bb..85cf039b5 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -145,9 +145,12 @@ invocation without touching each call site (MSBuild surfaces env vars as propert `UseSharedCompilation` is only defaulted to `true` when empty, so the env var wins). **What this does and does not shrink.** It helps the jobs that *compile* — `test`, `migrations`, -`api-docs` on an API-touching PR, and the in-Docker `build`. It does **not** help `format`: `dotnet -format` loads Roslyn in-process via MSBuildWorkspace and never spawns `csc`, so its measured 3.95 -GiB is unaffected. Don't size the `small` lane expecting `format` to have shrunk — it hasn't. +`api-docs` on an API-touching PR, and the in-Docker `build`. It never helped `format`: `dotnet +format` loaded Roslyn in-process via MSBuildWorkspace and never spawned `csc`, so the compiler-server +env vars left its measured 3.95 GiB untouched. (Moot since **ersatztv#469** switched `format` to +`dotnet format whitespace . --folder`, which skips the MSBuild/Roslyn workspace entirely — the job is +now a ~0.5 s, low-memory whitespace/BOM check with no Roslyn heap. See *Static analysis & +formatting → Formatting* below.) The same three are repeated in `dependency-scan.yml`; workflow `env:` does not cross workflow files. That one is lower-stakes (restore/list are MSBuild-driven, so it's lingering worker nodes rather @@ -674,10 +677,30 @@ Promoted rules are recorded here so the blocking subset stays intentional and re as a defensive default if server-rendered view code is ever reintroduced. **Formatting** — the inherited tree still contains legacy UTF-8 BOM/whitespace debt, so the standing -policy is **format as you touch**, not a mass rewrite (ersatztv#311). The Husky pre-commit hook runs -`dotnet format --verify-no-changes` for staged C# files, and the blocking `format` CI job repeats that -check for C# files changed by the PR. Untouched legacy files remain outside the gate; `.gitattributes` -pins line endings. A one-time full-tree normalization remains a separate, unmade decision. +policy is **format as you touch**, not a mass rewrite (ersatztv#311). The Husky pre-commit hook and +the blocking `format` CI job both run `dotnet format whitespace . --folder --verify-no-changes +--include ` — scoped to the files the commit/PR touches. Untouched legacy files remain +outside the gate; `.gitattributes` pins line endings. A one-time full-tree normalization remains a +separate, unmade decision. + +*Why `whitespace . --folder`, not the full `dotnet format ` (ersatztv#469)* — the gate only +needs to enforce `.editorconfig` **whitespace** (indent/EOL/trailing/final-newline) and **charset** +(no UTF-8 BOM). The old recipe (`dotnet format ErsatzTV.sln --no-restore --verify-no-changes +--include`) loaded the entire ~10-project MSBuild workspace and built a Roslyn compilation per +project *before* checking a single file — `--include` narrows *which* files are checked, never what +gets loaded. Measured whole-solution `dotnet format` ran **~480s locally**; folder mode runs in +**~0.5s** and needs no `dotnet restore` (the NuGet-cache + Restore steps were removed from the job). +`--folder` treats the tree as a plain folder of files, skipping MSBuild/Roslyn entirely, and still +reads `.editorconfig`. Verified **non-vacuous**: it exits non-zero on an injected trailing-whitespace +line (`error WHITESPACE`) and on a prepended UTF-8 BOM (`error CHARSET`), and exits 0 on a clean file. +**No coverage was lost**: the full `dotnet format` gate did **not** enforce the style/analyzer pass +either — a probe injecting a `warning`-severity naming violation (`local_constants` not `ALL_UPPER`) +passed the *full* solution format (exit 0), because the only `.editorconfig` rule above +`:suggestion`/`:none` severity is that one naming rule and `dotnet format`'s default severity ignores +it. The analyzers that **must** block (`NU1904`, `S3981`) are enforced at *compile time* via +`WarningsAsErrors` in `Directory.Build.props`, not by this job. Devs fix a violation with `dotnet +format whitespace . --folder --include ` (the full `dotnet format ErsatzTV.sln --include +` is a superset and also works). ## Migration integrity (EF Core, both providers) diff --git a/docs/decisions.md b/docs/decisions.md index 4c3b10e03..46cfb9089 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -2014,3 +2014,33 @@ performance half. per client and the TTL collapses steady-state load, so at most a handful of exactly-simultaneous cold callers re-run — a once-per-30s edge, not the repeated per-request cost the issue targets. Recorded here so a later reviewer doesn't read the absence of a `SemaphoreSlim` as an oversight. + +## 2026-07-19 — The `format` gate runs `dotnet format whitespace . --folder`, not the full solution format (#469) + +The blocking `format` CI job (and the matching Husky pre-commit hook) verify changed `.cs` files with +`dotnet format whitespace . --folder --verify-no-changes --include ` instead of the previous +`dotnet format ErsatzTV.sln --no-restore --verify-no-changes --include `. + +- **Why.** `--include` narrows *which* files are checked, never what gets loaded. The full recipe loaded + the whole ~10-project MSBuild workspace and built a Roslyn compilation per project before checking a + single line — a fixed cost independent of how few files changed. Measured **~480s** for a whole-solution + `dotnet format` locally (matching the issue's "7+ min"). `--folder` treats the tree as a plain folder of + files and skips MSBuild/Roslyn entirely: **~0.5s**, and it needs no `dotnet restore`, so the job's + NuGet-cache + Restore steps were deleted. It also drops the job's ~3.95 GiB Roslyn heap (the #406 + memory note about `format` not shrinking is now moot). +- **Coverage is unchanged, not merely "good enough".** Folder mode reads `.editorconfig` and enforces + exactly the two things this gate exists for — **whitespace** (indent/EOL/trailing/final-newline) and + **charset** (no UTF-8 BOM). Proven non-vacuous: exits non-zero with `error WHITESPACE` on an injected + trailing-whitespace line and `error CHARSET` on a prepended BOM; exits 0 on a clean file. What it drops + is the style/analyzer pass — but the *full* gate never enforced that either: a probe injecting a + `warning`-severity naming violation (`local_constants` not `ALL_UPPER`) **passed** the full solution + format (exit 0), because the only `.editorconfig` rule above `:suggestion`/`:none` severity is that one + naming rule and `dotnet format`'s default severity ignores it. The analyzers that must block (`NU1904`, + `S3981`) are enforced at compile time via `WarningsAsErrors` in `Directory.Build.props`, never by this + job. +- **Fix command for a violation:** `dotnet format whitespace . --folder --include `. The full + `dotnet format ErsatzTV.sln --include ` is a superset (also applies style) and still works, so + existing muscle memory and the #311 lore's `dotnet format --include` guidance are not broken. +- **Lane left on `ubuntu-latest`.** The job is now seconds-long and low-memory, so it could move to a + lighter lane, but that re-touches the per-lane memory-cap accounting (#406/#604) and is a + server-management capacity call — deliberately out of scope here. -- 2.47.3 From 33657b4753ab29a18793d4e0ec206c2ac5b94f40 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sun, 19 Jul 2026 19:34:43 +0200 Subject: [PATCH 2/2] docs(469): sweep remaining stale `format`-job cost/memory claims (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cold adversarial review (MERGEABLE) flagged two spots still describing the pre-#469 heavy `format` job as current, plus a wording imprecision: - ci-cd.md CI-lane table row: annotate `format` runtime 37s → ~0.5s (#469). - ci-cd.md #406 memory narrative: note the 3.95 GiB Roslyn heap is now moot (folder mode loads no workspace); api-docs remains the lane's real consumer. - Precise the coverage mechanism in ci-cd.md + decisions.md: the naming rule passes the full gate because naming violations have no `dotnet format` batch code-fixer (so `--verify-no-changes` sees no change), not merely a severity floor. Docs-only; no workflow/hook logic change from the reviewed commit. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/ci-cd.md | 14 +++++++++----- docs/decisions.md | 5 +++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 85cf039b5..6064e9af5 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -103,7 +103,7 @@ saturated and the `small` lane idle — **queue wait exceeded every job's runtim | `migrations` | 639s | 1428s | ubuntu-latest | | `functional-e2e` | 520s | 1447s | ubuntu-latest | | `api-docs` | 5s | **1722s** | ubuntu-latest → `small` → **reverted to ubuntu-latest** (#406) | -| `format` | 37s | **1731s** | ubuntu-latest → `small` → **reverted to ubuntu-latest** (#406) | +| `format` | 37s → **~0.5s** (#469) | **1731s** | ubuntu-latest → `small` → **reverted to ubuntu-latest** (#406) | | `docs-reminder` / `decisions-guard` | 10s | **5s** | small | `api-docs` and `format` moved to `small` because the queue wait dwarfed their runtime. Both lanes @@ -116,10 +116,13 @@ API-touching PR `api-docs` does a full `dotnet build`, so it is not always small be the deciding factor, and "capacity 4 absorbs that" held only because **nothing enforces the sum** of the lanes' per-job caps. Each job container is correctly capped (`--memory=10g`), but 6 slots × 10 GiB = **60 GiB on a 25 GiB host** that also runs prod media; on 2026-07-17 bumblebee hit load -340 with 21 GiB swapped. These are not small jobs — a live `docker stats` caught the `format` job +340 with 21 GiB swapped. These were not small jobs — a live `docker stats` caught the `format` job container at **3.95 GiB**, which the re-sized 2 GiB `small` lane would OOM-kill outright. #604 fixes the queue at the source instead (`ubuntu-latest` grown to 5 slots: a 48 GiB ci-runner at capacity 4 plus a bumblebee overflow slot), so the `small` lane can be reserved for genuinely-tiny shell jobs. +(The `format` half of this is now **moot**: **ersatztv#469** moved it to `dotnet format whitespace . +--folder`, which loads no Roslyn workspace — the job's 3.95 GiB heap and multi-minute runtime are +gone, so it is no longer a reason to keep the lane large. `api-docs` on an API-touching PR still is.) Queue wait is still a dominant cost and capacity is server-management's boundary — tracked in **server-management#604**. The redundant triple-build behind those runtimes is **ersatztv#398**. @@ -695,9 +698,10 @@ reads `.editorconfig`. Verified **non-vacuous**: it exits non-zero on an injecte line (`error WHITESPACE`) and on a prepended UTF-8 BOM (`error CHARSET`), and exits 0 on a clean file. **No coverage was lost**: the full `dotnet format` gate did **not** enforce the style/analyzer pass either — a probe injecting a `warning`-severity naming violation (`local_constants` not `ALL_UPPER`) -passed the *full* solution format (exit 0), because the only `.editorconfig` rule above -`:suggestion`/`:none` severity is that one naming rule and `dotnet format`'s default severity ignores -it. The analyzers that **must** block (`NU1904`, `S3981`) are enforced at *compile time* via +passed the *full* solution format (exit 0): the only `.editorconfig` rule above `:suggestion`/`:none` +severity is that one naming rule, and naming violations have no `dotnet format` batch code-fixer, so +`--verify-no-changes` reports no change regardless of severity. The analyzers that **must** block +(`NU1904`, `S3981`) are enforced at *compile time* via `WarningsAsErrors` in `Directory.Build.props`, not by this job. Devs fix a violation with `dotnet format whitespace . --folder --include ` (the full `dotnet format ErsatzTV.sln --include ` is a superset and also works). diff --git a/docs/decisions.md b/docs/decisions.md index 46cfb9089..0347c8a40 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -2034,8 +2034,9 @@ The blocking `format` CI job (and the matching Husky pre-commit hook) verify cha trailing-whitespace line and `error CHARSET` on a prepended BOM; exits 0 on a clean file. What it drops is the style/analyzer pass — but the *full* gate never enforced that either: a probe injecting a `warning`-severity naming violation (`local_constants` not `ALL_UPPER`) **passed** the full solution - format (exit 0), because the only `.editorconfig` rule above `:suggestion`/`:none` severity is that one - naming rule and `dotnet format`'s default severity ignores it. The analyzers that must block (`NU1904`, + format (exit 0): the only `.editorconfig` rule above `:suggestion`/`:none` severity is that one naming + rule, and naming violations have no `dotnet format` batch code-fixer, so `--verify-no-changes` reports + no change regardless of severity. The analyzers that must block (`NU1904`, `S3981`) are enforced at compile time via `WarningsAsErrors` in `Directory.Build.props`, never by this job. - **Fix command for a violation:** `dotnet format whitespace . --folder --include `. The full -- 2.47.3