Merge pull request 'feat(tradein/matching): region_code у houses — вывод, а не выдумка (#3051 п.4)' (#3121) from feat/3051-houses-region-code into main
All checks were successful
Deploy Trade-In / changes (push) Successful in 20s
Deploy Trade-In / build-browser (push) Successful in 36s
Deploy Trade-In / build-frontend (push) Successful in 2m30s
Deploy Trade-In / test (push) Successful in 3m54s
Deploy Trade-In / build-backend (push) Successful in 1m36s
Deploy Trade-In / deploy (push) Successful in 6m47s
Deploy Trade-In / deploy-status (push) Successful in 1s
Deploy Trade-In / perimeter-smoke (push) Successful in 11s
All checks were successful
Deploy Trade-In / changes (push) Successful in 20s
Deploy Trade-In / build-browser (push) Successful in 36s
Deploy Trade-In / build-frontend (push) Successful in 2m30s
Deploy Trade-In / test (push) Successful in 3m54s
Deploy Trade-In / build-backend (push) Successful in 1m36s
Deploy Trade-In / deploy (push) Successful in 6m47s
Deploy Trade-In / deploy-status (push) Successful in 1s
Deploy Trade-In / perimeter-smoke (push) Successful in 11s
This commit is contained in:
commit
c6c934fb99
6 changed files with 228 additions and 4 deletions
|
|
@ -44,6 +44,7 @@ import logging
|
||||||
from sqlalchemy import text
|
from sqlalchemy import text
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
|
|
||||||
|
from app.services import regions as regions_mod
|
||||||
from app.services.matching.normalize import (
|
from app.services.matching.normalize import (
|
||||||
EKB_CITY_TOKEN,
|
EKB_CITY_TOKEN,
|
||||||
address_fingerprint,
|
address_fingerprint,
|
||||||
|
|
@ -77,6 +78,7 @@ def match_or_create_house(
|
||||||
building_cadastral_number: str | None = None,
|
building_cadastral_number: str | None = None,
|
||||||
source_url: str | None = None,
|
source_url: str | None = None,
|
||||||
city: str | None = None,
|
city: str | None = None,
|
||||||
|
region_code: int | None = None,
|
||||||
) -> tuple[int | None, float, str]:
|
) -> tuple[int | None, float, str]:
|
||||||
"""Match existing house or create new canonical record.
|
"""Match existing house or create new canonical record.
|
||||||
|
|
||||||
|
|
@ -407,19 +409,46 @@ def match_or_create_house(
|
||||||
# New house — INSERT canonical record.
|
# New house — INSERT canonical record.
|
||||||
# geom column is auto-populated by houses_set_geom_trg BEFORE INSERT trigger from lat/lon.
|
# 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.
|
# 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}"
|
url = source_url or f"matching://{ext_source}/{ext_id}"
|
||||||
row = (
|
row = (
|
||||||
db.execute(
|
db.execute(
|
||||||
text("""
|
text("""
|
||||||
INSERT INTO houses (source, ext_house_id, url, address, lat, lon, year_built,
|
INSERT INTO houses (source, ext_house_id, url, address, lat, lon, year_built,
|
||||||
cadastral_number)
|
cadastral_number, region_code)
|
||||||
VALUES (
|
VALUES (
|
||||||
:src, :eid, :url,
|
:src, :eid, :url,
|
||||||
:addr,
|
:addr,
|
||||||
CAST(:lat AS double precision),
|
CAST(:lat AS double precision),
|
||||||
CAST(:lon AS double precision),
|
CAST(:lon AS double precision),
|
||||||
CAST(:yb AS integer),
|
CAST(:yb AS integer),
|
||||||
:cad
|
:cad,
|
||||||
|
CAST(:region_code AS smallint)
|
||||||
)
|
)
|
||||||
ON CONFLICT (source, ext_house_id) DO UPDATE SET
|
ON CONFLICT (source, ext_house_id) DO UPDATE SET
|
||||||
address = COALESCE(EXCLUDED.address, houses.address)
|
address = COALESCE(EXCLUDED.address, houses.address)
|
||||||
|
|
@ -434,6 +463,7 @@ def match_or_create_house(
|
||||||
"lon": lon,
|
"lon": lon,
|
||||||
"yb": year_built,
|
"yb": year_built,
|
||||||
"cad": cad,
|
"cad": cad,
|
||||||
|
"region_code": region_val,
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
.mappings()
|
.mappings()
|
||||||
|
|
|
||||||
|
|
@ -68,6 +68,7 @@ class RealMatcherAdapter:
|
||||||
building_cadastral_number: str | None = None,
|
building_cadastral_number: str | None = None,
|
||||||
source_url: str | None = None,
|
source_url: str | None = None,
|
||||||
city: str | None = None,
|
city: str | None = None,
|
||||||
|
region_code: int | None = None,
|
||||||
) -> tuple[int | None, float, str]:
|
) -> tuple[int | None, float, str]:
|
||||||
# house_id is None when the matcher refuses a numberless address without a
|
# house_id is None when the matcher refuses a numberless address without a
|
||||||
# cadastral number (method 'no_house_number', P1). Callers must tolerate None.
|
# cadastral number (method 'no_house_number', P1). Callers must tolerate None.
|
||||||
|
|
@ -82,6 +83,7 @@ class RealMatcherAdapter:
|
||||||
building_cadastral_number=building_cadastral_number,
|
building_cadastral_number=building_cadastral_number,
|
||||||
source_url=source_url,
|
source_url=source_url,
|
||||||
city=city,
|
city=city,
|
||||||
|
region_code=region_code,
|
||||||
)
|
)
|
||||||
|
|
||||||
def upsert_listing_source(
|
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:
|
if listing_id is not None:
|
||||||
try:
|
try:
|
||||||
with db.begin_nested():
|
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
|
matched += 1
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
# Best-effort hook: log and continue so the listings batch isn't aborted.
|
# 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(
|
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:
|
) -> None:
|
||||||
"""Hook scraped listing into matching service: resolve house, upsert listing_sources.
|
"""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,
|
building_cadastral_number=lot.building_cadastral_number,
|
||||||
source_url=lot.house_url or lot.source_url,
|
source_url=lot.house_url or lot.source_url,
|
||||||
city=city,
|
city=city,
|
||||||
|
# #3051 п.4: регион развёртки — новый дом наследует его только при
|
||||||
|
# непротиворечивых координатах (правило в matching/houses.py).
|
||||||
|
region_code=region_code,
|
||||||
)
|
)
|
||||||
|
|
||||||
# Mirror the resolved house into listings.house_id_fk so direct
|
# Mirror the resolved house into listings.house_id_fk so direct
|
||||||
|
|
|
||||||
|
|
@ -67,9 +67,16 @@ class HouseMatcher(Protocol):
|
||||||
building_cadastral_number: str | None = ...,
|
building_cadastral_number: str | None = ...,
|
||||||
source_url: str | None = ...,
|
source_url: str | None = ...,
|
||||||
city: str | None = ...,
|
city: str | None = ...,
|
||||||
|
region_code: int | None = ...,
|
||||||
) -> tuple[int | None, float, str]:
|
) -> tuple[int | None, float, str]:
|
||||||
"""Найти или создать канонический дом.
|
"""Найти или создать канонический дом.
|
||||||
|
|
||||||
|
NB (#3051 п.4): `region_code` — регион РАЗВЁРТКИ этой карточки (тот же, что
|
||||||
|
уходит в `listings.region_code`). Новый дом наследует его ТОЛЬКО если
|
||||||
|
координаты не противоречат (нет координат — тоже не противоречие); при
|
||||||
|
противоречии дом рождается с region_code=NULL (карантин), а не с выдуманным
|
||||||
|
регионом — см. правило в matching/houses.py.
|
||||||
|
|
||||||
NB (#2777): `city` — город-цель развёртки этой карточки (тот же, что уходит в
|
NB (#2777): `city` — город-цель развёртки этой карточки (тот же, что уходит в
|
||||||
`listings.city`). Единственное наблюдение города, НЕ выведенное из строки адреса;
|
`listings.city`). Единственное наблюдение города, НЕ выведенное из строки адреса;
|
||||||
без него бескоординатная карточка областного формата («ул. Кирова,4») матчится в
|
без него бескоординатная карточка областного формата («ул. Кирова,4») матчится в
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue