reviewer(ET): auto-commit from reviewer run_id=577

docs/work-items/ORCH-098/12-review.md

@@ -0,0 +1,92 @@
+---
+verdict: APPROVED
+work_item: ORCH-098
+stage: review
+author_agent: reviewer
+status: approved
+created_at: 2026-06-10
+model_used: claude-opus-4-8
+type: review
+work_item_id: ORCH-098
+version: 1
+---
+
+# Review ORCH-098 — FND: машинный журнал уроков
+
+## Summary
+Чистая аддитивная реализация observer-leaf'а `src/lessons.py` + таблица `lessons` строго по образцу
+`serial_gate`/`coverage_gate`/`metrics`. Все 9 критериев приёмки (AC-1…AC-9) выполнены, ТЗ (FR-1…FR-6)
+реализовано полностью, реализация байт-в-байт соответствует ADR-001 (D1…D6). Инварианты конвейера
+не тронуты, документация обновлена в том же PR. Тесты зелёные (13 в `test_lessons.py`, broader sanity
+17 passed). P0/P1 findings нет → **APPROVED**.
+
+## Соответствие ТЗ (ось 1)
+- **FR-1 / AC-1, AC-2** ✅ `CREATE TABLE IF NOT EXISTS lessons` в `db.init_db()` со всеми полями + 3 индекса;
+  колонки атрибуции (`attribution`/`target_repo`/`target_domain`) присутствуют сразу, нуллабельны,
+  плюс forward-safe `_ensure_column` на старой таблице. TC-01/TC-02 подтверждают идемпотентность и
+  nullable-атрибуцию.
+- **FR-2 / AC-6** ✅ `record()` — kill-switch-first no-op, never-raise обёртка над `db.record_lesson`.
+- **FR-3 / AC-3** ✅ Реализованы **4** типа автозаписи (> минимума 2–3): `gate_failure`
+  (`_handle_qg_failure_rollbacks`), `merge_hold` (`_handle_merge_verify` HOLD), `transient_retry`
+  (`launcher._finalize_transient` на исчерпании бюджета), `deploy_degraded` (post-deploy DEGRADED).
+  Каждая врезка обёрнута в локальный `try/except` → ошибка импорта/записи не пробивается в горячий путь.
+  Переменные контекста (`work_item_id`/`repo`/`branch`/`checks_failed`/`checks_total`) проверены в
+  scope перед точкой врезки в `run_post_deploy_monitor`. TC-08/TC-09 — интеграционные.
+- **FR-4 / AC-4** ✅ `GET /lessons` (фильтры type/status/repo/work_item/limit, всегда 200, read-only) +
+  read-only ключ `lessons` в `GET /queue`.
+- **FR-5 / AC-5** ✅ `POST /lessons` (`source="manual"`, не дедупится) + `POST /lessons/{id}` (update со
+  стампом `updated_at`). TC-07/TC-11.
+- **FR-6 / AC-7** ✅ `lessons_enabled=False` → все функции инертны без обращения к БД, эндпоинты отдают
+  `{"enabled": false}`. TC-05.
+
+## Соответствие ADR (ось 2)
+- D1 (таблица/индексы/forward-safe), D2 (no repo-scope, kill-switch only), D3 (4 детектора), D4 (дедуп
+  один indexed-SELECT по `idx_lessons_wi_type`, только `auto`, окно `lessons_dedup_window_s`), D5
+  (эндпоинты), D6 (изоляция от гейтов) — реализованы как описано.
+- **AC-8 проверен диффом:** `src/stages.py` (`STAGE_TRANSITIONS`) и `src/qg/` (`QG_CHECKS`/`check_*`) —
+  **не затронуты**; machine-verdict-ключи и схемы существующих таблиц не тронуты. Journal не участвует
+  в решении гейта (TC-12).
+- **Трассировка маркеров:** правок чужих `ORCH-NNN`-инвариантов нет — изменения чисто аддитивные
+  (новые блоки в choke-point'ах, существующая логика не переписана). Слом инвариантов отсутствует.
+
+## Качество кода (ось 3)
+- never-raise контракт соблюдён на всех публичных функциях leaf'а и во всех 4 врезках; DDL-хелперы
+  `db.py` корректно открывают/закрывают соединение в `finally` (паттерн `coverage_baseline`).
+- Тесты содержательные: idempotency, nullable-атрибуция, never-raise с замоканной падающей БД (TC-04),
+  kill-switch (TC-05), дедуп vs manual-passthrough (TC-07b), интеграция автозаписи (TC-08/TC-09),
+  эндпоинты (TC-10/TC-11), инварианты конвейера (TC-12). `pytest tests/test_lessons.py -q` → 13 passed.
+- **Регресс-тест (ORCH-019/BR-4):** N/A — задача не багфикс (foundation-feature, метки `Bug` нет,
+  полный маршрут с `architecture`/ADR).
+
+## Findings
+
+### P0 — Blocker
+- нет
+
+### P1 — Must fix
+- нет
+
+### P2 — Should fix
+- [ ] **Грубость дедупа `transient_retry` в `launcher`.** Врезка в `_finalize_transient` передаёт
+  `task_id`, но **не** `work_item_id` и не `stage`, тогда как дедуп-ключ D4 = `(work_item_id,
+  lesson_type, stage)`. Для launcher-уроков оба = `NULL`, поэтому два разных задания, исчерпавших
+  бюджет транзиентов в окне `lessons_dedup_window_s` (дефолт 1ч), коллапсируют в один ключ
+  `(NULL, transient_retry, NULL)` → второй урок подавляется (потеря межзадачного сигнала). Реализация
+  следует букве ADR-D4, но точность дедупа на этом детекторе ниже задуманной. Рекомендация (не блокер):
+  передавать `work_item_id` (если доступен в `job`) либо включить `task_id` в дедуп-ключ для
+  `transient_retry`. Сигнал редкий (только на исчерпании бюджета), поэтому влияние ограничено.
+
+## Документация
+Обновлена в том же PR — проверено явно:
+- `CLAUDE.md` — новый раздел «Машинный журнал уроков (ORCH-098)» (+38).
+- `docs/architecture/README.md` — компонент **Lessons journal**, таблица `lessons` в списке схемы БД,
+  3 новые строки API-таблицы (`GET /lessons`, `POST /lessons`, `POST /lessons/{id}`), `lessons` в
+  описании `GET /queue`.
+- `CHANGELOG.md` — запись `feat` (ORCH-098) с разбивкой D1…D5 + регресс.
+- ADR: локальный `06-adr/ADR-001-lessons-journal.md` (proposed) + сквозной
+  `docs/architecture/adr/adr-0033-lessons-journal.md` (существует).
+- `README.md` «Известные ограничения» — данный PR не закрывает ни одного пункта витрины (обзорные
+  доки обновлять не требуется).
+
+Документация = golden source: изменения `src/` сопровождены полным обновлением доков → P0 «src
+изменён, документация не обновлена» **не применяется**.