fix(chat): cut display polling to 4fps and restore query cache defaults

[Kitenite]

Summary

Alternative, subtractive version of #3170 by @thepathmakerz. Same root cause, same bug closed, ~-21 lines instead of +36.

Credit: all diagnostic work and the root-cause analysis belongs to @thepathmakerz in #3170. This PR just takes a more aggressive deletion path — please review alongside that PR and pick whichever fits better.

Root cause (same as #3170)

staleTime: 0, gcTime: 0 combined with 60fps polling (fps = 60 default, 16ms refetch) means React Query cannot dedupe or garbage-collect anything — it allocates and discards a fresh query cache entry ~60 times a second, forever. Over an hour that drifts into multi-GB heap and a CPU death spiral.

This version

• Remove staleTime: 0, gcTime: 0 from both hooks → React Query defaults (gcTime: 5min) kick back in. This is what fixes the leak.
• Default fps = 60 → fps = 4 (250ms poll) → kills the CPU spiral at the source.
• Delete the v2 hook's isRunning invalidation useEffect → redundant when the query is polling anyway; also drops now-unused utils.

No adaptive polling, no wasRunningRef, no function-form refetchInterval, no IDLE_* / GC_TIME_* constants.

Why 4fps doesn't hurt streaming UX

StreamingMessageText (apps/desktop/.../StreamingMessageText.tsx) already animates text reveal client-side at its own 16ms interval (STREAM_TEXT_TICK_MS), revealing 2 chars/tick from whatever buffer the server has delivered. Perceived smoothness is decoupled from the server poll rate — the poll just needs to keep the reveal buffer fed, which 4fps does with plenty of headroom vs. typical LLM token arrival (~50–200ms chunks).

Tradeoffs vs #3170

• Gives up: peak 60fps active-streaming cadence and the immediate refetch on agent stop. Both unnecessary given the client-side reveal animation and the ≤250ms natural catch-up.
• Gains: fewer moving parts (no dual cadence, no ref-based transition detection), no asymmetry between displayQuery's function-form refetchInterval and messagesQuery's static one, no cross-session wasRunningRef edge case.

Test plan

• bun run --filter @superset/chat typecheck passes
• bun run --filter @superset/desktop typecheck passes
• bun test src/client/hooks/use-chat-display — 5/5 pass
• Manual: 30+ min idle with a workspace open — verify flat heap in DevTools Memory
• Manual: send a message, confirm streaming reveal still looks smooth (client-side animation drives this)
• Manual: after agent stops, confirm final state appears within ~250ms

Closes #3049

---

Summary by cubic

Fixes chat memory leak and CPU spikes by restoring React Query cache defaults and cutting display polling to 4fps. Streaming stays smooth because the client animates text reveal.

• Bug Fixes
  • Removed staleTime: 0 and gcTime: 0 so the default 5‑minute cache GC works.
  • Set fps default from 60 to 4 (250ms poll) in both chat display hooks.
  • Deleted the redundant isRunning invalidation effect in the desktop workspace hook.

^{Written for commit 16356b7. Summary will update on new commits.}

Summary by CodeRabbit

• Chores
  • Optimized chat display refresh behavior to reduce polling frequency while maintaining message synchronization. These internal improvements help reduce unnecessary data requests and improve overall performance.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes reduce the chat display polling frequency from 60fps to 4fps, remove redundant cache invalidation logic, and adjust React Query cache configuration. These modifications address excessive refetching and memory churn that contributed to a renderer process memory leak.

Changes

Cohort / File(s)	Summary
Chat Display Polling & Caching apps/desktop/src/renderer/routes/.../ChatPane/hooks/useWorkspaceChatDisplay/useWorkspaceChatDisplay.ts, packages/chat/src/client/hooks/use-chat-display/use-chat-display.ts	Reduced default fps from 60 to 4, removed automatic cache invalidation effect for running queries, and eliminated aggressive staleTime: 0 and gcTime: 0 options to reduce polling frequency, cache churn, and garbage collection pressure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hop, hop, slow down the poll,
Four frames now control the scroll,
No more cache thrashing tight,
Memory breathes, the heap feels light,
V8's death spiral—now out of sight! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: reducing polling from 60fps to 4fps and restoring React Query cache defaults to fix the memory leak.
Description check	✅ Passed	The PR description is comprehensive, covering root cause, solution, rationale, tradeoffs, and test plan. It follows the template structure with clear sections.
Linked Issues check	✅ Passed	All code changes directly address issue #3049: removing staleTime/gcTime restores React Query garbage collection, fps reduction cuts CPU load, and deleting redundant effects eliminates unnecessary allocation.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the memory leak described in #3049. No unrelated modifications to other systems or features are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/chat-display-polling-memory-leak-minimal

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR fixes a memory leak and CPU spiral caused by staleTime: 0, gcTime: 0 combined with 60fps polling in the chat display hooks. By restoring React Query's default gcTime (5 min) and reducing the poll rate to 4fps (250ms), the fix prevents React Query from allocating and immediately discarding ~60 cache entries per second, which previously accumulated into multi-GB heap growth over extended sessions.

Key changes:

• fps default changed from 60 → 4 in both useChatDisplay hooks, reducing the poll interval from 16ms to 250ms.
• staleTime: 0, gcTime: 0 removed from both hooks' queryOptions, restoring React Query's built-in deduplication and garbage collection — this is the primary fix for the memory leak.
• Redundant isRunning invalidation useEffect removed from the v2 workspace hook; with 250ms polling already in place, immediate invalidation on run-state change is unnecessary.
• utils (tRPC utils) cleaned up from the v2 hook since it was only used by the now-deleted useEffect, and correctly retained in use-chat-display.ts where it is still needed for all command mutations.

Confidence Score: 5/5

Safe to merge — targeted, minimal fix with no regressions introduced.

The two changes (restoring gcTime and lowering fps) are both correct and complementary. The utils cleanup is accurate: removed where no longer needed, retained where still used for mutations. The deleted useEffect was genuinely redundant given the polling regime. No logic is restructured and no new surface area is introduced. The only note is a pre-existing minor inconsistency in the toRefetchIntervalMs fallback path that does not affect normal operation.

No files require special attention.

Important Files Changed

Filename	Overview
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/hooks/useWorkspaceChatDisplay/useWorkspaceChatDisplay.ts	v2 workspace hook: fps default 60→4, staleTime/gcTime removed, redundant isRunning invalidation useEffect deleted, and now-unused utils import cleaned up — all correct.
packages/chat/src/client/hooks/use-chat-display/use-chat-display.ts	Chat package hook: fps default 60→4, staleTime/gcTime removed; utils is correctly retained as it is still needed for all command mutations (sendMessage, stop, abort, respondTo*).

Sequence Diagram

sequenceDiagram
    participant UI as React Component
    participant Hook as useChatDisplay (fps=4)
    participant RQ as React Query Cache (gcTime=5min)
    participant Server as tRPC Server

    Note over UI,Server: Before fix: gcTime=0, fps=60 → 60 alloc+discard/sec
    Note over UI,Server: After fix: gcTime=5min, fps=4 → stable cache, 250ms poll

    loop Every 250ms (4fps)
        Hook->>RQ: refetchInterval fires
        RQ->>Server: GET getDisplayState
        Server-->>RQ: response cached (gcTime=5min)
        RQ-->>Hook: data (staleTime=0, refetch queued)
        Hook-->>UI: updated displayState + messages
    end

    UI->>Hook: sendMessage()
    Hook->>Server: mutation via useMutation
    Server-->>Hook: ack
    Note over Hook,RQ: Next 250ms poll picks up new state (no manual invalidation needed)

Loading

Comments Outside Diff (1)

1. apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/hooks/useWorkspaceChatDisplay/useWorkspaceChatDisplay.ts, line 13-16 (link)

   toRefetchIntervalMs fallback still targets 60fps

   This is pre-existing code, but now that the default has moved to 4fps, the invalid-input fallback (!Number.isFinite(fps) || fps <= 0 → 16ms) is conspicuously inconsistent. A caller who accidentally passes fps = 0 or fps = Infinity will silently receive 60fps polling (16ms) — the exact rate this PR is moving away from — rather than falling back to the new default of 4fps.

   Consider aligning the fallback with the new intent:

   Or simply default to 250ms directly. This wouldn't affect normal usage (the default fps = 4 is always valid), but it closes the gap for misconfigured callers.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/hooks/useWorkspaceChatDisplay/useWorkspaceChatDisplay.ts
   Line: 13-16
   
   Comment:
   **`toRefetchIntervalMs` fallback still targets 60fps**
   
   This is pre-existing code, but now that the default has moved to 4fps, the invalid-input fallback (`!Number.isFinite(fps) || fps <= 0 → 16ms`) is conspicuously inconsistent. A caller who accidentally passes `fps = 0` or `fps = Infinity` will silently receive 60fps polling (16ms) — the exact rate this PR is moving away from — rather than falling back to the new default of 4fps.
   
   Consider aligning the fallback with the new intent:
   
   
   
   Or simply default to 250ms directly. This wouldn't affect normal usage (the default `fps = 4` is always valid), but it closes the gap for misconfigured callers.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/hooks/useWorkspaceChatDisplay/useWorkspaceChatDisplay.ts
Line: 13-16

Comment:
**`toRefetchIntervalMs` fallback still targets 60fps**

This is pre-existing code, but now that the default has moved to 4fps, the invalid-input fallback (`!Number.isFinite(fps) || fps <= 0 → 16ms`) is conspicuously inconsistent. A caller who accidentally passes `fps = 0` or `fps = Infinity` will silently receive 60fps polling (16ms) — the exact rate this PR is moving away from — rather than falling back to the new default of 4fps.

Consider aligning the fallback with the new intent:

```suggestion
function toRefetchIntervalMs(fps: number): number {
	if (!Number.isFinite(fps) || fps <= 0) return Math.floor(1000 / 4);
	return Math.max(16, Math.floor(1000 / fps));
}
```

Or simply default to 250ms directly. This wouldn't affect normal usage (the default `fps = 4` is always valid), but it closes the gap for misconfigured callers.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(chat): cut display polling to 4fps a..." | Re-trigger Greptile}

[coderabbitai]

🧹 Nitpick comments (2)

packages/chat/src/client/hooks/use-chat-display/use-chat-display.ts (1)

28-31: Nit: invalid-fps fallback still returns a 60fps interval.

Now that the intended default is 4fps, the invalid/≤0 branch returning Math.floor(1000 / 60) (≈16 ms) is surprising — a malformed input ends up polling ~15× faster than an unspecified one. Consider aligning the fallback with the new default (e.g., 1000 / 4 = 250) or extracting a shared DEFAULT_FPS constant used by both the destructure default and this fallback. Low priority; pre-existing behavior.

♻️ Suggested tweak

+const DEFAULT_FPS = 4;
+
 function toRefetchIntervalMs(fps: number): number {
-	if (!Number.isFinite(fps) || fps <= 0) return Math.floor(1000 / 60);
+	if (!Number.isFinite(fps) || fps <= 0) return Math.floor(1000 / DEFAULT_FPS);
 	return Math.max(16, Math.floor(1000 / fps));
 }

And reuse DEFAULT_FPS in the destructure at line 116.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/chat/src/client/hooks/use-chat-display/use-chat-display.ts` around
lines 28 - 31, The fallback for invalid/≤0 fps in toRefetchIntervalMs currently
returns Math.floor(1000/60) which contradicts the new intended default of 4fps;
introduce a shared DEFAULT_FPS constant (value 4) and use it both in the
destructure default (where fps is set) and inside toRefetchIntervalMs so the
invalid-fps branch returns 1000 / DEFAULT_FPS (or Math.floor/Math.max as
currently applied) to keep behavior consistent across the module.

apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/hooks/useWorkspaceChatDisplay/useWorkspaceChatDisplay.ts (1)

13-16: Optional: de-duplicate with the package hook.

This file now mirrors packages/chat/src/client/hooks/use-chat-display/use-chat-display.ts quite closely — toRefetchIntervalMs, findLastUserMessageIndex, findLatestAssistantErrorMessage, withoutActiveTurnAssistantHistory, hasFileOrImagePart, countFileMessages, and getLegacyImagePayload are essentially duplicated, differing only in the DisplayStateOutput/ListMessagesOutput router source. Not blocking for this PR (out of scope and would widen the diff), but worth a follow-up to hoist the pure helpers into a shared module so future polling/GC tweaks don't have to be applied in two places.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/ChatPane/hooks/useWorkspaceChatDisplay/useWorkspaceChatDisplay.ts
around lines 13 - 16, Multiple pure helper functions (toRefetchIntervalMs,
findLastUserMessageIndex, findLatestAssistantErrorMessage,
withoutActiveTurnAssistantHistory, hasFileOrImagePart, countFileMessages,
getLegacyImagePayload) are duplicated here and in the package hook; extract
these helpers into a shared module and import them from both places. Create a
new utilities file that exports those functions with generic or shared types
(make parameter types accept the common shape used by
DisplayStateOutput/ListMessagesOutput), move the implementations there, update
this file and packages/chat hook to import the helpers instead of redefining
them, and run type checks to ensure the signatures match both call sites.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/ChatPane/hooks/useWorkspaceChatDisplay/useWorkspaceChatDisplay.ts:
- Around line 13-16: Multiple pure helper functions (toRefetchIntervalMs,
findLastUserMessageIndex, findLatestAssistantErrorMessage,
withoutActiveTurnAssistantHistory, hasFileOrImagePart, countFileMessages,
getLegacyImagePayload) are duplicated here and in the package hook; extract
these helpers into a shared module and import them from both places. Create a
new utilities file that exports those functions with generic or shared types
(make parameter types accept the common shape used by
DisplayStateOutput/ListMessagesOutput), move the implementations there, update
this file and packages/chat hook to import the helpers instead of redefining
them, and run type checks to ensure the signatures match both call sites.

In `@packages/chat/src/client/hooks/use-chat-display/use-chat-display.ts`:
- Around line 28-31: The fallback for invalid/≤0 fps in toRefetchIntervalMs
currently returns Math.floor(1000/60) which contradicts the new intended default
of 4fps; introduce a shared DEFAULT_FPS constant (value 4) and use it both in
the destructure default (where fps is set) and inside toRefetchIntervalMs so the
invalid-fps branch returns 1000 / DEFAULT_FPS (or Math.floor/Math.max as
currently applied) to keep behavior consistent across the module.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8769eb7b-1eb0-40ee-802a-b052dca221f0

📥 Commits

Reviewing files that changed from the base of the PR and between 56e6652 and 16356b7.

📒 Files selected for processing (2)

• apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/hooks/useWorkspaceChatDisplay/useWorkspaceChatDisplay.ts
• packages/chat/src/client/hooks/use-chat-display/use-chat-display.ts

[cubic-dev-ai]

No issues found across 2 files

[github-actions]

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

• ✅ Neon database branch
• ✅ Electric Fly.io app

Thank you for your contribution! 🎉