Merge pull request 'feat(tradein/estimator): switch core valuation call sites to scraper_kit (#2337)' (#2348) from feat/tradein-estimator-kit-switch into main
All checks were successful
Deploy Trade-In / changes (push) Successful in 11s
Deploy Trade-In / build-frontend (push) Has been skipped
Deploy Trade-In / build-browser (push) Has been skipped
Deploy Trade-In / test (push) Successful in 1m35s
Deploy Trade-In / build-backend (push) Successful in 59s
Deploy Trade-In / deploy (push) Successful in 44s

This commit is contained in:
lekss361 2026-07-03 23:49:10 +00:00
commit 7ebeddc2bc
6 changed files with 168 additions and 98 deletions

View file

@ -32,6 +32,23 @@ from datetime import UTC, date, datetime, timedelta
from typing import Any
from uuid import uuid4
from scraper_kit.providers.avito.imv import (
IMVAddressNotFoundError,
IMVAuthError,
IMVEvaluation,
IMVTransientError,
compute_imv_cache_key,
evaluate_via_imv,
save_imv_evaluation,
)
from scraper_kit.providers.cian.valuation import (
CianValuationResult,
estimate_via_cian_valuation,
)
from scraper_kit.providers.yandex.valuation import (
YandexValuationResult,
YandexValuationScraper,
)
from sqlalchemy import text
from sqlalchemy.orm import Session
@ -50,23 +67,8 @@ from app.services.dadata import clean_address as dadata_clean_address
from app.services.geocoder import GeocodeResult, geocode
from app.services.house_metadata import get_house_metadata
from app.services.matching.houses import match_house_readonly, match_or_create_house
from app.services.scrapers.avito_imv import (
IMVAddressNotFoundError,
IMVAuthError,
IMVEvaluation,
IMVTransientError,
compute_imv_cache_key,
evaluate_via_imv,
save_imv_evaluation,
)
from app.services.scrapers.cian_valuation import (
CianValuationResult,
estimate_via_cian_valuation,
)
from app.services.scrapers.yandex_valuation import (
YandexValuationResult,
YandexValuationScraper,
)
from app.services.scraper_adapters import RealScraperConfig
from app.services.scraper_settings import get_scraper_delay
logger = logging.getLogger(__name__)
@ -422,7 +424,7 @@ async def _get_or_fetch_imv_cached(
cache_key[:8],
existing["recommended_price"],
)
from app.services.scrapers.avito_imv import IMVGeo
from scraper_kit.providers.avito.imv import IMVGeo
return IMVEvaluation(
cache_key=existing["cache_key"],
@ -463,6 +465,7 @@ async def _get_or_fetch_imv_cached(
renovation_type=renovation_type,
has_balcony=has_balcony,
has_loggia=has_loggia,
config=RealScraperConfig(),
)
save_imv_evaluation(db, result, estimate_id=estimate_id_for_link)
logger.info(
@ -495,6 +498,7 @@ async def _get_or_fetch_imv_cached(
renovation_type=renovation_type,
has_balcony=has_balcony,
has_loggia=has_loggia,
config=RealScraperConfig(),
)
save_imv_evaluation(db, result, estimate_id=estimate_id_for_link)
logger.info(
@ -595,14 +599,21 @@ async def _get_or_fetch_yandex_valuation_cached(
# Fresh fetch
try:
async with YandexValuationScraper() as scraper:
async with YandexValuationScraper(
RealScraperConfig(), delay_provider=get_scraper_delay
) as scraper:
result = await scraper.fetch_house_history(
address=address,
offer_category=offer_category,
offer_type=offer_type,
)
except Exception as e:
logger.warning("yandex_valuation: fetch failed — estimator продолжает без Yandex: %s", e)
except Exception:
# logger.exception (не .warning) — намеренно: GlitchTip LoggingIntegration
# (main.py/scheduler_main.py) слушает event_level=logging.ERROR. WARNING,
# даже с exc_info=True, остаётся ниже порога и НЕ создаёт событие в GlitchTip
# (только breadcrumb) — config-wiring регрессия здесь была бы не видна
# мониторингу. .exception() логирует на ERROR + traceback (#2337).
logger.exception("yandex_valuation: fetch failed — estimator продолжает без Yandex")
return None
if result is None:
@ -3120,6 +3131,7 @@ async def estimate_quality(
cian_val = await _with_budget(
estimate_via_cian_valuation(
db,
config=RealScraperConfig(),
address=geo.full_address,
total_area=payload.area_m2,
rooms_count=payload.rooms,
@ -3140,8 +3152,13 @@ async def estimate_quality(
cian_val.sale_accuracy,
cian_val.external_house_id,
)
except Exception as exc:
logger.warning("cian_valuation: lookup failed (graceful): %s", exc)
except Exception:
# logger.exception (не .warning) — намеренно: GlitchTip LoggingIntegration
# (main.py/scheduler_main.py) слушает event_level=logging.ERROR. WARNING,
# даже с exc_info=True, остаётся ниже порога и НЕ создаёт событие в GlitchTip
# (только breadcrumb) — config-wiring регрессия здесь была бы не видна
# мониторингу. .exception() логирует на ERROR + traceback (#2337).
logger.exception("cian_valuation: lookup failed (graceful)")
# ── Pre-fetch: same-building anchor comps ─────────────────────────────────
# Guard mirrors the original in-block guard exactly; when false → ([], None).

View file

@ -12,7 +12,10 @@ For each house:
5. Mark houses.imv_status accordingly (ok/not_found/no_params/error/transient_error).
Resumable: re-running picks up from where the previous run stopped.
Deliberately NOT wired into scheduler triggered manually via admin API.
process_houses_imv_batch() IS wired into the scheduler: run_avito_city_sweep()
(scrape_pipeline.py) calls it automatically as the final IMV-phase of every
avito city sweep. backfill_house_imv() (batch/pending-status entrypoint) remains
manual-only, triggered via admin API.
"""
from __future__ import annotations
@ -26,24 +29,30 @@ from dataclasses import dataclass, field
from typing import Literal
from scraper_kit.browser_fetcher import BrowserFetcher
from sqlalchemy import text
from sqlalchemy.orm import Session
from app.core.config import settings
# app.services.scrapers.avito_imv остаётся legacy — миграция валюационных
# зависимостей (avito_imv/cian_valuation/yandex_valuation) выделена в отдельный,
# более рискованный issue #2308 (см. epic #2277, issue #2310 Group C). Мигрируем
# здесь только BrowserFetcher (#2310, выше) — avito_imv.evaluate_via_imv принимает
# его исключительно как TYPE_CHECKING-аннотацию (duck-typed на fetch_json), поэтому
# смена конкретного класса не требует правок avito_imv.py.
from app.services.scrapers.avito_imv import (
# #2337 (Group E4, эпик #2277): переключено на scraper_kit — тот же периметр риска,
# что и estimator.py (обе точки читают/пишут house_imv_evaluations, #651 IMV/Yandex
# blend). config=RealScraperConfig() обязателен для evaluate_via_imv — без него kit
# молча уходит без прокси (прямое подключение) вместо настроенного (см. #2334).
from scraper_kit.providers.avito.imv import (
IMVAddressNotFoundError,
IMVAuthError,
IMVEvaluation,
IMVTransientError,
evaluate_via_imv,
)
from sqlalchemy import text
from sqlalchemy.orm import Session
from app.core.config import settings
# RealScraperConfig НЕ импортируется на уровне модуля: app.services.scraper_adapters
# сам импортирует backfill_house_imv/process_houses_imv_batch ИЗ этого модуля
# (RealEnrichmentJobs-адаптер) — top-level import здесь создал бы circular import
# (проверено: ImportError "cannot import name 'RealScraperConfig' from partially
# initialized module" при импорте scraper_adapters первым). Ленивый import внутри
# _process_one_house() ломает цикл — к моменту вызова оба модуля уже полностью
# инициализированы.
logger = logging.getLogger(__name__)
@ -633,6 +642,8 @@ async def _process_one_house(
BrowserFetcher на весь батч evaluate_via_imv ходит через /fetch-json
sidecar (обходит datacenter-403). None curl_cffi-путь (как раньше).
"""
from app.services.scraper_adapters import RealScraperConfig # lazy — см. import-блок
hid: int = house["id"]
params = pick_lot_params(db, hid)
@ -651,7 +662,10 @@ async def _process_one_house(
try:
eval_result = await evaluate_via_imv(
address=enriched, browser_fetcher=browser_fetcher, **params
address=enriched,
browser_fetcher=browser_fetcher,
config=RealScraperConfig(),
**params,
)
except IMVAddressNotFoundError as exc:
_mark_status(db, hid, "not_found", str(exc)[:200])

View file

@ -436,8 +436,10 @@ class TestEnrichAddress:
@pytest.mark.asyncio
async def test_backfill_house_imv_ok_path():
"""Happy path: picks params, calls IMV, saves → saved=1."""
from scraper_kit.providers.avito.imv import IMVEvaluation, IMVGeo
from app.services.house_imv_backfill import backfill_house_imv
from app.services.scrapers.avito_imv import IMVEvaluation, IMVGeo
from app.services.scraper_adapters import RealScraperConfig
fake_geo = IMVGeo(geo_hash="tok", lat=56.83, lon=60.60)
fake_eval = IMVEvaluation(
@ -487,7 +489,7 @@ async def test_backfill_house_imv_ok_path():
"app.services.house_imv_backfill.evaluate_via_imv",
new_callable=AsyncMock,
return_value=fake_eval,
),
) as mock_evaluate,
patch("app.services.house_imv_backfill.save_imv_result") as mock_save,
):
mock_mappings = MagicMock()
@ -501,6 +503,12 @@ async def test_backfill_house_imv_ok_path():
assert result.errors == 0
assert result.status_counts.get("ok") == 1
mock_save.assert_called_once()
# #2337 regression guard (Group E4): kit evaluate_via_imv silently drops the
# Avito proxy (direct connection) unless config=RealScraperConfig() is passed
# at the call site — assert_called_once() alone wouldn't catch someone
# dropping that kwarg later (same pattern as #2306 cian_price_history guards).
_, call_kwargs = mock_evaluate.call_args
assert isinstance(call_kwargs.get("config"), RealScraperConfig)
@pytest.mark.asyncio
@ -537,8 +545,9 @@ async def test_backfill_house_imv_no_params():
@pytest.mark.asyncio
async def test_backfill_house_imv_not_found():
"""IMVAddressNotFoundError → status not_found, no crash."""
from scraper_kit.providers.avito.imv import IMVAddressNotFoundError
from app.services.house_imv_backfill import backfill_house_imv
from app.services.scrapers.avito_imv import IMVAddressNotFoundError
fake_params = {
"rooms": 2,

View file

@ -1,4 +1,5 @@
"""Tests for Cian Valuation integration in estimator.py (Stage 9)."""
import os
# Settings requires DATABASE_URL at init time. Set dummy DSN before any app import.
@ -27,9 +28,11 @@ def _fake_cian_result(**kwargs) -> CianValuationResult:
# ── Cache-layer unit tests ────────────────────────────────────────────────────
def test_cian_valuation_cache_key_deterministic() -> None:
"""compute_cache_key produces same hash for identical inputs."""
from app.services.scrapers.cian_valuation import compute_cache_key
k1 = compute_cache_key("ЕКБ, ул. Учителей, 18", 38.8, 1, 4, "cosmetic", "sale")
k2 = compute_cache_key("ЕКБ, ул. Учителей, 18", 38.8, 1, 4, "cosmetic", "sale")
assert k1 == k2
@ -38,6 +41,7 @@ def test_cian_valuation_cache_key_deterministic() -> None:
def test_cian_valuation_cache_key_differs_for_different_inputs() -> None:
from app.services.scrapers.cian_valuation import compute_cache_key
a = compute_cache_key("addr A", 38.8, 1, 4, "cosmetic", "sale")
b = compute_cache_key("addr B", 38.8, 1, 4, "cosmetic", "sale")
c = compute_cache_key("addr A", 50.0, 1, 4, "cosmetic", "sale")
@ -47,6 +51,7 @@ def test_cian_valuation_cache_key_differs_for_different_inputs() -> None:
# ── estimate_via_cian_valuation graceful-degradation tests ───────────────────
def test_estimate_via_cian_returns_none_when_no_cookies() -> None:
"""estimate_via_cian_valuation returns None when no cookies in DB (graceful)."""
db = MagicMock()
@ -59,6 +64,7 @@ def test_estimate_via_cian_returns_none_when_no_cookies() -> None:
return_value=None, # no cookies stored
):
from app.services.scrapers.cian_valuation import estimate_via_cian_valuation
result = await estimate_via_cian_valuation(
db,
address="ЕКБ, ул. Учителей, 18",
@ -75,9 +81,11 @@ def test_estimate_via_cian_returns_none_when_no_cookies() -> None:
# ── Estimator integration tests ───────────────────────────────────────────────
def _make_fake_geo():
"""Return GeocodeResult with provider field (not source)."""
from app.services.geocoder import GeocodeResult
return GeocodeResult(
lat=56.838,
lon=60.595,
@ -88,6 +96,7 @@ def _make_fake_geo():
def _make_payload():
from app.schemas.trade_in import TradeInEstimateInput
return TradeInEstimateInput(
address="ЕКБ, ул. Учителей, 18",
area_m2=38.8,
@ -127,6 +136,7 @@ def _common_patches(cian_mock):
def test_estimator_includes_cian_valuation_when_available() -> None:
"""When cian_valuation returns result with sale_price_rub, sources_used includes it."""
from app.services.estimator import estimate_quality
from app.services.scraper_adapters import RealScraperConfig
db = MagicMock()
payload = _make_payload()
@ -135,24 +145,29 @@ def test_estimator_includes_cian_valuation_when_available() -> None:
async def _run() -> None:
with (
patch("app.services.estimator.geocode",
new=AsyncMock(return_value=_make_fake_geo())),
patch("app.services.estimator.get_house_metadata",
new=AsyncMock(return_value=None)),
patch("app.services.estimator.geocode", new=AsyncMock(return_value=_make_fake_geo())),
patch("app.services.estimator.get_house_metadata", new=AsyncMock(return_value=None)),
patch("app.services.estimator._fetch_analogs", return_value=([], False, "W")),
patch("app.services.estimator._fetch_deals", return_value=[]),
patch("app.services.estimator._get_or_fetch_imv_cached",
new=AsyncMock(return_value=None)),
patch("app.services.estimator._get_or_fetch_yandex_valuation_cached",
new=AsyncMock(return_value=None)),
patch("app.services.estimator.estimate_via_cian_valuation",
new=cian_mock),
patch("app.services.estimator._get_asking_sold_ratio",
return_value=(None, None)),
patch(
"app.services.estimator._get_or_fetch_imv_cached", new=AsyncMock(return_value=None)
),
patch(
"app.services.estimator._get_or_fetch_yandex_valuation_cached",
new=AsyncMock(return_value=None),
),
patch("app.services.estimator.estimate_via_cian_valuation", new=cian_mock),
patch("app.services.estimator._get_asking_sold_ratio", return_value=(None, None)),
):
result = await estimate_quality(payload, db)
assert "cian_valuation" in result.sources_used
# #2337 regression guard (Group E4): estimate_via_cian_valuation's config
# param is MANDATORY on the kit function (TypeError if omitted) — but this
# mock wouldn't catch a missing/None config kwarg. Explicit check mirrors
# the #2306 cian_price_history guard pattern for this higher-stakes path.
_, call_kwargs = cian_mock.call_args
assert isinstance(call_kwargs.get("config"), RealScraperConfig)
anyio.run(_run)
@ -167,20 +182,19 @@ def test_estimator_graceful_when_cian_returns_none() -> None:
async def _run() -> None:
with (
patch("app.services.estimator.geocode",
new=AsyncMock(return_value=_make_fake_geo())),
patch("app.services.estimator.get_house_metadata",
new=AsyncMock(return_value=None)),
patch("app.services.estimator.geocode", new=AsyncMock(return_value=_make_fake_geo())),
patch("app.services.estimator.get_house_metadata", new=AsyncMock(return_value=None)),
patch("app.services.estimator._fetch_analogs", return_value=([], False, "W")),
patch("app.services.estimator._fetch_deals", return_value=[]),
patch("app.services.estimator._get_or_fetch_imv_cached",
new=AsyncMock(return_value=None)),
patch("app.services.estimator._get_or_fetch_yandex_valuation_cached",
new=AsyncMock(return_value=None)),
patch("app.services.estimator.estimate_via_cian_valuation",
new=cian_mock),
patch("app.services.estimator._get_asking_sold_ratio",
return_value=(None, None)),
patch(
"app.services.estimator._get_or_fetch_imv_cached", new=AsyncMock(return_value=None)
),
patch(
"app.services.estimator._get_or_fetch_yandex_valuation_cached",
new=AsyncMock(return_value=None),
),
patch("app.services.estimator.estimate_via_cian_valuation", new=cian_mock),
patch("app.services.estimator._get_asking_sold_ratio", return_value=(None, None)),
):
result = await estimate_quality(payload, db)
@ -200,20 +214,19 @@ def test_estimator_graceful_when_cian_raises() -> None:
async def _run() -> None:
with (
patch("app.services.estimator.geocode",
new=AsyncMock(return_value=_make_fake_geo())),
patch("app.services.estimator.get_house_metadata",
new=AsyncMock(return_value=None)),
patch("app.services.estimator.geocode", new=AsyncMock(return_value=_make_fake_geo())),
patch("app.services.estimator.get_house_metadata", new=AsyncMock(return_value=None)),
patch("app.services.estimator._fetch_analogs", return_value=([], False, "W")),
patch("app.services.estimator._fetch_deals", return_value=[]),
patch("app.services.estimator._get_or_fetch_imv_cached",
new=AsyncMock(return_value=None)),
patch("app.services.estimator._get_or_fetch_yandex_valuation_cached",
new=AsyncMock(return_value=None)),
patch("app.services.estimator.estimate_via_cian_valuation",
new=cian_mock),
patch("app.services.estimator._get_asking_sold_ratio",
return_value=(None, None)),
patch(
"app.services.estimator._get_or_fetch_imv_cached", new=AsyncMock(return_value=None)
),
patch(
"app.services.estimator._get_or_fetch_yandex_valuation_cached",
new=AsyncMock(return_value=None),
),
patch("app.services.estimator.estimate_via_cian_valuation", new=cian_mock),
patch("app.services.estimator._get_asking_sold_ratio", return_value=(None, None)),
):
result = await estimate_quality(payload, db)
@ -235,20 +248,19 @@ def test_estimator_cian_result_no_sale_price_not_added() -> None:
async def _run() -> None:
with (
patch("app.services.estimator.geocode",
new=AsyncMock(return_value=_make_fake_geo())),
patch("app.services.estimator.get_house_metadata",
new=AsyncMock(return_value=None)),
patch("app.services.estimator.geocode", new=AsyncMock(return_value=_make_fake_geo())),
patch("app.services.estimator.get_house_metadata", new=AsyncMock(return_value=None)),
patch("app.services.estimator._fetch_analogs", return_value=([], False, "W")),
patch("app.services.estimator._fetch_deals", return_value=[]),
patch("app.services.estimator._get_or_fetch_imv_cached",
new=AsyncMock(return_value=None)),
patch("app.services.estimator._get_or_fetch_yandex_valuation_cached",
new=AsyncMock(return_value=None)),
patch("app.services.estimator.estimate_via_cian_valuation",
new=cian_mock),
patch("app.services.estimator._get_asking_sold_ratio",
return_value=(None, None)),
patch(
"app.services.estimator._get_or_fetch_imv_cached", new=AsyncMock(return_value=None)
),
patch(
"app.services.estimator._get_or_fetch_yandex_valuation_cached",
new=AsyncMock(return_value=None),
),
patch("app.services.estimator.estimate_via_cian_valuation", new=cian_mock),
patch("app.services.estimator._get_asking_sold_ratio", return_value=(None, None)),
):
result = await estimate_quality(payload, db)

View file

@ -32,7 +32,9 @@ def test_imv_repair_map() -> None:
def test_imv_cache_miss_calls_evaluate() -> None:
"""Cache miss → calls evaluate_via_imv + save_imv_evaluation."""
from app.services.scrapers.avito_imv import IMVEvaluation, IMVGeo
from scraper_kit.providers.avito.imv import IMVEvaluation, IMVGeo
from app.services.scraper_adapters import RealScraperConfig
mock_db = MagicMock()
mock_db.execute.return_value.mappings.return_value.first.return_value = None # no cache hit
@ -56,11 +58,9 @@ def test_imv_cache_miss_calls_evaluate() -> None:
)
async def _run() -> None:
mock_evaluate = AsyncMock(return_value=fake_result)
with (
patch(
"app.services.estimator.evaluate_via_imv",
new=AsyncMock(return_value=fake_result),
),
patch("app.services.estimator.evaluate_via_imv", new=mock_evaluate),
patch("app.services.estimator.save_imv_evaluation", return_value=1),
):
result = await _get_or_fetch_imv_cached(
@ -77,6 +77,12 @@ def test_imv_cache_miss_calls_evaluate() -> None:
)
assert result is not None
assert result.recommended_price == 6_290_000
# #2337 regression guard (Group E4): kit evaluate_via_imv silently drops the
# Avito proxy (direct connection) unless config=RealScraperConfig() is passed
# at the call site — assert_called alone wouldn't catch someone dropping
# that kwarg later (same pattern as #2306 cian_price_history guards).
_, call_kwargs = mock_evaluate.call_args
assert isinstance(call_kwargs.get("config"), RealScraperConfig)
anyio.run(_run)

View file

@ -9,6 +9,11 @@ from datetime import date
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from scraper_kit.providers.yandex.valuation import (
ValuationHistoryItem,
ValuationHouseMeta,
YandexValuationResult,
)
from app.services.estimator import (
YANDEX_VALUATION_CACHE_TTL_HOURS,
@ -16,11 +21,7 @@ from app.services.estimator import (
_save_yandex_history_items,
_yandex_valuation_cache_key,
)
from app.services.scrapers.yandex_valuation import (
ValuationHistoryItem,
ValuationHouseMeta,
YandexValuationResult,
)
from app.services.scraper_settings import get_scraper_delay
def _sample_result(address: str = "Екатеринбург, ул. Учителей, 18") -> YandexValuationResult:
@ -108,6 +109,8 @@ async def test_cache_hit_returns_deserialized_result():
@pytest.mark.asyncio
async def test_cache_miss_triggers_fetch_and_persist():
"""On cache miss, fetch via scraper + INSERT into external_valuations."""
from app.services.scraper_adapters import RealScraperConfig
db = MagicMock()
db.execute.return_value.mappings.return_value.first.return_value = None # miss
fresh = _sample_result()
@ -115,10 +118,19 @@ async def test_cache_miss_triggers_fetch_and_persist():
fake_scraper.__aenter__ = AsyncMock(return_value=fake_scraper)
fake_scraper.__aexit__ = AsyncMock(return_value=False)
fake_scraper.fetch_house_history = AsyncMock(return_value=fresh)
with patch("app.services.estimator.YandexValuationScraper", return_value=fake_scraper):
with patch(
"app.services.estimator.YandexValuationScraper", return_value=fake_scraper
) as mock_scraper_cls:
out = await _get_or_fetch_yandex_valuation_cached(db, address=fresh.address)
assert out is fresh
fake_scraper.fetch_house_history.assert_awaited_once()
# #2337 regression guard (Group E4): kit YandexValuationScraper requires config=
# (TypeError if omitted) AND silently falls back to a hardcoded 5.0s throttle
# instead of the DB-configured anti-ban delay unless delay_provider= is passed.
# assert_called alone wouldn't catch either kwarg being dropped later.
call_args, call_kwargs = mock_scraper_cls.call_args
assert isinstance(call_args[0], RealScraperConfig)
assert call_kwargs.get("delay_provider") is get_scraper_delay
# 2 DB calls: 1 SELECT (miss) + 1 INSERT/UPSERT
assert db.execute.call_count >= 2