fix(web): reduce unnecessary re-renders in chat reference panel#1042
Merged
brendan-kellam merged 7 commits intomainfrom Mar 25, 2026
Merged
fix(web): reduce unnecessary re-renders in chat reference panel#1042brendan-kellam merged 7 commits intomainfrom
brendan-kellam merged 7 commits intomainfrom
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughExtracts CodeMirror reference decoration/interaction into a reusable extension, moves per-file fetching into a memoized container component, adjusts extension hook memoization, replaces domain routing with a single-tenant constant, and adds a changelog entry for improved citation rendering performance. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EditorView as CodeMirror EditorView
participant Extension as referencesHighlightExtension
participant ReactComp as ReferencedFileSourceListItem
participant Container as ReferencedFileSourceListItemContainer
User->>EditorView: hover / move / click
EditorView->>Extension: DOM event handlers (posAtCoords)
Extension->>Extension: resolve reference under cursor (shortest matching range)
alt hover changed
Extension->>ReactComp: onHoveredReferenceChanged(reference?)
end
alt click toggles selection
Extension->>ReactComp: onSelectedReferenceChanged(reference?)
end
Container->>ReactComp: provide fetched file source + references
ReactComp->>EditorView: dispatch setHoveredIdEffect / setSelectedIdEffect to sync external state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n file Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/features/chat/components/chatThread/chatThread.tsx`:
- Line 34: The code hardcodes SINGLE_TENANT_ORG_DOMAIN when building the
duplicate-chat route; instead use the active route/domain value (e.g., the
domain from the Next router query or the existing component state) so duplicates
stay within the current org. Locate occurrences of SINGLE_TENANT_ORG_DOMAIN in
chatThread.tsx (and the duplicate handler such as handleDuplicateChat or where
router.push/Link is used around lines 34 and 343-345) and replace them with the
current domain variable (e.g., router.query.domain or the component’s
selectedDomain prop/state) when constructing the `/app/{domain}/chat/{chatId}`
path. Ensure the domain falls back to SINGLE_TENANT_ORG_DOMAIN only if the
active route domain is undefined.
In
`@packages/web/src/features/chat/components/chatThread/referencedSourcesListView.tsx`:
- Around line 196-215: The parent effect that scrolls to a citation is exiting
early when FileSourceItem asynchronously fetches file data, so ensure the parent
still exposes the file anchor/editor reference (or is notified when the file
finishes loading) instead of relying on FileSourceItem's render timing;
specifically, in the render loop around getFileId(fileSource) and
FileSourceItem, either (A) render a stable placeholder anchor/editor ref in the
parent for each fileId (so the existing scroll effect can find the anchor even
on cache miss) and continue to forward setEditorRef into FileSourceItem, or (B)
add an explicit onFileLoaded callback prop on FileSourceItem and call
onFileLoaded(fileId) when its query completes so the parent scroll effect can
retry; update usages of FileSourceItem, getFileId, setEditorRef, and
collapsedFileIds accordingly.
In
`@packages/web/src/features/chat/components/chatThread/referencesHighlightExtension.ts`:
- Around line 77-84: The loop iterating from range.startLine to range.endLine
calls state.doc.line(line) without checking lower bounds; add a guard for line <
1 (e.g., if (line < 1) continue;) before any state.doc.line(...) calls so
negative or zero startLine values don't cause a RangeError; update the loop that
uses range.startLine, range.endLine, isSelected,
decorations.push(selectedLineDecoration.range(state.doc.line(line).from)) and
similar calls to short-circuit out-of-range low values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e4073964-112b-43e5-82cb-cb14d1ad570b
📒 Files selected for processing (6)
CHANGELOG.mdpackages/web/src/features/chat/components/chatThread/chatThread.tsxpackages/web/src/features/chat/components/chatThread/referencedFileSourceListItem.tsxpackages/web/src/features/chat/components/chatThread/referencedSourcesListView.tsxpackages/web/src/features/chat/components/chatThread/referencesHighlightExtension.tspackages/web/src/hooks/useExtensionWithDependency.ts
packages/web/src/features/chat/components/chatThread/referencedSourcesListView.tsx
Show resolved
Hide resolved
packages/web/src/features/chat/components/chatThread/referencesHighlightExtension.ts
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ReferencedSourcesListView's map body into a memoizedFileSourceItemcomponent with its ownuseQuery, stableuseCallbacks forrefandonExpandedChanged, and scoped hover/selected reference props — preventing unaffected file items from re-rendering on hover/select changesreferencesHighlightExtension— hover and select state changes are now dispatched as CodeMirrorStateEffects rather than causing theextensionsarray to be recreateduseExtensionWithDependencyto excludeextensionFactoryfrom itsuseMemodeps, makinglanguageExtensionandkeymapExtensionstable references across rendersFixes SOU-725
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes