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

7.4 KiB
Raw Blame History

type, work_item_id, verdict, version
type work_item_id verdict version
review ORCH-016 APPROVED 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.