From d0105470f4598bb28c072581e1e37d8bf7ac2c62 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 15 Aug 2026 20:16:11 +0300 Subject: [PATCH 1/3] =?UTF-8?q?feat(tradein/coverage):=20=D0=B1=D0=B5?= =?UTF-8?q?=D1=81=D0=BF=D0=BB=D0=B0=D1=82=D0=BD=D0=B0=D1=8F=20=D0=BF=D1=80?= =?UTF-8?q?=D0=BE=D0=B1=D0=B0=20=D0=BF=D0=BE=D0=BA=D1=80=D1=8B=D1=82=D0=B8?= =?UTF-8?q?=D1=8F=20=D0=B4=D0=BB=D1=8F=20=D0=BB=D0=B5=D0=BD=D0=B4=D0=B8?= =?UTF-8?q?=D0=BD=D0=B3=D0=B0=20=D0=9C=D0=95=D0=A0=D0=90=20(#2894)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /api/v1/trade-in/coverage — до оплаты пользователь видит только n похожих объявлений в радиусе 1000м и медианный возраст листинга, без единой цены. Один SQL (радиус GIST + rooms + area ±15% + freshness 14д + тот же дедуп/cap- канон, что у estimator._fetch_analogs), ноль внешних вызовов, ноль записей. Пороги ok/thin/not_covered — константы рядом с ручкой (зелёные города >=8, жёлтые >=12, остальные всегда not_covered). Поле median_listing_age_days (не "срок продажи" — возраст активного объявления, цензурированная выборка). RBAC не тронут — путь остаётся закрытым, открытие анонимного периметра вынесено в #2895. --- tradein-mvp/backend/app/api/v1/trade_in.py | 162 ++++++++++- tradein-mvp/backend/app/schemas/trade_in.py | 48 ++++ .../tests/test_coverage_probe_endpoint.py | 271 ++++++++++++++++++ 3 files changed, 480 insertions(+), 1 deletion(-) create mode 100644 tradein-mvp/backend/tests/test_coverage_probe_endpoint.py diff --git a/tradein-mvp/backend/app/api/v1/trade_in.py b/tradein-mvp/backend/app/api/v1/trade_in.py index 45d1a561..b0b4be85 100644 --- a/tradein-mvp/backend/app/api/v1/trade_in.py +++ b/tradein-mvp/backend/app/api/v1/trade_in.py @@ -10,7 +10,7 @@ import calendar import json import logging from datetime import UTC, date, datetime, timedelta -from typing import Annotated, Any +from typing import Annotated, Any, Literal from uuid import UUID from fastapi import APIRouter, Depends, File, Header, HTTPException, Request, Response, UploadFile @@ -25,6 +25,8 @@ from app.schemas.trade_in import ( AnalogLot, AvitoImvSummary, CianPriceChangeStats, + CoverageProbeInput, + CoverageProbeResponse, DkpCorridor, HouseAnalyticsKpi, HouseAnalyticsResponse, @@ -2549,3 +2551,161 @@ def get_sales_vs_listings( data_quality="street_only" if total_deals > 0 else "no_data", pairs=pairs, ) + + +# ── Coverage probe (#2894) — бесплатный шаг лэндинга, ЦЕНЫ НЕТ ───────────────── +# До оплаты человек видит, СКОЛЬКО похожих квартир продаётся рядом и КАК БЫСТРО +# они уходят — ни одной рублёвой цифры (см. CoverageProbeResponse docstring). +# Один SQL, ноль внешних вызовов, ноль записей — ручка дешёвая специально: её +# планируется открыть анонимам отдельной задачей (#2895, со своим consent- +# гейтом). RBAC здесь НЕ трогаем — путь остаётся закрытым (не в _PUBLIC_PATHS). +# строго 1000м по ТЗ #2894 (НЕ DEFAULT_RADIUS_M эстиматора — тот допускает fallback до 2000) +COVERAGE_RADIUS_M = 1000 +COVERAGE_AREA_TOLERANCE = 0.15 # ±15% площади +COVERAGE_FRESH_DAYS = 14 # объявления не старше 14 дней (тот же канон, что LISTINGS_FRESH_DAYS) + +# Списки городов и пороги — константа РЯДОМ С РУЧКОЙ (issue #2894 требование), не в БД. +COVERAGE_GREEN_CITIES = ("Екатеринбург", "Верхняя Пышма", "Берёзовский", "Среднеуральск") +COVERAGE_YELLOW_CITIES = ("Нижний Тагил", "Каменск-Уральский", "Первоуральск", "Ревда") +COVERAGE_GREEN_MIN_N = 8 +COVERAGE_YELLOW_MIN_N = 12 + + +def _fold_city(name: str) -> str: + """ёЁ→еЕ + casefold — та же normalization-идиома, что для адресов (см. #1774).""" + return name.strip().translate(str.maketrans("ёЁ", "ее")).casefold() + + +_COVERAGE_CITY_THRESHOLDS: dict[str, tuple[str, int]] = { + **{_fold_city(c): (c, COVERAGE_GREEN_MIN_N) for c in COVERAGE_GREEN_CITIES}, + **{_fold_city(c): (c, COVERAGE_YELLOW_MIN_N) for c in COVERAGE_YELLOW_CITIES}, +} + + +def _resolve_coverage_city(city_hint: str | None, cohort_city: str | None) -> tuple[str, int, bool]: + """Резолвит (display_city, threshold, is_supported) для пробы покрытия. + + Приоритет: явный city_hint фронта (тот же автокомплит, что заполняет + TradeInEstimateInput.city_hint) > мода city найденной SQL-когорты + (best-effort фолбэк, когда фронт его не передал). Город вне зелёного/ + жёлтого списка → threshold=0, is_supported=False — вызывающий обязан + трактовать это как not_covered независимо от n_listings. + """ + candidate = (city_hint or cohort_city or "").strip() + match = _COVERAGE_CITY_THRESHOLDS.get(_fold_city(candidate)) if candidate else None + if match is not None: + display, threshold = match + return display, threshold, True + return candidate, 0, False + + +@router.post("/coverage", response_model=CoverageProbeResponse) +def coverage_probe( + payload: CoverageProbeInput, + db: Annotated[Session, Depends(get_db)], +) -> CoverageProbeResponse: + """Бесплатная проба покрытия (issue #2894) — сколько похожих квартир рядом. + + Когорта — тот же дедуп/cap-канон, что radius-тиры в estimator._fetch_analogs + (rn_dup по (source, source_id), rn_addr cap по адресу, реюз тех же + приватных helper'ов эстиматора — импорт локальный, как и в остальных + ручках этого файла, чтобы не тащить тяжёлый app.services.estimator + в module-level import graph): ST_DWithin 1000м, rooms точное совпадение, + area ±15%, scraped_at не старше 14 дней, is_active. + + В ответе НЕТ ни одной цены — см. CoverageProbeResponse docstring. + + #oblast (2026-08): house_placement_history.exposure_days — реальная (не + цензурированная) экспозиция history-строк — НЕ используется здесь: это + house-level архив (join по house_id, не привязан к текущей radius/rooms/ + area когорте один-в-один), а не активные листинги в подобранном радиусе; + сведение двух разных когорт усложнило бы «один дешёвый SQL» без выигрыша + в честности (у нас и так честное имя поля — age активного объявления, не + срок продажи). См. openQuestions PR #2894 при ревью. + """ + from app.services.estimator import _RN_DUP_WINDOW, MAX_ANALOGS_PER_ADDRESS + + area_min = payload.area_m2 * (1 - COVERAGE_AREA_TOLERANCE) + area_max = payload.area_m2 * (1 + COVERAGE_AREA_TOLERANCE) + + row = ( + db.execute( + text( + f""" + WITH base AS ( + SELECT + city, + days_on_market, + row_number() OVER ( + PARTITION BY address ORDER BY scraped_at DESC + ) AS rn_addr, +{_RN_DUP_WINDOW} + FROM listings + WHERE is_active = true + AND rooms = :rooms + AND area_m2 BETWEEN :area_min AND :area_max + AND scraped_at > NOW() - (:fresh_days || ' days')::interval + AND ST_DWithin( + geom::geography, ST_MakePoint(:lon, :lat)::geography, :radius + ) + ) + SELECT + count(*) AS n_listings, + percentile_cont(0.5) WITHIN GROUP (ORDER BY days_on_market) + AS median_age_days, + mode() WITHIN GROUP (ORDER BY city) + FILTER (WHERE city IS NOT NULL) AS cohort_city + FROM base + WHERE rn_addr <= :max_per_addr + AND rn_dup = 1 + """ + ), + { + "rooms": payload.rooms, + "area_min": area_min, + "area_max": area_max, + "fresh_days": COVERAGE_FRESH_DAYS, + "lat": payload.lat, + "lon": payload.lon, + "radius": COVERAGE_RADIUS_M, + "max_per_addr": MAX_ANALOGS_PER_ADDRESS, + }, + ) + .mappings() + .fetchone() + ) + + n_listings = int(row["n_listings"]) if row else 0 + median_age = ( + round(row["median_age_days"]) + if row is not None and row["median_age_days"] is not None + else None + ) + cohort_city = row["cohort_city"] if row else None + + city, threshold, supported = _resolve_coverage_city(payload.city_hint, cohort_city) + + if not supported or n_listings == 0: + status: Literal["ok", "thin", "not_covered"] = "not_covered" + elif n_listings >= threshold: + status = "ok" + else: + status = "thin" + + logger.info( + "coverage probe rooms=%d area=%.1f city=%r status=%s n=%d", + payload.rooms, + payload.area_m2, + city, + status, + n_listings, + ) + + return CoverageProbeResponse( + status=status, + n_listings=n_listings, + median_listing_age_days=median_age, + radius_m=COVERAGE_RADIUS_M, + city=city, + threshold=threshold, + ) diff --git a/tradein-mvp/backend/app/schemas/trade_in.py b/tradein-mvp/backend/app/schemas/trade_in.py index d7666f84..d8fc5c9b 100644 --- a/tradein-mvp/backend/app/schemas/trade_in.py +++ b/tradein-mvp/backend/app/schemas/trade_in.py @@ -756,3 +756,51 @@ class LocationIndexResponse(BaseModel): radius_m: int nearby_poi: list[NearbyPoiOut] poi_status: str + + +class CoverageProbeInput(BaseModel): + """Вход POST /api/v1/trade-in/coverage (issue #2894) — бесплатная проба покрытия. + + lat/lon — координаты, уже разрезолвленные фронтом (тот же контракт, что + TradeInEstimateInput.lat/lon — geocode делает фронт/автокомплит, эта ручка + сама НИКОГО не геокодирует). city_hint — опционально, из того же + автокомплита (см. TradeInEstimateInput.city_hint); без него город + резолвится best-effort из моды city найденной когорты. + """ + + lat: float = Field(ge=-90, le=90) + lon: float = Field(ge=-180, le=180) + rooms: int = Field(ge=0, le=10) # 0 = студия + area_m2: float = Field(gt=10, lt=500) + city_hint: str | None = Field(default=None, max_length=100) + + +class CoverageProbeResponse(BaseModel): + """Ответ POST /api/v1/trade-in/coverage. + + НАМЕРЕННО без единой цены (ни медианы, ни диапазона, ни ₽/м²) — продуктовое + правило issue #2894: бесплатный шаг доказывает, что похожие квартиры есть + и как быстро они уходят, а саму цену продукт продаёт на платном шаге. + + status: + - "ok" — n_listings >= порога для этого города (зелёный/жёлтый список). + - "thin" — когорта непустая, но n_listings < порога. + - "not_covered" — город вне зелёного/жёлтого списка ИЛИ когорта пустая + (n_listings == 0) — независимо от того, поддерживается город или нет. + + median_listing_age_days — ЧЕСТНОЕ имя: возраст АКТИВНОГО объявления + (days_on_market на текущий момент), а НЕ срок до продажи. Цензурированная + выборка (активные объявления ещё висят) всегда завышена относительно + реального времени экспозиции проданных — не путать со «сроком продажи». + + threshold — n, начиная с которого статус переходит в "ok" для резолвленного + города; 0, если город не входит ни в один список (порог неприменим — + статус в этом случае всегда "not_covered" вне зависимости от n_listings). + """ + + status: Literal["ok", "thin", "not_covered"] + n_listings: int + median_listing_age_days: int | None + radius_m: int + city: str + threshold: int diff --git a/tradein-mvp/backend/tests/test_coverage_probe_endpoint.py b/tradein-mvp/backend/tests/test_coverage_probe_endpoint.py new file mode 100644 index 00000000..42445e28 --- /dev/null +++ b/tradein-mvp/backend/tests/test_coverage_probe_endpoint.py @@ -0,0 +1,271 @@ +"""Tests for POST /api/v1/trade-in/coverage (issue #2894). + +Бесплатная проба покрытия для публичного лэндинга «МЕРА» — до оплаты человек +видит, сколько похожих квартир продаётся рядом и как быстро они уходят, без +единой рублёвой цифры в ответе. Covers: + - пороги ok/thin/not_covered для зелёных/жёлтых/неподдерживаемых городов + - пустая когорта (n=0) → not_covered даже в поддерживаемом городе + - в ответе НЕТ ни одного price-подобного поля (падающий тест на регресс схемы) + - city_hint приоритетнее моды city из когорты + - median_listing_age_days — median(days_on_market), None при пустой когорте +""" + +from __future__ import annotations + +import os +import sys +from unittest.mock import MagicMock + +# psycopg v3 driver required; stub DATABASE_URL before any app import. +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +# WeasyPrint requires GTK — not present in CI/Windows. Stub before any app import +# (trade_in.py imports generate_trade_in_pdf at module load). +_wp_mock = MagicMock() +sys.modules.setdefault("weasyprint", _wp_mock) +sys.modules.setdefault("weasyprint.CSS", _wp_mock) +sys.modules.setdefault("weasyprint.HTML", _wp_mock) + +import pytest # noqa: E402 +from fastapi import FastAPI # noqa: E402 +from fastapi.testclient import TestClient # noqa: E402 + +# ── Helpers ─────────────────────────────────────────────────────────────────── + + +@pytest.fixture() +def trade_in_app() -> FastAPI: + """Minimal FastAPI app mounting only the trade-in router with DB overridden.""" + from app.api.v1 import trade_in as trade_in_module + from app.core.db import get_db + + application = FastAPI() + application.include_router(trade_in_module.router, prefix="/api/v1/trade-in") + + def _override_db(): + yield MagicMock() + + application.dependency_overrides[get_db] = _override_db + return application + + +def _db_mock_returning(row: dict | None) -> MagicMock: + """DB session mock — coverage_probe reads db.execute(...).mappings().fetchone().""" + db = MagicMock() + mapping_result = MagicMock() + mapping_result.fetchone.return_value = row + execute_result = MagicMock() + execute_result.mappings.return_value = mapping_result + db.execute.return_value = execute_result + return db + + +def _override(app: FastAPI, db: MagicMock) -> None: + from app.core.db import get_db + + app.dependency_overrides[get_db] = lambda: (yield db) + + +_BASE_PAYLOAD = {"lat": 56.8384, "lon": 60.6057, "rooms": 2, "area_m2": 50.0} + + +# ── Response schema: NO price anywhere (issue #2894 hard rule) ──────────────── + +_PRICE_LIKE_SUBSTRINGS = ("price", "cena", "цена", "rub", "₽", "cost") + + +def test_coverage_response_has_no_price_fields(trade_in_app: FastAPI) -> None: + """Regression guard: response schema must never grow a price-shaped field.""" + from app.schemas.trade_in import CoverageProbeResponse + + field_names = set(CoverageProbeResponse.model_fields.keys()) + offending = [f for f in field_names if any(sub in f.lower() for sub in _PRICE_LIKE_SUBSTRINGS)] + assert not offending, f"CoverageProbeResponse must not carry price fields: {offending}" + + +def test_coverage_actual_response_has_no_price_fields(trade_in_app: FastAPI) -> None: + """Same guard but on a live serialized response (belt-and-suspenders).""" + db = _db_mock_returning( + {"n_listings": 10, "median_age_days": 21.0, "cohort_city": "Екатеринбург"} + ) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + resp = client.post("/api/v1/trade-in/coverage", json=_BASE_PAYLOAD) + assert resp.status_code == 200 + data = resp.json() + offending = [k for k in data if any(sub in k.lower() for sub in _PRICE_LIKE_SUBSTRINGS)] + assert not offending, f"response body must not carry price fields: {offending} in {data}" + + +# ── Thresholds: green city ───────────────────────────────────────────────────── + + +def test_green_city_ok_at_threshold(trade_in_app: FastAPI) -> None: + """Екатеринбург (зелёный, порог 8) — n=8 ровно на границе → ok.""" + db = _db_mock_returning( + {"n_listings": 8, "median_age_days": 15.0, "cohort_city": "Екатеринбург"} + ) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + resp = client.post("/api/v1/trade-in/coverage", json=_BASE_PAYLOAD) + assert resp.status_code == 200 + data = resp.json() + assert data["status"] == "ok" + assert data["n_listings"] == 8 + assert data["threshold"] == 8 + assert data["city"] == "Екатеринбург" + assert data["radius_m"] == 1000 + assert data["median_listing_age_days"] == 15 + + +def test_green_city_thin_below_threshold(trade_in_app: FastAPI) -> None: + """Екатеринбург, n=7 (< порог 8) → thin, не ok и не not_covered.""" + db = _db_mock_returning( + {"n_listings": 7, "median_age_days": 10.0, "cohort_city": "Екатеринбург"} + ) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + resp = client.post("/api/v1/trade-in/coverage", json=_BASE_PAYLOAD) + data = resp.json() + assert data["status"] == "thin" + assert data["n_listings"] == 7 + assert data["threshold"] == 8 + + +# ── Thresholds: yellow city ───────────────────────────────────────────────────── + + +def test_yellow_city_ok_at_threshold(trade_in_app: FastAPI) -> None: + """Нижний Тагил (жёлтый, порог 12) — n=12 → ok.""" + db = _db_mock_returning( + {"n_listings": 12, "median_age_days": 30.0, "cohort_city": "Нижний Тагил"} + ) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + resp = client.post( + "/api/v1/trade-in/coverage", + json={**_BASE_PAYLOAD, "city_hint": "Нижний Тагил"}, + ) + data = resp.json() + assert data["status"] == "ok" + assert data["threshold"] == 12 + assert data["city"] == "Нижний Тагил" + + +def test_yellow_city_thin_below_threshold(trade_in_app: FastAPI) -> None: + """Ревда, n=11 (< порог 12) → thin.""" + db = _db_mock_returning({"n_listings": 11, "median_age_days": 40.0, "cohort_city": None}) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + resp = client.post("/api/v1/trade-in/coverage", json={**_BASE_PAYLOAD, "city_hint": "Ревда"}) + data = resp.json() + assert data["status"] == "thin" + assert data["threshold"] == 12 + + +# ── City outside both lists → always not_covered ──────────────────────────────── + + +def test_unsupported_city_not_covered_even_with_high_n(trade_in_app: FastAPI) -> None: + """Город вне списков → not_covered независимо от n_listings (даже n=500).""" + db = _db_mock_returning({"n_listings": 500, "median_age_days": 5.0, "cohort_city": "Серов"}) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + resp = client.post("/api/v1/trade-in/coverage", json={**_BASE_PAYLOAD, "city_hint": "Серов"}) + data = resp.json() + assert data["status"] == "not_covered" + assert data["threshold"] == 0 + assert data["n_listings"] == 500 # честно отдаём счётчик, статус его игнорирует + + +# ── Empty cohort ────────────────────────────────────────────────────────────── + + +def test_empty_cohort_supported_city_not_covered(trade_in_app: FastAPI) -> None: + """n=0 в поддерживаемом (зелёном) городе → not_covered, не thin — честнее.""" + db = _db_mock_returning({"n_listings": 0, "median_age_days": None, "cohort_city": None}) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + resp = client.post( + "/api/v1/trade-in/coverage", json={**_BASE_PAYLOAD, "city_hint": "Екатеринбург"} + ) + data = resp.json() + assert data["status"] == "not_covered" + assert data["n_listings"] == 0 + assert data["median_listing_age_days"] is None + + +def test_empty_cohort_no_row_at_all(trade_in_app: FastAPI) -> None: + """DB возвращает None (defensive — count(*) агрегат всегда даёт строку, но + coverage_probe обязан не падать, даже если mock/driver вернул пусто).""" + db = _db_mock_returning(None) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + resp = client.post("/api/v1/trade-in/coverage", json=_BASE_PAYLOAD) + assert resp.status_code == 200 + data = resp.json() + assert data["status"] == "not_covered" + assert data["n_listings"] == 0 + assert data["median_listing_age_days"] is None + + +# ── city_hint priority over cohort mode ───────────────────────────────────────── + + +def test_city_hint_overrides_cohort_mode(trade_in_app: FastAPI) -> None: + """city_hint (фронт) побеждает cohort_city (SQL mode) при определении города.""" + db = _db_mock_returning({"n_listings": 9, "median_age_days": 12.0, "cohort_city": "Серов"}) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + resp = client.post( + "/api/v1/trade-in/coverage", + json={**_BASE_PAYLOAD, "city_hint": "Екатеринбург"}, + ) + data = resp.json() + assert data["city"] == "Екатеринбург" + assert data["status"] == "ok" # n=9 >= 8 (зелёный порог), не серовский not_covered + + +def test_yo_fold_city_hint_matches(trade_in_app: FastAPI) -> None: + """«Березовский» без ё должен резолвиться в тот же зелёный порог, что «Берёзовский».""" + db = _db_mock_returning({"n_listings": 8, "median_age_days": 5.0, "cohort_city": None}) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + resp = client.post( + "/api/v1/trade-in/coverage", + json={**_BASE_PAYLOAD, "city_hint": "березовский"}, + ) + data = resp.json() + assert data["status"] == "ok" + assert data["threshold"] == 8 + + +# ── DB dedup / cap params passed through ──────────────────────────────────────── + + +def test_coverage_sql_uses_radius_1000_and_area_tolerance(trade_in_app: FastAPI) -> None: + """SQL params: radius=1000 (строго), area ±15%, rooms exact.""" + db = _db_mock_returning({"n_listings": 0, "median_age_days": None, "cohort_city": None}) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + client.post("/api/v1/trade-in/coverage", json=_BASE_PAYLOAD) + + assert db.execute.called + call_args = db.execute.call_args + params = call_args[0][1] if len(call_args[0]) > 1 else call_args[1].get("parameters", {}) + assert params["radius"] == 1000 + assert params["rooms"] == 2 + assert params["area_min"] == pytest.approx(50.0 * 0.85) + assert params["area_max"] == pytest.approx(50.0 * 1.15) + assert params["fresh_days"] == 14 -- 2.45.3 From 3e9af2fdef12c79c4fdbbb3b2d1cf3140621e1e3 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 15 Aug 2026 20:47:23 +0300 Subject: [PATCH 2/3] fix(tradein/coverage): sync cohort with paid estimator, honest age medians MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent review found two MAJOR defects in POST /api/v1/trade-in/coverage: MAJOR-1: the probe cohort WHERE clause was missing three predicates present in estimator._COMMON_WHERE / Tier W (novostroyki guard, geo_precision != 'city', price_rub > 0) — the free probe could answer "ok" at points where the paid estimator's own 1000m radius tier sees zero real analogs. Prod example: 56.868904/60.837955, 2 rooms, 50 m2 gave n_listings=22/status=ok while the estimator's cohort at the same radius was 0 (all 54 rows were novostroyki). Added the three predicates verbatim from estimator.py, plus both a static SQL-text regression test and a real-Postgres integration test (skip_allowlist.txt, same _live_session() pattern as test_gar_flats_loader) that inserts novostroyka/geo_precision=city/price=0 rows and asserts they are not counted. MAJOR-2: median_listing_age_days was computed from days_on_market, which on prod is populated almost exclusively by one source (yandex) — thin cohorts produced a "median" over 1-2 listings. Added n_with_age to the response (honest count of listings the median is based on); median is now null below COVERAGE_MIN_AGE_SAMPLES=5, and values above COVERAGE_MAX_AGE_DAYS=365 (near -certainly dead listings, per prod: 15% of fresh yandex rows exceed 365d, max 4261d) are excluded as outliers before the percentile is computed. MINOR: city_hint was trusted at face value and echoed back verbatim — a client could pass city_hint="Екатеринбург" with coordinates in Серов and get threshold=8/status=ok. _resolve_coverage_city now prioritizes the SQL cohort's mode city (ground truth) over the client hint, falling back to hint only when the cohort is empty (where status is forced not_covered anyway). Unmatched cities no longer echo the raw client string in the city field. --- tradein-mvp/backend/app/api/v1/trade_in.py | 74 ++++- tradein-mvp/backend/app/schemas/trade_in.py | 11 + tradein-mvp/backend/tests/skip_allowlist.txt | 6 + .../tests/test_coverage_probe_endpoint.py | 283 ++++++++++++++++-- 4 files changed, 338 insertions(+), 36 deletions(-) diff --git a/tradein-mvp/backend/app/api/v1/trade_in.py b/tradein-mvp/backend/app/api/v1/trade_in.py index b0b4be85..71402948 100644 --- a/tradein-mvp/backend/app/api/v1/trade_in.py +++ b/tradein-mvp/backend/app/api/v1/trade_in.py @@ -2564,6 +2564,17 @@ COVERAGE_RADIUS_M = 1000 COVERAGE_AREA_TOLERANCE = 0.15 # ±15% площади COVERAGE_FRESH_DAYS = 14 # объявления не старше 14 дней (тот же канон, что LISTINGS_FRESH_DAYS) +# MAJOR-2 (независимый ревью #2894): days_on_market на проде заполнена практически +# только у yandex (avito/cian/domklik — 0 заполнено) — возраст известен у меньшинства +# когорты, и на тонких когортах "медиана" считалась по 1-2 объявлениям. Ниже порога +# n_with_age медиану не отдаём (null) — не продуктовое решение, а честность при +# заведомо шумной статистике по единичным точкам. +COVERAGE_MIN_AGE_SAMPLES = 5 +# 15% свежих yandex-строк имеют days_on_market > 365 (максимум 4261) — это почти +# наверняка мёртвое/забытое объявление, которое никто не снял с публикации, а не +# сигнал о реальном времени экспозиции рынка. Отбрасываем как выброс из медианы. +COVERAGE_MAX_AGE_DAYS = 365 + # Списки городов и пороги — константа РЯДОМ С РУЧКОЙ (issue #2894 требование), не в БД. COVERAGE_GREEN_CITIES = ("Екатеринбург", "Верхняя Пышма", "Берёзовский", "Среднеуральск") COVERAGE_YELLOW_CITIES = ("Нижний Тагил", "Каменск-Уральский", "Первоуральск", "Ревда") @@ -2585,18 +2596,24 @@ _COVERAGE_CITY_THRESHOLDS: dict[str, tuple[str, int]] = { def _resolve_coverage_city(city_hint: str | None, cohort_city: str | None) -> tuple[str, int, bool]: """Резолвит (display_city, threshold, is_supported) для пробы покрытия. - Приоритет: явный city_hint фронта (тот же автокомплит, что заполняет - TradeInEstimateInput.city_hint) > мода city найденной SQL-когорты - (best-effort фолбэк, когда фронт его не передал). Город вне зелёного/ - жёлтого списка → threshold=0, is_supported=False — вызывающий обязан - трактовать это как not_covered независимо от n_listings. + MINOR fix (независимый ревью #2894): city_hint — это НЕ проверенный вход, + клиент им управляет напрямую (lat/lon в Серове + city_hint='Екатеринбург' + раньше давал threshold=8 и status='ok' — клиент выбирал себе порог). Источник + истины — мода city найденной SQL-когорты (то, что реально лежит в БД рядом с + переданными lat/lon); city_hint используется ТОЛЬКО как фолбэк, когда когорта + пуста (cohort_city is None) — в этом случае n_listings тоже 0, и caller всё + равно форсирует status="not_covered" независимо от threshold/supported, так + что подмена клиентом порога здесь не даёт эффекта. + Эхо произвольной клиентской строки в поле city убрано: candidate, не нашедший + совпадения в зелёном/жёлтом списке, отдаётся как "" (не supported), а не как + сырой ввод. """ - candidate = (city_hint or cohort_city or "").strip() + candidate = (cohort_city or city_hint or "").strip() match = _COVERAGE_CITY_THRESHOLDS.get(_fold_city(candidate)) if candidate else None if match is not None: display, threshold = match return display, threshold, True - return candidate, 0, False + return "", 0, False @router.post("/coverage", response_model=CoverageProbeResponse) @@ -2613,8 +2630,23 @@ def coverage_probe( в module-level import graph): ST_DWithin 1000м, rooms точное совпадение, area ±15%, scraped_at не старше 14 дней, is_active. + MAJOR-1 fix (независимый ревью #2894): когорта пробы обязана быть + ПОДМНОЖЕСТВОМ когорты платного эстиматора, не шире её — иначе проба честно + отвечает "ok" там, где платный расчёт увидит 0. Три предиката ниже — тот же + канон, что estimator._COMMON_WHERE (app/services/estimator.py:5441/5460) и + inline-копия Tier W (estimator.py:5910/5916/5932, radius-тир, откуда реально + берутся аналоги на 1000 м): guard новостроек, geo_precision != 'city' + (#769 Part E — city-centroid листинги без реального адреса), price_rub > 0. + В ответе НЕТ ни одной цены — см. CoverageProbeResponse docstring. + MAJOR-2 (независимый ревью #2894): days_on_market на проде фактически + заполнена только у ОДНОГО источника (yandex) — это ограничение данных, а + не продуктовое решение. n_with_age в ответе честно считает, по скольким + объявлениям взята медиана; ниже COVERAGE_MIN_AGE_SAMPLES — null (см. поле + в ответе). Значения > COVERAGE_MAX_AGE_DAYS (почти наверняка мёртвое + объявление) в расчёт медианы не берутся. + #oblast (2026-08): house_placement_history.exposure_days — реальная (не цензурированная) экспозиция history-строк — НЕ используется здесь: это house-level архив (join по house_id, не привязан к текущей radius/rooms/ @@ -2648,11 +2680,27 @@ def coverage_probe( AND ST_DWithin( geom::geography, ST_MakePoint(:lon, :lat)::geography, :radius ) + -- MAJOR-1: sync с estimator._COMMON_WHERE (5441) / Tier W (5916) — + AND price_rub > 0 + -- MAJOR-1: sync с estimator._COMMON_WHERE (5460) / Tier W (5932) — + -- guard новостроек, NULL = legacy вторичка до м.011 + AND (listing_segment IS NULL OR listing_segment = 'vtorichka') + -- MAJOR-1: sync с estimator Tier W (5910/5945-5948, #769 Part E) — + -- исключает city-centroid листинги без реального адреса; + -- IS DISTINCT FROM пропускает NULL (неизвестная точность) + AND (geo_precision IS DISTINCT FROM 'city') ) SELECT count(*) AS n_listings, + count(*) FILTER ( + WHERE days_on_market IS NOT NULL + AND days_on_market <= :max_age_days + ) AS n_with_age, percentile_cont(0.5) WITHIN GROUP (ORDER BY days_on_market) - AS median_age_days, + FILTER ( + WHERE days_on_market IS NOT NULL + AND days_on_market <= :max_age_days + ) AS median_age_days, mode() WITHIN GROUP (ORDER BY city) FILTER (WHERE city IS NOT NULL) AS cohort_city FROM base @@ -2669,6 +2717,7 @@ def coverage_probe( "lon": payload.lon, "radius": COVERAGE_RADIUS_M, "max_per_addr": MAX_ANALOGS_PER_ADDRESS, + "max_age_days": COVERAGE_MAX_AGE_DAYS, }, ) .mappings() @@ -2676,9 +2725,12 @@ def coverage_probe( ) n_listings = int(row["n_listings"]) if row else 0 + n_with_age = int(row["n_with_age"]) if row and row["n_with_age"] is not None else 0 median_age = ( round(row["median_age_days"]) - if row is not None and row["median_age_days"] is not None + if row is not None + and row["median_age_days"] is not None + and n_with_age >= COVERAGE_MIN_AGE_SAMPLES else None ) cohort_city = row["cohort_city"] if row else None @@ -2693,18 +2745,20 @@ def coverage_probe( status = "thin" logger.info( - "coverage probe rooms=%d area=%.1f city=%r status=%s n=%d", + "coverage probe rooms=%d area=%.1f city=%r status=%s n=%d n_with_age=%d", payload.rooms, payload.area_m2, city, status, n_listings, + n_with_age, ) return CoverageProbeResponse( status=status, n_listings=n_listings, median_listing_age_days=median_age, + n_with_age=n_with_age, radius_m=COVERAGE_RADIUS_M, city=city, threshold=threshold, diff --git a/tradein-mvp/backend/app/schemas/trade_in.py b/tradein-mvp/backend/app/schemas/trade_in.py index d8fc5c9b..4686b3b5 100644 --- a/tradein-mvp/backend/app/schemas/trade_in.py +++ b/tradein-mvp/backend/app/schemas/trade_in.py @@ -792,6 +792,16 @@ class CoverageProbeResponse(BaseModel): (days_on_market на текущий момент), а НЕ срок до продажи. Цензурированная выборка (активные объявления ещё висят) всегда завышена относительно реального времени экспозиции проданных — не путать со «сроком продажи». + ОГРАНИЧЕНИЕ ДАННЫХ (не продуктовое решение, см. coverage_probe docstring): + days_on_market на проде заполнена практически только у источника yandex — + возраст известен у меньшинства строк когорты. n_with_age ниже — честный + счётчик, по скольким объявлениям посчитана медиана; при n_with_age < порога + (COVERAGE_MIN_AGE_SAMPLES) median_listing_age_days принудительно null. + + n_with_age — сколько объявлений когорты реально имеют известный + (non-null, не-выброс) days_on_market и вошли в расчёт медианы. Фронт + обязан иметь возможность не показывать median_listing_age_days при + маленьком n_with_age — цифра "медиана" по 1-2 объявлениям не медиана. threshold — n, начиная с которого статус переходит в "ok" для резолвленного города; 0, если город не входит ни в один список (порог неприменим — @@ -801,6 +811,7 @@ class CoverageProbeResponse(BaseModel): status: Literal["ok", "thin", "not_covered"] n_listings: int median_listing_age_days: int | None + n_with_age: int radius_m: int city: str threshold: int diff --git a/tradein-mvp/backend/tests/skip_allowlist.txt b/tradein-mvp/backend/tests/skip_allowlist.txt index fe555229..fb780107 100644 --- a/tradein-mvp/backend/tests/skip_allowlist.txt +++ b/tradein-mvp/backend/tests/skip_allowlist.txt @@ -69,3 +69,9 @@ tests/test_2764_ban_kind_no_default.py::test_real_default_ban_kind_survives_the_ tests/test_house_imv_retry_stuck.py::test_explicit_only_status_still_takes_exhausted_houses tests/test_house_imv_retry_stuck.py::test_stuck_transient_house_returns_to_the_queue_by_itself tests/test_house_imv_retry_stuck.py::test_transient_attempts_counter_only_counts_transient + +# MAJOR-1 fix, coverage probe (#2894, независимый ревью) — тот же `_live_session()`. +# Проверяет, что novostroyki-строка / geo_precision='city'-строка / price_rub=0-строка +# физически не попадают в когорту (не только SQL-текст, который проверяется отдельным +# статическим тестом test_cohort_sql_excludes_* в этом же файле, идущим на обоих лэйнах). +tests/test_coverage_probe_endpoint.py::test_major1_cohort_excludes_novostroyki_and_city_precision_live diff --git a/tradein-mvp/backend/tests/test_coverage_probe_endpoint.py b/tradein-mvp/backend/tests/test_coverage_probe_endpoint.py index 42445e28..1b6abdc3 100644 --- a/tradein-mvp/backend/tests/test_coverage_probe_endpoint.py +++ b/tradein-mvp/backend/tests/test_coverage_probe_endpoint.py @@ -6,8 +6,13 @@ - пороги ok/thin/not_covered для зелёных/жёлтых/неподдерживаемых городов - пустая когорта (n=0) → not_covered даже в поддерживаемом городе - в ответе НЕТ ни одного price-подобного поля (падающий тест на регресс схемы) - - city_hint приоритетнее моды city из когорты - - median_listing_age_days — median(days_on_market), None при пустой когорте + - MAJOR-1 (независимый ревью #2894): когорта пробы — sync с + estimator._COMMON_WHERE / Tier W (novostroyki guard, geo_precision != 'city', + price_rub > 0), не шире когорты платного эстиматора + - MAJOR-2: median_listing_age_days честно null при тонкой n_with_age выборке, + выбросы (> COVERAGE_MAX_AGE_DAYS) не тянут медиану + - MINOR: когортный город (мода) побеждает city_hint при расхождении — клиент + не управляет порогом; неизвестный город не эхуется сырой строкой """ from __future__ import annotations @@ -49,6 +54,25 @@ def trade_in_app() -> FastAPI: return application +def _row( + n_listings: int, + median_age_days: float | None, + cohort_city: str | None, + n_with_age: int | None = None, +) -> dict: + """Строка, которую coverage_probe читает через db.execute(...).mappings().fetchone(). + + n_with_age по умолчанию = n_listings, если не задан явно (большинство старых + тестов не проверяют MAJOR-2 отдельно — сохраняем их поведение). + """ + return { + "n_listings": n_listings, + "median_age_days": median_age_days, + "cohort_city": cohort_city, + "n_with_age": n_with_age if n_with_age is not None else n_listings, + } + + def _db_mock_returning(row: dict | None) -> MagicMock: """DB session mock — coverage_probe reads db.execute(...).mappings().fetchone().""" db = MagicMock() @@ -85,9 +109,7 @@ def test_coverage_response_has_no_price_fields(trade_in_app: FastAPI) -> None: def test_coverage_actual_response_has_no_price_fields(trade_in_app: FastAPI) -> None: """Same guard but on a live serialized response (belt-and-suspenders).""" - db = _db_mock_returning( - {"n_listings": 10, "median_age_days": 21.0, "cohort_city": "Екатеринбург"} - ) + db = _db_mock_returning(_row(10, 21.0, "Екатеринбург")) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -103,9 +125,7 @@ def test_coverage_actual_response_has_no_price_fields(trade_in_app: FastAPI) -> def test_green_city_ok_at_threshold(trade_in_app: FastAPI) -> None: """Екатеринбург (зелёный, порог 8) — n=8 ровно на границе → ok.""" - db = _db_mock_returning( - {"n_listings": 8, "median_age_days": 15.0, "cohort_city": "Екатеринбург"} - ) + db = _db_mock_returning(_row(8, 15.0, "Екатеринбург")) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -118,13 +138,12 @@ def test_green_city_ok_at_threshold(trade_in_app: FastAPI) -> None: assert data["city"] == "Екатеринбург" assert data["radius_m"] == 1000 assert data["median_listing_age_days"] == 15 + assert data["n_with_age"] == 8 def test_green_city_thin_below_threshold(trade_in_app: FastAPI) -> None: """Екатеринбург, n=7 (< порог 8) → thin, не ok и не not_covered.""" - db = _db_mock_returning( - {"n_listings": 7, "median_age_days": 10.0, "cohort_city": "Екатеринбург"} - ) + db = _db_mock_returning(_row(7, 10.0, "Екатеринбург")) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -139,10 +158,8 @@ def test_green_city_thin_below_threshold(trade_in_app: FastAPI) -> None: def test_yellow_city_ok_at_threshold(trade_in_app: FastAPI) -> None: - """Нижний Тагил (жёлтый, порог 12) — n=12 → ok.""" - db = _db_mock_returning( - {"n_listings": 12, "median_age_days": 30.0, "cohort_city": "Нижний Тагил"} - ) + """Нижний Тагил (жёлтый, порог 12) — n=12 → ok. cohort_city совпадает с hint.""" + db = _db_mock_returning(_row(12, 30.0, "Нижний Тагил")) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -157,8 +174,8 @@ def test_yellow_city_ok_at_threshold(trade_in_app: FastAPI) -> None: def test_yellow_city_thin_below_threshold(trade_in_app: FastAPI) -> None: - """Ревда, n=11 (< порог 12) → thin.""" - db = _db_mock_returning({"n_listings": 11, "median_age_days": 40.0, "cohort_city": None}) + """Ревда, n=11 (< порог 12) → thin. Когорта пуста по городу → используем hint.""" + db = _db_mock_returning(_row(11, 40.0, None)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -173,7 +190,7 @@ def test_yellow_city_thin_below_threshold(trade_in_app: FastAPI) -> None: def test_unsupported_city_not_covered_even_with_high_n(trade_in_app: FastAPI) -> None: """Город вне списков → not_covered независимо от n_listings (даже n=500).""" - db = _db_mock_returning({"n_listings": 500, "median_age_days": 5.0, "cohort_city": "Серов"}) + db = _db_mock_returning(_row(500, 5.0, "Серов")) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -182,6 +199,7 @@ def test_unsupported_city_not_covered_even_with_high_n(trade_in_app: FastAPI) -> assert data["status"] == "not_covered" assert data["threshold"] == 0 assert data["n_listings"] == 500 # честно отдаём счётчик, статус его игнорирует + assert data["city"] == "" # MINOR: неизвестный город не эхуется сырой строкой # ── Empty cohort ────────────────────────────────────────────────────────────── @@ -189,7 +207,7 @@ def test_unsupported_city_not_covered_even_with_high_n(trade_in_app: FastAPI) -> def test_empty_cohort_supported_city_not_covered(trade_in_app: FastAPI) -> None: """n=0 в поддерживаемом (зелёном) городе → not_covered, не thin — честнее.""" - db = _db_mock_returning({"n_listings": 0, "median_age_days": None, "cohort_city": None}) + db = _db_mock_returning(_row(0, None, None, n_with_age=0)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -200,6 +218,7 @@ def test_empty_cohort_supported_city_not_covered(trade_in_app: FastAPI) -> None: assert data["status"] == "not_covered" assert data["n_listings"] == 0 assert data["median_listing_age_days"] is None + assert data["n_with_age"] == 0 def test_empty_cohort_no_row_at_all(trade_in_app: FastAPI) -> None: @@ -215,14 +234,37 @@ def test_empty_cohort_no_row_at_all(trade_in_app: FastAPI) -> None: assert data["status"] == "not_covered" assert data["n_listings"] == 0 assert data["median_listing_age_days"] is None + assert data["n_with_age"] == 0 -# ── city_hint priority over cohort mode ───────────────────────────────────────── +# ── MINOR: cohort mode (реальные данные из БД) побеждает city_hint ────────────── -def test_city_hint_overrides_cohort_mode(trade_in_app: FastAPI) -> None: - """city_hint (фронт) побеждает cohort_city (SQL mode) при определении города.""" - db = _db_mock_returning({"n_listings": 9, "median_age_days": 12.0, "cohort_city": "Серов"}) +def test_cohort_mode_overrides_city_hint_on_mismatch(trade_in_app: FastAPI) -> None: + """lat/lon в Серове + city_hint='Екатеринбург' — клиент не управляет порогом. + + Когорта реально нашлась в Серове (cohort_city="Серов", город вне списков) — + ответ обязан игнорировать спуфленный hint и не выдавать зелёный threshold=8. + Регресс на прод-инцидент из независимого ревью #2894. + """ + db = _db_mock_returning(_row(9, 12.0, "Серов")) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + resp = client.post( + "/api/v1/trade-in/coverage", + json={**_BASE_PAYLOAD, "city_hint": "Екатеринбург"}, + ) + data = resp.json() + assert data["city"] != "Екатеринбург" + assert data["status"] == "not_covered" # Серов вне зелёного/жёлтого списка + assert data["threshold"] == 0 + + +def test_city_hint_used_only_as_fallback_for_empty_cohort(trade_in_app: FastAPI) -> None: + """Когорта пуста (cohort_city=None) — hint используется как фолбэк для display, + но status всё равно not_covered (n_listings=0), так что подмена без эффекта.""" + db = _db_mock_returning(_row(0, None, None, n_with_age=0)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -232,12 +274,12 @@ def test_city_hint_overrides_cohort_mode(trade_in_app: FastAPI) -> None: ) data = resp.json() assert data["city"] == "Екатеринбург" - assert data["status"] == "ok" # n=9 >= 8 (зелёный порог), не серовский not_covered + assert data["status"] == "not_covered" def test_yo_fold_city_hint_matches(trade_in_app: FastAPI) -> None: """«Березовский» без ё должен резолвиться в тот же зелёный порог, что «Берёзовский».""" - db = _db_mock_returning({"n_listings": 8, "median_age_days": 5.0, "cohort_city": None}) + db = _db_mock_returning(_row(8, 5.0, None, n_with_age=8)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -250,12 +292,64 @@ def test_yo_fold_city_hint_matches(trade_in_app: FastAPI) -> None: assert data["threshold"] == 8 +# ── MAJOR-2: median age — n_with_age threshold + outlier clamp ───────────────── + + +def test_median_age_null_below_min_age_samples(trade_in_app: FastAPI) -> None: + """n_with_age=2 (< COVERAGE_MIN_AGE_SAMPLES=5) → median_listing_age_days null, + даже если SQL посчитал percentile — "медиана" по 1-2 объявлениям не медиана.""" + from app.api.v1.trade_in import COVERAGE_MIN_AGE_SAMPLES + + assert COVERAGE_MIN_AGE_SAMPLES == 5 + db = _db_mock_returning(_row(20, 40.0, "Екатеринбург", n_with_age=2)) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + resp = client.post("/api/v1/trade-in/coverage", json=_BASE_PAYLOAD) + data = resp.json() + assert data["n_listings"] == 20 # когорта покрытия не урезается возрастным фильтром + assert data["n_with_age"] == 2 + assert data["median_listing_age_days"] is None + + +def test_median_age_present_at_min_age_samples_threshold(trade_in_app: FastAPI) -> None: + """n_with_age=5 (== порог) → median_listing_age_days отдаётся.""" + db = _db_mock_returning(_row(20, 40.0, "Екатеринбург", n_with_age=5)) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + resp = client.post("/api/v1/trade-in/coverage", json=_BASE_PAYLOAD) + data = resp.json() + assert data["n_with_age"] == 5 + assert data["median_listing_age_days"] == 40 + + +def test_max_age_outlier_days_passed_to_sql(trade_in_app: FastAPI) -> None: + """COVERAGE_MAX_AGE_DAYS=365 передаётся в SQL как параметр — выбросы (мёртвые + объявления) отсекаются percentile_cont FILTER на стороне БД, не в Python.""" + from app.api.v1.trade_in import COVERAGE_MAX_AGE_DAYS + + assert COVERAGE_MAX_AGE_DAYS == 365 + db = _db_mock_returning(_row(0, None, None, n_with_age=0)) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + client.post("/api/v1/trade-in/coverage", json=_BASE_PAYLOAD) + + call_args = db.execute.call_args + params = call_args[0][1] if len(call_args[0]) > 1 else call_args[1].get("parameters", {}) + assert params["max_age_days"] == 365 + + sql_text = str(call_args[0][0]) + assert "days_on_market <= :max_age_days" in sql_text + + # ── DB dedup / cap params passed through ──────────────────────────────────────── def test_coverage_sql_uses_radius_1000_and_area_tolerance(trade_in_app: FastAPI) -> None: """SQL params: radius=1000 (строго), area ±15%, rooms exact.""" - db = _db_mock_returning({"n_listings": 0, "median_age_days": None, "cohort_city": None}) + db = _db_mock_returning(_row(0, None, None, n_with_age=0)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -269,3 +363,140 @@ def test_coverage_sql_uses_radius_1000_and_area_tolerance(trade_in_app: FastAPI) assert params["area_min"] == pytest.approx(50.0 * 0.85) assert params["area_max"] == pytest.approx(50.0 * 1.15) assert params["fresh_days"] == 14 + + +# ── MAJOR-1: cohort predicates — sync с estimator._COMMON_WHERE / Tier W ──────── +# +# Прямая регрессия из независимого ревью #2894: без этих трёх предикатов проба +# отвечает "ok" в точках, где платный эстиматор (radius Tier W, тот же 1000м) +# реально видит 0 — потому что вся когорта состоит из новостроек / city-centroid +# листингов, которые estimator._COMMON_WHERE / Tier W уже отсекают. Тест ловит +# случайное удаление ЛЮБОГО из трёх предикатов на уровне сгенерированного SQL — +# без живой БД, как и остальные тесты этого файла (см. test_gar_flats_loader.py +# для опционального real-Postgres-варианта аналогичной проверки в этом репо). + + +def test_cohort_sql_excludes_novostroyki(trade_in_app: FastAPI) -> None: + """Guard новостроек — sync с estimator._COMMON_WHERE (5460) / Tier W (5932).""" + db = _db_mock_returning(_row(0, None, None, n_with_age=0)) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + client.post("/api/v1/trade-in/coverage", json=_BASE_PAYLOAD) + + sql_text = str(db.execute.call_args[0][0]) + assert "listing_segment IS NULL OR listing_segment = 'vtorichka'" in sql_text + + +def test_cohort_sql_excludes_city_precision_geocodes(trade_in_app: FastAPI) -> None: + """geo_precision != 'city' — sync с estimator Tier W (5910/5945-5948, #769 Part E).""" + db = _db_mock_returning(_row(0, None, None, n_with_age=0)) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + client.post("/api/v1/trade-in/coverage", json=_BASE_PAYLOAD) + + sql_text = str(db.execute.call_args[0][0]) + assert "geo_precision IS DISTINCT FROM 'city'" in sql_text + + +def test_cohort_sql_excludes_zero_price(trade_in_app: FastAPI) -> None: + """price_rub > 0 — sync с estimator._COMMON_WHERE (5441) / Tier W (5916).""" + db = _db_mock_returning(_row(0, None, None, n_with_age=0)) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + client.post("/api/v1/trade-in/coverage", json=_BASE_PAYLOAD) + + sql_text = str(db.execute.call_args[0][0]) + assert "price_rub > 0" in sql_text + + +# ── MAJOR-1 (real-DB variant): предикаты реально фильтруют, не только в тексте ── +# +# Опциональный тест против настоящего Postgres (тот же паттерн self-skip, что +# test_gar_flats_loader.py::_live_session) — вставляет novostroyki-строку и +# строку с geo_precision='city' в когорту и проверяет, что они физически НЕ +# посчитаны. Требует TEST_DATABASE_URL/DATABASE_URL, указывающий на реальную +# Postgres+PostGIS БД (не дефолтный localhost:5432/test-заглушку) — иначе skip. + + +def _live_session(): # type: ignore[no-untyped-def] + try: + from sqlalchemy import create_engine + from sqlalchemy import text as sa_text + from sqlalchemy.orm import sessionmaker + + dsn = os.environ.get("TEST_DATABASE_URL") or os.environ.get("DATABASE_URL", "") + if not dsn or "localhost:5432/test" in dsn: + return None + engine = create_engine(dsn, future=True) + conn = engine.connect() + conn.execute(sa_text("SELECT 1")) + conn.close() + return sessionmaker(bind=engine, future=True)() + except Exception: + return None + + +# Координаты вне Свердловской обл. (реальные данные там ~56-60/58-64) — изолируют +# тестовую когорту от прод-данных без нужды в COMMIT/rollback гимнастики поверх +# чужой транзакции. +_LIVE_LAT, _LIVE_LON = 1.111, 2.222 + + +@pytest.mark.skipif(_live_session() is None, reason="нет доступной Postgres test-БД") +def test_major1_cohort_excludes_novostroyki_and_city_precision_live() -> None: + from sqlalchemy import text as sa_text + + from app.api.v1.trade_in import coverage_probe + from app.schemas.trade_in import CoverageProbeInput + + db = _live_session() + assert db is not None + try: + rows = [ + # (source_url suffix, listing_segment, geo_precision, price_rub) — все + # остальные поля общие: rooms=2, area_m2=50, is_active, scraped_at=NOW(). + ("ok-vtorichka", None, None, 5_000_000), # counted + ("bad-novostroyka", "novostroyki", None, 5_000_000), # excluded + ("bad-city-precision", None, "city", 5_000_000), # excluded + ("bad-zero-price", None, None, 0), # excluded + ] + for suffix, segment, geo_precision, price in rows: + url = f"https://test.invalid/coverage-major1-{suffix}" + db.execute( + sa_text( + """ + INSERT INTO listings + (source, source_url, source_id, dedup_hash, address, lat, lon, + rooms, area_m2, price_rub, is_active, scraped_at, + listing_segment, geo_precision) + VALUES + ('test', :url, :url, :url, 'test addr', :lat, :lon, + 2, 50.0, :price, true, NOW(), :segment, :geo_precision) + """ + ), + { + "url": url, + "lat": _LIVE_LAT, + "lon": _LIVE_LON, + "price": price, + "segment": segment, + "geo_precision": geo_precision, + }, + ) + + result = coverage_probe( + CoverageProbeInput(lat=_LIVE_LAT, lon=_LIVE_LON, rooms=2, area_m2=50.0), db + ) + # Только первая (ok-vtorichka) строка должна попадать в когорту — + # каждая следующая вставка не должна сдвигать счётчик. + assert result.n_listings == 1, ( + f"predicate regression: n_listings={result.n_listings} after inserting " + f"{suffix!r} (segment={segment!r} geo_precision={geo_precision!r} " + f"price={price}) — expected still 1 (only ok-vtorichka counted)" + ) + finally: + db.rollback() + db.close() -- 2.45.3 From 37e738c802a111866ece7d9afb6c5fd6e831b89d Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 15 Aug 2026 21:20:43 +0300 Subject: [PATCH 3/3] fix(tradein/coverage): resolve city by coordinates, not sweep-context city_hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Повторная проверка /coverage закрыла оба MAJOR из #2894, но выявила три новых дефекта: 1. Город больше не резолвится из моды listings.city найденной когорты — эта колонка хранит город SWEEP-контекста скрейпера (миграция 196), не геокод адреса объявления. Замер на проде: 90/90 строк в радиусе 1000м вокруг Берёзовского имеют city='Екатеринбург', 74/74 вокруг Ревды — city='Первоуральск'. Города-спутники из COVERAGE_GREEN/YELLOW_CITIES были физически недостижимы. Город теперь резолвится детерминированно по lat/lon запроса — ближайший центроид из статичной константы (8 городов, рядом с ручкой, не в БД — comment объясняет почему) в пределах 25 км. city_hint остаётся в схеме (фронт его шлёт для соседних ручек), но чисто информационный — на порог/статус не влияет. 2. test_max_age_outlier_days_passed_to_sql проверял подстроку, которая встречается в SQL дважды (count и percentile_cont) — мутация «убрать FILTER у percentile_cont, оставив у count» проходила зелёной. Добавлен живой поведенческий тест (вставляет когорту + выброс days_on_market=4000, проверяет что медиана не сдвигается) — ловит эту мутацию (подтверждено: median 8→9 при мутации). 3. _live_session() вызывался в pytest.mark.skipif на этапе сбора тестов и создавал никогда не закрываемый Session, плюс дублировался в теле теста. Заменено на _live_db_available() (open+close голого connection) для skipif и pytest-фикстуру live_session с гарантированным close/dispose. 4. Nit: пустая когорта в поддерживаемом городе отдавала status=not_covered вместе с ненулевым threshold — противоречило докстрингу CoverageProbeResponse.threshold ("0, когда порог неприменим"). threshold теперь всегда 0 при not_covered, независимо от причины. --- tradein-mvp/backend/app/api/v1/trade_in.py | 104 +++- tradein-mvp/backend/app/schemas/trade_in.py | 25 +- tradein-mvp/backend/tests/skip_allowlist.txt | 10 +- .../tests/test_coverage_probe_endpoint.py | 446 ++++++++++++------ 4 files changed, 408 insertions(+), 177 deletions(-) diff --git a/tradein-mvp/backend/app/api/v1/trade_in.py b/tradein-mvp/backend/app/api/v1/trade_in.py index 71402948..2e4b391e 100644 --- a/tradein-mvp/backend/app/api/v1/trade_in.py +++ b/tradein-mvp/backend/app/api/v1/trade_in.py @@ -9,6 +9,7 @@ import asyncio import calendar import json import logging +import math from datetime import UTC, date, datetime, timedelta from typing import Annotated, Any, Literal from uuid import UUID @@ -2592,28 +2593,72 @@ _COVERAGE_CITY_THRESHOLDS: dict[str, tuple[str, int]] = { **{_fold_city(c): (c, COVERAGE_YELLOW_MIN_N) for c in COVERAGE_YELLOW_CITIES}, } +# Повторная проверка ручки #2894 (2026-08): город раньше резолвился модой +# `listings.city` найденной когорты — оказалось, что `listings.city` это город +# СВИП-контекста скрейпера (миграция 196 — колонка заполняется тем городом, +# который скрейпер обходил, не геокодом самого объявления). Замер на проде: +# в радиусе 1000 м вокруг Берёзовского 90/90 строк имеют city='Екатеринбург'; +# вокруг Ревды 74/74 — city='Первоуральск'. Следствие: продавец в Берёзовском +# видел на лэндинге «Екатеринбург», а сами COVERAGE_GREEN/YELLOW_CITIES для +# городов-спутников были НЕДОСТИЖИМЫ (в БД нет ни одной строки с их city). +# Фикс — детерминированный резолв по координатам ЗАПРОСА (никакого участия +# клиента, никакой моды когорты): ближайший центроид города из списка ниже, +# если он в пределах COVERAGE_CITY_MATCH_RADIUS_KM. +# +# Координаты — константа РЯДОМ С РУЧКОЙ, не таблица в БД: единственный +# существующий кандидат на "готовый реестр городов" — это +# frontend/src/lib/city-registry.ts (OBLAST_CITIES) и backend +# geocoder.py::SVERDLOVSK_OBLAST_CITIES — оба хранят ТОЛЬКО текстовые лейблы +# (city_hint для геокодера), без координат. Заводить миграцию + таблицу ради +# статичного справочника из 8 географических центров населённых пунктов — +# оверинжиниринг; координаты (WGS84, общедоступные центры НП) живут здесь же, +# рядом с порогами, которые они резолвят. +COVERAGE_CITY_MATCH_RADIUS_KM = 25.0 # дальше — город не определён (not_covered) -def _resolve_coverage_city(city_hint: str | None, cohort_city: str | None) -> tuple[str, int, bool]: - """Резолвит (display_city, threshold, is_supported) для пробы покрытия. +_CITY_CENTROIDS_DEG: dict[str, tuple[float, float]] = { + "Екатеринбург": (56.8389, 60.6057), + "Верхняя Пышма": (56.9789, 60.5636), + "Берёзовский": (56.9096, 60.8034), + "Среднеуральск": (56.9848, 60.4759), + "Нижний Тагил": (57.9099, 59.9819), + "Каменск-Уральский": (56.4110, 61.9243), + "Первоуральск": (56.9083, 59.9483), + "Ревда": (56.7986, 59.9298), +} - MINOR fix (независимый ревью #2894): city_hint — это НЕ проверенный вход, - клиент им управляет напрямую (lat/lon в Серове + city_hint='Екатеринбург' - раньше давал threshold=8 и status='ok' — клиент выбирал себе порог). Источник - истины — мода city найденной SQL-когорты (то, что реально лежит в БД рядом с - переданными lat/lon); city_hint используется ТОЛЬКО как фолбэк, когда когорта - пуста (cohort_city is None) — в этом случае n_listings тоже 0, и caller всё - равно форсирует status="not_covered" независимо от threshold/supported, так - что подмена клиентом порога здесь не даёт эффекта. - Эхо произвольной клиентской строки в поле city убрано: candidate, не нашедший - совпадения в зелёном/жёлтом списке, отдаётся как "" (не supported), а не как - сырой ввод. + +def _haversine_km(lat1: float, lon1: float, lat2: float, lon2: float) -> float: + """Расстояние по большому кругу (км), радиус Земли 6371 км.""" + r_earth_km = 6371.0 + phi1, phi2 = math.radians(lat1), math.radians(lat2) + dphi = math.radians(lat2 - lat1) + dlambda = math.radians(lon2 - lon1) + a = math.sin(dphi / 2) ** 2 + math.cos(phi1) * math.cos(phi2) * math.sin(dlambda / 2) ** 2 + return 2 * r_earth_km * math.asin(math.sqrt(a)) + + +def _resolve_coverage_city(lat: float, lon: float) -> tuple[str, int, bool]: + """Резолвит (display_city, threshold, is_supported) для пробы покрытия — ПО КООРДИНАТАМ. + + Город = ближайший центроид из `_CITY_CENTROIDS_DEG`, если расстояние до него + < `COVERAGE_CITY_MATCH_RADIUS_KM`; иначе город не определён. Детерминированно + и без участия клиента — см. комментарий над `_CITY_CENTROIDS_DEG` про то, + почему `listings.city` (мода когорты) и `city_hint` (клиентский вход) сюда + больше НЕ допускаются в качестве источника истины. """ - candidate = (cohort_city or city_hint or "").strip() - match = _COVERAGE_CITY_THRESHOLDS.get(_fold_city(candidate)) if candidate else None - if match is not None: - display, threshold = match - return display, threshold, True - return "", 0, False + nearest_city: str | None = None + nearest_km = math.inf + for city, (clat, clon) in _CITY_CENTROIDS_DEG.items(): + distance_km = _haversine_km(lat, lon, clat, clon) + if distance_km < nearest_km: + nearest_km = distance_km + nearest_city = city + + if nearest_city is None or nearest_km > COVERAGE_CITY_MATCH_RADIUS_KM: + return "", 0, False + + display, threshold = _COVERAGE_CITY_THRESHOLDS[_fold_city(nearest_city)] + return display, threshold, True @router.post("/coverage", response_model=CoverageProbeResponse) @@ -2654,6 +2699,14 @@ def coverage_probe( сведение двух разных когорт усложнило бы «один дешёвый SQL» без выигрыша в честности (у нас и так честное имя поля — age активного объявления, не срок продажи). См. openQuestions PR #2894 при ревью. + + Повторная проверка ручки (2026-08): город больше НЕ берётся из моды + `listings.city` найденной когорты и НЕ зависит от `payload.city_hint` — + оба источника ненадёжны (см. комментарий над `_CITY_CENTROIDS_DEG`). + Город резолвится детерминированно по `payload.lat/lon` через + `_resolve_coverage_city` — `city_hint` в payload остаётся только + информационным полем (см. `CoverageProbeInput.city_hint`), на результат + не влияет. """ from app.services.estimator import _RN_DUP_WINDOW, MAX_ANALOGS_PER_ADDRESS @@ -2666,7 +2719,6 @@ def coverage_probe( f""" WITH base AS ( SELECT - city, days_on_market, row_number() OVER ( PARTITION BY address ORDER BY scraped_at DESC @@ -2700,9 +2752,7 @@ def coverage_probe( FILTER ( WHERE days_on_market IS NOT NULL AND days_on_market <= :max_age_days - ) AS median_age_days, - mode() WITHIN GROUP (ORDER BY city) - FILTER (WHERE city IS NOT NULL) AS cohort_city + ) AS median_age_days FROM base WHERE rn_addr <= :max_per_addr AND rn_dup = 1 @@ -2733,12 +2783,16 @@ def coverage_probe( and n_with_age >= COVERAGE_MIN_AGE_SAMPLES else None ) - cohort_city = row["cohort_city"] if row else None - city, threshold, supported = _resolve_coverage_city(payload.city_hint, cohort_city) + city, threshold, supported = _resolve_coverage_city(payload.lat, payload.lon) if not supported or n_listings == 0: status: Literal["ok", "thin", "not_covered"] = "not_covered" + # Nit-fix (повторная проверка #2894): threshold неприменим при + # not_covered — см. CoverageProbeResponse.threshold docstring. Раньше + # поддерживаемый (по координатам) город с пустой когортой отдавал + # реальный порог (8/12) вместе с not_covered — противоречило докстрингу. + threshold = 0 elif n_listings >= threshold: status = "ok" else: diff --git a/tradein-mvp/backend/app/schemas/trade_in.py b/tradein-mvp/backend/app/schemas/trade_in.py index 4686b3b5..8f245bb0 100644 --- a/tradein-mvp/backend/app/schemas/trade_in.py +++ b/tradein-mvp/backend/app/schemas/trade_in.py @@ -763,9 +763,18 @@ class CoverageProbeInput(BaseModel): lat/lon — координаты, уже разрезолвленные фронтом (тот же контракт, что TradeInEstimateInput.lat/lon — geocode делает фронт/автокомплит, эта ручка - сама НИКОГО не геокодирует). city_hint — опционально, из того же - автокомплита (см. TradeInEstimateInput.city_hint); без него город - резолвится best-effort из моды city найденной когорты. + сама НИКОГО не геокодирует). Город (и, соответственно, порог ok/thin) для + ответа резолвится ИСКЛЮЧИТЕЛЬНО из lat/lon — см. + `app.api.v1.trade_in._resolve_coverage_city`. + + city_hint — ИНФОРМАЦИОННОЕ поле, на результат НЕ влияет (повторная проверка + #2894, 2026-08). Раньше оно участвовало в резолве города как фолбэк — + убрано вместе с модой `listings.city`: оба источника ненадёжны (`city_hint` + — непроверенный клиентский вход, `listings.city` — город свип-контекста + скрейпера, не адреса объявления, см. комментарий в trade_in.py). Поле + оставлено в схеме, потому что фронт его уже шлёт в других ручках того же + автокомплита (см. TradeInEstimateInput.city_hint) — принимаем и молча + игнорируем, чтобы не ронять запрос лишней 422. """ lat: float = Field(ge=-90, le=90) @@ -804,8 +813,14 @@ class CoverageProbeResponse(BaseModel): маленьком n_with_age — цифра "медиана" по 1-2 объявлениям не медиана. threshold — n, начиная с которого статус переходит в "ok" для резолвленного - города; 0, если город не входит ни в один список (порог неприменим — - статус в этом случае всегда "not_covered" вне зависимости от n_listings). + города; 0 всегда, когда status == "not_covered" (порог неприменим — ни для + города вне зелёного/жёлтого списка, ни для поддерживаемого города с пустой + когортой), НЕ только для неподдерживаемого города. + + city — резолвится ИСКЛЮЧИТЕЛЬНО из lat/lon запроса (ближайший центроид из + зелёного/жёлтого списка в пределах `COVERAGE_CITY_MATCH_RADIUS_KM`), не из + `city_hint` и не из моды `listings.city` найденной когорты — см. + `app.api.v1.trade_in._resolve_coverage_city`. """ status: Literal["ok", "thin", "not_covered"] diff --git a/tradein-mvp/backend/tests/skip_allowlist.txt b/tradein-mvp/backend/tests/skip_allowlist.txt index fb780107..d97627f7 100644 --- a/tradein-mvp/backend/tests/skip_allowlist.txt +++ b/tradein-mvp/backend/tests/skip_allowlist.txt @@ -70,8 +70,16 @@ tests/test_house_imv_retry_stuck.py::test_explicit_only_status_still_takes_exhau tests/test_house_imv_retry_stuck.py::test_stuck_transient_house_returns_to_the_queue_by_itself tests/test_house_imv_retry_stuck.py::test_transient_attempts_counter_only_counts_transient -# MAJOR-1 fix, coverage probe (#2894, независимый ревью) — тот же `_live_session()`. +# MAJOR-1 fix, coverage probe (#2894, независимый ревью) — тот же `live_session` fixture +# (self-skip через `_live_db_available()`, живёт только при реальном Postgres DSN). # Проверяет, что novostroyki-строка / geo_precision='city'-строка / price_rub=0-строка # физически не попадают в когорту (не только SQL-текст, который проверяется отдельным # статическим тестом test_cohort_sql_excludes_* в этом же файле, идущим на обоих лэйнах). tests/test_coverage_probe_endpoint.py::test_major1_cohort_excludes_novostroyki_and_city_precision_live + +# MAJOR-2 поведенческий пин (повторная проверка #2894) — та же `live_session` fixture. +# Ловит мутацию «убрать FILTER у percentile_cont, оставив у count(*)», которую +# текстовый тест test_max_age_outlier_days_passed_to_sql пропускал (подстрока +# `days_on_market <= :max_age_days` встречается в SQL дважды). На мок-лэйне +# (deploy-tradein.yml, DSN-заглушка) реальной БД нет — self-skip. +tests/test_coverage_probe_endpoint.py::test_max_age_outlier_excluded_from_median_live diff --git a/tradein-mvp/backend/tests/test_coverage_probe_endpoint.py b/tradein-mvp/backend/tests/test_coverage_probe_endpoint.py index 1b6abdc3..65d85921 100644 --- a/tradein-mvp/backend/tests/test_coverage_probe_endpoint.py +++ b/tradein-mvp/backend/tests/test_coverage_probe_endpoint.py @@ -4,15 +4,19 @@ видит, сколько похожих квартир продаётся рядом и как быстро они уходят, без единой рублёвой цифры в ответе. Covers: - пороги ok/thin/not_covered для зелёных/жёлтых/неподдерживаемых городов - - пустая когорта (n=0) → not_covered даже в поддерживаемом городе + - пустая когорта (n=0) → not_covered даже в поддерживаемом городе; threshold + принудительно 0 в этом случае (nit-fix, повторная проверка #2894) - в ответе НЕТ ни одного price-подобного поля (падающий тест на регресс схемы) - MAJOR-1 (независимый ревью #2894): когорта пробы — sync с estimator._COMMON_WHERE / Tier W (novostroyki guard, geo_precision != 'city', price_rub > 0), не шире когорты платного эстиматора - MAJOR-2: median_listing_age_days честно null при тонкой n_with_age выборке, - выбросы (> COVERAGE_MAX_AGE_DAYS) не тянут медиану - - MINOR: когортный город (мода) побеждает city_hint при расхождении — клиент - не управляет порогом; неизвестный город не эхуется сырой строкой + выбросы (> COVERAGE_MAX_AGE_DAYS) не тянут медиану — запинено ЖИВЫМ SQL + (см. test_max_age_outlier_excluded_from_median_live), не только подстрокой + - Повторная проверка #2894 (2026-08): город резолвится ИСКЛЮЧИТЕЛЬНО по + lat/lon (ближайший центроид), НЕ по моде `listings.city` (город + свип-контекста скрейпера, не адреса объявления) и НЕ по `city_hint` + (непроверенный клиентский вход) — см. app.api.v1.trade_in._resolve_coverage_city """ from __future__ import annotations @@ -57,18 +61,19 @@ def trade_in_app() -> FastAPI: def _row( n_listings: int, median_age_days: float | None, - cohort_city: str | None, n_with_age: int | None = None, ) -> dict: """Строка, которую coverage_probe читает через db.execute(...).mappings().fetchone(). n_with_age по умолчанию = n_listings, если не задан явно (большинство старых тестов не проверяют MAJOR-2 отдельно — сохраняем их поведение). + + Повторная проверка #2894: строка больше не несёт cohort_city — город + резолвится по lat/lon запроса, не по SQL-агрегату (см. модуль-докстринг). """ return { "n_listings": n_listings, "median_age_days": median_age_days, - "cohort_city": cohort_city, "n_with_age": n_with_age if n_with_age is not None else n_listings, } @@ -90,8 +95,22 @@ def _override(app: FastAPI, db: MagicMock) -> None: app.dependency_overrides[get_db] = lambda: (yield db) +# Екатеринбург — совпадает (с точностью до сотен метров) с центроидом +# _CITY_CENTROIDS_DEG["Екатеринбург"], поэтому дефолтный payload детерминированно +# резолвится в зелёный город без доп. настройки координат в каждом тесте. _BASE_PAYLOAD = {"lat": 56.8384, "lon": 60.6057, "rooms": 2, "area_m2": 50.0} +# Координаты других городов из COVERAGE_GREEN/YELLOW_CITIES (те же значения, что +# _CITY_CENTROIDS_DEG в trade_in.py) — используются, когда тесту нужен НЕ ЕКБ. +_NIZHNY_TAGIL = {"lat": 57.9099, "lon": 59.9819} +_REVDA = {"lat": 56.7986, "lon": 59.9298} +_BEREZOVSKY = {"lat": 56.9096, "lon": 60.8034} + +# Реальные координаты Серова — ближайший поддерживаемый центроид (Нижний Тагил) +# в ~190 км, далеко за пределами COVERAGE_CITY_MATCH_RADIUS_KM=25 — гарантированно +# "город не определён", без совпадения ни с одним из 8 центроидов. +_FAR_AWAY_CITY = {"lat": 59.6047, "lon": 60.1970} + # ── Response schema: NO price anywhere (issue #2894 hard rule) ──────────────── @@ -109,7 +128,7 @@ def test_coverage_response_has_no_price_fields(trade_in_app: FastAPI) -> None: def test_coverage_actual_response_has_no_price_fields(trade_in_app: FastAPI) -> None: """Same guard but on a live serialized response (belt-and-suspenders).""" - db = _db_mock_returning(_row(10, 21.0, "Екатеринбург")) + db = _db_mock_returning(_row(10, 21.0)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -125,7 +144,7 @@ def test_coverage_actual_response_has_no_price_fields(trade_in_app: FastAPI) -> def test_green_city_ok_at_threshold(trade_in_app: FastAPI) -> None: """Екатеринбург (зелёный, порог 8) — n=8 ровно на границе → ok.""" - db = _db_mock_returning(_row(8, 15.0, "Екатеринбург")) + db = _db_mock_returning(_row(8, 15.0)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -143,7 +162,7 @@ def test_green_city_ok_at_threshold(trade_in_app: FastAPI) -> None: def test_green_city_thin_below_threshold(trade_in_app: FastAPI) -> None: """Екатеринбург, n=7 (< порог 8) → thin, не ok и не not_covered.""" - db = _db_mock_returning(_row(7, 10.0, "Екатеринбург")) + db = _db_mock_returning(_row(7, 10.0)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -158,15 +177,12 @@ def test_green_city_thin_below_threshold(trade_in_app: FastAPI) -> None: def test_yellow_city_ok_at_threshold(trade_in_app: FastAPI) -> None: - """Нижний Тагил (жёлтый, порог 12) — n=12 → ok. cohort_city совпадает с hint.""" - db = _db_mock_returning(_row(12, 30.0, "Нижний Тагил")) + """Нижний Тагил (жёлтый, порог 12) — n=12 → ok. Город резолвится из lat/lon.""" + db = _db_mock_returning(_row(12, 30.0)) _override(trade_in_app, db) client = TestClient(trade_in_app) - resp = client.post( - "/api/v1/trade-in/coverage", - json={**_BASE_PAYLOAD, "city_hint": "Нижний Тагил"}, - ) + resp = client.post("/api/v1/trade-in/coverage", json={**_BASE_PAYLOAD, **_NIZHNY_TAGIL}) data = resp.json() assert data["status"] == "ok" assert data["threshold"] == 12 @@ -174,51 +190,57 @@ def test_yellow_city_ok_at_threshold(trade_in_app: FastAPI) -> None: def test_yellow_city_thin_below_threshold(trade_in_app: FastAPI) -> None: - """Ревда, n=11 (< порог 12) → thin. Когорта пуста по городу → используем hint.""" - db = _db_mock_returning(_row(11, 40.0, None)) + """Ревда, n=11 (< порог 12) → thin.""" + db = _db_mock_returning(_row(11, 40.0)) _override(trade_in_app, db) client = TestClient(trade_in_app) - resp = client.post("/api/v1/trade-in/coverage", json={**_BASE_PAYLOAD, "city_hint": "Ревда"}) + resp = client.post("/api/v1/trade-in/coverage", json={**_BASE_PAYLOAD, **_REVDA}) data = resp.json() assert data["status"] == "thin" assert data["threshold"] == 12 -# ── City outside both lists → always not_covered ──────────────────────────────── +# ── City outside all centroids → always not_covered ───────────────────────────── def test_unsupported_city_not_covered_even_with_high_n(trade_in_app: FastAPI) -> None: - """Город вне списков → not_covered независимо от n_listings (даже n=500).""" - db = _db_mock_returning(_row(500, 5.0, "Серов")) + """Точка вне 25-км радиуса всех центроидов → not_covered независимо от n_listings + (даже n=500).""" + db = _db_mock_returning(_row(500, 5.0)) _override(trade_in_app, db) client = TestClient(trade_in_app) - resp = client.post("/api/v1/trade-in/coverage", json={**_BASE_PAYLOAD, "city_hint": "Серов"}) + resp = client.post("/api/v1/trade-in/coverage", json={**_BASE_PAYLOAD, **_FAR_AWAY_CITY}) data = resp.json() assert data["status"] == "not_covered" assert data["threshold"] == 0 assert data["n_listings"] == 500 # честно отдаём счётчик, статус его игнорирует - assert data["city"] == "" # MINOR: неизвестный город не эхуется сырой строкой + assert data["city"] == "" # город не определён — не эхуется сырой строкой # ── Empty cohort ────────────────────────────────────────────────────────────── def test_empty_cohort_supported_city_not_covered(trade_in_app: FastAPI) -> None: - """n=0 в поддерживаемом (зелёном) городе → not_covered, не thin — честнее.""" - db = _db_mock_returning(_row(0, None, None, n_with_age=0)) + """n=0 в поддерживаемом (зелёном) городе → not_covered, не thin — честнее. + + Nit-fix (повторная проверка #2894): threshold обязан быть 0, а не реальным + порогом города (8) — при not_covered threshold "неприменим" по докстрингу + CoverageProbeResponse, независимо от ПРИЧИНЫ not_covered. + """ + db = _db_mock_returning(_row(0, None, n_with_age=0)) _override(trade_in_app, db) client = TestClient(trade_in_app) - resp = client.post( - "/api/v1/trade-in/coverage", json={**_BASE_PAYLOAD, "city_hint": "Екатеринбург"} - ) + resp = client.post("/api/v1/trade-in/coverage", json=_BASE_PAYLOAD) data = resp.json() assert data["status"] == "not_covered" assert data["n_listings"] == 0 assert data["median_listing_age_days"] is None assert data["n_with_age"] == 0 + assert data["city"] == "Екатеринбург" # город резолвится по координатам всегда + assert data["threshold"] == 0 # nit: не 8, хотя город поддерживаемый def test_empty_cohort_no_row_at_all(trade_in_app: FastAPI) -> None: @@ -235,63 +257,70 @@ def test_empty_cohort_no_row_at_all(trade_in_app: FastAPI) -> None: assert data["n_listings"] == 0 assert data["median_listing_age_days"] is None assert data["n_with_age"] == 0 - - -# ── MINOR: cohort mode (реальные данные из БД) побеждает city_hint ────────────── - - -def test_cohort_mode_overrides_city_hint_on_mismatch(trade_in_app: FastAPI) -> None: - """lat/lon в Серове + city_hint='Екатеринбург' — клиент не управляет порогом. - - Когорта реально нашлась в Серове (cohort_city="Серов", город вне списков) — - ответ обязан игнорировать спуфленный hint и не выдавать зелёный threshold=8. - Регресс на прод-инцидент из независимого ревью #2894. - """ - db = _db_mock_returning(_row(9, 12.0, "Серов")) - _override(trade_in_app, db) - - client = TestClient(trade_in_app) - resp = client.post( - "/api/v1/trade-in/coverage", - json={**_BASE_PAYLOAD, "city_hint": "Екатеринбург"}, - ) - data = resp.json() - assert data["city"] != "Екатеринбург" - assert data["status"] == "not_covered" # Серов вне зелёного/жёлтого списка assert data["threshold"] == 0 -def test_city_hint_used_only_as_fallback_for_empty_cohort(trade_in_app: FastAPI) -> None: - """Когорта пуста (cohort_city=None) — hint используется как фолбэк для display, - но status всё равно not_covered (n_listings=0), так что подмена без эффекта.""" - db = _db_mock_returning(_row(0, None, None, n_with_age=0)) +# ── Город резолвится ТОЛЬКО по координатам — не по listings.city, не по city_hint ── + + +def test_city_resolved_from_coordinates_not_cohort_mode(trade_in_app: FastAPI) -> None: + """Точка в Берёзовском → city='Берёзовский' (а не 'Екатеринбург'). + + Регресс на прод-замер (повторная проверка #2894): в радиусе 1000м вокруг + Берёзовского 90/90 строк listings имеют city='Екатеринбург' (город + свип-контекста скрейпера, миграция 196) — старая логика (мода когорты) + отдала бы 'Екатеринбург'. Ручка больше НЕ читает cohort city из SQL вовсе. + """ + db = _db_mock_returning(_row(8, 5.0)) _override(trade_in_app, db) client = TestClient(trade_in_app) - resp = client.post( - "/api/v1/trade-in/coverage", - json={**_BASE_PAYLOAD, "city_hint": "Екатеринбург"}, - ) - data = resp.json() - assert data["city"] == "Екатеринбург" - assert data["status"] == "not_covered" - - -def test_yo_fold_city_hint_matches(trade_in_app: FastAPI) -> None: - """«Березовский» без ё должен резолвиться в тот же зелёный порог, что «Берёзовский».""" - db = _db_mock_returning(_row(8, 5.0, None, n_with_age=8)) - _override(trade_in_app, db) - - client = TestClient(trade_in_app) - resp = client.post( - "/api/v1/trade-in/coverage", - json={**_BASE_PAYLOAD, "city_hint": "березовский"}, - ) + resp = client.post("/api/v1/trade-in/coverage", json={**_BASE_PAYLOAD, **_BEREZOVSKY}) data = resp.json() + assert data["city"] == "Берёзовский" assert data["status"] == "ok" assert data["threshold"] == 8 +def test_far_from_all_centroids_not_covered(trade_in_app: FastAPI) -> None: + """Точка за пределами 25 км от всех 8 центроидов → not_covered, city="".""" + db = _db_mock_returning(_row(0, None, n_with_age=0)) + _override(trade_in_app, db) + + client = TestClient(trade_in_app) + resp = client.post("/api/v1/trade-in/coverage", json={**_BASE_PAYLOAD, **_FAR_AWAY_CITY}) + data = resp.json() + assert data["status"] == "not_covered" + assert data["city"] == "" + assert data["threshold"] == 0 + + +def test_city_hint_does_not_change_threshold_or_status(trade_in_app: FastAPI) -> None: + """city_hint — чисто информационное поле (повторная проверка #2894): точка в + Берёзовском + city_hint='Екатеринбург' обязана резолвиться в Берёзовский + (threshold=8, зелёный порог — оба города зелёные, поэтому дополнительно + проверяем n=8 → ok именно для Берёзовского, а не подмену клиентом города). + """ + db_with_hint = _db_mock_returning(_row(8, 5.0)) + _override(trade_in_app, db_with_hint) + client = TestClient(trade_in_app) + resp_with_hint = client.post( + "/api/v1/trade-in/coverage", + json={**_BASE_PAYLOAD, **_BEREZOVSKY, "city_hint": "Екатеринбург"}, + ) + + db_without_hint = _db_mock_returning(_row(8, 5.0)) + _override(trade_in_app, db_without_hint) + resp_without_hint = client.post( + "/api/v1/trade-in/coverage", json={**_BASE_PAYLOAD, **_BEREZOVSKY} + ) + + data_with, data_without = resp_with_hint.json(), resp_without_hint.json() + assert data_with["city"] == data_without["city"] == "Берёзовский" + assert data_with["threshold"] == data_without["threshold"] == 8 + assert data_with["status"] == data_without["status"] == "ok" + + # ── MAJOR-2: median age — n_with_age threshold + outlier clamp ───────────────── @@ -301,7 +330,7 @@ def test_median_age_null_below_min_age_samples(trade_in_app: FastAPI) -> None: from app.api.v1.trade_in import COVERAGE_MIN_AGE_SAMPLES assert COVERAGE_MIN_AGE_SAMPLES == 5 - db = _db_mock_returning(_row(20, 40.0, "Екатеринбург", n_with_age=2)) + db = _db_mock_returning(_row(20, 40.0, n_with_age=2)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -314,7 +343,7 @@ def test_median_age_null_below_min_age_samples(trade_in_app: FastAPI) -> None: def test_median_age_present_at_min_age_samples_threshold(trade_in_app: FastAPI) -> None: """n_with_age=5 (== порог) → median_listing_age_days отдаётся.""" - db = _db_mock_returning(_row(20, 40.0, "Екатеринбург", n_with_age=5)) + db = _db_mock_returning(_row(20, 40.0, n_with_age=5)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -326,11 +355,17 @@ def test_median_age_present_at_min_age_samples_threshold(trade_in_app: FastAPI) def test_max_age_outlier_days_passed_to_sql(trade_in_app: FastAPI) -> None: """COVERAGE_MAX_AGE_DAYS=365 передаётся в SQL как параметр — выбросы (мёртвые - объявления) отсекаются percentile_cont FILTER на стороне БД, не в Python.""" + объявления) отсекаются percentile_cont FILTER на стороне БД, не в Python. + + Слабая (текстовая) проверка — подстрока встречается в SQL ДВАЖДЫ (count и + percentile_cont), поэтому `assert "..." in sql_text` одна ловит только + "убрали оба FILTER", не "убрали один из двух". Реальный поведенческий пин — + test_max_age_outlier_excluded_from_median_live ниже (живой Postgres). + """ from app.api.v1.trade_in import COVERAGE_MAX_AGE_DAYS assert COVERAGE_MAX_AGE_DAYS == 365 - db = _db_mock_returning(_row(0, None, None, n_with_age=0)) + db = _db_mock_returning(_row(0, None, n_with_age=0)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -341,7 +376,10 @@ def test_max_age_outlier_days_passed_to_sql(trade_in_app: FastAPI) -> None: assert params["max_age_days"] == 365 sql_text = str(call_args[0][0]) - assert "days_on_market <= :max_age_days" in sql_text + # count==2: и в count(*) FILTER, и в percentile_cont(...) FILTER — обе нужны, + # чтобы n_with_age и median_listing_age_days считались по ОДНОМУ и тому же + # предикату (иначе честный n_with_age маскирует нечестный медианный расчёт). + assert sql_text.count("days_on_market <= :max_age_days") == 2 # ── DB dedup / cap params passed through ──────────────────────────────────────── @@ -349,7 +387,7 @@ def test_max_age_outlier_days_passed_to_sql(trade_in_app: FastAPI) -> None: def test_coverage_sql_uses_radius_1000_and_area_tolerance(trade_in_app: FastAPI) -> None: """SQL params: radius=1000 (строго), area ±15%, rooms exact.""" - db = _db_mock_returning(_row(0, None, None, n_with_age=0)) + db = _db_mock_returning(_row(0, None, n_with_age=0)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -378,7 +416,7 @@ def test_coverage_sql_uses_radius_1000_and_area_tolerance(trade_in_app: FastAPI) def test_cohort_sql_excludes_novostroyki(trade_in_app: FastAPI) -> None: """Guard новостроек — sync с estimator._COMMON_WHERE (5460) / Tier W (5932).""" - db = _db_mock_returning(_row(0, None, None, n_with_age=0)) + db = _db_mock_returning(_row(0, None, n_with_age=0)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -390,7 +428,7 @@ def test_cohort_sql_excludes_novostroyki(trade_in_app: FastAPI) -> None: def test_cohort_sql_excludes_city_precision_geocodes(trade_in_app: FastAPI) -> None: """geo_precision != 'city' — sync с estimator Tier W (5910/5945-5948, #769 Part E).""" - db = _db_mock_returning(_row(0, None, None, n_with_age=0)) + db = _db_mock_returning(_row(0, None, n_with_age=0)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -402,7 +440,7 @@ def test_cohort_sql_excludes_city_precision_geocodes(trade_in_app: FastAPI) -> N def test_cohort_sql_excludes_zero_price(trade_in_app: FastAPI) -> None: """price_rub > 0 — sync с estimator._COMMON_WHERE (5441) / Tier W (5916).""" - db = _db_mock_returning(_row(0, None, None, n_with_age=0)) + db = _db_mock_returning(_row(0, None, n_with_age=0)) _override(trade_in_app, db) client = TestClient(trade_in_app) @@ -412,31 +450,66 @@ def test_cohort_sql_excludes_zero_price(trade_in_app: FastAPI) -> None: assert "price_rub > 0" in sql_text -# ── MAJOR-1 (real-DB variant): предикаты реально фильтруют, не только в тексте ── +# ── Live-DB tests (self-skip без реальной Postgres+PostGIS) ──────────────────── # -# Опциональный тест против настоящего Postgres (тот же паттерн self-skip, что -# test_gar_flats_loader.py::_live_session) — вставляет novostroyki-строку и -# строку с geo_precision='city' в когорту и проверяет, что они физически НЕ -# посчитаны. Требует TEST_DATABASE_URL/DATABASE_URL, указывающий на реальную -# Postgres+PostGIS БД (не дефолтный localhost:5432/test-заглушку) — иначе skip. +# Опциональные тесты против настоящего Postgres (тот же паттерн self-skip, что +# test_gar_flats_loader.py::_live_session) — требуют TEST_DATABASE_URL/ +# DATABASE_URL, указывающий на реальную БД (не дефолтный localhost:5432/test- +# заглушку); иначе skip. В CI (ci-tradein.yml) этот DSN всегда живой Postgres+ +# PostGIS-контейнер. +# +# Fix (повторная проверка #2894): раньше `_live_session()` вызывался И в +# `pytest.mark.skipif(...)` (на этапе СБОРА тестов — соединение открывалось и +# никогда не закрывалось, при реальном DSN это утечка на КАЖДЫЙ импорт файла), +# И повторно внутри тела единственного live-теста. Теперь доступность БД +# проверяется отдельной дешёвой функцией с явным закрытием соединения +# (`_live_db_available`), а сама Session выдаётся pytest-фикстурой +# (`live_session`) с гарантированным close() в finally, а не ручным вызовом. -def _live_session(): # type: ignore[no-untyped-def] +def _live_db_available() -> bool: + """Дешёвая проверка доступности live-Postgres — соединение открывается и + СРАЗУ закрывается (`with engine.connect()`), никакого висящего ORM Session. + + Используется только в `pytest.mark.skipif(...)`, который вычисляется на + этапе сбора тестов — до фикстур. + """ try: from sqlalchemy import create_engine from sqlalchemy import text as sa_text - from sqlalchemy.orm import sessionmaker dsn = os.environ.get("TEST_DATABASE_URL") or os.environ.get("DATABASE_URL", "") if not dsn or "localhost:5432/test" in dsn: - return None + return False engine = create_engine(dsn, future=True) - conn = engine.connect() - conn.execute(sa_text("SELECT 1")) - conn.close() - return sessionmaker(bind=engine, future=True)() + try: + with engine.connect() as conn: + conn.execute(sa_text("SELECT 1")) + return True + finally: + engine.dispose() except Exception: - return None + return False + + +@pytest.fixture() +def live_session(): # type: ignore[no-untyped-def] + """Session для live-Postgres тестов — гарантированно закрывается после теста + (rollback + close + dispose в finally), в отличие от прежнего ручного вызова + `_live_session()` внутри тела каждого теста.""" + from sqlalchemy import create_engine + from sqlalchemy.orm import sessionmaker + + dsn = os.environ.get("TEST_DATABASE_URL") or os.environ.get("DATABASE_URL", "") + engine = create_engine(dsn, future=True) + session_factory = sessionmaker(bind=engine, future=True) + session = session_factory() + try: + yield session + finally: + session.rollback() + session.close() + engine.dispose() # Координаты вне Свердловской обл. (реальные данные там ~56-60/58-64) — изолируют @@ -445,58 +518,139 @@ def _live_session(): # type: ignore[no-untyped-def] _LIVE_LAT, _LIVE_LON = 1.111, 2.222 -@pytest.mark.skipif(_live_session() is None, reason="нет доступной Postgres test-БД") -def test_major1_cohort_excludes_novostroyki_and_city_precision_live() -> None: +@pytest.mark.skipif(not _live_db_available(), reason="нет доступной Postgres test-БД") +def test_major1_cohort_excludes_novostroyki_and_city_precision_live(live_session) -> None: # type: ignore[no-untyped-def] from sqlalchemy import text as sa_text from app.api.v1.trade_in import coverage_probe from app.schemas.trade_in import CoverageProbeInput - db = _live_session() - assert db is not None - try: - rows = [ - # (source_url suffix, listing_segment, geo_precision, price_rub) — все - # остальные поля общие: rooms=2, area_m2=50, is_active, scraped_at=NOW(). - ("ok-vtorichka", None, None, 5_000_000), # counted - ("bad-novostroyka", "novostroyki", None, 5_000_000), # excluded - ("bad-city-precision", None, "city", 5_000_000), # excluded - ("bad-zero-price", None, None, 0), # excluded - ] - for suffix, segment, geo_precision, price in rows: - url = f"https://test.invalid/coverage-major1-{suffix}" - db.execute( - sa_text( - """ - INSERT INTO listings - (source, source_url, source_id, dedup_hash, address, lat, lon, - rooms, area_m2, price_rub, is_active, scraped_at, - listing_segment, geo_precision) - VALUES - ('test', :url, :url, :url, 'test addr', :lat, :lon, - 2, 50.0, :price, true, NOW(), :segment, :geo_precision) - """ - ), - { - "url": url, - "lat": _LIVE_LAT, - "lon": _LIVE_LON, - "price": price, - "segment": segment, - "geo_precision": geo_precision, - }, - ) + db = live_session + rows = [ + # (source_url suffix, listing_segment, geo_precision, price_rub) — все + # остальные поля общие: rooms=2, area_m2=50, is_active, scraped_at=NOW(). + ("ok-vtorichka", None, None, 5_000_000), # counted + ("bad-novostroyka", "novostroyki", None, 5_000_000), # excluded + ("bad-city-precision", None, "city", 5_000_000), # excluded + ("bad-zero-price", None, None, 0), # excluded + ] + for suffix, segment, geo_precision, price in rows: + url = f"https://test.invalid/coverage-major1-{suffix}" + db.execute( + sa_text( + """ + INSERT INTO listings + (source, source_url, source_id, dedup_hash, address, lat, lon, + rooms, area_m2, price_rub, is_active, scraped_at, + listing_segment, geo_precision) + VALUES + ('test', :url, :url, :url, 'test addr', :lat, :lon, + 2, 50.0, :price, true, NOW(), :segment, :geo_precision) + """ + ), + { + "url": url, + "lat": _LIVE_LAT, + "lon": _LIVE_LON, + "price": price, + "segment": segment, + "geo_precision": geo_precision, + }, + ) - result = coverage_probe( - CoverageProbeInput(lat=_LIVE_LAT, lon=_LIVE_LON, rooms=2, area_m2=50.0), db - ) - # Только первая (ok-vtorichka) строка должна попадать в когорту — - # каждая следующая вставка не должна сдвигать счётчик. - assert result.n_listings == 1, ( - f"predicate regression: n_listings={result.n_listings} after inserting " - f"{suffix!r} (segment={segment!r} geo_precision={geo_precision!r} " - f"price={price}) — expected still 1 (only ok-vtorichka counted)" - ) - finally: - db.rollback() - db.close() + result = coverage_probe( + CoverageProbeInput(lat=_LIVE_LAT, lon=_LIVE_LON, rooms=2, area_m2=50.0), db + ) + # Только первая (ok-vtorichka) строка должна попадать в когорту — + # каждая следующая вставка не должна сдвигать счётчик. + assert result.n_listings == 1, ( + f"predicate regression: n_listings={result.n_listings} after inserting " + f"{suffix!r} (segment={segment!r} geo_precision={geo_precision!r} " + f"price={price}) — expected still 1 (only ok-vtorichka counted)" + ) + + +@pytest.mark.skipif(not _live_db_available(), reason="нет доступной Postgres test-БД") +def test_max_age_outlier_excluded_from_median_live(live_session) -> None: # type: ignore[no-untyped-def] + """MAJOR-2 поведенческий пин (повторная проверка #2894). + + Текстовый тест (test_max_age_outlier_days_passed_to_sql) проверял, что + подстрока `days_on_market <= :max_age_days` встречается в SQL — но она там + ДВАЖДЫ (count и percentile_cont), и мутация «убрать FILTER у + percentile_cont, оставив у count» проходила зелёной: n_with_age (из count) + оставался честным, а percentile_cont без FILTER считал медиану по ВСЕМ + days_on_market, включая выбросы. + + Вставляет когорту из 5 "нормальных" объявлений (days_on_market + 4/6/8/10/12, честная медиана — 8) и один выброс (days_on_market=4000, + > COVERAGE_MAX_AGE_DAYS=365). Проверяет, что после вставки выброса + n_with_age и median_listing_age_days НЕ меняются (выброс попадает только + в n_listings) — с правильными двумя FILTER это так; без FILTER у + percentile_cont медиана сдвинулась бы 8 → 9 (percentile_cont(0.5) по + [4,6,8,10,12,4000] = среднее 3-го и 4-го отсортированных значений = 9). + """ + from sqlalchemy import text as sa_text + + from app.api.v1.trade_in import coverage_probe + from app.schemas.trade_in import CoverageProbeInput + + db = live_session + normal_ages = [4, 6, 8, 10, 12] + for i, age in enumerate(normal_ages): + url = f"https://test.invalid/coverage-major2-normal-{i}" + db.execute( + sa_text( + """ + INSERT INTO listings + (source, source_url, source_id, dedup_hash, address, lat, lon, + rooms, area_m2, price_rub, is_active, scraped_at, days_on_market) + VALUES + ('test', :url, :url, :url, :addr, :lat, :lon, + 2, 50.0, 5000000, true, NOW(), :age) + """ + ), + { + "url": url, + "addr": f"test addr coverage-major2-{i}", + "lat": _LIVE_LAT, + "lon": _LIVE_LON, + "age": age, + }, + ) + + result = coverage_probe( + CoverageProbeInput(lat=_LIVE_LAT, lon=_LIVE_LON, rooms=2, area_m2=50.0), db + ) + assert result.n_listings == 5 + assert result.n_with_age == 5 + assert result.median_listing_age_days == 8 + + outlier_url = "https://test.invalid/coverage-major2-outlier" + db.execute( + sa_text( + """ + INSERT INTO listings + (source, source_url, source_id, dedup_hash, address, lat, lon, + rooms, area_m2, price_rub, is_active, scraped_at, days_on_market) + VALUES + ('test', :url, :url, :url, 'test addr coverage-major2-outlier', :lat, :lon, + 2, 50.0, 5000000, true, NOW(), 4000) + """ + ), + {"url": outlier_url, "lat": _LIVE_LAT, "lon": _LIVE_LON}, + ) + + result_with_outlier = coverage_probe( + CoverageProbeInput(lat=_LIVE_LAT, lon=_LIVE_LON, rooms=2, area_m2=50.0), db + ) + assert result_with_outlier.n_listings == 6 # выброс всё же попадает в n_listings + assert result_with_outlier.n_with_age == 5, ( + f"MAJOR-2 regression: outlier (days_on_market=4000 > MAX=365) leaked into " + f"n_with_age={result_with_outlier.n_with_age} — count(*) FILTER пропал/сломан" + ) + assert result_with_outlier.median_listing_age_days == 8, ( + f"MAJOR-2 regression: median_listing_age_days=" + f"{result_with_outlier.median_listing_age_days} shifted by outlier — " + f"percentile_cont(...) FILTER пропал (мутация «убрать FILTER у " + f"percentile_cont, оставив у count»)" + ) -- 2.45.3