Fix inline-code link underline hidden by run background in agent output

[warp-dev-github-integration]

Description

Fixes "wrong link highlighting" in agent rich output (reported in Slack): a detected link rendered as inline code (gray code background) lost all or part of its hover underline, in a width-dependent way.

Root cause: A paint-ordering bug in Line::paint_internal (crates/warpui_core/src/text_layout.rs). The run's background/border rect was painted after the run's glyphs and per-glyph underline. The hyperlink underline is a filled rect in the same paint layer as the background, so drawing the background last painted over and hid the underline on any backgrounded run (e.g. an inline-code link). Plain-text links (no run background) were never affected, which matches the reported symptom.

Fix: Split background/border painting into a new paint_run_background and call it before the glyph loop, so glyphs and the underline render on top of the background. paint_run_decorations now only handles the error underline and strikethrough (drawn after glyphs, unchanged); its now-unused font_cache/baseline_position_fn params were dropped.

Truncation correctness (from review): Because the background is now painted before the glyph loop, it must not be drawn for glyphs that the loop then truncates. Each run's actually-drawn glyph x-span is precomputed before drawing — mirroring the same ellipsis cutoff and remaining_width <= 0 stop conditions used by the glyph loop, for both clip directions including start-clipping's reversed iteration. A run with no visible glyphs paints no background, and a partially-truncated run's background/border is clamped to its visible glyph span (so it no longer extends past the visible glyphs or behind an ellipsis). The background is still painted before the per-glyph underline rects, preserving the underline fix.

Affected file: crates/warpui_core/src/text_layout.rs.

Linked Issue

Slack thread: https://warpdev.slack.com/archives/C0BCE7AELJ2/p1782330944826139?thread_ts=1782330944.826139&cid=C0BCE7AELJ2

Testing

Deterministic regression tests in crates/warpui_core/src/text_layout_tests.rs:

• test_run_background_painted_before_underline — asserts (via the resulting Scene draw order) the background rect is drawn before the underline rect, so the underline renders on top. Fails on the pre-fix paint order.
• test_run_background_clamped_to_visible_glyph_span — drives paint_run_background with a visible span narrower than the run and asserts the painted background rect is clamped to that span (36px), not the full run width (120px).
• test_fully_truncated_run_paints_no_background — a leading run consumes the paint bounds so a trailing backgrounded run is fully truncated; asserts no background rect is painted for it.

cargo nextest run -p warpui_core (298 tests) passes, cargo clippy -p warpui_core --all-targets is clean, and cargo fmt -p warpui_core -- --check is clean.

Note on the ellipsis-truncation path: the platform test FontDB reports a zero advance for the … glyph, so ellipsis_width is always 0 in unit tests and the end-to-end ellipsis branch in paint_internal cannot be driven from a unit test. The clamping arithmetic that fixes the ellipsis case is therefore pinned directly via test_run_background_clamped_to_visible_glyph_span, and the visible-span guard via test_fully_truncated_run_paints_no_background.

• I have manually tested my changes locally with ./script/run

Verification

The underline fix was verified on a real running Warp build by rendering a detected link as inline code (gray code background) at multiple terminal widths and comparing against a plain-text-link control:

• Before: the inline-code link's hover underline was partially or fully missing in a width-dependent way (the gray run background was painted over it). The plain-text control link always underlined fully.
• After: the inline-code link shows a full-width underline at every tested width (820 / 1000 / 1180 / 1360 px), matching the plain-text control. Pixel inspection of the cropped link token confirmed a continuous underline row spanning the full token width at each width, where the before-fix crops showed gaps / missing underline pixels under the backgrounded glyphs.

Before/after screenshots (full-width renders at the four widths plus the plain-text control, and the zoomed token crops) are posted in the Slack thread linked above; the "after" comparison image is also attached there (fixed_after.png).

Image embedding note: these screenshots were captured in a sandboxed environment and are shared via the Slack thread rather than embedded inline in this PR body. The behavior is additionally locked in by the regression tests above.

Agent Mode

• Warp Agent Mode - This PR was created via Warp's AI Agent Mode

Conversation: https://staging.warp.dev/conversation/cd3fc717-924c-4770-ac4d-f91f7f2e5774
Run: https://oz.staging.warp.dev/runs/019efb3c-3a78-772e-9788-7e10e517a52c

This PR was generated with Oz.

[oz-for-oss]

@kevinyang372

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

[oz-for-oss]

Overview

This PR changes Line::paint_internal so run background/border painting happens before glyphs and underline rects, and adds a scene draw-order regression test.

Concerns

• The new call site paints run backgrounds before ellipsis clipping decides whether any glyph in that run will actually be drawn; see the inline comment.
• This is a user-facing rendering change, but the PR description does not include screenshots or a screen recording demonstrating the inline-code link hover underline fix end to end. Please attach visual evidence from a local desktop run or a computer-use environment.

Verdict

Found: 0 critical, 2 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

[oz-for-oss]

⚠️ [IMPORTANT] This now paints the background before the ellipsis branch can decide to replace this run's glyphs; backgrounded text that gets truncated can still leave its full clipped background/border behind the ellipsis, so defer this until after the run is known to contribute visible glyphs or clip it to the painted glyph span.

[warp-dev-github-integration]

Good catch — fixed in 82b2787. Painting the background at the top of the 'runs: loop with the full run width meant a run truncated by the ellipsis still painted its background (behind the ellipsis) and a partially-clipped run painted a background wider than its visible glyphs.

I now precompute each run's actually-drawn glyph x-span before the glyph loop, simulating the exact same stop conditions the loop uses (the ClipStyle::Ellipsis && ellipsis_width > 0 && remaining_width < glyph.width cutoff and the remaining_width <= 0 stop) without mutating remaining_width or drawing. Then:

• if no glyphs are visible (visible_right <= visible_left), the background is skipped entirely (fully-truncated run paints nothing);
• otherwise paint_run_background clamps the background/border rect to [visible_left, visible_right] instead of the full run width.

The simulation uses drawn coordinates and handles both clip directions, including start-clipping's reversed iteration (line_origin.x + remaining_width + start_ellipsis_offset). The background is still painted before the per-glyph underline rects, so the underline fix is preserved.

Tests added: test_run_background_clamped_to_visible_glyph_span (clamps to the visible span, not run width) and test_fully_truncated_run_paints_no_background (guard), plus the existing test_run_background_painted_before_underline. Note: the platform test FontDB returns a zero advance for …, so ellipsis_width is 0 in unit tests and the end-to-end ellipsis branch can't be driven there; the clamping math is pinned directly instead. Details in the updated PR description.

[warp-dev-github-integration]

Thanks for the review — both points addressed in 82b2787 (pushed to this branch).

Concern 1 (truncation correctness): The background was being painted at the top of the per-run loop with the full run width, so a fully-truncated run still painted a background (behind the ellipsis) and a partially-truncated run painted one wider than its visible glyphs. Fixed by precomputing each run's actually-drawn glyph x-span before the glyph loop (mirroring the same ellipsis / remaining_width <= 0 stop conditions, for both clip directions including start-clipping's reversed iteration), skipping the background entirely when no glyphs are visible, and clamping the background/border rect to the visible span. The background is still painted before the per-glyph underline rects, so the underline fix is preserved. (More detail in the inline reply.)

Concern 2 (visual evidence): Added a Verification section to the PR description with the before/after behavior at multiple widths (820/1000/1180/1360 px plus a plain-text control) and the pixel-inspection method. The before/after screenshots and the fixed_after.png comparison are posted in the linked Slack thread; I was transparent that the images live in Slack rather than embedded inline because they were captured in a sandbox, and the behavior is locked in by regression tests.

Tests / checks: cargo nextest run -p warpui_core (298 passing, incl. 3 paint-order/truncation tests), cargo clippy -p warpui_core --all-targets, and cargo fmt -p warpui_core -- --check are all green. Working tree is clean with no temp scaffolding.

[warp-dev-github-integration]

/oz-review

[kevinyang372]

/oz-review

[oz-for-oss]

@kevinyang372

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

[oz-for-oss]

Overview

Reviewed the text layout change that moves run background/border painting before glyph and underline drawing, with regression coverage for underline draw order and visible-span clamping.

Concerns

• No blocking correctness or security issues found.
• One performance suggestion is inline: the visible-span simulation currently runs for every text run, even when a run has no background or border to paint.
• No approved or repository spec context was provided, so there were no spec commitments to compare against.

Verdict

Found: 0 critical, 0 important, 1 suggestions

Approve with nits

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

[oz-for-oss]

💡 [SUGGESTION] This pre-pass now runs for every text run, even when no background or border can be painted. Guard the visible-span simulation behind run.styles.border.is_some() || run.styles.background_color.is_some() (or compute it lazily) so normal text paint doesn't do a second glyph walk.

[warp-dev-github-integration]

Addressed in 8897490. The visible-span pre-pass (and the paint_run_background call) is now guarded by run.styles.border.is_some() || run.styles.background_color.is_some(), so normal text runs skip the extra glyph walk entirely and keep the previous single-walk hot path. The underline reorder and truncation clamping are unchanged for backgrounded/bordered runs, and all three tests still pass (cargo nextest run -p warpui_core, clippy, and fmt are green).