fix(tradein/deactivate): потолок эффективного TTL — защита от разгона пола и опечатки в расписании #2907

Merged
lekss361 merged 7 commits from fix/tradein-ttl-effective-cap into main 2026-08-15 18:06:55 +00:00

7 commits

Author SHA1 Message Date
bot-backend
08cff706ec docs(tradein/deactivate): убрать неверное число из обоснования потолка
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI / 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 / 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 4m33s
В шапке модуля, в комментарии миграции 264 и в докстринге теста стояло «23 687 из
44 744 avito-объявлений не подтверждались >7 суток» под заголовком «ЗАМЕР НА ПРОДЕ».

Число реальное, но приписано не тому. Перепроверено запросом 2026-08-15:
это ВСЕ источники вместе, и две трети — новостройки, которых оценщик не берёт
(он фильтрует listing_segment IS NULL OR = 'vtorichka'). У самого avito
просроченных строк ноль: 8 663 активных, максимальный возраст 10 суток.

Оставлять это в коде нельзя: следующий человек прочитает «avito раздут вдвое»,
проверит и не найдёт — а заодно потеряет доверие к остальным числам в том же
абзаце, которые верны и сверены с scrape_runs.counters.

Заодно явно записано, чего потолок НЕ делает: он не сжимает пул (0 деактиваций
замерено на всех четырёх джобах), а защищает от разгона пола и от опечатки в
расписании. Настоящий раздутый срез — строки с пустым сегментом, они чинятся
отдельной джобой.
2026-08-15 21:00:42 +03:00
bot-backend
19c9da8119 fix(tradein/deactivate): bool guard hole + unpinned test + yandex cap_mult gap (TTL-CAP round 3)
Три остатка ревью TTL-CAP:

1. cap_mult < 1 пропускал bool: jsonb true -> True < 1 ложно -> потолок =
   ttl_days*True = ttl_days -> пол молча отключается без ValueError. Тот же
   класс дыры возможен и через ttl_days=true (TTL молча = 1). Оба параметра
   теперь явно отклоняют bool ДО числового сравнения; воспроизведено на HEAD
   и закрыто тестами (True/False на обоих параметрах).

2. test_avito_prod_floor_is_capped_by_calibrated_cap_mult хардкодил cap_mult=6
   как вход -- мутация миграции 264 (6 -> 2) оставляла набор зелёным. Тест
   теперь читает cap_mult ИЗ ФАЙЛА миграции regex'ом, ожидаемый результат
   (потолок 60) остаётся зафиксированным числом -- дрейф калибровки в SQL
   теперь ломает тест.

3. Текст миграции 264 утверждал "yandex 43.0 -> потолок 60, запас есть" по
   статическому p99. Живые полы из scrape_runs.counters (08-10..08-15:
   75/75/75/39/52/54) и live-замер сегодня (79.2, n=1961) выше потолка 60 --
   тот же false-kill класс, что у avito. Откалибровал yandex отдельной
   миграцией 265 (cap_mult=3 -> потолок 90, тот же запас ~14%, что у avito),
   поправил таблицу в 264 на живые числа и пиннящий тест по образцу avito.

Численный эффект (live-замер 2026-08-15, до и после): next-run deactivated=0
на всех четырёх джобах что до, что после -- ветка по-прежнему НЕ сжимает пул
(avito/cian: живой пол уже ниже потолка, cap не участвует; yandex: 0 активных
строк старше 39 суток вообще, калибровка убирает будущий риск, не текущее
число; domklik: блокирован гейтом здоровья, confirmations 94 < 200). Ветка
остаётся тем, чем и была: защита от опечатки в расписании + калибровка, не
сжатие пула.

4508 backend-тестов зелёные (uv run pytest tests/), ruff чист на изменённых
файлах.
2026-08-15 20:42:00 +03:00
bot-backend
8fff174337 Merge remote-tracking branch 'forgejo/main' into fix/tradein-ttl-effective-cap 2026-08-15 20:26:40 +03:00
bot-backend
772ae116b5 fix(tradein/deactivate): validate cap_mult, calibrate avito, document scope gap
Round-2 review (MAJOR) left three items open:

1. cap_mult was threaded through as a jsonb default_params parameter but never
   validated, reproducing the exact ttl_days<=0 hole the earlier guard closed.
   Verified live: cap_mult=0 -> effective_ttl=0 -> whole active pool of the
   source would deactivate; cap_mult=0.5 pushes the ceiling BELOW the operator-
   configured ttl_days. Added `if cap_mult < 1: raise ValueError` next to the
   ttl_days guard (same fail-fast contract, before any SQL). Non-numeric values
   (e.g. a stringly-typed "6" from a typo in default_params) already fail safe
   via TypeError on the comparison, caught by the same except-block -> mark_failed.
   Covered with 5 new tests (zero/negative/<1/non-numeric/mark_failed routing).

2. The mechanical part of cap_mult (parameter + wiring) was merged but never
   calibrated for avito on prod -- no migration shipped, so prod default_params
   for deactivate_stale_avito still lacked "cap_mult" and ran with the module
   default (CAP_MULT=2, ceiling=20d), which is BELOW avito's own p99 revisit gap
   (42.1d) and below the observed prod peak (floor=52, three runs 08-10..08-12).
   Added data/sql/264_deactivate_stale_avito_cap_mult.sql (idempotent, same
   pattern as 219) setting cap_mult=6 for deactivate_stale_avito only (ceiling
   60d, matching the order of magnitude already used for cian/yandex). cian/
   yandex/domklik keep the CAP_MULT=2 default -- their p99 gaps (26.6/43.0/3.1)
   sit comfortably under their default ceilings (60/60/28), no override needed.
   Pinned the calibration with a dedicated test
   (test_avito_prod_floor_is_capped_by_calibrated_cap_mult) instead of leaving
   the avito slice skipped in the false-kill coverage test.

3. Confirmed (SSH read-only, prod counts): active rows aged >60d that this PR
   cannot touch regardless of cap_mult -- cian/novostroyki 9483, cian/NULL
   211, yandex/NULL 523 (0 inside the jobs' actual scope: cian/vtorichka,
   yandex/vtorichka). deactivate_stale_cian/_yandex are scoped to
   segments=['vtorichka'] by a deliberate, documented DECISION (blanket TTL on
   novostroyki risks killing live inventory cian/yandex don't fully sweep).
   Widening that scope is a separate, riskier investigation and is out of
   scope here -- documented the gap directly in the module docstring next to
   the existing DECISION so it isn't lost.

Verification (SSH read-only against prod, 2026-08-15): recomputed the exact
per-source formula the next scheduled run will use. In-scope next-run
deactivation is currently 0 for all four sources -- the active pool has
already self-corrected to be consistent with each source's own recent
effective TTL (yesterday's yandex run used effective=54, so no active row is
older than that yet). This matches the round-2 reviewer's own conclusion: the
cap is a preventative guardrail, not a retroactive cleanup, and isn't expected
to fire on the exact day it's calibrated. It is not idle, though -- live
recompute of yandex/vtorichka's raw (uncapped) floor right now is 78.2d,
already above its 60d ceiling; the trailing 6-day counters show the identical
loop (floor=75, deactivated=0, three days straight) already recurred twice
without this cap in place. The mechanism will bind the moment the pool ages
past the ceiling, which is exactly the recurrence it exists to stop.

Tests: 106 passed (test_deactivate_stale_ttl_cap.py,
test_deactivate_stale_revisit_floor.py, test_deactivate_stale_health_gate.py,
test_deactivate_stale_listings.py, test_migrations_manifest.py). ruff clean.
scripts/check-migration-lock-timeout.py: pass (UPDATE-only migration, no
blocking DDL, no SET LOCAL needed).
2026-08-15 19:58:19 +03:00
bot-backend
bef2a05f60 Merge remote-tracking branch 'forgejo/main' into local/fix-ttl-effective-cap-review2 2026-08-15 19:43:03 +03:00
bot-backend
cb79c67bfc fix(tradein/deactivate): make TTL-cap multiplier configurable per source
Review of 3a1e29a7 found CAP_MULT=2 is uniform across sources with wildly
different ttl_days, so it produces a different ABSOLUTE ceiling per source:
cian/yandex (ttl=30) -> 60d, avito (ttl=10) -> 20d, domklik (ttl=14) -> 28d.
That breaks exactly where the crawl's revisit tail doesn't scale with
ttl_days: avito's measured p99 revisit gap is 42.1d (_REVISIT_TAIL) --
above its own default cap of 20d -- so a legitimately slow-but-alive avito
crawl cycle would get its floor cut below the very tail the floor exists
to protect (the false-kill scenario #2659 was filed for). cian/yandex/
domklik aren't affected: their default ceilings (60/60/28) already sit
comfortably above their own measured tails (26.6/43.0/3.1).

Fix: cap_mult is now a function parameter (same pattern as
revisit_floor_quantile/min_confirmations) with the module constant CAP_MULT
as its default, wired through product_handlers via default_params["cap_mult"]
so a schedule can override it without touching the shared default. Also
closes the ttl_days<=0 edge case flagged in the same review: before the cap,
max(ttl_days, floor) tolerated a misconfigured ttl_days<=0 as long as the
floor was positive; with the cap, min(floor, ttl_days*cap_mult<=0) would
silently defeat that protection and match nearly the whole active pool.
ttl_days<=0 now raises ValueError before any SQL, same contract as the
existing staleness_column whitelist check.

Also verified (read-only, postgres-tradein) the review's core "no-op"
claim: false. scrape_runs.counters for the 6 days since the revisit-floor
went live (08-10..08-15) show the cap DID bind on 3 of 6 runs for avito
(floor 52 vs cap 20) and 3 of 6 for yandex (floor 75 vs cap 60) -- the
reviewer's "no source hits the cap" read a single-day trough right after
a natural recovery, not the whole observation window. See PR discussion
for the full counter history and refutation detail.

Refs #2659
2026-08-15 18:47:49 +03:00
bot-backend
3a1e29a7da fix(tradein/deactivate): cap effective TTL floor at 2x configured value
Revisit-floor (#2659) raises effective TTL via max(ttl_days, floor) with no
upper bound -- a positive feedback loop confirmed on prod: slow crawl raises
the floor, a high floor keeps stale listings marked active longer than a
fresh sweep needs to return, the "active" pool bloats with rot, and the next
floor measurement on that bloated pool comes out even higher. Yandex counters
sat at ttl_days_effective=75/75/75/39/52/54 for six runs straight with
deactivated=0; 23,687/44,744 "active" avito listings hadn't been confirmed in
>7 days, cian 10,572/19,514 and yandex 7,178/15,790 were >30 days stale, the
oldest "active" row hadn't been seen in 86 days.

CAP_MULT=2 caps the floor's upward push without disabling it -- the floor
still protects against premature deactivation during genuinely slow (but
alive) crawl cycles, it just can no longer grow unbounded. Beyond 2x, a
persistently low crawl rate is better handled by the existing health gate
(min_confirmations), which disables deactivation outright instead of
stretching TTL forever.

When the cap binds, counters gain ttl_floor_capped=1 + ttl_days_floor_raw
(the uncapped value) so it's visible in the run-history dashboard, not just
logs -- counters are stored as-is in scrape_runs.counters.

Single fix point: all four sources (avito/yandex/cian/domklik) route through
this one deactivate_stale_listings() via the product_handlers wildcard
"deactivate_stale_*" handler, so no other task file needed the change.
2026-08-15 17:56:12 +03:00