From af609c18f31b34d85391a99ad6c191292605bbac Mon Sep 17 00:00:00 2001 From: bot-backend Date: Fri, 31 Jul 2026 19:17:47 +0300 Subject: [PATCH] =?UTF-8?q?fix(tradein/geocoder):=20=D0=BF=D0=BE=D1=87?= =?UTF-8?q?=D0=B8=D0=BD=D0=B8=D1=82=D1=8C=20=D0=BC=D0=B0=D1=80=D0=BA=D0=B5?= =?UTF-8?q?=D1=80=20=D1=81=D0=B5=D0=BB=D0=B0=20+=20fail-closed=20=D0=B4?= =?UTF-8?q?=D0=BB=D1=8F=20=D1=8F=D0=B2=D0=BD=D0=BE=D0=B3=D0=BE=20city=5Fhi?= =?UTF-8?q?nt=20(#2582)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit H1 (deep-review PR #2589): `с\.` в _LOCALITY_MARKER_RE был мертворождён — хвостовой \b не может сработать между "." и пробелом (оба не-словесные), формат «с. <село>» (104/77к прод-адресов) не ловился НИ РАЗУ, зато ложно срабатывал на инициалах «им с.п.королева». Живой инцидент воспроизведён: "с. Новоалексеевское, ул. 8 Марта,37" снапалось в ЕКБ (совпадение до 5-го знака, село в 35 км западнее). Заменил на lookahead-форму для однобуквенных маркеров (с/д/дер/пгт/ст): "\.\s*(?=[а-яё])" — различает "д. 5" (дом-номер, не матчит) от "д. Кашино" (деревня, матчит), что заодно позволило безопасно включить "д."/"дер." (раньше исключённые из-за коллизии с house-маркером). M2: гейт по city_hint оставался list-based (_names_non_ekb_city(city_hint)) — для СТРУКТУРНОГО хинта (не свободного текста) fail-closed форма должна быть обратной: "не Екатеринбург → закрыто", а не "есть в списке 37 → закрыто". city_hint принимает произвольную строку без валидации (API напрямую, минуя 6-городный UI-selector) — city_hint="Реж" держал гейт открытым, geoportal отдавал точный ЕКБ-дом, city_ambiguous=False делало это уверенно-неверным. M3: добавлены изолирующие тесты — предыдущая параметризация "незнакомый топоним" всегда несла ещё и "обл."/"республика"/"край" одновременно с проверяемым сигналом, поэтому мёртвый "с\." прошёл незамеченным. Плюс честно зафиксирована известная дыра (гейт по city_hint) — "реж, ленина 5" без маркера остаётся в ЕКБ-дефолте (архитектурное решение отдельно, не список городов). docstring `_names_unrecognized_locality`/`_ekb_local_tiers_allowed` приведён в соответствие с фактическим поведением regex. --- tradein-mvp/backend/app/services/geocoder.py | 79 ++++++++++++++----- .../backend/tests/test_geocoder_bbox.py | 67 +++++++++++++++- .../tests/test_geocoder_cadastral_matcher.py | 52 ++++++++++++ 3 files changed, 174 insertions(+), 24 deletions(-) diff --git a/tradein-mvp/backend/app/services/geocoder.py b/tradein-mvp/backend/app/services/geocoder.py index 0f450ccc..0a503cca 100644 --- a/tradein-mvp/backend/app/services/geocoder.py +++ b/tradein-mvp/backend/app/services/geocoder.py @@ -295,11 +295,29 @@ def _names_non_ekb_city(address: str) -> bool: # Вместо перечисления (сама причина бага — список никогда не покроет все # города РФ) используем общий структурный признак «в адресе назван # город/регион»: маркер-слово, которое НИКОГДА не относится к улице/дому -# («обл.», «край», «республика», «г.», «пос.», «с.»). "д." сюда намеренно НЕ -# входит — это маркер дома (`_HOUSE_MARKER`: «ул. Ленина, д. 5» — основной -# формат ввода номера дома), включение сломало бы гейт почти на каждом адресе. +# («обл.», «край», «республика», «г.», «пос.»). +# +# Два разных класса маркеров — разная форма regex: +# 1. Маркеры БЕЗ обязательной точки после (обл/область/край/республика/респ/ +# г/пос/посёлок/поселок) — сохраняют старую форму `\bслово\.?\b`. Точка +# опциональна и есть трейлинг `\b`, который для этих слов срабатывает и +# без точки (следующий символ — пробел/запятая, не-словесный). +# 2. Маркеры-ОДНОБУКВЕННЫЕ сокращения, где точка ОБЯЗАТЕЛЬНА для отличия от +# других смыслов той же буквы («с» — типографский мусор/что угодно, «д» — +# дом-маркер `_HOUSE_MARKER`) — «с.», «д.», «дер.», «пгт.», «ст.». Тут +# хвостовой `\b` В ПРИНЦИПЕ не может сработать после точки перед пробелом +# (оба не-словесные — на стыке `.` и ` ` нет границы \w/не-\w) — было +# смерто-рождённой формой, вылавливавшей 0 из 104 прод-адресов «с. <село>» +# (deep-review PR #2589, H1). Вместо `\b` в конце — lookahead на кириллицу +# сразу после точки (с опциональными пробелами): `\.\s*(?=[а-яё])`. +# Это и различает «д. 5» (дом-маркер, номер, НЕ топоним — после точки +# цифра, lookahead не матчит) от «д. Кашино» (деревня — после точки буква, +# матчит) БЕЗ явного исключения "д." из списка: раньше "д." исключали +# целиком, потому что не было способа отличить дом от деревни — lookahead +# даёт этот способ, поэтому "д."/"дер." теперь можно включить безопасно. _LOCALITY_MARKER_RE = re.compile( - r"\b(?:обл\.?|область|край|республика|респ\.?|г|пос\.?|посёлок|поселок|с\.)\b" + r"\b(?:обл\.?|область|край|республика|респ\.?|г|пос\.?|посёлок|поселок)\b" + r"|\b(?:с|д|дер|пгт|ст)\.\s*(?=[а-яё])" ) @@ -309,10 +327,18 @@ def _names_unrecognized_locality(address: str) -> bool: Отличает «город не назван» (bare «Ленина 1» — так приходит основной трафик формы оценки; geoportal резолвит его как ЕКБ и это оправданный - дефолт) от «назван НЕЗНАКОМЫЙ город» (Ялта/Трёхгорный/Реж/Лёвиха — - структурный маркер топонима есть, но это не ЕКБ и не известный город - региона 66) — только во втором случае EKB-only локальные тиры нельзя - пускать. + дефолт) от «назван НЕЗНАКОМЫЙ город» (Ялта/Трёхгорный/Лёвиха, «с. Кашино», + «д. Родина» — структурный маркер топонима есть, но это не ЕКБ и не + известный город региона 66) — только во втором случае EKB-only локальные + тиры нельзя пускать. + + Известная дыра (сознательно, не «баг» — закрывать architectural-решением + отдельно, не перечислением городов): голый «Реж, Ленина 5» / «Арамиль, + Мира 1» БЕЗ структурного маркера («обл.»/«г.»/«с.» и т.п.) и без города + из списка `SVERDLOVSK_OBLAST_CITIES` не ловится — падает в «город не + назван» и уходит в ЕКБ-дефолт. Все живые прод-подмены (geocode_cache) + были с маркером/региональным префиксом — см. тесты `test_ekb_local_tiers_allowed` + (изолирующий кейс на голое «реж, ленина 5» зафиксирован как известная дыра). """ normalized = " ".join(address.lower().split()) if _EKATERINBURG_RE.search(normalized): @@ -330,15 +356,25 @@ def _ekb_local_tiers_allowed(address: str, city_hint: str | None = None) -> bool - известный город региона 66 в ТЕКСТЕ адреса (`_names_non_ekb_city`, без изменений, #11); - - `city_hint` называет известный город региона 66, отличный от ЕКБ — - #2580 (C1): без этой проверки `city_hint` мёртв для локальных тиров — - caller передал, например, "Нижний Тагил" для текста без города, но - всё равно получал бы "точный" ЕКБ-хит с `city_ambiguous=False` (хинт - ведь был!) — ложно-уверенный неверный результат; + - `city_hint` ЗАДАН и НЕ является Екатеринбургом (`hint_blocks` ниже) — + #2580 (C1) + #2589 (deep-review M2): `city_hint` — явный СТРУКТУРНЫЙ + вход от caller'а (не свободный текст адреса), поэтому для него + fail-closed форма ОБРАТНАЯ по сравнению с текстом адреса: «не + Екатеринбург → закрыто», а не «есть в списке 37 → закрыто». Список + 37 годился бы, только если единственный риск — известные города; но + `city_hint` принимает произвольную строку без валидации (в т.ч. через + API напрямую, минуя 6-городный selector UI) — «Реж»/любой седьмой + город, добавленный в реестр, до сих пор не в списке 37 → список-based + проверка держала гейт открытым, geoportal отдавал точный ЕКБ-дом, а + `city_ambiguous=False` (хинт ведь БЫЛ) делало это уверенно-неверным — + худший случай (без hint был бы честный `city_ambiguous=True`); - незнакомый топоним в ТЕКСТЕ адреса — структурный маркер - (`_names_unrecognized_locality`, #2582: «обл./край/республика/г./пос./ - с.») — покрывает города вне списка 37 (Реж/Лёвиха) и ДРУГИЕ РЕГИОНЫ РФ - целиком (Ялта, Трёхгорный), не требуя их перечислять. + (`_names_unrecognized_locality`, #2582: «обл./край/республика/г./пос.» + и однобуквенные «с./д./дер./пгт./ст.» с lookahead на кириллицу после + точки) — покрывает города вне списка 37 С маркером (Лёвиха, «с. + Новоалексеевское») и ДРУГИЕ РЕГИОНЫ РФ целиком (Ялта, Трёхгорный), не + требуя их перечислять. Голый город без маркера и вне списка (Реж без + «обл.») — известная дыра, см. `_names_unrecognized_locality`. До #2582 решение по третьему пункту принималось от противного: «нет в списке 37 городов → считаем Екатеринбургом» — молча ломало любой адрес @@ -348,13 +384,14 @@ def _ekb_local_tiers_allowed(address: str, city_hint: str | None = None) -> bool `city_hint`, противоречащий явному "Екатеринбург" в тексте адреса (`_names_non_ekb_city(address)` для такого текста вернёт False и не закроет гейт сам по себе — Екатеринбург в тексте не мешает), тем не менее - ЗАКРОЕТ гейт через `hint_names_non_ekb` — тот же trade-off, что уже был - одобрен в #2580 (`use_local_ekb`), сюда не переписываем. Нормальный кейс — - caller не передаёт противоречащие address и city_hint одновременно. + ЗАКРОЕТ гейт через `hint_blocks` — тот же trade-off, что уже был одобрен + в #2580 (`use_local_ekb`), сюда не переписываем. Нормальный кейс — caller + не передаёт противоречащие address и city_hint одновременно. """ - hint_names_non_ekb = bool(city_hint) and _names_non_ekb_city(city_hint) + hint = " ".join((city_hint or "").lower().split()) + hint_blocks = bool(hint) and not _EKATERINBURG_RE.search(hint) return not ( - _names_non_ekb_city(address) or hint_names_non_ekb or _names_unrecognized_locality(address) + _names_non_ekb_city(address) or hint_blocks or _names_unrecognized_locality(address) ) diff --git a/tradein-mvp/backend/tests/test_geocoder_bbox.py b/tradein-mvp/backend/tests/test_geocoder_bbox.py index f5d49d3b..eacc7d3a 100644 --- a/tradein-mvp/backend/tests/test_geocoder_bbox.py +++ b/tradein-mvp/backend/tests/test_geocoder_bbox.py @@ -18,6 +18,7 @@ import httpx import pytest from app.services.geocoder import ( + _LOCALITY_MARKER_RE, EKB_BBOX_TIGHT, EKB_BBOX_WIDE, OBLAST66_BBOX, @@ -232,12 +233,57 @@ def test_names_non_ekb_city(address: str, expected: bool) -> None: # явный Екатеринбург побеждает даже при наличии маркера "г": ("г. екатеринбург, ул. малышева, 30", False), ("г екатеринбург, ленинский р-н, ул серова, д 35", False), + # ── Изолирующие кейсы (#2589 M3) — параметризация выше зелёная и на + # маркер, и на «обл./республика/край» одновременно (все реальные + # прод-адреса их несут вместе); ниже — маркер БЕЗ регионального + # префикса, чтобы поймать регрессию именно сигнала-маркера (это и + # проехало у H1: мёртвый «с\.» был бы незамечен без таких кейсов). + ( + "с. новоалексеевское, ул. 8 марта,37", + True, + ), # живой инцидент (#2589 H1) — «с.» БЕЗ доп. маркера + ("с. кашино, ул. ленина, 5", True), # изолированный «с.» (M3) + ("д. родина, ул. мира, 1", True), # изолированный «д.» (M3, lookahead-фикс) + ("пгт. шаля, ул. советская, 2", True), # изолированный «пгт.» (M3) + ( + "пос. мичуринский, широкореченская ул., 45", + True, + ), # пригородный посёлок ЕКБ — маркер закрывает гейт (M3, честно + # закреплено: даже для формально «своих» посёлков без «Екатеринбург» + # в тексте фолбэк на oblast-aware внешние провайдеры — не регрессия + # координат, просто не через fast-path geoportal) + ("реж, ленина 5", False), # ИЗВЕСТНАЯ ДЫРА (#2589 M3): голый город вне + # списка 37 БЕЗ маркера — не ловится, уходит в ЕКБ-дефолт. Честно + # зафиксировано, не «фиксим» здесь (см. docstring — architectural + # follow-up отдельно, не через список городов). + # house-маркер НЕ должен матчить как топоним (регрессия #2582→#2589): + ("ул. ленина, д. 5", False), + ("д. 5", False), + ("д. 5 стр. 2", False), ], ) def test_names_unrecognized_locality(address: str, expected: bool) -> None: assert _names_unrecognized_locality(address) is expected +@pytest.mark.parametrize( + "text,expected", + [ + # Ровно набор из deep-review (#2589 H1) — «Проверь сам на живом Python». + ("д. 5", False), + ("д. 5 стр. 2", False), + ("ул. ленина, д. 5", False), + ("с. кашино", True), + ("д. родина", True), + ("пгт. шаля", True), + ], +) +def test_locality_marker_re_isolated(text: str, expected: bool) -> None: + """Regex-уровень, изолированно от `_names_unrecognized_locality`'s + Екатеринбург-early-return — прямая проверка `_LOCALITY_MARKER_RE` (#2589 M3/H1).""" + assert bool(_LOCALITY_MARKER_RE.search(text)) is expected + + @pytest.mark.parametrize( "address,expected", [ @@ -260,16 +306,24 @@ def test_names_unrecognized_locality(address: str, expected: bool) -> None: ("проспект Ленина, 1", True), # мкр-исключение (район внутри ЕКБ, не ЗАТО Заречный) — гейт открыт: ("мкр Заречный, ул. Ленина 5", True), + # изолирующий кейс (#2589 H1/M3) — «с.»-маркер БЕЗ доп. «обл.»/«республика»: + ("с. Новоалексеевское, ул. 8 Марта,37", False), + # house-маркер «д.» НЕ закрывает гейт (регрессия H1-фикса): + ("ул. Ленина, д. 5", True), ], ) def test_ekb_local_tiers_allowed(address: str, expected: bool) -> None: assert _ekb_local_tiers_allowed(address) is expected -# ── _ekb_local_tiers_allowed × city_hint (#2580 merge, #2582) ─────────────── +# ── _ekb_local_tiers_allowed × city_hint (#2580 merge, #2582, #2589 M2) ───── # Обе логики независимо закрывают гейт (OR трёх сигналов) — проверяем, что -# ни одна из четырёх веток не стала недостижимой после слияния #2580/#2582: +# ни одна из веток не стала недостижимой после слияния #2580/#2582/#2589-M2: # A. city_hint называет известный город региона 66 (не ЕКБ) → закрыто. +# A2. city_hint — ЛЮБАЯ непустая строка, отличная от "Екатеринбург" (в т.ч. +# НЕ из списка 37 — #2589 M2, было главной дырой: `city_hint="Реж"` +# раньше держал гейт открытым, geoportal отдавал точный ЕКБ-дом с +# `city_ambiguous=False` — уверенно-неверный результат) → закрыто. # B. незнакомый топоним в ТЕКСТЕ адреса (маркер) → закрыто. # C. ни того ни другого → открыто (дефолт ЕКБ). # D. явный city_hint="Екатеринбург" (без конфликта в тексте) → открыто. @@ -282,6 +336,12 @@ def test_ekb_local_tiers_allowed(address: str, expected: bool) -> None: # если сам текст адреса города вообще не называет (#2580 C1). ("Ленина, 1", "Нижний Тагил", False, "A: hint=известный не-ЕКБ город"), ("Серова 27", "Каменск-Уральский", False, "A: hint=известный не-ЕКБ город (2)"), + # A2 (#2589 M2 — ключевой регрессионный кейс фикса): hint называет + # город, КОТОРОГО НЕТ в списке 37 (произвольный текст через API, не + # UI-selector) → тоже закрыто. До фикса `_names_non_ekb_city(city_hint)` + # давал False (Реж не в списке) → гейт был открыт. + ("Ленина, 1", "Реж", False, "A2: hint=не-ЕКБ город ВНЕ списка 37"), + ("Серова 27", "Ялта", False, "A2: hint=город другого региона"), # B: незнакомый топоним в тексте (маркер) закрывает гейт независимо # от hint (#2582) — hint здесь не задан, чистая проверка B. ("свердловская обл., реж, ул. ленина, 5", None, False, "B: маркер в тексте"), @@ -289,7 +349,8 @@ def test_ekb_local_tiers_allowed(address: str, expected: bool) -> None: ("Малышева 30", None, True, "C: ничего не названо"), ("Серова 27", "", True, "C: пустой hint == отсутствию hint'а"), # D: явный city_hint="Екатеринбург" без конфликта в тексте → открыто — - # ключевая проверка, что #2580 C1 не сломал дефолтный ЕКБ-путь через hint. + # ключевая проверка, что #2580 C1 / #2589 M2 не сломали дефолтный + # ЕКБ-путь через hint. ("Ленина, 1", "Екатеринбург", True, "D: hint=Екатеринбург"), ("Серова 27", "екатеринбург", True, "D: hint=екатеринбург (регистр)"), ], diff --git a/tradein-mvp/backend/tests/test_geocoder_cadastral_matcher.py b/tradein-mvp/backend/tests/test_geocoder_cadastral_matcher.py index 70707996..3e179c8c 100644 --- a/tradein-mvp/backend/tests/test_geocoder_cadastral_matcher.py +++ b/tradein-mvp/backend/tests/test_geocoder_cadastral_matcher.py @@ -326,6 +326,10 @@ async def test_geocode_skips_ekb_local_tiers_for_non_ekb_city() -> None: "Лёвиха (город области ВНЕ списка 37)", ), ("свердловская обл., реж, ул. ленина, 5", "Реж (город области ВНЕ списка 37)"), + ( + "с. Новоалексеевское, ул. 8 Марта,37", + "Новоалексеевское (живой инцидент #2589 H1 — «с.»-маркер без доп. «обл.»)", + ), ], ) async def test_geocode_skips_ekb_local_tiers_for_unrecognized_locality( @@ -372,6 +376,7 @@ async def test_geocode_skips_ekb_local_tiers_for_unrecognized_locality( ("Серова 27", "ЕКБ без города (основной трафик формы оценки)"), ("Малышева 30", "ЕКБ без города (основной трафик формы оценки)"), ("г. Екатеринбург, ул. Серова, д. 27", "ЕКБ с явным городом"), + ("ул. Серова, д. 27", "house-маркер «д.» не закрывает гейт (#2589 H1 regression)"), ], ) async def test_geocode_still_uses_ekb_local_tiers_for_real_ekb_address( @@ -427,6 +432,53 @@ async def test_geocode_explicit_ekaterinburg_city_hint_still_uses_local_tiers() mock_geoportal.assert_called_once() +async def test_geocode_city_hint_unknown_city_skips_local_ekb_tiers() -> None: + """#2589 M2 (deep-review fix): `city_hint="Реж"` — город ВНЕ списка 37 и + без структурного маркера в самом тексте адреса — обязан закрыть ЕКБ-only + локальные тиры так же, как известный `city_hint`. До фикса + `hint_names_non_ekb = _names_non_ekb_city(city_hint)` был list-based и + держал гейт открытым для ЛЮБОГО города вне списка 37 (Реж, седьмой город + после расширения UI-selector'а, произвольный текст через API) — geoportal + отдавал точный ЕКБ-хит, а `city_ambiguous=False` (хинт ведь БЫЛ) делало + это уверенно-неверным результатом (хуже, чем честный `city_ambiguous=True` + без хинта вообще).""" + db = MagicMock() + ekb_hit = GeocodeSuggestion( + label="Ленина, 1, Екатеринбург", + full_address="Ленина, 1, Екатеринбург", + lat=56.83788, + lon=60.58018, + kind="house", + ) + rezh_result = GeocodeResult( + lat=57.371, lon=61.386, full_address="Ленина, 1, Реж", provider="nominatim" + ) + + with ( + patch("app.services.geocoder._cache_get", return_value=None), + patch("app.services.geocoder._cache_put"), + patch( + "app.services.geocoder._geoportal_house_match", return_value=ekb_hit + ) as mock_geoportal, + patch("app.services.geocoder._cadastral_house_match", return_value=ekb_hit) as mock_house, + patch("app.services.geocoder._cadastral_forward_sync", return_value=[]), + patch("app.services.geocoder.settings") as mock_settings, + patch( + "app.services.geocoder._nominatim_lookup", + new_callable=AsyncMock, + return_value=rezh_result, + ), + ): + mock_settings.yandex_geocoder_api_key = None + result = await geocode("Ленина, 1", db, city_hint="Реж") + + mock_geoportal.assert_not_called() + mock_house.assert_not_called() + assert result is not None + assert result.lat == pytest.approx(57.371) # Реж, НЕ подставленный ЕКБ-хит (56.83788) + assert result.lat != pytest.approx(56.83788) + + # ── suggest() wiring ─────────────────────────────────────────────────────────