Money import: keep closed accounts, cleared/reconciled state and transaction notes; plain-language review #1130

Open
opened 2026-10-05 08:42:47 +00:00 by kayg · 10 comments
Owner

Owner report (2026-10-05, importing the real Actual budget "Mi Casa": 86 accounts, 255 categories, 14,783 transactions over 63 months)

The review lists "Features not imported" with internal keys as labels. Data losses that must be fixed before the owner imports:

  1. closed account state (54): closed Actual accounts import as open. Preserve the closed state (closed accounts hidden from active lists, kept for history).
  2. reconciliation lock state (14,749): the reconciled/cleared marks are dropped. Preserve cleared and reconciled state per transaction.
  3. transaction notes joined with payee (4,149): notes are merged into the payee text. Keep notes as a separate transaction field (show them in the register).
    Also: account kind guessed from label (4): ask in the review step (pick the type) instead of guessing silently. Unsupported templates (417) and goal modes (517): keep the source text on the category so nothing is lost, and say so.

The review must explain every remaining difference in plain language ("54 accounts are closed in Actual and will be imported as closed"), never internal keys. Owner rule: no jargon in UI labels. Money changes need property tests plus a separate break-the-numbers review before merge.

## Owner report (2026-10-05, importing the real Actual budget "Mi Casa": 86 accounts, 255 categories, 14,783 transactions over 63 months) The review lists "Features not imported" with internal keys as labels. Data losses that must be fixed before the owner imports: 1. **closed account state (54)**: closed Actual accounts import as open. Preserve the closed state (closed accounts hidden from active lists, kept for history). 2. **reconciliation lock state (14,749)**: the reconciled/cleared marks are dropped. Preserve cleared and reconciled state per transaction. 3. **transaction notes joined with payee (4,149)**: notes are merged into the payee text. Keep notes as a separate transaction field (show them in the register). Also: **account kind guessed from label (4)**: ask in the review step (pick the type) instead of guessing silently. Unsupported templates (417) and goal modes (517): keep the source text on the category so nothing is lost, and say so. The review must explain every remaining difference in plain language ("54 accounts are closed in Actual and will be imported as closed"), never internal keys. Owner rule: no jargon in UI labels. Money changes need property tests plus a separate break-the-numbers review before merge.
Author
Owner

Started #1130 on job/moneyfid-1130, base 559e05cb9292080684cd65a5c1adcf871c1d0647. Reading Actual normalization, the Markdown schema and register APIs. Will preserve source fields and verify exact balances with property tests and a separate numbers check. No pushes or deploys.

Started #1130 on `job/moneyfid-1130`, base `559e05cb9292080684cd65a5c1adcf871c1d0647`. Reading Actual normalization, the Markdown schema and register APIs. Will preserve source fields and verify exact balances with property tests and a separate numbers check. No pushes or deploys.
Author
Owner

Finding: source notes can contain <!--, which makes an unescaped JSON property opaque to the Money Markdown reader and can hide later postings. Added one shared JSON-property encoder that escapes angle brackets and decodes back to the exact text. The production-format upgrade regression includes this case.

Committed the core schema slice as c85719904. cargo test -p calternal-money passed, the focused production-v1 upgrade test passed, and cargo clippy -p calternal-money --all-targets -- -D warnings passed. There are no Money SQL migrations in this revision or the captured production schema; the new optional Markdown fields need no Index migration.

Decision: preserve split children and transfer legs as non-posting source rows; simple transactions use only separate notes/status properties to avoid duplicating their full source fields. Ambiguous Card accounts require an explicit Cash/Card choice followed by a new verified preview before confirmation.

Finding: source notes can contain `<!--`, which makes an unescaped JSON property opaque to the Money Markdown reader and can hide later postings. Added one shared JSON-property encoder that escapes angle brackets and decodes back to the exact text. The production-format upgrade regression includes this case. Committed the core schema slice as `c85719904`. `cargo test -p calternal-money` passed, the focused production-v1 upgrade test passed, and `cargo clippy -p calternal-money --all-targets -- -D warnings` passed. There are no Money SQL migrations in this revision or the captured production schema; the new optional Markdown fields need no Index migration. Decision: preserve split children and transfer legs as non-posting source rows; simple transactions use only separate notes/status properties to avoid duplicating their full source fields. Ambiguous Card accounts require an explicit Cash/Card choice followed by a new verified preview before confirmation.
Author
Owner

Actual fidelity implementation committed as 80112167772c28d7fc134c9710a8fce4d42c611e. Property tests cover every simple Actual row through both adapters, and the separate raw-sum check covers differently marked transfer legs and split children. The full Money Plugin test run passes: test result: ok. 84 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 59.47s (see final report for exact final gate output). The unchanged byte-preservation tests caught new status child lines on legacy rows; fixed the code to keep the old #cleared form, rather than changing those expectations.

Actual fidelity implementation committed as `80112167772c28d7fc134c9710a8fce4d42c611e`. Property tests cover every simple Actual row through both adapters, and the separate raw-sum check covers differently marked transfer legs and split children. The full Money Plugin test run passes: `test result: ok. 84 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 59.47s` (see final report for exact final gate output). The unchanged byte-preservation tests caught new status child lines on legacy rows; fixed the code to keep the old #cleared form, rather than changing those expectations.
Author
Owner

UI and focused verification are committed through 305948e53; merged origin/dev once at e58ee0db7. The import review now requires Cash/Card choices for source accounts that previously relied on a Card name guess, reruns exact verification after choices, and uses human sentences for structured counts. The register shows separate Notes, closed-account history and locked reconciled marks; transfer cleared edits have per-account Undo.

The production web build passed. Focused Vitest: Tests 30 passed (30). The full web check stops before Svelte at a stale app-sidebar.svelte performance exception on origin/dev, filed as #1133. Existing Money syntax pins were renewed without increasing debt ceilings.

A final coverage audit found that Tracking postings use a system category. The writer now also retains their original Category identity as source metadata; the property fixture uses a real Category on Tracking rows. No budget posting is added. Rust server gates are running with four jobs, no incremental builds and system OpenSSL. Screenshots follow from the current production build.

UI and focused verification are committed through `305948e53`; merged `origin/dev` once at `e58ee0db7`. The import review now requires Cash/Card choices for source accounts that previously relied on a Card name guess, reruns exact verification after choices, and uses human sentences for structured counts. The register shows separate Notes, closed-account history and locked reconciled marks; transfer cleared edits have per-account Undo. The production web build passed. Focused Vitest: `Tests 30 passed (30)`. The full web check stops before Svelte at a stale `app-sidebar.svelte` performance exception on origin/dev, filed as #1133. Existing Money syntax pins were renewed without increasing debt ceilings. A final coverage audit found that Tracking postings use a system category. The writer now also retains their original Category identity as source metadata; the property fixture uses a real Category on Tracking rows. No budget posting is added. Rust server gates are running with four jobs, no incremental builds and system OpenSSL. Screenshots follow from the current production build.
Author
Owner

The expanded Actual property test now checks ordinary rows, split parents/children, paired transfers and Tracking category evidence. Transfer dates may differ within one source month; each register leg now uses its own Date, Notes and marks. The separate raw-row balance oracle and the plugin suite pass.

Closed Accounts have a separate history section built from the existing SidebarLinks component. Notes edits and cleared marks have Undo. Deferred Undo retains the original Budget and the existing User/lifecycle fence, and refuses to replace an already changed row. Focused Vitest: Tests 34 passed (34).

The server full suite reported this SLOW-only Calendar fixture, unrelated to Money:

search_reconcile_calendar_write_profile files=20000 writes=32 p50_ms=12447 p95_ms=13251 p95_budget_ms=10000
Calendar write p95 13.251828141s exceeded 10s
test result: FAILED. 254 passed; 1 failed; 10 ignored; 0 measured; 0 filtered out; finished in 237.79s

Per the job policy, the assertion stays unchanged and the full suite is not rerun for host latency. All non-SLOW failures found by this job have been fixed or filed (#1133). Final source-evidence schemas now describe their fields explicitly instead of exposing an untyped object. The final contract build and production screenshots are in progress.

The expanded Actual property test now checks ordinary rows, split parents/children, paired transfers and Tracking category evidence. Transfer dates may differ within one source month; each register leg now uses its own Date, Notes and marks. The separate raw-row balance oracle and the plugin suite pass. Closed Accounts have a separate history section built from the existing SidebarLinks component. Notes edits and cleared marks have Undo. Deferred Undo retains the original Budget and the existing User/lifecycle fence, and refuses to replace an already changed row. Focused Vitest: `Tests 34 passed (34)`. The server full suite reported this SLOW-only Calendar fixture, unrelated to Money: ```text search_reconcile_calendar_write_profile files=20000 writes=32 p50_ms=12447 p95_ms=13251 p95_budget_ms=10000 Calendar write p95 13.251828141s exceeded 10s test result: FAILED. 254 passed; 1 failed; 10 ignored; 0 measured; 0 filtered out; finished in 237.79s ``` Per the job policy, the assertion stays unchanged and the full suite is not rerun for host latency. All non-SLOW failures found by this job have been fixed or filed (#1133). Final source-evidence schemas now describe their fields explicitly instead of exposing an untyped object. The final contract build and production screenshots are in progress.
Author
Owner

Implemented #1130 on job/moneyfid-1130. HEAD: b16bfd30582038cc79a086d1dd78cb0cb8f8b0c2.

READY FOR MERGE: no. Money correctness checks pass. The required web check stops at an unchanged sidebar performance exception; this is filed as #1133. The full server test has one SLOW-only Calendar latency failure. No test expectation was weakened. No push, deploy or merge into dev was done. Origin/dev was fetched and merged into this job branch once before final gates.

Built:

  • Actual imports retain closed Accounts, independent cleared/reconciled marks and exact separate Notes. Reconciled register rows are locked. Closed Accounts stay out of active lists and remain reachable through history and stable links.
  • Review requires a Cash/Card choice for Accounts whose old importer guessed Card from the label. Selection triggers a new source verification before confirmation. Missing choices cannot publish. Tracking remains source-authoritative.
  • Category Notes, template settings and goal definitions are retained as source text. Structured server counts render as plain-language sentences. Internal keys do not appear as labels.
  • Split children, transfer legs and original Tracking Categories retain source evidence without extra postings. Transfer registers use each leg's source Date, Notes and state. Marking one leg leaves the other leg and balances intact.
  • Separate Notes are exposed through the API and shown and edited in the register. Notes edits and per-Account mark changes have Undo. Deferred Undo retains the initiating Budget and session and refuses to overwrite a changed row.
  • Synthetic Actual property tests cover amounts, Dates, Accounts, Categories, blank Payees, closed state, all mark combinations and Unicode/multiline Notes through both adapters and Markdown replay. A separate raw-row integer oracle checks balances. A separate split/transfer numbers test and a production money/1 upgrade regression pass.
  • The existing Actual import benchmark supports --fidelity. No performance measurement was run: the final verification policy reserves measurements for performance issues on the locked perf VM.

UX gaps closed:

  • Closed history and importing another Budget are reachable when a Budget already exists.
  • Notes have a separate native field, retain line breaks, and support Undo. Phone import fields fit their container.
  • Reconciled and cleared are shown independently; an uncleared reconciled row does not look cleared.
  • Review uses native labelled Account type controls, handles prototype-like source IDs, requires choices and uses human verification names.
  • Navigation cancels active import work and releases prepared previews. Undo is fenced to its session.

UX gaps left / known limits:

  • Existing split/transfer/foreign-currency amount editing remains unsupported. Source templates and goal modes are retained as text and do not execute automatically; review states this.
  • Four existing Svelte warnings remain in three unrelated files. Full web check is blocked by #1133; no new Money performance exceptions or raised debt ceilings were added.
  • The full server suite failed only its unrelated Calendar p95 latency assertion. This is SLOW-only under the job policy. The focused server OpenAPI contract test passes.
  • This job used synthetic Actual data. The owner must still confirm the private source import on the combined build.

Decisions:

  • Money on production has no SQL migration table or registered SQL migrations. Its schema is money/1 Markdown. Use optional backwards-compatible metadata properties and the deployed-format upgrade test; do not add a fictitious numbered SQL migration.
  • Preserve existing explicit source type handling and Tracking. Ask only where Card was previously inferred from a name. Use provisional Cash solely to produce the review; confirmation requires a choice and another verified preview.
  • Keep reconciled rows read-only to retain the source reconciliation lock. Preserve cleared independently, including the reconciled-but-uncleared combination.
  • Store original split/transfer/Tracking fields as inert JSON evidence. Stable Money anchors replace source Account/Category IDs; original transaction identity stays in evidence. Arithmetic continues to use the existing postings.
  • Escape JSON angle brackets so imported Notes cannot open opaque Markdown comments and hide postings. Blank Payees keep the existing import label; Notes remain exact and separate.
  • Extend the existing shared form field CSS for Money Notes instead of adding another field recipe. This is a small shared styling addition; other fields keep their rules.

Evidence: 36 production screenshots, with macOS platform emulation, 390/820/1440 px and light/dark. Six views per combination: import form, Account review, verified review, Budget/sidebar, register and Notes editor. The focused browser run checks real API data, type choices, closed history, independent marks and Notes Undo. Claude/orchestrator must review visual quality.

For the merge round:

  • After #1133 is fixed, bun run --cwd apps/web check must pass the full web guard chain.
  • cd apps/web && bun run test --maxWorkers=2 runs the full browser unit suite once on the combined branch.
  • CALTERNAL_SERVER_BIN=<combined-build-binary> bun apps/web/e2e/money.mjs verifies the full Money flow and legacy import scenarios against the combined production build.
  • CALTERNAL_SERVER_BIN=<combined-build-binary> bun tests/adversarial/money_import_review.mjs verifies the bounded import ownership, cancellation, identity and token-race round. Run the standard full authorization/robustness matrices once for the combined routes.
  • cargo test -p calternal-server -- --test-threads=4 must cover the combined server. Record the Calendar latency result without weakening its assertion.
  • Release build, staging, full e2e and real Mac interoperability belong to the merge round under the final verification policy.
  • A later performance job can run bun bench/money-import-actual-462.mjs --fidelity --transactions=1000000 with the shared release server on the perf VM under flock /root/perf.lock, record load inside the lock, and compare the recorded metrics with docs/perf/baseline.json.

Gate output below is verbatim. Full logs are retained under worktree artifacts/.

Files:

  • apps/web/e2e/money-fidelity-1130.mjs
  • apps/web/e2e/money.mjs
  • apps/web/src/lib/components/money/MoneyImport.svelte
  • apps/web/src/lib/components/money/MoneySidebar.svelte
  • apps/web/src/lib/components/money/TransactionForm.svelte
  • apps/web/src/lib/money/api.ts
  • apps/web/src/lib/money/import-review.test.ts
  • apps/web/src/lib/money/import-review.ts
  • apps/web/src/lib/money/review-session-984.svelte.test.ts
  • apps/web/src/routes/money/+page.svelte
  • apps/web/src/routes/money/[budget]/accounts/[[account]]/+page.svelte
  • apps/web/src/routes/settings/parts/settings-forms.css
  • bench/money-import-actual-462.mjs
  • bench/money-import-actual-fixture.py
  • contracts/openapi.json
  • contracts/perf/exceptions.json
  • contracts/perf/registry.json
  • crates/calternal-money/Cargo.toml
  • crates/calternal-money/src/codec.rs
  • crates/calternal-money/src/edit.rs
  • crates/calternal-money/src/import.rs
  • crates/calternal-money/src/ledger.rs
  • crates/calternal-money/tests/import.rs
  • crates/plugins/money/src/import.rs
  • crates/plugins/money/src/import_fidelity_1130.rs
  • crates/plugins/money/src/import_review_tests.rs
  • crates/plugins/money/src/lib.rs
  • crates/plugins/money/src/routes.rs
  • crates/plugins/money/src/tests.rs
  • crates/plugins/money/src/views.rs
  • packages/api-client/src/generated.ts

cargo fmt --check: exit 0; no output. All Cargo commands used OPENSSL_NO_VENDOR=1, four build jobs, no incremental output and line-tables-only debug information. The web production build preceded the server build.

Core clippy:

    Finished `dev` profile [unoptimized + debuginfo] target(s) in 4m 04s

Core tests:

test result: ok. 16 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s
test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s
test result: ok. 13 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 10.74s
test result: ok. 12 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.77s
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s
test result: ok. 4 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 4.95s
test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.10s
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Money Plugin clippy:

    Finished `dev` profile [unoptimized + debuginfo] target(s) in 8.89s

Money Plugin tests (includes property tests and focused API tests):

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 85 filtered out; finished in 40.42s
test result: ok. 85 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 45.19s
test result: ok. 27 passed; 0 failed; 3 ignored; 0 measured; 0 filtered out; finished in 8.76s
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Separate break-the-numbers check:

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 85 filtered out; finished in 0.36s

Server clippy:

    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1m 27s

Server tests: SLOW-only failure:

search_reconcile_calendar_write_profile files=20000 writes=32 p50_ms=12447 p95_ms=13251 p95_budget_ms=10000
Calendar write p95 13.251828141s exceeded 10s
test result: FAILED. 254 passed; 1 failed; 10 ignored; 0 measured; 0 filtered out; finished in 237.79s

Focused server contract test:

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 264 filtered out; finished in 0.14s

Focused Vitest: import-review, review-984, review-session-984:

 Test Files  3 passed (3)
      Tests  34 passed (34)

Svelte check (direct, because the full web guard stops first):

svelte-check found 0 errors and 4 warnings in 3 files

Required bun run check: exit 2:

perf-lint: INVALID: ('render.bound', 'apps/web/src/lib/components/app-sidebar.svelte', 'apps/web/src/lib/components/app-sidebar.svelte#each:10a1aaf3137a2426:1'): unused or changed exception

Focused production browser: exit 0:

PASS Actual review choices, closed history, per-row notes/state and macOS screenshots at 390/820/1440 in light/dark

Production web build: exit 0:

Compressed 879 static variants; saved 20855551 bytes.

Generated contract check: exit 0; current binary export and regenerated client produce no git diff. Separate storage, glass, type, focus and motion token checks: exit 0.

Cleanup completed; tracked worktree is clean. Web build output and worktree test scratch were removed. Cargo cleanup:

     Removed 18695 files, 13.6GiB total
Implemented #1130 on `job/moneyfid-1130`. HEAD: `b16bfd30582038cc79a086d1dd78cb0cb8f8b0c2`. READY FOR MERGE: **no**. Money correctness checks pass. The required web check stops at an unchanged sidebar performance exception; this is filed as [#1133](https://git.kayg.org/kayg/calternal/issues/1133). The full server test has one SLOW-only Calendar latency failure. No test expectation was weakened. No push, deploy or merge into dev was done. Origin/dev was fetched and merged into this job branch once before final gates. Built: - Actual imports retain closed Accounts, independent cleared/reconciled marks and exact separate Notes. Reconciled register rows are locked. Closed Accounts stay out of active lists and remain reachable through history and stable links. - Review requires a Cash/Card choice for Accounts whose old importer guessed Card from the label. Selection triggers a new source verification before confirmation. Missing choices cannot publish. Tracking remains source-authoritative. - Category Notes, template settings and goal definitions are retained as source text. Structured server counts render as plain-language sentences. Internal keys do not appear as labels. - Split children, transfer legs and original Tracking Categories retain source evidence without extra postings. Transfer registers use each leg's source Date, Notes and state. Marking one leg leaves the other leg and balances intact. - Separate Notes are exposed through the API and shown and edited in the register. Notes edits and per-Account mark changes have Undo. Deferred Undo retains the initiating Budget and session and refuses to overwrite a changed row. - Synthetic Actual property tests cover amounts, Dates, Accounts, Categories, blank Payees, closed state, all mark combinations and Unicode/multiline Notes through both adapters and Markdown replay. A separate raw-row integer oracle checks balances. A separate split/transfer numbers test and a production money/1 upgrade regression pass. - The existing Actual import benchmark supports `--fidelity`. No performance measurement was run: the final verification policy reserves measurements for performance issues on the locked perf VM. UX gaps closed: - Closed history and importing another Budget are reachable when a Budget already exists. - Notes have a separate native field, retain line breaks, and support Undo. Phone import fields fit their container. - Reconciled and cleared are shown independently; an uncleared reconciled row does not look cleared. - Review uses native labelled Account type controls, handles prototype-like source IDs, requires choices and uses human verification names. - Navigation cancels active import work and releases prepared previews. Undo is fenced to its session. UX gaps left / known limits: - Existing split/transfer/foreign-currency amount editing remains unsupported. Source templates and goal modes are retained as text and do not execute automatically; review states this. - Four existing Svelte warnings remain in three unrelated files. Full web check is blocked by #1133; no new Money performance exceptions or raised debt ceilings were added. - The full server suite failed only its unrelated Calendar p95 latency assertion. This is SLOW-only under the job policy. The focused server OpenAPI contract test passes. - This job used synthetic Actual data. The owner must still confirm the private source import on the combined build. Decisions: - Money on production has no SQL migration table or registered SQL migrations. Its schema is money/1 Markdown. Use optional backwards-compatible metadata properties and the deployed-format upgrade test; do not add a fictitious numbered SQL migration. - Preserve existing explicit source type handling and Tracking. Ask only where Card was previously inferred from a name. Use provisional Cash solely to produce the review; confirmation requires a choice and another verified preview. - Keep reconciled rows read-only to retain the source reconciliation lock. Preserve cleared independently, including the reconciled-but-uncleared combination. - Store original split/transfer/Tracking fields as inert JSON evidence. Stable Money anchors replace source Account/Category IDs; original transaction identity stays in evidence. Arithmetic continues to use the existing postings. - Escape JSON angle brackets so imported Notes cannot open opaque Markdown comments and hide postings. Blank Payees keep the existing import label; Notes remain exact and separate. - Extend the existing shared form field CSS for Money Notes instead of adding another field recipe. This is a small shared styling addition; other fields keep their rules. Evidence: [36 production screenshots](https://git.kayg.org/attachments/681f70c0-216c-4789-921e-04476c0f55fb), with macOS platform emulation, 390/820/1440 px and light/dark. Six views per combination: import form, Account review, verified review, Budget/sidebar, register and Notes editor. The focused browser run checks real API data, type choices, closed history, independent marks and Notes Undo. Claude/orchestrator must review visual quality. For the merge round: - After #1133 is fixed, `bun run --cwd apps/web check` must pass the full web guard chain. - `cd apps/web && bun run test --maxWorkers=2` runs the full browser unit suite once on the combined branch. - `CALTERNAL_SERVER_BIN=<combined-build-binary> bun apps/web/e2e/money.mjs` verifies the full Money flow and legacy import scenarios against the combined production build. - `CALTERNAL_SERVER_BIN=<combined-build-binary> bun tests/adversarial/money_import_review.mjs` verifies the bounded import ownership, cancellation, identity and token-race round. Run the standard full authorization/robustness matrices once for the combined routes. - `cargo test -p calternal-server -- --test-threads=4` must cover the combined server. Record the Calendar latency result without weakening its assertion. - Release build, staging, full e2e and real Mac interoperability belong to the merge round under the final verification policy. - A later performance job can run `bun bench/money-import-actual-462.mjs --fidelity --transactions=1000000` with the shared release server on the perf VM under `flock /root/perf.lock`, record load inside the lock, and compare the recorded metrics with `docs/perf/baseline.json`. Gate output below is verbatim. Full logs are retained under worktree `artifacts/`. Files: - `apps/web/e2e/money-fidelity-1130.mjs` - `apps/web/e2e/money.mjs` - `apps/web/src/lib/components/money/MoneyImport.svelte` - `apps/web/src/lib/components/money/MoneySidebar.svelte` - `apps/web/src/lib/components/money/TransactionForm.svelte` - `apps/web/src/lib/money/api.ts` - `apps/web/src/lib/money/import-review.test.ts` - `apps/web/src/lib/money/import-review.ts` - `apps/web/src/lib/money/review-session-984.svelte.test.ts` - `apps/web/src/routes/money/+page.svelte` - `apps/web/src/routes/money/[budget]/accounts/[[account]]/+page.svelte` - `apps/web/src/routes/settings/parts/settings-forms.css` - `bench/money-import-actual-462.mjs` - `bench/money-import-actual-fixture.py` - `contracts/openapi.json` - `contracts/perf/exceptions.json` - `contracts/perf/registry.json` - `crates/calternal-money/Cargo.toml` - `crates/calternal-money/src/codec.rs` - `crates/calternal-money/src/edit.rs` - `crates/calternal-money/src/import.rs` - `crates/calternal-money/src/ledger.rs` - `crates/calternal-money/tests/import.rs` - `crates/plugins/money/src/import.rs` - `crates/plugins/money/src/import_fidelity_1130.rs` - `crates/plugins/money/src/import_review_tests.rs` - `crates/plugins/money/src/lib.rs` - `crates/plugins/money/src/routes.rs` - `crates/plugins/money/src/tests.rs` - `crates/plugins/money/src/views.rs` - `packages/api-client/src/generated.ts` `cargo fmt --check`: exit 0; no output. All Cargo commands used OPENSSL_NO_VENDOR=1, four build jobs, no incremental output and line-tables-only debug information. The web production build preceded the server build. Core clippy: ```text Finished `dev` profile [unoptimized + debuginfo] target(s) in 4m 04s ``` Core tests: ```text test result: ok. 16 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s test result: ok. 13 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 10.74s test result: ok. 12 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.77s test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s test result: ok. 4 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 4.95s test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.10s test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s ``` Money Plugin clippy: ```text Finished `dev` profile [unoptimized + debuginfo] target(s) in 8.89s ``` Money Plugin tests (includes property tests and focused API tests): ```text test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 85 filtered out; finished in 40.42s test result: ok. 85 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 45.19s test result: ok. 27 passed; 0 failed; 3 ignored; 0 measured; 0 filtered out; finished in 8.76s test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s ``` Separate break-the-numbers check: ```text test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 85 filtered out; finished in 0.36s ``` Server clippy: ```text Finished `dev` profile [unoptimized + debuginfo] target(s) in 1m 27s ``` Server tests: SLOW-only failure: ```text search_reconcile_calendar_write_profile files=20000 writes=32 p50_ms=12447 p95_ms=13251 p95_budget_ms=10000 Calendar write p95 13.251828141s exceeded 10s test result: FAILED. 254 passed; 1 failed; 10 ignored; 0 measured; 0 filtered out; finished in 237.79s ``` Focused server contract test: ```text test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 264 filtered out; finished in 0.14s ``` Focused Vitest: import-review, review-984, review-session-984: ```text Test Files 3 passed (3) Tests 34 passed (34) ``` Svelte check (direct, because the full web guard stops first): ```text svelte-check found 0 errors and 4 warnings in 3 files ``` Required bun run check: exit 2: ```text perf-lint: INVALID: ('render.bound', 'apps/web/src/lib/components/app-sidebar.svelte', 'apps/web/src/lib/components/app-sidebar.svelte#each:10a1aaf3137a2426:1'): unused or changed exception ``` Focused production browser: exit 0: ```text PASS Actual review choices, closed history, per-row notes/state and macOS screenshots at 390/820/1440 in light/dark ``` Production web build: exit 0: ```text Compressed 879 static variants; saved 20855551 bytes. ``` Generated contract check: exit 0; current binary export and regenerated client produce no git diff. Separate storage, glass, type, focus and motion token checks: exit 0. Cleanup completed; tracked worktree is clean. Web build output and worktree test scratch were removed. Cargo cleanup: ```text Removed 18695 files, 13.6GiB total ```
Author
Owner

Independent moneyrev-1130 review started on job/moneyfid-1130 at b16bfd3058 (base origin/dev). Reading implementation and adding independent arithmetic and mutation regressions before reading author reports.

Independent moneyrev-1130 review started on job/moneyfid-1130 at b16bfd30582038cc79a086d1dd78cb0cb8f8b0c2 (base origin/dev). Reading implementation and adding independent arithmetic and mutation regressions before reading author reports.
Author
Owner

Independent review finding: six new tests pass, including 72 generated cases, raw closed/reconciled Account sums through both adapters, varying split/closed transfer postings, arbitrary Unicode metadata, a 90 KB note over local HTTP, reconciliation edit/Undo rejection, same-title re-import byte equality and partial-preparation cancellation cleanup. No arithmetic defect found so far. Origin/dev merged once at 8c941b9fa before final crate gates. New tests and evidence are in review-1130.md; gates running.

Independent review finding: six new tests pass, including 72 generated cases, raw closed/reconciled Account sums through both adapters, varying split/closed transfer postings, arbitrary Unicode metadata, a 90 KB note over local HTTP, reconciliation edit/Undo rejection, same-title re-import byte equality and partial-preparation cancellation cleanup. No arithmetic defect found so far. Origin/dev merged once at 8c941b9fa before final crate gates. New tests and evidence are in review-1130.md; gates running.
Author
Owner

Independent finding with failing regression: review1130_long_imported_note_allows_amount_edit_without_loss imports an 80 KB Unicode note on an unreconciled row marked editable, then changes its amount while resubmitting the unchanged note (the normal web-form request). Expected 200; received 400: notes are too long. Fixing the request validator to preserve byte-identical existing long notes only; new or changed oversized notes keep the existing 64 KiB limit. Evidence appended to review-1130.md.

Independent finding with failing regression: review1130_long_imported_note_allows_amount_edit_without_loss imports an 80 KB Unicode note on an unreconciled row marked editable, then changes its amount while resubmitting the unchanged note (the normal web-form request). Expected 200; received 400: notes are too long. Fixing the request validator to preserve byte-identical existing long notes only; new or changed oversized notes keep the existing 64 KiB limit. Evidence appended to review-1130.md.
Author
Owner

Independent review complete. SAFE TO MERGE: yes for #1130 number correctness.

Head SHA: 577b1b3d10fb8237144242cdc448475e2113c4c2.

Independent Money number review — #1130

Reviewed author head: b16bfd30582038cc79a086d1dd78cb0cb8f8b0c2.
Review branch: job/moneyfid-1130. Base: origin/dev.

Read the contracts, DESIGN §48, issue body and code before author reports.
No author result reports were read.

Initial findings

  • The importer stores source evidence separately from posting details.
    Expected Account balances in this review use raw integer sums.
  • DESIGN §48 creates a new Budget on import. A repeated title must reject
    confirmation and keep all existing bytes. It does not update a Budget.
  • Existing money/1 files have required frontmatter. Upgrade tests omit the
    new closed, notes, cleared, reconciled and source rows fields.
  • Money Undo uses the normal update and cleared routes. The review sends
    those same calls against locked rows.

Tests in progress

crates/plugins/money/src/review_1130.rs adds generated closed/reconciled
balance checks, arbitrary Unicode metadata checks, a real local HTTP lock,
re-import and cancellation round, and a legacy Home read check.

No confirmed implementation defect yet. Record failed tests here before fixes.

First focused run: 3 passed; the notes property fixture omitted the required
Account currency field. The empty-string shrink failed before metadata was
read. Added currency:: EUR to the new fixture. This was a review test error,
not an implementation finding. No existing expectations changed.

Focused results

All six independent review tests pass. The tests include 72 generated cases:
24 closed/reconciled balance cases through both adapters, 24 arbitrary Unicode
metadata cases and 24 varying two-child split and transfer cases with both
Accounts closed. The HTTP test retains a 90 KB Unicode note exactly. It rejects
amount, date, Account, delete and cleared Undo calls for locked rows. Re-import
with the same title returns 409 and keeps the Budget files and register equal.
Cancellation after the first staged file returns no plan and removes SQLite
scratch data. A cancelled HTTP preview cannot be confirmed and creates no Budget.

Verbatim focused result:

test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 86 filtered out; finished in 4.84s

Fetched and merged origin/dev once at 8c941b9fa. The only merge conflict
was the Money preview performance-debt hash. Retained the branch hash because
its Money preview handler and contract are retained; all other origin changes
remain in the merge. No number defect found in the focused round.

Finding 1 — unchanged long imported notes block ordinary edits

Failing test: review1130_long_imported_note_allows_amount_edit_without_loss.
An Actual import keeps a synthetic 80 KB Unicode note. The register marks its
unreconciled row editable. A normal amount edit submits the unchanged note,
as the web form does. The server rejects it before any write:

assertion `left == right` failed: {"error":{"code":"bad_request","message":"notes are too long"}}
  left: 400
 right: 200
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 92 filtered out; finished in 1.54s

Clear defect: the accepted import cannot use its advertised edit action. Keep
the 64 KiB bound for new text. Permit only byte-identical existing notes above
that bound during an edit. A larger replacement still must return 400.

During the new move check, the review test first used on_budget_balance,
which is not an AccountList field. The correct field is on_budget_total.
Fixed that new assertion. The existing author test also compares the absent
field on both sides, so it compares JSON nulls. Kept that existing assertion
as instructed. The new test checks the actual numeric field after a move.

Fix result and UX gaps closed

The unchanged-note exception passes. The new regression checks same-month
amount edits, a move to another month, exact retained Notes and the real
on-budget total. New and changed oversized notes still return 400. Reconciled
rows still cannot use this edit path. The advertised edit action now works for
the tested long imported note without changing its contents.

Final plugin gates (verbatim output excerpts; full logs in target/):

cargo fmt --check: no output; exit 0.

cargo clippy -p calternal-plugin-money --all-targets -- -D warnings:

    Checking calternal-plugin-money v0.0.1 (/home/kayg/Developer/calternal-wt/moneyfid-1130/crates/plugins/money)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 8.65s

cargo test -p calternal-plugin-money -- --test-threads=4:

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 92 filtered out; finished in 73.16s
test result: ok. 92 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 78.12s
test result: ok. 27 passed; 0 failed; 3 ignored; 0 measured; 0 filtered out; finished in 13.35s
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.04s

The earlier full run failed only at the new long-note regression. Repeated
the plugin gates once after the fix. No existing expectations were changed.

Pure Money gates

cargo clippy -p calternal-money --all-targets -- -D warnings:

    Checking calternal-money v0.0.1 (/home/kayg/Developer/calternal-wt/moneyfid-1130/crates/calternal-money)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 56.03s

cargo test -p calternal-money -- --test-threads=4:

test result: ok. 16 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s
test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.06s
test result: ok. 13 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 15.68s
test result: ok. 12 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.89s
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s
test result: ok. 4 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 3.72s
test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.60s
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Verdict, decisions and limits

SAFE TO MERGE: yes for the #1130 number-correctness review after the fix.
Fix commit: f67d3279fa9709489d037843d54555100872ddb1.
Original merge base: 570e084c8691a76b9bec904f47151f4ff5701298.
Merged origin/dev: f2a34dd6e9372cdd2c77c7b41c32e2c9a7a096b2.

Built seven independent tests, including 72 generated cases, and one bounded
real local HTTP round. Fixed one clear request-validation defect in a separate
commit. No new arithmetic rule, dependency, schema or migration was added.

Files changed by review work:

  • crates/plugins/money/src/review_1130.rs: independent tests.
  • crates/plugins/money/src/tests.rs: test module registration.
  • crates/plugins/money/src/routes.rs: unchanged long-note validation and docs.
  • review-1130.md: findings and gate evidence.
    The required origin merge also retained its upstream files and resolved the
    single generated performance-debt conflict.

Decisions: DESIGN does not specify how the new-text limit applies to imported
Notes. Keep the limit for new or changed text. Permit only byte-identical
existing Notes during edits. Same-title re-import returns 409 rather than
updating a Budget, as DESIGN §48 requires. Issue #1130 overrides the older
DESIGN statement that reconciliation locks are dropped.

Known gaps / UX gaps left: no remaining number defect found in the reviewed
paths. Five existing diagnostic tests stay ignored. The author's absent-field
comparison remains unchanged, as required. This review tests Money routes and
Money Undo calls; it does not establish lock enforcement through generic Files
replacement or Version restore routes. The local HTTP server uses an injected
data-scope User; it does not test full-server authentication. No UI files were
changed or screenshots taken. No performance run was required for this review
under the current verification policy.

For the merge round: run the normal combined server integration gates:
cargo clippy -p calternal-server --all-targets -- -D warnings and
cargo test -p calternal-server -- --test-threads=4. They must confirm the
combined server builds and its cross-plugin contracts pass. All number-review
scenarios requested for this job ran here; none are deferred.

Re-read the doc comments in all touched Rust files before this report.

Independent review complete. SAFE TO MERGE: yes for #1130 number correctness. Head SHA: `577b1b3d10fb8237144242cdc448475e2113c4c2`. # Independent Money number review — #1130 Reviewed author head: `b16bfd30582038cc79a086d1dd78cb0cb8f8b0c2`. Review branch: `job/moneyfid-1130`. Base: `origin/dev`. Read the contracts, DESIGN §48, issue body and code before author reports. No author result reports were read. ## Initial findings - The importer stores source evidence separately from posting details. Expected Account balances in this review use raw integer sums. - DESIGN §48 creates a new Budget on import. A repeated title must reject confirmation and keep all existing bytes. It does not update a Budget. - Existing `money/1` files have required frontmatter. Upgrade tests omit the new closed, notes, cleared, reconciled and source rows fields. - Money Undo uses the normal update and cleared routes. The review sends those same calls against locked rows. ## Tests in progress `crates/plugins/money/src/review_1130.rs` adds generated closed/reconciled balance checks, arbitrary Unicode metadata checks, a real local HTTP lock, re-import and cancellation round, and a legacy Home read check. No confirmed implementation defect yet. Record failed tests here before fixes. First focused run: 3 passed; the notes property fixture omitted the required Account currency field. The empty-string shrink failed before metadata was read. Added `currency:: EUR` to the new fixture. This was a review test error, not an implementation finding. No existing expectations changed. ## Focused results All six independent review tests pass. The tests include 72 generated cases: 24 closed/reconciled balance cases through both adapters, 24 arbitrary Unicode metadata cases and 24 varying two-child split and transfer cases with both Accounts closed. The HTTP test retains a 90 KB Unicode note exactly. It rejects amount, date, Account, delete and cleared Undo calls for locked rows. Re-import with the same title returns 409 and keeps the Budget files and register equal. Cancellation after the first staged file returns no plan and removes SQLite scratch data. A cancelled HTTP preview cannot be confirmed and creates no Budget. Verbatim focused result: ``` test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 86 filtered out; finished in 4.84s ``` Fetched and merged `origin/dev` once at `8c941b9fa`. The only merge conflict was the Money preview performance-debt hash. Retained the branch hash because its Money preview handler and contract are retained; all other origin changes remain in the merge. No number defect found in the focused round. ## Finding 1 — unchanged long imported notes block ordinary edits Failing test: `review1130_long_imported_note_allows_amount_edit_without_loss`. An Actual import keeps a synthetic 80 KB Unicode note. The register marks its unreconciled row editable. A normal amount edit submits the unchanged note, as the web form does. The server rejects it before any write: ``` assertion `left == right` failed: {"error":{"code":"bad_request","message":"notes are too long"}} left: 400 right: 200 test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 92 filtered out; finished in 1.54s ``` Clear defect: the accepted import cannot use its advertised edit action. Keep the 64 KiB bound for new text. Permit only byte-identical existing notes above that bound during an edit. A larger replacement still must return 400. During the new move check, the review test first used `on_budget_balance`, which is not an AccountList field. The correct field is `on_budget_total`. Fixed that new assertion. The existing author test also compares the absent field on both sides, so it compares JSON nulls. Kept that existing assertion as instructed. The new test checks the actual numeric field after a move. ## Fix result and UX gaps closed The unchanged-note exception passes. The new regression checks same-month amount edits, a move to another month, exact retained Notes and the real on-budget total. New and changed oversized notes still return 400. Reconciled rows still cannot use this edit path. The advertised edit action now works for the tested long imported note without changing its contents. Final plugin gates (verbatim output excerpts; full logs in `target/`): `cargo fmt --check`: no output; exit 0. `cargo clippy -p calternal-plugin-money --all-targets -- -D warnings`: ``` Checking calternal-plugin-money v0.0.1 (/home/kayg/Developer/calternal-wt/moneyfid-1130/crates/plugins/money) Finished `dev` profile [unoptimized + debuginfo] target(s) in 8.65s ``` `cargo test -p calternal-plugin-money -- --test-threads=4`: ``` test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 92 filtered out; finished in 73.16s test result: ok. 92 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 78.12s test result: ok. 27 passed; 0 failed; 3 ignored; 0 measured; 0 filtered out; finished in 13.35s test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.04s ``` The earlier full run failed only at the new long-note regression. Repeated the plugin gates once after the fix. No existing expectations were changed. ## Pure Money gates `cargo clippy -p calternal-money --all-targets -- -D warnings`: ``` Checking calternal-money v0.0.1 (/home/kayg/Developer/calternal-wt/moneyfid-1130/crates/calternal-money) Finished `dev` profile [unoptimized + debuginfo] target(s) in 56.03s ``` `cargo test -p calternal-money -- --test-threads=4`: ``` test result: ok. 16 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.06s test result: ok. 13 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 15.68s test result: ok. 12 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.89s test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s test result: ok. 4 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 3.72s test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.60s test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s ``` ## Verdict, decisions and limits **SAFE TO MERGE: yes** for the #1130 number-correctness review after the fix. Fix commit: `f67d3279fa9709489d037843d54555100872ddb1`. Original merge base: `570e084c8691a76b9bec904f47151f4ff5701298`. Merged origin/dev: `f2a34dd6e9372cdd2c77c7b41c32e2c9a7a096b2`. Built seven independent tests, including 72 generated cases, and one bounded real local HTTP round. Fixed one clear request-validation defect in a separate commit. No new arithmetic rule, dependency, schema or migration was added. Files changed by review work: - `crates/plugins/money/src/review_1130.rs`: independent tests. - `crates/plugins/money/src/tests.rs`: test module registration. - `crates/plugins/money/src/routes.rs`: unchanged long-note validation and docs. - `review-1130.md`: findings and gate evidence. The required origin merge also retained its upstream files and resolved the single generated performance-debt conflict. Decisions: DESIGN does not specify how the new-text limit applies to imported Notes. Keep the limit for new or changed text. Permit only byte-identical existing Notes during edits. Same-title re-import returns 409 rather than updating a Budget, as DESIGN §48 requires. Issue #1130 overrides the older DESIGN statement that reconciliation locks are dropped. Known gaps / UX gaps left: no remaining number defect found in the reviewed paths. Five existing diagnostic tests stay ignored. The author's absent-field comparison remains unchanged, as required. This review tests Money routes and Money Undo calls; it does not establish lock enforcement through generic Files replacement or Version restore routes. The local HTTP server uses an injected data-scope User; it does not test full-server authentication. No UI files were changed or screenshots taken. No performance run was required for this review under the current verification policy. For the merge round: run the normal combined server integration gates: `cargo clippy -p calternal-server --all-targets -- -D warnings` and `cargo test -p calternal-server -- --test-threads=4`. They must confirm the combined server builds and its cross-plugin contracts pass. All number-review scenarios requested for this job ran here; none are deferred. Re-read the doc comments in all touched Rust files before this report.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal#1130
No description provided.