From 6bf39ee9a7ecbc62dcc8b999af3c23b6d877c5a0 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 01:54:28 +0000 Subject: [PATCH] docs: review document for #399 Issue: #399 User-Visible: no --- docs/reviews/CODE-REVIEW-399-r2.md | 160 +++++++++++++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-399-r2.md diff --git a/docs/reviews/CODE-REVIEW-399-r2.md b/docs/reviews/CODE-REVIEW-399-r2.md new file mode 100644 index 00000000..aa28d103 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-399-r2.md @@ -0,0 +1,160 @@ +# CODE-REVIEW-399-r2 + +- Issue: https://github.com/Matysh/houseplan-card/issues/399 +- Этап: code (PROCESS.md §2.7) +- Заход: r2 · блокирующих циклов израсходовано 1/4 +- HEAD: `5c4d8cba9ebb2815dd45f91641d39621435a1144` (детач на + `origin/issue/399-backend-gate-honesty`) +- Материал: `git log --oneline origin/dev..HEAD`, + `git diff origin/dev...HEAD`; для этого раунда — дельта + `git diff 98028a30..HEAD` относительно SHA предыдущего код-ревью +- ТЗ на входе: `docs/specs/399-backend-gate-honesty.md`, ревизия 3, + принята зелёным вердиктом SPEC-REVIEW-399-r3 (без изменений с r1) + +## Дельта этого раунда + +Предыдущий код-ревью (CODE-REVIEW-399-r1, документ в `8b2a146c`) прошёл +на `98028a3093713122d6dbc1b08fc830b730e8a2a3` с вердиктом жёлтый, +High: 1. Автор ответил единственным коммитом: + +`5c4d8cba` — `test: prove AC5 by running the scanner, not the predicate (#399)` + +`git diff 98028a30..HEAD` (без документов ревью, которые сами по себе +не код): + +- `test/validate-workflow.test.mjs` — обход каталога вынесен в + экспортируемую функцию `pinViolations(directory)`; тест «AC5» + переписан: вместо вызова `installsPythonDeps` на строковых литералах + строится настоящий временный каталог из трёх файлов и вызывается + **та же** функция, что работает в проде. +- `scripts/mutation-gate.mjs` — мутант `workflow-scan-hardcodes-the-list` + меняет цель патча с прежней (`workflows = readdirSync(...)` → + список из одного имени) на новую (`files = readdirSync(...)` → + список из **двух настоящих** имён `['validate.yml', 'mutation-gate.yml']`). + +Больше ничего не менялось: `pyproject.toml`, `tests_backend/requirements.txt`, +`test/backend-pins.test.mjs`, `test/lint-scope.test.mjs` дельтой не +затронуты. Рёбейза не было (`git merge-base origin/dev HEAD` = tip `dev`, +проверено). Delta локальна и меньше по объёму, чем исходная задача — +полный разбор не требуется, разбираю только то, до чего дотягивается +изменение: AC5 и связанный с ним мутант. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **[High]** Тест, названный AC5, вызывал `installsPythonDeps` на строковых литералах внутри теста и не исполнял реальный обход каталога (`readdirSync(WORKFLOWS).filter(...)`). Воспроизведение: откат к списку из двух реальных имён + подложенный третий workflow с непинованной установкой → 10/10 зелёных. | Обход вынесен в `pinViolations(directory)` — функцию с каталогом как параметром. Тест AC5 (`test/validate-workflow.test.mjs:259`) строит временный каталог из трёх файлов (пинованный `validate.yml`, безобидный `docs.yml`, `zz-rogue.yml` с непинованной установкой) и вызывает именно эту функцию, а не предикат в изоляции. Утверждает на возвращённых `scanned` (3), `installers` (2) и точном списке `problems` для `zz-rogue.yml`. | Воспроизвёл ровно сценарий r1 повторно на текущем HEAD: `sed` вернул обход к `['validate.yml', 'mutation-gate.yml']` внутри `pinViolations` → `node --test test/validate-workflow.test.mjs` даёт **9/10**, падает именно тест «#399 AC5: тот же код ловит третий workflow в подставном каталоге» (`ENOENT` на втором захардкоженном имени, которого нет во временном каталоге) — не побочный assert, а прямое доказательство, что тест исполняет реальный путь. Дерево вернул (`git checkout -- test/validate-workflow.test.mjs`), `git status --short` пуст. | + +Мутант `workflow-scan-hardcodes-the-list` пересмотрен под замечание r1 +о том, что прежняя форма (список из одного имени) ловилась по +посторонней причине (`assert.ok(workflows.length >= 2, ...)`), а не по +существу. Новая форма подставляет список из **двух настоящих** имён — +неотличимую от корректного кода на сегодняшнем дереве, то есть более +вероятную форму регрессии. Прогнал штатным раннером: + +``` +node scripts/mutation-gate.mjs --id=workflow-scan-hardcodes-the-list +ok чистый прогон: node --test test/validate-workflow.test.mjs +ok workflow-scan-hardcodes-the-list: тест покраснел, как обязан +поймано 1 из 1 +``` + +Дерево после прогона чистое (`git status --short` пусто). + +## Как проверялось (этот раунд) + +Зелёного Validate на `5c4d8cba` нет — гейты прогнаны вручную. Диапазон +изменений — конфигурация CI/тестов, `src/**` не тронут ни в дельте, ни +в остальном диапазоне (см. r1). + +- `npx tsc --noEmit` — чисто, exit 0. +- `npm test` — **1667/1667 pass, 1 skipped**, совпадает с заявленным + автором числом и с результатом r1 (delta не должна была изменить + общее число тестов — так и есть: -1 старый AC5-тест, +1 новый, чистый + ноль). +- `npm run build` — собрался (`tsc --noEmit && rollup -c`, ~16s). + Сверка трёх копий бандла не делалась: дельта не трогает `src/**`, + контент бандла не мог измениться. +- `node scripts/check-docs.mjs` — не прогонялся: дельта не трогает + `src/**` (условие запуска по инструкции не выполнено, как и в r1). +- Геометрические инварианты, `golden:verify`, browser-смоки, + `pytest tests_backend`, `ruff` — не по необходимости и не по + возможности среды: дельта не касается геометрии, рендера, продуктового + Python-кода и видимого поведения; причины идентичны r1 (нет + установленных `homeassistant`/`ruff` в песочнице, проверено `pip show`). +- Воспроизведение регрессии, которую AC5 обязан ловить (см. таблицу + выше) — прогнано лично на этом HEAD, не унаследовано с чужих слов. +- Мутант `workflow-scan-hardcodes-the-list` прогнан штатным раннером на + новой форме патча (см. выше). + +## Что проверено и признано корректным + +- **AC5** — теперь доказан исполнением: `pinViolations` — единственная + точка обхода каталога, используемая и продовым тестом на реальном + `.github/workflows/`, и AC5-тестом на синтетическом каталоге. Оба пути + проходят через один и тот же код, различие — только во входных данных. + Регрессия из r1 (список из двух имён) теперь красит именно AC5-тест, + без постороннего assert. Мутант заточен под ту же форму регрессии и + ловится. +- Синтетический каталог AC5-теста корректно воспроизводит оба вида + нарушения контракта одним файлом (`zz-rogue.yml` одновременно не + ставит из файла пинов и не версионирован) — соответствует двум + проверкам внутри `pinViolations`, порядок утверждений в + `assert.deepEqual` детерминирован (оба сообщения относятся к одному + файлу, порядок не зависит от порядка обхода каталога `readdirSync`). + Учёл это отдельно, так как порядок `readdirSync` не гарантирован + POSIX — но здесь он не влияет на исход теста, что делает тест + устойчивым к этой недетерминированности, а не случайно совпадающим. + +## Унаследовано из r1 (без повторной проверки) + +Дельта этого раунда не касается перечисленного — принимаю выводы +CODE-REVIEW-399-r1 (`8b2a146c`, HEAD того раунда `98028a3093713122d6dbc1b08fc830b730e8a2a3`) +как есть: + +- **AC1** (пин фронтенда из констрейнтов HA) — подтверждён в r1 сетевым + источником (`gh api` к `home-assistant/core@2026.8.3`), файлы + `tests_backend/requirements.txt`, `test/backend-pins.test.mjs` дельтой + не тронуты. +- **AC2/AC3** (согласование `[tool.ruff] include` и шага линта в + `validate.yml`, `test/lint-scope.test.mjs`) — сверены в r1 построчно, + файлы дельтой не тронуты. +- **AC4** (позитивный признак `installsPythonDeps` вместо пропуска по + отсутствию подстроки) — сама функция `installsPythonDeps` не менялась + в этой дельте (менялась только функция, которая её использует — + `pinViolations`, и структура теста). +- **AC6** (гейты не деградировали) — подтверждён в r1 числом `npm test`; + в этом раунде число совпало снова, деградации нет. +- Мутанты `lint-scope-drifts`, `frontend-pin-drifts-from-ha` — прогнаны + и пойманы в r1, дельта их не затрагивает. +- Трейлеры `Issue: #399` / `User-Visible: no` на всех девяти коммитах + диапазона — перепроверены заново в этом раунде (см. «Как + проверялось»), не только унаследованы: результат идентичен r1. +- Low-наблюдение r1 про `pip3 install` / `python3 -m pip install`, + не распознаваемые `installsPythonDeps` — вне скоупа AC5 (тот описывает + текстовый `pip install`), дельта его не касается, остаётся + наблюдением без действия. +- Low-наблюдение r1 про склеенный текст в теле коммита `98028a30` — + тот коммит не переписывался, наблюдение остаётся снятым без действия. + +## Чего не проверял + +- `python -m pytest tests_backend`, `ruff check`, `mypy strict` — среда + не изменилась с r1, пакеты по-прежнему не установлены (не + перепроверял `pip show` повторно, так как дельта их не касается и + вывод не мог измениться). +- `npm run golden:verify`, geometry invariants, browser-смоки — не по + необходимости: дельта — только тестовый файл и файл мутантов, + видимое поведение продукта не существует для этой задачи вовсе. +- `node scripts/check-docs.mjs` — условие запуска (`src/**` в диффе) не + выполнено. +- Реальный HA-харнесс с новым пином фронтенда — ограничение среды, + идентичное r1, дельта его не касается. + +## Вывод + +High из r1 закрыт по существу и проверен воспроизведением, а не +заявлением автора: тест, названный AC5, теперь действительно исполняет +код обхода каталога и красится при возврате к хардкод-списку. Новых +находок в дельте нет. Задача может двигаться дальше без ещё одного +блокирующего цикла код-ревью.