From 5ecd5361fdee0a0ff6fb202f20ef39db002afc26 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Wed, 5 Aug 2026 20:20:41 +0500 Subject: [PATCH] =?UTF-8?q?fix(tradein/geocode):=20=D0=B3=D0=B5=D0=B9?= =?UTF-8?q?=D1=82=20=D0=BC=D1=83=D1=81=D0=BE=D1=80=D0=BD=D0=BE=D0=B3=D0=BE?= =?UTF-8?q?=20=D0=B3=D0=BE=D1=80=D0=BE=D0=B4=D0=B0=20=D0=B2=D1=8B=D0=BD?= =?UTF-8?q?=D0=B5=D1=81=D1=82=D0=B8=20=D0=B2=20=D0=BE=D0=B1=D1=89=D0=B8?= =?UTF-8?q?=D0=B9=20=D1=85=D0=B5=D0=BB=D0=BF=D0=B5=D1=80=20=D0=B8=20=D0=BF?= =?UTF-8?q?=D1=80=D0=BE=D1=88=D0=B8=D1=82=D1=8C=20=D0=B2=20admin-=D0=BF?= =?UTF-8?q?=D1=83=D1=82=D1=8C=20(#2603)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Первый коммит починил только scripts/geocode_deals_nominatim.py — ручной скрипт. Тот же дефект оставался на живом пути: POST /admin/geocode-missing?target=deals отдавал сырой row["city"] в city_hint, а deals.city росреестровое и в хвосте распределения содержит не-города («Бессонова», «Билейский рыбопитомник»). Любой не-ЕКБ хинт жёстко закрывает EKB-локальные тиры и уезжает префиксом в запрос провайдеру, то есть мусорный хинт хуже отсутствия хинта. Гейт вынесен в geocoder.known_city_hint (сверка с SVERDLOVSK_OBLAST_CITIES — тем же набором, который уже питает _names_non_ekb_city / _ekb_local_tiers_allowed) и переиспользуется всеми тремя потребителями city_hint: скриптом, admin-ручкой и задачей geocode_missing. Копий функции нет — четвёртый потребитель, если появится, получит гейт сам. Тесты: мусорный город -> хинт не передаётся, валидный -> передаётся; проверено фальсификацией (без фикса все три новых теста краснеют). --- tradein-mvp/backend/app/api/v1/admin.py | 11 ++- tradein-mvp/backend/app/services/geocoder.py | 35 +++++++++ .../backend/app/tasks/geocode_missing.py | 14 +++- .../scripts/geocode_deals_nominatim.py | 40 +++-------- .../scripts/test_geocode_deals_nominatim.py | 28 ++------ .../tests/tasks/test_geocode_missing.py | 72 +++++++++++++++++++ .../backend/tests/test_geocoder_city_hint.py | 28 ++++++++ 7 files changed, 170 insertions(+), 58 deletions(-) diff --git a/tradein-mvp/backend/app/api/v1/admin.py b/tradein-mvp/backend/app/api/v1/admin.py index 1e5add94..c564ed2b 100644 --- a/tradein-mvp/backend/app/api/v1/admin.py +++ b/tradein-mvp/backend/app/api/v1/admin.py @@ -72,7 +72,7 @@ from app.services import cian_session as cian_session_svc from app.services import domclick_session as domclick_session_svc from app.services import proxy_rotation as proxy_rotation_svc from app.services import scrape_runs as runs_mod -from app.services.geocoder import geocode +from app.services.geocoder import geocode, known_city_hint from app.services.scheduler import has_running_run from app.services.scraper_adapters import ( RealEnrichmentJobs, @@ -318,7 +318,14 @@ async def geocode_missing( # развёртки/импорта. Прокидываем как city_hint, а не полагаемся на то, что # геокодер угадает город по тексту address (голый "ул. Победы, 30" без # города в тексте иначе уходит в Екатеринбург). - city = row.get("city") + # + # known_city_hint (#2603) — гейт по словарю городов области: при + # target="deals" сюда приходит росреестровое поле, в хвосте которого + # лежат не-города («Бессонова», «Билейский рыбопитомник»), а мусорный + # хинт закрывает EKB-локальные тиры и уезжает префиксом в запрос + # провайдеру, т.е. вреднее отсутствия хинта. Общий хелпер, тот же, что у + # scripts/geocode_deals_nominatim.py и tasks/geocode_missing.py. + city = known_city_hint(row.get("city")) result = await geocode(clean, db, city_hint=city) if result is None: # Помечаем что пробовали — иначе ретрай на каждом cron. diff --git a/tradein-mvp/backend/app/services/geocoder.py b/tradein-mvp/backend/app/services/geocoder.py index d3c9eb55..f3cdb12e 100644 --- a/tradein-mvp/backend/app/services/geocoder.py +++ b/tradein-mvp/backend/app/services/geocoder.py @@ -156,6 +156,41 @@ SVERDLOVSK_OBLAST_CITIES = frozenset( # без district-префикса ложно ушёл бы в non-EKB gate. } ) + + +def known_city_hint(value: str | None) -> str | None: + """`value` как city_hint, если это узнаваемое имя города региона 66, иначе None. + + Для callers, которые берут город из КОЛОНКИ БД и передают его в `geocode()` + (#2603): `deals.city` — росреестровое поле, заполнено на 100%, но в хвосте + распределения лежит мусор («Бессонова», «Бердюгина», «Билейский + рыбопитомник» — улицы/урочища, попавшие в поле города). Мусорный хинт хуже + отсутствия хинта: любой не-ЕКБ `city_hint` жёстко закрывает EKB-локальные + тиры (`_ekb_local_tiers_allowed`) И подставляется префиксом в запрос + провайдеру (`_resolve_city_for_geocode`) — «Бессонова, Бессонова 10» + провайдер не резолвит вовсе. + + Словарь — `SVERDLOVSK_OBLAST_CITIES`, тот же, на котором стоят городские + гейты самого геокодера. Отдельного списка сознательно НЕ заводим: город, + отсутствующий в этом наборе, и так обрабатывается геокодером как незнакомый + (`_names_non_ekb_city` его не увидит, `estimator._resolve_target_city` не + резолвит) — т.е. новый город области в любом случае добавляется СЮДА, и + гейт хинта не создаёт новой связности. + + Цена решения (осознанная): легитимный, но не перечисленный населённый пункт + («Реж», «Арамиль», сёла/посёлки) хинта не получит и вернётся к поведению «по + тексту адреса» — то же, что было до прошивки хинта, без регрессии. + + Пользовательский ввод (`/geocode/lookup`, `/geocode/suggest`, + `TradeInEstimateInput.city_hint`) сюда НЕ заворачиваем: там город назвал + человек, и молча его игнорировать нельзя — для произвольной строки + fail-closed отрабатывает `_ekb_local_tiers_allowed` (#2580/#2589). + """ + if not value: + return None + return value if " ".join(value.lower().split()) in SVERDLOVSK_OBLAST_CITIES else None + + # Значение для DaData-констрейнта `locations: [{"region": ...}]`. # ВАЖНО: DaData хранит имя региона БЕЗ типа — `region="Свердловская"`, # `region_type="обл"` (тип лежит в отдельных полях `region_type` / diff --git a/tradein-mvp/backend/app/tasks/geocode_missing.py b/tradein-mvp/backend/app/tasks/geocode_missing.py index 35823593..e93d0289 100644 --- a/tradein-mvp/backend/app/tasks/geocode_missing.py +++ b/tradein-mvp/backend/app/tasks/geocode_missing.py @@ -41,7 +41,7 @@ from sqlalchemy.orm import Session from app.services import scrape_runs as runs_mod from app.services.estimator import _geocode_is_coarse -from app.services.geocoder import geocode +from app.services.geocoder import geocode, known_city_hint logger = logging.getLogger(__name__) @@ -77,7 +77,8 @@ async def geocode_missing_listings( не попадут в выдачу пользователю) 2. Для каждой пары (address, city): - - geocode(address, db, city_hint=city) — auto-cache (hit или miss) + - geocode(address, db, city_hint=known_city_hint(city)) — auto-cache + (hit или miss); хинт гейтится словарём городов области (#2603) - Если есть результат: UPDATE listings SET lat, lon WHERE address = :addr AND city IS NOT DISTINCT FROM :city AND lat IS NULL (IS NOT DISTINCT FROM, а не `=` — стандартная SQL NULL-семантика: `city = NULL` @@ -160,7 +161,14 @@ async def geocode_missing_listings( result.addresses_processed += 1 try: - geo = await geocode(address, db, city_hint=city) + # known_city_hint (#2603) — общий гейт по словарю городов области для + # всех DB-колоночных callers. Для listings.city он сегодня no-op + # (скрапер пишет только шесть кураторских имён из + # scraper_kit CITY_DISPLAY_NAMES, все они есть в словаре), но держит + # инвариант единым с deals-путями, где колонка росреестровая и в + # хвосте лежит мусор. Сырой `city` ниже остаётся ключом группы для + # UPDATE — гейт влияет только на подсказку геокодеру. + geo = await geocode(address, db, city_hint=known_city_hint(city)) except Exception as exc: logger.warning("geocode_missing: geocode raised for '%s': %s", address[:60], exc) result.addresses_failed += 1 diff --git a/tradein-mvp/backend/scripts/geocode_deals_nominatim.py b/tradein-mvp/backend/scripts/geocode_deals_nominatim.py index 99e981cd..c6876d83 100644 --- a/tradein-mvp/backend/scripts/geocode_deals_nominatim.py +++ b/tradein-mvp/backend/scripts/geocode_deals_nominatim.py @@ -40,8 +40,9 @@ A non-EKB hint is a HARD signal inside the geocoder: it closes the EKB-only local tiers (`_ekb_local_tiers_allowed`) and gets prefixed into the provider query, so a junk hint makes the result strictly WORSE than no hint at all. We therefore pass the hint only for values that are recognised cities of oblast -66 (`_city_hint` below); everything else degrades to the previous behaviour -(no hint, address text only). +66 — `geocoder.known_city_hint`, shared with the other DB-column callers +(`app/api/v1/admin.py`, `app/tasks/geocode_missing.py`); everything else +degrades to the previous behaviour (no hint, address text only). Street-level precision is accepted: the estimator's comparable search uses a 1000-2000 m radius, so a street-level point lands every deal on that street in @@ -105,15 +106,15 @@ except ImportError: # pragma: no cover — fallback for adhoc invocation try: from app.services.geocoder import ( # type: ignore[import-not-found] - SVERDLOVSK_OBLAST_CITIES, GeocodeResult, geocode, + known_city_hint, ) except ImportError: # pragma: no cover import sys sys.path.insert(0, str(Path(__file__).resolve().parents[1])) - from app.services.geocoder import SVERDLOVSK_OBLAST_CITIES, GeocodeResult, geocode + from app.services.geocoder import GeocodeResult, geocode, known_city_hint logging.basicConfig( level=logging.INFO, @@ -144,8 +145,8 @@ class AddressGroup: """One distinct (deals.address, deals.city) pair + its lat-IS-NULL row count. `city` is the raw Rosreestr value (may be NULL for non-rosreestr sources, and - may be junk — see `_city_hint`), kept verbatim so the UPDATE can target - exactly the rows this group came from. + may be junk — see `geocoder.known_city_hint`), kept verbatim so the UPDATE + can target exactly the rows this group came from. """ address: str @@ -153,26 +154,6 @@ class AddressGroup: city: str | None = None -def _city_hint(city: str | None) -> str | None: - """`city` if it is a recognised oblast-66 city name, else None (#2603). - - Dictionary = `geocoder.SVERDLOVSK_OBLAST_CITIES` — the single source of truth - already used by the geocoder's own city gates (`_names_non_ekb_city`, - `_ekb_local_tiers_allowed`) and by `estimator._resolve_target_city`. No local - list: a second one would drift from the gates the hint actually feeds. - - Filtering is not optional politeness: `deals.city` is Rosreestr-sourced and - its tail holds non-cities ('Бессонова', 'Бердюгина', 'Билейский - рыбопитомник'). Any non-EKB hint closes the EKB-only local tiers and is - prefixed into the provider query, so an unrecognised value would make the - geocode worse than passing nothing. - """ - if not city: - return None - normalized = " ".join(city.lower().split()) - return city if normalized in SVERDLOVSK_OBLAST_CITIES else None - - @dataclass class Stats: """Final-summary counters. @@ -383,9 +364,10 @@ async def _run_backfill( address = group.address city = group.city # Only a recognised oblast-66 city is fed to the geocoder; junk Rosreestr - # values degrade to None (see `_city_hint`). The raw `city` is still used - # for the UPDATE scope — it identifies the group either way. - hint = _city_hint(city) + # values degrade to None (geocoder.known_city_hint — shared with the + # other DB-column callers). The raw `city` is still used for the UPDATE + # scope — it identifies the group either way. + hint = known_city_hint(city) # The geocoder itself rejects <3 chars, but skip here too so the dry-run # report and counters stay honest (no phantom "processed" address). diff --git a/tradein-mvp/backend/tests/scripts/test_geocode_deals_nominatim.py b/tradein-mvp/backend/tests/scripts/test_geocode_deals_nominatim.py index dc47e02a..3e656a98 100644 --- a/tradein-mvp/backend/tests/scripts/test_geocode_deals_nominatim.py +++ b/tradein-mvp/backend/tests/scripts/test_geocode_deals_nominatim.py @@ -12,9 +12,10 @@ Coverage (per the issue's test plan): batch. - dedup: distinct addresses drive geocode call count (one call per address). - main() wiring: SessionLocal, --limit bind, returns geocoded count. - - #2603: grouping by the (address, city) PAIR, city_hint filtered against - geocoder.SVERDLOVSK_OBLAST_CITIES (junk Rosreestr city → no hint), and - writes scoped with `city IS NOT DISTINCT FROM`. + - #2603: grouping by the (address, city) PAIR, city_hint filtered through + the shared `geocoder.known_city_hint` (junk Rosreestr city → no hint; the + helper's own cases live in tests/test_geocoder_city_hint.py), and writes + scoped with `city IS NOT DISTINCT FROM`. No real Postgres. `geocode()` is async → patched with AsyncMock (mirrors tests/tasks/test_geocode_missing.py). The Session is a MagicMock that routes @@ -34,7 +35,6 @@ from app.services.geocoder import GeocodeResult from scripts.geocode_deals_nominatim import ( AddressGroup, Stats, - _city_hint, _mark_deals_tried, _run_backfill, _select_pending_addresses, @@ -483,26 +483,6 @@ def test_select_pending_addresses_returns_city_in_group(): assert "SELECT address, city, deals_count" in sql_str -def test_city_hint_keeps_known_oblast_city(): - """A recognised oblast-66 city passes through as the hint (case/spacing-insensitive).""" - assert _city_hint("Нижний Тагил") == "Нижний Тагил" - assert _city_hint("екатеринбург") == "екатеринбург" - assert _city_hint(" Каменск-Уральский ") == " Каменск-Уральский " # raw value kept - - -def test_city_hint_drops_junk_rosreestr_value(): - """Rosreestr tail values are NOT cities — a junk hint is worse than none. - - Any non-EKB hint closes the geocoder's EKB-only local tiers and is prefixed - into the provider query, so 'Бессонова' would actively degrade the result. - """ - assert _city_hint("Бессонова") is None - assert _city_hint("Бердюгина") is None - assert _city_hint("Билейский рыбопитомник") is None - assert _city_hint(None) is None - assert _city_hint("") is None - - async def test_run_backfill_passes_known_city_as_hint(): groups = [AddressGroup(address="Победы, 30", deals_count=2, city="Нижний Тагил")] db, coord_updates, _ = _make_db_mock(update_rowcount=2) diff --git a/tradein-mvp/backend/tests/tasks/test_geocode_missing.py b/tradein-mvp/backend/tests/tasks/test_geocode_missing.py index d1e24921..0e786ffb 100644 --- a/tradein-mvp/backend/tests/tasks/test_geocode_missing.py +++ b/tradein-mvp/backend/tests/tasks/test_geocode_missing.py @@ -811,6 +811,78 @@ async def test_admin_geocode_missing_passes_city_hint(target: str) -> None: assert f"FROM {target}" in str(db.execute.call_args_list[0][0][0]) +@pytest.mark.asyncio +@pytest.mark.parametrize("target", ["listings", "deals"]) +async def test_admin_geocode_missing_drops_junk_city_hint(target: str) -> None: + """Мусорный город из колонки НЕ уходит в city_hint (#2603). + + `deals.city` росреестровое: в хвосте «Бессонова», «Билейский рыбопитомник» — + улицы/урочища, а не города. Хинт из такого значения закрывает EKB-локальные + тиры и уезжает префиксом в запрос провайдеру, т.е. хуже отсутствия хинта. + Гейт общий с двумя другими DB-колоночными callers (geocoder.known_city_hint). + """ + from app.api.v1 import admin as admin_module + + rows = [{"id": 56, "address": "ул. Бессонова, 11", "city": "Бессонова"}] + + db = MagicMock() + select_result = MagicMock() + select_result.mappings.return_value.all.return_value = rows + update_result = MagicMock() + remaining_result = MagicMock() + remaining_result.scalar.return_value = 0 + db.execute.side_effect = [select_result, update_result, remaining_result] + + geo = GeocodeResult( + lat=56.838, + lon=60.605, + full_address="Екатеринбург, ул. Бессонова, 11", + provider="nominatim", # type: ignore[arg-type] + confidence="exact", + ) + + with patch( + "app.api.v1.admin.geocode", + new_callable=AsyncMock, + return_value=geo, + ) as mock_geo: + await admin_module.geocode_missing( + db, + limit=100, + target=target, # type: ignore[arg-type] + ) + + mock_geo.assert_called_once_with("ул. Бессонова, 11", db, city_hint=None) + + +@pytest.mark.asyncio +async def test_geocode_missing_listings_drops_junk_city_hint() -> None: + """Тот же гейт в ночной задаче: мусорный city → city_hint=None (#2603). + + Для listings.city это сегодня no-op (скрапер пишет шесть кураторских имён), + но инвариант «в geocode() уходит только словарный город» держим единым для + всех трёх DB-колоночных callers, чтобы седьмой не пришлось чинить заново. + Сырой city при этом остаётся ключом группы для UPDATE. + """ + rows = [{"address": "ул. Бессонова, 11", "city": "Бессонова", "listings_count": 2}] + db = _mock_db_rows(rows) + + with patch( + "app.tasks.geocode_missing.geocode", + new_callable=AsyncMock, + return_value=_make_geocode_result(), + ) as mock_geo: + await geocode_missing_listings(db, batch_size=10) + + mock_geo.assert_called_once_with("ул. Бессонова, 11", db, city_hint=None) + update_binds = [ + c[0][1] + for c in db.execute.call_args_list + if "UPDATE listings" in str(c[0][0]) and "SET lat" in str(c[0][0]) + ] + assert update_binds[0]["city"] == "Бессонова" # группа UPDATE — по сырому городу + + @pytest.mark.asyncio async def test_admin_geocode_missing_select_includes_city_column() -> None: """SELECT в admin.geocode_missing содержит колонку city (#2594).""" diff --git a/tradein-mvp/backend/tests/test_geocoder_city_hint.py b/tradein-mvp/backend/tests/test_geocoder_city_hint.py index 0565aa0d..7daa2023 100644 --- a/tradein-mvp/backend/tests/test_geocoder_city_hint.py +++ b/tradein-mvp/backend/tests/test_geocoder_city_hint.py @@ -38,9 +38,37 @@ from app.services.geocoder import ( _nominatim_suggest, _resolve_city_for_geocode, geocode, + known_city_hint, suggest, ) +# ── known_city_hint (#2603) ────────────────────────────────────────────────── + + +@pytest.mark.parametrize( + "value,expected", + [ + # Известные города области — значение проходит КАК ЕСТЬ (регистр/пробелы + # нормализует сам геокодер, caller'ы не должны его причёсывать). + ("Екатеринбург", "Екатеринбург"), + ("Нижний Тагил", "Нижний Тагил"), + ("нижний тагил", "нижний тагил"), + ("Каменск-Уральский", "Каменск-Уральский"), + # Мусор из хвоста росреестрового deals.city — это улицы/урочища, а не + # города; хинт из них закрыл бы EKB-тиры и уехал префиксом в запрос. + ("Бессонова", None), + ("Бердюгина", None), + ("Билейский рыбопитомник", None), + # Пусто — хинта нет. + (None, None), + ("", None), + (" ", None), + ], +) +def test_known_city_hint(value: str | None, expected: str | None) -> None: + assert known_city_hint(value) == expected + + # ── _resolve_city_for_geocode ────────────────────────────────────────────────