From 627a5359c363ec6579e52b6906519b271c0a026e Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 01:14:10 +0000 Subject: [PATCH] docs: review document for #399 Issue: #399 User-Visible: no --- docs/reviews/SPEC-REVIEW-399-r1.md | 73 ++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-399-r1.md diff --git a/docs/reviews/SPEC-REVIEW-399-r1.md b/docs/reviews/SPEC-REVIEW-399-r1.md new file mode 100644 index 00000000..d6425269 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-399-r1.md @@ -0,0 +1,73 @@ +# SPEC-REVIEW-399-r1 + +- Issue: https://github.com/Matysh/houseplan-card/issues/399 +- ТЗ: `docs/specs/399-backend-gate-honesty.md` +- Материал: SHA `a1d7e0da55322ff4309f139e22bc9a165b780f58` (ветка `issue/399-backend-gate-honesty`, единственный коммит поверх `dev`) +- Заход: r1 (первый), блокирующих циклов израсходовано 0/4 +- Трек: полный (issue не помечен `small`/`trivial`; спецификация сама называет причину — три несвязанные поверхности, что нарушает критерий «одна поверхность» §5) +- Вердикт: **жёлтый** + +## Скоуп + +Три независимые правки бэкенд-гейта, все — класс B (`test/**`, `.github/workflows/**`, `pyproject.toml`, `tests_backend/requirements.txt`), продуктовый код (`src/**`, `custom_components/**/*.py`) не затронут: + +1. пин `home-assistant-frontend` в `tests_backend/requirements.txt` не выведен из констрейнтов закреплённого `homeassistant`, а назначен вручную; +2. `[tool.ruff] include` в `pyproject.toml` шире, чем реально линтуемое в CI дерево; +3. `test/validate-workflow.test.mjs:216` пропускает файл без проверки, если в нём не нашлось ни имени пакета, ни пути к файлу пинов — то есть регресс к неверсионированной установке пройдёт гейт молча. + +## Как проверялось + +- прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (полностью, включая §2.9/§2.10 про делту — не применимо: это r1, предыдущего раунда нет); +- прочитано тело issue #399 (комментариев нет); +- сверены все три технические претензии ТЗ с текущим состоянием репозитория построчно: + - `tests_backend/requirements.txt` — подтверждено: `home-assistant-frontend==20260826.1` (строка 29 в файле) соседствует с `homeassistant==2026.8.3`, комментарий рядом объясняет только выбор `phcc`/`homeassistant`, но не `home-assistant-frontend`; + - `pyproject.toml:5` — подтверждено: `include` содержит ровно три дерева, как написано в ТЗ; + - `.github/workflows/validate.yml:787-788` — подтверждено: шаг «Линт бэкенда» гоняет только `custom_components/houseplan`; + - `test/validate-workflow.test.mjs:210-222` — подтверждено дословно, включая номер строки 216 и точный текст условия `continue`; подтверждено также, что цикл проверки идёт по жёстко заданному списку `['validate.yml', 'mutation-gate.yml']`, а не по каталогу `.github/workflows/*.yml`; + - `git grep "pip install" .github/workflows/*.yml` — только `validate.yml` и `mutation-gate.yml` ставят зависимости через pip; остальные пять workflow (`announce`, `docs-screenshots`, `performance`, `process`, `publish-prerelease`, `release-zip`, `release`) не участвуют вовсе — сегодня список из двух файлов исчерпывающий; + - история `#392` (`a3dcee52`, `eb5aa2a0`, `56986748`) реальна и соответствует контексту, на который ссылается ТЗ; + - формат мутантов (`backend-pins-check-opts-out`, `lint-scope-drifts`) сверен с существующими записями `scripts/mutation-gate.mjs` — совпадает по структуре (`id`, kebab-case), не выдумка; + - сверен прецедент `docs/specs/398-sysmodules-guard-scope.md` — тот же формат «Сценарий: разработчик / Пользователь продукта — ничего» уже принят для инфраструктурных гейт-задач этого типа (прошёл ревью ТЗ ранее), так что в #399 это не дефект. +- внешний факт «`package_constraints.txt` тега `2026.8.3` требует `home-assistant-frontend==20260729.7`» (сердце находки M3) независимо перепроверить не удалось — в этой среде нет доступа к сети (`WebFetch` заблокирован отсутствием разрешения). Это ограничение разбора, а не довод против ТЗ: AC1 обязывает доказательство без сети (снимок в репозитории + комментарий с источником), так что сам контракт не зависит от того, смог ли ревьюер ТЗ сходить в интернет — сверка достоверности исходного числа ляжет на код-ревью, где оно будет видно построчно. + +## Находки + +### Medium (в скоупе) — AC5 не фиксирует, какой набор файлов должен быть «отказом», а какой — «явно допустимым пропуском» + +**Файл**: `docs/specs/399-backend-gate-honesty.md`, AC5 (строки 125-128) и контракт п.(3) (строки 81-83). + +**В чём разрыв**: контракт п.(3) сформулирован широко — «если workflow ставит зависимости бэкенда, он обязан делать это из файла пинов; если не ставит вовсе — это тоже утверждение, которое надо доказать, а не предположить». Это читается как требование накрыть **все** файлы `.github/workflows/*.yml`, а не только два, которые проверка сегодня перебирает (`for (const file of ['validate.yml', 'mutation-gate.yml'])` — жёстко заданный массив, не обход каталога). + +AC5 при этом требует лишь «перечисление файлов, которые проверка обходит, зафиксировано в самом тесте» — и это выполнимо двумя существенно разными способами: + +1. **слабая версия**: оставить перебор тем же жёстко заданным массивом из двух файлов, внутри него заменить implicit `continue` на explicit разрешённый список причин пропуска. Формально удовлетворяет AC4 (синтетический workflow без обеих зацепок краснеет) и AC5 (список пропускаемых случаев есть в тесте) — но семь остальных workflow-файлов (`announce.yml`, `docs-screenshots.yml`, `performance.yml`, `process.yml`, `publish-prerelease.yml`, `release-zip.yml`, `release.yml`) как не проверялись, так и не проверяются, и, что важнее, **новый** workflow с неверсионированной установкой (например, будущий бэкенд-джоб не в `validate.yml`) вообще никогда не попадёт в цикл — гейт не заметит его так же тихо, как заметил бы файл вне списка ещё до этой правки; +2. **сильная версия**: перебор строится по фактическому листингу `.github/workflows/*.yml`, и для каждого файла тест либо требует пины, либо явно и поимённо объявляет его исключённым (обоснованно — «не устанавливает питон-зависимости»). Это единственная версия, которая закрывает контракт п.(3) буквально, и единственная, которая не оставляет тот же класс слепого пятна, который и стал поводом для находки Low «в» в аудите. + +Обе версии проходят AC4/AC5 как написано. Ревью ТЗ обязано различать такие пары («однозначность каждого AC» — задание reviewer'а), а здесь автор реализации может на совершенно законных основаниях выбрать первую и заявить AC выполненными, хотя цель issue («отсутствие обеих зацепок — это отказ, а не пропуск», дословно из тела issue) для будущих файлов не достигается. + +**Почему не High**: это не свидетельство того, что задача не может быть выполнена — реализация в любом прочтении будет проходить свои тесты и не ломает ничего рабочего; вопрос ровно в том, какой из двух контрактов фиксируется. Технический вопрос («сканировать каталог или тот же список из двух») — не продуктовый, значит не выносится владельцу (§7.1); его решает автор при реализации, но AC должен явно называть, какой из двух исходов — и ссылаться на п.(3) как на прямое указание. Это Medium в скоупе текущей задачи (правит формулировку AC5, не открывает новый surface) — правится без нового issue (#202). + +**Как закрыть**: одна фраза в AC5 или отдельным пунктом контракта — тест перебирает **все** файлы `.github/workflows/*.yml` (а не фиксированный список из двух), и для каждого, что не входит в проверяемый набор, в самом тесте лежит явное объяснение почему (например, «не запускает Python»), а не молчаливое умолчание через отсутствие подстроки. Дёшево: строчка текста, реализацию не расширяет — `readdirSync` вместо литерала массива на один порядок сложнее не становится. + +## Что проверено и корректно + +- Обязательные разделы §7.1 присутствуют все: сценарий, что человек увидит, проблема (по пунктам), скоуп/не-скоуп, контракт (по пунктам), UX (н/п — обоснованно), модель данных/миграция (н/п — обоснованно), i18n (нет новых строк — обоснованно), AC1–AC6 с указанием способа доказательства, план автотестов, риски, откат, release-артефакты. +- Продуктовое обрамление (`Сценарий` / `Что человек увидит`) совпадает по формату с уже принятым прецедентом (#398): «Пользователь продукта — ничего, разработчик — …» — корректно для чисто инфраструктурной задачи класса B, доменных персон `SCOPE.md` это не касается по существу вопроса. +- Классификация трека (полный, а не `small`) обоснована прямо в шапке ТЗ явным нарушенным критерием («три несвязанные поверхности») — соответствует требованию §2.2/§5 называть нарушенный критерий, а не полагаться на ощущение. +- Все технические утверждения о текущем состоянии кода (номера строк, содержимое `include`, содержимое workflow, логика `continue`) проверены построчным чтением файлов и совпадают дословно — никаких выданных за факт догадок не найдено. +- AC1–AC6 пронумерованы, для каждого указан способ доказательства (unit-тест либо явное «проверяется ревьюером» для AC3, что соответствует допустимому в DoR методу «ревью кода»). +- Риски названы по существу (объём ruff-долга при исходе (2), сетевая зависимость версии фронтенда, «ложное чувство завершённости») — не общие слова, а конкретные технические ловушки со смягчением. +- Откат — тривиальный revert, обосновано отсутствием продуктового кода в диффе. +- Не-скоуп корректно исключает соседние решения (#42 сужение линта как таковое, версии `homeassistant`/`phcc`, гвард `sys.modules` из #398) — предотвращает попутные правки за пределами трёх названных находок. +- «Одно число — один источник»: неприменимо, пользовательских чисел в диффе нет (весь дифф — CI-конфигурация и тесты). + +## Чего не проверял + +- Не проверял по сети фактическое содержимое `package_constraints.txt` тега `2026.8.3` репозитория `home-assistant/core` (претензия M3 из аудита) — инструмент веб-доступа в этой среде недоступен (запрос разрешения не подтверждён). Не блокирует вердикт: AC1 требует доказательства без сети, реализация и код-ревью проверят число построчно с открытым источником в комментарии. +- Не гонял `npm test` / `npx tsc --noEmit` / `npm run build` — это ревью ТЗ (документ, не код), гейты не относятся к этому этапу; они будут прогнаны на код-ревью после реализации. +- Не проверял сам аудит-документ `AUDIT-2026-08-31-v1700beta1.md` — файл не найден в репозитории (видимо, живёт вне репозитория, как и часть ревью до 1.62); тело issue содержит достаточный самостоятельный контекст, так что отсутствие исходника не мешает оценке ТЗ. +- Не проверял объём накопленного ruff-долга («порядка полусотни» находок при исходе (2)) — это оценка автора ТЗ для риска, не факт, требующий проверки на этом этапе; будет виден в CI при выборе исхода (2) на код-ревью. + +## Вердикт + +Жёлтый: единственная блокирующая-по-скоупу находка — Medium, без High. Автор уточняет формулировку AC5 (какой набор workflow-файлов покрывает проверка и почему), проходит повторный цикл ревью ТЗ (лимит лёгкого трека здесь неприменим — трек полный, лимит 4, израсходовано 0/4).