additional snapshot documentation

docs/README.md

@@ -79,6 +79,7 @@ Background and the "why" behind zfin's behavior.
   - [Cache implementation](dev/caching-implementation.md) -- contributor-level internals: layout, freshness model, fetch flowcharts
 - [Why multiple data providers](explanation/data-providers.md)
 - [Returns and performance](explanation/returns-and-performance.md)
+- [The snapshot model](explanation/snapshots-model.md) -- what a snapshot records, what it deliberately leaves out, and why
 - [The retirement projection model](explanation/projections-model.md)
 - [FAQ and troubleshooting](explanation/faq-troubleshooting.md)
 

docs/explanation/snapshots-model.md

@@ -0,0 +1,216 @@
+# The snapshot model
+
+A snapshot is the file [`zfin snapshot`](../reference/cli/snapshot.md)
+writes to `history/<date>-portfolio.srf`, and the thing every
+history- and comparison-oriented command reads back. This page explains
+what a snapshot is *for*, which is also an explanation of what it
+deliberately leaves out. For the day-to-day workflow, see
+[Snapshots and history](../guides/snapshots-and-history.md).
+
+## A snapshot is a valuation record
+
+A snapshot answers exactly one question:
+
+> What was this portfolio worth on date D, decomposed by symbol,
+> account, and tax type?
+
+That is the whole contract. A snapshot is **not** a faithful record of
+your lots. It records enough per-lot detail to *decompose a valuation*,
+and stops there.
+
+This is a deliberate choice rather than an oversight, and the rest of
+this page is the reasoning -- partly so the decision is auditable, and
+partly so the same field-by-field extension proposals don't have to be
+re-litigated every time someone notices something missing.
+
+## Two layers, and which one wins
+
+A snapshot file has two layers that are computed independently, and it
+matters which you read:
+
+1. **The totals layer** -- the `total`, `tax_type`, and `account` rows.
+   This is the **authoritative** valuation. It comes from
+   `valuation.portfolioSummary`, the same code path that produces the
+   headline numbers in `zfin portfolio`, and it includes
+   portfolio-level adjustments.
+2. **The lot layer** -- the `lot` rows. This is a **decomposition
+   aid**. It comes from a separate, direct walk over the portfolio's
+   lots, and it deliberately carries no portfolio-level adjustments.
+
+When the two disagree, **the totals layer wins**. See "Known
+shortcoming" below for the one case where they measurably do.
+
+## Why not a faithful lot record?
+
+Because zfin already has one, and it is better at the job.
+
+The git history of your `portfolio.srf` *is* the faithful lot record.
+It has complete fidelity (every field of every lot, not a projection of
+some of them), and it has true temporal resolution (one revision per
+edit, rather than one file per day).
+[`zfin contributions`](../reference/cli/contributions.md) is built
+directly on it: it walks git revisions, deserializes complete lots, and
+matches them across revisions by a composite key.
+
+Making snapshots into a *second* faithful lot record would create two
+records of lot identity with different cadences, different
+completeness, and different provenance -- and therefore an open-ended
+obligation to reconcile them whenever they disagree. Nothing currently
+asks for that.
+
+There is also a harder lesson embedded in the contributions code. Even
+with the *complete* lot available, matching lots across time is
+unreliable enough that it needs a fuzzy fallback key alongside the
+strict one, because ordinary bookkeeping breaks strict identity:
+certificate-of-deposit auto-renewals rewrite the open date, account
+renames change the account, and reconciliation tweaks move the open
+price. Cross-time lot matching is a genuinely hard problem that the
+git-revision pipeline solves with real machinery. A daily valuation
+file is not going to solve it as a side effect of carrying one more
+column.
+
+## Proposals that were considered and rejected
+
+If you are here because a field you want is missing, check this list
+first.
+
+### `open_date`
+
+The strongest-looking candidate, because without it a snapshot's lots
+have no temporal identity at all, so they cannot be matched to lots in
+any other snapshot.
+
+Rejected, because it doesn't actually deliver that. `open_date` is one
+component of the strict lot key; a snapshot carrying it would still
+lack everything the fuzzy fallback needs when the strict key breaks --
+which, per above, is routine. The result would *look* like it enabled
+cross-snapshot lot matching while quietly not doing so, which is worse
+than the honest absence. If cross-snapshot lot keying is genuinely
+needed, the answer is to extend the git-revision pipeline.
+
+There is one non-identity argument for `open_date`: holding-period
+analysis, such as decomposing unrealized gains into short- and
+long-term at a past date. `cost_basis` is already emitted, so
+`open_date` is the only missing input. This is a legitimately
+valuation-shaped use, and it is the argument to make if the field is
+ever wanted -- but it is currently hypothetical. zfin has no
+holding-period logic anywhere, and tax-loss harvesting is deliberately
+hand-declared in `accounts.srf` rather than computed from lots. Adding
+a field for a consumer that does not exist is how formats rot.
+
+### `split_factor`
+
+A stock lot's `shares` is written raw (as transacted) while its `value`
+is split-adjusted, so the two are not related by the obvious
+`shares * price` identity. It is tempting to emit the split factor to
+close that gap.
+
+Rejected, because it carries no information. All three quantities are
+already recoverable by algebra from what *is* emitted:
+
+```
+raw shares       == cost_basis / open_price
+effective shares == value / price
+split_factor     == value / (shares * price)
+```
+
+`compare.aggregateSnapshotStocks` uses the second of these and
+documents it. Emitting the split factor as well would introduce a
+*second* route to effective shares within a single record: identical
+whenever the writer is correct, divergent exactly when it is buggy, and
+with no rule for which one a reader should trust. That is the same
+two-sources-of-truth problem as the lot-record proposal, in miniature.
+
+The "it would let us validate the invariant" argument does not survive
+either: a check comparing the writer's output against the writer's own
+inputs, assigned in the same function moments earlier, cannot detect a
+wrong split factor, a wrong price, or a wrong share count. It can only
+catch a typo in a single expression, which is a unit test's job.
+
+### `drip`
+
+Rejected as a category error. `zfin contributions` classifies dividend
+reinvestment by reading the `drip` flag from git revisions of
+`portfolio.srf`; it never reads snapshots at all. Adding `drip` here
+would not affect that classification, or anything else.
+
+## Known shortcoming: the covered-call gap
+
+`sum(stock lot.value)` can slightly **exceed** `total::liquid` in the
+same file.
+
+The cause is the two-layer split. When you hold an open, in-the-money
+sold call, the totals layer caps the covered underlying's market value
+at the option's strike price -- the shares are effectively committed at
+that price, so valuing them at the higher market price would overstate
+the portfolio. The lot layer applies no such cap; each lot is marked at
+plain market value.
+
+Consequences, in ascending order of obscurity:
+
+- `total::liquid` is correct and remains the figure to read.
+- Summing lot values yourself will overstate the liquid total whenever
+  such a call was open on the snapshot date. The overstatement is
+  bounded by `(market - strike) * covered_shares` across affected
+  underlyings.
+- `history.aggregateSnapshotAllocations` derives per-symbol weights by
+  dividing summed lot values by the totals-layer liquid figure, so
+  those weights can sum to slightly more than 1 in the same
+  circumstance.
+
+This is documented rather than fixed because it is immaterial in
+practice: it requires an open ITM sold call on the snapshot date, and
+the affected consumer is an allocation weighting whose downstream use
+tolerates the error. Note also that it is **not diagnosable from a
+snapshot alone** -- deciding whether a given gap is a legitimate
+covered-call cap or a writer bug requires the option's `strike`, which
+the format does not emit. Emitting `strike` and `multiplier` is
+therefore the one field addition with a concrete, valuation-shaped
+consumer, should this ever need fixing properly.
+
+## Wire-format compatibility
+
+The format is [SRF](https://git.lerch.org/lobo/srf), and its
+compatibility behavior is what makes the format safe to extend:
+
+- **Default-valued fields are elided on write.** A field equal to its
+  default does not appear in the file at all.
+- **Unknown fields are ignored on read.** A reader skips fields it
+  doesn't know about.
+- Together these give compatibility in both directions: old readers
+  tolerate new files, and new readers tolerate old files.
+
+Two constraints follow for anyone extending a record type:
+
+- **Every field must have a default.** Fields are matched by name and
+  absent ones are filled from their default; a field with no default
+  makes every previously-written snapshot fail to parse.
+- **`kind` must stay first.** It is the union discriminator, and the
+  reader requires it as the first field on the line.
+
+### `snapshot_version` and ambiguous absence
+
+Each snapshot carries a `snapshot_version`, currently `1`. Nothing
+reads it to gate behavior, so bumping it is normally documentary.
+
+There is one case where it becomes load-bearing. Because default-valued
+fields are elided, a new field whose value happens to equal its default
+is absent from the file -- and a reader cannot tell that apart from "the
+writer predates this field entirely." **Bump `snapshot_version` when,
+and only when, a new field's absence is semantically ambiguous.**
+Novelty alone is not a reason.
+
+Worked examples: a `drip: bool = false` or an `open_date: ?Date = null`
+would each require a bump, because absence could mean either "false /
+unknown" or "old writer." A `split_factor: f64 = 1.0` would not, since
+`1.0` means "no adjustment" either way.
+
+### Writers and readers are separate deployments
+
+Snapshots are often written by a scheduled job and read by interactive
+commands, and those two can be running different builds of zfin for
+weeks at a time. So a newly added field appears only in files written
+after the *writer's* build is refreshed, and never appears in files
+already on disk. `zfin snapshot` never reads existing snapshots, so an
+older writer is safe; just don't assume a field's presence based on the
+version of the binary you happen to be reading with.

docs/guides/snapshots-and-history.md

@@ -19,8 +19,12 @@ zfin doesn't track your value automatically -- it reads what you have
 (or week), and the history/compare commands read those snapshots back.
 
 Snapshots live in `<portfolio-dir>/history/<date>-portfolio.srf`. Each
-is an immutable record of totals, per-account values, and lot-level
+is an immutable **valuation** record for one date: the portfolio's
-state for one date.
+totals, its per-account and per-tax-type values, and a per-lot
+breakdown of where that value sat. It records what things were *worth*,
+not the full detail of your lots -- see
+[The snapshot model](../explanation/snapshots-model.md) for what is and
+isn't captured, and why.
 
 ## 1. Write a snapshot
 

docs/reference/cli/snapshot.md

@@ -25,6 +25,33 @@ The file is a discriminated SRF whose records start with
 If the target file already exists and `--force` isn't passed, the run
 skips with a stderr message.
 
+## What gets written
+
+Each snapshot is a **valuation** record for one date, in two layers:
+
+| Record kind          | Contents                                                     |
+|----------------------|--------------------------------------------------------------|
+| `meta`               | As-of date, capture time, zfin version, quote-date span, stale-quote count. |
+| `total`              | `net_worth`, `liquid`, `illiquid`. **Authoritative** totals. |
+| `tax_type`           | Value per tax treatment (Traditional, Roth, Taxable, HSA).    |
+| `account`            | Value per account.                                           |
+| `lot`                | Per-lot decomposition: symbol, account, security type, shares, open price, cost basis, price, value, quote date/staleness. |
+
+Two things to know before reading `lot` rows:
+
+- The `total` rows are authoritative. Lot rows are a decomposition aid
+  and do not carry portfolio-level adjustments, so summing them is not
+  guaranteed to reproduce `total::liquid` exactly.
+- What `shares`, `open_price`, `cost_basis`, and `value` *mean* differs
+  by security type, and so do the relationships between them. In
+  particular, a stock lot's `shares` is the raw as-transacted count
+  while its `value` is split-adjusted, so `shares * price` is not
+  generally `value`.
+
+A snapshot deliberately omits most of a lot's detail -- it is not a
+lot-level record. [The snapshot model](../../explanation/snapshots-model.md)
+covers the full field semantics, what is left out, and why.
+
 ## Examples
 
 ```bash

src/history.zig

@@ -596,10 +596,31 @@ pub const SnapshotAllocations = struct {
 
 /// Aggregate a snapshot's `LotRow`s into per-symbol `Allocation`s.
 ///
-/// Matches the lot aggregation that `valuation.portfolioSummary` does
+/// Sums `value` per `symbol` and computes each weight against the
-/// for live portfolios: sum `value` per `symbol`, compute weight
+/// snapshot's `liquid` total. Non-stock lots contribute to
-/// against the snapshot's `liquid` total. Non-stock lots contribute
+/// `cash_value` / `cd_value` instead of the allocation list.
-/// to `cash_value` / `cd_value` instead of the allocation list.
+///
+/// KNOWN SHORTCOMING - weights can sum to slightly OVER 1. This does
+/// NOT exactly mirror the live-portfolio path in
+/// `valuation.portfolioSummary`, and the divergence is structural, not
+/// a rounding artifact. A snapshot has two layers (see the doc block in
+/// `models/snapshot.zig`): the `liquid` total came from
+/// `portfolioSummary`, which calls `adjustForCoveredCalls` to cap an
+/// underlying's market value at the strike for open in-the-money sold
+/// calls, whereas the per-lot `value` rows this function sums were
+/// written by a separate direct walk that applies no such cap. So when
+/// an ITM sold call was open at capture, the numerator here is larger
+/// than the denominator's basis and `sum(weight) > 1`.
+///
+/// Left as-is deliberately. The error is bounded by
+/// `(market - strike) * covered_shares` on the affected underlyings
+/// only, and is immaterial in the scenarios this feeds
+/// (`deriveAllocationSplit` plus the per-position trailing-returns
+/// loop). Note that it also cannot be detected from the snapshot alone:
+/// diagnosing it needs `strike`, which the format does not emit. Fixing
+/// it properly means deciding which layer is authoritative for per-lot
+/// marks, not renormalizing here - `total::liquid` is the authoritative
+/// valuation figure.
 ///
 /// Security-type strings come from `LotType.label()` in the snapshot
 /// writer - "Stock", "Cash", "CD", "Option", "Illiquid". Match is

src/models/snapshot.zig

@@ -14,6 +14,108 @@
 //! them in `commands/` would force analytics to depend on a command
 //! module, which would be backwards.
 //!
+//! ## What a snapshot IS: a valuation record, in two layers
+//!
+//! A snapshot answers exactly one question: "what was this portfolio
+//! worth on date D, decomposed by symbol / account / tax-type." It is
+//! deliberately NOT a faithful lot record. Two layers, and the
+//! distinction is load-bearing:
+//!
+//!   - `total` / `tax_type` / `account` rows are the AUTHORITATIVE
+//!     valuation layer. They come from `valuation.portfolioSummary`,
+//!     which applies portfolio-level adjustments.
+//!   - `lot` rows are a DECOMPOSITION AID. They are per-lot marks
+//!     produced by a separate direct walk of `portfolio.lots`, and they
+//!     deliberately do NOT carry portfolio-level adjustments.
+//!
+//! Consequence, and the one cross-layer gotcha: `sum(stock lot.value)`
+//! can EXCEED `total::liquid`. `portfolioSummary` calls
+//! `adjustForCoveredCalls` (`analytics/valuation.zig`), capping an
+//! underlying's market value at the strike for open in-the-money sold
+//! calls; `buildSnapshot`'s per-lot loop does not. When the two
+//! disagree, `total::liquid` wins. The gap is not recoverable from the
+//! file, because `strike` is not emitted (see the omissions list).
+//!
+//! If you ever need to key lots ACROSS snapshots, this is the wrong
+//! format and the answer is not to extend it. zfin already has a
+//! faithful lot record with full temporal identity: the git history of
+//! `portfolio.srf`, which `commands/contributions.zig` reads revision
+//! by revision, deserializing complete 22-field `Lot`s and keying them
+//! with `lotKey` + `secondaryKey`. Extend that pipeline instead. Full
+//! rationale (including why `open_date` and `split_factor` were each
+//! proposed and rejected) in `docs/explanation/snapshots-model.md`.
+//!
+//! ## Deliberately omitted `Lot` fields
+//!
+//! `Lot` has 22 fields. `LotRow` carries 5 of them straight through
+//! (`symbol`, `shares`, `open_price`, `account`, `security_type`),
+//! FOLDS 5 more into computed values, and DROPS the remaining 12
+//! outright. Grouped by why, so the next person who wants one back
+//! knows which argument they have to beat:
+//!
+//! DROPPED (12):
+//!
+//!   Lot identity / provenance - belongs to the git-revision pipeline,
+//!   not here: `open_date`, `close_date`, `close_price`, `drip`,
+//!   `note`, `label`.
+//!
+//!   Hand-edit inputs consumed only by `zfin import` (see
+//!   `Lot.hand_edited_fields`, which has zero influence on this
+//!   format): `price_date`, `rate`, `maturity_date`.
+//!
+//!   No valuation consumer: `strike`, `underlying`, `option_type`.
+//!
+//! FOLDED IN (5) - not lost, but already accounted for in an emitted
+//! value, so re-emitting them would create a second source of truth:
+//!
+//!     `ticker`       -> `symbol`, via `Lot.priceSymbol()`
+//!     `price`        -> `price`, via `buildFallbackPrices` (the manual
+//!                      override becomes the mark when no quote exists)
+//!     `price_ratio`  -> `price`, via `Lot.effectivePrice()`
+//!     `split_factor` -> `value`, via `Lot.effectiveShares()`
+//!     `multiplier`   -> option `value` / `cost_basis`
+//!
+//!   Each is recoverable by algebra when genuinely needed;
+//!   `compare.aggregateSnapshotStocks` documents the sanctioned
+//!   `value / price` recovery for split-adjusted shares.
+//!
+//! NAMING HAZARD: `Lot.price` and `LotRow.price` are different things
+//! that share a name. `Lot.price` is the user's hand-entered manual
+//! override INPUT; `LotRow.price` is the resulting per-share market
+//! price actually used for the mark.
+//!
+//! ## Per-security_type field semantics
+//!
+//! `LotRow` fields mean different things per `security_type`, and the
+//! invariants that hold are NOT uniform. Absence of this table is how
+//! that became a surprise. `buildSnapshot` is the authority:
+//!
+//!   Stock:
+//!     shares      raw, as-transacted (NOT split-adjusted)
+//!     open_price  raw
+//!     cost_basis  shares * open_price (split-invariant, so raw is fine)
+//!     price       share-class-adjusted market price
+//!     value       shares * split_factor * price  (split-ADJUSTED)
+//!     => `shares * price != value` on any split-affected lot. Do not
+//!        assume that identity. Effective shares are `value / price`.
+//!
+//!   Cash / CD / Illiquid:
+//!     shares      the face / dollar value
+//!     open_price  0, meaningless
+//!     cost_basis  0, meaningless
+//!     price       null (no per-share price exists)
+//!     value       == shares
+//!
+//!   Option:
+//!     shares      contract count, signed (negative = short)
+//!     open_price  premium per share
+//!     cost_basis  |shares| * open_price * multiplier
+//!     price       null
+//!     value       == cost_basis. Options are carried AT COST and are
+//!                 never marked to market in a snapshot.
+//!
+//!   Watch: not emitted at all - watchlist lots aren't positions.
+//!
 //! IMPORTANT: `kind` uses `= ""` as the default - a sentinel that never
 //! matches any real discriminator value. This satisfies two constraints
 //! simultaneously:
@@ -30,6 +132,46 @@
 //!
 //! Optional fields default to `null` so they're elided on null values -
 //! that's the behavior we want for `price`, `quote_date`, etc.
+//!
+//! ## Adding a field (read this first)
+//!
+//! srf's compatibility rules, verified against the pinned srf:
+//!   - Every field MUST have a default. srf coerces by NAME, filling
+//!     absent fields from their default; a field with no default raises
+//!     `FieldNotFoundOnFieldWithoutDefaultValue`, which would break
+//!     every previously-written snapshot on read.
+//!   - `kind` MUST stay at declaration index 0. Declaration order is
+//!     on-disk order, and the `SnapshotRecord` union dispatch in
+//!     `history.zig` reads exactly one field and requires it to be the
+//!     tag, else `ActiveTagNotFirstField`.
+//!   - Unknown fields are silently skipped on read and default-valued
+//!     fields are elided on write, so old readers tolerate new files
+//!     and new readers tolerate old files. Both directions hold.
+//!   - Prefer APPENDING. Nothing compares snapshots byte-wise (the
+//!     render tests are `std.mem.indexOf` substring assertions, not
+//!     golden files), but several of those substrings pin the field
+//!     immediately following `kind` - e.g. `"kind::lot,symbol::VTI"`.
+//!     Appending is safe; inserting right after `kind` is not.
+//!
+//! `snapshot_version` is currently written as 1 and never read to gate
+//! behavior, so bumping it is documentary - EXCEPT in one case that
+//! makes it load-bearing. Because srf elides default-valued fields, a
+//! new field whose value equals its default is absent from the file,
+//! and a reader cannot distinguish "the writer predates this field"
+//! from "the value is the default." Bump `snapshot_version` when, and
+//! only when, a new field's ABSENCE is semantically ambiguous. Novelty
+//! alone is not a reason. (Worked example: `drip: bool = false` and
+//! `open_date: ?Date = null` would both have required a bump; a
+//! `split_factor: f64 = 1.0` would not, since 1.0 means "no
+//! adjustment" whether elided by a new writer or missing from an old
+//! file.)
+//!
+//! Note also that snapshot writes and snapshot reads are separate
+//! deployments in practice: a scheduled job may run an older zfin than
+//! the interactive commands that read what it wrote. `zfin snapshot`
+//! never reads existing snapshots, so the write side is old-binary-
+//! safe, but do not assume a new field appears in files written before
+//! that job's build was refreshed.
 
 const std = @import("std");
 const Date = @import("../Date.zig");
@@ -63,12 +205,21 @@ pub const AccountRow = struct {
     value: f64,
 };
 
+/// One open lot's valuation on the snapshot date. See the
+/// "Per-security_type field semantics" block at the top of this file
+/// before reading any of `shares` / `open_price` / `cost_basis` /
+/// `value` - what they mean, and which invariants relate them, differ
+/// by `security_type`.
 pub const LotRow = struct {
     kind: []const u8 = "",
     symbol: []const u8,
     lot_symbol: []const u8,
     account: []const u8,
     security_type: []const u8,
+    /// RAW, as-transacted share count for stocks - NOT split-adjusted.
+    /// Face/dollar value for cash/CD/illiquid; signed contract count
+    /// for options. Effective (split-adjusted) shares are `value /
+    /// price`, never this field; see `compare.aggregateSnapshotStocks`.
     shares: f64,
     open_price: f64,
     cost_basis: f64,