From 96e93b675c9a0bb924f026ef6fe0f7bcda53504c Mon Sep 17 00:00:00 2001 From: IceCodeNew <32576256+IceCodeNew@users.noreply.github.com> Date: Tue, 4 Aug 2026 10:54:11 +0800 Subject: [PATCH 1/2] chore(prek): bump ruff hook to v0.16.0 Align the prek ruff pin with the uv lockfile (ruff 0.16.0) so local hooks and CI agree. ruff 0.16 no longer raises BLE001 where the rendered -text diagnostic guard logs and swallows, which lets the unused noqa directives be removed in the follow-up commit. --- prek.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prek.toml b/prek.toml index bb05f338..9792eed3 100644 --- a/prek.toml +++ b/prek.toml @@ -33,7 +33,7 @@ hooks = [{ id = "gitleaks" }] [[repos]] repo = "https://github.com/astral-sh/ruff-pre-commit" -rev = "01a675ea018f2fb714478a5ffb83fcea8374bb06" # frozen: v0.15.21 +rev = "cb8c523fd4835aba42af70f4cad5568db4df0b6c" # frozen: v0.16.0 hooks = [ { id = "ruff-check", args = ["--fix"] }, { id = "ruff-format" } From e5f4e7d8648a1f22329fc1f4463cc19a52cacee4 Mon Sep 17 00:00:00 2001 From: IceCodeNew <32576256+IceCodeNew@users.noreply.github.com> Date: Tue, 4 Aug 2026 10:54:22 +0800 Subject: [PATCH 2/2] refactor: resolve ruff stable lint findings (PLR0917, ISC004, RUF100) PR #134 enabled all stable ruff rules. Resolve the resulting findings by fixing the underlying structure rather than suppressing, except where a real fix is disproportional. PLR0917 (too many positional arguments): - service-status state store + protocol now take the ServiceStatusMessage object instead of unpacking five message fields; the monitor stops hand-unpacking messages at every call site. - BriefingService.__init__ is keyword-only (single production call site). - ServiceStatusMonitor moves the optional language flag to keyword-only. - The parametrized any-llm factory test uses keyword-only parameters. - _briefing_service keeps positional form (37 call sites) and is the only justified PLR0917 suppression. ISC004: wrap implicitly concatenated strings in collections with parens. RUF100: drop the two BLE001 noqas that ruff 0.16 no longer needs. --- tests/test_any_llm_provider.py | 1 + tests/test_geocoding.py | 16 +++--- tests/test_service.py | 39 +++++++------- tests/test_service_status.py | 51 +++++-------------- weather_briefing/cli.py | 20 ++++---- weather_briefing/delivery/base.py | 2 +- .../delivery/telegram_renderer.py | 6 ++- weather_briefing/llm/any_llm.py | 2 +- .../persistence/service_status.py | 44 ++++++++-------- weather_briefing/service.py | 1 + weather_briefing/service_status/monitor.py | 42 +++------------ 11 files changed, 91 insertions(+), 133 deletions(-) diff --git a/tests/test_any_llm_provider.py b/tests/test_any_llm_provider.py index 6ba89002..efb4c69c 100644 --- a/tests/test_any_llm_provider.py +++ b/tests/test_any_llm_provider.py @@ -299,6 +299,7 @@ async def test_any_llm_provider_assesses_notification_value_with_a_narrow_schema ) async def test_factory_normalizes_provider_native_request_errors( # noqa: PLR0913 monkeypatch, + *, provider_name: str, error_factory: Callable[[httpx.Response], Exception], operation: str, diff --git a/tests/test_geocoding.py b/tests/test_geocoding.py index 472e3d2b..994d6ee5 100644 --- a/tests/test_geocoding.py +++ b/tests/test_geocoding.py @@ -501,9 +501,11 @@ def handler(request: httpx.Request) -> httpx.Response: [ '"invalid"', '{"id":"example","name":"Example"}', - '{"id":"example","name":"Example","latitude":1,"longitude":2,' - '"country_code":null,"administrative_area":null,"timezone":null,' - '"is_mainland_china":false,"summary_language":"english"}', + ( + '{"id":"example","name":"Example","latitude":1,"longitude":2,' + '"country_code":null,"administrative_area":null,"timezone":null,' + '"is_mainland_china":false,"summary_language":"english"}' + ), ], ) async def test_resolver_rejects_invalid_cached_reverse_record(tmp_path: Path, cached: str) -> None: @@ -527,9 +529,11 @@ async def test_resolver_rejects_invalid_cached_reverse_record(tmp_path: Path, ca "cached", [ '{"id":"example","name":"Example"}', - '{"id":"example","name":"Example","latitude":1,"longitude":2,' - '"country_code":null,"administrative_area":null,"timezone":null,' - '"is_mainland_china":false,"summary_language":"english"}', + ( + '{"id":"example","name":"Example","latitude":1,"longitude":2,' + '"country_code":null,"administrative_area":null,"timezone":null,' + '"is_mainland_china":false,"summary_language":"english"}' + ), ], ) async def test_resolver_rejects_obsolete_cache_record(tmp_path: Path, cached: str) -> None: diff --git a/tests/test_service.py b/tests/test_service.py index c726bc47..beb3036c 100644 --- a/tests/test_service.py +++ b/tests/test_service.py @@ -238,7 +238,8 @@ def notification_decisions(self) -> NotificationDecisionProvider: ... -def _briefing_service( # noqa: PLR0913 +# Test factory mirroring BriefingService; kept positional across 37 call sites. +def _briefing_service( # noqa: PLR0913, PLR0917 settings: _TestSettings, location: ResolvedLocation, state: SQLiteStateStore, @@ -252,15 +253,15 @@ def _briefing_service( # noqa: PLR0913 llm.notification_decisions if isinstance(llm, _NotificationDecisionOwner) else RecordingNotificationDecisions() ) return _BriefingService( - settings, - location, - state, - rss_source, - llm, - notification_decisions, - delivery, - ops_delivery, - weather_context_provider, + settings=settings, + location=location, + state=state, + rss_source=rss_source, + llm=llm, + notification_decisions=notification_decisions, + delivery=delivery, + ops_delivery=ops_delivery, + weather_context_provider=weather_context_provider, ) @@ -1541,15 +1542,15 @@ async def test_forced_audible_briefing_does_not_depend_on_notification_decision( with SQLiteStateStore(tmp_path / "forced-audible.sqlite3") as state: service = _BriefingService( - settings, - _location(), - state, - EmptyRSSSource(), - llm, - decision_provider, - delivery, - delivery, - StaticWeatherContextProvider(), + settings=settings, + location=_location(), + state=state, + rss_source=EmptyRSSSource(), + llm=llm, + notification_decisions=decision_provider, + delivery=delivery, + ops_delivery=delivery, + weather_context_provider=StaticWeatherContextProvider(), ) body = await service.run( "briefing", diff --git a/tests/test_service_status.py b/tests/test_service_status.py index e2585f81..cf64853e 100644 --- a/tests/test_service_status.py +++ b/tests/test_service_status.py @@ -574,7 +574,7 @@ async def test_mismatched_official_language_is_translated(tmp_path: Path) -> Non ) with SQLiteStateStore(tmp_path / "state.sqlite3") as state: await ServiceStatusMonitor( - (provider,), state.service_status, (("test", delivery),), decision, translator, "zh-CN" + (provider,), state.service_status, (("test", delivery),), decision, translator, language="zh-CN" ).run(pendulum.now("UTC")) translator.translate_service_status.assert_awaited_once_with( @@ -597,7 +597,7 @@ async def test_translation_failure_falls_back_to_official_text(tmp_path: Path, c caplog.at_level("WARNING", logger="weather_briefing.service_status"), ): await ServiceStatusMonitor( - (provider,), state.service_status, (("test", delivery),), decision, translator, "zh-CN" + (provider,), state.service_status, (("test", delivery),), decision, translator, language="zh-CN" ).run(pendulum.now("UTC")) delivery.publish_alert.assert_awaited_once_with( @@ -634,22 +634,12 @@ def test_state_rejects_handling_a_changed_observation(tmp_path: Path) -> None: with SQLiteStateStore(tmp_path / "state.sqlite3") as state: state.service_status.observe_service_status_message( "source", - "incident", - "new", - "Title", - "monitoring", - "Body", - now, + _message(incident_id="incident", revision_id="new", title="Title", status="monitoring", body="Body"), ) with pytest.raises(RuntimeError, match="changed before handling"): state.service_status.mark_service_status_message_handled( "source", - "incident", - "old", - "Title", - "monitoring", - "Body", - (ServiceSurface.API,), + _message(incident_id="incident", revision_id="old", title="Title", status="monitoring", body="Body"), now, ) @@ -658,25 +648,15 @@ def test_state_rejects_invalid_stored_service_status_surfaces(tmp_path: Path) -> now = pendulum.now("UTC") state_path = tmp_path / "state.sqlite3" with SQLiteStateStore(state_path) as state: - state.service_status.observe_service_status_message( - "source", - "incident", - "revision", - "Title", - "monitoring", - "Body", - now, - ) - state.service_status.mark_service_status_message_handled( - "source", - "incident", - "revision", - "Title", - "monitoring", - "Body", - (ServiceSurface.API,), - now, + message = _message( + incident_id="incident", + revision_id="revision", + title="Title", + status="monitoring", + body="Body", ) + state.service_status.observe_service_status_message("source", message) + state.service_status.mark_service_status_message_handled("source", message, now) for stored_value, message in ( ("{}", "must be a list"), ("[1]", "must contain strings"), @@ -698,12 +678,7 @@ def test_state_rejects_deciding_a_changed_observation(tmp_path: Path) -> None: with SQLiteStateStore(tmp_path / "state.sqlite3") as state: state.service_status.observe_service_status_message( "source", - "incident", - "new", - "Title", - "monitoring", - "Body", - pendulum.now("UTC"), + _message(incident_id="incident", revision_id="new", title="Title", status="monitoring", body="Body"), ) with pytest.raises(RuntimeError, match="changed before its decision"): state.service_status.mark_service_status_message_decided( diff --git a/weather_briefing/cli.py b/weather_briefing/cli.py index fbb9a1a9..f6955910 100644 --- a/weather_briefing/cli.py +++ b/weather_briefing/cli.py @@ -158,20 +158,20 @@ async def _run_unlocked( briefing_sent_today=briefing_sent_today, ) service = BriefingService( - settings, - location, - state, - RSSSource( + settings=settings, + location=location, + state=state, + rss_source=RSSSource( client, max_attempts=settings.rss_max_attempts, retry_min_seconds=settings.rss_retry_min_seconds, retry_max_seconds=settings.rss_retry_max_seconds, ), - llm_provider, - notification_decisions, - delivery, - delivery, - _weather_context_provider(settings, client, location), + llm=llm_provider, + notification_decisions=notification_decisions, + delivery=delivery, + ops_delivery=delivery, + weather_context_provider=_weather_context_provider(settings, client, location), ) body = await service.run( kind, @@ -220,7 +220,7 @@ async def run_service_status() -> None: deliveries, notification_decisions, service_status_llm, - settings.service_status_language, + language=settings.service_status_language, ) published = await monitor.run(pendulum.now(settings.timezone)) _LOGGER.info("Service-status run published %d notification(s)", published) diff --git a/weather_briefing/delivery/base.py b/weather_briefing/delivery/base.py index ddc508f4..53409d6c 100644 --- a/weather_briefing/delivery/base.py +++ b/weather_briefing/delivery/base.py @@ -189,7 +189,7 @@ def rendered_text_logging_enabled(diagnostics: RenderedTextDiagnostics | None) - return False try: enabled = diagnostics.rendered_text_logging_enabled() - except Exception: # noqa: BLE001 + except Exception: _LOGGER.warning("Rendered text diagnostic state check failed", exc_info=True) return False return enabled and _LOGGER.isEnabledFor(logging.DEBUG) diff --git a/weather_briefing/delivery/telegram_renderer.py b/weather_briefing/delivery/telegram_renderer.py index bb161392..01d1d09e 100644 --- a/weather_briefing/delivery/telegram_renderer.py +++ b/weather_briefing/delivery/telegram_renderer.py @@ -31,8 +31,10 @@ def render_briefing( } source_links.update({document.id: _html_link(document.url, document.name) for document in context}) lines = [ - f"{_html_text(result.headline)} " - f"{_html_attribution(result.headline_source_ids, source_links, labels)}", + ( + f"{_html_text(result.headline)} " + f"{_html_attribution(result.headline_source_ids, source_links, labels)}" + ), "", ] lines.extend(_html_items(labels["weather"], result.conclusions, source_links, labels)) diff --git a/weather_briefing/llm/any_llm.py b/weather_briefing/llm/any_llm.py index 1a5a0dd5..1d31d5ec 100644 --- a/weather_briefing/llm/any_llm.py +++ b/weather_briefing/llm/any_llm.py @@ -239,7 +239,7 @@ def _sensitive_llm_diagnostics_enabled(diagnostics: SensitiveLLMDiagnostics | No return False try: return diagnostics.rendered_text_logging_enabled() - except Exception: # noqa: BLE001 + except Exception: _LOGGER.warning("Sensitive LLM diagnostic state check failed", exc_info=True) return False diff --git a/weather_briefing/persistence/service_status.py b/weather_briefing/persistence/service_status.py index a29f7eaa..94f273be 100644 --- a/weather_briefing/persistence/service_status.py +++ b/weather_briefing/persistence/service_status.py @@ -15,6 +15,8 @@ import pendulum + from weather_briefing.service_status.models import ServiceStatusMessage + @dataclass(frozen=True, slots=True) class ServiceStatusMessageState: @@ -86,15 +88,10 @@ def service_status_message_state( handled_surfaces=_stored_surfaces(row["handled_surfaces"]), ) - def observe_service_status_message( # noqa: PLR0913 + def observe_service_status_message( self, source_id: str, - incident_id: str, - revision_id: str, - title: str, - status: str, - body: str, - observed_at: pendulum.DateTime, + message: ServiceStatusMessage, ) -> None: """Persist an official message without claiming handling succeeded.""" self._connection.execute( @@ -108,7 +105,15 @@ def observe_service_status_message( # noqa: PLR0913 observed_status = excluded.observed_status, observed_body = excluded.observed_body, observed_at = excluded.observed_at""", - (source_id, incident_id, revision_id, title, status, body, storage_time(observed_at)), + ( + source_id, + message.incident_id, + message.revision_id, + message.title, + message.status, + message.body, + storage_time(message.published_at), + ), ) self._connection.commit() @@ -165,15 +170,10 @@ def mark_service_status_message_delivered( ) self._connection.commit() - def mark_service_status_message_handled( # noqa: PLR0913 + def mark_service_status_message_handled( self, source_id: str, - incident_id: str, - revision_id: str, - title: str, - status: str, - body: str, - surfaces: tuple[ServiceSurface, ...], + message: ServiceStatusMessage, handled_at: pendulum.DateTime, ) -> None: """Mark one observed message as delivered or intentionally skipped.""" @@ -187,15 +187,15 @@ def mark_service_status_message_handled( # noqa: PLR0913 handled_at = ? WHERE source_id = ? AND incident_id = ? AND observed_revision_id = ?""", ( - revision_id, - title, - status, - body, - json.dumps([surface.value for surface in surfaces], ensure_ascii=False, separators=(",", ":")), + message.revision_id, + message.title, + message.status, + message.body, + json.dumps([surface.value for surface in message.surfaces], ensure_ascii=False, separators=(",", ":")), storage_time(handled_at), source_id, - incident_id, - revision_id, + message.incident_id, + message.revision_id, ), ) if cursor.rowcount != 1: diff --git a/weather_briefing/service.py b/weather_briefing/service.py index 9f6bdae7..94c6fab5 100644 --- a/weather_briefing/service.py +++ b/weather_briefing/service.py @@ -47,6 +47,7 @@ class BriefingService: def __init__( # noqa: PLR0913 self, + *, settings: BriefingSettings, location: ResolvedLocation, state: SQLiteStateStore, diff --git a/weather_briefing/service_status/monitor.py b/weather_briefing/service_status/monitor.py index 910fd95a..a50a655c 100644 --- a/weather_briefing/service_status/monitor.py +++ b/weather_briefing/service_status/monitor.py @@ -17,7 +17,7 @@ from weather_briefing.notification_decision import NotificationDecisionProvider from weather_briefing.persistence.service_status import ServiceStatusMessageState - from .models import ServiceStatusMessage, ServiceStatusSnapshot, ServiceSurface + from .models import ServiceStatusMessage, ServiceStatusSnapshot from .statuspage import ServiceStatusProvider _LOGGER = logging.getLogger("weather_briefing.service_status") @@ -38,28 +38,18 @@ def service_status_message_state( """Return the durable state for one incident.""" ... - def observe_service_status_message( # noqa: PLR0913 + def observe_service_status_message( self, source_id: str, - incident_id: str, - revision_id: str, - title: str, - status: str, - body: str, - observed_at: pendulum.DateTime, + message: ServiceStatusMessage, ) -> None: """Record an official message before evaluating or delivering it.""" ... - def mark_service_status_message_handled( # noqa: PLR0913 + def mark_service_status_message_handled( self, source_id: str, - incident_id: str, - revision_id: str, - title: str, - status: str, - body: str, - surfaces: tuple[ServiceSurface, ...], + message: ServiceStatusMessage, handled_at: pendulum.DateTime, ) -> None: """Record successful delivery or an intentional skip.""" @@ -128,6 +118,7 @@ def __init__( # noqa: PLR0913 deliveries: tuple[tuple[str, ServiceStatusDelivery], ...], decision_provider: NotificationDecisionProvider, translator: ServiceStatusTranslator | None = None, + *, language: str = "en", ) -> None: """Configure providers, durable state, notification policy, and delivery.""" @@ -164,15 +155,7 @@ async def _process_message( previous = self._state.service_status_message_state(snapshot.source_id, message.incident_id) if previous is not None and previous.handled_revision_id == message.revision_id: return False - self._state.observe_service_status_message( - snapshot.source_id, - message.incident_id, - message.revision_id, - message.title, - message.status, - message.body, - message.published_at, - ) + self._state.observe_service_status_message(snapshot.source_id, message) if previous is None and message.status == "resolved": self._mark_handled(snapshot, message, now) return False @@ -223,16 +206,7 @@ def _mark_handled( message: ServiceStatusMessage, now: pendulum.DateTime, ) -> None: - self._state.mark_service_status_message_handled( - snapshot.source_id, - message.incident_id, - message.revision_id, - message.title, - message.status, - message.body, - message.surfaces, - now, - ) + self._state.mark_service_status_message_handled(snapshot.source_id, message, now) async def _localized_message(self, message: ServiceStatusMessage) -> tuple[str, str]: if self._translator is None or official_message_matches(message, self._language):