plan: refine (round 3)

Please throughoutly research the kotlin codebase for potential performance issues

plans/sh-testing-design.md

@@ -1,66 +1,64 @@
-# Product brief: Kotlin (Android/Desktop) performance audit and remediation
+# Performance hardening of the Kotlin (Android/desktop) client
 
 ## Goal
-Review the Compose Multiplatform Kotlin code under `apps/multiplatform` (shared `commonMain`, plus `androidMain`/`desktopMain`) and remove the concrete performance problems it contains. The two costs that matter to users are (1) UI jank — dropped frames while scrolling the chat list and an open chat, and (2) CPU/battery/memory spent per received or sent message. This brief records what the audit found and defines the target state; the plan lists the ordered fixes.
+Reduce jank, latency, and memory pressure in the SimpleX Chat multiplatform Kotlin client (`apps/multiplatform`, shared `commonMain` plus `androidMain`/`desktopMain`). The work is a targeted performance pass driven by a code audit of the message model, Compose UI, media pipeline, and the API/JSON receive/send path. Behaviour must be preserved exactly; only cost is removed.
 
-## Who is affected and when
-- Users in large groups or long chats (hundreds–thousands of loaded items), where per-message work is O(n) and repeats.
-- Users scrolling the chat list with many chats, or a chat with mixed content (text, images, video, voice, calls, group events).
-- Android users specifically for image previews (Android lacks caches that Desktop already has).
-- All users on every sent/received message (JSON encode/decode and terminal-log overhead).
+## Why now
+The audit found that the two most-exercised paths — receiving/updating a message in an open chat, and scrolling the message/chat lists — are super-linear in the number of loaded items, and that several per-frame costs (annotated-string building, base64 image decoding, date formatting) run uncached on the composition (main) thread. These compound in large groups and long chats, exactly where users notice slowness.
 
-## What the audit found (grounded in the code)
+## Scope — the concrete issues to address
 
-### A. Message-list state and merging — highest impact
-- `ChatModel.chatItems` and `ChatModel.chats` are `mutableStateOf(SnapshotStateList<…>)`; nearly every mutator (`addToChatItems`, `add/addAll/replaceAll/removeAt/removeAll`, `ChatModel.kt:500-506,3475-3540`) allocates a **new** `SnapshotStateList`, copies every element, and reassigns `.value`. Each message is O(n) copy; a burst of n messages is O(n²), and each reassignment invalidates every reader of the state, not just the changed row.
-- No id→index map for `chats` or `chatItems`; inserts/updates do linear `indexOfFirst`/`none` scans on every event (`ChatModel.kt:379,584,639,662`). Group members already have such a map (`groupMembersIndexes`), proving the pattern is available.
-- `MergedItems.create` (`ChatItemsMerger.kt:17-108`, driven by the `derivedStateOf` at `ChatView.kt:1795`) rebuilds the entire grouped/split model with three fresh allocations on **every** new message, reveal/collapse, or unread-count change. `itemSplits.contains(item.id)` (`ChatItemsMerger.kt:39`) is a linear `List` scan per item.
-- Pagination replaces the whole backing list on `Dispatchers.Main` (`ChatItemsLoader.kt:97,116,138,161`), forcing a full merge rebuild per "load more", and uses a `SnapshotStateList` as a scratch buffer (snapshot-record overhead for throwaway work, `ChatItemsLoader.kt:54`).
-- `upsertGroupMember` maps + element-wise `!=` compares + `replaceAll`s the whole item list on any member/connection-stat change, on Main (`ChatModel.kt:941-955`).
+### A. Message/chat state is O(N) per event and copies whole lists (model)
+`ChatModel.kt` stores `chats` and `chatItems` as flat `SnapshotStateList` with no id→index map (unlike `groupMembers`, which has `groupMembersIndexes`).
+- Every locate-by-id is a linear scan: `getChatIndex` (`:379`), and `indexOfFirst { it.id == cItem.id }` in `upsertChatItem` (`:639`), `updateChatItem` (`:662`), `addChatItem` dedup `none { it.id == … }` (`:584`). `Chat.id`/`ChatInfo.id` are computed string getters, so each comparison allocates.
+- Every single-element mutation rebuilds the entire list: `addToChatItems` (`:500-506`), `removeLastChatItems` (`:508`), `removeAllAndNotify` (`:3500`), `replaceAll` (`:3526`), and the `SnapshotStateList` `add`/`addAll`/`removeAt`/`removeAll` helpers (`:3475-3528`). A single incoming message into an N-item chat is O(N) copy + O(N) locate; a burst of M is ~O(N·M).
+- `upsertGroupMember` (`:941-955`) maps over the entire loaded message list per member update.
+- `addTerminalItem` (`:1228-1233`) does an O(n) `subList`+concat copy on every received message and command, even when no terminal view is open.
 
-### B. Compose recomposition in list rows
-- Neither LazyColumn sets `contentType`: chat list (`ChatListView.kt:1001`) and message list (`ChatView.kt:2381`). Keys are stable, but heterogeneous row layouts share one reuse pool, defeating slot reuse while scrolling.
-- Timestamp/date strings are recomputed per item per recomposition and each call allocates a `DateTimeFormatter` (`getTimestampText`/`getTimestampDateText`, `ChatModel.kt:3671-3711`; read via `get()` at `ChatModel.kt:3602,3113`; used at `CIMetaView.kt:125,192`, `ChatView.kt:3750-3752`, `ChatPreviewView.kt:412`). `getItemSeparation` formats dates ~4×/item.
-- `MarkdownText` rebuilds its `AnnotatedString` on every recomposition with no `remember` (`TextItemView.kt:200-351`); `reserveSpaceForMeta` concatenates with `+=` and re-reads prefs each recomposition (`CIMetaView.kt:130-195`).
-- `ChatItemView` receives ~35 freshly-allocated lambdas per pass (`ChatView.kt:1950`), so it is unstable and cannot be skipped.
-- Chat list filtering runs on every recomposition and copies the whole list first (`allChats.value.toList()` + `filteredChats`, `ChatListView.kt:943,1443-1471`); a per-item `derivedStateOf` is remembered on a new list instance so it is recreated every recomposition (`ChatListView.kt:1002-1004`); several `.filter{}` passes are unremembered (`ChatListView.kt:582-585,1186-1197`). `EventItemView` scans the full item list per event item (`ChatItemView.kt:605-641`).
+### B. Compose UI does uncached per-item work (rendering)
+- `MarkdownText` (`TextItemView.kt:124`) rebuilds the full `AnnotatedString` via `buildAnnotatedString` on every recomposition (`:200-211`, `:224-351`), for every visible message and every chat-list preview row. Largest per-item cost.
+- `MergedItems.create` (`ChatItemsMerger.kt:17`) is a full O(n) pass over all loaded items inside a `derivedStateOf` (`ChatView.kt:1795`); it re-runs on `revealedItems`/`unreadCount` changes (expand/collapse, mark-read while scrolling).
+- `filteredChats` (`ChatListView.kt:943`, `:1443`) filters the whole chat list and copies it (`allChats.value.toList()`) on every `ChatList` recomposition, including scroll-direction and keyboard-state changes, not only data/search changes. The per-row `nextChatSelected` `remember(chat.id, chats)` (`:1002`) is keyed on the new list instance, so it re-allocates a `derivedStateOf` per row each pass.
+- `getTimestampDateText` (`ChatModel.kt:3671`) builds `DateTimeFormatter` instances per call; called twice per item via `getItemSeparation` (`ChatView.kt:2416`, `:2420`).
+- The message `LazyColumn` (`ChatView.kt:2381`) supplies stable keys but no `contentType`, so heterogeneous rows (text/image/video/voice/event/banner) cannot reuse slots on scroll.
+- `ChatItemView` receives ~40 unstable inline lambdas (`ChatView.kt:1950`, defined `:531-798`), preventing recomposition skipping of visible items on any `ChatView`-level state change.
+- `EventItemView` (`ChatItemView.kt:640`) scans the whole item list per group-event item via `getConnectedMemberNames`.
 
-### C. Media/images (Android-specific gaps vs Desktop)
-- Android `base64ToBitmap` has no cache; Desktop has a 200-entry cache (`Images.android.kt:26-42` vs `Images.desktop.kt:24-31`) — Android re-decodes previews on every scroll-in.
-- Chat-list previews decode base64 on every recomposition with no `remember` on Android's uncached path (`ChatPreviewView.kt:320,357`).
-- Android `getLoadedImage` has no cache and re-reads (and re-decrypts) the file from disk on every scroll-back (`Utils.android.kt:172-194` vs Desktop's `loadedImageCache`).
-- Android decodes previews at full resolution — measures bounds then discards them instead of using `inSampleSize` (`Images.android.kt:32-37`); the correct downsampling helper already exists (`Utils.android.kt:197-210`).
-- First base64 decode of each image/video item runs synchronously on the composition thread (`CIImageView.kt:45`, `CIVideoView.kt:42`) although an off-thread `Base64AsyncImage` helper exists but is unused. `getMedia()` uses `runBlocking { getLoadedImage(...) }` (`ChatView.kt:3599`).
+### C. Media decoding is uncached and on the main thread on Android
+- Android `base64ToBitmap` (`Images.android.kt:26`) has no cache (desktop has `base64BitmapCache` LRU 200), decodes twice (bounds + full), and never applies `inSampleSize`, so previews decode at full resolution. It is called inside `remember(image)` on the composition thread in `CIImageView.kt:45`, `CIVideoView.kt:42`, and `ChatPreviewView.kt:320`.
+- `ProfileImage` default path (`ChatInfoImage.kt:111`, `async=false`, 52 call sites) calls `base64ToBitmap` in the composable body with no `remember`, decoding the avatar on every recomposition on the main thread.
+- Android `getLoadedImage` (`Utils.android.kt:172`) has no cache (desktop has `loadedImageCache`); the returned full-file `ByteArray` is then re-decoded by Coil at `Size.ORIGINAL` (`CIImageView.android.kt:30`) regardless of display size.
+- `decryptedUris` (`ChatModel.kt:4408`) is an unbounded map with no eviction.
 
-### D. JSON and threading
-- Every response and event is parsed twice: `APISerializer.deserialize` decodes to a `JsonElement` DOM then re-decodes that tree to `CR` (`SimpleXAPI.kt:6426-6441`); the fallback re-encodes the element to a string (a third pass).
-- The shared `json` has `prettyPrint = true` (`SimpleXAPI.kt:6355`) and is used to **encode outgoing commands** (`SimpleXAPI.kt:3998,4008,…`), so every sent message is pretty-printed before the core re-parses it; a `prettyPrint = false` `jsonShort` already exists but is not used there.
-- `terminalItems.value += item` copies the whole list on every message even when the terminal view is not open (`ChatModel.kt:1228-1232`, driven from `SimpleXAPI.kt:2792,842,854`).
-- The receive loop acquires a wake-lock and launches a new release coroutine per received message (`SimpleXAPI.kt:705-710`); `getUserChatData` runs the potentially large `updateChats` merge on `Dispatchers.Main` (`SimpleXAPI.kt:676-679`). Receive/decode itself is correctly on `Dispatchers.IO` (good).
+### D. API/JSON send/receive overhead
+- The shared `json` instance has `prettyPrint = true` (`SimpleXAPI.kt:6354`) and is used to encode outgoing commands, including the send-message hot path (`ApiSendMessages` `:3997`, reactions `:4017`, updates `:4008`). A non-pretty `jsonShort` (`:6370`) already exists but is unused here.
+- `APISerializer.deserialize` (`:6426`) parses every response twice: `decodeJsonElement()` to a DOM tree, then `decodeFromJsonElement` to the typed value — heaviest for the large payloads the code itself flags (`apiChats`, `apiChat`, `apiListMembers`).
+- The receive loop launches a throwaway 30s-delay coroutine per message to release a wake lock (`:705`).
 
-### E. Tooling gap
-- The Compose compiler plugin is applied but no stability configuration or metrics/reports output is enabled (`common/build.gradle.kts`), so recomposition regressions and unstable model classes are invisible.
-
-## User-visible behaviour after remediation
-- Smooth scrolling in the chat list and in open chats, including chats with mixed media and large groups; no perceptible per-frame stutter attributable to re-decoding images, reformatting timestamps, or rebuilding annotated text.
-- Lower CPU/battery use per received and sent message; incoming message bursts in large groups do not degrade quadratically.
-- Android image-preview scrolling matches Desktop responsiveness (cached decode, downsampled previews, no repeated disk reads).
-- Identical rendered output and behaviour — this is an internal optimization, not a feature or UX change.
+## User-visible behaviour after the change
+- Smoother scrolling in long chats, large groups, and the chat list, with fewer dropped frames when images/avatars are on screen.
+- Lower latency and less UI stutter when receiving bursts of messages, delivery receipts, edits, and reactions in an open chat.
+- Lower memory use and fewer full-resolution bitmap allocations on Android; parity with desktop's existing caches.
+- Faster send of messages and faster handling of large `apiChats`/member-list responses.
+- No change to displayed content, ordering, unread counts, merging/reveal behaviour, selection, or link/secret/mention handling.
 
 ## Success criteria
-- No behavioural or visual change: existing rendering, ordering, reveal/collapse, read-marking, pagination, and message send/receive all behave exactly as before; existing tests still pass.
-- Per-incoming-message work in an open chat is no longer O(n) in the number of loaded items for the common append case (id lookups O(1); merge updated incrementally or measurably cheaper).
-- Both message-list and chat-list LazyColumns declare `contentType`; scroll recomposition counts drop (verified with Compose compiler metrics / layout inspector).
-- Timestamp/date strings and annotated message text are computed once per item (cached/`remember`ed), not per recomposition.
-- On Android, base64 previews and loaded images are cached and downsampled; scrolling a chat back and forth does not re-decode or re-read from disk.
-- Outgoing command JSON is not pretty-printed; responses are parsed in a single pass; terminal-log growth does not copy the whole list per message when the terminal is closed.
-- Compose compiler metrics/report generation is available for regression tracking.
+- Receiving/updating a message in an open chat of N items is O(1)/O(log N) amortized for the locate and no longer copies the whole `chatItems` list per event; message-burst handling no longer scales ~O(N²).
+- Chat-list and chat-item lookups by id use an index/map rather than a linear scan with string-getter comparisons.
+- The per-item `AnnotatedString`, merged-items structure, date text, and Android base64/avatar decodes are computed off the main thread and/or cached so they do not re-run on unrelated recompositions.
+- Outgoing commands are serialized without pretty-print; response deserialization does not double-parse the payload for the common typed cases.
+- All existing unit tests pass, including `ChatItemsMergerTest`; behaviour (merging, unread bookkeeping in `ActiveChatState`, reveal/collapse, selection, scrolling-to-item, mark-read) is unchanged.
 
-## Edge cases and risks to preserve
-- `derivedStateOf` was previously removed from chat-list filtering due to an `IndexOutOfBoundsException` (comment at `ChatListView.kt:937-939`); any re-introduction of memoization must not reintroduce index desync between `chats` and per-item `index`.
-- Snapshot semantics: switching a `SnapshotStateList`-reassignment pattern to in-place mutation must keep Compose observing the right granularity — under-invalidation (stale UI) is as harmful as over-invalidation. Reveal/collapse, unread markers, splits, and "scroll to item" rely on current invalidation behaviour.
-- `contentType` values must be coarse enough to enable reuse yet not collapse incompatible layouts (e.g. banner vs message vs date separator).
-- Serialization changes must not alter the exact command strings/whitespace the core expects, nor drop `ignoreUnknownKeys`/coercion behaviour; the double-parse exists to inspect the response shape before decoding — the single-pass replacement must preserve error/unknown handling (`CR.Response`/`CR.Invalid`).
-- Image caches must be memory-bounded (LRU) and invalidated on the existing `clearImageCaches()` hook and on file change/deletion.
-- Off-loading merge/model work off `Dispatchers.Main` must preserve ordering guarantees relative to concurrent events.
-- Platform parity: Android and Desktop paths differ; fixes must not regress the platform that is already correct.
+## Non-goals
+- No feature or protocol changes; no change to the Haskell core or FFI signatures.
+- No visual redesign; no change to what is rendered.
+- The `messagesChannel` rendezvous/`trySend` drop behaviour (`SimpleXAPI.kt:524`, `:714`) is a correctness observation, not part of this performance pass, and is left as-is unless a fix is trivially safe.
+- iOS/Swift client is out of scope.
+
+## Key risks / edge cases
+- The list-copy pattern may be intentional to force `SnapshotStateList` identity change and trigger recomposition; any switch to in-place mutation must still notify Compose correctly and keep `mergedItems`/`derivedStateOf` observers firing.
+- An id→index map must be kept consistent across every add/remove/reorder/replace path (primary and secondary chat contexts, remote hosts) or lookups will desync.
+- Memoizing `AnnotatedString` must key on every input that affects output (text, formattedText, mentions, theme colours, live-typing/secret/highlight state) to avoid stale rendering.
+- Adding `contentType` must not change key stability or scroll-position restoration.
+- Bitmap caches on Android must be memory-bounded (LRU) and must not retain `Bitmap`s past the image's lifetime; avatar/preview caches keyed on the base64 string must handle profile-image updates.
+- Switching outgoing encode to `jsonShort` must preserve `encodeDefaults`/`explicitNulls` semantics so the core receives identical command JSON (only whitespace differs).