feat(theme): full theme integration — wire all colors/styles to SkinConfig (PR5) - #4582
feat(theme): full theme integration — wire all colors/styles to SkinConfig (PR5)#4582KUSH42 wants to merge 87 commits into
Conversation
bf71ce5 to
1eba630
Compare
35b1cbf to
da793cb
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.
_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.
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
The PR5 rebase of the 'Fix CLI ANSI auth and reasoning rendering' commit reverted the earlier fix that removed verbose from the callback gate. Reinstate: _current_reasoning_callback returns non-None only when show_reasoning is True, never when only verbose is set.
Each built-in skin now declares a preferred TUI spinner style via spinner.style in its definition: default → dots (classic braille) ares → arrows (directional combat feel) mono → none (no animation — minimal) slate → pulse (quarter-circle pulse) poseidon → bounce (wave-like bounce) sisyphus → grow (block-grow grind) charizard → star (star burst) SkinConfig.get_spinner_style() returns the key or None (falls back to display.spinner_style config). CLI init prefers skin style over config. spinner_loop and _get_tui_prompt_fragments updated to animate during both _command_running and _agent_running. Documents spinner.style in the skin YAML schema comment.
1a1c7a5 to
b4c6063
Compare
1ff487d to
6a28d97
Compare
…gate tool previews on verbose mode _flush_stream: both _stream_block_buf.flush() and _stream_code_hl.flush() were gated inside `if self._stream_buf:`, so an API error hitting right after a newline boundary (empty buffer) silently dropped any buffered block state (pending setext headings, partial tables, open code fences). Move both flush calls outside the guard so they always run when _RICH_RESPONSE is active. Also append _RST after _stream_code_hl.flush() output to ensure dangling ANSI color sequences from the syntax highlighter are always terminated. _on_tool_complete: code previews (render_read_file_preview, render_execute_code_preview, render_terminal_preview) were shown in any non-off mode. Gate them on tool_progress_mode == "verbose" since they are full raw output, not summaries. Edit diffs are unaffected and still render in new/all/verbose modes. Tests: 19 tests covering empty-buffer flush (the bug), code-hl RST, normal-path regressions (non-empty buffer + box border), and all verbose gating branches.
_emit_highlighted_lines now prepends " " to every line so tool output previews (read_file, terminal, execute_code) align visually under the " ┊ header" label above them. Streaming code block lines (StreamingCodeBlockHighlighter output in _emit_stream_text and _flush_stream) get the same 2-space prefix so inline code blocks in streamed responses match the tool preview indent. The _RST after a flushed tail is now a separate _cprint call that follows the per-line loop rather than being appended to the last line.
_handle_skin_command called set_active_skin and _apply_tui_skin_style but never updated _COMMAND_SPINNER_FRAMES, so the spinner kept the previous skin's style until restart. Apply the same skin→config→dots fallback resolution that __init__ uses.
…alette All tokens now use explicit hex values rather than terminal ANSI names, ensuring consistent rendering across terminal color schemes. Adds a 'name' token (warm off-white #E8E2D5) so plain identifiers have stable contrast instead of inheriting the terminal default. Differentiates string_escape/string_doc from string literals, decorators from functions, and aligns diff_deleted/diff_inserted hues with the skin's diff bg colors.
…ay config streaming: true is the better out-of-box experience for interactive use. spinner_style added to DEFAULT_CONFIG as empty string (defers to skin default) so it is discoverable via config show and scaffolded on init.
…cription example-skin.yaml was missing the spinner.style key added in feat(skin). Also corrects the hermes scheme description from "bold blues/greens/yellows" to "warm amber/gold truecolor" to match the new hex palette.
…eming Add missing display config keys introduced by the rich rendering pipeline (PR1–PR5) that had no entries in cli-config.yaml.example: code_highlight, syntax_bold, diff_max_lines, diff_max_files, preview_max_lines, title_spinner, title_base, spinner_style Also extend the inline skin schema comment to document the new skin sections: spinner.style, syntax_scheme, syntax_overrides, diff, markdown, and ui_ext.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the substantial theming work. The rich-rendering and syntax-scheme direction is not present on current main, but this branch needs focused salvage before it is safe to integrate.
Problems
agent/redact.py:143-148returns large inputs before_PREFIX_RE. The same PR declaresretaindb_,mem0_, andbrv_prefixes at lines 53-56, but none appears in the fast-marker lists at lines 105-115; a >8 KiB blob containing one leaks unchanged. Please remove this unrelated optimization or make its guard complete and tested.hermes_cli/main.py:1089restoressimple_term_menu. Current main removed these pickers in087be007because ESC/arrow handling and rendering were unreliable; retaincurses_radiolistinstead.hermes_cli/config.py:377flipsdisplay.streamingtotrue, while current main keeps the compatibility defaultfalseathermes_cli/config.py:1803. This is unrelated to themes.
Suggested changes
- Split/drop the unrelated redaction, menu, and streaming-default edits; rebase the theme-specific work onto current renderer and skin surfaces.
- Preserve the current curses picker path and existing display defaults.
Automated hermes-sweeper review.
| if not any(marker in text for marker in _FAST_MARKERS_CASE_SENSITIVE) and not any( | ||
| marker in lower_text for marker in _FAST_MARKERS_LOWER | ||
| ): | ||
| return text |
There was a problem hiding this comment.
This early return bypasses _PREFIX_RE for large input. The same branch recognizes retaindb_, mem0_, and brv_ prefixes, but none is in either fast-marker list, so a >8 KiB blob containing one of them returns unredacted. Please remove this optimization from the theme PR or derive and test a complete guard.
| print() | ||
| return idx | ||
| except Exception: | ||
| from simple_term_menu import TerminalMenu |
There was a problem hiding this comment.
Do not reintroduce simple_term_menu. Current main commit 087be007 deliberately migrated these pickers to curses_radiolist after confirming unreliable ESC/arrow behavior and ghost rendering; theme styling needs to use the curses path.
| "bell_on_complete": False, | ||
| "show_reasoning": False, | ||
| "streaming": False, | ||
| "streaming": True, |
There was a problem hiding this comment.
This unrelated default flip changes classic CLI behavior. Current main keeps display.streaming: false; please preserve that default and limit this PR to the theme/rendering scope.
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two open PRs address the rendering/theme complex. #4582 introduces the shared skin-driven syntax, diff, Markdown, and CLI rendering pipeline, while #6736 carries essentially that same foundation plus reasoning/thinking rendering and streaming changes.
Related pull requests
- #4582
related— (+7927/-99) — salvage as the consolidation base, not merge as-is: the diff implements the theme/rendering foundation, but the contributor review identifies unrelated and blocking regressions in large-input secret redaction, provider-menu selection, and the default streaming mode; those changes must be dropped or corrected during a rebase onto current main. - #6736
related— (+9590/-142) — extract the reasoning-specific delta into #4582 rather than merge this historical superset: beyond largely duplicating #4582, it adds rich reasoning paths, but its streaming description conflicts with its own line-buffering tests and it targets obsolete renderer locations. Despite the keep_open review on #6736, closing it after extraction is justified by the diff's broad duplication and the contributor's explicit recommendation for a targeted current-main port rather than direct salvage.
Duplicates
#6736 substantially duplicates and extends #4582 across the theme engine, rich-output module, CLI rendering, configuration, documentation, and tests; its distinct material is primarily the reasoning/thinking integration and associated streaming changes.
Suggested consolidation
Do not merge either PR as-is. Keep #4582 as the consolidation target, rebase and narrow it to theme-specific work while addressing its contributor review, then port only the current-main-compatible reasoning changes from #6736 after choosing and consistently documenting the intended streaming behavior; #6736 can then be closed as superseded by the consolidated #4582.
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 825 kB of PR diffs, 13 kB of issue/PR text, 4 kB of discussion (5 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Summary
Every hardcoded color and style in
rich_output.py,display.py,skills_hub.py,plugins_cmd.py, andmain.pyis now driven by the activeSkinConfig. Adds 10 named syntax color schemes.1 — SkinConfig foundation (
skin_engine.py)SYNTAX_SCHEMES: hermes, monokai, dracula, one-dark, github-dark, nord, catppuccin, tokyo-night, gruvbox, solarized-dark — each with 18–21 token entries including mandatorydiff_deleted/diff_insertedSkinConfigwithsyntax_scheme,syntax,diff,markdown,ui_extfields backed by_DIFF_DEFAULTS,_MARKDOWN_DEFAULTS,_UI_EXT_DEFAULTSget_syntax_styles(),get_diff(),get_markdown(),get_ui_ext()methodsregister_skin_callback/_invalidation_callbacks_build_skin_config()validates: unknown scheme → "hermes" + warning; hex colors checked with regex;menu_cursor/menu_highlightstring → list coercionsyntax_scheme: default→hermes, ares→gruvbox, mono→solarized-dark, slate→one-dark, poseidon→nord, sisyphus→hermes, charizard→monokai2 — Syntax highlighting wired to skin (
rich_output.py)_PygmentsToRichto per-instance__init__(styles: dict)— no more class-level_STYLES_get_logical_to_pygments()/_build_pygments_map(styles)to map logical token names to Pygments token objectsSyntaxHighlighternow builds its formatter via_build_fmt()readingget_active_skin().get_syntax_styles()SyntaxHighlighter.refresh()called by skin-switch callback registered indisplay.py3 — Markdown and diff rendering wired to skin
_MD_ANSI_CACHE/_MD_VAL_CACHE(None sentinels),_md_ansi(key),_md_val(key),_rebuild_md_cache()— cache built once per skin switch via invalidation callback_rich_style_to_ansi(style_str)— converts Rich style strings to ANSI escape sequences_MD_LINK_ANSI,_MD_CODE_ANSI, heading dicts, blockquote constants, bullets list with cache lookups_diff_cfg(key)lazy accessor; replaced all_DIFF_BG_ADD/_DIFF_BG_DELthroughout diff renderer_hex_to_ansi_fg/bg()helpers and_d(key)accessor indisplay.py; replaced_ANSI_DIM/FILE/HUNK/MINUS/PLUSconstants with functions4 — Context bar, tables, menus wired to skin
display.py:_ctx_color(pct)readscontext_bar_normal/warn/critfromskin.get_ui_ext();format_context_pressure()uses itskills_hub.py:_col_accent(),_col_dim(),_panel_border()helpers readingui_ext; all generic table columns and panels updatedplugins_cmd.py:cmd_list()readstable_col_accent/table_col_dimfrom skin at call timemain.py:_pt_style(key, fallback)helper; provider/model/reasoning menus use it instead of hardcoded("fg_green", "bold")Fixes, tests, and tooling
omitted_filescount off-by-one insummarize_rendered_diff— was+= 1 + max(0, ...), now+= max(0, ...)-/+sigils now share the same background colour as the diff content (previously had no background set)@@ ... @@) indented to align with line-number column; blank line inserted between file sectionstests/test_theme_integration.py— 47 integration smoke tests; parametrized across all 10 schemes; covers syntax, markdown cache, diff colors, hex helpers, context bar tiers,_pt_style,skills_hubhelpers, and skin validationtests/test_rich_output.py::TestMonokaiIntraDiff— 15 tests verifying monokai syntax colours survive_flat_del/_flat_add/_intra_diff/DiffRenderer.to_lines()end-to-end, including skin-switch resets coloursscripts/demo_themes.py—python scripts/demo_themes.py [skin]renders syntax, markdown, Rich diff, inline diff, and context bar for every builtin skindocs/skins/example-skin.yamlupdated to usemonokaias samplesyntax_schemeTest plan
pytest tests/test_theme_integration.py -v— all 47 tests passpytest tests/test_rich_output.py::TestMonokaiIntraDiff -v— all 15 tests passpython scripts/demo_themes.py— visual cycle through all 7 skins, no rendering errors