Skip to content

Commit 343c41e

Browse files
ci(coverage): fix three ways the coverage gate could pass without checking (#963)
Verifying the gate against a locally built binary turned up three defects in the gate itself, all of the same shape it exists to catch: it could report PASS without having looked at everything. Reading the ceiling aborted the whole gate on a long file. The ceiling came from `sed | grep -oE | head -1`. Under `set -o pipefail` head closes the pipe, grep dies of SIGPIPE, and the gate exits with no message at all. It does not bite on today's 7-line baseline file and it does bite on a long one, which is proved. Replaced with one awk that stops after the first number. The report's own truncation flag was never read. index_status lists at most 500 files per class (COVERAGE_FILE_CAP) and sets "truncated" when it dropped the rest. Checks 2 and 3 walk that list file by file, so a clipped list means they judge part of the repo and still print PASS. New check 0 stops instead. Today the list holds all 58 files and truncated is false, so this is a guard, not a fix for live behaviour. Check 1 named files it had already accepted. It subtracted allowlisted paths from the count but still printed them in the failure text, so the message disagreed with the number beside it. It now counts and names the same set. The gate also did not answer --help, which scripts/ci/README.md says every script there does. It fed --help to basename, printed a usage error from the wrong program and exited 0. It now prints a Usage: block and exits 0, rejects an unknown flag with exit 2 and the house line "Please consult --help.", and is enrolled in HELP_ENTRIES and STRICT_ENTRIES in the venue parity contract so the rule is enforced rather than only written down. Breaking --help fails that contract with exit 1, which is checked. Added the missing row to the scripts/ci/README.md table. The allowlist reason for scripts/setup-windows.ps1 carried the old numbers. After the range fix in the previous commit the file reports 113-113,245-326, so the widest range is 82 lines rather than 83 — 25.2% of 326. Still over the 25% limit, so the entry stays, and the reason now says why narrowing it by one line did not clear the gate. Verified against a locally built binary: healthy PASS, parse_partial=58 (ceiling 58) parse_unusable=0, exit 0 allowlist emptied FAIL naming setup-windows.ps1 at 25.2%, exit 1 ceiling at 57 FAIL naming the count, exit 1 both restored PASS, exit 0 Venue parity contract: 19 --help entries, 9 strict-flag entries, green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
1 parent af5581b commit 343c41e

4 files changed

Lines changed: 79 additions & 7 deletions

File tree

‎scripts/ci/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +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` |
1617
| `require-all-green.sh` | The aggregate gate: fail unless every needed job succeeded or legitimately skipped (was inline YAML). | `pr.yml ci-ok` |
1718
| `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 |
1819
| `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/coverage-gate-allowlist.txt‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,13 @@
77
#
88
# Blank lines and lines starting with # are ignored.
99

10-
# One range covers lines 245-327 of a 326-line file (25.5%). The tree-sitter
10+
# One range covers lines 245-326 of a 326-line file (25.2%). The tree-sitter
1111
# PowerShell grammar cannot parse the `} else {` branch that runs to the end
12-
# of the file, so those 83 lines really are absent from the graph. This is a
12+
# of the file, so those 82 lines really are absent from the graph. This is a
1313
# genuine grammar gap, not a reporting error. Every other file of 200+ lines
1414
# in this repo sits at 3.9% or below.
15+
#
16+
# The range read 113-113,113-113,245-327 until the duplicate and the past-EOF
17+
# end line were fixed in cbm_error_regions_push. Narrowing it by one line did
18+
# NOT clear the gate: 25.2% is still over the 25% limit, so this entry stays.
1519
scripts/setup-windows.ps1

‎scripts/ci/self-index-coverage-gate.sh‎

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,52 @@
1717
# on ONE CI leg and asserts proportions rather than exact line numbers.
1818
set -euo pipefail
1919

20-
BIN="${1:?usage: self-index-coverage-gate.sh <path-to-binary>}"
20+
usage() {
21+
cat <<'EOF'
22+
Usage: scripts/ci/self-index-coverage-gate.sh <path-to-codebase-memory-mcp-binary>
23+
24+
Index THIS repository with the given binary and fail if the repository's own
25+
parse-coverage report stops being useful advice (#963).
26+
27+
Four checks, all read from index_status:
28+
1. No file reports a whole-file parse failure (parse_unusable).
29+
2. No range list was clipped without saying so (a trailing "+<N>" marker).
30+
3. No single range covers more than 25% of a file of 200 lines or more.
31+
4. The flagged-file count stays at or below the checked-in ceiling.
32+
33+
Data files, both beside this script:
34+
coverage-gate-allowlist.txt paths the checks skip, each with a written
35+
reason above it.
36+
parse-partial-baseline.txt the ceiling for check 4.
37+
38+
Environment:
39+
MAX_SINGLE_RANGE_PCT Share limit for check 3 (default 25).
40+
MIN_FILE_LINES Files below this are exempt from check 3 (default 200).
41+
42+
Exit 0 when every check passes, 1 when any fails, 2 on a bad argument.
43+
44+
Options:
45+
-h, --help This text.
46+
EOF
47+
}
48+
49+
BIN=""
50+
while [ $# -gt 0 ]; do
51+
case "$1" in
52+
-h | --help) usage; exit 0 ;;
53+
-*) echo "self-index-coverage-gate: unknown argument '$1'. Please consult --help." >&2; exit 2 ;;
54+
*)
55+
[ -z "$BIN" ] || {
56+
echo "self-index-coverage-gate: one binary path only. Please consult --help." >&2
57+
exit 2
58+
}
59+
BIN="$1"
60+
;;
61+
esac
62+
shift
63+
done
64+
[ -n "$BIN" ] || { echo "self-index-coverage-gate: need a binary path. Please consult --help." >&2; exit 2; }
65+
2166
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
2267
ALLOWLIST="${REPO_ROOT}/scripts/ci/coverage-gate-allowlist.txt"
2368
BASELINE_FILE="${REPO_ROOT}/scripts/ci/parse-partial-baseline.txt"
@@ -62,17 +107,32 @@ ALLOWED="${WORK}/allowed.txt"
62107
FAILURES=0
63108
note_failure() { echo "FAIL: $*"; FAILURES=$((FAILURES + 1)); }
64109

110+
# ── 0. The report's own file list must be complete ────────────────────────
111+
# index_status lists at most 500 files per class and sets "truncated" when it
112+
# drops the rest. Checks 2 and 3 below read that list file by file, so a
113+
# clipped list means they judge only part of the repo and still print PASS.
114+
# That is the same silent clipping this gate exists to catch, so stop instead.
115+
for cls in parse_partial parse_unusable; do
116+
clipped="$(jq -r --arg c "$cls" '.structuredContent[$c].truncated // false' "${WORK}/status.json")"
117+
if [ "$clipped" = "true" ]; then
118+
note_failure "index_status clipped its ${cls} file list — the checks below would see only part of it"
119+
fi
120+
done
121+
65122
# ── 1. Nothing may fail across a whole file ────────────────────────────────
66123
# parse_unusable means one range covers 80%+ of the file, so the report tells
67124
# a reader to go read the source. Zero today; a new one is a real regression.
68-
UNUSABLE="$(jq -r '.structuredContent.parse_unusable.count // 0' "${WORK}/status.json")"
69125
UNUSABLE_LISTED="$(jq -r '[.structuredContent.parse_unusable.files[]?.path]|join(" ")' \
70126
"${WORK}/status.json")"
127+
UNUSABLE=0
128+
UNUSABLE_KEPT=""
71129
for p in $UNUSABLE_LISTED; do
72-
grep -qxF "$p" "$ALLOWED" && UNUSABLE=$((UNUSABLE - 1))
130+
grep -qxF "$p" "$ALLOWED" && continue
131+
UNUSABLE=$((UNUSABLE + 1))
132+
UNUSABLE_KEPT="${UNUSABLE_KEPT}${p} "
73133
done
74134
if [ "$UNUSABLE" -gt 0 ]; then
75-
note_failure "${UNUSABLE} file(s) report a whole-file parse failure: ${UNUSABLE_LISTED}"
135+
note_failure "${UNUSABLE} file(s) report a whole-file parse failure: ${UNUSABLE_KEPT}"
76136
fi
77137

78138
# ── 2. No range list may be silently clipped ──────────────────────────────
@@ -104,7 +164,12 @@ done < <(jq -r '.structuredContent.parse_partial.files[]?
104164
| "\(.path)\t\(.error_ranges // "")"' "${WORK}/status.json")
105165

106166
# ── 4. The flagged-file count must not drift upward unnoticed ─────────────
107-
CEILING="$(sed -e 's/#.*//' "$BASELINE_FILE" | grep -oE '[0-9]+' | head -1)"
167+
# One awk, not `grep | head -1`: under `set -o pipefail` head closes the pipe
168+
# early, grep dies of SIGPIPE, and the whole gate aborts with no message. It
169+
# does not bite on today's short file, and it would bite the day someone adds
170+
# a few comment lines.
171+
CEILING="$(awk '{ sub(/#.*/, "") } match($0, /[0-9]+/) { print substr($0, RSTART, RLENGTH); exit }' \
172+
"$BASELINE_FILE")"
108173
PARTIAL="$(jq -r '.structuredContent.parse_partial.count // 0' "${WORK}/status.json")"
109174
if [ "$PARTIAL" -gt "$CEILING" ]; then
110175
note_failure "parse_partial_count is ${PARTIAL}, above the ceiling ${CEILING} in $(basename "$BASELINE_FILE")"

‎tests/test_venue_parity_contract.sh‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,7 @@ scripts/smoke-invariants.sh
388388
scripts/ci/preflight-docker.sh
389389
scripts/ci/require-all-green.sh
390390
scripts/ci/verify-shard-union.sh
391+
scripts/ci/self-index-coverage-gate.sh
391392
scripts/ci/generate-sbom.py
392393
scripts/package-release.sh
393394
scripts/ci/smoke-artifact.sh
@@ -426,6 +427,7 @@ scripts/lint.sh
426427
scripts/smoke-local.sh
427428
scripts/soak-legs.sh
428429
scripts/ci/preflight-docker.sh
430+
scripts/ci/self-index-coverage-gate.sh
429431
test-infrastructure/vm/vm-smoke.sh
430432
scripts/smoke-invariants.sh
431433
"

0 commit comments

Comments
 (0)