Ревью #3552: направление расходится с #3556/#1970, где неменявшиеся
estimate_*-настройки переводят в константы движка. Две новые ручки нужны
только для бэктеста вариантов; условие возврата записано рядом с полями,
чтобы его увидел тот, кто будет делать шаг 4 #3234.
Refs #3234
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Что было. После «40-летия Комсомола» остались однобуквенные порядковые:
регексы требовали после «N-» минимум две буквы. Одна улица в трёх форматах
разбиралась по-разному: cian «улица 2-я Синичкина» давал ('', ''),
avito/yandex «2-я ул. Синичкина» и domklik «2-я Синичкина улица» теряли номер
('синичкина'). Потеря номера — это ложная склейка: «9-я Парковая, 5» и
«12-я Парковая, 5» получали один ключ ('парковая', '5').
Что сделано. В обоих регексах названия улицы добавлен необязательный
порядковый «N-x » перед (нумерованным) именем; формат «номер тип имя»
переставляется в «тип номер имя» до разбора. Номер дома с дефисом не тронут.
Замер на проде (tradein-postgres, SELECT, 17.09): 12 848 разных адресов с
шаблоном «цифра-дефис/пробел-буква», 29 959 объявлений. Разбор изменился у
8 713 объявлений; ни один адрес не потерял токен. Межисточниковых групп
(улица, дом) 1305 -> 1473. Из 81 прежней группы, которая распалась, 60 —
разные порядковые улицы, 3 — ложный mkr-ключ, 18 — голая и порядковая
улица в разных городах (одна — источник сам потерял номер). Гейт бэктеста
без изменений baseline.
Refs #2291
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Два новых теста класса 4 ставили monkeypatch.setattr(settings,
"estimate_dedup_analogs_enabled", True). PR #3556 (#2378) удаляет это поле из
Settings, и setattr по несуществующему атрибуту падает AttributeError. CI
гоняет голову ветки, а не merge-ref, поэтому второй смерженный PR пришёл бы
зелёным и уронил main. На main флаг и так ON по умолчанию
(test_dedup_default_is_on), так что строки лишние при любом порядке мержа.
Проверено: дифф PR, наложенный на голову #3556 (31c31f8f), до правки
2 failed / 144 passed (AttributeError), после 146 passed. Тесты не стали
пустыми: откат префиксного регекса без флага даёт assert 4 == 1 и 1 == 2.
Refs #2291
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Tier H («тот же класс дома») отдаётся только при ≥5 комплах, иначе молча уходит в
Tier W без фильтра по классу. Прод, Loki 30 сут (комментарий 12.09): 61 из 86
попыток не доживает, в 31 из них в своей полосе 0 комплов. Какой вариант лучше —
порог 5→4, окно этажей ±30%→±50% или оба — решает только MAPE бэктеста на сделках,
а оба числа были литералами: сравнить варианты без правки кода было нельзя.
estimate_tier_h_min_comps (5) и estimate_tier_h_floors_tol (0.30) в Settings,
_fetch_analogs берёт их оттуда. Дефолты дают те же tf_min/tf_max, что литералы
(сверено для total_floors 1..199), поведение прода не меняется. Бэктест варианта:
docker exec -e ESTIMATE_TIER_H_MIN_COMPS=4 -e ESTIMATE_TIER_H_FLOORS_TOL=0.5 …
Дефолт не меняю: бэктест вариантов не прогнан (нужен деплой этих ручек).
Refs #3234
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Регексы названия улицы в _parse_street_house пускали ведущую цифру только через
пробел («8 Марта»), а через дефис нет. «40-летия Комсомола», «22-го Партсъезда»,
«3-го Интернационала» разбирались так:
- avito «ул. 40-летия Комсомола,32Г» и yandex — ('', ''), дедупа нет вовсе;
- cian «мкр. ЖБИ, улица 40-летия Комсомола, 8А» — ('mkr жби', '40'): все дома
улицы получали один ключ и дом «40», разные квартиры с одинаковыми
этажом/площадью/ценой сливались в одну;
- domklik «60-летия Октября проспект» — ('октября', …).
Фикс: (?:\d+\s+)? → (?:\d+(?:\s+|-))? в _DEDUP_STREET_NAME_RE и
_DEDUP_STREET_NAME_SUFFIX_RE. На 5310 живых адресах прода с «цифра-буква»
меняется разбор у 730 объявлений, все новые токены — реальные нумерованные
улицы (14 штук, от «40-летия комсомола» до «2-ая новосибирская»).
Бэктест-гейт: baseline перегенерирован. В реплее дедуп включён (как на захвате),
поэтому правка разбора сдвигает пул: изменились 99 из 1600 сделок, у 48 ошибка
меньше, у 51 больше. expected_sold MAPE 15.88→15.92, покрытие вилки 1056→1057,
headline не сдвинулся.
Refs #2291
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ДомКлик: DomClickGeoProfile вместо зашитых _EKB_ADDRESS_GUID/_EKB_AREA_ID; GUID Москвы
и области проверены живьём, aids вне ЕКБ не нужен, гард по bbox профиля. Страница
прогрева 77/50 — апексный domclick.ru: субдомены msk./moskovskaya-oblast. отдают 301.
Циан/Авито/Яндекс: CityLocation.cian_host, три новых скоупа (moskva, moskovskaya_oblast,
moskva_i_mo), avito_slug_is_region, city=NULL у мультигородских скоупов. Неизвестный слаг
теперь падает с ValueError вместо молчаливого отката на Екатеринбург.
Якоря: Москва — сетка 25 точек под radius_m=8000; область — 22 города-спутника
(10 добраны из Nominatim) плюс 22 кластера лот-массы. Замер на проде: города радиусом
10 км дают 73.8% лот-массы области, вместе с кластерами — 95.2%.
Ценовые коридоры: планировщик plan_price_corridors со статистикой усечения плюс
BisectionStats в живом движке. Провайдеры их пока не передают — отдельный заход.
Расписаний scrape_schedules для 77/50 в этом PR нет: они пойдут после первого ручного
прогона, подтверждающего живость профилей.
Центроид строился по названию улицы без города — в области это схлопывало одноимённые
улицы разных городов. Ключ стал (region_code, населённый пункт, улица); НП берётся из
типа сегмента адреса, из реестра городов региона или из deals.city; при пустом НП ключ
у области отбрасывается, а не склеивается с чужим городом.
Фильтры по региону добавлены в SELECT кандидатов, в запрос домов и в UPDATE. Новый
--max-spread-km (5 км) отбрасывает бакет с разбросанными домами.
geocoder.py: поддержка региона 50 (маркер «московская» — намеренно не «москва»,
DaData-имя «Московская», новое поле Region.has_city_core=False, чтобы к адресу области
не приклеивался суффикс главного города).
Обе стороны раскладываются по одной сетке 0.1°x0.2°, медианы внутри ячейки, в итог
только ячейки с ≥10 сделок И ≥10 объявлений, агрегация взвешена числом сделок.
Замер на проде (SQL исполнен в транзакции с ROLLBACK): регион 50 — 0.8136 вместо
пулового 0.8851, регион 77 — 0.7287 вместо 0.7100. Разнонаправленный сдвиг — подпись
поправки состава. Регион 66 не тронут байт в байт.
Гарды: регион не получает строк при покрытии geom <50%, совпавших ячейках <3 или
попадании в пересечение <50% геокодированных сделок; DELETE идёт в любом случае.
Тест применял 015/051/052 безусловно, и на базе, прошедшей всю цепочку
миграций (CI и прод), повтор 015 падал:
psycopg.errors.UndefinedColumn: column "returning_count" of relation
"scrape_runs" does not exist
Файл 015 идемпотентен относительно себя, но не относительно схемы,
прошедшей 214 (DROP COLUMN IF EXISTS returning_count): CREATE TABLE IF
NOT EXISTS — no-op, а COMMENT ON COLUMN в конце того же файла обращается
к снесённой колонке. На чистой базе, где 015 ложится с нуля, этого не
видно по построению — потому прогон и был зелёным там, где его гонял я,
и красным там, где его гоняет CI.
Зависимости теперь применяются только когда scrape_schedules ещё нет; в
докстроке — рецепт прогона на ПОЛНОЙ схеме, тем же путём, что у CI.
Заодно закрыты три дыры, которые находились мутациями:
- окно расписания и повторное применение проверяются на живой БД (до
этого 6,7 → 6,23 и DELETE+INSERT вместо ON CONFLICT проходили насквозь);
идемпотентность меряется created_at строки, а не числом строк — замена
«удалить и вставить» тоже оставляет ровно одну строку, но стирает
last_run_at/next_run_at на каждом деплое;
- next_run_at в будущем — утверждение стояло в приёмке и ничем не
проверялось;
- handler сравнивается по САМОМУ job'у, а не по log_name: имя — второй
литерал конструктора Handler, и чужое тело под верным ключом
(_job_landing_stats) проходило проверку по имени.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Правки по deep-ревью PR #3508.
1. HIGH. `app/core/auth_db.py` строил второй движок БЕЗ `connect_args`, а моё
обоснование пропуска было ложным: на проде `IDENTITY_STORE=auth` во всех трёх
сервисах образа и `AUTH_DB_PASSWORD` задан (сверено `printenv` в контейнерах),
то есть реестр живой. Путь горячий: `core/rbac.py` резолвит session-cookie в
middleware, синхронно на event loop'е, на каждом запросе с cookie — значит
`ACCESS EXCLUSIVE` на `auth.sessions` вешал бы не четыре слота `/estimate`, а
весь uvicorn-воркер (он один), включая `/health`. Потолок — та же константа
`DB_CONNECT_ARGS`: одна на оба движка, а не защита на одном и мина на втором.
Срабатывание безопасно — вызов уже под `except Exception` с фолбэком.
2. MEDIUM. Испорченный `options` (`statement_timeout=30000zz`) проходил ЗЕЛЁНЫМ:
статическая проверка искала ПОДСТРОКУ (а `…=30000` — префикс испорченного), а
живая глушила отказ коннекта голым `except` → skip → запись в allowlist. На
проде это `FATAL: invalid value for parameter` на КАЖДОМ коннекте, то есть
полный отказ продукта при зелёном сьюте. Теперь: сравнение `options` на
РАВЕНСТВО, и `_live_engine` сначала пробует коннект БЕЗ `connect_args` — сервера
нет это пропуск, а «сервер есть, наши options он не принял» это падение.
3. LOW. У проверки согласованности был пол и не было крыши: `300_000` (пять
минут) зеленел. Добавлена симметричная граница `<= 2 ×` самого длинного
объявленного бюджета.
4. LOW. Три факта в комментариях исправлены:
* `pg_stat_statements` НЕ опора по планировщику — вытесняет записи с calls=1
(`dealloc` вырос за десять минут, из топа пропал `REFRESH MATERIALIZED VIEW`
30.85 с). Основная опора — `scrape_runs`;
* самый длинный set-based statement через движок — матч ГАР→houses: 2.07 с с
городским фильтром и 6.46 с без. Запас ~3×, а не 7×;
* `idle in transaction` 29 с — это tgbot (`services/tgbot/bridge.py`,
транзакция поверх long-poll Telegram; сверено 3 пробами: одна и та же
сессия, `SELECT value FROM tg_support_state …`), а не свипы. Решение не
ставить потолок на простой от этого только крепче.
Мутационная проверка (обе лэйны краснеют на каждой): испорченный `options`,
`_STATEMENT_TIMEOUT_MS = 300_000`, снятый `connect_args`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Задача landing_showcase_deals не запускалась вообще: строки в
scrape_schedules не было (0 строк по '%showcase%' на проде 12.09), а в
реестре product_handlers — обработчика. Пересчёт был ручным шагом, и
лэндинг показывал прогон от 30.08 — тринадцать суток.
Что сделано:
- Handler `landing_showcase_deals` в product_handlers: тело задачи
писалось под `python -m` и про run_id не знает, поэтому done/failed
ставит обработчик (как у refresh_search_matview).
- Миграция 303 сеет расписание: enabled=true, окно 06:00–07:00 UTC,
interval_days=1. Такт суточный не из-за данных — сделки Росреестра
квартальные, — а из-за кода: прогноз считает тот же спайн оценщика,
что и боевой расчёт, и любой деплой меняет числа на витрине, не
трогая ни одной сделки.
- Эта же строка заводит витрину в СУЩЕСТВУЮЩИЙ монитор свежести:
сводка просроченных источников (emit_stale_digest, #2670) ходит по
включённым расписаниям и бьёт ERROR → GlitchTip, когда источник
молчит дольше 3× своего такта. Своего монитора не заводим: витрина
была невидима не потому, что сводка не умеет про неё говорить, а
потому, что источника для сводки не существовало.
- На странице под таблицей — дата прогона рядом со счётчиками:
«Витрина пересчитана 30.08.2026». computed_at ручка /showcase отдавала
и раньше, фронт его не показывал; даты нет — предложения нет.
Тесты: обработчик резолвится тем же resolve_handler, что и боевой
_dispatch; миграция на живой БД реально кладёт строку и не задваивает
её при повторе; настоящий запрос сводки видит витрину и отдаёт её
просроченной после 3× такта (число тактов — литерал из приёмки, не
константа кита: взятое из неё ожидание уезжало вместе с ней и держало
тесты зелёными при факторе 3650).
Closes#3469
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
На боевой БД `statement_timeout`, `lock_timeout` и
`idle_in_transaction_session_timeout` равны 0, а у движка `app/core/db.py` не
было `connect_args` вовсе. После #3444/#3449 шаги БД на пути `/estimate` идут
через обёртку, которая при отмене по бюджету ДОЖИДАЕТСЯ своего потока (иначе он
остаётся сиротой в общей `Session`) — ожидание верное, но его верхняя граница
равна длительности самого запроса, а у запроса границы не было. Один
`ACCESS EXCLUSIVE` на таблице → четыре повисших запроса → `_ESTIMATE_CONCURRENCY`
исчерпан → `/estimate` отдаёт 429 всем остальным.
Потолок ставится на КОННЕКТЕ (libpq `options`), а не в обёртке: таймаут в
обёртке вернул бы ровно ту сироту, ради которой писался #3449.
statement_timeout = 30 с: выше самого длинного ОБЪЯВЛЕННОГО бюджета `/estimate`
(20 с, `estimate_avito_imv_timeout_s`) в 1.5 раза и в 7 раз выше самого долгого
ЗАМЕРЕННОГО запроса через этот движок (4.27 с, `pg_stat_statements` на проде за
16 суток), но конечен. lock_timeout = 5 с: та же величина, что у миграций
проекта, и больше `deadlock_timeout` (1 с на проде).
`idle_in_transaction_session_timeout` намеренно не трогаем: тем же движком живёт
планировщик, а его свипы держат транзакцию открытой всё время внешнего HTTP
(замер: живая сессия `idle in transaction` 29 с).
Задачи планировщика проверены, а не предположены: у `listing_source_snapshot`
свой `SET LOCAL statement_timeout = 900000`, и тест доказывает, что `SET LOCAL`
ПЕРЕКРЫВАЕТ сессионный потолок и не течёт за свою транзакцию. Самая долгая
чисто-БД задача по `scrape_runs` за 14 суток укладывается в 9.7 с целиком;
единственный запрос длиннее 20 с на всей БД (`REFRESH MATERIALIZED VIEW
CONCURRENTLY`, 30.85 с) идёт мимо движка — по своему сырому psycopg-соединению.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Фоновый дослальщик GlitchTip спит по настоящим часам: три попытки по
тридцать секунд. TestClient ждёт завершения background-задачи, привязанной
к ответу, поэтому один тест на отказ доставки держал весь прогон минуту, а
в CI прогон умирал по таймауту без единой строки об ошибке — набор
выглядел «медленным», хотя на деле висел.
Проверять надо, что дослальщик вызван и сколько раз, а не то, что
интерпретатор умеет спать. Файл тестов вебхука: было зависание, стало
19 passed за 1.36с.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JY6iWDnGDthdvsMWgK1BMG
Root cause of the red PR #3494 CI job (9% progress, 75s life, no error line):
test_send_message_rate_limits_across_different_topics_same_chat mocked
asyncio.sleep as a pure no-op without advancing time.monotonic. The 3rd send
(over the test's limit=2) entered TelegramGroupRateLimiter.acquire(), which
recomputes wait_s from the real, unmocked clock every iteration - since the
fake sleep never advances it, the window never expires and the while-loop
busy-spins forever instead of actually waiting, until pytest-timeout kills it.
Fixed by advancing a fake monotonic clock inside fake_sleep, matching the
already-correct pattern used by the other tests in this file.
Also added _reset_telegram_shared_client (tests/conftest.py, same pattern as
_reset_estimate_rate_limiter): app.services.tgbot.shared._client is a
module-level singleton whose rate limiter otherwise accumulates real
wall-clock timestamps across the whole pytest session, not per test.
Documented honestly in config.py: the API-role budget is shared between
support web-chat mirrors and GlitchTip alerts with no priority between them,
so a large alert burst can make the web-chat wait out its own timeout and
return 502 - flagged as a known follow-up, not fixed here.
NOTE: a full `pytest -q --timeout=60` run still hangs further into the suite,
at tests/test_glitchtip_webhook.py::test_telegram_failure_returns_502_not_500.
Not root-caused within this session's budget - the test's _fake_telegram_client
fixture correctly monkeypatches glitchtip_module.get_telegram_client, but the
anyio worker thread running the ASGI request is seen parked in a real
event-loop poll/select wait, consistent with an actual (non-mocked) sleep
somewhere in that path. Needs a follow-up session with a fresh time budget.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JY6iWDnGDthdvsMWgK1BMG
Deep review of PR #3494 found the rate limiter unusable as designed:
- H1: acquire() waited unbounded even for interactive HTTP handlers (support.py,
glitchtip.py already pass a narrow `timeout` — reuse it as the queue wait cap
instead of editing those handlers, which are out of scope here). New
TelegramRateLimitedError (subclass of TelegramError) gives a fast, honest
502 instead of hanging past the caller's own budget.
- H2: the limiter is per-process (in-memory), but two processes write to the
same group (uvicorn API + bot worker) — giving each the same 18/min doubled
the platform ceiling. Split into telegram_group_rate_limit_api_per_minute
(12) and _bot_per_minute (6), sum kept below ~20.
- M1: bridge.py sends without an explicit timeout inherited "wait forever",
stalling the single-threaded poll loop (open DB session) past the SIGTERM
drain window. Bounded via rate_limit_max_wait=20s at the six call sites.
- M2: _locks/_sent_at grew unbounded on every unique DM chat_id. Added
opportunistic cleanup of fully-expired entries.
- L1: the "queue full" warning now logs once per acquire() call, not once
per sleep iteration.
- Corrected a factual error in the docstring: TELEGRAM_SUPPORT_CHAT_ID and
TELEGRAM_ALERTS_CHAT_ID are the SAME group on prod (topics differ only).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JY6iWDnGDthdvsMWgK1BMG
