diff --git a/README.md b/README.md index 64b7c683..ce96c796 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ - 标题匹配配置规则的权威预报文章经 HTML 与页面噪声清洗后完整独立转发，并进入后续预报上下文。 - 日报通过可组合 provider 加入 API 天气预报、AQI、指数标准、PM2.5 浓度、生活指数及花粉过敏原信息，并用于穿衣、运动和口罩建议。 - 中国大陆天气默认使用 QWeather、Open-Meteo 的降级顺序，其他地区默认只使用 Open-Meteo；也可通过 `WEATHER_PROVIDERS` 显式指定主要来源和其他备用来源。 -- 支持多个关注地点；只给地名时通过 Open-Meteo Geocoding 解析并缓存坐标与国家信息，已有坐标时不发起地理编码请求。 +- 支持多个关注地点；只给地名时正向解析并缓存坐标与国家信息，只给坐标时反查地点名和行政信息，名称与坐标齐全时不发起定位请求。 - 完整地名无法解析时按可配置规则逐级降低查询精度；首次匹配会投递实际匹配地名和坐标，请用户确认并写回私密地点文件。 - 天气来源缺少空气质量时才使用可选 AQICN；两者都无法提供空气质量时给出明确配置错误。 - RSS 来源从可选 `rss-sources.json` 加载；没有该文件时小时任务仍由天气 API 正常运行。 @@ -35,7 +35,7 @@ cp locations.example.json locations.json uv run --frozen weather-briefing run briefing ``` -`env.example` 将必填项、条件必填项和选填项分别写在注释中，所有凭据和投递标识均为无效占位值。复制 `locations.example.json` 为被 Git 忽略的 `locations.json` 后可配置多个地点；示例使用北京市西城区中南海的公开坐标。每项必须有稳定 `id` 和 `name`，`latitude` 与 `longitude` 可同时删除，此时程序用 Open-Meteo Geocoding 解析并把结果缓存到 `state/`。 +`env.example` 将必填项、条件必填项和选填项分别写在注释中，所有凭据和投递标识均为无效占位值。复制 `locations.example.json` 为被 Git 忽略的 `locations.json` 后可配置多个地点；示例使用北京市西城区中南海的公开坐标。每项必须有稳定 `id`，并在 `name` 与成对的 `latitude`、`longitude` 之间至少提供一项：只有名称时程序正向解析并支持降精度回退，只有坐标时通过 Nominatim 反查规范地点名和行政信息，两者都有时不发起定位请求。解析结果缓存到 `state/`。 `LLM_PROVIDER=deepseek` 使用 `DEEPSEEK_API_KEY`、`DEEPSEEK_MODEL` 和可选的 `DEEPSEEK_BASE_URL`；DeepSeek provider 已预置官方 Base URL。`LLM_PROVIDER=openai-compatible` 使用 `LLM_API_KEY`、`LLM_MODEL` 和 `LLM_BASE_URL`。两套配置互不回退。 @@ -49,7 +49,7 @@ weather-briefing diagnostics rendered-text disable 容器部署通过同一运行实例执行，例如 `docker exec weather-briefing weather-briefing diagnostics rendered-text enable --for 15m`。该开关最长启用 24 小时并自动过期，状态保存在 `BRIEFING_STATE_PATH`。只有同时启用 `DEBUG` 和临时开关时才记录正文；日志包含简报、告警、权威预报以及 Telegram 分片的完整文本，可能暴露来源内容、来源 URL、坐标和其他位置上下文，排障后应立即关闭并妥善保护日志。token、chat ID 和请求 endpoint 不会写入这些诊断日志。 -定位层从地名解析国家或行政区代码。Open-Meteo 负责城市/邮编查询，空结果时由 OpenStreetMap Nominatim 解析详细地名；结果会持久缓存。已有坐标时使用中国大陆服务范围四至宽松包围盒作快速可能性判断。省略 `WEATHER_PROVIDERS` 时，中国大陆地点使用 QWeather、Open-Meteo，其他地点只使用 Open-Meteo；显式配置时首项是主要来源，后续项依次作为备用。 +定位层把名称或坐标补全为统一地点信息。Open-Meteo 负责城市/邮编正向查询，空结果时由 OpenStreetMap Nominatim 解析详细地名；只有坐标时由 Nominatim 反向查询规范地点名、国家和行政区。名称与坐标齐全时不请求定位服务，并使用中国大陆服务范围四至宽松包围盒作快速可能性判断；所有查询结果都会持久缓存。省略 `WEATHER_PROVIDERS` 时，中国大陆地点使用 QWeather、Open-Meteo，其他地点只使用 Open-Meteo；显式配置时首项是主要来源，后续项依次作为备用。 RSS 为可选补充数据。需要使用时复制 `rss-sources.example.json` 为被 Git 忽略的 `rss-sources.json` 并填写真实来源；其中 `name` 使用公众号、微博账号或发布机构等会显示给用户的公开名称。不创建该文件即可只使用天气 API。 diff --git a/docs/design.md b/docs/design.md index 4604f78e..13fd7c3b 100644 --- a/docs/design.md +++ b/docs/design.md @@ -28,9 +28,9 @@ CLI 根据 `BRIEFING_CRON` 判断当天最后一个 briefing 小时，并在对 ## 天气、空气质量与生活指数上下文 -`LocationSpec` 来自被 Git 忽略的 `BRIEFING_LOCATIONS_FILE`，支持多个地点且强制稳定 `id` 和完整 `name`。`CachedLocationResolver` 对已有经纬度直接构造 `ResolvedLocation`；缺少经纬度时调用 `GeocodingProvider`，并把解析结果写入运行状态目录。正向路径由 `PrecisionReducingGeocodingProvider` 包装 `FallbackGeocodingProvider`，先让 Open-Meteo 和 Nominatim 依次尝试完整名称，全部失败后才按数据驱动规则逐级降低查询精度。 +`LocationSpec` 来自被 Git 忽略的 `BRIEFING_LOCATIONS_FILE`，支持多个地点并强制稳定 `id`；用户必须在 `name` 与完整经纬度之间至少提供一项。`CachedLocationResolver` 将三种输入收敛为名称和坐标都完整的 `ResolvedLocation`：只有名称时执行正向解析，只有坐标时执行反向解析，两者都有时直接采用配置值。正向路径由 `PrecisionReducingGeocodingProvider` 包装 `FallbackGeocodingProvider`，先让 Open-Meteo 和 Nominatim 依次尝试完整名称，全部失败后才按数据驱动规则逐级降低查询精度。反向路径通过独立 `ReverseGeocodingProvider` 协议调用 Nominatim `/reverse`，使用 WGS84 坐标取得最接近的 OSM 地址对象、规范展示名、国家代码和行政区；反向结果不经过正向降精度规则。正反向结果都写入被 Git 忽略的定位缓存。 -service 将用户填写的完整地点名作为 `location_scope.full_name` 交给模型，定位结果中已知的行政区和国家代码只作为可选提示，未知字段不写入 payload，也不得由模型猜测。模型以完整地点名为主，按当地行政区划语义识别上级范围，不在核心代码中硬编码省、市、区或国外行政层级的名称与顺序。完整地点或其任一上级行政区受影响时相关，同级或下级的其他地区不相关。经纬度用于天气 API；由于 RSS 正文没有统一结构化灾害边界，灾害地域语义判断留在 LLM 输出契约中，核心编排不加入地区关键词表或供应商专用分支。 +service 将最终解析得到的完整地点名作为 `location_scope.full_name` 交给模型；用户提供名称时保留该名称，只有坐标时使用反向解析的规范展示名。定位结果中已知的行政区和国家代码只作为可选提示，未知字段不写入 payload，也不得由模型猜测。模型以完整地点名为主，按当地行政区划语义识别上级范围，不在核心代码中硬编码省、市、区或国外行政层级的名称与顺序。完整地点或其任一上级行政区受影响时相关，同级或下级的其他地区不相关。经纬度用于天气 API；由于 RSS 正文没有统一结构化灾害边界，灾害地域语义判断留在 LLM 输出契约中，核心编排不加入地区关键词表或供应商专用分支。 `PrecisionReducingGeocodingProvider` 包装完整 provider 链：首先用原始地名查询，所有 provider 均失败后才按 `geography.json` 中明确标注为中国大陆地名格式的规则逐级移除门牌、建筑或更细粒度片段，每个候选仍重新经过完整 provider 链。结果同时保留用户原始地名、provider 实际匹配地名及是否降低精度。首次降精度匹配时，CLI 通过当前投递 provider 发送匹配地名、经纬度和写回私密地点文件的建议；缓存结果不重复通知。 diff --git a/docs/requirements.md b/docs/requirements.md index 5f7372ec..bd27595d 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -25,9 +25,9 @@ 9. 不换算 AQI 或污染物浓度。每个 AQI 数值必须同时标明来源返回的指数标准；PM2.5 只在来源直接提供浓度和单位时展示。QWeather 的运动、穿衣、旅游、舒适度、交通等生活指数作为当日建议的输入。 10. RSS 是可选补充来源，从 `RSS_SOURCES_FILE` 指定的 JSON 文件读取；默认文件名为 `rss-sources.json`。每个来源可通过地点 ID 限定适用范围。文件不存在、为空数组或当前地点没有适用 RSS 时应用仍须正常运行，briefing 更新完全由天气 API 提供。仓库只提交 `rss-sources.example.json` 结构示例。 11. briefing 任务将当前 API 快照、上次成功发布以来积累的未发布内容与持久化历史快照交给 LLM。是否值得打扰必须按上次成功发布后的累计变化评估，而不只比较相邻两次运行。发送的判断标准是用户看到消息后是否需要针对新增或变化的信息采取准备行动；例如预计约一小时后影响当前地区的降雨，应说明时间、概率和雨量并提醒备伞或调整出行。只有这种可行动的短时变化、显著温度或风力变化、预警新增、升级、降级、解除或内容实质变化，以及确实影响关注地区的灾害动态才发送消息。普通天气复述、日期或节气知识、已在标题或预警章节表达的重复事实、空气质量小幅波动，以及仅仅持续生效但无变化的预警不构成投递理由。普通无变化结果应记录但不投递，并继续进入后续任务的总结范围，直到某轮成功投递。合并积压文章、历史简报和历史 API 快照时，模型必须先判断信息的时效性；气温、降水、风力、空气质量和短时预报等已被较新资料取代的内容不得进入当前简报，始终以时间最新且仍适用于当前时刻的资料为准。 -12. 用户可以在私密 JSON 文件中配置一个或多个关注地点。每个地点必须提供稳定 ID 和完整地名，经纬度为可选的一对字段；执行地理编码时由定位层补充国家和行政区代码。 -13. 地名缺少经纬度时，通过可替换且可组合的 geocoding provider 查询 WGS84 坐标、国家、行政区和时区，并在被 Git 忽略的运行状态目录缓存结果。默认先使用无需 API Key 的 Open-Meteo Geocoding 免费非商业 endpoint，详细地名无法解析时降级到 OpenStreetMap Nominatim。完整地名经过全部 provider 仍无法解析时，应按数据驱动规则逐级降低查询精度，每一级重新经过完整 provider 顺序。首次以较低精度匹配后，必须向投递渠道发送 provider 返回的匹配地名及经纬度，要求用户确认并建议将坐标写入私密地点文件；缓存命中不重复提醒。两个 Base URL、Open-Meteo 可选商业 API Key 及 Nominatim User-Agent 均可配置。 -14. 已提供经纬度时不得调用 geocoding API。程序可以用包含海南的中国大陆服务范围四至宽松包围盒，快速排除显然不在中国大陆的坐标；包围盒命中只代表“可能位于”，不是精确国界判断。地名解析结果应优先依据 provider 返回的国家与行政区判定中国大陆分支。 +12. 用户可以在私密 JSON 文件中配置一个或多个关注地点。每个地点必须提供稳定 ID，并在地名与完整经纬度之间至少提供一项；经纬度只能成对出现。只提供地名时执行正向解析，只提供经纬度时执行反向解析得到规范地点名、国家和行政区；两者都提供时直接使用配置坐标和用户地名，不发起额外定位请求。 +13. 地名缺少经纬度时，通过可替换且可组合的 geocoding provider 查询 WGS84 坐标、国家、行政区和时区，并在被 Git 忽略的运行状态目录缓存结果。默认先使用无需 API Key 的 Open-Meteo Geocoding 免费非商业 endpoint，详细地名无法解析时降级到 OpenStreetMap Nominatim。完整地名经过全部 provider 仍无法解析时，应按数据驱动规则逐级降低查询精度，每一级重新经过完整 provider 顺序。只有经纬度时，通过独立的 Nominatim reverse-geocoding 查询最接近的 OSM 地址对象，并缓存规范名称和行政信息。首次以较低精度正向匹配后，必须向投递渠道发送 provider 返回的匹配地名及经纬度，要求用户确认并建议将坐标写入私密地点文件；缓存命中不重复提醒。两个 Base URL、Open-Meteo 可选商业 API Key 及 Nominatim User-Agent 均可配置。 +14. 地名和经纬度同时提供时不得调用 geocoding API；只有经纬度时只执行反向解析，不得再发起正向解析。程序可以用包含海南的中国大陆服务范围四至宽松包围盒，快速排除显然不在中国大陆的坐标；包围盒命中只代表“可能位于”，不是精确国界判断。地名解析结果应优先依据 provider 返回的国家与行政区判定中国大陆分支。 15. 多地点的文章去重、历史简报、天气快照、预警和任务健康状态必须隔离，某一地点的记忆不得污染其他地点。 16. 应用时间值必须使用 Pendulum 的时区感知类型，绝不接受或处理无明确时区的时间。`feedparser` 归一化后的 RSS UTC 时间保持不变；当地日期筛选以地点的 IANA 时区构造日界线后直接比较时间点，只有调度和用户展示等需要当地语义的边界才转换时区。SQLite 持久化边界将时间转换为固定宽度 UTC 文本，使文本字典序与绝对时间顺序一致。 diff --git a/tests/test_config.py b/tests/test_config.py index 90ddc8fc..a74e4944 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -142,13 +142,14 @@ def test_rss_source_treats_null_optional_arrays_as_empty(monkeypatch, tmp_path: assert feed.location_ids == () -def test_location_file_supports_multiple_places_and_optional_coordinates(monkeypatch, tmp_path: Path) -> None: +def test_location_file_supports_name_coordinates_or_both(monkeypatch, tmp_path: Path) -> None: _required_environment(monkeypatch) location_file = tmp_path / "locations.json" location_file.write_text( '[{"id":"beijing","name":"北京市西城区中南海"},' '{"id":"beijing-fixed","name":"北京市西城区中南海",' - '"latitude":39.911389,"longitude":116.380556}]', + '"latitude":39.911389,"longitude":116.380556},' + '{"id":"coordinates-only","latitude":39.911389,"longitude":116.380556}]', encoding="utf-8", ) monkeypatch.setenv("BRIEFING_LOCATIONS_FILE", str(location_file)) @@ -156,9 +157,14 @@ def test_location_file_supports_multiple_places_and_optional_coordinates(monkeyp settings = Settings.from_env() - assert [location.id for location in settings.locations] == ["beijing", "beijing-fixed"] + assert [location.id for location in settings.locations] == [ + "beijing", + "beijing-fixed", + "coordinates-only", + ] assert settings.locations[0].latitude is None assert settings.locations[1].longitude == 116.380556 + assert settings.locations[2].name is None def test_rss_source_location_ids_must_reference_configured_locations(monkeypatch, tmp_path: Path) -> None: @@ -419,14 +425,14 @@ def test_duplicate_location_id_raises_error(self, monkeypatch, tmp_path: Path) - with pytest.raises(ConfigurationError, match="Duplicate location id"): Settings.from_env() - def test_location_without_name_raises_error(self, monkeypatch, tmp_path: Path) -> None: + def test_location_without_name_or_coordinates_raises_error(self, monkeypatch, tmp_path: Path) -> None: _required_environment(monkeypatch) location_file = tmp_path / "locations.json" location_file.write_text('[{"id":"beijing"}]', encoding="utf-8") monkeypatch.setenv("BRIEFING_LOCATIONS_FILE", str(location_file)) monkeypatch.setenv("RSS_SOURCES_FILE", str(tmp_path / "rss-sources.json")) - with pytest.raises(ConfigurationError, match="must have a name"): + with pytest.raises(ConfigurationError, match="must provide a name or coordinates"): Settings.from_env() def test_mismatched_lat_lon_raises_error(self, monkeypatch, tmp_path: Path) -> None: diff --git a/tests/test_geocoding.py b/tests/test_geocoding.py index 068b091e..6f09dca8 100644 --- a/tests/test_geocoding.py +++ b/tests/test_geocoding.py @@ -1,5 +1,7 @@ from pathlib import Path from types import SimpleNamespace +from typing import cast +from unittest.mock import AsyncMock import httpx import pytest @@ -8,9 +10,11 @@ CachedLocationResolver, FallbackGeocodingProvider, GeocodingError, + GeocodingProvider, NominatimGeocodingProvider, OpenMeteoGeocodingProvider, PrecisionReducingGeocodingProvider, + ReverseGeocodingProvider, _mainland_china_rules, _specific_location_name, possibly_mainland_china, @@ -19,6 +23,11 @@ from weather_briefing.reference_data import ReferenceDataError, reference_value +def _required_test_location_name(location: LocationSpec) -> str: + assert location.name is not None + return location.name + + async def test_open_meteo_geocoder_resolves_coordinates_and_country() -> None: def handler(request: httpx.Request) -> httpx.Response: assert request.url.params["name"] == "北京市西城区中南海" @@ -142,17 +151,100 @@ def handler(request: httpx.Request) -> httpx.Response: assert result.longitude == 116.380556 +async def test_nominatim_reverse_geocoder_resolves_name_and_administrative_area() -> None: + def handler(request: httpx.Request) -> httpx.Response: + assert request.url.path == "/reverse" + assert request.url.params["lat"] == "39.911389" + assert request.url.params["lon"] == "116.380556" + assert request.url.params["format"] == "jsonv2" + assert request.url.params["addressdetails"] == "1" + assert request.extensions["weather_briefing.api_call"] == ("nominatim", "reverse-geocoding") + return httpx.Response( + 200, + json={ + "display_name": "中南海, 西城区, 北京市, 中国", + "address": {"country_code": "cn", "state": "北京市"}, + }, + ) + + async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: + result = await NominatimGeocodingProvider(client, user_agent="weather-briefing-test/1").reverse_geocode( + LocationSpec("beijing", latitude=39.911389, longitude=116.380556) + ) + + assert result.name == "中南海, 西城区, 北京市, 中国" + assert result.latitude == 39.911389 + assert result.longitude == 116.380556 + assert result.country_code == "CN" + assert result.administrative_area == "北京市" + assert result.is_mainland_china is True + + +async def test_nominatim_reverse_geocoder_requires_coordinates() -> None: + async with httpx.AsyncClient() as client: + provider = NominatimGeocodingProvider(client, user_agent="weather-briefing-test/1") + with pytest.raises(GeocodingError, match="requires coordinates"): + await provider.reverse_geocode(LocationSpec("beijing", latitude=39.911389)) + + +@pytest.mark.parametrize( + "payload", + ( + [], + {"display_name": "Example", "address": []}, + {"display_name": "", "address": {}}, + ), +) +async def test_nominatim_reverse_geocoder_rejects_invalid_response(payload: object) -> None: + async with httpx.AsyncClient(transport=httpx.MockTransport(lambda _: httpx.Response(200, json=payload))) as client: + provider = NominatimGeocodingProvider(client, user_agent="weather-briefing-test/1") + with pytest.raises(GeocodingError, match="reverse geocoding failed"): + await provider.reverse_geocode(LocationSpec("example", latitude=1.0, longitude=2.0)) + + +async def test_nominatim_reverse_geocoder_handles_http_error() -> None: + async with httpx.AsyncClient(transport=httpx.MockTransport(lambda _: httpx.Response(500))) as client: + provider = NominatimGeocodingProvider(client, user_agent="weather-briefing-test/1") + with pytest.raises(GeocodingError, match="reverse geocoding failed"): + await provider.reverse_geocode(LocationSpec("example", latitude=1.0, longitude=2.0)) + + +async def test_nominatim_reverse_geocoder_rate_limits_consecutive_requests(monkeypatch) -> None: + sleep_calls: list[float] = [] + monotonic_values = iter((100.0, 100.0, 100.25, 100.25)) + + async def fake_sleep(delay: float) -> None: + sleep_calls.append(delay) + + monkeypatch.setattr("weather_briefing.geocoding.asyncio.sleep", fake_sleep) + monkeypatch.setattr( + "weather_briefing.geocoding.time", + SimpleNamespace(monotonic=lambda: next(monotonic_values)), + ) + + async with httpx.AsyncClient( + transport=httpx.MockTransport(lambda _: httpx.Response(200, json={"display_name": "Example", "address": {}})) + ) as client: + provider = NominatimGeocodingProvider(client, user_agent="weather-briefing-test/1") + location = LocationSpec("example", latitude=1.0, longitude=2.0) + await provider.reverse_geocode(location) + await provider.reverse_geocode(location) + + assert sleep_calls == [pytest.approx(0.75)] + + async def test_geocoder_reduces_precision_after_full_address_fails() -> None: queries: list[str] = [] class RoadLevelGeocoder: async def geocode(self, location: LocationSpec) -> ResolvedLocation: - queries.append(location.name) - if location.name.endswith("1号"): + location_name = _required_test_location_name(location) + queries.append(location_name) + if location_name.endswith("1号"): raise GeocodingError("no building match") return ResolvedLocation( location.id, - location.name, + location_name, 39.911389, 116.380556, "CN", @@ -179,9 +271,10 @@ class RecordingGeocoder: async def geocode(self, location: LocationSpec) -> ResolvedLocation: self.calls += 1 + location_name = _required_test_location_name(location) return ResolvedLocation( location.id, - location.name, + location_name, 39.911389, 116.380556, "CN", @@ -203,6 +296,118 @@ async def geocode(self, location: LocationSpec) -> ResolvedLocation: assert geocoder.calls == 1 +@pytest.mark.parametrize("name", (None, " ")) +async def test_resolver_reverse_geocodes_coordinate_only_location_and_caches_result( + tmp_path: Path, + name: str | None, +) -> None: + class RecordingReverseGeocoder: + calls = 0 + + async def reverse_geocode(self, location: LocationSpec) -> ResolvedLocation: + self.calls += 1 + assert location.latitude is not None + assert location.longitude is not None + return ResolvedLocation( + location.id, + "中南海, 西城区, 北京市, 中国", + location.latitude, + location.longitude, + "CN", + "北京市", + None, + True, + matched_name="中南海, 西城区, 北京市, 中国", + ) + + reverse_geocoder = RecordingReverseGeocoder() + forward_geocode = AsyncMock(side_effect=AssertionError("unexpected forward geocoding")) + forward_geocoder = cast(GeocodingProvider, SimpleNamespace(geocode=forward_geocode)) + resolver = CachedLocationResolver( + forward_geocoder, + tmp_path / "geocoding.json", + reverse_provider=reverse_geocoder, + ) + location = LocationSpec("beijing", name, latitude=39.911389, longitude=116.380556) + + first = await resolver.resolve_with_metadata(location) + second = await resolver.resolve_with_metadata(location) + + assert first.location.name == "中南海, 西城区, 北京市, 中国" + assert first.from_cache is False + assert second.location == first.location + assert second.from_cache is True + assert reverse_geocoder.calls == 1 + assert '"beijing:coords:39.9113890,116.3805560"' in (tmp_path / "geocoding.json").read_text(encoding="utf-8") + forward_geocode.assert_not_awaited() + + +async def test_resolver_requires_reverse_provider_for_coordinate_only_location(tmp_path: Path) -> None: + forward_geocode = AsyncMock(side_effect=AssertionError("unexpected forward geocoding")) + forward_geocoder = cast(GeocodingProvider, SimpleNamespace(geocode=forward_geocode)) + resolver = CachedLocationResolver(forward_geocoder, tmp_path / "geocoding.json") + + with pytest.raises(GeocodingError, match="No reverse geocoder configured"): + await resolver.resolve(LocationSpec("example", latitude=1.0, longitude=2.0)) + forward_geocode.assert_not_awaited() + + +@pytest.mark.parametrize("name", (None, " ")) +async def test_forward_geocoder_requires_location_name(name: str | None) -> None: + async with httpx.AsyncClient() as client: + with pytest.raises(GeocodingError, match="Forward geocoding requires a name"): + await OpenMeteoGeocodingProvider(client).geocode(LocationSpec("missing", name)) + + +async def test_forward_geocoder_strips_programmatic_location_name() -> None: + def handler(request: httpx.Request) -> httpx.Response: + assert request.url.params["name"] == "Example" + return httpx.Response( + 200, + json={ + "results": [ + { + "name": "Example", + "latitude": 1.0, + "longitude": 2.0, + "country_code": "XX", + } + ] + }, + ) + + async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: + result = await OpenMeteoGeocodingProvider(client).geocode(LocationSpec("example", " Example ")) + + assert result.name == "Example" + + +@pytest.mark.parametrize( + "cached", + ( + '"invalid"', + '{"id":"example","name":"Example"}', + ), +) +async def test_resolver_rejects_invalid_cached_reverse_record(tmp_path: Path, cached: str) -> None: + cache_path = tmp_path / "geocoding.json" + cache_path.write_text(f'{{"example:coords:1.0000000,2.0000000":{cached}}}', encoding="utf-8") + forward_geocode = AsyncMock(side_effect=AssertionError("unexpected forward geocoding")) + reverse_geocode = AsyncMock(side_effect=AssertionError("unexpected reverse geocoding")) + forward_geocoder = cast(GeocodingProvider, SimpleNamespace(geocode=forward_geocode)) + reverse_geocoder = cast(ReverseGeocodingProvider, SimpleNamespace(reverse_geocode=reverse_geocode)) + resolver = CachedLocationResolver( + forward_geocoder, + cache_path, + reverse_provider=reverse_geocoder, + ) + + with pytest.raises(GeocodingError, match="Invalid cached reverse geocoding record"): + await resolver.resolve(LocationSpec("example", latitude=1.0, longitude=2.0)) + forward_geocode.assert_not_awaited() + reverse_geocode.assert_not_awaited() + + async def test_resolver_rejects_obsolete_cache_record(tmp_path: Path) -> None: class NeverCalledGeocoder: async def geocode(self, location: LocationSpec) -> ResolvedLocation: @@ -231,12 +436,13 @@ async def geocode(self, location: LocationSpec) -> ResolvedLocation: resolver = CachedLocationResolver(FailingGeocoder(), tmp_path / "cache.json") - beijing = await resolver.resolve(LocationSpec("beijing", "Beijing", 39.9, 116.4)) + beijing = await resolver.resolve(LocationSpec("beijing", " Beijing ", 39.9, 116.4)) outside = await resolver.resolve(LocationSpec("outside", "Example", 0.0, 0.0)) latitude = reference_value("geography.json", "mainland_china_service_bounds", "latitude") longitude = reference_value("geography.json", "mainland_china_service_bounds", "longitude") assert beijing.is_mainland_china is True + assert beijing.name == "Beijing" assert outside.is_mainland_china is False assert possibly_mainland_china(latitude["maximum"], longitude["maximum"]) is True assert possibly_mainland_china(latitude["maximum"] + 0.01, longitude["maximum"]) is False @@ -502,7 +708,7 @@ async def test_precision_reducing_provider_exhausts_all_candidates() -> None: class FailingGeocoder: async def geocode(self, location: LocationSpec) -> ResolvedLocation: - calls.append(location.name) + calls.append(_required_test_location_name(location)) raise GeocodingError("no match") with pytest.raises(GeocodingError, match="No geocoder could resolve location at a safe precision"): @@ -518,12 +724,13 @@ async def test_precision_reducing_provider_continues_after_geocoding_error() -> class PartialGeocoder: async def geocode(self, location: LocationSpec) -> ResolvedLocation: - calls.append(location.name) - if "1号" in location.name: + location_name = _required_test_location_name(location) + calls.append(location_name) + if "1号" in location_name: raise GeocodingError("no building match") return ResolvedLocation( location.id, - location.name, + location_name, 39.9, 116.3, "CN", @@ -560,9 +767,10 @@ async def geocode(self, location: LocationSpec) -> ResolvedLocation: async def test_cached_resolver_handles_writes_to_new_directory(tmp_path: Path) -> None: class RecordingGeocoder: async def geocode(self, location: LocationSpec) -> ResolvedLocation: + location_name = _required_test_location_name(location) return ResolvedLocation( location.id, - location.name, + location_name, 1.0, 2.0, None, diff --git a/weather_briefing/cli.py b/weather_briefing/cli.py index 4d88d77f..98b216bc 100644 --- a/weather_briefing/cli.py +++ b/weather_briefing/cli.py @@ -220,6 +220,11 @@ async def run( ) delivery = _delivery_provider(settings, client, diagnostics) llm_provider = _llm_provider(settings, client) + nominatim_provider = NominatimGeocodingProvider( + client, + base_url=settings.nominatim_base_url, + user_agent=settings.geocoding_user_agent, + ) resolver = CachedLocationResolver( PrecisionReducingGeocodingProvider( FallbackGeocodingProvider( @@ -228,14 +233,11 @@ async def run( base_url=settings.geocoding_base_url, api_key=settings.geocoding_api_key, ), - NominatimGeocodingProvider( - client, - base_url=settings.nominatim_base_url, - user_agent=settings.geocoding_user_agent, - ), + nominatim_provider, ) ), settings.geocoding_cache_path, + reverse_provider=nominatim_provider, ) _LOGGER.info("Resolving %d location(s)", len(settings.locations)) resolutions = [await resolver.resolve_with_metadata(location) for location in settings.locations] diff --git a/weather_briefing/config.py b/weather_briefing/config.py index 755b0432..3c0bd95d 100644 --- a/weather_briefing/config.py +++ b/weather_briefing/config.py @@ -135,13 +135,13 @@ def _locations(path: Path) -> tuple[LocationSpec, ...]: seen_ids: set[str] = set() for item in items: location_id = str(item.get("id", "")).strip() - name = str(item.get("name", "")).strip() + name_value = item.get("name") + name = str(name_value).strip() if name_value is not None else None + name = name or None if not location_id or not location_id.replace("-", "").replace("_", "").isalnum(): raise ConfigurationError("Location id must use letters, numbers, '-' or '_'") if location_id in seen_ids: raise ConfigurationError(f"Duplicate location id: {location_id}") - if not name: - raise ConfigurationError(f"Location {location_id} must have a name") latitude_value = item.get("latitude") longitude_value = item.get("longitude") if (latitude_value is None) != (longitude_value is None): @@ -155,6 +155,8 @@ def _locations(path: Path) -> tuple[LocationSpec, ...]: raise ConfigurationError(f"Location {location_id} latitude is out of range") if longitude is not None and not -180 <= longitude <= 180: raise ConfigurationError(f"Location {location_id} longitude is out of range") + if name is None and latitude is None: + raise ConfigurationError(f"Location {location_id} must provide a name or coordinates") locations.append(LocationSpec(location_id, name, latitude, longitude)) seen_ids.add(location_id) return tuple(locations) diff --git a/weather_briefing/geocoding.py b/weather_briefing/geocoding.py index b2bcfeaf..8c937438 100644 --- a/weather_briefing/geocoding.py +++ b/weather_briefing/geocoding.py @@ -24,6 +24,10 @@ class GeocodingProvider(Protocol): async def geocode(self, location: LocationSpec) -> ResolvedLocation: ... +class ReverseGeocodingProvider(Protocol): + async def reverse_geocode(self, location: LocationSpec) -> ResolvedLocation: ... + + @cache def _mainland_china_rules() -> tuple[float, float, float, float, frozenset[str]]: try: @@ -65,8 +69,9 @@ def __init__( self._api_key = api_key async def geocode(self, location: LocationSpec) -> ResolvedLocation: + location_name = _required_location_name(location) params: dict[str, str | int] = { - "name": location.name, + "name": location_name, "count": 5, "language": "zh", "format": "json", @@ -83,17 +88,17 @@ async def geocode(self, location: LocationSpec) -> ResolvedLocation: payload = response.json() results = payload.get("results", []) if not isinstance(results, list) or not results: - raise GeocodingError(f"No geocoding result for location: {location.name}") + raise GeocodingError(f"No geocoding result for location: {location_name}") result = next( ( item for item in results - if isinstance(item, dict) and _open_meteo_result_matches(location.name, item) + if isinstance(item, dict) and _open_meteo_result_matches(location_name, item) ), None, ) if result is None: - raise GeocodingError(f"No matching geocoding result for location: {location.name}") + raise GeocodingError(f"No matching geocoding result for location: {location_name}") latitude = float(result["latitude"]) longitude = float(result["longitude"]) country_code = str(result.get("country_code", "")).upper() or None @@ -102,17 +107,17 @@ async def geocode(self, location: LocationSpec) -> ResolvedLocation: except GeocodingError: raise except (httpx.HTTPError, KeyError, TypeError, ValueError) as exc: - raise GeocodingError(f"Geocoding request or response validation failed for: {location.name}") from exc + raise GeocodingError(f"Geocoding request or response validation failed for: {location_name}") from exc return ResolvedLocation( id=location.id, - name=location.name, + name=location_name, latitude=latitude, longitude=longitude, country_code=country_code, administrative_area=administrative_area, timezone=timezone, is_mainland_china=_is_geocoded_mainland(country_code, administrative_area), - matched_name=str(result.get("name", "")).strip() or location.name, + matched_name=str(result.get("name", "")).strip() or location_name, ) @@ -133,9 +138,10 @@ def __init__( self._last_request_at = 0.0 async def geocode(self, location: LocationSpec) -> ResolvedLocation: + location_name = _required_location_name(location) async with self._lock: result: dict[str, object] | None = None - for query in _nominatim_queries(location.name): + for query in _nominatim_queries(location_name): delay = 1.0 - (time.monotonic() - self._last_request_at) if delay > 0: await asyncio.sleep(delay) @@ -160,16 +166,16 @@ async def geocode(self, location: LocationSpec) -> ResolvedLocation: ( item for item in results - if isinstance(item, dict) and _nominatim_result_matches(location.name, item) + if isinstance(item, dict) and _nominatim_result_matches(location_name, item) ), None, ) if result is not None: break except httpx.HTTPError as exc: - raise GeocodingError(f"Nominatim request failed for: {location.name}") from exc + raise GeocodingError(f"Nominatim request failed for: {location_name}") from exc if result is None: - raise GeocodingError(f"No Nominatim result for location: {location.name}") + raise GeocodingError(f"No Nominatim result for location: {location_name}") try: address = result.get("address", {}) latitude = float(result["lat"]) @@ -177,17 +183,65 @@ async def geocode(self, location: LocationSpec) -> ResolvedLocation: country_code = str(address.get("country_code", "")).upper() or None administrative_area = str(address.get("state") or address.get("province") or "").strip() or None except (KeyError, TypeError, ValueError, AttributeError) as exc: - raise GeocodingError(f"Nominatim response validation failed for: {location.name}") from exc + raise GeocodingError(f"Nominatim response validation failed for: {location_name}") from exc return ResolvedLocation( id=location.id, - name=location.name, + name=location_name, latitude=latitude, longitude=longitude, country_code=country_code, administrative_area=administrative_area, timezone=None, is_mainland_china=_is_geocoded_mainland(country_code, administrative_area), - matched_name=str(result.get("display_name", "")).strip() or location.name, + matched_name=str(result.get("display_name", "")).strip() or location_name, + ) + + async def reverse_geocode(self, location: LocationSpec) -> ResolvedLocation: + if location.latitude is None or location.longitude is None: + raise GeocodingError(f"Reverse geocoding requires coordinates for location: {location.id}") + async with self._lock: + delay = 1.0 - (time.monotonic() - self._last_request_at) + if delay > 0: + await asyncio.sleep(delay) + try: + response = await self._client.get( + f"{self._base_url}/reverse", + params={ + "lat": location.latitude, + "lon": location.longitude, + "format": "jsonv2", + "addressdetails": 1, + }, + headers={"User-Agent": self._user_agent}, + extensions=api_call_extensions("nominatim", "reverse-geocoding"), + ) + self._last_request_at = time.monotonic() + response.raise_for_status() + result = response.json() + if not isinstance(result, dict): + raise TypeError("Nominatim reverse response must be an object") + address = result.get("address", {}) + if not isinstance(address, dict): + raise TypeError("Nominatim reverse address must be an object") + display_name = str(result.get("display_name", "")).strip() + if not display_name: + raise ValueError("Nominatim reverse response has no display name") + country_code = str(address.get("country_code", "")).upper() or None + administrative_area = ( + str(address.get("state") or address.get("province") or address.get("region") or "").strip() or None + ) + except (httpx.HTTPError, TypeError, ValueError, AttributeError) as exc: + raise GeocodingError(f"Nominatim reverse geocoding failed for location: {location.id}") from exc + return ResolvedLocation( + id=location.id, + name=display_name, + latitude=location.latitude, + longitude=location.longitude, + country_code=country_code, + administrative_area=administrative_area, + timezone=None, + is_mainland_china=_is_geocoded_mainland(country_code, administrative_area), + matched_name=display_name, ) @@ -198,13 +252,14 @@ def __init__(self, *providers: GeocodingProvider) -> None: self._providers = providers async def geocode(self, location: LocationSpec) -> ResolvedLocation: + location_name = _required_location_name(location) errors: list[GeocodingError] = [] for provider in self._providers: try: return await provider.geocode(location) except GeocodingError as exc: errors.append(exc) - raise GeocodingError(f"No geocoder could resolve location: {location.name}") from errors[-1] + raise GeocodingError(f"No geocoder could resolve location: {location_name}") from errors[-1] class PrecisionReducingGeocodingProvider: @@ -212,11 +267,12 @@ def __init__(self, provider: GeocodingProvider) -> None: self._provider = provider async def geocode(self, location: LocationSpec) -> ResolvedLocation: + location_name = _required_location_name(location) try: return await self._provider.geocode(location) except GeocodingError as direct_error: last_error = direct_error - for candidate_name in _lower_precision_location_names(location.name): + for candidate_name in _lower_precision_location_names(location_name): candidate = replace(location, name=candidate_name) try: resolved = await self._provider.geocode(candidate) @@ -225,26 +281,36 @@ async def geocode(self, location: LocationSpec) -> ResolvedLocation: continue return replace( resolved, - name=location.name, + name=location_name, precision_reduced=True, ) - raise GeocodingError(f"No geocoder could resolve location at a safe precision: {location.name}") from last_error + raise GeocodingError(f"No geocoder could resolve location at a safe precision: {location_name}") from last_error class CachedLocationResolver: - def __init__(self, provider: GeocodingProvider, cache_path: Path) -> None: + def __init__( + self, + provider: GeocodingProvider, + cache_path: Path, + *, + reverse_provider: ReverseGeocodingProvider | None = None, + ) -> None: self._provider = provider self._cache_path = cache_path + self._reverse_provider = reverse_provider async def resolve(self, location: LocationSpec) -> ResolvedLocation: return (await self.resolve_with_metadata(location)).location async def resolve_with_metadata(self, location: LocationSpec) -> LocationResolution: + location_name = (location.name or "").strip() or None if location.latitude is not None and location.longitude is not None: + if location_name is None: + return await self._reverse_geocode(location, location.latitude, location.longitude) return LocationResolution( ResolvedLocation( id=location.id, - name=location.name, + name=location_name, latitude=location.latitude, longitude=location.longitude, country_code=None, @@ -255,20 +321,44 @@ async def resolve_with_metadata(self, location: LocationSpec) -> LocationResolut from_cache=False, ) cache = self._read_cache() - cache_key = f"{location.id}:{location.name}" + location_name = _required_location_name(location) + cache_key = f"{location.id}:{location_name}" cached = cache.get(cache_key) if isinstance(cached, dict): try: return LocationResolution(ResolvedLocation(**cached), from_cache=True) except TypeError as exc: - raise GeocodingError(f"Invalid cached geocoding record for location: {location.name}") from exc + raise GeocodingError(f"Invalid cached geocoding record for location: {location_name}") from exc if cached is not None: - raise GeocodingError(f"Invalid cached geocoding record for location: {location.name}") + raise GeocodingError(f"Invalid cached geocoding record for location: {location_name}") resolved = await self._provider.geocode(location) cache[cache_key] = asdict(resolved) self._write_cache(cache) return LocationResolution(resolved, from_cache=False) + async def _reverse_geocode( + self, + location: LocationSpec, + latitude: float, + longitude: float, + ) -> LocationResolution: + if self._reverse_provider is None: + raise GeocodingError(f"No reverse geocoder configured for location: {location.id}") + cache = self._read_cache() + cache_key = f"{location.id}:coords:{latitude:.7f},{longitude:.7f}" + cached = cache.get(cache_key) + if isinstance(cached, dict): + try: + return LocationResolution(ResolvedLocation(**cached), from_cache=True) + except TypeError as exc: + raise GeocodingError(f"Invalid cached reverse geocoding record for location: {location.id}") from exc + if cached is not None: + raise GeocodingError(f"Invalid cached reverse geocoding record for location: {location.id}") + resolved = await self._reverse_provider.reverse_geocode(location) + cache[cache_key] = asdict(resolved) + self._write_cache(cache) + return LocationResolution(resolved, from_cache=False) + def _read_cache(self) -> dict[str, Any]: if not self._cache_path.exists(): return {} @@ -294,6 +384,13 @@ def _is_geocoded_mainland(country_code: str | None, administrative_area: str | N return normalized not in _mainland_china_rules()[4] +def _required_location_name(location: LocationSpec) -> str: + name = (location.name or "").strip() + if not name: + raise GeocodingError(f"Forward geocoding requires a name for location: {location.id}") + return name + + def _nominatim_queries(name: str) -> tuple[str, ...]: normalized = _normalized_location_name(name) return tuple(dict.fromkeys((normalized, name))) diff --git a/weather_briefing/models.py b/weather_briefing/models.py index 7870fdc8..1c7eee2e 100644 --- a/weather_briefing/models.py +++ b/weather_briefing/models.py @@ -27,7 +27,7 @@ class ContextSourceConfig: @dataclass(frozen=True, slots=True) class LocationSpec: id: str - name: str + name: str | None = None latitude: float | None = None longitude: float | None = None