[Audit] Rendering pipeline (TUI/GUI) — 5 verified findings (2 medium, 3 low)

[cuttlefisch]

Batched findings for Rendering pipeline (TUI/GUI) from the pre-v0.15 codebase audit (branch audit/pre-v015-review).

Each was produced by an end-to-end capability trace, then independently re-verified by a reviewer whose
brief was to refute it. Only findings that survived refutation appear here, at the verifier's corrected
severity — across the audit, 102 claims became 89 confirmed and 29 were downgraded.

5 findings — 2 medium, 3 low. Tick them off individually; split any one
out into its own issue if it needs real design work.

---

1. The TUI computes gutter width four different ways at four call sites; three of them disagree with the one the text is actually drawn with

duplication · medium

Impact. Two concrete misplacements. (1) In a Conversation or Dashboard buffer in the TUI, render_buffer draws a 3-5 column line-number gutter (it never asks has_gutter) while set_cursor assumes width 0 — the caret sits several columns left of the text it is supposed to be in, and the GUI shows no gutter at all for the same buffer. (2) With show_line_numbers off, render_buffer uses gutter_w = 2 but the collab overlay uses gutter_width(len_lines()) (>= 3, e.g. 5 for a 1000-line file), so every remote peer's cursor bar and selection background is drawn 3 columns to the right of the character it marks; len_lines() vs display_line_count() adds a further off-by-one at each power-of-ten boundary because of ropey's phantom trailing line.

Evidence

Drawing (authoritative for where text lands) — crates/renderer/src/buffer_render.rs:80-84, note the missing has_gutter check:

let gutter_w = if editor.show_line_numbers {
    gutter_width(display_lines)
} else {
    2 // marker column + 1 padding
};

Cursor — crates/renderer/src/cursor.rs:27-33, which DOES check has_gutter:

let gutter_w = if !mae_core::BufferMode::has_gutter(&focused_buf.kind) {
    0
} else if editor.show_line_numbers {
    gutter_width(focused_buf.display_line_count())
} else { 2 };

Collab overlay — crates/renderer/src/lib.rs:566-570, which checks neither has_gutter nor show_line_numbers and uses len_lines() instead of display_line_count():

let gutter_w =
    mae_core::render_common::gutter::gutter_width(buf.rope().len_lines());
buffer_render::render_remote_cursors(frame, inner, editor, win, buf, gutter_w);

Scroll/wrap accounting — crates/mae/src/terminal_loop.rs:170-178 (a fourth copy). The GUI has exactly one: crates/gui/src/layout.rs:272-278, consumed by every GUI drawing site via FrameLayout.gutter_width. has_gutter is false for Conversation | Messages | Visual | Dashboard | Graph (crates/core/src/buffer_mode.rs:106-111), and Conversation/Dashboard fall through the TUI _ arm to render_buffer.

Verification

All four cited sites exist and say what is claimed. crates/renderer/src/buffer_render.rs:80-84 (render_buffer, reached via render_window from the _ arm) does let gutter_w = if editor.show_line_numbers { gutter_width(display_lines) } else { 2 } with no has_gutter check. crates/renderer/src/cursor.rs:27-33 DOES check if !mae_core::BufferMode::has_gutter(&focused_buf.kind) { 0 }. crates/renderer/src/lib.rs:566-570 is verbatim let gutter_w = mae_core::render_common::gutter::gutter_width(buf.rope().len_lines()); — no has_gutter, no show_line_numbers, and len_lines() not display_line_count() (display_line_count subtracts the ropey phantom trailing line, crates/core/src/buffer.rs:920-927). crates/mae/src/terminal_loop.rs:170-178 is a fourth copy (has_gutter + show_line_numbers + scrollbar). The GUI has exactly one, crates/gui/src/layout.rs:272-278, and it includes the has_gutter check. has_gutter is false for Conversation | Messages | Visual | Dashboard | Graph (crates/core/src/buffer_mode.rs:106-111); Messages/Visual/Graph have dedicated TUI match arms, so Conversation and Dashboard are the two kinds that fall through to _ -> render_window -> render_buffer. Dashboard is reachable there: splash only preempts window rendering when active_buffer().kind == Dashboard && window_count() == 1 (crates/core/src/render_common/splash.rs:104-105), so a split leaves Dashboard drawn through render_buffer with a gutter while set_cursor assumes 0.

Scope correction from verification: Real, but conditional rather than universal. In the default configuration for an ordinary Text buffer with show_line_numbers on and no collab session, the drawing site and the cursor site agree; only terminal_loop.rs disagrees (by the scrollbar column — see the next finding). The three genuine divergences are: (a) has_gutter==false kinds (Conversation, and Dashboard once the window is split) draw a gutter but position the caret at gutter 0; (b) the collab remote-cursor overlay uses gutter_width(len_lines()) unconditionally, so with show_line_numbers off it is >=1 column right of the drawn text; (c) len_lines() vs display_line_count() adds a further off-by-one at power-of-ten boundaries for files ending in a newline. All are cosmetic misplacement of caret/peer-cursor cells, not wrong text.

---

2. mae-renderer has 6 tests for ~3,700 lines, all six on one 8-line helper; the 577-line render_buffer, set_cursor, and 976-line popup_render have zero coverage, and no e2e test exercises concealment + highlight together

test-gap · medium

Impact. Findings 1-4 are all reachable through render_buffer and set_cursor and none of them would be caught by any existing test: there is no test that constructs a buffer with a concealed link and an overlapping selection or search match (finding 1 — a guaranteed panic), no test that folds lines and asserts the cursor's screen row (finding 2), no test that renders a has_gutter == false buffer and compares the drawing gutter to the cursor gutter (finding 3), and no test that compares editor.text_area_width to the renderer's own text_width (finding 4). The crate's whole test suite exercises a helper that is a thin wrapper over already-tested shared code — confirmation testing of the least risky 8 lines in the crate, which is exactly the failure mode #14 names.

Evidence

From docs/AUDIT_METRICS.json: crates/renderer/src/buffer_render.rs test_count: 6; every other file in the crate reports test_count: 0 — lib.rs (618 lines, render_frame 226 lines), cursor.rs (167, set_cursor 147), popup_render.rs (976, render_command_palette 162), shell_render.rs (269, max_match_arms: 20), which_key_render.rs (171, render_which_key_popup 147), splash_render.rs, status_render.rs, theme_convert.rs, graph_view_render.rs, messages_render.rs, file_tree_render.rs, debug_render.rs. All 6 existing tests are hex-color-preview unit tests over the 8-line apply_hex_color_preview (crates/renderer/src/buffer_render.rs:846-891: hex6_color_sets_bg, hex3_color_sets_bg, hex_color_contrast_fg_light_bg_gets_black, hex_color_contrast_fg_dark_bg_gets_white, hex_color_no_false_positive_on_non_hex, hex_color_multiple_on_same_line) — and that helper's real logic already lives in render_common::color::find_hex_color_runs, which has its own 9 tests. On the Scheme e2e side, tests/editor/ contains no rendering, scrolling, folding, or display-region file (its 27 files cover editing, nav, undo, keymaps, options, visual mode, windows, hooks, KB, collab). The GUI, by contrast, carries 95 tests including the deliberately adversarial compute_layout_pixel_y_is_monotonic_and_never_overflows_with_tight_epsilon (crates/gui/src/layout.rs:917-990).

Verification

Every number checks out against docs/AUDIT_METRICS.json and the source. crates/renderer/src totals 3,721 lines with test_count 6, all 6 in buffer_render.rs; every other file in the crate reports test_count 0 (lib.rs 618, cursor.rs 167, popup_render.rs 976, shell_render.rs 269, which_key_render.rs 171, plus splash/status/theme_convert/graph_view/messages/file_tree/debug/help/conversation). Metrics confirm max_fn_name: render_buffer, max_fn_lines: 577 and cursor.rs max_fn_name: set_cursor, max_fn_lines: 147, test_count: 0. The 6 test names are exactly as listed (crates/renderer/src/buffer_render.rs:846-891: hex6_color_sets_bg, hex3_color_sets_bg, hex_color_contrast_fg_light_bg_gets_black, hex_color_contrast_fg_dark_bg_gets_white, hex_color_no_false_positive_on_non_hex, hex_color_multiple_on_same_line) and apply_hex_color_preview (:684-692) is an 8-line wrapper over mae_core::render_common::color::find_hex_color_runs, which itself carries 9 tests (crates/core/src/render_common/color.rs). tests/editor/ contains exactly 27 .scm files, none covering rendering/scrolling/folding/display-regions. crates/gui reports 95 tests and compute_layout_pixel_y_is_monotonic_and_never_overflows_with_tight_epsilon is real at crates/gui/src/layout.rs:918.

---

3. TUI text_area_width subtracts a scrollbar column that the TUI never draws, so wrap accounting is one column narrower than the width text is actually wrapped at

bug · low

Impact. With word_wrap on in the TUI (the default scrollbar=true), the scroll guard believes every line wraps at width W-1 while the renderer wraps at W. Any line whose content is exactly W display columns is counted as 2 visual rows but drawn as 1, so ensure_scroll_wrapped and last_visible_wrapped_line under-fill the viewport and gj/gk step to the wrong row — the exact scroll-guard-fighting / ghost-line class that crates/gui/src/RENDERING.md warns about. With wrap off, ensure_scroll_horizontal(text_w) starts horizontal scrolling one column early, so the last visible column is permanently blank in the TUI.

Evidence

crates/mae/src/terminal_loop.rs:177-184 (the TUI event loop):

let scrollbar_w: usize = if editor.scrollbar { 1 } else { 0 };
let text_w = inner_w.saturating_sub(gutter_w).saturating_sub(scrollbar_w);
editor.text_area_width = text_w;
if !editor.word_wrap {
    editor.window_mgr.focused_window_mut().ensure_scroll_horizontal(text_w);
}

But the option is explicitly GUI-only — crates/core/src/options.rs:250-252:

opt!("scrollbar", &[],
    "Show vertical scrollbar in the GUI",
    OptionKind::Bool, "true", Some("editor.scrollbar"), &[]),

default "true". There is no scrollbar in the TUI at all: crates/renderer/src/ has no scrollbar.rs (cf. crates/gui/src/scrollbar.rs) and grepping scrollbar across crates/renderer/src/ returns nothing. The width the TUI actually wraps at is crates/renderer/src/buffer_render.rs:118:

let text_width = (area.width as usize).saturating_sub(gutter_w);

— no scrollbar subtraction. editor.text_area_width feeds line_text_visual_rows (crates/core/src/editor/render_ops.rs:228-237) and thus wrap_line_display_rows, the visual-rows cache, ensure_scroll_wrapped, and gj/gk (crates/core/src/editor/dispatch/nav.rs:86,116,157,175).

Verification

Verified. crates/mae/src/terminal_loop.rs:176-184 does let scrollbar_w: usize = if editor.scrollbar { 1 } else { 0 }; let text_w = inner_w.saturating_sub(gutter_w).saturating_sub(scrollbar_w); editor.text_area_width = text_w;. The option is documented GUI-only — crates/core/src/options.rs:250-252 opt!("scrollbar", &[], "Show vertical scrollbar in the GUI", OptionKind::Bool, "true", ...) — and grep -rn scrollbar crates/renderer/src/ returns zero hits, so the TUI draws none. The actual TUI wrap width is crates/renderer/src/buffer_render.rs:118 let text_width = (area.width as usize).saturating_sub(gutter_w); with no scrollbar subtraction. Root cause is visible: crates/mae/src/gui_app.rs:1429-1450 contains a byte-identical block (same inner_w, same gutter ladder, same scrollbar_w, same ensure_scroll_horizontal call), so the TUI copy is a copy-paste of the GUI's, principle #8/#13.

Scope correction from verification: The defect is real but its consequences are narrower than stated. word_wrap defaults to false (crates/core/src/options.rs:117-119), so the wrap/gj/gk mis-accounting requires opting in. With wrap off, ensure_scroll_horizontal (crates/core/src/window.rs:734-744) only shifts col_offset one column earlier than necessary — the rightmost column still shows real text, it is not "permanently blank". Accurate claim: with the default scrollbar=true, TUI editor.text_area_width is one column narrower than the width the TUI renderer actually wraps/draws at, because a GUI-only block was copied into the TUI event loop.

---

4. The entire per-character style-layering pipeline is duplicated verbatim between the TUI and GUI renderers instead of living in render_common

duplication · low

Impact. This is the structural cause of finding 1: the unclamped rope-to-display index bug had to be written twice and now has to be fixed twice, and any test written against one backend proves nothing about the other. It is also why render_buffer is 577 lines / nesting 7 and render_buffer_content is 609 lines / nesting 7 (docs/AUDIT_METRICS.json) against an 80-line / depth-4 ceiling — the accepted file-size exceptions in docs/AUDIT_BASELINE.json cover the files, not these functions. Extracting a render_common::char_styles producer (buffer + window + spans -> per-display-char style vector) would delete ~180 duplicated lines and make the index remapping a single, testable place.

Evidence

The same five layers, in the same order, with near-identical comments and identical index arithmetic, appear in both backends: TUI crates/renderer/src/buffer_render.rs:228-440 (syntax :238-282, display-region link :285-315, hex preview :318, cursorline :321-327, LSP highlight :330-360, selection :363-378, search :381-392, image :395-400, diagnostics :403-422, secondary cursors :425-440) and GUI crates/gui/src/buffer_render.rs:214-399 (// Layer 1: Tree-sitter syntax spans. :215, // Layer 1b: Display region link styling :271, // Layer 2: Hex color preview. :305, // Layer 3: Cursorline bg. :308, // Layer 3b: LSP document highlights :318, // Layer 4: Visual selection. :351, // Layer 5: Search highlights :381). The only real difference is the sink type (Vec<Style> vs Vec<CharStyle>). What IS shared is only the leaf helpers: render_common::color::find_hex_color_runs (called at crates/renderer/src/buffer_render.rs:685 and crates/gui/src/buffer_render.rs:991), render_common::diagnostics::compute_diagnostic_spans, render_common::gutter::*, display_region::rope_col_to_display_col. Principle #8 is explicit: "All layout math, content formatting, span computation, and data preparation lives in mae-core ... If two renderers compute the same thing, extract it." Even the ad-hoc fallback colors have drifted apart: TUI crates/renderer/src/buffer_render.rs:434-436 uses Color::Rgb(100, 100, 180) (0.39,0.39,0.71) where GUI crates/gui/src/cursor.rs:366 uses Color4f::new(0.6, 0.6, 0.9, 0.8) for the same ui.cursor.secondary fallback.

Verification

The duplication is real but the description is materially overstated. 'Duplicated verbatim ... near-identical comments and identical index arithmetic' does not hold: the GUI comments are // Layer 1: Tree-sitter syntax spans. (crates/gui/src/buffer_render.rs:215) while the TUI's is // Apply tree-sitter syntax highlights (lowest priority). (crates/renderer/src/buffer_render.rs:237); the GUI uses spans.partition_point(|s| s.byte_end <= line_byte_start) + break, the TUI does a full linear scan with continue — already different index/scan arithmetic. The claim also lists 10 TUI layers against 7 GUI layers under the header 'the same five layers': the GUI handles images (crates/gui/src/buffer_render.rs:596 'Pass 5 (image)'), inline diagnostics (:625-661) and secondary cursors (crates/gui/src/cursor.rs:355+) in separate passes/functions, not inside the per-char style loop. What does hold is the genuinely shared shape — syntax / display-region link / hex / cursorline / LSP highlight / selection / search layered per display char into a per-char style vector in both backends — and a concrete observable divergence: ui.cursor.secondary is defined by no shipped theme (grep across crates/ and assets/ returns only the two fallback sites), so the fallback ALWAYS fires and the two backends draw different colors — TUI Color::Rgb(100, 100, 180) (buffer_render.rs:434-436) vs GUI Color4f::new(0.6, 0.6, 0.9, 0.8) ~= rgb(153,153,230) (crates/gui/src/cursor.rs:366). ROADMAP.md's Architecture Debt already carries a generic entry for this class: 'Ad-hoc solution review: Thorough code review for hardcoded values, duplicated logic between TUI/GUI...'.

Scope correction from verification: Narrower accurate claim: the per-character style-layering core (syntax, display-region link, hex preview, cursorline, LSP highlight, selection, search) is independently implemented in both backends rather than in render_common, and it has already diverged in one user-visible way — the ui.cursor.secondary theme key is defined by no shipped theme, so both backends always take a hardcoded fallback and those fallbacks are different colors (rgb(100,100,180) TUI vs ~rgb(153,153,230) GUI). The '~180 duplicated lines', 'verbatim', and 'identical index arithmetic' figures are not supported; the function-length numbers cited (577/609) are correct but come from docs/AUDIT_METRICS.json and are already the subject of the ratcheted file/function-size backlog in ROADMAP.md.

---

5. The _-arm markup-span selection is duplicated across both lib.rs files and has already drifted: the GUI's large-file degrade guard was never added to the TUI; the breadcrumb bar hardcodes colors in the TUI only

parity-gap · low

Impact. Two live consequences. (1) On a file past large_file_lines, the GUI drops markup spans while the TUI still merges them — and because the markup cache is only viewport-local for large files (crates/renderer/src/lib.rs:225-253), a cache miss makes the TUI collect the entire rope into a String and run compute_markup_spans over it inside the render path, exactly the work the degrade mechanism exists to shed. (2) The TUI breadcrumb bar renders dark-gray-on-black regardless of theme, so it is unreadable on a light theme and ignores show_breadcrumbs' companion theming that the GUI honors — a hardcoded value where the rest of the pipeline is theme-driven (#7).

Evidence

GUI crates/gui/src/lib.rs:1210-1215:

let degraded = editor.should_degrade_features(win.buffer_idx);
let flavor = if degraded {
    mae_core::MarkupFlavor::None
} else {
    editor.effective_markup_flavor(win.buffer_idx)
};

TUI crates/renderer/src/lib.rs:528, same position in an otherwise byte-for-byte identical block (:521-552 vs :1203-1239):

let flavor = editor.effective_markup_flavor(win.buffer_idx);

The TUI then reaches the uncached fallback at crates/renderer/src/lib.rs:541-542:

let source: String = buf.rope().chars().collect();
enriched.extend(mae_core::compute_markup_spans(&source, flavor));

Separately, the breadcrumb bar — the same feature in the same function pair — is themed in the GUI (crates/gui/src/lib.rs:747-749):

let bg = theme::ts_bg(editor, "ui.statusline").unwrap_or(theme::DEFAULT_BG);
let fg = theme::ts_fg(editor, "comment");

and hardcoded in the TUI (crates/renderer/src/lib.rs:597):

let style = Style::default().fg(Color::DarkGray).bg(Color::Black);

crates/gui/src/RENDERING.md:41-45 claims this arm is already consolidated: "render_common::spans::highlight_spans_for_buffer() centralizes span selection ... Both renderers call this in their _ arm — if Some, use shared spans; if None, use syntax spans." Only the Some half is shared; the ~25-line None half is the duplicated, drifted code.

Verification

Both cited divergences exist exactly as quoted. GUI crates/gui/src/lib.rs:1210-1215 has let degraded = editor.should_degrade_features(win.buffer_idx); let flavor = if degraded { MarkupFlavor::None } else { editor.effective_markup_flavor(...) };; the TUI's otherwise near-identical block (crates/renderer/src/lib.rs:521-552) has only let flavor = editor.effective_markup_flavor(win.buffer_idx);. should_degrade_features exists at crates/core/src/editor/option_ops.rs:2669. Breadcrumbs: GUI crates/gui/src/lib.rs:747-749 uses theme::ts_bg(editor, "ui.statusline") / theme::ts_fg(editor, "comment"); TUI crates/renderer/src/lib.rs:597 is let style = Style::default().fg(Color::DarkGray).bg(Color::Black); — a hardcoded pair in a theme-driven pipeline (principle #7). RENDERING.md:41-45 does claim the _ arm is consolidated, and only the Some half is.

Scope correction from verification: The stated consequence (1) is largely defused upstream and should not be counted at medium. The TUI has its own large-file mitigation the finding does not account for: crates/renderer/src/lib.rs:224-253 pre-populates markup_cache viewport-locally (is_large = line_count > editor.large_file_lines -> compute_markup_spans_for_range) before the _ arm runs, and the arm's cache hit only requires matching generation+flavor, so the 'collect the entire rope into a String inside the render path' fallback effectively never fires for a large file. The residual real divergence is that the two backends use different large-file criteria (should_degrade_features = char count / long-line sampling vs large_file_lines), so e.g. a 15K-char single-line file degrades in the GUI but is fully markup-scanned in the TUI. Consequence (2), the hardcoded DarkGray-on-Black breadcrumb bar (unreadable on light themes, ignores the theme the rest of the pipeline honors), is fully confirmed and is the solid part of this finding.

[cuttlefisch]

Part of the pre-v0.15 codebase audit — tracking epic #592.