C3 (#137 ): pin what InputQuery makes of a metadata row

Nothing had ever handed InputQuery a cursor row. UnknownInputSizeTest drives the no-provider case thoroughly -- query returns null, measure() answers -- so firstRow's body, displayNameOrNull and sizeOrNull had never executed at all. Nine tests over FakeSafProvider's RowShape states. What they pin is not "reads a cursor" but the rule the class exists for: a size nobody could determine must arrive as null, never 0. Four separate ways a provider fails to give one -- a null cell, a missing column, a negative value, an empty cursor -- plus a provider that throws outright, which is the guard firstRow's KDoc is written for. Mutations run, all four bite: drop `takeIf { it >= 0 }` from sizeOrNull -> negative-size test red drop `!isNull(it)` from sizeOrNull -> null-size test red drop the runCatching in firstRow -> throwing-provider test red drop `!isNull(it)` from displayNameOrNull -> GREEN, does not bite That last one is recorded in the test's KDoc as a named exemption rather than papered over. Measured: MatrixCursor.getString on a null cell returns null while getLong returns 0. So the guard is load-bearing on the size path -- it is what stops a null becoming a real number -- and unfalsifiable on the name path, where getString already yields null. It stays regardless: Cursor.getString's contract makes throwing on null implementation-defined, and a real provider may do what MatrixCursor does not. InputQuery.kt now has no never-executed lines. Suite 456 -> 465 tests, branch coverage 63.8% -> 65.6%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
C0 (#134 ): move the fake providers to scaffolding, let them answer wrongly
2026-08-27 06:57:15 -05:00 · 2026-08-27 06:57:15 -05:00 · 2026-08-26 22:11:09 -05:00 · 2026-08-26 22:03:11 -05:00 · 2026-08-26 22:01:55 -05:00 · 2026-08-26 21:47:17 -05:00
14 changed files with 1375 additions and 139 deletions
@@ -0,0 +1,193 @@
+#!/usr/bin/env bash
+#
+# Exercises e2e-report-shape.sh's baseline counter against fixture source, with no emulator and
+# no CI run. Run it directly:
+#
+#   .github/scripts/e2e-report-shape-test.sh
+#
+# WHY THIS CAN EXIST AT ALL: the counter is a pure function of the working tree. It greps
+# `app/src/androidTest` for `@FailsOnEmulatorApi37` and compares the total against the number
+# committed in FailsOnEmulatorApi37.kt. Nothing about that needs a device, which is the whole
+# reason #120 could be measured rather than argued about.
+#
+# WHY A THROWAWAY REPO ROOT rather than a knob on the script. The report finds its own root from
+# `BASH_SOURCE`, so a copy of it placed at `<root>/.github/scripts/` reads `<root>/app/src/...`.
+# Building that root is three mkdirs and costs the shipped script nothing:
+#
+#   - the REAL script is what runs, byte for byte, so reverting the matcher reddens this test
+#     rather than a testing-only code path beside it;
+#   - no environment variable exists that could point the LIVE count somewhere else, which is
+#     the failure mode #83 built the baseline check to prevent in the first place;
+#   - XML_DIR resolves inside the throwaway root, so a stale app/build/outputs left by a real
+#     run on a developer machine cannot leak into the numbers here.
+#
+# WHAT IT GUARDS (#120). The old matcher looked for the string anywhere on any line, so a KDoc
+# saying `Deliberately not @FailsOnEmulatorApi37` counted as a marked test and every PR got a
+# deviation notice that was wrong. The obvious repair -- count only lines that are nothing but
+# the annotation -- silently stops counting `@FailsOnEmulatorApi37 @Test`, which is legal Kotlin,
+# and undercounting is the direction that hides a genuine new marker. The fixture carries every
+# shape at once -- three that count and three that must not, enumerated in its own header -- so
+# both mistakes fail here instead of on a PR: against testdata/marker-shapes the old matcher says
+# 5, own-line-only says 2, and the shipped one 3.
+#
+# NOT WIRED INTO CI, deliberately and as a known gap. Adding a step to the Static analysis job
+# would add a new way for a gating job to go red, and #120 was explicit that nothing about it may
+# change any job's status or the pass/fail rules. shellcheck still covers this file, since that
+# step reads `git ls-files '*.sh'` rather than a fixed list.
+set -uo pipefail
+
+SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
+REPORT="$SCRIPT_DIR/e2e-report-shape.sh"
+FIXTURE_DIR="$SCRIPT_DIR/testdata/marker-shapes"
+FIXTURE="$FIXTURE_DIR/MarkerShapes.kt"
+
+TMP="$(mktemp -d)"
+# Single quotes: the path is expanded when the trap fires, not when it is set.
+trap 'rm -rf -- "$TMP"' EXIT
+
+failures=0
+
+pass() { printf 'ok    %s\n' "$1"; }
+
+fail() {
+  failures=$((failures + 1))
+  printf 'FAIL  %s\n' "$1"
+  shift
+  printf '        %s\n' "$@"
+}
+
+# assert_contains <name> <haystack> <needle>
+# `case` rather than grep: the strings being matched carry backticks and an em dash, and this
+# way neither the shell nor a regex engine gets an opinion about them.
+assert_contains() {
+  case "$2" in
+    *"$3"*) pass "$1" ;;
+    *) fail "$1" "wanted to find: $3" "in:" "$2" ;;
+  esac
+}
+
+assert_absent() {
+  case "$2" in
+    *"$3"*) fail "$1" "did NOT want to find: $3" "in:" "$2" ;;
+    *) pass "$1" ;;
+  esac
+}
+
+# make_root <marked-tree-dir> <baseline-number>
+# Assembles a throwaway repo root around the given tree and prints its path.
+make_root() {
+  local tree="$1" baseline="$2" root testdir
+  root="$(mktemp -d "$TMP/root.XXXXXX")"
+  testdir="$root/app/src/androidTest/java/org/libremediaconverter"
+  mkdir -p "$root/.github/scripts" "$testdir"
+  cp -- "$REPORT" "$root/.github/scripts/"
+  cp -- "$tree"/*.kt "$testdir/"
+
+  # The synthetic stand-in for the committed baseline. Its KDoc names the marker the way the real
+  # file does -- in brackets, never with an `@` -- because the real file lives inside the tree
+  # being counted, so an `@` spelling here would add a phantom to every number below.
+  cat > "$testdir/FailsOnEmulatorApi37.kt" <<EOF
+package org.libremediaconverter
+
+/** Stand-in for the real marker file. Only [FAILS_ON_EMULATOR_API37_BASELINE] is read. */
+const val FAILS_ON_EMULATOR_API37_BASELINE = $baseline
+EOF
+
+  # A clean, untruncated run of exactly <baseline> tests, all failing -- which is what the
+  # advisory leg looks like when nothing has drifted. It leaves the marked count as the only
+  # field that can deviate, so every assertion below is about the thing under test.
+  cat > "$root/gradle.log" <<EOF
+> Task :app:connectedDebugAndroidTest
+Starting $baseline tests on test(AVD) - 16
+There was $baseline failure(s).
+EOF
+
+  printf '%s\n' "$root"
+}
+
+# run_report <root>  -- stdout of the real script; its summary lands in <root>/summary.md.
+run_report() {
+  E2E_WEDGED_AFTER='' GITHUB_STEP_SUMMARY="$1/summary.md" \
+    bash "$1/.github/scripts/e2e-report-shape.sh" 37 "$1/gradle.log" \
+    "$1/app/src/androidTest/java/org/libremediaconverter/FailsOnEmulatorApi37.kt"
+}
+
+# ---------------------------------------------------------------------------
+# The fixture still carries every shape.
+#
+# Three of the checks below are covered twice over -- deleting a real annotation moves the count
+# and fails a case further down. The two decoys are not: drop the KDoc mention and the count
+# stays 3, so the precision this whole ticket is about would stop being tested and nothing would
+# say so. That asymmetry is why the shapes are asserted by name rather than only by their effect
+# on the total.
+# ---------------------------------------------------------------------------
+fixture_text="$(cat -- "$FIXTURE")"
+assert_contains "fixture: the import"           "$fixture_text" 'import org.libremediaconverter.FailsOnEmulatorApi37'
+assert_contains "fixture: annotation own line"  "$fixture_text" '
+    @FailsOnEmulatorApi37
+    @Test'
+assert_contains "fixture: annotation with @Test on one line" "$fixture_text" '@FailsOnEmulatorApi37 @Test'
+assert_contains "fixture: annotation nested and indented" "$fixture_text" '
+        @FailsOnEmulatorApi37'
+assert_contains "fixture: KDoc mention (this is #120)" "$fixture_text" "* Deliberately not \`@FailsOnEmulatorApi37\`"
+assert_contains "fixture: commented-out annotation" "$fixture_text" '// @FailsOnEmulatorApi37'
+
+# ---------------------------------------------------------------------------
+# 1. The fixture's three real annotations against a baseline of 3: no deviation.
+#
+# This one case fails under both wrong matchers -- the old one counts 5, own-line-only counts 2 --
+# which is why it is first.
+# ---------------------------------------------------------------------------
+root="$(make_root "$FIXTURE_DIR" 3)"
+out="$(run_report "$root")"
+assert_contains "3 real markers, baseline 3: reports a match" "$out" '  baseline: matches (3 expected, 3 failed)'
+assert_absent   "3 real markers, baseline 3: says nothing about the tree" "$out" 'the tree carries'
+assert_contains "3 real markers, baseline 3: summary agrees" \
+  "$(cat -- "$root/summary.md")" '**Matches the committed baseline of 3**'
+
+# ---------------------------------------------------------------------------
+# 2. A fourth REAL annotation. The count has to move and the deviation has to fire.
+#
+# The important half of #120: precision was the bug, but a matcher that stopped noticing a new
+# marker would have been a worse one, silently.
+# ---------------------------------------------------------------------------
+plus_one="$(mktemp -d "$TMP/plusone.XXXXXX")"
+cp -- "$FIXTURE" "$plus_one/"
+cat > "$plus_one/FourthMarker.kt" <<'EOF'
+package org.libremediaconverter.fixture
+
+class FourthMarker {
+    @FailsOnEmulatorApi37
+    @Test
+    fun addedToday() = Unit
+}
+EOF
+root="$(make_root "$plus_one" 3)"
+out="$(run_report "$root")"
+assert_contains "a 4th real marker: the deviation fires, and counts 4" "$out" \
+  "  baseline DEVIATION: the tree carries 4 tests marked \`@FailsOnEmulatorApi37\` but the baseline says 3 — update FAILS_ON_EMULATOR_API37_BASELINE"
+assert_contains "a 4th real marker: the summary carries it too" "$(cat -- "$root/summary.md")" \
+  "- the tree carries 4 tests marked \`@FailsOnEmulatorApi37\` but the baseline says 3"
+
+# ---------------------------------------------------------------------------
+# 3. Delete the same-line annotation and the count must drop to 2.
+#
+# This is the trap, pinned down. `@FailsOnEmulatorApi37 @Test` on one line is what separates the
+# shipped matcher from `^[[:space:]]*@NAME[[:space:]]*$`, and without this case the fixture entry
+# guarding it could be deleted as decoration -- case 1 would then pass under the wrong matcher.
+# Here the same-line entry is worth exactly one, and it is asserted to be.
+# ---------------------------------------------------------------------------
+minus_same_line="$(mktemp -d "$TMP/minus.XXXXXX")"
+sed -e '/@FailsOnEmulatorApi37 @Test/d' -- "$FIXTURE" > "$minus_same_line/MarkerShapes.kt"
+root="$(make_root "$minus_same_line" 3)"
+out="$(run_report "$root")"
+assert_contains "same-line annotation removed: counts 2, so it was worth 1" "$out" \
+  "  baseline DEVIATION: the tree carries 2 tests marked \`@FailsOnEmulatorApi37\` but the baseline says 3 — update FAILS_ON_EMULATOR_API37_BASELINE"
+
+echo
+if [ "$failures" -eq 0 ]; then
+  echo "e2e-report-shape-test.sh: all checks passed"
+  exit 0
+fi
+echo "e2e-report-shape-test.sh: $failures check(s) failed"
+exit 1
@@ -30,16 +30,31 @@
 #
 # Usage:
 #   e2e-report-shape.sh <label> <gradle-log> [<baseline-file>]
+#   E2E_WEDGED_AFTER=<seconds>   the wrapper timeout killed gradle after that many seconds
 #
 # With a third argument the run is compared against the baseline in that file (advisory mode)
 # and a `::notice::` is emitted per deviation. NEVER `::error::`: the advisory job is
 # `continue-on-error: true` and stays that way, and an error annotation would be a new way for
 # a diagnostic to change a conclusion.
+#
+# WHY THE WEDGE ARRIVES AS AN ENV VAR (#118) rather than being read out of the log like every
+# other field: there is nothing in the log to read. A wedge is gradle never returning, so gradle
+# never printed a verdict, never printed a truncation line, and never aborted instrumentation --
+# the log of a wedged leg is the log of a run that simply stops. Measured on job 98035980326:
+# `expected: 59`, `received: 59`, `completed cleanly: yes`, six seconds before the wedge warning,
+# for a leg that the timeout had killed 22 minutes in. Only e2e-run.sh knows, because only it
+# saw `timeout` exit 124, so it says so. Guessing it from a log that ends abruptly would call
+# every cancelled run a wedge.
+#
+# It is read as a STRING and only ever interpolated into one. `[ -n ... ]`, never `-gt`: it
+# crosses a process boundary from a shell that deliberately sets it EMPTY on every non-wedge
+# path, and an arithmetic test on an empty string is the header's rule four paragraphs up.
 set -uo pipefail

 LABEL="${1:-unknown}"
 LOG="${2:-}"
 BASELINE_FILE="${3:-}"
+WEDGED_AFTER="${E2E_WEDGED_AFTER:-}"

 SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
 REPO_ROOT="$(cd -- "$SCRIPT_DIR/../.." && pwd)"
@@ -115,6 +130,13 @@ elif [ -n "$abort_received" ]; then
 elif [ -n "$expected" ] && [ -z "$abort_line" ]; then
  received="$expected"
  received_src="the run was not truncated, so every expected test reported"
+  # ... unless it was killed, in which case "not truncated" is only "gradle never got as far as
+  # saying so". This is the branch the wedged leg in #118 took -- with no XML written yet, the
+  # number is what the runner was TOLD to run, and the source line said the opposite in the same
+  # table that called the leg clean. The number is deliberately left alone: it is still the best
+  # available answer, and only the claim about where it came from was wrong.
+  [ -n "$WEDGED_AFTER" ] \
+    && received_src="no test XML was written and gradle never printed a truncation line — but the leg was killed mid-run, so this is what it was told to run, not what reported"
 fi

 failed="unknown"
@@ -152,6 +174,12 @@ elif [ "$no_run" = "nothing" ]; then
  # "cleanly" would be a lie about a run that left no evidence it happened.
  completed="unknown"
  completed_src="no runner output to read"
+elif [ -n "$WEDGED_AFTER" ]; then
+  # The wedge is checked LAST of the four, so it only ever overrides the `yes`. The two "no"s
+  # above are already right and name the abort, which the wedge row does not; `unknown` is
+  # already right too. A wedge on top of an abort is both facts, and both get printed.
+  completed="**no**"
+  completed_src="the wrapper timeout killed gradle after ${WEDGED_AFTER}s — instrumentation itself was never aborted, which is why nothing in the log says so"
 else
  completed="yes"
  completed_src="no truncation line and no \`INSTRUMENTATION_ABORTED\`"
@@ -180,11 +208,34 @@ if [ -n "$BASELINE_FILE" ]; then
  advisory="yes"
  [ -f "$BASELINE_FILE" ] \
    && baseline="$(sed -nE 's/^const val FAILS_ON_EMULATOR_API37_BASELINE = ([0-9]+).*/\1/p' "$BASELINE_FILE" | head -1)"
-  # The #81 check, verbatim: what the tree actually carries. Reported next to the baseline so a
-  # stale baseline shows up here rather than only once the emulator disagrees with it.
+  # What the tree actually carries. Reported next to the baseline so a stale baseline shows up
+  # here rather than only once the emulator disagrees with it.
+  #
+  # ANCHORED AT LINE START, AND WHITESPACE-OR-END-OF-LINE AFTER THE NAME (#120). The #81 check
+  # this replaces matched the string anywhere on any line, so #113's KDoc reading `Deliberately
+  # not @FailsOnEmulatorApi37` counted as a fourth marked test and the report announced a
+  # deviation on every PR. That is worse than a wrong number: #83 built this so a new failure
+  # could not be invisible, and a notice that is wrong every time teaches everyone to skim past
+  # deviation notices.
+  #
+  # THE OBVIOUS REPAIR IS A TRAP, and the reason for the second half of the pattern.
+  # `^[[:space:]]*@NAME[[:space:]]*$` -- "the annotation on a line of its own" -- also stops
+  # counting `@FailsOnEmulatorApi37 @Test`, which is legal Kotlin, and UNDERcounting is the
+  # dangerous direction: it hides a genuine new marker, which is the one thing this exists to
+  # catch. Measured against `testdata/marker-shapes`, a fixture carrying every shape at once:
+  # the old matcher says 5, own-line-only says 2, this one says 3. On the real tree, 4 / 3 / 3.
+  # e2e-report-shape-test.sh runs that fixture through this whole script.
+  #
+  # `^[[:space:]]*@` cannot match an `import` line, so the old `grep -v import` goes with it
+  # rather than staying to imply a filter is still doing work.
+  #
+  # This is a regex over source text and not a parser. An annotation inside a multi-line string,
+  # or inside a `/* */` block that opened mid-line, would still be counted. Neither exists here;
+  # if one ever does, this check wants a different tool rather than a longer regex.
  if [ -d "$REPO_ROOT/app/src/androidTest" ]; then
-    marked="$(grep -rn "@FailsOnEmulatorApi37" "$REPO_ROOT/app/src/androidTest" --include='*.kt' \
-      | grep -v import | grep -c FailsOn || true)"
+    marked="$(grep -rhcE '^[[:space:]]*@FailsOnEmulatorApi37([[:space:]]|$)' \
+      "$REPO_ROOT/app/src/androidTest" --include='*.kt' \
+      | awk '{ total += $1 } END { print total + 0 }' || true)"
  fi
 fi

@@ -217,6 +268,11 @@ echo "----- RUN SHAPE (api${LABEL}) -----"
 echo "  expected:          $expected"
 echo "  received:          $received"
 echo "  failed:            $failed"
+# Above `completed cleanly`, because it is the line that says what happened to the leg and the
+# other one only qualifies it. A reader who stops after three rows still sees it.
+if [ -n "$WEDGED_AFTER" ]; then
+  echo "  wedged:            yes -- gradle was killed after ${WEDGED_AFTER}s and never returned"
+fi
 echo "  completed cleanly: ${completed//\*/}"
 if [ -n "$abort_received" ]; then
  echo "  received before the abort: $abort_received"
@@ -249,6 +305,9 @@ if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then
    echo "| expected | $expected | $expected_src |"
    echo "| received | $received | $received_src |"
    echo "| failed | $failed | $failed_src |"
+    if [ -n "$WEDGED_AFTER" ]; then
+      echo "| wedged | **yes** | \`timeout\` fired after ${WEDGED_AFTER}s and killed gradle (exit 124), which is what e2e-run.sh then captured the wedge diagnostics for |"
+    fi
    echo "| completed cleanly | $completed | $completed_src |"
    if [ -n "$abort_received" ]; then
      echo "| received before the abort | $abort_received | the same line — the XML above counts the truncated test as a failure, this number does not |"
@@ -239,20 +239,34 @@ timeout -k 30s "$WEDGE_TIMEOUT" \
  ${E2E_EXTRA_GRADLE_ARGS:-} 2>&1 | tee "$GRADLE_LOG" || status=$?
 echo "::endgroup::"

+# Whether the wrapper timeout fired, decided ONCE. 124 is `timeout` saying it killed the
+# command, and two places downstream need that fact: capture_wedge below, and the report, which
+# otherwise calls a killed leg `completed cleanly: yes` (#118). Deriving it twice is how those
+# two would drift apart -- the report would keep printing after someone changed what a wedge
+# means here. It stays a string: empty on every other path, so those legs pass an empty
+# E2E_WEDGED_AFTER and the report behaves exactly as before.
+wedged=""
+[ "$status" -eq 124 ] && wedged="$WEDGE_TIMEOUT"
+
 # The run-shape report: expected/received/failed and whether the run finished, every time,
 # green or red. It never changes `status` -- it is a diagnostic, and the header's rule about
 # diagnostics applies to it as much as to every probe below.
 #
+# E2E_WEDGED_AFTER is the wedge, told to the report rather than left for it to infer. It cannot
+# be inferred: a wedge is gradle never returning, so gradle printed no verdict at all, and the
+# log the report reads looks like a run that simply stopped. Only this script knows the
+# difference, because only this script saw the exit status.
+#
 # The baseline argument, and only it, turns on the comparison, and only the advisory API 37 job
 # passes E2E_ADVISORY=1. Comparing on the gating legs would announce a deviation on all five of
 # them every run, since they run the whole suite rather than the marked three. They still get
 # the report: a truncated run reporting fewer results than it ran is what #108 looks like, and
 # `completed cleanly` is the field that shows it.
 if [ "${E2E_ADVISORY:-}" = "1" ]; then
-  bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" \
+  E2E_WEDGED_AFTER="$wedged" bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" \
    "$REPO_ROOT/app/src/androidTest/java/org/libremediaconverter/FailsOnEmulatorApi37.kt" || true
 else
-  bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" || true
+  E2E_WEDGED_AFTER="$wedged" bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" || true
 fi

 if [ "$status" -eq 0 ]; then
@@ -260,7 +274,7 @@ if [ "$status" -eq 0 ]; then
  exit 0
 fi

-if [ "$status" -eq 124 ]; then
+if [ -n "$wedged" ]; then
  capture_wedge "api${LABEL}"
 else
  echo "::error::E2E api${LABEL} failed (exit $status)"
@@ -0,0 +1,54 @@
+// NOT A TEST, AND NEVER COMPILED. This is fixture data for e2e-report-shape-test.sh, which
+// copies it into a throwaway repo root and runs the real report script against that. It lives
+// under .github/ deliberately: Gradle only compiles app/src/**, ktlint and detekt are applied to
+// :app only, and the report's own count reads app/src/androidTest -- so nothing here can reach
+// the build, the linters, or the number the advisory job compares against. Verified by running
+// the report against the real repo root with this file committed: still 3.
+//
+// It carries every shape the counter has to tell apart, in one file, because the bug in #120 was
+// exactly that two of them look alike to a substring match. Three count and three must not:
+//
+//   COUNTS       the annotation on its own line
+//   COUNTS       the annotation sharing a line with @Test -- legal Kotlin, and the case the
+//                obvious "own line only" repair silently drops
+//   COUNTS       the annotation indented inside a nested class
+//   must NOT     a KDoc mentioning it -- this is #120 itself, copied from Media3EngineTest
+//   must NOT     a commented-out annotation
+//   must NOT     the import
+//
+// Three count. That is what the synthetic baseline in the test is set to, so the fixture and the
+// baseline agree exactly the way the real tree and FAILS_ON_EMULATOR_API37_BASELINE do.
+//
+// The `@Test` here is spelled the way a real test spells it so the fixture reads like source
+// rather than like a regex exercise. Nothing runs it.
+
+package org.libremediaconverter.fixture
+
+import org.junit.Test
+import org.libremediaconverter.FailsOnEmulatorApi37
+
+class MarkerShapes {
+    @FailsOnEmulatorApi37
+    @Test
+    fun ownLine() = Unit
+
+    @FailsOnEmulatorApi37 @Test
+    fun sameLineAsTest() = Unit
+
+    /**
+     * Deliberately not `@FailsOnEmulatorApi37`: nothing here decodes or encodes, so no emulator
+     * codec is involved and the API 37 image has no quarrel with it.
+     */
+    @Test
+    fun mentionedInKdoc() = Unit
+
+    // @FailsOnEmulatorApi37 -- taken off on 2026-01-01, kept as a note rather than deleted
+    @Test
+    fun commentedOut() = Unit
+
+    class Nested {
+        @FailsOnEmulatorApi37
+        @Test
+        fun indentedDeeper() = Unit
+    }
+}
@@ -13,6 +13,7 @@
 .externalNativeBuild
 .cxx
 local.properties
+.kotlin

 # The FFmpeg AAR is committed under bin/ so test runs do not depend on a rebuild.
 # Build outputs from tools/ffmpeg are not.
@@ -98,6 +98,13 @@ days. Read it as the current answer, and see the git history if you need the old
  is written anyway and says nothing about it — `.github/scripts/e2e-report-shape.sh` is where that
  is measured and explained.

+  Every leg prints that table, advisory or not, and **on the wedge path it also carries a `wedged:`
+  row** (#118). `completed cleanly` only ever meant "instrumentation was not aborted", which stays
+  true of a leg the `WEDGE_TIMEOUT` killed 22 minutes in — so without that row the table read
+  `received: 59, completed cleanly: yes` for a leg that had just died. The wedge is passed to the
+  report as `E2E_WEDGED_AFTER` by `e2e-run.sh`, which is the only thing that can know it: a wedge
+  is gradle never returning, so the log it left says nothing about it.
+
 Still true, and the reason the advisory job is not simply deleted: **API 37 needs a manual check on
 the Pixel 10 Pro XL before each release.** Those three tests are the one thing CI cannot answer
 for.
@@ -123,10 +130,10 @@ install for code that can never run — and on API 37 the full APK does not fit
 - The `model` package is excluded from `ReturnCount` and `CyclomaticComplexMethod` only. It is the
  decision layer, where one branch is one documented user-visible outcome and the metric counts
  answers rather than complexity. Every other rule still applies there.
- **Coverage is reported, not gated** — **69.2% of lines (1519/2194), 53.2% of branches**,
-  measured 2026-08-24 with `./gradlew :app:jacocoTestReport`.
+- **Coverage is reported, not gated** — **84.9% of lines (1971/2321), 63.8% of branches**,
+  measured 2026-08-26 with `./gradlew :app:jacocoTestReport`, against 454 JVM tests in 67 classes.

-  **Every figure this file carried before that date was an artifact, roughly half the real one.**
+  **Every figure this file carried before 2026-08-24 was an artifact, roughly half the real one.**
  Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo
  skips no-location classes by default, and nothing told it otherwise — so **not one Robolectric
  test counted**, and Robolectric is what exercises the framework edge here. The
@@ -140,9 +147,12 @@ install for code that can never run — and on API 37 the full APK does not fit
  disproportionately Robolectric, so each one added denominator and no numerator — the measurement
  was punishing exactly the tests that were hardest to write.

-  Two things still hold. A floor needs a baseline that has settled, and this one has now moved by
-  39 points in a single build change, so it has not. And **re-measure before quoting** — that
-  instruction is the only reason this was caught.
+  Two things still hold. A floor needs a baseline that has settled, and this one has not: it moved
+  39 points in a single build change on 2026-08-24, then another 16 as the #52 test push and the
+  fixes it turned up landed — 69.2% -> 84.9% line, 53.2% -> 63.8% branch — while the denominator
+  grew 2194 -> 2321, because that work added production code of its own. And **re-measure before
+  quoting**: this entry was written quoting 81.4%, measured four hours earlier, and was already
+  three points stale by the time it was ready to merge.
 - **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit
  tests before it counts as done. If it is e2e testable, it gets e2e tests. Both clauses apply —
  a change that is both needs both.
@@ -236,3 +246,12 @@ Because versions float, a build can change without a commit. `./gradlew :app:dep
  `@OptIn`. Android lint's `UnsafeOptInUsageError` catches a missed one.
 - **Release builds ship both ABIs.** `-PabiFilters` is a test-run override only; `build.yml`
  verifies the released APK carries every ABI and that all native libraries are 16 KB aligned.
+- **A JUnit `Timeout` — rule or `@Test(timeout=)` — cannot be used in the JVM suite.** Both run the
+  test body on a separate thread, and every Compose test here goes through Robolectric's paused
+  main looper: `UnsupportedOperationException: main looper can only be controlled from main
+  thread`, from `ShadowPausedLooper` under `RobolectricIdlingStrategy.runUntilIdle`. The identical
+  tests pass with the timeout removed, so it is the mechanism, not the test. What bounds a hung
+  run instead is `timeout` on the `Test` tasks plus the jstack watchdog beside it in
+  `app/build.gradle.kts`, neither of which moves a thread. `HangBoundTest` guards both numbers,
+  and **a timed-out run writes no XML for the class that hung** — the dump is its only
+  attribution, so do not delete the watchdog as stray config.
@@ -1,5 +1,7 @@
 import org.gradle.api.tasks.PathSensitivity
 import org.gradle.testing.jacoco.tasks.JacocoReport
+import java.io.File
+import java.time.Duration

 plugins {
    // Applied by id: these two come from the root buildscript classpath, which is what
@@ -217,6 +219,115 @@ tasks.withType<Test>().configureEach {
        .withPropertyName("releaseWorkflow")
        .withPathSensitivity(PathSensitivity.RELATIVE)

+    // Same reasoning, same trap: HangBoundTest reads the two numbers below out of this file, and
+    // they are the one part of the change that does not compile. Without this the task stays
+    // UP-TO-DATE when the build script changes, so the guard would go stale on exactly the edit
+    // it exists to catch.
+    inputs.file(project.file("build.gradle.kts"))
+        .withPropertyName("moduleBuildScript")
+        .withPathSensitivity(PathSensitivity.RELATIVE)
+
+    // --- Bounding a hung run (#125) -----------------------------------------------------------
+    //
+    // This suite had no timeout of any kind, so a hang ran until something outside it gave up.
+    // #125 is a real Java-level deadlock -- a lock-order inversion between Room's
+    // TransactionExecutor and WorkManager's SerialExecutorImpl, reached through the WorkInfo flow
+    // -- and one local run sat in it for 47 minutes. On CI it would burn the Unit tests job's
+    // 30-minute cap and report as a job timeout with no cause at all.
+    //
+    // WHY NOT A JUnit `Timeout` RULE, which is the obvious answer: it runs the test body on a
+    // separate thread, and this suite is thread-affine. Measured here, `@Rule Timeout` and
+    // `@Test(timeout = ...)` against a `createComposeRule()` Robolectric test both give:
+    //
+    //     java.lang.UnsupportedOperationException: main looper can only be controlled from main
+    //       at org.robolectric.shadows.ShadowPausedLooper.executeOnLooper
+    //       at androidx.compose.ui.test.RobolectricIdlingStrategy.runUntilIdle
+    //
+    // The same two tests with the timeout removed pass, so that is the mechanism and not the
+    // probe. Nothing that moves a test off its own thread can be used here.
+    //
+    // `Task.timeout` moves nothing -- it stops the forked test JVM from outside. Its weakness is
+    // that it kills without a thread dump, and the jstack is the only reason #125 could be named
+    // at all; the watchdog below is what answers that, and it only dumps.
+    //
+    // THE NUMBER, against the slowest observed *pass* rather than the typical one. Eight CI runs
+    // sampled 2026-08-26, whole `./gradlew :app:testDebugUnitTest` invocation with compilation in
+    // it and this task a subset: 62, 76, 77, 79, 81, 84, 86 and 90 seconds. Locally the task
+    // itself is ~11 s over 454 tests. Ten minutes is ~6.7x the slowest of those and a third of
+    // the job's 30-minute cap, so a fired timeout still has room to be reported and uploaded. It
+    // is deliberately nowhere near the observed duration: a timeout that fires on a healthy slow
+    // runner turns a real signal into noise and teaches people to re-run reflexively.
+    timeout.set(Duration.ofMinutes(10))
+
+    // The dump, two minutes before the kill. jstack is what turned #125 from "CI timed out" into
+    // a named lock-order inversion, and `Task.timeout` on its own would have thrown it away.
+    //
+    // It is deliberately incapable of failing a build: it reads a live process and writes a file.
+    // Nothing here kills, interrupts or signals anything, so the worst a misfire can do is leave a
+    // stack trace nobody needed. It has one, and it is the ordinary CI shape rather than an exotic
+    // case: the worker is found by scanning this daemon's descendants for GradleWorkerMain, which
+    // cannot tell one invocation's worker from the next, and the Unit tests job runs
+    // testDebugUnitTest and jacocoTestReport back to back against the same daemon. If this task's
+    // own worker lived and died inside a single poll, the watchdog can adopt the following one.
+    //
+    // Everything it needs is read here, at configuration time, and captured by value. Reaching
+    // back through the task or the project from inside the action would not survive the
+    // configuration cache, which `gradle.properties` turns on for every build.
+    val threadDump = layout.buildDirectory.file("reports/hang/$name-threads.txt").get().asFile
+    val taskPath = path
+    val dumpAfterNanos = Duration.ofMinutes(8).toNanos()
+    val captureWindowNanos = Duration.ofMinutes(1).toNanos()
+    val pollMillis = 1_000L
+    doFirst {
+        val watchdog = Thread {
+            val startedAt = System.nanoTime()
+            var worker: ProcessHandle? = null
+            while (true) {
+                Thread.sleep(pollMillis)
+                val elapsed = System.nanoTime() - startedAt
+                val watched = worker
+                if (watched == null) {
+                    // Gradle forks the worker moments after this task starts. If none has shown
+                    // up by the end of the capture window there is nothing to watch, and going on
+                    // polling would only risk adopting some other build's.
+                    if (elapsed > captureWindowNanos) return@Thread
+                    worker = ProcessHandle.current().descendants()
+                        .filter { it.info().commandLine().orElse("").contains("GradleWorkerMain") }
+                        .findFirst().orElse(null)
+                } else if (!watched.isAlive) {
+                    return@Thread // the run finished; this is the healthy exit
+                } else if (elapsed >= dumpAfterNanos) {
+                    val jstack = File(File(System.getProperty("java.home"), "bin"), "jstack")
+                    threadDump.parentFile.mkdirs()
+                    if (jstack.canExecute()) {
+                        ProcessBuilder(jstack.absolutePath, "-l", watched.pid().toString())
+                            .redirectErrorStream(true)
+                            .redirectOutput(threadDump)
+                            .start()
+                            .waitFor()
+                    } else {
+                        threadDump.writeText("no jstack at ${jstack.absolutePath}\n")
+                    }
+                    // To stdout as well as to the file, and that is the half that matters on CI:
+                    // the Unit tests job uploads app/build/reports/tests/ and nothing else, so a
+                    // dump that only ever existed under reports/hang/ would be unreachable from a
+                    // red run -- which is the "timed out with no cause" this exists to end. The
+                    // step log always survives, and needs no workflow edit to say so.
+                    println(
+                        "$taskPath is still running after ${Duration.ofNanos(elapsed).toMinutes()} " +
+                            "minutes and is about to be timed out. Thread dump of pid " +
+                            "${watched.pid()}, also written to $threadDump -- look for 'Found one " +
+                            "Java-level deadlock' (that is #125).\n" + threadDump.readText(),
+                    )
+                    return@Thread
+                }
+            }
+        }
+        watchdog.isDaemon = true
+        watchdog.name = "hang-watchdog"
+        watchdog.start()
+    }
+
    extensions.configure<JacocoTaskExtension> {
        isIncludeNoLocationClasses = true
        excludes = listOf("jdk.internal.*")
@@ -178,7 +178,12 @@ object ContainerCapabilities {
            if (!probe.hasVideo) {
                return Validation.Invalid(
                    "This file has no video track to copy.",
-                    listOf(spec.copy(videoCodec = VideoCodec.NONE)),
+                    // Dropping the video is the right shape of answer, but it is only half of one:
+                    // `spec.copy(videoCodec = NONE)` is valid exactly when the audio axis already
+                    // happened to be fine, and refused otherwise — a Vorbis or PCM source into MP4,
+                    // an MP3 into WebM. Handing it to the shared path repairs both axes and drops
+                    // anything that still fails, so the chip cannot lead to a second error.
+                    suggestions(spec.copy(videoCodec = VideoCodec.NONE), probe, exclude = spec),
                )
            }
            val source = CodecNames.videoFromName(probe.videoCodec)
@@ -0,0 +1,98 @@
+package org.libremediaconverter.ci
+
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import java.io.File
+
+/**
+ * That a hung unit-test run still ends by itself, and still says why.
+ *
+ * `:app:testDebugUnitTest` had no timeout of any kind until #125 was filed. That ticket is a real
+ * Java-level deadlock between Room's `TransactionExecutor` and WorkManager's `SerialExecutorImpl`,
+ * reached through the WorkInfo flow the ViewModel collects, and one local run sat in it for 47
+ * minutes. Nothing inside the suite could break it: the deadlock is monitor contention, which is
+ * not interruptible, so it runs until something outside the JVM gives up.
+ *
+ * Two numbers in `app/build.gradle.kts` are what bound it now, and neither compiles, so nothing
+ * else would notice their removal:
+ *
+ *  - `timeout.set(...)` on every `Test` task, which stops the forked test JVM.
+ *  - the watchdog's `dumpAfterNanos`, which jstacks that JVM *before* the timeout kills it.
+ *
+ * The second is the one worth guarding hardest, and the one that most looks like stray config.
+ * Gradle's timeout kills without a thread dump, and the jstack -- with its "Found one Java-level
+ * deadlock" section naming both monitors -- is the only reason #125 could be described at all.
+ * The ordering between the two numbers is what makes it work: dump first, kill second. Reverse
+ * them, or delete the watchdog, and the suite still stops hanging but every hang from then on
+ * reports as a bare "Timeout has been exceeded" with nothing to read. Measured against a probe
+ * that hung one test: no test XML was written for the class that hung, so the hanging test itself
+ * gets no attribution from the report at all.
+ *
+ * The range on the timeout is not decoration either, and it is the half a future edit is most
+ * likely to get wrong. Below it, a healthy-but-slow runner trips the bound and a real signal
+ * becomes noise people learn to re-run through; above it, CI's 30-minute job cap fires first and
+ * the bound never gets to say anything.
+ *
+ * `ReleasePermissionTest` is the precedent and its caveat applies here too. This asserts the two
+ * numbers are present, sanely sized and correctly ordered. It cannot assert that the timeout
+ * fires -- that needs a hang, which is what the whole change exists to prevent. Refs #125.
+ */
+class HangBoundTest {
+
+    @Test
+    fun `every Test task is bounded, and bounded between the slow runner and the job cap`() {
+        assertTrue(
+            "app/build.gradle.kts sets its Test task timeout to ${timeoutMinutes}m, which is " +
+                "outside $SANE_MINUTES. Under that range a slow CI runner trips a bound meant for " +
+                "deadlocks -- the slowest observed passing run of the whole invocation was 90s. " +
+                "Over it, the Unit tests job's own 30-minute cap kills the job first and the " +
+                "timeout never reports. `null` means the line is gone or the block was rewritten, " +
+                "and without it #125's deadlock has nothing to stop it: monitor contention breaks " +
+                "no interrupt, so it runs until CI gives up and reports a timeout with no cause.",
+            timeoutMinutes in SANE_MINUTES,
+        )
+    }
+
+    @Test
+    fun `the thread dump is taken before the timeout kills the JVM it would dump`() {
+        assertTrue(
+            "app/build.gradle.kts takes its hang thread dump after ${dumpAfterMinutes}m but times " +
+                "the task out at ${timeoutMinutes}m, so the JVM is already dead when jstack runs " +
+                "and every future hang reports as a bare `Timeout has been exceeded`. The dump " +
+                "has to come first -- it is the only attribution a hanging test gets, since the " +
+                "test XML never names it.",
+            (dumpAfterMinutes ?: 0) < (timeoutMinutes ?: 0),
+        )
+    }
+
+    /** Minutes given to a whole `Test` task before Gradle stops the forked JVM. */
+    private val timeoutMinutes: Int?
+        get() = minutesIn("""timeout\.set\(Duration\.ofMinutes\((\d+)\)\)""")
+
+    /** Minutes the watchdog waits before jstacking the forked JVM. */
+    private val dumpAfterMinutes: Int?
+        get() = minutesIn("""val dumpAfterNanos = Duration\.ofMinutes\((\d+)\)""")
+
+    /**
+     * Read out of the build script rather than from a model: the numbers live in a Kotlin DSL block
+     * that no unit test can instantiate, and a scan that reports `null` when the shape changes is a
+     * better trade than not checking them at all.
+     */
+    private fun minutesIn(pattern: String): Int? =
+        Regex(pattern).find(buildScript.readText())?.groupValues?.get(1)?.toInt()
+
+    /**
+     * Found by walking up rather than by a fixed relative path: Gradle's working directory for the
+     * unit tests is the module, but that is a default rather than a promise.
+     */
+    private val buildScript: File
+        get() = generateSequence(File(".").absoluteFile) { it.parentFile }
+            .map { File(it, "app/build.gradle.kts") }
+            .firstOrNull { it.isFile }
+            ?: error("could not find app/build.gradle.kts above ${File(".").absolutePath}")
+
+    private companion object {
+        /** Above the slowest observed passing run, below the Unit tests job's `timeout-minutes`. */
+        val SANE_MINUTES = 3..29
+    }
+}
@@ -0,0 +1,234 @@
+package org.libremediaconverter.convert
+
+import android.content.ComponentName
+import android.content.ContentProvider
+import android.content.ContentValues
+import android.content.Context
+import android.content.IntentFilter
+import android.content.pm.ProviderInfo
+import android.database.Cursor
+import android.database.MatrixCursor
+import android.net.Uri
+import android.os.Bundle
+import android.provider.DocumentsContract
+import android.provider.OpenableColumns
+import org.robolectric.Robolectric
+import org.robolectric.Shadows.shadowOf
+import java.io.File
+
+/**
+ * Content providers more than one test needs, and the registration dance they all repeat.
+ *
+ * Only that. A stub that serves one test stays in that test, next to the assertion it exists for —
+ * the rule `work/WorkerStubs.kt` states, and the reason `UnreliableOutputStream` is still private to
+ * `OutputPublisherPublishTest`.
+ *
+ * These started life inside `OutputPublisherPublishTest`, which is the only thing that needed a
+ * provider at all. They moved here when `InputQuery`'s cursor reads turned out to need the same
+ * provider answering *badly* — see [RowShape].
+ */
+
+internal const val DOCUMENTS_AUTHORITY = "org.libremediaconverter.test.documents"
+internal const val PLAIN_AUTHORITY = "org.libremediaconverter.test.plain"
+
+/**
+ * How [FakeSafProvider] answers a metadata query.
+ *
+ * A provider is another app. It can be uninstalled, revoke its grant, crash, or simply answer
+ * something the caller did not expect — and "answered something unexpected" is not one case but
+ * several, which is why this is an enum rather than a boolean.
+ *
+ * The distinction that matters most to callers is **null versus missing versus zero versus
+ * negative**. `InputQuery` exists to stop the last three being conflated: `hasSpaceFor(0)` is only
+ * "is there 128 MB free", so a size nobody could determine must not arrive as `0`, and
+ * `OutputPublisher.destinationIsKnownEmpty` must answer `false` — never "empty, go ahead and
+ * delete" — for every one of them.
+ *
+ * Column-level granularity is deliberate. `OutputPublisher` reads only `SIZE`; `InputQuery` reads
+ * both, and reaches a different answer depending on which one is bad.
+ */
+internal enum class RowShape {
+    /** What a healthy provider answers: the file's real name and real length. */
+    NORMAL,
+
+    /** A row is present and its `DISPLAY_NAME` cell is null. */
+    NULL_DISPLAY_NAME,
+
+    /** A row is present and its `SIZE` cell is null. */
+    NULL_SIZE,
+
+    /** The cursor carries no `DISPLAY_NAME` column at all — `getColumnIndex` gives `-1`. */
+    NO_DISPLAY_NAME_COLUMN,
+
+    /** The cursor carries no `SIZE` column at all — `getColumnIndex` gives `-1`. */
+    NO_SIZE_COLUMN,
+
+    /**
+     * A size of `-1`.
+     *
+     * Not a corrupt provider: it is what anything without a fixed length reports — a pipe, or a
+     * provider streaming its answer — and it is a third way of saying "unknown", distinct from a
+     * null cell and from a missing column.
+     */
+    NEGATIVE_SIZE,
+
+    /**
+     * A cursor with the right columns and no rows in it.
+     *
+     * Distinct from returning `null`, which is what a provider that does not recognise the URI
+     * does. Both mean "no answer", and code that treats one as an answer and the other as an
+     * absence is wrong about one of them.
+     */
+    NO_ROWS,
+
+    /**
+     * The query itself throws.
+     *
+     * A resolver call is a call into another app, and that app can have been uninstalled, revoked
+     * its grant, or simply crashed. `InputQuery.firstRow`'s KDoc is explicit that "a file picker is
+     * not a place to bring the process down from", so this is the shape that proves the guard is
+     * one.
+     */
+    QUERY_THROWS,
+}
+
+/**
+ * A stand-in for the provider behind a SAF destination.
+ *
+ * It answers only what its callers ask of a document -- how many bytes are already there, what it
+ * is called, and delete it -- backed by a real file so the assertions are about the filesystem
+ * rather than about a mock's call log alone. The rest of the `ContentProvider` surface is stubbed.
+ *
+ * Writing is deliberately NOT routed through it. Robolectric's `ShadowContentResolver`
+ * consults its registered-stream map before it reaches any provider, which is what lets a
+ * test hand out a stream that writes some bytes and then fails -- a condition a real provider
+ * cannot be asked to produce on demand.
+ */
+internal open class FakeSafProvider : ContentProvider() {
+
+    override fun onCreate() = true
+
+    override fun query(
+        uri: Uri,
+        projection: Array<out String>?,
+        selection: String?,
+        selectionArgs: Array<out String>?,
+        sortOrder: String?,
+    ): Cursor? {
+        if (rowShape == RowShape.QUERY_THROWS) throw SecurityException("provider revoked the grant")
+        val file = backingFile(uri)
+        if (!file.exists()) return null
+        return MatrixCursor(columnsFor(rowShape)).apply {
+            if (rowShape != RowShape.NO_ROWS) addRow(cellsFor(rowShape, file))
+        }
+    }
+
+    override fun call(method: String, arg: String?, extras: Bundle?): Bundle? {
+        if (method != METHOD_DELETE_DOCUMENT) return null
+        val target = extras?.getParcelable(EXTRA_URI, Uri::class.java) ?: return null
+        deleteRequests += target
+        deleteFailure?.let { throw it }
+        backingFile(target).delete()
+        return Bundle()
+    }
+
+    override fun getType(uri: Uri) = "video/mp4"
+
+    override fun insert(uri: Uri, values: ContentValues?): Uri? = null
+
+    override fun delete(uri: Uri, selection: String?, selectionArgs: Array<out String>?) = 0
+
+    override fun update(uri: Uri, values: ContentValues?, selection: String?, selectionArgs: Array<out String>?) = 0
+
+    companion object {
+        // DocumentsContract.METHOD_DELETE_DOCUMENT and EXTRA_URI are hidden from the public
+        // SDK, so they cannot be referenced. These are the wire names
+        // DocumentsContract.deleteDocument() actually sends, which is what a provider sees.
+        const val METHOD_DELETE_DOCUMENT = "android:deleteDocument"
+        const val EXTRA_URI = "uri"
+
+        /** Where the "documents" really live. Set per test to a Robolectric temp path. */
+        lateinit var root: File
+
+        /** Every delete this provider was asked for, in order. Empty is an assertion too. */
+        val deleteRequests = mutableListOf<Uri>()
+
+        /** Armed by the test that needs the cleanup itself to fail. */
+        var deleteFailure: RuntimeException? = null
+
+        /**
+         * How the next query answers. [reset] puts it back to [RowShape.NORMAL], so a test that
+         * does not care never has to think about it.
+         */
+        var rowShape: RowShape = RowShape.NORMAL
+
+        fun backingFile(uri: Uri) = File(root, uri.lastPathSegment.orEmpty())
+
+        fun reset(directory: File) {
+            root = directory
+            deleteRequests.clear()
+            deleteFailure = null
+            rowShape = RowShape.NORMAL
+        }
+
+        private fun columnsFor(shape: RowShape): Array<String> = when (shape) {
+            RowShape.NO_DISPLAY_NAME_COLUMN -> arrayOf(OpenableColumns.SIZE)
+            RowShape.NO_SIZE_COLUMN -> arrayOf(OpenableColumns.DISPLAY_NAME)
+            else -> arrayOf(OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE)
+        }
+
+        private fun cellsFor(shape: RowShape, file: File): Array<Any?> = when (shape) {
+            RowShape.NO_DISPLAY_NAME_COLUMN -> arrayOf(file.length())
+            RowShape.NO_SIZE_COLUMN -> arrayOf<Any?>(file.name)
+            RowShape.NULL_DISPLAY_NAME -> arrayOf(null, file.length())
+            RowShape.NULL_SIZE -> arrayOf(file.name, null)
+            RowShape.NEGATIVE_SIZE -> arrayOf(file.name, UNKNOWN_LENGTH)
+            else -> arrayOf(file.name, file.length())
+        }
+
+        /** What `statSize` reports for anything without a fixed length. See [RowShape.NEGATIVE_SIZE]. */
+        private const val UNKNOWN_LENGTH = -1L
+    }
+}
+
+/**
+ * The same provider, registered WITHOUT the documents-provider intent filter.
+ *
+ * A separate class because the package manager keys providers by component name, so two
+ * authorities need two components. It exists to prove the guard is a guard: a content URI
+ * from something that is not a documents provider must not be handed to `deleteDocument`.
+ */
+internal class FakePlainProvider : FakeSafProvider()
+
+/**
+ * Stands [provider] up on [authority] so `contentResolver` and the package manager both know it.
+ *
+ * `isDocumentUri()` does not look at the URI alone: it asks the package manager whether anything
+ * answers `ACTION_DOCUMENTS_PROVIDER` for that authority. Registering the provider with the
+ * resolver is not enough, which is the whole reason [asDocumentsProvider] is a parameter rather
+ * than always true — the negative case is a test.
+ */
+internal fun registerProvider(
+    context: Context,
+    provider: Class<out FakeSafProvider>,
+    authority: String,
+    asDocumentsProvider: Boolean,
+) {
+    val info = ProviderInfo().apply {
+        this.authority = authority
+        packageName = context.packageName
+        name = provider.name
+        exported = true
+        grantUriPermissions = true
+    }
+    Robolectric.buildContentProvider(provider).create(info)
+
+    val packageManager = shadowOf(context.packageManager)
+    packageManager.addOrUpdateProvider(info)
+    if (asDocumentsProvider) {
+        packageManager.addIntentFilterForProvider(
+            ComponentName(context.packageName, provider.name),
+            IntentFilter(DocumentsContract.PROVIDER_INTERFACE),
+        )
+    }
+}
@@ -0,0 +1,187 @@
+package org.libremediaconverter.convert
+
+import android.content.Context
+import android.net.Uri
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import java.io.File
+
+/**
+ * What [InputQuery] makes of a metadata row.
+ *
+ * ## Why this is a separate file from `UnknownInputSizeTest`
+ *
+ * That test drives the case where **no provider is registered** — the query returns null and
+ * `measure()` answers instead — and it drives it thoroughly. What it never does is hand `InputQuery`
+ * a row. Before this file, nothing did: `firstRow`'s body, `displayNameOrNull` and `sizeOrNull` had
+ * never executed in the JVM suite, so every branch inside them was untested.
+ *
+ * ## What is actually being pinned
+ *
+ * Not "does it read a cursor" — that would pass against almost any implementation. The rule is that
+ * **a size nobody could determine must not arrive as a number**, and there are four separate ways a
+ * provider fails to determine one: a null cell, a missing column, a negative value, and no row at
+ * all. `InputQuery`'s KDoc states the stake:
+ *
+ * > a worker's input `Data` carries the size the *picker* found … `hasSpaceFor(0)` is only "is there
+ * > 128 MB free".
+ *
+ * So each of those four must produce `null`, and `null` specifically — not `0`, not `-1`. A test
+ * that asserted only "not the file's length" would pass on `0`, which is the exact conflation the
+ * class exists to end.
+ *
+ * ## Why every fall-through lands on null here
+ *
+ * [FakeSafProvider] does not implement `openFile`, so `measure()` cannot answer for these URIs
+ * either. That is deliberate: it isolates the cursor half. The other direction — the cursor says
+ * nothing and `measure()` succeeds — is `UnknownInputSizeTest`'s
+ * `a picked file no provider describes is measured rather than reported as empty`, and is not
+ * repeated here.
+ *
+ * ## What the mutations say, including the one that does not bite
+ *
+ * Measured against `MatrixCursor`, which is what these tests drive:
+ *
+ * | call on a null cell | result |
+ * |---|---|
+ * | `getString` | returns `null` |
+ * | `getLong` | returns **`0`** |
+ *
+ * That second row is why `sizeOrNull`'s `!isNull(it)` guard is load-bearing and why these tests
+ * bite: remove it and a null size arrives as `0`, a real number indistinguishable from an empty
+ * file, which is the precise conflation this class exists to end. Removing it reddens
+ * `a null size is unknown rather than zero`. Removing the trailing `takeIf { it >= 0 }` reddens
+ * `a negative size is unknown rather than reported`.
+ *
+ * **Named exemption: `displayNameOrNull`'s `!isNull(it)` guard is not pinned by anything here, and
+ * cannot be.** `getString` returns null for a null cell, so the fallback applies with or without
+ * the guard — removing it leaves every test in this file green. The guard is not redundant in
+ * production: `Cursor.getString`'s contract states that whether it throws on a null column is
+ * *implementation-defined*, and a real `ContentProvider` is free to throw where `MatrixCursor`
+ * returns null. It should stay. It simply cannot be falsified with this cursor, and saying so is
+ * better than implying `a null display name falls back without disturbing the size` covers it —
+ * that test pins the behaviour, not the guard.
+ */
+@RunWith(RobolectricTestRunner::class)
+class InputQueryCursorTest {
+
+    private lateinit var context: Context
+    private lateinit var uri: Uri
+
+    @Before
+    fun setUp() {
+        context = RuntimeEnvironment.getApplication()
+        FakeSafProvider.reset(File(context.cacheDir, "picked").apply { mkdirs() })
+        registerProvider(context, FakeSafProvider::class.java, DOCUMENTS_AUTHORITY, asDocumentsProvider = true)
+        uri = Uri.parse("content://$DOCUMENTS_AUTHORITY/document/holiday.mp4")
+        FakeSafProvider.backingFile(uri).writeBytes(ByteArray(PAYLOAD_BYTES))
+    }
+
+    @Test
+    fun `a provider that answers properly supplies both the name and the size`() {
+        val described = InputQuery.describe(context, uri)
+
+        assertEquals("holiday.mp4", described.displayName)
+        assertEquals(PAYLOAD_BYTES.toLong(), described.sizeBytes)
+    }
+
+    @Test
+    fun `a null display name falls back without disturbing the size`() {
+        FakeSafProvider.rowShape = RowShape.NULL_DISPLAY_NAME
+
+        val described = InputQuery.describe(context, uri)
+
+        assertEquals(InputQuery.FALLBACK_DISPLAY_NAME, described.displayName)
+        // The two columns are read independently. A provider that cannot name the file can still
+        // size it, and losing the size here would be a bug the name assertion alone would miss.
+        assertEquals(PAYLOAD_BYTES.toLong(), described.sizeBytes)
+    }
+
+    @Test
+    fun `a cursor with no display name column falls back rather than throwing`() {
+        // getColumnIndex returns -1 rather than throwing, so the `it >= 0` guard is the only thing
+        // between this and an IllegalArgumentException out of getString.
+        FakeSafProvider.rowShape = RowShape.NO_DISPLAY_NAME_COLUMN
+
+        val described = InputQuery.describe(context, uri)
+
+        assertEquals(InputQuery.FALLBACK_DISPLAY_NAME, described.displayName)
+        assertEquals(PAYLOAD_BYTES.toLong(), described.sizeBytes)
+    }
+
+    @Test
+    fun `a null size is unknown rather than zero`() {
+        FakeSafProvider.rowShape = RowShape.NULL_SIZE
+
+        assertNull(unknownSizeMessage("a null cell"), InputQuery.sizeOf(context, uri))
+    }
+
+    @Test
+    fun `a cursor with no size column is unknown rather than zero`() {
+        FakeSafProvider.rowShape = RowShape.NO_SIZE_COLUMN
+
+        assertNull(unknownSizeMessage("a missing column"), InputQuery.sizeOf(context, uri))
+    }
+
+    @Test
+    fun `a negative size is unknown rather than reported`() {
+        // What anything without a fixed length reports -- a pipe, or a provider streaming its
+        // answer. Passing -1 through would be worse than passing 0: hasSpaceFor compares it
+        // against free space, so it would read as "needs less than nothing".
+        FakeSafProvider.rowShape = RowShape.NEGATIVE_SIZE
+
+        assertNull(unknownSizeMessage("a negative size"), InputQuery.sizeOf(context, uri))
+    }
+
+    @Test
+    fun `a cursor with no rows is unknown rather than zero`() {
+        // Distinct from the provider returning null, which UnknownInputSizeTest covers. A cursor
+        // that exists and holds nothing still has to reach the same answer.
+        FakeSafProvider.rowShape = RowShape.NO_ROWS
+
+        val described = InputQuery.describe(context, uri)
+
+        assertEquals(InputQuery.FALLBACK_DISPLAY_NAME, described.displayName)
+        assertNull(unknownSizeMessage("an empty cursor"), described.sizeBytes)
+    }
+
+    @Test
+    fun `a provider that throws is survived rather than propagated`() {
+        // The guard firstRow's KDoc exists for: "a resolver call is a call into another app ... and
+        // a file picker is not a place to bring the process down from". Without the runCatching,
+        // this SecurityException reaches the caller and takes the pick with it.
+        FakeSafProvider.rowShape = RowShape.QUERY_THROWS
+
+        val described = InputQuery.describe(context, uri)
+
+        assertEquals(InputQuery.FALLBACK_DISPLAY_NAME, described.displayName)
+        assertNull(unknownSizeMessage("a provider that threw"), described.sizeBytes)
+    }
+
+    @Test
+    fun `a join total is unknown when any one input could not be sized`() {
+        // The consequence the four cases above exist for, asserted once at the place it lands.
+        // Summing the inputs that did answer would produce a lower bound indistinguishable from a
+        // real total, which is what the space check cannot tell apart.
+        FakeSafProvider.rowShape = RowShape.NULL_SIZE
+        val unsizable = InputQuery.sizeOf(context, uri)
+        FakeSafProvider.rowShape = RowShape.NORMAL
+        val sizable = InputQuery.sizeOf(context, uri)
+
+        assertEquals(PAYLOAD_BYTES.toLong(), sizable)
+        assertNull(unsizable)
+        assertNull("one unknown input makes the whole total unknown", InputQuery.total(listOf(sizable, unsizable)))
+    }
+
+    private fun unknownSizeMessage(cause: String) =
+        "$cause means nobody could size the file; that must be null, not 0 -- hasSpaceFor(0) is only a headroom check"
+
+    private companion object {
+        const val PAYLOAD_BYTES = 4096
+    }
+}
@@ -1,17 +1,7 @@
 package org.libremediaconverter.convert

-import android.content.ComponentName
-import android.content.ContentProvider
-import android.content.ContentValues
 import android.content.Context
-import android.content.IntentFilter
-import android.content.pm.ProviderInfo
-import android.database.Cursor
-import android.database.MatrixCursor
 import android.net.Uri
-import android.os.Bundle
-import android.provider.DocumentsContract
-import android.provider.OpenableColumns
 import org.junit.Assert.assertArrayEquals
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
@@ -20,7 +10,6 @@ import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
-import org.robolectric.Robolectric
 import org.robolectric.RobolectricTestRunner
 import org.robolectric.RuntimeEnvironment
 import org.robolectric.Shadows.shadowOf
@@ -31,91 +20,6 @@ import java.io.OutputStream
 /** What a destination volume says when it fills up mid-write. */
 private const val NO_SPACE = "No space left on device"

-private const val DOCUMENTS_AUTHORITY = "org.libremediaconverter.test.documents"
-private const val PLAIN_AUTHORITY = "org.libremediaconverter.test.plain"
-
-/**
- * A stand-in for the provider behind a SAF destination.
- *
- * It answers only what `publish()` asks of a destination -- how many bytes are already there,
- * and delete it -- backed by a real file so the assertions are about the filesystem rather
- * than about a mock's call log alone. The rest of the `ContentProvider` surface is stubbed.
- *
- * Writing is deliberately NOT routed through it. Robolectric's `ShadowContentResolver`
- * consults its registered-stream map before it reaches any provider, which is what lets a
- * test hand out a stream that writes some bytes and then fails -- the condition this whole
- * file exists for, and one a real provider cannot be asked to produce on demand.
- */
-internal open class FakeSafProvider : ContentProvider() {
-
-    override fun onCreate() = true
-
-    override fun query(
-        uri: Uri,
-        projection: Array<out String>?,
-        selection: String?,
-        selectionArgs: Array<out String>?,
-        sortOrder: String?,
-    ): Cursor? {
-        val file = backingFile(uri)
-        if (!file.exists()) return null
-        return MatrixCursor(arrayOf(OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE)).apply {
-            addRow(arrayOf<Any?>(file.name, file.length()))
-        }
-    }
-
-    override fun call(method: String, arg: String?, extras: Bundle?): Bundle? {
-        if (method != METHOD_DELETE_DOCUMENT) return null
-        val target = extras?.getParcelable(EXTRA_URI, Uri::class.java) ?: return null
-        deleteRequests += target
-        deleteFailure?.let { throw it }
-        backingFile(target).delete()
-        return Bundle()
-    }
-
-    override fun getType(uri: Uri) = "video/mp4"
-
-    override fun insert(uri: Uri, values: ContentValues?): Uri? = null
-
-    override fun delete(uri: Uri, selection: String?, selectionArgs: Array<out String>?) = 0
-
-    override fun update(uri: Uri, values: ContentValues?, selection: String?, selectionArgs: Array<out String>?) = 0
-
-    companion object {
-        // DocumentsContract.METHOD_DELETE_DOCUMENT and EXTRA_URI are hidden from the public
-        // SDK, so they cannot be referenced. These are the wire names
-        // DocumentsContract.deleteDocument() actually sends, which is what a provider sees.
-        const val METHOD_DELETE_DOCUMENT = "android:deleteDocument"
-        const val EXTRA_URI = "uri"
-
-        /** Where the "documents" really live. Set per test to a Robolectric temp path. */
-        lateinit var root: File
-
-        /** Every delete this provider was asked for, in order. Empty is an assertion too. */
-        val deleteRequests = mutableListOf<Uri>()
-
-        /** Armed by the test that needs the cleanup itself to fail. */
-        var deleteFailure: RuntimeException? = null
-
-        fun backingFile(uri: Uri) = File(root, uri.lastPathSegment.orEmpty())
-
-        fun reset(directory: File) {
-            root = directory
-            deleteRequests.clear()
-            deleteFailure = null
-        }
-    }
-}
-
-/**
- * The same provider, registered WITHOUT the documents-provider intent filter.
- *
- * A separate class because the package manager keys providers by component name, so two
- * authorities need two components. It exists to prove the guard is a guard: a content URI
- * from something that is not a documents provider must not be handed to `deleteDocument`.
- */
-internal class FakePlainProvider : FakeSafProvider()
-
 /**
 * A sink that behaves like a volume filling up.
 *
@@ -179,8 +83,8 @@ class OutputPublisherPublishTest {
    fun setUp() {
        context = RuntimeEnvironment.getApplication()
        FakeSafProvider.reset(File(context.cacheDir, "destinations").apply { mkdirs() })
-        register(FakeSafProvider::class.java, DOCUMENTS_AUTHORITY, asDocumentsProvider = true)
-        register(FakePlainProvider::class.java, PLAIN_AUTHORITY, asDocumentsProvider = false)
+        registerProvider(context, FakeSafProvider::class.java, DOCUMENTS_AUTHORITY, asDocumentsProvider = true)
+        registerProvider(context, FakePlainProvider::class.java, PLAIN_AUTHORITY, asDocumentsProvider = false)

        // SAF's CreateDocument contract hands back a document that already exists and is
        // empty, so that is the state every destination starts in here.
@@ -326,28 +230,4 @@ class OutputPublisherPublishTest {
            )
        }
    }
-
-    private fun register(provider: Class<out FakeSafProvider>, authority: String, asDocumentsProvider: Boolean) {
-        val info = ProviderInfo().apply {
-            this.authority = authority
-            packageName = context.packageName
-            name = provider.name
-            exported = true
-            grantUriPermissions = true
-        }
-        Robolectric.buildContentProvider(provider).create(info)
-
-        // isDocumentUri() does not look at the URI alone: it asks the package manager whether
-        // anything answers ACTION_DOCUMENTS_PROVIDER for that authority. Registering the
-        // provider with the resolver is not enough, which is the whole reason the negative
-        // case above can exist.
-        val packageManager = shadowOf(context.packageManager)
-        packageManager.addOrUpdateProvider(info)
-        if (asDocumentsProvider) {
-            packageManager.addIntentFilterForProvider(
-                ComponentName(context.packageName, provider.name),
-                IntentFilter(DocumentsContract.PROVIDER_INTERFACE),
-            )
-        }
-    }
 }
@@ -11,6 +11,11 @@ import org.junit.Test
 * `OutputFormat` used to be twelve hand-picked triples, and its KDoc defended that on the grounds
 * that a closed set was what made routing decidable. Opening it up moves that burden here, so this
 * is where decidability now has to be proven.
+ *
+ * That includes what a refusal offers instead. `Validation.Invalid` promises every suggestion is
+ * itself valid and names this class as the proof, so a branch that assembles its own suggestion
+ * list rather than going through `suggestions()` is only checked here if some row happens to reach
+ * it — which is how a dead-end chip survived two widenings of that table.
 */
 class ContainerCapabilitiesTest {

@@ -35,6 +40,29 @@ class ContainerCapabilitiesTest {
        container = Container.MP3,
    )

+    /**
+     * An audio-only input carrying a codec MP4 has no place for at all.
+     *
+     * Vorbis lives in Ogg and Matroska; MP4 carries AAC, MP3, Opus and FLAC. That gap is what turns
+     * a suggestion which merely drops the video track into a second refusal.
+     */
+    private val vorbisSource = InputProbe(
+        videoCodec = null,
+        audioCodec = "vorbis",
+        hasVideo = false,
+        kind = InputKind.AUDIO_ONLY,
+        container = Container.OGG,
+    )
+
+    /** The same shape, for the other codec MP4 refuses. One case is a coincidence; two is the rule. */
+    private val pcmSource = InputProbe(
+        videoCodec = null,
+        audioCodec = "pcm_s16le",
+        hasVideo = false,
+        kind = InputKind.AUDIO_ONLY,
+        container = Container.WAV,
+    )
+
    // --- copy and encode are different questions ----------------------------

    /**
@@ -97,7 +125,17 @@ class ContainerCapabilitiesTest {
        }
    }

-    /** A suggestion that is itself invalid is worse than no suggestion. */
+    /**
+     * A suggestion that is itself invalid is worse than no suggestion.
+     *
+     * Only a branch that assembles its own suggestion list can break that promise: [suggestions]
+     * ends by filtering on `validate(...).isValid`, so everything routed through it is valid by
+     * construction. Those branches are what this table has to cover — the image output, and copy
+     * the video from a file that has none, which built its list by hand and came back refused for
+     * a Vorbis or PCM source into MP4 and an MP3 into WebM. The Advanced picker showed a one-tap
+     * fix that led straight to a second error, through two widenings of this table that never
+     * reached the branch.
+     */
    @Test
    fun `every suggestion is itself valid`() {
        val cases = listOf(
@@ -109,15 +147,31 @@ class ContainerCapabilitiesTest {
            OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.NONE) to mp3Source,
            OutputSpec(Container.MP4, VideoCodec.NONE, AudioCodec.NONE) to mp3Source,
            OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.AAC) to mp3Source,
+            // Copy-the-video-from-a-file-with-no-video, the last branch that built its offer by
+            // hand. It escaped the five rows above because `spec.copy(videoCodec = NONE)` is valid
+            // exactly when the audio axis happens to be fine — true for the AAC and MP3 sources
+            // used there, false for any audio the target container cannot carry.
+            OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.COPY) to vorbisSource,
+            OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.COPY) to pcmSource,
+            OutputSpec(Container.WEBM, VideoCodec.COPY, AudioCodec.COPY) to mp3Source,
+            // The same branch with audio the container *can* hold, which is the half that already
+            // worked and must keep working: the repair here is a copy, so the offer is the very
+            // spec the caller handed to `suggestions`. It survives only because the exclusion is
+            // against what the user asked for rather than against the repair.
+            OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.COPY) to mp3Source,
+            // The one branch that still builds its list by hand, so that it is asserted rather
+            // than merely reasoned about: an image container takes `None + None` and nothing else,
+            // which makes its single offer valid by construction.
+            OutputSpec(Container.GIF, VideoCodec.H264, AudioCodec.AAC) to h264Source,
        )

        cases.forEach { (spec, probe) ->
            val invalid = ContainerCapabilities.validate(spec, probe) as? Validation.Invalid
                ?: throw AssertionError("expected $spec to be rejected")
-            assertTrue("no alternatives offered for $spec", invalid.suggestions.isNotEmpty())
+            assertTrue("no alternatives offered for $spec on $probe", invalid.suggestions.isNotEmpty())
            invalid.suggestions.forEach { suggestion ->
                assertTrue(
-                    "suggested $suggestion for $spec is itself invalid",
+                    "suggested $suggestion for $spec on $probe is itself invalid",
                    ContainerCapabilities.validate(suggestion, probe).isValid,
                )
            }
@@ -0,0 +1,327 @@
+# Coverage-read findings
+
+**Status:** five findings, none fixed, none urgent. F5 was added on 2026-08-27, found while decomposing #132 into children — it had been listed there as a test gap, and is not one. Every entry here is a *code* observation —
+something a test would document rather than repair. The test gaps found in the same read are
+tickets #132 and #133, not entries here; see [Not covered here](#not-covered-here).
+**Scope:** what a JaCoCo read on 2026-08-26 turned up that writing a test would not fix. This is
+a survey, not a work order. Acting on any entry is a separate decision and would be its own commit.
+**Last verified:** `main` at `dc8b7c3`, 2026-08-26. Coverage re-measured that day with
+`./gradlew :app:jacocoTestReport`: **84.9% line (1971/2321), 63.8% branch (900/1410)**, against
+**456 JVM tests in 68 classes**. `CLAUDE.md` quotes 454 in 67 from four hours earlier; the
+percentages are unchanged, so no figure there is stale.
+
+## Why this document is separate from `defect-audit.md`
+
+`defect-audit.md` is the record of the 2026-08-22 defect sweep: sixteen entries, each a thing that
+is *wrong at runtime*. Nothing here is wrong at runtime today. These are arms that cannot be
+reached, accessors nobody calls, and one KDoc that contradicts the code beside it — the category
+`defect-audit.md` calls **latent**, plus one that is not a defect at all and is recorded so the
+next coverage read does not re-file it.
+
+They are here rather than in that document because folding them in would inflate a sixteen-entry
+audit whose status metadata has already gone stale once, and because they share a provenance:
+every one fell out of reading a coverage report, and every one is the kind of thing a coverage
+report is *good* at surfacing and a test is bad at fixing. F5 is the clearest case — it was filed
+as a test gap first, and only stopped being one when someone went looking for its callers.
+
+Entry ids are `F1`–`F5` so they cannot be confused with `defect-audit.md`'s `D1`–`D16`.
+
+## How to read the confidence labels
+
+Same vocabulary as `defect-audit.md`, deliberately, so the two read alike:
+
+- **Confirmed by inspection** — the control flow is fully readable and the finding follows from it.
+- **Latent** — not reachable through today's UI, but wrong, and one change away from being live.
+- **No action** — recorded because it looks like a finding and is not.
+
+Nothing below was observed on a device, and nothing below needs to be: every entry is a claim about
+what the code says, checkable by reading it.
+
+---
+
+## F1 — `FFmpegCommandBuilder` emits a Vorbis encoder that `ContainerCapabilities` says does not exist
+
+**Severity: low · Latent · the more interesting reading is a missing feature, not dead code**
+
+```
+app/src/main/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilder.kt:188
+app/src/main/java/org/libremediaconverter/model/ContainerCapabilities.kt:84-91
+```
+
+`FFmpegCommandBuilder.audioArgs` carries a live Vorbis arm:
+
+```kotlin
+AudioCodec.VORBIS -> listOf("-c:a", "libvorbis", "-q:a", "5")
+```
+
+`ContainerCapabilities` states, immediately above the set that governs it, that no such thing
+exists:
+
+> `/** Vorbis is absent for the same reason: nothing here emits a Vorbis encoder. */`
+> `private val ENCODABLE_AUDIO = setOf(AAC, OPUS, MP3, FLAC, PCM)`
+
+One of those two is wrong. The comment is the one that is wrong as written — something here does
+emit a Vorbis encoder, twelve lines of `FFmpegCommandBuilder`.
+
+### Why the arm is unreachable today
+
+Traced, not assumed:
+
+| step | where | effect |
+|---|---|---|
+| `validate` runs before routing | `ConversionWorker.kt:123` | a spec is checked on every job, however it was enqueued |
+| `validateAudio` refuses non-encodable | `ContainerCapabilities.kt:246-251` | `VORBIS !in ENCODABLE_AUDIO` → `Invalid("This app cannot encode Vorbis audio.")` |
+| the only spec→plan encode path | `CopyPlanner.kt:104` | `AudioPlan.Encode(requested)` — but `requested` cannot be Vorbis by the row above |
+| the fallback encode path | `CopyPlanner.kt:112-115` | draws from `encodableAudio(container)`, itself filtered by `ENCODABLE_AUDIO` |
+
+So `AudioPlan.Encode(VORBIS)` is not constructible through the app, and line 188 is dead.
+
+### The reading that matters more
+
+`CARRIES_AUDIO` lists Vorbis for WebM (`ContainerCapabilities.kt:62`) and OGG (`:67`). Because
+`encodableAudio` filters through `ENCODABLE_AUDIO`, the picker offers **Opus and nothing else** for
+WebM, and Opus/FLAC for OGG. FFmpeg on this device can encode Vorbis — the command is written and
+correct — and the app declines to offer it.
+
+So the honest framing is not "delete a dead arm". It is: **is `ENCODABLE_AUDIO`'s omission of
+Vorbis a deliberate product call, or an accident that has been costing WebM/OGG users a format the
+app already supports?** Nothing in the repo records that decision.
+
+### The precedent for whichever way it goes
+
+`Media3Engine.audioMimeTypeFor` has the *same* Vorbis arm, and handles it exactly right
+(`Media3Engine.kt:221-233`): the KDoc names it dead, says why the arm stays anyway ("deleting a
+right answer out of unreachable code buys nothing"), and points at `Media3EngineMimeTypesTest`,
+which asserts which three of six codecs actually arrive — so the set moving fails a test rather
+than surprising someone.
+
+`FFmpegCommandBuilder`'s arm has none of that. Whatever is decided, the fix is to make the two
+files agree and to say so in one place.
+
+### What a fix has to decide
+
+1. Whether Vorbis belongs in `ENCODABLE_AUDIO`. If yes, this is a feature and needs an e2e test
+   that produces a playable Vorbis file; if no, go to 2.
+2. Correct the `ContainerCapabilities.kt:84` comment, which is false as written, and give the
+   `FFmpegCommandBuilder` arm the treatment `Media3Engine.kt:221-233` already models.
+
+---
+
+## F2 — `ConversionRequest.hardwareEncodeAvailable` is written, read by nothing, and its KDoc describes behaviour that was removed
+
+**Severity: low · Confirmed by inspection**
+
+```
+app/src/main/java/org/libremediaconverter/model/OutputFormat.kt:211-219
+app/src/main/java/org/libremediaconverter/work/ConversionWorker.kt:117
+```
+
+The property is set on every request:
+
+```kotlin
+hardwareEncodeAvailable = devices.canEncode(spec.videoCodec),
+```
+
+`grep -rn 'hardwareEncodeAvailable' app/src/main` returns **that line and nothing else**. No
+production code reads it. Its getter is one of three uncovered methods in `OutputFormat.kt`, which
+is what surfaced it.
+
+Its KDoc (`OutputFormat.kt:211-218`) explains at length what it is for:
+
+> Knowing this lets the Fast tier choose a genuinely fast software preset instead of a mislabelled
+> slow one.
+
+`FFmpegCommandBuilder` no longer does that, and its own test says so —
+`FFmpegCommandBuilderTest.kt:132`, `the encoder choice no longer depends on hardware availability`:
+
+> Once FFmpeg stopped selecting MediaCodec encoders, this flag only affects whether the router sends
+> the job to Media3 at all — not what FFmpeg does.
+
+That second clause is also not true. `ConversionRouter` decides hardware encodability by calling
+`device.canEncode(videoEncode)` itself (`ConversionRouter.kt:153`); it never reads
+`request.hardwareEncodeAvailable`. The flag is computed from the same source the router
+independently consults, carried through the request, and dropped.
+
+This is the shape of open issue **#68** — a KDoc promising a switch that does not exist.
+
+**Not harmful.** It costs one `canEncode` call per job and a field on a data class. It is recorded
+because the KDoc actively misleads: a reader changing the Fast-tier preset logic would look here
+first, and this is not where that decision lives.
+
+### What a fix has to decide
+
+Whether to delete the property (and the constructor parameter, and the four
+`FFmpegCommandBuilderTest` call sites that pass it) or to keep it and rewrite the KDoc to say it is
+vestigial. Deleting is cleaner; the test at `:132` is worth keeping either way, since it pins the
+"FFmpeg does not select MediaCodec encoders" rule that the deletion would otherwise erase.
+
+---
+
+## F3 — `ConversionRequest.videoCodec` and `.audioCodec` have no callers anywhere
+
+**Severity: low · Confirmed by inspection**
+
+```
+app/src/main/java/org/libremediaconverter/model/OutputFormat.kt:222-223
+```
+
+```kotlin
+val container: Container get() = spec.container      // used: FFmpegConcatCommand.kt:42, :80
+val videoCodec: VideoCodec get() = spec.videoCodec   // no callers
+val audioCodec: AudioCodec get() = spec.audioCodec   // no callers
+```
+
+Three delegating accessors on `ConversionRequest`; the first is used twice, the other two are used
+nowhere in `main`, `test` or `androidTest`. Everything that wants those values reads
+`request.spec.videoCodec` or takes the `OutputSpec` directly.
+
+**This is not a test gap and must not be filed as one.** A test asserting
+`request.videoCodec == request.spec.videoCodec` is vacuous by construction — it restates the
+implementation and would pass against any delegation, right or wrong. That is precisely the failure
+mode `CLAUDE.md` records from the mutation review (9 of 46 mutations vacuous, five over completely
+unguarded paths).
+
+The two accessors are either convenience worth keeping for symmetry with `container`, or two lines
+to delete. Deleting them costs nothing and removes two uncovered methods that will otherwise be
+re-found by every future coverage read.
+
+---
+
+## F4 — Two guards are reachable only by direct call, and that is correct
+
+**Severity: n/a · No action**
+
+```
+app/src/main/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilder.kt:167-168
+app/src/main/java/org/libremediaconverter/model/ConversionRouter.kt:175-176
+```
+
+```kotlin
+VideoCodec.COPY, VideoCodec.NONE -> error("encodeVideo called for $codec, which is not an encode")
+```
+
+```kotlin
+if (plan.video == VideoPlan.Copy && video == null) return false
+if (plan.audio == AudioPlan.Copy && audio == null) return false
+```
+
+Both sit in private functions (`encodeVideo`, `media3CanMux`), and both are unreachable because a
+caller upstream already excluded the case — which each says in its own comment. `ConversionRouter`'s
+is labelled "the second line of defence"; `CopyPlanner` is the first.
+
+**Recorded so the next coverage read does not treat them as gaps.** A second line of defence that
+can be provoked is not a second line of defence. Making these reachable from a test would mean
+widening the functions to `internal`, which buys a test that asserts an `error()` fires when called
+in a way production cannot call it. This is the same judgement issue **#88** reached about
+`getForegroundInfo` and closed on: naming the exemption rather than covering it.
+
+Neither should change unless the upstream guard does. If `CopyPlanner` ever stops resolving `COPY`
+before the builder sees it, `FFmpegCommandBuilder.kt:167` becomes live and wants a test that day.
+
+---
+
+## F5 — `ConversionNotifications.areEnabled()` is never called
+
+**Severity: low · Confirmed by inspection · found while decomposing the test-gap ticket**
+
+```
+app/src/main/java/org/libremediaconverter/work/ConversionNotifications.kt:60-62
+```
+
+```kotlin
+fun areEnabled(): Boolean = context.getSystemService(NotificationManager::class.java)
+    .areNotificationsEnabled()
+    .also { if (!it) Log.i(TAG, "Notifications disabled; progress will not be visible.") }
+```
+
+`grep -rn 'areEnabled' app/src` returns **that declaration and nothing else**. `ConversionNotifications`
+is constructed in both workers (`ConversionWorker.kt:55`, `ConcatWorker.kt:35`) and only `build()` is
+ever called on it.
+
+**This entry exists because it was very nearly filed as a test gap.** Its three lines are cold on the
+JVM, it has a KDoc explaining real user-visible stakes — a foreground service without
+`POST_NOTIFICATIONS` shows only in the Task Manager, so progress silently vanishes — and Robolectric
+can flip that permission in one line. Everything about it reads like a cheap, worthwhile test.
+
+It is not, because **the behaviour the KDoc describes does not happen**. Nothing consults
+`areEnabled()`, so nothing warns, degrades, or logs when notifications are off. A test would assert
+that a function nobody calls returns what the platform told it — green, vacuous, and actively
+misleading, since it would imply the app handles the disabled-notification case. That is the failure
+mode `CLAUDE.md` records from the mutation review, reached from the opposite direction: not a test
+that fails to bite, but a test with nothing to bite.
+
+### What a fix has to decide
+
+Whether the app should act on this at all. The KDoc argues it should — a conversion whose progress is
+invisible is a real complaint, and `ConversionViewModel` or the worker's foreground start is where a
+check would go. If yes, that is a **feature** with a test; if no, delete the method and the KDoc's
+claim with it. What must not happen is a test that makes the current state look handled.
+
+Related: **#16** is open on an adjacent gap — a user who *can* unblock a foreground-denied retry has
+no way to make it happen now.
+
+---
+
+## Summary
+
+| ID | Finding | Severity | Evidence | Action |
+|---|---|---|---|---|
+| F1 | `FFmpegCommandBuilder` emits a Vorbis encoder `ContainerCapabilities` says does not exist | low | confirmed by inspection; unreachability traced through four call sites | **decide**: feature or dead arm — the comment is false either way |
+| F2 | `hardwareEncodeAvailable` written, never read; KDoc describes removed behaviour | low | confirmed by inspection; `FFmpegCommandBuilderTest:132` corroborates | **decide**: delete or mark vestigial |
+| F3 | `ConversionRequest.videoCodec` / `.audioCodec` have no callers | low | confirmed by inspection | delete, or keep for symmetry — **not** a test gap |
+| F4 | Two private guards reachable only by direct call | n/a | confirmed by inspection | **no action** — named exemption, per #88 |
+| F5 | `ConversionNotifications.areEnabled()` is never called | low | confirmed by inspection; grep returns the declaration only | **decide**: act on it or delete it — **not** a test gap |
+
+Order, if these are acted on: **F1 and F5 first, separately.** They are the two with a possible
+user-visible answer — a format the app can produce and does not offer, and a warning the app
+documents and does not give — and either answer changes what the tidying should look like. F2 and F3
+are tidying and belong in one commit with each other, not with F1 or F5. F4 is finished by being
+written down.
+
+**F1 and F5 share a shape worth naming:** both are places where a comment describes behaviour the
+code does not have, and in both the tempting fix (delete the dead arm, test the dead method) would
+freeze the wrong answer in place. The decision comes first.
+
+## Not covered here
+
+**The test gaps from the same read.** Seven JVM-side gaps (**#132**) and three seam questions
+(**#133**) came out of this coverage read and are tracked there, because they are work rather than
+observations. This document holds only what a test would not fix. #133 also records why
+`AndroidDeviceCodecs.probe()` was considered and left out, so that spike is not run a third time.
+
+**`ConversionForegroundType.current()`**, which looked like the sharpest gap in the read and is not.
+Its API 33 and 34 arms are cold on the JVM, but issue **#88** already established that the class is
+covered by `ConversionWorkerTest.foregroundTypeMatchesTheRunningApiLevel` across the CI matrix, and
+that its 0% is the `testDebugUnitTest`-only measurement boundary.
+
+The premise worth re-checking was whether the 33/34 legs still complete, given #122's wedge.
+**They mostly do, and #122 is not resolved** — this entry said "they do" on first writing, from a
+single green run, and the PR carrying this very document proved that wrong:
+
+| run | API 33 leg | shape |
+|---|---|---|
+| `32933262839` (#127) | success, 7m16s | `expected 60, received 60, failed 0, completed cleanly: yes` |
+| `33033036857` (PR #131, docs-only) | **failure, 23m08s** | `expected 60, received 60, failed unknown, wedged: yes — gradle killed after 1200s` |
+
+Five of the last six completed API 33 legs passed in about seven minutes, so the wedge is
+intermittent rather than systematic. **What it costs is the verdict, not the execution**: `received:
+60` on the wedged run means all sixty tests still reported, so the API 33 regime *was* exercised —
+but `failed:` reads `unknown`, so that leg could not have told anyone if it had broken.
+
+That is why this stays a note and not a ticket, and also why it is not simply deleted: #88's
+reasoning holds, but the leg it rests on cannot be relied on to report a failure. A
+`@Config(sdk = 33)` / `@Config(sdk = 34)` JVM test would pin all three arms deterministically in one
+run for about three lines. Small, and worth doing the next time this file is opened — but it is
+insurance against a flaky leg, not the uncovered behaviour it first looked like.
+
+**The Compose screens' branch coverage.** `ConverterScreenKt` reports 110 of 200 branches missed and
+`JoinScreenKt` 60 of 82, which looks alarming and is not a signal: the Compose compiler synthesises
+`$changed`/`$dirty` recomposition-skip tests that JaCoCo counts as branches. The line figures are
+the real ones — **34 of 383** and **20 of 143** missed — and the screens are among the
+better-covered files in the repo, which is what #52, #57 and #61 were for. **Do not chase the
+branch number here.** If a future read wants a screen metric, use lines.
+
+**Anything requiring a device.** `MediaProbe`'s FFprobe half (`MediaProbe.kt:151, 156-158, 173-188`)
+and `FFmpegEngine` in full report 0% on the JVM and are covered by `androidTest`. JaCoCo measures
+`testDebugUnitTest` only; their zeroes are a boundary, as #84, #85, #86 and #88 each recorded
+before this.