diff --git a/docs/work-items/ORCH-081/12-review.md b/docs/work-items/ORCH-081/12-review.md new file mode 100644 index 0000000..1daa279 --- /dev/null +++ b/docs/work-items/ORCH-081/12-review.md @@ -0,0 +1,57 @@ +--- +type: review +work_item_id: ORCH-081 +verdict: APPROVED +version: 1 +--- + +# Review ORCH-081 (ORCH-52h) — устойчивость резолва `--effort` к пустому env + developer→xhigh + +## Summary +Фикс конфигурационного бага: в проде `resolve_agent_effort()` возвращал `''` для всех 6 агентов (пустые `ORCH_AGENT_EFFORT_*=` перебивают class-default pydantic), `--effort` не доходил до Claude CLI. Решение — вариант C по ADR-001: непустой **per-role floor** уровня 4 в `resolve_agent_effort`, значение = декларированный class-default поля `agent_effort_` через `model_fields[...].default`. `developer` поднят `high→xhigh` в `config.py` (единый источник правды, floor подтягивается автоматически). + +Реализация полностью соответствует ТЗ и ADR; вся документация синхронизирована в том же бранче; `pytest -q` — **1031 passed**. + +## Соответствие ТЗ (FR-1…FR-5) +- **FR-1** per-role floor при пустом env → каждая роль получает свой канон (`_agent_effort_floor`, TC-02). ✓ +- **FR-2** приоритет резолва сохранён: явный env/override/default побеждают floor (TC-04: `test_explicit_env_beats_floor`, `test_default_beats_floor`, `test_project_override_beats_floor`). ✓ +- **FR-3** валидация не регрессирует: непустая опечатка (`turbo`) не доходит до floor → дропается в `''` (TC-03 `test_floor_does_not_mask_typo`). ✓ +- **FR-4** `agent_effort_developer = "xhigh"` в `config.py`; `ORCH_AGENT_EFFORT_DEVELOPER=xhigh` + правка комментария split в `.env.example`. ✓ +- **FR-5** `xhigh ∈ VALID_EFFORTS`; сборка флага `--effort xhigh `/`--effort medium ` подтверждена (TC-05/TC-06). ✓ + +## Соответствие ADR-001 +- Floor как **строго уровень 4** ниже default, в резолвере — ✓ (вариант C, не field_validator/не hardcoded map). +- Floor = **class-default поля** (`type(settings).model_fields[...].default`), который пустой env перебить не может — ✓. +- `_resolve_agent_attr` (общий с model-резолвом) **не тронут** — ✓. +- Floor применяется **ДО валидации и только при пустом резолве** — ✓. +- Unknown-agent деградирует на class-default `agent_effort_default` (`high`) — ✓ (`test_empty_env_unknown_agent_floor_is_default`). +- Никаких изменений API / схемы БД / QG / model-резолва / пути проброса в `_spawn` — ✓. + +## Качество кода и тестов +- Чистый leaf-helper, подробные docstrings, контракт never-raise соблюдён. +- Тесты содержательные, покрывают все AC/FR (канон-дефолты, floor per-role, не-маскирование опечатки, приоритет на 3 уровнях, `xhigh`-валидность, сборка флага + негативные кейсы). + +## Findings + +### P0 — Blocker +- (нет) + +### P1 — Must fix +- (нет) + +### P2 — Should fix +- (нет) + +### P3 — Nice-to-have +- `tests/test_resolve_agent_effort.py:218-219` — продублирована строка `assert "--fallback-model" not in flags` в `test_flags_absent_when_model_empty`. Безвредно, можно убрать при случае. + +## Документация +Изменён `src/` → документация обновлена в том же бранче (доку-гейт пройден): +- `docs/architecture/README.md` — таблица «Модель и эффорт по ролям»: developer = `xhigh`; добавлена ремарка про per-role floor / устойчивость к пустому env (AC-4). ✓ +- `.env.example` — `ORCH_AGENT_EFFORT_DEVELOPER=xhigh` + комментарий split/floor (AC-4). ✓ +- `CHANGELOG.md` — запись `fix:` с разбором корня/фикса. ✓ +- `docs/work-items/ORCH-081/06-adr/ADR-001-effort-resolution-floor.md` — присутствует (Accepted). ✓ + +## Примечание (вне scope ревью) +- AC-6 — операционная проверка в проде после деплоя, фиксируется в `14-deploy-log.md` на стадии deploy. К коду PR не относится. +- `git diff main...HEAD` показывает также код ORCH-074 (`is_valid_model`/`resolve_agent_model`) из-за устаревшего локального `main`; собственно изменения ORCH-081 — коммит `56bf303` (+ README обновлён в линии бранча). На ревью это не влияет: HEAD-состояние корректно по всем осям.