Revert Sentry fix PRs #6201, #6205, #6207 — merged without approval

[beastoin]

Summary

• Reverts PR Fix WebSocket transcription disconnects — 64K Sentry events (#6193) #6201 (WebSocket transcription disconnects fix)
• Reverts PR Fix dock tile hang — 11.4K Sentry events (#6194) #6205 (Dock tile hang fix)
• Reverts PR Fix batch transcription 413 on long speech chunks (#6195) #6207 (Batch transcription 413 fix)

These 3 PRs were merged without manager approval. Reverting so the fixes can be reworked through the proper omi-pr-workflow with manager sign-off before merge.

Revert commits

1. df5982d9e — Revert Fix batch transcription 413 on long speech chunks (#6195) #6207
2. 8248f4dc4 — Revert Fix dock tile hang — 11.4K Sentry events (#6194) #6205
3. a67cb2f43 — Revert Fix WebSocket transcription disconnects — 64K Sentry events (#6193) #6201

🤖 Generated with Claude Code

by AI for @beastoin

[beastoin]

lgtm

[greptile-apps]

Greptile Summary

This PR is a clean, intentional revert of three PRs (#6201, #6205, #6207) that were merged without manager approval. The revert is structurally sound, but it re-introduces several known P1-level regressions across the desktop client that the original PRs were fixing.

Key regressions re-introduced by this revert:

• WebSocket transcription disconnects (proxy.rs, TranscriptionService.swift): Error variants are silently swallowed in the bidirectional proxy (while let Some(Ok(…))), so a transport error on one side causes the other to hang indefinitely. On the Swift side, connection "success" is inferred via a 0.5s timer rather than the URLSessionWebSocketDelegate.didOpenWithProtocol callback, making connection detection unreliable.
• Thread-safety / race condition (TranscriptionService.swift): handleDisconnection() is no longer idempotent — concurrent failure callbacks can race past the guard isConnected else { return } check and schedule duplicate reconnect tasks. The reconnect Task also captures self strongly, risking a retain cycle.
• HTTP 413 on long recordings (VADGateService.swift, AppState.swift): The batch audio buffer is again unbounded and the call site reverts to batchTranscribeFull with no splitting/retry, so any recording exceeding ~23s of stereo 16kHz audio will produce a 413 response and silently lose the transcription.
• Dock tile hang (CrispManager.swift, SidebarView.swift): unreadCount is incremented once per message inside a loop (multiple @Published writes per poll cycle) while SidebarView observes CrispManager via @ObservedObject, causing N SwiftUI re-renders per poll.
• Test coverage removed: The 467-line TranscriptionServiceTests.swift (ring buffer + state machine tests) is deleted with the revert.

All of the above are expected outcomes of the revert and are acknowledged in the PR description. The rework PRs should restore these fixes through the proper approval workflow.

Confidence Score: 4/5

Safe to merge as a process-compliance revert, but re-introduces P1 regressions in core transcription and UI stability that must be addressed in follow-up PRs.

The revert itself is clean and complete. Score is 4 rather than 5 because multiple P1 issues are intentionally re-introduced: WebSocket instability (proxy error swallowing + unreliable connection detection), HTTP 413 data loss on long recordings, a thread-safety race in TranscriptionService, and the dock tile hang. These affect the primary transcription user path and will require prompt follow-up.

desktop/Desktop/Sources/TranscriptionService.swift (thread-safety + connection detection), desktop/Backend-Rust/src/routes/proxy.rs (error swallowing + close frame forwarding), desktop/Desktop/Sources/VADGateService.swift (unbounded batch buffer).

Important Files Changed

Filename	Overview
desktop/Backend-Rust/src/routes/proxy.rs	Reverts WebSocket close-frame forwarding and error propagation; errors are silently swallowed by while let Some(Ok(…)), causing the surviving side to hang when one side disconnects.
desktop/Desktop/Sources/TranscriptionService.swift	Reverts thread-safe state management, delegate-based connection lifecycle, reconnect ring buffer, and batch-splitting; re-introduces race conditions, strong self capture, fragile 0.5s connection timer, and unbounded reconnect attempts cap.
desktop/Desktop/Sources/VADGateService.swift	Reverts maxBatchBytes cap and auto-emit logic; batch recordings longer than ~23s will produce HTTP 413 responses.
desktop/Desktop/Sources/MainWindow/CrispManager.swift	Reverts batched @published write; unreadCount is now incremented per-message, triggering multiple SwiftUI re-renders per poll cycle (dock tile hang regression).
desktop/Desktop/Sources/MainWindow/SidebarView.swift	Restores @ObservedObject var crispManager — correct for UI binding, but amplifies the per-message re-render issue in CrispManager.
desktop/Desktop/Sources/AppState.swift	Reverts call site from batchTranscribeWithSplitting back to batchTranscribeFull, removing automatic splitting/retry for large payloads.
desktop/Desktop/Tests/TranscriptionServiceTests.swift	Entire test file (467 lines) deleted as part of the revert; covers ReconnectAudioRingBuffer and TranscriptionService state machine — these tests will need to be re-added in the rework PR.
desktop/Desktop/Tests/OnboardingFlowTests.swift	Reverts test expectations for onboarding step count from 17 back to 5; looks like an unrelated onboarding flow migration was included in the reverted PR scope.

Sequence Diagram

sequenceDiagram
    participant App as macOS App
    participant TS as TranscriptionService
    participant Proxy as Rust Proxy
    participant DG as Deepgram WS

    Note over App,DG: After revert — known issues highlighted

    App->>TS: start(onTranscript:…)
    TS->>Proxy: WebSocket connect
    Proxy->>DG: WebSocket connect (upstream)
    Note over TS: ⚠️ 0.5s timer sets isConnected=true (no real handshake confirmation)
    TS-->>App: onConnected()

    App->>TS: sendAudio(data)
    TS->>Proxy: binary frame
    Proxy->>DG: binary frame

    DG-->>Proxy: transcript JSON
    Note over Proxy: ⚠️ If DG closes, close frame NOT forwarded to client
    Proxy-->>TS: transcript JSON
    TS-->>App: onTranscript(segment)

    Note over DG,Proxy: DG disconnects (error)
    Proxy->>Proxy: while let Some(Ok(…)) exits silently
    Note over Proxy: ⚠️ Client→Upstream task keeps running

    TS->>TS: handleDisconnection()
    Note over TS: ⚠️ Race: concurrent callbacks may both pass guard isConnected
    TS->>TS: reconnect (attempt 1..10)
    Note over TS: ⚠️ strong self capture in Task

Loading

Comments Outside Diff (5)

1. desktop/Desktop/Sources/TranscriptionService.swift, line 612-622 (link)

   Fragile 0.5s timer for connection detection re-introduced

   The reverted code uses a fixed DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) heuristic to decide the WebSocket handshake "succeeded", checking only whether webSocketTask.state == .running. This does not confirm a successful server handshake — the task can be .running before TLS or the WebSocket upgrade completes, or even during a failed connection attempt that hasn't yet surfaced an error.

   The removed code used URLSessionWebSocketDelegate.urlSession(_:webSocketTask:didOpenWithProtocol:) which fires only after the actual handshake succeeds. This regression is expected to cause the WebSocket transcription disconnect issues that PR Fix WebSocket transcription disconnects — 64K Sentry events (#6193) #6201 addressed. Worth ensuring the rework restores the delegate-based approach.

2. desktop/Desktop/Sources/TranscriptionService.swift, line 782-830 (link)

   Non-atomic handleDisconnection and strong self capture re-introduced

   Two issues are re-introduced here:

   1. Race condition in handleDisconnection: The guard isConnected else { return } / isConnected = false pattern is not atomic. handleDisconnection can be called concurrently from the URLSession delegate queue (receive error), the keepalive send callback, and the watchdog task. Two concurrent callers can both pass the guard before either writes false, triggering duplicate reconnect tasks and leaking Task handles.

   2. Strong self capture in reconnectTask: The closure captures self strongly, which can form a retain cycle and prevent TranscriptionService from being deallocated until the Task finishes:

   reconnectTask = Task {
       // ...
       self.connect()  // strong capture
   }

   Should be [weak self] with an early-exit guard:

   reconnectTask = Task { [weak self] in
       try? await Task.sleep(nanoseconds: UInt64(delay * 1_000_000_000))
       guard !Task.isCancelled, let self = self, self.shouldReconnect else { return }
       self.connect()
   }

   The reverted PR Fix WebSocket transcription disconnects — 64K Sentry events (#6193) #6201 code used a serial stateQueue to make handleDisconnection idempotent and used [weak self] correctly. Worth preserving both in the rework.

3. desktop/Backend-Rust/src/routes/proxy.rs, line 43-57 (link)

   Error variants silently dropped, close frame not forwarded re-introduced

   The reverted proxy code uses while let Some(Ok(msg)) = ... which silently drops all Err(_) results — a client or upstream transport error is treated identically to a clean end-of-stream. The removed PR Fix WebSocket transcription disconnects — 64K Sentry events (#6193) #6201 code matched on Err(_) and propagated a ProxyCloseOrigin so the surviving side received a proper close frame with a timeout.

   After this revert:

   • A network error on one side will cause the other side to hang indefinitely, waiting for messages that will never arrive.
   • The upstream (Deepgram) close frame is not forwarded to the client, so the client WebSocket connection is left open until an OS-level timeout fires.

   This is the root cause of the disconnect issues that PR Fix WebSocket transcription disconnects — 64K Sentry events (#6193) #6201 fixed. Both halves of the bidirectional proxy have this pattern.

4. desktop/Desktop/Sources/VADGateService.swift, line 644-659 (link)

   Unbounded batch buffer and 413 regression re-introduced

   The revert removes maxBatchBytes = 1_500_000 and the autoEmitBatchBuffer auto-emit guard. In batch mode, batchAudioBuffer now grows without bound for recordings longer than ~23s of stereo audio. The backend/proxy will respond with HTTP 413 once the payload exceeds the server body-size limit. AppState.swift is also reverted back to calling batchTranscribeFull (which has no retry/split logic) instead of batchTranscribeWithSplitting.

   Users recording extended conversations will silently lose their transcription. Worth prioritising the 413 guard in the rework PR.

5. desktop/Desktop/Sources/MainWindow/CrispManager.swift, line 153-157 (link)

   Per-message @Published writes re-introduced (dock tile hang)

   The reverted code increments unreadCount (a @Published property) once per message inside the loop, triggering a separate SwiftUI observation notification for every unread message in a single poll cycle. PR Fix dock tile hang — 11.4K Sentry events (#6194) #6205 batched these into a single post-loop write to avoid the rapid-fire @Published updates that were causing dock tile hangs.

   Additionally, SidebarView.swift is restored to observing CrispManager via @ObservedObject, which means every unreadCount increment will force a full SidebarView re-render. The combination of N increments × 1 re-render each is the root of the dock tile hang. The rework should preserve the batch-update pattern.

_{Reviews (1): Last reviewed commit: "Revert "Fix WebSocket transcription disc..." | Re-trigger Greptile}