fix(tradein/geocoder): темп Nominatim сдерживается перед запросом, а не после успеха (#2953) (#2954)

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

@@ -16,6 +16,7 @@ from __future__ import annotations
 import asyncio
 import logging
 import re
+import time
 from dataclasses import dataclass, replace
 from typing import Literal
 
@@ -29,6 +30,43 @@ from app.services import dadata
 
 logger = logging.getLogger(__name__)
 
+# ── Общий ограничитель темпа обращений к Nominatim (#2953) ──────────────────
+#
+# Политика Nominatim — 1 req/sec. Раньше её изображали три разрозненных
+# `asyncio.sleep(1.0)`, и ни один не давал ограничения на самом деле:
+#
+#   geocode()            — сон стоял ВНУТРИ `if result is not None`, то есть
+#                          только после УСПЕХА; после неудачи паузы не было;
+#   _nominatim_lookup()  — сон МЕЖДУ typo-вариантами, но не перед tier-1;
+#   цикл бэкфилла        — своей паузы не имеет вовсе.
+#
+# Для неразрешимого адреса получалось 5 запросов за 4 секунды, и сразу
+# следующий адрес без паузы. А неразрешимые адреса — не редкость: на
+# 20.08.2026 из 1471 активного объявления без координат 1424 лежали в
+# семидневном backoff, то есть череда неудач случается КАЖДЫЙ прогон
+# бэкфилла. Отсюда 194 события «429 Too many requests» за неделю.
+#
+# Здесь пауза берётся ПЕРЕД каждым запросом и одна на все точки вызова, а не
+# после успеха и не на каждую по отдельности.
+#
+# ponytail: ограничитель внутрипроцессный. Если бэкфилл и пользовательские
+# запросы разъедут по разным процессам, их темпы снова сложатся — тогда
+# понадобится общий счётчик (Redis). Пока доминирующий источник один
+# (бэкфилл, сотни запросов за прогон), внутрипроцессного достаточно.
+_NOMINATIM_MIN_INTERVAL_SEC = 1.0
+_nominatim_gate = asyncio.Lock()
+_nominatim_last_call_at = 0.0
+
+
+async def _nominatim_throttle() -> None:
+    """Держит паузу ≥ _NOMINATIM_MIN_INTERVAL_SEC между обращениями к Nominatim."""
+    global _nominatim_last_call_at
+    async with _nominatim_gate:
+        overdue = _NOMINATIM_MIN_INTERVAL_SEC - (time.monotonic() - _nominatim_last_call_at)
+        if overdue > 0:
+            await asyncio.sleep(overdue)
+        _nominatim_last_call_at = time.monotonic()
+
 
 # ── Result type ──────────────────────────────────────────────────────────────
 @dataclass(frozen=True, slots=True)
@@ -712,6 +750,7 @@ async def _nominatim_query(client: httpx.AsyncClient, address: str) -> dict | No
     (`address.state`) — отсекает кандидатов ЯВНО из другого региона (Тюмень и
     т.п.), даже если координаты попали в генеральный bbox.
     """
+    await _nominatim_throttle()
     response = await client.get(
         "https://nominatim.openstreetmap.org/search",
         params={
@@ -790,7 +829,6 @@ async def _nominatim_lookup(address: str, city_hint: str | None = None) -> Geoco
         # Tier 2: typo-variants
         if item is None:
             for variant in _typo_variants(address, limit=4):
-                await asyncio.sleep(1.0)  # Nominatim 1 req/sec policy
                 variant_city, _ = _resolve_city_for_geocode(variant, city_hint)
                 variant_query = f"{variant_city}, {variant}" if variant_city else variant
                 item = await _nominatim_query(client, variant_query)
@@ -881,6 +919,7 @@ async def _dadata_suggest(query: str, limit: int = 8) -> list[GeocodeSuggestion]
 
 async def _nominatim_query_multi(client: httpx.AsyncClient, query: str, limit: int) -> list[dict]:
     """Один Nominatim search с фильтром по bbox области (region 66). Возвращает up to N items."""
+    await _nominatim_throttle()
     response = await client.get(
         "https://nominatim.openstreetmap.org/search",
         params={
@@ -952,7 +991,6 @@ async def _nominatim_query_city_aware(
     if city_specified:
         return await _nominatim_query_multi(client, query, limit)
     ekb_data = await _nominatim_query_multi(client, f"{query}, Екатеринбург", limit)
-    await asyncio.sleep(1.0)  # Nominatim 1 req/sec policy — два запроса подряд
     bare_data = await _nominatim_query_multi(client, query, limit)
     return _dedupe_nominatim_items(ekb_data, bare_data)[:limit]
 
@@ -986,7 +1024,6 @@ async def _nominatim_suggest(
         # Tier 2: typo-варианты если оригинал пустой
         if not data:
             for variant in _typo_variants(query, limit=3):
-                await asyncio.sleep(1.0)  # Nominatim 1 req/sec
                 variant_city, variant_specified = _resolve_city_for_geocode(variant, city_hint)
                 data = await _nominatim_query_city_aware(
                     client, variant, variant_city, variant_specified, limit
@@ -1891,8 +1928,6 @@ async def _geocode_resolve(
             result = replace(result, city_ambiguous=city_ambiguous)
             await asyncio.to_thread(_cache_put, db, addr_norm, result)
             logger.info("geocode nominatim: %s → (%.5f, %.5f)", addr_norm, result.lat, result.lon)
-            # Nominatim rate-limit policy: 1 req/sec — спим после успешного запроса
-            await asyncio.sleep(1.0)
             return result
     except Exception:
         logger.exception("nominatim geocoder failed")
@@ -2018,6 +2053,7 @@ async def _nominatim_reverse(lat: float, lon: float) -> ReverseGeocodeResult | N
         "Accept-Language": "ru,en;q=0.8",
     }
     async with httpx.AsyncClient(timeout=10.0, headers=headers) as client:
+        await _nominatim_throttle()
         response = await client.get(
             "https://nominatim.openstreetmap.org/reverse",
             params={

tradein-mvp/backend/tests/test_2953_nominatim_throttle.py

@@ -0,0 +1,137 @@
+"""Темп обращений к Nominatim ограничен ПЕРЕД запросом, а не после успеха (#2953).
+
+Политику 1 req/sec раньше изображали три разрозненных `asyncio.sleep(1.0)`, и ни
+один не давал ограничения на самом деле: в `geocode()` сон стоял внутри
+`if result is not None` (то есть только после УСПЕХА), в `_nominatim_lookup` —
+между typo-вариантами, но не перед tier-1, а цикл бэкфилла
+(`tasks/geocode_missing.py`) своей паузы не имел вовсе. Для неразрешимого адреса
+выходило 5 запросов за 4 секунды и сразу следующий адрес без паузы — 194 события
+«429 Too many requests» за неделю на проде.
+
+Тест проверяет ПОВЕДЕНИЕ — расстояние между соседними исходящими запросами, — а
+не наличие нового символа. Это принципиально: `from app.services.geocoder import
+_nominatim_throttle` на origin/main дал бы ImportError, то есть «возможности
+нет», а не «значение неверное», и красный ничего бы не доказывал. Здесь на
+origin/main тесты падают именно на замере: соседние запросы идут вплотную.
+
+Интервал подменяется на малый (`raising=False` — на origin/main такого атрибута
+просто нет, подмена там безвредна), иначе тест ждал бы секунды реального времени.
+"""
+
+from __future__ import annotations
+
+import os
+
+os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
+
+import time
+from itertools import pairwise
+from unittest.mock import patch
+
+import httpx
+import pytest
+
+from app.services import geocoder
+from app.services.geocoder import _nominatim_query
+
+# Тестовый интервал: достаточно велик, чтобы отличаться от нуля на любом железе,
+# и достаточно мал, чтобы тест шёл десятки миллисекунд, а не секунды.
+_TEST_INTERVAL = 0.08
+
+_REAL_ASYNC_CLIENT = httpx.AsyncClient
+
+# Пустой ответ: Nominatim ничего не нашёл. Именно НЕУДАЧНЫЙ путь и разгонял темп —
+# сон-после-успеха на нём не срабатывал.
+_EMPTY_BODY: list[dict] = []
+
+
+def _timestamping_transport(stamps: list[float]) -> httpx.MockTransport:
+    def handler(_request: httpx.Request) -> httpx.Response:
+        stamps.append(time.monotonic())
+        return httpx.Response(200, json=_EMPTY_BODY)
+
+    return httpx.MockTransport(handler)
+
+
+def _gaps(stamps: list[float]) -> list[float]:
+    return [b - a for a, b in pairwise(stamps)]
+
+
+@pytest.fixture
+def throttle_reset(monkeypatch: pytest.MonkeyPatch) -> None:
+    """Малый интервал + сброс «времени последнего вызова» между тестами.
+
+    Без сброса второй тест в файле унаследовал бы отметку от первого и мог бы
+    пройти/упасть по чужой причине.
+    """
+    monkeypatch.setattr(geocoder, "_NOMINATIM_MIN_INTERVAL_SEC", _TEST_INTERVAL, raising=False)
+    monkeypatch.setattr(geocoder, "_nominatim_last_call_at", 0.0, raising=False)
+
+
+async def test_consecutive_queries_are_spaced(throttle_reset: None) -> None:
+    """Три запроса подряд разнесены не меньше чем на интервал.
+
+    На origin/main `_nominatim_query` не содержит паузы вообще, запросы уходят
+    вплотную — тест падает на первом же зазоре.
+    """
+    stamps: list[float] = []
+    transport = _timestamping_transport(stamps)
+
+    async with _REAL_ASYNC_CLIENT(transport=transport) as client:
+        for _ in range(3):
+            await _nominatim_query(client, "заведомо ненаходимый адрес")
+
+    assert len(stamps) == 3, f"ожидали 3 запроса, ушло {len(stamps)}"
+    gaps = _gaps(stamps)
+    assert all(g >= _TEST_INTERVAL * 0.9 for g in gaps), (
+        f"запросы идут вплотную: зазоры {[round(g, 4) for g in gaps]}, "
+        f"ожидалось ≥ {_TEST_INTERVAL}"
+    )
+
+
+async def test_failed_lookup_does_not_reset_the_pace(throttle_reset: None) -> None:
+    """Два НЕУДАЧНЫХ поиска подряд не разгоняют темп на стыке.
+
+    Это ровно тот путь, который темп не сдерживал: сон стоял после успеха, а
+    неудача уходила к следующему адресу без паузы. Проверяем зазор МЕЖДУ
+    вызовами — между последним запросом первого и первым запросом второго.
+    """
+    stamps: list[float] = []
+    transport = _timestamping_transport(stamps)
+
+    with patch(
+        "app.services.geocoder.httpx.AsyncClient",
+        lambda *a, **k: _REAL_ASYNC_CLIENT(transport=transport),
+    ):
+        await geocoder._nominatim_lookup("ненаходимый адрес один")
+        boundary_index = len(stamps)
+        await geocoder._nominatim_lookup("ненаходимый адрес два")
+
+    assert boundary_index > 0 and len(stamps) > boundary_index, (
+        f"оба поиска должны были сходить в сеть: {len(stamps)} запросов, "
+        f"граница {boundary_index}"
+    )
+    boundary_gap = stamps[boundary_index] - stamps[boundary_index - 1]
+    assert boundary_gap >= _TEST_INTERVAL * 0.9, (
+        f"на стыке двух неудачных поисков зазор {boundary_gap:.4f}с "
+        f"(ожидалось ≥ {_TEST_INTERVAL}) — темп сбрасывается именно после неудачи"
+    )
+
+
+async def test_first_call_is_not_delayed(throttle_reset: None) -> None:
+    """Контроль: первый запрос не ждёт впустую.
+
+    Если бы «время последнего вызова» инициализировалось текущим временем, каждый
+    холодный старт платил бы интервал ни за что. Тест сломается и в случае, если
+    интервал по недосмотру станет применяться дважды за запрос.
+    """
+    stamps: list[float] = []
+    transport = _timestamping_transport(stamps)
+
+    started = time.monotonic()
+    async with _REAL_ASYNC_CLIENT(transport=transport) as client:
+        await _nominatim_query(client, "первый запрос")
+    elapsed = time.monotonic() - started
+
+    assert len(stamps) == 1
+    assert elapsed < _TEST_INTERVAL, f"первый запрос прождал {elapsed:.4f}с — это лишнее"