[06/10] refactor: centralize packaged reference data

[IceCodeNew]

Summary

• add validated packaged-resource loading under weather_briefing.data
• separate localization validation from the legacy aggregate module
• migrate air-quality, allergen, and content-cleaning consumers
• isolate cached JSON values from caller mutation
• copy only selected values for nested reference lookups
• construct immutable string tuples without copying cached lists

Scope

Independently based on master. Merge as step 06; this layer is the resource-data prerequisite for #95.

Verification

• prek run --all-files
• 895 tests passed
• line coverage: 99.85% (master: 99.85%)
• branch coverage: 99.55% (master: 99.55%)

[coderabbitai]

Warning

Review limit reached

@IceCodeNew, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7c5c7b80-6b20-42d3-8dcd-63bcab3a6c3b

📥 Commits

Reviewing files that changed from the base of the PR and between 49f58fb and 9844c5b.

📒 Files selected for processing (9)

• tests/test_air_quality.py
• tests/test_allergen.py
• tests/test_reference_data.py
• weather_briefing/air_quality.py
• weather_briefing/allergen.py
• weather_briefing/content_cleaners.py
• weather_briefing/data/resources.py
• weather_briefing/localization.py
• weather_briefing/reference_data.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/weather-refactor-02-reference-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

• X
• Mastodon
• Reddit
• LinkedIn

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

[IceCodeNew]

/agentic_review

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.81%. Comparing base (49f58fb) to head (9844c5b).
⚠️ Report is 2 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master      #94   +/-   ##
=======================================
  Coverage   99.81%   99.81%           
=======================================
  Files          62       70    +8     
  Lines        9230     9323   +93     
  Branches      553      553           
=======================================
+ Hits         9213     9306   +93     
  Misses         12       12           
  Partials        5        5           

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

[qodo-code-review]

Code Review by Qodo

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

Context used

✅ Compliance rules (platform): 46 rules

1. deepcopy patched at import ✓ Resolved 📘 Rule violation ▣ Testability

Description

The new test monkeypatches deepcopy via weather_briefing.data.resources.deepcopy (an import-site
binding) instead of patching the defining symbol copy.deepcopy, which violates the patch-targeting
rule and makes patches more fragile to refactors.

Code

tests/test_reference_data.py[R283-292]

+def test_reference_string_tuple_does_not_copy_the_cached_list(monkeypatch) -> None:
+    from unittest.mock import Mock
+
+    import weather_briefing.data.resources as resources_module
+
+    cached = ["one", "two"]
+    monkeypatch.setattr(resources_module, "_load_reference_data", lambda filename: {"selected": cached})
+    unexpected_deepcopy = Mock(side_effect=AssertionError("reference_string_tuple must not copy the cached list"))
+    monkeypatch.setattr(resources_module, "deepcopy", unexpected_deepcopy)
+

Relevance

⭐⭐⭐ High

They accept test robustness/isolation tweaks; patch-target fix is small and reduces fragility.

PR-#11
PR-#92

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2274647 requires patching/mocking at the behavior’s defining module. The new test
explicitly patches resources_module.deepcopy, which is an import-site binding, and
weather_briefing.data.resources shows deepcopy is imported into that module namespace and used
from there.

Rule 2274647: Patch behavior at its defining module, not where it is imported
tests/test_reference_data.py[283-294]
weather_briefing/data/resources.py[5-7]
weather_briefing/data/resources.py[54-56]

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 test patches `deepcopy` at the import site (`weather_briefing.data.resources.deepcopy`) rather than at its defining module (`copy.deepcopy`), which violates the compliance requirement.

## Issue Context
`weather_briefing.data.resources` currently binds `deepcopy` via `from copy import deepcopy`, so tests patch the import-site name. To comply with the rule, prefer referencing `copy.deepcopy` in production code (and patching `copy.deepcopy` in tests).

## Fix Focus Areas
- tests/test_reference_data.py[283-294]
- weather_briefing/data/resources.py[5-7]
- weather_briefing/data/resources.py[54-56]

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

---

2. String tuple deepcopies list ✓ Resolved 🐞 Bug ➹ Performance

Description

reference_string_tuple() deep-copies the underlying list via reference_value() and then converts it
to a tuple, even though returning a tuple already prevents caller mutation. This introduces
avoidable per-call overhead in code paths that call reference_string_tuple() frequently (e.g.,
geocoding name normalization).

Code

weather_briefing/data/resources.py[R64-70]

+def reference_string_tuple(filename: str, *path: str) -> tuple[str, ...]:
+    """Read a non-empty string sequence from packaged reference data."""
+    value = reference_value(filename, *path)
+    if not isinstance(value, list) or not value or not all(isinstance(item, str) and item.strip() for item in value):
+        joined_path = ".".join(path)
+        raise ReferenceDataError(f"Reference data field must be a non-empty string list: {filename}:{joined_path}")
+    return tuple(value)

Relevance

⭐⭐⭐ High

Repo often accepts low-risk performance/behavior improvements around caching and resource access;
skipping deepcopy here is safe.

PR-#86
PR-#88
PR-#92

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

reference_value() returns deepcopy(value), so reference_string_tuple() currently deep-copies
the list on every call; geocoding normalization calls reference_string_tuple() directly in a
function that is not cached per input name.

weather_briefing/data/resources.py[42-53]
weather_briefing/data/resources.py[64-70]
weather_briefing/geocoding/matching.py[108-113]

Agent prompt

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

## Issue description
`reference_string_tuple()` currently calls `reference_value()`, which always does `deepcopy(value)`. For a validated `list[str]`, the subsequent `tuple(value)` already ensures the returned value is immutable, so the deepcopy is redundant work.

## Issue Context
This function is used in non-cached paths such as geocoding name normalization, so the extra allocations can recur per request.

## Fix Focus Areas
- weather_briefing/data/resources.py[42-53]
- weather_briefing/data/resources.py[64-70]

## Suggested fix
- Implement `reference_string_tuple()` without calling `reference_value()`:
 - validate filename
 - traverse the cached root returned by `_load_reference_data()` to the target value
 - validate it is a non-empty `list[str]` with non-whitespace items
 - return `tuple(value)` directly (no deepcopy)
- Alternatively, add a specialized helper in this module for reading a `list[str]` as an immutable tuple without copying the intermediate list.
This preserves the “callers can’t mutate cached data” guarantee because callers never receive the original list.

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

---

3. Cached mutable data ✓ Resolved 🐞 Bug ☼ Reliability

Description

weather_briefing.data.resources.load_reference_data() is @cache'd but returns the mutable dict
produced by json.loads, so any caller mutation (including nested lists/dicts reached via
reference_value()) will persist across the process and affect later lookups. This can create
order-dependent behavior and cross-test contamination now that this loader is the central
reference-data access point.

Code

weather_briefing/data/resources.py[R18-30]

+@cache
+def load_reference_data(filename: str) -> dict[str, object]:
+    """Load and validate one packaged JSON reference-data object."""
+    if PurePath(filename).name != filename or not filename.endswith(".json"):
+        raise ReferenceDataError("Reference data filename must identify one JSON file")
+    try:
+        text = resources.files(data).joinpath(filename).read_text(encoding="utf-8")
+        value = json.loads(text)
+    except (FileNotFoundError, OSError, json.JSONDecodeError) as exc:
+        raise ReferenceDataError(f"Unable to load reference data: {filename}") from exc
+    if not isinstance(value, dict):
+        raise ReferenceDataError(f"Reference data root must be an object: {filename}")
+    return value

Relevance

⭐⭐⭐ High

Team previously accepted cache/test-isolation fixes; returning mutable cached JSON risks cross-test
contamination and order-dependence.

PR-#86

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The loader is cached and returns the raw mutable JSON object, so subsequent calls return the same
mutable object instance and can reflect unintended mutations.

weather_briefing/data/resources.py[18-30]
weather_briefing/data/resources.py[33-42]

Agent prompt

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

## Issue description
`load_reference_data()` is decorated with `@cache` but returns a mutable `dict` (and nested mutable structures) from `json.loads()`. Because the cached object is returned by reference, any mutation by a caller will persist and affect subsequent reads.

## Issue Context
Reference data is intended to be treated as read-only domain constants; caching should not allow accidental in-process corruption.

## Fix Focus Areas
- weather_briefing/data/resources.py[18-30]

### Suggested implementation direction
- Introduce a private cached function that returns the parsed JSON object (or a deep-frozen version).
- Make the public `load_reference_data()` return an immutable view (e.g., recursively convert dict->`MappingProxyType` and list->tuple), **or** return a `copy.deepcopy()` of the cached object so callers can’t mutate shared state.
- Adjust the return type annotation accordingly (e.g., `Mapping[str, object]` if returning a mapping proxy).

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

---

View more (1)

4. Reference lookups copy whole file ✓ Resolved 🐞 Bug ➹ Performance

Description

weather_briefing.data.resources.load_reference_data() deep-copies the entire cached JSON document on
every call, and reference_value() calls load_reference_data() for each nested lookup, causing
repeated full-object copies even when only reading a small field. This creates unnecessary
CPU/allocation overhead for call sites that perform many reference_value/reference_string_tuple
reads during request processing (e.g., geocoding name normalization helpers).

Code

weather_briefing/data/resources.py[R19-23]

+def load_reference_data(filename: str) -> dict[str, object]:
+    """Load and validate one packaged JSON reference-data object."""
+    if PurePath(filename).name != filename or not filename.endswith(".json"):
+        raise ReferenceDataError("Reference data filename must identify one JSON file")
+    return deepcopy(_load_reference_data(filename))

Relevance

⭐⭐ Medium

Deepcopy seems intentional for immutability; optimizing it is non-trivial despite team accepting
performance/runtime fixes elsewhere.

PR-#86
PR-#92

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The deep-copy happens on every call to load_reference_data(), and reference_value() always calls it
before traversing a nested path; geocoding imports reference_value/reference_string_tuple via
reference_data and calls them from non-cached helpers, making repeated full-document copies likely
during geocoding operations.

weather_briefing/data/resources.py[19-47]
weather_briefing/reference_data.py[11-18]
weather_briefing/geocoding.py[20-21]
weather_briefing/geocoding.py[582-598]

Agent prompt

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

## Issue description
`load_reference_data()` returns `deepcopy(_load_reference_data(...))`, so every call copies the entire parsed JSON object graph. Because `reference_value()` calls `load_reference_data()` for each lookup, callers that repeatedly read small fields pay an O(size_of_file) copy cost per read.

## Issue Context
Parsing/I/O is cached in `_load_reference_data()`, but the deep-copy is outside the cache and is repeated. `weather_briefing.reference_data` re-exports these functions, so existing call sites (e.g. geocoding) are affected.

## Fix Focus Areas
- weather_briefing/data/resources.py[19-47]

### Concrete fix
- Keep `load_reference_data()` as the “return full independent dict” API (it can still deep-copy the root).
- Change `reference_value()` to:
 1) validate `filename` (share a small helper with `load_reference_data()`),
 2) traverse the cached root returned by `_load_reference_data(filename)` (no deep copy),
 3) `return deepcopy(value)` **only for the selected value** (so caller mutation remains isolated without copying the whole document).
- Optionally, for `reference_string()` you can skip `deepcopy()` since strings are immutable (but using `reference_value()` + type check is fine).

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

---

Previous review results

Review updated until commit 9844c5b ⚖️ Balanced

Results up to commit f1223f9 ⚖️ Balanced

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

1. Cached mutable data ✓ Resolved 🐞 Bug ☼ Reliability

Description

weather_briefing.data.resources.load_reference_data() is @cache'd but returns the mutable dict
produced by json.loads, so any caller mutation (including nested lists/dicts reached via
reference_value()) will persist across the process and affect later lookups. This can create
order-dependent behavior and cross-test contamination now that this loader is the central
reference-data access point.

Code

weather_briefing/data/resources.py[R18-30]

+@cache
+def load_reference_data(filename: str) -> dict[str, object]:
+    """Load and validate one packaged JSON reference-data object."""
+    if PurePath(filename).name != filename or not filename.endswith(".json"):
+        raise ReferenceDataError("Reference data filename must identify one JSON file")
+    try:
+        text = resources.files(data).joinpath(filename).read_text(encoding="utf-8")
+        value = json.loads(text)
+    except (FileNotFoundError, OSError, json.JSONDecodeError) as exc:
+        raise ReferenceDataError(f"Unable to load reference data: {filename}") from exc
+    if not isinstance(value, dict):
+        raise ReferenceDataError(f"Reference data root must be an object: {filename}")
+    return value

Relevance

⭐⭐⭐ High

Team previously accepted cache/test-isolation fixes; returning mutable cached JSON risks cross-test
contamination and order-dependence.

PR-#86

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The loader is cached and returns the raw mutable JSON object, so subsequent calls return the same
mutable object instance and can reflect unintended mutations.

weather_briefing/data/resources.py[18-30]
weather_briefing/data/resources.py[33-42]

Agent prompt

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

## Issue description
`load_reference_data()` is decorated with `@cache` but returns a mutable `dict` (and nested mutable structures) from `json.loads()`. Because the cached object is returned by reference, any mutation by a caller will persist and affect subsequent reads.

## Issue Context
Reference data is intended to be treated as read-only domain constants; caching should not allow accidental in-process corruption.

## Fix Focus Areas
- weather_briefing/data/resources.py[18-30]

### Suggested implementation direction
- Introduce a private cached function that returns the parsed JSON object (or a deep-frozen version).
- Make the public `load_reference_data()` return an immutable view (e.g., recursively convert dict->`MappingProxyType` and list->tuple), **or** return a `copy.deepcopy()` of the cached object so callers can’t mutate shared state.
- Adjust the return type annotation accordingly (e.g., `Mapping[str, object]` if returning a mapping proxy).

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

---

Results up to commit 4ef5927 ⚖️ Balanced

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

1. Reference lookups copy whole file ✓ Resolved 🐞 Bug ➹ Performance

Description

weather_briefing.data.resources.load_reference_data() deep-copies the entire cached JSON document on
every call, and reference_value() calls load_reference_data() for each nested lookup, causing
repeated full-object copies even when only reading a small field. This creates unnecessary
CPU/allocation overhead for call sites that perform many reference_value/reference_string_tuple
reads during request processing (e.g., geocoding name normalization helpers).

Code

weather_briefing/data/resources.py[R19-23]

+def load_reference_data(filename: str) -> dict[str, object]:
+    """Load and validate one packaged JSON reference-data object."""
+    if PurePath(filename).name != filename or not filename.endswith(".json"):
+        raise ReferenceDataError("Reference data filename must identify one JSON file")
+    return deepcopy(_load_reference_data(filename))

Relevance

⭐⭐ Medium

Deepcopy seems intentional for immutability; optimizing it is non-trivial despite team accepting
performance/runtime fixes elsewhere.

PR-#86
PR-#92

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The deep-copy happens on every call to load_reference_data(), and reference_value() always calls it
before traversing a nested path; geocoding imports reference_value/reference_string_tuple via
reference_data and calls them from non-cached helpers, making repeated full-document copies likely
during geocoding operations.

weather_briefing/data/resources.py[19-47]
weather_briefing/reference_data.py[11-18]
weather_briefing/geocoding.py[20-21]
weather_briefing/geocoding.py[582-598]

Agent prompt

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

## Issue description
`load_reference_data()` returns `deepcopy(_load_reference_data(...))`, so every call copies the entire parsed JSON object graph. Because `reference_value()` calls `load_reference_data()` for each lookup, callers that repeatedly read small fields pay an O(size_of_file) copy cost per read.

## Issue Context
Parsing/I/O is cached in `_load_reference_data()`, but the deep-copy is outside the cache and is repeated. `weather_briefing.reference_data` re-exports these functions, so existing call sites (e.g. geocoding) are affected.

## Fix Focus Areas
- weather_briefing/data/resources.py[19-47]

### Concrete fix
- Keep `load_reference_data()` as the “return full independent dict” API (it can still deep-copy the root).
- Change `reference_value()` to:
 1) validate `filename` (share a small helper with `load_reference_data()`),
 2) traverse the cached root returned by `_load_reference_data(filename)` (no deep copy),
 3) `return deepcopy(value)` **only for the selected value** (so caller mutation remains isolated without copying the whole document).
- Optionally, for `reference_string()` you can skip `deepcopy()` since strings are immutable (but using `reference_value()` + type check is fine).

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

---

Results up to commit f31b80d ⚖️ Balanced

No changes from previous review

Results up to commit 4f4fb4d ⚖️ Balanced

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

1. String tuple deepcopies list ✓ Resolved 🐞 Bug ➹ Performance

Description

reference_string_tuple() deep-copies the underlying list via reference_value() and then converts it
to a tuple, even though returning a tuple already prevents caller mutation. This introduces
avoidable per-call overhead in code paths that call reference_string_tuple() frequently (e.g.,
geocoding name normalization).

Code

weather_briefing/data/resources.py[R64-70]

+def reference_string_tuple(filename: str, *path: str) -> tuple[str, ...]:
+    """Read a non-empty string sequence from packaged reference data."""
+    value = reference_value(filename, *path)
+    if not isinstance(value, list) or not value or not all(isinstance(item, str) and item.strip() for item in value):
+        joined_path = ".".join(path)
+        raise ReferenceDataError(f"Reference data field must be a non-empty string list: {filename}:{joined_path}")
+    return tuple(value)

Relevance

⭐⭐⭐ High

Repo often accepts low-risk performance/behavior improvements around caching and resource access;
skipping deepcopy here is safe.

PR-#86
PR-#88
PR-#92

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

reference_value() returns deepcopy(value), so reference_string_tuple() currently deep-copies
the list on every call; geocoding normalization calls reference_string_tuple() directly in a
function that is not cached per input name.

weather_briefing/data/resources.py[42-53]
weather_briefing/data/resources.py[64-70]
weather_briefing/geocoding/matching.py[108-113]

Agent prompt

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

## Issue description
`reference_string_tuple()` currently calls `reference_value()`, which always does `deepcopy(value)`. For a validated `list[str]`, the subsequent `tuple(value)` already ensures the returned value is immutable, so the deepcopy is redundant work.

## Issue Context
This function is used in non-cached paths such as geocoding name normalization, so the extra allocations can recur per request.

## Fix Focus Areas
- weather_briefing/data/resources.py[42-53]
- weather_briefing/data/resources.py[64-70]

## Suggested fix
- Implement `reference_string_tuple()` without calling `reference_value()`:
 - validate filename
 - traverse the cached root returned by `_load_reference_data()` to the target value
 - validate it is a non-empty `list[str]` with non-whitespace items
 - return `tuple(value)` directly (no deepcopy)
- Alternatively, add a specialized helper in this module for reading a `list[str]` as an immutable tuple without copying the intermediate list.
This preserves the “callers can’t mutate cached data” guarantee because callers never receive the original list.

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

---

Results up to commit 391070c ⚖️ Balanced

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

1. deepcopy patched at import ✓ Resolved 📘 Rule violation ▣ Testability

Description

The new test monkeypatches deepcopy via weather_briefing.data.resources.deepcopy (an import-site
binding) instead of patching the defining symbol copy.deepcopy, which violates the patch-targeting
rule and makes patches more fragile to refactors.

Code

tests/test_reference_data.py[R283-292]

+def test_reference_string_tuple_does_not_copy_the_cached_list(monkeypatch) -> None:
+    from unittest.mock import Mock
+
+    import weather_briefing.data.resources as resources_module
+
+    cached = ["one", "two"]
+    monkeypatch.setattr(resources_module, "_load_reference_data", lambda filename: {"selected": cached})
+    unexpected_deepcopy = Mock(side_effect=AssertionError("reference_string_tuple must not copy the cached list"))
+    monkeypatch.setattr(resources_module, "deepcopy", unexpected_deepcopy)
+

Relevance

⭐⭐⭐ High

They accept test robustness/isolation tweaks; patch-target fix is small and reduces fragility.

PR-#11
PR-#92

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2274647 requires patching/mocking at the behavior’s defining module. The new test
explicitly patches resources_module.deepcopy, which is an import-site binding, and
weather_briefing.data.resources shows deepcopy is imported into that module namespace and used
from there.

Rule 2274647: Patch behavior at its defining module, not where it is imported
tests/test_reference_data.py[283-294]
weather_briefing/data/resources.py[5-7]
weather_briefing/data/resources.py[54-56]

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 test patches `deepcopy` at the import site (`weather_briefing.data.resources.deepcopy`) rather than at its defining module (`copy.deepcopy`), which violates the compliance requirement.

## Issue Context
`weather_briefing.data.resources` currently binds `deepcopy` via `from copy import deepcopy`, so tests patch the import-site name. To comply with the rule, prefer referencing `copy.deepcopy` in production code (and patching `copy.deepcopy` in tests).

## Fix Focus Areas
- tests/test_reference_data.py[283-294]
- weather_briefing/data/resources.py[5-7]
- weather_briefing/data/resources.py[54-56]

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

---

Results up to commit 9844c5b ⚖️ Balanced

No changes from previous review

[qodo-code-review]

PR Summary by Qodo

Refactor reference data loading into weather_briefing.data.resources

✨ Enhancement 🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Centralize packaged JSON reference-data loading and validation under "weather_briefing.data".
• Split localization-table validation into a dedicated module and update consumers.
• Prevent cached reference-data mutation and add regression tests for copy behavior.

Diagram

graph TD
  AQ["air_quality.py"] --> L10["localization.py"] --> R10["data/resources.py"] --> J10[("packaged JSON")]
  AL["allergen.py"] --> L10 --> R10 --> J10
  CC["content_cleaners.py"] --> R10 --> J10
  RD["reference_data.py"] --> R10 --> J10
  RD --> L10

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Deep-freeze cached roots (recursive immutability)

• ➕ Avoids repeated deepcopy of nested values on each access
• ➕ Stronger guarantees: callers cannot mutate any cached structure
• ➖ More code and higher complexity (recursive conversion, type preservation)
• ➖ Potential perf cost during initial load; more edge cases (lists, custom objects)

2. Schema-based validation (e.g., pydantic/dataclasses per resource)

• ➕ More explicit contracts per JSON file; clearer error reporting
• ➕ Easier evolution for complex resources with nested structures
• ➖ Adds dependencies/boilerplate and increases maintenance surface
• ➖ Overkill for simple key-path lookups and small resource set

3. No caching, always parse JSON per call

• ➕ Simplest semantics; no mutation hazards by construction
• ➕ Avoids cache invalidation considerations
• ➖ Significant perf regression; repeated disk/package reads and JSON parsing

Recommendation: The PR’s approach (cache parsed JSON, but return isolated copies for general lookups and avoid copying for safe tuple construction) is a good balance of simplicity, safety, and performance. The main alternative worth considering—deep-freezing the cached root—would tighten immutability guarantees but adds complexity and is likely unnecessary given the new targeted tests and selective copy strategy.

Files changed (9) +288 / -182

Enhancement (2) +191 / -0

resources.pyAdd validated packaged JSON resource loader with safe cached access +74/-0

Add validated packaged JSON resource loader with safe cached access

• Introduces a new module that validates JSON filenames, caches parsed roots, and provides typed accessors. Ensures callers cannot mutate cached data by deep-copying returned values while avoiding unnecessary copying for string tuple reads.

weather_briefing/data/resources.py

localization.pyAdd dedicated localization table validation and immutability wrapper +117/-0

Add dedicated localization table validation and immutability wrapper

• Extracts localization-table validation into its own cached accessor that validates supported tables/languages/fields and returns immutable MappingProxyType views (including validated language aliases).

weather_briefing/localization.py

Refactor (4) +25 / -162

air_quality.pyUse centralized resources + localization module for air quality formats +2/-1

Use centralized resources + localization module for air quality formats

• Replaces legacy reference_data imports with weather_briefing.data.resources accessors and the new localization_table provider.

weather_briefing/air_quality.py

allergen.pyUse centralized resources + localization module for allergen formats +2/-1

Use centralized resources + localization module for allergen formats

• Updates allergen module to import ReferenceDataError/reference_value from data.resources and localization_table from the new localization module.

weather_briefing/allergen.py

content_cleaners.pyRead content-cleaning selector tuples from data.resources +1/-1

Read content-cleaning selector tuples from data.resources

• Switches reference_string_tuple import to the new centralized resources module.

weather_briefing/content_cleaners.py

reference_data.pyConvert reference_data into compatibility re-export facade +20/-159

Convert reference_data into compatibility re-export facade

• Removes in-module JSON loading and localization validation, delegating to data.resources and localization. Keeps existing public API via explicit __all__ exports while other domain logic (e.g., Telegram classification, Open-Meteo descriptions) remains.

weather_briefing/reference_data.py

Tests (3) +72 / -20

test_air_quality.pyUpdate air-quality tests to new ReferenceDataError import +1/-1

Update air-quality tests to new ReferenceDataError import

• Switches ReferenceDataError import to weather_briefing.data.resources to match the new centralized reference data module.

tests/test_air_quality.py

test_allergen.pyUpdate allergen tests to new ReferenceDataError import +1/-1

Update allergen tests to new ReferenceDataError import

• Migrates ReferenceDataError import from the legacy reference_data module to weather_briefing.data.resources.

tests/test_allergen.py

test_reference_data.pyAlign reference-data tests to new modules and add cache isolation checks +70/-18

Align reference-data tests to new modules and add cache isolation checks

• Moves resource-loader tests to weather_briefing.data.resources, and shifts localization tests to patch _load_reference_data directly. Adds regression tests ensuring load_reference_data returns independent values, reference_value only deep-copies the selected subtree, and reference_string_tuple avoids copying cached lists.

tests/test_reference_data.py

[qodo-code-review]

Code review by qodo was updated up to the latest commit 4ef5927

[IceCodeNew]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[IceCodeNew]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[IceCodeNew]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

---

Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 55 minutes.

[IceCodeNew]

/agentic_review

[qodo-code-review]

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

[IceCodeNew]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 4f4fb4d

[IceCodeNew]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 391070c

[IceCodeNew]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 9844c5b

[qodo-code-review]

Code review by qodo was updated up to the latest commit 9844c5b