diff --git a/docs/work-items/ORCH-099/12-review.md b/docs/work-items/ORCH-099/12-review.md new file mode 100644 index 0000000..a667241 --- /dev/null +++ b/docs/work-items/ORCH-099/12-review.md @@ -0,0 +1,86 @@ +--- +verdict: APPROVED +work_item: ORCH-099 +stage: review +author_agent: reviewer +status: approved +created_at: 2026-06-10 +model_used: claude-opus-4-8 +type: review +work_item_id: ORCH-099 +version: 1 +--- + +# Review ORCH-099 — FND/F1a: лёгкий read-only `GET /metrics` (сырьё для sidecar F1b) + +## Summary + +Реализация полностью соответствует ТЗ (`02-trz.md`), критериям приёмки (`03-acceptance-criteria.md`) +и архитектурному решению (`06-adr/ADR-001` + сквозной `adr-0030`). Добавлен аддитивный, строго +read-only, never-raise эндпоинт `GET /metrics` через leaf-модуль `src/metrics.py` (`build_metrics()`, +паттерн `serial_gate.snapshot()`) + тонкая обёртка в `src/main.py` + три read-only helper'а в +`src/db.py`. Конвейерные инварианты целы: `STAGE_TRANSITIONS` / `QG_CHECKS` / `check_*` / +machine-verdict ключи / схема БД — не тронуты (в диффе нет `src/stages.py`/`src/qg/`; упоминания этих +имён — только в документации/комментариях). Полный регресс `pytest tests/ -q` — **1482 passed**. +Документация обновлена в том же PR. Блокирующих findings нет. + +**Оси проверки:** + +1. **Соответствие ТЗ** — ✅. FR-1 (`stages`, фильтр терминалов `{done,cancelled}`), FR-2 (`queue`: + counts+`cancelled`, depth, retries, breaker, max_concurrency), FR-3 (`agents`-liveness: + agent/run_id/job_id/pid/runtime_s/model/effort + `cpu_ticks`), FR-4 (`cost`: running+aggregate), + FR-5 (конверт `schema_version`/`generated_at`/`clk_tck`), FR-6 (never-raise по разделам) — + реализованы. AC-1…AC-8 проверены по коду и тестам (TC-01…TC-11), все зелёные. +2. **Соответствие ADR** — ✅. D1–D8 реализованы как описано: D3 фильтр терминалов на потребителе + (helper-инвариант ORCH-053/086 не тронут); D5 dedicated `get_running_agents()` вместо расширения + hot-path `get_running_jobs()` (ORCH-065); D6 `runtime_s` от `jobs.started_at`; D8 kill-switch + `metrics_endpoint_enabled` (дефолт `True`, `200` с минимальным телом при `False`). Глобальный + инвариант терминального множества `{done,cancelled}` (adr-0026) соблюдён. `validation_alias` + `ORCH_METRICS_ENABLED` — обоснованное усиление D8 (документированное имя контракта реально + управляет флагом), покрыто `tests/test_config.py`. Нарушений глобальных ADR нет. +3. **Качество кода** — ✅. Все колонки БД (`agent_runs.{cost_usd,*_tokens}`, `jobs.{pid,run_id, + started_at,repo,attempts,transient_attempts,available_at}`) сверены — существуют. Парсинг + `/proc//stat` устойчив к пробелам/скобкам в `comm` (`rfind(") ")`, индексы 11/12 = поля + 14/15); `_read_cpu_ticks` never-raise per-pid. Docstrings на всех публичных функциях, тесты + содержательные (живой pid → реальный int, мёртвый/`None` → `null`, бросающий источник → дефолт). +4. **Документация** — ✅ (см. секцию ниже). + +## Findings + +### P0 — Blocker +- Нет. + +### P1 — Must fix +- Нет. + +### P2 — Should fix +- Нет. + +### P3 — Nice-to-have (не блокирует) +- [ ] `db.get_running_agents()` вызывается дважды на один запрос `/metrics` — в `_build_agents` и в + `_build_cost` (`src/metrics.py:176` и `:206`) — два идентичных SELECT'а. На типовом объёме + (running-jobs ≤ `max_concurrency`) — пренебрежимо, AC-3 не нарушен; при желании можно выбрать строки + один раз и переиспользовать. Косметика, исправление не требуется для приёмки. + +## Документация + +Обновлена в том же PR (правило «доки = golden source», AC-7) — проверено явно: +- **`docs/architecture/README.md`** — новый компонент «Metrics endpoint» в списке, полный раздел- + контракт «Сырьё-эндпоинт `/metrics` для sidecar» (конверт, разделы, политика `schema_version`, + гарантии read-only/never-raise, kill-switch) и строка в таблице API. Соответствует фактическому + ответу эндпоинта. +- **`CHANGELOG.md`** — запись `## [Unreleased]` с пометкой `ORCH-099` (D1–D8 + тесты + откат). +- **`docs/work-items/ORCH-099/06-adr/ADR-001-metrics-endpoint.md`** — детальное решение (D1–D8). +- **`docs/architecture/adr/adr-0030-metrics-endpoint.md`** — сквозной ADR (новый компонент + наблюдаемости + публичный контракт), зарегистрирован в `docs/architecture/adr/README.md` (индекс + + «текущий максимум — `0030`»). +- **`.env.example`** — задокументирован `ORCH_METRICS_ENABLED=true`. + +`src/` изменён → документация обновлена (golden source соблюдён). Эпик-обзорные доки `README.md` +«Известные ограничения» этой задачей не затрагиваются (новый компонент, не закрытие ограничения). + +## Регресс / проверки +- `pytest tests/ -q` → **1482 passed** (новые `tests/test_metrics.py` TC-01…TC-11 + `test_config.py` + ×2; регресс `/health`//status//queue зелёный, TC-10). +- Дифф `src/stages.py` / `src/qg/` — пуст; machine-verdict ключи и схема БД — байт-в-байт прежние. +- Read-only подтверждён тестом снимка БД до/после (TC-09); never-raise — TC-05/TC-07.