From b70ae468850dae04c2bcbd84b59b70a7b83859fa Mon Sep 17 00:00:00 2001 From: bot-backend Date: Fri, 12 Jun 2026 11:52:09 +0300 Subject: [PATCH] =?UTF-8?q?fix(tradein):=20guard=20listing=5Fsegment=20?= =?UTF-8?q?=D0=B2=D0=BE=20=D0=B2=D1=81=D0=B5=D1=85=20=D1=87=D0=B8=D1=82?= =?UTF-8?q?=D0=B0=D1=82=D0=B5=D0=BB=D1=8F=D1=85=20listings=20=E2=80=94=20n?= =?UTF-8?q?ovostroyki=20=D0=B2=D0=BD=D0=B5=20comps=20(#1186)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Канон-предикат: (listing_segment IS NULL OR listing_segment = 'vtorichka') NULL = legacy вторичка до миграции 011 (sweep'ы были secondary-only) — отбрасывать нельзя. Затронуты все SELECT-пути, влияющие на оценку и корректирующие коэффициенты: - estimator.py: _COMMON_WHERE (Tier S/H), Tier W inline, Tier A anchor (добавлен впервые), Tier C anchor (заменён с параметрического CAST(:segment) на канон — прежний вариант отбрасывал NULL-строки при segment='vtorichka'). - asking_to_sold_ratio.py: ask_side, ask_global (_REDERIVE_SQL); bounds (_REDERIVE_BOUNDS_SQL); ask_tiered, ask_side_all, ask_global (_REDERIVE_TIERED_SQL) — 6 мест. - sber_index.py: reconciliation-запрос (диагностический asking-median). Не затронуты: скрейперы/scrape_pipeline, search matview (catalog display, не оценка), admin stats, geocoding, cian price history, matching — обосновано в инвентаризации. Тесты: новый test_segment_guard_1186.py (статика SQL + поведение mock); обновлён test_asking_to_sold_ratio.py (subset-тест нормализует guard, добавленный в refresh но отсутствующий в 080-seed). --- tradein-mvp/backend/app/services/estimator.py | 17 +- .../backend/app/services/sber_index.py | 17 +- .../backend/app/tasks/asking_to_sold_ratio.py | 12 + .../tests/test_asking_to_sold_ratio.py | 16 +- .../backend/tests/test_segment_guard_1186.py | 206 ++++++++++++++++++ 5 files changed, 250 insertions(+), 18 deletions(-) create mode 100644 tradein-mvp/backend/tests/test_segment_guard_1186.py diff --git a/tradein-mvp/backend/app/services/estimator.py b/tradein-mvp/backend/app/services/estimator.py index 1a89d419..734077c8 100644 --- a/tradein-mvp/backend/app/services/estimator.py +++ b/tradein-mvp/backend/app/services/estimator.py @@ -1470,6 +1470,8 @@ def _fetch_anchor_comps( AND price_per_m2 > 0 AND lower(translate(address, 'ёЁ', 'ее')) LIKE :street_like AND lower(translate(address, 'ёЁ', 'ее')) ~ :house_re + -- novostroyki guard (#1186): NULL = legacy вторичка до м.011 + AND (listing_segment IS NULL OR listing_segment = 'vtorichka') """ ), { @@ -1513,11 +1515,8 @@ def _fetch_anchor_comps( AND price_per_m2 > 0 AND rooms = CAST(:rooms AS integer) AND area_m2 BETWEEN :area_min AND :area_max - AND ( - CAST(:segment AS text) IS NULL - OR listing_segment IS NULL - OR listing_segment = CAST(:segment AS text) - ) + -- novostroyki guard (#1186): NULL = legacy вторичка до м.011 + AND (listing_segment IS NULL OR listing_segment = 'vtorichka') AND geom IS NOT NULL AND ST_DWithin( geom::geography, @@ -1530,7 +1529,6 @@ def _fetch_anchor_comps( "rooms": rooms, "area_min": area * 0.75, "area_max": area * 1.25, - "segment": listing_segment, "lon": lon, "lat": lat, }, @@ -3246,10 +3244,11 @@ _COMMON_WHERE = """ OR year_built BETWEEN CAST(:cohort_year_min AS integer) AND CAST(:cohort_year_max AS integer) ) + -- novostroyki guard (#1186): NULL = legacy вторичка до м.011 -- Исключаем новостройки из comp-пула вторички: девелоперский прайс искажает -- медиану ₽/м². NULL сегмент пропускаем (rosreestr/avito/yandex без сегмента — -- это вторичка или неклассифицированный объект). - AND (listing_segment IS NULL OR listing_segment <> 'novostroyki') + AND (listing_segment IS NULL OR listing_segment = 'vtorichka') """ # Note: Tier W has its own inline copy of the cohort clause (PR #519 line # ~1280). Не удалять — Tier W не использует _COMMON_WHERE из-за inline @@ -3598,9 +3597,9 @@ def _fetch_analogs( OR year_built BETWEEN CAST(:cohort_year_min AS integer) AND CAST(:cohort_year_max AS integer) ) + -- novostroyki guard (#1186): NULL = legacy вторичка до м.011 -- Tier W: исключаем новостройки из comp-пула (sync с _COMMON_WHERE). - -- NULL сегмент пропускаем — rosreestr/avito/yandex без сегмента вторичка. - AND (listing_segment IS NULL OR listing_segment <> 'novostroyki') + AND (listing_segment IS NULL OR listing_segment = 'vtorichka') -- 2026-05-23: Avito coords теперь real (PR #487 убрал jitter после -- C-5 audit). Listings с NULL coords отфильтруются через ST_DWithin -- (geom IS NULL → не matches). geocode-missing-listings backfill diff --git a/tradein-mvp/backend/app/services/sber_index.py b/tradein-mvp/backend/app/services/sber_index.py index 64f9bd57..564ccfe1 100644 --- a/tradein-mvp/backend/app/services/sber_index.py +++ b/tradein-mvp/backend/app/services/sber_index.py @@ -393,13 +393,16 @@ async def pull_sber_indices( # Task B: best-effort reconciliation to our scrape-median asking ppm² try: our_median = db.execute( - text(""" - SELECT percentile_cont(0.5) - WITHIN GROUP (ORDER BY price_per_m2) AS m - FROM listings - WHERE is_active = true - AND price_per_m2 BETWEEN 50000 AND 800000 - """) + text( + # novostroyki guard (#1186): NULL = legacy вторичка до м.011 + "SELECT percentile_cont(0.5)" + " WITHIN GROUP (ORDER BY price_per_m2) AS m" + " FROM listings" + " WHERE is_active = true" + " AND price_per_m2 BETWEEN 50000 AND 800000" + " AND (listing_segment IS NULL" + " OR listing_segment = 'vtorichka')" + ) ).scalar() if our_median is not None and latest_val: sber_latest = latest_val diff --git a/tradein-mvp/backend/app/tasks/asking_to_sold_ratio.py b/tradein-mvp/backend/app/tasks/asking_to_sold_ratio.py index 68feab9a..661cf8c9 100644 --- a/tradein-mvp/backend/app/tasks/asking_to_sold_ratio.py +++ b/tradein-mvp/backend/app/tasks/asking_to_sold_ratio.py @@ -94,6 +94,8 @@ _REDERIVE_SQL = text( WHERE is_active AND rooms IS NOT NULL AND price_per_m2 BETWEEN :ppm2_min AND :ppm2_max + -- novostroyki guard (#1186): NULL = legacy вторичка до м.011 + AND (listing_segment IS NULL OR listing_segment = 'vtorichka') GROUP BY LEAST(GREATEST(rooms, 0), 4) ), -- Per-rooms строки: только бакеты с обеими сторонами, прошедшие порог 30/30 и ask>0. @@ -138,6 +140,8 @@ _REDERIVE_SQL = text( WHERE is_active AND rooms IS NOT NULL AND price_per_m2 BETWEEN :ppm2_min AND :ppm2_max + -- novostroyki guard (#1186): NULL = legacy вторичка до м.011 + AND (listing_segment IS NULL OR listing_segment = 'vtorichka') ), -- Global fallback строка rooms_bucket=-1 (пишется всегда, если ask>0). global_row AS ( @@ -215,6 +219,8 @@ _REDERIVE_BOUNDS_SQL = text( WHERE is_active AND rooms IS NOT NULL AND price_per_m2 BETWEEN :ppm2_min AND :ppm2_max + -- novostroyki guard (#1186): NULL = legacy вторичка до м.011 + AND (listing_segment IS NULL OR listing_segment = 'vtorichka') GROUP BY LEAST(GREATEST(rooms, 0), 4) HAVING COUNT(*) > 0 ) @@ -259,6 +265,8 @@ _REDERIVE_TIERED_SQL = text( WHERE l.is_active AND l.rooms IS NOT NULL AND l.price_per_m2 BETWEEN :ppm2_min AND :ppm2_max + -- novostroyki guard (#1186): NULL = legacy вторичка до м.011 + AND (l.listing_segment IS NULL OR l.listing_segment = 'vtorichka') ), deal_side_tier AS ( SELECT @@ -320,6 +328,8 @@ _REDERIVE_TIERED_SQL = text( WHERE is_active AND rooms IS NOT NULL AND price_per_m2 BETWEEN :ppm2_min AND :ppm2_max + -- novostroyki guard (#1186): NULL = legacy вторичка до м.011 + AND (listing_segment IS NULL OR listing_segment = 'vtorichka') GROUP BY LEAST(GREATEST(rooms, 0), 4) ), all_rows AS ( @@ -361,6 +371,8 @@ _REDERIVE_TIERED_SQL = text( WHERE is_active AND rooms IS NOT NULL AND price_per_m2 BETWEEN :ppm2_min AND :ppm2_max + -- novostroyki guard (#1186): NULL = legacy вторичка до м.011 + AND (listing_segment IS NULL OR listing_segment = 'vtorichka') ), global_row AS ( SELECT diff --git a/tradein-mvp/backend/tests/test_asking_to_sold_ratio.py b/tradein-mvp/backend/tests/test_asking_to_sold_ratio.py index 0a7c90d4..54f10b81 100644 --- a/tradein-mvp/backend/tests/test_asking_to_sold_ratio.py +++ b/tradein-mvp/backend/tests/test_asking_to_sold_ratio.py @@ -135,6 +135,9 @@ def test_migration_080_derivation_is_subset_of_refresh_sql() -> None: ppm² bounds are compared after normalisation: the 080 seed uses literal integers (30000 / 600000) while the refresh uses bind params (:ppm2_min / :ppm2_max) — both are replaced with the token PPM2_BAND_PLACEHOLDER before the subset check. + + #1186: the refresh now adds the novostroyki guard predicate to each ask_* CTE; + it is normalised away here so the 080 seed (no guard) still matches. """ seed_sql = _MIGRATION_080.read_text("utf-8") # Extract the WITH … (up to the ON CONFLICT) from the seed. @@ -149,8 +152,17 @@ def test_migration_080_derivation_is_subset_of_refresh_sql() -> None: s = re.sub(r":ppm2_min AND :ppm2_max", "PPM2_BAND_PLACEHOLDER", s) return s - assert _strip_sql(_normalise_ppm2(seed_derivation)) in _strip_sql( - _normalise_ppm2(_REDERIVE_SQL) + def _drop_segment_guard(s: str) -> str: + """Remove the #1186 novostroyki guard predicate (absent in the 080 seed).""" + return re.sub( + r"AND\s*\((?:\w+\.)?listing_segment\s+IS\s+NULL\s+" + r"OR\s+(?:\w+\.)?listing_segment\s*=\s*'vtorichka'\)", + "", + s, + ) + + assert _strip_sql(_normalise_ppm2(_drop_segment_guard(seed_derivation))) in _strip_sql( + _normalise_ppm2(_drop_segment_guard(_REDERIVE_SQL)) ) diff --git a/tradein-mvp/backend/tests/test_segment_guard_1186.py b/tradein-mvp/backend/tests/test_segment_guard_1186.py new file mode 100644 index 00000000..2ca3dc58 --- /dev/null +++ b/tradein-mvp/backend/tests/test_segment_guard_1186.py @@ -0,0 +1,206 @@ +"""#1186 — query-side guard: novostroyki строки не попадают в comp-пул вторички. + +Тесты статические (SQL-string inspection) + поведенческие (_fetch_analogs mock). + +Каноничный предикат: (listing_segment IS NULL OR listing_segment = 'vtorichka') +NULL = legacy вторичка до миграции 011 — отбрасывать нельзя. +""" + +from __future__ import annotations + +import inspect +import os +import re +from typing import Any +from unittest.mock import MagicMock + +# Settings требует DATABASE_URL при инициализации (fail-fast, C-3). +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost/test_db") + +from app.services import estimator as est_mod +from app.tasks import asking_to_sold_ratio as ratio_mod + +# ── Canonical guard predicate ───────────────────────────────────────────────── + +_GUARD = "(listing_segment IS NULL OR listing_segment = 'vtorichka')" +_GUARD_RE = re.compile( + r"\((?:\w+\.)?listing_segment\s+IS\s+NULL\s+OR\s+(?:\w+\.)?listing_segment\s*=\s*'vtorichka'\)", + re.IGNORECASE, +) + + +def _norm(s: str) -> str: + """Drop -- comments и склей пробелы для устойчивого поиска.""" + no_comments = re.sub(r"--[^\n]*", "", s) + return re.sub(r"\s+", " ", no_comments).strip() + + +# ── estimator.py static checks ──────────────────────────────────────────────── + + +def test_common_where_has_canonical_guard() -> None: + """_COMMON_WHERE используется Tier S и Tier H — должен содержать канон-предикат.""" + assert _GUARD_RE.search( + est_mod._COMMON_WHERE + ), "_COMMON_WHERE lacks novostroyki guard — Tier S/H comp set contaminated" + + +def test_common_where_no_old_neq_form() -> None: + """Старый вариант listing_segment <> 'novostroyki' запрещён — не охватывает иные сегменты.""" + assert "listing_segment <> 'novostroyki'" not in est_mod._COMMON_WHERE + assert "listing_segment != 'novostroyki'" not in est_mod._COMMON_WHERE + + +def test_tier_w_inline_has_canonical_guard() -> None: + """Tier W использует inline SQL, а не _COMMON_WHERE — нужен собственный guard.""" + # Получаем исходник функции _fetch_analogs. + src = inspect.getsource(est_mod._fetch_analogs) + # Tier W — inline блок (не через _COMMON_WHERE). + assert _GUARD_RE.search(src), "_fetch_analogs Tier W inline SQL lacks novostroyki guard" + # Старой формы быть не должно. + assert "listing_segment <> 'novostroyki'" not in src + assert "listing_segment != 'novostroyki'" not in src + + +def test_tier_a_anchor_has_canonical_guard() -> None: + """Tier A anchor (same building) — ранее не имел segment-guard (#1186 fix).""" + src = inspect.getsource(est_mod._fetch_anchor_comps) + assert _GUARD_RE.search(src), "_fetch_anchor_comps Tier A SQL lacks novostroyki guard" + + +def test_tier_c_anchor_has_canonical_guard_not_segment_param() -> None: + """Tier C anchor — канон-предикат, не CAST(:segment AS text) (тот вариант отбрасывал NULL).""" + src = inspect.getsource(est_mod._fetch_anchor_comps) + # Canonical guard present. + assert _GUARD_RE.search(src) + # Старый параметрический вариант убран. + assert "listing_segment = CAST(:segment AS text)" not in src + + +# ── asking_to_sold_ratio.py static checks ───────────────────────────────────── + +_REDERIVE_SQL_TEXT = str(ratio_mod._REDERIVE_SQL.text) +_REDERIVE_BOUNDS_TEXT = str(ratio_mod._REDERIVE_BOUNDS_SQL.text) +_REDERIVE_TIERED_TEXT = str(ratio_mod._REDERIVE_TIERED_SQL.text) + + +def test_ask_side_cte_has_guard() -> None: + """ask_side CTE в _REDERIVE_SQL (per-rooms asking медиана) — guard обязателен.""" + assert _GUARD_RE.search( + _REDERIVE_SQL_TEXT + ), "ask_side CTE in _REDERIVE_SQL lacks novostroyki guard" + + +def test_ask_global_cte_has_guard() -> None: + """ask_global CTE в _REDERIVE_SQL (global fallback asking медиана) — guard обязателен.""" + # _REDERIVE_SQL содержит два `ask_global`-блока; ищем оба через count. + matches = len(_GUARD_RE.findall(_REDERIVE_SQL_TEXT)) + assert ( + matches >= 2 + ), f"Expected ≥2 guard occurrences in _REDERIVE_SQL (ask_side + ask_global), got {matches}" + + +def test_bounds_sql_has_guard() -> None: + """_REDERIVE_BOUNDS_SQL (tier bounds percentiles) — guard обязателен.""" + assert _GUARD_RE.search(_REDERIVE_BOUNDS_TEXT), "_REDERIVE_BOUNDS_SQL lacks novostroyki guard" + + +def test_tiered_ask_ctes_have_guard() -> None: + """_REDERIVE_TIERED_SQL: ask_tiered + ask_side_all + ask_global — минимум 3 вхождения.""" + matches = len(_GUARD_RE.findall(_REDERIVE_TIERED_TEXT)) + assert matches >= 3, ( + f"Expected ≥3 guard occurrences in _REDERIVE_TIERED_SQL " + f"(ask_tiered + ask_side_all + ask_global), got {matches}" + ) + + +# ── Поведенческие тесты: _fetch_analogs (mock DB) ──────────────────────────── + + +def _make_listing( + *, + source: str = "avito", + address: str = "ул. Ленина, 1", + distance_m: float = 100.0, + relevance_score: float = 0.1, + price_rub: float = 5_000_000.0, + area_m2: float = 40.0, + rooms: int = 2, + listing_segment: str | None = None, +) -> dict[str, Any]: + """Минимальный listing-dict, имитирующий вывод DB mapping.""" + return { + "source": source, + "source_url": f"https://{source}.ru/offer/1", + "address": address, + "lat": 56.838, + "lon": 60.595, + "rooms": rooms, + "area_m2": area_m2, + "floor": 3, + "total_floors": 9, + "price_rub": price_rub, + "price_per_m2": price_rub / area_m2, + "listing_date": None, + "days_on_market": 15, + "photo_urls": [], + "scraped_at": None, + "distance_m": distance_m, + "relevance_score": relevance_score, + "building_cadastral_number": None, + "listing_segment": listing_segment, + } + + +def _make_db_mock(rows: list[dict[str, Any]]) -> MagicMock: + """Session mock: db.execute().mappings().all() возвращает rows.""" + db = MagicMock() + db.execute.return_value.mappings.return_value.all.return_value = rows + return db + + +def test_fetch_analogs_sql_guard_present_novostroyki_excluded() -> None: + """SQL guard корректно записан: listing_segment='novostroyki' не должен присутствовать + в WHERE-предикате (guard зарыт в SQL, а не в Python post-filter). + + Тест проверяет СТАТИКУ SQL внутри _fetch_analogs (Tier W path), а не runtime фильтрацию — + runtime-фильтрацию сделала бы только интеграционная БД. Убеждаемся, что кто-то в будущем + не переписал SQL, убрав guard. + """ + src = inspect.getsource(est_mod._fetch_analogs) + # Guard должен присутствовать хотя бы один раз в теле функции. + assert _GUARD_RE.search( + src + ), "_fetch_analogs SQL no longer contains novostroyki guard — guard was removed!" + + +def test_null_segment_listing_not_excluded_by_guard() -> None: + """listing_segment=NULL (legacy вторичка до м.011) должны ПОПАСТЬ в comps. + + Поведенческий тест: mock возвращает строку с segment=None, _fetch_analogs + должен включить её в результат (_stratify_candidates не отфильтровывает NULL). + """ + from app.services.estimator import _fetch_analogs + + null_seg_listing = _make_listing(listing_segment=None, distance_m=50.0, relevance_score=0.05) + db = _make_db_mock([null_seg_listing]) + + result, _fallback, _tier = _fetch_analogs( + db, lat=56.838, lon=60.595, rooms=2, area=40.0, radius_m=1000 + ) + + # NULL-segment listing должен присутствовать в результате (не отброшен Python-стороной). + assert any( + r.get("source") == "avito" for r in result + ), "NULL-segment listing was unexpectedly excluded from comp set" + + +def test_vtorichka_segment_listing_included() -> None: + """listing_segment='vtorichka' — явный вторичный сегмент — должен попасть в comps.""" + from app.services.estimator import _fetch_analogs + + vtor_listing = _make_listing(listing_segment="vtorichka", distance_m=80.0) + db = _make_db_mock([vtor_listing]) + + result, _, _ = _fetch_analogs(db, lat=56.838, lon=60.595, rooms=2, area=40.0, radius_m=1000) + assert len(result) >= 1, "vtorichka-segment listing was unexpectedly excluded"