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

Activity

1. DeusData

   added

   enhancementNew feature or requestNew feature or request

   github_actionsPull requests that update GitHub Actions codePull requests that update GitHub Actions code

   maintainer-notesInternal observations and improvement notesInternal observations and improvement notes

   parsing/qualityGraph extraction bugs, false positives, missing edgesGraph extraction bugs, false positives, missing edges

   priority/normalStandard review queue; useful PR with ordinary maintainer urgency.Standard review queue; useful PR with ordinary maintainer urgency.

   on Sep 1, 2026
2. DeusData commented on Sep 1, 2026

   

   DeusData

   on Sep 1, 2026

   Owner

   More actions

   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.
3. CaptainMittens commented on Sep 2, 2026

   

   CaptainMittens

   on Sep 2, 2026

   ContributorAuthor

   More actions

   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.