Skip to content

fix(tui): truncate helpers can return a longer string than they got - #41060

Closed
Mute-404 wants to merge 1 commit into
anomalyco:devfrom
Mute-404:truncate-clamp
Closed

Mute-404 wants to merge 1 commit into
anomalyco:devfrom
Mute-404:truncate-clamp

Conversation

@Mute-404

@Mute-404 Mute-404 commented Aug 7, 2026 •

Copy link
Copy Markdown

Issue for this PR

Closes #37458

(I opened #41054 before finding #37458. Closing mine as a duplicate — this PR targets the existing one. Same bug was also reported in #20880 and #34395, and a fix in #34482 was closed by the automated PR cleanup in July rather than reviewed.)

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

truncateLeft and truncateMiddle in packages/tui/src/util/locale.ts compute a slice length that reaches zero or goes negative on small budgets. String.prototype.slice(-0) is slice(0), which returns the whole string, so both helpers hand back something longer than the input:

truncateMiddle("abcdefghij", 0) -> "…bcdefghij"
truncateMiddle("abcdefghij", 1) -> "…abcdefghij"
truncateLeft("abcdefghij", 1)   -> "…abcdefghij"

They grow the text exactly when the terminal has the least room for it.

Clamped inside both helpers so callers don't each need their own guard. truncateMiddle also got restructured slightly: keepEnd is now checked before slicing, which is what removes the slice(-0) path.

Worth noting for review: the Math.max(1, ...) at dialog-select.tsx:702 and dialog-move-session.tsx:151 looks like a guard against this, but 1 is inside the broken range, so it clamps straight into it. files.tsx:35 uses Math.max(2, ...) and happens to land just outside. After this change those clamps are redundant but harmless, so I left them alone to keep the diff focused.

How did you verify your code works?

Ran the actual call site expressions from the TUI against the real module, on dev and on this branch:

term | callsite                     | budget | before | after
  10 | dialog-select.tsx:702        |      1 |     40 |     1
  10 | dialog-move-session.tsx:151  |      1 |     40 |     1
  12 | dialog-select.tsx:702        |      1 |     40 |     1
  14 | dialog-select.tsx:702        |      2 |     41 |     2
  16 | dialog-move-session.tsx:151  |      2 |     41 |     2

9 call sites returned more than their budget before, 0 after. Output stays correct for normal widths (truncateMiddle("...", 5) gives "ab…ij").

$ bun test test/util-locale.test.ts
 4 pass
 0 fail
 20 expect() calls

util/locale.ts had no tests, which is presumably how this survived three reports. The tests assert the budget is never exceeded across the whole small range rather than checking specific strings.

Two things I deliberately left out:

  • packages/core/src/util/path.ts:31 has a byte-identical copy of the same bug. Different package, seemed like it should be its own PR — happy to include it here if you'd prefer.
  • These helpers budget in str.length (UTF-16 code units) while callers are budgeting terminal columns, so CJK and emoji still under-truncate. packages/tui/src/prompt/display.ts:7 shows the house approach with Bun.stringWidth + Intl.Segmenter. That's a behavior change and needs its own discussion.

Screenshots / recordings

This only shows up below roughly 16 columns, where the layout is already degraded. The table above is the measurable version.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Both helpers compute a slice length that goes to zero or negative on
small budgets, and slice(-0) hands back the whole string. So they grow
the text right when the terminal has the least room for it:

  truncateMiddle("abcdefghij", 0) === "…bcdefghij"
  truncateLeft("abcdefghij", 1)   === "…abcdefghij"

At 10 columns dialog-select.tsx:702 asks for 1 char and gets 40 back.
Same for dialog-move-session.tsx:151. The Math.max(1, ...) at those call
sites reads like a guard but clamps straight into the broken range.

Clamped inside the helpers so callers don't each need their own guard.
Adds tests for util/locale, which had none.
@github-actions github-actions Bot added needs:compliance This means the issue will auto-close after 2 hours. and removed needs:compliance This means the issue will auto-close after 2 hours. labels Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for updating your PR! It now meets our contributing guidelines. 👍

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Automated PR Cleanup

Thank you for contributing to opencode.

Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions.

This PR was closed because it matched the following cleanup criteria:

  • The PR was created more than 1 month ago
  • The PR had fewer than 2 positive reactions
  • Positive reactions are counted as thumbs-up, heart, celebration, or rocket reactions on the PR

PRs created within the last month are not affected by this cleanup.

If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate.

Thanks again for taking the time to contribute.

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.

truncateMiddle returns a string longer than maxLength when maxLength is 1 or 2

1 participant