Name the lines outside kcov's line set; bound defect (c) as fail-closed

The report printed a COUNT of traced lines that kcov never listed as
instrumentable. A count cannot be acted on: deciding whether such a line is a
real statement or a function declaration needs the line number. It now lists
them, the way it already lists uncovered lines.

Reading them across every lib with a harness splits them cleanly:

  benign  dwm_config.sh:27 (`heal_config() {`), transcribe_deps.sh:9
          (one-line function declaration), clean_audio_filters.sh:197,200
          (`((running++))`) -- correctly not statements
  real    aw_autostart.sh 68,70,73,79-81,83-84,87-88,94 -- plain echo, local,
          if, `sudo -u ... mkdir`, `cat >` heredoc. kcov lists nothing after
          the `{ ... } >>"$file"` command group at lines 61-65.

So kcov's LINE SET is incomplete too, not just its hit counts. This does NOT
inflate anything: a line reaches that list only because the TRACE SAW IT RUN,
so folding all 11 into both halves moves aw_autostart from 80/82 = 97.56% to
91/93 = 97.85% -- up. An off-set line is by construction an executed line, so
it can only ever be a missing COVERED line, never a missing uncovered one.

What it does mean is that such a subject is measured over a subset of its
statements. The percentage is honest about the subset, not the whole file,
which is why the lines are now printed rather than tallied.

Separately: nc_php.sh reports "kcov instrumented no lines" on the PRE-FIX
instrument too -- checked at 078d3463 in a throwaway worktree -- so it is not
a regression from the two-pass change. The 84/85 = 98.82% in earlier notes
does not reproduce and is recorded as unverified.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Krzysztof Rudnicki 2026-08-22 14:30:39 +02:00
parent 81191f40e6
commit 29b8c5a788
4 changed files with 113 additions and 13 deletions

View File

@ -160,3 +160,44 @@ substitution — the other half of defect (a). It is deliberately still in the
denominator: every line removed from a denominator inflates coverage, which denominator: every line removed from a denominator inflates coverage, which
is the fail-open direction, so a second exclusion rule should be justified is the fail-open direction, so a second exclusion rule should be justified
per line rather than generalised from one lib. per line rather than generalised from one lib.
## Defect (c): kcov's LINE SET is also incomplete — bounded, and fail-closed
The report prints, per subject, any line the trace saw execute that kcov never
listed as instrumentable at all:
```
outside kcov's line set (ran, but counted in neither half): 68, 70, 73, ...
```
Measured across every lib with a harness, these fall into two classes:
* **Not statements, correctly absent.** `dwm_config.sh:27` (`heal_config() {`,
a function declaration), `transcribe_deps.sh:9` (a one-line function
declaration), `clean_audio_filters.sh:197,200` (`((running++))`).
* **Ordinary statements, wrongly absent.** `aw_autostart.sh` lines 68, 70, 73,
79-81, 83-84, 87-88, 94 — plain `echo`, `local`, `if [[ ... ]]`, `sudo -u
... mkdir` and a `cat >` heredoc. The trigger is a `{ ... } >>"$file"`
command group with a redirection at lines 61-65: kcov lists nothing after it
in that function.
**This cannot inflate a percentage, and cannot hide an untested line.** A line
only reaches that list *because the trace saw it run*. Folding all 11 of
`aw_autostart.sh`'s into both halves moves it from 80/82 = 97.56% to
91/93 = 97.85% — up, not down. The direction is structural: an off-set line is
by construction an executed line, so it can only ever be a missing *covered*
line, never a missing *uncovered* one.
What it does mean is that a subject with off-set lines is measured over a
SUBSET of its statements. The percentage is honest about the subset; it is not
a claim about the whole file. Read the list before clearing such a lib off the
allowlist.
## `nc_php.sh` does not measure at all, on either instrument
`nc_php.sh` reports "kcov instrumented no lines" — reproduced twice on the
current instrument and once on the pre-fix instrument at commit `078d3463`,
via a throwaway worktree. It is **not** a regression from the two-pass change.
The 84/85 = 98.82% in earlier notes does not reproduce; whatever produced it
is not what `run_all.sh` does today. `test_nc_php.sh` exists and runs. Treat
the old figure as unverified and re-derive it before relying on it.

View File

@ -0,0 +1,50 @@
{
"intent": "Make the third kcov defect readable and bounded: list the individual lines the trace saw execute that kcov never listed as instrumentable, so a human can judge whether a subject's percentage covers its whole file, and record that this cannot inflate a number.",
"scope": [
"meta/scripts/shell_coverage_report.py - off_set becomes a list of line numbers, not a count",
"python_pkg/tests/test_shell_coverage_report.py - assertions follow the new return type",
"docs/kcov-under-report.md - documents defect (c) and the nc_php.sh finding",
"Non-goal: fixing defect (c). kcov's line set cannot be repaired from outside kcov."
],
"changes": [
"_measure returns the sorted off-set line numbers instead of len(); main prints them like the uncovered list, because deciding whether a line is a statement needs the number, not a count.",
"Documented that an off-set line is BY CONSTRUCTION an executed line, so folding them in can only raise a percentage -- aw_autostart.sh 80/82 = 97.56% becomes 91/93 = 97.85%. The list cannot hide an untested statement.",
"Documented that nc_php.sh fails to measure on the PRE-FIX instrument too, so it is not a regression from the two-pass change."
],
"verification": [
{
"command": "shell_coverage_jail.sh --measure aw_autostart.sh (through run_all.sh)",
"result": "pass",
"evidence": "80/82 = 97.56%, and the report now names lines 68, 70, 73, 79, 80, 81, 83, 84, 87, 88, 94. Reading them showed ordinary echo/local/if/cat statements; the trigger is the `{ ... } >>\"$file\"` command group at lines 61-65, after which kcov lists nothing in that function."
},
{
"command": "shell_coverage_jail.sh --measure dwm_config.sh / transcribe_deps.sh / clean_audio_filters.sh",
"result": "pass",
"evidence": "Off-set lines are dwm_config.sh:27 (`heal_config() {`), transcribe_deps.sh:9 (one-line function declaration) and clean_audio_filters.sh:197,200 (`((running++))`). All correctly not statements -- the benign class."
},
{
"command": "git worktree add --detach /tmp/claude-1000/wt_base 078d3463; shell_coverage_jail.sh --measure nc_php.sh",
"result": "pass",
"evidence": "\"kcov instrumented no lines\" on the PRE-FIX instrument, identical to the current one. Confirms nc_php.sh is a pre-existing condition and the old 84/85 = 98.82% figure does not reproduce. Worktree removed afterwards."
},
{
"command": "python3 -m pytest python_pkg/tests/test_shell_coverage_report.py python_pkg/tests/test_shell_coverage_lines.py",
"result": "pass",
"evidence": "29 passed; both modules hold 100% branch coverage (shell_coverage_report.py 62 stmts / 28 branches)."
},
{
"command": "pre-commit run --files (the four touched files)",
"result": "pass",
"evidence": "All hooks pass, no suppressions added."
}
],
"risks": [
"A subject with off-set lines is measured over a SUBSET of its statements, so its percentage is honest about that subset rather than the whole file. Mitigated by printing the lines so the gap is visible before a lib is cleared.",
"Defect (c) is not fixed and cannot be fixed from outside kcov; it is only bounded and made visible.",
"nc_php.sh currently yields no measurement at all, so it cannot be assessed for the allowlist until that is diagnosed separately."
],
"rollback": [
"git revert this commit; the previous behaviour (a count rather than a list) returns with no change to any percentage.",
"After rollback, re-run the two acceptance measurements (dot_resolver_install.sh 39/39, rpi_nc_ca.sh 73/73) to confirm the arithmetic is untouched."
]
}

View File

@ -81,11 +81,11 @@ def _find_source(subject: str, trace_dir: Path | None) -> Path | None:
def _measure( def _measure(
out_dir: Path, subject: str, trace_dir: Path | None out_dir: Path, subject: str, trace_dir: Path | None
) -> tuple[int, int, list[str], int]: ) -> tuple[int, int, list[str], list[str]]:
"""Return (covered, total, uncovered line numbers, off-set trace count).""" """Return (covered, total, uncovered lines, lines outside kcov's line set)."""
hits = _kcov_lines(out_dir, subject) hits = _kcov_lines(out_dir, subject)
if not hits: if not hits:
return 0, 0, [], 0 return 0, 0, [], []
source = _find_source(subject, trace_dir) source = _find_source(subject, trace_dir)
excluded = continuation_lines(source) if source is not None else set() excluded = continuation_lines(source) if source is not None else set()
@ -97,14 +97,20 @@ def _measure(
traced = traced_lines(trace_dir, subject) traced = traced_lines(trace_dir, subject)
# Lines the trace saw executing that kcov never listed as instrumentable. # Lines the trace saw executing that kcov never listed as instrumentable.
# Dropped from the numerator (they are not in the denominator), but # Dropped from BOTH numerator and denominator, so if any of them is a real
# surfaced: a non-zero count means kcov's LINE SET is wrong too, which is # statement the percentage is an OVER-estimate -- the fail-open direction.
# a third defect this design cannot correct for. # They are listed individually, not counted, because deciding whether they
off_set = len(traced - set(hits) - excluded) # are statements or non-statements needs the actual line numbers.
off_set = sorted(traced - set(hits) - excluded)
covered_set = (executed | traced) & denominator covered_set = (executed | traced) & denominator
uncovered = sorted(denominator - covered_set) uncovered = sorted(denominator - covered_set)
return len(covered_set), len(denominator), [str(n) for n in uncovered], off_set return (
len(covered_set),
len(denominator),
[str(n) for n in uncovered],
[str(n) for n in off_set],
)
# argv: <cov-dir> <subject> <minimum> [<trace-dir>]. The trace dir is optional # argv: <cov-dir> <subject> <minimum> [<trace-dir>]. The trace dir is optional
@ -133,7 +139,8 @@ def main() -> None:
sys.stdout.write(f" uncovered: {', '.join(uncovered)}\n") sys.stdout.write(f" uncovered: {', '.join(uncovered)}\n")
if off_set: if off_set:
sys.stdout.write( sys.stdout.write(
f" note: {off_set} traced line(s) are absent from kcov's line set\n" f" outside kcov's line set (ran, but counted in neither half): "
f"{', '.join(off_set)}\n"
) )
if percent < minimum: if percent < minimum:

View File

@ -70,7 +70,7 @@ def test_kcov_hits_alone_are_used_without_a_trace(
"""With no trace dir the result is kcov's own reading.""" """With no trace dir the result is kcov's own reading."""
cov = _cov(tmp_path, "s.sh", {1: 1, 2: 0}) cov = _cov(tmp_path, "s.sh", {1: 1, 2: 0})
covered, total, uncovered, off_set = mod._measure(cov, "s.sh", None) covered, total, uncovered, off_set = mod._measure(cov, "s.sh", None)
assert (covered, total, uncovered, off_set) == (1, 2, ["2"], 0) assert (covered, total, uncovered, off_set) == (1, 2, ["2"], [])
def test_trace_rescues_a_line_kcov_missed(mod: ModuleType, tmp_path: Path) -> None: def test_trace_rescues_a_line_kcov_missed(mod: ModuleType, tmp_path: Path) -> None:
@ -109,13 +109,15 @@ def test_traced_line_outside_the_line_set_is_counted_not_credited(
cov = _cov(tmp_path, "s.sh", {1: 1}) cov = _cov(tmp_path, "s.sh", {1: 1})
trace = _trace(tmp_path, source, [1, 3]) trace = _trace(tmp_path, source, [1, 3])
covered, total, _, off_set = mod._measure(cov, "s.sh", trace) covered, total, _, off_set = mod._measure(cov, "s.sh", trace)
assert (covered, total, off_set) == (1, 1, 1) # Line 3 ran but kcov never listed it: excluded from both halves, and
# named so a human can judge whether it is a statement.
assert (covered, total, off_set) == (1, 1, ["3"])
def test_empty_kcov_output_reports_zero_total(mod: ModuleType, tmp_path: Path) -> None: def test_empty_kcov_output_reports_zero_total(mod: ModuleType, tmp_path: Path) -> None:
"""No instrumented lines is a distinct, reportable state.""" """No instrumented lines is a distinct, reportable state."""
cov = _cov(tmp_path, "other.sh", {1: 1}) cov = _cov(tmp_path, "other.sh", {1: 1})
assert mod._measure(cov, "s.sh", None) == (0, 0, [], 0) assert mod._measure(cov, "s.sh", None) == (0, 0, [], [])
def test_line_without_a_number_is_skipped(mod: ModuleType, tmp_path: Path) -> None: def test_line_without_a_number_is_skipped(mod: ModuleType, tmp_path: Path) -> None:
@ -210,7 +212,7 @@ def test_main_notes_lines_outside_the_line_set(
cov = _cov(tmp_path, "s.sh", {1: 1}) cov = _cov(tmp_path, "s.sh", {1: 1})
trace = _trace(tmp_path, source, [1, 2]) trace = _trace(tmp_path, source, [1, 2])
_run_main(mod, monkeypatch, [str(cov), "s.sh", "100", str(trace)]) _run_main(mod, monkeypatch, [str(cov), "s.sh", "100", str(trace)])
assert "absent from kcov's line set" in capsys.readouterr().out assert "outside kcov's line set" in capsys.readouterr().out
def test_main_exits_when_below_the_minimum( def test_main_exits_when_below_the_minimum(