diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 58b8316..7d50b07 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -29,7 +29,7 @@ repos: - id: test name: Run zig build test entry: zig - args: ["build", "coverage", "-Dcoverage-threshold=65"] + args: ["build", "coverage", "-Dcoverage-threshold=69"] language: system types: [file] pass_filenames: false diff --git a/TODO.md b/TODO.md index 7561fc1..4162a90 100644 --- a/TODO.md +++ b/TODO.md @@ -264,71 +264,49 @@ renaming to `src/render.zig` to better describe what's left, or splitting the braille chart out (it's ~600 lines on its own). Not blocking — file it as cleanup if and when it bites. -## Bug: braille charts use raw `close`, not `adj_close` — cliff at splits +### `projections --vs ` doesn't support imported-only as-of dates — priority MEDIUM -**Reproduction:** `zfin quote SOXX` (or the TUI quote tab). The -braille chart drops sharply on **2024-03-07**, which is the -iShares Semiconductor ETF's 3-for-1 split date: +The crash that used to happen when `--vs ` resolved to an +imported-only date is fixed: `loadAsOfContext` now branches on +`resolution.source` and emits a graceful "no snapshot at that +date" error instead of panicking with `FileNotFound`. But the +feature itself is still missing - back-dating a `--vs` comparison +to a date that's only covered by `imported_values.srf` (no real +snapshot) is rejected outright. - - 2024-03-06 close: $689.60 - - 2024-03-07 close: $237.75 (≈ $689.60 / 2.9) +The `runBands` path (`projections --as-of `) +handles the imported-only case by loading today's portfolio +composition + scaling to the imported liquid total, then calling +`view.loadProjectionContextFromImported`. `loadAsOfContext` +needs the same plumbing - but as outparams, since the caller +(`computeKeyComparison`) needs to own `live_loaded` and +`live_pf_data` for the lifetime of the returned context. -The `adj_close` column in `~/.cache/zfin/SOXX/candles_daily.srf` -tracks correctly through the split (~$226 → ~$234), so the -provider data is fine. The bug is purely cosmetic: the chart -renders the *unadjusted* close price. +Two implementation shapes: -**Root cause:** `computeBrailleChart` in `src/format.zig:888` -indexes `data[i].close` instead of `data[i].adj_close`. Lines 901, -902, 904, 905, 935 all use `.close`. +A. **Add outparams to `loadAsOfContext`.** New + `live_loaded_out: *?cli.LoadedPortfolio` and + `live_pf_data_out: *?cli.PortfolioData` parameters. Caller + declares them and `defer`s their deinit. ~30 lines, but + duplicates the imported-only loading code (already lives in + `runBands`'s `else` branch around line 392-429 of + `src/commands/projections.zig`). -**Independent confirmation:** `zfin splits SOXX` returns -`2024-03-07 3:1` from Polygon. So the split data exists in the -provider layer (and gets cached as `splits.srf` once requested), -but the charting code path doesn't consult it. +B. **Extract a shared helper.** Pull the snapshot-vs-imported + branching from both `runBands` and `loadAsOfContext` into one + `loadProjectionContextForResolution` that returns a + discriminated union (snapshot ctx with snap_bundle owned, or + imported ctx with live_loaded + live_pf_data owned). Both + call sites use it. ~60 lines but eliminates the duplication + that AGENTS.md warns about (the two paths drifting causes + `compare --projections` to disagree with standalone + `projections --as-of`). -**Fix candidates:** - -A. **Switch `computeBrailleChart` to consume `adj_close` directly.** - Simplest. Affects every chart caller (quote, history, projections - median band, TUI quote/projections tabs). Cosmetic only — no - computation depends on it. The price-axis labels would render - adjusted prices, which may surprise users used to seeing the - raw last-close. Mitigate with a comment in the chart's right-edge - label region or a header note. - -B. **Pass a flag to `computeBrailleChart` selecting `close` vs - `adj_close`.** Default to adjusted; let the quote tab show raw. - More flexible, marginally more code. - -C. **Add a `chart_close` accessor to `Candle` that returns - `adj_close` if non-zero, else `close`.** Same effect as (A) with a - defensive fallback. - -D. **Apply split adjustments at chart-data prep time using - `splits.srf`.** Walk the candle slice with the split history and - pre-multiply pre-split closes by the cumulative ratio. More - work, but produces a chart-axis dollar value the user expects - ("today's last close was $X, the chart starts at $Y from N - years ago"). This is what most charting libraries do. - Requires plumbing `DataService.getSplits` into the chart-prep - path on every chart caller, OR doing the adjustment once in the - service layer alongside candle fetching. Not all callers have a - `DataService` reference today (e.g., `runProjection`'s synthetic - median-band candles). - -**Recommendation:** Start with (A) or (C) — single-line fix, gets -the cliff out of all charts immediately. (D) is the "correct" fix -but a bigger refactor; file as a follow-up if (A)/(C) lands first. - -**Other affected symbols:** Any held position with a split in the -last 10 years will have the same artifact. Check NVDA (10:1 split -on 2024-06-10) for a louder example. - -**Priority:** LOW. Cosmetic only — analytics already use -`adj_close` correctly via the per-position trailing-returns path. -But it's confusing when scanning a chart and seeing a 50% drop -that isn't real. +Recommendation: B. The duplication risk is real - the +`computeKeyComparison` doc-block already calls out that "if you +change inputs to either of these loaders, change them in BOTH +places." Adding a third copy of the imported-only loader code +makes that worse. ## Audit: manual-check accounts mechanism — priority HIGH diff --git a/src/analytics/indicators.zig b/src/analytics/indicators.zig index 4588459..bc0d918 100644 --- a/src/analytics/indicators.zig +++ b/src/analytics/indicators.zig @@ -144,10 +144,15 @@ pub fn rsi( return result; } -/// Extract close prices from candles into a contiguous f64 slice. +/// Extract chart-ready close prices from candles into a contiguous f64 slice. +/// Uses `Candle.chartClose()` (split-adjusted when available) so chart +/// renderers don't show false cliffs at split dates. The only callers +/// today are chart code paths in `tui/chart.zig`; if a future caller +/// genuinely needs raw `close`, add a separate `rawClosePrices` helper +/// rather than re-purposing this one. pub fn closePrices(alloc: std.mem.Allocator, candles: []const Candle) ![]f64 { const result = try alloc.alloc(f64, candles.len); - for (candles, 0..) |c, i| result[i] = c.close; + for (candles, 0..) |c, i| result[i] = c.chartClose(); return result; } diff --git a/src/format.zig b/src/format.zig index e56c336..af29a91 100644 --- a/src/format.zig +++ b/src/format.zig @@ -863,12 +863,15 @@ pub fn computeBrailleChart( const dot_rows: usize = chart_height * 4; // vertical dot resolution - // Find min/max close prices - var min_price: f64 = data[0].close; - var max_price: f64 = data[0].close; + // Find min/max chart-close prices (split-adjusted when available). + // See `Candle.chartClose` — using raw `close` here would render + // false cliffs at split dates. + var min_price: f64 = data[0].chartClose(); + var max_price: f64 = data[0].chartClose(); for (data) |d| { - if (d.close < min_price) min_price = d.close; - if (d.close > max_price) max_price = d.close; + const cc = d.chartClose(); + if (cc < min_price) min_price = cc; + if (cc > max_price) max_price = cc; } if (max_price == min_price) max_price = min_price + 1.0; const price_range = max_price - min_price; @@ -896,7 +899,7 @@ pub fn computeBrailleChart( for (0..n_cols) |col| { const data_idx_f: f64 = @as(f64, @floatFromInt(col)) * @as(f64, @floatFromInt(data.len - 1)) / @as(f64, @floatFromInt(n_cols - 1)); const data_idx: usize = @min(@as(usize, @intFromFloat(data_idx_f)), data.len - 1); - const close = data[data_idx].close; + const close = data[data_idx].chartClose(); const norm = (close - min_price) / price_range; // 0 = min, 1 = max // Inverted: 0 = top dot row, dot_rows-1 = bottom const y_f = (1.0 - norm) * @as(f64, @floatFromInt(dot_rows - 1)); @@ -1349,6 +1352,29 @@ test "computeBrailleChart insufficient data" { try std.testing.expectError(error.InsufficientData, result); } +test "computeBrailleChart uses adj_close to avoid split cliff" { + // Regression: SOXX 3:1 on 2024-03-07 used to render a sharp drop + // because the chart consumed raw `close` instead of `adj_close`. + // Build a synthetic 4-candle slice that mimics a 3:1 split: raw + // close drops 300 → 100, but adj_close is constant at 100. The + // chart should see a flat line, not a cliff. + const alloc = std.testing.allocator; + const candles = [_]Candle{ + .{ .date = Date.fromYmd(2024, 3, 5), .open = 300, .high = 300, .low = 300, .close = 300, .adj_close = 100, .volume = 1000 }, + .{ .date = Date.fromYmd(2024, 3, 6), .open = 300, .high = 300, .low = 300, .close = 300, .adj_close = 100, .volume = 1000 }, + .{ .date = Date.fromYmd(2024, 3, 7), .open = 100, .high = 100, .low = 100, .close = 100, .adj_close = 100, .volume = 1000 }, + .{ .date = Date.fromYmd(2024, 3, 8), .open = 100, .high = 100, .low = 100, .close = 100, .adj_close = 100, .volume = 1000 }, + }; + var chart = try computeBrailleChart(alloc, &candles, 20, 4, .{ 0x7f, 0xd8, 0x8f }, .{ 0xe0, 0x6c, 0x75 }); + defer chart.deinit(alloc); + // Min and max labels should reflect the adjusted price (~$100), + // not the raw close range (300 → 100). The exact values vary + // because computeBrailleChart bumps max by $1 internally when + // min == max, but neither label should mention $300. + try std.testing.expect(std.mem.indexOf(u8, chart.maxLabel(), "300") == null); + try std.testing.expect(std.mem.indexOf(u8, chart.minLabel(), "300") == null); +} + test "fmtContractLine" { var buf: [128]u8 = undefined; const contract = OptionContract{ diff --git a/src/models/candle.zig b/src/models/candle.zig index 508fbea..9f346a7 100644 --- a/src/models/candle.zig +++ b/src/models/candle.zig @@ -11,4 +11,71 @@ pub const Candle = struct { /// If the provider does not supply this, it equals `close`. adj_close: f64, volume: u64, + + /// Close value to use when rendering charts. Returns `adj_close` + /// if it is non-zero, otherwise falls back to raw `close`. + /// + /// Why: charts should render split-adjusted prices so the line + /// doesn't show artificial cliffs at split dates (e.g. SOXX + /// 3-for-1 on 2024-03-07, NVDA 10-for-1 on 2024-06-10). Some + /// older cached candles or providers that don't populate + /// `adj_close` leave it at 0; in that case the raw close is + /// the best we can do. + pub fn chartClose(self: Candle) f64 { + return if (self.adj_close != 0) self.adj_close else self.close; + } }; + +test "chartClose prefers adj_close when populated" { + const std = @import("std"); + const c = Candle{ + .date = Date.fromYmd(2024, 3, 7), + .open = 700.0, + .high = 700.0, + .low = 230.0, + .close = 237.75, // raw post-split + .adj_close = 234.0, // split-adjusted + .volume = 0, + }; + try std.testing.expectEqual(@as(f64, 234.0), c.chartClose()); +} + +test "chartClose falls back to close when adj_close is zero" { + const std = @import("std"); + const c = Candle{ + .date = Date.fromYmd(2024, 1, 1), + .open = 100.0, + .high = 100.0, + .low = 100.0, + .close = 100.0, + .adj_close = 0, + .volume = 0, + }; + try std.testing.expectEqual(@as(f64, 100.0), c.chartClose()); +} + +test "chartClose synthetic split has continuous chart values" { + const std = @import("std"); + // Pre-split: close=300, adj_close=100 (after a 3:1 split) + // Post-split: close=100, adj_close=100 + // Chart should see [100, 100] not [300, 100]. + const pre = Candle{ + .date = Date.fromYmd(2024, 3, 6), + .open = 300, + .high = 300, + .low = 300, + .close = 300, + .adj_close = 100, + .volume = 0, + }; + const post = Candle{ + .date = Date.fromYmd(2024, 3, 7), + .open = 100, + .high = 100, + .low = 100, + .close = 100, + .adj_close = 100, + .volume = 0, + }; + try std.testing.expectEqual(pre.chartClose(), post.chartClose()); +} diff --git a/src/tui.zig b/src/tui.zig index 5718405..24bf0a2 100644 --- a/src/tui.zig +++ b/src/tui.zig @@ -2338,7 +2338,7 @@ test "glyph non-ASCII returns space" { test "Tab label" { try testing.expectEqualStrings(" 1:Portfolio ", tabLabel(.portfolio)); - try testing.expectEqualStrings(" 6:Analysis ", tabLabel(.analysis)); + try testing.expectEqualStrings(" 2:Analysis ", tabLabel(.analysis)); } test "buildHelpLines: header, global section, active-tab section, footer" { diff --git a/src/tui/chart.zig b/src/tui/chart.zig index e4c89f2..c6a2acb 100644 --- a/src/tui/chart.zig +++ b/src/tui/chart.zig @@ -317,7 +317,15 @@ pub fn renderChart( const vol_h_px = (vols[ci] / vol_max) * vol_panel_h; const bar_top = vol_bottom_y - vol_h_px; - const is_up = candle.close >= candle.open; + // Up/down coloring: compare today's chart-close to + // yesterday's chart-close (both split-adjusted when + // available). Comparing close-vs-open here would render + // a spurious "down" day on every split date because + // `Candle.open` is not split-adjusted but `chartClose` + // is — see `Candle.chartClose` for context. + const cc = candle.chartClose(); + const prev_cc = if (ci > 0) data[ci - 1].chartClose() else cc; + const is_up = cc >= prev_cc; const col = if (is_up) blendColor(th.positive, 50, bg) else blendColor(th.negative, 50, bg); ctx.setSourceToPixel(col); ctx.resetPath();