diff options
| author | Danilo M. <danix@danix.xyz> | 2026-07-01 10:53:58 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-07-01 10:53:58 +0200 |
| commit | 1c807048f8c754b421dae5a07ee8c053e3788348 (patch) | |
| tree | 252dcd2c199c3a8a68e76539eb2a07f695c49549 | |
| parent | 69647b1ca6f2e11429850b0782c65d04b86509cb (diff) | |
| download | firefly-cli-1c807048f8c754b421dae5a07ee8c053e3788348.tar.gz firefly-cli-1c807048f8c754b421dae5a07ee8c053e3788348.zip | |
feat: tx add --skip-dupes for idempotent imports (v0.3.4)
Re-running an import (a retried or double-read batch) created phantom
duplicate transactions and drifted balances, with no signal.
--skip-dupes searches for an existing tx with the same amount + date +
source + destination before writing; on a match it emits
{"skipped": "duplicate", "matched_id": "<id>"} and exits 0 without writing.
Off by default (one extra search per add only when set). dry-run wins over
skip-dupes: a preview never triggers the search. Match is amount+date+
accounts, not description, so genuinely distinct same-value same-day
transfers between the same accounts look like duplicates; documented.
Also document a Firefly quirk in SKILL.md: a missing/deleted transaction id
returns 401 (not 404) on tx get/edit/delete, so a 401 after delete confirms
the record is gone rather than signalling an auth failure.
Verified live on the test instance: create -> identical --skip-dupes skips
with matched_id and no write, --dry-run --skip-dupes previews without
searching, no-match --skip-dupes writes; records cleaned up after.
PATCH: new optional flag, contract unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| -rw-r--r-- | SKILL.md | 17 | ||||
| -rw-r--r-- | completions/firefly.bash | 2 | ||||
| -rw-r--r-- | firefly_cli/__init__.py | 2 | ||||
| -rw-r--r-- | firefly_cli/commands/transaction.py | 14 | ||||
| -rw-r--r-- | pyproject.toml | 2 | ||||
| -rw-r--r-- | tests/unit/test_commands_transaction.py | 66 |
6 files changed, 93 insertions, 10 deletions
@@ -56,7 +56,8 @@ firefly account balance <name|id> [--at YYYY-MM-DD] firefly account create <name> --type asset|expense|revenue [--opening-balance N] [--currency CODE] firefly tx add <amount> --from <acct> --to <acct> - [--desc TEXT] [--date YYYY-MM-DD] [--category NAME] [--tags a,b] [--type T] [--dry-run] + [--desc TEXT] [--date YYYY-MM-DD] [--category NAME] [--tags a,b] [--type T] + [--dry-run] [--skip-dupes] firefly tx edit <id> [--amount N] [--date YYYY-MM-DD] [--desc TEXT] [--from <acct>] [--to <acct>] [--category NAME] [--tags a,b] [--type T] # only fields passed are changed @@ -110,6 +111,16 @@ sends nothing, printing `{"dry_run": true, "would_send": {...}}`. A missing account is still a hard error (exit 1). Recipe: dry-run every row, create any accounts the errors name, then run the batch for real. +**Idempotent re-runs (avoid duplicate rows):** re-importing the same rows +(a retried or double-read batch) otherwise creates phantom duplicates and +drifts balances. Pass `--skip-dupes` to `tx add`: before writing it searches +for an existing tx with the same amount + date + source + destination, and if +one is found emits `{"skipped": "duplicate", "matched_id": "<id>"}` and exits 0 +without writing. Off by default (one extra search per add only when set). Note +it matches amount+date+accounts, not description, so two genuinely distinct +purchases of the same value, same day, between the same accounts look like a +duplicate; omit `--skip-dupes` where that is expected. + **Check a balance:** ```bash firefly account balance test01 # -> {"id","name","current_balance"} @@ -148,6 +159,10 @@ firefly tx delete 76 --yes # remove a duplicate a truncated list makes a correct ledger look wrong. - Amounts are strings in responses, often with trailing zeros (`"0.010000000000"`). Compare numerically, do not string-match. +- A missing or deleted transaction id returns `API error 401: Unauthenticated.` + (not 404) on `tx get`/`tx edit`/`tx delete`. This is a Firefly quirk, not an + auth failure: if other commands work, the id simply does not exist. After a + `tx delete`, a 401 on that id confirms it is gone. - Dates in `tx list` filter by transaction date; omit them to use Firefly's default period, which may hide older transactions. Pass an explicit `--since` to be sure. diff --git a/completions/firefly.bash b/completions/firefly.bash index 2dbe218..68c4b74 100644 --- a/completions/firefly.bash +++ b/completions/firefly.bash @@ -40,7 +40,7 @@ _firefly() { "account balance") leaf_opts="--at";; "account create") leaf_opts="--currency --opening-balance --type";; "account list") leaf_opts="--type";; - "tx add") leaf_opts="--category --date --desc --dry-run --from --tags --to --type";; + "tx add") leaf_opts="--category --date --desc --dry-run --from --skip-dupes --tags --to --type";; "tx delete") leaf_opts="--yes";; "tx edit") leaf_opts="--amount --category --date --desc --from --tags --to --type";; "tx list") leaf_opts="--account --all --limit --since --until";; diff --git a/firefly_cli/__init__.py b/firefly_cli/__init__.py index 23ac8eb..b7a8764 100644 --- a/firefly_cli/__init__.py +++ b/firefly_cli/__init__.py @@ -2,4 +2,4 @@ # Copyright (C) 2026 Danilo M. <danix@danix.xyz> # Licensed under the GNU General Public License v2.0 only. -__version__ = "0.3.3" +__version__ = "0.3.4" diff --git a/firefly_cli/commands/transaction.py b/firefly_cli/commands/transaction.py index 72d2606..8d858b6 100644 --- a/firefly_cli/commands/transaction.py +++ b/firefly_cli/commands/transaction.py @@ -32,6 +32,8 @@ def _add_args(p): help="withdrawal|deposit|transfer (overrides inference)") p.add_argument("--dry-run", dest="dry_run", action="store_true", help="resolve accounts and show what would be sent; write nothing") + p.add_argument("--skip-dupes", dest="skip_dupes", action="store_true", + help="skip if a tx with same amount+date+source+destination exists") @registry.command("tx add", help="record a transaction; source/destination resolve to accounts, category/tags auto-create", args=_add_args) def cmd_add(args, ctx): @@ -54,8 +56,20 @@ def cmd_add(args, ctx): split["tags"] = [t.strip() for t in args.tags.split(",") if t.strip()] if args.dry_run: # Accounts already resolved above (missing name = hard error). Write nothing. + # dry-run wins over skip-dupes: caller wants a preview, not a write. output.emit({"dry_run": True, "would_send": split}, human=ctx.human) return 0 + if args.skip_dupes: + # Firefly's search matches amount numerically; names quoted for spaces. + query = (f'amount_is:{split["amount"]} date_on:{split["date"]} ' + f'source_account_is:"{src["name"]}" ' + f'destination_account_is:"{dst["name"]}"') + hits = output.unwrap(ctx.client.request( + "GET", "/api/v1/search/transactions", params={"query": query})) + if hits: + output.emit({"skipped": "duplicate", "matched_id": hits[0].get("id")}, + human=ctx.human) + return 0 resp = ctx.client.request("POST", "/api/v1/transactions", body={"transactions": [split]}) output.emit(output.unwrap(resp), human=ctx.human) diff --git a/pyproject.toml b/pyproject.toml index 4b4bf9b..91e78c0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "firefly-iii-agent" -version = "0.3.3" +version = "0.3.4" description = "CLI tool for agent interaction with Firefly III" readme = "README.md" requires-python = ">=3.11" diff --git a/tests/unit/test_commands_transaction.py b/tests/unit/test_commands_transaction.py index a6b5228..13d6469 100644 --- a/tests/unit/test_commands_transaction.py +++ b/tests/unit/test_commands_transaction.py @@ -19,7 +19,7 @@ class TestTxAdd(unittest.TestCase): "attributes": {}}} args = MagicMock(amount="42.50", source="Checking", dest="Groceries", desc="food", date="2026-06-30", category=None, - tags=None, type=None, dry_run=False) + tags=None, type=None, dry_run=False, skip_dupes=False) rc = tx.cmd_add(args, ctx) self.assertEqual(rc, 0) method, path = client.request.call_args[0][:2] @@ -40,7 +40,7 @@ class TestTxAdd(unittest.TestCase): client.request.return_value = {"data": {"id": "1", "attributes": {}}} args = MagicMock(amount="1000", source="Salary", dest="Checking", desc="pay", date=None, category=None, tags=None, - type=None, dry_run=False) + type=None, dry_run=False, skip_dupes=False) tx.cmd_add(args, ctx) self.assertEqual(client.request.call_args[1]["body"]["transactions"][0]["type"], "deposit") @@ -51,7 +51,7 @@ class TestTxAdd(unittest.TestCase): client.request.return_value = {"data": {"id": "1", "attributes": {}}} args = MagicMock(amount="5", source="A", dest="B", desc=None, date=None, category=None, tags="food,fun", type="transfer", - dry_run=False) + dry_run=False, skip_dupes=False) tx.cmd_add(args, ctx) split = client.request.call_args[1]["body"]["transactions"][0] self.assertEqual(split["type"], "transfer") @@ -64,7 +64,7 @@ class TestTxAdd(unittest.TestCase): client.request.return_value = {"data": {"id": "1", "attributes": {}}} args = MagicMock(amount="5", source="A", dest="B", desc=None, date=None, category="Brand New Cat", tags=None, type="withdrawal", - dry_run=False) + dry_run=False, skip_dupes=False) tx.cmd_add(args, ctx) split = client.request.call_args[1]["body"]["transactions"][0] self.assertEqual(split["category_name"], "Brand New Cat") @@ -73,7 +73,7 @@ class TestTxAdd(unittest.TestCase): ctx, client, resolver = make_ctx() resolver.account.side_effect = lambda n: {"id": "1", "type": "asset", "name": n} args = MagicMock(amount="5", source="A", dest="B", desc="x", date="2026-06-01", - category=None, tags=None, type="withdrawal", dry_run=True) + category=None, tags=None, type="withdrawal", dry_run=True, skip_dupes=False) rc = tx.cmd_add(args, ctx) self.assertEqual(rc, 0) client.request.assert_not_called() # accounts resolved, nothing written @@ -84,12 +84,66 @@ class TestTxAdd(unittest.TestCase): ctx, client, resolver = make_ctx() resolver.account.side_effect = ResolutionError('No account named "B"') args = MagicMock(amount="5", source="A", dest="B", desc=None, date=None, - category=None, tags=None, type="withdrawal", dry_run=True) + category=None, tags=None, type="withdrawal", dry_run=True, skip_dupes=False) with self.assertRaises(ResolutionError): tx.cmd_add(args, ctx) client.request.assert_not_called() resolver.category.assert_not_called() + def test_skip_dupes_skips_when_match_exists(self): + ctx, client, resolver = make_ctx() + resolver.account.side_effect = lambda n: { + "A": {"id": "1", "name": "A", "type": "asset"}, + "B": {"id": "2", "name": "B", "type": "expense"}, + }[n] + # search finds an existing tx -> skip, no POST + client.request.return_value = {"data": [ + {"id": "441", "attributes": {}}]} + args = MagicMock(amount="9.99", source="A", dest="B", desc="x", + date="2026-06-10", category=None, tags=None, + type=None, dry_run=False, skip_dupes=True) + rc = tx.cmd_add(args, ctx) + self.assertEqual(rc, 0) + # exactly one call, the GET search; no POST + self.assertEqual(client.request.call_count, 1) + method, path = client.request.call_args[0][:2] + self.assertEqual(method, "GET") + q = client.request.call_args[1]["params"]["query"] + self.assertIn("amount_is:9.99", q) + self.assertIn("date_on:2026-06-10", q) + self.assertIn("source_account_is:", q) + self.assertIn("destination_account_is:", q) + + def test_skip_dupes_writes_when_no_match(self): + ctx, client, resolver = make_ctx() + resolver.account.side_effect = lambda n: { + "A": {"id": "1", "name": "A", "type": "asset"}, + "B": {"id": "2", "name": "B", "type": "expense"}, + }[n] + # first call = search (empty), second = POST + client.request.side_effect = [ + {"data": []}, + {"data": {"id": "99", "attributes": {}}}, + ] + args = MagicMock(amount="9.99", source="A", dest="B", desc="x", + date="2026-06-10", category=None, tags=None, + type=None, dry_run=False, skip_dupes=True) + rc = tx.cmd_add(args, ctx) + self.assertEqual(rc, 0) + self.assertEqual(client.request.call_count, 2) + self.assertEqual(client.request.call_args[0][:2], + ("POST", "/api/v1/transactions")) + + def test_dry_run_beats_skip_dupes_no_search(self): + ctx, client, resolver = make_ctx() + resolver.account.side_effect = lambda n: {"id": "1", "type": "asset", "name": n} + args = MagicMock(amount="5", source="A", dest="B", desc=None, date="2026-06-01", + category=None, tags=None, type="withdrawal", + dry_run=True, skip_dupes=True) + rc = tx.cmd_add(args, ctx) + self.assertEqual(rc, 0) + client.request.assert_not_called() # dry-run wins: no search, no write + class TestTxEdit(unittest.TestCase): def test_edit_sends_only_provided_fields(self): ctx, client, resolver = make_ctx() |
