Coverage instrument: a closing brace plus an operator is not a statement

continuation_lines() excluded the continuation lines of multi-line QUOTED arguments but nothing else, so a line holding only a group's closing brace and an operator -- `} |`, `} >>"$REPORT_FILE"` -- stayed in the denominator while bash attributed no statement to it. That capped four fixes/lib libs below 100% with no test able to move them. Why the anchor makes this safe: bash attributes every command INSIDE a group to its own line, and a pipe's right-hand side to its own line, leaving the brace-plus-operator line with nothing. Anchoring the pattern on a LEADING `}` therefore excludes only lines that can hold no statement. A one-liner `{ echo a; } >file` begins with `{` and is untouched; a bare `}` closing a function has no operator and is untouched. Measured effect, all through fixes/lib/tests/run_all.sh: bt_audio.sh 96.97% -> 100.00% (127/127) arch_perf_report.sh 98.33% -> 100.00% (59/59) bt_report.sh 100.00% -> 100.00% (37/37) bt_pairing.sh 100.00% -> 100.00% (65/65) bt_adapter.sh 98.55% -> 98.55% (68/69, capped by a different flavour) Two gained, three held, none went DOWN -- the handoff's regression signal. That is not luck: _measure computes covered_set = (executed|traced) & denominator, so for a lib already at 100% the covered set IS the denominator and removing a subset cannot lower it. The five gated pacman_hook_stall libs are safe by the same arithmetic. Acceptance test from the handoff passes unchanged: dot_resolver_install.sh 39/39 and rpi_nc_ca.sh 73/73, both 100.00%. The fail-open check that actually matters, since over-detection INFLATES coverage and could promote an untested lib: the new predicate was run over every lib in the allowlist plus the cleared ones, printing each newly excluded line WITH its text. All 17 across 10 libs are genuine brace-plus-operator lines; no ordinary statement was caught. Tests cover both directions, including the two negative cases (a bare `}` and a single-line `{ ...; } >file`), keeping the module at 100% branch coverage. The quote scanner moved into _quote_after() to stay under the C901 complexity limit rather than suppressing it. Deliberately NOT fixed here: the backslash-continued ASSIGNMENT flavour that caps bt_adapter.sh line 97. bash attributes those to the LAST line of the run but a backslash-continued plain COMMAND to its FIRST, so that predicate needs a command-vs-assignment distinction. One line in one lib; not worth the landmine today. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-11-10 20:07:04 +01:00 · 2026-08-22 18:29:47 +02:00 · 2026-08-22 18:29:47 +02:00 · d989cd7ebf
commit d989cd7ebf
parent ffcf99f102
3 changed files with 129 additions and 22 deletions
--- a/docs/superpowers/evidence/closing-brace-continuation-2026-08-22.json
+++ b/docs/superpowers/evidence/closing-brace-continuation-2026-08-22.json
@ -0,0 +1,44 @@
+{
+  "intent": "Teach continuation_lines() that a line holding only a group's closing brace plus an operator (`} |`, `} >>file`) is not a statement, so kcov stops inflating the denominator with it. This is defect (a) in docs/kcov-under-report.md, in the flavour that caps four fixes/lib libs.",
+  "scope": [
+    "meta/scripts/lib/shell_coverage_lines.py (one regex and one branch)",
+    "python_pkg/tests/test_shell_coverage_lines.py (four new cases: two positive, two negative)",
+    "Non-goal: the backslash-continued ASSIGNMENT flavour that caps bt_adapter.sh line 97 -- it needs a command-vs-assignment distinction and is one line in one lib",
+    "Non-goal: clearing any lib from meta/shell-coverage-allowlist.txt, which follows in its own commit once each is re-measured"
+  ],
+  "changes": [
+    "Added _CLOSING_BRACE_OP = ^\\s*\\}\\s*(\\||>>?|<|&>) and one branch in continuation_lines() that excludes matching lines when no quote is open.",
+    "Anchoring on a LEADING closing brace is what makes this safe. bash attributes each command inside a group to its own line, and a pipe's right-hand side to its own line, so the brace-plus-operator line is left with no statement. A one-liner `{ echo a; } >file` starts with `{` and does not match; a bare `}` closing a function carries no operator and does not match.",
+    "Added four tests: `} |` and `} >>\"$REPORT_FILE\"` are continuations; a bare `}` closing a function and a single-line `{ echo a; } >\"$out\"` are NOT. The negative pair is the important half, because over-detection shrinks the denominator and INFLATES coverage -- the fail-open direction this test module's own header calls out."
+  ],
+  "verification": [
+    {
+      "command": "python3 -m coverage run --branch --source=meta/scripts/lib -m pytest python_pkg/tests/test_shell_coverage_lines.py",
+      "result": "pass",
+      "evidence": "18 passed. shell_coverage_lines.py: 56 statements, 40 branches, 0 missed, 0 partial = 100% -- so the new branch is covered in both directions, as the repo's fail_under=100 branch gate requires."
+    },
+    {
+      "command": "Audit every line the new predicate newly excludes across the whole allowlist plus the cleared libs",
+      "result": "pass",
+      "evidence": "17 lines excluded across 10 libs, and every one re-matched the brace-plus-operator shape when printed with its text: integrity.sh:68/118, aw_autostart.sh:65, rpi_nc_ssl.sh:88, wg_sshd.sh:66, arch_perf_probes.sh:46/93, arch_perf_report.sh:20, bt_audio.sh:90/113/137/145/157, nvidia_config.sh:68, trace_prefix.sh:100, trace_stubs.sh:69, adb_trusted.sh:29. No ordinary statement was caught."
+    },
+    {
+      "command": "bash meta/scripts/shell_coverage_jail.sh --subject .../fixes/lib/tests/run_all.sh --measure <lib> --min 1 --fail-on-case-error -- \"\", for five libs",
+      "result": "pass",
+      "evidence": "bt_audio.sh 96.97% -> 100.00% (127/127), arch_perf_report.sh 98.33% -> 100.00% (59/59), bt_report.sh 100.00% (37/37) unchanged, bt_pairing.sh 100.00% (65/65) unchanged, bt_adapter.sh 98.55% (68/69) unchanged. Two libs gained, three held, none went DOWN -- the handoff's stated regression signal."
+    },
+    {
+      "command": "The handoff's acceptance test: dot_resolver_install.sh and rpi_nc_ca.sh through features/lib/tests/run_all.sh with its jail_args",
+      "result": "pass",
+      "evidence": "dot_resolver_install.sh: 39/39 = 100.00% and rpi_nc_ca.sh: 73/73 = 100.00%, exactly the figures the handoff names as the instrument's acceptance criteria."
+    }
+  ],
+  "risks": [
+    "A denominator change touches every measured lib, which is why it lands alone. The arithmetic bounds the blast radius: _measure computes denominator = kcov's line set minus excluded, and covered_set = (executed | traced) & denominator, so for any lib already at 100.00% the covered set IS the denominator and removing a subset leaves it at 100.00%. The five gated pacman_hook_stall libs, bt_report and bt_pairing therefore cannot regress, which the measurements above confirm for the two that were re-run.",
+    "The predicate is deliberately narrow. It does not cover `} &&`, `} ||` or a closing brace followed by a comment, and it does not touch the backslash-continued assignment flavour. Each would need its own evidence that bash attributes no statement to the line."
+  ],
+  "rollback": [
+    "git revert the commit; continuation_lines() returns to quote-state detection only and the four new tests go with it.",
+    "After rollback, re-measure bt_audio.sh and arch_perf_report.sh and expect them back at 96.97% and 98.33%; neither is on the allowlist-cleared list, so no gate breaks."
+  ]
+}
--- a/meta/scripts/lib/shell_coverage_lines.py
+++ b/meta/scripts/lib/shell_coverage_lines.py
@ -66,6 +66,16 @@ def traced_paths(trace_dir: Path, subject: str) -> list[Path]:
    return found


+# A line holding only a group's closing brace plus an operator: `} |`,
+# `} >>file`, `} <in`, `} &>log`. kcov instruments these, but bash attributes
+# no statement to them -- each command INSIDE the group is attributed to its
+# own line, and a pipe's right-hand side to its own line, leaving the brace
+# line with nothing. Anchoring on a leading `}` is what keeps this safe: a
+# one-liner `{ echo a; } >file` begins with `{` and is a real statement, and a
+# bare `}` closing a function carries no operator.
+_CLOSING_BRACE_OP = re.compile(r"^\s*\}\s*(\||>>?|<|&>)")
+
+
 def continuation_lines(source: Path) -> set[int]:
    """Return lines that are *inside* a multi-line quoted argument.

@ -75,9 +85,13 @@ def continuation_lines(source: Path) -> set[int]:
    them as executable at all, which is the correct reading: they are data
    inside an argument, not statements.

-    Detected by tracking quote state across the file: any line that *begins*
-    while a quote opened on an earlier line is still open is a continuation.
-    Only the opening line of such a construct is a statement.
+    Two shapes qualify:
+
+    * a line that *begins* while a quote opened on an earlier line is still
+      open -- the continuation of a multi-line quoted argument. Only the
+      opening line of such a construct is a statement.
+    * a line holding just a group's closing brace and an operator (``} |``,
+      ``} >>file``). See ``_CLOSING_BRACE_OP``.
    """
    inside: set[int] = set()
    quote: str | None = None
@ -86,24 +100,35 @@ def continuation_lines(source: Path) -> set[int]:
    ):
        if quote is not None:
            inside.add(number)
-        index = 0
-        while index < len(raw):
-            char = raw[index]
-            if quote is None:
-                if char == "#":
-                    # A comment outside quotes ends the line's significance.
-                    break
-                if char == "\\":
-                    index += 2
-                    continue
-                if char in ("'", '"'):
-                    quote = char
-            elif char == quote:
-                quote = None
-            elif quote == '"' and char == "\\":
-                # Only double quotes honour backslash escapes; inside single
-                # quotes a backslash is a literal character.
+        quote = _quote_after(raw, quote)
+        if quote is None and _CLOSING_BRACE_OP.match(raw):
+            inside.add(number)
+    return inside
+
+
+def _quote_after(raw: str, quote: str | None) -> str | None:
+    """Return the open-quote state at the end of ``raw``, given its state at the start.
+
+    ``None`` means no quote is open. Escapes are honoured outside quotes and
+    inside double quotes, but not inside single quotes, which take a
+    backslash literally.
+    """
+    index = 0
+    while index < len(raw):
+        char = raw[index]
+        if quote is None:
+            if char == "#":
+                # A comment outside quotes ends the line's significance.
+                break
+            if char == "\\":
                index += 2
                continue
-            index += 1
-    return inside
+            if char in ("'", '"'):
+                quote = char
+        elif char == quote:
+            quote = None
+        elif quote == '"' and char == "\\":
+            index += 2
+            continue
+        index += 1
+    return quote
--- a/python_pkg/tests/test_shell_coverage_lines.py
+++ b/python_pkg/tests/test_shell_coverage_lines.py
@ -79,6 +79,44 @@ def test_multiline_double_quote_marks_inner_lines(
    assert mod.continuation_lines(src) == {2, 3}


+def test_closing_brace_with_pipe_is_a_continuation(
+    mod: ModuleType, tmp_path: Path
+) -> None:
+    """`} |` carries no statement: bash attributes each part to its own line."""
+    src = _write(
+        tmp_path,
+        "a.sh",
+        "{\n\techo a\n\techo b\n} |\n\tconsumer\n",
+    )
+    assert mod.continuation_lines(src) == {4}
+
+
+def test_closing_brace_with_redirect_is_a_continuation(
+    mod: ModuleType, tmp_path: Path
+) -> None:
+    """The arch_perf_report.sh shape: a group appended to a report file."""
+    src = _write(
+        tmp_path,
+        "a.sh",
+        '{\n\techo a\n} >>"$REPORT_FILE"\necho after\n',
+    )
+    assert mod.continuation_lines(src) == {3}
+
+
+def test_bare_closing_brace_is_a_statement(mod: ModuleType, tmp_path: Path) -> None:
+    """A `}` closing a function has no operator and must stay in the count."""
+    src = _write(tmp_path, "a.sh", "f() {\n\techo a\n}\nf\n")
+    assert mod.continuation_lines(src) == set()
+
+
+def test_single_line_group_with_redirect_is_a_statement(
+    mod: ModuleType, tmp_path: Path
+) -> None:
+    """`{ echo a; } >file` begins with `{`, so the whole line is a statement."""
+    src = _write(tmp_path, "a.sh", '{ echo a; } >"$out"\necho after\n')
+    assert mod.continuation_lines(src) == set()
+
+
 def test_comment_outside_quotes_ends_the_line(mod: ModuleType, tmp_path: Path) -> None:
    """An apostrophe inside a comment must not open a quote state."""
    src = _write(tmp_path, "a.sh", "# don't be fooled\necho after\n")