Closes#158. First of wave 2 (#153), and first deliberately — #155 wants to pin the join-side arity guard, and doing that before this would freeze the duplication in place.
The ticket named three; there are four."Joining failed." is the exact join-side twin of "Conversion failed." and was missed because the scan that produced the original list used {15,70} and the string is fifteen characters. Re-scanning at {8,90} found it.
Why these four and not every repeated string
Each is a case where one layer's message is another layer's fallback. The worker writes GENERIC_FAILURE_MESSAGE into KEY_ERROR; the ViewModel says the same sentence when KEY_ERROR never arrived at all, because the worker was killed first. The user cannot tell those two apart and should not have to — so they are one sentence, by construction rather than by coincidence.
Placement follows the convention already in the codebase, which OutputPublisher.kt states outright: "Kept next to STAGED_FILE_GONE_MESSAGE for the same reason it is: both ViewModels need it and staging is what it is about." The arity rule is the worker's — request(...) takes a List<Uri> and checks nothing about its length — so the constant lives there and the ViewModel reads it.
The two Log.e literals stay. A log has a different audience and carries the exception with it; coupling it to the user-facing wording would mean rewording the screen to change a log. That is in the KDoc so the next scan doesn't read it as a miss.
The test is cross-layer, deliberately
#158's done-when is explicit that "a test asserting the constant equals its own value is worth nothing." Sharing a constant makes two sites agree by construction; what it cannot show is that both layers still reach it.
So SharedFailureMessagesTest drives each layer for real — the ViewModel through onInputsPicked, the worker through doWork — and asserts the two answers are the same string, taken from two running layers rather than one declaration.
mutation
result
ViewModel keeps its own drifted literal ("at least 2 files")
red
ViewModel's arity guard removed entirely
red
That this was worth doing shows in what was pinned before: RefusedJobTest (#139) pinned the worker's copy of the arity message and nothing pinned the ViewModel's — so the screen's wording could drift and no test would say a word.
Not done here
"Saved ${s.displayName}." appears in both screens and is left alone. The distinction is real rather than an oversight: the four above are cross-layer fallbacks, where drift means the user sees different words for one condition. Two screens each wording their own success text is ordinary UI, and drift there is cosmetic.
Closes #158. First of wave 2 (#153), and first deliberately — #155 wants to pin the join-side arity guard, and doing that before this would freeze the duplication in place.
## What was duplicated
| message | sites | now |
|---|---|---|
| `"Pick at least two files to join."` | `ConcatWorker:42`, `JoinViewModel:197` | `ConcatWorker.TOO_FEW_INPUTS_MESSAGE` |
| `"Joining failed."` | `ConcatWorker:110`, `JoinViewModel:298` | `ConcatWorker.GENERIC_FAILURE_MESSAGE` |
| `"Conversion failed."` | `ConversionWorker:316`, `ConversionViewModel:508` | `ConversionWorker.GENERIC_FAILURE_MESSAGE` |
| `"Could not save the file."` | `ConversionViewModel:568`, `JoinViewModel:349` | `SAVE_FAILED_MESSAGE`, beside `STAGED_FILE_GONE_MESSAGE` |
**The ticket named three; there are four.** `"Joining failed."` is the exact join-side twin of `"Conversion failed."` and was missed because the scan that produced the original list used `{15,70}` and the string is fifteen characters. Re-scanning at `{8,90}` found it.
## Why these four and not every repeated string
Each is a case where **one layer's message is another layer's fallback**. The worker writes `GENERIC_FAILURE_MESSAGE` into `KEY_ERROR`; the ViewModel says the same sentence when `KEY_ERROR` never arrived at all, because the worker was killed first. The user cannot tell those two apart and should not have to — so they are one sentence, by construction rather than by coincidence.
Placement follows the convention already in the codebase, which `OutputPublisher.kt` states outright: *"Kept next to `STAGED_FILE_GONE_MESSAGE` for the same reason it is: both ViewModels need it and staging is what it is about."* The arity rule is the worker's — `request(...)` takes a `List<Uri>` and checks nothing about its length — so the constant lives there and the ViewModel reads it.
**The two `Log.e` literals stay.** A log has a different audience and carries the exception with it; coupling it to the user-facing wording would mean rewording the screen to change a log. That is in the KDoc so the next scan doesn't read it as a miss.
## The test is cross-layer, deliberately
#158's done-when is explicit that *"a test asserting the constant equals its own value is worth nothing."* Sharing a constant makes two sites agree by construction; what it cannot show is that both layers still **reach** it.
So `SharedFailureMessagesTest` drives each layer for real — the ViewModel through `onInputsPicked`, the worker through `doWork` — and asserts the two answers are the same string, taken from two running layers rather than one declaration.
| mutation | result |
|---|---|
| ViewModel keeps its own drifted literal (`"at least 2 files"`) | **red** |
| ViewModel's arity guard removed entirely | **red** |
That this was worth doing shows in what was pinned before: `RefusedJobTest` (#139) pinned the *worker's* copy of the arity message and nothing pinned the ViewModel's — so the screen's wording could drift and no test would say a word.
## Not done here
`"Saved ${s.displayName}."` appears in both screens and is left alone. The distinction is real rather than an oversight: the four above are cross-layer fallbacks, where drift means the user sees different words for one condition. Two screens each wording their own success text is ordinary UI, and drift there is cosmetic.
Gate green: `assembleDebug`, `testDebugUnitTest`, `compileDebugAndroidTestKotlin`, `ktlintCheck`, `detekt`, `lintDebug`.
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 #158. First of wave 2 (#153), and first deliberately — #155 wants to pin the join-side arity guard, and doing that before this would freeze the duplication in place.
What was duplicated
"Pick at least two files to join."ConcatWorker:42,JoinViewModel:197ConcatWorker.TOO_FEW_INPUTS_MESSAGE"Joining failed."ConcatWorker:110,JoinViewModel:298ConcatWorker.GENERIC_FAILURE_MESSAGE"Conversion failed."ConversionWorker:316,ConversionViewModel:508ConversionWorker.GENERIC_FAILURE_MESSAGE"Could not save the file."ConversionViewModel:568,JoinViewModel:349SAVE_FAILED_MESSAGE, besideSTAGED_FILE_GONE_MESSAGEThe ticket named three; there are four.
"Joining failed."is the exact join-side twin of"Conversion failed."and was missed because the scan that produced the original list used{15,70}and the string is fifteen characters. Re-scanning at{8,90}found it.Why these four and not every repeated string
Each is a case where one layer's message is another layer's fallback. The worker writes
GENERIC_FAILURE_MESSAGEintoKEY_ERROR; the ViewModel says the same sentence whenKEY_ERRORnever arrived at all, because the worker was killed first. The user cannot tell those two apart and should not have to — so they are one sentence, by construction rather than by coincidence.Placement follows the convention already in the codebase, which
OutputPublisher.ktstates outright: "Kept next toSTAGED_FILE_GONE_MESSAGEfor the same reason it is: both ViewModels need it and staging is what it is about." The arity rule is the worker's —request(...)takes aList<Uri>and checks nothing about its length — so the constant lives there and the ViewModel reads it.The two
Log.eliterals stay. A log has a different audience and carries the exception with it; coupling it to the user-facing wording would mean rewording the screen to change a log. That is in the KDoc so the next scan doesn't read it as a miss.The test is cross-layer, deliberately
#158's done-when is explicit that "a test asserting the constant equals its own value is worth nothing." Sharing a constant makes two sites agree by construction; what it cannot show is that both layers still reach it.
So
SharedFailureMessagesTestdrives each layer for real — the ViewModel throughonInputsPicked, the worker throughdoWork— and asserts the two answers are the same string, taken from two running layers rather than one declaration."at least 2 files")That this was worth doing shows in what was pinned before:
RefusedJobTest(#139) pinned the worker's copy of the arity message and nothing pinned the ViewModel's — so the screen's wording could drift and no test would say a word.Not done here
"Saved ${s.displayName}."appears in both screens and is left alone. The distinction is real rather than an oversight: the four above are cross-layer fallbacks, where drift means the user sees different words for one condition. Two screens each wording their own success text is ordinary UI, and drift there is cosmetic.Gate green:
assembleDebug,testDebugUnitTest,compileDebugAndroidTestKotlin,ktlintCheck,detekt,lintDebug.