From 1e2eba95b1fff2bb21397b8982ce693b20d0c589 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 21 Aug 2026 14:07:33 +0000 Subject: [PATCH] docs: review document for #230 Issue: #230 User-Visible: no --- docs/reviews/CODE-REVIEW-230-r2.md | 192 +++++++++++++++++++++++++++++ 1 file changed, 192 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-230-r2.md diff --git a/docs/reviews/CODE-REVIEW-230-r2.md b/docs/reviews/CODE-REVIEW-230-r2.md new file mode 100644 index 00000000..e31d285d --- /dev/null +++ b/docs/reviews/CODE-REVIEW-230-r2.md @@ -0,0 +1,192 @@ +# CODE-REVIEW-230-r2 + +- Issue: [#230](https://github.com/Matysh/houseplan-card/issues/230) +- Ветка: `issue/230-hatch-density-normalization` +- Коммит на ревью: `6836561340886b113aadc66534c28f6bf75dce0f` (HEAD) +- ТЗ: `docs/specs/230-hatch-density-normalization.md`, ревью ТЗ зелёное на заходе r2 + (`docs/reviews/SPEC-REVIEW-230-r2.md`) +- Заход код-ревью: r2 · блокирующих циклов израсходовано 1/4 до этого вердикта + +## 0. Раунд r1 — SHA (находка) + +Комментарий с вердиктом r1 (2026-08-21T12:33:44Z, «Вердикт: жёлтый · заход r1 · +… High: 2 · Medium: 0») **не называет SHA**, на котором получен разбор — сам +документ `docs/reviews/CODE-REVIEW-230-r1.md` его называет (`edf1cca`), но +комментарий в issue самодостаточным не является. Восстановлено сопоставлением +меток времени: `edf1cca` запушен в 12:11:58 UTC (после конвертации из +`+03:00`), вердикт — 12:33:44 UTC, следующие коммиты (`d3468c1`, `6811cc0`, +`6836561`) запушены в 13:42–13:49 UTC — то есть строго после вердикта. +Подтверждено списком прогонов CI (`gh run list`): push с `headSha=edf1cca` +(`32480753934`, 12:12 UTC) идёт непосредственно перед вердиктом, следующий push +— уже `6811cc0` (`32488390779`, 13:44 UTC). SHA r1 = `edf1cca8e3651003808627246312a37923a2e181`. + +Это Low-находка процесса ревью (не продукта): следующий раз называть SHA прямо +в первой строке вердикта, а не только в документе. + +## 1. Дельта r1 → r2 + +`git diff edf1cca..HEAD --stat`: + +``` +docs/reviews/CODE-REVIEW-230-r1.md | 244 ++++++++++++++++++++++++++ +scripts/mutation-gate.mjs | 12 +- +2 files changed, 252 insertions(+), 4 deletions(-) +``` + +Только `scripts/mutation-gate.mjs` — продуктовая (в терминах гейта) правка. +Коммит `579d6e5` (документ r1) не в счёт. Три «настоящих» коммита между r1 и r2: + +- `d3468c1` — снимает два golden-эталона из `edf1cca` (ревёрт без переписывания + истории). +- `6811cc0` — возвращает те же два эталона отдельным коммитом с трейлерами + `Release:`/`Baseline-Reviewed:`. +- `6836561` — правит guard четырёх юнит-мутантов #230 в `scripts/mutation-gate.mjs`. + +Baseline-файлы (`demo/golden/baselines/{baselines-index.json, +large-house-zoom-040-dark.png, large-house-zoom-250-dark.png}`) в диффе не +видны, потому что revert (`d3468c1`) и повторное принятие (`6811cc0`) +взаимно гасятся — проверено напрямую: + +``` +$ git diff edf1cca:demo/golden/baselines/large-house-zoom-040-dark.png \ + 6811cc0:demo/golden/baselines/large-house-zoom-040-dark.png # пусто +$ git diff edf1cca:demo/golden/baselines/large-house-zoom-250-dark.png \ + 6811cc0:demo/golden/baselines/large-house-zoom-250-dark.png # пусто +$ git diff edf1cca:demo/golden/baselines/baselines-index.json \ + 6811cc0:demo/golden/baselines/baselines-index.json # пусто +``` + +Т.е. итоговые байты эталонов в `6811cc0` побайтово совпадают с тем, что уже +было golden-проверено в CI на `edf1cca` (job `golden: success`, прогон +`32480753934`) — переприкрепление трейлеров не изменило содержимое, только +его коммит-обёртку. Делта локальна: не затрагивает `src/**`, +`test/wall-thickness.test.mjs`, `demo/smoke_wall_hatch_density.mjs`, +документацию — только процессный артефакт (трейлеры коммита) и оснастку +мутационного гейта. Полный разбор не требуется по §2.10: это не ребейз, +не смена контракта, не новая подсистема, объём несопоставим с исходной +задачей. + +## 2. Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **High-1.** Коммит `edf1cca` меняет `demo/golden/baselines/**`, но не несёт `Release:`/`Baseline-Reviewed:` — гейт `provenance` красный на этом SHA (прогон `32480753934`). | `d3468c1` снимает эталоны из `edf1cca` (ревёрт, история не переписана); `6811cc0` возвращает те же эталоны отдельным коммитом с обоими трейлерами. | `git show -s --format=%B 6811cc0` содержит `Release: v1.67.0-beta.1` и `Baseline-Reviewed: …/runs/32480753934`; `node scripts/validate-commit-provenance.mjs --range 579d6e5..6811cc0` — 0 ошибок (проверено мной, см. §3); CI push `32488390779` (headSha=`6811cc0`) — job `provenance: success`. | +| **High-2.** 4 из 7 мутантов #230 (`hatch-step-ignores-cell-cm`, `-inverted`, `-unclamped`, `hatch-density-solid-threshold-off`) не пересобирают `test-build/` в изолированном worktree — «чистый прогон» падает `ERR_MODULE_NOT_FOUND` до применения мутации, тест не проверяет ничего. | `6836561` меняет guard всех четырёх на `npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs && node --test …` — тот же паттерн, что у соседних рабочих юнит-мутантов. | `git diff edf1cca..HEAD -- scripts/mutation-gate.mjs` (12 строк, все 4 ID); лично прогнано `node scripts/mutation-gate.mjs --id=<каждый из 4>` в реальном изолированном `git worktree` (тот же механизм, что и у эксплуатируемого гейта) — все четыре: `поймано 1 из 1` (см. §3). | +| **Low-1.** AC7 не перепроверяет отсутствие `scale` на `cell_cm: 25`, снята с записью, без правки. | Делта её не касается. | Не изменялось — наследуется как снятая. | + +Оба High закрыты содержательно (не только заявлением автора — обе закрывающие +правки проверены мной повторным исполнением, см. §3) и процедурно (трейлеры на +месте, гейт `provenance` зелёный, мутанты реально ловят мутацию в том же +харнессе, что использует прод-гейт). + +## 3. Как проверялось в r2 — таблица гейтов + +| Гейт | Прогнан | Результат | +|---|---|---| +| `npx tsc --noEmit` | да | чисто | +| `npm test` | да | 1014/1014 | +| `npm run build` + `sha256sum` трёх копий бандла | да | все три идентичны (`a6b5f9ec…`), `git status` чист после пересборки | +| `node scripts/mutation-gate.mjs --id=hatch-step-ignores-cell-cm` | да | `поймано 1 из 1` (реальный изолированный `git worktree`, не догадка) | +| `node scripts/mutation-gate.mjs --id=hatch-step-inverted` | да | `поймано 1 из 1` | +| `node scripts/mutation-gate.mjs --id=hatch-step-unclamped` | да | `поймано 1 из 1` | +| `node scripts/mutation-gate.mjs --id=hatch-density-solid-threshold-off` | да | `поймано 1 из 1` | +| `node scripts/mutation-gate.mjs --check` (все 80 мутантов реестра, дешёвая проверка применимости патчей) | да | все `ok`, ни одного конфликта якорей от правки #230 | +| `node scripts/validate-commit-provenance.mjs --range 579d6e5..6811cc0` | да | 0 ошибок — прямая проверка закрытия High-1 | +| `node scripts/validate-commit-provenance.mjs --range origin/dev..HEAD` | да | `edf1cca` по-прежнему красен индивидуально (ожидаемо, см. §5) — вся остальная история чиста | +| `gh run list` / `gh run view` по SHA веток (живой CI) | да | `edf1cca` → `provenance: failure`, остальные job (`frontend`, `golden`, `smoke`, `performance_smoke`, `process-gate`) — `success`; `6811cc0` и `6836561` → `provenance`/`process-gate`: `success` | +| `node scripts/process-gate.mjs` | да | диапазон `origin/dev..HEAD`, 9 коммитов, 0 предупреждений | +| `node scripts/check-docs.mjs` | да | чисто (7 файлов, 10 внешних ссылок) — делта документацию не трогает, прогнан для общей гигиены | +| `npm run golden:verify` | да (сверх минимума) | все 82 сцены `passed`, 0 расхождений — прямое подтверждение, что revert+reapply не изменил ничего рендерящегося | +| `python -m pytest tests_backend -q` | нет | делта не трогает `custom_components/**/*.py` | +| `demo/smoke_wall_hatch_density.mjs` и прочие именованные смоки AC7/8/12 | нет | делта их не касается (не трогает `src/**`, сам смок, ни рендереры); r1 уже прогнал и подтвердил `OK` на этом же коде — наследуется, см. §4 | +| Полный `demo/smoke_*` (127 шт.) | нет | делта локальна (оснастка гейта мутаций + перестановка трейлеров), не задета ни одна новая поверхность | +| `performance_smoke` | нет | не в AC, делта не трогает рендер-код | +| Полный `mutation-gate.mjs` без `--id` (~80 мутантов, дорогая пересборка на каждый) | нет | делта касается только 4 конкретных ID — они прогнаны точечно; `--check` (дёшево) подтвердил, что остальные патчи не сломаны текстуально | + +## 4. Унаследовано из r1 + +Без повторной проверки принято из `docs/reviews/CODE-REVIEW-230-r1.md` +(документ ревью r1, SHA разбора `edf1cca8e3651003808627246312a37923a2e181`, +r1-вердикт от 2026-08-21T12:33:44Z): + +- **AC1–AC6, AC9** — численные свойства `wallHatchStepUnits` / + `wallHatchNeedsSolid` (эталон при `cell_cm: 5`, физическая пропорция, + пределы клампа, откат на дефолт, отсутствие ложного «solid» для тонкой + стены). Делта не трогает `src/wall-thickness.ts` и + `test/wall-thickness.test.mjs` — доказательство не задето. +- **AC7, AC8** — отсутствие `scale` в `patternTransform` на эталоне и + неизменность паттерна при смене зума (смок `wall_hatch_density`). Делта не + трогает ни смок, ни `src/houseplan-card.ts`. +- **AC12** — оба рендерера (`houseplan-card.ts`, `space-render.ts`) читают шаг + из одной функции, побитовое совпадение паттерна. Делта не трогает + `src/space-render.ts`. +- **AC10** (по существу) — 82 golden-сцены без расхождений; в r2 перепрогнано + заново (см. §3) и результат совпал, но именно AC10 инвариантен к делте — + перепрогон был мерой избыточной осторожности, а не необходимостью. +- **Low-1** — AC7 не перепроверяет отсутствие `scale` отдельно на `cell_cm: 25`; + снята с записью в r1, делта её не касается, остаётся снятой. +- **Раздел «Что проверено и корректно» r1** целиком (формула, оба рендерера на + одной функции, толщина штриха, порог `solid` через «или», трейлеры на месте + в `edf1cca`, `docs/WALL-THICKNESS.md` обновлён) — делта не трогает ни один из + перечисленных файлов кроме самих коммит-трейлеров, которые в r2 проверены + заново (§2, §3). + +## 5. Находки этого раунда + +Нет High и Medium. Одна Low, о процессе ревью, не о продукте: + +**Low-2.** Комментарий с вердиктом r1 не называл SHA разбора — пришлось +восстанавливать по меткам времени и `gh run list` (§0). Не блокирует, правки в +код не требует; на будущее — называть SHA первой строкой вердикта. + +Отдельно зафиксирую вопрос, который не является находкой #230, а подтверждён +как заведомо принятое поведение процесса: `edf1cca` навсегда останется +индивидуально «красным» по `validate-commit-provenance.mjs`, если гонять +валидатор на диапазоне `origin/dev..HEAD` целиком (я проверил — до сих пор +краснеет). Это не дефект: `.github/workflows/validate.yml` гоняет `provenance` +по диапазону **этого push** (`before..head`), а не по всей истории ветки +(намеренно, комментарий в `resolveValidationRange` про #165 — иначе +переоценивались бы уже закрытые issue), и живой CI подтверждает: push с +`headSha=edf1cca` был красным именно на этом SHA и это уже разошлось по логам +CI — история не переписывается по правилу AGENTS.md, факт нарушения +зафиксирован навечно, а не скрыт. Автор прозрачно рассказал про это в issue. +Дальнейших вопросов к этому нет. + +## 6. Что проверено и корректно (специфично для r2) + +- Оба High из r1 закрыты не только текстом хендоффа, но повторным исполнением: + `validate-commit-provenance.mjs` на трейлерах `6811cc0`, живой CI на + `6811cc0`/`6836561`, и все 4 мутанта — лично прогнаны через + `node scripts/mutation-gate.mjs --id=` в реальном изолированном + `git worktree` (тот же путь, которым гоняет прод-гейт), а не через + «применение патча вручную» — именно ручной способ и был причиной, почему + автор не поймал High-2 в первый раз. +- Байтовое содержимое golden-эталонов в `6811cc0` идентично содержимому, + которое уже прошло `golden: success` в CI на `edf1cca` — переприкрепление + трейлеров не подменило картинку тайком. +- `mutation-gate.mjs --check` (все 80 записей реестра) подтверждает, что + правка guard-строк не сломала применимость патчей ни для одного мутанта, + включая три «смоковых» мутанта #230, которые делта не трогала. +- Три копии бандла побайтово идентичны после пересборки — делта не затрагивает + сборку, но гигиенический прогон подтверждает отсутствие дрейфа. + +## 7. Чего не проверял + +- Полный `demo/smoke_*` (127 файлов) — делта не задевает ни одну новую + поверхность; именованные смоки AC7/8/12 не перепрогонялись, так как делта их + не касается (наследуются из r1, §4). +- `python -m pytest tests_backend` — делта не трогает `custom_components/**/*.py`. +- `performance_smoke` — не в AC, делта не трогает путь рендера. +- Полный `node scripts/mutation-gate.mjs` без `--id` (все ~80 мутантов, + дорогая пересборка бандла на каждого) — прогнаны точечно только 4 + исправленных ID; дёшево проверено `--check` для остальных 76 (применимость + патчей, не исполнение guard). +- Реестр мутантов #220/#229 (тот же класс дефекта guard, что был в High-2) — + автор явно вывел это за скоуп #230 в issue #235; не моя проверка в этом + раунде. + +## 8. Вердикт + +Зелёный. Оба High закрыты и перепроверены исполнением, а не заявлением. Новых +High/Medium делта не создала. Единственная находка раунда (Low-2, про формат +вердикта) не блокирует и не требует правки кода.