From fe59f51b19796ba6a18bf8787308650a0b209862 Mon Sep 17 00:00:00 2001 From: martin Date: Sat, 30 May 2026 00:51:54 +0200 Subject: [PATCH] fix: route query serializer through in-tree to_string (#144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `rustfava.rustledger.query._directive_to_source` was a parallel re-implementation of source emission that only handled a subset of each directive's fields. When `RLConnection.execute()` had no original source to hand to rledger, it called the parallel serializer, silently dropping tags, links, directive and posting metadata, per-posting flags, cost basis `{price, date}`, per-posting prices `@`/`@@`, balance tolerance `~`, and the booking method on `open`. Net effect: BQL queries returned the wrong rows or wrong numbers with no error path. From the bug report (#144): - `WHERE 'tag' IN tags` matched 0 rows because tags were stripped. - `WHERE 'link' IN links` matched 0 rows for the same reason. - `WHERE META('key') = 'x'` matched 0 rows because metadata was stripped. - `SELECT cost(position)` returned `None` because `{price, date}` was dropped, breaking capital-gains math. - `SELECT sum(convert(position, 'USD'))` for a cross-currency transaction returned an unbalanced bag because the `@@` total-price annotation was dropped. `rustfava.beans.str.to_string` is the singledispatch formatter already used elsewhere in the codebase and already handles all of the above except balance tolerance. This change: - Rewrites `_entries_to_source` as a thin wrapper over `to_string`. Custom directives whose type begins with `fava` are still skipped because rledger cannot parse them — that was the only behavior in the old serializer worth preserving. - Deletes `_directive_to_source` (~90 lines). - Adds balance-tolerance support to `_format_balance` in `beans/str.py`. The tolerance is inserted post-alignment so the currency column math stays correct. - Adds regression tests covering tags, links, directive metadata, posting metadata, posting flags, cost basis, prices, booking method, and balance tolerance for the round-trip path. The two suggested fixes in the issue text both involved delegating to `beancount.parser.printer.EntryPrinter`. That path was not taken because `beancount` is in `[project.optional-dependencies] beancount-compat` and isn't always present at runtime — reusing the in-tree formatter avoids making it a hard dependency and matches the maintainer's existing pattern. Limitations not addressed here: - `RLPosting.price` carries only an `RLAmount`, not a per-unit-vs-total flag, so the regenerated source emits `@` even when the original was `@@`. This is lossy at parse time, not serialize time, and is mathematically equivalent for sum-after-conversion queries; a separate PR against the rustledger JSON contract would be needed to preserve the literal `@@` form. --- src/rustfava/beans/str.py | 10 ++- src/rustfava/rustledger/query.py | 110 ++++--------------------- tests/test_beans_str.py | 17 ++++ tests/test_rustledger_query.py | 134 +++++++++++++++++++++++++++++++ 4 files changed, 176 insertions(+), 95 deletions(-) create mode 100644 tests/test_rustledger_query.py diff --git a/src/rustfava/beans/str.py b/src/rustfava/beans/str.py index 83265f5f7..18c85c871 100644 --- a/src/rustfava/beans/str.py +++ b/src/rustfava/beans/str.py @@ -260,13 +260,21 @@ def _format_balance( indent: int = 2, ) -> str: """Format a balance entry.""" + tolerance = getattr(entry, "tolerance", None) amount_str = amount_to_string(entry.amount) line = f"{entry.date.isoformat()} balance {entry.account} {amount_str}" lines = [line] meta = dict(entry.meta) if entry.meta else {} lines.extend(_format_meta(meta, indent)) result = "\n".join(lines) - return align(result, currency_column) + aligned = align(result, currency_column) + if tolerance is not None: + # Beancount syntax is ` ~ `. Insert + # the tolerance after alignment so the column math stays correct. + needle = f"{entry.amount.number} {entry.amount.currency}" + replacement = f"{entry.amount.number} ~ {tolerance} {entry.amount.currency}" + aligned = aligned.replace(needle, replacement, 1) + return aligned @to_string.register(Open) diff --git a/src/rustfava/rustledger/query.py b/src/rustfava/rustledger/query.py index 87621c445..763799381 100644 --- a/src/rustfava/rustledger/query.py +++ b/src/rustfava/rustledger/query.py @@ -6,6 +6,7 @@ from decimal import Decimal from typing import TYPE_CHECKING +from rustfava.beans.str import to_string from rustfava.rustledger.engine import RustledgerEngine from rustfava.rustledger.types import RLAmount @@ -233,99 +234,20 @@ def execute(self, query_string: str) -> RLCursor: def _entries_to_source(entries: Sequence[Directive]) -> str: """Convert entries back to beancount source for querying. - This is a fallback when the original source isn't available. + Delegates to ``rustfava.beans.str.to_string``, which is the same + formatter rustfava uses elsewhere and handles tags, links, metadata, + posting flags, cost basis, prices, booking methods, and balance + tolerance. Custom directives whose type begins with ``fava`` are + skipped because rledger cannot parse them. """ - lines = [] + parts: list[str] = [] for entry in entries: - line = _directive_to_source(entry) - if line: - lines.append(line) - return "\n".join(lines) - - -def _directive_to_source(directive: Directive) -> str: - """Convert a directive to beancount source line.""" - date = directive.date.isoformat() - dtype = type(directive).__name__.lower().removeprefix("rl") - - if dtype == "open": - currencies = " ".join(getattr(directive, "currencies", [])) - account = getattr(directive, "account", "") - return f'{date} open {account} {currencies}'.strip() - - if dtype == "close": - account = getattr(directive, "account", "") - return f'{date} close {account}' - - if dtype == "balance": - amt = getattr(directive, "amount", None) - account = getattr(directive, "account", "") - if amt: - return f'{date} balance {account} {amt.number} {amt.currency}' - return f'{date} balance {account}' - - if dtype == "transaction": - flag = getattr(directive, "flag", "*") - payee = getattr(directive, "payee", None) - narration = getattr(directive, "narration", "") - - if payee: - header = f'{date} {flag} "{payee}" "{narration}"' - else: - header = f'{date} {flag} "{narration}"' - - posting_lines = [] - for p in getattr(directive, "postings", []): - if p.units: - posting_lines.append(f' {p.account} {p.units.number} {p.units.currency}') - else: - posting_lines.append(f' {p.account}') - - return header + "\n" + "\n".join(posting_lines) - - if dtype == "price": - amt = getattr(directive, "amount", None) - currency = getattr(directive, "currency", "") - if amt: - return f'{date} price {currency} {amt.number} {amt.currency}' - return f'{date} price {currency}' - - if dtype == "commodity": - currency = getattr(directive, "currency", "") - return f'{date} commodity {currency}' - - if dtype == "event": - event_type = getattr(directive, "type", "") - desc = getattr(directive, "description", "") - return f'{date} event "{event_type}" "{desc}"' - - if dtype == "note": - comment = getattr(directive, "comment", "") - account = getattr(directive, "account", "") - return f'{date} note {account} "{comment}"' - - if dtype == "document": - filename = getattr(directive, "filename", "") - account = getattr(directive, "account", "") - return f'{date} document {account} "{filename}"' - - if dtype == "pad": - source_account = getattr(directive, "source_account", "") - account = getattr(directive, "account", "") - return f'{date} pad {account} {source_account}' - - if dtype == "query": - name = getattr(directive, "name", "") - query_string = getattr(directive, "query_string", "") - return f'{date} query "{name}" "{query_string}"' - - if dtype == "custom": - # Skip fava-specific custom directives that rustledger can't parse - custom_type = getattr(directive, "type", "") - if custom_type.startswith("fava"): - return "" - values = getattr(directive, "values", []) - values_str = " ".join(f'"{v}"' for v in values) - return f'{date} custom "{custom_type}" {values_str}' - - return "" + if ( + type(entry).__name__ == "RLCustom" + and getattr(entry, "type", "").startswith("fava") + ): + continue + rendered = to_string(entry) + if rendered: + parts.append(rendered) + return "\n".join(parts) + ("\n" if parts else "") diff --git a/tests/test_beans_str.py b/tests/test_beans_str.py index df387fba2..972a079af 100644 --- a/tests/test_beans_str.py +++ b/tests/test_beans_str.py @@ -117,3 +117,20 @@ def test_to_string_transaction_with_price() -> None: Liabilities:US:Chase:Slate -10.00 USD Expenses:Food 10.00 USD @ 10 EUR """) + + +def test_to_string_balance_with_tolerance() -> None: + # Round-trip parse: tolerance must appear between number and currency. + # Without it FX-rounded assertions silently flip red after a regen + # (see https://github.com/rustledger/rustfava/issues/144). + toleranced = create.balance( + {}, + datetime.date(2024, 12, 31), + "Assets:DE:Bank", + "900.00 EUR", + tolerance=Decimal("0.05"), + ) + rendered = to_string(toleranced) + assert "900.00 ~ 0.05 EUR" in rendered + assert "balance Assets:DE:Bank" in rendered + assert rendered.startswith("2024-12-31 ") diff --git a/tests/test_rustledger_query.py b/tests/test_rustledger_query.py new file mode 100644 index 000000000..ead560790 --- /dev/null +++ b/tests/test_rustledger_query.py @@ -0,0 +1,134 @@ +"""Tests for ``rustfava.rustledger.query._entries_to_source``. + +Regression coverage for https://github.com/rustledger/rustfava/issues/144 — +the query-time serializer used to ignore the in-tree ``to_string`` +formatter and silently drop tags, links, metadata, posting flags, cost +basis, prices, booking methods, and balance tolerance. +""" + +from __future__ import annotations + +import datetime +from decimal import Decimal + +from rustfava.beans import create +from rustfava.rustledger.query import _entries_to_source +from rustfava.rustledger.types import RLCustom +from rustfava.rustledger.types import RLCustomValue +from rustfava.rustledger.types import RLOpen + + +def test_entries_to_source_preserves_transaction_tags_links_and_metadata() -> None: + postings = [ + create.posting( + "Assets:US:Bank", + "-1000.00 USD", + flag="!", + meta={"confidence": "high"}, + ), + create.posting( + "Assets:DE:Bank", + "900.00 EUR", + price="1.1111 USD", + ), + ] + txn = create.transaction( + {"category": "international"}, + datetime.date(2024, 2, 15), + "*", + "Wise", + "USD->EUR transfer", + tags=frozenset({"fx-2024"}), + links=frozenset({"transfer-batch-12"}), + postings=postings, + ) + + source = _entries_to_source([txn]) + + # Tags + links must appear on the header. + assert "#fx-2024" in source + assert "^transfer-batch-12" in source + # Directive metadata must survive. + assert 'category: "international"' in source + # Posting metadata must survive. + assert 'confidence: "high"' in source + # Per-posting flag must survive. + assert "! Assets:US:Bank" in source + # Per-posting price must survive (rustledger normalizes @@ to @). + assert "@ 1.1111 USD" in source + + +def test_entries_to_source_preserves_cost_basis() -> None: + postings = [ + create.posting( + "Assets:US:Brokerage", + "10 AAPL", + cost=create.cost(Decimal("170.50"), "USD", datetime.date(2024, 3, 20)), + ), + create.posting("Assets:US:Bank", "-1705.00 USD"), + ] + txn = create.transaction( + {}, + datetime.date(2024, 3, 20), + "*", + "Schwab", + "Buy 10 AAPL", + postings=postings, + ) + + source = _entries_to_source([txn]) + + # `{price currency, date}` is what makes capital-gains math possible. + assert "{170.50 USD, 2024-03-20}" in source + + +def test_entries_to_source_preserves_balance_tolerance() -> None: + bal = create.balance( + {}, + datetime.date(2024, 12, 31), + "Assets:DE:Bank", + "900.00 EUR", + tolerance=Decimal("0.05"), + ) + + source = _entries_to_source([bal]) + + assert "balance Assets:DE:Bank" in source + assert "900.00 ~ 0.05 EUR" in source + + +def test_entries_to_source_preserves_open_booking_method() -> None: + opn = RLOpen( + meta={}, + date=datetime.date(2024, 1, 1), + account="Assets:US:Brokerage", + currencies=(), + booking="STRICT", + ) + + source = _entries_to_source([opn]) + + assert "open Assets:US:Brokerage" in source + assert '"STRICT"' in source + + +def test_entries_to_source_skips_fava_custom_directives() -> None: + # `custom "fava-option" ...` is not parseable by rledger and was + # explicitly skipped in the old serializer; keep that behavior. + fava_custom = RLCustom( + meta={}, + date=datetime.date(2024, 1, 1), + type="fava-option", + values=(RLCustomValue("title", dtype=str), RLCustomValue("Test", dtype=str)), + ) + other_custom = RLCustom( + meta={}, + date=datetime.date(2024, 1, 1), + type="budget", + values=(RLCustomValue("Expenses:Food", dtype=str),), + ) + + source = _entries_to_source([fava_custom, other_custom]) + + assert "fava-option" not in source + assert "budget" in source