From 3c5f535e6c01f644cf96069a8217a7615d6329dd Mon Sep 17 00:00:00 2001 From: bot-backend Date: Thu, 6 Aug 2026 03:57:46 +0500 Subject: [PATCH] =?UTF-8?q?fix(tradein/admin):=20=D0=B3=D0=B5=D0=B9=D1=82?= =?UTF-8?q?=20=D0=BE=D1=82=D0=BC=D0=B5=D0=BD=D1=8B=20=D0=BF=D0=BE=20=D0=B8?= =?UTF-8?q?=D1=81=D1=82=D0=BE=D1=87=D0=BD=D0=B8=D0=BA=D1=83,=20=D1=87?= =?UTF-8?q?=D0=B5=D1=81=D1=82=D0=BD=D1=8B=D0=B9=20=D0=BA=D0=BE=D0=BC=D0=BC?= =?UTF-8?q?=D0=B5=D0=BD=D1=82=D0=B0=D1=80=D0=B8=D0=B9=20view,=20=D0=BB?= =?UTF-8?q?=D0=B8=D0=BC=D0=B8=D1=82=2050=20(#2674)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ревью PR #2684 — четыре MINOR. 1. Починка фильтра открыла кнопку отмены на все 53 источника. Раньше таблица была пуста на каждой вкладке, поэтому кнопка не рендерилась НИ РАЗУ и дыра не проявлялась: ручки отмены source не проверяют вовсе. Оператор на вкладке Авито мог бы «отменить» refresh_search_matview — задача продолжила бы работать под статусом 'cancelled' (ещё один врущий статус ровно в тот день, когда их вычищаем), а has_running_run перестал бы держать single-run guard, который существует из-за инцидента с двойным свипом и баном (2026-05-31). Гейт поставлен на общем узле всех пяти ручек — scrape_runs.honors_cancel + отказ в mark_cancelled, — а не в UI: иначе ручной POST по-прежнему снимал бы guard. Флаг cancellable отдаётся в строке, UI по нему прячет кнопку. Состав набора выведен из call-site'ов runs.is_cancelled: city-sweep'ы (все площадки и города), full-load'ы, avito_newbuilding_sweep, rosreestr_dkp_import. Правило НЕ «любой *_sweep»: yandex_newbuilding_sweep отмену не опрашивает. 2. Комментарий пересозданного v_data_quality утверждал, что его обновляет /api/v1/admin/data-quality. Читателей у view нет ни одного — живая ручка строит свой запрос. PR с тезисом «ложный показатель хуже отсутствующего» не имеет права переносить в прод ложное утверждение о читателе. 3. Лимит выдачи 20 → 50: первые 20 строк по started_at на три четверти — сердцебиение proxy_healthcheck (1631 из 3245), часовой сбор мог не поместиться. Привязка к вкладке НЕ возвращается. 4. Тест «действующее определение view» искал маркер подстрокой с OR REPLACE — миграция с обычным CREATE VIEW или парой DROP+CREATE была бы невидима, и тест проверял бы 214, пока показатель уже вернулся. Заменено регуляркой на обе формы. Фальсификация трёх новых тестов патч-методом — все три красные. Полный прогон 3490 passed / 9 skipped, tsc --noEmit чистый. --- tradein-mvp/backend/app/api/v1/admin.py | 7 ++ .../backend/app/services/scrape_runs.py | 41 ++++++++- .../data/sql/214_drop_dead_run_metrics.sql | 9 +- .../tests/test_2674_dead_admin_metrics.py | 84 ++++++++++++++++++- tradein-mvp/backend/tests/test_city_sweep.py | 4 +- .../src/components/scrapers/RunsTable.tsx | 13 ++- 6 files changed, 149 insertions(+), 9 deletions(-) diff --git a/tradein-mvp/backend/app/api/v1/admin.py b/tradein-mvp/backend/app/api/v1/admin.py index 288e71b8..3777218c 100644 --- a/tradein-mvp/backend/app/api/v1/admin.py +++ b/tradein-mvp/backend/app/api/v1/admin.py @@ -2201,6 +2201,12 @@ class UnifiedScrapeRunRow(BaseModel): run_id: int source: str status: str + # #2674: чинить фильтр без этого флага было бы регрессом. Пока таблица была + # пуста на всех вкладках, кнопка отмены не рендерилась ни разу; теперь оператор + # видит все 53 источника — и без флага мог бы «отменить» задачу, которая отмену + # не опрашивает (см. scrape_runs.honors_cancel): статус соврал бы, а + # has_running_run перестал бы держать single-run guard. + cancellable: bool = False params: dict | None = None counters: dict | None = None total_seen: int | None = None @@ -2307,6 +2313,7 @@ def list_scrape_runs_unified( run_id=r["run_id"], source=r["source"], status=r["status"], + cancellable=runs_mod.honors_cancel(str(r["source"])), params=r.get("params"), counters=r.get("counters"), total_seen=r.get("total_seen"), diff --git a/tradein-mvp/backend/app/services/scrape_runs.py b/tradein-mvp/backend/app/services/scrape_runs.py index 9090601d..b89812e1 100644 --- a/tradein-mvp/backend/app/services/scrape_runs.py +++ b/tradein-mvp/backend/app/services/scrape_runs.py @@ -250,6 +250,27 @@ def update_heartbeat(db: Session, run_id: int, counters: dict[str, int]) -> None db.commit() +# Источники, чей джоб РЕАЛЬНО опрашивает status='cancelled' в своём цикле. +# Всё остальное отменить нельзя: строка стала бы 'cancelled', а задача продолжила бы +# работать — это, во-первых, ещё один врущий статус, во-вторых (хуже) обход guard'а +# has_running_run: он перестанет видеть прогон как running и пустит второй свип на том +# же прокси-IP → бан (инцидент 2026-05-31, runs #26+#27). +# Состав проверен по call-site'ам runs.is_cancelled: kit pipeline (city-sweep'ы всех +# площадок и городов, full-load'ы, avito_newbuilding_sweep) + rosreestr_dkp_import +# (scheduler.py). yandex_newbuilding_sweep отмену НЕ опрашивает — поэтому правило не +# «любой *_sweep». Актуально с #2674: до починки фильтра таблица прогонов была пуста +# на всех вкладках, кнопка отмены не рендерилась ни разу и дыра не проявлялась. +_CANCEL_HONORING_EXACT = frozenset({"avito_newbuilding_sweep", "rosreestr_dkp_import"}) +_CANCEL_HONORING_SUBSTRINGS = ("city_sweep", "full_load") + + +def honors_cancel(source: str) -> bool: + """True, если джоб этого source опрашивает отмену и реально остановится.""" + return source in _CANCEL_HONORING_EXACT or any( + key in source for key in _CANCEL_HONORING_SUBSTRINGS + ) + + def is_cancelled(db: Session, run_id: int) -> bool: """Проверить status='cancelled' (cooperative cancel в long-running pipeline).""" row = db.execute( @@ -374,7 +395,25 @@ def mark_banned(db: Session, run_id: int, error: str, counters: dict[str, int]) def mark_cancelled(db: Session, run_id: int) -> bool: - """Set status='cancelled' если currently 'running'. Returns True если cancelled.""" + """Set status='cancelled' если currently 'running'. Returns True если cancelled. + + Отказ (False) для source'ов, чей джоб отмену не опрашивает — см. honors_cancel: + там 'cancelled' был бы враньём в статусе и снял бы has_running_run-guard. + Ручки отмены source не проверяют (любая из пяти принимает любой run_id), поэтому + гейт стоит здесь — на общем узле всех пяти. + """ + row = db.execute( + text("SELECT source FROM scrape_runs WHERE id = :run_id"), + {"run_id": run_id}, + ).fetchone() + if row is not None and not honors_cancel(str(row.source)): + logger.warning( + "mark_cancelled отказ: run_id=%d source=%s не опрашивает отмену — " + "задача продолжила бы работать под статусом 'cancelled'", + run_id, + row.source, + ) + return False result = db.execute( text( """ diff --git a/tradein-mvp/backend/data/sql/214_drop_dead_run_metrics.sql b/tradein-mvp/backend/data/sql/214_drop_dead_run_metrics.sql index bfb456f1..5b8095e6 100644 --- a/tradein-mvp/backend/data/sql/214_drop_dead_run_metrics.sql +++ b/tradein-mvp/backend/data/sql/214_drop_dead_run_metrics.sql @@ -92,8 +92,15 @@ SELECT NOW() - (SELECT max(scraped_at) FROM listings WHERE source = 'yandex') AS yandex_last_scrape_ago, (SELECT count(*) FROM v_price_divergence) AS price_disagreements_count; +-- Комментарий из 095 утверждал, что view «refreshed on-demand by /api/v1/admin/ +-- data-quality endpoint». Это неправда с момента переписывания ручки: живой +-- /api/v1/admin/scraper/data-quality строит собственный запрос по listings/houses и +-- этого view не касается, читателей в коде нет ни одного (проверено #2674). PR, +-- тезис которого «ложный показатель хуже отсутствующего», не имеет права нести +-- ложное утверждение о читателе — пишем как есть. COMMENT ON VIEW v_data_quality IS - 'KPI snapshot. Refreshed on-demand by /api/v1/admin/data-quality endpoint (Master Plan sec 8.1). ' + 'KPI-снимок для РУЧНЫХ psql-запросов. Читателей в коде нет (проверено #2674): ' + '/api/v1/admin/scraper/data-quality считает свои метрики сам и этот view не трогает. ' '#2674: outliers_flagged убран — is_outlier не писал никто, «выброс» определён только ' 'внутри одной подборки аналогов (estimator._filter_outliers), не на объявлении.'; diff --git a/tradein-mvp/backend/tests/test_2674_dead_admin_metrics.py b/tradein-mvp/backend/tests/test_2674_dead_admin_metrics.py index 72178b9e..ced39ac9 100644 --- a/tradein-mvp/backend/tests/test_2674_dead_admin_metrics.py +++ b/tradein-mvp/backend/tests/test_2674_dead_admin_metrics.py @@ -187,6 +187,77 @@ def test_list_all_selects_no_dropped_columns(list_all) -> None: assert not still_there, f"list_all всё ещё выбирает дропнутые колонки: {still_there}" +# ══ 4b. Побочка починки фильтра: кнопка отмены открылась на все источники ═════ + + +def test_cancel_flag_true_only_for_jobs_that_poll_cancellation() -> None: + """honors_cancel = ровно те source'ы, чей джоб опрашивает runs.is_cancelled. + + 'yandex_newbuilding_sweep' в наборе НЕ должен быть, хотя и *_sweep: его таск + (app/tasks/yandex_newbuilding_sweep.py) отмену не опрашивает — поэтому правило + не может быть «любой sweep». + """ + honoring = [ + "avito_city_sweep", + "cian_city_sweep_nizhniy_tagil", + "domclick_city_sweep", + "avito_full_load_exhaustive", + "cian_full_load", + "avito_newbuilding_sweep", + "rosreestr_dkp_import", + ] + ignoring = [ + "proxy_healthcheck", + "deactivate_stale_avito", + "refresh_search_matview", + "sber_index_pull", + "yandex_newbuilding_sweep", + "house_imv_backfill", + ] + assert [s for s in honoring if not runs_mod.honors_cancel(s)] == [] + assert [s for s in ignoring if runs_mod.honors_cancel(s)] == [] + + +def test_row_carries_cancellable_so_ui_hides_the_button(client_factory) -> None: + """Строка отдаёт cancellable — без него UI показал бы «Отменить» у любого + running-прогона, включая proxy_healthcheck (1631 из 3245).""" + from unittest.mock import patch + + base = { + "status": "running", + "params": None, + "counters": None, + "total_seen": None, + "new_count": None, + "started_at": None, + "finished_at": None, + "heartbeat_at": None, + "error_text": None, + } + rows = [ + {"run_id": 1, "source": "avito_city_sweep", **base}, + {"run_id": 2, "source": "proxy_healthcheck", **base}, + ] + with patch("app.services.scrape_runs.list_all", return_value=(2, rows)): + r = client_factory(MagicMock()).get("/api/v1/admin/scrape/runs") + + assert r.status_code == 200 + assert [row["cancellable"] for row in r.json()["rows"]] == [True, False] + + +def test_mark_cancelled_refuses_non_cooperating_source() -> None: + """Гейт на общем узле всех пяти ручек отмены: 'cancelled' у задачи, которая + отмену не опрашивает, — это враньё в статусе И снятие has_running_run-guard + (второй свип на том же прокси → бан, инцидент 2026-05-31).""" + db = MagicMock() + db.execute.return_value.fetchone.return_value = MagicMock(source="proxy_healthcheck") + + assert runs_mod.mark_cancelled(db, 42) is False + # UPDATE не выполнялся — только SELECT source. + assert db.execute.call_count == 1 + db.commit.assert_not_called() + + # ══ 1-3. Схема: колонок больше нет, и v_data_quality не рапортует выбросы ═════ @@ -211,11 +282,18 @@ def test_latest_v_data_quality_no_longer_reports_outliers() -> None: Red на origin/main: там последним был 095_dead_schema.sql со строкой `(SELECT count(*) FROM listings WHERE is_outlier = true) AS outliers_flagged` — показатель, который не мог быть ненулевым, потому что колонку не писал никто. + + Ищем обе формы DDL (`CREATE VIEW` и `CREATE OR REPLACE VIEW`): миграция с парой + DROP+CREATE иначе оказалась бы невидимой, и тест продолжил бы проверять эту + миграцию, пока показатель уже вернулся в прод. Порядок = лексикографический: + деплой применяет файлы отсортированными, последний по имени — последний в проде. """ - marker = "CREATE OR REPLACE VIEW v_data_quality" - creators = sorted(p for p in _SQL_DIR.glob("*.sql") if marker in p.read_text(encoding="utf-8")) + marker = re.compile(r"CREATE\s+(?:OR\s+REPLACE\s+)?VIEW\s+v_data_quality\b") + creators = sorted(p for p in _SQL_DIR.glob("*.sql") if marker.search(p.read_text("utf-8"))) assert creators, "не найдено ни одной миграции, создающей v_data_quality" latest = creators[-1].read_text(encoding="utf-8") - body = latest.split("CREATE OR REPLACE VIEW v_data_quality")[-1].split(";")[0] + hit = marker.search(latest) + assert hit is not None + body = latest[hit.end() :].split(";")[0] assert "outliers_flagged" not in body assert "is_outlier" not in body diff --git a/tradein-mvp/backend/tests/test_city_sweep.py b/tradein-mvp/backend/tests/test_city_sweep.py index 2e763b20..3f132e1f 100644 --- a/tradein-mvp/backend/tests/test_city_sweep.py +++ b/tradein-mvp/backend/tests/test_city_sweep.py @@ -150,7 +150,9 @@ def test_scrape_runs_mark_cancelled_returns_bool() -> None: from app.services.scrape_runs import mark_cancelled mock_db = MagicMock() - mock_db.execute.return_value.fetchone.return_value = MagicMock() # row found + # source обязателен: #2674 добавил гейт honors_cancel — отменять можно только то, + # что отмену опрашивает (иначе 'cancelled' у живой задачи + обход has_running_run). + mock_db.execute.return_value.fetchone.return_value = MagicMock(source="avito_city_sweep") result = mark_cancelled(mock_db, 10) assert result is True diff --git a/tradein-mvp/frontend/src/components/scrapers/RunsTable.tsx b/tradein-mvp/frontend/src/components/scrapers/RunsTable.tsx index a94ef81e..ceb10b56 100644 --- a/tradein-mvp/frontend/src/components/scrapers/RunsTable.tsx +++ b/tradein-mvp/frontend/src/components/scrapers/RunsTable.tsx @@ -16,6 +16,8 @@ export interface ScrapeRunFull { run_id: number; source: string; status: string; + /** Джоб этого источника реально опрашивает отмену (бэкенд, scrape_runs.honors_cancel). */ + cancellable: boolean; params: Record | null; counters: Record | null; total_seen: number | null; @@ -68,7 +70,10 @@ function useScrapeRunSources() { }); } -function useScraperRuns(status: RunStatusFilter, sourceFilter: string, limit = 20) { +// limit=50 (API допускает 200): при выдаче по всем источникам первые 20 строк по +// started_at — на три четверти сердцебиение proxy_healthcheck (1631 прогон из 3245), +// и часовой сбор мог не поместиться на страницу (#2674). +function useScraperRuns(status: RunStatusFilter, sourceFilter: string, limit = 50) { return useQuery({ queryKey: ["scrape-runs", status, sourceFilter, limit], queryFn: () => { @@ -208,7 +213,7 @@ export function RunsTable({ source }: RunsTableProps) {

История прогонов

- Последние 20 прогонов по всем источникам. Автообновление каждые 8 сек. + Последние 50 прогонов по всем источникам. Автообновление каждые 8 сек.

{/* Source filter */} @@ -377,7 +382,9 @@ export function RunsTable({ source }: RunsTableProps) { )} - {r.status === "running" && ( + {/* cancellable — от бэкенда (#2674): у остальных источников + отмена поставила бы статус 'cancelled' работающей задаче. */} + {r.status === "running" && r.cancellable && (