diff --git a/tradein-mvp/backend/app/services/geocoder.py b/tradein-mvp/backend/app/services/geocoder.py index 1bf0817f..0a5d8ebf 100644 --- a/tradein-mvp/backend/app/services/geocoder.py +++ b/tradein-mvp/backend/app/services/geocoder.py @@ -231,6 +231,68 @@ def _names_non_ekb_city(address: str) -> bool: return False +# ── Fail-closed EKB-local-tiers gate (#2582) ───────────────────────────────── +# `_names_non_ekb_city` выше ловит только 37 ПЕРЕЧИСЛЕННЫХ городов области — +# любой другой топоним (другой регион РФ целиком, ИЛИ областной город вне +# списка: Реж/Арамиль/Рефтинский/Лёвиха) молча проходил как «не назван», и +# EKB-only локальные тиры (geoportal/cad_buildings) снапали улицу+дом в ЕКБ по +# коллизии (см. живые случаи в geocode_cache: «республика крым, ялта, ...» → +# координаты ЕКБ, «челябинская обл., трёхгорный, ...» → координаты ЕКБ). +# +# Вместо перечисления (сама причина бага — список никогда не покроет все +# города РФ) используем общий структурный признак «в адресе назван +# город/регион»: маркер-слово, которое НИКОГДА не относится к улице/дому +# («обл.», «край», «республика», «г.», «пос.», «с.»). "д." сюда намеренно НЕ +# входит — это маркер дома (`_HOUSE_MARKER`: «ул. Ленина, д. 5» — основной +# формат ввода номера дома), включение сломало бы гейт почти на каждом адресе. +_LOCALITY_MARKER_RE = re.compile( + r"\b(?:обл\.?|область|край|республика|респ\.?|г|пос\.?|посёлок|поселок|с\.)\b" +) + + +def _names_unrecognized_locality(address: str) -> bool: + """True если адрес называет топоним, но это не Екатеринбург и не город из + известного списка `SVERDLOVSK_OBLAST_CITIES` (#2582). + + Отличает «город не назван» (bare «Ленина 1» — так приходит основной + трафик формы оценки; geoportal резолвит его как ЕКБ и это оправданный + дефолт) от «назван НЕЗНАКОМЫЙ город» (Ялта/Трёхгорный/Реж/Лёвиха — + структурный маркер топонима есть, но это не ЕКБ и не известный город + региона 66) — только во втором случае EKB-only локальные тиры нельзя + пускать. + """ + normalized = " ".join(address.lower().split()) + if _EKATERINBURG_RE.search(normalized): + return False # явный ЕКБ — приоритет, см. _names_non_ekb_city + return bool(_LOCALITY_MARKER_RE.search(normalized)) + + +def _ekb_local_tiers_allowed(address: str) -> bool: + """Fail-closed гейт локальных ЕКБ-тиров geocoder (`geocode()`/`suggest()`, #2582). + + ЕКБ-only локальные реестры (geoportal/cad_buildings) используются ТОЛЬКО + когда Екатеринбург подтверждён явно, или в адресе вообще не назван + никакой город/регион. Если назван ЛЮБОЙ другой топоним — известный город + региона (`_names_non_ekb_city`) или незнакомый (`_names_unrecognized_locality`, + маркер структуры адреса) — гейт закрыт, идём во внешние oblast/nation-aware + провайдеры (Yandex/Nominatim, оба с region cross-check и bbox-фильтром по + всей области/стране — им незнакомый топоним не страшен). + + До #2582 решение принималось от противного: «нет в списке 37 городов → + считаем Екатеринбургом» — молча ломало любой адрес вне списка, включая + другие регионы РФ целиком (см. docstring `_names_unrecognized_locality`). + + Explicit-Екатеринбург проверяется отдельно (не только внутри вложенных + хелперов) — это единственный сигнал, который обязан перевешивать ЛЮБОЙ + маркер топонима (напр. «г. Екатеринбург» сам содержит маркер «г»). + """ + if _EKATERINBURG_RE.search(" ".join(address.lower().split())): + return True # явный ЕКБ — приоритет над любым другим топонимом/маркером + if _names_non_ekb_city(address) or _names_unrecognized_locality(address): + return False + return True # города/региона не названо вообще — основной трафик, дефолт ЕКБ + + # ── Address normalisation ─────────────────────────────────────────────────── def normalize_address(address: str) -> str: """Нормализация для cache lookup: lowercase + trim + collapse whitespace. @@ -1099,11 +1161,12 @@ async def suggest(query: str, db: Session | None = None, limit: int = 8) -> list return [] # Tier 1: cadastral FDW (если db доступна) — самый быстрый, без внешних запросов. - # EKB-only (см. `geocode()` #11) — пропускаем, если query явно называет - # другой город области, иначе не-ЕКБ автокомплит может всплыть ЕКБ-домом - # с совпадающими улица+дом. Внешние тиры (2/3/4 ниже) не гейтим — они уже - # oblast-aware. - if db is not None and not _names_non_ekb_city(query): + # EKB-only fail-closed гейт (#2582, было #11) — пропускаем, если query + # явно называет другой топоним (известный город области ИЛИ незнакомый — + # см. `_ekb_local_tiers_allowed`), иначе не-ЕКБ автокомплит может всплыть + # ЕКБ-домом с совпадающими улица+дом. Внешние тиры (2/3/4 ниже) не гейтим — + # они уже oblast-aware. + if db is not None and _ekb_local_tiers_allowed(query): # 1a. Anchored house-match: парсим street+house → точный матч по дом-маркеру. # Решает кейс «Серова 27» где raw-ILIKE по readable_address давал 0 hits. parsed = _parse_street_house(query.strip()) @@ -1170,11 +1233,15 @@ async def geocode(address: str, db: Session) -> GeocodeResult | None: # 2. Локальные источники по street+house (без внешнего API). parsed = _parse_street_house(address.strip()) - # #11: EKB-only локальные тиры (geoportal/cadastral) не должны отвечать на - # адрес другого города области — иначе улица+дом, коллизящие с ЕКБ-домом - # (напр. "проспект Ленина 1" есть и в Нижнем Тагиле, и в ЕКБ), снапаются в - # ЕКБ. Пропускаем сразу к oblast-aware внешним провайдерам ниже (3/4). - use_local_ekb = not _names_non_ekb_city(address) + # #2582 (было #11): EKB-only локальные тиры (geoportal/cadastral) не должны + # отвечать на адрес другого города/региона — иначе улица+дом, коллизящие с + # ЕКБ-домом (напр. "проспект Ленина 1" есть и в Нижнем Тагиле, и в ЕКБ), + # снапаются в ЕКБ. Fail-closed: разрешаем локальные тиры ТОЛЬКО если + # Екатеринбург подтверждён явно или город/регион вообще не назван — см. + # `_ekb_local_tiers_allowed`. Раньше решение принималось от противного + # (список из 37 городов — «нет в списке → считаем ЕКБ»), из-за чего ЛЮБОЙ + # другой регион РФ (Ялта, Трёхгорный) молча резолвился в координаты ЕКБ. + use_local_ekb = _ekb_local_tiers_allowed(address) # 2a. Геопортал ЕКБ — ПЕРВЫЙ локальный tier (полнее cad_buildings ~на 70%). if use_local_ekb and parsed is not None: diff --git a/tradein-mvp/backend/tests/test_geocoder_bbox.py b/tradein-mvp/backend/tests/test_geocoder_bbox.py index b0afa8ed..bd2750ae 100644 --- a/tradein-mvp/backend/tests/test_geocoder_bbox.py +++ b/tradein-mvp/backend/tests/test_geocoder_bbox.py @@ -21,8 +21,10 @@ from app.services.geocoder import ( EKB_BBOX_TIGHT, EKB_BBOX_WIDE, OBLAST66_BBOX, + _ekb_local_tiers_allowed, _has_oblast_marker, _names_non_ekb_city, + _names_unrecognized_locality, _nominatim_query, _yandex_lookup, is_within_ekb_bbox, @@ -202,6 +204,68 @@ def test_names_non_ekb_city(address: str, expected: bool) -> None: assert _names_non_ekb_city(address) is expected +# ── _names_unrecognized_locality / _ekb_local_tiers_allowed (#2582) ───────── +# Fail-closed гейт: EKB-only локальные тиры (geoportal/cad_buildings) больше +# НЕ держатся на списке из 37 городов ("нет в списке → считаем ЕКБ"). Живые +# прод-случаи из geocode_cache (все получили координаты ЕКБ по ошибке до +# фикса): Ялта (другой регион), Трёхгорный (другой регион), Лёвиха/Реж +# (реальные города Свердловской области ВНЕ списка 37). + + +@pytest.mark.parametrize( + "address,expected", + [ + # Живые прод-случаи (#2582) — региональный/settlement маркер есть, + # но это не Екатеринбург и не город из известного списка: + ("республика крым, ялта, ул. репина, 18с2", True), + ("челябинская обл., трёхгорный, ул. ленина, 15", True), + ("свердловская обл., кировградский р-н, пос. лёвиха, ул. карла маркса, 8", True), + ("свердловская обл., реж, ул. ленина, 5", True), + ("ульяновская обл., димитровград, ул. менделеева, 11", True), + # известный город списка ловится отдельно (_names_non_ekb_city), + # эта функция на него не обязана реагировать без маркера: + ("нижний тагил, ленина 10", False), + # города/региона вообще не названо — основной трафик, не топоним: + ("малышева 30", False), + ("серова 27", False), # word-boundary — не город "серов" + ("проспект ленина, 1", False), + # явный Екатеринбург побеждает даже при наличии маркера "г": + ("г. екатеринбург, ул. малышева, 30", False), + ("г екатеринбург, ленинский р-н, ул серова, д 35", False), + ], +) +def test_names_unrecognized_locality(address: str, expected: bool) -> None: + assert _names_unrecognized_locality(address) is expected + + +@pytest.mark.parametrize( + "address,expected", + [ + # Живые прод-случаи (#2582) — гейт обязан закрыться: + ("республика крым, ялта, ул. репина, 18с2", False), + ("челябинская обл., трёхгорный, ул. ленина, 15", False), + ("свердловская обл., кировградский р-н, пос. лёвиха, ул. карла маркса, 8", False), + ("свердловская обл., реж, ул. ленина, 5", False), + # известный город региона 66 (список) — тоже закрыт: + ("Нижний Тагил, проспект Ленина, 1", False), + ("Каменск-Уральский, ул. Ленина 1", False), + # ЕКБ явно назван — гейт открыт (regardless маркера "г"): + ("г. Екатеринбург, ул. Малышева, 30", True), + ("620144, Свердловская обл, г Екатеринбург, Ленинский р-н, ул Серова, д 35", True), + # города/региона вообще не названо — ОСНОВНОЙ ТРАФИК (форма оценки без + # города) — дефолт ЕКБ должен остаться рабочим, иначе деградация #1: + ("Малышева 30", True), + ("Серова 27", True), + ("Космонавтов 7б", True), + ("проспект Ленина, 1", True), + # мкр-исключение (район внутри ЕКБ, не ЗАТО Заречный) — гейт открыт: + ("мкр Заречный, ул. Ленина 5", True), + ], +) +def test_ekb_local_tiers_allowed(address: str, expected: bool) -> None: + assert _ekb_local_tiers_allowed(address) is expected + + # ── Accept-site two-pass tie-break + region cross-check ───────────────────── diff --git a/tradein-mvp/backend/tests/test_geocoder_cadastral_matcher.py b/tradein-mvp/backend/tests/test_geocoder_cadastral_matcher.py index c5d407fc..7bd013af 100644 --- a/tradein-mvp/backend/tests/test_geocoder_cadastral_matcher.py +++ b/tradein-mvp/backend/tests/test_geocoder_cadastral_matcher.py @@ -309,6 +309,98 @@ async def test_geocode_skips_ekb_local_tiers_for_non_ekb_city() -> None: mock_nominatim.assert_called_once() +# ── geocode() fail-closed gate for UNRECOGNIZED locality (#2582) ──────────── +# Живые прод-случаи из geocode_cache: адрес называет город/регион ВНЕ списка +# из 37 (или другой регион РФ целиком) — до фикса `_names_non_ekb_city` +# молчаливо считал это «город не назван» → EKB-only тиры отвечали первыми и +# снапали улицу+дом в координаты ЕКБ. + + +@pytest.mark.parametrize( + ("address", "label"), + [ + ("республика крым, ялта, ул. репина, 18с2", "Ялта (другой регион)"), + ("челябинская обл., трёхгорный, ул. ленина, 15", "Трёхгорный (другой регион)"), + ( + "свердловская обл., кировградский р-н, пос. лёвиха, ул. карла маркса, 8", + "Лёвиха (город области ВНЕ списка 37)", + ), + ("свердловская обл., реж, ул. ленина, 5", "Реж (город области ВНЕ списка 37)"), + ], +) +async def test_geocode_skips_ekb_local_tiers_for_unrecognized_locality( + address: str, label: str +) -> None: + """EKB-only локальные тиры (geoportal/cadastral) НЕ вызываются для адреса, + называющего незнакомый топоним — идём сразу к внешнему oblast/nation-aware + провайдеру (#2582).""" + db = MagicMock() + nominatim_result = GeocodeResult( + lat=45.0, + lon=34.0, + full_address=address, + provider="nominatim", + confidence="approximate", + ) + + with ( + patch("app.services.geocoder._cache_get", return_value=None), + patch("app.services.geocoder._geoportal_house_match") as mock_geoportal, + patch("app.services.geocoder._cadastral_house_match") as mock_house, + patch("app.services.geocoder._cadastral_forward_sync") as mock_forward, + patch("app.services.geocoder._cache_put"), + patch("app.services.geocoder.settings") as mock_settings, + patch( + "app.services.geocoder._nominatim_lookup", + new_callable=AsyncMock, + return_value=nominatim_result, + ) as mock_nominatim, + ): + mock_settings.yandex_geocoder_api_key = None + result = await geocode(address, db) + + assert result is not None, label + mock_geoportal.assert_not_called() + mock_house.assert_not_called() + mock_forward.assert_not_called() + mock_nominatim.assert_called_once() + + +@pytest.mark.parametrize( + ("address", "label"), + [ + ("Серова 27", "ЕКБ без города (основной трафик формы оценки)"), + ("Малышева 30", "ЕКБ без города (основной трафик формы оценки)"), + ("г. Екатеринбург, ул. Серова, д. 27", "ЕКБ с явным городом"), + ], +) +async def test_geocode_still_uses_ekb_local_tiers_for_real_ekb_address( + address: str, label: str +) -> None: + """Регрессия: реальный ЕКБ-адрес (с городом и без) по-прежнему резолвится + через geoportal (fast-path) — fail-closed гейт не должен деградировать + основной трафик (#2582).""" + db = MagicMock() + hit = GeocodeSuggestion( + label="ул. Серова, д. 27, Екатеринбург", + full_address="ул. Серова, д. 27, Екатеринбург", + lat=56.81188, + lon=60.59739, + kind="house", + ) + + with ( + patch("app.services.geocoder._cache_get", return_value=None), + patch("app.services.geocoder._geoportal_house_match", return_value=hit) as mock_geoportal, + patch("app.services.geocoder._cache_put"), + ): + result = await geocode(address, db) + + assert result is not None, label + assert result.lat == pytest.approx(56.81188), label + mock_geoportal.assert_called_once() + + # ── suggest() wiring ───────────────────────────────────────────────────────── @@ -409,3 +501,46 @@ async def test_suggest_skips_ekb_local_tier_for_non_ekb_city() -> None: mock_house.assert_not_called() mock_forward.assert_not_called() mock_nominatim.assert_called_once() + + +@pytest.mark.parametrize( + ("query", "label"), + [ + ("республика крым, ялта, ул. репина, 18с2", "Ялта (другой регион)"), + ("свердловская обл., реж, ул. ленина, 5", "Реж (город области ВНЕ списка 37)"), + ], +) +async def test_suggest_skips_ekb_local_tier_for_unrecognized_locality( + query: str, label: str +) -> None: + """Незнакомый топоним (не в списке 37, не Екатеринбург) → EKB-only + cadastral Tier 1 не вызывается; falls through к внешним тирам (#2582).""" + db = MagicMock() + nominatim_hit = [ + GeocodeSuggestion( + label="ул. Репина, 18с2", + full_address=query, + lat=44.5, + lon=34.16, + kind="house", + ) + ] + + with ( + patch("app.services.geocoder._cadastral_house_match") as mock_house, + patch("app.services.geocoder._cadastral_forward_sync") as mock_forward, + patch("app.services.geocoder.settings") as mock_settings, + patch( + "app.services.geocoder._nominatim_suggest", + new_callable=AsyncMock, + return_value=nominatim_hit, + ) as mock_nominatim, + ): + mock_settings.dadata_api_token = None + mock_settings.yandex_geocoder_api_key = None + results = await suggest(query, db=db, limit=8) + + assert len(results) == 1, label + mock_house.assert_not_called() + mock_forward.assert_not_called() + mock_nominatim.assert_called_once()