feat(ui): enhance color styling with adaptive colors and improve term…#22
feat(ui): enhance color styling with adaptive colors and improve term…#22josephgoksu wants to merge 2 commits intomainfrom
Conversation
…inal width handling
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
internal/ui/spinner.go
Outdated
| select { | ||
| case <-s.stop: | ||
| fmt.Fprint(os.Stderr, "\r\033[K") | ||
| fmt.Fprint(os.Stderr, "\r"+strings.Repeat(" ", 80)+"\r") |
There was a problem hiding this comment.
Hardcoded 80-character clear regresses wide-terminal support.
The old implementation used the ANSI EL escape (\033[K), which clears the entire line regardless of terminal width. This replacement with strings.Repeat(" ", 80) is hardcoded to 80 characters—on terminals wider than 80 columns, spinner frames extending beyond column 80 will leave visible artifacts after the spinner stops.
Notably, GetTerminalWidth() was introduced in this same PR but is not used here. The fix is to either restore the ANSI escape or use the new helper:
| fmt.Fprint(os.Stderr, "\r"+strings.Repeat(" ", 80)+"\r") | |
| fmt.Fprint(os.Stderr, "\r\033[K") |
Or alternatively:
| fmt.Fprint(os.Stderr, "\r"+strings.Repeat(" ", 80)+"\r") | |
| fmt.Fprint(os.Stderr, "\r"+strings.Repeat(" ", GetTerminalWidth())+"\r") |
However, note that Comment on internal/ui/table.go line 133 flags that GetTerminalWidth() queries os.Stdout, not os.Stderr, so the second approach would require that helper to accept an *os.File parameter.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/ui/spinner.go
Line: 45
Comment:
Hardcoded 80-character clear regresses wide-terminal support.
The old implementation used the ANSI EL escape (`\033[K`), which clears the entire line regardless of terminal width. This replacement with `strings.Repeat(" ", 80)` is hardcoded to 80 characters—on terminals wider than 80 columns, spinner frames extending beyond column 80 will leave visible artifacts after the spinner stops.
Notably, `GetTerminalWidth()` was introduced in this same PR but is not used here. The fix is to either restore the ANSI escape or use the new helper:
```suggestion
fmt.Fprint(os.Stderr, "\r\033[K")
```
Or alternatively:
```suggestion
fmt.Fprint(os.Stderr, "\r"+strings.Repeat(" ", GetTerminalWidth())+"\r")
```
However, note that Comment on `internal/ui/table.go` line 133 flags that `GetTerminalWidth()` queries `os.Stdout`, not `os.Stderr`, so the second approach would require that helper to accept an `*os.File` parameter.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Fixed — restored ANSI escape and removed unused strings import.
| if total > available { | ||
| // Proportionally shrink columns, but keep a minimum of 4 chars | ||
| ratio := float64(available) / float64(total) | ||
| for i := range widths { | ||
| newW := int(float64(widths[i]) * ratio) | ||
| if newW < 4 { | ||
| newW = 4 | ||
| } | ||
| widths[i] = newW | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Minimum-clamp can still produce overflowing column totals.
After applying the proportional ratio (line 59), each column is floored to a minimum of 4 characters (lines 62–64). With many columns, the total 4*n + overhead can exceed termWidth, meaning the table will overflow even after the "constrain" pass. There is no post-clamp overflow check.
Example: termWidth=80, 15 columns, overhead=29, available=51, ratio≈0.34. Each newW = int(10*0.34)=3, clamped to 4. Post-clamp total = 4*15+29 = 89 > 80.
A follow-up validation is needed after the clamp loop to either re-trim the widest columns or document that overflow is accepted:
| if total > available { | |
| // Proportionally shrink columns, but keep a minimum of 4 chars | |
| ratio := float64(available) / float64(total) | |
| for i := range widths { | |
| newW := int(float64(widths[i]) * ratio) | |
| if newW < 4 { | |
| newW = 4 | |
| } | |
| widths[i] = newW | |
| } | |
| } | |
| } | |
| // After the minimum-clamp loop, check if post-clamp total still exceeds available | |
| postClampTotal := 0 | |
| for _, w := range widths { | |
| postClampTotal += w | |
| } | |
| if postClampTotal > available { | |
| // Post-clamp overflow detected. Accept gracefully or trim. | |
| // Consider logging a debug message or trimming extra from widest columns. | |
| } |
At minimum, add a comment documenting that the 4-char floor can cause overflow.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/ui/table.go
Line: 57-68
Comment:
Minimum-clamp can still produce overflowing column totals.
After applying the proportional `ratio` (line 59), each column is floored to a minimum of 4 characters (lines 62–64). With many columns, the total `4*n + overhead` can exceed `termWidth`, meaning the table will overflow even after the "constrain" pass. There is no post-clamp overflow check.
Example: `termWidth=80`, 15 columns, overhead=29, available=51, ratio≈0.34. Each `newW = int(10*0.34)=3`, clamped to `4`. Post-clamp total = `4*15+29 = 89 > 80`.
A follow-up validation is needed after the clamp loop to either re-trim the widest columns or document that overflow is accepted:
```suggestion
// After the minimum-clamp loop, check if post-clamp total still exceeds available
postClampTotal := 0
for _, w := range widths {
postClampTotal += w
}
if postClampTotal > available {
// Post-clamp overflow detected. Accept gracefully or trim.
// Consider logging a debug message or trimming extra from widest columns.
}
```
At minimum, add a comment documenting that the 4-char floor can cause overflow.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed — added post-clamp overflow loop that trims widest columns until total fits within available width.
internal/ui/table.go
Outdated
| w, _, err := term.GetSize(int(os.Stdout.Fd())) | ||
| if err != nil || w <= 0 { | ||
| return 80 | ||
| } | ||
| return w | ||
| } |
There was a problem hiding this comment.
GetTerminalWidth() queries Stdout but may be used for output written to other streams.
GetTerminalWidth() calls term.GetSize(int(os.Stdout.Fd())), hard-coding the query to stdout. When Table.ColumnWidths() uses this function (line 48), the returned width assumes output will be written to stdout. However, a caller could render the table string and write it to stderr or another stream. In redirected pipelines (e.g., taskwing ask > output.txt), os.Stdout may not be a TTY while os.Stderr is a TTY, causing GetTerminalWidth() to return the fallback 80 even though the actual output stream is wider.
Consider accepting a parameter specifying which stream to query:
| w, _, err := term.GetSize(int(os.Stdout.Fd())) | |
| if err != nil || w <= 0 { | |
| return 80 | |
| } | |
| return w | |
| } | |
| func GetTerminalWidthFor(f *os.File) int { | |
| w, _, err := term.GetSize(int(f.Fd())) | |
| if err != nil || w <= 0 { | |
| return 80 | |
| } | |
| return w | |
| } | |
| // GetTerminalWidth returns the current stdout terminal width, defaulting to 80. | |
| func GetTerminalWidth() int { | |
| return GetTerminalWidthFor(os.Stdout) | |
| } |
Then update Table.ColumnWidths() to accept an optional output stream parameter, or document that it assumes stdout.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/ui/table.go
Line: 133-138
Comment:
`GetTerminalWidth()` queries `Stdout` but may be used for output written to other streams.
`GetTerminalWidth()` calls `term.GetSize(int(os.Stdout.Fd()))`, hard-coding the query to stdout. When `Table.ColumnWidths()` uses this function (line 48), the returned width assumes output will be written to stdout. However, a caller could render the table string and write it to `stderr` or another stream. In redirected pipelines (e.g., `taskwing ask > output.txt`), `os.Stdout` may not be a TTY while `os.Stderr` is a TTY, causing `GetTerminalWidth()` to return the fallback `80` even though the actual output stream is wider.
Consider accepting a parameter specifying which stream to query:
```suggestion
func GetTerminalWidthFor(f *os.File) int {
w, _, err := term.GetSize(int(f.Fd()))
if err != nil || w <= 0 {
return 80
}
return w
}
// GetTerminalWidth returns the current stdout terminal width, defaulting to 80.
func GetTerminalWidth() int {
return GetTerminalWidthFor(os.Stdout)
}
```
Then update `Table.ColumnWidths()` to accept an optional output stream parameter, or document that it assumes stdout.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed — extracted GetTerminalWidthFor(f *os.File) that accepts any file descriptor. GetTerminalWidth() remains as a backward-compatible wrapper.
|
@greptile |
| badge := lipgloss.NewStyle(). | ||
| Foreground(lipgloss.Color("0")). | ||
| Foreground(lipgloss.AdaptiveColor{Light: "231", Dark: "0"}). | ||
| Background(color). | ||
| Padding(0, 1). | ||
| Bold(true) |
There was a problem hiding this comment.
Poor contrast: white text on warm light-mode badge backgrounds
In light mode, the badge foreground is set to "231" (effectively #ffffff white). For several badge background colors in light mode, this produces insufficient contrast:
ColorYellow.Light = "136"→ approximately#af8700(goldenrod). White text on goldenrod ≈ 2.8:1 contrast ratio — fails WCAG AA (requires 4.5:1 for normal text).ColorWarning.Light = "172"→ approximately#d78700(amber/orange). White text on amber ≈ 3.1:1 — also fails.ColorText.Light = "235"→ approximately#262626(near-black). White text on near-black is fine (high contrast), but this will be invisible to users.
The dark-mode pairing ("0" = black on vivid colors) is generally fine, but the light-mode foreground of "231" is the wrong direction. On a light terminal background the badge background colors are mid-toned; the readable foreground choice should be dark (e.g., "0" or "232"), not white.
| badge := lipgloss.NewStyle(). | |
| Foreground(lipgloss.Color("0")). | |
| Foreground(lipgloss.AdaptiveColor{Light: "231", Dark: "0"}). | |
| Background(color). | |
| Padding(0, 1). | |
| Bold(true) | |
| badge := lipgloss.NewStyle(). | |
| Foreground(lipgloss.AdaptiveColor{Light: "0", Dark: "0"}). | |
| Background(color). | |
| Padding(0, 1). | |
| Bold(true) |
If you need a lighter foreground on dark badge backgrounds in dark mode, consider using {Light: "0", Dark: "231"} (dark text in light mode, white text in dark mode), which inverts the current (broken) mapping.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/ui/styles.go
Line: 129-133
Comment:
**Poor contrast: white text on warm light-mode badge backgrounds**
In light mode, the badge foreground is set to `"231"` (effectively `#ffffff` white). For several badge background colors in light mode, this produces insufficient contrast:
- `ColorYellow.Light = "136"` → approximately `#af8700` (goldenrod). White text on goldenrod ≈ 2.8:1 contrast ratio — fails WCAG AA (requires 4.5:1 for normal text).
- `ColorWarning.Light = "172"` → approximately `#d78700` (amber/orange). White text on amber ≈ 3.1:1 — also fails.
- `ColorText.Light = "235"` → approximately `#262626` (near-black). White text on near-black is fine (high contrast), but this will be invisible to users.
The dark-mode pairing (`"0"` = black on vivid colors) is generally fine, but the light-mode foreground of `"231"` is the wrong direction. On a light terminal background the badge background colors are mid-toned; the readable foreground choice should be dark (e.g., `"0"` or `"232"`), not white.
```suggestion
badge := lipgloss.NewStyle().
Foreground(lipgloss.AdaptiveColor{Light: "0", Dark: "0"}).
Background(color).
Padding(0, 1).
Bold(true)
```
If you need a lighter foreground on dark badge backgrounds in dark mode, consider using `{Light: "0", Dark: "231"}` (dark text in light mode, white text in dark mode), which inverts the current (broken) mapping.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed — badge foreground changed from Light:231 (white) to Light:0 (black) for proper contrast on light-mode backgrounds.
internal/ui/styles.go
Outdated
| ColorTableRowEven = lipgloss.AdaptiveColor{Light: "254", Dark: "236"} // Subtle background | ||
| ColorTableRowOdd = lipgloss.AdaptiveColor{Light: "231", Dark: "234"} // Slightly different |
There was a problem hiding this comment.
ColorTableRowEven / ColorTableRowOdd are dead constants
These two color variables are declared but have zero usages anywhere in the codebase (verified with a project-wide search). StyleTableRowEven and StyleTableRowOdd reference ColorText and ColorDim respectively — not these variables. They appear to be legacy background-color hints that were never wired up.
Either:
- Apply them as
Background(...)colors inStyleTableRowEven/StyleTableRowOdd, or - Remove the declarations to avoid confusion.
Keeping unreferenced exported color constants inflates the public surface of the ui package and misleads readers into thinking alternating-row background coloring is already implemented.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/ui/styles.go
Line: 91-92
Comment:
**`ColorTableRowEven` / `ColorTableRowOdd` are dead constants**
These two color variables are declared but have zero usages anywhere in the codebase (verified with a project-wide search). `StyleTableRowEven` and `StyleTableRowOdd` reference `ColorText` and `ColorDim` respectively — not these variables. They appear to be legacy background-color hints that were never wired up.
Either:
1. Apply them as `Background(...)` colors in `StyleTableRowEven`/`StyleTableRowOdd`, or
2. Remove the declarations to avoid confusion.
Keeping unreferenced exported color constants inflates the public surface of the `ui` package and misleads readers into thinking alternating-row background coloring is already implemented.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed — removed unused ColorTableRowEven and ColorTableRowOdd declarations.
| // Auto-constrain to terminal width when MaxWidth is not set | ||
| if t.MaxWidth == 0 { | ||
| termWidth := GetTerminalWidth() | ||
| // Account for leading space + column separators (2 chars between each column) | ||
| overhead := 1 + (len(widths)-1)*2 | ||
| available := termWidth - overhead |
There was a problem hiding this comment.
Overhead formula underestimates when len(widths) == 0
When t.Headers is empty the guard in Render() returns early, so ColumnWidths() will never reach this block in the current code. However, ColumnWidths() is a public method and can be called directly on a zero-header Table. In that case:
overhead = 1 + (0 - 1) * 2 → 1 + (-2) → -1
available = termWidth - (-1) → termWidth + 1
available > 0 is satisfied but the total loop over an empty slice produces 0 > available → false, so no crash occurs. Still, -1 overhead is semantically wrong and makes the guard appear broader than it is. A minimal defensive fix:
| // Auto-constrain to terminal width when MaxWidth is not set | |
| if t.MaxWidth == 0 { | |
| termWidth := GetTerminalWidth() | |
| // Account for leading space + column separators (2 chars between each column) | |
| overhead := 1 + (len(widths)-1)*2 | |
| available := termWidth - overhead | |
| // Account for leading space + column separators (2 chars between each column) | |
| overhead := 1 | |
| if len(widths) > 1 { | |
| overhead += (len(widths) - 1) * 2 | |
| } | |
| available := termWidth - overhead |
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/ui/table.go
Line: 46-51
Comment:
**Overhead formula underestimates when `len(widths) == 0`**
When `t.Headers` is empty the guard in `Render()` returns early, so `ColumnWidths()` will never reach this block in the current code. However, `ColumnWidths()` is a public method and can be called directly on a zero-header `Table`. In that case:
```
overhead = 1 + (0 - 1) * 2 → 1 + (-2) → -1
available = termWidth - (-1) → termWidth + 1
```
`available > 0` is satisfied but the `total` loop over an empty slice produces `0 > available` → false, so no crash occurs. Still, `-1` overhead is semantically wrong and makes the guard appear broader than it is. A minimal defensive fix:
```suggestion
// Account for leading space + column separators (2 chars between each column)
overhead := 1
if len(widths) > 1 {
overhead += (len(widths) - 1) * 2
}
available := termWidth - overhead
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed — overhead formula now guards against len(widths) <= 1 to avoid negative overhead.
…ling - Restore ANSI escape for spinner line clearing instead of hardcoded 80-char width - Add post-clamp overflow redistribution in table column sizing - Extract GetTerminalWidthFor(f *os.File) to support non-stdout streams - Fix badge foreground contrast for light-mode terminals (white -> black) - Remove unused ColorTableRowEven/ColorTableRowOdd declarations - Guard overhead formula against empty widths slice - Move Supported Models section up in README
|
@greptile |
| "plan": ColorSuccess, | ||
| "note": ColorText, | ||
| "metadata": ColorCyan, | ||
| "documentation": ColorYellow, |
There was a problem hiding this comment.
ColorText misused as badge background for "note" type
ColorText is defined as {Light: "235", Dark: "252"}. In light-mode terminals, "235" resolves to approximately #262626 (near-black). Using a foreground-text color as a badge background causes a visual inversion in light mode: a "note" badge will render with a near-black background, which is jarring and semantically wrong when every other UI element in that theme is light.
In the old code this was lipgloss.Color("252") — a consistent light-gray in all modes. The new mapping produces:
- Dark mode:
"252"(#d0d0d0, light gray) background — visually light and subtle, fine. - Light mode:
"235"(#262626, near-black) background — visually an inverted/dark badge in a light theme, inconsistent with all other badge backgrounds.
Consider introducing a dedicated neutral adaptive color for "note" badges, e.g.:
// in styles.go
ColorNeutral = lipgloss.AdaptiveColor{Light: "248", Dark: "252"} // Neutral gray for note badgesThen in CategoryBadge:
| "documentation": ColorYellow, | |
| "note": ColorNeutral, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/ui/styles.go
Line: 119
Comment:
**`ColorText` misused as badge background for "note" type**
`ColorText` is defined as `{Light: "235", Dark: "252"}`. In light-mode terminals, `"235"` resolves to approximately `#262626` (near-black). Using a foreground-text color as a badge *background* causes a visual inversion in light mode: a "note" badge will render with a near-black background, which is jarring and semantically wrong when every other UI element in that theme is light.
In the old code this was `lipgloss.Color("252")` — a consistent light-gray in all modes. The new mapping produces:
- **Dark mode:** `"252"` (#d0d0d0, light gray) background — visually light and subtle, fine.
- **Light mode:** `"235"` (#262626, near-black) background — visually an inverted/dark badge in a light theme, inconsistent with all other badge backgrounds.
Consider introducing a dedicated neutral adaptive color for "note" badges, e.g.:
```go
// in styles.go
ColorNeutral = lipgloss.AdaptiveColor{Light: "248", Dark: "252"} // Neutral gray for note badges
```
Then in `CategoryBadge`:
```suggestion
"note": ColorNeutral,
```
How can I resolve this? If you propose a fix, please make it concise.| } | ||
| available := termWidth - overhead | ||
| if available > 0 { | ||
| total := 0 |
There was a problem hiding this comment.
Mixed byte-count vs. terminal-column comparison in auto-constrain logic
widths[i] values are populated using len(h) (header) and len(cell) (row cells), which return byte lengths. GetTerminalWidth() returns terminal columns (visual width). For pure ASCII these are equivalent, but multi-byte UTF-8 content (e.g., emoji in headers like "🔍 Symbol" used in explain.go, or CJK characters) will cause total (bytes) to exceed available (columns) even when the table would visually fit, triggering unnecessary proportional shrinking. In extreme cases with wide characters, columns get over-shrunk and the table renders narrower than the terminal.
The fix is to measure visual width using lipgloss.Width(s) (which accounts for Unicode and ANSI sequences) when calculating widths:
// Instead of len(h) / len(cell), use:
import "github.com/charmbracelet/lipgloss"
// In ColumnWidths():
for i, h := range t.Headers {
widths[i] = lipgloss.Width(h)
}
for _, row := range t.Rows {
for i, cell := range row {
if i < len(widths) && lipgloss.Width(cell) > widths[i] {
widths[i] = lipgloss.Width(cell)
}
}
}padRight has the same len(s) issue — it should also use lipgloss.Width(s) to avoid producing visually misaligned padding for Unicode content.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/ui/table.go
Line: 53-56
Comment:
**Mixed byte-count vs. terminal-column comparison in auto-constrain logic**
`widths[i]` values are populated using `len(h)` (header) and `len(cell)` (row cells), which return **byte lengths**. `GetTerminalWidth()` returns **terminal columns** (visual width). For pure ASCII these are equivalent, but multi-byte UTF-8 content (e.g., emoji in headers like `"🔍 Symbol"` used in `explain.go`, or CJK characters) will cause `total` (bytes) to exceed `available` (columns) even when the table would visually fit, triggering unnecessary proportional shrinking. In extreme cases with wide characters, columns get over-shrunk and the table renders narrower than the terminal.
The fix is to measure visual width using `lipgloss.Width(s)` (which accounts for Unicode and ANSI sequences) when calculating `widths`:
```go
// Instead of len(h) / len(cell), use:
import "github.com/charmbracelet/lipgloss"
// In ColumnWidths():
for i, h := range t.Headers {
widths[i] = lipgloss.Width(h)
}
for _, row := range t.Rows {
for i, cell := range row {
if i < len(widths) && lipgloss.Width(cell) > widths[i] {
widths[i] = lipgloss.Width(cell)
}
}
}
```
`padRight` has the same `len(s)` issue — it should also use `lipgloss.Width(s)` to avoid producing visually misaligned padding for Unicode content.
How can I resolve this? If you propose a fix, please make it concise.| ``` | ||
|
|
||
| **Real session, real numbers** — asked *"What are the bottlenecks in our engineering process?"*: | ||
| - **Without TaskWing:** 8 Glob/Grep searches, 12 file reads, 25,000 tokens, 3 minutes | ||
| - **With TaskWing MCP:** 1 query, 1,500 tokens, 42 seconds — synthesized answer with code references | ||
|
|
||
| That's **90% fewer tokens** and **75% faster** time-to-answer. |
There was a problem hiding this comment.
Highly specific benchmark claims lack reproducibility caveats
The figures 25,000 tokens, 1,500 tokens, 42 seconds, and 3 minutes are labeled "Real session, real numbers" but will vary significantly by repository size, AI model, query complexity, network latency, and TaskWing knowledge-base completeness. Presenting single-datapoint figures as representative benchmarks without any caveat could mislead users whose results differ substantially.
Consider adding a qualifier such as:
| ``` | |
| **Real session, real numbers** — asked *"What are the bottlenecks in our engineering process?"*: | |
| - **Without TaskWing:** 8 Glob/Grep searches, 12 file reads, 25,000 tokens, 3 minutes | |
| - **With TaskWing MCP:** 1 query, 1,500 tokens, 42 seconds — synthesized answer with code references | |
| That's **90% fewer tokens** and **75% faster** time-to-answer. | |
| **Real session, real numbers** *(results vary by repository size and model)* — asked *"What are the bottlenecks in our engineering process?"*: |
Or add a footnote explaining the test environment (repo LOC, model used, knowledge-base node count).
Prompt To Fix With AI
This is a comment left during a code review.
Path: README.md
Line: 50-56
Comment:
**Highly specific benchmark claims lack reproducibility caveats**
The figures `25,000 tokens`, `1,500 tokens`, `42 seconds`, and `3 minutes` are labeled "Real session, real numbers" but will vary significantly by repository size, AI model, query complexity, network latency, and TaskWing knowledge-base completeness. Presenting single-datapoint figures as representative benchmarks without any caveat could mislead users whose results differ substantially.
Consider adding a qualifier such as:
```suggestion
**Real session, real numbers** *(results vary by repository size and model)* — asked *"What are the bottlenecks in our engineering process?"*:
```
Or add a footnote explaining the test environment (repo LOC, model used, knowledge-base node count).
How can I resolve this? If you propose a fix, please make it concise.
1 participant
…inal width handling
Greptile Summary
This PR upgrades the
internal/uipackage from hardcodedlipgloss.Colorvalues tolipgloss.AdaptiveColorthroughout, enabling proper light/dark terminal theme adaptation. It also introducesGetTerminalWidth()and auto-constraining column logic inTable.ColumnWidths(), migratesStyleBoldto use lipgloss (gainingNO_COLORsupport), widens thePanel.BorderColorAPI to accept anylipgloss.TerminalColor, and adds a "Why TaskWing?" performance comparison section to the README.Key issues found:
styles.go(logic): The"note"category inCategoryBadgemaps toColorTextas its badge background. In light modeColorText.Light = "235"resolves to near-black (#262626), producing an inverted dark badge inside a light theme — semantically wrong and visually inconsistent with all other badge types.table.go(logic): The new auto-constrain block inColumnWidths()sumswidths[i]values that are byte lengths (fromlen(h)/len(cell)) and compares them against the terminal column count fromGetTerminalWidth(). For multi-byte Unicode content (emoji, CJK), the byte total overstates visual width, causing unnecessary and incorrect proportional shrinking. ThepadRighthelper has the samelen(s)issue; both should uselipgloss.Width()for visual measurement.README.md(style): The "Real session, real numbers" benchmark figures (token counts, timing) are single-datapoint, environment-specific measurements presented without caveats, which may mislead users whose repositories or models produce different results.Confidence Score: 3/5
ColorTextused as a badge background produces a visually inverted dark badge in light mode, a UX regression, and (2) the new auto-constrain path inColumnWidths()mixes byte counts with terminal column counts, silently producing incorrect column widths for any Unicode content. These are functional regressions introduced by this PR, not pre-existing issues.internal/ui/styles.go(note badge background),internal/ui/table.go(byte-count vs column-count in auto-constrain), andinternal/ui/spinner.go(hardcoded 80-char clear).Important Files Changed
lipgloss.Colortolipgloss.AdaptiveColorfor light/dark theme support; addsColorPurpleandColorBarEmptyshared constants.ColorTextis incorrectly reused as the "note" badge background, producing a near-black badge in light mode (a semantic inversion).ColorTableRowEven/ColorTableRowOddremain declared but are still not applied inStyleTableRowEven/StyleTableRowOdd.GetTerminalWidth()and a new auto-constrain block inColumnWidths(). The constrain logic mixes byte-count widths (len(h),len(cell)) with terminal column count (GetTerminalWidth()), causing incorrect shrink ratios for Unicode content. The header/cell byte-count issue also affectspadRight.\033[K) with a hardcoded 80-space clear. This regresses wide-terminal support and thestringsimport is added specifically forstrings.Repeat.lipgloss.Colorliterals with adaptive color constants; changesscoreToColorreturn type fromlipgloss.Colortolipgloss.TerminalColor. Inlinelipgloss.AdaptiveColor{Light: "6", Dark: "14"}literal is duplicated across two render functions instead of being extracted to a named constant in styles.go.AdaptiveColor.configValueStylenow usesLight: "136"(goldenrod, ~#af8700) as text foreground; on a white terminal background this yields approximately 3:1 contrast, borderline for readability.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Terminal Background Detection\nlipgloss.HasDarkBackground] --> B{Light or Dark?} B -->|Light| C[Resolve AdaptiveColor.Light] B -->|Dark| D[Resolve AdaptiveColor.Dark] C --> E[Named Color Constants\ne.g. ColorPrimary, ColorText, ColorWarning] D --> E E --> F[Package-Level Styles\nStyleTitle, StyleError, StyleSuccess ...] E --> G[In-Function Styles\nconfig_menu, context_view, spinner ...] E --> H[CategoryBadge\nnode type → AdaptiveColor background] F --> I[UI Components] G --> I H --> I I --> J[Table.Render] I --> K[Panel.Render] I --> L[Spinner.Start] I --> M[RenderContextResults] J --> N{MaxWidth set?} N -->|Yes| O[Apply MaxWidth cap per column] N -->|No| P[GetTerminalWidth via os.Stdout\nAuto-constrain column widths] P -->|byte-count total vs column-count available\n⚠️ Mixed units for Unicode| Q[Proportional shrink + min-4 clamp] O --> R[Render string output] Q --> RLast reviewed commit: 3a83f9b