Skip to content

fix(logging): use English operational alerts - #89

Merged
IceCodeNew merged 2 commits into
masterfrom
codex/english-operational-logs
Jul 22, 2026
Merged

fix(logging): use English operational alerts#89
IceCodeNew merged 2 commits into
masterfrom
codex/english-operational-logs

Conversation

@IceCodeNew

@IceCodeNew IceCodeNew commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • render all application-owned operational alerts in English
  • preserve user-selected output and opaque provider or location data in its original language
  • document the English logging requirement in AGENTS.md

Root cause

Operational alerts were written in Chinese even though they are diagnostic output. When stdout delivery is configured, those alerts are printed directly and become mixed-language operational logs. PR #88 placed the existing task-failure alert behind a new delivery-availability branch, which exposed the inconsistency in the changed control flow.

Repository audit

  • converted task failure, repeated RSS failure, stale RSS source, context budget, and reduced-precision location alerts to English
  • confirmed all application logger templates are English
  • left LLM prompts, user-selected briefing output, regional source content, and opaque provider or location values unchanged

Review fixes

  • replaced precise coordinates in the new fallback test with public dummy values
  • Qodo: 0 bugs, 0 rule violations; coordinate finding resolved on ef7c232
  • CodeRabbit: 0 actionable comments on the complete five-file diff at 3ce3ebc; the test-only follow-up review was rate limited

Validation

  • prek run --all-files
  • 868 passed
  • line coverage: 99.85% (master: 99.85%)
  • branch coverage: 99.54% (master: 99.52%)

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The change standardizes application-owned alert and operational messages in English. CLI location notices add an English fallback for unmatched names, while service alerts preserve dynamic source IDs, thresholds, and diagnostic details. Tests now assert the updated wording.

Changes

English alert messaging

Layer / File(s) Summary
CLI precision notices
AGENTS.md, weather_briefing/cli.py, tests/test_cli.py
Precision-reduction alert titles and explanatory notices use English wording, with a fallback when no matched location name is available.
Service operational alerts
weather_briefing/service.py, tests/test_service.py
Task failure, RSS health, stale-source, and context-budget alerts use English wording while retaining dynamic details and updated assertions.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: 🕐 20-40 Minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 clearly describes the main change: switching operational alerts to English.
✨ 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/english-operational-logs

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.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Use English for operational alerts while preserving user/provider language

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

Grey Divider

AI Description

• Render application-owned operational alerts in English for consistent ops logs
• Preserve user-selected output and opaque provider/location payloads without translation
• Update tests and contributor guidance to enforce the English-alert requirement
Diagram

graph TD
  U["User run / scheduler"] --> CLI["CLI runner"] --> SVC["Briefing service"] --> OPS["Ops delivery"] --> DEST["Alert destinations"]
  CFG[("locations.json")] --> CLI
  CLI -->|"precision-reduced match"| OPS
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Centralize ops-alert strings as constants/templates
  • ➕ Prevents future drift between title/body phrasing across call sites
  • ➕ Simplifies enforcing the “English ops alerts” rule
  • ➖ Slightly more indirection for a small number of messages
  • ➖ May be premature if alert surface stays small
2. Introduce i18n keys + language selection for ops logs
  • ➕ Scales cleanly if ops alerts later need localization
  • ➕ Keeps business logic independent of message text
  • ➖ Adds new infrastructure and maintenance burden
  • ➖ Overkill given the explicit requirement to standardize on English for ops alerts

Recommendation: The PR’s direct conversion to English is the right choice given the explicit operational requirement and limited message surface. If more ops alerts are added, consider centralizing alert templates (constants) to keep enforcement and phrasing consistent without adopting a full i18n layer.

Files changed (5) +54 / -22

Bug fix (2) +18 / -14
cli.pyTranslate location confirmation alert and notice template to English +6/-5

Translate location confirmation alert and notice template to English

• Updates the location precision-reduction operational alert title to English. Converts the user-facing operational notice template (including the missing matched-name fallback) to English while keeping interpolated location data intact.

weather_briefing/cli.py

service.pyTranslate ops alerts for task failures, RSS health, and context budget +12/-9

Translate ops alerts for task failures, RSS health, and context budget

• Converts operational alert titles and bodies from Chinese to English for task failure alerts, repeated RSS failure alerts, stale RSS source alerts, and context budget overflow alerts. Keeps dynamic/opaque source identifiers appended without translation.

weather_briefing/service.py

Tests (2) +35 / -8
test_cli.pyUpdate CLI precision-reduction alert assertions and add fallback coverage +21/-2

Update CLI precision-reduction alert assertions and add fallback coverage

• Switches the precision-reduction notice expectation from Chinese to English. Adds a new test ensuring an English fallback string is used when no matched name is present, and tightens alert title/body assertions.

tests/test_cli.py

test_service.pyUpdate service ops-alert tests to assert English titles/bodies +14/-6

Update service ops-alert tests to assert English titles/bodies

• Rewrites assertions for context budget, task failure, stale RSS, and repeated RSS failure alerts to validate English phrasing while still preserving embedded opaque identifiers. Uses startswith/contains checks to avoid over-coupling to full message text where appropriate.

tests/test_service.py

Documentation (1) +1 / -0
AGENTS.mdDocument English-only requirement for ops alerts +1/-0

Document English-only requirement for ops alerts

• Adds an explicit guideline that application-owned log messages and operational alerts must be written in English. Clarifies that user-selected output and opaque provider/user payloads should not be translated for logging.

AGENTS.md

@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.79%. Comparing base (957419c) to head (ef7c232).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #89   +/-   ##
=======================================
  Coverage   99.79%   99.79%           
=======================================
  Files          45       45           
  Lines        8908     8917    +9     
  Branches      538      538           
=======================================
+ Hits         8890     8899    +9     
  Misses         13       13           
  Partials        5        5           

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

@qodo-code-review

qodo-code-review Bot commented Jul 22, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 39 rules

Grey Divider


Remediation recommended

1. Real GPS coordinates in tests ✓ Resolved 📘 Rule violation ⛨ Security
Description
The newly added test constructs a ResolvedLocation with precise real-world latitude/longitude
values, which can expose sensitive location data through test output when assertions fail. Tests
should use clearly synthetic/public example coordinates or lower-precision dummy values.
Code

tests/test_cli.py[R214-225]

+def test_precision_reduction_notice_uses_english_fallback_for_missing_match() -> None:
+    location = ResolvedLocation(
+        "example",
+        "Test City",
+        39.911389,
+        116.380556,
+        "CN",
+        "Beijing",
+        "Asia/Shanghai",
+        True,
+        precision_reduced=True,
+    )
Relevance

⭐⭐⭐ High

PR #80 removed realistic lat/lon test literals, replacing with dummy 1.0/1.0 for privacy compliance.

PR-#80

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
tests/test_cli.py adds a new test that embeds precise GPS coordinates in a committed test fixture,
which violates the rules prohibiting hard-coded private runtime data and sensitive identifiers in
tests.

Rule 2141693: Disallow hard-coded secrets and private runtime data in committed code and tests
Rule 2141750: Tests must not log or assert on sensitive real-world secrets or identifiers
tests/test_cli.py[214-225]

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

## Issue description
A new test hard-codes precise latitude/longitude values (`39.911389`, `116.380556`). This risks committing sensitive real-world location identifiers and can leak them via failing assertion output.

## Issue Context
Compliance requires avoiding hard-coded private runtime data (including precise real-world coordinates) in committed code/tests, and avoiding tests that log/assert on such identifiers.

## Fix Focus Areas
- tests/test_cli.py[214-225]

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


Grey Divider

Qodo Logo

Comment thread tests/test_cli.py
@IceCodeNew
IceCodeNew merged commit 6b40212 into master Jul 22, 2026
18 checks passed
@IceCodeNew
IceCodeNew deleted the codex/english-operational-logs branch July 22, 2026 01:56
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