Files
orchestrator/docs/work-items/ORCH-087/12-review.md

73 lines
5.2 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
---
type: review
work_item_id: ORCH-087
verdict: APPROVED
version: 1
---
# Review ORCH-087
## Summary
Задача закрывает три проблемы live-трекера: (G1) осиротевшие «замёрзшие» карточки,
(BR-EFF) эффорт в строке стадии, (BR-G5) честное итоговое время, плюс попутный CI-фикс
пути per-run логов. Реализация соответствует ТЗ, ADR-001 и критериям приёмки. Все 1090
тестов зелёные. Документация (CLAUDE.md, README.md, docs/architecture/README.md,
CHANGELOG.md, ADR) обновлена в том же PR. Машина стадий и реестр QG не тронуты; миграции
аддитивны/идемпотентны; never-raise сохранён. Найдена одна косметика P3 (неточный
inline-комментарий), не влияющая на поведение. Блокеров нет.
## Соответствие ТЗ / ADR
- **G1 (BR-G1, AC-1.x):** аддитивный леджер `tracker_messages(task_id, message_id,
created_at, deleted_at)` + хелперы `add_tracker_message` / `get_open_tracker_messages` /
`mark_tracker_message_deleted` (`src/db.py`). На каждом bump зачищаются ВСЕ незакрытые
mid (union скаляр+леджер). Контракт `delete_telegram` (True=gone вкл. `_DELETE_GONE_MARKERS`,
False=transient) совпадает с логикой `if delete_telegram(old): mark_deleted(...)`;
transient остаётся открытым для ретрая. Новый mid в леджер ТОЛЬКО при `send is not None`
(R-3/BR-6). Скаляр `tracker_message_id` сохранён (BC). ✔ соответствует ADR §G1 (вариант A1).
- **G3 (AC-3.x):** ключ `confirm_deploy` добавлен в `_LIVE_BRANCH_LABELS` — цикл
`Awaiting Deploy → Deploying → Confirm Deploy → Monitoring → Done` полон. ✔
- **BR-EFF (AC-E.x):** колонка `agent_runs.effort TEXT` (`_ensure_column`, идемпотентно);
стамп фактического `resolve_agent_effort` в `launcher._spawn` через `UPDATE` по `run_id`
(never-block, обёрнут try/except); рендер `· {model} · {effort}`, пустой → опускается. ✔
- **BR-G5 (AC-5.x):** done-строка переписана на три подписанных метрики
`⏱️ Агенты · твоё{~cap} · общее с ожиданием`; кап `tracker_brd_review_cap_s` (дефолт 2ч,
маркер `~`); `_capped_review_str` never-raise; agent-сумма не регрессировала. ✔
- **BR-G6 (AC-6.x):** `src/reconciler.py` / `tests/test_reconciler.py` НЕ тронуты;
`git merge-base --is-ancestor origin/main HEAD` → true; origin/main содержит merge ORCH-086;
маркеры ORCH-086 (`skipped_terminal_total`/`state_uuid`/terminal) на месте. ✔
- **Инварианты (AC-X.5):** `STAGE_TRANSITIONS` / `QG_CHECKS` без изменений; миграции
`CREATE TABLE/INDEX IF NOT EXISTS` + `_ensure_column` — аддитивны/идемпотентны (enduro не
трогается); `disable_notification` / `plane_issue_link` / `disable_web_page_preview` —
сохранены. ✔
- **ADR (AC-0.x):** ADR-001 отвечает на 4 вопроса §4 BRD, содержит таблицу staging-
воспроизведения и known-limitation Telegram 48ч (AC-1.4). Код фикса соответствует ADR. ✔
## Findings
### P0 — Blocker
- (нет)
### P1 — Must fix
- (нет)
### P2 — Should fix
- (нет)
### P3 — Nice to have
- [ ] `src/notifications.py` (~стр. 460, докстринг блока рендера эффорта): комментарий
утверждает «Historical rows with NULL effort fall back to the config-resolved effort for
the agent», но `_run_effort` фолбэка на `resolve_agent_effort` НЕ делает — при пустом/NULL
effort возвращает `""` и суффикс опускается. Поведение корректно и соответствует AC-E.4
(fallback по ТЗ §5 был «Допустим», не обязателен); неточен лишь комментарий — стоит убрать
вводящую в заблуждение фразу или реально добавить фолбэк. Не влияет на работу.
## Документация
Обновлена в ТОМ ЖЕ PR (AC-X.3 выполнен):
- `CLAUDE.md` — §Нотификации/Telegram live-tracker (зачистка сирот, эффорт, честное время).
- `docs/architecture/README.md` — компонент Notifications + отдельный раздел ORCH-087.
- `README.md` — таблица env (`ORCH_RUNS_DIR`).
- `CHANGELOG.md` — `## [Unreleased]` запись (ORCH-087 трекер + CI-фикс пути логов).
- ADR `06-adr/ADR-001-tracker-orphan-cleanup.md` — присутствует, покрывает G0/механизм/формулу.
Замечаний по документации нет.