diff --git a/tradein-mvp/backend/app/services/estimator.py b/tradein-mvp/backend/app/services/estimator.py index d533bb6e..45c2d552 100644 --- a/tradein-mvp/backend/app/services/estimator.py +++ b/tradein-mvp/backend/app/services/estimator.py @@ -1622,6 +1622,20 @@ def _normalize_building_key( (корпус) схлопываются к base (тот же дом). Литеры — РАЗНЫЕ дома (204г ≠ 204д). - street_core прогоняется через _STREET_ALIAS_MAP (ткачева→ткачей). + #2581: город/р-н НАМЕРЕННО дропается (не возвращается в ключе) — это + единственное, что делает «Ткачёва 13»-вариант с городом и без города давать + один ключ (см. test_normalize_tkachei13_all_db_variants_same_key). Городской + токен как 4-й элемент ключа НЕ добавлен: (а) часть источников (Avito + anonymous-адреса) вообще не несёт городской токен в тексте — ключ с + обязательным городом сломал бы их матчинг; (б) написание города варьируется + (ЕКБ/Екатеринбург/г. Екатеринбург) — ещё один normalization-слой, дающий + те же false-negative риски, которые уже решает `_CITY_TOKENS`-дропинг. + Кросс-городская коллизия (одноимённая улица+дом в разных городах области) + закрыта на SQL-уровне через ST_DWithin от subject-координат — см. Tier A + в `_fetch_anchor_comps` (#2581) и Tier S в `_fetch_analogs` (#oblast-D, + f9ae6f0c) — геопредикат надёжнее строкового city-токена и не ломает то, + что уже нормализуется здесь. + Returns (street_core, base_no, letter) — любой элемент None если не извлёкся. Best-effort: при пустом адресе → (None, None, None). """ @@ -1722,6 +1736,18 @@ def _anchor_comp_from_row(r: Any) -> dict[str, Any]: } +# #2581: Tier A ("same building") ST_DWithin safety-radius. Reuses the SAME +# city-scale DEFAULT_RADIUS_M already used by Tier S's mirrored geo-bound fix +# (f9ae6f0c, #oblast-D). The address-string match (_normalize_building_key + +# _house_boundary_regex) already establishes "same street + house number" — +# this radius only needs to reject GENUINELY cross-city collisions (e.g. +# «улица Ленина» exists in both Екатеринбург AND Серов/Нижний Тагил, 150+ km +# apart) — it does not need to be building-tight like Tier C's 500m +# micro-radius (that tier's precision comes from proximity alone, without a +# street/house string match to lean on). +ANCHOR_TIER_A_RADIUS_M = DEFAULT_RADIUS_M + + def _fetch_anchor_comps( db: Session, *, @@ -1734,9 +1760,19 @@ def _fetch_anchor_comps( ) -> tuple[list[dict[str, Any]], str | None]: """Тированный набор комплов для same-building якоря. Стоп на 1-м тире с ≥ min_comps. - Tier A — SAME BUILDING: normalized street + base house no (+ литера если есть). - RELAXED rooms (без фильтра), БЕЗ area±15%. Не группируем по house_id_fk — - один дом дробится на несколько fk (Хохрякова 48 = 7085/9878/12797). + Tier A — SAME BUILDING: normalized street + base house no (+ литера если есть) + + ST_DWithin(ANCHOR_TIER_A_RADIUS_M) от subject lat/lon (#2581 — до этого + SQL не имел ГЕО-предиката вовсе, и одноимённая улица+дом в ДРУГОМ городе + (область — 368 городов, «Ленина»/«Мира»/... повторяются) молча матчила + ЕКБ-листинги для областного subject'а — ~40 191 из ~40 200 активных + листингов ЕКБ, distance неизвестна без фильтра). RELAXED rooms (без + фильтра), БЕЗ area±15%. Не группируем по house_id_fk — один дом дробится + на несколько fk (Хохрякова 48 = 7085/9878/12797); ST_DWithin — тот же + компромисс, что и Tier S ниже (см. _fetch_analogs), не house_id_fk. + lat/lon subject'а обязательны (гейт как у Tier C) — без них геопредикат + невозможен, и Tier A целиком пропускается (в проде geo ВСЕГДА есть — + estimate_quality возвращает _empty_estimate раньше при неудачном + geocode, см. `if geo is None`). Tier C — micro-radius ≤500m (ST_DWithin) + вторичка-канон guard (#1186): NULL = legacy вторичка + rooms match + area±25%. (Tier B «тот же ЖК» — skip: complex_id/cian_zhk_url ненадёжны.) @@ -1752,7 +1788,7 @@ def _fetch_anchor_comps( # ── Tier A: same building ──────────────────────────────────────────────── street, base_no, letter = _normalize_building_key(address) - if street and base_no is not None: + if street and base_no is not None and lat is not None and lon is not None: # ё→е в SQL для symmetry с нормализатором. psycopg v3: bind через :param, # оператор ~. Boundary-regex вынесен в _house_boundary_regex (общий с # Tier S radius-fallback ниже, см. _fetch_analogs). @@ -1771,11 +1807,20 @@ def _fetch_anchor_comps( AND price_per_m2 > 0 AND lower(translate(address, 'ёЁ', 'ее')) LIKE :street_like AND lower(translate(address, 'ёЁ', 'ее')) ~ :house_re + AND geom IS NOT NULL + AND ST_DWithin( + geom::geography, + ST_MakePoint(:lon, :lat)::geography, + :radius + ) """ ), { "street_like": "%" + street + "%", "house_re": house_re, + "lon": lon, + "lat": lat, + "radius": ANCHOR_TIER_A_RADIUS_M, }, ) .mappings() @@ -1900,12 +1945,17 @@ def _band_haircut(anchor_ppm2: float) -> float: LOW audit #3: 0.04/0.07 (и mid из settings.asking_to_sold_haircut) — EKB-secondary-market calibration constants, но применяются ENGINE-WIDE (нет city-параметра ни здесь, ни у единственного вызывающего - `_compute_same_building_anchor`). Реального импакта на не-ЕКБ область пока нет - (same-building anchor pool для oblast сейчас не формируется — anchor_ppm2 сюда - просто не доходит), но это доверие к отсутствию данных, а не к дизайну. Как - только oblast anchor pools появятся (см. #oblast-D fallback выше), эти пороги - нужно пересмотреть/сделать per-city — не оставлять ЕКБ-калибровку по умолчанию - для другого рынка. No behavior change here (doc-only). + `_compute_same_building_anchor`). #2581 update: та формулировка была НЕВЕРНОЙ — + same-building anchor pool для oblast ВСЕГДА мог сформироваться (Tier A до + #2581 не имел гео-предиката вообще, поэтому for oblast-subject'ов он либо + молча тянул ЕКБ-листинги по одноимённой улице/дому, либо — для действительно + уникальных названий — честно матчил местные листинги, если они были). После + #2581 (ST_DWithin(ANCHOR_TIER_A_RADIUS_M) на Tier A) anchor_ppm2 ДЛЯ ОБЛАСТИ + доходит сюда легитимно (местные комплы того же дома в Серове/Тагиле/etc, если + они есть в БД), но всё ещё через ЕКБ-калиброванный haircut — эти пороги + по-прежнему стоит пересмотреть/сделать per-city, не оставлять ЕКБ-калибровку + по умолчанию для другого рынка. No behavior change here (doc-only, кроме + исправления ложной посылки). """ if anchor_ppm2 >= 350_000: return 0.04 diff --git a/tradein-mvp/backend/tests/test_estimator_deals_headline_fallback_oblast_d.py b/tradein-mvp/backend/tests/test_estimator_deals_headline_fallback_oblast_d.py index 93b7f502..2315dd61 100644 --- a/tradein-mvp/backend/tests/test_estimator_deals_headline_fallback_oblast_d.py +++ b/tradein-mvp/backend/tests/test_estimator_deals_headline_fallback_oblast_d.py @@ -17,6 +17,7 @@ DB-facing helpers only) — the same pattern as test_estimator_radius_floor.py. from __future__ import annotations +import math import os from datetime import UTC, datetime from typing import Any @@ -100,6 +101,24 @@ def _run_estimate( # what the always-executed final fallback tier returns. return_value=(list(analogs), False, "W"), ), + # #2581: explicit, not accidental. Before this fix, `db = MagicMock()` + # was left UNCONFIGURED for `_fetch_anchor_comps` — since it's not + # patched here, it ran for REAL against the mock session, and + # `db.execute(...).mappings().all()` on a bare MagicMock silently + # returns `[]` (MagicMock's default `__iter__` == `iter([])`), so + # `_fetch_anchor_comps` ALWAYS returned `([], None)` regardless of + # what the real SQL would do. That made this whole test file blind + # to the #2581 anchor cross-city leak: `anchor_tier` could never + # observe becoming 'A' here, so a regression that makes Tier A + # wrongly match an EKB listing for a Серов/Тагил subject (which + # would then BLOCK this very deals-headline-fallback via the + # `anchor_tier is None` gate, see _price_from_inputs #oblast-D) was + # invisible. Patched explicitly now so the assumption is documented + # and intentional. The behavioral regression test itself lives in + # test_non_ekb_anchor_not_leaked_from_ekb_street_collision below, + # which does NOT patch `_fetch_anchor_comps` — it exercises the + # real SQL/geo-bound logic instead. + patch("app.services.estimator._fetch_anchor_comps", return_value=([], None)), patch("app.services.estimator._fetch_deals", return_value=[]), patch( "app.services.estimator._get_or_fetch_imv_cached", @@ -236,3 +255,258 @@ def test_ekb_with_dense_listings_ignores_deals_fallback() -> None: # n_analogs must reflect the real listing count (deals-fallback never ran). assert est.n_analogs == len(analogs) assert 140_000 <= est.median_price_per_m2 <= 150_000 + + +# ── #2581 regression: Tier A anchor must not leak cross-city street collisions ─ +# +# Unlike the tests above (which explicitly patch `_fetch_anchor_comps` — see +# the comment on that patch in `_run_estimate`), the tests below do NOT patch +# it: they exercise the real Tier A SQL/geo-bound logic against a hand-rolled +# `db.execute` fake that computes genuine haversine distance, mirroring what +# Postgres' ST_DWithin would decide. This is what actually catches a #2581-class +# regression; the tests above only prove the deals-fallback logic given +# anchor_tier=None as an already-resolved input. + + +def _haversine_m(lat1: float, lon1: float, lat2: float, lon2: float) -> float: + """Real great-circle distance — stands in for what Postgres ST_DWithin computes.""" + r = 6_371_000.0 + p1, p2 = math.radians(lat1), math.radians(lat2) + dphi = math.radians(lat2 - lat1) + dlmb = math.radians(lon2 - lon1) + a = math.sin(dphi / 2) ** 2 + math.cos(p1) * math.cos(p2) * math.sin(dlmb / 2) ** 2 + return 2 * r * math.asin(math.sqrt(a)) + + +# A real EKB listing on "ул. Ленина" — SAME normalized street+house-number key +# as the Серов subject below (_normalize_building_key drops the city token), +# ~280 km away. Pre-#2581 the Tier A SQL had NO geo predicate at all, so an +# address-string match against this row would have been returned +# unconditionally — the actual reported bug (ЕКБ "Ленина 5" leaking into a +# Серов estimate at ~150-190k ₽/м² vs. the real ~30k deal corridor). +_EKB_LENINA_LISTING = { + "price_per_m2": 186_000.0, + "area_m2": 45.0, + "rooms": 2, + "floor": 5, + "total_floors": 16, + "address": "Екатеринбург, ул. Ленина, 5", + "source": "cian", + "source_url": "https://cian.ru/sale/flat/leak1/", + "price_rub": 186_000.0 * 45.0, + "listing_date": None, + "days_on_market": 12, + "photo_urls": [], + "lat": 56.838, + "lon": 60.595, + "listing_segment": "vtorichka", + "source_id": "leak1", +} + + +def _ekb_lenina_pool(prices_per_m2: list[float]) -> list[dict[str, Any]]: + """N distinct EKB "ул. Ленина" comps (>= estimate_sb_min_comps=4 needed for + Tier A to actually FIRE — see _fetch_anchor_comps `len(comps) >= min_comps`). + Distinct floor/source_id/price_rub per row so `_dedup_cross_source` (#2265, + street+floor+area+price physical key) treats them as distinct units, not + cross-posted duplicates of the same lot. + """ + return [ + { + **_EKB_LENINA_LISTING, + "source_id": f"leak{i}", + "source_url": f"https://cian.ru/sale/flat/leak{i}/", + "floor": 3 + 2 * i, + "price_per_m2": ppm2, + "price_rub": ppm2 * _EKB_LENINA_LISTING["area_m2"], + } + for i, ppm2 in enumerate(prices_per_m2) + ] + + +def _serov_geo() -> Any: + from app.services.geocoder import GeocodeResult + + return GeocodeResult( + lat=59.6047, + lon=60.5876, + full_address="Свердловская обл., Серов, ул. Ленина, 5", + provider="nominatim", + ) + + +def _serov_payload() -> Any: + from app.schemas.trade_in import TradeInEstimateInput + + return TradeInEstimateInput( + address="Серов, ул. Ленина, 5", area_m2=45.0, rooms=2, floor=5, total_floors=9 + ) + + +def _fake_anchor_sql_execute( + row_pool: list[dict[str, Any]], subject_lat: float, subject_lon: float +) -> Any: + """`db.execute` side_effect faking real ST_DWithin filtering for the Tier A + anchor SQL only. Identifies that query by its distinctive bound params + (`house_re` + `street_like`, unique to Tier A in the whole module). + + If the query ALSO binds `lat`/`lon`/`radius` (post-#2581), computes the + real haversine distance and returns rows from `row_pool` ONLY if within + `radius` of the bound subject `lat`/`lon` — exactly what Postgres' + ST_DWithin would decide. If those params are ABSENT (pre-#2581 — the SQL + had no geo predicate at all), returns `row_pool` UNCONDITIONALLY — this is + the faithful old-code behaviour (matched by address string alone, + regardless of distance), NOT an error: the fake must reproduce the actual + bug for the "prove it fails on old code" check to be meaningful, rather + than accidentally passing via an unrelated KeyError caught by + `_fetch_anchor_comps`'s own try/except. + + Everything else (Tier C anchor, IMV anchor, etc.) degrades to the same + empty-result default a bare, unconfigured `MagicMock()` gives. + """ + + def _side_effect(query: Any, params: dict[str, Any] | None = None) -> MagicMock: + result = MagicMock() + params = params or {} + if "house_re" in params and "street_like" in params: + if "lat" in params and "lon" in params and "radius" in params: + dist = _haversine_m(params["lat"], params["lon"], subject_lat, subject_lon) + rows = row_pool if dist <= params["radius"] else [] + else: + rows = row_pool + result.mappings.return_value.all.return_value = rows + else: + result.mappings.return_value.all.return_value = [] + return result + + return _side_effect + + +def test_non_ekb_anchor_not_leaked_from_ekb_street_collision() -> None: + """#2581: Серов «ул. Ленина, 5» must NOT anchor on an EKB «ул. Ленина, 5» listing. + + Pre-fix this would have returned tier='A' from the EKB listing (~186k + ₽/м²) and, critically, the `anchor_tier is None` gate on the + deals-headline-fallback (_price_from_inputs, #oblast-D) would then have + BLOCKED the honest deal-corridor headline too — Серов would surface the + EKB-leaked ~186k figure, never even reaching the deals-fallback path. + """ + dkp_raw = { + "count": 12, + "low_ppm2": 25_000, + "median_ppm2": 30_000, + "high_ppm2": 38_000, + "period_months": 12, + } + db = MagicMock() + # >= estimate_sb_min_comps EKB comps — realistic (40 191 of ~40 200 active + # listings are EKB) and necessary for Tier A to actually fire pre-fix. + db.execute.side_effect = _fake_anchor_sql_execute( + _ekb_lenina_pool([178_000.0, 186_000.0, 190_000.0, 184_000.0]), + subject_lat=56.838, + subject_lon=60.595, + ) + geo = _serov_geo() + payload = _serov_payload() + + async def _run() -> Any: + from app.services.estimator import estimate_quality + + with ( + patch("app.services.estimator.geocode", new=AsyncMock(return_value=geo)), + patch("app.services.estimator.dadata_clean_address", new=AsyncMock(return_value=None)), + patch("app.services.estimator.match_house_readonly", return_value=None), + patch("app.services.estimator.get_house_metadata", new=AsyncMock(return_value=None)), + patch("app.services.estimator._fetch_analogs", return_value=([], False, "W")), + patch("app.services.estimator._fetch_deals", return_value=[]), + patch( + "app.services.estimator._get_or_fetch_imv_cached", + new=AsyncMock(return_value=None), + ), + patch( + "app.services.estimator._get_or_fetch_yandex_valuation_cached", + new=AsyncMock(return_value=None), + ), + patch( + "app.services.estimator.estimate_via_cian_valuation", + new=AsyncMock(return_value=None), + ), + patch("app.services.estimator._fetch_dkp_corridor", return_value=dkp_raw), + patch("app.services.estimator._get_asking_sold_ratio", return_value=(None, None)), + ): + return await estimate_quality(payload, db) + + est = anyio.run(_run) + + assert est.median_price_per_m2 == 30_000, ( + f"headline={est.median_price_per_m2} must come from the honest deal " + "corridor (30_000), not an EKB-leaked Tier A anchor (~186k) — the " + "cross-city street-name collision must be rejected by ST_DWithin" + ) + assert est.median_price_per_m2 < 100_000, "must NOT be EKB-leaked (~186k)" + assert est.n_analogs == 0 + assert est.confidence == "low" + + +def test_ekb_anchor_still_works_with_real_same_city_comps() -> None: + """#2581 control: EKB same-building anchor must keep working post-fix. + + Multiple EKB listings on the subject's own street/house, all within the + ST_DWithin radius of the subject's own coordinates, must still form a + Tier A anchor — proving the geo-bound only rejects genuinely distant + (cross-city) collisions, not legitimate same-building EKB matches. + """ + from app.schemas.trade_in import TradeInEstimateInput + from app.services.geocoder import GeocodeResult + + subject_lat, subject_lon = 56.838, 60.595 + comps = _ekb_lenina_pool([140_000.0, 145_000.0, 150_000.0, 148_000.0]) + + db = MagicMock() + db.execute.side_effect = _fake_anchor_sql_execute( + comps, subject_lat=subject_lat, subject_lon=subject_lon + ) + + geo = GeocodeResult( + lat=subject_lat, + lon=subject_lon, + full_address="Свердловская обл., Екатеринбург, ул. Ленина, 5", + provider="nominatim", + ) + payload = TradeInEstimateInput( + address="Екатеринбург, ул. Ленина, 5", area_m2=45.0, rooms=2, floor=5, total_floors=16 + ) + + async def _run() -> Any: + from app.services.estimator import estimate_quality + + with ( + patch("app.services.estimator.geocode", new=AsyncMock(return_value=geo)), + patch("app.services.estimator.dadata_clean_address", new=AsyncMock(return_value=None)), + patch("app.services.estimator.match_house_readonly", return_value=None), + patch("app.services.estimator.get_house_metadata", new=AsyncMock(return_value=None)), + patch("app.services.estimator._fetch_analogs", return_value=([], False, "W")), + patch("app.services.estimator._fetch_deals", return_value=[]), + patch( + "app.services.estimator._get_or_fetch_imv_cached", + new=AsyncMock(return_value=None), + ), + patch( + "app.services.estimator._get_or_fetch_yandex_valuation_cached", + new=AsyncMock(return_value=None), + ), + patch( + "app.services.estimator.estimate_via_cian_valuation", + new=AsyncMock(return_value=None), + ), + patch("app.services.estimator._fetch_dkp_corridor", return_value=None), + patch("app.services.estimator._get_asking_sold_ratio", return_value=(None, None)), + ): + return await estimate_quality(payload, db) + + est = anyio.run(_run) + + # Same-building anchor engaged (EKB doesn't degrade): headline built from + # the 4 same-building comps, not left n/a / not routed through deals. + assert est.n_analogs == 4 + assert 138_000 <= est.median_price_per_m2 <= 152_000 diff --git a/tradein-mvp/backend/tests/test_tier_a_primary_1774.py b/tradein-mvp/backend/tests/test_tier_a_primary_1774.py index 262fc372..373ab2b0 100644 --- a/tradein-mvp/backend/tests/test_tier_a_primary_1774.py +++ b/tradein-mvp/backend/tests/test_tier_a_primary_1774.py @@ -76,14 +76,30 @@ def _db_mock(rows: list[dict[str, Any]]) -> MagicMock: return db -def _fetch(db: MagicMock) -> tuple[list[dict[str, Any]], str | None]: - """Вызов _fetch_anchor_comps с Tier A-релевантными аргументами (без lat/lon → Tier C skip).""" +# #2581: subject lat/lon — совпадают с _row()'s hardcoded lat/lon (56.838/60.595), +# т.е. subject и comps в одной точке ЕКБ → ST_DWithin(ANCHOR_TIER_A_RADIUS_M) +# тривиально проходит, не мешая #1774 novostroyki-gating semantics ниже. +_LAT = 56.838 +_LON = 60.595 + + +def _fetch( + db: MagicMock, *, lat: float | None = None, lon: float | None = None +) -> tuple[list[dict[str, Any]], str | None]: + """Вызов _fetch_anchor_comps с Tier A-релевантными аргументами. + + lat/lon по умолчанию None — тесты, ожидающие tier=None (novostroyki-gate + отбраковал все comps до min_comps), передают None намеренно: с #2581 + геогейтом Tier A целиком пропускается без lat/lon (см. Tier C — тот же + паттерн), что и раньше давало tier=None (Tier C тоже требовал lat/lon). + Тесты, ожидающие tier=='A', передают _LAT/_LON явно. + """ return _fetch_anchor_comps( db, address=_ADDRESS, target_house_id=None, - lat=None, - lon=None, + lat=lat, + lon=lon, rooms=2, area=50.0, ) @@ -106,7 +122,7 @@ def test_tier_a_includes_novostroyki_when_secondary_present() -> None: ] db = _db_mock(rows) with patch.object(est_mod.settings, "estimate_sb_min_comps", 4): - comps, tier = _fetch(db) + comps, tier = _fetch(db, lat=_LAT, lon=_LON) assert tier == "A" # Все 6 (4 вторички + 2 novostroyki-переуступки) учтены. assert len(comps) == 6 @@ -178,7 +194,7 @@ def test_tier_a_dedup_same_source_id_collapses() -> None: ] db = _db_mock(rows) with patch.object(est_mod.settings, "estimate_sb_min_comps", 4): - comps, tier = _fetch(db) + comps, tier = _fetch(db, lat=_LAT, lon=_LON) assert tier == "A" # 6 строк, но 2 cian-строки с одинаковым source_id → 1 comp → итого 5. assert len(comps) == 5 @@ -214,7 +230,7 @@ def test_tier_a_dedup_same_source_id_different_url_collapses() -> None: ] db = _db_mock(rows) with patch.object(est_mod.settings, "estimate_sb_min_comps", 4): - comps, tier = _fetch(db) + comps, tier = _fetch(db, lat=_LAT, lon=_LON) assert tier == "A" # source_id-primary схлопывает несмотря на разные url → 5 comps. assert len(comps) == 5 @@ -233,7 +249,7 @@ def test_tier_a_dedup_null_url_keeps_distinct_rows() -> None: ] db = _db_mock(rows) with patch.object(est_mod.settings, "estimate_sb_min_comps", 4): - comps, tier = _fetch(db) + comps, tier = _fetch(db, lat=_LAT, lon=_LON) assert tier == "A" # 4 разных лота (разные source_id/площадь) → 4 comps, ничего не схлопнуто. assert len(comps) == 4