Skip to content

feat(compliance): report how many files each run examined - #11

Merged
sakanni merged 4 commits into
developfrom
feat/compliance-coverage-denominator
Aug 25, 2026
Merged

feat(compliance): report how many files each run examined#11
sakanni merged 4 commits into
developfrom
feat/compliance-coverage-denominator

Conversation

@sakanni

@sakanni sakanni commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Problem

A compliance run that examined every file it was handed and a run that examined none produce the
same output and the same green check. There is no number anywhere that distinguishes them.

Both runners walk the same loop with four exits:

foreach (var file in files)
    if (!FileFilter.IsRelevantFile(file, checkType)) continue;   dropped, silently
    if (!File.Exists(file))          { "[SKIP] File not found";  } dropped, prints
    if (resultForThisFile == null)   { "[SKIP] No result";       } dropped, prints
    mergedResult = mergedResult.Merge(resultForThisFile);          examined

Three of the four are drops, and the first prints nothing at all — not even under --output console. That one matters most, because it is reachable from a pattern/filter disagreement: a
file can be selected by the changed-file pattern, counted into the decision not to skip the
check, handed to the runner, and then rejected by the runner's own filter. *AssemblyInfo.cs
selects Properties/NotAssemblyInfo.cs; the project filter requires the name to equal
AssemblyInfo.cs exactly. The check reports success having inspected nothing, and says so
nowhere.

What changed

A denominator, from both runners. Coverage: 1 of 4 file(s) examined; 1 not relevant to this check, 1 not found on disk, 1 returned no result. Reasons that did not occur are omitted rather
than listed at zero, so the one that fired is not buried.

A warning when files were handed in and none was examined, carrying the breakdown rather
than the bare fact. 3 file(s) were handed to this check and none was examined: 3 not relevant to this check. The check reports success without having inspected anything. If the files were dropped as not relevant, the pattern that selected them and the filter that rejected them disagree. The bare fact does not distinguish a pull request that changed nothing relevant from a
selection layer handing over files the check will never accept; the breakdown does.

Both runners, because both have the identical loop. Instrumenting one would be an arbitrary
asymmetry, and the gap would be found again later.

Where the counts go, and where they deliberately do not

Surface Gets Why
console, github the Coverage: line human-facing
json, sarif counts inside the payload, no line these put a payload on stdout and nothing may precede it
job summary the count, alongside pass/fail see below

The last row is the one worth explaining. The action wraps the runner in ::group::, which the
GitHub UI collapses by default, so a number printed only to stdout is invisible to a reader
who is not already suspicious. The summary is the surface people actually read, and it previously
carried one line saying only whether the check passed. The action now reads the count off the one
line the runner controls and puts it there. Both ends of that contract are asserted, so renaming
the line on one side fails a test rather than silently dropping the count.

What is deliberately not done

Drop reasons are counted, not sub-classified. A file dropped as not relevant may have been
excluded on purpose — project compliance deliberately skips test projects and anything under
.ci/ — or may be a pattern/filter disagreement. Telling those apart needs the filter to explain
itself, which is a change to a class both runners share. Making the ratio visible and diagnosing
it are separate jobs, and this is the first.

Nothing fails that did not fail before. No exit code changes, no merged status changes. The
question of whether examining nothing should fail is untouched and still open. This reports a
number and decides nothing, which is why it does not need that question answered first.

Verification

80 tests, 9 of them on arithmetic and wording, in the hermetic project. The accounting is a
separate class rather than counters inlined in each entry point: those entry points compile
against BHoM types, so inlined counters could only be checked by a full CI run against a real
dependency closure. Extracted, they are tested on a bare runner in milliseconds. The wording is
asserted as well as the arithmetic, because that text is the deliverable — it is what a reader
sees when a check reports success having examined nothing — and it should change by a deliberate
edit to a test, not by drift.

The machine-readable property is pinned, not merely avoided. A test parses the json payload
with coverage present and asserts the Coverage: line is absent from stdout. If the gating is
ever removed, that test fails rather than a downstream consumer.

The preceding commit characterises the old behaviour, so the diff shows the change
assertion by assertion rather than only as new code. Three pre-existing tests asserted the
silence this removes and were updated in their own commit rather than quietly: two of them
claimed a passing run produces no annotation lines, but asserted something stronger — that
stdout was entirely empty — and that emptiness was the defect.

Confirmed where the text lands, not only that it was written. Running the check for real on a
pull request whose only changed file is selected by the pattern and rejected by the filter:

  • the run carries a warning annotation titled Compliance coverage, visible without expanding
    anything, naming the counts and the likely cause;

  • the job summary reads

    project compliance passed

    Coverage: 0 of 1 file(s) examined; 1 not relevant to this check.

That is item-3's reachable case reporting itself. An earlier iteration of this branch put
Files examined: 0 of 1 file(s) examined in the summary, which was found by reading the rendered
output rather than the code, and is why the summary now takes the runner's line verbatim.

What to look at

  • FileAccounting.CoverageLine() and ExaminedNothingWarning(). That wording is the deliverable.
  • The format gate in OutputEmitter. Removing it would put a line ahead of a payload.
  • The count reconciliation test: if the four counters ever stop summing to the input, the
    denominator is lying and every number above it is unreadable.

Both compliance runners walk the files they are handed through the same four-exit
loop: dropped by the filter, absent from disk, no result returned, or examined.
Three of those are drops and one of the three is completely silent. Neither runner
reports how many files it examined, so a run that checked every file and a run that
checked none produce the same output and the same green check.

Characterisation tests, written to pass against today's behaviour so the absence is
recorded rather than described. Each says what it should become, in the same style
as the entry-point tests beside them.

Five assertions on the runners: no coverage line from either runner when nothing is
examined, no output at all when the filter discards a file, and no counts in the
json payload for a machine-readable consumer. The filter-discard case uses a name
that a '*AssemblyInfo.cs' pathspec selects and the project filter then rejects,
which is the shape that produces a green check having inspected nothing.

Two on the action: the runner invocation is inside a collapsed group, so anything
printed there is hidden by default, and the job summary carries only pass or fail.
The first is asserted because the second's reasoning depends on it.
A compliance run that examined every file it was handed and one that examined none
produced the same output and the same green check. Both runners now report a
denominator, so a green can be read as evidence rather than taken on trust.

The accounting is a separate class rather than counters inlined in each entry
point. Those entry points compile against BHoM types, so inlined counters could
only be checked by a full CI run against a real dependency closure; extracted, the
arithmetic and the wording are tested directly on a bare runner. It holds counters
and formats strings and decides nothing.

Both runners walk the same four-exit loop, so both are instrumented. Leaving one of
two identical loops uninstrumented would be an arbitrary asymmetry and the gap
would be found again later.

Where the counts go, and where they deliberately do not:

  console and github get a Coverage line naming files examined out of files handed
  in, plus each drop reason that actually occurred. Reasons that did not occur are
  omitted rather than listed at zero, so the one that fired is not buried.

  json and sarif get the counts inside the payload. They must not get the line:
  those formats put a payload on stdout and nothing may precede it. A test asserts
  the payload parses with coverage present, so the gating is held in place rather
  than merely observed to work once.

  The job summary gets the count, read off the one line the runner controls. The
  runner's own output sits inside a log group the UI collapses by default, so a
  number printed only there is invisible to a reader who is not already suspicious.
  Both ends of that contract are asserted.

When files were handed in and none was examined, a warning names the breakdown
rather than the bare fact. The breakdown is what separates a pull request that
changed nothing relevant from a selection pattern handing over files the check will
never accept: a file can be selected by the changed-file pattern, counted into the
decision not to skip, handed over, and then rejected by the runner's own filter.

Drop reasons are counted but not sub-classified. A file dropped as not relevant may
have been excluded on purpose, since project compliance deliberately skips test
projects and anything under .ci/. Telling those apart would require the filter to
explain itself, which is a change to a class both runners share. Making the ratio
visible and diagnosing it are separate jobs.

Reporting only. No exit code changes, no merged status changes, and the question of
whether examining nothing should fail is untouched and still open.
Three pre-existing tests asserted the silence this change removes.

ExaminedCount_IsNotReported recorded that no count existed and said adding one was
the cheapest partial mitigation. The count now exists, so the assertion is inverted
and renamed. It still does not touch the pass-versus-fail question, which is what
that test was careful to say and is still open.

The two GitHubOutput_ForPassingRun_ProducesNoAnnotationLines tests, one per runner,
asserted that stdout was entirely empty for a run whose only file was filtered out.
That is a stronger claim than their names make, and the silence it protected was
the defect: a run that examined nothing looked exactly like a run that examined
everything. They now assert what the name means, that a passing run produces no
finding annotations, and additionally that it reports the zero it examined. Renamed
to match.
The summary reworded the runner's line and produced 'Files examined: 0 of 1
file(s) examined', which reads badly and put the same sentence in two places where
it could drift. The runner owns the wording; the action owns where it appears.
@sakanni
sakanni merged commit f64d528 into develop Aug 25, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant