fix(chat): stop the composer-footer render loop (#5162) - #5274
Conversation
`ChatPageScaffold` measured its floating composer footer with a ResizeObserver whose effect depended on the `footer` node itself. Every call site passes inline JSX, so `footer` took a fresh object identity on each render: the effect re-ran every pass, tearing down and rebuilding the observer and re-measuring. Because `ResizeObserver.observe` delivers an immediate initial observation, each render scheduled another `setFooterHeight`, and any measurement disagreeing with the committed height re-rendered, re-ran the effect, and cascaded until React aborted with "Maximum update depth exceeded". Typing was the easiest trigger — the composer's auto-growing textarea lives inside that footer, so every keystroke changed the measured height, which is why Sentry blamed `ChatComposer`'s `onChange`. Subscribe on footer *presence* instead: the observer already reports every size change, so it only needs re-subscribing when the footer element mounts or unmounts. Measurements matching the committed height are dropped so a settled layout can never schedule a re-render. Also harden the main chat, which has the same shape: - `Conversations`' composer-footer observer now skips no-op updates, so a sub-pixel rounding oscillation can't feed the same cascade. - `selectedThreadToolTimeline` / `selectedThreadProcessing` in both `Conversations` and `ChatThreadView` allocated a fresh `[]` per render, invalidating the `backgroundProcesses` memo every pass. Use the stable empty constants the surrounding code already established, which also clears both `react-hooks/exhaustive-deps` warnings there. Regression test asserts the footer observer is built once across re-renders while typing (it was one per keystroke before) and that the scroll region still reserves the measured height.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
M3gA-Mind has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe changes stabilize empty conversation-state references and revise footer height measurement to avoid redundant React updates. Regression tests cover observer reuse, measured padding updates, and simulated footer resizing. ChangesChat stability improvements
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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: 2
🤖 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 `@app/src/features/conversations/Conversations.tsx`:
- Around line 166-172: Add regression tests for stable empty fallback identities
in app/src/features/conversations/Conversations.tsx:166-172 and
app/src/features/conversations/components/ChatThreadView.tsx:106-111. Rerender
Conversations without a selected thread and ChatThreadView with threadId={null},
then verify the respective background-process/processing derivation is not
recomputed due to a new fallback identity; cover the related usage sites at
Conversations.tsx:1605-1610 and ChatThreadView.tsx:262-267.
- Around line 1757-1763: Add a regression test covering the composer footer
measurement observer in Conversations: after an initial measurement updates the
height, trigger a second identical measurement and verify the component does not
perform another state-driven render update. Keep the existing changed-height
padding assertion intact and target the setComposerFooterHeight no-op path.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cf0d6de6-7a3e-4576-be75-e8a390a6c59e
📒 Files selected for processing (4)
app/src/components/orchestration/AgentChatPanel.tsxapp/src/components/orchestration/__tests__/AgentChatPanel.test.tsxapp/src/features/conversations/Conversations.tsxapp/src/features/conversations/components/ChatThreadView.tsx
…nce (tinyhumansai#5162) Two coverage gaps CodeRabbit flagged on the render-loop fix. `ChatThreadView.memoIdentity.test.tsx` (new) pins the stable-empty-fallback contract: it wraps `selectBackgroundProcesses` (delegating to the real implementation) and asserts the `backgroundProcesses` memo is not re-derived across re-renders, both with no thread selected and for a thread with no timeline yet. Verified failing against the previous per-render `[]` fallback — the selector ran 3x and 2x respectively instead of once. `Conversations.render.test.tsx` gains a convergence test for the composer-footer ResizeObserver: a settled layout re-reporting the same height stops costing renders entirely, while a genuine resize still updates the padding. Render passes are counted through `useUsageState`, which `Conversations` calls exactly once per render. That second test deliberately does NOT claim to discriminate the `prev === next ? prev : next` guard in `measure()`. React's own Object.is bail-out already absorbs an identical setState, so render counts are identical with and without that guard — measured 8 → 9, 9, 9 either way. Asserting otherwise would be a test that passes for the wrong reason. The guard stays as defensive, self-documenting code; what the test pins is the property that actually matters: repeat notifications converge and never walk the padding.
There was a problem hiding this comment.
M3gA-Mind has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
1 participant
Summary
Maximum update depth exceeded(SentryTAURI-REACT-2G, ~12 events / ~5 users across 3 shortIds).ChatPageScaffold's composer-footerResizeObservereffect depended on thefooterReact node, which is inline JSX at every call site and therefore has a new object identity on every render.Conversations) so a sub-pixel rounding oscillation cannot feed the same cascade.[]allocations in the chat hot path with the stableEMPTY_*constants the surrounding code already established (also clears bothreact-hooks/exhaustive-depswarnings there).Problem
ChatPageScaffold(app/src/components/orchestration/AgentChatPanel.tsx) measures its floating composer footer with aResizeObserverand feeds the height into the scroll region's bottom padding. The effect's dependency was thefooternode itself:Because both call sites pass inline JSX,
footertakes a fresh object identity on every render. Three effects compounded:ResizeObserver.observe()delivers an immediate initial observation, so every render scheduled anothersetFooterHeight.setFooterHeightwas unconditional, so any measurement disagreeing with committed state re-rendered → newfooteridentity → back to step 1.That cascade runs until React trips its nested-update limit and throws
Maximum update depth exceeded.Why Sentry blamed
ChatComposer.onChange: the composer's auto-growing textarea lives inside that footer, so each keystroke changed the measured height and pumped the cycle. The composer was the trigger, not the defect — which is why the previous mitigations taggedTAURI-REACT-2GinChatComposer/Conversations/ThreadGoalChipnever closed the issue.Solution
AgentChatPanel.tsx(root cause). Key the subscription on footer presence (hasFooter = Boolean(footer)) rather than the node. The observer already reports every size change, so it only needs re-subscribing when the footer element mounts or unmounts — not when its children re-render. A localapplyHeighthelper drops measurements equal to the committed height, so a settled layout can never schedule a re-render and therefore can never feed a cascade.Conversations.tsx(same shape, hardening). The main chat's composer-footer observer now skips no-op updates. Its deps were already stable, so this was not looping, but it watches the footer that contains the composer while feeding the message list's padding — the exact coupling that lets sub-pixel rounding oscillate.Conversations.tsx+ChatThreadView.tsx(render churn).selectedThreadToolTimeline/selectedThreadProcessingallocated a fresh[]per render, giving them a new identity every pass and invalidating thebackgroundProcessesuseMemoevery time. Both files already defineEMPTY_*constants for precisely this reason; these two sites were missed.Behavior is preserved throughout — the observer still reports every real size change.
Submission Checklist
diff-cover) meet the gate enforced by.github/workflows/ci-lite.yml. Runpnpm test:coverageandpnpm test:rustlocally; PRs below 80% on changed lines will not merge.N/A: behaviour-only change— Coverage matrix updated — added/removed/renamed feature rows indocs/TEST-COVERAGE-MATRIX.mdreflect this change (no feature rows added/removed/renamed)N/A— All affected feature IDs from the matrix are listed in the PR description under## Related(no matrix rows change)N/A: no release-cut surface touched— Manual smoke checklist updated if this touches release-cut surfaces (docs/RELEASE-MANUAL-SMOKE.md)Closes #NNNin the## RelatedsectionRegression test
app/src/components/orchestration/__tests__/AgentChatPanel.test.tsxadds acomposer footer measurementblock that stubsResizeObserver(modelling the real initial observation onobserve()) and reports a realoffsetHeightfor the footer wrapper only.does not rebuild the footer ResizeObserver on every render while typing— the failure-path test. Verified failing before / passing after: with only the dependency array reverted to[footer](test hook kept in place), it reports 4 footer observers instead of 1 — one per keystroke render.still reserves the measured footer height on the scroll region— the happy path, guarding that the changed subscription key didn't break the layout feature: a size change through the observer still updatespaddingBottom.useStickToBottomalso builds aResizeObserver, so the assertions select ours by the element it observes.Impact
Related
app/src/components/chat/ChatComposer.tsx(lines ~112-129) is a leftover mitigation for this issue — it mutates a ref and callssetTimeoutduring render (an impure render side effect) andconsole.warns in production. Left in place to keep this diff focused; worth removing now that the root cause is fixed.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check— Prettier clean on all four changed filespnpm typecheck— cleanvitest relatedover the three changed source files — 18 files / 287 tests passed; full frontend suite 768 files / 8971 tests passed, 0 failedN/A: no Rust changed— Rust fmt/check (if changed)N/A: no Tauri shell changed— Tauri fmt/check (if changed)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Maximum update depth exceededwhile typing.Parity Contract
paddingBottom; only the subscription key (presence vs node identity) and the no-op-update guard changed.typeof ResizeObserver === 'undefined'one-shot fallback is retained; the!elearly return now also covers!hasFooter, and both still reset the height to0.Duplicate / Superseded PR Handling
Summary by CodeRabbit
Bug Fixes
Tests