fix(tradein/geocode): гейт мусорного города вынести в общий хелпер и прошить в admin-путь (#2603)
All checks were successful
CI / changes (pull_request) Successful in 7s
CI Trade-In / changes (pull_request) Successful in 7s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 2m42s
All checks were successful
CI / changes (pull_request) Successful in 7s
CI Trade-In / changes (pull_request) Successful in 7s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 2m42s
Первый коммит починил только 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. Копий функции нет — четвёртый потребитель, если появится, получит гейт сам. Тесты: мусорный город -> хинт не передаётся, валидный -> передаётся; проверено фальсификацией (без фикса все три новых теста краснеют).
This commit is contained in:
parent
ec17886d60
commit
5ecd5361fd
7 changed files with 170 additions and 58 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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` /
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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).
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)."""
|
||||
|
|
|
|||
|
|
@ -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 ────────────────────────────────────────────────
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue