Skip to content

fix: clarify RSS failure alert periods - #56

Merged
IceCodeNew merged 1 commit into
masterfrom
codex/clarify-rss-failure-alert
Jul 17, 2026
Merged

fix: clarify RSS failure alert periods#56
IceCodeNew merged 1 commit into
masterfrom
codex/clarify-rss-failure-alert

Conversation

@IceCodeNew

Copy link
Copy Markdown
Owner

Summary

  • describe the RSS health threshold as consecutive scheduler runs
  • distinguish one scheduler failure period from the adapter HTTP attempts inside that run
  • cover the user-facing alert text directly

Root cause

The alert said a source had failed three times even though one failed scheduler run can contain several HTTP attempts, making the counter semantics ambiguous during incident response.

Validation

  • prek run --all-files
  • uv run --with pytest --with pytest-cov -- pytest --cov --cov-branch --cov-report=xml (593 passed)

@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in: 9 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: f26857f4-9b42-4fa2-8c16-5ec957cae6a6

📥 Commits

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

📒 Files selected for processing (3)
  • docs/requirements.md
  • tests/test_service.py
  • weather_briefing/service.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/clarify-rss-failure-alert

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 17, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #56      +/-   ##
==========================================
+ Coverage   99.64%   99.72%   +0.07%     
==========================================
  Files          38       38              
  Lines        6232     6436     +204     
  Branches      341      363      +22     
==========================================
+ Hits         6210     6418     +208     
+ Misses         15       13       -2     
+ Partials        7        5       -2     

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

@qodo-code-review

qodo-code-review Bot commented Jul 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 31 rules

Grey Divider


Remediation recommended

1. Alert count can underreport ✓ Resolved 🐞 Bug ≡ Correctness
Description
The RSS failure alert body interpolates the configured threshold as the “consecutive failed rounds”
count, but the alert selection triggers on consecutive_failures >= threshold. If alert publishing
fails and is retried after additional failing runs, the message will claim only N failed rounds even
though the state indicates more than N failures.
Code

weather_briefing/service.py[R205-207]

                "天气 RSS 源持续获取失败",
-                f"以下 RSS 源已连续失败 {self._settings.rss_failure_threshold} 次:{', '.join(rss_failure_alert_ids)}",
+                f"以下 RSS 源已连续 {self._settings.rss_failure_threshold} 个调度轮次获取失败:"
+                f"{', '.join(rss_failure_alert_ids)}",
Relevance

⭐⭐⭐ High

Team prioritizes accurate ops reporting; PRs #13 and #48 adjusted RSS alert/retry semantics to avoid
misleading counts.

PR-#13
PR-#48

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The alert body always uses the threshold value, while eligibility is based on `consecutive_failures
>= threshold` and an alert is only marked as delivered after a successful publish; therefore,
retries after a publish failure can have consecutive_failures > threshold while still rendering
the threshold as the count.

weather_briefing/service.py[199-209]
weather_briefing/service.py[367-377]
weather_briefing/state.py[549-567]

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 RSS failure alert message prints `rss_failure_threshold` as if it were the current consecutive failure count. However, sources are selected for alert when `consecutive_failures >= threshold`, and `failure_alerted_at` is only set after a successful publish. This makes the alert text incorrect (underreporting) when publishing fails and the alert is retried later.

### Issue Context
- The service selects sources for alert using `rss_sources_requiring_failure_alert(..., threshold)`.
- Publishing failures are swallowed and do not mark the alert as delivered, so the same sources will be retried on later runs.

### Fix Focus Areas
- weather_briefing/service.py[199-209]
- weather_briefing/service.py[367-377]
- weather_briefing/state.py[549-567]

### Suggested fix approaches
Pick one:
1) **Accurate counts:** Change `rss_sources_requiring_failure_alert` to return `(source_id, consecutive_failures)` (or add a new query method), and render the actual counts in the alert body.
2) **At least correct wording (minimal):** Change the body text to say “已连续**至少** {threshold} 个调度轮次获取失败” to avoid claiming an exact count.

Update/adjust affected tests accordingly (e.g., substring assertions).

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


Grey Divider

Previous review results

Review updated until commit a3f57e2

Results up to commit d489603


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


Remediation recommended
1. Alert count can underreport ✓ Resolved 🐞 Bug ≡ Correctness
Description
The RSS failure alert body interpolates the configured threshold as the “consecutive failed rounds”
count, but the alert selection triggers on consecutive_failures >= threshold. If alert publishing
fails and is retried after additional failing runs, the message will claim only N failed rounds even
though the state indicates more than N failures.
Code

weather_briefing/service.py[R205-207]

                "天气 RSS 源持续获取失败",
-                f"以下 RSS 源已连续失败 {self._settings.rss_failure_threshold} 次:{', '.join(rss_failure_alert_ids)}",
+                f"以下 RSS 源已连续 {self._settings.rss_failure_threshold} 个调度轮次获取失败:"
+                f"{', '.join(rss_failure_alert_ids)}",
Relevance

⭐⭐⭐ High

Team prioritizes accurate ops reporting; PRs #13 and #48 adjusted RSS alert/retry semantics to avoid
misleading counts.

PR-#13
PR-#48

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The alert body always uses the threshold value, while eligibility is based on `consecutive_failures
>= threshold` and an alert is only marked as delivered after a successful publish; therefore,
retries after a publish failure can have consecutive_failures > threshold while still rendering
the threshold as the count.

weather_briefing/service.py[199-209]
weather_briefing/service.py[367-377]
weather_briefing/state.py[549-567]

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 RSS failure alert message prints `rss_failure_threshold` as if it were the current consecutive failure count. However, sources are selected for alert when `consecutive_failures >= threshold`, and `failure_alerted_at` is only set after a successful publish. This makes the alert text incorrect (underreporting) when publishing fails and the alert is retried later.

### Issue Context
- The service selects sources for alert using `rss_sources_requiring_failure_alert(..., threshold)`.
- Publishing failures are swallowed and do not mark the alert as delivered, so the same sources will be retried on later runs.

### Fix Focus Areas
- weather_briefing/service.py[199-209]
- weather_briefing/service.py[367-377]
- weather_briefing/state.py[549-567]

### Suggested fix approaches
Pick one:
1) **Accurate counts:** Change `rss_sources_requiring_failure_alert` to return `(source_id, consecutive_failures)` (or add a new query method), and render the actual counts in the alert body.
2) **At least correct wording (minimal):** Change the body text to say “已连续**至少** {threshold} 个调度轮次获取失败” to avoid claiming an exact count.

Update/adjust affected tests accordingly (e.g., substring assertions).

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


Qodo Logo

Comment thread weather_briefing/service.py
@IceCodeNew
IceCodeNew force-pushed the codex/clarify-rss-failure-alert branch from d489603 to a3f57e2 Compare July 17, 2026 03:11
@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 a3f57e2

@IceCodeNew
IceCodeNew marked this pull request as ready for review July 17, 2026 03:24
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Clarify RSS failure alerts as consecutive scheduler runs

🐞 Bug fix 📝 Documentation 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Clarify that RSS failure thresholds count consecutive scheduler runs, not HTTP retry attempts.
• Update ops alert wording to explicitly reference “调度轮次” for incident-response clarity.
• Align tests and reliability docs with the new user-facing alert text.
Diagram

graph TD
  T["Test: rss failure alert"] --> S["Briefing service _run"] --> A["RSS adapter fetch"] --> ST["State: consecutive runs"] --> TH["Threshold check"] --> P["Ops publisher"] --> M["Alert text (调度轮次)"]
  T --> M
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Include both scheduler-run and HTTP-attempt counts in the alert
  • ➕ Removes any remaining ambiguity during incidents (operators see both dimensions)
  • ➕ Helps distinguish flaky network from repeated scheduler failures
  • ➖ Requires additional instrumentation/state plumbing to track attempts reliably
  • ➖ May increase alert verbosity and cognitive load if not carefully formatted
2. Link alerts to a specific run identifier/time window
  • ➕ Makes correlation with logs/traces faster during incident response
  • ➕ Avoids confusion when multiple runs happen close together
  • ➖ Requires consistent run-id generation and propagation into message formatting
  • ➖ Less universally helpful than fixing the core counter semantics

Recommendation: Keep the PR’s current approach (clarify the semantics in the user-facing text, docs, and tests) because it resolves the incident-response ambiguity without changing alerting behavior or state tracking. If future incidents still need more granularity, consider adding HTTP-attempt counts as an optional extra line in the alert body.

Files changed (3) +5 / -4

Bug fix (1) +2 / -1
service.pyClarify RSS failure alert body to reference scheduler runs +2/-1

Clarify RSS failure alert body to reference scheduler runs

• Adjusts the ops alert body string to say failures occurred across at least N scheduler runs (调度轮次), removing ambiguity with adapter-level HTTP retries.

weather_briefing/service.py

Tests (1) +1 / -1
test_service.pyAssert updated ops alert wording for RSS failure threshold +1/-1

Assert updated ops alert wording for RSS failure threshold

• Updates the RSS failure alert test expectation to match the clarified, user-facing message text that references consecutive scheduler runs.

tests/test_service.py

Documentation (1) +2 / -2
requirements.mdDocument RSS failure threshold as consecutive scheduler runs +2/-2

Document RSS failure threshold as consecutive scheduler runs

• Refines the reliability requirements to state that the RSS failure threshold counts consecutive scheduler runs. Clarifies that multiple HTTP attempts/retries inside a single run only increment the consecutive-failure counter once.

docs/requirements.md

@IceCodeNew
IceCodeNew merged commit 125a6a9 into master Jul 17, 2026
20 checks passed
@IceCodeNew
IceCodeNew deleted the codex/clarify-rss-failure-alert branch July 17, 2026 03:25
@qodo-code-review

Copy link
Copy Markdown

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

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