Merge remote-tracking branch 'forgejo/main' into feat/tradein-651-652-accuracy

# Conflicts:
#	tradein-mvp/backend/app/core/config.py
#	tradein-mvp/backend/app/services/estimator.py

tradein-mvp/backend/app/api/v1/trade_in.py

@@ -61,7 +61,23 @@ async def estimate(
     """
     account_quota.check_and_raise(db, x_authenticated_user)
     from app.services.estimator import estimate_quality
-    result = await estimate_quality(payload, db, created_by=x_authenticated_user)
+
+    # #654: ранее любое исключение estimate_quality всплывало необработанным и
+    # маскировалось апстрим-прокси (Caddy) как непрозрачный 502. Ловим, логируем
+    # через logger.exception (→ GlitchTip/Sentry получает stack trace) и отдаём
+    # явный 503 — так любая БУДУЩАЯ реальная ошибка становится видимой, а не
+    # «глотается» шлюзом. HTTPException пробрасываем как есть (это не сбой).
+    # created_by (#656) прокидываем в estimate_quality для скоупа /history.
+    try:
+        result = await estimate_quality(payload, db, created_by=x_authenticated_user)
+    except HTTPException:
+        raise
+    except Exception:
+        logger.exception("estimate failed for address=%r", payload.address)
+        raise HTTPException(
+            status_code=503,
+            detail="estimate temporarily unavailable — try again shortly",
+        ) from None
     account_quota.increment(db, x_authenticated_user)
     return result
 

tradein-mvp/backend/app/core/config.py

@@ -67,6 +67,21 @@ class Settings(BaseSettings):
     estimate_imv_blend_weight: float = 0.5     # вес якоря в blend: median*(1-w)+A*w
     estimate_imv_blend_threshold: float = 1.15  # якорь должен быть > медианы ×1.15
 
+    # ── Estimate enrichment time-budgets (#654) ──────────────────────────────
+    # POST /estimate делает несколько ПОСЛЕДОВАТЕЛЬНЫХ блокирующих сетевых
+    # вызовов (geocode → Overpass → Yandex valuation → IMV → Cian). Yandex
+    # valuation (внутренний httpx timeout 30s) НЕ gated на наличие floor и
+    # выполняется на каждой оценке — главный подозреваемый на gateway-таймаут
+    # (Caddy 502/504). Эти budget'ы оборачивают самые медленные ungated-вызовы
+    # в asyncio.wait_for(): при превышении источник деградирует в None (тот же
+    # graceful-путь что и сетевая ошибка), а НЕ роняет весь /estimate в 5xx.
+    # Держать суммарный budget ниже Caddy read/write timeout (см.
+    # deploy/Caddyfile.tradein-fragment). ENV: ESTIMATE_YANDEX_VALUATION_TIMEOUT_S,
+    # ESTIMATE_GEOCODE_BUDGET_S, ESTIMATE_HOUSE_META_TIMEOUT_S.
+    estimate_yandex_valuation_timeout_s: float = 8.0
+    estimate_geocode_budget_s: float = 12.0
+    estimate_house_meta_timeout_s: float = 8.0
+
     # Лимит успешных оценок trade-in за календарный месяц на аккаунт (#658).
     # Конфигурируется через env ESTIMATE_QUOTA_LIMIT. Default 15.
     estimate_quota_limit: int = 15

tradein-mvp/backend/app/services/estimator.py

@@ -19,6 +19,7 @@
 
 from __future__ import annotations
 
+import asyncio
 import hashlib
 import json
 import logging
@@ -843,6 +844,26 @@ def _fetch_dkp_corridor(
     }
 
 
+# ── Time-budget guard (#654) ────────────────────────────────────────────────
+async def _with_budget(coro: Any, budget_s: float, *, label: str) -> Any:
+    """Await `coro` under an asyncio.wait_for() time budget.
+
+    On timeout the coroutine is cancelled and we return None — mapping a slow
+    upstream onto the SAME graceful "None" path these enrichments already take
+    on network error, so a single slow source degrades the estimate instead of
+    blowing the gateway read timeout (#654: opaque Caddy 502/504).
+
+    budget_s <= 0 disables the guard (await directly) — escape hatch via config.
+    """
+    if budget_s is None or budget_s <= 0:
+        return await coro
+    try:
+        return await asyncio.wait_for(coro, timeout=budget_s)
+    except TimeoutError:
+        # asyncio.TimeoutError is an alias of builtin TimeoutError (py3.11+).
+        logger.warning("%s exceeded %.1fs budget — degrading to None (#654)", label, budget_s)
+        return None
+
 # ── Public ───────────────────────────────────────────────────────────────────
 async def estimate_quality(
     payload: TradeInEstimateInput, db: Session, created_by: str | None = None
@@ -859,10 +880,15 @@ async def estimate_quality(
     Returns:
         AggregatedEstimate с estimate_id, медианой, диапазоном, аналогами.
     """
-    # 1. Geocode
+    # 1. Geocode (#654: time-budgeted — Yandex/Nominatim retry chain can stack
+    #    multiple network round-trips + 1s Nominatim rate-limit sleeps).
     geo: GeocodeResult | None = None
     if payload.address:
-        geo = await geocode(payload.address, db)
+        geo = await _with_budget(
+            geocode(payload.address, db),
+            settings.estimate_geocode_budget_s,
+            label="geocode",
+        )
 
     if geo is None:
         # Без координат не можем искать через PostGIS. Возвращаем low confidence.
@@ -903,10 +929,16 @@ async def estimate_quality(
     # 2. #392: обогащаем год / тип дома из картографии (OSM Overpass), если
     #    пользователь их не указал — это улучшает house-match аналогов (#6).
     #    Best-effort: при недоступности OSM target_* остаются None.
+    #    #654: time-budgeted — Overpass httpx timeout 15s сам по себе близок к
+    #    gateway-таймауту; деградируем в None при превышении budget.
     target_year = payload.year_built
     target_house_type = payload.house_type
     if target_year is None or target_house_type is None:
-        house_meta = await get_house_metadata(geo.lat, geo.lon, db)
+        house_meta = await _with_budget(
+            get_house_metadata(geo.lat, geo.lon, db),
+            settings.estimate_house_meta_timeout_s,
+            label="house_metadata(overpass)",
+        )
         if house_meta is not None:
             if target_year is None:
                 target_year = house_meta.year_built
@@ -1118,11 +1150,18 @@ async def estimate_quality(
         sources_used_pre = sorted(set(sources_used_pre) | {"avito_imv"})
 
     # ── Stage 8: Yandex Valuation as on-demand source (anonymous, cached 24h) ──
+    # #654: главный латентность-подозреваемый. Этот источник UNGATED — бежит на
+    # КАЖДОЙ оценке (в т.ч. без floor/total_floors), а его внутренний httpx
+    # timeout 30s + curl_cffi impersonation + sleep_between_requests могут одни
+    # превысить gateway read timeout → opaque 502. Оборачиваем в budget: при
+    # превышении → None (cache-hit путь быстрый, timeout бьёт только по медленному
+    # cache-miss fetch). Деградация идентична сетевой ошибке внутри функции.
     yandex_val: YandexValuationResult | None = None
     if geo is not None and geo.full_address:
-        yandex_val = await _get_or_fetch_yandex_valuation_cached(
+        yandex_val = await _with_budget(
-            db,
+            _get_or_fetch_yandex_valuation_cached(db, address=geo.full_address),
-            address=geo.full_address,
+            settings.estimate_yandex_valuation_timeout_s,
+            label="yandex_valuation",
         )
         if yandex_val is not None:
             sources_used_pre = sorted(set(sources_used_pre) | {"yandex_valuation"})

tradein-mvp/backend/tests/test_estimator_null_floor_timeout.py

@@ -0,0 +1,232 @@
+"""Regression tests for #654 — POST /estimate 502 on null floor / slow upstream.
+
+Two contracts locked here:
+  1. estimate with floor=None & total_floors=None returns an AggregatedEstimate
+     (no exception) AND the floor-gated sources (Avito IMV + Cian valuation) are
+     NOT awaited — this is the gating contract that makes null-floor safe.
+  2. A Yandex-valuation timeout (asyncio.TimeoutError / httpx.TimeoutException)
+     degrades gracefully: estimate WITHOUT 'yandex_valuation' in sources_used,
+     no 5xx. Locks the #654 time-budget → None mapping.
+
+Style mirrors test_estimator_cian_integration.py: MagicMock db, geocode /
+_fetch_analogs / _fetch_deals patched, scrapers stubbed.
+"""
+
+import os
+
+# Settings requires DATABASE_URL at init time. Set dummy DSN before any app import.
+os.environ.setdefault("DATABASE_URL", "postgresql://test:test@localhost/test_db")
+
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import anyio
+import httpx
+
+
+def _make_fake_geo():
+    from app.services.geocoder import GeocodeResult
+
+    return GeocodeResult(
+        lat=56.838,
+        lon=60.595,
+        full_address="Свердловская обл., Екатеринбург, ул. Учителей, 18",
+        provider="nominatim",
+    )
+
+
+def _make_payload_null_floor():
+    """Simplified-form payload: floor + total_floors are None (the #654 trigger)."""
+    from app.schemas.trade_in import TradeInEstimateInput
+
+    return TradeInEstimateInput(
+        address="ЕКБ, ул. Учителей, 18",
+        area_m2=38.8,
+        rooms=1,
+        floor=None,
+        total_floors=None,
+    )
+
+
+def _make_payload_full():
+    from app.schemas.trade_in import TradeInEstimateInput
+
+    return TradeInEstimateInput(
+        address="ЕКБ, ул. Учителей, 18",
+        area_m2=38.8,
+        rooms=1,
+        floor=4,
+        total_floors=16,
+    )
+
+
+def test_estimate_null_floor_skips_imv_and_cian() -> None:
+    """floor=None & total_floors=None → AggregatedEstimate, IMV+Cian NOT awaited.
+
+    Locks the gating contract: the two floor-required sources must be skipped
+    (not called) when floor data is absent — confirming the null path is safe
+    and the 502 was never a null-floor crash (#654).
+    """
+    from app.services.estimator import estimate_quality
+
+    db = MagicMock()
+    payload = _make_payload_null_floor()
+
+    imv_mock = AsyncMock(return_value=None)
+    cian_mock = AsyncMock(return_value=None)
+    yandex_mock = AsyncMock(return_value=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._fetch_analogs", return_value=([], False, "W")),
+            patch("app.services.estimator._fetch_deals", return_value=[]),
+            patch("app.services.estimator._get_or_fetch_imv_cached", new=imv_mock),
+            patch("app.services.estimator._get_or_fetch_yandex_valuation_cached",
+                  new=yandex_mock),
+            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)
+
+        # No exception — got a real estimate back.
+        assert result.estimate_id is not None
+        # Gating contract: floor-required sources never awaited.
+        imv_mock.assert_not_awaited()
+        cian_mock.assert_not_awaited()
+        assert "avito_imv" not in result.sources_used
+        assert "cian_valuation" not in result.sources_used
+
+    anyio.run(_run)
+
+
+def test_estimate_full_floor_awaits_imv_and_cian() -> None:
+    """Sanity counter-test: with floor present, IMV+Cian ARE awaited.
+
+    Guards against a regression where the gating predicate gets inverted /
+    broken — proves the null-floor skip above is the gate, not a no-op.
+    """
+    from app.services.estimator import estimate_quality
+
+    db = MagicMock()
+    payload = _make_payload_full()
+
+    imv_mock = AsyncMock(return_value=None)
+    cian_mock = AsyncMock(return_value=None)
+    yandex_mock = AsyncMock(return_value=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._fetch_analogs", return_value=([], False, "W")),
+            patch("app.services.estimator._fetch_deals", return_value=[]),
+            patch("app.services.estimator._get_or_fetch_imv_cached", new=imv_mock),
+            patch("app.services.estimator._get_or_fetch_yandex_valuation_cached",
+                  new=yandex_mock),
+            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 result.estimate_id is not None
+        cian_mock.assert_awaited_once()
+        # IMV is additionally gated on house_type/renovation mapping; with
+        # house_type=None it is correctly skipped — so we assert on Cian only,
+        # which is gated purely on floor/total_floors presence.
+
+    anyio.run(_run)
+
+
+def test_estimate_yandex_timeout_degrades_no_5xx() -> None:
+    """Yandex valuation raising TimeoutError → estimate without it, no exception.
+
+    The _with_budget() guard wraps the call; asyncio.wait_for surfaces a
+    TimeoutError which must map to the same graceful None path as a network
+    error. Estimator returns an AggregatedEstimate; 'yandex_valuation' absent.
+    """
+    from app.services.estimator import estimate_quality
+
+    db = MagicMock()
+    payload = _make_payload_null_floor()
+
+    # Yandex call hangs/raises a timeout — simulate the slow-upstream case.
+    yandex_mock = AsyncMock(side_effect=TimeoutError("yandex slow"))
+
+    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._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=yandex_mock),
+            patch("app.services.estimator.estimate_via_cian_valuation",
+                  new=AsyncMock(return_value=None)),
+            patch("app.services.estimator._get_asking_sold_ratio",
+                  return_value=(None, None)),
+        ):
+            result = await estimate_quality(payload, db)
+
+        assert result.estimate_id is not None  # no 5xx — degraded gracefully
+        assert "yandex_valuation" not in result.sources_used
+
+    anyio.run(_run)
+
+
+def test_estimate_yandex_httpx_timeout_at_scraper_degrades_no_5xx() -> None:
+    """Real network path: scraper raises httpx.TimeoutException on cache-miss fetch.
+
+    Here we DON'T patch _get_or_fetch_yandex_valuation_cached — we let the real
+    cache helper run with a cache MISS, then make the underlying scraper raise
+    httpx.TimeoutException. The helper's internal `except Exception` maps it to
+    None (graceful), so the estimate completes without 'yandex_valuation' and
+    no 5xx. This is the production-realistic timeout path (#654).
+    """
+    from app.services.estimator import estimate_quality
+
+    db = MagicMock()
+    # Cache MISS so the helper proceeds to the (timing-out) fresh fetch.
+    db.execute.return_value.mappings.return_value.first.return_value = None
+    payload = _make_payload_null_floor()
+
+    fake_scraper = MagicMock()
+    fake_scraper.__aenter__ = AsyncMock(return_value=fake_scraper)
+    fake_scraper.__aexit__ = AsyncMock(return_value=False)
+    fake_scraper.fetch_house_history = AsyncMock(
+        side_effect=httpx.TimeoutException("read timeout")
+    )
+
+    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._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.YandexValuationScraper",
+                  return_value=fake_scraper),
+            patch("app.services.estimator.estimate_via_cian_valuation",
+                  new=AsyncMock(return_value=None)),
+            patch("app.services.estimator._get_asking_sold_ratio",
+                  return_value=(None, None)),
+        ):
+            result = await estimate_quality(payload, db)
+
+        assert result.estimate_id is not None
+        assert "yandex_valuation" not in result.sources_used
+
+    anyio.run(_run)