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>
This commit is contained in:
Neil
2026-08-13 00:02:30 -07:00
co-authored by Orca
parent 3339053877
commit a013b5d35d
2 changed files with 55 additions and 11 deletions
@@ -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'])
})
})
@@ -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
}