mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-18 04:41:56 +00:00
fix(cli): vertical fallback for markdown tables wider than terminal (#23948)
Follow-up to #23863 (CJK table alignment). The realigner was correctly padding pipes to identical column offsets, but when a table's natural width exceeds terminal cells it produced lines that the terminal soft-wrapped mid-cell, destroying column alignment visually even though the bytes were perfectly padded. Reported as 'columns are not aligned' on tables containing one long row alongside several short rows. Approach mirrors Claude Code's MarkdownTable.tsx narrow-terminal fallback: when realign_markdown_tables is given an available_width budget and the rebuilt horizontal table exceeds it, render each body row as 'Header: value' lines separated by a thin ─ rule. Word-wraps oversize values at the budget with a 2-space continuation indent. - agent/markdown_tables.py: realign_markdown_tables(text, available_width=None); threshold check at the top of _render_block flips into a new _render_vertical fallback. Includes _wrap_to_width with hard-break for tokens longer than the budget. - cli.py: helper _terminal_width_for_streaming() returns shutil.get_terminal_size().columns minus _STREAM_PAD and a 2-cell safety margin; passed to all three realign call sites (_render_final_assistant_content for strip+render Panel paths, and the streaming flushers in _emit_stream_text / _flush_stream). - tests/agent/test_markdown_tables.py: 4 new tests covering the overflow-vertical fallback for ASCII + CJK content, the 'fits → keep horizontal' case, and the long-cell wrap with indent. Live-verified: with COLUMNS=100, the user's reported 'long row in ASCII table' case now renders as vertical key-value rows that all fit the panel; the 6-column CJK comparison table still renders as an aligned horizontal table because it fits inside 100 cols.
This commit is contained in:
parent
825bd50e6b
commit
ea1d0462cf
3 changed files with 281 additions and 8 deletions
|
|
@ -102,8 +102,16 @@ def looks_like_table_row(row: str) -> bool:
|
|||
return stripped.count("|") >= 2
|
||||
|
||||
|
||||
def _render_block(rows: List[List[str]]) -> List[str]:
|
||||
"""Render ``rows`` (header + body, divider implied) at uniform widths."""
|
||||
def _render_block(rows: List[List[str]], available_width: int | None = None) -> List[str]:
|
||||
"""Render ``rows`` (header + body, divider implied) at uniform widths.
|
||||
|
||||
If ``available_width`` is given and the rebuilt horizontal table
|
||||
would exceed it, fall back to a vertical key-value rendering so
|
||||
rows do not soft-wrap mid-cell — terminal soft-wrap destroys
|
||||
column alignment visually even when the underlying bytes are
|
||||
perfectly padded, which is exactly the "tables look broken"
|
||||
user report this code path is meant to address.
|
||||
"""
|
||||
|
||||
ncols = max(len(r) for r in rows)
|
||||
rows = [r + [""] * (ncols - len(r)) for r in rows]
|
||||
|
|
@ -113,6 +121,13 @@ def _render_block(rows: List[List[str]]) -> List[str]:
|
|||
for c in range(ncols)
|
||||
]
|
||||
|
||||
# Total horizontal width for the rendered row:
|
||||
# `| ` + cell + ` ` for each column, plus the final closing `|`.
|
||||
horizontal_width = sum(widths) + 3 * ncols + 1
|
||||
|
||||
if available_width is not None and horizontal_width > max(available_width, 20):
|
||||
return _render_vertical(rows, ncols, available_width)
|
||||
|
||||
def _row(cells: List[str]) -> str:
|
||||
return (
|
||||
"| "
|
||||
|
|
@ -127,11 +142,135 @@ def _render_block(rows: List[List[str]]) -> List[str]:
|
|||
return out
|
||||
|
||||
|
||||
def realign_markdown_tables(text: str) -> str:
|
||||
def _wrap_to_width(text: str, width: int) -> List[str]:
|
||||
"""Soft-wrap ``text`` at word boundaries to fit ``width`` display cells.
|
||||
|
||||
Falls back to hard-breaking the longest word if a single token is
|
||||
wider than ``width``. Empty input yields a single empty string so
|
||||
the caller's row count stays predictable.
|
||||
"""
|
||||
|
||||
if width <= 0 or not text:
|
||||
return [text]
|
||||
|
||||
words = text.split()
|
||||
if not words:
|
||||
return [""]
|
||||
|
||||
lines: List[str] = []
|
||||
current = ""
|
||||
current_w = 0
|
||||
|
||||
def _hard_break(word: str, w: int) -> List[str]:
|
||||
out: List[str] = []
|
||||
buf = ""
|
||||
bw = 0
|
||||
for ch in word:
|
||||
cw = _disp_width(ch) or 1
|
||||
if bw + cw > w and buf:
|
||||
out.append(buf)
|
||||
buf = ch
|
||||
bw = cw
|
||||
else:
|
||||
buf += ch
|
||||
bw += cw
|
||||
if buf:
|
||||
out.append(buf)
|
||||
return out
|
||||
|
||||
for word in words:
|
||||
ww = _disp_width(word)
|
||||
if not current:
|
||||
if ww <= width:
|
||||
current = word
|
||||
current_w = ww
|
||||
else:
|
||||
pieces = _hard_break(word, width)
|
||||
lines.extend(pieces[:-1])
|
||||
current = pieces[-1] if pieces else ""
|
||||
current_w = _disp_width(current)
|
||||
continue
|
||||
if current_w + 1 + ww <= width:
|
||||
current += " " + word
|
||||
current_w += 1 + ww
|
||||
else:
|
||||
lines.append(current)
|
||||
if ww <= width:
|
||||
current = word
|
||||
current_w = ww
|
||||
else:
|
||||
pieces = _hard_break(word, width)
|
||||
lines.extend(pieces[:-1])
|
||||
current = pieces[-1] if pieces else ""
|
||||
current_w = _disp_width(current)
|
||||
if current:
|
||||
lines.append(current)
|
||||
return lines or [""]
|
||||
|
||||
|
||||
def _render_vertical(
|
||||
rows: List[List[str]], ncols: int, available_width: int
|
||||
) -> List[str]:
|
||||
"""Render a too-wide table as vertical ``Header: value`` rows.
|
||||
|
||||
Mirrors Claude Code's narrow-terminal fallback in
|
||||
``MarkdownTable.tsx``: each body row becomes a small block of
|
||||
``Header: cell-value`` lines (continuation lines indented two
|
||||
spaces) separated by a thin ``─`` divider between rows. Keeps
|
||||
every line narrower than ``available_width`` so the terminal does
|
||||
not soft-wrap mid-cell.
|
||||
"""
|
||||
|
||||
if not rows:
|
||||
return []
|
||||
|
||||
headers = rows[0] + [""] * (ncols - len(rows[0]))
|
||||
body = rows[1:]
|
||||
|
||||
labels = [h or f"Column {i + 1}" for i, h in enumerate(headers)]
|
||||
|
||||
sep_width = max(20, min(40, available_width - 2)) if available_width else 30
|
||||
separator = "─" * sep_width
|
||||
indent = " "
|
||||
indent_w = _disp_width(indent)
|
||||
|
||||
out: List[str] = []
|
||||
for ri, row in enumerate(body):
|
||||
if ri > 0:
|
||||
out.append(separator)
|
||||
for ci in range(ncols):
|
||||
label = labels[ci]
|
||||
value = row[ci] if ci < len(row) else ""
|
||||
label_w = _disp_width(label)
|
||||
first_budget = max(10, available_width - label_w - 2)
|
||||
cont_budget = max(10, available_width - indent_w)
|
||||
if not value:
|
||||
out.append(f"{label}:")
|
||||
continue
|
||||
wrapped = _wrap_to_width(value, first_budget)
|
||||
out.append(f"{label}: {wrapped[0]}")
|
||||
if len(wrapped) > 1:
|
||||
# Re-flow continuation text at the wider continuation
|
||||
# budget — words split across the narrower first-line
|
||||
# budget should re-pack greedily for the rest.
|
||||
cont_text = " ".join(wrapped[1:])
|
||||
for cl in _wrap_to_width(cont_text, cont_budget):
|
||||
if cl.strip():
|
||||
out.append(f"{indent}{cl}")
|
||||
return out
|
||||
|
||||
|
||||
def realign_markdown_tables(text: str, available_width: int | None = None) -> str:
|
||||
"""Rewrite every ``| ... |`` + divider block with wcwidth-aware padding.
|
||||
|
||||
Lines that are not part of a recognised table are returned verbatim,
|
||||
so this is safe to apply to arbitrary assistant prose.
|
||||
|
||||
If ``available_width`` is given (terminal cells available for the
|
||||
rendered table), tables wider than that are rendered as vertical
|
||||
key-value pairs instead of a horizontal pipe-bordered grid. This
|
||||
avoids the terminal soft-wrapping mid-cell, which destroys column
|
||||
alignment visually even when the bytes are perfectly padded.
|
||||
"""
|
||||
|
||||
if "|" not in text:
|
||||
|
|
@ -161,7 +300,7 @@ def realign_markdown_tables(text: str) -> str:
|
|||
j += 1
|
||||
|
||||
if any(c for c in header) or body:
|
||||
out.extend(_render_block([header] + body))
|
||||
out.extend(_render_block([header] + body, available_width))
|
||||
i = j
|
||||
continue
|
||||
out.append(line)
|
||||
|
|
|
|||
40
cli.py
40
cli.py
|
|
@ -1354,16 +1354,48 @@ def _preserve_windows_dot_segments_for_markdown(text: str) -> str:
|
|||
return _WINDOWS_PATH_WITH_DOT_SEGMENT_RE.sub(_protect, text)
|
||||
|
||||
|
||||
def _terminal_width_for_streaming() -> int:
|
||||
"""Display cells available inside the streamed response box.
|
||||
|
||||
The streaming path indents every line by ``_STREAM_PAD`` (4 cells)
|
||||
inside an open response panel. The realigner uses this number as
|
||||
its budget when deciding whether to keep a horizontal table or
|
||||
fall back to vertical key-value rendering. We subtract a small
|
||||
safety margin so terminal-resize races don't push a borderline
|
||||
table into mid-cell soft-wrap.
|
||||
"""
|
||||
|
||||
try:
|
||||
cols = shutil.get_terminal_size((80, 24)).columns
|
||||
except Exception:
|
||||
cols = 80
|
||||
return max(20, cols - len(_STREAM_PAD) - 2)
|
||||
|
||||
|
||||
def _render_final_assistant_content(text: str, mode: str = "render"):
|
||||
"""Render final assistant content as markdown, stripped text, or raw text."""
|
||||
from rich.markdown import Markdown
|
||||
|
||||
# Estimate the cells available to the rendered table. The Panel
|
||||
# used by the background-task / final-response path has 4 cells of
|
||||
# left+right padding plus 1 cell of border on each side, plus the
|
||||
# _STREAM_PAD indent that streamed content uses. Subtract a small
|
||||
# safety margin so resize races don't push a borderline table into
|
||||
# soft-wrap.
|
||||
try:
|
||||
cols = shutil.get_terminal_size((80, 24)).columns
|
||||
except Exception:
|
||||
cols = 80
|
||||
panel_width = max(20, cols - 12)
|
||||
|
||||
normalized_mode = str(mode or "render").strip().lower()
|
||||
if normalized_mode == "strip":
|
||||
# Strip first — inline markdown inside cells (`code`, **bold**, ~~strike~~)
|
||||
# changes cell display width — then re-align so the column padding
|
||||
# reflects the final visible text, not the marker-decorated source.
|
||||
return _RichText(realign_markdown_tables(_strip_markdown_syntax(text)))
|
||||
return _RichText(
|
||||
realign_markdown_tables(_strip_markdown_syntax(text), panel_width)
|
||||
)
|
||||
if normalized_mode == "raw":
|
||||
return _rich_text_from_ansi(text or "")
|
||||
|
||||
|
|
@ -1374,7 +1406,7 @@ def _render_final_assistant_content(text: str, mode: str = "render"):
|
|||
# (narrow panels, etc.) at least see consistent input.
|
||||
plain = _rich_text_from_ansi(text or "").plain
|
||||
plain = _preserve_windows_dot_segments_for_markdown(plain)
|
||||
plain = realign_markdown_tables(plain)
|
||||
plain = realign_markdown_tables(plain, panel_width)
|
||||
return Markdown(plain)
|
||||
|
||||
|
||||
|
|
@ -3662,7 +3694,7 @@ class HermesCLI:
|
|||
joined = "\n".join(buf)
|
||||
if self.final_response_markdown == "strip":
|
||||
joined = _strip_markdown_syntax(joined)
|
||||
block = realign_markdown_tables(joined)
|
||||
block = realign_markdown_tables(joined, _terminal_width_for_streaming())
|
||||
for ln in block.split("\n"):
|
||||
_emit_one(ln)
|
||||
|
||||
|
|
@ -3726,7 +3758,7 @@ class HermesCLI:
|
|||
self._in_stream_table = False
|
||||
if self.final_response_markdown == "strip":
|
||||
joined = _strip_markdown_syntax(joined)
|
||||
block = realign_markdown_tables(joined)
|
||||
block = realign_markdown_tables(joined, _terminal_width_for_streaming())
|
||||
for ln in block.split("\n"):
|
||||
_cprint(f"{_STREAM_PAD}{_tc}{ln}{_RST}" if _tc else f"{_STREAM_PAD}{ln}")
|
||||
|
||||
|
|
|
|||
|
|
@ -156,6 +156,108 @@ def test_passes_non_table_lines_through_around_a_table():
|
|||
assert all(o == offsets[0] for o in offsets)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Vertical fallback for tables wider than the terminal
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_overflow_falls_back_to_vertical_when_table_wider_than_terminal():
|
||||
"""A horizontal table that would exceed the available width must
|
||||
drop to vertical key-value rendering so the terminal does not
|
||||
soft-wrap mid-cell (which destroys column alignment visually)."""
|
||||
|
||||
src = dedent(
|
||||
"""\
|
||||
| Item | Description | Notes |
|
||||
|------|-------------|-------|
|
||||
| a | short | ok |
|
||||
| b | this is a much longer description that stretches the column wider than the others by a lot | fine |
|
||||
| c | tiny | - |
|
||||
"""
|
||||
)
|
||||
|
||||
out = realign_markdown_tables(src, available_width=100)
|
||||
|
||||
# No horizontal pipe-bordered rows: vertical mode emits "Header: value"
|
||||
# lines and a ─ separator instead.
|
||||
assert "|" not in out
|
||||
assert "Item: a" in out
|
||||
assert "Description: short" in out
|
||||
assert "Notes: ok" in out
|
||||
# Body rows separated by ─ rule
|
||||
assert "──" in out
|
||||
|
||||
# Every emitted line fits the available width.
|
||||
for line in out.split("\n"):
|
||||
assert wcswidth(line) <= 100, f"line wider than budget: {line!r}"
|
||||
|
||||
|
||||
def test_horizontal_kept_when_table_fits():
|
||||
"""A table that fits the terminal must keep the horizontal
|
||||
pipe-bordered rendering — vertical fallback only kicks in when
|
||||
soft-wrap is unavoidable."""
|
||||
|
||||
src = dedent(
|
||||
"""\
|
||||
| Name | Age |
|
||||
|------|-----|
|
||||
| Alice | 30 |
|
||||
| Bob | 25 |
|
||||
"""
|
||||
)
|
||||
|
||||
out = realign_markdown_tables(src, available_width=100)
|
||||
|
||||
# Pipe-bordered rendering survives.
|
||||
body_rows = [ln for ln in out.split("\n") if ln.strip().startswith("|")]
|
||||
assert len(body_rows) == 4
|
||||
offsets = [_column_offsets(r) for r in body_rows]
|
||||
assert all(o == offsets[0] for o in offsets)
|
||||
|
||||
|
||||
def test_vertical_fallback_wraps_long_cell_text_with_indent():
|
||||
src = dedent(
|
||||
"""\
|
||||
| Key | Value |
|
||||
|-----|-------|
|
||||
| x | this value is long enough that wrapping the value to fit a narrow terminal width is required even in vertical mode |
|
||||
"""
|
||||
)
|
||||
|
||||
out = realign_markdown_tables(src, available_width=60)
|
||||
|
||||
lines = out.split("\n")
|
||||
assert lines[0].startswith("Key: x")
|
||||
# First "Value:" line + at least one continuation indented by 2 spaces.
|
||||
value_idx = next(i for i, l in enumerate(lines) if l.startswith("Value:"))
|
||||
assert lines[value_idx + 1].startswith(" ")
|
||||
# Every line still fits the budget.
|
||||
for line in lines:
|
||||
assert wcswidth(line) <= 60
|
||||
|
||||
|
||||
def test_overflow_falls_back_to_vertical_for_cjk_too():
|
||||
"""CJK content can also push a table over the terminal budget;
|
||||
the vertical fallback should kick in regardless of script."""
|
||||
|
||||
src = dedent(
|
||||
"""\
|
||||
| 模型 | 描述 | 备注 |
|
||||
|------|------|------|
|
||||
| 千问 | 一个相当长的描述用于把列宽撑得超过可用终端宽度从而触发竖排回退 | 通过 |
|
||||
| 文心 | 短 | × |
|
||||
"""
|
||||
)
|
||||
|
||||
out = realign_markdown_tables(src, available_width=50)
|
||||
|
||||
assert "|" not in out
|
||||
assert "模型: 千问" in out
|
||||
assert "模型: 文心" in out
|
||||
for line in out.split("\n"):
|
||||
assert wcswidth(line) <= 50, f"line wider than budget: {line!r}"
|
||||
|
||||
|
||||
def test_handles_ragged_rows_by_padding_short_rows():
|
||||
src = dedent(
|
||||
"""\
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue