[04/10] refactor: package geocoding

[IceCodeNew]

Summary

• separate geocoding contracts, candidate matching, providers, resolver, and composition
• preserve the public geocoding API through the package initializer
• keep provider-specific parsing and fallback behavior with their owners
• reject non-object Open-Meteo responses through the provider fallback boundary

Scope

Independently based on master. Merged as step 04 in the numbered series.

Verification

• prek run --all-files
• 886 tests passed
• line coverage: 99.85%
• branch coverage: 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: 58 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: 1dc7fe4a-8395-413d-86b2-52d98b47b7d4

📥 Commits

Reviewing files that changed from the base of the PR and between 26dcad5 and a64e13d.

📒 Files selected for processing (9)

• tests/test_geocoding.py
• weather_briefing/geocoding.py
• weather_briefing/geocoding/__init__.py
• weather_briefing/geocoding/base.py
• weather_briefing/geocoding/composition.py
• weather_briefing/geocoding/matching.py
• weather_briefing/geocoding/nominatim.py
• weather_briefing/geocoding/open_meteo.py
• weather_briefing/geocoding/resolver.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/weather-refactor-06-geocoding

---

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.79%. Comparing base (0e4a484) to head (a64e13d).
⚠️ Report is 3 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master      #98   +/-   ##
=======================================
  Coverage   99.79%   99.79%           
=======================================
  Files          46       52    +6     
  Lines        9119     9158   +39     
  Branches      552      552           
=======================================
+ Hits         9100     9139   +39     
  Misses         14       14           
  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): 41 rules

1. Uncaught Open-Meteo payload ✓ Resolved 🐞 Bug ☼ Reliability

Description

OpenMeteoGeocodingProvider.geocode() calls payload.get(...) without verifying response.json()
returned a dict, so a valid non-object JSON payload raises AttributeError and escapes as an
unexpected exception. This bypasses FallbackGeocodingProvider (which only catches GeocodingError)
and can abort geocoding instead of falling back to the next provider.

Code

weather_briefing/geocoding/open_meteo.py[R47-49]

+            payload = response.json()
+            results = payload.get("results", [])
+            if not isinstance(results, list):

Relevance

⭐⭐⭐ High

Team consistently adds JSON-boundary type checks to avoid unexpected exceptions escaping intended
error handling.

PR-#84
PR-#24
PR-#82

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

payload is used as if it were a dict (payload.get(...)) but the exception handler does not catch
AttributeError; therefore valid non-object JSON responses will escape as AttributeError. Since
the fallback wrapper only catches GeocodingError, this unexpected exception will prevent trying
subsequent providers.

weather_briefing/geocoding/open_meteo.py[40-55]
weather_briefing/geocoding/open_meteo.py[73-79]
weather_briefing/geocoding/composition.py[21-35]

Agent prompt

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

### Issue description
`OpenMeteoGeocodingProvider.geocode()` assumes `response.json()` returns a mapping and immediately calls `payload.get("results", ...)`. If Open-Meteo ever returns valid JSON whose root is not an object (e.g., a list/string/number/null), this raises `AttributeError` which is not caught and will propagate as a non-`GeocodingError`, bypassing fallback behavior.

### Issue Context
Other invalid-response shapes are already converted into `GeocodingError` (e.g., `results` not a list), and fallback composition only catches `GeocodingError`.

### Fix Focus Areas
- weather_briefing/geocoding/open_meteo.py[40-55]
- weather_briefing/geocoding/open_meteo.py[73-79]

### Suggested fix
Add an explicit `isinstance(payload, dict)` check after `payload = response.json()`. If it is not a dict, log `outcome="invalid-response"` and raise `GeocodingError("Open-Meteo geocoding returned an invalid response ...")` (or include `AttributeError` in the caught exceptions, but explicit validation is preferred for clarity and consistent logging).

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

---

[IceCodeNew]

/agentic_review

[qodo-code-review]

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

[IceCodeNew]

/agentic_review

[qodo-code-review]

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

[qodo-code-review]

PR Summary by Qodo

Refactor geocoding into a package while preserving the public API

✨ Enhancement 🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Split geocoding into focused modules (contracts, matching, providers, resolver, composition).
• Preserve existing public imports by re-exporting symbols from the geocoding package initializer.
• Harden Open-Meteo response validation and extend tests to cover invalid payload shapes.

Diagram

graph TD
A["Callers"] --> B["geocoding package API"] --> C["Cached resolver"] --> D["Providers"] --> E{{"Geocoding APIs"}}
C --> F["Matching rules"] --> G["Reference data"]
C --> H[("Local cache JSON")]
subgraph Legend
  direction LR
  _m["Module"] ~~~ _db[("Cache") ] ~~~ _ext{{"External"}}
end

Loading

High-Level Assessment

The chosen approach—splitting the monolithic module into a package with clear responsibilities while re-exporting the original public API—is the most maintainable option with minimal downstream churn. Alternatives like keeping a single file or introducing a parallel v2 API would either preserve complexity or force consumer migrations without clear benefit.

Files changed (8) +710 / -13

Bug fix (1) +93 / -0

open_meteo.pyMove Open-Meteo adapter and validate payload shape before reading results +93/-0

Move Open-Meteo adapter and validate payload shape before reading results

• Port Open-Meteo geocoding into its own provider module and add validation that the top-level JSON payload is an object before accessing the results list. Candidate-selection logging remains provider-owned while leveraging shared matching helpers.

weather_briefing/geocoding/open_meteo.py

Refactor (6) +592 / -0

__init__.pyAdd package initializer that preserves the public geocoding API +20/-0

Add package initializer that preserves the public geocoding API

• Introduce weather_briefing.geocoding as a package and re-export the prior public surface (providers, resolver, composition, and helper functions) via __all__. This keeps consumer imports stable after the refactor.

weather_briefing/geocoding/init.py

base.pyExtract geocoding contracts, safe error type, and logging helper +62/-0

Extract geocoding contracts, safe error type, and logging helper

• Move GeocodingError, provider Protocols, and candidate-selection logging into a dedicated module. Add a shared required_location_name helper to centralize forward-geocoding validation.

weather_briefing/geocoding/base.py

composition.pyExtract provider composition (fallback + precision reduction) +68/-0

Extract provider composition (fallback + precision reduction)

• Move FallbackGeocodingProvider and PrecisionReducingGeocodingProvider into their own module. Composition now depends on the shared base contracts and matching-driven precision reduction.

weather_briefing/geocoding/composition.py

matching.pyExtract matching logic and reference-driven geographic rules +129/-0

Extract matching logic and reference-driven geographic rules

• Centralize mainland-China bounds/rules validation, location name normalization and matching, and provider-specific match helpers. Functions are promoted to module-level utilities to be shared by providers and resolver.

weather_briefing/geocoding/matching.py

nominatim.pyMove Nominatim adapter into its own module and isolate rate limiting +168/-0

Move Nominatim adapter into its own module and isolate rate limiting

• Port Nominatim forward and reverse geocoding into a dedicated provider module using shared matching and base utilities. Rate limiting is factored into a small helper method for clearer test patch points.

weather_briefing/geocoding/nominatim.py

resolver.pyMove cached location resolver into its own module +145/-0

Move cached location resolver into its own module

• Extract CachedLocationResolver and cache serialization logic into a dedicated module. Resolver continues to support coordinate-only inputs (optional reverse geocoding) and caches provider-derived metadata in a stable JSON format.

weather_briefing/geocoding/resolver.py

Tests (1) +25 / -13

test_geocoding.pyUpdate imports/monkeypatch targets for new geocoding package layout +25/-13

Update imports/monkeypatch targets for new geocoding package layout

• Adjust tests to import matching helpers from weather_briefing.geocoding.matching and to monkeypatch the new module locations (nominatim.* and matching.*). Add a new test ensuring Open-Meteo geocoding rejects non-object JSON payloads.

tests/test_geocoding.py

[qodo-code-review]

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

[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.