clean up error output

src/cache/store.zig

@@ -1,4 +1,5 @@
 const std = @import("std");
+const builtin = @import("builtin");
 const log = std.log.scoped(.cache);
 const srf = @import("srf");
 const srf_opts = @import("../srf_opts.zig");
@@ -2190,7 +2191,10 @@ pub fn deserializePortfolioDiag(
         var lot = fields.to(Lot, srf_opts.user_edited) catch |err| {
             if (diags) |d| {
                 try appendParseDiag(allocator, d, data, line, @errorName(err));
-            } else {
+            } else if (!builtin.is_test) {
+                // Quiet under `zig build test`: fixtures feed malformed
+                // records on purpose to pin the skip, and the warn spam
+                // pollutes every run's output.
                 std.log.warn("portfolio: could not parse record at line {d}: {s}", .{ line, @errorName(err) });
             }
             skipped += 1;
@@ -2214,7 +2218,7 @@ pub fn deserializePortfolioDiag(
                 else => {
                     if (diags) |d| {
                         try appendParseDiag(allocator, d, data, line, "no symbol");
-                    } else {
+                    } else if (!builtin.is_test) {
                         std.log.warn("portfolio: record at line {d} has no symbol, skipping", .{line});
                     }
                     if (lot.note) |n| allocator.free(n);
@@ -2234,7 +2238,7 @@ pub fn deserializePortfolioDiag(
     // 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) {
+    if (skipped > 0 and diags == null and !builtin.is_test) {
         std.log.warn("portfolio: {d} record(s) could not be parsed and were skipped", .{skipped});
     }
 

src/data/imported_values.zig

@@ -34,10 +34,13 @@
 //!   "model said you're already there", or absent.
 
 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(.imported_values);
+
 // ── Types ────────────────────────────────────────────────────
 
 /// Projection of "when can the user retire," as captured at a
@@ -179,10 +182,25 @@ pub fn parseImportedValues(
     while (it.next() catch return error.InvalidSrf) |fields| {
         // 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);
+        // a partial series would be worse than none.
+        //
+        // Collapses to `InvalidSrf` on purpose, and logs the real error
+        // instead of returning it. `history.resolveAsOfDate` enumerates
+        // this function's parse errors to degrade gracefully to
+        // "no data at or before" - and srf's coercion errors are an
+        // open-ended set, so propagating them raw would silently fall
+        // through that switch and surface an unexplained error instead
+        // of the actionable "run `zfin snapshot` or populate
+        // imported_values.srf" message. The log carries the cause; the
+        // error set stays enumerable.
+        const point = fields.to(HistoryPoint, srf_opts.user_edited) catch |err| {
+            // Quiet under `zig build test`, where a fixture feeds a
+            // malformed row on purpose to pin the collapse above.
+            if (!builtin.is_test) {
+                log.warn("imported_values.srf: record {d}: {s}", .{ points.items.len + 1, @errorName(err) });
+            }
+            return error.InvalidSrf;
+        };
         try points.append(allocator, point);
     }
 
@@ -289,6 +307,45 @@ test "parseImportedValues: missing optional fields" {
     try std.testing.expectEqual(@as(?ProjectedRetirement, null), iv.points[0].projected_retirement);
 }
 
+test "parseImportedValues: an unparseable record collapses to InvalidSrf" {
+    // REGRESSION GUARD. `history.resolveAsOfDate` enumerates this
+    // function's parse errors to degrade gracefully to
+    // `NoDataAtOrBefore`, which is what produces the actionable "run
+    // `zfin snapshot` or populate imported_values.srf" message. srf's
+    // coercion errors are an open-ended set, so letting one through raw
+    // fell past that switch and surfaced an unexplained error instead.
+    //
+    // `liquid` is declared numeric; a string separator makes it a
+    // coercion failure that `strings_to_numbers` cannot rescue, since
+    // "abc" is not a number either.
+    const data =
+        \\#!srfv1
+        \\date::2014-07-03,liquid::abc
+        \\
+    ;
+    try std.testing.expectError(
+        error.InvalidSrf,
+        parseImportedValues(std.testing.allocator, data),
+    );
+}
+
+test "parseImportedValues: a missing required field also collapses to InvalidSrf" {
+    // A second, structurally different srf error. `date` and `liquid`
+    // have no defaults, so omitting one fails coercion via a different
+    // path than a bad value does - and it must collapse identically,
+    // because the point is that the caller's error set stays enumerable
+    // no matter which way coercion fails.
+    const data =
+        \\#!srfv1
+        \\liquid:num:1280036.42
+        \\
+    ;
+    try std.testing.expectError(
+        error.InvalidSrf,
+        parseImportedValues(std.testing.allocator, data),
+    );
+}
+
 test "parseImportedValues: empty file (header only)" {
     const data = "#!srfv1\n";
     var iv = try parseImportedValues(std.testing.allocator, data);

src/history.zig

@@ -1433,6 +1433,46 @@ test "resolveAsOfDate: imported-only falls back to imported_values" {
     try testing.expectEqual(@as(f64, 1_510_000), r.liquid);
 }
 
+test "resolveAsOfDate: a malformed imported_values.srf degrades to NoDataAtOrBefore" {
+    // THE REGRESSION THIS GUARDS. The switch below enumerates
+    // `parseImportedValues`' parse errors to degrade gracefully, which is
+    // what lets `resolveAsOfOrExplain` print the actionable "run
+    // `zfin snapshot` or populate imported_values.srf" message.
+    //
+    // srf's coercion errors are an open-ended set, so when
+    // `parseImportedValues` was briefly changed to propagate them raw
+    // they fell straight through this switch and the user got an
+    // unexplained `IntNotNumberType` instead. The collapse to
+    // `InvalidSrf` is load-bearing, not laziness - see the comment in
+    // `parseImportedValues`.
+    const io = std.testing.io;
+    var tmp = std.testing.tmpDir(.{});
+    defer tmp.cleanup();
+
+    // `liquid` is declared numeric; a string separator with a
+    // non-numeric value is a coercion failure `strings_to_numbers`
+    // cannot rescue.
+    try tmp.dir.writeFile(io, .{ .sub_path = "imported_values.srf", .data =
+        \\#!srfv1
+        \\date::2016-01-03,liquid::abc
+        \\
+    });
+
+    const hist_dir = try tmp.dir.realPathFileAlloc(io, ".", testing.allocator);
+    defer testing.allocator.free(hist_dir);
+
+    var arena = std.heap.ArenaAllocator.init(testing.allocator);
+    defer arena.deinit();
+
+    // No snapshots either, so the imported path is the only candidate -
+    // and an unusable file must read as "no data", not as a raw parse
+    // error escaping to the command.
+    try testing.expectError(
+        error.NoDataAtOrBefore,
+        resolveAsOfDate(io, arena.allocator(), hist_dir, Date.fromYmd(2016, 6, 1)),
+    );
+}
+
 test "resolveAsOfDate: snapshot wins over imported when both present" {
     const io = std.testing.io;
     var tmp = std.testing.tmpDir(.{});