fix(tradein/geocode): геокодировать только активные объявления (#2604)

Ночная очередь geocode_missing_listings была на 98.5% забита is_active=false
объявлениями чужих регионов (Новосибирск/Казань/Челябинск/Тюмень/Ижевск...) без
улицы и дома. ORDER BY listings_count DESC ставил такой мусор в начало очереди
(у 'Новосибирская обл.,Новосибирск' — 214 listings, у реального адреса — 1-2),
поэтому Nominatim-бюджет (1 req/sec) съедался мусором и до активных адресов
дело не доходило: 8 ночных прогонов подряд saved=0.

Добавлен AND is_active в SELECT. UPDATE (lat/lon и оба tried_at) намеренно
оставлены без этого фильтра — координаты и backoff-метка принадлежат паре
(address, city) как тексту, не конкретному listing; is_active=false дубликат
той же пары и так навсегда исключён из будущих SELECT, а unfiltered UPDATE
проставляет ему ответ бесплатно (Nominatim-вызов уже оплачен активным
листингом) на случай реактивации.

Refs #2604

tradein-mvp/backend/app/tasks/geocode_missing.py

@@ -12,6 +12,14 @@ Pattern: dedup по паре (address, city) — 1 уникальная пара
 не схлопываться в один geocode-вызов и один UPDATE по тексту адреса).
 Rate limit: Nominatim 1 req/sec (#2593: Yandex Geocoder tier удалён из geocoder).
 
+SELECT фильтрует `is_active` (#2604 п.1): на проде очередь была на 98.5% забита
+мёртвыми объявлениями чужих регионов (Новосибирск/Казань/Челябинск/…) без is_active —
+`ORDER BY listings_count DESC` ставил их В НАЧАЛО (у мусорного адреса вида
+«Новосибирская обл.,Новосибирск» — сотни listings, у реального адреса — 1-2), поэтому
+весь batch-бюджет (Nominatim 1 req/sec) съедался мусором и до настоящих адресов дело
+не доходило (8 ночных прогонов подряд: saved=0). UPDATE после успешного/неуспешного
+geocode НЕ фильтрует is_active — см. комментарии у соответствующих UPDATE ниже.
+
 Отличие от /admin/geocode-missing (per-ID):
   - Этот модуль группирует по (address, city) → меньше API calls (dedup), но не
     схлопывает разные города с одинаковым текстом адреса.
@@ -59,11 +67,14 @@ async def geocode_missing_listings(
     """Geocode listings с NULL coords (любой source).
 
     Steps:
-    1. SELECT address, city FROM listings WHERE lat IS NULL AND address IS NOT NULL
-       GROUP BY address, city ORDER BY COUNT(*) DESC LIMIT batch_size
+    1. SELECT address, city FROM listings WHERE lat IS NULL AND is_active
+       AND address IS NOT NULL GROUP BY address, city ORDER BY COUNT(*) DESC
+       LIMIT batch_size
        (приоритет парам address+city с большим числом listings — больший ROI per
        geocode call; группировка по паре, НЕ только по address — #2594 шаг 2/3:
-       один и тот же текст адреса в разных городах — разные записи)
+       один и тот же текст адреса в разных городах — разные записи. `is_active` —
+       #2604 п.1: не тратим Nominatim-бюджет на мёртвые объявления, которые никогда
+       не попадут в выдачу пользователю)
 
     2. Для каждой пары (address, city):
        - geocode(address, db, city_hint=city) — auto-cache (hit или miss)
@@ -98,6 +109,14 @@ async def geocode_missing_listings(
     # переезд адреса в кэше или смена провайдера), либо tried_at IS NULL (ещё не пробовали).
     # Это делает функцию loop-safe: при вызове несколько раз в одном прогоне
     # failed-пары не переотбираются бесконечно.
+    #
+    # AND is_active (#2604 п.1) — очередь без этого фильтра на 98.5% состояла из
+    # is_active=false объявлений чужих регионов (Новосибирск/Казань/Челябинск/…),
+    # а ORDER BY listings_count DESC ставил самый мусорный адрес («Новосибирская
+    # обл.,Новосибирск», сотни listings) В НАЧАЛО — весь batch съедался мусором,
+    # который пользователь никогда не увидит (is_active=false), 8 ночных прогонов
+    # подряд saved=0. Активные объявления с валидным адресом почти всегда попадают
+    # в topN только теперь, когда мусор не конкурирует за место в LIMIT.
     rows = (
         db.execute(
             text(
@@ -105,6 +124,7 @@ async def geocode_missing_listings(
             SELECT address, city, COUNT(*) AS listings_count
             FROM listings
             WHERE lat IS NULL
+              AND is_active
               AND address IS NOT NULL
               AND length(trim(address)) >= 5
               AND (geocode_tried_at IS NULL
@@ -150,6 +170,14 @@ async def geocode_missing_listings(
                 # IS NOT DISTINCT FROM — city=NULL это отдельная группа, обычное
                 # `=` не поймает NULL-город и не должно задеть другой город с тем
                 # же текстом адреса.
+                # Намеренно БЕЗ `AND is_active` (#2604 п.2): tried_at — backoff-метка
+                # для (address, city) КАК ТЕКСТА, а не для конкретного listing.
+                # is_active=false дубликат этой пары и так никогда не будет выбран
+                # SELECT'ом заново (is_active=false исключён там навсегда) — фильтр
+                # здесь был бы no-op для неактивных строк. Единственный случай когда
+                # это имеет значение — если строка позже реактивируется (is_active
+                # → true): тогда tried_at уже стоит и backoff корректно защищает от
+                # немедленного повторного запроса того же заведомо неудачного адреса.
                 db.execute(
                     text(
                         "UPDATE listings SET geocode_tried_at = NOW()"
@@ -171,6 +199,11 @@ async def geocode_missing_listings(
             )
             if not dry_run:
                 # Пометить tried_at — geocoder не нашёл адрес, backoff 7 дней.
+                # Намеренно БЕЗ `AND is_active` (#2604 п.2) — то же обоснование, что
+                # и в except-ветке выше: backoff привязан к тексту (address, city),
+                # не к конкретному listing, is_active=false строка и так не выбирается
+                # SELECT'ом заново; при реактивации backoff корректно защитит от
+                # немедленного повтора заведомо неудачного запроса.
                 db.execute(
                     text(
                         "UPDATE listings SET geocode_tried_at = NOW()"
@@ -211,6 +244,18 @@ async def geocode_missing_listings(
         # city IS NOT DISTINCT FROM :city — обновляем ТОЛЬКО пару (address, city), из
         # которой был geocode-запрос; иначе тот же текст адреса в другом городе
         # (city IS NULL или другой явный город) перезаписался бы чужими координатами.
+        #
+        # Намеренно БЕЗ `AND is_active` (#2604 п.1): координаты — свойство физического
+        # адреса, а не свойство конкретного объявления. Если у этой же пары
+        # (address, city) есть is_active=false дубликат с lat IS NULL, он получит те же
+        # координаты бесплатно — Nominatim-вызов уже оплачен геокодом активного
+        # листинга, доп. запроса не будет. SELECT выше и так навсегда исключает
+        # is_active=false строки из очереди — без этого UPDATE такой дубликат остался
+        # бы с NULL lat/lon НАВСЕГДА (переезд в EKB-only локальные реестры/analytics по
+        # координатам сломан для него), хотя ответ уже есть в руках. Единственный
+        # довод «за» фильтр — консистентность с SELECT — не перевешивает: это не
+        # ошибка данных (координаты адреса объективны и не зависят от активности),
+        # а чистый выигрыш (та же строка при реактивации уже готова, доп. cost = 0).
         update_result = db.execute(
             text(
                 """
@@ -327,6 +372,13 @@ async def run_geocode_missing_listings(
                 )
                 break
             if res.addresses_total < batch_size:
+                # #2604 п.3: с is_active-фильтром в SELECT очередь резко уже (была
+                # 14294 строк/98.5% мёртвых, стало ~220 активных → десятки уникальных
+                # пар address+city после GROUP BY) — этот дренаж почти всегда сработает
+                # уже на первой итерации (addresses_total < default batch_size=200), и
+                # это ПРАВИЛЬНОЕ поведение: разгребли всё что было, ждём следующего
+                # прогона. Никакого деления тут нет (только сравнение int), пустая
+                # очередь (addresses_total=0) ловится веткой выше, а не этой.
                 logger.info(
                     "run_geocode_missing_listings: run_id=%d — дренаж "
                     "(addresses_total=%d < batch_size=%d), завершаем",

tradein-mvp/backend/tests/tasks/test_geocode_missing.py

@@ -342,6 +342,31 @@ async def test_geocode_missing_recent_tried_at_excluded_via_where() -> None:
     assert "7 days" in sql_text
 
 
+@pytest.mark.asyncio
+async def test_geocode_missing_select_filters_is_active() -> None:
+    """SELECT содержит `AND is_active` (#2604 п.1).
+
+    На проде очередь без этого фильтра была на 98.5% забита is_active=false
+    объявлениями чужих регионов (Новосибирск/Казань/Челябинск/…) без улицы и дома;
+    `ORDER BY listings_count DESC` ставил самый мусорный адрес («Новосибирская
+    обл.,Новосибирск», 214 listings) В НАЧАЛО очереди — весь Nominatim-бюджет
+    (1 req/sec) съедался мусором, до реальных активных адресов дело не доходило
+    (8 ночных прогонов подряд: saved=0). Falsification-проба: на коде ДО фикса
+    `"AND is_active" in sql_text` ложно, тест падает; после фикса проходит.
+    """
+    db = MagicMock()
+    select_result = MagicMock()
+    select_result.mappings.return_value.all.return_value = []
+    db.execute.return_value = select_result
+
+    with patch("app.tasks.geocode_missing.geocode", new_callable=AsyncMock):
+        await geocode_missing_listings(db, batch_size=10)
+
+    first_call = db.execute.call_args_list[0]
+    sql_text = str(first_call[0][0])
+    assert "AND is_active" in sql_text
+
+
 @pytest.mark.asyncio
 async def test_run_geocode_missing_listings_terminates_on_drained() -> None:
     """run_geocode_missing_listings завершается когда addresses_total == 0 (ничего pending)."""
@@ -560,6 +585,91 @@ async def test_geocode_missing_failed_pair_tried_at_update_scoped_to_city() -> N
     assert params["city"] == "Нижний Тагил"
 
 
+# ── #2604 п.1/п.2: UPDATE decisions — locked in by test, not just comment ────
+
+
+@pytest.mark.asyncio
+async def test_geocode_missing_success_update_not_filtered_by_is_active() -> None:
+    """Decision #2604 п.1 (UPDATE lat/lon): намеренно БЕЗ `is_active` в WHERE.
+
+    Координаты — свойство физического адреса (address, city), не свойство
+    конкретного listing. is_active=false дубликат ЭТОЙ ЖЕ пары никогда не будет
+    независимо отобран SELECT'ом (он навсегда исключён оттуда) — без unfiltered
+    UPDATE такой дубликат остался бы с NULL lat/lon навсегда, хотя ответ уже
+    получен и оплачен Nominatim-вызовом активного листинга.
+    """
+    rows = [{"address": "ул. Тестовая, 1", "city": "Екатеринбург", "listings_count": 2}]
+    db = MagicMock()
+    select_result = MagicMock()
+    select_result.mappings.return_value.all.return_value = rows
+    update_result = MagicMock()
+    update_result.rowcount = 2
+    db.execute.side_effect = [select_result, update_result]
+
+    with patch(
+        "app.tasks.geocode_missing.geocode",
+        new_callable=AsyncMock,
+        return_value=_make_geocode_result("nominatim"),
+    ):
+        await geocode_missing_listings(db, batch_size=200)
+
+    update_call = db.execute.call_args_list[1]
+    sql = str(update_call.args[0])
+    assert "is_active" not in sql
+
+
+@pytest.mark.asyncio
+async def test_geocode_missing_notfound_tried_at_update_not_filtered_by_is_active() -> None:
+    """Decision #2604 п.2 (geo is None → tried_at UPDATE): намеренно БЕЗ `is_active`.
+
+    tried_at — backoff-метка для (address, city) КАК ТЕКСТА, не для конкретного
+    listing; is_active=false дубликат и так никогда не переотбирается SELECT'ом.
+    Единственный сценарий где это важно — реактивация (is_active → true) той же
+    строки: backoff уже стоит и корректно защищает от немедленного повтора
+    заведомо неудачного адреса.
+    """
+    rows = [{"address": "несуществующий адрес", "city": None, "listings_count": 1}]
+    db = MagicMock()
+    select_result = MagicMock()
+    select_result.mappings.return_value.all.return_value = rows
+    tried_at_result = MagicMock()
+    db.execute.side_effect = [select_result, tried_at_result]
+
+    with patch(
+        "app.tasks.geocode_missing.geocode",
+        new_callable=AsyncMock,
+        return_value=None,
+    ):
+        await geocode_missing_listings(db, batch_size=200)
+
+    update_call = db.execute.call_args_list[1]
+    sql = str(update_call.args[0])
+    assert "is_active" not in sql
+
+
+@pytest.mark.asyncio
+async def test_geocode_missing_exception_tried_at_update_not_filtered_by_is_active() -> None:
+    """Decision #2604 п.2 (geocode() raises → tried_at UPDATE): та же логика, что и
+    в NOT-FOUND ветке выше — намеренно БЕЗ `is_active`, зафиксировано тестом."""
+    rows = [{"address": "ул. Битая, 99", "city": None, "listings_count": 1}]
+    db = MagicMock()
+    select_result = MagicMock()
+    select_result.mappings.return_value.all.return_value = rows
+    tried_at_result = MagicMock()
+    db.execute.side_effect = [select_result, tried_at_result]
+
+    with patch(
+        "app.tasks.geocode_missing.geocode",
+        new_callable=AsyncMock,
+        side_effect=RuntimeError("timeout"),
+    ):
+        await geocode_missing_listings(db, batch_size=200)
+
+    update_call = db.execute.call_args_list[1]
+    sql = str(update_call.args[0])
+    assert "is_active" not in sql
+
+
 # ── Integration-style: estimator Avito exclusion removed ─────────────────────