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

6.3 KiB
Raw Blame History

verdict, work_item, stage, author_agent, status, created_at, model_used, type, work_item_id, version
verdict work_item stage author_agent status created_at model_used type work_item_id version
APPROVED ORCH-099 review reviewer approved 2026-06-10 claude-opus-4-8 review ORCH-099 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/ -q1482 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. D1D8 реализованы как описано: 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, мёртвый/Nonenull, бросающий источник → дефолт).
  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 (D1D8 + тесты + откат).
  • docs/work-items/ORCH-099/06-adr/ADR-001-metrics-endpoint.md — детальное решение (D1D8).
  • 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/ -q1482 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.