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

verdict, work_item, stage, author_agent, status, created_at, model_used, type, work_item_id, version

verdict	work_item	stage	author_agent	status	created_at	model_used	type	work_item_id	version
APPROVED	ORCH-098	review	reviewer	approved	2026-06-10	claude-opus-4-8	review	ORCH-098	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 изменён, документация не обновлена» не применяется.