fix(webview): resolve memory leak in ChatView by stabilizing callback props

[samhvw8]

Related GitHub Issue

This will related to grey screen issues which cause by webview memory leak
Closes: #3927

Description

• Stabilize handleSendMessage using clineAskRef to prevent frequent re-creation
• Stabilize toggleRowExpansion by extracting handleSetExpandedRow and managing dependencies
• Re-integrate scrolling logic into useEffect hook to avoid destabilizing callbacks
• Add everVisibleMessagesTsRef to reduce unnecessary ChatRow remounts by Virtuoso
• Update onToggleExpand signature to accept timestamp parameter for better stability
• Remove diagnostic console.log statements used for debugging callback changes

These changes address detached DOM elements memory leak caused by frequent callback re-creation triggering unnecessary component re-renders and preventing proper garbage collection of chat message DOM nodes.

Test Procedure

1. open 3-4 Roo instance (1 can be but has run very long chat conversation) 3-4 or more will make it faster to get grey screen
2. open memory tab in developer console like above image wait till webview > 3gb or more it will crash

Type of Change

• 🐛 Bug Fix: Non-breaking change that fixes an issue.
• ✨ New Feature: Non-breaking change that adds functionality.
• 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
• ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
• 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
• 📚 Documentation: Updates to documentation files.
• ⚙️ Build/CI: Changes to the build process or CI configuration.
• 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

• Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
• Scope: My changes are focused on the linked issue (one major feature/fix per PR).
• Self-Review: I have performed a thorough self-review of my code.
• Code Quality:
  • My code adheres to the project's style guidelines.
  • There are no new linting errors or warnings (npm run lint).
  • All debug code (e.g., console.log) has been removed.
• Testing:
  • New and/or updated tests have been added to cover my changes.
  • All tests pass locally (npm test).
  • The application builds successfully with my changes.
• Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
• Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
• Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
• Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Documentation Updates

Additional Notes

Get in Touch

---

Important

Fix memory leak in ChatView by stabilizing callback props and optimizing component re-renders.

• Behavior:
  • Stabilize handleSendMessage using clineAskRef in ChatView.tsx to prevent frequent re-creation.
  • Stabilize toggleRowExpansion by extracting handleSetExpandedRow and managing dependencies in ChatView.tsx.
  • Re-integrate scrolling logic into useEffect hook in ChatView.tsx to avoid destabilizing callbacks.
  • Add everVisibleMessagesTsRef in ChatView.tsx to reduce unnecessary ChatRow remounts by Virtuoso.
  • Update onToggleExpand signature in ChatRow.tsx to accept timestamp parameter for better stability.
• Misc:
  • Remove diagnostic console.log statements used for debugging callback changes.

^{This description was created by for c5193cc. You can customize this summary. It will automatically update as commits are pushed.}

[xyOz-dev]

Hey, Reviewed the memory implications of this refactor. While the performance improvements are solid, there are a few memory concerns we should address:

Main Issue: The everVisibleMessagesTsRef Set will accumulate message timestamps indefinitely until task change, which could get problematic for long-running tasks.

Recommended fixes:

1. Add Set size limit:

   // Limit the Set to last N visible messages
   const MAX_VISIBLE_MESSAGES_CACHE = 200;
   
   // In the visibleMessages effect:
   newVisibleMessages.forEach((msg) => {
     everVisibleMessagesTsRef.current.add(msg.ts)
     if (everVisibleMessagesTsRef.current.size > MAX_VISIBLE_MESSAGES_CACHE) {
       const oldest = Math.min(...everVisibleMessagesTsRef.current)
       everVisibleMessagesTsRef.current.delete(oldest)
     }
   })

2. Consider WeakSet if possible: If we can restructure to use message objects instead of timestamps, WeakSet would handle cleanup automatically.

3. Add periodic cleanup: Clear entries older than X minutes/hours for really long tasks (not a great idea but may be useful).

4. Alternative approach: Instead of tracking "ever visible", could we just check the current filter logic each time? Might be worth benchmarking if the performance gain justifies the memory cost.

The other refs are fine - minimal overhead and good for performance. But definitely think we should address the growing Set before this possibly gets merged.

In the end our issue is memory, and this does increase memory usage currently resulting in a backwards approach

[samhvw8]

@xyOz-dev is it look good to you ?

[samhvw8]

Thanks for iterating on this, may be annoying but every attempt we can make to minimize memory usage is game-changing for our current gray screening situation.

The un-bounded Set I flagged earlier has been replaced with an LRUCache, which is definitely a step forward, but a few memory / purity nits remain:

1. Cache sizing / lifetime
   • max: 1000 + ttl: 1 h is pretty generous – a long-running chat can still pin ~1 000 timestamps for an hour.
   – i'd suggest max ≈ 250 and ttl ≈ 10-15 min (or whatever feels safe after UX testing).
   • Add cleanup on unmount:

   useEffect(() => () => everVisibleMessagesTsRef.current.clear(), [])

   • Optional: setInterval(() => cache.purgeStale(), 60_000) to keep the heap tidy while the view is open.

2. Inline expand handlers
   We now have ~10 onToggleExpand={() => onToggleExpand(message.ts)} closures per render.
   Minor but easy win:

   const handleExpand = useCallback((ts: number) => onToggleExpand(ts), [onToggleExpand])
   ...
   <ToolRow onToggleExpand={handleExpand} />

Aside from those small tweaks the refactor looks good and removes the old timer leak so i would personally say it would be merge ready post these changes after further review.

i don't understand the 2. Inline expand handlers

onToggleExpand={() => onToggleExpand(message.ts)}

and

const handleExpand = useCallback((ts: number) => onToggleExpand(ts), [onToggleExpand])
onToggleExpand={handleExpand} 

it not the same thing, handleExpand still take an params is ts, but onToggleExpand on ToolRow is not take anything params

[samhvw8]

@daniel-lxs @xyOz-dev updated !, can you guys pls check again

[samhvw8]

@mrubens @xyOz-dev @daniel-lxs updated !

[daniel-lxs]

LGTM, all the feedback was addressed properly.