Render ANSI escape codes instead of printing them as text - #108
Open
dchaudhari7177 wants to merge 1 commit into
Open
Render ANSI escape codes instead of printing them as text#108dchaudhari7177 wants to merge 1 commit into
dchaudhari7177 wants to merge 1 commit into
Conversation
Terminal output reaches the transcript with its SGR sequences intact -- /context is the usual source -- and it was escaped as literal text, so the page showed "\x1b[1mContext Usage\x1b[22m ...". The output most worth reading was the least readable thing on it. render_ansi_text() escapes the text first and then converts SGR sequences into spans: bold, dim, italic, underline, strikethrough, the 16 named colours, the 256-entry xterm palette, and truecolor. Only markup this function builds itself is trusted, and it is built from parsed integers, so HTML in the output is still escaped. Non-SGR sequences (cursor moves, erase-line) carry nothing to show and are dropped rather than displayed. Applied to the four tool_result paths that previously escaped straight into a <pre>. Text with no escape sequences takes an early return and renders byte for byte as before -- all 18 snapshots are unchanged. Closes simonw#95
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
1 participant
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.
Closes #95.
Before / after
The reporter's
/contextoutput arrives in the JSONL with its SGR sequences intact and was escaped as literal text:Now:
Colour is kept rather than stripped, because for
/contextspecifically the colour is the information — the shaded blocks are the usage bar.What's supported
render_ansi_text()handles bold, dim, italic, underline, strikethrough, their matching off-codes (22 turns off both bold and dim), the 16 named colours in both normal and bright form, foreground and background, the 256-entry xterm palette (38;5;N), and truecolor (38;2;R;G;B— the form/contextemits).Non-SGR sequences — cursor movement, erase-line, and the two-character
ESCforms — are dropped, not rendered. They carry nothing to show, and leaving them would just be a different kind of noise.On safety
The text is HTML-escaped first, always. The only markup trusted is what this function generates, and that is built from integers parsed out of the parameter list — a colour can only ever become
#rrggbb.test_html_in_ansi_output_is_still_escapedpins that:\x1b[1m<script>alert(1)</script>still comes out as<script>.This matters here because it is adjacent to #104 and my #107, which are about exactly this class of problem.
Scope
Applied to the four
tool_resultpaths that previously escaped straight into a<pre>— including the two either side of a commit card, so output split by a commit card is handled consistently.Text containing no
\x1btakes an early return and is escaped exactly as before, so nothing that was not already broken changes. All 18 snapshots pass unmodified, which is the evidence for that.Tests
Seven tests in
TestAnsiEscapeCodes, written first and watched fail (5 of 7 failed before the change; the two that passed are the escaping and no-op controls, which is what they are for). They cover the literal-code regression from the issue, bold becoming markup rather than being discarded, truecolor, 256-colour, non-SGR removal, the escaping guarantee, and a plain-text control.uv run pytest: 129 passed, 18 snapshots passed.uv run black .clean.Also checked by hand against the exact byte sequence from the issue.
AI disclosure
Written with Claude Code (Claude Opus 5), following AGENTS.md: tests first, watched fail, then the implementation, then Black. I ran the suite and checked the rendered output against the issue's sample myself.