feat(cli): streaming markdown rendering in terminal output - #1986
feat(cli): streaming markdown rendering in terminal output#1986unmodeled-tyler wants to merge 1 commit into
Conversation
9de0b37 to
fe42b0f
Compare
…tput Add a lightweight, skin-aware streaming markdown renderer that formats model responses in the terminal. Addresses the core rendering goal from issue NousResearch#504 and builds on the approach discussed in PR NousResearch#601. Streaming path: each completed line passes through a stateful StreamingMarkdownRenderer that produces simple ANSI SGR codes compatible with prompt_toolkit's ANSI() parser and patch_stdout pipeline. Non-streaming path: responses rendered via Rich Markdown through the existing ChatConsole adapter. Supported elements: - Headers (H1-H6), bold, italic, strikethrough, inline code, links - Fenced code blocks with language labels - Ordered and unordered lists (nested) - Tables with full-table buffering for column alignment - Blockquotes with nesting, including tables and code blocks inside - Horizontal rules, HTML stripping, backslash escapes Colors are pulled from the active skin engine (banner_accent for headers, ui_label for code, banner_dim for chrome) so rendering adapts to all built-in and user-defined skins. No new dependencies — uses only stdlib (re, shutil, unicodedata) plus the existing Rich library for the non-streaming fallback.
fe42b0f to
27e0928
Compare
|
Orb Code Review (powered by GLM-4.7 on Orb Cloud) SummaryThis PR implements a massive codebase cleanup for Hermes Agent while adding a custom streaming markdown renderer for CLI output. The changes include: removing 29,698 lines of code across 227 files, deleting numerous platform integrations (dingtalk, matrix, mattermost, etc.), extensive test file cleanup, and introduction of a new lightweight ANSI markdown renderer with streaming support. ArchitectureThe PR represents a major architectural simplification:
The markdown renderer supports streaming text arrival, proper ANSI escape sequences, and fallback to universal SGR codes when theme engine is unavailable. IssuesCritical1. hermes_cli/markdown_renderer.py:47-50 - HTML security filtering may be insufficient: _RE_HTML_DANGEROUS = re.compile(r"<\s*(script|style|iframe)\b[^>]*>.*?</\s*\1\s*>", re.IGNORECASE)
_RE_HTML_DANGEROUS_OPEN = re.compile(r"<\s*(script|style|iframe)\b[^>]*>", re.IGNORECASE)The regex-based HTML filtering has limitations:
Recommendation: Use proper HTML sanitization library: import bleach
def sanitize_html(text: str) -> str:
"""Remove dangerous HTML while preserving safe formatting."""
allowed_tags = ['b', 'i', 'u', 'strong', 'em', 'code', 'pre']
allowed_attrs = {}
return bleach.clean(text, tags=allowed_tags, attributes=allowed_attrs, strip=True)2. hermes_cli/markdown_renderer.py:85-86 - Character width calculation may be incorrect: elif (
0x2500 <= cp <= 0x257F # box drawing — always 1-wide
or 0x2580 <= cp <= 0x259F # block elements — always 1-wide
):
width +=1The box drawing and block element ranges may not be correctly classified. According to Unicode standards, these can be narrow or double-width depending on terminal implementation. 3. Massive code deletion without clear justification - The PR deletes 29,698 lines including:
This appears to be a major feature removal without migration path or deprecation notice. Warnings1. hermes_cli/markdown_renderer.py:28-35 - Hardcoded decorative width limit: # Maximum width for decorative elements (rules, fences) so they don't
# stretch absurdly on ultra-wide terminals.
_MAX_DECOR_WIDTH = 80The 80-character limit may cause issues on modern wide terminal displays. Consider making this configurable: import os
_MAX_DECOR_WIDTH = int(os.getenv('HERMES_MAX_WIDTH', '120'))2. hermes_cli/markdown_renderer.py:40-45 - Regex patterns may have edge cases: _RE_BOLD_ITALIC = re.compile(r"\*\*\*(.+?)\*\*\*")
_RE_BOLD = re.compile(r"\*\*(.+?)\*\*")
_RE_BOLD_UND = re.compile(r"__(.+?)__")These regex patterns don't handle:
3. Missing feature migration plan - No documentation about:
Suggestions1. Add comprehensive tests for markdown renderer:
2. Documentation improvements:
3. Performance optimization: # Consider caching width calculations for common characters
from functools import lru_cache
@lru_cache(maxsize=1024)
def _display_width(text: str) -> int:
# ... existing logic4. Better error handling: def render_line(line: str, theme: Optional[Dict[str, str]] = None) -> str:
"""Render a single line of markdown to ANSI codes.
Args:
line: Markdown text to render
theme: Optional theme dictionary for color mapping
Returns:
ANSI-formatted string suitable for terminal output
Raises:
ValueError: If input contains unsupported markdown constructs
"""
try:
# ... existing logic
except Exception as e:
return f"[RENDER ERROR: {e}] {line}"5. Consider existing markdown libraries: Before building a custom renderer, evaluate:
6. Backward compatibility: Provide fallback mode: # If custom renderer fails, fall back to basic formatting
def safe_render(line: str) -> str:
try:
return render_line(line, current_theme)
except Exception:
return basic_escape(line) # Simple escape sequences7. Configuration options: # hermes_cli/config.py
class MarkdownConfig:
enabled: bool = True
theme: str = "default"
max_width: int = 120
enable_emoji: bool = True
enable_tables: bool = True8. Security audit - The HTML filtering regex needs comprehensive review:
9. Deprecation process - For deleted features:
10. Unicode improvements: # Use wcwidth library for accurate character width calculation
# pip install wcwidth
import wcwidth
def _display_width(text: str) -> int:
return wcwidth.wcswidth(text)Cross-file Impact
AssessmentThis PR represents a massive architectural change that significantly simplifies the Hermes Agent codebase but removes substantial functionality without proper migration process. The new streaming markdown renderer is well-implemented but has security concerns that need addressing. Strengths:
Critical issues to address:
Implementation quality:
Missing elements:
Community impact:
The markdown renderer implementation shows technical competence, but the massive feature removal and security concerns suggest this needs more work before merge. The community should have input on which features are safe to remove, and a proper deprecation process should be followed. |
1 similar comment
|
Orb Code Review (powered by GLM-4.7 on Orb Cloud) SummaryThis PR implements a massive codebase cleanup for Hermes Agent while adding a custom streaming markdown renderer for CLI output. The changes include: removing 29,698 lines of code across 227 files, deleting numerous platform integrations (dingtalk, matrix, mattermost, etc.), extensive test file cleanup, and introduction of a new lightweight ANSI markdown renderer with streaming support. ArchitectureThe PR represents a major architectural simplification:
The markdown renderer supports streaming text arrival, proper ANSI escape sequences, and fallback to universal SGR codes when theme engine is unavailable. IssuesCritical1. hermes_cli/markdown_renderer.py:47-50 - HTML security filtering may be insufficient: _RE_HTML_DANGEROUS = re.compile(r"<\s*(script|style|iframe)\b[^>]*>.*?</\s*\1\s*>", re.IGNORECASE)
_RE_HTML_DANGEROUS_OPEN = re.compile(r"<\s*(script|style|iframe)\b[^>]*>", re.IGNORECASE)The regex-based HTML filtering has limitations:
Recommendation: Use proper HTML sanitization library: import bleach
def sanitize_html(text: str) -> str:
"""Remove dangerous HTML while preserving safe formatting."""
allowed_tags = ['b', 'i', 'u', 'strong', 'em', 'code', 'pre']
allowed_attrs = {}
return bleach.clean(text, tags=allowed_tags, attributes=allowed_attrs, strip=True)2. hermes_cli/markdown_renderer.py:85-86 - Character width calculation may be incorrect: elif (
0x2500 <= cp <= 0x257F # box drawing — always 1-wide
or 0x2580 <= cp <= 0x259F # block elements — always 1-wide
):
width +=1The box drawing and block element ranges may not be correctly classified. According to Unicode standards, these can be narrow or double-width depending on terminal implementation. 3. Massive code deletion without clear justification - The PR deletes 29,698 lines including:
This appears to be a major feature removal without migration path or deprecation notice. Warnings1. hermes_cli/markdown_renderer.py:28-35 - Hardcoded decorative width limit: # Maximum width for decorative elements (rules, fences) so they don't
# stretch absurdly on ultra-wide terminals.
_MAX_DECOR_WIDTH = 80The 80-character limit may cause issues on modern wide terminal displays. Consider making this configurable: import os
_MAX_DECOR_WIDTH = int(os.getenv('HERMES_MAX_WIDTH', '120'))2. hermes_cli/markdown_renderer.py:40-45 - Regex patterns may have edge cases: _RE_BOLD_ITALIC = re.compile(r"\*\*\*(.+?)\*\*\*")
_RE_BOLD = re.compile(r"\*\*(.+?)\*\*")
_RE_BOLD_UND = re.compile(r"__(.+?)__")These regex patterns don't handle:
3. Missing feature migration plan - No documentation about:
Suggestions1. Add comprehensive tests for markdown renderer:
2. Documentation improvements:
3. Performance optimization: # Consider caching width calculations for common characters
from functools import lru_cache
@lru_cache(maxsize=1024)
def _display_width(text: str) -> int:
# ... existing logic4. Better error handling: def render_line(line: str, theme: Optional[Dict[str, str]] = None) -> str:
"""Render a single line of markdown to ANSI codes.
Args:
line: Markdown text to render
theme: Optional theme dictionary for color mapping
Returns:
ANSI-formatted string suitable for terminal output
Raises:
ValueError: If input contains unsupported markdown constructs
"""
try:
# ... existing logic
except Exception as e:
return f"[RENDER ERROR: {e}] {line}"5. Consider existing markdown libraries: Before building a custom renderer, evaluate:
6. Backward compatibility: Provide fallback mode: # If custom renderer fails, fall back to basic formatting
def safe_render(line: str) -> str:
try:
return render_line(line, current_theme)
except Exception:
return basic_escape(line) # Simple escape sequences7. Configuration options: # hermes_cli/config.py
class MarkdownConfig:
enabled: bool = True
theme: str = "default"
max_width: int = 120
enable_emoji: bool = True
enable_tables: bool = True8. Security audit - The HTML filtering regex needs comprehensive review:
9. Deprecation process - For deleted features:
10. Unicode improvements: # Use wcwidth library for accurate character width calculation
# pip install wcwidth
import wcwidth
def _display_width(text: str) -> int:
return wcwidth.wcswidth(text)Cross-file Impact
AssessmentThis PR represents a massive architectural change that significantly simplifies the Hermes Agent codebase but removes substantial functionality without proper migration process. The new streaming markdown renderer is well-implemented but has security concerns that need addressing. Strengths:
Critical issues to address:
Implementation quality:
Missing elements:
Community impact:
The markdown renderer implementation shows technical competence, but the massive feature removal and security concerns suggest this needs more work before merge. The community should have input on which features are safe to remove, and a proper deprecation process should be followed. |
1 similar comment
|
Thanks for this @unmodeled-tyler — the streaming-flicker problem you're solving is real (raw markdown source while streaming, then a Rich Panel re-render at end-of-turn) and #684 still tracks it. Closing this one without merging, for two reasons:
Your edge-case work (East Asian width, nested blockquote/code/table state) is genuinely well done — if #12836 lands and gaps remain, those are worth porting on top. Appreciate the contribution. |
Summary
StreamingMarkdownRendererthat formats model responses in the terminal during streaming, addressing the core rendering goal from Feature: Enhanced CLI TUI — Rich Markdown Rendering, Diff Previews, Token Tracking & Navigable Output #504 / Feature: CLI Rich Rendering & Block Navigation — Streaming Markdown, Syntax Diffs & Output Indexing #684_cprint) but avoids thepatch_stdout/ complex ANSI compatibility issues by using simple SGR escape codes for the streaming pathMarkdownthrough the existingChatConsoleadapterWhat's rendered
Headers (H1–H6), bold, italic,
strikethrough,inline code, links, fenced code blocks with language labels, ordered/unordered lists (nested), tables with full-table buffering for column alignment, blockquotes with nesting (including tables and code blocks inside), horizontal rules, HTML tag stripping, and backslash escapes.Skin integration
Colors are pulled from the active skin engine at render time:
banner_accent→ headersui_label→ code / code spansbanner_dim→ fences, rules, blockquote barsTested across all built-in skins (default, ares, mono, slate, poseidon, sisyphus).
Architecture
_emit_stream_text()→StreamingMarkdownRenderer.render_line()→_cprint()— line-by-line, stateful (tracks code blocks and buffers tables), produces only basic SGR escape codes that pass cleanly through prompt_toolkit'sANSI()parserChatConsole().print(Panel(Markdown(response)))— Rich Markdown through the existing adapterre,shutil,unicodedata) plus the existingrichlibraryFiles changed
hermes_cli/markdown_renderer.pyStreamingMarkdownRendererclass (470 lines)tests/test_cli_markdown_renderer.pycli.pyTest plan
pytest tests/test_cli_markdown_renderer.py— 40/40 passingpytest tests/test_cli_init.py tests/test_cli_skin_integration.py— no regressionsstreaming: trueacross all built-in skinsstreaming: false(Rich Markdown Panel path)Screenshots