refactor(scripts): rename translation macro and enforce no-default-override rule

[julianam-w]

No description provided.

[github-actions]

Claude finished @julianam-w's task in 2m 21s —— View job

---

Code Review

• Gather context (REVIEW.md, CLAUDE.md, AGENT.md)
• Read changed files
• Run validation scripts (skipped — shell execution not permitted in this run)
• Post review feedback

---

⚠️ 1 suggestion, 3 nits — no blockers found.

Summary

The bulk of new commits since the last review are ruff auto-formatting (5269b94) and a CI workflow tweak (debbe20). The core functional logic — translation_utils.py, assert_no_default_overrides, renamed macro, and tests — is unchanged and still looks solid.

The three items from the previous review remain open.

---

🟡 Suggestion — Migration note still missing (unaddressed from prior review)

AGENT.md was updated to reflect the new macro name but has no guidance for deployment-repo operators on what breaks and how to fix it. Any deployment CSV that currently defines a default column for a stringId that exists in report_translations_standard.csv will now cause CI to hard-exit with no upgrade path in the docs.

Suggested addition to the Translation system section of AGENT.md:

Deployment CSVs (report_translations_<deployment>.csv) must not define a default translation for any stringId that already exists in report_translations_standard.csv. Only language-specific columns (e.g. en, fr) are permitted for those rows. If your deployment CSV currently has a default column for a standard stringId, remove it — leaving only the language-specific overrides.

Fix this →

---

🔵 Nit — Silent enforcement skip when standard CSV is absent (new)

scripts/check_translations.py lines 33–38:

if not standard_csv_path.exists():
    cprint(f"⚠️ File does not exist: {standard_csv_path}", "warning")
standard = read_translations_csv(standard_csv_path)  # returns {} if missing
localised = read_translations_csv(deployment_csv_path)
assert_no_default_overrides(localised, standard)       # standard={} → always passes

read_translations_csv returns {} for a missing file, so assert_no_default_overrides(localised, {}) will never find a violation — string_id in standard is always False. The warning is printed, but the guard is silently bypassed. The same pattern exists in generate_translation_macro.py lines 20–24. Consider turning the warning into an early exit, or at minimum make the skip explicit:

if not standard_csv_path.exists():
    cprint(f"⚠️ File does not exist: {standard_csv_path} — skipping default-override check", "warning")
    standard = {}
else:
    standard = read_translations_csv(standard_csv_path)
    assert_no_default_overrides(localised, standard)

---

🔵 Nit — Duplicate assert_no_default_overrides call (unaddressed from prior review)

check_translations.py:38 and generate_translation_macro.py:24 both call assert_no_default_overrides. build_reporting_assets.py runs check_translations.py via subprocess (line 44) and then calls generate_translation_macro() directly (line 56), so the check fires twice per build. It's harmless and generate_translation_macro.py being invokable standalone justifies the guard — but a brief comment would prevent future confusion:

# Re-checks even if called via build_reporting_assets (where check_translations already ran)
# so this script remains safe to invoke standalone.
assert_no_default_overrides(localised, standard)

---

🔵 Nit — python-ci.yml still pins to @main (unaddressed from prior review)

.github/workflows/python-ci.yml line 10:

uses: beyondessential/maui-team/.github/workflows/python-ci.yml@main

@main is a mutable ref. If intentional org convention, ignore this — but if not, consider pinning to a tag or SHA for reproducibility.

---

Regression testing checklist

python -m pytest
python scripts/validate_report_configs.py
python scripts/check_translations.py
python scripts/generate_translation_macro.py