test: compute the per-file coverage table from the tracefile (#974)

[OffgridwithJD]

Closes #974. The nightly's per-file coverage table named the best-covered files as the worst, and the table is now computed from the tracefile instead of parsed out of lcov --list.

what the nightly printed:  columnar_parquet_codec.c | 2.0%  100|3200%  2|   -    0
what its records say:      LF:100 LH:100  FNF:2 FNH:2  BRF:64 BRH:47
what this prints:          columnar_parquet_codec.c  100.0% 100/100  100.0% 2/2  73.4% 47/64

A file at 100% presented as 2.0% and sorted to the top of a list headed least covered first. A function rate above 100% on its face. An empty branch column where the tracefile carries 13,900 branches. Four of the files it named as least covered were between 94% and 100%.

Why, as far as I established it

The table came from lcov --list. The same step's lcov --summary, four lines earlier on the same tracefile, was correct — and so is lcov --list run here on the run's own uploaded coverage.info. The two builds differ only in distro patch level: the runner installs 2.0-4ubuntu2, this was checked on 2.0-1.

I did not bisect which patch does it and I am not guessing. The point of the change is that the output was wrong and the input was fine, so the fix is to stop depending on the part that varies.

What replaces it

test/pgc_coverage_table.py reads LF/LH, FNF/FNH, BRF/BRH and divides. Those counters are stated outright in the tracefile and cannot be got wrong by a patch to --list.

Verified two ways, not one:

files: generator 39, lcov 39
disagreements vs lcov --list (2.0-1):   0
disagreements vs the raw LF/LH records: 0

Checking against another tool alone would only prove the two agree; checking against the raw counters proves the arithmetic.

lcov --summary is untouched. It was right on both builds and it is what the summary block uses.

Two deliberate choices about the format

hit/found, not just the total. 93.7% 22765 cannot be checked by a reader; 93.7% 21320/22765 can. A reader who can check the number is the only one who will notice when it is wrong again — which is the whole reason this sat in the log for two nights.

An absent counter prints -, never 0.0%. A header with no branches shown at 0.0% sorts to the top and reads as the least covered file in the tree. That is how a table misleads while every individual number in it is defensible.

Twelve arms, and four of them were wrong first

They drive the generator over a synthetic tracefile rather than grepping for the call, because a static check that the runner invokes the right script cannot tell whether the script is correct, and "the table is wrong" was the defect. The fixture carries one partially covered file, one at 100%, and one with no branch counter at all.

Four failed when first written, each the arm's fault and not the generator's:

what failed	why
no lcov --list survives → got 1	the match was my own comment explaining the replaced call
lcov --summary stays → wanted 1, got 2	the real call plus my comment again
an absent counter prints a dash	asserted $6; absent cells collapse to one token each, so they are $4 and $5
never reads as 0.0% → got 1	0\.0% is a substring of 50.0%, the line rate of the no-branch fixture

The first two are the third instance today of a comment carrying a token tripping a grep that wanted the call — a pgc_summary mention in #969 and a shellcheck mention in #972 were the others. They now count over code with comments stripped, under a premise that stripping comments did not strip the code, since a grep over an emptied file reports the same 0 as a grep that matches nothing.

The fourth is anchored to (^|[^0-9.])0\.0%, with a control: a genuinely zero-covered fixture still matches, so the anchor narrowed the pattern without defeating the assertion.

Gate

harness_selftest   rc=0  815 checks  0 FAIL   (803 on main; +12 are these arms)
docs_style         rc=0  9 checks    0 FAIL
shellcheck -S warning   clean on both changed shell files
bash -n            both parse
ledger             12 rows added, census DERIVED 846 -> 858
  census stated 858, ledger holds 858: they agree
  coverage registered=252 | covered=2, not covered=250, ceiling=250
  gate rc=0

The registered list came from bash test/run_all_versions.sh --list-suites (252, clean), not from a grep of the SUITES array — four of my extractions of that array returned 560, 333, 284 and 267 names, because its closing paren is not the first ^) after it.

One thing whoever merges needs to know

#972 moves checks_never_observed_red on the same line. Whichever of the two merges second needs a rebase and the census re-derived on the other's value, not added to it. #972 takes it to 847; this one takes 846 to 858; the correct result after both is whatever awk -F'\t' '$4=="never"' | wc -l says on the merged ledger, which is the only way that number has ever been right on purpose.

Closes #974.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

[OffgridwithJD]

Rebased onto 94295d81 after #972 merged, so this is the PR that paid the ledger tax. New head c302c670.

The sequence, which is exactly what I predicted on #432 two hours ago and is worth having on the record as the measured cost rather than the estimate:

1  drop my earlier ledger commit      it derived the census against 846, now wrong
2  rebase onto main                   CHANGELOG conflicted with #972's entry
3  re-run harness_selftest            on the REBASED tree, not the old log
4  guard that log                     rc=0, numeric total above a floor, 0 FAILs, PASSED
5  merge it
6  DERIVE the census from the file
7  re-run CI

The derived value is 859, which happens to equal main's 847 plus my twelve — and it is not set that way. Had I written 859 by addition it would have been right today and wrong the first time two ledger-touching PRs land in an order I did not predict. That has already happened once today: I derived against 846, #972 landed first, and the number I had was stale before it was reviewed.

ledger census: rows=859 | never observed red=859, ever red=0, new this run=0
  census stated 859, ledger holds 859: they agree
ledger coverage: registered=252 | covered=2, not covered=250, ceiling=250
gate rc=0
harness_selftest: rc=0, checks run: 816, 0 FAIL

816 rather than 815, because the rebase brought in #972's new premise check as well as my twelve.

The CHANGELOG conflict

#972's entry is on main where mine inserts. Both kept, in landing order, with each asserted to appear exactly once in the result — the identity check rather than an eyeball, because a fold loses one silently and the file is long enough that nobody would notice.

Unchanged by the rebase

test/pgc_coverage_table.py, the runner change, and all twelve arms are the same as at bfd4b057. The only difference is the base, the CHANGELOG position, and the census value.

Two PRs, thirteen checks, fourteen steps of tax

That is the number #432 was missing. Not an argument against seeding — an argument that the ledger's key shape has to be settled before 240 suites pay this on every PR, because changing it afterwards means regenerating 240 suites' rows while everyone in the meantime pays the tax against a key already known to be wrong.

[OffgridwithJD]

I ran run_coverage.sh end to end, because nothing in the PR gate runs it — and a mistake in this PR would land as a nightly-only failure, which is the exact class the last two PRs were about.

That gap is worth naming plainly: #975 changes the script the nightly's coverage job executes, and the thirteen checks on this PR do not execute it. The twelve arms in 250 drive the generator and assert the call site, which is the right shape, but neither runs the script. So I ran it.

PGC_COV_OUT=/tmp/covrun PGC_COV_SUITES=smoke bash test/run_coverage.sh /usr/local/pg18a/bin/pg_config

run_coverage rc=0
-- suites: 1 passed, 0 failed, 0 skipped
coverage.info   492326 bytes
html/index.html   6327 bytes
over-100% cells in the table:  0

and the table, on a tracefile this run produced rather than one I downloaded:

-- per file, least covered first
  file                              lines                 functions             branches
  columnar_compat.h                    0.0%          0/15    0.0%           0/1    0.0%           0/6
  columnar_parquet_codec.c             0.0%         0/100    0.0%           0/2    0.0%          0/64
  columnar_parquet_format.h            0.0%           0/5       -                  0.0%          0/26
  columnar_iceberg_fdw.c               1.1%         8/758    8.8%          3/34    0.2%         1/588
  (19 further file(s) not shown, all covered at least as well as the last row)

The coverage is low because only smoke ran, which is the point of PGC_COV_SUITES — it reaches the table step without paying for the full corpus.

Two things that only an end-to-end run could show:

columnar_parquet_format.h prints - for functions, because that file has no function counter at all. That is the absent-counter case the arms assert, occurring naturally rather than in a fixture I built to produce it. Under lcov --list's column layout it would have been a 0.0% sorting to the top of a list headed least covered.

And python3 is not a new dependency: the coverage job already installs python3-pip and already runs python3 -c 'import pyarrow...' as a step, so the interpreter is present where this runs. I checked rather than assumed, because adding a silent dependency to a nightly-only script is how this kind of thing breaks.

The prefix, because an instrumented build is a write to shared state

The run installs a --coverage build into /usr/local/pg18a, so the restore was in the same script rather than in my memory:

pg18a .so before: 196f8f07445d
pg18a .so after:  bdb86145faf1   (rebuilt from main)
smoke rc=0, prefix usable

Gate, unchanged

13/13 at c302c670. The end-to-end run above is extra evidence, not a new head — no code moved.

[jdatcmd]

Approved at c302c670. Driven on a tracefile carrying the exact record you cite, not read.

The defect and the fix, reproduced

Fed the generator LF:100 LH:100 FNF:2 FNH:2 BRF:64 BRH:47 — the record the nightly rendered as 2.0% 100 | 3200% 2:

file                          lines            functions        branches
zero.c                          0.0%    0/50     0.0%    0/4      -
half.c                         50.0% 100/200    50.0%   5/10      -
columnar_parquet_codec.c      100.0% 100/100   100.0%    2/2   73.4%  47/64
nobranch.c                    100.0%   10/10   100.0%    1/1      -

A file at 100% reads as 100%, and it sorts to the bottom of a list headed "least covered first" rather than the top. The hit/found format is the part I'd defend hardest: 93.7% 22765 cannot be checked by a reader and 93.7% 21320/22765 can, and this whole issue exists because nobody checked.

Edges I attacked

LF:0 LH:0                 -        no division by zero
no counters at all        -
LF:notanumber             -        the int() guard holds
empty tracefile           rc=1, "no SF records ... nothing to rank"
missing file              rc=1
a file with no branches   -        NOT 0.0%, so it does not sort to the top

That last one is the quiet correctness in the change: - and 0.0% are different facts, and conflating them would put every branch-free file at the head of the worst-covered list.

One finding, non-blocking, and it is the PR's own argument turned on itself

An impossible rate is rendered as excellent coverage and sorted out of sight.

SF:/src/d_impossible.c
LF:2
LH:7

a_worst.c           1.0%     1/100
b_mid.c            50.0%    50/100
c_good.c           99.0%    99/100
d_impossible.c    350.0%       7/2     <- last, i.e. "best covered in the tree"
e_nodata.c             -

350.0% 7/2 is not a coverage figure; hit > found is arithmetically impossible. The table prints it without comment and ranks it below a genuinely 99%-covered file.

The nightly's 3200% is exactly this signal, and it went unread for however long the table has been wrong. At the top of the list it was missed; at the bottom it would be missed more surely. Your docstring makes the case better than I can — "a reader who can check the number is the only one who will notice when it is wrong again" — and a reader cannot check 7/2 against anything, because it is not wrong by a little.

Cheap fix if you want it: treat hit > found as a distinct state, print it as impossible or suffix it, and sort it with the worst rather than the best. I would not hold the PR for it; a corrupt tracefile is not reachable from lcov's own writer, which is why this is a finding and not a blocker.

The --list removal, counted over code

Your own trap, checked the way you taught it two hours ago:

--list, whole file    1     <- the comment explaining the removal
--list, code only     0
--summary, code       1     correct on both builds, so it stays
pgc_coverage_table.py 1     wired in

Your four faults are the reason I trust the twelve arms

Two arms matched your own comment rather than the call, one asserted the wrong field because absent cells collapse to one token, and the fourth was the substring trap — 0\.0% matching inside 50.0%, which was the line rate of your own fixture. You then ran a control with a genuinely zero-covered file to prove the anchor had not defeated the assertion.

An anchor is an instrument and needs its own negative case. That control is the part most people skip, and it is the difference between a tightened pattern and a broken one.

I could not reproduce your 39-file cross-check here — lcov --summary produces nothing against a synthetic tracefile whose SF: paths do not exist, and the container carries 2.0-1, the build that was already correct. Yours was done against the nightly's own uploaded tracefile, which is the right artifact, and the arithmetic is independently checkable from the records regardless.