//! `review` TUI tab — per-holding performance and risk dashboard.
//!
//! The TUI surface for the `review` view, rendered as a wide line-list
//! table. Mirrors the portfolio tab's sort-key conventions (`>` next
//! column, `<` previous column, `o` reverse direction) so muscle memory
//! transfers between the two tabs.
//!
//! State is small: cursor/scroll offsets aren't tracked here (the App-
//! level scroll handling does it), and the `view` itself is rebuilt on
//! activate/reload from the per-portfolio shared cache.
//!
//! All data sources are App-scoped (`app.portfolio.{summary, file,
//! account_map}`, `app.svc` cached candles/dividends), so the tab's
//! lifecycle is straightforward — load on activate, free on deinit.

const std = @import("std");
const vaxis = @import("vaxis");
const zfin = @import("../root.zig");
const Money = @import("../Money.zig");
const fmt = @import("../format.zig");
const theme = @import("theme.zig");
const tui = @import("../tui.zig");
const framework = @import("tab_framework.zig");
const review_view = @import("../views/review.zig");
const observations_view = @import("../views/observations_view.zig");
const observations = @import("../analytics/observations.zig");
const Journal = @import("../data/Journal.zig");
const input_buffer = @import("input_buffer.zig");
const portfolio_risk = @import("../analytics/portfolio_risk.zig");

const App = tui.App;
const StyledLine = tui.StyledLine;
const StyleSpan = tui.StyleSpan;

// ── Tab-local action enum ─────────────────────────────────────

pub const Action = enum {
    /// Move the active sort field to the next column (right).
    sort_col_next,
    /// Move the active sort field to the previous column (left).
    sort_col_prev,
    /// Flip the current sort direction (asc ↔ desc).
    sort_reverse,
    /// Toggle expansion of the cursor-selected finding (no-op when
    /// the cursor is on a holdings row).
    toggle_expand,
    /// Acknowledge the cursor-selected finding (no-op on holdings
    /// rows or already-acked findings). Appends an `.acknowledged`
    /// entry to the journal and rebuilds the findings view.
    ack,
    /// Un-acknowledge the cursor-selected finding (no-op unless
    /// the cursor is on an acked finding). Sets the journal entry's
    /// state back to `.active` and records an `unacknowledged_at`
    /// breadcrumb.
    unack,
    /// Toggle whether already-acknowledged findings are shown in
    /// the findings table.
    toggle_show_acked,
    /// Set the cursor row's symbol as the App-level active symbol.
    /// Holdings rows expose their `symbol` directly; finding rows
    /// expose their `target`, which is a symbol for per-symbol
    /// findings (most of them) and a label like "Technology" for
    /// sector-level findings. The latter case is harmless — the
    /// active symbol just doesn't change anything per-tab.
    select_symbol,
    /// Toggle the symbol-info overlay popup. Open at cursor
    /// row's symbol when closed; close when open and cursor is
    /// on the same symbol; re-target when open and cursor is on
    /// a different symbol. For findings rows, the symbol used is
    /// the finding's `target` (which is the symbol for
    /// per-symbol findings; harmless no-op label like "Technology"
    /// for sector-level ones).
    toggle_overlay,
};

/// Modal sub-state. `.normal` lets keystrokes flow through the
/// global keymap (sort, scroll, tab nav). `.ack_note` puts the tab
/// into note-input mode: the inline input bar swallows keys via
/// `handleKeyMulti`, Esc cancels, Ctrl+Enter commits the
/// accumulated
/// fragments to a new journal entry.
pub const InputMode = enum {
    normal,
    ack_note,
};

/// Section the unified cursor currently sits in. Computed on demand
/// from `state.cursor` and the row counts via `cursorSection`.
pub const CursorSection = enum {
    /// Cursor index is in `[0, view.rows.len)` — points at a holding.
    holdings,
    /// Cursor index is in `[view.rows.len, view.rows.len + findings_view.rows.len)`
    /// — points at a finding.
    findings,
    /// Both tables are empty (or no view loaded).
    empty,
};

// ── Tab-private state ─────────────────────────────────────────

pub const State = struct {
    /// Whether `loadData` has populated `view` for the currently
    /// loaded portfolio. Cleared by `reload` to force re-build.
    loaded: bool = false,
    /// Computed view. Owned by State; freed in `deinit` and `reload`.
    view: ?review_view.ReviewView = null,
    /// Active sort field. Default `.sector` (asc) provides the
    /// "grouped by sector with symbol-asc tiebreaker" entry state
    /// — see `views/review.sortRows` for why the sector column
    /// bakes in the symbol-asc pre-pass.
    sort_field: review_view.SortField = .sector,
    sort_dir: review_view.SortDirection = .asc,
    /// Content-row index of the column-header line, written by
    /// `buildStyledLines` after appending the header. Used by
    /// `handleMouse` to detect clicks on the header (column-sort
    /// hit-test) vs. clicks on data rows.
    header_row: usize = 0,
    /// Content-row index of the first holdings data row (right
    /// after the header). Used by `handleMouse` to translate clicks
    /// into a holdings_cursor index.
    holdings_first_row: usize = 0,
    /// Content-row index of the first findings data row (right
    /// after the "Findings (...)" heading + separator). Zero when
    /// the findings section isn't rendered. Used by `handleMouse`
    /// to translate clicks into a findings_cursor index.
    findings_first_row: usize = 0,
    /// Content-row count of the findings table (including expansion
    /// lines). Used by `handleMouse` to bound clicks. Zero when no
    /// findings section is rendered.
    findings_row_count: usize = 0,

    // ── Observations & journal (M2) ──────────────────────────

    /// User's acknowledgment journal (`acknowledgments.srf`). Loaded
    /// once on first activation; mutated by `ack` actions and
    /// re-saved atomically. Freed in `deinit`.
    journal: ?Journal = null,
    /// Joined view of `view.observations` + `journal`, rebuilt on
    /// every reload + ack. Owned by State; freed in `deinit` and
    /// `reload`.
    findings_view: ?observations_view.FindingsView = null,
    /// Whether already-acknowledged findings are rendered. Toggled
    /// by `toggle_show_acked` (`v` key by default).
    show_acked: bool = false,
    /// True while the observation panel has unresolved async
    /// checks. Set when `loadData` builds a view whose panel
    /// isn't complete; cleared by `tick` once every check
    /// resolves (at which point findings_view is rebuilt to
    /// include the late results). Keeps the per-frame tick poll
    /// cost at zero for the common all-sync case.
    observations_pending: bool = false,

    // ── Single-cursor (single-sheet) navigation ──────────────

    /// Unified row cursor over the virtual stream
    /// `[holdings rows | findings rows]`:
    ///   - indices `[0, view.rows.len)` → a holdings row.
    ///   - indices `[view.rows.len, view.rows.len + findings_view.rows.len)`
    ///     → a findings row, with local index `cursor - view.rows.len`.
    /// `j`/`k` advances by ±1 with wrap at both boundaries; the
    /// transition from "last holding" to "first finding" is just
    /// an increment in this index space, which is the entire
    /// point of the unified model.
    cursor: usize = 0,
    /// Currently expanded finding-row local index (or null when
    /// none is expanded). Indexed against `findings_view.rows`,
    /// NOT the unified cursor space. Only one finding can be
    /// expanded at a time.
    expanded_finding: ?usize = null,

    // ── Inline note-input state (M2 step 8b) ──────────────────

    /// Modal sub-state of the tab. Drives `handleKey` dispatch:
    /// `.normal` lets keystrokes flow through the global keymap;
    /// `.ack_note` swallows them via `handleKeyMulti` to build up
    /// the user's reasoning before Ctrl+Enter commits the ack.
    input_mode: InputMode = .normal,
    /// Multi-fragment input buffer for the current ack note.
    /// Populated by `handleKeyMulti`; flushed on Enter (one
    /// fragment per Enter) or Ctrl+Enter (commits the ack).
    // SAFETY: only read up to `note_len` bytes. `note_len` starts
    // at 0 (no reads), and `handleKeyMulti` only writes a byte
    // before incrementing `note_len`.
    note_buf: [512]u8 = undefined,
    note_len: usize = 0,
    /// Fragments of the in-progress ack note. Each is a heap-owned
    /// slice. Freed and cleared on commit / cancel.
    note_fragments: std.ArrayList([]u8) = .empty,
};

// ── Tab framework contract ────────────────────────────────────

pub const meta: framework.TabMeta(Action) = .{
    .label = "Review",
    .default_bindings = &.{
        .{ .action = .sort_col_next, .key = .{ .codepoint = '>' } },
        .{ .action = .sort_col_prev, .key = .{ .codepoint = '<' } },
        .{ .action = .sort_reverse, .key = .{ .codepoint = 'o' } },
        .{ .action = .toggle_expand, .key = .{ .codepoint = vaxis.Key.enter } },
        .{ .action = .ack, .key = .{ .codepoint = 'a' } },
        .{ .action = .unack, .key = .{ .codepoint = 'U' } },
        .{ .action = .toggle_show_acked, .key = .{ .codepoint = 'v' } },
        // 's' / space: select cursor row's symbol as App-active.
        // Mirrors portfolio_tab so muscle memory transfers.
        .{ .action = .select_symbol, .key = .{ .codepoint = 's' } },
        .{ .action = .select_symbol, .key = .{ .codepoint = vaxis.Key.space } },
        // Capital K: Vim-style "what is this" toggle. Same
        // binding as portfolio_tab so the gesture transfers.
        .{ .action = .toggle_overlay, .key = .{ .codepoint = 'K' } },
    },
    .action_labels = std.enums.EnumArray(Action, []const u8).init(.{
        .sort_col_next = "Sort: next column",
        .sort_col_prev = "Sort: previous column",
        .sort_reverse = "Sort: reverse direction",
        .toggle_expand = "Expand/collapse finding",
        .ack = "Acknowledge finding",
        .unack = "Un-acknowledge finding",
        .toggle_show_acked = "Toggle show acked findings",
        .select_symbol = "Select symbol",
        .toggle_overlay = "Show symbol details",
    }),
    .status_hints = &.{
        .ack,
        .unack,
        .toggle_show_acked,
        .select_symbol,
        .toggle_overlay,
        .sort_col_prev,
        .sort_col_next,
    },
};

/// Sort fields cycled through by `sort_col_next` / `sort_col_prev`,
/// in column-display order. The "default grouping" (null) is the
/// entry state and is reachable by cycling past the end of the array.
/// Sort fields cycled through by `sort_col_next` / `sort_col_prev`,
/// in column-display order (matches `col_order` below). Tax is the
/// last column visually, so it's the last cycle slot too — `<>` then
/// walks left-to-right across the visible columns the way the user
/// expects.
const sortable_fields = [_]review_view.SortField{
    .symbol,    .sector,    .weight,     .return_1y,
    .return_3y, .return_5y, .return_10y, .vol_3y,
    .vol_10y,   .sharpe_3y, .sharpe_10y, .maxdd_5y,
    .tax_pct,
};

pub const tab = struct {
    pub const ActionT = Action;
    pub const StateT = State;

    pub fn init(state: *State, app: *App) !void {
        _ = app;
        state.* = .{};
    }

    pub fn deinit(state: *State, app: *App) void {
        deinitState(state, app.allocator);
    }

    pub fn activate(state: *State, app: *App) !void {
        if (tab.isDisabled(app)) return;
        if (state.loaded) return;
        // Load the journal before building findings; `loadData`
        // joins both into `findings_view`.
        if (state.journal == null) loadJournal(state, app);
        loadData(state, app);
    }

    pub const deactivate = framework.noopDeactivate(State);

    /// Framework poll-tick hook: true while the observation panel
    /// has async checks in flight. Drives the App's poll timer so
    /// the `tick` hook below gets called without user input. The
    /// framework only asks the ACTIVE tab, so switching away
    /// pauses UI polling automatically (the checks keep running
    /// in the background; `tick` picks the results up when the
    /// user returns).
    pub fn wantsPollTick(state: *State, app: *App) bool {
        _ = app;
        return state.observations_pending;
    }

    /// Manual refresh: drops the cached view and re-builds. Also
    /// invalidates the shared `account_map` and `classification_map`
    /// on PortfolioData so the next load re-reads `accounts.srf` /
    /// `metadata.srf` from disk (the user may have edited either).
    /// The journal is reloaded too in case the user edited
    /// acknowledgments.srf out-of-band.
    pub fn reload(state: *State, app: *App) !void {
        if (state.view) |*v| v.deinit(app.allocator);
        state.view = null;
        if (state.findings_view) |*fv| fv.deinit(app.allocator);
        state.findings_view = null;
        state.loaded = false;
        app.portfolio.invalidateAccountMap();
        app.portfolio.invalidateClassificationMap();
        loadJournal(state, app);
        loadData(state, app);
    }

    /// Poll in-flight async observation checks. No-op (one bool
    /// test) unless `loadData` left the panel incomplete. When
    /// the last pending check resolves, rebuild the findings
    /// view so late findings appear, and clear the flag —
    /// `wantsPollTick` then answers false and the App's poll
    /// timer stops re-arming.
    ///
    /// Redraw cadence: while `wantsPollTick` answers true, the
    /// App keeps a 100ms vxfw Tick timer armed (see
    /// `typeErasedEventHandler`), so this hook gets called
    /// even when the user isn't generating events. Without
    /// that timer, async results would sit invisible until
    /// the next keypress.
    pub fn tick(state: *State, app: *App, frame: u64) void {
        _ = frame;
        if (!state.observations_pending) return;
        const view = if (state.view) |*v| v else return;
        const panel = if (view.observations) |*p| p else return;
        if (!panel.isComplete()) return;
        state.observations_pending = false;
        rebuildFindingsView(state, app);
    }

    pub fn handleAction(state: *State, app: *App, action: Action) void {
        switch (action) {
            .sort_col_next => {
                state.sort_field = nextSortField(state.sort_field);
                applySort(state);
            },
            .sort_col_prev => {
                state.sort_field = prevSortField(state.sort_field);
                applySort(state);
            },
            .sort_reverse => {
                state.sort_dir = if (state.sort_dir == .asc) .desc else .asc;
                applySort(state);
            },
            .toggle_expand => {
                if (cursorSection(state) != .findings) return;
                const fv = state.findings_view orelse return;
                if (fv.rows.len == 0) return;
                const local = cursorLocalIndex(state);
                if (state.expanded_finding == local) {
                    state.expanded_finding = null;
                } else {
                    state.expanded_finding = local;
                }
            },
            .ack => ackCurrentFinding(state, app),
            .unack => unackCurrentFinding(state, app),
            .toggle_show_acked => {
                state.show_acked = !state.show_acked;
                rebuildFindingsView(state, app);
            },
            .select_symbol => selectSymbolAtCursor(state, app),
            .toggle_overlay => toggleOverlayAtCursor(state, app),
        }
    }

    /// Review requires a loaded portfolio file and per-symbol
    /// allocations. Same gate as the analysis tab.
    pub fn isDisabled(app: *App) bool {
        return app.portfolio.file == null;
    }

    /// Drop cached findings on portfolio reload.
    ///
    /// `state.view` and `state.findings_view` hold pointers into
    /// the previous portfolio's memory (positions / allocation
    /// symbols), so they MUST be invalidated before the
    /// underlying data is freed. Cursors / expansion state get
    /// reset because they're row indices into the stale
    /// findings_view; preserving them risks pointing past the
    /// end of the rebuilt list.
    ///
    /// The journal is also reloaded — the user may have hand-
    /// edited `acknowledgments.srf` while the TUI was running.
    /// (Same rationale as the user-requested `reload` action.)
    ///
    /// Deliberately does NOT eager-rebuild, even when this tab
    /// is active. The reload broadcast fires BEFORE pd.reload
    /// resets the portfolio arena; rebuilding now would store
    /// borrowed pointers (e.g. ReviewRow.bucket borrowing from
    /// ClassificationEntry.bucket) that get freed seconds later
    /// when the arena resets, producing use-after-free crashes
    /// or visual corruption on the next draw. The orchestrator
    /// (portfolio_tab.reloadPortfolioFile) calls
    /// `app.loadTabData()` AFTER pd.reload completes, which
    /// re-activates the active tab against fresh data.
    pub fn onPortfolioReload(state: *State, app: *App) void {
        if (state.view) |*v| v.deinit(app.allocator);
        state.view = null;
        if (state.findings_view) |*fv| fv.deinit(app.allocator);
        state.findings_view = null;
        if (state.journal) |*j| j.deinit();
        state.journal = null;
        state.loaded = false;
        state.cursor = 0;
        state.expanded_finding = null;
    }

    /// Mouse handling: left-click on the column-header row sorts
    /// by that column. Re-clicking the active column flips
    /// direction; clicking a different column resets to its
    /// `defaultDir` (asc for symbol/sector, desc for numeric
    /// columns — best/worst-first matches the typical "show me
    /// the leaders" reading).
    ///
    /// Click on a holdings data row → cursor moves to that
    /// holding (unified index = local index). Click on a findings
    /// data row → cursor moves to that finding (unified index =
    /// holdings_len + local index). Clicks on expansion lines
    /// don't move the cursor.
    ///
    /// Wheel events fall through to App's scroll handling via
    /// `onWheelMove` returning false (see below).
    pub fn handleMouse(state: *State, app: *App, mouse: vaxis.Mouse) bool {
        if (mouse.button != .left) return false;
        if (mouse.type != .press) return false;
        const view = state.view orelse return false;
        const content_row = @as(usize, @intCast(mouse.row)) + app.scroll_offset;

        // Header row click ⇒ column-sort hit-test.
        if (content_row == state.header_row) {
            const click_col: usize = @intCast(mouse.col);
            if (!applyHeaderClick(state, click_col)) return false;
            applySort(state);
            return true;
        }

        // Holdings data-row click — set cursor to the unified
        // index for that holding (which equals the local
        // holdings index).
        if (content_row >= state.holdings_first_row and
            content_row < state.holdings_first_row + view.rows.len)
        {
            state.cursor = content_row - state.holdings_first_row;
            return true;
        }

        // Findings data-row click. Note: with expansion lines
        // mixed in, a click on an expansion line stays "on" the
        // finding above it (we don't move cursor in that case).
        if (state.findings_row_count > 0 and
            content_row >= state.findings_first_row and
            content_row < state.findings_first_row + state.findings_row_count)
        {
            const fv = state.findings_view orelse return true;
            // Walk forward from the first finding row, counting
            // logical findings until we either hit content_row or
            // exhaust the rows. This handles inline expansion: the
            // expansion lines belong to the finding above them.
            var visual_row = state.findings_first_row;
            for (fv.rows, 0..) |_, i| {
                const expansion = if (state.expanded_finding == i)
                    expansionLineCount(fv.rows[i]) + (if (state.expanded_finding == i) inputBarLineCount(state) else 0)
                else
                    0;
                const next_visual_row = visual_row + 1 + expansion;
                if (content_row >= visual_row and content_row < next_visual_row) {
                    state.cursor = view.rows.len + i;
                    // Clicking a different finding collapses any
                    // prior expansion. Mirrors `onCursorMove`'s
                    // single-expansion invariant.
                    if (state.expanded_finding) |exp| {
                        if (exp != i) state.expanded_finding = null;
                    }
                    return true;
                }
                visual_row = next_visual_row;
            }
            return true;
        }

        return false;
    }

    /// j/k/up/down/wheel advance the unified cursor by ±1 with
    /// wrap. Going down past the last holding lands on the first
    /// finding; going down past the last finding wraps to the
    /// first holding; going up from the first holding wraps to
    /// the last finding.
    ///
    /// Delta magnitude is clamped to ±1: keyboard always sends
    /// ±1 already; wheel sends ±3 per detent and we don't want
    /// it to skip rows. PageUp/PageDown go through the framework's
    /// viewport-scroll path, not here.
    ///
    /// Returns true on success so the framework consumes the
    /// event; returns false when both tables are empty so the
    /// framework falls through to viewport scroll.
    pub fn onCursorMove(state: *State, app: *App, delta: isize) bool {
        const total = cursorTotal(state);
        if (total == 0) return false;
        const step: isize = if (delta > 0) 1 else if (delta < 0) -1 else 0;
        state.cursor = wrapCursor(state.cursor, step, total);

        // Collapse a stale expansion when the cursor moves off the
        // expanded finding's row. Single-expansion-at-a-time
        // invariant; the user can re-expand with Enter.
        if (state.expanded_finding) |exp| {
            const local: ?usize = switch (cursorSection(state)) {
                .findings => cursorLocalIndex(state),
                else => null,
            };
            if (local == null or local.? != exp) state.expanded_finding = null;
        }

        ensureCursorVisible(state, &app.scroll_offset, app.visible_height);
        return true;
    }

    // No `onWheelMove`: by omitting it, wheel events fall through
    // to the framework's `onCursorMove` path. Wheel = j/k for the
    // user. Page-scrolling the viewport happens via PageUp /
    // PageDown / Home / End at the global key layer.

    /// Tab-local key handler. Runs before the global keymap so we
    /// can swallow keystrokes when the inline note-input bar is
    /// open. Returns true on consume; false to fall through to
    /// normal action dispatch.
    pub fn handleKey(state: *State, app: *App, key: vaxis.Key) bool {
        if (state.input_mode != .ack_note) return false;

        switch (input_buffer.handleKeyMulti(&state.note_buf, &state.note_len, key)) {
            .cancelled => {
                cancelAckNote(state, app);
                return true;
            },
            .fragment => {
                // Capture the fragment data BEFORE resetting len.
                const dup = app.allocator.dupe(u8, state.note_buf[0..state.note_len]) catch |err| {
                    app.setStatus("Note buffer alloc failed");
                    std.log.scoped(.review_tab).warn("frag dup failed: {s}", .{@errorName(err)});
                    return true;
                };
                state.note_fragments.append(app.allocator, dup) catch |err| {
                    app.allocator.free(dup);
                    app.setStatus("Note buffer alloc failed");
                    std.log.scoped(.review_tab).warn("frag append failed: {s}", .{@errorName(err)});
                    return true;
                };
                state.note_len = 0;
                return true;
            },
            .committed => {
                commitAckNote(state, app);
                return true;
            },
            .edited, .ignored => return true, // swallow even ignored so global keys don't fire mid-input
        }
    }
};

/// Pure-state header-click handler: maps `click_col` to a column,
/// updates `state.sort_field` / `state.sort_dir` per the
/// "re-click flips, new column resets to defaultDir" rule, and
/// returns true when a sort change should be applied. False means
/// the click landed on a gap, the prefix, or past the rightmost
/// column — caller should not invoke `applySort`.
///
/// Extracted from `handleMouse` so the sort-mutation logic can be
/// unit-tested without an `*App`.
fn applyHeaderClick(state: *State, click_col: usize) bool {
    const hit = hitTestHeader(click_col) orelse return false;
    const sf = hit.sortField() orelse return false;
    if (state.sort_field == sf) {
        state.sort_dir = state.sort_dir.flip();
    } else {
        state.sort_field = sf;
        state.sort_dir = hit.defaultDir();
    }
    return true;
}

// ── Sort cycling ──────────────────────────────────────────────

/// Cycle forward through `sortable_fields`, wrapping at the end.
fn nextSortField(curr: review_view.SortField) review_view.SortField {
    for (sortable_fields, 0..) |f, i| {
        if (f == curr) {
            const next_idx = (i + 1) % sortable_fields.len;
            return sortable_fields[next_idx];
        }
    }
    return sortable_fields[0]; // shouldn't happen — recover gracefully
}

/// Cycle backward through `sortable_fields`, wrapping at the start.
fn prevSortField(curr: review_view.SortField) review_view.SortField {
    for (sortable_fields, 0..) |f, i| {
        if (f == curr) {
            const prev_idx = if (i == 0) sortable_fields.len - 1 else i - 1;
            return sortable_fields[prev_idx];
        }
    }
    return sortable_fields[sortable_fields.len - 1];
}

fn applySort(state: *State) void {
    const view = &(state.view orelse return);
    review_view.sortRows(view.rows, state.sort_field, state.sort_dir);
}

/// Total length of the unified cursor space (holdings + findings).
/// Returns 0 when neither view is loaded or both are empty.
fn cursorTotal(state: *const State) usize {
    const holdings = if (state.view) |v| v.rows.len else 0;
    const findings = if (state.findings_view) |fv| fv.rows.len else 0;
    return holdings + findings;
}

/// Number of holdings rows. Convenience for cursor-section math.
fn holdingsLen(state: *const State) usize {
    return if (state.view) |v| v.rows.len else 0;
}

/// Which section the cursor is currently in. `.empty` means there
/// are no rows to point at.
fn cursorSection(state: *const State) CursorSection {
    const total = cursorTotal(state);
    if (total == 0) return .empty;
    return if (state.cursor < holdingsLen(state)) .holdings else .findings;
}

/// Local index of the cursor within its section. For holdings:
/// `cursor` itself. For findings: `cursor - holdings_len`. Caller
/// guarantees the cursor is in-range (cursorSection != .empty).
fn cursorLocalIndex(state: *const State) usize {
    const holdings = holdingsLen(state);
    return if (state.cursor < holdings) state.cursor else state.cursor - holdings;
}

/// Advance the unified cursor by `delta` with wrapping at both
/// boundaries. Holdings → findings is a normal increment in the
/// unified index space (no special-case); the wrap kicks in only
/// at index 0 (k from top wraps to last finding) and at total-1
/// (j from last finding wraps to first holding). Caller guarantees
/// `total > 0`.
fn wrapCursor(current: usize, delta: isize, total: usize) usize {
    std.debug.assert(total > 0);
    // Use signed arithmetic so a single big negative delta can wrap
    // through zero without underflow on the usize side.
    const total_i: isize = @intCast(total);
    var next: isize = @intCast(current);
    next += delta;
    next = @mod(next, total_i);
    // @mod with a negative operand can return a negative result
    // depending on the operand's sign; normalize.
    if (next < 0) next += total_i;
    return @intCast(next);
}

/// Adjust `scroll_offset` so the cursor row's visual position is
/// inside the viewport. Mirrors portfolio_tab's pattern: clamp
/// the offset up when the cursor sits above the viewport, clamp
/// it down when below.
///
/// The cursor's visual row is computed via `cursorVisualRow`,
/// which walks the rendered-row layout (header lines, holdings
/// rows, gap lines, findings rows + per-row expansions). Because
/// `holdings_first_row` and `findings_first_row` are recorded
/// during `buildStyledLines`, this function is only meaningful
/// after at least one render frame has run; on the first j/k
/// before any draw, both are zero and the offset stays at zero
/// (acceptable — the cursor IS at row zero anyway).
fn ensureCursorVisible(state: *const State, scroll_offset: *usize, visible_height: usize) void {
    const visual = cursorVisualRow(state) orelse return;
    if (visual < scroll_offset.*) {
        scroll_offset.* = visual;
        return;
    }
    if (visible_height > 0 and visual >= scroll_offset.* + visible_height) {
        scroll_offset.* = visual - visible_height + 1;
    }
}

/// Compute the cursor's content-area row index (0 = first
/// rendered line). Returns null when neither view is loaded or
/// when the section indices haven't been populated yet (first
/// frame). Findings rows account for any inline expansion of
/// preceding findings so the cursor lands on the actual logical
/// finding, not on its expanded detail.
fn cursorVisualRow(state: *const State) ?usize {
    const section = cursorSection(state);
    if (section == .empty) return null;
    const local = cursorLocalIndex(state);
    switch (section) {
        .empty => return null,
        .holdings => {
            // holdings_first_row is the visual row of holdings[0].
            return state.holdings_first_row + local;
        },
        .findings => {
            const fv = state.findings_view orelse return null;
            // findings_first_row is the visual row of findings[0],
            // populated only when there's a non-empty findings
            // section. Add per-prior-finding expansion lines if
            // any of them are currently expanded.
            if (state.findings_first_row == 0) return null;
            var visual: usize = state.findings_first_row;
            for (fv.rows[0..local], 0..) |row, i| {
                visual += 1; // the row itself
                if (state.expanded_finding == i) {
                    visual += expansionLineCount(row);
                    visual += inputBarLineCount(state);
                }
            }
            return visual;
        },
    }
}

/// Number of *expansion* lines a single finding contributes when
/// expanded, in addition to the row itself. Used by `handleMouse`
/// to map clicks back to logical finding indices when expansion
/// lines are interleaved.
fn expansionLineCount(row: observations_view.FindingRow) usize {
    // Layout matches `appendExpansionLines`: always one detail line.
    // Plus one ack-date line + one note line per fragment when acked.
    var count: usize = 1; // detail line
    if (row.ack_entry) |entry| {
        count += 1; // ack date line
        count += entry.notes.len;
    }
    return count;
}

/// Number of input-bar lines contributed when the inline note-input
/// modal is open. Layout matches `appendInputBarLines`: one line per
/// completed fragment + one in-progress line + one hint line.
fn inputBarLineCount(state: *const State) usize {
    if (state.input_mode != .ack_note) return 0;
    return state.note_fragments.items.len + 2;
}

/// Acknowledge the cursor-selected finding. Enters note-input
/// mode: the user types their reasoning (Enter completes a
/// fragment, Ctrl+Enter commits all fragments + writes the ack,
/// Esc cancels). Pressing Ctrl+Enter immediately produces an ack
/// with no notes — supported intentionally so users who don't
/// want to write reasoning can dismiss findings quickly.
fn ackCurrentFinding(state: *State, app: *App) void {
    if (cursorSection(state) != .findings) {
        app.setStatus("Move cursor to a finding before acknowledging");
        return;
    }
    const fv = state.findings_view orelse return;
    if (fv.rows.len == 0) return;
    const local = cursorLocalIndex(state);
    if (local >= fv.rows.len) return;

    const row = fv.rows[local];
    if (row.is_acked) {
        app.setStatus("Already acknowledged (press U to un-acknowledge)");
        return;
    }

    // Auto-expand so the inline input bar has somewhere to render.
    state.expanded_finding = local;
    state.input_mode = .ack_note;
    state.note_len = 0;
    // `note_fragments` should already be empty (we only enter
    // input mode from `.normal`, and `commitAckNote`/`cancelAckNote`
    // both clear it on exit). Defensive: clear anyway.
    clearNoteFragments(state, app.allocator);
    app.setStatus("Type reasoning. Enter = next line. Ctrl+Enter = save. Esc = cancel.");
}

/// Un-acknowledge the cursor-selected finding. Flips the journal
/// entry's state back to `.active` and records an
/// `unacknowledged_at` breadcrumb. Single-keystroke action — no
/// reasoning text required (the breadcrumb itself is the audit
/// trail).
fn unackCurrentFinding(state: *State, app: *App) void {
    if (cursorSection(state) != .findings) {
        app.setStatus("Move cursor to a finding before un-acknowledging");
        return;
    }
    const fv = state.findings_view orelse return;
    if (fv.rows.len == 0) return;
    const local = cursorLocalIndex(state);
    if (local >= fv.rows.len) return;

    const row = fv.rows[local];
    if (!row.is_acked) {
        app.setStatus("Not acknowledged (press a to acknowledge)");
        return;
    }

    const journal = if (state.journal) |*j| j else {
        app.setStatus("Journal not loaded");
        return;
    };

    const path = journalPath(app) orelse {
        app.setStatus("No portfolio anchor");
        return;
    };
    defer app.allocator.free(path);

    journal.setState(app.io, path, row.kind, row.target, .active, app.today) catch |err| {
        app.setStatus("Un-acknowledgment failed");
        std.log.scoped(.review_tab).warn("journal.setState failed: {s}", .{@errorName(err)});
        return;
    };

    rebuildFindingsView(state, app);
    app.setStatus("Un-acknowledged");
}

/// Select the cursor row's symbol as the App-active symbol. For
/// holdings rows the symbol comes from `view.rows[local].symbol`;
/// for findings rows it comes from `findings_view.rows[local].target`
/// (which is a symbol for per-symbol findings; for sector-level
/// findings the target is a sector label, which still gets passed
/// through but won't match anything per-symbol downstream — a
/// harmless no-op).
fn selectSymbolAtCursor(state: *State, app: *App) void {
    const symbol: []const u8 = switch (cursorSection(state)) {
        .empty => return,
        .holdings => blk: {
            const view = state.view orelse return;
            const local = cursorLocalIndex(state);
            if (local >= view.rows.len) return;
            break :blk view.rows[local].symbol;
        },
        .findings => blk: {
            const fv = state.findings_view orelse return;
            const local = cursorLocalIndex(state);
            if (local >= fv.rows.len) return;
            break :blk fv.rows[local].target;
        },
    };
    app.setActiveSymbol(symbol);
    var tmp_buf: [256]u8 = undefined;
    const msg = std.fmt.bufPrint(&tmp_buf, "Active: {s}", .{symbol}) catch "Active";
    app.setStatus(msg);
}

/// Resolve the cursor row's symbol and call `app.toggleOverlay`.
/// Empty cursor section → empty symbol → toggleOverlay closes
/// any open overlay. For findings rows, the target field doubles
/// as the symbol (per-symbol findings) or as a non-symbol label
/// (sector findings) — the latter makes the overlay show "no
/// metadata.srf entry" for the bogus key, which is fine.
fn toggleOverlayAtCursor(state: *State, app: *App) void {
    const symbol: []const u8 = switch (cursorSection(state)) {
        .empty => "",
        .holdings => blk: {
            const view = state.view orelse break :blk "";
            const local = cursorLocalIndex(state);
            if (local >= view.rows.len) break :blk "";
            break :blk view.rows[local].symbol;
        },
        .findings => blk: {
            const fv = state.findings_view orelse break :blk "";
            const local = cursorLocalIndex(state);
            if (local >= fv.rows.len) break :blk "";
            break :blk fv.rows[local].target;
        },
    };
    app.toggleOverlay(symbol);
}

/// Free `state.note_fragments` items + the slice. Idempotent.
fn clearNoteFragments(state: *State, allocator: std.mem.Allocator) void {
    for (state.note_fragments.items) |frag| allocator.free(frag);
    state.note_fragments.clearAndFree(allocator);
}

/// Cancel the in-progress ack note. Returns to `.normal` mode and
/// clears all input state.
fn cancelAckNote(state: *State, app: *App) void {
    state.input_mode = .normal;
    state.note_len = 0;
    clearNoteFragments(state, app.allocator);
    // No setStatus: the disappearing input bar is its own feedback.
}

/// Commit the in-progress ack note: dupe any final unflushed
/// fragment, write the journal entry, rebuild the findings view,
/// and reset input state.
fn commitAckNote(state: *State, app: *App) void {
    defer {
        state.input_mode = .normal;
        state.note_len = 0;
        clearNoteFragments(state, app.allocator);
    }

    const fv = state.findings_view orelse return;
    if (cursorSection(state) != .findings) return;
    const local = cursorLocalIndex(state);
    if (fv.rows.len == 0 or local >= fv.rows.len) return;
    const row = fv.rows[local];

    const journal = if (state.journal) |*j| j else {
        app.setStatus("Journal not loaded");
        return;
    };

    const path = journalPath(app) orelse {
        app.setStatus("No portfolio anchor");
        return;
    };
    defer app.allocator.free(path);

    // Flush trailing unfinished fragment, if any.
    if (state.note_len > 0) {
        const dup = app.allocator.dupe(u8, state.note_buf[0..state.note_len]) catch |err| {
            app.setStatus("Note buffer alloc failed");
            std.log.scoped(.review_tab).warn("note dup failed: {s}", .{@errorName(err)});
            return;
        };
        state.note_fragments.append(app.allocator, dup) catch |err| {
            app.allocator.free(dup);
            app.setStatus("Note buffer alloc failed");
            std.log.scoped(.review_tab).warn("note append failed: {s}", .{@errorName(err)});
            return;
        };
    }

    // Build a `[]const []const u8` view over the fragments for
    // `journal.append`. The journal dupes them again internally,
    // so our local strings are freed normally by `clearNoteFragments`.
    const fragments_view = state.note_fragments.items;

    journal.append(
        app.io,
        path,
        .{
            .observation = row.kind,
            .target = row.target,
            .acknowledged_at = app.today,
            .state = .acknowledged,
        },
        fragments_view,
    ) catch |err| {
        app.setStatus("Acknowledgment failed");
        std.log.scoped(.review_tab).warn("journal.append failed: {s}", .{@errorName(err)});
        return;
    };

    rebuildFindingsView(state, app);
    // No setStatus on success: the visible removal of the row
    // from the findings list (or the "[acked]" prefix when
    // show_acked is on) is feedback enough. Leaving the prior
    // help/hint visible is the user's preference.
}

// ── Data loading ──────────────────────────────────────────────

fn loadData(state: *State, app: *App) void {
    state.loaded = true;
    // Sync data is populated by pd.load at App init.
    const pf = app.portfolio.file orelse return;
    const summary_ptr = if (app.portfolio.summary) |*s| s else return;

    // Tier 1 accessors block on their respective worker futures.
    // Review needs candles, dividends, classification, and the
    // account map; this is the first tab where all four workers'
    // costs may show up as visible wait. Most users will hit
    // portfolio first (where snapshots blocks if needed) before
    // navigating to review, so the candles + dividends workers
    // are usually done by the time we get here.
    const candle_map = app.portfolio.candles() orelse {
        app.setStatus("Portfolio data not loaded");
        return;
    };
    const dividend_map = app.portfolio.dividends();
    const acct_map_opt: ?zfin.analysis.AccountMap = if (app.portfolio.accountMap()) |amp| amp.* else null;
    const cm_ptr = app.portfolio.classificationMap() orelse {
        app.setStatus("No metadata.srf found. Run: zfin enrich <portfolio.srf> > metadata.srf");
        return;
    };

    if (state.view) |*v| v.deinit(app.allocator);
    state.view = review_view.buildReview(
        app.allocator,
        app.io,
        summary_ptr.*,
        candle_map,
        dividend_map,
        pf,
        cm_ptr.*,
        acct_map_opt,
        app.today,
        app.anchorPath() orelse "",
    ) catch {
        app.setStatus("Error computing review");
        return;
    };

    applySort(state);
    rebuildFindingsView(state, app);

    // If any check is still running (async dispatch), flag it so
    // the findings view refreshes when results land.
    // `isComplete` also transitions newly-finished checks, so a
    // fast async check that finished during buildReview is
    // caught right here without waiting for the next frame.
    // While the flag is set, `wantsPollTick` answers true and
    // the App keeps a vxfw Tick timer armed so the poll runs
    // even with no user input.
    if (state.view) |*v| {
        if (v.observations) |*panel| {
            state.observations_pending = !panel.isComplete();
        }
    }
}

/// Resolve the absolute path to `acknowledgments.srf` next to the
/// portfolio file. Returns null when no portfolio is loaded.
/// Caller owns the returned slice.
fn journalPath(app: *App) ?[]const u8 {
    const ppath = app.anchorPath() orelse return null;
    const dir_end = if (std.mem.lastIndexOfScalar(u8, ppath, std.fs.path.sep)) |idx| idx + 1 else 0;
    return std.fmt.allocPrint(app.allocator, "{s}acknowledgments.srf", .{ppath[0..dir_end]}) catch null;
}

/// Load the journal from disk. Missing file ⇒ empty journal (first
/// run case). Parse error ⇒ empty journal + error status; the user
/// will see the bad file but actions won't crash. Existing
/// `state.journal` is freed first.
fn loadJournal(state: *State, app: *App) void {
    if (state.journal) |*j| j.deinit();
    state.journal = null;

    const path = journalPath(app) orelse {
        // No portfolio anchor — synthesize an empty journal so
        // ack-flow code can rely on `state.journal` being non-null
        // once `loadData` runs.
        const empty_entries = app.allocator.alloc(Journal.Entry, 0) catch return;
        state.journal = .{ .allocator = app.allocator, .entries = empty_entries };
        return;
    };
    defer app.allocator.free(path);

    state.journal = Journal.load(app.allocator, app.io, path) catch |err| {
        // Surface the specific error so the user can fix the file.
        // Using a stack buffer because setStatus copies into App's
        // own fixed buffer; we don't need a heap allocation.
        var msg_buf: [128]u8 = undefined;
        const msg = std.fmt.bufPrint(
            &msg_buf,
            "acknowledgments.srf: {s}",
            .{@errorName(err)},
        ) catch "acknowledgments.srf: parse error";
        app.setStatus(msg);
        // Synthesize empty so ack flow remains usable.
        const empty_entries = app.allocator.alloc(Journal.Entry, 0) catch return;
        state.journal = .{ .allocator = app.allocator, .entries = empty_entries };
        return;
    };
}

/// Rebuild `findings_view` from `state.view.observations` and
/// `state.journal`. Frees any prior findings_view first. Both inputs
/// must be present; if either is missing, leaves `findings_view = null`.
///
/// Always clears `expanded_finding`: any expansion held across a
/// rebuild is unsafe because row indices may have shifted (acking
/// removes a row; `show_acked` toggle adds/removes rows). The user
/// can re-expand with Enter.
fn rebuildFindingsView(state: *State, app: *App) void {
    if (state.findings_view) |*fv| fv.deinit(app.allocator);
    state.findings_view = null;
    state.expanded_finding = null;

    const view = state.view orelse return;
    const panel = if (view.observations) |*p| p else return;
    const journal = if (state.journal) |*j| j else return;

    state.findings_view = observations_view.build(app.allocator, panel, journal, state.show_acked) catch |err| {
        std.log.scoped(.review_tab).warn("findings build failed: {s}", .{@errorName(err)});
        return;
    };

    // Clamp the unified cursor in case `show_acked` toggling or
    // an ack shrunk the row count beneath the previous cursor
    // position. The cursor's holdings range is unaffected; only
    // the findings tail can shrink.
    const total = cursorTotal(state);
    if (total == 0) {
        state.cursor = 0;
    } else if (state.cursor >= total) {
        state.cursor = total - 1;
    }
}

/// State teardown that doesn't require an App. Lets tests exercise
/// the cleanup path directly under `testing.allocator`.
pub fn deinitState(state: *State, allocator: std.mem.Allocator) void {
    if (state.view) |*v| v.deinit(allocator);
    if (state.findings_view) |*fv| fv.deinit(allocator);
    if (state.journal) |*j| j.deinit();
    for (state.note_fragments.items) |frag| allocator.free(frag);
    state.note_fragments.deinit(allocator);
    state.* = .{};
}

// ── Rendering ─────────────────────────────────────────────────

/// Column widths in display-column order. Tax% is LAST: it's a
/// contextual hint, not a primary metric, and shouldn't anchor the
/// eye. The first numeric column the reader sees is Wt% (how big is
/// this position?), which is the right anchor for "what am I
/// looking at."
const col_symbol: usize = 8;
const col_sector: usize = 20;
const col_weight: usize = 7;
const col_pct: usize = 8;
const col_sharpe: usize = 8;
const col_maxdd: usize = 10;
const col_tax: usize = 7;

/// Display-column header tag for each column, used by the header
/// renderer, the click hit-test, and (via `sortField`) the sort
/// dispatcher when the user clicks a column header.
const Col = enum {
    symbol,
    sector,
    weight,
    return_1y,
    return_3y,
    return_5y,
    return_10y,
    vol_3y,
    vol_10y,
    sharpe_3y,
    sharpe_10y,
    maxdd_5y,
    tax,

    fn width(self: Col) usize {
        return switch (self) {
            .symbol => col_symbol,
            .sector => col_sector,
            .weight => col_weight,
            .return_1y, .return_3y, .return_5y, .return_10y => col_pct,
            .vol_3y, .vol_10y => col_pct,
            .sharpe_3y, .sharpe_10y => col_sharpe,
            .maxdd_5y => col_maxdd,
            .tax => col_tax,
        };
    }

    fn header(self: Col) []const u8 {
        return switch (self) {
            .symbol => "Symbol",
            .sector => "Sector",
            .weight => "Wt%",
            .return_1y => "1Y",
            .return_3y => "3Y",
            .return_5y => "5Y",
            .return_10y => "10Y",
            .vol_3y => "3Y-Vol",
            .vol_10y => "10Y-Vol",
            .sharpe_3y => "3Y-SR",
            .sharpe_10y => "10Y-SR",
            .maxdd_5y => "5Y-MaxDD",
            .tax => "Tax%",
        };
    }

    /// Map this column to the corresponding view-level SortField.
    /// All review columns are sortable today, but the function
    /// returns an optional so future "non-sortable" columns
    /// (e.g. an action button) can decline.
    fn sortField(self: Col) ?review_view.SortField {
        return switch (self) {
            .symbol => .symbol,
            .sector => .sector,
            .weight => .weight,
            .return_1y => .return_1y,
            .return_3y => .return_3y,
            .return_5y => .return_5y,
            .return_10y => .return_10y,
            .vol_3y => .vol_3y,
            .vol_10y => .vol_10y,
            .sharpe_3y => .sharpe_3y,
            .sharpe_10y => .sharpe_10y,
            .maxdd_5y => .maxdd_5y,
            .tax => .tax_pct,
        };
    }

    /// Default sort direction for this column when freshly
    /// selected. String columns sort ascending (alphabetical);
    /// numeric columns sort descending (best first). MaxDD is the
    /// odd one — descending puts the WORST drawdowns first, which
    /// is what the user actually wants ("show me the most-bruised
    /// holdings"). Vol same logic.
    fn defaultDir(self: Col) review_view.SortDirection {
        return switch (self) {
            .symbol, .sector => .asc,
            else => .desc,
        };
    }
};

/// Order in which columns appear in the rendered row.
const col_order = [_]Col{
    .symbol,     .sector,
    .weight,     .return_1y,
    .return_3y,  .return_5y,
    .return_10y, .vol_3y,
    .vol_10y,    .sharpe_3y,
    .sharpe_10y, .maxdd_5y,
    .tax,
};

/// Bytes consumed by `RowBuilder.prefix` at the start of every
/// row. Click hit-testing offsets by this amount.
const row_prefix_cols: usize = 2;

/// Map a click column (`mouse.col`, screen columns from the left)
/// onto a `Col` value, accounting for the row prefix and the
/// single-space gap between columns. Returns null when the click
/// lands in the prefix, on a gap, or past the right edge of the
/// last column.
fn hitTestHeader(click_col: usize) ?Col {
    if (click_col < row_prefix_cols) return null;
    var pos = row_prefix_cols;
    inline for (col_order, 0..) |col, idx| {
        if (idx > 0) {
            // Gap column: clicks here are unambiguously between
            // two columns. Treat them as "no hit" rather than
            // surprising the user by attributing them to either
            // neighbor.
            if (click_col == pos) return null;
            pos += 1;
        }
        const col_end = pos + col.width();
        if (click_col >= pos and click_col < col_end) return col;
        pos = col_end;
    }
    return null;
}

/// Builds a row's text + per-cell style spans simultaneously. Each
/// `cell()` call appends one cell (padded to its target display width)
/// and records a span for any non-default intent.
const RowBuilder = struct {
    arena: std.mem.Allocator,
    th: theme.Theme,
    text: std.ArrayList(u8) = .empty,
    spans: std.ArrayList(StyleSpan) = .empty,
    /// Display column the next cell will start at. Updated by
    /// `cell()` after each append.
    col: usize = 0,

    /// Append the leading row-prefix indent. `active` switches
    /// the marker between "  " (inactive) and "* " (the App-
    /// active symbol). Two display columns either way, so
    /// `row_prefix_cols` is unchanged.
    fn prefix(self: *RowBuilder, active: bool) !void {
        const marker: []const u8 = if (active) "* " else "  ";
        try self.text.appendSlice(self.arena, marker);
        self.col += 2;
    }

    /// Append a padded cell with content `s` (already-formatted text)
    /// at display width `w`. Alignment: right (numeric / dash cells).
    /// Records a style span if `intent` resolves to a non-default
    /// style (positive/negative/warning/muted).
    fn cellRight(self: *RowBuilder, s: []const u8, w: usize, intent: fmt.StyleIntent) !void {
        var pad_buf: [64]u8 = undefined;
        const padded = fmt.padLeftToCols(&pad_buf, s, w);
        try self.appendCell(padded, w, intent);
    }

    /// Append a left-aligned padded cell (used for symbol, sector
    /// labels). Pads with trailing spaces. Truncates input to width
    /// when oversize. Display-column aware on the input length so
    /// abbreviated multibyte sectors align correctly.
    fn cellLeft(self: *RowBuilder, s: []const u8, w: usize, intent: fmt.StyleIntent) !void {
        const trimmed = fmt.truncateToCols(s, w);
        var pad_buf: [128]u8 = undefined;
        const have_cols = fmt.displayCols(trimmed);
        const pad_cols = if (have_cols < w) w - have_cols else 0;
        // Layout: [content bytes][pad_cols spaces].
        if (trimmed.len + pad_cols > pad_buf.len) {
            try self.appendCell(trimmed, w, intent);
            return;
        }
        @memcpy(pad_buf[0..trimmed.len], trimmed);
        @memset(pad_buf[trimmed.len .. trimmed.len + pad_cols], ' ');
        try self.appendCell(pad_buf[0 .. trimmed.len + pad_cols], w, intent);
    }

    /// Append a single-space gap between cells.
    fn gap(self: *RowBuilder) !void {
        try self.text.append(self.arena, ' ');
        self.col += 1;
    }

    fn appendCell(
        self: *RowBuilder,
        padded: []const u8,
        w: usize,
        intent: fmt.StyleIntent,
    ) !void {
        const start = self.col;
        try self.text.appendSlice(self.arena, padded);
        // Trust that `padded` is exactly `w` display columns wide
        // (every caller guarantees that via padLeft/padRight).
        const end = start + w;
        self.col = end;
        if (intent != .normal) {
            try self.spans.append(self.arena, .{
                .start = start,
                .end = end,
                .style = self.th.styleFor(intent),
            });
        }
    }

    fn build(self: *RowBuilder, base_style: vaxis.Style) !StyledLine {
        const text = try self.text.toOwnedSlice(self.arena);
        const spans = try self.spans.toOwnedSlice(self.arena);
        return .{
            .text = text,
            .style = base_style,
            .spans = if (spans.len > 0) spans else null,
        };
    }
};

pub fn buildStyledLines(state: *State, app: *App, arena: std.mem.Allocator) ![]const StyledLine {
    var lines: std.ArrayList(StyledLine) = .empty;
    const th = app.theme;

    try lines.append(arena, .{ .text = "", .style = th.contentStyle() });
    try lines.append(arena, .{ .text = "  Portfolio Review", .style = th.headerStyle() });

    const view = state.view orelse {
        try lines.append(arena, .{ .text = "", .style = th.contentStyle() });
        try lines.append(arena, .{ .text = "  No data. Load a portfolio with -p <path>.", .style = th.mutedStyle() });
        return lines.toOwnedSlice(arena);
    };

    try lines.append(arena, .{
        .text = try std.fmt.allocPrint(arena, "  As of {f}    Liquid: {f}    Holdings: {d}", .{
            view.as_of, Money.from(view.total_liquid), view.rows.len,
        }),
        .style = th.mutedStyle(),
    });
    try lines.append(arena, .{ .text = "", .style = th.contentStyle() });

    // Status grid: per-check pass/warn/flag glyphs at the top of
    // the tab, before the holdings table. Gives the user an
    // at-a-glance "what's wrong" before they scan rows. Rendered
    // even when zero findings (the all-✅ row IS the signal).
    if (view.observations) |panel| {
        try appendStatusGrid(arena, &lines, panel, th);
        try lines.append(arena, .{ .text = "", .style = th.contentStyle() });
    }

    // Header row — purple+bold (`headerStyle`) with sort indicators
    // on the active column. Record the row index so `handleMouse`
    // can detect column-header clicks for click-to-sort.
    state.header_row = lines.items.len;
    try lines.append(arena, try buildHeaderLine(arena, th, state.sort_field, state.sort_dir));

    // Sort-status indicator (so user sees what they're sorted by).
    const sort_label = sortFieldLabel(state.sort_field);
    const dir_arrow: []const u8 = if (state.sort_dir == .asc) "↑" else "↓";
    try lines.append(arena, .{
        .text = try std.fmt.allocPrint(arena, "  Sort: {s} {s}    [<] prev  [>] next  [o] reverse", .{
            sort_label, dir_arrow,
        }),
        .style = th.dimStyle(),
    });

    // Rows. Highlight the row under the unified cursor when the
    // cursor is in the holdings section. Mark the App-active
    // symbol's row with a `*` prefix (mirrors portfolio_tab) so
    // selection has a visible affordance separate from the
    // cursor highlight.
    state.holdings_first_row = lines.items.len;
    const cursor_in_holdings = cursorSection(state) == .holdings;
    const cursor_local = if (cursor_in_holdings) cursorLocalIndex(state) else 0;
    for (view.rows, 0..) |r, i| {
        const is_cursor = cursor_in_holdings and i == cursor_local;
        const is_active = app.symbol.len > 0 and std.mem.eql(u8, r.symbol, app.symbol);
        try lines.append(arena, try formatRow(arena, th, r, is_cursor, is_active));
    }

    // Totals separator.
    try lines.append(arena, try buildSeparatorLine(arena, th));

    // Totals row.
    try lines.append(arena, try formatTotalsRow(arena, th, view.totals));

    if (anyReweightFlag(view.totals.reweight_flags)) {
        try lines.append(arena, .{ .text = "", .style = th.contentStyle() });
        try lines.append(arena, .{
            .text = "  * Reweighted: at least one holding lacked full-window candle coverage.",
            .style = th.mutedStyle(),
        });
        try lines.append(arena, .{
            .text = "    Affected metrics renormalized weights across participating holdings.",
            .style = th.mutedStyle(),
        });
    }

    // Findings section. Rendered below the totals row + reweight
    // footer. M2 step 8a: simple severity-glyph + text rows with
    // cursor highlight; expansion + inline note input land in 8b.
    if (state.findings_view) |fv| {
        try appendFindingsSection(arena, &lines, state, fv, th, app.symbol);
    }

    return lines.toOwnedSlice(arena);
}

/// Glyph used in the findings table's severity column. Each is two
/// display columns wide (emoji-presentation) so renderers don't have
/// to width-correct.
/// Per-finding-row glyph indicating severity. Each glyph string
/// includes a trailing U+FE0F variation selector to force emoji
/// presentation. The variation selector also serves a critical
/// rendering role here: `drawStyledContent` allocates one buffer
/// cell per UTF-8 sequence (advancing col by 1), so a single-
/// codepoint emoji like ❌ takes 1 buffer cell while terminals
/// render it as 2 visual cols, desyncing the col counter from
/// terminal display. Appending FE0F gives it a second codepoint
/// (which gets its own cell) so buffer-col advancement matches
/// terminal-col advancement at exactly 2 per emoji.
fn severityGlyph(sev: observations.Severity) []const u8 {
    return switch (sev) {
        .warn => "⚠️", // U+26A0 + FE0F
        .flag => "❌\u{FE0F}",
        .err => "🛑\u{FE0F}",
    };
}

/// Glyph for an entire check's state — what shows in the status
/// grid at the top of the tab. `pass` and `skipped` get distinct
/// glyphs (not just "no glyph") because users want to see "yes I
/// ran every check, here's why each one is OK".
///
/// Pending is reserved for the future async dispatch path; today
/// every check is sync so we never produce it. Keeping the case
/// in the renderer means the renderer is ready when the async
/// path lands.
/// Per-check status-grid glyph. See `severityGlyph` for the
/// FE0F-trailing convention — it forces emoji presentation and
/// gives the renderer a second cell to track so buffer-col
/// advancement matches terminal-col advancement.
fn checkStatusGlyph(result: observations.CheckResult) []const u8 {
    return switch (result) {
        .pass => "✅\u{FE0F}",
        .warn => "⚠️", // U+26A0 + FE0F (already 2 codepoints)
        .flag => "❌\u{FE0F}",
        .skipped => "➖\u{FE0F}",
        .err => "🛑\u{FE0F}",
    };
}

/// Theme style for a status-grid cell. Mirrors finding-row styling
/// so the status grid and the findings table feel cohesive.
fn checkStatusStyle(th: theme.Theme, result: observations.CheckResult) vaxis.Style {
    return switch (result) {
        .pass => th.mutedStyle(),
        .warn => th.warningStyle(),
        .flag, .err => th.negativeStyle(),
        .skipped => th.mutedStyle(),
    };
}

/// Width of one status-grid cell's label component, in display
/// columns. Sized to fit the longest registered check label.
/// Currently "Position concentration" at 22 chars is the longest;
/// "Sector concentration" (20), "Drift since last view" (21), and
/// "Sector dominance" (16) are all shorter.
const status_label_cols: usize = 22;

/// Number of cells per row in the status grid. 3 is the safe
/// default for ~110-column terminals; 4 fits if the terminal is
/// wider but we'd need to plumb terminal width through, which
/// hasn't been worth the layering yet. 3-up is what the design
/// committed to as the v1 baseline.
const status_cells_per_row: usize = 3;

/// Append the per-check status grid to `lines`. One row per
/// `status_cells_per_row` checks, each cell laid out as
/// "<right-padded label> <glyph>". Cells separated by 2 spaces.
///
/// Style: each cell takes its severity's color (warn = yellow,
/// flag/err = red, pass/skipped = muted). Because StyledLine is
/// one-style-per-line, every cell on the line gets the same
/// style. To preserve per-cell color, we emit one StyledLine
/// PER CELL — visually still on the same row because vaxis
/// concatenates lines that don't span the full width...
///
/// Actually no, each StyledLine takes its own row. So we need to
/// pre-pick one style per row by promoting the worst severity in
/// that row's cells. That keeps the line count bounded and each
/// row's color signals "this row contains at least one warn /
/// flag" without per-cell colors.
fn appendStatusGrid(
    arena: std.mem.Allocator,
    lines: *std.ArrayList(StyledLine),
    panel: observations.CheckPanel,
    th: theme.Theme,
) !void {
    if (panel.pending.len == 0) return;

    var i: usize = 0;
    while (i < panel.pending.len) {
        const end = @min(i + status_cells_per_row, panel.pending.len);

        // Promote the row's worst-severity color so multi-cell rows
        // with mixed states still draw the user's eye to the bad
        // ones. Order from best to worst: pass < skipped < warn <
        // flag/err. Pending checks rank with pass (no news isn't
        // bad news yet).
        var row_style = th.mutedStyle();
        var worst: u8 = 0; // 0=pass/skipped/pending, 1=warn, 2=flag/err
        for (panel.pending[i..end]) |pc| {
            const result = switch (pc.state) {
                .complete => |r| r,
                .pending => continue,
            };
            const rank: u8 = switch (result) {
                .pass, .skipped => 0,
                .warn => 1,
                .flag, .err => 2,
            };
            if (rank > worst) {
                worst = rank;
                row_style = checkStatusStyle(th, result);
            }
        }

        var text: std.ArrayList(u8) = .empty;
        try text.appendSlice(arena, "  ");
        for (panel.pending[i..end], 0..) |pc, col| {
            if (col > 0) try text.appendSlice(arena, "  ");
            const result: ?observations.CheckResult = switch (pc.state) {
                .complete => |r| r,
                .pending => null,
            };
            try appendStatusCell(arena, &text, pc.check.label, result);
        }
        try lines.append(arena, .{ .text = try text.toOwnedSlice(arena), .style = row_style });

        i = end;
    }
}

/// Append one cell's bytes to `text`: right-padded label + space
/// + glyph. With ASCII glyphs (always 1 col), the cell ends at
/// exactly `status_cell_cols` display columns. A `null` result
/// means the check is still running (async dispatch in flight);
/// rendered as an hourglass.
fn appendStatusCell(
    arena: std.mem.Allocator,
    text: *std.ArrayList(u8),
    label: []const u8,
    result: ?observations.CheckResult,
) !void {
    const lbl_cols = label.len; // ASCII labels: byte count == display cols
    if (lbl_cols < status_label_cols) {
        try text.appendNTimes(arena, ' ', status_label_cols - lbl_cols);
    }
    try text.appendSlice(arena, label);
    try text.append(arena, ' ');
    if (result) |r| {
        try text.appendSlice(arena, checkStatusGlyph(r));
    } else {
        try text.appendSlice(arena, "⏳\u{FE0F}");
    }
}

/// Append the findings section to `lines`. Layout:
///
///     <blank line>
///     Findings (N active, M acked)
///     <separator>
///     <glyph> <text>          ← per row
///     <glyph> <text>          ←  acked rows shown only when state.show_acked
///
/// Cursor highlight: when the unified `state.cursor` falls inside
/// the findings section and resolves to this row's local index,
/// the row gets the theme's selected-style applied.
fn appendFindingsSection(
    arena: std.mem.Allocator,
    lines: *std.ArrayList(StyledLine),
    state: *State,
    fv: observations_view.FindingsView,
    th: theme.Theme,
    active_symbol: []const u8,
) !void {
    try lines.append(arena, .{ .text = "", .style = th.contentStyle() });

    const heading = try std.fmt.allocPrint(
        arena,
        "  Findings ({d} active, {d} acked, {d} resolved){s}",
        .{
            fv.total_active,
            fv.total_acked,
            fv.total_resolved,
            if (state.show_acked) "  [showing acked]" else "",
        },
    );
    try lines.append(arena, .{ .text = heading, .style = th.headerStyle() });
    try lines.append(arena, try buildSeparatorLine(arena, th));

    if (fv.rows.len == 0) {
        const msg = if (fv.total_acked > 0 and !state.show_acked)
            "  No active findings. Press 'v' to show acknowledged."
        else
            "  No findings.";
        try lines.append(arena, .{ .text = msg, .style = th.mutedStyle() });
        state.findings_first_row = 0;
        state.findings_row_count = 0;
        return;
    }

    state.findings_first_row = lines.items.len;
    const start_count = lines.items.len;

    for (fv.rows, 0..) |row, i| {
        // Cursor highlight when the unified cursor is on this
        // finding row. `cursor` indexes the unified stream
        // [holdings | findings]; convert to a local findings
        // index and compare.
        const is_cursor = blk: {
            if (cursorSection(state) != .findings) break :blk false;
            break :blk cursorLocalIndex(state) == i;
        };
        // Active marker: when the finding's target matches the
        // App-active symbol, use a `*` prefix instead of two
        // spaces. Width unchanged. For sector-level findings the
        // target isn't a symbol; the comparison is harmless and
        // simply never matches.
        const is_active = active_symbol.len > 0 and std.mem.eql(u8, row.target, active_symbol);
        const marker: []const u8 = if (is_active) "* " else "  ";
        const text = try std.fmt.allocPrint(arena, "{s}{s} {s}{s}", .{
            marker,
            severityGlyph(row.severity),
            if (row.is_acked) "[acked] " else "",
            row.text,
        });
        const style = if (is_cursor)
            th.selectStyle()
        else if (row.is_acked)
            th.mutedStyle()
        else switch (row.severity) {
            .warn => th.warningStyle(),
            .flag, .err => th.negativeStyle(),
        };
        try lines.append(arena, .{ .text = text, .style = style });

        // Inline expansion: when this is the expanded finding,
        // render the details + ack notes (if any) immediately
        // below. Plus the inline input bar when we're collecting
        // an ack note for this finding.
        if (state.expanded_finding == i) {
            try appendExpansionLines(arena, lines, row, th);
            if (state.input_mode == .ack_note) {
                try appendInputBarLines(arena, lines, state, th);
            }
        }
    }

    state.findings_row_count = lines.items.len - start_count;
}

/// Append the expanded-row detail lines for a finding. For now:
/// `kind/target` line, separator-style note lines if the row is
/// acked. Future: ack date, multi-line note rendering, the
/// inline note-input bar (step 8b).
fn appendExpansionLines(
    arena: std.mem.Allocator,
    lines: *std.ArrayList(StyledLine),
    row: observations_view.FindingRow,
    th: theme.Theme,
) !void {
    const detail = try std.fmt.allocPrint(arena, "      kind: {s}  target: {s}", .{
        row.kind,
        row.target,
    });
    try lines.append(arena, .{ .text = detail, .style = th.mutedStyle() });

    if (row.ack_entry) |entry| {
        const ack_line = try std.fmt.allocPrint(arena, "      acknowledged {f}", .{
            entry.ack.acknowledged_at,
        });
        try lines.append(arena, .{ .text = ack_line, .style = th.mutedStyle() });
        for (entry.notes) |note| {
            const note_line = try std.fmt.allocPrint(arena, "      | {s}", .{note});
            try lines.append(arena, .{ .text = note_line, .style = th.mutedStyle() });
        }
    }
}

/// Append the inline note-input bar lines for the active ack flow.
/// Layout: one line per already-completed fragment (style: input),
/// then one line for the in-progress fragment with a trailing `_`
/// to indicate the cursor. Final line: hint about Enter / Ctrl+Enter /
/// Esc bindings.
fn appendInputBarLines(
    arena: std.mem.Allocator,
    lines: *std.ArrayList(StyledLine),
    state: *const State,
    th: theme.Theme,
) !void {
    for (state.note_fragments.items) |frag| {
        const line = try std.fmt.allocPrint(arena, "      > {s}", .{frag});
        try lines.append(arena, .{ .text = line, .style = th.inputStyle() });
    }
    const in_progress = try std.fmt.allocPrint(arena, "      > {s}_", .{state.note_buf[0..state.note_len]});
    try lines.append(arena, .{ .text = in_progress, .style = th.inputStyle() });
    try lines.append(arena, .{
        .text = "      [Enter] next line  [Ctrl+Enter] save  [Esc] cancel",
        .style = th.inputHintStyle(),
    });
}

/// Build the column-header row, with sort indicators on the
/// active column and the same purple/bold header style the
/// portfolio tab uses. The active sort column embeds a `▲`/`▼`
/// glyph (one display column, replacing one space of padding) so
/// the user can see at a glance which column they're sorted by
/// without reading the "Sort:" line below.
fn buildHeaderLine(
    arena: std.mem.Allocator,
    th: theme.Theme,
    sort_field: review_view.SortField,
    sort_dir: review_view.SortDirection,
) !StyledLine {
    var rb: RowBuilder = .{ .arena = arena, .th = th };
    try rb.prefix(false);
    // Per-column header, with `colLabel` baking in a sort indicator
    // when the column matches the active sort field. `colLabel`
    // returns a slice whose display width equals the column width;
    // we hand it to `appendCell` directly without re-padding.
    inline for (col_order, 0..) |col, idx| {
        if (idx > 0) try rb.gap();
        const ind: ?[]const u8 = blk: {
            const col_sf = col.sortField() orelse break :blk null;
            if (sort_field == col_sf) break :blk sort_dir.indicator();
            break :blk null;
        };
        // `comptime` widths required by colLabel.
        const w = comptime col.width();
        var buf: [64]u8 = undefined;
        const left_align = (col == .symbol or col == .sector);
        const lbl = tui.colLabel(&buf, col.header(), w, left_align, ind);
        try rb.appendCell(lbl, w, .normal);
    }
    return rb.build(th.headerStyle());
}

fn buildSeparatorLine(arena: std.mem.Allocator, th: theme.Theme) !StyledLine {
    var sep: std.ArrayList(u8) = .empty;
    try sep.appendSlice(arena, "  ");
    inline for (col_order, 0..) |col, idx| {
        if (idx > 0) try sep.append(arena, ' ');
        var k: usize = 0;
        while (k < col.width()) : (k += 1) try sep.appendSlice(arena, "─");
    }
    return .{ .text = try sep.toOwnedSlice(arena), .style = th.mutedStyle() };
}

/// Solid horizontal rule the same display width as the holdings
/// table separator, used under the Findings heading. The findings
/// rows aren't column-aligned (they're free-text wrap) so a
/// column-gapped rule looks broken there. This one's just a
/// continuous run of `─`.
fn buildSolidSeparatorLine(arena: std.mem.Allocator, th: theme.Theme) !StyledLine {
    // Total display width matches `buildSeparatorLine`:
    //   2 (prefix) + sum(col widths) + (n_cols - 1) (single-space gaps)
    const total_width: usize = comptime blk: {
        var sum: usize = 2;
        for (col_order, 0..) |col, idx| {
            if (idx > 0) sum += 1;
            sum += col.width();
        }
        break :blk sum;
    };
    var sep: std.ArrayList(u8) = .empty;
    try sep.appendSlice(arena, "  ");
    var k: usize = 2;
    while (k < total_width) : (k += 1) try sep.appendSlice(arena, "─");
    return .{ .text = try sep.toOwnedSlice(arena), .style = th.mutedStyle() };
}

fn anyReweightFlag(f: portfolio_risk.ReweightFlags) bool {
    return f.vol_3y or f.vol_10y or f.sharpe_3y or f.sharpe_10y or f.maxdd_5y or
        f.return_1y or f.return_3y or f.return_5y or f.return_10y;
}

fn sortFieldLabel(f: review_view.SortField) []const u8 {
    return switch (f) {
        .symbol => "Symbol",
        // Sector sort bakes in a symbol-asc tiebreaker (see
        // `views/review.sortRows`); the suffix tells the user
        // why VTI ends up before AAPL inside the same sector.
        .sector => "Sector (Symbol tiebreaker)",
        .weight => "Weight",
        .tax_pct => "Tax%",
        .return_1y => "1Y Return",
        .return_3y => "3Y Return",
        .return_5y => "5Y Return",
        .return_10y => "10Y Return",
        .vol_3y => "3Y Vol",
        .vol_10y => "10Y Vol",
        .sharpe_3y => "3Y Sharpe",
        .sharpe_10y => "10Y Sharpe",
        .maxdd_5y => "5Y MaxDD",
    };
}

/// Format a holding row with per-cell intent-driven coloring.
fn formatRow(
    arena: std.mem.Allocator,
    th: theme.Theme,
    r: review_view.ReviewRow,
    is_cursor: bool,
    is_active: bool,
) !StyledLine {
    var rb: RowBuilder = .{ .arena = arena, .th = th };
    try rb.prefix(is_active);

    var pct_buf: [16]u8 = undefined;

    try rb.cellLeft(r.symbol, col_symbol, .normal);
    try rb.gap();

    try rb.cellLeft(zfin.analysis.abbreviateSector(r.bucket), col_sector, .normal);
    try rb.gap();

    try rb.cellRight(fmt.fmtPct(&pct_buf, r.weight, .{}), col_weight, .normal);
    try rb.gap();

    var b1: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&b1, r.return_1y, .{ .signed = true }), col_pct, review_view.returnIntent(r.return_1y));
    try rb.gap();
    var b2: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&b2, r.return_3y, .{ .signed = true }), col_pct, review_view.returnIntent(r.return_3y));
    try rb.gap();
    var b3: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&b3, r.return_5y, .{ .signed = true }), col_pct, review_view.returnIntent(r.return_5y));
    try rb.gap();
    var b4: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&b4, r.return_10y, .{ .signed = true }), col_pct, review_view.returnIntent(r.return_10y));
    try rb.gap();

    var v3: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&v3, r.vol_3y, .{}), col_pct, review_view.volIntent(r.vol_3y));
    try rb.gap();
    var v10: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&v10, r.vol_10y, .{}), col_pct, review_view.volIntent(r.vol_10y));
    try rb.gap();

    var s3: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtSharpeOpt(&s3, r.sharpe_3y, .{}), col_sharpe, review_view.sharpeIntent(r.sharpe_3y));
    try rb.gap();
    var s10: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtSharpeOpt(&s10, r.sharpe_10y, .{}), col_sharpe, review_view.sharpeIntent(r.sharpe_10y));
    try rb.gap();

    var dd: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&dd, r.maxdd_5y, .{}), col_maxdd, review_view.maxddIntent(r.maxdd_5y));
    try rb.gap();

    var tax: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&tax, r.tax_pct, .{}), col_tax, .muted);

    // When this row is under the cursor, override the base style
    // with selectStyle so the highlight reads on whatever theme
    // the user has. Per-cell intent spans (positive/negative
    // returns, etc.) still apply on top.
    const base = if (is_cursor) th.selectStyle() else th.contentStyle();
    return rb.build(base);
}

fn formatTotalsRow(
    arena: std.mem.Allocator,
    th: theme.Theme,
    t: review_view.ReviewTotals,
) !StyledLine {
    var rb: RowBuilder = .{ .arena = arena, .th = th };
    try rb.prefix(false);

    try rb.cellLeft("Total", col_symbol, .normal);
    try rb.gap();
    try rb.cellLeft("", col_sector, .normal);
    try rb.gap();

    var pct_buf: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPct(&pct_buf, t.weight, .{}), col_weight, .normal);
    try rb.gap();

    var b1: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&b1, t.return_1y, .{ .signed = true, .asterisk = t.reweight_flags.return_1y }), col_pct, review_view.returnIntent(t.return_1y));
    try rb.gap();
    var b2: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&b2, t.return_3y, .{ .signed = true, .asterisk = t.reweight_flags.return_3y }), col_pct, review_view.returnIntent(t.return_3y));
    try rb.gap();
    var b3: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&b3, t.return_5y, .{ .signed = true, .asterisk = t.reweight_flags.return_5y }), col_pct, review_view.returnIntent(t.return_5y));
    try rb.gap();
    var b4: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&b4, t.return_10y, .{ .signed = true, .asterisk = t.reweight_flags.return_10y }), col_pct, review_view.returnIntent(t.return_10y));
    try rb.gap();

    var v3: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&v3, t.vol_3y, .{ .asterisk = t.reweight_flags.vol_3y }), col_pct, review_view.volIntent(t.vol_3y));
    try rb.gap();
    var v10: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&v10, t.vol_10y, .{ .asterisk = t.reweight_flags.vol_10y }), col_pct, review_view.volIntent(t.vol_10y));
    try rb.gap();

    var s3: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtSharpeOpt(&s3, t.sharpe_3y, .{ .asterisk = t.reweight_flags.sharpe_3y }), col_sharpe, review_view.sharpeIntent(t.sharpe_3y));
    try rb.gap();
    var s10: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtSharpeOpt(&s10, t.sharpe_10y, .{ .asterisk = t.reweight_flags.sharpe_10y }), col_sharpe, review_view.sharpeIntent(t.sharpe_10y));
    try rb.gap();

    var dd: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&dd, t.maxdd_5y, .{ .asterisk = t.reweight_flags.maxdd_5y }), col_maxdd, review_view.maxddIntent(t.maxdd_5y));
    try rb.gap();

    var tax: [16]u8 = undefined;
    try rb.cellRight(fmt.fmtPctOpt(&tax, t.tax_pct, .{}), col_tax, .muted);

    return rb.build(th.headerStyle());
}

// ── Tests ─────────────────────────────────────────────────────

const testing = std.testing;

test "nextSortField: cycles forward and wraps at end" {
    // sortable_fields starts with .symbol; from .symbol → .sector.
    try testing.expectEqual(review_view.SortField.sector, nextSortField(.symbol));
    // From the last entry (.tax_pct), wraps to the first.
    try testing.expectEqual(review_view.SortField.symbol, nextSortField(.tax_pct));
    // From .maxdd_5y (second-to-last) → .tax_pct (last).
    try testing.expectEqual(review_view.SortField.tax_pct, nextSortField(.maxdd_5y));
}

test "prevSortField: cycles backward and wraps at start" {
    // .symbol is first; prev wraps to last (.tax_pct).
    try testing.expectEqual(review_view.SortField.tax_pct, prevSortField(.symbol));
    try testing.expectEqual(review_view.SortField.symbol, prevSortField(.sector));
    // .tax_pct (last) ← .maxdd_5y.
    try testing.expectEqual(review_view.SortField.maxdd_5y, prevSortField(.tax_pct));
}

test "Col.sortField: every column has a sort target" {
    inline for (std.meta.fields(Col)) |f| {
        const c: Col = @enumFromInt(f.value);
        try testing.expect(c.sortField() != null);
    }
}

test "Col.defaultDir: strings asc, numerics desc" {
    try testing.expectEqual(review_view.SortDirection.asc, Col.symbol.defaultDir());
    try testing.expectEqual(review_view.SortDirection.asc, Col.sector.defaultDir());
    try testing.expectEqual(review_view.SortDirection.desc, Col.weight.defaultDir());
    try testing.expectEqual(review_view.SortDirection.desc, Col.return_3y.defaultDir());
    try testing.expectEqual(review_view.SortDirection.desc, Col.maxdd_5y.defaultDir());
    try testing.expectEqual(review_view.SortDirection.desc, Col.tax.defaultDir());
}

test "hitTestHeader: prefix click misses" {
    try testing.expect(hitTestHeader(0) == null);
    try testing.expect(hitTestHeader(1) == null);
}

test "hitTestHeader: column-start hits" {
    // Symbol cell starts at col 2 (after the 2-col prefix).
    try testing.expectEqual(Col.symbol, hitTestHeader(2).?);
    try testing.expectEqual(Col.symbol, hitTestHeader(9).?); // last col of symbol (width 8)
    // Gap between symbol(end=10) and sector starts at 10 → null.
    try testing.expect(hitTestHeader(10) == null);
    // Sector occupies cols 11..30 (width 20).
    try testing.expectEqual(Col.sector, hitTestHeader(11).?);
    try testing.expectEqual(Col.sector, hitTestHeader(30).?);
    // Gap at 31 → null; weight at 32..38 (width 7).
    try testing.expect(hitTestHeader(31) == null);
    try testing.expectEqual(Col.weight, hitTestHeader(32).?);
}

test "hitTestHeader: tax (last column) is hittable" {
    // Tax is the rightmost column. Compute its expected start by
    // walking col_order — easier than hardcoding column-end math
    // that drifts if a future change inserts a column.
    var pos: usize = row_prefix_cols;
    inline for (col_order, 0..) |c, idx| {
        if (idx > 0) pos += 1; // gap
        if (c == .tax) break;
        pos += c.width();
    }
    try testing.expectEqual(Col.tax, hitTestHeader(pos).?);
    try testing.expectEqual(Col.tax, hitTestHeader(pos + Col.tax.width() - 1).?);
    // Past the right edge → null.
    try testing.expect(hitTestHeader(pos + Col.tax.width()) == null);
}

test "anyReweightFlag: detects any flag" {
    try testing.expectEqual(false, anyReweightFlag(.{}));
    try testing.expectEqual(true, anyReweightFlag(.{ .vol_3y = true }));
    try testing.expectEqual(true, anyReweightFlag(.{ .return_10y = true }));
    try testing.expectEqual(true, anyReweightFlag(.{ .maxdd_5y = true }));
}

test "sortFieldLabel: covers every field variant" {
    inline for (std.meta.fields(review_view.SortField)) |f| {
        const variant: review_view.SortField = @enumFromInt(f.value);
        const label = sortFieldLabel(variant);
        try testing.expect(label.len > 0);
    }
}

test "formatRow: produces a styled line containing symbol + sector" {
    var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
    defer arena_state.deinit();
    const arena = arena_state.allocator();

    const r: review_view.ReviewRow = .{
        .symbol = "VTI",
        .bucket = "Diversified",
        .tax_pct = 0.40,
        .weight = 0.33,
        .return_1y = 0.15,
        .return_3y = 0.18,
        .return_5y = 0.12,
        .return_10y = 0.14,
        .vol_3y = 0.16,
        .vol_10y = 0.17,
        .sharpe_3y = 1.10,
        .sharpe_10y = 0.85,
        .maxdd_5y = 0.25,
    };
    const line = try formatRow(arena, theme.default_theme, r, false, false);
    try testing.expect(std.mem.indexOf(u8, line.text, "VTI") != null);
    try testing.expect(std.mem.indexOf(u8, line.text, "Diversified") != null);
    try testing.expect(std.mem.indexOf(u8, line.text, "+15.0%") != null);
}

test "formatRow: nulls render as em-dashes" {
    var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
    defer arena_state.deinit();
    const arena = arena_state.allocator();

    const r: review_view.ReviewRow = .{
        .symbol = "NEW",
        .bucket = "Bonds",
        .tax_pct = null,
        .weight = 0.05,
        .return_1y = null,
        .return_3y = null,
        .return_5y = null,
        .return_10y = null,
        .vol_3y = null,
        .vol_10y = null,
        .sharpe_3y = null,
        .sharpe_10y = null,
        .maxdd_5y = null,
    };
    const line = try formatRow(arena, theme.default_theme, r, false, false);
    try testing.expect(std.mem.count(u8, line.text, "—") >= 8);
}

test "formatRow: abbreviates Communication Services" {
    var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
    defer arena_state.deinit();
    const arena = arena_state.allocator();

    const r: review_view.ReviewRow = .{
        .symbol = "GOOGL",
        .bucket = "Communication Services",
        .tax_pct = null,
        .weight = 0.05,
        .return_1y = null,
        .return_3y = null,
        .return_5y = null,
        .return_10y = null,
        .vol_3y = null,
        .vol_10y = null,
        .sharpe_3y = null,
        .sharpe_10y = null,
        .maxdd_5y = null,
    };
    const line = try formatRow(arena, theme.default_theme, r, false, false);
    try testing.expect(std.mem.indexOf(u8, line.text, "Comm. Services") != null);
    try testing.expect(std.mem.indexOf(u8, line.text, "Communication Services") == null);
}

test "formatRow: is_active=true emits leading '* ' marker" {
    var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
    defer arena_state.deinit();
    const arena = arena_state.allocator();

    const r: review_view.ReviewRow = .{
        .symbol = "VTI",
        .bucket = "Diversified",
        .tax_pct = null,
        .weight = 0.5,
        .return_1y = null,
        .return_3y = null,
        .return_5y = null,
        .return_10y = null,
        .vol_3y = null,
        .vol_10y = null,
        .sharpe_3y = null,
        .sharpe_10y = null,
        .maxdd_5y = null,
    };
    const active = try formatRow(arena, theme.default_theme, r, false, true);
    try testing.expect(std.mem.startsWith(u8, active.text, "* "));

    // Inactive baseline: prefix is the two-space indent.
    const inactive = try formatRow(arena, theme.default_theme, r, false, false);
    try testing.expect(std.mem.startsWith(u8, inactive.text, "  "));
    try testing.expect(!std.mem.startsWith(u8, inactive.text, "* "));
}

test "formatTotalsRow: contains Total label and weight" {
    var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
    defer arena_state.deinit();
    const arena = arena_state.allocator();

    const t: review_view.ReviewTotals = .{
        .weight = 1.0,
        .return_1y = 0.20,
        .return_3y = 0.15,
        .return_5y = 0.13,
        .return_10y = 0.14,
        .vol_3y = 0.13,
        .vol_10y = 0.16,
        .sharpe_3y = 1.05,
        .sharpe_10y = 0.95,
        .maxdd_5y = 0.22,
        .tax_pct = 0.50,
        .reweight_flags = .{},
    };
    const line = try formatTotalsRow(arena, theme.default_theme, t);
    try testing.expect(std.mem.indexOf(u8, line.text, "Total") != null);
    try testing.expect(std.mem.indexOf(u8, line.text, "100.0%") != null);
    try testing.expect(std.mem.indexOf(u8, line.text, "+20.0%") != null);
}

test "formatTotalsRow: reweight flag adds asterisk" {
    var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
    defer arena_state.deinit();
    const arena = arena_state.allocator();

    const t: review_view.ReviewTotals = .{
        .weight = 1.0,
        .return_1y = null,
        .return_3y = null,
        .return_5y = null,
        .return_10y = null,
        .vol_3y = 0.13,
        .vol_10y = null,
        .sharpe_3y = null,
        .sharpe_10y = null,
        .maxdd_5y = null,
        .tax_pct = null,
        .reweight_flags = .{ .vol_3y = true },
    };
    const line = try formatTotalsRow(arena, theme.default_theme, t);
    try testing.expect(std.mem.indexOf(u8, line.text, "13.0%*") != null);
}

test "applySort: explicit field replaces default grouping" {
    var rows = [_]review_view.ReviewRow{
        .{
            .symbol = "AAPL",
            .bucket = "Technology",
            .tax_pct = null,
            .weight = 0.05,
            .return_1y = null,
            .return_3y = null,
            .return_5y = null,
            .return_10y = null,
            .vol_3y = null,
            .vol_10y = null,
            .sharpe_3y = null,
            .sharpe_10y = null,
            .maxdd_5y = null,
        },
        .{
            .symbol = "VTI",
            .bucket = "Equity / Corporate",
            .tax_pct = null,
            .weight = 0.40,
            .return_1y = null,
            .return_3y = null,
            .return_5y = null,
            .return_10y = null,
            .vol_3y = null,
            .vol_10y = null,
            .sharpe_3y = null,
            .sharpe_10y = null,
            .maxdd_5y = null,
        },
        .{
            .symbol = "MSFT",
            .bucket = "Technology",
            .tax_pct = null,
            .weight = 0.15,
            .return_1y = null,
            .return_3y = null,
            .return_5y = null,
            .return_10y = null,
            .vol_3y = null,
            .vol_10y = null,
            .sharpe_3y = null,
            .sharpe_10y = null,
            .maxdd_5y = null,
        },
    };

    const view: review_view.ReviewView = .{
        .rows = rows[0..],
        .totals = .{
            .weight = 1.0,
            .return_1y = null,
            .return_3y = null,
            .return_5y = null,
            .return_10y = null,
            .vol_3y = null,
            .vol_10y = null,
            .sharpe_3y = null,
            .sharpe_10y = null,
            .maxdd_5y = null,
            .tax_pct = null,
            .reweight_flags = .{},
        },
        .as_of = zfin.Date.fromYmd(2026, 6, 4),
        .total_liquid = 0,
        .portfolio_path = "",
    };
    var state: State = .{ .view = view, .sort_field = .sector, .sort_dir = .asc };
    applySort(&state);
    // Default grouping (sector asc with symbol-asc tiebreaker) →
    // Equity/Corporate first (only VTI), then Technology with
    // AAPL before MSFT (alphabetical).
    try testing.expectEqualStrings("Equity / Corporate", state.view.?.rows[0].bucket);
    try testing.expectEqualStrings("VTI", state.view.?.rows[0].symbol);
    try testing.expectEqualStrings("Technology", state.view.?.rows[1].bucket);
    try testing.expectEqualStrings("AAPL", state.view.?.rows[1].symbol);
    try testing.expectEqualStrings("Technology", state.view.?.rows[2].bucket);
    try testing.expectEqualStrings("MSFT", state.view.?.rows[2].symbol);

    // Explicit weight desc
    state.sort_field = .weight;
    state.sort_dir = .desc;
    applySort(&state);
    try testing.expectEqualStrings("VTI", state.view.?.rows[0].symbol);
    try testing.expectEqualStrings("MSFT", state.view.?.rows[1].symbol);
    try testing.expectEqualStrings("AAPL", state.view.?.rows[2].symbol);
}

test "applyHeaderClick: prefix click does not modify state" {
    var state: State = .{ .sort_field = .sector, .sort_dir = .asc };
    try testing.expect(!applyHeaderClick(&state, 0));
    try testing.expect(!applyHeaderClick(&state, 1));
    // State unchanged.
    try testing.expectEqual(review_view.SortField.sector, state.sort_field);
    try testing.expectEqual(review_view.SortDirection.asc, state.sort_dir);
}

test "applyHeaderClick: gap-column click does not modify state" {
    var state: State = .{ .sort_field = .sector, .sort_dir = .asc };
    // Symbol cell ends at col 9 (prefix=2, width=8). Gap is col 10.
    try testing.expect(!applyHeaderClick(&state, 10));
    try testing.expectEqual(review_view.SortField.sector, state.sort_field);
}

test "applyHeaderClick: click on Symbol column sets sort to symbol with default asc" {
    var state: State = .{ .sort_field = .weight, .sort_dir = .desc };
    // Symbol occupies cols 2..9.
    try testing.expect(applyHeaderClick(&state, 2));
    try testing.expectEqual(review_view.SortField.symbol, state.sort_field);
    try testing.expectEqual(review_view.SortDirection.asc, state.sort_dir);
}

test "applyHeaderClick: click on numeric column sets sort with default desc" {
    var state: State = .{ .sort_field = .symbol, .sort_dir = .asc };
    // Weight column starts at col 32 (see hitTestHeader test).
    try testing.expect(applyHeaderClick(&state, 32));
    try testing.expectEqual(review_view.SortField.weight, state.sort_field);
    try testing.expectEqual(review_view.SortDirection.desc, state.sort_dir);
}

test "applyHeaderClick: clicking active column flips direction" {
    var state: State = .{ .sort_field = .weight, .sort_dir = .desc };
    // Re-click weight column.
    try testing.expect(applyHeaderClick(&state, 32));
    try testing.expectEqual(review_view.SortField.weight, state.sort_field);
    try testing.expectEqual(review_view.SortDirection.asc, state.sort_dir);
    // Click again — flip back.
    try testing.expect(applyHeaderClick(&state, 32));
    try testing.expectEqual(review_view.SortDirection.desc, state.sort_dir);
}

test "handleAction: sort_col_next walks through fields" {
    var state: State = .{ .sort_field = .symbol, .sort_dir = .asc };
    // handleAction takes *App but never reads it; pass undefined.
    var app: App = undefined;
    tab.handleAction(&state, &app, .sort_col_next);
    try testing.expectEqual(review_view.SortField.sector, state.sort_field);
}

test "handleAction: sort_col_prev walks backward" {
    var state: State = .{ .sort_field = .sector, .sort_dir = .asc };
    var app: App = undefined;
    tab.handleAction(&state, &app, .sort_col_prev);
    try testing.expectEqual(review_view.SortField.symbol, state.sort_field);
}

test "handleAction: sort_reverse flips direction" {
    var state: State = .{ .sort_field = .sector, .sort_dir = .asc };
    var app: App = undefined;
    tab.handleAction(&state, &app, .sort_reverse);
    try testing.expectEqual(review_view.SortDirection.desc, state.sort_dir);
    tab.handleAction(&state, &app, .sort_reverse);
    try testing.expectEqual(review_view.SortDirection.asc, state.sort_dir);
}

test "deinitState: cleans up view (leak check)" {
    // Allocate a real ReviewView. deinitState must free everything
    // for testing.allocator to pass.
    const Date = zfin.Date;

    var allocs = [_]@import("../analytics/valuation.zig").Allocation{
        .{
            .symbol = "VTI",
            .display_symbol = "VTI",
            .shares = 100,
            .avg_cost = 200,
            .current_price = 220,
            .market_value = 22000,
            .cost_basis = 20000,
            .weight = 1.0,
            .unrealized_gain_loss = 2000,
            .unrealized_return = 0.10,
        },
    };
    const summary: @import("../analytics/valuation.zig").PortfolioSummary = .{
        .total_value = 22000,
        .total_cost = 20000,
        .unrealized_gain_loss = 2000,
        .unrealized_return = 0.10,
        .realized_gain_loss = 0,
        .allocations = allocs[0..],
    };
    var lots = [_]zfin.Lot{
        .{ .symbol = "VTI", .shares = 100, .open_date = Date.fromYmd(2022, 1, 10), .open_price = 200 },
    };
    const portfolio: zfin.Portfolio = .{ .lots = lots[0..], .allocator = testing.allocator };
    var class_entries = [_]zfin.classification.ClassificationEntry{
        .{ .symbol = "VTI", .sector = "Equity / Corporate", .pct = 100.0 },
    };
    const cm: zfin.classification.ClassificationMap = .{
        .entries = class_entries[0..],
        .allocator = testing.allocator,
    };
    var candle_map = std.StringHashMap([]const zfin.Candle).init(testing.allocator);
    defer candle_map.deinit();

    const view = try review_view.buildReview(
        testing.allocator,
        std.testing.io,
        summary,
        &candle_map,
        null,
        portfolio,
        cm,
        null,
        Date.fromYmd(2026, 6, 4),
        "test.srf",
    );

    // Build a dividend map with one allocated entry — but DO NOT
    // attach it to State (dividend_map lives on App now). Free it
    // after deinitState so the test's testing.allocator stays
    // satisfied.
    var dividend_map = std.StringHashMap([]const zfin.Dividend).init(testing.allocator);
    const divs = try testing.allocator.alloc(zfin.Dividend, 1);
    divs[0] = .{ .ex_date = Date.fromYmd(2024, 6, 15), .pay_date = Date.fromYmd(2024, 7, 1), .amount = 1.5 };
    try dividend_map.put("VTI", divs);

    var state: State = .{
        .loaded = true,
        .view = view,
        .sort_field = .sector,
        .sort_dir = .asc,
    };

    // Single deinit must free everything State owns.
    deinitState(&state, testing.allocator);
    try testing.expect(state.view == null);

    // Manually clean up our standalone dividend_map (not owned by
    // State — App owns these in production).
    testing.allocator.free(divs);
    dividend_map.deinit();
}

// ── Step 8a observations integration tests ─────────────────────

test "severityGlyph: covers every severity" {
    try testing.expect(severityGlyph(.warn).len > 0);
    try testing.expect(severityGlyph(.flag).len > 0);
    try testing.expect(severityGlyph(.err).len > 0);
}

test "appendFindingsSection: empty findings with no acked produces 'No findings.'" {
    var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
    defer arena_state.deinit();
    const arena = arena_state.allocator();

    var lines: std.ArrayList(StyledLine) = .empty;
    const fv: observations_view.FindingsView = .{
        .rows = &.{},
        .total_active = 0,
        .total_acked = 0,
        .total_resolved = 0,
    };
    var state: State = .{};

    try appendFindingsSection(arena, &lines, &state, fv, theme.default_theme, "");

    try testing.expect(lines.items.len >= 4);
    var found_no_findings = false;
    for (lines.items) |ln| {
        if (std.mem.indexOf(u8, ln.text, "No findings.") != null) found_no_findings = true;
    }
    try testing.expect(found_no_findings);
}

test "appendFindingsSection: hidden-acked hint when all findings are acked" {
    var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
    defer arena_state.deinit();
    const arena = arena_state.allocator();

    var lines: std.ArrayList(StyledLine) = .empty;
    const fv: observations_view.FindingsView = .{
        .rows = &.{},
        .total_active = 0,
        .total_acked = 3,
        .total_resolved = 0,
    };
    var state: State = .{ .show_acked = false };

    try appendFindingsSection(arena, &lines, &state, fv, theme.default_theme, "");

    var hint_found = false;
    for (lines.items) |ln| {
        if (std.mem.indexOf(u8, ln.text, "Press 'v'") != null) hint_found = true;
    }
    try testing.expect(hint_found);
}

test "appendFindingsSection: renders cursor highlight on focused row" {
    var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
    defer arena_state.deinit();
    const arena = arena_state.allocator();

    var lines: std.ArrayList(StyledLine) = .empty;
    var rows = [_]observations_view.FindingRow{
        .{ .severity = .warn, .kind = "k", .target = "t1", .text = "t1 finding", .is_acked = false },
        .{ .severity = .flag, .kind = "k", .target = "t2", .text = "t2 finding", .is_acked = false },
    };
    const fv: observations_view.FindingsView = .{
        .rows = &rows,
        .total_active = 2,
        .total_acked = 0,
        .total_resolved = 0,
    };
    // Empty holdings view + 2 findings: unified cursor 1 lands
    // on the second finding (local index 1). State must reference
    // the same findings_view so cursor section resolves correctly.
    const empty_view: review_view.ReviewView = .{
        .rows = &.{},
        .totals = std.mem.zeroes(review_view.ReviewTotals),
        .as_of = zfin.Date.fromYmd(2026, 6, 8),
        .total_liquid = 0,
        .portfolio_path = "x",
    };
    var state: State = .{
        .view = empty_view,
        .cursor = 1,
        .findings_view = fv,
    };

    try appendFindingsSection(arena, &lines, &state, fv, theme.default_theme, "");

    // Find the row containing "t1 finding" and "t2 finding"; t2's
    // style should match selectStyle while t1's should not.
    const sel = theme.default_theme.selectStyle();
    var t1_style: ?vaxis.Style = null;
    var t2_style: ?vaxis.Style = null;
    for (lines.items) |ln| {
        if (std.mem.indexOf(u8, ln.text, "t1 finding") != null) t1_style = ln.style;
        if (std.mem.indexOf(u8, ln.text, "t2 finding") != null) t2_style = ln.style;
    }
    try testing.expect(t1_style != null and t2_style != null);
    try testing.expect(std.meta.eql(t2_style.?, sel));
    try testing.expect(!std.meta.eql(t1_style.?, sel));

    // findings_view + view borrow stack allocations; clear before
    // drop to avoid double-free.
    state.findings_view = null;
    state.view = null;
}

test "wrapCursor: forward delta within range" {
    try testing.expectEqual(@as(usize, 3), wrapCursor(2, 1, 5));
    try testing.expectEqual(@as(usize, 4), wrapCursor(0, 4, 5));
}

test "wrapCursor: forward delta wraps past the end" {
    // From last row, +1 wraps to 0.
    try testing.expectEqual(@as(usize, 0), wrapCursor(4, 1, 5));
    // From row 3, +5 lands at row 3 again (3+5 mod 5 = 3).
    try testing.expectEqual(@as(usize, 3), wrapCursor(3, 5, 5));
    // Big positive jump still wraps cleanly.
    try testing.expectEqual(@as(usize, 1), wrapCursor(0, 6, 5));
}

test "wrapCursor: backward delta wraps from 0 to last" {
    try testing.expectEqual(@as(usize, 4), wrapCursor(0, -1, 5));
    try testing.expectEqual(@as(usize, 1), wrapCursor(0, -4, 5));
    try testing.expectEqual(@as(usize, 0), wrapCursor(0, -5, 5));
}

test "wrapCursor: large negative deltas wrap modulo total" {
    // -7 from index 2 in a 5-row table: 2 + (-7) = -5; mod 5 = 0.
    try testing.expectEqual(@as(usize, 0), wrapCursor(2, -7, 5));
}

test "expansionLineCount: minimum is 1 (detail line)" {
    const row: observations_view.FindingRow = .{
        .severity = .warn,
        .kind = "k",
        .target = "t",
        .text = "x",
        .is_acked = false,
    };
    try testing.expectEqual(@as(usize, 1), expansionLineCount(row));
}

test "expansionLineCount: acked with two notes adds 1 + 2 = 3" {
    const notes = [_][]const u8{ "first", "second" };
    const entry = Journal.Entry{
        .ack = .{
            .observation = "k",
            .target = "t",
            .acknowledged_at = zfin.Date.fromYmd(2026, 1, 1),
            .state = .acknowledged,
        },
        .notes = &notes,
    };
    const row: observations_view.FindingRow = .{
        .severity = .warn,
        .kind = "k",
        .target = "t",
        .text = "x",
        .is_acked = true,
        .ack_entry = &entry,
    };
    // Detail (1) + ack date (1) + 2 notes = 4.
    try testing.expectEqual(@as(usize, 4), expansionLineCount(row));
}

test "inputBarLineCount: zero when not in ack_note mode" {
    const state: State = .{};
    try testing.expectEqual(@as(usize, 0), inputBarLineCount(&state));
}

test "inputBarLineCount: one in-progress line + one hint when in ack_note with no fragments" {
    const state: State = .{ .input_mode = .ack_note };
    try testing.expectEqual(@as(usize, 2), inputBarLineCount(&state));
}

test "clearNoteFragments: idempotent on empty list" {
    var state: State = .{};
    clearNoteFragments(&state, testing.allocator);
    clearNoteFragments(&state, testing.allocator);
    try testing.expectEqual(@as(usize, 0), state.note_fragments.items.len);
}

test "clearNoteFragments: frees fragment slices" {
    var state: State = .{};
    const frag1 = try testing.allocator.dupe(u8, "first");
    const frag2 = try testing.allocator.dupe(u8, "second");
    try state.note_fragments.append(testing.allocator, frag1);
    try state.note_fragments.append(testing.allocator, frag2);

    clearNoteFragments(&state, testing.allocator);
    try testing.expectEqual(@as(usize, 0), state.note_fragments.items.len);
}

// ── handleAction: M2 actions (no App access) ───────────────

test "handleAction: toggle_expand on findings cursor toggles expanded_finding" {
    // Build a minimal findings_view with one row so toggle_expand
    // has something to expand to. Cursor lands at unified index 0
    // which means "first finding" when view has 0 holdings.
    var rows = [_]observations_view.FindingRow{
        .{ .severity = .warn, .kind = "k", .target = "t", .text = "x", .is_acked = false },
    };
    // Empty holdings view + one finding → unified cursor at 0
    // resolves to findings local index 0.
    const empty_view: review_view.ReviewView = .{
        .rows = &.{},
        .totals = std.mem.zeroes(review_view.ReviewTotals),
        .as_of = zfin.Date.fromYmd(2026, 6, 8),
        .total_liquid = 0,
        .portfolio_path = "x",
    };
    var state: State = .{
        .view = empty_view,
        .cursor = 0,
        .findings_view = .{
            .rows = &rows,
            .total_active = 1,
            .total_acked = 0,
            .total_resolved = 0,
        },
    };
    var app: App = undefined;

    tab.handleAction(&state, &app, .toggle_expand);
    try testing.expect(state.expanded_finding != null);
    try testing.expectEqual(@as(usize, 0), state.expanded_finding.?);

    tab.handleAction(&state, &app, .toggle_expand);
    try testing.expect(state.expanded_finding == null);

    // findings_view + view borrow stack allocations; clear before
    // deinit to avoid double-free.
    state.findings_view = null;
    state.view = null;
}

test "handleAction: toggle_expand on empty findings is no-op" {
    // Empty holdings + empty findings: cursor at 0 in an empty
    // unified space → cursorSection returns .empty; toggle_expand
    // returns without setting expanded_finding.
    const empty_view: review_view.ReviewView = .{
        .rows = &.{},
        .totals = std.mem.zeroes(review_view.ReviewTotals),
        .as_of = zfin.Date.fromYmd(2026, 6, 8),
        .total_liquid = 0,
        .portfolio_path = "x",
    };
    var state: State = .{
        .view = empty_view,
        .findings_view = .{
            .rows = &.{},
            .total_active = 0,
            .total_acked = 0,
            .total_resolved = 0,
        },
    };
    var app: App = undefined;
    tab.handleAction(&state, &app, .toggle_expand);
    try testing.expect(state.expanded_finding == null);

    state.view = null;
    state.findings_view = null;
}

test "handleAction: toggle_expand on cursor in holdings is no-op" {
    // Holdings row exists, cursor is on it (no findings) → no
    // finding to expand.
    var rows = [_]review_view.ReviewRow{.{
        .symbol = "X",
        .bucket = "S",
        .tax_pct = 0,
        .weight = 1.0,
        .return_1y = null,
        .return_3y = null,
        .return_5y = null,
        .return_10y = null,
        .vol_3y = null,
        .vol_10y = null,
        .sharpe_3y = null,
        .sharpe_10y = null,
        .maxdd_5y = null,
    }};
    var state: State = .{
        .cursor = 0,
        .view = .{
            .rows = &rows,
            .totals = std.mem.zeroes(review_view.ReviewTotals),
            .as_of = zfin.Date.fromYmd(2026, 6, 8),
            .total_liquid = 0,
            .portfolio_path = "x",
        },
    };
    var app: App = undefined;
    tab.handleAction(&state, &app, .toggle_expand);
    try testing.expect(state.expanded_finding == null);

    state.view = null;
}

// ── onCursorMove: unified cursor with wrap ─────────────────────

test "onCursorMove: walks through holdings then wraps into findings" {
    // 3 holdings + 2 findings = 5 unified rows.
    var holdings = [_]review_view.ReviewRow{ .{
        .symbol = "A",
        .bucket = "S",
        .tax_pct = 0,
        .weight = 0.33,
        .return_1y = null,
        .return_3y = null,
        .return_5y = null,
        .return_10y = null,
        .vol_3y = null,
        .vol_10y = null,
        .sharpe_3y = null,
        .sharpe_10y = null,
        .maxdd_5y = null,
    }, .{
        .symbol = "B",
        .bucket = "S",
        .tax_pct = 0,
        .weight = 0.33,
        .return_1y = null,
        .return_3y = null,
        .return_5y = null,
        .return_10y = null,
        .vol_3y = null,
        .vol_10y = null,
        .sharpe_3y = null,
        .sharpe_10y = null,
        .maxdd_5y = null,
    }, .{
        .symbol = "C",
        .bucket = "S",
        .tax_pct = 0,
        .weight = 0.34,
        .return_1y = null,
        .return_3y = null,
        .return_5y = null,
        .return_10y = null,
        .vol_3y = null,
        .vol_10y = null,
        .sharpe_3y = null,
        .sharpe_10y = null,
        .maxdd_5y = null,
    } };
    var findings = [_]observations_view.FindingRow{
        .{ .severity = .warn, .kind = "k", .target = "F1", .text = "x", .is_acked = false },
        .{ .severity = .flag, .kind = "k", .target = "F2", .text = "y", .is_acked = false },
    };
    var state: State = .{
        .cursor = 0,
        .view = .{
            .rows = &holdings,
            .totals = std.mem.zeroes(review_view.ReviewTotals),
            .as_of = zfin.Date.fromYmd(2026, 6, 8),
            .total_liquid = 0,
            .portfolio_path = "x",
        },
        .findings_view = .{
            .rows = &findings,
            .total_active = 2,
            .total_acked = 0,
            .total_resolved = 0,
        },
    };
    var app: App = undefined;
    app.scroll_offset = 0;
    app.visible_height = 100;

    // Walk j three times: cursor 0 → 1 → 2 → 3 (which is the
    // first finding — section transition with no special key).
    try testing.expect(tab.onCursorMove(&state, &app, 1));
    try testing.expectEqual(CursorSection.holdings, cursorSection(&state));
    try testing.expectEqual(@as(usize, 1), state.cursor);

    try testing.expect(tab.onCursorMove(&state, &app, 1));
    try testing.expectEqual(@as(usize, 2), state.cursor);
    try testing.expectEqual(CursorSection.holdings, cursorSection(&state));

    try testing.expect(tab.onCursorMove(&state, &app, 1));
    try testing.expectEqual(@as(usize, 3), state.cursor);
    try testing.expectEqual(CursorSection.findings, cursorSection(&state));
    try testing.expectEqual(@as(usize, 0), cursorLocalIndex(&state));

    // One more j → second finding.
    try testing.expect(tab.onCursorMove(&state, &app, 1));
    try testing.expectEqual(@as(usize, 4), state.cursor);

    // One more j → wraps back to first holding.
    try testing.expect(tab.onCursorMove(&state, &app, 1));
    try testing.expectEqual(@as(usize, 0), state.cursor);
    try testing.expectEqual(CursorSection.holdings, cursorSection(&state));

    state.view = null;
    state.findings_view = null;
}

test "onCursorMove: backward from first holding wraps to last finding" {
    var holdings = [_]review_view.ReviewRow{.{
        .symbol = "A",
        .bucket = "S",
        .tax_pct = 0,
        .weight = 1.0,
        .return_1y = null,
        .return_3y = null,
        .return_5y = null,
        .return_10y = null,
        .vol_3y = null,
        .vol_10y = null,
        .sharpe_3y = null,
        .sharpe_10y = null,
        .maxdd_5y = null,
    }};
    var findings = [_]observations_view.FindingRow{
        .{ .severity = .warn, .kind = "k", .target = "F1", .text = "x", .is_acked = false },
    };
    var state: State = .{
        .cursor = 0,
        .view = .{
            .rows = &holdings,
            .totals = std.mem.zeroes(review_view.ReviewTotals),
            .as_of = zfin.Date.fromYmd(2026, 6, 8),
            .total_liquid = 0,
            .portfolio_path = "x",
        },
        .findings_view = .{
            .rows = &findings,
            .total_active = 1,
            .total_acked = 0,
            .total_resolved = 0,
        },
    };
    var app: App = undefined;
    app.scroll_offset = 0;
    app.visible_height = 100;

    try testing.expect(tab.onCursorMove(&state, &app, -1));
    try testing.expectEqual(@as(usize, 1), state.cursor);
    try testing.expectEqual(CursorSection.findings, cursorSection(&state));

    state.view = null;
    state.findings_view = null;
}

test "onCursorMove: cursor move clears stale expansion" {
    var findings = [_]observations_view.FindingRow{
        .{ .severity = .warn, .kind = "k", .target = "t1", .text = "x", .is_acked = false },
        .{ .severity = .warn, .kind = "k", .target = "t2", .text = "x", .is_acked = false },
        .{ .severity = .warn, .kind = "k", .target = "t3", .text = "x", .is_acked = false },
    };
    const empty_view: review_view.ReviewView = .{
        .rows = &.{},
        .totals = std.mem.zeroes(review_view.ReviewTotals),
        .as_of = zfin.Date.fromYmd(2026, 6, 8),
        .total_liquid = 0,
        .portfolio_path = "x",
    };
    var state: State = .{
        .view = empty_view,
        .cursor = 0, // first finding (no holdings)
        .expanded_finding = 0,
        .findings_view = .{
            .rows = &findings,
            .total_active = 3,
            .total_acked = 0,
            .total_resolved = 0,
        },
    };
    var app: App = undefined;
    app.scroll_offset = 0;
    app.visible_height = 100;
    try testing.expect(tab.onCursorMove(&state, &app, 1));
    try testing.expectEqual(@as(usize, 1), state.cursor);
    try testing.expect(state.expanded_finding == null); // moved off, expansion collapsed

    state.view = null;
    state.findings_view = null;
}

test "onCursorMove: empty table returns false" {
    const empty_view: review_view.ReviewView = .{
        .rows = &.{},
        .totals = std.mem.zeroes(review_view.ReviewTotals),
        .as_of = zfin.Date.fromYmd(2026, 6, 8),
        .total_liquid = 0,
        .portfolio_path = "x",
    };
    var state: State = .{ .view = empty_view, .findings_view = .{
        .rows = &.{},
        .total_active = 0,
        .total_acked = 0,
        .total_resolved = 0,
    } };
    var app: App = undefined;
    app.scroll_offset = 0;
    app.visible_height = 100;
    try testing.expect(!tab.onCursorMove(&state, &app, 1));

    state.view = null;
    state.findings_view = null;
}

test "onCursorMove: wheel-sized delta still moves by one row" {
    // Wheel events arrive as ±3 per detent. The cursor should
    // step by ±1 regardless — wheel == j/k, not "skip three rows".
    var findings = [_]observations_view.FindingRow{
        .{ .severity = .warn, .kind = "k", .target = "F1", .text = "x", .is_acked = false },
        .{ .severity = .warn, .kind = "k", .target = "F2", .text = "x", .is_acked = false },
        .{ .severity = .warn, .kind = "k", .target = "F3", .text = "x", .is_acked = false },
        .{ .severity = .warn, .kind = "k", .target = "F4", .text = "x", .is_acked = false },
    };
    const empty_view: review_view.ReviewView = .{
        .rows = &.{},
        .totals = std.mem.zeroes(review_view.ReviewTotals),
        .as_of = zfin.Date.fromYmd(2026, 6, 8),
        .total_liquid = 0,
        .portfolio_path = "x",
    };
    var state: State = .{
        .view = empty_view,
        .cursor = 0,
        .findings_view = .{
            .rows = &findings,
            .total_active = 4,
            .total_acked = 0,
            .total_resolved = 0,
        },
    };
    var app: App = undefined;
    app.scroll_offset = 0;
    app.visible_height = 100;

    // Wheel down by 3: cursor moves by 1 only.
    try testing.expect(tab.onCursorMove(&state, &app, 3));
    try testing.expectEqual(@as(usize, 1), state.cursor);

    // Wheel up by 3: cursor moves by 1 only.
    try testing.expect(tab.onCursorMove(&state, &app, -3));
    try testing.expectEqual(@as(usize, 0), state.cursor);

    state.view = null;
    state.findings_view = null;
}

// onWheelMove no longer declared on this tab — wheel falls
// through to onCursorMove via the framework. See onCursorMove
// tests above for behavior.

// ── handleMouse: holdings & findings click translates to cursor ─

test "handleMouse: click on holdings row sets unified cursor" {
    const rows = try testing.allocator.alloc(review_view.ReviewRow, 3);
    defer testing.allocator.free(rows);
    for (rows, 0..) |*r, i| {
        r.* = .{
            .symbol = "X",
            .bucket = "S",
            .tax_pct = 0,
            .weight = 0.33,
            .return_1y = null,
            .return_3y = null,
            .return_5y = null,
            .return_10y = null,
            .vol_3y = null,
            .vol_10y = null,
            .sharpe_3y = null,
            .sharpe_10y = null,
            .maxdd_5y = null,
        };
        _ = i;
    }
    var state: State = .{
        // Pre-existing cursor on the second finding (will be
        // moved by the click).
        .cursor = 100,
        .holdings_first_row = 5,
        .header_row = 4,
        .view = .{
            .rows = rows,
            .totals = std.mem.zeroes(review_view.ReviewTotals),
            .as_of = zfin.Date.fromYmd(2026, 6, 8),
            .total_liquid = 0,
            .portfolio_path = "x",
        },
    };
    var app: App = undefined;
    app.scroll_offset = 0;

    const consumed = tab.handleMouse(&state, &app, .{
        .button = .left,
        .type = .press,
        .row = 6, // content_row 6 = holdings_first_row+1 → cursor 1
        .col = 0,
        .mods = .{},
    });
    try testing.expect(consumed);
    try testing.expectEqual(CursorSection.holdings, cursorSection(&state));
    try testing.expectEqual(@as(usize, 1), state.cursor);

    state.view = null; // borrowed slice; avoid double-free
}

test "handleMouse: click on findings row sets unified cursor" {
    var rows = [_]observations_view.FindingRow{
        .{ .severity = .warn, .kind = "k", .target = "t1", .text = "x", .is_acked = false },
        .{ .severity = .warn, .kind = "k", .target = "t2", .text = "x", .is_acked = false },
    };
    // handleMouse early-returns if state.view is null. Give it an
    // empty ReviewView so the findings-click branch is reachable.
    const empty_view: review_view.ReviewView = .{
        .rows = &.{},
        .totals = std.mem.zeroes(review_view.ReviewTotals),
        .as_of = zfin.Date.fromYmd(2026, 6, 8),
        .total_liquid = 0,
        .portfolio_path = "x",
    };
    var state: State = .{
        .cursor = 0, // start on first holding (none here)
        .view = empty_view,
        .findings_first_row = 10,
        .findings_row_count = 2,
        .findings_view = .{
            .rows = &rows,
            .total_active = 2,
            .total_acked = 0,
            .total_resolved = 0,
        },
    };
    var app: App = undefined;
    app.scroll_offset = 0;

    const consumed = tab.handleMouse(&state, &app, .{
        .button = .left,
        .type = .press,
        .row = 11, // → 2nd finding
        .col = 0,
        .mods = .{},
    });
    try testing.expect(consumed);
    // Empty holdings + 2 findings → unified cursor 1 = local
    // findings index 1.
    try testing.expectEqual(CursorSection.findings, cursorSection(&state));
    try testing.expectEqual(@as(usize, 1), state.cursor);
    try testing.expectEqual(@as(usize, 1), cursorLocalIndex(&state));

    state.view = null;
    state.findings_view = null;
}

test "handleMouse: non-press / non-left events ignored" {
    var state: State = .{};
    var app: App = undefined;
    app.scroll_offset = 0;

    try testing.expect(!tab.handleMouse(&state, &app, .{
        .button = .right,
        .type = .press,
        .row = 0,
        .col = 0,
        .mods = .{},
    }));
    try testing.expect(!tab.handleMouse(&state, &app, .{
        .button = .left,
        .type = .release,
        .row = 0,
        .col = 0,
        .mods = .{},
    }));
}

// ── status-grid tests ─────────────────────────────────────────

test "checkStatusGlyph: covers every CheckResult variant" {
    try testing.expect(checkStatusGlyph(.pass).len > 0);
    try testing.expect(checkStatusGlyph(.{ .warn = &.{} }).len > 0);
    try testing.expect(checkStatusGlyph(.{ .flag = &.{} }).len > 0);
    try testing.expect(checkStatusGlyph(.skipped).len > 0);
    try testing.expect(checkStatusGlyph(.{ .err = "" }).len > 0);
}

test "checkStatusGlyph: distinct glyph per result" {
    // The whole point of the grid is that users can disambiguate
    // states at a glance — no two results should map to the same
    // glyph.
    const pass_g = checkStatusGlyph(.pass);
    const warn_g = checkStatusGlyph(.{ .warn = &.{} });
    const flag_g = checkStatusGlyph(.{ .flag = &.{} });
    const skipped_g = checkStatusGlyph(.skipped);
    const err_g = checkStatusGlyph(.{ .err = "" });
    try testing.expect(!std.mem.eql(u8, pass_g, warn_g));
    try testing.expect(!std.mem.eql(u8, pass_g, flag_g));
    try testing.expect(!std.mem.eql(u8, pass_g, skipped_g));
    try testing.expect(!std.mem.eql(u8, pass_g, err_g));
    try testing.expect(!std.mem.eql(u8, warn_g, flag_g));
    try testing.expect(!std.mem.eql(u8, warn_g, skipped_g));
    try testing.expect(!std.mem.eql(u8, warn_g, err_g));
    try testing.expect(!std.mem.eql(u8, flag_g, skipped_g));
    try testing.expect(!std.mem.eql(u8, flag_g, err_g));
    try testing.expect(!std.mem.eql(u8, skipped_g, err_g));
}

test "appendStatusGrid: one row per status_cells_per_row checks" {
    var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
    defer arena_state.deinit();
    const arena = arena_state.allocator();

    // Build a 6-check panel by hand. Pass, warn, flag, skipped,
    // err, pass — covers every glyph.
    const checks = [_]observations.Check{
        .{
            .name = "a",
            .label = "Alpha",
            // SAFETY: test-only Check; pre-baked CheckResults bypass dispatch.
            .run = undefined,
        },
        .{
            .name = "b",
            .label = "Bravo",
            // SAFETY: test-only Check; pre-baked CheckResults bypass dispatch.
            .run = undefined,
        },
        .{
            .name = "c",
            .label = "Charlie",
            // SAFETY: test-only Check; pre-baked CheckResults bypass dispatch.
            .run = undefined,
        },
        .{
            .name = "d",
            .label = "Delta",
            // SAFETY: test-only Check; pre-baked CheckResults bypass dispatch.
            .run = undefined,
        },
        .{
            .name = "e",
            .label = "Echo",
            // SAFETY: test-only Check; pre-baked CheckResults bypass dispatch.
            .run = undefined,
        },
        .{
            .name = "f",
            .label = "Foxtrot",
            // SAFETY: test-only Check; pre-baked CheckResults bypass dispatch.
            .run = undefined,
        },
    };
    var pending = [_]observations.PendingCheck{
        .{ .check = &checks[0], .state = .{ .complete = .pass } },
        .{ .check = &checks[1], .state = .{ .complete = .{ .warn = &.{} } } },
        .{ .check = &checks[2], .state = .{ .complete = .{ .flag = &.{} } } },
        .{ .check = &checks[3], .state = .{ .complete = .skipped } },
        .{ .check = &checks[4], .state = .{ .complete = .{ .err = "" } } },
        .{ .check = &checks[5], .state = .{ .complete = .pass } },
    };
    const panel: observations.CheckPanel = .{
        .allocator = testing.allocator,
        .io = std.testing.io,
        .pending = &pending,
    };

    var lines: std.ArrayList(StyledLine) = .empty;
    try appendStatusGrid(arena, &lines, panel, theme.default_theme);

    // 6 cells / 3 per row = 2 rows.
    try testing.expectEqual(@as(usize, 2), lines.items.len);

    // First row should contain Alpha + Bravo + Charlie labels.
    try testing.expect(std.mem.indexOf(u8, lines.items[0].text, "Alpha") != null);
    try testing.expect(std.mem.indexOf(u8, lines.items[0].text, "Bravo") != null);
    try testing.expect(std.mem.indexOf(u8, lines.items[0].text, "Charlie") != null);
    // Second row: Delta + Echo + Foxtrot.
    try testing.expect(std.mem.indexOf(u8, lines.items[1].text, "Delta") != null);
    try testing.expect(std.mem.indexOf(u8, lines.items[1].text, "Echo") != null);
    try testing.expect(std.mem.indexOf(u8, lines.items[1].text, "Foxtrot") != null);
}

test "appendStatusGrid: empty panel produces no lines" {
    var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
    defer arena_state.deinit();
    const arena = arena_state.allocator();

    const panel: observations.CheckPanel = .{
        .allocator = testing.allocator,
        .io = std.testing.io,
        .pending = &.{},
    };
    var lines: std.ArrayList(StyledLine) = .empty;
    try appendStatusGrid(arena, &lines, panel, theme.default_theme);
    try testing.expectEqual(@as(usize, 0), lines.items.len);
}

// ── cursor section helpers ────────────────────────────────────

test "cursorSection: empty when no view loaded" {
    const state: State = .{};
    try testing.expectEqual(CursorSection.empty, cursorSection(&state));
}

test "cursorSection: holdings when cursor < holdings_len" {
    var rows = [_]review_view.ReviewRow{ .{
        .symbol = "A",
        .bucket = "S",
        .tax_pct = 0,
        .weight = 1.0,
        .return_1y = null,
        .return_3y = null,
        .return_5y = null,
        .return_10y = null,
        .vol_3y = null,
        .vol_10y = null,
        .sharpe_3y = null,
        .sharpe_10y = null,
        .maxdd_5y = null,
    }, .{
        .symbol = "B",
        .bucket = "S",
        .tax_pct = 0,
        .weight = 1.0,
        .return_1y = null,
        .return_3y = null,
        .return_5y = null,
        .return_10y = null,
        .vol_3y = null,
        .vol_10y = null,
        .sharpe_3y = null,
        .sharpe_10y = null,
        .maxdd_5y = null,
    } };
    var state: State = .{
        .cursor = 1,
        .view = .{
            .rows = &rows,
            .totals = std.mem.zeroes(review_view.ReviewTotals),
            .as_of = zfin.Date.fromYmd(2026, 6, 8),
            .total_liquid = 0,
            .portfolio_path = "x",
        },
    };
    try testing.expectEqual(CursorSection.holdings, cursorSection(&state));
    try testing.expectEqual(@as(usize, 1), cursorLocalIndex(&state));
    state.view = null;
}

test "cursorSection: findings when cursor >= holdings_len" {
    var findings = [_]observations_view.FindingRow{
        .{ .severity = .warn, .kind = "k", .target = "t", .text = "x", .is_acked = false },
    };
    const empty_view: review_view.ReviewView = .{
        .rows = &.{},
        .totals = std.mem.zeroes(review_view.ReviewTotals),
        .as_of = zfin.Date.fromYmd(2026, 6, 8),
        .total_liquid = 0,
        .portfolio_path = "x",
    };
    var state: State = .{
        .view = empty_view,
        .cursor = 0,
        .findings_view = .{
            .rows = &findings,
            .total_active = 1,
            .total_acked = 0,
            .total_resolved = 0,
        },
    };
    try testing.expectEqual(CursorSection.findings, cursorSection(&state));
    try testing.expectEqual(@as(usize, 0), cursorLocalIndex(&state));
    state.view = null;
    state.findings_view = null;
}

// ── unack action ──────────────────────────────────────────────

test "unackCurrentFinding: cursor in holdings emits status hint" {
    // Holdings row exists; cursor on it. unack should set a
    // status message and not touch any journal.
    var rows = [_]review_view.ReviewRow{.{
        .symbol = "X",
        .bucket = "S",
        .tax_pct = 0,
        .weight = 1.0,
        .return_1y = null,
        .return_3y = null,
        .return_5y = null,
        .return_10y = null,
        .vol_3y = null,
        .vol_10y = null,
        .sharpe_3y = null,
        .sharpe_10y = null,
        .maxdd_5y = null,
    }};
    var state: State = .{
        .cursor = 0,
        .view = .{
            .rows = &rows,
            .totals = std.mem.zeroes(review_view.ReviewTotals),
            .as_of = zfin.Date.fromYmd(2026, 6, 8),
            .total_liquid = 0,
            .portfolio_path = "x",
        },
    };
    var app: App = undefined;
    app.status_len = 0;
    unackCurrentFinding(&state, &app);
    // Status was set (we don't pin the exact text — just that
    // something was written). app.status_len is the source of
    // truth; without setStatus called it stays 0.
    try testing.expect(app.status_len > 0);

    state.view = null;
}

test "unackCurrentFinding: cursor on un-acked finding emits status hint" {
    var findings = [_]observations_view.FindingRow{
        .{ .severity = .warn, .kind = "k", .target = "t", .text = "x", .is_acked = false },
    };
    const empty_view: review_view.ReviewView = .{
        .rows = &.{},
        .totals = std.mem.zeroes(review_view.ReviewTotals),
        .as_of = zfin.Date.fromYmd(2026, 6, 8),
        .total_liquid = 0,
        .portfolio_path = "x",
    };
    var state: State = .{
        .view = empty_view,
        .cursor = 0,
        .findings_view = .{
            .rows = &findings,
            .total_active = 1,
            .total_acked = 0,
            .total_resolved = 0,
        },
    };
    var app: App = undefined;
    app.status_len = 0;
    unackCurrentFinding(&state, &app);
    try testing.expect(app.status_len > 0);

    state.view = null;
    state.findings_view = null;
}

// ── select_symbol action ──────────────────────────────────────
//
// We can't unit-test the full path because `app.setActiveSymbol`
// dispatches `onSymbolChange` across all tab modules and needs a
// fully wired App. The empty-state guard is the testable slice;
// full-flow coverage comes from the integration tests / dogfood.

test "selectSymbolAtCursor: empty state is a no-op (no setActiveSymbol)" {
    var state: State = .{};
    var app: App = undefined;
    app.status_len = 0;
    selectSymbolAtCursor(&state, &app);
    try testing.expectEqual(@as(usize, 0), app.status_len);
}
