Nothing had ever handed InputQuery a cursor row. UnknownInputSizeTest
drives the no-provider case thoroughly -- query returns null, measure()
answers -- so firstRow's body, displayNameOrNull and sizeOrNull had never
executed at all.
Nine tests over FakeSafProvider's RowShape states. What they pin is not
"reads a cursor" but the rule the class exists for: a size nobody could
determine must arrive as null, never 0. Four separate ways a provider
fails to give one -- a null cell, a missing column, a negative value, an
empty cursor -- plus a provider that throws outright, which is the guard
firstRow's KDoc is written for.
Mutations run, all four bite:
drop `takeIf { it >= 0 }` from sizeOrNull -> negative-size test red
drop `!isNull(it)` from sizeOrNull -> null-size test red
drop the runCatching in firstRow -> throwing-provider test red
drop `!isNull(it)` from displayNameOrNull -> GREEN, does not bite
That last one is recorded in the test's KDoc as a named exemption rather
than papered over. Measured: MatrixCursor.getString on a null cell returns
null while getLong returns 0. So the guard is load-bearing on the size path
-- it is what stops a null becoming a real number -- and unfalsifiable on
the name path, where getString already yields null. It stays regardless:
Cursor.getString's contract makes throwing on null implementation-defined,
and a real provider may do what MatrixCursor does not.
InputQuery.kt now has no never-executed lines. Suite 456 -> 465 tests,
branch coverage 63.8% -> 65.6%.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two of #132's items are cursor-shaped -- InputQuery's row reads (#137) and
OutputPublisher.destinationIsKnownEmpty's short-circuits (#140) -- and the
provider that could drive them lived inside OutputPublisherPublishTest and
could only answer correctly. Its row was always (file.name, file.length()).
Moved FakeSafProvider, FakePlainProvider and the registration helper to
FakeProviders.kt, same package, following StagingCleanupSupport.kt and
ParkedPickDispatcher.kt. UnreliableOutputStream stays behind: it serves one
test, which is the line WorkerStubs.kt draws.
Added RowShape, seven ways a provider can answer a metadata query. Column
granularity is deliberate -- OutputPublisher reads only SIZE, InputQuery
reads both and reaches different answers depending on which is bad -- and so
is keeping null, missing, negative and no-row distinct rather than folding
them into one "bad" case. That distinction is the whole reason InputQuery
exists: hasSpaceFor(0) is only "is there 128 MB free", so a size nobody
could determine must not arrive as 0.
No production change. OutputPublisherPublishTest, OutputPublisherStagingTest
and UnknownInputSizeTest pass unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A gap audit over the eight branches merged together looked for lines still
never executed and asked, for each, whether something already accounts for it.
Everything mapped except one: `ConcatWorker.kt:42`, the refusal of a join with
fewer than two inputs.
Its neighbour maps. `ConcatWorker.kt:40` -- the missing-URI-array arm, two lines
above -- is covered on the device by
`UnopenableUriTest.aJoinWithNoInputArrayFailsWithAMessage`. That is invisible to
JaCoCo, which measures `testDebugUnitTest` only, so the report shows both arms
cold and cannot distinguish the one that is e2e-covered from the one nothing
touches. Only reading the androidTest source separates them.
`grep` says nothing in either source set mentions "Pick at least two files to
join." Two tests here now do:
- `a join of a single file is refused with a message rather than joined` pins
the verdict and the message together, via `Failure.equals`, for the reason the
file's header already gives.
- `a join of two files is not refused for its count` is the control that puts
the assertion on the boundary rather than on the string. It refuses the
*space* rather than letting the job run: the next thing past the count guard
is `ConcatEngine`, which is native, and `NamingPublisher`'s KDoc already
records that no JVM test gets past it. A failure carrying the space message is
proof execution reached line 57, which is proof it cleared line 42, at no
engine cost.
Reachability is the header's argument plus one of its own: `request(...)` takes
a `List<Uri>` and checks nothing about its length, so a one-item join is a
well-formed call rather than a corrupted queue entry.
Mutations, each killing exactly the test it should:
| mutation | red |
|---|---|
| guard deleted outright | `a join of a single file is refused...` |
| `uris.size < 2` -> `< 3` | `a join of two files is not refused for its count` |
Restored, both green. Gate green: assembleDebug, testDebugUnitTest,
compileDebugAndroidTestKotlin, ktlintCheck, detekt, lintDebug.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#84 closed by classifying probeWithExtractor and probeForConcat as
device-bound and explicitly not a gap. That was right about FFprobe and
right about the measurement boundary, and wrong that these are only
orchestration. The track walk is a branch matrix, and androidTest reaches
it only through whatever the committed fixtures happen to contain -- so
none of its rules is *chosen* by any test there.
#133 offered two ways to reach it: drive ShadowMediaExtractor, or cut the
loop into a pure function. Taking the second, which is the pattern
CLAUDE.md names and work/FailureOutcome.kt documents. extractedFrom and
concatInputFrom take List<MediaFormat>; what is left needing a device --
setDataSource, getTrackFormat, release -- is one three-line extension
function, which is the thin edge androidTest should be covering.
The two are deliberately not merged despite the overlap. One reads
duration and not frame rate; the other reads frame rate and not duration.
A merged version would compute both for every caller, and ConcatPlanner
treats an unknown frame rate as "cannot prove a match" -- so a field the
join flow does not need must not start arriving as a number.
Eleven tests over cases no fixture provides: two video tracks, two audio
tracks, audio outlasting video, a track with no KEY_DURATION, audio
declared before video, a subtitle track, and no tracks at all.
Six mutations, six red:
last video track wins first-video test
last audio track wins first-audio test
duration = last rather than max longest-track test
drop the containsKey guard six tests (getLong throws on a missing key)
guess a frame rate of 30 no-frame-rate test
join takes the last video track join frame-rate test
MediaProbe's missed branches drop 91 -> 70; what is left is the FFprobe
half and the two catch arms, which are native and device-bound exactly as
#84 said.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both exits were cold, and both are reachable for the same reason: a job
does not have to come from the picker. WorkManager keeps work for about a
week, so a downgrade or rollback hands this build a job enqueued by
another one, and request(...) is callable directly.
:62 -- a missing KEY_INPUT_URI -- was untested everywhere, JVM and device.
The nearest e2e test, ForcedFailureTest.aMissingInputFailsRatherThanCrashing,
passes a URI pointing at a file that does not exist, which reaches the
engine and fails much later with a different message.
:124-126 -- the Validation.Invalid refusal -- had no test at all, though
its comment names both arrival paths it exists for.
Five tests, in a new file because both are about the *message*. A refusal
that fails with empty output Data renders the UI's generic "Conversion
failed." with nothing else to say, which is the defect shape
DeniedForegroundStartTest records from the device pass; asserting the
verdict alone would pass against exactly that.
Two of the five are there to stop the others passing for the wrong reason:
`a refused spec never reaches an engine` says it failed *before*
converting rather than during, and `a valid spec is not refused` is the
control -- without it every assertion here would still pass against a
worker that refused everything.
Three mutations, three red:
change the no-input message no-input message test
drop the validation refusal both refusal tests
validate but keep converting both refusal tests
L61-62 and L123-126 are now fully covered, branches included.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ConversionWorker has WorkerCancellationTest and DeniedForegroundStartTest.
Its twin had the retry case only -- `a join whose foreground start is
denied` already existed -- so two of ConcatWorker's three failure exits
were cold: the CancellationException arm, and FOREGROUND_DENIED.
Four tests, added to the files that own each rule rather than to a new
ConcatWorker file, which is how this suite is organised: a file per rule,
tested across both workers.
The cancellation seam is worth a look in review. The conversion twin
cancels inside the engine, which is honest there because
ConversionDependencies has a seam for it. ConcatWorker calls ConcatEngine
directly and has none -- it is native and nothing here gets past it -- so
the cancellation is injected at the only other point inside the try,
setForeground. That is a real shape rather than a contrivance: a job
cancelled while WorkManager is promoting it is exactly when that window is
open, and the catch arm cannot tell where in the try it came from.
FailedFuture moved to WorkerStubs.kt on the way. Two tests now inject two
different failures through it, and Kotlin will not take two file-private
top-level classes of one name in one package.
Four mutations, four red, each isolated:
cancellation arm -> Result.failure propagation test only
drop delete on cancellation cancellation-partial test only
FOREGROUND_DENIED -> Result.retry past-the-bound test only
drop delete on the Throwable path give-up-partial test only
ConcatWorker's :92, :95-96 and :105-106 are covered; missed branches 4 -> 3.
What is left is what the ticket scoped out: the two input guards (e2e), the
ConcatEngine success path (native), and getForegroundInfo (#88's named
exemption).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two halves of ContainerCapabilities.validate were written together
and only one of them was ever checked. Six audio outcomes had no test --
every one a string the user reads -- while the video twin of each was
already covered.
Seven tests, deliberately shaped like their twins rather than as a fresh
idea about what to assert:
unidentifiable source audio on a COPY twin of `an unidentifiable
source codec cannot be copied`
container cannot hold the copied source twin of `a codec the container
cannot hold is refused...`
container cannot carry it on encode twin of `H265 in AVI is refused`
this app cannot encode it twin of `copying is offered as
the fix when...`
accepts(_, AudioCodec.NONE, _) -> true twin of the VideoCodec.NONE arm
accepts(_, AudioCodec.COPY, _) throws twin of `resolving COPY before
asking the matrix is required`
The seventh is not the audio axis: validateVideo's copy-into-a-container-
that-cannot-hold-it refusal was the one video outcome with no test, and it
is the same shape and the same file.
Each asserts the message verbatim and re-validates every suggestion the
refusal offers. Validation.Invalid promises its suggestions are themselves
valid and names this class as the proof; the existing property test walks
the presets, and no preset reaches suggestions() through validateAudio.
Seven mutations run, seven red, each isolated to exactly one test:
CARRIES_AUDIO check -> false encode-path test only
drop the COPY error arm resolve-first test only
AudioCodec.NONE -> false no-audio-track test only
drop ENCODABLE_AUDIO check unencodable test only
drop audio copy container check audio-copy test only
drop video copy container check video-copy test only
drop unidentified-audio guard unidentifiable test only
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WorkerEnumFallbackTest already existed for this defect class -- a name
this build does not define, read above the try, throwing out of doWork
entirely: FAILED with reschedule=false, empty output Data so the screen
said "Conversion failed." with nothing else, and the staged file never
deleted. It covered 2 of the 5 above-the-try reads. readSpec's three
were the ones left, and all three were cold.
The baseline is the part worth reviewing. readSpec returns the *entire*
fallback spec the moment any one axis fails to resolve, so a test
starting from MP4_H265 -- which is itself the fallback -- cannot tell a
worker that read the spec correctly from one that gave up on it. These
start from MKV/H.264, which differs on container and video codec at
once, and assert the spec that actually reached the transcoder rather
than only that a Result came back.
Mutations run, four for three tests:
KEY_CONTAINER `?: return fallback` -> `?: error(...)` -> container test red
KEY_VIDEO_CODEC same -> video test red
KEY_AUDIO_CODEC same -> audio test red
fallback = MP4_H264 instead of MP4_H265 -> all three red
The first three confirm the tests are isolated to their own axis; the
fourth confirms they pin *which* spec ran, which is what "a Result at
all" would have missed.
readSpec is now fully covered, branches included.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found while decomposing #132 into children. It was item 6 there, and it
looked like the cheapest item on the list: three cold lines, a KDoc with
real user-visible stakes, and a permission Robolectric can flip in one
line.
grep -rn 'areEnabled' app/src returns the declaration and nothing else.
Both workers construct ConversionNotifications and only ever call
build(). So the behaviour the KDoc describes -- warning when progress
will be invisible -- does not happen, and a test would assert that a
function nobody calls returns what the platform told it. Green, vacuous,
and worse than nothing, because it would imply the disabled-notification
case is handled.
Recorded rather than tested, and the summary now names what F1 and F5
have in common: a comment describing behaviour the code lacks, where the
tempting fix freezes the wrong answer.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The ConversionForegroundType note asserted that #122's wedge no longer
kills the API 33 leg, on the evidence of a single run. The PR carrying
this document then wedged that exact leg: 23m08s, "wedged: yes --
gradle was killed after 1200s and never returned", failed: unknown.
Corrected to what the runs actually show: intermittent, not resolved --
five of the last six completed legs passed in ~7 minutes. And the
distinction the wedge row exists to draw is now stated, because it is
what keeps #88's reasoning intact: received: 60 means all sixty tests
still reported, so the API 33 regime was exercised; it is the failed
count that reads "unknown", so the leg could not have reported a break.
Also names what that changes -- a @Config(sdk = 33/34) JVM test is
worth three lines as insurance against a leg that cannot be trusted to
go red, which is a different and much smaller claim than the uncovered
behaviour this first looked like.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#132 holds the seven JVM test gaps from the same read, #133 the three
seam questions. The doc drew the line between them in prose already;
this makes it followable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four things came out of re-measuring coverage that a test would document
rather than repair, so they go in a doc rather than a ticket:
- F1 FFmpegCommandBuilder emits a Vorbis encoder ContainerCapabilities'
own comment says nothing emits. Traced unreachable through four call
sites, but the interesting reading is the other one: FFmpeg can encode
Vorbis, WebM and OGG carry it, and the picker never offers it.
- F2 ConversionRequest.hardwareEncodeAvailable is written once and read
by nothing; its KDoc describes a Fast-tier preset choice that was
removed, and the router computes the same answer itself.
- F3 ConversionRequest.videoCodec/.audioCodec have no callers anywhere.
Named as NOT a test gap: asserting a delegation restates it.
- F4 Two private guards reachable only by direct call. No action, per
the judgement #88 reached about getForegroundInfo.
Also records two things the read makes look like gaps and are not: the
Compose screens' branch numbers (inflated by compiler-synthesised
recomposition checks; the line figures are 34/383 and 20/143), and
ConversionForegroundType, where #88's premise was re-checked against
#122's wedge and holds -- the API 33 leg completes 60/60 cleanly.
Entry ids are F1-F4 so they cannot be confused with defect-audit.md's
D1-D16, and the confidence vocabulary is deliberately that document's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It has never been committed, so nothing is wrong today -- but nothing stops it
either, and `git add -A` would stage Kotlin build-session state into history.
It belongs beside /build, .gradle and .cxx, which are the same category and are
already here. Placed with them rather than in a section of its own, and left
without a comment: unlike tools/ffmpeg/out/ and .claude/, there is no non-obvious
choice here to explain.
Verified rather than assumed:
$ git check-ignore -v .kotlin
.gitignore:16:.kotlin .kotlin
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comment described adopting a later build's worker as an edge case. It is the
ordinary CI shape: the worker is found by scanning this daemon's descendants for
GradleWorkerMain, which cannot tell one invocation from the next, and the Unit
tests job runs testDebugUnitTest and jacocoTestReport back to back against one
daemon. Still harmless -- the watchdog only reads and writes -- but a reader
should not have to rediscover that.
Refs #125.
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>
The advisory baseline check has announced a deviation on every PR since #113:
the tree "carries 4 tests marked @FailsOnEmulatorApi37" where it carries three
and FAILS_ON_EMULATOR_API37_BASELINE says three. The fourth is a KDoc in
Media3EngineTest saying the opposite -- "Deliberately not
`@FailsOnEmulatorApi37`: nothing here decodes or encodes" -- which the old
matcher counted because it looked for the string anywhere on any line.
Neither ingredient was wrong on its own, and the number is not the real damage.
#83 added this check so that a new failure joining the known ones could not be
invisible; a notice that is wrong every single time teaches everyone to skim
past deviation notices, which is precisely the signal it was built to create.
Editing the baseline to 4 would have silenced it by breaking it -- the check
would then have been wrong the moment someone added or removed a real marker.
Anchor the pattern at line start and require whitespace or end-of-line after the
name. The second half is the part that is easy to get wrong: "only the
annotation on a line of its own" also stops counting `@FailsOnEmulatorApi37
@Test`, which is legal Kotlin, and undercounting is the dangerous direction --
it hides a genuine new marker, the one thing this exists to catch. Measured
against a fixture carrying every shape at once: the old matcher 5, own-line-only
2, this one 3; on the real tree 4 / 3 / 3, so the baseline is untouched.
`grep -v import` goes too, since `^[[:space:]]*@` cannot match an import.
The check is a pure function of the working tree, so the fixture is committed
and e2e-report-shape-test.sh runs the real report against it -- inside a
throwaway repo root, which the script finds from BASH_SOURCE, so no knob had to
be added that could point the live count somewhere else. The fixture sits under
.github/, where Gradle does not compile it and :app's ktlint and detekt do not
see it; running the report against the real root with it committed still
reports 3.
Every other path through the report is byte-identical to the previous version on
both stdout and the job summary -- passing, failing, wedged, no-run, and
advisory-with-an-unreadable-baseline all diff empty -- and the two advisory legs
differ only by the false line disappearing. No job's status or pass/fail rules
change; the advisory leg stays continue-on-error and stays red by design.
The test is deliberately not wired into CI: adding a step to Static analysis
would add a new way for a gating job to go red, which #120 ruled out. shellcheck
still covers the file, since that step reads `git ls-files '*.sh'`.
Closes#120
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>
Two accuracy fixes to notes the earlier commits left behind.
The test KDocs carried "roughly 1-in-130" and a 400-leg-attempt denominator.
Both come from earlier comments on #49 that its own census later replaced --
that ticket has three recorded corrections to its rate claims, and a
superseded figure in a permanent comment is the exact thing its author kept
having to fix. What survives the corrections is the count and the spread:
four occurrences, API 33, 35 and 36, every one on attempt 1 and green on
re-run.
The save exemption described a real defect with nowhere to look it up. It is
#123 now, so the KDoc names a number instead of trailing off.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The note claimed `save` is left unguarded because nothing can overwrite what
it writes. That half is true -- the only observation that could belongs to a
job already in a terminal state. The other half was missing: a save whose
copy is still in flight when the user taps Start over lands `Saved` on a
screen they have just cleared.
Guarding it would drop that write instead, which reports nothing for a file
that may genuinely have reached the destination. That is a question about
what the screen should offer during a save, and answering it in a race fix
would be deciding it by accident.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`reattach()` read `_state.value`, found it `Idle`, and then handed the job
to `observe()` -- which launches a *separate* coroutine that cannot write
until its `collect` has resumed with a `WorkInfo`. So the check happened at
one moment and the write landed at another, with a whole pick able to fit
in between: the user tapped, their metadata query suspended, the guard saw
an empty screen, and the finished job from an earlier session wrote over
`Ready(picked)` a moment later.
The comment above that guard said "no suspension point between this check
and the assignment below, so nothing can interleave". There is no
assignment below, and the two lines are in different coroutines. That
sentence is why this sat as flaky CI for two days rather than being read as
the product race it is.
`ScreenOwnership` makes the answer the test already encodes -- the user's
pick wins -- true rather than probable. A claim is taken synchronously when
the user acts; every write that lands after a suspension point checks the
claim it was made under and drops itself if that claim has been superseded.
Dropped, not reordered: a write that is dropped cannot come back later.
Cancelling the superseded observer was never enough on its own. `Job.cancel`
is honoured at the next suspension point, and a collector that has already
resumed and is on its way to `_state.value = ...` has none left; the write
lands anyway. It also cannot help at all in the case reported, where nothing
supersedes the observation until after it has been launched.
`JoinViewModel` had the identical shape and nothing watching it, so it gets
the same fix and the counterpart test that was missing. Its pick dispatcher
becomes injectable for the same reason `ConversionViewModel`'s already was:
without that seam there is no way to ask what happens while a pick is still
in flight.
Closes#49
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>
Refusing "copy the video" for a file that has none built its one suggestion by
hand — drop the video track and leave everything else alone. That is valid only
when the audio axis already happened to be fine. For any audio the target cannot
carry (Vorbis or PCM into MP4, MP3 into WebM) the offer is refused in the next
breath, so the Advanced picker showed a one-tap fix leading straight to a second
error. Nothing unsafe shipped — ConversionWorker re-validates — but it is a dead
end, and it contradicted the promise Validation.Invalid makes in its own KDoc.
Route it through the shared repair-and-filter path instead, as every other branch
does. Excluding what the *user* asked for rather than the already-repaired spec
is what keeps the case that worked working: an MP3 into MP4 still gets its copy
offered, because the repair of a copyable track is that same copy.
Only a branch that builds its own list can break that promise at all, since
suggestions() ends by filtering on validate().isValid. The property test now
covers both of them — this one and the image output — rather than reaching them
by luck, which is how a dead-end chip survived two earlier widenings of it. Its
failures name the probe too: three rows share a spec and differ only in the input.
Closes#114
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The KDoc claimed dynamic colour "stays switchable so users can opt back to the
brand palette". Nothing switches it: MainActivity is the only caller and passes
no arguments, so dynamicColor is always true and the two brand-palette branches
are dead. A reader who trusted that sentence would go looking for a setting that
has never existed.
Replace the claim with what is true today and point at #68, which holds the
decision -- add a switch, delete the dead branches along with the template
palette, or replace that palette first. None of the three is taken here.
ThemeKt had no test, so nothing would have caught the branches being swapped
either. Assert what the theme resolves by reading MaterialTheme.colorScheme
inside the content lambda: the two live branches on background luminance, which
is the one thing two schemes off the same device palette do not share, and the
dead pair by passing dynamicColor explicitly. Both KDocs say plainly that the
test is the only thing that passes it, so the coverage is not misread as
evidence a switch exists -- which is the misreading #68 exists to prevent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
save()'s onFailure keeps the staged file on purpose -- it can be the only copy
of an hour of transcoding, and the destination did not receive it -- and then
handed the screen a Failed carrying a message and nothing else. That branch
rendered exactly one control: "Start over", wired to reset(), which discards
precisely the file the comment above it goes out of its way to keep. The intent
was already written down in main; the UI did not honour it, and the only rescue
was process death followed by reattach -- unadvertised, and bounded by a sweep
that collects anything a day old.
Failed now carries a PendingSave, and only where the failure came from save().
A transcode that died staged nothing and must not sprout a save button, so the
handle is nullable and the observe() arm leaves it null; so does a save that
found the file already gone. The branch renders "Try saving again" above "Start
over", opening the same CreateDocument flow with the same name and type the
first attempt used. A retry that fails again lands back on a carrying Failed
rather than a bare one, so the second failure cannot eat what the first kept.
Start over still deletes from there, and that is a decision rather than an
inheritance: deletion is the user's choice only once the alternative has been
offered. pendingStaged remains the single owner of the delete, so the carried
handle is a view of it rather than a second owner and no path out of the state
can drop a file the old shape could not.
pendingSave() exists so save() and each screen's CreateDocument registration
answer "what would a save target" once instead of twice -- the entry points
cast to Converted/Joined, which answered null for a Failed and fell back to the
current pickers, wrong for any spec edited since the job ran and for every
reattached job.
Both tabs, since JoinViewModel and JoinScreen have the same shape.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JMR-devandClaude Opus 5