fix(tui): width-aware markdown table rendering with vertical fallback… - #1
Conversation
…#26195) * refactor(tui): thread cols through Md/StreamingMd/renderTable, update cache key * feat(tui): three-tier width calc + full-line string rendering in renderTable Replaces the old renderTable (L203-244) with: - Empty table guard - Ragged row normalization - Three-tier column width calculation (ideal → proportional shrink → hard scale) - Rounding remainder distribution - Full-line string rendering (one <Text> per row, not per cell) - wrap=truncate-end on all table lines - All cells rendered as plain text via stripInlineMarkup No wrapping or vertical fallback yet — those come in Phase 3 and 4. * feat(tui): wrapCell with grapheme-safe hard-break + multi-line row rendering Adds: - Intl.Segmenter-based grapheme splitting (fallback to [...word]) - wrapCell() for width-correct word wrapping on stripped text - Multi-line row rendering with LineEntry metadata (header/separator/body) - Post-render safety condition (maxLineWidth computed, vertical fallback in Task 4) - Non-wrapping path preserved for tables that fit at ideal widths * feat(tui): vertical key-value fallback with scaled threshold + safety check Wires: - Scaled row-height threshold (numCols<=3: 8, <=6: 5, else: 4) - Post-render safety check (maxLineWidth > available space) - Header-only edge case - Vertical format: bold headers, stripped cell text, clamped separator width - Iterates headers (not rows) for consistent key-value fields on ragged rows * test(tui): pass cols to Md in test helpers, add width-overflow assertions - renderAtWidth now passes cols={columns} to <Md> so width-aware code paths are exercised in tests - tableFuzz: every rendered line must fit within allocated width (stringWidth) - tableRepro: separator regex updated to match truncation ellipsis - stringWidth imported from @hermes/ink for CJK-correct assertions * fix(tui): address adversarial review — comment tier 3 budget overshoot, eliminate redundant wrapCell - Add comment on Tier 3 MIN_COL_WIDTH clamp exceeding budget (self-heals via safetyOverflow) - Track tallestBodyRow during allEntries build pass instead of re-wrapping every cell in a second traversal (eliminates O(cells) of redundant stripInlineMarkup+stringWidth) * fix(tui): pass cols to recursive fenced-markdown Md, fix test frame extraction - Thread cols into <Md> for fenced markdown blocks (L734) so nested tables use the width-aware renderer instead of max-content path - Fix renderAtWidth helpers to extract final Ink repaint frame instead of concatenating all intermediate frames (REPAINT_RE split) - Add fenced-markdown-table fixture to tableFuzz (exercises the nested path) * chore: remove repro test suites and tmux driver script These were scaffolding for development/reproduction — not needed in the PR.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds responsive markdown table layout support via an optional ChangesResponsive Markdown Table Layout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
Add an official, production-grade WhatsApp integration via Meta's Business Cloud API as a complement to the existing Baileys bridge. No bridge subprocess, no QR codes, no account-ban risk — at the cost of a Meta Business account and a public HTTPS webhook URL. Setup is fully wizard-driven: 'hermes whatsapp-cloud' walks through every credential with paste-time validation (catches the #1 trap of pasting a phone number into the Phone Number ID field), generates a verify token, and ends with copy-paste instructions for the cloudflared / Meta-dashboard / Business Manager pieces that can't be automated. The wizard also points users at Meta's Business Manager for setting the bot's display name and profile picture. Feature set: - Inbound: text, images (with native-vision routing), voice notes (STT), documents (small text inlined, larger cached), reply context. - Outbound: text with WhatsApp-flavored markdown conversion, images, videos, documents, opus voice notes via ffmpeg with MP3 fallback. - Native interactive buttons for clarify, dangerous-command approval, and slash-command confirmation flows — matches the Telegram / Discord UX, graceful degrades to plain text. - Read receipts (blue double-checkmarks) and typing indicator, using Meta's combined endpoint so they fire in a single API call. - Webhook security: X-Hub-Signature-256 HMAC verification (raw body, constant-time), wamid deduplication, group-shaped-message refusal (groups deferred to v2 — Baileys still covers them). - Full integration with the gateway's session, cron, display-tier, prompt-hint, and auth-allowlist systems. Cloud and Baileys can run side-by-side against different phone numbers. Also wires STT (speech-to-text) through Nous's managed audio gateway for Nous subscribers — previously the default stt.provider=local required a separate faster-whisper install. New subscribers now get voice-note transcription out of the box. Docs: 418-line user guide at website/docs/user-guide/messaging/ whatsapp-cloud.md, sidebar entry, environment-variables reference, ADDING_A_PLATFORM.md updated with the optional interactive-UX contract for future adapter authors. Tests: 100 dedicated tests for the adapter, 32 for the setup wizard, 20 for the Nous subscription STT wiring, plus regression coverage across display_config, prompt_builder, and the cron scheduler. Known limitations (deferred until clear demand signal): - Group chats — use the Baileys bridge if you need them. - Message templates for 24-hour-window outside-conversation sends — reactive chat is unaffected; cron / delegate_task with gaps > 24h will fail with a clear error. The agent's system prompt warns the model about this so it knows to mention it when scheduling delayed messages.
… fail on '(empty)' sentinel Two related bugs caused subagent delegation to silently return empty summaries with 0 tokens when the user configured delegation.provider=bedrock alongside delegation.base_url=https://bedrock-runtime.<region>.amazonaws.com. Root cause #1 — misrouting in _resolve_delegation_credentials(): The configured_base_url branch unconditionally forced provider='custom' and api_mode='chat_completions', only specializing for chatgpt.com, anthropic, and kimi hosts. Bedrock (and other native-SDK providers) fell through as 'custom' + chat_completions, which then POSTed OpenAI-shaped JSON at Bedrock's native API. Bedrock rejected the payload and returned nothing, which looked like an empty LLM response to the child agent. Fix: when provider is one of {bedrock, vertex, google, google-genai}, skip the base_url short-circuit and fall through to resolve_runtime_provider(), which knows how to construct the proper SDK client. base_url can still be forwarded through that path for regional overrides. Root cause #2 — '(empty)' sentinel accepted as success: After N retries of empty LLM responses, run_agent.py emits the literal string '(empty)' as final_response. _run_single_child then hit `elif summary:` — '(empty)' is truthy, so status became 'completed' and the parent surfaced a blank result with no error. Users saw api_calls=4, tokens=0, duration~0.4s, status=completed. Fix: treat final_response.strip() == '(empty)' as a failure so the parent surfaces it instead of silently accepting zero-content 'success'. Both paths were reproduced in a live Hermes TUI session on us-west-2 Bedrock (provider=bedrock, model=us.anthropic.claude-sonnet-4-6) and are covered by new tests in tests/tools/test_delegate.py.
… (NousResearch#26195)
refactor(tui): thread cols through Md/StreamingMd/renderTable, update cache key
feat(tui): three-tier width calc + full-line string rendering in renderTable
Replaces the old renderTable (L203-244) with:
No wrapping or vertical fallback yet — those come in Phase 3 and 4.
Adds:
Wires:
These were scaffolding for development/reproduction — not needed in the PR.
What does this PR do?
Related Issue
Fixes #
Type of Change
Changes Made
How to Test
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
hermes --toolsets skills -q "Use the X skill to do Y"Screenshots / Logs
Summary by CodeRabbit