fix(app): session review re-rendering too aggressively

1 files changed, 67 insertions, 47 deletions

diff --git a/packages/ui/src/components/session-review.tsx b/packages/ui/src/components/session-review.tsx
index 15464d3ba..7f737032e 100644
--- a/packages/ui/src/components/session-review.tsx
+++ b/packages/ui/src/components/session-review.tsx
@@ -189,8 +189,10 @@ export const SessionReview = (props: SessionReviewProps) => {
   const [opened, setOpened] = createSignal<SessionReviewFocus | null>(null)
 
   const open = () => props.open ?? store.open
+  const files = createMemo(() => props.diffs.map((d) => d.file))
+  const diffs = createMemo(() => new Map(props.diffs.map((d) => [d.file, d] as const)))
   const diffStyle = () => props.diffStyle ?? (props.split ? "split" : "unified")
-  const hasDiffs = () => props.diffs.length > 0
+  const hasDiffs = () => files().length > 0
 
   const handleChange = (open: string[]) => {
     props.onOpenChange?.(open)
@@ -199,7 +201,7 @@ export const SessionReview = (props: SessionReviewProps) => {
   }
 
   const handleExpandOrCollapseAll = () => {
-    const next = open().length > 0 ? [] : props.diffs.map((d) => d.file)
+    const next = open().length > 0 ? [] : files()
     handleChange(next)
   }
 
@@ -322,51 +324,54 @@ export const SessionReview = (props: SessionReviewProps) => {
       <div data-slot="session-review-container" class={props.classes?.container}>
         <Show when={hasDiffs()} fallback={props.empty}>
           <Accordion multiple value={open()} onChange={handleChange}>
-            <For each={props.diffs}>
-              {(diff) => {
+            <For each={files()}>
+              {(file) => {
                 let wrapper: HTMLDivElement | undefined
 
-                const expanded = createMemo(() => open().includes(diff.file))
+                const diff = createMemo(() => diffs().get(file))
+                const item = () => diff()!
+
+                const expanded = createMemo(() => open().includes(file))
                 const [force, setForce] = createSignal(false)
 
-                const comments = createMemo(() => (props.comments ?? []).filter((c) => c.file === diff.file))
+                const comments = createMemo(() => (props.comments ?? []).filter((c) => c.file === file))
                 const commentedLines = createMemo(() => comments().map((c) => c.selection))
 
-                const beforeText = () => (typeof diff.before === "string" ? diff.before : "")
-                const afterText = () => (typeof diff.after === "string" ? diff.after : "")
-                const changedLines = () => diff.additions + diff.deletions
+                const beforeText = () => (typeof item().before === "string" ? item().before : "")
+                const afterText = () => (typeof item().after === "string" ? item().after : "")
+                const changedLines = () => item().additions + item().deletions
 
                 const tooLarge = createMemo(() => {
                   if (!expanded()) return false
                   if (force()) return false
-                  if (isImageFile(diff.file)) return false
+                  if (isImageFile(file)) return false
                   return changedLines() > MAX_DIFF_CHANGED_LINES
                 })
 
-                const isAdded = () => diff.status === "added" || (beforeText().length === 0 && afterText().length > 0)
+                const isAdded = () => item().status === "added" || (beforeText().length === 0 && afterText().length > 0)
                 const isDeleted = () =>
-                  diff.status === "deleted" || (afterText().length === 0 && beforeText().length > 0)
-                const isImage = () => isImageFile(diff.file)
-                const isAudio = () => isAudioFile(diff.file)
+                  item().status === "deleted" || (afterText().length === 0 && beforeText().length > 0)
+                const isImage = () => isImageFile(file)
+                const isAudio = () => isAudioFile(file)
 
-                const diffImageSrc = dataUrlFromValue(diff.after) ?? dataUrlFromValue(diff.before)
-                const [imageSrc, setImageSrc] = createSignal<string | undefined>(diffImageSrc)
+                const diffImageSrc = createMemo(() => dataUrlFromValue(item().after) ?? dataUrlFromValue(item().before))
+                const [imageSrc, setImageSrc] = createSignal<string | undefined>(diffImageSrc())
                 const [imageStatus, setImageStatus] = createSignal<"idle" | "loading" | "error">("idle")
 
-                const diffAudioSrc = dataUrlFromValue(diff.after) ?? dataUrlFromValue(diff.before)
-                const [audioSrc, setAudioSrc] = createSignal<string | undefined>(diffAudioSrc)
+                const diffAudioSrc = createMemo(() => dataUrlFromValue(item().after) ?? dataUrlFromValue(item().before))
+                const [audioSrc, setAudioSrc] = createSignal<string | undefined>(diffAudioSrc())
                 const [audioStatus, setAudioStatus] = createSignal<"idle" | "loading" | "error">("idle")
                 const [audioMime, setAudioMime] = createSignal<string | undefined>(undefined)
 
                 const selectedLines = createMemo(() => {
                   const current = selection()
-                  if (!current || current.file !== diff.file) return null
+                  if (!current || current.file !== file) return null
                   return current.range
                 })
 
                 const draftRange = createMemo(() => {
                   const current = commenting()
-                  if (!current || current.file !== diff.file) return null
+                  if (!current || current.file !== file) return null
                   return current.range
                 })
 
@@ -418,6 +423,21 @@ export const SessionReview = (props: SessionReviewProps) => {
                 }
 
                 createEffect(() => {
+                  if (!isImage()) return
+                  const src = diffImageSrc()
+                  setImageSrc(src)
+                  setImageStatus("idle")
+                })
+
+                createEffect(() => {
+                  if (!isAudio()) return
+                  const src = diffAudioSrc()
+                  setAudioSrc(src)
+                  setAudioStatus("idle")
+                  setAudioMime(undefined)
+                })
+
+                createEffect(() => {
                   comments()
                   scheduleAnchors()
                 })
@@ -430,7 +450,7 @@ export const SessionReview = (props: SessionReviewProps) => {
                 })
 
                 createEffect(() => {
-                  if (!open().includes(diff.file)) return
+                  if (!open().includes(file)) return
                   if (!isImage()) return
                   if (imageSrc()) return
                   if (imageStatus() !== "idle") return
@@ -440,7 +460,7 @@ export const SessionReview = (props: SessionReviewProps) => {
                   if (!reader) return
 
                   setImageStatus("loading")
-                  reader(diff.file)
+                  reader(file)
                     .then((result) => {
                       const src = dataUrl(result)
                       if (!src) {
@@ -456,7 +476,7 @@ export const SessionReview = (props: SessionReviewProps) => {
                 })
 
                 createEffect(() => {
-                  if (!open().includes(diff.file)) return
+                  if (!open().includes(file)) return
                   if (!isAudio()) return
                   if (audioSrc()) return
                   if (audioStatus() !== "idle") return
@@ -465,7 +485,7 @@ export const SessionReview = (props: SessionReviewProps) => {
                   if (!reader) return
 
                   setAudioStatus("loading")
-                  reader(diff.file)
+                  reader(file)
                     .then((result) => {
                       const src = dataUrl(result)
                       if (!src) {
@@ -489,7 +509,7 @@ export const SessionReview = (props: SessionReviewProps) => {
                     return
                   }
 
-                  setSelection({ file: diff.file, range })
+                  setSelection({ file, range })
                 }
 
                 const handleLineSelectionEnd = (range: SelectedLineRange | null) => {
@@ -500,8 +520,8 @@ export const SessionReview = (props: SessionReviewProps) => {
                     return
                   }
 
-                  setSelection({ file: diff.file, range })
-                  setCommenting({ file: diff.file, range })
+                  setSelection({ file, range })
+                  setCommenting({ file, range })
                 }
 
                 const openComment = (comment: SessionReviewComment) => {
@@ -517,22 +537,22 @@ export const SessionReview = (props: SessionReviewProps) => {
 
                 return (
                   <Accordion.Item
-                    value={diff.file}
-                    id={diffId(diff.file)}
-                    data-file={diff.file}
+                    value={file}
+                    id={diffId(file)}
+                    data-file={file}
                     data-slot="session-review-accordion-item"
-                    data-selected={props.focusedFile === diff.file ? "" : undefined}
+                    data-selected={props.focusedFile === file ? "" : undefined}
                   >
                     <StickyAccordionHeader>
                       <Accordion.Trigger>
                         <div data-slot="session-review-trigger-content">
                           <div data-slot="session-review-file-info">
-                            <FileIcon node={{ path: diff.file, type: "file" }} />
+                            <FileIcon node={{ path: file, type: "file" }} />
                             <div data-slot="session-review-file-name-container">
-                              <Show when={diff.file.includes("/")}>
-                                <span data-slot="session-review-directory">{`\u202A${getDirectory(diff.file)}\u202C`}</span>
+                              <Show when={file.includes("/")}>
+                                <span data-slot="session-review-directory">{`\u202A${getDirectory(file)}\u202C`}</span>
                               </Show>
-                              <span data-slot="session-review-filename">{getFilename(diff.file)}</span>
+                              <span data-slot="session-review-filename">{getFilename(file)}</span>
                               <Show when={props.onViewFile}>
                                 <Tooltip value="Open file" placement="top" gutter={4}>
                                   <button
@@ -541,7 +561,7 @@ export const SessionReview = (props: SessionReviewProps) => {
                                     aria-label="Open file"
                                     onClick={(e) => {
                                       e.stopPropagation()
-                                      props.onViewFile?.(diff.file)
+                                      props.onViewFile?.(file)
                                     }}
                                   >
                                     <Icon name="open-file" size="small" />
@@ -557,7 +577,7 @@ export const SessionReview = (props: SessionReviewProps) => {
                                   <span data-slot="session-review-change" data-type="added">
                                     {i18n.t("ui.sessionReview.change.added")}
                                   </span>
-                                  <DiffChanges changes={diff} />
+                                  <DiffChanges changes={item()} />
                                 </div>
                               </Match>
                               <Match when={isDeleted()}>
@@ -571,7 +591,7 @@ export const SessionReview = (props: SessionReviewProps) => {
                                 </span>
                               </Match>
                               <Match when={true}>
-                                <DiffChanges changes={diff} />
+                                <DiffChanges changes={item()} />
                               </Match>
                             </Switch>
                             <span data-slot="session-review-diff-chevron">
@@ -586,7 +606,7 @@ export const SessionReview = (props: SessionReviewProps) => {
                         data-slot="session-review-diff-wrapper"
                         ref={(el) => {
                           wrapper = el
-                          anchors.set(diff.file, el)
+                          anchors.set(file, el)
                           scheduleAnchors()
                         }}
                       >
@@ -594,7 +614,7 @@ export const SessionReview = (props: SessionReviewProps) => {
                           <Switch>
                             <Match when={isImage() && imageSrc()}>
                               <div data-slot="session-review-image-container">
-                                <img data-slot="session-review-image" src={imageSrc()} alt={diff.file} />
+                                <img data-slot="session-review-image" src={imageSrc()} alt={file} />
                               </div>
                             </Match>
                             <Match when={isImage() && isDeleted()}>
@@ -634,7 +654,7 @@ export const SessionReview = (props: SessionReviewProps) => {
                             <Match when={!isImage()}>
                               <Dynamic
                                 component={diffComponent}
-                                preloadedDiff={diff.preloaded}
+                                preloadedDiff={item().preloaded}
                                 diffStyle={diffStyle()}
                                 onRendered={() => {
                                   props.onDiffRendered?.()
@@ -646,12 +666,12 @@ export const SessionReview = (props: SessionReviewProps) => {
                                 selectedLines={selectedLines()}
                                 commentedLines={commentedLines()}
                                 before={{
-                                  name: diff.file!,
-                                  contents: typeof diff.before === "string" ? diff.before : "",
+                                  name: file,
+                                  contents: typeof item().before === "string" ? item().before : "",
                                 }}
                                 after={{
-                                  name: diff.file!,
-                                  contents: typeof diff.after === "string" ? diff.after : "",
+                                  name: file,
+                                  contents: typeof item().after === "string" ? item().after : "",
                                 }}
                               />
                             </Match>
@@ -689,10 +709,10 @@ export const SessionReview = (props: SessionReviewProps) => {
                                   onCancel={() => setCommenting(null)}
                                   onSubmit={(comment) => {
                                     props.onLineComment?.({
-                                      file: diff.file,
+                                      file,
                                       selection: range(),
                                       comment,
-                                      preview: selectionPreview(diff, range()),
+                                      preview: selectionPreview(item(), range()),
                                     })
                                     setCommenting(null)
                                   }}