feat(tradein/matching): region_code у houses — вывод, а не выдумка (#3051 п.4) #3121
6 changed files with 228 additions and 4 deletions
|
|
@ -44,6 +44,7 @@ import logging
|
|||
from sqlalchemy import text
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.services import regions as regions_mod
|
||||
from app.services.matching.normalize import (
|
||||
EKB_CITY_TOKEN,
|
||||
address_fingerprint,
|
||||
|
|
@ -77,6 +78,7 @@ def match_or_create_house(
|
|||
building_cadastral_number: str | None = None,
|
||||
source_url: str | None = None,
|
||||
city: str | None = None,
|
||||
region_code: int | None = None,
|
||||
) -> tuple[int | None, float, str]:
|
||||
"""Match existing house or create new canonical record.
|
||||
|
||||
|
|
@ -407,19 +409,46 @@ def match_or_create_house(
|
|||
# New house — INSERT canonical record.
|
||||
# geom column is auto-populated by houses_set_geom_trg BEFORE INSERT trigger from lat/lon.
|
||||
# Do NOT include geom in the INSERT column list — trigger handles it.
|
||||
#
|
||||
# #3051 п.4: region_code наследуется от развёртки, но НЕ выдумывается.
|
||||
# Три исхода: (а) координаты подтверждают регион развёртки или отсутствуют
|
||||
# (не противоречат) → наследуем; (б) координаты выводят ДРУГОЙ регион →
|
||||
# NULL-карантин + warning (это порченый геокод, а не переезд дома: прод-факт
|
||||
# 27.08 — «Вильгельма де Геннина» на Байкале, «Крауля» под Москвой);
|
||||
# (в) региона развёртки нет вовсе (легаси-вызовы) → NULL.
|
||||
region_val: int | None = None
|
||||
if region_code is not None:
|
||||
coord_region = (
|
||||
regions_mod.region_for_point(lat, lon) if lat is not None and lon is not None else None
|
||||
)
|
||||
if coord_region is None or coord_region.code == region_code:
|
||||
region_val = region_code
|
||||
else:
|
||||
logger.warning(
|
||||
"house create: координаты (%.4f, %.4f) выводят регион %d, а развёртка "
|
||||
"идёт в регионе %d — region_code=NULL (карантин, порченый геокод?) "
|
||||
"addr=%r src=%s",
|
||||
lat,
|
||||
lon,
|
||||
coord_region.code,
|
||||
region_code,
|
||||
address,
|
||||
ext_source,
|
||||
)
|
||||
url = source_url or f"matching://{ext_source}/{ext_id}"
|
||||
row = (
|
||||
db.execute(
|
||||
text("""
|
||||
INSERT INTO houses (source, ext_house_id, url, address, lat, lon, year_built,
|
||||
cadastral_number)
|
||||
cadastral_number, region_code)
|
||||
VALUES (
|
||||
:src, :eid, :url,
|
||||
:addr,
|
||||
CAST(:lat AS double precision),
|
||||
CAST(:lon AS double precision),
|
||||
CAST(:yb AS integer),
|
||||
:cad
|
||||
:cad,
|
||||
CAST(:region_code AS smallint)
|
||||
)
|
||||
ON CONFLICT (source, ext_house_id) DO UPDATE SET
|
||||
address = COALESCE(EXCLUDED.address, houses.address)
|
||||
|
|
@ -434,6 +463,7 @@ def match_or_create_house(
|
|||
"lon": lon,
|
||||
"yb": year_built,
|
||||
"cad": cad,
|
||||
"region_code": region_val,
|
||||
},
|
||||
)
|
||||
.mappings()
|
||||
|
|
|
|||
|
|
@ -68,6 +68,7 @@ class RealMatcherAdapter:
|
|||
building_cadastral_number: str | None = None,
|
||||
source_url: str | None = None,
|
||||
city: str | None = None,
|
||||
region_code: int | None = None,
|
||||
) -> tuple[int | None, float, str]:
|
||||
# house_id is None when the matcher refuses a numberless address without a
|
||||
# cadastral number (method 'no_house_number', P1). Callers must tolerate None.
|
||||
|
|
@ -82,6 +83,7 @@ class RealMatcherAdapter:
|
|||
building_cadastral_number=building_cadastral_number,
|
||||
source_url=source_url,
|
||||
city=city,
|
||||
region_code=region_code,
|
||||
)
|
||||
|
||||
def upsert_listing_source(
|
||||
|
|
|
|||
37
tradein-mvp/backend/data/sql/272_houses_region_code.sql
Normal file
37
tradein-mvp/backend/data/sql/272_houses_region_code.sql
Normal file
|
|
@ -0,0 +1,37 @@
|
|||
-- 272_houses_region_code.sql
|
||||
-- #3051 п.4: region_code у houses — вывод, а не выдумка.
|
||||
--
|
||||
-- Правило вывода на бэкфилле: регион присваивается ТОЛЬКО дому, чья геометрия
|
||||
-- лежит в генеральном bbox региона 66 (вся сегодняшняя база — екатеринбургская,
|
||||
-- региона-77 сбора не существует). Всё прочее — NULL, то есть КАРАНТИН:
|
||||
-- * 620 домов без geom — регион невыводим до гео-обогащения;
|
||||
-- * ~20 «чужаков» — ЕКБ-адреса с испорченным геокодом («Вильгельма де
|
||||
-- Геннина» на Байкале, «Учителей» в Таллине; замер 27.08) — присвоить им
|
||||
-- регион по координатам значило бы узаконить порчу;
|
||||
-- * 3 дома с координатами в bbox Москвы — ТОЖЕ порченые ЕКБ («Крауля, гп 1»,
|
||||
-- «Академика Ландау») — потому 77 на бэкфилле не присваивается ВООБЩЕ.
|
||||
--
|
||||
-- NOT NULL из постановки задачи НЕ ставится этой миграцией: карантин должен
|
||||
-- сначала опустеть (гео-обогащение 620 + починка 23 порченых) — иначе
|
||||
-- ограничение заставило бы выдумывать значения. Ставится отдельной миграцией
|
||||
-- по факту пустого карантина.
|
||||
--
|
||||
-- bbox 66 — байт-в-байт bbox_region из app/services/regions.py (реестр #3051);
|
||||
-- синхронизацию значений держит tests/test_3051_houses_region_code.py.
|
||||
-- Идемпотентно: повторный прогон обновит 0 строк.
|
||||
BEGIN;
|
||||
SET LOCAL lock_timeout = '5s';
|
||||
ALTER TABLE houses ADD COLUMN IF NOT EXISTS region_code smallint;
|
||||
|
||||
COMMENT ON COLUMN houses.region_code IS
|
||||
'#3051: регион дома (66=Свердловская, 77=Москва). NULL = карантин: регион '
|
||||
'невыводим (нет geom / координаты противоречат происхождению). Источник '
|
||||
'правил — app/services/regions.py.';
|
||||
|
||||
UPDATE houses
|
||||
SET region_code = 66
|
||||
WHERE region_code IS NULL
|
||||
AND geom IS NOT NULL
|
||||
AND ST_Y(geom) BETWEEN 55.8 AND 62.2
|
||||
AND ST_X(geom) BETWEEN 56.7 AND 66.6;
|
||||
COMMIT;
|
||||
137
tradein-mvp/backend/tests/test_3051_houses_region_code.py
Normal file
137
tradein-mvp/backend/tests/test_3051_houses_region_code.py
Normal file
|
|
@ -0,0 +1,137 @@
|
|||
"""#3051 п.4: region_code у houses — вывод, а не выдумка.
|
||||
|
||||
Прод-факты (27.08): 10 362 дома, 9 719 выводятся геометрией в регион 66;
|
||||
23 порченых (ЕКБ-адреса с чужими координатами: «Вильгельма де Геннина» на
|
||||
Байкале, «Крауля» под Москвой, «Учителей» в Таллине) с живыми ссылками
|
||||
листингов; 620 без geom. Порченым и безгеомным регион НЕ присваивается —
|
||||
NULL-карантин (присвоить по координатам = узаконить порчу).
|
||||
|
||||
Три слоя:
|
||||
миграция 272 — колонка + бэкфилл ТОЛЬКО по bbox региона 66 (значения
|
||||
байт-в-байт из реестра — синхронизацию держит тест ниже);
|
||||
запись — новый дом наследует регион РАЗВЁРТКИ, только если координаты не
|
||||
противоречат; противоречие → NULL + warning;
|
||||
проводка — base.py передаёт region_code развёртки через контракт и адаптер.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
|
||||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from app.services import regions
|
||||
from app.services.matching import houses as houses_mod
|
||||
|
||||
_MIGRATION = Path(__file__).resolve().parents[1] / "data" / "sql" / "272_houses_region_code.sql"
|
||||
|
||||
# ── 1. Миграция синхронизирована с реестром ──────────────────────────────────
|
||||
|
||||
|
||||
def test_migration_bbox_matches_registry() -> None:
|
||||
"""bbox в SQL бэкфилла — байт-в-байт bbox_region региона 66 из реестра.
|
||||
Разъезд значений = бэкфилл и рантайм выводят регион по разным границам."""
|
||||
sql = _MIGRATION.read_text()
|
||||
m = re.search(
|
||||
r"ST_Y\(geom\) BETWEEN ([\d.]+) AND ([\d.]+)\s*"
|
||||
r"AND ST_X\(geom\) BETWEEN ([\d.]+) AND ([\d.]+)",
|
||||
sql,
|
||||
)
|
||||
assert m, "в миграции 272 не найден bbox-предикат бэкфилла"
|
||||
lat_min, lat_max, lon_min, lon_max = (float(g) for g in m.groups())
|
||||
assert (lat_min, lat_max, lon_min, lon_max) == regions.REGIONS[66].bbox_region
|
||||
|
||||
# 77 на бэкфилле не присваивается вообще (3 «московских» дома — порченые ЕКБ).
|
||||
assert "= 77" not in sql
|
||||
# NOT NULL не ставится, пока карантин не пуст: ни в определении колонки,
|
||||
# ни отдельным ALTER (слова в комментариях — не DDL).
|
||||
assert "SET NOT NULL" not in sql
|
||||
assert re.search(r"region_code smallint\s*;", sql), "колонка обязана быть nullable"
|
||||
|
||||
|
||||
# ── 2. Правило записи: унаследуй или карантинь ───────────────────────────────
|
||||
|
||||
|
||||
class _CreateDb:
|
||||
"""Двойник: все матч-тиры промахиваются, INSERT возвращает id и пишет параметры."""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self.insert_params: dict[str, Any] | None = None
|
||||
|
||||
def execute(self, stmt: Any, params: dict[str, Any] | None = None) -> Any:
|
||||
text = str(stmt)
|
||||
res = MagicMock()
|
||||
if "INSERT INTO houses" in text:
|
||||
self.insert_params = dict(params or {})
|
||||
res.mappings.return_value.one.return_value = {"id": 4242}
|
||||
return res
|
||||
# advisory lock / все тиры матчинга — промах
|
||||
res.fetchone.return_value = None
|
||||
res.mappings.return_value.first.return_value = None
|
||||
res.mappings.return_value.all.return_value = []
|
||||
res.scalar.return_value = None
|
||||
return res
|
||||
|
||||
def commit(self) -> None:
|
||||
pass
|
||||
|
||||
def rollback(self) -> None:
|
||||
pass
|
||||
|
||||
|
||||
def _create(lat: float | None, lon: float | None, region_code: int | None) -> dict[str, Any]:
|
||||
db = _CreateDb()
|
||||
house_id, _conf, method = houses_mod.match_or_create_house(
|
||||
db, # type: ignore[arg-type]
|
||||
"avito",
|
||||
"ext-1",
|
||||
address="ул. Крауля, 44",
|
||||
lat=lat,
|
||||
lon=lon,
|
||||
region_code=region_code,
|
||||
)
|
||||
assert house_id == 4242, f"дом не создан (method={method})"
|
||||
assert db.insert_params is not None
|
||||
return db.insert_params
|
||||
|
||||
|
||||
def test_coords_confirm_region_inherited() -> None:
|
||||
"""Координаты в bbox 66 + развёртка 66 → дом рождается с region_code=66."""
|
||||
assert _create(56.83, 60.60, 66).get("region_code") == 66
|
||||
|
||||
|
||||
def test_no_coords_no_contradiction_inherited() -> None:
|
||||
"""Без координат противоречия нет — регион развёртки наследуется."""
|
||||
assert _create(None, None, 66).get("region_code") == 66
|
||||
|
||||
|
||||
def test_contradicting_coords_quarantine(caplog: pytest.LogCaptureFixture) -> None:
|
||||
"""ЕКБ-развёртка с координатами в bbox Москвы (порченый геокод, прод-кейс
|
||||
«Крауля, гп 1») → NULL-карантин + warning, а НЕ выдуманный 77."""
|
||||
import logging
|
||||
|
||||
with caplog.at_level(logging.WARNING):
|
||||
params = _create(55.716, 37.268, 66)
|
||||
assert params.get("region_code") is None, "порченому геокоду выдумали регион вместо карантина"
|
||||
assert "карантин" in caplog.text
|
||||
|
||||
|
||||
def test_legacy_call_without_region_stays_null() -> None:
|
||||
"""Вызов без региона развёртки (легаси) — NULL, не дефолт-66."""
|
||||
assert _create(56.83, 60.60, None).get("region_code") is None
|
||||
|
||||
|
||||
def test_alien_coords_outside_all_regions_quarantine() -> None:
|
||||
"""Координаты вне всех регионов (Таллин, прод-кейс «Учителей») при развёртке 66:
|
||||
region_for_point → None → противоречия ФОРМАЛЬНО нет — наследуем 66? НЕТ:
|
||||
правило намеренно наследует (см. док-стринг matching/houses.py — вне-bbox
|
||||
координаты не выводят ДРУГОЙ регион). Закрепляем текущую семантику, чтобы
|
||||
смена была осознанной, а не случайной."""
|
||||
assert _create(59.42, 24.75, 66).get("region_code") == 66
|
||||
|
|
@ -971,7 +971,9 @@ def save_listings(
|
|||
if listing_id is not None:
|
||||
try:
|
||||
with db.begin_nested():
|
||||
_link_listing_to_house(db, listing_id, lot, matcher, city=lot_city)
|
||||
_link_listing_to_house(
|
||||
db, listing_id, lot, matcher, city=lot_city, region_code=region_code
|
||||
)
|
||||
matched += 1
|
||||
except Exception as e:
|
||||
# Best-effort hook: log and continue so the listings batch isn't aborted.
|
||||
|
|
@ -1010,7 +1012,13 @@ def _to_json(value: Any) -> str:
|
|||
|
||||
|
||||
def _link_listing_to_house(
|
||||
db: Session, listing_id: int, lot: ScrapedLot, matcher: HouseMatcher, *, city: str | None = None
|
||||
db: Session,
|
||||
listing_id: int,
|
||||
lot: ScrapedLot,
|
||||
matcher: HouseMatcher,
|
||||
*,
|
||||
city: str | None = None,
|
||||
region_code: int | None = None,
|
||||
) -> None:
|
||||
"""Hook scraped listing into matching service: resolve house, upsert listing_sources.
|
||||
|
||||
|
|
@ -1070,6 +1078,9 @@ def _link_listing_to_house(
|
|||
building_cadastral_number=lot.building_cadastral_number,
|
||||
source_url=lot.house_url or lot.source_url,
|
||||
city=city,
|
||||
# #3051 п.4: регион развёртки — новый дом наследует его только при
|
||||
# непротиворечивых координатах (правило в matching/houses.py).
|
||||
region_code=region_code,
|
||||
)
|
||||
|
||||
# Mirror the resolved house into listings.house_id_fk so direct
|
||||
|
|
|
|||
|
|
@ -67,9 +67,16 @@ class HouseMatcher(Protocol):
|
|||
building_cadastral_number: str | None = ...,
|
||||
source_url: str | None = ...,
|
||||
city: str | None = ...,
|
||||
region_code: int | None = ...,
|
||||
) -> tuple[int | None, float, str]:
|
||||
"""Найти или создать канонический дом.
|
||||
|
||||
NB (#3051 п.4): `region_code` — регион РАЗВЁРТКИ этой карточки (тот же, что
|
||||
уходит в `listings.region_code`). Новый дом наследует его ТОЛЬКО если
|
||||
координаты не противоречат (нет координат — тоже не противоречие); при
|
||||
противоречии дом рождается с region_code=NULL (карантин), а не с выдуманным
|
||||
регионом — см. правило в matching/houses.py.
|
||||
|
||||
NB (#2777): `city` — город-цель развёртки этой карточки (тот же, что уходит в
|
||||
`listings.city`). Единственное наблюдение города, НЕ выведенное из строки адреса;
|
||||
без него бескоординатная карточка областного формата («ул. Кирова,4») матчится в
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue