contributions dates trigger on snapshots for both before and after side

docs/reference/config/projections-srf.md

@@ -67,13 +67,14 @@ sleeve, and would not be comparable against the portfolio row (which is
 renormalized). Both rows therefore describe invested assets only.
 
 The row labels say so, and they are renormalized to match: a leg reads
-`SPYM (89.4% of benchmark)`, not its raw portfolio weight, so that
+`SPY (84.2% of benchmark)`, not its raw portfolio weight, so that
 multiplying the printed weights by the printed returns reproduces the
 printed blend. The blend row names the share of the portfolio it covers,
-`Benchmark (94.9% of portfolio)`, and omits that clause entirely once
+`Benchmark (95.0% of portfolio)`, and omits that clause entirely once
 coverage reaches 99.5%. A leg's raw portfolio weight is still recoverable
-as `of benchmark x of portfolio` -- here `0.894 x 0.949 = 84.9%`, which
+as `of benchmark x of portfolio` -- for an 80/15 split that is
-is the figure the `Target allocation` line reports.
+`0.842 x 0.950 = 80.0%`, which is the figure the `Target allocation` line
+reports.
 
 The year columns are **total return** (dividend-reinvested). The `Week`
 column is price-only, because dividends over seven days are negligible;

src/commands/contributions.zig

@@ -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
+    \\  --since <DATE>           the commit recording DATE's snapshot vs
-    \\                           working copy when dirty)
+    \\                           HEAD (or working copy when dirty)
-    \\  --since <D1> --until <D2>  commit-at-or-before(D1) vs
+    \\  --since <D1> --until <D2>  the commits recording each snapshot
-    \\                             commit-at-or-before(D2)
     \\  --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-
+    \\  --since <DATE>           Earliest side (the commit recording
-    \\                           or-before).
+    \\                           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
+    // Validate `--since on or before --until` ordering for the date form, matching
-    // date-only form, matching legacy behavior. We can only check
+    // legacy behavior. Only checkable when BOTH sides carry a date - a `--commit-*`
-    // when BOTH sides are date_at_or_before (the commit-spec form
+    // ref is opaque until git resolves it.
-    // 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 (specDate(b)) |bd| if (specDate(a)) |ad| {
-            if (b.date_at_or_before.days > a.date_at_or_before.days) {
+            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,12 +764,90 @@ 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
+    // `computeAttributionSpec` calls this speculatively and an unresolvable window
+    // is an ordinary null return - but that is the `compare` path, and `compare`'s
+    // headline is exactly what a straddled anchor corrupts. This note only fires
+    // 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
+/// reads `history/<date>-portfolio.srf` off disk while the attribution side reads
+/// `portfolio.srf` at a commit, and those two can describe different portfolios.
+/// When they do, `gains = delta - contributions` mixes windows and the error is
+/// invisible in the output - it looks like a plausible number. The motivating case
+/// had a review's contributions and a five-figure outflow counted in two
+/// consecutive reports because the anchor predated the reconcile.
+///
+/// `corrected_from` is a note rather than a warning: the anchor moved and the
+/// answer is now right, but the operator should know their history has a commit
+/// where the snapshot and the portfolio disagreed, because that is a staging
+/// slip that will recur.
+fn maybeSnapshotAnchorNote(io: std.Io, anchor: ?git.SnapshotAnchor) void {
+    const a = anchor orelse return;
+    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 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 {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);
+    }
+}
+
 /// 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)".
@@ -761,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) {
+    const requested_date = specDate(s) orelse return;
-        .date_at_or_before => |d| d,
+
-        else => 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.
@@ -780,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} - " ++
+        "(git {s} uses commit {s} from {f}, {d} day{s} before requested {f}; no " ++
-            "use --commit-{s} HEAD or a later date to pin to your latest reconciliation commit)\n",
+            "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),
@@ -840,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) {
@@ -3429,12 +3564,13 @@ fn printNone(out: *std.Io.Writer, color: bool, muted: [3]u8) !void {
     try cli.printFg(out, color, muted, "  (none)\n", .{});
 }
 
-/// Symbol column. 16 because `Engagement Ring` is 15 - illiquid assets carry
+/// Symbol column. 16 rather than 14, because a portfolio may carry illiquid assets
-/// free-form names rather than tickers, so the old 14 was sized for a population
+/// under free-form names instead of tickers, and 15-character names are ordinary.
-/// this report no longer only contains.
+/// The old width was sized for a population this report no longer only contains.
 const sym_w = 16;
 
-/// Account column. 30 because `Fidelity Emil 401(k) Roth BL` is 28.
+/// Account column. 30, because a plan account's full name runs to about 28
+/// characters once it carries a plan type and a sub-account qualifier.
 const acct_w = 30;
 
 /// Pad `s` into a `w`-wide field, always leaving at least one space behind it.
@@ -3442,10 +3578,10 @@ const acct_w = 30;
 /// The guarantee is the point. `{s:<N}` OVERFLOWS instead of truncating, so a
 /// single over-long value both shifted its own row right AND consumed the gutter,
 /// welding itself to the next field. And no fixed `N` avoids it here: symbols in
-/// this report run from a 3-character ticker to a 24-character option
+/// this report run from a 3-character ticker to a ~24-character OCC option
-/// (`AMZN 09/18/2026 280.00 C`), with hand-named illiquid assets in between. So
+/// description, with free-form illiquid asset names in between. So the widths above
-/// the widths above are chosen for the common case and this keeps the rare long
+/// are chosen for the common case and this keeps the rare long one readable - it
-/// one readable - it loses its alignment, not its whitespace.
+/// loses its alignment, not its whitespace.
 fn padTo(out: *std.Io.Writer, s: []const u8, w: usize) !void {
     try out.writeAll(s);
     try out.splatByteAll(' ', if (s.len >= w) 1 else w - s.len);
@@ -3477,16 +3613,17 @@ test "padTo: pads short values to width and never welds a long one" {
     try padTo(&w1, "CASH", 16);
     try std.testing.expectEqualStrings("CASH            ", w1.buffered());
 
-    // Exactly one under: still a gutter, and it is the last width that aligns.
+    // Exactly one under the width: still a gutter, and the last length that aligns.
     var w2 = std.Io.Writer.fixed(&buf);
-    try padTo(&w2, "Engagement Ring", 16);
+    try padTo(&w2, "Fifteen Chars15", 16);
-    try std.testing.expectEqualStrings("Engagement Ring ", w2.buffered());
+    try std.testing.expectEqualStrings("Fifteen Chars15 ", w2.buffered());
 
     // Over: alignment is gone, whitespace is not. `{s:<16}` produced no space
-    // here at all, which ran the value into the next column.
+    // here at all, which ran the value into the next column. An OCC option
+    // description is the realistic overflow.
     var w3 = std.Io.Writer.fixed(&buf);
-    try padTo(&w3, "AMZN 09/18/2026 280.00 C", 16);
+    try padTo(&w3, "ZZZZ 01/01/2030 100.00 C", 16);
-    try std.testing.expectEqualStrings("AMZN 09/18/2026 280.00 C ", w3.buffered());
+    try std.testing.expectEqualStrings("ZZZZ 01/01/2030 100.00 C ", w3.buffered());
 }
 
 fn printTotalLine(out: *std.Io.Writer, label: []const u8, v: f64, color: bool, hdr: [3]u8) !void {
@@ -3844,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" };
@@ -6114,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();

src/git.zig

@@ -116,6 +116,51 @@ pub const CommitRange = struct {
     before_rev: []const u8,
     /// null = working copy; non-null = a concrete git revision.
     after_rev: ?[]const u8,
+    /// 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.
+    corrected_from: ?[]const u8 = null,
+    /// Set when NO commit touching the snapshot describes the same positions as
+    /// the copy on disk. `commit` is then a best effort and the value side and the
+    /// attribution side are KNOWN to disagree - which is the one situation that
+    /// silently corrupts a weekly headline, so it must be said out loud.
+    unmatched: bool = false,
 };
 
 // ── Implementation ───────────────────────────────────────────
@@ -428,9 +473,17 @@ pub fn lastCommitTimestampForPath(
 /// The weekly review writes `history/<date>-portfolio.srf` for each market day and
 /// commits the whole batch together with the reconciled `portfolio.srf`. So the
 /// commit that ADDED a given Friday's snapshot IS that week's reconciliation
-/// commit, by construction. Resolving it needs no commit-message convention, is
+/// commit, *provided the batch really was one commit*. Resolving it needs no
-/// unaffected by how many other commits landed that day, and survives later
+/// commit-message convention and is unaffected by how many other commits landed
-/// rewrites of the file's contents.
+/// that day.
+///
+/// That proviso is load-bearing and it has been violated: on 2026-08-29 one commit
+/// added the snapshot and touched no portfolio, and the whole reconcile landed the
+/// next day. Callers wanting the attribution anchor should therefore go through
+/// `snapshotAnchor`, which wraps this and verifies the answer against the snapshot
+/// on disk. This function stays narrow - "when did this path first appear" - and
+/// is deliberately still the STARTING point, because the alternatives below are
+/// all worse in the common case.
 ///
 /// The alternatives all failed in practice:
 ///
@@ -671,6 +724,113 @@ pub fn commitTimestamp(
 ///   3. Both null -> full legacy mode: HEAD~1..HEAD (clean) or
 ///      HEAD..working-copy (dirty). Back-compat with pre-flag
 ///      `zfin contributions` invocations.
+/// Which commit's `portfolio.srf` corresponds to the snapshot being used as the
+/// "then" side.
+///
+/// `commitThatAdded` answers a subtly different question - "when did this file
+/// first appear" - and those diverge whenever the snapshot is REGENERATED in a
+/// later commit that also moved `portfolio.srf`. That happens when a review is
+/// committed in two passes: the first commit carries the snapshot, the second
+/// carries the reconcile and force-refreshes the snapshot to match. Anchoring on
+/// the first commit then puts the window's start BEFORE a whole review's worth of
+/// contributions, so the next window re-counts them and both reports show the same
+/// money. The value side reads the snapshot off DISK and stays right, so only the
+/// attribution side straddles - which is why the totals look plausible.
+///
+/// The rule is deliberately conservative: **keep `commitThatAdded` unless its
+/// snapshot disagrees with the one on disk about POSITIONS.** Two failure modes
+/// have to be told apart and only share counts do it:
+///
+///   - A regenerated snapshot changes `shares`, because the portfolio changed.
+///     That is the case above, and the anchor must move forward.
+///   - A retroactive RESTATEMENT changes prices, tickers and values across many
+///     snapshots at once while every share count stays put. One such commit
+///     rewrote 97 snapshots and also touched `portfolio.srf`, so "last commit
+///     touching the snapshot" and "last commit touching both" BOTH resolve to it
+///     and both collapse the window. Comparing shares ignores it, correctly.
+///
+/// When the anchor does move, it moves to the EARLIEST matching commit rather
+/// than the latest: the latest would land on a restatement in the mixed case.
+///
+/// Returns null when the snapshot was never committed, so the caller can fall back.
+fn snapshotAnchor(
+    io: std.Io,
+    arena: std.mem.Allocator,
+    env: *const std.process.Environ.Map,
+    root: []const u8,
+    snap_rel: []const u8,
+    date: Date,
+) Error!?SnapshotAnchor {
+    const first = (try commitThatAdded(io, arena, env, root, snap_rel)) orelse return null;
+
+    // The on-disk copy is the reference because it is what the value side reads.
+    const disk_path = std.fs.path.join(arena, &.{ root, snap_rel }) catch return error.OutOfMemory;
+    const disk_text = std.Io.Dir.cwd().readFileAlloc(io, disk_path, arena, .limited(32 * 1024 * 1024)) catch
+        // No working copy to compare against (a bare checkout, or the file was
+        // deleted). Nothing to verify, so keep the historical behaviour.
+        return .{ .commit = first, .date = date };
+    const want = positionFingerprint(disk_text);
+
+    const first_text = show(io, arena, env, root, first, snap_rel) catch return .{ .commit = first, .date = date };
+    if (positionFingerprint(first_text) == want) return .{ .commit = first, .date = date };
+
+    // It disagrees. Walk every commit that touched the snapshot, oldest first, and
+    // take the first whose positions match what the value side will read.
+    const touches = listCommitsTouching(io, arena, env, root, snap_rel, null) catch
+        return .{ .commit = first, .date = date, .unmatched = true };
+    var i = touches.len;
+    while (i > 0) {
+        i -= 1;
+        const sha = touches[i].commit;
+        const text = show(io, arena, env, root, sha, snap_rel) catch continue;
+        if (positionFingerprint(text) != want) continue;
+        if (std.mem.eql(u8, sha, first)) return .{ .commit = first, .date = date };
+        return .{ .commit = sha, .date = date, .corrected_from = first };
+    }
+    return .{ .commit = first, .date = date, .unmatched = true };
+}
+
+/// Order-insensitive hash of the (account, lot_symbol, shares) triples in a snapshot.
+///
+/// Deliberately a scan and NOT an srf parse. This is a change DETECTOR, not a
+/// reader: it decides whether two snapshots describe the same positions, and the
+/// asymmetry of the costs sets the bar. A false positive moves the anchor forward
+/// past commits it should not have, silently shrinking a window. A false negative
+/// leaves a week's contributions counted twice in a headline. Both are bad, which
+/// is why the three fields below are chosen precisely rather than generously.
+///
+/// `lot_symbol` and NOT `symbol`. `symbol` is the RESOLVED market ticker, and that
+/// is precisely the field a ticker restatement rewrites - one such commit updated
+/// the resolved ticker on a couple of synthetic lots across ~100 snapshots while
+/// every share count and every price stayed identical. Hashing `symbol` read that
+/// as a position change and moved the anchor forward past a reconcile, which is
+/// the exact regression `commitThatAdded` was chosen to avoid. `lot_symbol` is the
+/// portfolio's own lot identity, equal to `symbol` for ordinary holdings and
+/// stable across a restatement.
+///
+/// Prices, values and cost bases are ignored for the same reason.
+fn positionFingerprint(text: []const u8) u64 {
+    var total: u64 = 0;
+    var lines = std.mem.splitScalar(u8, text, '\n');
+    while (lines.next()) |line| {
+        if (std.mem.indexOf(u8, line, "kind::lot") == null) continue;
+        var h: u64 = 0xcbf29ce484222325;
+        for ([_][]const u8{ "account::", "lot_symbol::", "shares:num:" }) |key| {
+            const at = std.mem.indexOf(u8, line, key) orelse continue;
+            const rest = line[at + key.len ..];
+            const end = std.mem.indexOfScalar(u8, rest, ',') orelse rest.len;
+            for (std.mem.trim(u8, rest[0..end], " \t\r")) |c| {
+                h = (h ^ c) *% 0x100000001b3;
+            }
+            h = (h ^ 0xff) *% 0x100000001b3;
+        }
+        // Summed rather than chained, so record ORDER cannot change the result -
+        // a re-serialised snapshot may legitimately reorder lots.
+        total +%= h;
+    }
+    return total;
+}
+
 pub fn resolveCommitRangeSpec(
     io: std.Io,
     arena: std.mem.Allocator,
@@ -687,6 +847,30 @@ pub fn resolveCommitRangeSpec(
     }
 
     // Resolve each endpoint independently.
+    //
+    // The before side is asked TWICE when it is a `.snapshot_add`: once here for
+    // the sha and once for the anchor detail the caller warns on. Both hit the
+    // 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 => {},
+    };
+
     const before_rev: []const u8 = if (before) |b|
         try resolveSpec(io, arena, env, repo, rel_paths, b)
     else if (dirty)
@@ -704,7 +888,7 @@ pub fn resolveCommitRangeSpec(
     else
         "HEAD";
 
-    return .{ .before_rev = before_rev, .after_rev = after_rev };
+    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.
@@ -723,7 +907,7 @@ fn resolveSpec(io: std.Io, arena: std.mem.Allocator, env: *const std.process.Env
         },
         .snapshot_add => |d| blk: {
             const snap_rel = try snapshotRelPath(arena, rel_paths, d);
-            if (try commitThatAdded(io, arena, env, repo.root, snap_rel)) |sha| break :blk sha;
+            if (try snapshotAnchor(io, arena, env, repo.root, snap_rel, d)) |a| break :blk a.commit;
             // No committed snapshot for that date. Fall back to the date behaviour
             // rather than erroring: a portfolio kept without a history directory is
             // a legitimate configuration, and this spec should degrade to the old
@@ -1109,3 +1293,116 @@ test "snapshotRelPath places history beside the portfolio, not at the repo root"
     const empty = try snapshotRelPath(arena, &.{}, d);
     try std.testing.expectEqualStrings("history/2026-08-21-portfolio.srf", empty);
 }
+
+test "positionFingerprint: shares matter, resolved ticker and prices do not" {
+    const base =
+        \\kind::lot,symbol::AAA,lot_symbol::SYNTH-A,account::Acct One,shares:num:100.5,price:num:10.00
+        \\kind::lot,symbol::BBB,lot_symbol::BBB,account::Acct Two,shares:num:2000,price:num:20.00
+        \\
+    ;
+    // A ticker restatement: the RESOLVED symbol moves while `lot_symbol`, shares
+    // and account do not. Prices move too. Must read as unchanged.
+    const restated =
+        \\kind::lot,symbol::AAAX,lot_symbol::SYNTH-A,account::Acct One,shares:num:100.5,price:num:10.00
+        \\kind::lot,symbol::BBB,lot_symbol::BBB,account::Acct Two,shares:num:2000,price:num:31.00
+        \\
+    ;
+    try std.testing.expectEqual(positionFingerprint(base), positionFingerprint(restated));
+
+    // A withdrawal: shares move. This is the case that has to be caught.
+    const withdrawn =
+        \\kind::lot,symbol::AAA,lot_symbol::SYNTH-A,account::Acct One,shares:num:100.5,price:num:10.00
+        \\kind::lot,symbol::BBB,lot_symbol::BBB,account::Acct Two,shares:num:1500,price:num:20.00
+        \\
+    ;
+    try std.testing.expect(positionFingerprint(base) != positionFingerprint(withdrawn));
+
+    // Record order is not content: a re-serialised snapshot may reorder lots.
+    const reordered =
+        \\kind::lot,symbol::BBB,lot_symbol::BBB,account::Acct Two,shares:num:2000,price:num:20.00
+        \\kind::lot,symbol::AAA,lot_symbol::SYNTH-A,account::Acct One,shares:num:100.5,price:num:10.00
+        \\
+    ;
+    try std.testing.expectEqual(positionFingerprint(base), positionFingerprint(reordered));
+
+    // Same lot and shares in a different account is not the same portfolio.
+    const here = "kind::lot,symbol::BBB,lot_symbol::BBB,account::Acct Two,shares:num:2000\n";
+    const there = "kind::lot,symbol::BBB,lot_symbol::BBB,account::Acct Three,shares:num:2000\n";
+    try std.testing.expect(positionFingerprint(here) != positionFingerprint(there));
+}
+
+test "snapshotAnchor moves off the add-commit only when positions changed" {
+    // The two-pass-commit shape, reproduced: commit A adds the snapshot, commit B
+    // regenerates it with a withdrawal applied. Anchoring on A puts a whole
+    // review's contributions inside the FOLLOWING window.
+    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.createDirPath(std.testing.io, "history");
+    const snap = "history/2026-08-28-portfolio.srf";
+    const v_added = "kind::lot,symbol::BBB,lot_symbol::BBB,account::Acct Two,shares:num:2000,price:num:20.00\n";
+    const v_final = "kind::lot,symbol::BBB,lot_symbol::BBB,account::Acct Two,shares:num:1500,price:num:20.00\n";
+
+    try tmp.dir.writeFile(std.testing.io, .{ .sub_path = snap, .data = v_added });
+    try test_git.run(allocator, dir, null, &.{ "init", "-q" });
+    try test_git.run(allocator, dir, null, &.{ "config", "user.email", "test@example.com" });
+    try test_git.run(allocator, dir, null, &.{ "config", "user.name", "Test" });
+    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-08-29T12:00:00", &.{ "commit", "-q", "-m", "adds snapshot only" });
+
+    var env = gitTestEnv(allocator);
+    defer env.deinit();
+
+    const first = (try commitThatAdded(std.testing.io, allocator, &env, dir, snap)).?;
+    defer allocator.free(first);
+
+    var arena_state = std.heap.ArenaAllocator.init(allocator);
+    defer arena_state.deinit();
+    const arena = arena_state.allocator();
+    const d = Date.fromYmd(2026, 8, 28);
+
+    // While the on-disk copy still matches the add-commit, nothing moves and
+    // nothing is reported.
+    const clean = (try snapshotAnchor(std.testing.io, arena, &env, dir, snap, d)).?;
+    try std.testing.expectEqualStrings(first, clean.commit);
+    try std.testing.expect(clean.corrected_from == null);
+    try std.testing.expect(!clean.unmatched);
+
+    // Now regenerate the snapshot with the withdrawal applied, and commit.
+    try tmp.dir.writeFile(std.testing.io, .{ .sub_path = snap, .data = v_final });
+    try test_git.run(allocator, dir, null, &.{ "add", "." });
+    try test_git.run(allocator, dir, "2026-08-30T12:00:00", &.{ "commit", "-q", "-m", "regenerate with reconcile" });
+
+    const moved = (try snapshotAnchor(std.testing.io, arena, &env, dir, snap, d)).?;
+    try std.testing.expect(!std.mem.eql(u8, first, moved.commit));
+    try std.testing.expect(moved.corrected_from != null);
+    try std.testing.expectEqualStrings(first, moved.corrected_from.?);
+    try std.testing.expect(!moved.unmatched);
+
+    // A pure ticker restatement on top must NOT move it again - that is the
+    // bulk-restatement case, and moving would swallow a reconcile.
+    const v_restated = "kind::lot,symbol::BBBX,lot_symbol::BBB,account::Acct Two,shares:num:1500,price:num:31.00\n";
+    try tmp.dir.writeFile(std.testing.io, .{ .sub_path = snap, .data = v_restated });
+    try test_git.run(allocator, dir, null, &.{ "add", "." });
+    try test_git.run(allocator, dir, "2026-08-31T12:00:00", &.{ "commit", "-q", "-m", "restate ticker" });
+
+    const after_restate = (try snapshotAnchor(std.testing.io, arena, &env, dir, snap, d)).?;
+    try std.testing.expectEqualStrings(moved.commit, after_restate.commit);
+
+    // And when the working copy describes positions no commit ever held, say so
+    // rather than picking one - the two sides genuinely disagree.
+    try tmp.dir.writeFile(std.testing.io, .{
+        .sub_path = snap,
+        .data = "kind::lot,symbol::BBB,lot_symbol::BBB,account::Acct Two,shares:num:1.0,price:num:20.00\n",
+    });
+    const orphan = (try snapshotAnchor(std.testing.io, arena, &env, dir, snap, d)).?;
+    try std.testing.expect(orphan.unmatched);
+    try std.testing.expectEqualStrings(first, orphan.commit);
+}

src/views/projections.zig

@@ -152,11 +152,11 @@ pub fn fmtAllocationNote(buf: []u8, target_stock_pct: ?f64, current_stock_pct: f
 /// weight present (`benchmark.blendOptional`), which is what stops a cash sleeve
 /// dragging the benchmark down against a portfolio return that excludes it too.
 ///
-/// The labels have to agree with that arithmetic. Printing the raw 84.9%/10.0%
+/// The labels have to agree with that arithmetic. On an 80/15 split, printing the
-/// beside a row computed from 89.5%/10.5% invites the reader to multiply out and
+/// raw 80.0%/15.0% beside a row computed from 84.2%/15.8% invites the reader to
-/// get 17.14% where the row says 18.06%. Both numbers are correct and the
+/// multiply out and get 16.15% where the row says 17.00%. Both numbers are correct
-/// mismatch is entirely in the caption - the same trap that already cost an hour
+/// and the mismatch is entirely in the caption - the same trap that already cost an
-/// on the `--as-of` table and is why `review` prints that table's date.
+/// hour on the `--as-of` table, which is why that table now prints its own date.
 ///
 /// Nothing is lost by renormalizing the caption: the raw portfolio weight of
 /// either leg is `stock/100 * covered`.