Skip to content

fix(terminal): clamp wide graphemes in narrow table cells#88557

Merged
steipete merged 3 commits into
openclaw:mainfrom
jbetala7:fix/table-wide-grapheme-cell-overflow
May 31, 2026
Merged

fix(terminal): clamp wide graphemes in narrow table cells#88557
steipete merged 3 commits into
openclaw:mainfrom
jbetala7:fix/table-wide-grapheme-cell-overflow

Conversation

@jbetala7

@jbetala7 jbetala7 commented May 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #88556.

renderTable broke its border-alignment invariant when a wide (width-2) CJK/emoji grapheme landed in a column whose content width is 1: wrapLine cannot split a single grapheme, and padCell only padded (never truncated), so the content row rendered one column wider than the borders and pushed the right border out of alignment. The repo already asserts this invariant for wide graphemes at a comfortable width (table.test.ts — "keeps table borders aligned when cells contain wide emoji graphemes"); this PR closes the narrow-column boundary it missed.

renderTable backs many CLI status/list tables (devices, plugins, skills, nodes, pairing, dns), whose cells can hold CJK/emoji names, and columns shrink on narrow terminals — so the misalignment is user-visible.

Change

  • Add truncateToVisibleWidth(input, maxWidth) to packages/terminal-core/src/ansi.ts: drops whole grapheme clusters that would overflow the budget (never emits a half-width glyph) while copying ANSI sequences verbatim, including trailing resets, so a clamped styled cell does not bleed styling into padding/borders. Result always satisfies visibleWidth(result) <= maxWidth.
  • padCell clamps over-wide content through that helper before padding, and the alignment branches emit the clamped content, so every rendered line keeps visibleWidth(line) === width.
  • Tests: unit coverage for truncateToVisibleWidth (budget + ANSI preservation) and two renderTable alignment regression tests (narrow width-1 cell, and a narrow flex column receiving wide content).

Verification

  • node scripts/run-vitest.mjs run packages/terminal-core/src/ansi.test.ts packages/terminal-core/src/table.test.ts → 28 passed.
  • node scripts/run-tsgo.mjs -p tsconfig.core.json (clean, non-incremental) → green.
  • oxfmt written on the four touched files; git diff --check clean.
  • Note: the repo's check-prod-types lane is currently red on main itself due to a pre-existing tsgo:extensions error in extensions/acpx/src/runtime.ts (SessionRecord cast) that this PR does not touch.

Real behavior proof

Behavior addressed: terminal table border misalignment when a width-2 grapheme lands in a width-1 (or otherwise too-narrow) column.
Real environment tested: local Node 24 dev checkout, OpenClaw main @ 444562b, exercising renderTable/padCell directly via node --import tsx.
Exact steps or command run after this patch:

node --import tsx -e '
import { renderTable } from "./packages/terminal-core/src/table.ts";
import { visibleWidth } from "./packages/terminal-core/src/ansi.ts";
const out = renderTable({
  width: 10, border: "ascii",
  columns: [{ key: "A", header: "long header here" }, { key: "B", header: "", flex: true }],
  rows: [{ A: "data", B: "中" }],
});
const lines = out.trimEnd().split("\n");
console.log("line widths:", JSON.stringify(lines.map(visibleWidth)));
console.log(out);
'

Evidence after fix:

line widths: [24,24,24,24,24]
+------------------+---+
| long header here |   |
+------------------+---+
| data             |   |
+------------------+---+

Observed result after fix: all five lines are 24 columns; the right border is aligned. Before the fix the data row measured 25 against 24-wide borders.
What was not tested: live terminal rendering on a physical narrow-width TTY (validated programmatically via visibleWidth); no channel/gateway path is involved.

@jbetala7
fix(terminal): clamp wide graphemes in narrow table cells
6d9724b 
A width-2 CJK/emoji grapheme placed in a column whose content width is 1
cannot be wrapped by wrapLine, and padCell only padded (never truncated),
so the content row rendered one column wider than the borders and pushed
the right border out of alignment. Add an ANSI-aware truncateToVisibleWidth
helper and clamp over-wide cells with it so every rendered line keeps the
documented visibleWidth(line) === width invariant.
@jbetala7 jbetala7 mentioned this pull request May 31, 2026
Closed
@openclaw-barnacle openclaw-barnacle Bot added the proof: supplied External PR includes structured after-fix real behavior proof. label May 31, 2026
@clawsweeper

clawsweeper Bot commented May 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed May 31, 2026, 11:51 AM ET / 15:51 UTC.

Summary
The PR adds an ANSI-aware visible-width truncation helper, applies it in terminal table cell padding, and adds regression tests for narrow cells containing wide CJK/emoji graphemes.

PR surface: Source +51, Tests +58. Total +109 across 4 files.

Reproducibility: yes. Current main's padCell returns over-wide text unchanged when visibleWidth(text) >= width, and wrapLine can pass a width-2 grapheme through a width-1 cell; I did not execute the Node repro in this read-only review.

Review metrics: none identified.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Next step before merge

  • No ClawSweeper repair lane is indicated; the focused contributor PR has no blocking code findings and needs normal maintainer/CI merge handling.

Security
Cleared: The diff is limited to terminal-core rendering and tests, with no dependency, workflow, package-resolution, secret, or new code-execution surface changes.

Review details

Best possible solution:

Land the focused terminal-core clamp with its regression tests after normal maintainer review and required checks, then let it resolve #88556.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main's padCell returns over-wide text unchanged when visibleWidth(text) >= width, and wrapLine can pass a width-2 grapheme through a width-1 cell; I did not execute the Node repro in this read-only review.

Is this the best way to solve the issue?

Yes. Clamping in padCell is the narrowest maintainable fix because it is the final cell-width enforcement point before borders are emitted, and the helper preserves ANSI resets while enforcing the visible-width invariant.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against a71b121c69f0.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live Node output showing all rendered table lines have equal visible widths, with a before-fix comparison and no channel/gateway path involved.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P2: This is a normal-priority CLI terminal rendering correctness fix with limited blast radius in shared table output.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live Node output showing all rendered table lines have equal visible widths, with a before-fix comparison and no channel/gateway path involved.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live Node output showing all rendered table lines have equal visible widths, with a before-fix comparison and no channel/gateway path involved.
Evidence reviewed

PR surface:

Source +51, Tests +58. Total +109 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 57 6 +51
Tests 2 59 1 +58
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 116 7 +109

What I checked:

  • Root policy read: Read the full root AGENTS.md from disk and applied its OpenClaw PR review, proof, security, and history guidance; no scoped packages/AGENTS.md was present for packages/terminal-core. (AGENTS.md:1, a71b121c69f0)
  • Current main root cause: On current main, padCell returns the original text unchanged when visibleWidth(text) >= width, so an unsplittable width-2 grapheme can remain wider than a width-1 content cell. (packages/terminal-core/src/table.ts:50, a71b121c69f0)
  • Patch clamps the implicated path: The PR head imports truncateToVisibleWidth and clamps over-wide cell content before alignment padding, then computes padding from the clamped visible width. (packages/terminal-core/src/table.ts:54, 7a697bc277ef)
  • ANSI-aware truncation helper: The new helper drops grapheme clusters that would overflow the visible-width budget while still copying ANSI/OSC sequences such as trailing resets/link closes. (packages/terminal-core/src/ansi.ts:130, 7a697bc277ef)
  • Regression tests added: The PR adds table tests for a width-1 fixed cell and a narrow flex column, both asserting equal visible widths for all rendered lines with wide content. (packages/terminal-core/src/table.test.ts:203, 7a697bc277ef)
  • Helper tests added: The PR adds unit coverage for truncating ASCII, CJK, emoji, zero budget, and styled ANSI strings without exceeding the requested visible width. (packages/terminal-core/src/ansi.test.ts:44, 7a697bc277ef)

Likely related people:

  • steipete: Current-main blame for the terminal-core package snapshot points to Peter Steinberger on the central padCell and ANSI helper files. (role: recent area contributor; confidence: high; commits: 3ebbf9a0c1b9; files: packages/terminal-core/src/table.ts, packages/terminal-core/src/ansi.ts, packages/terminal-core/src/table.test.ts)
  • vincentkoc: Older terminal table history shows Vincent Koc adding grapheme display-width handling, table wrapping, and the existing wide-emoji alignment coverage this PR extends. (role: feature-history owner; confidence: high; commits: 04e103d10ef7, 1ec49e33f3e7, 4efe7a4dcd0b; files: src/terminal/table.ts, src/terminal/ansi.ts, src/terminal/table.test.ts)
  • Shakker: Recent history includes an ANSI helper change around OSC payload stripping, which is adjacent to this PR's ANSI-preserving truncation behavior. (role: adjacent ANSI contributor; confidence: medium; commits: 4c7bffc3a725; files: src/terminal/ansi.ts, src/terminal/ansi.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels May 31, 2026
@byungskers

byungskers commented May 31, 2026

Copy link
Copy Markdown

Good catch on the width-2 grapheme overflow case, and I like that you covered both CJK and emoji paths. One edge case worth considering: do we need an explicit test for OSC8 hyperlinks or other zero-width control sequences that span past the truncation point? truncateToVisibleWidth() keeps ANSI sequences flowing after the visible budget is spent, which is great for resets, but a hyperlink open/close pair would be another nice invariant to lock down.

@steipete steipete self-assigned this May 31, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 31, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels May 31, 2026
@steipete

steipete commented May 31, 2026

Copy link
Copy Markdown
Contributor

Land-ready proof for #88557.

Summary:

Local proof:

  • node scripts/run-vitest.mjs run packages/terminal-core/src/ansi.test.ts packages/terminal-core/src/table.test.ts
  • Result: 2 files, 28 tests passed.
  • Issue repro table now renders line widths [24, 24, 24, 24, 24].

CI proof:

  • PR CI run 26717035619: all PR-relevant terminal/core/build/type/lint/security lanes passed.
  • check-dependencies is red for unrelated current-main deadcode: ui/src/ui/browser-redact.ts. Same failure is present on current main run 26717029674 at a71b121.
  • checks-node-agentic-agents-core was rerun once and still fails in src/agents/bash-tools*.test.ts. Those files are outside this PR diff and unrelated to terminal table rendering.

Known proof gap:

  • Merging requires maintainer/admin override for the unrelated red CI lanes above.

@steipete
steipete merged commit 29dd784 into openclaw:main May 31, 2026
249 of 255 checks passed
@clawsweeper clawsweeper Bot mentioned this pull request Jun 17, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: renderTable misaligns borders when a wide CJK/emoji grapheme lands in a narrow (width-1) column

3 participants

@jbetala7 @byungskers @steipete