[03/10] refactor: package configuration

[IceCodeNew]

Summary

• separate environment parsing, file validation, feed loading, location locking, and settings composition
• preserve the public configuration API through the package initializer
• keep configuration validation at its input boundaries
• reject whitespace-only configuration paths

Scope

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

Verification

• prek run --all-files
• 890 tests passed
• line coverage: 99.87%
• 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: 55 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: f15705d7-974b-43cb-b292-a63c22fbbc3f

📥 Commits

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

📒 Files selected for processing (9)

• tests/test_config.py
• weather_briefing/config.py
• weather_briefing/config/__init__.py
• weather_briefing/config/base.py
• weather_briefing/config/environment.py
• weather_briefing/config/feeds.py
• weather_briefing/config/files.py
• weather_briefing/config/locations.py
• weather_briefing/config/settings.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/weather-refactor-05-config

---

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

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
+ Coverage   99.79%   99.81%   +0.02%     
==========================================
  Files          46       52       +6     
  Lines        9119     9172      +53     
  Branches      552      553       +1     
==========================================
+ Hits         9100     9155      +55     
+ Misses         14       12       -2     
  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. Env path values not validated ✓ Resolved 📘 Rule violation ≡ Correctness

Description

Path-like environment variables are converted into Path(...) without rejecting empty/whitespace
values, which can resolve to Path('.') and lead to unintended reads/writes or confusing failures.
This violates the requirement to validate configuration values (including basic constraints like
non-empty strings) at load time and fail fast with clear errors.

Code

weather_briefing/config/environment.py[R151-153]

+def state_path_from_env() -> Path:
+    """Return the configured SQLite state path without loading all settings."""
+    return Path(clean_env(os.getenv("BRIEFING_STATE_PATH", "state/weather.sqlite3")))

Relevance

⭐⭐⭐ High

Team recently accepted strict non-empty validation for config-derived strings; aligns with fail-fast
config boundary intent.

PR-#92

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2152168 requires configuration keys to be validated for type and basic constraints
and to fail fast on invalid values. BRIEFING_STATE_PATH, BRIEFING_LOCATIONS_FILE,
RSS_SOURCES_FILE, and GEOCODING_CACHE_PATH are converted directly into Path(...) without
rejecting empty strings, allowing invalid/ambiguous paths like Path('') → ..

Rule 2152168: Validate configuration at load time and reject invalid or unknown values
weather_briefing/config/environment.py[151-153]
weather_briefing/config/settings.py[81-82]
weather_briefing/config/settings.py[132-132]

Agent prompt

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

## Issue description
Several path-like environment variables are accepted as empty strings and passed to `Path(...)` without validation. In Python, `Path("")` resolves to the current directory (`.`), which is almost never a valid configuration value and can cause unintended filesystem effects.

## Issue Context
This PR introduces/centralizes environment parsing and config validation. Per the compliance requirement, config values should be validated for basic constraints (e.g., non-empty strings) and rejected with clear errors rather than being silently coerced into an unintended value.

## Fix Focus Areas
- weather_briefing/config/environment.py[151-153]
- weather_briefing/config/settings.py[81-82]
- weather_briefing/config/settings.py[132-132]

ⓘ 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 ff1828f

[IceCodeNew]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 8906f4d

[qodo-code-review]

PR Summary by Qodo

Refactor configuration into dedicated package modules

✨ Enhancement 🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Split runtime configuration into focused modules (env, files, feeds, locations, settings).
• Preserve the public config API via weather_briefing.config package exports.
• Tighten input-boundary validation (reject empty paths; surface JSON read failures).

Diagram

graph TD
  A["App / callers"] --> B["config/__init__.py"] --> C["config/settings.py"] --> D["config/environment.py"]
  C --> E["config/feeds.py"] --> F["config/files.py"]
  C --> G["config/locations.py"] --> F

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Adopt a settings/validation library (e.g., pydantic-settings)

• ➕ Centralized schema, validation, and error reporting
• ➕ Less custom parsing code to maintain long-term
• ➖ Adds a non-trivial dependency and migration surface
• ➖ May not fit existing error-message expectations/tests

2. Keep a single config module but split into internal sections

• ➕ Fewer modules/exports to manage
• ➕ Simpler import graph for small projects
• ➖ Harder to enforce input-boundary validation and keep responsibilities isolated
• ➖ Refactor series goal (package-level separation) is not fully met

Recommendation: Proceed with the current modular split: it keeps the public API stable while making responsibilities explicit (env parsing vs file IO vs feeds vs locations vs settings composition). Given the series goal and desire to keep validation at boundaries without adding dependencies, this approach is the best tradeoff; the main reviewer focus should be verifying identical behavior and error semantics at the package exports.

Files changed (8) +699 / -3

Refactor (7) +664 / -0

__init__.pyPreserve public config API via package re-exports +14/-0

Preserve public config API via package re-exports

• Introduces the weather_briefing.config package initializer that re-exports ConfigurationError, Settings, backfill_location_fields, state_path_from_env, and weather_providers_for. This keeps existing import paths stable while the implementation moves into submodules.

weather_briefing/config/init.py

base.pyCentralize ConfigurationError definition +5/-0

Centralize ConfigurationError definition

• Adds a dedicated base module holding ConfigurationError so it can be shared consistently across new config submodules.

weather_briefing/config/base.py

environment.pyExtract environment parsing and provider selection helpers +188/-0

Extract environment parsing and provider selection helpers

• Moves env parsing/validation helpers (ints, floats, booleans, cron hours, provider ordering, publisher selection) into a focused module. Adds path_from_env() to ensure path-like environment variables are non-empty, tightening boundary validation.

weather_briefing/config/environment.py

feeds.pyIsolate RSS source config loading/validation +74/-0

Isolate RSS source config loading/validation

• Introduces load_feeds() to parse rss-sources JSON and validate feed fields and optional pattern/selector arrays. Reuses shared JSON/file helpers to keep file parsing behavior consistent.

weather_briefing/config/feeds.py

files.pyAdd shared JSON file and field parsing helpers +47/-0

Add shared JSON file and field parsing helpers

• Provides json_file/json_array helpers that normalize JSON decoding and OS read errors into ConfigurationError. Also centralizes required/optional string field validation for reuse by feeds and locations.

weather_briefing/config/files.py

locations.pyMove location loading and locked backfill into dedicated module +173/-0

Move location loading and locked backfill into dedicated module

• Implements load_locations() plus the existing backfill_location_fields() behavior with explicit file locking and JSON validation. Keeps lock timeout/retry constants local to the locations module and preserves prior error handling semantics.

weather_briefing/config/locations.py

settings.pyCompose Settings from the new config submodules +163/-0

Compose Settings from the new config submodules

• Rebuilds Settings.from_env() on top of environment.py, feeds.py, and locations.py, keeping the Settings dataclass interface intact. Uses path_from_env() for path variables, ensuring whitespace-only values are rejected early.

weather_briefing/config/settings.py

Tests (1) +35 / -3

test_config.pyUpdate imports and expand config validation coverage +35/-3

Update imports and expand config validation coverage

• Switches tests to import the location loader from weather_briefing.config.locations and updates monkeypatch targets for lock timing constants. Adds regression tests to ensure JSON read failures are surfaced as ConfigurationError and empty/whitespace path environment values are rejected by Settings.from_env().

tests/test_config.py

[qodo-code-review]

Code review by qodo was updated up to the latest commit 8906f4d

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