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

70 lines
4.6 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.
---
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/ → обновлена документация» выполнено.