perf(client): equal query refetches no longer re-render PR rows

apps/web/src/components/chat/ThreadDetailsPrRow.browser.test.tsx

@@ -1,4 +1,5 @@
 import { EnvironmentId, type PullRequestCheck } from "@t3tools/contracts";
+import { AsyncResult } from "effect/reactivity";
 import { act } from "react";
 import { createRoot, type Root } from "react-dom/client";
 import { afterEach, beforeEach, expect, it, vi } from "vite-plus/test";
@@ -13,34 +14,33 @@ vi.mock(import("~/state/entities"), async (importOriginal) => ({
   ...(await importOriginal()),
   useServerConfigs: (() => new Map()) as never,
 }));
+const queryData = () => ({
+  number: 1,
+  title: "Test PR",
+  state: "open",
+  isDraft: false,
+  mergeability: "mergeable",
+  headBranch: "feature",
+  baseBranch: "main",
+  changedFiles: 1,
+  additions: 1,
+  deletions: 0,
+  checks: [state.status, ...(state.extraStatus ? [state.extraStatus] : [])].map((status) => ({
+    name: `CI-${status}`,
+    status,
+    description: null,
+    url: null,
+  })),
+  capabilities: { actions: ["merge"], mergeMethods: ["merge"] },
+  viewerPermissions: { actions: ["merge"] },
+  mergeCapabilities: { merge: true, squash: false, rebase: false },
+});
 vi.mock(import("~/state/query"), async (importOriginal) => ({
   ...(await importOriginal()),
-  useEnvironmentQuery: (() => ({
-    data: {
-      number: 1,
-      title: "Test PR",
-      state: "open",
-      isDraft: false,
-      mergeability: "mergeable",
-      headBranch: "feature",
-      baseBranch: "main",
-      changedFiles: 1,
-      additions: 1,
-      deletions: 0,
-      checks: [state.status, ...(state.extraStatus ? [state.extraStatus] : [])].map((status) => ({
-        name: `CI-${status}`,
-        status,
-        description: null,
-        url: null,
-      })),
-      capabilities: { actions: ["merge"], mergeMethods: ["merge"] },
-      viewerPermissions: { actions: ["merge"] },
-      mergeCapabilities: { merge: true, squash: false, rebase: false },
-    },
-    dataUpdatedAt: 1,
-    isPending: false,
-    refresh: vi.fn(),
-  })) as never,
+  useEnvironmentQuerySelector: ((_atom: unknown, select: (result: unknown) => unknown) =>
+    select(AsyncResult.success(queryData()))) as never,
+  useEnvironmentQueryIsNewer: () => true,
+  useEnvironmentQueryRefresh: () => ({ refresh: vi.fn(), isPending: () => false }),
 }));
 vi.mock(import("../pullRequest/usePullRequestActions"), async (importOriginal) => ({
   ...(await importOriginal()),

apps/web/src/components/chat/ThreadDetailsPrRow.tsx

@@ -1,5 +1,6 @@
 import { ThreadDetailsControl } from "./ThreadDetailsControl";
 import { PullRequestGlyph } from "../pullRequest/pullRequestIcons";
+import { selectEnvironmentQueryData } from "@t3tools/client-runtime/state/runtime";
 import type { EnvironmentProject } from "@t3tools/client-runtime/state/shell";
 import type { EnvironmentId, ProjectId, PullRequestRef } from "@t3tools/contracts";
 import { sourceControlRepositorySelector } from "@t3tools/shared/sourceControl";
@@ -11,14 +12,19 @@ import {
   GitBranchIcon,
   TriangleAlertIcon,
 } from "lucide-react";
+import { AsyncResult } from "effect/reactivity";
 import { useState, type MouseEvent as ReactMouseEvent } from "react";
 
 import { useLiveRefresh } from "~/hooks/useLiveRefresh";
 import { usePullRequestChecksRefresh } from "~/hooks/usePullRequestChecksRefresh";
 import { cn } from "~/lib/utils";
 import { useServerConfigs } from "~/state/entities";
 import { pullRequestEnvironment } from "~/state/pullRequests";
-import { useEnvironmentQuery } from "~/state/query";
+import {
+  useEnvironmentQueryIsNewer,
+  useEnvironmentQueryRefresh,
+  useEnvironmentQuerySelector,
+} from "~/state/query";
 
 import {
   buildFixFindingsHandoff,
@@ -64,6 +70,10 @@ import {
   THREAD_DETAILS_PANEL_SPLIT_SEPARATOR_CLASS,
 } from "./threadDetailsPanelStyles";
 
+// A server without checks for this pull request answers null; polling it again finds nothing new.
+const isSettledWithoutChecks = (result: AsyncResult.AsyncResult<unknown, unknown>) =>
+  result._tag === "Success" && result.value === null;
+
 export function ThreadDetailsPrRow({
   environmentId,
   pr,
@@ -101,37 +111,46 @@ export function ThreadDetailsPrRow({
           ? { projectId: project.id as ProjectId, repository, number }
           : null
       : null;
-  const detailQuery = useEnvironmentQuery(
+  const detailAtom =
     reference === null
       ? null
       : pullRequestEnvironment.detail({
           environmentId,
           input: { ...reference, allowStale: false },
-        }),
-  );
+        });
+  // Read the queries part by part: every poll publishes a new result (a new
+  // timestamp, `waiting` on and off) even when nothing in it changed.
+  const detailData = useEnvironmentQuerySelector(detailAtom, selectEnvironmentQueryData);
+  const detailQuery = useEnvironmentQueryRefresh(detailAtom);
   const supportsChecks =
     serverConfigs.get(environmentId)?.environment.capabilities.pullRequestChecks === true;
-  const checksQuery = useEnvironmentQuery(
-    supportsChecks && reference !== null && detailQuery.data !== null
+  const checksAtom =
+    supportsChecks && reference !== null && detailData !== null
       ? pullRequestEnvironment.checks({ environmentId, input: reference })
-      : null,
-  );
+      : null;
+  const checksData = useEnvironmentQuerySelector(checksAtom, selectEnvironmentQueryData);
+  const checksSettledEmpty = useEnvironmentQuerySelector(checksAtom, isSettledWithoutChecks);
+  const checksQuery = useEnvironmentQueryRefresh(checksAtom);
+  const checksAreNewer = useEnvironmentQueryIsNewer(checksAtom, detailAtom);
   const detail =
-    detailQuery.data === null
+    detailData === null
       ? null
-      : checksQuery.data !== null && checksQuery.dataUpdatedAt >= detailQuery.dataUpdatedAt
-        ? { ...detailQuery.data, ...checksQuery.data }
-        : detailQuery.data;
+      : checksData !== null && checksAreNewer
+        ? { ...detailData, ...checksData }
+        : detailData;
   const open = reference !== null && (detail?.state ?? pr?.state) === "open";
   const refreshKey = `${environmentId}:${project?.id}:${reference?.host}:${reference?.repository}:${number}`;
-  useLiveRefresh(detailQuery.isPending ? null : detailQuery.refresh, {
-    enabled: open,
-    key: `workspace-pr:${refreshKey}`,
-    intervalMs: 10 * 60_000,
-  });
+  useLiveRefresh(
+    () => {
+      if (!detailQuery.isPending()) detailQuery.refresh();
+    },
+    { enabled: open, key: `workspace-pr:${refreshKey}`, intervalMs: 10 * 60_000 },
+  );
   usePullRequestChecksRefresh({
-    refresh: checksQuery.isPending || detailQuery.isPending ? null : checksQuery.refresh,
-    enabled: open && supportsChecks && !(checksQuery.isSuccess && checksQuery.data === null),
+    refresh: () => {
+      if (!checksQuery.isPending() && !detailQuery.isPending()) checksQuery.refresh();
+    },
+    enabled: open && supportsChecks && !checksSettledEmpty,
     key: `workspace-pr-checks:${refreshKey}`,
     checks: detail?.checks ?? [],
   });

apps/web/src/state/query.ts

@@ -1,7 +1,8 @@
-import { useAtomRefresh, useAtomValue } from "@effect/atom-react";
+import { RegistryContext, useAtomRefresh, useAtomValue } from "@effect/atom-react";
 import * as Cause from "effect/Cause";
 import * as Option from "effect/Option";
 import { AsyncResult, Atom } from "effect/reactivity";
+import { useContext } from "react";
 
 const EMPTY_ASYNC_RESULT_ATOM = Atom.make(AsyncResult.initial<never, never>(false)).pipe(
   Atom.withLabel("web-environment-query:empty"),
@@ -24,6 +25,14 @@ export function formatEnvironmentQueryError(cause: Cause.Cause<unknown>): string
     : "The environment request failed.";
 }
 
+function queryUpdatedAt(result: AsyncResult.AsyncResult<unknown, unknown>): number {
+  return result._tag === "Success"
+    ? result.timestamp
+    : result._tag === "Failure"
+      ? (Option.getOrNull(result.previousSuccess)?.timestamp ?? 0)
+      : 0;
+}
+
 export function useEnvironmentQuery<A, E>(
   atom: Atom.Atom<AsyncResult.AsyncResult<A, E>> | null,
 ): EnvironmentQueryView<A, E> {
@@ -32,12 +41,7 @@ export function useEnvironmentQuery<A, E>(
   const refresh = useAtomRefresh(selectedAtom);
   return {
     data: Option.getOrNull(AsyncResult.value(result)),
-    dataUpdatedAt:
-      result._tag === "Success"
-        ? result.timestamp
-        : result._tag === "Failure"
-          ? (Option.getOrNull(result.previousSuccess)?.timestamp ?? 0)
-          : 0,
+    dataUpdatedAt: queryUpdatedAt(result),
     error: result._tag === "Failure" ? formatEnvironmentQueryError(result.cause) : null,
     failure:
       result._tag === "Failure" ? Option.getOrNull(Cause.findErrorOption(result.cause)) : null,
@@ -46,3 +50,59 @@ export function useEnvironmentQuery<A, E>(
     refresh,
   };
 }
+
+/**
+ * Reads one part of a query's result. Every refetch publishes a new result (a
+ * new timestamp, `waiting` on and off), and this re-renders only when the
+ * selected part changes. `select` must be defined outside the component: a new
+ * function per render would rebuild the derived atom every render.
+ */
+export function useEnvironmentQuerySelector<A, E, B>(
+  atom: Atom.Atom<AsyncResult.AsyncResult<A, E>> | null,
+  select: (result: AsyncResult.AsyncResult<A, E>) => B,
+): B {
+  return useAtomValue(atom ?? EMPTY_ASYNC_RESULT_ATOM, select);
+}
+
+type QueryResultAtom = Atom.Atom<AsyncResult.AsyncResult<unknown, unknown>>;
+// Lives as long as both queries do, so a row keeps the same comparison atom across renders.
+const queryIsNewerAtoms = new WeakMap<
+  QueryResultAtom,
+  WeakMap<QueryResultAtom, Atom.Atom<boolean>>
+>();
+function queryIsNewerAtom(query: QueryResultAtom, than: QueryResultAtom): Atom.Atom<boolean> {
+  let byThan = queryIsNewerAtoms.get(query);
+  if (byThan === undefined) {
+    byThan = new WeakMap();
+    queryIsNewerAtoms.set(query, byThan);
+  }
+  let atom = byThan.get(than);
+  if (atom === undefined) {
+    atom = Atom.make((get) => queryUpdatedAt(get(query)) >= queryUpdatedAt(get(than)));
+    byThan.set(than, atom);
+  }
+  return atom;
+}
+
+/** Whether `query` last succeeded no earlier than `than`, without re-rendering on every refetch. */
+export function useEnvironmentQueryIsNewer(
+  query: QueryResultAtom | null,
+  than: QueryResultAtom | null,
+): boolean {
+  return useAtomValue(
+    queryIsNewerAtom(query ?? EMPTY_ASYNC_RESULT_ATOM, than ?? EMPTY_ASYNC_RESULT_ATOM),
+  );
+}
+
+/**
+ * The query's refresh, plus its pending state read when called rather than
+ * subscribed to, for callers that only gate a refresh on it.
+ */
+export function useEnvironmentQueryRefresh<A, E>(
+  atom: Atom.Atom<AsyncResult.AsyncResult<A, E>> | null,
+): { readonly refresh: () => void; readonly isPending: () => boolean } {
+  const registry = useContext(RegistryContext);
+  const selectedAtom = atom ?? EMPTY_ASYNC_RESULT_ATOM;
+  const refresh = useAtomRefresh(selectedAtom);
+  return { refresh, isPending: () => atom !== null && registry.get(selectedAtom).waiting };
+}

packages/client-runtime/src/state/runtime.test.ts

@@ -31,6 +31,7 @@ import {
   environmentRpcKey,
   createAtomCommandScheduler,
   createEnvironmentQueryAtomFamily,
+  selectEnvironmentQueryData,
   createEnvironmentSubscriptionAtomFamily,
   createRuntimeCommand,
   scheduleAtomCommandEffect,
@@ -373,6 +374,62 @@ describe("finite environment subscription lifecycle", () => {
 });
 
 describe("environment query lifecycle", () => {
+  it.effect("keeps the previous value when a refetch returns equal content", () =>
+    Effect.scoped(
+      Effect.gen(function* () {
+        let checks = [{ name: "ci", status: "passed" }];
+        const execute = Effect.sync(() => ({ checks: checks.map((check) => ({ ...check })) }));
+        const harness = yield* makeEnvironmentQueryHarness(execute);
+        const registry = yield* mountEnvironmentQuery(harness.atom);
+
+        const first = yield* readEnvironmentQuery(registry, harness.atom);
+        registry.refresh(harness.atom);
+        const equal = yield* readEnvironmentQuery(registry, harness.atom);
+        expect(equal).toBe(first);
+
+        checks = [{ name: "ci", status: "failed" }];
+        registry.refresh(harness.atom);
+        const changed = yield* readEnvironmentQuery(registry, harness.atom);
+        expect(changed).not.toBe(first);
+        expect(changed.checks[0]?.status).toBe("failed");
+      }),
+    ),
+  );
+
+  it.effect("does not notify a data-only reader when a refetch returns equal content", () =>
+    Effect.scoped(
+      Effect.gen(function* () {
+        let status = "passed";
+        const execute = Effect.sync(() => ({ checks: [{ name: "ci", status }] }));
+        const harness = yield* makeEnvironmentQueryHarness(execute);
+        const registry = yield* mountEnvironmentQuery(harness.atom);
+        yield* readEnvironmentQuery(registry, harness.atom);
+
+        const data = Atom.map(harness.atom, selectEnvironmentQueryData);
+        const seen: Array<unknown> = [];
+        let resultNotifications = 0;
+        const unsubscribeData = registry.subscribe(data, (value) => seen.push(value), {
+          immediate: true,
+        });
+        yield* Effect.addFinalizer(() => Effect.sync(unsubscribeData));
+        const unsubscribeResult = registry.subscribe(harness.atom, () => resultNotifications++);
+        yield* Effect.addFinalizer(() => Effect.sync(unsubscribeResult));
+
+        registry.refresh(harness.atom);
+        yield* readEnvironmentQuery(registry, harness.atom);
+        // The result itself still moves (timestamp, `waiting`), which SWR relies on.
+        expect(resultNotifications).toBeGreaterThan(0);
+        expect(seen).toHaveLength(1);
+
+        status = "failed";
+        registry.refresh(harness.atom);
+        yield* readEnvironmentQuery(registry, harness.atom);
+        expect(seen).toHaveLength(2);
+        expect(seen[1]).toEqual({ checks: [{ name: "ci", status: "failed" }] });
+      }),
+    ),
+  );
+
   it.effect(
     "retries an interrupted query without exposing a failure during session replacement",
     () =>

packages/client-runtime/src/state/runtime.ts

@@ -1,6 +1,7 @@
 import type { QueryPersistence } from "./queryPersistence.ts";
 import { createCommandPermissions } from "./commandPermissions.ts";
 import { followStreamInEnvironment } from "./environmentStreams.ts";
+import { reuseUnchanged } from "./serverConfigProjection.ts";
 export { runStreamInEnvironment, followStreamInEnvironment } from "./environmentStreams.ts";
 import {
   type ClientGuardedRpcTag,
@@ -514,6 +515,14 @@ const refreshEveryUntilFinal =
       { initialValueTarget: self },
     );
 
+/**
+ * A query result's data alone. A refetch always publishes a new result (a new
+ * timestamp, `waiting` on and off), while equal data keeps its identity, so a
+ * reader of only this skips the refetches that changed nothing.
+ */
+export const selectEnvironmentQueryData = <A, E>(result: AsyncResult.AsyncResult<A, E>): A | null =>
+  Option.getOrNull(AsyncResult.value(result));
+
 export function createEnvironmentQueryAtomFamily<R, ER, Input, A, E>(
   runtime: Atom.AtomRuntime<EnvironmentRegistry.EnvironmentRegistry | R, ER>,
   options: EnvironmentQueryAtomOptions<
@@ -560,8 +569,17 @@ export function createEnvironmentQueryAtomFamily<R, ER, Input, A, E>(
         const connection = Option.getOrNull(
           AsyncResult.value(get(connectionAtom(target.environmentId))),
         );
+        // A refetch that returns equal content keeps the previous value's
+        // references, so readers of the data skip re-rendering on idle polls.
+        const share = (value: A): A =>
+          reuseUnchanged(
+            Option.getOrUndefined(
+              Option.flatMap(get.self<AsyncResult.AsyncResult<A, unknown>>(), AsyncResult.value),
+            ),
+            value,
+          );
         const emit = (value: A) =>
-          Effect.sync(() => get.setSelf(AsyncResult.success(value, { waiting: true }))).pipe(
+          Effect.sync(() => get.setSelf(AsyncResult.success(share(value), { waiting: true }))).pipe(
             // A value set while the atom builds wins over the build's own
             // result, so a query that then settled synchronously would stay
             // waiting forever. Suspend on a promise (`yieldNow` is drained
@@ -604,7 +622,7 @@ export function createEnvironmentQueryAtomFamily<R, ER, Input, A, E>(
               );
           }
         };
-        if (!options.persistence) return execute();
+        if (!options.persistence) return execute().pipe(Effect.map(share));
         const persistence = options.persistence;
         const cacheKey = JSON.stringify([options.label, target.input]);
         const previous = Option.flatMap(
@@ -623,6 +641,7 @@ export function createEnvironmentQueryAtomFamily<R, ER, Input, A, E>(
           }
           if (Option.isNone(previous) && Option.isSome(cached)) yield* emit(cached.value);
           return yield* execute().pipe(
+            Effect.map(share),
             Effect.tap((value) => persistence.save(target.environmentId, cacheKey, value)),
           );
         });