perf: send only tail diff of streaming response to renderer

[RyanGroch]

Replace the full streamingContent IPC payload with a streamingPatch of {offset, content} — the renderer reconstructs as current.slice(0, offset) + content. cleanFullResponse may rewrite earlier bytes inside in-progress dyad-tag attribute values, so the sender computes offset as the longest common prefix length rather than assuming pure appends.

Also memoize the per-piece markdown and custom-tag renderers so completed segments skip ReactMarkdown's re-parse on every chunk.

[wwwillchen]

@BugBot run

[dyad-assistant]

🔍 Code Review Summary (Codex)

Verdict: ✅ YES - Ready to merge
Recommendation: ready

✅ No significant issues found.

---

Generated by Codex

[gemini-code-assist]

Code Review

This pull request optimizes chat streaming performance by reducing IPC overhead and React rendering costs. It introduces a tail-patching mechanism for streaming updates, where only the changed portion of a message is sent to the renderer based on the longest common prefix. Additionally, it implements memoization for markdown and custom tag components in the UI to prevent redundant parsing and rendering of completed message segments during active streams. I have no feedback to provide as there were no review comments to evaluate.

[cubic-dev-ai]

No issues found across 7 files

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0d953a5464

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[dyad-assistant]

🔍 Dyadbot Code Review Summary

Verdict: 🤔 NOT SURE - Potential issues

Reviewed by 3 independent agents: Correctness Expert, Code Health Expert, UX Wizard.

Issues Summary

Severity	File	Issue
🟡 MEDIUM	src/hooks/useStreamChat.ts (+ 2 others)	Patch-application logic copy-pasted across 3 hooks
🟡 MEDIUM	src/ipc/handlers/chat_stream_handlers.ts + local_agent_handler.ts	LCP computation duplicated across 2 backend handlers
🟡 MEDIUM	rules/electron-ipc.md	Documentation still describes old two-mode protocol

🟢 Low Priority Notes (3 items)

• streamingContent handlers are now dead code - src/hooks/useStreamChat.ts:259, usePlanImplementation.ts:133, useResolveMergeConflictsWithAI.ts:142 — No sender emits streamingContent anymore; all three else if (streamingContent \!== undefined) branches are unreachable. If intentionally kept for transition, add a comment.
• MemoMarkdown duplicates VanillaMarkdownParser - src/components/chat/DyadMarkdownParser.tsx:223 — These render identical JSX. Consider const MemoMarkdown = React.memo(VanillaMarkdownParser).
• Inline object literals in MemoMarkdown - src/components/chat/DyadMarkdownParser.tsx:228 — remarkPlugins={[remarkGfm]} and components={{...}} create fresh refs on each render. Hoisting to module-level constants would complete the memoization optimization.

🚫 Dropped False Positives (2 items)

• lastSentRef desync after pre-streaming compaction — Dropped: At line 515, fullResponse="" so lastSentRef.value="" matches the renderer's empty placeholder content after the direct safeSend at line 518. No desync occurs.
• sendFullMessages path sets lastSentRef but renderer may differ — Dropped: Too theoretical; mitigated by all code paths coordinating through the same fullResponse variable.

---

Generated by Dyadbot multi-agent code review

[dyad-assistant]

Multi-agent review: 3 MEDIUM issues found

[wwwillchen]

@BugBot run

[dyad-assistant]

🔍 Code Review Summary (Codex)

Verdict: ✅ YES - Ready to merge
Recommendation: ready

✅ No significant issues found.

---

Generated by Codex

[dyad-assistant]

🔍 Dyadbot Code Review Summary

Verdict: ✅ YES - Ready to merge

Reviewed by 3 independent agents: Correctness Expert, Code Health Expert, UX Wizard.

Issues Summary

Severity	File	Issue
🟡 MEDIUM	src/hooks/useStreamChat.ts:258 (+ 2 hooks)	streamingContent handlers are dead code — no sender emits it

🟢 Low Priority Notes (5 items)

• tagInfoEqual skips fullMatch without comment - src/components/chat/DyadMarkdownParser.tsx:241 — renderCustomTag doesn't use fullMatch, so the omission is correct, but a brief comment would prevent future contributors from "fixing" it.
• computeStreamingPatch return type duplicates StreamingPatch - src/ipc/utils/stream_text_utils.ts:9 — The return type { offset: number; content: string } | null is the same shape as the exported StreamingPatch type from chat.ts. Same for applyStreamingPatch's parameter. Using the canonical type would keep things consistent.
• Inline remarkPlugins/components in MemoMarkdown - src/components/chat/DyadMarkdownParser.tsx:229 — These create fresh refs on each render of MemoMarkdown. Hoisting to module-level constants would complete the memoization optimization.
• MemoMarkdown duplicates VanillaMarkdownParser - src/components/chat/DyadMarkdownParser.tsx:223 — These render identical JSX. Consider const MemoMarkdown = React.memo(VanillaMarkdownParser).
• Index-based keys on contentPieces - src/components/chat/DyadMarkdownParser.tsx:195 — Pre-existing pattern, but now that memoization is in play, piece shifts during streaming could cause unnecessary unmount/remount cycles. Content-based keys would be more robust.

🚫 Dropped False Positives (5 items)

• Unnecessary state update when streamingMessageId not found — Dropped: Pre-existing pattern in the streamingContent handler above it. Not a regression introduced by this PR.
• No validation that offset is non-negative in schema — Dropped: Internal IPC within same Electron bundle, not a trust boundary. computeStreamingPatch guarantees valid non-negative output.
• Full-messages send mutates chat.messages in place — Dropped: Pre-existing behavior (placeholderMsg.content = fullResponse) not introduced by this PR.
• Silent content corruption if renderer state drifts — Dropped: Drift scenarios (page reload, error boundary recovery) would require re-establishing the entire stream. Periodic full-message sends and the initial baseline provide natural recovery points.
• MemoCustomTag Fragment wrapper overhead — Dropped: Negligible performance impact, may be needed for JSX return type compatibility.

---

Generated by Dyadbot multi-agent code review

[dyad-assistant]

Multi-agent review: 1 issue found

[wwwillchen]

@BugBot run

[dyad-assistant]

🔍 Code Review Summary (Codex)

Verdict: ✅ YES - Ready to merge
Recommendation: ready

✅ No significant issues found.

---

Generated by Codex

[dyad-assistant]

Multi-agent review: 2 issue(s) found

[dyad-assistant]

��� Dyadbot Code Review Summary

Verdict: ✅ YES - Ready to merge

Reviewed by 3 independent agents: Correctness Expert, Code Health Expert, UX Wizard.

Issues Summary

Severity	File	Issue
🟡 MEDIUM	rules/electron-ipc.md:118	Doc says "three modes" but only lists two
🟡 MEDIUM	local_agent_handler.ts:533	Bare safeSend bypasses sendResponseChunk's lastSentRef encapsulation

🟢 Low Priority Notes (4 items)

• Schema/rules mode count mismatch — src/ipc/types/chat.ts:122 says "Supports two modes" while rules/electron-ipc.md:118 says "three modes". Align after fixing the rules doc.
• sendResponseChunk now takes 8 positional parameters — local_agent_handler.ts:1624 — Consider grouping into an options object for self-documenting call sites.
• applyStreamingPatch uses inline type instead of StreamingPatch — src/lib/applyStreamingPatch.ts:13 — Could import the canonical StreamingPatch type from @/ipc/types/chat.
• MemoCustomTag re-renders all tags on isStreaming transition — DyadMarkdownParser.tsx:264 — Already-completed tags (inProgress: false) don't depend on isStreaming, but re-render anyway when streaming ends. One-time cost, not per-chunk.

🚫 Dropped False Positives (5 items)

• No unit tests for computeStreamingPatch — Dropped: Valid concern but the function logic is straightforward (LCP + slice). Process concern, not a code bug. Worth adding in a follow-up.
• Patch offset exceeding content length produces garbage — Dropped: Theoretical; sender and renderer are in the same Electron bundle with synchronous IPC, so desync can't occur via message loss.
• VanillaMarkdownParser still creates fresh prop refs — Dropped: Pre-existing code not changed by this PR. Not in the streaming hot path.
• Unnecessary Map allocation when streamingMessageId not found — Dropped: Over-optimization for a case that shouldn't occur in normal operation (placeholder is established before patches arrive).
• streamTestResponse uses different protocol than production — Dropped: Out of scope for this PR. Test code using a simpler protocol is acceptable.

---

Generated by Dyadbot multi-agent code review

[wwwillchen]

@BugBot run

[dyad-assistant]

🔍 Code Review Summary (Codex)

Verdict: ✅ YES - Ready to merge
Recommendation: ready

✅ No significant issues found.

---

Generated by Codex

[dyad-assistant]

🔍 Dyadbot Code Review Summary

Verdict: ✅ YES - Ready to merge

Reviewed by 3 independent agents: Correctness Expert, Code Health Expert, UX Wizard.

Issues Summary

✅ No HIGH or MEDIUM issues found.

🟢 Low Priority Notes (1 item)

• Return type should use StreamingPatch named type — src/ipc/utils/stream_text_utils.ts:9-12 — computeStreamingPatch returns { offset: number; content: string } | null (anonymous type) instead of StreamingPatch | null. Using the named type would enforce coupling with the schema and surface any future changes as compile errors.

🚫 Dropped False Positives (13 items)

• Dropped IPC message permanently corrupts stream — Dropped: Electron IPC is a reliable same-process V8 bridge, not a lossy network protocol. Messages don't drop or arrive out of order.
• Initial full-messages send bypasses lastSentContent tracking — Dropped: References code outside the diff; lastSentContent="" initialization matches the placeholder's initial content "".
• Post-stream full-messages send bypasses tracking — Dropped: References code outside the diff; no subsequent patches are sent after this point.
• Array index as React key defeats memoization — Dropped: Pre-existing pattern not introduced by this PR. During streaming, pieces are always appended at the tail, so index keys are stable for completed pieces.
• inProgress type allows undefined — Dropped: parseCustomTags always sets inProgress to a boolean; theoretical concern with no runtime impact.
• Pattern inconsistency between handlers — Dropped: Already covered by existing comment Query | Question about model availability via OpenRouter #3 (LCP extraction). Remaining structural difference is justified — chat_stream_handlers.ts has one call site vs many in local_agent_handler.ts.
• fullMatch field never read — Dropped: Pre-existing dead field outside PR scope.
• VanillaMarkdownParser creates fresh arrays — Dropped: Pre-existing pattern in unchanged code.
• Comparator needs more explanation — Dropped: Comment already references getState and "finished" return value — adequate explanation.
• Argument order unintuitive — Dropped: Very minor style preference; both call sites pass correctly and naming is clear.
• LCP may split surrogate pairs — Dropped: When LCP stops between a pair, the high surrogate matched in both strings, so recombination always produces a valid UTF-16 pair.
• Stale pending state on cancellation — Dropped: Standard React re-render behavior propagates the new isStreaming value through the memo gate correctly.
• No guard on out-of-range offset — Dropped: JS String.slice auto-clamps to string bounds, and Electron IPC can't drop messages.

---

Generated by Dyadbot multi-agent code review

[github-actions]

🎭 Playwright Test Results

❌ Some tests failed

OS	Passed	Failed	Flaky	Skipped
🍎 macOS	290	1	3	6

Summary: 290 passed, 1 failed, 3 flaky, 6 skipped

Failed Tests

🍎 macOS

• custom_apps_folder.spec.ts > custom folder change doesn't make apps inaccessible
  • Error: expect(received).toBe(expected) // Object.is equality

📋 Re-run Failing Tests (macOS)

Copy and paste to re-run all failing spec files locally:

npm run e2e \
  e2e-tests/custom_apps_folder.spec.ts

⚠️ Flaky Tests

🍎 macOS

• cloud_sandbox.spec.ts > cloud sandbox undo restores the remote snapshot (passed after 1 retry)
• custom_apps_folder.spec.ts > new apps are stored in the user's custom folder (passed after 1 retry)
• local_agent_file_upload.spec.ts > local-agent - upload file to codebase (passed after 1 retry)

---

📊 View full report