VS Code parity (Tier 1 of #699): GTK chrome metrics — explorer row, tab/breadcrumb height, tab ordinals, mnemonics, indent guides

[JDonaghy]

Tier 1 of #699 (VS Code visual-parity audit). Six metric changes; no new features, no
theme-colour work (that is Tier 2), no quadraui changes (that is Tier 3).

These are what make vimcode's chrome read ~15–25% larger than VS Code's at the same DPI.

Changes

1. Explorer row height 28.0 → 22.0 px — src/gtk/mod.rs:1572
   (explorer_row_height_cell). Matches VS Code's list.rowHeight.

2. Decouple the tab-bar height from the editor font. Today it is
   ceil(line_height * 1.6), so raising settings.font_size inflates the chrome.
   Make it a fixed 35 px (VS Code's tab height). Four duplicate sites compute
   this — fix all of them, ideally by hoisting one shared constant/helper:
   src/gtk/mod.rs:1416, :2647, :10207, :10346.

3. Decouple the breadcrumb row from the editor font. Same four sites add a full
   line_height when settings.breadcrumbs is on, which is why the breadcrumb renders
   as large as the code. Target: a 22 px row with ~12 px text, independent of
   font_size.

4. Drop the ordinal prefix from tab labels — src/render.rs:10017, plus the two
   [No Name] arms at :10022 and :10025. " {i+1}: {name} " → "{name}".
   Not purely cosmetic: quadraui::TabItem's doc contract says backends locate the
   filename "after the last \": \"" in order to underline it, so re-check the tab-bar
   underline and close-glyph hit path with the prefix gone — do not just change the
   format string. src/gtk/mod.rs's CLOSE_* metrics and tighten_close_bounds are
   the relevant hit-test code.

5. Stop underlining menu mnemonics unconditionally — src/render.rs:3265 passes
   format!("&{name}") for every item and quadraui's GTK menu bar underlines the char
   after & with no Alt gate (quadraui/src/gtk/menu_bar.rs:105-108). VS Code reveals
   mnemonics only while Alt is held. Prefer gating the & on Alt-held state; if that
   state is not reachable from this call site, drop the & and say so in the PR.

6. Turn indent guides on by default — the feature is fully implemented on both
   sides (quadraui::EditorLine::indent_guides, painted at
   quadraui/src/gtk/editor.rs:294) and gated on a vimcode setting that currently
   defaults off. VS Code draws them by default. src/core/settings.rs.

Acceptance

Black-box via GtkDriver (src/gtk/testing.rs), asserting on rendered output:

• explorer row pitch == 22 px
• tab-bar height stays 35 px across at least two different settings.font_size
  values — a single-font-size assertion passes with the bug reinstated, so it is not
  coverage for v0.3.0: TUI-only binary (vcd) #2
• breadcrumb row height and text size independent of font_size
• a tab renders as main.rs, not 1: main.rs, and its close glyph still hit-tests at
  the right x-range
• menu-bar items render with no underline when Alt is not held

Follow the repo's RED-first rule: remove the fix, confirm the test goes red, restore,
and say so in the PR.

Files

• src/gtk/mod.rs
• src/render.rs
• src/core/settings.rs
• src/gtk/testing.rs (tests)

[JDonaghy]

Coordinator Assignment

Machine: dellserver
Repo: vimcode
Files: (unspecified)
Do not touch: (nothing else in flight)

Briefing

Issue #700: VS Code parity (Tier 1 of #699): GTK chrome metrics — explorer row, tab/breadcrumb height, tab ordinals, mnemonics, indent guides

Tier 1 of #699 (VS Code visual-parity audit). Six metric changes; no new features, no
theme-colour work (that is Tier 2), no quadraui changes (that is Tier 3).

These are what make vimcode's chrome read ~15–25% larger than VS Code's at the same DPI.

Changes

1. Explorer row height 28.0 → 22.0 px — src/gtk/mod.rs:1572
   (explorer_row_height_cell). Matches VS Code's list.rowHeight.

2. Decouple the tab-bar height from the editor font. Today it is
   ceil(line_height * 1.6), so raising settings.font_size inflates the chrome.
   Make it a fixed 35 px (VS Code's tab height). Four duplicate sites compute
   this — fix all of them, ideally by hoisting one shared constant/helper:
   src/gtk/mod.rs:1416, :2647, :10207, :10346.

3. Decouple the breadcrumb row from the editor font. Same four sites add a full
   line_height when settings.breadcrumbs is on, which is why the breadcrumb renders
   as large as the code. Target: a 22 px row with ~12 px text, independent of
   font_size.

4. Drop the ordinal prefix from tab labels — src/render.rs:10017, plus the two
   [No Name] arms at :10022 and :10025. " {i+1}: {name} " → "{name}".
   Not purely cosmetic: quadraui::TabItem's doc contract says backends locate the
   filename "after the last \": \"" in order to underline it, so re-check the tab-bar
   underline and close-glyph hit path with the prefix gone — do not just change the
   format string. src/gtk/mod.rs's CLOSE_* metrics and tighten_close_bounds are
   the relevant hit-test code.

5. Stop underlining menu mnemonics unconditionally — src/render.rs:3265 passes
   format!("&{name}") for every item and quadraui's GTK menu bar underlines the char
   after & with no Alt gate (quadraui/src/gtk/menu_bar.rs:105-108). VS Code reveals
   mnemonics only while Alt is held. Prefer gating the & on Alt-held state; if that
   state is not reachable from this call site, drop the & and say so in the PR.

6. Turn indent guides on by default — the feature is fully implemented on both
   sides (quadraui::EditorLine::indent_guides, painted at
   quadraui/src/gtk/editor.rs:294) and gated on a vimcode setting that currently
   defaults off. VS Code draws them by default. src/core/settings.rs.

Acceptance

Black-box via GtkDriver (src/gtk/testing.rs), asserting on rendered output:

• explorer row pitch == 22 px
• tab-bar height stays 35 px across at least two different settings.font_size
  values — a single-font-size assertion passes with the bug reinstated, so it is not
  coverage for v0.3.0: TUI-only binary (vcd) #2
• breadcrumb row height and text size independent of font_size
• a tab renders as main.rs, not 1: main.rs, and its close glyph still hit-tests at
  the right x-range
• menu-bar items render with no underline when Alt is not held

Follow the repo's RED-first rule: remove the fix, confirm the test goes red, restore,
and say so in the PR.

Files

• src/gtk/mod.rs
• src/render.rs
• src/core/settings.rs
• src/gtk/testing.rs (tests)

[JDonaghy]

⏱ Needs attention

Machine: dellserver
Assignment: 925cbec336c9
Issue: #700
Reason: Running too long

Running 55.4m, past the 45m threshold for type='work'.

Detection + surfacing only — nothing was killed or reassigned. A human should take a look; use coord log <id> to see what it's doing, or coord retry/coord stop to intervene.

[JDonaghy]

Coordinator: Assignment Complete

Machine: dellserver
Status: done
Exit code: 0
Duration: 59m 28s

[JDonaghy]

🩹 Phantom row auto-healed

Machine: dellserver
Assignment: 925cbec336c9
Issue: #700
Stage: work

work row running 59m (past its 45m needs-attention threshold + 10m buffer) — session confirmed dead on machine 'dellserver'

Action taken: finalized phantom session (done)

This session's own recorded machine reported nothing active for it, and the row had already sat well past its own needs-attention threshold — the same phantom-row recovery coord diagnose --reset performs by hand ran automatically. The branch and any commits are preserved, the stage is re-dispatchable, and this repo's drive-queue slot is freed. If this looks wrong, coord log <id> / coord diagnose <repo> <issue> can confirm what happened.

[JDonaghy]

Coordinator: Assignment Complete

Machine: dellserver
Status: done
Exit code: 0
Duration: 4m 51s
Log: /home/john/.coord/logs/1877f9aef829.log

[JDonaghy]

Coordinator: Assignment Complete

Machine: precision
Status: done
Exit code: 0
Duration: —

Summary

Verdict: request-changes

Review findings

Blocking findings

• Item 1 (explorer row height 28.0 → 22.0, src/gtk/mod.rs:1568) has zero test coverage, and — per the PR's own commit message — the edited field (explorer_row_height_cell) is dead code on the live paint path: the explorer tree is actually painted through quadraui's TreeController via backend.line_height(), which never reads this cell. explorer_row_height_cell is only consumed by explorer_row_at, which is reached solely from Msg::ExplorerClick/Msg::ExplorerRightClick, and nothing in the codebase constructs those variants (verified independently: git grep "Msg::ExplorerClick {" in the PR tip only matches the enum definition and the match arm, never a construction site). So the real, on-screen explorer row pitch is completely unchanged by this PR, the issue's acceptance bullet "explorer row pitch == 22 px (black-box via GtkDriver)" is unmet, and no test was added (rightly so — none could pass, since nothing changed). Per this repo's CLAUDE.md Platform-Neutrality Rule, the correct next step for a live fix is a quadraui-side row-height override for draw_tree (analogous to what draw_tab_bar/draw_status_bar already get) — but nothing in the diff shows that a quadraui issue was filed/linked, and merging this as closing VS Code parity (Tier 1 of #699): GTK chrome metrics — explorer row, tab/breadcrumb height, tab ordinals, mnemonics, indent guides #700 would silently misrepresent explorer-row VS Code parity as fixed when it is not. Please either (a) file and link the quadraui follow-up issue and keep item 1 explicitly open/tracked rather than letting VS Code parity (Tier 1 of #699): GTK chrome metrics — explorer row, tab/breadcrumb height, tab ordinals, mnemonics, indent guides #700 close on it, or (b) make this gap unmissable in the PR description/issue before merge.

Non-blocking concerns

• Item 5 (src/render.rs:3272, dropping the & from menu labels) is a genuine no-op for rendered output, not just an incomplete fix. Checked against the pinned quadraui rev (29b98d6647ed808236dc3a11e97cd5d070192e91): MenuBar::find_alt_target/alt_char_byte_range both fall back to "underline/match the first character" when a label has no &. Every entry in vimcode's MENU_STRUCTURE already has its (unused) _alt char equal to the label's own first letter ('f' for "File", 'e' for "Edit", etc.), and the old code always did format!("&{name}"), i.e. always marked the first letter anyway. So old label "&File" and new label "File" produce byte-for-byte identical rendered output (first-letter underline, unconditionally) — the PR changes representation but not behavior at all. The commit message says as much ("this alone does NOT suppress the underline"), which is honest, but it's worth being explicit that this isn't a partial fix, it's a pure no-op; the acceptance bullet "menu-bar items render with no underline when Alt is not held" is completely unaddressed. This matches the issue's own documented fallback ("if that state is not reachable from this call site, drop the & and say so"), so I'm not blocking on it, but please confirm a quadraui issue was actually filed for the Alt-gate/no-&-means-no-underline gap, per the Platform-Neutrality Rule's mandatory step 2.
• Item 3 (breadcrumb decoupling) only decouples the row height (BREADCRUMB_ROW_HEIGHT_PX = 22.0, well tested by breadcrumb_row_adds_a_fixed_22px_not_a_whole_line_height). The issue's target of "~12 px text, independent of font_size" is explicitly not addressed — Backend::draw_status_bar has no per-call font override in the pinned quadraui rev, and breadcrumb text glyph size still tracks settings.font_size. This is disclosed in the code comments and commit message, and is consistent with the "no quadraui changes" tier boundary, but the acceptance bullet "breadcrumb row height and text size independent of font_size" is only half-satisfied and has no test for the text-size half (understandably, since it isn't fixed). Same ask as above: confirm/link the quadraui follow-up.
• Item 6's new regression test (src/core/settings.rs) only asserts settings.indent_guides defaults to true — a state assertion, not rendered output. CLAUDE.md is explicit that state-populated assertions are the exact anti-pattern that let Command palette fails to open silently (broken on develop) #587/GTK live render path silently drops 13 populated ScreenLayout fields (rest of #587's blast radius) #592 (ScreenLayout.picker) hide for months. Low severity here because default_indent_guides() -> true already landed in a prior commit (verified: true at the branch's merge-base too) and the actual paint-gating logic is pre-existing/untouched by this PR, so there's no fresh behavior to black-box test — but a GtkDriver/TuiDriver test that actually confirms indent guides paint by default would be stronger coverage than pinning the settings field alone.

Nits

• The TUI snapshot diffs (command_line.snap, insert_mode.snap, etc.) now show [No Name]× with zero spacing between the tab label and the close glyph, versus the old [No Name] × which had a one-column gap (previously supplied incidentally by the trailing space in " 1: name "). GTK still gets breathing room via the CLOSE_TAB_INNER_GAP pixel constant, but TUI's tab format string has nothing filling that role now. Cosmetically cramped; consider a deliberate single-space pad before the close glyph in the TUI tab format if that's not intentional.
• render_content_paints_group_divider_via_shell_app (src/tui_main/shell_app.rs) weakens its bound from col < right_tab_start to col <= right_tab_start. The accompanying comment explains why (dropping the leading " N: " pad means a pane's own tab text can now start in the same column its body divider occupies), and the reasoning checks out, but it's worth a second pair of eyes since it slightly loosens what was previously a strict inequality in a hit-test-adjacent test.

Independent verification performed: read git show 4e43768...:CLAUDE.md for project rules; diffed against merge-base c19d2ed; cross-checked TabInfo/TabItem/tighten_close_bounds claims and the mnemonic underline/Alt-activation logic directly against the pinned quadraui rev source (~/src/quadraui @ 29b98d6647ed808236dc3a11e97cd5d070192e91); confirmed Msg::ExplorerClick is never constructed in the PR tip's src/gtk/mod.rs. Did not run the test suite (per instructions).

[JDonaghy]

Review Complete — ⚠️ Changes Requested

Reviewer findings could not be posted directly to PR #702 (gh pr review was rejected — likely a self-review restriction). Findings are reproduced here.

Note: a completion comment was posted earlier but findings could not be extracted at that time. These are the retroactive findings.

Blocking findings

• Item 1 (explorer row height 28.0 → 22.0, src/gtk/mod.rs:1568) has zero test coverage, and — per the PR's own commit message — the edited field (explorer_row_height_cell) is dead code on the live paint path: the explorer tree is actually painted through quadraui's TreeController via backend.line_height(), which never reads this cell. explorer_row_height_cell is only consumed by explorer_row_at, which is reached solely from Msg::ExplorerClick/Msg::ExplorerRightClick, and nothing in the codebase constructs those variants (verified independently: git grep "Msg::ExplorerClick {" in the PR tip only matches the enum definition and the match arm, never a construction site). So the real, on-screen explorer row pitch is completely unchanged by this PR, the issue's acceptance bullet "explorer row pitch == 22 px (black-box via GtkDriver)" is unmet, and no test was added (rightly so — none could pass, since nothing changed). Per this repo's CLAUDE.md Platform-Neutrality Rule, the correct next step for a live fix is a quadraui-side row-height override for draw_tree (analogous to what draw_tab_bar/draw_status_bar already get) — but nothing in the diff shows that a quadraui issue was filed/linked, and merging this as closing VS Code parity (Tier 1 of #699): GTK chrome metrics — explorer row, tab/breadcrumb height, tab ordinals, mnemonics, indent guides #700 would silently misrepresent explorer-row VS Code parity as fixed when it is not. Please either (a) file and link the quadraui follow-up issue and keep item 1 explicitly open/tracked rather than letting VS Code parity (Tier 1 of #699): GTK chrome metrics — explorer row, tab/breadcrumb height, tab ordinals, mnemonics, indent guides #700 close on it, or (b) make this gap unmissable in the PR description/issue before merge.

Non-blocking concerns

• Item 5 (src/render.rs:3272, dropping the & from menu labels) is a genuine no-op for rendered output, not just an incomplete fix. Checked against the pinned quadraui rev (29b98d6647ed808236dc3a11e97cd5d070192e91): MenuBar::find_alt_target/alt_char_byte_range both fall back to "underline/match the first character" when a label has no &. Every entry in vimcode's MENU_STRUCTURE already has its (unused) _alt char equal to the label's own first letter ('f' for "File", 'e' for "Edit", etc.), and the old code always did format!("&{name}"), i.e. always marked the first letter anyway. So old label "&File" and new label "File" produce byte-for-byte identical rendered output (first-letter underline, unconditionally) — the PR changes representation but not behavior at all. The commit message says as much ("this alone does NOT suppress the underline"), which is honest, but it's worth being explicit that this isn't a partial fix, it's a pure no-op; the acceptance bullet "menu-bar items render with no underline when Alt is not held" is completely unaddressed. This matches the issue's own documented fallback ("if that state is not reachable from this call site, drop the & and say so"), so I'm not blocking on it, but please confirm a quadraui issue was actually filed for the Alt-gate/no-&-means-no-underline gap, per the Platform-Neutrality Rule's mandatory step 2.
• Item 3 (breadcrumb decoupling) only decouples the row height (BREADCRUMB_ROW_HEIGHT_PX = 22.0, well tested by breadcrumb_row_adds_a_fixed_22px_not_a_whole_line_height). The issue's target of "~12 px text, independent of font_size" is explicitly not addressed — Backend::draw_status_bar has no per-call font override in the pinned quadraui rev, and breadcrumb text glyph size still tracks settings.font_size. This is disclosed in the code comments and commit message, and is consistent with the "no quadraui changes" tier boundary, but the acceptance bullet "breadcrumb row height and text size independent of font_size" is only half-satisfied and has no test for the text-size half (understandably, since it isn't fixed). Same ask as above: confirm/link the quadraui follow-up.
• Item 6's new regression test (src/core/settings.rs) only asserts settings.indent_guides defaults to true — a state assertion, not rendered output. CLAUDE.md is explicit that state-populated assertions are the exact anti-pattern that let Command palette fails to open silently (broken on develop) #587/GTK live render path silently drops 13 populated ScreenLayout fields (rest of #587's blast radius) #592 (ScreenLayout.picker) hide for months. Low severity here because default_indent_guides() -> true already landed in a prior commit (verified: true at the branch's merge-base too) and the actual paint-gating logic is pre-existing/untouched by this PR, so there's no fresh behavior to black-box test — but a GtkDriver/TuiDriver test that actually confirms indent guides paint by default would be stronger coverage than pinning the settings field alone.

Nits

• The TUI snapshot diffs (command_line.snap, insert_mode.snap, etc.) now show [No Name]× with zero spacing between the tab label and the close glyph, versus the old [No Name] × which had a one-column gap (previously supplied incidentally by the trailing space in " 1: name "). GTK still gets breathing room via the CLOSE_TAB_INNER_GAP pixel constant, but TUI's tab format string has nothing filling that role now. Cosmetically cramped; consider a deliberate single-space pad before the close glyph in the TUI tab format if that's not intentional.
• render_content_paints_group_divider_via_shell_app (src/tui_main/shell_app.rs) weakens its bound from col < right_tab_start to col <= right_tab_start. The accompanying comment explains why (dropping the leading " N: " pad means a pane's own tab text can now start in the same column its body divider occupies), and the reasoning checks out, but it's worth a second pair of eyes since it slightly loosens what was previously a strict inequality in a hit-test-adjacent test.

Independent verification performed: read git show 4e43768...:CLAUDE.md for project rules; diffed against merge-base c19d2ed; cross-checked TabInfo/TabItem/tighten_close_bounds claims and the mnemonic underline/Alt-activation logic directly against the pinned quadraui rev source (~/src/quadraui @ 29b98d6647ed808236dc3a11e97cd5d070192e91); confirmed Msg::ExplorerClick is never constructed in the PR tip's src/gtk/mod.rs. Did not run the test suite (per instructions).

[JDonaghy]

Coordinator: Assignment Complete

Machine: dellserver
Status: done
Exit code: 0
Duration: 15m 45s
Log: /home/john/.coord/logs/53c42752062c.log

[JDonaghy]

Coordinator: Assignment Complete

Machine: dellserver
Status: done
Exit code: 0
Duration: 7m 51s
Log: /home/john/.coord/logs/a3de7015e3b6.log

[JDonaghy]

Coordinator: Assignment Complete

Machine: precision
Status: done
Exit code: 0
Duration: —

Summary

Verdict: approve

Review findings

Blocking findings

None.

Non-blocking concerns

• Item 1 (explorer row height) is still not actually fixed on the live paint path — the worker cannot file GitHub issues, so the required quadraui-side draw_tree row-height override is still outstanding. The fix commit does exactly what the prior review's option (b) asked: it replaced the doc comment on explorer_row_height_cell (src/gtk/mod.rs:492-514) with a loud, unmissable warning (⚠️, bold) stating the field is dead on the paint path, naming the required quadraui-side fix, and stating a quadraui issue must be filed before treating VS Code parity (Tier 1 of #699): GTK chrome metrics — explorer row, tab/breadcrumb height, tab ordinals, mnemonics, indent guides #700's explorer-row-pitch acceptance bullet as satisfied. The commit message repeats this explicitly. This is consistent with this repo's Platform-Neutrality Rule (never hack around a missing quadraui primitive in a backend file; file a quadraui issue instead) and is an honest, hard-to-miss escalation rather than a silent gap. The coordinator still needs to actually file/link that quadraui issue and keep item 1 open before VS Code parity (Tier 1 of #699): GTK chrome metrics — explorer row, tab/breadcrumb height, tab ordinals, mnemonics, indent guides #700 is treated as closed — that's now a coordinator action item, not something further vimcode-side code can fix.
• Same pattern applied consistently to items 3 (breadcrumb text size still tracks font_size — needs a draw_status_bar font override in quadraui) and 5 (menu mnemonic underline — quadraui's alt_char_byte_range still underlines char 0 when there's no &, so dropping the ampersand doesn't fully satisfy "no underline when Alt not held"): both now carry the same "a quadraui issue must be filed, don't treat this acceptance bullet as satisfied" marker. Worth confirming these follow-up quadraui issues actually get filed before VS Code parity (Tier 1 of #699): GTK chrome metrics — explorer row, tab/breadcrumb height, tab ordinals, mnemonics, indent guides #700 is closed.

Nits

None.

Verified in this iteration's single fix commit (f1076d0):

• Item 6 (indent guides default-on) now has real pixel-level black-box coverage: indent_guides_paint_by_default (src/gtk/testing.rs) renders the same indented buffer through GtkDriver with Engine::new()'s real default vs. indent_guides: false forced off, and asserts the two frames' pixels differ in the indent-guide column region — not just that settings.indent_guides is true (the exact ScreenLayout.picker/Command palette fails to open silently (broken on develop) #587 antipattern this repo's CLAUDE.md warns about). RED-first is documented (forcing both harnesses off collapses the diff to 0). This is solid rendered-output coverage, not state-only.
• TUI close-glyph gap nit: root-caused and fixed at the single TabInfo source (build_tab_bar_for_group_by_id in render.rs) by appending one trailing space to the label (name + " "), rather than patching downstream at the quadraui::TabItem conversion — correctly reasoned, since tab_hit_width/tab_hit_width-adjacent hit-test math measures TabInfo.name directly, so padding added only at the TabItem step would have desynced hit-test width from painted width (the Tab hit-testing still uses .chars().count(): wide (CJK/emoji) tab names misalign tooltip and click/drag boxes (fast follow-up to #620) #654 class of bug). The six affected TUI snapshots were regenerated consistently ([No Name]× → [No Name] ×), and the shared TabInfo builder is used by both TUI and GTK screen-layout construction, so both backends stay in sync.
• No regressions spotted: the state.display_name().to_string() → state.display_name() change in the same hunk is a harmless redundant-clone cleanup (display_name() already returns String). src/core/settings.rs and src/gtk/click.rs are untouched by this fix commit (already reviewed in iteration 0, no re-review needed per scope).

Overall: the fix commit correctly and completely addresses the prior review's blocking finding (via honest documentation/escalation rather than a fake fix, which is the right call given the Platform-Neutrality Rule and that this worker cannot file GitHub issues), adds genuine rendered-pixel coverage for item 6, and fixes the close-glyph nit soundly with matching snapshot updates. Approving.