84.9% line / 63.8% branch, 454 tests -> 87.1% line (2025/2324), 69.1% branch
(974/1410), 502 tests in 71 classes, as #132 and #133's ten children land.
The entry already instructs re-measuring before quoting, and that is why this is
here rather than in the batch: quoting these numbers before the work merged would
have described a tree that did not exist. It nearly went wrong the other way too
-- the first measurement for this commit was taken against a main that was three
merges stale and read 85.1%.
Also says something the bare numbers do not. Branch moved 5.3 points against
line's 2.2, and that asymmetry is the expected shape of this kind of work rather
than a curiosity: those children targeted decision code -- enum fallbacks,
refusal arms, cursor shapes, a `when` over container rules -- where one test
chooses a branch the suite had never taken. Line coverage barely notices that.
Branch coverage is the whole point.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The JVM suite had no timeout of any kind, so #125's Room/WorkManager lock-order
inversion ran until something outside it gave up: 47 minutes locally, and on CI
it would burn the Unit tests job's 30-minute cap and report as a job timeout
with no cause. The deadlock is monitor contention, which no interrupt breaks, so
nothing inside the JVM could have ended it either.
The obvious fix does not work here. A JUnit `Timeout` -- as a rule or as
`@Test(timeout = ...)` -- runs the test body on a separate thread, and every Compose test in this
source set goes through Robolectric's paused main looper. Both forms fail with
"main looper can only be controlled from main thread"; the same tests with the
timeout removed pass, so it is the mechanism and not the probe.
So the bound comes from outside the test JVM, where it moves no threads:
`timeout` on the Test tasks kills the forked worker, and a watchdog jstacks that
worker two minutes earlier. The jstack is the point. Gradle's timeout on its own
kills silently, a timed-out run writes no XML for the class that hung, and the
JVM's own "Found one Java-level deadlock" section naming both monitors is the
only reason #125 could be described at all -- so it goes to stdout as well as to
a file, because the Unit tests job uploads only reports/tests/.
Ten minutes is against the slowest observed passing run, not the typical one:
eight CI samples of the whole invocation ranged 62-90s, so this is ~6.7x that
and a third of the job cap. A timeout that fires on a healthy slow runner turns
a real signal into noise.
Both numbers live in a build script that nothing compiles, so HangBoundTest
reads them back and the build script joins build.yml as a declared input --
without that the guard would go stale on exactly the edit it exists to catch.
Refs #125.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This PR was opened quoting 81.4%, measured on b53f326. Holding it until #116,
#117, #119, #121 and #124 merged was the point: by the time it was ready the
number had moved three points, which is the same staleness the entry is about.
Measured on 93ebfa6 with ./gradlew :app:jacocoTestReport:
LINE 1971/2321 84.9% (81.4% four hours earlier, 69.2% on 2026-08-24)
BRANCH 900/1410 63.8% (60.2%, then 53.2%)
454 JVM tests in 67 classes, all green.
The note now says the entry went stale while it was open, because that is a
better argument for the rule than the rule restating itself.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The report added by #111 runs on every path out of e2e-run.sh, including the
wedge, and until now it answered a question it had not been asked. On job
98035980326 -- API 34, a docs-only PR -- it printed `received: 59` and
`completed cleanly: yes` six seconds before `##[warning] ... WEDGED`, for a leg
the WEDGE_TIMEOUT had killed 22 minutes in. `completed cleanly` means only
"instrumentation was not aborted", which was true; a reader scanning the table
had to notice a separate warning line to learn the leg had died.
The wedge cannot be read out of the log, which is why it is passed in: a wedge
is gradle never returning, so gradle printed no verdict, no truncation line and
no INSTRUMENTATION_ABORTED, and the log it leaves is the log of a run that just
stops. Only e2e-run.sh saw `timeout` exit 124. It now derives that fact once and
tells the report as E2E_WEDGED_AFTER, and reuses the same variable for
capture_wedge so the two cannot drift.
The table gains a `wedged:` row above `completed cleanly`, and `completed
cleanly` flips to no -- but only where it would have said yes. An abort already
says no and names the abort, which the wedge row does not, and a run that left
no evidence still says unknown; a wedge on top of either prints both facts.
`received`'s source line told the same lie in the same table -- "the run was not
truncated, so every expected test reported" is only "gradle never got as far as
saying so" when the leg was killed -- so it is qualified on that path. The
number itself is unchanged, and so is `failed: unknown`: gradle printed no
summary line, so that count genuinely is not knowable.
Nothing here decides anything. No exit status, no pass/fail rule, no baseline
comparison and no `::notice::` behaviour changes; the leg already failed
correctly and still does.
Verified against captured CI output rather than a live emulator, as #111 was and
for the same reason -- this host cannot run API 37 and cannot wedge on demand.
Four real logs (the wedged leg, a green API 34 leg, a failing gating leg, and an
advisory leg with its baseline deviation) through both versions of the script,
in both env states, comparing stdout and the job summary: only the wedged run
with the signal set differs, byte for byte.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CLAUDE.md's own rule is "re-measure before quoting", and the figure it carried
was measured on 2026-08-24 -- before the #52 children, the MediaProbe and codec
tests, and the guards from #100/#107 landed. Quoting it now would understate the
suite by twelve points, which is the same failure the bullet directly below it
was written to describe.
Measured on b53f326 with ./gradlew :app:jacocoTestReport:
LINE 1847/2268 81.4% (was 1519/2194, 69.2%)
BRANCH 837/1390 60.2% (was 53.2%)
against 417 JVM tests in 60 classes, all green.
The denominator moved too, 2194 -> 2268: the same push added production code of
its own, so this is not a pure numerator gain and the note now says so. The
Robolectric/isIncludeNoLocationClasses history is left exactly as it was -- it
explains why every pre-2026-08-24 figure was an artifact, and that is still the
most useful thing in the entry. "That date" is now spelled out, since the
headline date above it has moved and the phrase no longer points at itself.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The API 37 entry names how many instrumented tests there are and how
many the gating leg runs, and this PR adds one. Nothing asserts those
figures, which is exactly why they rot quietly: 59/56 becomes 60/57.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
That job is continue-on-error and red on every PR by design, which CLAUDE.md
states plainly -- and that instruction is exactly why nobody reads it. Nothing in
a red X separates "the known three" from "the known three plus yours".
A bare failure count would not have fixed it, and this is measured rather than
assumed. The run is usually truncated: seven of eight advisory runs read on
2026-08-25 ended in `Test run failed to complete. Expected 3 tests, received 2.`
with INSTRUMENTATION_ABORTED, and one did not. A count taken from a truncated run
misleads in both directions -- a fourth marked test can still yield the same
number if the abort lands earlier, and the known set getting worse can lower it.
The test XML does not rescue it either, which was the thing worth checking before
building on it: it IS written for an aborted run, and it reports a tidy
tests="3" failures="3" for a run the runner had just described as truncated. So
the XML is the authority on how many results landed, the runner's own output is
the only authority on whether the run finished, and the report reads both and
says which number came from where.
The baseline is one number beside the marker, because the marker means "cannot
pass on this image": the count is both how many tests the advisory leg runs and
how many should fail. A smaller failure count is the interesting direction -- it
means one now passes, which is the documented trigger for deleting the
annotation.
Nothing about the job's status changes. It stays continue-on-error, stays red,
stays out of the required contexts; a deviation is a ::notice::, never an
::error::. The report is a separate script so it can be run against a real log
saved from a real CI run, which is how the comparison was shown to fire.
The gating legs get the shape without the comparison: they run the whole suite,
so comparing there would announce a deviation five times a run -- but a truncated
run reporting fewer results than it ran is what #108 looks like, and "completed
cleanly" is the field that would show it.
Closes#83
The shellcheck step added a few hours ago reads `git ls-files '*.sh'`. That is four files.
It does not read the inline `run:` blocks, and a good deal of this repo's bash lives there:
the release verification in build.yml, the emulator setup and teardown in status_check.yml
and api37-debug.yml. "shellcheck runs in CI" was true of the files and not of the blocks,
and CLAUDE.md said so rather than pretending otherwise.
actionlint closes that half. It parses each workflow and runs shellcheck over every `run:`,
on top of its own checks for expression syntax, `needs:` references, matrix keys and action
input names.
Pinned by digest, for the reason shellcheck is pinned -- a new rule making untouched files
fail is a red build whose diff cannot explain it -- and for a second reason of its own.
actionlint's documented install is
bash <(curl -s https://raw.githubusercontent.com/.../download-actionlint.bash)
off a moving branch. Running that in a repository that pins every action by SHA would
contradict its own supply-chain posture more than the linter is worth. That is why #70 was
filed instead of bolted onto the shellcheck commit.
It reported exactly one finding, and it is fixed here rather than suppressed: build.yml
parsed `ls` to pick the release APK (SC2012). The glob was already in the line, so a bash
array reads it without the pipe. Gradle's output names have no spaces today, which is the
kind of assumption that holds right up until it does not.
Proved it catches something, rather than trusting a green run: planting `if [ $UNQUOTED =
bad ]` into a build.yml `run:` block produces
shellcheck reported issue in this script: SC2086:info:4:6:
Removed again afterwards. A linter that cannot be shown to catch a plant is not wired in,
it is just running -- and SC2086 in a `run:` block is invisible to the .sh-file step, which
is the whole argument for this commit.
CLAUDE.md loses the "does not cover inline run: blocks" caveat, because it no longer does.
Both linters verified clean at their pinned digests.
Closes#70.
#80 added SafPickerRoundTripTest's rotation case to @FailsOnEmulatorApi37, because a
real rotation aborts the framework on android-37.0. Three tests carry the marker now --
two in Media3EngineTest, one in SafPickerRoundTripTest -- and five statements still
described two.
Four were counts, and wrong:
status_check.yml "notAnnotation removes the two tests that do not pass"
status_check.yml "The two API 37 tests the gating row above excludes"
CLAUDE.md "the gating leg runs the other 55" (59 - 3 = 56)
CLAUDE.md "do not read a green run as evidence those two tests pass"
The fifth was worse, because it was not a count. The advisory job's header justified its
name with an invariant:
"It is named for WHAT IT RUNS, deliberately. Both tests drive a full H.264 -> H.265
hardware transcode through Media3Engine"
The rotation case drives no transcode. So the comment did not merely miscount -- it
asserted a property of the job's contents that had stopped being true, and that property
was the entire argument for the name.
The name is unchanged, deliberately, and the header now says so instead of implying the
question never arose. This is not a required context, it is red on every PR by design, and
it is one people have learned to look for; renaming a check costs more than the imprecision
does. What replaced the invariant is the honest rule: THE MARKER IS THE DEFINITION, NOT THE
NAME -- this job holds the tests that cannot pass on the API 37 emulator image, whatever
their subject.
Two things stay as they were because they are still true. "the two Media3EngineTest cases
that pass here" is correct: that class has four tests and two carry the marker. And the
decoder theory is still a claim about the Media3 pair alone, so it now says so rather than
being read as covering a rotation failure it has nothing to do with.
Nothing about the job's behaviour changes: same name, same continue-on-error, same marker,
same selection on both rows. Verified: yaml parses, five jobs, matrix still 33/34/35/36/37.
The check to re-run when a test next joins or leaves the marker, which is the event that
broke this twice:
grep -rn "@FailsOnEmulatorApi37" app/src/androidTest --include='*.kt' | grep -v import | grep -c FailsOn
It must equal the number every corrected comment states. It is 3.
Closes#81.
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.
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.
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>
The previous commit settled for Java 24 everywhere because Kotlin 2.2.10 refuses
jvmTarget 25. That was the wrong constraint to accept, for two reasons.
The first is that 24 turned out to be unbuyable. Adoptium's repository carries
8, 11, 17, 21, 25 and 26 -- no 24, because it is a non-LTS that went end of life
in July 2025. The builds passed only because Gradle quietly auto-provisioned
24.0.2+12 through foojay, and .idea/misc.xml had been pointed at a temurin-24
that cannot be installed. A toolchain nobody can install is not pinned, it is
lucky.
The second is that the cap was never on the toolchain at all. Kotlin's ceiling
applies to jvmTarget -- the bytecode -- and the JDK running the build is a
separate axis. Conflating them is what steered this at 24 in the first place.
So the fix is the one the sibling repo already uses: put KGP on the root
buildscript classpath, where AGP's built-in Kotlin picks it up instead of the
2.2.10 it bundles. Kotlin 2.4.10 supports jvmTarget through 26, which lifts the
ceiling above the toolchain rather than under it. The Compose compiler plugin is
versioned in lockstep and reads the same catalog entry, so the two cannot drift,
and the module now applies both by id() because they come from the classpath
rather than from plugin resolution.
Checked rather than assumed, since a silent downgrade would look identical to
success: compiled classes report major version 69, which is Java 25. D8 dexes
them, R8 minifies them, and ktlint, detekt, lint, the unit tests and the
androidTest compile are all green on top.
25 is the right landing place independent of all this: it is LTS, it is in the
Adoptium repository, and temurin-25-jdk is already installed here -- so the
daemon runs on a real system JDK rather than a provisioned copy of an unpatched
one.
Two catalog plugin aliases went with it. android-application and kotlin-compose
now resolve from the buildscript classpath, so leaving aliases behind would have
left two entries that read like the source of truth and control nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Library versions now read "1.+" instead of "1.19.0". Three groups stay pinned,
and the reasons differ:
agp/kotlin/ksp are version-locked to each other -- AGP 9.3.1's POM declares
kotlin-gradle-plugin 2.2.10, so a float that picked up Kotlin 2.4.x would put
the Compose compiler ahead of the Kotlin AGP actually compiles with.
ktlint/detekt/jacoco because a linter is not a library. A library bump that
misbehaves usually still compiles; a new lint rule makes files nobody touched
stop passing, turning a PR red for something absent from its diff. Upgrading
those is worth a commit that reads the new findings.
The FFmpeg AAR is a committed file, not a coordinate.
The componentSelection block is the part that makes this safe rather than the
part that makes it work. Gradle resolves "+" to the highest version it can find
and does not skip prereleases, and androidx routinely publishes alphas numbered
above the current stable: lifecycle 2.12.0-alpha01, work 2.12.0-rc01, navigation
2.10.0-rc01, datastore 1.3.0-alpha10, annotation 1.11.0-alpha01 all outrank the
releases this app uses. Without the guard, five dependencies would have moved
onto unreleased code on the next build with nothing in the diff to say so. With
it, every float resolves to exactly the version that was pinned before -- checked
against :app:dependencies, not assumed.
So this changes nothing today. Every library was already at its newest stable
when the catalog was audited; floating is about what happens next month, not
this commit.
Trying a prerelease is still possible: name the exact version, which pins it
rather than floating it. That is the right way round -- an alpha should be a
deliberate act with a version number attached to it.
Verified: ktlint, detekt, lint, unit tests, androidTest compile, assembleDebug
all green, and the configuration cache still reuses across runs of the same task
set.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two things the first pass got wrong.
CLAUDE.md said "use a JDK 17-21, AGP 9 does not support 25+". That was carried
over from the sibling repo and is not true here: gradle-daemon-jvm.properties
pins toolchainVersion=25, so Gradle provisions and runs the daemon on Java 25
whatever JAVA_HOME says -- JAVA_HOME only picks the launcher. `gradlew --version`
prints both, and shows them differing on this machine right now. It also means
CI's java-version: '17' is not the JDK that compiles anything, and that the
daemon JVM is the same on a laptop as on a runner, which is a better guarantee
than the one the file claimed.
Marking ConversionDependencies @UnstableApi propagates to its callers, and
FakeFailures in androidTest calls it. That is a warning rather than an error in
Kotlin, and lint does not read the androidTest source set, so the previous
commit compiled clean while leaving one file inconsistent with the very pattern
it described. Marked now.
Left alone deliberately: gradlew.bat. The new `*.bat text eol=crlf` attribute
governs how it is checked out from here on, which is the point of adding it,
and the file already has CRLF in both the tree and the index. Rewriting the
stored bytes of the wrapper script to prove the attribute works is not this
branch's business.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
detekt found 29 and Android lint 6, on a codebase neither had ever seen. Each
one was either fixed or relaxed with the reason written next to it; nothing was
suppressed to make the build quiet.
Fixed, because the tool was right:
- ConversionDependencies constructs Media3Engine, which is @UnstableApi, and
was not marked. Every other type here that touches Media3 propagates the
marker rather than swallowing it with @OptIn, so this one does too. Lint
was the only thing that had ever noticed.
- MediaProbe converted microseconds to milliseconds with a bare 1000, twice,
in a file that also handles a seconds-based duration from a different API.
US_PER_MS and MS_PER_SECOND now say which is which -- that confusion is a
real bug source in media code, not a style question.
- Foreground service types compared SDK_INT against 34 and 35 as raw ints
while the doc comment above spelled the version names out. VERSION_CODES
says it in the code.
- take(3) is a product decision about how many alternatives an error offers.
It means nothing until it is named; MAX_SUGGESTIONS does.
- setProgress(100, ...) is a percentage max, now PERCENT_MAX.
Relaxed, because the rule did not fit:
- The model package is excluded from ReturnCount and CyclomaticComplexMethod
ONLY. It is the decision layer: ConversionRouter.route scores 17 because
the app can give 17 distinct answers to "which engine, and why", each with
its own user-visible reason, and route's own comment records that their
ORDER decides which message is shown. Counting those as complexity measures
how many answers exist, not how hard the code is to follow. Everything else
-- LongMethod, NestedBlockDepth, ComplexCondition -- still applies there.
- A flat `when` used as a lookup table scores a point per entry, so
MediaProbe's demuxer-name to Container map read as complexity 21 with no
nesting and no state. ignoreSingleWhenExpression is the rule's own answer.
- TooGenericExceptionCaught off. MediaProbe, ConversionWorker and ConcatWorker
sit in front of native code that reports a malformed file as anything from
IllegalArgumentException to a bare RuntimeException, undocumented.
Enumerating that list means guessing, and a wrong guess crashes the app on a
file it could have reported as unreadable. SwallowedException stays on, so
these still have to log and handle.
- SI thresholds in the byte formatter, via ignoreNumbers. Each literal sits on
the line with the unit string it belongs to; BYTES_PER_MB would need a Long
and a Double and say nothing the line does not.
- allowedFunctionsPerObject, which the first pass simply missed.
Lint's three version-freshness nags are off. They do not describe this code --
they go red the day someone else publishes a release, which turns a PR red for
something its author cannot see in their diff, and they want the network at
lint time. Upgrades here are deliberate; Kotlin in particular is pinned to AGP's
bundled KGP and is not free to follow the newest release.
UsableSpace is informational rather than disabled, because it is a real finding
that this commit is choosing not to act on. hasSpaceFor reads File.usableSpace,
which ignores reclaimable cache, so the app can refuse a conversion it had room
for. StorageManager.getAllocatableBytes is the better answer, but it changes
when a job is rejected and can throw -- a behaviour change to a safety check,
which deserves its own commit and its own test rather than a drive-by here.
informational keeps it in every lint report instead of hiding it.
Also adds the CI gate and a CLAUDE.md. Coverage is reported and not gated: the
measured baseline is 31% of lines, which is exactly why LibreMail's 0.84 floor
was evidence about LibreMail and not a number to copy.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JMR-devandClaude Opus 5