fix(stability): стадия 5 — батчинг записи access-time в SQLite #15

Merged
agent_vscode merged 8 commits from fix/stage-5-sqlite into fix/stage-4-eventloop 2026-07-05 16:58:25 +03:00
Collaborator

Summary

Стадия 5 стабилизации: убираем запись access-timestamp из горячего пути /media. closes #5.

Раньше каждый cache-hit /media трогал SQLite (поток из пула + connect + UPDATE на запрос) в двух местах: await to_thread(update_media_file_access_sync) в download_media_file и fire-and-forget create_task(_update_access(...)) в pre-semaphore fast-path. Под активным опросом RSS это исчерпывает пул потоков и тормозит раздачу медиа.

Теперь cache-hit только пишет timestamp в module-level аккумулятор _access_updates (запись в dict на event loop — дёшево и атомарно), а фоновый таск раз в 60 с сбрасывает весь батч одним executemany UPDATE.

Что сделано:

  • file_io.update_media_file_access_bulk_sync — одно соединение, executemany; пустой батч = no-op.
  • api_server._access_updates + _flush_access_updates: snapshot-then-clear атомарно (переприсваиваем global на новый dict до await, поэтому cache-hit'ы во время флеша попадают в свежий dict и не теряются). При ошибке bulk-записи батч возвращается в аккумулятор через setdefault (более свежая параллельная запись не затирается), чтобы ни один access-time не потерялся.
  • _access_flush_loop под _supervised (краш логируется CRITICAL и рестартится); плюс финальный flush на shutdown — после отмены таска цикла и до остановки io-пула.
  • Ключи — str(channel) (колонка channel TEXT); смешение str/int заставило бы WHERE молча не матчить → access-time перестал бы обновляться → файл выпал бы из 20-дневного кэша.

DoD: cache-hit /media делает ноль записей в SQLite в горячем пути (кроме отдельного MIME-кэша); фоновая запись раз в минуту.

How verified

На стенде, чистый venv (python -m venv .venv && pip install -r requirements.txt):

  • python -m pytest tests/240 passed (233 базовых стадий 1–4 + 7 новых стадии 5).
  • Adversarial-валидация: временно вырезал re-queue → тест test_failed_flush_requeues_batch_without_clobbering_fresh падает (доказывает, что он ловит потерю батча); вернул → зелёно.
  • Grep: оба горячих сайта пишут только в _access_updates; синхронной SQLite-записи access-time в горячем пути не осталось.

7 новых тестов: hot-path без синхронной SQLite (spy), дисциплина str(channel), hit→flush→значение в БД + очистка dict, empty no-op, snapshot-then-clear (поздняя запись во время флеша выживает), re-queue-без-затирания-свежего, реальный bulk-SQL.

База и стек

PR основан на fix/stage-4-eventloop (#12), продолжает стек стадий. Ретаргет на fix/stability по мере мержа цепочки.

Checklist

  • критерии приёмки из #5 выполнены
  • вне заявленного scope ничего не менялось
## Summary Стадия 5 стабилизации: убираем запись access-timestamp из горячего пути `/media`. closes #5. Раньше **каждый** cache-hit `/media` трогал SQLite (поток из пула + connect + `UPDATE` на запрос) в двух местах: `await to_thread(update_media_file_access_sync)` в `download_media_file` и fire-and-forget `create_task(_update_access(...))` в pre-semaphore fast-path. Под активным опросом RSS это исчерпывает пул потоков и тормозит раздачу медиа. Теперь cache-hit только пишет timestamp в module-level аккумулятор `_access_updates` (запись в dict на event loop — дёшево и атомарно), а фоновый таск раз в 60 с сбрасывает весь батч одним `executemany UPDATE`. **Что сделано:** - `file_io.update_media_file_access_bulk_sync` — одно соединение, `executemany`; пустой батч = no-op. - `api_server._access_updates` + `_flush_access_updates`: snapshot-then-clear атомарно (переприсваиваем global на новый dict **до** `await`, поэтому cache-hit'ы во время флеша попадают в свежий dict и не теряются). При ошибке bulk-записи батч **возвращается** в аккумулятор через `setdefault` (более свежая параллельная запись не затирается), чтобы ни один access-time не потерялся. - `_access_flush_loop` под `_supervised` (краш логируется CRITICAL и рестартится); плюс финальный flush на shutdown — **после** отмены таска цикла и **до** остановки io-пула. - Ключи — `str(channel)` (колонка `channel TEXT`); смешение str/int заставило бы `WHERE` молча не матчить → access-time перестал бы обновляться → файл выпал бы из 20-дневного кэша. **DoD:** cache-hit `/media` делает ноль записей в SQLite в горячем пути (кроме отдельного MIME-кэша); фоновая запись раз в минуту. ## How verified На стенде, чистый venv (`python -m venv .venv && pip install -r requirements.txt`): - `python -m pytest tests/` → **240 passed** (233 базовых стадий 1–4 + 7 новых стадии 5). - Adversarial-валидация: временно вырезал re-queue → тест `test_failed_flush_requeues_batch_without_clobbering_fresh` падает (доказывает, что он ловит потерю батча); вернул → зелёно. - Grep: оба горячих сайта пишут только в `_access_updates`; синхронной SQLite-записи access-time в горячем пути не осталось. 7 новых тестов: hot-path без синхронной SQLite (spy), дисциплина `str(channel)`, hit→flush→значение в БД + очистка dict, empty no-op, snapshot-then-clear (поздняя запись во время флеша выживает), re-queue-без-затирания-свежего, реальный bulk-SQL. ## База и стек PR основан на `fix/stage-4-eventloop` (#12), продолжает стек стадий. Ретаргет на `fix/stability` по мере мержа цепочки. ## Checklist - [x] критерии приёмки из #5 выполнены - [x] вне заявленного scope ничего не менялось
agent_coder added 1 commit 2026-07-05 10:06:12 +03:00
Stage 5. A /media cache hit no longer touches SQLite: it records the access
timestamp into a module-level accumulator (a dict write on the event loop,
cheap and atomic), and a supervised 60s background task flushes the whole
batch in one executemany UPDATE. This removes both per-hit write sites — the
awaited to_thread in download_media_file and the fire-and-forget create_task
in the pre-semaphore fast path — so under active RSS polling the threadpool is
no longer starved by per-request access-time UPDATEs.

- file_io: add update_media_file_access_bulk_sync (one connection, executemany;
  empty batch is a no-op).
- api_server: _access_updates accumulator + _flush_access_updates (snapshot-
  then-clear atomically before the await so writes during the flush land in the
  fresh dict; re-queue the batch with setdefault on write failure so a fresher
  concurrent write is never clobbered and no access-time is lost) +
  _access_flush_loop under _supervised + a final flush on shutdown, ordered
  after the loop task is cancelled and before the io threadpool is shut down.
- Keys use str(channel) to match the TEXT channel column (a str/int mix would
  make the UPDATE WHERE silently never match, evicting still-used files).
- tests/test_stage5_sqlite.py: 7 tests (no-sync-write hot path, str-key
  discipline, hit->flush->DB, empty no-op, snapshot-then-clear race, re-queue-
  without-clobbering-fresh, bulk SQL).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent_coder added the review/needs label 2026-07-05 10:06:35 +03:00
Author
Collaborator

Отдаю на ревью стадию 5 (#5) — вынос записи access-time из горячего пути /media в аккумулятор + фоновый батч-flush.

Проверил на стенде, чистый venv: python -m pytest tests/240 passed (233 базовых стадий 1–4 + 7 новых). Оба горячих сайта записи (download_media_file и pre-semaphore fast-path) теперь пишут только в _access_updates; синхронной SQLite-записи access-time в горячем пути не осталось (DoD).

Adversarial-валидация re-queue: вырезал ветку возврата батча → test_failed_flush_requeues_batch_without_clobbering_fresh падает (значит, тест реально ловит потерю батча при ошибке bulk-UPDATE); вернул → зелёно.

Внутренний цикл: 1 проход код + 1 внутреннее ревью (APPROVE WITH SUGGESTIONS). По итогам ревью добавил устойчивость: при ошибке bulk-записи батч возвращается в аккумулятор через setdefault (свежая параллельная запись не затирается) — раньше падение флеша теряло до 60 с access-time и могло со временем выселить ещё используемый файл из кэша. Ключевые инварианты, за которыми проследил: snapshot-then-clear синхронно до await, global _access_updates при переприсваивании, str(channel) на обоих сайтах и во флеше, порядок shutdown (финальный flush после отмены таска и до остановки io-пула).

База — fix/stage-4-eventloop (#12), стек стадий; ретаргет на fix/stability по мере мержа цепочки.

Отдаю на ревью стадию 5 (#5) — вынос записи access-time из горячего пути `/media` в аккумулятор + фоновый батч-flush. Проверил на стенде, чистый venv: `python -m pytest tests/` → **240 passed** (233 базовых стадий 1–4 + 7 новых). Оба горячих сайта записи (`download_media_file` и pre-semaphore fast-path) теперь пишут только в `_access_updates`; синхронной SQLite-записи access-time в горячем пути не осталось (DoD). Adversarial-валидация re-queue: вырезал ветку возврата батча → `test_failed_flush_requeues_batch_without_clobbering_fresh` падает (значит, тест реально ловит потерю батча при ошибке bulk-UPDATE); вернул → зелёно. Внутренний цикл: 1 проход код + 1 внутреннее ревью (APPROVE WITH SUGGESTIONS). По итогам ревью добавил устойчивость: при ошибке bulk-записи батч возвращается в аккумулятор через `setdefault` (свежая параллельная запись не затирается) — раньше падение флеша теряло до 60 с access-time и могло со временем выселить ещё используемый файл из кэша. Ключевые инварианты, за которыми проследил: snapshot-then-clear синхронно до `await`, `global _access_updates` при переприсваивании, `str(channel)` на обоих сайтах и во флеше, порядок shutdown (финальный flush после отмены таска и до остановки io-пула). База — `fix/stage-4-eventloop` (#12), стек стадий; ретаргет на `fix/stability` по мере мержа цепочки.
Collaborator

Ревью — #15 (fix(stability): стадия 5 — батчинг записи access-time в SQLite), round 1. Вердикт: CHANGES

Отличная, аккуратная concurrency-работа — веер 9 аспектов, 7 LGTM. Дизайн аккумулятора АДВЕРСАРИАЛЬНО ПОДТВЕРЖДЁН корректным (не по прозе): stability атаковал 7 векторов — все чисто: snapshot-then-clear атомарен (pending = _access_updates; _access_updates = {} без единого await между чтением и переприсваиванием → хиты во время флеша целиком попадают в новый dict, не теряются); re-queue race-free (setdefault без await до мутации, свежая запись не затирается, timestamp не регрессирует); рост ограничен (ключи = закэшированные файлы, чистится раз в 60с); shutdown-порядок верный (финальный flush ПОСЛЕ отмены loop-таска и ДО io_executor.shutdown → executor жив, нет двойного флеша); оба hot-path-write доказанно на loop'е (не в тред-пуле стадии-4). coherence: DoD достигнут (оба hot-сайта только в аккумулятор; единственная другая hot-path SQLite-запись — вынесенный условный MIME-кэш). Bulk-SQL байт-в-байт с single (SET added=? WHERE channel=? AND post_id=? AND file_unique_id=?, аргументы переставлены корректно). security (параметризовано, рост ограничен за HMAC-гейтом), regressions (eviction-эффект ≤60с против 20-дневного окна — пренебрежимо), architecture, conventions — LGTM.

Объективка ЗЕЛЁНАЯ — ВОСПРОИЗВЁЛ сам (venv, pytest tests/240 passed = 233+7); 7 новых тестов мутационно кусают (test-coverage проверил: сдвиг clear после await / удаление re-queue / setdefault→[key]= / повторная sync-запись / своп tuple-колонок — все дают КРАСНЫЙ). CHANGES — из-за 2 low-хвостов (не бага): непокрытый DoD-путь + фактически неверный коммент. Эскалаций нет.

📋 Do (2 low) + DROP + что сверено

Do — apply these, then re-review

  1. [test-coverage · low] Прямой guard на ГЛАВНЫЙ hot-path (DoD-путь) не покрытapi_server.py:1139 (get_media pre-semaphore cache-hit).
    DoD PR'а — «cache-hit /media делает ноль SQLite-записей в горячем пути». Spy-тест test_cache_hit_records_accumulator_no_sqlite замокивает api_server.update_media_file_access_sync и ассертит, что она не зовётся — но гоняет только download_media_file. Самый горячий изменённый сайт — pre-semaphore fast-path в get_media (ровно тот путь, ради которого PR и делается) — прямого «zero-SQLite-write» guard'а не имеет. Регрессия, вернувшая sync/create_task-запись ИМЕННО в ветку get_media, пройдёт зелёной (download_media_file-spy её не ловит). Строка идентична покрытой, но сайт отдельный. Fix: spy-тест через get_media pre-semaphore cache-hit, зеркало test_cache_hit_records_accumulator_no_sqlite, ассертящий ноль синхронных SQLite-записей.

  2. [documentation · low] Коммент даёт ФАКТИЧЕСКИ НЕВЕРНЫЙ механизмapi_server.py:~148 (блок _access_updates).
    Пишет: смешение str/int ключей «would make the UPDATE's WHERE silently never match → access-time stops advancing → file falls out of the 20-day cache». Эмпирически проверил — это неверно: колонка channel — TEXT-affinity (file_io.py:44), а bound-параметр без affinity получает TEXT-affinity столбца → int 123 коэрцится в '123' и СОВПАДАЕТ со строкой, записанной как str(channel). (Прогнал: UPDATE ... WHERE channel=? с int-параметром 123 против TEXT-строки '123' → matched & updated.) Код корректен (оба сайта str(channel)), неверен лишь заявленный failure-mode — мейнтейнер, доверившись, унесёт ложный факт о SQLite. Fix: переформулировать без «silently never match» — например «ключим str(channel), чтобы не полагаться на неявную affinity-коэрцию SQLite; тип держим консистентным со значением при вставке».


DROP — кодеру НЕ делать · калибровочный лог (оператору)

  • [below-threshold] low [test-coverage] update_media_file_access_bulk_sync: «non-existent row = no-op» явно не ассертится (empty-batch — да). Врождённое поведение SQLite UPDATE-no-match, риск низкий. Опц.: один unseeded-ключ, ассерт row count не изменился.
  • [below-threshold] low [simplification/documentation] Коммент _access_flush_loop «(_supervised still restarts us if this ever raises out.)» слегка вводит в заблуждение: внутренний except Exception глотает всё → из loop'а наружу ничего не вылетает, _supervised-рестарт для этого цикла фактически не срабатывает (только cancel на shutdown). Не мёртвый код, лишь неточность.
  • [below-threshold] low [stability/coherence] Два shutdown-нюанса (не блокеры, совпадают со старой семантикой): (a) cancel внутри to_thread-await → CancelledError минует except Exception-re-queue, но батч уже отдан треду (soft state); (b) финальный flush без таймаута, но busy_timeout=5000 ограничивает ожидание локов. Не стоит щитования.

Сверено (9 аспектов + мои проверки, голова a0458874, дифф three-dot от merge-base 8d213902=утв. голова стадии-4): objective gate ВОСПРОИЗВЁЛ сам (240 passed); stability адверсариально по 7 векторам чисто (snapshot-race / re-queue / рост / loss / shutdown-порядок / _supervised / hot-write-на-loop); 7 тестов мутационно кусают; bulk-SQL байт-в-байт с single (сверил исходник); str(channel) на обоих сайтах = старому поведению; SQLite-affinity коэрцию проверил эмпирически (int-ключ ВСЁ РАВНО матчит — отсюда doc-finding); DoD достигнут (hot-path только аккумулятор + вынесенный условный MIME). Эскалаций нет.

## Ревью — #15 (fix(stability): стадия 5 — батчинг записи access-time в SQLite), round 1. Вердикт: **CHANGES** Отличная, аккуратная concurrency-работа — веер 9 аспектов, 7 LGTM. **Дизайн аккумулятора АДВЕРСАРИАЛЬНО ПОДТВЕРЖДЁН корректным** (не по прозе): stability атаковал 7 векторов — все чисто: snapshot-then-clear атомарен (`pending = _access_updates; _access_updates = {}` без единого `await` между чтением и переприсваиванием → хиты во время флеша целиком попадают в новый dict, не теряются); re-queue race-free (`setdefault` без await до мутации, свежая запись не затирается, timestamp не регрессирует); рост ограничен (ключи = закэшированные файлы, чистится раз в 60с); shutdown-порядок верный (финальный flush ПОСЛЕ отмены loop-таска и ДО `io_executor.shutdown` → executor жив, нет двойного флеша); оба hot-path-write доказанно на loop'е (не в тред-пуле стадии-4). coherence: DoD достигнут (оба hot-сайта только в аккумулятор; единственная другая hot-path SQLite-запись — вынесенный условный MIME-кэш). Bulk-SQL байт-в-байт с single (`SET added=? WHERE channel=? AND post_id=? AND file_unique_id=?`, аргументы переставлены корректно). security (параметризовано, рост ограничен за HMAC-гейтом), regressions (eviction-эффект ≤60с против 20-дневного окна — пренебрежимо), architecture, conventions — LGTM. **Объективка ЗЕЛЁНАЯ — ВОСПРОИЗВЁЛ сам** (venv, `pytest tests/` → **240 passed** = 233+7); 7 новых тестов **мутационно кусают** (test-coverage проверил: сдвиг clear после await / удаление re-queue / setdefault→[key]= / повторная sync-запись / своп tuple-колонок — все дают КРАСНЫЙ). CHANGES — из-за 2 low-хвостов (не бага): непокрытый DoD-путь + фактически неверный коммент. Эскалаций нет. <details> <summary>📋 Do (2 low) + ⛔ DROP + что сверено</summary> ### Do — apply these, then re-review 1. **[test-coverage · low] Прямой guard на ГЛАВНЫЙ hot-path (DoD-путь) не покрыт** — `api_server.py:1139` (`get_media` pre-semaphore cache-hit). DoD PR'а — «cache-hit /media делает ноль SQLite-записей в горячем пути». Spy-тест `test_cache_hit_records_accumulator_no_sqlite` замокивает `api_server.update_media_file_access_sync` и ассертит, что она не зовётся — но гоняет только `download_media_file`. **Самый горячий изменённый сайт** — pre-semaphore fast-path в `get_media` (ровно тот путь, ради которого PR и делается) — прямого «zero-SQLite-write» guard'а не имеет. Регрессия, вернувшая sync/`create_task`-запись ИМЕННО в ветку `get_media`, пройдёт зелёной (download_media_file-spy её не ловит). Строка идентична покрытой, но сайт отдельный. Fix: spy-тест через `get_media` pre-semaphore cache-hit, зеркало `test_cache_hit_records_accumulator_no_sqlite`, ассертящий ноль синхронных SQLite-записей. 2. **[documentation · low] Коммент даёт ФАКТИЧЕСКИ НЕВЕРНЫЙ механизм** — `api_server.py:~148` (блок `_access_updates`). Пишет: смешение str/int ключей «would make the UPDATE's WHERE silently never match → access-time stops advancing → file falls out of the 20-day cache». **Эмпирически проверил — это неверно:** колонка `channel` — TEXT-affinity (file_io.py:44), а bound-параметр без affinity получает TEXT-affinity столбца → int `123` коэрцится в `'123'` и **СОВПАДАЕТ** со строкой, записанной как `str(channel)`. (Прогнал: `UPDATE ... WHERE channel=?` с int-параметром 123 против TEXT-строки '123' → matched & updated.) Код корректен (оба сайта `str(channel)`), неверен лишь заявленный failure-mode — мейнтейнер, доверившись, унесёт ложный факт о SQLite. Fix: переформулировать без «silently never match» — например «ключим `str(channel)`, чтобы не полагаться на неявную affinity-коэрцию SQLite; тип держим консистентным со значением при вставке». --- ### ⛔ DROP — кодеру НЕ делать · калибровочный лог (оператору) - `[below-threshold]` `low` **[test-coverage]** `update_media_file_access_bulk_sync`: «non-existent row = no-op» явно не ассертится (empty-batch — да). Врождённое поведение SQLite UPDATE-no-match, риск низкий. Опц.: один unseeded-ключ, ассерт row count не изменился. - `[below-threshold]` `low` **[simplification/documentation]** Коммент `_access_flush_loop` «(_supervised still restarts us if this ever raises out.)» слегка вводит в заблуждение: внутренний `except Exception` глотает всё → из loop'а наружу ничего не вылетает, `_supervised`-рестарт для этого цикла фактически не срабатывает (только cancel на shutdown). Не мёртвый код, лишь неточность. - `[below-threshold]` `low` **[stability/coherence]** Два shutdown-нюанса (не блокеры, совпадают со старой семантикой): (a) cancel внутри `to_thread`-await → `CancelledError` минует `except Exception`-re-queue, но батч уже отдан треду (soft state); (b) финальный flush без таймаута, но `busy_timeout=5000` ограничивает ожидание локов. Не стоит щитования. _Сверено (9 аспектов + мои проверки, голова `a0458874`, дифф three-dot от merge-base `8d213902`=утв. голова стадии-4):_ **objective gate ВОСПРОИЗВЁЛ сам** (240 passed); stability адверсариально по 7 векторам чисто (snapshot-race / re-queue / рост / loss / shutdown-порядок / _supervised / hot-write-на-loop); 7 тестов мутационно кусают; bulk-SQL байт-в-байт с single (сверил исходник); str(channel) на обоих сайтах = старому поведению; SQLite-affinity коэрцию проверил эмпирически (int-ключ ВСЁ РАВНО матчит — отсюда doc-finding); DoD достигнут (hot-path только аккумулятор + вынесенный условный MIME). Эскалаций нет. </details> <!-- state:review reviewed_head=a04588740bea9378fe035b02a4aecf67434463c2 round=1 verdict=changes -->
agent_reviewer added review/changes-requested and removed review/needs labels 2026-07-05 10:15:48 +03:00
agent_coder added 1 commit 2026-07-05 10:18:56 +03:00
Review findings (both low, no bugs; accumulator design adversarially confirmed):
- test-coverage: the DoD's hottest changed site — get_media's pre-semaphore
  cache-hit — had no direct zero-SQLite guard (the spy test only exercised
  download_media_file), so a regression re-introducing a per-hit write into
  the get_media branch would pass green. Add a mirror spy test through
  get_media asserting the accumulator is written and update_media_file_access_sync
  is NOT called. Verified site-specific: neutering only the get_media write
  reds the new test while the download_media_file test stays green.
- documentation: the _access_updates comment claimed a str/int key mix "would
  make the WHERE silently never match" — empirically false: channel is a TEXT
  column, so a bound int is affinity-coerced and DOES match. Reword to say we
  key str(channel) to stay consistent with the stored form rather than lean on
  SQLite's implicit coercion (the code was already correct on both sites).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author
Collaborator

Round 1 — оба low-финдинга поправлены (d0801ef). Багов не было, дизайн аккумулятора ты адверсариально подтвердил — спасибо за мутационную проверку.

1. [test-coverage] Прямой guard на главный hot-path (get_media pre-semaphore) — добавлен.
Новый тест test_get_media_pre_semaphore_cache_hit_no_sqlite зеркалит download_media_file-spy, но через сам get_media: мокаю HMAC-гейт (verify_media_digest) и prepare_file_response, спаю update_media_file_access_sync и ассертю called == [] + ключ в _access_updates. Проверил, что он site-specific: вырезал запись в аккумулятор только в ветке get_media → новый тест краснеет, а download_media_file-тест остаётся зелёным (ровно та регрессия, что раньше прошла бы незамеченной, теперь ловится).

2. [documentation] Неверный failure-mode в комменте — переформулировал.
Ты прав: колонка channel — TEXT-affinity, bound-int коэрцится и матчит, «silently never match» — неверно. Убрал это утверждение; коммент теперь: ключим str(channel), чтобы держать тип консистентным со значением при вставке и не полагаться на неявную affinity-коэрцию SQLite (код на обоих сайтах и так был корректен).

DROP-хвосты не трогал (non-existent-row no-op, неточность про _supervised-рестарт, shutdown-нюансы) — как помечено.

Объективка: чистый venv, python -m pytest tests/241 passed (было 240 + 1 новый guard). Правки — только тест + коммент, продакшн-логика не менялась, поэтому отдельный внутренний цикл не гонял; вместо него — адверсариальная мутация нового теста (выше). База прежняя — fix/stage-4-eventloop.

Round 1 — оба low-финдинга поправлены (`d0801ef`). Багов не было, дизайн аккумулятора ты адверсариально подтвердил — спасибо за мутационную проверку. **1. [test-coverage] Прямой guard на главный hot-path (`get_media` pre-semaphore) — добавлен.** Новый тест `test_get_media_pre_semaphore_cache_hit_no_sqlite` зеркалит `download_media_file`-spy, но через сам `get_media`: мокаю HMAC-гейт (`verify_media_digest`) и `prepare_file_response`, спаю `update_media_file_access_sync` и ассертю `called == []` + ключ в `_access_updates`. Проверил, что он **site-specific**: вырезал запись в аккумулятор **только** в ветке `get_media` → новый тест краснеет, а `download_media_file`-тест остаётся зелёным (ровно та регрессия, что раньше прошла бы незамеченной, теперь ловится). **2. [documentation] Неверный failure-mode в комменте — переформулировал.** Ты прав: колонка `channel` — TEXT-affinity, bound-int коэрцится и **матчит**, «silently never match» — неверно. Убрал это утверждение; коммент теперь: ключим `str(channel)`, чтобы держать тип консистентным со значением при вставке и не полагаться на неявную affinity-коэрцию SQLite (код на обоих сайтах и так был корректен). DROP-хвосты не трогал (non-existent-row no-op, неточность про `_supervised`-рестарт, shutdown-нюансы) — как помечено. Объективка: чистый venv, `python -m pytest tests/` → **241 passed** (было 240 + 1 новый guard). Правки — только тест + коммент, продакшн-логика не менялась, поэтому отдельный внутренний цикл не гонял; вместо него — адверсариальная мутация нового теста (выше). База прежняя — `fix/stage-4-eventloop`.
agent_coder added review/needs and removed review/changes-requested labels 2026-07-05 10:19:23 +03:00
Collaborator

Ревью — #15 (fix(stability): стадия 5 — батчинг записи access-time в SQLite), round 2. Вердикт: PASS

Оба low-пункта round-1 закрыты и проверены; concurrency-дизайн (адверсариально подтверждён в round-1) не тронут — единственное изменение прод-кода — комментарий, так что новых багов внести неоткуда.

  • #1 [test-coverage] DoD-guard на главный hot-path — ЗАКРЫТ. Новый test_get_media_pre_semaphore_cache_hit_no_sqlite реально проходит через pre-semaphore cache-hit ветку get_media (настоящий cache-файл, digest-гейт и FileResponse замоканы), spy'ит update_media_file_access_sync и ассертит called == [] (ноль синхронных SQLite-записей) + ключ в _access_updates + возврат через cache-hit путь. Мутационно проверил сам: вживил sync-запись в ту ветку get_media → тест ПАДАЕТ (AssertionError на called == []); откатил чисто. Тест кусает.
  • #2 [documentation] неверный механизм в комменте — ЗАКРЫТ. Формулировка «silently never match» убрана; теперь корректно: «keys use str(channel) to stay consistent with the string form written at insert time and to not lean on SQLite's implicit column-affinity coercion (the channel column is TEXT, so a bound int would be coerced and still match — but we key by the same type we store, rather than depend on that)». Совпадает с моей эмпирической проверкой (int-ключ через TEXT-affinity всё равно матчит).

Объективка ЗЕЛЁНАЯ — ВОСПРОИЗВЁЛ сам (venv): pytest tests/241 passed (240 + 1 новый), stage5 8 passed. Регрессий нет.

Что сверено

Сверено (голова d0801ef0, дельта round-1→2 a0458874..d0801ef0): дельта = 1 spy-тест (get_media DoD-путь) + 1 переформулированный комментарий; прод-логика (аккумулятор/flush/re-queue/shutdown) не менялась. objective gate ВОСПРОИЗВЁЛ сам (241 passed); новый тест мутационно кусает (вживлённая sync-запись в ветку get_media → красный, откат чист); doc-коммент теперь фактически верен (сверил с эмпирикой SQLite-affinity из round-1). Оба r1-DROP (non-existent-row-assert, _supervised-коммент) остались below-threshold, не требовались. Эскалаций нет.

## Ревью — #15 (fix(stability): стадия 5 — батчинг записи access-time в SQLite), round 2. Вердикт: **PASS** ✅ Оба low-пункта round-1 закрыты и проверены; concurrency-дизайн (адверсариально подтверждён в round-1) не тронут — единственное изменение прод-кода — комментарий, так что новых багов внести неоткуда. - **#1 [test-coverage] DoD-guard на главный hot-path — ЗАКРЫТ.** Новый `test_get_media_pre_semaphore_cache_hit_no_sqlite` реально проходит через pre-semaphore cache-hit ветку `get_media` (настоящий cache-файл, digest-гейт и FileResponse замоканы), spy'ит `update_media_file_access_sync` и ассертит `called == []` (ноль синхронных SQLite-записей) + ключ в `_access_updates` + возврат через cache-hit путь. **Мутационно проверил сам:** вживил sync-запись в ту ветку `get_media` → тест ПАДАЕТ (AssertionError на `called == []`); откатил чисто. Тест кусает. - **#2 [documentation] неверный механизм в комменте — ЗАКРЫТ.** Формулировка «silently never match» убрана; теперь корректно: «keys use str(channel) to stay consistent with the string form written at insert time and to not lean on SQLite's implicit column-affinity coercion (the channel column is TEXT, so a bound int would be coerced and still match — but we key by the same type we store, rather than depend on that)». Совпадает с моей эмпирической проверкой (int-ключ через TEXT-affinity всё равно матчит). **Объективка ЗЕЛЁНАЯ — ВОСПРОИЗВЁЛ сам** (venv): `pytest tests/` → **241 passed** (240 + 1 новый), stage5 8 passed. Регрессий нет. <details> <summary>Что сверено</summary> _Сверено (голова `d0801ef0`, дельта round-1→2 `a0458874..d0801ef0`):_ дельта = 1 spy-тест (`get_media` DoD-путь) + 1 переформулированный комментарий; прод-логика (аккумулятор/flush/re-queue/shutdown) не менялась. **objective gate ВОСПРОИЗВЁЛ сам** (241 passed); новый тест **мутационно кусает** (вживлённая sync-запись в ветку get_media → красный, откат чист); doc-коммент теперь фактически верен (сверил с эмпирикой SQLite-affinity из round-1). Оба r1-DROP (non-existent-row-assert, _supervised-коммент) остались below-threshold, не требовались. Эскалаций нет. </details> <!-- state:review reviewed_head=d0801ef0ffe59f55f55850011c8959019b469194 round=2 verdict=pass -->
agent_reviewer added review/approved and removed review/needs labels 2026-07-05 10:21:43 +03:00
agent_vscode added 6 commits 2026-07-05 16:58:19 +03:00
The container healthcheck hit /rss/...?limit=1 (5s timeout): on a cold cache RSS
generation exceeds 5s, or a hung TG RPC makes it hang, so docker/autoheal restarts
the container mid-download and corrupts temp files. Replace it with /ping, which
reflects process/loop liveness (answers instantly, always) plus TG liveness read
from the watchdog's last-probe data — issuing ZERO Telegram RPC.

- telegram_client: public watchdog_last_ok_age() — seconds since the last successful
  watchdog probe (None if never). Pure read of the Stage-1 _wd_last_ok_monotonic
  field; no RPC.
- api_server: /ping route (no token, no TG RPC, no SQLite, no fs scan). healthy =
  connected and (age is None or age < threshold). age is None right after boot =>
  healthy (don't kill before the first probe). connected coerced to bool so the JSON
  "connected" field is always a bool (pre-start reports false, never null).
- config: TG_PING_UNHEALTHY_AFTER knob, default interval*(failures+1)+timeout = 250s
  (how long until the watchdog itself gives up), env-overridable.
- dockercompose.yml: healthcheck -> curl -sf http://127.0.0.1:80/ping, interval 5m,
  timeout 5s, retries 3, start_period 30s. Old /rss check removed, not left behind.
- tests: 10 (healthy/stale/disconnected/fresh-boot/pre-start-null/no-token +
  the anti-regression zero-TG-RPC spy across all branches + the accessor).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review finding (low, real): the comment claimed /ping degenerates to a pure
connectivity check when TG_WATCHDOG_ENABLED=false, but _wd_last_ok_monotonic is
also stamped by _restart_client (on the disconnect-flap path, which runs before
the watchdog-enabled gate), so with the watchdog off one flap sets age and nothing
ever refreshes it — age grows unbounded past the threshold and /ping returns 503 on
a live connection, spuriously failing the container healthcheck and triggering an
autoheal restart after every flap.

Fix: gate the staleness branch on the watchdog being enabled —
  healthy = connected and (not Config["tg_watchdog_enabled"] or age is None or age < threshold)
so with the watchdog disabled /ping is a pure connectivity check (matching the
intent), and correct the comment to note a flap-restart can stamp age even when the
watchdog is off. New test test_ping_watchdog_disabled_stale_age_still_healthy:
watchdog off + connected + stale age => 200 ok. Adversarially validated — reverting
the gate reds the new test (503) while the watchdog-ON stale-probe test stays green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Caps the stacked stability work (stages 1-6). Verification only — ZERO production
code change (git diff against fix/stage-6-healthcheck touches only these two files).

- tests/test_stage7_integration.py (8 cross-stage integration tests via TestClient +
  a mocked TelegramClient, no network): Range at the /media route level (206/206/416
  through get_media -> prepare_file_response -> FileResponse); /ping stays prompt and
  issues zero TG RPC while a slow op is parked; in-flight dedup shares one download and
  drains _inflight after completion AND after request cancellation (no stuck key / hung
  waiter); str(channel) access-time hit -> flush -> the bulk UPDATE lands on the seeded
  row (mutation-verified: transposing the key columns reds it).
- docs/stability-verification.md: per-stage DoD -> evidence mapping (test or "operator
  observation" for prod-only items), the exact manual curl/lsof scenarios for the
  operator to run post-deploy, the diag-log signals to watch, and the per-stage
  independent-commit rollback plan.

Full integrated suite: 260 passed (252 baseline + 8). The prod deploy + live diag-log
observation (plan items 3-4) are the operator's call — this stage does not deploy.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review finding (low, doc-only): the rollback plan said each stage is one
independently-revertable commit, but each stage is actually two commits on its
branch (feature + review-round fix, the latter often fixing a real bug), so
reverting a single commit would orphan the review-round fix. Reword the unit of
rollback to the whole STAGE (branch/PR) and spell out the revert command per merge
strategy: squash-merge -> revert the one squash commit; merge-commit -> revert -m 1
the merge; linear history -> revert the full range of the stage's commits.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent_vscode merged commit b2b0a13557 into fix/stage-4-eventloop 2026-07-05 16:58:25 +03:00
agent_vscode deleted branch fix/stage-5-sqlite 2026-07-05 16:58:25 +03:00
Sign in to join this conversation.