Merge pull request 'test(tradein/scraper-kit): golden-parity + config-footgun proof for avito_imv (#2334)' (#2344) from feat/tradein-migrate-avito-imv-kit into main

tradein-mvp/backend/tests/scrapers/test_avito_imv_kit_parity.py

@@ -0,0 +1,309 @@
+"""Golden-parity + config-footgun proof for `evaluate_via_imv` — issue #2334.
+
+Group E1 of the scraper_kit migration epic (#2277 → #2308 → #2334): migrates
+`app/services/scrapers/avito_imv.py` readiness onto its
+`scraper_kit.providers.avito.imv` equivalent. Unlike prior groups (#2305/#2306),
+`evaluate_via_imv` had ZERO parity coverage before this file — `_parse_price` alone
+is covered by `tests/scrapers/test_admin_avito_kit_parity.py` (Group A, admin debug
+route), but the full 3-request async flow (geocode A/B + evaluate C) that
+`app/services/estimator.py` and `app/services/house_imv_backfill.py` depend on was
+never proven byte-for-byte identical between legacy and kit.
+
+Two independent things are proven here:
+
+1. **Golden parity** — legacy `evaluate_via_imv` and kit `evaluate_via_imv` give the
+   IDENTICAL `IMVEvaluation` on the SAME input, using REAL Avito API responses
+   recorded live (`tests/fixtures/avito_imv_geo_position.json` +
+   `avito_imv_getdata.json`, see `tests/test_avito_imv_parse.py` for provenance —
+   issue #565, 2026-05 capture, ЕКБ ул. Малышева 125). Both sides run through the
+   `browser_fetcher=` transport (see `tests/scrapers/test_avito_imv_browser_transport.py`)
+   so the test is offline/deterministic and never touches `curl_cffi`/proxy config —
+   that's a SEPARATE axis, covered below.
+
+2. **Config-footgun regression** — CONFIRMED by recon before this issue started:
+   kit `evaluate_via_imv(config: ScraperConfig | None = None, ...)` only reads
+   `config.scraper_proxy_url` when `config is not None`; legacy unconditionally reads
+   `settings.scraper_proxy_url` itself. Call the kit fn WITHOUT `config=` → own
+   curl_cffi session gets built with NO proxy (`proxies=None`) even though a real
+   proxy is configured — a silent behaviour change vs legacy. WITH
+   `config=RealScraperConfig()` (the established DI pattern, see
+   `app/services/cian_price_history.py` / `tests/test_scraper_kit_pricehistory_session_parity.py::
+   test_fetch_detail_own_session_proxy_wiring_parity` for precedent) the proxy is
+   used identically to legacy. NOTE: neither `estimator.py` nor
+   `house_imv_backfill.py` is switched to the kit import in this issue (see report in
+   PR/issue #2334) — these tests exist to prove the KIT SIDE is safe to wire up
+   whenever that switch happens (#2337 for estimator.py).
+"""
+
+from __future__ import annotations
+
+import asyncio
+import json
+import os
+from pathlib import Path
+from typing import Any
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+# app.core.config.Settings() requires DATABASE_URL at import time; app.services.
+# scraper_adapters (RealScraperConfig) imports app.core.config transitively.
+# Mirror tests/test_scraper_kit_pricehistory_session_parity.py.
+os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost/test_db")
+
+from scraper_kit.providers.avito.imv import (
+    IMVAddressNotFoundError as KitIMVAddressNotFoundError,
+)
+from scraper_kit.providers.avito.imv import _parse_geo_position as kit_parse_geo_position
+from scraper_kit.providers.avito.imv import _parse_placement_history as kit_parse_history
+from scraper_kit.providers.avito.imv import _parse_suggestions as kit_parse_suggestions
+from scraper_kit.providers.avito.imv import compute_imv_cache_key as kit_cache_key
+from scraper_kit.providers.avito.imv import evaluate_via_imv as kit_evaluate_via_imv
+
+from app.core.config import settings
+from app.services.scraper_adapters import RealScraperConfig
+from app.services.scrapers.avito_imv import _parse_geo_position as legacy_parse_geo_position
+from app.services.scrapers.avito_imv import _parse_placement_history as legacy_parse_history
+from app.services.scrapers.avito_imv import _parse_suggestions as legacy_parse_suggestions
+from app.services.scrapers.avito_imv import compute_imv_cache_key as legacy_cache_key
+from app.services.scrapers.avito_imv import evaluate_via_imv as legacy_evaluate_via_imv
+from tests.support.parity import ParityMismatchError, assert_parity
+
+_FIXTURES = Path(__file__).parent.parent / "fixtures"
+
+
+def _load_fixture(name: str) -> dict[str, Any]:
+    with (_FIXTURES / name).open(encoding="utf-8") as f:
+        return json.load(f)
+
+
+# ---------------------------------------------------------------------------
+# Cheap pure-function parity (closes the gap beyond `_parse_price`, already
+# covered by tests/scrapers/test_admin_avito_kit_parity.py Group A).
+# ---------------------------------------------------------------------------
+
+
+def test_compute_imv_cache_key_parity() -> None:
+    assert_parity(
+        legacy_fn=legacy_cache_key,
+        kit_fn=kit_cache_key,
+        fixtures=[
+            ("ЕКБ ул. Малышева, 125", 2, 54.0, 4, 9, "panel", "cosmetic", True, False),
+        ],
+    )
+
+
+def test_parse_geo_position_parity_real_fixture() -> None:
+    data_b = _load_fixture("avito_imv_geo_position.json")
+    assert_parity(
+        legacy_fn=lambda: legacy_parse_geo_position(data_b, lat=56.840872, lon=60.654077),
+        kit_fn=lambda: kit_parse_geo_position(data_b, lat=56.840872, lon=60.654077),
+        fixtures=[()],
+    )
+
+
+def test_parse_placement_history_parity_real_fixture() -> None:
+    data_c = _load_fixture("avito_imv_getdata.json")
+    assert_parity(
+        legacy_fn=legacy_parse_history,
+        kit_fn=kit_parse_history,
+        fixtures=[(data_c,)],
+    )
+
+
+def test_parse_suggestions_parity_real_fixture() -> None:
+    data_c = _load_fixture("avito_imv_getdata.json")
+    assert_parity(
+        legacy_fn=legacy_parse_suggestions,
+        kit_fn=kit_parse_suggestions,
+        fixtures=[(data_c,)],
+    )
+
+
+# ---------------------------------------------------------------------------
+# Golden parity — full evaluate_via_imv() flow on REAL captured API fixtures
+# ---------------------------------------------------------------------------
+
+# Same address/lat/lon/lot-params as tests/test_avito_imv_parse.py::
+# test_fixtures_feed_full_evaluation_shape — keeps this test consistent with the
+# established "reference realistic case" derived from the live 2026-05 capture
+# (issue #565), rather than inventing a new ad-hoc fixture.
+_ADDRESS = "Свердловская область, Екатеринбург, улица Малышева, 125"
+_LAT = 56.840872
+_LON = 60.654077
+_LOT_PARAMS: dict[str, Any] = {
+    "rooms": 2,
+    "area_m2": 54.0,
+    "floor": 4,
+    "floor_at_home": 9,
+    "house_type": "panel",
+    "renovation_type": "cosmetic",
+    "has_balcony": True,
+    "has_loggia": False,
+}
+
+
+def _canned_browser_responses() -> list[dict[str, Any]]:
+    """4 responses in call order: warm-up GET, coords A, position B, get-data C.
+
+    B and C bodies are the REAL live-captured Avito responses (fixtures/*.json);
+    A is synthesized with the SAME lat/lon as the fixture B (both refer to the same
+    ул. Малышева, 125 lookup) so the flow is internally consistent end-to-end.
+    """
+    warmup = {"status": 200, "body": "<html>warm</html>"}
+    coords_a_body = {
+        "normalizedAddress": _ADDRESS,
+        "point": {"latitude": _LAT, "longitude": _LON},
+    }
+    coords_a = {"status": 200, "body": json.dumps(coords_a_body)}
+    position_b = {"status": 200, "body": json.dumps(_load_fixture("avito_imv_geo_position.json"))}
+    getdata_c = {"status": 200, "body": json.dumps(_load_fixture("avito_imv_getdata.json"))}
+    return [warmup, coords_a, position_b, getdata_c]
+
+
+def _run_legacy_evaluation() -> Any:
+    bf = AsyncMock()
+    bf.fetch_json = AsyncMock(side_effect=_canned_browser_responses())
+    return asyncio.run(legacy_evaluate_via_imv(address=_ADDRESS, browser_fetcher=bf, **_LOT_PARAMS))
+
+
+def _run_kit_evaluation() -> Any:
+    bf = AsyncMock()
+    bf.fetch_json = AsyncMock(side_effect=_canned_browser_responses())
+    return asyncio.run(kit_evaluate_via_imv(address=_ADDRESS, browser_fetcher=bf, **_LOT_PARAMS))
+
+
+def test_legacy_and_kit_imv_evaluation_are_class_distinct() -> None:
+    """Sanity-check ДО harness'а — same reason as test_avito_detail_kit_parity.py:
+    legacy/kit IMVEvaluation are different classes (different modules), so plain
+    `==` is always False even on identical field values.
+    """
+    legacy_result = _run_legacy_evaluation()
+    kit_result = _run_kit_evaluation()
+
+    assert type(legacy_result) is not type(kit_result)
+    assert legacy_result != kit_result
+    assert legacy_result.recommended_price == kit_result.recommended_price
+    assert legacy_result.geo.geo_hash == kit_result.geo.geo_hash
+
+
+def test_evaluate_via_imv_golden_parity_real_fixtures() -> None:
+    """Proves kit evaluate_via_imv gives BYTE-IDENTICAL IMVEvaluation to legacy on a
+    real recorded Avito API response (no synthetic мокы для price/history/suggestions —
+    только step A синтетика для координат, зеркалящих ту же самую фикстуру B).
+    """
+    assert_parity(
+        legacy_fn=_run_legacy_evaluation,
+        kit_fn=_run_kit_evaluation,
+        fixtures=[()],
+    )
+
+
+def test_parity_harness_catches_imv_field_divergence() -> None:
+    """Доказывает, что harness реально ЛОВИТ расхождение на этой dataclass-форме —
+    мутируем kit-результат (recommended_price) и убеждаемся в ParityMismatchError,
+    называющем именно это поле (mirrors test_avito_detail_kit_parity.py pattern).
+    """
+    import dataclasses
+
+    legacy_result = _run_legacy_evaluation()
+    kit_result = _run_kit_evaluation()
+    mutated_kit_result = dataclasses.replace(
+        kit_result, recommended_price=kit_result.recommended_price + 1
+    )
+
+    with pytest.raises(ParityMismatchError) as exc_info:
+        assert_parity(
+            legacy_fn=lambda: legacy_result,
+            kit_fn=lambda: mutated_kit_result,
+            fixtures=[()],
+        )
+
+    assert "recommended_price" in str(exc_info.value)
+
+
+# ---------------------------------------------------------------------------
+# Config-footgun regression (#2334 CONFIRMED recon finding)
+# ---------------------------------------------------------------------------
+#
+# evaluate_via_imv's own-curl_cffi-session path (browser_fetcher=None, the default)
+# is the one that reads proxy config. Mirrors
+# tests/test_scraper_proxy.py::test_avito_imv_own_session_receives_proxies (legacy
+# reference) and tests/test_scraper_kit_pricehistory_session_parity.py::
+# test_fetch_detail_own_session_proxy_wiring_parity (kit config= precedent for a
+# sibling provider, #2306).
+
+
+def _mock_own_session_flow() -> MagicMock:
+    """Session double that lets warm-up + geocode-A pass, then fails at geocode-B
+    (missing geoFieldsHash) — raises IMVAddressNotFoundError AFTER the curl_cffi
+    session constructor already ran, which is all these tests need to capture.
+    """
+    mock_session = MagicMock()
+    mock_warmup_resp = MagicMock(status_code=200)
+    mock_session.get = AsyncMock(return_value=mock_warmup_resp)  # warm-up + geocode A
+    mock_geo_resp = MagicMock(status_code=200)
+    mock_geo_resp.json = MagicMock(return_value={"result": {"point": None}})  # no geoFieldsHash
+    mock_session.post = AsyncMock(return_value=mock_geo_resp)  # geocode B
+    mock_session.close = AsyncMock()
+    return mock_session
+
+
+async def _call_kit_own_session(*, config: Any = None) -> None:
+    with pytest.raises(KitIMVAddressNotFoundError):
+        await kit_evaluate_via_imv(
+            address="ул. Тургенева, 4",
+            rooms=2,
+            area_m2=50.0,
+            floor=3,
+            floor_at_home=9,
+            house_type="panel",
+            renovation_type="cosmetic",
+            has_balcony=False,
+            has_loggia=False,
+            config=config,
+        )
+
+
+def test_kit_evaluate_via_imv_without_config_drops_proxy(monkeypatch: pytest.MonkeyPatch) -> None:
+    """CONFIRMED footgun: kit evaluate_via_imv called WITHOUT config= builds its own
+    curl_cffi session with proxies=None — even though a real proxy IS configured in
+    settings — because `_env = config.scraper_proxy_url if config is not None else None`
+    never reads settings when config is absent (no fallback to env/settings inside kit).
+    """
+    monkeypatch.setattr(settings, "scraper_proxy_url_env", "http://test-proxy.local:8080")
+    mock_session = _mock_own_session_flow()
+    captured: dict[str, Any] = {}
+
+    def _fake_session(*args: Any, **kwargs: Any) -> MagicMock:
+        captured.update(kwargs)
+        return mock_session
+
+    with patch("curl_cffi.requests.AsyncSession", _fake_session):
+        asyncio.run(_call_kit_own_session(config=None))
+
+    assert captured.get("proxies") is None
+
+
+def test_kit_evaluate_via_imv_with_config_uses_proxy(monkeypatch: pytest.MonkeyPatch) -> None:
+    """With config=RealScraperConfig() (the established DI pattern for this epic) the
+    kit path reads the SAME settings.scraper_proxy_url as legacy and wires it into the
+    curl_cffi session identically — this is the regression guard: if someone later
+    "simplifies away" the config kwarg threading in evaluate_via_imv, this test fails
+    on `captured.get("proxies") is None` instead of the expected proxy dict.
+    """
+    monkeypatch.setattr(settings, "scraper_proxy_url_env", "http://test-proxy.local:8080")
+    mock_session = _mock_own_session_flow()
+    captured: dict[str, Any] = {}
+
+    def _fake_session(*args: Any, **kwargs: Any) -> MagicMock:
+        captured.update(kwargs)
+        return mock_session
+
+    with patch("curl_cffi.requests.AsyncSession", _fake_session):
+        asyncio.run(_call_kit_own_session(config=RealScraperConfig()))
+
+    assert captured.get("proxies") == {
+        "http": "http://test-proxy.local:8080",
+        "https": "http://test-proxy.local:8080",
+    }