From cf3e06c9101c0a18d99d789369f54a9b182168ab Mon Sep 17 00:00:00 2001 From: le king fu Date: Tue, 9 Jun 2026 21:31:34 -0400 Subject: [PATCH] fix(balance): scope v16 abort guard to convertible accounts (#228) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v16 belt-and-suspenders abort guard keyed on `a.symbol IS NOT NULL` instead of convertibility. A simple-category account carrying a residual symbol (left by a priced→simple recategorization; AccountForm renders the symbol field unconditionally) has quantity-NULL lines by construction; they satisfied the over-broad predicate, so the guard inserted 0, the CHECK(ok = 1) failed, and the whole v16 migration aborted — the app no longer started for that profile. JOIN balance_categories and require `c.asset_type IS NOT NULL` in the guard subquery, in all three copies (Migration v16, V16_SQL mirror, and the V16_CORRUPT injected-failure test, kept statement-equivalent). Add a regression test seeding a simple account with a residual symbol plus a quantity-NULL line: v16 now applies without abort and leaves the account intact (not converted, no security/holding, qty/value preserved). Modifying v16 in place is safe: it has never shipped in a tagged release (v0.9.1 stopped at v13; v14-v16 are unreleased), so no persisted profile carries its checksum yet. Co-Authored-By: Claude Opus 4.8 (1M context) --- src-tauri/src/lib.rs | 122 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 119 insertions(+), 3 deletions(-) diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index b11a659..4549108 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -306,7 +306,8 @@ pub fn run() { INSERT INTO _v16_guard(ok) SELECT CASE WHEN EXISTS ( \ SELECT 1 FROM balance_snapshot_lines sl \ JOIN balance_accounts a ON a.id = sl.account_id \ - WHERE a.symbol IS NOT NULL AND sl.quantity IS NULL \ + JOIN balance_categories c ON c.id = a.balance_category_id \ + WHERE a.symbol IS NOT NULL AND c.asset_type IS NOT NULL AND sl.quantity IS NULL \ AND NOT EXISTS (SELECT 1 FROM balance_snapshot_holdings h WHERE h.snapshot_line_id = sl.id) \ ) THEN 0 ELSE 1 END; \ DROP TABLE _v16_guard;", @@ -2306,7 +2307,8 @@ mod tests { INSERT INTO _v16_guard(ok) SELECT CASE WHEN EXISTS ( \ SELECT 1 FROM balance_snapshot_lines sl \ JOIN balance_accounts a ON a.id = sl.account_id \ - WHERE a.symbol IS NOT NULL AND sl.quantity IS NULL \ + JOIN balance_categories c ON c.id = a.balance_category_id \ + WHERE a.symbol IS NOT NULL AND c.asset_type IS NOT NULL AND sl.quantity IS NULL \ AND NOT EXISTS (SELECT 1 FROM balance_snapshot_holdings h WHERE h.snapshot_line_id = sl.id) \ ) THEN 0 ELSE 1 END; \ DROP TABLE _v16_guard;"; @@ -2495,6 +2497,115 @@ mod tests { assert_eq!(v, 21.0, "intact line keeps its value"); } + #[test] + fn migration_v16_leaves_simple_account_with_residual_symbol_intact() { + // Regression #228: the v16 abort guard keyed on `a.symbol IS NOT NULL` + // instead of convertibility. A SIMPLE-category account carrying a + // residual symbol (left by a priced→simple recategorization; AccountForm + // renders the symbol field unconditionally and updateBalanceAccount + // preserves it) has quantity-NULL lines by construction. Those satisfied + // the over-broad guard predicate (symbol NOT NULL AND qty NULL AND no + // holding) → guard inserts 0 → CHECK(ok = 1) fails → the whole v16 + // migration aborts → the app no longer starts for that profile. The fix + // scopes the guard to convertible accounts (c.asset_type IS NOT NULL). + let conn = db_through_v13(); + conn.execute_batch(V14_SQL).expect("apply v14"); + conn.execute_batch(V15_SQL).expect("apply v15"); + + // Simple category: kind = 'simple', asset_type NULL by construction. + conn.execute( + "INSERT INTO balance_categories (key, i18n_key, kind, sort_order, is_seed) \ + VALUES ('custom_simple', 'custom', 'simple', 90, 0)", + [], + ) + .unwrap(); + let simple_cat: i64 = conn + .query_row("SELECT last_insert_rowid()", [], |r| r.get(0)) + .unwrap(); + + // Account under the simple category, carrying a residual symbol. + conn.execute( + "INSERT INTO balance_accounts (balance_category_id, name, symbol, kind) \ + VALUES (?1, 'Compte residuel', 'RESID', 'simple')", + [simple_cat], + ) + .unwrap(); + let acc: i64 = conn + .query_row("SELECT last_insert_rowid()", [], |r| r.get(0)) + .unwrap(); + + // One snapshot line with quantity NULL (simple-kind line invariant). + conn.execute( + "INSERT INTO balance_snapshots (snapshot_date) VALUES ('2026-06-01')", + [], + ) + .unwrap(); + let snap_id: i64 = conn + .query_row("SELECT last_insert_rowid()", [], |r| r.get(0)) + .unwrap(); + conn.execute( + "INSERT INTO balance_snapshot_lines \ + (snapshot_id, account_id, quantity, unit_price, value, price_source) \ + VALUES (?1, ?2, NULL, NULL, 123.0, 'manual')", + rusqlite::params![snap_id, acc], + ) + .unwrap(); + let line: i64 = conn + .query_row("SELECT last_insert_rowid()", [], |r| r.get(0)) + .unwrap(); + + // v16 must apply cleanly. Pre-fix, the over-broad guard flagged this line + // (symbol NOT NULL, qty NULL, no holding) and aborted the whole migration. + conn.execute_batch(V16_SQL) + .expect("v16 must not abort on a simple account with a residual symbol"); + + // The residual symbol minted no security (simple category has no + // asset_type → excluded from the securities INSERT, step 1). + let resid_secs: i64 = conn + .query_row( + "SELECT COUNT(*) FROM balance_securities WHERE symbol = 'RESID'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(resid_secs, 0, "no security for a simple account's residual symbol"); + + // No holding attached to the line. + let holdings: i64 = conn + .query_row( + "SELECT COUNT(*) FROM balance_snapshot_holdings WHERE snapshot_line_id = ?1", + [line], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(holdings, 0, "no holding minted for the simple account"); + + // The line is untouched (qty/unit_price stay NULL, value preserved). + let (l_qty, l_price, l_val): (Option, Option, f64) = conn + .query_row( + "SELECT quantity, unit_price, value FROM balance_snapshot_lines WHERE id = ?1", + [line], + |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)), + ) + .unwrap(); + assert!(l_qty.is_none(), "simple line quantity stays NULL"); + assert!(l_price.is_none(), "simple line unit_price stays NULL"); + assert_eq!(l_val, 123.0, "simple line value preserved"); + + // The account itself is NOT converted: kind stays 'simple', the residual + // symbol is preserved, and detailed_since stays NULL (criterion: intact). + let (a_kind, a_symbol, a_detailed_since): (String, Option, Option) = conn + .query_row( + "SELECT kind, symbol, detailed_since FROM balance_accounts WHERE id = ?1", + [acc], + |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)), + ) + .unwrap(); + assert_eq!(a_kind, "simple", "simple account must not be converted to detailed"); + assert_eq!(a_symbol.as_deref(), Some("RESID"), "residual symbol preserved"); + assert!(a_detailed_since.is_none(), "detailed_since stays NULL"); + } + #[test] fn migration_v16_is_idempotent_on_rerun() { let (conn, line_a, _line_b) = db_pre_v16(); @@ -2534,6 +2645,10 @@ mod tests { // The corrupted batch: step 1 + step 3 + guard, with step 2 (holdings) // intentionally removed. NULLing now happens on lines with no holding. + // The guard carries the same narrowed predicate as production (#228): the + // abort is driven by the convertible line_a (AAPL, asset_type = 'stock'), + // NULLed without a holding; line_b (asset_type NULL) no longer trips the + // guard, but the convertible line alone is enough to prove abort+rollback. const V16_CORRUPT: &str = "\ INSERT INTO balance_securities (symbol, currency, asset_type) \ SELECT DISTINCT UPPER(TRIM(a.symbol)), a.currency, c.asset_type \ @@ -2547,7 +2662,8 @@ mod tests { INSERT INTO _v16_guard(ok) SELECT CASE WHEN EXISTS ( \ SELECT 1 FROM balance_snapshot_lines sl \ JOIN balance_accounts a ON a.id = sl.account_id \ - WHERE a.symbol IS NOT NULL AND sl.quantity IS NULL \ + JOIN balance_categories c ON c.id = a.balance_category_id \ + WHERE a.symbol IS NOT NULL AND c.asset_type IS NOT NULL AND sl.quantity IS NULL \ AND NOT EXISTS (SELECT 1 FROM balance_snapshot_holdings h WHERE h.snapshot_line_id = sl.id) \ ) THEN 0 ELSE 1 END; \ DROP TABLE _v16_guard;";