Should CI gate this repo's own parse-coverage report against regression?

[CaptainMittens]

What problem does this solve?

This repository's own parse-coverage report can get worse without anyone noticing. Nothing
in CI reads it, so a change that widens a parse-error range, or drops a region from the
index, lands green. The coverage work tracked in #963 improved the report itself, but the
report is only useful if a regression in it is caught at the time it happens rather than
months later.

I hit this while working on the report: I could not tell whether a number I was looking at
was a regression or had always been that way, because no run had ever recorded a baseline.

Proposed solution

Add a CI check that reads this repository's own parse-coverage report and fails a pull
request when it goes backwards against a recorded ceiling.

The reason this needs a decision from you rather than a pull request is that it adds a
required check to pr-smoke, and CONTRIBUTING.md line 128 puts that class of change
behind a design discussion. There are real trade-offs:

• A ceiling has to move when main legitimately changes. It already did once: the ceiling
  rose from 58 to 59 because main added src/daemon/runtime.c:47, whose function-style
  _Atomic(type) the C grammar cannot parse. That rise came from main, not from the
  branch under test, and a contributor hitting it would have no way to tell.
• A required check that can fail for reasons outside the contributor's change is a tax on
  every pull request, not only on the ones that touch coverage.

So the question is whether you want the gate at all, and if so whether it should be
required or advisory to begin with.

Alternatives considered

• Advisory only. Report the number on each pull request without failing. Catches the
  regression in review rather than automatically, and costs nothing when main shifts.
• Nightly rather than per-PR. Removes the tax from pull requests entirely and still
  catches a regression within a day, at the cost of not naming the change that caused it.
• Do nothing. The report stays useful for a person who reads it, and silently drifts
  otherwise.

I have an implementation ready either way — it is currently open as a pull request, split
out of the parse-coverage fix at review request precisely because it is a maintainer
decision rather than part of a bug fix. Happy to close that and keep only whichever shape
you prefer, or none.

Confirmations

• I searched existing issues and this is not a duplicate.

[DeusData]

Thank you for bringing the required-check policy here instead of hiding it in the implementation. The maintainer decision is now recorded on #1968: the gate is accepted in principle as a required check once merge-base comparison is included, so unrelated pull requests do not fail merely because main moved the baseline. Leaving this issue open and linked until the implementation lands.

[CaptainMittens]

The merge-base comparison you set as the precondition is in #1968 now.

Both trade-offs you named in the issue are gone, and they were the same trade-off. The gate no longer reads a recorded ceiling. It resolves the base commit, checks it out into a temporary worktree, indexes that tree and the branch with the same binary, and fails only on a finding present on the branch and absent at the base. That covers all four checks, not just the count:

#	Check	Fails when
1	whole-file parse failure	the path is flagged at head and not at the base
2	a +N clipping marker	the marker is new at head
3	one range over 25% of a file of 200+ lines	the file was within the share at the base
4	parse_partial_count	it is above the base's count

So the 58 -> 59 rise from src/daemon/runtime.c:47 would now move both sides together and fail nothing. Same for #1824 — Blazor .razor maps to C#, the markup lands in ERROR regions by design, and the count rises by roughly the repository's .razor count. That is a coverage improvement, and under the old shape it read as a failure on whatever branch happened to open next.

scripts/ci/parse-partial-baseline.txt stops being a gate. The script still prints the recorded number so a reader can see the drift, and says in its own output that nothing fails on it. Nobody has to raise that number again.

Every run prints both sides, so a failure reads as "the base reports X, this branch reports Y":

==> base 5fbab7bb7332bd06aaa880653ddfb2696e648f90: parse_partial=59 unusable=0
==> head:            parse_partial=59 unusable=0
==> allowlisted=1 recorded=59 (record only, not enforced)
PASS: this branch did not make the parse-coverage report worse than the merge base

One limitation, stated in the script header, the scripts/ci/README.md row and the pull request: both trees use the same binary, so a branch that changes the extractor itself moves base and head together and this gate will not fail on it. Catching that needs the base commit's own binary, which is a second full build — 11m49s in the current job against the 26s the whole gate step takes. I did not want to spend that silently, so I did not spend it. The FLOOR in tests/test_index_resilience.c still stops the signal being switched off, and both absolute counts now print on every run, so a jump is visible in the log even when it does not fail. If you would rather have the second build, say so and I will add it.

The cost went from one index to two: 45s locally for both, against 26s for the single index in job 99872656120. The build step in that same job took 11m49s, so the gate is still a rounding error next to it.

Whether it lands required or advisory is still yours to call, and #1968 changes nothing about that — pr.yml runs it as an ordinary step, so making it non-blocking is a continue-on-error line whenever you want it.