feat(mera/b2c): правовая рамка — согласие до сохранения, удаление по сроку и по запросу — этап 4 из 8 #2547

Merged

lekss361 merged 7 commits from feat/mera-b2c-privacy into main 2026-08-06 17:04:27 +00:00

Conversation 3 Commits 7 Files changed 16 +2062 -12

lekss361 commented 2026-07-28 12:25:33 +00:00

Owner

Copy link

Четвёртый этап плана B2C. Наружу ничего не открывает, мержится независимо от DNS.

Три дефекта, каждый блокировал легальный публичный запуск

Адрес физлица сохранялся до любого согласия. Согласие фиксировалось только на форме заявки — то есть после записи адреса в базу. Для пилота с договором это терпимо, для человека с улицы нет. Теперь проверка стоит первой строкой расчёта, до геокодирования и до обоих мест записи адреса.

Срок жизни оценки не приводил к удалению. Поле срока применялось только как фильтр при чтении, физического удаления не было ни в одной из 23 фоновых задач — данные жили вечно вопреки декларации. У заявок срока не было вовсе.

Пути «удалите мои данные» не существовало.

Решения, которые стоит отметить

Хранение согласия — колонками на самой оценке, 1:1 с уже работающим прецедентом для заявок: IP клиента, версия политики, дословный снимок текста. Отдельную таблицу событий не заводили сознательно — согласие даётся ровно на создание этой строки, и когда строка удаляется по сроку, исчезновение доказательства вместе с данными логично: персональных данных больше нет, свидетельствовать не о чем.

Проверка не выводится из пустого имени пользователя. Первая версия так и делала — и сломала 92 несвязанных теста оценщика, которые вызывают расчёт без имени, проверяя ценовую логику. Заменено на явный флаг, который выставляет единственный боевой вызывающий. B2B-поток не тронут: поле согласия опционально, иначе сломались бы пилоты, чей фронт его вообще не шлёт.

Задача удаления засеяна выключенной. Это первая автоматическая задача в trade-in, которая удаляет персональные данные — первый прогон должен пройти под наблюдением, а не по расписанию. Тот же приём уже применялся в проекте. Включается одной командой после проверки.

Честно зафиксированные ограничения

Аноним без ссылки на оценку, без телефона и без обращения в поддержку — неидентифицируем. Удалить его данные без дополнительной идентификации невозможно, схема этого не позволяет. Записано в докстринге, а не умолчано.

Удаление чистит только копию в базе. Зеркало переписки в Telegram-топике не удаляется ничем в кодовой базе — нужен отдельный ручной шаг через API бота. Зафиксировано в коде.

События аудита этим механизмом не чистятся — там в полезной нагрузке есть адрес. Является ли журнал аудита законным основанием пережить запрос на удаление — вопрос к юристу, не инженерное решение.

Что за юристом

Конкретные сроки хранения — инженерное предложение с обоснованием, не юридический вывод. Для сконвертированных заявок нужен другой, договорной срок, и механизма пометки «сконвертирован» в схеме сейчас нет.

Test plan

• uv run pytest -q — 2775 passed, 9 skipped
• ruff check чисто, проверка на ловушку :x::type чисто
• Тесты: согласие фиксируется до создания оценки, задача удаления реально удаляет и идемпотентна, расхождение текста согласия ловится, B2B-поток не сломан
• Тест синхронности текста согласия фронт↔бэк — теперь настоящий, а не комментарий
• Первый прогон задачи удаления — под наблюдением, вручную
• Миграции против живой базы не применялись

Четвёртый этап плана B2C. Наружу ничего не открывает, мержится независимо от DNS. ## Три дефекта, каждый блокировал легальный публичный запуск **Адрес физлица сохранялся до любого согласия.** Согласие фиксировалось только на форме заявки — то есть **после** записи адреса в базу. Для пилота с договором это терпимо, для человека с улицы нет. Теперь проверка стоит первой строкой расчёта, до геокодирования и до обоих мест записи адреса. **Срок жизни оценки не приводил к удалению.** Поле срока применялось только как фильтр при чтении, физического удаления не было ни в одной из 23 фоновых задач — данные жили вечно вопреки декларации. У заявок срока не было вовсе. **Пути «удалите мои данные» не существовало.** ## Решения, которые стоит отметить **Хранение согласия — колонками на самой оценке**, 1:1 с уже работающим прецедентом для заявок: IP клиента, версия политики, дословный снимок текста. Отдельную таблицу событий не заводили сознательно — согласие даётся ровно на создание этой строки, и когда строка удаляется по сроку, исчезновение доказательства вместе с данными логично: персональных данных больше нет, свидетельствовать не о чем. **Проверка не выводится из пустого имени пользователя.** Первая версия так и делала — и сломала **92 несвязанных теста** оценщика, которые вызывают расчёт без имени, проверяя ценовую логику. Заменено на явный флаг, который выставляет единственный боевой вызывающий. B2B-поток не тронут: поле согласия опционально, иначе сломались бы пилоты, чей фронт его вообще не шлёт. **Задача удаления засеяна выключенной.** Это первая автоматическая задача в trade-in, которая удаляет персональные данные — первый прогон должен пройти под наблюдением, а не по расписанию. Тот же приём уже применялся в проекте. Включается одной командой после проверки. ## Честно зафиксированные ограничения **Аноним без ссылки на оценку, без телефона и без обращения в поддержку — неидентифицируем.** Удалить его данные без дополнительной идентификации невозможно, схема этого не позволяет. Записано в докстринге, а не умолчано. **Удаление чистит только копию в базе.** Зеркало переписки в Telegram-топике не удаляется ничем в кодовой базе — нужен отдельный ручной шаг через API бота. Зафиксировано в коде. **События аудита этим механизмом не чистятся** — там в полезной нагрузке есть адрес. Является ли журнал аудита законным основанием пережить запрос на удаление — вопрос к юристу, не инженерное решение. ## Что за юристом Конкретные сроки хранения — инженерное предложение с обоснованием, не юридический вывод. Для сконвертированных заявок нужен другой, договорной срок, и механизма пометки «сконвертирован» в схеме сейчас нет. ## Test plan - [x] `uv run pytest -q` — **2775 passed**, 9 skipped - [x] `ruff check` чисто, проверка на ловушку `:x::type` чисто - [x] Тесты: согласие фиксируется до создания оценки, задача удаления реально удаляет и идемпотентна, расхождение текста согласия ловится, B2B-поток не сломан - [x] Тест синхронности текста согласия фронт↔бэк — теперь настоящий, а не комментарий - [ ] Первый прогон задачи удаления — под наблюдением, вручную - [ ] Миграции против живой базы не применялись

lekss361 added 1 commit 2026-07-28 12:25:34 +00:00

feat(mera/b2c): правовая рамка — согласие до сохранения, удаление по сроку и по запросу (этап 4 из 8) ... 5626d9e720

Три дефекта, каждый блокировал легальный публичный запуск.

1. Адрес физлица сохранялся в базу ДО любого согласия: согласие фиксировалось
   только на форме заявки, то есть ПОСЛЕ записи адреса. Для пилота с договором
   терпимо, для человека с улицы — нет. Проверка согласия поставлена первой
   строкой расчёта, до геокодирования и до обоих мест записи адреса.

   Хранение — колонками на самой оценке, 1:1 с уже работающим прецедентом для
   заявок (миграция 182): IP клиента, версия политики, дословный снимок текста.
   Отдельная таблица событий не заводилась: согласие даётся ровно на создание
   этой строки, и когда строка удаляется по сроку, исчезновение доказательства
   вместе с данными логично.

   Enforcement НЕ выводится из пустого created_by — первая версия так и делала
   и сломала 92 несвязанных теста оценщика, которые зовут расчёт без имени
   пользователя, проверяя ценовую логику. Вместо этого явный флаг, который
   выставляет единственный боевой вызывающий. B2B-поток не тронут: поле
   согласия опционально, иначе сломались бы пилоты, чей фронт его не шлёт.

2. Срок жизни оценки применялся только как фильтр при чтении — физического
   удаления не было ни в одной фоновой задаче, данные жили вечно вопреки
   декларированному сроку. Заведена задача удаления пачками с ограничением на
   прогон и коммитом после каждой пачки, идемпотентная. В расписании она
   ВЫКЛЮЧЕНА: это первая автоматическая задача, удаляющая персональные данные,
   и первый прогон должен быть под наблюдением.

3. Пути «удалите мои данные» не было. Добавлен сервис удаления и админская
   ручка. Ключи: имя пользователя, идентификатор оценки, телефон, чат в
   телеграме.

   Честно зафиксировано в коде: аноним без ссылки на оценку, без оставленного
   телефона и без обращения в поддержку неидентифицируем — удалить его данные
   без дополнительной идентификации нельзя. Отдельно: удаление чистит только
   копию в базе, зеркало переписки в телеграм-топике не удаляется ничем в
   кодовой базе, нужен ручной шаг.

4. Соответствие текста согласия на фронте и снимка на бэке держалось на
   комментарии. Теперь есть тест, который ловит расхождение.

Сроки хранения вынесены в настройки. Значение для заявок предложено инженерно
(типичный отраслевой диапазон), юридически обоснованный срок — за юристом, и
это записано в коде.

Тесты: 2775 passed.

lekss361 commented 2026-07-31 20:17:56 +00:00

Author

Owner

Copy link

Замечание со стороны (в код ветки не лезу): коллизия номеров миграций.

Этот PR добавляет 192_trade_in_estimates_consent_proof.sql и 193_trade_in_privacy_retention.sql, но 192 и 193 уже заняты на main — 192_tradein_users_auth.sql и 193_tradein_users_seed.sql (смержены в #2597). Свободные номера на сейчас — 197+ (196 занят #2598, da329cda).

Без переименования это ломает pytest-гейт деплоя уже ПОСЛЕ мержа, когда откатывать дороже. Замечено при ревью соседнего PR.

Замечание со стороны (в код ветки не лезу): коллизия номеров миграций. Этот PR добавляет `192_trade_in_estimates_consent_proof.sql` и `193_trade_in_privacy_retention.sql`, но 192 и 193 уже заняты на `main` — `192_tradein_users_auth.sql` и `193_tradein_users_seed.sql` (смержены в #2597). Свободные номера на сейчас — 197+ (196 занят #2598, `da329cda`). Без переименования это ломает pytest-гейт деплоя уже ПОСЛЕ мержа, когда откатывать дороже. Замечено при ревью соседнего PR.

lekss361 referenced this pull request 2026-08-02 13:52:43 +00:00

chore(tradein/scraper): cut wasteful scraping load - cian_full_load tempo + zero-yield jobs #2633

bot-backend commented 2026-08-05 22:50:14 +00:00

Collaborator

Copy link

Замечание не по существу PR, а по номерам миграций — заметил, пока чинил соседнее.

Этот PR приносит 192_trade_in_estimates_consent_proof.sql и 193_trade_in_privacy_retention.sql, а в main уже лежат 192_tradein_users_auth.sql и 193_tradein_users_seed.sql. Имена файлов разные, поэтому применение не сломается и ничего не пропустится — учёт идёт по полному имени. Но номера коллидируют.

Стоит перенумеровать перед мержем. Сегодня ночью в такую же коллизию въехали два моих агента (оба взяли 212, потому что каждый смотрел только на свою ветку), и заодно выяснилось, почему гейт-тест это не ловит: он сравнивает префиксы файлов в одном рабочем дереве, а чужой файл из параллельной ветки там физически отсутствует. То есть защита исправна, но кросс-веточную коллизию поймать не может по построению — она видна только после мержа, когда номер уже разошёлся.

Свободные номера на сейчас: в main последний 213, плюс заняты открытыми PR 214 (#2684) и 215 (#2685). То есть безопасный старт — с 216, но перед коммитом лучше сверить ещё раз: за ночь список подвинулся четыре раза.

Отдельно завёл #2683 про то, что файл-манифест применённых миграций отстал на 24 записи и сам себе противоречит — там же предложение строить защиту от коллизий на сверке с main в CI, а не на манифесте.

Замечание не по существу PR, а по номерам миграций — заметил, пока чинил соседнее. Этот PR приносит `192_trade_in_estimates_consent_proof.sql` и `193_trade_in_privacy_retention.sql`, а в `main` уже лежат `192_tradein_users_auth.sql` и `193_tradein_users_seed.sql`. Имена файлов разные, поэтому применение не сломается и ничего не пропустится — учёт идёт по полному имени. Но номера коллидируют. Стоит перенумеровать перед мержем. Сегодня ночью в такую же коллизию въехали два моих агента (оба взяли 212, потому что каждый смотрел только на свою ветку), и заодно выяснилось, **почему гейт-тест это не ловит**: он сравнивает префиксы файлов **в одном рабочем дереве**, а чужой файл из параллельной ветки там физически отсутствует. То есть защита исправна, но кросс-веточную коллизию поймать не может по построению — она видна только после мержа, когда номер уже разошёлся. Свободные номера на сейчас: в `main` последний **213**, плюс заняты открытыми PR **214** (#2684) и **215** (#2685). То есть безопасный старт — с 216, но перед коммитом лучше сверить ещё раз: за ночь список подвинулся четыре раза. Отдельно завёл #2683 про то, что файл-манифест применённых миграций отстал на 24 записи и сам себе противоречит — там же предложение строить защиту от коллизий на сверке с `main` в CI, а не на манифесте.

bot-backend referenced this pull request from a commit 2026-08-06 12:52:56 +00:00

fix(tradein/payments): NULLS NOT DISTINCT дедуп, pd_erased_at, статусы T-Bank

bot-backend referenced this pull request from a commit 2026-08-06 16:07:23 +00:00

fix(tradein/payments): разрешить коллизию номера миграции 228 -> 232

bot-backend added 4 commits 2026-08-06 16:10:48 +00:00

Merge remote-tracking branch 'forgejo/main' into pr2547-privacy-work ... 6820337da0

# Conflicts:
#	tradein-mvp/backend/app/services/estimator.py
#	tradein-mvp/backend/app/services/product_handlers.py

chore(tradein/privacy): перенумерация миграций и merge main - разблокировка PR (#2547) ... dccd2d4272

192/193 -> 229/230: main занял 192_tradein_users_auth.sql и
193_tradein_users_seed.sql за время простоя PR. 228 зарезервирован
открытым PR #2732 (228_payments.sql) - следующие реально свободные
229/230, порядок consent_proof -> retention сохранён.

Правки ссылок на старые имена/префиксы: docstring-заголовки самих
SQL-файлов, перекрёстная ссылка 229 -> 230 в комментарии-докстринге,
комментарии migration 192/193 в lead.py / config.py / schemas/trade_in.py
/ purge_expired_trade_in_data.py, переменные и имена тестов в
test_estimate_consent_gate.py / test_purge_expired_trade_in_data.py.
(Оставлены нетронутыми ссылки на migration 192/193 в auth_session.py и
test_team_api.py - это про другие, уже существующие на main миграции
192_tradein_users_auth.sql / 193_tradein_users_seed.sql, не про эту
пару.)

Merge remote-tracking branch 'forgejo/main' into pr2547-privacy-work b93bee5393

chore(tradein/privacy): перенумерация 231 и merge main - коллизия префикса (#2547) 3ee99efaa4

bot-backend referenced this pull request from a commit 2026-08-06 16:10:49 +00:00

chore(tradein/privacy): перенумерация миграций и merge main - разблокировка PR (#2547)

bot-backend referenced this pull request from a commit 2026-08-06 16:10:49 +00:00

chore(tradein/privacy): перенумерация 231 и merge main - коллизия префикса (#2547)

lekss361 commented 2026-08-06 16:31:38 +00:00

Author

Owner

Copy link

Deep review — 🟠 HIGH, merge на паузе

Гейт пройден: CI Trade-In / backend-tests = success 3m21s на 3ee99efa (явный success, не отсутствие красного). Миграции 229/231 прогнаны на живой прод-схеме в BEGIN…ROLLBACK, тело дважды — чисто. Согласие-гейт разобран, B2B не сломан. Одна находка держит мерж.

---

🟠 HIGH — предикат удаления = 24-часовой TTL ссылки, применённый к архиву истории пилотов

app/tasks/purge_expired_trade_in_data.py:56-66 удаляет trade_in_estimates WHERE expires_at < NOW(), а expires_at = created_at + trade_in_estimate_retention_hours (24ч) ставится каждой оценке, включая B2B.

Замер на проде прямо сейчас:

total 1057 | expires_at < NOW() → 1040 | из них с created_by → 911
admin 571 · kopylov 110 · user2 77 · praktika 50 · pilottest 40 · admintest 24 · user1 24 · <NULL> 129

max_batches × batch_size = 20 × 500 = 10000 ≫ 1040 → вся таблица уходит за ПЕРВЫЙ прогон, необратимо.

Что при этом ломается:

• GET /api/v1/trade-in/history (app/api/v1/trade_in.py:735-748) — фильтра expires_at там нет. Это раздел «История» в Topbar (frontend/src/components/trade-in/Topbar.tsx:153, frontend/src/app/history/page.tsx:28) и источник KPI v2-дашборда (frontend/src/components/trade-in/v2/mappers.ts:2257+). Пилоты теряют весь архив оценок.
• GET /team/employees/{id}/history (app/api/v1/team.py:806) — LEFT JOIN trade_in_estimates → median_price/confidence/n_analogs становятся NULL по всей истории сотрудников.
• cache-stats (trade_in.py:793-805) — estimates_total / avg_median_price / repeat_address_pct схлопываются до последних суток.

Корень — смешение двух смыслов. Сегодня expires_at это TTL ссылки/PDF (get_estimate → 404, estimate_pdf → 410 «estimate expired (24h TTL)»), а не срок хранения записи. Комментарий в config.py:822 это прямо и говорит («оценка живёт «сессию» клиента, не архив») — но продукт использует таблицу именно как архив.

Ровно ту же асимметрию PR уже проводит сам: _estimate_consent_persist_fields осознанно НЕ пишет доказательство согласия для B2B, потому что «согласие закрыто договором, НЕ UI-чекбоксом». Со сроком хранения аргумент симметричен: у пилота по договору свой срок, не анонимные 24 часа.

Почему не 🔴: задача действительно засеяна выключенной — проверено втройне: 231:82 seed false; kit-планировщик выбирает только WHERE enabled = true (packages/scraper-kit/src/scraper_kit/orchestration/scheduler.py:730); прод-прогон подтвердил enabled=f. На деплое не запустится ничего.

Почему всё-таки держим: докстринг миграции 231 (строки 36-39) выдаёт оператору однострочную команду включения и описывает первый прогон просто как «supervised», без единого слова о том, что он снесёт 911 строк пилотов. В Test plan этого же PR стоит незакрытый пункт «Первый прогон задачи удаления — под наблюдением, вручную» — то есть спусковой крючок уже стоит в очереди.

Любой из вариантов закрывает, все маленькие:

(a) сузить до популяции, ради которой PR и делался:

WHERE expires_at < NOW() AND created_by IS NULL

(129 строк сегодня, все legacy/безхозные — архив пилотов цел);

(b) развести часы: удалять по created_at < NOW() - make_interval(days => :purge_days) с новой настройкой trade_in_estimate_purge_days (напр. 180), оставив expires_at как TTL ссылки;

(c) если 24ч — это и правда срок хранения записи в т.ч. для пилотов, то это продуктовое решение, которое принимается явно и с переделкой /history заранее, а не обнаруживается по факту пустого архива.

Независимо от выбора: перенести предупреждение в докстринг рядом с командой включения и добавить preflight-счётчик, чтобы оператор перед включением видел «будет удалено N строк, из них M с владельцем».

---

🟡 MEDIUM — удаление по телефону это точное сравнение строк → тихий no-op

app/services/data_erasure.py:126-131 матчит phone = :phone буквально. lead.py:69-84 хранит телефон как прислали (только regex + счёт цифр, нормализации в E.164 нет намеренно, #2376). Оператор, вводящий 8 (999) 123-45-67 против сохранённого +79991234567, получает {"trade_in_leads_deleted": 0} и HTTP 200 — запрос на удаление отчитывается выполненным, данные остаются. Для legal-erasure ручки тихий промах это худший из режимов отказа.

OR (:phone IS NOT NULL
    AND regexp_replace(phone, '\D', '', 'g') = regexp_replace(:phone, '\D', '', 'g'))

(4 строки на проде, вопрос производительности не стоит)

---

🟢 Мелочи

1. 231:56-64 — ADD COLUMN … SET NOT NULL без DEFAULT. Миграции применяются ДО перезапуска контейнеров, поэтому старый код, вставляющий лид в это ~минутное окно, словит NOT NULL violation. При 4 лидах в месяц вероятность пренебрежимая, но ALTER COLUMN expires_at SET DEFAULT NOW() + interval '180 days' закрывает бесплатно.
2. Ограничения из описания действительно живут в коде, а не только в PR: аноним-неидентифицируем (data_erasure.py:11-29), Telegram-зеркало (31-40 + врезка на 152-155), user_events не чистится (42-51). Это сделано хорошо. Не зафиксировано нигде другое: при открытии анонимного флоу адрес попадёт в geocode_cache через ручку автодополнения/геокодинга раньше чекбокса — гейт закрывает только путь /estimate. Строчка в блок _ESTIMATE_CONSENT_* пригодится следующему этапу.
3. Сроки параметризованы, не захардкожены: trade_in_estimate_retention_hours=24, trade_in_lead_retention_days=180, trade_in_purge_batch_size=500 в config.py:817-844, все через ENV; прежние два хардкода timedelta(hours=24) в estimator.py убраны. Единственный литерал — interval '180 days' в backfill'е 231, и это правильно (миграция = снимок на момент применения) и там же объяснено.
4. _DEFAULT_MAX_BATCHES = 20 живёт модульной константой, тогда как batch_size — в settings. Асимметрия косметическая: default_params несёт оба, переопределяется на уровне расписания.

---

Что проверено и сходится

• Миграции. 229 и 231 применяются на реальной прод-схеме и идемпотентны (dry-run в BEGIN…ROLLBACK, тело дважды: проход 1 — весь DDL, проход 2 — сплошь «already exists, skipping»). Seed садится как enabled=f, next_run_at 2026-08-07 02:00+00, {"batch_size":500,"max_batches":20}. ON CONFLICT (source) опирается на существующий scrape_schedules_source_key UNIQUE (source). Backfill проставил 4 лидам Dec 2026 / Jan 2027, под удаление сейчас не попадает ни один.
• Фальсификация CHECK. INSERT … consent=false → violates check constraint "trade_in_estimates_consent_not_false". Работает.
• Номера свободны везде. 229/231 нет на текущем main (там 228 и 230), нет в _schema_migrations на проде, и ни один открытый PR их не занимает (#2732 и #2742 оба взяли 232 — это их коллизия, не этого PR).
• B2B не сломан. require_consent = x_authenticated_user is None; /api/v1/trade-in/estimate не входит в _PUBLIC_PATHS (rbac.py:71-86), а session-cookie путь инжектит X-Authenticated-User прямо в ASGI scope (_propagate_authenticated_user, rbac.py:207) — значит хендлер сегодня всегда видит username, и ветка гейта недостижима на проде. consent: bool | None = None в схеме оставляет payload'ы пилотов (которые поле не шлют) валидными.
• Согласие до первой записи адреса — по всем путям. Гейт стоит первым оператором тела estimate_quality (estimator.py:3504-3513), до geocode() и до обоих INSERT (основной + _empty_estimate). schedule_event(estimate_request, payload={address,…}) срабатывает уже ПОСЛЕ возврата из estimate_quality (trade_in.py:207-220) — на 422 адрес в user_events не попадает. RequestAuditMiddleware пишет только status_code, тело не логирует. account_quota.check_and_raise при username=None это no-op.
• Идемпотентность и частичный сбой purge. Каждый батч со своим commit; исключение посреди прогона откатывает только незакоммиченный батч, mark_failed фиксирует накопленные счётчики, повторный запуск просто матчит меньше строк. Тесты это покрывают (test_purge_expired_trade_in_data.py:167-198).
• Каскады. estimate_photos хранит content bytea в самой БД (007_estimate_photos.sql:12) — CASCADE уносит фото целиком, осиротевших файлов на диске не остаётся.
• Атрибуция удаления. RequestAuditMiddleware пишет admin_action для мутирующих /api/v1/admin/** (app/core/request_audit.py:91-100) → кто и когда запросил стирание, в журнале есть (без тела).

---

Пересечение с платёжным контуром (#2732)

Коротко: pd_erased_at это заглушка, а не механизм — но расширять #2547 сейчас НЕ надо. Расширять на PR-D, до первого живого платежа.

Почему не сейчас: 232_payments.sql ещё не применён (на проде _schema_migrations заканчивается на 230), а 231 идёт раньше 232 — задача, ссылающаяся на payments, свалится на схеме, где таблицы ещё нет. Плюс PAYMENTS_ENABLED/TBANK_* на проде отсутствуют, защищать сегодня нечего.

Что именно и когда — три пункта, все вместе с PR-D или до него, потому что после первого живого платежа они становятся ретроактивно невыполнимыми:

1. payment_notifications.body — настоящая дыра. 232_payments.sql:214: body jsonb NOT NULL -- полное тело нотификации как есть, append-only, без TTL и без пути стирания. При подключённой кассе в теле приезжает Receipt.Email / Receipt.Phone. Самое дешёвое и надёжное — не сохранять: вырезать поддерево Receipt перед INSERT (скаляры, участвующие в подписи, остаются — подпись считается по top-level полям, ср. #2733). Позже редактировать append-only хранилище дорого, не класть туда — одна строка в PR-D.
2. payments.customer_email / customer_phone в erase_person_data — именно UPDATE, не DELETE (фискальный след по 54-ФЗ обязан пережить: order_id / amount_kopecks / product_code / status остаются):

UPDATE payments SET customer_email = NULL, customer_phone = NULL, pd_erased_at = NOW()
WHERE (estimate_id = ANY(CAST(:ids AS uuid[]))
       OR lead_id = ANY(CAST(:lead_ids AS uuid[]))
       OR created_by = :username)
  AND pd_erased_at IS NULL

Здесь ловушка порядка: id лидов надо захватить ДО того, как шаг 2 их удалит — ровно та же проблема, которую модуль уже решает для оценок (data_erasure.py:103-107). Сейчас функция собирает estimate_ids, но не lead_ids, поэтому правка в PR-D будет не чисто аддитивной. Фиксирую сейчас, чтобы не переоткрывать потом.
3. TTL для payment_notifications — это артефакт идемпотентности и отладки, не фискальный документ. Третий вызов _drain_expired в этой же задаче, когда 232 будет применён. Естественное место, новой машинерии не требует.

Отдельно, в плюс #2732 и стоит сохранить: payments_lead_idx по lead_id там есть (232_payments.sql:195) с комментарием ровно про seq scan от FK ON DELETE SET NULL — а это именно то, что ночная чистка лидов из этого PR и провоцировала бы. Взаимодействие двух PR здесь учтено.

---

Дальше

1. Одна из правок (a)/(b)/(c) по HIGH + предупреждение рядом с командой включения.
2. Опционально в этом же заходе — digits-only матч по телефону (MEDIUM).
3. Пуш → перечитаю новый SHA, дождусь явного success CI Trade-In / backend-tests, смержу.

Не мержу до этого: 911 строк пилотов (включая user2/brusnika, доступ которому вернули три коммита назад) не восстанавливаются.

## Deep review — 🟠 HIGH, merge на паузе Гейт пройден: `CI Trade-In / backend-tests` = success 3m21s на `3ee99efa` (явный success, не отсутствие красного). Миграции 229/231 прогнаны на живой прод-схеме в `BEGIN…ROLLBACK`, тело дважды — чисто. Согласие-гейт разобран, B2B не сломан. Одна находка держит мерж. --- ### 🟠 HIGH — предикат удаления = 24-часовой TTL ссылки, применённый к архиву истории пилотов `app/tasks/purge_expired_trade_in_data.py:56-66` удаляет `trade_in_estimates WHERE expires_at < NOW()`, а `expires_at = created_at + trade_in_estimate_retention_hours` (24ч) ставится **каждой** оценке, включая B2B. Замер на проде прямо сейчас: ``` total 1057 | expires_at < NOW() → 1040 | из них с created_by → 911 admin 571 · kopylov 110 · user2 77 · praktika 50 · pilottest 40 · admintest 24 · user1 24 · <NULL> 129 ``` `max_batches × batch_size = 20 × 500 = 10000` ≫ 1040 → **вся таблица уходит за ПЕРВЫЙ прогон**, необратимо. Что при этом ломается: - `GET /api/v1/trade-in/history` (`app/api/v1/trade_in.py:735-748`) — фильтра `expires_at` там **нет**. Это раздел «История» в Topbar (`frontend/src/components/trade-in/Topbar.tsx:153`, `frontend/src/app/history/page.tsx:28`) и источник KPI v2-дашборда (`frontend/src/components/trade-in/v2/mappers.ts:2257+`). Пилоты теряют весь архив оценок. - `GET /team/employees/{id}/history` (`app/api/v1/team.py:806`) — `LEFT JOIN trade_in_estimates` → `median_price`/`confidence`/`n_analogs` становятся NULL по всей истории сотрудников. - `cache-stats` (`trade_in.py:793-805`) — `estimates_total` / `avg_median_price` / `repeat_address_pct` схлопываются до последних суток. Корень — смешение двух смыслов. Сегодня `expires_at` это TTL **ссылки/PDF** (`get_estimate` → 404, `estimate_pdf` → 410 «estimate expired (24h TTL)»), а не срок хранения **записи**. Комментарий в `config.py:822` это прямо и говорит («оценка живёт «сессию» клиента, не архив») — но продукт использует таблицу именно как архив. Ровно ту же асимметрию PR уже проводит сам: `_estimate_consent_persist_fields` осознанно НЕ пишет доказательство согласия для B2B, потому что «согласие закрыто договором, НЕ UI-чекбоксом». Со сроком хранения аргумент симметричен: у пилота по договору свой срок, не анонимные 24 часа. **Почему не 🔴:** задача действительно засеяна выключенной — проверено втройне: `231:82` seed `false`; kit-планировщик выбирает только `WHERE enabled = true` (`packages/scraper-kit/src/scraper_kit/orchestration/scheduler.py:730`); прод-прогон подтвердил `enabled=f`. На деплое не запустится ничего. **Почему всё-таки держим:** докстринг миграции 231 (строки 36-39) выдаёт оператору однострочную команду включения и описывает первый прогон просто как «supervised», без единого слова о том, что он снесёт 911 строк пилотов. В Test plan этого же PR стоит незакрытый пункт «Первый прогон задачи удаления — под наблюдением, вручную» — то есть спусковой крючок уже стоит в очереди. Любой из вариантов закрывает, все маленькие: **(a)** сузить до популяции, ради которой PR и делался: ```sql WHERE expires_at < NOW() AND created_by IS NULL ``` (129 строк сегодня, все legacy/безхозные — архив пилотов цел); **(b)** развести часы: удалять по `created_at < NOW() - make_interval(days => :purge_days)` с новой настройкой `trade_in_estimate_purge_days` (напр. 180), оставив `expires_at` как TTL ссылки; **(c)** если 24ч — это и правда срок хранения записи в т.ч. для пилотов, то это продуктовое решение, которое принимается явно и с переделкой `/history` заранее, а не обнаруживается по факту пустого архива. Независимо от выбора: перенести предупреждение в докстринг рядом с командой включения и добавить preflight-счётчик, чтобы оператор перед включением видел «будет удалено N строк, из них M с владельцем». --- ### 🟡 MEDIUM — удаление по телефону это точное сравнение строк → тихий no-op `app/services/data_erasure.py:126-131` матчит `phone = :phone` буквально. `lead.py:69-84` хранит телефон **как прислали** (только regex + счёт цифр, нормализации в E.164 нет намеренно, #2376). Оператор, вводящий `8 (999) 123-45-67` против сохранённого `+79991234567`, получает `{"trade_in_leads_deleted": 0}` и HTTP 200 — запрос на удаление отчитывается выполненным, данные остаются. Для legal-erasure ручки тихий промах это худший из режимов отказа. ```sql OR (:phone IS NOT NULL AND regexp_replace(phone, '\D', '', 'g') = regexp_replace(:phone, '\D', '', 'g')) ``` (4 строки на проде, вопрос производительности не стоит) --- ### 🟢 Мелочи 1. `231:56-64` — `ADD COLUMN … SET NOT NULL` без DEFAULT. Миграции применяются ДО перезапуска контейнеров, поэтому старый код, вставляющий лид в это ~минутное окно, словит NOT NULL violation. При 4 лидах в месяц вероятность пренебрежимая, но `ALTER COLUMN expires_at SET DEFAULT NOW() + interval '180 days'` закрывает бесплатно. 2. Ограничения из описания действительно живут в коде, а не только в PR: аноним-неидентифицируем (`data_erasure.py:11-29`), Telegram-зеркало (`31-40` + врезка на `152-155`), `user_events` не чистится (`42-51`). Это сделано хорошо. Не зафиксировано нигде другое: при открытии анонимного флоу адрес попадёт в `geocode_cache` через ручку автодополнения/геокодинга **раньше** чекбокса — гейт закрывает только путь `/estimate`. Строчка в блок `_ESTIMATE_CONSENT_*` пригодится следующему этапу. 3. Сроки параметризованы, не захардкожены: `trade_in_estimate_retention_hours=24`, `trade_in_lead_retention_days=180`, `trade_in_purge_batch_size=500` в `config.py:817-844`, все через ENV; прежние два хардкода `timedelta(hours=24)` в estimator.py убраны. Единственный литерал — `interval '180 days'` в backfill'е 231, и это правильно (миграция = снимок на момент применения) и там же объяснено. 4. `_DEFAULT_MAX_BATCHES = 20` живёт модульной константой, тогда как `batch_size` — в settings. Асимметрия косметическая: `default_params` несёт оба, переопределяется на уровне расписания. --- ### Что проверено и сходится - **Миграции.** 229 и 231 применяются на реальной прод-схеме и идемпотентны (dry-run в `BEGIN…ROLLBACK`, тело дважды: проход 1 — весь DDL, проход 2 — сплошь «already exists, skipping»). Seed садится как `enabled=f`, `next_run_at 2026-08-07 02:00+00`, `{"batch_size":500,"max_batches":20}`. `ON CONFLICT (source)` опирается на существующий `scrape_schedules_source_key UNIQUE (source)`. Backfill проставил 4 лидам Dec 2026 / Jan 2027, под удаление сейчас не попадает ни один. - **Фальсификация CHECK.** `INSERT … consent=false` → `violates check constraint "trade_in_estimates_consent_not_false"`. Работает. - **Номера свободны везде.** 229/231 нет на текущем main (там 228 и 230), нет в `_schema_migrations` на проде, и ни один открытый PR их не занимает (#2732 и #2742 оба взяли 232 — это их коллизия, не этого PR). - **B2B не сломан.** `require_consent = x_authenticated_user is None`; `/api/v1/trade-in/estimate` не входит в `_PUBLIC_PATHS` (`rbac.py:71-86`), а session-cookie путь инжектит `X-Authenticated-User` прямо в ASGI scope (`_propagate_authenticated_user`, `rbac.py:207`) — значит хендлер сегодня всегда видит username, и ветка гейта недостижима на проде. `consent: bool | None = None` в схеме оставляет payload'ы пилотов (которые поле не шлют) валидными. - **Согласие до первой записи адреса — по всем путям.** Гейт стоит первым оператором тела `estimate_quality` (`estimator.py:3504-3513`), до `geocode()` и до обоих INSERT (основной + `_empty_estimate`). `schedule_event(estimate_request, payload={address,…})` срабатывает уже ПОСЛЕ возврата из `estimate_quality` (`trade_in.py:207-220`) — на 422 адрес в `user_events` не попадает. `RequestAuditMiddleware` пишет только `status_code`, тело не логирует. `account_quota.check_and_raise` при username=None это no-op. - **Идемпотентность и частичный сбой purge.** Каждый батч со своим commit; исключение посреди прогона откатывает только незакоммиченный батч, `mark_failed` фиксирует накопленные счётчики, повторный запуск просто матчит меньше строк. Тесты это покрывают (`test_purge_expired_trade_in_data.py:167-198`). - **Каскады.** `estimate_photos` хранит `content bytea` в самой БД (`007_estimate_photos.sql:12`) — CASCADE уносит фото целиком, осиротевших файлов на диске не остаётся. - **Атрибуция удаления.** `RequestAuditMiddleware` пишет `admin_action` для мутирующих `/api/v1/admin/**` (`app/core/request_audit.py:91-100`) → кто и когда запросил стирание, в журнале есть (без тела). --- ### Пересечение с платёжным контуром (#2732) Коротко: **`pd_erased_at` это заглушка, а не механизм — но расширять #2547 сейчас НЕ надо.** Расширять на PR-D, до первого живого платежа. Почему не сейчас: `232_payments.sql` ещё не применён (на проде `_schema_migrations` заканчивается на 230), а 231 идёт раньше 232 — задача, ссылающаяся на `payments`, свалится на схеме, где таблицы ещё нет. Плюс `PAYMENTS_ENABLED`/`TBANK_*` на проде отсутствуют, защищать сегодня нечего. Что именно и когда — три пункта, все вместе с PR-D или до него, потому что после первого живого платежа они становятся ретроактивно невыполнимыми: 1. **`payment_notifications.body` — настоящая дыра.** `232_payments.sql:214`: `body jsonb NOT NULL -- полное тело нотификации как есть`, append-only, без TTL и без пути стирания. При подключённой кассе в теле приезжает `Receipt.Email` / `Receipt.Phone`. Самое дешёвое и надёжное — **не сохранять**: вырезать поддерево `Receipt` перед INSERT (скаляры, участвующие в подписи, остаются — подпись считается по top-level полям, ср. #2733). Позже редактировать append-only хранилище дорого, не класть туда — одна строка в PR-D. 2. **`payments.customer_email` / `customer_phone` в `erase_person_data`** — именно `UPDATE`, не `DELETE` (фискальный след по 54-ФЗ обязан пережить: `order_id` / `amount_kopecks` / `product_code` / `status` остаются): ```sql UPDATE payments SET customer_email = NULL, customer_phone = NULL, pd_erased_at = NOW() WHERE (estimate_id = ANY(CAST(:ids AS uuid[])) OR lead_id = ANY(CAST(:lead_ids AS uuid[])) OR created_by = :username) AND pd_erased_at IS NULL ``` Здесь ловушка порядка: id лидов надо захватить ДО того, как шаг 2 их удалит — ровно та же проблема, которую модуль уже решает для оценок (`data_erasure.py:103-107`). Сейчас функция собирает estimate_ids, но не lead_ids, поэтому правка в PR-D будет не чисто аддитивной. Фиксирую сейчас, чтобы не переоткрывать потом. 3. **TTL для `payment_notifications`** — это артефакт идемпотентности и отладки, не фискальный документ. Третий вызов `_drain_expired` в этой же задаче, когда 232 будет применён. Естественное место, новой машинерии не требует. Отдельно, в плюс #2732 и стоит сохранить: `payments_lead_idx` по `lead_id` там есть (`232_payments.sql:195`) с комментарием ровно про seq scan от FK `ON DELETE SET NULL` — а это именно то, что ночная чистка лидов из этого PR и провоцировала бы. Взаимодействие двух PR здесь учтено. --- ### Дальше 1. Одна из правок (a)/(b)/(c) по HIGH + предупреждение рядом с командой включения. 2. Опционально в этом же заходе — digits-only матч по телефону (MEDIUM). 3. Пуш → перечитаю новый SHA, дождусь явного success `CI Trade-In / backend-tests`, смержу. Не мержу до этого: 911 строк пилотов (включая user2/brusnika, доступ которому вернули три коммита назад) не восстанавливаются.

bot-backend referenced this pull request from a commit 2026-08-06 16:49:38 +00:00

fix(tradein/privacy): не удалять B2B-строки в purge + находить телефон в другом формате при erasure (#2547)

bot-backend added 1 commit 2026-08-06 16:49:38 +00:00

fix(tradein/privacy): не удалять B2B-строки в purge + находить телефон в другом формате при erasure (#2547) ... 881730bf20

Deep-review HIGH: purge_expired_trade_in_data удалял trade_in_estimates по
expires_at без разбора B2B/B2C -- эта колонка TTL ссылки/PDF, а не срок
хранения строки, и её единообразно проставляет каждой оценке estimator.py.
Прод-аудит: 1040/1057 строк просрочены, 911 из них у пилотов (admin,
kopylov, brusnika, praktika, pilottest, admintest, user1). DELETE теперь
ограничен created_by IS NULL -- ровно анонимная B2C-популяция (129 строк).
Докстринг миграции 231 переписан: явные цифры аудита, необратимость,
чек-лист (свежий SELECT count + один supervised прогон) перед enable.

Deep-review MEDIUM: erase_person_data сравнивал phone точным =, а lead.py
сохраняет номер как прислали (без нормализации, намеренно) -- разное
форматирование одного и того же номера не находилось, 0 строк удалялось,
но ответ всё равно был 200 "данные удалены". Сравнение переведено на
regexp_replace(x, '\D', '', 'g') с обеих сторон.

Оба фикса проверены живьём (throwaway Postgres 16 в docker, вне обычного
mock-only CI-лейна): без гварда пилотская строка удалялась вместе с
анонимной; без нормализации разноформатный телефон не находился. С
фиксами -- находит/не находит ровно как задумано. Добавлены self-skipping
live-DB тесты (паттерн test_house_dedup_merge.py::_live_session) плюс
статические SQL-guard тесты.

bot-backend referenced this pull request from a commit 2026-08-06 16:59:59 +00:00

fix(tradein/privacy): нормализация телефона к каноническому РФ-виду при erasure (#2547)

bot-backend added 1 commit 2026-08-06 16:59:59 +00:00

fix(tradein/privacy): нормализация телефона к каноническому РФ-виду при erasure (#2547) ... 4ee4d4b8e2

Follow-up к прошлому фиксу (regexp_replace \D): чистое удаление
форматирования не закрывало разрыв, который сам ревьюер привёл в примере --
"+7 999 123-45-67" и "89991234567" после digit-stripping дают РАЗНЫЕ строки
(79991234567 vs 89991234567, différent на первой цифре) -- классическая для
РФ путаница 8/+7 trunk-префикса.

_ru_phone_norm_sql(expr) добавляет второй шаг: если после digit-stripping
получилось РОВНО 11 цифр с ведущей '8' -- заменить её на '7'. Точное
тождество для российской нумерации, не эвристика (обсуждали: усечение до
"последних 10 цифр" риск-скориальнее -- склеивает номера разных стран,
удаление чужих данных хуже неудаления своих). Оба вызова
(_PHONE_COLUMN_NORM_SQL / _PHONE_PARAM_NORM_SQL) строят SQL-структуру из
статичных фрагментов (имя колонки / CAST(:phone AS text)) -- ни один
телефон не попадает в текст запроса напрямую.

Живая проверка (throwaway Postgres 16 в docker): лид "89991234567" находится
и удаляется по запросу "+7 999 123-45-67" -- ровно кейс из ревью. Встроенный
counterfactual в самом тесте доказывает, что чистый digit-strip (прошлая
версия фикса) для этой пары находит 0 строк. Negative control: номер,
отличающийся одной значащей цифрой, НЕ удаляется (защита от ложного
совпадения = удаления чужих данных).

lekss361 merged commit d87c9fa191 into main 2026-08-06 17:04:27 +00:00

lekss361 referenced this pull request from a commit 2026-08-06 17:04:28 +00:00

Merge pull request 'feat(mera/b2c): правовая рамка — согласие до сохранения, удаление по сроку и по запросу — этап 4 из 8' (#2547) from feat/mera-b2c-privacy into main

lekss361 referenced this pull request 2026-08-06 17:19:35 +00:00

chore(tradein/db): уборка временных таблиц, дублей индексов и звёздочки в v_data_quality #2746

Light1YT referenced this pull request from a commit 2026-08-06 18:14:37 +00:00

fix(tests): объявить 4 live-DB проверки приватности в skip_allowlist

bot-backend referenced this pull request 2026-08-06 18:14:56 +00:00

fix(tests): объявить 4 live-DB проверки приватности в skip_allowlist #2750

lekss361 referenced this pull request 2026-08-06 18:39:53 +00:00

tradein/db: миграция 229 завела дубль индекса по expires_at — точная копия индекса из 001 #2752

bot-backend referenced this pull request from a commit 2026-08-06 18:48:29 +00:00

feat(tradein/payments): оплаченный отчёт хранится год — retain_until и предохранители в задаче удаления

lekss361 referenced this pull request 2026-08-06 18:56:34 +00:00

feat(tradein/payments): оплаченный отчёт хранится год — retain_until и предохранители в задаче удаления #2754

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

Fable 5 ревью

Находки полного мульти-агентного ревью (Claude Fable 5, 2026-06-09)

  

GG-форсайт

Новое ТЗ от Ген Геныча (июнь 2026): Site Finder v2 — ИИ-прогноз продуктовой стратегии девелопера. Актуальный приоритет.

  

admin

Admin UI / internal tools / pipeline tracker

  

analytics

Аналитика данных / matching

  

auth

Authentication / authorization / multi-tenancy

  

automation

Автоматизация / cron / scheduler

  

bug

Something isn't working

  

business

Коммерческая воронка / пилоты / pipeline / реестр ПО

  

chore

Operational housekeeping

  

ci

Forgejo Actions / build pipeline

  

compliance

152-ФЗ / реестр ПО / legal / TOS

  

data

Data sources, parsers, ETL

  

data-moat

Уникальные данные — sales tracker, CRM aggregation

  

docs

README / runbooks / onboarding

  

duplicate

This issue or pull request already exists

  

dx

Developer experience / build speed

  

enhancement

New feature or request

  

feedback/max

Feedback from Max (Senior partner)

  

generative

Generative Design — concept generation engine

  

needs-discussion

Pending team sync / decision

  

needs-human

Human review/decision required

  

observability

Monitoring / metrics / logs / Sentry / Uptime

  

pause-bots

Kill-switch — all autonomous bots pause

  

performance

Latency / throughput / DB query tuning

  

priority/p0

Critical / production-blocker

  

priority/p1

High priority

  

priority/p2

Normal priority

  

priority/p3

Низкий приоритет (follow-up, tech-debt)

  

scope/backend

Backend Python — FastAPI, Celery, scrapers

  

scope/db

DB migrations, indexes, SQL

  

scope/devops

docker-compose, Caddy, workflows

  

scope/frontend

Frontend — Next.js, React, TypeScript

  

scope/qa

Tests, smoke, Playwright

  

scrapers

Скрапперы (Avito/Cian/Yandex/Rosreestr)

  

security

Security audit / secrets / vulnerabilities

  

site-finder

Site Finder scoring quality & data layer

  

stage/1

Stage 1: Generative MVP (нед 2-7)

  

stage/2

Stage 2: Site Finder MVP (нед 8-12)

  

status/blocked

Blocked — needs human intervention

  

status/done

Closed, completed

  

status/needs-analysis

Raw/fuzzy task filed by human — auto-analyst подхватывает: code-archeology + декомпозиция в actionable scope/* + status/ready (или split на под-issues)

  

status/needs-fix

Reviewer/QA вернул worker на доработку — worker сам чинит свой PR (fixup), НЕ требует human (в отличие от status/blocked)

  

status/qa

PR merged, awaiting smoke test

  

status/ready

Ready for pickup by worker bot

  

status/review

PR open, awaiting code review

  

status/wip

Work in progress (worker claimed)

  

tech-debt

Технический долг

  

tradein

Trade-In MVP проект

  

ux

User experience / responsive / conversion

  

week ревью 1

Week review 1 — находки multi-agent аудита проекта (2026-06-14)

  

wontfix

This will not be worked on

  

ИРД

Исходно-разрешительная документация — слой ограничений участка для analyze-таблицы

  

вторичка

Trade-In Estimator — проект оценки вторичного жилья

No labels

Fable 5 ревью

GG-форсайт

admin

analytics

auth

automation

bug

business

chore

ci

compliance

data

data-moat

docs

duplicate

dx

enhancement

feedback/max

generative

needs-discussion

needs-human

observability

pause-bots

performance

priority/p0

priority/p1

priority/p2

priority/p3

scope/backend

scope/db

scope/devops

scope/frontend

scope/qa

scrapers

security

site-finder

stage/1

stage/2

status/blocked

status/done

status/needs-analysis

status/needs-fix

status/qa

status/ready

status/review

status/wip

tech-debt

tradein

ux

week ревью 1

wontfix

ИРД

вторичка

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lekss361/gendesign#2547

No description provided.