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: