From a013b5d35d9ad1823e00dad7bf4db56ec0b41cc0 Mon Sep 17 00:00:00 2001
From: Neil <4138956+nwparker@users.noreply.github.com>
Date: Thu, 13 Aug 2026 00:02:30 -0700
Subject: [PATCH] perf(renderer): drop dead reuseEqualRecordMap key loop

A key missing from next always shows up as either a key-count mismatch or a failed previous lookup in the first pass, so the second pass over the previous keys could never flip identical. Cover the identity contract with focused unit tests, including the same-count key swap that the removed loop appeared to guard.

Co-authored-by: Orca <help@stably.ai>
---
 .../slices/repo-identity-reconcile.test.ts    | 53 ++++++++++++++++++-
 .../store/slices/repo-identity-reconcile.ts   | 13 ++---
 2 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/src/renderer/src/store/slices/repo-identity-reconcile.test.ts b/src/renderer/src/store/slices/repo-identity-reconcile.test.ts
index dda221e18b0..ecc1fa7d59e 100644
--- a/src/renderer/src/store/slices/repo-identity-reconcile.test.ts
+++ b/src/renderer/src/store/slices/repo-identity-reconcile.test.ts
@@ -1,7 +1,7 @@
 import { describe, expect, it } from 'vitest'
 import fs from 'node:fs'
 import type { Repo } from '../../../../shared/types'
-import { reconcileFetchedRepos } from './repo-identity-reconcile'
+import { reconcileFetchedRepos, reuseEqualRecordMap } from './repo-identity-reconcile'
 
 function makeRepo(id: string, overrides: Partial<Repo> = {}): Repo {
   return { id, path: `/${id}`, displayName: id, badgeColor: '#000', addedAt: 1, ...overrides }
@@ -123,3 +123,54 @@ describe('reconcileFetchedRepos', () => {
     expect(source).not.toMatch(/previous\.find|next\.find|findIndex/)
   })
 })
+
+describe('reuseEqualRecordMap', () => {
+  const row = (value: string): { value: string; nested: { tags: string[] } } => ({
+    value,
+    nested: { tags: [value, 'shared'] }
+  })
+
+  it('returns the previous map when every key holds an equal value', () => {
+    const previous = { a: row('a'), b: row('b') }
+    expect(reuseEqualRecordMap(previous, { a: row('a'), b: row('b') })).toBe(previous)
+  })
+
+  it('reuses the previous entry for structurally equal nested values', () => {
+    const previous = { a: row('a'), b: row('b') }
+    const result = reuseEqualRecordMap(previous, { a: row('a'), b: row('changed') })
+    expect(result).not.toBe(previous)
+    expect(result.a).toBe(previous.a)
+    expect(result.b).toEqual(row('changed'))
+  })
+
+  it('returns a new map when a value changed', () => {
+    const previous = { a: row('a') }
+    const result = reuseEqualRecordMap(previous, { a: row('changed') })
+    expect(result).not.toBe(previous)
+    expect(result.a).toEqual(row('changed'))
+  })
+
+  it('returns a new map when a key is added', () => {
+    const previous = { a: row('a') }
+    const result = reuseEqualRecordMap(previous, { a: row('a'), b: row('b') })
+    expect(result).not.toBe(previous)
+    expect(result.a).toBe(previous.a)
+    expect(Object.keys(result)).toEqual(['a', 'b'])
+  })
+
+  it('returns a new map when a key is removed', () => {
+    const previous = { a: row('a'), b: row('b') }
+    const result = reuseEqualRecordMap(previous, { a: row('a') })
+    expect(result).not.toBe(previous)
+    expect(Object.keys(result)).toEqual(['a'])
+  })
+
+  it('returns a new map when a key is swapped for a same-count replacement', () => {
+    // Why: equal key counts alone must not read as unchanged — the replacement key misses the
+    // previous lookup, which is why no second pass over the previous keys is needed.
+    const previous = { a: row('a'), b: row('b') }
+    const result = reuseEqualRecordMap(previous, { a: row('a'), c: row('b') })
+    expect(result).not.toBe(previous)
+    expect(Object.keys(result)).toEqual(['a', 'c'])
+  })
+})
diff --git a/src/renderer/src/store/slices/repo-identity-reconcile.ts b/src/renderer/src/store/slices/repo-identity-reconcile.ts
index ea823a00108..db59e619d97 100644
--- a/src/renderer/src/store/slices/repo-identity-reconcile.ts
+++ b/src/renderer/src/store/slices/repo-identity-reconcile.ts
@@ -89,8 +89,9 @@ export function reuseEqualRecordMap<T>(
   next: Readonly<Record<string, T>>
 ): Readonly<Record<string, T>> {
   const nextKeys = Object.keys(next)
-  const previousKeys = Object.keys(previous)
-  let identical = nextKeys.length === previousKeys.length
+  // Why: a matching key count plus every `next` key resolving to an equal `previous` entry below
+  // means the key sets match, so a removed key always lands as either a count or a lookup miss.
+  let identical = nextKeys.length === Object.keys(previous).length
   const reconciled: Record<string, T> = {}
   for (const key of nextKeys) {
     const existing = Object.hasOwn(previous, key) ? previous[key] : undefined
@@ -101,13 +102,5 @@ export function reuseEqualRecordMap<T>(
     identical = false
     reconciled[key] = next[key]
   }
-  if (identical) {
-    for (const key of previousKeys) {
-      if (!Object.hasOwn(next, key)) {
-        identical = false
-        break
-      }
-    }
-  }
   return identical ? previous : reconciled
 }