perf(android): prepare selected images off Main with bounded ownership

[sambitcreate]

Selected-image preparation previously performed metadata inspection and bitmap conversion on Main. Run bounded serial preparation off Main, sample large conversion decodes, preserve alpha/EXIF orientation and byte-identical valid passthrough, and release bitmap intermediates. Pending batches and uploads block Send; cancellation and client changes fence late work.

Validation: initial full JVM suite 251/251; lint and instrumentation compilation passed; three synthetic codec tests executed on the Android 16 emulator. Restoring Main dispatch makes the new preparation regression fail. Fresh-context GPT-6 Astra medium review found no actionable product issue and independently passed that regression.

Validation limitation: the final full JVM run was 250/251, with heldTurnReceiptPreservesOtherOwnerAndCannotCrossRemoval timing out during its initial load before Send/image work. The single permitted targeted retry passed 2/2. A test-only follow-up fixes a separately proven harness lifetime defect: cancelled ViewModel children now finish before Main is reset. A controlled held-finalizer regression fails without the join. Fresh-context review of the follow-up found no actionable issues and passed 5/5 focused checks. A further review follow-up protects the entire clear-and-join barrier with NonCancellable, including when the test caller is already cancelled. Its cancelled-caller held-finalizer regression fails without that protection; implementer and fresh reviewer each passed five focused checks. The previously recorded changed clean-source full suite still fails at initial loading (251/252), without the teardown exception; a later instrumented diagnostic passes 252/252 but does not establish the timeout cause or a clean-source full pass. No timeout increase, retry workaround, or claim of a final full-suite pass.

Before/after evidence and exact commands: docs/performance/android-image-preparation.md. No shared wire contract, transcript presentation or cache architecture change. No physical-device latency/energy measurement.

[pullfrog]

✅ No new issues found.

Reviewed changes I reviewed PR #284 at e3e2800, covering selected-image preparation, Send admission, cancellation/client fencing, bitmap ownership, and the changed JVM and instrumented tests.

• Off-Main preparation. URI metadata and reads, validation, image/text conversion, and Base64 preparation run on a shared serial IO worker; per-chat batches reserve Send before their first suspension.
• Image conversion. Large decodes are sampled before scaling, converted alpha and EXIF orientation are preserved, original validated PNG/JPEG payloads remain byte-identical, and owned bitmap intermediates are recycled.
• Cancellation and coverage. Queued batch ownership, Send gating, cancellation, and invalid-selection isolation have behavioral JVM coverage; instrumented tests exercise validation, alpha/sampling, passthrough, and converted JPEG orientation. The PR records the final full-suite timeout and its targeted rerun rather than claiming a final full-suite pass.

  ^{｜ View workflow run ｜ Using GPT Luna ｜ 𝕏}

[very-hermes-bot]

Hermes Review Bot

Confidence: 5

Engine: agy/gemini-3.8-flash-high
Review mode: full
Head: 5826ff70beb356e8c9afa432b1380577ea483ea2
Generated: 2026-09-29T00:27:28+00:00
Reviews: 1

Summary

Selected-image preparation and validation on Android are moved off the Main thread onto a bounded serial IO worker, resolving UI jank during media attachment. The pipeline introduces power-of-two decode sampling toward the largest 3072px candidate before exact scaling, applies native EXIF orientation while preserving alpha, and cleanly recycles intermediate bitmaps. AidenChatViewModel tracks active and queued preparation batches to synchronously reserve canSend before background dispatch, while client-identity checks and cooperative cancellation points prevent late uploads from leaking across session changes. Maintainers should double-check the test harness teardown in AidenChatTest.kt, where ViewModelStore.clearAndJoin() now shields child scope cancellation with NonCancellable to avoid premature main-dispatcher resets.

Confidence Score: 5/5

5/5: The complete diff was inspected across callers and callees, verifying coroutine dispatching, bitmap lifecycle/recycling, EXIF matrix transformations, cancellation rethrowing, and the test harness join barrier.

📁 Important Files Changed

• android/app/src/main/java/sbtbiswas/AidenOnTheGo/features/remote/AidenChatFeature.kt: Introduces onWorker with cross-chat serialization, adds inSampleSize power-of-two downsampling, applies EXIF orientation via Matrix, guarantees intermediate bitmap recycling in finally blocks, and threads cancellation checks throughout image decoding and compression.
• android/app/src/main/java/sbtbiswas/AidenOnTheGo/features/chat/AidenChatViewModel.kt: Implements prepareAndUpload() with a batch counter (preparingAttachmentBatches) and chat-level mutex to reserve canSend, moves upload-cache Base64 decoding off Main, and guards attachment mutation with active client checks and cancellation propagation.
• android/app/src/main/java/sbtbiswas/AidenOnTheGo/features/chat/AidenChatDetailScreen.kt: Launches preparation undispatched from the UI scope, delegates file reading and image transformation to prepareAndUpload(), and adds cooperative cancellation checking to readContentUriBounded().
• android/app/src/test/java/sbtbiswas/AidenOnTheGo/AidenChatTest.kt: Adds clearAndJoin() wrapped in NonCancellable to ensure cancelled ViewModel child scopes complete before Dispatchers.resetMain(), and adds regression tests verifying off-Main execution, Send reservation across multiple batches, and cancellation handling.
• android/app/src/androidTest/java/sbtbiswas/AidenOnTheGo/features/chat/AidenImageCarouselUiTest.kt: Adds on-device instrumented tests validating rejection of oversized dimensions, sample-down behavior with alpha preservation, and EXIF orientation preservation for JPEG conversions.
• docs/performance/android-image-preparation.md: Records before/after performance invariants, synthetic verification runs, emulator test results, and test lifetime investigation notes.

Findings

No findings.

Sequence Diagram

sequenceDiagram
    autonumber
    actor User
    participant Screen as AidenChatDetailScreen (Main)
    participant VM as AidenChatViewModel (Main)
    participant Worker as AidenAttachmentPreparation (IO)
    participant Client as AidenRemoteClient (IO)

    User->>Screen: Select image URIs
    Screen->>VM: prepareAndUpload(uris) [undispatched]
    Note over VM: Increment preparingAttachmentBatches<br/>(reserves canSend synchronously)
    VM->>Worker: onWorker { read & imageUpload }
    Note over Worker: Sample decode (inSampleSize)<br/>Scale & apply EXIF orientation<br/>Recycle intermediate Bitmaps
    Worker-->>VM: AidenAttachmentUpload.Image
    VM->>Client: uploadAttachment(chatId, upload)
    Client-->>VM: AidenAttachmentReference
    Note over VM: Append to pendingAttachments<br/>Save raw bytes to cache<br/>Decrement preparingAttachmentBatches
    VM-->>Screen: Re-evaluate canSend = true

Loading

[]

---

Last reviewed commit: 5826ff70beb3
Reviews (1) · Comment /hermes review to trigger a new review · /hermes review full for full re-review

[pullfrog]

Important

The follow-up cleanup barrier can still be bypassed when its caller is already cancelled, leaving ViewModel jobs alive as Main is reset.

Reviewed changes This incremental review covers the test-lifetime and diagnostic follow-ups added after the prior review, through e2d1a5d.

• Test teardown barrier. AidenChatTest now captures ViewModel scope jobs, clears the store and joins them before Main reset or fixture teardown; a held-finalizer regression verifies the wait, and affected cleanup sites also join the coordinator scope.
• Timeout evidence. The performance ledger and papercut tracker distinguish the unresolved clean-source startup timeouts from the passing instrumented diagnostic run without claiming a clean full-suite pass.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using GPT Luna ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes I reviewed the cleanup-barrier follow-up added since the prior Pullfrog review at e2d1a5d. The focused local Gradle invocation did not complete within this environment's 120-second command limit, so I could not independently rerun the documented 5/5 focused result.

• Protected teardown on cancellation. Wrapped job capture, store clearing, and joins in NonCancellable, preserving the dispatcher while preventing caller cancellation from bypassing the ViewModel cleanup barrier.
• Covered cancelled callers and updated evidence. Added a held-finalizer regression for already-cancelled cleanup and retained the distinction between the resolved teardown issue and the unexplained initial-load timeout.

  ^{｜ View workflow run ｜ Using GPT Luna ｜ 𝕏}