reviewer(ET): auto-commit from reviewer run_id=782
This commit is contained in:
@@ -1,35 +1,37 @@
|
||||
---
|
||||
verdict: REQUEST_CHANGES
|
||||
verdict: APPROVED # APPROVED | REQUEST_CHANGES — строго одно из двух, UPPERCASE
|
||||
work_item: ORCH-120
|
||||
stage: review
|
||||
author_agent: reviewer
|
||||
status: changes-requested
|
||||
status: approved
|
||||
created_at: 2026-06-17
|
||||
model_used: claude-opus-4-8
|
||||
type: review
|
||||
work_item_id: ORCH-120
|
||||
version: 1
|
||||
version: 2
|
||||
---
|
||||
|
||||
# Review ORCH-120 — Открытые вопросы аналитика → Needs Input
|
||||
|
||||
## Summary
|
||||
|
||||
Реализация по существу **сильная и завершённая**: мёртвый путь «аналитик задаёт блокирующие
|
||||
вопросы → `01-questions.md` → Needs Input» корректно активирован четырьмя согласованными
|
||||
изменениями (контракт промпта + канон артефакта; приоритет вопросов над `files_ok`; авто-park
|
||||
через ось «пауза» ORCH-124; resume + unpark). Чистая логика вынесена в leaf `src/analyst_questions.py`
|
||||
(never-raise, kill-switch, self-hosting-скоуп — зеркало `coverage_gate`/`serial_gate`). Side-effects
|
||||
изолированы в `stage_engine` (`_decide_analysis_outcome`/`_emit_analysis_*`). ORCH-089 autoApprove-блок
|
||||
перенесён **байт-в-байт**. `STAGE_TRANSITIONS` / реестр и имена `QG_CHECKS` / `check_*` /
|
||||
machine-verdict-ключи / схема БД — **подтверждённо не тронуты** (`git diff` по `src/stages.py`,
|
||||
`src/qg/checks.py`, `src/db.py` пуст). Все 11 AC реализованы и покрыты; обязательный регресс-тест
|
||||
TC-01 — **подтверждённо красный** на дофиксовом коде (`files_ok` проверялся до вопросов) и зелёный
|
||||
после. Полный регресс `pytest tests/` — **2205 passed**.
|
||||
Реализация **сильная, завершённая и корректная**. Ранее **мёртвый** путь «аналитик задаёт
|
||||
блокирующие вопросы → `01-questions.md` → Needs Input» активирован четырьмя согласованными
|
||||
изменениями (контракт промпта + канон артефакта; приоритет вопросов над `files_ok`; авто-park через
|
||||
ось «пауза» ORCH-124; resume + unpark). Чистая логика вынесена в leaf `src/analyst_questions.py`
|
||||
(never-raise, kill-switch, self-hosting-скоуп — зеркало `coverage_gate`/`serial_gate`); side-effects
|
||||
изолированы в `stage_engine` (`_decide_analysis_outcome` / `_emit_analysis_needs_input` /
|
||||
`_emit_analysis_in_review` / `_emit_analysis_empty`). ORCH-089 autoApprove-блок перенесён
|
||||
**байт-в-байт** (сверено по `git show origin/main`). `STAGE_TRANSITIONS` / реестр и имена `QG_CHECKS`
|
||||
/ `check_*` / machine-verdict-ключи / схема БД — **подтверждённо не тронуты** (пустой `git diff` по
|
||||
`src/stages.py`, `src/qg/checks.py`, `src/db.py`). Все 11 AC реализованы и покрыты; обязательный
|
||||
регресс-тест TC-01 (Bug-трек, ORCH-019 BR-4) — валидный фиксатор дефекта (красный на дофиксовом
|
||||
`files_ok`-first порядке, зелёный после). Полный регресс `pytest tests/` — **2205 passed** (86s).
|
||||
|
||||
Блокирующий дефект **один**: PR меняет функциональность, описанную в **витрине системы**
|
||||
`docs/overview/` (ось пауз serial-gate), но витрина не обновлена в том же PR — нарушение оси
|
||||
обзорных доков (ORCH-011 / ORCH-079, правило для агентов №6). Это требует `REQUEST_CHANGES`.
|
||||
**Блокировавший ранее дефект устранён.** Предыдущая ревизия (v1, run_id=780) выносила
|
||||
`REQUEST_CHANGES` из-за единственного P1 — необновлённой **витрины системы** `docs/overview/`
|
||||
(ось ORCH-011 / ORCH-079). Коммит `19c3177` обновил витрину в этом же PR (см. раздел «Документация»);
|
||||
ось закрыта, `tests/test_system_docs.py` зелёный. Новых P0/P1/P2 нет → **APPROVED**.
|
||||
|
||||
## Findings
|
||||
|
||||
@@ -37,68 +39,67 @@ TC-01 — **подтверждённо красный** на дофиксово
|
||||
*(нет)*
|
||||
|
||||
### P1 — Must fix
|
||||
|
||||
- [ ] **Витрина системы `docs/overview/` не обновлена при изменении функциональности, которую она
|
||||
описывает (ось ORCH-011 / ORCH-079; CLAUDE.md правило №6; reviewer-prompt `<constraints>`).**
|
||||
ORCH-120 вводит **движковый авто-park/unpark** оси «пауза» (`db.set_task_paused` в
|
||||
`_emit_analysis_needs_input`, `clear_task_paused` в `handle_status_start`) — задача ставится на
|
||||
паузу **автоматически**, без действия оператора. Однако витрина описывает ось пауз как
|
||||
**исключительно операторскую**, и теперь это утверждение неполно/противоречит коду:
|
||||
- `docs/overview/tech-pipeline.md:110-112` — «пауза без блокировки: **оператор может явно**
|
||||
поставить более раннюю задачу на паузу (durable-сигнал `tasks.paused_at`)». Нет упоминания
|
||||
авто-park движком на Needs Input.
|
||||
- `docs/overview/tech-observability.md:25-26` — «Паузу/возобновление задачи в serial gate
|
||||
**оператор включает явными эндпоинтами** `POST /serial-gate/pause|resume`». После ORCH-120
|
||||
пауза **также** триггерится движком автоматически (analyst Needs Input) и снимается на resume.
|
||||
- `docs/overview/tech-agents.md:11` — строка агента `analyst` перечисляет выходы как 4 deliverable
|
||||
без нового канала «блокирующие вопросы → `01-questions.md` (when-applicable, сигнальный) →
|
||||
Needs Input».
|
||||
|
||||
Витрина — единая точка входа «бизнес + тех» (ORCH-011); читатель получает картину, где пауза —
|
||||
только ручная, что больше не соответствует поведению. **Требуется** обновить
|
||||
`docs/overview/` в этом же PR: как минимум дополнить абзац пауз в `tech-pipeline.md` и пункт
|
||||
пауз в `tech-observability.md` упоминанием авто-park/unpark движком на Needs Input (узкий триггер,
|
||||
под флагом `analyst_needs_input_autopause_enabled`, скоуп self-hosting); опционально — отметить в
|
||||
`tech-agents.md` when-applicable канал `01-questions.md`. После обновления — `tests/test_system_docs.py`
|
||||
должен оставаться зелёным.
|
||||
|
||||
*Примечание по классификации:* остальная документация (README архитектуры, `CHANGELOG.md`,
|
||||
`PIPELINE_DOCS.md`, `analyst.md`, скелет `docs/_templates/01-questions.md`, work-item ADR-001 +
|
||||
сквозной adr-0053) **обновлена качественно** — поэтому это не «`src/` изменён, документация не
|
||||
обновлена» (P0), а точечное необновление **обзорной витрины** (явная ось ≥P1).
|
||||
*(нет)*
|
||||
|
||||
### P2 — Should fix
|
||||
*(нет)*
|
||||
|
||||
### P3 — Nice to have
|
||||
- [ ] Косметика (не привязано к правилу, не блокирует): `_decide_analysis_outcome` в gate-off ветке
|
||||
повторно собирает путь `01-questions.md` (`os.path.join` + `os.path.isfile`), который уже
|
||||
инкапсулирован в `analyst_questions.questions_active`; а `_emit_analysis_*` повторно резолвят
|
||||
`get_worktree_path`. Дублирование намеренно (gate-off ветка = «оригинальный байт-в-байт порядок»),
|
||||
поведенчески безвредно — при будущем рефакторе можно консолидировать резолв worktree.
|
||||
|
||||
## Документация
|
||||
|
||||
**Обновлено (проверено по diff):**
|
||||
**Обновлено (проверено по diff) — golden source синхронизирован с кодом:**
|
||||
- `docs/overview/tech-pipeline.md` — абзац «пауза без блокировки» теперь называет **два** источника
|
||||
паузы (оператор `POST /serial-gate/pause` + **движок** авто-park на Needs Input, под флагом
|
||||
`analyst_needs_input_autopause_enabled`, скоуп self-hosting; симметричный unpark на resume).
|
||||
- `docs/overview/tech-observability.md` — пункт `GET /queue` обновлён: пауза/возобновление в serial
|
||||
gate — от оператора **и** от движка (авто-park на Needs Input).
|
||||
- `docs/overview/tech-agents.md` — строка `analyst` дополнена when-applicable сигнальным
|
||||
`01-questions.md` + врезка о канале «блокирующие вопросы → Needs Input».
|
||||
- `docs/architecture/README.md` — новый раздел «Открытые вопросы аналитика → Needs Input (ORCH-120 —
|
||||
реализовано)» со ссылкой на adr-0053.
|
||||
- `CHANGELOG.md` — запись `[Unreleased]` с полным описанием 4 изменений и флагов.
|
||||
- `CHANGELOG.md` — запись `[Unreleased]` с полным описанием 4 изменений, флагов и витрины.
|
||||
- `docs/_standards/PIPELINE_DOCS.md` — строка манифеста для `01-questions.md` (владелец `analyst`,
|
||||
`when-applicable`, сигнальный, не machine-verdict) + примечание о префиксе `01-` (DQ-4).
|
||||
- `.openclaw/agents/analyst.md` — контракт «блокирующие вопросы → `01-questions.md`, НЕ фабриковать
|
||||
deliverables» + поведение на resume; канон 52d сохранён (анти-дрейф-assert в
|
||||
`tests/test_agent_prompts_canon.py` зелёный).
|
||||
- `docs/_templates/01-questions.md` — новый скелет (frontmatter 52c с плейсхолдерами,
|
||||
контекст / блокирующие вопросы / что разблокирует анализ).
|
||||
- `.env.example` — 3 новых ключа `ORCH_ANALYST_*` с описанием и безопасными дефолтами.
|
||||
deliverables» + поведение на resume; канон 52d сохранён (анти-дрейф-assert
|
||||
`test_orch120_analyst_documents_questions_channel` + канон-тесты зелёные).
|
||||
- `docs/_templates/01-questions.md` — новый скелет (frontmatter 52c с плейсхолдерами; контекст /
|
||||
блокирующие вопросы / что разблокирует анализ).
|
||||
- `.env.example` — 3 ключа `ORCH_ANALYST_*` с описанием и безопасными дефолтами.
|
||||
- ADR: `docs/work-items/ORCH-120/06-adr/ADR-001-analyst-open-questions-needs-input.md` + сквозной
|
||||
`docs/architecture/adr/adr-0053-analyst-open-questions-needs-input-flow.md`.
|
||||
|
||||
**Нужно обновить (P1):** `docs/overview/` — витрина системы (см. P1 выше). Конкретно
|
||||
`tech-pipeline.md` (абзац пауз) и `tech-observability.md` (пункт пауз `GET /queue`); опц.
|
||||
`tech-agents.md` (канал `01-questions.md`).
|
||||
**Обзорные доки / витрина (ORCH-011 / ORCH-079):** `README.md` «Известные ограничения» проверен —
|
||||
**нет** пункта, который закрывается этой задачей (мёртвый путь вопросов не значился ограничением),
|
||||
обновление не требуется. Витрина `docs/overview/` обновлена в том же PR (см. выше). Ось закрыта.
|
||||
|
||||
**Нужно обновить:** ничего.
|
||||
|
||||
## Проверки осей (для прозрачности)
|
||||
|
||||
- **Соответствие ТЗ/AC:** AC-1…AC-11 реализованы и покрыты (`tests/test_orch120_*` + assert канона).
|
||||
Полный регресс зелёный (2205 passed). TC-01 подтверждённо RED→GREEN.
|
||||
- **Соответствие ADR/трассировка:** реализация = ADR-001/adr-0053. `STAGE_TRANSITIONS`/`QG_CHECKS`/
|
||||
`check_*`/machine-verdict/схема БД не тронуты. ORCH-089 autoApprove перенесён вербатим;
|
||||
инварианты ORCH-066/088/124 не сломаны (маркеры сверены).
|
||||
- **Качество кода + багфикс-регресс (ORCH-019 BR-4):** leaf-паттерн чистый, never-raise во всех
|
||||
врезках, kill-switch + скоуп корректны; задача — Bug→escalate full-cycle, обязательный
|
||||
тест-фиксатор дефекта (TC-01) присутствует и валиден.
|
||||
- **Документация:** номерные/стандартные доки обновлены; **обзорная витрина — нет** (P1).
|
||||
- **Соответствие ТЗ/AC:** AC-1…AC-11 реализованы и покрыты
|
||||
(`tests/test_orch120_analyst_needs_input.py` TC-01…TC-10, `..._serial_gate_needs_input.py` TC-04
|
||||
интеграционно через реальный `claim_next_job`, `..._resume_unpark.py` TC-05 + autopause-off,
|
||||
`..._questions_artifact_canon.py`, assert канона). Полный регресс **2205 passed**. TC-01 — валидный
|
||||
обязательный фиксатор (RED→GREEN), требование ORCH-019 BR-4 для Bug→escalate full-cycle выполнено.
|
||||
- **Соответствие ADR / трассировка (ORCH-078):** реализация = ADR-001 / adr-0053 (D1…D5, DQ-1…DQ-4).
|
||||
`STAGE_TRANSITIONS` / `QG_CHECKS` / `check_*` / machine-verdict / схема БД — байт-в-байт (пустой
|
||||
diff). ОRCH-089 autoApprove перенесён вербатим; ось «пауза» ORCH-124 переиспользована (новых
|
||||
колонок нет); инварианты ORCH-066 (Needs Input только у аналитика — не расширен), ORCH-088/124
|
||||
(serial-gate/пауза — лишь корректно триггерится, код не тронут), ORCH-090 (relaunch-guard — unpark
|
||||
врезан ПОСЛЕ гейта, под `current_stage=='analysis'`, не ослаблен) — сверены, не сломаны.
|
||||
- **Качество кода:** leaf-паттерн чистый (импорт только `os`/`logging`/`config` + ленивый
|
||||
`qg.checks`), never-raise во всех публичных функциях и врезках, kill-switch + скоуп корректны
|
||||
(`questions_gate_applies`/`autopause_applies`), docstrings на публичных функциях содержательны.
|
||||
Freshness-supersede (DQ-2) детерминирован и offline; fail-направление к Needs Input — безопасно
|
||||
(«не строить на домыслах»). Гейт-off ветка восстанавливает оригинальный порядок байт-в-байт (AC-9).
|
||||
- **Документация:** golden source (номерные/стандартные доки, ADR, CHANGELOG, `.env.example`) **и**
|
||||
обзорная витрина `docs/overview/` обновлены в том же PR. Ось обзорных доков закрыта.
|
||||
</content>
|
||||
</invoke>
|
||||
|
||||
Reference in New Issue
Block a user