Skip to content

feat: Rich-based rendering engine with intra-line diff highlighting (PR1) - #4470

Open
KUSH42 wants to merge 13 commits into
NousResearch:mainfrom
KUSH42:feat/rich-diff-renderer
Open

feat: Rich-based rendering engine with intra-line diff highlighting (PR1)#4470
KUSH42 wants to merge 13 commits into
NousResearch:mainfrom
KUSH42:feat/rich-diff-renderer

Conversation

@KUSH42

@KUSH42 KUSH42 commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Introduces agent/rich_output.py — a self-contained Rich/Pygments rendering toolkit with no project-specific imports — and upgrades the write-action diff previews added in #4423 with line numbers, per-file summary headers, and intra-line character-level highlighting.

Relationship to #4423: #4423 landed _render_inline_unified_diff, _summarize_rendered_diff_sections, and render_edit_diff_with_delta in display.py — the full write-action diff preview pipeline with a plain ANSI renderer. This PR plugs a Rich-based DiffRenderer into that pipeline: _render_inline_unified_diff now tries the Rich path first and falls back to #4423's ANSI output on any failure. No changes to the preview trigger logic or the tool_start/complete_callback plumbing.

Current diff vs new diff (includes changes introduced with PR2-5)
Image

Standalone
Image
Image
Image
Image

Changes

agent/rich_output.py (new module)

Core rendering toolkit — consumed here and by the follow-up PRs:

  • LanguageDetector — extension map + content-pattern heuristics
  • SyntaxHighlighter — Pygments → Rich markup → ANSI string
  • FilePathFormatter — per-filetype icons, compact relative paths
  • DiffRenderer — unified diff → Rich Text objects with line numbers
  • _intra_diff — character-level segment lists via SequenceMatcher (exposed for testing)
  • _parse_diff_filename — strips a/ / b/ prefixes, handles /dev/null (exposed for testing)
  • clean_command_output — strips venv/stacktrace noise from command output

agent/display.py (modifications)

  • Lazy-imports DiffRenderer and SyntaxHighlighter from rich_output at module load; sets _RICH_OUTPUT = True/False so the rest of display.py degrades gracefully if rich/pygments are absent
  • _render_inline_unified_diff — enhanced: tries DiffRenderer.to_lines() first, falls back to the feat: add inline diff previews for write actions #4423 ANSI path on any exception
  • highlight_code() — new public function wrapping SyntaxHighlighter.to_ansi(); used by the follow-up syntax highlighting PR

What DiffRenderer adds over the #4423 ANSI path

  • Line numbers — dim right-justified line numbers on every added/deleted line
  • Per-file summary header● filename.py Added N lines, removed M lines above each file section
  • Intra-line character-level diff — changed chars highlighted bright-red/green bold, unchanged chars on the base diff background; similarity threshold 0.5 — below it flat highlighting is used instead
  • Per-run del/add pairing — consecutive -/+ runs zipped for intra-line comparison, avoiding cross-hunk false pairings
  • Console width — passed from shutil.get_terminal_size; truncation delegated to the existing _summarize_rendered_diff_sections wrapper (max_lines=0 in the Rich path)

Tests

51 passing in tests/test_rich_output.py. tests/test_display.py assertions updated for the new header format.

@KUSH42
KUSH42 force-pushed the feat/rich-diff-renderer branch from 2aff5d2 to 80dab43 Compare April 1, 2026 17:52
@KUSH42
KUSH42 marked this pull request as draft April 1, 2026 18:08
@KUSH42
KUSH42 marked this pull request as ready for review April 1, 2026 18:14
@KUSH42
KUSH42 force-pushed the feat/rich-diff-renderer branch from 00e8cf7 to 4f42b5b Compare April 1, 2026 21:49
@KUSH42 KUSH42 changed the title feat: Rich-based rendering engine with intra-line diff highlighting feat: Rich-based rendering engine with intra-line diff highlighting (PR1) Apr 3, 2026
KUSH42 added 5 commits April 3, 2026 21:51
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.
KUSH42 added 5 commits April 5, 2026 02:47
_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.
@KUSH42
KUSH42 force-pushed the feat/rich-diff-renderer branch from 35722d2 to ea4b477 Compare April 7, 2026 23:13
KUSH42 added 3 commits April 8, 2026 01:24
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.
@KUSH42
KUSH42 force-pushed the feat/rich-diff-renderer branch from 8e7b587 to 1b2708e Compare April 8, 2026 02:30
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels May 1, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed renderer and focused unit coverage. The line-numbered/intra-line diff idea is still absent from current main, but a salvage needs adjustment for current surfaces.

Problems

  • agent/display.py:451 emits ANSI lines from DiffRenderer, but current ui-tui/src/app/createGatewayEventHandler.ts:740 applies stripAnsi to inline_diff. The Rich backgrounds and intra-line emphasis therefore disappear in the TUI, which consumes the same edit-preview callback path.
  • The PR also changes unrelated security-sensitive redaction at agent/redact.py:135, plus file-read, reasoning, and spinner behavior. Current main has later redaction work (fdb9620ac, c1c179a23, a57306654) that a renderer salvage must retain.

Suggested changes

  • Carry structured diff styling to the TUI, or render the diff natively there, and add a tool.complete.inline_diff integration test.
  • Split the unrelated changes before salvaging the renderer.

Automated hermes-sweeper review.

Comment thread agent/display.py
"""
if _RICH_OUTPUT:
try:
return _rich_diff.to_lines(diff, max_lines=0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns ANSI-only output. Current ui-tui/src/app/createGatewayEventHandler.ts:740 strips ANSI from inline_diff, so the proposed backgrounds and intra-line emphasis do not reach the TUI. Please carry structured styling through the event or add a TUI-native renderer and integration coverage.

Comment thread agent/redact.py
return text
if not _REDACT_ENABLED:
return text
# Fast path for large plain text blobs with no secret-like markers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This security-sensitive redaction optimization is unrelated to the Rich diff feature. Please split it from this renderer PR; current main has later redaction fixes that should be preserved independently during salvage.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants