enhance(actions): replace `ansi_up` with first-party code by silverwind · Pull Request #38619 · go-gitea/gitea

silverwind · 2026-07-24T18:04:07Z

Replaces the ansi_up dependency with first-party code and fixes a number of bugs in turn.

Improved perf, around 20 times faster rendering for plain and 5 times faster for colored.
Render many SGR features like hyperlinks, blink, inverse, conceal, strikethrough, overline, underline styles and underline color.
A non-hyperlink OSC and the final byte of a two-byte escape no longer leak into the output as text.
An escape sequence never carries across a line boundary, so a truncated one is dropped instead of corrupting a later line.
Attributes render as CSS classes instead of inline styles.
Invisible text now is selectable.
Remove z-index workaround (uses color-mix).

Constructing an AnsiUp rebuilds its color palettes on every call, which dominates the cost of rendering a long action log. Lines containing no escape sequence render identically as a text node, so the parser and the HTML round-trip can be skipped for them. Measured over 100k plain lines: ~180ms -> ~4ms. Assisted-by: Claude Code:Opus 5

Assisted-by: Claude Code:Opus 5

The URL post-process condition appeared in both exit paths. Moving the ansi_up branch into a helper that returns the HTML leaves a single tail, so the check is written once. It now tests the source line rather than the rendered HTML, which is equivalent: escaping and span markup can neither introduce nor remove "://". Assisted-by: Claude Code:Opus 5

Copilot

Pull request overview

This PR optimizes the Actions log renderer by avoiding ansi_up processing for log lines that clearly contain no ANSI escapes (or carriage-return progress updates), significantly reducing render time for large volumes of plain-text output.

Changes:

Add a fast-path in renderAnsiInto to use textContent when no ANSI escape (\x1b) or carriage return (\r) is present.
Refactor ANSI conversion into a helper (renderAnsiToHtml) while preserving existing URL post-processing behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: silverwind <me@silverwind.io>

Signed-off-by: silverwind <me@silverwind.io>

silverwind · 2026-07-25T04:46:57Z

Should move ansi_up to first-party code eventually to optimize/improve it further. I do recall is has a few unresolved bugs/unhandled sequences (for example drudru/ansi_up#78 (comment)).

silverwind · 2026-07-25T04:53:39Z

Claude does not like f04e432, i'll try something. For reference:

_buffer persists across lines and corrupts later output.**

'abc\x1b[3'    -> abc            (buffer retains "\x1b[3")
'plain'        -> plain          (fast path, buffer untouched)
'\x1b[32mgreen'-> [3<span class="ansi-green-fg">green</span>"   ← literal "[3" leaked

The log viewer used about a sixth of ansi_up, and the parts it did not use got in the way. Its per-instance color palette setup dominated rendering, its OSC 8 hyperlinks duplicated the URL linkification that runs afterwards and were then skipped by it, and its buffer for incomplete escape sequences carried a half-written sequence across line boundaries, leaking the leftover into a later line. Keeping a single instance to carry color between lines also meant sharing hidden mutable state, which needed a cache keyed by step index and an adjacency check on line numbers to stay correct. Rendering is now a pure function of the line and the incoming AnsiStyle, covering SGR attributes, the 16 named colors, the 256-color palette and truecolor. Each log stream owns an AnsiLineRenderer holding just that style, so color still carries between a step's lines, while nothing is shared between steps, jobs or runs. Escape sequences are never carried across a line boundary, so a truncated one is dropped rather than corrupting later output. Validated against ansi_up over a 4000 input corpus, per line and across a stateful sequence, before dropping the dependency. /devtest/ansi-render renders every supported feature. Assisted-by: Claude Code:Opus 5

An unrelated js-yaml downgrade was picked up from a local install and committed alongside the ansi_up removal. Only the ansi_up entries belong in this branch. Assisted-by: Claude Code:Opus 5

Dropping them lost a capability: ansi_up turned an OSC 8 sequence into a real link, and the URL post-process preserves it by skipping anchors, so a runner emitting one used to get a clickable link with its own label. A web target becomes an anchor, any other scheme renders its label as plain text rather than discarding both. The label keeps the style in effect, and the anchor opens in a new tab, matching the links the URL post-process makes. Audited against ansi_up over a 5725 input corpus: 5697 render identically and every remaining difference is a fix. OSC 8 aside, ansi_up leaks a non-hyperlink OSC as visible text and leaves the final byte of a two-byte escape behind, both of which are now consumed. The devtest attribute rows follow the layout of the colors.py sample. Assisted-by: Claude Code:Opus 5

"4:0" turns the underline off, but reading the parameter as a plain integer turned it on, so an underline leaked into everything that followed. Split a parameter from its sub-parameters and let "4:0" disable it. 58 sets the underline color and takes the same arguments as 38 and 48. They were not consumed, so a spec like "58;2;135;0;255" was read as ordinary codes, and its "0" reset the whole style mid-line. Consume them, without rendering a color. Assisted-by: Claude Code:Opus 5

ansi_up ignored SGR 7, 8, 9 and 53, so text a terminal shows inverted, hidden, struck through or overlined rendered as if unstyled. Inverse swaps foreground and background, falling back to the console colors when either side is a terminal default. Their resets, 27 to 29 and 55, come with them. Underline styles and the underline color are rendered too, so "4:3" draws a curly underline rather than a straight one and 58 colors it. The style lookup is indexed by number, so a non-numeric sub-parameter cannot reach an inherited property. Hovering a log link no longer shifts its underline: the global "a:hover" sets text-underline-position to "under", which moved it. Adds an injection suite covering markup in text and in hyperlink labels, attempts to break out of the span and href attributes, and script, data and scheme-relative hyperlink targets, asserting a serialise and parse round trip produces no markup. Assisted-by: Claude Code:Opus 5

Every attribute the renderer applies is a css class now, leaving inline styles only for the colors a class cannot express: the 256-color cube and truecolor, which are arbitrary per line. Those became "#rrggbb" literals, so a palette entry is just a string, either a class name or a color, and the wrapper object around it is gone. Faint no longer uses opacity. It paints a translucent color on a nested span, where currentcolor resolves to whatever the outer span set, so the stacking context that made log lines render over the sticky step header is gone with it. That drops the z-index workaround from go-gitea#36749, and the z-index:0 pairing it relied on, which a flex item without a z-index already gets against a positive-z-index sibling. Conceal paints the text in the background color instead of hiding it, so it stays selectable, and blink animates the color for the same reason. Conceal sits after the color rules, since it has to win against a foreground set on the same element. Also from a cleanup pass over the branch: the cursor-move replacement was dead, the single-part and carriage-return render paths were the same loop, the underline style folded into the underline field, and capture groups are numbered because named ones cost an object per match. The devtest is three sections, and the dependency diff is only the ansi_up removal again after an unrelated js-yaml bump was picked up from a local install. Assisted-by: Claude Code:Opus 5

wxiaoguang · 2026-07-25T06:31:36Z

Claude does not like f04e432, i'll try something. For reference:

_buffer persists across lines and corrupts later output.**

Why you expect that you will get such output? It is ridiculous

silverwind · 2026-07-25T06:33:43Z

This is now a full rewrite to replace ansi_up, fixing a number of issues related to SGR code rendering and removing the previous z-index workaround. PR description is updated.

wxiaoguang · 2026-07-25T06:35:28Z

I don't see any real problem by using existing "ansi_up" package at the moment and don't want to spend time (no enough time) on reviewing more AI slop.

TBH, I don't trust your code from AI.

ref:

not needed, not interested, no time

silverwind · 2026-07-25T06:42:55Z

I don't see any real problem by using existing "ansi_up" package

OSC rendering are all real bugs, and the perf gain is nice to have.

Conceal lost the cascade to the inline color that the 256-color cube and truecolor emit, so concealed text stayed legible, and painting it in the console background assumed no other background was set under it. The renderer now emits no foreground at all when conceal is set and the class is transparent, which works over any background and depends on no ordering. SGR 5 and 25 render, reusing the animation the console renderer's .term-fg5 already uses. Three changes take colored lines from 206ms to 99ms per 100k: htmlEscape runs behind a test, since most log text has nothing to escape and it makes five passes; a bare reset returns the initial style without cloning or parsing, which is the most common sequence in colored output; and only the underline code reads a sub-parameter, so parameters are no longer split for it. Plain lines stay at 9ms. Also: the two color slots share one helper, the truncated tail is one indexOf, and a redundant early return in renderText is gone. Assisted-by: Claude Code:Opus 5

The class was a shell around a private function with one caller, which had to take and return the style just to hand it back. Merged, renderInto only mutates the element and the renderer's own style, so its name matches what it does. The codes that only set fields are a table rather than twenty else-if arms, the newline strip tests once instead of twice, and the single-use color and attribute helpers are inlined. The devtest is one continuous console rather than sectioned blocks, with the escaped source rendered as faint lines in the same stream. Assisted-by: Claude Code:Opus 5

silverwind · 2026-07-25T07:15:34Z

Cleaned this up considerably including PR description, ready for review.

…y type URLs were found by walking the parsed DOM after the fact, which meant a second pass and a second way to build an anchor, kept apart only by the walk skipping A nodes. Linkifying the text run as it is escaped removes both, and the OSC 8 and bare-url paths now share one anchor constructor. The initial style is readonly rather than frozen, which the compiler checks and costs nothing at runtime. applySgr hands the shared instance back for a reset, so the guarantee is worth keeping. renderPart is a method, so the style threads through the field instead of a return value, and rgbHex is one line. Tests follow the shape they had. Assisted-by: Claude Code:Opus 5

colord already ships in the bundle and is what the rest of the frontend uses for color, so the hand-rolled bit shift goes. It costs about 4ms per 100k truecolor lines and nothing on lines that use the named or 256-color palettes, which never reach it. The rest is size: the style type is laid out like the initial value below it, the linkify decision moved into the function that escapes so the nested ternary is gone, and the comments a name already carried are dropped. Assisted-by: Claude Code:Opus 5

Assembling it at runtime worked around "no-script-url", which does not apply here: the rule flags a literal that starts with the scheme, and this one is a whole escape sequence. Assisted-by: Claude Code:Opus 5

…ment Assisted-by: Claude Code:Opus 5

The decoration color repeated the themed lookup that applyColor already does for the foreground and background, on a line long enough to hide it. applyColor now takes the slot optionally: without one there is no class variant, so a themed color resolves through its variable inline. Assisted-by: Claude Code:Opus 5

silverwind · 2026-07-25T07:54:00Z

More reductions done, diff is down to +296 -96.

The three lines share one css property, so a class each could not combine them. Building the value from three custom properties with empty fallbacks worked, but needed a comment to be readable. Three flags is seven combinations, so plain rules say it instead, one line shorter. Assisted-by: Claude Code:Opus 5

Signed-off-by: silverwind <me@silverwind.io>

silverwind · 2026-07-26T12:21:04Z

I don't see any real problem by using existing "ansi_up" package

ansi_up has many bugs and I fixed all known ones here. I think you should reconsider having a look.

I think it's good we ship ansi rendering first-party because then we can deeply integrate. The other option would be me publishing this as a npm package with class names being the interface towards our CSS.

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 24, 2026

github-actions Bot added the topic/gitea-actions related to the actions of Gitea label Jul 24, 2026

silverwind added 2 commits July 24, 2026 20:06

refactor: inline the single-use fast-path condition

b79b0c8

Assisted-by: Claude Code:Opus 5

silverwind requested a review from Copilot July 24, 2026 18:33

Copilot started reviewing on behalf of silverwind July 24, 2026 18:33 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

Comment thread web_src/js/render/ansi.ts Outdated

expand comment

bff0488

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: silverwind <me@silverwind.io>

silverwind commented Jul 24, 2026

View reviewed changes

Comment thread web_src/js/render/ansi.ts Outdated

Apply suggestion from @silverwind

7dadd84

Signed-off-by: silverwind <me@silverwind.io>

This comment was marked as outdated.

Sign in to view

fix ansi render instance & state

f04e432

wxiaoguang changed the title ~~perf(actions): skip ansi_up for log lines without escape sequences~~ perf(actions): skip ansi_up for plain log lines, use shared render instance Jul 25, 2026

wxiaoguang previously approved these changes Jul 25, 2026

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 25, 2026

silverwind marked this pull request as draft July 25, 2026 04:54

silverwind added 6 commits July 25, 2026 07:24

fix: restore the js-yaml version

bc47ff5

An unrelated js-yaml downgrade was picked up from a local install and committed alongside the ansi_up removal. Only the ansi_up entries belong in this branch. Assisted-by: Claude Code:Opus 5

silverwind changed the title ~~perf(actions): skip ansi_up for plain log lines, use shared render instance~~ refactor(actions): replace ansi_up with a purpose-built ANSI renderer Jul 25, 2026

silverwind marked this pull request as ready for review July 25, 2026 06:33

silverwind changed the title ~~refactor(actions): replace ansi_up with a purpose-built ANSI renderer~~ enhance(actions): replace ansi_up with a purpose-built ANSI renderer Jul 25, 2026

github-actions Bot added the type/enhancement An improvement of existing functionality label Jul 25, 2026

wxiaoguang removed their request for review July 25, 2026 06:35

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 25, 2026

GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 25, 2026

silverwind added 2 commits July 25, 2026 09:06

silverwind changed the title ~~enhance(actions): replace ansi_up with a purpose-built ANSI renderer~~ enhance(actions): replace ansi_up with first-party code Jul 25, 2026

silverwind added 5 commits July 25, 2026 09:33

test: use the javascript scheme literally in the hyperlink case

706c503

Assembling it at runtime worked around "no-script-url", which does not apply here: the rule flags a literal that starts with the scheme, and this one is a whole escape sequence. Assisted-by: Claude Code:Opus 5

refactor(actions): wrap the initial style check, drop a restating com…

acc65ea

…ment Assisted-by: Claude Code:Opus 5

silverwind commented Jul 25, 2026

View reviewed changes

Comment thread templates/devtest/ansi-render.tmpl Outdated

Apply suggestion from @silverwind

38cb2c7

Signed-off-by: silverwind <me@silverwind.io>

silverwind commented Jul 25, 2026

View reviewed changes

Comment thread web_src/js/render/ansi.test.ts

silverwind commented Jul 25, 2026

View reviewed changes

Comment thread web_src/js/render/ansi.ts Outdated

Apply suggestion from @silverwind

cfe71ed

Signed-off-by: silverwind <me@silverwind.io>

silverwind commented Jul 26, 2026

View reviewed changes

Comment thread web_src/js/render/ansi.ts Outdated

silverwind added 2 commits July 26, 2026 13:06

Apply suggestion from @silverwind

e96ecda

Signed-off-by: silverwind <me@silverwind.io>

Merge branch 'main' into actions-log-ansi-fast-path

ec70602

Uh oh!

Uh oh!

Conversation

silverwind commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

silverwind commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

silverwind commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxiaoguang commented Jul 25, 2026

Uh oh!

silverwind commented Jul 25, 2026

Uh oh!

wxiaoguang commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

silverwind commented Jul 25, 2026

Uh oh!

silverwind commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

silverwind commented Jul 25, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

silverwind commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

silverwind commented Jul 24, 2026 •

edited

Loading

silverwind commented Jul 25, 2026 •

edited

Loading

silverwind commented Jul 25, 2026 •

edited

Loading

wxiaoguang commented Jul 25, 2026 •

edited

Loading

silverwind commented Jul 25, 2026 •

edited

Loading

silverwind commented Jul 26, 2026 •

edited

Loading