perf: reduce UI lag via render optimization, event consolidation, and debug cleanup#440
Merged
pedramamini merged 10 commits intomainfrom Feb 24, 2026
Merged
perf: reduce UI lag via render optimization, event consolidation, and debug cleanup#440pedramamini merged 10 commits intomainfrom
pedramamini merged 10 commits intomainfrom
Conversation
…splitting, and memoization - Conditionally mount NewInstanceModal, EditAgentModal, PromptComposerModal, ExecutionQueueBrowser (eliminates ~115+ wasted renders when closed) - Wrap all AppModals sub-groups + parent with React.memo - Memoize gitInfo derivation in MainPanel with primitive deps - Remove unused isLoading from GitFileStatusContext (no consumer uses it) - Move inline sessions.find() to useMemo in AppModals - Update test mocks to match new GitFileStatusContextValue interface
Three hooks (useActivityTracker, useHandsOnTimeTracker, useGitStatusPolling) each registered their own keydown/mousedown/wheel/touchstart listeners, totaling 13+ global event listeners all doing the same thing. Consolidated into a single shared activity bus with 5 passive listeners. Also fixes showSessionJumpNumbers re-registering 3 listeners on every modifier key press/release (deps array included state → now uses ref), and converts canSummarizeActiveTab from eager IIFE to lazy getter.
Remove 7 console.log calls from useSessionDebounce that fired at high frequency during AI streaming. Replace index-based keys with stable identifiers in 6 components where items can be removed or filtered (staged images, diff tabs, log entries, quit modal agents).
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a shared activity bus; memoizes and conditionalizes modal renderers; centralizes extension color logic; stabilizes React keys and removes index-based removals; drops git-status Changes
Sequence Diagram(s)sequenceDiagram
participant Hook as Activity Hook
participant Bus as Activity Bus
participant DOM as Global Listeners
participant CB as Subscriber Callback
Note over Hook,Bus: Subscription flow
Hook->>Bus: subscribeToActivity(callback)
activate Bus
Bus->>DOM: attach passive listeners (keydown/mousedown/wheel/touchstart/click)
Bus->>CB: register callback
deactivate Bus
Note over DOM,Bus: On user activity
DOM->>Bus: handleActivity(event)
Bus->>CB: dispatch(event) to registered callbacks
CB->>Hook: onActivity invoked
Note over Hook,Bus: Unsubscribe flow
Hook->>Bus: unsubscribe()
activate Bus
Bus->>CB: deregister callback
Bus->>DOM: if no subscribers -> detach listeners
deactivate Bus
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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 unit tests (beta)
Comment |
Greptile SummaryThis PR implements comprehensive performance optimizations focused on reducing UI lag during AI streaming and high-frequency user interactions. Major changes:
All test files have been updated to reflect the new patterns. The changes align with performance best practices from Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Before["Before: 13+ Event Listeners"]
UAT[useActivityTracker]
UHOT[useHandsOnTimeTracker]
UGSP[useGitStatusPolling]
UAT -->|keydown| W1[Window Event]
UAT -->|mousedown| W2[Window Event]
UAT -->|wheel| W3[Window Event]
UAT -->|touchstart| W4[Window Event]
UHOT -->|keydown| W5[Window Event]
UHOT -->|mousedown| W6[Window Event]
UHOT -->|wheel| W7[Window Event]
UHOT -->|touchstart| W8[Window Event]
UHOT -->|click| W9[Window Event]
UGSP -->|keydown| W10[Window Event]
UGSP -->|mousedown| W11[Window Event]
UGSP -->|wheel| W12[Window Event]
UGSP -->|touchstart| W13[Window Event]
end
subgraph After["After: Shared Activity Bus"]
AB[activityBus.ts]
AB -->|passive: true| E1[keydown]
AB -->|passive: true| E2[mousedown]
AB -->|passive: true| E3[wheel]
AB -->|passive: true| E4[touchstart]
AB -->|passive: true| E5[click]
H1[useActivityTracker] -->|subscribe| AB
H2[useHandsOnTimeTracker] -->|subscribe| AB
H3[useGitStatusPolling] -->|subscribe| AB
AB -->|notify| H1
AB -->|notify| H2
AB -->|notify| H3
end
style Before fill:#fee,stroke:#f66
style After fill:#efe,stroke:#6f6
style AB fill:#ffd,stroke:#cc6
Last reviewed commit: 0e76d33 |
![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
🤖 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/QuitConfirmModal.tsx`:
- Around line 147-149: The key for the mapped elements in displayNames.map
currently uses only the agent name (key={name}), which can produce duplicate
keys if names repeat; change the key to a composite that guarantees uniqueness
(e.g., include the map index or a unique id) so React can track items reliably —
update the mapping in displayNames.map and the span key to something like a
fallback composite (e.g., `${name}-${index}`) or use the agent's unique id if
available.
Update test queries to handle compact + full responsive views that both render in DOM (hidden via CSS). Use getAllByText for ambiguous text matches in GitStatusWidget and MainPanel. Flush rAF callback in WizardConversationView to reset programmatic scroll guard before simulating user scroll.
…ottom Previously, auto-scrolling to show new content required the autoScrollAiMode preference to be enabled. Now, if the user is already at the bottom of the chat stream, new content (including their own sent messages) will automatically scroll into view regardless of the preference setting.
Consolidate duplicate getExtensionColor from TabBar and TabSwitcherModal into shared utility. Add 6 missing categories (images, Java, C/C++, Ruby, SQL, PDF/Office). Default fallback now derives from theme accent instead of invisible border/dim colors.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/components/TabSwitcherModal.tsx (1)
109-116:⚠️ Potential issue | 🟡 MinorRemove orphaned JSDoc comment left behind from the deleted inline function.
Lines 109–113 were the docblock for the in-file
getExtensionColorthat has been removed and replaced with an import. The block now floats immediately before theContextGaugeJSDoc (lines 114–116), resulting in two back-to-back JSDoc blocks with no declaration between them.🧹 Proposed fix
-/** - * Get color for file extension badge. - * Returns a muted color based on file type for visual differentiation. - * When colorBlindMode is enabled, uses Wong's colorblind-safe palette. - * (Synchronized with TabBar.tsx for consistency) - */ /** * Circular progress gauge component */ function ContextGauge({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/TabSwitcherModal.tsx` around lines 109 - 116, Remove the orphaned JSDoc block that described getExtensionColor (the docblock now sitting immediately before the ContextGauge JSDoc) so there aren't two back-to-back JSDoc comments with no declaration; locate the dangling comment that references getExtensionColor and delete it now that getExtensionColor is imported, leaving only the ContextGauge JSDoc above its declaration.
🧹 Nitpick comments (1)
src/renderer/constants/colorblindPalettes.ts (1)
176-205: Heads-up: five new categories reuse existing colorblind-palette colors.The new assignments share exact
rgbavalues with existing categories:
New category Shared color with imagerust(both#EE3377Magenta)javahtml(both#CC3311Vermillion)rubyconfig(both#EE7733Orange)datastyles(both#AA4499Purple)documentgo(both#44AA99Blue-Green)With only 8–10 distinct hues in Wong's palette and 14 categories, perfect uniqueness is impossible, so this is an inherent trade-off. Worth documenting in the JSDoc that these pairings are intentional budget allocations rather than oversights, so future contributors don't try to "fix" them by adding visually similar non-Wong colors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/constants/colorblindPalettes.ts` around lines 176 - 205, The new category color assignments (image, java, ruby, data, document) intentionally reuse existing palette hues (matching rust, html, config, styles, go) due to the limited 8–10 distinct Wong hues; add a JSDoc block at the top of the colorblindPalettes export (in src/renderer/constants/colorblindPalettes.ts) noting that these pairings are deliberate "budget allocations" rather than oversights, list the specific reused mappings (image→rust, java→html, ruby→config, data→styles, document→go), and explain that perfect uniqueness is impossible with Wong's palette so future contributors should not change colors to non-Wong variants.
🤖 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/utils/extensionColors.ts`:
- Around line 171-183: When colorBlindMode is true, the code should not fall
through to the EXTENSION_MAP lookup if getColorBlindExtensionColor(extension,
isLightTheme) returns null; change the logic in the block using
colorBlindMode/getColorBlindExtensionColor so that if cbColors is falsy you
immediately return the accent-based default (the same fallback used when no
EXTENSION_MAP entry exists) instead of continuing to the EXTENSION_MAP lookup,
referencing getColorBlindExtensionColor, EXTENSION_MAP and EXTENSION_COLORS to
locate where to return the accent default; alternatively, if you prefer not to
change control flow, update the misleading comment to state that a missing
color-blind palette will fall back to the EXTENSION_MAP lookup.
---
Outside diff comments:
In `@src/renderer/components/TabSwitcherModal.tsx`:
- Around line 109-116: Remove the orphaned JSDoc block that described
getExtensionColor (the docblock now sitting immediately before the ContextGauge
JSDoc) so there aren't two back-to-back JSDoc comments with no declaration;
locate the dangling comment that references getExtensionColor and delete it now
that getExtensionColor is imported, leaving only the ContextGauge JSDoc above
its declaration.
---
Nitpick comments:
In `@src/renderer/constants/colorblindPalettes.ts`:
- Around line 176-205: The new category color assignments (image, java, ruby,
data, document) intentionally reuse existing palette hues (matching rust, html,
config, styles, go) due to the limited 8–10 distinct Wong hues; add a JSDoc
block at the top of the colorblindPalettes export (in
src/renderer/constants/colorblindPalettes.ts) noting that these pairings are
deliberate "budget allocations" rather than oversights, list the specific reused
mappings (image→rust, java→html, ruby→config, data→styles, document→go), and
explain that perfect uniqueness is impossible with Wong's palette so future
contributors should not change colors to non-Wong variants.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/__tests__/renderer/components/TerminalOutput.test.tsxsrc/__tests__/renderer/utils/extensionColors.test.tssrc/renderer/components/TabBar.tsxsrc/renderer/components/TabSwitcherModal.tsxsrc/renderer/components/TerminalOutput.tsxsrc/renderer/constants/colorblindPalettes.tssrc/renderer/utils/extensionColors.ts
Resolved 10 merge conflicts across 4 files: - GitStatusWidget.test.tsx: kept dual-render getAllByText assertions - MainPanel.test.tsx: kept regex-based context widget queries - WizardInputPanel.tsx: kept accessibility button wrapper from react-doctor - InputArea.tsx: kept accessibility button wrapper from react-doctor
- QuitConfirmModal: use composite key `${name}-${index}` to prevent
duplicate React keys when agents share display names
- extensionColors: early-return accent fallback in colorblind mode so
we never serve non-colorblind-safe colors from EXTENSION_MAP
Tests were asserting theme.colors.border for unknown extension badges, but the extensionColors utility now derives from theme accent to ensure all pills are visible. Updated assertions to match.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/__tests__/renderer/components/MainPanel.test.tsx (1)
2390-2418:⚠️ Potential issue | 🟡 MinorTest documents a
console.errorswallow that violates the Sentry reporting guideline.The test asserts
console.erroris called whenclipboard.writeTextrejects, which means the underlyingMainPanelcomponent silently catches the clipboard error and logs it. Per coding guidelines, unexpected/unhandled errors must not be absorbed bytry/catch→console.error; they should be reported viacaptureExceptionfromsrc/utils/sentry.The fix belongs in
MainPanel's clipboard handler, and this test should then be updated to assertcaptureExceptionwas called (or let the error propagate if it is truly unrecoverable). As per coding guidelines: "Do NOT silently swallow exceptions with try-catch-console.error blocks. Use Sentry utilities (captureException, captureMessage) from 'src/utils/sentry' for explicit error reporting with context."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/MainPanel.test.tsx` around lines 2390 - 2418, The clipboard error is currently swallowed with console.error inside MainPanel's clipboard handler; update the handler (e.g., the copy/onCopy/handleCopy method in the MainPanel component) to call captureException from 'src/utils/sentry' (import it at the top) instead of console.error (or rethrow if unrecoverable), passing the caught error and relevant context; then update the test to mock and assert that captureException was called (or adjust the test to expect the error to propagate) instead of asserting console.error.
🤖 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/utils/extensionColors.ts`:
- Around line 163-206: getExtensionColor currently lowercases the extension only
in the non-colorblind path, so in colorBlindMode calls to
getColorBlindExtensionColor(extension, ...) can miss matches; change the
function to normalize the incoming extension to a lowercase form (e.g., compute
const ext = extension.toLowerCase() at the top) and pass that ext into
getColorBlindExtensionColor and use ext for the EXTENSION_MAP lookup so both
color-blind and regular paths use the same normalized key.
---
Outside diff comments:
In `@src/__tests__/renderer/components/MainPanel.test.tsx`:
- Around line 2390-2418: The clipboard error is currently swallowed with
console.error inside MainPanel's clipboard handler; update the handler (e.g.,
the copy/onCopy/handleCopy method in the MainPanel component) to call
captureException from 'src/utils/sentry' (import it at the top) instead of
console.error (or rethrow if unrecoverable), passing the caught error and
relevant context; then update the test to mock and assert that captureException
was called (or adjust the test to expect the error to propagate) instead of
asserting console.error.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/__tests__/renderer/components/GitStatusWidget.test.tsxsrc/__tests__/renderer/components/InlineWizard/WizardConversationView.test.tsxsrc/__tests__/renderer/components/MainPanel.test.tsxsrc/renderer/App.tsxsrc/renderer/components/InlineWizard/WizardInputPanel.tsxsrc/renderer/components/InputArea.tsxsrc/renderer/components/MainPanel.tsxsrc/renderer/components/QuitConfirmModal.tsxsrc/renderer/components/TabSwitcherModal.tsxsrc/renderer/components/TerminalOutput.tsxsrc/renderer/hooks/session/useHandsOnTimeTracker.tssrc/renderer/utils/extensionColors.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- src/renderer/components/InlineWizard/WizardInputPanel.tsx
- src/renderer/components/TerminalOutput.tsx
- src/tests/renderer/components/GitStatusWidget.test.tsx
- src/renderer/App.tsx
- src/tests/renderer/components/InlineWizard/WizardConversationView.test.tsx
- src/renderer/components/QuitConfirmModal.tsx
- src/renderer/components/TabSwitcherModal.tsx
- src/renderer/components/InputArea.tsx
Comment on lines
+163
to
+206
| export function getExtensionColor( | ||
| extension: string, | ||
| theme: Theme, | ||
| colorBlindMode?: boolean | ||
| ): { bg: string; text: string } { | ||
| const isLightTheme = theme.mode === 'light'; | ||
|
|
||
| // Colorblind-safe path — never fall through to non-colorblind-safe colors | ||
| if (colorBlindMode) { | ||
| const cbColors = getColorBlindExtensionColor(extension, isLightTheme); | ||
| if (cbColors) return cbColors; | ||
| // Unknown extension in colorblind mode: skip the regular palette and | ||
| // jump straight to the theme accent so we never serve unsafe colors. | ||
| const accentRgb = hexToRgb(theme.colors.accent); | ||
| if (accentRgb) { | ||
| return isLightTheme | ||
| ? { bg: `rgba(${accentRgb.r}, ${accentRgb.g}, ${accentRgb.b}, 0.15)`, text: `rgba(${accentRgb.r}, ${accentRgb.g}, ${accentRgb.b}, 0.9)` } | ||
| : { bg: `rgba(${accentRgb.r}, ${accentRgb.g}, ${accentRgb.b}, 0.3)`, text: `rgba(${accentRgb.r}, ${accentRgb.g}, ${accentRgb.b}, 0.9)` }; | ||
| } | ||
| return isLightTheme | ||
| ? { bg: 'rgba(107, 114, 128, 0.15)', text: 'rgba(75, 85, 99, 0.9)' } | ||
| : { bg: 'rgba(156, 163, 175, 0.3)', text: 'rgba(209, 213, 219, 0.9)' }; | ||
| } | ||
|
|
||
| // Look up extension in the map | ||
| const ext = extension.toLowerCase(); | ||
| const category = EXTENSION_MAP[ext]; | ||
| if (category) { | ||
| const colors = EXTENSION_COLORS[category]; | ||
| return isLightTheme ? colors.light : colors.dark; | ||
| } | ||
|
|
||
| // Default: derive from theme accent so every pill is visible | ||
| const rgb = hexToRgb(theme.colors.accent); | ||
| if (rgb) { | ||
| return isLightTheme | ||
| ? { bg: `rgba(${rgb.r}, ${rgb.g}, ${rgb.b}, 0.15)`, text: `rgba(${rgb.r}, ${rgb.g}, ${rgb.b}, 0.9)` } | ||
| : { bg: `rgba(${rgb.r}, ${rgb.g}, ${rgb.b}, 0.3)`, text: `rgba(${rgb.r}, ${rgb.g}, ${rgb.b}, 0.9)` }; | ||
| } | ||
|
|
||
| // Ultimate fallback (non-hex accent like rgb/hsl) — still visible | ||
| return isLightTheme | ||
| ? { bg: 'rgba(107, 114, 128, 0.15)', text: 'rgba(75, 85, 99, 0.9)' } | ||
| : { bg: 'rgba(156, 163, 175, 0.3)', text: 'rgba(209, 213, 219, 0.9)' }; |
There was a problem hiding this comment.
Normalize extension case for color-blind path too.
Right now, only the non-colorblind path lowercases the extension. If the caller passes .TS/.JPG, getColorBlindExtensionColor may miss and fall back to the accent, losing consistent per-type coloring.
🛠️ Proposed fix
export function getExtensionColor(
extension: string,
theme: Theme,
colorBlindMode?: boolean
): { bg: string; text: string } {
const isLightTheme = theme.mode === 'light';
+ const ext = extension.toLowerCase();
// Colorblind-safe path — never fall through to non-colorblind-safe colors
if (colorBlindMode) {
- const cbColors = getColorBlindExtensionColor(extension, isLightTheme);
+ const cbColors = getColorBlindExtensionColor(ext, isLightTheme);
if (cbColors) return cbColors;
// Unknown extension in colorblind mode: skip the regular palette and
// jump straight to the theme accent so we never serve unsafe colors.
const accentRgb = hexToRgb(theme.colors.accent);
@@
// Look up extension in the map
- const ext = extension.toLowerCase();
const category = EXTENSION_MAP[ext];📝 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.
Suggested change
| export function getExtensionColor( | |
| extension: string, | |
| theme: Theme, | |
| colorBlindMode?: boolean | |
| ): { bg: string; text: string } { | |
| const isLightTheme = theme.mode === 'light'; | |
| // Colorblind-safe path — never fall through to non-colorblind-safe colors | |
| if (colorBlindMode) { | |
| const cbColors = getColorBlindExtensionColor(extension, isLightTheme); | |
| if (cbColors) return cbColors; | |
| // Unknown extension in colorblind mode: skip the regular palette and | |
| // jump straight to the theme accent so we never serve unsafe colors. | |
| const accentRgb = hexToRgb(theme.colors.accent); | |
| if (accentRgb) { | |
| return isLightTheme | |
| ? { bg: `rgba(${accentRgb.r}, ${accentRgb.g}, ${accentRgb.b}, 0.15)`, text: `rgba(${accentRgb.r}, ${accentRgb.g}, ${accentRgb.b}, 0.9)` } | |
| : { bg: `rgba(${accentRgb.r}, ${accentRgb.g}, ${accentRgb.b}, 0.3)`, text: `rgba(${accentRgb.r}, ${accentRgb.g}, ${accentRgb.b}, 0.9)` }; | |
| } | |
| return isLightTheme | |
| ? { bg: 'rgba(107, 114, 128, 0.15)', text: 'rgba(75, 85, 99, 0.9)' } | |
| : { bg: 'rgba(156, 163, 175, 0.3)', text: 'rgba(209, 213, 219, 0.9)' }; | |
| } | |
| // Look up extension in the map | |
| const ext = extension.toLowerCase(); | |
| const category = EXTENSION_MAP[ext]; | |
| if (category) { | |
| const colors = EXTENSION_COLORS[category]; | |
| return isLightTheme ? colors.light : colors.dark; | |
| } | |
| // Default: derive from theme accent so every pill is visible | |
| const rgb = hexToRgb(theme.colors.accent); | |
| if (rgb) { | |
| return isLightTheme | |
| ? { bg: `rgba(${rgb.r}, ${rgb.g}, ${rgb.b}, 0.15)`, text: `rgba(${rgb.r}, ${rgb.g}, ${rgb.b}, 0.9)` } | |
| : { bg: `rgba(${rgb.r}, ${rgb.g}, ${rgb.b}, 0.3)`, text: `rgba(${rgb.r}, ${rgb.g}, ${rgb.b}, 0.9)` }; | |
| } | |
| // Ultimate fallback (non-hex accent like rgb/hsl) — still visible | |
| return isLightTheme | |
| ? { bg: 'rgba(107, 114, 128, 0.15)', text: 'rgba(75, 85, 99, 0.9)' } | |
| : { bg: 'rgba(156, 163, 175, 0.3)', text: 'rgba(209, 213, 219, 0.9)' }; | |
| export function getExtensionColor( | |
| extension: string, | |
| theme: Theme, | |
| colorBlindMode?: boolean | |
| ): { bg: string; text: string } { | |
| const isLightTheme = theme.mode === 'light'; | |
| const ext = extension.toLowerCase(); | |
| // Colorblind-safe path — never fall through to non-colorblind-safe colors | |
| if (colorBlindMode) { | |
| const cbColors = getColorBlindExtensionColor(ext, isLightTheme); | |
| if (cbColors) return cbColors; | |
| // Unknown extension in colorblind mode: skip the regular palette and | |
| // jump straight to the theme accent so we never serve unsafe colors. | |
| const accentRgb = hexToRgb(theme.colors.accent); | |
| if (accentRgb) { | |
| return isLightTheme | |
| ? { bg: `rgba(${accentRgb.r}, ${accentRgb.g}, ${accentRgb.b}, 0.15)`, text: `rgba(${accentRgb.r}, ${accentRgb.g}, ${accentRgb.b}, 0.9)` } | |
| : { bg: `rgba(${accentRgb.r}, ${accentRgb.g}, ${accentRgb.b}, 0.3)`, text: `rgba(${accentRgb.r}, ${accentRgb.g}, ${accentRgb.b}, 0.9)` }; | |
| } | |
| return isLightTheme | |
| ? { bg: 'rgba(107, 114, 128, 0.15)', text: 'rgba(75, 85, 99, 0.9)' } | |
| : { bg: 'rgba(156, 163, 175, 0.3)', text: 'rgba(209, 213, 219, 0.9)' }; | |
| } | |
| // Look up extension in the map | |
| const category = EXTENSION_MAP[ext]; | |
| if (category) { | |
| const colors = EXTENSION_COLORS[category]; | |
| return isLightTheme ? colors.light : colors.dark; | |
| } | |
| // Default: derive from theme accent so every pill is visible | |
| const rgb = hexToRgb(theme.colors.accent); | |
| if (rgb) { | |
| return isLightTheme | |
| ? { bg: `rgba(${rgb.r}, ${rgb.g}, ${rgb.b}, 0.15)`, text: `rgba(${rgb.r}, ${rgb.g}, ${rgb.b}, 0.9)` } | |
| : { bg: `rgba(${rgb.r}, ${rgb.g}, ${rgb.b}, 0.3)`, text: `rgba(${rgb.r}, ${rgb.g}, ${rgb.b}, 0.9)` }; | |
| } | |
| // Ultimate fallback (non-hex accent like rgb/hsl) — still visible | |
| return isLightTheme | |
| ? { bg: 'rgba(107, 114, 128, 0.15)', text: 'rgba(75, 85, 99, 0.9)' } | |
| : { bg: 'rgba(156, 163, 175, 0.3)', text: 'rgba(209, 213, 219, 0.9)' }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/utils/extensionColors.ts` around lines 163 - 206,
getExtensionColor currently lowercases the extension only in the non-colorblind
path, so in colorBlindMode calls to getColorBlindExtensionColor(extension, ...)
can miss matches; change the function to normalize the incoming extension to a
lowercase form (e.g., compute const ext = extension.toLowerCase() at the top)
and pass that ext into getColorBlindExtensionColor and use ext for the
EXTENSION_MAP lookup so both color-blind and regular paths use the same
normalized key.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/__tests__/renderer/components/TabBar.test.tsx (1)
4816-4817: LGTM — verify hardcoded color stays in sync withdarkTheme.colors.accent
rgba(189, 147, 249, 0.3)correctly matchesdarkTheme.colors.accent = '#bd93f9'at 0.3 opacity. The value is hardcoded, so ifgetExtensionColor's opacity factor or the Dracula accent ever changes, this test silently diverges. Deriving it inline (e.g. parsing the hex and embedding the channel values fromdarkTheme.colors.accent) would make the coupling explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/TabBar.test.tsx` around lines 4816 - 4817, The test hardcodes the RGBA value for unknown extensions; update the assertion to derive the expected color from darkTheme.colors.accent so it stays in sync with getExtensionColor's opacity; in TabBar.test.tsx import or reference darkTheme.colors.accent and compute the expected string by converting the hex (darkTheme.colors.accent) to an rgba with alpha 0.3 (you can add a small helper hexToRgba used only in the test), then replace the hardcoded 'rgba(189, 147, 249, 0.3)' in the expect(badge).toHaveStyle(...) assertion with the computed expected value so badge validation uses the derived color.
🤖 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/__tests__/renderer/components/TabBar.test.tsx`:
- Around line 5271-5272: The colorblind-mode unknown-extension fallback uses
opacity 0.3 instead of the 0.35 used by all other colorblind dark-mode badges;
update the fallback in getExtensionColor (the unknown-extension branch in
src/renderer/utils/extensionColors.ts) to use 0.35 for dark colorblind mode, and
adjust the test expectation in TabBar.test.tsx (the assertion that checks badge
backgroundColor) to expect 'rgba(189, 147, 249, 0.35)'.
---
Nitpick comments:
In `@src/__tests__/renderer/components/TabBar.test.tsx`:
- Around line 4816-4817: The test hardcodes the RGBA value for unknown
extensions; update the assertion to derive the expected color from
darkTheme.colors.accent so it stays in sync with getExtensionColor's opacity; in
TabBar.test.tsx import or reference darkTheme.colors.accent and compute the
expected string by converting the hex (darkTheme.colors.accent) to an rgba with
alpha 0.3 (you can add a small helper hexToRgba used only in the test), then
replace the hardcoded 'rgba(189, 147, 249, 0.3)' in the
expect(badge).toHaveStyle(...) assertion with the computed expected value so
badge validation uses the derived color.
Comment on lines
+5271
to
+5272
| // Colorblind mode also uses accent-derived fallback for unknown extensions | ||
| expect(badge).toHaveStyle({ backgroundColor: 'rgba(189, 147, 249, 0.3)' }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find files related to extension colors
fd -t f -e ts -e tsx -e js -e jsx | xargs rg -l "getExtensionColor" | head -20Repository: RunMaestro/Maestro
Length of output: 277
🏁 Script executed:
#!/bin/bash
# Search for the getExtensionColor function definition
rg -n "getExtensionColor\s*=|function getExtensionColor" --type ts --type tsx -B2 -A15 | head -100Repository: RunMaestro/Maestro
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Look for opacity/colorblind related constants and logic
rg -n "0\.35|0\.3\b" --type ts --type tsx -B3 -A3 | grep -E "(colorblind|opacity|0\.3|0\.35)" | head -50Repository: RunMaestro/Maestro
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Check the test file at the specific lines to understand context
sed -n '5260,5280p' src/__tests__/renderer/components/TabBar.test.tsxRepository: RunMaestro/Maestro
Length of output: 700
🏁 Script executed:
#!/bin/bash
# Read the extensionColors implementation
cat -n src/renderer/utils/extensionColors.tsRepository: RunMaestro/Maestro
Length of output: 9570
🏁 Script executed:
#!/bin/bash
# Read colorblind palettes constants
cat -n src/renderer/constants/colorblindPalettes.tsRepository: RunMaestro/Maestro
Length of output: 12087
🏁 Script executed:
#!/bin/bash
# Check the test file for extensionColors to see expected values
cat -n src/__tests__/renderer/utils/extensionColors.test.tsRepository: RunMaestro/Maestro
Length of output: 5954
Colorblind-mode unknown-extension fallback uses 0.3 opacity — inconsistent with every other colorblind dark-mode badge (0.35)
Every colorblind-mode known-extension badge in dark mode uses 0.35 opacity (TypeScript, Markdown, JSON, CSS, Python, Rust, Go, Shell, Image, Java, C++, Ruby, SQL, and others all use 0.35). The unknown-extension fallback in getExtensionColor (line 180 of src/renderer/utils/extensionColors.ts), however, hardcodes 0.3. This fallback branch should apply the same 0.35 opacity multiplier used by the colorblind-safe palette for all known extensions.
Update the fallback to use 0.35 instead of 0.3 for dark mode in colorblind mode, and update the test expectation accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/__tests__/renderer/components/TabBar.test.tsx` around lines 5271 - 5272,
The colorblind-mode unknown-extension fallback uses opacity 0.3 instead of the
0.35 used by all other colorblind dark-mode badges; update the fallback in
getExtensionColor (the unknown-extension branch in
src/renderer/utils/extensionColors.ts) to use 0.35 for dark colorblind mode, and
adjust the test expectation in TabBar.test.tsx (the assertion that checks badge
backgroundColor) to expect 'rgba(189, 147, 249, 0.35)'.
…ertions) Adopted origin/main's more precise within()-scoped test assertions for GitStatusWidget, targeting .header-git-status-full and .header-git-status-compact CSS classes instead of broad getAllByText.
reachraza
added a commit
that referenced
this pull request
Feb 25, 2026
…se 3 patterns - Resolved 3 conflicts (App.tsx, SessionList.tsx, useSessionListProps.ts): keep Phase 3 direct store subscriptions; add only handleArchiveGroupChat handler - groupChatStates, allGroupChatParticipantStates, contextWarning thresholds already read from Zustand stores inside SessionList — not re-added as props - Fixed unused React import in SshRemoteIgnoreSection.tsx (introduced by main) - Updated groupChat handler registration test for new groupChat:archive IPC channel New features from main: - feat: archive state for group chat (#377) - feat: worktree from Auto Run (WorktreeRunSection in BatchRunnerModal) - perf: render optimization, event consolidation, debug cleanup (#440) - fix: hide file path on scroll in file preview header (#448) - fix: unstable useSessionStore filter selector, worktree detection, PR creation - docs: Encore Features section, performance profiling guide
This was referenced Feb 26, 2026
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
2 participants
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
AppModals,MainPanel,GitStatusContext,settingsStore)activityBus.ts), reducing per-keystroke overhead from 13 handlers to 1useSessionDebounce(7console.logcalls firing at high frequency during AI streaming)Test plan
npx vitest run— existing tests pass (3 pre-existing failures from missinggenerated/promptsmodule, unrelated)Summary by CodeRabbit