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

longer walkbacks

Browse source
This commit is contained in:
Emil Lerch 2026-07-18 12:46:56 -07:00
parent 6e48954190
commit 1888dfda60
Signed by: lobo
GPG key ID: A7B62D657EF764F8
4 changed files with 274 additions and 167 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

235
src/commands/audit/hygiene.zig
View file

@ -23,6 +23,7 @@ const portfolio_mod = @import("../../models/portfolio.zig");
const Date = @import("../../Date.zig");
const srf = @import("srf");
const git = @import("../../git.zig");
const test_git = @import("../../testutil/git.zig");
const common = @import("common.zig");
const fidelity = @import("fidelity.zig");
@ -36,6 +37,12 @@ const audit_file_max_age_hours = 24;
const audit_file_max_size_non_csv = 512 * 1024; // 512KB, for non-CSV files only
pub const default_stale_days: u32 = 3;
const stale_warning_multiplier: u32 = 2; // yellow -> red at 2× threshold
/// Safety cap on how many portfolio commits the account-cadence walk
/// scans when resolving each account's last-update timestamp. The walk
/// normally early-exits once every account resolves; this bounds the
/// pathological case where an account never changed in tracked history
/// (so it never resolves) from forcing a scan of the entire repo.
const max_history_commits_scanned: usize = 500;
/// Type of a discovered brokerage file.
const BrokerFileKind = enum {
@ -237,6 +244,89 @@ fn lotToString(allocator: std.mem.Allocator, lot: portfolio_mod.Lot) ![]const u8
return std.fmt.allocPrint(allocator, "{f}", .{srf.fmt(portfolio_mod.Lot, &lots, .{ .emit_directives = false })});
}
/// Resolve, per account, the committer timestamp (Unix epoch seconds)
/// of the newest commit in which that account's lots changed in
/// `ri.rel_path`. Walks the file's git history newest-to-oldest,
/// diffing adjacent commit pairs and attributing each change to the
/// newer commit of the pair; results are written into `out`.
///
/// The walk covers full history (no cadence-derived cutoff): an
/// account more than 2x its cadence overdue - exactly the ones we most
/// want an age readout for - would otherwise always fall outside a
/// 2x-cadence window and degrade to "no update history found". It
/// stops early once every account in `all_accounts` has a timestamp,
/// or after scanning `max_history_commits_scanned` commits (the safety
/// valve for a never-changed account).
///
/// Accounts with no detectable change in tracked history are simply
/// absent from `out` (e.g. an account present since before the first
/// portfolio commit, whose initial add has no parent commit to diff
/// against); the caller renders those as "no update history found".
///
/// Keys written into `out` are borrowed from `all_accounts` (stable
/// working-copy account-name pointers), never from the transient
/// historical portfolio strings. Git unavailability degrades to an
/// empty result rather than an error.
fn findLastUpdateTimestamps(
io: std.Io,
allocator: std.mem.Allocator,
env: *const std.process.Environ.Map,
ri: git.RepoInfo,
all_accounts: *const std.StringHashMap(void),
out: *std.StringHashMap(i64),
) !void {
const commits = git.listCommitsTouching(io, allocator, env, ri.root, ri.rel_path, null) catch &.{};
defer git.freeCommitTouches(allocator, commits);
var prev_data: ?[]const u8 = null;
defer if (prev_data) |pd| allocator.free(pd);
for (commits, 0..) |ct, ci| {
// Stop once every account is resolved, or at the scan cap.
if (out.count() >= all_accounts.count()) break;
if (ci >= max_history_commits_scanned) break;
const rev_data = git.show(io, allocator, env, ri.root, ct.commit, ri.rel_path) catch continue;
if (ci > 0) {
if (prev_data) |pd| {
// rev_data is older, pd is newer (commits are newest-first).
var old_pf = zfin.cache.deserializePortfolio(allocator, rev_data) catch {
allocator.free(rev_data);
continue;
};
defer old_pf.deinit();
var new_pf = zfin.cache.deserializePortfolio(allocator, pd) catch {
allocator.free(rev_data);
continue;
};
defer new_pf.deinit();
var mods = findModifiedAccounts(allocator, old_pf, new_pf) catch {
allocator.free(rev_data);
continue;
};
defer mods.deinit();
// The newer commit's timestamp is when these accounts changed.
const update_ts = commits[ci - 1].timestamp;
// Match against stable working-copy account names.
var acct_iter = all_accounts.keyIterator();
while (acct_iter.next()) |stable_name| {
if (out.contains(stable_name.*)) continue;
if (mods.contains(stable_name.*)) {
try out.put(stable_name.*, update_ts);
}
}
}
}
if (prev_data) |pd| allocator.free(pd);
prev_data = rev_data;
}
}
/// Staleness color based on age vs threshold.
/// Returns CLR_MUTED for within threshold, warning for 1-2x, negative for >2x.
fn stalenessColor(age_days: i32, threshold: u32) [3]u8 {
@ -670,68 +760,14 @@ pub fn runHygieneCheck(
}
// Find last update time for each account via git history.
// Walk commits newest-to-oldest, diffing adjacent pairs to find
// which accounts changed. Use working-copy account names as keys
// (stable lifetime) rather than historical portfolio strings.
// Only walk back far enough to hit red status (2× max cadence).
// Walks the portfolio file's full history (no cadence-derived
// cutoff), keyed by stable working-copy account names. Absent
// entries render as "no update history found" below.
var last_update_ts = std.StringHashMap(i64).init(allocator);
defer last_update_ts.deinit();
if (repo_info) |ri| {
// Compute the furthest we need to look back: 2× the max cadence
var max_threshold: u32 = 14; // 2× weekly default
for (account_map.entries) |entry| {
if (entry.update_cadence.thresholdDays()) |td| {
const red = td * stale_warning_multiplier;
if (red > max_threshold) max_threshold = red;
}
}
var since_buf: [32]u8 = undefined;
const since = std.fmt.bufPrint(&since_buf, "{d} days ago", .{max_threshold}) catch "30 days ago";
const commits = git.listCommitsTouching(io, allocator, env, ri.root, ri.rel_path, since) catch &.{};
defer git.freeCommitTouches(allocator, commits);
var prev_data: ?[]const u8 = null;
defer if (prev_data) |pd| allocator.free(pd);
for (commits, 0..) |ct, ci| {
// Stop early if every account already has a timestamp
if (last_update_ts.count() >= all_accounts.count()) break;
const rev_data = git.show(io, allocator, env, ri.root, ct.commit, ri.rel_path) catch continue;
if (ci > 0) {
if (prev_data) |pd| {
// rev_data is older, pd is newer (commits are newest-first)
var old_pf = zfin.cache.deserializePortfolio(allocator, rev_data) catch {
allocator.free(rev_data);
continue;
};
defer old_pf.deinit();
var new_pf = zfin.cache.deserializePortfolio(allocator, pd) catch continue;
defer new_pf.deinit();
var mods = findModifiedAccounts(allocator, old_pf, new_pf) catch continue;
defer mods.deinit();
// The newer commit's timestamp is when these accounts were updated
const update_ts = commits[ci - 1].timestamp;
// Match against stable working-copy account names
var acct_iter = all_accounts.keyIterator();
while (acct_iter.next()) |stable_name| {
if (last_update_ts.contains(stable_name.*)) continue;
if (mods.contains(stable_name.*)) {
try last_update_ts.put(stable_name.*, update_ts);
}
}
}
}
if (prev_data) |pd| allocator.free(pd);
prev_data = rev_data;
}
try findLastUpdateTimestamps(io, allocator, env, ri, &all_accounts, &last_update_ts);
}
// Display overdue accounts
@ -1716,3 +1752,86 @@ test "runHygieneCheck: Section 6 flags an un-opted-in symbol's split, not an opt
try std.testing.expect(std.mem.indexOf(u8, sec6, "NVDA") != null);
try std.testing.expect(std.mem.indexOf(u8, sec6, "AMZN") == null);
}
/// Format a two-account portfolio.srf for the git-history test. Only
/// `shares` varies between revisions, which is enough for
/// findModifiedAccounts to flag the account.
fn testPortfolioSrf(buf: []u8, ira_shares: []const u8, roth_shares: []const u8) ![]const u8 {
return std.fmt.bufPrint(buf, "#!srfv1\n" ++
"symbol::VOO,shares:num:{s},open_date::2026-01-01,open_price:num:1.00,account::Sample IRA\n" ++
"symbol::BND,shares:num:{s},open_date::2026-01-01,open_price:num:1.00,account::Sample Roth\n", .{ ira_shares, roth_shares });
}
test "findLastUpdateTimestamps: resolves an account changed in a non-newest commit" {
const allocator = std.testing.allocator;
const io = std.testing.io;
// Skip if `git` isn't on PATH (CI sandbox without git).
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(io, ".", &path_buf);
const dir = path_buf[0..dir_len];
var buf: [512]u8 = undefined;
// Commit A (oldest): IRA=100, Roth=50.
try tmp.dir.writeFile(io, .{ .sub_path = "portfolio.srf", .data = try testPortfolioSrf(&buf, "100", "50") });
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", "portfolio.srf" });
try test_git.run(allocator, dir, "2026-01-10T12:00:00", &.{ "commit", "-q", "-m", "A" });
// Commit B: IRA changes to 200 (Roth unchanged). This is the change
// the old 2x-cadence lookback window could exclude, leaving IRA as
// "no update history found".
try tmp.dir.writeFile(io, .{ .sub_path = "portfolio.srf", .data = try testPortfolioSrf(&buf, "200", "50") });
try test_git.run(allocator, dir, null, &.{ "add", "portfolio.srf" });
try test_git.run(allocator, dir, "2026-02-15T12:00:00", &.{ "commit", "-q", "-m", "B" });
// Commit C (newest): Roth changes to 60 (IRA unchanged).
try tmp.dir.writeFile(io, .{ .sub_path = "portfolio.srf", .data = try testPortfolioSrf(&buf, "200", "60") });
try test_git.run(allocator, dir, null, &.{ "add", "portfolio.srf" });
try test_git.run(allocator, dir, "2026-07-01T12:00:00", &.{ "commit", "-q", "-m", "C" });
var env = try std.testing.environ.createMap(allocator);
defer env.deinit();
const pf_path = try std.fs.path.join(allocator, &.{ dir, "portfolio.srf" });
defer allocator.free(pf_path);
const ri = try git.findRepo(io, allocator, &env, pf_path);
defer {
allocator.free(ri.root);
allocator.free(ri.rel_path);
}
// Authoritative committer timestamps, newest-first: [0]=C, [1]=B, [2]=A.
const commits = try git.listCommitsTouching(io, allocator, &env, ri.root, ri.rel_path, null);
defer git.freeCommitTouches(allocator, commits);
try std.testing.expectEqual(@as(usize, 3), commits.len);
var all_accounts = std.StringHashMap(void).init(allocator);
defer all_accounts.deinit();
try all_accounts.put("Sample IRA", {});
try all_accounts.put("Sample Roth", {});
var out = std.StringHashMap(i64).init(allocator);
defer out.deinit();
try findLastUpdateTimestamps(io, allocator, &env, ri, &all_accounts, &out);
// Both accounts resolve. Critically "Sample IRA" - last changed in
// commit B, NOT the newest commit - is found rather than absent.
const ira_ts = out.get("Sample IRA") orelse return error.IraUnresolved;
const roth_ts = out.get("Sample Roth") orelse return error.RothUnresolved;
// Each change attributed to the commit that actually made it.
try std.testing.expectEqual(commits[1].timestamp, ira_ts); // B
try std.testing.expectEqual(commits[0].timestamp, roth_ts); // C
try std.testing.expect(ira_ts < roth_ts);
}

5
src/git.zig
View file

@ -126,8 +126,11 @@ pub const CommitRange = struct {
/// out by hooks that runs git against a different repo must explicitly
/// opt out of the inherited env.
///
/// Exposed (pub) so the git-in-test-repo helpers in `testutil/git.zig`
/// reuse this exact scrubbing rather than re-implementing it.
///
/// Caller owns the returned map; free with `.deinit()`.
fn scrubbedEnv(
pub fn scrubbedEnv(
allocator: std.mem.Allocator,
base: *const std.process.Environ.Map,
) std.mem.Allocator.Error!std.process.Environ.Map {

125
src/portfolio_loader.zig
View file

@ -59,6 +59,7 @@ const zfin = @import("root.zig");
const framework = @import("commands/framework.zig");
const stderr = @import("stderr.zig");
const git = @import("git.zig");
const test_git = @import("testutil/git.zig");
const enrichSplits = @import("models/portfolio.zig").enrichSplits;
// ── Portfolio loading ────────────────────────────────────────
@ -806,96 +807,11 @@ test "loadFromBytes: all-empty bytes returns empty portfolio" {
// Kept minimal (one happy-path, one missing-file case); the
// bulk of the logic is in `loadFromBytes`, covered above.
/// Build an environment map from the parent process with the GIT_*
/// variables stripped out.
///
/// When these tests run inside a git hook (pre-commit, prek, etc.),
/// the hook runner sets `GIT_INDEX_FILE`, `GIT_DIR`, and
/// `GIT_WORK_TREE` to point at the outer repo's staging state. Git
/// inherits those env vars unconditionally - `git -C <tmpdir>`
/// changes the CWD but does NOT clear these env vars. The result is
/// that `git init` in our temp dir succeeds but subsequent `git
/// add`/`git commit` operate against the OUTER repo's index, with
/// blob references that don't exist in our temp dir's object store
/// ("invalid object 100644 <hash> for '<outer-repo-path>'").
///
/// The hook runner can't fix this for us; per upstream guidance,
/// hooks (and code shelled out by hooks) that run git against a
/// different repo must explicitly opt out of the inherited env. See
/// https://github.com/j178/prek/issues/1786 for the prek-specific
/// instance and https://pre-commit.com/ for the analogous pre-commit
/// guidance.
fn buildScrubbedEnv(allocator: std.mem.Allocator) !std.process.Environ.Map {
var map = try testing.environ.createMap(allocator);
errdefer map.deinit();
// Strip every GIT_* variable. Iterating-while-removing isn't
// supported on the underlying ArrayHashMap, so collect first
// then remove. Keys are duped because `swapRemove` frees the
// map's owned key buffer, which would otherwise leave us with
// dangling pointers in `keys_to_remove`.
var keys_to_remove: std.ArrayList([]u8) = .empty;
defer {
for (keys_to_remove.items) |k| allocator.free(k);
keys_to_remove.deinit(allocator);
}
var it = map.iterator();
while (it.next()) |entry| {
if (std.mem.startsWith(u8, entry.key_ptr.*, "GIT_")) {
try keys_to_remove.append(allocator, try allocator.dupe(u8, entry.key_ptr.*));
}
}
for (keys_to_remove.items) |key| _ = map.swapRemove(key);
return map;
}
/// Run a one-shot git command in `cwd` for test setup. Panics on
/// failure - these tests can't proceed without a working repo.
///
/// Uses `buildScrubbedEnv` to drop inherited `GIT_*` env vars; see
/// that function's doc comment for why this matters under git
/// hooks.
fn gitInTestRepo(allocator: std.mem.Allocator, cwd: []const u8, argv: []const []const u8) !void {
const full_argv = try allocator.alloc([]const u8, argv.len + 3);
defer allocator.free(full_argv);
full_argv[0] = "git";
full_argv[1] = "-C";
full_argv[2] = cwd;
@memcpy(full_argv[3..], argv);
var env_map = try buildScrubbedEnv(allocator);
defer env_map.deinit();
const result = try std.process.run(allocator, testing.io, .{
.argv = full_argv,
.environ_map = &env_map,
.stdout_limit = .limited(64 * 1024),
});
defer allocator.free(result.stdout);
defer allocator.free(result.stderr);
switch (result.term) {
.exited => |code| if (code != 0) {
std.log.err("git command failed (code {d}): {s}\nstderr: {s}", .{ code, std.mem.join(allocator, " ", argv) catch "?", result.stderr });
return error.GitFailed;
},
else => return error.GitFailed,
}
}
test "loadPortfolioFromPathsAtRev: union of two committed files at HEAD" {
const allocator = testing.allocator;
// Skip if `git` isn't on PATH (CI sandbox without git).
{
const probe = std.process.run(allocator, testing.io, .{
.argv = &.{ "git", "--version" },
.stdout_limit = .limited(1024),
}) catch return;
defer allocator.free(probe.stdout);
defer allocator.free(probe.stderr);
}
if (!test_git.available(allocator)) return;
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
@ -917,12 +833,12 @@ test "loadPortfolioFromPathsAtRev: union of two committed files at HEAD" {
try tmp.dir.writeFile(testing.io, .{ .sub_path = "portfolio.srf", .data = file_a });
try tmp.dir.writeFile(testing.io, .{ .sub_path = "portfolio_other.srf", .data = file_b });
try gitInTestRepo(allocator, dir, &.{ "init", "-q" });
try gitInTestRepo(allocator, dir, &.{ "config", "user.email", "test@example.com" });
try gitInTestRepo(allocator, dir, &.{ "config", "user.name", "Test" });
try gitInTestRepo(allocator, dir, &.{ "config", "commit.gpgsign", "false" });
try gitInTestRepo(allocator, dir, &.{ "add", "portfolio.srf", "portfolio_other.srf" });
try gitInTestRepo(allocator, dir, &.{ "commit", "-q", "-m", "initial" });
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", "portfolio.srf", "portfolio_other.srf" });
try test_git.run(allocator, dir, null, &.{ "commit", "-q", "-m", "initial" });
const p1 = try std.fs.path.join(allocator, &.{ dir, "portfolio.srf" });
defer allocator.free(p1);
@ -950,14 +866,7 @@ test "loadPortfolioFromPathsAtRev: file added later is silently skipped at earli
const allocator = testing.allocator;
// Skip if `git` isn't on PATH.
{
const probe = std.process.run(allocator, testing.io, .{
.argv = &.{ "git", "--version" },
.stdout_limit = .limited(1024),
}) catch return;
defer allocator.free(probe.stdout);
defer allocator.free(probe.stderr);
}
if (!test_git.available(allocator)) return;
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
@ -973,12 +882,12 @@ test "loadPortfolioFromPathsAtRev: file added later is silently skipped at earli
;
try tmp.dir.writeFile(testing.io, .{ .sub_path = "portfolio.srf", .data = file_a });
try gitInTestRepo(allocator, dir, &.{ "init", "-q" });
try gitInTestRepo(allocator, dir, &.{ "config", "user.email", "test@example.com" });
try gitInTestRepo(allocator, dir, &.{ "config", "user.name", "Test" });
try gitInTestRepo(allocator, dir, &.{ "config", "commit.gpgsign", "false" });
try gitInTestRepo(allocator, dir, &.{ "add", "portfolio.srf" });
try gitInTestRepo(allocator, dir, &.{ "commit", "-q", "-m", "initial" });
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", "portfolio.srf" });
try test_git.run(allocator, dir, null, &.{ "commit", "-q", "-m", "initial" });
// Add and commit a second portfolio file. After this commit,
// commit 1 (where portfolio_other.srf doesn't exist yet) is
@ -992,8 +901,8 @@ test "loadPortfolioFromPathsAtRev: file added later is silently skipped at earli
\\
;
try tmp.dir.writeFile(testing.io, .{ .sub_path = "portfolio_other.srf", .data = file_b });
try gitInTestRepo(allocator, dir, &.{ "add", "portfolio_other.srf" });
try gitInTestRepo(allocator, dir, &.{ "commit", "-q", "-m", "add second" });
try test_git.run(allocator, dir, null, &.{ "add", "portfolio_other.srf" });
try test_git.run(allocator, dir, null, &.{ "commit", "-q", "-m", "add second" });
const p1 = try std.fs.path.join(allocator, &.{ dir, "portfolio.srf" });
defer allocator.free(p1);

76
src/testutil/git.zig Normal file
View file

@ -0,0 +1,76 @@
//! Test-only helpers for building throwaway git repositories.
//!
//! Consolidates the "run a git command in a temp repo" pattern shared by
//! the portfolio-loader and audit-hygiene tests, so there is exactly one
//! copy of the `GIT_*` scrubbing + `git -C <dir>` invocation + error
//! handling. The scrubbing itself is delegated to `git.scrubbedEnv` (the
//! same routine production `runGit` uses), so test setup and production
//! agree on how the inherited hook environment is neutralized.
const std = @import("std");
const git = @import("../git.zig");
/// Returns false when `git` isn't on PATH so a caller can skip (early
/// return) rather than fail in a sandbox without git:
///
/// if (!test_git.available(allocator)) return;
pub fn available(allocator: std.mem.Allocator) bool {
const probe = std.process.run(allocator, std.testing.io, .{
.argv = &.{ "git", "--version" },
.stdout_limit = .limited(1024),
}) catch return false;
allocator.free(probe.stdout);
allocator.free(probe.stderr);
return true;
}
/// Run `git -C <cwd> <argv...>` for test setup.
///
/// Inherited `GIT_*` vars are stripped (via `git.scrubbedEnv`) so the
/// command works even when the test process runs inside a git hook whose
/// `GIT_DIR` / `GIT_WORK_TREE` point at the outer repo.
///
/// When `date_iso` is non-null it pins both author and committer dates
/// (e.g. `"2026-02-15T12:00:00"`), which makes commit timestamps
/// deterministic for history-walk tests. Returns `error.GitFailed`
/// (after logging the argv + stderr) on a non-zero exit.
pub fn run(
allocator: std.mem.Allocator,
cwd: []const u8,
date_iso: ?[]const u8,
argv: []const []const u8,
) !void {
const full_argv = try allocator.alloc([]const u8, argv.len + 3);
defer allocator.free(full_argv);
full_argv[0] = "git";
full_argv[1] = "-C";
full_argv[2] = cwd;
@memcpy(full_argv[3..], argv);
var base = try std.testing.environ.createMap(allocator);
defer base.deinit();
var env = try git.scrubbedEnv(allocator, &base);
defer env.deinit();
if (date_iso) |d| {
try env.put("GIT_AUTHOR_DATE", d);
try env.put("GIT_COMMITTER_DATE", d);
}
const result = try std.process.run(allocator, std.testing.io, .{
.argv = full_argv,
.environ_map = &env,
.stdout_limit = .limited(64 * 1024),
});
defer allocator.free(result.stdout);
defer allocator.free(result.stderr);
switch (result.term) {
.exited => |code| if (code != 0) {
const joined = std.mem.join(allocator, " ", argv) catch null;
defer if (joined) |j| allocator.free(j);
std.log.err("git command failed (code {d}): {s}\nstderr: {s}", .{ code, joined orelse "?", result.stderr });
return error.GitFailed;
},
else => return error.GitFailed,
}
}