fix: close capability review gaps

[IceCodeNew]

Summary

• reuse the shared weather-context dated dispatch from capability composition
• preserve the non-callable dated-provider guard at the shared boundary
• define ALLERGEN as structured AllergenSnapshot output, distinct from QWeather lifestyle advice
• record the review, stacked-PR, native macOS, formatting, and coverage checks that prevent recurrence

Verification

• prek run --all-files
• .venv/bin/pytest --cov --cov-branch --cov-report=term-missing (686 passed, 99% total coverage)

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 12 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: 4a6b58ff-7535-4a0b-a98b-dd73b245b215

📥 Commits

Reviewing files that changed from the base of the PR and between 413d7df and ced5782.

📒 Files selected for processing (5)

• AGENTS.md
• docs/design.md
• docs/notes.md
• weather_briefing/capabilities.py
• weather_briefing/weather_context.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/fix-capability-review

---

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.76%. Comparing base (413d7df) to head (ced5782).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #76      +/-   ##
==========================================
- Coverage   99.76%   99.76%   -0.01%     
==========================================
  Files          41       41              
  Lines        7580     7576       -4     
  Branches      425      423       -2     
==========================================
- Hits         7562     7558       -4     
  Misses         13       13              
  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 (1) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 31 rules

1. ALLERGEN contract duplicated in docs 📘 Rule violation ⚙ Maintainability ⭐ New

Description

The ALLERGEN capability contract is described in both docs/design.md and docs/notes.md with
overlapping multi-sentence text, but docs/notes.md does not point to a canonical source. This
duplication can drift over time and violates the requirement to link to a single authoritative
document instead of repeating detailed contracts.

Code

docs/notes.md[40]

+`ALLERGEN` capability 只声明 adapter 能提供独立、结构化的 `AllergenSnapshot`，而不是泛指任何与过敏有关的用户文本。路由元数据需要表达可独立替换的数据契约，因此 QWeather 类型 7 综合过敏指数仍归入 `LIFESTYLE`；面向简报完整性的 `has_allergen_information` 可以同时为真，用来要求模型给出过敏建议。这一选择接受了“QWeather 有过敏建议但 `supports(ALLERGEN)` 为假”的表面差异，以避免把综合指数误当成具体花粉浓度或可替换的过敏原数据源。若未来路由需要查找“任意过敏建议”，应新增不同 capability，而不能扩宽 `ALLERGEN`；若 QWeather 提供结构化花粉种类和浓度，则应重新声明其支持 `ALLERGEN`。

Relevance

⭐⭐⭐ High

Similar docs-duplication finding was at least partially accepted; notes.md expected to link
canonical doc (PR #62, #54).

PR-#62
PR-#54

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2141669 requires avoiding duplicated documentation and linking to the canonical
source. The ALLERGEN contract is described in both docs/design.md and docs/notes.md as
detailed, multi-sentence text without an explicit canonical-link pattern for this specific contract.

Rule 2141669: Avoid duplicating existing documentation; link to the canonical source instead
docs/notes.md[40-40]
docs/design.md[51-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
`docs/notes.md` repeats the `ALLERGEN` capability contract that is also documented in `docs/design.md`, without linking to a single canonical source.

## Issue Context
The repo now has two separate documents explaining the same contract details; this increases the likelihood of future divergence.

## Fix Focus Areas
- docs/notes.md[40-40]
- docs/design.md[51-51]

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

---

2. ALLERGEN semantics missing in notes ✓ Resolved 📘 Rule violation ⌂ Architecture

Description

This change defines ALLERGEN as meaning a structured AllergenSnapshot (and explicitly not
QWeather lifestyle allergy advice), but the corresponding architectural
rationale/trade-offs/boundaries are not captured in docs/notes.md as required. This leaves a
non-obvious capability contract change insufficiently documented for future maintainers.

Code

docs/design.md[51]

+能力组合边界由 `capabilities.py` 的 `CapabilityProviderSet` 承担。天气、空气质量、结构化过敏原、生活指数、预警和短时预报属于可独立声明的 capability；现有 QWeather/Open-Meteo 完整上下文 adapter 暂时挂在天气槽位，AQICN 挂在空气质量槽位。`ALLERGEN` 表示 adapter 能提供独立 `AllergenSnapshot`，不表示任意与过敏有关的文本；QWeather 类型 7 综合过敏指数仍属于 `LIFESTYLE`，即使它会标记文档包含过敏建议。这样本地气象机构可以只实现预警或 nowcast，而不必伪装为完整天气 provider；后续能力 provider 不应为填充无关字段而发起额外请求。

Relevance

⭐⭐⭐ High

Team repeatedly records architectural boundaries/assumptions in docs/notes.md; notes updates merged
in PR #54/#58.

PR-#54
PR-#58
PR-#37

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

docs/design.md introduces/clarifies a non-obvious capability contract: ALLERGEN means the
provider can supply a structured AllergenSnapshot, and it is distinct from QWeather lifestyle
allergy advice. docs/notes.md (the required decision-log location) contains general
capability-boundary rationale but does not record this specific ALLERGEN semantic decision with
rationale/trade-offs/boundaries.

Rule 2141673: Document non-obvious architectural decisions in docs/notes.md
docs/design.md[51-61]
docs/notes.md[1-4]
docs/notes.md[36-40]

Agent prompt

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

## Issue description
The PR updates the capability contract/semantics for `ALLERGEN` (structured `AllergenSnapshot` vs. QWeather lifestyle allergy advice) in `docs/design.md`, but there is no matching entry in `docs/notes.md` documenting the decision with rationale, trade-offs, and operating boundaries.

## Issue Context
Per the compliance checklist, non-obvious architectural decisions introduced or modified in a change must be recorded in `docs/notes.md` with enough detail to explain why the design is correct and when it should be revisited.

## Fix Focus Areas
- docs/notes.md[1-4]
- docs/notes.md[36-40]
- docs/design.md[51-61]

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

---

Previous review results

Review updated until commit ced5782 ⚖️ Balanced

Results up to commit 3fe8d8a ⚖️ Balanced

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

1. ALLERGEN semantics missing in notes ✓ Resolved 📘 Rule violation ⌂ Architecture

Description

This change defines ALLERGEN as meaning a structured AllergenSnapshot (and explicitly not
QWeather lifestyle allergy advice), but the corresponding architectural
rationale/trade-offs/boundaries are not captured in docs/notes.md as required. This leaves a
non-obvious capability contract change insufficiently documented for future maintainers.

Code

docs/design.md[51]

+能力组合边界由 `capabilities.py` 的 `CapabilityProviderSet` 承担。天气、空气质量、结构化过敏原、生活指数、预警和短时预报属于可独立声明的 capability；现有 QWeather/Open-Meteo 完整上下文 adapter 暂时挂在天气槽位，AQICN 挂在空气质量槽位。`ALLERGEN` 表示 adapter 能提供独立 `AllergenSnapshot`，不表示任意与过敏有关的文本；QWeather 类型 7 综合过敏指数仍属于 `LIFESTYLE`，即使它会标记文档包含过敏建议。这样本地气象机构可以只实现预警或 nowcast，而不必伪装为完整天气 provider；后续能力 provider 不应为填充无关字段而发起额外请求。

Relevance

⭐⭐⭐ High

Team repeatedly records architectural boundaries/assumptions in docs/notes.md; notes updates merged
in PR #54/#58.

PR-#54
PR-#58
PR-#37

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

docs/design.md introduces/clarifies a non-obvious capability contract: ALLERGEN means the
provider can supply a structured AllergenSnapshot, and it is distinct from QWeather lifestyle
allergy advice. docs/notes.md (the required decision-log location) contains general
capability-boundary rationale but does not record this specific ALLERGEN semantic decision with
rationale/trade-offs/boundaries.

Rule 2141673: Document non-obvious architectural decisions in docs/notes.md
docs/design.md[51-61]
docs/notes.md[1-4]
docs/notes.md[36-40]

Agent prompt

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

## Issue description
The PR updates the capability contract/semantics for `ALLERGEN` (structured `AllergenSnapshot` vs. QWeather lifestyle allergy advice) in `docs/design.md`, but there is no matching entry in `docs/notes.md` documenting the decision with rationale, trade-offs, and operating boundaries.

## Issue Context
Per the compliance checklist, non-obvious architectural decisions introduced or modified in a change must be recorded in `docs/notes.md` with enough detail to explain why the design is correct and when it should be revisited.

## Fix Focus Areas
- docs/notes.md[1-4]
- docs/notes.md[36-40]
- docs/design.md[51-61]

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

---

Results up to commit ced5782 ⚖️ Balanced

No changes from previous review

[IceCodeNew]

/agentic_review

[qodo-code-review]

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

[qodo-code-review]

PR Summary by Qodo

Fix dated weather-context dispatch and clarify ALLERGEN capability contract

🐞 Bug fix 📝 Documentation 🕐 20-40 Minutes

AI Description

• Route dated weather-context fetches through shared boundary logic.
• Reinstate non-callable dated-provider guard to prevent invalid dispatch.
• Clarify ALLERGEN as structured AllergenSnapshot, not lifestyle “allergy advice”.
• Document stacked-PR, review-thread, macOS, formatting, and coverage safeguards.

Diagram

graph TD
A["CapabilityProviderSet"] --> B["_fetch_context()"] --> C["fetch_weather_context()"] --> D{"forecast_date is None?"}
D -->|"yes"| E["provider.fetch()"]
D -->|"no"| F{"Dated + callable?"}
F -->|"ok"| G["fetch_for_date()"]
F -->|"error"| H["WeatherContextError"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep capability-local dated dispatch logic

• ➕ Avoids coupling capability composition to weather_context helpers
• ➕ Keeps all dispatch rules in one file (capabilities.py) for capability owners
• ➖ Duplicates boundary rules and risks divergence (as evidenced by guard mismatch)
• ➖ Harder to enforce consistent error semantics across call sites

2. Make dated fetch a required abstract method (no runtime callable guard)

• ➕ Moves correctness to type/interface level
• ➕ Reduces runtime branching and simplifies call sites
• ➖ Less tolerant of adapters that intentionally set fetch_for_date to non-callable/None as a guard
• ➖ Potentially larger refactor across existing providers and tests

Recommendation: Prefer the PR’s approach: centralize current/dated routing in fetch_weather_context() and keep the explicit callable guard. It prevents drift across boundaries while preserving the existing defensive contract for providers that advertise the dated interface but intentionally do not expose a callable dated fetch.

Files changed (5) +26 / -13

Bug fix (2) +8 / -12

capabilities.pyDelegate current/dated context fetch to shared boundary function +4/-11

Delegate current/dated context fetch to shared boundary function

• Replaces inline forecast-date routing and dated-provider checks with a call to weather_context.fetch_weather_context(). This reduces duplicated dispatch logic at the capability composition boundary and aligns behavior with the shared contract.

weather_briefing/capabilities.py

weather_context.pyRestore callable guard for fetch_for_date dispatch +4/-1

Restore callable guard for fetch_for_date dispatch

• Adds a callable check for provider.fetch_for_date before invoking it for target forecast dates. Ensures non-callable guards raise a WeatherContextError rather than failing later with a TypeError or an inconsistent boundary behavior.

weather_briefing/weather_context.py

Documentation (3) +18 / -1

AGENTS.mdTighten stacked-PR and review-thread completion rules +15/-0

Tighten stacked-PR and review-thread completion rules

• Adds guidance to avoid repeated rebases/pushes in stacked PRs before upstream merges. Strengthens review-completion criteria (commit OID pinning, thread-aware checks, latest-head bot findings), plus macOS-native testing, post-rebase Ruff formatting, and repository-wide hook/coverage run order.

AGENTS.md

design.mdClarify ALLERGEN capability as structured AllergenSnapshot +1/-1

Clarify ALLERGEN capability as structured AllergenSnapshot

• Refines the capability boundary description to distinguish structured ALLERGEN output from QWeather lifestyle allergy advice. Documents that QWeather type-7 comprehensive allergy index remains part of LIFESTYLE despite mentioning allergy suggestions.

docs/design.md

notes.mdExplain ALLERGEN semantics and routing implications +2/-0

Explain ALLERGEN semantics and routing implications

• Adds a note defining ALLERGEN as a replaceable, structured AllergenSnapshot contract rather than generic allergy-related text. Records the intentional tradeoff that an adapter may provide allergy advice while supports(ALLERGEN) remains false, and outlines how future capabilities should be introduced instead of widening ALLERGEN.

docs/notes.md

[qodo-code-review]

1. allergen contract duplicated in docs 📘 Rule violation ⚙ Maintainability

The ALLERGEN capability contract is described in both docs/design.md and docs/notes.md with
overlapping multi-sentence text, but docs/notes.md does not point to a canonical source. This
duplication can drift over time and violates the requirement to link to a single authoritative
document instead of repeating detailed contracts.

Agent Prompt

## Issue description
`docs/notes.md` repeats the `ALLERGEN` capability contract that is also documented in `docs/design.md`, without linking to a single canonical source.

## Issue Context
The repo now has two separate documents explaining the same contract details; this increases the likelihood of future divergence.

## Fix Focus Areas
- docs/notes.md[40-40]
- docs/design.md[51-51]

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

[qodo-code-review]

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