fix(tradein): миграция 285 не трогает строки триггера (deep-review блокер)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / changes (pull_request) Successful in 11s
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 4m56s

Утверждение «формула триггера = формула миграции» было ложным по ИСТОЧНИКУ базы:
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 не чиним.
This commit is contained in:
bot-backend 2026-09-06 01:08:09 +05:00
parent 769a36098b
commit afa93f9a1e
2 changed files with 87 additions and 18 deletions

View file

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

View file

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