The three #52 test PRs landed 56 new tests and the coverage figure moved 29.8% -> 29.7%.
That looked like the tests being worthless. It was the measurement.
Robolectric loads every class it touches through its own sandbox classloader, and those
classes arrive with no source location. JaCoCo skips no-location classes unless told
otherwise, and nothing here told it. So not one Robolectric test has ever contributed
coverage in this repo -- and Robolectric is what exercises the framework edge: both
workers, the publisher, both ViewModels, every Compose screen.
Same commit, same 335 tests, same 0 failures, only the block below added:
LINE 652/2194 29.7% -> 1519/2194 69.2%
BRANCH 425/1424 29.8% -> 758/1424 53.2%
OutputPublisher 0.0% -> 97.5% MainActivityKt 6.8% -> 86.4%
ConversionViewModel 0.0% -> 85.4% ConverterScreenKt 6.6% -> 62.8%
The discriminator, so this is not cargo cult: inside ConverterScreenKt, `describe` is the
one non-Composable and is exercised by a plain JVM test. It reported 8/8 covered while
every @Composable in the same class reported 0 -- including ones whose mutations
demonstrably failed the build when reverted. Across files the split is exactly
Robolectric-vs-not: StagingSweep, tested purely, 100%; OutputPublisher, ConversionViewModel
and FailureOutcome, tested under Robolectric, 0%.
`excludes = listOf("jdk.internal.*")` is not decoration. Without it JaCoCo walks
JDK-internal classes Robolectric has no location for either and the test JVM dies rather
than reporting a number.
CLAUDE.md's coverage bullet is rewritten, because it was wrong twice over. The figure was
an artifact, and the explanation attached to it -- that coverage fell as the suite grew
from 11 test files to 43 because the denominator outran the numerator on framework-edge
code "the JVM cannot reach" -- described a cause that does not exist. The JVM reaches that
code fine. The new tests were disproportionately Robolectric, so each one added denominator
and no numerator: the measurement was punishing precisely the tests that were hardest to
write, and the conclusion drawn from it was that writing them had not helped.
Mutation, run both ways on this branch: remove the block and jacocoTestReport collapses
back to 29.7% / 29.8%; restore it and it returns to 69.2% / 53.2%.
Two things that were true stay true. There is still no coverage gate, and a floor still
needs a settled baseline -- this one just moved 39 points in one build change. And
"re-measure before quoting it" was already written down; following it is the only reason
this was found.
CLAUDE.md carried three claims about instrumented tests. All three were false, one of
them contradicted a paragraph forty lines below it in the same file, and a subagent
working on #58 hit the contradiction and had to stop and flag it rather than trust the
project's own instructions. That is the cost being paid here: this file is what every
contributor and every agent reads first.
"Instrumented tests do not run locally" -- they do, API 33-36, since 22c7914.
"Emulators segfault on this host" -- solved 2026-08-22; it was SwiftShader's
Reactor JIT meeting SELinux execheap, not a
broken machine, and another renderer avoids
it. docs/local-emulator.md is titled
"Emulators do run on this host".
"CI's matrix therefore stops at API 36" -- the matrix has been 33/34/35/36/37 since
#56 merged, with a gating API 37 leg.
The contradiction was the worst of it. The testing-norm section added in #51 says "E2E is
runnable locally now", so the file simultaneously told you the emulator works and that it
segfaults on every AVD. A reader has no way to tell which half is current, and the wrong
half is the one that stops work: an agent that believes emulators are impossible here does
not try, and the local e2e half of the definition-of-done in #51 quietly stops being
enforceable.
The replacement says what is true now and names what is still true and why -- API 37 still
needs the manual Pixel check before a release, because the two advisory tests are the one
thing CI cannot answer for. It also states plainly that the advisory job is red on every
PR by design, which is the other thing agents keep rediscovering the hard way: three
separate subagents have now flagged that failure as possibly theirs.
The norm bullet now points at the section rather than re-arguing it, so there is one place
to correct next time rather than two that can drift apart again.
Same defect class as R14, R15, R20 and R25, all of which were documentation claims this
repo's own review falsified. The pattern is not that the docs were careless; it is that
they were written at a moment and the moment moved.
The step added in the previous commit went red on its own PR, and the reason is the one
CLAUDE.md already gives for pinning ktlint, detekt and JaCoCo: "a new rule in a linter
makes files nobody touched stop passing, so CI goes red on a PR whose diff cannot explain
it." Here it was not even a new rule, just a different version of the same tool.
The runner's ambient shellcheck is 0.9.0. The container used to check locally was 0.11.0.
They disagree about how to report `on_signal`, which is installed as the INT and TERM trap
eleven lines below its declaration and so is never called by name:
0.11.0 SC2329, once, on the function declaration -- "never invoked"
0.9.0 SC2317, seven times, one per command in the body -- "appears to be unreachable"
The disable directive named SC2329, so 0.11.0 was silent and 0.9.0 reported seven findings.
Nothing about the script was wrong; the local check simply was not the check CI ran.
Two changes, because either alone still leaves a way to be surprised:
- CI runs shellcheck from an image pinned by digest, so an upgrade is a line in this
file that someone chose, not something that arrives on a Tuesday. The version is
still printed, so a finding out of nowhere can be tied to that line.
- The directive names SC2317 and SC2329 both, so a contributor whose distro ships 0.9.0
gets the same answer locally as CI gives. Verified against both images: clean under
0.9.0 and clean under 0.11.0.
CLAUDE.md now says to check with the pinned digest rather than with whatever is installed,
which is what would have caught this before the push.
"Size unknown" is the line a stream fixing D5 reported as untestable. It is two
assertTextEquals calls, and it needed two rather than one: the size line renders
independently of the probe, so it is asserted with a probe and without one. That
independence is the contract, and a test of the probed case alone would leave the
branch a user hits first -- the card is on screen before the probe finishes --
unguarded.
The rest of the card degrades in words the same way, and none of it was covered:
the four InputKind branches, "No video track", "No audio track", describeVideo's
"Unknown", and the two `> 0` guards that drop the dimension and length rows
rather than printing 0 and 0:00. Each guard gets a case on both sides, because
the present side alone stays green when the guard is deleted -- what deleting it
produces is "Size: 0x0" and "Length: 0:00", the same invented-measurement defect
as "0 B".
The four pure helpers go in a plain JVM class beside it, with formatBytes pinned
at each threshold and one byte below it. A `>=` quietly becoming a `>` is only
visible from a value sitting exactly on the boundary.
Two things the issue could not have known:
- Its second acceptance criterion, "delete the return@Column and watch the
Reading... test go red", cannot happen -- it does not compile. The early return
is what smart-casts `probe` non-null, so ten uses below it fail with "Only safe
(?.) or non-null asserted (!!.) calls are allowed on a nullable receiver". The
exit is enforced by the compiler, not by a test. Both compilable regressions
someone would land instead are covered and were run red.
- CodecNames.describeAudio has no UNPARSEABLE arm, unlike describeVideo, so it
answers the raw sentinel rather than "Unrecognised". Unreachable today, because
the UNPARSEABLE kind renders the explanatory line instead of rows. Left alone;
recorded on the PR for R38.5.
The divider's absence is not asserted and cannot be: Material 3 renders it as a
Box with no semantics modifier, so it contributes no node. What is asserted is
everything it precedes, plus the card's child count. The class KDoc says so.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`AdvancedPicker` is the one leaf on the converter screen that carries its own
state, and `ValidationError` is deliberately invoked after the
`AnimatedVisibility` that gates the chip rows -- so an invalid spec explains
itself and offers one-tap fixes while the section is collapsed. That is the
only route out of an invalid spec for a user who never opened Advanced, it was
completely untested, and folding the two `if` blocks into one is a plausible
tidy-up that compiles.
`AdvancedPickerTest` covers the gate in both directions, clicks each of the
four colliding chip labels through its own row tag, and does every assertion
about the error card with the toggle untouched.
`AdvancedPanelSavedStateTest` is the `DestinationSaverTest` split for
`expanded`: `StateRestorationTester` saves into an in-memory map, so it proves
`rememberSaveable` is in use and nothing about the representation. Driving a
real `SaveableStateRegistry` shows the picker saves the `MutableState` itself
rather than the `Boolean`, which only survives a rotation because
`mutableStateOf` on Android returns a `Parcelable` one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two gaps, both found the same way -- by something going wrong quietly.
`gh issue create` does not touch the project board. The issue is created, carries its
labels, and is invisible in the Kanban, which looks exactly like a ticket nobody filed.
On 2026-08-24 eight issues filed as a scripted batch all reached the board and one filed
as a one-off minutes later did not; it surfaced only because someone went looking for it.
A batch carries the board step inside its loop. One-offs are where it slips, so
tools/github/file-issue.sh is for one-offs.
Three things it does that a two-command shell snippet would not:
- Resolves the project, Status field and option ids BY NAME, every run. Caching them
is the obvious optimisation and the wrong one -- a renamed or reordered column would
then have this writing a stale id into the board with no error anywhere.
- Reads the item back. A mutation returning 200 says the request was accepted, not that
the board shows what was asked for; the read-back is the only step that checks the
claim this script exists to make. It is a GraphQL query because REST cannot do it --
the `fields` array REST returns on a project item carries Title and nothing else, so
a REST-only check reports every item's Status as unset.
- Exits 3, loudly, with the issue number on a line of its own, when the issue was
created but the board step failed. That exact combination is the failure being
prevented; it must never be the quiet path.
Shell was the other language here with nothing checking it -- four scripts, one of them
the CI entry point. shellcheck now runs in the Static analysis job over
`git ls-files '*.sh'`, so a script added later is covered without editing the workflow,
and it runs at full severity with `info` included.
That raises two findings today and both are the tool being wrong, so both are answered
with a targeted `disable` carrying its reason rather than by lowering the severity:
run-e2e.sh's `on_signal` is reported as never invoked when it is installed as the INT and
TERM trap eleven lines below it, and the `$names` inside file-issue.sh's queries are
GraphQL variables that must not expand -- expanding them would send the shell's idea of
$owner to the API instead of declaring a parameter. A blanket --severity=warning would
have hidden both, and the next real finding with them.
The gradle step gains `if: !cancelled()` so a shellcheck failure cannot cost the
ktlint/detekt/lint lists -- the same reason that step already passes --continue.
Not covered, deliberately: shellcheck here reads .sh files, not the inline `run:` blocks
in the workflows, where a good deal of this repo's bash actually lives. actionlint does
read them, and finds one pre-existing info-level issue in build.yml. Wiring it in means
pinning a container digest, because every action here is pinned by SHA and actionlint's
usual installer is a curl-pipe-bash off a moving branch. Its own ticket, not this commit.
The format, quality and engine pickers are the same dozen lines with a
different enum substituted, and both ways they can go wrong are silent.
An onClick that closes over the picker's `selected` parameter instead of
the chip's own entry returns one constant for every chip; an inverted
`entry == selected` lights every chip but the right one. Neither throws,
neither changes the labels on screen, and a test that only asserted the
callback ran would pass over the first of them.
So each click test presses every chip in the row and compares the whole
recorded list against `entries`, which makes the constant load-bearing
rather than the click count, and each selection test asserts over every
chip rather than only the one that should be lit.
Verified by mutation, not by the suite going green:
- `onSelect(format)` -> `onSelect(OutputFormat.MP4_H264)` fails with
`expected:<[MP4_H264, MP4_H265, WEBM_VP9, ...]> but was:<[MP4_H264,
MP4_H264, MP4_H264, ...]>`
- `format == selected` -> `format != selected` fails both format
selection tests on `Selected = 'true'` for a chip that should not be
- `onSelect(preference)` -> `{}` fails with `expected:<[AUTO,
PREFER_HARDWARE, FORCE_SOFTWARE]> but was:<[]>`
- `selected.description` -> `QualityTier.FAST.description` fails the
quality prose test on the missing BEST line
Labels are read off the enums so a reword cannot redden this file for
the wrong reason. `EnginePreference` has no label of its own, so the
screen's own `label()` supplies that set. The one display literal with
no symbol behind it, the custom-spec line, was copied out of the source
byte for byte because it holds a U+2014 that would fail silently if
retyped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Kotlin `private` on a top-level declaration is file-scoped, so every leaf
composable in the two screens was invisible even to the JVM test source set,
which is a friend of main. The only three declarations src/test could name were
ConverterScreen, JoinScreen and AppRoot -- there was nothing to write a test
against, which is why #52 could not be started as filed.
`internal` is the same choice MainActivity already documents for Destination:
the unit tests can name it, and it stays invisible to anything outside the
module. Eleven declarations in ConverterScreen and FileRow in JoinScreen.
The tag table is the other half. Tests reference a symbol rather than a literal,
which is what keeps the five children that follow independent: "Cancel", "Start
over" and "Save file" are each rendered by both screens and by more than one
state branch, so rewording one would otherwise redden several PRs at once and no
diff would explain why. There were zero testTag, semantics or contentDescription
calls anywhere in main before this.
Every tag is applied inside main. A tag a test hands down as a Modifier proves
only that the test set it -- it would survive the affordance losing its own tag
entirely, which is the vacuous shape CLAUDE.md records nine of in one review.
That is why FileRow and DetailRow derive theirs from data they already hold
rather than taking an index from the call site.
TestTags is public rather than internal, and R38.8 is the reason. It reads the
table from androidTest, and whether that is a friend source set of main under
AGP 9 had no in-tree answer -- nothing referenced a main internal from there.
Settled by compiling one: it is a friend, so internal would work today. Public
anyway, because that friendship is AGP wiring rather than something this project
states, and the KDoc now carries the measurement so nobody has to repeat it.
Smoke tests cover each leaf: it renders, and its tag resolves to exactly one
node. Counting rather than asserting existence is deliberate, since a duplicated
tag fails differently depending on which finder a later test happens to use. The
state-branch tags -- Convert, Cancel, Save file, Start over, the progress bars --
have no bite yet: reaching a branch needs the state seam R38.5 extracts, and the
state matrix is R38.6/R38.7 by design.
Five mutations, all red on the named test alone: FORMAT_CHIPS deleted,
FILE_CARD_NAME deleted, detailRow's tag no longer derived from its label,
FileRow's no longer derived from its name, and JOIN_MORE given JOIN's value --
the last caught only by the uniqueness check, which is what it is for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ConversionViewModelProbeFailureTest.an OutOfMemoryError is not swallowed`
deliberately lets a real error escape `viewModelScope.launch`, which has no
exception handler by design -- the ViewModel's KDoc says an OOM raised in the
probe should reach the thread's handler and take the process down.
On the JVM something else takes it. kotlinx-coroutines-test installs a
process-wide collector for uncaught coroutine errors, keeps whatever it catches,
and hands the backlog to the next `runTest` that starts, which throws
UncaughtExceptionsBeforeTest. Every Compose test is a `runTest`: that is how
`createComposeRule` runs a composition. So the error lands on an unrelated test
in an unrelated file, and the message names neither the test that caused it nor
the error's origin.
Nothing has hit it yet only because the sole Compose test in the repo happens to
run before the ViewModel one. R38 adds six more Compose classes in exactly the
two packages that surround it, and the first two of them made the suite fail in
two different files on two consecutive runs of identical, green code -- the
throw is on a real Dispatchers.IO thread, delivered after the state assertion
that ends the test responsible, so which class catches it is a race.
Drain it where a Compose rule is built. A @Before cannot: the rule's `runTest`
wraps the statement that calls it, so it has already thrown. @BeforeClass cannot
either, because Robolectric runs it outside the sandbox classloader, where the
collector is a different object. Constructing the rule is early enough, since
JUnit builds a fresh test-class instance -- and every @get:Rule field on it --
before evaluating any rule.
This is containment, not the cure. The cure is a seam: give the probe hop an
injectable dispatcher the way the constructor already does for
cleanupDispatcher, so the error has somewhere to land. That is a production
change and deserves its own commit.
kotlinx-coroutines-test was already on the unit-test classpath through
compose-ui-test-junit4; it is declared now because a file imports it. Pinned,
like robolectric and the linters: org.jetbrains.kotlinx is not one of the groups
the prerelease guard covers, so a float here would be free to take a milestone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI has never run the API level this app targets. The reason it did not was
never "API 37 is untestable" -- it was that two tests fail on the emulator
image, so one row would be permanently red or permanently allow-listed. This
splits that row instead of choosing between those two.
E2E API 37 gates. It runs 55 of the suite's 57 instrumented tests and must be
green. E2E API 37 Media3 hardware transcode runs the other two, reports, and
never blocks (continue-on-error). Both are driven off ONE marker,
@FailsOnEmulatorApi37: the gating job passes notAnnotation, the advisory job
passes annotation. Two lists would drift, and drift is silent in both
directions -- a test that ends up in neither job reads as green. Excluding by
class was not an option either: Media3EngineTest has four tests and two of
them pass here, so notClass would have thrown away real coverage.
The advisory job is named for what it runs, not for what we think is wrong.
Both its tests drive a full H.264 -> H.265 hardware transcode, which is what
distinguishes them from the two Media3EngineTest cases that pass -- those
never decode video. The goldfish-decoder theory sits in a comment inside the
job, where it can be corrected without renaming a check people have learned to
look for; docs/api-37-emulator-crash.md keeps measurement and inference apart.
The SystemUI disable moves into .github/scripts/e2e-run.sh behind
E2E_DISABLE_SYSTEM_UI, unset everywhere but the two API 37 jobs, so the other
four legs run byte-identical commands -- the same shape as
E2E_EXTRA_GRADLE_ARGS. It runs BEFORE the streamed logcat starts, deliberately:
`adb shell stop` would end that logcat and nothing restarts it, so a disable
placed after it would cost the leg its diagnostics for the part of the run that
matters. The body is probe v2 from api37-debug.yml -- the version measured 4/4
-- not the older one-round form: three rounds, waits for system_server to
actually be gone, verifies against `pm list packages -d`, and requires a 45 s
window with zero new aborts. The weaker probe reported success on a run that
then started SystemUI eight more times.
The caveat is written next to the row rather than left implicit: this leg runs
with SystemUI disabled and the framework restarted under it, a device
configuration no other leg and no Pixel run uses. Anything that touches system
UI must not trust it, and the Pixel check before each release is still the only
API 37 run with SystemUI intact.
docs/api-37-emulator-crash.md's "So should CI take API 37?" said no on three
reasons. Two were claims about CI that had never been measured; the section now
carries the eight runs that measured them, and the third reason is what the
split answers. docs/local-emulator.md and api37-debug.yml's header carried the
same "the matrix stops at 36" claim and are corrected with it.
CLAUDE.md is left alone deliberately -- its "CI's matrix therefore stops at API
36" clause is now false, and that correction is parked in the doc's existing
"Correction owed to CLAUDE.md" section, where two others are already waiting.
Making E2E API 37 an actually-required check is a repository-settings change
and must come after this is on main: adding a required context that does not
exist on the default branch blocks every PR.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three additions to docs/api-37-emulator-crash.md, all from a CI investigation
run through .github/workflows/api37-debug.yml.
A third measured bullet: API 36 against API 37, back to back, same two tests,
same renderer, same SystemUI-disable path. 37.0 fails both on
c2.goldfish.h264.decoder (32660148155); 36 passes both in 4.603 s with the
same decoder in its logcat (32660152961). That falsifies "the stripped
configuration is what breaks these tests" -- a reading the other measurements
never addressed, because they all compare against a device that still had
SystemUI. It carries its two uncontrolled variables rather than dropping them:
API 36's framework restart happened with zero aborts logged where API 37's had
two, so a restart under an active abort loop is still uncontrolled; and the
images differ on the encoder side, which is a second reason "broken h264
decoder" is the wrong shape of claim.
The decoder-mechanism bullet is unchanged and still labelled inference. This
adds a measurement next to it; it does not retract anything.
The intact-SystemUI counterfactual is unmeasurable on a GitHub runner, and now
says why. Seven dispatches, zero verdicts, with a mechanism rather than bad
luck: while the framework crash-loops the guest cannot reliably create per-user
private directories, so an app installed during the loop has no cache dir and
the fixture copy dies in @Before before any codec exists. googlesdksetup and
nexuslauncher hit the same thing. The result XML masks it behind an
UninitializedPropertyAccessException in tearDown, which reads as a defect in
this repository and is not one.
Abort cadence corrected. "Roughly every 20 s" was the watchdog's sampling
interval, not the cadence: measured gaps are 20-90 s, median 60-70 s, three to
five per run, with sys.boot_completed held at 1 throughout. The wrong figure
lived in api37-debug.yml's own comments, so that line is corrected too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four dispatches of one configuration -- API 37.0, swiftshader_indirect,
SystemUI disabled -- came back three green and one not, and the odd one out
was not a different failure so much as the same run without the fix applied.
In 32646029143 `pm disable-user` reported `new state: disabled-user` and
SystemUI then started eight more times:
14:41:24 ActivityManager: Start proc 6412:com.android.systemui ... GradientColorWallpaper
14:45:05 ActivityManager: Start proc 17299:com.android.systemui ... GradientColorWallpaper
with ten more RegionSampling aborts and a surfaceflinger pid that never sat
still (489, 1570, 3524, 4396, 6038, 7987, 9732, 11520, 13208, 15048). The
framework is being SIGKILLed every twenty seconds while this runs, so a
package-state change can go down with the system_server that accepted it.
Two things were wrong, and the second is why the first went unnoticed:
- one disable attempt was treated as sufficient
- the wait after `adb shell stop` was not a wait. It asked `service check`
0.3 s later and got `found` from the system_server that was still on its
way out, so it never waited for anything. Both the good and the bad run
printed `services back after 5 s`, which is how a broken fix looked
identical to a working one.
Now: up to three rounds of disable -> take the framework down and confirm
system_server is actually gone -> bring it back -> verify the package is in
`pm list packages -d` -> require a 45 s window with zero new aborts. Nothing
is believed because a command said so.
Also adds measure_baseline, default true. The 45 s pre-measurement is what
makes the rate comparable with the local figures, but it is 45 s of
crash-looping before the disable has to land, which is a worse starting
point than a real leg would have.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first three dispatches came back with every watchdog sample reading
`boot=? surfaceflinger=none zygote64=none dma_aborts=0`, on runs where the
device demonstrably booted and the action's own adb was working two steps
away. The watchdog was not measuring anything.
The emulator action puts platform-tools on PATH with core.addPath, which
writes GITHUB_PATH and therefore only affects LATER steps. The watchdog is
started before the action -- that is the whole point of it -- so it inherits
the runner's own PATH, where a bare `adb` is not necessarily anything. Every
call failed into `2>/dev/null` and the sampler dutifully recorded the silence
as zero.
It now resolves adb by path, preferring ANDROID_HOME, re-resolving on every
iteration in case platform-tools arrives later, and echoing the path it
settled on. The launch step prints ANDROID_HOME and `command -v adb` for the
same reason: a repeat of this failure should be one line to spot, not three
runs of quiet zeros.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
status_check.yml stops its E2E matrix at 36 and says the android-37.0 image is
why. That is established locally under -gpu host and under ANGLE, and it is not
established for CI: runners use -gpu swiftshader_indirect, and the one local
measurement of that mode was void for a local reason -- Fedora denies execheap
to SwiftShader's JIT, so the emulator died before the guest mattered. What CI
does at API 37 has therefore never actually been measured.
This is that E2E job with the matrix replaced by workflow_dispatch inputs, so a
hypothesis costs a dispatch rather than a commit: renderer, API level, image
target, channel, SystemUI disable, boot timeout, whether the suite runs at all,
and free-form emulator and Gradle arguments. It triggers on nothing else and
gates nothing.
It calls .github/scripts/e2e-run.sh rather than forking it, and pins the same
disk-size, ram-size, action SHAs and KVM setup as the job it copies, so a run
here measures the renderer and not a different device.
The watchdog is load-bearing rather than decorative. The emulator action calls
killEmulator() from its own catch block, so a run whose emulator never boots is
torn down before any script: line executes and leaves nothing behind -- which is
the exact failure shape API 37 is suspected of. It starts before the action,
samples sys.boot_completed, the surfaceflinger and zygote pids and the
hasReadColorBufferDma abort count every 20 s, and keeps a rolling copy of the
crash buffer so the last read survives the teardown.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Stated as a project norm: if a piece is unit testable it gets unit tests, and
if it is e2e testable it gets e2e tests, before it counts as done. Both clauses,
not either/or.
Recorded here rather than left as a habit because the recent review measured
what happens without it. Forty-six mutations were run against a 257-test suite;
thirty-six bit and NINE were vacuous, five of those passing the entire suite
while a reattachment code path sat completely unguarded. That code had shipped,
been reviewed, and looked tested. "The suite is green" was true and meant
nothing.
The convention also names the two things that make it enforceable rather than
aspirational. Unit-testable is broader than it looks, because the pure-seam
pattern converts device-bound logic into a testable function plus a thin edge,
and Robolectric now covers the rest including Compose. And e2e is genuinely
runnable locally since the emulator renderer cause was found -- until last
night, "run the instrumented suite" was not a request anyone could act on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CLAUDE.md has said "~31% of lines" since the lint/format work landed. Measured
on main today it is 29.8% (629/2113 lines, 408/1424 branches).
The number went DOWN, which is worth stating rather than quietly correcting.
The JVM suite went from 11 test files to 43 over the same period, so the
intuition -- and the review finding that prompted this, which called the
direction certain -- was that coverage must have risen. It did not: main source
grew from 4,114 to 5,715 lines as the fixes added Reattachment, JobSnapshots,
JobTags, InputQuery, StagingNames, StagingSweep, NativeLoadFailure and an
Application class. The denominator outran the numerator.
That is not an argument against the tests. It is an argument against quoting a
coverage percentage from memory, which is exactly how the stale figure survived.
The line now carries the measurement, its date, and the instruction to re-measure.
R30 / #39
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three separate mutations passed the full 257-test suite, all for the same reason: the tool
was tested and the thing that calls it was not.
The join half of per-job staging. Reverting ConcatWorker to the constant the audit's own
D8 table names -- "joined.<ext>", one string for every join of a format -- left everything
green: PerJobStagingTest drives only the conversion worker, and StagingNamesTest pins only
the pure function. The new case drives two real ConcatWorkers with different ids and reads
what they asked for rather than what is on disk, because ConcatEngine is native, so neither
join gets past it here and the catch on the way out deletes what it staged. The recorder
moves into WorkerStubs, which is what that file is for, and the enum test that already had
a private copy now uses it.
The process-start sweep. Deleting the one line in LibreMediaConverterApp.onCreate() -- the
only reason that class exists, and the backstop for every leak discardStaged cannot reach
-- left everything green too. The test stages one file a day old and one written now, calls
onCreate() again, and asserts both halves: the abandoned one is collected and the live one
is not. The second half is what says this is a sweep rather than the clearStaging() it
replaced, which could take a file out from under a running job. The mtime is set explicitly,
because "written long enough ago" is not something a test can wait for when the period is
twenty-four hours. Casting the Robolectric application to LibreMediaConverterApp is an
assertion in itself: it fails if android:name ever stops pointing here, in which case the
swept line would be correct code that never runs.
The backup and device-transfer exclusions. Reverting data_extraction_rules.xml to the
template's boilerplate left the unit tests green AND lintDebug green -- it is a resource,
so nothing was reading it -- and the failure it causes is one nobody meets in development.
WorkManager's queue is the app's whole backup payload, and its rows name content:// grants
and cacheDir paths that do not survive a transfer; reattachment queries by tag on launch,
so a fresh install would come up attached to a job the user never ran on it. The test reads
the compiled resource table, so what it pins is what the APK carries, and it asserts domain
and path for all four entries in both sections -- an <exclude> with no path is skipped
unchecked by lint's own detector, so half an entry could protect nothing. Its KDoc records
the one thing it does not cover: the manifest attribute that points the system at the file.
R8 / #17, R9 / #18, R11 / #20
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
R16 / #25 -- the branch put API 37 into the default APIS list, where it is permanently two
failures short of green, so a bare `run-e2e.sh` exits 1 by design and nothing said so.
Somebody running it from habit, a wrapper or a hook gets a red exit forever and either
stops reading exit codes or debugs a normal state.
Documented rather than suppressed. The script's own comment already argued that an
expected-red level belongs in the exit code -- reversing that is the branch owner's call,
not a correction -- and the review's alternative needs an exact-set comparison of the
failing test names before it can subtract 37's contribution, which is a new mechanism that
cannot be validated without a device. So:
- the header now states the exit code (0 all green / 1 any level red / 2 refused to
start), says a bare run is 1 by design and why, and gives `run-e2e.sh 33 34 35 36` as
the sweep that can be green;
- a red sweep prints one note after the summary saying the same thing, because the exit
code is read in the terminal and not in the docs -- but ONLY when 37.x is the only level
that went red. `overall` is set by any red level, so a note keyed on "37 was in the
list" would have called a genuine API 34 failure "by design", which is the defect this
is meant to prevent, one layer up. mark_red records which level it was, where the loop
already knows;
- docs/local-emulator.md says it where the default is documented.
R27 / #36 -- 792286a appended the caveat that the harness path reproduces a rate collapse
rather than a clean zero, but left "that is the confirmation that region sampling is the
sole trigger" standing three lines above it, which the caveat contradicts. Now "the
strongest evidence that region sampling is the dominant trigger", with the residue named:
no measurement here separates a second caller of the readback path from a disable that did
not fully take, and the file says so rather than picking one.
R28 / #37 -- "the capability is negotiated regardless of renderer" leaned on the string
search, which shows only that `ANDROID_EMU_read_color_buffer_dma` is implemented in one
shared component, not that it is negotiated on every path. The aborts are the actual
evidence -- the assertion that fires is `!hasReadColorBufferDma` and it fires under ANGLE
too -- and they suffice alone; the string search is demoted to a supporting note. Worth
getting right because the doc says the upstream report should lead with this model.
Two follow-ons that belong with R17 / #26 and land here rather than in their own commit:
bash runs a trap only between commands, so the handler starts when the foreground command
returns -- immediate under Ctrl-C, which reaches that command too, but not under a `kill
-INT` aimed at the script alone; that is now written next to the handler. And
delete_created_avds no longer discards avdmanager's status: an emulator that was SIGKILLed
did not get to remove its own lock files, avdmanager can refuse over them, and silence
there would leak exactly what the trap exists to clean up.
`bash -n` clean; the stub smoke harness (real script, fake SDK binaries, boot-failure path,
no Gradle and no emulator) now also checks that a 37-only red prints the note after the
summary, that a red API 34 alongside it suppresses the note, that a 34-only sweep says
nothing, and that a refused AVD deletion is reported. shellcheck is not installed here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
publish() opened the destination stream outside its guarded region, justified by "nothing
has been written at that point, so there is nothing of ours to remove". That reasoning is
wrong about what exists: SAF's CreateDocument contract creates the document before
publish() is ever called -- which is why every fixture in OutputPublisherPublishTest
starts as an existing empty file. A provider that then hands out no stream, because it
dropped between the picker and the write or simply returns null, left a zero-byte file at
the name the user chose while the screen said the save had failed.
The open moves inside the try, so the same two bounds that already govern a failed copy
govern this: only a document URI, and only a destination positively known to be empty. The
dead-provider case is untouched and now demonstrably by the guard rather than by the
placement -- nothing answers for that authority, so no size can be read, and "I could not
tell" still refuses to authorise a delete. Its test comment said the old thing and now says
that one.
The space arithmetic overflows in two places, both live on main and independent of the
allocatable-versus-usable question that stays parked:
- hasSpaceFor computed `free > required + headroom`. A request within 128 MiB of
Long.MAX_VALUE wraps that sum negative, and every free-space measurement beats a
negative number, so the check answers "plenty of room" to the largest request it can be
handed. Rewritten as `free - headroom > required` with both operands clamped at zero,
which is the form the parked branch's StagingSpace.hasRoomFor already argues for.
- InputQuery.total folded a join's inputs with nothing stopping the sum from wrapping,
and that is the reachable half: no single file overflows, three four-exabyte inputs do.
It saturates at Long.MAX_VALUE now, which the check above then refuses.
SpaceArithmeticTest ties the two together in the shape the defect had -- the total that
came out negative is handed straight to the space check -- and keeps one allowed case so
the refusals cannot pass by refusing everything. The negative-size clamp is deliberately
left unasserted, with a comment saying why: it only changes the answer when free space is
below the headroom, which a test reading the host's real cache volume cannot arrange.
R6 / #15, R23 / #32
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both workers read enums out of their input Data with Enum.valueOf, and all three reads sit
ABOVE the try. A name this build does not define -- which is what a downgrade or a
rollback with work still in the queue produces, since WorkManager keeps work for about a
week -- threw IllegalArgumentException straight out of doWork(). That is D13's signature
verbatim: FAILURE with reschedule = false, output Data with zero entries so the screen
says "Conversion failed." and nothing else, and no staged.delete(), so the partial stays
in cache. readSpec() twelve lines below already handles exactly this case, and its KDoc
says why.
So all three take readSpec's shape: entries.firstOrNull { it.name == name } ?: default.
Consistency argues for it as much as correctness does -- the file already contains the
right answer to this question, three times.
WorkerEnumFallbackTest reaches each read. Two of them pin the value that replaces the
unknown name rather than only that nothing threw: a quality tier falling back to something
arbitrary would convert at a setting nobody chose, and a join's format decides the
extension its output is staged with, which is where FFmpeg infers the container from. The
engine-preference case asserts through the space check instead, because predicting which
engine AUTO picks would tie the test to a routing rule it is not about. Against the
unfixed code all three fail with "No enum constant ...".
MAX_FOREGROUND_START_ATTEMPTS had no test of its value. Both existing cases are written
against the symbol, which pins the relationship and leaves the number free: changed to 2,
the job gives up about ninety seconds after process death -- exactly the long conversion
the retry exists to protect -- and all 257 tests stayed green.
The new assertion is the property the KDoc argues, not the literal: summed against
WorkRequest's own DEFAULT_BACKOFF_DELAY_MILLIS and MAX_BACKOFF_MILLIS, the attempts have
to span at least eight hours, which is what makes "the user will have opened the app by
then" a claim rather than a hope. A deliberate re-tune that keeps the property passes; the
accident does not, and reports the span it got (0.025 hours at 2).
R22 / #31, R10 / #19
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
R4 / #13 and R20 / #29 are one defect: an absolute test total in an unregenerated
document, written the same day it went stale. This branch was cut at 22c7914, where
app/src/androidTest held 49 @Test methods; main is 57 (ReattachOnLaunchTest added eight
in ec969c4). So the release instruction "expect 49 / 0 / 0 / 2, and if you get 40 you are
on an old checkout" becomes false the moment this branch merges -- on the one check that
has no CI backstop -- and docs/local-emulator.md's headline promises a 49-test local
baseline main no longer produces.
Re-derived rather than renumbered, because a third total would go stale the same way:
- The total is the size of app/src/androidTest on the checkout that ran, and the reported
total has equalled that checkout's @Test count everywhere it has been checked: 40 at
edd6385 (the Pixel run), 49 at 22c7914 (the four local levels and API 37), 57 at
18c53a3 (counted, not run). The new "Reading these totals" section states that, gives
the one-line grep, and makes the *mismatch* the signal: a total that disagrees with
your own checkout's count means an old checkout, a stale build or tests that never ran.
The pre-release Pixel instruction now reads "that many tests, 0 failures, 0 errors, 2
skipped" -- the invariant, not the total.
- Measurements are kept verbatim and anchored to 22c7914 (the sweep table, the API 35
control, the tests="49" XML quote, the 51-on-screen console block). Only the claims
built on top of them were rewritten.
Two claims went with the number, both of which a rebase would have preserved:
- "47 of 49" is not a defensible ratio when two of the 49 are skips. 49 = 45 passed + 2
failed + 2 skipped, and that is what it now says.
- "against the Pixel's 49 of 49" and "matches the physical Pixel 10 Pro XL baseline of
49 / 0 / 0 / 2 exactly" describe a run that never happened: the Pixel measured
40 / 0 / 0 / 2 at edd6385, nine tests earlier, as the same file says a hundred lines
further down. Both documents projected the local total onto the phone and called it a
match. What compares between them is 0 failures and the same two skips.
Also re-derived in the CLAUDE.md wording docs/local-emulator.md proposes, since that text
is meant to be pasted out of the branch and would have carried "49 tests / 2 failures /
2 skipped" with it. CLAUDE.md itself is still untouched.
Counts re-checked with git grep at each of the three commits; nothing here needed a
device, and none was used.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reattachment.choose has twenty tests and every mutation aimed at it bites. Everything
that computes its inputs had none, and five mutations there passed the whole 257-test
suite. Four are closed here, each verified by applying the mutation and watching the new
test go red.
jobSnapshots() is the half that has to touch WorkManager and the filesystem, so it is
where the untested values live. JobSnapshotsTest drives it against a real WorkManager and
a real cacheDir:
- A zero-byte staged file is not an output. Relaxing the filter to `exists()` -- which
is what a job killed before its engine wrote anything leaves behind -- made the
snapshot claim a result, and the user would meet a Save button for a zero-byte
"conversion". The same case pins that the path is still reported and that the mtime
stays 0 for a file that is not a result.
- Each result carries its own file's mtime. Hardcoding it to zero starves the
newest-file tie-break of the only data it has, which is precisely the failure the
tie-break exists to prevent: the query has no ORDER BY, so an arbitrary winner keeps
winning every launch. Timestamps are set with setLastModified and compared against what
the filesystem stored, because mtime granularity is not this test's claim to make.
ReattachGuardsTest covers the two decisions the ViewModel makes that the pure rule cannot:
- A file picked while the query was still in flight is not reattached over. Deleting the
guard turns the user's pick into yesterday's job -- with the Save button pointing at a
file the card does not name. Made deterministic by holding WorkManager's task executor
rather than by racing two IO hops: the query cannot finish until the pick has landed.
The test also asserts the brake really gripped, so a reattachment that never arrived
cannot pass for one that was refused.
- An Ambiguous result is offered without being attributed. Two finished jobs naming one
staged file is what the device produced before staging was keyed on the job id; taking
the first job's tags labels the file with the other conversion's name, which is the
confident lie the KDoc rejects. The neutral label and the absent size are both pinned.
ReattachmentTest's FAILED exclusion was only ever tested with pathless FAILED jobs, so a
narrow regression ranking a FAILED job that carries a file like a result passed all 257
tests. The live shape is the 2 MB orphan the device pass found: a job killed mid-write
leaves a partial, and under that regression the user is offered a truncated file with a
Save button. One fixture with outputPath and outputExists set closes it.
save() re-checks the staged file, in both ViewModels. The check reattachment made ran
inside a tag query that can be hours older than the tap, and cacheDir is what the OS
empties when it wants space and what the sweep collects after a day. The file's absence
used to arrive as staged.inputStream() throwing, and e.message put
"/data/user/0/.../4b4882....mp4: open failed: ENOENT" on screen -- a true statement about
a path the user has never seen and cannot act on. It now reads as a sentence with an
action in it. The message is one constant next to OutputPublisher because both ViewModels
need it and staging is what it is about.
Reattachment's KDoc claimed a defect that was fixed in the commit before it -- that "Start
over" keeps its staged file -- which would send a maintainer to re-fix D2. Rewritten to
say what is actually true: the delete happens, and the gap it leaves is the reset() whose
delete is cancelled with the Activity, which is the sweep's job and is named in the
sweep's own KDoc.
R1 / #10, R2 / #11, R24 / #33, R25 / #34
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four corrections to the harness, none of which changes what a successful sweep does.
R17 / #26 -- no trap. Ctrl-C during a sweep (now up to five boots long) left headless
qemu on console port 5560 and an lmc_e2e_apiNN AVD behind. The next run's `emulator
-port` then collides with the orphan and `emu_adb` can resolve to it -- on a workstation
with the Pixel plugged in, exactly the ambiguity the ANDROID_SERIAL pinning exists to
prevent. `cleanup` (stop_emulator + delete_created_avds, KEEP_AVD honoured) is now on
EXIT, INT and TERM. It is idempotent and the normal path calls it explicitly before the
summary, so cleanup output cannot land after the summary and the EXIT trap finds nothing
to redo. The interrupt path passes a 6-second grace rather than 30: Ctrl-C has already
reached the emulator through the foreground process group, so that wait is only for it to
finish writing, and `kill -9` follows regardless. `exit "$overall"` stays the last line,
so the exit code an EXIT trap could have swallowed is still the one that escapes. The
emulator logs in $LOG_DIR are deliberately kept -- they are the only evidence a failed
boot leaves.
R31 / #40 -- `kill -9 "${EMU_PID:-0}"`. EMU_PID is empty, not unset, if the background
launch never produced a job, so `:-0` converted "nothing to kill" into pid 0, which POSIX
reads as the sender's whole process group. The `kill -0` wait loop had the same shape and
would have spent its full grace period testing the group. All three sites now take a bare
`$EMU_PID` behind one `[ -n ... ] || return 0` guard. boot_emulator's own `kill -0` is
left alone: it runs only after the assignment and cannot reach the group form.
R33 / #42 -- ensure_avd wrote CI's RAM and disk pins to a hardcoded
$HOME/.android/avd/... path and checked nothing. With ANDROID_AVD_HOME (or
ANDROID_USER_HOME, or ANDROID_SDK_HOME) set, the sed failed and the level ran on at
default RAM and userdata, which surfaces much later as "not enough space" and reads as a
device problem. `avd_config_path` now looks in every directory avdmanager honours -- no
precedence is asserted, the existence check decides -- and a level that cannot be found
or written fails instead of running unpinned.
R34 / #43 -- disable_region_sampling's "one blocking wait on the device" did not wait:
`adb shell stop` does not clear sys.boot_completed, so the property still read 1 and the
loop returned at once. Deleted, and the comment now names the service-check loop below it
as the actual wait -- which polls the better thing anyway, since `Can't find service:
package` is the failure it exists to prevent. That loop also says so when it gives up
after 150 s instead of proceeding silently. Deliberately not doing the `setprop
sys.boot_completed 0` variant: the loop tested for an empty value, so a 0 would not have
made it wait either, and the `!= 1` form it would need is an unbounded loop inside `adb
shell` with no timeout.
Checked with `bash -n` and with two stub harnesses in place of a device (shellcheck is
not installed here): one drives the extracted lifecycle functions against fake binaries
and asserts pid 0 really does hit the sender's process group, that an empty EMU_PID now
signals nothing and returns at once, that SIGINT cleans up once and exits 130 within
seconds, that KEEP_AVD survives the trap path, and that an explicit exit status survives
the EXIT trap; the other runs the real script end to end on the boot-failure path, which
stops short of e2e-run.sh, and checks the pins land in config.ini, the created AVD is
removed, a misplaced config.ini fails the level, and `set -u` is not tripped anywhere.
No emulator was booted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three defects in the three preceding commits, found on review. A commit set
whose subject is stale dates and inferred status cannot carry either.
Dates. Both correction blocks were stamped 2026-08-23. The commits are dated
2026-08-22, as is every other date in these two files and the review that
produced them -- a day in the future, in the one place a reader checks to see
how fresh a correction is. Corrected to the commit date, and the D6 note now
carries one too.
Coherence. The Status line was changed to say fix status "tracks main,
re-checked at 18c53a3" while "Last verified: 2026-08-22, against main at
903b43c" stood two lines below it, unchanged. A reader would take the whole
document as anchored to 903b43c -- exactly the failure being corrected. The two
anchors now say what each covers and that they move independently: the as-found
bodies are frozen at 903b43c, the status is not.
Inferred count. "detekt 0, lint clean" was carried over from the old text on the
strength of a BUILD SUCCESSFUL, which means "nothing above threshold", not
"nothing found" -- and this project deliberately keeps a real lint finding
visible (`informational += "UsableSpace"`), so "clean" was wrong as well as
unmeasured. Read off the reports instead: detekt.xml has zero <error> elements,
lint-results-debug.txt says 0 errors, 0 warnings, 1 hint. Stated that way, which
is also the convention the audit's own opening table already uses.
README's correction note is tightened from nine lines to six. It sits on the
first screen and nothing load-bearing is dropped.
Gate green: ktlintCheck, detekt, lintDebug.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
R18 / #27. The front-page status line said the FFmpeg format tests "have been
written but not yet executed on a device". They have been executed, repeatedly
and green, and this is the one line a contributor uses to decide whether the
FFmpeg path is trustworthy -- understating it costs more than a stale detail
elsewhere would.
FFmpegEngineTest's nine format tests -- mp3, gif, matroska, flac, wav, opus,
H.264, H.265, and the one asserting a failure surfaces as an exception rather
than a silent empty file -- were present at every commit cited below, checked by
counting @Test in that file at each:
- Physical Pixel 10 Pro XL, API 37, 2026-08-21 at edd6385: 40 / 0 / 0 / 2.
- The audit's own Pixel pass, 2026-08-22: 49 / 0 / 0 / 2.
- Four local emulator levels, API 33-36, 2026-08-22 at 22c7914: 49 / 0 / 0 / 2
each.
Zero failures in all of them, and the two skips are RealMediaBenchmark's
assumption-guarded tests, not these. CI's e2e matrix in status_check.yml covers
API 33-36 and the workflow triggers on pull_request against main, so "on every
pull request" is checked rather than assumed.
The replacement names the runs instead of a count, since a total is the part
that goes stale -- androidTest has moved 40 to 49 to 57 in a few days. It also
keeps the API 37 caveat visible: no CI row, so that level stays a manual Pixel
check before each release.
Held, not decided here: whether a front-page status line should carry a
verification claim at all. Fixing what it says is separable from deciding what
it should be for.
Gate green: ktlintCheck, detekt, lintDebug.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
R14 / #23, R15 / #24. "On testing these" proposed a plan; the twelve fixes then
executed it, so the section describes a state that no longer exists. It is
re-dated rather than deleted -- the reasoning is why the test stack looks the
way it does -- with each stale claim marked where it stands.
R15 / #24, four statements, each checked against this checkout rather than
against another document:
- "exactly one dependency, testImplementation(libs.junit)". There are five:
junit, robolectric, androidx.work.testing, the Compose BOM platform and
compose-ui-test-junit4.
- "a testOptions { unitTests.isIncludeAndroidResources = true } block, which
this module does not currently have at all". app/build.gradle.kts:105-110.
- "work-testing, compose-ui-test-junit4 and espresso-core ... have zero users."
By import, work-testing has seven files under app/src/test and
androidx.compose.ui.test has one. espresso-core really is still at zero, so
that third is kept as the only part still standing.
- The preamble's "OutputPublisher, both ViewModels, both Workers and
MainActivity have no JVM unit tests at all". 25 JVM test files were added over
that set, 180 tests to 257.
That last one is also the derivation of CLAUDE.md's ~31% coverage figure, so the
reasoning is kept verbatim and only its tense and scope are fixed: the ~31% is
what those ~1,200 untested lines produced at 903b43c, it predates the new tests,
and jacoco has not been re-run. The figure itself is deliberately not touched
here -- re-measuring it and updating all three sites together is its own change.
R14 / #23. The audit asserted in two places that instrumented tests cannot run
on this host, and recorded the opposite in a third. 22c7914 is merged and
tools/local-emulator/run-e2e.sh runs API 33-36 locally, so the section's
impossibility argument for "pure seams plus Robolectric" is restated on the
grounds that survive -- speed and determinism, which is the weaker claim and
worth making honestly. D6's "it is an instrumented test, so it runs on CI, not
locally" gets the same treatment, and went further the other way: the
StateRestorationTester test that entry asked for is AppRootRestorationTest,
which runs under Robolectric on :app:testDebugUnitTest. The API 37 rule is
explicitly left standing -- that image is broken and the Pixel check before each
release is unaffected.
No as-found body was rewritten; D6's note is appended to its "Fix direction and
test" guidance, not to its description of the defect. Gate green: ktlintCheck,
detekt, lintDebug.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
R3 / #12, R12 / #21, R13 / #22. Three status claims in the audit were false
against `main` at 18c53a3. The audit is read as the work queue for the ticket
phase, so each correction is stated in the document rather than made quietly --
a reader who believed the old claim needs to see that it changed.
R3 / #12. D5 and D7 were marked `open` and "in progress on
fix/space-proxy-and-notification". Both merged hours before: b86df47 (D5) and
c2e6344 (D7) are ancestors of 18c53a3 (`git merge-base --is-ancestor`), and so
is the branch. Anyone working the table would have re-implemented merged work.
The header count was also wrong in its own arithmetic -- "ten fixed, two in
progress, one parked" covers thirteen of sixteen entries, dropping D12, D15 and
D16. It now states four numbers that sum, and says the Fix column tracks `main`
at a named commit so the next reader knows what it is relative to.
R12 / #21. "Where the fixes live" sent the reader to `feat/defect-fixes-base`,
for which `git show-ref` finds nothing -- no local ref, no remote, deleted when
it merged -- and quoted 242 JVM tests. A real run on this branch gives 257 / 0 /
0 / 0 across 34 classes; the 15 missing are UnknownInputSizeTest, SpaceCheckTest
and ProgressNotificationTest. The replacement names `main`, anchors the total to
this commit and gives the command to re-derive it, since the number is only as
fresh as the document.
The same paragraph's "API 33-36 now run locally, 49 tests each" is anchored to
22c7914, the commit that measured it: androidTest is 57 `@Test` on `main`, and
an unanchored total invites a reader to mistake drift for breakage. The
surviving invariant -- 0 failures, 0 errors, 2 skipped, same total at every
level and on the Pixel -- is stated instead.
R13 / #22. D11 was marked `merged`, but 7db3200's own body says one of its four
rows was deliberately skipped: "Not touched: OutputPublisher's hasSpaceFor
KDoc". Still true -- nothing in OutputPublisher.kt mentions D1 -- and that row
is the one place a reader of the code would learn the parked defect exists. The
summary row now says "less the OutputPublisher KDoc row -- held with D1", with
the reasoning at the entry.
The as-found bodies are untouched throughout, including D11's own item table:
corrections are carried as marked editorial notes beside them, the pattern D1's
entry already uses. Gate green: ktlintCheck, detekt, lintDebug.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Jason Ross
JMR-dev