fix: always swap conversation data on tab switch#166
fix: always swap conversation data on tab switch#166psypeal wants to merge 9 commits intomatt1398:mainfrom
Conversation
# Conflicts: # src/renderer/components/chat/ContextBadge.tsx # src/renderer/utils/contextTracker.ts
# Conflicts: # src/renderer/components/chat/ContextBadge.tsx
sessionAnalyzer now reads pre-computed costUsd from calculateMetrics() instead of re-computing costs independently. Parent cost uses detail.metrics.costUsd, subagent cost uses proc.metrics.costUsd. This ensures the cost analysis panel and chat header always agree. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Electron's before-input-event preventDefault blocks both Chromium's built-in reload AND keydown propagation to the renderer. Route Ctrl+R through IPC instead: main process intercepts and sends session:refresh, preload bridges to renderer, store listener refreshes in place. Switch all refresh paths (IPC, refresh button, keyboard shortcut) to refreshSessionInPlace to avoid unmounting the scroll container, then dispatch a custom event to smoothly scroll to the bottom. Fixes matt1398#85
# Conflicts: # src/renderer/utils/sessionAnalyzer.ts # test/renderer/utils/sessionAnalyzer.test.ts
…hlighting - Add BashToolViewer with syntax-highlighted command input via CodeBlockViewer - Add CollapsibleOutputSection (collapsed by default) for tool output - Apply collapsible output to DefaultToolViewer and BashToolViewer - Add bash/zsh/fish keyword sets for syntax highlighting - Add keyword sets for c, cpp, java, kotlin, swift, lua, html, yaml - Fix markdown/html false type-coloring on capitalized words - Add markdown preview toggle to ReadToolViewer (defaults to preview) - Default WriteToolViewer to preview mode for markdown files - Extend comment detection for zsh, fish, yaml (#) and lua (--)
Remove the sessionChanged guard from setActiveTab — the global conversation may belong to a different tab even when selectedSessionId matches (multi-tab scenario). Always swap from per-tab cache or trigger a fetch to ensure the correct conversation is displayed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a custom session refresh mechanism by intercepting Ctrl+R/Cmd+R in the main process and notifying the renderer via IPC, resolving issues #58 and #85. It adds a specialized BashToolViewer with syntax highlighting and collapsible output, and provides Markdown preview support for Read and Write tool viewers. The syntax highlighter is expanded to support multiple new languages, and tab state management is refactored for better consistency. Feedback is provided to remove a redundant language check in the syntax highlighter.
|
|
||
| // If no highlighting support, return plain text as single-element array | ||
| if (keywords.size === 0 && !['json', 'css', 'html', 'bash', 'markdown'].includes(language)) { | ||
| if (keywords.size === 0 && !['json', 'css', 'bash'].includes(language)) { |
There was a problem hiding this comment.
The inclusion of 'bash' in the exclusion list is now redundant because bash has been added to the KEYWORDS map (line 405). Since keywords.size will be greater than 0 for bash, the condition keywords.size === 0 will be false, and the code will correctly proceed to the highlighting loop regardless of whether 'bash' is in this array. This matches the logic applied to 'html' which was correctly removed from this list.
| if (keywords.size === 0 && !['json', 'css', 'bash'].includes(language)) { | |
| if (keywords.size === 0 && !['json', 'css'].includes(language)) { |
📝 WalkthroughWalkthroughThis PR implements session refresh triggered by Ctrl+R/Cmd+R, distinguishing it from hard reload (Ctrl+Shift+R). It adds an IPC channel for main-to-renderer communication, updates preload APIs, refactors tool viewers with a new Bash viewer and collapsible output component, improves syntax highlighting coverage, and adds Markdown preview toggles in file viewers. Changes
Possibly related PRs
Suggested labels
✨ Finishing Touches⚔️ Resolve merge conflicts
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: 4
🧹 Nitpick comments (4)
src/main/index.ts (1)
492-496: Consider addingSESSION_REFRESHto local constants for consistency.The channel name
'session:refresh'is hardcoded here, while other IPC channels likeSSH_STATUSandCONTEXT_CHANGEDare defined as local constants at lines 47-51 to avoid preload boundary violations. AddingSESSION_REFRESHto those local constants would improve maintainability and consistency.♻️ Suggested addition to local constants
// IPC channel constants (duplicated from `@preload` to avoid boundary violation) const SSH_STATUS = 'ssh:status'; const CONTEXT_CHANGED = 'context:changed'; const HTTP_SERVER_START = 'httpServer:start'; const HTTP_SERVER_STOP = 'httpServer:stop'; const HTTP_SERVER_GET_STATUS = 'httpServer:getStatus'; +const SESSION_REFRESH = 'session:refresh';Then use
SESSION_REFRESHinstead of the string literal at line 494.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/index.ts` around lines 492 - 496, The hardcoded IPC channel 'session:refresh' should be extracted to a local constant to match existing pattern; add a new constant name SESSION_REFRESH alongside SSH_STATUS and CONTEXT_CHANGED and replace the string literal in the mainWindow.webContents.send call inside the keyboard handler (the block checking input.control/meta/shift and input.key.toLowerCase() === 'r') with SESSION_REFRESH to avoid preload boundary issues and keep channel names consistent.src/renderer/components/chat/ChatHistory.tsx (1)
380-387: Global event may scroll inactive tabs in split-pane layouts.The
session-refresh-scroll-bottomevent is dispatched globally (fromsrc/renderer/store/index.ts:282,src/renderer/hooks/useKeyboardShortcuts.ts:267, andsrc/renderer/components/layout/TabBar.tsx:221), but this listener doesn't check ifisThisTabActivebefore scrolling. In split-pane scenarios, all mountedChatHistoryinstances will callscrollToBottom, potentially disrupting scroll positions in inactive panes.Consider guarding the scroll action:
♻️ Proposed fix to scope scroll to active tab
// Listen for session-refresh-scroll-bottom events (from Ctrl+R / refresh button) useEffect(() => { const handler = (): void => { + if (!isThisTabActive) return; scrollToBottom('smooth'); }; window.addEventListener('session-refresh-scroll-bottom', handler); return () => window.removeEventListener('session-refresh-scroll-bottom', handler); - }, [scrollToBottom]); + }, [scrollToBottom, isThisTabActive]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/ChatHistory.tsx` around lines 380 - 387, The global "session-refresh-scroll-bottom" handler in ChatHistory currently calls scrollToBottom for every mounted instance, which scrolls inactive split-pane tabs; update the useEffect that registers the handler to check the component's isThisTabActive (or only attach the listener when isThisTabActive is true) before calling scrollToBottom, and include isThisTabActive in the effect dependencies so the listener is added/removed correctly; reference the useEffect in ChatHistory and the scrollToBottom and isThisTabActive symbols when making the change.src/renderer/hooks/useKeyboardShortcuts.ts (1)
263-268: Consider adding error handling for the refresh operation.The
Promise.allchain lacks a.catch()handler. If either promise rejects, the failure is silent andsession-refresh-scroll-bottomwon't be dispatched. For a user-initiated refresh (Cmd+R), consider providing feedback on failure.♻️ Proposed fix to add error handling
void Promise.all([ refreshSessionInPlace(selectedProjectId, selectedSessionId), fetchSessions(selectedProjectId), ]).then(() => { window.dispatchEvent(new CustomEvent('session-refresh-scroll-bottom')); + }).catch((error) => { + logger.error('Failed to refresh session', error); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/useKeyboardShortcuts.ts` around lines 263 - 268, The Promise.all call using refreshSessionInPlace(selectedProjectId, selectedSessionId) and fetchSessions(selectedProjectId) has no error handling—add a .catch handler to log the error (console.error) and surface user feedback (e.g., dispatch a distinct CustomEvent like 'session-refresh-failed' or call the app's toast/notification API) so failures aren't silent; keep the existing .then to dispatch 'session-refresh-scroll-bottom' only on success and ensure the .catch receives the same error context (project/session IDs) for clearer diagnostics.src/renderer/components/chat/items/linkedTool/CollapsibleOutputSection.tsx (1)
30-39: Consider addingaria-expandedfor accessibility.The button toggles visibility but lacks the
aria-expandedattribute, which helps screen readers announce the current state.♿ Proposed accessibility improvement
<button type="button" className="mb-1 flex items-center gap-2 text-xs" style={{ color: 'var(--tool-item-muted)', background: 'none', border: 'none', padding: 0, cursor: 'pointer' }} onClick={() => setIsExpanded((prev) => !prev)} + aria-expanded={isExpanded} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/items/linkedTool/CollapsibleOutputSection.tsx` around lines 30 - 39, The toggle button in CollapsibleOutputSection is missing accessibility state; update the button element (the one using onClick={() => setIsExpanded(prev => !prev)} and rendering isExpanded, Chevron icons and StatusDot) to include aria-expanded set to the boolean isExpanded (aria-expanded={isExpanded}) so screen readers announce the current state; additionally ensure the collapsible content element uses an id referenced by aria-controls on the same button (aria-controls="...") and toggles its visibility/aria-hidden in sync with isExpanded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/chat/items/linkedTool/ReadToolViewer.tsx`:
- Line 58: The viewMode state in ReadToolViewer is only initialized once and
therefore can leak between reused component instances; add an effect that
watches the active file identity (e.g., the isMarkdownFile prop or the linked
tool's file id) and calls setViewMode(isMarkdownFile ? 'preview' : 'code')
whenever that identity changes so the view resets correctly when switching
files. Ensure the useEffect references the unique symbols used in this component
(viewMode, setViewMode, isMarkdownFile or the linked tool/file id) and updates
viewMode accordingly.
In `@src/renderer/components/chat/items/linkedTool/WriteToolViewer.tsx`:
- Line 24: The component initializes viewMode based on isMarkdownFile but never
updates it when linkedTool (or its result/file type) changes; add a
React.useEffect that depends on isMarkdownFile (and/or linkedTool/result) to
call setViewMode(isMarkdownFile ? 'preview' : 'code') so the UI switches when
the file type changes; apply the same pattern to ReadToolViewer; target the
viewMode and setViewMode state in WriteToolViewer and ReadToolViewer and ensure
the effect runs when isMarkdownFile (or linkedTool/result) changes.
In `@src/renderer/components/chat/items/LinkedToolItem.tsx`:
- Line 175: The code currently passes variant={toolVariant} into BaseItem but
BaseItemProps doesn't define variant, causing a TS error; remove variant from
the BaseItem usage (the instance that renders BaseItem) and instead pass
variant={toolVariant} into the BashToolViewer component (the BashToolViewer
invocation around lines ~189-190). Update the JSX so BaseItem no longer receives
variant and BashToolViewer receives the toolVariant prop, ensuring the
BashToolViewer prop type accepts variant if needed.
In `@src/renderer/store/slices/tabSlice.ts`:
- Around line 299-316: Change the cache-hit logic to treat an existing
tabSessionData entry as a cache regardless of whether conversation is null: if
cachedTabData exists (not just cachedTabData.conversation), call set(...) to
restore the cached state's
conversationLoading/sessionDetailLoading/sessionDetailError/sessionClaudeMdStats/sessionContextStats/sessionPhaseInfo/visibleAIGroupId/selectedAIGroup
(preserving their in-flight/loading values) instead of calling
get().fetchSessionDetail(...) which triggers duplicate requests; apply this same
change to both places where the code currently checks cachedTabData.conversation
(the branches that call get().fetchSessionDetail), and only call
fetchSessionDetail when no cachedTabData exists at all.
---
Nitpick comments:
In `@src/main/index.ts`:
- Around line 492-496: The hardcoded IPC channel 'session:refresh' should be
extracted to a local constant to match existing pattern; add a new constant name
SESSION_REFRESH alongside SSH_STATUS and CONTEXT_CHANGED and replace the string
literal in the mainWindow.webContents.send call inside the keyboard handler (the
block checking input.control/meta/shift and input.key.toLowerCase() === 'r')
with SESSION_REFRESH to avoid preload boundary issues and keep channel names
consistent.
In `@src/renderer/components/chat/ChatHistory.tsx`:
- Around line 380-387: The global "session-refresh-scroll-bottom" handler in
ChatHistory currently calls scrollToBottom for every mounted instance, which
scrolls inactive split-pane tabs; update the useEffect that registers the
handler to check the component's isThisTabActive (or only attach the listener
when isThisTabActive is true) before calling scrollToBottom, and include
isThisTabActive in the effect dependencies so the listener is added/removed
correctly; reference the useEffect in ChatHistory and the scrollToBottom and
isThisTabActive symbols when making the change.
In `@src/renderer/components/chat/items/linkedTool/CollapsibleOutputSection.tsx`:
- Around line 30-39: The toggle button in CollapsibleOutputSection is missing
accessibility state; update the button element (the one using onClick={() =>
setIsExpanded(prev => !prev)} and rendering isExpanded, Chevron icons and
StatusDot) to include aria-expanded set to the boolean isExpanded
(aria-expanded={isExpanded}) so screen readers announce the current state;
additionally ensure the collapsible content element uses an id referenced by
aria-controls on the same button (aria-controls="...") and toggles its
visibility/aria-hidden in sync with isExpanded.
In `@src/renderer/hooks/useKeyboardShortcuts.ts`:
- Around line 263-268: The Promise.all call using
refreshSessionInPlace(selectedProjectId, selectedSessionId) and
fetchSessions(selectedProjectId) has no error handling—add a .catch handler to
log the error (console.error) and surface user feedback (e.g., dispatch a
distinct CustomEvent like 'session-refresh-failed' or call the app's
toast/notification API) so failures aren't silent; keep the existing .then to
dispatch 'session-refresh-scroll-bottom' only on success and ensure the .catch
receives the same error context (project/session IDs) for clearer diagnostics.
🪄 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: 206ebec3-d523-40f7-9a0b-71f86eea5560
📒 Files selected for processing (20)
src/main/index.tssrc/preload/constants/ipcChannels.tssrc/preload/index.tssrc/renderer/api/httpClient.tssrc/renderer/components/chat/ChatHistory.tsxsrc/renderer/components/chat/items/LinkedToolItem.tsxsrc/renderer/components/chat/items/linkedTool/BashToolViewer.tsxsrc/renderer/components/chat/items/linkedTool/CollapsibleOutputSection.tsxsrc/renderer/components/chat/items/linkedTool/DefaultToolViewer.tsxsrc/renderer/components/chat/items/linkedTool/ReadToolViewer.tsxsrc/renderer/components/chat/items/linkedTool/WriteToolViewer.tsxsrc/renderer/components/chat/items/linkedTool/index.tssrc/renderer/components/chat/viewers/syntaxHighlighter.tssrc/renderer/components/layout/TabBar.tsxsrc/renderer/hooks/useKeyboardShortcuts.tssrc/renderer/store/index.tssrc/renderer/store/slices/tabSlice.tssrc/renderer/utils/toolRendering/index.tssrc/renderer/utils/toolRendering/toolContentChecks.tssrc/shared/types/api.ts
| : undefined; | ||
|
|
||
| const isMarkdownFile = /\.mdx?$/i.test(filePath); | ||
| const [viewMode, setViewMode] = React.useState<'code' | 'preview'>(isMarkdownFile ? 'preview' : 'code'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether ReadToolViewer remounts on tool-item switch (key-based mount)
# If not remounted, the state-sync effect above is required.
rg -n --type=tsx -C4 'ReadToolViewer|key=' src/renderer/components/chatRepository: matt1398/claude-devtools
Length of output: 95
🏁 Script executed:
#!/bin/bash
# Correct the file type and search for ReadToolViewer usage
rg -n --type=typescript -C4 'ReadToolViewer' src/renderer/components/chatRepository: matt1398/claude-devtools
Length of output: 102
🏁 Script executed:
#!/bin/bash
# Also search for the component definition and its parent usages
fd --type f -e tsx -e ts 'linkedTool|ReadToolViewer' src/renderer/components/chatRepository: matt1398/claude-devtools
Length of output: 132
🏁 Script executed:
#!/bin/bash
# Find the parent component that uses ReadToolViewer
rg -n '<ReadToolViewer' src/renderer/components --type=typescript -C6Repository: matt1398/claude-devtools
Length of output: 102
🏁 Script executed:
#!/bin/bash
# Search for ReadToolViewer usage without type flag
rg 'ReadToolViewer' src/renderer/components --context=5Repository: matt1398/claude-devtools
Length of output: 4901
🏁 Script executed:
#!/bin/bash
# Read the ReadToolViewer component to check dependencies and useState
cat -n src/renderer/components/chat/items/linkedTool/ReadToolViewer.tsx | head -100Repository: matt1398/claude-devtools
Length of output: 4047
🏁 Script executed:
#!/bin/bash
# Find parent components that render ReadToolViewer
rg '<ReadToolViewer' src/renderer/components --context=8Repository: matt1398/claude-devtools
Length of output: 3313
🏁 Script executed:
#!/bin/bash
# Check LinkedToolItem.tsx to verify the parent rendering context
cat -n src/renderer/components/chat/items/LinkedToolItem.tsx | grep -A 15 -B 5 'ReadToolViewer'Repository: matt1398/claude-devtools
Length of output: 1874
Sync viewMode when the active file changes.
Line 58 initializes viewMode only on mount. When users switch between different linked tools (markdown and non-markdown files), the component instance is reused without remounting, causing the view mode to leak from the previous file. For example, if a markdown file is viewed in preview mode, switching to a code file will incorrectly remain in preview mode.
Proposed fix
const isMarkdownFile = /\.mdx?$/i.test(filePath);
const [viewMode, setViewMode] = React.useState<'code' | 'preview'>(isMarkdownFile ? 'preview' : 'code');
+ React.useEffect(() => {
+ setViewMode(isMarkdownFile ? 'preview' : 'code');
+ }, [isMarkdownFile]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [viewMode, setViewMode] = React.useState<'code' | 'preview'>(isMarkdownFile ? 'preview' : 'code'); | |
| const isMarkdownFile = /\.mdx?$/i.test(filePath); | |
| const [viewMode, setViewMode] = React.useState<'code' | 'preview'>(isMarkdownFile ? 'preview' : 'code'); | |
| React.useEffect(() => { | |
| setViewMode(isMarkdownFile ? 'preview' : 'code'); | |
| }, [isMarkdownFile]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/chat/items/linkedTool/ReadToolViewer.tsx` at line 58,
The viewMode state in ReadToolViewer is only initialized once and therefore can
leak between reused component instances; add an effect that watches the active
file identity (e.g., the isMarkdownFile prop or the linked tool's file id) and
calls setViewMode(isMarkdownFile ? 'preview' : 'code') whenever that identity
changes so the view resets correctly when switching files. Ensure the useEffect
references the unique symbols used in this component (viewMode, setViewMode,
isMarkdownFile or the linked tool/file id) and updates viewMode accordingly.
| const isCreate = toolUseResult?.type === 'create'; | ||
| const isMarkdownFile = /\.mdx?$/i.test(filePath); | ||
| const [viewMode, setViewMode] = React.useState<'code' | 'preview'>('code'); | ||
| const [viewMode, setViewMode] = React.useState<'code' | 'preview'>(isMarkdownFile ? 'preview' : 'code'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether WriteToolViewer remounts with a distinct key when tool/file changes.
rg -n --type=tsx -C4 'WriteToolViewer|key=' src/renderer/components/chatRepository: matt1398/claude-devtools
Length of output: 95
🏁 Script executed:
# Search for WriteToolViewer usage to understand remounting behavior
rg -n 'WriteToolViewer' src/renderer/components --type ts --type tsxRepository: matt1398/claude-devtools
Length of output: 95
🏁 Script executed:
# Also check for key prop patterns in chat component files
rg -n 'key=' src/renderer/components/chat -A 2 -B 2Repository: matt1398/claude-devtools
Length of output: 25345
🏁 Script executed:
# Look at the parent component to understand when/how filePath changes
fd -e ts -e tsx src/renderer/components/chat | head -20Repository: matt1398/claude-devtools
Length of output: 485
🏁 Script executed:
# Search directly for WriteToolViewer component
rg -n 'WriteToolViewer' src/rendererRepository: matt1398/claude-devtools
Length of output: 727
🏁 Script executed:
# Also search in all locations (not just renderer)
rg -n 'WriteToolViewer'Repository: matt1398/claude-devtools
Length of output: 727
🏁 Script executed:
# Look at the WriteToolViewer file itself to understand its props
cat -n src/renderer/components/chat/items/linkedTool/WriteToolViewer.tsx | head -50Repository: matt1398/claude-devtools
Length of output: 2316
🏁 Script executed:
# Check LinkedToolItem and how it's rendered - look for parent usage and key patterns
rg -n 'LinkedToolItem' src/renderer/components/chat -B 2 -A 2Repository: matt1398/claude-devtools
Length of output: 9529
🏁 Script executed:
# Check the full LinkedToolItem component around line 184
sed -n '175,195p' src/renderer/components/chat/items/LinkedToolItem.tsxRepository: matt1398/claude-devtools
Length of output: 901
🏁 Script executed:
# Check if itemId is based on tool.id and if it's stable across updates
sed -n '130,170p' src/renderer/components/chat/items/ExecutionTrace.tsxRepository: matt1398/claude-devtools
Length of output: 1629
🏁 Script executed:
# Also check DisplayItemList to see how itemKey is constructed for tools
sed -n '145,165p' src/renderer/components/chat/DisplayItemList.tsxRepository: matt1398/claude-devtools
Length of output: 838
🏁 Script executed:
# Check ReadToolViewer to see if it has similar initialization and if it's been fixed
cat -n src/renderer/components/chat/items/linkedTool/ReadToolViewer.tsx | head -40Repository: matt1398/claude-devtools
Length of output: 1479
🏁 Script executed:
# Check if LinkedToolItem receives key in DisplayItemList - look for the full context
sed -n '148,162p' src/renderer/components/chat/DisplayItemList.tsxRepository: matt1398/claude-devtools
Length of output: 698
🏁 Script executed:
# Check the structure of linkedTool/item.tool to understand if it's immutable or can change
rg -n 'linkedTool.*filePath|item\.tool' src/renderer/components/chat/items/linkedTool -A 1 | head -30Repository: matt1398/claude-devtools
Length of output: 50
🏁 Script executed:
# Get full context around LinkedToolItem rendering in DisplayItemList - see if key is assigned
sed -n '145,170p' src/renderer/components/chat/DisplayItemList.tsxRepository: matt1398/claude-devtools
Length of output: 962
🏁 Script executed:
# Check how items array is constructed and if tool objects are immutable or mutable
rg -n 'case.*tool|linkedTool' src/renderer/components/chat/DisplayItemList.tsx -B 3 -A 8 | head -50Repository: matt1398/claude-devtools
Length of output: 780
🏁 Script executed:
# Check if itemKey is being used as key prop for LinkedToolItem - search around line 153-165
sed -n '148,175p' src/renderer/components/chat/DisplayItemList.tsxRepository: matt1398/claude-devtools
Length of output: 1071
🏁 Script executed:
# Check if the items array content is mutable or immutable when tool result updates
rg -n 'items.*=' src/renderer/components/chat/DisplayItemList.tsx | head -10Repository: matt1398/claude-devtools
Length of output: 148
🏁 Script executed:
# Look at ReadToolViewer to see if it also has the same pattern (useState without useEffect)
sed -n '17,60p' src/renderer/components/chat/items/linkedTool/ReadToolViewer.tsxRepository: matt1398/claude-devtools
Length of output: 1598
🏁 Script executed:
# Critical check: does LinkedToolItem get a key prop in DisplayItemList?
sed -n '150,180p' src/renderer/components/chat/DisplayItemList.tsx | grep -E 'key=|linkedKey|itemKey.*='Repository: matt1398/claude-devtools
Length of output: 185
🏁 Script executed:
# Let me see the full map function in DisplayItemList to confirm key prop usage
sed -n '100,120p' src/renderer/components/chat/DisplayItemList.tsxRepository: matt1398/claude-devtools
Length of output: 839
🏁 Script executed:
# Check if there's a key prop being passed in the rendered element
rg -n 'return.*itemKey' src/renderer/components/chat/DisplayItemList.tsx -A 15 | head -40Repository: matt1398/claude-devtools
Length of output: 50
🏁 Script executed:
# Check the return statement in the map function - what wraps the element?
sed -n '100,330p' src/renderer/components/chat/DisplayItemList.tsx | tail -50Repository: matt1398/claude-devtools
Length of output: 1865
🏁 Script executed:
# More focused: look at the end of the switch and what happens to element and itemKey
sed -n '285,310p' src/renderer/components/chat/DisplayItemList.tsxRepository: matt1398/claude-devtools
Length of output: 1123
🏁 Script executed:
# Perfect! Now let's check if ReadToolViewer also has the same issue - does it have a useEffect?
grep -n "useEffect\|viewMode" src/renderer/components/chat/items/linkedTool/ReadToolViewer.tsxRepository: matt1398/claude-devtools
Length of output: 602
🏁 Script executed:
# Check WriteToolViewer for any useEffect
grep -n "useEffect\|viewMode" src/renderer/components/chat/items/linkedTool/WriteToolViewer.tsxRepository: matt1398/claude-devtools
Length of output: 602
🏁 Script executed:
# Look at the full wrapper - see the key being passed
sed -n '325,345p' src/renderer/components/chat/DisplayItemList.tsxRepository: matt1398/claude-devtools
Length of output: 558
Add a useEffect to sync viewMode state with isMarkdownFile.
Line 24 initializes viewMode correctly, but it won't update if the linkedTool prop changes after mount (e.g., if the write result is updated with a different file). Since LinkedToolItem is keyed by item.tool.id, the component won't remount on result updates—only the prop changes. Add a useEffect to keep the mode in sync with the file type:
const [viewMode, setViewMode] = React.useState<'code' | 'preview'>(isMarkdownFile ? 'preview' : 'code');
+ React.useEffect(() => {
+ setViewMode(isMarkdownFile ? 'preview' : 'code');
+ }, [isMarkdownFile]);Note: ReadToolViewer has the same pattern and should also receive this sync effect.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/chat/items/linkedTool/WriteToolViewer.tsx` at line
24, The component initializes viewMode based on isMarkdownFile but never updates
it when linkedTool (or its result/file type) changes; add a React.useEffect that
depends on isMarkdownFile (and/or linkedTool/result) to call
setViewMode(isMarkdownFile ? 'preview' : 'code') so the UI switches when the
file type changes; apply the same pattern to ReadToolViewer; target the viewMode
and setViewMode state in WriteToolViewer and ReadToolViewer and ensure the
effect runs when isMarkdownFile (or linkedTool/result) changes.
| highlightClasses={highlightClasses} | ||
| highlightStyle={highlightStyle} | ||
| notificationDotColor={notificationDotColor} | ||
| variant={toolVariant} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check BaseItemProps interface definition
rg -nA 40 'interface BaseItemProps' --type=ts src/renderer/components/chat/items/BaseItem.tsx
# Also check if variant is used anywhere in BaseItem implementation
rg -n 'variant' src/renderer/components/chat/items/BaseItem.tsxRepository: matt1398/claude-devtools
Length of output: 1569
Pass variant to BashToolViewer directly, not to BaseItem.
The BaseItemProps interface does not include a variant property. The code passes variant={toolVariant} to BaseItem on line 175, which will cause a TypeScript error. Move the variant prop to the BashToolViewer component on line 189-190 where it belongs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/chat/items/LinkedToolItem.tsx` at line 175, The code
currently passes variant={toolVariant} into BaseItem but BaseItemProps doesn't
define variant, causing a TS error; remove variant from the BaseItem usage (the
instance that renders BaseItem) and instead pass variant={toolVariant} into the
BashToolViewer component (the BashToolViewer invocation around lines ~189-190).
Update the JSX so BaseItem no longer receives variant and BashToolViewer
receives the toolVariant prop, ensuring the BashToolViewer prop type accepts
variant if needed.
| // Always swap conversation to match this tab — the global conversation | ||
| // may belong to a different tab even when selectedSessionId matches. | ||
| if (hasCachedData) { | ||
| // Swap global state from per-tab cache (no re-fetch) | ||
| set({ | ||
| sessionDetail: cachedTabData.sessionDetail, | ||
| conversation: cachedTabData.conversation, | ||
| conversationLoading: false, | ||
| sessionDetailLoading: false, | ||
| sessionDetailError: null, | ||
| sessionClaudeMdStats: cachedTabData.sessionClaudeMdStats, | ||
| sessionContextStats: cachedTabData.sessionContextStats, | ||
| sessionPhaseInfo: cachedTabData.sessionPhaseInfo, | ||
| visibleAIGroupId: cachedTabData.visibleAIGroupId, | ||
| selectedAIGroup: cachedTabData.selectedAIGroup, | ||
| }); | ||
| } else { | ||
| void get().fetchSessionDetail(foundWorktree, sessionId, tabId); |
There was a problem hiding this comment.
Treat an in-flight tabSessionData entry as cache, not a miss.
Lines 301 and 337 treat conversation != null as the only cache hit. But fetchSessionDetail() creates tabSessionData[tabId] with loading flags before the request resolves in src/renderer/store/slices/sessionDetailSlice.ts Lines 150-172, and only fills conversation after success in src/renderer/store/slices/sessionDetailSlice.ts Lines 428-448. If the user re-activates that tab while the first request is still in flight, this branch fires a second fetch and flips the global loading state again. That duplicates requests and can reintroduce the flicker this PR is trying to remove.
Please branch on the presence/loading state of cachedTabData, or restore the tab’s cached loading state instead of re-fetching whenever conversation is still null.
🛠️ Suggested direction
- const hasCachedData = cachedTabData?.conversation != null;
+ const isTabDataCached = cachedTabData != null;
...
- if (hasCachedData) {
+ if (isTabDataCached) {
set({
sessionDetail: cachedTabData.sessionDetail,
conversation: cachedTabData.conversation,
- conversationLoading: false,
- sessionDetailLoading: false,
- sessionDetailError: null,
+ conversationLoading: cachedTabData.conversationLoading,
+ sessionDetailLoading: cachedTabData.sessionDetailLoading,
+ sessionDetailError: cachedTabData.sessionDetailError,
sessionClaudeMdStats: cachedTabData.sessionClaudeMdStats,
sessionContextStats: cachedTabData.sessionContextStats,
sessionPhaseInfo: cachedTabData.sessionPhaseInfo,
visibleAIGroupId: cachedTabData.visibleAIGroupId,
selectedAIGroup: cachedTabData.selectedAIGroup,
});
} else {
void get().fetchSessionDetail(/* same args as today */);
}Apply the same adjustment in both branches.
Also applies to: 335-352
🧰 Tools
🪛 ESLint
[error] 316-316: Remove this use of the "void" operator.
(sonarjs/void-use)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/store/slices/tabSlice.ts` around lines 299 - 316, Change the
cache-hit logic to treat an existing tabSessionData entry as a cache regardless
of whether conversation is null: if cachedTabData exists (not just
cachedTabData.conversation), call set(...) to restore the cached state's
conversationLoading/sessionDetailLoading/sessionDetailError/sessionClaudeMdStats/sessionContextStats/sessionPhaseInfo/visibleAIGroupId/selectedAIGroup
(preserving their in-flight/loading values) instead of calling
get().fetchSessionDetail(...) which triggers duplicate requests; apply this same
change to both places where the code currently checks cachedTabData.conversation
(the branches that call get().fetchSessionDetail), and only call
fetchSessionDetail when no cachedTabData exists at all.
1 participant
Summary
sessionChangedguard fromsetActiveTab— always swap conversation data when switching tabsselectedSessionIdmatches (multi-tab scenario)Test plan
pnpm typecheckpasses (pre-existing errors inLinkedToolItem.tsxare unrelated)pnpm test— all 652 tests pass🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes