mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,207 @@
|
||||
# CODE-REVIEW-532-r1
|
||||
|
||||
Вердикт: **жёлтый** · заход r1 · блокирующих циклов израсходовано 0 из 4 · High: 0 · Medium: 2 (в скоупе, возврат автору)
|
||||
|
||||
Материал: `bb04f8605f9003c0ec6643359fa47af9319247bc` (ветка приведена к `dev`
|
||||
конвейером; `origin/dev..HEAD` содержит один коммит, диплист диффа полный, не по
|
||||
дельте — цикл первый).
|
||||
|
||||
## Скоуп
|
||||
|
||||
Issue #532: тройной `drop-shadow` внешнего контура плана (`.hp-paperg`) живёт в
|
||||
общем композиционном слое с планом, поэтому любая перерисовка плана (ховер,
|
||||
панорама) заново прогоняет три прохода размытия по габариту всего листа —
|
||||
на машине владельца (Firefox) это ~9 к/с и 23 МБ текстур/кадр. Правка:
|
||||
`will-change: filter` на правиле `.stage.daycycle .hp-paperg,
|
||||
.hp-static-stage.daycycle .hp-paperg`, уводящее отфильтрованную бумагу в свой
|
||||
слой. Побочный эффект контракта (К2, принят на спек-ревью r2): промоушен делит
|
||||
SVG на два слоя, из-за чего диагональная штриховка стен антиалиасится иначе —
|
||||
4 golden-кадра `day-cycle-{dawn,day,dusk,night}-dark` расходятся и подлежат
|
||||
пересъёмке на кандидате беты, а не в этом коммите.
|
||||
|
||||
Продуктовая рамка: View-режим — продукт для двух из трёх персон (`SCOPE.md`),
|
||||
настенный планшет и телефон читают план именно там; ускорение растеризации
|
||||
дневного цикла закрывает именно этот сценарий, палитру/геометрию/слои
|
||||
окружения/блендинг не трогает (К3, К4 — вне скоупа, подтверждено кодом, см.
|
||||
ниже). Продукт кода вне `plan.styles.ts` не тронут.
|
||||
|
||||
Спек-ревью прошло два раунда (r1, r2 — оба зелёные); r2 переписал К2/AC3 после
|
||||
того, как автор сам обнаружил на реализации, что «golden останется зелёным» не
|
||||
подтвердилось, и вернул ТЗ на переработку вместо подгонки факта под старый
|
||||
текст. Код-ревью (этот документ) — первый заход, полный разбор.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Дешёвые гейты (`tsc`, `npm test`, `npm run build` + сверка 3 копий бандла,
|
||||
`check-docs.mjs`) подтверждены зелёным прогоном Validate на этом самом SHA
|
||||
(https://github.com/Matysh/houseplan-card/actions/runs/34622197915) — повторно
|
||||
не гонял, кроме случаев ниже, где перегон был нужен как побочный эффект.
|
||||
|
||||
Гейты, которые я прогнал сам (а не поверил хендоффу):
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| Смок-свидетель | `node demo/smoke_daycycle_raster.mjs` (после `npm run bundle:sync`, локальный ratio 0.73 — другая машина, тот же порядок, что у автора 0.76/0.82) | OK, 8/8 |
|
||||
| Мутант | `node scripts/mutation-gate.mjs --id=daycycle-outline-not-promoted` | `тест покраснел, как обязан` — поймано 1 из 1 |
|
||||
| Селектор смоков | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | НЕОПРЕДЕЛЁННОСТЬ (0 символов на изменённой строке) — совпадает с заявленным риском 5 |
|
||||
| Полная golden-матрица | `node demo/golden/run.mjs --mode=verify` (169 сценариев) | ровно 4 `different`: `day-cycle-{dawn,day,dusk,night}-dark`; всё остальное `passed`, включая 7 сценариев, которые в хендоффе автора были помечены как «шум песочницы» — здесь (пиновый Chromium) они все зелёные |
|
||||
| Мой собственный замер среднего цвета кадра по всем 4 кадрам (не только `night-dark`, как в хендоффе) | одноразовый скрипт через `createImageBitmap`/`getImageData`, baseline vs `artifacts/golden/actual/*`, удалён после прогона | `dawn` Δ−0.001, `day` Δ−0.014, `dusk` Δ+0.047, `night` Δ+0.080 из 255 — все четыре внутри заявленного допуска «не дальше 0.1» |
|
||||
| Ограничивающий прямоугольник расхождения по всем 4 кадрам | тот же одноразовый прогон | `[120‑121, 36, 867, 757‑758]` — идентичен по всем четырём кадрам и совпадает с рамкой, которую автор привёл для `night-dark`; расхождение не задевает ореол снаружи плана |
|
||||
| Три копии бандла | `diff -q dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` (+ `houseplan-assets.json`) | идентичны |
|
||||
|
||||
Не прогонял: `pytest tests_backend` (бэкенд не задет), `npm run invariants`
|
||||
(диффа геометрии/модели/`layout`/`marker.space`/`open_spans` нет — правка
|
||||
чисто в CSS), performance-профили Validate (не названы в AC), полный
|
||||
`npm test`/`tsc`/`build` заново как отдельный гейт (уже зелёные на этом SHA
|
||||
по Validate; `npm run build`+`bundle:sync` я всё равно выполнил как побочный
|
||||
эффект, чтобы поднять локальный смок/golden — красных мест не дали).
|
||||
`smoke_glow*`/`smoke_discovery_filters` не гонял: слабое совпадение по имени
|
||||
(«свет», «фильтр»), но К3/К4 прямо объявляют слои окружения и блендинг вне
|
||||
скоупа, а код-факт (`svgScreenBlendSupported` не читает `bg_mode`) это
|
||||
подтверждает — смотреть не было необходимости.
|
||||
|
||||
Рабочая копия оставлена чистой: все временные скрипты и `artifacts/golden/`
|
||||
удалены после использования, `git status` пуст.
|
||||
|
||||
## AC · чем доказан · чем краснеет (проверено самостоятельно)
|
||||
|
||||
| # | AC | Чем доказан | Чем краснеет | Проверено |
|
||||
|---|---|---|---|---|
|
||||
| AC1 | Подсказка `will-change: filter` стоит только при `daynight`, статичный фон — без неё | `demo/smoke_daycycle_raster.mjs`, часть 1 | мутант `daycycle-outline-not-promoted` | сам прогнал: смок зелёный на коде, красный на мутанте |
|
||||
| AC2 | Отношение растеризации на панораме ≤ 2.0 | тот же смок, часть 2 (CDP `RasterTask`) | тот же мутант (без декларации отношение ~15) | сам прогнал: 0.73 на коде, мутант красный |
|
||||
| AC3 | Изменились ровно 4 кадра `day-cycle-*-dark`; расхождение — антиалиасинг штриховки, не изменение вида (ореол цел, средний цвет кадра сдвинут ≤0.1/255) | полная golden-матрица + разбор пикселей | 5-й кадр либо сдвиг среднего цвета | сам прогнал полную матрицу (169 сценариев): ровно 4 different; сам посчитал средний цвет и bbox расхождения для **всех четырёх** кадров (не только `night-dark`, как в хендоффе) — все внутри допуска, bbox идентичен по всем четырём |
|
||||
| AC4 | Статичный фон не платит: ни `filter`, ни `will-change` | тот же смок | декларация без `.daycycle` | сам прогнал вместе с AC1 |
|
||||
| AC5 | `docs/SUN.md` описывает промоушен как часть контракта; оба чейнджлога на месте | построчная сверка ревьюером | расхождение документа и кода | сверил построчно — обе строки чейнджлога и абзац SUN.md на месте, но см. находки ниже |
|
||||
|
||||
AC3 закрыт по существу мной лично, а не на веру: спек-ревью r2 прямо
|
||||
отметило, что автор экстраполировал порог сдвига цвета с одного разобранного
|
||||
кадра (`night-dark`) на оставшиеся три и делегировало код-ревью обязанность
|
||||
проверить это «на реальном прогоне». В хендоффе автора эта проверка
|
||||
по-прежнему покрывала только `night-dark` количественно (три другие только
|
||||
помечены `different` в таблице). Я прогнал полную матрицу и лично посчитал
|
||||
средний цвет и bbox расхождения для `dawn`/`day`/`dusk`-dark — все три ведут
|
||||
себя идентично `night-dark` (тот же bbox, тот же порядок сдвига цвета), так
|
||||
что риск экстраполяции снят фактом, а не доверием.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium-1 (в скоупе) — чейнджлог утверждает то, что опровергает сам контракт задачи
|
||||
|
||||
`docs/CHANGELOG.md` и `docs/CHANGELOG.ru.md` (новая запись в `## Unreleased`):
|
||||
|
||||
> The picture is unchanged, to the pixel ([#532]).
|
||||
> Картинка не изменилась ни на пиксель ([#532]).
|
||||
|
||||
Это буквально неверно и противоречит К2/AC3 этой же задачи: 3.3–3.5 %
|
||||
пикселей на каждом из четырёх кадров дневного цикла расходятся (максимальное
|
||||
отклонение канала 81 из 255) — именно поэтому эталоны `day-cycle-*-dark`
|
||||
пересматриваются отдельным коммитом на кандидате беты. Сообщение самого
|
||||
коммита формулирует это точнее («the diagonal wall hatch anti-aliases
|
||||
differently and the four day-cycle baselines are re-taken»), а публичный
|
||||
чейнджлог — нет.
|
||||
|
||||
**Воспроизведение:** сравнить текст `docs/CHANGELOG.md` с любым из четырёх
|
||||
диффов golden-матрицы (`node demo/golden/run.mjs --mode=verify`,
|
||||
`day-cycle-night-dark`: 26 442 / 770 640 пикселей выше `maxChannelDelta: 10`).
|
||||
|
||||
**Правка:** перефразировать обе строки чейнджлога так, чтобы не утверждать
|
||||
побайтовую идентичность — например, в духе текста коммита («картинка читается
|
||||
так же; антиалиасинг штриховки стен смещается на несколько пикселей, эталоны
|
||||
дневного цикла пересматриваются отдельно, см. #532»).
|
||||
|
||||
### Medium-2 (в скоупе) — `docs/SUN.md` приписывает измерение в Chromium браузеру Firefox
|
||||
|
||||
`docs/SUN.md`, новый абзац (AC5):
|
||||
|
||||
> …without the hint every repaint of the plan re-ran three blur passes over
|
||||
> the whole sheet, which cost **a Firefox window** about **fifteen times**
|
||||
> the rasterization of a static background (#532).
|
||||
|
||||
Отношение «пятнадцать раз» — это измерение мутанта/смока через CDP-трассировку
|
||||
в headless **Chromium** (см. хендофф: «без правки… отношение 15.06»; тот же
|
||||
порядок в моём собственном прогоне мутанта). Профиль владельца в Firefox дал
|
||||
качественные цифры (23 МБ текстур/кадр, ~9 к/с, вклад drop-shadow 72–75 % от
|
||||
стоимости растеризации), но никогда не давал коэффициент «×15» — сам документ
|
||||
ТЗ и ревью r1/r2 явно фиксируют, что Gecko не профилировался этим свидетелем
|
||||
(«Chromium — не Gecko… приговор по Gecko даёт профиль владельца»). Формулировка
|
||||
SUN.md смешивает два источника числа под одной меткой, что противоречит
|
||||
собственной дисциплине проекта «одно число — один источник».
|
||||
|
||||
**Воспроизведение:** сверить абзац SUN.md с хендоффом/комментарием аналитики —
|
||||
цифра 15× нигде не связана с Firefox, только с Chromium/CDP.
|
||||
|
||||
**Правка:** заменить «a Firefox window» на «our Chromium/CI measurement» (или
|
||||
эквивалент), не приписывая коэффициент профилю владельца.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Селектор и объявление `will-change: filter` в `src/styles/plan.styles.ts`
|
||||
дословно совпадают с К1 (в т.ч. вторая ветка селектора `.hp-static-stage`);
|
||||
комментарий над правилом даёт числа Chromium/CDP корректно (852 мс / 28 мс /
|
||||
108 мс) — в отличие от SUN.md, здесь атрибуция источника числа верна.
|
||||
- `.hp-paperg` действительно группа только бумажных силуэтов
|
||||
(`src/houseplan-card.ts:11534`, `_paperShapes(space.rooms)`), не меняется
|
||||
при ховере/панораме — механика из ТЗ подтверждена чтением, не только словом
|
||||
аналитика.
|
||||
- `scripts/mutation-gate.mjs`: патч `daycycle-outline-not-promoted` находит
|
||||
ровно ту строку, которая добавлена в диффе (сверено побайтово через `grep`),
|
||||
мутация подтверждена прогоном.
|
||||
- `demo/golden/matrix.mjs` содержит ровно 4 сценария с `bgMode: 'daynight'` —
|
||||
структурно подтверждает и «К2 живёт только под `.daycycle`», и «ровно четыре
|
||||
кадра» из AC3.
|
||||
- `svgScreenBlendSupported` (`src/glow-blend.ts`) — рантайм-проба, не читающая
|
||||
`bg_mode` — подтверждает К4 (блендинг света вне скоупа) кодом.
|
||||
- `docs/images/screenshots.json`: изменился только `sourceFingerprint`
|
||||
(отпечаток `src/**`), все 11 `imageSha256` не изменились — соответствует
|
||||
заявлению «`npm run docs:accept -- --identical`», документация не поехала.
|
||||
- Три копии бандла (`dist/`, `custom_components/houseplan/frontend/`)
|
||||
побайтово идентичны; сборка воспроизводится (`npm run build` +
|
||||
`bundle:sync` на этом дереве прошли чисто).
|
||||
- Коммит несёт `Issue: #532` и `User-Visible: yes`; обе строки чейнджлога — в
|
||||
том же коммите, что и поведение (не в отдельном).
|
||||
- Продуктовый вопрос автора («приемлемо ли платить пересъёмкой 4 эталонов за
|
||||
скорость») не новый: это ровно К2/AC3, уже разобранные и принятые зелёным
|
||||
спек-ревью r2 с полным раскрытием факта (byte-for-byte ореол, сдвиг
|
||||
среднего цвета, план пересъёмки с `Release:`/`Baseline-Reviewed:`). Код
|
||||
реализует то, что было одобрено; переоткрывать это как продуктовое решение
|
||||
на этапе код-ревью нет оснований — я лишь перепроверил цифру на всех
|
||||
четырёх кадрах вместо одного (см. AC3 выше).
|
||||
- Риск 5 из ТЗ (`smoke-select` не находит связь) подтверждён независимо:
|
||||
тот же вывод «НЕОПРЕДЕЛЁННОСТЬ» на моём прогоне, что и в хендоффе.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- `pytest tests_backend` — диффа в `custom_components/**/*.py` нет.
|
||||
- `npm run invariants` — диффа геометрии/модели (рёбра, `layout`,
|
||||
`marker.space`, `open_spans`) нет, только CSS-свойство.
|
||||
- Performance-профили Validate (`performance_smoke` и т.п.) — не названы в
|
||||
AC, диффа в этих путях нет.
|
||||
- Приговор по настоящему Firefox (профиль владельца до/после) — вне
|
||||
возможностей код-ревью; ТЗ явно оставляет это отдельным шагом суждения
|
||||
владельца, а не AC этой задачи.
|
||||
- Полный `smoke_*` матрица (244 файла) — не прогонял целиком; `smoke-select`
|
||||
честно вернул «неопределённость», просмотрел список смоков с похожими
|
||||
именами (`glow*`, `discovery_filters`) и решил не гонять — К3/К4 прямо
|
||||
выводят эти поверхности из скоупа, и код (`svgScreenBlendSupported`) это
|
||||
подтверждает.
|
||||
|
||||
---
|
||||
|
||||
**Материал раунда:** SHA `bb04f8605f9003c0ec6643359fa47af9319247bc`,
|
||||
`origin/dev..HEAD` = 1 коммит, диапазон диффа `origin/dev...HEAD` (53 файла,
|
||||
включая 3 копии бандла и golden-fingerprint документации). Первый заход —
|
||||
раздела «Унаследовано из r0» нет.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/532-daycycle-raster`, коммит `bb04f8605f90` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `959371418fa5d512b6314804fb4d9cf13e86efe4`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 959371418fa5
|
||||
```
|
||||
- Тело issue: `12e66588332b94c78100156f01e4832881b8163fb6f8fdca83acfcb8e90f4545`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user