fix(desktop): improve light theme terminal color contrast by Kitenite · Pull Request #475 · superset-sh/superset

Kitenite · 2025-12-22T23:12:13Z

Summary

Fix invisible text in terminal when using light theme (especially in Claude Code diffs)
Update brightWhite from #ffffff to #1a1a1a - was completely invisible on white background
Darken all ANSI colors for proper contrast on light backgrounds

Test plan

Switch to light theme in desktop app
Run Claude Code and trigger a diff (e.g., make a code change)
Verify all diff text is visible and readable
Check that other terminal colors (red for removals, green for additions) have good contrast

🤖 Generated with Claude Code

Summary by CodeRabbit

Style
- Refined light terminal color palette for improved contrast and readability on light backgrounds; adjusted standard and bright colors, cursors, and selections.
New Features
- Terminal sessions can now accept a dark/light theme preference and will set a matching terminal environment variable.
- Light theme exposes extended 256-color ANSI palette and an inactive selection background for richer, more consistent terminal rendering.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Fix invisible text in terminal diffs when using light theme. The previous colors (especially brightWhite: #ffffff) were invisible on the white background. Updated all ANSI colors to darker variants optimized for light background readability. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

coderabbitai · 2025-12-22T23:12:22Z

Walkthrough

Adds light-theme extended ANSI colors and selectionInactiveBackground, introduces an optional themeType ("dark" | "light") that is threaded through the renderer, trpc router, session and env builders, and exposes COLORFGBG in the terminal environment for light/dark detection.

Changes

Cohort / File(s)	Summary
Light theme ANSI & extended set `apps/desktop/src/shared/themes/built-in/light.ts`	Adds internal `generateLightExtendedAnsi()` to produce extended ANSI (16–255), updates standard and bright ANSI values, adjusts background/foreground/cursor/selection colors, and exposes `extendedAnsi` and `selectionInactiveBackground` in the light theme object.
Theme types & mapping updates `apps/desktop/src/shared/themes/types.ts`, `apps/desktop/src/renderer/stores/theme/utils/terminal-theme.ts`	Adds optional `selectionInactiveBackground?: string` and `extendedAnsi?: string[]` to `TerminalColors`; `toXtermTheme` now includes `selectionInactiveBackground` and `extendedAnsi` in the returned theme mapping.
Terminal session / env / router plumbing `apps/desktop/src/main/lib/terminal/env.ts`, `apps/desktop/src/main/lib/terminal/session.ts`, `apps/desktop/src/main/lib/terminal/types.ts`, `apps/desktop/src/lib/trpc/routers/terminal/terminal.ts`	Adds optional `themeType?: "dark"
Renderer: Terminal component theme propagation `apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx`	Reads current theme via `useTheme`, tracks `theme.type` in a ref, and passes `themeType` into terminal create/attach and restart flows so terminals are created/reattached with the preferred theme.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Settings + Themes #158 — Modifies the built-in light theme and terminal ANSI mapping; strongly related to the light-theme palette and extendedAnsi additions.
xterm ui #115 — Touches terminal router/session plumbing and theme/terminal initialization; related to adding themeType through create/attach flows.
implement more font and xterm rendering improvements #324 — Updates terminal session/env/type surfaces; related to COLORFGBG and session/env plumbing changes.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title clearly summarizes the main change: improving light theme terminal color contrast, which is the core objective across all modified files.
Description check	✅ Passed	The description includes a clear summary, test plan checklist, and context about the fix. However, it lacks explicit sections for 'Related Issues', 'Type of Change', and 'Additional Notes' as specified in the template.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch naughty-butterfly-133510

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c51199f and da897cf.

📒 Files selected for processing (8)

apps/desktop/src/lib/trpc/routers/terminal/terminal.ts
apps/desktop/src/main/lib/terminal/env.ts
apps/desktop/src/main/lib/terminal/session.ts
apps/desktop/src/main/lib/terminal/types.ts
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/renderer/stores/theme/utils/terminal-theme.ts
apps/desktop/src/shared/themes/built-in/light.ts
apps/desktop/src/shared/themes/types.ts

🚧 Files skipped from review as they are similar to previous changes (1)

apps/desktop/src/shared/themes/built-in/light.ts

🧰 Additional context used

📓 Path-based instructions (10)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Avoid using any type in TypeScript - maintain type safety unless absolutely necessary

Files:

apps/desktop/src/renderer/stores/theme/utils/terminal-theme.ts
apps/desktop/src/main/lib/terminal/types.ts
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts
apps/desktop/src/main/lib/terminal/env.ts
apps/desktop/src/main/lib/terminal/session.ts
apps/desktop/src/shared/themes/types.ts

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix

Files:

apps/desktop/src/renderer/stores/theme/utils/terminal-theme.ts
apps/desktop/src/main/lib/terminal/types.ts
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts
apps/desktop/src/main/lib/terminal/env.ts
apps/desktop/src/main/lib/terminal/session.ts
apps/desktop/src/shared/themes/types.ts

apps/desktop/src/renderer/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)

Files:

apps/desktop/src/renderer/stores/theme/utils/terminal-theme.ts
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx

apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers

Files:

apps/desktop/src/renderer/stores/theme/utils/terminal-theme.ts
apps/desktop/src/main/lib/terminal/types.ts
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/main/lib/terminal/env.ts
apps/desktop/src/main/lib/terminal/session.ts

apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc
Use alias as defined in tsconfig.json when possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from @trpc/server/observable instead of async generators, as the library explicitly checks isObservable(result) and throws an error otherwise

Files:

apps/desktop/src/renderer/stores/theme/utils/terminal-theme.ts
apps/desktop/src/main/lib/terminal/types.ts
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts
apps/desktop/src/main/lib/terminal/env.ts
apps/desktop/src/main/lib/terminal/session.ts
apps/desktop/src/shared/themes/types.ts

apps/desktop/src/main/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()

Files:

apps/desktop/src/main/lib/terminal/types.ts
apps/desktop/src/main/lib/terminal/env.ts
apps/desktop/src/main/lib/terminal/session.ts

**/{components,features}/**/[!.]*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export

Files:

apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx

**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them

Files:

apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx

**/*.{tsx,css}

📄 CodeRabbit inference engine (AGENTS.md)

Use React + TailwindCSS v4 + shadcn/ui for UI development

Files:

apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx

**/{components,features}/**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file

Files:

apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx

🧠 Learnings (1)

📚 Learning: 2025-12-21T04:39:28.543Z

Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`

Applied to files:

apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx

🧬 Code graph analysis (2)

apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (2)

apps/desktop/src/renderer/stores/theme/index.ts (1)

useTheme (5-5)

apps/desktop/src/renderer/stores/theme/store.ts (1)

useTheme (195-195)

apps/desktop/src/main/lib/terminal/env.ts (1)

apps/desktop/src/resources/public/theme-boot.js (1)

themeType (4-4)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Build

🔇 Additional comments (9)

apps/desktop/src/main/lib/terminal/types.ts (1)

56-57: LGTM!

The optional themeType parameter is well-defined with a clear JSDoc comment explaining its purpose for light mode detection via COLORFGBG.

apps/desktop/src/main/lib/terminal/env.ts (1)

74-74: LGTM!

The themeType parameter is properly integrated into the environment builder. The COLORFGBG logic is clear and well-documented, with appropriate defaults for dark mode when themeType is not provided.

Also applies to: 84-84, 91-103

apps/desktop/src/main/lib/terminal/session.ts (1)

76-76: LGTM!

The themeType is correctly destructured from session parameters and passed through to the environment builder, maintaining consistency in the theme propagation flow.

Also applies to: 92-92

apps/desktop/src/shared/themes/types.ts (1)

83-83: LGTM!

The new optional properties extend terminal color support beyond the standard 16-color palette while maintaining backward compatibility. The comment on line 105 clearly documents the extended ANSI color range.

Also applies to: 105-106

apps/desktop/src/lib/trpc/routers/terminal/terminal.ts (2)

49-49: LGTM!

The themeType is correctly destructured from the input and passed through to terminalManager.createOrAttach, properly propagating the theme preference into the terminal session creation flow.

Also applies to: 75-75

37-37: No changes needed for Zod 4 enum syntax.

The array literal syntax z.enum(["dark", "light"]) is the correct and recommended approach in Zod 4. In Zod 4, z.enum() supports array literals, object literals, and TypeScript enums directly. The code is syntactically correct. The only breaking change in Zod 4 regarding enums was that z.nativeEnum() was deprecated in favor of z.enum(), which now has overloaded support for TypeScript enum objects as input, but this does not affect the array literal syntax being used here.

Likely an incorrect or invalid review comment.

apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (2)

11-11: LGTM!

The addition of useTheme and themeTypeRef correctly tracks the active theme type without causing terminal recreation on theme changes. The ref pattern prevents stale closure issues.

Also applies to: 52-52, 57-59

265-265: LGTM!

The themeType is correctly passed to both initial terminal creation and restart flows using themeTypeRef.current, ensuring the latest theme preference is consistently applied throughout the terminal lifecycle.

Also applies to: 324-324

apps/desktop/src/renderer/stores/theme/utils/terminal-theme.ts (1)

15-15: The selectionInactiveBackground and extendedAnsi properties are supported by xterm.js 5.5.0. These properties were introduced in xterm.js 5.0.0, with ITheme.selectionInactiveBackground for inactive selection color and support for extended ANSI colors 16-255. No changes needed.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Standard ANSI colors are used as backgrounds in diffs, so they need to be light/pastel for dark text readability. Bright colors are used as foreground text, so they should be dark for visibility on white background. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Claude Code diffs use white/brightWhite for text content. On light backgrounds, these must be dark colors for visibility. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

…ght theme Claude Code may use 256-color mode (colors 16-255). The grayscale colors (232-255) go from dark to light in standard palette, making light colors invisible on light backgrounds. This inverts the grayscale portion so higher indices are darker, maintaining visibility. Also adds: - selectionForeground for visible selected text - selectionInactiveBackground support in types - extendedAnsi support in theme conversion 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Claude Code and other CLI tools use the COLORFGBG environment variable to detect light/dark terminal mode. This sets: - Light mode: COLORFGBG="0;15" (dark fg on light bg) - Dark mode: COLORFGBG="15;0" (light fg on dark bg) This allows Claude Code to adapt its color output for light terminals, making diffs and other colored output visible. Changes: - Add themeType to terminal session params - Pass theme type from renderer to main process via tRPC - Set COLORFGBG in terminal environment based on theme type 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

charliecreates Bot requested a review from CharlieHelps December 22, 2025 23:12

Kitenite and others added 4 commits December 22, 2025 15:13

Kitenite closed this Dec 23, 2025

Kitenite deleted the naughty-butterfly-133510 branch December 25, 2025 18:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(desktop): improve light theme terminal color contrast#475

fix(desktop): improve light theme terminal color contrast#475
Kitenite wants to merge 5 commits intomainfrom
naughty-butterfly-133510

Kitenite commented Dec 22, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Dec 22, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Kitenite commented Dec 22, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Pre-merge checks and finishing touches

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Kitenite commented Dec 22, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Dec 22, 2025 •

edited

Loading