diff --git a/AGENTS.md b/AGENTS.md index 06ef503..8c6f20c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -517,6 +517,16 @@ User input -> main.zig (CLI dispatch) or tui.zig (TUI event loop) - **Internal imports use file paths, not module names.** Only external dependencies (`srf`, `vaxis`, `z2d`) use `@import("name")`. Internal code uses relative paths like `@import("Date.zig")` or `@import("models/portfolio.zig")`. This is intentional - it lets `refAllDecls` in the test binary discover all tests across the entire source tree. +- **User-authored SRF schemas live with their models.** Each + hand-edited file's model module declares `pub const srf_schema` + (record type, reference-doc path, optional per-discriminator + `field_rules`, optional `semanticCheck`); `src/srf_lint.zig` is the + registry and walker, holding no rules of its own. Valid-name sets are + derived from `std.meta.fields`, `field_rules` is comptime-exhaustive + over the struct, and a doc-sync test ties each model to its reference + page - so none of the three can drift from the fields. See "Adding a + field to a user-authored SRF model". + - **DataService is the sole data source.** Both CLI and TUI go through `DataService` for all fetched data. Never call provider APIs directly from commands or TUI tabs. - **Providers are lazily initialized.** `DataService` fields like `td`, `pg`, `fh` start as `null` and are created on first use via `getProvider()`. The provider field name is derived from the type name at comptime. @@ -544,7 +554,7 @@ User input -> main.zig (CLI dispatch) or tui.zig (TUI event loop) | `src/tui/` | Nine-tab interactive TUI. Each tab is a separate file conforming to the framework contract documented in `tab_framework.zig`: `portfolio_tab.zig`, `analysis_tab.zig`, `review_tab.zig`, `projections_tab.zig`, `history_tab.zig`, `quote_tab.zig`, `performance_tab.zig`, `earnings_tab.zig`, `options_tab.zig`. Plus `keybinds.zig` (configurable input + scoped bindings), `theme.zig` (configurable colors), `chart.zig` (Kitty graphics chart renderer), `projection_chart.zig` (percentile-band overlay), `input_buffer.zig` (modal text-input state machine). The `App` orchestrator lives in the parent `src/tui.zig`. | | `src/cache/` | `store.zig`: SRF cache read/write with TTL freshness checks. | | `src/net/` | `http.zig`: HTTP client with retry and error classification. `RateLimiter.zig`: token-bucket rate limiter. | -| `build/` | Build-time support: `Coverage.zig` (kcov integration), `download_kcov.zig` (kcov binary fetcher), `gen_shiller.zig` (CSV -> comptime data converter), `bcov.css` (kcov report styling). | +| `build/` | Build-time support: `Coverage.zig` (kcov integration), `download_kcov.zig` (kcov binary fetcher), `gen_shiller.zig` (CSV -> comptime data converter), `gen_config_docs.zig` (extracts backticked identifiers from `docs/reference/config/*.md` for the schema doc-sync test, since `@embedFile` cannot escape `src/`), `bcov.css` (kcov report styling). | ## Code patterns and conventions @@ -753,6 +763,61 @@ snapshot`, and that is the correct single-file load. The distinction: snapshot files are immutable historical records, not the user's currently-edited portfolio. +### Adding a field to a user-authored SRF model + +Every hand-edited SRF file has a schema that its own model module +declares as `pub const srf_schema`, registered in `src/srf_lint.zig`'s +`schemas`. The build enforces the contract, so you will be told what +to do rather than having to remember: + +1. **Add the field to the model struct.** Nothing else, then + `zig build test`. +2. If the model declares `field_rules` (only `Lot` does today), the + build **fails** with the field name and three copy-pasteable + options: read for every discriminator value, read only for some + (`.only = &.{...}`), or `.derived = true`. Classify it. +3. The doc-sync test then **fails** until the field appears in the + model's reference page (`doc_path`), or is added to `undocumented` + with a comment saying why it is not user-facing. + +Both failures are the point. Rules and docs that live away from the +fields they describe drift from them - `portfolio-srf.md` documented +14 of `Lot`'s 22 fields before this existed, and adding a field had no +consequence. + +Why this matters: SRF's `fields.to(T)` **silently discards** any record +key that names no field of `T` (`field_match` in srf.zig is write-only +- see the module doc on `src/srf_lint.zig`). 19 of `Lot`'s 22 fields +have defaults, so a typo'd key is indistinguishable from an omitted +one. `zfin doctor` reports these per-file with a reference-page +pointer; `zfin audit`'s Section 8 is the routine-use summary, silent +when clean. + +The suggestion matchers are deliberately conservative - case-insensitive +equality and either-direction prefix, no edit distance - so they cannot +produce a wrong guess. They therefore **miss mid-word substitutions** +(`update_cadance` -> `update_cadence` gets no suggestion). The backstop +is that `audit` prints the file's full valid-name set, derived from +`std.meta.fields`, whenever a finding is about a field name +(`Result.hasNameFindings`), and `doctor` points at the reference page, +which the doc-sync test guarantees is complete. Keep both if you touch +the renderers; they are what make the conservatism affordable. + +`Rule.only` is a claim that the value is **silently ignored** for every +other discriminator value, so each one must cite the gate that proves +it. When a field's reach is ambiguous, leave it permissive - a missed +nudge costs nothing, a false "zfin ignores this" sends someone chasing +a bug that does not exist. + +Rules that aren't about a single field's name go in the schema's +`semanticCheck(rec: Record, ctx: srf_lint.Context, sink: *srf_lint.Sink) !void` +(the signature is comptime-checked). `ctx.today` is the real current +day - the date rules (`Lot`'s future `close_date` / `open_date` / +`price_date`) are nonsensical against a back-dated reference. The same +permissive rule applies: `Lot`'s "close_date without close_price" fires +only for stock lots because realized P&L is only computed for them, and +each such scope limit cites its gate in a comment. + ### Adding a new provider 1. Create `src/providers/newprovider.zig` following the existing struct pattern diff --git a/build.zig b/build.zig index 2de1310..5f6fbb1 100644 --- a/build.zig +++ b/build.zig @@ -73,6 +73,7 @@ pub fn build(b: *std.Build) void { .{ .name = "websocket", .module = websocket_dep.module("websocket") }, .{ .name = "build_info", .module = build_info }, .{ .name = "shiller_year", .module = shiller_mod }, + .{ .name = "config_docs", .module = configDocsModule(b) }, }; // Generate Shiller annual returns data from ie_data.csv. @@ -164,6 +165,57 @@ pub fn build(b: *std.Build) void { } } +/// Produce the `config_docs` module: the set of backticked identifiers +/// in each `docs/reference/config/*.md` reference page, extracted by +/// `build/gen_config_docs.zig`. +/// +/// Consumed by `src/srf_lint.zig`'s doc-sync test, which asserts every +/// field of every user-authored model appears in that model's reference +/// page. `@embedFile` cannot reach outside `src/`, hence the generator. +/// +/// The directory is ENUMERATED rather than listed, so adding a new +/// reference page needs no edit here. Each page is passed as a file arg +/// so the build re-runs when one changes. +fn configDocsModule(b: *std.Build) *std.Build.Module { + const rel = "docs/reference/config"; + + const gen = b.addExecutable(.{ + .name = "gen_config_docs", + .root_module = b.createModule(.{ + .root_source_file = b.path("build/gen_config_docs.zig"), + .target = b.graph.host, + }), + }); + const run = b.addRunArtifact(gen); + const output = run.addOutputFileArg("config_docs.zig"); + + var names: std.ArrayList([]const u8) = .empty; + if (b.build_root.handle.openDir(b.graph.io, rel, .{ .iterate = true })) |dir| { + var d = dir; + defer d.close(b.graph.io); + var it = d.iterate(); + while (it.next(b.graph.io) catch null) |entry| { + if (entry.kind != .file) continue; + if (!std.mem.endsWith(u8, entry.name, ".md")) continue; + names.append(b.allocator, b.dupe(entry.name)) catch @panic("OOM"); + } + } else |_| {} + + // Sorted so the generated file - and therefore the build cache + // hash - does not depend on directory-iteration order. + std.mem.sort([]const u8, names.items, {}, struct { + fn lessThan(_: void, a: []const u8, c: []const u8) bool { + return std.mem.lessThan(u8, a, c); + } + }.lessThan); + + for (names.items) |n| { + run.addFileArg(b.path(b.fmt("{s}/{s}", .{ rel, n }))); + } + + return b.createModule(.{ .root_source_file = output }); +} + /// Produce the `build_info` module exposing `version` (derived from `git /// describe`) and `build_timestamp` (committer timestamp of HEAD). /// Consumed as `@import("build_info")` from `src/version.zig`. diff --git a/build/gen_config_docs.zig b/build/gen_config_docs.zig new file mode 100644 index 0000000..a29d0de --- /dev/null +++ b/build/gen_config_docs.zig @@ -0,0 +1,139 @@ +/// Build-time generator: extracts the set of backticked identifiers +/// from each `docs/reference/config/*.md` reference page and emits them +/// as a Zig source file. +/// +/// Exists because `@embedFile` cannot escape the package path, so +/// `src/` code has no way to read `docs/`. The alternative - reading +/// the markdown at test time via cwd - would make the doc-sync check +/// dependent on where the test binary was launched from, and a check +/// that can silently skip is a check that rots. +/// +/// Only the NAME SET is extracted, never the prose. That keeps the +/// output free of string-escaping concerns and small (a few hundred +/// identifiers), and it is exactly what the consumer needs: +/// `srf_lint`'s doc-sync test asserts every field of each model appears +/// in that model's reference page. +/// +/// An "identifier" here is a backtick-delimited token matching +/// `[a-z][a-z0-9_]*` - the shape of every SRF field name. Prose words +/// in backticks (`true`, `null`, `stock`) come along harmlessly; the +/// test only ever asks whether a known field name is PRESENT, never +/// whether an extracted name is a real field. +const std = @import("std"); + +pub fn main(init: std.process.Init) !void { + const allocator = init.arena.allocator(); + const io = init.io; + + const args = try init.minimal.args.toSlice(allocator); + if (args.len < 3) { + var stderr_buf: [160]u8 = undefined; + var stderr = std.Io.File.stderr().writer(io, &stderr_buf); + try stderr.interface.writeAll("Usage: gen_config_docs ...\n"); + try stderr.interface.flush(); + std.process.exit(1); + } + + const out_file = try std.Io.Dir.cwd().createFile(io, args[1], .{}); + defer out_file.close(io); + var out_buf: [4096]u8 = undefined; + var file_writer = out_file.writer(io, &out_buf); + const w = &file_writer.interface; + + try w.writeAll( + \\// Auto-generated from docs/reference/config/*.md - do not edit. + \\// Regenerate: zig build (runs build/gen_config_docs.zig) + \\ + \\/// One reference page's backticked identifiers. + \\pub const Doc = struct { + \\ /// Basename, e.g. "portfolio-srf.md". + \\ file: []const u8, + \\ names: []const []const u8, + \\}; + \\ + \\pub const docs = [_]Doc{ + \\ + ); + + for (args[2..]) |path| { + const body = try std.Io.Dir.cwd().readFileAlloc(io, path, allocator, .limited(4 * 1024 * 1024)); + const base = std.fs.path.basename(path); + + var names: std.ArrayList([]const u8) = .empty; + try collectIdents(allocator, body, &names); + + try w.print(" .{{ .file = \"{s}\", .names = &.{{", .{base}); + for (names.items, 0..) |n, i| { + if (i % 6 == 0) try w.writeAll("\n "); + try w.print("\"{s}\", ", .{n}); + } + try w.writeAll("\n } },\n"); + } + + try w.writeAll( + \\}; + \\ + \\/// The `Doc` for `file`, or null when that page was not built in. + \\pub fn find(file: []const u8) ?Doc { + \\ for (docs) |d| { + \\ if (std.mem.eql(u8, d.file, file)) return d; + \\ } + \\ return null; + \\} + \\ + \\const std = @import("std"); + \\ + ); + try w.flush(); +} + +/// Append every deduplicated backticked `[a-z][a-z0-9_]*` token in +/// `body` to `out`. Slices borrow from `body`. +fn collectIdents( + allocator: std.mem.Allocator, + body: []const u8, + out: *std.ArrayList([]const u8), +) !void { + var i: usize = 0; + while (i < body.len) { + if (body[i] != '`') { + i += 1; + continue; + } + // Skip fenced code blocks wholesale - a ```zig sample can + // mention a field in a context the reference table does not, + // and counting it would let a field be "documented" by an + // example alone. + if (std.mem.startsWith(u8, body[i..], "```")) { + const rest = body[i + 3 ..]; + const close = std.mem.indexOf(u8, rest, "```") orelse break; + i += 3 + close + 3; + continue; + } + const rest = body[i + 1 ..]; + const close = std.mem.indexOfScalar(u8, rest, '`') orelse break; + const raw = rest[0..close]; + i += 1 + close + 1; + // The docs name a field both bare (`symbol`, in reference + // tables) and on-wire (`symbol::`, `shares:num:100`, in prose + // and examples). Cut at the first separator so both spellings + // count as documenting the field. + const tok = raw[0 .. std.mem.indexOfScalar(u8, raw, ':') orelse raw.len]; + if (!isIdent(tok)) continue; + for (out.items) |seen| { + if (std.mem.eql(u8, seen, tok)) break; + } else { + try out.append(allocator, tok); + } + } +} + +fn isIdent(tok: []const u8) bool { + if (tok.len == 0) return false; + if (tok[0] < 'a' or tok[0] > 'z') return false; + for (tok) |c| { + const ok = (c >= 'a' and c <= 'z') or (c >= '0' and c <= '9') or c == '_'; + if (!ok) return false; + } + return true; +} diff --git a/docs/guides/audit-against-brokerage.md b/docs/guides/audit-against-brokerage.md index 9f8502f..ad2dd96 100644 --- a/docs/guides/audit-against-brokerage.md +++ b/docs/guides/audit-against-brokerage.md @@ -102,6 +102,8 @@ With no flags, `zfin audit` first prints a health report: - **Accounts overdue for update** -- accounts past their `update_cadence` (see [accounts.srf](set-up-accounts.md#3-tune-the-maintenance-cadence)). - **Brokerage files** it discovered, which it then reconciles. +- **Config file problems** -- field-name typos in your `.srf` files, impossible + lot dates, and `accounts.srf` values zfin had to ignore. ``` Portfolio hygiene @@ -117,6 +119,108 @@ With no flags, `zfin audit` first prints a health report: "No update history found" is a nudge, not an error -- silence accounts you don't actively track with `update_cadence::none`. +### Config file problems + +Your `.srf` files can be wrong in ways that don't stop them loading. +zfin reads what it understands and quietly ignores or works around the +rest, so a mistake can change your numbers with no error at all. +`zfin audit` checks `portfolio*.srf`, `accounts.srf`, `metadata.srf`, +`transaction_log.srf`, `projections.srf`, and `watchlist.srf` for three +kinds of problem, and prints nothing when there aren't any: + +``` + Config file problems + portfolio.srf + line 3: unrecognized field 'price_dat' - did you mean 'price_date'? + line 4: BND close_date 2062-03-14 is in the future - the lot is still counted as held until then + line 5: AAPL has close_date but no close_price - the sale's realized gain is recorded as 0 + valid fields: + symbol shares open_date open_price close_date close_price note + label account security_type maturity_date rate drip ticker price + price_date price_ratio split_factor underlying strike multiplier + option_type + accounts.srf + line 2: account 'Sample Brokerage': audit_large_lot_threshold must be > 0 (got 0); ignored, so the audit uses its default + line 4: account 'Sample Roth': tax_mix_* must not name the account's own tax_type; the tax mix is ignored and the account counts wholly as roth +``` + +Each finding names the line of the record it came from, and the date +and account findings also say what zfin is doing instead of what you +wrote. + +#### 1. Field names (every file) + +SRF matches record keys against field names exactly. A key that matches +nothing is **discarded without complaint** -- a misspelled `price_dat::` +is indistinguishable from having left the price date out. Most fields +have defaults, so most typos change behavior with no output at all. + +| Report | Meaning | +|-----------------------------|----------------------------------------------------------------------------------------| +| `did you mean '...'?` | A dropped or doubled character. The suggested name is a real field. | +| `differs only by case` | `Account` is not `account`. Matching is case-sensitive, so this silently does nothing. | +| `ignored for security_type` | A real field that this kind of lot never reads -- e.g. `rate` outside a `cd`. | +| `appears twice` | SRF keeps the **first** occurrence. Editing the second one has no effect. | +| `derived by zfin` | zfin computes this field; a hand-written value is overwritten. Delete it. | +| `unrecognized field` | No match and no close relative. Check it against the `valid fields` list. | + +The suggestions are deliberately cautious -- they only fire when a real +field name is a prefix of what you typed, or differs from it only by +case -- so a suggestion is never a guess. That means a typo in the +*middle* of a name (`update_cadance`) gets no suggestion, which is why +the **`valid fields` list** is printed whenever a field name is in +question. It comes straight from the code, so unlike any document it +cannot be out of date. + +#### 2. Lot dates (`portfolio*.srf`) + +These fields parse fine but describe a lot that can't exist. They matter +more than they look: most of zfin decides whether a lot is held by +comparing `close_date` to today, but `zfin contributions` treats a lot +as sold the moment it has any `close_date` at all -- so a bad date makes +the two disagree. + +| Report | What zfin does with it | +|--------------------------------------|--------------------------------------------------------------------------------------------| +| `close_date ... is in the future` | Counts the lot as held until that date. Usually a mistyped year. | +| `close_date ... is before open_date` | The lot is never counted as held on any date. | +| `has close_date but no close_price` | Stock lots only. The sale's realized gain is recorded as 0. | +| `has close_price but no close_date` | The lot stays held. On a stock lot, its row shows the close price instead of the live one. | +| `open_date ... is in the future` | The lot is left out of your positions until that date. | +| `price_date ... is in the future` | Stock lots only. The manual price counts as fresh, so it is never flagged as stale. | + +A close dated today is already closed, and a same-day buy-and-sell +(`close_date` equal to `open_date`) is fine. A future `maturity_date` is +normal -- that is the field for a CD or option that ends on a known +date. Watch lots are exempt, since zfin reads only their `symbol`. See +[Closed lots](../reference/config/portfolio-srf.md#closed-lots). + +#### 3. Account values (`accounts.srf`) + +Values zfin rejects and replaces with a default, so the account loads -- +just not the way you wrote it. + +| Report | What zfin does instead | +|-----------------------------------------------|-------------------------------------------------------------------------| +| `audit_large_lot_threshold must be > 0` | Uses the audit's built-in threshold. | +| `harvested must be a finite number` | Drops the harvested figure (only reachable by typing `inf` or `nan`). | +| `tax_mix_* ...` (any of the four rules below) | Ignores the whole tax mix; the account counts wholly as its `tax_type`. | + +The four tax-mix rules: every `tax_mix_*` carve-out must be above 0 and +a finite number, none may name the account's own `tax_type`, and +together they must sum to less than 100. See +[`accounts.srf`](../reference/config/accounts-srf.md). + +These three are also logged to stderr whenever `accounts.srf` loads; +the audit is where they get a line number and can't scroll past. + +#### Also in `zfin doctor` + +`zfin doctor` reports the same findings one file at a time, with a link +to each file's reference page, and also covers `keys.srf`, `theme.srf`, +and `acknowledgments.srf`. Neither command changes its exit code over +any of these -- they're nudges, not failures. + ## Auto-discovery (and your download folder) With no `--fidelity`/`--schwab` flag, zfin looks for exports in two diff --git a/docs/reference/config/keys-srf.md b/docs/reference/config/keys-srf.md index 580d763..d537448 100644 --- a/docs/reference/config/keys-srf.md +++ b/docs/reference/config/keys-srf.md @@ -18,6 +18,12 @@ One binding per line: action::ACTION_NAME,key::KEY_STRING[,scope::SCOPE] ``` +| Field | Type | Required | Description | +|----------|--------|----------|---------------------------------------------------------------------------------------------------------| +| `action` | string | Yes | The action to bind. Global actions are listed below; a tab-local `scope` changes which names are valid. | +| `key` | string | Yes | The key combination, e.g. `q`, `ctrl+c`, `F5`, `page_down`. | +| `scope` | string | No | Omitted or `global` for a global binding; a tab name (e.g. `options`) for a tab-local one. | + - **Modifiers:** `ctrl+`, `alt+`, `shift+` (e.g. `ctrl+c`). - **Special keys:** `tab`, `enter`, `escape`, `space`, `backspace`, `left`, `right`, `up`, `down`, `page_up`, `page_down`, `home`, diff --git a/docs/reference/config/portfolio-srf.md b/docs/reference/config/portfolio-srf.md index 347b4ab..63832ea 100644 --- a/docs/reference/config/portfolio-srf.md +++ b/docs/reference/config/portfolio-srf.md @@ -42,6 +42,7 @@ symbol::VTI,shares:num:100,open_date::2024-01-15,open_price:num:220.50,account:: | `security_type` | string | No | `stock` (default), `option`, `cd`, `cash`, `illiquid`, `watch`. | | `account` | string | No | Account name. Should match an `account::` entry in [`accounts.srf`](accounts-srf.md). | | `note` | string | No | Free-text note (shown in cash/CD/illiquid tables). | +| `label` | string | No | Display name for the symbol column, overriding `symbol`/`ticker` | | `ticker` | string | No | Ticker alias used for price fetching when `symbol` is a CUSIP. | | `price` | number | No | Manual price override (for securities the providers don't cover). | | `price_date` | string | No | Date of the manual price (`YYYY-MM-DD`), for staleness display. | @@ -110,6 +111,22 @@ two things beyond bookkeeping: `close_price` the current market price stands in, which is close enough for a recent sale and wrong for an old one. +A `close_date` must not be in the future. A close dated today is already +closed, but a later date leaves the lot counted as held until then -- +while `zfin contributions` treats it as sold the moment the field +appears. For a scheduled end (a CD or option), use `maturity_date`. + +`zfin audit` and `zfin doctor` flag these lot-lifecycle mistakes: + +- `close_date` in the future, or before `open_date`. +- `close_date` without `close_price` on a stock lot (the realized gain + is recorded as 0), or `close_price` without `close_date` on any lot. +- `open_date` in the future (the lot is not counted until then). +- `price_date` in the future on a stock lot (the manual price is never + flagged as stale). + +Watch lots are exempt; zfin reads only their `symbol`. + You can either edit the lot where it sits or move it to a sibling file such as `portfolio_closed.srf` -- the `portfolio*.srf` glob picks it up either way, so realized gains and back-dated (`--as-of`) views stay diff --git a/docs/reference/config/theme-srf.md b/docs/reference/config/theme-srf.md index 9f0ba47..0b281b4 100644 --- a/docs/reference/config/theme-srf.md +++ b/docs/reference/config/theme-srf.md @@ -46,6 +46,7 @@ negative::#e06c75 | `info` | Informational highlights | | `select_bg` / `select_fg` | Selected row | | `border` | Borders and rules | +| `bar_fill` | Bar-chart fill | Run `zfin interactive --default-theme` for the full key list with the default values filled in. diff --git a/docs/reference/config/transaction-log-srf.md b/docs/reference/config/transaction-log-srf.md index 52318d8..b5a0e0a 100644 --- a/docs/reference/config/transaction-log-srf.md +++ b/docs/reference/config/transaction-log-srf.md @@ -47,6 +47,7 @@ transfer::2026-05-02,type::cash,amount:num:4700,from::Sample IRA,to::Sample Brok | `from` | string | Yes | Source account name (matches an `account::` in your portfolio). | | `to` | string | Yes | Destination account name. | | `dest_lot` | string | Yes | Where it landed: `cash`, or `SYMBOL@YYYY-MM-DD` for a specific lot. | +| `note` | string | No | Free-text reminder of why the transfer happened. | ## `type::cash` vs `type::in_kind` diff --git a/src/analytics/analysis.zig b/src/analytics/analysis.zig index 9c6cb64..c566a34 100644 --- a/src/analytics/analysis.zig +++ b/src/analytics/analysis.zig @@ -12,6 +12,7 @@ const ClassificationEntry = @import("../models/classification.zig").Classificati const Portfolio = @import("../models/portfolio.zig").Portfolio; const Date = @import("../Date.zig"); const fmt = @import("../format.zig"); +const srf_lint = @import("../srf_lint.zig"); const log = std.log.scoped(.accounts); @@ -368,6 +369,94 @@ pub const AccountTaxEntry = struct { } }; +/// Every semantic problem `accounts.srf` validation can find in one +/// entry. All three fall back to a safe default rather than failing the +/// parse, which is exactly why they need reporting: the account still +/// loads, just not the way the user wrote it. +/// +/// Single source of truth for these rules. `parseAccountsFile` logs +/// them to stderr as it parses; `srf_schema.semanticCheck` surfaces the +/// same set through `zfin doctor` / `zfin audit`, where they are +/// attached to a line number and cannot scroll past. Both call +/// `checkEntry` - the conditions are written once. +pub const EntryProblems = struct { + /// `audit_large_lot_threshold` was zero or negative. Zero would + /// flag every new lot; negative is meaningless. + bad_lot_threshold: ?f64 = null, + /// `harvested` was `inf`/`nan` - only reachable by hand-typing it. + non_finite_harvested: ?f64 = null, + /// A declared `tax_mix_*` set that breaks the carve-out rules, so + /// the account reverts to its bare `tax_type`. + tax_mix: ?TaxMixProblem = null, + + pub fn any(self: EntryProblems) bool { + return self.bad_lot_threshold != null or + self.non_finite_harvested != null or + self.tax_mix != null; + } +}; + +/// Validate one parsed `accounts.srf` entry. Pure - no logging, no +/// allocation - so both the parser and the linter can call it. +pub fn checkEntry(entry: AccountTaxEntry) EntryProblems { + var p: EntryProblems = .{}; + if (entry.audit_large_lot_threshold) |t| { + if (t <= 0) p.bad_lot_threshold = t; + } + if (entry.harvested) |h| { + if (!std.math.isFinite(h)) p.non_finite_harvested = h; + } + p.tax_mix = entry.taxMixChecked().problem; + return p; +} + +/// Schema contract for `accounts.srf`. See `srf_lint.validateSchema`. +/// +/// No `field_rules`: nothing here is conditional on another field's +/// value. The `tax_mix_*` family IS interdependent, but that is a +/// field-COMBINATION rule rather than a per-field applicability one, so +/// it goes through `semanticCheck` instead. +pub const srf_schema = struct { + pub const Record = AccountTaxEntry; + pub const file_label: []const u8 = "accounts.srf"; + pub const doc_path: []const u8 = "reference/config/accounts-srf.md"; + + /// Surface `checkEntry`'s findings through the lint channel. + /// + /// These were already detected at parse time and logged with + /// `std.log.warn`, which still happens - but a stderr line emitted + /// mid-render, with no file or line number, is how they came to be + /// ignored. Reporting them here attaches each to the record that + /// caused it and puts it in a report the user is reading on purpose. + pub fn semanticCheck(rec: Record, ctx: srf_lint.Context, sink: *srf_lint.Sink) !void { + // No date rules here: `doctor`'s Section B already reports a + // future `harvested_date` / `tax_mix_date`, grouped by account. + _ = ctx; + const p = checkEntry(rec); + if (p.bad_lot_threshold) |t| { + try sink.addOwned(.semantic, "audit_large_lot_threshold", try std.fmt.allocPrint( + sink.allocator, + "account '{s}': audit_large_lot_threshold must be > 0 (got {d}); ignored, so the audit uses its default", + .{ rec.account, t }, + )); + } + if (p.non_finite_harvested) |h| { + try sink.addOwned(.semantic, "harvested", try std.fmt.allocPrint( + sink.allocator, + "account '{s}': harvested must be a finite number (got {d}); ignored", + .{ rec.account, h }, + )); + } + if (p.tax_mix) |problem| { + try sink.addOwned(.semantic, "tax_mix", try std.fmt.allocPrint( + sink.allocator, + "account '{s}': {s}; the tax mix is ignored and the account counts wholly as {s}", + .{ rec.account, problem.label(), @tagName(rec.tax_type) }, + )); + } + } +}; + /// Update cadence for manual account maintenance. Parsed from accounts.srf. /// Default is `weekly` (fail-open: every account nags until explicitly silenced). pub const UpdateCadence = enum { @@ -559,39 +648,43 @@ pub fn parseAccountsFile(allocator: std.mem.Allocator, data: []const u8) !Accoun continue; }; + // Validate once, report twice: these same problems are surfaced + // with line numbers by `srf_schema.semanticCheck` (see + // `src/srf_lint.zig`). The conditions live in `checkEntry` so + // the two channels cannot disagree about what is wrong. + const problems = checkEntry(entry); + // A zero/negative large-lot threshold is nonsensical (zero // flags every new lot; negative is meaningless). Reject it and // treat the account as unset so the audit uses its default. - const lot_threshold: ?f64 = if (entry.audit_large_lot_threshold) |t| blk: { - if (t > 0) break :blk t; + const lot_threshold: ?f64 = if (problems.bad_lot_threshold) |t| blk: { // No-op under `zig build test`: the parser's own tests feed // invalid thresholds (0, negative) on purpose to verify they // are rejected, and the warn spam pollutes test output. if (!builtin.is_test) log.warn("accounts.srf: account '{s}': audit_large_lot_threshold must be > 0 (got {d}); ignoring", .{ entry.account, t }); break :blk null; - } else null; + } else entry.audit_large_lot_threshold; // `harvested` is a magnitude: the annotation's parens carry the // "this is a loss" convention, so accept either sign from the // user and normalize. A non-finite value can only arrive from a // hand-typed `inf`/`nan`; there is nothing sensible to display // for it, so drop the field rather than render garbage. - const harvested: ?f64 = if (entry.harvested) |h| blk: { - if (std.math.isFinite(h)) break :blk @abs(h); + const harvested: ?f64 = if (problems.non_finite_harvested) |h| blk: { // Silent under `zig build test`: the parser's own tests feed // non-finite values on purpose, and the warn spam pollutes // test output. if (!builtin.is_test) log.warn("accounts.srf: account '{s}': harvested must be a finite number (got {d}); ignoring", .{ entry.account, h }); break :blk null; - } else null; + } else if (entry.harvested) |h| @abs(h) else null; // A declared tax mix that breaks the rules falls back to the // bare `tax_type`. Warn so a typo isn't invisible; the raw // fields are deliberately left in place so `zfin doctor` can // report the same problem against the file the user edits. - if (entry.taxMixChecked().problem) |p| { + if (problems.tax_mix) |p| { // Silent under `zig build test`: the parser's own tests feed // invalid mixes on purpose to verify the fallback, and the // warn spam pollutes test output. @@ -3318,3 +3411,91 @@ test "abbreviateSector: known long labels collapse, others pass through" { try std.testing.expectEqualStrings("Equity / Corporate", abbreviateSector("Equity / Corporate")); try std.testing.expectEqualStrings("", abbreviateSector("")); } + +// ── checkEntry / srf_schema.semanticCheck ── + +/// Lint `data` as accounts.srf and return only the semantic findings +/// (the raw field-name pass is `srf_lint`'s own territory). +fn lintAccounts(data: []const u8) !srf_lint.Result { + var r = try srf_lint.check(std.testing.allocator, data, srf_lint.shapeOfSchema(srf_schema)); + errdefer r.deinit(); + try srf_lint.checkSemantic(srf_schema, &r, data, .{ .today = Date.fromYmd(2026, 1, 1) }); + return r; +} + +test "checkEntry: a clean entry has no problems" { + const e = AccountTaxEntry{ .account = "Sample IRA", .tax_type = .traditional }; + try std.testing.expect(!checkEntry(e).any()); +} + +test "checkEntry: zero and negative large-lot thresholds are rejected, positive kept" { + const zero = AccountTaxEntry{ .account = "Sample IRA", .tax_type = .roth, .audit_large_lot_threshold = 0 }; + try std.testing.expectEqual(@as(?f64, 0), checkEntry(zero).bad_lot_threshold); + + const neg = AccountTaxEntry{ .account = "Sample IRA", .tax_type = .roth, .audit_large_lot_threshold = -5 }; + try std.testing.expectEqual(@as(?f64, -5), checkEntry(neg).bad_lot_threshold); + + const ok = AccountTaxEntry{ .account = "Sample IRA", .tax_type = .roth, .audit_large_lot_threshold = 25_000 }; + try std.testing.expectEqual(@as(?f64, null), checkEntry(ok).bad_lot_threshold); +} + +test "checkEntry: non-finite harvested is rejected, a negative magnitude is not" { + const inf = AccountTaxEntry{ .account = "Sample Brokerage", .tax_type = .taxable, .harvested = std.math.inf(f64) }; + try std.testing.expect(checkEntry(inf).non_finite_harvested != null); + + const nan = AccountTaxEntry{ .account = "Sample Brokerage", .tax_type = .taxable, .harvested = std.math.nan(f64) }; + try std.testing.expect(checkEntry(nan).non_finite_harvested != null); + + // A negative figure is legal - the parser normalizes with @abs. + const neg = AccountTaxEntry{ .account = "Sample Brokerage", .tax_type = .taxable, .harvested = -1234.0 }; + try std.testing.expectEqual(@as(?f64, null), checkEntry(neg).non_finite_harvested); +} + +test "checkEntry: a tax mix naming the account's own tax_type is a problem" { + const e = AccountTaxEntry{ .account = "Sample IRA", .tax_type = .traditional, .tax_mix_traditional = 30 }; + try std.testing.expectEqual(TaxMixProblem.redundant_primary, checkEntry(e).tax_mix.?); +} + +test "srf_schema.semanticCheck: reports the bad large-lot threshold with the account name" { + var r = try lintAccounts("#!srfv1\naccount::Sample Brokerage,tax_type::taxable,audit_large_lot_threshold:num:0\n"); + defer r.deinit(); + try std.testing.expectEqual(@as(usize, 1), r.findings.len); + try std.testing.expectEqual(srf_lint.Kind.semantic, r.findings[0].kind); + + var buf: [srf_lint.Finding.describe_max]u8 = undefined; + const msg = r.findings[0].describe(&buf); + try std.testing.expect(std.mem.indexOf(u8, msg, "Sample Brokerage") != null); + try std.testing.expect(std.mem.indexOf(u8, msg, "must be > 0") != null); +} + +test "srf_schema.semanticCheck: reports non-finite harvested" { + var r = try lintAccounts("#!srfv1\naccount::Sample Trust,tax_type::taxable,harvested::inf\n"); + defer r.deinit(); + var buf: [srf_lint.Finding.describe_max]u8 = undefined; + try std.testing.expectEqual(@as(usize, 1), r.findings.len); + const msg = r.findings[0].describe(&buf); + try std.testing.expect(std.mem.indexOf(u8, msg, "Sample Trust") != null); + try std.testing.expect(std.mem.indexOf(u8, msg, "finite") != null); +} + +test "srf_schema.semanticCheck: reports a broken tax mix and names the fallback" { + var r = try lintAccounts("#!srfv1\naccount::Sample IRA,tax_type::traditional,tax_mix_traditional:num:30\n"); + defer r.deinit(); + var buf: [srf_lint.Finding.describe_max]u8 = undefined; + try std.testing.expectEqual(@as(usize, 1), r.findings.len); + const msg = r.findings[0].describe(&buf); + try std.testing.expect(std.mem.indexOf(u8, msg, "Sample IRA") != null); + // The fallback behavior is the actionable part, not just "invalid". + try std.testing.expect(std.mem.indexOf(u8, msg, "counts wholly as traditional") != null); +} + +test "srf_schema.semanticCheck: a clean accounts.srf reports nothing" { + var r = try lintAccounts( + \\#!srfv1 + \\account::Sample Brokerage,tax_type::taxable,harvested:num:1200,harvested_date::2025-01-01 + \\account::Sample IRA,tax_type::traditional,tax_mix_roth:num:20 + \\ + ); + defer r.deinit(); + try std.testing.expect(r.isClean()); +} diff --git a/src/analytics/projections.zig b/src/analytics/projections.zig index 9619b59..4de2a44 100644 --- a/src/analytics/projections.zig +++ b/src/analytics/projections.zig @@ -722,6 +722,20 @@ const SrfProjection = union(enum) { event: SrfEvent, }; +/// Schema contract for `projections.srf`. See +/// `srf_lint.validateSchema`. +/// +/// The highest-value file in the registry to lint: all 17 `SrfConfig` +/// fields have defaults, so SRF silently discards a typo'd one, and +/// `parseProjectionsConfig` has an infallible signature that degrades +/// every failure to defaults. A misspelled `target_spending` therefore +/// changes projected retirement math with no output whatsoever. +pub const srf_schema = struct { + pub const Record = SrfProjection; + pub const file_label: []const u8 = "projections.srf"; + pub const doc_path: []const u8 = "reference/config/projections-srf.md"; +}; + /// Clamp on the magnitude of `spending_change` (10%/yr real, in /// either direction). A larger drift is almost certainly a units /// typo - someone entering a fraction (0.02) where a whole percent diff --git a/src/commands/audit/hygiene.zig b/src/commands/audit/hygiene.zig index b03f9d9..5fe0559 100644 --- a/src/commands/audit/hygiene.zig +++ b/src/commands/audit/hygiene.zig @@ -20,6 +20,10 @@ const contributions = @import("../contributions.zig"); const Money = @import("../../Money.zig"); const analysis = @import("../../analytics/analysis.zig"); const portfolio_mod = @import("../../models/portfolio.zig"); +const classification = @import("../../models/classification.zig"); +const transaction_log = @import("../../models/transaction_log.zig"); +const projections = @import("../../analytics/projections.zig"); +const srf_lint = @import("../../srf_lint.zig"); const Date = @import("../../Date.zig"); const srf = @import("srf"); const git = @import("../../git.zig"); @@ -1265,9 +1269,112 @@ pub fn runHygieneCheck( } } else |_| {} + // ── Section 8: Config file problems ── + // + // SRF discards a record key that names no field of the target + // struct, silently: `price_dat::2024-01-01` is indistinguishable + // from having omitted the price date. 19 of `Lot`'s 22 fields have + // defaults, so most typos here change behavior with no output at + // all. Rules live with the models (`srf_schema`); `srf_lint` walks + // the records. `zfin doctor` reports the same findings per-file with + // a reference-page pointer; this is the routine-use summary. + // + // Silent when clean. + // + // `as_of` is passed as the lint's `today`: its date rules ("a + // close_date in the future") only make sense against the real + // current day, and audit sets `as_of = ctx.today` unconditionally + // (`commands/audit.zig`) - there is no `--as-of` flag to back-date it. + try printFieldNameSection(io, allocator, portfolio_paths, portfolio_path, as_of, color, out); + try out.print("\n", .{}); } +/// Section 8: field names that no model explains, plus each model's +/// `semanticCheck` rules (e.g. `Lot`'s impossible lifecycle dates). +/// +/// Covers every `portfolio*.srf` in the glob plus the portfolio's +/// siblings. `keys.srf` / `theme.srf` are deliberately EXCLUDED - they +/// live under `~/.config/zfin` and configure the TUI, not the +/// portfolio, so they are `doctor`'s business. This section is +/// "portfolio hygiene". +fn printFieldNameSection( + io: std.Io, + allocator: std.mem.Allocator, + portfolio_paths: []const []const u8, + anchor: []const u8, + today: Date, + color: bool, + out: *std.Io.Writer, +) !void { + var header_shown = false; + + for (portfolio_paths) |p| { + try lintOne(io, allocator, portfolio_mod.srf_schema, p, today, color, &header_shown, out); + } + const siblings = .{ + .{ analysis.srf_schema, "accounts.srf" }, + .{ classification.srf_schema, "metadata.srf" }, + .{ transaction_log.srf_schema, "transaction_log.srf" }, + .{ projections.srf_schema, "projections.srf" }, + .{ cli.srf_schema, "watchlist.srf" }, + }; + inline for (siblings) |pair| { + if (cli.siblingPath(allocator, anchor, pair[1])) |path| { + defer allocator.free(path); + try lintOne(io, allocator, pair[0], path, today, color, &header_shown, out); + } else |_| {} + } +} + +/// Lint one file, emitting the shared section header on first finding so +/// a clean run prints nothing at all. +fn lintOne( + io: std.Io, + allocator: std.mem.Allocator, + comptime S: type, + path: []const u8, + today: Date, + color: bool, + header_shown: *bool, + out: *std.Io.Writer, +) !void { + const bytes = std.Io.Dir.cwd().readFileAlloc(io, path, allocator, .limited(64 * 1024 * 1024)) catch return; + defer allocator.free(bytes); + + var result = srf_lint.check(allocator, bytes, comptime srf_lint.shapeOfSchema(S)) catch return; + defer result.deinit(); + // Model-owned semantic rules (a second, typed pass - SRF's + // iterators are single-pass, so `to()` cannot share the raw walk). + try srf_lint.checkSemantic(S, &result, bytes, .{ .today = today }); + result.sort(); + if (result.findings.len == 0) return; + + if (!header_shown.*) { + try out.print("\n", .{}); + try cli.printFg(out, color, cli.CLR_MUTED, " Config file problems\n", .{}); + header_shown.* = true; + } + + try cli.printFg(out, color, cli.CLR_HEADER, " {s}\n", .{std.fs.path.basename(path)}); + var buf: [srf_lint.Finding.describe_max]u8 = undefined; + for (result.findings) |f| { + try cli.printFg(out, color, cli.CLR_WARNING, " line {d}: {s}\n", .{ f.first_line, f.describe(&buf) }); + } + if (result.truncated) { + try cli.printFg(out, color, cli.CLR_WARNING, " (more suppressed; fix these first)\n", .{}); + } + // The authoritative field list, derived from the struct - so unlike + // a doc page it cannot be out of date. Printed whenever there is + // any NAME finding, which is what lets the suggestion matcher stay + // conservative: even with no guess to offer, the user gets the set. + // Skipped for lifecycle-only reports, where it would be noise. + if (result.hasNameFindings()) { + try cli.printFg(out, color, cli.CLR_MUTED, " valid fields:\n", .{}); + try srf_lint.writeValidNames(out, result.shape, 8, 76); + } +} + // ── Tests ──────────────────────────────────────────────────── test "accumulatePresent: unions account numbers across calls" { @@ -2284,3 +2391,167 @@ test "findLastUpdateTimestamps: resolves an account changed in a non-newest comm try std.testing.expectEqual(commits[0].timestamp, roth_ts); // C try std.testing.expect(ira_ts < roth_ts); } + +// ── Section 8: unrecognized field names ── + +/// Shared harness for the Section 8 integration tests. Writes +/// `portfolio.srf` + `accounts.srf` into a tmpdir, runs the full +/// hygiene check, and returns the whole report for the caller to slice. +/// +/// Caller owns the returned bytes. +fn runSection8( + allocator: std.mem.Allocator, + portfolio_srf: []const u8, + accounts_srf: []const u8, +) ![]u8 { + const io = std.testing.io; + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + + try tmp.dir.writeFile(io, .{ .sub_path = "portfolio.srf", .data = portfolio_srf }); + try tmp.dir.writeFile(io, .{ .sub_path = "accounts.srf", .data = accounts_srf }); + + 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]; + + // No API keys / no server -> hermetic. + var svc = zfin.DataService.init(io, allocator, .{ .cache_dir = dir }); + defer svc.deinit(); + + 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); + + var aw: std.Io.Writer.Allocating = .init(allocator); + defer aw.deinit(); + + try runHygieneCheck(io, allocator, &env, &svc, pf_path, &.{pf_path}, 3, false, zfin.Date.fromYmd(2026, 1, 1), 1_767_225_600, false, .never, &aw.writer); + return allocator.dupe(u8, aw.written()); +} + +const section8_header = "Config file problems"; + +test "runHygieneCheck: Section 8 is silent for a clean portfolio" { + const allocator = std.testing.allocator; + const out = try runSection8( + allocator, + \\#!srfv1 + \\symbol::VTI,shares:num:100,open_date::2024-01-15,open_price:num:220.50,account::Sample Brokerage + \\ + , + "#!srfv1\naccount::Sample Brokerage,tax_type::taxable\n", + ); + defer allocator.free(out); + try std.testing.expect(std.mem.indexOf(u8, out, "Portfolio hygiene") != null); + try std.testing.expect(std.mem.indexOf(u8, out, section8_header) == null); +} + +test "runHygieneCheck: Section 8 reports a typo in portfolio.srf with a suggestion" { + const allocator = std.testing.allocator; + const out = try runSection8( + allocator, + \\#!srfv1 + \\symbol::VTI,shares:num:100,open_date::2024-01-15,open_price:num:220.50,price_dat::2025-01-01 + \\ + , + "#!srfv1\n", + ); + defer allocator.free(out); + + const start = std.mem.indexOf(u8, out, section8_header) orelse return error.Section8Missing; + const sec = out[start..]; + try std.testing.expect(std.mem.indexOf(u8, sec, "portfolio.srf") != null); + try std.testing.expect(std.mem.indexOf(u8, sec, "price_dat") != null); + try std.testing.expect(std.mem.indexOf(u8, sec, "did you mean 'price_date'?") != null); + // The valid-name footer is what makes the conservative matcher + // sufficient, so it must actually be emitted. + try std.testing.expect(std.mem.indexOf(u8, sec, "valid fields:") != null); + try std.testing.expect(std.mem.indexOf(u8, sec, "option_type") != null); +} + +test "runHygieneCheck: Section 8 covers accounts.srf, not just the portfolio" { + const allocator = std.testing.allocator; + const out = try runSection8( + allocator, + \\#!srfv1 + \\symbol::VTI,shares:num:100,open_date::2024-01-15,open_price:num:220.50,account::Sample IRA + \\ + , + "#!srfv1\naccount::Sample IRA,tax_type::traditional,tax_mix_rothh:num:20\n", + ); + defer allocator.free(out); + + const start = std.mem.indexOf(u8, out, section8_header) orelse return error.Section8Missing; + const sec = out[start..]; + try std.testing.expect(std.mem.indexOf(u8, sec, "accounts.srf") != null); + try std.testing.expect(std.mem.indexOf(u8, sec, "did you mean 'tax_mix_roth'?") != null); +} + +test "runHygieneCheck: Section 8 flags a field that does not apply to the security_type" { + const allocator = std.testing.allocator; + const out = try runSection8( + allocator, + \\#!srfv1 + \\symbol::VTI,shares:num:100,open_date::2024-01-15,open_price:num:220.50,rate:num:4.25 + \\ + , + "#!srfv1\n", + ); + defer allocator.free(out); + + const start = std.mem.indexOf(u8, out, section8_header) orelse return error.Section8Missing; + const sec = out[start..]; + try std.testing.expect(std.mem.indexOf(u8, sec, "'rate' ignored for security_type stock") != null); + try std.testing.expect(std.mem.indexOf(u8, sec, "read only for cd") != null); +} + +test "runHygieneCheck: Section 8 flags a future close_date" { + // `runSection8` pins as_of to 2026-01-01, which the lint uses as + // `today`. A close in 2062 is the typo'd-year case. + const allocator = std.testing.allocator; + const out = try runSection8( + allocator, + \\#!srfv1 + \\symbol::VTI,shares:num:100,open_date::2024-01-15,open_price:num:220.50,close_date::2062-03-14,close_price:num:300 + \\ + , + "#!srfv1\n", + ); + defer allocator.free(out); + + const start = std.mem.indexOf(u8, out, section8_header) orelse return error.Section8Missing; + const sec = out[start..]; + try std.testing.expect(std.mem.indexOf(u8, sec, "line 2: VTI close_date 2062-03-14 is in the future") != null); +} + +test "runHygieneCheck: Section 8 stays silent for a correctly closed lot" { + const allocator = std.testing.allocator; + const out = try runSection8( + allocator, + \\#!srfv1 + \\symbol::VTI,shares:num:100,open_date::2024-01-15,open_price:num:220.50,close_date::2025-03-14,close_price:num:300 + \\ + , + "#!srfv1\n", + ); + defer allocator.free(out); + try std.testing.expect(std.mem.indexOf(u8, out, section8_header) == null); +} + +test "runHygieneCheck: Section 8 omits the valid-field list for date-only findings" { + const allocator = std.testing.allocator; + const out = try runSection8( + allocator, + \\#!srfv1 + \\symbol::VTI,shares:num:100,open_date::2024-01-15,open_price:num:220.50,close_date::2062-03-14,close_price:num:300 + \\ + , + "#!srfv1\n", + ); + defer allocator.free(out); + const start = std.mem.indexOf(u8, out, section8_header) orelse return error.Section8Missing; + try std.testing.expect(std.mem.indexOf(u8, out[start..], "valid fields:") == null); +} diff --git a/src/commands/common.zig b/src/commands/common.zig index aa39de1..cfe4d89 100644 --- a/src/commands/common.zig +++ b/src/commands/common.zig @@ -1127,14 +1127,27 @@ pub fn resolveAsOfOrExplain( // ── Watchlist loading ──────────────────────────────────────── +/// One record from `watchlist.srf`. +/// +/// Named (rather than declared inline in `loadWatchlist`) so +/// `srf_schema` below can derive the valid-field set from it - an +/// anonymous struct would leave the lint with a hand-copied name list +/// that could drift from the parser. +pub const WatchEntry = struct { symbol: []const u8 }; + +/// Schema contract for `watchlist.srf`. See `srf_lint.validateSchema`. +pub const srf_schema = struct { + pub const Record = WatchEntry; + pub const file_label: []const u8 = "watchlist.srf"; + pub const doc_path: []const u8 = "reference/config/watchlist-srf.md"; +}; + /// Load a watchlist SRF file containing symbol records. /// Returns owned symbol strings. Returns null if file missing or empty. pub fn loadWatchlist(io: std.Io, allocator: std.mem.Allocator, path: []const u8) ?[][]const u8 { const file_data = std.Io.Dir.cwd().readFileAlloc(io, path, allocator, .limited(1024 * 1024)) catch return null; defer allocator.free(file_data); - const WatchEntry = struct { symbol: []const u8 }; - var reader = std.Io.Reader.fixed(file_data); var it = srf.iterator(&reader, allocator, .{ .parse_allocator = .none }) catch return null; defer it.deinit(); diff --git a/src/commands/doctor.zig b/src/commands/doctor.zig index c992198..96e23ad 100644 --- a/src/commands/doctor.zig +++ b/src/commands/doctor.zig @@ -32,11 +32,15 @@ const fmt = cli.fmt; const Config = zfin.Config; const Lot = @import("../models/portfolio.zig").Lot; +const portfolio_mod = @import("../models/portfolio.zig"); +const srf_lint = @import("../srf_lint.zig"); const Date = @import("../Date.zig"); const cache = @import("../cache/store.zig"); const freshness = @import("../cache/freshness.zig"); const classification = @import("../models/classification.zig"); const analysis = @import("../analytics/analysis.zig"); +const projections = @import("../analytics/projections.zig"); +const Journal = @import("../data/Journal.zig"); const transaction_log = @import("../models/transaction_log.zig"); const imported_values = @import("../data/imported_values.zig"); const history = @import("../history.zig"); @@ -514,6 +518,66 @@ fn checkSrfFile( } } +/// Lint one user-authored SRF file's FIELD NAMES, appending one `.warn` +/// row per rolled-up finding plus a pointer to the reference page. +/// +/// Distinct from the `checkSrfFile` row above it, which answers "does +/// this file parse". A file can parse perfectly and still be wrong: +/// SRF discards a key that names no field of the target struct without +/// any error, so `price_dat::2024-01-01` is indistinguishable from +/// having omitted the price date entirely. See `src/srf_lint.zig`. +/// +/// Silent when the file is absent, unreadable, or clean - its presence +/// and parseability are already reported by the row above, and +/// repeating that here would double every message. +/// +/// `.warn`, never `.fail`: a typo is a nudge, and `doctor`'s exit code +/// is reserved for a file that exists but cannot be parsed at all. +fn appendFieldNameChecks( + io: std.Io, + arena: std.mem.Allocator, + checks: *std.ArrayList(Check), + comptime S: type, + path: []const u8, + today: Date, +) !void { + const bytes = std.Io.Dir.cwd().readFileAlloc(io, path, arena, .limited(64 * 1024 * 1024)) catch return; + var result = srf_lint.check(arena, bytes, comptime srf_lint.shapeOfSchema(S)) catch return; + defer result.deinit(); + // Model-owned semantic rules (a second, typed pass - SRF's + // iterators are single-pass, so `to()` cannot share the raw walk). + try srf_lint.checkSemantic(S, &result, bytes, .{ .today = today }); + result.sort(); + if (result.findings.len == 0) return; + + const base = std.fs.path.basename(path); + for (result.findings) |f| { + // `describe` copies into arena memory, which outlives + // `result.deinit()` below; `f.detail` would not. + const buf = try arena.alloc(u8, srf_lint.Finding.describe_max); + try checks.append(arena, .{ + .status = .warn, + .label = try std.fmt.allocPrint(arena, " {s}:{d}", .{ base, f.first_line }), + .detail = f.describe(buf), + }); + } + if (result.truncated) { + try checks.append(arena, .{ + .status = .warn, + .label = try std.fmt.allocPrint(arena, " {s}", .{base}), + .detail = "more findings suppressed; fix these first", + }); + } + // The reference page is guaranteed complete - `srf_lint`'s doc-sync + // test fails the build if a model field is missing from it - so + // pointing there is sound advice rather than a shrug. + try checks.append(arena, .{ + .status = .info, + .label = " field reference", + .detail = try std.fmt.allocPrint(arena, "docs/{s}", .{S.doc_path}), + }); +} + /// Join a sibling filename onto the anchor portfolio's directory. // ── run ─────────────────────────────────────────────────────── @@ -556,6 +620,7 @@ pub fn run(ctx: *framework.RunCtx, _: ParsedArgs) !void { for (pf.paths) |rp| { const c = try checkPortfolioFile(io, arena, rp.path, &all_lots); try checks.append(arena, c); + try appendFieldNameChecks(io, arena, &checks, portfolio_mod.srf_schema, rp.path, ctx.today); } } @@ -564,6 +629,7 @@ pub fn run(ctx: *framework.RunCtx, _: ParsedArgs) !void { { const r = checkSrfFile(io, arena, "accounts.srf", try cli.siblingPath(arena, a, "accounts.srf"), .optional, vAccounts); try checks.append(arena, r); + try appendFieldNameChecks(io, arena, &checks, analysis.srf_schema, try cli.siblingPath(arena, a, "accounts.srf"), ctx.today); if (r.status == .ok) { const path = try cli.siblingPath(arena, a, "accounts.srf"); if (std.Io.Dir.cwd().readFileAlloc(io, path, arena, .limited(16 * 1024 * 1024))) |b| { @@ -575,6 +641,7 @@ pub fn run(ctx: *framework.RunCtx, _: ParsedArgs) !void { { const r = checkSrfFile(io, arena, "metadata.srf", try cli.siblingPath(arena, a, "metadata.srf"), .optional, vMetadata); try checks.append(arena, r); + try appendFieldNameChecks(io, arena, &checks, classification.srf_schema, try cli.siblingPath(arena, a, "metadata.srf"), ctx.today); if (r.status == .ok) { const path = try cli.siblingPath(arena, a, "metadata.srf"); if (std.Io.Dir.cwd().readFileAlloc(io, path, arena, .limited(16 * 1024 * 1024))) |b| { @@ -586,6 +653,7 @@ pub fn run(ctx: *framework.RunCtx, _: ParsedArgs) !void { { const r = checkSrfFile(io, arena, "transaction_log.srf", try cli.siblingPath(arena, a, "transaction_log.srf"), .optional, vTransfers); try checks.append(arena, r); + try appendFieldNameChecks(io, arena, &checks, transaction_log.srf_schema, try cli.siblingPath(arena, a, "transaction_log.srf"), ctx.today); if (r.status == .ok) { const path = try cli.siblingPath(arena, a, "transaction_log.srf"); if (std.Io.Dir.cwd().readFileAlloc(io, path, arena, .limited(16 * 1024 * 1024))) |b| { @@ -594,6 +662,12 @@ pub fn run(ctx: *framework.RunCtx, _: ParsedArgs) !void { } } try checks.append(arena, checkSrfFile(io, arena, "projections.srf", try cli.siblingPath(arena, a, "projections.srf"), .optional, validateSrf)); + try appendFieldNameChecks(io, arena, &checks, projections.srf_schema, try cli.siblingPath(arena, a, "projections.srf"), ctx.today); + // `watchlist.srf` has no parse-check row of its own - + // `loadWatchlist` degrades to null - but its one field is + // as typo-able as any other. + try appendFieldNameChecks(io, arena, &checks, cli.srf_schema, try cli.siblingPath(arena, a, "watchlist.srf"), ctx.today); + try appendFieldNameChecks(io, arena, &checks, Journal.srf_schema, try cli.siblingPath(arena, a, "acknowledgments.srf"), ctx.today); // imported_values.srf and the snapshots both live under // /history/, NOT directly beside the // portfolio file. @@ -609,6 +683,12 @@ pub fn run(ctx: *framework.RunCtx, _: ParsedArgs) !void { // keys.srf / theme.srf live under $HOME/.config/zfin. try checks.append(arena, checkUserConfigFiles(io, arena, config, .keys)); try checks.append(arena, checkUserConfigFiles(io, arena, config, .theme)); + if (userConfigPath(arena, config, "keys.srf")) |p| { + try appendFieldNameChecks(io, arena, &checks, keybinds.srf_schema, p, ctx.today); + } + if (userConfigPath(arena, config, "theme.srf")) |p| { + try appendFieldNameChecks(io, arena, &checks, theme.srf_schema, p, ctx.today); + } try sections.append(arena, .{ .title = "Files", .checks = checks.items }); } @@ -729,10 +809,28 @@ fn checkPortfolioFile(io: std.Io, arena: std.mem.Allocator, path: []const u8, al if (err == error.FileNotFound) return .{ .status = .warn, .label = "portfolio.srf", .detail = "not found" }; return .{ .status = .fail, .label = "portfolio.srf", .detail = std.fmt.allocPrint(arena, "unreadable: {s}", .{@errorName(err)}) catch "unreadable" }; }; - const pf = cache.deserializePortfolio(arena, bytes) catch |err| { + // Ask for diagnostics, not just the lot count. A record that fails + // to coerce is SKIPPED, not fatal, so the plain + // `deserializePortfolio` reports `.ok` with a quietly-smaller + // number - and nobody has a baseline for "how many lots should + // there be". Reporting the skip count is the only way a dropped + // record is visible at all. + var diags: cache.ParseDiagnostics = .empty; + const pf = cache.deserializePortfolioDiag(arena, bytes, &diags) catch |err| { return .{ .status = .fail, .label = path, .detail = std.fmt.allocPrint(arena, "parse error: {s}", .{@errorName(err)}) catch "parse error" }; }; try all_lots.appendSlice(arena, pf.lots); + if (diags.items.len > 0) { + return .{ + .status = .warn, + .label = path, + .detail = std.fmt.allocPrint( + arena, + "{d} lots; {d} record(s) SKIPPED - {s}", + .{ pf.lots.len, diags.items.len, diags.items[0] }, + ) catch "records skipped", + }; + } return .{ .status = .ok, .label = path, .detail = std.fmt.allocPrint(arena, "{d} lots", .{pf.lots.len}) catch "" }; } @@ -807,6 +905,15 @@ const UserConfigKind = enum { keys, theme }; /// Check `$HOME/.config/zfin/{keys,theme}.srf`. These resolve from /// $HOME only (not ZFIN_HOME / cwd), mirroring the TUI loader. +/// `$HOME/.config/zfin/`, or null when HOME is unset. +/// Mirrors the path `checkUserConfigFiles` builds, so the field-name +/// lint reads the same file the parse-check reported on. +fn userConfigPath(arena: std.mem.Allocator, config: Config, filename: []const u8) ?[]const u8 { + const home = if (config.environ_map) |em| em.get("HOME") else null; + if (home == null) return null; + return std.fs.path.join(arena, &.{ home.?, ".config", "zfin", filename }) catch null; +} + fn checkUserConfigFiles(io: std.Io, arena: std.mem.Allocator, config: Config, kind: UserConfigKind) Check { const filename = switch (kind) { .keys => "keys.srf", @@ -1617,3 +1724,182 @@ test "checkCandleFreshness: an untracked laggard does not warn" { const c = try checkCandleFreshness(arena, &store, &tracked, &.{}, now_s); try testing.expectEqual(Status.ok, c.status); } + +// ── appendFieldNameChecks ── + +/// Fixed `today` so the date rules' verdicts don't drift with the clock. +const fixed_today = Date.fromYmd(2026, 1, 1); + +/// Write `data` to a tmpdir and collect the field-name checks it +/// produces for schema `S`. Caller owns nothing - everything lives in +/// the arena the caller passes. +fn fieldChecksFor( + arena: std.mem.Allocator, + comptime S: type, + filename: []const u8, + data: []const u8, +) !std.ArrayList(Check) { + const io = std.testing.io; + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + try tmp.dir.writeFile(io, .{ .sub_path = filename, .data = data }); + + 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(arena, &.{ path_buf[0..dir_len], filename }); + + var checks: std.ArrayList(Check) = .empty; + try appendFieldNameChecks(io, arena, &checks, S, path, fixed_today); + return checks; +} + +test "appendFieldNameChecks: clean file adds nothing" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const checks = try fieldChecksFor( + arena, + portfolio_mod.srf_schema, + "portfolio.srf", + "#!srfv1\nsymbol::VTI,shares:num:1,open_date::2024-01-01,open_price:num:1\n", + ); + try std.testing.expectEqual(@as(usize, 0), checks.items.len); +} + +test "appendFieldNameChecks: missing file adds nothing" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + var checks: std.ArrayList(Check) = .empty; + try appendFieldNameChecks(std.testing.io, arena, &checks, portfolio_mod.srf_schema, "/nonexistent/portfolio.srf", fixed_today); + try std.testing.expectEqual(@as(usize, 0), checks.items.len); +} + +test "appendFieldNameChecks: a typo warns and points at the reference page" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const checks = try fieldChecksFor( + arena, + portfolio_mod.srf_schema, + "portfolio.srf", + "#!srfv1\nsymbol::VTI,shares:num:1,open_date::2024-01-01,open_price:num:1,price_dat::2025-01-01\n", + ); + // One finding row plus the reference-page pointer. + try std.testing.expectEqual(@as(usize, 2), checks.items.len); + + try std.testing.expectEqual(Status.warn, checks.items[0].status); + try std.testing.expect(std.mem.indexOf(u8, checks.items[0].label, "portfolio.srf:2") != null); + try std.testing.expect(std.mem.indexOf(u8, checks.items[0].detail, "did you mean 'price_date'?") != null); + + // `.info`, not `.warn` - the pointer is not itself a problem. + try std.testing.expectEqual(Status.info, checks.items[1].status); + try std.testing.expect(std.mem.indexOf(u8, checks.items[1].detail, "portfolio-srf.md") != null); +} + +test "appendFieldNameChecks: never returns .fail, so a typo cannot change the exit code" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + // One of every finding kind the walker can produce. + const checks = try fieldChecksFor( + arena, + portfolio_mod.srf_schema, + "portfolio.srf", + \\#!srfv1 + \\shares:num:1,open_date::2024-01-01,open_price:num:1,price_dat::2025-01-01 + \\shares:num:1,open_date::2024-01-01,open_price:num:1,Account::X + \\shares:num:1,open_date::2024-01-01,open_price:num:1,rate:num:5 + \\shares:num:1,open_date::2024-01-01,open_price:num:1,split_factor:num:2 + \\shares:num:1,open_date::2024-01-01,open_price:num:1,nonsense::x + \\ + , + ); + try std.testing.expect(checks.items.len > 1); + for (checks.items) |c| { + try std.testing.expect(c.status != .fail); + } + // And `countByStatus` agrees, which is what gates the exit code. + const sections = [_]Section{.{ .title = "Files", .checks = checks.items }}; + try std.testing.expectEqual(@as(usize, 0), countByStatus(§ions, .fail)); +} + +test "checkPortfolioFile: a skipped record warns instead of quietly lowering the count" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + // Second record omits `open_price`, which has no default - SRF + // cannot coerce it, so the loader drops the record. Before this + // check asked for diagnostics the report said `[OK] 1 lots`. + try tmp.dir.writeFile(io, .{ + .sub_path = "portfolio.srf", + .data = + \\#!srfv1 + \\symbol::VTI,shares:num:1,open_date::2024-01-01,open_price:num:1 + \\symbol::SPY,shares:num:2,open_date::2024-01-02 + \\ + , + }); + 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(arena, &.{ path_buf[0..dir_len], "portfolio.srf" }); + + var lots: std.ArrayList(Lot) = .empty; + const c = try checkPortfolioFile(io, arena, path, &lots); + try std.testing.expectEqual(Status.warn, c.status); + try std.testing.expect(std.mem.indexOf(u8, c.detail, "SKIPPED") != null); + try std.testing.expectEqual(@as(usize, 1), lots.items.len); +} + +// ── userConfigPath ── + +test "userConfigPath: builds the $HOME/.config/zfin path" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + var env = try std.testing.environ.createMap(std.testing.allocator); + defer env.deinit(); + try env.put("HOME", "/home/placeholder"); + + const p = userConfigPath(arena, .{ .cache_dir = "/tmp/placeholder-cache", .environ_map = &env }, "keys.srf"); + try std.testing.expectEqualStrings("/home/placeholder/.config/zfin/keys.srf", p.?); +} + +test "userConfigPath: null without an environment, so the lint is skipped rather than guessing" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + + // No environ map at all - the same branch a HOME-less environment + // takes. Returning null means `run` skips the keys/theme lint + // instead of inventing a path. + try std.testing.expectEqual( + @as(?[]const u8, null), + userConfigPath(arena_state.allocator(), .{ .cache_dir = "/tmp/placeholder-cache" }, "theme.srf"), + ); +} + +test "appendFieldNameChecks: lot lifecycle rules reach doctor as warnings" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const checks = try fieldChecksFor( + arena, + portfolio_mod.srf_schema, + "portfolio.srf", + "#!srfv1\nsymbol::VTI,shares:num:1,open_date::2024-01-01,open_price:num:1,close_date::2062-03-14,close_price:num:2\n", + ); + // One finding plus the reference-page pointer. + try std.testing.expectEqual(@as(usize, 2), checks.items.len); + try std.testing.expectEqual(Status.warn, checks.items[0].status); + try std.testing.expect(std.mem.indexOf(u8, checks.items[0].detail, "close_date 2062-03-14 is in the future") != null); +} diff --git a/src/data/Journal.zig b/src/data/Journal.zig index 589c21a..d15c34a 100644 --- a/src/data/Journal.zig +++ b/src/data/Journal.zig @@ -107,6 +107,19 @@ const JournalRecord = union(enum) { note: NoteRecord, }; +/// Schema contract for `acknowledgments.srf`. See +/// `srf_lint.validateSchema`. +/// +/// `Acknowledgment` does NOT redeclare the `type` tag field, unlike +/// `projections.srf`'s variants - SRF's `to()` consumes the tag before +/// recursing into the variant, so both spellings are legal. The shape +/// builder accepts `type::` for every variant either way. +pub const srf_schema = struct { + pub const Record = JournalRecord; + pub const file_label: []const u8 = "acknowledgments.srf"; + pub const doc_path: []const u8 = "reference/config/acknowledgments-srf.md"; +}; + /// In-memory ack with its notes already grouped. Built by `load`; /// consumed by callers that want "the ack and its reasoning together." pub const Entry = struct { diff --git a/src/models/classification.zig b/src/models/classification.zig index 6687a20..5646b37 100644 --- a/src/models/classification.zig +++ b/src/models/classification.zig @@ -53,6 +53,13 @@ pub const ClassificationEntry = struct { splits_current_through: ?Date = null, }; +/// Schema contract for `metadata.srf`. See `srf_lint.validateSchema`. +pub const srf_schema = struct { + pub const Record = ClassificationEntry; + pub const file_label: []const u8 = "metadata.srf"; + pub const doc_path: []const u8 = "reference/config/metadata-srf.md"; +}; + /// Parsed classification data for the entire portfolio. pub const ClassificationMap = struct { entries: []ClassificationEntry, diff --git a/src/models/portfolio.zig b/src/models/portfolio.zig index 4c331e5..e27fa00 100644 --- a/src/models/portfolio.zig +++ b/src/models/portfolio.zig @@ -3,6 +3,7 @@ const Date = @import("../Date.zig"); const Candle = @import("candle.zig").Candle; const split = @import("split.zig"); const Split = split.Split; +const srf_lint = @import("../srf_lint.zig"); // ── Pricing model ──────────────────────────────────────────── // @@ -448,6 +449,199 @@ pub const Lot = struct { } }; +/// Schema contract for `portfolio.srf`, consumed by `srf_lint` and +/// rendered by `zfin doctor` / `zfin audit`. +/// +/// Lives here rather than in `srf_lint.zig` on purpose: rules kept away +/// from the fields they describe drift from them. `field_rules` is +/// checked for exhaustiveness over `std.meta.fields(Lot)` at comptime +/// in both directions, so adding a field to `Lot` fails the build until +/// it is classified here, and renaming one fails the build too. +pub const srf_schema = struct { + pub const Record = Lot; + pub const file_label: []const u8 = "portfolio.srf"; + pub const doc_path: []const u8 = "reference/config/portfolio-srf.md"; + + /// Fields deliberately absent from the reference doc. Everything + /// else must appear there or the doc-sync test fails. + pub const undocumented = [_][]const u8{ + // DERIVED by `enrichSplits`; documenting it would invite + // hand-editing, which `field_rules` flags below. + "split_factor", + }; + + pub const scope_discriminator: []const u8 = "security_type"; + + /// Which `LotType`s each field is actually read for. + /// + /// A `.only` entry is a claim that the value is SILENTLY IGNORED + /// for every other type, so each one cites the gate that proves it. + /// When a field's reach is ambiguous the entry is deliberately + /// permissive (no `.only`) - a missed nudge costs nothing, while a + /// false "zfin ignores this" would send someone chasing a + /// non-existent bug. + pub const field_rules = [_]srf_lint.Rule{ + .{ .name = "symbol" }, + .{ .name = "shares" }, + .{ .name = "open_date" }, + .{ .name = "open_price" }, + .{ .name = "close_date" }, + .{ .name = "close_price" }, + .{ .name = "note" }, + .{ .name = "label" }, + .{ .name = "account" }, + .{ .name = "security_type" }, + + // Read ungated by `lotIsOpenAsOf` (this file, "Matured on or + // before `as_of`"), so a maturity on a stock lot really does + // close it. NOT option/cd-only despite the doc comment. + .{ .name = "maturity_date" }, + + // CD yield. Read only by `analytics/reconcile/common.zig`'s + // CD-allowance math and the CD table's rate column in + // `views/portfolio_sections.zig`. + .{ .name = "rate", .only = &.{"cd"} }, + + // Dividend-reinvestment grouping is a stock-table concern, but + // `commands/contributions.zig`'s lot differ reads it for any + // type. Permissive. + .{ .name = "drip" }, + + // Pricing fields. `analytics/valuation.zig`'s + // `buildFallbackPrices` gates manual prices to `.stock`, but + // `commands/audit.zig`'s reconcile price map reads `lot.price` + // ungated - so a manual price on a non-stock lot is not + // reliably ignored. Permissive until those two agree. + .{ .name = "ticker" }, + .{ .name = "price" }, + .{ .name = "price_date" }, + .{ .name = "price_ratio" }, + + // Populated by `enrichSplits`, never by hand. + .{ .name = "split_factor", .derived = true }, + + // Option mechanics. Every read is behind an `.option` gate: + // `models/option.zig`'s lot matcher, `analytics/valuation.zig`'s + // covered-call detection, and the three `@abs(shares) * + // open_price * multiplier` sites (this file's + // `totalOptionCostAsOf`, `analytics/analysis.zig`, and + // `commands/snapshot.zig`). + .{ .name = "underlying", .only = &.{"option"} }, + .{ .name = "strike", .only = &.{"option"} }, + .{ .name = "multiplier", .only = &.{"option"} }, + .{ .name = "option_type", .only = &.{"option"} }, + }; + + /// Lot lifecycle rules - dates and close fields that parse fine but + /// describe a lot that cannot exist. + /// + /// These matter more than they look because zfin has THREE + /// definitions of "closed", and bad lifecycle data is exactly what + /// makes them disagree: + /// + /// - `lotIsOpenAsOf` (holdings, snapshots, analysis): closed iff + /// `close_date <= as_of`. + /// - `commands/contributions.zig`'s lot differ and + /// `commands/import.zig`: closed iff `close_date != null`. + /// - `views/portfolio_sections.zig`'s `effectivePriceFor` (lot + /// rows): priced at `close_price` whenever it is set. + /// + /// A future `close_date` is therefore held by the first, sold by the + /// second, and priced as sold by the third. Stopping the input here + /// is the cheap fix; reconciling the three definitions is separate + /// work. + /// + /// Watch lots are skipped throughout: `Portfolio.watchSymbols` reads + /// only `symbol`, so their dates are inert. + pub fn semanticCheck(rec: Record, ctx: srf_lint.Context, sink: *srf_lint.Sink) !void { + if (rec.security_type == .watch) return; + const a = sink.allocator; + const name = lotName(rec); + + if (rec.close_date) |cd| { + // End-of-day semantics: a close dated today is already + // closed, so only strictly-later dates are wrong. + if (ctx.today.lessThan(cd)) { + try sink.addOwned(.semantic, "close_date", try std.fmt.allocPrint( + a, + "{s} close_date {f} is in the future - the lot is still counted as held until then", + .{ name, cd }, + )); + } + // Equal is a same-day round trip, which is legitimate. + if (cd.lessThan(rec.open_date)) { + try sink.addOwned(.semantic, "close_date", try std.fmt.allocPrint( + a, + "{s} close_date {f} is before open_date {f} - the lot can never have been held", + .{ name, cd, rec.open_date }, + )); + } + // Stock-only: both position builders skip non-stock lots, + // so realized P&L is a stock concept. A cash or CD lot + // closed without a price is plausible - its shares ARE its + // dollar value. + if (rec.close_price == null and rec.security_type == .stock) { + try sink.addOwned(.semantic, "close_price", try std.fmt.allocPrint( + a, + "{s} has close_date but no close_price - the sale's realized gain is recorded as 0", + .{name}, + )); + } + } else if (rec.close_price != null) { + // Only a stock lot's row consults `close_price` while open + // (`effectivePriceFor`); for every other type it is inert. + const consequence: []const u8 = if (rec.security_type == .stock) + "the lot is still held, and its row shows the close price instead of the live one" + else + "the lot is still held and the close price is ignored"; + try sink.addOwned(.semantic, "close_price", try std.fmt.allocPrint( + a, + "{s} has close_price but no close_date - {s}", + .{ name, consequence }, + )); + } + + if (ctx.today.lessThan(rec.open_date)) { + try sink.addOwned(.semantic, "open_date", try std.fmt.allocPrint( + a, + "{s} open_date {f} is in the future - the lot is not counted as held until then", + .{ name, rec.open_date }, + )); + } + + // Stock-only: manual-price staleness (`commands/audit/hygiene.zig`) + // only examines stock lots, so that is the only place a future + // date suppresses anything. + if (rec.security_type == .stock) { + if (rec.price_date) |pd| { + if (ctx.today.lessThan(pd)) { + try sink.addOwned(.semantic, "price_date", try std.fmt.allocPrint( + a, + "{s} price_date {f} is in the future - the manual price counts as fresh, so it will not be flagged stale", + .{ name, pd }, + )); + } + } + } + } + + /// How a finding names the lot. Cash lots often have no symbol, so + /// fall back to the type rather than printing an empty string - the + /// line number already pins down which record it is. + fn lotName(lot: Lot) []const u8 { + const s = lot.displaySymbol(); + if (s.len > 0) return s; + return switch (lot.security_type) { + .cash => "cash lot", + .cd => "CD lot", + .option => "option lot", + .illiquid => "illiquid lot", + .stock => "stock lot", + .watch => "watch lot", + }; + } +}; + /// Populate each open stock lot's `split_factor` from the fetched split /// `corpus` (keyed by `priceSymbol()`), in place. /// @@ -2163,3 +2357,179 @@ test "extraPriceSymbols: order is stable - watch lots first, then the watchlist try std.testing.expectEqualStrings("QTUM", out[3]); } } + +// ── srf_schema.semanticCheck: lot lifecycle rules ── + +/// `today` for these tests. Fixed, so a verdict can't flip as the +/// calendar moves past a fixture date. +const lint_today = Date.fromYmd(2026, 1, 1); + +/// Run both lint passes over `data` as portfolio.srf and return only +/// the semantic findings' descriptions, joined by newlines. Caller +/// frees. +fn lifecycleFindings(data: []const u8) ![]u8 { + const allocator = std.testing.allocator; + var r = try srf_lint.check(allocator, data, srf_lint.shapeOfSchema(srf_schema)); + defer r.deinit(); + try srf_lint.checkSemantic(srf_schema, &r, data, .{ .today = lint_today }); + r.sort(); + + var aw: std.Io.Writer.Allocating = .init(allocator); + errdefer aw.deinit(); + var buf: [srf_lint.Finding.describe_max]u8 = undefined; + for (r.findings) |f| { + if (f.kind != .semantic) continue; + try aw.writer.print("{d}: {s}\n", .{ f.first_line, f.describe(&buf) }); + } + return aw.toOwnedSlice(); +} + +fn expectLifecycle(data: []const u8, want: []const []const u8) !void { + const got = try lifecycleFindings(data); + defer std.testing.allocator.free(got); + var lines: usize = 0; + var it = std.mem.splitScalar(u8, std.mem.trimEnd(u8, got, "\n"), '\n'); + while (it.next()) |l| { + if (l.len > 0) lines += 1; + } + for (want) |w| { + // On a miss, let `expectEqualStrings` print both sides: it always + // fails here (a substring was not found), and it reports through + // `std.testing` rather than a hand-rolled print. + if (std.mem.indexOf(u8, got, w) == null) return std.testing.expectEqualStrings(w, got); + } + if (lines != want.len) { + const joined = try std.mem.join(std.testing.allocator, "\n", want); + defer std.testing.allocator.free(joined); + return std.testing.expectEqualStrings(joined, got); + } +} + +test "lifecycle: a fully valid open and closed lot are clean" { + try expectLifecycle( + \\#!srfv1 + \\symbol::VTI,shares:num:10,open_date::2024-01-15,open_price:num:220 + \\symbol::SPY,shares:num:5,open_date::2024-01-15,open_price:num:470,close_date::2025-06-01,close_price:num:560 + \\ + , &.{}); +} + +test "lifecycle: future close_date is flagged" { + try expectLifecycle( + \\#!srfv1 + \\symbol::VTI,shares:num:10,open_date::2024-01-15,open_price:num:220,close_date::2062-03-14,close_price:num:300 + \\ + , &.{"2: VTI close_date 2062-03-14 is in the future - the lot is still counted as held until then"}); +} + +test "lifecycle: a close dated today is already closed, not future" { + // End-of-day semantics, matching `lotIsOpenAsOf`. + try expectLifecycle( + \\#!srfv1 + \\symbol::VTI,shares:num:10,open_date::2024-01-15,open_price:num:220,close_date::2026-01-01,close_price:num:300 + \\ + , &.{}); +} + +test "lifecycle: close before open is flagged; a same-day round trip is not" { + try expectLifecycle( + \\#!srfv1 + \\symbol::VTI,shares:num:10,open_date::2024-06-01,open_price:num:220,close_date::2024-05-01,close_price:num:230 + \\symbol::SPY,shares:num:10,open_date::2024-06-01,open_price:num:470,close_date::2024-06-01,close_price:num:471 + \\ + , &.{"2: VTI close_date 2024-05-01 is before open_date 2024-06-01"}); +} + +test "lifecycle: stock lot closed without a price loses its realized gain" { + try expectLifecycle( + \\#!srfv1 + \\symbol::SPY,shares:num:5,open_date::2024-01-15,open_price:num:470,close_date::2025-06-01 + \\ + , &.{"2: SPY has close_date but no close_price - the sale's realized gain is recorded as 0"}); +} + +test "lifecycle: cash and CD lots closed without a price are fine" { + // Their shares ARE their dollar value; there is no realized P&L to lose. + try expectLifecycle( + \\#!srfv1 + \\security_type::cash,shares:num:5000,open_date::2024-01-01,open_price:num:1,close_date::2025-01-01,account::Sample Brokerage + \\security_type::cd,symbol::CD1,shares:num:10000,open_date::2024-01-01,open_price:num:10000,close_date::2025-01-01 + \\ + , &.{}); +} + +test "lifecycle: close_price without close_date - stock row shows the wrong price" { + try expectLifecycle( + \\#!srfv1 + \\symbol::SPY,shares:num:5,open_date::2024-01-15,open_price:num:470,close_price:num:560 + \\ + , &.{"2: SPY has close_price but no close_date - the lot is still held, and its row shows the close price instead of the live one"}); +} + +test "lifecycle: close_price without close_date - non-stock says it is ignored" { + try expectLifecycle( + \\#!srfv1 + \\security_type::cd,symbol::CD1,shares:num:10000,open_date::2024-01-01,open_price:num:10000,close_price:num:10000 + \\ + , &.{"2: CD1 has close_price but no close_date - the lot is still held and the close price is ignored"}); +} + +test "lifecycle: future open_date is flagged" { + try expectLifecycle( + \\#!srfv1 + \\symbol::VTI,shares:num:10,open_date::2062-01-15,open_price:num:220 + \\ + , &.{"2: VTI open_date 2062-01-15 is in the future - the lot is not counted as held until then"}); +} + +test "lifecycle: future price_date is flagged on stock lots only" { + // Manual-price staleness only ever examines stock lots, so a future + // price_date on a CD suppresses nothing and is left alone. + try expectLifecycle( + \\#!srfv1 + \\symbol::NON40,shares:num:10,open_date::2024-01-15,open_price:num:150,price:num:163,price_date::2062-02-27 + \\symbol::NON41,shares:num:10,open_date::2024-01-15,open_price:num:150,price:num:163,price_date::2025-12-31 + \\security_type::cd,symbol::CD1,shares:num:10000,open_date::2024-01-01,open_price:num:10000,price_date::2062-01-01 + \\ + , &.{"2: NON40 price_date 2062-02-27 is in the future - the manual price counts as fresh"}); +} + +test "lifecycle: watch lots are exempt" { + // `Portfolio.watchSymbols` reads only `symbol`; dates are inert. + try expectLifecycle( + \\#!srfv1 + \\security_type::watch,symbol::NVDA,shares:num:0,open_date::2062-01-01,open_price:num:0,close_date::2063-01-01 + \\ + , &.{}); +} + +test "lifecycle: maturity_date in the future is legitimate" { + try expectLifecycle( + \\#!srfv1 + \\security_type::cd,symbol::CD1,shares:num:10000,open_date::2025-06-01,open_price:num:10000,maturity_date::2027-06-01,rate:num:4.5 + \\ + , &.{}); +} + +test "lifecycle: two bad lots stay two findings, and one lot can have several" { + // Rollup identity includes `detail`, which names the symbol, so the + // same rule on different lots must not collapse into one line. + try expectLifecycle( + \\#!srfv1 + \\symbol::VTI,shares:num:10,open_date::2024-01-15,open_price:num:220,close_date::2062-03-14 + \\symbol::SPY,shares:num:5,open_date::2024-01-15,open_price:num:470,close_date::2062-03-14,close_price:num:1 + \\ + , &.{ + "2: VTI close_date 2062-03-14 is in the future", + "2: VTI has close_date but no close_price", + "3: SPY close_date 2062-03-14 is in the future", + }); +} + +test "lifecycle: a lot without a symbol is named by its type" { + try expectLifecycle( + \\#!srfv1 + \\security_type::cash,shares:num:5000,open_date::2062-01-01,open_price:num:1,account::Sample Brokerage + \\ + , &.{"2: cash lot open_date 2062-01-01 is in the future"}); +} diff --git a/src/models/transaction_log.zig b/src/models/transaction_log.zig index 349f3a6..32a12f2 100644 --- a/src/models/transaction_log.zig +++ b/src/models/transaction_log.zig @@ -184,6 +184,14 @@ pub const TransferRecord = struct { } }; +/// Schema contract for `transaction_log.srf`. See +/// `srf_lint.validateSchema`. +pub const srf_schema = struct { + pub const Record = TransferRecord; + pub const file_label: []const u8 = "transaction_log.srf"; + pub const doc_path: []const u8 = "reference/config/transaction-log-srf.md"; +}; + /// Parsed transaction log. `transfers` is allocator-owned; all string /// fields on each record (including the symbol inside `dest_lot.lot`) /// are also owned by the log's allocator. diff --git a/src/srf_lint.zig b/src/srf_lint.zig new file mode 100644 index 0000000..33fa50c --- /dev/null +++ b/src/srf_lint.zig @@ -0,0 +1,1802 @@ +//! SRF schema lint - catch hand-edit typos in user-authored SRF files. +//! +//! ## The problem +//! +//! SRF's `fields.to(T)` silently discards any record key that doesn't +//! name a field of `T`. The relevant loop is `srf.zig`'s: +//! +//! ```zig +//! while (try self.next()) |f| { +//! var field_match = false; // declared here... +//! inline for (std.meta.fields(T)) |type_field| { +//! if (!field_match and std.mem.eql(u8, type_field.name, f.key) ...) { +//! field_match = true; // ...set here... +//! } +//! } +//! } // ...and never read. +//! ``` +//! +//! `field_match` is write-only, so a typo'd key is byte-for-byte +//! equivalent to omitting the field. There is no strict mode, no +//! unknown-field callback, and no `error.UnknownField` anywhere in the +//! library. A field WITHOUT a default errors as +//! `FieldNotFoundOnFieldWithoutDefaultValue` (which names the missing +//! field, not the wrong key); a field WITH a default is silently +//! skipped. Most fields on zfin's user-authored models have defaults: +//! 19 of `Lot`'s 22, 14 of `AccountTaxEntry`'s 16, and all 17 of +//! `SrfConfig`'s - the last behind an infallible parser, so a typo +//! there changes projected retirement math with zero output. +//! +//! ## Division of responsibility +//! +//! This module is an AGGREGATION POINT, not a rule book. It owns the +//! raw-record walker, the name matchers, rollup, and the comptime +//! contract validator. It knows nothing about any particular model. +//! +//! Each model module owns its own schema and declares it as +//! `pub const srf_schema` (see `validateSchema` for the contract, and +//! `models/portfolio.zig` for the reference implementation). That +//! placement is deliberate: rules that live away from the fields they +//! describe drift from them, which is exactly how +//! `docs/reference/config/portfolio-srf.md` came to document 14 of +//! `Lot`'s 22 fields. Three mechanisms keep a schema honest: +//! +//! 1. The valid-name set is DERIVED (`std.meta.fields(Record)`), +//! never declared, so it cannot drift. +//! 2. `field_rules` is checked for exhaustiveness over +//! `std.meta.fields(Record)` in BOTH directions at comptime - a +//! new field fails the build until classified, and a renamed +//! field fails the build too. +//! 3. `doc_path` + `undocumented` drive a doc-sync test, so adding +//! a field without documenting it fails `zig build test`. +//! +//! ## Why a separate pass +//! +//! SRF's `RecordIterator`/`FieldIterator` are single-pass and +//! consuming - no peek, no rewind - and `to()` drains the field +//! iterator. Raw inspection and `to()` are therefore mutually +//! exclusive on the same record, so this is a second walk over the +//! same bytes rather than a change to any existing parse path. It runs +//! only from `zfin doctor` and `zfin audit`, never on the hot path. +//! With `.parse_allocator = .none` the keys borrow from the input +//! buffer, so a clean file allocates nothing but the result arena. + +const std = @import("std"); +const srf = @import("srf"); +const comptime_validator = @import("comptime_validator.zig"); +const Date = @import("Date.zig"); +/// Backticked identifiers found in each `docs/reference/config/*.md` +/// page, extracted at build time by `build/gen_config_docs.zig`. +/// `@embedFile` cannot reach outside `src/`, hence the generator. +const config_docs = @import("config_docs"); + +/// Longest record this lints in full. Every zfin model is far smaller +/// (`Lot`, the widest, has 22 fields), so the cap only bites on a +/// malformed file; records beyond it are still name-checked, they just +/// stop contributing conditional-applicability findings (which need +/// the whole record buffered to locate the discriminator). +const max_fields_per_record = 64; + +/// Upper bound on DISTINCT findings, after rollup. A file that trips +/// more than this is misconfigured in a way a longer list won't +/// clarify, and the renderers have to print whatever we return. +const max_findings = 200; + +/// Shortest name length eligible for prefix matching. Below this the +/// suggestion is noise: `pc` would "match" `pct`. +/// +/// Three is safe for every model in the registry - no valid field name +/// is a 3-character prefix of another name in the same record's set +/// (`tax_type` and `tax_mix_*` share `tax_` but neither prefixes the +/// other). A test pins that property so a future field addition can't +/// quietly break it. +const min_prefix_len = 3; + +// ── Findings ────────────────────────────────────────────────── + +pub const Kind = enum { + /// Key matches no field, and no matcher found a near relative. + unknown, + /// Key differs from a real field only by case. SRF matches field + /// names with `std.mem.eql`, so this silently does nothing. + case_mismatch, + /// Key is a prefix of a real field or vice versa - the shape of a + /// dropped or doubled character. + near_miss, + /// Key appears twice in one record. SRF keeps the FIRST and + /// silently drops the rest, so editing the second does nothing. + duplicate, + /// Real field, but never read for this record's discriminator + /// value (e.g. `rate` on a `security_type::stock` lot). + inapplicable, + /// Real field that zfin derives and never reads from a + /// hand-edited file (e.g. `Lot.split_factor`). + derived, + /// Model-specific rule, reported by the schema's `semanticCheck`. + semantic, +}; + +/// One rolled-up problem. Identity is `(kind, key)`; repeats across +/// records bump `count` and record up to two more line numbers rather +/// than emitting another entry - one typo in a copy-pasted template +/// must not produce 200 lines of output. +pub const Finding = struct { + kind: Kind, + /// The offending field name. Borrows from the `data` passed to + /// `check`, so `data` must outlive the `Result`. + key: []const u8, + /// The real field name this probably meant. Set for + /// `case_mismatch` and `near_miss`. Static (a comptime field + /// name), never owned. + suggestion: ?[]const u8 = null, + /// Free-text explanation. Static or owned by the `Result` arena. + detail: []const u8 = "", + first_line: u32, + count: u32 = 1, + /// Second and third line this appeared on, for a "lines 12, 19, + /// 26, ..." tail. Only `extra_len` entries are meaningful. + extra_lines: [2]u32 = .{ 0, 0 }, + extra_len: u8 = 0, + + fn noteRepeat(self: *Finding, line: u32) void { + self.count += 1; + if (self.extra_len < self.extra_lines.len) { + self.extra_lines[self.extra_len] = line; + self.extra_len += 1; + } + } + + /// Longest string `describe` can produce. Field names are bounded + /// by Zig identifier length in practice, and `detail` by + /// `describeOnly`'s output over one discriminator enum. + pub const describe_max = 320; + + /// One-line human description, written into `buf` and returned as a + /// slice of it. Shared by `zfin doctor` and `zfin audit` so the two + /// surfaces cannot word the same finding differently. + /// + /// `buf` should be at least `describe_max` bytes; a shorter buffer + /// truncates rather than failing, because a clipped diagnostic is + /// still more useful than none. + pub fn describe(self: Finding, buf: []u8) []const u8 { + var w = std.Io.Writer.fixed(buf); + self.write(&w) catch return buf[0..w.end]; + return buf[0..w.end]; + } + + fn write(self: Finding, w: *std.Io.Writer) !void { + switch (self.kind) { + .unknown => try w.print("unrecognized field '{s}'", .{self.key}), + .case_mismatch => try w.print( + "field '{s}' differs only by case from '{s}' - SRF field names are case-sensitive", + .{ self.key, self.suggestion orelse "" }, + ), + .near_miss => try w.print( + "unrecognized field '{s}' - did you mean '{s}'?", + .{ self.key, self.suggestion orelse "" }, + ), + .duplicate => try w.print("field '{s}' appears twice in one record; {s}", .{ self.key, self.detail }), + .inapplicable => try w.print("field '{s}' {s}", .{ self.key, self.detail }), + .derived => try w.print("field '{s}' is {s}", .{ self.key, self.detail }), + .semantic => try w.writeAll(self.detail), + } + if (self.count > 1) { + try w.print(" ({d} records: lines {d}", .{ self.count, self.first_line }); + for (self.extra_lines[0..self.extra_len]) |l| try w.print(", {d}", .{l}); + try w.writeAll(if (self.count > 1 + self.extra_len) ", ...)" else ")"); + } + } +}; + +/// Findings for one file, plus the shape they were checked against so +/// a renderer can print the valid-name footer. +/// +/// Owns an arena for any allocated `detail` strings. `Finding.key` +/// borrows from the `data` slice passed to `check`. +pub const Result = struct { + findings: []const Finding, + shape: Shape, + /// True when `max_findings` was hit and some were dropped. + truncated: bool = false, + arena: std.heap.ArenaAllocator, + + pub fn deinit(self: *Result) void { + self.arena.deinit(); + } + + pub fn isClean(self: Result) bool { + return self.findings.len == 0; + } + + /// True when some finding is about a field NAME (unknown, wrong + /// case, near miss) - the cases where printing the valid-name list + /// helps. A report of only lifecycle or duplicate findings names + /// real fields already, and the list would just be noise. + pub fn hasNameFindings(self: Result) bool { + for (self.findings) |f| { + switch (f.kind) { + .unknown, .case_mismatch, .near_miss => return true, + .duplicate, .inapplicable, .derived, .semantic => {}, + } + } + return false; + } + + /// Order findings by the line they were first seen on, then by key. + /// + /// Needed because `checkSemantic` appends a whole second pass after + /// `check`'s, so an unsorted report interleaves line 4 before line + /// 2 and reads like a bug. Call after the last pass that appends. + pub fn sort(self: *Result) void { + // `findings` is const to callers but owned by our arena, so the + // cast is sound - nothing else can alias it. + const items = @constCast(self.findings); + std.mem.sort(Finding, items, {}, struct { + fn lessThan(_: void, a: Finding, b: Finding) bool { + if (a.first_line != b.first_line) return a.first_line < b.first_line; + return std.mem.lessThan(u8, a.key, b.key); + } + }.lessThan); + } +}; + +/// Inputs a schema's `semanticCheck` may need beyond the record itself. +/// +/// Passed by value alongside the record rather than stored on `Sink`, +/// which collects findings and should not also carry inputs. +pub const Context = struct { + /// The current calendar day - not an `as_of`. Rules like "a + /// `close_date` in the future" are nonsensical against a + /// back-dated reference: they would flag every real close made + /// after it. Captured once at the unit-of-work entry point and + /// threaded down, per the `today` rule in AGENTS.md. + today: Date, +}; + +/// Collects findings during a walk. Passed to a schema's +/// `semanticCheck` so model-owned rules report through the same +/// channel as the generic ones. +/// +/// Deliberately has no format-string method: `addOwned` takes a string +/// the caller built with `std.fmt.allocPrint(sink.allocator, ...)` and +/// assumes ownership of it. That keeps `anytype` out of the contract +/// while leaving ownership explicit at the call site. +pub const Sink = struct { + /// The `Result` arena. Anything allocated here lives as long as + /// the `Result`. + allocator: std.mem.Allocator, + findings: std.ArrayList(Finding), + truncated: bool = false, + /// Line of the record being walked. Set by `check`; read by + /// `semanticCheck` implementations via `add*`. + line: u32 = 0, + + /// Add a finding whose `detail` is a static string. + pub fn addStatic(self: *Sink, kind: Kind, key: []const u8, detail: []const u8) !void { + try self.push(.{ .kind = kind, .key = key, .detail = detail, .first_line = self.line }); + } + + /// Add a finding whose `detail` was allocated from + /// `self.allocator`. The `Result` arena frees it. + pub fn addOwned(self: *Sink, kind: Kind, key: []const u8, detail: []const u8) !void { + try self.push(.{ .kind = kind, .key = key, .detail = detail, .first_line = self.line }); + } + + fn addSuggestion(self: *Sink, kind: Kind, key: []const u8, suggestion: []const u8) !void { + try self.push(.{ .kind = kind, .key = key, .suggestion = suggestion, .first_line = self.line }); + } + + /// Roll `f` into an existing matching finding, or append it. + /// + /// Identity is `(kind, key, detail)` - `detail` included on purpose. + /// The generic kinds carry a static per-kind detail, so they still + /// collapse (one typo across 200 records stays one line). A + /// `semantic` finding's detail names the specific record ("account + /// 'Sample IRA': ..."), so including it keeps two accounts with the + /// same broken field from merging into one line that names only the + /// first. + fn push(self: *Sink, f: Finding) !void { + for (self.findings.items) |*existing| { + if (existing.kind == f.kind and + std.mem.eql(u8, existing.key, f.key) and + std.mem.eql(u8, existing.detail, f.detail)) + { + existing.noteRepeat(f.first_line); + return; + } + } + if (self.findings.items.len >= max_findings) { + self.truncated = true; + return; + } + try self.findings.append(self.allocator, f); + } +}; + +// ── Shape: what a record is allowed to contain ──────────────── + +/// A conditional-applicability rule for one field. +pub const Rule = struct { + name: []const u8, + /// Discriminator values this field is actually read for. `null` + /// means "read for all of them". + only: ?[]const []const u8 = null, + /// Derived by zfin, never read from a hand-edited file. Presence + /// in a user file is itself a finding. + derived: bool = false, +}; + +/// Conditional-applicability rules for a flat record, keyed off one +/// enum field's value. +pub const Scopes = struct { + /// Field whose value selects applicability (e.g. `security_type`). + discriminator: []const u8, + /// Value used when the discriminator is absent from a record. + /// DERIVED from the field's declared default by `shapeOfSchema`, + /// never hand-written, so it cannot drift from the struct. + default_value: []const u8, + rules: []const Rule, + + fn ruleFor(self: Scopes, name: []const u8) ?Rule { + for (self.rules) |r| { + if (std.mem.eql(u8, r.name, name)) return r; + } + return null; + } +}; + +/// What field names a record may carry. +pub const Shape = union(enum) { + /// A plain struct: one fixed name set for every record. + flat: Flat, + /// A tagged union: the tag field's value selects the name set. + tagged: Tagged, + + pub const Flat = struct { + names: []const []const u8, + scopes: ?Scopes = null, + }; + + pub const Tagged = struct { + tag_key: []const u8, + variants: []const Variant, + + fn variantFor(self: Tagged, tag_value: []const u8) ?Variant { + for (self.variants) |v| { + if (std.mem.eql(u8, v.tag_value, tag_value)) return v; + } + return null; + } + }; + + pub const Variant = struct { + tag_value: []const u8, + names: []const []const u8, + }; +}; + +/// Field names of `T`, as a comptime slice. The single source of the +/// valid-name set - derived, so it cannot drift from the struct. +/// +/// The data is held as a container-level `const` so it has static +/// storage and the returned slice stays valid when this is called from +/// a runtime context. +pub fn namesOf(comptime T: type) []const []const u8 { + const Holder = struct { + const names = blk: { + const fields = std.meta.fields(T); + var n: [fields.len][]const u8 = undefined; + for (fields, 0..) |f, i| n[i] = f.name; + break :blk n; + }; + }; + return &Holder.names; +} + +/// Build the `Shape` for a schema module, validating its contract. +/// +/// Handles struct and tagged-union records. For a union, the tag key +/// is `Record.srf_tag_field` when declared and `"type"` otherwise, +/// matching SRF's own rule, and the tag key is always accepted even +/// when the variant struct does not redeclare it - `to()` consumes the +/// tag before recursing into the variant, so `SrfConfig` (which +/// redeclares `type`) and `Journal.Acknowledgment` (which does not) +/// must both lint clean. +pub fn shapeOfSchema(comptime S: type) Shape { + const Holder = struct { + const shape = blk: { + validateSchema(S); + const Record = S.Record; + break :blk switch (@typeInfo(Record)) { + .@"struct" => Shape{ .flat = .{ + .names = namesOf(Record), + .scopes = if (@hasDecl(S, "field_rules")) scopesOfSchema(S) else null, + } }, + .@"union" => tagged: { + const tag_key = if (@hasDecl(Record, "srf_tag_field")) Record.srf_tag_field else "type"; + const vfields = std.meta.fields(Record); + var variants: [vfields.len]Shape.Variant = undefined; + for (vfields, 0..) |vf, i| { + // The tag key is valid for every variant + // whether or not the variant redeclares it. + const inner = namesOf(vf.type); + var names: [inner.len + 1][]const u8 = undefined; + names[0] = tag_key; + var n: usize = 1; + for (inner) |name| { + if (!std.mem.eql(u8, name, tag_key)) { + names[n] = name; + n += 1; + } + } + const frozen = names; + variants[i] = .{ .tag_value = vf.name, .names = frozen[0..n] }; + } + const frozen_variants = variants; + break :tagged Shape{ .tagged = .{ .tag_key = tag_key, .variants = &frozen_variants } }; + }, + else => @compileError("srf_schema `" ++ @typeName(Record) ++ + "`: Record must be a struct or tagged union"), + }; + }; + }; + return Holder.shape; +} + +/// Assemble `Scopes` from a schema's `field_rules`, deriving +/// `default_value` from the discriminator field's declared default so +/// the two cannot disagree. +fn scopesOfSchema(comptime S: type) Scopes { + comptime { + const Record = S.Record; + const disc = S.scope_discriminator; + const D = @FieldType(Record, disc); + const default_ptr = for (std.meta.fields(Record)) |f| { + if (std.mem.eql(u8, f.name, disc)) break f.default_value_ptr; + } else unreachable; + if (default_ptr == null) { + @compileError("srf_schema `" ++ @typeName(Record) ++ "`: discriminator `" ++ disc ++ + "` must have a default value (it selects applicability for records that omit it)"); + } + const default_tag: D = @as(*const D, @ptrCast(@alignCast(default_ptr.?))).*; + return .{ + .discriminator = disc, + .default_value = @tagName(default_tag), + .rules = &S.field_rules, + }; + } +} + +// ── Comptime contract validation ────────────────────────────── + +/// Assert a model's `srf_schema` conforms to the contract, with a +/// copy-pasteable `@compileError` when it does not. Mirrors +/// `tui/tab_framework.zig`'s `validateTabModule`. +/// +/// Required: +/// pub const Record = ; +/// pub const file_label = "portfolio.srf"; +/// pub const doc_path = "reference/config/portfolio-srf.md"; +/// +/// Optional: +/// pub const undocumented = [_][]const u8{ ... }; +/// pub const scope_discriminator = "security_type"; // with field_rules +/// pub const field_rules = [_]srf_lint.Rule{ ... }; // with scope_discriminator +/// pub fn semanticCheck(rec: Record, ctx: srf_lint.Context, sink: *srf_lint.Sink) !void +pub fn validateSchema(comptime S: type) void { + comptime { + const kind = "SRF schema"; + const name = if (@hasDecl(S, "file_label")) S.file_label else @typeName(S); + + if (!@hasDecl(S, "Record")) { + @compileError(kind ++ " `" ++ name ++ "` is missing `pub const Record = ;`"); + } + comptime_validator.expectDeclWithType( + kind, + name, + S, + "file_label", + []const u8, + "pub const file_label: []const u8 = \"portfolio.srf\";", + ); + comptime_validator.expectDeclWithType( + kind, + name, + S, + "doc_path", + []const u8, + "pub const doc_path: []const u8 = \"reference/config/portfolio-srf.md\";", + ); + + const has_disc = @hasDecl(S, "scope_discriminator"); + const has_rules = @hasDecl(S, "field_rules"); + if (has_disc != has_rules) { + @compileError(kind ++ " `" ++ name ++ "`: `scope_discriminator` and `field_rules` " ++ + "must be declared together (one selects applicability, the other lists it)"); + } + if (has_rules) validateRules(S, kind, name); + + if (@hasDecl(S, "semanticCheck")) { + comptime_validator.expectFnInferredError( + kind, + name, + S, + "semanticCheck", + &.{ S.Record, Context, *Sink }, + void, + "pub fn semanticCheck(rec: Record, ctx: srf_lint.Context, sink: *srf_lint.Sink) !void", + ); + } + } +} + +/// Exhaustiveness check over `field_rules`, in both directions. +/// +/// This is the mechanism that stops the rules from drifting from the +/// struct: adding a field to `Record` fails the build until it is +/// classified, and renaming one fails the build here too. `only` +/// values are checked against the discriminator enum's tag names, so a +/// typo in the rules themselves is also a compile error. +fn validateRules(comptime S: type, comptime kind: []const u8, comptime name: []const u8) void { + comptime { + // The exhaustiveness check is O(fields x rules) string + // comparisons - 22 x 22 for `Lot` - which overruns the default + // branch budget on its own. + @setEvalBranchQuota(100_000); + const Record = S.Record; + const disc = S.scope_discriminator; + const fields = std.meta.fields(Record); + + if (!@hasField(Record, disc)) { + @compileError(kind ++ " `" ++ name ++ "`: scope_discriminator `" ++ disc ++ + "` is not a field of " ++ @typeName(Record)); + } + const D = @FieldType(Record, disc); + if (@typeInfo(D) != .@"enum") { + @compileError(kind ++ " `" ++ name ++ "`: scope_discriminator `" ++ disc ++ + "` must be an enum field, got " ++ @typeName(D)); + } + + // Every field classified exactly once. + for (fields) |f| { + var seen = 0; + for (S.field_rules) |r| { + if (std.mem.eql(u8, r.name, f.name)) seen += 1; + } + if (seen == 0) { + @compileError(kind ++ " `" ++ name ++ "`: field `" ++ f.name ++ + "` is not classified in `field_rules`. Add one of:\n" ++ + " .{ .name = \"" ++ f.name ++ "\" }, // read for every " ++ disc ++ "\n" ++ + " .{ .name = \"" ++ f.name ++ "\", .only = &.{.some_value} }, // read only for those\n" ++ + " .{ .name = \"" ++ f.name ++ "\", .derived = true }, // zfin derives it; never hand-edited"); + } + if (seen > 1) { + @compileError(kind ++ " `" ++ name ++ "`: field `" ++ f.name ++ + "` is classified more than once in `field_rules`"); + } + } + + // Every rule names a real field, and every `only` value a real tag. + for (S.field_rules) |r| { + if (!@hasField(Record, r.name)) { + @compileError(kind ++ " `" ++ name ++ "`: `field_rules` entry `" ++ r.name ++ + "` is not a field of " ++ @typeName(Record) ++ " (renamed or removed?)"); + } + if (r.derived and r.only != null) { + @compileError(kind ++ " `" ++ name ++ "`: `field_rules` entry `" ++ r.name ++ + "` sets both `derived` and `only`; a derived field is never hand-edited for any " ++ disc); + } + if (r.only) |vals| { + if (vals.len == 0) { + @compileError(kind ++ " `" ++ name ++ "`: `field_rules` entry `" ++ r.name ++ + "` has an empty `only` list; omit `only` for \"read everywhere\" or set `derived`"); + } + for (vals) |v| { + if (!@hasField(D, v)) { + @compileError(kind ++ " `" ++ name ++ "`: `field_rules` entry `" ++ r.name ++ + "` lists `only` value `" ++ v ++ "`, which is not a tag of " ++ @typeName(D)); + } + } + } + } + } +} + +// ── Name matching ───────────────────────────────────────────── + +fn indexOfName(names: []const []const u8, key: []const u8) ?usize { + for (names, 0..) |n, i| { + if (std.mem.eql(u8, n, key)) return i; + } + return null; +} + +/// A real field name differing from `key` only by case. SRF matches +/// with `std.mem.eql`, so these silently do nothing. +fn caseMatch(names: []const []const u8, key: []const u8) ?[]const u8 { + for (names) |n| { + if (std.ascii.eqlIgnoreCase(n, key)) return n; + } + return null; +} + +/// A real field name in a prefix relationship with `key`, in either +/// direction - the shape of a dropped or doubled trailing character +/// (`price_dat`, `price_datee`) or a truncation. +/// +/// Deliberately exact rather than an edit-distance score: no threshold +/// to tune, and it cannot produce a wrong suggestion. It catches +/// strictly fewer typos than Levenshtein would, and the renderers make +/// up the difference: `audit` prints the file's full valid-name set +/// whenever a finding is about a name (`Result.hasNameFindings`), and +/// `doctor` points at the reference page, which the doc-sync test keeps +/// complete. +/// +/// Picks the candidate whose LENGTH is closest to `key`'s, because one +/// real field can prefix another: `Lot` has both `price` and +/// `price_date`, so `price_dat` prefix-matches both and first-hit-wins +/// would answer `price`. A regression test walks every registered +/// model's fields and asserts each one's truncated and doubled forms +/// suggest it back. +fn prefixMatch(names: []const []const u8, key: []const u8) ?[]const u8 { + if (key.len < min_prefix_len) return null; + var best: ?[]const u8 = null; + var best_delta: usize = std.math.maxInt(usize); + for (names) |n| { + if (n.len < min_prefix_len) continue; + if (!std.mem.startsWith(u8, n, key) and !std.mem.startsWith(u8, key, n)) continue; + const delta = if (n.len > key.len) n.len - key.len else key.len - n.len; + if (delta < best_delta) { + best_delta = delta; + best = n; + } + } + return best; +} + +// ── The walk ────────────────────────────────────────────────── + +/// One record's raw keys, buffered so the discriminator can be located +/// before applicability is judged (it may appear after the field it +/// governs). +const RecordBuf = struct { + // SAFETY: only `keys[0..len]` is ever read, and `push` writes each + // slot before incrementing `len`. + keys: [max_fields_per_record][]const u8 = undefined, + len: usize = 0, + overflowed: bool = false, + /// Raw string value of the discriminator/tag field, when present. + disc_value: ?[]const u8 = null, + + fn push(self: *RecordBuf, key: []const u8) void { + if (self.len == self.keys.len) { + self.overflowed = true; + return; + } + self.keys[self.len] = key; + self.len += 1; + } +}; + +/// Walk `data` as SRF and report every key that `shape` does not +/// explain. `data` must outlive the returned `Result`. +pub fn check(allocator: std.mem.Allocator, data: []const u8, shape: Shape) !Result { + var arena = std.heap.ArenaAllocator.init(allocator); + errdefer arena.deinit(); + + var sink: Sink = .{ .allocator = arena.allocator(), .findings = .empty }; + + var reader = std.Io.Reader.fixed(data); + // A file that isn't SRF at all is `doctor`'s existing parse-check's + // problem, not ours - report no findings rather than a confusing + // wall of "unknown field". + var it = srf.iterator(&reader, arena.allocator(), .{ .parse_allocator = .none }) catch { + return .{ .findings = &.{}, .shape = shape, .arena = arena }; + }; + defer it.deinit(); + + while (it.next() catch null) |fields| { + // Matches `cache/store.zig`'s diagnostics: the record's first + // line, captured before the field walk advances it. + const line: u32 = @intCast(it.state.line); + sink.line = line; + + var buf: RecordBuf = .{}; + const disc_name: ?[]const u8 = switch (shape) { + .flat => |f| if (f.scopes) |s| s.discriminator else null, + .tagged => |t| t.tag_key, + }; + + while (fields.next() catch null) |f| { + buf.push(f.key); + if (disc_name) |dn| { + if (buf.disc_value == null and std.mem.eql(u8, f.key, dn)) { + if (f.value) |v| { + if (v == .string) buf.disc_value = v.string; + } + } + } + } + + try checkRecord(&sink, shape, buf); + } + + const findings = try sink.findings.toOwnedSlice(arena.allocator()); + return .{ + .findings = findings, + .shape = shape, + .truncated = sink.truncated, + .arena = arena, + }; +} + +fn checkRecord(sink: *Sink, shape: Shape, buf: RecordBuf) !void { + const names: []const []const u8, const scopes: ?Scopes = switch (shape) { + .flat => |f| .{ f.names, f.scopes }, + .tagged => |t| blk: { + const tag_value = buf.disc_value orelse return; // untagged record: `to()` errors on it + const variant = t.variantFor(tag_value) orelse return; // unknown tag: ditto + break :blk .{ variant.names, null }; + }, + }; + + // Which valid names have been consumed, for duplicate detection. + var seen = [_]bool{false} ** max_fields_per_record; + + const disc_value: []const u8 = if (scopes) |s| (buf.disc_value orelse s.default_value) else ""; + + for (buf.keys[0..buf.len]) |key| { + if (indexOfName(names, key)) |idx| { + if (idx < seen.len) { + if (seen[idx]) { + try sink.addStatic(.duplicate, key, "SRF keeps the first occurrence and ignores the rest"); + continue; + } + seen[idx] = true; + } + // Known field. Is it read for this record? + if (scopes) |s| { + if (buf.overflowed) continue; + const rule = s.ruleFor(key) orelse continue; + if (rule.derived) { + try sink.addStatic(.derived, key, "derived by zfin; remove it from your file"); + } else if (rule.only) |vals| { + if (indexOfName(vals, disc_value) == null) { + const detail = try describeOnly(sink.allocator, s.discriminator, vals, disc_value); + try sink.addOwned(.inapplicable, key, detail); + } + } + } + continue; + } + if (caseMatch(names, key)) |n| { + try sink.addSuggestion(.case_mismatch, key, n); + } else if (prefixMatch(names, key)) |n| { + try sink.addSuggestion(.near_miss, key, n); + } else { + try sink.addStatic(.unknown, key, ""); + } + } +} + +/// "ignored for security_type stock; read only for cd". +fn describeOnly( + allocator: std.mem.Allocator, + discriminator: []const u8, + only: []const []const u8, + actual: []const u8, +) ![]const u8 { + var aw: std.Io.Writer.Allocating = .init(allocator); + errdefer aw.deinit(); + try aw.writer.print("ignored for {s} {s}; read only for ", .{ discriminator, actual }); + for (only, 0..) |v, i| { + if (i > 0) try aw.writer.writeAll(if (i + 1 == only.len) " and " else ", "); + try aw.writer.writeAll(v); + } + return aw.toOwnedSlice(); +} + +/// Run a schema's model-owned `semanticCheck` over `data`, appending +/// to `result`. +/// +/// A SECOND typed pass, separate from `check`'s raw one, because SRF's +/// iterators are single-pass: `to()` drains the fields that the raw +/// walk needs. Records that fail to coerce are skipped silently - the +/// typed parser's own diagnostics (and `doctor`'s parse-check) already +/// report those, and duplicating them here would double every message. +pub fn checkSemantic(comptime S: type, result: *Result, data: []const u8, ctx: Context) !void { + if (!@hasDecl(S, "semanticCheck")) return; + const allocator = result.arena.allocator(); + + var sink: Sink = .{ .allocator = allocator, .findings = .empty }; + try sink.findings.appendSlice(allocator, result.findings); + sink.truncated = result.truncated; + + var reader = std.Io.Reader.fixed(data); + var it = srf.iterator(&reader, allocator, .{ .parse_allocator = .none }) catch return; + defer it.deinit(); + + while (it.next() catch null) |fields| { + sink.line = @intCast(it.state.line); + const rec = fields.to(S.Record, @import("srf_opts.zig").user_edited) catch continue; + try S.semanticCheck(rec, ctx, &sink); + } + + result.findings = try sink.findings.toOwnedSlice(allocator); + result.truncated = sink.truncated; +} + +/// Write the valid field names for `shape` to `w`, wrapped to `width` +/// columns and indented by `indent` spaces. +/// +/// Renderers call this whenever `Result.hasNameFindings`. It is +/// what makes the deliberately-conservative matchers sufficient: even +/// when no suggestion can be offered, the user gets the authoritative +/// list - derived from the struct, so unlike the reference docs it +/// cannot be out of date. +pub fn writeValidNames(w: *std.Io.Writer, shape: Shape, indent: usize, width: usize) !void { + switch (shape) { + .flat => |f| try writeNameList(w, "", f.names, indent, width), + .tagged => |t| { + for (t.variants) |v| { + var label_buf: [64]u8 = undefined; + const label = std.fmt.bufPrint(&label_buf, "{s}::{s} ", .{ t.tag_key, v.tag_value }) catch ""; + try writeNameList(w, label, v.names, indent, width); + } + }, + } +} + +fn writeNameList( + w: *std.Io.Writer, + label: []const u8, + names: []const []const u8, + indent: usize, + width: usize, +) !void { + try w.splatByteAll(' ', indent); + try w.writeAll(label); + var col = indent + label.len; + for (names) |n| { + // +1 for the separating space. Wrap before overflowing so a + // narrow terminal does not ragged-wrap mid-name. + if (col > indent and col + n.len + 1 > width) { + try w.writeAll("\n"); + try w.splatByteAll(' ', indent + 2); + col = indent + 2; + } + try w.writeAll(n); + try w.writeAll(" "); + col += n.len + 1; + } + try w.writeAll("\n"); +} + +// ── Registry ────────────────────────────────────────────────── + +/// Every user-authored SRF file zfin reads, paired with the model that +/// owns its schema. Nine one-liners, mirroring `tui.zig`'s +/// `tab_modules`. +/// +/// **Adding a tenth user-authored file means adding it here.** That is +/// the one drift this design does not close at comptime - there is no +/// way to ask Zig "who references `srf_opts.user_edited`" - but it is +/// the cheap kind: a new file goes unchecked, nothing becomes wrong. +/// `grep -rn srf_opts.user_edited src/` is the authoritative index, and +/// that constant's doc comment carries the same reminder. +/// +/// `history/imported_values.srf` is deliberately ABSENT. It is +/// generated by `tools/import_values.zig` from a spreadsheet export and +/// hand-editing it is explicitly disallowed (see the module doc on +/// `data/imported_values.zig`), so there are no hand-typed field names +/// to get wrong. It is also the only `user_edited` parse site with no +/// `docs/reference/config/*-srf.md` page, which independently confirms +/// the classification. +pub const schemas = .{ + @import("models/portfolio.zig").srf_schema, + @import("analytics/analysis.zig").srf_schema, + @import("models/classification.zig").srf_schema, + @import("models/transaction_log.zig").srf_schema, + @import("analytics/projections.zig").srf_schema, + @import("data/Journal.zig").srf_schema, + @import("tui/keybinds.zig").srf_schema, + @import("tui/theme.zig").srf_schema, + @import("commands/common.zig").srf_schema, +}; + +/// Validate every registered schema at build time. Mirrors +/// `tui.zig`'s comptime sweep over `tab_modules`. +pub const validated_schemas = blk: { + for (schemas) |S| _ = shapeOfSchema(S); + break :blk true; +}; + +/// Number of registered schemas. +pub const schema_count = schemas.len; + +// ── Tests ───────────────────────────────────────────────────── + +const testing = std.testing; + +// Every field of every registered model must appear in that model's +// reference page, or be listed in the schema's `undocumented`. +// +// This is the mechanism that keeps the docs from drifting the way +// `portfolio-srf.md` already had: it documented 14 of `Lot`'s 22 +// fields in its table, and adding a field had no consequence. Now it +// does - this test fails until the field is documented or explicitly +// exempted, and the exemption is a visible decision in the schema. +// +// A field counts as documented if it appears anywhere outside a fenced +// code block, in either spelling the docs use: bare (`` `symbol` ``, +// reference tables) or on-wire (`` `symbol::` ``, prose). Code fences +// are excluded on purpose - an example that happens to mention a field +// is not a description of it. +test "doc sync: every model field appears in its reference page" { + // Referencing this forces the comptime sweep over `schemas`, so a + // schema with a contract violation or a non-exhaustive + // `field_rules` fails the build rather than going unvalidated. + try testing.expect(validated_schemas); + + var aw: std.Io.Writer.Allocating = .init(testing.allocator); + defer aw.deinit(); + + var missing: usize = 0; + inline for (schemas) |S| { + const doc_file = comptime std.fs.path.basename(S.doc_path); + const doc = config_docs.find(doc_file) orelse { + std.debug.print("srf_schema '{s}': no reference page named '{s}'\n", .{ S.file_label, doc_file }); + return error.MissingReferencePage; + }; + switch (comptime shapeOfSchema(S)) { + .flat => |f| missing += try reportUndocumented(&aw.writer, S, doc, f.names, ""), + .tagged => |t| { + for (t.variants) |v| { + missing += try reportUndocumented(&aw.writer, S, doc, v.names, t.tag_key); + } + }, + } + } + if (missing > 0) { + std.debug.print( + \\ + \\{d} model field(s) are not described in their reference page: + \\{s} + \\Document each one, or add it to that schema's `undocumented` + \\list with a comment saying why it is not user-facing. + \\ + , .{ missing, aw.written() }); + } + try testing.expectEqual(@as(usize, 0), missing); +} + +/// Write a line to `w` for each field of `S` absent from its reference +/// page, and return how many there were. +/// +/// Reports through a writer rather than printing: zlint's `no-print` +/// rule exempts `test` blocks but not the helpers they call, and +/// funnelling the text back to the one caller is both cleaner and +/// keeps the diagnostic in a single flush. +fn reportUndocumented( + w: *std.Io.Writer, + comptime S: type, + doc: config_docs.Doc, + names: []const []const u8, + tag_key: []const u8, +) !usize { + const exempt: []const []const u8 = if (@hasDecl(S, "undocumented")) &S.undocumented else &.{}; + var n: usize = 0; + for (names) |name| { + // The union tag key is SRF machinery, not a model field. + if (tag_key.len > 0 and std.mem.eql(u8, name, tag_key)) continue; + if (indexOfName(doc.names, name) != null) continue; + if (indexOfName(exempt, name) != null) continue; + try w.print(" {s}: field '{s}' is undocumented in {s}\n", .{ S.file_label, name, doc.file }); + n += 1; + } + return n; +} + +test "registry: every schema has a distinct file label and doc page" { + try testing.expect(schema_count == 9); + inline for (schemas, 0..) |A, i| { + inline for (schemas, 0..) |B, j| { + if (comptime i >= j) continue; + try testing.expect(!std.mem.eql(u8, A.file_label, B.file_label)); + try testing.expect(!std.mem.eql(u8, A.doc_path, B.doc_path)); + } + } +} + +test "registry: every schema builds a usable shape and lints a clean empty file" { + inline for (schemas) |S| { + var r = try check(testing.allocator, "#!srfv1\n", comptime shapeOfSchema(S)); + defer r.deinit(); + try testing.expect(r.isClean()); + } +} + +test "registry: a one-character typo of any real field suggests that field back" { + // The property that matters to a user, checked against the REAL + // models rather than a fixture: drop the last character of a field + // name, or double it, and the lint must point at the field you + // meant. `Lot` alone has `price`, `price_date` and `price_ratio`, + // so this is where a naive first-hit-wins matcher goes wrong. + var aw: std.Io.Writer.Allocating = .init(testing.allocator); + defer aw.deinit(); + + var bad: usize = 0; + inline for (schemas) |S| { + switch (comptime shapeOfSchema(S)) { + .flat => |f| bad += try reportBadSuggestions(&aw.writer, S.file_label, f.names), + .tagged => |t| { + for (t.variants) |v| bad += try reportBadSuggestions(&aw.writer, S.file_label, v.names); + }, + } + } + if (bad > 0) std.debug.print("\n{s}", .{aw.written()}); + try testing.expectEqual(@as(usize, 0), bad); +} + +/// Write a line to `w` for each field whose one-character typo forms +/// resolve to the wrong suggestion, and return how many there were. +fn reportBadSuggestions(w: *std.Io.Writer, label: []const u8, names: []const []const u8) !usize { + var buf: [128]u8 = undefined; + var bad: usize = 0; + for (names) |name| { + if (name.len < min_prefix_len + 1) continue; + + // Dropped trailing character. If the truncation IS another real + // field, an exact match wins and no suggestion is wanted. + const truncated = name[0 .. name.len - 1]; + if (indexOfName(names, truncated) == null) { + const got = prefixMatch(names, truncated); + if (got == null or !std.mem.eql(u8, got.?, name)) { + try w.print(" {s}: '{s}' (from '{s}') suggested {?s}, want '{s}'\n", .{ label, truncated, name, got, name }); + bad += 1; + } + } + + // Doubled trailing character. + @memcpy(buf[0..name.len], name); + buf[name.len] = name[name.len - 1]; + const doubled = buf[0 .. name.len + 1]; + if (indexOfName(names, doubled) == null) { + const got = prefixMatch(names, doubled); + if (got == null or !std.mem.eql(u8, got.?, name)) { + try w.print(" {s}: '{s}' (from '{s}') suggested {?s}, want '{s}'\n", .{ label, doubled, name, got, name }); + bad += 1; + } + } + } + return bad; +} + +// ── Fixtures ────────────────────────────────────────────────── + +const TestLotType = enum { stock, option, cd, cash }; + +const TestLot = struct { + symbol: []const u8 = "", + shares: f64, + account: ?[]const u8 = null, + security_type: TestLotType = .stock, + rate: ?f64 = null, + strike: ?f64 = null, + split_factor: f64 = 1.0, +}; + +const test_lot_schema = struct { + pub const Record = TestLot; + pub const file_label: []const u8 = "test_lot.srf"; + pub const doc_path: []const u8 = "reference/config/test-lot-srf.md"; + pub const scope_discriminator: []const u8 = "security_type"; + pub const field_rules = [_]Rule{ + .{ .name = "symbol" }, + .{ .name = "shares" }, + .{ .name = "account" }, + .{ .name = "security_type" }, + .{ .name = "rate", .only = &.{"cd"} }, + .{ .name = "strike", .only = &.{"option"} }, + .{ .name = "split_factor", .derived = true }, + }; +}; + +const TestUnion = union(enum) { + config: struct { type: []const u8 = "", horizon: u16 = 0 }, + birthdate: struct { date: []const u8 = "", person: u8 = 1 }, +}; + +const test_union_schema = struct { + pub const Record = TestUnion; + pub const file_label: []const u8 = "test_union.srf"; + pub const doc_path: []const u8 = "reference/config/test-union-srf.md"; +}; + +fn lintLot(data: []const u8) !Result { + return check(testing.allocator, data, shapeOfSchema(test_lot_schema)); +} + +fn findingFor(r: Result, key: []const u8) ?Finding { + for (r.findings) |f| { + if (std.mem.eql(u8, f.key, key)) return f; + } + return null; +} + +// ── check: the raw walk ─────────────────────────────────────── + +test "check: clean file produces no findings" { + const data = + \\#!srfv1 + \\symbol::VTI,shares:num:100,account::Sample Brokerage + \\symbol::SPY,shares:num:50,account::Sample IRA + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + try testing.expect(r.isClean()); +} + +test "check: unknown key with no relative" { + const data = + \\#!srfv1 + \\symbol::VTI,shares:num:100,cost_basis:num:1000 + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + try testing.expectEqual(@as(usize, 1), r.findings.len); + try testing.expectEqual(Kind.unknown, r.findings[0].kind); + try testing.expectEqualStrings("cost_basis", r.findings[0].key); + try testing.expectEqual(@as(?[]const u8, null), r.findings[0].suggestion); +} + +test "check: case-only mismatch names the real field" { + const data = + \\#!srfv1 + \\Symbol::VTI,shares:num:100 + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + const f = findingFor(r, "Symbol") orelse return error.MissingFinding; + try testing.expectEqual(Kind.case_mismatch, f.kind); + try testing.expectEqualStrings("symbol", f.suggestion.?); +} + +test "check: near miss on a dropped trailing character" { + const data = + \\#!srfv1 + \\symbol::VTI,shares:num:100,accoun::Sample IRA + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + const f = findingFor(r, "accoun") orelse return error.MissingFinding; + try testing.expectEqual(Kind.near_miss, f.kind); + try testing.expectEqualStrings("account", f.suggestion.?); +} + +test "check: near miss on a doubled trailing character" { + const data = + \\#!srfv1 + \\symbol::VTI,shares:num:100,accountt::Sample IRA + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + const f = findingFor(r, "accountt") orelse return error.MissingFinding; + try testing.expectEqual(Kind.near_miss, f.kind); + try testing.expectEqualStrings("account", f.suggestion.?); +} + +test "check: prefix matching ignores keys below the length floor" { + // `sy` is a prefix of `symbol` but too short to suggest against. + const data = + \\#!srfv1 + \\shares:num:100,sy::VTI + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + const f = findingFor(r, "sy") orelse return error.MissingFinding; + try testing.expectEqual(Kind.unknown, f.kind); +} + +test "check: duplicate key in one record" { + const data = + \\#!srfv1 + \\symbol::VTI,shares:num:100,shares:num:200 + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + const f = findingFor(r, "shares") orelse return error.MissingFinding; + try testing.expectEqual(Kind.duplicate, f.kind); +} + +test "check: inapplicable field for the record's discriminator" { + const data = + \\#!srfv1 + \\symbol::VTI,shares:num:100,rate:num:5.25 + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + const f = findingFor(r, "rate") orelse return error.MissingFinding; + try testing.expectEqual(Kind.inapplicable, f.kind); + // Default discriminator value is derived from the struct, so an + // absent `security_type` still reports as `stock`. + try testing.expect(std.mem.indexOf(u8, f.detail, "security_type stock") != null); + try testing.expect(std.mem.indexOf(u8, f.detail, "cd") != null); +} + +test "check: applicable field for the right discriminator is clean" { + const data = + \\#!srfv1 + \\symbol::CD1,shares:num:1000,security_type::cd,rate:num:5.25 + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + try testing.expect(r.isClean()); +} + +test "check: discriminator is honored even when it appears after the field" { + // `strike` precedes `security_type`, so the record must be + // buffered before applicability is judged. + const data = + \\#!srfv1 + \\symbol::AMZN,shares:num:1,strike:num:200,security_type::option + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + try testing.expect(r.isClean()); +} + +test "check: derived field in a hand-edited file" { + const data = + \\#!srfv1 + \\symbol::VTI,shares:num:100,split_factor:num:4 + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + const f = findingFor(r, "split_factor") orelse return error.MissingFinding; + try testing.expectEqual(Kind.derived, f.kind); +} + +test "check: repeated typo rolls up instead of repeating" { + const data = + \\#!srfv1 + \\symbol::A,shares:num:1,accoun::X + \\symbol::B,shares:num:2,accoun::X + \\symbol::C,shares:num:3,accoun::X + \\symbol::D,shares:num:4,accoun::X + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + try testing.expectEqual(@as(usize, 1), r.findings.len); + const f = r.findings[0]; + try testing.expectEqual(@as(u32, 4), f.count); + try testing.expectEqual(@as(u32, 2), f.first_line); + // Two more lines retained for the "lines 2, 3, 4, ..." tail. + try testing.expectEqual(@as(u8, 2), f.extra_len); + try testing.expectEqual(@as(u32, 3), f.extra_lines[0]); + try testing.expectEqual(@as(u32, 4), f.extra_lines[1]); +} + +test "check: line numbers point at the offending record" { + const data = + \\#!srfv1 + \\symbol::A,shares:num:1 + \\symbol::B,shares:num:2 + \\symbol::C,shares:num:3,bogus::x + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + const f = findingFor(r, "bogus") orelse return error.MissingFinding; + try testing.expectEqual(@as(u32, 4), f.first_line); +} + +test "check: non-SRF input reports nothing rather than a wall of unknowns" { + var r = try lintLot("this is not an srf file at all\n"); + defer r.deinit(); + try testing.expect(r.isClean()); +} + +test "check: empty file is clean" { + var r = try lintLot("#!srfv1\n"); + defer r.deinit(); + try testing.expect(r.isClean()); +} + +// ── Tagged unions ───────────────────────────────────────────── + +test "shapeOfSchema: tagged union dispatches on the tag value" { + const shape = shapeOfSchema(test_union_schema); + try testing.expectEqualStrings("type", shape.tagged.tag_key); + try testing.expectEqual(@as(usize, 2), shape.tagged.variants.len); + + const data = + \\#!srfv1 + \\type::config,horizon:num:30 + \\type::birthdate,date::1980-01-01,person:num:1 + \\ + ; + var r = try check(testing.allocator, data, shape); + defer r.deinit(); + try testing.expect(r.isClean()); +} + +test "shapeOfSchema: tag key is valid whether or not the variant redeclares it" { + // `config` redeclares `type`; `birthdate` does not. Both must + // accept `type::` without reporting it as unknown. + const shape = shapeOfSchema(test_union_schema); + for (shape.tagged.variants) |v| { + try testing.expect(indexOfName(v.names, "type") != null); + } + // And it appears exactly once, not twice, for the redeclaring one. + const cfg = shape.tagged.variantFor("config").?; + var type_count: usize = 0; + for (cfg.names) |n| { + if (std.mem.eql(u8, n, "type")) type_count += 1; + } + try testing.expectEqual(@as(usize, 1), type_count); +} + +test "check: typo inside a union variant is caught against that variant" { + const data = + \\#!srfv1 + \\type::config,horizonn:num:30 + \\ + ; + var r = try check(testing.allocator, data, shapeOfSchema(test_union_schema)); + defer r.deinit(); + const f = findingFor(r, "horizonn") orelse return error.MissingFinding; + try testing.expectEqual(Kind.near_miss, f.kind); + try testing.expectEqualStrings("horizon", f.suggestion.?); +} + +test "check: a field valid on another variant is not valid on this one" { + const data = + \\#!srfv1 + \\type::config,person:num:2 + \\ + ; + var r = try check(testing.allocator, data, shapeOfSchema(test_union_schema)); + defer r.deinit(); + const f = findingFor(r, "person") orelse return error.MissingFinding; + try testing.expectEqual(Kind.unknown, f.kind); +} + +test "check: unknown tag value is left to the typed parser" { + const data = + \\#!srfv1 + \\type::nonsense,whatever::x + \\ + ; + var r = try check(testing.allocator, data, shapeOfSchema(test_union_schema)); + defer r.deinit(); + try testing.expect(r.isClean()); +} + +// ── Derivation ──────────────────────────────────────────────── + +test "namesOf: derives the full field set" { + const names = namesOf(TestLot); + try testing.expectEqual(@as(usize, 7), names.len); + try testing.expect(indexOfName(names, "symbol") != null); + try testing.expect(indexOfName(names, "split_factor") != null); + try testing.expect(indexOfName(names, "nope") == null); +} + +test "scopesOfSchema: default_value is derived from the struct default" { + const shape = shapeOfSchema(test_lot_schema); + try testing.expectEqualStrings("stock", shape.flat.scopes.?.default_value); + try testing.expectEqualStrings("security_type", shape.flat.scopes.?.discriminator); +} + +test "prefixMatch: picks the closest-length candidate, not the first" { + // `Lot`'s real shape: a short field that prefixes two longer ones. + const names: []const []const u8 = &.{ "price", "price_date", "price_ratio" }; + try testing.expectEqualStrings("price_date", prefixMatch(names, "price_dat").?); + try testing.expectEqualStrings("price_date", prefixMatch(names, "price_datee").?); + try testing.expectEqualStrings("price_ratio", prefixMatch(names, "price_rati").?); + try testing.expectEqualStrings("price", prefixMatch(names, "pricee").?); + // Below the floor, no guess at all. + try testing.expectEqual(@as(?[]const u8, null), prefixMatch(names, "pr")); +} + +test "check: findings are capped so a garbage file cannot flood the report" { + var aw: std.Io.Writer.Allocating = .init(testing.allocator); + defer aw.deinit(); + try aw.writer.writeAll("#!srfv1\n"); + // Each record carries a DISTINCT bogus key, so rollup cannot + // collapse them and the cap is what bounds the output. + for (0..max_findings + 50) |i| { + try aw.writer.print("shares:num:1,zz{d}::x\n", .{i}); + } + var r = try lintLot(aw.writer.buffered()); + defer r.deinit(); + try testing.expectEqual(@as(usize, max_findings), r.findings.len); + try testing.expect(r.truncated); +} + +test "Sink.addOwned detail is freed with the result" { + // Exercises the arena-ownership contract: `describeOnly` + // allocates, and `std.testing.allocator` fails the test if + // `Result.deinit` does not release it. + const data = + \\#!srfv1 + \\symbol::VTI,shares:num:100,rate:num:1,strike:num:2 + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + try testing.expectEqual(@as(usize, 2), r.findings.len); +} + +// ── describe / writeValidNames ──────────────────────────────── + +fn describeOne(data: []const u8, buf: []u8) ![]const u8 { + var r = try lintLot(data); + defer r.deinit(); + if (r.findings.len == 0) return error.NoFinding; + // `describe` writes into `buf`, which outlives `r`, but `key` + // borrows from `data` - so this only holds while `data` is alive. + return r.findings[0].describe(buf); +} + +test "describe: unknown field" { + var buf: [Finding.describe_max]u8 = undefined; + const msg = try describeOne("#!srfv1\nshares:num:1,cost_basis:num:5\n", &buf); + try testing.expectEqualStrings("unrecognized field 'cost_basis'", msg); +} + +test "describe: near miss names the field it meant" { + var buf: [Finding.describe_max]u8 = undefined; + const msg = try describeOne("#!srfv1\nshares:num:1,accoun::X\n", &buf); + try testing.expectEqualStrings("unrecognized field 'accoun' - did you mean 'account'?", msg); +} + +test "describe: case mismatch explains why it silently did nothing" { + var buf: [Finding.describe_max]u8 = undefined; + const msg = try describeOne("#!srfv1\nshares:num:1,Account::X\n", &buf); + try testing.expect(std.mem.indexOf(u8, msg, "differs only by case from 'account'") != null); + try testing.expect(std.mem.indexOf(u8, msg, "case-sensitive") != null); +} + +test "describe: duplicate explains that the first wins" { + var buf: [Finding.describe_max]u8 = undefined; + const msg = try describeOne("#!srfv1\nshares:num:1,shares:num:2\n", &buf); + try testing.expect(std.mem.indexOf(u8, msg, "appears twice") != null); + try testing.expect(std.mem.indexOf(u8, msg, "keeps the first") != null); +} + +test "describe: inapplicable names the discriminator and the types that read it" { + var buf: [Finding.describe_max]u8 = undefined; + const msg = try describeOne("#!srfv1\nshares:num:1,rate:num:5\n", &buf); + try testing.expectEqualStrings( + "field 'rate' ignored for security_type stock; read only for cd", + msg, + ); +} + +test "describe: derived field says to remove it" { + var buf: [Finding.describe_max]u8 = undefined; + const msg = try describeOne("#!srfv1\nshares:num:1,split_factor:num:4\n", &buf); + try testing.expectEqualStrings("field 'split_factor' is derived by zfin; remove it from your file", msg); +} + +test "describe: rolled-up repeat reports the count and the first lines" { + const data = + \\#!srfv1 + \\shares:num:1,accoun::X + \\shares:num:2,accoun::X + \\shares:num:3,accoun::X + \\shares:num:4,accoun::X + \\ + ; + var buf: [Finding.describe_max]u8 = undefined; + const msg = try describeOne(data, &buf); + try testing.expect(std.mem.indexOf(u8, msg, "(4 records: lines 2, 3, 4, ...)") != null); +} + +test "describe: exactly three occurrences omits the ellipsis" { + const data = + \\#!srfv1 + \\shares:num:1,accoun::X + \\shares:num:2,accoun::X + \\shares:num:3,accoun::X + \\ + ; + var buf: [Finding.describe_max]u8 = undefined; + const msg = try describeOne(data, &buf); + try testing.expect(std.mem.indexOf(u8, msg, "(3 records: lines 2, 3, 4)") != null); + try testing.expect(std.mem.indexOf(u8, msg, "...") == null); +} + +test "describe: a short buffer truncates rather than failing" { + var r = try lintLot("#!srfv1\nshares:num:1,cost_basis:num:5\n"); + defer r.deinit(); + var tiny: [8]u8 = undefined; + const msg = r.findings[0].describe(&tiny); + try testing.expect(msg.len <= tiny.len); + try testing.expectEqualStrings("unrecogn", msg); +} + +test "writeValidNames: flat shape lists every field, wrapped" { + var aw: std.Io.Writer.Allocating = .init(testing.allocator); + defer aw.deinit(); + try writeValidNames(&aw.writer, shapeOfSchema(test_lot_schema), 4, 40); + const out = aw.written(); + // Every field present, including the derived one - the list is + // "what SRF will match", not "what you should write". + for (namesOf(TestLot)) |n| { + try testing.expect(std.mem.indexOf(u8, out, n) != null); + } + // Wrapped: more than one line, none wildly over the limit. + var it = std.mem.splitScalar(u8, std.mem.trimEnd(u8, out, "\n"), '\n'); + var lines: usize = 0; + while (it.next()) |line| { + lines += 1; + try testing.expect(line.len <= 44); + } + try testing.expect(lines > 1); +} + +test "writeValidNames: tagged shape lists each variant separately" { + var aw: std.Io.Writer.Allocating = .init(testing.allocator); + defer aw.deinit(); + try writeValidNames(&aw.writer, shapeOfSchema(test_union_schema), 2, 100); + const out = aw.written(); + try testing.expect(std.mem.indexOf(u8, out, "type::config") != null); + try testing.expect(std.mem.indexOf(u8, out, "type::birthdate") != null); + try testing.expect(std.mem.indexOf(u8, out, "horizon") != null); + try testing.expect(std.mem.indexOf(u8, out, "person") != null); +} + +// ── checkSemantic / Result.sort ──────────────────────────────── + +/// A schema whose `semanticCheck` reports a model-specific rule, used to +/// exercise the typed second pass without depending on any real model's +/// current rule set. +const SemRecord = struct { + name: []const u8 = "", + pct: f64 = 0, +}; + +/// Fixed `today` for tests, so a date rule's verdict cannot change as +/// the calendar moves. +const test_ctx: Context = .{ .today = Date.fromYmd(2026, 1, 1) }; + +const sem_schema = struct { + pub const Record = SemRecord; + pub const file_label: []const u8 = "sem.srf"; + pub const doc_path: []const u8 = "reference/config/sem-srf.md"; + + pub fn semanticCheck(rec: Record, ctx: Context, sink: *Sink) !void { + _ = ctx; + if (rec.pct > 100) { + try sink.addOwned(.semantic, "pct", try std.fmt.allocPrint( + sink.allocator, + "'{s}': pct must be <= 100 (got {d})", + .{ rec.name, rec.pct }, + )); + } + } +}; + +test "checkSemantic: model-owned rules reach the findings list" { + const data = + \\#!srfv1 + \\name::ok,pct:num:50 + \\name::bad,pct:num:150 + \\ + ; + var r = try check(testing.allocator, data, shapeOfSchema(sem_schema)); + defer r.deinit(); + // Field names are all valid, so the raw pass finds nothing. + try testing.expect(r.isClean()); + + try checkSemantic(sem_schema, &r, data, test_ctx); + try testing.expectEqual(@as(usize, 1), r.findings.len); + try testing.expectEqual(Kind.semantic, r.findings[0].kind); + try testing.expectEqual(@as(u32, 3), r.findings[0].first_line); + + var buf: [Finding.describe_max]u8 = undefined; + try testing.expectEqualStrings("'bad': pct must be <= 100 (got 150)", r.findings[0].describe(&buf)); +} + +test "checkSemantic: a schema without the hook is a no-op" { + const data = "#!srfv1\nsymbol::VTI,shares:num:1,account::Sample Brokerage\n"; + var r = try check(testing.allocator, data, shapeOfSchema(test_lot_schema)); + defer r.deinit(); + try checkSemantic(test_lot_schema, &r, data, test_ctx); + try testing.expect(r.isClean()); +} + +test "checkSemantic: records that fail to coerce are skipped, not reported twice" { + // `pct` is a string where a number is declared; under + // `user_edited` coercion that is tolerated, so the record still + // reaches `semanticCheck`. A record missing a required field would + // be skipped - the typed parser's own diagnostics already cover it. + const data = + \\#!srfv1 + \\name::bad,pct::150 + \\ + ; + var r = try check(testing.allocator, data, shapeOfSchema(sem_schema)); + defer r.deinit(); + try checkSemantic(sem_schema, &r, data, test_ctx); + try testing.expectEqual(@as(usize, 1), r.findings.len); +} + +test "Result.sort: orders by line so the two passes interleave correctly" { + // Raw-pass finding on line 4, semantic-pass findings on 2 and 3 - + // the order they are produced in is not the order to read them in. + const data = + \\#!srfv1 + \\name::a,pct:num:150 + \\name::b,pct:num:200 + \\name::c,pct:num:1,bogus::x + \\ + ; + var r = try check(testing.allocator, data, shapeOfSchema(sem_schema)); + defer r.deinit(); + try checkSemantic(sem_schema, &r, data, test_ctx); + try testing.expectEqual(@as(usize, 3), r.findings.len); + + // Unsorted, the raw finding comes first. + try testing.expectEqual(@as(u32, 4), r.findings[0].first_line); + + r.sort(); + try testing.expectEqual(@as(u32, 2), r.findings[0].first_line); + try testing.expectEqual(@as(u32, 3), r.findings[1].first_line); + try testing.expectEqual(@as(u32, 4), r.findings[2].first_line); +} + +test "rollup: semantic findings on different records stay separate" { + // Regression guard. Identity used to be `(kind, key)` alone, which + // merged these two into one line naming only 'a' - silently hiding + // that 'b' was broken too. Both records trip the same field with a + // different detail, so both must survive. + const data = + \\#!srfv1 + \\name::a,pct:num:150 + \\name::b,pct:num:200 + \\ + ; + var r = try check(testing.allocator, data, shapeOfSchema(sem_schema)); + defer r.deinit(); + try checkSemantic(sem_schema, &r, data, test_ctx); + try testing.expectEqual(@as(usize, 2), r.findings.len); + + var buf: [Finding.describe_max]u8 = undefined; + r.sort(); + try testing.expect(std.mem.indexOf(u8, r.findings[0].describe(&buf), "'a'") != null); + try testing.expect(std.mem.indexOf(u8, r.findings[1].describe(&buf), "'b'") != null); +} + +test "rollup: identical generic findings still collapse to one line" { + // The other half of the same property: a static per-kind detail + // means the same typo across many records is ONE finding, so a + // copy-pasted mistake does not produce a wall of output. + const data = + \\#!srfv1 + \\shares:num:1,accoun::X + \\shares:num:2,accoun::X + \\shares:num:3,accoun::X + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + try testing.expectEqual(@as(usize, 1), r.findings.len); + try testing.expectEqual(@as(u32, 3), r.findings[0].count); +} + +test "check: a record wider than the buffer is still name-checked" { + // The `max_fields_per_record` guard. Applicability needs the whole + // record buffered (the discriminator may come last), so an + // over-wide record stops contributing `inapplicable` findings - but + // it must still get its names checked, and must not crash. + var aw: std.Io.Writer.Allocating = .init(testing.allocator); + defer aw.deinit(); + try aw.writer.writeAll("#!srfv1\nshares:num:1"); + for (0..max_fields_per_record + 10) |i| { + try aw.writer.print(",zz{d}::x", .{i}); + } + try aw.writer.writeAll("\n"); + + var r = try lintLot(aw.writer.buffered()); + defer r.deinit(); + // Bounded by the buffer, so not every bogus key is reported - but + // the ones that fit are, and nothing panicked. + try testing.expect(r.findings.len > 0); + try testing.expect(r.findings.len <= max_fields_per_record); +} + +test "Result.sort: two findings on one line are ordered by key" { + // The tiebreaker. Without it the order of same-line findings + // depends on field declaration order, which makes report diffs + // noisy for no reason. + const data = + \\#!srfv1 + \\shares:num:1,zebra::x,alpha::y + \\ + ; + var r = try lintLot(data); + defer r.deinit(); + try testing.expectEqual(@as(usize, 2), r.findings.len); + r.sort(); + try testing.expectEqualStrings("alpha", r.findings[0].key); + try testing.expectEqualStrings("zebra", r.findings[1].key); +} + +// ── Context ─────────────────────────────────────────────────── + +const DatedRecord = struct { + name: []const u8 = "", + on: ?Date = null, +}; + +/// Flags any `on` date after `ctx.today`. Exercises the one thing the +/// real date rules depend on: that `checkSemantic` delivers the caller's +/// `today`, not some other clock. +const dated_schema = struct { + pub const Record = DatedRecord; + pub const file_label: []const u8 = "dated.srf"; + pub const doc_path: []const u8 = "reference/config/dated-srf.md"; + + pub fn semanticCheck(rec: Record, ctx: Context, sink: *Sink) !void { + const on = rec.on orelse return; + if (ctx.today.lessThan(on)) try sink.addStatic(.semantic, "on", "in the future"); + } +}; + +test "checkSemantic: ctx.today reaches the hook" { + const data = + \\#!srfv1 + \\name::a,on::2026-06-01 + \\ + ; + // Same record, two different `today`s, opposite verdicts - so the + // hook must be reading the context rather than a clock of its own. + { + var r = try check(testing.allocator, data, shapeOfSchema(dated_schema)); + defer r.deinit(); + try checkSemantic(dated_schema, &r, data, .{ .today = Date.fromYmd(2026, 1, 1) }); + try testing.expectEqual(@as(usize, 1), r.findings.len); + } + { + var r = try check(testing.allocator, data, shapeOfSchema(dated_schema)); + defer r.deinit(); + try checkSemantic(dated_schema, &r, data, .{ .today = Date.fromYmd(2027, 1, 1) }); + try testing.expect(r.isClean()); + } +} + +test "Result.hasNameFindings: only name problems warrant the valid-field list" { + { + var r = try lintLot("#!srfv1\nshares:num:1,accoun::X\n"); + defer r.deinit(); + try testing.expect(r.hasNameFindings()); + } + { + var r = try lintLot("#!srfv1\nshares:num:1,Account::X\n"); + defer r.deinit(); + try testing.expect(r.hasNameFindings()); + } + { + // Real fields, wrong use - the list would tell the user nothing. + var r = try lintLot("#!srfv1\nshares:num:1,shares:num:2,rate:num:1,split_factor:num:2\n"); + defer r.deinit(); + try testing.expect(!r.isClean()); + try testing.expect(!r.hasNameFindings()); + } +} diff --git a/src/srf_opts.zig b/src/srf_opts.zig index 2e24060..010281f 100644 --- a/src/srf_opts.zig +++ b/src/srf_opts.zig @@ -18,6 +18,17 @@ const srf = @import("srf"); /// `projections.srf`, `imported_values.srf`, `acknowledgments.srf`, and /// the keybind config. /// +/// **`grep -rn srf_opts.user_edited src/` is the authoritative index of +/// hand-edited SRF files, and `src/srf_lint.zig`'s `schemas` registry +/// must cover the same set.** A new parse site here needs a matching +/// `srf_schema` on its model and an entry in that registry, or the +/// file's field names go unchecked - SRF silently discards a key that +/// names no field of the target struct, so a typo in an unregistered +/// file is invisible. (The one deliberate exception is +/// `imported_values.srf`, which is machine-generated; see the registry's +/// doc comment.) Zig cannot enforce this - there is no way to ask which +/// files reference a constant - so it is a convention, hence this note. +/// /// Accepts a string where a number was declared, because a hand-typed /// `close_price::200.00` instead of `close_price:num:200.00` is a /// slip, not a different intent. SRF's own doc says as much: strict diff --git a/src/tui/keybinds.zig b/src/tui/keybinds.zig index 3c90e74..e79a127 100644 --- a/src/tui/keybinds.zig +++ b/src/tui/keybinds.zig @@ -427,6 +427,13 @@ const RawRecord = struct { scope: ?[]const u8 = null, }; +/// Schema contract for `keys.srf`. See `srf_lint.validateSchema`. +pub const srf_schema = struct { + pub const Record = RawRecord; + pub const file_label: []const u8 = "keys.srf"; + pub const doc_path: []const u8 = "reference/config/keys-srf.md"; +}; + /// Load keybindings from an SRF file. Returns null if the file doesn't exist /// or can't be parsed. On success, the caller owns the returned KeyMap and /// must call deinit(). diff --git a/src/tui/theme.zig b/src/tui/theme.zig index ad923c6..152c3ba 100644 --- a/src/tui/theme.zig +++ b/src/tui/theme.zig @@ -224,6 +224,19 @@ const field_names = [_]struct { name: []const u8, offset: usize }{ .{ .name = "bar_fill", .offset = @offsetOf(Theme, "bar_fill") }, }; +/// Schema contract for `theme.srf`. See `srf_lint.validateSchema`. +/// +/// `Record = Theme` rather than a parallel name list: `loadFromData` +/// matches keys against the hand-maintained `field_names` table above, +/// but the authoritative set is `std.meta.fields(Theme)`, and deriving +/// from the struct means the lint cannot disagree with the type even if +/// `field_names` falls behind it. +pub const srf_schema = struct { + pub const Record = Theme; + pub const file_label: []const u8 = "theme.srf"; + pub const doc_path: []const u8 = "reference/config/theme-srf.md"; +}; + fn colorPtr(theme: *Theme, offset: usize) *Color { const bytes: [*]u8 = @ptrCast(theme); return @ptrCast(@alignCast(bytes + offset));