From 27fdd55aaef3968a4738b0d5eec93d53b0927768 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Thu, 27 Aug 2026 13:51:48 +0500 Subject: [PATCH] =?UTF-8?q?feat(tradein/matching):=20region=5Fcode=20?= =?UTF-8?q?=D1=83=20houses=20=E2=80=94=20=D0=B2=D1=8B=D0=B2=D0=BE=D0=B4,?= =?UTF-8?q?=20=D0=B0=20=D0=BD=D0=B5=20=D0=B2=D1=8B=D0=B4=D1=83=D0=BC=D0=BA?= =?UTF-8?q?=D0=B0=20(#3051=20=D0=BF.4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Миграция 272: колонка + бэкфилл ТОЛЬКО по bbox региона 66 (значения байт-в-байт из реестра, синхронизацию держит тест). Карантин NULL: 620 домов без geom, 23 порченых (ЕКБ-адреса с чужими координатами — «Вильгельма де Геннина» на Байкале, «Крауля» под Москвой, «Учителей» в Таллине, с живыми ссылками листингов) — им регион не присваивается, включая 3 дома с координатами в bbox Москвы (порча, не переезд). NOT NULL из постановки — отдельной миграцией, когда карантин опустеет. Запись: новый дом наследует регион РАЗВЁРТКИ (base.py → контракт → адаптер → matching), только если координаты не противоречат; координаты другого региона → NULL-карантин + warning. Вне-bbox координаты при живой развёртке наследуют её регион (адрес и развёртка согласны, координатам веры нет) — семантика закреплена тестом, чтобы смена была осознанной. Матчинг-запросы НЕ тронуты — гард по региону это #3052. Co-Authored-By: Claude Opus 5 --- .../backend/app/services/matching/houses.py | 34 ++++- .../backend/app/services/scraper_adapters.py | 2 + .../data/sql/272_houses_region_code.sql | 37 +++++ .../tests/test_3051_houses_region_code.py | 137 ++++++++++++++++++ .../scraper-kit/src/scraper_kit/base.py | 15 +- .../scraper-kit/src/scraper_kit/contracts.py | 7 + 6 files changed, 228 insertions(+), 4 deletions(-) create mode 100644 tradein-mvp/backend/data/sql/272_houses_region_code.sql create mode 100644 tradein-mvp/backend/tests/test_3051_houses_region_code.py diff --git a/tradein-mvp/backend/app/services/matching/houses.py b/tradein-mvp/backend/app/services/matching/houses.py index 531cb1fc..562454a5 100644 --- a/tradein-mvp/backend/app/services/matching/houses.py +++ b/tradein-mvp/backend/app/services/matching/houses.py @@ -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() diff --git a/tradein-mvp/backend/app/services/scraper_adapters.py b/tradein-mvp/backend/app/services/scraper_adapters.py index 01bf055a..837da32f 100644 --- a/tradein-mvp/backend/app/services/scraper_adapters.py +++ b/tradein-mvp/backend/app/services/scraper_adapters.py @@ -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( diff --git a/tradein-mvp/backend/data/sql/272_houses_region_code.sql b/tradein-mvp/backend/data/sql/272_houses_region_code.sql new file mode 100644 index 00000000..9e1b5a27 --- /dev/null +++ b/tradein-mvp/backend/data/sql/272_houses_region_code.sql @@ -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; diff --git a/tradein-mvp/backend/tests/test_3051_houses_region_code.py b/tradein-mvp/backend/tests/test_3051_houses_region_code.py new file mode 100644 index 00000000..8c4228e7 --- /dev/null +++ b/tradein-mvp/backend/tests/test_3051_houses_region_code.py @@ -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 diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/base.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/base.py index 2cca7a21..d8676724 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/base.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/base.py @@ -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 diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/contracts.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/contracts.py index 931648dc..16f50bea 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/contracts.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/contracts.py @@ -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») матчится в -- 2.45.3