ConversionViewModel.save()'s onFailure keeps the staged file deliberately — "a failed save may
mean the staged file is the only copy of an hour of transcoding" — 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 calls discardStaged() on precisely the file the comment above
it goes out of its way to keep. The intent was already in main; the UI did not honour it. The
only rescue was process death followed by reattach — unadvertised, and bounded by the <24h sweep. JoinViewModel / JoinScreen had the same shape, and are fixed with it.
PendingSave(staged, suggestedName, mimeType) is shared by both state machines and lives next to STAGED_FILE_GONE_MESSAGE in OutputPublisher.kt, for the reason that constant is already there.
Nullable rather than a SaveFailed variant of its own: what the screen does with the message is
identical either way, so a second variant would make every exhaustive when grow an arm that
duplicates this one — and Failed(message = …) construction sites elsewhere keep compiling.
It is a view of the staged file, never an owner.pendingStaged stays the single handle reset() deletes through, so a carrying Failed that is dropped without being read cannot lose
anything the message-only Failed could not. That is the answer to "a Failed that carries a file
is a new invariant for reset()".
What happens to the file on each path out of Failed
Path
File
Retry succeeds
published, then deleted; pendingStaged cleared; state Saved
Retry fails again
kept, and the state carries it again — not a bare Failed
Retry finds it gone
nothing to publish or discard; plain Failed with STAGED_FILE_GONE_MESSAGE
Start over
discarded exactly once, through the publisher
Process death
the sweep, as before
Start over still deletes, and reset()'s KDoc now says why that is a decision rather than an
inheritance: deletion is acceptable only once the alternative has been offered, and the screen puts
"Try saving again" directly above it. Until that button existed, this delete was the only thing a
failed save could lead to.
One thing beyond the ticket
Both entry points derived the save dialog's MIME type with (state as? Converted)?.mimeType ?: settings.spec.mimeType, which answers null for a Failed — so a retry would have opened CreateDocument with the current pickers' type. Wrong for any job whose spec was edited since,
and for every reattached job, whose spec was never in those settings at all. Both now read state.pendingSave()?.mimeType, the same function save() picks its file with, so the two cannot
disagree. Extracting it is also what makes that derivation JVM-testable.
Tests
Both halves are JVM, through the ConverterScreenContent / JoinScreenContent seam from #77 —
no ViewModel in the screen tests, no WorkManager.
FailedSaveRetryTest (new, 12 cases, both ViewModels): a failed save carries the handle; a retry
publishes and leaves nothing staged; a retry that fails again still carries it; reset() from a
carrying Failed discards exactly once; a retry whose file has gone reuses STAGED_FILE_GONE_MESSAGE and carries nothing; a transcode failure carries nothing to save;
a retry offers the type the job chose rather than the one the pickers now show.
ConverterStateAffordancesTest / JoinStateAffordancesTest: a carrying Failed renders both
buttons and hands back the job's name; a plain Failed renders neither save affordance.
Mutations, run and confirmed red
Delete the "Try saving again" Button from ConverterScreen's Failed branch → ConverterStateAffordancesTest 2 failed: a failed save offers the file again as well as a
restart, tapping try saving again hands back the name the job chose.
save()'s onFailure emits ConversionState.Failed(e.message ?: "Could not save the file.") → FailedSaveRetryTest 5 failed, naming the lost handle: a failed save must leave the staged file
offerable, not just on disk. The same edit in JoinViewModel reddens the other 3.
Make the plain Failed carry a handle — the WorkInfo.State.FAILED arm of ConversionViewModel.observe passes a PendingSave built from the job's output data → a transcode failure carries nothing to save fails with expected null, but was: <PendingSave(staged=, suggestedName=, mimeType=)>.
Supplementary, for the screen half of (3), since a screen test constructs its own state and so
cannot see (3)'s edit: making ConverterScreen's Failed branch render the retry button
unconditionally — if (retry == null) → if (false), plus retry?.suggestedName.orEmpty(),
because the guard is what makes retry non-null — reddens a transcode failure offers no way to
save, and nothing else. Two edits rather than one, which is why it is reported as supplementary
and not as the mutation: (3) above is a single production line and stands on its own.
Gate
assembleDebug + testDebugUnitTest + compileDebugAndroidTestKotlin + ktlintCheck + detekt + lintDebug. detekt reports 0 findings; lint reports 0 errors, 0 warnings and the one pre-existing UsableSpace hint on OutputPublisher.kt:99 — pre-existing, on a line this diff does not touch.
Named exemptions
No new instrumented test. Nothing in this change depends on device behaviour — the publish
failure has to be injected on a device exactly as on the JVM, and the JVM source set already runs
the real ViewModel through a real WorkManager and the real composable. SafPickerRoundTripTest
still covers the real CreateDocument round trip, whose shape is unchanged.
The two destinationMime lines themselves live in the entry points, above the seam. What
they read, pendingSave(), is asserted directly.
That the destination received the bytes on a retry.RecordingPublisher.publish is a stub,
which is what makes a save fail deterministically. OutputPublisherPublishTest owns real writes.
onInputPicked from a carrying Failed. It overwrites the state without discarding — from Converted exactly as much — and neither branch renders a picker. Pre-existing, and not widened
here: the carried handle is a view of pendingStaged, never a second owner.
Closes #30.
`ConversionViewModel.save()`'s `onFailure` keeps the staged file deliberately — "a failed save may
mean the staged file is the only copy of an hour of transcoding" — 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 calls `discardStaged()` on precisely the file the comment above
it goes out of its way to keep. The intent was already in `main`; the UI did not honour it. The
only rescue was process death followed by reattach — unadvertised, and bounded by the <24h sweep.
`JoinViewModel` / `JoinScreen` had the same shape, and are fixed with it.
## The state shape
`Failed(message)` gains a nullable second field:
```kotlin
data class Failed(val message: String, val retry: PendingSave? = null) : ConversionState
```
`PendingSave(staged, suggestedName, mimeType)` is shared by both state machines and lives next to
`STAGED_FILE_GONE_MESSAGE` in `OutputPublisher.kt`, for the reason that constant is already there.
Nullable rather than a `SaveFailed` variant of its own: what the screen does with the *message* is
identical either way, so a second variant would make every exhaustive `when` grow an arm that
duplicates this one — and `Failed(message = …)` construction sites elsewhere keep compiling.
**It is a view of the staged file, never an owner.** `pendingStaged` stays the single handle
`reset()` deletes through, so a carrying `Failed` that is dropped without being read cannot lose
anything the message-only `Failed` could not. That is the answer to "a `Failed` that carries a file
is a new invariant for `reset()`".
## What happens to the file on each path out of `Failed`
| Path | File |
|---|---|
| Retry succeeds | published, then deleted; `pendingStaged` cleared; state `Saved` |
| Retry fails again | kept, and the state carries it again — not a bare `Failed` |
| Retry finds it gone | nothing to publish or discard; plain `Failed` with `STAGED_FILE_GONE_MESSAGE` |
| Start over | discarded exactly once, through the publisher |
| Process death | the sweep, as before |
**Start over still deletes**, and `reset()`'s KDoc now says why that is a decision rather than an
inheritance: deletion is acceptable only once the alternative has been offered, and the screen puts
"Try saving again" directly above it. Until that button existed, this delete was the only thing a
failed save could lead to.
## One thing beyond the ticket
Both entry points derived the save dialog's MIME type with `(state as? Converted)?.mimeType ?:
settings.spec.mimeType`, which answers null for a `Failed` — so a retry would have opened
`CreateDocument` with the *current pickers*' type. Wrong for any job whose spec was edited since,
and for every reattached job, whose spec was never in those settings at all. Both now read
`state.pendingSave()?.mimeType`, the same function `save()` picks its file with, so the two cannot
disagree. Extracting it is also what makes that derivation JVM-testable.
## Tests
Both halves are JVM, through the `ConverterScreenContent` / `JoinScreenContent` seam from #77 —
no ViewModel in the screen tests, no WorkManager.
- `FailedSaveRetryTest` (new, 12 cases, both ViewModels): a failed save carries the handle; a retry
publishes and leaves nothing staged; a retry that fails again still carries it; `reset()` from a
carrying `Failed` discards exactly once; a retry whose file has gone reuses
`STAGED_FILE_GONE_MESSAGE` and carries nothing; a **transcode failure carries nothing to save**;
a retry offers the type the job chose rather than the one the pickers now show.
- `ConverterStateAffordancesTest` / `JoinStateAffordancesTest`: a carrying `Failed` renders both
buttons and hands back the job's name; a plain `Failed` renders neither save affordance.
### Mutations, run and confirmed red
1. Delete the "Try saving again" `Button` from `ConverterScreen`'s `Failed` branch →
`ConverterStateAffordancesTest` 2 failed: *a failed save offers the file again as well as a
restart*, *tapping try saving again hands back the name the job chose*.
2. `save()`'s `onFailure` emits `ConversionState.Failed(e.message ?: "Could not save the file.")` →
`FailedSaveRetryTest` 5 failed, naming the lost handle: *a failed save must leave the staged file
offerable, not just on disk*. The same edit in `JoinViewModel` reddens the other 3.
3. Make the plain `Failed` carry a handle — the `WorkInfo.State.FAILED` arm of
`ConversionViewModel.observe` passes a `PendingSave` built from the job's output data →
*a transcode failure carries nothing to save* fails with `expected null, but was:
<PendingSave(staged=, suggestedName=, mimeType=)>`.
Supplementary, for the screen half of (3), since a screen test constructs its own state and so
cannot see (3)'s edit: making `ConverterScreen`'s `Failed` branch render the retry button
unconditionally — `if (retry == null)` → `if (false)`, plus `retry?.suggestedName.orEmpty()`,
because the guard is what makes `retry` non-null — reddens *a transcode failure offers no way to
save*, and nothing else. Two edits rather than one, which is why it is reported as supplementary
and not as the mutation: (3) above is a single production line and stands on its own.
### Gate
`assembleDebug` + `testDebugUnitTest` + `compileDebugAndroidTestKotlin` + `ktlintCheck` + `detekt` +
`lintDebug`. detekt reports 0 findings; lint reports 0 errors, 0 warnings and the one pre-existing
`UsableSpace` hint on `OutputPublisher.kt:99` — pre-existing, on a line this diff does not touch.
## Named exemptions
- **No new instrumented test.** Nothing in this change depends on device behaviour — the publish
failure has to be injected on a device exactly as on the JVM, and the JVM source set already runs
the real ViewModel through a real WorkManager and the real composable. `SafPickerRoundTripTest`
still covers the real `CreateDocument` round trip, whose shape is unchanged.
- **The two `destinationMime` lines themselves** live in the entry points, above the seam. What
they read, `pendingSave()`, is asserted directly.
- **That the destination received the bytes on a retry.** `RecordingPublisher.publish` is a stub,
which is what makes a save fail deterministically. `OutputPublisherPublishTest` owns real writes.
- **`onInputPicked` from a carrying `Failed`.** It overwrites the state without discarding — from
`Converted` exactly as much — and neither branch renders a picker. Pre-existing, and not widened
here: the carried handle is a view of `pendingStaged`, never a second owner.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
CI note, so the run's red conclusion is not misread.
All nine gating jobs are green. The one remaining failure is E2E API 37 Media3 hardware transcode (advisory), which is continue-on-error and red on every
PR by design — read the @FailsOnEmulatorApi37 marker, not the job name.
Two gating legs failed on the first attempt and passed on rerun, unchanged APK:
E2E API 35 — ReattachOnLaunchTest.doesNotOverwriteAPickTheUserHasAlreadyMade, the known
flake (#49). It asserts that reattachment does not take the screen from a user who has already
picked; this diff touches neither reattach() nor onInputPicked.
E2E API 37 — SafPickerRoundTripTest.pickingAFileThroughTheSystemPickerFillsInTheFileCard,
failing with no windows at all (UiAutomation.getWindows() is empty) — the framework going down
under the run, which is what that class's KDoc describes for the android-37.x images and what #108 tracks. That method is measured passing at API 37, so it is deliberately not marked; this
is the image, not the test. This diff does not touch ACTION_OPEN_DOCUMENT.
Both tests passed on API 33, 34 and 36 in the same first run, on the same APK, which is what says
the failures are environmental rather than the change.
CI note, so the run's red conclusion is not misread.
**All nine gating jobs are green.** The one remaining failure is
`E2E API 37 Media3 hardware transcode (advisory)`, which is `continue-on-error` and red on every
PR by design — read the `@FailsOnEmulatorApi37` marker, not the job name.
Two gating legs failed on the **first** attempt and passed on rerun, unchanged APK:
- **E2E API 35** — `ReattachOnLaunchTest.doesNotOverwriteAPickTheUserHasAlreadyMade`, the known
flake (#49). It asserts that reattachment does not take the screen from a user who has already
picked; this diff touches neither `reattach()` nor `onInputPicked`.
- **E2E API 37** — `SafPickerRoundTripTest.pickingAFileThroughTheSystemPickerFillsInTheFileCard`,
failing with `no windows at all (UiAutomation.getWindows() is empty)` — the framework going down
under the run, which is what that class's KDoc describes for the android-37.x images and what
#108 tracks. That method is measured **passing** at API 37, so it is deliberately not marked; this
is the image, not the test. This diff does not touch `ACTION_OPEN_DOCUMENT`.
Both tests passed on API 33, 34 and 36 in the same first run, on the same APK, which is what says
the failures are environmental rather than the change.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #30.
ConversionViewModel.save()'sonFailurekeeps the staged file deliberately — "a failed save maymean the staged file is the only copy of an hour of transcoding" — and then handed the screen a
Failedcarrying a message and nothing else. That branch rendered exactly one control: Startover, wired to
reset(), which callsdiscardStaged()on precisely the file the comment aboveit goes out of its way to keep. The intent was already in
main; the UI did not honour it. Theonly rescue was process death followed by reattach — unadvertised, and bounded by the <24h sweep.
JoinViewModel/JoinScreenhad the same shape, and are fixed with it.The state shape
Failed(message)gains a nullable second field:PendingSave(staged, suggestedName, mimeType)is shared by both state machines and lives next toSTAGED_FILE_GONE_MESSAGEinOutputPublisher.kt, for the reason that constant is already there.Nullable rather than a
SaveFailedvariant of its own: what the screen does with the message isidentical either way, so a second variant would make every exhaustive
whengrow an arm thatduplicates this one — and
Failed(message = …)construction sites elsewhere keep compiling.It is a view of the staged file, never an owner.
pendingStagedstays the single handlereset()deletes through, so a carryingFailedthat is dropped without being read cannot loseanything the message-only
Failedcould not. That is the answer to "aFailedthat carries a fileis a new invariant for
reset()".What happens to the file on each path out of
FailedpendingStagedcleared; stateSavedFailedFailedwithSTAGED_FILE_GONE_MESSAGEStart over still deletes, and
reset()'s KDoc now says why that is a decision rather than aninheritance: deletion is acceptable only once the alternative has been offered, and the screen puts
"Try saving again" directly above it. Until that button existed, this delete was the only thing a
failed save could lead to.
One thing beyond the ticket
Both entry points derived the save dialog's MIME type with
(state as? Converted)?.mimeType ?: settings.spec.mimeType, which answers null for aFailed— so a retry would have openedCreateDocumentwith the current pickers' type. Wrong for any job whose spec was edited since,and for every reattached job, whose spec was never in those settings at all. Both now read
state.pendingSave()?.mimeType, the same functionsave()picks its file with, so the two cannotdisagree. Extracting it is also what makes that derivation JVM-testable.
Tests
Both halves are JVM, through the
ConverterScreenContent/JoinScreenContentseam from #77 —no ViewModel in the screen tests, no WorkManager.
FailedSaveRetryTest(new, 12 cases, both ViewModels): a failed save carries the handle; a retrypublishes and leaves nothing staged; a retry that fails again still carries it;
reset()from acarrying
Faileddiscards exactly once; a retry whose file has gone reusesSTAGED_FILE_GONE_MESSAGEand carries nothing; a transcode failure carries nothing to save;a retry offers the type the job chose rather than the one the pickers now show.
ConverterStateAffordancesTest/JoinStateAffordancesTest: a carryingFailedrenders bothbuttons and hands back the job's name; a plain
Failedrenders neither save affordance.Mutations, run and confirmed red
ButtonfromConverterScreen'sFailedbranch →ConverterStateAffordancesTest2 failed: a failed save offers the file again as well as arestart, tapping try saving again hands back the name the job chose.
save()'sonFailureemitsConversionState.Failed(e.message ?: "Could not save the file.")→FailedSaveRetryTest5 failed, naming the lost handle: a failed save must leave the staged fileofferable, not just on disk. The same edit in
JoinViewModelreddens the other 3.Failedcarry a handle — theWorkInfo.State.FAILEDarm ofConversionViewModel.observepasses aPendingSavebuilt from the job's output data →a transcode failure carries nothing to save fails with
expected null, but was: <PendingSave(staged=, suggestedName=, mimeType=)>.Supplementary, for the screen half of (3), since a screen test constructs its own state and so
cannot see (3)'s edit: making
ConverterScreen'sFailedbranch render the retry buttonunconditionally —
if (retry == null)→if (false), plusretry?.suggestedName.orEmpty(),because the guard is what makes
retrynon-null — reddens a transcode failure offers no way tosave, and nothing else. Two edits rather than one, which is why it is reported as supplementary
and not as the mutation: (3) above is a single production line and stands on its own.
Gate
assembleDebug+testDebugUnitTest+compileDebugAndroidTestKotlin+ktlintCheck+detekt+lintDebug. detekt reports 0 findings; lint reports 0 errors, 0 warnings and the one pre-existingUsableSpacehint onOutputPublisher.kt:99— pre-existing, on a line this diff does not touch.Named exemptions
failure has to be injected on a device exactly as on the JVM, and the JVM source set already runs
the real ViewModel through a real WorkManager and the real composable.
SafPickerRoundTripTeststill covers the real
CreateDocumentround trip, whose shape is unchanged.destinationMimelines themselves live in the entry points, above the seam. Whatthey read,
pendingSave(), is asserted directly.RecordingPublisher.publishis a stub,which is what makes a save fail deterministically.
OutputPublisherPublishTestowns real writes.onInputPickedfrom a carryingFailed. It overwrites the state without discarding — fromConvertedexactly as much — and neither branch renders a picker. Pre-existing, and not widenedhere: the carried handle is a view of
pendingStaged, never a second owner.🤖 Generated with Claude Code
CI note, so the run's red conclusion is not misread.
All nine gating jobs are green. The one remaining failure is
E2E API 37 Media3 hardware transcode (advisory), which iscontinue-on-errorand red on everyPR by design — read the
@FailsOnEmulatorApi37marker, not the job name.Two gating legs failed on the first attempt and passed on rerun, unchanged APK:
ReattachOnLaunchTest.doesNotOverwriteAPickTheUserHasAlreadyMade, the knownflake (#49). It asserts that reattachment does not take the screen from a user who has already
picked; this diff touches neither
reattach()noronInputPicked.SafPickerRoundTripTest.pickingAFileThroughTheSystemPickerFillsInTheFileCard,failing with
no windows at all (UiAutomation.getWindows() is empty)— the framework going downunder the run, which is what that class's KDoc describes for the android-37.x images and what
#108 tracks. That method is measured passing at API 37, so it is deliberately not marked; this
is the image, not the test. This diff does not touch
ACTION_OPEN_DOCUMENT.Both tests passed on API 33, 34 and 36 in the same first run, on the same APK, which is what says
the failures are environmental rather than the change.