-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix performance of chat panel during streaming #3020
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,15 @@ | ||
| import { FuzzyEditFileTool, SearchReplaceEditTool, SearchReplaceMultiEditFileTool, TerminalCommandTool, TypecheckTool, WebSearchTool, WriteFileTool } from '@onlook/ai'; | ||
| import type { WebSearchResult } from '@onlook/models'; | ||
| import type { ToolUIPart } from 'ai'; | ||
| import { observer } from 'mobx-react-lite'; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify the necessity of the observer wrapper and consider adding memoization. The component is now wrapped with Additionally, other components in this PR (mentioned in the AI summary) follow a Run the following script to verify if If -import { observer } from 'mobx-react-lite';
+import { memo } from 'react';
+import { observer } from 'mobx-react-lite';
-export const ToolCallDisplay = observer(ToolCallDisplayComponent);
+export const ToolCallDisplay = memo(observer(ToolCallDisplayComponent));Also applies to: 12-12, 230-230 🏁 Script executed: #!/bin/bash
# Check if ToolUIPart type contains MobX observable definitions
# Search for ToolUIPart type definition
ast-grep --pattern 'type ToolUIPart = $$$'
# Also search for interface definition
ast-grep --pattern 'interface ToolUIPart {
$$$
}'
# Check for MobX observable usage in types
rg -n --type=ts --type=tsx 'ToolUIPart' -A 10 -B 2Length of output: 27 🏁 Script executed: #!/bin/bash
# Correct search for ToolUIPart type definition using proper ripgrep syntax
# Search for ToolUIPart type definition in TypeScript files
echo "=== Searching for ToolUIPart type/interface definition ==="
rg -n 'type ToolUIPart|interface ToolUIPart' -A 15
# Search for where ToolUIPart is imported/used in the tool-call-display file
echo -e "\n=== Checking imports in tool-call-display.tsx ==="
rg -n 'ToolUIPart' apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsx -B 5 -A 3
# Look for MobX observable patterns in ToolUIPart related files
echo -e "\n=== Checking for MobX observable patterns ==="
rg -n '@observable|makeObservable|observable\(' -A 3 -B 1Length of output: 2019 🏁 Script executed: #!/bin/bash
# View the actual component implementation to check for MobX store/observable access
echo "=== Full tool-call-display.tsx component ==="
cat apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsx
echo -e "\n=== Check for store hooks or observable access patterns ==="
rg -n 'useStore|useObserver|observer\(|\.get\(\)|@observable' apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsxLength of output: 8589 Remove the unnecessary The component only receives plain props (strings, boolean, Remove the observer wrapper and import: -import { observer } from 'mobx-react-lite';
import stripAnsi from 'strip-ansi';
-export const ToolCallDisplay = observer(ToolCallDisplayComponent);
+export const ToolCallDisplay = ToolCallDisplayComponent;
🤖 Prompt for AI Agents |
||
| import stripAnsi from 'strip-ansi'; | ||
| import { type z } from 'zod'; | ||
| import { BashCodeDisplay } from '../../code-display/bash-code-display'; | ||
| import { CollapsibleCodeBlock } from '../../code-display/collapsible-code-block'; | ||
| import { SearchSourcesDisplay } from '../../code-display/search-sources-display'; | ||
| import { ToolCallSimple } from './tool-call-simple'; | ||
|
|
||
| export const ToolCallDisplay = ({ | ||
| const ToolCallDisplayComponent = ({ | ||
| messageId, | ||
| toolPart, | ||
| isStream, | ||
|
|
@@ -224,4 +225,6 @@ export const ToolCallDisplay = ({ | |
| key={toolPart.toolCallId} | ||
| /> | ||
| ); | ||
| } | ||
| }; | ||
|
|
||
| export const ToolCallDisplay = observer(ToolCallDisplayComponent); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bun version inconsistency: test job now pins version to
1.2.21while typecheck job still useslatest. Consider aligning them for consistent CI builds.