From 14f55afb283f2beec4ad3241e0d60b05d3217249 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Wed, 10 Jun 2026 10:57:25 -0700 Subject: [PATCH] update review tab UX/introduce shift+mousewheel for scrolling --- src/tui.zig | 20 +- src/tui/portfolio_tab.zig | 14 +- src/tui/review_tab.zig | 1145 ++++++++++++++++++++++++++++--------- 3 files changed, 912 insertions(+), 267 deletions(-) diff --git a/src/tui.zig b/src/tui.zig index aaff92f..ae19b36 100644 --- a/src/tui.zig +++ b/src/tui.zig @@ -593,11 +593,27 @@ pub const App = struct { switch (mouse.button) { .wheel_up => { - self.wheelBy(-3); + // shift+wheel scrolls the viewport (look-around, + // no cursor change). Plain wheel routes through + // `wheelBy` which prefers the active tab's + // `onCursorMove`/`onWheelMove`. We pick shift + // (not ctrl) because ctrl+wheel is the + // near-universal "zoom" gesture in browsers and + // editors — bending that to "scroll" would + // confuse muscle memory. + if (mouse.mods.shift) { + self.scrollViewportBy(-3); + } else { + self.wheelBy(-3); + } return ctx.consumeAndRedraw(); }, .wheel_down => { - self.wheelBy(3); + if (mouse.mods.shift) { + self.scrollViewportBy(3); + } else { + self.wheelBy(3); + } return ctx.consumeAndRedraw(); }, .left => { diff --git a/src/tui/portfolio_tab.zig b/src/tui/portfolio_tab.zig index 5a78e26..3723e24 100644 --- a/src/tui/portfolio_tab.zig +++ b/src/tui/portfolio_tab.zig @@ -456,16 +456,10 @@ pub const tab = struct { return true; } - /// Mouse wheel: always scroll the viewport, never move the - /// cursor. Keeps wheel-as-look-around and cursor-as-pointer - /// distinct. The framework falls through to viewport scroll - /// when this returns false. - pub fn onWheelMove(state: *State, app: *App, delta: isize) bool { - _ = state; - _ = app; - _ = delta; - return false; - } + // No `onWheelMove`: by omitting it, wheel events fall through + // to the framework's `onCursorMove` path. Wheel moves the + // cursor like j/k. PageUp / PageDown / Home / End scroll the + // viewport. /// Pre-empt key handler. Called by the framework BEFORE /// global keymap matching runs. When portfolio is in a diff --git a/src/tui/review_tab.zig b/src/tui/review_tab.zig index bb4ebbe..21267da 100644 --- a/src/tui/review_tab.zig +++ b/src/tui/review_tab.zig @@ -41,27 +41,28 @@ pub const Action = enum { sort_col_prev, /// Flip the current sort direction (asc ↔ desc). sort_reverse, - /// Move keyboard focus to the holdings table. - focus_holdings, - /// Move keyboard focus to the findings table. - focus_findings, - /// Toggle expansion of the cursor-selected finding (findings - /// table only). No-op if focus is the holdings table. + /// Toggle expansion of the cursor-selected finding (no-op when + /// the cursor is on a holdings row). toggle_expand, - /// Acknowledge the cursor-selected finding (findings table - /// only). Appends an `acknowledged` entry to the journal and - /// rebuilds the findings view. No-op if focus is the holdings - /// table. + /// 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, -}; - -/// Which table currently receives cursor-movement keystrokes. -pub const FocusTarget = enum { - holdings, - findings, + /// 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, }; /// Modal sub-state. `.normal` lets keystrokes flow through the @@ -75,6 +76,18 @@ pub const InputMode = enum { 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 { @@ -126,20 +139,22 @@ pub const State = struct { /// by `toggle_show_acked` (`v` key by default). show_acked: bool = false, - // ── Two-cursor focus model ─────────────────────────────── + // ── Single-cursor (single-sheet) navigation ────────────── - /// Which table currently receives cursor-movement keystrokes. - focus: FocusTarget = .holdings, - /// Holdings-table cursor (row index into `view.rows`). Persists - /// across focus toggles so jumping between tables doesn't lose - /// place. - holdings_cursor: usize = 0, - /// Findings-table cursor (row index into `findings_view.rows`). - /// Persists across focus toggles. - findings_cursor: usize = 0, - /// Currently expanded finding-row index (or null when none is - /// expanded). The expansion is rendered inline in the findings - /// table; only one finding can be expanded at a time. + /// 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) ────────────────── @@ -170,31 +185,30 @@ pub const meta: framework.TabMeta(Action) = .{ .{ .action = .sort_col_next, .key = .{ .codepoint = '>' } }, .{ .action = .sort_col_prev, .key = .{ .codepoint = '<' } }, .{ .action = .sort_reverse, .key = .{ .codepoint = 'o' } }, - // `[` and `]` flip focus between the holdings and findings - // tables. Single-key alternatives (`h`/`l`, `Tab`) collide - // with global tab navigation; brackets are unused and - // visually evoke "swap left/right pane". - .{ .action = .focus_holdings, .key = .{ .codepoint = '[' } }, - .{ .action = .focus_findings, .key = .{ .codepoint = ']' } }, .{ .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 } }, }, .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", - .focus_holdings = "Focus: holdings table", - .focus_findings = "Focus: findings table", .toggle_expand = "Expand/collapse finding", .ack = "Acknowledge finding", + .unack = "Un-acknowledge finding", .toggle_show_acked = "Toggle show acked findings", + .select_symbol = "Select symbol", }), .status_hints = &.{ - .focus_holdings, - .focus_findings, .ack, + .unack, .toggle_show_acked, + .select_symbol, .sort_col_prev, .sort_col_next, }, @@ -274,28 +288,24 @@ pub const tab = struct { state.sort_dir = if (state.sort_dir == .asc) .desc else .asc; applySort(state); }, - .focus_holdings => { - state.focus = .holdings; - state.expanded_finding = null; - }, - .focus_findings => { - state.focus = .findings; - }, .toggle_expand => { - if (state.focus != .findings) return; + if (cursorSection(state) != .findings) return; const fv = state.findings_view orelse return; if (fv.rows.len == 0) return; - if (state.expanded_finding == state.findings_cursor) { + const local = cursorLocalIndex(state); + if (state.expanded_finding == local) { state.expanded_finding = null; } else { - state.expanded_finding = state.findings_cursor; + 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), } } @@ -329,8 +339,7 @@ pub const tab = struct { if (state.journal) |*j| j.deinit(); state.journal = null; state.loaded = false; - state.holdings_cursor = 0; - state.findings_cursor = 0; + state.cursor = 0; state.expanded_finding = null; if (app.active_tab == .review) { tab.activate(state, app) catch |err| std.log.debug("review activate failed: {t}", .{err}); @@ -344,9 +353,11 @@ pub const tab = struct { /// columns — best/worst-first matches the typical "show me /// the leaders" reading). /// - /// Click on a holdings data row → focus holdings + move - /// cursor to that row. Click on a findings data row → focus - /// findings + move cursor to that finding's index. + /// 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). @@ -364,25 +375,23 @@ pub const tab = struct { return true; } - // Holdings data-row click. + // 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.focus = .holdings; - state.holdings_cursor = content_row - state.holdings_first_row; + 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 falls into this range - // too — we treat that as "focus findings, leave cursor - // alone" so the user doesn't get teleported when they - // click a note line by accident. + // 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) { - state.focus = .findings; 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 @@ -396,14 +405,10 @@ pub const tab = struct { 0; const next_visual_row = visual_row + 1 + expansion; if (content_row >= visual_row and content_row < next_visual_row) { - state.findings_cursor = i; - // Clicking a different row collapses any prior - // expansion. Mirrors `onCursorMove`'s "single - // expansion at a time" invariant; without this, - // a stale `expanded_finding` keeps rendering - // detail/note lines for the *old* row index, - // which now points at a different finding after - // any rebuild that shifted indices. + 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; } @@ -417,45 +422,45 @@ pub const tab = struct { return false; } - /// j/k/up/down route to the focused table's cursor. Returns - /// true on success so the framework consumes the event; - /// returns false when there's no row to move to (empty table) - /// so the framework falls through to viewport scroll. + /// 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 { - _ = app; - switch (state.focus) { - .holdings => { - const view = state.view orelse return false; - if (view.rows.len == 0) return false; - state.holdings_cursor = clampCursor(state.holdings_cursor, delta, view.rows.len); - return true; - }, - .findings => { - const fv = state.findings_view orelse return false; - if (fv.rows.len == 0) return false; - state.findings_cursor = clampCursor(state.findings_cursor, delta, fv.rows.len); - // Collapsing the expansion when the cursor moves - // off the previously-expanded row keeps the - // single-expansion invariant. Users can re-expand - // with Enter. - if (state.expanded_finding) |exp| { - if (exp != state.findings_cursor) state.expanded_finding = null; - } - return true; - }, + 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; } - /// Wheel events scroll the viewport, never the cursor. Returns - /// false unconditionally so the framework falls through to its - /// scroll-by-delta path. Required for the multi-cursor review - /// tab; see framework docs. - pub fn onWheelMove(state: *State, app: *App, delta: isize) bool { - _ = state; - _ = app; - _ = delta; - return false; - } + // 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 @@ -544,19 +549,112 @@ fn applySort(state: *State) void { review_view.sortRows(view.rows, state.sort_field, state.sort_dir); } -/// Clamp `current + delta` into `[0, len-1]`. Used by `onCursorMove` -/// to safely translate j/k presses into a new cursor index. Caller -/// should have already checked `len > 0`. -fn clampCursor(current: usize, delta: isize, len: usize) usize { - std.debug.assert(len > 0); - const max_idx = len - 1; - if (delta < 0) { - const abs_delta: usize = @intCast(-delta); - return if (abs_delta >= current) 0 else current - abs_delta; - } else { - const abs_delta: usize = @intCast(delta); - const proposed = current +| abs_delta; - return @min(proposed, max_idx); +/// 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; + }, } } @@ -590,22 +688,23 @@ fn inputBarLineCount(state: *const State) usize { /// 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 (state.focus != .findings) { - app.setStatus("Switch to findings table (]) before acknowledging"); + 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; - if (state.findings_cursor >= fv.rows.len) return; + const local = cursorLocalIndex(state); + if (local >= fv.rows.len) return; - const row = fv.rows[state.findings_cursor]; + const row = fv.rows[local]; if (row.is_acked) { - app.setStatus("Already acknowledged"); + app.setStatus("Already acknowledged (press U to un-acknowledge)"); return; } // Auto-expand so the inline input bar has somewhere to render. - state.expanded_finding = state.findings_cursor; + state.expanded_finding = local; state.input_mode = .ack_note; state.note_len = 0; // `note_fragments` should already be empty (we only enter @@ -615,6 +714,77 @@ fn ackCurrentFinding(state: *State, app: *App) void { 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); +} + /// 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); @@ -641,8 +811,10 @@ fn commitAckNote(state: *State, app: *App) void { } const fv = state.findings_view orelse return; - if (fv.rows.len == 0 or state.findings_cursor >= fv.rows.len) return; - const row = fv.rows[state.findings_cursor]; + 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"); @@ -828,14 +1000,15 @@ fn rebuildFindingsView(state: *State, app: *App) void { return; }; - // Clamp the cursor in case `show_acked` toggling or an ack shrunk - // the row count beneath the previous cursor position. - if (state.findings_view) |fv| { - if (fv.rows.len == 0) { - state.findings_cursor = 0; - } else if (state.findings_cursor >= fv.rows.len) { - state.findings_cursor = fv.rows.len - 1; - } + // 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; } } @@ -1002,10 +1175,13 @@ const RowBuilder = struct { /// `cell()` after each append. col: usize = 0, - /// Append the leading row-prefix indent (matches " " at the - /// start of every line in the file). - fn prefix(self: *RowBuilder) !void { - try self.text.appendSlice(self.arena, " "); + /// 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; } @@ -1122,10 +1298,18 @@ pub fn buildStyledLines(state: *State, app: *App, arena: std.mem.Allocator) ![]c .style = th.dimStyle(), }); - // Rows. + // 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; - for (view.rows) |r| { - try lines.append(arena, try formatRow(arena, th, r)); + 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. @@ -1150,7 +1334,7 @@ pub fn buildStyledLines(state: *State, app: *App, arena: std.mem.Allocator) ![]c // 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); + try appendFindingsSection(arena, &lines, state, fv, th, app.symbol); } return lines.toOwnedSlice(arena); @@ -1310,15 +1494,16 @@ fn appendStatusCell( /// ← per row /// ← acked rows shown only when state.show_acked /// -/// Cursor highlight: when `state.focus == .findings` and the row -/// index matches `state.findings_cursor`, the row gets the theme's -/// selected-style applied. +/// 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() }); @@ -1350,8 +1535,23 @@ fn appendFindingsSection( const start_count = lines.items.len; for (fv.rows, 0..) |row, i| { - const is_cursor = state.focus == .findings and i == state.findings_cursor; - const text = try std.fmt.allocPrint(arena, " {s} {s}{s}", .{ + // 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, @@ -1445,7 +1645,7 @@ fn buildHeaderLine( sort_dir: review_view.SortDirection, ) !StyledLine { var rb: RowBuilder = .{ .arena = arena, .th = th }; - try rb.prefix(); + 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; @@ -1532,9 +1732,11 @@ 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(); + try rb.prefix(is_active); var pct_buf: [16]u8 = undefined; @@ -1581,7 +1783,12 @@ fn formatRow( var tax: [16]u8 = undefined; try rb.cellRight(fmt.fmtPctOpt(&tax, r.tax_pct, .{}), col_tax, .muted); - return rb.build(th.contentStyle()); + // 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( @@ -1590,7 +1797,7 @@ fn formatTotalsRow( t: review_view.ReviewTotals, ) !StyledLine { var rb: RowBuilder = .{ .arena = arena, .th = th }; - try rb.prefix(); + try rb.prefix(false); try rb.cellLeft("Total", col_symbol, .normal); try rb.gap(); @@ -1745,7 +1952,7 @@ test "formatRow: produces a styled line containing symbol + sector" { .sharpe_10y = 0.85, .maxdd_5y = 0.25, }; - const line = try formatRow(arena, theme.default_theme, r); + 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); @@ -1771,7 +1978,7 @@ test "formatRow: nulls render as em-dashes" { .sharpe_10y = null, .maxdd_5y = null, }; - const line = try formatRow(arena, theme.default_theme, r); + const line = try formatRow(arena, theme.default_theme, r, false, false); try testing.expect(std.mem.count(u8, line.text, "—") >= 8); } @@ -1795,11 +2002,40 @@ test "formatRow: abbreviates Communication Services" { .sharpe_10y = null, .maxdd_5y = null, }; - const line = try formatRow(arena, theme.default_theme, r); + 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", + .sector_mid = "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(); @@ -2108,7 +2344,7 @@ test "appendFindingsSection: empty findings with no acked produces 'No findings. }; var state: State = .{}; - try appendFindingsSection(arena, &lines, &state, fv, theme.default_theme); + try appendFindingsSection(arena, &lines, &state, fv, theme.default_theme, ""); try testing.expect(lines.items.len >= 4); var found_no_findings = false; @@ -2132,7 +2368,7 @@ test "appendFindingsSection: hidden-acked hint when all findings are acked" { }; var state: State = .{ .show_acked = false }; - try appendFindingsSection(arena, &lines, &state, fv, theme.default_theme); + try appendFindingsSection(arena, &lines, &state, fv, theme.default_theme, ""); var hint_found = false; for (lines.items) |ln| { @@ -2147,19 +2383,33 @@ test "appendFindingsSection: renders cursor highlight on focused row" { const arena = arena_state.allocator(); var lines: std.ArrayList(StyledLine) = .empty; - const rows = [_]observations_view.FindingRow{ + 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 = @constCast(&rows), + .rows = &rows, .total_active = 2, .total_acked = 0, .total_resolved = 0, }; - var state: State = .{ .focus = .findings, .findings_cursor = 1 }; + // 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); + 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. @@ -2173,18 +2423,36 @@ test "appendFindingsSection: renders cursor highlight on focused row" { 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 "clampCursor: forward delta clamps at len-1" { - try testing.expectEqual(@as(usize, 4), clampCursor(2, 5, 5)); - try testing.expectEqual(@as(usize, 0), clampCursor(0, 0, 5)); - try testing.expectEqual(@as(usize, 4), clampCursor(4, 1, 5)); +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 "clampCursor: backward delta clamps at 0" { - try testing.expectEqual(@as(usize, 0), clampCursor(2, -5, 5)); - try testing.expectEqual(@as(usize, 0), clampCursor(0, -1, 5)); - try testing.expectEqual(@as(usize, 1), clampCursor(2, -1, 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)" { @@ -2249,34 +2517,29 @@ test "clearNoteFragments: frees fragment slices" { try testing.expectEqual(@as(usize, 0), state.note_fragments.items.len); } -// ── handleAction: new M2 actions (no App access) ─────────────── - -test "handleAction: focus_findings sets focus" { - var state: State = .{}; - var app: App = undefined; - tab.handleAction(&state, &app, .focus_findings); - try testing.expectEqual(FocusTarget.findings, state.focus); -} - -test "handleAction: focus_holdings clears expansion" { - var state: State = .{ .focus = .findings, .expanded_finding = 2 }; - var app: App = undefined; - tab.handleAction(&state, &app, .focus_holdings); - try testing.expectEqual(FocusTarget.holdings, state.focus); - try testing.expect(state.expanded_finding == null); -} +// ── 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. - const rows = [_]observations_view.FindingRow{ + // 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 = .{ - .focus = .findings, - .findings_cursor = 0, + .view = empty_view, + .cursor = 0, .findings_view = .{ - .rows = @constCast(&rows), + .rows = &rows, .total_active = 1, .total_acked = 0, .total_resolved = 0, @@ -2291,14 +2554,25 @@ test "handleAction: toggle_expand on findings cursor toggles expanded_finding" { tab.handleAction(&state, &app, .toggle_expand); try testing.expect(state.expanded_finding == null); - // findings_view is borrowed in the test (rows are stack-allocated), - // so we must clear the field before deinit() to avoid a double-free. + // 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 = .{ - .focus = .findings, + .view = empty_view, .findings_view = .{ .rows = &.{}, .total_active = 0, @@ -2309,44 +2583,33 @@ test "handleAction: toggle_expand on empty findings is no-op" { 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 from holdings focus is no-op" { - var state: State = .{ .focus = .holdings }; - var app: App = undefined; - tab.handleAction(&state, &app, .toggle_expand); - try testing.expect(state.expanded_finding == null); -} - -// ── onCursorMove: routes to focused table ────────────────────── - -test "onCursorMove: holdings focus moves holdings_cursor" { - // Build a minimal ReviewView with 5 rows. - const rows = try testing.allocator.alloc(review_view.ReviewRow, 5); - defer testing.allocator.free(rows); - for (rows, 0..) |*r, i| { - r.* = .{ - .symbol = "X", - .sector_mid = "Other", - .tax_pct = 0, - .weight = 0.2, - .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; - } +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", + .sector_mid = "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 = .{ - .focus = .holdings, - .holdings_cursor = 0, + .cursor = 0, .view = .{ - .rows = rows, + .rows = &rows, .totals = std.mem.zeroes(review_view.ReviewTotals), .as_of = zfin.Date.fromYmd(2026, 6, 8), .total_liquid = 0, @@ -2354,63 +2617,265 @@ test "onCursorMove: holdings focus moves holdings_cursor" { }, }; var app: App = undefined; - try testing.expect(tab.onCursorMove(&state, &app, 2)); - try testing.expectEqual(@as(usize, 2), state.holdings_cursor); - try testing.expect(tab.onCursorMove(&state, &app, 100)); // clamps - try testing.expectEqual(@as(usize, 4), state.holdings_cursor); + tab.handleAction(&state, &app, .toggle_expand); + try testing.expect(state.expanded_finding == null); - // Borrowed view; clear before drop to avoid double-free. state.view = null; } -test "onCursorMove: findings focus moves findings_cursor and clears stale expansion" { - const rows = [_]observations_view.FindingRow{ +// ── 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", + .sector_mid = "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", + .sector_mid = "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", + .sector_mid = "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", + .sector_mid = "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 = .{ - .focus = .findings, - .findings_cursor = 0, + .view = empty_view, + .cursor = 0, // first finding (no holdings) .expanded_finding = 0, .findings_view = .{ - .rows = @constCast(&rows), + .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.findings_cursor); + 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" { - var state: State = .{ .focus = .findings, .findings_view = .{ + 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; } -// ── onWheelMove: always returns false ────────────────────────── - -test "onWheelMove: always returns false" { - var state: State = .{}; +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; - try testing.expect(!tab.onWheelMove(&state, &app, 1)); - try testing.expect(!tab.onWheelMove(&state, &app, -3)); + 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 cursor + focus" { +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| { @@ -2432,7 +2897,9 @@ test "handleMouse: click on holdings row sets cursor + focus" { _ = i; } var state: State = .{ - .focus = .findings, + // Pre-existing cursor on the second finding (will be + // moved by the click). + .cursor = 100, .holdings_first_row = 5, .header_row = 4, .view = .{ @@ -2454,14 +2921,14 @@ test "handleMouse: click on holdings row sets cursor + focus" { .mods = .{}, }); try testing.expect(consumed); - try testing.expectEqual(FocusTarget.holdings, state.focus); - try testing.expectEqual(@as(usize, 1), state.holdings_cursor); + 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 cursor + focus" { - const rows = [_]observations_view.FindingRow{ +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 }, }; @@ -2475,12 +2942,12 @@ test "handleMouse: click on findings row sets cursor + focus" { .portfolio_path = "x", }; var state: State = .{ - .focus = .holdings, + .cursor = 0, // start on first holding (none here) .view = empty_view, .findings_first_row = 10, .findings_row_count = 2, .findings_view = .{ - .rows = @constCast(&rows), + .rows = &rows, .total_active = 2, .total_acked = 0, .total_resolved = 0, @@ -2497,8 +2964,11 @@ test "handleMouse: click on findings row sets cursor + focus" { .mods = .{}, }); try testing.expect(consumed); - try testing.expectEqual(FocusTarget.findings, state.focus); - try testing.expectEqual(@as(usize, 1), state.findings_cursor); + // 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; @@ -2643,3 +3113,168 @@ test "appendStatusGrid: empty panel produces no lines" { 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", + .sector_mid = "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", + .sector_mid = "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", + .sector_mid = "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); +}