diff --git a/src/components/chat/DiffView.tsx b/src/components/chat/DiffView.tsx index eeeaad7..00c66c0 100644 --- a/src/components/chat/DiffView.tsx +++ b/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) diff --git a/src/components/chat/MessageBubble.tsx b/src/components/chat/MessageBubble.tsx index b3ee15e..0f82eca 100644 --- a/src/components/chat/MessageBubble.tsx +++ b/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 - if (prev.message.id !== next.message.id) return false + // This prevents completed messages from re-rendering during streaming. + // 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 - const prevLast = prev.parts[prev.parts.length - 1] - const nextLast = next.parts[next.parts.length - 1] - if (!prevLast && !nextLast) return true - if (!prevLast || !nextLast) return false - return prevLast.type === nextLast.type && prevLast.text === nextLast.text + for (let i = 0; i < prev.parts.length; i++) { + if (prev.parts[i] !== next.parts[i]) return false + } + return true }, ) diff --git a/src/components/chat/diff-compute.test.ts b/src/components/chat/diff-compute.test.ts new file mode 100644 index 0000000..5e9e8b0 --- /dev/null +++ b/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/) +}) diff --git a/src/components/chat/diff-compute.ts b/src/components/chat/diff-compute.ts new file mode 100644 index 0000000..321115f --- /dev/null +++ b/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 +// / 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 +} diff --git a/src/components/markdown/Markdown.tsx b/src/components/markdown/Markdown.tsx index 5360a5c..1066ab0 100644 --- a/src/components/markdown/Markdown.tsx +++ b/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 }, - heading2: { 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 }, + h1: { fontSize: 22, fontWeight: "700" as const, color: "#0a0a0a", marginBottom: 8, marginTop: 12 }, + h2: { fontSize: 19, fontWeight: "600" as const, color: "#0a0a0a", marginBottom: 6, marginTop: 10 }, + 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" }, - heading2: { ...lightTheme.heading2, color: "#ffffff" }, - heading3: { ...lightTheme.heading3, color: "#ffffff" }, + h1: { ...lightTheme.h1, color: "#ffffff" }, + h2: { ...lightTheme.h2, 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 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