lobo/zfin
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages 2 Wiki Activity Actions

contributions dates trigger on snapshots for both before and after side
All checks were successful
Generic zig build / build (push) Successful in 6m7s
Details
Generic zig build / publish-macos (push) Successful in 12s
Details
Generic zig build / deploy (push) Successful in 19s
Details

Browse source
This commit is contained in:
Emil Lerch 2026-09-05 13:59:00 -07:00
parent 83f30d0d5a
commit 1c1c6716ee
Signed by: lobo
GPG key ID: A7B62D657EF764F8
2 changed files with 246 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

240
src/commands/contributions.zig
View file

@ -246,6 +246,7 @@ const TimeRange = @import("TimeRange.zig");
const analysis = @import("../analytics/analysis.zig");
const transaction_log = @import("../models/transaction_log.zig");
const portfolio_loader = @import("../portfolio_loader.zig");
const history = @import("../history.zig");
const test_git = @import("../testutil/git.zig");
const Money = @import("../Money.zig");
const Date = zfin.Date;
@ -280,17 +281,21 @@ pub const meta: framework.Meta = .{
\\ No flags (default):
\\ dirty working tree: HEAD vs working copy
\\ clean working tree: HEAD~1 vs HEAD (review last commit)
\\ --since <DATE> commit-at-or-before(DATE) vs HEAD (or
\\ working copy when dirty)
\\ --since <D1> --until <D2> commit-at-or-before(D1) vs
\\ commit-at-or-before(D2)
\\ --since <DATE> the commit recording DATE's snapshot vs
\\ HEAD (or working copy when dirty)
\\ --since <D1> --until <D2> the commits recording each snapshot
\\ --until <DATE> alone rejected; window is ambiguous
\\
\\Date forms: YYYY-MM-DD or relative (1W/1M/1Q/1Y).
\\
\\A date resolves to the commit that recorded `history/DATE-portfolio.srf`,
\\which is the same anchor `zfin compare` uses - so the two agree on what a
\\given week means. With no snapshot committed for that date it falls back to
\\commit-at-or-before(DATE) and says so.
\\
\\Options:
\\ --since <DATE> Earliest side (resolves to commit-at-
\\ or-before).
\\ --since <DATE> Earliest side (the commit recording
\\ that date's snapshot).
\\ --until <DATE> Latest side. Pair with --since.
\\ --commit-before <SPEC> Pin the before commit directly. Same
\\ grammar as --commit-after, minus
@ -346,29 +351,47 @@ pub fn parseArgs(ctx: *framework.RunCtx, cmd_args: []const []const u8) !ParsedAr
// Translate Endpoint to CommitSpec. `--since` produces a date
// endpoint; `--commit-before` a commit_spec endpoint. Map both
// to the same CommitSpec union the existing run() expects.
//
// A date becomes `.snapshot_add`, NOT `.date_at_or_before`, so this command
// resolves a window the same way `compare` does. They used to disagree, and
// the disagreement was load-bearing in the worst way: `compare`'s uncounted
// line says "see `zfin contributions`" and REPORT_RUNBOOK sends you to
// `zfin contributions --since 1W` to explain `compare`'s own contributions
// figure - while `commitAtOrBeforeDate` could land on a commit BEFORE the
// reconcile that the snapshot-anchored side had already stepped past. The two
// then reported different money for the same requested week.
//
// Safe as a default because `.snapshot_add` degrades to exactly the old
// `commitAtOrBeforeDate` behaviour whenever no snapshot for that date was
// committed (see `git.resolveSpec`), so any date without a snapshot resolves
// bit-for-bit as before. Where a snapshot DOES exist it also inherits the
// anchor verification and its warning, which the date path never had.
var parsed: ParsedArgs = .{};
if (tr_result.range.before) |ep| switch (ep) {
.date => |d| parsed.before = .{ .date_at_or_before = d },
.date => |d| parsed.before = .{ .snapshot_add = d },
.commit_spec => |s| parsed.before = s,
.live => unreachable,
};
if (tr_result.range.after) |ep| switch (ep) {
.date => |d| parsed.after = .{ .date_at_or_before = d },
.date => |d| parsed.after = .{ .snapshot_add = d },
.commit_spec => |s| parsed.after = s,
.live => unreachable,
};
// Validate `--since on or before --until` ordering for the
// date-only form, matching legacy behavior. We can only check
// when BOTH sides are date_at_or_before (the commit-spec form
// is opaque until git resolves it).
// Validate `--since on or before --until` ordering for the date form, matching
// legacy behavior. Only checkable when BOTH sides carry a date - a `--commit-*`
// ref is opaque until git resolves it.
//
// Goes through `specDate` rather than testing for one variant. It used to read
// `b == .date_at_or_before`, which silently stopped rejecting inverted windows
// the moment a date started producing `.snapshot_add`.
if (parsed.before) |b| if (parsed.after) |a| {
if (b == .date_at_or_before and a == .date_at_or_before) {
if (b.date_at_or_before.days > a.date_at_or_before.days) {
if (specDate(b)) |bd| if (specDate(a)) |ad| {
if (bd.days > ad.days) {
cli.stderrPrint(io, "Error: --since must be on or before --until.\n");
return error.InvalidArg;
}
}
};
};
return parsed;
}
@ -682,7 +705,17 @@ fn resolveEndpoints(
dirty: bool,
verbosity: Verbosity,
) !Endpoints {
const range = git.resolveCommitRangeSpec(io, arena, env, repo, rel_paths, before, after, dirty) catch |err| {
// Snap any requested date to the snapshot that actually exists at-or-before it,
// BEFORE handing it to git. `compare` has always done this and it is the other
// half of making the two commands agree: a bare `--since 1W` names a date one
// week back, which lands on a weekend more often than not, and there is no
// snapshot for a Saturday. Without snapping, `.snapshot_add` finds no file,
// degrades to `commitAtOrBeforeDate`, and the whole point is lost - which is
// exactly what `--since 1W` did while `compare 1W` was already correct.
const before_snapped = snapSpec(io, arena, repo, rel_paths, before, verbosity, "since");
const after_snapped = snapSpec(io, arena, repo, rel_paths, after, verbosity, "until");
const range = git.resolveCommitRangeSpec(io, arena, env, repo, rel_paths, before_snapped, after_snapped, dirty) catch |err| {
if (verbosity == .verbose) {
switch (err) {
error.NoCommitAtOrBefore => {
@ -711,12 +744,12 @@ 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 buildLabelFromSpecs(arena, range, before, after, dirty);
const label = try buildLabelFromSpecs(arena, range, before_snapped, after_snapped, 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 (before != null and after != null and verbosity == .verbose) {
if (before_snapped != null and after_snapped != null and verbosity == .verbose) {
if (range.after_rev) |after_rev| {
if (std.mem.eql(u8, range.before_rev, after_rev)) {
cli.stderrPrint(io, "Warning: before and after resolve to the same commit; no changes to report.\n");
@ -731,7 +764,10 @@ fn resolveEndpoints(
// `docs/notes/commit-window-edge-case.md` (aka TODO.md) for the
// motivating scenario.
if (verbosity == .verbose) {
try maybeSnapNote(io, arena, env, repo, before, range.before_rev, "before");
try maybeSnapNote(io, arena, env, repo, before_snapped, range.before_rev, "before", range.snapshot_anchor);
if (range.after_rev) |ar| {
try maybeSnapNote(io, arena, env, repo, after_snapped, ar, "after", range.snapshot_anchor_after);
}
}
// Deliberately NOT gated on `verbosity`. `.silent` exists because
@ -741,10 +777,49 @@ fn resolveEndpoints(
// when a snapshot WAS resolved and disagreed with the copy on disk, so it can
// never fire on the case `.silent` was introduced for.
maybeSnapshotAnchorNote(io, range.snapshot_anchor);
maybeSnapshotAnchorNote(io, range.snapshot_anchor_after);
return .{ .range = range, .label = label };
}
/// Replace a `.snapshot_add` date with the nearest snapshot at-or-before it.
///
/// Returns the spec untouched for every other variant, and untouched when no
/// snapshot resolves at all - the degraded path is `git.resolveSpec`'s to handle,
/// and `maybeSnapNote` reports it from there.
///
/// Announces a snap the way `compare` does, because silently moving the date a
/// user typed is how two commands come to disagree about the same week while both
/// look right.
fn snapSpec(
io: std.Io,
arena: std.mem.Allocator,
repo: git.RepoInfo,
rel_paths: []const []const u8,
spec: ?git.CommitSpec,
verbosity: Verbosity,
flag: []const u8,
) ?git.CommitSpec {
const s = spec orelse return spec;
const requested = switch (s) {
.snapshot_add => |d| d,
else => return spec,
};
const first = if (rel_paths.len > 0) rel_paths[0] else return spec;
const pf_path = std.fs.path.join(arena, &.{ repo.root, first }) catch return spec;
const hist_dir = history.deriveHistoryDir(arena, pf_path) catch return spec;
const resolved = history.resolveSnapshotDate(io, arena, hist_dir, requested) catch return spec;
if (resolved.exact) return spec;
if (verbosity == .verbose) {
var buf: [200]u8 = undefined;
const msg = std.fmt.bufPrint(&buf, "(--{s} {f} has no snapshot; using {f}, the nearest at-or-before)\n", .{ flag, requested, resolved.actual }) catch "";
if (msg.len > 0) cli.stderrPrint(io, msg);
}
return .{ .snapshot_add = resolved.actual };
}
/// Say something when the snapshot anchor was not clean.
///
/// Both cases mean the same thing operationally: the value side of a comparison
@ -761,14 +836,14 @@ fn resolveEndpoints(
/// slip that will recur.
fn maybeSnapshotAnchorNote(io: std.Io, anchor: ?git.SnapshotAnchor) void {
const a = anchor orelse return;
var buf: [420]u8 = undefined;
var buf: [460]u8 = undefined;
if (a.corrected_from) |from| {
const msg = std.fmt.bufPrint(&buf, "Note: the {f} snapshot was regenerated after it was first committed; anchoring attribution on {s} rather than {s}, which described different positions.\n", .{ a.date, shortSha(a.commit), shortSha(from) }) catch return;
const msg = std.fmt.bufPrint(&buf, "Note: the {f} snapshot was regenerated after it was first committed; anchoring the {s} side on {s} rather than {s}, which described different positions.\n", .{ a.date, a.side.label(), shortSha(a.commit), shortSha(from) }) catch return;
cli.stderrPrint(io, msg);
return;
}
if (a.unmatched) {
const msg = std.fmt.bufPrint(&buf, "Warning: no commit of the {f} snapshot matches the copy on disk, so the value and attribution sides describe different portfolios. Contributions and gains for this window are NOT reliable - re-run `zfin snapshot --force` for that date and commit it alongside portfolio.srf.\n", .{a.date}) catch return;
const msg = std.fmt.bufPrint(&buf, "Warning: no commit of the {f} snapshot matches the copy on disk, so the {s} side's values and attribution describe different portfolios. Contributions and gains for this window are NOT reliable - re-run `zfin snapshot --force` for that date and commit it alongside portfolio.srf.\n", .{ a.date, a.side.label() }) catch return;
cli.stderrPrint(io, msg);
}
}
@ -797,12 +872,23 @@ fn maybeSnapNote(
spec: ?git.CommitSpec,
resolved_ref: []const u8,
label: []const u8,
/// The resolved anchor for this side, when the spec was `.snapshot_add`. Null
/// means no snapshot for that date was committed and resolution FELL BACK to
/// `commitAtOrBeforeDate` - which is precisely the case this note is for.
anchor: ?git.SnapshotAnchor,
) !void {
const s = spec orelse return;
const requested_date = switch (s) {
.date_at_or_before => |d| d,
else => return,
};
const requested_date = specDate(s) orelse return;
// A snapshot-anchored side that actually found its snapshot is not drifting -
// it is pinned to the commit that recorded that date, which is the point. Only
// the degraded path, where no snapshot existed and it fell back to a date, can
// land arbitrarily far from what was asked for.
//
// Written as "did the anchor resolve" rather than "is the spec a date", because
// the previous form (`.date_at_or_before => |d| d, else => return`) silenced
// this note outright the moment `--since` began producing `.snapshot_add`.
if (s == .snapshot_add and anchor != null) return;
// Get the committer-date of the resolved commit. `%ct` gives a
// Unix timestamp.
@ -816,8 +902,9 @@ fn maybeSnapNote(
var msg_buf: [320]u8 = undefined;
const msg = std.fmt.bufPrint(
&msg_buf,
"(git {s} uses commit {s} from {f}, {d} day{s} before requested {f} - " ++
"use --commit-{s} HEAD or a later date to pin to your latest reconciliation commit)\n",
"(git {s} uses commit {s} from {f}, {d} day{s} before requested {f}; no " ++
"snapshot for that date is committed, so there was nothing to anchor on - " ++
"use --commit-{s} to pin a revision explicitly)\n",
.{
label,
shortSha(resolved_ref),
@ -876,6 +963,18 @@ fn buildLabelFromSpecs(
return buildLabel(arena, range, before_date, after_date, dirty);
}
/// The date a spec carries, for the two variants that carry one.
///
/// Exists so callers stop pattern-matching a single variant. Both `--since 1W` and
/// `compare`'s snapshot anchoring are "a date the user named"; a check written
/// against `.date_at_or_before` alone goes quietly blind when that mapping changes.
fn specDate(spec: git.CommitSpec) ?zfin.Date {
return switch (spec) {
.date_at_or_before, .snapshot_add => |d| d,
.git_ref, .working_copy => null,
};
}
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) {
@ -3882,16 +3981,50 @@ test "parseArgs: empty args produces both null" {
try testing.expect(parsed.after == null);
}
test "parseArgs: --since populates before as date_at_or_before" {
test "parseArgs: --since populates before as snapshot_add" {
// `.snapshot_add`, not `.date_at_or_before`. This is what makes a date resolve
// the same way `compare` resolves one, so the two commands report the same
// money for the same requested week. It degrades to the old date behaviour
// inside `git.resolveSpec` when no snapshot exists, so nothing is lost.
const today = zfin.Date.fromYmd(2026, 5, 9);
const args = [_][]const u8{ "--since", "2026-04-01" };
const parsed = try parseArgsForTest(today, &args);
switch (parsed.before.?) {
.date_at_or_before => |d| try testing.expect(d.eql(zfin.Date.fromYmd(2026, 4, 1))),
.snapshot_add => |d| try testing.expect(d.eql(zfin.Date.fromYmd(2026, 4, 1))),
else => try testing.expect(false),
}
}
test "parseArgs: --until populates after as snapshot_add too" {
// Both endpoints, because both can be wrong and they fail in opposite
// directions - a before anchor that is too early re-counts a window, an after
// anchor that is too early drops it.
const today = zfin.Date.fromYmd(2026, 5, 9);
const args = [_][]const u8{ "--since", "2026-04-01", "--until", "2026-04-15" };
const parsed = try parseArgsForTest(today, &args);
switch (parsed.after.?) {
.snapshot_add => |d| try testing.expect(d.eql(zfin.Date.fromYmd(2026, 4, 15))),
else => try testing.expect(false),
}
}
test "specDate: reads the date out of either date-carrying variant" {
// The ordering check used to test for `.date_at_or_before` by hand and went
// blind the moment a date started producing `.snapshot_add`. Routing both
// through one accessor is what stops that recurring.
const d = zfin.Date.fromYmd(2026, 4, 1);
try testing.expect(specDate(.{ .date_at_or_before = d }).?.eql(d));
try testing.expect(specDate(.{ .snapshot_add = d }).?.eql(d));
try testing.expect(specDate(.{ .git_ref = "HEAD" }) == null);
try testing.expect(specDate(.working_copy) == null);
}
test "parseArgs: an inverted window is still rejected after the spec change" {
const today = zfin.Date.fromYmd(2026, 5, 9);
const args = [_][]const u8{ "--since", "2026-04-15", "--until", "2026-04-01" };
try testing.expectError(error.InvalidArg, parseArgsForTest(today, &args));
}
test "parseArgs: --since + --until populates both" {
const today = zfin.Date.fromYmd(2026, 5, 9);
const args = [_][]const u8{ "--since", "2026-04-01", "--until", "2026-05-01" };
@ -6152,6 +6285,53 @@ test "computeReport: per-account totals separate drip_confirmed from rollup" {
// which requires a real repo and is covered by `src/git.zig` tests
// plus manual smoke-testing.
test "maybeSnapNote fires for a degraded snapshot_add, and stays quiet for a resolved one" {
// The regression this guards was nearly shipped. `maybeSnapNote` read
// `.date_at_or_before => |d| d, else => return`, so the moment `--since` began
// producing `.snapshot_add` the note went silent - including for a date far
// outside the snapshot series, which is the one case it exists to catch.
//
// Snapshot-anchored-and-resolved is not drift: the anchor is deliberately the
// commit that recorded that date. Snapshot-anchored-and-degraded is, because
// resolution fell back to whatever commit happened to precede the date.
const allocator = std.testing.allocator;
if (!test_git.available(allocator)) return;
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
var path_buf: [std.fs.max_path_bytes]u8 = undefined;
const dir_len = try tmp.dir.realPathFile(std.testing.io, ".", &path_buf);
const dir = path_buf[0..dir_len];
try tmp.dir.writeFile(std.testing.io, .{ .sub_path = "portfolio.srf", .data = "security_type::cash,shares:num:1.00,account::A\n" });
try test_git.run(allocator, dir, null, &.{ "init", "-q" });
try test_git.run(allocator, dir, null, &.{ "config", "user.email", "t@e.com" });
try test_git.run(allocator, dir, null, &.{ "config", "user.name", "T" });
try test_git.run(allocator, dir, null, &.{ "config", "commit.gpgsign", "false" });
try test_git.run(allocator, dir, null, &.{ "add", "." });
try test_git.run(allocator, dir, "2026-01-05T12:00:00", &.{ "commit", "-q", "-m", "one" });
var arena_state = std.heap.ArenaAllocator.init(allocator);
defer arena_state.deinit();
const arena = arena_state.allocator();
var env = try std.testing.environ.createMap(allocator);
defer env.deinit();
const repo = git.findRepo(std.testing.io, arena, &env, dir) catch return;
const far_future = zfin.Date.fromYmd(2026, 6, 1);
// Degraded: no `history/` at all, so `.snapshot_add` fell back to a commit five
// months earlier. `anchor == null` is the signal, and the note must fire.
try maybeSnapNote(std.testing.io, arena, &env, repo, .{ .snapshot_add = far_future }, "HEAD", "before", null);
// Resolved: an anchor came back, so the gap is intentional and there is
// nothing to report. Reaching the timestamp lookup at all would be the bug.
try maybeSnapNote(std.testing.io, arena, &env, repo, .{ .snapshot_add = far_future }, "HEAD", "before", .{
.commit = "HEAD",
.date = far_future,
});
}
test "resolveEndpoints: legacy dirty -> HEAD vs working copy" {
var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator);
defer arena_state.deinit();

38
src/git.zig
View file

@ -116,18 +116,42 @@ pub const CommitRange = struct {
before_rev: []const u8,
/// null = working copy; non-null = a concrete git revision.
after_rev: ?[]const u8,
/// How the before-side anchor was chosen, when it came from a `.snapshot_add`
/// How each anchor was chosen, when that side came from a `.snapshot_add`
/// spec. Non-null only in that case, and only worth reading to WARN - the
/// resolution has already happened. See `snapshotAnchor`.
///
/// BOTH sides, because both can be wrong and they fail in opposite
/// directions: a before anchor that is too early re-counts a window's
/// contributions, an after anchor that is too early drops them. Only the
/// before side was checked at first, which left `compare <D1> <D2>` - the
/// two-snapshot form, where `after` is a real revision rather than the working
/// copy - unverified on the half that silently under-reports.
snapshot_anchor: ?SnapshotAnchor = null,
snapshot_anchor_after: ?SnapshotAnchor = null,
};
/// The outcome of resolving a `.snapshot_add` anchor, so the caller can say
/// something when it was not clean.
/// Which endpoint a `SnapshotAnchor` describes.
pub const AnchorSide = enum {
before,
after,
pub fn label(self: AnchorSide) []const u8 {
return switch (self) {
.before => "before",
.after => "after",
};
}
};
pub const SnapshotAnchor = struct {
commit: []const u8,
/// The snapshot's own date, for the message.
date: Date,
/// Which endpoint this describes, so a message can name it. A reader who is
/// told an anchor moved needs to know which end of the window moved.
side: AnchorSide = .before,
/// Set when the anchor moved FORWARD off `commitThatAdded` because that
/// commit's snapshot described different positions than the snapshot on disk.
/// Names the commit we moved off.
@ -829,10 +853,20 @@ pub fn resolveCommitRangeSpec(
// same git plumbing, and paying for it keeps `resolveSpec` returning a plain
// sha for every other spec rather than threading an optional through all six.
var anchor: ?SnapshotAnchor = null;
var anchor_after: ?SnapshotAnchor = null;
if (before) |b| switch (b) {
.snapshot_add => |d| {
const snap_rel = try snapshotRelPath(arena, rel_paths, d);
anchor = try snapshotAnchor(io, arena, env, repo.root, snap_rel, d);
if (anchor) |*a| a.side = .before;
},
else => {},
};
if (after) |a| switch (a) {
.snapshot_add => |d| {
const snap_rel = try snapshotRelPath(arena, rel_paths, d);
anchor_after = try snapshotAnchor(io, arena, env, repo.root, snap_rel, d);
if (anchor_after) |*x| x.side = .after;
},
else => {},
};
@ -854,7 +888,7 @@ pub fn resolveCommitRangeSpec(
else
"HEAD";
return .{ .before_rev = before_rev, .after_rev = after_rev, .snapshot_anchor = anchor };
return .{ .before_rev = before_rev, .after_rev = after_rev, .snapshot_anchor = anchor, .snapshot_anchor_after = anchor_after };
}
/// Resolve one non-working `CommitSpec` to a string git can consume.