fix(cli): align session tables with CJK content via display-width padding#44211
Open
AIalliAI wants to merge 1 commit into
Open
fix(cli): align session tables with CJK content via display-width padding#44211AIalliAI wants to merge 1 commit into
AIalliAI wants to merge 1 commit into
Conversation
…ding The /resume inline list (HermesCLI._show_recent_sessions) and the `hermes sessions list` table (cmd_sessions) pad cells with str.format width specs and truncate with str slices, both of which count characters rather than terminal columns. CJK characters are double-width, so any title or preview containing them shifted every column to its right. Add _disp_width/_clip_to_width/_pad_to_width helpers to hermes_cli/main.py — the same wcwidth-based approach (and -1-clamping convention) agent/markdown_tables.py already uses for this problem — and use them in both renderers. wcwidth is already a transitive guarantee via prompt_toolkit. Output is byte-identical for pure-ASCII rows, and titles in the /resume table still render unclipped (NousResearch#14082); only the preview is clipped there, by columns instead of characters. Fixes NousResearch#44199 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Requesting maintainer review — this is ready to land from my side. Standalone fork CI is pending first-run approval here; the rollup branch in #44061 carrying this session's batch is fully green on upstream CI (all test shards, typecheck, e2e). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #44199
Problem
The
/resumeinline list (HermesCLI._show_recent_sessions, cli.py) and thehermes sessions listtable (cmd_sessions, hermes_cli/main.py) pad cells withstr.formatwidth specs ({var:<32}) and truncate withstrslices ([:38]). Both count characters, but CJK characters occupy two terminal columns, so any title or preview containing them shifts every column to its right:Fix
Rather than switching these renderers to
rich.table.Table, this adds three small display-width helpers tohermes_cli/main.py—_disp_width/_clip_to_width/_pad_to_width— using the samewcwidth-based approach (including the clamp-negative-to-zero convention) thatagent/markdown_tables.pyalready uses for exactly this problem. Both renderers now truncate and pad by terminal columns._clip_to_widthnever splits a double-width character across the column boundary.This keeps the diff minimal and the output byte-identical for pure-ASCII rows, so nothing depending on the current format changes.
wcwidthis already imported unconditionally byagent/markdown_tables.pyand is guaranteed present transitively via the pinnedprompt_toolkit.After:
Notes on scope
/resumetable are padded but deliberately not clipped: /resume fails on title match because display truncates titles to 30 chars #14082 made long titles render in full there (enforced bytest_resume_list_shows_full_long_titles), so an over-long title still trades alignment for visibility, exactly as on main. Only the preview is clipped — now by columns instead of characters.cmd_sessionskeeps its existing[:30]title truncation semantics, just measured in columns.Tests
tests/hermes_cli/test_session_table_width.py(new): unit coverage for the three helpers — CJK widths, never splitting a double-width char at the boundary, negative-wcswidthclamping, ASCII equivalence withstr.ljust.tests/cli/test_cli_resume_command.py: new test asserting the ID column starts at the same display column for a CJK row and an ASCII row.tests/cli/suite passes (the one failure,test_resume_quiet_stderr.py::test_session_not_found_goes_to_stdout_in_full_mode, is a pre-existing test-ordering flake that fails identically on pristineorigin/mainwhen the whole directory runs together, and passes in isolation both there and on this branch).🤖 Generated with Claude Code