Skip to content

Commit f3e1253

Browse files
ci(coverage): compare the parse-coverage gate against the merge base (#963)
The gate failed a pull request on the state of the report, not on the change the pull request made. Check 4 compared parse_partial_count against a number checked into scripts/ci/parse-partial-baseline.txt, and checks 1-3 asserted zero findings outright. Any of the four could go red for something main did. That is not theoretical. #1972 was this exact thing: main gained src/daemon/runtime.c, the count went 58 -> 59 on its own, and the number had to be raised by hand. #1824 will do it again and larger. Blazor .razor files map to C#, their markup lands in ERROR regions by design, and the count rises by roughly the repo's .razor count. A coverage improvement would read as a gate failure on an unrelated branch, and the person who hit it would have no way to tell that from a real regression. The gate now resolves the base commit, checks it out into a temporary worktree, and indexes both trees with the same binary. All four checks compare the two: 1. a whole-file parse failure fails only when it is new at head 2. a "+N" clipping marker fails only when it is new at head 3. a range over 25% of its file fails only when the file was within the share at the base 4. the flagged-file count fails only when it is above the base's parse-partial-baseline.txt stops being a gate. The script still prints the recorded number so a reader can see the drift, and says plainly that nothing fails on it. Nobody has to raise that number again. What this cannot see: both trees are indexed with the same binary, so a branch that changes the extractor itself moves the base side and the head side together and this gate will not fail on it. Catching that needs the base commit's own binary, which means a second full build -- about twelve minutes against the twenty-six seconds the whole gate step takes. Two things still cover it: the FLOOR asserted in tests/test_index_resilience.c stops the signal being switched off, and the absolute counts for both sides now print on every run, so a jump is visible in the log even when it does not fail. The script header and the scripts/ci/README.md row both say so. tests/test_coverage_gate_contract.sh pins the behaviour. It drives the production script with a fake binary that prints canned JSON, so no seam is added to the script itself and no indexing happens. Fifteen cases: each of the four findings present at both sides (pass) and new at head (fail), the count equal to, below and above the base, the recorded number not gating, both sides printing, a clipped file list still stopping the run outright, the allowlist skipping a path, and an unresolvable base commit stopping the run. Verified by reverting each of the four comparisons one at a time and confirming the matching "present at both sides" case goes red, then restoring. A real run against a built binary passes with both sides reported, and takes 45s for two indexes. pr.yml passes COVERAGE_GATE_BASE_SHA so the gate uses the commit GitHub itself used to build the merge, rather than falling back to the first parent of HEAD. Refs #963, #1972 Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
1 parent 8ae5eac commit f3e1253

6 files changed

Lines changed: 456 additions & 94 deletions

File tree

‎.github/workflows/pr.yml‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ jobs:
5959
# The paginated files endpoint remains filename-only for this gate.
6060
FILES=$(gh api --paginate "repos/$REPO/pulls/$PR/files?per_page=100" --jq '.[].filename')
6161
printf '%s\n' "$FILES"
62-
if printf '%s\n' "$FILES" | grep -qE '^(src/|internal/|install\.(sh|ps1)|scripts/build\.sh|scripts/smoke-test\.sh|scripts/smoke-local\.sh|scripts/smoke-fixture-server\.py|scripts/gen-third-party-notices\.sh|scripts/env\.sh|scripts/ci/(self-index-coverage-gate\.sh|coverage-gate-allowlist\.txt|parse-partial-baseline\.txt)|test-infrastructure/vm/(vm-smoke\.sh|windows-user-path-guard\.ps1)|Makefile\.cbm)'; then
62+
if printf '%s\n' "$FILES" | grep -qE '^(src/|internal/|install\.(sh|ps1)|scripts/build\.sh|scripts/smoke-test\.sh|scripts/smoke-local\.sh|scripts/smoke-fixture-server\.py|scripts/gen-third-party-notices\.sh|scripts/env\.sh|scripts/ci/(self-index-coverage-gate\.sh|coverage-gate-allowlist\.txt|parse-partial-baseline\.txt)|tests/test_coverage_gate_contract\.sh|test-infrastructure/vm/(vm-smoke\.sh|windows-user-path-guard\.ps1)|Makefile\.cbm)'; then
6363
echo "product=true" >> "$GITHUB_OUTPUT"
6464
else
6565
echo "product=false" >> "$GITHUB_OUTPUT"
@@ -134,6 +134,12 @@ jobs:
134134
# proportions, never exact line numbers.
135135
- name: Parse-coverage gate (Ubuntu)
136136
if: matrix.os == 'ubuntu-latest'
137+
env:
138+
# The gate compares this branch against the merge base, so it needs
139+
# the commit GitHub used to build the merge. Without this it falls
140+
# back to the first parent of HEAD, which is the same commit on a
141+
# pull_request checkout — this makes it explicit rather than lucky.
142+
COVERAGE_GATE_BASE_SHA: ${{ github.event.pull_request.base.sha }}
137143
run: scripts/ci/self-index-coverage-gate.sh "$(pwd)/build/c/codebase-memory-mcp"
138144

139145
- name: Build prod + smoke (macOS)

‎scripts/ci/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ CI and the local infrastructure — both of which the venue-parity contract
1313
| `preflight-docker.sh` | Same idea for Colima/docker: prune runner-unlike residue, assert free space on the filesystem backing the docker data root (not the VM's `/`). Build cache + named volumes KEPT (the local analogue of actions/cache); `--deep` drops them. | `test-infrastructure/run.sh` |
1414
| `check-glibc-compat.sh` | Run a linux binary in debian:bullseye (glibc 2.31) — the portable binary must start on old glibc. | `_smoke.yml` portable legs |
1515
| `generate-sbom.py` | The release SPDX SBOM (vendored versions reviewable here, diffable by vendoring PRs — was inline YAML). | `release.yml` |
16-
| `self-index-coverage-gate.sh` | Index this repo with the binary just built and fail the PR if its own parse-coverage report stops being useful advice: no whole-file failure, no range list clipped without saying so, no single range over 25% of a file of 200+ lines, and the flagged-file count at or below the checked-in ceiling. Reads its two data files, `coverage-gate-allowlist.txt` (paths it skips, each with a written reason) and `parse-partial-baseline.txt` (the ceiling). Ubuntu leg only — the flagged lines depend on which conditional-compilation branches the preprocessor keeps. | `pr.yml pr-smoke` |
16+
| `self-index-coverage-gate.sh` | Index the merge base and this branch with the binary just built, and fail the PR only for a finding the branch ADDED (#963): a whole-file parse failure, a range list clipped without saying so, a single range over 25% of a file of 200+ lines, or a flagged-file count above the merge base's. Comparing against the base rather than a fixed number stops main moving the baseline and turning unrelated PRs red (#1972). Reads two data files: `coverage-gate-allowlist.txt` (paths it skips, each with a written reason) and `parse-partial-baseline.txt` (a record it prints, never a gate). Both trees are indexed with the SAME binary, so it cannot see an extractor change that moves both sides together. Ubuntu leg only — the flagged lines depend on which conditional-compilation branches the preprocessor keeps. | `pr.yml pr-smoke` |
1717
| `require-all-green.sh` | The aggregate gate: fail unless every needed job succeeded or legitimately skipped (was inline YAML). | `pr.yml ci-ok` |
1818
| `verify-shard-union.sh` | Prove sharded test legs lost nothing: shard count agreement, indices 1..n, identical suite lists, union of slices == full list (was inline YAML). | `_test.yml` shard-completeness |
1919
| `prepare-release-candidates.sh` | Copy one linker output into stripped/unstripped candidates, finalize signatures, composition-check them without execution, and record their hashes. | `_build.yml`, local artifact smoke |

‎scripts/ci/parse-partial-baseline.txt‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,20 @@
1-
# Ceiling for parse_partial_count when this repo indexes itself.
1+
# A written record of parse_partial_count when this repo indexes itself.
22
#
3-
# The number below is what the gate allows. It complements the FLOOR asserted
4-
# in tests/test_index_resilience.c, which stops the signal being switched off
5-
# by accident. Raising this number is allowed but should be explained in the
6-
# commit that does it.
3+
# NOTHING FAILS ON THIS NUMBER. The gate reports it and moves on.
4+
#
5+
# It used to be a ceiling. main could move the count on its own and every open
6+
# pull request then went red for a reason none of them caused — that is #1972,
7+
# where main gained src/daemon/runtime.c and the count went 58 -> 59 with no
8+
# branch involved. The gate now indexes the merge base and compares against
9+
# that, so the number below does not gate anything.
10+
#
11+
# What it is still for: a reader can see how far the count has drifted since
12+
# anyone last looked, which is the one thing a merge-base comparison cannot
13+
# show. Update it when you have measured a new number and can say why it moved.
14+
#
15+
# It complements the FLOOR asserted in tests/test_index_resilience.c, which
16+
# stops the coverage signal being switched off by accident. That one IS
17+
# enforced.
718
#
819
# 58 -> 59 on 2026-08-31. main gained src/daemon/runtime.c, whose line 47 the
920
# tree-sitter C grammar cannot parse. The rise came from main, not from a

0 commit comments

Comments
 (0)