From 5cb9bb117a7b1aef8732f67d825f16e17ebd0b69 Mon Sep 17 00:00:00 2001
From: Greyson Parrelli <greyson@signal.org>
Date: Thu, 27 Aug 2026 10:22:32 -0400
Subject: [PATCH] Fix review perf issues.

---
 web/src/App.tsx       | 21 ++++++++++-----------
 web/src/lib/useSSE.ts | 41 ++++++++++++++++++++++++++++++++++++-----
 2 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/web/src/App.tsx b/web/src/App.tsx
index 71df920..e223a7d 100644
--- a/web/src/App.tsx
+++ b/web/src/App.tsx
@@ -373,21 +373,20 @@ export default function App() {
   }, []);
 
   // Live updates. Every event on this stream is about this review — the stream is
-  // the tab's own — so anything that isn't the opening handshake means the comment
-  // list moved and is worth refetching.
+  // the tab's own — so anything that arrives means the comment list moved and is
+  // worth refetching.
+  //
+  // The handshake refetches too, which it did not used to. The stream is dropped
+  // while the pane is hidden (see useSSE) and reconnects when it comes back, so
+  // `connected` is the one moment the rail is known to be behind: every comment
+  // written while this pane was off screen is a frame nobody was listening for.
+  // Treating the handshake as "something changed" is what closes that gap — and
+  // the same gap after a dropped connection, which was always there.
   //
   // Comments only. The diff is not pushed: the server has no watcher on the work
   // tree, and a comment arriving is a change to the rail, which can be applied
   // under the reader safely. A changed *diff* cannot — see the poll below.
-  useSSE(
-    useCallback(
-      (e) => {
-        if (e.type === 'connected') return;
-        refetchComments();
-      },
-      [refetchComments],
-    ),
-  );
+  useSSE(useCallback(() => refetchComments(), [refetchComments]));
 
   // Notice that the diff on screen has been overtaken, and say so. Nothing here
   // reloads anything: it sets `stale`, the banner offers the refresh, and the
diff --git a/web/src/lib/useSSE.ts b/web/src/lib/useSSE.ts
index fedd26f..03937bf 100644
--- a/web/src/lib/useSSE.ts
+++ b/web/src/lib/useSSE.ts
@@ -13,6 +13,22 @@ export interface ServerEvent {
 // The stream is scoped by the URL it is opened on — the tab's own `api/events` —
 // so unlike the tool this came from there is nothing to filter here: activity in
 // another tab's review never arrives in the first place.
+//
+// ## Why this closes while the pane is hidden
+//
+// Every pane is a webview onto the same origin, and they all share one WebKit
+// network session — so they also share its cap of six HTTP/1.1 connections per
+// host. A stream held open for the pane's whole lifetime is one of those six
+// spent permanently, and with a review open in seven tabs the panes starve each
+// other: the diff, comment and repo fetches queue behind streams that are only
+// ever going to sit there, and a small diff that the server answers in
+// milliseconds never loads at all.
+//
+// So a hidden pane gives its connection back, exactly as the diff poll gives up
+// its timer, and takes one again when it comes back on screen. What that costs
+// is the events that arrived while it was away, which is why reconnecting
+// resyncs the comment list rather than assuming the rail is still current — see
+// the `connected` handler in App.
 export function useSSE(onEvent: (e: ServerEvent) => void): void {
   const handler = useRef(onEvent);
   handler.current = onEvent;
@@ -22,8 +38,17 @@ export function useSSE(onEvent: (e: ServerEvent) => void): void {
     let closed = false;
     let retry: ReturnType<typeof setTimeout> | undefined;
 
+    const disconnect = () => {
+      if (retry) {
+        clearTimeout(retry);
+        retry = undefined;
+      }
+      es?.close();
+      es = null;
+    };
+
     const connect = () => {
-      if (closed) return;
+      if (closed || es || document.hidden) return;
       es = new EventSource(`${apiBase}/events`);
       // The server's opening nudge is an SSE comment, so it never reaches
       // onmessage; synthesize it, so a caller that wants to know the stream is up
@@ -37,16 +62,22 @@ export function useSSE(onEvent: (e: ServerEvent) => void): void {
         }
       };
       es.onerror = () => {
-        es?.close();
-        if (!closed) retry = setTimeout(connect, 2000);
+        disconnect();
+        if (!closed && !document.hidden) retry = setTimeout(connect, 2000);
       };
     };
 
+    const onVisibility = () => {
+      if (document.hidden) disconnect();
+      else connect();
+    };
+
     connect();
+    document.addEventListener('visibilitychange', onVisibility);
     return () => {
       closed = true;
-      if (retry) clearTimeout(retry);
-      es?.close();
+      document.removeEventListener('visibilitychange', onVisibility);
+      disconnect();
     };
   }, []);
 }