fix: validate configuration inputs

[IceCodeNew]

Summary

• reject unsupported DEBUG literals instead of silently disabling diagnostics
• validate required context-source strings with indexed field paths
• reject non-array RSS option fields while preserving null as empty
• document the strict configuration contract

Validation

• uv run pytest tests/test_config.py (91 passed)
• uv run --with pytest --with pytest-cov -- pytest --cov --cov-branch --cov-report=xml (578 passed; misses/partials 15/7)
• prek run --all-files
• CodeRabbit CLI: 0 findings

[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: 34 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: ab2ffd4d-1db9-4b56-9f83-f6948e8923cf

📥 Commits

Reviewing files that changed from the base of the PR and between b6a1c5c and 6cd15d5.

📒 Files selected for processing (3)

• docs/requirements.md
• tests/test_config.py
• weather_briefing/config.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/validate-config-inputs

---

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.64%. Comparing base (b6a1c5c) to head (6cd15d5).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master      #46   +/-   ##
=======================================
  Coverage   99.64%   99.64%           
=======================================
  Files          38       38           
  Lines        6115     6210   +95     
  Branches      331      341   +10     
=======================================
+ Hits         6093     6188   +95     
  Misses         15       15           
  Partials        7        7           

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

[IceCodeNew]

/agentic_review

[qodo-code-review]

Code Review by Qodo

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

Context used

✅ Compliance rules (platform): 27 rules

✅ REVIEW.md

1. RSS array entries unvalidated ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New

Description

_optional_string_array() coerces every list element with str() and allows empty strings, so
misconfigured RSS selectors/regex/title-pattern lists can either fail later at runtime
(ContentCleaningError) or accidentally match every title (because "" in title is always true). This
undermines the PR’s goal of failing fast with precise configuration errors for invalid inputs.

Code

weather_briefing/config.py[R121-127]

+def _optional_string_array(item: dict[str, Any], source_id: str, field: str) -> tuple[str, ...]:
+    value = item.get(field)
+    if value is None:
+        return ()
+    if not isinstance(value, list):
+        raise ConfigurationError(f"RSS source {source_id} field {field} must be a JSON array")
+    return tuple(str(entry) for entry in value)

Evidence

The new helper only checks that the field is a list and then stringifies elements. Those values are
later used as substring patterns for title classification and as selector/regex inputs for HTML
cleaning, where invalid/empty entries can cause match-all behavior or runtime cleaning errors rather
than a configuration-time ConfigurationError with a field path.

weather_briefing/config.py[121-127]
weather_briefing/sources.py[110-118]
weather_briefing/content_cleaners.py[39-56]
weather_briefing/service.py[428-440]

Agent prompt

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

### Issue description
`_optional_string_array()` currently validates only that a field is a JSON list, then stringifies every element and returns it. This permits non-string entries and empty strings, which can later:
- break RSS cleaning at runtime (invalid CSS selectors / invalid regex patterns)
- cause unintended match-all behavior for title pattern lists (empty substring matches every title)

### Issue Context
The PR already strengthened config validation and error messaging. Completing element-level validation for RSS optional arrays aligns with the new strict contract and prevents delayed runtime errors.

### Fix Focus Areas
- weather_briefing/config.py[121-127]

### Suggested fix
- Validate that every list entry is a `str` and (after `.strip()`) is non-empty; otherwise raise `ConfigurationError` including `source_id` and `field`.
- Consider field-specific syntax checks during config load:
 - for `*_patterns`: attempt `re.compile()` and raise `ConfigurationError` on `re.error`
 - for `*_selectors`: validate selector syntax (e.g., via soupsieve compilation) and raise `ConfigurationError` on syntax errors
- Add tests covering:
 - empty string entries in `verbatim_title_patterns` / `forecast_title_patterns`
 - non-string entries (numbers/objects) in selector/pattern arrays
 - invalid regex / invalid selector strings producing `ConfigurationError` at load time

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

---

2. Quoted bool whitespace rejected ✓ Resolved 🐞 Bug ≡ Correctness

Description

_boolean() does not strip whitespace after _clean_env() removes optional outer quotes, so DEBUG
values like "\" false \"" become " false " and now raise ConfigurationError unexpectedly. This can
break startup in dotenv/Compose-style environments where quoted values may carry incidental
whitespace.

Code

weather_briefing/config.py[R100-106]

+def _boolean(name: str, default: bool) -> bool:
+    value = _clean_env(os.getenv(name, str(default))).casefold()
+    if value in {"1", "true", "yes"}:
+        return True
+    if value in {"0", "false", "no", ""}:
+        return False
+    raise ConfigurationError(f"{name} must be one of: true, false, 1, 0, yes, no")

Evidence

_clean_env() strips before quote-removal but does not strip after unquoting, and _boolean()
casefolds the unquoted result directly; this makes quoted values with internal leading/trailing
spaces fail validation. The repo’s example env file notes quoted values may be passed literally,
motivating robust handling.

weather_briefing/config.py[31-37]
weather_briefing/config.py[100-106]
env.example[2-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
`_boolean()` uses `_clean_env(...).casefold()` without a final `.strip()`. If the env var is quoted with spaces inside the quotes (e.g. `DEBUG=" false "`), `_clean_env()` removes the quotes but preserves the inner spaces, and `_boolean()` rejects it as an unknown literal.

## Issue Context
The repo explicitly mentions that dotenv/Compose accept quoted values and that quotes may be passed literally into containers, so accepting optional quotes should be robust to incidental whitespace.

## Fix Focus Areas
- weather_briefing/config.py[31-37]
- weather_briefing/config.py[100-106]

## Suggested change
Either:
- Update `_clean_env()` to `return value[1:-1].strip()` when unquoting, or
- Update `_boolean()` to do `value = _clean_env(...).strip().casefold()`.

Add a small test for `DEBUG='" false "'` (and optionally `'"TRUE "'`) to lock behavior.

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

---

3. Context type error unindexed ✓ Resolved 🐞 Bug ◔ Observability

Description

_context_source() raises a generic "CONTEXT_SOURCES_JSON must be a JSON array of objects" when an
element is not an object, even though the top-level value is already an array, and it omits the
failing index. This makes malformed arrays harder to debug and conflicts with the documented
expectation of field-path style errors.

Code

weather_briefing/config.py[R130-133]

+def _context_source(item: object, index: int) -> ContextSourceConfig:
+    if not isinstance(item, dict):
+        raise ConfigurationError("CONTEXT_SOURCES_JSON must be a JSON array of objects")
+

Evidence

Settings.from_env() enumerates the array and passes an index into _context_source(), but
_context_source() does not include that index in its non-dict error path. Tests and docs emphasize
field-path error reporting (e.g., CONTEXT_SOURCES_JSON[0].field), so omitting the index here is
inconsistent and makes diagnosis harder.

weather_briefing/config.py[285-292]
weather_briefing/config.py[130-133]
tests/test_config.py[586-594]
docs/requirements.md[47-51]

Agent prompt

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

## Issue description
When `CONTEXT_SOURCES_JSON` is a JSON array but contains a non-object element (e.g. `[1]`), `_context_source()` throws an error message that (a) reads like a top-level schema failure and (b) does not include the element index, despite having it.

## Issue Context
This PR adds indexed field-path errors for missing required fields; the same approach should apply to element type errors for consistency and debuggability.

## Fix Focus Areas
- weather_briefing/config.py[130-133]
- weather_briefing/config.py[285-292]

## Suggested change
Change the non-dict branch to something like:
- `raise ConfigurationError(f"CONTEXT_SOURCES_JSON[{index}] must be a JSON object")`

Optionally adjust the non-list check in `from_env()` to a clearer top-level message (e.g. "must be a JSON array") so element-type failures and top-level-type failures are distinguishable.

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

---

Previous review results

Review updated until commit 6cd15d5

Results up to commit d12024e

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

1. Quoted bool whitespace rejected ✓ Resolved 🐞 Bug ≡ Correctness

Description

_boolean() does not strip whitespace after _clean_env() removes optional outer quotes, so DEBUG
values like "\" false \"" become " false " and now raise ConfigurationError unexpectedly. This can
break startup in dotenv/Compose-style environments where quoted values may carry incidental
whitespace.

Code

weather_briefing/config.py[R100-106]

+def _boolean(name: str, default: bool) -> bool:
+    value = _clean_env(os.getenv(name, str(default))).casefold()
+    if value in {"1", "true", "yes"}:
+        return True
+    if value in {"0", "false", "no", ""}:
+        return False
+    raise ConfigurationError(f"{name} must be one of: true, false, 1, 0, yes, no")

Evidence

_clean_env() strips before quote-removal but does not strip after unquoting, and _boolean()
casefolds the unquoted result directly; this makes quoted values with internal leading/trailing
spaces fail validation. The repo’s example env file notes quoted values may be passed literally,
motivating robust handling.

weather_briefing/config.py[31-37]
weather_briefing/config.py[100-106]
env.example[2-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
`_boolean()` uses `_clean_env(...).casefold()` without a final `.strip()`. If the env var is quoted with spaces inside the quotes (e.g. `DEBUG=" false "`), `_clean_env()` removes the quotes but preserves the inner spaces, and `_boolean()` rejects it as an unknown literal.

## Issue Context
The repo explicitly mentions that dotenv/Compose accept quoted values and that quotes may be passed literally into containers, so accepting optional quotes should be robust to incidental whitespace.

## Fix Focus Areas
- weather_briefing/config.py[31-37]
- weather_briefing/config.py[100-106]

## Suggested change
Either:
- Update `_clean_env()` to `return value[1:-1].strip()` when unquoting, or
- Update `_boolean()` to do `value = _clean_env(...).strip().casefold()`.

Add a small test for `DEBUG='" false "'` (and optionally `'"TRUE "'`) to lock behavior.

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

---

2. Context type error unindexed ✓ Resolved 🐞 Bug ◔ Observability

Description

_context_source() raises a generic "CONTEXT_SOURCES_JSON must be a JSON array of objects" when an
element is not an object, even though the top-level value is already an array, and it omits the
failing index. This makes malformed arrays harder to debug and conflicts with the documented
expectation of field-path style errors.

Code

weather_briefing/config.py[R130-133]

+def _context_source(item: object, index: int) -> ContextSourceConfig:
+    if not isinstance(item, dict):
+        raise ConfigurationError("CONTEXT_SOURCES_JSON must be a JSON array of objects")
+

Evidence

Settings.from_env() enumerates the array and passes an index into _context_source(), but
_context_source() does not include that index in its non-dict error path. Tests and docs emphasize
field-path error reporting (e.g., CONTEXT_SOURCES_JSON[0].field), so omitting the index here is
inconsistent and makes diagnosis harder.

weather_briefing/config.py[285-292]
weather_briefing/config.py[130-133]
tests/test_config.py[586-594]
docs/requirements.md[47-51]

Agent prompt

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

## Issue description
When `CONTEXT_SOURCES_JSON` is a JSON array but contains a non-object element (e.g. `[1]`), `_context_source()` throws an error message that (a) reads like a top-level schema failure and (b) does not include the element index, despite having it.

## Issue Context
This PR adds indexed field-path errors for missing required fields; the same approach should apply to element type errors for consistency and debuggability.

## Fix Focus Areas
- weather_briefing/config.py[130-133]
- weather_briefing/config.py[285-292]

## Suggested change
Change the non-dict branch to something like:
- `raise ConfigurationError(f"CONTEXT_SOURCES_JSON[{index}] must be a JSON object")`

Optionally adjust the non-list check in `from_env()` to a clearer top-level message (e.g. "must be a JSON array") so element-type failures and top-level-type failures are distinguishable.

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

[IceCodeNew]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Fix: strict validation for configuration inputs (DEBUG, context sources, RSS arrays)

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Reject unsupported DEBUG literals instead of silently disabling diagnostics.
• Validate CONTEXT_SOURCES_JSON objects with indexed field-path errors and required strings.
• Enforce RSS optional-array fields are arrays of non-empty strings; keep null as empty.
• Document the stricter configuration contract and expected error messaging.

Diagram

graph TD
  env{{"Env vars"}} --> settings["Settings.from_env"] --> out["Settings"]
  settings --> bool["_boolean(DEBUG)"]
  settings --> feeds["_feeds()"] --> rss[("rss-sources.json")]
  feeds --> optarr["_optional_string_array()"]
  settings --> ctx["_context_source()"] --> ctxjson[("CONTEXT_SOURCES_JSON")]

  subgraph Legend
    direction LR
    _ext{{"External input"}} ~~~ _proc["Validator/Parser"] ~~~ _store[("JSON source")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Adopt a schema library (Pydantic / attrs validators) for Settings + JSON inputs

• ➕ Single source of truth for types/ranges/required fields
• ➕ Consistent error formatting and nested field-path reporting
• ➕ Less bespoke validation code over time as config grows
• ➖ Adds a heavier dependency and migration cost
• ➖ May constrain current lightweight dataclass-based design
• ➖ Error messages might become less tailored without customization

2. Use JSON Schema for RSS sources and CONTEXT_SOURCES_JSON validation

• ➕ Keeps validation declarative and close to the JSON contract
• ➕ Easy to evolve with explicit versioned schemas
• ➕ Good tooling support for pre-flight validation
• ➖ Another dependency/runtime step
• ➖ Still requires custom glue for env parsing (e.g., DEBUG) and improved messages
• ➖ Schema validation exceptions may need mapping to current error wording

Recommendation: Current approach (small focused validators + targeted tests) is a good fit: it improves correctness and produces actionable, indexed error messages without introducing new heavy dependencies. Consider JSON Schema/Pydantic only if configuration surface area grows substantially or multiple config formats need to share a unified contract.

Files changed (3) +225 / -14

Bug fix (1) +69 / -12

config.pyHarden Settings parsing with strict boolean and structured JSON validators +69/-12

Harden Settings parsing with strict boolean and structured JSON validators

• Adds a strict _boolean() parser for DEBUG that rejects unknown literals rather than silently evaluating false. Introduces _optional_string_array() to validate RSS optional arrays (type, non-empty strings, and optional selector/regex compilation), preserving null as empty. Adds _context_source() to validate CONTEXT_SOURCES_JSON entries with index-aware field-path errors and required string trimming, and wires these helpers into Settings.from_env and feed loading.

weather_briefing/config.py

Tests (1) +155 / -1

test_config.pyAdd regression tests for strict DEBUG, context source paths, and RSS array validation +155/-1

Add regression tests for strict DEBUG, context source paths, and RSS array validation

• Introduces tests ensuring RSS optional-array fields reject non-arrays, require non-empty string entries, and fail fast on invalid selector/regex syntax. Expands DEBUG parsing coverage to accept quoted/whitespace variants, accept falsey literals, and reject unknown values. Adds CONTEXT_SOURCES_JSON tests for indexed error paths, required string fields, and stripping of surrounding whitespace.

tests/test_config.py

Documentation (1) +1 / -1

requirements.mdDocument stricter configuration validation and error-message expectations +1/-1

Document stricter configuration validation and error-message expectations

• Expands the reliability/architecture requirements to explicitly require type/range/required validation, strict boolean literal handling, and JSON array type enforcement. Clarifies that RSS optional arrays treat null as empty while rejecting invalid element types and invalid selector/regex syntax with field-path errors.

docs/requirements.md

[qodo-code-review]

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