Files
orchestrator/docs/work-items/ORCH-118/12-review.md
claude-bot 5b36ca0a82
All checks were successful
CI / test (push) Successful in 1m10s
CI / test (pull_request) Successful in 1m11s
reviewer(ET): auto-commit from reviewer run_id=729
2026-06-16 00:22:52 +03:00

124 lines
11 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.
---
verdict: APPROVED
work_item: ORCH-118
stage: review
author_agent: reviewer
status: approved
created_at: 2026-06-16
model_used: claude-opus-4-8
type: review
work_item_id: ORCH-118
version: 1
---
# Review ORCH-118 — карта LLM-консультаций, control-path-ось «avoidable», roadmap и политика
> Машинный вердикт читается ТОЛЬКО из `verdict:` во frontmatter. `APPROVED` → дальше по конвейеру.
## Summary
ORCH-118 — зонтичная **inventory-first, docs + tests only** задача (RCA-трек ORCH-110…117): выпускает
доказательную карту LLM-консультаций, нормативную политику использования LLM, упорядоченный roadmap
детерминизации и набор структурных анти-дрейф тестов. Реализация раннеров — вне скоупа (FR-7).
Работа выполнена на эталонном уровне: скоуп выдержан **байт-в-байт** (`src/**` не тронут вообще),
каждый `file:line`-якорь карты резолвится в реальный код, все FR-1…FR-8 и AC-1…AC-10 закрыты, golden-source
синхронизирован, полный прогон `pytest tests/ -q`**зелёный (2081 passed)**. Один **P2** (косметика):
в трёх docs-артефактах (оба ADR + 10-tech-risks) на EOF протекли служебные теги `</content>`/`</invoke>`
из механизма записи файла. Это не ломает гейты/тесты/парсинг машинных блоков и не влияет на рантайм →
не блокирует. Рекомендуется зачистить.
Проверка проведена буквально по файлам: запущены оба новых тест-файла (13 passed), `test_system_docs.py`
(29 passed) и полный `tests/` (2081 passed); сверены ключевые якоря `launcher.py` и `src/qg/checks.py`;
подтверждено `git diff origin/main...HEAD -- src/` = **пусто**.
> ⚠️ **Замечание по базе ревью.** Локальный `main` в worktree **устаревший** (`64ba121`, до мержей
> ORCH-112/113/114/117). Истинная база ветки — `origin/main` (`13589fc`, мерж ORCH-117). Ревью
> проводилось против `origin/main...HEAD` (реальный changeset ORCH-118 — 16 файлов, +2365), а НЕ против
> stale `main...HEAD` (который ложно тянет чужие уже-смерженные `src/**`-правки ORCH-110…117).
## Оси проверки
### 1. Соответствие ТЗ (02-trz / 03-acceptance-criteria)
- **FR-1 / AC-1 (инвентарь полон и привязан):** карта `llm-call-sites.md` перечисляет `S0`, `A1…A6`,
`D1/D2` со всеми обязательными полями (`location`/`trigger`/`stage`/`output`/`machine-verdict key`/
`output consumer`/`est. tokens`/`consults-LLM`/`axis`/`classification`/`rationale`). **Все якоря
резолвятся** — сверено: `launcher.py:472`=`def _spawn(`, `610-614`=сборка `--system-prompt "$(cat …)"`,
`838`=`_monitor_agent`, `389/394`=перехваты `deploy-finalizer`/`post-deploy-monitor`, `407/428`=«Not an
LLM spawn»; `checks.py:33/62/82/182/226/336/413/473/538/599/657` — все 11 точно совпали с целевыми
`def`. ✔
- **FR-2 / AC-2 (таксономия 4 класса, выведена из оси):** классы определены, каждому site присвоен ровно
один (TC-04 тотальность), правило вывода `P→keep`, `C+!деривируем→keep`, `C+деривируем→replace/hybrid`
закреплено тестом (TC-04 axis-consistency). ✔
- **FR-3 / AC-3 (детерминизм не-агентских путей):** §3 карты + TC-02 с file:line; дискриминатор —
«консультирует LLM», а не «спавнит subprocess». ✔
- **FR-4 / AC-4 (roadmap + первый срез):** машинный блок roadmap с rank/deps/savings(источник `agent_runs`)/
risk/hybrid/followup_type/first_slice; ровно один `first_slice=yes`=deployer (TC-07). ✔
- **FR-5 / AC-5 (политика):** `llm-usage-policy.md` нормативна, критерии keep/replace через ось,
определение «avoidable» как двухбитный предикат (TC-08). ✔
- **FR-6 / AC-6 (структурные тесты):** TC-01…TC-14 покрывают единственный транспорт (a)+отсутствие иного
(f/TC-12), детерминированные пути (b), промпты↔файлы (c), тотальность (d), capability≠consultation (e),
control-path-разметку (g/TC-13), avoidable-набор (h/TC-14). Offline, stdlib-only, осмысленные. ✔
- **FR-7 / AC-7 (скоуп-гард):** `git diff origin/main...HEAD -- src/` пусто; `STAGE_TRANSITIONS`/
`QG_CHECKS`/`check_*`/machine-verdict/схема БД — нетронуты (TC-09 фиксирует снимок); новых раннеров нет. ✔
- **FR-8 / AC-10 (control-path-ось + «avoidable»):** ось C/P размечена по ролям с доказательством-потребителем,
термин определён, набор `{tester, deployer}` назван поимённо и отделён от `{reviewer}` (C-keep) и
`{analyst,architect,developer}` (P), сверено с `src/qg/checks.py` (TC-13/TC-14). ✔
- **AC-9 (анти-фабрикация ID):** follow-up'ы названы по роли; все `ORCH-1XX`-ссылки резолвятся в реальные
work-item-папки (TC-11 зелёный; подтверждено наличие ORCH-110/111/112/113/114/117). ✔
### 2. Соответствие ADR (06-adr / adr-0047 / глобальные ADR)
- Деливерабл-доки **зеркалят** канонические таблицы ADR-001 D2/D4 без расхождений (поля инвентаря,
классы, потребители, avoidable-набор, первый срез=deployer-staging). ✔
- D1 (3 durable-дока в `docs/architecture/`), D3 (3 оси + определение), D5 (offline-тесты, конъюнкция
признаков транспорта против false-positive на `preflight.py`/`config.py`), D6 (deployer первым), D7
(скоуп-гард) — реализованы верно. ✔
- **Глобальные ADR не нарушены:** machine-verdict-ключи и реестр `QG_CHECKS` байт-в-байт (TC-09);
трассировка ORCH-078 — маркированные инварианты `src/**` не правились (src не тронут). ✔
### 3. Качество кода (тестов)
- Тесты содержательные, не тривиальные: парсят машинные блоки карты/roadmap/политики и сверяют с
ground-truth кода (`src/qg/checks.py`, `launcher.py`, `.openclaw/agents/`). `_function_body` устойчив к
дрейфу строк; TC-01 корректно требует **конъюнкцию** `CLAUDE_BIN`+`--system-prompt`+launcher (исключая
ложные срабатывания); TC-12 закрывает «второй транспорт». Без сети/LLM/subprocess-к-модели. ✔
- **Регресс-тест-фиксатор (ORCH-019 BR-4) — N/A:** ORCH-118 — не багфикс-трек, а полный маршрут
design/inventory; обязательного теста-фиксатора дефекта не требуется. ✔
- Security/утечки — N/A (рантайм не меняется). ✔
### 4. Документация (golden-source — обязательная ось)
- **AC-8 синхронизация:** `docs/architecture/README.md` (секция + ссылка adr-0047), витрина
`docs/overview/tech-quality-security.md` (раздел «Где уместен LLM» + ссылки на все 3 дока, ORCH-011),
`CHANGELOG.md` — обновлены в этом же PR. `test_system_docs.py` зелёный (29 passed). ✔
- ADR заведён (work-item ADR-001 + сквозной adr-0047). ✔
- Поскольку `src/**` не менялся, ось «изменён src → обнови доку» неприменима как P0; обзорные доки
(ORCH-079/011) обновлены. ✔
## Findings
### P0 — Blocker
- (нет)
### P1 — Must fix
- (нет)
### P2 — Should fix
- [ ] **Протёкшие служебные теги на EOF трёх docs-артефактов.** На конце файлов остались артефакты
механизма записи, не относящиеся к содержимому (контент перед ними полный):
- `docs/work-items/ORCH-118/06-adr/ADR-001-llm-call-site-map-and-determinization-roadmap.md:294-295`
`</content>` + `</invoke>`
- `docs/architecture/adr/adr-0047-llm-usage-policy-and-call-site-map.md:114``</content>`
- `docs/work-items/ORCH-118/10-tech-risks.md:43``</content>`
Правило: «Документация = golden source» (CLAUDE.md §2) — durable-ADR не должен нести постороннюю
разметку. Не блокирует (тесты/гейты/машинные блоки карты не затронуты, рантайм-риска нет; три
новых dev-дока `llm-call-sites.md`/`llm-usage-policy.md`/`llm-determinization-roadmap.md`**чистые**).
Рекомендация: удалить хвостовые строки с тегами в этом же PR.
## Документация
**Обновлена полностью и корректно.** В PR синхронизированы все golden-source точки: `docs/architecture/README.md`
(+секция ORCH-118 и ссылка на adr-0047), витрина `docs/overview/tech-quality-security.md` (раздел про
карту LLM + ссылки на 3 дока), `CHANGELOG.md`, оба ADR (work-item + сквозной adr-0047). Три новых durable-дока
в `docs/architecture/` (`llm-call-sites.md`/`llm-determinization-roadmap.md`/`llm-usage-policy.md`) консистентны
между собой и с ADR; машинные блоки прибиты тестами. Единственное замечание по документации — косметический
P2 выше (хвостовые служебные теги в 2 ADR + tech-risks), не требующий отката.