fix: validate application configuration contracts

[IceCodeNew]

Summary

• reject non-string location and RSS scalar fields with JSON-path-aware configuration errors
• validate the fixed weather provider and publisher contracts during settings loading
• keep any-llm provider validation delegated to the SDK's dynamic provider registry

Verification

• mise exec -- uv run --with pytest -- pytest tests/test_config.py -q (119 passed)
• mise exec -- uv run --with pytest --with pytest-cov -- pytest --cov --cov-branch --cov-report=xml (608 passed; 15 missed lines and 7 partial branches, unchanged)
• mise exec -- prek run --all-files

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 17 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

Run ID: 47e60dc1-6e9c-43d1-96a9-881724003610

📥 Commits

Reviewing files that changed from the base of the PR and between 7730ec5 and e1b07a3.

📒 Files selected for processing (6)

• AGENTS.md
• tests/test_cli.py
• tests/test_config.py
• weather_briefing/cli.py
• weather_briefing/config.py
• weather_briefing/registries.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/validate-app-providers

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.72%. Comparing base (7730ec5) to head (e1b07a3).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master      #52   +/-   ##
=======================================
  Coverage   99.72%   99.72%           
=======================================
  Files          38       39    +1     
  Lines        6436     6498   +62     
  Branches      363      363           
=======================================
+ Hits         6418     6480   +62     
  Misses         13       13           
  Partials        5        5           

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

[IceCodeNew]

/agentic_review

[IceCodeNew]

/agentic_review

[qodo-code-review]

Code Review by Qodo

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

Context used

✅ Compliance rules (platform): 29 rules

1. Duplicated provider registries ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

config.py now hard-codes SUPPORTED_WEATHER_PROVIDERS / SUPPORTED_PUBLISHERS for validation, while
runtime support is separately defined in cli.py (WEATHER_PROVIDER_BUILDERS and publisher dispatch).
If a new provider/publisher is added in only one place, settings loading can reject a
runtime-supported option (or runtime can fail for a config-validated option).

Code

weather_briefing/config.py[R27-29]

+SUPPORTED_WEATHER_PROVIDERS = frozenset({"open-meteo", "qweather"})
+SUPPORTED_PUBLISHERS = frozenset({"stdout", "telegram"})
+

Relevance

⭐⭐ Medium

No historical evidence on de-duplicating supported registries; config validation PRs exist but not
this drift concern.

PR-#46
PR-#41

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR adds hard-coded supported sets and validates against them in config.py, while cli.py
independently defines what is actually supported at runtime via its builder registry and publisher
dispatch; this duplication can drift over time.

weather_briefing/config.py[23-29]
weather_briefing/config.py[182-199]
weather_briefing/cli.py[306-327]
weather_briefing/cli.py[385-434]

Agent prompt

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

## Issue description
`SUPPORTED_WEATHER_PROVIDERS` / `SUPPORTED_PUBLISHERS` were introduced in `weather_briefing/config.py` to validate env configuration, but the runtime capabilities are defined elsewhere in `weather_briefing/cli.py`. This creates two sources of truth that can drift, producing confusing behavior (e.g., a provider works at runtime but cannot be configured, or config passes but runtime cannot build it).

## Issue Context
- Config-time validation uses `SUPPORTED_WEATHER_PROVIDERS` and `SUPPORTED_PUBLISHERS`.
- Runtime behavior uses `WEATHER_PROVIDER_BUILDERS` keys and the publisher branching in `_delivery_provider`.

## Fix Focus Areas
- weather_briefing/config.py[27-200]
- weather_briefing/cli.py[306-435]

## Suggested approach
- Introduce a single shared registry module (e.g., `weather_briefing/registries.py`) that defines:
 - `SUPPORTED_WEATHER_PROVIDERS` (or `WEATHER_PROVIDER_NAMES`)
 - `SUPPORTED_PUBLISHERS`
- Import and use those constants both in:
 - `config.py` validation (`_configured_weather_providers()`, `_publisher()`)
 - `cli.py` dispatch (ensure `WEATHER_PROVIDER_BUILDERS` keys exactly match the shared provider names; optionally assert at import time).

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

---

Previous review results

Review updated until commit e1b07a3

Results up to commit 33098a0

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

1. Duplicated provider registries ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

config.py now hard-codes SUPPORTED_WEATHER_PROVIDERS / SUPPORTED_PUBLISHERS for validation, while
runtime support is separately defined in cli.py (WEATHER_PROVIDER_BUILDERS and publisher dispatch).
If a new provider/publisher is added in only one place, settings loading can reject a
runtime-supported option (or runtime can fail for a config-validated option).

Code

weather_briefing/config.py[R27-29]

+SUPPORTED_WEATHER_PROVIDERS = frozenset({"open-meteo", "qweather"})
+SUPPORTED_PUBLISHERS = frozenset({"stdout", "telegram"})
+

Relevance

⭐⭐ Medium

No historical evidence on de-duplicating supported registries; config validation PRs exist but not
this drift concern.

PR-#46
PR-#41

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR adds hard-coded supported sets and validates against them in config.py, while cli.py
independently defines what is actually supported at runtime via its builder registry and publisher
dispatch; this duplication can drift over time.

weather_briefing/config.py[23-29]
weather_briefing/config.py[182-199]
weather_briefing/cli.py[306-327]
weather_briefing/cli.py[385-434]

Agent prompt

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

## Issue description
`SUPPORTED_WEATHER_PROVIDERS` / `SUPPORTED_PUBLISHERS` were introduced in `weather_briefing/config.py` to validate env configuration, but the runtime capabilities are defined elsewhere in `weather_briefing/cli.py`. This creates two sources of truth that can drift, producing confusing behavior (e.g., a provider works at runtime but cannot be configured, or config passes but runtime cannot build it).

## Issue Context
- Config-time validation uses `SUPPORTED_WEATHER_PROVIDERS` and `SUPPORTED_PUBLISHERS`.
- Runtime behavior uses `WEATHER_PROVIDER_BUILDERS` keys and the publisher branching in `_delivery_provider`.

## Fix Focus Areas
- weather_briefing/config.py[27-200]
- weather_briefing/cli.py[306-435]

## Suggested approach
- Introduce a single shared registry module (e.g., `weather_briefing/registries.py`) that defines:
 - `SUPPORTED_WEATHER_PROVIDERS` (or `WEATHER_PROVIDER_NAMES`)
 - `SUPPORTED_PUBLISHERS`
- Import and use those constants both in:
 - `config.py` validation (`_configured_weather_providers()`, `_publisher()`)
 - `cli.py` dispatch (ensure `WEATHER_PROVIDER_BUILDERS` keys exactly match the shared provider names; optionally assert at import time).

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

---

[IceCodeNew]

@qodo-code-review 已采纳 “Duplicated provider registries” 意见：应用固定的 weather provider/publisher 名称现集中在 weather_briefing/registries.py 的 StrEnum 中；配置校验与运行时 builder map 共用该声明，并新增一致性测试防止 registry 漂移。修复已 amend 至 998e053，定向测试 202 passed、全量 608 passed，覆盖率未下降，prek 全通过。

[IceCodeNew]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 998e053

[IceCodeNew]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Validate config contracts for providers, publishers, and JSON scalar fields

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Reject non-string/blank location and RSS scalar fields with JSON-path-aware errors.
• Validate fixed weather provider and publisher names during Settings loading.
• Centralize CLI publisher construction via a builder registry keyed by enums.

Diagram

graph TD
  Env["Env vars"] --> Cfg["config.py"] --> Settings["Settings.from_env()"] --> CLI["cli.py runtime"]
  Files["JSON config files"] --> Cfg
  Reg["registries.py enums"] --> Cfg --> Val["Contract checks"]
  Reg --> CLI --> Builders["Builder registries"] --> Runtime["Publishers & providers"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Schema-driven config validation (Pydantic/JSON Schema)

• ➕ Single declarative source for required/optional fields and types
• ➕ Automatic path-aware errors and richer validation (formats, min/max, patterns)
• ➖ Adds dependency/complexity for a relatively small config surface
• ➖ Potentially harder to align with current lightweight env-based parsing

2. Single registry for all providers (including SDK-dynamic)

• ➕ One place to validate all provider names
• ➕ Could simplify error messages and discovery
• ➖ Conflicts with goal of deferring third-party/dynamic namespaces to owning SDK
• ➖ Risk of duplicated whitelist and drift vs SDK registry

Recommendation: Current approach is solid for the app-owned contract surface: it keeps validation local, adds path-aware errors, and uses enums as the single source of truth for fixed choices. Avoid validating third-party dynamic provider namespaces in-app (stay delegated to the SDK); if config complexity grows substantially, consider moving to schema-driven validation for maintainability.

Files changed (6) +181 / -47

Enhancement (1) +17 / -0

registries.pyAdd StrEnum registries for application-owned provider and publisher names +17/-0

Add StrEnum registries for application-owned provider and publisher names

• Defines WeatherProviderName and PublisherName as StrEnum values to centralize and type the fixed configuration contract for adapters the application owns. These enums are referenced by config validation, CLI wiring, and tests.

weather_briefing/registries.py

Bug fix (1) +40 / -17

config.pyAdd strict string-field helpers and validate supported providers/publishers +40/-17

Add strict string-field helpers and validate supported providers/publishers

• Introduces reusable helpers for required/optional non-empty string fields that raise ConfigurationError with a JSON-path-like prefix. Uses enum-backed supported sets to reject unknown WEATHER_PROVIDERS entries and invalid PUBLISHER values during Settings.from_env(), and updates location/feed parsing to stop coercing non-strings.

weather_briefing/config.py

Refactor (1) +46 / -21

cli.pyIntroduce publisher builder registry and use enum names for provider keys +46/-21

Introduce publisher builder registry and use enum names for provider keys

• Refactors publisher selection into PUBLISHER_BUILDERS with dedicated stdout/telegram builder functions and consistent unsupported-publisher handling. Switches weather provider comparisons/keys to use WeatherProviderName and updates WEATHER_PROVIDER_BUILDERS accordingly.

weather_briefing/cli.py

Tests (2) +77 / -9

test_cli.pyAssert CLI builder registries cover all declared provider/publisher names +5/-2

Assert CLI builder registries cover all declared provider/publisher names

• Updates tests to verify WEATHER_PROVIDER_BUILDERS and new PUBLISHER_BUILDERS exactly match the enum-defined names. This prevents drift between configuration contracts and runtime construction wiring.

tests/test_cli.py

test_config.pyExpand config tests for JSON-path errors and unsupported provider/publisher values +72/-7

Expand config tests for JSON-path errors and unsupported provider/publisher values

• Updates RSS source tests to assert path-aware error messages and adds coverage rejecting non-string scalar fields. Adds tests rejecting non-string location id/name fields and validates that unsupported WEATHER_PROVIDERS and PUBLISHER values fail fast with explicit messages.

tests/test_config.py

Documentation (1) +1 / -0

AGENTS.mdDocument config validation boundary and fixed-vs-dynamic provider guidance +1/-0

Document config validation boundary and fixed-vs-dynamic provider guidance

• Adds a guideline to validate configuration at the input boundary without coercing invalid scalar types. Clarifies that fixed application-owned choices should be rejected early while dynamic provider namespaces remain owned by the SDK.

AGENTS.md

[qodo-code-review]

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