more flexible tui async plan

.mise.toml

@@ -2,4 +2,4 @@
 prek = "0.4.1"
 zig = "0.16.0"
 zls = "0.16.0"
-"ubi:DonIsaac/zlint" = "0.7.9"
+"ubi:DonIsaac/zlint" = "0.8.1"

.pre-commit-config.yaml

@@ -32,7 +32,7 @@ repos:
       - id: test
         name: Run zig build test
         entry: zig
-        args: ["build", "coverage", "-Dcoverage-threshold=74"]
+        args: ["build", "coverage", "-Dcoverage-threshold=75"]
         language: system
         types: [file]
         pass_filenames: false

TODO.md

@@ -5,144 +5,6 @@ ordered roughly by priority within each section. Priority labels
 (`HIGH` / `MEDIUM` / `LOW`) mark items that deserve explicit
 ranking; unlabeled items are "someday, if the mood strikes."
 
-## Review tab: cursor + symbol selection + drill-down — priority MEDIUM
-
-The review tab is currently a static table — you can sort it but
-not select a row. Common workflow: see something interesting on
-the review tab (e.g. NKE has the worst trailing returns), want to
-jump straight into the per-symbol detail tabs (performance, quote,
-options) for it. Today that takes either:
-
-- typing `/<symbol>` to set the active symbol manually, or
-- switching to portfolio tab, finding the row, pressing `s` /
-  space / left-click on it, then switching back.
-
-Both are small but noticeable papercuts when scanning the review
-table.
-
-### What to add
-
-Mirror the portfolio tab's pattern (`src/tui/portfolio_tab.zig`):
-
-- `State.cursor: usize` — selected row index.
-- `j`/`k` and arrow keys move the cursor (already routed by the
-  framework's `onCursorMove` hook; opt in by declaring it on the
-  tab).
-- Mouse wheel scrolls (already handled by App when the tab opts
-  out via `handleMouse` returning false on wheel events).
-- Mouse click on a data row sets the cursor to that row.
-- Cursor row gets `selectStyle` highlight in the row's
-  `StyleSpan` set so it visibly stands out.
-- Active-symbol indicator: rows whose `symbol` matches `app.symbol`
-  get an asterisk or similar marker (matches portfolio tab's
-  star convention).
-- Press `s` (or space, or Enter) to set `app.symbol` to the
-  cursor row's symbol — same `select_symbol` action portfolio tab
-  binds. The framework validator already prevents tab-local
-  bindings from colliding with global keys, so reusing `s`/space
-  is fine because portfolio tab does it too.
-
-### Drill-down navigation
-
-A natural extension once selection works: a hotkey that both
-selects the symbol AND switches to a per-symbol tab (performance
-is the obvious target since that's the deep-dive surface). Maybe
-`Enter` with a row selected → set symbol + jump to performance
-tab. Compare: portfolio tab's Enter is "expand/collapse"; review
-rows don't expand, so Enter is free.
-
-### Tests
-
-- Cursor moves on j/k/arrows, clamps at edges.
-- Click on row N sets cursor to N.
-- `select_symbol` action sets `app.symbol` and triggers a
-  `loadTabData()` if a downstream tab is active.
-- Active-symbol asterisk renders for the row matching `app.symbol`.
-
-## TUI: share candle/dividend maps across tabs — priority MEDIUM
-
-`App.ensurePortfolioDataLoaded` builds a complete per-symbol
-`candle_map` via `buildPortfolioData`, uses it once to compute
-historical-snapshot values, then **frees it** at
-`src/tui.zig:1404-1406`. Every per-position TUI tab that
-subsequently needs candles (`review`, `performance`, parts of
-`portfolio` and `quote`) re-reads them from the SRF cache via
-`getCachedCandles` — ~27 redundant reads per tab activation on
-a 27-symbol portfolio, each running its own SRF iterator pass.
-
-Most visible in debug builds (~2s tab activation); release
-mode is sub-second but still measurable on first switch.
-
-### Fix sketch
-
-Promote `candle_map` and add a `dividend_map` to fields on
-`App.portfolio.PortfolioData`. Tabs read from
-`app.portfolio.candle_map` / `app.portfolio.dividend_map`
-instead of re-fetching. Lifetime tied to the existing
-`summary` ownership: cleared atomically on portfolio reload,
-freed once in `PortfolioData.deinit`.
-
-The review tab's `State.dividend_map` field is removed —
-that data lives on App now.
-
-### Loading strategy options
-
-**Eager.** Populate both maps inside
-`ensurePortfolioDataLoaded` so they're ready when the first
-per-symbol tab activates. Pros: tab switches are always
-instant. Cons: pays the dividend-cache read cost (~27 SRF
-reads) at TUI startup even if the user never opens a tab
-that needs them.
-
-**Lazy.** App exposes `ensureDividendMap()` (parallel to
-`ensureAccountMap`); first tab to need dividends pays the
-load. Pros: no startup cost for users who don't open
-review/performance/etc. Cons: first review-tab activation
-still slow.
-
-**Async (recommended middle ground).** On TUI startup, spawn
-a background task using Zig 0.16.0's `Io` async to populate
-both maps. Tabs read through a synchronization wrapper —
-something like:
-
-```zig
-pub const PortfolioCache = struct {
-    candle_map: ?std.StringHashMap([]const Candle) = null,
-    dividend_map: ?std.StringHashMap([]const Dividend) = null,
-    ready: std.Thread.Semaphore,  // or io.Async equivalent
-
-    pub fn waitReady(self: *PortfolioCache) void { ... }
-};
-```
-
-Tabs that need the maps call `waitReady()` (cheap when
-already loaded; brief block on first call if the background
-task hasn't finished). Uses Zig 0.16's `io` async layer so
-we don't manage thread lifecycle directly. Pros: zero
-perceived startup cost AND zero perceived tab-switch cost
-for typical workflows. Cons: more design work; need to
-handle the failure case (background task errored — fall
-back to lazy load).
-
-### Other call sites to audit
-
-- `src/tui/portfolio_tab.zig:1756` — re-reads cached candles
-  to compute the latest-quote-date footer. Re-route through
-  the shared map.
-- `src/tui/quote_tab.zig` and `src/tui/performance_tab.zig` —
-  these are intentionally per-symbol-narrow (single ticker
-  detail views), so they probably stay on `getTrailingReturns`
-  but could short-circuit when the candles are already in
-  the shared map.
-
-### Tests
-
-- Lifetime test: portfolio reload clears both maps before
-  the next load assigns new ones; no use-after-free.
-- Async path: failure in the background task surfaces as a
-  visible status message but doesn't break tab activation
-  (lazy fallback still works).
-
 ## Projections: future enhancements
 
 - **Configurable return cap per position — priority MEDIUM.**

src/analytics/analysis.zig

@@ -242,9 +242,13 @@ pub const AnalysisResult = struct {
     /// before aggregation. The right field for portfolio-level
     /// debt-to-equity analysis.
     asset_category: []BreakdownItem,
-    /// Breakdown by asset class (US Large Cap, Bonds, Cash & CDs, etc.)
-    asset_class: []BreakdownItem,
-    /// Breakdown by sector (Technology, Healthcare, etc.) -- equities only
+    /// Breakdown by sector bucket (Technology, US Healthcare ETF,
+    /// US Large Cap, etc.). Aggregates by `entry.bucket` —
+    /// pre-filled by parseClassificationFile via `deriveBucket`,
+    /// or curated by the user. Replaces the historical separate
+    /// "Asset Class" + "Sector" breakdowns: the bucket is a
+    /// single semantically-meaningful label that combines what
+    /// each was trying to express.
     sector: []BreakdownItem,
     /// Breakdown by geographic region (US, International, etc.)
     geo: []BreakdownItem,
@@ -259,7 +263,6 @@ pub const AnalysisResult = struct {
 
     pub fn deinit(self: *AnalysisResult, allocator: std.mem.Allocator) void {
         allocator.free(self.asset_category);
-        allocator.free(self.asset_class);
         allocator.free(self.sector);
         allocator.free(self.geo);
         allocator.free(self.account);
@@ -285,11 +288,10 @@ pub const Section = struct {
 /// adding/reordering a section is a one-place edit. Order is
 /// from coarsest (Asset Category, 4 buckets) to finest
 /// (per-account / per-tax-type).
-pub fn breakdownSections(r: *const AnalysisResult) [6]Section {
+pub fn breakdownSections(r: *const AnalysisResult) [5]Section {
     return .{
         .{ .items = r.asset_category, .title = "Asset Category" },
-        .{ .items = r.asset_class, .title = "Asset Class" },
-        .{ .items = r.sector, .title = "Sector (Equities)" },
+        .{ .items = r.sector, .title = "Sector" },
         .{ .items = r.geo, .title = "Geographic" },
         .{ .items = r.account, .title = "By Account" },
         .{ .items = r.tax_type, .title = "By Tax Type" },
@@ -432,6 +434,9 @@ pub fn bucketSector(sector: []const u8) []const u8 {
     // Plain-English asset-class words (hand-written metadata).
     if (std.mem.eql(u8, sector, "Bonds")) return bucket_fixed_income;
     if (std.mem.eql(u8, sector, "Cash")) return bucket_cash;
+    if (std.mem.eql(u8, sector, "Cash & CDs")) return bucket_cash;
+    if (std.mem.eql(u8, sector, "Options")) return bucket_other;
+    if (std.mem.eql(u8, sector, "Unclassified")) return bucket_other;
     // "Diversified" means "broad equity fund holding all
     // sectors" — S&P 500 ETF, total-market index, etc.
     if (std.mem.eql(u8, sector, "Diversified")) return bucket_equity;
@@ -456,42 +461,69 @@ pub fn bucketSector(sector: []const u8) []const u8 {
     };
     for (gics) |g| if (std.mem.eql(u8, sector, g)) return bucket_equity;
 
-    // Everything else: derivatives, real property, sentinels
-    // (TODO/Unknown/empty), unrecognized future labels.
-    return bucket_other;
+    // Strings containing `/` are NPORT-P shapes that didn't match
+    // any prefix above (e.g. "Direct Real Property / Other",
+    // "Direct Credit Risk / Other", "Other / Corporate"). Bucket
+    // these as Other — they're real-property, credit derivatives,
+    // and miscellaneous categories that don't fit the equity /
+    // fixed-income / cash trichotomy.
+    if (std.mem.indexOfScalar(u8, sector, '/') != null) return bucket_other;
+
+    // Empty string / explicit sentinels → Other. Explicit
+    // because the curated-bucket fallback below would otherwise
+    // assume any non-empty unknown string is equity.
+    if (sector.len == 0) return bucket_other;
+    if (std.mem.eql(u8, sector, "TODO")) return bucket_other;
+    if (std.mem.eql(u8, sector, "Unknown")) return bucket_other;
+
+    // Word-content checks for composite bucket strings produced by
+    // `deriveBucket` (or hand-curated `bucket::` overrides):
+    //   "US Bonds", future "International Bonds" / "EM Bonds" → Fixed Income
+    //   "US Cash", "Cash & CDs" (handled above) → Cash
+    if (std.mem.endsWith(u8, sector, " Bonds") or std.mem.endsWith(u8, sector, " bonds")) {
+        return bucket_fixed_income;
+    }
+    if (std.mem.endsWith(u8, sector, " Cash") or std.mem.endsWith(u8, sector, " cash")) {
+        return bucket_cash;
+    }
+
+    // Default for any remaining no-`/` non-cruft string: equity.
+    // Catches curated buckets like "US Large Cap", "US Mid Cap",
+    // "US Small Cap", "US Dividend Equity", "US Healthcare ETF",
+    // "International Developed", "Emerging Markets", and any
+    // future user-defined bucket. The convention is: composite
+    // buckets describe an equity sleeve unless they explicitly
+    // say otherwise (Bonds/Cash/Options/Unclassified handled
+    // above).
+    return bucket_equity;
 }
 
 // ── Sector display granularity ───────────────────────────────
 
-/// Granularity tier for the Sector breakdown display. Lets the
-/// user toggle between coarse bucket-level summary and the raw
-/// fine-grained NPORT-P decomposition.
+/// Granularity tier for the Sector breakdown display. Two
+/// tiers: `coarse` (4 macro buckets — Equity / Fixed Income /
+/// Cash / Other) and `fine` (the raw bucket strings the
+/// classification layer produced — every "US Large Cap" / "US
+/// Bonds" / GICS-sector / etc. row distinct).
+///
+/// History: this used to be a three-tier enum (coarse / mid /
+/// fine). The middle tier collapsed NPORT-P sub-flavors (all
+/// Debt / * → "Bonds", all Asset-Backed / * → "Bonds", etc.)
+/// while keeping GICS sectors distinct. After the bucket
+/// commit, classification rows expose a single curated bucket
+/// label per entry — so the NPORT-P-flavor collapse the mid
+/// tier did is now done at parse time. Mid and fine ended up
+/// nearly identical and mid was dropped.
 pub const Granularity = enum {
     /// Four buckets: Equity / Fixed Income / Cash / Other.
     /// Same labels as the Asset Category breakdown.
     coarse,
-    /// ~12-16 buckets: collapses NPORT-P sub-flavors but keeps
-    /// GICS sectors distinct. Default for most users — answers
-    /// "how exposed am I to bonds vs stocks vs cash, and which
-    /// sectors within stocks?".
-    mid,
-    /// Raw NPORT-P strings: every Debt / X variant, every
-    /// Asset-Backed / Y variant, every Derivative / Z variant
-    /// is its own row. Useful for spotting fine-grained
-    /// concentrations within the bond sleeve (e.g. "am I overweight
-    /// US Treasury vs Municipal?").
+    /// One row per distinct bucket label — the raw shape of
+    /// what `entry.bucket` produces. Default. This is what
+    /// the user wants for "what are my actual positions?"
     fine,
 };
 
-// Mid-granularity bucket labels. Static literals so they can be
-// used as stable HashMap keys without duping.
-pub const mid_bonds: []const u8 = "Bonds";
-pub const mid_equity_corporate: []const u8 = "Equity / Corporate";
-pub const mid_equity_preferred: []const u8 = "Equity Preferred";
-pub const mid_cash_equivalents: []const u8 = "Cash & Equivalents";
-pub const mid_derivatives: []const u8 = "Derivatives";
-pub const mid_other: []const u8 = "Other";
-
 /// Display-friendly abbreviations for sector labels that don't fit
 /// cleanly in narrow columns. Returns the input unchanged when no
 /// abbreviation is registered for it; consumers that need a fixed
@@ -507,75 +539,21 @@ pub fn abbreviateSector(s: []const u8) []const u8 {
 }
 
 /// Map a sector string through the chosen granularity. Returns
-/// a static literal (or, at fine granularity, the input slice
-/// itself) suitable for use as a stable HashMap key.
+/// a static literal (at coarse) or the input slice (at fine)
+/// suitable for use as a stable HashMap key.
 ///
 /// Granularity tiers:
 ///
 ///   - **coarse**: delegates to `bucketSector` — Equity / Fixed Income
 ///     / Cash / Other (4 buckets).
-///   - **mid**: collapses NPORT-P sub-flavors (all Debt / * → Bonds,
-///     all Asset-Backed / * → Bonds, all STIV / * → Cash & Equivalents,
-///     all Derivative / * → Derivatives) while keeping GICS sectors
-///     distinct (Technology, Healthcare, etc.).
 ///   - **fine**: passthrough — returns the input unchanged.
 pub fn collapseSector(sector: []const u8, granularity: Granularity) []const u8 {
     return switch (granularity) {
         .fine => sector,
         .coarse => bucketSector(sector),
-        .mid => midBucket(sector),
     };
 }
 
-/// Mid-granularity bucket lookup. Pure data, no allocation.
-fn midBucket(sector: []const u8) []const u8 {
-    // Bond-shaped (NPORT-P): all Debt / *, Loan / *, Asset-Backed / *.
-    if (std.mem.startsWith(u8, sector, "Debt")) return mid_bonds;
-    if (std.mem.startsWith(u8, sector, "Loan")) return mid_bonds;
-    if (std.mem.startsWith(u8, sector, "Asset-Backed")) return mid_bonds;
-    // Plain-English bond label from legacy hand-written metadata.
-    if (std.mem.eql(u8, sector, "Bonds")) return mid_bonds;
-
-    // Cash-shaped (NPORT-P): STIV variants + repurchase agreement.
-    if (std.mem.startsWith(u8, sector, "Short-Term Investment Vehicle")) return mid_cash_equivalents;
-    if (std.mem.startsWith(u8, sector, "Repurchase Agreement")) return mid_cash_equivalents;
-    if (std.mem.eql(u8, sector, "Cash")) return mid_cash_equivalents;
-    if (std.mem.eql(u8, sector, "Cash & CDs")) return mid_cash_equivalents;
-
-    // Equity Preferred is distinct at mid because it's a hybrid.
-    if (std.mem.startsWith(u8, sector, "Equity Preferred")) return mid_equity_preferred;
-
-    // Generic Equity / * collapses to a single Equity / Corporate
-    // bucket. Specifically "Equity / Corporate" / "Equity / Other"
-    // / "Equity / Registered Fund" all merge here.
-    if (std.mem.startsWith(u8, sector, "Equity")) return mid_equity_corporate;
-
-    // Derivatives.
-    if (std.mem.startsWith(u8, sector, "Derivative")) return mid_derivatives;
-
-    // GICS sector names pass through unchanged. Same exact-match
-    // list as `bucketSector`.
-    const gics = [_][]const u8{
-        "Technology",
-        "Healthcare",
-        "Financial Services",
-        "Financials",
-        "Consumer Cyclical",
-        "Consumer Defensive",
-        "Energy",
-        "Utilities",
-        "Real Estate",
-        "Industrials",
-        "Basic Materials",
-        "Communication Services",
-        "Diversified",
-    };
-    for (gics) |g| if (std.mem.eql(u8, sector, g)) return sector;
-
-    // Direct Real Property, Direct Credit Risk, sentinels — Other.
-    return mid_other;
-}
-
 /// Compute portfolio analysis from allocations and classification metadata.
 /// `allocations` are the stock/ETF positions with market values.
 /// `classifications` is the metadata file data.
@@ -595,9 +573,17 @@ pub fn analyzePortfolio(
     account_map: ?AccountMap,
     as_of: Date,
 ) !AnalysisResult {
-    // Accumulators: label -> dollar amount
-    var ac_map = std.StringHashMap(f64).init(allocator);
-    defer ac_map.deinit();
+    // Accumulators: label -> dollar amount.
+    //
+    // sector_map and asset_cat_map are both keyed by the
+    // `bucket` field on ClassificationEntry (pre-filled by
+    // parseClassificationFile via deriveBucket). Buckets are
+    // either user-curated, GICS-like sectors, or composite
+    // "{geo} {asset_class}" labels — meaningful units for
+    // concentration rollup. The raw `entry.sector` is no
+    // longer used for either map: NPORT-P fund-decomp
+    // categories ("Equity / Corporate") would lump genuinely
+    // different funds together.
     var sector_map = std.StringHashMap(f64).init(allocator);
     defer sector_map.deinit();
     // 4-bucket coarse breakdown (Equity/Fixed Income/Cash/Other).
@@ -614,7 +600,7 @@ pub fn analyzePortfolio(
     var unclassified_list = std.ArrayList([]const u8).empty;
     errdefer unclassified_list.deinit(allocator);
 
-    // Process each equity allocation (for asset class, sector, geo, unclassified)
+    // Process each equity allocation (for sector, geo, unclassified)
     for (allocations) |alloc| {
         const mv = alloc.market_value;
         if (mv <= 0) continue;
@@ -630,24 +616,34 @@ pub fn analyzePortfolio(
                 const frac = entry.pct / 100.0;
                 const portion = mv * frac;
 
-                if (entry.asset_class) |ac| {
-                    const prev = ac_map.get(ac) orelse 0;
-                    try ac_map.put(ac, prev + portion);
+                // Sector breakdown: roll up by bucket (the
+                // pre-filled deriveBucket result on the entry).
+                if (entry.bucket) |b| {
+                    const prev = sector_map.get(b) orelse 0;
+                    try sector_map.put(b, prev + portion);
                 }
-                // Asset-category bucket: prefer `sector` (richer
-                // signal). Fall back to `asset_class` for legacy
-                // hand-written entries that didn't include a
-                // sector. Counted exactly once per entry.
+                // Asset Category 4-bucket coarse breakdown
+                // (Equity / Fixed Income / Cash / Other) keeps
+                // using the raw `entry.sector` as input. Reasons:
+                //   1. `bucketSector` recognizes the NPORT-P
+                //      prefixes ("Equity / *", "Debt / *", etc.)
+                //      directly. The user-facing Sector breakdown
+                //      bucket might be "US ETF" (a composite that
+                //      doesn't carry the asset-type signal),
+                //      but the underlying sector still does.
+                //   2. The Asset Category breakdown is the
+                //      coarse "what's exposed to equity drawdowns?"
+                //      view — invariant to the user's bucket
+                //      curation, since it's a fundamental property
+                //      of the holding.
                 if (entry.sector) |s| {
-                    const prev = sector_map.get(s) orelse 0;
-                    try sector_map.put(s, prev + portion);
-                    const bucket = bucketSector(s);
-                    const bprev = asset_cat_map.get(bucket) orelse 0;
-                    try asset_cat_map.put(bucket, bprev + portion);
+                    const cat = bucketSector(s);
+                    const cprev = asset_cat_map.get(cat) orelse 0;
+                    try asset_cat_map.put(cat, cprev + portion);
                 } else if (entry.asset_class) |ac| {
-                    const bucket = bucketAssetClass(ac);
-                    const bprev = asset_cat_map.get(bucket) orelse 0;
-                    try asset_cat_map.put(bucket, bprev + portion);
+                    const cat = bucketAssetClass(ac);
+                    const cprev = asset_cat_map.get(cat) orelse 0;
+                    try asset_cat_map.put(cat, cprev + portion);
                 }
                 if (entry.geo) |g| {
                     const prev = geo_map.get(g) orelse 0;
@@ -698,26 +694,34 @@ pub fn analyzePortfolio(
         try acct_map.put(acct, prev + value);
     }
 
-    // Add non-stock asset classes (combine Cash + CDs)
+    // Add non-stock holdings (cash, CDs, options) into the
+    // coarse asset_category breakdown. They have no entry in
+    // the classification map (it's keyed by ticker), so we
+    // route them to coarse buckets directly.
     const cash_total = portfolio.totalCash(as_of);
     const cd_total = portfolio.totalCdFaceValue(as_of);
     const cash_cd_total = cash_total + cd_total;
     if (cash_cd_total > 0) {
-        const prev = ac_map.get("Cash & CDs") orelse 0;
-        try ac_map.put("Cash & CDs", prev + cash_cd_total);
         const gprev = geo_map.get("US") orelse 0;
         try geo_map.put("US", gprev + cash_cd_total);
         // Literal cash and CDs roll into the coarse Cash bucket.
         const bprev = asset_cat_map.get(bucket_cash) orelse 0;
         try asset_cat_map.put(bucket_cash, bprev + cash_cd_total);
+        // Also surface in the Sector breakdown as "Cash & CDs"
+        // so users with significant cash positions see the
+        // line. Without this, the Sector breakdown would
+        // silently omit cash entirely.
+        const sprev = sector_map.get("Cash & CDs") orelse 0;
+        try sector_map.put("Cash & CDs", sprev + cash_cd_total);
     }
     const opt_total = portfolio.totalOptionCost(as_of);
     if (opt_total > 0) {
-        const prev = ac_map.get("Options") orelse 0;
-        try ac_map.put("Options", prev + opt_total);
         // Options are derivatives; coarse bucket is Other.
         const bprev = asset_cat_map.get(bucket_other) orelse 0;
         try asset_cat_map.put(bucket_other, bprev + opt_total);
+        // Surface in Sector breakdown too.
+        const sprev = sector_map.get("Options") orelse 0;
+        try sector_map.put("Options", sprev + opt_total);
     }
 
     // Tax type breakdown: map each account's total to its tax type
@@ -735,7 +739,6 @@ pub fn analyzePortfolio(
 
     return .{
         .asset_category = try mapToSortedBreakdown(allocator, asset_cat_map, total),
-        .asset_class = try mapToSortedBreakdown(allocator, ac_map, total),
         .sector = try mapToSortedBreakdown(allocator, sector_map, total),
         .geo = try mapToSortedBreakdown(allocator, geo_map, total),
         .account = try mapToSortedBreakdown(allocator, acct_map, total),
@@ -1338,12 +1341,63 @@ test "bucketSector: GICS sector names → Equity" {
     }
 }
 
-test "bucketSector: sentinels and unrecognized → Other" {
+test "bucketSector: sentinels stay Other" {
     try std.testing.expectEqualStrings(bucket_other, bucketSector("TODO"));
     try std.testing.expectEqualStrings(bucket_other, bucketSector("Unknown"));
     try std.testing.expectEqualStrings(bucket_other, bucketSector(""));
-    try std.testing.expectEqualStrings(bucket_other, bucketSector("Fintech"));
-    try std.testing.expectEqualStrings(bucket_other, bucketSector("Some Future Label"));
+    try std.testing.expectEqualStrings(bucket_other, bucketSector("Unclassified"));
+}
+
+test "bucketSector: curated-bucket-shaped unknown strings default to Equity" {
+    // After the bucket commit, `bucketSector` is called with
+    // either NPORT-P-shaped strings, GICS sector names, or
+    // composite/curated bucket labels (from `deriveBucket` or
+    // user-curated `bucket::` overrides). For composite-shaped
+    // strings that don't match any explicit Bonds/Cash/Options
+    // pattern, the default is Equity — composite buckets
+    // describe equity sleeves unless they say otherwise. This
+    // is the right default because:
+    //   1. The user's primary use of the Asset Category
+    //      breakdown is "what fraction is exposed to equity
+    //      drawdowns?" — a curated bucket like "US Large Cap"
+    //      definitely IS equity.
+    //   2. The cost of the wrong default is asymmetric: a real
+    //      bond bucket mis-bucketed as Equity will show in the
+    //      4-bucket coarse breakdown as overweight equity (very
+    //      visible bug). A real equity bucket mis-bucketed as
+    //      Other will silently disappear from the
+    //      stocks/bonds/cash header (very subtle bug).
+    try std.testing.expectEqualStrings(bucket_equity, bucketSector("Fintech"));
+    try std.testing.expectEqualStrings(bucket_equity, bucketSector("Some Future Label"));
+    try std.testing.expectEqualStrings(bucket_equity, bucketSector("US Large Cap"));
+    try std.testing.expectEqualStrings(bucket_equity, bucketSector("US Mid Cap"));
+    try std.testing.expectEqualStrings(bucket_equity, bucketSector("US Small Cap"));
+    try std.testing.expectEqualStrings(bucket_equity, bucketSector("US Dividend Equity"));
+    try std.testing.expectEqualStrings(bucket_equity, bucketSector("US Healthcare ETF"));
+    try std.testing.expectEqualStrings(bucket_equity, bucketSector("International Developed"));
+    try std.testing.expectEqualStrings(bucket_equity, bucketSector("Emerging Markets"));
+}
+
+test "bucketSector: composite Bonds buckets → Fixed Income" {
+    try std.testing.expectEqualStrings(bucket_fixed_income, bucketSector("US Bonds"));
+    try std.testing.expectEqualStrings(bucket_fixed_income, bucketSector("International Bonds"));
+    try std.testing.expectEqualStrings(bucket_fixed_income, bucketSector("EM Bonds"));
+}
+
+test "bucketSector: composite Cash buckets → Cash" {
+    try std.testing.expectEqualStrings(bucket_cash, bucketSector("Cash & CDs"));
+}
+
+test "bucketSector: Options keyword → Other" {
+    try std.testing.expectEqualStrings(bucket_other, bucketSector("Options"));
+}
+
+test "bucketSector: NPORT-P fallthrough (slash without recognized prefix) → Other" {
+    // Strings containing `/` that didn't match any specific
+    // NPORT-P prefix branch are real-property / credit-risk /
+    // miscellaneous categories. Bucket as Other.
+    try std.testing.expectEqualStrings(bucket_other, bucketSector("Other / Corporate"));
+    try std.testing.expectEqualStrings(bucket_other, bucketSector("Direct Real Property / Other"));
 }
 
 test "bucketSector: returns same pointer for repeated calls (static-string property)" {
@@ -1408,114 +1462,8 @@ test "collapseSector .coarse: delegates to bucketSector" {
     try std.testing.expectEqualStrings(bucket_other, collapseSector("Derivative / Other", .coarse));
 }
 
-test "collapseSector .mid: all Debt / * collapse to Bonds" {
-    const cases = [_][]const u8{
-        "Debt / Corporate",
-        "Debt / US Treasury",
-        "Debt / Municipal",
-        "Debt / Non-US Sovereign",
-        "Debt / US Gov Agency",
-        "Debt / US GSE",
-    };
-    for (cases) |s| {
-        try std.testing.expectEqualStrings(mid_bonds, collapseSector(s, .mid));
-    }
-}
-
-test "collapseSector .mid: all Asset-Backed and Loan variants collapse to Bonds" {
-    try std.testing.expectEqualStrings(mid_bonds, collapseSector("Asset-Backed / Corporate Mortgage", .mid));
-    try std.testing.expectEqualStrings(mid_bonds, collapseSector("Asset-Backed CBO/CDO / Corporate", .mid));
-    try std.testing.expectEqualStrings(mid_bonds, collapseSector("Asset-Backed Other / Corporate", .mid));
-    try std.testing.expectEqualStrings(mid_bonds, collapseSector("Loan / Corporate", .mid));
-    try std.testing.expectEqualStrings(mid_bonds, collapseSector("Bonds", .mid));
-}
-
-test "collapseSector .mid: all STIV / Repurchase collapse to Cash & Equivalents" {
-    try std.testing.expectEqualStrings(mid_cash_equivalents, collapseSector("Short-Term Investment Vehicle / Corporate", .mid));
-    try std.testing.expectEqualStrings(mid_cash_equivalents, collapseSector("Short-Term Investment Vehicle / Registered Fund", .mid));
-    try std.testing.expectEqualStrings(mid_cash_equivalents, collapseSector("Short-Term Investment Vehicle / Private Fund", .mid));
-    try std.testing.expectEqualStrings(mid_cash_equivalents, collapseSector("Repurchase Agreement / Other", .mid));
-    try std.testing.expectEqualStrings(mid_cash_equivalents, collapseSector("Cash", .mid));
-    try std.testing.expectEqualStrings(mid_cash_equivalents, collapseSector("Cash & CDs", .mid));
-}
-
-test "collapseSector .mid: Equity Preferred is its own bucket" {
-    // Hybrid security — distinct from generic equity at mid.
-    try std.testing.expectEqualStrings(mid_equity_preferred, collapseSector("Equity Preferred / Corporate", .mid));
-}
-
-test "collapseSector .mid: Equity / * (non-Preferred) collapses to Equity / Corporate" {
-    try std.testing.expectEqualStrings(mid_equity_corporate, collapseSector("Equity / Corporate", .mid));
-    try std.testing.expectEqualStrings(mid_equity_corporate, collapseSector("Equity / Other", .mid));
-    try std.testing.expectEqualStrings(mid_equity_corporate, collapseSector("Equity / Registered Fund", .mid));
-}
-
-test "collapseSector .mid: GICS sectors stay distinct" {
-    // The whole point of mid: collapse NPORT-P sub-flavors but
-    // keep GICS sector breakdown so users see stock concentrations.
-    try std.testing.expectEqualStrings("Technology", collapseSector("Technology", .mid));
-    try std.testing.expectEqualStrings("Healthcare", collapseSector("Healthcare", .mid));
-    try std.testing.expectEqualStrings("Financial Services", collapseSector("Financial Services", .mid));
-    try std.testing.expectEqualStrings("Financials", collapseSector("Financials", .mid));
-    try std.testing.expectEqualStrings("Diversified", collapseSector("Diversified", .mid));
-    try std.testing.expectEqualStrings("Energy", collapseSector("Energy", .mid));
-}
-
-test "collapseSector .mid: Derivative variants collapse to Derivatives" {
-    try std.testing.expectEqualStrings(mid_derivatives, collapseSector("Derivative / Corporate", .mid));
-    try std.testing.expectEqualStrings(mid_derivatives, collapseSector("Derivative / Other", .mid));
-    try std.testing.expectEqualStrings(mid_derivatives, collapseSector("Derivative-FX / Other", .mid));
-    try std.testing.expectEqualStrings(mid_derivatives, collapseSector("Derivative-FX / Corporate", .mid));
-}
-
-test "collapseSector .mid: real property and unrecognized -> Other" {
-    try std.testing.expectEqualStrings(mid_other, collapseSector("Direct Real Property / Other", .mid));
-    try std.testing.expectEqualStrings(mid_other, collapseSector("Direct Credit Risk / Other", .mid));
-    try std.testing.expectEqualStrings(mid_other, collapseSector("TODO", .mid));
-    try std.testing.expectEqualStrings(mid_other, collapseSector("Unknown", .mid));
-    try std.testing.expectEqualStrings(mid_other, collapseSector("", .mid));
-    try std.testing.expectEqualStrings(mid_other, collapseSector("Some Future Label", .mid));
-}
-
-test "collapseSector .mid: returns same pointer for same bucket (static-string property)" {
-    // Stable HashMap keys without duping.
-    const a = collapseSector("Debt / Corporate", .mid);
-    const b = collapseSector("Debt / US Treasury", .mid);
-    try std.testing.expectEqual(@intFromPtr(a.ptr), @intFromPtr(b.ptr));
-    try std.testing.expectEqual(@intFromPtr(mid_bonds.ptr), @intFromPtr(a.ptr));
-}
-
 // ── collapseBreakdownAtGranularity ────────────────────────────
 
-test "collapseBreakdownAtGranularity: VBTLX-shape Debt sleeves collapse to Bonds at mid" {
-    // VBTLX has six different Debt / X rows. At mid granularity
-    // they should all sum into one Bonds row.
-    const allocator = std.testing.allocator;
-    const items = [_]BreakdownItem{
-        .{ .label = "Debt / Corporate", .weight = 0.40, .value = 40_000.0 },
-        .{ .label = "Debt / US Treasury", .weight = 0.30, .value = 30_000.0 },
-        .{ .label = "Debt / Non-US Sovereign", .weight = 0.10, .value = 10_000.0 },
-        .{ .label = "Debt / Municipal", .weight = 0.05, .value = 5_000.0 },
-        .{ .label = "Debt / US Gov Agency", .weight = 0.04, .value = 4_000.0 },
-        .{ .label = "Debt / US GSE", .weight = 0.01, .value = 1_000.0 },
-        .{ .label = "Short-Term Investment Vehicle / Registered Fund", .weight = 0.10, .value = 10_000.0 },
-    };
-    const result = try collapseBreakdownAtGranularity(allocator, &items, .mid, 100_000.0);
-    defer allocator.free(result);
-
-    // Two output buckets: Bonds (sum of all Debt/*) and
-    // Cash & Equivalents (the STIV row).
-    try std.testing.expectEqual(@as(usize, 2), result.len);
-    var bonds_value: f64 = 0;
-    var cash_value: f64 = 0;
-    for (result) |item| {
-        if (std.mem.eql(u8, item.label, mid_bonds)) bonds_value = item.value;
-        if (std.mem.eql(u8, item.label, mid_cash_equivalents)) cash_value = item.value;
-    }
-    try std.testing.expectApproxEqAbs(@as(f64, 90_000), bonds_value, 1.0);
-    try std.testing.expectApproxEqAbs(@as(f64, 10_000), cash_value, 1.0);
-}
-
 test "collapseBreakdownAtGranularity: coarse collapses everything to 4 buckets" {
     const allocator = std.testing.allocator;
     const items = [_]BreakdownItem{
@@ -1568,7 +1516,7 @@ test "collapseBreakdownAtGranularity: fine returns equivalent breakdown unchange
 test "collapseBreakdownAtGranularity: empty input -> empty output" {
     const allocator = std.testing.allocator;
     const items = [_]BreakdownItem{};
-    const result = try collapseBreakdownAtGranularity(allocator, &items, .mid, 100_000.0);
+    const result = try collapseBreakdownAtGranularity(allocator, &items, .fine, 100_000.0);
     defer allocator.free(result);
     try std.testing.expectEqual(@as(usize, 0), result.len);
 }
@@ -1584,7 +1532,7 @@ test "collapseBreakdownAtGranularity: total values preserved through collapse" {
         .{ .label = "Equity / Corporate", .weight = 0.20, .value = 20.0 },
         .{ .label = "Technology", .weight = 0.05, .value = 5.0 },
     };
-    const result = try collapseBreakdownAtGranularity(allocator, &items, .mid, 100.0);
+    const result = try collapseBreakdownAtGranularity(allocator, &items, .fine, 100.0);
     defer allocator.free(result);
 
     var total: f64 = 0;
@@ -1672,16 +1620,14 @@ test "bucketAssetClass: returns same pointer for same bucket (static-string prop
 
 // ── breakdownSections ─────────────────────────────────────────
 
-test "breakdownSections: returns 6 sections" {
+test "breakdownSections: returns 5 sections" {
     var ac_cat = [_]BreakdownItem{};
-    var ac = [_]BreakdownItem{};
     var sec = [_]BreakdownItem{};
     var geo = [_]BreakdownItem{};
     var acct = [_]BreakdownItem{};
     var tax = [_]BreakdownItem{};
     const result = AnalysisResult{
         .asset_category = &ac_cat,
-        .asset_class = &ac,
         .sector = &sec,
         .geo = &geo,
         .account = &acct,
@@ -1690,19 +1636,17 @@ test "breakdownSections: returns 6 sections" {
         .total_value = 0,
     };
     const sections = breakdownSections(&result);
-    try std.testing.expectEqual(@as(usize, 6), sections.len);
+    try std.testing.expectEqual(@as(usize, 5), sections.len);
 }
 
 test "breakdownSections: titles in expected order, no leading whitespace, unique" {
     var ac_cat = [_]BreakdownItem{};
-    var ac = [_]BreakdownItem{};
     var sec = [_]BreakdownItem{};
     var geo = [_]BreakdownItem{};
     var acct = [_]BreakdownItem{};
     var tax = [_]BreakdownItem{};
     const result = AnalysisResult{
         .asset_category = &ac_cat,
-        .asset_class = &ac,
         .sector = &sec,
         .geo = &geo,
         .account = &acct,
@@ -1714,8 +1658,7 @@ test "breakdownSections: titles in expected order, no leading whitespace, unique
 
     const expected = [_][]const u8{
         "Asset Category",
-        "Asset Class",
-        "Sector (Equities)",
+        "Sector",
         "Geographic",
         "By Account",
         "By Tax Type",
@@ -1743,16 +1686,14 @@ test "breakdownSections: items.ptr points to AnalysisResult fields" {
     var ac_cat = [_]BreakdownItem{
         .{ .label = "Equity", .weight = 1.0, .value = 100.0 },
     };
-    var ac = [_]BreakdownItem{
-        .{ .label = "US Large Cap", .weight = 0.5, .value = 50.0 },
+    var sec = [_]BreakdownItem{
+        .{ .label = "Technology", .weight = 0.5, .value = 50.0 },
     };
-    var sec = [_]BreakdownItem{};
     var geo = [_]BreakdownItem{};
     var acct = [_]BreakdownItem{};
     var tax = [_]BreakdownItem{};
     const result = AnalysisResult{
         .asset_category = &ac_cat,
-        .asset_class = &ac,
         .sector = &sec,
         .geo = &geo,
         .account = &acct,
@@ -1763,23 +1704,20 @@ test "breakdownSections: items.ptr points to AnalysisResult fields" {
     const sections = breakdownSections(&result);
 
     try std.testing.expectEqual(result.asset_category.ptr, sections[0].items.ptr);
-    try std.testing.expectEqual(result.asset_class.ptr, sections[1].items.ptr);
-    try std.testing.expectEqual(result.sector.ptr, sections[2].items.ptr);
-    try std.testing.expectEqual(result.geo.ptr, sections[3].items.ptr);
-    try std.testing.expectEqual(result.account.ptr, sections[4].items.ptr);
-    try std.testing.expectEqual(result.tax_type.ptr, sections[5].items.ptr);
+    try std.testing.expectEqual(result.sector.ptr, sections[1].items.ptr);
+    try std.testing.expectEqual(result.geo.ptr, sections[2].items.ptr);
+    try std.testing.expectEqual(result.account.ptr, sections[3].items.ptr);
+    try std.testing.expectEqual(result.tax_type.ptr, sections[4].items.ptr);
 }
 
 test "breakdownSections: Asset Category is first (coarse-to-fine ordering)" {
     var ac_cat = [_]BreakdownItem{};
-    var ac = [_]BreakdownItem{};
     var sec = [_]BreakdownItem{};
     var geo = [_]BreakdownItem{};
     var acct = [_]BreakdownItem{};
     var tax = [_]BreakdownItem{};
     const result = AnalysisResult{
         .asset_category = &ac_cat,
-        .asset_class = &ac,
         .sector = &sec,
         .geo = &geo,
         .account = &acct,

src/atomic.zig

@@ -46,7 +46,12 @@ pub fn writeFileAtomic(
         });
         errdefer {
             tmp_file.close(io);
-            std.Io.Dir.cwd().deleteFile(io, tmp_path) catch {};
+            // Best-effort cleanup of the temp file while unwinding
+            // the primary error; a failed delete leaves a stray
+            // .tmp that the next write overwrites.
+            std.Io.Dir.cwd().deleteFile(io, tmp_path) catch |err| {
+                std.log.debug("atomic write cleanup deleteFile({s}): {t}", .{ tmp_path, err });
+            };
         }
 
         try tmp_file.writeStreamingAll(io, bytes);
@@ -58,7 +63,11 @@ pub fn writeFileAtomic(
     }
 
     std.Io.Dir.cwd().rename(tmp_path, std.Io.Dir.cwd(), path, io) catch |err| {
-        std.Io.Dir.cwd().deleteFile(io, tmp_path) catch {};
+        // Same best-effort cleanup as above; the rename failure is
+        // the error the caller needs to see.
+        std.Io.Dir.cwd().deleteFile(io, tmp_path) catch |del_err| {
+            std.log.debug("atomic write cleanup deleteFile({s}): {t}", .{ tmp_path, del_err });
+        };
         return err;
     };
 }

src/cache/store.zig

@@ -315,8 +315,15 @@ pub const Store = struct {
 
     /// Read and deserialize cached data. With `.fresh_only`, returns null if stale.
     /// With `.any`, returns data regardless of freshness.
+    ///
+    /// `allocator` owns the returned `CacheResult.data`. It can be
+    /// the same as `self.allocator` (the historical default) or a
+    /// caller-supplied arena. Internal scratch (raw cache bytes,
+    /// SRF iterator state) still uses `self.allocator` because it
+    /// gets freed before this function returns.
     pub fn read(
         self: *Store,
+        allocator: std.mem.Allocator,
         comptime T: type,
         symbol: []const u8,
         comptime postProcess: ?*const fn (*T, std.mem.Allocator) anyerror!void,
@@ -362,16 +369,16 @@ pub const Store = struct {
             const timestamp = it.created orelse std.Io.Timestamp.now(self.io, .real).toSeconds();
 
             if (T == EtfProfile) {
-                const profile = deserializeEtfProfile(self.allocator, &it) catch return null;
+                const profile = deserializeEtfProfile(allocator, &it) catch return null;
                 return .{ .data = profile, .timestamp = timestamp };
             }
             if (T == OptionsChain) {
-                const items = deserializeOptions(self.allocator, &it) catch return null;
+                const items = deserializeOptions(allocator, &it) catch return null;
                 return .{ .data = items, .timestamp = timestamp };
             }
         }
 
-        return readSlice(T, self.io, self.allocator, data, postProcess, freshness);
+        return readSlice(T, self.io, allocator, data, postProcess, freshness);
     }
 
     /// Serialize data and write to cache with the given TTL.
@@ -499,7 +506,7 @@ pub const Store = struct {
         // below frees these duped strings after we're done with the
         // merged list. Keep the post-process logic in lockstep with
         // the deinit handling — they're a pair.
-        const existing_result = self.read(T, symbol, null, .any);
+        const existing_result = self.read(self.allocator, T, symbol, null, .any);
         const existing: []const T = if (existing_result) |r| r.data else &.{};
         defer if (existing_result != null) {
             if (comptime @hasDecl(T, "deinit")) {
@@ -743,7 +750,7 @@ pub const Store = struct {
             self.appendRaw(symbol, .candles_daily, srf_data) catch |append_err| {
                 // Append failed (file missing?) — fall back to full load + rewrite
                 log.debug("{s}: append failed ({s}), falling back to full rewrite", .{ symbol, @errorName(append_err) });
-                if (self.read(Candle, symbol, null, .any)) |existing| {
+                if (self.read(self.allocator, Candle, symbol, null, .any)) |existing| {
                     defer self.allocator.free(existing.data);
                     const merged = self.allocator.alloc(Candle, existing.data.len + new_candles.len) catch return;
                     defer self.allocator.free(merged);
@@ -1286,6 +1293,219 @@ pub const Store = struct {
 
     // ── Private serialization: generic ───────────────────────────
 
+    /// Comptime: does T have any `[]const u8` fields (or
+    /// `?[]const u8`)? Drives the `parse_allocator` choice in
+    /// `readSlice` — types that don't need to retain string
+    /// values past `fields.to(T, .{})` can use `.none` and save
+    /// the allocator hit per parsed value.
+    ///
+    /// Conservative: any slice-of-u8 field (with or without
+    /// optional, with or without const) flips this to false.
+    /// Composite types (custom structs with their own SRF parse
+    /// hooks) are NOT inspected — if a field's type isn't a
+    /// plain slice-of-u8, we assume it might internally allocate
+    /// strings during its custom parse and treat it as
+    /// string-bearing. This is the safe default; a future audit
+    /// can opt specific composites in.
+    fn hasNoStringFields(comptime T: type) bool {
+        const info = @typeInfo(T);
+        if (info != .@"struct") return false;
+        inline for (info.@"struct".fields) |f| {
+            const FT = f.type;
+            if (FT == []const u8 or FT == []u8 or
+                FT == ?[]const u8 or FT == ?[]u8) return false;
+            // Composite (struct / union / enum) field: assume it
+            // might be a wrapper that stashes a string. Bail.
+            const fti = @typeInfo(FT);
+            switch (fti) {
+                .int, .float, .bool, .@"enum" => {},
+                .optional => |opt| {
+                    const ci = @typeInfo(opt.child);
+                    switch (ci) {
+                        .int, .float, .bool, .@"enum" => {},
+                        else => return false,
+                    }
+                },
+                .@"struct" => {
+                    // Allow only the project's `Date` (pure i32
+                    // wrapper). Detected by name (the @typeName
+                    // result for our `src/Date.zig` ends in
+                    // "Date" — sometimes shown as just "Date",
+                    // sometimes as a longer-qualified path
+                    // depending on how the type was reached).
+                    if (!std.mem.endsWith(u8, @typeName(FT), "Date")) return false;
+                },
+                else => return false,
+            }
+        }
+        return true;
+    }
+
+    // ── hasNoStringFields tests ─────────────────────────────
+    //
+    // Pin the comptime predicate that drives the parse_allocator
+    // choice in `readSlice`. If a future field added to one of
+    // these types changes the classification, the test catches
+    // it before the perf optimization silently regresses (or
+    // worse — if a Candle-shape gets a `?[]const u8` field
+    // added without updating the test, parse_alloc would stay
+    // `.none` and the new string field would be a borrowed slice
+    // into freed-by-defer iterator memory).
+
+    test "hasNoStringFields: Candle is pure-numeric (Date+5×f64+u64)" {
+        try std.testing.expect(hasNoStringFields(Candle));
+    }
+
+    test "hasNoStringFields: Split is pure-numeric (Date+2×f64)" {
+        try std.testing.expect(hasNoStringFields(Split));
+    }
+
+    test "hasNoStringFields: Dividend has currency string -> false" {
+        // Dividend.currency is `?[]const u8` — caller keeps it
+        // past the iterator, so we MUST dupe.
+        try std.testing.expect(!hasNoStringFields(Dividend));
+    }
+
+    test "hasNoStringFields: EarningsEvent has string fields -> false" {
+        try std.testing.expect(!hasNoStringFields(EarningsEvent));
+    }
+
+    test "hasNoStringFields: EtfProfile has string fields -> false" {
+        try std.testing.expect(!hasNoStringFields(EtfProfile));
+    }
+
+    test "hasNoStringFields: synthetic shapes" {
+        // Pure ints/floats/bools/enums + Date — should pass.
+        const Pure = struct {
+            a: i32,
+            b: f64,
+            c: bool,
+            d: enum { x, y },
+            e: Date,
+            f: ?u32,
+        };
+        try std.testing.expect(hasNoStringFields(Pure));
+
+        // Bare []const u8 — should fail.
+        const HasString = struct {
+            a: i32,
+            b: []const u8,
+        };
+        try std.testing.expect(!hasNoStringFields(HasString));
+
+        // Optional []const u8 — should fail.
+        const HasOptString = struct {
+            a: i32,
+            b: ?[]const u8,
+        };
+        try std.testing.expect(!hasNoStringFields(HasOptString));
+
+        // []u8 (mutable) — should also fail. We don't ship any
+        // mutable-slice fields today, but the predicate guards
+        // against future drift.
+        const HasMutString = struct {
+            a: i32,
+            b: []u8,
+        };
+        try std.testing.expect(!hasNoStringFields(HasMutString));
+    }
+
+    test "hasNoStringFields: composite struct field that's not Date is treated as string-bearing" {
+        // Conservative default: if a field's type is a struct we
+        // don't recognize as Date, we don't try to inspect it
+        // recursively — assume it might allocate during its
+        // custom parse hook.
+        const InnerWithString = struct {
+            s: []const u8,
+        };
+        const Outer = struct {
+            x: i32,
+            y: InnerWithString,
+        };
+        try std.testing.expect(!hasNoStringFields(Outer));
+    }
+
+    test "hasNoStringFields: non-struct types return false" {
+        // The predicate is meaningful only for record types
+        // parsed by SRF (always structs in zfin). Anything else
+        // returns false defensively.
+        try std.testing.expect(!hasNoStringFields(u32));
+        try std.testing.expect(!hasNoStringFields([]const u8));
+    }
+
+    /// Hand-rolled specialized coercer for Candle records.
+    /// Bypasses SRF's generalized `fields.to(T, ...)` for the
+    /// hot Candle parse path: zfin's cold candle load deserializes
+    /// hundreds of thousands of records of fixed 7-field shape,
+    /// where `fields.to`'s per-field framework cost (coerce()
+    /// boundary, found-bitmap bookkeeping, inline-for dispatch
+    /// chain) dominates. Direct first-byte switch + struct
+    /// assignment is ~25x faster in ReleaseFast for the same
+    /// correct result on well-formed cache files.
+    ///
+    /// Trade-off vs `fields.to`: this skips default-value
+    /// fallback, missing-field detection, and `coerce()`'s
+    /// strict type discipline. Adequate for our cache-write
+    /// invariant (every candle file we write contains exactly
+    /// the 7 fields below); inadequate for parsing arbitrary
+    /// user-supplied SRF data.
+    ///
+    /// Cache discipline: keys we don't recognize (the `else`
+    /// arm) are silently skipped, matching `fields.to`'s
+    /// behavior on unknown fields. Records with missing fields
+    /// produce a Candle with the zero-init default for the
+    /// absent field — also matching the broader `fields.to`
+    /// contract since Candle's fields have no SRF defaults.
+    ///
+    /// See SRF's `pub fn to` doc comment for the broader
+    /// "specialized vs generalized" trade-off discussion.
+    fn coerceCandleSpecialized(fields: srf.RecordIterator.FieldIterator) !Candle {
+        var c: Candle = .{
+            .date = Date.fromYmd(1970, 1, 1),
+            .open = 0,
+            .high = 0,
+            .low = 0,
+            .close = 0,
+            .adj_close = 0,
+            .volume = 0,
+        };
+        while (try fields.next()) |f| {
+            const key = f.key;
+            const val = f.value orelse continue;
+            // Switch on the first byte. All 7 Candle field names
+            // are first-byte-unique:
+            //   d -> date          o -> open          h -> high
+            //   l -> low           c -> close         a -> adj_close
+            //   v -> volume
+            if (key.len == 0) continue;
+            switch (key[0]) {
+                'd' => if (val == .string) {
+                    c.date = try Date.parse(val.string);
+                },
+                'o' => if (val == .number) {
+                    c.open = val.number;
+                },
+                'h' => if (val == .number) {
+                    c.high = val.number;
+                },
+                'l' => if (val == .number) {
+                    c.low = val.number;
+                },
+                'c' => if (val == .number) {
+                    c.close = val.number;
+                },
+                'a' => if (val == .number) {
+                    c.adj_close = val.number;
+                },
+                'v' => if (val == .number) {
+                    c.volume = @as(u64, @intFromFloat(val.number));
+                },
+                else => {},
+            }
+        }
+        return c;
+    }
+
     /// Generic SRF deserializer with optional freshness check.
     /// Single-pass: creates one iterator, optionally checks freshness, extracts
     /// `#!created=` timestamp, and deserializes all records.
@@ -1298,15 +1518,32 @@ pub const Store = struct {
         comptime freshness: Freshness,
     ) ?CacheResult(T) {
         var reader = std.Io.Reader.fixed(data);
-        // `.parse_allocator = .{ .custom = .initTo(allocator) }` tells SRF
-        // to dupe field values (the data we keep) into the caller's
-        // allocator while letting field keys borrow from `data` (we only
-        // need them long enough for `fields.to(T, .{})` to match against
-        // compile-time field names). Records returned from `it.next()`
-        // then own their value strings via the caller's allocator,
-        // ready to outlive the iterator without any further duping.
+        // Choose `parse_allocator` based on whether T has string
+        // fields the caller needs to keep past the iterator.
+        //
+        //   - **Pure-numeric types** (`Candle`: Date+5×f64+u64) have
+        //     zero `[]const u8` fields. The only string seen during
+        //     parse is the `date` value, which Date's custom-parse
+        //     hook converts to `i32` immediately. Nothing needs to
+        //     outlive the iterator. Use `.none` — borrowed slices
+        //     into the input bytes; no allocator hits per record.
+        //   - **String-bearing types** (Dividend, EarningsEvent,
+        //     OptionsChain) have currency / frequency / source /
+        //     option_type fields the caller keeps. Use the custom
+        //     allocator so values are duped into the caller's
+        //     storage and survive `it.deinit()`.
+        //
+        // Why a comptime branch and not a static setting per
+        // call site: keeps `readSlice` generic over T and routes
+        // the optimization through type information that's
+        // already comptime-known. Adding a new pure-numeric type
+        // (e.g. Split) is a one-line edit to the comptime check.
+        const parse_alloc: srf.ParseAllocator = if (comptime hasNoStringFields(T))
+            .none
+        else
+            .{ .custom = .initTo(allocator) };
         var it = srf.iterator(&reader, allocator, .{
-            .parse_allocator = .{ .custom = .initTo(allocator) },
+            .parse_allocator = parse_alloc,
         }) catch return null;
         defer it.deinit();
 
@@ -1331,8 +1568,23 @@ pub const Store = struct {
             }
         }
 
+        // Per-record coercion. Most types use SRF's generalized
+        // `fields.to(T, .{})` — correct for any struct shape but
+        // pays a per-field abstraction cost (coerce() boundary,
+        // found-bitmap bookkeeping, inline-for dispatch chain).
+        //
+        // Candle takes the specialized fast path: every cached
+        // candle file is millions of records of the same fixed
+        // 7-field shape, and the cold-load wall time was almost
+        // entirely `fields.to`. The hand-rolled coercer is ~25x
+        // faster in ReleaseFast for the same correctness on
+        // well-formed cache files. See SRF's `fields.to` doc
+        // comment for the trade-off discussion.
         while (it.next() catch return null) |fields| {
-            var item = fields.to(T, .{}) catch continue;
+            var item: T = if (comptime T == Candle)
+                coerceCandleSpecialized(fields) catch continue
+            else
+                fields.to(T, .{}) catch continue;
             if (comptime postProcess) |pp| {
                 pp(&item, allocator) catch {
                     if (comptime @hasDecl(T, "deinit")) item.deinit(allocator);
@@ -1731,7 +1983,7 @@ test "writeMerged Dividend: empty cache writes input sorted descending" {
     };
     s.write(Dividend, "TEST", incoming[0..], .{ .seconds = Ttl.dividends });
 
-    const result = s.read(Dividend, "TEST", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Dividend, "TEST", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
     defer for (result.data) |d| d.deinit(allocator);
 
@@ -1768,7 +2020,7 @@ test "writeMerged Dividend: existing entries preserved on key collision" {
     };
     s.write(Dividend, "TEST", incoming[0..], .{ .seconds = Ttl.dividends });
 
-    const result = s.read(Dividend, "TEST", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Dividend, "TEST", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
     defer for (result.data) |d| d.deinit(allocator);
 
@@ -1800,7 +2052,7 @@ test "writeMerged Dividend: union sorted desc, new entry added" {
     };
     s.write(Dividend, "TEST", incoming[0..], .{ .seconds = Ttl.dividends });
 
-    const result = s.read(Dividend, "TEST", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Dividend, "TEST", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
     defer for (result.data) |d| d.deinit(allocator);
 
@@ -1901,7 +2153,7 @@ test "writeMerged Dividend: field-level upgrade fills nulls (Tiingo-then-Polygon
     };
     s.writeWithSource(Dividend, "TEST", polygon_view[0..], .{ .seconds = Ttl.dividends }, "polygon");
 
-    const result = s.read(Dividend, "TEST", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Dividend, "TEST", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
     defer for (result.data) |d| d.deinit(allocator);
 
@@ -1951,7 +2203,7 @@ test "writeMerged Dividend: currency upgrade does not double-free" {
     s.writeWithSource(Dividend, "TEST", polygon_view[0..], .{ .seconds = Ttl.dividends }, "polygon");
 
     // Read back and verify the upgrade landed.
-    const result = s.read(Dividend, "TEST", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Dividend, "TEST", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
     defer for (result.data) |d| d.deinit(allocator);
 
@@ -1995,7 +2247,7 @@ test "writeMerged Dividend: existing currency preserved on second write with dif
     defer for (second) |d| d.deinit(allocator);
     s.writeWithSource(Dividend, "TEST", second[0..], .{ .seconds = Ttl.dividends }, "polygon");
 
-    const result = s.read(Dividend, "TEST", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Dividend, "TEST", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
     defer for (result.data) |d| d.deinit(allocator);
 
@@ -2029,7 +2281,7 @@ test "writeMerged Dividend: type unknown counts as null and gets upgraded" {
     };
     s.writeWithSource(Dividend, "TEST", polygon_view[0..], .{ .seconds = Ttl.dividends }, "polygon");
 
-    const result = s.read(Dividend, "TEST", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Dividend, "TEST", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
     defer for (result.data) |d| d.deinit(allocator);
 
@@ -2072,7 +2324,7 @@ test "writeMerged Dividend: non-null fields are not overwritten" {
     };
     s.writeWithSource(Dividend, "TEST", second[0..], .{ .seconds = Ttl.dividends }, "polygon");
 
-    const result = s.read(Dividend, "TEST", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Dividend, "TEST", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
     defer for (result.data) |d| d.deinit(allocator);
 
@@ -2118,7 +2370,7 @@ test "writeMerged Dividend: upgrade is no-op when both have same fields" {
     s.writeWithSource(Dividend, "TEST", repeat[0..], .{ .seconds = Ttl.dividends }, "polygon");
 
     // The merged result is still just one record (no duplication).
-    const result = s.read(Dividend, "TEST", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Dividend, "TEST", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
     defer for (result.data) |d| d.deinit(allocator);
     try std.testing.expectEqual(@as(usize, 1), result.data.len);
@@ -2161,7 +2413,7 @@ test "writeMerged Dividend: near-match dedup catches last-biz-day vs calendar-en
     };
     s.writeWithSource(Dividend, "FDRXX", tiingo_view[0..], .{ .seconds = Ttl.dividends }, "tiingo");
 
-    const result = s.read(Dividend, "FDRXX", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Dividend, "FDRXX", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
     defer for (result.data) |d| d.deinit(allocator);
 
@@ -2196,7 +2448,7 @@ test "writeMerged Dividend: near-match dedup respects 3-day window upper bound"
     };
     s.writeWithSource(Dividend, "TEST", second[0..], .{ .seconds = Ttl.dividends }, "tiingo");
 
-    const result = s.read(Dividend, "TEST", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Dividend, "TEST", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
     defer for (result.data) |d| d.deinit(allocator);
 
@@ -2229,7 +2481,7 @@ test "writeMerged Dividend: near-match dedup respects amount tolerance" {
     };
     s.writeWithSource(Dividend, "TEST", second[0..], .{ .seconds = Ttl.dividends }, "tiingo");
 
-    const result = s.read(Dividend, "TEST", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Dividend, "TEST", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
     defer for (result.data) |d| d.deinit(allocator);
 
@@ -2267,7 +2519,7 @@ test "writeMerged Dividend: near-match dedup tolerates Tiingo amount rounding" {
     };
     s.writeWithSource(Dividend, "FAGIX", tiingo_view[0..], .{ .seconds = Ttl.dividends }, "tiingo");
 
-    const result = s.read(Dividend, "FAGIX", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Dividend, "FAGIX", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
     defer for (result.data) |d| d.deinit(allocator);
 
@@ -2304,7 +2556,7 @@ test "writeMerged Split: near-match dedup is a no-op (no amount field)" {
     };
     s.writeWithSource(Split, "TEST", second[0..], .{ .seconds = Ttl.splits }, "tiingo");
 
-    const result = s.read(Split, "TEST", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Split, "TEST", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
 
     try std.testing.expectEqual(@as(usize, 2), result.data.len);
@@ -2329,7 +2581,7 @@ test "writeMerged Split: SPYM-style supplementary entry added" {
     };
     s.write(Split, "SPYM", tiingo_view[0..], .{ .seconds = Ttl.splits });
 
-    const result = s.read(Split, "SPYM", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Split, "SPYM", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
 
     try std.testing.expectEqual(@as(usize, 1), result.data.len);
@@ -2362,7 +2614,7 @@ test "writeMerged Split: forward-looking Polygon entry preserved across Tiingo r
     };
     s.write(Split, "TEST", tiingo_view[0..], .{ .seconds = Ttl.splits });
 
-    const result = s.read(Split, "TEST", null, .any) orelse return error.NoCache;
+    const result = s.read(s.allocator, Split, "TEST", null, .any) orelse return error.NoCache;
     defer allocator.free(result.data);
 
     // Both entries must remain — Polygon's forward-looking entry survives.
@@ -2769,7 +3021,7 @@ test "Store.read self-heals torn candles_daily and wipes the pair" {
     try store.writeRaw("FRDM", .candles_meta, intact_meta);
 
     // Reading candles MUST signal cache miss.
-    const result = store.read(Candle, "FRDM", null, .any);
+    const result = store.read(store.allocator, Candle, "FRDM", null, .any);
     try std.testing.expect(result == null);
 
     // Both files are wiped.
@@ -2819,7 +3071,7 @@ test "Store.read does not self-heal an intact candles_daily" {
     try store.writeRaw("OK", .candles_meta, good_meta);
 
     // Reading should succeed, and the cache files must still be there.
-    const result = store.read(Candle, "OK", null, .any);
+    const result = store.read(store.allocator, Candle, "OK", null, .any);
     try std.testing.expect(result != null);
     if (result) |r| {
         defer testing.allocator.free(r.data);

src/commands/analysis.zig

@@ -6,7 +6,7 @@ const fmt = cli.fmt;
 const Money = @import("../Money.zig");
 
 pub const ParsedArgs = struct {
-    sector_detail: zfin.analysis.Granularity = .mid,
+    sector_detail: zfin.analysis.Granularity = .fine,
 };
 
 pub const meta: framework.Meta = .{
@@ -25,8 +25,7 @@ pub const meta: framework.Meta = .{
     \\Options:
     \\  --sector-detail LEVEL    Sector display granularity:
     \\                             coarse  - 4 buckets (Equity / Fixed Income / Cash / Other)
-    \\                             mid     - ~12 buckets (default; collapses NPORT-P sub-flavors)
-    \\                             fine    - raw NPORT-P breakdown (every Debt / X variant separate)
+    \\                             fine    - one row per bucket label (default)
     \\
     \\Run `zfin enrich <portfolio.srf> > metadata.srf` to bootstrap
     \\classifications, then edit by hand.
@@ -45,12 +44,10 @@ pub fn parseArgs(ctx: *framework.RunCtx, cmd_args: []const []const u8) !ParsedAr
             const value = cmd_args[i];
             if (std.mem.eql(u8, value, "coarse")) {
                 parsed.sector_detail = .coarse;
-            } else if (std.mem.eql(u8, value, "mid")) {
-                parsed.sector_detail = .mid;
             } else if (std.mem.eql(u8, value, "fine")) {
                 parsed.sector_detail = .fine;
             } else {
-                cli.stderrPrint(ctx.io, "Error: --sector-detail must be one of: coarse, mid, fine\n");
+                cli.stderrPrint(ctx.io, "Error: --sector-detail must be one of: coarse, fine\n");
                 return error.InvalidSectorDetail;
             }
         } else {
@@ -126,7 +123,7 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
 
     // Load account tax type metadata (optional). Anchor-derived path
     // is correct: accounts.srf is one-per-portfolio-set.
-    var acct_map_opt: ?zfin.analysis.AccountMap = svc.loadAccountMap(anchor_path);
+    var acct_map_opt: ?zfin.analysis.AccountMap = svc.loadAccountMap(allocator, anchor_path);
     defer if (acct_map_opt) |*am| am.deinit();
 
     var result = zfin.analysis.analyzePortfolio(
@@ -353,10 +350,6 @@ test "display shows all sections" {
         .{ .label = "Fixed Income", .weight = 0.15, .value = 15000.0 },
         .{ .label = "Cash", .weight = 0.05, .value = 5000.0 },
     };
-    const asset_class = [_]zfin.analysis.BreakdownItem{
-        .{ .label = "US Large Cap", .weight = 0.60, .value = 60000.0 },
-        .{ .label = "International", .weight = 0.40, .value = 40000.0 },
-    };
     const sector = [_]zfin.analysis.BreakdownItem{
         .{ .label = "Technology", .weight = 0.35, .value = 35000.0 },
     };
@@ -367,7 +360,6 @@ test "display shows all sections" {
     const unclassified = [_][]const u8{"WEIRD"};
     const result: zfin.analysis.AnalysisResult = .{
         .asset_category = @constCast(&asset_category),
-        .asset_class = @constCast(&asset_class),
         .sector = @constCast(&sector),
         .geo = @constCast(&geo),
         .account = @constCast(&empty),
@@ -383,8 +375,6 @@ test "display shows all sections" {
     try std.testing.expect(std.mem.indexOf(u8, out, "Fixed Income 15.0%") != null);
     try std.testing.expect(std.mem.indexOf(u8, out, "Cash 5.0%") != null);
     try std.testing.expect(std.mem.indexOf(u8, out, "Asset Category") != null);
-    try std.testing.expect(std.mem.indexOf(u8, out, "Asset Class") != null);
-    try std.testing.expect(std.mem.indexOf(u8, out, "US Large Cap") != null);
     try std.testing.expect(std.mem.indexOf(u8, out, "Sector") != null);
     try std.testing.expect(std.mem.indexOf(u8, out, "Technology") != null);
     try std.testing.expect(std.mem.indexOf(u8, out, "Geographic") != null);
@@ -462,7 +452,6 @@ test "display: includes umbrella section when account_map is provided" {
 
     const result: zfin.analysis.AnalysisResult = .{
         .asset_category = @constCast(&asset_category),
-        .asset_class = @constCast(&empty),
         .sector = @constCast(&empty),
         .geo = @constCast(&empty),
         .account = @constCast(&account),

src/commands/audit.zig

@@ -1322,7 +1322,7 @@ fn runHygieneCheck(
     defer portfolio.deinit();
 
     // Load accounts.srf
-    var account_map = svc.loadAccountMap(portfolio_path) orelse {
+    var account_map = svc.loadAccountMap(allocator, portfolio_path) orelse {
         cli.stderrPrint(io, "Error: Cannot read/parse accounts.srf (needed for account mapping)\n");
         return;
     };
@@ -1829,7 +1829,7 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
     const portfolio_path = loaded.anchor();
 
     // Load accounts.srf
-    var account_map = svc.loadAccountMap(portfolio_path) orelse {
+    var account_map = svc.loadAccountMap(allocator, portfolio_path) orelse {
         cli.stderrPrint(io, "Error: Cannot read/parse accounts.srf (needed for account number mapping)\n");
         return;
     };

src/commands/common.zig

@@ -306,7 +306,7 @@ pub fn loadPortfolioPrices(
     return result;
 }
 
-const LoadSummaryStats = struct {
+pub const LoadSummaryStats = struct {
     total: usize,
     from_cache: usize,
     from_server: usize,
@@ -319,7 +319,7 @@ const LoadSummaryStats = struct {
 /// failure here would only mean the user doesn't see the
 /// "Loaded N symbols ..." line; the load itself already
 /// succeeded. Catch + log at the boundary.
-fn printLoadSummary(io: std.Io, color: bool, s: LoadSummaryStats) void {
+pub fn printLoadSummary(io: std.Io, color: bool, s: LoadSummaryStats) void {
     if (builtin.is_test) return;
     printLoadSummaryImpl(io, color, s) catch |err| {
         std.log.debug("printLoadSummary failed: {t}", .{err});

src/commands/contributions.zig

@@ -510,7 +510,7 @@ fn prepareReport(
     // reclassification fires at diff time. When missing or
     // unparseable, computeReport falls back to the default cash_delta
     // classification — no account gets the opt-in treatment.
-    var account_map_opt = svc.loadAccountMap(portfolio_path);
+    var account_map_opt = svc.loadAccountMap(allocator, portfolio_path);
     defer if (account_map_opt) |*am| am.deinit();
 
     // Load transaction_log.srf from BOTH sides of the diff. The

src/commands/enrich.zig

@@ -1,9 +1,11 @@
 const std = @import("std");
+const srf = @import("srf");
 const zfin = @import("../root.zig");
 const cli = @import("common.zig");
 const framework = @import("framework.zig");
 const isCusipLike = @import("../models/portfolio.zig").isCusipLike;
 const ClassificationRecord = zfin.classification.ClassificationRecord;
+const ClassificationEntry = zfin.classification.ClassificationEntry;
 
 pub const ParsedArgs = struct {
     /// Optional symbol (e.g. "AAPL"). Null = portfolio mode (uses
@@ -356,9 +358,7 @@ fn enrichSymbol(io: std.Io, allocator: std.mem.Allocator, svc: *zfin.DataService
         if (c.name) |name| {
             try out.print("# {s}\n", .{name});
         }
-        try out.print("symbol::{s},sector::{s},geo::{s},asset_class::{s}\n", .{
-            sym, derived.sector, derived.geo, derived.asset_class,
-        });
+        try emitRecordLine(out, sym, c.name, derived.sector, derived.geo, derived.asset_class, null);
     }
 
     stderrSymbolProvenance(io, sym, kindFromSource(c.source), null);
@@ -405,7 +405,7 @@ fn emitEtfRows(
     } else {
         try out.print("# {s}\n", .{sym});
     }
-    try emitFundLines(sym, asset_class, sectors, c.sector, geo, out);
+    try emitFundLines(sym, c.name, asset_class, sectors, c.sector, geo, out);
 }
 
 /// Wikidata didn't return a classification for `sym` (either the
@@ -478,6 +478,7 @@ fn hasDominantEquitySector(fund_sectors: ?[]const FundSector) bool {
 /// available.
 fn emitFundLines(
     sym: []const u8,
+    name: ?[]const u8,
     asset_class: []const u8,
     fund_sectors: ?[]const FundSector,
     inferred_sector: ?[]const u8,
@@ -501,10 +502,7 @@ fn emitFundLines(
                     inferred_sector.?
                 else
                     s.description;
-                try out.print(
-                    "symbol::{s},sector::{s},geo::{s},asset_class::{s},pct:num:{d:.2}\n",
-                    .{ sym, sector_str, geo_str, asset_class, s.pct },
-                );
+                try emitRecordLine(out, sym, name, sector_str, geo_str, asset_class, s.pct);
             }
             return;
         }
@@ -513,7 +511,42 @@ fn emitFundLines(
     // one TODO line — but if title-keyword inference returned
     // a sector, use it instead of "TODO".
     const sector_str = inferred_sector orelse "TODO";
-    try out.print("symbol::{s},sector::{s},geo::{s},asset_class::{s}\n", .{ sym, sector_str, geo_str, asset_class });
+    try emitRecordLine(out, sym, name, sector_str, geo_str, asset_class, null);
+}
+
+/// Emit one classification record line. Delegates to the SRF
+/// library's writer-side formatter — that handles field ordering
+/// (driven by `ClassificationEntry`'s field declaration order),
+/// escaping for values containing commas/newlines, and default-
+/// value elision (e.g. an entry with `pct = 100.0` omits the
+/// `pct:num:` field; null-valued optional fields are omitted
+/// entirely).
+///
+/// `emit_directives = false` suppresses the `#!srfv1` header so
+/// this can be called per-record interspersed with the comment
+/// scaffold the enrich output uses.
+fn emitRecordLine(
+    out: *std.Io.Writer,
+    sym: []const u8,
+    name: ?[]const u8,
+    sector: []const u8,
+    geo: []const u8,
+    asset_class: []const u8,
+    pct: ?f64,
+) !void {
+    const entry: ClassificationEntry = .{
+        .symbol = sym,
+        .name = name,
+        .sector = sector,
+        .geo = geo,
+        .asset_class = asset_class,
+        // The default is 100.0; setting it explicitly here
+        // (not via `if (pct) |p| p else 100.0`) so the formatter's
+        // default-elision rule keeps single-class rows lean.
+        .pct = pct orelse 100.0,
+    };
+    const items = [_]ClassificationEntry{entry};
+    try out.print("{f}", .{srf.fmt(ClassificationEntry, &items, .{ .emit_directives = false })});
 }
 
 /// What `getEtfMetrics` provides that `enrich` actually uses:
@@ -743,9 +776,8 @@ fn enrichPortfolio(ctx: *framework.RunCtx, svc: *zfin.DataService) !void {
             if (c.name) |name| {
                 try out.print("# {s}\n", .{name});
             }
-            try out.print("symbol::{s},sector::{s},geo::{s},asset_class::{s}\n\n", .{
-                sym, derived.sector, derived.geo, derived.asset_class,
-            });
+            try emitRecordLine(out, sym, c.name, derived.sector, derived.geo, derived.asset_class, null);
+            try out.print("\n", .{});
         }
 
         switch (kindFromSource(c.source)) {
@@ -1209,13 +1241,56 @@ test "hasDominantEquitySector: null and empty -> false" {
 test "emitFundLines: null sectors -> single TODO line" {
     var out_buf: [256]u8 = undefined;
     var out: std.Io.Writer = .fixed(&out_buf);
-    try emitFundLines("VTI", "ETF", null, null, null, &out);
+    try emitFundLines("VTI", null, "ETF", null, null, null, &out);
     try std.testing.expectEqualStrings(
         "symbol::VTI,sector::TODO,geo::US,asset_class::ETF\n",
         out.buffered(),
     );
 }
 
+test "emitFundLines: name field is emitted as `name::Foo` between symbol and sector" {
+    var out_buf: [512]u8 = undefined;
+    var out: std.Io.Writer = .fixed(&out_buf);
+    const sectors = [_]FundSector{
+        .{ .description = "Equity / Corporate", .pct = 99.5 },
+    };
+    try emitFundLines("SPY", "SPDR S&P 500 ETF Trust", "ETF", sectors[0..], null, null, &out);
+    try std.testing.expectEqualStrings(
+        "symbol::SPY,name::SPDR S&P 500 ETF Trust,sector::Equity / Corporate,geo::US,asset_class::ETF,pct:num:99.5\n",
+        out.buffered(),
+    );
+}
+
+test "emitRecordLine: name=null and pct=null produces shortest form" {
+    var out_buf: [256]u8 = undefined;
+    var out: std.Io.Writer = .fixed(&out_buf);
+    try emitRecordLine(&out, "AAPL", null, "Technology", "US", "US Large Cap", null);
+    try std.testing.expectEqualStrings(
+        "symbol::AAPL,sector::Technology,geo::US,asset_class::US Large Cap\n",
+        out.buffered(),
+    );
+}
+
+test "emitRecordLine: name set, pct null" {
+    var out_buf: [256]u8 = undefined;
+    var out: std.Io.Writer = .fixed(&out_buf);
+    try emitRecordLine(&out, "AAPL", "Apple Inc", "Technology", "US", "US Large Cap", null);
+    try std.testing.expectEqualStrings(
+        "symbol::AAPL,name::Apple Inc,sector::Technology,geo::US,asset_class::US Large Cap\n",
+        out.buffered(),
+    );
+}
+
+test "emitRecordLine: name set, pct set (multi-class fund row)" {
+    var out_buf: [256]u8 = undefined;
+    var out: std.Io.Writer = .fixed(&out_buf);
+    try emitRecordLine(&out, "FAGIX", "Fidelity Capital and Income Fund", "Debt / Corporate", "US", "Fund", 47.69);
+    try std.testing.expectEqualStrings(
+        "symbol::FAGIX,name::Fidelity Capital and Income Fund,sector::Debt / Corporate,geo::US,asset_class::Fund,pct:num:47.69\n",
+        out.buffered(),
+    );
+}
+
 test "emitFundLines: populated sectors -> one line per sector with pct" {
     var out_buf: [512]u8 = undefined;
     var out: std.Io.Writer = .fixed(&out_buf);
@@ -1223,7 +1298,7 @@ test "emitFundLines: populated sectors -> one line per sector with pct" {
         .{ .description = "Debt / Corporate", .pct = 47.69 },
         .{ .description = "Equity / Corporate", .pct = 22.49 },
     };
-    try emitFundLines("FAGIX", "Fund", sectors[0..], null, null, &out);
+    try emitFundLines("FAGIX", null, "Fund", sectors[0..], null, null, &out);
 
     const written = out.buffered();
     try std.testing.expect(std.mem.indexOf(u8, written, "symbol::FAGIX,sector::Debt / Corporate,geo::US,asset_class::Fund,pct:num:47.69") != null);
@@ -1235,7 +1310,7 @@ test "emitFundLines: empty slice -> single TODO line (treats empty as null)" {
     var out_buf: [256]u8 = undefined;
     var out: std.Io.Writer = .fixed(&out_buf);
     const empty: [0]FundSector = .{};
-    try emitFundLines("VTI", "ETF", empty[0..], null, null, &out);
+    try emitFundLines("VTI", null, "ETF", empty[0..], null, null, &out);
     try std.testing.expectEqualStrings(
         "symbol::VTI,sector::TODO,geo::US,asset_class::ETF\n",
         out.buffered(),
@@ -1251,7 +1326,7 @@ test "emitFundLines: negative pct values render correctly" {
         .{ .description = "Repurchase Agreement / Other", .pct = -29.72 },
         .{ .description = "Derivative-FX / Other", .pct = -0.84 },
     };
-    try emitFundLines("PTY", "Fund", sectors[0..], null, null, &out);
+    try emitFundLines("PTY", null, "Fund", sectors[0..], null, null, &out);
 
     const written = out.buffered();
     try std.testing.expect(std.mem.indexOf(u8, written, "pct:num:-29.72") != null);
@@ -1264,7 +1339,7 @@ test "emitFundLines: ETF asset_class flows through" {
     const sectors = [_]FundSector{
         .{ .description = "Equity / Corporate", .pct = 99.86 },
     };
-    try emitFundLines("SOXX", "ETF", sectors[0..], null, null, &out);
+    try emitFundLines("SOXX", null, "ETF", sectors[0..], null, null, &out);
     try std.testing.expectEqualStrings(
         "symbol::SOXX,sector::Equity / Corporate,geo::US,asset_class::ETF,pct:num:99.86\n",
         out.buffered(),

src/commands/import.zig

@@ -371,7 +371,7 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
     // way every other zfin command does. We don't need the
     // service for anything else (no price fetching), but reusing
     // its helper keeps sibling-file resolution consistent.
-    var account_map = svc.loadAccountMap(target_path) orelse {
+    var account_map = svc.loadAccountMap(allocator, target_path) orelse {
         cli.stderrPrint(io, "Error: Cannot read/parse accounts.srf next to the target portfolio.\n");
         cli.stderrPrint(io, "       Import needs `institution::` + `account_number::` entries to map\n");
         cli.stderrPrint(io, "       brokerage account numbers to portfolio account names.\n");

src/commands/review.zig

@@ -13,11 +13,28 @@ const zfin = @import("../root.zig");
 const cli = @import("common.zig");
 const framework = @import("framework.zig");
 const review_view = @import("../views/review.zig");
+const observations_view = @import("../views/observations_view.zig");
+const observations = @import("../analytics/observations.zig");
+const Journal = @import("../data/Journal.zig");
 const portfolio_risk = @import("../analytics/portfolio_risk.zig");
 
 pub const ParsedArgs = struct {
     sort: ?review_view.SortField = null,
     sort_dir: review_view.SortDirection = .desc,
+    /// Whether to render acknowledged findings in the findings
+    /// table. Default false (active findings only).
+    show_acked: bool = false,
+    /// Which observation checks to run + display. `.all` runs every
+    /// registered check; `.fast` runs only short-running ones (none
+    /// in M2 — every check is fast). `.none` skips the engine
+    /// entirely (don't render the findings section).
+    checks: ChecksMode = .all,
+};
+
+pub const ChecksMode = enum {
+    all,
+    fast,
+    none,
 };
 
 pub const meta: framework.Meta = .{
@@ -44,6 +61,11 @@ pub const meta: framework.Meta = .{
     \\                             5y-maxdd
     \\  --asc                    Sort ascending (default: descending for
     \\                           numeric fields, ascending for symbol/sector)
+    \\  --checks=MODE            Observation engine mode: all (default),
+    \\                           fast (skip long-running checks), none
+    \\                           (suppress findings section).
+    \\  --show-acked             Include already-acknowledged findings
+    \\                           in the findings table.
     \\
     \\Reads classifications from `metadata.srf` and account tax types
     \\from `accounts.srf`. Tax% is the share of each holding's market
@@ -51,7 +73,7 @@ pub const meta: framework.Meta = .{
     \\
     ,
     .uppercase_first_arg = false,
-    .user_errors = error{ UnexpectedArg, InvalidSortField },
+    .user_errors = error{ UnexpectedArg, InvalidSortField, InvalidChecksMode },
 };
 
 pub fn parseArgs(ctx: *framework.RunCtx, cmd_args: []const []const u8) !ParsedArgs {
@@ -73,6 +95,14 @@ pub fn parseArgs(ctx: *framework.RunCtx, cmd_args: []const []const u8) !ParsedAr
             parsed.sort_dir = .asc;
         } else if (std.mem.eql(u8, arg, "--desc")) {
             parsed.sort_dir = .desc;
+        } else if (std.mem.eql(u8, arg, "--show-acked")) {
+            parsed.show_acked = true;
+        } else if (std.mem.startsWith(u8, arg, "--checks=")) {
+            const value = arg["--checks=".len..];
+            parsed.checks = parseChecksMode(value) orelse {
+                cli.stderrPrint(ctx.io, "Error: --checks must be one of: all, fast, none\n");
+                return error.InvalidChecksMode;
+            };
         } else {
             cli.stderrPrint(ctx.io, "Error: 'review' takes no positional arguments\n");
             return error.UnexpectedArg;
@@ -81,6 +111,13 @@ pub fn parseArgs(ctx: *framework.RunCtx, cmd_args: []const []const u8) !ParsedAr
     return parsed;
 }
 
+fn parseChecksMode(s: []const u8) ?ChecksMode {
+    if (std.mem.eql(u8, s, "all")) return .all;
+    if (std.mem.eql(u8, s, "fast")) return .fast;
+    if (std.mem.eql(u8, s, "none")) return .none;
+    return null;
+}
+
 /// Comma-joined valid sort fields for the user-facing error message.
 /// Built once at comptime so we don't allocate at error time.
 const joined_sort_fields: []const u8 = blk: {
@@ -151,7 +188,7 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
     };
     defer cm.deinit();
 
-    var acct_map_opt: ?zfin.analysis.AccountMap = svc.loadAccountMap(anchor_path);
+    var acct_map_opt: ?zfin.analysis.AccountMap = svc.loadAccountMap(allocator, anchor_path);
     defer if (acct_map_opt) |*am| am.deinit();
 
     // Per-symbol cached dividends so total-return windows include
@@ -166,13 +203,14 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
         dividend_map.deinit();
     }
     for (pf_data.summary.allocations) |a| {
-        if (svc.getCachedDividends(a.symbol)) |divs| {
-            try dividend_map.put(a.symbol, divs);
+        if (svc.getCachedDividends(allocator, a.symbol)) |divs| {
+            try dividend_map.put(a.symbol, divs.data);
         }
     }
 
     var view = try review_view.buildReview(
         allocator,
+        io,
         pf_data.summary,
         &pf_data.candle_map,
         &dividend_map,
@@ -184,6 +222,14 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
     );
     defer view.deinit(allocator);
 
+    // CLI is one-shot output — block until every async check
+    // resolves so the rendered grid + findings are complete.
+    // (The TUI renders progressively instead; see review_tab's
+    // tick hook.)
+    if (view.observations) |*panel| {
+        for (panel.pending) |*pc| _ = pc.awaitResult(io);
+    }
+
     // Sort: explicit --sort overrides the default grouping.
     if (parsed.sort) |field| {
         review_view.sortRows(view.rows, field, parsed.sort_dir);
@@ -191,7 +237,7 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
         review_view.sortGroupedByDefault(view.rows);
     }
 
-    try render(out, color, view);
+    try render(allocator, io, out, color, view, anchor_path, parsed);
 }
 
 // ── Rendering ─────────────────────────────────────────────────
@@ -227,7 +273,15 @@ const col_widths = [_]usize{
     col_tax,
 };
 
-fn render(out: *std.Io.Writer, color: bool, view: review_view.ReviewView) !void {
+fn render(
+    allocator: std.mem.Allocator,
+    io: std.Io,
+    out: *std.Io.Writer,
+    color: bool,
+    view: review_view.ReviewView,
+    anchor_path: []const u8,
+    parsed: ParsedArgs,
+) !void {
     try cli.printBold(out, color, "\nPortfolio Review ({s})\n", .{view.portfolio_path});
     try cli.setFg(out, color, cli.CLR_MUTED);
     try out.print("  As of {f}    Liquid: {f}    Holdings: {d}\n\n", .{
@@ -235,6 +289,14 @@ fn render(out: *std.Io.Writer, color: bool, view: review_view.ReviewView) !void
     });
     try cli.reset(out, color);
 
+    // Status grid: per-check pass/warn/flag glyphs across the top.
+    // Mirrors the TUI review tab so the "at a glance" experience
+    // matches when the user pipes the CLI output.
+    if (view.observations) |panel| {
+        try renderStatusGrid(out, color, panel);
+        try out.print("\n", .{});
+    }
+
     // Header row. Column order matches `col_widths`.
     try cli.setFg(out, color, cli.CLR_MUTED);
     try out.print("  ", .{});
@@ -274,9 +336,173 @@ fn render(out: *std.Io.Writer, color: bool, view: review_view.ReviewView) !void
         try cli.reset(out, color);
     }
 
+    // Findings section. Render unless `--checks=none` was passed.
+    if (parsed.checks != .none) {
+        try renderFindings(allocator, io, out, color, &view, anchor_path, parsed.show_acked);
+    }
+
     try out.print("\n", .{});
 }
 
+/// Render the per-check status grid to stdout. Layout mirrors
+/// the TUI's `appendStatusGrid`: 3 cells per row, each cell
+/// "<right-padded label> <glyph>". Color promoted to the row's
+/// worst severity so multi-cell rows still draw the user's eye
+/// to the bad ones.
+fn renderStatusGrid(
+    out: *std.Io.Writer,
+    color: bool,
+    panel: observations.CheckPanel,
+) !void {
+    if (panel.pending.len == 0) return;
+
+    const status_label_cols: usize = 22;
+    const cells_per_row: usize = 3;
+
+    var i: usize = 0;
+    while (i < panel.pending.len) {
+        const end = @min(i + cells_per_row, panel.pending.len);
+
+        // Row color = worst severity in the row.
+        var worst: u8 = 0; // 0=pass/skipped, 1=warn, 2=flag/err
+        var worst_color: [3]u8 = cli.CLR_MUTED;
+        for (panel.pending[i..end]) |pc| {
+            // The CLI awaits every check before rendering (see
+            // run()), so .pending here would be a logic bug —
+            // skip defensively rather than crash.
+            const result = switch (pc.state) {
+                .complete => |r| r,
+                .pending => continue,
+            };
+            const rank: u8 = switch (result) {
+                .pass, .skipped => 0,
+                .warn => 1,
+                .flag, .err => 2,
+            };
+            if (rank > worst) {
+                worst = rank;
+                worst_color = switch (result) {
+                    .warn => cli.CLR_WARNING,
+                    .flag, .err => cli.CLR_NEGATIVE,
+                    else => cli.CLR_MUTED,
+                };
+            }
+        }
+        try cli.setFg(out, color, worst_color);
+
+        try out.print("  ", .{});
+        for (panel.pending[i..end], 0..) |pc, col| {
+            if (col > 0) try out.print("  ", .{});
+
+            const label = pc.check.label;
+            const lbl_cols = label.len; // ASCII labels: byte count == display cols
+
+            // Right-pad label.
+            if (lbl_cols < status_label_cols) {
+                var k: usize = 0;
+                while (k < status_label_cols - lbl_cols) : (k += 1) try out.print(" ", .{});
+            }
+            try out.print("{s} ", .{label});
+
+            const glyph: []const u8 = switch (pc.state) {
+                .complete => |r| switch (r) {
+                    .pass => "✅\u{FE0F}",
+                    .warn => "⚠️",
+                    .flag => "❌\u{FE0F}",
+                    .skipped => "➖\u{FE0F}",
+                    .err => "🛑\u{FE0F}",
+                },
+                .pending => "⏳\u{FE0F}", // see defensive note above
+            };
+            try out.print("{s}", .{glyph});
+        }
+        try out.print("\n", .{});
+        try cli.reset(out, color);
+
+        i = end;
+    }
+}
+
+/// Render the findings section to stdout. Loads the journal from
+/// the portfolio's directory (missing → empty), joins with the
+/// observation panel via `observations_view.build`, and writes a
+/// styled findings table similar to the TUI's. The CLI is read-only
+/// — acks must come from the TUI.
+fn renderFindings(
+    allocator: std.mem.Allocator,
+    io: std.Io,
+    out: *std.Io.Writer,
+    color: bool,
+    view: *const review_view.ReviewView,
+    anchor_path: []const u8,
+    show_acked: bool,
+) !void {
+    const panel = if (view.observations) |*p| p else return;
+
+    // Load the journal. Missing file ⇒ empty journal (first run).
+    const dir_end = if (std.mem.lastIndexOfScalar(u8, anchor_path, std.fs.path.sep)) |idx| idx + 1 else 0;
+    const journal_path = try std.fmt.allocPrint(allocator, "{s}acknowledgments.srf", .{anchor_path[0..dir_end]});
+    defer allocator.free(journal_path);
+
+    var j = Journal.load(allocator, io, journal_path) catch |err| blk: {
+        cli.stderrPrint(io, "Warning: ");
+        cli.stderrPrint(io, journal_path);
+        cli.stderrPrint(io, ": ");
+        cli.stderrPrint(io, @errorName(err));
+        cli.stderrPrint(io, " — proceeding with empty journal.\n");
+        const empty = try allocator.alloc(Journal.Entry, 0);
+        break :blk Journal{ .allocator = allocator, .entries = empty };
+    };
+    defer j.deinit();
+
+    var fv = try observations_view.build(allocator, panel, &j, show_acked);
+    defer fv.deinit(allocator);
+
+    try out.print("\n", .{});
+    try cli.setFg(out, color, cli.CLR_HEADER);
+    try out.print("  Findings ({d} active, {d} acked, {d} resolved)", .{
+        fv.total_active,
+        fv.total_acked,
+        fv.total_resolved,
+    });
+    if (show_acked) try out.print("  [showing acked]", .{});
+    try out.print("\n", .{});
+    try cli.reset(out, color);
+    try writeSeparator(out, color);
+
+    if (fv.rows.len == 0) {
+        try cli.setFg(out, color, cli.CLR_MUTED);
+        if (fv.total_acked > 0 and !show_acked) {
+            try out.print("  No active findings. Use --show-acked to see acknowledged.\n", .{});
+        } else {
+            try out.print("  No findings.\n", .{});
+        }
+        try cli.reset(out, color);
+        return;
+    }
+
+    for (fv.rows) |row| {
+        const glyph: []const u8 = switch (row.severity) {
+            .warn => "⚠️",
+            .flag => "❌\u{FE0F}",
+            .err => "🛑\u{FE0F}",
+        };
+        const ansi: [3]u8 = if (row.is_acked)
+            cli.CLR_MUTED
+        else switch (row.severity) {
+            .warn => cli.CLR_WARNING,
+            .flag, .err => cli.CLR_NEGATIVE,
+        };
+        try cli.setFg(out, color, ansi);
+        try out.print("  {s} {s}{s}\n", .{
+            glyph,
+            if (row.is_acked) "[acked] " else "",
+            row.text,
+        });
+        try cli.reset(out, color);
+    }
+}
+
 fn writeSeparator(out: *std.Io.Writer, color: bool) !void {
     try cli.setFg(out, color, cli.CLR_MUTED);
     try out.print("  ", .{});
@@ -297,7 +523,7 @@ fn anyReweightFlag(f: portfolio_risk.ReweightFlags) bool {
 fn renderRow(out: *std.Io.Writer, color: bool, r: review_view.ReviewRow) !void {
     try out.print("  ", .{});
     try out.print("{s:<8}", .{fmt.truncateToCols(r.symbol, col_symbol)});
-    try out.print(" {s:<20}", .{fmt.truncateToCols(zfin.analysis.abbreviateSector(r.sector_mid), col_sector)});
+    try out.print(" {s:<20}", .{fmt.truncateToCols(zfin.analysis.abbreviateSector(r.bucket), col_sector)});
     try out.print(" ", .{});
     try renderPctCell(out, color, .normal, r.weight, col_weight, false);
     try out.print(" ", .{});
@@ -475,6 +701,44 @@ test "parseArgs: positional arg errors" {
     try testing.expectError(error.UnexpectedArg, parseArgs(&ctx, &args));
 }
 
+test "parseArgs: --show-acked sets the flag" {
+    var ctx: framework.RunCtx = undefined;
+    ctx.io = std.testing.io;
+    const args = [_][]const u8{"--show-acked"};
+    const parsed = try parseArgs(&ctx, &args);
+    try testing.expect(parsed.show_acked);
+}
+
+test "parseArgs: --checks=fast sets ChecksMode.fast" {
+    var ctx: framework.RunCtx = undefined;
+    ctx.io = std.testing.io;
+    const args = [_][]const u8{"--checks=fast"};
+    const parsed = try parseArgs(&ctx, &args);
+    try testing.expectEqual(ChecksMode.fast, parsed.checks);
+}
+
+test "parseArgs: --checks=none sets ChecksMode.none" {
+    var ctx: framework.RunCtx = undefined;
+    ctx.io = std.testing.io;
+    const args = [_][]const u8{"--checks=none"};
+    const parsed = try parseArgs(&ctx, &args);
+    try testing.expectEqual(ChecksMode.none, parsed.checks);
+}
+
+test "parseArgs: --checks=BOGUS errors" {
+    var ctx: framework.RunCtx = undefined;
+    ctx.io = std.testing.io;
+    const args = [_][]const u8{"--checks=bogus"};
+    try testing.expectError(error.InvalidChecksMode, parseArgs(&ctx, &args));
+}
+
+test "parseChecksMode: covers all variants" {
+    try testing.expectEqual(ChecksMode.all, parseChecksMode("all").?);
+    try testing.expectEqual(ChecksMode.fast, parseChecksMode("fast").?);
+    try testing.expectEqual(ChecksMode.none, parseChecksMode("none").?);
+    try testing.expect(parseChecksMode("nope") == null);
+}
+
 test "joinSortFields: contains all field names" {
     const joined = joinSortFields();
     for (review_view.sort_field_names) |name| {
@@ -554,7 +818,7 @@ test "renderRow: writes complete row with all fields" {
     var w: std.Io.Writer = .fixed(&buf);
     const r: review_view.ReviewRow = .{
         .symbol = "VTI",
-        .sector_mid = "Diversified",
+        .bucket = "Diversified",
         .tax_pct = 0.40,
         .weight = 0.33,
         .return_1y = 0.15,
@@ -583,7 +847,7 @@ test "renderRow: nulls render as em-dashes" {
     var w: std.Io.Writer = .fixed(&buf);
     const r: review_view.ReviewRow = .{
         .symbol = "NEW",
-        .sector_mid = "Bonds",
+        .bucket = "Bonds",
         .tax_pct = null,
         .weight = 0.05,
         .return_1y = null,
@@ -662,7 +926,7 @@ test "render: emits header, separator, rows, and totals" {
     var rows = [_]review_view.ReviewRow{
         .{
             .symbol = "VTI",
-            .sector_mid = "Diversified",
+            .bucket = "Diversified",
             .tax_pct = 1.0,
             .weight = 0.6,
             .return_1y = 0.15,
@@ -677,7 +941,7 @@ test "render: emits header, separator, rows, and totals" {
         },
         .{
             .symbol = "BND",
-            .sector_mid = "Bonds",
+            .bucket = "Bonds",
             .tax_pct = 0.0,
             .weight = 0.4,
             .return_1y = 0.04,
@@ -711,7 +975,7 @@ test "render: emits header, separator, rows, and totals" {
         .total_liquid = 1_000_000.0,
         .portfolio_path = "test_portfolio.srf",
     };
-    try render(&w, false, view);
+    try render(testing.allocator, std.testing.io, &w, false, view, "test_portfolio.srf", .{ .checks = .none });
     const out = w.buffered();
     try testing.expect(std.mem.indexOf(u8, out, "Portfolio Review") != null);
     try testing.expect(std.mem.indexOf(u8, out, "test_portfolio.srf") != null);
@@ -748,7 +1012,7 @@ test "render: emits reweight footnote when any flag set" {
         .total_liquid = 0,
         .portfolio_path = "x.srf",
     };
-    try render(&w, false, view);
+    try render(testing.allocator, std.testing.io, &w, false, view, "test_portfolio.srf", .{ .checks = .none });
     const out = w.buffered();
     try testing.expect(std.mem.indexOf(u8, out, "Reweighted") != null);
 }

src/commands/snapshot.zig

@@ -274,7 +274,7 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
     //
     // Not applied in auto mode: auto mode's as_of already comes from
     // cache mode and is guaranteed to be a trading day.
-    if (as_of_override != null and !hasAnyTradingDayCandle(svc, syms, as_of)) {
+    if (as_of_override != null and !hasAnyTradingDayCandle(svc, allocator, syms, as_of)) {
         var msg_buf: [256]u8 = undefined;
         const msg = std.fmt.bufPrint(
             &msg_buf,
@@ -291,7 +291,7 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
     defer symbol_prices.deinit();
 
     for (syms) |sym| {
-        if (svc.getCachedCandles(sym)) |cs| {
+        if (svc.getCachedCandles(allocator, sym)) |cs| {
             defer cs.deinit();
             if (zfin.valuation.candleCloseOnOrBefore(cs.data, as_of)) |cad| {
                 try symbol_prices.put(sym, cad);
@@ -559,12 +559,13 @@ pub fn probeFreshAsOfDate(
 /// signals a non-trading day for our purposes.
 pub fn hasAnyTradingDayCandle(
     svc: *zfin.DataService,
+    allocator: std.mem.Allocator,
     symbols: []const []const u8,
     date: Date,
 ) bool {
     for (symbols) |sym| {
         if (portfolio_mod.isMoneyMarketSymbol(sym)) continue;
-        const cs = svc.getCachedCandles(sym) orelse continue;
+        const cs = svc.getCachedCandles(allocator, sym) orelse continue;
         defer cs.deinit();
         // Linear scan from the end — recent dates are where `date` is
         // most likely to land for a backfill.
@@ -593,7 +594,7 @@ pub fn collectQuoteDates(
     for (symbols, 0..) |sym, idx| {
         const is_mm = portfolio_mod.isMoneyMarketSymbol(sym);
         var last_date: ?Date = null;
-        if (svc.getCachedCandles(sym)) |cs| {
+        if (svc.getCachedCandles(allocator, sym)) |cs| {
             defer cs.deinit();
             if (cs.data.len > 0) last_date = cs.data[cs.data.len - 1].date;
         }
@@ -922,7 +923,7 @@ fn runAnalysis(
     var cm = zfin.classification.parseClassificationFile(allocator, meta_data) catch return error.BadMetadata;
     defer cm.deinit();
 
-    var acct_map_opt: ?zfin.analysis.AccountMap = svc.loadAccountMap(portfolio_path);
+    var acct_map_opt: ?zfin.analysis.AccountMap = svc.loadAccountMap(allocator, portfolio_path);
     defer if (acct_map_opt) |*am| am.deinit();
 
     return zfin.analysis.analyzePortfolio(

src/data/Journal.zig

@@ -0,0 +1,705 @@
+//! Journal of acknowledged observations: `acknowledgments.srf`.
+//!
+//! The observation engine produces findings ("Position concentration: NVDA
+//! at 18.2%") on every run. The user can acknowledge a finding via the
+//! review tab to record reasoning ("Holding through earnings cycle") and
+//! suppress it from the active findings list. The journal is the durable
+//! record of those acknowledgments.
+//!
+//! ## File format
+//!
+//! Compact-form SRF, discriminated union by `type::`. Records have a
+//! positional relationship: each `type::note` record attaches to the
+//! most-recently-seen `type::acknowledgment`.
+//!
+//! ### `type::acknowledgment`
+//!
+//!   - `observation::` — check name, e.g. `position_concentration`.
+//!   - `target::` — per-check string convention. `"NVDA"` for single-symbol
+//!     observations; `"sector:Technology"` for sector-scoped; `"VTI,SCHD"`
+//!     for pair-based observations like sector dominance.
+//!   - `acknowledged_at::` — date the user first acked. Immutable after
+//!     creation.
+//!   - `state::` — `active` | `acknowledged` | `resolved`.
+//!   - `unacknowledged_at::` — info-only breadcrumb, set when the user
+//!     most recently un-acked. Persists across re-acks.
+//!   - `resolved_at::` — info-only, set when the engine auto-resolves.
+//!
+//! Each ack is uniquely identified by `(observation, target)`. There is
+//! never more than one entry per pair — `setState` mutates in place; we
+//! don't preserve transition history (git tracks that on the file).
+//!
+//! ### `type::note`
+//!
+//! Zero or more per ack. One field:
+//!
+//!   - `line::` — single-line content. Multi-line notes are written as N
+//!     consecutive note records following the ack.
+//!
+//! Notes are positional: a note record attaches to the most-recent
+//! preceding ack record. A note record before any ack is a hard parse
+//! error (`error.OrphanedNote`). The visual layout in the file
+//! (ack followed by indented-feeling note records) makes the
+//! relationship obvious to a human reader.
+//!
+//! Example:
+//!
+//! ```
+//! #!srfv1
+//! type::acknowledgment,observation::position_concentration,target::NVDA,acknowledged_at::2026-06-08,state::acknowledged
+//! type::note,line::Holding through earnings cycle.
+//! type::note,line::Will trim by Q3 2026.
+//! type::acknowledgment,observation::sector_dominance,target::VTI,SCHD,acknowledged_at::2026-06-08,state::acknowledged
+//! ```
+//!
+//! ## Lifecycle
+//!
+//! - **Read:** single-pass iterator over the file. Acks push a new
+//!   `Entry`; notes append to the last entry. Orphan note ⇒
+//!   `error.OrphanedNote`.
+//! - **Write:** `append` / `setState` mutate the in-memory `entries` and
+//!   atomic-rewrite the file via `atomic.writeFileAtomic`.
+//! - **Concurrency:** the file is per-portfolio (sibling of
+//!   `portfolio.srf`); concurrent zfin invocations on the same portfolio
+//!   would race, but that's the existing convention for every sibling
+//!   file in this codebase.
+
+const std = @import("std");
+const srf = @import("srf");
+const Date = @import("../Date.zig");
+const atomic = @import("../atomic.zig");
+
+const Journal = @This();
+
+allocator: std.mem.Allocator,
+entries: []Entry,
+
+pub const State = enum {
+    active,
+    acknowledged,
+    resolved,
+};
+
+/// One acknowledgment record. Uniquely identified by
+/// `(observation, target)`.
+pub const Acknowledgment = struct {
+    observation: []const u8,
+    target: []const u8,
+    acknowledged_at: Date,
+    state: State,
+    unacknowledged_at: ?Date = null,
+    resolved_at: ?Date = null,
+};
+
+/// Wire-format note record: just a single line of text. Used only by
+/// the SRF parser/formatter; the rest of the codebase sees notes as
+/// `[]const u8` slices on `Entry.notes`.
+const NoteRecord = struct {
+    line: []const u8,
+};
+
+/// Discriminated-union over the two SRF record types in the journal.
+/// SRF dispatches on the `type` tag field by convention. Internal to
+/// the parser/formatter.
+const JournalRecord = union(enum) {
+    acknowledgment: Acknowledgment,
+    note: NoteRecord,
+};
+
+/// 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 {
+    ack: Acknowledgment,
+    /// Note fragments in the order the user entered them. Each is an
+    /// allocator-owned slice; freed by `Journal.deinit`.
+    notes: []const []const u8,
+
+    /// Concatenate the notes with newlines into a single string.
+    /// Allocator-owned; caller frees.
+    pub fn fullNote(self: Entry, allocator: std.mem.Allocator) ![]u8 {
+        return try std.mem.join(allocator, "\n", self.notes);
+    }
+};
+
+pub fn deinit(self: *Journal) void {
+    const a = self.allocator;
+    for (self.entries) |entry| {
+        a.free(entry.ack.observation);
+        a.free(entry.ack.target);
+        for (entry.notes) |line| a.free(line);
+        a.free(entry.notes);
+    }
+    a.free(self.entries);
+    self.* = undefined;
+}
+
+/// Find the entry matching `(observation, target)`. Returns null if
+/// not found. There's only ever one entry per pair (we don't
+/// preserve transition history), so no tiebreaker is needed.
+pub fn findByTarget(
+    self: *const Journal,
+    observation: []const u8,
+    target: []const u8,
+) ?*const Entry {
+    for (self.entries) |*e| {
+        if (!std.mem.eql(u8, e.ack.observation, observation)) continue;
+        if (!std.mem.eql(u8, e.ack.target, target)) continue;
+        return e;
+    }
+    return null;
+}
+
+/// Load and parse a journal file. Returns an empty journal when the
+/// file doesn't exist (first-time use case). `path` should be an
+/// absolute or cwd-relative path to `acknowledgments.srf`.
+pub fn load(
+    allocator: std.mem.Allocator,
+    io: std.Io,
+    path: []const u8,
+) !Journal {
+    const file_data = std.Io.Dir.cwd().readFileAlloc(io, path, allocator, .limited(1024 * 1024)) catch |err| switch (err) {
+        error.FileNotFound => {
+            return .{ .allocator = allocator, .entries = try allocator.alloc(Entry, 0) };
+        },
+        else => return err,
+    };
+    defer allocator.free(file_data);
+    return try parse(allocator, file_data);
+}
+
+/// Parse pre-read file bytes into a `Journal`. Used by `load` and by
+/// tests that supply synthetic data.
+///
+/// Walks records in a single pass. Each `type::acknowledgment`
+/// pushes a new entry with empty notes. Each `type::note` appends
+/// to the most-recent entry's notes. A note before any ack
+/// returns `error.OrphanedNote`.
+///
+/// **Strict**: any record that fails to deserialize (missing
+/// required field, unknown enum variant, garbage bytes) propagates
+/// the error out of `parse`. We don't silently skip — a malformed
+/// record means user-visible data loss (acks suppress findings;
+/// dropping an ack pops a finding back into the active list with
+/// no explanation). Better to fail loud at load time so the user
+/// can fix the file.
+pub fn parse(allocator: std.mem.Allocator, data: []const u8) !Journal {
+    // Empty input ⇒ empty journal. `srf.iterator` requires a version
+    // banner on the first line and errors out otherwise; short-circuit.
+    if (data.len == 0) {
+        return .{
+            .allocator = allocator,
+            .entries = try allocator.alloc(Entry, 0),
+        };
+    }
+
+    var entries = std.ArrayList(Entry).empty;
+    errdefer freeEntries(allocator, &entries);
+
+    // Per-entry notes lists. Lives parallel to `entries` and is
+    // converted to owned slices at the end. We use a separate list
+    // (instead of mutating each entry's `notes` field as we go)
+    // because `Entry.notes` is `[]const []const u8` — a const
+    // slice — so we can't append to it after the entry is created.
+    var notes_per_entry = std.ArrayList(std.ArrayList([]const u8)).empty;
+    errdefer {
+        for (notes_per_entry.items) |*notes| {
+            for (notes.items) |line| allocator.free(line);
+            notes.deinit(allocator);
+        }
+        notes_per_entry.deinit(allocator);
+    }
+
+    var reader = std.Io.Reader.fixed(data);
+    var it = srf.iterator(&reader, allocator, .{ .parse_allocator = .none }) catch return error.InvalidData;
+    defer it.deinit();
+
+    while (try it.next()) |fields| {
+        const rec = try fields.to(JournalRecord, .{});
+        switch (rec) {
+            .acknowledgment => |a| {
+                try entries.append(allocator, .{
+                    .ack = .{
+                        .observation = try allocator.dupe(u8, a.observation),
+                        .target = try allocator.dupe(u8, a.target),
+                        .acknowledged_at = a.acknowledged_at,
+                        .state = a.state,
+                        .unacknowledged_at = a.unacknowledged_at,
+                        .resolved_at = a.resolved_at,
+                    },
+                    .notes = &.{}, // placeholder; replaced below
+                });
+                try notes_per_entry.append(allocator, std.ArrayList([]const u8).empty);
+            },
+            .note => |n| {
+                if (entries.items.len == 0) return error.OrphanedNote;
+                const last_notes = &notes_per_entry.items[notes_per_entry.items.len - 1];
+                try last_notes.append(allocator, try allocator.dupe(u8, n.line));
+            },
+        }
+    }
+
+    // Move notes lists into their entries' `notes` fields as owned
+    // slices. After this each entry owns its own notes; the
+    // outer `notes_per_entry` list is empty.
+    for (entries.items, 0..) |*entry, i| {
+        entry.notes = try notes_per_entry.items[i].toOwnedSlice(allocator);
+    }
+    notes_per_entry.deinit(allocator);
+
+    return .{
+        .allocator = allocator,
+        .entries = try entries.toOwnedSlice(allocator),
+    };
+}
+
+/// Free a partially-built entries list (for parse errdefer).
+fn freeEntries(allocator: std.mem.Allocator, entries: *std.ArrayList(Entry)) void {
+    for (entries.items) |entry| {
+        allocator.free(entry.ack.observation);
+        allocator.free(entry.ack.target);
+        for (entry.notes) |line| allocator.free(line);
+        allocator.free(entry.notes);
+    }
+    entries.deinit(allocator);
+}
+
+/// Append a new acknowledgment with notes, then atomic-rewrite the
+/// file. The journal's in-memory state is updated to reflect the new
+/// entry; caller owns the journal as before.
+///
+/// `note_fragments` is the list of single-line strings the user
+/// entered (one Enter press per fragment in the TUI's note input).
+/// Pass an empty slice to record an ack with no reasoning.
+pub fn append(
+    self: *Journal,
+    io: std.Io,
+    path: []const u8,
+    new_ack: Acknowledgment,
+    note_fragments: []const []const u8,
+) !void {
+    const a = self.allocator;
+
+    // Build the new entry's owned strings first. errdefer cleanup
+    // is fiddly because we need to roll back partial allocations
+    // on any failure; doing it in stages keeps the pattern clear.
+    const owned_obs = try a.dupe(u8, new_ack.observation);
+    errdefer a.free(owned_obs);
+    const owned_target = try a.dupe(u8, new_ack.target);
+    errdefer a.free(owned_target);
+
+    var owned_notes = try a.alloc([]const u8, note_fragments.len);
+    errdefer a.free(owned_notes);
+    var note_idx: usize = 0;
+    errdefer for (owned_notes[0..note_idx]) |line| a.free(line);
+    for (note_fragments, 0..) |frag, i| {
+        owned_notes[i] = try a.dupe(u8, frag);
+        note_idx = i + 1;
+    }
+
+    // Grow the entries slice by one and append.
+    var new_entries = try a.alloc(Entry, self.entries.len + 1);
+    errdefer a.free(new_entries);
+    @memcpy(new_entries[0..self.entries.len], self.entries);
+    new_entries[self.entries.len] = .{
+        .ack = .{
+            .observation = owned_obs,
+            .target = owned_target,
+            .acknowledged_at = new_ack.acknowledged_at,
+            .state = new_ack.state,
+            .unacknowledged_at = new_ack.unacknowledged_at,
+            .resolved_at = new_ack.resolved_at,
+        },
+        .notes = owned_notes,
+    };
+
+    // Replace the slice WITHOUT freeing the old strings — they're
+    // shallow-copied into new_entries above. Just free the old slice.
+    a.free(self.entries);
+    self.entries = new_entries;
+
+    try writeFile(self, io, path);
+}
+
+/// Update the state of an existing acknowledgment, set the relevant
+/// breadcrumb timestamp, and atomic-rewrite the file. The state
+/// transition machine:
+///
+///   - `active → acknowledged` — clears `unacknowledged_at`.
+///   - `acknowledged → active` — sets `unacknowledged_at = today`.
+///   - `* → resolved` — sets `resolved_at = today`.
+///   - `resolved → active` — clears `resolved_at`.
+///
+/// Returns `error.AckNotFound` if no entry matches `(observation,
+/// target)`.
+pub fn setState(
+    self: *Journal,
+    io: std.Io,
+    path: []const u8,
+    observation: []const u8,
+    target: []const u8,
+    new_state: State,
+    today: Date,
+) !void {
+    var found: ?*Entry = null;
+    for (self.entries) |*e| {
+        if (!std.mem.eql(u8, e.ack.observation, observation)) continue;
+        if (!std.mem.eql(u8, e.ack.target, target)) continue;
+        found = e;
+        break;
+    }
+    const entry = found orelse return error.AckNotFound;
+
+    switch (new_state) {
+        .acknowledged => {
+            entry.ack.unacknowledged_at = null;
+        },
+        .active => {
+            if (entry.ack.state == .acknowledged) entry.ack.unacknowledged_at = today;
+            if (entry.ack.state == .resolved) entry.ack.resolved_at = null;
+        },
+        .resolved => {
+            entry.ack.resolved_at = today;
+        },
+    }
+    entry.ack.state = new_state;
+
+    try writeFile(self, io, path);
+}
+
+/// Atomic file write: serialize all records, then hand to
+/// `atomic.writeFileAtomic` which writes to `<path>.tmp`, fsyncs,
+/// and renames. Crash-safe.
+fn writeFile(self: *const Journal, io: std.Io, path: []const u8) !void {
+    const a = self.allocator;
+
+    // Build the file contents in memory. For a journal of typical
+    // size (dozens to low hundreds of records) this is trivially
+    // small; if it grows large we revisit streaming.
+    var buf: std.Io.Writer.Allocating = .init(a);
+    defer buf.deinit();
+
+    // Flatten into a single records slice so `srf.fmt` writes the
+    // `#!srfv1` directive header once at the top and emits every
+    // record through its native formatter. Ack records are followed
+    // immediately by their note records, in entry-list order.
+    var total_records: usize = self.entries.len;
+    for (self.entries) |e| total_records += e.notes.len;
+    var records = try a.alloc(JournalRecord, total_records);
+    defer a.free(records);
+    var ri: usize = 0;
+    for (self.entries) |e| {
+        records[ri] = .{ .acknowledgment = e.ack };
+        ri += 1;
+        for (e.notes) |line| {
+            records[ri] = .{ .note = .{ .line = line } };
+            ri += 1;
+        }
+    }
+
+    try buf.writer.print("{f}", .{srf.fmt(JournalRecord, records, .{})});
+
+    try atomic.writeFileAtomic(io, a, path, buf.writer.buffered());
+}
+
+// ── Tests ────────────────────────────────────────────────────
+
+const testing = std.testing;
+
+test "parse: empty input produces empty journal" {
+    var journal = try parse(testing.allocator, "");
+    defer journal.deinit();
+    try testing.expectEqual(@as(usize, 0), journal.entries.len);
+}
+
+test "parse: single ack with two notes round-trips" {
+    const data =
+        \\#!srfv1
+        \\type::acknowledgment,observation::position_concentration,target::NVDA,acknowledged_at::2026-06-12,state::acknowledged
+        \\type::note,line::Holding through earnings cycle.
+        \\type::note,line::Will trim by Q3 2026.
+    ;
+    var journal = try parse(testing.allocator, data);
+    defer journal.deinit();
+    try testing.expectEqual(@as(usize, 1), journal.entries.len);
+    const entry = journal.entries[0];
+    try testing.expectEqualStrings("position_concentration", entry.ack.observation);
+    try testing.expectEqualStrings("NVDA", entry.ack.target);
+    try testing.expectEqual(State.acknowledged, entry.ack.state);
+    try testing.expectEqual(@as(usize, 2), entry.notes.len);
+    try testing.expectEqualStrings("Holding through earnings cycle.", entry.notes[0]);
+    try testing.expectEqualStrings("Will trim by Q3 2026.", entry.notes[1]);
+}
+
+test "parse: notes attach to the most-recent preceding ack" {
+    const data =
+        \\#!srfv1
+        \\type::acknowledgment,observation::p,target::A,acknowledged_at::2026-06-12,state::active
+        \\type::note,line::for A
+        \\type::acknowledgment,observation::p,target::B,acknowledged_at::2026-06-13,state::active
+        \\type::note,line::for B
+        \\type::note,line::also for B
+    ;
+    var journal = try parse(testing.allocator, data);
+    defer journal.deinit();
+    try testing.expectEqual(@as(usize, 2), journal.entries.len);
+    try testing.expectEqual(@as(usize, 1), journal.entries[0].notes.len);
+    try testing.expectEqualStrings("for A", journal.entries[0].notes[0]);
+    try testing.expectEqual(@as(usize, 2), journal.entries[1].notes.len);
+    try testing.expectEqualStrings("for B", journal.entries[1].notes[0]);
+    try testing.expectEqualStrings("also for B", journal.entries[1].notes[1]);
+}
+
+test "parse: orphan note before any ack returns error.OrphanedNote" {
+    const data =
+        \\#!srfv1
+        \\type::note,line::orphaned
+        \\type::acknowledgment,observation::p,target::T,acknowledged_at::2026-06-12,state::active
+    ;
+    try testing.expectError(error.OrphanedNote, parse(testing.allocator, data));
+}
+
+test "findByTarget: finds matching, returns null for non-match" {
+    const data =
+        \\#!srfv1
+        \\type::acknowledgment,observation::position_concentration,target::NVDA,acknowledged_at::2026-06-12,state::acknowledged
+        \\type::acknowledgment,observation::sector_concentration,target::sector:Technology,acknowledged_at::2026-06-13,state::acknowledged
+    ;
+    var journal = try parse(testing.allocator, data);
+    defer journal.deinit();
+
+    const found = journal.findByTarget("position_concentration", "NVDA").?;
+    try testing.expectEqualStrings("NVDA", found.ack.target);
+
+    const not_found = journal.findByTarget("position_concentration", "AAPL");
+    try testing.expect(not_found == null);
+}
+
+test "Entry.fullNote: joins fragments with newlines" {
+    const data =
+        \\#!srfv1
+        \\type::acknowledgment,observation::p,target::T,acknowledged_at::2026-06-12,state::active
+        \\type::note,line::first
+        \\type::note,line::second
+        \\type::note,line::third
+    ;
+    var journal = try parse(testing.allocator, data);
+    defer journal.deinit();
+    const full = try journal.entries[0].fullNote(testing.allocator);
+    defer testing.allocator.free(full);
+    try testing.expectEqualStrings("first\nsecond\nthird", full);
+}
+
+test "Entry.fullNote: empty notes returns empty string" {
+    const data =
+        \\#!srfv1
+        \\type::acknowledgment,observation::p,target::T,acknowledged_at::2026-06-12,state::active
+    ;
+    var journal = try parse(testing.allocator, data);
+    defer journal.deinit();
+    const full = try journal.entries[0].fullNote(testing.allocator);
+    defer testing.allocator.free(full);
+    try testing.expectEqualStrings("", full);
+}
+
+test "parse: optional unacknowledged_at and resolved_at fields work when omitted" {
+    const data =
+        \\#!srfv1
+        \\type::acknowledgment,observation::p,target::T,acknowledged_at::2026-06-12,state::acknowledged
+    ;
+    var journal = try parse(testing.allocator, data);
+    defer journal.deinit();
+    try testing.expect(journal.entries[0].ack.unacknowledged_at == null);
+    try testing.expect(journal.entries[0].ack.resolved_at == null);
+}
+
+test "parse: optional unacknowledged_at and resolved_at fields work when set" {
+    const data =
+        \\#!srfv1
+        \\type::acknowledgment,observation::p,target::T,acknowledged_at::2026-06-12,state::active,unacknowledged_at::2026-08-01,resolved_at::2026-12-01
+    ;
+    var journal = try parse(testing.allocator, data);
+    defer journal.deinit();
+    try testing.expectEqual(Date.fromYmd(2026, 8, 1).days, journal.entries[0].ack.unacknowledged_at.?.days);
+    try testing.expectEqual(Date.fromYmd(2026, 12, 1).days, journal.entries[0].ack.resolved_at.?.days);
+}
+
+test "parse: malformed record returns parse error" {
+    // A record with no `type::` discriminator should fail SRF
+    // deserialization (ActiveTagNotFirstField), and we propagate
+    // the error rather than silently skipping. The exact error
+    // variant is SRF's choice; we just assert that an error is
+    // returned.
+    const data =
+        \\#!srfv1
+        \\type::acknowledgment,observation::p,target::T,acknowledged_at::2026-06-12,state::acknowledged
+        \\garbage_field::nope
+        \\type::acknowledgment,observation::q,target::U,acknowledged_at::2026-06-13,state::active
+    ;
+    try testing.expectError(error.ActiveTagNotFirstField, parse(testing.allocator, data));
+}
+
+// ── I/O tests (load + append + setState round-trips) ──────────
+
+test "load: missing file returns empty journal" {
+    const allocator = std.testing.allocator;
+    const io = std.testing.io;
+    var tmp = std.testing.tmpDir(.{});
+    defer tmp.cleanup();
+    const dir_path = try tmp.dir.realPathFileAlloc(io, ".", allocator);
+    defer allocator.free(dir_path);
+
+    const path = try std.fmt.allocPrint(allocator, "{s}/does_not_exist.srf", .{dir_path});
+    defer allocator.free(path);
+
+    var journal = try load(allocator, io, path);
+    defer journal.deinit();
+    try testing.expectEqual(@as(usize, 0), journal.entries.len);
+}
+
+test "append + load round-trip: ack with two notes" {
+    const allocator = std.testing.allocator;
+    const io = std.testing.io;
+    var tmp = std.testing.tmpDir(.{});
+    defer tmp.cleanup();
+    const dir_path = try tmp.dir.realPathFileAlloc(io, ".", allocator);
+    defer allocator.free(dir_path);
+
+    const path = try std.fmt.allocPrint(allocator, "{s}/journal.srf", .{dir_path});
+    defer allocator.free(path);
+
+    var journal = try load(allocator, io, path);
+    defer journal.deinit();
+    try testing.expectEqual(@as(usize, 0), journal.entries.len);
+
+    const fragments = [_][]const u8{ "first thought", "follow-up rationale" };
+    try journal.append(io, path, .{
+        .observation = "position_concentration",
+        .target = "NVDA",
+        .acknowledged_at = Date.fromYmd(2026, 6, 8),
+        .state = .acknowledged,
+    }, &fragments);
+
+    var reloaded = try load(allocator, io, path);
+    defer reloaded.deinit();
+    try testing.expectEqual(@as(usize, 1), reloaded.entries.len);
+    try testing.expectEqualStrings("position_concentration", reloaded.entries[0].ack.observation);
+    try testing.expectEqualStrings("NVDA", reloaded.entries[0].ack.target);
+    try testing.expectEqual(State.acknowledged, reloaded.entries[0].ack.state);
+    try testing.expectEqual(@as(usize, 2), reloaded.entries[0].notes.len);
+    try testing.expectEqualStrings("first thought", reloaded.entries[0].notes[0]);
+    try testing.expectEqualStrings("follow-up rationale", reloaded.entries[0].notes[1]);
+}
+
+test "append: two acks land in append-order on reload" {
+    const allocator = std.testing.allocator;
+    const io = std.testing.io;
+    var tmp = std.testing.tmpDir(.{});
+    defer tmp.cleanup();
+    const dir_path = try tmp.dir.realPathFileAlloc(io, ".", allocator);
+    defer allocator.free(dir_path);
+
+    const path = try std.fmt.allocPrint(allocator, "{s}/journal.srf", .{dir_path});
+    defer allocator.free(path);
+
+    var journal = try load(allocator, io, path);
+    defer journal.deinit();
+
+    try journal.append(io, path, .{
+        .observation = "k",
+        .target = "B",
+        .acknowledged_at = Date.fromYmd(2026, 6, 8),
+        .state = .acknowledged,
+    }, &.{});
+    try journal.append(io, path, .{
+        .observation = "k",
+        .target = "A",
+        .acknowledged_at = Date.fromYmd(2026, 6, 8),
+        .state = .acknowledged,
+    }, &.{});
+
+    var reloaded = try load(allocator, io, path);
+    defer reloaded.deinit();
+    try testing.expectEqual(@as(usize, 2), reloaded.entries.len);
+    try testing.expectEqualStrings("B", reloaded.entries[0].ack.target);
+    try testing.expectEqualStrings("A", reloaded.entries[1].ack.target);
+}
+
+test "setState: acknowledged → active sets unacknowledged_at" {
+    const allocator = std.testing.allocator;
+    const io = std.testing.io;
+    var tmp = std.testing.tmpDir(.{});
+    defer tmp.cleanup();
+    const dir_path = try tmp.dir.realPathFileAlloc(io, ".", allocator);
+    defer allocator.free(dir_path);
+
+    const path = try std.fmt.allocPrint(allocator, "{s}/journal.srf", .{dir_path});
+    defer allocator.free(path);
+
+    var journal = try load(allocator, io, path);
+    defer journal.deinit();
+
+    try journal.append(io, path, .{
+        .observation = "k",
+        .target = "X",
+        .acknowledged_at = Date.fromYmd(2026, 6, 8),
+        .state = .acknowledged,
+    }, &.{});
+
+    try journal.setState(io, path, "k", "X", .active, Date.fromYmd(2026, 6, 9));
+
+    var reloaded = try load(allocator, io, path);
+    defer reloaded.deinit();
+    try testing.expectEqual(State.active, reloaded.entries[0].ack.state);
+    try testing.expect(reloaded.entries[0].ack.unacknowledged_at != null);
+    try testing.expect(reloaded.entries[0].ack.unacknowledged_at.?.eql(Date.fromYmd(2026, 6, 9)));
+}
+
+test "setState: missing target returns AckNotFound" {
+    const allocator = std.testing.allocator;
+    const io = std.testing.io;
+    var tmp = std.testing.tmpDir(.{});
+    defer tmp.cleanup();
+    const dir_path = try tmp.dir.realPathFileAlloc(io, ".", allocator);
+    defer allocator.free(dir_path);
+
+    const path = try std.fmt.allocPrint(allocator, "{s}/journal.srf", .{dir_path});
+    defer allocator.free(path);
+
+    var journal = try load(allocator, io, path);
+    defer journal.deinit();
+
+    try testing.expectError(
+        error.AckNotFound,
+        journal.setState(io, path, "k", "missing", .resolved, Date.fromYmd(2026, 6, 8)),
+    );
+}
+
+test "setState: → resolved sets resolved_at" {
+    const allocator = std.testing.allocator;
+    const io = std.testing.io;
+    var tmp = std.testing.tmpDir(.{});
+    defer tmp.cleanup();
+    const dir_path = try tmp.dir.realPathFileAlloc(io, ".", allocator);
+    defer allocator.free(dir_path);
+
+    const path = try std.fmt.allocPrint(allocator, "{s}/journal.srf", .{dir_path});
+    defer allocator.free(path);
+
+    var journal = try load(allocator, io, path);
+    defer journal.deinit();
+
+    try journal.append(io, path, .{
+        .observation = "k",
+        .target = "X",
+        .acknowledged_at = Date.fromYmd(2026, 6, 8),
+        .state = .acknowledged,
+    }, &.{});
+
+    try journal.setState(io, path, "k", "X", .resolved, Date.fromYmd(2026, 6, 10));
+
+    var reloaded = try load(allocator, io, path);
+    defer reloaded.deinit();
+    try testing.expectEqual(State.resolved, reloaded.entries[0].ack.state);
+    try testing.expect(reloaded.entries[0].ack.resolved_at != null);
+    try testing.expect(reloaded.entries[0].ack.resolved_at.?.eql(Date.fromYmd(2026, 6, 10)));
+}

src/data/staleness.zig

@@ -37,6 +37,7 @@ const Date = @import("../Date.zig");
 const risk = @import("../analytics/risk.zig");
 const shiller = @import("shiller.zig");
 const review = @import("../views/review.zig");
+const observations = @import("../analytics/observations.zig");
 
 /// A hand-maintained data source that nags once a year if it hasn't
 /// been refreshed by its annual `(due_month, due_day)`.
@@ -77,6 +78,13 @@ pub const entries = [_]StaleEntry{
         .due_day = 1,
         .source_file = "src/views/review.zig",
     },
+    .{
+        .name = "Observation engine thresholds",
+        .last_updated = observations.observation_thresholds_last_reviewed,
+        .due_month = 6,
+        .due_day = 1,
+        .source_file = "src/analytics/observations.zig",
+    },
 };
 
 /// Write a warning line for each entry in `entries` that is overdue
@@ -292,7 +300,7 @@ test "silent when today is one day before due" {
 test "real registry compiles and is non-empty" {
     // Guard that the registry stays wired up; doesn't assert any
     // particular nag behavior (real dates drift over time).
-    try std.testing.expect(entries.len >= 3);
+    try std.testing.expect(entries.len >= 4);
     for (entries) |e| {
         try std.testing.expect(e.name.len > 0);
         try std.testing.expect(e.source_file.len > 0);

src/models/classification.zig

@@ -15,6 +15,19 @@ const srf = @import("srf");
 /// A single classification entry for a symbol.
 pub const ClassificationEntry = struct {
     symbol: []const u8,
+    /// Human-readable security name (e.g., "Amazon", "SPDR S&P 500
+    /// ETF Trust"). Optional — older metadata.srf files may not
+    /// have this field. Renderers fall back to `symbol` /
+    /// `display_symbol` when null.
+    name: ?[]const u8 = null,
+    /// User-curated grouping label that overrides the auto-derived
+    /// bucket for concentration / dominance checks and the
+    /// analysis tab's Sector breakdown. Use this when the upstream
+    /// `sector` field is the NPORT-P "Equity / Corporate" mush
+    /// that doesn't actually distinguish your holdings (e.g. SPY
+    /// vs FRDM vs HFXI all tagged the same way). When null,
+    /// `deriveBucket` falls back to a sensible default.
+    bucket: ?[]const u8 = null,
     /// Sector (e.g., "Technology", "Healthcare", "Financials")
     sector: ?[]const u8 = null,
     /// Geographic region (e.g., "US", "International Developed", "Emerging Markets")
@@ -33,6 +46,8 @@ pub const ClassificationMap = struct {
     pub fn deinit(self: *ClassificationMap) void {
         for (self.entries) |e| {
             self.allocator.free(e.symbol);
+            if (e.name) |n| self.allocator.free(n);
+            if (e.bucket) |b| self.allocator.free(b);
             if (e.sector) |s| self.allocator.free(s);
             if (e.geo) |g| self.allocator.free(g);
             if (e.asset_class) |a| self.allocator.free(a);
@@ -42,13 +57,15 @@ pub const ClassificationMap = struct {
 };
 
 /// Parse a metadata SRF file into a ClassificationMap.
-/// Each record has: symbol::<SYM>,sector::<S>,geo::<G>,asset_class::<A>,pct:num:<P>
+/// Each record has: symbol::<SYM>,name::<N>,bucket::<B>,sector::<S>,geo::<G>,asset_class::<A>,pct:num:<P>
 /// All fields except symbol are optional. pct defaults to 100.
 pub fn parseClassificationFile(allocator: std.mem.Allocator, data: []const u8) !ClassificationMap {
     var entries = std.ArrayList(ClassificationEntry).empty;
     errdefer {
         for (entries.items) |e| {
             allocator.free(e.symbol);
+            if (e.name) |n| allocator.free(n);
+            if (e.bucket) |b| allocator.free(b);
             if (e.sector) |s| allocator.free(s);
             if (e.geo) |g| allocator.free(g);
             if (e.asset_class) |a| allocator.free(a);
@@ -62,8 +79,18 @@ pub fn parseClassificationFile(allocator: std.mem.Allocator, data: []const u8) !
 
     while (try it.next()) |fields| {
         const entry = fields.to(ClassificationEntry, .{}) catch continue;
+        // Pre-fill `bucket` if the user didn't curate one. This
+        // shifts the cost of `deriveBucket` to parse time and
+        // makes downstream code free to read `entry.bucket`
+        // directly without juggling allocator parameters.
+        const built_bucket: []const u8 = if (entry.bucket) |b|
+            try allocator.dupe(u8, b)
+        else
+            try deriveBucket(entry, allocator);
         try entries.append(allocator, .{
             .symbol = try allocator.dupe(u8, entry.symbol),
+            .name = if (entry.name) |n| try allocator.dupe(u8, n) else null,
+            .bucket = built_bucket,
             .sector = if (entry.sector) |s| try allocator.dupe(u8, s) else null,
             .geo = if (entry.geo) |g| try allocator.dupe(u8, g) else null,
             .asset_class = if (entry.asset_class) |a| try allocator.dupe(u8, a) else null,
@@ -77,16 +104,58 @@ pub fn parseClassificationFile(allocator: std.mem.Allocator, data: []const u8) !
     };
 }
 
+/// Resolve a classification entry to its display bucket. Used by
+/// the review tab's Sector column, by `analyzePortfolio`'s sector
+/// rollup, and by the observation engine's concentration /
+/// dominance checks.
+///
+/// Four-tier fallback (caller owns the returned slice; allocated
+/// via `allocator`):
+///   1. `entry.bucket` if set — user-curated, always wins.
+///   2. `entry.sector` if set AND doesn't contain '/' — GICS-style
+///      sector ("Technology", "Healthcare"). The '/' rules out
+///      NPORT-P fund-decomp categories ("Equity / Corporate")
+///      that are noise rather than meaningful sectors.
+///   3. Composite "<geo> <asset_class>" if both are set. For
+///      funds without a curated bucket, this gives a meaningful
+///      grouping like "International Developed Fund" or "US ETF".
+///   4. Literal "Unclassified".
+pub fn deriveBucket(entry: ClassificationEntry, allocator: std.mem.Allocator) ![]const u8 {
+    if (entry.bucket) |b| return try allocator.dupe(u8, b);
+    if (entry.sector) |s| {
+        if (std.mem.indexOfScalar(u8, s, '/') == null) return try allocator.dupe(u8, s);
+    }
+    if (entry.geo != null and entry.asset_class != null) {
+        const g = entry.geo.?;
+        const ac = entry.asset_class.?;
+        // Avoid duplicate-geo composites like "US US Large Cap".
+        // If the asset_class starts with the geo prefix (followed
+        // by a space or end-of-string), use it alone. Same for
+        // common geographic-noun asset classes that already imply
+        // their region ("International Developed", "Emerging
+        // Markets") — these don't need a geo prefix.
+        const ac_starts_with_geo = std.mem.startsWith(u8, ac, g) and
+            (ac.len == g.len or ac[g.len] == ' ');
+        const ac_has_implicit_geo = std.mem.startsWith(u8, ac, "International") or
+            std.mem.startsWith(u8, ac, "Emerging");
+        if (ac_starts_with_geo or ac_has_implicit_geo) {
+            return try allocator.dupe(u8, ac);
+        }
+        return try std.fmt.allocPrint(allocator, "{s} {s}", .{ g, ac });
+    }
+    return try allocator.dupe(u8, "Unclassified");
+}
+
 test "parse classification file" {
     const data =
         \\#!srfv1
         \\# Stock: single sector
-        \\symbol::AMZN,sector::Technology,geo::US,asset_class::US Large Cap
+        \\symbol::AMZN,name::Amazon,sector::Technology,geo::US,asset_class::US Large Cap
         \\
         \\# Target date fund: blended
-        \\symbol::TGT2035,asset_class::US Large Cap,pct:num:55
-        \\symbol::TGT2035,asset_class::Bonds,pct:num:15
-        \\symbol::TGT2035,asset_class::International Developed,pct:num:20
+        \\symbol::TGT2035,name::Target Retirement 2035,asset_class::US Large Cap,pct:num:55
+        \\symbol::TGT2035,name::Target Retirement 2035,asset_class::Bonds,pct:num:15
+        \\symbol::TGT2035,name::Target Retirement 2035,asset_class::International Developed,pct:num:20
     ;
     const allocator = std.testing.allocator;
     var cm = try parseClassificationFile(allocator, data);
@@ -94,15 +163,169 @@ test "parse classification file" {
 
     try std.testing.expectEqual(@as(usize, 4), cm.entries.len);
     try std.testing.expectEqualStrings("AMZN", cm.entries[0].symbol);
+    try std.testing.expectEqualStrings("Amazon", cm.entries[0].name.?);
     try std.testing.expectEqualStrings("Technology", cm.entries[0].sector.?);
     try std.testing.expectEqualStrings("US", cm.entries[0].geo.?);
     try std.testing.expectApproxEqAbs(@as(f64, 100.0), cm.entries[0].pct, 0.01);
 
     try std.testing.expectEqualStrings("TGT2035", cm.entries[1].symbol);
+    try std.testing.expectEqualStrings("Target Retirement 2035", cm.entries[1].name.?);
     try std.testing.expectEqualStrings("US Large Cap", cm.entries[1].asset_class.?);
     try std.testing.expectApproxEqAbs(@as(f64, 55.0), cm.entries[1].pct, 0.01);
 }
 
+test "parse classification file: missing name field stays null (backwards compat)" {
+    // Older metadata.srf files predate the name:: field. Parsing
+    // must still succeed; consumers fall back to symbol /
+    // display_symbol when name is null.
+    const data =
+        \\#!srfv1
+        \\symbol::AMZN,sector::Technology,geo::US,asset_class::US Large Cap
+    ;
+    const allocator = std.testing.allocator;
+    var cm = try parseClassificationFile(allocator, data);
+    defer cm.deinit();
+
+    try std.testing.expectEqual(@as(usize, 1), cm.entries.len);
+    try std.testing.expectEqualStrings("AMZN", cm.entries[0].symbol);
+    try std.testing.expect(cm.entries[0].name == null);
+    // `bucket` is pre-filled by the parser via deriveBucket. For
+    // a GICS-style sector ("Technology"), it equals the sector.
+    try std.testing.expectEqualStrings("Technology", cm.entries[0].bucket.?);
+    try std.testing.expectEqualStrings("Technology", cm.entries[0].sector.?);
+}
+
+test "parse classification file: bucket round-trips" {
+    const data =
+        \\#!srfv1
+        \\symbol::SPY,name::SPDR S&P 500 ETF Trust,bucket::US Large Cap,sector::Equity / Corporate,geo::US,asset_class::ETF
+    ;
+    const allocator = std.testing.allocator;
+    var cm = try parseClassificationFile(allocator, data);
+    defer cm.deinit();
+
+    try std.testing.expectEqual(@as(usize, 1), cm.entries.len);
+    try std.testing.expectEqualStrings("SPY", cm.entries[0].symbol);
+    try std.testing.expectEqualStrings("US Large Cap", cm.entries[0].bucket.?);
+    try std.testing.expectEqualStrings("Equity / Corporate", cm.entries[0].sector.?);
+}
+
+test "deriveBucket: returns user-curated bucket when set" {
+    const e: ClassificationEntry = .{
+        .symbol = "SPY",
+        .bucket = "US Large Cap",
+        .sector = "Equity / Corporate", // would otherwise force fallback
+        .geo = "US",
+        .asset_class = "ETF",
+    };
+    const out = try deriveBucket(e, std.testing.allocator);
+    defer std.testing.allocator.free(out);
+    try std.testing.expectEqualStrings("US Large Cap", out);
+}
+
+test "deriveBucket: returns sector when GICS-like (no '/')" {
+    const e: ClassificationEntry = .{
+        .symbol = "AMZN",
+        .sector = "Technology",
+        .geo = "US",
+        .asset_class = "US Large Cap",
+    };
+    const out = try deriveBucket(e, std.testing.allocator);
+    defer std.testing.allocator.free(out);
+    try std.testing.expectEqualStrings("Technology", out);
+}
+
+test "deriveBucket: composite fallback when sector is NPORT-P mush" {
+    const e: ClassificationEntry = .{
+        .symbol = "HFXI",
+        .sector = "Equity / Corporate",
+        .geo = "International Developed",
+        .asset_class = "Fund",
+    };
+    const out = try deriveBucket(e, std.testing.allocator);
+    defer std.testing.allocator.free(out);
+    try std.testing.expectEqualStrings("International Developed Fund", out);
+}
+
+test "deriveBucket: returns Unclassified when nothing usable is set" {
+    const e: ClassificationEntry = .{
+        .symbol = "UNK",
+    };
+    const out = try deriveBucket(e, std.testing.allocator);
+    defer std.testing.allocator.free(out);
+    try std.testing.expectEqualStrings("Unclassified", out);
+}
+
+test "deriveBucket: NPORT-P sector with no geo/asset_class falls through to Unclassified" {
+    // Defensive: sector is NPORT-P-style (skipped by the GICS
+    // filter) AND we don't have both geo and asset_class to
+    // build a composite. Falls through to Unclassified.
+    const e: ClassificationEntry = .{
+        .symbol = "X",
+        .sector = "Debt / Corporate",
+        .geo = "US",
+        // asset_class missing
+    };
+    const out = try deriveBucket(e, std.testing.allocator);
+    defer std.testing.allocator.free(out);
+    try std.testing.expectEqualStrings("Unclassified", out);
+}
+
+test "deriveBucket: composite avoids duplicate geo when asset_class already starts with it" {
+    // Hand-written entries often have geographically-prefixed
+    // asset_class values like "US Large Cap" alongside
+    // geo="US". The naive composite "{geo} {asset_class}" then
+    // produces "US US Large Cap" which is ugly and clusters
+    // incorrectly in the breakdown. Detect the duplicate prefix
+    // and use the asset_class alone.
+    const e: ClassificationEntry = .{
+        .symbol = "VOO",
+        .geo = "US",
+        .asset_class = "US Large Cap",
+    };
+    const out = try deriveBucket(e, std.testing.allocator);
+    defer std.testing.allocator.free(out);
+    try std.testing.expectEqualStrings("US Large Cap", out);
+}
+
+test "deriveBucket: composite uses asset_class alone for International/Emerging implicit-geo classes" {
+    // "International Developed" and "Emerging Markets" are
+    // already geographic; the composite shouldn't re-prepend
+    // the geo.
+    const e1: ClassificationEntry = .{
+        .symbol = "VEA",
+        .geo = "International Developed",
+        .asset_class = "International Developed",
+    };
+    const out1 = try deriveBucket(e1, std.testing.allocator);
+    defer std.testing.allocator.free(out1);
+    try std.testing.expectEqualStrings("International Developed", out1);
+
+    const e2: ClassificationEntry = .{
+        .symbol = "VWO",
+        .geo = "Emerging Markets",
+        .asset_class = "Emerging Markets",
+    };
+    const out2 = try deriveBucket(e2, std.testing.allocator);
+    defer std.testing.allocator.free(out2);
+    try std.testing.expectEqualStrings("Emerging Markets", out2);
+}
+
+test "deriveBucket: composite still prepends geo when asset_class is generic (Fund/ETF/Bonds)" {
+    // The whole point of the composite is to disambiguate
+    // generic asset_class labels by their geo. Make sure we
+    // don't accidentally regress on this case while fixing
+    // the duplicate-prefix one.
+    const e: ClassificationEntry = .{
+        .symbol = "BND",
+        .geo = "US",
+        .asset_class = "Fund",
+    };
+    const out = try deriveBucket(e, std.testing.allocator);
+    defer std.testing.allocator.free(out);
+    try std.testing.expectEqualStrings("US Fund", out);
+}
+
 // ── ClassificationRecord ─────────────────────────────────────
 //
 // Distinct from `ClassificationEntry` above: that one represents

src/net/RateLimiter.zig

@@ -64,9 +64,14 @@ pub fn tryAcquire(self: *RateLimiter) bool {
 /// Acquire a token, blocking (sleeping) until one is available.
 pub fn acquire(self: *RateLimiter) void {
     while (!self.tryAcquire()) {
-        // Sleep for the time needed to generate 1 token
+        // Sleep for the time needed to generate 1 token. An
+        // interrupted sleep (cancelation propagating through the
+        // Io) just loops back to tryAcquire — the next refill
+        // covers whatever fraction of the wait elapsed.
         const wait_ns: u64 = @intFromFloat(1.0 / self.refill_rate_per_ns);
-        std.Io.sleep(self.io, .{ .nanoseconds = @intCast(wait_ns) }, .awake) catch {};
+        std.Io.sleep(self.io, .{ .nanoseconds = @intCast(wait_ns) }, .awake) catch |err| {
+            std.log.scoped(.rate_limiter).debug("acquire sleep interrupted: {t}", .{err});
+        };
     }
 }
 
@@ -74,7 +79,11 @@ pub fn acquire(self: *RateLimiter) void {
 /// Use after receiving a server-side 429 to wait before retrying.
 pub fn backoff(self: *RateLimiter) void {
     const wait_ns: u64 = @max(self.estimateWaitNs(), 2 * std.time.ns_per_s);
-    std.Io.sleep(self.io, .{ .nanoseconds = @intCast(wait_ns) }, .awake) catch {};
+    // Interrupted backoff sleep degrades to a shorter wait; the
+    // caller's retry may hit 429 again and re-backoff.
+    std.Io.sleep(self.io, .{ .nanoseconds = @intCast(wait_ns) }, .awake) catch |err| {
+        std.log.scoped(.rate_limiter).debug("backoff sleep interrupted: {t}", .{err});
+    };
 }
 
 /// Returns estimated wait time in nanoseconds until a token is available.

src/padded.zig

@@ -28,9 +28,11 @@ pub fn Padded(comptime T: type) type {
             // we can measure its length and pad. 64 bytes covers
             // every realistic format-method output in this codebase
             // (Money's worst case is ~14 chars; Date is exactly 10).
+            // An inner value exceeding the buffer surfaces as
+            // WriteFailed to the caller rather than UB.
             var tmp: [64]u8 = undefined;
             var fixed = std.Io.Writer.fixed(&tmp);
-            self.inner.format(&fixed) catch unreachable;
+            try self.inner.format(&fixed);
             const text = fixed.buffered();
 
             const pad = if (text.len >= self.width) 0 else self.width - text.len;

src/portfolio_loader.zig

@@ -519,11 +519,10 @@ pub fn buildPortfolioData(
         candle_map.deinit();
     }
     for (syms) |sym| {
-        if (svc.getCachedCandles(sym)) |cs| {
-            // cs.data is owned by svc.allocator, which matches the
-            // caller's `allocator` in practice (they're wired to the
-            // same root). Store the raw slice; PortfolioData.deinit
-            // below frees via the caller's allocator.
+        if (svc.getCachedCandles(allocator, sym)) |cs| {
+            // cs.data is owned by the caller's `allocator`. Store
+            // the raw slice; PortfolioData.deinit (or the arena
+            // reset, in TUI) below frees via the same allocator.
             try candle_map.put(sym, cs.data);
         }
     }
@@ -743,7 +742,7 @@ fn gitInTestRepo(allocator: std.mem.Allocator, cwd: []const u8, argv: []const []
     defer allocator.free(result.stderr);
     switch (result.term) {
         .exited => |code| if (code != 0) {
-            std.debug.print("git command failed (code {d}): {s}\nstderr: {s}\n", .{ code, std.mem.join(allocator, " ", argv) catch "?", result.stderr });
+            std.log.err("git command failed (code {d}): {s}\nstderr: {s}", .{ code, std.mem.join(allocator, " ", argv) catch "?", result.stderr });
             return error.GitFailed;
         },
         else => return error.GitFailed,

src/providers/tiingo.zig

@@ -96,8 +96,11 @@ pub const Tiingo = struct {
     ) !CandleAndCorporateActions {
         var from_buf: [10]u8 = undefined;
         var to_buf: [10]u8 = undefined;
-        const from_str = std.fmt.bufPrint(&from_buf, "{f}", .{from}) catch unreachable;
-        const to_str = std.fmt.bufPrint(&to_buf, "{f}", .{to}) catch unreachable;
+        // Date's `{f}` output is exactly 10 bytes (YYYY-MM-DD), so
+        // these cannot fail in practice; `try` instead of
+        // `catch unreachable` to keep the error path honest.
+        const from_str = try std.fmt.bufPrint(&from_buf, "{f}", .{from});
+        const to_str = try std.fmt.bufPrint(&to_buf, "{f}", .{to});
 
         const symbol_url = try std.fmt.allocPrint(allocator, base_url ++ "/{s}/prices", .{symbol});
         defer allocator.free(symbol_url);

src/providers/twelvedata.zig

@@ -52,8 +52,11 @@ pub const TwelveData = struct {
 
         var from_buf: [10]u8 = undefined;
         var to_buf: [10]u8 = undefined;
-        const from_str = std.fmt.bufPrint(&from_buf, "{f}", .{from}) catch unreachable;
-        const to_str = std.fmt.bufPrint(&to_buf, "{f}", .{to}) catch unreachable;
+        // Date's `{f}` output is exactly 10 bytes (YYYY-MM-DD), so
+        // these cannot fail in practice; `try` instead of
+        // `catch unreachable` to keep the error path honest.
+        const from_str = try std.fmt.bufPrint(&from_buf, "{f}", .{from});
+        const to_str = try std.fmt.bufPrint(&to_buf, "{f}", .{to});
 
         // TwelveData's max outputsize is 5000 data points per request.
         // For daily candles this covers ~20 years of trading days (~252/year),

src/service.zig

@@ -445,7 +445,7 @@ pub const DataService = struct {
         // through to provider fetch. Skip-network does the opposite:
         // returns cached even if stale, never touches the network.
         if (!opts.force_refresh) {
-            if (s.read(T, symbol, postProcess, .fresh_only)) |cached| {
+            if (s.read(self.allocator, T, symbol, postProcess, .fresh_only)) |cached| {
                 log.debug("{s}: {s} fresh in local cache", .{ symbol, @tagName(data_type) });
                 return .{ .data = cached.data, .source = .cached, .timestamp = cached.timestamp, .allocator = self.allocator };
             }
@@ -454,7 +454,7 @@ pub const DataService = struct {
         if (opts.skip_network) {
             // Offline mode: return whatever's cached, even if stale.
             // Cache miss is FetchFailed (not a network error).
-            if (s.read(T, symbol, postProcess, .any)) |cached| {
+            if (s.read(self.allocator, T, symbol, postProcess, .any)) |cached| {
                 log.info("{s}: {s} stale-cached returned (skip_network)", .{ symbol, @tagName(data_type) });
                 return .{ .data = cached.data, .source = .cached, .timestamp = cached.timestamp, .allocator = self.allocator };
             }
@@ -463,7 +463,7 @@ pub const DataService = struct {
 
         // Try server sync before hitting providers (skipped on force_refresh).
         if (!opts.force_refresh and self.syncFromServer(symbol, data_type)) {
-            if (s.read(T, symbol, postProcess, .fresh_only)) |cached| {
+            if (s.read(self.allocator, T, symbol, postProcess, .fresh_only)) |cached| {
                 log.debug("{s}: {s} synced from server and fresh", .{ symbol, @tagName(data_type) });
                 return .{ .data = cached.data, .source = .cached, .timestamp = cached.timestamp, .allocator = self.allocator };
             }
@@ -742,7 +742,7 @@ pub const DataService = struct {
                     log.debug("{s}: skip_network and only TwelveData cached — treating as unavailable", .{symbol});
                     return DataError.FetchFailed;
                 }
-                if (s.read(Candle, symbol, null, .any)) |r| {
+                if (s.read(self.allocator, Candle, symbol, null, .any)) |r| {
                     if (!s.isCandleMetaFresh(symbol)) {
                         log.info("{s}: candles stale-cached returned (skip_network)", .{symbol});
                     }
@@ -758,7 +758,7 @@ pub const DataService = struct {
             } else if (!opts.force_refresh and s.isCandleMetaFresh(symbol)) {
                 // Fresh — deserialize candles and return
                 log.debug("{s}: candles fresh in local cache", .{symbol});
-                if (s.read(Candle, symbol, null, .any)) |r|
+                if (s.read(self.allocator, Candle, symbol, null, .any)) |r|
                     return .{ .data = r.data, .source = .cached, .timestamp = mr.created, .allocator = self.allocator };
             } else {
                 // Stale — try server sync before incremental fetch.
@@ -767,7 +767,7 @@ pub const DataService = struct {
                 if (!opts.force_refresh and self.syncCandlesFromServer(symbol)) {
                     if (s.isCandleMetaFresh(symbol)) {
                         log.debug("{s}: candles synced from server and fresh", .{symbol});
-                        if (s.read(Candle, symbol, null, .any)) |r|
+                        if (s.read(self.allocator, Candle, symbol, null, .any)) |r|
                             return .{ .data = r.data, .source = .cached, .timestamp = std.Io.Timestamp.now(self.io, .real).toSeconds(), .allocator = self.allocator };
                     }
                     log.debug("{s}: candles synced from server but stale, falling through to incremental fetch", .{symbol});
@@ -779,7 +779,7 @@ pub const DataService = struct {
                 // If last cached date is today or later, just refresh the TTL (meta only)
                 if (!fetch_from.lessThan(today)) {
                     s.updateCandleMeta(symbol, m.last_close, m.last_date, m.provider, m.fail_count);
-                    if (s.read(Candle, symbol, null, .any)) |r|
+                    if (s.read(self.allocator, Candle, symbol, null, .any)) |r|
                         return .{ .data = r.data, .source = .cached, .timestamp = std.Io.Timestamp.now(self.io, .real).toSeconds(), .allocator = self.allocator };
                 } else {
                     // Incremental fetch from day after last cached candle
@@ -794,13 +794,13 @@ pub const DataService = struct {
                             // If degraded (fail_count >= 3), return stale data rather than failing
                             if (new_fail_count >= 3) {
                                 log.warn("{s}: degraded after {d} consecutive failures, returning stale data", .{ symbol, new_fail_count });
-                                if (s.read(Candle, symbol, null, .any)) |r|
+                                if (s.read(self.allocator, Candle, symbol, null, .any)) |r|
                                     return .{ .data = r.data, .source = .cached, .timestamp = mr.created, .allocator = self.allocator };
                             }
                             return DataError.TransientError;
                         }
                         // Non-transient failure — return stale data if available
-                        if (s.read(Candle, symbol, null, .any)) |r|
+                        if (s.read(self.allocator, Candle, symbol, null, .any)) |r|
                             return .{ .data = r.data, .source = .cached, .timestamp = mr.created, .allocator = self.allocator };
                         return DataError.FetchFailed;
                     };
@@ -810,12 +810,12 @@ pub const DataService = struct {
                         // No new candles (weekend/holiday) — refresh TTL, reset fail_count
                         self.allocator.free(new_candles);
                         s.updateCandleMeta(symbol, m.last_close, m.last_date, result.provider, 0);
-                        if (s.read(Candle, symbol, null, .any)) |r|
+                        if (s.read(self.allocator, Candle, symbol, null, .any)) |r|
                             return .{ .data = r.data, .source = .cached, .timestamp = std.Io.Timestamp.now(self.io, .real).toSeconds(), .allocator = self.allocator };
                     } else {
                         // Append new candles to existing file + update meta, reset fail_count
                         s.appendCandles(symbol, new_candles, result.provider, 0);
-                        if (s.read(Candle, symbol, null, .any)) |r| {
+                        if (s.read(self.allocator, Candle, symbol, null, .any)) |r| {
                             self.allocator.free(new_candles);
                             return .{ .data = r.data, .source = .fetched, .timestamp = std.Io.Timestamp.now(self.io, .real).toSeconds(), .allocator = self.allocator };
                         }
@@ -835,7 +835,7 @@ pub const DataService = struct {
         if (!opts.force_refresh and self.syncCandlesFromServer(symbol)) {
             if (s.isCandleMetaFresh(symbol)) {
                 log.debug("{s}: candles synced from server and fresh (no prior cache)", .{symbol});
-                if (s.read(Candle, symbol, null, .any)) |r|
+                if (s.read(self.allocator, Candle, symbol, null, .any)) |r|
                     return .{ .data = r.data, .source = .cached, .timestamp = std.Io.Timestamp.now(self.io, .real).toSeconds(), .allocator = self.allocator };
             }
             log.debug("{s}: candles synced from server but stale, falling through to full fetch", .{symbol});
@@ -910,7 +910,7 @@ pub const DataService = struct {
         const today = fmt.todayDate(self.io);
 
         if (!opts.force_refresh) {
-            if (s.read(EarningsEvent, symbol, earningsPostProcess, .fresh_only)) |cached| {
+            if (s.read(self.allocator, EarningsEvent, symbol, earningsPostProcess, .fresh_only)) |cached| {
                 // Check if any past/today earnings event is still missing actual results.
                 // If so, the announcement likely just happened — force a refresh.
                 // (Suppressed when opts.skip_network — offline mode never refetches.)
@@ -929,7 +929,7 @@ pub const DataService = struct {
 
         if (opts.skip_network) {
             // Offline mode: fall back to any cached entry (even stale) before giving up.
-            if (s.read(EarningsEvent, symbol, earningsPostProcess, .any)) |cached| {
+            if (s.read(self.allocator, EarningsEvent, symbol, earningsPostProcess, .any)) |cached| {
                 log.info("{s}: earnings stale-cached returned (skip_network)", .{symbol});
                 return .{ .data = cached.data, .source = .cached, .timestamp = cached.timestamp, .allocator = self.allocator };
             }
@@ -938,7 +938,7 @@ pub const DataService = struct {
 
         // Try server sync before hitting FMP (skipped on force_refresh).
         if (!opts.force_refresh and self.syncFromServer(symbol, .earnings)) {
-            if (s.read(EarningsEvent, symbol, earningsPostProcess, .fresh_only)) |cached| {
+            if (s.read(self.allocator, EarningsEvent, symbol, earningsPostProcess, .fresh_only)) |cached| {
                 log.debug("{s}: earnings synced from server and fresh", .{symbol});
                 return .{ .data = cached.data, .source = .cached, .timestamp = cached.timestamp, .allocator = self.allocator };
             }
@@ -1097,14 +1097,14 @@ pub const DataService = struct {
         var s = self.store();
 
         if (!opts.force_refresh) {
-            if (s.read(Wikidata.ClassificationRecord, symbol, null, .fresh_only)) |cached| {
+            if (s.read(self.allocator, Wikidata.ClassificationRecord, symbol, null, .fresh_only)) |cached| {
                 log.debug("{s}: classification fresh in local cache", .{symbol});
                 return .{ .data = cached.data, .source = .cached, .timestamp = cached.timestamp, .allocator = self.allocator };
             }
         }
 
         if (opts.skip_network) {
-            if (s.read(Wikidata.ClassificationRecord, symbol, null, .any)) |cached| {
+            if (s.read(self.allocator, Wikidata.ClassificationRecord, symbol, null, .any)) |cached| {
                 log.info("{s}: classification stale-cached returned (skip_network)", .{symbol});
                 return .{ .data = cached.data, .source = .cached, .timestamp = cached.timestamp, .allocator = self.allocator };
             }
@@ -1113,7 +1113,7 @@ pub const DataService = struct {
 
         // Try server sync before hitting Wikidata.
         if (!opts.force_refresh and self.syncFromServer(symbol, .classification)) {
-            if (s.read(Wikidata.ClassificationRecord, symbol, null, .fresh_only)) |cached| {
+            if (s.read(self.allocator, Wikidata.ClassificationRecord, symbol, null, .fresh_only)) |cached| {
                 log.debug("{s}: classification synced from server", .{symbol});
                 return .{ .data = cached.data, .source = .cached, .timestamp = cached.timestamp, .allocator = self.allocator };
             }
@@ -1417,14 +1417,14 @@ pub const DataService = struct {
         var s = self.store();
 
         if (!opts.force_refresh) {
-            if (s.read(Edgar.EntityFactRecord, cik, null, .fresh_only)) |cached| {
+            if (s.read(self.allocator, Edgar.EntityFactRecord, cik, null, .fresh_only)) |cached| {
                 log.debug("CIK {s}: entity_facts fresh in local cache", .{cik});
                 return .{ .data = cached.data, .source = .cached, .timestamp = cached.timestamp, .allocator = self.allocator };
             }
         }
 
         if (opts.skip_network) {
-            if (s.read(Edgar.EntityFactRecord, cik, null, .any)) |cached| {
+            if (s.read(self.allocator, Edgar.EntityFactRecord, cik, null, .any)) |cached| {
                 log.info("CIK {s}: entity_facts stale-cached returned (skip_network)", .{cik});
                 return .{ .data = cached.data, .source = .cached, .timestamp = cached.timestamp, .allocator = self.allocator };
             }
@@ -1432,7 +1432,7 @@ pub const DataService = struct {
         }
 
         if (!opts.force_refresh and self.syncFromServer(cik, .entity_facts)) {
-            if (s.read(Edgar.EntityFactRecord, cik, null, .fresh_only)) |cached| {
+            if (s.read(self.allocator, Edgar.EntityFactRecord, cik, null, .fresh_only)) |cached| {
                 log.debug("CIK {s}: entity_facts synced from server", .{cik});
                 return .{ .data = cached.data, .source = .cached, .timestamp = cached.timestamp, .allocator = self.allocator };
             }
@@ -1491,7 +1491,7 @@ pub const DataService = struct {
         var s = self.store();
 
         if (!opts.force_refresh) {
-            if (s.read(Edgar.EtfMetricRecord, symbol, null, .fresh_only)) |cached| {
+            if (s.read(self.allocator, Edgar.EtfMetricRecord, symbol, null, .fresh_only)) |cached| {
                 log.debug("{s}: etf_metrics fresh in local cache", .{symbol});
                 return .{
                     .data = cached.data,
@@ -1503,7 +1503,7 @@ pub const DataService = struct {
         }
 
         if (opts.skip_network) {
-            if (s.read(Edgar.EtfMetricRecord, symbol, null, .any)) |cached| {
+            if (s.read(self.allocator, Edgar.EtfMetricRecord, symbol, null, .any)) |cached| {
                 log.info("{s}: etf_metrics stale-cached returned (skip_network)", .{symbol});
                 return .{
                     .data = cached.data,
@@ -1516,7 +1516,7 @@ pub const DataService = struct {
         }
 
         if (!opts.force_refresh and self.syncFromServer(symbol, .etf_metrics)) {
-            if (s.read(Edgar.EtfMetricRecord, symbol, null, .fresh_only)) |cached| {
+            if (s.read(self.allocator, Edgar.EtfMetricRecord, symbol, null, .fresh_only)) |cached| {
                 log.debug("{s}: etf_metrics synced from server", .{symbol});
                 return .{
                     .data = cached.data,
@@ -1618,7 +1618,7 @@ pub const DataService = struct {
         var s = self.store();
 
         if (!opts.force_refresh) {
-            if (s.read(Edgar.MutualFundTickerEntry, "_edgar", null, .fresh_only)) |cached| {
+            if (s.read(self.allocator, Edgar.MutualFundTickerEntry, "_edgar", null, .fresh_only)) |cached| {
                 if (cached.data.len > 0) {
                     return Edgar.TickerMap(Edgar.MutualFundTickerEntry).fromEntries(self.allocator, cached.data);
                 }
@@ -1646,7 +1646,7 @@ pub const DataService = struct {
         var s = self.store();
 
         if (!opts.force_refresh) {
-            if (s.read(Edgar.CompanyTickerEntry, "_edgar", null, .fresh_only)) |cached| {
+            if (s.read(self.allocator, Edgar.CompanyTickerEntry, "_edgar", null, .fresh_only)) |cached| {
                 if (cached.data.len > 0) {
                     return Edgar.TickerMap(Edgar.CompanyTickerEntry).fromEntries(self.allocator, cached.data);
                 }
@@ -1875,34 +1875,38 @@ pub const DataService = struct {
     /// Read candles from cache only (no network fetch). Used by TUI for display.
     /// Returns null if no cached data exists or if the entry is a negative cache (fetch_failed).
     ///
-    /// Returns a `FetchResult(Candle)` so the caller can `result.deinit()`
-    /// without needing to know the service's internal allocator.
-    pub fn getCachedCandles(self: *DataService, symbol: []const u8) ?FetchResult(Candle) {
+    /// `allocator` owns the returned `FetchResult.data`. Pass an
+    /// arena for "lives until reload" use cases (TUI per-portfolio
+    /// data); pass a per-call arena for CLI batch commands.
+    pub fn getCachedCandles(self: *DataService, allocator: std.mem.Allocator, symbol: []const u8) ?FetchResult(Candle) {
         var s = self.store();
         if (s.isNegative(symbol, .candles_daily)) return null;
-        const result = s.read(Candle, symbol, null, .any) orelse return null;
-        return .{ .data = result.data, .source = .cached, .timestamp = result.timestamp, .allocator = self.allocator };
+        const result = s.read(allocator, Candle, symbol, null, .any) orelse return null;
+        return .{ .data = result.data, .source = .cached, .timestamp = result.timestamp, .allocator = allocator };
     }
 
-    /// Read dividends from cache only (no network fetch).
-    pub fn getCachedDividends(self: *DataService, symbol: []const u8) ?[]Dividend {
+    /// Read dividends from cache only (no network fetch). See
+    /// `getCachedCandles` for the allocator contract.
+    pub fn getCachedDividends(self: *DataService, allocator: std.mem.Allocator, symbol: []const u8) ?FetchResult(Dividend) {
         var s = self.store();
-        const result = s.read(Dividend, symbol, null, .any) orelse return null;
-        return result.data;
+        const result = s.read(allocator, Dividend, symbol, null, .any) orelse return null;
+        return .{ .data = result.data, .source = .cached, .timestamp = result.timestamp, .allocator = allocator };
     }
 
-    /// Read earnings from cache only (no network fetch).
-    pub fn getCachedEarnings(self: *DataService, symbol: []const u8) ?[]EarningsEvent {
+    /// Read earnings from cache only (no network fetch). See
+    /// `getCachedCandles` for the allocator contract.
+    pub fn getCachedEarnings(self: *DataService, allocator: std.mem.Allocator, symbol: []const u8) ?FetchResult(EarningsEvent) {
         var s = self.store();
-        const result = s.read(EarningsEvent, symbol, earningsPostProcess, .any) orelse return null;
-        return result.data;
+        const result = s.read(allocator, EarningsEvent, symbol, earningsPostProcess, .any) orelse return null;
+        return .{ .data = result.data, .source = .cached, .timestamp = result.timestamp, .allocator = allocator };
     }
 
-    /// Read options from cache only (no network fetch).
-    pub fn getCachedOptions(self: *DataService, symbol: []const u8) ?[]OptionsChain {
+    /// Read options from cache only (no network fetch). See
+    /// `getCachedCandles` for the allocator contract.
+    pub fn getCachedOptions(self: *DataService, allocator: std.mem.Allocator, symbol: []const u8) ?FetchResult(OptionsChain) {
         var s = self.store();
-        const result = s.read(OptionsChain, symbol, null, .any) orelse return null;
-        return result.data;
+        const result = s.read(allocator, OptionsChain, symbol, null, .any) orelse return null;
+        return .{ .data = result.data, .source = .cached, .timestamp = result.timestamp, .allocator = allocator };
     }
 
     // ── Portfolio price loading ──────────────────────────────────
@@ -2028,8 +2032,6 @@ pub const DataService = struct {
         /// Drives `--refresh-data=never`.
         skip_network: bool = false,
         color: bool = true,
-        /// Maximum concurrent server sync requests. 0 = auto (8).
-        max_concurrent: usize = 0,
 
         /// Map this config to the per-call `FetchOptions` shape.
         /// Convenience for paths that need to pass through to
@@ -2206,7 +2208,6 @@ pub const DataService = struct {
                 needs_fetch.items,
                 &result,
                 &server_failures,
-                config,
                 aggregate_progress,
                 total_count,
             );
@@ -2238,27 +2239,32 @@ pub const DataService = struct {
         return result;
     }
 
-    /// Parallel server sync using thread pool.
+    /// Parallel server sync via `std.Io.Group`.
+    ///
+    /// Concurrency shape: one task per symbol, spawned into a
+    /// single `Group`. The `std.Io` implementation owns
+    /// scheduling and concurrency limits (e.g. `Io.Threaded`
+    /// sizes its pool from CPU count); we don't second-guess it
+    /// with our own worker cap or work-stealing queue.
+    ///
+    /// Each task hits `io.checkCancel()` before its sync, so a
+    /// cancelation request propagating through `Group.await`
+    /// stops pending work at task granularity.
     fn parallelServerSync(
         self: *DataService,
         symbols: []const []const u8,
         result: *LoadAllResult,
         failures: *std.ArrayList([]const u8),
-        config: LoadAllConfig,
         aggregate_progress: ?AggregateProgressCallback,
         total_count: usize,
     ) void {
-        const max_threads = if (config.max_concurrent > 0) config.max_concurrent else 8;
-        const thread_count = @min(symbols.len, max_threads);
-
         if (aggregate_progress) |p| p.emit(result.cached_count, total_count, .server_sync);
 
-        // Shared state for worker threads
+        // Shared state for tasks
         var completed = AtomicCounter{};
-        var next_index = AtomicCounter{};
         const sync_results = self.allocator.alloc(ServerSyncResult, symbols.len) catch {
             // Allocation failed — fall back to marking all as failures
-            for (symbols) |sym| failures.append(self.allocator, sym) catch |err| log.warn("parallelFetch slots-alloc-fallback failures append({s}): {t}", .{ sym, err });
+            for (symbols) |sym| failures.append(self.allocator, sym) catch |err| log.warn("parallelServerSync slots-alloc-fallback failures append({s}): {t}", .{ sym, err });
             return;
         };
         defer self.allocator.free(sync_results);
@@ -2268,62 +2274,40 @@ pub const DataService = struct {
             sr.* = .{ .symbol = symbols[i], .success = false };
         }
 
-        // Spawn worker threads
-        var threads = self.allocator.alloc(std.Thread, thread_count) catch {
-            for (symbols) |sym| failures.append(self.allocator, sym) catch |err| log.warn("parallelFetch threads-alloc-fallback failures append({s}): {t}", .{ sym, err });
-            return;
-        };
-        defer self.allocator.free(threads);
-
-        const WorkerContext = struct {
-            svc: *DataService,
-            symbols: []const []const u8,
-            results: []ServerSyncResult,
-            next_index: *AtomicCounter,
-            completed: *AtomicCounter,
-        };
-
-        var ctx = WorkerContext{
-            .svc = self,
-            .symbols = symbols,
-            .results = sync_results,
-            .next_index = &next_index,
-            .completed = &completed,
-        };
-
         const worker = struct {
-            fn run(wctx: *WorkerContext) void {
-                while (true) {
-                    const idx = wctx.next_index.increment();
-                    if (idx >= wctx.symbols.len) break;
-
-                    const sym = wctx.symbols[idx];
-                    const success = wctx.svc.syncCandlesFromServer(sym);
-                    wctx.results[idx].success = success;
-                    _ = wctx.completed.increment();
-                }
+            fn run(io: std.Io, svc: *DataService, slot: *ServerSyncResult, done: *AtomicCounter) std.Io.Cancelable!void {
+                defer _ = done.increment();
+                try io.checkCancel();
+                slot.success = svc.syncCandlesFromServer(slot.symbol);
             }
         };
 
-        // Start threads
-        var spawned: usize = 0;
-        for (threads) |*t| {
-            t.* = std.Thread.spawn(.{}, worker.run, .{&ctx}) catch continue;
-            spawned += 1;
+        // Spawn one task per symbol. Group.async requires an
+        // eventual Group.await/cancel to release resources; the
+        // single await below covers all paths.
+        var group: std.Io.Group = .init;
+        for (sync_results) |*sr| {
+            group.async(self.io, worker.run, .{ self.io, self, sr, &completed });
         }
 
-        // Progress reporting while waiting
+        // Progress reporting while the group runs
         if (aggregate_progress) |p| {
             while (completed.load() < symbols.len) {
-                std.Io.sleep(self.io, std.Io.Duration.fromMilliseconds(50), .awake) catch |err| log.debug("parallelFetch progress-poll sleep interrupted: {t}", .{err});
+                std.Io.sleep(self.io, std.Io.Duration.fromMilliseconds(50), .awake) catch |err| {
+                    log.debug("parallelServerSync progress-poll sleep interrupted: {t}", .{err});
+                    break;
+                };
                 p.emit(result.cached_count + completed.load(), total_count, .server_sync);
             }
         }
 
-        // Wait for all threads
-        for (threads[0..spawned]) |t| {
-            t.join();
-        }
+        // Wait for all tasks. On cancelation the unstarted tasks
+        // exit at their checkCancel point; partial results (slots
+        // that completed) are still processed below — they came
+        // from successful cache writes.
+        group.await(self.io) catch |err| {
+            log.debug("parallelServerSync group await: {t}", .{err});
+        };
 
         // Process results
         for (sync_results) |sr| {
@@ -2682,7 +2666,7 @@ pub const DataService = struct {
     /// Load and parse accounts.srf from the same directory as the given portfolio path.
     /// Returns null if the file doesn't exist or can't be parsed.
     /// Caller owns the returned AccountMap and must call deinit().
-    pub fn loadAccountMap(self: *DataService, portfolio_path: []const u8) ?analysis.AccountMap {
+    pub fn loadAccountMap(self: *DataService, allocator: std.mem.Allocator, portfolio_path: []const u8) ?analysis.AccountMap {
         const dir_end = if (std.mem.lastIndexOfScalar(u8, portfolio_path, std.fs.path.sep)) |idx| idx + 1 else 0;
         const acct_path = std.fmt.allocPrint(self.allocator, "{s}accounts.srf", .{portfolio_path[0..dir_end]}) catch return null;
         defer self.allocator.free(acct_path);
@@ -2690,7 +2674,7 @@ pub const DataService = struct {
         const data = std.Io.Dir.cwd().readFileAlloc(self.io, acct_path, self.allocator, .limited(1024 * 1024)) catch return null;
         defer self.allocator.free(data);
 
-        return analysis.parseAccountsFile(self.allocator, data) catch null;
+        return analysis.parseAccountsFile(allocator, data) catch null;
     }
 
     /// Load and parse `transaction_log.srf` from the same directory as

src/tui/analysis_tab.zig

@@ -10,12 +10,11 @@ const StyledLine = tui.StyledLine;
 
 // ── Tab-local action enum ─────────────────────────────────────
 //
-// Cycle the Sector breakdown's display granularity through
-// coarse → mid → fine → coarse. Default tier is mid (~12-16
-// buckets, NPORT-P sub-flavors collapsed but GICS sectors
-// distinct). Coarse delegates to the same 4-bucket shape as
-// the Asset Category section. Fine is the raw NPORT-P
-// breakdown (every Debt / X variant separate).
+// Toggle the Sector breakdown's display granularity between
+// coarse (4 macro buckets — Equity / Fixed Income / Cash /
+// Other) and fine (one row per bucket label, the default).
+// Coarse delegates to the same 4-bucket shape as the Asset
+// Category section.
 
 pub const Action = enum { cycle_sector_granularity };
 
@@ -28,15 +27,10 @@ pub const State = struct {
     /// Computed analysis output. Owned by State; freed in
     /// `deinit` and `reload`.
     result: ?zfin.analysis.AnalysisResult = null,
-    /// Per-portfolio classification metadata (`metadata.srf`).
-    /// Used only by analysis today; lives here because no other
-    /// tab consumes it. Loaded lazily on first activation; freed
-    /// in `deinit`.
-    classification_map: ?zfin.classification.ClassificationMap = null,
-    /// Sector display granularity. Cycled via `cycle_sector_granularity`
-    /// action. Default `mid` matches the CLI default
+    /// Sector display granularity. Toggled via `cycle_sector_granularity`
+    /// action. Default `fine` matches the CLI default
     /// (`zfin analysis` without `--sector-detail`).
-    sector_granularity: zfin.analysis.Granularity = .mid,
+    sector_granularity: zfin.analysis.Granularity = .fine,
 };
 
 // ── Tab framework contract ────────────────────────────────────
@@ -47,7 +41,7 @@ pub const meta: framework.TabMeta(Action) = .{
         .{ .action = .cycle_sector_granularity, .key = .{ .codepoint = 'm' } },
     },
     .action_labels = std.enums.EnumArray(Action, []const u8).init(.{
-        .cycle_sector_granularity = "Cycle sector granularity (coarse / mid / fine)",
+        .cycle_sector_granularity = "Toggle sector granularity (coarse / fine)",
     }),
     .status_hints = &.{
         .cycle_sector_granularity,
@@ -65,7 +59,6 @@ pub const tab = struct {
 
     pub fn deinit(state: *State, app: *App) void {
         if (state.result) |*ar| ar.deinit(app.allocator);
-        if (state.classification_map) |*cm| cm.deinit();
         state.* = .{};
     }
 
@@ -78,18 +71,16 @@ pub const tab = struct {
     pub const deactivate = framework.noopDeactivate(State);
 
     /// Force re-fetch on user request. Frees the analysis result
-    /// AND the shared `account_map` on App (analysis's refresh
-    /// also re-reads accounts.srf). The classification_map persists
-    /// — it's per-portfolio, not per-symbol or per-refresh.
+    /// and invalidates the shared `account_map` + `classification_map`
+    /// on PortfolioData so the next load re-reads `accounts.srf`
+    /// and `metadata.srf` from disk (the user may have edited
+    /// either).
     pub fn reload(state: *State, app: *App) !void {
         if (state.result) |*ar| ar.deinit(app.allocator);
         state.result = null;
         state.loaded = false;
-        // Refresh-analysis intentionally drops the shared account
-        // map so the next load re-reads `accounts.srf` from disk
-        // (the user may have edited it).
-        if (app.portfolio.account_map) |*am| am.deinit();
-        app.portfolio.account_map = null;
+        app.portfolio.invalidateAccountMap();
+        app.portfolio.invalidateClassificationMap();
         loadData(state, app);
     }
 
@@ -99,12 +90,11 @@ pub const tab = struct {
         _ = app;
         switch (action) {
             .cycle_sector_granularity => {
-                // coarse → mid → fine → coarse. The display layer
+                // Binary toggle: coarse ↔ fine. The display layer
                 // re-aggregates `result.sector` through the new
                 // granularity on the next render.
                 state.sector_granularity = switch (state.sector_granularity) {
-                    .coarse => .mid,
-                    .mid => .fine,
+                    .coarse => .fine,
                     .fine => .coarse,
                 };
             },
@@ -119,6 +109,34 @@ pub const tab = struct {
     pub fn isDisabled(app: *App) bool {
         return app.portfolio.file == null;
     }
+
+    /// Drop cached analysis result on portfolio reload. The
+    /// `result` holds pointers into the previous portfolio's
+    /// memory (allocations/symbols), so we have to invalidate
+    /// it before the underlying data is freed.
+    ///
+    /// `classification_map` and `account_map` live on
+    /// PortfolioData and are reset by pd's own load path —
+    /// nothing to free here for those.
+    ///
+    /// Deliberately does NOT eager-rebuild, even when this tab
+    /// is active. The reload broadcast fires BEFORE pd.reload
+    /// resets the portfolio arena; rebuilding now would store
+    /// borrowed pointers (e.g. BreakdownItem.label borrowing
+    /// from ClassificationEntry.bucket) that get freed seconds
+    /// later when the arena resets, producing use-after-free
+    /// crashes on the next draw. The orchestrator
+    /// (portfolio_tab.reloadPortfolioFile) calls
+    /// `app.loadTabData()` AFTER pd.reload completes, which
+    /// re-activates the active tab against fresh data.
+    pub fn onPortfolioReload(state: *State, app: *App) void {
+        if (state.result) |*ar| ar.deinit(app.allocator);
+        state.result = null;
+        // classification_map lives on PortfolioData now; the
+        // reload's broadcast already invalidated it via pd's own
+        // reset path. Nothing to free here.
+        state.loaded = false;
+    }
 };
 
 // ── Data loading ──────────────────────────────────────────────
@@ -126,41 +144,20 @@ pub const tab = struct {
 fn loadData(state: *State, app: *App) void {
     state.loaded = true;
 
-    // Ensure portfolio is loaded first
-    app.ensurePortfolioDataLoaded();
+    // PortfolioData was loaded at App init; sync fields are
+    // populated. If they're still null, no portfolio is loaded
+    // (welcome screen) and we exit silently.
     const pf = app.portfolio.file orelse return;
-    const summary = app.portfolio.summary orelse return;
+    const summary_ptr = if (app.portfolio.summary) |*s| s else return;
 
-    // Load classification metadata file
-    if (state.classification_map == null) {
-        // Look for metadata.srf next to the portfolio file
-        if (app.anchorPath()) |ppath| {
-            // Derive metadata path: same directory as portfolio, named "metadata.srf"
-            const dir_end = if (std.mem.lastIndexOfScalar(u8, ppath, std.fs.path.sep)) |idx| idx + 1 else 0;
-            const meta_path = std.fmt.allocPrint(app.allocator, "{s}metadata.srf", .{ppath[0..dir_end]}) catch return;
-            defer app.allocator.free(meta_path);
-
-            const file_data = std.Io.Dir.cwd().readFileAlloc(app.io, meta_path, app.allocator, .limited(1024 * 1024)) catch {
-                app.setStatus("No metadata.srf found. Run: zfin enrich <portfolio.srf> > metadata.srf");
-                return;
-            };
-            defer app.allocator.free(file_data);
-
-            state.classification_map = zfin.classification.parseClassificationFile(app.allocator, file_data) catch {
-                app.setStatus("Error parsing metadata.srf");
-                return;
-            };
-        }
-    }
-
-    // Load account tax type metadata file (optional)
-    app.ensureAccountMap();
-
-    loadDataFinish(state, app, pf, summary);
+    loadDataFinish(state, app, pf, summary_ptr.*);
 }
 
 fn loadDataFinish(state: *State, app: *App, pf: zfin.Portfolio, summary: zfin.valuation.PortfolioSummary) void {
-    const cm = state.classification_map orelse {
+    // classificationMap() blocks on the classification-map
+    // worker; first call may briefly wait while metadata.srf
+    // parses. Returns null when metadata.srf is missing.
+    const cm_ptr = app.portfolio.classificationMap() orelse {
         app.setStatus("No classification data. Run: zfin enrich <portfolio.srf> > metadata.srf");
         return;
     };
@@ -168,13 +165,17 @@ fn loadDataFinish(state: *State, app: *App, pf: zfin.Portfolio, summary: zfin.va
     // Free previous result
     if (state.result) |*ar| ar.deinit(app.allocator);
 
+    // accountMap() blocks on the worker future — first call may
+    // briefly wait if the worker is still finishing. After this,
+    // subsequent calls are sync.
+    const acct_map_opt: ?zfin.analysis.AccountMap = if (app.portfolio.accountMap()) |amp| amp.* else null;
     state.result = zfin.analysis.analyzePortfolio(
         app.allocator,
         summary.allocations,
-        cm,
+        cm_ptr.*,
         pf,
         summary.total_value,
-        app.portfolio.account_map,
+        acct_map_opt,
         app.today, // live mode in TUI → resolves to app.today
     ) catch {
         app.setStatus("Error computing analysis");
@@ -194,7 +195,8 @@ pub fn buildStyledLines(state: *State, app: *App, arena: std.mem.Allocator) ![]c
         total_value = summary.total_value;
         if (app.portfolio.file) |pf| {
             const benchmark = @import("../analytics/benchmark.zig");
-            const cm_entries = if (state.classification_map) |cm| cm.entries else &.{};
+            const cm_entries: []const zfin.classification.ClassificationEntry =
+                if (app.portfolio.classificationMap()) |cm| cm.entries else &.{};
             const split = benchmark.deriveAllocationSplit(
                 summary.allocations,
                 cm_entries,
@@ -207,7 +209,11 @@ pub fn buildStyledLines(state: *State, app: *App, arena: std.mem.Allocator) ![]c
             cash_pct = split.cash_pct;
         }
     }
-    return renderAnalysisLines(arena, app.theme, state.result, stock_pct, bond_pct, cash_pct, total_value, state.sector_granularity, app.portfolio.account_map);
+    // accountMap() blocks on its worker; first call may briefly
+    // wait. Returns null when no portfolio is loaded or
+    // accounts.srf is missing.
+    const acct_map_opt: ?zfin.analysis.AccountMap = if (app.portfolio.accountMap()) |amp| amp.* else null;
+    return renderAnalysisLines(arena, app.theme, state.result, stock_pct, bond_pct, cash_pct, total_value, state.sector_granularity, acct_map_opt);
 }
 
 /// Render analysis tab content. Pure function — no App dependency.
@@ -271,7 +277,6 @@ pub fn renderAnalysisLines(
     // gets freed at tab.deinit time.
     const display_result: zfin.analysis.AnalysisResult = .{
         .asset_category = result.asset_category,
-        .asset_class = result.asset_class,
         .sector = collapsed_sector,
         .geo = result.geo,
         .account = result.account,
@@ -289,7 +294,7 @@ pub fn renderAnalysisLines(
         // section's title string). For the Sector section,
         // append the current granularity in parens so the user
         // knows what the `g` hot-key cycled to.
-        const title_text = if (std.mem.eql(u8, sec.title, "Sector (Equities)"))
+        const title_text = if (std.mem.eql(u8, sec.title, "Sector"))
             try std.fmt.allocPrint(arena, "  Sector ({s} — press 'm' to cycle)", .{granularityLabel(sector_granularity)})
         else
             try std.fmt.allocPrint(arena, "  {s}", .{sec.title});
@@ -361,7 +366,6 @@ pub fn renderUmbrellaSection(
 fn granularityLabel(g: zfin.analysis.Granularity) []const u8 {
     return switch (g) {
         .coarse => "coarse",
-        .mid => "mid",
         .fine => "fine",
     };
 }
@@ -446,22 +450,26 @@ test "renderAnalysisLines with data" {
     const arena = arena_state.allocator();
     const th = theme.default_theme;
 
-    var asset_class = [_]zfin.analysis.BreakdownItem{
-        .{ .label = "US Stock", .weight = 0.60, .value = 120000 },
-        .{ .label = "Int'l Stock", .weight = 0.40, .value = 80000 },
+    // Use sector breakdown (the main fine-grained slice) as the
+    // populated section for this test. asset_class is gone — its
+    // role is subsumed by the bucket-driven `sector` field.
+    // GICS-style labels are stable through `bucketSector` and
+    // are the natural shape for direct GICS-tagged equities.
+    var sector = [_]zfin.analysis.BreakdownItem{
+        .{ .label = "Technology", .weight = 0.60, .value = 120000 },
+        .{ .label = "Healthcare", .weight = 0.40, .value = 80000 },
     };
     const result = zfin.analysis.AnalysisResult{
         .asset_category = &.{},
-        .asset_class = &asset_class,
-        .sector = &.{},
+        .sector = &sector,
         .geo = &.{},
         .account = &.{},
         .tax_type = &.{},
         .unclassified = &.{},
         .total_value = 200000,
     };
-    const lines = try renderAnalysisLines(arena, th, result, 0.80, 0.15, 0.05, 200000, .mid, null);
-    // Should have header section + asset class items
+    const lines = try renderAnalysisLines(arena, th, result, 0.80, 0.15, 0.05, 200000, .fine, null);
+    // Should have header section + sector items
     try testing.expect(lines.len >= 5);
     // Find "Portfolio Analysis" header
     var found_header = false;
@@ -472,12 +480,12 @@ test "renderAnalysisLines with data" {
     }
     try testing.expect(found_header);
     try testing.expect(found_cash_in_summary);
-    // Find asset class data
-    var found_us = false;
+    // Find sector breakdown data
+    var found_tech = false;
     for (lines) |l| {
-        if (std.mem.indexOf(u8, l.text, "US Stock") != null) found_us = true;
+        if (std.mem.indexOf(u8, l.text, "Technology") != null) found_tech = true;
     }
-    try testing.expect(found_us);
+    try testing.expect(found_tech);
 }
 
 test "renderAnalysisLines no data" {
@@ -486,7 +494,7 @@ test "renderAnalysisLines no data" {
     const arena = arena_state.allocator();
     const th = theme.default_theme;
 
-    const lines = try renderAnalysisLines(arena, th, null, 0, 0, 0, 0, .mid, null);
+    const lines = try renderAnalysisLines(arena, th, null, 0, 0, 0, 0, .fine, null);
     try testing.expectEqual(@as(usize, 5), lines.len);
     try testing.expect(std.mem.indexOf(u8, lines[3].text, "No analysis data") != null);
 }
@@ -499,25 +507,46 @@ test "tab.init produces zero-defaulted state" {
     try tab.init(&state, &dummy_app);
     try testing.expectEqual(false, state.loaded);
     try testing.expect(state.result == null);
-    try testing.expect(state.classification_map == null);
-    // Default sector granularity is mid (matches CLI default).
-    try testing.expectEqual(zfin.analysis.Granularity.mid, state.sector_granularity);
+    // classification_map lives on PortfolioData now (not on tab state).
+    // Default sector granularity is fine (matches CLI default).
+    try testing.expectEqual(zfin.analysis.Granularity.fine, state.sector_granularity);
 }
 
-test "handleAction cycles sector granularity coarse → mid → fine → coarse" {
+test "onPortfolioReload clears state without eager rebuild" {
+    // Regression for the use-after-free crash where eager-rebuild
+    // inside onPortfolioReload stored borrowed pointers from the
+    // about-to-be-freed portfolio arena. Contract: this hook
+    // MUST clear state and set loaded=false, but MUST NOT call
+    // activate() or otherwise rebuild from app.portfolio (which
+    // is about to be reset by pd.reload).
+    //
+    // We can't easily verify "no eager rebuild" mechanically
+    // without a full App harness, but we CAN verify the
+    // post-conditions are exactly state-cleared.
+    var state: State = .{
+        .loaded = true,
+        // result stays null — non-null would need allocator to free.
+    };
+    var dummy_app: tui.App = undefined; // not touched when result is null
+
+    tab.onPortfolioReload(&state, &dummy_app);
+
+    try testing.expectEqual(false, state.loaded);
+    try testing.expect(state.result == null);
+}
+
+test "handleAction toggles sector granularity fine ↔ coarse" {
     var state: State = .{};
     var dummy_app: tui.App = undefined; // handleAction doesn't touch app
 
-    // mid → fine
-    try testing.expectEqual(zfin.analysis.Granularity.mid, state.sector_granularity);
-    tab.handleAction(&state, &dummy_app, .cycle_sector_granularity);
+    // Default is fine.
     try testing.expectEqual(zfin.analysis.Granularity.fine, state.sector_granularity);
     // fine → coarse
     tab.handleAction(&state, &dummy_app, .cycle_sector_granularity);
     try testing.expectEqual(zfin.analysis.Granularity.coarse, state.sector_granularity);
-    // coarse → mid (full cycle)
+    // coarse → fine (full cycle)
     tab.handleAction(&state, &dummy_app, .cycle_sector_granularity);
-    try testing.expectEqual(zfin.analysis.Granularity.mid, state.sector_granularity);
+    try testing.expectEqual(zfin.analysis.Granularity.fine, state.sector_granularity);
 }
 
 test "renderAnalysisLines: granularity label appears in Sector section title" {
@@ -532,7 +561,6 @@ test "renderAnalysisLines: granularity label appears in Sector section title" {
     };
     const result = zfin.analysis.AnalysisResult{
         .asset_category = &.{},
-        .asset_class = &.{},
         .sector = &sector,
         .geo = &.{},
         .account = &.{},
@@ -541,15 +569,6 @@ test "renderAnalysisLines: granularity label appears in Sector section title" {
         .total_value = 100_000,
     };
 
-    // mid label
-    {
-        const lines = try renderAnalysisLines(arena, th, result, 0.80, 0.20, 0.0, 100_000, .mid, null);
-        var found_mid = false;
-        for (lines) |l| {
-            if (std.mem.indexOf(u8, l.text, "Sector (mid") != null) found_mid = true;
-        }
-        try testing.expect(found_mid);
-    }
     // fine label
     {
         const lines = try renderAnalysisLines(arena, th, result, 0.80, 0.20, 0.0, 100_000, .fine, null);
@@ -570,43 +589,6 @@ test "renderAnalysisLines: granularity label appears in Sector section title" {
     }
 }
 
-test "renderAnalysisLines: mid granularity collapses Debt rows" {
-    var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
-    defer arena_state.deinit();
-    const arena = arena_state.allocator();
-    const th = theme.default_theme;
-
-    // Two Debt rows that should collapse to one Bonds row at mid.
-    var sector = [_]zfin.analysis.BreakdownItem{
-        .{ .label = "Debt / Corporate", .weight = 0.40, .value = 40_000 },
-        .{ .label = "Debt / US Treasury", .weight = 0.30, .value = 30_000 },
-        .{ .label = "Equity / Corporate", .weight = 0.30, .value = 30_000 },
-    };
-    const result = zfin.analysis.AnalysisResult{
-        .asset_category = &.{},
-        .asset_class = &.{},
-        .sector = &sector,
-        .geo = &.{},
-        .account = &.{},
-        .tax_type = &.{},
-        .unclassified = &.{},
-        .total_value = 100_000,
-    };
-
-    const lines = try renderAnalysisLines(arena, th, result, 0.30, 0.70, 0.0, 100_000, .mid, null);
-
-    // At mid, Bonds appears (collapsed) and the individual Debt
-    // rows do NOT appear.
-    var has_bonds_row = false;
-    var has_raw_treasury = false;
-    for (lines) |l| {
-        if (std.mem.indexOf(u8, l.text, "Bonds") != null and std.mem.indexOf(u8, l.text, "70.0%") != null) has_bonds_row = true;
-        if (std.mem.indexOf(u8, l.text, "Debt / US Treasury") != null) has_raw_treasury = true;
-    }
-    try testing.expect(has_bonds_row);
-    try testing.expect(!has_raw_treasury);
-}
-
 test "renderAnalysisLines: umbrella section appears at the bottom when account_map provided" {
     var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
     defer arena_state.deinit();
@@ -619,7 +601,6 @@ test "renderAnalysisLines: umbrella section appears at the bottom when account_m
     };
     const result = zfin.analysis.AnalysisResult{
         .asset_category = &.{},
-        .asset_class = &.{},
         .sector = &.{},
         .geo = &.{},
         .account = &account,
@@ -635,7 +616,7 @@ test "renderAnalysisLines: umbrella section appears at the bottom when account_m
     );
     defer am.deinit();
 
-    const lines = try renderAnalysisLines(arena, th, result, 0.80, 0.0, 0.0, 100_000, .mid, am);
+    const lines = try renderAnalysisLines(arena, th, result, 0.80, 0.0, 0.0, 100_000, .fine, am);
 
     var found_header = false;
     var found_total_liquid = false;
@@ -667,7 +648,6 @@ test "renderAnalysisLines: umbrella section absent when account_map is null" {
     };
     const result = zfin.analysis.AnalysisResult{
         .asset_category = &.{},
-        .asset_class = &.{},
         .sector = &.{},
         .geo = &.{},
         .account = &account,
@@ -676,7 +656,7 @@ test "renderAnalysisLines: umbrella section absent when account_map is null" {
         .total_value = 100_000,
     };
 
-    const lines = try renderAnalysisLines(arena, th, result, 1.0, 0, 0, 100_000, .mid, null);
+    const lines = try renderAnalysisLines(arena, th, result, 1.0, 0, 0, 100_000, .fine, null);
 
     for (lines) |l| {
         try testing.expect(std.mem.indexOf(u8, l.text, "Umbrella exposure") == null);
@@ -698,7 +678,6 @@ test "renderAnalysisLines: umbrella respects shielded:bool:false override (DCP c
     };
     const result = zfin.analysis.AnalysisResult{
         .asset_category = &.{},
-        .asset_class = &.{},
         .sector = &.{},
         .geo = &.{},
         .account = &account,
@@ -714,7 +693,7 @@ test "renderAnalysisLines: umbrella respects shielded:bool:false override (DCP c
     );
     defer am.deinit();
 
-    const lines = try renderAnalysisLines(arena, th, result, 1.0, 0, 0, 2_500_000, .mid, am);
+    const lines = try renderAnalysisLines(arena, th, result, 1.0, 0, 0, 2_500_000, .fine, am);
 
     // IRA shielded ($1M); DCP exposed ($1.5M). Total $2.5M.
     var found_shielded_1m = false;

src/tui/history_tab.zig

@@ -152,6 +152,8 @@ pub const State = struct {
     /// Per-bucket expansion set. Keyed by `BucketKey` (tier + days)
     /// to disambiguate edge-aligned parents and children. Initialized
     /// in `init` (requires an allocator).
+    // SAFETY: overwritten by `init()` before any read; the framework
+    // contract guarantees `init` runs before `activate`/draw paths.
     expanded_buckets: std.AutoHashMap(BucketKey, void) = undefined,
 };
 
@@ -200,10 +202,8 @@ pub const tab = struct {
 
     pub fn activate(state: *State, app: *App) !void {
         if (state.loaded) return;
-        // History reads `app.portfolio.summary` and `.file`.
-        // Ensure they're populated even when the user jumps
-        // straight here without visiting portfolio first.
-        app.ensurePortfolioDataLoaded();
+        // History reads `app.portfolio.summary` and `.file`,
+        // both populated synchronously by pd.load at App init.
         loadData(state, app);
     }
 
@@ -215,6 +215,33 @@ pub const tab = struct {
         loadData(state, app);
     }
 
+    /// Drop cached timeline and compare view on portfolio reload.
+    /// `state.tl` borrows symbol strings from the previous
+    /// portfolio's memory, and `compare_resources.then_live_map`
+    /// / `now_live_map` borrow keys from `app.portfolio` — all
+    /// of which `pd.reload` is about to free. Drop them now.
+    ///
+    /// Deliberately does NOT eager-rebuild, even when this tab
+    /// is active. The reload broadcast fires BEFORE pd.reload
+    /// resets the portfolio arena; rebuilding now would store
+    /// borrowed pointers that get freed seconds later. The
+    /// orchestrator (portfolio_tab.reloadPortfolioFile) calls
+    /// `app.loadTabData()` AFTER pd.reload completes, which
+    /// re-activates the active tab against fresh data.
+    ///
+    /// Selections + cursor are reset because the row indices
+    /// they refer to may no longer be valid after the new
+    /// timeline is loaded (different snapshot count, etc.).
+    pub fn onPortfolioReload(state: *State, app: *App) void {
+        freeLoaded(state, app);
+        state.loaded = false;
+        state.cursor = 0;
+        state.selections = .{ null, null };
+        state.table_first_line = 0;
+        state.table_row_count = 0;
+        state.expanded_buckets.clearRetainingCapacity();
+    }
+
     pub const tick = framework.noopTick(State);
 
     pub fn handleAction(state: *State, app: *App, action: Action) void {
@@ -288,6 +315,17 @@ pub const tab = struct {
         return true;
     }
 
+    /// Mouse wheel: always scroll the viewport, never move the
+    /// cursor. Keeps wheel-as-look-around and cursor-as-pointer
+    /// distinct. The framework falls through to viewport scroll
+    /// when this returns false.
+    pub fn onWheelMove(state: *State, app: *App, delta: isize) bool {
+        _ = state;
+        _ = app;
+        _ = delta;
+        return false;
+    }
+
     /// Mouse handling: a left-click on a row moves the cursor and
     /// toggles tier expansion (no-op for non-tier rows). Returns
     /// `true` if the click landed on a data row in the recent-

src/tui/input_buffer.zig

@@ -1,21 +1,45 @@
 //! Shared input-buffer state machine for the TUI's modal text
-//! prompts (symbol input, projections' as-of date input, etc).
+//! prompts (symbol input, projections' as-of date input, ack notes).
 //!
-//! Pure free function over `(buf, len_ptr, key)` — no App or
-//! tab-state coupling. Callers own:
+//! Two flavors:
 //!
-//! - The byte buffer (typically a fixed-size `[16]u8`).
+//! - `handleKey` — single-line input. Enter commits.
+//! - `handleKeyMulti` — multi-fragment input. Enter completes a
+//!   *fragment* (without committing); Ctrl+Enter commits the whole
+//!   accumulated input. Used by the review tab's ack-note flow,
+//!   where multi-line reasoning is decomposed into N journal note
+//!   records.
+//!
+//! Both are pure free functions over `(buf, len_ptr, key)` — no App
+//! or tab-state coupling. Callers own:
+//!
+//! - The byte buffer (typically a fixed-size `[16]u8` or larger).
 //! - The `len: *usize` cursor into it.
-//! - Mode/modal cleanup on `cancelled` and `committed` results.
+//! - Mode/modal cleanup on `cancelled`/`committed` results.
 //! - Side effects (status messages, downstream dispatch, etc).
+//! - For `handleKeyMulti`: the accumulated fragment list. The
+//!   state machine signals "fragment complete" via `.fragment`;
+//!   the caller copies `buf[0..len]` into its fragment list and
+//!   resets `len.*` to 0 before the next call.
 //!
-//! The state machine handles only:
-//! - Esc → reset `len` to 0, return `.cancelled`.
-//! - Enter → return `.committed` (caller reads `buf[0..len]`).
-//! - Backspace → decrement `len`.
-//! - Ctrl+U → reset `len` to 0 (readline-style clear).
-//! - Printable ASCII → append byte, increment `len` (capped at
-//!   buffer length).
+//! ## Keybind philosophy
+//!
+//! Modal input keys (Esc, Enter, Backspace, Ctrl+U, Ctrl+Enter) are
+//! **hardcoded here** and intentionally NOT routed through the
+//! tab-framework's keybinds system. The keybinds system is for
+//! *actions* the user wires to whatever key they want; modal-input
+//! mechanics are part of the input idiom itself, like vim's `:`
+//! command-mode keys aren't user-configurable. If a user really
+//! wants different keys for "submit my note", that's a TODO entry
+//! against this file (low priority — nobody's asked).
+//!
+//! Ctrl+Enter as the multi-fragment commit key is the universal
+//! "submit multi-line text" idiom (Slack, Discord, Notion, GitHub
+//! comments). Some legacy terminals can't distinguish Ctrl+Enter
+//! from plain Enter — they send the same byte sequence. For those,
+//! Ctrl+D is accepted as a fallback so the feature still works on
+//! every terminal we ship to. The doc/help text only mentions
+//! Ctrl+Enter as the primary; Ctrl+D is undocumented but functional.
 
 const std = @import("std");
 const vaxis = @import("vaxis");
@@ -65,6 +89,80 @@ pub fn handleKey(buf: []u8, len: *usize, key: vaxis.Key) Result {
     return .ignored;
 }
 
+/// Outcome of one `handleKeyMulti` call. Adds `.fragment` to the
+/// single-line variants; otherwise the same contract.
+pub const MultiResult = enum {
+    /// Esc pressed. `len.*` reset to 0. Caller should also clear
+    /// any accumulated fragment list and exit input mode.
+    cancelled,
+    /// Enter pressed (no modifier). `len.*` is unchanged — the
+    /// fragment data is at `buf[0..len.*]`. Caller must copy it
+    /// into the fragment list and then set `len.* = 0` itself
+    /// before the next call.
+    fragment,
+    /// Ctrl+Enter (or Ctrl+D fallback) pressed. `len.*` is
+    /// unchanged so the caller can flush any final unfinished
+    /// fragment (`buf[0..len.*]`) before joining the accumulated
+    /// fragment list and committing.
+    committed,
+    /// Character appended / removed / cleared. Caller redraws.
+    edited,
+    /// Key didn't match any input-buffer semantic. Caller may
+    /// layer on its own handling.
+    ignored,
+};
+
+/// Apply a key event to the multi-fragment input buffer state
+/// machine. Used by the review tab's ack-note flow. Semantics:
+///
+/// - **Esc** ⇒ `.cancelled`. `len.*` reset to 0.
+/// - **Enter** (no modifier) ⇒ `.fragment`. `len.*` unchanged;
+///   caller reads `buf[0..len.*]`, copies it into its fragment
+///   list, then sets `len.* = 0` for the next fragment.
+/// - **Ctrl+Enter** (or **Ctrl+D** as legacy-terminal fallback)
+///   ⇒ `.committed`. `len.*` unchanged so the caller can flush
+///   any final unfinished fragment before joining all fragments
+///   and writing the journal record.
+/// - **Backspace, Ctrl+U, printable ASCII**: same as `handleKey`.
+pub fn handleKeyMulti(buf: []u8, len: *usize, key: vaxis.Key) MultiResult {
+    if (key.codepoint == vaxis.Key.escape) {
+        len.* = 0;
+        return .cancelled;
+    }
+    // Ctrl+Enter (and Ctrl+D fallback) commits the whole input.
+    // Must come BEFORE the bare-Enter check below: vaxis sets
+    // `codepoint = Key.enter` for both bare and modifier-prefixed
+    // Enter, so the codepoint-only check would match Ctrl+Enter
+    // first and return `.fragment` instead of `.committed`.
+    if (key.matches(vaxis.Key.enter, .{ .ctrl = true })) {
+        return .committed;
+    }
+    if (key.matches('d', .{ .ctrl = true })) {
+        return .committed;
+    }
+    if (key.codepoint == vaxis.Key.enter) {
+        // Caller reads `buf[0..len.*]` to capture the fragment, then
+        // resets `len.*`. Leaving len untouched here keeps the API
+        // discoverable: the data the caller wants is right where it
+        // left it.
+        return .fragment;
+    }
+    if (key.codepoint == vaxis.Key.backspace) {
+        if (len.* > 0) len.* -= 1;
+        return .edited;
+    }
+    if (key.matches('u', .{ .ctrl = true })) {
+        len.* = 0;
+        return .edited;
+    }
+    if (key.codepoint < std.math.maxInt(u7) and std.ascii.isPrint(@intCast(key.codepoint)) and len.* < buf.len) {
+        buf[len.*] = @intCast(key.codepoint);
+        len.* += 1;
+        return .edited;
+    }
+    return .ignored;
+}
+
 // ── Tests ─────────────────────────────────────────────────────
 
 const testing = std.testing;
@@ -137,3 +235,133 @@ test "handleKey: unrecognized key returns ignored" {
     try testing.expectEqual(Result.ignored, result);
     try testing.expectEqual(@as(usize, 0), len);
 }
+
+// ── Multi-fragment tests ──────────────────────────────────────
+
+test "handleKeyMulti: escape resets len and returns cancelled" {
+    var buf: [64]u8 = undefined;
+    var len: usize = 5;
+    const result = handleKeyMulti(&buf, &len, .{ .codepoint = vaxis.Key.escape });
+    try testing.expectEqual(MultiResult.cancelled, result);
+    try testing.expectEqual(@as(usize, 0), len);
+}
+
+test "handleKeyMulti: enter returns fragment without changing len" {
+    var buf: [64]u8 = undefined;
+    @memcpy(buf[0..5], "hello");
+    var len: usize = 5;
+    const result = handleKeyMulti(&buf, &len, .{ .codepoint = vaxis.Key.enter });
+    try testing.expectEqual(MultiResult.fragment, result);
+    try testing.expectEqual(@as(usize, 5), len);
+    try testing.expectEqualStrings("hello", buf[0..len]);
+}
+
+test "handleKeyMulti: ctrl+D returns committed without changing len" {
+    var buf: [64]u8 = undefined;
+    @memcpy(buf[0..3], "abc");
+    var len: usize = 3;
+    const result = handleKeyMulti(&buf, &len, .{ .codepoint = 'd', .mods = .{ .ctrl = true } });
+    try testing.expectEqual(MultiResult.committed, result);
+    try testing.expectEqual(@as(usize, 3), len);
+    try testing.expectEqualStrings("abc", buf[0..len]);
+}
+
+test "handleKeyMulti: ctrl+Enter returns committed (primary commit key)" {
+    var buf: [64]u8 = undefined;
+    @memcpy(buf[0..3], "abc");
+    var len: usize = 3;
+    const result = handleKeyMulti(&buf, &len, .{ .codepoint = vaxis.Key.enter, .mods = .{ .ctrl = true } });
+    try testing.expectEqual(MultiResult.committed, result);
+    try testing.expectEqual(@as(usize, 3), len);
+}
+
+test "handleKeyMulti: bare Enter still returns fragment (not committed)" {
+    // Regression test for the order-sensitive matcher: with bare
+    // Enter we want `.fragment`; with Ctrl+Enter we want
+    // `.committed`. The matcher must check Ctrl+Enter first so the
+    // codepoint-only check on bare Enter doesn't shadow it.
+    var buf: [64]u8 = undefined;
+    var len: usize = 5;
+    const result = handleKeyMulti(&buf, &len, .{ .codepoint = vaxis.Key.enter });
+    try testing.expectEqual(MultiResult.fragment, result);
+    try testing.expectEqual(@as(usize, 5), len);
+}
+
+test "handleKeyMulti: printable ASCII appends" {
+    var buf: [64]u8 = undefined;
+    var len: usize = 0;
+    const result = handleKeyMulti(&buf, &len, .{ .codepoint = 'x' });
+    try testing.expectEqual(MultiResult.edited, result);
+    try testing.expectEqual(@as(usize, 1), len);
+    try testing.expectEqual(@as(u8, 'x'), buf[0]);
+}
+
+test "handleKeyMulti: backspace decrements len" {
+    var buf: [64]u8 = undefined;
+    var len: usize = 3;
+    const result = handleKeyMulti(&buf, &len, .{ .codepoint = vaxis.Key.backspace });
+    try testing.expectEqual(MultiResult.edited, result);
+    try testing.expectEqual(@as(usize, 2), len);
+}
+
+test "handleKeyMulti: ctrl+U clears buffer" {
+    var buf: [64]u8 = undefined;
+    var len: usize = 5;
+    const result = handleKeyMulti(&buf, &len, .{ .codepoint = 'u', .mods = .{ .ctrl = true } });
+    try testing.expectEqual(MultiResult.edited, result);
+    try testing.expectEqual(@as(usize, 0), len);
+}
+
+test "handleKeyMulti: full caller flow — two fragments then commit" {
+    // Simulate the review-tab ack flow: type "first", Enter, type
+    // "second", Ctrl+D. Caller maintains an `ArrayList([]const u8)`
+    // of fragments; we mock that here as a fixed-size accumulator.
+    var buf: [64]u8 = undefined;
+    var len: usize = 0;
+    var fragments_storage: [4][32]u8 = undefined;
+    var fragment_lens: [4]usize = undefined;
+    var fragment_count: usize = 0;
+
+    // Type "first"
+    for ("first") |c| {
+        _ = handleKeyMulti(&buf, &len, .{ .codepoint = c });
+    }
+    try testing.expectEqual(@as(usize, 5), len);
+
+    // Enter ⇒ fragment
+    const r1 = handleKeyMulti(&buf, &len, .{ .codepoint = vaxis.Key.enter });
+    try testing.expectEqual(MultiResult.fragment, r1);
+    @memcpy(fragments_storage[fragment_count][0..len], buf[0..len]);
+    fragment_lens[fragment_count] = len;
+    fragment_count += 1;
+    len = 0; // caller resets
+
+    // Type "second"
+    for ("second") |c| {
+        _ = handleKeyMulti(&buf, &len, .{ .codepoint = c });
+    }
+    try testing.expectEqual(@as(usize, 6), len);
+
+    // Ctrl+D ⇒ committed
+    const r2 = handleKeyMulti(&buf, &len, .{ .codepoint = 'd', .mods = .{ .ctrl = true } });
+    try testing.expectEqual(MultiResult.committed, r2);
+    // Caller flushes the trailing unfinished fragment
+    @memcpy(fragments_storage[fragment_count][0..len], buf[0..len]);
+    fragment_lens[fragment_count] = len;
+    fragment_count += 1;
+
+    try testing.expectEqual(@as(usize, 2), fragment_count);
+    try testing.expectEqualStrings("first", fragments_storage[0][0..fragment_lens[0]]);
+    try testing.expectEqualStrings("second", fragments_storage[1][0..fragment_lens[1]]);
+}
+
+test "handleKeyMulti: ctrl+D with empty buffer still commits" {
+    // User types "first", Enter, then immediately Ctrl+D — final
+    // fragment is empty. Caller should detect len == 0 and skip the
+    // empty trailing fragment.
+    var buf: [64]u8 = undefined;
+    var len: usize = 0;
+    const result = handleKeyMulti(&buf, &len, .{ .codepoint = 'd', .mods = .{ .ctrl = true } });
+    try testing.expectEqual(MultiResult.committed, result);
+    try testing.expectEqual(@as(usize, 0), len);
+}

src/tui/options_tab.zig

@@ -256,6 +256,17 @@ pub const tab = struct {
         ensureCursorVisible(state, &app.scroll_offset, app.visible_height);
         return true;
     }
+
+    /// Mouse wheel: always scroll the viewport, never move the
+    /// cursor. Keeps wheel-as-look-around and cursor-as-pointer
+    /// distinct. The framework falls through to viewport scroll
+    /// when this returns false.
+    pub fn onWheelMove(state: *State, app: *App, delta: isize) bool {
+        _ = state;
+        _ = app;
+        _ = delta;
+        return false;
+    }
 };
 
 // ── Cursor movement / visibility (private; called from onCursorMove) ──

src/tui/performance_tab.zig

@@ -157,7 +157,22 @@ fn loadData(state: *State, app: *App) void {
         app.symbol_data.etf_loaded = true;
         if (app.svc.getEtfProfile(app.symbol, .{})) |etf_result| {
             if (etf_result.data.isEtf()) {
+                // Take ownership of the EtfProfile data. We
+                // deliberately don't call etf_result.deinit
+                // here — the data fields (symbol, name,
+                // holdings, sectors) are now owned by
+                // symbol_data and will be freed by
+                // symbol_data.clear() on next symbol change.
+                // The FetchResult wrapper itself is a struct
+                // with no separate heap allocation; dropping
+                // it leaks nothing.
                 app.symbol_data.etf_profile = etf_result.data;
+            } else {
+                // Non-ETF: the profile data was still allocated
+                // by getEtfProfile (symbol dupe at minimum,
+                // possibly name from Wikidata fallback). Free
+                // it via the FetchResult contract.
+                etf_result.deinit();
             }
         } else |_| {}
     }

src/tui/portfolio_tab.zig

@@ -4,11 +4,8 @@ const zfin = @import("../root.zig");
 const fmt = @import("../format.zig");
 const Money = @import("../Money.zig");
 const views = @import("../views/portfolio_sections.zig");
-const portfolio_loader = @import("../portfolio_loader.zig");
 const theme = @import("theme.zig");
 const tui = @import("../tui.zig");
-const projections_tab = @import("projections_tab.zig");
-const analysis_tab = @import("analysis_tab.zig");
 const framework = @import("tab_framework.zig");
 
 const App = tui.App;
@@ -138,6 +135,11 @@ pub const Action = enum {
     /// Select the cursor row's symbol as the currently-active
     /// symbol for the per-symbol tabs (quote/perf/options/etc.).
     select_symbol,
+    /// Toggle the symbol-info overlay popup. Open at cursor
+    /// row's symbol when closed; close when open and cursor is
+    /// on the same symbol; re-target when open and cursor is on
+    /// a different symbol.
+    toggle_overlay,
 };
 
 // ── Tab-private state ─────────────────────────────────────────
@@ -263,6 +265,10 @@ pub const meta: framework.TabMeta(Action) = .{
         .{ .action = .clear_account_filter, .key = .{ .codepoint = vaxis.Key.escape } },
         .{ .action = .select_symbol, .key = .{ .codepoint = 's' } },
         .{ .action = .select_symbol, .key = .{ .codepoint = vaxis.Key.space } },
+        // Capital K: Vim-style "what is this" toggle. Opens
+        // (or re-targets) the symbol-info overlay for the
+        // cursor row's symbol.
+        .{ .action = .toggle_overlay, .key = .{ .codepoint = 'K' } },
     },
     .action_labels = std.enums.EnumArray(Action, []const u8).init(.{
         .expand_collapse = "Expand/collapse position",
@@ -272,12 +278,14 @@ pub const meta: framework.TabMeta(Action) = .{
         .open_account_picker = "Filter by account",
         .clear_account_filter = "Clear account filter",
         .select_symbol = "Select symbol",
+        .toggle_overlay = "Show symbol details",
     }),
     .status_hints = &.{
         .sort_col_prev,
         .sort_col_next,
         .sort_reverse,
         .open_account_picker,
+        .toggle_overlay,
     },
 };
 
@@ -316,15 +324,29 @@ pub const tab = struct {
 
     pub const deactivate = framework.noopDeactivate(State);
 
-    /// Manual refresh (r/F5): drop the cached aggregate summary
-    /// and re-fetch live prices via `loadPortfolioData`. Distinct
-    /// from `reloadPortfolioFile` (R), which re-reads
-    /// `portfolio.srf` from disk. The framework calls this from
-    /// `refreshCurrentTab`; the file-reload path has its own
-    /// separate action.
+    /// Manual refresh (r/F5): re-fetch live prices and rebuild
+    /// the summary. Distinct from `reloadPortfolioFile` (R),
+    /// which also re-reads the portfolio file from disk. Refresh
+    /// keeps the same captured paths and just re-runs the load.
     pub fn reload(state: *State, app: *App) !void {
-        app.portfolio.loaded = false;
-        app.freePortfolioSummary();
+        // Collect watchlist symbols from app.watchlist (the
+        // separate `watchlist.srf` file). Portfolio's own
+        // `watch` lots are picked up by pd.load via the parsed
+        // file. Allocate against app.allocator; `pd.load`
+        // borrows during the call.
+        var watch_syms: std.ArrayList([]const u8) = .empty;
+        defer watch_syms.deinit(app.allocator);
+        if (app.watchlist) |wl| {
+            for (wl) |sym| watch_syms.append(app.allocator, sym) catch |err| std.log.debug("watch_syms append failed: {t}", .{err});
+        }
+        _ = app.portfolio.reload(app.today, .{
+            .force_refresh = true,
+            .watchlist_syms = watch_syms.items,
+        }) catch |err| {
+            app.setStatus("Error refreshing portfolio data");
+            std.log.scoped(.tui).warn("portfolio.reload: {t}", .{err});
+            return;
+        };
         loadPortfolioData(state, app);
     }
 
@@ -386,6 +408,17 @@ pub const tab = struct {
                 const msg = std.fmt.bufPrint(&tmp_buf, "Active: {s}", .{row.symbol}) catch "Active";
                 app.setStatus(msg);
             },
+            .toggle_overlay => {
+                // Resolve the cursor row's symbol; pass empty
+                // string when not on a symbol row so toggleOverlay
+                // closes any open overlay.
+                const sym: []const u8 = blk: {
+                    if (state.rows.items.len == 0) break :blk "";
+                    if (state.cursor >= state.rows.items.len) break :blk "";
+                    break :blk state.rows.items[state.cursor].symbol;
+                };
+                app.toggleOverlay(sym);
+            },
         }
     }
 
@@ -394,6 +427,31 @@ pub const tab = struct {
     /// concern handled by `drawWelcomeScreen`).
     pub const isDisabled = framework.alwaysEnabled();
 
+    /// Drop UI state that referenced the previous portfolio.
+    ///
+    /// `account_list` holds borrowed strings into the old
+    /// `Portfolio.lots`; `rows` was built against the old
+    /// summary; `cursor` / `expanded` / `cash_expanded` /
+    /// `illiquid_expanded` are row indices and toggles into
+    /// stale data. All MUST be cleared before the new
+    /// portfolio renders to avoid pointing past the end of the
+    /// rebuilt list.
+    ///
+    /// Account filter (`state.account_filter`) is preserved
+    /// because it's an owned copy of the filter NAME — the
+    /// next render will re-resolve it against the rebuilt
+    /// account list (and quietly drop it if the account no
+    /// longer exists).
+    pub fn onPortfolioReload(state: *State, app: *App) void {
+        state.account_list.clearRetainingCapacity();
+        state.rows.clearRetainingCapacity();
+        state.expanded = @splat(false);
+        state.cash_expanded = false;
+        state.illiquid_expanded = false;
+        state.cursor = 0;
+        app.scroll_offset = 0;
+    }
+
     /// Sync the cursor to the new scroll extreme.
     pub fn onScroll(state: *State, app: *App, where: framework.ScrollEdge) void {
         _ = app;
@@ -420,6 +478,11 @@ pub const tab = struct {
         return true;
     }
 
+    // No `onWheelMove`: by omitting it, wheel events fall through
+    // to the framework's `onCursorMove` path. Wheel moves the
+    // cursor like j/k. PageUp / PageDown / Home / End scroll the
+    // viewport.
+
     /// Pre-empt key handler. Called by the framework BEFORE
     /// global keymap matching runs. When portfolio is in a
     /// modal sub-state (`state.modal != .none`) we route to the
@@ -589,9 +652,9 @@ fn mapIntent(th: theme.Theme, intent: fmt.StyleIntent) vaxis.Style {
 ///   2. Manual refresh (r/F5): refreshCurrentTab() clears portfolio_loaded → loadTabData() → here
 ///   3. Disk reload (R): reloadPortfolioFile() — separate function, cache-only, no network
 ///
-/// On first call, uses prefetched_prices (populated before TUI started).
-/// On refresh, fetches live via svc.loadPrices. Tab switching skips this
-/// entirely because the portfolio_loaded guard in loadTabData() short-circuits.
+/// Tab switching is a no-op when portfolio.summary is already
+/// populated; the row build is cheap so re-running it on a
+/// re-activate has no measurable cost.
 /// Set up the portfolio tab's UI state from current
 /// `app.portfolio` data: sort allocations per current sort
 /// field, build the account list, recompute filtered positions
@@ -614,9 +677,8 @@ fn mapIntent(th: theme.Theme, intent: fmt.StyleIntent) vaxis.Style {
 /// owns. Visiting portfolio after analysis pre-loaded the data
 /// will still rebuild the row list — cheap.)
 pub fn loadPortfolioData(state: *State, app: *App) void {
-    app.ensurePortfolioDataLoaded();
-
-    // App may have failed to load — check before touching summary.
+    // Summary is populated synchronously by pd.load; if it's
+    // null here, no portfolio is loaded (welcome screen).
     const summary = app.portfolio.summary orelse return;
 
     sortPortfolioAllocations(state, app);
@@ -994,10 +1056,10 @@ pub fn buildAccountList(state: *State, app: *App) void {
         }
     }
 
-    app.ensureAccountMap();
-
-    // Phase 1: add accounts in accounts.srf order (if available)
-    if (app.portfolio.account_map) |am| {
+    // Phase 1: add accounts in accounts.srf order (if available).
+    // accountMap() blocks on its worker; first call may briefly
+    // wait until the account_map worker finishes loading.
+    if (app.portfolio.accountMap()) |am| {
         for (am.entries) |entry| {
             if (seen.contains(entry.account)) {
                 state.account_list.append(app.allocator, entry.account) catch continue;
@@ -1265,8 +1327,11 @@ pub fn drawContent(state: *State, app: *App, arena: std.mem.Allocator, buf: []va
                 }
             }
 
-            // Historical portfolio value snapshots
-            if (app.portfolio.historical_snapshots) |snapshots| {
+            // Historical portfolio value snapshots. snapshots()
+            // blocks on the snapshots worker the first time it's
+            // called; portfolio is the first tab the user sees,
+            // so this is where the wait (if any) happens.
+            if (app.portfolio.snapshots()) |snapshots| {
                 try lines.append(arena, .{ .text = "", .style = th.contentStyle() });
                 var hist_parts: [6][]const u8 = undefined;
                 for (zfin.valuation.HistoricalPeriod.all, 0..) |period, pi| {
@@ -1681,152 +1746,83 @@ pub fn buildWelcomeScreenLines(
 /// the initial load uses, so a manual reload sees the merged view
 /// of every `portfolio*.srf` in the resolved directory — same as
 /// the CLI.
+/// Reload portfolio file from disk. Re-parses files at the
+/// captured paths, re-fetches prices (cache-only — no network),
+/// and rebuilds the summary + spawns the workers. Distinct
+/// from the in-place refresh action (r/F5) which forces a live
+/// fetch.
+///
+/// Goes through the same `loadPortfolioFromPaths` the initial
+/// load uses, so a manual reload sees the merged view of every
+/// `portfolio*.srf` in the resolved directory — same as the CLI.
+///
+/// Lifecycle: broadcast onPortfolioReload BEFORE pd.reload so
+/// every tab clears derived state that borrows from the
+/// portfolio arena. Tabs MUST NOT eager-rebuild from those
+/// hooks — pd.reload is about to free the arena out from under
+/// them. Instead, after pd.reload completes, app.loadTabData()
+/// re-activates the currently-active tab against fresh data.
+/// Inactive tabs stay in `loaded = false` and lazy-rebuild on
+/// next switch.
 pub fn reloadPortfolioFile(state: *State, app: *App) void {
-    // Save the account filter name before freeing the old portfolio.
-    // account_filter is an owned copy so it survives the portfolio free,
-    // but account_list entries borrow from the portfolio and will dangle.
-    state.account_list.clearRetainingCapacity();
-
-    // Re-read the portfolio file(s)
-    if (app.portfolio.file) |*pf| pf.deinit();
-    app.portfolio.file = null;
-
-    if (app.portfolio_paths.len == 0) {
+    if (app.portfolio.paths.len == 0) {
         app.setStatus("No portfolio file to reload");
         return;
     }
 
-    if (portfolio_loader.loadPortfolioFromPaths(app.io, app.allocator, app.portfolio_paths, app.today)) |loaded| {
-        // Take the merged Portfolio; discard the auxiliary slices
-        // we don't keep on App. Note we deliberately don't replace
-        // `portfolio_paths` here — those still come from the
-        // initial resolution. If new portfolio files appear, the
-        // user can restart the TUI to pick them up.
-        app.portfolio.file = loaded.portfolio;
-        app.allocator.free(loaded.syms);
-        app.allocator.free(loaded.positions);
-        for (loaded.file_datas) |d| app.allocator.free(d);
-        app.allocator.free(loaded.file_datas);
-        // The path slice + ResolvedPaths the loader allocated for
-        // its own LoadedPortfolio are NOT what App stores. Free
-        // them; App's `portfolio_paths` stays put.
-        app.allocator.free(loaded.paths);
-        if (loaded.resolved_paths) |rp| rp.deinit();
-    } else {
-        app.setStatus("Error reloading portfolio file");
-        return;
-    }
+    // Broadcast onPortfolioReload to every tab so each tab
+    // invalidates its own derived state (cached results, view
+    // models, cursor / expansion indices that pointed into the
+    // about-to-be-freed portfolio data). MUST happen BEFORE
+    // pd.reload starts freeing/recreating the underlying data.
+    // Tabs MUST NOT eager-rebuild from this hook (see above).
+    app.broadcast("onPortfolioReload", .{});
 
-    // Reload watchlist file too (if separate)
+    // Reload watchlist file too (if separate). pd doesn't read
+    // watchlist.srf — that's a TUI-side concern.
     tui.freeWatchlist(app.allocator, app.watchlist);
     app.watchlist = null;
     if (app.watchlist_path) |path| {
         app.watchlist = tui.loadWatchlist(app.io, app.allocator, path);
     }
 
-    // Recompute summary using cached prices (no network)
-    app.freePortfolioSummary();
-    state.expanded = @splat(false);
-    state.cash_expanded = false;
-    state.illiquid_expanded = false;
-    state.cursor = 0;
-    app.scroll_offset = 0;
-    state.rows.clearRetainingCapacity();
-
-    const pf = app.portfolio.file orelse return;
-    const positions = pf.positions(app.today, app.allocator) catch {
-        app.setStatus("Error computing positions");
-        return;
-    };
-    defer app.allocator.free(positions);
-
-    var prices = std.StringHashMap(f64).init(app.allocator);
-    defer prices.deinit();
-
-    const syms = pf.stockSymbols(app.allocator) catch {
-        app.setStatus("Error getting symbols");
-        return;
-    };
-    defer app.allocator.free(syms);
-
-    var latest_date: ?zfin.Date = null;
-    var missing: usize = 0;
-    for (syms) |sym| {
-        // Cache only — no network
-        const candles_slice = app.svc.getCachedCandles(sym);
-        if (candles_slice) |cs| {
-            defer cs.deinit();
-            if (cs.data.len > 0) {
-                prices.put(sym, cs.data[cs.data.len - 1].close) catch |err| std.log.debug("price put failed: {t}", .{err});
-                const d = cs.data[cs.data.len - 1].date;
-                if (latest_date == null or d.days > latest_date.?.days) latest_date = d;
-            }
-        } else {
-            missing += 1;
-        }
-    }
-    app.portfolio.latest_quote_date = latest_date;
-
-    // Build portfolio summary, candle map, and historical snapshots from cache
-    var pf_data = portfolio_loader.buildPortfolioData(app.allocator, pf, positions, syms, &prices, app.svc, app.today) catch |err| switch (err) {
-        error.NoAllocations => {
-            app.setStatus("No cached prices available");
-            return;
-        },
-        error.SummaryFailed => {
-            app.setStatus("Error computing portfolio summary");
-            return;
-        },
-        else => {
-            app.setStatus("Error building portfolio data");
-            return;
-        },
-    };
-    app.portfolio.summary = pf_data.summary;
-    app.portfolio.historical_snapshots = pf_data.snapshots;
-    {
-        var it = pf_data.candle_map.valueIterator();
-        while (it.next()) |v| app.allocator.free(v.*);
-        pf_data.candle_map.deinit();
+    var watch_syms: std.ArrayList([]const u8) = .empty;
+    defer watch_syms.deinit(app.allocator);
+    if (app.watchlist) |wl| {
+        for (wl) |sym| watch_syms.append(app.allocator, sym) catch |err| std.log.debug("watch_syms append failed: {t}", .{err});
     }
 
+    // pd.reload re-uses captured paths, re-parses, re-fetches
+    // prices (.force_refresh = false → honor cache TTLs), and
+    // spawns fresh workers.
+    _ = app.portfolio.reload(app.today, .{
+        .watchlist_syms = watch_syms.items,
+    }) catch |err| {
+        app.setStatus("Error reloading portfolio file");
+        std.log.scoped(.tui).warn("portfolio.reload: {t}", .{err});
+        return;
+    };
+    if (app.portfolio.summary == null) return;
+
+    // Always rebuild portfolio_tab UI state (regardless of
+    // which tab is active) — the onPortfolioReload broadcast
+    // cleared it above, and if we leave it empty the user
+    // could switch back without an activate firing in some
+    // paths. Cheap (sort + filter on ~30 holdings).
     sortPortfolioAllocations(state, app);
     buildAccountList(state, app);
     recomputeFilteredPositions(state, app);
     rebuildPortfolioRows(state, app);
 
-    // Invalidate analysis data -- it holds pointers into old portfolio memory
-    if (app.states.analysis.result) |*ar| ar.deinit(app.allocator);
-    app.states.analysis.result = null;
-    app.states.analysis.loaded = false;
-    // Note: `analysis_tab.tab.isDisabled` derives availability from
-    // `app.portfolio.file`, so we don't need to clear a `disabled`
-    // flag here — it's recomputed at every read.
+    // Re-activate the currently-active tab (whatever it is) so
+    // it rebuilds derived state from the freshly-reloaded
+    // portfolio data. Tabs that gate on `state.loaded` already
+    // got `loaded = false` from the broadcast above, so this
+    // call triggers their full re-load. For portfolio_tab
+    // itself this is idempotent with the manual rebuild above.
+    app.loadTabData();
 
-    // If currently on the analysis tab, eagerly recompute so the user
-    // doesn't see an error message before switching away and back.
-    if (app.active_tab == .analysis) {
-        analysis_tab.tab.activate(&app.states.analysis, app) catch |err| std.log.debug("analysis activate failed: {t}", .{err});
-    }
-
-    // Invalidate projections data — projections.srf may have changed.
-    // Always drop the cached context so a stale render doesn't leak;
-    // re-fetch only if the user is actively looking at projections.
-    // (When not active, the next `activate` lazily re-fetches.)
-    if (app.active_tab == .projections) {
-        projections_tab.tab.reload(&app.states.projections, app) catch |err| std.log.debug("projections reload failed: {t}", .{err});
-    } else {
-        projections_tab.freeLoaded(&app.states.projections, app);
-        app.states.projections.loaded = false;
-    }
-
-    if (missing > 0) {
-        var warn_buf: [128]u8 = undefined;
-        const warn_msg = std.fmt.bufPrint(&warn_buf, "Reloaded. {d} symbols missing cached prices", .{missing}) catch "Reloaded (some prices missing)";
-        app.setStatus(warn_msg);
-    } else {
-        app.setStatus("Portfolio reloaded from disk");
-    }
+    app.setStatus("Portfolio reloaded from disk");
 }
 
 // ── Account picker ────────────────────────────────────────────

src/tui/projections_tab.zig

@@ -257,10 +257,9 @@ pub const tab = struct {
 
     pub fn activate(state: *State, app: *App) !void {
         if (state.loaded) return;
-        // Projections reads `app.portfolio.summary` and
-        // `.file`. Ensure they're populated even when the user
-        // jumps straight here without visiting portfolio first.
-        app.ensurePortfolioDataLoaded();
+        // Projections reads `app.portfolio.summary` and `.file`,
+        // both populated synchronously by pd.load at App init.
+        // No further data fetching needed here.
         loadData(state, app);
     }
 
@@ -411,6 +410,25 @@ pub const tab = struct {
     pub fn isDisabled(app: *App) bool {
         return app.portfolio.file == null;
     }
+
+    /// Drop cached projection data on portfolio reload. The
+    /// projection result holds pointers into the previous
+    /// portfolio's memory; invalidate before the underlying data
+    /// is freed.
+    ///
+    /// Deliberately does NOT eager-rebuild, even when this tab
+    /// is active. The reload broadcast fires BEFORE pd.reload
+    /// resets the portfolio arena; rebuilding now would store
+    /// borrowed pointers that get freed seconds later when the
+    /// arena resets, producing use-after-free crashes on the
+    /// next draw. The orchestrator
+    /// (portfolio_tab.reloadPortfolioFile) calls
+    /// `app.loadTabData()` AFTER pd.reload completes, which
+    /// re-activates the active tab against fresh data.
+    pub fn onPortfolioReload(state: *State, app: *App) void {
+        freeLoaded(state, app);
+        state.loaded = false;
+    }
 };
 
 /// Format the "overlay unavailable" status hint shown when the user

src/tui/tab_framework.zig

@@ -54,6 +54,23 @@
 //!     pub fn handlePaste(state: *State, app: *App, text: []const u8) bool { ... }
 //!     pub fn statusOverride(state: *State, app: *App) ?framework.StatusOverride { ... }
 //!
+//!     /// Optional: does this tab currently have async work in
+//!     /// flight that needs poll-driven redraws? While the ACTIVE
+//!     /// tab answers true, the App keeps a one-shot vxfw Tick
+//!     /// timer armed (~100ms cadence) so the draw loop wakes up
+//!     /// and runs the tab's `tick` hook even with no user input —
+//!     /// without it, async results would sit invisible until the
+//!     /// next keypress.
+//!     ///
+//!     /// Answer from your own state (e.g. "my observation panel
+//!     /// has pending checks"); do NOT cache the answer anywhere on
+//!     /// App. The framework derives the polling decision fresh
+//!     /// after every event, so tab switches and work completion
+//!     /// are picked up automatically with zero choreography.
+//!     /// Inactive tabs are never asked — switching away pauses
+//!     /// UI polling while background work continues.
+//!     pub fn wantsPollTick(state: *State, app: *App) bool { ... }
+//!
 //!     // ── Context-change hooks (optional) ─────────────────────
 //!     // Fire when a global context this tab depends on changes.
 //!     // Tabs that don't care simply omit the method. Contrast with
@@ -62,22 +79,60 @@
 //!     // lazily on next `activate`.
 //!     pub fn onSymbolChange(state: *State, app: *App) void { ... }
 //!
+//!     /// Fired when the portfolio file is reloaded (user pressed
+//!     /// `r`/F5, file watcher triggered, etc.). Every tab that
+//!     /// holds derived state pointing into the previous portfolio
+//!     /// (cached `findings_view`, analysis `result`, projection
+//!     /// caches, row indices, account list) MUST drop it here —
+//!     /// the underlying portfolio data has already been freed by
+//!     /// the time this is called.
+//!     ///
+//!     /// Tabs that don't hold portfolio-derived state simply omit
+//!     /// the hook. Broadcast via `App.broadcast`; called BEFORE
+//!     /// the new portfolio is loaded so tabs see a clean-slate
+//!     /// state, not a half-populated one.
+//!     pub fn onPortfolioReload(state: *State, app: *App) void { ... }
+//!
 //!     /// Fired when the user invokes a global scroll-to-extreme
 //!     /// action (`g`/`G`). Tabs with a cursor reset it to match
 //!     /// the new scroll position. Tabs without a cursor omit
 //!     /// this hook.
 //!     pub fn onScroll(state: *State, app: *App, where: ScrollEdge) void { ... }
 //!
-//!     /// Fired when the user invokes a relative cursor-move
-//!     /// (`j`/`k`, ↑/↓, mouse wheel). `delta` is signed: positive
-//!     /// = down, negative = up. Magnitude is 1 for keys, larger
-//!     /// for wheel events. Tabs with a row cursor step it,
-//!     /// clamp to row count, and ensure visibility; return
-//!     /// `true` to consume. Tabs without a cursor (or with empty
-//!     /// rows) return `false` so the framework falls through to
-//!     /// scroll-by-`delta` instead.
+//!     /// Fired when the user invokes a relative cursor-move via
+//!     /// keyboard (`j`/`k`, ↑/↓). `delta` is signed: positive =
+//!     /// down, negative = up. Magnitude is 1 per keypress. Tabs
+//!     /// with a row cursor step it, clamp to row count, and
+//!     /// ensure visibility; return `true` to consume. Tabs without
+//!     /// a cursor (or with empty rows) return `false` so the
+//!     /// framework falls through to scroll-by-`delta` instead.
+//!     ///
+//!     /// **Mouse wheel events go through `onWheelMove`, NOT this
+//!     /// hook.** A tab that wants wheel-as-cursor-move (the legacy
+//!     /// portfolio-tab convention) implements `onWheelMove` to
+//!     /// delegate to `onCursorMove`. A tab that wants
+//!     /// wheel-as-viewport-scroll (the cleaner default for tabs
+//!     /// with multiple cursor regions) declines `onWheelMove` and
+//!     /// the framework scrolls instead.
 //!     pub fn onCursorMove(state: *State, app: *App, delta: isize) bool { ... }
 //!
+//!     /// Fired when the user wheels the mouse. `delta` is signed:
+//!     /// positive = down, negative = up. Magnitude is whatever the
+//!     /// terminal reports per wheel detent (typically 3-5 lines on
+//!     /// most platforms; the framework already debounces).
+//!     ///
+//!     /// Return `true` to consume; return `false` to fall through
+//!     /// to viewport scroll. If a tab omits this hook entirely,
+//!     /// the framework's default behavior is to delegate to
+//!     /// `onCursorMove` — which preserves the legacy
+//!     /// "wheel moves cursor" behavior for single-cursor tabs.
+//!     ///
+//!     /// New multi-region tabs (e.g. review tab with separate
+//!     /// holdings + findings tables) should declare this hook and
+//!     /// return `false` so wheel always scrolls the viewport,
+//!     /// reserving cursor movement for keyboard and click.
+//!     pub fn onWheelMove(state: *State, app: *App, delta: isize) bool { ... }
+//!
 //!     // ── Misc (required) ─────────────────────────────────────
 //!     pub fn isDisabled(app: *App) bool { ... }
 //! };
@@ -433,6 +488,16 @@ pub fn validateTabModule(comptime Module: type) void {
                 "pub fn statusOverride(state: *State, app: *App) ?StatusOverride { ... }",
             );
         }
+        if (@hasDecl(tab_decl, "wantsPollTick")) {
+            validator.expectFn(
+                "Tab module",
+                mod_name,
+                tab_decl,
+                "wantsPollTick",
+                fn (*State, *App) bool,
+                "pub fn wantsPollTick(state: *State, app: *App) bool { ... }",
+            );
+        }
 
         // ── Draw hooks (mutually exclusive, exactly one required) ──
         //
@@ -494,6 +559,16 @@ pub fn validateTabModule(comptime Module: type) void {
                 "pub fn onSymbolChange(state: *State, app: *App) void { ... }",
             );
         }
+        if (@hasDecl(tab_decl, "onPortfolioReload")) {
+            validator.expectFn(
+                "Tab module",
+                mod_name,
+                tab_decl,
+                "onPortfolioReload",
+                fn (*State, *App) void,
+                "pub fn onPortfolioReload(state: *State, app: *App) void { ... }",
+            );
+        }
         if (@hasDecl(tab_decl, "onScroll")) {
             validator.expectFn(
                 "Tab module",
@@ -514,6 +589,16 @@ pub fn validateTabModule(comptime Module: type) void {
                 "pub fn onCursorMove(state: *State, app: *App, delta: isize) bool { ... }",
             );
         }
+        if (@hasDecl(tab_decl, "onWheelMove")) {
+            validator.expectFn(
+                "Tab module",
+                mod_name,
+                tab_decl,
+                "onWheelMove",
+                fn (*State, *App, isize) bool,
+                "pub fn onWheelMove(state: *State, app: *App, delta: isize) bool { ... }",
+            );
+        }
     }
 }
 

src/views/observations_view.zig

@@ -0,0 +1,393 @@
+//! View model joining `analytics/observations` engine output with the
+//! `data/journal` acknowledgments file. Produces a flat list of
+//! `FindingRow`s ready for both the CLI findings table and the TUI
+//! review tab.
+//!
+//! ## Responsibility split
+//!
+//! - The **engine** (`analytics/observations.zig`) produces a
+//!   `CheckPanel` whose `pending` slice has one entry per registered
+//!   check, each with a `CheckResult` of `pass`/`warn`/`flag`/`skipped`/`err`.
+//! - The **journal** (`data/Journal.zig`) holds the user's
+//!   acknowledgments — durable records keyed by `(observation, target)`.
+//! - This **view** matches the two: every `Observation` in a
+//!   `warn`/`flag` result becomes a `FindingRow`. The row is marked
+//!   acked iff a `Journal.Entry` exists with `state == .acknowledged`
+//!   and matching `(observation, target)`.
+//!
+//! ## Lifetime
+//!
+//! `FindingRow.text` and friends are **borrowed** from the panel and
+//! journal — the view does NOT copy strings. This keeps allocation
+//! cheap (one slice for the rows array) and is safe because callers
+//! always hold the panel and journal alive for the entire render
+//! frame. `FindingsView.deinit` only frees the rows slice.
+//!
+//! ## Filtering
+//!
+//! Resolved entries (state == `.resolved`) are never shown — by
+//! definition, the engine no longer emits the finding, so there's
+//! nothing to suppress. Only `acknowledged` rows can be filtered out
+//! via `show_acked = false`.
+
+const std = @import("std");
+const observations = @import("../analytics/observations.zig");
+const Journal = @import("../data/Journal.zig");
+
+const Observation = observations.Observation;
+const Severity = observations.Severity;
+const CheckPanel = observations.CheckPanel;
+
+/// One displayable finding. Borrows all string data from the
+/// underlying `Observation` and (optionally) the matching journal
+/// entry. Lifetime is bounded by the panel + journal.
+pub const FindingRow = struct {
+    severity: Severity,
+    /// Borrowed from `Observation.kind`.
+    kind: []const u8,
+    /// Borrowed from `Observation.target`.
+    target: []const u8,
+    /// Borrowed from `Observation.text`.
+    text: []const u8,
+    /// True iff a journal entry matches `(kind, target)` with state
+    /// `.acknowledged`. When true, `ack_entry` is non-null.
+    is_acked: bool,
+    /// The matching journal entry (when `is_acked == true`), so
+    /// renderers can pull notes/acknowledged_at out without re-doing
+    /// the lookup. Null when no ack matches.
+    ack_entry: ?*const Journal.Entry = null,
+};
+
+/// Result of `build`. Owns only the `rows` slice; all string data
+/// is borrowed.
+pub const FindingsView = struct {
+    rows: []FindingRow,
+    /// Number of un-acked findings (severity warn or flag, not
+    /// suppressed). Independent of `show_acked` — counts the underlying
+    /// engine output.
+    total_active: usize,
+    /// Number of findings whose journal entry is in `.acknowledged`
+    /// state.
+    total_acked: usize,
+    /// Number of journal entries currently in `.resolved` state. These
+    /// are never rendered as rows; surfaced only for the header line
+    /// ("3 active, 1 acked, 2 resolved").
+    total_resolved: usize,
+
+    pub fn deinit(self: *FindingsView, allocator: std.mem.Allocator) void {
+        allocator.free(self.rows);
+        self.* = undefined;
+    }
+};
+
+/// Build a `FindingsView` from a panel + journal. `show_acked`
+/// controls whether acked findings appear in `rows` (they're always
+/// counted in `total_acked`).
+pub fn build(
+    allocator: std.mem.Allocator,
+    panel: *const CheckPanel,
+    journal: *const Journal,
+    show_acked: bool,
+) !FindingsView {
+    var rows = std.ArrayList(FindingRow).empty;
+    errdefer rows.deinit(allocator);
+
+    var total_active: usize = 0;
+    var total_acked: usize = 0;
+
+    for (panel.pending) |pc| {
+        const obs_slice: []const Observation = switch (pc.state) {
+            .complete => |r| switch (r) {
+                .warn => |o| o,
+                .flag => |o| o,
+                .pass, .skipped, .err => continue,
+            },
+            // Still running — no findings to show yet. The caller
+            // rebuilds the view when the panel completes (TUI
+            // polls via tick; CLI awaits before building).
+            .pending => continue,
+        };
+
+        for (obs_slice) |obs| {
+            const ack = findAck(journal, obs.kind, obs.target);
+            const is_acked = ack != null;
+
+            if (is_acked) {
+                total_acked += 1;
+            } else {
+                total_active += 1;
+            }
+
+            if (is_acked and !show_acked) continue;
+
+            try rows.append(allocator, .{
+                .severity = obs.severity,
+                .kind = obs.kind,
+                .target = obs.target,
+                .text = obs.text,
+                .is_acked = is_acked,
+                .ack_entry = ack,
+            });
+        }
+    }
+
+    // Count resolved entries for the header. These never produce rows
+    // — the engine's failure to re-emit the finding is what marks
+    // them resolved — but we surface the count so the user knows
+    // their journal still tracks them.
+    var total_resolved: usize = 0;
+    for (journal.entries) |*e| {
+        if (e.ack.state == .resolved) total_resolved += 1;
+    }
+
+    return .{
+        .rows = try rows.toOwnedSlice(allocator),
+        .total_active = total_active,
+        .total_acked = total_acked,
+        .total_resolved = total_resolved,
+    };
+}
+
+/// Look up a journal entry by `(observation, target)`, returning
+/// only entries currently in `.acknowledged` state. Returns null
+/// when no match.
+///
+/// Linear scan is fine for realistic journal sizes (tens of entries).
+/// If a portfolio ever accumulates hundreds of acks we can index by
+/// (observation, target) at load time.
+fn findAck(journal: *const Journal, observation: []const u8, target: []const u8) ?*const Journal.Entry {
+    for (journal.entries) |*e| {
+        if (e.ack.state != .acknowledged) continue;
+        if (!std.mem.eql(u8, e.ack.observation, observation)) continue;
+        if (!std.mem.eql(u8, e.ack.target, target)) continue;
+        return e;
+    }
+    return null;
+}
+
+// ── Tests ─────────────────────────────────────────────────────
+
+const testing = std.testing;
+const Date = @import("../Date.zig");
+
+fn makeObs(kind: []const u8, target: []const u8, text: []const u8) Observation {
+    return .{
+        .severity = .warn,
+        .kind = kind,
+        .target = target,
+        .text = text,
+    };
+}
+
+fn makePanel(allocator: std.mem.Allocator, obs_slice: []const Observation) !CheckPanel {
+    // Allocate a single-check panel with the supplied observations as
+    // a `warn` result. Test-only helper. Dupes all strings so that
+    // `panel.deinit`'s `freeObservations` path (which calls
+    // `allocator.free` on each string) operates on owned memory rather
+    // than caller-supplied literals.
+    const owned_obs = try allocator.alloc(Observation, obs_slice.len);
+    errdefer allocator.free(owned_obs);
+    for (obs_slice, 0..) |o, i| {
+        owned_obs[i] = .{
+            .severity = o.severity,
+            .kind = try allocator.dupe(u8, o.kind),
+            .target = try allocator.dupe(u8, o.target),
+            .text = try allocator.dupe(u8, o.text),
+        };
+    }
+
+    const check_singleton = struct {
+        const c: observations.Check = .{
+            .name = "test_check",
+            .label = "Test Check",
+            // SAFETY: test-only Check; tests fabricate panels with
+            // pre-baked CheckResults via `state = .{ .complete = ... }`,
+            // so `runChecks` never dispatches via this fn pointer.
+            .run = undefined,
+        };
+    };
+
+    const pending = try allocator.alloc(observations.PendingCheck, 1);
+    pending[0] = .{
+        .check = &check_singleton.c,
+        .state = .{ .complete = .{ .warn = owned_obs } },
+    };
+    return .{ .allocator = allocator, .io = std.testing.io, .pending = pending };
+}
+
+fn makeJournalWithAck(
+    allocator: std.mem.Allocator,
+    observation: []const u8,
+    target: []const u8,
+    state: Journal.State,
+) !Journal {
+    const obs_dup = try allocator.dupe(u8, observation);
+    const tgt_dup = try allocator.dupe(u8, target);
+    const empty_notes = try allocator.alloc([]const u8, 0);
+    const entries = try allocator.alloc(Journal.Entry, 1);
+    entries[0] = .{
+        .ack = .{
+            .observation = obs_dup,
+            .target = tgt_dup,
+            .acknowledged_at = Date.fromYmd(2026, 1, 1),
+            .state = state,
+        },
+        .notes = empty_notes,
+    };
+    return .{ .allocator = allocator, .entries = entries };
+}
+
+fn makeEmptyJournal(allocator: std.mem.Allocator) !Journal {
+    const entries = try allocator.alloc(Journal.Entry, 0);
+    return .{ .allocator = allocator, .entries = entries };
+}
+
+test "build: unacked finding produces a row, no ack_entry" {
+    const obs = [_]Observation{
+        makeObs("position_concentration", "NVDA", "NVDA at 18%"),
+    };
+    var panel = try makePanel(testing.allocator, &obs);
+    defer panel.deinit();
+
+    var journal = try makeEmptyJournal(testing.allocator);
+    defer journal.deinit();
+
+    var view = try build(testing.allocator, &panel, &journal, false);
+    defer view.deinit(testing.allocator);
+
+    try testing.expectEqual(@as(usize, 1), view.rows.len);
+    try testing.expectEqual(@as(usize, 1), view.total_active);
+    try testing.expectEqual(@as(usize, 0), view.total_acked);
+    try testing.expect(!view.rows[0].is_acked);
+    try testing.expect(view.rows[0].ack_entry == null);
+}
+
+test "build: acked finding suppressed when show_acked is false" {
+    const obs = [_]Observation{
+        makeObs("position_concentration", "NVDA", "NVDA at 18%"),
+    };
+    var panel = try makePanel(testing.allocator, &obs);
+    defer panel.deinit();
+
+    var journal = try makeJournalWithAck(
+        testing.allocator,
+        "position_concentration",
+        "NVDA",
+        .acknowledged,
+    );
+    defer journal.deinit();
+
+    var view = try build(testing.allocator, &panel, &journal, false);
+    defer view.deinit(testing.allocator);
+
+    try testing.expectEqual(@as(usize, 0), view.rows.len);
+    try testing.expectEqual(@as(usize, 0), view.total_active);
+    try testing.expectEqual(@as(usize, 1), view.total_acked);
+}
+
+test "build: acked finding rendered when show_acked is true" {
+    const obs = [_]Observation{
+        makeObs("position_concentration", "NVDA", "NVDA at 18%"),
+    };
+    var panel = try makePanel(testing.allocator, &obs);
+    defer panel.deinit();
+
+    var journal = try makeJournalWithAck(
+        testing.allocator,
+        "position_concentration",
+        "NVDA",
+        .acknowledged,
+    );
+    defer journal.deinit();
+
+    var view = try build(testing.allocator, &panel, &journal, true);
+    defer view.deinit(testing.allocator);
+
+    try testing.expectEqual(@as(usize, 1), view.rows.len);
+    try testing.expect(view.rows[0].is_acked);
+    try testing.expect(view.rows[0].ack_entry != null);
+}
+
+test "build: resolved entries don't filter findings" {
+    // Engine emits a finding for NVDA. Journal has a *resolved* entry
+    // for NVDA. The finding should NOT be suppressed — resolved means
+    // "engine stopped emitting it last time", but here it's emitting
+    // again.
+    const obs = [_]Observation{
+        makeObs("position_concentration", "NVDA", "NVDA at 18%"),
+    };
+    var panel = try makePanel(testing.allocator, &obs);
+    defer panel.deinit();
+
+    var journal = try makeJournalWithAck(
+        testing.allocator,
+        "position_concentration",
+        "NVDA",
+        .resolved,
+    );
+    defer journal.deinit();
+
+    var view = try build(testing.allocator, &panel, &journal, false);
+    defer view.deinit(testing.allocator);
+
+    try testing.expectEqual(@as(usize, 1), view.rows.len);
+    try testing.expect(!view.rows[0].is_acked);
+    try testing.expectEqual(@as(usize, 1), view.total_resolved);
+}
+
+test "build: target mismatch — different symbol doesn't match ack" {
+    const obs = [_]Observation{
+        makeObs("position_concentration", "NVDA", "NVDA at 18%"),
+        makeObs("position_concentration", "AAPL", "AAPL at 17%"),
+    };
+    var panel = try makePanel(testing.allocator, &obs);
+    defer panel.deinit();
+
+    // Ack only NVDA; AAPL should still appear.
+    var journal = try makeJournalWithAck(
+        testing.allocator,
+        "position_concentration",
+        "NVDA",
+        .acknowledged,
+    );
+    defer journal.deinit();
+
+    var view = try build(testing.allocator, &panel, &journal, false);
+    defer view.deinit(testing.allocator);
+
+    try testing.expectEqual(@as(usize, 1), view.rows.len);
+    try testing.expectEqualStrings("AAPL", view.rows[0].target);
+    try testing.expectEqual(@as(usize, 1), view.total_active);
+    try testing.expectEqual(@as(usize, 1), view.total_acked);
+}
+
+test "build: pass/skipped/err checks contribute no rows" {
+    // Build a panel with a pass result by hand.
+    const check_singleton = struct {
+        const c: observations.Check = .{
+            .name = "pass_check",
+            .label = "Pass Check",
+            // SAFETY: test-only Check; the panel is built with a
+            // pre-baked `.complete = .pass` result so `runChecks`
+            // never dispatches through this pointer.
+            .run = undefined,
+        };
+    };
+    const pending = try testing.allocator.alloc(observations.PendingCheck, 1);
+    pending[0] = .{
+        .check = &check_singleton.c,
+        .state = .{ .complete = .pass },
+    };
+    var panel = CheckPanel{ .allocator = testing.allocator, .io = std.testing.io, .pending = pending };
+    defer panel.deinit();
+
+    var journal = try makeEmptyJournal(testing.allocator);
+    defer journal.deinit();
+
+    var view = try build(testing.allocator, &panel, &journal, false);
+    defer view.deinit(testing.allocator);
+
+    try testing.expectEqual(@as(usize, 0), view.rows.len);
+    try testing.expectEqual(@as(usize, 0), view.total_active);
+    try testing.expectEqual(@as(usize, 0), view.total_acked);
+}

src/views/projections.zig

@@ -735,7 +735,7 @@ fn buildContextFromParts(
     var pos_returns: std.ArrayListUnmanaged(benchmark.PositionReturn) = .empty;
     defer pos_returns.deinit(alloc);
     for (allocations) |a| {
-        const candles_res = svc.getCachedCandles(a.symbol) orelse continue;
+        const candles_res = svc.getCachedCandles(alloc, a.symbol) orelse continue;
         defer candles_res.deinit();
         const candles = history.sliceCandlesAsOf(candles_res.data, as_of);
         if (candles.len > 0) {

src/views/review.zig

@@ -33,6 +33,7 @@ const analysis = @import("../analytics/analysis.zig");
 const performance = @import("../analytics/performance.zig");
 const risk = @import("../analytics/risk.zig");
 const portfolio_risk = @import("../analytics/portfolio_risk.zig");
+const observations_engine = @import("../analytics/observations.zig");
 const classification = @import("../models/classification.zig");
 const valuation = @import("../analytics/valuation.zig");
 const format = @import("../format.zig");
@@ -76,10 +77,11 @@ pub const SortDirection = enum {
 pub const ReviewRow = struct {
     /// Display ticker — same convention as `Allocation.display_symbol`.
     symbol: []const u8,
-    /// Mid-granularity sector label (`analytics/analysis.midBucket`
-    /// applied to the user's `metadata.srf` classification). Falls back
+    /// Sector bucket label (`entry.bucket` from the user's
+    /// `metadata.srf` classification, populated by
+    /// `parseClassificationFile` via `deriveBucket`). Falls back
     /// to "Unclassified" when the symbol has no classification entry.
-    sector_mid: []const u8,
+    bucket: []const u8,
     /// Fraction of the holding's market value in taxable accounts
     /// (0.0 = fully tax-advantaged, 1.0 = fully taxable). Computed
     /// from per-lot accounts; null when the AccountMap is missing
@@ -150,9 +152,15 @@ pub const ReviewView = struct {
     total_liquid: f64,
     /// Anchor portfolio file path for the header line. Borrowed.
     portfolio_path: []const u8,
+    /// Observation engine output. Null when the engine wasn't run
+    /// (e.g. unit tests of the renderer that fabricate views without
+    /// going through `buildReview`). Populated by `buildReview` after
+    /// computing rows + totals.
+    observations: ?observations_engine.CheckPanel = null,
 
     pub fn deinit(self: *ReviewView, allocator: std.mem.Allocator) void {
         allocator.free(self.rows);
+        if (self.observations) |*panel| panel.deinit();
     }
 };
 
@@ -175,6 +183,7 @@ pub const ReviewView = struct {
 /// `as_of` is the reference date for trailing-window math.
 pub fn buildReview(
     allocator: std.mem.Allocator,
+    io: std.Io,
     summary: valuation.PortfolioSummary,
     candle_map: *const std.StringHashMap([]const zfin.Candle),
     dividend_map: ?*const std.StringHashMap([]const zfin.Dividend),
@@ -199,7 +208,7 @@ pub fn buildReview(
         else
             null;
 
-        const sector_mid = sectorForSymbol(a.symbol, classifications);
+        const bucket = bucketForSymbol(a.symbol, classifications);
         const tax_pct = computeTaxPct(a.symbol, portfolio, account_map, as_of);
 
         const tr_returns = computeTrailingReturns(candles, dividends, as_of);
@@ -207,7 +216,7 @@ pub fn buildReview(
 
         try rows.append(allocator, .{
             .symbol = a.display_symbol,
-            .sector_mid = sector_mid,
+            .bucket = bucket,
             .tax_pct = tax_pct,
             .weight = a.weight,
             .return_1y = annualizedFromResult(tr_returns.one_year, false),
@@ -233,12 +242,26 @@ pub fn buildReview(
 
     const totals = computeTotals(rows.items, synth);
 
+    const rows_slice = try rows.toOwnedSlice(allocator);
+    errdefer allocator.free(rows_slice);
+
+    // Run the observations engine over the now-built rows + totals.
+    // Sync today; the API takes `io` so a future async dispatch path
+    // doesn't require a signature change.
+    const obs_ctx: observations_engine.CheckCtx = .{
+        .allocator = allocator,
+        .rows = rows_slice,
+        .totals = totals,
+    };
+    const panel = try observations_engine.runChecks(allocator, io, obs_ctx, &observations_engine.default_checks);
+
     return .{
-        .rows = try rows.toOwnedSlice(allocator),
+        .rows = rows_slice,
         .totals = totals,
         .as_of = as_of,
         .total_liquid = summary.total_value,
         .portfolio_path = portfolio_path,
+        .observations = panel,
     };
 }
 
@@ -406,7 +429,7 @@ fn sortLessThan(ctx: SortCtx, a: ReviewRow, b: ReviewRow) bool {
 fn extractStr(field: SortField, r: ReviewRow) []const u8 {
     return switch (field) {
         .symbol => r.symbol,
-        .sector => r.sector_mid,
+        .sector => r.bucket,
         else => unreachable,
     };
 }
@@ -451,26 +474,28 @@ fn sortFloatByDir(a: ?f64, b: ?f64, dir: SortDirection) bool {
 
 // ── Internal helpers ──────────────────────────────────────────
 
-/// Walk the classification map for a symbol. Returns the mid-granularity
-/// sector label (a static literal per `analysis.midBucket`) or
-/// "Unclassified" if no entry exists.
-fn sectorForSymbol(symbol: []const u8, classifications: classification.ClassificationMap) []const u8 {
-    // Find the FIRST matching classification entry. Multi-row
-    // classifications (target-date funds with proportional splits) are
-    // collapsed to their primary sector here — the review view is
-    // per-holding, not per-classification-component, so we pick the
-    // most-weighted entry.
-    var best_pct: f64 = 0;
-    var best_sector: ?[]const u8 = null;
-    for (classifications.entries) |e| {
+/// Resolve a symbol's display bucket. Walks the classification
+/// map; returns the most-weighted entry's `bucket` field. The
+/// `bucket` field is always populated by `parseClassificationFile`
+/// (via `deriveBucket` fallback when not user-curated), so the
+/// returned slice is always non-empty and borrows from the
+/// ClassificationMap's storage.
+fn bucketForSymbol(
+    symbol: []const u8,
+    classifications: classification.ClassificationMap,
+) []const u8 {
+    var best_pct: f64 = -1;
+    var best_idx: ?usize = null;
+    for (classifications.entries, 0..) |e, i| {
         if (!std.mem.eql(u8, e.symbol, symbol)) continue;
-        if (e.sector == null) continue;
         if (e.pct > best_pct) {
             best_pct = e.pct;
-            best_sector = e.sector;
+            best_idx = i;
         }
     }
-    if (best_sector) |s| return analysis.collapseSector(s, .mid);
+    if (best_idx) |i| {
+        if (classifications.entries[i].bucket) |b| return b;
+    }
     return "Unclassified";
 }
 
@@ -657,7 +682,7 @@ const testing = std.testing;
 fn makeRow(symbol: []const u8, sector: []const u8, weight: f64) ReviewRow {
     return .{
         .symbol = symbol,
-        .sector_mid = sector,
+        .bucket = sector,
         .tax_pct = null,
         .weight = weight,
         .return_1y = null,
@@ -745,9 +770,9 @@ test "sortRows: by symbol desc is reverse alphabetical (string desc path)" {
 
 test "sortRows: by tax_pct asc (covers ascending float comparator path)" {
     var rows = [_]ReviewRow{
-        .{ .symbol = "A", .sector_mid = "x", .tax_pct = 0.8, .weight = 0.1, .return_1y = null, .return_3y = null, .return_5y = null, .return_10y = null, .vol_3y = null, .vol_10y = null, .sharpe_3y = null, .sharpe_10y = null, .maxdd_5y = null },
-        .{ .symbol = "B", .sector_mid = "x", .tax_pct = 0.1, .weight = 0.1, .return_1y = null, .return_3y = null, .return_5y = null, .return_10y = null, .vol_3y = null, .vol_10y = null, .sharpe_3y = null, .sharpe_10y = null, .maxdd_5y = null },
-        .{ .symbol = "C", .sector_mid = "x", .tax_pct = 0.5, .weight = 0.1, .return_1y = null, .return_3y = null, .return_5y = null, .return_10y = null, .vol_3y = null, .vol_10y = null, .sharpe_3y = null, .sharpe_10y = null, .maxdd_5y = null },
+        .{ .symbol = "A", .bucket = "x", .tax_pct = 0.8, .weight = 0.1, .return_1y = null, .return_3y = null, .return_5y = null, .return_10y = null, .vol_3y = null, .vol_10y = null, .sharpe_3y = null, .sharpe_10y = null, .maxdd_5y = null },
+        .{ .symbol = "B", .bucket = "x", .tax_pct = 0.1, .weight = 0.1, .return_1y = null, .return_3y = null, .return_5y = null, .return_10y = null, .vol_3y = null, .vol_10y = null, .sharpe_3y = null, .sharpe_10y = null, .maxdd_5y = null },
+        .{ .symbol = "C", .bucket = "x", .tax_pct = 0.5, .weight = 0.1, .return_1y = null, .return_3y = null, .return_5y = null, .return_10y = null, .vol_3y = null, .vol_10y = null, .sharpe_3y = null, .sharpe_10y = null, .maxdd_5y = null },
     };
     sortRows(&rows, .tax_pct, .asc);
     try testing.expectEqualStrings("B", rows[0].symbol);
@@ -771,8 +796,8 @@ test "sortRows: every numeric SortField variant is reachable via extractFloat" {
     };
     inline for (numeric_fields) |field| {
         var rows = [_]ReviewRow{
-            .{ .symbol = "low", .sector_mid = "x", .tax_pct = 0.1, .weight = 0.1, .return_1y = 0.1, .return_3y = 0.1, .return_5y = 0.1, .return_10y = 0.1, .vol_3y = 0.1, .vol_10y = 0.1, .sharpe_3y = 0.1, .sharpe_10y = 0.1, .maxdd_5y = 0.1 },
-            .{ .symbol = "high", .sector_mid = "x", .tax_pct = 0.9, .weight = 0.9, .return_1y = 0.9, .return_3y = 0.9, .return_5y = 0.9, .return_10y = 0.9, .vol_3y = 0.9, .vol_10y = 0.9, .sharpe_3y = 0.9, .sharpe_10y = 0.9, .maxdd_5y = 0.9 },
+            .{ .symbol = "low", .bucket = "x", .tax_pct = 0.1, .weight = 0.1, .return_1y = 0.1, .return_3y = 0.1, .return_5y = 0.1, .return_10y = 0.1, .vol_3y = 0.1, .vol_10y = 0.1, .sharpe_3y = 0.1, .sharpe_10y = 0.1, .maxdd_5y = 0.1 },
+            .{ .symbol = "high", .bucket = "x", .tax_pct = 0.9, .weight = 0.9, .return_1y = 0.9, .return_3y = 0.9, .return_5y = 0.9, .return_10y = 0.9, .vol_3y = 0.9, .vol_10y = 0.9, .sharpe_3y = 0.9, .sharpe_10y = 0.9, .maxdd_5y = 0.9 },
         };
         sortRows(&rows, field, .desc);
         try testing.expectEqualStrings("high", rows[0].symbol);
@@ -924,23 +949,23 @@ test "sortGroupedByDefault: groups by sector then symbol asc within group" {
     // Sectors alphabetical: Bonds → Equity / Corporate → Technology.
     // Within each sector, symbols alphabetical (deterministic, easy
     // to scan for a specific ticker).
-    try testing.expectEqualStrings("Bonds", rows[0].sector_mid);
+    try testing.expectEqualStrings("Bonds", rows[0].bucket);
     try testing.expectEqualStrings("AGG", rows[0].symbol);
-    try testing.expectEqualStrings("Bonds", rows[1].sector_mid);
+    try testing.expectEqualStrings("Bonds", rows[1].bucket);
     try testing.expectEqualStrings("BND", rows[1].symbol);
-    try testing.expectEqualStrings("Equity / Corporate", rows[2].sector_mid);
+    try testing.expectEqualStrings("Equity / Corporate", rows[2].bucket);
     try testing.expectEqualStrings("VTI", rows[2].symbol);
-    try testing.expectEqualStrings("Technology", rows[3].sector_mid);
+    try testing.expectEqualStrings("Technology", rows[3].bucket);
     try testing.expectEqualStrings("AAPL", rows[3].symbol);
-    try testing.expectEqualStrings("Technology", rows[4].sector_mid);
+    try testing.expectEqualStrings("Technology", rows[4].bucket);
     try testing.expectEqualStrings("MSFT", rows[4].symbol);
 }
 
 test "sortRows: nulls sort to end on both directions" {
     var rows_desc = [_]ReviewRow{
-        .{ .symbol = "A", .sector_mid = "x", .tax_pct = null, .weight = 0.1, .return_1y = null, .return_3y = 0.10, .return_5y = null, .return_10y = null, .vol_3y = null, .vol_10y = null, .sharpe_3y = null, .sharpe_10y = null, .maxdd_5y = null },
-        .{ .symbol = "B", .sector_mid = "x", .tax_pct = null, .weight = 0.1, .return_1y = null, .return_3y = 0.20, .return_5y = null, .return_10y = null, .vol_3y = null, .vol_10y = null, .sharpe_3y = null, .sharpe_10y = null, .maxdd_5y = null },
-        .{ .symbol = "C", .sector_mid = "x", .tax_pct = null, .weight = 0.1, .return_1y = null, .return_3y = null, .return_5y = null, .return_10y = null, .vol_3y = null, .vol_10y = null, .sharpe_3y = null, .sharpe_10y = null, .maxdd_5y = null },
+        .{ .symbol = "A", .bucket = "x", .tax_pct = null, .weight = 0.1, .return_1y = null, .return_3y = 0.10, .return_5y = null, .return_10y = null, .vol_3y = null, .vol_10y = null, .sharpe_3y = null, .sharpe_10y = null, .maxdd_5y = null },
+        .{ .symbol = "B", .bucket = "x", .tax_pct = null, .weight = 0.1, .return_1y = null, .return_3y = 0.20, .return_5y = null, .return_10y = null, .vol_3y = null, .vol_10y = null, .sharpe_3y = null, .sharpe_10y = null, .maxdd_5y = null },
+        .{ .symbol = "C", .bucket = "x", .tax_pct = null, .weight = 0.1, .return_1y = null, .return_3y = null, .return_5y = null, .return_10y = null, .vol_3y = null, .vol_10y = null, .sharpe_3y = null, .sharpe_10y = null, .maxdd_5y = null },
     };
     sortRows(&rows_desc, .return_3y, .desc);
     try testing.expectEqualStrings("B", rows_desc[0].symbol); // 0.20
@@ -966,8 +991,8 @@ test "WeightedAvg: all-null returns null" {
 
 test "computeTotals: weighted average returns + synthetic risk pass-through" {
     const rows = [_]ReviewRow{
-        .{ .symbol = "A", .sector_mid = "x", .tax_pct = 1.0, .weight = 0.6, .return_1y = 0.10, .return_3y = 0.10, .return_5y = null, .return_10y = null, .vol_3y = 0.20, .vol_10y = null, .sharpe_3y = 1.0, .sharpe_10y = null, .maxdd_5y = 0.30 },
-        .{ .symbol = "B", .sector_mid = "y", .tax_pct = 0.0, .weight = 0.4, .return_1y = 0.20, .return_3y = 0.05, .return_5y = null, .return_10y = null, .vol_3y = 0.10, .vol_10y = null, .sharpe_3y = 0.5, .sharpe_10y = null, .maxdd_5y = 0.10 },
+        .{ .symbol = "A", .bucket = "x", .tax_pct = 1.0, .weight = 0.6, .return_1y = 0.10, .return_3y = 0.10, .return_5y = null, .return_10y = null, .vol_3y = 0.20, .vol_10y = null, .sharpe_3y = 1.0, .sharpe_10y = null, .maxdd_5y = 0.30 },
+        .{ .symbol = "B", .bucket = "y", .tax_pct = 0.0, .weight = 0.4, .return_1y = 0.20, .return_3y = 0.05, .return_5y = null, .return_10y = null, .vol_3y = 0.10, .vol_10y = null, .sharpe_3y = 0.5, .sharpe_10y = null, .maxdd_5y = 0.10 },
     };
     const synth: portfolio_risk.SyntheticRisk = .{
         .vol_3y = 0.13,
@@ -989,37 +1014,40 @@ test "computeTotals: weighted average returns + synthetic risk pass-through" {
     try testing.expectApproxEqAbs(@as(f64, 0.18), t.maxdd_5y.?, 0.0001);
 }
 
-test "sectorForSymbol: returns Unclassified for unknown symbol" {
+test "bucketForSymbol: returns Unclassified for unknown symbol" {
     var entries = [_]classification.ClassificationEntry{};
     const cm: classification.ClassificationMap = .{
         .entries = entries[0..],
         .allocator = testing.allocator,
     };
-    try testing.expectEqualStrings("Unclassified", sectorForSymbol("NOPE", cm));
+    try testing.expectEqualStrings("Unclassified", bucketForSymbol("NOPE", cm));
 }
 
-test "sectorForSymbol: returns mid-bucket for classified symbol" {
+test "bucketForSymbol: returns bucket field directly (parser pre-fills it)" {
+    // bucketForSymbol no longer calls deriveBucket — it just
+    // reads `entry.bucket` which is pre-populated by
+    // `parseClassificationFile`. Tests that synthesize entries
+    // by hand must set `bucket` themselves.
     var entries = [_]classification.ClassificationEntry{
-        .{ .symbol = "AAPL", .sector = "Technology", .pct = 100.0 },
+        .{ .symbol = "AAPL", .bucket = "Technology", .sector = "Technology", .pct = 100.0 },
     };
     const cm: classification.ClassificationMap = .{
         .entries = entries[0..],
         .allocator = testing.allocator,
     };
-    // Technology is a GICS sector; mid-bucket passes it through.
-    try testing.expectEqualStrings("Technology", sectorForSymbol("AAPL", cm));
+    try testing.expectEqualStrings("Technology", bucketForSymbol("AAPL", cm));
 }
 
-test "sectorForSymbol: collapses NPORT-P sub-flavors via mid-bucket" {
+test "bucketForSymbol: multi-row entry picks most-weighted row's bucket" {
     var entries = [_]classification.ClassificationEntry{
-        .{ .symbol = "BND", .sector = "Debt / US Treasury", .pct = 100.0 },
+        .{ .symbol = "X", .bucket = "Minor", .pct = 10.0 },
+        .{ .symbol = "X", .bucket = "Major", .pct = 90.0 },
     };
     const cm: classification.ClassificationMap = .{
         .entries = entries[0..],
         .allocator = testing.allocator,
     };
-    // "Debt / *" maps to "Bonds" at mid granularity.
-    try testing.expectEqualStrings("Bonds", sectorForSymbol("BND", cm));
+    try testing.expectEqualStrings("Major", bucketForSymbol("X", cm));
 }
 
 test "volIntent: thresholds bucketize correctly" {
@@ -1126,8 +1154,12 @@ test "buildReview: end-to-end with testing allocator (leak check)" {
     defer candle_map.deinit();
 
     var class_entries = [_]classification.ClassificationEntry{
-        .{ .symbol = "VTI", .sector = "Equity / Corporate", .pct = 100.0 },
-        .{ .symbol = "BND", .sector = "Debt / US Treasury", .pct = 100.0 },
+        // bucket is normally pre-filled by parseClassificationFile;
+        // hand-synthesized entries set it explicitly. For NPORT-P-
+        // style sectors with geo+asset_class, deriveBucket would
+        // produce the composite "{geo} {asset_class}".
+        .{ .symbol = "VTI", .bucket = "US ETF", .sector = "Equity / Corporate", .geo = "US", .asset_class = "ETF", .pct = 100.0 },
+        .{ .symbol = "BND", .bucket = "US Fund", .sector = "Debt / US Treasury", .geo = "US", .asset_class = "Fund", .pct = 100.0 },
     };
     const cm: classification.ClassificationMap = .{
         .entries = class_entries[0..],
@@ -1145,6 +1177,7 @@ test "buildReview: end-to-end with testing allocator (leak check)" {
 
     var view = try buildReview(
         testing.allocator,
+        std.testing.io,
         summary,
         &candle_map,
         null, // no dividend map
@@ -1161,13 +1194,15 @@ test "buildReview: end-to-end with testing allocator (leak check)" {
 
     // Tax%: VTI is in taxable account (1.0), BND in Roth (0.0).
     // Find each row by symbol since order isn't yet sorted.
+    // Buckets: both are NPORT-P-style sectors so deriveBucket
+    // falls through to the (geo, asset_class) composite.
     for (view.rows) |r| {
         if (std.mem.eql(u8, r.symbol, "VTI")) {
             try testing.expectApproxEqAbs(@as(f64, 1.0), r.tax_pct.?, 0.001);
-            try testing.expectEqualStrings("Equity / Corporate", r.sector_mid);
+            try testing.expectEqualStrings("US ETF", r.bucket);
         } else if (std.mem.eql(u8, r.symbol, "BND")) {
             try testing.expectApproxEqAbs(@as(f64, 0.0), r.tax_pct.?, 0.001);
-            try testing.expectEqualStrings("Bonds", r.sector_mid);
+            try testing.expectEqualStrings("US Fund", r.bucket);
         }
     }