Close the 149 lines the coverage gate never saw

[gaborbernat]

Problem

The native coverage gate has never enforced LLVM's total. It runs with --fail-uncovered-lines 0 and no --fail-under-lines, while the frontend gate does use --fail-under-lines 100. PR #2162 adds the missing enforcement.

With it applied, the workspace does not pass:

regions    435171   missed 4661   98.93%
functions   30480   missed   45   99.85%
lines      265382   missed  149   99.94%

So 149 lines and 45 functions are uncovered today and the existing gate is blind to all of them. That blindness has a mechanism, established in #2162: when a closure shares a physical line with its covered call, cargo-llvm-cov drops that line from the Uncovered Lines list while LLVM keeps counting it missed. A file can therefore have missed lines and print nothing under Uncovered Lines, which is exactly what --fail-uncovered-lines 0 inspects.

This issue is the remediation. #2162 is the gate, and the two are deliberately separate so a large clean-up does not ride along inside a CI change.

Required change

Bring the workspace to LLVM's 100% on lines and functions, so #2162 can land green.

Locating the offenders needs the technique from #2162, because LLVM does not expose these lines directly (llvm-project#126307). The workable locator is a file with missed lines but nothing listed under Uncovered Lines. #2162's report step names files, so start from its output rather than guessing.

Read each one before covering it. An uncovered line is a symptom. It means the code is dead, or a path someone assumed exists does not, or an error arm skips work that should have happened first — and only the first is a delete-it case. Writing a test to turn a line green without answering that question buries a defect under a passing gate. Two lanes found real bugs in their own new code exactly this way.

Also expect that some of the 149 are genuinely one-per-file and unrelated to each other. Batch them per crate rather than per line, and say in each pull request which files it closed.

Acceptance criteria

• just coverage-native with --fail-under-lines 100 passes on x86_64-linux.
• Every line closed is closed by a test that fails if the behaviour changes, not by a test that merely executes it.
• Every line deleted rather than tested is named with the reason it was unreachable.
• No #[coverage(off)], no threshold lowered, no test excluded.

Boundary

Coverage of existing code. Not a behaviour change — if closing a line reveals a defect, file that separately and say so, rather than fixing it silently inside a coverage pass. The gate itself is #2162 and must not be weakened here to make this easier.

Note the coverage gate is x86_64-linux only, so any claim about totals carries that qualifier.

Correction: these are mostly not dead code

This issue originally said to expect a material share of the 45 missed functions to be dead rather than untested, and that deleting them would be cheaper than testing them. Evidence gathered since says otherwise, and planning around the deletion assumption would mislead.

The offenders inspected so far are the untaken error-propagation branch of ?, sharing a physical line with the covered call. In peryx-ha-distributed/src/blob_plane.rs, all 24 visible ones are of that shape — meta.view_frontier(...)?, blobs.head(...).await?, write.commit(digest).await?. The success path is covered; the error region is not; lcov drops the line; LLVM counts it missed. A sample of peryx-http/src/handlers/cache.rs shows the same shape in its .map_err(|reason| ...) closures.

Those stores and blob reads genuinely can fail, so the branches are reachable, not dead. The remediation is therefore largely fault-injection tests, not deletions, and the repository already has the redb StorageBackend fault seam built for exactly this.

Two files were inspected closely and sampled respectively, so treat this as a strong signal rather than a proven claim across all 64 files. Where a line genuinely is dead, deleting it remains correct — just do not expect that to be the common case.

Locating the lines

lcov cannot find them. For blob_plane.rs, LLVM's summary reports 8 missed lines while lcov emits 297 DA records with none at zero, and the HTML report contains no uncovered-line rows at all. Both wrappers drop precisely the missed lines, which is llvm-project#126307. Parsing lcov for ,0 returns nothing for every one of these 64 files.

llvm-cov show --show-line-counts-or-regions does expose them, as ^0 region markers.

A scoped per-crate run cannot select the work. It attributes 5906 zero-count function records workspace-wide, including 37 in analytics.rs — a file CI reports as 0 missed, because other crates' tests execute those generic instantiations. Selecting from a scoped run means writing tests for lines that were never uncovered. The per-file table from the coverage job on #2162's head is the authoritative set; retrieve it with gh run view --job <job-id> --log, since --log-failed does not contain it.

Scope correction: lines only

This issue's title and body speak of "149 lines and 45 functions". The gate #2162 adds is --fail-under-lines, so only the 149 lines block it. The 45 missed functions and 4661 missed regions are real and worth closing eventually, but they are not what this gate measures, and folding them in would recreate the large clean-up that was deliberately split out.

Close the 149. Leave functions and regions to their own issue if anyone wants them.

Sequencing, settled

The remediation lands first; then #2162 flips the threshold on a workspace that already passes, and its own CI run is the proof.

Do not land #2162 first. The standing rule here is that every open pull request is green — merging a gate main cannot pass makes that unsatisfiable for every lane at once, and every subsequent pull request then shows a red required check that has nothing to do with its change. The signal stops meaning anything exactly when this remediation needs it.

Do not take the middle path of setting the threshold to today's number and ratcheting later. A threshold below 100 institutionalises the gap, and #2146 is the standing lesson that a gate which does not gate is worse than no gate, because it reads as green.

The actual mechanism: instantiation groups, not the merged file view

Established by reconstructing the algorithm and reproducing the per-file missed counts exactly on all 57 files, matching CI's table on 53 of 56 shared files with the three deltas at one line each.

cargo-llvm-cov's per-file summary.lines is not computed from the merged file view that lcov and the HTML report render. It is the sum over instantiation groups — functions grouped by definition start location (file, line, col) — where each group is merged by max(Covered) and max(NumLines) across its instantiations.

So LLVM reports the single best instantiation of a generic function, never the union of all of them. That is why lcov shows nothing: in the merged view these lines are covered.

The 140 lines split into two different kinds of work

• 62 lines are genuinely untested. No instantiation covers them. These are the fault-injection tests, using the redb StorageBackend seam.
• 78 lines are already exercised by at least one instantiation, just not by the best one.

blob_plane.rs is the clean case. pull_referenced<T> has three live instantiations:

HttpCapLimited   uncovered = [54, 75, 76, 77, 78]
Faulty           uncovered = [67, 71, 72, 73, 74]
Loopback         uncovered = [54, 68, 75, 76, 77, 78, 79, 80, 81, 82]
intersection     = []

Every line is covered by some test. The reported misses are the arms one instantiation happens not to walk, and a test for the "uncovered" behaviour already exists and passes — it drives a different T.

For those 78, writing a new test with a new type makes the number worse, because it adds another partial instantiation. The remediation is to make one instantiation walk every arm: consolidate onto a single test transport, or add the missing cases to the binary that links the highest-covered compilation.

Land these as two pull requests, 62 first and 78 second. They are different techniques, not different sizes of the same work, and mixing them makes a review carry two unrelated mental models.

Open risk: the lib-versus-bin split may be unclosable

peryx/src/process.rs::logging_layer has two instantiations — the lib copy covering 17 of 19 lines, and the bin copy covering 8. No unit test can raise the bin copy.

If that generalises, --fail-under-lines 100 is demanding something no amount of testing can deliver, and #2162 needs rethinking rather than more remediation. Settle those five lines before closing the other 135. The options, if it proves unclosable, are to exclude bin targets and state what that gives up, to compare the merged file view instead of the instantiation-group sum, or to carry a documented exception list — each changes what the gate means, so decide deliberately.

RESOLVED: the lib-versus-bin risk does not exist. The gate's premise holds.

The framing above — that a poorly covered binary instantiation might make a line unreachable — was wrong, and the correction is the useful part.

LLVM takes the best instantiation, so a badly covered copy never has to improve. The group merge is max(Covered) / max(NumLines), so a group reports zero missed exactly when some single instantiation covers all of its mapped lines. One compilation walking every arm is enough; the others are invisible to the score.

The only way that could fail is a group where the instantiation holding max(NumLines) cannot itself be fully covered — then max(Covered) from a smaller instantiation could never catch up. Checked across the whole workspace:

offending groups: uniform NumLines = 97   varying = 0

All 97 offending groups have identical NumLines across every instantiation, with zero exceptions, because NumLines is a property of the source function rather than of the monomorphisation. The failure mode does not occur anywhere in the offender set.

So every one of the 140 lines is reachable, and none of the three costly options is needed — no excluding bin targets, no changing what the gate compares, no exception list.

Concretely for process.rs::logging_layer: both instantiations have NumLines = 19. The lib-test copy covers 17 and misses the Journald and Syslog arms; the fixture-binary copy covers 8. Their union is 19 but neither reaches it alone. The existing test_logging_layers_cover_formats_and_platform_sinks already loops over Journald and Syslog — it simply lands in the compilation missing Stdout and File. Extending that one loop to all four sinks puts all 19 lines in a single compilation.

The 62 resolve into two mechanisms

Fault-path closures. .map_err(|error| error.to_string())? and })?; bodies that run only when the operation fails — the fsck.rs, cache.rs, admin.rs and summary.rs lines. The summary.rs group is unreached because the scanned prefix is empty in every test. This is the redb StorageBackend seam work.

Comparator closures never invoked, and this one points at behaviour nobody has verified. names.sort_by_key and drivers.sort_unstable_by_key are never called, because a sort comparator does not run on a collection of fewer than two elements and every fixture has exactly one index and one driver. The docstring on inspectors says the order exists so "a report an operator diffs may not reorder between runs" — a stability guarantee that has never been exercised. Closing it needs a two-element fixture, not fault injection.