diff --git a/crates/ledger-cli/tests/attention_parity.rs b/crates/ledger-cli/tests/attention_parity.rs index 9ae3c8d..0a14ec4 100644 --- a/crates/ledger-cli/tests/attention_parity.rs +++ b/crates/ledger-cli/tests/attention_parity.rs @@ -1,14 +1,19 @@ //! The viewer's "Needs attention" and `books.start`'s next steps are one //! list: both come from `ledger_package::attention`, the viewer over its //! `Snapshot`, the server over its `Store`. This drives both on the same -//! books (the demo company, and a book written entry by entry) and compares -//! them item for item. +//! books (the demo company, a book written entry by entry, and that book +//! after its stored file and its database are damaged) and compares them +//! item for item. #![cfg(feature = "gui")] +use std::collections::HashSet; +use std::time::Instant; + use chrono::NaiveDate; use ledger_core::{AccountType, Currency, Money, Side}; use ledger_gui::{Source, data}; use rust_decimal::Decimal; +use serde_json::Value; use serde_json::json; mod common; @@ -20,13 +25,18 @@ fn usd(cents: i64) -> Money { Money::new(Decimal::new(cents, 2), Currency::USD) } +fn start(wire: &mut common::Wire) -> Value { + common::answer_of(&wire.call("books.start", json!({"focus": "daily"}))) + .unwrap_or_else(|e| panic!("books.start: {e}")) +} + /// The items the viewer draws and the next steps the server lists: the /// server's attention steps are the ones whose `why` is one of the viewer's /// titles, and they must be the same titles, in the same order. The viewer -/// has at most 12 (books.start lists the top 12). -fn compare(place: &common::Place, wire: &mut common::Wire, today: NaiveDate) { - let answer = common::answer_of(&wire.call("books.start", json!({"focus": "daily"}))) - .unwrap_or_else(|e| panic!("books.start: {e}")); +/// has at most 12 (books.start lists the top 12). Returns the viewer's +/// titles. +fn compare(place: &common::Place, wire: &mut common::Wire, today: NaiveDate) -> Vec { + let answer = start(wire); let books = data::load_once(&Source { data_dir: place.books_data(), today: Some(today), @@ -53,11 +63,13 @@ fn compare(place: &common::Place, wire: &mut common::Wire, today: NaiveDate) { assert_eq!(&a.title, title); assert!(a.prompt.contains("wanderbooks"), "{}", a.prompt); } + viewer } /// A book written entry by entry, pseudo-randomly but deterministically (a /// fixed seed, so a failure reproduces): a chart, three drafts, posted -/// entries without receipts, and one that names no counterparty. +/// entries without receipts, one that names no counterparty, and one with +/// a filed receipt (the blob the damage test removes). fn write_random(dir: &std::path::Path, today: NaiveDate) { let _ = std::fs::remove_dir_all(dir); let mut st = ledger_store::Store::create(dir, Currency::USD).unwrap(); @@ -132,6 +144,29 @@ fn write_random(dir: &std::path::Path, today: NaiveDate) { drop(st); } +/// A blob's file in the content-addressed store (`//`). +fn blob_path(data_dir: &std::path::Path, sha: &str) -> std::path::PathBuf { + data_dir + .join("blobs") + .join(&sha[0..2]) + .join(&sha[2..4]) + .join(sha) +} + +/// The viewer's items and the server's steps, and both name `expected`. +fn compare_and_expect( + place: &common::Place, + wire: &mut common::Wire, + today: NaiveDate, + expected: &str, +) { + let viewer = compare(place, wire, today); + assert!( + viewer.iter().any(|t| t.contains(expected)), + "the viewer should say {expected:?}: {viewer:?}" + ); +} + #[test] fn the_viewer_and_books_start_see_the_same_attention() { let today = ledger_core::settings::today_in(None); @@ -150,13 +185,68 @@ fn the_viewer_and_books_start_see_the_same_attention() { let mut wire = place.start(None); wire.initialize("attention-parity"); compare(&place, &mut wire, today); + // What the real integrity check costs books.start: the cold call + // (which also opens the store) and a warm one, and how many stored + // files it hashes (one per distinct blob, as the viewer does). + let books = data::load_once(&Source { + data_dir: place.books_data(), + today: Some(today), + }) + .unwrap(); + let blobs: HashSet<&str> = books + .attachments + .iter() + .map(|a| a.blob_sha.as_str()) + .collect(); + let cold = Instant::now(); + let _ = start(&mut wire); + let cold = cold.elapsed(); + let warm = Instant::now(); + let _ = start(&mut wire); + let warm = warm.elapsed(); + println!( + "books.start on the demo: {} attachments, {} distinct blobs; cold {cold:?}, warm {warm:?}", + books.attachments.len(), + blobs.len() + ); } - // A book written entry by entry. + // A book written entry by entry, then damaged: a stored file removed + // and the database changed from outside. Both lists must still agree, + // and both must name the damage (a clean-books-only test missed this). { let place = common::Place::new("attention-parity-random"); write_random(&place.books_data(), today); let mut wire = place.start(Some(SETTINGS)); wire.initialize("attention-parity"); compare(&place, &mut wire, today); + + // The stored file: remove its bytes (the blob store never deletes; + // the test does, on its own books). + let books = data::load_once(&Source { + data_dir: place.books_data(), + today: Some(today), + }) + .unwrap(); + let sha = books + .attachments + .first() + .expect("the fixture filed a receipt") + .blob_sha + .clone(); + let path = blob_path(&place.books_data(), &sha); + std::fs::remove_file(&path).unwrap(); + compare_and_expect(&place, &mut wire, today, "damaged or missing"); + + // The database: change a projected row the journal doesn't say. + let db = ledger_store::database_in_use(&place.books_data()); + let conn = rusqlite::Connection::open(&db).unwrap(); + conn.execute( + "UPDATE accounts SET name = name || ' (changed outside the journal)' \ + WHERE id = (SELECT MIN(id) FROM accounts)", + [], + ) + .unwrap(); + drop(conn); + compare_and_expect(&place, &mut wire, today, "doesn't match the journal"); } } diff --git a/crates/ledger-mcp/src/tools/kept.rs b/crates/ledger-mcp/src/tools/kept.rs index fffd0fd..96c622e 100644 --- a/crates/ledger-mcp/src/tools/kept.rs +++ b/crates/ledger-mcp/src/tools/kept.rs @@ -834,12 +834,7 @@ impl BooksServer { financing_accounts: &cfg.roles.financing, investing_accounts: &cfg.roles.investing, }; - let attention = ledger_package::find_with( - store, - today, - &attention_settings, - &ledger_package::Integrity::default(), - ); + let attention = ledger_package::find(store, today, &attention_settings); for a in attention.iter().filter(|a| a.tool.is_some()).take(12) { let Some((tool, args)) = &a.tool else { continue; diff --git a/crates/ledger-package/src/attention.rs b/crates/ledger-package/src/attention.rs index 2c6fb73..ab7f33f 100644 --- a/crates/ledger-package/src/attention.rs +++ b/crates/ledger-package/src/attention.rs @@ -192,12 +192,21 @@ impl Default for Integrity { impl Integrity { /// Check the database and every stored file: the attachment's bytes - /// are read (and hash-checked) through the books. + /// are read (and hash-checked) through the books, once per blob that + /// is intact; a damaged blob is reported for each attachment filed + /// from it. pub fn check(books: &impl PackageBooks) -> Integrity { + let mut intact: std::collections::HashSet = std::collections::HashSet::new(); let mut file_failures = Vec::new(); for a in books.all_attachments().unwrap_or_default() { - if let Err(e) = books.attachment_bytes(a.id) { - file_failures.push((a.id, e.to_string())); + if intact.contains(&a.blob_sha) { + continue; + } + match books.attachment_bytes(a.id) { + Ok(_) => { + intact.insert(a.blob_sha.clone()); + } + Err(e) => file_failures.push((a.id, e.to_string())), } } Integrity { diff --git a/crates/ledger-package/src/books.rs b/crates/ledger-package/src/books.rs index 839971e..deb0d4b 100644 --- a/crates/ledger-package/src/books.rs +++ b/crates/ledger-package/src/books.rs @@ -62,9 +62,8 @@ pub trait PackageBooks { fn reconciliations(&self) -> Result, StoreError>; /// How the writer's database compares with the journal. A snapshot - /// compares them; the store is the only writer and applies each line - /// before it returns (a failed apply heals on its next open), so it - /// reports a match. + /// compares them; the store makes the same comparison (see its + /// implementation). fn disk_check(&self) -> DiskCheck { DiskCheck::Matches } @@ -104,6 +103,17 @@ impl PackageBooks for Store { fn reconciliations(&self) -> Result, StoreError> { Store::reconciliations(self, None) } + + /// The same comparison the viewer makes, not an assumption: the + /// store's database is the file the viewer checks, and it can be + /// damaged or changed from outside. Loading a snapshot of the data + /// directory compares it against a fresh projection of the journal. + fn disk_check(&self) -> DiskCheck { + match Snapshot::load(Store::data_dir(self)) { + Ok(s) => Snapshot::check_disk(&s), + Err(e) => DiskCheck::Unreadable(e.to_string()), + } + } } impl PackageBooks for Snapshot { diff --git a/docs/testing.md b/docs/testing.md index 125c099..840fee7 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -2927,7 +2927,9 @@ steps, `why` equal to the title, at most 12. `PackageBooks` gained `settings`, `journal`, `statements`, `reconciliations` and `disk_check`, so the engine can read an entry's origin from the journal's event types (as the viewer's builder did) and the same statements, reconciliations and -database check through either view. +database check through either view. `Store::disk_check` makes the viewer's +comparison (a snapshot of the data directory) rather than assuming a +match; `books.start` calls `find`, which runs the real check. - **ledger-package (unit tests in `src/attention.rs`):** none of its own (the engine's conditions are exercised end to end); the crate's tests @@ -2941,9 +2943,13 @@ database check through either view. `tests/edges.rs`, `tests/old_books.rs`, `tests/parties.rs`, `tests/spec_gui.rs` exercise the items through the viewer's wrapper. - **ledger-cli:** `tests/attention_parity.rs` - `the_viewer_and_books_start_see_the_same_attention` (the demo books and a - book written entry by entry: the viewer's titles, in order, are exactly - `books.start`'s attention steps). + `the_viewer_and_books_start_see_the_same_attention` (the demo books, a + book written entry by entry, and that book after its stored file is + removed and its database is changed from outside: the viewer's titles, + in order, are exactly `books.start`'s attention steps, and both name the + `stored_files` and `database` items). The demo half measures + `books.start` with the real check (138 attachments and blobs: about + 0.26 s warm on a debug build). - **unchanged:** `crates/ledger-cli/tests/spec_kept.rs`'s `books.start` tests (the fixed steps keep their order; attention items sit between the backup step and the focus). @@ -2960,5 +2966,7 @@ not marked on file) and `files_1099`'s `any`; `paid_this_year`'s `max` `period_month`'s split; the severity arms (undetermined 1099, refuse tax category, parties require); `dollars`' thousands grouping and its negative-zero test; `filter(|a| a.tool.is_some()).take(12)` and -`step_title`'s 160-character cut in `start_out`; the viewer wrapper's +`step_title`'s 160-character cut in `start_out`; `Integrity::check`'s +intact-blob set (a damaged blob reported per attachment) and `Store:: +disk_check`'s snapshot load and `Unreadable` arm; the viewer wrapper's `Target` arms. diff --git a/plan/served.md b/plan/served.md index c285b80..fa6a457 100644 --- a/plan/served.md +++ b/plan/served.md @@ -1246,8 +1246,15 @@ Each unit follows the 8.0.4 template. "A-n" is an owner answer numbered in 8.3.1 - `books.start` lists at most 12 attention steps (the eventual `needs_attention` top 12; counts by kind come with P5), each step's `why` cut to 160 characters (a free-text description could otherwise - push the result past 8 KB), and passes `Integrity::default()`: the - server's own damage check is `books.verify`. + push the result past 8 KB), and calls `find` — the real integrity + check, so a damaged database or stored file produces the same items + for the server as for the viewer. `Store::disk_check` makes the + viewer's comparison (a snapshot of its data directory) rather than + assuming a match; the check hashes each distinct stored blob once and + replays the journal once. On the demo books (138 attachments, 138 + distinct blobs, debug build) `books.start` takes about 0.26 s warm, + 0.29 s cold; a real book's cost scales with its stored bytes, and a + cache would be the next step if that ever exceeds a second. - The W-9 and reconciliation prompts and the 1099 predicate moved to `ledger-package`; `pages/parties.rs`, `pages/reconciliation.rs` and `Books::files_1099` delegate to them (two files beyond the Build's