feat: skin-aware code themes, /markdown toggle, fast-path & fallback (on top of KUSH42's renderer) - #5084
Open
lucaspirola wants to merge 42 commits into
Open
feat: skin-aware code themes, /markdown toggle, fast-path & fallback (on top of KUSH42's renderer)#5084lucaspirola wants to merge 42 commits into
lucaspirola wants to merge 42 commits into
Conversation
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.
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.
…responses Adds apply_block_line (headings, hr, blockquotes, lists, ref-link suppression) and extends apply_inline_markdown (images, links, HTML inline tags). format_response gains a pass-2 that applies both renderers to every non-highlighted line. The streaming path chains the same pair in _emit_stream_text and _flush_stream. Bug fixes included: - _code_highlight_active was gating apply_block_line/apply_inline_markdown in the streaming path; removed the guard (_RICH_RESPONSE is the correct gate; _code_highlight_active controls only tool-output highlighting) - _MD_ITALIC_UNDER_RE rejected phrases with spaces; changed [^_\s\n]+ to [^_\n]+ (word-boundary lookbehind prevents snake_case false positives) - _MD_REF_LINK_RE had a $ anchor that blocked titled reference-link definitions from being suppressed; removed $ - Blockquote inline spans reset to terminal default; added reset_suffix to the apply_inline_markdown call in the blockquote branch - format_response splitlines(keepends=True) fed \n into capture groups, silently dropping block elements; switched to splitlines() + manual join - CommonMark backslash escapes (\] → ]) were passed through literally; added step-7 re.sub pass in apply_inline_markdown - Pygments plain-text lexer emits lines with no ANSI codes; pass-2 guard "\x1b" in l then fell through and applied markdown to code-fence content (### was stripped, ** rendered, etc.); _highlight now prepends \x1b[0m to bare lines so the guard is always satisfied
…e correctness - Anchor code-fence regex to line start ((?m)^) so fences prefixed with > are not incorrectly consumed as code block openers; previously caused blockquote lines to be syntax-highlighted and the \x1b guard to skip apply_block_line, rendering raw > instead of the ▌ gutter - Remove fence delimiter preservation from format_response; add \033[0m fallback for plain-text lexer output so pass 2 always skips code content - Update _highlight group numbers to match new 3-group regex (backticks, lang, code) - Keep URL visible in link rendering ([text](url) → underlined text (url)) so users can copy and ctrl+click
Adds apply_block_line() for single-line-detectable block elements — headings (h1-h6 with tiered ANSI styles), horizontal rules, blockquotes with ▌ gutter, unordered list bullets by indent depth, and reference link suppression. Extends apply_inline_markdown with image placeholders, link underlining, and <em>/<strong> HTML tags. format_response and the streaming path now chain apply_block_line before apply_inline_markdown.
- format_response dropped trailing \n from rendered block elements (heading/hr/blockquote/list) because splitlines(keepends=True) fed newline-bearing lines into apply_block_line whose capture groups stop at \n — switched to splitlines() + manual join with newline restoration - Blockquote branch called apply_inline_markdown without reset_suffix so inline spans (bold, code) inside a blockquote reset to terminal default instead of restoring the dim gutter style — pass reset_suffix=_BLOCKQUOTE_ANSI - _MD_REF_LINK_RE had a trailing $ anchor that prevented suppression of reference-link definitions with optional titles ([ref]: url "Title") — removed $ - Remove dead _MD_OL_RE (compiled but never used) - Add 5 new tests covering the three bug fixes
…e correctness - Anchor code-fence regex to line start ((?m)^) so fences prefixed with > are not incorrectly consumed as code block openers; previously caused blockquote lines to be syntax-highlighted and the \x1b guard to skip apply_block_line, rendering raw > instead of the ▌ gutter - Remove fence delimiter preservation from format_response; add \033[0m fallback for plain-text lexer output so pass 2 always skips code content - Update _highlight group numbers to match new 3-group regex (backticks, lang, code) - Keep URL visible in link rendering ([text](url) → underlined text (url)) so users can copy and ctrl+click
…is accurate _render_inline_unified_diff was calling to_lines() with the hardcoded width=220 default, so Rich rendered each diff line as a single entry regardless of terminal width. The 80-line budget in _summarize_rendered_diff_sections counted these entries, not visual rows — on a 100-col terminal a 200-char diff line counted as 1 but visually wrapped to 2 rows, making truncation appear absent. Fix: resolve actual terminal width via shutil.get_terminal_size() and pass it to to_lines() so Rich wraps at the real column boundary and the budget correctly caps visual output.
_MD_ITALIC_UNDER_RE used [^_\s\n]+ which excluded spaces, so multi-word spans like _super bold and italic_ never matched and the underscores were emitted literally. Changed to [^_\n]+ to allow spaces while still preventing cross-line and nested-underscore matches.
…d style restoration Add <u> (underline, \033[4m) and <mark> (reverse-video highlight, \033[7m) to apply_inline_markdown. These are processed as step 0 — before markdown spans — via a recursive apply_inline_markdown call on the tag content with the wrapper style as reset_suffix. This ensures inner bold/italic resets restore the outer underline/highlight rather than dropping it: <u>**bold** normal</u> → \033[4m\033[1mbold\033[0m\033[4m normal\033[0m → "bold" renders bold+underline, "normal" renders underline Without the recursive reset_suffix, the \033[0m after "bold" would clear underline and "normal" would render plain.
Add missing markdown and HTML inline spans: Markdown: - ***text*** / ___text___ → bold+italic (\033[1;3m), processed before bold/italic individually so *** is consumed cleanly HTML tags: - <i> → italic (alias for <em>) - <b> → bold (alias for <strong>) - <s>, <strike>, <del> → strikethrough - <code>, <kbd> → inline code style - <ins> → underline (recursive reset_suffix like <u>) - <sup>, <sub> → tags stripped, content preserved
_italic_ and __bold__ inside **bold** failed because the ANSI code \033[1m ends in 'm' (a \w char), which caused the (?<![_\w]) lookbehind on the underscore italic/bold regex to reject the match. Fix: steps 2–5 in apply_inline_markdown now use _span(), which recursively calls apply_inline_markdown on the captured inner content before wrapping it in the outer span's ANSI. The recursive call processes the inner text without any surrounding ANSI context, so the lookbehind fires correctly. The ANSI guard (if '\x1b' not in inner) prevents double-processing HTML wrapper spans already handled in step 0.
… spans `StreamingCodeBlockHighlighter.process_line` was calling `_highlight_inline_code` for prose lines, which returns a new string object (not the same reference as `line`). In cli.py the `if out is line:` identity check then treated the result as an already-highlighted code block and emitted it as-is, skipping `_apply_inline_md(_apply_block_line(line))` entirely. Fix: return the original `line` from `process_line` for prose so the `is` sentinel works as intended and the full markdown pipeline runs in cli.py. Also unify code-span styling: `_protect_code` in `apply_inline_markdown` now uses `_ANSI_INLINE_CODE_START` (dark background + bright white, backticks preserved) to match `_highlight_inline_code`, replacing the previous plain bright-white-only `_MD_CODE_ANSI` style.
…combos Add tests for the case that triggered the bold/italic rendering bug: any inline style (bold, italic, bold-italic, strikethrough, underline, mark, ins) alongside a backtick code span on the same line. New tests across three layers: - TestApplyInlineMarkdown: unit-level combos — strikethrough+code, underline+code, bold-italic+code, mark+code, ins+code, multiple code spans with bold, all four inline styles on one line - TestApplyBlockLine: blockquote+code, blockquote+bold+code, heading+code, heading+bold+code - TestStreamingCodeBlockHighlighter: streaming pipeline bold-italic+code; also tighten test_plain_line_with_inline_code_styled to assert identity is preserved (the sentinel that cli.py relies on) Also fix stale comment in test_inline_code_not_applied_inside_fenced_block that said 48;5;237 was never used by apply_inline_markdown (it is now).
…king blocks When apply_inline_markdown processes a line, each code span ends with \033[0m which resets ALL styles — including the \033[2m (dim) prefix applied to every reasoning line. Text following a code span would then render in normal white instead of dim gray. Fix: pass reset_suffix=_DIM to apply_inline_markdown for all three reasoning render sites so every span's closing reset restores dim: - _stream_reasoning_delta: complete lines and force-flushed partials - _close_reasoning_box: remaining buffer flush - non-streaming post-turn reasoning display (show_reasoning=True path) Guarded by _RICH_RESPONSE so the plain-text path is unchanged.
Adds rendering for the three markdown elements that require multi-line state: setext headings, multi-line blockquote continuation, and tables. Also adds line numbers to fenced code blocks. - render_stateful_blocks(): string-level pass 2 in format_response; single left-to-right scan handling setext h1/h2, blockquote lazy continuation with ▌ gutter, and pipe table buffering/rendering - StreamingBlockBuffer: state machine inserted before StreamingCodeBlockHighlighter in the streaming pipeline; same four- priority rules with _emit_next slot for mode-transition buffering - _number_code_lines(): dim right-justified line numbers prepended to every highlighted fenced code block (both batch and streaming paths) - Blockquote + code: ``` fence in streaming blockquote mode exits the blockquote so StreamingCodeBlockHighlighter can highlight it normally - ANSI lines inside a blockquote keep the ▌ gutter instead of exiting - format_response is now a three-pass pipeline (fences → stateful blocks → per-line block/inline) - cli.py: StreamingBlockBuffer threaded into streaming loop and flush
… markdown _render_table computed column widths using len() on raw cell text (e.g. "**bold**" = 8), then format_response pass 3 applied apply_inline_markdown to every table line, replacing "**bold**" with \033[1mbold\033[0m (4 visual chars). The padding was calculated for 8 but the visual content was 4, shifting subsequent columns right. Fix: apply apply_inline_markdown to each cell inside _render_table, measure widths via _visual_len() (ANSI-stripped len), and pad with the visual remainder. The resulting ANSI-containing rows are skipped by pass 3 (\x1b guard), preventing any double-application. Add _ANSI_ESC_RE and _visual_len() helpers. Add alignment regression test.
_visual_len used len() which counts Unicode code points, not terminal columns. Wide characters (east_asian_width W/F — e.g. ✅ ❌ 🚀) are 1 code point but occupy 2 terminal columns, causing all subsequent columns to shift left by 1 for each emoji in the row. Fix: iterate over the stripped string with unicodedata.east_asian_width, counting W/F chars as 2. Also handle U+FE0F (emoji presentation selector): it is 0-width itself but upgrades a preceding neutral char (e.g. ⚠) to 2-wide, matching modern terminal emulator behaviour.
Two bugs:
1. _is_heading_candidate returned True for ordered-list items ("1. text")
because apply_block_line passes them through unchanged (same object).
Add _MD_OL_START_RE guard so "N. ..." lines are never treated as
setext heading candidates, even when followed by a "---" separator.
Without this, OL items followed by a horizontal rule (which also
matches the setext H2 pattern) were promoted to dim-white headings.
2. buf_tail lines flushed at stream-end (StreamingBlockBuffer.flush())
went directly to _cprint without apply_block_line / apply_inline_md
or the _tc gold-colour wrapper. Apply the same processing as the
normal streaming path so links, bold, italic, and response colour are
all rendered correctly for lines that were still buffered at flush time.
Adds two tests covering the OL-after-HR setext false-positive.
Replace plain underline (\033[4m) on links with bright-blue underline (\033[4;94m) so link text is visually distinct from the surrounding gold response text. Adds _MD_LINK_ANSI constant for the style.
\033[4;94m (ansibrightblue) renders as default color under patch_stdout. Switch to \033[38;2;88;166;255m\033[4m (#58A6FF, GitHub dark-mode blue) which follows the same truecolor path as the gold response text (_tc) and renders correctly in the streaming pipeline.
_MD_BARE_URL_RE now matches: - https?:// (existing) - file:// (e.g. file:///home/user/tmp) - ftps?:// (ftp/ftps) - www. (bare domain, negative lookbehind prevents mid-word match) All four protocol families receive the same bright-blue underline style as markdown [text](url) links.
… spans _MD_BARE_URL_RE's character class [^\s<>\[\]()\"] did not exclude \x1b (ESC). When a URL appeared inside a bold or italic span such as **https://x.com**, the _span callback recursively called apply_inline_markdown on the inner text, styling the URL with {link_ansi}url\033[0m{reset}. The outer apply_inline_markdown then ran step 6b2 again on the full line (now containing those ANSI codes). The regex matched url\x1b (capturing the ESC byte), leaving the bare [0m as orphaned literal text — rendered visibly as [0m[0m in the terminal. Add \x1b to both character-class exclusions so the regex stops at ESC bytes. Add regression test asserting no orphaned [0m in plain text.
Deletion lines stored ln_old (old-file counter) while context and addition lines used ln_new (new-file counter). When a hunk starts with ln_old > ln_new (e.g. @@ -59,16 +58,8 @@), each context line advances both counters equally, so ln_old stays ahead. After N context lines the first deletion displayed a line number N higher than ln_new — producing the jarring "60, 62, +61" and "53, 52, 53" patterns seen when deletions and their paired additions disagreed on line numbers. Store deletion line numbers as ln_new + len(del_run) so that deletions, additions, and context lines all stay on the same new-file scale. Paired del/add lines now share the same line number. ln_old continues to advance correctly for context-line accounting.
GFM allows leading and trailing pipes to be omitted on table rows. Previously only strict rows (| A | B |) were recognised; loose rows (A | B | C) were emitted as plain text. Three changes: * _split_row: strip leading/trailing | before splitting so both formats parse correctly. * render_stateful_blocks / StreamingBlockBuffer (Priority 3): once a separator row has been seen (_sep_idx set), accept any pipe-bearing line as a data row — not just strict | … | rows. * render_stateful_blocks / StreamingBlockBuffer (Priority 4): when a strict table row or a loose separator arrives and the pending line already contains pipes, rescue the pending line as the loose table header instead of flushing it as prose. Covers the mixed case (loose header/data + strict separator) and the fully-loose case (no boundary pipes on any row).
When a line contained both an inline image and a markdown link, step 6a (image rendering) emitted \033[0m mid-string. Step 6b's link regex _MD_LINK_RE then matched the bare '[0m' as the opening bracket of a link, consuming it along with the subsequent link text and leaving an orphaned ESC byte (\x1b) before the substituted link ANSI colour code. The orphaned \x1b caused the terminal to misinterpret the following CSI sequence, printing the raw ANSI bytes ([38;2;88;166;255m0m …) as visible text instead of applying colour. Fix: add (?<!\x1b) negative lookbehind to _MD_LINK_RE so a '[' that is immediately preceded by an ESC byte (i.e. is part of an ANSI CSI sequence) is never treated as the start of a link pattern.
…ext-in-blockquote, ref link resolution Five missing markdown features added to the terminal renderer: * Ordered lists — digits + '.' or ')' delimiter, dim numeral styling, continuation line handling in stateful paths, indent depth by 2-space scale matching UL * Task lists — '[ ]' renders as dim ○, '[x]'/'[X]' renders as bold green ✓; detected inside UL match branch, no new state required * Nested blockquotes — _bq_depth: int replaces _in_blockquote: bool throughout render_stateful_blocks and StreamingBlockBuffer; each depth level adds 2-space indent and one extra dim layer (capped at 3) * Setext headings inside blockquotes — stateful scan detects '==='/'---' inner content following a blockquote pending line, styles as h1/h2, re-wraps in gutter * Link reference definitions — pre-pass collects [label]: url defs into ref_map; apply_inline_markdown resolves [text][ref] and [text][] before the inline link step; StreamingBlockBuffer accumulates ref_map as defs arrive; batch path (format_response) does a full pre-pass 253 tests passing.
…kdown features Bug fix: _REF_DEF_RE only matched double-quoted titles; parenthesized (Title) and single-quoted 'Title' variants were silently dropped from ref_map, leaving [text][ref] unresolved. Extended the optional title group to cover all three CommonMark title forms. 49 new edge-case tests across ordered lists, task lists, nested blockquotes, setext-in-blockquote, and ref link resolution — covering delimiter variants, blank-line interaction, mixed list types, streaming path correctness, and depth/reset behaviour. 302 tests passing.
Two table rendering bugs fixed: 1. Strict tables (pipes at both ends of every row) now render with a full box frame using box-drawing characters (┌┬┐│├┼┤└┴┘─). A row separator is drawn between every pair of content rows. 2. Loose table separators without pipes (e.g. "--- --- ---") were silently dropped because detection required "|" in the separator line. Now uses _TABLE_SEP_RE (^[\s:\-|]+$) + "-" presence check, so pipe-free separators are handled correctly. Implementation: - Rename _TABLE_ROW_RE → _TABLE_STRICT_ROW_RE; add _TABLE_LOOSE_ROW_RE and _TABLE_SEP_RE regexes per spec - _render_table: add framed: bool param; framed path draws box chars with ANSI-aware column widths and inter-row dividers - Track _table_strict in render_stateful_blocks and StreamingBlockBuffer; set from first accumulated row (header); pass framed=_table_strict to _render_table on flush - Update loose separator detection in both state machines to use _TABLE_SEP_RE instead of "|" in line guard - Update tests: rename import, fix test_table_no_separator expectation (strict tables always have box chars now)
…tore outer style after inline spans List items (UL, OL, task lists) call apply_inline_markdown internally without reset_suffix. The returned string already contains \x1b, so the outer apply_inline_markdown call (which carries reset_suffix=_DIM) hits the early-exit and never restores the outer style. Fix: add reset_suffix parameter to apply_block_line and forward it to every inner apply_inline_markdown call — including the checkbox symbols in task lists and the dim numeral in ordered lists. Update all seven call sites in cli.py to pass reset_suffix to apply_block_line. Reproducer: any UL/OL/task-list line with a backtick span inside a reasoning/think block loses dim formatting for text after the code span.
… del block Commit 58ba8fe fixed non-monotonic del/add numbers on offset hunks (@@ -59 +58 @@) by storing ln_new+offset for deletions instead of ln_old. This broke when a context line appears between two deletion runs: both the preceding deletion and the context line end up with the same line number, and subsequent deletions pick up a shifted new-file scale that no longer matches their old-file positions. Fix: store ln_old in del_run at append time. At flush time, paired deletions (those with a matching addition) take the addition's new-file line number so paired del/add lines always show the same number. Unpaired deletions fall back to their saved ln_old, keeping numbers correct and monotonic even when context lines interrupt a deletion block.
…graceful fallback Add five improvements on top of the rich_output markdown rendering engine: - Skin-aware code themes: SyntaxHighlighter loads Pygments theme from active skin's `code_theme` key, updates on /skin change - /markdown [on|off] command (alias /md): toggle Rich Markdown rendering with config persistence via display.markdown - Fast-path detection: skip markdown parser for plain-text responses using regex check on first 500 chars - Graceful fallback: try/except wrappers in both streaming and non-streaming render paths so renderer crashes never hide response content - Platform hint: CLI now tells the LLM to use markdown freely since the terminal supports full rendering Addresses: NousResearch#3621, NousResearch#4236, NousResearch#684, NousResearch#4518 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
8 tasks
lucaspirola
changed the title
lucaspirola
changed the title
Contributor
|
I did a cleanup pass on the original stacked renderer branches and found a few fixes that were present later in that stack but had never been backported properly. I pushed a focused delta branch here for cherry-pick rather than asking you to reconcile the older stack:
That delta covers:
If helpful, the intended action is just: |
This was referenced Apr 22, 2026
19 tasks
3 tasks
alt-glitch
added
type/feature
Contributor
1 similar comment
Contributor
6 tasks
teknium1
reviewed
Jul 12, 2026
teknium1
left a comment
teknium1
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the substantial renderer work. Current main independently renders final CLI responses through Rich Markdown in cli.py:2467-2502 and has a display.final_response_markdown mode in hermes_cli/config.py:1806; it does not currently provide this PR's /markdown command or code_theme skin integration.
Problems
agent/rich_output.py:1826checks onlytext[:500]before deciding whether to render Markdown.cli.py:6433-6437uses that result as the render gate, so Markdown following a long plain-text preamble is emitted raw. This needs a regression test and a correctness-preserving gate.
Suggested changes
- Salvage the missing toggle/theme behavior onto the existing
cli.py:_render_final_assistant_contentpath rather than introducing a parallel 2,008-line renderer. - Document the user-facing setting and command.
Automated hermes-sweeper review.
| fast-path gate before ``format_response()`` so plain-text responses skip | ||
| the full parsing pipeline. | ||
| """ | ||
| sample = text[:500] if len(text) > 500 else text |
Collaborator
There was a problem hiding this comment.
This gate misses valid Markdown after a 500-character plain-text preamble, and the CLI uses it to bypass rendering entirely. Please scan enough input to preserve correctness (or remove the gate) and add a regression test with a late code fence or emphasis span.
teknium1
added
sweeper:risk-compatibility
19 tasks
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds full Rich Markdown rendering for CLI responses using Rich's built-in `Markdown` class, with skin integration and user control:
Files changed (3 files)
Issues addressed
Closes #3621, closes #4236, relates to #684
Test plan
🤖 Generated with Claude Code