From 29b8c5a7886dce2ad212bab2c9fd65bb1fc5393b Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sat, 22 Aug 2026 14:30:39 +0200 Subject: [PATCH] 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) --- docs/kcov-under-report.md | 41 +++++++++++++++ .../kcov-offset-lines-2026-08-22.json | 50 +++++++++++++++++++ meta/scripts/shell_coverage_report.py | 25 ++++++---- .../tests/test_shell_coverage_report.py | 10 ++-- 4 files changed, 113 insertions(+), 13 deletions(-) create mode 100644 docs/superpowers/evidence/kcov-offset-lines-2026-08-22.json diff --git a/docs/kcov-under-report.md b/docs/kcov-under-report.md index 05431381..ecf9e037 100644 --- a/docs/kcov-under-report.md +++ b/docs/kcov-under-report.md @@ -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 is the fail-open direction, so a second exclusion rule should be justified 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. diff --git a/docs/superpowers/evidence/kcov-offset-lines-2026-08-22.json b/docs/superpowers/evidence/kcov-offset-lines-2026-08-22.json new file mode 100644 index 00000000..05cd34dd --- /dev/null +++ b/docs/superpowers/evidence/kcov-offset-lines-2026-08-22.json @@ -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." + ] +} diff --git a/meta/scripts/shell_coverage_report.py b/meta/scripts/shell_coverage_report.py index 5cfb24fc..27adc728 100755 --- a/meta/scripts/shell_coverage_report.py +++ b/meta/scripts/shell_coverage_report.py @@ -81,11 +81,11 @@ def _find_source(subject: str, trace_dir: Path | None) -> Path | None: def _measure( out_dir: Path, subject: str, trace_dir: Path | None -) -> tuple[int, int, list[str], int]: - """Return (covered, total, uncovered line numbers, off-set trace count).""" +) -> tuple[int, int, list[str], list[str]]: + """Return (covered, total, uncovered lines, lines outside kcov's line set).""" hits = _kcov_lines(out_dir, subject) if not hits: - return 0, 0, [], 0 + return 0, 0, [], [] source = _find_source(subject, trace_dir) excluded = continuation_lines(source) if source is not None else set() @@ -97,14 +97,20 @@ def _measure( traced = traced_lines(trace_dir, subject) # Lines the trace saw executing that kcov never listed as instrumentable. - # Dropped from the numerator (they are not in the denominator), but - # surfaced: a non-zero count means kcov's LINE SET is wrong too, which is - # a third defect this design cannot correct for. - off_set = len(traced - set(hits) - excluded) + # Dropped from BOTH numerator and denominator, so if any of them is a real + # statement the percentage is an OVER-estimate -- the fail-open direction. + # They are listed individually, not counted, because deciding whether they + # are statements or non-statements needs the actual line numbers. + off_set = sorted(traced - set(hits) - excluded) covered_set = (executed | traced) & denominator 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: []. The trace dir is optional @@ -133,7 +139,8 @@ def main() -> None: sys.stdout.write(f" uncovered: {', '.join(uncovered)}\n") if off_set: 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: diff --git a/python_pkg/tests/test_shell_coverage_report.py b/python_pkg/tests/test_shell_coverage_report.py index 94ff60be..cbd08fc2 100644 --- a/python_pkg/tests/test_shell_coverage_report.py +++ b/python_pkg/tests/test_shell_coverage_report.py @@ -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.""" cov = _cov(tmp_path, "s.sh", {1: 1, 2: 0}) 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: @@ -109,13 +109,15 @@ def test_traced_line_outside_the_line_set_is_counted_not_credited( cov = _cov(tmp_path, "s.sh", {1: 1}) trace = _trace(tmp_path, source, [1, 3]) 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: """No instrumented lines is a distinct, reportable state.""" 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: @@ -210,7 +212,7 @@ def test_main_notes_lines_outside_the_line_set( cov = _cov(tmp_path, "s.sh", {1: 1}) trace = _trace(tmp_path, source, [1, 2]) _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(