fix(chat): fix 5 message-rendering correctness bugs (#132)

- MessageBubble: memo comparator only checked the last part's `.text`,
  which is always undefined for tool parts, so tool-state/token/cost
  updates never re-rendered when a tool part was last. Replace with a
  full reference-equality sweep over message + all parts (the store
  always replaces changed refs, so this catches every real change).
- DiffView: computeDiff's O(a.length*b.length) LCS table was unbounded,
  risking OOM/ANR on large diffs, and the rendered line list was
  unbounded too. Add a size-guarded fallback (simple truncated
  remove/add diff) and cap the normal path's rendered lines, both with
  a truncation marker. Extracted computeDiff into a plain
  diff-compute.ts module (mirrors src/lib/scroll-config.ts) so it's
  unit-testable with node:test, which can't render .tsx components.
- DiffView: normalize line endings (\r?\n) before diffing so a CRLF
  vs LF mismatch doesn't show a whole file as changed.
- Markdown: the module-scope singleton CustomRenderer's github-slugger
  never reset, so useMarkdown's keys climbed on every streamed token,
  remounting the whole subtree. Scope the renderer per `children` via
  useMemo instead.
- Markdown: theme objects used heading1/heading2/heading3/listItem,
  but react-native-marked's MarkedStyles expects h1/h2/h3/li, so the
  custom heading/list styling was silently dropped. Rename the keys in
  both themes.


Claude-Session: https://claude.ai/code/session_01T12AhSnQVrSxNnvwfCx2z6

Co-authored-by: engineer <engineer@macbookpro.lan>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

src/components/chat/DiffView.tsx

@@ -1,5 +1,6 @@
 import { View, Text, StyleSheet, Platform, ScrollView } from "react-native"
 import { WIDE_CONTENT_SCROLL_CONFIG } from "../../lib/scroll-config"
+import { computeDiff } from "./diff-compute"
 
 const mono = Platform.OS === "ios" ? "Menlo" : "monospace"
 
@@ -9,67 +10,6 @@ interface Props {
   isDark: boolean
 }
 
-interface DiffLine {
-  type: "add" | "remove" | "context"
-  text: string
-}
-
-function computeDiff(before: string, after: string): DiffLine[] {
-  const a = before.split("\n")
-  const b = after.split("\n")
-  const lines: DiffLine[] = []
-
-  // Simple LCS-based diff
-  const m = a.length
-  const n = b.length
-  const dp: number[][] = Array.from({ length: m + 1 }, () => Array(n + 1).fill(0))
-
-  for (let i = 1; i <= m; i++) {
-    for (let j = 1; j <= n; j++) {
-      dp[i][j] = a[i - 1] === b[j - 1] ? dp[i - 1][j - 1] + 1 : Math.max(dp[i - 1][j], dp[i][j - 1])
-    }
-  }
-
-  // Backtrack
-  const result: DiffLine[] = []
-  let i = m
-  let j = n
-  while (i > 0 || j > 0) {
-    if (i > 0 && j > 0 && a[i - 1] === b[j - 1]) {
-      result.push({ type: "context", text: a[i - 1] })
-      i--
-      j--
-    } else if (j > 0 && (i === 0 || dp[i][j - 1] >= dp[i - 1][j])) {
-      result.push({ type: "add", text: b[j - 1] })
-      j--
-    } else {
-      result.push({ type: "remove", text: a[i - 1] })
-      i--
-    }
-  }
-
-  result.reverse()
-
-  // Collapse long context runs (show max 3 context lines between changes)
-  const collapsed: DiffLine[] = []
-  let contextRun = 0
-  for (const line of result) {
-    if (line.type === "context") {
-      contextRun++
-      if (contextRun <= 3) {
-        collapsed.push(line)
-      } else if (contextRun === 4) {
-        collapsed.push({ type: "context", text: "..." })
-      }
-    } else {
-      contextRun = 0
-      collapsed.push(line)
-    }
-  }
-
-  return collapsed
-}
-
 export function DiffView({ before, after, isDark }: Props) {
   const lines = computeDiff(before, after)
 

src/components/chat/MessageBubble.tsx

@@ -113,17 +113,19 @@ export const MessageBubble = memo(
   },
   (prev, next) => {
     // Only re-render if message content actually changed
-    // This prevents completed messages from re-rendering during streaming
+    // This prevents completed messages from re-rendering during streaming.
-    if (prev.message.id !== next.message.id) return false
+    // The store replaces changed parts/messages with NEW object references,
+    // so a reference-equality sweep over every part catches every real change
+    // (including tool parts, which have no `.text`) while still skipping
+    // unchanged (completed) messages during other messages' streaming.
+    if (prev.message !== next.message) return false
     if (prev.isDark !== next.isDark) return false
     if (prev.onLongPress !== next.onLongPress) return false
     if (prev.parts.length !== next.parts.length) return false
-    // Compare the last part's text content - this is what changes during streaming
+    for (let i = 0; i < prev.parts.length; i++) {
-    const prevLast = prev.parts[prev.parts.length - 1]
+      if (prev.parts[i] !== next.parts[i]) return false
-    const nextLast = next.parts[next.parts.length - 1]
+    }
-    if (!prevLast && !nextLast) return true
+    return true
-    if (!prevLast || !nextLast) return false
-    return prevLast.type === nextLast.type && prevLast.text === nextLast.text
   },
 )
 

src/components/chat/diff-compute.test.ts

@@ -0,0 +1,54 @@
+import { test } from "node:test"
+import assert from "node:assert/strict"
+import { computeDiff } from "./diff-compute.ts"
+
+// GitHub bug: computeDiff split on a literal "\n", so a CRLF `before` diffed
+// against an LF `after` treated every line as changed (each "line\r" !==
+// "line") even when only one line actually changed. Normalizing both sides
+// with /\r?\n/ before diffing fixes this.
+test("computeDiff normalizes CRLF vs LF so only the actually-changed line diffs", () => {
+  const before = "line1\r\nline2\r\nline3"
+  const after = "line1\nCHANGED\nline3"
+
+  const result = computeDiff(before, after)
+
+  const removed = result.filter((l) => l.type === "remove")
+  const added = result.filter((l) => l.type === "add")
+
+  assert.equal(removed.length, 1, "expected exactly one removed line, not a whole-file diff")
+  assert.equal(added.length, 1, "expected exactly one added line, not a whole-file diff")
+  assert.equal(removed[0]?.text, "line2")
+  assert.equal(added[0]?.text, "CHANGED")
+
+  // Unchanged lines must still show up as context, proving they matched
+  // across the CRLF/LF boundary instead of being treated as changed.
+  const contextTexts = result.filter((l) => l.type === "context").map((l) => l.text)
+  assert.ok(contextTexts.includes("line1"))
+  assert.ok(contextTexts.includes("line3"))
+})
+
+// GitHub bug: the unbounded O(a.length * b.length) LCS table + backtrack is
+// an OOM/ANR risk for large diffs. Above the size guard, computeDiff must
+// skip the DP table and fall back to a truncated remove/add rendering
+// instead of hanging or building a huge table.
+test("computeDiff falls back to a truncated diff for huge inputs instead of hanging", () => {
+  const lineCount = 2000
+  const before = Array.from({ length: lineCount }, (_, i) => `before line ${i}`).join("\n")
+  const after = Array.from({ length: lineCount }, (_, i) => `after line ${i}`).join("\n")
+
+  const start = Date.now()
+  const result = computeDiff(before, after)
+  const elapsedMs = Date.now() - start
+
+  // Generous bound: a correct truncated fallback is near-instant; a
+  // regression back to the unbounded O(n*m) table over 2000x2000 lines
+  // would take drastically longer than this.
+  assert.ok(elapsedMs < 2000, `computeDiff took ${elapsedMs}ms, expected a fast truncated fallback`)
+
+  // Truncated to at most 400 lines per side plus one synthetic marker line.
+  assert.ok(result.length <= 801, `expected truncated output, got ${result.length} lines`)
+
+  const last = result[result.length - 1]
+  assert.equal(last?.type, "context")
+  assert.match(last?.text ?? "", /diff too large to display in full/)
+})

src/components/chat/diff-compute.ts

@@ -0,0 +1,101 @@
+// Pure (no React Native imports) diff computation shared by
+// src/components/chat/DiffView.tsx. Kept in its own plain module — same
+// pattern as src/lib/scroll-config.ts — so it can be unit-tested with
+// node:test (no react-test-renderer needed) while DiffView imports the same
+// runtime logic it renders, so the test and the real component can't drift
+// apart.
+
+export interface DiffLine {
+  type: "add" | "remove" | "context"
+  text: string
+}
+
+// Above this size the O(a.length * b.length) LCS table (and the matching
+// backtrack array) becomes an OOM/ANR risk on-device — a 2000-line file both
+// sides is a 4,000,000-cell table. Guard on both a hard per-side line count
+// and the product so two moderately sized files can't multiply into a huge
+// table either.
+const MAX_DIFF_LINES = 800
+const MAX_DIFF_CELLS = 250_000
+// Even a "normal" LCS diff can produce thousands of rendered native rows for
+// a large file with few matching lines (e.g. a full rewrite). Cap the final
+// rendered line count so DiffView can never mount an unbounded number of
+// <View>/<Text> rows.
+const MAX_RENDERED_LINES = 600
+const TRUNCATED_SIDE_LINES = 400
+
+function truncationMarker(totalLines: number): DiffLine {
+  return {
+    type: "context",
+    text: `… diff too large to display in full (${totalLines} lines) — view on your computer`,
+  }
+}
+
+export function computeDiff(before: string, after: string): DiffLine[] {
+  // Normalize line endings so a CRLF/LF mismatch doesn't make every line
+  // look changed.
+  const a = before.split(/\r?\n/)
+  const b = after.split(/\r?\n/)
+
+  // Fallback path for huge inputs: skip the O(a.length * b.length) LCS table
+  // entirely and render a simple, truncated remove-then-add diff.
+  if (a.length > MAX_DIFF_LINES || b.length > MAX_DIFF_LINES || a.length * b.length > MAX_DIFF_CELLS) {
+    const removed = a.slice(0, TRUNCATED_SIDE_LINES).map((text) => ({ type: "remove" as const, text }))
+    const added = b.slice(0, TRUNCATED_SIDE_LINES).map((text) => ({ type: "add" as const, text }))
+    return [...removed, ...added, truncationMarker(a.length + b.length)]
+  }
+
+  // Simple LCS-based diff
+  const m = a.length
+  const n = b.length
+  const dp: number[][] = Array.from({ length: m + 1 }, () => Array(n + 1).fill(0))
+
+  for (let i = 1; i <= m; i++) {
+    for (let j = 1; j <= n; j++) {
+      dp[i][j] = a[i - 1] === b[j - 1] ? dp[i - 1][j - 1] + 1 : Math.max(dp[i - 1][j], dp[i][j - 1])
+    }
+  }
+
+  // Backtrack
+  const result: DiffLine[] = []
+  let i = m
+  let j = n
+  while (i > 0 || j > 0) {
+    if (i > 0 && j > 0 && a[i - 1] === b[j - 1]) {
+      result.push({ type: "context", text: a[i - 1] })
+      i--
+      j--
+    } else if (j > 0 && (i === 0 || dp[i][j - 1] >= dp[i - 1][j])) {
+      result.push({ type: "add", text: b[j - 1] })
+      j--
+    } else {
+      result.push({ type: "remove", text: a[i - 1] })
+      i--
+    }
+  }
+
+  result.reverse()
+
+  // Collapse long context runs (show max 3 context lines between changes)
+  const collapsed: DiffLine[] = []
+  let contextRun = 0
+  for (const line of result) {
+    if (line.type === "context") {
+      contextRun++
+      if (contextRun <= 3) {
+        collapsed.push(line)
+      } else if (contextRun === 4) {
+        collapsed.push({ type: "context", text: "..." })
+      }
+    } else {
+      contextRun = 0
+      collapsed.push(line)
+    }
+  }
+
+  if (collapsed.length > MAX_RENDERED_LINES) {
+    return [...collapsed.slice(0, MAX_RENDERED_LINES), truncationMarker(collapsed.length)]
+  }
+
+  return collapsed
+}

src/components/markdown/Markdown.tsx

@@ -1,4 +1,4 @@
-import type { ReactNode } from "react"
+import { useMemo, type ReactNode } from "react"
 import { View, Text, useColorScheme, Platform, type StyleProp, type ViewStyle, type TextStyle } from "react-native"
 import { useMarkdown, Renderer } from "react-native-marked"
 import { CodeBlock } from "./CodeBlock"
@@ -56,16 +56,14 @@ class CustomRenderer extends Renderer {
   }
 }
 
-const renderer = new CustomRenderer()
-
 const mono = Platform.OS === "ios" ? "Menlo" : "monospace"
 
 const lightTheme = {
   text: { color: "#0a0a0a", fontSize: 15, lineHeight: 22 },
   paragraph: { marginTop: 0, marginBottom: 8 },
-  heading1: { fontSize: 22, fontWeight: "700" as const, color: "#0a0a0a", marginBottom: 8, marginTop: 12 },
+  h1: { fontSize: 22, fontWeight: "700" as const, color: "#0a0a0a", marginBottom: 8, marginTop: 12 },
-  heading2: { fontSize: 19, fontWeight: "600" as const, color: "#0a0a0a", marginBottom: 6, marginTop: 10 },
+  h2: { fontSize: 19, fontWeight: "600" as const, color: "#0a0a0a", marginBottom: 6, marginTop: 10 },
-  heading3: { fontSize: 16, fontWeight: "600" as const, color: "#0a0a0a", marginBottom: 4, marginTop: 8 },
+  h3: { fontSize: 16, fontWeight: "600" as const, color: "#0a0a0a", marginBottom: 4, marginTop: 8 },
   link: { color: "#8b5cf6" },
   blockquote: {
     backgroundColor: "transparent",
@@ -94,7 +92,7 @@ const lightTheme = {
     borderRadius: 3,
   },
   list: { marginBottom: 4 },
-  listItem: { marginBottom: 2 },
+  li: { marginBottom: 2 },
   hr: { backgroundColor: "#e5e5e5", height: 1, marginVertical: 12 },
   strong: { fontWeight: "700" as const },
   em: { fontStyle: "italic" as const },
@@ -105,9 +103,9 @@ const lightTheme = {
 const darkTheme = {
   ...lightTheme,
   text: { ...lightTheme.text, color: "#e5e5e5" },
-  heading1: { ...lightTheme.heading1, color: "#ffffff" },
+  h1: { ...lightTheme.h1, color: "#ffffff" },
-  heading2: { ...lightTheme.heading2, color: "#ffffff" },
+  h2: { ...lightTheme.h2, color: "#ffffff" },
-  heading3: { ...lightTheme.heading3, color: "#ffffff" },
+  h3: { ...lightTheme.h3, color: "#ffffff" },
   link: { color: "#a78bfa" },
   blockquote: {
     ...lightTheme.blockquote,
@@ -134,6 +132,17 @@ export function Markdown({ children }: Props) {
   const isDark = useColorScheme() === "dark"
   const theme = isDark ? darkTheme : lightTheme
 
+  // A module-scope singleton renderer would share one CustomRenderer (and
+  // its underlying github-slugger) across every Markdown instance and every
+  // streamed token forever. github-slugger never resets, so its heading-slug
+  // keys only ever climb — which fed into useMarkdown's memoized parser and
+  // made the emitted React keys change on every token, remounting the whole
+  // subtree (resetting code-block scroll position, flashing content). Scoping
+  // the renderer to `children` resets the slugger per parse, so keys are
+  // deterministic (and stable) for a given value, while re-renders with an
+  // unchanged value stay memoized instead of creating a new renderer.
+  const renderer = useMemo(() => new CustomRenderer(), [children])
+
   // react-native-marked's default <RNMarkdown> export renders blocks inside a
   // FlatList. Chat messages are rendered inside app/session/[id].tsx's own
   // *inverted* FlatList (each row a MessageBubble) — nesting one