feat: syntax highlighting for tool outputs and LLM responses (PR2) - #4471
feat: syntax highlighting for tool outputs and LLM responses (PR2)#4471KUSH42 wants to merge 20 commits into
Conversation
465b334 to
d9ecc84
Compare
d3045a2 to
47ce516
Compare
Introduces agent/rich_output.py — a self-contained Rich/Pygments rendering toolkit with no project-specific imports. Public API: - LanguageDetector: extension map + content-pattern heuristics - SyntaxHighlighter: Pygments → Rich markup → ANSI string - FilePathFormatter: per-filetype icons, compact relative paths - DiffRenderer: unified diff → Rich Text with line numbers - clean_command_output: strip venv/stacktrace noise from command output DiffRenderer replaces _render_inline_unified_diff in display.py: - Intra-line character-level highlighting via SequenceMatcher (threshold 0.5) - Per-run del/add pairing to avoid cross-hunk false matches - Summary header: ● filename.py Added N lines, removed M lines - Console width from shutil.get_terminal_size, not hardcoded Tests: 51 passing in tests/test_rich_output.py
Pygments emits Error tokens for content its markdown lexer cannot tokenize (emoji in headings, unknown syntax, etc.). Mapping Error to "bold red on red" matched the diff-deletion colour, causing spurious red backgrounds on unrelated text. Changed to "bold red" (text colour only), consistent with Generic.Error.
Adds _DIFF_MAX_LINES = 80 and a max_lines parameter to to_lines(). Outputs beyond the cap get a dim footer matching _highlight_block's style. Passes max_lines=0 in _render_inline_unified_diff so the section-level budget in _summarize_rendered_diff_sections is unaffected.
Apply Pygments token colours to diff line content so that keywords, strings, comments, numbers etc. are visually distinct within the red/green diff bands — matching the style shown in tools like delta. Changes: * Two new colour constants — _DIFF_BG_ADD_HL / _DIFF_BG_DEL_HL — a noticeably brighter shade of the base diff background, used to mark changed character ranges in intra-line diffs without touching the foreground (no conflict with syntax token colours). * _syntax_text(content, filename) — new helper that calls SyntaxHighlighter.to_markup(), converts to a Rich Text (foreground colours only), and strips the trailing newline Pygments always appends so line lengths stay accurate. * _flat_add / _flat_del — now accept an optional filename hint and apply syntax highlighting via _syntax_text, then overlay the base diff background with Text.stylize(). * _intra_diff — redesigned: syntax-highlight both lines first, apply the base diff background across the whole text, then apply the brighter highlight background (bold) only over changed character ranges via a second stylize() pass. Foreground colours come entirely from syntax; background colours entirely from diff state. * _style() — passes filename (explicit_filename or from_path) through to all three helpers above, and syntax-highlights context lines (dim style overlaid on syntax colours). * Tests updated to match the new interface: _intra_diff now returns ([Text], [Text]) with spans rather than lists of single-span segments; changed-region detection checks for bright bg spans instead of bright foreground colour names.
Line numbers and the -/+ prefix had no background set, so they rendered on the terminal default (black) while the content immediately to their right had the dark-red/dark-green diff background — creating a jarring visual break. Extend bgcolor to the line-number and sigil Text objects so the entire deleted/added row is uniformly on the diff background, matching the visual style of delta, GitHub, and VS Code's diff view.
0824037 to
c73585a
Compare
kshitijk4poor
left a comment
There was a problem hiding this comment.
Really like the renderer work here — the core implementation is strong, the heuristics are thoughtful, and the targeted tests look good. I ran tests/test_display.py and tests/test_rich_output.py locally and they passed.
That said, I don’t think this is ready to merge yet as a user-facing config feature.
A few things still need tightening before merge:
- If this is intended to be config-driven, not command-driven, we shouldn’t leave partial
/code-highlighthandling incli.py. That path is incomplete anyway, and I don’t think it should exist at all for the intended UX here. display.code_highlightshould be wired as a real first-class display config option, not just read with a fallback. If we’re exposing this as config, it should be present in defaults, loaded consistently, and covered by tests for default/true/false behavior.- The config semantics are currently inconsistent:
display.code_highlightgates some preview surfaces, but assistant response code highlighting still happens independently in both streaming and final response rendering. As written,code_highlightdoes not actually mean “code highlighting on/off”, which makes the UX ambiguous. - There are also a few implementation-level correctness issues called out inline below that should be fixed before merge.
The renderer itself looks good. This is mostly about finishing the product surface and closing the remaining correctness gaps so the feature behaves consistently.
| # Match fenced code blocks of any depth (3+ backticks); \1 backreference | ||
| # ensures the closing fence uses the same backtick sequence as the opener. | ||
| # Apply inline-code highlighting only to prose segments between/around blocks. | ||
| fence_re = re.compile(r"(`{3,})(\w*)\n(.*?)\1", re.DOTALL) |
There was a problem hiding this comment.
Broaden fenced-code language regexes beyond \w
Markdown info strings commonly include characters like +, -, or # (for example c++, objective-c, shell-session, f#). With (\w*) here, those fences don’t match correctly in format_response(), and the same restriction in StreamingCodeBlockHighlighter._FENCE_OPEN_RE means streamed responses never enter code-block mode either.
In those cases users still see raw triple-backtick fences instead of the new highlighted rendering.
There was a problem hiding this comment.
Fixed on this branch. The fenced-block matchers now accept common Markdown info-string punctuation instead of \\w*, so cases like c++, objective-c, shell-session, and f# enter the highlighting path in both batch and streaming rendering. Added regression coverage.
|
|
||
| # Check the leading verb (strip path prefix, e.g. /usr/bin/cat → cat) | ||
| from pathlib import Path as _Path | ||
| verb = _Path(tokens[0]).name |
There was a problem hiding this comment.
Require a read verb before syntax-highlighting terminal output
This helper says it only fires for known file-reader verbs, but it never actually checks _FILE_READ_COMMANDS. Right now anything that isn’t an explicit executor can still get treated as source if a later token looks like a filename.
That means commands like git diff app.py, pytest tests/test_app.py, or ruff check foo.py can have stdout rendered as though it were source code even though it’s diff or diagnostic output. The preview becomes misleading for many normal terminal commands that merely mention a filename.
There was a problem hiding this comment.
Fixed on this branch. Terminal source-highlighting now returns early unless the leading verb is in the explicit file-read set, so commands like git diff app.py, pytest tests/test_app.py, and ruff check foo.py no longer get treated as source previews. Added regression coverage.
| """ | ||
| _print = print_fn or print | ||
| _print(f"\033[2m ┊ {header}\033[0m") | ||
| if not _RICH_OUTPUT: |
There was a problem hiding this comment.
Cap highlighted tool previews before printing every line
_highlight_block() currently emits every highlighted line. render_read_file_preview() and render_terminal_preview() both call this helper, so a single read_file on a long source file or a cat/sed terminal command can dump hundreds of lines into the interactive UI before the assistant responds.
This new preview path needs the same kind of truncation budget as the diff renderer to avoid flooding the session.
There was a problem hiding this comment.
Fixed on this branch. Highlighted tool previews now use a 40-line budget with an omission footer instead of dumping the full highlighted block into the interactive UI. Added regression coverage for both read-file and generic highlighted-block paths.
c73585a to
a4b8d10
Compare
_MAX_INLINE_DIFF_LINES (80), _MAX_INLINE_DIFF_FILES (6), and _PREVIEW_MAX_LINES (40) were hardcoded in agent/display.py with no user-facing knob. Wire them to config.yaml under display: diff_max_lines: 80 # lines shown per inline diff before "… omitted" summary diff_max_files: 6 # file sections shown per inline diff preview_max_lines: 40 # lines shown in read_file/execute_code/terminal previews Adds set_diff_limits() and set_preview_max_lines() setters in display.py; cli.py reads and applies all three at init alongside code_highlight.
Two bugs in _current_reasoning_callback(): 1. show_reasoning=True + streaming=False returned None — non-streaming mode got no live reasoning callback even when user enabled it (post-turn box still worked, but no intermediate display during tool-call loops) 2. verbose=True + show_reasoning=False returned _on_reasoning — verbose mode leaked reasoning into the display regardless of the user's explicit setting Fix: show_reasoning is the sole gate. When on, pick callback by streaming mode (stream_reasoning_delta vs on_reasoning). Verbose no longer overrides.
…y's fixes - test_reasoning_command.py: install prompt_toolkit/fire stubs at module level (conditional on missing) so all 58 tests can import from cli without patching individually; fixes 38 pre-existing failures. Correct two wrong assertions in TestReasoningDisplayModeSelection that reflected old buggy behaviour (show_reasoning=True+non-streaming returned None; verbose leaked callback). Add test_show_reasoning_off_returns_none and test_verbose_without_show_reasoning_returns_none. - test_display.py: import set_diff_limits/set_preview_max_lines; add TestDisplayLimitSetters covering global mutation and truncation behaviour.
spinner_loop only fast-refreshed (0.1 s) during _command_running, leaving _agent_running on the 1 s idle cadence — so the status-bar prompt showed a static ⚕ and tool-progress updates in the spinner widget lagged by up to a second. Add _agent_running to the fast-refresh branch alongside _command_running, and replace the static ⚕ in _get_tui_prompt_fragments with _command_spinner_frame() so the braille dots animate while the agent is working, matching the slash-command experience.
Spinner style - Add _SPINNER_STYLES dict (dots, bounce, grow, arrows, star, moon, pulse, clock, none) so users can pick via display.spinner_style in config.yaml; default remains "dots" (braille). - At HermesCLI init, read spinner_style and override the module-level _COMMAND_SPINNER_FRAMES global so _command_spinner_frame() picks the right sequence for both the status-bar prompt and the tab title. Terminal tab / window title - Add display.title_spinner (default true) and display.title_base (default "Hermes") config keys. - spinner_loop emits OSC 0 (\033]0;…\007) via app.output.write_raw() at each 0.1 s tick while active, giving a live "⠋ Hermes" → "⠙ Hermes" animation in the terminal tab; resets to the bare base string on idle. Errors are silenced so terminals that don't support OSC 0 aren't affected.
…hlight active - Import LanguageDetector into display.py and instantiate as _rich_detector; render_read_file_preview and _extract_file_language_from_command were calling _rich_detector.detect_from_filename but the name was never bound (SyntaxHighlighter has no detect_from_filename method). - Suppress execute_code first-line snippet in get_cute_tool_message when _code_highlight_active is True — the highlighted block renders immediately after, so printing the snippet again is redundant duplication. - Add missing json, _highlight_block, _result_succeeded, get_cute_tool_message imports to test_display.py so the test suite can actually run.
70ad38e to
add8dbe
Compare
Wires the SyntaxHighlighter/LanguageDetector from rich_output.py into tool result display and LLM response rendering. execute_code preview: - Highlighted Python block printed after successful execution - Gated on _result_succeeded — nothing shown after a failed run - Cute-msg drops the inline snippet when highlight is active (no duplication) read_file preview: - ┊ 📄 filename.py header + syntax-highlighted content - Language from extension only; unknown types skipped silently terminal preview: - Verb-based language detection from the command - _FILE_EXEC_COMMANDS blocklist (node, python3, bash, …) prevents runtime stdout from being mistaken for source code LLM response rendering: - format_response() highlights fenced code blocks in complete responses - StreamingCodeBlockHighlighter state machine for streaming: buffers fenced blocks, flushes highlighted on closing fence, plain text passes through immediately with response text colour preserved Plumbing: - Verbosity gate: all previews suppressed when tool_progress_mode == "off" - display.code_highlight config key + /code-highlight toggle - set_code_highlight_active() keeps display.py decoupled from CLI state - Module-level _rich_detector singleton (no per-call instantiation) Tests: 135 passing (tests/test_display.py + tests/test_rich_output.py)
format_response wrapped highlighted code in ``` delimiters under the theory that "the Panel still looks like a code block". In practice the ANSI-highlighted block reads cleanly without them, and keeping the fences caused raw backtick lines to appear in the rendered response.
_number_code_lines() prepends dim right-justified line numbers with a │ separator to every line of a highlighted code block: 1 │ import requests 2 │ import time ... Called from both format_response._highlight (batch path) and StreamingCodeBlockHighlighter._flush_block (streaming path) so line numbers appear consistently in all LLM response code blocks.
…endering bugs _PygmentsToRich.format() previously escaped brackets by replacing: [ → \[ (correct: literal "[" in Rich markup) ] → \] (WRONG: Rich has no \] escape; renders as literal \]) This caused two bugs: 1. Haskell type signatures like [Integer] rendered as [Integer\] 2. A trailing \ token (e.g. Haskell lambda \a) before a close tag like [/bold yellow] formed the Rich escape \[ — consuming the closing tag and leaking the tag text into the output. Fix: use rich.markup.escape() which doubles \ → \\ and escapes [ → \[ while leaving ] alone (] requires no escaping in Rich markup). Apply to ALL tokens (styled and unstyled) and to all fallback paths. Also add _highlight_inline_code() and format_response() for inline code span styling in LLM response text.
…diff test assertions - to_lines: width=0 silenced Rich Console entirely; resolve to terminal width via shutil.get_terminal_size when no explicit width is given - tests: remove _DIFF_BG_ADD_HL/_DIFF_BG_DEL_HL imports removed in skin rewrite; update _intra_diff delete/insert tests to check across the full segment list rather than assuming a single-element list
add8dbe to
4bdbac2
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the renderer work. The feature is still relevant on current main, but this branch needs integration work before it can provide the advertised behavior.
Problems
agent/display.py:638,:658, and:749add preview helpers, but the final PR diff adds no production invocation. Current tool completion rendering is atcli.py:11078-11107; it only invokesrender_edit_diff_with_delta. Meanwhile PRcli.py:1180-1182enablescode_highlightand suppresses the existing execute-code snippet, so the default can hide that snippet without rendering the replacement preview.agent/rich_output.py:759and:791still use\w*for Markdown fence info strings. Valid identifiers such asc++,f#,objective-c, andshell-sessiondo not enter either highlighting path.
Suggested changes
- Integrate the previews at the actual completed-tool callback and cover those end-to-end display paths.
- Use a non-whitespace info-string parser in both complete and streaming renderers, with punctuation-language regressions.
- Adapt the renderer to the current TUI bridge (
tui_gateway/render.py:24-47), which expectsrender_diffandStreamingRendererrather than the names supplied here.
Automated hermes-sweeper review.
| return False | ||
|
|
||
|
|
||
| def render_execute_code_preview(code: str, print_fn=None) -> bool: |
There was a problem hiding this comment.
This preview helper is only defined and unit-tested in this PR; no production path calls it (likewise for the read-file and terminal helpers). The current completed-tool hook is cli.py:11078-11107, so display.code_highlight currently suppresses the existing execute-code snippet without emitting this replacement preview. Please wire the helpers through the real completion path and add an integration test.
| # Match fenced code blocks of any depth (3+ backticks); \1 backreference | ||
| # ensures the closing fence uses the same backtick sequence as the opener. | ||
| # Apply inline-code highlighting only to prose segments between/around blocks. | ||
| fence_re = re.compile(r"(`{3,})(\w*)\n(.*?)\1", re.DOTALL) |
There was a problem hiding this comment.
\w* still rejects common valid Markdown info strings such as c++, f#, objective-c, and shell-session; the streaming opener at line 791 has the same restriction. Use a non-whitespace info-string grammar in both places and add regressions for punctuation-bearing language names.
Wires the
SyntaxHighlighter/DiffRendererfrom the diff-renderer PR into tool result display and LLM response rendering.Changes
execute_codepreview_result_succeeded— no highlight shown after a failed runread_filepreview┊ 📄 filename.pyheader + syntax-highlighted contentterminalpreview_FILE_EXEC_COMMANDSblocklist (node,python3,bash,ruby, …) — runtime stdout is never mistaken for source codeLLM response rendering
format_response()for complete responses: replaces fenced code blocks with ANSI-highlighted versions_number_code_lines: dimi │prefix on every fenced code block line, right-justified line number +│separatorStreamingCodeBlockHighlighterstate machine for streaming: buffers lines inside fences, flushes the highlighted block on the closing fence, passes plain text through immediately with response text colour preservedPlumbing
tool_progress_mode == "off"display.code_highlight+/code-highlighttoggleset_code_highlight_active()indisplay.pykeeps it decoupled from CLI state╌╌ N more lines omitted ╌╌footerTests
185 passing (
tests/test_display.py+tests/test_rich_output.py).