Merge pull request 'i18n review fixes (#63)' (#10) from fix/i18n-review into main

Reviewed-on: #10

AGENTS.md

@@ -117,7 +117,10 @@ python3 build_zip.py   # -> bal-electrum-plugin.zip (deterministic, prints sha25
 ```
 
 - `build_zip.py` needs **Babel** (`pip install babel`): it compiles the
-  translation catalogs into the zip and stops if Babel is missing.
+  translation catalogs into the zip and stops if Babel is missing. The i18n
+  tests need it too. Install it in the **runtime** env used for releases
+  (`source "$BAL_HOME/electrum/env/bin/activate" && pip install babel`);
+  `make-release.sh` checks it before running anything.
 - `make-release.sh` requires gpg and Gitea credentials (`~/.git-credentials`
   or `GITEA_USER`/`GITEA_TOKEN`). It bumps `bal/manifest.json` — bump the
   version there, never invent a new source of truth.

CHANGELOG.md

@@ -3549,3 +3549,66 @@ delivery; when it is not, the plugin raises an explicit error (see below).
   catalogs refreshed (removed format strings marked obsolete).
 
 **Outcome:** DONE.
+
+---
+
+## 63. i18n review fixes (Truman's review of the translation PR)
+
+**Date:** 2026-10-07
+
+**What changed:**
+
+- `make-release.sh`
+  - Step 3 now also runs `tests/test_i18n.py`, `tests/test_translations.py`,
+    `tests/test_cli_*.py` and `tests/test_import_will_details.py` (the i18n
+    tests were never run by the release gate; the CLI and will-import tests
+    were not either).
+  - Checks up front that Babel is importable by `python3` and stops with
+    "pip install babel" otherwise: the i18n tests (step 3) and `build_zip.py`
+    (step 5) both need it, so a release no longer stops half-way.
+- `AGENTS.md`: Babel must be installed in the runtime env used for releases.
+- `pyproject.toml`: `[tool.ruff.lint.isort] known-first-party = ["bal"]`.
+  WHY: the reported I001 in `tests/test_translations.py` is not reproducible
+  from the repository root (ruff 0.15.11 and 0.6.9: clean), but it appears as
+  soon as ruff classifies `bal` as third-party (then 20 files report I001).
+  Removing the blank line would break the check where `bal` is first-party;
+  declaring it makes the result the same on every machine.
+- `bal/core/willexecutors.py` - `get_willexecutor_transactions` returns COPIES
+  of the executor records. It used to write `txs`, `txsids` and the
+  translated `broadcast_status` into the will item's own `we` dict, which is
+  saved in the wallet (stored data depended on the UI language).
+- `bal/core/will.py` - `WillItem` drops those transient fields when loading a
+  will saved by an older version, and maps a translated leading "New"
+  ("Nuovo", "Nuevo" - saved by i18n test builds) back to "New".
+- `bal/gui/qt/window.py`, `bal/cli/controller.py`: the status history of a new
+  will item is saved as the literal "New" (was `_("New")`).
+- `tests/test_core_status_language_neutral.py` (new, 7 tests; 5 fail on
+  `main`): English "New" stored, translated "New" mapped back, the mapping
+  covers every catalog's translation of "New", the push helper does not
+  modify the will item, old transient fields dropped on load.
+
+**Scope:** this entry contains ONLY the i18n review fixes. The other fixes
+prepared in the same session (invalidation fee, Check `KeyError`, cascade,
+deselected executor) are not part of it and will be proposed separately.
+
+**Verification (on `main` 827a1d1):**
+
+- New test file `tests/test_core_status_language_neutral.py`: 5 of 7 fail on
+  `main`, 7 of 7 pass with this change.
+- Extended release test list (as above) plus the new test file: 446 passed,
+  1 failed (`main`: 439 passed, same failure). The failure is pre-existing:
+  `test_group_e_mock_karen7::test_e5` needs `tests/karen7`, a wallet file that
+  is not in the repository.
+- Whole `tests/` directory: 609 passed, 26 failed, 3 collection errors
+  (`main`: 602 passed, and the SAME 26 failures and 3 errors, test by test).
+  All pre-existing and unrelated: 9 need `tests/karen7`; 21 in
+  `test_auto_rebuild_on_new_tx.py`, `test_cli_autorebuild.py` and
+  `test_cli_controller_offline.py` fail only when the whole directory runs
+  together (they pass when each file runs alone); 3 audio tests need an audio
+  device. `test_gui_export_dialogs.py::test_audio_import_receive_wiring` hangs
+  forever, also on `main`, so it was deselected.
+- `ruff check bal/ tests/`: the same 3 pre-existing I001 as on `main`
+  (`dialogs.py`, `test_core_animated_qr.py`, `test_core_qr_transfer.py`),
+  none new.
+
+**Outcome:** DONE (pending Truman's review).

bal/cli/controller.py

@@ -385,7 +385,9 @@ class BalController:
                 tx["heirsvalue"] = txs[txid].heirsvalue
                 tx["description"] = txs[txid].description
                 tx["willexecutor"] = copy_structure(txs[txid].willexecutor)
-                tx["status"] = _("New")
+                # Stored history: always English (translated only for display by
+                # format_status_history); never _() here.
+                tx["status"] = "New"
                 tx["baltx_fees"] = txs[txid].tx_fees
                 tx["time"] = creation_time
                 tx["heirs"] = copy_structure(txs[txid].heirs)

bal/core/will.py

@@ -1364,6 +1364,37 @@ class WillItem(Logger):
     # format_status_history() still translates them.
     LEGACY_STATUS_LABELS = (N_("New"),)
 
+    # Translations of "New" that test builds of the i18n work saved INTO the
+    # status history (window.py / the CLI used ``_("New")`` when building).
+    # They are mapped back to "New" on load so the history is language
+    # neutral again. Keep in sync with the "New" msgstr of every catalog in
+    # bal/locale (tests/test_core_status_language_neutral.py checks it).
+    TRANSLATED_NEW_LABELS = ("Nuovo", "Nuevo")
+
+    # Fields that Willexecutors.get_willexecutor_transactions /
+    # push_transactions_to_willexecutor used to write into the item's own
+    # executor record (older versions); they are transient push data.
+    TRANSIENT_PUSH_FIELDS = ("txs", "txsids", "broadcast_status")
+
+    @staticmethod
+    def _untranslate_new_status(status):
+        """Map a translated leading "New" of a saved history back to "New"."""
+        head, sep, rest = status.partition(".")
+        if head in WillItem.TRANSLATED_NEW_LABELS:
+            return "New" + sep + rest
+        return status
+
+    @staticmethod
+    def _without_transient_push_fields(we):
+        """Return ``we`` without the transient push fields (a copy if needed)."""
+        if not isinstance(we, dict) or not any(
+            k in we for k in WillItem.TRANSIENT_PUSH_FIELDS
+        ):
+            return we
+        return {
+            k: v for k, v in we.items() if k not in WillItem.TRANSIENT_PUSH_FIELDS
+        }
+
     def set_status(self, status, value=True):
         """Set a status flag and apply the related side effects.
 
@@ -1445,8 +1476,10 @@ class WillItem(Logger):
             w = data
         self.tx = Will.get_tx_from_any(w["tx"])
         self.heirs = w.get("heirs", None)
-        self.we = w.get("willexecutor", None)
-        self.status = w.get("status") or ""
+        self.we = WillItem._without_transient_push_fields(
+            w.get("willexecutor", None)
+        )
+        self.status = WillItem._untranslate_new_status(w.get("status") or "")
         self.description = w.get("description", None)
         self.time = w.get("time", None)
         self.change = w.get("change", None)

bal/core/willexecutors.py

@@ -231,6 +231,13 @@ class Willexecutors:
 
     @staticmethod
     def get_willexecutor_transactions(will, force=False):
+        """Group the valid, signed, not-yet-pushed will txs by will-executor.
+
+        Returns ``{url: executor_record}`` where each record gets ``txs`` (the
+        raw txs, one per line) and ``txsids``.
+
+        The returned records are COPIES: the will items are never modified.
+        """
         willexecutors = {}
         for wid, willitem in will.items():
             if willitem.get_status("VALID"):
@@ -240,10 +247,18 @@ class Willexecutors:
                             url = willexecutor["url"]
                             if willexecutor and Willexecutors.is_selected(willexecutor):
                                 if url not in willexecutors:
-                                    willexecutor["txs"] = ""
-                                    willexecutor["txsids"] = []
-                                    willexecutor["broadcast_status"] = _("Waiting...")
-                                    willexecutors[url] = willexecutor
+                                    # Work on a COPY of the record: the item's
+                                    # own ``we`` dict is saved in the wallet, so
+                                    # the transient push fields below (raw txs,
+                                    # txids and the - translated - broadcast
+                                    # status shown in the push dialog) must
+                                    # never end up in it (stored data must not
+                                    # depend on the UI language, see bal/i18n.py).
+                                    record = dict(willexecutor)
+                                    record["txs"] = ""
+                                    record["txsids"] = []
+                                    record["broadcast_status"] = _("Waiting...")
+                                    willexecutors[url] = record
                                 willexecutors[url]["txs"] += str(willitem.tx) + "\n"
                                 willexecutors[url]["txsids"].append(wid)
 

bal/gui/qt/window.py

@@ -551,7 +551,9 @@ class BalWindow:
                     tx["heirsvalue"] = txs[txid].heirsvalue
                     tx["description"] = txs[txid].description
                     tx["willexecutor"] = copy_structure(txs[txid].willexecutor)
-                    tx["status"] = _("New")
+                    # Stored history: always English (translated only for display by
+                    # format_status_history); never _() here.
+                    tx["status"] = "New"
                     tx["baltx_fees"] = txs[txid].tx_fees
                     tx["time"] = creation_time
                     tx["heirs"] = copy_structure(txs[txid].heirs)

make-release.sh

@@ -5,7 +5,7 @@
 #   ./make-release.sh           # read version from bal/manifest.json
 #   ./make-release.sh v0.6.2    # bump manifest to 0.6.2, then release
 #
-# Requires: git, gpg, curl, python3, sha256sum
+# Requires: git, gpg, curl, python3, sha256sum, Babel for python3 (pip install babel)
 # Optional: ruff (lint skipped if not installed)
 # Credentials: ~/.git-credentials or GITEA_USER / GITEA_TOKEN env vars
 
@@ -33,6 +33,12 @@ else
     NEW_VER=""
 fi
 
+# Babel is needed by the i18n tests (step 3) and by build_zip.py, which
+# compiles the translation catalogs into the zip (step 5). Check it up front
+# so a release never stops half-way.
+python3 -c "import babel" 2>/dev/null \
+    || die "Babel is missing for $(command -v python3): pip install babel"
+
 echo "=== Release ${TAG} ==="
 echo "Current manifest version: ${CURRENT_VER}"
 [ -n "$NEW_VER" ] && echo "New version (will bump):   ${NEW_VER}"
@@ -68,6 +74,8 @@ if QT_QPA_PLATFORM=offscreen PYTHONPATH=electrum-src python3 -m pytest \
       tests/test_group_d_alarms.py tests/test_group_e_mock_karen7.py \
       tests/test_group_f_heir_change_rebuild.py tests/test_group_g_basic_calendar.py \
       tests/test_group_h_v048.py \
+      tests/test_i18n.py tests/test_translations.py \
+      tests/test_cli_*.py tests/test_import_will_details.py \
       -q 2>&1; then
     echo "All tests passed."
 else

pyproject.toml

@@ -6,6 +6,12 @@ target-version = "py312"
 select =["E", "W", "F", "I", "N", "B", "INT"]  # INT: gettext rules (see bal/i18n.py)
 ignore = ["E501"]
 
+[tool.ruff.lint.isort]
+# Classify the plugin package explicitly: otherwise ruff decides from where it
+# is run / how the project root is detected, and the same import block is
+# "sorted" on one machine and I001 on another.
+known-first-party = ["bal"]
+
 [tool.ruff.lint.pep8-naming]
 classmethod-decorators = ["classmethod", "classproperty"]  # electrum.util.classproperty uses cls
 

tests/test_core_status_language_neutral.py

@@ -0,0 +1,136 @@
+"""
+Stored will data must not depend on the GUI language (rule in bal/i18n.py).
+
+Covers the two findings of the i18n review:
+
+* the status history of a new will item is saved as English "New" (not
+  ``_("New")``), and histories saved by test builds as "Nuovo"/"Nuevo" are
+  mapped back to "New" on load;
+* the push helper no longer writes transient fields (``txs``, ``txsids`` and
+  the translated ``broadcast_status``) into the will item's executor record,
+  and such fields saved by older versions are dropped on load.
+
+Run (standalone or with pytest):
+    QT_QPA_PLATFORM=offscreen PYTHONPATH=electrum-src \
+        python3 tests/test_core_status_language_neutral.py
+"""
+
+import os
+import re
+import sys
+from pathlib import Path
+from unittest.mock import patch
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), os.pardir))
+
+from electrum.transaction import Transaction
+
+from bal.core.will import WillItem
+from bal.core.willexecutors import Willexecutors
+
+# WillItem parses a raw tx and calls add_info_from_wallet; no live wallet here.
+_patcher = patch.object(Transaction, "add_info_from_wallet")
+_patcher.start()
+
+ROOT = Path(__file__).resolve().parent.parent
+
+_VALID_TX_HEX = (
+    "01000000012a5c9a94fcde98f5581cd00162c60a13936ceb75389ea65b"
+    "f38633b424eb4031000000006c493046022100a82bbc57a0136751e543"
+    "3f41cf000b3f1a99c6744775e76ec764fb78c54ee100022100f9e80b7d"
+    "e89de861dc6fb0c1429d5da72c2b6b2ee2406bc9bfb1beedd729d98501"
+    "2102e61d176da16edd1d258a200ad9759ef63adf8e14cd97f53227bae3"
+    "5cdb84d2f6ffffffff0140420f00000000001976a914230ac37834073a"
+    "42146f11ef8414ae929feaafc388ac00000000"
+)
+URL = "https://we.example"
+
+
+def _item(status="", we=None):
+    return WillItem(
+        {"tx": _VALID_TX_HEX, "heirs": {}, "willexecutor": we, "status": status}
+    )
+
+
+# ------------------------------------------------------------------ #
+# Status history "New"
+# ------------------------------------------------------------------ #
+
+def test_builders_store_english_new():
+    """window.py and the CLI controller must not translate the stored status."""
+    for rel in ("bal/gui/qt/window.py", "bal/cli/controller.py"):
+        src = (ROOT / rel).read_text(encoding="utf-8")
+        assert re.search(r'tx\["status"\]\s*=\s*"New"', src), rel
+        assert not re.search(r'tx\["status"\]\s*=\s*_\(', src), rel
+
+
+def test_translated_new_is_mapped_back_on_load():
+    assert _item("Nuovo.Signed.Pushed").status == "New.Signed.Pushed"
+    assert _item("Nuevo").status == "New"
+    assert _item("New.Signed").status == "New.Signed"
+
+
+def test_only_the_leading_new_is_touched():
+    assert _item(".Nuovo").status == ".Nuovo"
+    assert _item("Signed.Nuovo").status == "Signed.Nuovo"
+
+
+def test_translated_new_labels_match_the_catalogs():
+    """Every catalog's translation of "New" must be recognised on load."""
+    found = set()
+    for po in (ROOT / "bal" / "locale").glob("*/LC_MESSAGES/bal.po"):
+        text = po.read_text(encoding="utf-8")
+        m = re.search(r'^msgid "New"\nmsgstr "(.*)"$', text, re.MULTILINE)
+        if m and m.group(1):
+            found.add(m.group(1))
+    assert found, "no catalog translates 'New'"
+    assert found <= set(WillItem.TRANSLATED_NEW_LABELS), found
+
+
+# ------------------------------------------------------------------ #
+# Transient push fields
+# ------------------------------------------------------------------ #
+
+class FakeItem:
+    def __init__(self):
+        self.we = {"url": URL, "selected": True, "address": "a", "base_fee": 1}
+        self.tx = "rawtx"
+
+    def get_status(self, name):
+        return name in ("VALID", "COMPLETE")
+
+
+def test_push_helper_does_not_modify_the_will_item():
+    item = FakeItem()
+    before = dict(item.we)
+    out = Willexecutors.get_willexecutor_transactions({"t1": item})
+    assert out[URL]["txsids"] == ["t1"]
+    assert "broadcast_status" in out[URL]
+    assert item.we == before
+    assert out[URL] is not item.we
+
+
+def test_transient_fields_saved_by_older_versions_are_dropped_on_load():
+    we = {
+        "url": URL,
+        "selected": True,
+        "txs": "raw\n",
+        "txsids": ["t1"],
+        "broadcast_status": "Riuscito",
+    }
+    item = _item("New", we)
+    assert item.we == {"url": URL, "selected": True}
+    assert "broadcast_status" not in item.to_dict()["willexecutor"]
+
+
+def test_clean_executor_record_is_kept_as_is():
+    we = {"url": URL, "selected": True}
+    assert _item("New", we).we is we
+
+
+if __name__ == "__main__":
+    for name in sorted(dir()):
+        if name.startswith("test_"):
+            globals()[name]()
+            print(f"  [OK] {name}")
+    print("[OK] All language-neutral status tests passed")