Files
orchestrator/docs/work-items/ORCH-119/12-review.md
claude-bot 1b09d1a525
All checks were successful
CI / test (push) Successful in 1m27s
CI / test (pull_request) Successful in 1m19s
reviewer(ET): auto-commit from reviewer run_id=790
2026-06-17 17:08:26 +03:00

13 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-119 review reviewer approved 2026-06-17 claude-opus-4-8 review ORCH-119 3

Review ORCH-119 — source-backed 00-business-request.md вместо хардкода TBD

Summary

Узко-скоупленный багфикс (Bug-трек). Раздел «Description» файла 00-business-request.md теперь несёт фактический текст запроса из Plane-issue вместо литерала TBD — на обоих путях создания: прямой (путь A) и отложенный срез ветки (путь B, ORCH-088, доминирует на self-hosting orchestrator). Реализация точно зеркалирует прецедент tasks.title, как и предписал ADR-001: аддитивная колонка tasks.description через _ensure_column, записываемая внутри того же атомарного INSERT create_task_atomic (race-safe vs ORCH-053), читаемая в _spawn_materialize_deferred_branch на момент claim (без сети в горячем пути, NFR-4). Тело рендерится чистым unit-тестируемым хелпером _render_business_request с fail-safe fallback-маркером.

Это независимый повторный review (v3). Все 4 оси перепроверены лично против origin/main (f50f61c, = merge-base): скоуп src/ — ровно 3 файла, предписанных ТЗ §2 (webhooks/plane.py, agents/launcher.py, db.py); тесты — test_orch119_business_request.py (новый, +263), test_serial_gate_branch.py (+spy-арг); CHANGELOG.md + work-item доки. Полный прогон pytest tests/ -q — зелёный (2215 passed, изолированно от конкурентных сессий).

Соответствие ТЗ / AC (ось 1) — PASS

AC / FR Статус Подтверждение
AC-1 / FR-1 — реальное описание вместо TBD, header/Work Item ID целы _render_business_request рендерит тело из description; TC-01 (genuine red→green).
AC-2 / FR-2 — прямой путь A start_pipeline передаёт description в _create_initial_docs (plane.py:736); TC-04 ассертит отсутствие ## Description\n\nTBD\n.
AC-3 / FR-3 — отложенный путь B / durable-персист персист в атомарном INSERT create_task_atomic (plane.py:665, db.py); чтение SELECT …, description в _spawn → 5-й арг _materialize_deferred_branch (_br_row[3]); TC-03.
AC-4 / FR-4 — fail-safe fallback _BUSINESS_REQUEST_FALLBACK + try/except → never-raise; TC-02 (""/" "/"\n\t"/None).
AC-5 — гейты/схема не тронуты коммит не касается stages.py/qg/checks.py; единственная схема — аддитивная tasks.description, базовый CREATE TABLE tasks цел; TC-07.
AC-6 — идемпотентность / BC Gitea 422return (тело не перезаписывается, plane.py:1006); _ensure_column no-op; TC-05/TC-06.
AC-7 — регресс red→green + полный прогон зелёный На origin/main _render_business_request отсутствует (grep=0) и TBD хардкоден (plane.py:945) → TC-01 настоящий red; green после. Полный прогон 2215 passed (изолированно).
AC-8 — сопровождение CHANGELOG.md содержит запись ORCH-119.

description определён в scope start_pipeline (plane.py:538) и обогащается из Plane API (plane.py:560568) до обоих call-site (:665, :736) — NameError исключён; персистится тот же enriched-текст, что уходит analyst-job (:751) — консистентно.

Соответствие ADR (ось 2) — PASS

  • D1 (аддитивная колонка в атомарном INSERT) — реализовано буквально: _ensure_column(conn,"tasks","description","TEXT") рядом с tasks.title; запись встроена в существующий INSERT create_task_atomic, не отдельным UPDATE → нет race-окна vs ORCH-053.
  • D2 (чистый рендер-хелпер + проброс на обоих путях) — _render_business_request чистый/network-free; проброс A и B.
  • D3 (fail-safe fallback + идемпотентность) — маркер + never-raise + 422-no-op.
  • D4 (инварианты) — ORCH-088 момент/условие отложенного среза не меняются (дополняется только источник данных), ORCH-053 семантика (row, created) цела, гейты байт-в-байт, без kill-switch (обосновано — фикс дефекта).
  • Трассировка (TRACEABILITY): правки блоков с маркерами ORCH-088 (_materialize_deferred_branch) и ORCH-053 (create_task_atomic) сверены с их 06-adr — аддитивные, все новые параметры с дефолтами; зафиксированные инварианты не сломаны; комментарии кода корректно ссылаются на оба ADR.
  • Сквозной (global) ADR не требуется — локальное аддитивное решение по прецеденту.

Качество кода (ось 3) — PASS

  • Чистый, network-free, unit-тестируемый хелпер; docstrings на всех новых/изменённых функциях.
  • never-raise контракт соблюдён (try/except в рендере; создание задачи не падает).
  • BC: все добавленные параметры аддитивны с дефолтами; единственный прочий потребитель сигнатуры (_docs_spy в test_serial_gate_branch.py) синхронно обновлён под аддитивный description=None.
  • Багфикс-регресс (ORCH-019 / BR-4) — фиксатор присутствует: TC-01 настоящий red до фикса (_render_business_request отсутствовал + _create_initial_docs хардкодил TBD на main), green после; усилен TC-04 (бьёт реальный баг-путь _create_initial_docs).
  • Тесты содержательные: 7 TC покрывают регресс, fallback (4 параметра), оба пути, schema-BC, идемпотентность 422, multiline/спецсимволы verbatim, анти-регресс гейтов.
  • Личная проверка: test_orch119_business_request.py + test_serial_gate_branch.py — 13 passed; test_stage_engine.py — 52 passed (изолированно); полный прогон — 2215 passed.

Документация (ось 4) — PASS

Изменён src/ → проверка обязательна. Документация обновлена в том же PR:

  • CHANGELOG.md — подробная запись ORCH-119 (AC-8).
  • ADR-001 (06-adr/ADR-001-source-backed-business-request-doc.md) + 08-data-requirements.md (документирует схему tasks.description) — присутствуют, исчерпывающи.
  • API — изменений нет (ТЗ §4) → таблица API в docs/architecture/README.md обновления не требует.
  • Стадии/QG — не тронуты → docs/architecture/README.md / internals.md обновления не требуют.
  • Витрина docs/overview/ (ORCH-011): tech-agents.md по-прежнему верно указывает 00-business-request.md как вход аналитика; tech-data-model.md описывает tasks концептуально (без перечня колонок) — добавление колонки его не устаревает; смена тела артефакта (внутренняя деталь генерации) описанных стадий/гейтов/агентов/способностей не меняет. Обновление не требуется.
  • README.md «Известные ограничения» (ORCH-079): проверено — TBD-баг 00-business-request.md не числится среди 3 открытых ограничений (Telegram-48h / intra-repo deps / batch-автоном) — закрывать/снимать нечего, обновление README не требуется.
  • Центрального schema-дока колонок tasks нет — конвенция проекта: per-task скалярные колонки документируются ADR задачи + data-requirements + CHANGELOG (так же оформлены tasks.title / tasks.track / tasks.cancelled_at). ORCH-119 ей следует.

Итог по документации: при изменении src/ документация обновлена в том же PR — P0 «src/ изменён, документация не обновлена» не наступает.

Findings

P0 — Blocker

  • Нет.

P1 — Must fix

  • Нет.

P2 — Should fix

  • Нет.

P3 — Nice-to-have (не блокирует)

  • CHANGELOG.md over-claim после rebase. Хвост записи ORCH-119 описывает «Дополнительно в том же PR починена тест-гермеичность tests/test_orch123_staging_runner_exec.py…». Этот файл отсутствует в diff против origin/main — фикс (fresh_db пинит repos_dir) уже в main (проверено: git show origin/main:tests/test_orch123_staging_runner_exec.py уже содержит пин). Описывает изменение вне фактического скоупа PR (артефакт rebase). Безвредно (ядро ORCH-119 задокументировано точно), но для аккуратности — подрезать хвост. На корректность фикса не влияет.
  • tests/test_orch119_business_request.py мутирует os.environ (ORCH_DB_PATH/ ORCH_REPOS_DIR) на уровне модуля — добавляет к общей import-pollution-поверхности сьюта. Соответствует существующей конвенции многих тест-файлов проекта (не новый грех), на корректность не влияет.
  • _render_business_request: try/except Exception вокруг (description or "").strip().strip() на строке не бросает, ветка недостижима. Намеренный never-break «belt-and-suspenders» в стиле кодбейза; оставить как есть допустимо.

Документация

Статус: обновлена в том же PR — достаточно. CHANGELOG.md (AC-8) + ADR-001 + 08-data-requirements.md (схема tasks.description) — присутствуют и исчерпывающи. Витрина docs/overview/, обзорный README.md («Известные ограничения»), PIPELINE_DOCS.md/HANDOFF_PROTOCOL.md проверены явно — не устаревают (артефакт остаётся информационным, продюсер/владелец/гейт-статус не изменились; TBD-баг не числился среди открытых ограничений). Центрального schema-дока колонок tasks нет — аддитивная колонка документирована по конвенции (ADR + data-requirements + CHANGELOG, как tasks.title/track/cancelled_at). Необновлённой обязательной документации не найдено. (Единственный документ-нюанс — P3-over-claim в CHANGELOG.md про вне-скоупный test_orch123, не блокирует.)

Вердикт

APPROVED. Нет P0/P1/P2. Все AC/FR/ADR удовлетворены; реализация — точное зеркало одобренного прецедента tasks.title с нулевым касанием конвейерной машинерии и сохранёнными инвариантами ORCH-088/ORCH-053; обязательный регресс-тест-фиксатор присутствует (genuine red→green, проверено против origin/main); полный прогон зелёный (2215 passed). Документация синхронна. Готово к стадии testing.