Synchronise SafPickerRoundTripTest on state, not on timing (#268)

Conversation 1 Commits 2 Files Changed 2

+161 -39

2 Commits

Author	SHA1	Message	Date
JMR-devandClaude Opus 5	b0b8b66d31	Name reattach's third outcome, which this KDoc denied existed The determinism argument said no coroutine had a `_state` write left in flight, because `reattach` "has either returned on its `_state.value !is Idle` guard or found nothing". There is a third outcome: `pruneWork()` is async, so `reattach` can find an unpruned job, pass that guard, and start an `observe()` that is a live coroutine with writes ahead of it. The conclusion survives, by a mechanism the paragraph did not mention. `reattach` reads `ownership.current` before its query and hands that token to `observe`, while `onInputPicked` calls `ownership.claim()` synchronously on the pick -- so once a detail row exists that observation is superseded and every emission returns at `stillHeldBy` before it writes. The claim is therefore "every write in flight is landed or superseded", not "no other coroutine started". This is the KDoc a future reader opens to learn why the test cannot flake, and CLAUDE.md records the same failure mode twice already -- E1/E3, and #226's KDoc that described a draft rather than the code. A correct test with an incomplete explanation is its own defect. Comment only; no test logic changed. Committed with --no-verify on the repo owner's explicit say-so: the gate's cache is keyed on the `app/src` tree hash, so a comment costs a full 33-36 sweep, and CI is already green on the parent commit. ktlintCheck, detekt and compileDebugAndroidTestKotlin were run by hand first and pass -- those are what a comment edit can actually break. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-07 16:19:24 -05:00
JMR-devandClaude Opus 5	9fd96d08fd	Synchronise SafPickerRoundTripTest on state, not on timing (#268 ) Its two picker tests failed on roughly half of gating runs, by two measured mechanisms. Both are removed here rather than re-tuned; the fix is in the test. A -- the Convert tap was lost in the post-probe relayout. `ConversionViewModel.onInputPicked` writes `_state` twice: name and size first, then the probe. The second write grows the file card and moves the Convert button. Compose computes the tap's coordinate from the semantics node and dispatches afterwards, so a relayout in that gap hit-tests a stationary coordinate against the new layout and the touch lands on whatever moved into the button's place -- silently. Measured as the gap between the pick's FFprobe closing and the tap: 319 ms and 421 ms passed; 46 ms, 98 ms and 124 ms did not. `convertToTheDefaultFormat` now waits for the `Container` detail row before tapping. That row is composed only under `input.probe != null`, so its presence means both of `onInputPicked`'s writes have landed and been laid out -- and nothing else in the ViewModel has a `_state` write in flight at that moment (`reattach` returned on its non-Idle guard, `observe` starts inside `convert()`). The card cannot change height again before the tap. That is a different claim from waiting longer. B -- the app was not the focused window when Compose was queried. One failure had a 416 ms gap, so it was not A: the tap landed, `GrantPermissionsActivity` started, back was pressed, and nothing was ever enqueued. A back press goes to whichever window holds input focus, while `Until.hasObject` answers about the accessibility tree -- which can carry the dialog's nodes first -- so a back that arrives one window early lands on `MainActivity` and finishes it. `POST_NOTIFICATIONS` is now held before the tap instead of the dialog being dismissed after it. `RequestPermission.getSynchronousResult` returns without starting anything when the permission is already granted, so there is no foreign window, no back press, and nothing the test injects can finish the Activity. `@Before` asserts the grant rather than assuming it. The class KDoc claimed granting "was tried first and did not take". Re-measured at API 34, six consecutive runs: zero `REQUEST_PERMISSIONS` starts, zero `GrantPermissionsActivity`, and exactly two `Scheduling work ID` lines per run -- one per converting test, so neither tap was lost. Also adds a fail-fast that says the Convert tap started nothing, instead of spending the 300 s conversion budget and then naming `action.saveFile`. It is a diagnostic, explicitly not the synchronisation. Not fixed in production. The double write is deliberate, documented progressive disclosure -- blocking the screen on an FFprobe process spawn reads as the app ignoring the tap -- and a layout fix (pinning the button, reserving the card's height) would make A less likely for one widget where waiting on the probe makes it impossible for every tap. The ticket's argument that each added `OutputFormat` widens A does not hold either: the format `FlowRow`'s height is fixed for a given entry list and does not change when the probe lands. What displaces Convert is the card growing, independent of chip count. Mutation, run not predicted: deleting `publish`'s `if (destinationWasEmpty) deletePartialOutput(...)` arm reddens `aFailedSaveDeletesTheDocumentItCouldNotWrite` with "publish did not delete the document it could not write", and reddens nothing else -- its sibling stays green, since the success path never enters that catch. Counts re-derived and unchanged: 72 androidTest tests, 7 markers, 65 gating, FAILS_ON_EMULATOR_API37_BASELINE = 7. Both tests keep @FailsOnEmulatorApi37. `NotificationCancelActionTest`'s KDoc said the suite grants no runtime permissions; that is no longer true and it now says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-07 15:44:20 -05:00

Author

SHA1

Message

Date

JMR-devandClaude Opus 5

b0b8b66d31

Name reattach's third outcome, which this KDoc denied existed

The determinism argument said no coroutine had a `_state` write left in
flight, because `reattach` "has either returned on its `_state.value !is Idle`
guard or found nothing". There is a third outcome: `pruneWork()` is async, so
`reattach` can find an unpruned job, pass that guard, and start an `observe()`
that is a live coroutine with writes ahead of it.

The conclusion survives, by a mechanism the paragraph did not mention.
`reattach` reads `ownership.current` before its query and hands that token to
`observe`, while `onInputPicked` calls `ownership.claim()` synchronously on the
pick -- so once a detail row exists that observation is superseded and every
emission returns at `stillHeldBy` before it writes. The claim is therefore
"every write in flight is landed or superseded", not "no other coroutine
started".

This is the KDoc a future reader opens to learn why the test cannot flake, and
CLAUDE.md records the same failure mode twice already -- E1/E3, and #226's KDoc
that described a draft rather than the code. A correct test with an incomplete
explanation is its own defect.

Comment only; no test logic changed. Committed with --no-verify on the repo
owner's explicit say-so: the gate's cache is keyed on the `app/src` tree hash,
so a comment costs a full 33-36 sweep, and CI is already green on the parent
commit. ktlintCheck, detekt and compileDebugAndroidTestKotlin were run by hand
first and pass -- those are what a comment edit can actually break.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

2026-09-07 16:19:24 -05:00

JMR-devandClaude Opus 5

9fd96d08fd

Synchronise SafPickerRoundTripTest on state, not on timing (#268 )

Its two picker tests failed on roughly half of gating runs, by two measured
mechanisms. Both are removed here rather than re-tuned; the fix is in the test.

**A -- the Convert tap was lost in the post-probe relayout.**
`ConversionViewModel.onInputPicked` writes `_state` twice: name and size first,
then the probe. The second write grows the file card and moves the Convert
button. Compose computes the tap's coordinate from the semantics node and
dispatches afterwards, so a relayout in that gap hit-tests a stationary
coordinate against the new layout and the touch lands on whatever moved into
the button's place -- silently. Measured as the gap between the pick's FFprobe
closing and the tap: 319 ms and 421 ms passed; 46 ms, 98 ms and 124 ms did not.

`convertToTheDefaultFormat` now waits for the `Container` detail row before
tapping. That row is composed only under `input.probe != null`, so its presence
means both of `onInputPicked`'s writes have landed and been laid out -- and
nothing else in the ViewModel has a `_state` write in flight at that moment
(`reattach` returned on its non-Idle guard, `observe` starts inside `convert()`).
The card cannot change height again before the tap. That is a different claim
from waiting longer.

**B -- the app was not the focused window when Compose was queried.**
One failure had a 416 ms gap, so it was not A: the tap landed,
`GrantPermissionsActivity` started, back was pressed, and nothing was ever
enqueued. A back press goes to whichever window holds *input* focus, while
`Until.hasObject` answers about the accessibility tree -- which can carry the
dialog's nodes first -- so a back that arrives one window early lands on
`MainActivity` and finishes it.

`POST_NOTIFICATIONS` is now held before the tap instead of the dialog being
dismissed after it. `RequestPermission.getSynchronousResult` returns without
starting anything when the permission is already granted, so there is no
foreign window, no back press, and nothing the test injects can finish the
Activity. `@Before` asserts the grant rather than assuming it.

The class KDoc claimed granting "was tried first and did not take". Re-measured
at API 34, six consecutive runs: zero `REQUEST_PERMISSIONS` starts, zero
`GrantPermissionsActivity`, and exactly two `Scheduling work ID` lines per run
-- one per converting test, so neither tap was lost.

Also adds a fail-fast that says the Convert tap started nothing, instead of
spending the 300 s conversion budget and then naming `action.saveFile`. It is a
diagnostic, explicitly not the synchronisation.

Not fixed in production. The double write is deliberate, documented progressive
disclosure -- blocking the screen on an FFprobe process spawn reads as the app
ignoring the tap -- and a layout fix (pinning the button, reserving the card's
height) would make A less likely for one widget where waiting on the probe makes
it impossible for every tap. The ticket's argument that each added `OutputFormat`
widens A does not hold either: the format `FlowRow`'s height is fixed for a given
entry list and does not change when the probe lands. What displaces Convert is
the card growing, independent of chip count.

Mutation, run not predicted: deleting `publish`'s `if (destinationWasEmpty)
deletePartialOutput(...)` arm reddens `aFailedSaveDeletesTheDocumentItCouldNotWrite`
with "publish did not delete the document it could not write", and reddens
nothing else -- its sibling stays green, since the success path never enters
that catch.

Counts re-derived and unchanged: 72 androidTest tests, 7 markers, 65 gating,
FAILS_ON_EMULATOR_API37_BASELINE = 7. Both tests keep @FailsOnEmulatorApi37.

`NotificationCancelActionTest`'s KDoc said the suite grants no runtime
permissions; that is no longer true and it now says so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

2026-09-07 15:44:20 -05:00

Synchronise SafPickerRoundTripTest on state, not on timing (#268) #269