reviewer(ET): auto-commit from reviewer run_id=396
This commit is contained in:
69
docs/work-items/ORCH-074/12-review.md
Normal file
69
docs/work-items/ORCH-074/12-review.md
Normal file
@@ -0,0 +1,69 @@
|
||||
---
|
||||
type: review
|
||||
work_item_id: ORCH-074
|
||||
verdict: APPROVED
|
||||
version: 1
|
||||
---
|
||||
|
||||
# Review ORCH-074
|
||||
|
||||
## Summary
|
||||
PR закрывает оба зафиксированных дефекта каркаса выбора модели (ORCH-41) в рамках
|
||||
скоупа G1 + G2 (+ защитный гард точки чтения fallback при выключенном G4), без
|
||||
изменения механизма резолва, API или схемы БД. Реализация точно соответствует
|
||||
ADR-001 и ТЗ; документация синхронизирована в том же PR; все 1012 тестов зелёные.
|
||||
Вердикт — **APPROVED**, P0/P1 findings нет.
|
||||
|
||||
## Соответствие ТЗ и AC
|
||||
- **AC-1 (G1):** `grep -L "^model:" .openclaw/agents/*.md` возвращает все 6 файлов;
|
||||
ни одной строки `^model:` не осталось. frontmatter остаётся валидным YAML
|
||||
(`name`/`description`/`tools` сохранены) — покрыто `test_agent_frontmatter_no_model.py`.
|
||||
- **AC-2 (G2 never-break):** `resolve_agent_model` валидирует имя через `is_valid_model`
|
||||
ПЕРЕД возвратом, мусорный уровень логируется (`logger.warning`) и пропускается;
|
||||
при невалидных всех уровнях → `""` (CLI-дефолт), исключение не бросается. TC-03..05.
|
||||
- **AC-3:** все 6 агентов резолвятся в `claude-opus-4-8` (TC-07), значение в README-таблице
|
||||
и `.env.example`.
|
||||
- **AC-4 (G3):** N/A — отказ зафиксирован в ADR.
|
||||
- **AC-5 (G4):** `agent_fallback_model=""` (выкл); тот же предикат гардит inline-чтение
|
||||
fallback в `_spawn` (код-факт TRZ §4 учтён) — мусорный fallback дропается. ADR помечает N/A.
|
||||
- **AC-6 (доки):** README (новая секция «Модель и эффорт по ролям» + валидация),
|
||||
`CLAUDE.md`, `.env.example` синхронизированы; стале-упоминаний `claude-sonnet-4-6`/
|
||||
`claude-opus-4-7` как модели агента в актуальных доках нет (`grep` пуст).
|
||||
- **AC-7:** `pytest tests/ -q` → 1012 passed.
|
||||
- **AC-8:** валидный enduro per-project override проходит без изменения поведения (TC-08).
|
||||
- **AC-9:** ADR-001 фиксирует G1 «убрать», предикат G2 (формат-чек vs allowlist с
|
||||
обоснованием), решения по G4 и G3.
|
||||
|
||||
## Соответствие ADR
|
||||
Реализация 1:1 с ADR-001: `is_valid_model` + `_MODEL_NAME_RE` (`^claude-[a-z0-9.-]+$`)
|
||||
рядом с `VALID_EFFORTS`; один предикат, две точки вызова (резолв модели и чтение
|
||||
fallback); каскад приоритетов ORCH-41 сохранён (рефакторинг на генератор
|
||||
`_agent_model_candidates` с валидацией-со-скипом); версия модели по-прежнему живёт
|
||||
только в `config.py::agent_model_default`. Глобальные ADR не нарушены.
|
||||
|
||||
## Качество кода
|
||||
- `is_valid_model` корректно обрабатывает `None`/пустое/whitespace (`if not name`),
|
||||
никогда не бросает; содержательные docstrings с обоснованием формат-чека.
|
||||
- never-break соблюдён в обеих точках; `if fb` short-circuit сохраняет нулевую
|
||||
регрессию для текущего пустого fallback.
|
||||
- Тесты содержательные: предикат (accept/reject), каскад-скип, граничные кейсы,
|
||||
регрессия per-project override, выключенный G4.
|
||||
|
||||
## Findings
|
||||
|
||||
### P0 — Blocker
|
||||
- нет
|
||||
|
||||
### P1 — Must fix
|
||||
- нет
|
||||
|
||||
### P2 — Should fix
|
||||
- нет
|
||||
|
||||
## Документация
|
||||
Обновлена полностью в этом же PR: `docs/architecture/README.md` (компонент Agent
|
||||
Launcher + новая секция «Модель и эффорт по ролям» с таблицей и описанием валидации),
|
||||
`CLAUDE.md` (строка про источник модели и валидацию), `.env.example` (блок
|
||||
`ORCH_AGENT_MODEL_*`/`ORCH_AGENT_EFFORT_*`/`ORCH_AGENT_FALLBACK_MODEL`),
|
||||
`CHANGELOG.md` (запись по задаче), ADR `06-adr/ADR-001-model-name-validation.md`.
|
||||
Требование «изменён src/ → обновлена документация» выполнено.
|
||||
Reference in New Issue
Block a user