From e5c17de6217ec5938d18d8c51e2c0d7ead116a19 Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sun, 30 Aug 2026 20:45:09 -0400 Subject: [PATCH 1/5] fix(coverage): stop reporting a duplicate range and a line past EOF (#963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scripts/setup-windows.ps1 has 326 lines and its parse-coverage report read "113-113,113-113,245-327" — the same line named twice, and an end line that does not exist. Two separate faults, both in cbm_error_regions_push. Past-EOF end line. A tree-sitter node that ends at column 0 stopped right after the previous line's newline, so it holds no text on the row it points at. Adding 1 to that row named a line past the end of the file whenever the region ran to EOF. The node here is start=(244,2) end=(326,0). Clamp the end to the row above when the end column is 0 and the node spans more than one row. Duplicate range. Line 113 carries two separate ERROR nodes, at columns 25-29 and 31-32, and each pushed its own range. A line range says nothing new the second time. Drop a range that exactly repeats the one already open. The drop runs BEFORE the cap check, so a repeat is never miscounted as a range the cap threw away. Only an EXACT repeat is dropped, never a range that merely overlaps. Each range is judged separately afterwards by cbm_region_is_recovered, which asks whether definitions starting inside that range cover it. Two ranges holding the same numbers always get the same verdict, so dropping the repeat changes nothing. Two different ranges do not. Merging 3-3 into 2-3 hands the wider range's covering definition to an error that definition does not explain, and a real parse failure then vanishes from the report. That is not hypothetical. An earlier version of this commit merged on overlap and broke perl_malformed_source_remains_partial_issue1838, the test added with the Perl grammar refresh in 17b5a432. The malformed fixture produces two ERROR nodes, at lines 2-3 and 3-3. Merged, the 2-3 range looks fully covered by before_error and is removed, so parse_incomplete comes back false on a file that plainly does not parse. That test now pins this boundary. The real file reports "113-113,245-326". Two tests, both proved RED first with the exact expected text: coverage_repeated_error_line_reports_one_range_issue963 "2-2,2-2" != "2-2" coverage_range_never_ends_past_the_last_line_issue963 "1-5" != "1-4" Suites run on this change: parse_coverage 34, extraction 325, pipeline 264, mcp 246, index_resilience 7 — all passing. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joshua Richter --- internal/cbm/cbm.c | 39 ++++++++++++++++++++++++++++-- tests/test_parse_coverage.c | 47 +++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index 124d082f7..0bd718f82 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -794,12 +794,47 @@ typedef struct { } cbm_error_regions_t; static void cbm_error_regions_push(cbm_error_regions_t *acc, TSNode n) { + TSPoint start = ts_node_start_point(n); + TSPoint end = ts_node_end_point(n); + uint32_t start_line = start.row + 1; + uint32_t end_line = end.row + 1; + + /* A node that ends at column 0 stopped right after the previous line's + * newline, so it holds no text on the row it points at. Counting that row + * named a line past the end of the file whenever the region ran to EOF: + * scripts/setup-windows.ps1 has 326 lines and reported "245-327". */ + if (end.column == 0 && end.row > start.row) { + end_line = end.row; + } + + /* One line can carry several error nodes, and repeating the same line range + * says nothing new. Line 113 of scripts/setup-windows.ps1 has two error + * nodes, at columns 25-29 and 31-32, and the report read "113-113,113-113". + * Drop the repeat. + * + * Only an EXACT repeat of the range already open is dropped. Do not merge + * ranges that merely overlap. Each range is judged separately later by + * cbm_region_is_recovered, which asks whether definitions starting inside + * that range cover it. Two ranges with the same numbers always get the same + * verdict, so collapsing them changes nothing. Two DIFFERENT ranges do not: + * merging 3-3 into 2-3 hands the wider range's covering definition to an + * error the definition does not explain, and a real parse failure then + * disappears from the report. tests/test_parse_coverage.c pins that case in + * perl_malformed_source_remains_partial_issue1838. + * + * This runs BEFORE the cap check, so a dropped repeat never counts as a + * range the cap threw away. */ + if (acc->count > 0 && start_line == acc->starts[acc->count - 1] && + end_line == acc->ends[acc->count - 1]) { + return; + } + if (acc->count >= CBM_MAX_ERROR_REGIONS) { acc->dropped++; return; } - acc->starts[acc->count] = ts_node_start_point(n).row + 1; - acc->ends[acc->count] = ts_node_end_point(n).row + 1; + acc->starts[acc->count] = start_line; + acc->ends[acc->count] = end_line; acc->count++; } diff --git a/tests/test_parse_coverage.c b/tests/test_parse_coverage.c index 4b2692dee..f657e1e81 100644 --- a/tests/test_parse_coverage.c +++ b/tests/test_parse_coverage.c @@ -834,6 +834,51 @@ TEST(c_thread_local_grammar_limit_is_pinned_issue963) { PASS(); } +/* Two error nodes can sit on ONE line. Line 113 of scripts/setup-windows.ps1 + * does exactly that, and the report used to read "113-113,113-113" — the same + * line named twice. A line range says nothing new the second time, so repeated + * or overlapping regions must collapse into one. */ +static const char *PS_TWO_ERRORS_ONE_LINE = "Write-Host \"start\"\n" /* 1 */ + "wsl.exe -- bash -c $Command 2>&1\n" /* 2 */ + "Write-Host \"end\"\n"; /* 3 */ + +/* An error region that runs to the end of the file stops just after the last + * newline. Tree-sitter calls that position row N, column 0 — a row that holds + * no text. Reading it as a line number named a line past the end of the file: + * scripts/setup-windows.ps1 has 326 lines and the report said "245-327". */ +static const char *PS_ERROR_TO_EOF = "} else {\n" /* 1 */ + " if ($a) {\n" /* 2 */ + " Write-Host x\n" /* 3 */ + "}\n"; /* 4 */ + +TEST(coverage_repeated_error_line_reports_one_range_issue963) { + CBMFileResult *r = + cbm_extract_file(PS_TWO_ERRORS_ONE_LINE, (int)strlen(PS_TWO_ERRORS_ONE_LINE), + CBM_LANG_POWERSHELL, "covproj", "two_errors.ps1", 0, NULL, NULL); + ASSERT_NOT_NULL(r); + ASSERT_TRUE(r->parse_incomplete); + ASSERT_NOT_NULL(r->error_ranges); + /* Line 2 carries two separate error nodes. It must be named once. */ + ASSERT_STR_EQ(r->error_ranges, "2-2"); + ASSERT_EQ(r->error_region_count, 1); + cbm_free_result(r); + PASS(); +} + +TEST(coverage_range_never_ends_past_the_last_line_issue963) { + int len = (int)strlen(PS_ERROR_TO_EOF); + CBMFileResult *r = cbm_extract_file(PS_ERROR_TO_EOF, len, CBM_LANG_POWERSHELL, "covproj", + "error_to_eof.ps1", 0, NULL, NULL); + ASSERT_NOT_NULL(r); + ASSERT_TRUE(r->parse_incomplete); + ASSERT_NOT_NULL(r->error_ranges); + /* The file has four lines and ends with a newline. Line 5 does not exist. */ + ASSERT_STR_EQ(r->error_ranges, "1-4"); + ASSERT_NULL(strstr(r->error_ranges, "5")); + cbm_free_result(r); + PASS(); +} + SUITE(parse_coverage) { RUN_TEST(c_ifdef_split_brace_sets_parse_incomplete); RUN_TEST(c_ifdef_split_brace_neighbors_still_extracted); @@ -870,4 +915,6 @@ SUITE(parse_coverage) { RUN_TEST(real_error_before_eof_still_flagged_with_trailing_blank_issue1746); RUN_TEST(width_bearing_error_at_eof_still_flagged_with_trailing_blank_issue1746); RUN_TEST(c_thread_local_grammar_limit_is_pinned_issue963); + RUN_TEST(coverage_repeated_error_line_reports_one_range_issue963); + RUN_TEST(coverage_range_never_ends_past_the_last_line_issue963); } From 4aeb0b90073593b78601cd0dede932c38c46e9a3 Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sun, 30 Aug 2026 00:39:43 -0400 Subject: [PATCH 2/5] ci(coverage): fail a PR when this repo's own parse-coverage report goes bad (#963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A coverage range is advice — "these lines are missing from the graph, read them". It stops being advice when it names most of the file, and it stops being honest when the list was clipped without saying so. Both happened here: src/cli/cli.c reported its whole 13,046 lines as one range, and two caps in series dropped ranges with no signal. Nothing would have caught either. scripts/ci/self-index-coverage-gate.sh indexes this repo with the binary just built and fails on any of four things: 1. A file reports a whole-file parse failure (parse_unusable). Zero today. 2. Any range string carries the "+" truncation marker. With the cap at 256, a file that still overflows is worth stopping for. 3. Any single range covers more than 25% of its file, for files of 200 lines or more. The floor matters: a 5-line PL/SQL limitation fixture with a 3-line range is 60% of itself and says nothing about report quality. 4. parse_partial_count rises above the ceiling in parse-partial-baseline.txt (58 today). This complements the FLOOR in tests/test_index_resilience.c, which stops the signal being switched off by accident. Every check was verified to FAIL, not just to pass: empty allowlist -> setup-windows.ps1 flagged at 25.5% MAX_SINGLE_RANGE_PCT=3 -> cli.c flagged at 3.9% ceiling 57 -> parse_partial_count 58 flagged a repo of broken files -> 4 whole-file failures flagged a 1200-line garbage file -> its clipped range list flagged scripts/setup-windows.ps1 is the one allowlist entry, and it is a real gap rather than noise: one range covers lines 245-327 of a 326-line file because the tree-sitter PowerShell grammar cannot parse the `} else {` branch running to EOF, so those 83 lines genuinely are absent from the graph. Every other file of 200+ lines sits at 3.9% or below, so the 25% threshold has room and should not be raised to hide this. Wired into the existing pr-smoke job, Ubuntu leg only. That job is already in ci-ok's needs, so the gate is a required check with no workflow-graph surgery. Ubuntu only because the flagged ranges depend on which conditional-compilation branches the preprocessor keeps — on a machine where _WIN32 is defined a different set of lines is flagged, which is why the gate asserts proportions and never exact line numbers. The changes filter now notices edits to the gate, the allowlist and the baseline. Runs in 21 seconds. Not extended into scripts/smoke-invariants.sh on purpose: that runs from smoke.yml, whose triggers are workflow_dispatch and push to qa/smoke-**, and which is documented non-gating — it would never run on a PR. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joshua Richter --- .github/workflows/pr.yml | 11 ++- scripts/ci/coverage-gate-allowlist.txt | 15 ++++ scripts/ci/parse-partial-baseline.txt | 7 ++ scripts/ci/self-index-coverage-gate.sh | 118 +++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 scripts/ci/coverage-gate-allowlist.txt create mode 100644 scripts/ci/parse-partial-baseline.txt create mode 100755 scripts/ci/self-index-coverage-gate.sh diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index c18a45d13..8d854b06c 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -59,7 +59,7 @@ jobs: # The paginated files endpoint remains filename-only for this gate. FILES=$(gh api --paginate "repos/$REPO/pulls/$PR/files?per_page=100" --jq '.[].filename') printf '%s\n' "$FILES" - 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|test-infrastructure/vm/(vm-smoke\.sh|windows-user-path-guard\.ps1)|Makefile\.cbm)'; then + 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 echo "product=true" >> "$GITHUB_OUTPUT" else echo "product=false" >> "$GITHUB_OUTPUT" @@ -127,6 +127,15 @@ jobs: CCACHE_DIR: ${{ github.workspace }}/.ccache CCACHE_MAXSIZE: 1000M + # Index this repo with the binary just built and check its own + # parse-coverage report is still useful advice (#963). Ubuntu only: the + # flagged line ranges depend on which conditional-compilation branches + # the preprocessor keeps, so they differ per platform. The gate asserts + # proportions, never exact line numbers. + - name: Parse-coverage gate (Ubuntu) + if: matrix.os == 'ubuntu-latest' + run: scripts/ci/self-index-coverage-gate.sh "$(pwd)/build/c/codebase-memory-mcp" + - name: Build prod + smoke (macOS) if: matrix.os == 'macos-14' run: | diff --git a/scripts/ci/coverage-gate-allowlist.txt b/scripts/ci/coverage-gate-allowlist.txt new file mode 100644 index 000000000..eb5159993 --- /dev/null +++ b/scripts/ci/coverage-gate-allowlist.txt @@ -0,0 +1,15 @@ +# Files the self-index coverage gate skips, one repo-relative path per line. +# +# Adding a line here is a deliberate decision, not a convenience. It says: +# "we know this file reports a wide parse-coverage range, we have looked at +# why, and we accept it." Write the reason above the path. A line with no +# reason should be removed rather than trusted. +# +# Blank lines and lines starting with # are ignored. + +# One range covers lines 245-327 of a 326-line file (25.5%). The tree-sitter +# PowerShell grammar cannot parse the `} else {` branch that runs to the end +# of the file, so those 83 lines really are absent from the graph. This is a +# genuine grammar gap, not a reporting error. Every other file of 200+ lines +# in this repo sits at 3.9% or below. +scripts/setup-windows.ps1 diff --git a/scripts/ci/parse-partial-baseline.txt b/scripts/ci/parse-partial-baseline.txt new file mode 100644 index 000000000..0803effe1 --- /dev/null +++ b/scripts/ci/parse-partial-baseline.txt @@ -0,0 +1,7 @@ +# Ceiling for parse_partial_count when this repo indexes itself. +# +# The number below is what the gate allows. It complements the FLOOR asserted +# in tests/test_index_resilience.c, which stops the signal being switched off +# by accident. Raising this number is allowed but should be explained in the +# commit that does it. +58 diff --git a/scripts/ci/self-index-coverage-gate.sh b/scripts/ci/self-index-coverage-gate.sh new file mode 100755 index 000000000..e2eebca7e --- /dev/null +++ b/scripts/ci/self-index-coverage-gate.sh @@ -0,0 +1,118 @@ +#!/usr/bin/env bash +# Regression guard: this repo's own parse-coverage report must stay useful. +# +# A coverage range is advice — "these lines are missing from the graph, read +# them". Advice stops being advice when it names most of the file, and it +# stops being honest when the list was clipped without saying so. Both things +# happened here before (#963): src/cli/cli.c reported its whole 13,046 lines +# as one range, and two caps in series dropped ranges with no signal at all. +# +# This indexes the repo with a given binary and fails if any of that comes back. +# +# Usage: self-index-coverage-gate.sh +# +# NOTE ON PLATFORM: the ranges depend on which conditional-compilation branches +# the preprocessor keeps. On a machine where _WIN32 is defined the discarded +# branches swap and a different set of lines is flagged. That is why this runs +# on ONE CI leg and asserts proportions rather than exact line numbers. +set -euo pipefail + +BIN="${1:?usage: self-index-coverage-gate.sh }" +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +ALLOWLIST="${REPO_ROOT}/scripts/ci/coverage-gate-allowlist.txt" +BASELINE_FILE="${REPO_ROOT}/scripts/ci/parse-partial-baseline.txt" + +# Share of a file one range may cover before it stops being useful advice. +# The worst real offender today is src/cli/cli.c at 3.9%, so this has room. +MAX_SINGLE_RANGE_PCT="${MAX_SINGLE_RANGE_PCT:-25}" +# Files below this are exempt: a 5-line fixture with a 3-line range is 60% of +# itself and says nothing about report quality. +MIN_FILE_LINES="${MIN_FILE_LINES:-200}" + +command -v jq >/dev/null || { echo "FAIL: jq is required"; exit 1; } + +WORK="$(mktemp -d)" +# The runtime dir holds a unix socket, and a socket path has a hard length +# limit (~104 bytes). macOS puts mktemp under /var/folders//T/, which +# blows that limit and fails with "secure CLI coordination could not be +# created (endpoint)". Keep the runtime dir short and separate from the cache. +RUNTIME="/tmp/cbm-gate.$$" +trap 'rm -rf "$WORK" "$RUNTIME"' EXIT +export CBM_CACHE_DIR="${WORK}/cache" +export CBM_RUNTIME_DIR="$RUNTIME" +mkdir -p "$CBM_CACHE_DIR" "$RUNTIME" + +echo "==> indexing ${REPO_ROOT} with $(basename "$BIN")" +"$BIN" cli index_repository --repo-path "$REPO_ROOT" --mode full --json \ + > "${WORK}/index.json" 2>"${WORK}/index.err" || { + echo "FAIL: index_repository exited non-zero"; tail -20 "${WORK}/index.err"; exit 1; } + +PROJECT="$(jq -r '.structuredContent.project // empty' "${WORK}/index.json")" +[ -n "$PROJECT" ] || { echo "FAIL: index_repository did not name a project"; exit 1; } + +"$BIN" cli index_status --project "$PROJECT" --json > "${WORK}/status.json" 2>/dev/null || { + echo "FAIL: index_status exited non-zero"; exit 1; } + +# Allowlisted paths, comments and blanks stripped. +ALLOWED="${WORK}/allowed.txt" +: > "$ALLOWED" +[ -f "$ALLOWLIST" ] && sed -e 's/#.*//' -e 's/[[:space:]]*$//' "$ALLOWLIST" \ + | grep -v '^$' > "$ALLOWED" || true + +FAILURES=0 +note_failure() { echo "FAIL: $*"; FAILURES=$((FAILURES + 1)); } + +# ── 1. Nothing may fail across a whole file ──────────────────────────────── +# parse_unusable means one range covers 80%+ of the file, so the report tells +# a reader to go read the source. Zero today; a new one is a real regression. +UNUSABLE="$(jq -r '.structuredContent.parse_unusable.count // 0' "${WORK}/status.json")" +UNUSABLE_LISTED="$(jq -r '[.structuredContent.parse_unusable.files[]?.path]|join(" ")' \ + "${WORK}/status.json")" +for p in $UNUSABLE_LISTED; do + grep -qxF "$p" "$ALLOWED" && UNUSABLE=$((UNUSABLE - 1)) +done +if [ "$UNUSABLE" -gt 0 ]; then + note_failure "${UNUSABLE} file(s) report a whole-file parse failure: ${UNUSABLE_LISTED}" +fi + +# ── 2. No range list may be silently clipped ────────────────────────────── +# A trailing "+" says the producer's cap threw N ranges away. With the cap +# at 256 a file that still overflows is worth stopping for. +TRUNCATED="$(jq -r '[.structuredContent.parse_partial.files[]? + | select(.error_ranges? // "" | test("\\+[0-9]+$")) | .path] | join(" ")' \ + "${WORK}/status.json")" +for p in $TRUNCATED; do + grep -qxF "$p" "$ALLOWED" && continue + note_failure "$p carries a +N truncation marker — its range list was clipped" +done + +# ── 3. No single range may cover a quarter of its file ──────────────────── +while IFS=$'\t' read -r path ranges; do + [ -n "$path" ] || continue + grep -qxF "$path" "$ALLOWED" && continue + [ -f "${REPO_ROOT}/${path}" ] || continue + total="$(wc -l < "${REPO_ROOT}/${path}" | tr -d ' ')" + [ "$total" -ge "$MIN_FILE_LINES" ] || continue + widest="$(printf '%s' "$ranges" | tr ',' '\n' | grep '^[0-9]' \ + | awk -F- '{d=$2-$1+1; if (d>m) m=d} END {print m+0}')" + pct="$(awk -v a="$widest" -v b="$total" 'BEGIN{printf "%.1f", 100*a/b}')" + over="$(awk -v p="$pct" -v lim="$MAX_SINGLE_RANGE_PCT" 'BEGIN{print (p>lim)?1:0}')" + if [ "$over" = "1" ]; then + note_failure "$path has one range of ${widest} lines — ${pct}% of ${total}, over ${MAX_SINGLE_RANGE_PCT}%" + fi +done < <(jq -r '.structuredContent.parse_partial.files[]? + | "\(.path)\t\(.error_ranges // "")"' "${WORK}/status.json") + +# ── 4. The flagged-file count must not drift upward unnoticed ───────────── +CEILING="$(sed -e 's/#.*//' "$BASELINE_FILE" | grep -oE '[0-9]+' | head -1)" +PARTIAL="$(jq -r '.structuredContent.parse_partial.count // 0' "${WORK}/status.json")" +if [ "$PARTIAL" -gt "$CEILING" ]; then + note_failure "parse_partial_count is ${PARTIAL}, above the ceiling ${CEILING} in $(basename "$BASELINE_FILE")" +fi + +echo "==> parse_partial=${PARTIAL} (ceiling ${CEILING}) parse_unusable=${UNUSABLE} allowlisted=$(wc -l < "$ALLOWED" | tr -d ' ')" +if [ "$FAILURES" -gt 0 ]; then + echo "FAIL: ${FAILURES} coverage-gate check(s) failed" + exit 1 +fi +echo "PASS: parse-coverage report is within bounds" From 1c68d0988daef50bf25f94ccf3b68319ef468f6a Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sun, 30 Aug 2026 20:45:25 -0400 Subject: [PATCH 3/5] ci(coverage): fix three ways the coverage gate could pass without checking (#963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Signed-off-by: Joshua Richter --- scripts/ci/README.md | 1 + scripts/ci/coverage-gate-allowlist.txt | 8 ++- scripts/ci/self-index-coverage-gate.sh | 75 ++++++++++++++++++++++++-- tests/test_venue_parity_contract.sh | 2 + 4 files changed, 79 insertions(+), 7 deletions(-) diff --git a/scripts/ci/README.md b/scripts/ci/README.md index f296e6fc5..a6cab8f5d 100644 --- a/scripts/ci/README.md +++ b/scripts/ci/README.md @@ -13,6 +13,7 @@ CI and the local infrastructure — both of which the venue-parity contract | `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` | | `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 | | `generate-sbom.py` | The release SPDX SBOM (vendored versions reviewable here, diffable by vendoring PRs — was inline YAML). | `release.yml` | +| `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` | | `require-all-green.sh` | The aggregate gate: fail unless every needed job succeeded or legitimately skipped (was inline YAML). | `pr.yml ci-ok` | | `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 | | `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 | diff --git a/scripts/ci/coverage-gate-allowlist.txt b/scripts/ci/coverage-gate-allowlist.txt index eb5159993..697cae590 100644 --- a/scripts/ci/coverage-gate-allowlist.txt +++ b/scripts/ci/coverage-gate-allowlist.txt @@ -7,9 +7,13 @@ # # Blank lines and lines starting with # are ignored. -# One range covers lines 245-327 of a 326-line file (25.5%). The tree-sitter +# One range covers lines 245-326 of a 326-line file (25.2%). The tree-sitter # PowerShell grammar cannot parse the `} else {` branch that runs to the end -# of the file, so those 83 lines really are absent from the graph. This is a +# of the file, so those 82 lines really are absent from the graph. This is a # genuine grammar gap, not a reporting error. Every other file of 200+ lines # in this repo sits at 3.9% or below. +# +# The range read 113-113,113-113,245-327 until the duplicate and the past-EOF +# end line were fixed in cbm_error_regions_push. Narrowing it by one line did +# NOT clear the gate: 25.2% is still over the 25% limit, so this entry stays. scripts/setup-windows.ps1 diff --git a/scripts/ci/self-index-coverage-gate.sh b/scripts/ci/self-index-coverage-gate.sh index e2eebca7e..00662a2b6 100755 --- a/scripts/ci/self-index-coverage-gate.sh +++ b/scripts/ci/self-index-coverage-gate.sh @@ -17,7 +17,52 @@ # on ONE CI leg and asserts proportions rather than exact line numbers. set -euo pipefail -BIN="${1:?usage: self-index-coverage-gate.sh }" +usage() { + cat <<'EOF' +Usage: scripts/ci/self-index-coverage-gate.sh + +Index THIS repository with the given binary and fail if the repository's own +parse-coverage report stops being useful advice (#963). + +Four checks, all read from index_status: + 1. No file reports a whole-file parse failure (parse_unusable). + 2. No range list was clipped without saying so (a trailing "+" marker). + 3. No single range covers more than 25% of a file of 200 lines or more. + 4. The flagged-file count stays at or below the checked-in ceiling. + +Data files, both beside this script: + coverage-gate-allowlist.txt paths the checks skip, each with a written + reason above it. + parse-partial-baseline.txt the ceiling for check 4. + +Environment: + MAX_SINGLE_RANGE_PCT Share limit for check 3 (default 25). + MIN_FILE_LINES Files below this are exempt from check 3 (default 200). + +Exit 0 when every check passes, 1 when any fails, 2 on a bad argument. + +Options: + -h, --help This text. +EOF +} + +BIN="" +while [ $# -gt 0 ]; do + case "$1" in + -h | --help) usage; exit 0 ;; + -*) echo "self-index-coverage-gate: unknown argument '$1'. Please consult --help." >&2; exit 2 ;; + *) + [ -z "$BIN" ] || { + echo "self-index-coverage-gate: one binary path only. Please consult --help." >&2 + exit 2 + } + BIN="$1" + ;; + esac + shift +done +[ -n "$BIN" ] || { echo "self-index-coverage-gate: need a binary path. Please consult --help." >&2; exit 2; } + REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" ALLOWLIST="${REPO_ROOT}/scripts/ci/coverage-gate-allowlist.txt" BASELINE_FILE="${REPO_ROOT}/scripts/ci/parse-partial-baseline.txt" @@ -62,17 +107,32 @@ ALLOWED="${WORK}/allowed.txt" FAILURES=0 note_failure() { echo "FAIL: $*"; FAILURES=$((FAILURES + 1)); } +# ── 0. The report's own file list must be complete ──────────────────────── +# index_status lists at most 500 files per class and sets "truncated" when it +# drops the rest. Checks 2 and 3 below read that list file by file, so a +# clipped list means they judge only part of the repo and still print PASS. +# That is the same silent clipping this gate exists to catch, so stop instead. +for cls in parse_partial parse_unusable; do + clipped="$(jq -r --arg c "$cls" '.structuredContent[$c].truncated // false' "${WORK}/status.json")" + if [ "$clipped" = "true" ]; then + note_failure "index_status clipped its ${cls} file list — the checks below would see only part of it" + fi +done + # ── 1. Nothing may fail across a whole file ──────────────────────────────── # parse_unusable means one range covers 80%+ of the file, so the report tells # a reader to go read the source. Zero today; a new one is a real regression. -UNUSABLE="$(jq -r '.structuredContent.parse_unusable.count // 0' "${WORK}/status.json")" UNUSABLE_LISTED="$(jq -r '[.structuredContent.parse_unusable.files[]?.path]|join(" ")' \ "${WORK}/status.json")" +UNUSABLE=0 +UNUSABLE_KEPT="" for p in $UNUSABLE_LISTED; do - grep -qxF "$p" "$ALLOWED" && UNUSABLE=$((UNUSABLE - 1)) + grep -qxF "$p" "$ALLOWED" && continue + UNUSABLE=$((UNUSABLE + 1)) + UNUSABLE_KEPT="${UNUSABLE_KEPT}${p} " done if [ "$UNUSABLE" -gt 0 ]; then - note_failure "${UNUSABLE} file(s) report a whole-file parse failure: ${UNUSABLE_LISTED}" + note_failure "${UNUSABLE} file(s) report a whole-file parse failure: ${UNUSABLE_KEPT}" fi # ── 2. No range list may be silently clipped ────────────────────────────── @@ -104,7 +164,12 @@ done < <(jq -r '.structuredContent.parse_partial.files[]? | "\(.path)\t\(.error_ranges // "")"' "${WORK}/status.json") # ── 4. The flagged-file count must not drift upward unnoticed ───────────── -CEILING="$(sed -e 's/#.*//' "$BASELINE_FILE" | grep -oE '[0-9]+' | head -1)" +# One awk, not `grep | head -1`: under `set -o pipefail` head closes the pipe +# early, grep dies of SIGPIPE, and the whole gate aborts with no message. It +# does not bite on today's short file, and it would bite the day someone adds +# a few comment lines. +CEILING="$(awk '{ sub(/#.*/, "") } match($0, /[0-9]+/) { print substr($0, RSTART, RLENGTH); exit }' \ + "$BASELINE_FILE")" PARTIAL="$(jq -r '.structuredContent.parse_partial.count // 0' "${WORK}/status.json")" if [ "$PARTIAL" -gt "$CEILING" ]; then note_failure "parse_partial_count is ${PARTIAL}, above the ceiling ${CEILING} in $(basename "$BASELINE_FILE")" diff --git a/tests/test_venue_parity_contract.sh b/tests/test_venue_parity_contract.sh index 4bcd694d7..001e5f856 100755 --- a/tests/test_venue_parity_contract.sh +++ b/tests/test_venue_parity_contract.sh @@ -388,6 +388,7 @@ scripts/smoke-invariants.sh scripts/ci/preflight-docker.sh scripts/ci/require-all-green.sh scripts/ci/verify-shard-union.sh +scripts/ci/self-index-coverage-gate.sh scripts/ci/generate-sbom.py scripts/package-release.sh scripts/ci/smoke-artifact.sh @@ -426,6 +427,7 @@ scripts/lint.sh scripts/smoke-local.sh scripts/soak-legs.sh scripts/ci/preflight-docker.sh +scripts/ci/self-index-coverage-gate.sh test-infrastructure/vm/vm-smoke.sh scripts/smoke-invariants.sh " From 87c11884504285cad1db9af997d380e08c4635e0 Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Mon, 31 Aug 2026 15:52:28 -0400 Subject: [PATCH 4/5] ci(coverage): raise the parse-partial ceiling to 59, for a gap main added (#963) The gate's check 4 failed on its own pull request: parse_partial_count is 59 against a ceiling of 58. The rise did not come from this branch. Measured with the binary built from this branch, three trees indexed: this branch, no merge 58 matches the baseline as written origin/main alone 59 main merged into it 59 identical file list to main alone CI tests the merge of a pull request into main, so the gate sees 59. The one file main added to the flagged list is src/daemon/runtime.c, at one line: src/daemon/runtime.c 47-47 1 line of 3291 0.03% of the file Line 47 is a function-style _Atomic declaration: static _Atomic(cbm_daemon_runtime_containment_hook_t) runtime_containment_hook_seam; The tree-sitter C grammar does not parse that form. The keyword form four lines above it, `static _Atomic uint32_t ...`, parses fine. This is the same class of grammar limitation this branch already pins for _Thread_local in tests/test_parse_coverage.c. It arrived with fc1b1ee7 on main. So the ceiling moves to 59 rather than the file being allowlisted. An allowlist entry is for a file whose single range is over the 25% limit and has a written reason to stay; one line out of 3291 is nowhere near it, and hiding the file would remove a real gap from the count the ceiling exists to watch. Known cost, worth stating plainly: a ceiling checked against a moving main drifts. Any merge to main that adds a partially-parsing file reddens every open pull request until someone edits this file by hand, and the pull request that goes red is never the one that caused it. This commit does not fix that - the fix is to compare against the merge base instead of a checked-in number, which changes what the gate is and belongs to whoever owns CI policy here. Filed separately. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joshua Richter --- scripts/ci/parse-partial-baseline.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/ci/parse-partial-baseline.txt b/scripts/ci/parse-partial-baseline.txt index 0803effe1..b5d22da69 100644 --- a/scripts/ci/parse-partial-baseline.txt +++ b/scripts/ci/parse-partial-baseline.txt @@ -4,4 +4,8 @@ # in tests/test_index_resilience.c, which stops the signal being switched off # by accident. Raising this number is allowed but should be explained in the # commit that does it. -58 +# +# 58 -> 59 on 2026-08-31. main gained src/daemon/runtime.c, whose line 47 the +# tree-sitter C grammar cannot parse. The rise came from main, not from a +# branch. See the commit for the measurement. +59 From 2b1dafd83d6df8b0ad8a064e1b23bc27ecf4de7d Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Tue, 1 Sep 2026 20:15:26 -0400 Subject: [PATCH 5/5] 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 --- .github/workflows/pr.yml | 8 +- scripts/ci/README.md | 2 +- scripts/ci/parse-partial-baseline.txt | 21 +- scripts/ci/self-index-coverage-gate.sh | 275 +++++++++++++++++-------- scripts/test.sh | 3 + tests/test_coverage_gate_contract.sh | 241 ++++++++++++++++++++++ 6 files changed, 456 insertions(+), 94 deletions(-) create mode 100755 tests/test_coverage_gate_contract.sh diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 8d854b06c..2db1e0d31 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -59,7 +59,7 @@ jobs: # The paginated files endpoint remains filename-only for this gate. FILES=$(gh api --paginate "repos/$REPO/pulls/$PR/files?per_page=100" --jq '.[].filename') printf '%s\n' "$FILES" - 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 + 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 echo "product=true" >> "$GITHUB_OUTPUT" else echo "product=false" >> "$GITHUB_OUTPUT" @@ -134,6 +134,12 @@ jobs: # proportions, never exact line numbers. - name: Parse-coverage gate (Ubuntu) if: matrix.os == 'ubuntu-latest' + env: + # The gate compares this branch against the merge base, so it needs + # the commit GitHub used to build the merge. Without this it falls + # back to the first parent of HEAD, which is the same commit on a + # pull_request checkout — this makes it explicit rather than lucky. + COVERAGE_GATE_BASE_SHA: ${{ github.event.pull_request.base.sha }} run: scripts/ci/self-index-coverage-gate.sh "$(pwd)/build/c/codebase-memory-mcp" - name: Build prod + smoke (macOS) diff --git a/scripts/ci/README.md b/scripts/ci/README.md index a6cab8f5d..f14549451 100644 --- a/scripts/ci/README.md +++ b/scripts/ci/README.md @@ -13,7 +13,7 @@ CI and the local infrastructure — both of which the venue-parity contract | `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` | | `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 | | `generate-sbom.py` | The release SPDX SBOM (vendored versions reviewable here, diffable by vendoring PRs — was inline YAML). | `release.yml` | -| `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` | +| `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` | | `require-all-green.sh` | The aggregate gate: fail unless every needed job succeeded or legitimately skipped (was inline YAML). | `pr.yml ci-ok` | | `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 | | `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 | diff --git a/scripts/ci/parse-partial-baseline.txt b/scripts/ci/parse-partial-baseline.txt index b5d22da69..0f9d95688 100644 --- a/scripts/ci/parse-partial-baseline.txt +++ b/scripts/ci/parse-partial-baseline.txt @@ -1,9 +1,20 @@ -# Ceiling for parse_partial_count when this repo indexes itself. +# A written record of parse_partial_count when this repo indexes itself. # -# The number below is what the gate allows. It complements the FLOOR asserted -# in tests/test_index_resilience.c, which stops the signal being switched off -# by accident. Raising this number is allowed but should be explained in the -# commit that does it. +# NOTHING FAILS ON THIS NUMBER. The gate reports it and moves on. +# +# It used to be a ceiling. main could move the count on its own and every open +# pull request then went red for a reason none of them caused — that is #1972, +# where main gained src/daemon/runtime.c and the count went 58 -> 59 with no +# branch involved. The gate now indexes the merge base and compares against +# that, so the number below does not gate anything. +# +# What it is still for: a reader can see how far the count has drifted since +# anyone last looked, which is the one thing a merge-base comparison cannot +# show. Update it when you have measured a new number and can say why it moved. +# +# It complements the FLOOR asserted in tests/test_index_resilience.c, which +# stops the coverage signal being switched off by accident. That one IS +# enforced. # # 58 -> 59 on 2026-08-31. main gained src/daemon/runtime.c, whose line 47 the # tree-sitter C grammar cannot parse. The rise came from main, not from a diff --git a/scripts/ci/self-index-coverage-gate.sh b/scripts/ci/self-index-coverage-gate.sh index 00662a2b6..3e2a35432 100755 --- a/scripts/ci/self-index-coverage-gate.sh +++ b/scripts/ci/self-index-coverage-gate.sh @@ -7,7 +7,25 @@ # happened here before (#963): src/cli/cli.c reported its whole 13,046 lines # as one range, and two caps in series dropped ranges with no signal at all. # -# This indexes the repo with a given binary and fails if any of that comes back. +# THIS GATE COMPARES AGAINST THE MERGE BASE, NOT AGAINST A FIXED NUMBER. +# It indexes the base tree and the head tree with the SAME binary and fails +# only on a finding the branch added. An earlier version failed against a +# ceiling checked into a file, which meant main could move the number and turn +# every open pull request red for a reason none of them caused. That happened +# once already (#1972, the count went 58 -> 59 because main gained a file), +# and #1824 will do it again and larger — Blazor .razor files map to C#, the +# markup lands in ERROR regions by design, and the count rises by roughly the +# repo's .razor count. A coverage improvement must not read as a gate failure. +# +# WHAT THIS CANNOT SEE. Both trees are indexed with the same binary, so the +# comparison isolates what the TREE changed. 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 this whole step takes. What still covers it: the FLOOR asserted in +# tests/test_index_resilience.c stops the signal being switched off, and the +# absolute counts for both sides are printed below on every run, so a jump is +# visible in the log even when it does not fail. # # Usage: self-index-coverage-gate.sh # @@ -21,21 +39,28 @@ usage() { cat <<'EOF' Usage: scripts/ci/self-index-coverage-gate.sh -Index THIS repository with the given binary and fail if the repository's own -parse-coverage report stops being useful advice (#963). +Index the merge base and this branch with the given binary, and fail when the +branch made the repository's own parse-coverage report worse (#963). + +Four checks. Each one reads index_status for both trees and fails only on a +finding present on this branch and absent at the merge base: + 1. A file reports a whole-file parse failure (parse_unusable). + 2. A range list was clipped without saying so (a trailing "+" marker). + 3. A single range covers more than 25% of a file of 200 lines or more. + 4. The flagged-file count is higher than the merge base's count. -Four checks, all read from index_status: - 1. No file reports a whole-file parse failure (parse_unusable). - 2. No range list was clipped without saying so (a trailing "+" marker). - 3. No single range covers more than 25% of a file of 200 lines or more. - 4. The flagged-file count stays at or below the checked-in ceiling. +The base commit is taken from COVERAGE_GATE_BASE_SHA, or from the first parent +when HEAD is a merge commit (CI checks out refs/pull/N/merge), or from +`git merge-base origin/main HEAD`. Data files, both beside this script: coverage-gate-allowlist.txt paths the checks skip, each with a written reason above it. - parse-partial-baseline.txt the ceiling for check 4. + parse-partial-baseline.txt a written record of the count, reported but + NOT enforced. Nothing fails on it. Environment: + COVERAGE_GATE_BASE_SHA The commit to compare against. MAX_SINGLE_RANGE_PCT Share limit for check 3 (default 25). MIN_FILE_LINES Files below this are exempt from check 3 (default 200). @@ -82,102 +107,178 @@ WORK="$(mktemp -d)" # blows that limit and fails with "secure CLI coordination could not be # created (endpoint)". Keep the runtime dir short and separate from the cache. RUNTIME="/tmp/cbm-gate.$$" -trap 'rm -rf "$WORK" "$RUNTIME"' EXIT -export CBM_CACHE_DIR="${WORK}/cache" +BASE_TREE="${WORK}/base-tree" +cleanup() { + # The base tree is a real git worktree, so it has a registration in the + # repository that outlives a plain rm -rf. Retire it first, then the dirs. + if [ -d "$BASE_TREE" ]; then + git -C "$REPO_ROOT" worktree remove --force "$BASE_TREE" >/dev/null 2>&1 || true + fi + git -C "$REPO_ROOT" worktree prune >/dev/null 2>&1 || true + rm -rf "$WORK" "$RUNTIME" +} +trap cleanup EXIT export CBM_RUNTIME_DIR="$RUNTIME" -mkdir -p "$CBM_CACHE_DIR" "$RUNTIME" +mkdir -p "$RUNTIME" + +FAILURES=0 +note_failure() { echo "FAIL: $*"; FAILURES=$((FAILURES + 1)); } + +# ── Which commit are we comparing against? ──────────────────────────────── +# CI checks out refs/pull/N/merge, so the first parent IS the base commit. +# The env var comes from the workflow and wins, because it is the value +# GitHub itself used to build that merge. +resolve_base() { + if [ -n "${COVERAGE_GATE_BASE_SHA:-}" ]; then + printf '%s' "$COVERAGE_GATE_BASE_SHA" + return 0 + fi + if git -C "$REPO_ROOT" rev-parse --verify -q 'HEAD^2' >/dev/null 2>&1; then + git -C "$REPO_ROOT" rev-parse 'HEAD^1' + return 0 + fi + git -C "$REPO_ROOT" merge-base origin/main HEAD 2>/dev/null || return 1 +} -echo "==> indexing ${REPO_ROOT} with $(basename "$BIN")" -"$BIN" cli index_repository --repo-path "$REPO_ROOT" --mode full --json \ - > "${WORK}/index.json" 2>"${WORK}/index.err" || { - echo "FAIL: index_repository exited non-zero"; tail -20 "${WORK}/index.err"; exit 1; } +BASE_SHA="$(resolve_base || true)" +[ -n "$BASE_SHA" ] || { echo "FAIL: could not work out the base commit — set COVERAGE_GATE_BASE_SHA"; exit 1; } -PROJECT="$(jq -r '.structuredContent.project // empty' "${WORK}/index.json")" -[ -n "$PROJECT" ] || { echo "FAIL: index_repository did not name a project"; exit 1; } +# The CI checkout is shallow, so the base commit's tree may not be present. +if ! git -C "$REPO_ROOT" cat-file -e "${BASE_SHA}^{tree}" 2>/dev/null; then + echo "==> fetching base commit ${BASE_SHA}" + git -C "$REPO_ROOT" fetch --no-tags --depth=1 origin "$BASE_SHA" >/dev/null 2>&1 || { + echo "FAIL: could not fetch base commit ${BASE_SHA}"; exit 1; } +fi -"$BIN" cli index_status --project "$PROJECT" --json > "${WORK}/status.json" 2>/dev/null || { - echo "FAIL: index_status exited non-zero"; exit 1; } +# A real worktree rather than an archive: the head side IS a git checkout, and +# the indexer's git passes must see the same shape on both sides. It lives +# outside REPO_ROOT so the head index never walks into it. +git -C "$REPO_ROOT" worktree add --detach "$BASE_TREE" "$BASE_SHA" >/dev/null 2>&1 || { + echo "FAIL: could not check out base commit ${BASE_SHA}"; exit 1; } -# Allowlisted paths, comments and blanks stripped. +# Allowlisted paths, comments and blanks stripped. Applied to BOTH sides, so +# an allowlisted path can neither fail the branch nor mask a base finding. ALLOWED="${WORK}/allowed.txt" : > "$ALLOWED" [ -f "$ALLOWLIST" ] && sed -e 's/#.*//' -e 's/[[:space:]]*$//' "$ALLOWLIST" \ | grep -v '^$' > "$ALLOWED" || true -FAILURES=0 -note_failure() { echo "FAIL: $*"; FAILURES=$((FAILURES + 1)); } +# ── Index one tree and write its findings as sorted path lists ──────────── +# Writes .unusable, .truncated, .wide and +# .count. Fails the run outright when index_status clipped its own +# file list, because the three list-based checks would then judge only part of +# the tree and still print PASS — the same silent clipping this gate exists to +# catch. +analyze_tree() { + tree_root="$1" + prefix="$2" + label="$3" -# ── 0. The report's own file list must be complete ──────────────────────── -# index_status lists at most 500 files per class and sets "truncated" when it -# drops the rest. Checks 2 and 3 below read that list file by file, so a -# clipped list means they judge only part of the repo and still print PASS. -# That is the same silent clipping this gate exists to catch, so stop instead. -for cls in parse_partial parse_unusable; do - clipped="$(jq -r --arg c "$cls" '.structuredContent[$c].truncated // false' "${WORK}/status.json")" - if [ "$clipped" = "true" ]; then - note_failure "index_status clipped its ${cls} file list — the checks below would see only part of it" - fi -done + cache="${WORK}/cache-${label}" + mkdir -p "$cache" + export CBM_CACHE_DIR="$cache" -# ── 1. Nothing may fail across a whole file ──────────────────────────────── -# parse_unusable means one range covers 80%+ of the file, so the report tells -# a reader to go read the source. Zero today; a new one is a real regression. -UNUSABLE_LISTED="$(jq -r '[.structuredContent.parse_unusable.files[]?.path]|join(" ")' \ - "${WORK}/status.json")" -UNUSABLE=0 -UNUSABLE_KEPT="" -for p in $UNUSABLE_LISTED; do - grep -qxF "$p" "$ALLOWED" && continue - UNUSABLE=$((UNUSABLE + 1)) - UNUSABLE_KEPT="${UNUSABLE_KEPT}${p} " -done -if [ "$UNUSABLE" -gt 0 ]; then - note_failure "${UNUSABLE} file(s) report a whole-file parse failure: ${UNUSABLE_KEPT}" -fi + echo "==> indexing ${label} tree with $(basename "$BIN")" + "$BIN" cli index_repository --repo-path "$tree_root" --mode full --json \ + > "${prefix}.index.json" 2>"${prefix}.index.err" || { + echo "FAIL: index_repository exited non-zero for the ${label} tree" + tail -20 "${prefix}.index.err"; exit 1; } -# ── 2. No range list may be silently clipped ────────────────────────────── -# A trailing "+" says the producer's cap threw N ranges away. With the cap -# at 256 a file that still overflows is worth stopping for. -TRUNCATED="$(jq -r '[.structuredContent.parse_partial.files[]? - | select(.error_ranges? // "" | test("\\+[0-9]+$")) | .path] | join(" ")' \ - "${WORK}/status.json")" -for p in $TRUNCATED; do - grep -qxF "$p" "$ALLOWED" && continue - note_failure "$p carries a +N truncation marker — its range list was clipped" -done + project="$(jq -r '.structuredContent.project // empty' "${prefix}.index.json")" + [ -n "$project" ] || { echo "FAIL: index_repository did not name a ${label} project"; exit 1; } + + "$BIN" cli index_status --project "$project" --json > "${prefix}.status.json" 2>/dev/null || { + echo "FAIL: index_status exited non-zero for the ${label} tree"; exit 1; } + + status="${prefix}.status.json" + + for cls in parse_partial parse_unusable; do + clipped="$(jq -r --arg c "$cls" '.structuredContent[$c].truncated // false' "$status")" + if [ "$clipped" = "true" ]; then + note_failure "index_status clipped its ${cls} file list on the ${label} tree — the checks below would see only part of it" + fi + done + + # 1. Whole-file parse failures. + jq -r '.structuredContent.parse_unusable.files[]?.path' "$status" \ + | grep -vxF -f "$ALLOWED" 2>/dev/null | sort -u > "${prefix}.unusable" || : > "${prefix}.unusable" + + # 2. Range lists the producer's cap clipped, marked with a trailing "+". + jq -r '.structuredContent.parse_partial.files[]? + | select(.error_ranges? // "" | test("\\+[0-9]+$")) | .path' "$status" \ + | grep -vxF -f "$ALLOWED" 2>/dev/null | sort -u > "${prefix}.truncated" || : > "${prefix}.truncated" + + # 3. One range covering more than its share of the file. The line count + # comes from the tree being analysed, because a file can grow or shrink + # between the two commits. + : > "${prefix}.wide" + while IFS=$'\t' read -r path ranges; do + [ -n "$path" ] || continue + grep -qxF "$path" "$ALLOWED" && continue + [ -f "${tree_root}/${path}" ] || continue + total="$(wc -l < "${tree_root}/${path}" | tr -d ' ')" + [ "$total" -ge "$MIN_FILE_LINES" ] || continue + widest="$(printf '%s' "$ranges" | tr ',' '\n' | grep '^[0-9]' \ + | awk -F- '{d=$2-$1+1; if (d>m) m=d} END {print m+0}')" + pct="$(awk -v a="$widest" -v b="$total" 'BEGIN{printf "%.1f", 100*a/b}')" + over="$(awk -v p="$pct" -v lim="$MAX_SINGLE_RANGE_PCT" 'BEGIN{print (p>lim)?1:0}')" + if [ "$over" = "1" ]; then + printf '%s\t%s\t%s\t%s\n' "$path" "$widest" "$pct" "$total" >> "${prefix}.wide" + fi + done < <(jq -r '.structuredContent.parse_partial.files[]? + | "\(.path)\t\(.error_ranges // "")"' "$status") + sort -u -o "${prefix}.wide" "${prefix}.wide" + + # 4. The flagged-file count. + jq -r '.structuredContent.parse_partial.count // 0' "$status" > "${prefix}.count" +} -# ── 3. No single range may cover a quarter of its file ──────────────────── -while IFS=$'\t' read -r path ranges; do +analyze_tree "$BASE_TREE" "${WORK}/base" base +analyze_tree "$REPO_ROOT" "${WORK}/head" head + +BASE_COUNT="$(cat "${WORK}/base.count")" +HEAD_COUNT="$(cat "${WORK}/head.count")" + +# ── The four checks, each on what the branch ADDED ──────────────────────── +while read -r p; do + [ -n "$p" ] || continue + note_failure "$p reports a whole-file parse failure on this branch and not at the merge base" +done < <(comm -13 "${WORK}/base.unusable" "${WORK}/head.unusable") + +while read -r p; do + [ -n "$p" ] || continue + note_failure "$p carries a +N truncation marker on this branch and not at the merge base — its range list was clipped" +done < <(comm -13 "${WORK}/base.truncated" "${WORK}/head.truncated") + +# Compare by path, not by the whole row: a file already over the share at the +# merge base must not fail here just because the range moved by a line. +cut -f1 "${WORK}/base.wide" | sort -u > "${WORK}/base.wide.paths" +while IFS=$'\t' read -r path widest pct total; do [ -n "$path" ] || continue - grep -qxF "$path" "$ALLOWED" && continue - [ -f "${REPO_ROOT}/${path}" ] || continue - total="$(wc -l < "${REPO_ROOT}/${path}" | tr -d ' ')" - [ "$total" -ge "$MIN_FILE_LINES" ] || continue - widest="$(printf '%s' "$ranges" | tr ',' '\n' | grep '^[0-9]' \ - | awk -F- '{d=$2-$1+1; if (d>m) m=d} END {print m+0}')" - pct="$(awk -v a="$widest" -v b="$total" 'BEGIN{printf "%.1f", 100*a/b}')" - over="$(awk -v p="$pct" -v lim="$MAX_SINGLE_RANGE_PCT" 'BEGIN{print (p>lim)?1:0}')" - if [ "$over" = "1" ]; then - note_failure "$path has one range of ${widest} lines — ${pct}% of ${total}, over ${MAX_SINGLE_RANGE_PCT}%" - fi -done < <(jq -r '.structuredContent.parse_partial.files[]? - | "\(.path)\t\(.error_ranges // "")"' "${WORK}/status.json") - -# ── 4. The flagged-file count must not drift upward unnoticed ───────────── -# One awk, not `grep | head -1`: under `set -o pipefail` head closes the pipe -# early, grep dies of SIGPIPE, and the whole gate aborts with no message. It -# does not bite on today's short file, and it would bite the day someone adds -# a few comment lines. -CEILING="$(awk '{ sub(/#.*/, "") } match($0, /[0-9]+/) { print substr($0, RSTART, RLENGTH); exit }' \ - "$BASELINE_FILE")" -PARTIAL="$(jq -r '.structuredContent.parse_partial.count // 0' "${WORK}/status.json")" -if [ "$PARTIAL" -gt "$CEILING" ]; then - note_failure "parse_partial_count is ${PARTIAL}, above the ceiling ${CEILING} in $(basename "$BASELINE_FILE")" + grep -qxF "$path" "${WORK}/base.wide.paths" && continue + note_failure "$path has one range of ${widest} lines — ${pct}% of ${total}, over ${MAX_SINGLE_RANGE_PCT}%, and it was within the share at the merge base" +done < "${WORK}/head.wide" + +if [ "$HEAD_COUNT" -gt "$BASE_COUNT" ]; then + note_failure "parse_partial_count is ${HEAD_COUNT} on this branch against ${BASE_COUNT} at the merge base" +fi + +# ── Report ─────────────────────────────────────────────────────────────── +# parse-partial-baseline.txt is a written record, not a gate. It is printed so +# a reader can see the count drift, and nothing fails on it: enforcing it is +# what turned an unrelated branch red when main moved (#1972). +RECORDED="$(awk '{ sub(/#.*/, "") } match($0, /[0-9]+/) { print substr($0, RSTART, RLENGTH); exit }' \ + "$BASELINE_FILE" 2>/dev/null || echo "")" +echo "==> base ${BASE_SHA}: parse_partial=${BASE_COUNT} unusable=$(wc -l < "${WORK}/base.unusable" | tr -d ' ')" +echo "==> head: parse_partial=${HEAD_COUNT} unusable=$(wc -l < "${WORK}/head.unusable" | tr -d ' ')" +echo "==> allowlisted=$(wc -l < "$ALLOWED" | tr -d ' ') recorded=${RECORDED:-none} (record only, not enforced)" +if [ -n "$RECORDED" ] && [ "$HEAD_COUNT" != "$RECORDED" ]; then + echo "NOTE: parse_partial_count is ${HEAD_COUNT}, and $(basename "$BASELINE_FILE") records ${RECORDED}." + echo "NOTE: that is not a failure. If this branch changed the extractor, both sides above moved together and this gate cannot tell." fi -echo "==> parse_partial=${PARTIAL} (ceiling ${CEILING}) parse_unusable=${UNUSABLE} allowlisted=$(wc -l < "$ALLOWED" | tr -d ' ')" if [ "$FAILURES" -gt 0 ]; then echo "FAIL: ${FAILURES} coverage-gate check(s) failed" exit 1 fi -echo "PASS: parse-coverage report is within bounds" +echo "PASS: this branch did not make the parse-coverage report worse than the merge base" diff --git a/scripts/test.sh b/scripts/test.sh index 0aafe6c20..0f4d31dfc 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -272,6 +272,9 @@ bash "$ROOT/tests/test_language_count_contract.sh" echo "=== Step 0x: packaging version-metadata contract ===" bash "$ROOT/tests/test_version_metadata_contract.sh" +echo "=== Step 0y: parse-coverage gate contract (#963) ===" +bash "$ROOT/tests/test_coverage_gate_contract.sh" + # Verify compiler supports target arch verify_compiler "$CC" diff --git a/tests/test_coverage_gate_contract.sh b/tests/test_coverage_gate_contract.sh new file mode 100755 index 000000000..8755fbfe5 --- /dev/null +++ b/tests/test_coverage_gate_contract.sh @@ -0,0 +1,241 @@ +#!/usr/bin/env bash +# Contract: scripts/ci/self-index-coverage-gate.sh fails a branch only for a +# finding the branch ADDED (#963, #1972). +# +# The gate used to compare against a number checked into the repository. Main +# could move that number on its own, and every open pull request went red for +# a reason none of them caused. The gate now indexes the merge base and the +# branch with the same binary and compares the two. This test pins that. +# +# No seam is added to the production script. It calls the binary as +# "$BIN cli index_repository …" and "$BIN cli index_status …", so a fake $BIN +# that prints canned JSON exercises every check without indexing anything. +# Same idea as tests/repro/repro_script_summary.sh. +set -euo pipefail +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +WORKDIR="$(mktemp -d)" +cleanup() { + # Each gate run makes its own worktree and removes it. A run that dies + # early can leave a registration behind, so prune before deleting. + git -C "$REPO" worktree prune >/dev/null 2>&1 || true + rm -rf "$WORKDIR" +} +REPO="$WORKDIR/repo" +trap cleanup EXIT +fail() { + echo "FAIL: $*" >&2 + exit 1 +} + +FIXTURE="$WORKDIR/fixture" +mkdir -p "$FIXTURE" + +# ── A repository with a base commit and a head commit ───────────────────── +mkdir -p "$REPO/scripts/ci" "$REPO/src" +cp "$ROOT/scripts/ci/self-index-coverage-gate.sh" "$REPO/scripts/ci/" +printf '# no allowlisted paths\n' > "$REPO/scripts/ci/coverage-gate-allowlist.txt" +printf '# recorded count\n7\n' > "$REPO/scripts/ci/parse-partial-baseline.txt" +# Check 3 measures a range against the file's own length, and exempts +# anything under 200 lines. These two are long enough to be measured. +for f in big allowed; do + awk 'BEGIN { for (i = 1; i <= 300; i++) print "int line_" i ";" }' > "$REPO/src/$f.c" +done +git -C "$REPO" init -q +git -C "$REPO" config user.email "gate@test.invalid" +git -C "$REPO" config user.name "Gate Test" +git -C "$REPO" config commit.gpgsign false +# The fake binary reads this file to tell which tree it was pointed at. +printf 'base\n' > "$REPO/.side" +git -C "$REPO" add -A +git -C "$REPO" -c core.hooksPath=/dev/null commit -qm "base" +BASE_SHA="$(git -C "$REPO" rev-parse HEAD)" +printf 'head\n' > "$REPO/.side" +git -C "$REPO" add -A +git -C "$REPO" -c core.hooksPath=/dev/null commit -qm "head" + +# ── The fake binary ─────────────────────────────────────────────────────── +# index_repository answers with the tree's own side as the project name, so +# the index_status call that follows can be answered for the right side. +FAKE_BIN="$WORKDIR/fake-cbm" +cat > "$FAKE_BIN" <<'FAKE_EOF' +#!/usr/bin/env bash +set -euo pipefail +sub="" +repo_path="" +project="" +while [ $# -gt 0 ]; do + case "$1" in + index_repository | index_status) sub="$1" ;; + --repo-path) repo_path="$2"; shift ;; + --project) project="$2"; shift ;; + esac + shift +done +case "$sub" in +index_repository) + side="$(tr -d '[:space:]' < "${repo_path}/.side")" + printf '{"structuredContent":{"project":"%s"}}\n' "$side" + ;; +index_status) + cat "${GATE_FIXTURE_DIR}/${project}.json" + ;; +*) + echo "fake-cbm: unexpected call" >&2 + exit 1 + ;; +esac +FAKE_EOF +chmod +x "$FAKE_BIN" + +# ── One gate run ────────────────────────────────────────────────────────── +# $1 case name, $2 expected outcome (pass|fail), $3 base JSON, $4 head JSON. +run_case() { + name="$1" + expect="$2" + printf '%s\n' "$3" > "${FIXTURE}/base.json" + printf '%s\n' "$4" > "${FIXTURE}/head.json" + out="$( + COVERAGE_GATE_BASE_SHA="$BASE_SHA" \ + GATE_FIXTURE_DIR="$FIXTURE" \ + bash "$REPO/scripts/ci/self-index-coverage-gate.sh" "$FAKE_BIN" 2>&1 + )" && rc=0 || rc=$? + if [ "$expect" = "pass" ] && [ "$rc" -ne 0 ]; then + printf '%s\n' "$out" >&2 + fail "${name}: expected the gate to pass, it exited ${rc}" + fi + if [ "$expect" = "fail" ] && [ "$rc" -eq 0 ]; then + printf '%s\n' "$out" >&2 + fail "${name}: expected the gate to fail, it exited 0" + fi + LAST_OUT="$out" + echo "ok: ${name}" +} + +# ── JSON builders ───────────────────────────────────────────────────────── +# $1 partial count, $2 partial files array, $3 unusable files array, +# $4 partial truncated flag. +status() { + cat < base ${BASE_SHA}: parse_partial=40"*) ;; +*) fail "the base side was not printed" ;; +esac +case "$LAST_OUT" in +*"==> head: parse_partial=40"*) ;; +*) fail "the head side was not printed" ;; +esac + +# ── A clipped file list still stops the run outright ────────────────────── +# This check cannot be differential. A clipped list means the three list-based +# checks above saw only part of the tree and would print PASS anyway. +run_case "a clipped file list at head fails" fail \ + "$(status 3 "$NARROW" "$NO_FILES")" "$(status 3 "$NARROW" "$NO_FILES" true)" +case "$LAST_OUT" in +*"clipped its parse_partial file list on the head tree"*) ;; +*) fail "the clipped-list failure did not name the tree" ;; +esac + +# ── The allowlist skips a path on both sides ────────────────────────────── +printf '# a written reason belongs above each path\nsrc/allowed.c\n' \ + > "$REPO/scripts/ci/coverage-gate-allowlist.txt" +git -C "$REPO" add -A +git -C "$REPO" -c core.hooksPath=/dev/null commit -qm "allowlist src/allowed.c" +run_case "an allowlisted path newly over the share passes" pass \ + "$(status 3 "$NARROW" "$NO_FILES")" "$(status 3 "$WIDE_ALLOWED" "$NO_FILES")" + +# ── The base commit has to be resolvable ────────────────────────────────── +printf '%s\n' "$(status 3 "$NARROW" "$NO_FILES")" > "${FIXTURE}/base.json" +printf '%s\n' "$(status 3 "$NARROW" "$NO_FILES")" > "${FIXTURE}/head.json" +out="$( + COVERAGE_GATE_BASE_SHA="0000000000000000000000000000000000000000" \ + GATE_FIXTURE_DIR="$FIXTURE" \ + bash "$REPO/scripts/ci/self-index-coverage-gate.sh" "$FAKE_BIN" 2>&1 +)" && rc=0 || rc=$? +[ "$rc" -ne 0 ] || fail "an unreachable base commit must not pass" +case "$out" in +*"base commit"*) ;; +*) fail "an unreachable base commit did not say so: $out" ;; +esac +echo "ok: an unreachable base commit stops the run" + +# ── The interface contract ──────────────────────────────────────────────── +out="$(bash "$REPO/scripts/ci/self-index-coverage-gate.sh" --help 2>&1)" || fail "--help exited non-zero" +case "$out" in +*"Usage:"*) ;; +*) fail "--help printed no Usage: block" ;; +esac +case "$out" in +*"merge base"*) ;; +*) fail "--help does not describe the merge-base comparison" ;; +esac +echo "ok: --help describes the merge-base comparison" + +echo "PASS: coverage-gate contract"