diff --git a/src/commands/common.zig b/src/commands/common.zig index f3a94d9..946d331 100644 --- a/src/commands/common.zig +++ b/src/commands/common.zig @@ -2,6 +2,7 @@ const std = @import("std"); const zfin = @import("../root.zig"); const srf = @import("srf"); const history = @import("../history.zig"); +const git = @import("../git.zig"); pub const fmt = @import("../format.zig"); // ── Default CLI colors (match TUI default Monokai theme) ───── @@ -574,7 +575,184 @@ pub fn parseRequiredDateOrStderr( }; } -// ── Snapshot resolution (CLI) ──────────────────────────────── +// ── Commit-spec parsing (shared by contributions / compare) ── + +/// Re-export of `git.CommitSpec` so call sites already using `cli.*` +/// don't need a second import. +pub const CommitSpec = git.CommitSpec; + +pub const CommitSpecError = error{ + Empty, + InvalidFormat, + /// Catch-all for a token that doesn't match any known commit-spec + /// shape. Different from `InvalidFormat` in that the string + /// could be a SHA or ref — git will decide at invocation time. + /// We err on this only when the token has obviously wrong shape. + UnknownForm, +}; + +/// Parse a user-facing commit spec into a `CommitSpec`. +/// +/// Accepts (in priority order): +/// - case-insensitive `working` / `WORKING` / `wc` / `WC` / +/// `working-copy` → `.working_copy` +/// - `YYYY-MM-DD` → `.date_at_or_before` +/// - Relative date form (`1W`, `1M`, `1Q`, `1Y` — same grammar as +/// `--as-of`), resolved against `today` → `.date_at_or_before` +/// - Strings starting with `HEAD` (`HEAD`, `HEAD~N`) → `.git_ref` +/// - Pure hex ≥ 7 chars → `.git_ref` (SHA, full or abbreviated) +/// +/// Anything else is rejected as `UnknownForm`. Trimming applied. +/// +/// `today` is injected for test determinism, matching +/// `parseAsOfDate`'s contract. +pub fn parseCommitSpec(input: []const u8, today: zfin.Date) CommitSpecError!CommitSpec { + const s = std.mem.trim(u8, input, " \t\r\n"); + if (s.len == 0) return error.Empty; + + // Working-copy sentinel (case-insensitive). + if (std.ascii.eqlIgnoreCase(s, "working") or + std.ascii.eqlIgnoreCase(s, "wc") or + std.ascii.eqlIgnoreCase(s, "working-copy")) + { + return .working_copy; + } + + // YYYY-MM-DD — 10 chars, two dashes at fixed positions. + if (s.len == 10 and s[4] == '-' and s[7] == '-') { + const d = zfin.Date.parse(s) catch return error.InvalidFormat; + return .{ .date_at_or_before = d }; + } + + // Relative date form (1W, 1M, 1Q, 1Y). Disambiguated from short + // SHAs (both can lead with digits) by the trailing unit letter — + // W/M/Q/Y case-insensitive. Without it, a token like "1234567" + // could be either a 7-char abbreviated SHA or garbage; we treat + // it as SHA and let git decide. + if (s.len >= 2 and std.ascii.isDigit(s[0])) { + const last = std.ascii.toLower(s[s.len - 1]); + if (last == 'w' or last == 'm' or last == 'q' or last == 'y') { + const as_of = parseAsOfDate(s, today) catch return error.InvalidFormat; + if (as_of) |d| return .{ .date_at_or_before = d }; + return error.InvalidFormat; + } + } + + // HEAD / HEAD~N refs. + if (std.mem.startsWith(u8, s, "HEAD")) { + return .{ .git_ref = s }; + } + + // Pure hex with sensible length → treat as SHA; let git validate. + if (s.len >= 7) { + var all_hex = true; + for (s) |c| { + if (!std.ascii.isHex(c)) { + all_hex = false; + break; + } + } + if (all_hex) return .{ .git_ref = s }; + } + + return error.UnknownForm; +} + +/// Human-friendly explanation of a `parseCommitSpec` error. +pub fn fmtCommitSpecError(buf: []u8, input: []const u8, err: CommitSpecError) []const u8 { + return switch (err) { + error.Empty => std.fmt.bufPrint(buf, "Commit spec is empty.", .{}) catch "Commit spec is empty.", + error.InvalidFormat => std.fmt.bufPrint( + buf, + "Commit spec {s} has a date-like shape but couldn't be parsed. Expected YYYY-MM-DD or N[WMQY].", + .{input}, + ) catch input, + error.UnknownForm => std.fmt.bufPrint( + buf, + "Commit spec {s} is not recognized. Accepts: 'working', YYYY-MM-DD, relative (1W/1M/1Q/1Y), HEAD / HEAD~N, or a 7+ hex SHA.", + .{input}, + ) catch input, + }; +} + +// ── parseCommitSpec tests ────────────────────────────────── + +test "parseCommitSpec: working-copy sentinels" { + const today = zfin.Date.fromYmd(2026, 5, 9); + try std.testing.expect((try parseCommitSpec("working", today)) == .working_copy); + try std.testing.expect((try parseCommitSpec("WORKING", today)) == .working_copy); + try std.testing.expect((try parseCommitSpec("wc", today)) == .working_copy); + try std.testing.expect((try parseCommitSpec("WC", today)) == .working_copy); + try std.testing.expect((try parseCommitSpec("working-copy", today)) == .working_copy); +} + +test "parseCommitSpec: YYYY-MM-DD → date" { + const today = zfin.Date.fromYmd(2026, 5, 9); + const spec = try parseCommitSpec("2026-05-04", today); + switch (spec) { + .date_at_or_before => |d| { + try std.testing.expectEqual(@as(i16, 2026), d.year()); + try std.testing.expectEqual(@as(u8, 5), d.month()); + try std.testing.expectEqual(@as(u8, 4), d.day()); + }, + else => try std.testing.expect(false), + } +} + +test "parseCommitSpec: relative 1W → date" { + const today = zfin.Date.fromYmd(2026, 5, 9); + const spec = try parseCommitSpec("1W", today); + switch (spec) { + .date_at_or_before => |d| { + // 1W ago from 2026-05-09 is 2026-05-02 + try std.testing.expectEqual(@as(i16, 2026), d.year()); + try std.testing.expectEqual(@as(u8, 5), d.month()); + try std.testing.expectEqual(@as(u8, 2), d.day()); + }, + else => try std.testing.expect(false), + } +} + +test "parseCommitSpec: HEAD and HEAD~N as git_ref" { + const today = zfin.Date.fromYmd(2026, 5, 9); + switch (try parseCommitSpec("HEAD", today)) { + .git_ref => |r| try std.testing.expectEqualStrings("HEAD", r), + else => try std.testing.expect(false), + } + switch (try parseCommitSpec("HEAD~1", today)) { + .git_ref => |r| try std.testing.expectEqualStrings("HEAD~1", r), + else => try std.testing.expect(false), + } + switch (try parseCommitSpec("HEAD~12", today)) { + .git_ref => |r| try std.testing.expectEqualStrings("HEAD~12", r), + else => try std.testing.expect(false), + } +} + +test "parseCommitSpec: SHA as git_ref" { + const today = zfin.Date.fromYmd(2026, 5, 9); + switch (try parseCommitSpec("6942020", today)) { + .git_ref => |r| try std.testing.expectEqualStrings("6942020", r), + else => try std.testing.expect(false), + } + switch (try parseCommitSpec("6942020abcdef1234567890", today)) { + .git_ref => |r| try std.testing.expectEqualStrings("6942020abcdef1234567890", r), + else => try std.testing.expect(false), + } +} + +test "parseCommitSpec: rejects unknown shapes" { + const today = zfin.Date.fromYmd(2026, 5, 9); + try std.testing.expectError(error.Empty, parseCommitSpec("", today)); + try std.testing.expectError(error.Empty, parseCommitSpec(" ", today)); + try std.testing.expectError(error.UnknownForm, parseCommitSpec("xyz", today)); // < 7 chars, not HEAD/working + try std.testing.expectError(error.UnknownForm, parseCommitSpec("banana", today)); +} + +test "parseCommitSpec: trims whitespace" { + const today = zfin.Date.fromYmd(2026, 5, 9); + try std.testing.expect((try parseCommitSpec(" working ", today)) == .working_copy); +} /// Snap a requested snapshot date to the nearest earlier snapshot /// that exists in `hist_dir`, printing CLI-friendly stderr messages diff --git a/src/commands/compare.zig b/src/commands/compare.zig index 19eb873..c0305ff 100644 --- a/src/commands/compare.zig +++ b/src/commands/compare.zig @@ -48,6 +48,7 @@ const std = @import("std"); const zfin = @import("../root.zig"); const cli = @import("common.zig"); +const git = @import("../git.zig"); const fmt = cli.fmt; const Date = zfin.Date; const history = @import("../history.zig"); @@ -77,41 +78,104 @@ pub fn run( ) !void { // ── Parse args ─────────────────────────────────────────── // - // `--projections` is an opt-in flag that embeds the - // projected-return + safe-withdrawal delta block between the - // Liquid totals and the per-symbol table. Kept opt-in because - // building the projection side for both dates roughly doubles - // command runtime (Monte Carlo SWR search + benchmark trailing - // returns for both endpoints). Strip it before positional date - // parsing so it can appear anywhere in the arg list. We do NOT - // accept `-p` as a shortcut because `-p` is already the global - // `--portfolio` flag at the top level — shadowing it here would - // confuse users who reach for the short form. + // Compare has four orthogonal axes for specifying the before/ + // after endpoints, each with a convenient default and an explicit + // override: // - // `--no-events` mirrors the `projections` command's flag of the - // same name: it suppresses life events in the underlying - // projection simulation. Only meaningful alongside - // `--projections` (silently ignored otherwise). + // Snapshot (for liquid totals + per-symbol table): + // --snapshot-before — default from positional arg 1 + // --snapshot-after — default: live (or arg 2 if given) + // + // Commit (for the attribution / contributions diff): + // --commit-before — default from positional arg 1 + // --commit-after — default: HEAD clean / WORKING dirty + // + // Positional date(s) still drive the happy path (one or two + // dates → all four axes). The overrides let the user pull a + // single axis independently — e.g. `compare 1W --commit-before HEAD` + // when they committed mid-week and the 1W snapshot is still correct + // but the 1W commit-at-or-before resolves to the wrong reconciliation. + // + // Other flags: --projections (embeds the projected-return delta + // block), --no-events (with --projections, excludes life events). + // + // We do NOT accept `-p` as a shortcut because `-p` is already the + // global `--portfolio` flag; shadowing it would confuse users + // reaching for the short form. var with_projections = false; var events_enabled = true; + var snapshot_before_override: ?Date = null; + var snapshot_after_override: ?Date = null; + var snapshot_after_live = false; // true if user explicitly set --snapshot-after live + var commit_before_override: ?git.CommitSpec = null; + var commit_after_override: ?git.CommitSpec = null; var positional: std.ArrayList([]const u8) = .empty; defer positional.deinit(allocator); - for (cmd_args) |a| { + + const today_for_parse = fmt.todayDate(); + var arg_i: usize = 0; + while (arg_i < cmd_args.len) : (arg_i += 1) { + const a = cmd_args[arg_i]; if (std.mem.eql(u8, a, "--projections")) { with_projections = true; } else if (std.mem.eql(u8, a, "--no-events")) { events_enabled = false; + } else if (std.mem.eql(u8, a, "--snapshot-before") or std.mem.eql(u8, a, "--snapshot-after")) { + if (arg_i + 1 >= cmd_args.len) { + try cli.stderrPrint("Error: "); + try cli.stderrPrint(a); + try cli.stderrPrint(" requires a date (YYYY-MM-DD, relative like 1W, or 'live' for --snapshot-after).\n"); + return error.UnexpectedArg; + } + const value = cmd_args[arg_i + 1]; + const is_after = std.mem.eql(u8, a, "--snapshot-after"); + // --snapshot-after supports 'live' (= current portfolio, + // not a snapshot file). --snapshot-before does not. + const parsed = cli.parseAsOfDate(value, today_for_parse) catch |err| { + var ebuf: [256]u8 = undefined; + const msg = cli.fmtAsOfParseError(&ebuf, value, err); + try cli.stderrPrint(msg); + try cli.stderrPrint("\n"); + return error.InvalidDate; + }; + if (parsed) |d| { + if (is_after) snapshot_after_override = d else snapshot_before_override = d; + } else if (is_after) { + snapshot_after_live = true; + } else { + try cli.stderrPrint("Error: --snapshot-before cannot be 'live' — the before side must be an actual snapshot.\n"); + return error.InvalidDate; + } + arg_i += 1; + } else if (std.mem.eql(u8, a, "--commit-before") or std.mem.eql(u8, a, "--commit-after")) { + if (arg_i + 1 >= cmd_args.len) { + try cli.stderrPrint("Error: "); + try cli.stderrPrint(a); + try cli.stderrPrint(" requires a value (working, YYYY-MM-DD, 1W/1M/1Q/1Y, HEAD, HEAD~N, or SHA).\n"); + return error.UnexpectedArg; + } + const value = cmd_args[arg_i + 1]; + const spec = cli.parseCommitSpec(value, today_for_parse) catch |err| { + var ebuf: [256]u8 = undefined; + const msg = cli.fmtCommitSpecError(&ebuf, value, err); + try cli.stderrPrint(msg); + try cli.stderrPrint("\n"); + return error.InvalidDate; + }; + if (std.mem.eql(u8, a, "--commit-before")) { + if (spec == .working_copy) { + try cli.stderrPrint("Error: --commit-before cannot be `working` — diffing the working copy against itself is meaningless.\n"); + return error.InvalidDate; + } + commit_before_override = spec; + } else { + commit_after_override = spec; + } + arg_i += 1; } else if (a.len > 0 and a[0] == '-' and !std.mem.eql(u8, a, "-")) { - // Any other dash-prefixed token is an unknown compare flag. - // Catch it explicitly with a pointed message rather than - // letting it fall through to the date parser (which would - // emit a generic "invalid date" error). Most likely cause: - // user reached for `-p` expecting `--projections` — that - // shortcut was intentionally not exposed to avoid - // shadowing the global `-p` / `--portfolio` flag. try cli.stderrPrint("Error: unknown flag for 'compare': "); try cli.stderrPrint(a); - try cli.stderrPrint("\nKnown flags: --projections, --no-events.\n"); + try cli.stderrPrint("\nKnown flags: --projections, --no-events, --snapshot-before, --snapshot-after, --commit-before, --commit-after.\n"); if (std.mem.eql(u8, a, "-p")) { try cli.stderrPrint(" (Tip: the projections flag is spelled `--projections` in full.\n"); try cli.stderrPrint(" `-p` is reserved for the global --portfolio option and must appear\n"); @@ -124,31 +188,35 @@ pub fn run( } const args = positional.items; - if (args.len == 0) { - try cli.stderrPrint("Error: 'compare' requires one or two dates.\n"); + // Require at least one source of the "then" date — either a + // positional arg or a --snapshot-before / --commit-before + // override that gives us an anchor. + const have_then_anchor = args.len >= 1 or snapshot_before_override != null or commit_before_override != null; + if (!have_then_anchor) { + try cli.stderrPrint("Error: 'compare' requires a before-side anchor (positional date, --snapshot-before, or --commit-before).\n"); try cli.stderrPrint("Usage:\n"); - try cli.stderrPrint(" zfin compare [--projections [--no-events]] (compare date vs current)\n"); - try cli.stderrPrint(" zfin compare [--projections [--no-events]] (compare two dates)\n"); + try cli.stderrPrint(" zfin compare (compare date vs current)\n"); + try cli.stderrPrint(" zfin compare (compare two dates)\n"); + try cli.stderrPrint(" zfin compare --snapshot-before [--commit-before ] (explicit axes)\n"); try cli.stderrPrint("Dates accept YYYY-MM-DD or relative shortcuts: 1W, 1M, 1Q, 1Y.\n"); - try cli.stderrPrint("Flags:\n"); - try cli.stderrPrint(" --projections Include projected return + safe-withdrawal deltas.\n"); - try cli.stderrPrint(" --no-events (with --projections) Exclude life events from the simulation.\n"); + try cli.stderrPrint("See `zfin help` for --commit-before/--commit-after/--snapshot-before/--snapshot-after details.\n"); return error.MissingDateArg; } if (args.len > 2) { - try cli.stderrPrint("Error: 'compare' takes at most two dates.\n"); + try cli.stderrPrint("Error: 'compare' takes at most two positional dates.\n"); return error.UnexpectedArg; } const today = fmt.todayDate(); - const date1 = cli.parseRequiredDateOrStderr(args[0], today, "date1") catch |err| switch (err) { - error.InvalidDate => return error.InvalidDate, - }; - // Resolve (then_date, now_date, now_is_live). In single-date mode the - // user-given date is "then" and today is "now" (from the live - // portfolio). In two-date mode both are snapshots and we swap to - // guarantee older → newer. + // Parse positional dates (if any). These feed the snapshot axes + // by default; explicit overrides win. + const date1: ?Date = if (args.len >= 1) + (cli.parseRequiredDateOrStderr(args[0], today, "date1") catch |err| switch (err) { + error.InvalidDate => return error.InvalidDate, + }) + else + null; const date2: ?Date = if (args.len == 2) (cli.parseRequiredDateOrStderr(args[1], today, "date2") catch |err| switch (err) { error.InvalidDate => return error.InvalidDate, @@ -156,33 +224,64 @@ pub fn run( else null; - const now_is_live = date2 == null; - // SAFETY: both unconditionally assigned in every branch of the - // `if (now_is_live)` block below before first read. - var then_date: Date = undefined; - // SAFETY: see above. - var now_date: Date = undefined; + // Resolve the snapshot "then" / "now" dates. + // then_date_requested: user-facing before date (for reporting + + // attribution fallback). Derived from + // --snapshot-before override first, else + // positional[0]. + // now_date_requested: user-facing after date. From + // --snapshot-after override if non-live, + // else positional[1] if given, else today. + // now_is_live: true when no --snapshot-after-date and + // no positional[1] — equivalent to the + // old single-date mode. + const then_requested_opt: ?Date = snapshot_before_override orelse date1; + // If neither a snapshot override nor a positional date was given, + // we still need a then-date for the snapshot side. Fall back to + // the commit-before's date if it's a date spec. + const then_requested: Date = if (then_requested_opt) |d| d else fallback: { + if (commit_before_override) |cb| { + switch (cb) { + .date_at_or_before => |d| break :fallback d, + else => { + try cli.stderrPrint("Error: --commit-before with a non-date SPEC requires an explicit --snapshot-before date for the liquid comparison.\n"); + return error.MissingDateArg; + }, + } + } + unreachable; // have_then_anchor guarantees at least one source + }; + + const now_is_live = !snapshot_after_live and snapshot_after_override == null and date2 == null; + const now_requested: Date = if (snapshot_after_override) |d| d else if (date2) |d| d else today; + + // Validate snapshot date ordering. if (now_is_live) { - if (date1.days == today.days) { + if (then_requested.days == today.days) { try cli.stderrPrint("Error: cannot compare today against today's live portfolio.\n"); return error.SameDate; } - // A future date in single-date mode is nonsense — the "then" - // snapshot wouldn't exist and we'd compare live-now against - // live-now anyway. Reject early. - if (date1.days > today.days) { + if (then_requested.days > today.days) { try cli.stderrPrint("Error: cannot compare against a future date.\n"); return error.InvalidDate; } - then_date = date1; - now_date = today; - } else { - if (date1.days == date2.?.days) { - try cli.stderrPrint("Error: both dates are the same — nothing to compare.\n"); + } else if (!snapshot_after_live) { + if (then_requested.days == now_requested.days) { + try cli.stderrPrint("Error: before and after dates are the same — nothing to compare.\n"); return error.SameDate; } - then_date = if (date1.days < date2.?.days) date1 else date2.?; - now_date = if (date1.days < date2.?.days) date2.? else date1; + } + + // Swap if user provided positional dates in reverse order and no + // explicit override. Overrides are respected as given (user + // asked for it explicitly). + var then_date: Date = then_requested; + var now_date: Date = now_requested; + if (!now_is_live and snapshot_before_override == null and snapshot_after_override == null and + date1 != null and date2 != null and date1.?.days > date2.?.days) + { + then_date = date2.?; + now_date = date1.?; } // ── Resolve history dir ────────────────────────────────── @@ -192,22 +291,11 @@ pub fn run( // ── Snap dates to nearest-earlier snapshots ────────────── // // The requested date may not correspond to an actual snapshot - // file (e.g. `1W` resolves to a weekday but snapshots only exist - // for business days, or the cron was down that day). Snap to the - // most recent snapshot at-or-before the requested date — same - // semantics as `projections --as-of`. When the snap is inexact, - // overwrite the date used downstream so the rendered header - // reflects what was actually loaded. - // - // IMPORTANT: we keep the *original requested* dates around for - // attribution. The attribution is a git-window query that should - // match what `zfin contributions --since ` would - // produce — if we passed the snapped dates to attribution, the - // git window would widen every time the requested date doesn't - // land on a snapshot, causing compare's attribution to disagree - // with contributions' totals over the same nominal window. - // Snapshot loading uses the snapped dates (so we actually find a - // file on disk); attribution uses the original dates. + // file. Snap to the most recent snapshot at-or-before. Record + // the original requested date so we can pass it to the + // attribution git-window independently (liquid uses the snap'd + // date; attribution uses the requested date unless --commit-* + // overrides). var arena_state = std.heap.ArenaAllocator.init(allocator); defer arena_state.deinit(); const arena = arena_state.allocator(); @@ -276,16 +364,29 @@ pub fn run( } } + // Build the CommitSpecs for attribution. Explicit --commit-* + // overrides win; otherwise fall back to the requested snapshot + // date. now_is_live implies `null` for after (= working copy + // default handling downstream). + const attr_before: git.CommitSpec = commit_before_override orelse + .{ .date_at_or_before = then_date_requested }; + const attr_after_opt: ?git.CommitSpec = if (commit_after_override) |s| + s + else if (now_is_live) + null + else + .{ .date_at_or_before = now_date_requested }; + if (now_is_live) { var now_live = try LiveSide.load(allocator, svc, portfolio_path, color); defer now_live.deinit(allocator); - // Attribution spans then_date_requested → HEAD (or working - // copy if dirty). `computeAttribution` with until=null uses - // exactly that semantics. Using the REQUESTED date (not the - // snapped-to-snapshot date) keeps the git window aligned - // with `zfin contributions --since `. - const attribution = contributions.computeAttribution(allocator, svc, portfolio_path, then_date_requested, null, color); + // Attribution uses the resolved CommitSpecs so --commit-* + // overrides + date fallbacks share one classifier. The + // spec-form computeAttribution is identical to the date form + // wrapper — same classifier, same windowing — just without + // the Date → CommitSpec.date_at_or_before translation. + const attribution = contributions.computeAttributionSpec(allocator, svc, portfolio_path, attr_before, attr_after_opt, color); try renderFromParts(out, color, allocator, .{ .then_date = then_date, @@ -302,11 +403,7 @@ pub fn run( var now_side = try compare_core.loadSnapshotSide(allocator, hist_dir, now_date); defer now_side.deinit(allocator); - // Attribution spans then_date_requested → now_date_requested. - // Using the REQUESTED dates (not the snapped-to-snapshot - // dates) keeps the git window aligned with what `zfin - // contributions --since --until ` would produce. - const attribution = contributions.computeAttribution(allocator, svc, portfolio_path, then_date_requested, now_date_requested, color); + const attribution = contributions.computeAttributionSpec(allocator, svc, portfolio_path, attr_before, attr_after_opt, color); try renderFromParts(out, color, allocator, .{ .then_date = then_date, diff --git a/src/commands/contributions.zig b/src/commands/contributions.zig index 5cabf25..8c6447f 100644 --- a/src/commands/contributions.zig +++ b/src/commands/contributions.zig @@ -131,8 +131,8 @@ pub fn run( allocator: std.mem.Allocator, svc: *zfin.DataService, portfolio_path: []const u8, - since: ?Date, - until: ?Date, + before: ?git.CommitSpec, + after: ?git.CommitSpec, color: bool, out: *std.Io.Writer, ) !void { @@ -144,15 +144,16 @@ pub fn run( defer arena_state.deinit(); const arena = arena_state.allocator(); - // Enforce the "--until without --since is ambiguous" rule at the - // entry point so `resolveEndpoints`/`git.resolveCommitRange` can - // assume the invariant. - if (since == null and until != null) { - try cli.stderrPrint("Error: --until requires --since. Use `contributions --since ` or both.\n"); + // Enforce the "an `after` with no `before` is ambiguous" rule at + // the entry point so `resolveEndpoints`/`git.resolveCommitRangeSpec` + // can assume the invariant. The legacy no-flag path passes both + // as null and falls through to HEAD~1..HEAD / HEAD..WC. + if (before == null and after != null) { + try cli.stderrPrint("Error: --until / --commit-after requires --since / --commit-before.\n"); return; } - var ctx = prepareReport(allocator, arena, svc, portfolio_path, since, until, color, .verbose) catch return; + var ctx = prepareReport(allocator, arena, svc, portfolio_path, before, after, color, .verbose) catch return; defer ctx.deinit(); try printReport(out, &ctx.report, ctx.endpoints.label, color); @@ -198,8 +199,8 @@ fn prepareReport( arena: std.mem.Allocator, svc: *zfin.DataService, portfolio_path: []const u8, - since: ?Date, - until: ?Date, + before_spec: ?git.CommitSpec, + after_spec: ?git.CommitSpec, color: bool, verbosity: Verbosity, ) PrepareError!ReportContext { @@ -224,7 +225,7 @@ fn prepareReport( } const dirty = status == .modified; - const endpoints = resolveEndpoints(arena, repo, since, until, dirty, verbosity) catch return error.PrepareFailed; + const endpoints = resolveEndpoints(arena, repo, before_spec, after_spec, dirty, verbosity) catch return error.PrepareFailed; // Pull both sides: before is always from git; after is either // from git (at some revision) or from the working copy. @@ -338,23 +339,27 @@ const Verbosity = enum { verbose, silent }; fn resolveEndpoints( arena: std.mem.Allocator, repo: git.RepoInfo, - since: ?Date, - until: ?Date, + before: ?git.CommitSpec, + after: ?git.CommitSpec, dirty: bool, verbosity: Verbosity, ) !Endpoints { - const range = git.resolveCommitRange(arena, repo, since, until, dirty) catch |err| { + const range = git.resolveCommitRangeSpec(arena, repo, before, after, dirty) catch |err| { if (verbosity == .verbose) { switch (err) { error.NoCommitAtOrBefore => { - // Report which flag triggered it. When both are set - // we can't easily tell from here; message covers both. - var since_buf: [10]u8 = undefined; - const since_str = if (since) |s| s.format(&since_buf) else "(unset)"; + // Report which spec triggered it. When both are + // date specs we can't easily tell from here; + // covers both by naming the before-side. + var before_buf: [10]u8 = undefined; + const before_str = specDisplayString(before, &before_buf); var msg_buf: [256]u8 = undefined; - const msg = std.fmt.bufPrint(&msg_buf, "Error: no commit of {s} at or before {s}.\n", .{ repo.rel_path, since_str }) catch "Error: no commit at or before requested date.\n"; + const msg = std.fmt.bufPrint(&msg_buf, "Error: no commit of {s} at or before {s}.\n", .{ repo.rel_path, before_str }) catch "Error: no commit at or before requested date.\n"; try cli.stderrPrint(msg); }, + error.InvalidArg => { + try cli.stderrPrint("Error: --commit-before cannot be `working` — diffing the working copy against itself is meaningless.\n"); + }, else => { try cli.stderrPrint("Error resolving commit range: "); try cli.stderrPrint(@errorName(err)); @@ -368,22 +373,154 @@ fn resolveEndpoints( // Label the endpoints based on the resolution mode. Matches the // legacy phrasing where possible so existing test assertions still // pass. - const label = try buildLabel(arena, range, since, until, dirty); + const label = try buildLabelFromSpecs(arena, range, before, after, dirty); // Same-commit warning for the two-date window case. Legit confusion // trigger — the user asked for a diff between two dates that both // snap to the same commit (e.g., no activity in the window). - if (since != null and until != null and verbosity == .verbose) { + if (before != null and after != null and verbosity == .verbose) { if (range.after_rev) |after_rev| { if (std.mem.eql(u8, range.before_rev, after_rev)) { - try cli.stderrPrint("Warning: --since and --until resolve to the same commit; no changes to report.\n"); + try cli.stderrPrint("Warning: before and after resolve to the same commit; no changes to report.\n"); } } } + // Snap-note warning: when the user provided a date-form spec and + // the resolved commit's committer-date is >1 day before the + // requested date, emit a muted hint so the user can verify the + // selected commit matches their intent. See + // `docs/notes/commit-window-edge-case.md` (aka TODO.md) for the + // motivating scenario. + if (verbosity == .verbose) { + try maybeSnapNote(arena, repo, before, range.before_rev, "before"); + } + return .{ .range = range, .label = label }; } +/// Render a `CommitSpec` for user-facing error messages. Dates and +/// working-copy sentinels get formatted; refs are passed through. +/// When `spec` is null, returns "(unset)". +fn specDisplayString(spec: ?git.CommitSpec, date_buf: *[10]u8) []const u8 { + const s = spec orelse return "(unset)"; + return switch (s) { + .git_ref => |r| r, + .date_at_or_before => |d| d.format(date_buf), + .working_copy => "working", + }; +} + +/// If the user's before spec was a date form and the resolved commit +/// sits more than 1 day earlier than the requested date, print a +/// muted hint. Catches the "I committed after my review date" case +/// where `--since 1W` picks up a commit 7+ days before the cutoff. +fn maybeSnapNote( + arena: std.mem.Allocator, + repo: git.RepoInfo, + spec: ?git.CommitSpec, + resolved_ref: []const u8, + label: []const u8, +) !void { + const s = spec orelse return; + const requested_date = switch (s) { + .date_at_or_before => |d| d, + else => return, + }; + + // Get the committer-date of the resolved commit. `%ct` gives a + // Unix timestamp. + const ts = git.commitTimestamp(arena, repo.root, resolved_ref) catch return; + const commit_date = zfin.Date.fromEpoch(ts); + if (!commit_date.lessThan(requested_date)) return; + + const gap_days = requested_date.days - commit_date.days; + if (gap_days <= 1) return; + + var req_buf: [10]u8 = undefined; + var commit_buf: [10]u8 = undefined; + var msg_buf: [320]u8 = undefined; + const msg = std.fmt.bufPrint( + &msg_buf, + "(git {s} uses commit {s} from {s}, {d} day{s} before requested {s} — " ++ + "use --commit-{s} HEAD or a later date to pin to your latest reconciliation commit)\n", + .{ + label, + shortSha(resolved_ref), + commit_date.format(&commit_buf), + gap_days, + if (gap_days == 1) "" else "s", + requested_date.format(&req_buf), + label, + }, + ) catch return; + cli.stderrPrint(msg) catch {}; +} + +/// Abbreviate a commit ref for display. SHAs get shortened to 7 +/// chars; symbolic refs (HEAD, HEAD~1) stay as-is. +fn shortSha(ref: []const u8) []const u8 { + if (std.mem.startsWith(u8, ref, "HEAD")) return ref; + if (ref.len > 7) return ref[0..7]; + return ref; +} + +/// Build a report-header label from the resolved range + the +/// user-provided specs. Date specs render their requested form; +/// refs render verbatim; null falls back to the legacy HEAD~/HEAD +/// naming. +fn buildLabelFromSpecs( + arena: std.mem.Allocator, + range: git.CommitRange, + before: ?git.CommitSpec, + after: ?git.CommitSpec, + dirty: bool, +) ![]const u8 { + // Preserve the original date-based label when both specs are + // date-form (most common user-facing flow). Fall back to a + // spec-agnostic rendering otherwise. + const before_date: ?Date = if (before) |b| switch (b) { + .date_at_or_before => |d| d, + else => null, + } else null; + const after_date: ?Date = if (after) |a| switch (a) { + .date_at_or_before => |d| d, + else => null, + } else null; + + // If either spec is a non-date form, emit a label describing + // the resolved range. Otherwise use the legacy date/label path + // for back-compat with snapshot tests. + const has_non_date = (before != null and before_date == null) or + (after != null and after_date == null); + if (has_non_date) { + const before_str = try specLabel(arena, before, range.before_rev); + const after_str = try specLabelAfter(arena, after, range.after_rev); + return std.fmt.allocPrint(arena, "{s} vs {s}", .{ before_str, after_str }); + } + + return buildLabel(arena, range, before_date, after_date, dirty); +} + +fn specLabel(arena: std.mem.Allocator, spec: ?git.CommitSpec, resolved_ref: []const u8) ![]const u8 { + const s = spec orelse return arena.dupe(u8, resolved_ref); + return switch (s) { + .git_ref => |r| arena.dupe(u8, r), + .date_at_or_before => |d| blk: { + var buf: [10]u8 = undefined; + const date_str = d.format(&buf); + break :blk std.fmt.allocPrint(arena, "commit at-or-before {s}", .{date_str}); + }, + .working_copy => arena.dupe(u8, "working copy"), + }; +} + +fn specLabelAfter(arena: std.mem.Allocator, spec: ?git.CommitSpec, resolved_ref: ?[]const u8) ![]const u8 { + if (spec) |s| return specLabel(arena, s, resolved_ref orelse "working"); + if (resolved_ref) |r| return arena.dupe(u8, r); + return arena.dupe(u8, "working copy"); +} + /// Build the human-readable header label for a resolved range. fn buildLabel( arena: std.mem.Allocator, @@ -485,7 +622,7 @@ pub fn computeAttribution( ) ?AttributionSummary { // `--until` without `--since` is ambiguous; caller is expected to // enforce this at the entry point, but guard here too — the - // prepareReport path sends it through `git.resolveCommitRange` + // prepareReport path sends it through `git.resolveCommitRangeSpec` // which asserts the invariant. if (since == null and until != null) return null; @@ -493,9 +630,43 @@ pub fn computeAttribution( defer arena_state.deinit(); const arena = arena_state.allocator(); - var ctx = prepareReport(allocator, arena, svc, portfolio_path, since, until, color, .silent) catch return null; + // Wrap legacy date-based inputs as CommitSpec.date_at_or_before + // for the shared prepareReport path. + const before: ?git.CommitSpec = if (since) |d| .{ .date_at_or_before = d } else null; + const after: ?git.CommitSpec = if (until) |d| .{ .date_at_or_before = d } else null; + + var ctx = prepareReport(allocator, arena, svc, portfolio_path, before, after, color, .silent) catch return null; defer ctx.deinit(); + return summarizeAttribution(ctx); +} + +/// Spec-based variant of `computeAttribution` for callers that +/// already have `CommitSpec`s (e.g. `compare --commit-before HEAD`). +/// Uses exactly the same classifier as the date form; only the +/// argument shape differs. +pub fn computeAttributionSpec( + allocator: std.mem.Allocator, + svc: *zfin.DataService, + portfolio_path: []const u8, + before: ?git.CommitSpec, + after: ?git.CommitSpec, + color: bool, +) ?AttributionSummary { + if (before == null and after != null) return null; + + var arena_state = std.heap.ArenaAllocator.init(allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + var ctx = prepareReport(allocator, arena, svc, portfolio_path, before, after, color, .silent) catch return null; + defer ctx.deinit(); + + return summarizeAttribution(ctx); +} + +fn summarizeAttribution(ctx: ReportContext) AttributionSummary { + // Aggregate. Classification logic matches the full-report sections: // - New contributions: new_stock + new_cash + new_cd + new_option // + cash_contribution (opt-in cash_delta) diff --git a/src/git.zig b/src/git.zig index 33c61f1..e19ab3d 100644 --- a/src/git.zig +++ b/src/git.zig @@ -40,6 +40,10 @@ pub const Error = error{ /// `resolveCommitRange` was asked for a `since` date with no commit /// at or before it — nothing to diff against. NoCommitAtOrBefore, + /// The caller passed an invalid argument combination — e.g. + /// `CommitSpec.working_copy` on the "before" side, which is + /// nonsensical. + InvalidArg, OutOfMemory, }; @@ -51,6 +55,32 @@ pub const RepoInfo = struct { rel_path: []const u8, }; +/// A user-facing commit endpoint. Captures the three input shapes +/// accepted by `--commit-before` / `--commit-after` (and the +/// date-oriented `--since` / `--until` / compare positional args, +/// which the command layer parses into this type). +/// +/// - `git_ref` — a string `git show :` will accept +/// directly (SHA, HEAD, HEAD~N). Validation deferred to git. +/// - `date_at_or_before` — a calendar date. Resolved at +/// `resolveCommitRange` time via `commitAtOrBeforeDate`. Kept as +/// a date (not pre-resolved to a SHA) so the snap-note warning +/// can compare the resolved commit's timestamp against the +/// originally-requested date at report time. +/// - `working_copy` — the filesystem state (possibly dirty). +/// Valid only as the "after" endpoint — nonsensical as a +/// "before" because diffing the working copy against itself +/// produces nothing. +/// +/// Defined here (not in `commands/common.zig`) because the concept +/// is git-layer. `commands/common.zig` has the user-input parser +/// that produces values of this type. +pub const CommitSpec = union(enum) { + git_ref: []const u8, + date_at_or_before: Date, + working_copy, +}; + /// Working-tree state of a single tracked path. pub const PathStatus = enum { /// Tracked and has staged and/or unstaged changes. @@ -370,6 +400,38 @@ pub fn commitAtOrBeforeDate( return try allocator.dupe(u8, trimmed); } +/// Get the committer-date (Unix timestamp, seconds since epoch) of +/// a given ref. Accepts anything `git log -1 --format=%ct ` +/// accepts: SHAs, HEAD, HEAD~N. Used by the contributions pipeline +/// to emit a snap-note when a date-form spec resolves to a commit +/// that's far from the user's requested date. +pub fn commitTimestamp( + allocator: std.mem.Allocator, + root: []const u8, + ref: []const u8, +) Error!i64 { + const result = std.process.Child.run(.{ + .allocator = allocator, + .argv = &.{ + "git", "-C", root, + "log", "-1", "--format=%ct", + ref, + }, + .max_output_bytes = 4 * 1024, + }) catch return error.GitUnavailable; + defer allocator.free(result.stdout); + defer allocator.free(result.stderr); + + switch (result.term) { + .Exited => |code| if (code != 0) return error.GitLogFailed, + else => return error.GitLogFailed, + } + + const trimmed = std.mem.trim(u8, result.stdout, " \t\r\n"); + if (trimmed.len == 0) return error.GitLogFailed; + return std.fmt.parseInt(i64, trimmed, 10) catch return error.GitLogFailed; +} + /// Resolve a before/after commit range for diffing `repo.rel_path`. /// /// Three modes selected by `since` / `until`: @@ -395,6 +457,86 @@ pub fn commitAtOrBeforeDate( /// /// Pure SHA-level output — no labels, no stderr side effects. All /// allocations use `arena`. +/// Resolve a before/after commit range for diffing `repo.rel_path`. +/// +/// Takes `CommitSpec` for each endpoint. When `before` is null, +/// applies the legacy default (HEAD~1 clean / HEAD dirty). When +/// `after` is null, applies the legacy default for the after side +/// based on `dirty`. When explicitly provided, the specs are +/// honored as given. +/// +/// Returns `error.NoCommitAtOrBefore` when a `date_at_or_before` +/// spec resolves to "no commit at or before this date." Returns +/// `error.InvalidArg` when `before` is `.working_copy` (nonsensical). +/// +/// Pure SHA-level output — no labels, no stderr side effects. All +/// allocations use `arena`. +/// +/// Three-tier rule for clarity: +/// +/// 1. Both specs explicit → honor as given. +/// 2. One null, one explicit → fill the null from legacy defaults, +/// keeping the explicit side untouched. +/// 3. Both null → full legacy mode: HEAD~1..HEAD (clean) or +/// HEAD..working-copy (dirty). Back-compat with pre-flag +/// `zfin contributions` invocations. +pub fn resolveCommitRangeSpec( + arena: std.mem.Allocator, + repo: RepoInfo, + before: ?CommitSpec, + after: ?CommitSpec, + dirty: bool, +) Error!CommitRange { + // Before can't be working_copy — would be diffing against itself. + if (before) |b| { + if (b == .working_copy) return error.InvalidArg; + } + + // Resolve each endpoint independently. + const before_rev: []const u8 = if (before) |b| + try resolveSpec(arena, repo, b) + else if (dirty) + "HEAD" + else + "HEAD~1"; + + const after_rev: ?[]const u8 = if (after) |a| + (switch (a) { + .working_copy => null, + else => try resolveSpec(arena, repo, a), + }) + else if (dirty) + null + else + "HEAD"; + + return .{ .before_rev = before_rev, .after_rev = after_rev }; +} + +/// Resolve one non-working `CommitSpec` to a string git can consume. +/// Caller handles the `.working_copy` case separately (it's not a +/// git ref). +fn resolveSpec(arena: std.mem.Allocator, repo: RepoInfo, spec: CommitSpec) Error![]const u8 { + return switch (spec) { + .git_ref => |r| r, + .date_at_or_before => |d| blk: { + var buf: [10]u8 = undefined; + const date_str = d.format(&buf); + const sha = (try commitAtOrBeforeDate(arena, repo.root, repo.rel_path, date_str)) orelse + return error.NoCommitAtOrBefore; + break :blk sha; + }, + .working_copy => error.InvalidArg, + }; +} + +/// Back-compat wrapper for the original `Date`-based API. Existing +/// callers (legacy `zfin contributions --since / --until`) keep +/// working unchanged. New callers using explicit commit refs go +/// through `resolveCommitRangeSpec`. +/// +/// `until` without `since` is rejected via assertion — the window is +/// ambiguous without a starting point. pub fn resolveCommitRange( arena: std.mem.Allocator, repo: RepoInfo, @@ -403,35 +545,9 @@ pub fn resolveCommitRange( dirty: bool, ) Error!CommitRange { std.debug.assert(!(since == null and until != null)); - - // Legacy path: no date window. HEAD~1..HEAD (clean) or - // HEAD..working-copy (dirty). Returns the symbolic revisions - // verbatim — callers feed them directly to `git show`. - if (since == null) { - return if (dirty) - .{ .before_rev = "HEAD", .after_rev = null } - else - .{ .before_rev = "HEAD~1", .after_rev = "HEAD" }; - } - - var since_buf: [10]u8 = undefined; - const since_str = since.?.format(&since_buf); - const since_sha = (try commitAtOrBeforeDate(arena, repo.root, repo.rel_path, since_str)) orelse - return error.NoCommitAtOrBefore; - - if (until) |until_date| { - var until_buf: [10]u8 = undefined; - const until_str = until_date.format(&until_buf); - const until_sha = (try commitAtOrBeforeDate(arena, repo.root, repo.rel_path, until_str)) orelse - return error.NoCommitAtOrBefore; - return .{ .before_rev = since_sha, .after_rev = until_sha }; - } - - // --since only: after side is HEAD (or working copy if dirty). - return if (dirty) - .{ .before_rev = since_sha, .after_rev = null } - else - .{ .before_rev = since_sha, .after_rev = "HEAD" }; + const before: ?CommitSpec = if (since) |d| .{ .date_at_or_before = d } else null; + const after: ?CommitSpec = if (until) |d| .{ .date_at_or_before = d } else null; + return resolveCommitRangeSpec(arena, repo, before, after, dirty); } // ── Tests ──────────────────────────────────────────────────── diff --git a/src/main.zig b/src/main.zig index 32f7dba..a452ec9 100644 --- a/src/main.zig +++ b/src/main.zig @@ -78,6 +78,16 @@ const usage = \\ working copy when dirty). Default: HEAD~1..HEAD. \\ --until Upper bound. Pair with --since to diff two \\ commits within a date window. + \\ --commit-before Pin the before commit directly. Takes precedence + \\ over --since's date-based resolution. SPEC accepts + \\ YYYY-MM-DD, relative (1W/1M/1Q/1Y), HEAD, HEAD~N, + \\ or a 7+ hex SHA. Useful when you committed after + \\ your review date and --since 1W lands on the + \\ wrong commit (try --commit-before HEAD). + \\ --commit-after Pin the after commit. Same grammar as + \\ --commit-before, plus `working` / `WORKING` for + \\ the filesystem working copy. Mutually exclusive + \\ with --until. \\ \\Projections command options: \\ --no-events Exclude life events from simulation (baseline view) @@ -99,6 +109,19 @@ const usage = \\ --no-events (with --projections) Exclude life events from the \\ underlying projection simulation. Matches the \\ `projections --no-events` flag. + \\ --snapshot-before Override the before-snapshot independently of + \\ the positional date. DATE accepts YYYY-MM-DD or + \\ relative (1W/1M/1Q/1Y). Defaults from positional. + \\ --snapshot-after Override the after-snapshot. Accepts the same + \\ plus `live` for the current portfolio. Defaults + \\ from positional arg 2, else live. + \\ --commit-before Pin the attribution's before commit. Same SPEC + \\ grammar as the contributions flag. When the + \\ positional date's commit-at-or-before lands on + \\ the wrong commit (committed after review day), + \\ pass `HEAD` or an explicit SHA. + \\ --commit-after Pin the attribution's after commit. Accepts + \\ `working` / `WORKING` for the working copy. \\ \\Environment Variables: \\ TWELVEDATA_API_KEY Twelve Data API key (primary: prices) @@ -505,6 +528,8 @@ fn runCli() !u8 { } else if (std.mem.eql(u8, command, "contributions")) { var since: ?zfin.Date = null; var until: ?zfin.Date = null; + var before_spec: ?cli.CommitSpec = null; + var after_spec: ?cli.CommitSpec = null; var i: usize = 0; while (i < cmd_args.len) : (i += 1) { const a = cmd_args[i]; @@ -539,18 +564,70 @@ fn runCli() !u8 { until = resolved; } i += 1; // consume the value + } else if (std.mem.eql(u8, a, "--commit-before") or std.mem.eql(u8, a, "--commit-after")) { + if (i + 1 >= cmd_args.len) { + try cli.stderrPrint("Error: "); + try cli.stderrPrint(a); + try cli.stderrPrint(" requires a value (working, YYYY-MM-DD, 1W/1M/1Q/1Y, HEAD, HEAD~N, or SHA).\n"); + return 1; + } + const value = cmd_args[i + 1]; + const today = cli.fmt.todayDate(); + const spec = cli.parseCommitSpec(value, today) catch |err| { + var buf: [256]u8 = undefined; + const msg = cli.fmtCommitSpecError(&buf, value, err); + try cli.stderrPrint(msg); + try cli.stderrPrint("\n"); + return 1; + }; + if (std.mem.eql(u8, a, "--commit-before")) { + if (spec == .working_copy) { + try cli.stderrPrint("Error: --commit-before cannot be `working` — diffing the working copy against itself is meaningless.\n"); + return 1; + } + before_spec = spec; + } else { + after_spec = spec; + } + i += 1; // consume the value } else { try reportUnexpectedArg("contributions", a); return 1; } } + // Conflict detection: --since and --commit-before describe the + // same axis, same for --until and --commit-after. Taking both + // would be ambiguous about which wins. + if (since != null and before_spec != null) { + try cli.stderrPrint("Error: --since and --commit-before both specify the before side. Pick one.\n"); + return 1; + } + if (until != null and after_spec != null) { + try cli.stderrPrint("Error: --until and --commit-after both specify the after side. Pick one.\n"); + return 1; + } if (since != null and until != null and since.?.days > until.?.days) { try cli.stderrPrint("Error: --since must be on or before --until.\n"); return 1; } + // Resolve to CommitSpec for the command. Date flags become + // `.date_at_or_before`, commit flags pass through. + const before_final: ?cli.CommitSpec = if (before_spec) |s| + s + else if (since) |d| + .{ .date_at_or_before = d } + else + null; + const after_final: ?cli.CommitSpec = if (after_spec) |s| + s + else if (until) |d| + .{ .date_at_or_before = d } + else + null; + const pf = resolveUserPath(allocator, config, globals.portfolio_path, zfin.Config.default_portfolio_filename); defer if (pf.resolved) |r| r.deinit(allocator); - try commands.contributions.run(allocator, &svc, pf.path, since, until, color, out); + try commands.contributions.run(allocator, &svc, pf.path, before_final, after_final, color, out); } else if (std.mem.eql(u8, command, "snapshot")) { const pf = resolveUserPath(allocator, config, globals.portfolio_path, zfin.Config.default_portfolio_filename); defer if (pf.resolved) |r| r.deinit(allocator);