fix(tui): always render selected columns; stop dropping by terminal width

[avihut]

Summary

Stops the TUI from silently dropping low-priority columns (LastCommit, Hash, Owner, Age, ...) when the natural table width exceeds the terminal. The full intended column set now renders every time; fit_widths_to_available shrinks Branch / Path / LastCommit with ellipses and accepts overflow, mirroring the blocking-list renderer's tabled::Width::truncate(...).priority(Priority::max(true)).

Affects daft list (LiveTable), daft sync, daft clone, daft prune, daft repo remove. All five flow through src/output/tui/render.rs::render_table so the fix is centralized to one match-expression collapse.

Behavior change worth surfacing

For phased commands (sync, clone, prune, repo remove) with no --columns flag, the Hash column is now always visible. Pre-fix it was the first column dropped on most realistic terminal widths, so most users have never seen it by default. Consistent with the spec — flagging here in case reviewers want to push back.

A pre-existing asymmetry is preserved: daft sync with no flag falls back to ALL_COLUMNS (includes Hash); daft sync --columns +size uses the modifier-mode base ListColumn::tui_defaults() which doesn't include Hash. Out of scope for this PR; can be reconciled in a follow-up.

AC deviation

Issue acceptance criterion #4 asked for a YAML regression scenario verifying narrow-terminal behavior. YAML can't exercise this path: xtask/src/manual_test/runner.rs runs commands via Command::new("bash").output() with piped stdio, so terminal_size returns None and the renderer's narrow-width branches never fire. Setting COLUMNS=60 in scenario env: doesn't help — terminal_size reads ioctl, not env. Replaced with two TestBackend tests in render.rs::tests that exercise render_table directly at a constrained width with a long branch name (forcing fit_widths_to_available to shrink Branch from natural to min while keeping all 11 columns).

Cleanup deferred

columns_explicit: bool is threaded through live_table.rs, operation_table.rs, state.rs, list_live.rs, prune.rs, clone.rs, sync.rs, repo/remove.rs. After this fix no render-layer logic branches on it. Marked with // Unused by render after #494; pending removal in a follow-up. comments on the two config-struct fields; full rip is a separate PR.

Test plan

• mise run fmt clean
• mise run clippy clean (zero warnings)
• mise run test:unit — 1701 pass, 0 fail (4 deleted column_selection_* tests replaced by 2 TestBackend regression tests in render.rs)
• Manual smoke per test-plans/render-all-columns.md: 3 widths (60/100/160) × 2 commands (daft list, daft sync), plus mid-sync resize regression check

Fixes #494

🤖 Generated with Claude Code

[avihut]

/claude review

[claude]

Claude finished @avihut's task in 3m 13s —— View job

---

Code Review: fix(tui): always render selected columns; stop dropping by terminal width

• Read changed files
• Analyze Rust correctness and idioms
• Check test coverage (regression test for bug fix)
• Review shell-eval'd hot-path constraints
• Check security and performance
• Post findings

---

Overall assessment: The fix is correct and well-scoped. The select_columns drop loop was the root cause and its removal is clean. fit_widths_to_available correctly mirrors the blocking renderer's priority-shrink-then-accept-overflow approach. No security issues, no unsafe code, no hot-path impact (this is TUI render-loop code, not shell-init / completions eval path).

---

Issues

1. Stale // Priority N doc comments on Column variants — columns.rs:9-32

After removing the priority() method, the doc comments on the enum variants still say "Priority N (always shown)":

/// Sync/prune status indicator. Priority 0 (always shown).
Status,
/// Current/default branch annotation. Priority 1 (always shown).
Annotation,
/// Branch name. Priority 2 (always shown).
Branch,
/// Worktree path. Priority 3.
Path,

"Always shown" is now false for Branch (it can be shrunk to min width), and the priority numbers are orphaned — the priority() method that consumed them was deleted in this same PR. These comments will mislead future maintainers.

Fix this →

2. Deleted size_excluded_from_default_responsive_columns test leaves an unchecked contract

The removed test verified that Size never appears in the default column set. Its property still holds by inspection (ALL_COLUMNS at columns.rs:96-108 explicitly omits Column::Size), but there's now no unit test pinning this invariant. The deleted test's comment said: "Regression: Size must never appear in the default responsive set". Given that Size triggers an expensive filesystem walk, this is worth keeping as a compile-time or test-time assertion.

Minimal addition to columns.rs tests:

#[test]
fn all_columns_never_includes_size() {
    assert!(
        !ALL_COLUMNS.contains(&Column::Size),
        "Size requires --columns +size; it must not appear in the default set"
    );
}

Fix this →

3. Regression test width (100) is actually already overflowing — comment it

At width=100 with a long branch name, fit_widths_to_available hits all three column minimums and still overflows by ~4 chars (11 columns × content + 10 × 2 spacing ≈ 104 at minimums). The test still passes because header.contains("Commit") checks the 6-char header label whose start position (~88) is within the buffer. The test is correct but this is subtle: it's simultaneously testing both "no column dropped" (the regression) and "accept overflow" (the spec). A short comment in the test would clarify this isn't a test of "all columns fit" but of "all column headers are present regardless":

// At width=100 with 11 columns the table exceeds the buffer by ~4 chars
// (all shrinkable columns hit their minimums). This is correct behavior:
// the spec accepts overflow rather than dropping columns. We verify column
// *presence* (header label visible), not pixel-level fit.

4. Minor: columns_explicit dead field (noted, deferred — no action needed)

Both LiveTableConfig::columns_explicit and TableConfig::columns_explicit are written but not read after this PR. The PR explicitly flags these with // Unused by render after #494; pending removal in a follow-up. on both. This is acceptable; public struct fields don't trigger Clippy dead_code warnings. No action needed in this PR.

---

Positive notes

• fit_widths_to_available is a clean algorithm: widest-first shrink to per-column minimums then accept overflow. The u32 accumulator guards against overflow on pathological content widths. O(overflow_pixels × 3) — trivially fast.
• The if columns.contains(&Column::Status) { columns } else { prepend } restructuring (was if !contains { prepend } else { columns }) is semantically identical — correct inversion.
• The centralization to a single render_table match-expression collapse is the right architectural call; the old per-command columns_explicit/select_columns branching was the compounding factor that made the bug hard to spot.
• TestBackend regression tests are a pragmatic replacement for the YAML scenarios that can't exercise ioctl-based terminal_size. The AC deviation is well-reasoned and documented.
• truncate_with_ellipsis handles the last_commit_age + last_commit_subject split correctly (only the subject is truncated, preserving the relative-age prefix).

---

Blocking? No. Issues 1 and 2 are minor (stale comments / missing pin test). Issue 3 is a clarity suggestion. The fix itself is correct and the test coverage is adequate for the regression case.