docs: review document for #521

Issue: #521
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-10 20:48:53 +00:00
parent 9a3af808d9
commit 3f3583ac58
+274
View File
@@ -0,0 +1,274 @@
# CODE-REVIEW-521-r2
Issue: #521 «fix: alignment guides follow the live gesture again»
Материал ревью: `9a3af808d95684b32df8f102da4211f3f800703c` (ветка
`issue/521-align-guides-live`). Диапазон делты этого раунда —
`0715321fdc69a2955d7850f3f384c1a0fdc25311..9a3af808` (материал r1 → материал
r2): один коммит `deebcfb0` (публикация `CODE-REVIEW-521-r1.md` конвейером,
0 изменений кода) и один продуктовый/тестовый коммит `9a3af808` «fix: hide the
settled guides copy for the whole gesture, and prove it».
Заход: r2 · блокирующих циклов израсходовано 1 из 4 (r1 — жёлтый).
## Скоуп
Единственная находка r1 (Medium, в скоупе) — AC5 заявлял доказательство
(«снятие подавления осевой копии слоя даёт 2 группы `.alignguides` — смок
красный»), которого прежний свидетель не давал: осевая копия
`.hp-editor-only-layer` в сценарии смока всегда была пуста, потому что смок ни
разу не форсировал осевую отрисовку посреди самого жеста. Дельта r1→r2 —
исправление ровно этого: переписанный сценарий `devices` в
`demo/smoke_align_guides.mjs` форсирует настоящую осевую отрисовку
(`c._hdrH += 0.01`) в середине перетаскивания, проверяет, что она
действительно произошла, что владение слоем перешло к осевшей сцене с
правильной (живой) точкой, что видимая группа ровно одна на каждом шаге
передачи, и что следующее движение возвращает владение живому художнику; плюс
новый мутант `live-editor-keeps-the-settled-guides-visible` в
`scripts/mutation-gate.mjs`, откатывающий подавление обратно в ветку `plan`
(как было до задачи).
Продуктовый код (`src/live-editor.ts`) в этой дельте не изменился по существу:
единственная правка — комментарий, объясняющий измеренный механизм передачи
слоя (было общее «плановый режим уже это делал», стало «владение чередуется
само, `_commitLiveEditor()` … измерено: две `.alignline` …»). Логика
`makeTransparent(...)` вне ветки `plan` осталась той же, что была принята и
проверена в r1 — сама защита не менялась, менялось только доказательство её
работы.
Дельта локальна по критериям §2.10: не ребейз (единственный промежуточный
коммит — публикация документа предыдущего раунда), не смена контракта
поведения (контракт п.3/AC5 переформулированы под тот же механизм, а не
заменены другим), не новая подсистема, объём (17 строк комментариев в
`live-editor.ts`, 16 строк нового мутанта, ~90 строк в свидетеле) далеко не
сопоставим с исходной задачей. Полный повторный разбор всей задачи не
требуется — по AC1–AC4, AC6–AC9 переверяю только то, что могла задеть дельта
(сам свидетель), остальное наследую из r1.
### Отдельная находка: тело issue правилось после зелёного ревью ТЗ (#517)
Хеш нормализованного тела issue на момент зелёного `SPEC-REVIEW-521-r3.md`
(«Материал раунда», конец документа) — `1e7f8d192f340e80e878aec0d6de383c982263254b408f8fc928f383130f5684`.
Текущий хеш (пересчитан дважды независимо: `gh issue view --json body` и
`gh api repos/.../issues/521 -q .body`, оба дают одинаковый нормализованный
текст) — `d117679d25abeab7ebfad3ed7902a8222625fc2ad43d0396b56d6fdacc12b5d3`.
Хеши расходятся — тело менялось после зелёного ревью ТЗ. Дельту GitHub не
хранит, поэтому проверяю не разницу, а текущий текст целиком (см. «Что
проверено» ниже).
Что именно изменилось, восстанавливается по признанию автора в комментарии
«Исправление по код-ревью r1 → заход r2»: «ТЗ: пункт контракта 3 и AC5
переписаны под измеренный механизм». Прочитал текущий пункт 3 «Контракта» и
строку AC5 в теле issue — они дословно описывают именно тот механизм
(чередование владения через `_commitLiveEditor()`, измеренные «две
`.alignline`, осевая отстаёт на шаг»), который выяснился при разборе r1 и
теперь доказан переписанным свидетелем. Текст не расходится с кодом и тестами
этого раунда — я сверил это построчно (см. «Что проверено»), а не принял на
слово автора.
**Оценка:** это находка процесса — правка раздела `## ТЗ`, уже прошедшего
зелёное ревью, должна по идее возвращаться на новый цикл `S4-spec-review`, а
не вноситься по ходу код-ревью. Но по существу правка не меняет ни персону, ни
сценарий, ни объём видимого изменения, ни один AC-номер — она уточняет
формулировку контракта и доказательства AC5 под механизм, который сам же
код-ревью r1 и вскрыл, и делает это прозрачно (объяснено в комментарии, а не
тихой правкой). Отдельного цикла ревью ТЗ эта поправка не заслуживает: именно
для такого случая существует делегирование «технический спор решает ревью
кода» (`PROCESS.md` §7.1) — здесь спора и нет, есть согласие с находкой r1,
выраженное в тексте. Классифицирую как **Low**, снимаю решением ревьюера:
текст ТЗ проверен целиком и корректен, действие не требуется.
## Как проверялось
Валидация на материале раунда (`9a3af808`) зелёная
(https://github.com/Matysh/houseplan-card/actions/runs/34526718586), включая
шесть параллельных джобов «Мутанты по диффу» — я прочитал логи всех шести
шардов напрямую (`gh run view --job <id> --log`), а не поверил статусу
«success» на слово:
| Гейт | Прогнал | Результат |
|---|---|---|
| Validate на `9a3af808` (`tsc`, `test`, `build`+bundle-sync, `no-new-any`) | нет, зачтено по зелёному прогону CI (см. ссылку) | success |
| Логи 6 шардов «Мутанты по диффу» в этом прогоне | да, прочитаны построчно | все 5 мутантов задачи пойманы: `live-editor-devices-drops-align-guides` (шард 5/6), `live-editor-decor-drops-align-guides` (4/6), `live-editor-plan-drops-align-guides` (1/6), `align-point-reads-frozen-snapshot` (2/6), **`live-editor-keeps-the-settled-guides-visible` (6/6)** — новый мутант этого раунда, «тест покраснел, как обязан» |
| `npm run bundle:sync` (пересборка + сверка `dist`/`custom_components/.../frontend`/`demo/srv/assets`) | да, локально (нужно было поднять `demo/srv/assets`, он не коммитится) | чисто, `git status` не показал diff после сборки — включает `tsc --noEmit` |
| `node demo/smoke_align_guides.mjs` (полный сценарий, все 34 поля) | да | все `true`, `OK` |
| `node scripts/mutation-gate.mjs --id=live-editor-keeps-the-settled-guides-visible` | да, независимо от CI, в рабочем дереве материала | «поймано 1 из 1» |
| Ещё 4 мутанта задачи по одному (`--id=…`) | да, все 4 | все «поймано 1 из 1» |
| `node --test test/smoke-harness-contract.test.mjs` (AC8) | да | 7/7 |
| `node scripts/check-docs.mjs` | да | зелёный, 7 файлов, 12 внешних ссылок |
| `node scripts/smoke-select.mjs --base 0715321f --head HEAD` (выборка по дельте раунда, не по всей задаче) | да | 1 прямое совпадение — `demo/smoke_align_guides.mjs` (символ `_alignPoint`); других смоков дельта не касается |
| `node demo/smoke_isometric_live_touch.mjs`, `node demo/smoke_touch_tips.mjs` | да (продуктовый код не менялся между r1/r2, но риск ТЗ по touch не был явно закрыт в r1 — закрываю здесь) | оба зелёные, все поля `true` |
| `cmp dist/houseplan-card.js custom_components/.../houseplan-card.js`, то же для `houseplan-panel.js` и `houseplan-assets.json` | да | побайтово идентичны |
| `git show -s --format=full 9a3af808` (трейлеры) | да | `Issue: #521`, `User-Visible: no` — верно: дельта не меняет видимого поведения (сама правка уже была `User-Visible: yes` в `53585b45`, эта дельта — доказательство и комментарий) |
| Хеш тела issue (`issueBodyDigest` из `scripts/review-doc-guard.mjs`, два независимых способа получения текста) | да | не совпадает с зафиксированным в зелёном `SPEC-REVIEW-521-r3.md` — находка выше |
Не прогонял: `golden:verify` (дельта раунда не меняет визуал — только
комментарий, тестовый файл и конфиг мутанта; видимый рендер не тронут),
`pytest tests_backend` (Python не тронут), полный набор из 239 смоков (не
обосновано ни диффом, ни `smoke-select`), performance-профили (см. ниже),
WSL/HA-харнесс.
**Перф (AC9).** Не перезамерял. Диф `src/live-editor.ts` между материалом r1 и
материалом r2 — только комментарии (проверил построчно: `if`/`makeTransparent`
вызовы в `paintHouseplanEditor` и `editorTemplate` идентичны байт в байт по
существу, разница — только текст комментариев). Комментарии не выполняются,
поэтому число операций в кадре жеста не изменилось ни на одну со времён r1, где
я независимо (не на слово автора) прогнал `benchmark:large-house-interaction`
+ `benchmark:compare` с собственной базой на `fa7ac02c` и получил зелёный отчёт
без регрессии (см. `docs/reviews/CODE-REVIEW-521-r1.md`, таблица гейтов). Этот
вывод наследуется без повторного прогона.
## Закрытие раунда r1
| Находка r1 | Чем закрыта | Где это видно |
|---|---|---|
| Medium: AC5 заявлял «снятие подавления даёт 2 группы `.alignguides` — смок красный», но смок никогда не форсировал осевую отрисовку посреди жеста, поэтому осевая копия всегда была пуста и мутация не проверялась | Сценарий `devices` смока форсирует настоящую осевую отрисовку через `c._hdrH += 0.01` (свойство вне `liveProperties`/`hoverProperties`/`gestureProperties` — проверил по `src/live-editor.ts:80-86`, оно не маршрутизируется в живой путь ни при каком состоянии жеста), проверяет, что рендер действительно случился (`handoverHappened`, счётчик `willUpdate`), что слой перешёл сцене с правильной живой точкой (`handoverGivesTheLayerBackToTheSettledScene`, `handoverGuideStaysOnTheLivePoint`), что видимая группа ровно одна (`handoverKeepsExactlyOneGuide`), и что следующее движение возвращает владение живому художнику (`nextMoveTakesTheLayerBack`). Новый мутант `live-editor-keeps-the-settled-guides-visible` откатывает `makeTransparent(...)` обратно в ветку `plan` — смок красный именно на `nextMoveTakesTheLayerBack` | `demo/smoke_align_guides.mjs:162-197`; собственный прогон смока — все поля `true`; собственный прогон `mutation-gate.mjs --id=live-editor-keeps-the-settled-guides-visible` — «поймано 1 из 1»; то же самое независимо подтверждено логом шарда 6/6 в Validate на `9a3af808` |
| (побочно устранённая слабость свидетеля, не отдельная находка r1) счёт направляющих по числу узлов в DOM вместо видимых — гашение делает копию прозрачной, а не удаляет | `visible(selector)` фильтрует по `layer.style.opacity !== '0'` вместо сырого `querySelectorAll`; проверил, что `makeTransparent` (`src/live-editor.ts:157-163`) действительно ставит `element.style.opacity = '0'` строкой, а `restore()` снимает через `removeProperty('opacity')` | `demo/smoke_align_guides.mjs:27-36`, `src/live-editor.ts:157-163`, `:213-220` |
Находка закрыта по существу: не «сделали AC5 неопровержимым переписыванием
слов», а заставили смок действительно воспроизвести сценарий (внешний осевой
рендер посреди активного жеста), который единственный делает защиту
наблюдаемой, и подтвердили мутацией, что без защиты сценарий ловится.
## Унаследовано из r1
Без повторной проверки в этом раунде приняты выводы
`docs/reviews/CODE-REVIEW-521-r1.md` (материал:
`0715321fdc69a2955d7850f3f384c1a0fdc25311`), поскольку дельта r1→r2 их не
задевает:
- диагноз и причинность регресса (`c0d61ca3`/#451, оба слома — слой и
замороженная точка) — не менялись, дельта их не касается;
- AC1 (направляющая в трёх живых жестах), AC2 (живая точка), AC3, AC4 —
свидетель для `decor`/`plan`-сценариев не менялся вовсе в этой дельте
(правки смока — только в сценарии `devices`, между строками 162 и 197);
мутанты `live-editor-devices-drops-align-guides`,
`live-editor-decor-drops-align-guides`, `live-editor-plan-drops-align-guides`,
`align-point-reads-frozen-snapshot` перепрогнаны мной заново тем не менее
(дёшево, один и тот же смок теперь длиннее) — все 4 по-прежнему «поймано 1 из
1»;
- AC6 (один расчёт кандидатов на кадр) — код и проверка не менялись в этой
дельте, наследуется техническое заключение r1 («проверено чтением», без
отдельного мутанта — осталось так же, это не регрессия дельты);
- AC7 (старые гарантии, #400) — не менялся код и не менялся тот участок
смока;
- AC8 (свидетель не фабрикует состояние) — тест-линт `test/smoke-harness-contract.test.mjs`
не менялся; перепрогнал (7/7) на новом более длинном смоке — линт по-прежнему
проверяет отсутствие `_deviceDrag =`/`_decorDraft =`, новые строки (`c._hdrH
= …`, `c._layout = …` уже был в старой версии) под запрет не подпадают;
- AC9 (перф) — см. «Как проверялось» выше, база `fa7ac02c` и зелёный отчёт r1
наследуются без повторного прогона, так как дельта не трогает исполняемый
код;
- бюджет `core-file-budget` и мягкость `_renderAlignGuides` на карточке
(`this._editorRuntime?._renderAlignGuides() ?? nothing`) — не менялись,
проверено в r1 чтением;
- трейлеры и changelog коммита `53585b45` — не менялись, `User-Visible: yes`
подтверждён в r1.
Технический раздел «Чего не проверял» из r1 (golden, pytest, полная матрица
смоков, WSL/HA-харнесс) остаётся в силе тем же образом — дельта раунда их не
задевает и не расширяет их необходимость.
## Находки
### Low-1 (снимается, см. выше) — тело issue правилось после зелёного ревью ТЗ без нового цикла `S4-spec-review`
См. раздел «Отдельная находка» в «Скоупе». Текст пункта 3 «Контракта» и AC5
переписан автором вместе с исправлением по код-ревью r1, без возврата в
`S3-spec`/`S4-spec-review`. Проверено: новая формулировка точно описывает
реализованный и теперь доказанный механизм, персона/сценарий/объём/номера AC
не изменились, изменение раскрыто в комментарии автора. Действия не требую;
записываю, чтобы хеш в следующем зелёном документе (материал раунда ниже)
зафиксировал новое значение и цепочка не молчала.
Других находок нет.
## Что проверено и корректно
- **Механизм передачи владения слоем воспроизведён чтением независимо от
комментария автора.** `updated()` (`houseplan-card.ts:4157`) на каждом
осевом рендере зовёт `_commitLiveEditor()` →
`commitHouseplanEditor` (`live-editor.ts:439-454`): отменяет
незавершённый `raf`, вызывает `restore(state)` (снимает `opacity`/`visibility`,
выставленные `makeTransparent`/`dim`/`hide`) и очищает живой корень
(`render(nothing, target)`). Значит после любого settled-рендера слой физически
переходит к осевшей сцене чистым, без «памяти» о прежнем подавлении.
- **`_hdrH` гарантированно не маршрутизируется в живой путь.** Прочитал
`liveProperties`/`hoverProperties`/`gestureProperties`
(`live-editor.ts:80-86`) и `routeHouseplanEditorUpdate`
(`live-editor.ts:120-145`) — `_hdrH` не входит ни в один набор, значит
`namedLive` ложно и осевой рендер происходит независимо от активного жеста.
Форсирование смоком (`c._hdrH += 0.01`) действительно моделирует внешний
settled-рендер (приход `hass`, ресайз), а не подглядывает во внутреннее
состояние жеста.
- **`_alignPoint` — геттер, а не хранимое состояние**
(`houseplan-card.ts:12879`), поэтому и живой, и осевой путь при вызове
`_renderAlignGuides()` во время самого settled-рендера читают одну и ту же
живую точку — вот почему «отставание на шаг» проявляется только НА
СЛЕДУЮЩЕМ живом кадре после хендовера, а не в момент самого хендовера; смок
проверяет обе фазы раздельно (`handoverGuideStaysOnTheLivePoint`, затем
`nextMoveTakesTheLayerBack`), и это совпадает с объяснением автора.
- **Мутант `live-editor-keeps-the-settled-guides-visible` патчит именно тот
код, который сейчас в файле** — сверил `find`-строку патча
(`scripts/mutation-gate.mjs`) с текущим `paintHouseplanEditor`
(`live-editor.ts:357-360`) посимвольно, совпадает; применимость и результат
подтверждены собственным прогоном и логом CI.
- **AC8 не сломан новыми строками смока.** `c._hdrH = …`, `c._layout = …` (уже
было раньше), `c.requestUpdate()` не запрещены тест-линтом — запрещены
только присваивания `_deviceDrag =`/`_decorDraft =`; перепрогнал линт, 7/7.
- **Touch-контракт не задет** (риск ТЗ явно требовал: «существующие
touch-смоки обязаны остаться зелёными») — `smoke_isometric_live_touch.mjs` и
`smoke_touch_tips.mjs` зелёные на материале раунда; в r1 эти два смока не
были явно названы прогнанными, закрываю пробел здесь, а не переношу его
дальше.
- **«Одно число — один источник» не применимо.** Направляющая — линия и точка,
не число; пользователю в этом диффе не показывается ни одна величина
дважды.
- **Бандл и трейлеры.** `bundle:sync` пересобрал дерево с нуля, `dist`/
`custom_components/.../frontend`/`demo/srv/assets` побайтово совпали с
закоммиченными файлами; `git show -s --format=full 9a3af808` — трейлеры
`Issue: #521`/`User-Visible: no` корректны, changelog не тронут и не должен
быть (дельта раунда не меняет видимое поведение сверх уже опубликованного в
`53585b45`).
## Чего не проверял
- `npm run golden:verify` — дельта раунда не меняет визуал (комментарий +
тестовый файл + конфиг мутанта), в r1 видимый рендер тоже не менялся кроме
уже принятого identical-скриншота.
- `python -m pytest tests_backend` — Python не тронут ни в одном коммите
задачи.
- Полная матрица из 239 смоков — не обоснована ни диффом раунда, ни
`smoke-select.mjs` (одно прямое совпадение); полный набор — предрелизный
гейт.
- Не перезамерял `benchmark:large-house-interaction` — обосновано выше
(дельта раунда не исполняемый код).
- Не запрашивал у владельца дополнительного цикла `S4-spec-review` по поводу
правки ТЗ после зелёного вердикта r3 — see Low-1: решил вопрос по существу
сам, поскольку это чисто техническая правка формулировки под уже найденный в
код-ревью механизм, а не продуктовый вопрос из числа тех, что задаются
владельцу (§7.1).
## Вердикт
Единственная находка предыдущего раунда закрыта по существу и подтверждена
воспроизведением (собственный прогон смока и мутанта плюс независимое чтение
логов CI). Новых блокирующих находок нет; единственная новая находка —
процедурная (Low), снята решением ревьюера с записью выше. High: 0. Задача
готова к очереди на пре-релиз.
Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0 → в задаче
Документ: docs/reviews/CODE-REVIEW-521-r2.md
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `issue/521-align-guides-live`, коммит `9a3af808d956` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `20fbfc5c7f020a361c04f252900511c2e383ceba`
```
git log --all --format='%H %T' | grep 20fbfc5c7f02
```
- Тело issue: `d117679d25abeab7ebfad3ed7902a8222625fc2ad43d0396b56d6fdacc12b5d3`
- Вердикт конвейера: `green` · High 0