Skip to content

fix(tui): enforce backgrounds on light themes to prevent invisible text - #195

Merged
Desperado merged 2 commits into
mainfrom
fix/theme-contrast-mismatch
Sep 11, 2026
Merged

Desperado merged 2 commits into
mainfrom
fix/theme-contrast-mismatch

Conversation

@Desperado

Copy link
Copy Markdown
Contributor
  • Forces SurfaceDark background on pickerBox and inputBoxStyle so TUI menus remain readable on mismatched terminal backgrounds
  • Synchronizes glamour markdown rendering with the active app theme instead of relying on unreliable terminal auto-detection
  • Updates status bar to dynamically obey selected theme colors

- Forces `SurfaceDark` background on `pickerBox` and `inputBoxStyle` so TUI menus remain readable on mismatched terminal backgrounds
- Synchronizes `glamour` markdown rendering with the active app theme instead of relying on unreliable terminal auto-detection
- Updates status bar to dynamically obey selected theme colors
@qualitymaxapp

qualitymaxapp Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

QualityMax Review

Verdict: COMMENT · Confidence: evidence-backed scan

Files eligible: 5 · Files reviewed: 5 · Files with findings: 0 · Findings: 0 · Inline cards: 0

Priority findings

priority location finding
— — No blocking findings

Review gates

gate status
AI diff review completed · eligible 5, reviewed 5 · LLM · served gemini-3.1-flash-lite
SAST completed · eligible 5, reviewed 5 · hybrid · served qwen3.7-plus
Overall review evidence clean
Inline evidence not needed

Important files

file risk note next step
— — No findings —

Change diagram — Flow

graph TD
    A[ApplyTheme(Theme)] --> B[Update Global Styles]
    B --> C[refreshLiveTerminalRenderers]
    C --> D{For each Terminal}
    D --> E[t.setMarkdownStyle(dark)]
    E --> F[Rebuild glamour.TermRenderer]
    F --> G[Update t.renderer under rendMu]
Loading

Review lifecycle

Use the inline cards to inspect evidence and suggested remediation. Re-run the QualityMax review after pushing a fix; unchanged cards are identified by their stable finding marker. Dismiss with a reason through the existing QualityMax/GitHub review feedback flow. 0 prior card(s) are stale/resolved on this head. @qmax Q&A is tracked separately.

Proof legend: VERIFIED independently judged patch · REPRODUCED verified finding · GROUNDED deterministic evidence · MODEL-ONLY model judgment.

QualityMax project results are available in the configured project.

Receipt · commit 09a1139151d07934b2c9083a06cb5d407ef39b3d · run 2026-09-11T20:59:13+00:00 · model served qwen3.7-plus, gemini-3.1-flash-lite · model requested qwen3.7-plus, gemini-3.1-flash-lite · model review recorded — 955 model output tokens · model source repository ai_review_preferences.preferred_model · re-review 2 · proof counts {}

- Rebuild glamour renderer on live theme switch via terminal registry, so
  /theme and /set theme take effect immediately instead of after restart
- Guard renderer swaps with a mutex so mid-session style changes cannot
  race an in-flight markdown render
- Raise SurfaceSelect contrast vs SurfaceDark in all themes (picker
  selection highlight was invisible on the new boxed background)
- Paint statusMetricsStyle with SurfaceDark so the metrics line matches
  the status bar instead of floating on the terminal background
- Drop stale 'takes full effect on restart' message
- Add regression tests: polarity flag, style backgrounds, palette
  contrast, renderer swap + live refresh

@qualitymaxapp qualitymaxapp Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QualityMax Review — canonical overview updated; inline findings are attached to this review.

@qualitymaxapp

qualitymaxapp Bot commented Sep 11, 2026

Copy link
Copy Markdown

⚠️ QualityMax Pipeline

Gate Result
🔍 AI diff review ✅ Clean · gemini-3.1-flash-lite · completed · 5 eligible / 5 reviewed · gemini-3.1-flash-lite
🔍 SAST completed · 5 eligible / 5 reviewed · qwen3.7-plus
🔍 Canonical PR review delivery completed · 0 eligible / 0 reviewed · exact-head review #5183375423 and overview #5640162349 confirmed
🧪 Repo Tests ✅ 799/799 passed (go)

Powered by QualityMax — AI-Powered Test Automation

@Desperado

Copy link
Copy Markdown
Contributor Author

Adversarial review — findings resolved ✅

Follow-up review of this PR flagged 5 issues, all fixed in 09a1139 and verified (go build, go vet, go test -race ./internal/tui ./internal/repl, full go test ./... — all green).

# Severity Finding Resolution
1 Medium-High Glamour markdown renderer was built once in NewTerminal() reading ThemeIsDark, so a live /theme switch left markdown on the old polarity until restart ApplyTheme now rebuilds renderers of all live terminals via a registry (refreshLiveTerminalRenderers); renderer swap is mutex-guarded so it can't race an in-flight render; stale "takes full effect on restart" message removed
2 Medium Picker selection highlight nearly invisible: pickerRowSelected bg (SurfaceSelect 237) vs new pickerBox bg (SurfaceDark 236) — Δ1 step in every dark theme Palette raised in all 10 themes (dark: 237→241, neon 236→240, radiance 225→217, goldenhour 223→220)
3 Medium statusMetricsStyle had no background — the metrics line still floated in the exact mismatched-terminal scenario this PR targets Now paints SurfaceDark in both ApplyTheme and the input.go default
4 Low gofmt violation introduced in theme.go Fixed (only pre-existing diff.go noise remains, untouched by this PR)
5 Coverage theme_test.go would pass on the original invisible-text bug 5 new regression tests: polarity flag, style-background assertions, SurfaceSelect↔SurfaceDark contrast ≥4 steps, renderer polarity swap, live-terminal refresh

Also verified clean during review: startup ordering (theme applied before every NewTerminal() call site), lipgloss.SetHasDarkBackground has no other consumer, and the ThemeIsDark=true default matches the historic fallback.

@Desperado
Desperado merged commit b5a3b51 into main Sep 11, 2026
6 of 7 checks passed
@Desperado
Desperado deleted the fix/theme-contrast-mismatch branch September 11, 2026 21:42
Desperado added a commit that referenced this pull request Sep 12, 2026
Adversarial-review follow-ups:

- Foreground-only picker chrome (labels, headers, footers, dividers,
  hints) now keys off the terminal's actual background polarity instead
  of the previewed theme's. Previewing a light theme on a dark terminal
  (or vice versa) previously rendered near-black labels on a dark
  background — the exact invisible-text bug #195 tried to fix. Values
  match same-polarity themes exactly, so matched setups are unchanged.
- Gate tea.ClearScreen on actual polarity flips: clearing erases the
  visible screenful in place, so every arrow key was destroying the
  on-screen transcript. Same-polarity moves rely on the renderer's
  line diff; cancel only clears when the restore flips polarity.
- Skip glamour renderer rebuilds when polarity is unchanged.
- Tests: same-polarity move must not clear, no-flip cancel must quit
  without clear, chrome polarity adaptation, renderer rebuild elision.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant