fix(tradein): считать domklik diff_percent из цен, отвергать не-проценты (#3225)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / changes (pull_request) Successful in 10s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 5m6s

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
This commit is contained in:
bot-backend 2026-09-06 00:48:08 +05:00
parent 7afaa12d75
commit 769a36098b
5 changed files with 211 additions and 55 deletions

View file

@ -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;

View file

@ -33,6 +33,7 @@ importers) — `scraper_kit.domclick_exceptions` остаётся единств
from __future__ import annotations from __future__ import annotations
import json import json
import logging
from datetime import UTC, datetime, timedelta from datetime import UTC, datetime, timedelta
from unittest.mock import AsyncMock, MagicMock from unittest.mock import AsyncMock, MagicMock
@ -40,7 +41,7 @@ import httpx
import pytest import pytest
from scraper_kit.browser_fetcher import SidecarBanPageError from scraper_kit.browser_fetcher import SidecarBanPageError
from scraper_kit.domclick_exceptions import DomClickBlockedError, DomClickParseError 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 ( from scraper_kit.providers.domclick.detail import (
DomClickDetailEnrichment, DomClickDetailEnrichment,
_extract_ssr_state, _extract_ssr_state,
@ -86,6 +87,7 @@ _SSR_LITERAL = """{
"priceInfo": { "priceInfo": {
"priceHistory": [ "priceHistory": [
{"date": "2026-04-15T09:02:26.548728+03:00", "price": 13400000, "diff": -400000, "state": "less"}, {"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": "not-a-date", "price": 4800000, "diff": -1.0, "state": "less"},
{"date": "2026-05-01T10:00:00Z", "price": null, "diff": 0, "state": "more"} {"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: def test_parse_detail_html_price_changes() -> None:
e = parse_detail_html(_HTML, _CARD_URL) e = parse_detail_html(_HTML, _CARD_URL)
# entry1 ок; entry2 bad-date skip; entry3 price=null skip → 1 запись # 2 валидные записи; bad-date skip; price=null skip. Порядок — по change_time.
assert len(e.price_changes) == 1 assert len(e.price_changes) == 2
change = e.price_changes[0] first, second = e.price_changes
assert change["price_rub"] == 13400000 assert [c["price_rub"] for c in e.price_changes] == [13800000, 13400000]
assert change["diff_percent"] == -400000 # Процент считается из соседних price_rub, поле diff площадки (рубли) игнорируется
assert isinstance(change["change_time"], datetime) # (#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. # ISO8601-with-offset: tz-aware, offset СОХРАНЁН (+03:00), НЕ сконвертирован в UTC.
assert change["change_time"].tzinfo is not None assert second["change_time"].tzinfo is not None
assert change["change_time"].utcoffset() == timedelta(hours=3) 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: 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"] 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 = MagicMock()
db.execute.return_value.rowcount = 1 db.execute.return_value.rowcount = 1
ct = datetime(2026, 4, 1, tzinfo=UTC) ct = datetime(2026, 4, 1, tzinfo=UTC)
e = DomClickDetailEnrichment( e = DomClickDetailEnrichment(
item_id="x", item_id="x",
source_url=_CARD_URL, 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) with caplog.at_level(logging.WARNING):
assert _insert_diff_param(db) == 999999.99 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 = MagicMock()
db.execute.return_value.rowcount = 1 db.execute.return_value.rowcount = 1
ct = datetime(2026, 4, 1, tzinfo=UTC) 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}], price_changes=[{"change_time": ct, "price_rub": 5000000, "diff_percent": -5000000}],
) )
save_detail_enrichment(db, 1, e) 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: 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 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 перед фиксом) — # Нет отдельной tests/ директории для scraper-kit-пакета (см. audit перед фиксом) —
# прямые unit-тесты хелпера живут здесь, рядом с save_detail_enrichment-тестами, # прямые unit-тесты хелпера живут здесь, рядом с save_detail_enrichment-тестами,
# которые его же и используют. # которые его же и используют.
def test_clamp_diff_percent_extreme_positive() -> None: def test_validate_diff_percent_rejects_rubles(caplog: pytest.LogCaptureFixture) -> None:
assert clamp_diff_percent(5000000) == 999999.99 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: def test_validate_diff_percent_rejects_extreme_negative() -> None:
assert clamp_diff_percent(-5000000) == -999999.99 assert validate_diff_percent(-5000000) is None
def test_clamp_diff_percent_in_range_unchanged() -> None: def test_validate_diff_percent_boundary_kept() -> None:
assert clamp_diff_percent(-2.5) == -2.5 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: def test_validate_diff_percent_in_range_unchanged() -> None:
assert clamp_diff_percent(None) is 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: def test_validate_diff_percent_none() -> None:
assert clamp_diff_percent(True) is None assert validate_diff_percent(None) is None
assert clamp_diff_percent(False) 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) ─────────────────────────────────────────────────── # ── canon_sale_type (#2674) ───────────────────────────────────────────────────

View file

@ -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, см. миграцию offer_price_history.diff_percent процент, а не рубли. Тип колонки NUMERIC(8,2)
131_fix_diff_percent_overflow.sql). Та миграция клампит diff_percent, который вмещает ±999999.99, и до #3225 значение просто зажималось в эти границы
СЧИТАЕТ БД-триггер record_listing_price_change() на UPDATE listings, но триггер (clamp_diff_percent, миграция 131_fix_diff_percent_overflow.sql). Клампинг спасал
не срабатывает на прямые INSERT из provider-модулей (domclick/cian detail), от "numeric field overflow", но пропускал в колонку ДРУГУЮ ВЕЛИЧИНУ: домкликовский
которые пишут diff_percent, взятый напрямую из скрапленного HTML/JSON внешнего loader клал поле источника ``diff``, а там рубли. Прод 29.08.2026: 8900 из 11 075
источника. Garbage/экстремальное значение с редизайна source-страницы иначе непустых значений с |diff| > 50, p50 = 50 010.
переполняет колонку и спамит "numeric field overflow" в проде.
Поэтому теперь не зажимаем, а отвергаем: |x| > 100 для процента не крайний
случай, а сигнал, что пишут не проценты. Значение NULL + warning с listing_id и
сырым значением, чтобы источник расхождения было видно в логах.
Цена решения: настоящий рост цены больше чем в 2 раза (+100%) тоже уйдёт в NULL.
Это осознанный размен колонка аналитическая, а молчаливый мусор в ней дороже
пропущенного выброса.
""" """
from __future__ import annotations 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: def validate_diff_percent(
"""Clamp raw scraped diff_percent into offer_price_history's NUMERIC(8,2) bound. value: float | int | None,
listing_id: int | str | None = None,
Mirrors the DB-trigger clamp in migration 131_fix_diff_percent_overflow.sql ) -> float | None:
(LEAST(GREATEST(x, -999999.99), 999999.99)) so direct application-level INSERTs """Процент → он же; не-процент (|x| > 100) и нечисло → None + warning."""
(bypassing that trigger) cannot overflow the column either.
"""
if value is None or isinstance(value, bool): if value is None or isinstance(value, bool):
return None return None
try: try:
numeric = float(value) numeric = float(value)
except (TypeError, ValueError): except (TypeError, ValueError):
logger.warning(
"offer_price_history: diff_percent %r не число, listing_id=%s → NULL",
value,
listing_id,
)
return None 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

View file

@ -25,7 +25,7 @@ from sqlalchemy.orm import Session
from scraper_kit.ceiling_height import plausible_ceiling_m from scraper_kit.ceiling_height import plausible_ceiling_m
from scraper_kit.cian_exceptions import CianBlockedError from scraper_kit.cian_exceptions import CianBlockedError
from scraper_kit.cian_state_parser import extract_all_states, extract_state 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._base import build_curl_cffi_session
from scraper_kit.providers._proxy import curl_proxy_url from scraper_kit.providers._proxy import curl_proxy_url
from scraper_kit.repair_state_normalizer import ( from scraper_kit.repair_state_normalizer import (
@ -488,7 +488,7 @@ def save_detail_enrichment(
"lid": listing_id, "lid": listing_id,
"ct": change["change_time"], "ct": change["change_time"],
"price": change["price_rub"], "price": change["price_rub"],
"diff": clamp_diff_percent(change.get("diff_percent")), "diff": validate_diff_percent(change.get("diff_percent"), listing_id),
}, },
) )

View file

@ -62,7 +62,7 @@ from scraper_kit.domclick_exceptions import (
DomClickParseError, DomClickParseError,
) )
from scraper_kit.browser_fetcher import SidecarBanPageError 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 ( from scraper_kit.repair_state_normalizer import (
infer_repair_state_from_text, infer_repair_state_from_text,
normalize_repair_state, normalize_repair_state,
@ -234,8 +234,7 @@ def _extract_ssr_state(html: str) -> dict[str, Any]:
# DOMCLICK_BLOCK_MARKERS (#2636: расхождение списков привело к misclassify # DOMCLICK_BLOCK_MARKERS (#2636: расхождение списков привело к misclassify
# blocked→failed). Логируем голову HTML, чтобы дрейф маркеров был виден. # blocked→failed). Логируем голову HTML, чтобы дрейф маркеров был виден.
logger.warning( logger.warning(
"domclick_detail: __SSR_STATE__ not found, no known block marker — " "domclick_detail: __SSR_STATE__ not found, no known block marker — head=%r",
"head=%r",
html[:300].replace("\n", " "), html[:300].replace("\n", " "),
) )
raise DomClickParseError("__SSR_STATE__ not found") raise DomClickParseError("__SSR_STATE__ not found")
@ -481,17 +480,24 @@ def parse_detail_html(html: str, source_url: str) -> DomClickDetailEnrichment:
# Skip при нераспарсенной дате ИЛИ отсутствующей/невалидной цене. # Skip при нераспарсенной дате ИЛИ отсутствующей/невалидной цене.
if change_time is None or price_rub is None: if change_time is None or price_rub is None:
continue continue
diff = entry.get("diff")
diff_percent = (
diff if isinstance(diff, int | float) and not isinstance(diff, bool) else None
)
price_changes.append( price_changes.append(
{ {
"change_time": change_time, "change_time": change_time,
"price_rub": price_rub, "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 живёт ТОП-УРОВНЕМ # AVM (Layer C, тот же fetch) — DomClick price prediction живёт ТОП-УРОВНЕМ
# в SSR-стейте. Может быть плоским или обёрнут в .result/.data — defensively # в SSR-стейте. Может быть плоским или обёрнут в .result/.data — defensively
@ -813,7 +819,7 @@ def save_detail_enrichment(
"lid": listing_id, "lid": listing_id,
"ct": ct, "ct": ct,
"price": price, "price": price,
"diff": clamp_diff_percent(change.get("diff_percent")), "diff": validate_diff_percent(change.get("diff_percent"), listing_id),
}, },
) )
except Exception as exc: except Exception as exc: