-
Notifications
You must be signed in to change notification settings - Fork 1
markdown rendering in TTY #46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
emesal
wants to merge
32
commits into
main
Choose a base branch
from
feature/markdown-rendering
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
+3,804
−85
Conversation
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
using streamdown-rs (https://github.com/fed-stew/streamdown-rs, MIT license) activates when **both** conditions are met: a) stdout is a TTY b) --json-output is *not* set note: each tool-call round creates a fresh markdown parser, since each round is a separate LLM response stream. this means that parser state (being inside a code fence etc) does *not* carry between rounds. remaining work before merge: - add --raw CLI option to force passthrough/no markdown rendering - add config.toml/local.toml option to enable/disable markdown renderer - ensure adequate tests are implemented - documentation updates - fmt clippy etc the deets: new src/markdown.rs: MarkdownStream struct with - TTY detection via std::io::IsTerminal, streamdown pipeline for terminals, passthrough for piped output - calling chibi with --json-output forces passthrough - line buffering in write_chunk() accumulates chunks and processes complete lines w Parser::parse_line() -> Renderer::render_event() - finish() flushes remaining partial line and calls Parser::finalie() to close open blocks - StdoutWriter newtype so the Renderer can persist while stdout is locked per-write src/main.rs: - added mod markdown, declaration src/api/mod.rs: four changes - replaced use tokio::io::{AsyncWriteExt, stdout} with use crate::markdown::MarkdownStream - let ´mut stdout = stdout() replaced with `let mut md = MarkdownStream::new()` inside the loop so that parser state resets between tool-call rounds. - replaced both stdout.write_all(...).await?; std.flush().await? sites with md.write_chink(...)? - added md.finish()? call after the streaming loop (guarded by !json_mode)
precedence chain is --raw (CLI) -> 'render_markdown' in local.toml -> config.toml -> default is *true*
docs/cli-reference.md: add --raw docs/configuration: added Output Rendering section + render_markdown = true, documenting TTY behaviour and --raw CLAUDE.md: more bloat
only when stdout is TTY it works, but lots of polish still needed
markdown expects to begin at column 0 or else things look bad
support for rendering images that exist on disk: load them, resize to terminal width and render with truecolor ANSI. validates the full pipeline without network complexity (which will be a later headache) not in this phase: - no url fetching - no data uri support - no caching - no ASCII fallback (placeholder on failure)
cargo audit has three warnings for unmaintained dependencies
replacing the previous stub. HTTPS-only by default with configurable size limit (10 MB), timeout (5s), Content-Type validation, and redirect downgrade protection. three new config fields control behavior: - image_max_download_bytes - image_fetch_timeout_seconds - image_allow_http details: Phase 3: Remote URL Image Fetching is implemented across 6 files: - src/config.rs - added 3 new config fields to Config, LocalConfig, and ResolvedConfig: - image_max_download_bytes (default: 10 MB) - image_fetch_timeout_seconds (default: 5s) - image_allow_http (default: false) - added get_field() and list_fields() support for inspection - src/state/mod.rs - threaded new fields through resolve_config() initial construction and local override arms - updated all test Config/LocalConfig constructions - src/markdown.rs - expanded MarkdownConfig with the 3 fetch-related fields - added internal ImageFetchConfig struct for grouping fetch parameters - expanded MarkdownStream to carry fetch config, threaded through render_events() - fetch_remote_image(): rejects HTTP when disallowed, uses tokio::task::block_in_place + Handle::current().block_on() for sync→async bridging - fetch_image_bytes(): async function with custom redirect policy (blocks HTTPS→HTTP downgrades, max 5 hops), Content-Type validation, Content-Length pre-check, streaming body with chunk-by-chunk size enforcement - try_render_image(): now dispatches http:///https:// URLs to fetch_remote_image() instead of returning an error - added 3 new tests: fetch_rejects_http_when_not_allowed, fetch_allows_http_when_configured, try_render_image_dispatches_https - src/api/mod.rs - passes new config fields into MarkdownConfig construction - src/main.rs - changed render_markdown_output to accept &MarkdownConfig instead of bool - added helpers: md_config_from_resolved() and md_config_defaults() - updated show_log() to accept &ResolvedConfig - updated all 4 call sites - docs/images.md - documented HTTPS/HTTP URL support, remote fetch safeguards, and the 3 new config settings also: - cargo fmt + clippy
also change behaviour to only display alt text as a fallback - src/config.rs — added 3 default functions, 3 fields to Config/LocalConfig/ResolvedConfig, wired into get_field()/list_fields(), updated test helper, added 3 get_field tests + extended 2 existing tests. - src/state/mod.rs — resolve_config() initializes and applies local overrides for the 3 new fields. updated all test Config/LocalConfig constructors. added 2 tests for override and default behavior. - src/markdown.rs — added 3 fields to MarkdownConfig. new ImageDisplayConfig struct, stored in MarkdownStream. try_render_image() now uses max_width_percent and max_height_lines for sizing, supports center/right/left alignment with per-row padding, and no longer prints alt text below rendered images. updated render_events() signature and all call sites. added default_display_config() test helper, updated existing test. - src/main.rs — md_config_from_resolved(), md_config_defaults(), and render_markdown_output() pass the 3 new fields. - src/api/mod.rs — MarkdownConfig construction includes the 3 new fields. - docs/images.md — added "Display options" section documenting the 3 settings. clarified alt text is fallback-only. fmt + clippy
add graceful degradation for image rendering, truecolor->ansi->ascii->alt text. images should now work across all terminal types, from modern truecolor terminals to text-only consoles. auto-detection checks COLORTERM and TERM environment variables to select the best available mode. can be overridden via explicit mode selection and specific modes disabled via configuration. configuration changes: - add image_render_mode: "auto" | "truecolor" | "ansi" | "ascii" | "placeholder" - add image_enable_truecolor, image_enable_ansi, image_enable_ascii flags - all fields support global and per-context overrides implementation: - add detect_terminal_capability() for COLORTERM/TERM parsing - add resolve_render_mode() for fallback chain logic - add render_ansi() using imgcatr's 16-color approximation - add render_ascii() with 8-character intensity mapping - refactor render_truecolor() to share alignment logic - update MarkdownStream to pass render_mode through pipeline docs: - docs/images.md: add rendering modes and compatibility sections - docs/configuration.md: new image rendering options - also env vars detection behavior fmt + clippy
- one unified cache in CHIBI_HOME - configurable parameters in config.toml/local.toml are image_cache_enabled (default true) image_cache_max_bytes (default 100M) image_cache_max_age_days (default 30)
this moves the code box labels from an empty row just under the top border, to instead be printed on top of it
see docs/markdown-themes.md for details
MarkdownStyle -> RenderStyle type alias - config.rs: replaced the MarkdownStyle struct + Default impl + to_render_style() method with pub type MarkdownStyle = streamdown_render::RenderStyle and a standalone default_markdown_style() function - markdown.rs: changed config.markdown_style.to_render_style() -> config.markdown_style.clone() - main.rs: changed MarkdownStyle::default() i-> default_markdown_style() MarkdownConfig deduplication - markdown.rs: added #[derive(Clone)] and MarkdownConfig::from_resolved() constructor - main.rs: render_markdown_output now takes owned MarkdownConfig; md_config_from_resolved delegates to from_resolved() - api/mod.rs: replaced 17-line manual MarkdownConfig construction with one from_resolved() call multi-value --debug - input.rs: Flags::debug changed from Option<DebugKey> to Vec<DebugKey>; added DebugKey::parse_list() for comma splitting - cli.rs: uses parse_list() instead of from_str(); debug_implies_no_chibi uses .iter().any() - api/request.rs: PromptOptions.debug changed from Option<&DebugKey> to &[DebugKey] - api/logging.rs: log_to_jsonl, log_request_if_enabled, log_response_meta_if_enabled accept &[DebugKey] - main.rs: all debug key extraction uses .iter().find_map() / .iter().any() force_render in streaming path - api/request.rs: added force_render: bool to PromptOptions - main.rs: passes force_markdown into PromptOptions::new() - api/mod.rs: uses options.force_render instead of hardcoded false docs - docs/markdown-themes.md: "invalid hex colors will fall back to defaults" -> "may produce unexpected rendering results" - docs/cli-reference.md: updated --debug to show comma syntax, added "Combining Debug Keys" section
- extracted ImageConfig struct to collapse resolve_config() from ~50 override lines into a single merge_with() call. md_config_defaults() now uses ImageConfig::default() instead of hardcoded values. - added ImageAlignment and ConfigImageRenderMode enus with serde rename_all = "lowercase". invalid values are now caught at toml parse time - removed extraneous flush() in StdoutWriter::write() - fix ASCII renderer padding mismatch - removed unused _alt parameter from try_render_image and all call sites - fix block_in_place stalling by wrapping fetch_image_bytes in tokio::task_spawn so the fetch runs on a separate worker thread - updated docs to reflect changes to [image] toml section format - fmt + clippy
now that --debug force-markdown,md= is possible, this workaround is no longer needed
- add missing tests - cache metadata race condition - fix misleading comment in block_in_place
d82baa0 to
55f03ef
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
using streamdown-rs (https://github.com/fed-stew/streamdown-rs, MIT license)
activates when both conditions are met:
a) stdout is a TTY
b) --json-output is not set
note: each tool-call round creates a fresh markdown parser, since each round is a separate LLM response stream. this means that parser state (being inside a code fence etc) does not carry between rounds.
remaining work before merge:
the deets:
new src/markdown.rs: MarkdownStream struct with
src/main.rs:
src/api/mod.rs: four changes
let mut md = MarkdownStream::new()inside the loop so that parser state resets between tool-call rounds.