diff --git a/AGENTS.md b/AGENTS.md index 72c6924..f0312c8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -316,6 +316,11 @@ across the codebase so search-and-replace stays trivial): *5678` - Filenames: `Sample_IRA_1234.txt`, `Sample_IRA_5678.txt`, `smpl_1234`, `smpl-ira-1234` +- Holder names: **`Riley` is an approved placeholder holder name** and + appears in existing fixtures (e.g. ` Riley 401(k)`). It is + NOT a real family member. Do not "scrub" it, and do not ask whether it + needs scrubbing - it is already the placeholder. Reuse it rather than + inventing new person names. - Account numbers: `1234`, `5678`, `9012`, `3456`, `7890`, or alphanumeric like `Z123`, `Z111`, `Z222`. Do not use real trailing-digit values from the user's actual accounts file. @@ -386,6 +391,13 @@ Two known classes: approved placeholder vocabulary above - e.g. the real name `Inherited IRA` is a substring of the sanctioned fixture value `Sample Inherited IRA`. +- **Institution names and the placeholder holder name.** Broker brands + (Fidelity, Schwab, Vanguard, ...) appear throughout source legitimately - + in parser names, money-market symbol lists, and doc comments - and are + not PII. Because real account names combine an institution with a + holder, a token list built from `accounts.srf` will match all of those. + `Riley` likewise: it is the approved placeholder holder name, so + ` Riley 401(k)` in a fixture is already scrubbed. So the rule is: **inspect the context of every hit**, and confirm each is either coincidental or a generic category before dismissing it. A diff --git a/src/cache/store.zig b/src/cache/store.zig index dd27385..2206391 100644 --- a/src/cache/store.zig +++ b/src/cache/store.zig @@ -321,8 +321,66 @@ pub const Store = struct { return stats; } - // ── Generic typed API ──────────────────────────────────────── + /// Every cache key on disk, sorted. Caller owns the strings and the outer + /// slice. + /// + /// Deliberately dumb: it lists directory names and makes no judgement about + /// what a key means. A key may be a ticker, a CUSIP, or an EDGAR CIK - this + /// is a generic SRF store and classifying them is not its business. + /// + /// Callers filter by what they actually need, which is usually cheaper and + /// always more accurate than guessing from the name. A candle-staleness + /// sweep, for instance, wants keys that have candle meta, so + /// `readCandleMeta() != null` is both its filter and its data - and it + /// excludes CIK keys, negative-cached symbols and the EDGAR indexes for + /// free, without this function knowing any of those exist. + /// + /// The one exclusion is the store's own `_`-prefixed synthetic keys + /// (`_edgar` for the EDGAR ticker indexes, `_torn` for archived torn-body + /// forensics). That is self-knowledge, not domain knowledge: this store + /// created them. + /// + /// Read-only. Returns empty (not an error) when the cache directory does + /// not exist yet. + pub fn cacheKeys(self: *Store, allocator: std.mem.Allocator) ![][]const u8 { + const io = self.io; + var out = std.ArrayList([]const u8).empty; + errdefer { + for (out.items) |k| allocator.free(k); + out.deinit(allocator); + } + var dir = std.Io.Dir.cwd().openDir(io, self.cache_dir, .{ .iterate = true }) catch + return out.toOwnedSlice(allocator); + defer dir.close(io); + + var iter = dir.iterate(); + while (iter.next(io) catch null) |entry| { + if (entry.kind != .directory) continue; + if (entry.name.len == 0 or entry.name[0] == '_') continue; + const owned = try allocator.dupe(u8, entry.name); + { + errdefer allocator.free(owned); + try out.append(allocator, owned); + } + } + + const keys = try out.toOwnedSlice(allocator); + std.mem.sort([]const u8, keys, {}, struct { + fn lt(_: void, a: []const u8, b: []const u8) bool { + return std.mem.order(u8, a, b) == .lt; + } + }.lt); + return keys; + } + + /// Free a `cacheKeys` result. + pub fn freeCacheKeys(allocator: std.mem.Allocator, keys: [][]const u8) void { + for (keys) |k| allocator.free(k); + allocator.free(keys); + } + + // ── Generic typed API ──────────────────────────────────────── /// Map a model type to its cache DataType. pub fn dataTypeFor(comptime T: type) DataType { return switch (T) { @@ -3761,3 +3819,63 @@ test "deserializePortfolio: underscore digit separators parse (they always did)" try std.testing.expectEqual(@as(usize, 1), p.lots.len); try std.testing.expectApproxEqAbs(@as(f64, 1_234_567), p.lots[0].shares, 0.5); } + +test "cacheKeys: directory names only, sorted, store-internal keys excluded" { + 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); + + // Unsorted on disk, salted with the shapes a real cache holds. Note + // `0000320193` (a CIK) IS returned - classifying keys is the caller's job, + // and a candle sweep drops it for free because it has no candle meta. + for ([_][]const u8{ "NVDA", "AAPL", "_edgar", "_torn", "0000320193", "BRK-B" }) |name| + try tmp.dir.createDir(io, name, std.Io.File.Permissions.default_dir); + (try tmp.dir.createFile(io, "cusip_tickers.srf", .{})).close(io); + + var s = Store.init(io, allocator, dir_path); + const keys = try s.cacheKeys(allocator); + defer Store.freeCacheKeys(allocator, keys); + + try std.testing.expectEqual(@as(usize, 4), keys.len); + try std.testing.expectEqualStrings("0000320193", keys[0]); + try std.testing.expectEqualStrings("AAPL", keys[1]); + try std.testing.expectEqualStrings("BRK-B", keys[2]); + try std.testing.expectEqualStrings("NVDA", keys[3]); +} + +test "cacheKeys: a missing cache directory is empty, not an error" { + const allocator = std.testing.allocator; + const io = std.testing.io; + // First run on a fresh machine: callers must report "nothing cached" + // rather than failing. + var s = Store.init(io, allocator, "/nonexistent/zfin-cache-path"); + const keys = try s.cacheKeys(allocator); + defer Store.freeCacheKeys(allocator, keys); + try std.testing.expectEqual(@as(usize, 0), keys.len); +} + +/// OOM-path wrapper for `checkAllAllocationFailures`. +fn cacheKeysOom(a: std.mem.Allocator, s: *Store) !void { + const keys = try s.cacheKeys(a); + Store.freeCacheKeys(a, keys); +} + +test "cacheKeys: every allocation-failure path unwinds cleanly" { + // The same check on `Portfolio.fetchedSymbols` found a real double-free in + // this exact shape of code - an inner errdefer left armed past the point + // the list took ownership. + 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); + for ([_][]const u8{ "AAPL", "NVDA", "MSFT" }) |name| + try tmp.dir.createDir(io, name, std.Io.File.Permissions.default_dir); + + var s = Store.init(io, allocator, dir_path); + try std.testing.checkAllAllocationFailures(allocator, cacheKeysOom, .{&s}); +} diff --git a/src/commands/cache.zig b/src/commands/cache.zig index 5ab8a15..a340bbd 100644 --- a/src/commands/cache.zig +++ b/src/commands/cache.zig @@ -95,46 +95,26 @@ fn runStats(ctx: *framework.RunCtx) !void { const now_s = std.Io.Timestamp.now(io, .real).toSeconds(); try out.print("Cache directory: {s}\n\n", .{config.cache_dir}); - var dir = std.Io.Dir.cwd().openDir(io, config.cache_dir, .{ .iterate = true }) catch { + // Enumerate via the store rather than walking the directory here. The + // staleness sweep needs the same list, and two hand-rolled walks would + // drift the way the watch-symbol unions did. + var store = Store.init(io, allocator, config.cache_dir); + const symbols = store.cacheKeys(allocator) catch { try out.print(" (empty -- no cached data)\n", .{}); return; }; - defer dir.close(io); + defer Store.freeCacheKeys(allocator, symbols); - // Collect and sort symbol names - var symbols: std.ArrayList([]const u8) = .empty; - defer { - for (symbols.items) |s| allocator.free(s); - symbols.deinit(allocator); - } - - var iter = dir.iterate(); - while (iter.next(io) catch null) |entry| { - if (entry.kind == .directory) { - const name = allocator.dupe(u8, entry.name) catch continue; - symbols.append(allocator, name) catch { - allocator.free(name); - continue; - }; - } - } - - if (symbols.items.len == 0) { + if (symbols.len == 0) { try out.print(" (empty -- no cached data)\n", .{}); return; } - std.mem.sort([]const u8, symbols.items, {}, struct { - fn cmp(_: void, a: []const u8, b: []const u8) bool { - return std.mem.order(u8, a, b) == .lt; - } - }.cmp); - // Track totals var total_size: u64 = 0; var total_files: usize = 0; - for (symbols.items) |symbol| { + for (symbols) |symbol| { try out.print("{s}\n", .{symbol}); // Print header @@ -205,7 +185,7 @@ fn runStats(ctx: *framework.RunCtx) !void { var total_buf: [10]u8 = undefined; try out.print("{d} symbol(s), {d} file(s), {s} total\n", .{ - symbols.items.len, + symbols.len, total_files, formatSize(&total_buf, total_size), });