Skip to content

fix(tui): truncate the spinner label by display width, not rune count - #1433

Merged
dennisonbertram merged 1 commit into
mainfrom
fix/1432-wide-rune-truncation
Sep 8, 2026
Merged

fix(tui): truncate the spinner label by display width, not rune count#1433
dennisonbertram merged 1 commit into
mainfrom
fix/1432-wide-rune-truncation

Conversation

@dennisonbertram

Copy link
Copy Markdown
Owner

Closes #1432

The bug

shortenLabel exists to guarantee one thing: at narrow widths the label yields so the (esc to interrupt) hint survives, because the hint is the only actionable part of that line. For double-width characters it did the opposite.

width=40  "· Waiting for 模型模型模型… (esc to inte"
width=40  "· Running 🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀… (esc to "
width=50  "· Waiting for 模型模型模型模型模型模型模型模型 (es"

Cause

Two units, mixed. The budget was computed in display columns:

budget := width - lipgloss.Width(glyph) - lipgloss.Width(CancelHint) - 2

and the truncation was applied in runes:

runes := []rune(label)
if len(runes) > budget {
    label = string(runes[:budget-1]) + "…"
}

CJK and emoji occupy two columns per rune, so a rune-budgeted label could be twice its allowance in columns. The line overran, and the final MaxWidth clamp trimmed from the right — taking the hint.

The rendered width was never wrong, which is exactly why this hid. The clamp always produced a correctly-sized line; it just produced the wrong one.

Fix

truncateToWidth accumulates lipgloss.Width per rune until the budget is reached, counting the ellipsis's own width.

Deliberately not grapheme-cluster aware: a combining mark or ZWJ emoji sequence can still be split. That is a strict improvement over rune counting and a far smaller change than full segmentation, which stays out of scope until a real case appears rather than being built speculatively.

After

w=40  "· Waiting for 模型模… (esc to interrupt)"
w=50  "· Waiting for 模型模型模型模型… (esc to interrupt)"
w=40  "· Running 🚀🚀🚀🚀🚀… (esc to interrupt)"
w=40  "· Waiting for some-l… (esc to interrupt)"
w=40  "· Running bash (esc to interrupt)"          (unchanged, fits)

Verification

TestSpinnerKeepsCancelHintWithWideRunes — CJK, emoji, and an ASCII control at widths 40 and 50. Red first on both wide cases:

width 40: cancel hint truncated, got "· Waiting for 模型模型模型… (esc to inte"
width 40: cancel hint truncated, got "· Running 🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀… (esc to "

It asserts the hint's survival, not the line's width. Asserting width is what let this hide — the width was always right. The ASCII control guards the other direction: a fix that satisfied the hint assertion by over-truncating every label would show up there as a needlessly stunted rendering.

Full TUI suite green under -race. Rendered output inspected directly, not only asserted.

How this was found

By gpt-6-astra through the Surplus proxy — an untrusted external reviewer handed the packed source with no tools and no filesystem access — then confirmed locally by rendering the cases before believing any of it.

Our own tests were green throughout and stayed green, because they were ASCII-only. The code asserted a guarantee in a comment that it did not keep, and an outside reader that could not run anything caught what the suite structurally could not.

Honest signal-to-noise from that review: three findings, one confirmed (this), one unconfirmed and overlapping with it (a claim that MaxWidth may wrap rather than clip — not observed), one false positive (elapsed-time nondeterminism in snapshots, which set startTime explicitly).

🤖 Generated with Claude Code

https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5

shortenLabel guarantees the cancel hint survives at narrow widths. For
double-width characters it did the opposite: the hint was cut off while
the label stayed long.

The function budgeted in display columns via lipgloss.Width but
truncated by rune count, and CJK and emoji occupy two columns per rune.
The label overran its allowance, and the final MaxWidth clamp trimmed
from the right — taking the hint. The rendered width was always correct,
which is why nothing caught it, and every existing test passed because
they were all ASCII, where columns and runes are the same number.

truncateToWidth now accumulates display width per rune, counting the
ellipsis. It is deliberately not grapheme-aware; splitting a combining
mark or ZWJ sequence remains possible, which is still strictly better
than rune counting and much smaller than full segmentation.

Found by the untrusted external reviewer (gpt-6-astra via the Surplus
proxy) reading the packed source with no tools, then confirmed locally by
rendering the cases rather than taking the report at face value.

Closes #1432

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-08T17:46:58.622331Z 1fe3b02 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@dennisonbertram
dennisonbertram merged commit 0391d15 into main Sep 8, 2026
2 checks passed
@dennisonbertram
dennisonbertram deleted the fix/1432-wide-rune-truncation branch September 8, 2026 17:45

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1fe3b02af1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +138 to +140
// ASCII control: a fix that over-truncates everything to satisfy the
// hint assertion would show up here as a needlessly stunted label.
{name: "ascii control", label: "Waiting for some-long-model-name-v2"},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Make the ASCII control assert retained label text

This control does not detect the over-truncation it claims to guard against: an implementation that reduces every overlong label to only (or drops it entirely) still passes because the shared assertions check only that the hint survives and the result fits. Assert an expected retained ASCII prefix or minimum label width so this case actually distinguishes correct truncation from needless label loss.

AGENTS.md reference: AGENTS.md:L31-L31

Useful? React with 👍 / 👎.

Comment on lines +316 to +318
for _, r := range label {
w := lipgloss.Width(string(r))
if width+w > room {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Truncate using whole grapheme display widths

For multi-rune emoji such as ❤️ or 1️⃣, display width is not additive per rune: the variation-selector/keycap runes measure as zero alone, while the completed grapheme occupies two columns. This loop therefore undercharges these labels, can return a value wider than budget, and causes View's final MaxWidth clamp to truncate the cancel hint again at narrow widths. Iterate grapheme clusters (or use an ANSI/grapheme-aware width truncator) so the accumulated width matches lipgloss.Width of the returned prefix.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: spinner truncates by rune count but budgets by display width, so wide labels destroy the cancel hint

1 participant