From 29e83341b5d2198a6c3fafb0779d28332f17e30c Mon Sep 17 00:00:00 2001 From: claude-bot Date: Sat, 6 Jun 2026 04:36:40 +0000 Subject: [PATCH] architect(ET): auto-commit from architect run_id=140 --- .../ADR-001-embed-findings-in-task-desc.md | 143 ++++++++++++++++++ docs/work-items/ORCH-046/10-tech-risks.md | 29 ++++ 2 files changed, 172 insertions(+) create mode 100644 docs/work-items/ORCH-046/06-adr/ADR-001-embed-findings-in-task-desc.md create mode 100644 docs/work-items/ORCH-046/10-tech-risks.md diff --git a/docs/work-items/ORCH-046/06-adr/ADR-001-embed-findings-in-task-desc.md b/docs/work-items/ORCH-046/06-adr/ADR-001-embed-findings-in-task-desc.md new file mode 100644 index 0000000..b6bca38 --- /dev/null +++ b/docs/work-items/ORCH-046/06-adr/ADR-001-embed-findings-in-task-desc.md @@ -0,0 +1,143 @@ +# ADR-001: дословный текст findings reviewer/tester встраивается в `task_desc` через отдельный graceful-парсер + +- **Статус:** Accepted +- **Дата:** 2026-06-06 +- **Задача:** ORCH-046 +- **Область:** ЯДРО `src/stage_engine.py` (rollback-ветки) + новый модуль `src/review_parse.py`. Общий прод-инстанс (orchestrator + enduro-trails), self-hosting. + +## Контекст + +При заворотах задачи на `development` (откат) `stage_engine` формирует `task_desc`, +который попадает в `.task-dev.md` запускаемого developer-агента. В двух ветках +`_handle_qg_failure_rollbacks` этот текст содержит **только ссылку на файл-артефакт**: + +- reviewer REQUEST_CHANGES (`src/stage_engine.py` ~стр. 419) → `…Fix findings in docs/work-items//12-review.md`; +- tester `check_tests_passed` FAIL (~стр. 455) → `…Fix failures described in docs/work-items//13-test-report.md`. + +Developer-агент получает инструкцию «иди читай файл»; ключевые претензии (P0/P1 +ревьюера, причина падения тестера) теряются — агент повторяет ту же ошибку, и +задача заворачивается снова. Это «испорченный телефон»: расход retry-бюджета +(`MAX_DEVELOPER_RETRIES = 3`), токенов и простой конвейера (для всех проектов +общего инстанса). + +Ограничение из BRD/ТЗ (вариант A, выбран Owner): минимальная, низкорисковая +правка ядра. Любая регрессия в формировании `task_desc` или (тем более) +исключение в `advance_stage` останавливает конвейер ВСЕХ проектов — следовательно +извлечение текста обязано быть полностью graceful. + +## Решение + +Встраивать **дословный текст ключевых замечаний** в `task_desc` при заворотах, +сохраняя ссылку на полный файл как дополнительный контекст. Извлечение вынести в +отдельный defensive-модуль, чтобы изолировать blast radius от ядра. + +1. **Новый модуль `src/review_parse.py`** с двумя чистыми функциями чтения с диска: + - `extract_review_findings(path: str) -> str` — дословные пункты P0/P1 из секции + `## Findings` файла `12-review.md`; + - `extract_test_failures(path: str) -> str` — релевантный фрагмент тела + `13-test-report.md` (приоритет: `## Вывод pytest` → FAIL-строки `## Результаты` + → `## Итог`). + - **Контракт «never raise»** (как `src/frontmatter.py` и + `src/qg/checks.py::_parse_tests_verdict`): любая ошибка — нет файла, IOError, + кривой markdown/YAML, нет секции — возвращает `""`. Логирование на + `logger.debug` (`logging.getLogger("orchestrator.review_parse")`). Никаких + сетевых вызовов. + - Результат усекается модульными лимитами `MAX_FINDINGS_CHARS`, + `MAX_FAILURES_CHARS` (≈2000) с маркером `…(truncated)`; полный контекст всегда + остаётся в файле. + +2. **Две ветки `_handle_qg_failure_rollbacks` в `src/stage_engine.py`** строят путь + через `get_worktree_path(repo, branch)` (как `_check_review_approved_by_branch`), + вызывают соответствующий парсер и: + - если результат непустой — встраивают findings/фрагмент **дословно** под + подзаголовком + оставляют ссылку как «полный контекст»; + - если результат пустой — `task_desc` остаётся **как сейчас** (graceful fallback + на ссылку-строку); + - tester-ветка дополнительно всегда включает `reason` из гейта (он уже доступен). + +3. **Изоляция ядра.** Меняется ТОЛЬКО содержимое строки `task_desc`, передаваемой в + `enqueue_job`. Последовательность отката (`update_task_stage` → + `notify_stage_change` → `plane_notify_stage` → [`set_issue_in_progress` для + тестера] → проверка `_developer_retry_count` < `MAX_DEVELOPER_RETRIES` → + `enqueue_job`/`send_telegram`), значения `AdvanceResult` (`rolled_back_to`, + `enqueued_agent`, `enqueued_job_id`, `alerted`) и Plane-комментарии — без + изменений. + +### Почему отдельный модуль, а не inline в `stage_engine` + +- Тестируемость: парсер покрывается юнит-тестами `tests/test_review_parse.py` + изолированно от тяжёлого `advance_stage`. +- Blast radius: вся парсинг-логика (и её исключения) физически отделена от + hot-path ядра; ядро только подставляет строку и делает try-around-граничный + fallback. +- Согласованность с уже принятым паттерном defensive-парсеров + (`frontmatter.py`). + +### Почему НЕ переиспользуется `frontmatter.read_frontmatter_value` + +Тот хелпер читает одиночное значение из YAML-frontmatter. Здесь нужно извлекать +**тело markdown** (подсекции `## Findings`/`### P0`, фрагменты `## Вывод pytest`), +а не frontmatter-ключ. Это другая задача парсинга; общий контракт «never raise» +повторяется намеренно (как уже зафиксировано в ORCH-016/ADR-001 §5 — слияние +парсеров отдельной задачей). + +### Почему per-work-item ADR, а не глобальный + +Изменение НЕ добавляет гейт/стадию/компонент и НЕ меняет топологию или реестр +`QG_CHECKS` — это обогащение содержимого `task_desc` в существующих rollback-ветках +плюс вспомогательный модуль. По прецеденту ORCH-047/ADR-001 такого класса правки +фиксируются per-work-item ADR. Глобальный `docs/architecture/adr/` не требуется. + +### Альтернативы (отклонены) + +- **Inline-парсинг прямо в `stage_engine`** — отклонено: раздувает ядро, хуже + тестируется, исключения ближе к hot-path. +- **Менять промпты reviewer/tester, чтобы они сами клали суть в `task_desc`** — + отклонено: `task_desc` формирует ядро, а не агент; зависит от дисциплины двух + агентов вместо детерминированного кода; шире поверхность регрессии. +- **Передавать весь файл целиком в `task_desc`** — отклонено: раздувает промпт + developer-агента и стоимость токенов; теряется фокус на must-fix. Усечение по + P0/P1 + лимит решает проблему «испорченного телефона» дешевле. + +## Последствия + +- **Плюс:** developer-агент видит суть претензий (P0/P1, причина FAIL) сразу в + `.task-dev.md`; меньше повторных заворотов, экономия retry-бюджета и токенов на + всех проектах общего инстанса. +- **Плюс:** при битом/отсутствующем артефакте поведение не хуже текущего (ссылка + сохраняется); ссылка на полный файл присутствует всегда (AC-3). +- **Плюс:** изменение аддитивное — публичные сигнатуры (`advance_stage`, `_run_qg`, + `check_*`), реестр `QG_CHECKS`, webhook-пути и `build_status_comment` не + затрагиваются; снапшот `test_qg_registry_snapshot` остаётся зелёным (AC-7). +- **Минус/ограничение:** парсинг тела markdown чувствительнее к формату артефактов, + чем чтение одного frontmatter-ключа. Митигировано: распознавание P0/P1 устойчиво + к регистру/тире; при несовпадении формата — пустой результат и fallback на + ссылку (никогда не исключение). +- **Минус:** усечение лимитом может обрезать длинные findings — приемлемо, полный + контекст остаётся в файле, ссылка сохранена. +- **Self-hosting риск:** правка ядра в общем прод-контейнере. Обязателен прогон + `deploy-staging` (8501) перед прод-деплоем; прод-контейнер `orchestrator` (8500) + не перезапускать в рамках разработки/тестинга. Граничный риск — исключение из + парсера в `advance_stage`; закрыт контрактом «never raise» + юнит-кейсами на + битый/пустой/отсутствующий ввод (AC-4, AC-5). + +## Влияние на документацию (golden source) + +В PR разработки (вместе с кодом) обновить: +- `docs/architecture/README.md` — раздел **Stage Engine** / **Откаты**: упомянуть, + что `task_desc` при заворотах reviewer/tester несёт дословные findings + ссылку, + и новый модуль `src/review_parse.py` (defensive, never-raise). +- `CHANGELOG.md` — запись ORCH-046. +- `docs/architecture/internals.md` — по усмотрению reviewer, если детализируется + поток отката. + +## Связи + +- BRD/ТЗ/AC: `docs/work-items/ORCH-046/01-brd.md`, `02-trz.md`, `03-acceptance-criteria.md`. +- Образцы паттерна «never raise»: `src/frontmatter.py`, + `src/qg/checks.py::_parse_tests_verdict`. +- Каноны артефактов: `.openclaw/agents/reviewer.md` (`12-review.md` `## Findings`), + `.openclaw/agents/tester.md` (`13-test-report.md` `result:` + тело). +- Прецедент per-work-item ADR на правку парсинга: ORCH-047/ADR-001. +- Технические риски: `docs/work-items/ORCH-046/10-tech-risks.md`. +- Staging-страховка: `docs/architecture/adr/adr-0003-staging-gate.md`. diff --git a/docs/work-items/ORCH-046/10-tech-risks.md b/docs/work-items/ORCH-046/10-tech-risks.md new file mode 100644 index 0000000..a232e08 --- /dev/null +++ b/docs/work-items/ORCH-046/10-tech-risks.md @@ -0,0 +1,29 @@ +# Технические риски — ORCH-046 + +Work Item ID: ORCH-046 +Stage: architecture +Author: architect +Date: 2026-06-06 + +Связано: `06-adr/ADR-001-embed-findings-in-task-desc.md`. + +| # | Риск | Вероятн. | Влияние | Митигация | Контроль (AC/тест) | +|---|------|----------|---------|-----------|--------------------| +| R-1 | Исключение из парсера всплывает в `advance_stage` → встаёт конвейер ВСЕХ проектов (self-hosting, общий инстанс) | Низк. | **Критич.** | Контракт «never raise» в `review_parse.py`; вызов в `stage_engine` обёрнут так, что пустой результат → fallback на прежнюю ссылку-строку | AC-4; юнит-кейсы «нет файла / битый YAML / пустой / только P3» в `tests/test_review_parse.py` | +| R-2 | Регрессия в последовательности отката или полях `AdvanceResult` (меняется не только `task_desc`) | Низк. | Высок. | Жёсткий инвариант ТЗ §3.3: трогать ТОЛЬКО строку `task_desc`; порядок вызовов и условия retry неизменны | AC-6; существующие `tests/test_stage_engine.py` (rollback/retry) зелёные | +| R-3 | Парсер чувствителен к формату артефактов: дрейф `12-review.md`/`13-test-report.md` → пустой результат | Сред. | Низк. | Распознавание P0/P1 устойчиво к регистру/тире; при несовпадении → `""` + fallback на ссылку (деградация, не отказ) | AC-1/AC-2/AC-4 | +| R-4 | Раздувание `task_desc` длинными findings → рост стоимости/потеря фокуса developer-агента | Сред. | Низк. | Лимиты `MAX_FINDINGS_CHARS`/`MAX_FAILURES_CHARS` (~2000) + маркер `…(truncated)`; only P0/P1 (P2/P3 отброшены) | AC-1; проверка усечения в юнит-тестах | +| R-5 | Случайный выход за out-of-scope (правка `check_*`, `QG_CHECKS`, сигнатур, webhooks, `build_status_comment`) | Низк. | Сред. | Явный out-of-scope в ТЗ §4/§6; ревью diff | AC-7; `test_qg_registry_snapshot` зелёный | +| R-6 | Прод-деплой self без страховки staging | Низк. | **Критич.** | Обязательная стадия `deploy-staging` (8501); прод `orchestrator` (8500) не рестартить в разработке/тестинге | adr-0003; стадийный гейт `check_staging_status` | +| R-7 | Дублирование defensive-парсинга (3-й модуль рядом с `frontmatter.py` и `_parse_tests_verdict`) → техдолг | Сред. | Низк. | Осознанно принято (как ORCH-016/ADR-001 §5): малый blast radius важнее DRY; слияние парсеров — отдельная follow-up задача | — (техдолг, не блокер) | + +## Заметки + +- **Граничный try в ядре.** Даже при контракте «never raise» в `review_parse`, + вызов в `stage_engine` следует считать недоверенным: подстановка результата в + `task_desc` не должна зависеть от внутренней корректности парсера. Fallback на + ссылку-строку обязателен и при пустом результате, и при любой неожиданности. +- **Эскалация не требуется.** Изменение укладывается в принципы (минимум + зависимостей, raw-парсинг без новых либ, без новых компонентов/стадий/QG). + Лейбл `arch:major-change` НЕ ставится; возврат в Анализ не требуется — ТЗ + реализуемо без нарушения принципов.