diff --git a/tradein-mvp/backend/app/services/geocoder.py b/tradein-mvp/backend/app/services/geocoder.py index 1ec06c79..4e0bd190 100644 --- a/tradein-mvp/backend/app/services/geocoder.py +++ b/tradein-mvp/backend/app/services/geocoder.py @@ -462,12 +462,21 @@ async def _nominatim_query(client: httpx.AsyncClient, address: str) -> dict | No @retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=1, max=8)) -async def _nominatim_lookup(address: str) -> GeocodeResult | None: +async def _nominatim_lookup(address: str, city_hint: str | None = None) -> GeocodeResult | None: """OSM Nominatim — бесплатно, без ключа, 1 req/sec policy. Бан-policy: User-Agent с email обязателен. Tier 1: bounded область (region 66) на оригинальный адрес. Tier 2: bounded область (region 66) на typo-варианты (Цвилинга → Цвиллинга). + + #2580 (C): city_hint, если известен, подставляется в текст запроса — без + него `_nominatim_query` полагается ТОЛЬКО на oblast66-bbox фильтр + tie-break + (предпочитает tight-ЕКБ bbox), который для одноимённых улиц ВНУТРИ региона + (напр. "Ленина" — и в Екатеринбурге, и в с. Свердловское) не различает город. + Эмпирически подтверждено: "Ленина 1" без города → случайное село внутри + области; "Нижний Тагил, Ленина 1" → корректно резолвится. Раз Yandex-ключ + сейчас недействителен (#2585), это единственный живой внешний провайдер — + city_hint должен реально влиять на его результат, не только на кэш-ключ. """ headers = { "User-Agent": f"TradeInMVP/0.1 (contact: {settings.contact_email})", @@ -475,15 +484,19 @@ async def _nominatim_lookup(address: str) -> GeocodeResult | None: "Accept-Language": "ru,en;q=0.8", "Referer": "https://tradein-mvp.local/", } + city, _ = _resolve_city_for_geocode(address, city_hint) + query = f"{city}, {address}" if city else address async with httpx.AsyncClient(timeout=10.0, headers=headers) as client: # Tier 1: оригинал - item = await _nominatim_query(client, address) + item = await _nominatim_query(client, query) # Tier 2: typo-variants if item is None: for variant in _typo_variants(address, limit=4): await asyncio.sleep(1.0) # Nominatim 1 req/sec policy - item = await _nominatim_query(client, variant) + variant_city, _ = _resolve_city_for_geocode(variant, city_hint) + variant_query = f"{variant_city}, {variant}" if variant_city else variant + item = await _nominatim_query(client, variant_query) if item is not None: logger.info("nominatim typo-fixed: %s → %s", address, variant) break @@ -837,38 +850,97 @@ async def _nominatim_query_multi(client: httpx.AsyncClient, query: str, limit: i return data if isinstance(data, list) else [] +def _dedupe_nominatim_items(*item_lists: list[dict]) -> list[dict]: + """Объединяет несколько списков raw Nominatim items в один, без дублей. + + Дедуп по `place_id` (если есть), иначе по округлённым координатам. Порядок + сохраняется: элементы из более раннего списка идут первыми (приоритет). + """ + seen: set[tuple[object, ...]] = set() + out: list[dict] = [] + for items in item_lists: + for item in items: + place_id = item.get("place_id") + key: tuple[object, ...] + if place_id is not None: + key = ("place_id", place_id) + else: + try: + key = ("latlon", round(float(item["lat"]), 5), round(float(item["lon"]), 5)) + except (KeyError, ValueError, TypeError): + key = ("raw", item.get("display_name")) + if key in seen: + continue + seen.add(key) + out.append(item) + return out + + +async def _nominatim_query_city_aware( + client: httpx.AsyncClient, query: str, city: str | None, city_specified: bool, limit: int +) -> list[dict]: + """Строит и выполняет Nominatim-запрос(ы) с учётом того, известен ли город. + + Три случая (см. `_resolve_city_for_geocode`): + 1. `city` не None (`city_hint` подставлен) → один suffixed-запрос с ним. + 2. `city` is None, но `city_specified=True` (маркер УЖЕ в тексте адреса, + например "Екатеринбург, Малышева 30") → запрос БЕЗ доп. суффикса — город + уже есть в тексте, дублировать его нельзя (иначе "X, Екатеринбург, + Екатеринбург" ломает матчинг). + 3. `city` is None и `city_specified=False` — город НЕизвестен вообще (#2580 / + C2, regression test "Победы 20"): один bare-запрос БЕЗ текстового суффикса + неожиданно теряет часть настоящих ЕКБ-адресов — Nominatim без подсказки о + городе иногда предпочитает street-level матч в соседнем городе-спутнике + (напр. "Победы 20" без суффикса → улица Победы, Верхняя Пышма) более + специфичному named-place матчу в ЕКБ ("Парк Победы" ЖК, Екатеринбург). + Поэтому делаем ДВА запроса — bare (честный oblast-wide поиск, не теряет + реальные адреса других городов) И ЕКБ-suffixed (majority трафика) — и + ОБЪЕДИНЯЕМ результаты (не заменяем один другим): оба честных кандидата + остаются в списке, пользователь выбирает нужный сам из подсказок. + ЕКБ-кандидаты идут первыми (majority-случай, привычный порядок). + """ + if city: + return await _nominatim_query_multi(client, f"{query}, {city}", limit) + if city_specified: + return await _nominatim_query_multi(client, query, limit) + ekb_data = await _nominatim_query_multi(client, f"{query}, Екатеринбург", limit) + await asyncio.sleep(1.0) # Nominatim 1 req/sec policy — два запроса подряд + bare_data = await _nominatim_query_multi(client, query, limit) + return _dedupe_nominatim_items(ekb_data, bare_data)[:limit] + + @retry(stop=stop_after_attempt(2), wait=wait_exponential(multiplier=1, min=1, max=4)) async def _nominatim_suggest( query: str, limit: int = 8, city_hint: str | None = None ) -> list[GeocodeSuggestion]: """Nominatim в режиме suggest. С typo-fallback (для случаев когда Yandex недоступен). - Суффикс города навязывается ТОЛЬКО если он известен: адрес уже называет - город/область региона 66, либо передан явный `city_hint`. Раньше (#2576) - при отсутствии обоих сюда молча подставлялся суффикс ", Екатеринбург" — - географию поиска это не расширяло/не сужало (`_nominatim_query_multi` и - так bounded=1 по ВСЕЙ области `OBLAST66_VIEWBOX`), но текстовый суффикс - смещал ранжирование Nominatim в пользу ЕКБ-совпадений даже для адресов - из других городов области. + Суффикс города навязывается, только если он известен: адрес уже называет + город/область региона 66, либо передан явный `city_hint`. Если город + НЕизвестен — см. `_nominatim_query_city_aware` (dual-query, C2): раньше + (#2576) здесь молча подставлялся суффикс ", Екатеринбург" всегда; чистое + удаление суффикса (без dual-query) регрессило часть реальных ЕКБ-адресов + (см. C2 в #2580) — поэтому оба честных варианта объединяются, не заменяют + друг друга. """ headers = { "User-Agent": f"TradeInMVP/0.1 (contact: {settings.contact_email})", "Accept": "application/json", "Accept-Language": "ru,en;q=0.8", } - city, _ = _resolve_city_for_geocode(query, city_hint) - suffixed_query = f"{query}, {city}" if city else query + city, city_specified = _resolve_city_for_geocode(query, city_hint) async with httpx.AsyncClient(timeout=8.0, headers=headers) as client: # Tier 1: оригинальный query - data = await _nominatim_query_multi(client, suffixed_query, limit) + data = await _nominatim_query_city_aware(client, query, city, city_specified, limit) # Tier 2: typo-варианты если оригинал пустой if not data: for variant in _typo_variants(query, limit=3): await asyncio.sleep(1.0) # Nominatim 1 req/sec - variant_city, _ = _resolve_city_for_geocode(variant, city_hint) - variant_query = f"{variant}, {variant_city}" if variant_city else variant - data = await _nominatim_query_multi(client, variant_query, limit) + variant_city, variant_specified = _resolve_city_for_geocode(variant, city_hint) + data = await _nominatim_query_city_aware( + client, variant, variant_city, variant_specified, limit + ) if data: logger.info("nominatim suggest typo-fixed: %s → %s", query, variant) break @@ -1221,7 +1293,10 @@ async def suggest( # другой город области, иначе не-ЕКБ автокомплит может всплыть ЕКБ-домом # с совпадающими улица+дом. Внешние тиры (2/3/4 ниже) не гейтим — они уже # oblast-aware. - if db is not None and not _names_non_ekb_city(query): + # #2580 (C1): city_hint тоже гейтит — иначе он мёртвый параметр для этого + # тира (см. `geocode()` use_local_ekb выше — тот же принцип). + hint_names_non_ekb = bool(city_hint) and _names_non_ekb_city(city_hint) + if db is not None and not (_names_non_ekb_city(query) or hint_names_non_ekb): # 1a. Anchored house-match: парсим street+house → точный матч по дом-маркеру. # Решает кейс «Серова 27» где raw-ILIKE по readable_address давал 0 hits. parsed = _parse_street_house(query.strip()) @@ -1307,7 +1382,16 @@ async def geocode(address: str, db: Session, city_hint: str | None = None) -> Ge # адрес другого города области — иначе улица+дом, коллизящие с ЕКБ-домом # (напр. "проспект Ленина 1" есть и в Нижнем Тагиле, и в ЕКБ), снапаются в # ЕКБ. Пропускаем сразу к oblast-aware внешним провайдерам ниже (3/4). - use_local_ekb = not _names_non_ekb_city(address) + # #2580 (C1): city_hint ДОЛЖЕН участвовать в этом гейте — иначе вызывающий + # код, явно передавший city_hint="Нижний Тагил" для "Ленина 1" (в самом + # тексте адреса города нет), всё равно попадает на geoportal/cadastral + # (ЕКБ-only базы), получает "точный" ЕКБ-хит и city_ambiguous=False (хинт + # ведь был!) — т.е. систему, которая раньше просто не знала город, теперь + # ложно-уверенно утверждает неверный. `_names_non_ekb_city` НЕ переписываем + # (гейт/gazetteer — отдельный дефект #2582), только добавляем city_hint + # на вход той же самой проверки. + hint_names_non_ekb = bool(city_hint) and _names_non_ekb_city(city_hint) + use_local_ekb = not (_names_non_ekb_city(address) or hint_names_non_ekb) # 2a. Геопортал ЕКБ — ПЕРВЫЙ локальный tier (полнее cad_buildings ~на 70%). if use_local_ekb and parsed is not None: @@ -1418,7 +1502,7 @@ async def geocode(address: str, db: Session, city_hint: str | None = None) -> Ge # 4. Nominatim fallback try: - result = await _nominatim_lookup(address) + result = await _nominatim_lookup(address, city_hint) if result is not None: result = replace(result, city_ambiguous=city_ambiguous) await asyncio.to_thread(_cache_put, db, addr_norm, result) diff --git a/tradein-mvp/backend/tests/test_geocoder_city_hint.py b/tradein-mvp/backend/tests/test_geocoder_city_hint.py index fd2159e8..5a5666c2 100644 --- a/tradein-mvp/backend/tests/test_geocoder_city_hint.py +++ b/tradein-mvp/backend/tests/test_geocoder_city_hint.py @@ -34,12 +34,14 @@ from app.services.geocoder import ( EKB_BBOX, OBLAST66_VIEWBOX, GeocodeResult, + GeocodeSuggestion, _cache_key, _nominatim_suggest, _resolve_city_for_geocode, _yandex_lookup, _yandex_suggest, geocode, + suggest, ) # ── _resolve_city_for_geocode ──────────────────────────────────────────────── @@ -227,16 +229,11 @@ async def test_yandex_suggest_explicit_ekb_unchanged() -> None: # ── _nominatim_suggest ─────────────────────────────────────────────────────── -async def test_nominatim_suggest_no_city_no_suffix() -> None: - """#2576: без города — Nominatim-запрос БЕЗ ", Екатеринбург"-суффикса. - - Географию не расширяет/не сужает (`_nominatim_query_multi` уже bounded=1 - по всей области `OBLAST66_VIEWBOX`) — но суффикс раньше смещал ранжирование - Nominatim в пользу ЕКБ-совпадений для адресов из других городов области. - - NB: с пустым результатом (как здесь) `_nominatim_suggest` уходит дальше в - typo-fallback Tier 2 (несколько доп. вызовов) — берём ПЕРВЫЙ вызов (Tier 1, - оригинальный query), не последний. +async def test_nominatim_suggest_no_city_dual_query_both_variants_sent() -> None: + """#2580 (C2): без города — Nominatim получает ОБА запроса: bare (честный, + без города) И ЕКБ-suffixed (majority-трафик). Не подмена одним вариантом — + объединение (см. `test_nominatim_suggest_pobedy20_ekb_result_not_lost` ниже + — чистое удаление суффикса теряло реальные ЕКБ-адреса). """ calls: list[str] = [] @@ -244,14 +241,18 @@ async def test_nominatim_suggest_no_city_no_suffix() -> None: calls.append(query) return [] - with patch( - "app.services.geocoder._nominatim_query_multi", new=AsyncMock(side_effect=fake_query_multi) + with ( + patch( + "app.services.geocoder._nominatim_query_multi", + new=AsyncMock(side_effect=fake_query_multi), + ), + patch("app.services.geocoder.asyncio.sleep", new=AsyncMock()), ): result = await _nominatim_suggest("Ленина, 1") assert result == [] - assert calls[0] == "Ленина, 1" - assert "Екатеринбург" not in calls[0] + assert "Ленина, 1" in calls # bare — честный, без города + assert "Ленина, 1, Екатеринбург" in calls # ЕКБ-вариант — не потерян async def test_nominatim_suggest_city_hint_suffix() -> None: @@ -284,6 +285,81 @@ async def test_nominatim_suggest_explicit_ekb_unchanged() -> None: assert calls[0] == "Екатеринбург, Малышева 30" +async def test_nominatim_suggest_pobedy20_ekb_result_not_lost() -> None: + """#2580 (C2) regression — "Победы 20" (реальный кейс с прода, подтверждён + ревьюером): без города ЕКБ-кандидат ('Парк Победы' ЖК, Екатеринбург) должен + остаться в подсказках, НЕ потеряться в пользу похожего street-level матча + в Верхней Пышме. + + Симулирует реальные координаты: + 'Победы 20, Екатеринбург' → 56.899, 60.579 (ЖК "Парк Победы", Екатеринбург) + 'Победы 20' → 56.964, 60.610 (ул. Победы, Верхняя Пышма) + """ + ekb_item = { + "place_id": 1001, + "lat": "56.899", + "lon": "60.579", + "display_name": 'ЖК "Парк Победы", Орджоникидзевский район, Екатеринбург', + "address": {"road": "Победы", "house_number": "20", "suburb": "Орджоникидзевский район"}, + } + pyshma_item = { + "place_id": 1002, + "lat": "56.964", + "lon": "60.610", + "display_name": "улица Победы, 20, Верхняя Пышма", + "address": {"road": "улица Победы", "house_number": "20"}, + } + + async def fake_query_multi(client, query, limit): + if query.endswith(", Екатеринбург"): + return [ekb_item] + return [pyshma_item] + + with ( + patch( + "app.services.geocoder._nominatim_query_multi", + new=AsyncMock(side_effect=fake_query_multi), + ), + patch("app.services.geocoder.asyncio.sleep", new=AsyncMock()), + ): + result = await _nominatim_suggest("Победы 20") + + assert result, "ожидались подсказки" + ekb_hits = [r for r in result if r.lat == pytest.approx(56.899)] + assert ekb_hits, "ЕКБ-кандидат ('Парк Победы') должен остаться в подсказках, не потеряться" + # ЕКБ-кандидат идёт первым (majority-трафик — привычный порядок для основных пользователей). + assert result[0].lat == pytest.approx(56.899) + # Верхняя Пышма тоже осталась в списке — honest alternative, не подменена. + pyshma_hits = [r for r in result if r.lat == pytest.approx(56.964)] + assert pyshma_hits, "не-ЕКБ кандидат тоже должен остаться (объединение, не замена)" + + +async def test_nominatim_suggest_dedupe_across_dual_query() -> None: + """Если bare и ЕКБ-suffixed запросы возвращают ОДИН и тот же item (по place_id) + — он не дублируется в итоговом списке подсказок.""" + same_item = { + "place_id": 42, + "lat": "56.838", + "lon": "60.605", + "display_name": "ул. Малышева, 30, Екатеринбург", + "address": {"road": "ул. Малышева", "house_number": "30"}, + } + + async def fake_query_multi(client, query, limit): + return [same_item] + + with ( + patch( + "app.services.geocoder._nominatim_query_multi", + new=AsyncMock(side_effect=fake_query_multi), + ), + patch("app.services.geocoder.asyncio.sleep", new=AsyncMock()), + ): + result = await _nominatim_suggest("Малышева 30") + + assert len(result) == 1, "одинаковый place_id из обоих запросов не должен дублироваться" + + # ── geocode() — city_ambiguous flag ────────────────────────────────────────── @@ -298,6 +374,101 @@ def _geocode_patches(yandex_result: GeocodeResult | None): ) +# ── C1 (#2580) — city_hint должен доходить до локальных ЕКБ-only тиров ────── + + +async def test_geocode_city_hint_non_ekb_skips_local_ekb_tiers() -> None: + """#2580 (C1): city_hint="Нижний Тагил" должен ЗАПРЕТИТЬ geoportal/cadastral + (ЕКБ-only базы) — иначе они возвращают "точный" ЕКБ-хит для улицы, которая + совпадает по названию, а `city_ambiguous=False` (хинт был!) делает такой + неверный результат ложно-уверенным. Мок geoportal нарочно возвращает ЕКБ-хит + (как в проде) — фикс должен НЕ дать ему сработать вообще. + """ + db = MagicMock() + ekb_hit = GeocodeSuggestion( + label="Ленина, 1, Екатеринбург", + full_address="Ленина, 1, Екатеринбург", + lat=56.83788, + lon=60.58018, + kind="house", + ) + tagil_result = GeocodeResult( + lat=57.905, lon=59.950, full_address="Ленина, 1, Нижний Тагил", provider="yandex" + ) + with patch("app.services.geocoder.settings") as mock_settings: + mock_settings.yandex_geocoder_api_key = "fake" + with contextlib.ExitStack() as stack: + stack.enter_context(patch("app.services.geocoder._cache_get", return_value=None)) + stack.enter_context(patch("app.services.geocoder._cache_put")) + geoportal_mock = stack.enter_context( + patch("app.services.geocoder._geoportal_house_match", return_value=ekb_hit) + ) + cadastral_mock = stack.enter_context( + patch("app.services.geocoder._cadastral_house_match", return_value=ekb_hit) + ) + stack.enter_context( + patch("app.services.geocoder._cadastral_forward_sync", return_value=[]) + ) + stack.enter_context( + patch( + "app.services.geocoder._yandex_lookup", + new=AsyncMock(return_value=tagil_result), + ) + ) + result = await geocode("Ленина, 1", db, city_hint="Нижний Тагил") + + geoportal_mock.assert_not_called() + cadastral_mock.assert_not_called() + assert result is not None + assert result.lat == pytest.approx(57.905) # Тагил, НЕ подставленный ЕКБ-хит (56.838) + assert result.lat != pytest.approx(56.83788) + + +async def test_geocode_real_ekb_address_still_uses_local_tiers() -> None: + """Сквозной кейс: реальный ЕКБ-адрес БЕЗ city_hint по-прежнему резолвится через + локальный geoportal-тир (ЕКБ-путь не деградировал после C1-фикса).""" + db = MagicMock() + ekb_hit = GeocodeSuggestion( + label="Малышева, 30, Екатеринбург", + full_address="Малышева, 30, Екатеринбург", + lat=56.8389, + lon=60.6057, + kind="house", + ) + with contextlib.ExitStack() as stack: + stack.enter_context(patch("app.services.geocoder._cache_get", return_value=None)) + stack.enter_context(patch("app.services.geocoder._cache_put")) + geoportal_mock = stack.enter_context( + patch("app.services.geocoder._geoportal_house_match", return_value=ekb_hit) + ) + result = await geocode("Малышева, 30", db) + + geoportal_mock.assert_called_once() + assert result is not None + assert result.lat == pytest.approx(56.8389) + assert result.confidence == "exact" + assert result.city_ambiguous is True # город не указан — честный флаг + + +async def test_suggest_city_hint_non_ekb_skips_cadastral_tier1() -> None: + """#2580 (C1): suggest(city_hint="Нижний Тагил") — Tier 1 (кадастр ЕКБ) НЕ должен + вызываться (раньше был мёртвым параметром для этого тира).""" + db = MagicMock() + with contextlib.ExitStack() as stack: + house_mock = stack.enter_context(patch("app.services.geocoder._cadastral_house_match")) + forward_mock = stack.enter_context(patch("app.services.geocoder._cadastral_forward_sync")) + mock_settings = stack.enter_context(patch("app.services.geocoder.settings")) + mock_settings.dadata_api_token = None + mock_settings.yandex_geocoder_api_key = None + stack.enter_context( + patch("app.services.geocoder._nominatim_suggest", new=AsyncMock(return_value=[])) + ) + await suggest("Ленина, 1", db=db, city_hint="Нижний Тагил") + + house_mock.assert_not_called() + forward_mock.assert_not_called() + + async def test_geocode_city_ambiguous_true_when_no_city_known() -> None: """Ни адрес, ни city_hint не называют город → city_ambiguous=True.""" db = MagicMock()