From 4b1a282234323dc9a2d1f26aec7ffd979f1b7404 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Fri, 31 Jul 2026 16:42:29 +0300 Subject: [PATCH 1/2] =?UTF-8?q?fix(tradein/geocoder):=20=D0=BD=D0=B5=20?= =?UTF-8?q?=D0=BF=D0=BE=D0=B4=D1=81=D1=82=D0=B0=D0=B2=D0=BB=D1=8F=D1=82?= =?UTF-8?q?=D1=8C=20=D0=95=D0=BA=D0=B0=D1=82=D0=B5=D1=80=D0=B8=D0=BD=D0=B1?= =?UTF-8?q?=D1=83=D1=80=D0=B3=20=D0=BC=D0=BE=D0=BB=D1=87=D0=B0=20=E2=80=94?= =?UTF-8?q?=20=D1=8F=D0=B2=D0=BD=D1=8B=D0=B9=20city=5Fhint=20(#2576)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Раньше _yandex_lookup/_yandex_suggest/_nominatim_suggest молча подставляли "Екатеринбург, " в запрос, если в адресе не было маркера города/области. Житель Нижнего Тагила, вводя «Ленина, 1», получал уверенно неверную цену по екатеринбургской улице Ленина (обе улицы называются одинаково) — фронт город вообще не передаёт. - geocode()/suggest() принимают опциональный city_hint: str | None; без него внешние тиры больше НЕ подставляют город, а bias (ll/spn) смещается на всю область (OBLAST66_VIEWBOX) вместо ЕКБ-центра. Явный маркер города в адресе или city_hint сохраняют прежнее поведение (ЕКБ-путь не деградирует). - GeocodeResult.city_ambiguous — честный флаг «город определил провайдер, а не пользователь» (не эвристика на корректность), проброшен в AggregatedEstimate.target_city_ambiguous (ephemeral, не персистится). - Cache-ключ geocode_cache учитывает city_hint (address|city=...) — без hint'а формат не меняется (backward-compat), с hint'ом разные города для одного текста адреса больше не делят одну запись. - API: /api/v1/geocode/lookup, /suggest и POST /trade-in/estimate получили опциональный city_hint — контракт не ломается (default None). 23 новых теста в test_geocoder_city_hint.py; проверено что они падают (ImportError на _cache_key) на коде до фикса через git stash. --- tradein-mvp/backend/app/api/v1/geocode.py | 28 +- tradein-mvp/backend/app/schemas/trade_in.py | 12 + tradein-mvp/backend/app/services/estimator.py | 7 +- tradein-mvp/backend/app/services/geocoder.py | 219 ++++++++-- .../backend/tests/test_geocoder_city_hint.py | 413 ++++++++++++++++++ 5 files changed, 637 insertions(+), 42 deletions(-) create mode 100644 tradein-mvp/backend/tests/test_geocoder_city_hint.py diff --git a/tradein-mvp/backend/app/api/v1/geocode.py b/tradein-mvp/backend/app/api/v1/geocode.py index d581f401..7b981887 100644 --- a/tradein-mvp/backend/app/api/v1/geocode.py +++ b/tradein-mvp/backend/app/api/v1/geocode.py @@ -21,14 +21,27 @@ router = APIRouter() async def lookup( address: Annotated[str, Query(min_length=3, max_length=500)], db: Annotated[Session, Depends(get_db)], + city_hint: Annotated[ + str | None, + Query( + max_length=100, + description=( + "Город, если известен вызывающему (например выбран пользователем " + "на предыдущем шаге UI). #2576: без него геокодер БОЛЬШЕ НЕ " + "подставляет 'Екатеринбург' молча — ответ может помечаться " + "city_ambiguous=true." + ), + ), + ] = None, ) -> GeocodeResult: """Геокодинг адреса → lat/lon. Примеры: /api/v1/geocode/lookup?address=ул.+Малышева+30+Екатеринбург /api/v1/geocode/lookup?address=Куйбышева+50+Екатеринбург + /api/v1/geocode/lookup?address=Ленина+1&city_hint=Нижний+Тагил """ - result = await geocode(address, db) + result = await geocode(address, db, city_hint=city_hint) if result is None: raise HTTPException(status_code=404, detail=f"Address not found: {address}") return result @@ -55,6 +68,16 @@ async def suggest_addresses( q: Annotated[str, Query(min_length=2, max_length=200, description="Запрос для автокомплита")], limit: Annotated[int, Query(ge=1, le=15)] = 8, db: Annotated[Session, Depends(get_db)] = None, # type: ignore[assignment] + city_hint: Annotated[ + str | None, + Query( + max_length=100, + description=( + "Город, если известен вызывающему (#2576) — без него подсказки " + "БОЛЬШЕ НЕ ограничиваются молчаливо Екатеринбургом." + ), + ), + ] = None, ) -> SuggestResponse: """Автокомплит адресов в Свердловской области (region 66; ЕКБ — основной трафик, остаётся быстрым fast-path). @@ -66,8 +89,9 @@ async def suggest_addresses( Пример: /api/v1/geocode/suggest?q=Малышева /api/v1/geocode/suggest?q=Цвиллинга # → пусто, такой улицы в ЕКБ нет + /api/v1/geocode/suggest?q=Ленина+1&city_hint=Нижний+Тагил """ - items = await suggest(q, db=db, limit=limit) + items = await suggest(q, db=db, limit=limit, city_hint=city_hint) return SuggestResponse( items=[ SuggestItem( diff --git a/tradein-mvp/backend/app/schemas/trade_in.py b/tradein-mvp/backend/app/schemas/trade_in.py index 69dee79d..c2e42560 100644 --- a/tradein-mvp/backend/app/schemas/trade_in.py +++ b/tradein-mvp/backend/app/schemas/trade_in.py @@ -27,6 +27,12 @@ class TradeInEstimateInput(BaseModel): # geocode() (который падает на DaData-формах при мёртвом Yandex-ключе). lat: float | None = Field(default=None, ge=-90, le=90) lon: float | None = Field(default=None, ge=-180, le=180) + # #2576: город, если известен фронту (например выбран отдельным полем UI). + # Опционально — без него geocode() внутри estimate_quality() БОЛЬШЕ НЕ + # подставляет "Екатеринбург" молча (см. app.services.geocoder), что раньше + # давало уверенно неверную цену для жителей других городов области (те же + # улица+дом существуют и в ЕКБ, и, например, в Нижнем Тагиле). + city_hint: str | None = Field(default=None, max_length=100) # ФИАС/ГАР OBJECTGUID целевого дома, если фронт разрешил его через suggest # (SuggestItem.fias_id у house-level кандидата). Прокидывается в матчер # (Tier 0.5 fias_exact) ПЕРВЫМ, до fias из DaData /clean. Additive/optional — @@ -185,6 +191,12 @@ class AggregatedEstimate(BaseModel): target_address: str | None = None # geocoded full address target_lat: float | None = None target_lon: float | None = None + # #2576: True если ни адрес, ни `TradeInEstimateInput.city_hint` не называли + # город явно — итоговый город (и, соответственно, набор аналогов/цена) + # определил геокодер-провайдер, а не пользователь. Честный сигнал для + # UI (снизить доверие / переспросить город), НЕ персистится в БД + # (ephemeral, только для текущего POST /estimate ответа). + target_city_ambiguous: bool = False sources_used: list[str] = Field(default_factory=list) # ['avito', 'cian', 'rosreestr'] data_freshness_minutes: int | None = None # сколько минут назад был самый свежий парсинг # абсолютный timestamp самого свежего парсинга аналогов diff --git a/tradein-mvp/backend/app/services/estimator.py b/tradein-mvp/backend/app/services/estimator.py index d533bb6e..250a1ad5 100644 --- a/tradein-mvp/backend/app/services/estimator.py +++ b/tradein-mvp/backend/app/services/estimator.py @@ -3180,8 +3180,12 @@ async def estimate_quality( payload.lon, ) if geo is None and payload.address: + # #2576: city_hint прокидывается из payload — БЕЗ него geocode() больше не + # подставляет "Екатеринбург" молча (см. app.services.geocoder). Опционально: + # фронт пока (до отдельного изменения UI) его не шлёт, geo.city_ambiguous + # честно сигнализирует об этом ниже. geo = await _with_budget( - geocode(payload.address, db), + geocode(payload.address, db, city_hint=payload.city_hint), settings.estimate_geocode_budget_s, label="geocode", ) @@ -3880,6 +3884,7 @@ async def estimate_quality( target_address=geo.full_address, target_lat=geo.lat, target_lon=geo.lon, + target_city_ambiguous=geo.city_ambiguous, sources_used=sources_used, data_freshness_minutes=freshness_min, last_scraped_at=last_scraped_at, diff --git a/tradein-mvp/backend/app/services/geocoder.py b/tradein-mvp/backend/app/services/geocoder.py index 1bf0817f..1ec06c79 100644 --- a/tradein-mvp/backend/app/services/geocoder.py +++ b/tradein-mvp/backend/app/services/geocoder.py @@ -16,7 +16,7 @@ from __future__ import annotations import asyncio import logging import re -from dataclasses import dataclass +from dataclasses import dataclass, replace from typing import Literal import httpx @@ -38,6 +38,12 @@ class GeocodeResult: full_address: str provider: Literal["nominatim", "yandex", "cache"] confidence: Literal["exact", "approximate", "locality"] = "approximate" + # #2576: True если город НЕ был указан пользователем (ни в тексте адреса, ни + # через `city_hint`) — т.е. итоговый город результата определил провайдер + # (или локальный ЕКБ-тир), а не вызывающий код. Не эвристика на «правильность» + # результата — честный сигнал «доверяй, но проверяй», чтобы вызывающий код мог + # понизить confidence / переспросить город у пользователя. См. `_resolve_city_for_geocode`. + city_ambiguous: bool = False # ── EKB bounding boxes ─────────────────────────────────────────────────────── @@ -191,6 +197,53 @@ def _has_oblast_marker(text_lower: str) -> bool: return False +def _resolve_city_for_geocode(address: str, city_hint: str | None) -> tuple[str | None, bool]: + """Определяет, какой город подставлять в запрос внешнему провайдеру (Yandex/ + Nominatim), когда сам текст адреса города не называет. + + Приоритет: + 1. Адрес уже содержит маркер города/области региона 66 (`_has_oblast_marker`) + → город уже указан пользователем в тексте адреса, ничего подставлять не + нужно. Возвращает (None, True). + 2. `city_hint` передан вызывающим кодом (например, фронт знает выбранный + город из предыдущего шага UI) → подставляем его. Возвращает (city, True). + 3. Ни то, ни другое → раньше (#2576) здесь молча подставлялся "Екатеринбург" + — для жителей других городов области это давало уверенно неверную цену + («Ленина, 1» в Нижнем Тагиле снапалось на екатеринбургскую улицу Ленина, + обе улицы называются одинаково). Теперь НЕ подставляем никакой город — + провайдер ищет по OBLAST66 viewbox/bbox (см. `_yandex_bias`, + `OBLAST66_VIEWBOX`), без привязки к конкретному городу. Возвращает + (None, False) — второй элемент False сигнализирует, что город + пользователь НЕ указывал (источник `GeocodeResult.city_ambiguous`). + + Returns: + (city_or_none, city_specified_by_user). + """ + if _has_oblast_marker(address.lower()): + return None, True + hint = (city_hint or "").strip() + if hint: + return hint, True + return None, False + + +def _yandex_bias(address: str, city_hint: str | None) -> dict[str, str]: + """ll/spn soft-bias для Yandex Geocoder. + + ЕКБ-центр (`EKB_BBOX`) — ТОЛЬКО если контекст однозначно про Екатеринбург + (явное слово в адресе либо `city_hint`). Иначе — центр всей области + (`OBLAST66_VIEWBOX`): раньше bias всегда указывал на ЕКБ независимо от + того, назвал ли пользователь город (#2576) — молчаливый перекос в пользу + ЕКБ даже без текстового префикса "Екатеринбург, ". + """ + normalized = " ".join(address.lower().split()) + if _EKATERINBURG_RE.search(normalized): + return EKB_BBOX + if city_hint and _EKATERINBURG_RE.search(" ".join(city_hint.lower().split())): + return EKB_BBOX + return OBLAST66_VIEWBOX + + # Города региона 66 КРОМЕ Екатеринбурга — используется чтобы отсечь EKB-only # локальные тиры (geoportal/cadastral, см. `geocode()`) от адреса другого # города области. re.escape на элементах SVERDLOVSK_OBLAST_CITIES-{ekb}. @@ -240,6 +293,29 @@ def normalize_address(address: str) -> str: return " ".join(address.lower().strip().split()) +def _cache_key(address_norm: str, city_hint: str | None) -> str: + """Ключ `geocode_cache.address_normalized` — адрес, дополненный городом, + если он известен вызывающему коду. + + #2576: раньше ключ был просто нормализованный адрес — одинаковый для + «Ленина, 1» независимо от того, кто спрашивает (ЕКБ или Нижний Тагил). + Т.к. геокодер раньше молча предполагал ЕКБ, оба города писали/читали ОДНУ + и ту же строку кэша → взаимная порча (первый запрос «застолбил» город для + второго). С `city_hint` разные города для одного текста адреса больше не + делят один ключ. + + БЕЗ `city_hint` формат ключа не меняется (backward-compatible с уже + накопленным кэшем) — коллизия между городами для запросов без hint'а + остаётся возможной (структурно неизбежно, пока вызывающий код не начнёт + передавать city_hint повсеместно), но `city_ambiguous` на результате + честно сигнализирует об этом вызывающему. + """ + city_norm = " ".join((city_hint or "").lower().strip().split()) + if not city_norm: + return address_norm + return f"{address_norm}|city={city_norm}" + + # Согласные, которые часто пишут с одной буквой вместо двух (RU typos). _DOUBLE_CONSONANTS = "лнмссккттпп" @@ -451,17 +527,24 @@ def _yandex_region_ok(geo_object: dict) -> bool | None: # ── Provider: Yandex Geocoder (требует key, лучшее покрытие РФ) ───────────── @retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=1, max=8)) -async def _yandex_lookup(address: str, api_key: str) -> GeocodeResult | None: +async def _yandex_lookup( + address: str, api_key: str, city_hint: str | None = None +) -> GeocodeResult | None: """Yandex Geocoder — 25K req/day free для самопод, лучше РФ. Docs: https://yandex.ru/dev/maps/geocoder/doc/desc/concepts/input_params.html - Запрашиваем с ll+spn (центр ЕКБ) для приоритизации местных результатов, - но БЕЗ rspn — чтобы fuzzy matching работал при опечатках. + Запрашиваем с ll+spn (центр ЕКБ, если контекст ЕКБ, иначе центр всей + области — см. `_yandex_bias`) для приоритизации местных результатов, но + БЕЗ rspn — чтобы fuzzy matching работал при опечатках. """ - # Не навязываем "Екатеринбург, " если в адресе уже есть город/область региона 66 - # (типичный кейс из suggest, либо явный запрос по другому городу области). - geocode_query = address if _has_oblast_marker(address.lower()) else f"Екатеринбург, {address}" + # Город в запрос подставляем ТОЛЬКО если он известен (адрес уже называет + # город/область региона 66, либо явный `city_hint`) — раньше (#2576) сюда + # молча подставлялся "Екатеринбург" при отсутствии обоих, что давало + # уверенно неверную цену жителям других городов области. + city, _ = _resolve_city_for_geocode(address, city_hint) + geocode_query = f"{city}, {address}" if city else address + bias = _yandex_bias(address, city_hint) async with httpx.AsyncClient(timeout=10.0) as client: response = await client.get( "https://geocode-maps.yandex.ru/1.x/", @@ -471,8 +554,8 @@ async def _yandex_lookup(address: str, api_key: str) -> GeocodeResult | None: "format": "json", "results": 5, # берем top-5, отфильтруем по ЕКБ bbox ниже "lang": "ru_RU", - "ll": EKB_BBOX["ll"], - "spn": EKB_BBOX["spn"], + "ll": bias["ll"], + "spn": bias["spn"], }, ) response.raise_for_status() @@ -641,17 +724,27 @@ async def _dadata_suggest(query: str, limit: int = 8) -> list[GeocodeSuggestion] async def _yandex_geocode_request( - client: httpx.AsyncClient, api_key: str, query: str, limit: int, bounded: bool + client: httpx.AsyncClient, + api_key: str, + query: str, + limit: int, + bounded: bool, + bias: dict[str, str] | None = None, ) -> list[dict]: - """Single Yandex Geocoder request — bounded=True → строго в ЕКБ через rspn=1.""" + """Single Yandex Geocoder request — bounded=True → строго внутри `bias` bbox через rspn=1. + + `bias` — ll/spn (`EKB_BBOX` или `OBLAST66_VIEWBOX`). По умолчанию `EKB_BBOX` + (backward-compat для вызовов без явного bias). + """ + b = bias or EKB_BBOX params: dict[str, str] = { "apikey": api_key, "geocode": query, "format": "json", "results": str(limit), "lang": "ru_RU", - "ll": EKB_BBOX["ll"], - "spn": EKB_BBOX["spn"], + "ll": b["ll"], + "spn": b["spn"], } if bounded: params["rspn"] = "1" @@ -662,40 +755,50 @@ async def _yandex_geocode_request( @retry(stop=stop_after_attempt(2), wait=wait_exponential(multiplier=1, min=1, max=4)) -async def _yandex_suggest(query: str, api_key: str, limit: int = 8) -> list[GeocodeSuggestion]: +async def _yandex_suggest( + query: str, api_key: str, limit: int = 8, city_hint: str | None = None +) -> list[GeocodeSuggestion]: """Yandex Geocoder с авто-fallback на typo-tolerant режим. - Tier 1: bounded ЕКБ (rspn=1) — быстрый путь для основного (ЕКБ) трафика. - Tier 2: bounded ЕКБ на typo-variants (удвоение согласных). + Tier 1: bounded (rspn=1) — быстрый путь. Bounded на ЕКБ, если контекст + однозначно про ЕКБ (текст адреса/`city_hint`), иначе bounded на ВСЮ область + (`OBLAST66_VIEWBOX`) — раньше (#2576) Tier 1/2 всегда форсили bounded-ЕКБ + с "Екатеринбург, "-префиксом даже когда пользователь не называл город, из-за + чего автокомплит для жителей других городов области либо не находил ничего, + либо подсовывал ЕКБ-варианты вместо нужного города. + Tier 2: bounded на typo-variants (удвоение согласных), тот же bias. Tier 3: без rspn — fuzzy по всей стране, фильтр результатов по bbox области (region 66) — ловит легитимные Нижний Тагил/Серов/etc, которые Tier 1/2 - (bounded строго ЕКБ) структурно вернуть не могут. + (bounded) структурно вернуть не могут при неверном bias. """ - prefixed_query = query if _has_oblast_marker(query.lower()) else f"Екатеринбург, {query}" + city, _ = _resolve_city_for_geocode(query, city_hint) + prefixed_query = f"{city}, {query}" if city else query + bias = _yandex_bias(query, city_hint) async with httpx.AsyncClient(timeout=8.0) as client: - # Tier 1: strict bounded на оригинал (ЕКБ fast path) + # Tier 1: strict bounded на оригинал members = await _yandex_geocode_request( client, api_key, prefixed_query, limit, bounded=True, + bias=bias, ) results = _parse_yandex_members(members) if results: return results - # Tier 2: bounded на typo-варианты (тот же ЕКБ fast path) + # Tier 2: bounded на typo-варианты (тот же bias) for variant in _typo_variants(query, limit=4): - variant_query = ( - variant if _has_oblast_marker(variant.lower()) else f"Екатеринбург, {variant}" - ) + variant_city, _ = _resolve_city_for_geocode(variant, city_hint) + variant_query = f"{variant_city}, {variant}" if variant_city else variant members = await _yandex_geocode_request( client, api_key, variant_query, limit, bounded=True, + bias=bias, ) results = _parse_yandex_members(members) if results: @@ -708,6 +811,7 @@ async def _yandex_suggest(query: str, api_key: str, limit: int = 8) -> list[Geoc prefixed_query, limit, bounded=False, + bias=bias, ) results = _parse_yandex_members(members) in_oblast = [r for r in results if is_within_oblast66_bbox(r.lat, r.lon)] @@ -734,18 +838,26 @@ async def _nominatim_query_multi(client: httpx.AsyncClient, query: str, limit: i @retry(stop=stop_after_attempt(2), wait=wait_exponential(multiplier=1, min=1, max=4)) -async def _nominatim_suggest(query: str, limit: int = 8) -> list[GeocodeSuggestion]: +async def _nominatim_suggest( + query: str, limit: int = 8, city_hint: str | None = None +) -> list[GeocodeSuggestion]: """Nominatim в режиме suggest. С typo-fallback (для случаев когда Yandex недоступен). - Суффикс ", Екатеринбург" навязывается ТОЛЬКО если в запросе ещё нет города/области - региона 66 — иначе не режем явные запросы по другим городам области. + Суффикс города навязывается ТОЛЬКО если он известен: адрес уже называет + город/область региона 66, либо передан явный `city_hint`. Раньше (#2576) + при отсутствии обоих сюда молча подставлялся суффикс ", Екатеринбург" — + географию поиска это не расширяло/не сужало (`_nominatim_query_multi` и + так bounded=1 по ВСЕЙ области `OBLAST66_VIEWBOX`), но текстовый суффикс + смещал ранжирование Nominatim в пользу ЕКБ-совпадений даже для адресов + из других городов области. """ headers = { "User-Agent": f"TradeInMVP/0.1 (contact: {settings.contact_email})", "Accept": "application/json", "Accept-Language": "ru,en;q=0.8", } - suffixed_query = query if _has_oblast_marker(query.lower()) else f"{query}, Екатеринбург" + city, _ = _resolve_city_for_geocode(query, city_hint) + suffixed_query = f"{query}, {city}" if city else query async with httpx.AsyncClient(timeout=8.0, headers=headers) as client: # Tier 1: оригинальный query data = await _nominatim_query_multi(client, suffixed_query, limit) @@ -754,9 +866,8 @@ async def _nominatim_suggest(query: str, limit: int = 8) -> list[GeocodeSuggesti if not data: for variant in _typo_variants(query, limit=3): await asyncio.sleep(1.0) # Nominatim 1 req/sec - variant_query = ( - variant if _has_oblast_marker(variant.lower()) else f"{variant}, Екатеринбург" - ) + 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) if data: logger.info("nominatim suggest typo-fixed: %s → %s", query, variant) @@ -1086,13 +1197,20 @@ def _cadastral_reverse_sync(db: Session, lat: float, lon: float, radius_m: int = return str(row.readable_address) -async def suggest(query: str, db: Session | None = None, limit: int = 8) -> list[GeocodeSuggestion]: +async def suggest( + query: str, db: Session | None = None, limit: int = 8, city_hint: str | None = None +) -> list[GeocodeSuggestion]: """Автокомплит адресов в Свердловской области (region 66; ЕКБ — основной трафик, остаётся быстрым fast-path). Cadastral FDW → DaData → Yandex → Nominatim → []. db: если передан — cadastral lookup через gendesign_cad_buildings (первый tier). + city_hint: город, если известен вызывающему коду (#2576) — прокидывается в + Yandex/Nominatim тиры, чтобы НЕ подставлять "Екатеринбург" молча, когда + пользователь его не называл. Опционально, backward-compatible (None — + прежнее поведение минус молчаливый EKB-дефолт, см. `_resolve_city_for_geocode`). DaData /suggest (PR Q2) — token-only, 10k/день, заменяет Yandex который - заблокирован (1k/день demo limit исчерпан). + заблокирован (1k/день demo limit исчерпан). DaData region-constraint уже + охватывает всю область (не только ЕКБ) — city_hint ей не нужен. Без кэша (дешёво, провайдеры толерируют автокомплит-запросы). """ if not query or len(query.strip()) < 2: @@ -1131,7 +1249,9 @@ async def suggest(query: str, db: Session | None = None, limit: int = 8) -> list # Tier 3: Yandex (legacy — оставляем как fallback, если key есть) if settings.yandex_geocoder_api_key: try: - results = await _yandex_suggest(query, settings.yandex_geocoder_api_key, limit) + results = await _yandex_suggest( + query, settings.yandex_geocoder_api_key, limit, city_hint=city_hint + ) if results: return results except Exception: @@ -1139,33 +1259,46 @@ async def suggest(query: str, db: Session | None = None, limit: int = 8) -> list # Tier 4: Nominatim (последний fallback — OSM, без ключа) try: - return await _nominatim_suggest(query, limit) + return await _nominatim_suggest(query, limit, city_hint=city_hint) except Exception: logger.exception("nominatim suggest failed") return [] # ── Public API ─────────────────────────────────────────────────────────────── -async def geocode(address: str, db: Session) -> GeocodeResult | None: +async def geocode(address: str, db: Session, city_hint: str | None = None) -> GeocodeResult | None: """Геокодинг с кэшем. Cadastral FDW → Yandex → Nominatim → None. Args: address: пользовательский ввод (может быть грязным — нормализуем). db: сессия Postgres для cache lookup/write и cadastral FDW lookup. + city_hint: город, если известен вызывающему коду (#2576) — например + выбран пользователем на предыдущем шаге UI. Опциональный, не + ломает существующий контракт. Прокидывается в Yandex/Nominatim + внешние тиры вместо молчаливой подстановки "Екатеринбург" и + участвует в cache-ключе (см. `_cache_key`), чтобы ответы для + разных городов по одному и тому же тексту адреса не перезатирали + друг друга. Returns: GeocodeResult или None если ни один провайдер не отвечает. + `result.city_ambiguous=True`, если ни адрес, ни `city_hint` не + называли город явно — итоговый город определил провайдер/локальный + тир, а не пользователь (честный сигнал, не эвристика на корректность). """ if not address or len(address.strip()) < 3: return None - addr_norm = normalize_address(address) + _, city_specified = _resolve_city_for_geocode(address, city_hint) + city_ambiguous = not city_specified + + addr_norm = _cache_key(normalize_address(address), city_hint) # 1. Cache (sync DB-IO → offload в threadpool, чтобы не блокировать event loop) cached = await asyncio.to_thread(_cache_get, db, addr_norm) if cached is not None: logger.info("geocode cache hit: %s", addr_norm) - return cached + return replace(cached, city_ambiguous=city_ambiguous) # 2. Локальные источники по street+house (без внешнего API). parsed = _parse_street_house(address.strip()) @@ -1191,6 +1324,7 @@ async def geocode(address: str, db: Session) -> GeocodeResult | None: full_address=hit.full_address, provider="cache", confidence="exact", + city_ambiguous=city_ambiguous, ) await asyncio.to_thread(_cache_put, db, addr_norm, result) logger.info( @@ -1215,6 +1349,7 @@ async def geocode(address: str, db: Session) -> GeocodeResult | None: full_address=hit.full_address, provider="nominatim", # treat as "local" — same confidence as nominatim confidence="exact", + city_ambiguous=city_ambiguous, ) await asyncio.to_thread(_cache_put, db, addr_norm, result) logger.info( @@ -1238,6 +1373,7 @@ async def geocode(address: str, db: Session) -> GeocodeResult | None: full_address=s.full_address, provider="nominatim", # treat as "local" — same confidence as nominatim confidence="exact", + city_ambiguous=city_ambiguous, ) await asyncio.to_thread(_cache_put, db, addr_norm, result) logger.info( @@ -1248,22 +1384,26 @@ async def geocode(address: str, db: Session) -> GeocodeResult | None: # 3. Yandex (если есть key) с typo-fallback if settings.yandex_geocoder_api_key: try: - result = await _yandex_lookup(address, settings.yandex_geocoder_api_key) + result = await _yandex_lookup(address, settings.yandex_geocoder_api_key, city_hint) # Если результат вне области (region 66) — пробуем typo-варианты in_oblast = result is not None and is_within_oblast66_bbox(result.lat, result.lon) if result is not None and in_oblast: + result = replace(result, city_ambiguous=city_ambiguous) await asyncio.to_thread(_cache_put, db, addr_norm, result) logger.info("geocode yandex: %s → (%.5f, %.5f)", addr_norm, result.lat, result.lon) return result # Tier 2: typo-variants for variant in _typo_variants(address, limit=4): try: - result = await _yandex_lookup(variant, settings.yandex_geocoder_api_key) + result = await _yandex_lookup( + variant, settings.yandex_geocoder_api_key, city_hint + ) except Exception: continue if result is None: continue if is_within_oblast66_bbox(result.lat, result.lon): + result = replace(result, city_ambiguous=city_ambiguous) await asyncio.to_thread(_cache_put, db, addr_norm, result) logger.info( "geocode yandex typo-fixed: %s → %s → (%.5f, %.5f)", @@ -1280,6 +1420,7 @@ async def geocode(address: str, db: Session) -> GeocodeResult | None: try: result = await _nominatim_lookup(address) if result is not None: + result = replace(result, city_ambiguous=city_ambiguous) await asyncio.to_thread(_cache_put, db, addr_norm, result) logger.info("geocode nominatim: %s → (%.5f, %.5f)", addr_norm, result.lat, result.lon) # Nominatim rate-limit policy: 1 req/sec — спим после успешного запроса diff --git a/tradein-mvp/backend/tests/test_geocoder_city_hint.py b/tradein-mvp/backend/tests/test_geocoder_city_hint.py new file mode 100644 index 00000000..fd2159e8 --- /dev/null +++ b/tradein-mvp/backend/tests/test_geocoder_city_hint.py @@ -0,0 +1,413 @@ +"""Тесты #2576 — geocoder больше НЕ подставляет "Екатеринбург" молча. + +Проблема (issue #2576 / эпик расширения на область): `_yandex_lookup`, +`_yandex_suggest`, `_nominatim_suggest` при отсутствии маркера города/области в +самом адресе всегда молча подставляли "Екатеринбург" — житель Нижнего Тагила, +вводя «Ленина, 1», получал уверенно неверную цену по екатеринбургской улице +Ленина (обе улицы называются одинаково). + +Покрывают: +- `_resolve_city_for_geocode` — приоритет: маркер в адресе > `city_hint` > None. +- `_yandex_lookup` — без города НЕ получает "Екатеринбург, "-префикс и bias + смещён на всю область (не форсит ЕКБ-центр); с `city_hint` — префикс из hint'а; + с явным "Екатеринбург" в адресе — поведение НЕ изменилось (как раньше). +- `_yandex_suggest` (Tier 1 bounded) — то же самое, плюс bias/rspn. +- `_nominatim_suggest` — то же самое (суффикс города, не префикс). +- `geocode()` — `city_ambiguous=True` когда город не указан ни в адресе, ни в + `city_hint`; `False` когда указан явно (текстом или через `city_hint`). +- Cache-ключ (`_cache_key`) — разные `city_hint` для одного текста адреса НЕ + делят одну запись кэша (regression test на cache poisoning). +""" + +from __future__ import annotations + +import contextlib +import os +from unittest.mock import AsyncMock, MagicMock, patch + +os.environ.setdefault("DATABASE_URL", "postgresql://test:test@localhost/test_db") + +import httpx +import pytest + +from app.services.geocoder import ( + EKB_BBOX, + OBLAST66_VIEWBOX, + GeocodeResult, + _cache_key, + _nominatim_suggest, + _resolve_city_for_geocode, + _yandex_lookup, + _yandex_suggest, + geocode, +) + +# ── _resolve_city_for_geocode ──────────────────────────────────────────────── + + +@pytest.mark.parametrize( + "address,city_hint,expected", + [ + # Ни маркер, ни hint — раньше здесь молча подставлялся "Екатеринбург". + ("Ленина, 1", None, (None, False)), + ("Ленина, 1", "", (None, False)), + ("Ленина, 1", " ", (None, False)), + # city_hint передан явно вызывающим кодом. + ("Ленина, 1", "Нижний Тагил", ("Нижний Тагил", True)), + # Маркер уже в адресе — hint игнорируется (marker имеет приоритет). + ("Нижний Тагил, Ленина, 1", "Серов", (None, True)), + ("Екатеринбург, Малышева 30", None, (None, True)), + ("Екатеринбург, Малышева 30", "Серов", (None, True)), + ], +) +def test_resolve_city_for_geocode( + address: str, city_hint: str | None, expected: tuple[str | None, bool] +) -> None: + assert _resolve_city_for_geocode(address, city_hint) == expected + + +# ── _cache_key — cache poisoning между городами ────────────────────────────── + + +def test_cache_key_without_hint_unchanged() -> None: + """Без city_hint формат ключа НЕ меняется — backward-compat с накопленным кэшем.""" + assert _cache_key("ленина, 1", None) == "ленина, 1" + assert _cache_key("ленина, 1", "") == "ленина, 1" + + +def test_cache_key_different_cities_do_not_collide() -> None: + """#2576: разные города для одного текста адреса — разные cache-ключи.""" + key_tagil = _cache_key("ленина, 1", "Нижний Тагил") + key_ekb = _cache_key("ленина, 1", "Екатеринбург") + key_none = _cache_key("ленина, 1", None) + + assert key_tagil != key_ekb + assert key_tagil != key_none + assert key_ekb != key_none + + +def test_cache_key_hint_normalized() -> None: + """city_hint нормализуется (case/whitespace) — не создаёт лишних ключей.""" + assert _cache_key("ленина, 1", "Нижний Тагил") == _cache_key("ленина, 1", "нижний тагил ") + + +# ── _yandex_lookup — query string + bias ───────────────────────────────────── + +_REAL_ASYNC_CLIENT = httpx.AsyncClient + + +def _yandex_client_factory(transport: httpx.MockTransport): + def factory(*_: object, **__: object) -> httpx.AsyncClient: + return _REAL_ASYNC_CLIENT(transport=transport) + + return factory + + +def _empty_yandex_payload() -> dict: + return {"response": {"GeoObjectCollection": {"featureMember": []}}} + + +async def test_yandex_lookup_no_city_no_prefix_and_oblast_bias() -> None: + """#2576: без города в адресе/hint — Yandex-запрос БЕЗ "Екатеринбург, "-префикса, + bias смещён на всю область (не форсит ЕКБ-центр по умолчанию).""" + captured: dict[str, str | None] = {} + + def handler(request: httpx.Request) -> httpx.Response: + captured["geocode"] = request.url.params.get("geocode") + captured["ll"] = request.url.params.get("ll") + captured["spn"] = request.url.params.get("spn") + return httpx.Response(200, json=_empty_yandex_payload()) + + transport = httpx.MockTransport(handler) + with patch("app.services.geocoder.httpx.AsyncClient", _yandex_client_factory(transport)): + result = await _yandex_lookup("Ленина, 1", "fake-key") + + assert result is None # пустой featureMember + assert captured["geocode"] == "Ленина, 1" + assert "Екатеринбург" not in (captured["geocode"] or "") + assert captured["ll"] == OBLAST66_VIEWBOX["ll"] + assert captured["spn"] == OBLAST66_VIEWBOX["spn"] + + +async def test_yandex_lookup_city_hint_prefix() -> None: + """city_hint="Нижний Тагил" → запрос получает префикс из hint'а, не "Екатеринбург".""" + captured: dict[str, str | None] = {} + + def handler(request: httpx.Request) -> httpx.Response: + captured["geocode"] = request.url.params.get("geocode") + captured["ll"] = request.url.params.get("ll") + return httpx.Response(200, json=_empty_yandex_payload()) + + transport = httpx.MockTransport(handler) + with patch("app.services.geocoder.httpx.AsyncClient", _yandex_client_factory(transport)): + await _yandex_lookup("Ленина, 1", "fake-key", city_hint="Нижний Тагил") + + assert captured["geocode"] == "Нижний Тагил, Ленина, 1" + # Тагил — не ЕКБ-контекст → bias не форсит ЕКБ-центр. + assert captured["ll"] == OBLAST66_VIEWBOX["ll"] + + +async def test_yandex_lookup_explicit_ekaterinburg_unchanged() -> None: + """Явное "Екатеринбург" в адресе → поведение НЕ изменилось (как до фикса).""" + captured: dict[str, str | None] = {} + + def handler(request: httpx.Request) -> httpx.Response: + captured["geocode"] = request.url.params.get("geocode") + captured["ll"] = request.url.params.get("ll") + return httpx.Response(200, json=_empty_yandex_payload()) + + transport = httpx.MockTransport(handler) + with patch("app.services.geocoder.httpx.AsyncClient", _yandex_client_factory(transport)): + await _yandex_lookup("Екатеринбург, Малышева 30", "fake-key") + + assert captured["geocode"] == "Екатеринбург, Малышева 30" + assert captured["ll"] == EKB_BBOX["ll"] + + +# ── _yandex_suggest (Tier 1 bounded) ───────────────────────────────────────── + + +async def test_yandex_suggest_no_city_uses_oblast_bounded() -> None: + """#2576: автокомплит без города — bounded по ВСЕЙ области, без city-префикса + (раньше Tier 1 всегда форсил bounded-ЕКБ с "Екатеринбург, ").""" + calls: list[tuple[str, bool, dict[str, str] | None]] = [] + + async def fake_request(client, api_key, query, limit, bounded, bias=None): + calls.append((query, bounded, bias)) + return [] + + with patch( + "app.services.geocoder._yandex_geocode_request", new=AsyncMock(side_effect=fake_request) + ): + result = await _yandex_suggest("Ленина, 1", "fake-key") + + assert result == [] + assert calls, "expected at least one Yandex request" + first_query, first_bounded, first_bias = calls[0] + assert first_query == "Ленина, 1" + assert "Екатеринбург" not in first_query + assert first_bounded is True + assert first_bias == OBLAST66_VIEWBOX + + +async def test_yandex_suggest_city_hint_prefix_bounded() -> None: + calls: list[tuple[str, bool, dict[str, str] | None]] = [] + + async def fake_request(client, api_key, query, limit, bounded, bias=None): + calls.append((query, bounded, bias)) + return [] + + with patch( + "app.services.geocoder._yandex_geocode_request", new=AsyncMock(side_effect=fake_request) + ): + await _yandex_suggest("Ленина, 1", "fake-key", city_hint="Нижний Тагил") + + first_query, _, first_bias = calls[0] + assert first_query == "Нижний Тагил, Ленина, 1" + assert first_bias == OBLAST66_VIEWBOX + + +async def test_yandex_suggest_explicit_ekb_unchanged() -> None: + calls: list[tuple[str, bool, dict[str, str] | None]] = [] + + async def fake_request(client, api_key, query, limit, bounded, bias=None): + calls.append((query, bounded, bias)) + return [] + + with patch( + "app.services.geocoder._yandex_geocode_request", new=AsyncMock(side_effect=fake_request) + ): + await _yandex_suggest("Екатеринбург, Малышева 30", "fake-key") + + first_query, _, first_bias = calls[0] + assert first_query == "Екатеринбург, Малышева 30" + assert first_bias == EKB_BBOX + + +# ── _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), не последний. + """ + calls: list[str] = [] + + async def fake_query_multi(client, query, limit): + calls.append(query) + return [] + + with patch( + "app.services.geocoder._nominatim_query_multi", new=AsyncMock(side_effect=fake_query_multi) + ): + result = await _nominatim_suggest("Ленина, 1") + + assert result == [] + assert calls[0] == "Ленина, 1" + assert "Екатеринбург" not in calls[0] + + +async def test_nominatim_suggest_city_hint_suffix() -> None: + calls: list[str] = [] + + async def fake_query_multi(client, query, limit): + calls.append(query) + return [] + + with patch( + "app.services.geocoder._nominatim_query_multi", new=AsyncMock(side_effect=fake_query_multi) + ): + await _nominatim_suggest("Ленина, 1", city_hint="Нижний Тагил") + + assert calls[0] == "Ленина, 1, Нижний Тагил" + + +async def test_nominatim_suggest_explicit_ekb_unchanged() -> None: + calls: list[str] = [] + + async def fake_query_multi(client, query, limit): + calls.append(query) + return [] + + with patch( + "app.services.geocoder._nominatim_query_multi", new=AsyncMock(side_effect=fake_query_multi) + ): + await _nominatim_suggest("Екатеринбург, Малышева 30") + + assert calls[0] == "Екатеринбург, Малышева 30" + + +# ── geocode() — city_ambiguous flag ────────────────────────────────────────── + + +def _geocode_patches(yandex_result: GeocodeResult | None): + return ( + patch("app.services.geocoder._cache_get", return_value=None), + patch("app.services.geocoder._cache_put"), + patch("app.services.geocoder._geoportal_house_match", return_value=None), + patch("app.services.geocoder._cadastral_house_match", return_value=None), + patch("app.services.geocoder._cadastral_forward_sync", return_value=[]), + patch("app.services.geocoder._yandex_lookup", new=AsyncMock(return_value=yandex_result)), + ) + + +async def test_geocode_city_ambiguous_true_when_no_city_known() -> None: + """Ни адрес, ни city_hint не называют город → city_ambiguous=True.""" + db = MagicMock() + yandex_result = GeocodeResult(lat=56.838, lon=60.605, full_address="что-то", provider="yandex") + with patch("app.services.geocoder.settings") as mock_settings: + mock_settings.yandex_geocoder_api_key = "fake" + with contextlib.ExitStack() as stack: + for cm in _geocode_patches(yandex_result): + stack.enter_context(cm) + result = await geocode("Малышева, 30", db) + + assert result is not None + assert result.city_ambiguous is True + + +async def test_geocode_city_ambiguous_false_when_marker_present() -> None: + """Явный "Екатеринбург" в адресе → город указан пользователем → city_ambiguous=False.""" + db = MagicMock() + yandex_result = GeocodeResult( + lat=56.838, lon=60.605, full_address="Екатеринбург, Малышева, 30", provider="yandex" + ) + with patch("app.services.geocoder.settings") as mock_settings: + mock_settings.yandex_geocoder_api_key = "fake" + with contextlib.ExitStack() as stack: + for cm in _geocode_patches(yandex_result): + stack.enter_context(cm) + result = await geocode("Екатеринбург, Малышева, 30", db) + + assert result is not None + assert result.city_ambiguous is False + + +async def test_geocode_city_ambiguous_false_when_city_hint_given() -> None: + """city_hint передан вызывающим кодом → город указан → city_ambiguous=False.""" + db = MagicMock() + yandex_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: + for cm in _geocode_patches(yandex_result): + stack.enter_context(cm) + result = await geocode("Ленина, 1", db, city_hint="Нижний Тагил") + + assert result is not None + assert result.city_ambiguous is False + + +# ── geocode() — cache не смешивает города ──────────────────────────────────── + + +async def test_geocode_cache_does_not_mix_cities() -> None: + """#2576 regression: два города для одного текста адреса не делят cache-запись. + + Без city_hint-aware ключа второй вызов (Тагил) читал бы уже закэшированный + (первым вызовом, ЕКБ) результат — координаты ЕКБ вместо Тагила. + """ + store: dict[str, GeocodeResult] = {} + + def fake_cache_get(db, addr_norm): + return store.get(addr_norm) + + def fake_cache_put(db, addr_norm, result): + store[addr_norm] = result + + async def fake_yandex_lookup(address, api_key, city_hint=None): + if city_hint == "Нижний Тагил": + return GeocodeResult( + lat=57.905, lon=59.950, full_address="Нижний Тагил, Ленина, 1", provider="yandex" + ) + return GeocodeResult( + lat=56.838, lon=60.605, full_address="Екатеринбург, Ленина, 1", provider="yandex" + ) + + db = MagicMock() + 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", side_effect=fake_cache_get) + ) + stack.enter_context( + patch("app.services.geocoder._cache_put", side_effect=fake_cache_put) + ) + stack.enter_context( + patch("app.services.geocoder._geoportal_house_match", return_value=None) + ) + stack.enter_context( + patch("app.services.geocoder._cadastral_house_match", return_value=None) + ) + stack.enter_context( + patch("app.services.geocoder._cadastral_forward_sync", return_value=[]) + ) + stack.enter_context( + patch( + "app.services.geocoder._yandex_lookup", + new=AsyncMock(side_effect=fake_yandex_lookup), + ) + ) + + r_ekb = await geocode("Ленина, 1", db, city_hint="Екатеринбург") + r_tagil = await geocode("Ленина, 1", db, city_hint="Нижний Тагил") + # Повторный запрос ЕКБ — должен снова попасть в СВОЙ кэш (не Тагила). + r_ekb_again = await geocode("Ленина, 1", db, city_hint="Екатеринбург") + + assert r_ekb is not None and r_tagil is not None and r_ekb_again is not None + assert r_ekb.lat == pytest.approx(56.838) + assert r_tagil.lat == pytest.approx(57.905) + assert r_ekb_again.lat == pytest.approx(56.838) + assert r_ekb.lat != r_tagil.lat + # Два разных ключа реально осели в fake-store (не перезаписали друг друга). + assert len(store) == 2 -- 2.45.3 From b3c76c8c6263784c65b3b9b12eb3eac0150c655d Mon Sep 17 00:00:00 2001 From: bot-backend Date: Fri, 31 Jul 2026 17:55:18 +0300 Subject: [PATCH 2/2] =?UTF-8?q?fix(tradein/geocoder):=20city=5Fhint=20?= =?UTF-8?q?=D0=B4=D0=BE=D0=BB=D0=B6=D0=B5=D0=BD=20=D0=B4=D0=BE=D1=85=D0=BE?= =?UTF-8?q?=D0=B4=D0=B8=D1=82=D1=8C=20=D0=B4=D0=BE=20=D0=BB=D0=BE=D0=BA?= =?UTF-8?q?=D0=B0=D0=BB=D1=8C=D0=BD=D1=8B=D1=85=20=D1=82=D0=B8=D1=80=D0=BE?= =?UTF-8?q?=D0=B2=20+=20=D0=BD=D0=B5=20=D1=82=D0=B5=D1=80=D1=8F=D1=82?= =?UTF-8?q?=D1=8C=20=D0=95=D0=9A=D0=91-=D0=BF=D1=80=D0=B8=D0=B2=D1=8F?= =?UTF-8?q?=D0=B7=D0=BA=D1=83=20=D0=B2=20Tier=204=20(#2576)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deep-review PR #2580 нашёл два блокера в предыдущем фиксе (#2576): C1 — city_hint не участвовал в гейте локальных ЕКБ-only тиров (geoportal/cadastral, `use_local_ekb` в geocode() и Tier 1 в suggest()). Явный city_hint="Нижний Тагил" для "Ленина 1" всё равно попадал на ЕКБ-only базы, получал "точный" ЕКБ-хит и при этом city_ambiguous=False (хинт ведь был) — то есть система теперь ложно-уверенно утверждала неверный город. Фикс: city_hint участвует в той же проверке _names_non_ekb_city (гейт/ gazetteer #2582 не трогаю — только добавляю вход). C2 — снятие суффикса ", Екатеринбург" в _nominatim_suggest для случая "город неизвестен" регрессило часть реальных ЕКБ-адресов: без текстовой подсказки о городе Nominatim иногда предпочитает street-level матч в соседнем городе-спутнике (эмпирика ревьюера: "Победы 20" без суффикса → Верхняя Пышма вместо ЖК "Парк Победы" в Екатеринбурге). Решение — dual-query: bare (честный, без города) И ЕКБ-suffixed запросы объединяются (не заменяют друг друга), оба честных кандидата остаются в подсказках, пользователь выбирает сам. Extra round-trip только для последнего fallback-тира (cadastral/DaData/Yandex уже не сработали) — не задевает основной трафик. Заодно (🟠, дешёвая правка): city_hint прокинут в _nominatim_lookup — с тех пор как Yandex-ключ недействителен (#2585), это единственный живой внешний провайдер, и его tie-break (предпочитает tight-ЕКБ bbox) без города не различает одноимённые улицы внутри региона. 4 новых теста (C1×2, C2×2 + dedupe) — проверено что все 4 падают на коде до этого коммита через git stash (только geocoder.py, тесты оставлены). --- tradein-mvp/backend/app/services/geocoder.py | 122 +++++++++-- .../backend/tests/test_geocoder_city_hint.py | 199 ++++++++++++++++-- 2 files changed, 288 insertions(+), 33 deletions(-) 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() -- 2.45.3