#595 Stage 2 — TuiShellApp::render_content paints for real through &mut dyn Backend

[JDonaghy]

Part of epic #595, milestone #9 TUI ShellApp Migration. Stage 2 of the work order. Depends on Stage 1.

Goal

Wire TuiShellApp::render_content (src/tui_main/shell_app.rs) to actually paint, through &mut dyn Backend, now that Stage 1 has made the paint path trait-portable.

Scope

• render_content currently computes the screen layout (pure, no Frame needed) but does not paint — see the shell_app.rs module doc. Call the now-portable draw_frame + panel paint path against layout.main_content_bounds.
• Keep the dormant/coexistence stance (still not wired to main.rs/tui_bin.rs; the live event_loop() remains authoritative until Stage 6).

Acceptance

• Add driver_with_shell paint assertions (screen_contains) proving the editor + panels render via the ShellApp path.
• cargo test --no-default-features green.

Dependencies

Depends on Stage 1. Prereq for Stages 3/4/5 wiring against a painting render_content. Refs: PLAN.md Stage 2.

[JDonaghy]

Coordinator: Assignment Complete

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

Summary

Filed #607/#608/#609 (render_content painting-completeness follow-ons) + extended #605's deps, then implemented #601: TuiShellApp::render_content now paints editor windows, tab/breadcrumb bars, per-window status lines, and the 4 editor-anchored popups through &mut dyn Backend. Widened render_tab_bar/draw_breadcrumb_bar/render_window_status_line/render_editor_hover_popup to the trait object; made render_window/render_all_windows's Frame param Option; extracted paint_editor_popups so draw_frame and render_content share one code path; added build_screen_for_shell_content. 2 new driver_with_shell screen_contains tests. Build/clippy/fmt clean, full test suite green (44/44 binaries). Pushed to issue-601-595-stage-2-tuishellapp-render-content-p, not yet a PR.

[JDonaghy]

⏱ Needs attention

Machine: elitebook
Assignment: 66c7e39b346f
Issue: #601
Reason: Running too long

Running 64.9m, past the 60m threshold for type='smoke'.

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: elitebook
Status: done
Exit code: 0
Duration: —

Summary

Smoke test PASSED for #601: build clean, all 7 tui_main::shell_app tests pass incl. both driver_with_shell paint assertions (editor text + multi-window tab bars)

[JDonaghy]

Coordinator: Assignment Complete

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

Summary

Stage 2 render_content diff reviewed: trait-widening is sound (set_theme swap verified no-op), tests are meaningful, scope clean, no platform-neutrality violation

Verdict: approve

Review findings

Review: #601 — TuiShellApp::render_content paints for real (Stage 2)

Branch: issue-601-595-stage-2-tuishellapp-render-content-p
Diff base: origin/develop...origin/issue-601-595-stage-2-tuishellapp-render-content-p
Single commit: 5730bc1 feat(#601): TUI Stage 2 -- TuiShellApp::render_content paints for real

Summary of what I checked

Since my reviewer role forbids running cargo build/cargo test myself (a separate
pre-merge smoke gate covers that), I did a careful static/structural review instead:
traced every signature change to its call sites, cross-checked the widened
&mut dyn quadraui::Backend parameters against the real quadraui trait/inherent-method
split, and verified the APIs the new tests call actually exist with matching signatures.

Scope

Touches exactly 4 files: PLAN.md, src/tui_main/{panels,render_impl,shell_app}.rs.
No GTK changes, no Cargo/build-system changes, no scope creep. This matches the issue
(TUI-only Stage 2 of the dormant TuiShellApp migration).

Correctness — signature widening (&mut TuiBackend → &mut dyn quadraui::Backend)

Widened render_tab_bar, draw_breadcrumb_bar, render_editor_hover_popup,
render_window_status_line, render_all_windows/render_window. Verified:

• draw_frame (the live path) keeps a concrete backend: &mut super::backend::TuiBackend
  parameter and calls these helpers — this compiles via Rust's automatic unsized
  coercion at the call site, so the live TUI path is unaffected.
• The set_current_theme → set_theme swap in the newly-widened functions is required
  (trait objects can't call the concrete type's inherent set_current_theme, only
  trait methods) and is behavior-preserving: quadraui's impl Backend for TuiBackend
  (quadraui/src/tui/backend.rs:780) implements set_theme as self.set_current_theme(theme),
  so nothing changes at runtime.
• Removed use quadraui::Backend; in the two panels.rs functions that now take the
  trait object directly — correct, since method calls on dyn Trait don't need the
  trait imported (only concrete types calling via trait methods do). Other still-concrete
  call sites in the same file correctly keep their local use quadraui::Backend;.
• render_all_windows/render_window's new frame: Option<&mut ratatui::Frame>: None
  from render_content cleanly skips cursor placement and render_separators (raw
  Buffer writes with no trait equivalent yet) while draw_frame keeps passing
  Some(frame) unchanged. Both call sites in draw_frame were updated consistently
  (I grepped for all render_all_windows(/render_window( call sites — no stragglers).
• paint_editor_popups extraction is a straight, faithful move of the five popup blocks
  out of draw_frame (compared old vs. new hunks line-by-line) — logic unchanged, just
  parameterized on area: Rect instead of re-deriving frame.area() per block.

TuiShellApp::render_content itself

• Computes area from layout.main_content_bounds with a width/height < 1.0 early-return
  guard, sets the theme, builds the screen via the new build_screen_for_shell_content,
  then paints windows → tab bars → breadcrumb bars → editor popups, in the same order
  draw_frame uses (windows first so tab bars draw on top).
• build_screen_for_shell_content is a faithful "doesn't double-subtract chrome" variant
  of build_screen_for_tui, well justified in its doc comment (main_content_bounds already
  excludes activity-bar/sidebar width, since quadraui's AppShell::render painted that
  chrome and carved it out already).
• RefCell/Cell field borrows (completion_layout, editor_hover_link_rects,
  editor_hover_scrollbar, editor_hover_popup_rect) are each on distinct fields, no
  double-borrow risk; Cell<Option<(u16,u16,u16,u16)>>::get()/set() round-trip is fine
  since the tuple is Copy.
• Verified Engine::buffer_mut() (src/core/engine/accessors.rs:150),
  Buffer::insert(usize, &str) (src/core/buffer.rs:49), and
  Engine::open_editor_group(SplitDirection) (src/core/engine/windows.rs:1951) all exist
  with signatures matching the new tests' usage.
• render::tab_bar_draw_targets/breadcrumb_draw_targets signatures match the call sites
  added in shell_app.rs.

Tests

Two new tests replace the previous "just don't panic" smoke assertion with actual content
assertions, exactly per the acceptance criteria:

• render_content_paints_editor_text_via_shell_app — inserts a marker string, asserts
  driver.screen().contains(...).
• render_content_paints_multiple_windows_via_shell_app — opens a vertical split, asserts
  the marker appears exactly twice (once per pane), exercising the multi-window/tab-bar
  path (render_all_windows + tab_bar_draw_targets's editor_group_split branch).

Both are meaningful regression tests, not rubber-stamp assertions, and match the issue's
"Add driver_with_shell paint assertions (screen_contains)" acceptance line.

Platform-neutrality rule

No new per-backend feature logic — this is the TUI side literally catching up to the
GTK ShellApp (#493) parity that already shipped, converting existing TUI paint call
sites to route through the shared quadraui::Backend trait so the render path becomes
portable. No GTK changes were needed or made. Consistent with GOALS.md/milestone #7's
direction, not a violation of the mandatory rule.

Documentation

PLAN.md updates are detailed, cross-reference the right issue numbers (#600/#601/#602/
#604/#605/#607/#608/#609), and accurately describe what did/didn't land. The shell_app.rs
module doc and new/updated function doc comments are unusually thorough and internally
consistent with the diff's actual behavior — I didn't find any doc comment overclaiming
what the code does.

Findings

None blocking. I did not find any correctness, scope, security, or platform-neutrality
issues in this diff. The trait-widening is well-reasoned and I independently verified the
one subtlety that could have silently broken behavior (the set_current_theme →
set_theme swap) against quadraui's actual trait/inherent-method implementations, and it
is a no-op change at runtime.

Per my reviewer role I did not execute cargo build/cargo test --no-default-features/
cargo clippy myself — that's the separate pre-merge smoke gate's job — but nothing in
this diff structurally suggests it would fail to build or test green.

Verdict: approve