amadan
ios: fix intermittent chat unresponsiveness — nav dedup + drop dead per-frame frame tracking (#580) (#599)
* ios: make chat-list navigation push idempotent (#580) A member reported that repeatedly tapping a chat row while the UI was stalled opened the chat, and hitting back then surfaced a phantom second copy of it. The taps had all registered — each ran the list's unguarded `path.append(.thread(id))` — but a stalled main thread meant nothing committed visually until later, and a NavigationStack path is an array that holds duplicates, so the stack ended up with several identical `.thread(id)` entries. Guard the push: `ChatNavGuard.shouldPush` skips it when that chat is already on top, collapsing queued taps to a single push (the same idiom routeNotificationTap already uses). This makes navigation robust *to* a main-thread stall; it does not remove the stall (layout-loop / gesture fixes still to come). The guard is a pure helper so it's deterministically unit-testable without the SwiftUI/UIKit runtime the stall lives in — ChatNavGuardTests documents the bug (queued taps duplicate) and pins the fix. Also adds docs/threading-580-repro.md: a signal-based reproduction recipe for the wider #580 symptoms. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ios+mac: stop the onChange(of: CGRect) layout churn (#580) The image / video / YouTube cards tracked their `.global` frame with a GeometryReader + onChange. That continuously observes a value that changes on EVERY layout pass, and a chat open runs several layout passes per displayed frame (scroll-settle pin, avatar spring, composer inset, keyboard) — so the frame changes several times per frame and SwiftUI flags "onChange(of: CGRect) action tried to update multiple times per frame": real main-thread layout churn, a #580 stall suspect. This is NOT a self-invalidation loop — an earlier attempt to move the write off @State did not silence it (confirmed by breakpoint on the image bubble's onChange). The trigger is observing a high-frequency value at all. And that value is read ONLY at tap time and ONLY on macOS (the lightbox zooms out from it; iOS ignores it — every onImageTap consumer takes `_`). So compile the tracker out on iOS and keep it on macOS, via a shared `trackingFrame(into:)` modifier (LiveFrame holder stays a plain reference so the macOS write never invalidates the view). iOS loses nothing and sheds the churn; macOS behaviour (lightbox zoom origin) is unchanged. Runtime behaviour the test suite can't cover — validate via the console signal (docs/threading-580-repro.md): the warning should no longer appear on iOS while scrolling / animating the keyboard in a media-heavy chat. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> ---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
8 files changed,
+231
−22
.gitignore+2 −0docs/threading-580-repro.md+72 −0ios/LChat/Views/ThreadListView.swift+16 −1ios/LChatCore/Sources/LChatCore/ChatNavGuard.swift+25 −0ios/LChatCore/Sources/LChatCore/LiveFrame.swift+42 −0ios/LChatCore/Sources/LChatCore/MessageBubble.swift+10 −14ios/LChatCore/Sources/LChatCore/YouTubeCard.swift+5 −7ios/LChatCore/Tests/LChatCoreTests/ChatNavGuardTests.swift+59 −0
diff --git a/.gitignore b/.gitignore| index 5c585ea..c664576 100644 |
| --- a/.gitignore |
| +++ b/.gitignore |
| @@ -29,3 +29,5 @@ CLAUDE.local.md |
| # SwiftPM build dir (swift test in ios/LChatCore) |
| ios/LChatCore/.build/ |
| +# Per-user Xcode state for the SwiftPM package (scheme mgmt, breakpoints, etc.) |
| +ios/LChatCore/.swiftpm/ |
diff --git a/docs/threading-580-repro.md b/docs/threading-580-repro.md| new file mode 100644 |
| index 0000000..1660937 |
| --- /dev/null |
| +++ b/docs/threading-580-repro.md |
| @@ -0,0 +1,72 @@ |
| +# 🤖 Reproducing the iOS threading issues (#580) |
| + |
| +Working notes for validating the fixes on this branch. The user-visible hang is |
| +intermittent because it needs a main-thread stall to coincide with a tap — so |
| +instead of chasing the rare hang, reproduce the **mechanism signals** (console |
| +warnings), which fire far more reliably, and treat "signal gone" as the pass |
| +condition. |
| + |
| +## The signals to watch (device console — Xcode debug console or Console.app) |
| + |
| +| Signal | What it means | |
| +|---|---| |
| +| `onChange(of: CGRect) action tried to update multiple times per frame` | The `globalFrame` layout feedback loop in image/link/YouTube bubbles (`MessageBubble.swift`, `YouTubeCard.swift`, `ChatView.swift`). Main-thread layout churn. | |
| +| `System gesture gate timed out` | The system back/edge gesture waited on the feed's gesture stack (per-row swipe-to-reply + long-press + iOS-26 content-pop) and gave up. | |
| +| `Got a keyboard will change frame / will hide notification, but keyboard was not even present` | Spurious keyboard notifications — tied to the slow-to-focus symptom. | |
| + |
| +Filter Console.app on the app process with text `multiple times per frame` or |
| +`gesture gate`. |
| + |
| +## Setup that maximizes the odds |
| + |
| +- A space with **many chats** (warming starts 5 live feed-model sockets). |
| +- Open a chat that has **images and/or a link-preview or YouTube card** on |
| + screen (those bubbles carry the `globalFrame` tracker that fuels the loop). |
| +- A **second device / the web client** generating churn into the space: flap |
| + presence (foreground/background), type, send messages. |
| +- Use the **slowest device** available — main-thread stalls are amplified there. |
| + |
| +## Per-symptom recipe |
| + |
| +1. **Layout loop** — open the media chat; raise/dismiss the keyboard and scroll, |
| + repeatedly. Expect the `onChange(of: CGRect)` warning to spam (near |
| + deterministic). |
| +2. **Back delay** — with churn active, open the media chat and immediately tap |
| + back; repeat ~20×. Watch for the visible delay + `System gesture gate`. |
| +3. **Phantom second copy** (the clearest one) — tap a chat row; if nothing |
| + commits, tap it again a few times; when it finally opens, hit back. On the |
| + buggy build "back" lands on a duplicate copy of the same chat. |
| +4. **Blank feed** — from the list, tap the media chat → immediately back → |
| + immediately tap it again, ~10× fast. Feed opens empty; a tiny scroll redraws. |
| +5. **Slow focus** — open the media chat, immediately tap the composer. Delay |
| + before the caret/keyboard; correlate with the CGRect warning. |
| + |
| +## Fixes on this branch |
| + |
| +- **Phantom second copy (#580, symptom 6):** navigation push is now idempotent — |
| + `ChatNavGuard.shouldPush` skips a push when that chat is already on top, so |
| + queued taps can't stack duplicate `.thread(id)` entries |
| + (`ThreadListView.onSelect`). Deterministic regression test: |
| + `ChatNavGuardTests`. This makes navigation robust *to* a stall; it does not |
| + remove the stall. |
| + |
| +- **`onChange(of: CGRect)` layout churn (likely a root-cause contributor):** the |
| + image / video / YouTube cards tracked their `.global` frame with a |
| + `GeometryReader` + `onChange`. That's a *continuous observation of a value |
| + that changes on every layout pass* — and a chat open runs several layout |
| + passes per displayed frame (scroll-settle pin, avatar spring, composer inset, |
| + keyboard), so the frame changes several times per frame and SwiftUI flags |
| + `onChange(of: CGRect) action tried to update multiple times per frame`. |
| + (Not a self-invalidation loop — moving the write off `@State` did NOT silence |
| + it, confirmed by breakpoint at `MessageBubble` image bubble: the trigger is |
| + observing a high-frequency value at all.) The frame is read **only at tap |
| + time** and **only on macOS** (the lightbox zooms out from it; iOS ignores it — |
| + every `onImageTap` consumer takes `_`), so the tracker is compiled out on iOS |
| + via `trackingFrame(into:)` and kept on macOS. **Validate via the console |
| + signal** — the warning should no longer appear while scrolling / animating the |
| + keyboard in a media chat on iOS (runtime behaviour the test suite can't cover). |
| + |
| +_Still to land (tracked, not yet on this branch): apply `EdgeOnlyBackGesture` |
| +to `NativeChatScreen` on iOS 26; coalesce presence frames / skip no-op row |
| +reconfigures; make the feed reveal robust to a missed opening pin (blank |
| +feed)._ |
diff --git a/ios/LChat/Views/ThreadListView.swift b/ios/LChat/Views/ThreadListView.swift| index d1d4ab0..cb6bd51 100644 |
| --- a/ios/LChat/Views/ThreadListView.swift |
| +++ b/ios/LChat/Views/ThreadListView.swift |
| @@ -51,6 +51,13 @@ struct ThreadListView: View { |
| store.instanceName.isEmpty ? store.instance.displayName : store.instanceName |
| } |
| + // The chat currently on top of the stack (nil = this list is on top) — |
| + // feeds the idempotent-push guard so queued taps can't stack duplicates. |
| + private var topThreadID: String? { |
| + if case let .thread(id)? = path.last { return id } |
| + return nil |
| + } |
| + |
| var body: some View { |
| // A UIKit UICollectionView list, not SwiftUI's List — see ThreadListCollection |
| // for why (stock UITableView row highlight that SwiftUI's List can't match). |
| @@ -58,7 +65,15 @@ struct ThreadListView: View { |
| if search.trimmingCharacters(in: .whitespaces).isEmpty { |
| ThreadListCollection( |
| threads: visibleThreads, |
| - onSelect: { path.append(.thread($0)) }, |
| + // Idempotent push (issue #580): while the main thread is |
| + // stalled a member's taps queue, and each ran append — so |
| + // "back" surfaced a phantom second copy of the chat. Skip |
| + // the push when that chat is already on top. |
| + onSelect: { id in |
| + if ChatNavGuard.shouldPush(threadID: id, topThreadID: topThreadID) { |
| + path.append(.thread(id)) |
| + } |
| + }, |
| onClose: { store.setHidden($0, true) }, |
| onPin: { store.setPinned($0, !$0.pinned) }, |
| onToggleUnread: { $0.unread ? store.markRead($0) : store.markUnread($0) } |
diff --git a/ios/LChatCore/Sources/LChatCore/ChatNavGuard.swift b/ios/LChatCore/Sources/LChatCore/ChatNavGuard.swift| new file mode 100644 |
| index 0000000..2a9bb15 |
| --- /dev/null |
| +++ b/ios/LChatCore/Sources/LChatCore/ChatNavGuard.swift |
| @@ -0,0 +1,25 @@ |
| +import Foundation |
| + |
| +// 🤖 Navigation push guard (issue #580, iOS threading). |
| +// |
| +// The chat list pushes a chat with `path.append(.thread(id))`. That append is |
| +// unguarded, and a `NavigationStack` path is an ARRAY — it holds duplicates |
| +// happily. When the main thread stalls (the layout-loop / socket-churn hangs |
| +// we're chasing), a member's taps on a row don't visually commit, so they tap |
| +// again… and every tap already ran `append`. When the stack finally commits it |
| +// holds two (or more) identical `.thread(id)` entries, and "back" surfaces a |
| +// phantom second copy of the same chat. |
| +// |
| +// The fix is idempotence: only push when the target chat isn't already on top |
| +// of the stack, collapsing queued taps to a single push. This mirrors the guard |
| +// the notification-tap path already uses (`path.last != .thread(id)` in |
| +// RootView.routeNotificationTap). Kept as a pure helper so it's unit-testable |
| +// without the SwiftUI/UIKit runtime the bug actually lives in. |
| +public enum ChatNavGuard { |
| + /// Whether tapping into `threadID` should push a new chat screen, given the |
| + /// thread currently on top of the navigation stack (`nil` = the top isn't a |
| + /// chat, e.g. the chat list itself). |
| + public static func shouldPush(threadID: String, topThreadID: String?) -> Bool { |
| + topThreadID != threadID |
| + } |
| +} |
diff --git a/ios/LChatCore/Sources/LChatCore/LiveFrame.swift b/ios/LChatCore/Sources/LChatCore/LiveFrame.swift| new file mode 100644 |
| index 0000000..924b9f2 |
| --- /dev/null |
| +++ b/ios/LChatCore/Sources/LChatCore/LiveFrame.swift |
| @@ -0,0 +1,42 @@ |
| +import SwiftUI |
| + |
| +// 🤖 A card's live `.global` frame, for the Mac lightbox zoom-out (issue #580). |
| +// |
| +// The image / video / YouTube cards need the thumbnail's window frame ONLY at |
| +// tap time, and ONLY on the Mac (the lightbox zooms out from it — iOS ignores |
| +// the value entirely: every `onImageTap` consumer takes `_`). |
| +// |
| +// The frame was tracked with a GeometryReader + `onChange(of:)`. That's a |
| +// continuous observation of a value that changes on EVERY layout pass — and a |
| +// chat open runs several layout passes per displayed frame (scroll-settle pin, |
| +// avatar spring, composer inset, keyboard), so the frame changes several times |
| +// per frame and SwiftUI flags "onChange(of: CGRect) action tried to update |
| +// multiple times per frame": real main-thread layout churn (a #580 stall |
| +// suspect). Moving the write off @State was NOT enough — the loop wasn't |
| +// self-invalidation, it's that we were observing a high-frequency value at all. |
| +// |
| +// So the tracker is compiled out on iOS (the value is dead there) via |
| +// `trackingFrame(into:)`, and kept on macOS where the lightbox needs it. The |
| +// holder stays a plain reference so the write never invalidates the view. |
| +// Only ever touched on the main actor (SwiftUI view code), hence @MainActor. |
| +@MainActor |
| +final class LiveFrame { |
| + var rect: CGRect = .zero |
| +} |
| + |
| +extension View { |
| + /// Keep `holder.rect` current with this view's `.global` frame — macOS only |
| + /// (the only platform that reads it; see LiveFrame). A no-op on iOS, which |
| + /// is what removes the layout-churn `onChange(of: CGRect)` there. |
| + func trackingFrame(into holder: LiveFrame) -> some View { |
| + #if os(macOS) |
| + background(GeometryReader { g in |
| + Color.clear |
| + .onAppear { holder.rect = g.frame(in: .global) } |
| + .onChange(of: g.frame(in: .global)) { _, f in holder.rect = f } |
| + }) |
| + #else |
| + self |
| + #endif |
| + } |
| +} |
diff --git a/ios/LChatCore/Sources/LChatCore/MessageBubble.swift b/ios/LChatCore/Sources/LChatCore/MessageBubble.swift| index 0088f04..c20d6b8 100644 |
| --- a/ios/LChatCore/Sources/LChatCore/MessageBubble.swift |
| +++ b/ios/LChatCore/Sources/LChatCore/MessageBubble.swift |
| @@ -728,7 +728,9 @@ public struct MessageBubble: View { |
| @Environment(\.colorScheme) private var scheme |
| @State private var poster: PlatformImage? |
| // The card's live window frame — the video lightbox zooms out from here. |
| - @State private var globalFrame = CGRect.zero |
| + // A non-observed holder, not @State: reactively storing it looped layout |
| + // (see LiveFrame / #580). Read at tap time only. |
| + @State private var liveFrame = LiveFrame() |
| var body: some View { |
| let box = displaySize() |
| @@ -778,13 +780,9 @@ public struct MessageBubble: View { |
| } |
| } |
| .frame(width: box.width, height: box.height) |
| - .background(GeometryReader { g in |
| - Color.clear |
| - .onAppear { globalFrame = g.frame(in: .global) } |
| - .onChange(of: g.frame(in: .global)) { _, f in globalFrame = f } |
| - }) |
| + .trackingFrame(into: liveFrame) |
| .contentShape(RoundedRectangle(cornerRadius: 18, style: .continuous)) |
| - .onTapGesture { onTap?(attachment, globalFrame) } |
| + .onTapGesture { onTap?(attachment, liveFrame.rect) } |
| .task(id: attachment.id) { |
| guard poster == nil, let posterSource else { return } |
| poster = await BlobImageLoader.shared.image(posterSource, |
| @@ -816,7 +814,9 @@ public struct MessageBubble: View { |
| @State private var image: PlatformImage? |
| @State private var blur: PlatformImage? |
| // The thumbnail's live window frame — the lightbox zooms out from here. |
| - @State private var globalFrame = CGRect.zero |
| + // A non-observed holder, not @State: reactively storing it looped layout |
| + // (see LiveFrame / #580). Read at tap time only. |
| + @State private var liveFrame = LiveFrame() |
| // Animated GIFs only: reduced-motion users get them paused at first. |
| @State private var playing = !gifsStartPaused() |
| @@ -848,18 +848,14 @@ public struct MessageBubble: View { |
| } |
| } |
| .frame(width: box.width, height: box.height) |
| - .background(GeometryReader { g in |
| - Color.clear |
| - .onAppear { globalFrame = g.frame(in: .global) } |
| - .onChange(of: g.frame(in: .global)) { _, f in globalFrame = f } |
| - }) |
| + .trackingFrame(into: liveFrame) |
| .contentShape(RoundedRectangle(cornerRadius: 18, style: .continuous)) |
| .onTapGesture { |
| guard let image else { return } |
| // An animated GIF claims the click for play/pause (as on the |
| // web); everything else keeps the zoom-in lightbox click. |
| if isAnimatedGIF(image) { playing.toggle() } |
| - else { onTap?(attachment, image, globalFrame) } |
| + else { onTap?(attachment, image, liveFrame.rect) } |
| } |
| .task(id: attachment.id) { |
| guard image == nil, let source else { return } |
diff --git a/ios/LChatCore/Sources/LChatCore/YouTubeCard.swift b/ios/LChatCore/Sources/LChatCore/YouTubeCard.swift| index 09eebdc..12cfca7 100644 |
| --- a/ios/LChatCore/Sources/LChatCore/YouTubeCard.swift |
| +++ b/ios/LChatCore/Sources/LChatCore/YouTubeCard.swift |
| @@ -15,7 +15,9 @@ public struct YouTubeCard: View { |
| public let onPlay: ((String, Int, CGRect) -> Void)? |
| @State private var playing = false |
| - @State private var globalFrame = CGRect.zero |
| + // Non-observed holder, not @State: reactively storing the frame looped |
| + // layout (see LiveFrame / #580). Read at tap time only. |
| + @State private var liveFrame = LiveFrame() |
| public init(id: String, start: Int, onPlay: ((String, Int, CGRect) -> Void)? = nil) { |
| self.id = id |
| @@ -35,7 +37,7 @@ public struct YouTubeCard: View { |
| } else { |
| Button { |
| if let onPlay { |
| - onPlay(id, start, globalFrame) |
| + onPlay(id, start, liveFrame.rect) |
| } else { |
| playing = true |
| } |
| @@ -60,11 +62,7 @@ public struct YouTubeCard: View { |
| } |
| } |
| .frame(width: width, height: height) |
| - .background(GeometryReader { g in |
| - Color.clear |
| - .onAppear { globalFrame = g.frame(in: .global) } |
| - .onChange(of: g.frame(in: .global)) { _, f in globalFrame = f } |
| - }) |
| + .trackingFrame(into: liveFrame) |
| .clipShape(RoundedRectangle(cornerRadius: 18, style: .continuous)) |
| .task(id: id) { |
| if thumb == nil { thumb = await YouTubeThumbCache.shared.image(for: id) } |
diff --git a/ios/LChatCore/Tests/LChatCoreTests/ChatNavGuardTests.swift b/ios/LChatCore/Tests/LChatCoreTests/ChatNavGuardTests.swift| new file mode 100644 |
| index 0000000..e087b02 |
| --- /dev/null |
| +++ b/ios/LChatCore/Tests/LChatCoreTests/ChatNavGuardTests.swift |
| @@ -0,0 +1,59 @@ |
| +import XCTest |
| +@testable import LChatCore |
| + |
| +// 🤖 Regression guard for issue #580 (iOS threading). A stalled main thread let |
| +// a member's repeated taps on a chat row queue; each ran the list's |
| +// `path.append(.thread(id))`, so the NavigationStack ended up holding duplicate |
| +// copies of one chat and "back" surfaced a phantom second copy. The push must |
| +// be idempotent while that chat is already on top. |
| +// |
| +// This can't reproduce the underlying STALL (that lives in the SwiftUI layout |
| +// engine / gesture system), but it pins the state-machine invariant the stall |
| +// exposed: N taps on the same chat → exactly one push. |
| +final class ChatNavGuardTests: XCTestCase { |
| + |
| + // Model the list's onSelect reducer over a stack of pushed thread ids |
| + // (empty = the chat list on top). Mirrors ThreadListView's push. |
| + private func applyTaps(_ taps: [String], guarded: Bool) -> [String] { |
| + var stack: [String] = [] |
| + for id in taps { |
| + if !guarded || ChatNavGuard.shouldPush(threadID: id, topThreadID: stack.last) { |
| + stack.append(id) |
| + } |
| + } |
| + return stack |
| + } |
| + |
| + // The bug, reproduced: four queued taps on ONE chat push four copies when |
| + // unguarded — exactly the "back went to a second copy" report. |
| + func testQueuedTapsDuplicateWithoutGuard() { |
| + XCTAssertEqual(applyTaps(["T1", "T1", "T1", "T1"], guarded: false), |
| + ["T1", "T1", "T1", "T1"]) |
| + } |
| + |
| + // The fix: the same queued taps collapse to a single push. |
| + func testQueuedTapsCollapseWithGuard() { |
| + XCTAssertEqual(applyTaps(["T1", "T1", "T1", "T1"], guarded: true), ["T1"]) |
| + } |
| + |
| + // Guarding must not block legitimate navigation between DIFFERENT chats. |
| + func testDistinctChatsStillPush() { |
| + XCTAssertEqual(applyTaps(["T1", "T2", "T3"], guarded: true), ["T1", "T2", "T3"]) |
| + } |
| + |
| + // Re-opening a chat you just left is legitimate — it isn't on top anymore. |
| + func testReopenAfterLeavingPushes() { |
| + var stack = ["T1"] |
| + stack.removeLast() // back to the list |
| + if ChatNavGuard.shouldPush(threadID: "T1", topThreadID: stack.last) { |
| + stack.append("T1") |
| + } |
| + XCTAssertEqual(stack, ["T1"]) |
| + } |
| + |
| + func testShouldPushPredicate() { |
| + XCTAssertTrue(ChatNavGuard.shouldPush(threadID: "T1", topThreadID: nil)) // from the list |
| + XCTAssertTrue(ChatNavGuard.shouldPush(threadID: "T2", topThreadID: "T1")) // switching chats |
| + XCTAssertFalse(ChatNavGuard.shouldPush(threadID: "T1", topThreadID: "T1")) // already on top |
| + } |
| +} |