Merge pull request 'fix(tradein/data): убрать ложный region_code=66 у объявлений чужих городов (#2604)' (#2612) from fix/tradein-region-code-foreign-cities into main

tradein-mvp/backend/data/sql/200_region_code_foreign_cities.sql

@@ -0,0 +1,100 @@
+-- 200_region_code_foreign_cities.sql
+-- Issue #2604 п.2 — убрать ложную метку региона у объявлений Avito из чужих
+-- городов (Новосибирск, Казань, Челябинск, Тюмень и ещё ~1600 слагов).
+--
+-- ПРОБЛЕМА: 16930 строк listings (source='avito') несут region_code = 66
+-- (Свердловская обл.), хотя source_url указывает на город ВНЕ наших шести —
+-- это неправда. Строки — наследие массового заброса 18 июня (сплошной
+-- multi-city SERP-краул до появления гео-фильтра карточек, коммит
+-- f0264237, 20 июня), который с тех пор не проставлял target_city_slug на
+-- SERP-запрос и не отсеивал карточки чужих городов на этапе сбора. Канал
+-- давно закрыт (тот же класс проблемы, что чинили 196/197 для listings.city),
+-- новых таких строк не поступает — все 16930 сейчас is_active = false.
+--
+-- ПОЧЕМУ NULL, А НЕ НАСТОЯЩИЙ РЕГИОН: вывести реальный регион из текста
+-- адреса/URL можно было бы (slug города в source_url), но это требовало бы
+-- поддерживать растущий справочник ~1600 чужих региональных кодов ради
+-- колонки, которую сегодня не читает НИ ОДНА живая выборка (проверено grep:
+-- только исторические миграции 077_*/091_* и один комментарий). Честное
+-- «неизвестно» (NULL) дешевле и не создаёт вторую ложь взамен первой.
+--
+-- ПОЧЕМУ ТОЛЬКО AVITO: у cian/domklik/yandex region_code=66 определяется не
+-- заброс-механизмом чужого города (там его и не было), а параметром region=
+-- самого запроса (cian) / отсутствием городской привязки в URL вовсе
+-- (domklik/yandex) — то есть в подавляющем большинстве region_code=66 у них
+-- ВЕРНЫЙ. Среди них нашлось лишь 27 строк с адресом, похожим на чужой город
+-- (текстовый разбор, ненадёжный сигнал) — сознательно НЕ трогаем, отдельная
+-- задача при желании её довести.
+--
+-- ИСТОЧНИК СЛАГА: первый сегмент пути после хоста —
+--   https://www.avito.ru/nizhniy_tagil/kvartiry/... -> 'nizhniy_tagil'
+-- извлекается regex `substring(source_url from 'avito\.ru/([^/]+)/')` —
+-- тот же идиом, что и в 197 (проверено: 'www.' перед 'avito.ru' в общий
+-- матч не проваливается, слаг 'www' ни разу не извлёкся — все 45472
+-- source_url на проде имеют форму 'https://www.avito.ru/...'). Точный
+-- сегмент пути, НЕ `LIKE '%slug%'` — среди наших шести слагов нет
+-- подстрочных коллизий друг с другом (ekaterinburg, nizhniy_tagil,
+-- kamensk-uralskiy, pervouralsk, verhnyaya_pyshma, serov — все взаимно
+-- не substring), поэтому точное сравнение через WHERE ... NOT IN (...) над
+-- извлечённым сегментом безопасно.
+--
+-- Наши шесть слагов — АВИТОВСКОЕ написание (см. CityLocation(...).avito_slug
+-- в packages/scraper-kit/src/scraper_kit/orchestration/pipeline.py,
+-- CITY_LOCATIONS ~ строки 330-336 + EKB default для 'ekaterinburg'):
+--   kamensk-uralskiy   — ЧЕРЕЗ ДЕФИС (не 'kamensk_uralskiy', наш внутренний
+--                        city_slug/CITY_LOCATIONS-ключ — через подчёркивание)
+--   verhnyaya_pyshma   — БЕЗ 'k' (не 'verkhnyaya_pyshma', наш внутренний ключ)
+-- Побайтно сверено с 197_backfill_listings_city_from_url.sql, который решает
+-- ту же задачу маппинга avito_slug -> наши города.
+--
+-- ЗАМЕРЫ (SELECT, read-only, прод, перед миграцией):
+--   Наши шесть городов (НЕ должны попасть под UPDATE):        28542 строк
+--   Кандидаты на UPDATE (source='avito', НЕ наши 6, region_code=66):
+--                                                               16930 строк
+--     из них is_active = false:                                16930 (100%)
+--     из них region_code = 66 (единственное текущее значение): 16930 (100%)
+--   Avito-строк с region_code уже NULL среди кандидатов:            0
+--     (UPDATE их не задевает по построению — WHERE region_code IS NOT NULL)
+--   Avito-строк с нераспознаваемым source_url (слаг не извлёкся):    0
+--   total avito = 45472 = 28542 (наши 6) + 16930 (кандидаты) — сходится.
+--
+-- ПРОИЗВОДИТЕЛЬНОСТЬ: триггеры на listings — column-scoped
+-- (`listings_price_change_trg` на UPDATE OF price_rub,
+-- `listings_set_geom_trg` на UPDATE OF lat, lon) — UPDATE только по
+-- region_code их не пробуждает. Но `tsv` (GENERATED ALWAYS ... STORED над
+-- description+address) пересчитывается на КАЖДОМ UPDATE независимо от того,
+-- какие колонки менялись. EXPLAIN (без ANALYZE, план не исполняется) на
+-- проде показывает Bitmap Heap Scan по listings_source_idx (source='avito')
+-- — тот же путь доступа, что и в 197. 197 обновила 27706 строк с тем же tsv
+-- recalculation за 4.1с; здесь строк меньше (16930, ~61% от 27706) —
+-- ожидаемая длительность ~2.5-3с. Никакого DDL, GIST/geom не затронуты.
+--
+-- Idempotency: `AND region_code IS NOT NULL` — повторный прогон находит 0
+-- строк (все затронутые строки уже NULL после первого прогона), UPDATE
+-- становится no-op. WHERE ограничен ровно source='avito' и slug вне наших
+-- шести — наши города и другие источники никогда не попадают в scope.
+--
+-- ГРАНИЦЫ: НЕ трогает region_code наших шести городов, НЕ трогает
+-- cian/domklik/yandex/n1, НЕ трогает city/is_active/скраперы/
+-- DEFAULT_REGION_CODE. Ничего не удаляет, ничего не деактивирует. Только
+-- UPDATE одной колонки одной таблицы.
+--
+-- Dependencies: 002_core_tables.sql (listings.region_code — nullable int,
+-- без DEFAULT на уровне таблицы).
+
+BEGIN;
+
+UPDATE listings
+SET region_code = NULL
+WHERE source = 'avito'
+  AND region_code IS NOT NULL
+  AND substring(source_url from 'avito\.ru/([^/]+)/') NOT IN (
+      'ekaterinburg',
+      'nizhniy_tagil',
+      'kamensk-uralskiy',
+      'pervouralsk',
+      'verhnyaya_pyshma',
+      'serov'
+  );
+
+COMMIT;

tradein-mvp/backend/tests/test_migration_200_region_code_foreign_cities.py

@@ -0,0 +1,192 @@
+"""Static guards for migration 200 (issue #2604 п.2 — убрать ложный
+region_code=66 у объявлений Avito из чужих городов).
+
+Прод применяет data/sql построчно строго (ON_ERROR_STOP). Полный DB-прогон
+требует живой БД; здесь фиксируем структурные инварианты, которые ГАРАНТИРУЮТ
+идемпотентность, скоуп (только Avito, только чужие города, не наши шесть) и
+НЕдеструктивность к самим listings-строкам по построению.
+"""
+
+from __future__ import annotations
+
+import re
+from pathlib import Path
+
+_SQL_DIR = Path(__file__).resolve().parents[1] / "data" / "sql"
+_MIGRATION_200 = _SQL_DIR / "200_region_code_foreign_cities.sql"
+
+_OUR_SIX_SLUGS = (
+    "ekaterinburg",
+    "nizhniy_tagil",
+    "kamensk-uralskiy",
+    "pervouralsk",
+    "verhnyaya_pyshma",
+    "serov",
+)
+
+
+def _sql() -> str:
+    return _MIGRATION_200.read_text(encoding="utf-8")
+
+
+def _executable_sql() -> str:
+    """SQL без построчных `--`-комментариев — только исполняемый код."""
+    lines = []
+    for raw in _sql().splitlines():
+        code = raw.split("--", 1)[0]
+        if code.strip():
+            lines.append(code)
+    return "\n".join(lines)
+
+
+def _flat(text: str) -> str:
+    return re.sub(r"\s+", " ", text).strip().lower()
+
+
+def test_migration_200_exists() -> None:
+    assert _MIGRATION_200.exists(), f"missing migration: {_MIGRATION_200}"
+
+
+def test_migration_200_is_transactional() -> None:
+    sql = _sql()
+    assert "BEGIN;" in sql
+    assert "COMMIT;" in sql
+
+
+def test_migration_200_only_avito() -> None:
+    """WHERE ограничен source='avito' — cian/domklik/yandex/n1 не трогаются
+    (у них region_code=66 в основном верен; 27 подозрительных строк там —
+    сознательно вне scope этой миграции, ненадёжный сигнал)."""
+    flat = _flat(_executable_sql())
+    assert "where source = 'avito'" in flat
+
+
+def test_migration_200_idempotent_guard_present() -> None:
+    """`AND region_code IS NOT NULL` — повторный прогон находит 0 строк
+    (уже NULL после первого прогона), UPDATE становится no-op."""
+    flat = _flat(_executable_sql())
+    assert "and region_code is not null" in flat
+
+
+def test_migration_200_sets_null_not_a_guessed_region() -> None:
+    """SET region_code = NULL — честное «неизвестно», не подставной код
+    другого региона (мы не выводим регион из текста адреса)."""
+    flat = _flat(_executable_sql())
+    assert "set region_code = null" in flat
+
+
+def test_migration_200_excludes_exactly_our_six_cities() -> None:
+    """WHERE ... NOT IN покрывает ровно наши шесть слагов — не больше (не
+    расширяем защищённый список произвольно), не меньше (иначе один из наших
+    городов ложно попадёт под обнуление)."""
+    flat = _flat(_executable_sql())
+    for slug in _OUR_SIX_SLUGS:
+        assert f"'{slug}'" in flat, f"missing protected avito slug: {slug}"
+
+
+def test_migration_200_kamensk_slug_uses_dash_not_underscore() -> None:
+    """Avito отдаёт 'kamensk-uralskiy' (дефис) — НЕ наш внутренний city_slug
+    'kamensk_uralskiy' (подчёркивание, CITY_LOCATIONS ключ в pipeline.py).
+    Регресс на подчёркивание означал бы, что реальный Каменск-Уральский
+    ложно обнуляется этой миграцией."""
+    flat = _flat(_executable_sql())
+    assert "'kamensk-uralskiy'" in flat
+    assert "'kamensk_uralskiy'" not in flat
+
+
+def test_migration_200_pyshma_slug_matches_avito_not_internal_key() -> None:
+    """Avito слаг — 'verhnyaya_pyshma' (без 'k'), а не наш внутренний ключ
+    'verkhnyaya_pyshma' (с 'k', CITY_LOCATIONS в pipeline.py)."""
+    flat = _flat(_executable_sql())
+    assert "'verhnyaya_pyshma'" in flat
+    assert "'verkhnyaya_pyshma'" not in flat
+
+
+def test_migration_200_slugs_match_pipeline_source_of_truth() -> None:
+    """Шесть защищённых слагов побайтно совпадают с CityLocation(...)
+    .avito_slug в scraper_kit.orchestration.pipeline (CITY_LOCATIONS +
+    'ekaterinburg' EKB-дефолт) — иначе список разойдётся с источником
+    истины и миграция начнёт либо обнулять свои города, либо пропускать
+    чужие."""
+    pipeline_path = (
+        Path(__file__).resolve().parents[2]
+        / "packages"
+        / "scraper-kit"
+        / "src"
+        / "scraper_kit"
+        / "orchestration"
+        / "pipeline.py"
+    )
+    pipeline_src = pipeline_path.read_text(encoding="utf-8")
+
+    sql = _sql()
+    for slug in _OUR_SIX_SLUGS:
+        assert slug in sql, f"missing avito slug in migration: {slug}"
+        # 'ekaterinburg' — EKB-дефолт, в pipeline.py не встречается как
+        # avito_slug строкой (нет явного CityLocation для ЕКБ, city_slug=None
+        # -> _avito_slug fallback на city_slug), остальные пять — явные
+        # CityLocation(...).avito_slug значения в CITY_LOCATIONS.
+        if slug != "ekaterinburg":
+            assert slug in pipeline_src, (
+                f"avito_slug {slug!r} в миграции 200 не найден в pipeline.py "
+                "CITY_LOCATIONS — риск расхождения защищённого списка с "
+                "источником истины"
+            )
+
+
+def test_migration_200_no_substring_collision_between_slugs() -> None:
+    """Ни один из шести слагов не является подстрокой другого — точное
+    сравнение сегмента пути через NOT IN (...) безопасно, LIKE '%slug%' не
+    нужен и не используется."""
+    for a in _OUR_SIX_SLUGS:
+        for b in _OUR_SIX_SLUGS:
+            if a == b:
+                continue
+            assert a not in b, f"{a!r} is a substring of {b!r} — collision risk"
+
+    flat = _flat(_executable_sql())
+    assert "like '%" not in flat
+
+
+def test_migration_200_extracts_exact_path_segment() -> None:
+    """Слаг извлекается точным сегментом пути через substring(...) regex
+    (тот же идиом, что 197), не LIKE-паттерном."""
+    flat = _flat(_executable_sql())
+    assert "substring(source_url from 'avito" in flat
+
+
+def test_migration_200_no_ddl() -> None:
+    """Только UPDATE данных — никакого ALTER/CREATE/DROP."""
+    flat = _flat(_executable_sql())
+    assert "alter table" not in flat
+    assert "create table" not in flat
+    assert "drop table" not in flat
+    assert flat.count("update listings") == 1
+
+
+def test_migration_200_no_destructive_ddl() -> None:
+    """Миграция не должна содержать DROP TABLE / TRUNCATE / DELETE — ничего
+    не удаляется, ничего не деактивируется."""
+    flat = _flat(_executable_sql())
+    assert "drop table" not in flat
+    assert "truncate" not in flat
+    assert "delete from" not in flat
+    assert "is_active" not in flat
+
+
+def test_migration_200_does_not_touch_other_sources_or_city() -> None:
+    """Явно вне scope: cian/domklik/yandex/n1 и listings.city не
+    упоминаются в исполняемом SQL этой миграции."""
+    flat = _flat(_executable_sql())
+    assert "cian" not in flat
+    assert "domklik" not in flat
+    assert "yandex" not in flat
+    assert " n1 " not in flat
+    assert "set city" not in flat
+
+
+def test_migration_200_no_psycopg_trap() -> None:
+    """Никаких :param::type — psycopg v3 требует CAST(... AS type) (не
+    применимо в чистом .sql без bind params, но проверяем на регресс
+    copy-paste из Python-кода)."""
+    assert not re.search(r":\w+::", _sql())