diff --git a/tradein-mvp/backend/app/services/house_imv_backfill.py b/tradein-mvp/backend/app/services/house_imv_backfill.py index 38bdd005..cc172e1c 100644 --- a/tradein-mvp/backend/app/services/house_imv_backfill.py +++ b/tradein-mvp/backend/app/services/house_imv_backfill.py @@ -30,6 +30,7 @@ from dataclasses import dataclass, field from typing import Literal from scraper_kit.browser_fetcher import BrowserFetcher +from scraper_kit.house_type_normalizer import normalize_house_type # #2337 (Group E4, эпик #2277): переключено на scraper_kit — тот же периметр риска, # что и estimator.py (обе точки читают/пишут house_imv_evaluations, #651 IMV/Yandex @@ -65,22 +66,63 @@ _HEARTBEAT_EVERY_N_HOUSES = 5 # ── house_type normalisation ───────────────────────────────────────────────── +# Ключи — КАНОНИЧНЫЕ значения listings.house_type (после normalize_house_type), +# значения — вокабуляр Avito IMV. _HOUSE_TYPE_TO_IMV: dict[str, str] = { "panel": "panel", "brick": "brick", "monolith": "monolithic", - "monolithic": "monolithic", "monolith_brick": "monolithic", # Avito API не принимает гибриды "block": "block", "wood": "wood", } -_HOUSE_TYPE_DEFAULT = "panel" # самый распространённый в ЕКБ -def _map_house_type(raw: str | None) -> str: - if not raw: - return _HOUSE_TYPE_DEFAULT - return _HOUSE_TYPE_TO_IMV.get(raw.lower().strip(), _HOUSE_TYPE_DEFAULT) +def _map_house_type(raw: str | None) -> str | None: + """Наш house_type → вокабуляр Avito IMV. None = тип неизвестен, запрос не шлём. + + Сырое значение сначала прогоняем через общий normalize_house_type (#2007): он + знает camelCase-вокабуляр Циана (monolithBrick / gasSilicateBlock / + aerocreteBlock / stalin / ...) и SCREAMING-вокабуляр Яндекса, а нераспознанное + ('other', 'wireframe', пустое) схлопывает в None. Приведения к нижнему регистру + тут мало: ключ канона пишется через подчёркивание (monolith_brick), поэтому + 'monolithbrick' в словарь не попадал. + + #2674: раньше здесь стоял дефолт 'panel' — и когда типа нет вовсе, и когда он + есть, но не распознан. Панель — почти самый дешёвый класс (медиана по нашим же + 2685 оценкам: block 122.6k < panel 128.8k < brick 131.1k < monolithic 145.9k + ₽/м²), то есть дефолт систематически ЗАНИЖАЛ оценку: на проде 363 дома совсем + без типа + 75 домов с camelCase-типом (56 из них monolithBrick, −11.7% к + monolithic) уехали как панель. Теперь неизвестный тип → None → дом помечается + и запрос к площадке не тратится (см. _process_one_house). + """ + canon = normalize_house_type(raw) + if canon is None: + return None + return _HOUSE_TYPE_TO_IMV.get(canon) + + +def _map_renovation_type(repair_state: str | None) -> str: + """listings.repair_state → renovation_type вокабуляра Avito IMV. + + Переиспользуем _IMV_REPAIR_MAP эстиматора — единственный источник правды для + этого соответствия (needs_repair→required / standard→cosmetic / good→euro / + excellent→designer). Импорт ленивый: estimator тянет scraper_adapters, а тот + импортирует этот модуль (circular — см. блок импортов выше). + + #2674: раньше здесь стоял литерал 'cosmetic' — все 2685 запросов ушли как + «косметический ремонт», хотя мода по объявлениям этих же домов совсем другая + (standard 4564 / good 4118 / needs_repair 2279 / excellent 1631 — косметика + лишь 36%). + + Неизвестный ремонт (498 домов из 2685 — ни одного объявления с repair_state) + ОСТАЁТСЯ 'cosmetic', в отличие от неизвестного типа дома: это середина + порядковой шкалы (required < cosmetic < euro < designer), а не её край, + поэтому системного сдвига цены в одну сторону не даёт. + """ + from app.services.estimator import _IMV_REPAIR_MAP # lazy — см. import-блок + + return _IMV_REPAIR_MAP.get(repair_state) or "cosmetic" # ── Region bbox prefix для Avito geocoder ──────────────────────────────────── @@ -135,7 +177,8 @@ def pick_lot_params(db: Session, house_id: int) -> dict: AS integer) AS floor, CAST(percentile_cont(0.5) WITHIN GROUP (ORDER BY total_floors) AS integer) AS total_floors, - mode() WITHIN GROUP (ORDER BY house_type) AS house_type + mode() WITHIN GROUP (ORDER BY house_type) AS house_type, + mode() WITHIN GROUP (ORDER BY repair_state) AS repair_state FROM listings WHERE house_id_fk = :hid AND rooms IS NOT NULL @@ -173,7 +216,12 @@ def pick_lot_params(db: Session, house_id: int) -> dict: "floor": floor, "floor_at_home": floor_at_home, "house_type": _map_house_type(row["house_type"] or (house and house["house_type"])), - "renovation_type": "cosmetic", + "renovation_type": _map_renovation_type(row["repair_state"]), + # has_balcony/has_loggia остаются константами намеренно (#2674): покрытие + # listings.has_balcony 13.8%, listings.balcony_loggia 9.4%, и две колонки + # противоречат друг другу (по has_balcony «есть» у 62%, а по + # balcony_loggia самый частый случай — loggia 5650 против balcony 2794). + # Мода по одному-двум объявлениям на таком покрытии — шум, а не данные. "has_balcony": True, "has_loggia": False, } @@ -652,6 +700,14 @@ async def _process_one_house( _mark_status(db, hid, "no_params", "no listings with rooms+area") return "no_params" + # #2674: тип дома неизвестен (нет ни в объявлениях, ни в houses — либо + # вокабуляр не распознан). Раньше такой дом молча уезжал как 'panel' и + # занижал оценку. Лучше не тратить запрос и честно пометить дом — тот же + # путь, что и при отсутствии комнат/площади. + if params["house_type"] is None: + _mark_status(db, hid, "no_params", "unknown house_type") + return "no_params" + address = house.get("address") or house.get("full_address") if not address: _mark_status(db, hid, "no_address", "house.address is NULL") diff --git a/tradein-mvp/backend/app/services/product_handlers.py b/tradein-mvp/backend/app/services/product_handlers.py index ffdaf553..1f484ac4 100644 --- a/tradein-mvp/backend/app/services/product_handlers.py +++ b/tradein-mvp/backend/app/services/product_handlers.py @@ -401,17 +401,27 @@ async def _job_house_imv_backfill( only_status=only_status, heartbeat=_heartbeat, ) - ctx.runs.mark_done( - db, - run_id, - { - "checked": result.checked, - "saved": result.saved, - "skipped": result.skipped, - "errors": result.errors, - "duration_sec": int(result.duration_sec), - }, - ) + counters = { + "checked": result.checked, + "saved": result.saved, + "skipped": result.skipped, + "errors": result.errors, + "duration_sec": int(result.duration_sec), + } + # Честный статус (#2674, тот же класс, что #2670/#2657): успех — это + # «сделали то, что собирались», а не «не поймали известное исключение». + # На проде так ушли в done 31 прогон подряд: saved=0 при errors≈35 из 50. + # Ноль сохранённых БЕЗ ошибок (всё отфильтровано в skipped) — честная + # пустота, она по-прежнему done. + if result.saved == 0 and result.errors > 0: + ctx.runs.mark_failed( + db, + run_id, + f"saved=0 при errors={result.errors} (checked={result.checked})", + counters, + ) + else: + ctx.runs.mark_done(db, run_id, counters) except Exception as exc: logger.exception("scheduler: house_imv_backfill crashed run_id=%d", run_id) try: diff --git a/tradein-mvp/backend/tests/test_backfill_wave2.py b/tradein-mvp/backend/tests/test_backfill_wave2.py index 8aaefd7b..5ac78324 100644 --- a/tradein-mvp/backend/tests/test_backfill_wave2.py +++ b/tradein-mvp/backend/tests/test_backfill_wave2.py @@ -367,23 +367,28 @@ class TestHouseTypeMap: assert _map_house_type("brick") == "brick" assert _map_house_type("monolith") == "monolithic" assert _map_house_type("monolith_brick") == "monolithic" - assert _map_house_type("monolithic") == "monolithic" assert _map_house_type("block") == "block" assert _map_house_type("wood") == "wood" - def test_unknown_falls_back_to_panel(self): + def test_unknown_is_none_not_panel(self): + """#2674: дефолт 'panel' убран — он занижал оценку. Неизвестное → None. + + Полное покрытие camelCase-вокабуляра и skip-пути: + tests/test_house_imv_params_honesty.py. + """ from app.services.house_imv_backfill import _map_house_type - assert _map_house_type("unknown_type") == "panel" - assert _map_house_type(None) == "panel" - assert _map_house_type("") == "panel" + assert _map_house_type("unknown_type") is None + assert _map_house_type(None) is None + assert _map_house_type("") is None def test_case_insensitive(self): from app.services.house_imv_backfill import _map_house_type assert _map_house_type("PANEL") == "panel" assert _map_house_type("Brick") == "brick" - assert _map_house_type("MONOLITH_BRICK") == "monolithic" + # SCREAMING-вокабуляр Яндекса (MONOLIT_BRICK, одна «т») — реальный токен. + assert _map_house_type("MONOLIT_BRICK") == "monolithic" class TestRegionPrefix: diff --git a/tradein-mvp/backend/tests/test_house_imv_params_honesty.py b/tradein-mvp/backend/tests/test_house_imv_params_honesty.py new file mode 100644 index 00000000..224d9566 --- /dev/null +++ b/tradein-mvp/backend/tests/test_house_imv_params_honesty.py @@ -0,0 +1,206 @@ +"""#2674 — домовая оценка Авито перестаёт врать про ремонт и тип дома. + +Покрывает три дефекта из эпика: + 1. renovation_type берётся из моды listings.repair_state и проходит через + существующий estimator._IMV_REPAIR_MAP (был захардкожен литерал 'cosmetic': + 2685 из 2685 запросов ушли как «косметический ремонт»). + 2. Неизвестный тип дома НЕ уезжает дефолтом 'panel' (самый дешёвый класс → + системное занижение), а помечает дом и экономит запрос. Отдельно — + camelCase-вокабуляр Циана (monolithBrick / gasSilicateBlock / stalin) + распознаётся, к нижнему регистру он не приводится. + 3. Прогон с saved=0 и ненулевыми errors не помечается 'done'. + +БД и сеть замоканы — реального Postgres/Авито не нужно. +""" + +from __future__ import annotations + +import os +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +from app.services import house_imv_backfill as hib +from app.services.product_handlers import _job_house_imv_backfill + +# ── (1) renovation_type из данных через существующий маппинг ────────────────── + + +def _db_for_pick(listing_row: dict[str, Any], house_row: dict[str, Any] | None) -> MagicMock: + """MagicMock-Session: два .mappings().first() подряд (listings-агрегат, houses).""" + db = MagicMock() + db.execute.return_value.mappings.return_value.first.side_effect = [listing_row, house_row] + return db + + +def _listing_row(**over: Any) -> dict[str, Any]: + base: dict[str, Any] = { + "rooms": 2, + "area_m2": 52.0, + "floor": 3, + "total_floors": 10, + "house_type": "panel", + "repair_state": None, + } + base.update(over) + return base + + +@pytest.mark.parametrize( + ("repair_state", "expected"), + [ + ("needs_repair", "required"), + ("standard", "cosmetic"), + ("good", "euro"), + ("excellent", "designer"), + ], +) +def test_renovation_type_comes_from_listings_via_estimator_map( + repair_state: str, expected: str +) -> None: + """Мода repair_state → renovation_type ровно по estimator._IMV_REPAIR_MAP.""" + from app.services.estimator import _IMV_REPAIR_MAP + + params = hib.pick_lot_params(_db_for_pick(_listing_row(repair_state=repair_state), None), 1) + + assert params["renovation_type"] == expected + # Не второй словарь: значение обязано совпадать с источником правды. + assert params["renovation_type"] == _IMV_REPAIR_MAP[repair_state] + + +def test_renovation_type_not_hardcoded_cosmetic() -> None: + """Regression #2674: 'good' больше не превращается в 'cosmetic'.""" + params = hib.pick_lot_params(_db_for_pick(_listing_row(repair_state="good"), None), 1) + assert params["renovation_type"] != "cosmetic" + + +def test_unknown_repair_state_stays_cosmetic() -> None: + """Анти-оверрич: ремонт неизвестен → середина шкалы 'cosmetic', дом не теряем.""" + params = hib.pick_lot_params(_db_for_pick(_listing_row(repair_state=None), None), 1) + assert params["renovation_type"] == "cosmetic" + assert params["house_type"] == "panel" # дом всё ещё пригоден к запросу + + +# ── (2) тип дома: неизвестный не врёт, camelCase распознаётся ───────────────── + + +@pytest.mark.parametrize( + ("raw", "expected"), + [ + # camelCase из Циана — нижним регистром НЕ лечится (ключ канона через '_'). + ("monolithBrick", "monolithic"), + ("gasSilicateBlock", "block"), + ("aerocreteBlock", "block"), + ("stalin", "brick"), + # каноничные значения продолжают работать + ("panel", "panel"), + ("monolith", "monolithic"), + ("monolith_brick", "monolithic"), + ], +) +def test_map_house_type_recognises_camel_case(raw: str, expected: str) -> None: + assert hib._map_house_type(raw) == expected + + +@pytest.mark.parametrize("raw", [None, "", "other", "wireframe", "какая-то дичь"]) +def test_map_house_type_unknown_is_none_not_panel(raw: str | None) -> None: + """Regression #2674: нет типа / не распознан → None, а НЕ дефолт 'panel'.""" + assert hib._map_house_type(raw) is None + + +def test_pick_lot_params_unknown_house_type_yields_none() -> None: + """Типа нет ни в listings, ни в houses → house_type=None (не 'panel').""" + db = _db_for_pick(_listing_row(house_type=None), {"house_type": None, "total_floors": 9}) + assert hib.pick_lot_params(db, 1)["house_type"] is None + + +@pytest.mark.asyncio +async def test_unknown_house_type_skips_request_and_marks_house() -> None: + """Неизвестный тип → запрос к площадке НЕ уходит, дом помечен no_params.""" + params = { + "rooms": 2, + "area_m2": 52.0, + "floor": 3, + "floor_at_home": 10, + "house_type": None, + "renovation_type": "cosmetic", + "has_balcony": True, + "has_loggia": False, + } + houses = [{"id": 11, "address": "ул. X, 1", "full_address": None, "lat": 56.8, "lon": 60.6}] + db = MagicMock() + + with ( + patch.object(hib, "pick_lot_params", return_value=params), + patch.object(hib, "evaluate_via_imv", new_callable=AsyncMock) as mock_eval, + patch.object(hib, "_mark_status") as mock_mark, + ): + db.execute.return_value.mappings.return_value.all.return_value = houses + result = await hib.backfill_house_imv(db, batch_size=10, request_delay_sec=0.0) + + mock_eval.assert_not_called() + mock_mark.assert_called_once_with(db, 11, "no_params", "unknown house_type") + assert result.skipped == 1 + assert result.saved == 0 + + +# ── (3) прогон с нулём сохранённых и ошибками не «успешен» ──────────────────── + + +class _RunsRecorder: + """Duck-typed ctx.runs: пишет, чем закончился прогон.""" + + def __init__(self) -> None: + self.calls: list[tuple[str, dict[str, Any]]] = [] + + def update_heartbeat(self, db: Any, run_id: int, counters: dict[str, Any]) -> None: + return None + + def mark_done(self, db: Any, run_id: int, counters: dict[str, Any]) -> None: + self.calls.append(("mark_done", counters)) + + def mark_failed(self, db: Any, run_id: int, error: str, counters: dict[str, Any]) -> None: + self.calls.append(("mark_failed", counters)) + + +async def _drive_job(*, saved: int, errors: int, skipped: int = 0) -> list[tuple[str, dict]]: + runs = _RunsRecorder() + enrichment = MagicMock() + enrichment.house_imv_backfill = AsyncMock( + return_value=hib.HouseIMVBackfillResult( + checked=saved + errors + skipped, + saved=saved, + skipped=skipped, + errors=errors, + duration_sec=1.0, + ) + ) + ctx = MagicMock(runs=runs, enrichment=enrichment) + await _job_house_imv_backfill(MagicMock(), 1, {}, ctx) + return runs.calls + + +@pytest.mark.asyncio +async def test_zero_saved_with_errors_is_not_done() -> None: + """Прод-случай: 31 прогон подряд saved=0 / errors≈35 из 50 уходил в 'done'.""" + calls = await _drive_job(saved=0, errors=35, skipped=15) + assert calls[-1][0] == "mark_failed" + assert calls[-1][1]["saved"] == 0 + assert calls[-1][1]["errors"] == 35 + + +@pytest.mark.asyncio +async def test_honest_empty_stays_done() -> None: + """Анти-оверрич: ноль сохранённых без ошибок (всё в skipped) — честная пустота.""" + calls = await _drive_job(saved=0, errors=0, skipped=50) + assert calls[-1][0] == "mark_done" + + +@pytest.mark.asyncio +async def test_partial_success_stays_done() -> None: + """Анти-оверрич: что-то сохранили — прогон успешен, даже если были ошибки.""" + calls = await _drive_job(saved=3, errors=7) + assert calls[-1][0] == "mark_done"