better errors for malformed portfolio files

This commit is contained in:
Emil Lerch 2026-08-19 15:45:02 -07:00
parent 0dbac7d416
commit 5ae4065d9e
Signed by: lobo
GPG key ID: A7B62D657EF764F8
7 changed files with 548 additions and 10 deletions

View file

@ -547,7 +547,17 @@ pub fn parseAccountsFile(allocator: std.mem.Allocator, data: []const u8) !Accoun
defer it.deinit();
while (try it.next()) |fields| {
const entry = fields.to(AccountTaxEntry, srf_opts.user_edited) catch continue;
const entry = fields.to(AccountTaxEntry, srf_opts.user_edited) catch |err| {
// Skip the account rather than losing the whole file, but
// name the error: a dropped account loses its tax type,
// cadence and carve-outs, and every consumer then silently
// falls back to defaults. Quiet under `zig build test`,
// where fixtures feed malformed records on purpose.
if (!builtin.is_test) {
log.warn("accounts.srf: skipping malformed record: {s}", .{@errorName(err)});
}
continue;
};
// A zero/negative large-lot threshold is nonsensical (zero
// flags every new lot; negative is meaningless). Reject it and

View file

@ -763,7 +763,17 @@ pub fn parseProjectionsConfig(data: ?[]const u8) UserConfig {
var annotation_count: u8 = 0;
while (it.next() catch null) |field_it| {
const rec = field_it.to(SrfProjection, srf_opts.user_edited) catch continue;
const rec = field_it.to(SrfProjection, srf_opts.user_edited) catch |err| {
// Skip the record rather than losing the whole file, but
// name the error: a dropped record reverts that setting to
// its default without saying so. Quiet under
// `zig build test`, where fixtures feed malformed records
// on purpose.
if (!builtin.is_test) {
log.warn("projections.srf: skipping malformed record: {s}", .{@errorName(err)});
}
continue;
};
switch (rec) {
.config => |c| {
config.target_stock_pct = c.target_stock_pct orelse config.target_stock_pct;

250
src/cache/store.zig vendored
View file

@ -2,6 +2,7 @@ const std = @import("std");
const log = std.log.scoped(.cache);
const srf = @import("srf");
const srf_opts = @import("../srf_opts.zig");
const format = @import("../format.zig");
const atomic = @import("../atomic.zig");
const version = @import("../version.zig");
const Date = @import("../Date.zig");
@ -2090,8 +2091,79 @@ pub fn serializePortfolio(allocator: std.mem.Allocator, lots: []const Lot) ![]co
return aw.toOwnedSlice();
}
/// Collected diagnostics for records that could not be parsed. Messages
/// are allocator-owned; the caller frees each one and the list.
///
/// One message per skipped record, so `items.len` is the skip count.
pub const ParseDiagnostics = std.ArrayList([]const u8);
/// Longest stretch of a malformed record echoed back to the user.
///
/// SRF returns which error occurred but not which field caused it, so
/// showing the record is how the user finds the culprit. 120 columns is
/// wide enough for a typical lot to appear whole and narrow enough not
/// to wrap and bury the message it is attached to.
const diag_record_cols: usize = 120;
/// Append a "could not parse this record" diagnostic naming the error
/// and echoing the record.
///
/// `data` is the whole file and `line` is 1-based, so the record text is
/// recovered by counting newlines - `srf`'s `state.current_line` is
/// consumed as fields are read and would only yield the unparsed
/// remainder. Multi-line (`#!long`) records show their first line, which
/// is enough to locate them.
fn appendParseDiag(
allocator: std.mem.Allocator,
diags: *ParseDiagnostics,
data: []const u8,
line: usize,
detail: []const u8,
) !void {
const raw = nthLine(data, line);
const shown = format.truncateToCols(raw, diag_record_cols);
const msg = if (shown.len < raw.len)
try std.fmt.allocPrint(allocator, "line {d}: {s}\n {s}...", .{ line, detail, shown })
else
try std.fmt.allocPrint(allocator, "line {d}: {s}\n {s}", .{ line, detail, shown });
errdefer allocator.free(msg);
try diags.append(allocator, msg);
}
/// The `line`-th line of `data`, 1-based, without its terminator.
/// Returns an empty slice when `line` is out of range. Borrowed.
fn nthLine(data: []const u8, line: usize) []const u8 {
if (line == 0) return data[0..0];
var n: usize = 1;
var it = std.mem.splitScalar(u8, data, '\n');
while (it.next()) |l| : (n += 1) {
if (n == line) return std.mem.trimEnd(u8, l, "\r");
}
return data[0..0];
}
/// Deserialize a portfolio from SRF data. Caller owns the returned Portfolio.
pub fn deserializePortfolio(allocator: std.mem.Allocator, data: []const u8) !Portfolio {
return deserializePortfolioDiag(allocator, data, null);
}
/// `deserializePortfolio`, optionally collecting a diagnostic per
/// skipped record into `diags`.
///
/// A skipped lot silently changes every figure zfin prints - net worth,
/// allocation, contributions, compare - so the caller needs to be able
/// to say so next to those figures rather than hoping a `std.log.warn`
/// on stderr was noticed. `portfolio_loader` collects these into
/// `LoadedPortfolio.warnings`, and `cli.loadPortfolio` prints them.
///
/// Passing null keeps the previous behaviour exactly: warn to the log
/// and carry on. That is what the git-historical and import paths want,
/// where a warning about an old revision would be noise.
pub fn deserializePortfolioDiag(
allocator: std.mem.Allocator,
data: []const u8,
diags: ?*ParseDiagnostics,
) !Portfolio {
var lots: std.ArrayList(Lot) = .empty;
errdefer {
for (lots.items) |lot| {
@ -2115,8 +2187,12 @@ pub fn deserializePortfolio(allocator: std.mem.Allocator, data: []const u8) !Por
// `user_edited` coercion: see `srf_opts.zig` for why hand-edited
// files get different options from cache files. The `catch`
// below still handles genuinely unparseable values.
var lot = fields.to(Lot, srf_opts.user_edited) catch {
std.log.warn("portfolio: could not parse record at line {d}", .{line});
var lot = fields.to(Lot, srf_opts.user_edited) catch |err| {
if (diags) |d| {
try appendParseDiag(allocator, d, data, line, @errorName(err));
} else {
std.log.warn("portfolio: could not parse record at line {d}: {s}", .{ line, @errorName(err) });
}
skipped += 1;
continue;
};
@ -2136,7 +2212,11 @@ pub fn deserializePortfolio(allocator: std.mem.Allocator, data: []const u8) !Por
.cash => try allocator.dupe(u8, "CASH"),
.illiquid => try allocator.dupe(u8, "ILLIQUID"),
else => {
std.log.warn("portfolio: record at line {d} has no symbol, skipping", .{line});
if (diags) |d| {
try appendParseDiag(allocator, d, data, line, "no symbol");
} else {
std.log.warn("portfolio: record at line {d} has no symbol, skipping", .{line});
}
if (lot.note) |n| allocator.free(n);
if (lot.label) |l| allocator.free(l);
if (lot.account) |a| allocator.free(a);
@ -2151,7 +2231,10 @@ pub fn deserializePortfolio(allocator: std.mem.Allocator, data: []const u8) !Por
try lots.append(allocator, lot);
}
if (skipped > 0) {
// Only log the rollup when nobody is collecting: a caller with
// `diags` reports the count itself, in band, and would otherwise say
// it twice.
if (skipped > 0 and diags == null) {
std.log.warn("portfolio: {d} record(s) could not be parsed and were skipped", .{skipped});
}
@ -4171,6 +4254,165 @@ test "deserializePortfolio: underscore digit separators parse (they always did)"
try std.testing.expectApproxEqAbs(@as(f64, 1_234_567), p.lots[0].shares, 0.5);
}
test "deserializePortfolioDiag: a bad enum names the error and echoes the record" {
// The shape `strings_to_numbers` cannot help with: enum coercion
// ignores that option entirely, so a typo'd security_type is the
// realistic way a lot gets dropped. srf reports WHICH error but not
// which field, so the record text is how the user finds `stok`.
const allocator = std.testing.allocator;
const data =
"#!srfv1\n" ++
"symbol::AAPL,shares:num:100,open_date::2024-01-15,open_price:num:140.00\n" ++
"symbol::MSFT,security_type::stok,shares:num:50,open_date::2024-02-01,open_price:num:400.00\n";
var diags: ParseDiagnostics = .empty;
defer {
for (diags.items) |w| allocator.free(w);
diags.deinit(allocator);
}
var p = try deserializePortfolioDiag(allocator, data, &diags);
defer p.deinit();
// The good lot survives; the bad one is dropped, not fatal.
try std.testing.expectEqual(@as(usize, 1), p.lots.len);
try std.testing.expectEqualStrings("AAPL", p.lots[0].symbol);
// One message per skipped record, so len is the count.
try std.testing.expectEqual(@as(usize, 1), diags.items.len);
const msg = diags.items[0];
try std.testing.expect(std.mem.indexOf(u8, msg, "line 3") != null);
try std.testing.expect(std.mem.indexOf(u8, msg, "StringValueNotValidEnumMember") != null);
// The record is echoed so the culprit is visible without opening the file.
try std.testing.expect(std.mem.indexOf(u8, msg, "security_type::stok") != null);
}
test "deserializePortfolioDiag: a symbol-less non-cash record is reported too" {
// The second skip path. A record with no symbol that is not cash or
// illiquid has nothing to key on, and previously vanished with only
// a log line.
const allocator = std.testing.allocator;
const data =
"#!srfv1\n" ++
"shares:num:100,open_date::2024-01-15,open_price:num:140.00\n";
var diags: ParseDiagnostics = .empty;
defer {
for (diags.items) |w| allocator.free(w);
diags.deinit(allocator);
}
var p = try deserializePortfolioDiag(allocator, data, &diags);
defer p.deinit();
try std.testing.expectEqual(@as(usize, 0), p.lots.len);
try std.testing.expectEqual(@as(usize, 1), diags.items.len);
try std.testing.expect(std.mem.indexOf(u8, diags.items[0], "no symbol") != null);
try std.testing.expect(std.mem.indexOf(u8, diags.items[0], "line 2") != null);
}
test "deserializePortfolioDiag: an over-long record is truncated with a marker" {
// A wide direct-indexing or option line would wrap and bury the
// message it is attached to, so the echo is clipped to
// `diag_record_cols` display columns.
const allocator = std.testing.allocator;
var long: std.ArrayList(u8) = .empty;
defer long.deinit(allocator);
try long.appendSlice(allocator, "#!srfv1\nsymbol::MSFT,security_type::stok,shares:num:50,open_date::2024-02-01,open_price:num:400.00,note::");
try long.appendSlice(allocator, "x" ** 200);
try long.append(allocator, '\n');
var diags: ParseDiagnostics = .empty;
defer {
for (diags.items) |w| allocator.free(w);
diags.deinit(allocator);
}
var p = try deserializePortfolioDiag(allocator, long.items, &diags);
defer p.deinit();
try std.testing.expectEqual(@as(usize, 1), diags.items.len);
const msg = diags.items[0];
try std.testing.expect(std.mem.endsWith(u8, msg, "..."));
// Clipped well short of the 200-char note.
try std.testing.expect(msg.len < 200);
}
test "deserializePortfolioDiag: a clean file produces no diagnostics" {
const allocator = std.testing.allocator;
const data =
"#!srfv1\n" ++
"symbol::AAPL,shares:num:100,open_date::2024-01-15,open_price:num:140.00\n";
var diags: ParseDiagnostics = .empty;
defer diags.deinit(allocator);
var p = try deserializePortfolioDiag(allocator, data, &diags);
defer p.deinit();
try std.testing.expectEqual(@as(usize, 1), p.lots.len);
try std.testing.expectEqual(@as(usize, 0), diags.items.len);
}
test "deserializePortfolio: null diags keeps the old skip-and-carry-on behaviour" {
// The 16 call sites that did not opt in must be unaffected: the bad
// record is skipped, the good one survives, nothing is returned
// about it.
const data =
"#!srfv1\n" ++
"symbol::AAPL,shares:num:100,open_date::2024-01-15,open_price:num:140.00\n" ++
"symbol::MSFT,security_type::stok,shares:num:50,open_date::2024-02-01,open_price:num:400.00\n";
var p = try deserializePortfolio(std.testing.allocator, data);
defer p.deinit();
try std.testing.expectEqual(@as(usize, 1), p.lots.len);
}
test "deserializePortfolio: null diags still logs the symbol-less skip" {
// Covers the non-collecting branch of the second skip path: a record
// with no symbol that is neither cash nor illiquid. The 16 call sites
// that never opted in must keep skipping it rather than aborting.
const prev_level = std.testing.log_level;
std.testing.log_level = .err;
defer std.testing.log_level = prev_level;
const data =
"#!srfv1\n" ++
"shares:num:100,open_date::2024-01-15,open_price:num:140.00\n" ++
"symbol::AAPL,shares:num:10,open_date::2024-01-15,open_price:num:150.00\n";
var p = try deserializePortfolio(std.testing.allocator, data);
defer p.deinit();
try std.testing.expectEqual(@as(usize, 1), p.lots.len);
try std.testing.expectEqualStrings("AAPL", p.lots[0].symbol);
}
test "deserializePortfolioDiag: an illiquid record with no symbol gets a placeholder" {
// Cash and illiquid are the two types allowed to omit `symbol` -
// they get a placeholder rather than being skipped.
const allocator = std.testing.allocator;
const data =
"#!srfv1\n" ++
"security_type::illiquid,shares:num:450000,open_date::2020-06-01,open_price:num:350000\n";
var diags: ParseDiagnostics = .empty;
defer diags.deinit(allocator);
var p = try deserializePortfolioDiag(allocator, data, &diags);
defer p.deinit();
try std.testing.expectEqual(@as(usize, 1), p.lots.len);
try std.testing.expectEqualStrings("ILLIQUID", p.lots[0].symbol);
try std.testing.expectEqual(@as(usize, 0), diags.items.len);
}
test "nthLine: 1-based, terminator-free, out of range is empty" {
const data = "alpha\nbeta\r\ngamma";
try std.testing.expectEqualStrings("alpha", nthLine(data, 1));
try std.testing.expectEqualStrings("beta", nthLine(data, 2)); // \r trimmed
try std.testing.expectEqualStrings("gamma", nthLine(data, 3)); // no trailing \n
try std.testing.expectEqualStrings("", nthLine(data, 4));
try std.testing.expectEqualStrings("", nthLine(data, 0));
}
test "cacheKeys: directory names only, sorted, store-internal keys excluded" {
const allocator = std.testing.allocator;
const io = std.testing.io;

View file

@ -499,9 +499,126 @@ pub fn loadPortfolio(ctx: *framework.RunCtx, as_of: zfin.Date) ?LoadedPortfolio
portfolio_loader.applySplitAdjustment(svc, ctx.allocator, &loaded, as_of, fetchOptionsFromPolicy(ctx.globals.refresh_policy));
}
// A failed write here means stdout is broken, so the command's own
// output is about to fail too. Refuse to hand back a portfolio we
// could not attach the caveat to rather than letting it be reported
// as if it were complete.
printPortfolioWarnings(ctx.out, ctx.color, loaded.warnings) catch |err| {
stderrPrint(ctx.io, "Error reporting skipped portfolio records: ");
stderrPrint(ctx.io, @errorName(err));
stderrPrint(ctx.io, "\n");
loaded.deinit(ctx.allocator);
return null;
};
return loaded;
}
/// Report records that were dropped during the load, before whatever the
/// command is about to print.
///
/// This is the one place it happens. All 16 CLI entry points reach a live
/// portfolio through `loadPortfolio`, so a single call there covers them
/// without each command remembering to. It deliberately is NOT called
/// from `portfolio_loader`: `contributions` loads two historical
/// revisions through the same loader, and warnings about a months-old
/// commit would be noise rather than news.
///
/// Written to the command's writer rather than `std.log`, because the
/// point is to be read. A dropped lot silently lowers net worth,
/// allocation percentages and every derived figure; a stderr line is easy
/// to miss with stdout piped or a screen of tables following it. Hence
/// the closing sentence naming the consequence rather than just a count.
///
/// Takes the writer and color flag rather than a `RunCtx` so the exact
/// wording can be asserted in a test - this output is the whole point of
/// collecting the warnings, so it should not be the untested part.
fn printPortfolioWarnings(out: *std.Io.Writer, color: bool, warnings: []const []const u8) !void {
if (warnings.len == 0) return;
try setFg(out, color, CLR_WARNING);
try out.print("warning: {d} record(s) skipped while reading the portfolio\n", .{warnings.len});
try reset(out, color);
for (warnings) |w| {
try setFg(out, color, CLR_MUTED);
try out.print(" {s}\n", .{w});
try reset(out, color);
}
try setFg(out, color, CLR_WARNING);
try out.writeAll(" Figures below exclude them.\n\n");
try reset(out, color);
}
test "printPortfolioWarnings: nothing to report writes nothing" {
var aw: std.Io.Writer.Allocating = .init(std.testing.allocator);
defer aw.deinit();
try printPortfolioWarnings(&aw.writer, false, &.{});
try std.testing.expectEqualStrings("", aw.written());
}
test "printPortfolioWarnings: names the count and states the consequence" {
// The consequence line is the reason this is in band at all: the
// count alone does not tell the user their totals are short.
var aw: std.Io.Writer.Allocating = .init(std.testing.allocator);
defer aw.deinit();
const warnings = [_][]const u8{
"portfolio.srf: line 3: StringValueNotValidEnumMember\n symbol::MSFT,security_type::stok",
"portfolio_closed.srf: line 9: no symbol\n shares:num:5",
};
try printPortfolioWarnings(&aw.writer, false, &warnings);
const text = aw.written();
try std.testing.expect(std.mem.startsWith(u8, text, "warning: 2 record(s) skipped while reading the portfolio\n"));
try std.testing.expect(std.mem.indexOf(u8, text, "portfolio.srf: line 3: StringValueNotValidEnumMember") != null);
try std.testing.expect(std.mem.indexOf(u8, text, "portfolio_closed.srf: line 9: no symbol") != null);
try std.testing.expect(std.mem.endsWith(u8, text, " Figures below exclude them.\n\n"));
}
test "loadWatchlist: valid symbols load, a malformed record is skipped" {
// `loadWatchlist` had no coverage at all. A record missing `symbol`
// fails coercion and must be skipped rather than costing the rest of
// the watchlist.
const prev_level = std.testing.log_level;
std.testing.log_level = .err;
defer std.testing.log_level = prev_level;
const io = std.testing.io;
const allocator = std.testing.allocator;
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
try tmp.dir.writeFile(io, .{ .sub_path = "watchlist.srf", .data =
\\#!srfv1
\\symbol::AAPL
\\note::no symbol here
\\symbol::MSFT
\\
});
var path_buf: [std.fs.max_path_bytes]u8 = undefined;
const dir_len = try tmp.dir.realPathFile(io, ".", &path_buf);
const path = try std.fs.path.join(allocator, &.{ path_buf[0..dir_len], "watchlist.srf" });
defer allocator.free(path);
const syms = loadWatchlist(io, allocator, path) orelse {
try std.testing.expect(false);
return;
};
defer {
for (syms) |sym| allocator.free(sym);
allocator.free(syms);
}
try std.testing.expectEqual(@as(usize, 2), syms.len);
try std.testing.expectEqualStrings("AAPL", syms[0]);
try std.testing.expectEqualStrings("MSFT", syms[1]);
}
test "loadWatchlist: a missing file is null, not an error" {
const allocator = std.testing.allocator;
try std.testing.expect(loadWatchlist(std.testing.io, allocator, "does_not_exist_watchlist.srf") == null);
}
// As-of date parsing (shared by CLI --as-of and TUI date popup)
pub const AsOfParseError = error{
@ -1008,7 +1125,16 @@ pub fn loadWatchlist(io: std.Io, allocator: std.mem.Allocator, path: []const u8)
var syms: std.ArrayList([]const u8) = .empty;
while (it.next() catch null) |fields| {
const entry = fields.to(WatchEntry, srf_opts.user_edited) catch continue;
const entry = fields.to(WatchEntry, srf_opts.user_edited) catch |err| {
// Skip the entry rather than losing the whole watchlist, but
// name the error - otherwise a symbol just stops appearing.
// Quiet under `zig build test`, where fixtures feed
// malformed records on purpose.
if (!builtin.is_test) {
std.log.warn("watchlist.srf: skipping malformed record: {s}", .{@errorName(err)});
}
continue;
};
const duped = allocator.dupe(u8, entry.symbol) catch continue;
syms.append(allocator, duped) catch {
allocator.free(duped);

View file

@ -177,7 +177,12 @@ pub fn parseImportedValues(
errdefer points.deinit(allocator);
while (it.next() catch return error.InvalidSrf) |fields| {
const point = fields.to(HistoryPoint, srf_opts.user_edited) catch return error.InvalidSrf;
// Aborts rather than skipping: this is a small curated series,
// so one unparseable row means the transcription is suspect and
// a partial series would be worse than none. The error passes
// through instead of collapsing to `InvalidSrf`, so the caller
// can report WHY it failed rather than just that it did.
const point = try fields.to(HistoryPoint, srf_opts.user_edited);
try points.append(allocator, point);
}

View file

@ -10,10 +10,13 @@
/// symbol::02315N600,asset_class::International Developed,pct:num:20
/// symbol::02315N600,asset_class::Bonds,pct:num:15
const std = @import("std");
const builtin = @import("builtin");
const srf = @import("srf");
const srf_opts = @import("../srf_opts.zig");
const Date = @import("../Date.zig");
const log = std.log.scoped(.metadata);
/// A single classification entry for a symbol.
pub const ClassificationEntry = struct {
symbol: []const u8,
@ -90,7 +93,16 @@ pub fn parseClassificationFile(allocator: std.mem.Allocator, data: []const u8) !
defer it.deinit();
while (try it.next()) |fields| {
const entry = fields.to(ClassificationEntry, srf_opts.user_edited) catch continue;
const entry = fields.to(ClassificationEntry, srf_opts.user_edited) catch |err| {
// Skip the row rather than losing the whole file, but name
// the error: a silently dropped row quietly changes a
// breakdown's percentages. Quiet under `zig build test`,
// where fixtures feed malformed rows on purpose.
if (!builtin.is_test) {
log.warn("metadata.srf: skipping malformed record: {s}", .{@errorName(err)});
}
continue;
};
// Pre-fill `bucket` if the user didn't curate one. This
// shifts the cost of `deriveBucket` to parse time and
// makes downstream code free to read `entry.bucket`
@ -288,6 +300,28 @@ test "parse classification file: a hand-typed string separator on pct still pars
try std.testing.expectApproxEqAbs(@as(f64, 40), map.entries[1].pct, 0.001);
}
test "parse classification file: a record missing symbol is skipped, not fatal" {
// `symbol` has no default, so a record without it fails coercion.
// One bad row must not cost the whole file - every other symbol's
// sector and geo would silently vanish from the breakdowns.
const prev_level = std.testing.log_level;
std.testing.log_level = .err;
defer std.testing.log_level = prev_level;
const data =
\\#!srfv1
\\symbol::AAPL,sector::Technology
\\sector::Healthcare,pct:num:100
\\symbol::MSFT,sector::Technology
;
var map = try parseClassificationFile(std.testing.allocator, data);
defer map.deinit();
try std.testing.expectEqual(@as(usize, 2), map.entries.len);
try std.testing.expectEqualStrings("AAPL", map.entries[0].symbol);
try std.testing.expectEqualStrings("MSFT", map.entries[1].symbol);
}
test "parse classification file: bucket round-trips" {
const data =
\\#!srfv1

View file

@ -90,6 +90,16 @@ pub const LoadedPortfolio = struct {
portfolio: zfin.Portfolio,
positions: []const zfin.Position,
syms: []const []const u8,
/// One message per record that could not be parsed and was
/// therefore left out of `portfolio`. Empty on a clean load.
///
/// A dropped lot silently changes every figure derived from this
/// portfolio, so these travel with the load rather than going only
/// to `std.log`: whoever prints the figures can print the caveat
/// beside them. `commands.common.loadPortfolio` does exactly that.
/// Each message is prefixed with the file it came from, since the
/// load is a union over the whole `portfolio*.srf` glob. Owned.
warnings: []const []const u8 = &.{},
pub fn deinit(self: *LoadedPortfolio, allocator: std.mem.Allocator) void {
allocator.free(self.syms);
@ -97,6 +107,8 @@ pub const LoadedPortfolio = struct {
self.portfolio.deinit();
for (self.file_datas) |d| allocator.free(d);
allocator.free(self.file_datas);
for (self.warnings) |w| allocator.free(w);
allocator.free(self.warnings);
// Path-string ownership: `resolved_paths` (if present) owns
// the underlying path strings. The `paths` slice is the
// borrowed view; free only its outer storage.
@ -387,6 +399,11 @@ fn loadFromBytes(
var lots_owner: LotsOwner = .merged_list;
var success = false;
// Per-record parse diagnostics, accumulated across every file in the
// glob. Handed to the caller on success so it can report alongside
// the figures the missing lots have silently changed.
var warnings: zfin.cache.ParseDiagnostics = .empty;
defer if (!success) {
switch (lots_owner) {
.merged_list => {
@ -403,6 +420,8 @@ fn loadFromBytes(
.combined_struct => combined.deinit(),
.none => {},
}
for (warnings.items) |w| allocator.free(w);
warnings.deinit(allocator);
for (file_datas_owned) |d| allocator.free(d);
allocator.free(file_datas_owned);
allocator.free(paths_owned);
@ -422,12 +441,36 @@ fn loadFromBytes(
// without trying to parse.
if (data.len == 0) continue;
var portfolio = zfin.cache.deserializePortfolio(allocator, data) catch {
// Diagnostics are gathered per file so each message can name
// the file it came from - the load is a union over the glob, and
// "line 183" is ambiguous across several portfolio files.
var file_diags: zfin.cache.ParseDiagnostics = .empty;
defer {
for (file_diags.items) |w| allocator.free(w);
file_diags.deinit(allocator);
}
var portfolio = zfin.cache.deserializePortfolioDiag(allocator, data, &file_diags) catch {
var msg_buf: [512]u8 = undefined;
const msg = std.fmt.bufPrint(&msg_buf, "Error: Cannot parse portfolio file: {s}\n", .{paths_owned[idx]}) catch "Error: Cannot parse portfolio file\n";
stderr.print(io, msg);
return null;
};
for (file_diags.items) |raw_msg| {
const named = std.fmt.allocPrint(allocator, "{s}: {s}", .{
std.fs.path.basename(paths_owned[idx]),
raw_msg,
}) catch {
portfolio.deinit();
return null;
};
warnings.append(allocator, named) catch {
allocator.free(named);
portfolio.deinit();
return null;
};
}
for (portfolio.lots) |lot| {
merged.append(allocator, lot) catch {
portfolio.deinit();
@ -457,6 +500,12 @@ fn loadFromBytes(
return null;
};
const warnings_owned = warnings.toOwnedSlice(allocator) catch {
allocator.free(syms);
allocator.free(positions);
return null;
};
success = true;
return .{
.paths = paths_owned,
@ -465,6 +514,7 @@ fn loadFromBytes(
.portfolio = combined,
.positions = positions,
.syms = syms,
.warnings = warnings_owned,
};
}
@ -778,6 +828,67 @@ test "loadFromBytes: union of two synthetic SRF files" {
try testing.expectEqualStrings("MSFT", loaded.portfolio.lots[1].symbol);
}
test "loadFromBytes: a dropped record is reported and names its file" {
// The union spans several files, so "line 3" alone is ambiguous -
// each message carries the basename it came from. Only the second
// file has a bad record here, and only it should be named.
const allocator = testing.allocator;
const file_a =
\\#!srfv1
\\symbol::AAPL,shares:num:10,open_date::2024-01-15,open_price:num:150,account::Sample IRA
\\
;
const file_b =
\\#!srfv1
\\symbol::MSFT,shares:num:5,open_date::2024-02-01,open_price:num:300,account::Sample Roth
\\symbol::NVDA,security_type::stok,shares:num:1,open_date::2024-03-01,open_price:num:900
\\
;
const paths = try allocator.dupe([]const u8, &.{ "portfolio.srf", "portfolio_closed.srf" });
const datas = try dupeBytes(allocator, &.{ file_a, file_b });
var loaded = loadFromBytes(testing.io, allocator, paths, null, datas, zfin.Date.fromYmd(2026, 5, 23)) orelse {
try testing.expect(false);
return;
};
defer loaded.deinit(allocator);
// Two good lots merged; the bad one dropped rather than fatal.
try testing.expectEqual(@as(usize, 2), loaded.portfolio.lots.len);
try testing.expectEqual(@as(usize, 1), loaded.warnings.len);
const w = loaded.warnings[0];
try testing.expect(std.mem.startsWith(u8, w, "portfolio_closed.srf: "));
try testing.expect(std.mem.indexOf(u8, w, "line 3") != null);
try testing.expect(std.mem.indexOf(u8, w, "StringValueNotValidEnumMember") != null);
try testing.expect(std.mem.indexOf(u8, w, "security_type::stok") != null);
}
test "loadFromBytes: a clean load reports nothing" {
// Guards against the warnings slice being non-empty (or unfreed) on
// the happy path - `testing.allocator` catches the leak either way.
const allocator = testing.allocator;
const file_a =
\\#!srfv1
\\symbol::AAPL,shares:num:10,open_date::2024-01-15,open_price:num:150,account::Sample IRA
\\
;
const paths = try allocator.dupe([]const u8, &.{"portfolio.srf"});
const datas = try dupeBytes(allocator, &.{file_a});
var loaded = loadFromBytes(testing.io, allocator, paths, null, datas, zfin.Date.fromYmd(2026, 5, 23)) orelse {
try testing.expect(false);
return;
};
defer loaded.deinit(allocator);
try testing.expectEqual(@as(usize, 0), loaded.warnings.len);
}
test "loadFromBytes: single file with valid contents" {
const allocator = testing.allocator;