Skip to content

perf(mobile): stabilize session mutation callbacks to stop row re-renders - #6765

Merged
iscekic merged 4 commits into
mainfrom
kwf/janitor-mobile-agents-list-968fa7d079
Sep 28, 2026
Merged

iscekic merged 4 commits into
mainfrom
kwf/janitor-mobile-agents-list-968fa7d079

Conversation

@iscekic

@iscekic iscekic commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Fix proof

useSessionMutations returns deleteSession and renameSession as inline closures re-created on every render, but session-list-content.tsx lists both in renderItem's useCallback dependency array, so rend

Asserted value: apps/mobile/src/lib/hooks/use-session-mutations.ts. Sense check (model): The diff wraps renameSessionAsync in useCallback with the stable [renameSessionMutationAsync] dep and binds deleteSessionAsync to the stable mutateAsync, so the returned callbacks listed in renderItem's deps no longer change identity.

The scripts were proven on an earlier base, so only the head ran.

Head b0266073993e

Head log: backend-assert 01a0aa76cd02 exited 0
$ git diff --unified=0 2b2b4faac4eff1d704026c2c251ff23326db390a b0266073993e70c55515e30a40199210a54677f7 -- apps/mobile/src/lib/hooks/use-session-mutations.ts
diff --git a/apps/mobile/src/lib/hooks/use-session-mutations.ts b/apps/mobile/src/lib/hooks/use-session-mutations.ts
--- a/apps/mobile/src/lib/hooks/use-session-mutations.ts
+++ b/apps/mobile/src/lib/hooks/use-session-mutations.ts
+    (sessionId: string, title: string) => {
@@ -231,0 +247,6 @@ export function useSessionMutations() {
+    [renameSessionAsync]
+  );
+
+  return {
+    deleteSession,
+    renameSession,

Fix proof

useSessionMutations returns deleteSession and renameSession as inline closures re-created on every render, but session-list-content.tsx lists both in renderItem's useCallback dependency array, so rend

Asserted value: apps/mobile/src/lib/hooks/use-session-mutations.ts. Sense check (model): use-session-mutations.ts adds const renameSessionAsync = useCallback(..., [renameSessionMutationAsync]) and aliases deleteSessionAsync from deleteSessionMutation.mutateAsync, wrapping the returned closures as the claim requires.

The scripts were proven on an earlier base, so only the head ran.

Head 674e656afd0d

Head log: backend-assert e75b8badae3d exited 0
$ git diff --unified=0 332f0c033e0781f85b7782c75ee4f2f595a7e2a8 674e656afd0dda2cd2c75bcf1247d20a66eabdf9 -- apps/mobile/src/lib/hooks/use-session-mutations.ts
diff --git a/apps/mobile/src/lib/hooks/use-session-mutations.ts b/apps/mobile/src/lib/hooks/use-session-mutations.ts
--- a/apps/mobile/src/lib/hooks/use-session-mutations.ts
+++ b/apps/mobile/src/lib/hooks/use-session-mutations.ts
+    (sessionId: string, title: string) => {
@@ -231,0 +247,6 @@ export function useSessionMutations() {
+    [renameSessionAsync]
+  );
+
+  return {
+    deleteSession,
+    renameSession,

Changelog for users

  • Visible agents list rows no longer re-render during pull to refresh, pagination, and focus updates.

Changelog for maintainers

  • deleteSession, renameSession, and renameSessionAsync now keep stable identities, so list row renderers stay memoized.
  • renameSessionAsync reads the observer-bound mutateAsync, which React Query keeps stable; verify that on dependency upgrades.
  • Per-session sequencing through chainSave, operation epochs, and the detail caller's rejection contract stay unchanged.
  • The rename path still records the user's title before the write so the unnamed placeholder never hides it.
  • The hook still returns a fresh object each render; only its callbacks are stable, so object-level dependencies still invalidate.

E2E proof

useSessionMutations returns deleteSession and renameSession as inline closures re-created on every render, but session-list-content.tsx lists both in renderItem's useCallback dependency array, so renderItem's identity changes on every list render and every visible stored row (a non-memoized heavy component) re-renders during pull, pagination and focus updates; wrap the returned functions in useCallback so the row renderer stays stable.

Code trace: apps/mobile/src/lib/hooks/use-session-mutations.ts:2 changed in 08961922ed6ab87d484adad461e58dd28e58e9c4. Sense check (model): use-session-mutations.ts:2 now defines renameSessionAsync = useCallback(async (...), [renameSessionMutationAsync]) and binds the callbacks to the stable mutateAsync, so the returned functions no longer get a new identity each render and renderItem's dependency array stays stable.

Changed lines
+import { useCallback } from 'react';
+  // The mutation result object is rebuilt every render, so depend on the
+  // observer-bound `mutateAsync` (stable for the hook's lifetime) instead.
+  // Callers list these callbacks in their own dependency arrays; an unstable
+  // identity there would re-render every visible row on unrelated updates.
+  const deleteSessionAsync = deleteSessionMutation.mutateAsync;
+  const renameSessionMutationAsync = renameSessionMutation.mutateAsync;
+
-  const renameSessionAsync = async (sessionId: string, title: string) => {
-    const epoch = currentAuthEpoch();
-    const input = { session_id: sessionId, title };
-    // Record the user's own title before the write so the render paths never
-    // hide it as the backend's unnamed placeholder (the rename API accepts any
-    // nonblank title, including one that looks like the placeholder).
-    rememberUserSessionTitle(sessionId, title);
-    operationEpochs.set(input, epoch);
-    await chainSave(sessionId, async () => {
-      assertCurrentOperation(epoch);
-      await renameSessionMutation.mutateAsync(input);
-      assertCurrentOperation(epoch);
-    });
-  };
+  const renameSessionAsync = useCallback(
+    async (sessionId: string, title: string) => {
+      const epoch = currentAuthEpoch();
+      const input = { session_id: sessionId, title };
+      // Record the user's own title before the write so the render paths never
+      // hide it as the backend's unnamed placeholder (the rename API accepts any
+      // nonblank title, including one that looks like the placeholder).
+      rememberUserSessionTitle(sessionId, title);
+      operationEpochs.set(input, epoch);
+      await chainSave(sessionId, async () => {
+        assertCurrentOperation(epoch);
+        await renameSessionMutationAsync(input);
+        assertCurrentOperation(epoch);
+      });
+    },
+    [renameSessionMutationAsync]
+  );
-  return {
Owner request

Fix 1 janitor finding in mobile/agents-list. Fix every one; the proof covers each.

  1. useSessionMutations returns deleteSession and renameSession as inline closures re-created on every render, but session-list-content.tsx lists both in renderItem's useCallback dependency array, so renderItem's identity changes on every list render and every visible stored row (a non-memoized heavy component) re-renders during pull, pagination and focus updates; wrap the returned functions in useCallback so the row renderer stays stable.
    Trace: apps/mobile/src/lib/hooks/use-session-mutations.ts:205: useSessionMutations returns deleteSession and renameSession as inline closures re-created on every render, but session-list-content.tsx lists both in renderItem's useCallback dependency array, so renderItem's identity changes on every list render and every visible stored row (a non-memoized heavy component) re-renders during pull, pagination and focus updates; wrap the returned functions in useCallback so the row renderer stays stable. (janitor area performance-reliability).
    Files: apps/mobile/src/lib/hooks/use-session-mutations.ts.

@iscekic iscekic added the kwf-janitor Admitted to the workflow from a janitor finding label Sep 26, 2026
@kilo-code-bot

kilo-code-bot Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of the new test-only commit: the shared expectMutationAccountUnchanged helper extraction and the useCallback-as-identity React mocks are correct, and the callback-stabilization refactor in use-session-mutations.ts remains behaviorally equivalent with stable dependency arrays.

Files Reviewed (4 files)
  • apps/mobile/src/lib/active-sessions-live-sync.test-helpers.ts
  • apps/mobile/src/lib/hooks/use-session-mutations.test.ts
  • apps/mobile/src/lib/hooks/use-session-mutations.user-title.test.ts
  • apps/mobile/src/lib/hooks/use-session-mutations.ts
Previous Review Summaries (2 snapshots, latest commit 674e656)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 674e656)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of the new test-only commit: the shared expectMutationAccountUnchanged helper extraction and the useCallback-as-identity React mocks are correct, and the callback-stabilization refactor in use-session-mutations.ts remains behaviorally equivalent with stable dependency arrays.

Files Reviewed (4 files)
  • apps/mobile/src/lib/active-sessions-live-sync.test-helpers.ts
  • apps/mobile/src/lib/hooks/use-session-mutations.test.ts
  • apps/mobile/src/lib/hooks/use-session-mutations.user-title.test.ts
  • apps/mobile/src/lib/hooks/use-session-mutations.ts

Previous review (commit 8cdacd3)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Single-file memoization refactor in use-session-mutations.ts; the wrapped callbacks are behaviorally equivalent, and the mutateAsync identity they depend on is stable per mutation observer, so the dependency arrays are sound.

Files Reviewed (1 file)
  • apps/mobile/src/lib/hooks/use-session-mutations.ts

Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@iscekic iscekic self-assigned this Sep 26, 2026
@iscekic

iscekic commented Sep 26, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Fixed failing checks in 674e656.

@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 26, 2026
@iscekic iscekic added merge-by-human the merge bot routed this PR to a human merge-by-bot and removed human-ready The PR is ready for human review. merge-by-human the merge bot routed this PR to a human labels Sep 26, 2026
@iscekic
iscekic marked this pull request as draft September 28, 2026 08:41
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 08:45
@iscekic
iscekic marked this pull request as draft September 28, 2026 08:53
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 09:02
@iscekic
iscekic marked this pull request as draft September 28, 2026 09:18
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 09:30
Comment thread apps/mobile/src/lib/hooks/use-session-mutations.test.ts
@iscekic
iscekic marked this pull request as draft September 28, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kwf-janitor Admitted to the workflow from a janitor finding merge-by-bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants