fix(golden): keep regional locale variants as distinct golden files

[PeterJhongLinksys]

Problem

Generating golden snapshots for all locales failed midway. Regional locale
variants were mishandled in two ways:

1. Crash — _resolveLocales built Locale('es', 'ar') from a lowercase
   --dart-define=locales=es_ar. Flutter's localizations assert on
   non-standard forms like es_ar, aborting the entire run (set -e in
   run_generate_loc_snapshots.sh then stopped all remaining locales).
2. Collision — golden file names used only locale.languageCode, so es
   and es_AR both wrote ...-es.png. The regional variant silently
   overwrote the base language golden (same for fr/fr_CA, pt/pt_PT,
   zh/zh_TW).

Changes

• Add _localeTag() helper: regional variants keep their country code in
  golden file names and test descriptions (...-es_AR.png), joined with _
  (never -) so the report parser's - split keeps the country code attached.
• Normalize the country code to uppercase in _resolveLocales(), so lowercase
  input like es_ar resolves to Locale('es', 'AR') — matching the generated
  supportedLocales and avoiding the assertion.
• Update generate_gallery_report.dart parser comments to document the new
  languageCode_COUNTRY locale tag format (parse logic already handled it).

Verification

Regenerated all 26 locales via
./run_generate_loc_snapshots.sh -l "ar,da,de,el,en,es,es_ar,fi,fr,fr_ca,id,it,ja,ko,nb,nl,pl,pt,pt_pt,ru,sv,th,tr,vi,zh,zh_TW":

• 26/26 locales passed (357 tests each), 0 failures, no locale assertion.
• Regional variants now produce distinct goldens: es_AR, fr_CA, pt_PT,
  zh_TW each have 357 files, no longer overwriting the base language.
• Gallery report generated successfully (9282 images).

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[AustinChangLinksys]

🤖 Automated Review — Round 1 · 4512047..c42ce02 (full)

Verdict: ✅ APPROVE — Core fix is correct; regional locale variants now generate distinct golden files. Two maintainability warnings worth addressing.

	Conf.	Where	Issue (one-liner)
⚠️	🟢High	golden_runner.dart:210–219 / generate_gallery_report.dart:146–151	[both reviewers] Cross-file naming contract between _localeTag and report parser enforced only by comments — silent mismatch risk if separator ever changes
⚠️	🟡Med	golden_runner.dart:201–204	[single] _localeTag docstring claims "matches Locale.toString()" but ignores scriptCode — inaccurate for 3-part locales
⚠️	🟢High	golden_runner.dart:34–35	[both reviewers] 3-part locale (e.g. zh_Hant_TW) silently drops third segment; pre-existing, but toUpperCase() now masks the mis-assignment
💡	🟢High	golden_runner.dart:210 / test/	[both reviewers] No unit tests for _localeTag / _resolveLocales; golden naming breakage is costly to diagnose
💡	🟢High	golden_runner.dart:221–223	[single] _goldenFileName doc comment still shows {locale} — should describe the new {localeTag} format
💡	⚪Low	golden_runner.dart:28	[single] Empty element in dart-define (e.g. en,,es_AR) produces Locale(''); pre-existing
💡	⚪Low	generate_gallery_report.dart:82	[single] Length guard < 4 is applied before removing -dark suffix — semantically misleading but no actual bug

Confidence: 🟢High = code-verified · 🟡Med = located + reasoned, not fully confirmed · ⚪Low = speculative, please double-check.
Items marked [both reviewers] were independently flagged by two agents → higher confidence.

⚠️ Warning Details

W-1 — [both reviewers] Cross-file format contract enforced only by comments 🟢 High

golden_runner.dart:210–219 ↔ generate_gallery_report.dart:146–151

_localeTag() (new function) decides the naming format — es_AR joined with _ — and _parseGoldenFile in generate_gallery_report.dart implicitly relies on locale segments never containing -. Both files now share a comment explaining this invariant, but there is no shared constant, no shared function, and no integration test to enforce it. If a future change switches the separator in _localeTag to -, _parseGoldenFile's split('-') will silently misparse the locale segment (e.g. es-AR → device=es, locale=AR), corrupting all regional golden reports without any compilation error.

Fix: Extract _localeTag to a shared utility importable by both files (e.g. test/golden_test/golden_framework/golden_locale_format.dart), or add an integration test that round-trips _goldenFileName output through _parseGoldenFile.

---

W-2 — [single] _localeTag docstring inaccurate re: Locale.toString() 🟡 Med

golden_runner.dart:201–204

The doc comment states: "This matches Flutter's Locale.toString() and the ARB naming." Flutter's Locale.toString() includes scriptCode when present (e.g. zh_Hant_TW), but _localeTag only reads countryCode. For Locale('zh', null, 'Hant') it would return zh rather than zh_Hant. This is not a bug today (all supported locales use countryCode only), but the misleading doc could cause a future implementer to assume 3-part locales are handled.

Fix: Correct the comment to note scriptCode is intentionally ignored and document that all current supported locales use countryCode only.

---

W-3 — [both reviewers] 3-part locale silently drops third segment (pre-existing, exacerbated) 🟢 High

golden_runner.dart:34–35

final parts = s.trim().split('_');
return parts.length > 1
    ? Locale(parts[0], parts[1].toUpperCase())  // parts[2] silently ignored
    : Locale(parts[0]);

For --dart-define=locales=zh_Hant_TW, parts = ['zh', 'Hant', 'TW'] → produces Locale('zh', 'HANT') instead of Locale('zh', 'TW'). Pre-existing before this PR; however, the toUpperCase() addition now makes the incorrect output (HANT) pass Flutter's case-format assertion while remaining semantically wrong. Currently AppLocalizations.supportedLocales uses Locale('zh', 'TW') (not a 3-part subtag form), so there is no immediate breakage, but the silent truncation is a defensive gap.

Fix:

if (parts.length >= 3) {
  // 3-part BCP47 subtag (language_script_country): take country code (parts[2])
  return Locale(parts[0], parts[2].toUpperCase());
} else if (parts.length == 2) {
  return Locale(parts[0], parts[1].toUpperCase());
} else {
  return Locale(parts[0]);
}

✅ What looks good

• Core fix is correct: _localeTag uses _ as separator (never -), ensuring _parseGoldenFile's split('-') correctly identifies es_AR as a single locale segment rather than two fields — the root collision bug is resolved.
• toUpperCase() normalization is necessary: AppLocalizations.supportedLocales defines Locale('es', 'AR') (uppercase country); without normalization, --dart-define=locales=es_ar would trigger Flutter's locale format assertion.
• All three call sites updated consistently: goldenTest description in the state-loop (line 88), in the interaction-loop (line 146), and _goldenFileName (line 228) — no omissions.
• _localeTag edge cases handled: both countryCode == null and countryCode.isEmpty are guarded, preventing trailing-underscore artifacts such as en_.
• Report parser requires no functional change: generate_gallery_report.dart's dash-split logic is naturally compatible with underscore-joined locale tags; only the inline comment needed updating, and the author correctly identified this.

Cross-reviewed by two independent agents (security+correctness / architecture+maintainability). Automated — please sanity-check before merge.