Closes#61. Child 5 of 8 decomposing #52 — the risky cut, taken with #57–#60 as the net.
What moved
ConverterScreen and JoinScreen each inlined their whole when (state) inside the public entry
point, so state could only ever arrive as viewModel.state. The when now lives in ConverterScreenContent(state, settings, validation, actions, modifier) and JoinScreenContent(state, actions, modifier), both internal @Composable. Each entry point keeps
its launchers, its collectAsStateWithLifecycle calls, and the destinationMime line the CreateDocument contract is remembered against — 50 lines on the converter side, 27 on the join
side, all of it launcher wiring that only #64 can reach.
The callbacks travel in internal data class ConverterActions (12) and internal data class JoinActions (5) rather than as loose parameters. detekt's LongParameterList sits at its default
threshold of six and config/detekt/detekt.yml relaxes LongMethod, CyclomaticComplexMethod and TooManyFunctions for @Composable but not that rule — AdvancedPicker already sits exactly on
it. Data classes are exempt, which is the whole reason for the shape. Join gets a holder too, at five
callbacks, so both screens present the same seam to #62 and #63.
Behaviour is unchanged, and here is why that is checkable rather than asserted
The body was cut and pasted, not retyped — an extraction script did the move, and every
substitution inside it was viewModel::x → actions.onX or a launcher call → an action call, with
the expected occurrence count asserted before each replacement. So the U+2026 in "Converting… ",
the U+2014 in "Done — " and "Paused. … —", and the U+00B7 in describe are the same bytes the
existing tests match on. is ConversionState.Idle -> Unit and is JoinState.Idle -> Unit in the
nested when — permanently unreachable and deliberately kept, per #52 — survive the move verbatim.
The one visible structural difference: onClick = { requestNotifications.launch(...) } became onClick = actions.onConvert, and the two onClick = { pickInput.launch(arrayOf("*/*")) } became onClick = actions.onPickInput. Same call, one indirection later.
The actions holder is not wrapped in remember. A fresh instance per recomposition means *ScreenContent never skips, which is what it did before too — the parent recomposes only when
state, settings, validation or a launcher registration changes, and the content recomposed with it.
Remembering it would have to key on the launchers, and a stale launcher captured in a holder is a
real bug where an extra recomposition is not.
The regression gate
All seven tests from #58/#59/#60 pass unedited.git status on this branch shows exactly two
modified files and two new ones; not one line of any existing test changed.
suite
tests
failures
ConverterLeafTagsTest
11
0
JoinLeafTagsTest
1
0
FileCardTest
13
0
ConverterFormattersTest
13
0
ConverterPickerSelectionTest
9
0
AdvancedPickerTest
9
0
AdvancedPanelSavedStateTest
2
0
TagTableUniquenessTest
1
0
That they could not have broken is structural, and worth stating because it is the claim this
ticket is riskiest about: every one of them composes a leaf directly, none references either entry
point, and the diff stops above FormatPicker in ConverterScreen.kt and above FileRow in JoinScreen.kt. TestTags.kt is untouched — no tag was added, moved or renamed, so TagTableUniquenessTest had nothing new to check.
This PR's own bite
ConverterScreenContentTest and JoinScreenContentTest, two tests each, one per direction of the
seam. Both states are unreachable through a real ViewModel: Converted and Joined exist only
after a worker run has already succeeded, carrying a staged file and the name the job chose.
Four mutations, each reverted after:
1. ConverterScreen.kt — delete .testTag(TestTags.SAVE_FILE) from the Converted branch's Save
button. Both converter tests go red:
ConverterScreenContentTest > a converted job renders the save button FAILED
java.lang.AssertionError: Failed: assertExists.
Reason: Expected exactly '1' node but could not find any node that satisfies: (TestTag = 'action.saveFile')
ConverterScreenContentTest > tapping save hands back the name the finished job chose FAILED
java.lang.AssertionError: Action performScrollTo() failed.
Reason: Expected exactly '1' node but could not find any node that satisfies: (TestTag = 'action.saveFile')
2. ConverterScreen.kt — onClick = { actions.onSave(s.suggestedName) } → onClick = {}. This
is the direction that did not exist before this PR: state goes in, an action comes back out.
ConverterScreenContentTest > tapping save hands back the name the finished job chose FAILED
java.lang.AssertionError: expected:<[holiday.mp4]> but was:<[]>
3. JoinScreen.kt — delete .testTag(TestTags.SAVE_FILE) from the Joined branch's Save button.
JoinScreenContentTest > a finished join renders the save button FAILED
java.lang.AssertionError: Failed: assertExists.
Reason: Expected exactly '1' node but could not find any node that satisfies: (TestTag = 'action.saveFile')
JoinScreenContentTest > tapping save hands back the name the finished join chose FAILED
java.lang.AssertionError: Action performScrollTo() failed.
Reason: Expected exactly '1' node but could not find any node that satisfies: (TestTag = 'action.saveFile')
JoinScreenContentTest > tapping save hands back the name the finished join chose FAILED
java.lang.AssertionError: expected:<[joined.mp4]> but was:<[]>
Asserting the name rather than "something was called" is deliberate. The suggested name comes from
the job — ConversionWorker.KEY_SUGGESTED_NAME — and is what the save dialog opens with, so a Save
button wired to the wrong branch's state would hand over the wrong one and a bare call-count check
would stay green.
Deliberately not covered
The state matrix. Which affordances each of the six ConversionStates and six JoinStates
renders is #62 and #63. This PR asserts only that the injection point exists and works in both
directions, so those two can be written without colliding with it.
The residual launcher wiring in both entry points — the OpenDocument / CreateDocument / RequestPermission registrations and the destinationMime fallback. Reachable only from #64.
No coverage number is quoted anywhere here, per #52's correction and CLAUDE.md.
Compose ConverterScreenContent / JoinScreenContent directly; createDrainedComposeRule(), not createComposeRule().
The actions holders are internal data classes in the screen files. Build one with {} for
everything you are not asserting on, exactly as these two tests do.
Converted/Joined render inside a verticalScroll column, so performScrollTo() before performClick(). Idle does not — it is the centred branch outside the scroll.
File("no-such-path") gives length() == 0L without throwing, so the size lines render as 0 B
and 0 MB and no TemporaryFolder is needed.
routeReason blank is what keeps the routing AssistChip out of the tree. That chip is #62's
case, and it carries no tag today.
#63 is the one that closes JoinScreenKt at 9.2%; the leaf and seam tests here do not.
Gate
./gradlew :app:assembleDebug :app:testDebugUnitTest :app:compileDebugAndroidTestKotlin :app:ktlintCheck :app:detekt :app:lintDebug --continue — BUILD SUCCESSFUL. detekt reports zero
findings; formatting is whatever ktlintFormat produced, including its collapse of JoinScreenContent's signature onto one line.
One more check, because the net cannot see it
The #58–#60 suites cover the leaves inside the branches, not the branches themselves, and the two
new tests here cover Converted and Joined. A tag accidentally dropped from Ready, Converting, Waiting, Saved or Failed during the move would therefore pass everything above, including TagTableUniquenessTest — which reads the table, not its usage. So the multiset of TestTags.*
references in each screen file was compared against origin/main:
Identical, both files — 23 references across 21 distinct symbols on the converter side, 9 across 8 on
the join side, same counts before and after. No branch lost or gained an affordance.
Beyond that, git diff matched the whole moved body as context rather than as changed lines: every
hunk outside the new header is 1–3 lines wide and contains only a named substitution. The block is
byte-identical apart from those.
Closes #61. Child 5 of 8 decomposing #52 — the risky cut, taken with #57–#60 as the net.
## What moved
`ConverterScreen` and `JoinScreen` each inlined their whole `when (state)` inside the public entry
point, so state could only ever arrive as `viewModel.state`. The `when` now lives in
`ConverterScreenContent(state, settings, validation, actions, modifier)` and
`JoinScreenContent(state, actions, modifier)`, both `internal @Composable`. Each entry point keeps
its launchers, its `collectAsStateWithLifecycle` calls, and the `destinationMime` line the
`CreateDocument` contract is remembered against — 50 lines on the converter side, 27 on the join
side, all of it launcher wiring that only #64 can reach.
The callbacks travel in `internal data class ConverterActions` (12) and `internal data class
JoinActions` (5) rather than as loose parameters. detekt's `LongParameterList` sits at its default
threshold of six and `config/detekt/detekt.yml` relaxes `LongMethod`, `CyclomaticComplexMethod` and
`TooManyFunctions` for `@Composable` but **not** that rule — `AdvancedPicker` already sits exactly on
it. Data classes are exempt, which is the whole reason for the shape. Join gets a holder too, at five
callbacks, so both screens present the same seam to #62 and #63.
## Behaviour is unchanged, and here is why that is checkable rather than asserted
The body was **cut and pasted, not retyped** — an extraction script did the move, and every
substitution inside it was `viewModel::x` → `actions.onX` or a launcher call → an action call, with
the expected occurrence count asserted before each replacement. So the U+2026 in `"Converting… "`,
the U+2014 in `"Done — "` and `"Paused. … —"`, and the U+00B7 in `describe` are the same bytes the
existing tests match on. `is ConversionState.Idle -> Unit` and `is JoinState.Idle -> Unit` in the
nested `when` — permanently unreachable and deliberately kept, per #52 — survive the move verbatim.
The one visible structural difference: `onClick = { requestNotifications.launch(...) }` became
`onClick = actions.onConvert`, and the two `onClick = { pickInput.launch(arrayOf("*/*")) }` became
`onClick = actions.onPickInput`. Same call, one indirection later.
The actions holder is **not** wrapped in `remember`. A fresh instance per recomposition means
`*ScreenContent` never skips, which is what it did before too — the parent recomposes only when
state, settings, validation or a launcher registration changes, and the content recomposed with it.
Remembering it would have to key on the launchers, and a stale launcher captured in a holder is a
real bug where an extra recomposition is not.
## The regression gate
**All seven tests from #58/#59/#60 pass unedited.** `git status` on this branch shows exactly two
modified files and two new ones; not one line of any existing test changed.
| suite | tests | failures |
|---|---|---|
| `ConverterLeafTagsTest` | 11 | 0 |
| `JoinLeafTagsTest` | 1 | 0 |
| `FileCardTest` | 13 | 0 |
| `ConverterFormattersTest` | 13 | 0 |
| `ConverterPickerSelectionTest` | 9 | 0 |
| `AdvancedPickerTest` | 9 | 0 |
| `AdvancedPanelSavedStateTest` | 2 | 0 |
| `TagTableUniquenessTest` | 1 | 0 |
That they *could not* have broken is structural, and worth stating because it is the claim this
ticket is riskiest about: every one of them composes a leaf directly, none references either entry
point, and the diff stops above `FormatPicker` in `ConverterScreen.kt` and above `FileRow` in
`JoinScreen.kt`. `TestTags.kt` is untouched — no tag was added, moved or renamed, so
`TagTableUniquenessTest` had nothing new to check.
## This PR's own bite
`ConverterScreenContentTest` and `JoinScreenContentTest`, two tests each, one per direction of the
seam. Both states are unreachable through a real ViewModel: `Converted` and `Joined` exist only
after a worker run has already succeeded, carrying a staged file and the name the job chose.
Four mutations, each reverted after:
**1. `ConverterScreen.kt` — delete `.testTag(TestTags.SAVE_FILE)` from the `Converted` branch's Save
button.** Both converter tests go red:
```
ConverterScreenContentTest > a converted job renders the save button FAILED
java.lang.AssertionError: Failed: assertExists.
Reason: Expected exactly '1' node but could not find any node that satisfies: (TestTag = 'action.saveFile')
ConverterScreenContentTest > tapping save hands back the name the finished job chose FAILED
java.lang.AssertionError: Action performScrollTo() failed.
Reason: Expected exactly '1' node but could not find any node that satisfies: (TestTag = 'action.saveFile')
```
**2. `ConverterScreen.kt` — `onClick = { actions.onSave(s.suggestedName) }` → `onClick = {}`.** This
is the direction that did not exist before this PR: state goes in, an action comes back out.
```
ConverterScreenContentTest > tapping save hands back the name the finished job chose FAILED
java.lang.AssertionError: expected:<[holiday.mp4]> but was:<[]>
```
**3. `JoinScreen.kt` — delete `.testTag(TestTags.SAVE_FILE)` from the `Joined` branch's Save button.**
```
JoinScreenContentTest > a finished join renders the save button FAILED
java.lang.AssertionError: Failed: assertExists.
Reason: Expected exactly '1' node but could not find any node that satisfies: (TestTag = 'action.saveFile')
JoinScreenContentTest > tapping save hands back the name the finished join chose FAILED
java.lang.AssertionError: Action performScrollTo() failed.
Reason: Expected exactly '1' node but could not find any node that satisfies: (TestTag = 'action.saveFile')
```
**4. `JoinScreen.kt` — `onClick = { actions.onSave(s.suggestedName) }` → `onClick = {}`.**
```
JoinScreenContentTest > tapping save hands back the name the finished join chose FAILED
java.lang.AssertionError: expected:<[joined.mp4]> but was:<[]>
```
Asserting the *name* rather than "something was called" is deliberate. The suggested name comes from
the job — `ConversionWorker.KEY_SUGGESTED_NAME` — and is what the save dialog opens with, so a Save
button wired to the wrong branch's state would hand over the wrong one and a bare call-count check
would stay green.
## Deliberately not covered
- **The state matrix.** Which affordances each of the six `ConversionState`s and six `JoinState`s
renders is #62 and #63. This PR asserts only that the injection point exists and works in both
directions, so those two can be written without colliding with it.
- **The residual launcher wiring** in both entry points — the `OpenDocument` / `CreateDocument` /
`RequestPermission` registrations and the `destinationMime` fallback. Reachable only from #64.
- No coverage number is quoted anywhere here, per #52's correction and `CLAUDE.md`.
## For #62 and #63
- Compose `ConverterScreenContent` / `JoinScreenContent` directly; `createDrainedComposeRule()`, not
`createComposeRule()`.
- The actions holders are `internal data class`es in the screen files. Build one with `{}` for
everything you are not asserting on, exactly as these two tests do.
- `Converted`/`Joined` render inside a `verticalScroll` column, so `performScrollTo()` before
`performClick()`. `Idle` does not — it is the centred branch outside the scroll.
- `File("no-such-path")` gives `length() == 0L` without throwing, so the size lines render as `0 B`
and `0 MB` and no `TemporaryFolder` is needed.
- `routeReason` blank is what keeps the routing `AssistChip` out of the tree. That chip is #62's
case, and it carries no tag today.
- `#63` is the one that closes `JoinScreenKt` at 9.2%; the leaf and seam tests here do not.
## Gate
`./gradlew :app:assembleDebug :app:testDebugUnitTest :app:compileDebugAndroidTestKotlin
:app:ktlintCheck :app:detekt :app:lintDebug --continue` — BUILD SUCCESSFUL. detekt reports zero
findings; formatting is whatever `ktlintFormat` produced, including its collapse of
`JoinScreenContent`'s signature onto one line.
## One more check, because the net cannot see it
The #58–#60 suites cover the leaves *inside* the branches, not the branches themselves, and the two
new tests here cover `Converted` and `Joined`. A tag accidentally dropped from `Ready`, `Converting`,
`Waiting`, `Saved` or `Failed` during the move would therefore pass everything above, including
`TagTableUniquenessTest` — which reads the table, not its usage. So the multiset of `TestTags.*`
references in each screen file was compared against `origin/main`:
```
git show origin/main:.../ConverterScreen.kt | grep -o 'TestTags\.[A-Za-z_.]*' | sort | uniq -c
grep -o 'TestTags\.[A-Za-z_.]*' .../ConverterScreen.kt | sort | uniq -c
```
Identical, both files — 23 references across 21 distinct symbols on the converter side, 9 across 8 on
the join side, same counts before and after. No branch lost or gained an affordance.
Beyond that, `git diff` matched the whole moved body as *context* rather than as changed lines: every
hunk outside the new header is 1–3 lines wide and contains only a named substitution. The block is
byte-identical apart from those.
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 #61. Child 5 of 8 decomposing #52 — the risky cut, taken with #57–#60 as the net.
What moved
ConverterScreenandJoinScreeneach inlined their wholewhen (state)inside the public entrypoint, so state could only ever arrive as
viewModel.state. Thewhennow lives inConverterScreenContent(state, settings, validation, actions, modifier)andJoinScreenContent(state, actions, modifier), bothinternal @Composable. Each entry point keepsits launchers, its
collectAsStateWithLifecyclecalls, and thedestinationMimeline theCreateDocumentcontract is remembered against — 50 lines on the converter side, 27 on the joinside, all of it launcher wiring that only #64 can reach.
The callbacks travel in
internal data class ConverterActions(12) andinternal data class JoinActions(5) rather than as loose parameters. detekt'sLongParameterListsits at its defaultthreshold of six and
config/detekt/detekt.ymlrelaxesLongMethod,CyclomaticComplexMethodandTooManyFunctionsfor@Composablebut not that rule —AdvancedPickeralready sits exactly onit. Data classes are exempt, which is the whole reason for the shape. Join gets a holder too, at five
callbacks, so both screens present the same seam to #62 and #63.
Behaviour is unchanged, and here is why that is checkable rather than asserted
The body was cut and pasted, not retyped — an extraction script did the move, and every
substitution inside it was
viewModel::x→actions.onXor a launcher call → an action call, withthe expected occurrence count asserted before each replacement. So the U+2026 in
"Converting… ",the U+2014 in
"Done — "and"Paused. … —", and the U+00B7 indescribeare the same bytes theexisting tests match on.
is ConversionState.Idle -> Unitandis JoinState.Idle -> Unitin thenested
when— permanently unreachable and deliberately kept, per #52 — survive the move verbatim.The one visible structural difference:
onClick = { requestNotifications.launch(...) }becameonClick = actions.onConvert, and the twoonClick = { pickInput.launch(arrayOf("*/*")) }becameonClick = actions.onPickInput. Same call, one indirection later.The actions holder is not wrapped in
remember. A fresh instance per recomposition means*ScreenContentnever skips, which is what it did before too — the parent recomposes only whenstate, settings, validation or a launcher registration changes, and the content recomposed with it.
Remembering it would have to key on the launchers, and a stale launcher captured in a holder is a
real bug where an extra recomposition is not.
The regression gate
All seven tests from #58/#59/#60 pass unedited.
git statuson this branch shows exactly twomodified files and two new ones; not one line of any existing test changed.
ConverterLeafTagsTestJoinLeafTagsTestFileCardTestConverterFormattersTestConverterPickerSelectionTestAdvancedPickerTestAdvancedPanelSavedStateTestTagTableUniquenessTestThat they could not have broken is structural, and worth stating because it is the claim this
ticket is riskiest about: every one of them composes a leaf directly, none references either entry
point, and the diff stops above
FormatPickerinConverterScreen.ktand aboveFileRowinJoinScreen.kt.TestTags.ktis untouched — no tag was added, moved or renamed, soTagTableUniquenessTesthad nothing new to check.This PR's own bite
ConverterScreenContentTestandJoinScreenContentTest, two tests each, one per direction of theseam. Both states are unreachable through a real ViewModel:
ConvertedandJoinedexist onlyafter a worker run has already succeeded, carrying a staged file and the name the job chose.
Four mutations, each reverted after:
1.
ConverterScreen.kt— delete.testTag(TestTags.SAVE_FILE)from theConvertedbranch's Savebutton. Both converter tests go red:
2.
ConverterScreen.kt—onClick = { actions.onSave(s.suggestedName) }→onClick = {}. Thisis the direction that did not exist before this PR: state goes in, an action comes back out.
3.
JoinScreen.kt— delete.testTag(TestTags.SAVE_FILE)from theJoinedbranch's Save button.4.
JoinScreen.kt—onClick = { actions.onSave(s.suggestedName) }→onClick = {}.Asserting the name rather than "something was called" is deliberate. The suggested name comes from
the job —
ConversionWorker.KEY_SUGGESTED_NAME— and is what the save dialog opens with, so a Savebutton wired to the wrong branch's state would hand over the wrong one and a bare call-count check
would stay green.
Deliberately not covered
ConversionStates and sixJoinStatesrenders is #62 and #63. This PR asserts only that the injection point exists and works in both
directions, so those two can be written without colliding with it.
OpenDocument/CreateDocument/RequestPermissionregistrations and thedestinationMimefallback. Reachable only from #64.CLAUDE.md.For #62 and #63
ConverterScreenContent/JoinScreenContentdirectly;createDrainedComposeRule(), notcreateComposeRule().internal data classes in the screen files. Build one with{}foreverything you are not asserting on, exactly as these two tests do.
Converted/Joinedrender inside averticalScrollcolumn, soperformScrollTo()beforeperformClick().Idledoes not — it is the centred branch outside the scroll.File("no-such-path")giveslength() == 0Lwithout throwing, so the size lines render as0 Band
0 MBand noTemporaryFolderis needed.routeReasonblank is what keeps the routingAssistChipout of the tree. That chip is #62'scase, and it carries no tag today.
#63is the one that closesJoinScreenKtat 9.2%; the leaf and seam tests here do not.Gate
./gradlew :app:assembleDebug :app:testDebugUnitTest :app:compileDebugAndroidTestKotlin :app:ktlintCheck :app:detekt :app:lintDebug --continue— BUILD SUCCESSFUL. detekt reports zerofindings; formatting is whatever
ktlintFormatproduced, including its collapse ofJoinScreenContent's signature onto one line.One more check, because the net cannot see it
The #58–#60 suites cover the leaves inside the branches, not the branches themselves, and the two
new tests here cover
ConvertedandJoined. A tag accidentally dropped fromReady,Converting,Waiting,SavedorFailedduring the move would therefore pass everything above, includingTagTableUniquenessTest— which reads the table, not its usage. So the multiset ofTestTags.*references in each screen file was compared against
origin/main:Identical, both files — 23 references across 21 distinct symbols on the converter side, 9 across 8 on
the join side, same counts before and after. No branch lost or gained an affordance.
Beyond that,
git diffmatched the whole moved body as context rather than as changed lines: everyhunk outside the new header is 1–3 lines wide and contains only a named substitution. The block is
byte-identical apart from those.