Skip to content

fix: ignore stale weather data - #124

Merged
IceCodeNew merged 5 commits into
masterfrom
codex/drop-stale-air-quality-conflicts
Jul 29, 2026
Merged

fix: ignore stale weather data#124
IceCodeNew merged 5 commits into
masterfrom
codex/drop-stale-air-quality-conflicts

Conversation

@IceCodeNew

@IceCodeNew IceCodeNew commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Summary

  • exclude current weather, air-quality, and allergen documents individually when they are more than two hours behind the freshest available observation
  • apply the same freshness boundary to deferred weather, temperature, precipitation, wind, and air-quality information before it can form a current conclusion or trigger a later delivery
  • retain exactly-two-hour boundary data and exempt explicitly requested forecast dates

Root cause

Freshness was enforced only for air-quality documents. Other stale current fields could remain in the briefing, and deferred short-lived information relied on a less precise recency instruction.

User impact

Briefings no longer treat source data from many hours earlier as current, whether it arrives in the current collection or remains in deferred history. Active warnings, disaster tracking, and dated forecasts continue to use their own validity rules.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Current weather, air-quality, and allergen documents now use explicit observation times for two-hour freshness filtering. Stale current data is excluded, while forecasts, dated forecast requests, and valid alerts follow separate rules. Documentation and tests define and verify these behaviors.

Changes

Document freshness handling

Layer / File(s) Summary
Freshness rules and prompt contract
README.md, docs/design.md, docs/requirements.md, weather_briefing/data/system_prompt.txt, tests/test_prompts.py
Documents and prompt rules define the two-hour threshold, observation-time sources, boundary behavior, and exclusions for forecasts and valid alerts.
Collection freshness filtering
weather_briefing/application/collection.py
Collection derives per-document observation times, filters stale current documents, logs discarded items, and skips filtering for specified forecast dates.
Freshness behavior validation
tests/test_collection.py
Tests cover stale data removal, the inclusive boundary, allergen timestamps, forecast retention, dated forecasts, and ambiguous timezone handling.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ProviderSet
  participant collect_weather_documents
  participant SnapshotTimeResolver
  participant StaleCurrentFilter
  ProviderSet->>collect_weather_documents: provide WeatherContextSnapshot values
  collect_weather_documents->>SnapshotTimeResolver: attach observation times to documents
  SnapshotTimeResolver-->>collect_weather_documents: return timed SourceDocument values
  collect_weather_documents->>StaleCurrentFilter: filter current documents within two hours
  StaleCurrentFilter-->>collect_weather_documents: return eligible documents
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and clearly reflects the main change: filtering out stale weather-related data.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/drop-stale-air-quality-conflicts

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

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.86%. Comparing base (96a0735) to head (912a4a7).
⚠️ Report is 4 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #124   +/-   ##
=======================================
  Coverage   99.86%   99.86%           
=======================================
  Files         116      117    +1     
  Lines       12345    12435   +90     
  Branches      737      742    +5     
=======================================
+ Hits        12328    12418   +90     
  Misses         12       12           
  Partials        5        5           

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

@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@qodo-code-review

qodo-code-review Bot commented Jul 29, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 46 rules
✅ REVIEW.md

Grey Divider


Remediation recommended

1. design.md duplicates freshness requirements ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
docs/design.md adds detailed freshness and deferred-publication rules that are already stated as
product requirements in docs/requirements.md, duplicating functional requirements instead of
referencing them. This increases drift risk (one doc may be updated without the other) and violates
the design doc constraint to remain a current technical contract without restating requirements.
Code

docs/design.md[R138-140]

+多个来源同时提供当前资料时,应用层以最新资料为基准,只保留落后不超过两小时的当前来源文档。天气使用快照更新时间,空气质量和过敏原优先使用各自的观测时刻;一个字段过时不会连带删除同一来源仍然新鲜的其他字段。指定日期的预报不使用这项筛选。
+
+未发送文章和历史快照继续保留供累计变化判断。模型输入契约要求天气、气温、降水、风力和空气质量等短时信息在落后最新适用资料超过两小时后只作为变化历史,不得写入当前结论或触发补发;预警、灾害跟踪和指定日期预报仍按各自的有效性规则判断。
Relevance

●●● Strong

Team enforces “design.md shouldn’t restate requirements”; similar duplication trimmed/linked in PR
#92 (accepted).

PR-#92
PR-#102

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2141667 requires docs/design.md to be limited to the current technical contract
and not restate functional requirements already defined elsewhere. The added docs/design.md
paragraphs describe freshness/deferred behavior in detail, while docs/requirements.md now includes
the same two-hour freshness constraints, indicating duplication rather than a reference.

Rule 2141667: Keep docs/design.md limited to the current technical contract
docs/design.md[138-140]
docs/requirements.md[27-29]
docs/requirements.md[56-56]

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/design.md` now restates functional requirements (freshness cutoff, deferred handling) that are already defined in `docs/requirements.md`. The design doc should reference requirements rather than duplicating them.

## Issue Context
The new paragraphs in `docs/design.md` (freshness filtering and deferred publication constraints) substantially overlap with the newly updated bullets in `docs/requirements.md`.

## Fix Focus Areas
- docs/design.md[138-140]
- docs/requirements.md[27-29]
- docs/requirements.md[56-56]

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


2. Mixed tz datetimes can crash ✓ Resolved 🐞 Bug ☼ Reliability
Description
_filter_stale_current_documents() orders and compares observed_at timestamps without enforcing
timezone awareness, so a mix of naive and tz-aware pendulum.DateTime values can raise TypeError
during max()/comparisons and abort collection. This failure mode is introduced by the new
stale-filtering logic.
Code

weather_briefing/application/collection.py[R86-93]

+    available_times = tuple(observed_at for _, observed_at in timed_documents if observed_at is not None)
+    latest_time = max(available_times)
+    earliest_retained_time = latest_time.subtract(hours=_CURRENT_DOCUMENT_MAX_LAG_HOURS)
+    retained = tuple(
+        document
+        for document, observed_at in timed_documents
+        if observed_at is None or observed_at >= earliest_retained_time
+    )
Relevance

●●● Strong

Timezone-awareness guards are routinely required; PR #100 accepted adding require_aware_datetime
before datetime arithmetic/comparisons.

PR-#100

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new filter computes a threshold using max() and then performs datetime comparisons;
Python/Pendulum will raise when comparing naive vs aware datetimes. The codebase already provides
require_aware_datetime() and has tests asserting naive datetimes must be rejected, indicating
timezone-awareness is an expected contract.

weather_briefing/application/collection.py[82-103]
weather_briefing/time_utils.py[12-16]
tests/test_state.py[312-318]
PR-#100

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

### Issue description
`_filter_stale_current_documents()` assumes all `observed_at` values are timezone-aware and comparable. If any provider (or caller/fixture) passes a naive `pendulum.DateTime`, comparisons like `max(available_times)` and `observed_at >= earliest_retained_time` can raise at runtime.

### Issue Context
The repo already has a standard helper to reject naive timestamps (`require_aware_datetime`). Similar timezone-awareness guards were previously needed elsewhere in the codebase.

### Fix Focus Areas
- weather_briefing/application/collection.py[82-103]
- weather_briefing/application/collection.py[106-117]

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



Informational

3. Naive pendulum.datetime(..., tz=None) 📘 Rule violation ≡ Correctness ⭐ New
Description
The new test cases construct Pendulum datetimes with tz=None, creating naive timestamps. This
violates the requirement to use timezone-aware Pendulum DateTime objects and can normalize unsafe
patterns into the codebase.
Code

tests/test_collection.py[R256-278]

+            _snapshot(
+                "qweather",
+                pendulum.datetime(2026, 7, 27, 21, tz=None),
+                air_quality_effective_at=None,
+            ),
+            "Weather snapshot weather:qweather observation time",
+        ),
+        (
+            _snapshot(
+                "qweather",
+                pendulum.datetime(2026, 7, 27, 21, tz="Asia/Shanghai"),
+                air_quality_effective_at=pendulum.datetime(2026, 7, 27, 21, tz=None),
+            ),
+            "Air-quality snapshot air-quality:qweather observation time",
+        ),
+        (
+            _snapshot(
+                "qweather",
+                pendulum.datetime(2026, 7, 27, 21, tz="Asia/Shanghai"),
+                air_quality_effective_at=None,
+                include_allergen=True,
+                allergen_observed_at=pendulum.datetime(2026, 7, 27, 21, tz=None),
+            ),
Relevance

● Weak

Naive tz=None appears intentional in negative test; team enforces awareness via
require_aware_datetime (accepted in PR#100).

PR-#100

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2141709 requires Pendulum datetime constructions to be timezone-aware. The test
constructs multiple Pendulum datetimes with tz=None at the cited lines, which creates naive
datetimes.

Rule 2141709: Use timezone-aware Pendulum DateTime objects instead of naive datetimes
tests/test_collection.py[256-278]

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

## Issue description
`tests/test_collection.py` creates naive Pendulum datetimes via `pendulum.datetime(..., tz=None)`. The compliance rule requires timezone-aware Pendulum `DateTime` objects (and avoiding naive datetimes in general).

## Issue Context
These naive datetimes are only used to assert that `require_aware_datetime()` rejects missing timezone information.

## Fix Focus Areas
- tests/test_collection.py[256-278]

## Suggested approach
- Replace `pendulum.datetime(..., tz=None)` inputs with a small test stub object that has `tzinfo = None` (so the error path is still exercised) instead of constructing a naive datetime object.
- Keep the existing assertion on the error message/context so behavior remains covered.

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


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Previous review results

Review updated until commit 912a4a7

Results up to commit 2917a25 ⚖️ Balanced


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


Remediation recommended
1. design.md duplicates freshness requirements ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
docs/design.md adds detailed freshness and deferred-publication rules that are already stated as
product requirements in docs/requirements.md, duplicating functional requirements instead of
referencing them. This increases drift risk (one doc may be updated without the other) and violates
the design doc constraint to remain a current technical contract without restating requirements.
Code

docs/design.md[R138-140]

+多个来源同时提供当前资料时,应用层以最新资料为基准,只保留落后不超过两小时的当前来源文档。天气使用快照更新时间,空气质量和过敏原优先使用各自的观测时刻;一个字段过时不会连带删除同一来源仍然新鲜的其他字段。指定日期的预报不使用这项筛选。
+
+未发送文章和历史快照继续保留供累计变化判断。模型输入契约要求天气、气温、降水、风力和空气质量等短时信息在落后最新适用资料超过两小时后只作为变化历史,不得写入当前结论或触发补发;预警、灾害跟踪和指定日期预报仍按各自的有效性规则判断。
Relevance

●●● Strong

Team enforces “design.md shouldn’t restate requirements”; similar duplication trimmed/linked in PR
#92 (accepted).

PR-#92
PR-#102

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2141667 requires docs/design.md to be limited to the current technical contract
and not restate functional requirements already defined elsewhere. The added docs/design.md
paragraphs describe freshness/deferred behavior in detail, while docs/requirements.md now includes
the same two-hour freshness constraints, indicating duplication rather than a reference.

Rule 2141667: Keep docs/design.md limited to the current technical contract
docs/design.md[138-140]
docs/requirements.md[27-29]
docs/requirements.md[56-56]

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/design.md` now restates functional requirements (freshness cutoff, deferred handling) that are already defined in `docs/requirements.md`. The design doc should reference requirements rather than duplicating them.

## Issue Context
The new paragraphs in `docs/design.md` (freshness filtering and deferred publication constraints) substantially overlap with the newly updated bullets in `docs/requirements.md`.

## Fix Focus Areas
- docs/design.md[138-140]
- docs/requirements.md[27-29]
- docs/requirements.md[56-56]

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


2. Mixed tz datetimes can crash ✓ Resolved 🐞 Bug ☼ Reliability
Description
_filter_stale_current_documents() orders and compares observed_at timestamps without enforcing
timezone awareness, so a mix of naive and tz-aware pendulum.DateTime values can raise TypeError
during max()/comparisons and abort collection. This failure mode is introduced by the new
stale-filtering logic.
Code

weather_briefing/application/collection.py[R86-93]

+    available_times = tuple(observed_at for _, observed_at in timed_documents if observed_at is not None)
+    latest_time = max(available_times)
+    earliest_retained_time = latest_time.subtract(hours=_CURRENT_DOCUMENT_MAX_LAG_HOURS)
+    retained = tuple(
+        document
+        for document, observed_at in timed_documents
+        if observed_at is None or observed_at >= earliest_retained_time
+    )
Relevance

●●● Strong

Timezone-awareness guards are routinely required; PR #100 accepted adding require_aware_datetime
before datetime arithmetic/comparisons.

PR-#100

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new filter computes a threshold using max() and then performs datetime comparisons;
Python/Pendulum will raise when comparing naive vs aware datetimes. The codebase already provides
require_aware_datetime() and has tests asserting naive datetimes must be rejected, indicating
timezone-awareness is an expected contract.

weather_briefing/application/collection.py[82-103]
weather_briefing/time_utils.py[12-16]
tests/test_state.py[312-318]
PR-#100

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

### Issue description
`_filter_stale_current_documents()` assumes all `observed_at` values are timezone-aware and comparable. If any provider (or caller/fixture) passes a naive `pendulum.DateTime`, comparisons like `max(available_times)` and `observed_at >= earliest_retained_time` can raise at runtime.

### Issue Context
The repo already has a standard helper to reject naive timestamps (`require_aware_datetime`). Similar timezone-awareness guards were previously needed elsewhere in the codebase.

### Fix Focus Areas
- weather_briefing/application/collection.py[82-103]
- weather_briefing/application/collection.py[106-117]

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


Qodo Logo

@IceCodeNew
IceCodeNew marked this pull request as ready for review July 29, 2026 09:08
@qodo-code-review

Copy link
Copy Markdown

ⓘ Your Qodo trial ends soon. Ask your workspace admin to set up billing to keep reviews running after the trial. Manage billing

@qodo-code-review

qodo-code-review Bot commented Jul 29, 2026

Copy link
Copy Markdown

PR Summary by Qodo

Ignore stale current observations beyond 2-hour freshness window

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Drop current weather/air-quality/allergen documents older than two hours behind freshest
 observation.
• Enforce timezone-aware observation timestamps for current snapshots and raise on ambiguity.
• Document and test freshness rules, including deferred-content guidance and forecast-date
 exemptions.
Diagram

graph TD
  providers["Weather providers"] --> collect["collect_weather_documents"] --> attach["_snapshot_documents_with_times"] --> filter["_filter_stale_current_documents"] --> docs[("Current source docs")] --> llm["LLM briefing"]
  prompt["system_prompt.txt"] --> llm
  tests["tests/test_collection.py"] --> collect
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Filter at snapshot/provider level
  • ➕ Avoids generating SourceDocuments that will be discarded
  • ➕ Can drop entire stale provider snapshots in one decision
  • ➖ Harder to exempt specific documents (e.g., forecast AQ vs observation AQ) without duplicating logic
  • ➖ Less transparent logging/diagnostics at the document level
2. Use wall-clock 'now' freshness instead of 'freshest observation'
  • ➕ Simpler mental model: everything must be newer than now-2h
  • ➕ Independent of outlier providers reporting future-ish timestamps
  • ➖ Does not match the stated product rule (compare relative to latest available observation)
  • ➖ More likely to discard all sources during upstream delays even when relative comparisons are still meaningful

Recommendation: Keep the current document-level, freshest-observation-relative filter. It matches the product requirements (relative freshness across sources), supports per-document exemptions (forecast vs observation), and is backed by targeted tests for boundary conditions and timestamp validity.

Files changed (7) +378 / -6

Bug fix (1) +70 / -2
collection.pyFilter stale current documents using per-document observation times +70/-2

Filter stale current documents using per-document observation times

• Extends weather document collection to compute timezone-aware observation times per SourceDocument (weather, air-quality observation, allergen) and filter out those older than latest_time minus two hours. Skips filtering when a forecast_date is explicitly requested and logs discarded stale documents for observability.

weather_briefing/application/collection.py

Tests (2) +298 / -0
test_collection.pyAdd coverage for stale-current filtering, exemptions, and timezone validation +291/-0

Add coverage for stale-current filtering, exemptions, and timezone validation

• Introduces tests ensuring only stale current documents are dropped, boundary (exactly two hours) is retained, and allergens/AQ use their own observation times with weather fallback. Verifies forecast documents and dated forecast runs are exempt, and that naive (timezone-less) timestamps raise ValueError with contextual messages.

tests/test_collection.py

test_prompts.pyAssert prompt contains deferred freshness window guidance +7/-0

Assert prompt contains deferred freshness window guidance

• Adds a test that the system prompt includes explicit instructions not to publish deferred weather content older than the two-hour window, while preserving exemptions for active warnings, disaster tracking, and dated forecasts.

tests/test_prompts.py

Documentation (3) +7 / -2
README.mdDocument 2-hour freshness window for current source comparisons +2/-0

Document 2-hour freshness window for current source comparisons

• Adds user-facing documentation that current observations are only compared if within two hours of the freshest available data. Clarifies that stale weather/air-quality/allergen observations are excluded while explicitly requested forecast dates are unaffected.

README.md

design.mdSpecify per-document timestamps used for freshness comparison +2/-0

Specify per-document timestamps used for freshness comparison

• Documents which timestamps are used for freshness evaluation (weather snapshot update time, AQ/allergen observation times with fallback). Notes that the application layer enforces the unified freshness window and exempts dated forecasts and non-current semantics.

docs/design.md

requirements.mdDefine freshness rules for deferred and current multi-source data +3/-2

Define freshness rules for deferred and current multi-source data

• Updates requirements to state that deferred fast-expiring content older than two hours can only serve as history and cannot form current conclusions or trigger delivery. Adds/clarifies that only current data within two hours of the freshest source participates in conclusions, and manual runs should only send still-valid backlog.

docs/requirements.md

Other (1) +3 / -2
system_prompt.txtClarify two-hour expiry rules for deferred fast-changing context +3/-2

Clarify two-hour expiry rules for deferred fast-changing context

• Updates the system prompt to apply a two-hour freshness boundary to deferred fast-expiring weather-related information: expired items can only be used as change history and must not form current conclusions or trigger publishing. Explicitly preserves special validity rules for warnings, disasters, and dated forecasts.

weather_briefing/data/system_prompt.txt

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 0cb8a79

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@docs/requirements.md`:
- Line 56: Update the current-air-quality observation requirement in
docs/requirements.md to define “时刻接近” precisely: exclude observations more than
two hours older than the newest observation, while retaining observations
exactly two hours older. Keep the wording focused on user-visible product
behavior without adding implementation details.
🪄 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 Plus

Run ID: fa4fbc98-278f-4526-9c62-4895b30c0d7d

📥 Commits

Reviewing files that changed from the base of the PR and between 1444046 and 0cb8a79.

📒 Files selected for processing (6)
  • README.md
  • docs/design.md
  • docs/notes.md
  • docs/requirements.md
  • tests/test_collection.py
  • weather_briefing/application/collection.py

Comment thread docs/requirements.md Outdated
@IceCodeNew
IceCodeNew marked this pull request as draft July 29, 2026 09:57
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 9c0ea27

@IceCodeNew
IceCodeNew marked this pull request as ready for review July 29, 2026 10:10
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 9c0ea27

@IceCodeNew IceCodeNew changed the title fix: ignore stale air quality conflicts fix: ignore stale weather source snapshots Jul 29, 2026
@IceCodeNew
IceCodeNew marked this pull request as draft July 29, 2026 11:29
@IceCodeNew IceCodeNew changed the title fix: ignore stale weather source snapshots fix: ignore stale weather data Jul 29, 2026
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

Comment thread docs/design.md Outdated
Comment thread weather_briefing/application/collection.py
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 2917a25

@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@IceCodeNew
IceCodeNew marked this pull request as ready for review July 29, 2026 11:44

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/application/collection.py`:
- Around line 118-135: In weather_briefing/application/collection.py lines
118-135, update the air-quality and allergen handling in the snapshot collection
flow to remove the weather_observed_at fallback and exclude or reject current
sub-documents whose own observation timestamp is missing, while preserving
independently timestamped observations. In docs/design.md line 138, document
that air-quality and allergen freshness never falls back to the weather
snapshot. In tests/test_collection.py lines 188-201, replace the allergen
fallback expectation with exclusion of an allergen lacking observed_at and add
the equivalent exclusion case for air quality.
🪄 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 Plus

Run ID: ad985fa1-eb36-4799-ae32-cb02d2a0521f

📥 Commits

Reviewing files that changed from the base of the PR and between 9c0ea27 and 0fa6ed7.

📒 Files selected for processing (7)
  • README.md
  • docs/design.md
  • docs/requirements.md
  • tests/test_collection.py
  • tests/test_prompts.py
  • weather_briefing/application/collection.py
  • weather_briefing/data/system_prompt.txt
🚧 Files skipped from review as they are similar to previous changes (1)
  • README.md

Comment thread weather_briefing/application/collection.py
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 0fa6ed7

@IceCodeNew

Copy link
Copy Markdown
Owner Author

Qodo finding 3 reviewed; not adopting it. The tz=None values appear only in negative boundary tests whose purpose is to prove that ambiguous provider timestamps are rejected. Replacing them with timezone-aware values would stop exercising the failure contract. This follows the existing negative test pattern in tests/test_state.py; application inputs and successful-path fixtures remain timezone-aware.

@qodo-code-review

Copy link
Copy Markdown

Qodo finding 3 reviewed; not adopting it. The tz=None values appear only in negative boundary tests whose purpose is to prove that ambiguous provider timestamps are rejected. Replacing them with timezone-aware values would stop exercising the failure contract. This follows the existing negative test pattern in tests/test_state.py; application inputs and successful-path fixtures remain timezone-aware.

Understood. finding 3 is intentionally retained as a negative test for rejecting timezone-naive provider timestamps, consistent with the existing test pattern. No changes are needed.

@IceCodeNew
IceCodeNew merged commit 7c2c18c into master Jul 29, 2026
14 checks passed
@IceCodeNew
IceCodeNew deleted the codex/drop-stale-air-quality-conflicts branch July 29, 2026 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant