Quote the rate the ticket settled on, and point the save gap at its ticket

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>
2026-08-25 22:38:49 -05:00
parent 3599307040
commit c4bb7d4d2d
5 changed files with 17 additions and 16 deletions
@@ -207,8 +207,8 @@ class ConversionViewModel @JvmOverloads constructor(
     * land on top of a [reset] taken while its copy was in flight, putting `Saved` on a screen the
     * user has just cleared. Guarding it would drop that write instead, reporting nothing for a
     * file that may genuinely have reached the user's destination. Which of those two is right is
-     * a question about what the screen should offer during a save, not about this race, and it is
-     * left open rather than answered in passing.
+     * a question about what the screen should offer during a save, not about this race, so it is
+     * filed as issue #123 rather than decided here in passing.
     */
    private val ownership = ScreenOwnership()

@@ -109,8 +109,8 @@ class JoinViewModel @JvmOverloads constructor(
     * lands after a suspension point is guarded: the one in [onInputsPicked] and the one in
     * [observe]. [save] is the one left out, deliberately and with the same limit its counterpart
     * in `ConversionViewModel` spells out: nothing can overwrite what it writes, but it can still
-     * land on top of a [reset] taken while its copy was in flight, and which way that should go is
-     * a question about the save screen rather than about this race.
+     * land on top of a [reset] taken while its copy was in flight. Which way that should go is a
+     * question about the save screen rather than about this race -- issue #123.
     */
    private val ownership = ScreenOwnership()

@@ -27,10 +27,10 @@ import java.io.File
 * second, which is the same defect the ticket reported against reattachment with a different
 * coroutine on the losing side.
 *
- * Both cases below existed before the fix and neither was reported, because a pick that loses to
- * another pick still shows *a* file the user chose. That is what made this worth closing in the
- * same change: one rule covering every deferred write is checkable, where "the observer checks and
- * the pick does not" is a rule nobody can hold in their head.
+ * Both cases below were measured rather than assumed: deleting either guard turns the matching
+ * test red, and deleting the probe one turns nine other tests red with it. Neither was ever
+ * reported, because a pick that loses to another pick still shows *a* file the user chose -- which
+ * is what made it worth closing alongside #49 rather than leaving as a second thing to find.
 */
@UnstableApi
@RunWith(RobolectricTestRunner::class)
@@ -21,10 +21,11 @@ import java.io.File
 * Issue #49, on the JVM and without the race.
 *
 * `ReattachOnLaunchTest.doesNotOverwriteAPickTheUserHasAlreadyMade` has been catching this on
- * devices since 2026-08-24 — four times in 400 gating leg-attempts, on API 33, 35 and 36 — and
- * every occurrence passed on re-run, which is why it was read as flaky infrastructure for two
- * days. It is not. The assertion it fails on is `expected null, but was:<Converted>`: a finished
- * job from an earlier session taking a screen the user had already picked a file on.
+ * devices since 2026-08-24 — four occurrences, spread across API 33, 35 and 36, which is what
+ * ruled out an emulator-image quirk. Every one was on attempt 1 and every one passed on re-run,
+ * which is why it was read as flaky infrastructure for two days. It is not. The assertion it fails
+ * on is `expected null, but was:<Converted>`: a finished job from an earlier session taking a
+ * screen the user had already picked a file on.
 *
 * The defect is a check-then-act whose act is deferred into another coroutine. `reattach()` reads
 * `_state.value` and then calls `observe()`, which *launches* a collector that has to suspend on
@@ -92,7 +93,7 @@ class ReattachmentOwnershipTest {
     * as a message posted to the main looper — and that looper is paused, so it cannot run until
     * something pumps it. `onInputPicked` is an ordinary synchronous call from this thread. The
     * pick is therefore *always* issued before the guard runs; none of it is left to timing, which
-     * is the whole point of writing this here rather than relying on the 1-in-130 device sighting.
+     * is the whole point of writing it here rather than relying on the rare device sighting.
     */
    @Test
    fun `a conversion found while the user was picking never reaches the screen`() {
@@ -41,9 +41,9 @@ import java.io.File
 * assignment below, so nothing can interleave". There is no assignment below, and the two lines
 * are in different coroutines.
 *
- * The convert side has been failing this on CI for two days at roughly 1-in-130 and was read as
- * flaky infrastructure. `JoinViewModel` has the identical shape and no test at all, which is why
- * this one was written before the fix rather than after it.
+ * The convert side has been failing this on CI for two days — four occurrences across three API
+ * levels, each read as flaky infrastructure. `JoinViewModel` has the identical shape and no test
+ * at all, which is why this one was written before the fix rather than after it.
 */
@UnstableApi
@RunWith(RobolectricTestRunner::class)