One pass over the LOW-tier review items plus two folded issues:
- Delete validate_journal_entry (dead since the Fava migration; it
validated the pre-string-amount model) with its exports, unused
crud imports, and tests. Beancount validates entries now.
- Migration m006: UNIQUE index on user_roles(user_id, role_id) after
deduping; assign_user_role inserts with ON CONFLICT DO NOTHING and
returns the existing assignment — closes the auto-assign
check-then-act race on concurrent logins.
- Extract _get_username_from_user_id (110 lines in views_api, fresh
LNbits Database per call inside per-row hot paths) into
user_lookup.py with one shared core-DB handle, a 60s TTL cache and
a batch get_usernames API (review #18).
- Receivable-entry responses report CLEARED, matching the flag the
formatter actually writes; PENDING misled the UI (libra-#35).
- Replace the remaining print() calls in tasks.py with logger.
- get_all_accounts derives valid roots from
account_utils.ACCOUNT_TYPE_ROOTS instead of a hardcoded tuple, and
the no-op per-test rate-limit reset is gone (libra-#54).
- Delete migrations_old.py.bak, MIGRATION_SQUASH_SUMMARY.md,
docs/PHASE*_COMPLETE.md and the rendered .html; .gitignore data/
(it holds the runtime .lnbits_auth_key secret).
- Track docs/CODE-REVIEW-2026-06.md with finding statuses updated for
the PR #55-#59 + chore/hygiene series.
- CLAUDE.md notes LNbits pins Pydantic v1: keep .dict(), don't
"modernize" to .model_dump().
Note: format_payment_entry's is_payable docstring (flagged in review
follow-up) turned out to be consistent with the body — no change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Settlement correctness cluster from CODE-REVIEW-2026-06 (#2, #8, #13)
plus libra-#38:
- format_net_settlement_entry now enforces the same inline balance
constraint as the fiat formatter (payment = receivable - payable
+ credit) and grows an optional credit leg. An unbalanced
settlement raises instead of reaching the ledger.
- on_invoice_paid settles only what the payment covers: a partial
payment clears that much receivable; excess (or a payment with
nothing owed) becomes user credit. Previously the full prior
balance was cleared against a smaller payment, shipping unbalanced
postings. Settlement links are attached only when the payment
clears the full open balance, and only for same-currency entries.
- get_unsettled_entries_bql returns each entry's real posting
currency (was hardcoded "EUR") and exact Decimal amount strings
(was float). /receivables/settle nets only entries denominated in
the settlement currency.
- fiat_rate/btc_rate metadata computed via Decimal (new
fiat_rate_metadata helper) instead of float division — cost-basis
records no longer carry float drift.
- format_posting_at_average_cost omits the cost braces when
cost_currency is unset ("SATS {}" is invalid Beancount).
- Underpay error payload serializes amounts as exact Decimal strings.
- validate_metadata catches decimal.InvalidOperation so bad
fiat_amount input becomes ValidationError (libra-#38); flipped the
tracking xfail.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
_open_directive_exists hardcoded '^YYYY-MM-DD open ' (dash-only, 2-digit,
single-space), but Beancount's DATE token (parser/lexer.l) is
(17|18|19|20)[0-9]{2}[-/][0-9]+[-/][0-9]+ and inter-token whitespace is any
[ \t\r] run. So a validly-formatted existing Open written as '2024/3/5 open X'
or '2020-01-01 open X' escaped detection → duplicate Open appended →
bean-check rejects the file. Anchor on Beancount's actual date pattern and
[ \t]+ separators. Adds parametrized coverage for slash/single-digit/multi-
space/tab variants.
Found in a coherence pass over the Beancount source.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The existence check matched 'open <name>' anywhere in the chart source,
so a prior account's description metadata or a comment mentioning the
name produced a false 409, while a real directive with an inline comment
and no space ('open X;legacy') was missed → a duplicate Open was appended
and bean-check then rejected the file, breaking every later /api/source
write. Extract the check into a pure _open_directive_exists() anchored to
'^YYYY-MM-DD open <name>' with an account-boundary negative-lookahead, and
unit-test both failure directions plus prefix/child non-matches.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Approving a pending entry created with a reference (e.g. invoice
"42-144") 404'd with "Pending entry unknown not found": the list
endpoints recovered the entry id by parsing links for a libra- prefix,
but reference-bearing entries displace that link with the fused
"{reference}-{entry_id}" form, so the id surfaced as the literal
"unknown" and the approve call round-tripped it.
Make the entry-id transaction metadata the single canonical identity:
- _extract_entry_id() resolves metadata-first (libra- link parsing kept
only for pre-dfdcc44 ledger history); used by /entries/user,
/entries/pending, approve, and reject.
- Creation endpoints no longer fuse the reference with the entry id —
the user reference becomes its own sanitized link and round-trips
verbatim in API responses. Typed exp-/rcv-/inc- links stay as the
settlement-tracking handles.
- format_revenue_entry now writes entry-id metadata like its siblings
and sanitizes its reference link (was appended raw); generic
POST /entries sanitizes its reference link too.
- User-journal reference extraction skips all system link prefixes
(typed links used to leak into the reference field).
Contract documented in CLAUDE.md (Data Integrity → Entry Identity &
Links), pinned by tests/test_entry_identity_api.py and formatter
contract tests in test_unit.py.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
113 passing tests + 3 skipped + 8 xfailed across 10 files, covering user
expense and income flow, admin receivable/revenue, settings + auth gates,
void/reject, manual payment requests, balance display, Lightning auth
paths, reconciliation API, and pure-function units. Runs against a real
Fava subprocess and full LNbits app via asgi_lifespan; the harness
captures the auth-flow / settings / env-var disciplines surfaced during
build-out (see tests/README.md and tests/conftest.py docstring).
Eight xfailed/skipped tests carry full implementations gated behind issues
#38, #39, #40 — they flip back on automatically when those land.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
