From 0ea96cd38439ac202a244a69b07b1609863ef76d Mon Sep 17 00:00:00 2001 From: Padreug Date: Wed, 17 Jun 2026 10:06:28 +0200 Subject: [PATCH] fix(accounts): anchor duplicate-account detection to a real Open directive (libra-#48) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existence check matched 'open ' 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 ' 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) --- fava_client.py | 36 ++++++++++++++++++++++++--------- tests/test_unit.py | 50 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/fava_client.py b/fava_client.py index f176031..053ed0d 100644 --- a/fava_client.py +++ b/fava_client.py @@ -60,6 +60,30 @@ def _escape_beancount_string(value: str) -> str: ) +def _open_directive_exists(source: str, account_name: str) -> bool: + """Return True if `source` already contains an Open directive for exactly + `account_name`. + + Anchored to a real `YYYY-MM-DD open ` directive line (re.MULTILINE) + so the account name can't match text inside another account's description + metadata or a comment (false positive → spurious 409). The trailing + negative-lookahead `(?![\\w:-])` requires the next char not to be an + account-continuation char, so: + - a prefix (Expenses:Gas) does not match a longer sibling + (Expenses:GasStation / Expenses:Gas:Vehicle), and + - a real directive with an inline comment and no space + (`open Expenses:Gas;legacy`) is still detected (`;` ends the name), + which the previous `(?:\\s|$)` boundary missed → duplicate write. + """ + return bool( + re.search( + rf"^\d{{4}}-\d{{2}}-\d{{2}} open {re.escape(account_name)}(?![\w:-])", + source, + re.MULTILINE, + ) + ) + + class FavaClient: """ Async client for Fava REST API. @@ -1644,15 +1668,9 @@ class FavaClient: source = source_data["source"] # Step 2: Check if account already exists (may have been - # created by a concurrent request). Anchor on the Open - # directive and require the account to be followed by - # whitespace/end-of-line so a prefix (Expenses:Gas) does - # not match a longer sibling (Expenses:GasStation). - if re.search( - rf"open {re.escape(account_name)}(?:\s|$)", - source, - re.MULTILINE, - ): + # created by a concurrent request). See + # _open_directive_exists for the anchoring rationale. + if _open_directive_exists(source, account_name): logger.info(f"Account {account_name} already exists in {target_file}") return { "data": sha256sum, diff --git a/tests/test_unit.py b/tests/test_unit.py index 2fa41ec..4f97b03 100644 --- a/tests/test_unit.py +++ b/tests/test_unit.py @@ -31,9 +31,59 @@ bf = _module("beancount_format") au = _module("account_utils") val = _module("core.validation") mdl = _module("models") +fc = _module("fava_client") AccountType = mdl.AccountType +# --------------------------------------------------------------------------- +# fava_client._open_directive_exists — duplicate-account detection +# --------------------------------------------------------------------------- + + +def test_open_directive_exists_matches_real_directive(): + src = "2020-01-01 open Expenses:Vehicle:Gas\n" + assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True + + +def test_open_directive_exists_matches_currency_constrained_and_metadata(): + src = ( + "2020-01-01 open Expenses:Vehicle:Gas EUR, SATS\n" + ' added_by: "abc"\n' + ) + assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True + + +def test_open_directive_exists_matches_inline_comment_without_space(): + # Valid Beancount: the account token ends at ';'. The old (?:\\s|$) boundary + # missed this → duplicate Open written → bean-check breaks. + src = "2020-01-01 open Expenses:Vehicle:Gas;legacy-import\n" + assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True + + +def test_open_directive_exists_ignores_name_inside_description(): + # The name appears only inside another account's description metadata. + src = ( + "2020-01-01 open Expenses:Notes\n" + ' description: "remember to open Expenses:Vehicle:Gas next month"\n' + ) + assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False + + +def test_open_directive_exists_ignores_comment_line(): + src = "; TODO: open Expenses:Vehicle:Gas eventually\n" + assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False + + +def test_open_directive_exists_does_not_match_longer_sibling(): + src = "2020-01-01 open Expenses:Vehicle:GasStation\n" + assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False + + +def test_open_directive_exists_does_not_match_deeper_child(): + src = "2020-01-01 open Expenses:Vehicle:Gas:Premium\n" + assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False + + # --------------------------------------------------------------------------- # beancount_format.sanitize_link # ---------------------------------------------------------------------------