From 769a36098b3ff2886eb7bba3a5bedde4009a5426 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sun, 6 Sep 2026 00:48:08 +0500 Subject: [PATCH 1/2] =?UTF-8?q?fix(tradein):=20=D1=81=D1=87=D0=B8=D1=82?= =?UTF-8?q?=D0=B0=D1=82=D1=8C=20domklik=20diff=5Fpercent=20=D0=B8=D0=B7=20?= =?UTF-8?q?=D1=86=D0=B5=D0=BD,=20=D0=BE=D1=82=D0=B2=D0=B5=D1=80=D0=B3?= =?UTF-8?q?=D0=B0=D1=82=D1=8C=20=D0=BD=D0=B5-=D0=BF=D1=80=D0=BE=D1=86?= =?UTF-8?q?=D0=B5=D0=BD=D1=82=D1=8B=20(#3225)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit offer_price_history.diff_percent у domklik содержал РУБЛИ: загрузчик карточки клал поле источника priceHistory.diff как есть, а clamp_diff_percent только зажимал его в ±999999.99 — заведомо неправдоподобное значение проходило молча. Прод 29.08.2026: 8900 из 11 075 непустых значений с |diff| > 50, p50 = -50 010. - парсер Домклика считает процент сам из соседних price_rub (сортировка по change_time); у самой ранней записи предыдущей цены нет -> NULL, не 0; - clamp_diff_percent -> validate_diff_percent: |x| > 100 не зажимается, а отвергается (NULL + warning с listing_id и сырым значением). Гейт стоит в общем хелпере, поэтому закрывает и cian-путь; - миграция 285 пересчитывает уже собранные domklik-строки оконной lag() по (listing_id, change_time); идемпотентна (UPDATE только IS DISTINCT FROM). Closes #3225 --- .../285_domklik_diff_percent_recompute.sql | 94 +++++++++++++++++++ .../tests/scrapers/test_domclick_detail.py | 87 +++++++++++------ .../src/scraper_kit/offer_price_history.py | 57 +++++++---- .../src/scraper_kit/providers/cian/detail.py | 4 +- .../scraper_kit/providers/domclick/detail.py | 24 +++-- 5 files changed, 211 insertions(+), 55 deletions(-) create mode 100644 tradein-mvp/backend/data/sql/285_domklik_diff_percent_recompute.sql diff --git a/tradein-mvp/backend/data/sql/285_domklik_diff_percent_recompute.sql b/tradein-mvp/backend/data/sql/285_domklik_diff_percent_recompute.sql new file mode 100644 index 00000000..2acaaf57 --- /dev/null +++ b/tradein-mvp/backend/data/sql/285_domklik_diff_percent_recompute.sql @@ -0,0 +1,94 @@ +-- 285_domklik_diff_percent_recompute.sql +-- Пересчитать offer_price_history.diff_percent у domklik из соседних price_rub (#3225). +-- +-- ЧТО БЫЛО НЕ ТАК +-- Загрузчик карточки Домклика клал в diff_percent поле источника priceHistory.diff, +-- а там РУБЛИ, не проценты. Замер на проде 29.08.2026: 8900 из 11 075 непустых +-- значений с |diff| > 50; перцентили p05 = −600 000, p50 = −50 010, p95 = +300 000. +-- У одного и того же listing_id 406163 в колонке соседствуют −200000.00 (рубли, из +-- загрузчика) и −0.82 (настоящий процент, из триггера record_listing_price_change). +-- Код починен в том же PR (providers/domclick/detail.py + offer_price_history.py: +-- процент считается из соседних цен, неправдоподобное значение отвергается, а не +-- зажимается). Эта миграция отрабатывает задним числом по уже собранным строкам. +-- +-- ПОЧЕМУ ПЕРЕСЧИТЫВАЕМ ВСЕ DOMKLIK-СТРОКИ, А НЕ ТОЛЬКО ИСПОРЧЕННЫЕ +-- Отличить строку загрузчика от строки триггера по данным нечем: триггер пишет +-- source = NEW.source, то есть тоже 'domklik' (131_fix_diff_percent_overflow.sql). +-- Косвенный признак есть — у триггерной строки change_time = recorded_at (обе now()), +-- у загрузчика change_time историческая и много раньше recorded_at, — но опираться +-- на него незачем: формула триггера («предыдущая цена → текущая, round 2») это ровно +-- то, что считает оконная lag() ниже, поэтому у триггерных строк пересчёт даёт то же +-- самое значение и UPDATE их не трогает (WHERE ... IS DISTINCT FROM). Фильтр по +-- |x| > 100 тоже не берём: он пропустил бы испорченные строки с мелким рублёвым +-- diff (например −50 рублей выглядит как правдоподобные −50%). +-- +-- ОКНО СЧИТАЕМ ПО ВСЕЙ ИСТОРИИ ЛИСТИНГА, А ПИШЕМ ТОЛЬКО В DOMKLIK +-- PARTITION BY listing_id без фильтра по source: «предыдущая цена» — это предыдущая +-- запись листинга, кто бы её ни записал. Если сузить окно до source = 'domklik', +-- у листинга со смешанными источниками соседом станет не та строка. +-- +-- САМАЯ РАННЯЯ ЗАПИСЬ ЛИСТИНГА → NULL, НЕ 0 +-- Предыдущей цены нет — процента не существует. Ноль здесь читался бы как «цена не +-- менялась», то есть как измерение, которого не было. Так же ведёт себя и код. +-- +-- ИДЕМПОТЕНТНОСТЬ +-- Пересчёт детерминирован (те же строки → те же значения), а UPDATE ограничен +-- `IS DISTINCT FROM` — повторный прогон трогает 0 строк. Новых объектов схемы нет. +BEGIN; +-- Конвенция проекта (#2752): массовый UPDATE берёт блокировки на строках +-- offer_price_history и без lock_timeout встанет в очередь за чужой сессией, +-- утащив за собой запросы приложения. +SET LOCAL lock_timeout = '5s'; + +DO $$ +DECLARE + before_notnull bigint; + before_bad bigint; + after_notnull bigint; + after_bad bigint; + touched bigint; +BEGIN + SELECT count(*) FILTER (WHERE diff_percent IS NOT NULL), + count(*) FILTER (WHERE abs(diff_percent) > 100) + INTO before_notnull, before_bad + FROM offer_price_history + WHERE source = 'domklik'; + + RAISE NOTICE 'domklik ДО: diff_percent непустых = %, из них |x| > 100 = %', + before_notnull, before_bad; + + WITH neighbours AS ( + SELECT id, + source, + price_rub, + lag(price_rub) OVER (PARTITION BY listing_id ORDER BY change_time, id) + AS prev_price + FROM offer_price_history + ), + recomputed AS ( + SELECT id, + CASE WHEN prev_price > 0 + THEN round((price_rub - prev_price) / prev_price * 100, 2) + END AS new_diff + FROM neighbours + WHERE source = 'domklik' + ) + UPDATE offer_price_history oph + SET diff_percent = r.new_diff + FROM recomputed r + WHERE oph.id = r.id + AND oph.diff_percent IS DISTINCT FROM r.new_diff; + + GET DIAGNOSTICS touched = ROW_COUNT; + + SELECT count(*) FILTER (WHERE diff_percent IS NOT NULL), + count(*) FILTER (WHERE abs(diff_percent) > 100) + INTO after_notnull, after_bad + FROM offer_price_history + WHERE source = 'domklik'; + + RAISE NOTICE 'domklik ПОСЛЕ: diff_percent непустых = % (было %), |x| > 100 = % (было %), переписано строк = %', + after_notnull, before_notnull, after_bad, before_bad, touched; +END $$; + +COMMIT; diff --git a/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py b/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py index b96b9036..1d3835dc 100644 --- a/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py +++ b/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py @@ -33,6 +33,7 @@ importers) — `scraper_kit.domclick_exceptions` остаётся единств from __future__ import annotations import json +import logging from datetime import UTC, datetime, timedelta from unittest.mock import AsyncMock, MagicMock @@ -40,7 +41,7 @@ import httpx import pytest from scraper_kit.browser_fetcher import SidecarBanPageError from scraper_kit.domclick_exceptions import DomClickBlockedError, DomClickParseError -from scraper_kit.offer_price_history import clamp_diff_percent +from scraper_kit.offer_price_history import validate_diff_percent from scraper_kit.providers.domclick.detail import ( DomClickDetailEnrichment, _extract_ssr_state, @@ -86,6 +87,7 @@ _SSR_LITERAL = """{ "priceInfo": { "priceHistory": [ {"date": "2026-04-15T09:02:26.548728+03:00", "price": 13400000, "diff": -400000, "state": "less"}, + {"date": "2026-03-01T09:00:00+03:00", "price": 13800000, "diff": -200000, "state": "less"}, {"date": "not-a-date", "price": 4800000, "diff": -1.0, "state": "less"}, {"date": "2026-05-01T10:00:00Z", "price": null, "diff": 0, "state": "more"} ] @@ -219,15 +221,34 @@ def test_parse_detail_html_full() -> None: def test_parse_detail_html_price_changes() -> None: e = parse_detail_html(_HTML, _CARD_URL) - # entry1 ок; entry2 bad-date skip; entry3 price=null skip → 1 запись - assert len(e.price_changes) == 1 - change = e.price_changes[0] - assert change["price_rub"] == 13400000 - assert change["diff_percent"] == -400000 - assert isinstance(change["change_time"], datetime) + # 2 валидные записи; bad-date skip; price=null skip. Порядок — по change_time. + assert len(e.price_changes) == 2 + first, second = e.price_changes + assert [c["price_rub"] for c in e.price_changes] == [13800000, 13400000] + # Процент считается из соседних price_rub, поле diff площадки (рубли) игнорируется + # (#3225): 13 400 000 / 13 800 000 − 1 = −2.9%, а НЕ −400000. + assert first["diff_percent"] is None # самая ранняя запись: предыдущей цены нет + assert second["diff_percent"] == -2.9 + assert isinstance(second["change_time"], datetime) # ISO8601-with-offset: tz-aware, offset СОХРАНЁН (+03:00), НЕ сконвертирован в UTC. - assert change["change_time"].tzinfo is not None - assert change["change_time"].utcoffset() == timedelta(hours=3) + assert second["change_time"].tzinfo is not None + assert second["change_time"].utcoffset() == timedelta(hours=3) + + +def test_parse_detail_html_price_changes_single_entry_has_no_percent() -> None: + html = _wrap( + { + "productCard": { + "priceInfo": { + "priceHistory": [ + {"date": "2026-04-15T09:00:00+03:00", "price": 9000000, "diff": -300000} + ] + } + } + } + ) + e = parse_detail_html(html, _CARD_URL) + assert [c["diff_percent"] for c in e.price_changes] == [None] def test_parse_detail_html_raw_extra() -> None: @@ -588,20 +609,23 @@ def _insert_diff_param(db: MagicMock) -> float | None: return insert_calls[0][0][1]["diff"] -def test_save_detail_enrichment_clamps_extreme_positive_diff() -> None: +def test_save_detail_enrichment_rejects_rubles_as_diff(caplog: pytest.LogCaptureFixture) -> None: + """Величина не того рода (рубли) → NULL + warning, НЕ 999999.99 (#3225).""" db = MagicMock() db.execute.return_value.rowcount = 1 ct = datetime(2026, 4, 1, tzinfo=UTC) e = DomClickDetailEnrichment( item_id="x", source_url=_CARD_URL, - price_changes=[{"change_time": ct, "price_rub": 5000000, "diff_percent": 5000000}], + price_changes=[{"change_time": ct, "price_rub": 5000000, "diff_percent": 250000}], ) - save_detail_enrichment(db, 1, e) - assert _insert_diff_param(db) == 999999.99 + with caplog.at_level(logging.WARNING): + save_detail_enrichment(db, 777, e) + assert _insert_diff_param(db) is None + assert any("250000" in r.getMessage() and "777" in r.getMessage() for r in caplog.records) -def test_save_detail_enrichment_clamps_extreme_negative_diff() -> None: +def test_save_detail_enrichment_rejects_extreme_negative_diff() -> None: db = MagicMock() db.execute.return_value.rowcount = 1 ct = datetime(2026, 4, 1, tzinfo=UTC) @@ -611,7 +635,7 @@ def test_save_detail_enrichment_clamps_extreme_negative_diff() -> None: price_changes=[{"change_time": ct, "price_rub": 5000000, "diff_percent": -5000000}], ) save_detail_enrichment(db, 1, e) - assert _insert_diff_param(db) == -999999.99 + assert _insert_diff_param(db) is None def test_save_detail_enrichment_normal_diff_unchanged() -> None: @@ -640,31 +664,40 @@ def test_save_detail_enrichment_none_diff_unchanged() -> None: assert _insert_diff_param(db) is None -# ── clamp_diff_percent (scraper_kit.offer_price_history) — standalone unit ──── +# ── validate_diff_percent (scraper_kit.offer_price_history) — standalone unit ─ # Нет отдельной tests/ директории для scraper-kit-пакета (см. audit перед фиксом) — # прямые unit-тесты хелпера живут здесь, рядом с save_detail_enrichment-тестами, # которые его же и используют. -def test_clamp_diff_percent_extreme_positive() -> None: - assert clamp_diff_percent(5000000) == 999999.99 +def test_validate_diff_percent_rejects_rubles(caplog: pytest.LogCaptureFixture) -> None: + with caplog.at_level(logging.WARNING): + assert validate_diff_percent(250000, listing_id=406163) is None + assert any("250000" in r.getMessage() and "406163" in r.getMessage() for r in caplog.records) -def test_clamp_diff_percent_extreme_negative() -> None: - assert clamp_diff_percent(-5000000) == -999999.99 +def test_validate_diff_percent_rejects_extreme_negative() -> None: + assert validate_diff_percent(-5000000) is None -def test_clamp_diff_percent_in_range_unchanged() -> None: - assert clamp_diff_percent(-2.5) == -2.5 +def test_validate_diff_percent_boundary_kept() -> None: + assert validate_diff_percent(100) == 100.0 + assert validate_diff_percent(-100) == -100.0 + assert validate_diff_percent(100.01) is None -def test_clamp_diff_percent_none() -> None: - assert clamp_diff_percent(None) is None +def test_validate_diff_percent_in_range_unchanged() -> None: + assert validate_diff_percent(-2.5) == -2.5 + assert validate_diff_percent(-0.82) == -0.82 -def test_clamp_diff_percent_bool_treated_as_none() -> None: - assert clamp_diff_percent(True) is None - assert clamp_diff_percent(False) is None +def test_validate_diff_percent_none() -> None: + assert validate_diff_percent(None) is None + + +def test_validate_diff_percent_bool_treated_as_none() -> None: + assert validate_diff_percent(True) is None + assert validate_diff_percent(False) is None # ── canon_sale_type (#2674) ─────────────────────────────────────────────────── diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/offer_price_history.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/offer_price_history.py index 52396975..54e77602 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/offer_price_history.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/offer_price_history.py @@ -1,30 +1,53 @@ -"""Клампинг diff_percent перед прямым INSERT в offer_price_history. +"""Гейт diff_percent перед прямым INSERT в offer_price_history. -offer_price_history.diff_percent — NUMERIC(8,2) (±999999.99, см. миграцию -131_fix_diff_percent_overflow.sql). Та миграция клампит diff_percent, который -СЧИТАЕТ БД-триггер record_listing_price_change() на UPDATE listings, но триггер -не срабатывает на прямые INSERT из provider-модулей (domclick/cian detail), -которые пишут diff_percent, взятый напрямую из скрапленного HTML/JSON внешнего -источника. Garbage/экстремальное значение с редизайна source-страницы иначе -переполняет колонку и спамит "numeric field overflow" в проде. +offer_price_history.diff_percent — процент, а не рубли. Тип колонки NUMERIC(8,2) +вмещает ±999999.99, и до #3225 значение просто зажималось в эти границы +(clamp_diff_percent, миграция 131_fix_diff_percent_overflow.sql). Клампинг спасал +от "numeric field overflow", но пропускал в колонку ДРУГУЮ ВЕЛИЧИНУ: домкликовский +loader клал поле источника ``diff``, а там рубли. Прод 29.08.2026: 8900 из 11 075 +непустых значений с |diff| > 50, p50 = −50 010. + +Поэтому теперь не зажимаем, а отвергаем: |x| > 100 для процента — не крайний +случай, а сигнал, что пишут не проценты. Значение → NULL + warning с listing_id и +сырым значением, чтобы источник расхождения было видно в логах. + +Цена решения: настоящий рост цены больше чем в 2 раза (+100%) тоже уйдёт в NULL. +Это осознанный размен — колонка аналитическая, а молчаливый мусор в ней дороже +пропущенного выброса. """ from __future__ import annotations -_DIFF_PERCENT_BOUND = 999999.99 +import logging + +logger = logging.getLogger(__name__) + +_DIFF_PERCENT_MAX_ABS = 100.0 -def clamp_diff_percent(value: float | int | None) -> float | None: - """Clamp raw scraped diff_percent into offer_price_history's NUMERIC(8,2) bound. - - Mirrors the DB-trigger clamp in migration 131_fix_diff_percent_overflow.sql - (LEAST(GREATEST(x, -999999.99), 999999.99)) so direct application-level INSERTs - (bypassing that trigger) cannot overflow the column either. - """ +def validate_diff_percent( + value: float | int | None, + listing_id: int | str | None = None, +) -> float | None: + """Процент → он же; не-процент (|x| > 100) и нечисло → None + warning.""" if value is None or isinstance(value, bool): return None try: numeric = float(value) except (TypeError, ValueError): + logger.warning( + "offer_price_history: diff_percent %r не число, listing_id=%s → NULL", + value, + listing_id, + ) return None - return min(max(numeric, -_DIFF_PERCENT_BOUND), _DIFF_PERCENT_BOUND) + if abs(numeric) > _DIFF_PERCENT_MAX_ABS: + logger.warning( + "offer_price_history: diff_percent %r отвергнут (|x| > %s — это не процент), " + "listing_id=%s → NULL", + value, + _DIFF_PERCENT_MAX_ABS, + listing_id, + ) + return None + return numeric diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/cian/detail.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/cian/detail.py index 196845d3..24150a5c 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/cian/detail.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/cian/detail.py @@ -25,7 +25,7 @@ from sqlalchemy.orm import Session from scraper_kit.ceiling_height import plausible_ceiling_m from scraper_kit.cian_exceptions import CianBlockedError from scraper_kit.cian_state_parser import extract_all_states, extract_state -from scraper_kit.offer_price_history import clamp_diff_percent +from scraper_kit.offer_price_history import validate_diff_percent from scraper_kit.providers._base import build_curl_cffi_session from scraper_kit.providers._proxy import curl_proxy_url from scraper_kit.repair_state_normalizer import ( @@ -488,7 +488,7 @@ def save_detail_enrichment( "lid": listing_id, "ct": change["change_time"], "price": change["price_rub"], - "diff": clamp_diff_percent(change.get("diff_percent")), + "diff": validate_diff_percent(change.get("diff_percent"), listing_id), }, ) diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/domclick/detail.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/domclick/detail.py index 7802d22d..279962cf 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/domclick/detail.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/domclick/detail.py @@ -62,7 +62,7 @@ from scraper_kit.domclick_exceptions import ( DomClickParseError, ) from scraper_kit.browser_fetcher import SidecarBanPageError -from scraper_kit.offer_price_history import clamp_diff_percent +from scraper_kit.offer_price_history import validate_diff_percent from scraper_kit.repair_state_normalizer import ( infer_repair_state_from_text, normalize_repair_state, @@ -234,8 +234,7 @@ def _extract_ssr_state(html: str) -> dict[str, Any]: # DOMCLICK_BLOCK_MARKERS (#2636: расхождение списков привело к misclassify # blocked→failed). Логируем голову HTML, чтобы дрейф маркеров был виден. logger.warning( - "domclick_detail: __SSR_STATE__ not found, no known block marker — " - "head=%r", + "domclick_detail: __SSR_STATE__ not found, no known block marker — head=%r", html[:300].replace("\n", " "), ) raise DomClickParseError("__SSR_STATE__ not found") @@ -481,17 +480,24 @@ def parse_detail_html(html: str, source_url: str) -> DomClickDetailEnrichment: # Skip при нераспарсенной дате ИЛИ отсутствующей/невалидной цене. if change_time is None or price_rub is None: continue - diff = entry.get("diff") - diff_percent = ( - diff if isinstance(diff, int | float) and not isinstance(diff, bool) else None - ) price_changes.append( { "change_time": change_time, "price_rub": price_rub, - "diff_percent": diff_percent, + "diff_percent": None, } ) + # Процент считаем САМИ из соседних price_rub (#3225): поле источника + # ``diff`` — это РУБЛИ, а колонка diff_percent — проценты. У самой ранней + # записи (и при нулевой предыдущей цене) предыдущей цены нет → NULL, не 0. + price_changes.sort(key=lambda c: c["change_time"]) + prev_price: int | None = None + for change in price_changes: + if prev_price: + change["diff_percent"] = round( + (change["price_rub"] - prev_price) / prev_price * 100, 2 + ) + prev_price = change["price_rub"] # AVM (Layer C, тот же fetch) — DomClick price prediction живёт ТОП-УРОВНЕМ # в SSR-стейте. Может быть плоским или обёрнут в .result/.data — defensively @@ -813,7 +819,7 @@ def save_detail_enrichment( "lid": listing_id, "ct": ct, "price": price, - "diff": clamp_diff_percent(change.get("diff_percent")), + "diff": validate_diff_percent(change.get("diff_percent"), listing_id), }, ) except Exception as exc: From afa93f9a1e0aacc3cd7b5b2f340019adfd4ab7f9 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sun, 6 Sep 2026 01:08:09 +0500 Subject: [PATCH 2/2] =?UTF-8?q?fix(tradein):=20=D0=BC=D0=B8=D0=B3=D1=80?= =?UTF-8?q?=D0=B0=D1=86=D0=B8=D1=8F=20285=20=D0=BD=D0=B5=20=D1=82=D1=80?= =?UTF-8?q?=D0=BE=D0=B3=D0=B0=D0=B5=D1=82=20=D1=81=D1=82=D1=80=D0=BE=D0=BA?= =?UTF-8?q?=D0=B8=20=D1=82=D1=80=D0=B8=D0=B3=D0=B3=D0=B5=D1=80=D0=B0=20(de?= =?UTF-8?q?ep-review=20=D0=B1=D0=BB=D0=BE=D0=BA=D0=B5=D1=80)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Утверждение «формула триггера = формула миграции» было ложным по ИСТОЧНИКУ базы: record_listing_price_change (131:76-86) считает процент от listings.OLD.price_rub, а не от предыдущей строки offer_price_history. Пересчёт по lag() портил честные значения: у листинга, чья история начинается с триггерной строки, lag() = NULL → −0.82 уходил в NULL; у триггерной строки с соседом-строкой загрузчика база чужая. Теперь пересчитываем и берём как базу ТОЛЬКО строки загрузчика (change_time <> recorded_at — триггер ставит обе метки одним now()). Это ровно то, что делает починенный код: процент внутри истории самой карточки. Окно lag() сужено до листингов с domklik-строками — иначе оконная функция шла по всей таблице под lock_timeout = 5s и валила деплой. Признак закреплён тестом на INSERT загрузчика (recorded_at не указан → DEFAULT NOW()); при добавлении recorded_at в INSERT тест краснеет — проверено. В шапке миграции отмечена асимметрия: старые cian-строки с |x| > 100 не чиним. --- .../285_domklik_diff_percent_recompute.sql | 74 ++++++++++++++----- .../tests/scrapers/test_domclick_detail.py | 31 ++++++++ 2 files changed, 87 insertions(+), 18 deletions(-) diff --git a/tradein-mvp/backend/data/sql/285_domklik_diff_percent_recompute.sql b/tradein-mvp/backend/data/sql/285_domklik_diff_percent_recompute.sql index 2acaaf57..f00e2dd9 100644 --- a/tradein-mvp/backend/data/sql/285_domklik_diff_percent_recompute.sql +++ b/tradein-mvp/backend/data/sql/285_domklik_diff_percent_recompute.sql @@ -11,21 +11,47 @@ -- процент считается из соседних цен, неправдоподобное значение отвергается, а не -- зажимается). Эта миграция отрабатывает задним числом по уже собранным строкам. -- --- ПОЧЕМУ ПЕРЕСЧИТЫВАЕМ ВСЕ DOMKLIK-СТРОКИ, А НЕ ТОЛЬКО ИСПОРЧЕННЫЕ --- Отличить строку загрузчика от строки триггера по данным нечем: триггер пишет --- source = NEW.source, то есть тоже 'domklik' (131_fix_diff_percent_overflow.sql). --- Косвенный признак есть — у триггерной строки change_time = recorded_at (обе now()), --- у загрузчика change_time историческая и много раньше recorded_at, — но опираться --- на него незачем: формула триггера («предыдущая цена → текущая, round 2») это ровно --- то, что считает оконная lag() ниже, поэтому у триггерных строк пересчёт даёт то же --- самое значение и UPDATE их не трогает (WHERE ... IS DISTINCT FROM). Фильтр по --- |x| > 100 тоже не берём: он пропустил бы испорченные строки с мелким рублёвым --- diff (например −50 рублей выглядит как правдоподобные −50%). +-- ТРОГАЕМ ТОЛЬКО СТРОКИ ЗАГРУЗЧИКА: change_time <> recorded_at +-- В таблице два писателя с РАЗНОЙ базой отсчёта, и смешивать их нельзя: +-- • загрузчик (domclick/detail.py) считает процент внутри истории самой карточки — +-- база это предыдущая запись priceHistory, то есть предыдущая строка загрузчика; +-- • триггер record_listing_price_change (131_fix_diff_percent_overflow.sql:76-86) +-- считает от listings.OLD.price_rub — от цены В КАРТОЧКЕ ЛИСТИНГА на момент +-- upsert'а. Эта база в offer_price_history может вообще не лежать отдельной +-- строкой. +-- Поэтому раннее утверждение «формула триггера = формула миграции, пересчёт даст то +-- же значение» ЛОЖНО, и опереться на признак происхождения как раз нужно. Без него +-- миграция портит честные данные двумя способами: (1) у листинга, чья история +-- начинается с триггерной строки, lag() = NULL → честный −0.82 перезаписывается в +-- NULL; (2) у триггерной строки, соседом которой по lag() оказалась строка +-- загрузчика, честный процент пересчитывается от чужой базы. +-- Признак точный, а не эвристический: триггер подставляет now() и в change_time, и +-- в recorded_at ОДНИМ INSERT'ом, а now() стабилен внутри транзакции → у триггерной +-- строки метки равны побайтово. Загрузчик пишет в change_time дату источника, а +-- recorded_at не указывает вовсе (DEFAULT NOW(), 023_offer_price_history.sql:19) → +-- расхождение в месяцы. Совпадение исторической даты с моментом вставки с точностью +-- до микросекунды недостижимо. Признак закреплён тестом +-- test_save_detail_enrichment_leaves_recorded_at_to_default. -- --- ОКНО СЧИТАЕМ ПО ВСЕЙ ИСТОРИИ ЛИСТИНГА, А ПИШЕМ ТОЛЬКО В DOMKLIK --- PARTITION BY listing_id без фильтра по source: «предыдущая цена» — это предыдущая --- запись листинга, кто бы её ни записал. Если сузить окно до source = 'domklik', --- у листинга со смешанными источниками соседом станет не та строка. +-- ОКНО lag() — ТОЖЕ ТОЛЬКО ПО СТРОКАМ ЗАГРУЗЧИКА +-- Триггерные строки не переписываем И не используем как базу. Иначе строка +-- загрузчика получила бы базой триггерную строку, которой в истории карточки нет, — +-- и результат разошёлся бы с тем, что теперь пишет починенный код. Задача миграции +-- ровно в том, чтобы задним числом дать те же значения, что даёт код: prev — это +-- предыдущая запись priceHistory карточки. По source окно не сужаем: «предыдущая +-- цена» — это предыдущая запись листинга, а у листинга со смешанными источниками +-- фильтр по source подсунул бы не ту строку. +-- +-- ФИЛЬТР ПО |x| > 100 НЕ БЕРЁМ +-- Он пропустил бы испорченные строки с мелким рублёвым diff (−50 рублей выглядит +-- как правдоподобные −50%). +-- +-- АСИММЕТРИЯ: СТАРЫЕ CIAN-СТРОКИ НЕ ЧИНИМ +-- Гейт validate_diff_percent общий для всех источников (|x| > 100 → NULL + warning), +-- а миграция — только про domklik. Уже лежащие в таблице cian-строки с |x| > 100 +-- остаются как есть: их история приходит из cian_price_history.py со своим полем и +-- своей базой, отдельным замером не подтверждена, а править вслепую по чужому +-- источнику — это второй #3225, а не его починка. Отдельная задача. -- -- САМАЯ РАННЯЯ ЗАПИСЬ ЛИСТИНГА → NULL, НЕ 0 -- Предыдущей цены нет — процента не существует. Ноль здесь читался бы как «цена не @@ -52,18 +78,29 @@ BEGIN count(*) FILTER (WHERE abs(diff_percent) > 100) INTO before_notnull, before_bad FROM offer_price_history - WHERE source = 'domklik'; + WHERE source = 'domklik' + AND change_time <> recorded_at; - RAISE NOTICE 'domklik ДО: diff_percent непустых = %, из них |x| > 100 = %', + RAISE NOTICE 'domklik (строки загрузчика) ДО: diff_percent непустых = %, из них |x| > 100 = %', before_notnull, before_bad; WITH neighbours AS ( + -- Окно только по листингам, у которых есть domklik-строки: без этого сужения + -- lag() прогоняется по ВСЕЙ таблице, и под lock_timeout = 5s деплой падает. + -- Оба скана идут по индексам 023: oph_source_time_idx (source, change_time) + -- для подзапроса и oph_listing_time_idx (listing_id, change_time) для окна. SELECT id, source, price_rub, lag(price_rub) OVER (PARTITION BY listing_id ORDER BY change_time, id) AS prev_price FROM offer_price_history + WHERE listing_id IN ( + SELECT DISTINCT listing_id + FROM offer_price_history + WHERE source = 'domklik' + ) + AND change_time <> recorded_at ), recomputed AS ( SELECT id, @@ -85,9 +122,10 @@ BEGIN count(*) FILTER (WHERE abs(diff_percent) > 100) INTO after_notnull, after_bad FROM offer_price_history - WHERE source = 'domklik'; + WHERE source = 'domklik' + AND change_time <> recorded_at; - RAISE NOTICE 'domklik ПОСЛЕ: diff_percent непустых = % (было %), |x| > 100 = % (было %), переписано строк = %', + RAISE NOTICE 'domklik (строки загрузчика) ПОСЛЕ: diff_percent непустых = % (было %), |x| > 100 = % (было %), переписано строк = %', after_notnull, before_notnull, after_bad, before_bad, touched; END $$; diff --git a/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py b/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py index 1d3835dc..537bfc00 100644 --- a/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py +++ b/tradein-mvp/backend/tests/scrapers/test_domclick_detail.py @@ -578,6 +578,37 @@ def test_save_detail_enrichment_writes_columns() -> None: db.commit.assert_called_once() +def test_save_detail_enrichment_leaves_recorded_at_to_default() -> None: + """Строка загрузчика обязана иметь change_time <> recorded_at. + + По этому признаку миграция 285 отличает строки загрузчика от строк триггера + record_listing_price_change (тот пишет обе метки одним now(), то есть равными) и + пересчитывает diff_percent только у первых. У триггера ДРУГАЯ база отсчёта — + listings.OLD.price_rub, — поэтому пересчёт его строк ломает честные значения. + Признак держится на двух вещах: change_time = дата источника (глубоко в прошлом), + а recorded_at загрузчик не указывает вовсе → DEFAULT NOW() на вставке. + Пропишут recorded_at в этот INSERT — признак сломается и тест покраснеет. + """ + db = MagicMock() + db.execute.return_value.rowcount = 1 + ct = datetime(2026, 4, 1, tzinfo=UTC) + e = DomClickDetailEnrichment( + item_id="2075729321", + source_url=_CARD_URL, + price_changes=[{"change_time": ct, "price_rub": 5000000, "diff_percent": -2.5}], + ) + + assert save_detail_enrichment(db, 999, e) is True + + insert_calls = [c for c in db.execute.call_args_list if "offer_price_history" in str(c[0][0])] + assert len(insert_calls) == 1 + insert_sql = str(insert_calls[0][0][0]) + assert "recorded_at" not in insert_sql + # change_time — историческая дата источника, а не момент вставки. + assert insert_calls[0][0][1]["ct"] == ct + assert ct < datetime.now(UTC) + + def test_save_detail_enrichment_listing_not_found() -> None: db = MagicMock() db.execute.return_value.rowcount = 0