fix: enforce forecast attribution and allergen advice - #31
Conversation
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThe change adds allergen availability metadata, introduces typed advice topics, strengthens source citation validation and conflict-handling prompts, enforces required forecast advice, and centralizes source attribution in HTML and plain-text renderers. ChangesBriefing contracts and validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant QWeatherProvider
participant BriefingService
participant LLM
participant parse_result
participant Renderer
QWeatherProvider->>BriefingService: provide weather documents and allergen metadata
BriefingService->>LLM: send required advice topics
LLM->>parse_result: return sourced briefing JSON
parse_result->>BriefingService: return validated BriefingResult
BriefingService->>Renderer: render structured conclusions and advice
Renderer-->>BriefingService: return attributed HTML or plain text
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
907dab8 to
819b6ab
Compare
Review feedback handledIndependently verified all six reported items.
Validation: 447 tests passed; line coverage 99.70%; branch coverage 98.31%; all pre-commit hooks passed. Amended commit: |
|
@coderabbitai review |
✅ Action performedReview finished.
|
819b6ab to
5e38c20
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@weather_briefing/llm.py`:
- Around line 103-110: Update _summarize’s sourced-field parsing to normalize
malformed model responses into LLMError: make cited_source_ids safely validate
source_ids before iterating, and use sourced_text(value, key) for conclusions
and disaster tracking plus sourced_text(value, "advice") for advice. Ensure
missing, null, non-string, or empty text fields raise LLMError so the existing
retry path handles them.
In `@weather_briefing/service.py`:
- Around line 259-262: Update the validation around the required-topic checks
and the corresponding logic at the alternate location to ensure every allergen
advice item cites at least one current document whose has_allergen_information
is true. Do not accept allergen advice solely because its source ID is known;
preserve existing validation for other topics and raise LLMError when the
allergen citation requirement is unmet.
In `@weather_briefing/weather_context.py`:
- Around line 228-232: Replace the hardcoded QWeather allergy type value in the
daily advice logic with a value loaded from a validated data configuration file
under the existing weather briefing data resources. Add the configuration entry,
load it through the established configuration mechanism, and use that value in
the allergen_advice_available check while preserving the current matching
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 21cd6a34-0ca6-4ab6-b87d-a5613eac879e
📒 Files selected for processing (14)
docs/design.mdtests/test_allergen.pytests/test_llm.pytests/test_prompts.pytests/test_render.pytests/test_service.pytests/test_weather_context.pyweather_briefing/allergen.pyweather_briefing/llm.pyweather_briefing/models.pyweather_briefing/prompts.pyweather_briefing/render.pyweather_briefing/service.pyweather_briefing/weather_context.py
5e38c20 to
6981ebe
Compare
Review-autofix completeVerified the local Codex review and the completed CodeRabbit review independently.
Final validation: 468 tests passed; line coverage 99.70%; branch coverage 98.34%; all pre-commit hooks and CI checks passed. CodeRabbit completed with no unresolved threads. Copilot could not run because the requester quota is exhausted. Final amended commit: |
Summary
Root cause
Headline and overview fields had no source-ID fields, so their claims could not be attributed. Lifestyle advice was prompt-only: the validator checked source IDs, publication state, and message length, but did not verify required advice categories. QWeather allergy index data therefore reached the model but could be silently omitted.
Validation
mise exec -- prek runmise exec -- uv run --with pytest --with pytest-cov -- pytest --cov --cov-branch --cov-report=xmlSummary by CodeRabbit