Skip to content

refactor: externalize localization reference data - #77

Merged
IceCodeNew merged 1 commit into
masterfrom
codex/externalize-localization-data
Jul 21, 2026
Merged

refactor: externalize localization reference data#77
IceCodeNew merged 1 commit into
masterfrom
codex/externalize-localization-data

Conversation

@IceCodeNew

@IceCodeNew IceCodeNew commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Follow-up to #72 and #73.

Addresses the localization reference-data findings from:

Moves weather-document, QWeather, air-quality, allergen, and briefing scaffold translations into packaged localization.json. A cached, read-only loader validates the complete table set, supported languages, required fields, non-blank values, and aliases before adapters or renderers consume the data. The architecture note records the startup-failure, restart, and runtime-extension boundaries.

Verification:

  • 803 tests passed with branch coverage
  • line coverage 99.85% (increased from 99.78%)
  • application reference_data.py line and branch coverage 100%
  • prek run --all-files
  • built wheel contains weather_briefing/data/localization.json

Summary by CodeRabbit

  • New Features

    • Added centralized localization data for weather briefings, air quality, allergens, and supported language variants.
    • Added support for English, Japanese, Simplified Chinese, and Traditional Chinese labels and formatting.
  • Bug Fixes

    • Added validation to detect incomplete, invalid, or missing localization data.
    • Localization data is now protected from unintended runtime changes.
  • Documentation

    • Documented localization data requirements, validation behavior, packaging updates, and restart considerations.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 163c26a9-1f5c-4939-8edf-63acb7e256ef

📥 Commits

Reviewing files that changed from the base of the PR and between baf5ca8 and f9910b0.

📒 Files selected for processing (8)
  • docs/notes.md
  • tests/test_reference_data.py
  • weather_briefing/air_quality.py
  • weather_briefing/allergen.py
  • weather_briefing/data/localization.json
  • weather_briefing/reference_data.py
  • weather_briefing/render.py
  • weather_briefing/weather_context.py

📝 Walkthrough

Walkthrough

Localization strings move into localization.json. localization_table() validates, aliases, caches, and freezes tables, while weather and rendering modules consume them. Tests cover valid values, malformed data, alias errors, and immutability.

Changes

Localization reference-data migration

Layer / File(s) Summary
Localization schema and loader
weather_briefing/data/localization.json, weather_briefing/reference_data.py, docs/notes.md
Adds localized tables and aliases, validates their structure and values, returns immutable cached mappings, and documents the loading boundaries.
Weather module localization wiring
weather_briefing/air_quality.py, weather_briefing/allergen.py, weather_briefing/weather_context.py, weather_briefing/render.py
Replaces embedded language dictionaries with localization_table() lookups and updates rendering helpers to accept mapping-like label tables.
Localization validation coverage
tests/test_reference_data.py
Tests packaged translations, incomplete or malformed tables, invalid aliases, unknown tables, whitespace values, and immutable results.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant WeatherModules
  participant localization_table
  participant localization_json
  participant Renderer
  WeatherModules->>localization_table: request air_quality, allergen, weather_document, or qweather table
  localization_table->>localization_json: load tables and aliases
  localization_table-->>WeatherModules: return validated immutable mappings
  WeatherModules->>Renderer: provide localized labels and templates
Loading

Possibly related PRs

Suggested labels: 🕐 40+ Minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: moving localization reference data out of code and into packaged data.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/externalize-localization-data

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.78%. Comparing base (baf5ca8) to head (f9910b0).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff            @@
##           master      #77    +/-   ##
========================================
  Coverage   99.78%   99.78%            
========================================
  Files          45       45            
  Lines        8394     8534   +140     
  Branches      488      497     +9     
========================================
+ Hits         8376     8516   +140     
  Misses         13       13            
  Partials        5        5            

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@qodo-code-review

qodo-code-review Bot commented Jul 21, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 37 rules
✅ REVIEW.md

Grey Divider


Remediation recommended

1. Missing notes for localization_table ✓ Resolved 📘 Rule violation ⌂ Architecture ⭐ New
Description
The PR introduces a cached, validated localization reference-data subsystem (localization_table()
+ packaged localization.json), which is a non-obvious architectural decision. There is no
corresponding entry in docs/notes.md documenting the decision, rationale, trade-offs, and
operating boundaries as required.
Code

weather_briefing/reference_data.py[R134-169]

+@cache
+def localization_table(name: str) -> Mapping[str, Mapping[str, str]]:
+    """Return one fully validated localized scaffold table."""
+    tables = reference_value("localization.json", "tables")
+    aliases = reference_value("localization.json", "aliases")
+    if (
+        not isinstance(tables, dict)
+        or set(tables) != set(_LOCALIZATION_FIELDS)
+        or not isinstance(aliases, dict)
+        or not set(aliases).issubset(_LOCALIZATION_FIELDS)
+    ):
+        raise ReferenceDataError("Localization data must contain every supported table")
+    table = tables.get(name)
+    if name not in _LOCALIZATION_FIELDS or not isinstance(table, dict):
+        raise ReferenceDataError(f"Unknown localization table: {name}")
+    if set(table) != _LOCALIZATION_LANGUAGES:
+        raise ReferenceDataError(f"Localization table must contain every supported language: {name}")
+    expected_fields = _LOCALIZATION_FIELDS[name]
+    validated: dict[str, Mapping[str, str]] = {}
+    for language, labels in table.items():
+        if not _is_localization_labels(labels, expected_fields):
+            raise ReferenceDataError(f"Invalid localization fields: {name}:{language}")
+        validated[language] = MappingProxyType(dict(labels))
+    table_aliases = aliases.get(name, {})
+    if not isinstance(table_aliases, dict):
+        raise ReferenceDataError(f"Localization aliases must be an object: {name}")
+    for alias, target in table_aliases.items():
+        if (
+            not _is_normalized_language(alias)
+            or not isinstance(target, str)
+            or target not in validated
+            or alias in validated
+        ):
+            raise ReferenceDataError(f"Invalid localization alias: {name}:{alias}")
+        validated[alias] = validated[target]
+    return MappingProxyType(validated)
Relevance

⭐⭐⭐ High

Team frequently adds/updates docs/notes.md for non-obvious design changes (e.g., #37, #58, #63).

PR-#37
PR-#58
PR-#63

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2141673 requires documenting non-obvious architectural decisions in
docs/notes.md. The change adds a cached and validated localization table loader
(localization_table()), but docs/notes.md contains no corresponding entry describing this new
mechanism and its rationale/boundaries.

Rule 2141673: Document non-obvious architectural decisions in docs/notes.md
weather_briefing/reference_data.py[134-169]
docs/notes.md[1-4]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new non-obvious architectural decision was introduced (validated, cached localization reference-data loader), but `docs/notes.md` was not updated with the decision rationale, trade-offs, and operating boundaries.

## Issue Context
This change adds `localization_table()` (cached) backed by packaged `localization.json`, including strict validation and language aliasing. Per compliance, non-obvious architecture choices must be captured in `docs/notes.md` so future maintainers understand why this approach was chosen and when it should be revisited.

## Fix Focus Areas
- docs/notes.md[1-20]
- weather_briefing/reference_data.py[134-169]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Whitespace values pass validation ✓ Resolved 🐞 Bug ≡ Correctness
Description
localization_table() validates translation values with truthiness (value) rather than
value.strip(), so whitespace-only strings pass validation and can render as blank
labels/separators. This contradicts the repo’s existing “non-empty string” convention used elsewhere
in reference-data validation.
Code

weather_briefing/reference_data.py[R143-147]

+    for language, labels in table.items():
+        if not _is_normalized_language(language) or not isinstance(labels, dict):
+            raise ReferenceDataError(f"Invalid localization language entry: {name}:{language}")
+        if set(labels) != expected_fields or not all(isinstance(value, str) and value for value in labels.values()):
+            raise ReferenceDataError(f"Invalid localization fields: {name}:{language}")
Relevance

⭐⭐⭐ High

Repo frequently enforces non-empty strings via .strip() checks; similar strict validation accepted
in PRs.

PR-#33
PR-#9
PR-#46

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The current validation checks value (truthiness) which treats whitespace-only strings as valid,
while existing reference-data string validation uses .strip() and tests explicitly cover
whitespace-only rejection.

weather_briefing/reference_data.py[143-147]
weather_briefing/reference_data.py[106-112]
tests/test_reference_data.py[66-72]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`localization_table()` considers any truthy string value valid, so values like `'   '` pass validation. That can produce blank UI/output text while still being considered “valid reference data”.

## Issue Context
Other reference-data helpers (e.g., `reference_string`) reject whitespace-only strings via `.strip()`, so localization validation should align with that behavior.

## Fix Focus Areas
- weather_briefing/reference_data.py[143-147]
- weather_briefing/reference_data.py[106-112]
- tests/test_reference_data.py[66-72]

## Implementation notes
- Change the validation to require `isinstance(value, str) and value.strip()`.
- Add/extend a test that mutates one localization field to `'   '` and asserts `localization_table(...)` raises `ReferenceDataError` with the expected message.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Mutable cached localization data ✓ Resolved 🐞 Bug ☼ Reliability
Description
localization_table() returns a cached mutable nested dict, and alias entries reuse the same inner
dict object as the target language. Any accidental mutation of returned mappings will persist for
the cache/process lifetime and can affect unrelated rendering paths.
Code

weather_briefing/reference_data.py[R124-161]

+@cache
+def localization_table(name: str) -> dict[str, dict[str, str]]:
+    """Return one fully validated localized scaffold table."""
+    tables = reference_value("localization.json", "tables")
+    aliases = reference_value("localization.json", "aliases")
+    if (
+        not isinstance(tables, dict)
+        or set(tables) != set(_LOCALIZATION_FIELDS)
+        or not isinstance(aliases, dict)
+        or not set(aliases).issubset(_LOCALIZATION_FIELDS)
+    ):
+        raise ReferenceDataError("Localization data must contain every supported table")
+    table = tables.get(name)
+    if name not in _LOCALIZATION_FIELDS or not isinstance(table, dict):
+        raise ReferenceDataError(f"Unknown localization table: {name}")
+    if set(table) != _LOCALIZATION_LANGUAGES:
+        raise ReferenceDataError(f"Localization table must contain every supported language: {name}")
+    expected_fields = _LOCALIZATION_FIELDS[name]
+    validated: dict[str, dict[str, str]] = {}
+    for language, labels in table.items():
+        if not _is_normalized_language(language) or not isinstance(labels, dict):
+            raise ReferenceDataError(f"Invalid localization language entry: {name}:{language}")
+        if set(labels) != expected_fields or not all(isinstance(value, str) and value for value in labels.values()):
+            raise ReferenceDataError(f"Invalid localization fields: {name}:{language}")
+        validated[language] = dict(labels)
+    table_aliases = aliases.get(name, {})
+    if not isinstance(table_aliases, dict):
+        raise ReferenceDataError(f"Localization aliases must be an object: {name}")
+    for alias, target in table_aliases.items():
+        if (
+            not _is_normalized_language(alias)
+            or not isinstance(target, str)
+            or target not in validated
+            or alias in validated
+        ):
+            raise ReferenceDataError(f"Invalid localization alias: {name}:{alias}")
+        validated[alias] = validated[target]
+    return validated
Relevance

⭐⭐ Medium

No prior review evidence about avoiding mutable @cache dicts or aliasing inner dicts; unclear team
stance.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The function is explicitly cached and returns the mutable validated dict; aliases are assigned to
the exact same inner dict object as their target, increasing the blast radius if any mutation
occurs.

weather_briefing/reference_data.py[124-161]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`localization_table()` is `@cache`d but returns a mutable dict structure directly; additionally, aliases are implemented by pointing `validated[alias]` at `validated[target]` (same inner dict). If any caller mutates the returned dict (outer or inner), the mutation becomes global and persistent.

## Issue Context
While current call sites appear read-only, returning cached mutable state is a foot-gun for future edits/tests and can cause hard-to-debug cross-request contamination.

## Fix Focus Areas
- weather_briefing/reference_data.py[124-161]

## Implementation notes
Choose one:
1) Make the returned structures immutable:
  - Build inner dicts, then wrap them with `types.MappingProxyType`.
  - Wrap the outer dict as well.
  - Update the return type annotation to `Mapping[str, Mapping[str, str]]` (or similar).

2) If keeping `dict` return type, defensively copy on return:
  - Keep the cached value internal (immutable or private), and return a deep copy to callers.
  - Also ensure aliases do not share the same mutable inner dict (e.g., `validated[alias] = dict(validated[target])`).

Add a regression test that mutates the returned mapping and verifies a subsequent `localization_table(...)` call still returns the original values.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit f9910b0

Results up to commit 0ddd323 ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Remediation recommended
1. Whitespace values pass validation ✓ Resolved 🐞 Bug ≡ Correctness
Description
localization_table() validates translation values with truthiness (value) rather than
value.strip(), so whitespace-only strings pass validation and can render as blank
labels/separators. This contradicts the repo’s existing “non-empty string” convention used elsewhere
in reference-data validation.
Code

weather_briefing/reference_data.py[R143-147]

+    for language, labels in table.items():
+        if not _is_normalized_language(language) or not isinstance(labels, dict):
+            raise ReferenceDataError(f"Invalid localization language entry: {name}:{language}")
+        if set(labels) != expected_fields or not all(isinstance(value, str) and value for value in labels.values()):
+            raise ReferenceDataError(f"Invalid localization fields: {name}:{language}")
Relevance

⭐⭐⭐ High

Repo frequently enforces non-empty strings via .strip() checks; similar strict validation accepted
in PRs.

PR-#33
PR-#9
PR-#46

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The current validation checks value (truthiness) which treats whitespace-only strings as valid,
while existing reference-data string validation uses .strip() and tests explicitly cover
whitespace-only rejection.

weather_briefing/reference_data.py[143-147]
weather_briefing/reference_data.py[106-112]
tests/test_reference_data.py[66-72]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`localization_table()` considers any truthy string value valid, so values like `'   '` pass validation. That can produce blank UI/output text while still being considered “valid reference data”.

## Issue Context
Other reference-data helpers (e.g., `reference_string`) reject whitespace-only strings via `.strip()`, so localization validation should align with that behavior.

## Fix Focus Areas
- weather_briefing/reference_data.py[143-147]
- weather_briefing/reference_data.py[106-112]
- tests/test_reference_data.py[66-72]

## Implementation notes
- Change the validation to require `isinstance(value, str) and value.strip()`.
- Add/extend a test that mutates one localization field to `'   '` and asserts `localization_table(...)` raises `ReferenceDataError` with the expected message.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Mutable cached localization data ✓ Resolved 🐞 Bug ☼ Reliability
Description
localization_table() returns a cached mutable nested dict, and alias entries reuse the same inner
dict object as the target language. Any accidental mutation of returned mappings will persist for
the cache/process lifetime and can affect unrelated rendering paths.
Code

weather_briefing/reference_data.py[R124-161]

+@cache
+def localization_table(name: str) -> dict[str, dict[str, str]]:
+    """Return one fully validated localized scaffold table."""
+    tables = reference_value("localization.json", "tables")
+    aliases = reference_value("localization.json", "aliases")
+    if (
+        not isinstance(tables, dict)
+        or set(tables) != set(_LOCALIZATION_FIELDS)
+        or not isinstance(aliases, dict)
+        or not set(aliases).issubset(_LOCALIZATION_FIELDS)
+    ):
+        raise ReferenceDataError("Localization data must contain every supported table")
+    table = tables.get(name)
+    if name not in _LOCALIZATION_FIELDS or not isinstance(table, dict):
+        raise ReferenceDataError(f"Unknown localization table: {name}")
+    if set(table) != _LOCALIZATION_LANGUAGES:
+        raise ReferenceDataError(f"Localization table must contain every supported language: {name}")
+    expected_fields = _LOCALIZATION_FIELDS[name]
+    validated: dict[str, dict[str, str]] = {}
+    for language, labels in table.items():
+        if not _is_normalized_language(language) or not isinstance(labels, dict):
+            raise ReferenceDataError(f"Invalid localization language entry: {name}:{language}")
+        if set(labels) != expected_fields or not all(isinstance(value, str) and value for value in labels.values()):
+            raise ReferenceDataError(f"Invalid localization fields: {name}:{language}")
+        validated[language] = dict(labels)
+    table_aliases = aliases.get(name, {})
+    if not isinstance(table_aliases, dict):
+        raise ReferenceDataError(f"Localization aliases must be an object: {name}")
+    for alias, target in table_aliases.items():
+        if (
+            not _is_normalized_language(alias)
+            or not isinstance(target, str)
+            or target not in validated
+            or alias in validated
+        ):
+            raise ReferenceDataError(f"Invalid localization alias: {name}:{alias}")
+        validated[alias] = validated[target]
+    return validated
Relevance

⭐⭐ Medium

No prior review evidence about avoiding mutable @cache dicts or aliasing inner dicts; unclear team
stance.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The function is explicitly cached and returns the mutable validated dict; aliases are assigned to
the exact same inner dict object as their target, increasing the blast radius if any mutation
occurs.

weather_briefing/reference_data.py[124-161]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`localization_table()` is `@cache`d but returns a mutable dict structure directly; additionally, aliases are implemented by pointing `validated[alias]` at `validated[target]` (same inner dict). If any caller mutates the returned dict (outer or inner), the mutation becomes global and persistent.

## Issue Context
While current call sites appear read-only, returning cached mutable state is a foot-gun for future edits/tests and can cause hard-to-debug cross-request contamination.

## Fix Focus Areas
- weather_briefing/reference_data.py[124-161]

## Implementation notes
Choose one:
1) Make the returned structures immutable:
  - Build inner dicts, then wrap them with `types.MappingProxyType`.
  - Wrap the outer dict as well.
  - Update the return type annotation to `Mapping[str, Mapping[str, str]]` (or similar).

2) If keeping `dict` return type, defensively copy on return:
  - Keep the cached value internal (immutable or private), and return a deep copy to callers.
  - Also ensure aliases do not share the same mutable inner dict (e.g., `validated[alias] = dict(validated[target])`).

Add a regression test that mutates the returned mapping and verifies a subsequent `localization_table(...)` call still returns the original values.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

Comment thread weather_briefing/reference_data.py
Comment thread weather_briefing/reference_data.py
@IceCodeNew
IceCodeNew force-pushed the codex/externalize-localization-data branch from 0ddd323 to 8af6316 Compare July 21, 2026 02:24
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

Comment thread weather_briefing/reference_data.py
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 8af6316

@IceCodeNew
IceCodeNew force-pushed the codex/externalize-localization-data branch from 8af6316 to 6fcb49d Compare July 21, 2026 02:32
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 6fcb49d

@IceCodeNew
IceCodeNew force-pushed the codex/externalize-localization-data branch from 6fcb49d to f9910b0 Compare July 21, 2026 02:39
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit f9910b0

@IceCodeNew
IceCodeNew marked this pull request as ready for review July 21, 2026 02:45
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Refactor: externalize localization scaffolds into packaged localization.json

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Move renderer/adapter localization scaffolds into packaged
 weather_briefing/data/localization.json.
• Add cached localization_table() loader with strict schema + alias validation and immutable
 results.
• Switch adapters/renderers to consume validated tables and expand reference-data test coverage.
Diagram

graph TD
  A[("localization.json")] --> B["reference_data.localization_table()"]
  B --> C["air_quality.py"]
  B --> D["allergen.py"]
  B --> E["weather_context.py"]
  B --> F["render.py"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Adopt gettext/Babel-style catalogs (e.g., .po/.mo)
  • ➕ Standard tooling for translators (diff/merge, pluralization, extraction).
  • ➕ Clear separation of message IDs vs rendered strings.
  • ➖ Heavier dependency/toolchain for a small, fixed set of scaffold strings.
  • ➖ Would require refactoring call sites to use message IDs and a translation runtime.
2. Schema validation via JSON Schema / Pydantic models
  • ➕ Smaller diffs and narrower blast radius per table edit.
  • ➕ Potentially simpler caching/partial reload semantics later.
  • ➖ More packaging entries/files to manage and validate.
  • ➖ Harder to enforce cross-table invariants (supported language set, alias policies) consistently.

Recommendation: The current approach (single packaged JSON + strict startup validation + immutable cached tables) fits the stated boundaries: small, app-shipped scaffolds with intentional fail-fast behavior and restart-required updates. If localization scope expands to runtime-installed language packs or significantly more languages/tables, revisit toward a versioned resource layer (potentially using standard i18n tooling) and consider declarative schema validation to reduce Python-side maintenance.

Files changed (8) +522 / -249

Enhancement (1) +110 / -1
reference_data.pyImplement cached, validated localization_table() loader returning immutable mappings +110/-1

Implement cached, validated localization_table() loader returning immutable mappings

• Adds table schemas (required fields per table) and a fixed supported-language set. Implements 'localization_table()' which validates table presence, language completeness, exact required keys, non-blank strings, normalized alias tags, and alias targets, returning 'MappingProxyType' wrappers for immutability.

weather_briefing/reference_data.py

Refactor (5) +248 / -248
air_quality.pyReplace in-module air-quality translation constants with localization_table() +2/-63

Replace in-module air-quality translation constants with localization_table()

• Removes the hard-coded '_AIR_QUALITY_FORMATS' dict and loads the same labels from 'localization_table('air_quality')'. Keeps existing formatting behavior but centralizes translations in packaged reference data.

weather_briefing/air_quality.py

allergen.pyReplace in-module allergen translation constants with localization_table() +2/-43

Replace in-module allergen translation constants with localization_table()

• Removes the hard-coded '_ALLERGEN_FORMATS' dict and loads labels from 'localization_table('allergen')'. This aligns allergen scaffolding with the new externalized localization reference data.

weather_briefing/allergen.py

localization.jsonAdd packaged localization tables and language aliases +233/-0

Add packaged localization tables and language aliases

• Introduces a new reference-data file containing translation tables for air quality, allergens, briefing renderer labels, QWeather templates, and weather-document labels. Defines language aliases for the briefing table (e.g., zh/zh-Hans/zh-Hant) that map to supported base tags.

weather_briefing/data/localization.json

render.pyLoad briefing renderer labels from localization_table and widen type hints +8/-60

Load briefing renderer labels from localization_table and widen type hints

• Replaces embedded briefing label dictionaries with 'localization_table('briefing')', preserving language matching via 'LanguageSupport'. Updates helper function signatures to accept 'Mapping[str, str]' to reflect read-only mappings.

weather_briefing/render.py

weather_context.pyExternalize weather-document labels and QWeather templates via localization_table() +3/-82

Externalize weather-document labels and QWeather templates via localization_table()

• Removes hard-coded '_WEATHER_DOCUMENT_LABELS' and '_QWEATHER_FORMATS' dictionaries and replaces them with validated tables loaded from 'localization_table('weather_document')' and 'localization_table('qweather')'. Keeps existing language support configuration while centralizing templates in reference data.

weather_briefing/weather_context.py

Tests (1) +160 / -0
test_reference_data.pyAdd comprehensive tests for localization_table validation and immutability +160/-0

Add comprehensive tests for localization_table validation and immutability

• Introduces helpers to mutate a deep-copied localization payload and adds many negative-path tests covering missing tables/languages/fields, whitespace values, invalid alias roots and targets, and non-object structures. Verifies returned tables are 'MappingProxyType' and alias lookups resolve to immutable mappings.

tests/test_reference_data.py

Documentation (1) +4 / -0
notes.mdDocument localization reference-data lifecycle and failure boundaries +4/-0

Document localization reference-data lifecycle and failure boundaries

• Adds an architecture note explaining why localization scaffolds are treated as packaged reference data. Clarifies strict startup validation, immutability/caching, restart requirements, and when a more dynamic localization layer would be warranted.

docs/notes.md

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit f9910b0

@IceCodeNew
IceCodeNew merged commit 671e069 into master Jul 21, 2026
18 checks passed
@IceCodeNew
IceCodeNew deleted the codex/externalize-localization-data branch July 21, 2026 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant