add doctor command
This commit is contained in:
parent
839f0e759f
commit
c832ecf1bf
6 changed files with 1198 additions and 56 deletions
77
TODO.md
77
TODO.md
|
|
@ -141,61 +141,36 @@ have PNG export and were deferred:
|
|||
exports PNG natively today; would need an external dependency
|
||||
or a pixel-buffer-to-format conversion.
|
||||
|
||||
## `zfin doctor` health-check command — priority LOW
|
||||
## Note-field handling: holistic review (priority LOW)
|
||||
|
||||
Front-door command for the file constellation and environment.
|
||||
Answers "is my zfin setup sane?" without making any changes.
|
||||
The lot `note::` field is nominally a human annotation, but it leaks
|
||||
into behavior in at least one place: for CUSIP-like holdings with a
|
||||
note, `valuation.shortLabel(note)` becomes the allocation's
|
||||
`display_symbol` (`src/analytics/valuation.zig`, ~line 396), and the
|
||||
classification engine then matches `metadata.srf` entries against BOTH
|
||||
the allocation symbol AND `display_symbol` (`src/analytics/analysis.zig`,
|
||||
~line 611). So a free-text note can silently become a
|
||||
classification-matching key, which is surprising and fragile (editing
|
||||
a note could change what classifies).
|
||||
|
||||
The configuration surface has grown organically and only the README
|
||||
explains the file layout:
|
||||
Surfaced while building `zfin doctor`: its metadata cross-reference
|
||||
deliberately checks only `lot.priceSymbol()` (the ticker alias or raw
|
||||
symbol), NOT the note-derived `display_symbol`, because coupling a
|
||||
diagnostic to free-text note content felt wrong. That asymmetry is the
|
||||
tell: the cross-ref and the engine now disagree on what counts as
|
||||
"classified" for a note-bearing CUSIP.
|
||||
|
||||
- `portfolio.srf` — lots
|
||||
- `metadata.srf` — classifications
|
||||
- `accounts.srf` — tax types
|
||||
- `projections.srf` — retirement config
|
||||
- `transaction_log.srf` — internal transfers
|
||||
- `imported_values.srf` — back-history
|
||||
- `history/*-portfolio.srf` — snapshots
|
||||
- `~/.config/zfin/keys.srf` — keybinds
|
||||
- `~/.config/zfin/theme.srf` — colors
|
||||
Do a pass over every `note` consumer and classify each use as
|
||||
display-only vs behavior-affecting; decide whether note-derived values
|
||||
should ever be matching keys, document/justify any that stay, and then
|
||||
reconcile `doctor`'s metadata cross-ref with whatever the engine
|
||||
settles on. Starting points (grep `\.note` and `note::`):
|
||||
|
||||
Plus 5 API keys, a cache directory, and an optional `ZFIN_SERVER`.
|
||||
|
||||
### v1 scope (full health check)
|
||||
|
||||
- **File inventory + parse check.** For each of the files above:
|
||||
does it exist, where was it resolved from (cwd vs `ZFIN_HOME` vs
|
||||
`~/.config/zfin`), and does it parse cleanly. No fixes.
|
||||
- **Cross-reference checks.** Every account in `portfolio.srf`
|
||||
has an `accounts.srf` entry. Every held symbol has a
|
||||
`metadata.srf` entry (or is opted out). Every snapshot file
|
||||
parses as a portfolio. `transaction_log.srf` records reference
|
||||
real account names.
|
||||
- **Environment audit.** Which API keys are set (presence only,
|
||||
never the value). Cache size + symbol count. `ZFIN_SERVER`
|
||||
reachability if set (HEAD/OPTIONS, low timeout). Staleness of
|
||||
hand-maintained data files (T-bill rates, Shiller `ie_data.csv`)
|
||||
— same registry as `data/staleness.zig`.
|
||||
- **Output shape.** Sectioned, color-coded. Every check is
|
||||
`OK` / `WARN` / `FAIL` with a one-line context. Exit code 0 on
|
||||
all-OK or warnings only; non-zero only on FAIL. Suitable for
|
||||
CI / cron / pre-commit.
|
||||
|
||||
### Driver
|
||||
|
||||
The file constellation has grown to nine files in three locations,
|
||||
plus five API keys, plus the cache, plus the optional server.
|
||||
Today only the README explains the structure. A `doctor` command
|
||||
surfaces it, catches regressions, and is the obvious place to point
|
||||
new users (or to point future-self after a long break).
|
||||
|
||||
### Open question for when this is picked up
|
||||
|
||||
Does this *replace* the portfolio-hygiene portion of `audit`, or
|
||||
live alongside it? Probably alongside — `audit` is reconciliation
|
||||
against external broker exports, `doctor` is internal-state
|
||||
validation. But worth confirming the boundary before implementing
|
||||
to avoid duplicated checks.
|
||||
- `valuation.shortLabel` -> `display_symbol`, used as a classification
|
||||
match key in `analysis.zig` (the main offender).
|
||||
- Cash / illiquid / CD row rendering (display labels; likely fine).
|
||||
- `transaction_log` transfer `note` (annotation).
|
||||
- audit / contributions matchers (do any key off notes?).
|
||||
|
||||
## Split `audit.zig` into per-broker reconcilers — priority LOW
|
||||
|
||||
|
|
|
|||
75
src/cache/store.zig
vendored
75
src/cache/store.zig
vendored
|
|
@ -274,6 +274,52 @@ pub const Store = struct {
|
|||
};
|
||||
}
|
||||
|
||||
/// Aggregate on-disk cache statistics.
|
||||
pub const DiskStats = struct { symbols: usize = 0, files: usize = 0, bytes: u64 = 0 };
|
||||
|
||||
/// Walk the cache directory read-only and tally symbol
|
||||
/// subdirectories, total data files, and total bytes. Top-level
|
||||
/// files (e.g. `cusip_tickers.srf`) count toward `files`/`bytes`
|
||||
/// but not `symbols`. Returns all-zero (not an error) when the
|
||||
/// cache directory does not exist yet. No mutation, no fetches.
|
||||
pub fn diskStats(self: *Store) DiskStats {
|
||||
const io = self.io;
|
||||
var stats: DiskStats = .{};
|
||||
var dir = std.Io.Dir.cwd().openDir(io, self.cache_dir, .{ .iterate = true }) catch return stats;
|
||||
defer dir.close(io);
|
||||
|
||||
var iter = dir.iterate();
|
||||
while (iter.next(io) catch null) |entry| {
|
||||
switch (entry.kind) {
|
||||
.file => {
|
||||
const path = std.fs.path.join(self.allocator, &.{ self.cache_dir, entry.name }) catch continue;
|
||||
defer self.allocator.free(path);
|
||||
const st = std.Io.Dir.cwd().statFile(io, path, .{}) catch continue;
|
||||
stats.files += 1;
|
||||
stats.bytes += st.size;
|
||||
},
|
||||
.directory => {
|
||||
stats.symbols += 1;
|
||||
const subpath = std.fs.path.join(self.allocator, &.{ self.cache_dir, entry.name }) catch continue;
|
||||
defer self.allocator.free(subpath);
|
||||
var sub = std.Io.Dir.cwd().openDir(io, subpath, .{ .iterate = true }) catch continue;
|
||||
defer sub.close(io);
|
||||
var sub_iter = sub.iterate();
|
||||
while (sub_iter.next(io) catch null) |f| {
|
||||
if (f.kind != .file) continue;
|
||||
const fpath = std.fs.path.join(self.allocator, &.{ subpath, f.name }) catch continue;
|
||||
defer self.allocator.free(fpath);
|
||||
const st = std.Io.Dir.cwd().statFile(io, fpath, .{}) catch continue;
|
||||
stats.files += 1;
|
||||
stats.bytes += st.size;
|
||||
}
|
||||
},
|
||||
else => {},
|
||||
}
|
||||
}
|
||||
return stats;
|
||||
}
|
||||
|
||||
// ── Generic typed API ────────────────────────────────────────
|
||||
|
||||
/// Map a model type to its cache DataType.
|
||||
|
|
@ -1952,6 +1998,35 @@ test "writeMerged Dividend: union sorted desc, new entry added" {
|
|||
try std.testing.expect(result.data[2].ex_date.eql(Date.fromYmd(2024, 2, 15)));
|
||||
}
|
||||
|
||||
test "diskStats: empty cache is all zeros; populated counts symbols/files/bytes" {
|
||||
const allocator = std.testing.allocator;
|
||||
const io = std.testing.io;
|
||||
var tmp = std.testing.tmpDir(.{});
|
||||
defer tmp.cleanup();
|
||||
const dir_path = try tmp.dir.realPathFileAlloc(io, ".", allocator);
|
||||
defer allocator.free(dir_path);
|
||||
|
||||
var s = Store.init(io, allocator, dir_path);
|
||||
|
||||
// Fresh tmp dir → nothing cached.
|
||||
const empty = s.diskStats();
|
||||
try std.testing.expectEqual(@as(usize, 0), empty.symbols);
|
||||
try std.testing.expectEqual(@as(usize, 0), empty.files);
|
||||
try std.testing.expectEqual(@as(u64, 0), empty.bytes);
|
||||
|
||||
// Two symbols; AAA carries two data types, BBB one.
|
||||
var divs = [_]Dividend{.{ .ex_date = Date.fromYmd(2024, 5, 15), .amount = 0.50, .type = .regular }};
|
||||
var splits = [_]Split{.{ .date = Date.fromYmd(2020, 8, 31), .numerator = 4, .denominator = 1 }};
|
||||
s.write(Dividend, "AAA", divs[0..], .{ .seconds = Ttl.dividends });
|
||||
s.write(Split, "AAA", splits[0..], .{ .seconds = Ttl.splits });
|
||||
s.write(Dividend, "BBB", divs[0..], .{ .seconds = Ttl.dividends });
|
||||
|
||||
const stats = s.diskStats();
|
||||
try std.testing.expectEqual(@as(usize, 2), stats.symbols); // AAA, BBB
|
||||
try std.testing.expectEqual(@as(usize, 3), stats.files); // AAA/{dividends,splits} + BBB/dividends
|
||||
try std.testing.expect(stats.bytes > 0);
|
||||
}
|
||||
|
||||
test "writeMerged Dividend: no-change merge still rewrites to refresh expires" {
|
||||
// The "nothing changed" path used to skip the rewrite as an
|
||||
// optimization. Problem: once the on-disk `#!expires=` aged
|
||||
|
|
|
|||
|
|
@ -289,7 +289,7 @@ fn getFileInfo(io: std.Io, allocator: std.mem.Allocator, cache_dir: []const u8,
|
|||
};
|
||||
}
|
||||
|
||||
fn formatSize(buf: *[10]u8, size: u64) []const u8 {
|
||||
pub fn formatSize(buf: *[10]u8, size: u64) []const u8 {
|
||||
if (size < 1024) {
|
||||
return std.fmt.bufPrint(buf, "{d} B", .{size}) catch "?";
|
||||
} else if (size < 1024 * 1024) {
|
||||
|
|
|
|||
1061
src/commands/doctor.zig
Normal file
1061
src/commands/doctor.zig
Normal file
File diff suppressed because it is too large
Load diff
|
|
@ -87,6 +87,23 @@ pub const entries = [_]StaleEntry{
|
|||
},
|
||||
};
|
||||
|
||||
/// Per-entry refresh verdict as of `as_of`.
|
||||
pub const Status = enum { ok, overdue };
|
||||
|
||||
/// Verdict for a single entry as of `as_of`, applying the same window
|
||||
/// logic as `check` (see the module doc-block). Exposed so callers
|
||||
/// that want an explicit per-entry OK/overdue line (e.g. `zfin doctor`)
|
||||
/// can render every entry, not just the overdue ones that `check`
|
||||
/// emits.
|
||||
pub fn entryStatus(entry: StaleEntry, as_of: Date) Status {
|
||||
const this_years_due = Date.fromYmd(as_of.year(), entry.due_month, entry.due_day);
|
||||
// Not yet nag season.
|
||||
if (as_of.lessThan(this_years_due)) return .ok;
|
||||
// Already refreshed this cycle.
|
||||
if (!entry.last_updated.lessThan(this_years_due)) return .ok;
|
||||
return .overdue;
|
||||
}
|
||||
|
||||
/// Write a warning line for each entry in `entries` that is overdue
|
||||
/// for refresh as of `as_of`. Writes nothing when every entry is
|
||||
/// fresh. Entries are processed in order; multiple warnings are
|
||||
|
|
@ -98,15 +115,12 @@ pub fn check(
|
|||
) !void {
|
||||
var wrote_any = false;
|
||||
for (list) |entry| {
|
||||
if (entryStatus(entry, as_of) == .ok) continue;
|
||||
const this_years_due = Date.fromYmd(
|
||||
as_of.year(),
|
||||
entry.due_month,
|
||||
entry.due_day,
|
||||
);
|
||||
// Not yet nag season.
|
||||
if (as_of.lessThan(this_years_due)) continue;
|
||||
// Already refreshed this cycle.
|
||||
if (!entry.last_updated.lessThan(this_years_due)) continue;
|
||||
|
||||
if (wrote_any) try writer.writeAll("\n");
|
||||
wrote_any = true;
|
||||
|
|
@ -308,3 +322,19 @@ test "real registry compiles and is non-empty" {
|
|||
try std.testing.expect(e.due_day >= 1 and e.due_day <= 31);
|
||||
}
|
||||
}
|
||||
|
||||
test "entryStatus: ok before due date" {
|
||||
const e: StaleEntry = .{ .name = "X", .last_updated = Date.fromYmd(2020, 1, 1), .due_month = 4, .due_day = 1, .source_file = "x" };
|
||||
try std.testing.expectEqual(Status.ok, entryStatus(e, Date.fromYmd(2026, 3, 31)));
|
||||
}
|
||||
|
||||
test "entryStatus: overdue on/after due date when stale" {
|
||||
const e: StaleEntry = .{ .name = "X", .last_updated = Date.fromYmd(2025, 3, 15), .due_month = 4, .due_day = 1, .source_file = "x" };
|
||||
try std.testing.expectEqual(Status.overdue, entryStatus(e, Date.fromYmd(2026, 4, 1)));
|
||||
try std.testing.expectEqual(Status.overdue, entryStatus(e, Date.fromYmd(2026, 6, 15)));
|
||||
}
|
||||
|
||||
test "entryStatus: ok when refreshed this cycle" {
|
||||
const e: StaleEntry = .{ .name = "X", .last_updated = Date.fromYmd(2026, 4, 1), .due_month = 4, .due_day = 1, .source_file = "x" };
|
||||
try std.testing.expectEqual(Status.ok, entryStatus(e, Date.fromYmd(2026, 6, 15)));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -43,6 +43,7 @@ const command_modules = .{
|
|||
|
||||
// Infrastructure
|
||||
.cache = @import("commands/cache.zig"),
|
||||
.doctor = @import("commands/doctor.zig"),
|
||||
.version = @import("commands/version.zig"),
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue