reviewer(ET): auto-commit from reviewer run_id=402
All checks were successful
CI / test (push) Successful in 24s
CI / test (pull_request) Successful in 30s

This commit is contained in:
2026-06-08 22:44:18 +03:00
parent 56bf30323d
commit 7e7bb836f0

View File

@@ -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_<agent>` через `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-состояние корректно по всем осям.