From 7da2a2b9ad536b115aac25627b4a7b7b11ec8a40 Mon Sep 17 00:00:00 2001 From: J3vb <192430104+J3vb@users.noreply.github.com> Date: Sun, 30 Aug 2026 13:48:02 +0200 Subject: [PATCH] =?UTF-8?q?feat(b3-6):=20coverage=20floor=20=E2=80=94=20sc?= =?UTF-8?q?ript,=20floors=20and=20CI=20gate=20on=20the=20ubuntu=20leg=20(S?= =?UTF-8?q?-06)=20(#1453)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(b3-6): coverage floor — script, floors and CI step (S-06) scripts/coverage-floor.sh reads a Go coverage profile and fails when the aggregate, or one of the core packages (ws, service, permissions, auth, db), is below the figure recorded in coverage-floor.json. Statement counts come from the profile's block lines, not `go tool cover -func`, so the per-package figures are exact; percentages are truncated to one decimal and compared in tenths. Exclusions (generated db/dbgen, cmd/) live in the floor file. Measured with the CI command on this branch's base (origin/dev 75d64dd4): aggregate 79.1 (11241/14194), auth 90.8, db 79.4, permissions 100.0, service 67.8, ws 84.5. The plan's starting aggregate was 74.6 (B0 baseline at an older SHA); the ratchet applies to this PR too, so the floor lands at the measured figure. Wired into ci.yml right after the race/coverage test step, on the Linux leg only: OS-tagged files swap in and out of the build and several tests skip on Windows, so one leg keeps the figure deterministic. The ratchet rule is in Server/CLAUDE.md. --floor and OWNCORD_COVERAGE_FLOOR override the committed floors, so the negative control needs no edit of the tracked JSON. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01KmiqjgTuov1stBTB6uGkvo * docs(b3-6): evidence block for item 1 (coverage floor) Numbers, RED/GREEN commands and the Linux-leg decision, appended to the B3-6 section per the item's exit criteria. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01KmiqjgTuov1stBTB6uGkvo * fix(b3-6): coverage floor — fail closed on JSON shape, honour --floor anywhere, LC_ALL=C, gate after the last test step Review findings on item 1. The floor-file parser now fails closed instead of silently reading less than the file says. A Prettier-wrapped "exclude" array used to yield zero exclusions and misreport as a coverage drop; an "exclude" key with no parsed entries is now exit 2 naming the one-line-array rule. A package entry sharing a line with the closing brace ("ws": 99.0 }) used to be dropped, so that floor stopped being enforced; the block now closes after the line is parsed, and an entry on the "packages": { line is parsed too. Two entries on one line are exit 2 rather than one silently ignored. All three rules are in the header comment. --floor is parsed in any argument position, so `coverage-floor.sh coverage.out --floor red.json` no longer silently uses the committed floors; an unknown flag or a second positional argument is a usage message and exit 2. LC_ALL=C is exported for mawk's locale-dependent decimal handling. The ci.yml step moves after the deadlock and tag-gated test steps — it still reads the profile the race step wrote, and still runs only on the ubuntu leg, but a floor miss no longer hides those steps' results. Server/CLAUDE.md gains the no-trailing-slash rule for exclusions. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01KmiqjgTuov1stBTB6uGkvo * fix(b3-6): coverage floor — every package entry must parse, and the five core packages must all be present (Codex P2 on #1453) A package entry whose value did not match the numeric regex — valid JSON such as "auth": "90.8" — was silently ignored, and the only completeness check was "at least one package parsed", so a malformed value quietly removed a floor. That contradicts the script's own fail-closed contract. Inside the "packages" block, any line that is not a well-formed "name": entry (or the block's closing brace) is now exit 2, naming the line number, the line, and the shape rule. The entry match is anchored, so it also subsumes the previous "two entries on one line" heuristic. The five core packages — ws, service, permissions, auth, db — must each have a parsed floor; a missing one is exit 2 naming it. Extra packages beyond the core set stay allowed. Header comment and the plan's evidence block record both rules. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01KmiqjgTuov1stBTB6uGkvo * fix(b3-6): coverage floor — floors set from the Linux leg (CI run 33302062524) The floors were first measured locally on Windows; the gate runs on the ubuntu leg, and the first CI run of #1453 showed the two legs differ. Each floor is now the Linux value truncated to one decimal, measured by the gate itself on the leg that enforces it: aggregate 79.9 (11344/14191), auth 90.8 (418/460), db 79.3 (1738/2189), permissions 100.0 (94/94), service 67.8 (1204/1775), ws 86.9 (3271/3763). The deltas are the ones the pre-merge analysis predicted. ws is higher on Linux because three EnsureLiveKitBinary tests and one harvest_s5 case skip on Windows, and that lifts the aggregate with it; db is one statement larger on Linux (lockfile_unix.go has 11 statements where lockfile_windows.go has 10), which costs it a tenth. auth, permissions and service carry no OS-conditional code and are identical on both legs. The evidence block now carries both columns, cites the run id, and keeps the 74.6 note. Running the new floors against the local Windows profile fails on aggregate and ws by exactly those deltas, which is expected and is why the gate is Linux-only. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01KmiqjgTuov1stBTB6uGkvo * fix(b3-6): coverage floor — a tenth of headroom where the Linux leg varied run-to-run (ws, aggregate; runs 33302062524 and 33302732286) Run 33302062524 measured ws at 3271/3763 (86.9); run 33302732286, same commit, measured 3267/3763 (86.8) and failed the floor set from the first. Four statements of run-to-run variance in ws under -race — timing-dependent branches — which moves the aggregate with it (11344 -> 11340). The reviewer's zero-headroom concern is now evidence rather than a prediction. Floors are therefore the lowest observed Linux figure, truncated to 0.1, minus 0.1 where the package varied between runs: ws 86.9 -> 86.7, aggregate 79.9 -> 79.8. auth 90.8, db 79.3, permissions 100.0 and service 67.8 are unchanged — their statement counts are identical in both runs, so they take no headroom. Server/CLAUDE.md states that rule next to the ratchet, and notes that a Windows run reports aggregate and ws under floor by design. The evidence block carries both runs' covered/total per package, which varied, and the resulting floors. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01KmiqjgTuov1stBTB6uGkvo --------- Co-authored-by: Claude Fable 5 --- .github/workflows/ci.yml | 10 ++ Server/CLAUDE.md | 23 +++ Server/coverage-floor.json | 12 ++ Server/scripts/coverage-floor.sh | 134 ++++++++++++++++++ ...rver-architecture-guardrails-2026-08-29.md | 63 ++++++++ 5 files changed, 242 insertions(+) create mode 100644 Server/coverage-floor.json create mode 100755 Server/scripts/coverage-floor.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 57df43d6..53ee0eea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -88,6 +88,16 @@ jobs: go test -tags wazero -count=1 ./plugin/... go test -tags otel -count=1 ./telemetry/... + # Coverage ratchet (B3-6 item 1). Reads the profile the race step wrote, + # but placed after the other test steps so a floor miss does not hide + # their results. Linux leg only: the profile is not the same on both legs + # — OS-tagged files swap in and out and several tests skip on Windows — + # so the floors are pinned to one leg and the figure stays deterministic. + # Ratchet rule in Server/CLAUDE.md. + - name: Check coverage floor + if: matrix.os == 'ubuntu-latest' + run: bash scripts/coverage-floor.sh coverage.out + - name: Upload Go coverage if: always() uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 diff --git a/Server/CLAUDE.md b/Server/CLAUDE.md index ded59782..cafe7dd7 100644 --- a/Server/CLAUDE.md +++ b/Server/CLAUDE.md @@ -51,3 +51,26 @@ prometheus. class, a reason, and the exact helper calls it is frozen at — a row is an inventory, not a licence for the function, so a second raw call inside a listed one still fails. That list only shrinks too. + +## Coverage floor + +`coverage-floor.json` holds the aggregate floor and one floor per core package +(`ws`, `service`, `permissions`, `auth`, `db`); `db/dbgen` and `cmd/` are +excluded there because they are generated or entry points, and an exclusion is +spelled without a trailing slash (`cmd`, not `cmd/`). CI checks it on the Linux +leg, after the test steps that share the job. Locally, from `Server/`: + +```bash +go test -race ./... -coverprofile=coverage.out -cover +bash scripts/coverage-floor.sh coverage.out +``` + +**Ratchet.** A floor is the **lowest Linux figure observed** for that package, +truncated to 0.1, **minus 0.1 where the package varied between runs** — `ws` +and the aggregate do vary, because a few `-race` branches in `ws` are +timing-dependent and move four or so statements per run. A PR that raises a +figure raises that floor in the same PR; the number in the file is what the +branch measured, not a stale one. Nobody lowers a floor without a hold-point +(HP) entry recording why. Coverage also differs between the Linux and Windows +legs, so the floors track the Linux figure and the check runs only there — on +Windows the script will report `aggregate` and `ws` under floor, by design. diff --git a/Server/coverage-floor.json b/Server/coverage-floor.json new file mode 100644 index 00000000..b56db817 --- /dev/null +++ b/Server/coverage-floor.json @@ -0,0 +1,12 @@ +{ + "_doc": "Coverage floors enforced by scripts/coverage-floor.sh, measured on the Linux CI leg. Ratchet rule and shape: Server/CLAUDE.md.", + "aggregate": 79.8, + "exclude": ["db/dbgen", "cmd"], + "packages": { + "auth": 90.8, + "db": 79.3, + "permissions": 100.0, + "service": 67.8, + "ws": 86.7 + } +} diff --git a/Server/scripts/coverage-floor.sh b/Server/scripts/coverage-floor.sh new file mode 100755 index 00000000..92ca473c --- /dev/null +++ b/Server/scripts/coverage-floor.sh @@ -0,0 +1,134 @@ +#!/usr/bin/env bash +# Coverage floor gate (B3-6 item 1). Fails when the aggregate, or any core +# package named in the floor file, is below its floor. Ratchet: Server/CLAUDE.md. +# +# From Server/: +# bash scripts/coverage-floor.sh coverage.out +# bash scripts/coverage-floor.sh --floor /tmp/red.json coverage.out +# OWNCORD_COVERAGE_FLOOR=/tmp/red.json bash scripts/coverage-floor.sh coverage.out +# +# The floor-file shape is fixed, because awk parses it - jq is not guaranteed on +# the runners - and the parser fails closed (exit 2) rather than enforcing less +# than the file says. Four rules: every line inside "packages" is one +# "name": entry, the number unquoted; all five core packages +# (ws, service, permissions, auth, db) are present; the "exclude" array is all +# on ONE line, because a Prettier-wrapped array would parse as no exclusions at +# all; package names are module-relative directories with no trailing slash +# ("cmd", not "cmd/"). +# {"aggregate": , "exclude": ["db/dbgen", "cmd"], +# "packages": {"": , ...}} +# "exclude" prefixes are dropped before anything is counted. A percentage is +# covered/total statements truncated to one decimal, so what this prints is +# exactly what the floor file records. LC_ALL=C and [ \t] rather than +# [[:space:]]: the ubuntu runner's awk may be mawk. +set -euo pipefail +export LC_ALL=C + +usage() { + echo "usage: coverage-floor.sh [--floor ] []" >&2 + exit 2 +} + +floor="${OWNCORD_COVERAGE_FLOOR:-coverage-floor.json}" +profile="" +while [ $# -gt 0 ]; do + case "$1" in + --floor) + floor="${2:?--floor needs a path}" + shift 2 + ;; + -*) usage ;; + *) + [ -z "$profile" ] || usage + profile="$1" + shift + ;; + esac +done +profile="${profile:-coverage.out}" + +for f in "$floor" "$profile"; do + [ -f "$f" ] || { + echo "coverage-floor: no such file: $f" >&2 + exit 2 + } +done + +awk ' +FNR == NR { # pass 1: the floor file + if ($0 ~ /"packages"[ \t]*:/) { + inpkg = 1 + sub(/^.*"packages"[ \t]*:[ \t]*[{]/, "") # keep any entry on this line + } + if ($0 ~ /"exclude"[ \t]*:/) { + sawexcl = 1 + n = split($0, a, "\"") + for (i = 4; i <= n; i += 2) if (a[i] != "") excl[++ne] = a[i] + next + } + entry = $0 # strip the block brace, comma + if (inpkg) sub(/[ \t]*[}].*$/, "", entry) + gsub(/^[ \t]+|[ \t]*,[ \t]*$|[ \t]+$/, "", entry) + ok = (entry ~ /^"[^"]+"[ \t]*:[ \t]*[0-9]+([.][0-9]+)?$/) + if (inpkg && entry != "" && !ok) + err = sprintf("floor file line %d is not a \"name\": entry: %s", FNR, $0) + if (ok) { + key = val = entry + sub(/^"/, "", key); sub(/".*$/, "", key) + sub(/^[^:]*:[ \t]*/, "", val) + if (key == "aggregate") { aggfloor = val + 0; haveagg = 1 } + else if (inpkg) { pkgfloor[key] = val + 0; order[++np] = key } + } + if (inpkg && $0 ~ /}/) inpkg = 0 # close AFTER parsing the line + next +} +/^mode:/ { next } +{ # pass 2: the coverage profile + rel = $1 + sub(/:[0-9]+\.[0-9]+,[0-9]+\.[0-9]+$/, "", rel) # drop the block range + sub(/^.*\/Server\//, "", rel) # module-relative path + pkg = (rel ~ /\//) ? rel : "." + sub(/\/[^\/]*$/, "", pkg) + for (i = 1; i <= ne; i++) + if (index(pkg "/", excl[i] "/") == 1) next + tot += $2; ptot[pkg] += $2 + if ($3 > 0) { cov += $2; pcov[pkg] += $2 } +} +function check(name, c, t, fl, tenths, under) { # figures compared in tenths + tenths = int(c * 1000 / t) + under = (tenths < int(fl * 10 + 0.5)) + printf "coverage-floor: %s %s %.1f%% (floor %.1f%%, %d/%d statements)\n", (under ? "FAIL" : "ok"), name, tenths / 10, fl, c, t + return under +} +END { + if (err != "") { + print "coverage-floor: " err + exit 2 + } + if (sawexcl && ne == 0) { + print "coverage-floor: \"exclude\" must be one single-line array, e.g. \"exclude\": [\"db/dbgen\", \"cmd\"]; a wrapped array parses as no exclusions" + exit 2 + } + if (!haveagg || tot == 0) { + print "coverage-floor: floor file or coverage profile is empty/malformed" + exit 2 + } + nc = split("ws service permissions auth db", core, " ") + for (i = 1; i <= nc; i++) + if (!(core[i] in pkgfloor)) { + print "coverage-floor: floor file has no floor for core package " core[i] + exit 2 + } + bad = check("aggregate", cov, tot, aggfloor) + for (i = 1; i <= np; i++) { + p = order[i] + if (!(p in ptot)) { + printf "coverage-floor: FAIL %s: no statements in the coverage profile\n", p + bad = 1 + continue + } + if (check(p, pcov[p], ptot[p], pkgfloor[p])) bad = 1 + } + exit bad +} +' "$floor" "$profile" diff --git a/docs/plans/b3-server-architecture-guardrails-2026-08-29.md b/docs/plans/b3-server-architecture-guardrails-2026-08-29.md index d65e8d60..b2b90cba 100644 --- a/docs/plans/b3-server-architecture-guardrails-2026-08-29.md +++ b/docs/plans/b3-server-architecture-guardrails-2026-08-29.md @@ -701,6 +701,69 @@ baseline) in this section's evidence block. 200→213), which is why rows are keyed by `.` and never by `file:line`; the hit count is unchanged at 21. No production code changed. +#### Evidence — item 1 (coverage floor) + +- Branch `feat/b3-6-coverage-floor`; commits: `843dd6c6` feat(b3-6): coverage + floor — script, floors and CI step (S-06), the commit carrying this block, and + the review-fix commit that follows them on this branch. +- RED: `bash scripts/coverage-floor.sh --floor <99-aggregate.json> coverage.out` + → `coverage-floor: FAIL aggregate 79.1% (floor 99.0%, 11241/14194 statements)` + (exit 1); `bash scripts/coverage-floor.sh --floor <99-ws.json> coverage.out` + → `coverage-floor: FAIL ws 84.5% (floor 99.0%, 3181/3763 statements)` + (exit 1). The parser also fails closed on a floor file it cannot read whole: a + Prettier-wrapped `"exclude"` array exits 2 rather than silently excluding + nothing; a package entry sharing the closing-brace line is enforced, not + dropped (`"ws": 99.0 }` → `FAIL ws 84.5% (floor 99.0%)`, exit 1); a value that + is not an unquoted number exits 2 naming the line (`"auth": "90.8"` → + `floor file line 6 is not a "name": entry`); and a floor file missing + any of the five core packages exits 2 naming it (`no floor for core package +db`). No control file is committed. +- GREEN: the `Check coverage floor` step of CI run `33302062524` (ubuntu leg) → + exit 0, six lines, `coverage-floor: ok aggregate 79.9% (floor 79.9%, 11344/14191 statements)` + and one `ok` line per core package. The RED runs above are local, against the + Windows profile, which is why their measured column reads 79.1/84.5 rather + than the committed Linux floors. +- Numbers. The floors are **Linux** figures, measured by the gate itself on the + ubuntu leg of PR #1453 — the leg that enforces them. Two runs were needed: + the second (`33302732286`) failed `ws` at 86.8 against a floor of 86.9 set + from the first (`33302062524`), so the leg is not bit-for-bit repeatable. The + rule the numbers now follow: **the lowest observed Linux figure, truncated to + 0.1, minus 0.1 where the package varied between runs.** + + | figure | run 33302062524 | run 33302732286 | varies | floor | + | ------------- | --------------- | --------------- | ------ | --------- | + | aggregate | 11344/14191 | 11340/14191 | yes | **79.8** | + | `auth` | 418/460 | 418/460 | no | **90.8** | + | `db` | 1738/2189 | 1738/2189 | no | **79.3** | + | `permissions` | 94/94 | 94/94 | no | **100.0** | + | `service` | 1204/1775 | 1204/1775 | no | **67.8** | + | `ws` | 3271/3763 | 3267/3763 | yes | **86.7** | + + Only `ws` moves — four statements, timing-dependent branches under `-race` — + and it carries the aggregate with it; the other four packages are identical + across both runs, so they take no headroom. + + The Windows/Linux gap is separate and was measured locally with + `go test -race -timeout 20m ./... -coverprofile=coverage.out -cover` at the + merge base with `origin/dev` (`75d64dd4`): aggregate 79.1 (11241/14194), + `auth` 90.8, `db` 79.4 (1738/2188), `permissions` 100.0, `service` 67.8, `ws` + 84.5 (3181/3763). It falls exactly where the pre-merge analysis said it would: + `ws` is higher on Linux because three `EnsureLiveKitBinary` tests and one + `harvest_s5` case skip on Windows, and `db` is one statement larger on Linux + (`lockfile_unix.go` has 11 statements where `lockfile_windows.go` has 10), + which costs it a tenth. `auth`, `permissions` and `service` carry no + OS-conditional code and are identical everywhere. Running the committed + floors against a Windows profile therefore reports `aggregate` and `ws` under + floor — expected, and the reason the gate is Linux-only. + + The spec's starting aggregate of 74.6 is the B0 baseline at an older SHA; the + ratchet rule applies to this PR, so the committed floor is 79.8. + +- Verified against HEAD: `ci.yml:73` matched the spec. The gate step runs on the + **ubuntu-latest** leg only, and after the job's other test steps so a floor + miss does not hide them. Two legs would not be deterministic: the table above + is the measured proof that the profile is not the same on both. + ## B3-7 — Alpha-shaped test dataset Roadmap workstream 12. Beside the slice.