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

5.0 KiB
Raw Blame History

type, work_item_id, verdict, version
type work_item_id verdict version
review ORCH-081 APPROVED 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 -q1031 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.exampleORCH_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-состояние корректно по всем осям.