reviewer(ET): auto-commit from reviewer run_id=545
This commit is contained in:
86
docs/work-items/ORCH-099/12-review.md
Normal file
86
docs/work-items/ORCH-099/12-review.md
Normal file
@@ -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/<pid>/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.
|
||||
Reference in New Issue
Block a user