From 46c5d0be1d98816a3dbd970c9743e371db1c8a64 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 13:15:39 +0000 Subject: [PATCH] docs: review document for #140 Issue: #140 User-Visible: no --- docs/reviews/CODE-REVIEW-140-r2.md | 206 +++++++++++++++++++++++++++++ 1 file changed, 206 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-140-r2.md diff --git a/docs/reviews/CODE-REVIEW-140-r2.md b/docs/reviews/CODE-REVIEW-140-r2.md new file mode 100644 index 00000000..85ce67ec --- /dev/null +++ b/docs/reviews/CODE-REVIEW-140-r2.md @@ -0,0 +1,206 @@ +# CODE-REVIEW-140-r2 + +- **Issue:** [#140](https://github.com/Matysh/houseplan-card/issues/140) — узкий футер диалогов свойств проёма и физического объекта +- **Трек:** `trivial` (короткий трек, §5.1 PROCESS.md), лимит код-ревью — 2 цикла +- **Диапазон:** `origin/dev..HEAD` = 2 коммита + - `5ed28211610fb5d5125f9a15450c9294523cb5d2` "Fix editor property dialog footer width" + - `1937c325729181bfdba2f66cdc31e567668114c7` "docs: review document for #140" +- **Ревьюер:** Claude (код-ревью), свежая сессия без контекста реализации +- **Причина второго цикла:** r1 (`docs/reviews/CODE-REVIEW-140-r1.md`) вынес + зелёный вердикт `0 High / 0 Medium`. Слияние в `dev` конфликтовало, задача + вернулась в `S6-in-progress` не для правки кода, а для ребейза (PROCESS.md + §10.4: «после ребейза на ушедший вперёд `dev` это другой код», повторное + ревью не формальность). Автор перебазировал ветку на `origin/dev` (`f66cf8a`, + #138 — автозамыкание комнат вдоль общей стены, не связано с этой задачей) и + запушил `--force-with-lease`; новые SHA — те же два коммита выше. Продуктовая + правка не менялась. + +## Скоуп + +Диапазон `git diff origin/dev...HEAD` затрагивает те же 8 файлов, что и в r1, +плюс сам документ r1 (класс C, второй коммит): + +- `src/houseplan-card.ts` — атрибут `wide` на `` в + `_renderOpeningDialog()` (теперь :16481, было :16431 до ребейза — сдвиг + строк вызван несвязанным #138, попавшим в `dev` раньше) и + `_renderPhysicalDialog()` (теперь :16864). Diff — ровно две изменённые + строки, идентичен r1 посимвольно (сверено `git diff` — правка не тронута + ребейзом). +- `test/golden-matrix.test.mjs` — тот же расширенный subtest, без изменений + относительно r1. +- `demo/smoke_dialog_footer_width.mjs` — новый smoke, 215 строк, без изменений + относительно r1. +- `docs/CHANGELOG.md` / `docs/CHANGELOG.ru.md` — записи о #140 сохранены рядом + с записями #138, вошедшими из `dev` при ребейзе; конфликт разрешён + корректно, обе записи присутствуют. +- `dist/houseplan-card.js`, `custom_components/houseplan/frontend/houseplan-card.js`, + `demo/srv/assets/houseplan-card.js` — три копии, пересобраны из совмещённого + источника, идентичны друг другу. +- `docs/reviews/CODE-REVIEW-140-r1.md` — второй коммит, класс C, + `User-Visible: no`, ничего в продукте не меняет. + +`src/hp-dialog.ts` и `src/styles.ts` не тронуты (`git diff origin/dev...HEAD -- +src/hp-dialog.ts` — 0 строк) — общий контракт переноса футера и вторая ширина +диалога вне скоупа, как и требовал владелец. + +Коммит #138 (`f66cf8a`), вошедший в `dev` между r1 и ребейзом, правит +`src/houseplan-card.ts` в области авто-замыкания комнат — другая функция, +другая часть файла; пересечения со строками `_renderOpeningDialog`/ +`_renderPhysicalDialog` нет, конфликт при ребейзе был чисто позиционным +(сдвиг строк), не содержательным — подтверждено чтением объединённого diff: +после ребейза в файле присутствуют ровно две правки `wide`, без остатков +маркеров слияния и без побочных изменений в теле методов. + +Продуктовая линия по `docs/SCOPE.md` не изменилась относительно r1: правка +полирует J6 в admin-only поверхности редактора Плана, ничего не открывает +заново и не противоречит инвариантам View. + +## Как проверялось + +Задача второго цикла — доказать, что ребейз не изменил поведение и не привнёс +взаимодействие с #138, а не переоткрывать разбор AC с нуля (он не изменился +относительно r1). Прогнаны все гейты, которые прогонялись в r1, плюс их +падающая версия — заново, на новом основании, а не по памяти прошлого цикла. + +| Гейт | Команда | Результат | +|---|---|---| +| Typecheck | `npx tsc --noEmit` | green, без вывода | +| Unit | `npm test` | green, **796/796** (было 789/789 в r1 — рост за счёт #138, вошедшего через ребейз; это не регресс) | +| Build + сверка трёх копий | `npm run build`, `sha256sum` трёх файлов | green; все три `cd4360780cdfc7c0fcd7004d95b9cf5dd9cffad673b32575b2b5062e9f42e74b`, совпадает с хешем, который автор назвал в комментарии после ребейза | +| Bundle reproducibility | пересборка из чистого дерева, `cmp` с закоммиченным `dist/houseplan-card.js` | идентичны байт-в-байт, `git status --short` после пересборки пуст | +| Целевой smoke | `node demo/smoke_dialog_footer_width.mjs` (бандл собран и скопирован в `demo/srv/assets` перед прогоном) | `OK`, все проверки `true`, числа ниже | +| Offline process-gate | `node scripts/process-gate.mjs` | «гейт пройден, предупреждений 1» — единственное предупреждение (п.3, отсутствие `docs/specs/140-*.md`) ожидаемо для трека `trivial` | + +**Числа из прогона smoke** (десктоп 1000×820, узкий 320×820) — воспроизведены +независимо на ребейзнутом коде, совпадают с r1: + +| Диалог | Язык | `surfaceWidth` | `spareWidth` | `oneRow` | +|---|---|---|---|---| +| opening | en | 500.00 | 240.31 | true | +| physical | en | 500.00 | 240.31 | true | +| opening | ru | 500.00 | 187.09 | true | +| physical | ru | 500.00 | 187.09 | true | + +Узкая ширина (320 px): `opening`/`physical`/`draft`/`space` — `surfaceWidth = +300.8`, `insideViewport: true`, `noHorizontalOverflow: true`, `wrapped: true` +(перенос сохранён). Черновой сегмент (`draft`) — `buttons: 4`, +`buttonsContained: true`, без требования одной строки — соответствует +«не в скоупе» из issue. + +**Дисциплина «тест должен уметь падать» — перепроверена мной лично на +ребейзнутом коде, не принята на слово из r1:** + +1. Unit: временно убрал `wide` из `_renderOpeningDialog()` (`sed` по номеру + строки на новом основании, :16481), прогнал + `node --test test/golden-matrix.test.mjs` → subtest 14 «all destructive + editor dialogs use the medium shell…» упал: + `AssertionError [ERR_ASSERTION]`, `failureType: 'testCodeFailure'`. Откатил + (`git checkout -- src/houseplan-card.ts`), `git status --short` снова пуст. +2. Smoke: тем же способом убрал `wide`, пересобрал (`npm run build`), скопировал + в `demo/srv/assets/houseplan-card.js`, прогнал smoke → + `FAILED (2): opening_en_medium_shell: expected true, got false`, + `opening_ru_medium_shell: expected true, got false` (остальные проверки, + включая диалог физического объекта — за него отдельная правка не + откатывалась — остались `true`, как и должно быть при точечном отказе). + Восстановил (`git checkout`), пересобрал, скопировал бандл во все три + расположения, сверил SHA-256 — снова `cd436078…`, совпадает с + закоммиченным. + +Оба теста реагируют именно на регресс той точки, которую защищают, и делают +это на **текущем**, ребейзнутом дереве — не на старом коде из r1. + +## AC — разбор + +AC не менялись относительно r1 (issue не редактировался между циклами) и +подтверждены заново на ребейзнутой ветке: + +- **AC1** (три кнопки в одну строку, оба диалога, en/ru) — доказано smoke: + `buttons: 3`, `oneRow: true` во всех четырёх комбинациях диалог×язык. +- **AC2** (положительный запас, перенос не срабатывает, числа названы) — + доказано smoke: `spareWidth` 240.31 px (en) / 187.09 px (ru) для обоих + диалогов, числа воспроизведены мной отдельным прогоном на новом + основании. +- **AC3** (узкий экран без горизонтального скролла, перенос сохранён, диалог + пространства и 4-кнопочный футер черновика не регрессируют) — доказано + smoke на 320 px: `insideViewport: true`, `noHorizontalOverflow: true`, + `wrapped: true` для всех четырёх форм; `draft` контейнируется без требования + одной строки. + +Все три AC доказаны исполняемым smoke на актуальном (пост-ребейз) коде, тесты +проверены на способность падать в этом же цикле. + +## Проверено чтением, не исполнением + +- Продуктовый diff (`git diff origin/dev...HEAD -- src/houseplan-card.ts`) + после ребейза — ровно две строки, символьно идентичен диффу из r1; + посторонних изменений в теле `_renderOpeningDialog`/`_renderPhysicalDialog` + или соседних методах нет. +- Коммит #138, вошедший в `dev` между циклами, правит другую область того же + файла (авто-замыкание комнат); пересечения со строками диалогов нет — + подтверждено `git show --stat f66cf8a` и чтением объединённого файла: + никаких маркеров конфликта, дублирующихся блоков или забытых `wide` не + осталось. +- `docs/CHANGELOG.md`/`docs/CHANGELOG.ru.md` после разрешения конфликта несут + обе записи (#138 и #140), ни одна не потеряна и не задвоена. +- Трейлеры: коммит реализации — `Issue: #140`, `User-Visible: yes`, оба + changelog правлены в этом же коммите; коммит документа ревью r1 — + `Issue: #140`, `User-Visible: no` (корректно, документ ревью не меняет + продукт). +- Ветка `issue/140-dialog-footer-width` соответствует трейлерам; офлайн + `process-gate.mjs` подтверждает это же с нулевыми блокирующими нарушениями. +- `src/hp-dialog.ts` и `src/styles.ts` не тронуты (0 строк diff) — общий + контракт ширины/переноса вне скоупа, как и в r1. + +## Что проверено и корректно + +- Ребейз не изменил продуктовую правку: diff двух строк `wide` идентичен r1. +- Взаимодействие с вошедшим между циклами #138 отсутствует — разные функции, + разные строки, конфликт был позиционным, не содержательным. +- Все три копии бандла синхронны и воспроизводимы локальной пересборкой + байт-в-байт; хеш совпадает с тем, что автор назвал в комментарии после + ребейза. +- Unit и smoke реально проверяют регресс (падают при откате `wide`), + перепроверено мной на текущем дереве, а не принято на слово из r1. +- Все три AC подтверждены исполняемым smoke с числами. +- Трейлеры, changelog, именование ветки — в порядке на обоих коммитах. +- Разрешение конфликта changelog корректно: обе записи (#138, #140) + присутствуют, ни одна не потеряна. + +## Чего не проверял — и почему + +- **`node demo/smoke_*.mjs` (остальные 126 из 127)** — не прогонял. Diff + реализации не изменился относительно r1 и по-прежнему не трогает + canvas-геометрию, солнце, LQI, освещение, изометрию, тач-инпут; #138, + вошедший через ребейз, уже покрыт своим собственным + `demo/smoke_room_autoclose.mjs` в отдельном цикле ревью (issue #138) и не + входит в диапазон, который проверяет этот документ. Прогон всех 127 уместен + «когда задача задевает всё» — здесь не тот случай (§8 PROCESS.md, #127). +- **`npm run golden:verify`** — не прогонял. Как и в r1: ни один + golden-сценарий (`demo/golden/matrix.mjs`) не открывает + `_renderOpeningDialog`/`_renderPhysicalDialog`; `styles.ts` не тронут, риска + регресса в покрытых сценах нет. +- **`python -m pytest tests_backend -q`** — не прогонял. + `custom_components/**/*.py` не входит в diff ни этой задачи, ни #138. +- **Performance-профили** — не прогонял. Ни AC, ни diff не называют + чувствительных к перфу путей. +- **Полный повторный разбор AC "с нуля"** — не проводился как отдельная + процедура: AC и продуктовый код идентичны r1, поэтому анализ AC1–AC3 в этом + документе опирается на новый прогон тех же доказательств, а не переизобретает + разбор; отличие от r1 сознательно сосредоточено на том, что реально могло + измениться при ребейзе (сдвиг строк, взаимодействие с #138, целостность + changelog, свежесть бандла). + +## Находки + +Нет. High: 0, Medium: 0, Low: 0. + +## Вердикт + +Ребейз на `origin/dev` не изменил продуктовую правку и не создал скрытого +взаимодействия с вошедшим между циклами #138 (проверено чтением объединённого +diff, а не принято на слово). Все гейты зелены на новом основании, дисциплина +«тест должен уметь падать» перепроверена лично на ребейзнутом коде для обоих +задействованных тестов, три копии бандла синхронны и воспроизводимы, трейлеры +и changelog в порядке. Скоуп не расширен и не сужен. + +**Вердикт: зелёный · цикл r2/2 · High: 0 · Medium: 0 → нет новых issue**