reviewer(ET): auto-commit from reviewer run_id=97
This commit is contained in:
120
docs/work-items/ORCH-016/12-review.md
Normal file
120
docs/work-items/ORCH-016/12-review.md
Normal file
@@ -0,0 +1,120 @@
|
||||
---
|
||||
type: review
|
||||
work_item_id: ORCH-016
|
||||
verdict: APPROVED
|
||||
version: 1
|
||||
---
|
||||
|
||||
# Review ORCH-016 — Единый status-коммент агентов в Plane
|
||||
|
||||
## Summary
|
||||
|
||||
PR реализует ТЗ ORCH-016 и ADR-001 полностью: вводится единый хелпер
|
||||
`src/usage.build_status_comment(...)` для всех ролей (analyst…deployer),
|
||||
строка `Длительность: …` с явным `duration_s` от launcher и DB-фоллбэком для
|
||||
аналитика, defensive YAML-парсер `src/frontmatter.read_frontmatter_value`,
|
||||
HTML-формат с эмодзи / Verdict / Документы / `<sub>` тех-хвостом. Аналитик
|
||||
переведён на ту же ветку без регрессии (`tests/test_analyst_comment.py` +
|
||||
`tests/test_analyst_status_only_regression.py` зелёные). `usage_comment` стал
|
||||
deprecated-обёрткой, `artifact_links` теперь возвращает HTML-фрагменты
|
||||
(breaking-change только для внутреннего вызова из удаляемого пути).
|
||||
Документация обновлена: CHANGELOG.md (`Added` + `Changed`),
|
||||
`docs/architecture/README.md` (новый подраздел «Plane Sync: единый
|
||||
status-коммент агентов»), ADR-001 заведён в
|
||||
`docs/work-items/ORCH-016/06-adr/`.
|
||||
|
||||
Прохождение тестов:
|
||||
- 60 новых ORCH-016 тестов: PASS (TC-01…TC-23 покрывают AC-1…AC-14).
|
||||
- TC-20 (`test_qg_registry_snapshot.py`) подтверждает: `QG_CHECKS` и
|
||||
`STAGE_TRANSITIONS` бит-идентичны (AC-11).
|
||||
- Полный прогон: 392 PASS, 4 FAIL (`tests/test_m6_sequence.py::*`,
|
||||
`tests/test_plane_webhook.py::test_orchestrator_project_routes_to_orchestrator_repo`,
|
||||
`tests/test_plane_webhook.py::test_prefixes_independent_per_project`).
|
||||
Эти 4 фейла **предсуществуют на `main`** (проверено: `git checkout main --
|
||||
src/ tests/` → те же 4 фейла; ORCH-016 их не индуцировал). AC-10 «no
|
||||
regression» соблюдено.
|
||||
|
||||
Соответствие ТЗ (`02-trz.md`):
|
||||
- §1 модули: тронуты строго заявленные (`usage.py`, `stage_engine.py`,
|
||||
`agents/launcher.py`, новый `frontmatter.py`); `qg/checks.py` сознательно
|
||||
не трогается (ADR-001 §5, alt-6).
|
||||
- §2.1–§2.5 формат, описания, verdict, ссылки, duration — реализовано.
|
||||
- §3 API не меняется; §4 БД не меняется; §5 новых QG нет — подтверждено
|
||||
TC-20.
|
||||
- §6 docstrings, graceful frontmatter / duration, `fmt_duration` — чистая,
|
||||
AC-13 happy + edge кейсы зелёные.
|
||||
- §7 артефакты: ADR заведён.
|
||||
- §8 документация: README архитектуры и CHANGELOG обновлены, `CLAUDE.md`
|
||||
не трогается (правила не меняются).
|
||||
- §9 запреты: `QG_CHECKS` / `STAGE_TRANSITIONS` / `add_comment` /
|
||||
`_headers_for` / `PLANE_BOT_TOKENS` не тронуты; `--no-verify` не
|
||||
использован.
|
||||
|
||||
Соответствие ADR-001:
|
||||
- §1 единственный публичный `build_status_comment(...)` с указанной
|
||||
сигнатурой ✓
|
||||
- §2 описания per-agent ✓
|
||||
- §3 `<sub>` тех-хвост ✓
|
||||
- §4 русская метка `Длительность:` ✓
|
||||
- §5 `src/frontmatter.py` ✓
|
||||
- §6 `get_agent_duration` с указанным SQL ✓
|
||||
- §7 HTML-якоря, `<br>` разделители ✓
|
||||
- §8 `fmt_duration` контракт ✓
|
||||
|
||||
Self-hosting (ADR-001 «Последствия»): хелперы — чистый код, без рестарта
|
||||
прод-контейнера; пройдёт стандартный staging-гейт.
|
||||
|
||||
## Findings
|
||||
|
||||
### P0 — Blocker
|
||||
- Нет.
|
||||
|
||||
### P1 — Must fix
|
||||
- Нет.
|
||||
|
||||
### P2 — Should fix
|
||||
- Нет.
|
||||
|
||||
### P3 — Nice to have
|
||||
- `src/usage.py` `_AGENT_DESCRIPTIONS` и встроенные строки в
|
||||
`build_status_comment` (например, `"Длительность: " f"{d_text}"` и
|
||||
`"Завершил " "архитектурную " "проработку. " "См. ADR ниже."`) разбиты
|
||||
на множественные смежные литералы. Python склеит их корректно, но
|
||||
читаемость страдает — рассмотреть однострочный литерал в follow-up.
|
||||
- `03-acceptance-criteria.md` AC-3 формулирует пример как
|
||||
`verdict: APPROVE`, тогда как канонический QG (`check_reviewer_verdict`,
|
||||
`src/qg/checks.py:306`) ожидает строго `verdict: APPROVED`. На
|
||||
отображение коммента это не влияет (билдер показывает то, что лежит
|
||||
во frontmatter), но в самом AC лучше было бы зафиксировать тот же
|
||||
термин, что в QG. Чинить артефакт стадии analysis из стадии review —
|
||||
out-of-scope (правило: «не править артефакты других этапов»);
|
||||
оставляю как заметку на follow-up для аналитика.
|
||||
- `_post_usage_comments` для `deployer` всегда (включая
|
||||
`deploy-staging`) дополнительно постит `task_summary_comment`. ТЗ §2.6
|
||||
и AC-7 явно это не запрещают (саммари не считается status-комментом),
|
||||
и `tests/test_post_usage_comments_integration.py::test_deployer_staging_picks_15_log`
|
||||
это поведение фиксирует. Поведение работает, но смысловой саммари
|
||||
«Итого по задаче» на staging-стадии (задача не завершена) — слегка
|
||||
ранний. Кандидат на уточнение требований в отдельной задаче.
|
||||
|
||||
## Документация
|
||||
|
||||
- `CHANGELOG.md` — раздел `Unreleased` дополнен записями `Added` и
|
||||
`Changed` с упоминанием ORCH-016, `build_status_comment`,
|
||||
`fmt_duration`, `get_agent_duration`, `src/frontmatter.py` и
|
||||
ссылки на ADR. ✓
|
||||
- `docs/architecture/README.md` — добавлен подраздел «Plane Sync:
|
||||
единый status-коммент агентов (ORCH-016)» с описанием формата
|
||||
HTML-блока, источниками длительности и вердиктов, явным указанием,
|
||||
что реестр гейтов и стадий не меняется. ✓
|
||||
- `docs/work-items/ORCH-016/06-adr/ADR-001-unified-status-comment.md` —
|
||||
заведён, статус `Accepted`, покрывает все 5 открытых вопросов ТЗ
|
||||
и пять альтернатив. ✓
|
||||
- `CLAUDE.md` — правки не требовались (правила агентов и канон
|
||||
документации без изменений), что и заявлено в ADR-001.
|
||||
- `docs/architecture/internals.md` — упоминания про `usage.py` /
|
||||
комменты не имеет, обновление не требуется (как и оговорено
|
||||
ADR-001 §1).
|
||||
|
||||
Документация = golden source соблюдён: изменения в `src/` сопровождены
|
||||
синхронным обновлением документации в том же PR.
|
||||
Reference in New Issue
Block a user