fix(tradein/geocoder): stop apt number leaking into house + houses bbox/sibling guard
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 8s
CI Trade-In / browser-tests (pull_request) Has been skipped
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 5m0s

Review round 2 on #2626 (local houses fallback) found two HIGH-severity bugs
verified live against prod data:

1. _extract_local_house_token took the LAST digit-like token in the raw
   address, so "...Педагогическая, д 15, кв 11" resolved house=11 (apartment
   number) instead of 15 -- confidently returning a stranger's building with
   confidence='exact', written to geocode_cache. Fixed by stripping the
   apartment/office/floor/entrance tail (кв/оф/пом/подъезд/этаж -- NOT
   корп/к, which is part of the house number) before extracting the token.
   Fixes the exact prod case from the review plus the corpus+apartment
   combo ("д 26 к 1, кв 41" -> 26к1, not 41).

2. houses is not an EKB-only table (21% of rows with coords are outside the
   metro, some as far as another city) -- "улица Маяковского, 7" in houses
   resolves to Серов, not Екатеринбург, and use_local_ekb only gates the
   user's query text, not the source row. Added an is_within_ekb_bbox_wide
   check on every candidate row before it can become a match.

Also addressed two MEDIUM findings from the same review:

3. The "<номер> -> <номер>к1" corpus guess only checked uniqueness among
   к1-labelled rows, so real multi-building addresses (Онуфриева 24: к1/к2/к3,
   250-400m apart) resolved confidently to к1 anyway. Guess is now skipped
   when any other corpus/slash variant of the same base number exists among
   the street's candidates.

4. Houses-fallback results are no longer cached in geocode_cache -- the
   source (scraped listings) is less reliable than geoportal/cadastral/
   Nominatim, and the lookup is cheap/local, so caching only extended the
   lifetime of a possible bad match. Side benefit: address_refined now
   survives every repeat request of the same raw address, not just the
   first.

Also added ORDER BY address, id to the underlying query so the coordinate
dedup picks a deterministic row (LOW finding #5).

14 new/updated tests in test_geocoder_local_houses_fallback.py cover all
five findings against real prod address/houses-row fixtures. Full geocoder
+ dadata + estimator/pdf regression suite (402 tests) green.
This commit is contained in:
bot-backend 2026-08-15 18:50:17 +03:00
parent c75206c348
commit fa84705ec7
2 changed files with 256 additions and 30 deletions

View file

@ -51,11 +51,14 @@ class GeocodeResult:
# корпус («49» вместо реального «49к1») — houses-фолбэк нашёл ОДНОЗНАЧНЫЙ дом по
# нормализованному совпадению. Честный сигнал вызывающему коду «адрес уточнён
# автоматически», НЕ эвристика на корректность — см. `geocode()`/`_local_houses_match`.
# Известный предел: `geocode_cache` НЕ хранит этот флаг (схему не трогаем) —
# на повторный запрос ТОГО ЖЕ сырого адреса из кэша координаты корректные, но
# `address_refined` вернётся `False` (та же судьба у `city_ambiguous` при
# cache-hit — см. `_geocode_resolve`, восстанавливается `replace()` из
# текущего вызова, а не из кэша).
# Houses-фолбэк НЕ пишет свой результат в `geocode_cache` (менее надёжный
# источник координат, чем geoportal/cadastral/Nominatim — #2626 review R2 #4),
# поэтому этот сигнал переживает КАЖДЫЙ повторный запрос того же сырого
# адреса. `geocode_cache` вообще не хранит этот флаг (схему не трогаем) —
# если бы houses-хит когда-нибудь попал в кэш, на cache-hit `address_refined`
# вернулся бы `False` (та же судьба у `city_ambiguous` при cache-hit — см.
# `_geocode_resolve`, восстанавливается `replace()` из текущего вызова, а не
# из кэша).
address_refined: bool = False
@ -1361,13 +1364,33 @@ def _norm_local_house(raw: str) -> str:
return s
# Хвостовой мусор ПОСЛЕ номера дома — квартира/офис/помещение/подъезд/этаж.
# НЕ включает «корп/корпус/к» (в отличие от `_RE_APT_TAIL` выше) — корпус тут
# ЧАСТЬ номера дома, который должен остаться видимым для `_LOCAL_HOUSE_TOKEN_RE`
# («49к1», «26 к 1» — корпус нельзя терять). Без этой зачистки
# `_extract_local_house_token` (берёт ПОСЛЕДНЕЕ число в строке) находит номер
# квартиры/этажа вместо дома — прод-баг #2626 review R2 #1: «...Педагогическая,
# д 15, кв 11» отдавал дом «11» (координаты ЧУЖОГО здания) вместо «15».
_RE_LOCAL_APT_TAIL = re.compile(
r"[,\s]\s*(?:кв|квартира|оф|офис|пом|помещение|лит|подъезд|этаж)\.?\s*\d.*$",
re.IGNORECASE,
)
def _extract_local_house_token(address: str) -> str | None:
"""Номер дома из ПОЛЬЗОВАТЕЛЬСКОГО адреса — с учётом «/N» и «корпус N» хвостов,
которые `_parse_street_house`/`_HOUSE_NUM` обрезают (см. коммент у
`_LOCAL_HOUSE_TOKEN_RE`). Берём ПОСЛЕДНЕЕ совпадение номер дома в русском
адресе почти всегда в хвосте строки. None, если цифр нет вовсе.
Квартирный/этажный/подъездный хвост зачищается ДО поиска номера
(`_RE_LOCAL_APT_TAIL`) иначе «последнее число в строке» это номер
квартиры/этажа, а не дома (см. докстринг у `_RE_LOCAL_APT_TAIL`).
"""
s = _RE_POSTAL.sub(" ", " ".join(address.lower().strip().split())).strip(" ,.")
if not s:
return None
s = _RE_LOCAL_APT_TAIL.sub(" ", s).strip(" ,.")
if not s:
return None
matches = list(_LOCAL_HOUSE_TOKEN_RE.finditer(s))
@ -1431,25 +1454,53 @@ def _street_tail_matches(row_street_norm: str, query_street_norm: str) -> bool:
return row_street_norm == query_street_norm or row_street_norm.endswith(" " + query_street_norm)
# «24к1» → «24» (базовый номер варианта с корпусом/слэшем); «44» (голый номер,
# без суффикса) → None. Используется ТОЛЬКО для sibling-guard (см. ниже) —
# отличить «этот дом однозначно к1» от «этого дома несколько корпусов, а у
# нас в вводе просто нет данных, какой именно».
_LOCAL_HOUSE_VARIANT_BASE_RE = re.compile(r"^(\d+)(?:к\d+|/\d+)$")
def _local_houses_match(db: Session, street: str, house: str) -> GeocodeSuggestion | None:
"""Последний локальный тир `geocode()` (#2626) — fallback на `houses`
(скрейпленные листинги avito/cian/derived/yandex, own DB table, БЕЗ FDW).
Вызывается ТОЛЬКО когда geoportal/cadastral/Nominatim уже не дали результата.
Два независимых допущения, оба defensive (при неоднозначности None, не гадаем):
Допущения, все defensive (при неоднозначности None, не гадаем):
1. Улица матчится «по хвосту» (`_street_tail_matches`) ловит расхождение
разговорного/сокращённого имени («Онуфриева») и канонического ГАР-имени в
houses («Начдива Онуфриева»).
2. Номер дома сперва точное совпадение; нет пробуем `<номер>к1` (частый
случай: пользователь ввёл «49», у дома есть только корпус «49к1»). ЛЮБОЙ
шаг, где кандидатов больше одного (после дедупа по координатам разные
source-строки ОДНОГО дома не в счёт), возвращает None угадывать нельзя.
2. Координаты строки-кандидата обязаны лежать в широком ЕКБ-bbox
(`is_within_ekb_bbox_wide`) `houses` НЕ ЕКБ-only реестр (в отличие от
geoportal/cad_buildings): 21% строк с координатами лежат вне области ЕКБ,
местами вплоть до другого региона (#2626 review R2 #2 — прод-пример
«улица Маяковского, 7» в houses это Серов, а не запрошенный
Екатеринбург). `use_local_ekb` в `geocode()` гейтит только ЗАПРОС
пользователя, не страхует от грязной строки-источника.
3. Номер дома сперва точное совпадение; нет пробуем `<номер>к1` (частый
случай: пользователь ввёл «49», у дома есть только корпус «49к1»), но
ТОЛЬКО если среди кандидатов улицы НЕТ других корпусов/дробей этого же
номера («24к2», «24/2» и т.п.) иначе «к1» такая же угадайка, как и
любой другой корпус, и реальные дома могут быть в 250-400м друг от друга
(#2626 review R2 #3, прод-пример «Начдива Онуфриева, 24»: 24к1/24к2/24к3
три разных здания).
4. ЛЮБОЙ шаг, где кандидатов больше одного (после дедупа по округлённым
координатам разные source-строки ОДНОГО дома не в счёт), возвращает
None угадывать нельзя.
SQL дешёвый ILIKE-префильтр по последнему слову улицы (нет индекса на
`houses.address`, но тир последний и редкий не на каждый запрос), вся
точная логика (суффикс улицы + равенство номера) в Python, что и делает
её юнит-тестируемой без реальной БД (см. `test_geocoder_local_houses_fallback.py`).
`houses.address`, но тир последний и редкий не на каждый запрос) с
детерминированным ORDER BY (дедуп по координатам иначе непредсказуемо
выбирал бы, какая из двух ~идентичных source-строк станет ответом
#2626 review R2 #5); вся точная логика (суффикс улицы, bbox, равенство
номера) в Python, что и делает её юнит-тестируемой без реальной БД
(см. `test_geocoder_local_houses_fallback.py`).
Результат этого тира НЕ кэшируется в `geocode_cache` вызывающей стороной
(см. `geocode()`) `houses`-координаты из скрейпленных объявлений менее
надёжны, чем geoportal/cadastral/Nominatim, а сам lookup дешёвый и локальный
(#2626 review R2 #4).
"""
query_street_norm = _clean_local_house_street(street)
if not query_street_norm:
@ -1466,6 +1517,7 @@ def _local_houses_match(db: Session, street: str, house: str) -> GeocodeSuggesti
FROM houses
WHERE address ILIKE CAST('%' || :w || '%' AS text)
AND lat IS NOT NULL AND lon IS NOT NULL
ORDER BY address, id
"""),
{"w": last_word},
).fetchall()
@ -1478,23 +1530,32 @@ def _local_houses_match(db: Session, street: str, house: str) -> GeocodeSuggesti
)
return None
# Street-tail + bbox фильтр — один проход, дальше переиспользуется и для
# точного совпадения, и для corpus-1 догадки, и для sibling-guard.
street_rows: list[tuple[str, float, float, str]] = [] # (house_norm, lat, lon, addr)
for r in rows:
parsed = _row_local_house(str(r.address or ""))
if parsed is None:
continue
row_street_norm, row_house_norm = parsed
if not _street_tail_matches(row_street_norm, query_street_norm):
continue
lat, lon = float(r.lat), float(r.lon)
if not is_within_ekb_bbox_wide(lat, lon):
continue
street_rows.append((row_house_norm, lat, lon, str(r.address)))
def _candidates(house_norm: str) -> list[tuple[str, float, float]]:
out: list[tuple[str, float, float]] = []
seen_coords: set[tuple[float, float]] = set()
for r in rows:
parsed = _row_local_house(str(r.address or ""))
if parsed is None:
continue
row_street_norm, row_house_norm = parsed
for row_house_norm, lat, lon, addr in street_rows:
if row_house_norm != house_norm:
continue
if not _street_tail_matches(row_street_norm, query_street_norm):
continue
coord_key = (round(float(r.lat), 4), round(float(r.lon), 4)) # ~11m — дедуп источников
coord_key = (round(lat, 4), round(lon, 4)) # ~11m — дедуп источников
if coord_key in seen_coords:
continue
seen_coords.add(coord_key)
out.append((str(r.address), float(r.lat), float(r.lon)))
out.append((addr, lat, lon))
return out
exact = _candidates(query_house_norm)
@ -1514,6 +1575,21 @@ def _local_houses_match(db: Session, street: str, house: str) -> GeocodeSuggesti
# если запрошенный номер — голое число (не пытаемся достраивать «49/2» → «49/2к1»).
if query_house_norm.isdigit():
corpus1 = f"{query_house_norm}к1"
siblings = {
row_house_norm
for row_house_norm, _lat, _lon, _addr in street_rows
if row_house_norm != corpus1
and (m := _LOCAL_HOUSE_VARIANT_BASE_RE.match(row_house_norm)) is not None
and m.group(1) == query_house_norm
}
if siblings:
logger.info(
"local houses fallback: корпус-1 %r неоднозначен — есть другие "
"корпуса/дроби %s — skip",
corpus1,
sorted(siblings),
)
return None
guessed = _candidates(corpus1)
if len(guessed) == 1:
addr, lat, lon = guessed[0]
@ -1810,7 +1886,10 @@ async def _geocode_resolve(
# резолвился Nominatim'ом (разговорное/усечённое имя улицы или отсутствующий
# в вводе корпус). См. `_local_houses_match`. EKB-only гейт — тот же, что у
# geoportal/cadastral (houses — преимущественно ЕКБ-трафик, тот же риск
# коллизии улица+дом с другим городом региона, что и мотивировал #2582).
# коллизии улица+дом с другим городом региона, что и мотивировал #2582);
# координаты строки-кандидата ДОПОЛНИТЕЛЬНО проверяются bbox-ом внутри
# `_local_houses_match` (гейт здесь фильтрует только запрос пользователя,
# не грязь в самой таблице — #2626 review R2 #2).
if use_local_ekb and parsed is not None:
local_street, _parsed_house = parsed
local_house = _extract_local_house_token(address) or _parsed_house
@ -1825,7 +1904,12 @@ async def _geocode_resolve(
city_ambiguous=city_ambiguous,
address_refined=True,
)
await asyncio.to_thread(_cache_put, db, addr_norm, result)
# НЕ кэшируем: houses-координаты (скрейпленные листинги) менее
# надёжны, чем geoportal/cadastral/Nominatim, а сам lookup дешёвый
# и локальный — кэш только продлевал бы жизнь возможной ошибке
# источника (#2626 review R2 #4). Побочный эффект: `address_refined`
# переживает КАЖДЫЙ повторный запрос этого сырого адреса, а не
# только первый (было известным пределом до этого фикса).
logger.info(
"geocode local houses fallback: %s → (%.5f, %.5f) [%s]",
addr_norm,

View file

@ -83,6 +83,32 @@ def test_norm_local_house(raw: str, expected: str) -> None:
("Крестинского 49к1", "49к1"),
("8 Марта 204", "204"), # digit-leading street name doesn't confuse it
("Малышева 30", "30"),
# #2626 review R2 #1 — прод-баг: квартира подменяла дом («д 15, кв 11»
# → дом «11», чужое здание). Реальные строки из trade_in_estimates:
(
"620078, Свердловская обл, г Екатеринбург, Кировский р-н, "
"ул Педагогическая, д 15, кв 11",
"15",
),
(
"620078, Свердловская обл, г Екатеринбург, Кировский р-н, "
"ул Педагогическая, д 15, кв 48",
"15",
),
# корпус ПЕРЕД квартирой — «26 к 1» обязан остаться частью номера дома,
# «кв 41» — уйти:
(
"620149, Свердловская обл, г Екатеринбург, Ленинский р-н, "
"ул Начдива Онуфриева, д 26 к 1, кв 41",
"26к1",
),
# подъезд/этаж — тот же класс бага, что и квартира (последнее число в
# строке — не дом):
(
"Россия, Свердловская область, Екатеринбург, Трамвайный переулок, "
"2к2, подъезд 1, этаж 25, кв. 205",
"2к2",
),
],
)
def test_extract_local_house_token(address: str, expected: str) -> None:
@ -186,7 +212,30 @@ def test_local_houses_match_exact_house_number() -> None:
def test_local_houses_match_street_tail_and_corpus1_guess() -> None:
"""«Онуфриева, 24» (без «Начдива», без корпуса) → единственный «24к1» реестра."""
"""«Онуфриева, 24» (без «Начдива», без корпуса), реестр — ЕДИНСТВЕННЫЙ
корпус «24к1» уверенная догадка (нет sibling-корпусов не угадайка)."""
db = _db_with_rows(
[
_make_row(
"р-н Ленинский, мкр. Юго-Западный, улица Начдива Онуфриева, 24к1",
56.802928,
60.551696,
),
]
)
hit = _local_houses_match(db, "онуфриева", "24")
assert hit is not None
assert hit.lat == pytest.approx(56.802928)
assert hit.lon == pytest.approx(60.551696)
def test_local_houses_match_corpus1_guess_skipped_when_sibling_corpus_exists() -> None:
"""#2626 review R2 #3, прод-данные: «Начдива Онуфриева, 24» реально ТРИ
разных здания (24к1/24к2/24к3, 250-400м друг от друга). Догадка «24к1»
не угадывает конкретное здание среди known-siblings честный None, не
«уверенный» результат с confidence='exact' на случайно выбранном доме."""
db = _db_with_rows(
[
_make_row(
@ -199,11 +248,19 @@ def test_local_houses_match_street_tail_and_corpus1_guess() -> None:
]
)
hit = _local_houses_match(db, "онуфриева", "24")
assert _local_houses_match(db, "онуфриева", "24") is None
assert hit is not None
assert hit.lat == pytest.approx(56.802928)
assert hit.lon == pytest.approx(60.551696)
def test_local_houses_match_corpus1_guess_skipped_when_slash_sibling_exists() -> None:
"""Sibling-guard ловит не только «кN», но и «/N» вариант того же номера."""
db = _db_with_rows(
[
_make_row("улица X, 24к1", 56.80, 60.60),
_make_row("улица X, 24/2", 56.81, 60.61),
]
)
assert _local_houses_match(db, "x", "24") is None
def test_local_houses_match_no_corpus1_candidate_returns_none() -> None:
@ -278,6 +335,50 @@ def test_local_houses_match_returns_none_on_db_error() -> None:
assert _local_houses_match(db, "онуфриева", "24") is None
# ── bbox guard: `houses` is NOT EKB-only (#2626 review R2 #2) ───────────────
def test_local_houses_match_rejects_row_outside_ekb_bbox() -> None:
"""Прод-кейс: «улица Маяковского, 7» в `houses` — это Серов (56.6/60.66 —
~310км от ЕКБ), не Екатеринбург. `use_local_ekb` в `geocode()` гейтит только
ЗАПРОС пользователя, не координаты строки-источника bbox-фильтр внутри
`_local_houses_match` обязан отбросить такую строку, а не вернуть её как
confidence='exact' совпадение чужого города."""
db = _db_with_rows(
[_make_row("улица Маяковского, 7", 59.652903, 60.659674)], # Серов, не ЕКБ
)
assert _local_houses_match(db, "маяковского", "7") is None
def test_local_houses_match_accepts_row_inside_ekb_bbox_wide() -> None:
"""Контроль: легитимная ЕКБ-строка (в т.ч. приграничье, в WIDE, не в TIGHT)
по-прежнему проходит bbox-фильтр не режет реальные ЕКБ-дома."""
db = _db_with_rows(
[_make_row("Екатеринбург, улица Маяковского, 8", 56.862701, 60.620274)],
)
hit = _local_houses_match(db, "маяковского", "8")
assert hit is not None
assert hit.lat == pytest.approx(56.862701)
# ── deterministic ORDER BY (#2626 review R2 #5) ──────────────────────────────
def test_local_houses_match_query_has_deterministic_order_by() -> None:
"""Без ORDER BY дедуп по округлённым координатам оставлял бы ПЕРВУЮ строку
в порядке сканирования недетерминированно между вызовами. SQL обязан
сортировать явно."""
db = _db_with_rows([])
_local_houses_match(db, "x", "1")
sql_text = str(db.execute.call_args[0][0])
assert "ORDER BY" in sql_text.upper()
# ── geocode() wiring — last-resort tier, sets address_refined ───────────────
@ -298,7 +399,7 @@ async def test_geocode_falls_back_to_local_houses_after_nominatim_miss() -> None
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._cache_put"),
patch("app.services.geocoder._cache_put") as mock_cache_put,
patch(
"app.services.geocoder._nominatim_lookup",
new_callable=AsyncMock,
@ -316,6 +417,9 @@ async def test_geocode_falls_back_to_local_houses_after_nominatim_miss() -> None
assert result.confidence == "exact"
assert result.address_refined is True
mock_local.assert_called_once()
# #2626 review R2 #4 — houses-фолбэк дешёвый и менее надёжный источник
# координат, чем geoportal/cadastral/Nominatim — свой результат не кэширует.
mock_cache_put.assert_not_called()
async def test_geocode_address_refined_false_when_earlier_tier_hits() -> None:
@ -366,3 +470,41 @@ async def test_geocode_returns_none_when_local_houses_also_misses() -> None:
assert result is None
mock_local.assert_called_once()
async def test_geocode_local_houses_apartment_number_does_not_leak_into_house() -> None:
"""End-to-end regression, #2626 review R2 #1: реальный прод-адрес с хвостом
«кв 11» должен резолвиться в дом 15 (`Педагогическая ул.,15`), а НЕ в дом 11
(`Педагогическая ул.,11` чужое здание) `_local_houses_match` не
замокан, проверяем полную цепочку `geocode()` `_extract_local_house_token`
SQL-lookup."""
db = _db_with_rows(
[
_make_row("Педагогическая ул.,11", 56.835387, 60.654104),
_make_row("Педагогическая ул.,15", 56.835284, 60.655829),
]
)
with (
patch("app.services.geocoder._cache_get", return_value=None),
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._cache_put") as mock_cache_put,
patch(
"app.services.geocoder._nominatim_lookup",
new_callable=AsyncMock,
return_value=None,
),
):
result = await geocode(
"620078, Свердловская обл, г Екатеринбург, Кировский р-н, "
"ул Педагогическая, д 15, кв 11",
db,
)
assert result is not None
assert result.lat == pytest.approx(56.835284)
assert result.lon == pytest.approx(60.655829)
assert result.address_refined is True
mock_cache_put.assert_not_called()