Skip to content

Commit ff0c60a

Browse files
altimate-harness-bot[bot]saravmajesticanandgupta42
authored
fix: improve text contrast on light terminal backgrounds (#712)
* fix: improve light terminal detection and text contrast - Add COLORFGBG env var fallback when OSC 11 detection times out - Use dark foreground (#1a1a1a) instead of palette[7] (#c0c0c0) for light mode system theme - Use backgroundElement for inline code blocks to ensure contrast on transparent backgrounds Closes #704 * test: add tests for #704 light terminal fixes - Test markup.raw.inline uses backgroundElement (non-transparent) - Test system theme light-mode foreground fallback contrast - Test COLORFGBG env var parsing logic - Update test helper to match production inline code background change * test: add bug replication tests for #704 Adds tests that reproduce the exact bugs: - BUG test: proves palette[7] (#c0c0c0) has ~1.3:1 contrast on white (invisible) - BUG test: proves old inline code bg was transparent (no contrast) - FIX test: verifies new fg (#1a1a1a) has >=3:1 contrast on white - FIX test: verifies inline code bg is now opaque - FIX test: verifies dark mode is unaffected (still uses palette[7]) * chore: clean up verbose comments in fix * chore: add altimate_change markers to upstream patches * fix: [#704] address consensus review feedback on light terminal detection Addresses the multi-model consensus code review on PR #712. **COLORFGBG detection**: - Extract `detectModeFromCOLORFGBG` into a pure TypeScript helper at `packages/opencode/src/cli/cmd/tui/util/terminal-detection.ts` so tests exercise the real production code (was previously inlined in `app.tsx` with test duplicates that couldn't catch regressions). - Narrow the light-mode threshold from `bg >= 8` to `bg === 7 || bg === 15`. The old `>= 8` misclassified bright red (9), blue (12), magenta (13) as "light" even though they have low luminance; it also missed index 7 (light gray), which is a common light-terminal background. - Reject out-of-range (< 0, > 15), non-numeric ("default"), and empty values explicitly rather than silently defaulting to dark. - Tolerate surrounding whitespace. - Use `parseInt(..., 10)` with an explicit radix. - Check `COLORFGBG` eagerly in `getTerminalBackgroundColor()` before issuing the OSC 11 query. Light terminals that don't support OSC 11 (common with urxvt, gnome-terminal) no longer pay the 1-second timeout on every launch. **System-theme light-mode foreground fallback**: - `generateSystem` now falls back to `colors.palette[0] ?? "#1a1a1a"` in light mode (dark mode keeps `colors.palette[7]`). Respects the user's palette when provided; still guarantees a readable near-black on white when the palette is empty. **Tests**: - Rewrite `theme-light-mode-704.test.ts` to import the real `detectModeFromCOLORFGBG` — reverting the production helper now fails the 18 COLORFGBG tests. - Add coverage for: `0;7` (light-gray bg, now light), `15;8` (dark-gray bg, now dark), `0;9`/`0;12`/`0;13` (bright-red/blue/magenta, now dark), `default;default` (Alacritty/Kitty), out-of-range values, negatives, empty string, whitespace tolerance, undefined. - Add a light-mode regression test confirming fallback prefers `palette[0]` over `#1a1a1a` when present. - Add `defaultForeground`-is-honored regression. - Add dark-mode `markup.raw.inline` regression test in `theme-light-mode.test.ts` to cover the cross-mode impact of the `backgroundElement` change. - Consolidate duplicate `COLORFGBG` tests (previously in both files). Closes #704 (follow-up to #712/#617). --------- Co-authored-by: saravmajestic <saravmajestic@altimate.ai> Co-authored-by: anandgupta42 <anand@altimate.ai>
1 parent c3fc44d commit ff0c60a

6 files changed

Lines changed: 308 additions & 4 deletions

File tree

.github/meta/commit.txt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
1-
release: v0.5.20
1+
release: v0.5.21
2+
3+
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

packages/opencode/src/cli/cmd/tui/app.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,19 @@ import { PromptRefProvider, usePromptRef } from "./context/prompt"
110110
import { TuiConfigProvider } from "./context/tui-config"
111111
import { TuiConfig } from "@/config/tui"
112112

113+
// altimate_change start — fix: pure helper extracted to util/terminal-detection for test coverage (#704)
114+
import { detectModeFromCOLORFGBG } from "./util/terminal-detection"
115+
// altimate_change end
116+
113117
async function getTerminalBackgroundColor(): Promise<"dark" | "light"> {
114118
// can't set raw mode if not a TTY
115119
if (!process.stdin.isTTY) return "dark"
116120

121+
// altimate_change start — fix: check COLORFGBG eagerly to avoid 1s startup delay on terminals without OSC 11 (#704)
122+
const envMode = detectModeFromCOLORFGBG(process.env.COLORFGBG)
123+
if (envMode === "light") return "light"
124+
// altimate_change end
125+
117126
return new Promise((resolve) => {
118127
let timeout: NodeJS.Timeout
119128

packages/opencode/src/cli/cmd/tui/context/theme.tsx

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,10 +430,17 @@ export function tint(base: RGBA, overlay: RGBA, alpha: number): RGBA {
430430

431431
function generateSystem(colors: TerminalColors, mode: "dark" | "light"): ThemeJson {
432432
const bg = RGBA.fromHex(colors.defaultBackground ?? colors.palette[0]!)
433-
const fg = RGBA.fromHex(colors.defaultForeground ?? colors.palette[7]!)
434433
const transparent = RGBA.fromInts(0, 0, 0, 0)
435434
const isDark = mode == "dark"
436435

436+
// altimate_change start — fix: light-mode foreground fallback (#704)
437+
// Dark mode keeps palette[7] (standard light-gray fg on dark bg).
438+
// Light mode: palette[7] is typically #c0c0c0 — invisible on white.
439+
// Prefer the user's palette[0] (near-black) when present; #1a1a1a otherwise.
440+
const fgFallback = isDark ? colors.palette[7]! : (colors.palette[0] ?? "#1a1a1a")
441+
const fg = RGBA.fromHex(colors.defaultForeground ?? fgFallback)
442+
// altimate_change end
443+
437444
const col = (i: number) => {
438445
const value = colors.palette[i]
439446
if (value) return RGBA.fromHex(value)
@@ -948,7 +955,9 @@ function getSyntaxRules(theme: Theme) {
948955
scope: ["markup.raw.inline"],
949956
style: {
950957
foreground: theme.markdownCode,
951-
background: theme.background, // inline code blends with page background
958+
// altimate_change start — fix: inline code contrast on transparent backgrounds
959+
background: theme.backgroundElement,
960+
// altimate_change end
952961
},
953962
},
954963
{
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// altimate_change start — fix: pure-TS helper extracted from app.tsx for direct test coverage (#704)
2+
/**
3+
* Detect terminal background mode from the COLORFGBG env var.
4+
*
5+
* Format is `fg;bg` or `fg;default;bg` (rxvt/urxvt). The last semicolon-
6+
* separated component is the background palette index. Only indices that
7+
* are canonically light (7 = light-gray, 15 = bright-white) classify as
8+
* "light" — other bright indices (9 red, 12 blue, 13 magenta) are dark
9+
* by luminance and must not be treated as light.
10+
*
11+
* Returns `null` when the value is missing, malformed (e.g. "default"),
12+
* or outside the 0-15 ANSI range.
13+
*/
14+
export function detectModeFromCOLORFGBG(value: string | undefined): "dark" | "light" | null {
15+
if (!value) return null
16+
const parts = value.split(";")
17+
const last = parts[parts.length - 1]?.trim()
18+
if (!last) return null
19+
const bg = parseInt(last, 10)
20+
if (!Number.isInteger(bg) || bg < 0 || bg > 15) return null
21+
return bg === 7 || bg === 15 ? "light" : "dark"
22+
}
23+
// altimate_change end
Lines changed: 246 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,246 @@
1+
import { describe, expect, test } from "bun:test"
2+
import { RGBA } from "@opentui/core"
3+
import { detectModeFromCOLORFGBG } from "@/cli/cmd/tui/util/terminal-detection"
4+
import github from "@/cli/cmd/tui/context/theme/github.json"
5+
import solarized from "@/cli/cmd/tui/context/theme/solarized.json"
6+
import flexoki from "@/cli/cmd/tui/context/theme/flexoki.json"
7+
8+
/**
9+
* Regression tests for issue #704 — code output renders as white text on
10+
* light terminal backgrounds.
11+
*
12+
* The COLORFGBG tests exercise the real production helper
13+
* (`detectModeFromCOLORFGBG` in `util/terminal-detection.ts`). Reverting
14+
* the fix in that file will cause these tests to fail.
15+
*
16+
* The theme-level tests (system-theme foreground fallback, inline-code
17+
* background) reproduce the logic locally rather than importing from
18+
* `theme.tsx`. The .tsx module cannot be imported from `bun:test`
19+
* because `@opentui/solid`'s JSX runtime types don't resolve in the
20+
* test loader (tracked for a follow-up pure-TS extraction). The local
21+
* copies are kept in lockstep with production via manual review.
22+
*/
23+
24+
// ─── Pure test helpers (WCAG contrast + ANSI palette resolution) ───────────
25+
26+
function ansiToRgba(code: number): RGBA {
27+
if (code < 16) {
28+
const ansiColors = [
29+
"#000000", "#800000", "#008000", "#808000",
30+
"#000080", "#800080", "#008080", "#c0c0c0",
31+
"#808080", "#ff0000", "#00ff00", "#ffff00",
32+
"#0000ff", "#ff00ff", "#00ffff", "#ffffff",
33+
]
34+
return RGBA.fromHex(ansiColors[code] ?? "#000000")
35+
}
36+
if (code < 232) {
37+
const index = code - 16
38+
const b = index % 6
39+
const g = Math.floor(index / 6) % 6
40+
const r = Math.floor(index / 36)
41+
const val = (x: number) => (x === 0 ? 0 : x * 40 + 55)
42+
return RGBA.fromInts(val(r), val(g), val(b))
43+
}
44+
if (code < 256) {
45+
const gray = (code - 232) * 10 + 8
46+
return RGBA.fromInts(gray, gray, gray)
47+
}
48+
return RGBA.fromInts(0, 0, 0)
49+
}
50+
51+
type ThemeJson = { defs?: Record<string, string>; theme: Record<string, unknown> }
52+
53+
function resolveTheme(theme: ThemeJson, mode: "dark" | "light"): Record<string, RGBA> {
54+
const defs = theme.defs ?? {}
55+
type ColorValue = string | number | RGBA | { dark: string; light: string }
56+
function resolveColor(c: ColorValue): RGBA {
57+
if (c instanceof RGBA) return c
58+
if (typeof c === "string") {
59+
if (c === "transparent" || c === "none") return RGBA.fromInts(0, 0, 0, 0)
60+
if (c.startsWith("#")) return RGBA.fromHex(c)
61+
if (defs[c] != null) return resolveColor(defs[c])
62+
if (theme.theme[c] !== undefined) return resolveColor(theme.theme[c] as ColorValue)
63+
throw new Error("Color reference not found: " + c)
64+
}
65+
if (typeof c === "number") return ansiToRgba(c)
66+
return resolveColor(c[mode])
67+
}
68+
const resolved: Record<string, RGBA> = {}
69+
for (const [key, value] of Object.entries(theme.theme)) {
70+
if (key === "selectedListItemText" || key === "backgroundMenu" || key === "thinkingOpacity") continue
71+
resolved[key] = resolveColor(value as ColorValue)
72+
}
73+
resolved.backgroundMenu = theme.theme.backgroundMenu
74+
? resolveColor(theme.theme.backgroundMenu as ColorValue)
75+
: resolved.backgroundElement!
76+
return resolved
77+
}
78+
79+
function contrastRatio(fg: RGBA, bg: RGBA): number {
80+
function relLum(c: RGBA): number {
81+
const [r, g, b] = c.toInts()
82+
const srgb = [r, g, b].map((v) => {
83+
const s = v / 255
84+
return s <= 0.03928 ? s / 12.92 : Math.pow((s + 0.055) / 1.055, 2.4)
85+
})
86+
return 0.2126 * srgb[0]! + 0.7152 * srgb[1]! + 0.0722 * srgb[2]!
87+
}
88+
const l1 = relLum(fg)
89+
const l2 = relLum(bg)
90+
return (Math.max(l1, l2) + 0.05) / (Math.min(l1, l2) + 0.05)
91+
}
92+
93+
// ─── detectModeFromCOLORFGBG: uses REAL production helper ──────────────────
94+
95+
describe("issue #704: detectModeFromCOLORFGBG (real production helper)", () => {
96+
test("0;15 (bright white bg) -> light", () => {
97+
expect(detectModeFromCOLORFGBG("0;15")).toBe("light")
98+
})
99+
100+
test("0;7 (light-gray bg) -> light", () => {
101+
expect(detectModeFromCOLORFGBG("0;7")).toBe("light")
102+
})
103+
104+
test("15;0 (black bg) -> dark", () => {
105+
expect(detectModeFromCOLORFGBG("15;0")).toBe("dark")
106+
})
107+
108+
test("15;8 (dark-gray bg) -> dark", () => {
109+
expect(detectModeFromCOLORFGBG("15;8")).toBe("dark")
110+
})
111+
112+
test("0;9 (bright red bg) -> dark (bright != light)", () => {
113+
expect(detectModeFromCOLORFGBG("0;9")).toBe("dark")
114+
})
115+
116+
test("0;12 (bright blue bg) -> dark", () => {
117+
expect(detectModeFromCOLORFGBG("0;12")).toBe("dark")
118+
})
119+
120+
test("0;13 (bright magenta bg) -> dark", () => {
121+
expect(detectModeFromCOLORFGBG("0;13")).toBe("dark")
122+
})
123+
124+
test("0;7;15 (3-part, last is bg) -> light", () => {
125+
expect(detectModeFromCOLORFGBG("0;7;15")).toBe("light")
126+
})
127+
128+
test("15;0;0 (3-part, last is bg) -> dark", () => {
129+
expect(detectModeFromCOLORFGBG("15;0;0")).toBe("dark")
130+
})
131+
132+
test("default;default (Alacritty/Kitty) -> null", () => {
133+
expect(detectModeFromCOLORFGBG("default;default")).toBe(null)
134+
})
135+
136+
test("15;default -> null", () => {
137+
expect(detectModeFromCOLORFGBG("15;default")).toBe(null)
138+
})
139+
140+
test("0;99 (out-of-range) -> null", () => {
141+
expect(detectModeFromCOLORFGBG("0;99")).toBe(null)
142+
})
143+
144+
test("0;256 (out-of-range) -> null", () => {
145+
expect(detectModeFromCOLORFGBG("0;256")).toBe(null)
146+
})
147+
148+
test("0;-1 (negative) -> null", () => {
149+
expect(detectModeFromCOLORFGBG("0;-1")).toBe(null)
150+
})
151+
152+
test("empty string -> null", () => {
153+
expect(detectModeFromCOLORFGBG("")).toBe(null)
154+
})
155+
156+
test("undefined -> null", () => {
157+
expect(detectModeFromCOLORFGBG(undefined)).toBe(null)
158+
})
159+
160+
test("' 0;15 ' (whitespace tolerated) -> light", () => {
161+
expect(detectModeFromCOLORFGBG(" 0;15 ")).toBe("light")
162+
})
163+
164+
test("abc (non-numeric) -> null", () => {
165+
expect(detectModeFromCOLORFGBG("abc")).toBe(null)
166+
})
167+
})
168+
169+
// ─── Theme-level tests (pure-TS reproduction of generateSystem) ────────────
170+
171+
type TerminalColors = {
172+
defaultBackground?: string
173+
defaultForeground?: string
174+
palette: string[]
175+
}
176+
177+
function generateSystemLike(colors: TerminalColors, mode: "dark" | "light") {
178+
const bg = RGBA.fromHex(colors.defaultBackground ?? colors.palette[0]!)
179+
const isDark = mode === "dark"
180+
// Mirror of theme.tsx: light-mode fallback prefers palette[0], else #1a1a1a
181+
const fgFallback = isDark ? colors.palette[7]! : (colors.palette[0] ?? "#1a1a1a")
182+
const fg = RGBA.fromHex(colors.defaultForeground ?? fgFallback)
183+
return { bg, fg }
184+
}
185+
186+
const LIGHT_TERMINAL: TerminalColors = {
187+
defaultBackground: "#ffffff",
188+
defaultForeground: undefined,
189+
palette: [
190+
"#000000", "#800000", "#008000", "#808000",
191+
"#000080", "#800080", "#008080", "#c0c0c0",
192+
"#808080", "#ff0000", "#00ff00", "#ffff00",
193+
"#0000ff", "#ff00ff", "#00ffff", "#ffffff",
194+
],
195+
}
196+
197+
describe("issue #704: system theme light-mode foreground fallback", () => {
198+
test("light mode: fallback is not palette[7] (#c0c0c0)", () => {
199+
const { fg } = generateSystemLike(LIGHT_TERMINAL, "light")
200+
expect(fg.equals(RGBA.fromHex("#c0c0c0"))).toBe(false)
201+
})
202+
203+
test("light mode: fallback has WCAG-AA contrast on white", () => {
204+
const { fg } = generateSystemLike(LIGHT_TERMINAL, "light")
205+
const whiteBg = RGBA.fromHex("#ffffff")
206+
expect(contrastRatio(fg, whiteBg)).toBeGreaterThanOrEqual(4.5)
207+
})
208+
209+
test("light mode: fallback respects user palette[0] when provided", () => {
210+
const custom: TerminalColors = { ...LIGHT_TERMINAL, palette: ["#222244", ...LIGHT_TERMINAL.palette.slice(1)] }
211+
const { fg } = generateSystemLike(custom, "light")
212+
expect(fg.equals(RGBA.fromHex("#222244"))).toBe(true)
213+
})
214+
215+
test("dark mode regression: fallback is palette[7]", () => {
216+
const darkTerminal: TerminalColors = { ...LIGHT_TERMINAL, defaultBackground: "#1a1a1a" }
217+
const { fg } = generateSystemLike(darkTerminal, "dark")
218+
expect(fg.equals(RGBA.fromHex("#c0c0c0"))).toBe(true)
219+
})
220+
221+
test("defaultForeground is always honored when provided", () => {
222+
const explicit: TerminalColors = { ...LIGHT_TERMINAL, defaultForeground: "#113355" }
223+
const { fg } = generateSystemLike(explicit, "light")
224+
expect(fg.equals(RGBA.fromHex("#113355"))).toBe(true)
225+
})
226+
})
227+
228+
describe("issue #704: markup.raw.inline uses backgroundElement (named themes)", () => {
229+
const LIGHT_THEMES: [string, ThemeJson][] = [
230+
["github", github as unknown as ThemeJson],
231+
["solarized", solarized as unknown as ThemeJson],
232+
["flexoki", flexoki as unknown as ThemeJson],
233+
]
234+
235+
test.each(LIGHT_THEMES)(
236+
"%s light: backgroundElement is opaque and gives markdownCode visible contrast",
237+
(_name, themeJson) => {
238+
const resolved = resolveTheme(themeJson, "light")
239+
expect(resolved.backgroundElement!.a).toBeGreaterThan(0)
240+
const ratio = contrastRatio(resolved.markdownCode!, resolved.backgroundElement!)
241+
// 2.0 matches the threshold used elsewhere in this suite — inline code
242+
// colors are syntax-intent (semantic) and don't need full WCAG-AA text contrast.
243+
expect(ratio).toBeGreaterThanOrEqual(2)
244+
},
245+
)
246+
})

packages/opencode/test/cli/tui/theme-light-mode.test.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ function getSyntaxRules(theme: Theme): SyntaxRule[] {
139139
},
140140
{
141141
scope: ["markup.raw.inline"],
142-
style: { foreground: theme.markdownCode, background: theme.background },
142+
style: { foreground: theme.markdownCode, background: theme.backgroundElement },
143143
},
144144
{ scope: ["markup.link"], style: { foreground: theme.markdownLink, underline: true } },
145145
{ scope: ["spell", "nospell"], style: { foreground: theme.text } },
@@ -352,4 +352,19 @@ describe("dark theme: regression check", () => {
352352
expect(defaultRule.style.foreground!.a).toBeGreaterThan(0)
353353
},
354354
)
355+
356+
test.each(DARK_THEMES)(
357+
"%s: markup.raw.inline background is opaque in dark mode (issue #704 cross-mode regression)",
358+
(_name, themeJson) => {
359+
const resolved = resolveTheme(themeJson, "dark")
360+
const rules = getSyntaxRules(resolved)
361+
362+
const inlineRule = rules.find((r) => r.scope.includes("markup.raw.inline"))!
363+
expect(inlineRule.style.background).toBeDefined()
364+
expect(inlineRule.style.background!.a).toBeGreaterThan(0)
365+
366+
const ratio = contrastRatio(inlineRule.style.foreground!, inlineRule.style.background!)
367+
expect(ratio).toBeGreaterThanOrEqual(2)
368+
},
369+
)
355370
})

0 commit comments

Comments
 (0)