⚡️(front) throttle chat stream re-renders - #628
Conversation
Signed-off-by: Laurent Paoletti <lp@providenz.fr>
|
WalkthroughThe frontend adds render-loop tracking and React update-depth capture through Sentry, instruments several conversation components, and throttles AI chat stream updates to 50 milliseconds. The changelog documents the stream throttling change. ChangesFrontend diagnostics and stream updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/frontend/apps/conversations/src/utils/debugRenderLoop.ts (2)
146-156: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winSentry context is never cleared, so it can taint unrelated later events.
Sentry.setContext('render_loop', report)(andrender_loop_errorininstallRenderLoopCapture) write onto the current/global scope with no expiry or reset. Once a component crossesWARN_THRESHOLD, every subsequent Sentry event captured for the rest of the session/scope — including ones completely unrelated to a render loop — will carry this stalerender_loopcontext, which can mislead whoever triages those later reports.Suggested fix: clear the context after a short window
const report = { component: name, renders, elapsedMs, busiest: snapshot() }; Sentry.setContext('render_loop', report); Sentry.addBreadcrumb({ category: 'render-loop', level: 'warning', message: `${name} rendered ${renders}x in ${elapsedMs}ms`, data: report, }); + // Avoid tainting unrelated events reported long after the loop resolved. + if (typeof window !== 'undefined') { + window.setTimeout(() => Sentry.setContext('render_loop', null), 5000); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/apps/conversations/src/utils/debugRenderLoop.ts` around lines 146 - 156, Update the render-loop reporting flow around Sentry.setContext('render_loop', report) and the render_loop_error context in installRenderLoopCapture so each context is cleared after a short bounded window, preventing it from attaching to unrelated later events. Preserve the existing report and breadcrumb payloads while ensuring the cleanup applies to both context keys.
55-66: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSynchronous
localStoragereads on the render hot path.
isConsoleEnabled()/isVerbose()calllocalStorage.getItemand are invoked up to 3x pertrackRender(lines 108, 118, 139) plus once perdebugSamplecall — on every render of the instrumented components, including during 20x/s stream updates.localStorageaccess isn't free; doing it synchronously on this exact hot path works against the PR's stated goal of reducing main-thread work during streaming.Consider caching the debug level once (e.g., read at module init, refresh on a
storageevent or a coarse interval) instead of re-reading it on every render/call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/apps/conversations/src/utils/debugRenderLoop.ts` around lines 55 - 66, Cache the value read by debugLevel instead of calling localStorage.getItem on every isConsoleEnabled, isVerbose, trackRender, and debugSample invocation. Initialize the cache once and refresh it only through a storage-event listener or another coarse-grained mechanism, while preserving the current null behavior when localStorage is unavailable.src/frontend/apps/conversations/src/features/chat/components/InputChat.tsx (1)
461-495: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSome tracked props are known to change identity every parent render, diluting the diff signal.
handleSubmit,onStop, andonToggleWebSearchmap tohandleSubmitWrapper,handleStop, andtoggleWebSearchinChat.tsx, which are plain inline functions re-created on everyChatrender (notuseCallback-wrapped);onModelSelect(handleModelSelect) is similarly unmemoized. Because their identity always differs between renders, they'll routinely appear in this component'schangedKeysoutput even when the real cause of a re-render lies elsewhere, burying the signal this tool exists to surface.Since fixing the parent's memoization is out of scope here, consider dropping these known-unstable function props from the tracked context (or noting them as expected-noisy) so the diff output stays focused on values that actually indicate InputChat's own state driving a loop.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/apps/conversations/src/features/chat/components/InputChat.tsx` around lines 461 - 495, Update the diagnostic payload passed to trackRender in InputChat to omit the known-unstable callback props handleSubmit, onStop, onToggleWebSearch, and onModelSelect. Keep the remaining tracked values unchanged so changedKeys focuses on meaningful render differences.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/frontend/apps/conversations/src/utils/debugRenderLoop.ts`:
- Around line 91-133: Update trackRender so render-time calls do not mutate
counters or record Sentry diagnostic state during React Strict Mode double
invocation. Move counting and changed-key processing into an effect-based path,
or gate all counter increments and related state updates behind the opt-in debug
flag before recording diagnostics; preserve verbose logging only when debugging
is enabled.
---
Nitpick comments:
In `@src/frontend/apps/conversations/src/features/chat/components/InputChat.tsx`:
- Around line 461-495: Update the diagnostic payload passed to trackRender in
InputChat to omit the known-unstable callback props handleSubmit, onStop,
onToggleWebSearch, and onModelSelect. Keep the remaining tracked values
unchanged so changedKeys focuses on meaningful render differences.
In `@src/frontend/apps/conversations/src/utils/debugRenderLoop.ts`:
- Around line 146-156: Update the render-loop reporting flow around
Sentry.setContext('render_loop', report) and the render_loop_error context in
installRenderLoopCapture so each context is cleared after a short bounded
window, preventing it from attaching to unrelated later events. Preserve the
existing report and breadcrumb payloads while ensuring the cleanup applies to
both context keys.
- Around line 55-66: Cache the value read by debugLevel instead of calling
localStorage.getItem on every isConsoleEnabled, isVerbose, trackRender, and
debugSample invocation. Initialize the cache once and refresh it only through a
storage-event listener or another coarse-grained mechanism, while preserving the
current null behavior when localStorage is unavailable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 53f396e5-82c4-43e3-80ab-e46f458c70e2
📒 Files selected for processing (11)
CHANGELOG.mdsrc/frontend/apps/conversations/src/core/config/ConfigProvider.tsxsrc/frontend/apps/conversations/src/features/auth/components/Auth.tsxsrc/frontend/apps/conversations/src/features/chat/api/useChat.tsxsrc/frontend/apps/conversations/src/features/chat/components/Chat.tsxsrc/frontend/apps/conversations/src/features/chat/components/InputChat.tsxsrc/frontend/apps/conversations/src/features/left-panel/components/left-panel/LeftPanel.tsxsrc/frontend/apps/conversations/src/layouts/MainLayout.tsxsrc/frontend/apps/conversations/src/pages/_app.tsxsrc/frontend/apps/conversations/src/utils/debugRenderLoop.tssrc/frontend/apps/conversations/src/utils/index.ts
| export const trackRender = ( | ||
| name: string, | ||
| context?: Record<string, unknown>, | ||
| ) => { | ||
| const now = Date.now(); | ||
| let counter = counters.get(name); | ||
|
|
||
| if (!counter || now - counter.windowStart > WINDOW_MS) { | ||
| counter = { count: 0, windowStart: now, warned: false, diffLogs: 0 }; | ||
| counters.set(name, counter); | ||
| } | ||
|
|
||
| counter.count += 1; | ||
| const previousValues = counter.previousValues; | ||
| counter.previousValues = context; | ||
| counter.lastContext = context; | ||
|
|
||
| if (isVerbose()) { | ||
| console.log(`[render-loop] ${name} #${counter.count}`, context ?? ''); | ||
| } | ||
|
|
||
| // Computed even when the console is silent: the key names ride along to | ||
| // Sentry, and they are what identifies the culprit there. | ||
| if (context && previousValues && counter.count >= DIFF_THRESHOLD) { | ||
| const changed = changedKeys(previousValues, context); | ||
| counter.lastChangedKeys = changed; | ||
|
|
||
| if (isConsoleEnabled() && counter.diffLogs < DIFF_LOGS_PER_WINDOW) { | ||
| counter.diffLogs += 1; | ||
| console.log( | ||
| `[render-loop] ${name} #${counter.count} changed: ${changed.join(', ') || '(nothing of its own)'}`, | ||
| changed.reduce<Record<string, unknown>>((acc, key) => { | ||
| acc[key] = { from: previousValues[key], to: context[key] }; | ||
| return acc; | ||
| }, {}), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| if (counter.count < WARN_THRESHOLD || counter.warned) { | ||
| return; | ||
| } | ||
| counter.warned = true; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -HI '^next\.config\.(js|mjs|ts|cjs)$' --exec cat {}Repository: suitenumerique/conversations
Length of output: 1536
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== next config files =="
fd -HI '^next\.config\.(js|mjs|ts|cjs)$'
echo
echo "== debugRenderLoop references and definitions =="
fd -HI '^debugRenderLoop\.ts$'
xargs -r ast-grep outline --view expanded
rg -n "reactStrictMode|trackRender|ReactStrictMode|setNextRuntime" .github .next node_modules src -g '!node_modules' || true
echo
echo "== likely usage sites =="
rg -n "trackRender\(" src || true
echo
echo "== package versions =="
for f in package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lockb bun.lock; do
[ -e "$f" ] && echo "--- $f ---"
if [ "$f" = "package.json" ]; then
node -e "const p=require('package.json'); console.log(JSON.stringify({dependencies:p.dependencies,devDependencies:p.devDependencies,scripts:p.scripts}, null, 2))"
else
sed -n '1,120p' "$f"
fi
doneRepository: suitenumerique/conversations
Length of output: 518
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== next config files =="
fd -HI '^next\.config\.(js|mjs|ts|cjs)$' --exec sh -c 'echo "--- $1 ---"; cat "$1"' sh {}
echo
echo "== debugRenderLoop references and definition =="
fd -HI '^debugRenderLoop\.ts$' --exec sh -c 'echo "--- $1 ---"; wc -l "$1"; ast-grep --view compact outline "$1"; ast-grep --view expanded outline "$1"' sh {}
echo
echo "== search strict mode and trackRender usage =="
rg -n "reactStrictMode|ReactStrictMode|setNextRuntime|trackRender\(" --glob '!node_modules/**' --glob '!*.lock' . || true
echo
echo "== package versions snippets =="
for f in src/frontend/apps/conversations/package.json package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lock; do
[ -e "$f" ] || continue
echo "--- $f ---"
if [ "$f" = "src/frontend/apps/conversations/package.json" ] || [ "$f" = "package.json" ]; then
node -e "const fs=require('fs'); const p=JSON.parse(fs.readFileSync(process.argv[1],'utf8')); console.log(JSON.stringify({dependencies:p.dependencies,devDependencies:p.devDependencies,scripts:p.scripts}, null, 2))" "$f"
else
sed -n '1,180p' "$f"
fi
doneRepository: suitenumerique/conversations
Length of output: 2100
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== trackRender definition and usage =="
sed -n '1,170p' src/frontend/apps/conversations/src/utils/debugRenderLoop.ts
echo "--- trackRender call sites ---"
rg -n "trackRender\(" src/frontend/apps/conversations/src --glob '!node_modules/**' || true
echo
echo "== strict mode config search =="
rg -n "reactStrictMode|ReactStrictMode|setNextRuntime" --glob '!node_modules/**' --glob '!*.lock' . || true
echo
echo "== package versions snippets =="
for f in src/frontend/apps/conversations/package.json package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lock; do
[ -e "$f" ] || continue
echo "--- $f ---"
if [ "$f" = "src/frontend/apps/conversations/package.json" ] || [ "$f" = "package.json" ]; then
node -e "const fs=require('fs'); const p=JSON.parse(fs.readFileSync(process.argv[1],'utf8')); console.log(JSON.stringify({dependencies:p.dependencies,devDependencies:p.devDependencies,scripts:p.scripts}, null, 2))" "$f"
else
sed -n '1,180p' "$f"
fi
done
echo
echo "== Next.js documentation availability and strict mode config reference =="
node - <<'JS'
try {
const fs = require('fs');
const path = require('path');
const nextPkgPath = path.join(process.cwd(), 'node_modules/next/package.json');
console.log(fs.existsSync(nextPkgPath) ? 'next module present' : 'next module absent');
} catch (e) {
console.log('next module check unavailable:', e.message);
}
JSRepository: suitenumerique/conversations
Length of output: 10831
Avoid mutating counters from render bodies.
trackRender is called from component renders and increments module-level state, which React double-invokes during development under reactStrictMode. That can double counts and change key diffs, making this diagnostic flag loop activity even when the real app behavior is not looping. Move the counting/diff check to an effect or gate incrementing under the opt-in debug flag before recording Sentry state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/frontend/apps/conversations/src/utils/debugRenderLoop.ts` around lines 91
- 133, Update trackRender so render-time calls do not mutate counters or record
Sentry diagnostic state during React Strict Mode double invocation. Move
counting and changed-key processing into an effect-based path, or gate all
counter increments and related state updates behind the opt-in debug flag before
recording diagnostics; preserve verbose logging only when debugging is enabled.
1 participant
Purpose
Chat responses were re-rendering the entire conversation view once per streamed chunk, roughly a hundred times per second on a long answer. This burned a large amount of main thread work on every message, and is the suspected cause of the maximum update depth exceeded errors reported from production.
Proposal
Summary by CodeRabbit
Bug Fixes
Diagnostics