docs: review document for #663

Issue: #663
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-26 23:09:27 +00:00
parent d64338224d
commit 8176f1190f
+221
View File
@@ -0,0 +1,221 @@
# CODE-REVIEW-663-r2
Материал раунда: `d64338224da8606bf2b346906b8f6f2a9bd0a15e` (ветка `issue/663-stairs`,
рабочая копия уже на нём). Предыдущий материал (r1): `59a60d526db73974a81e57b739458f59c18cfe7f`.
Дельта: `git diff 59a60d52..d6433822` — 2 коммита (`3f8aae83` — публикация
`CODE-REVIEW-663-r1.md`, класс C, не код; `d6433822` — сам фикс), 5
содержательных файлов: `src/stairs.ts` (+7/-3), `test/stairs.test.mjs` (+25),
`scripts/mutation-registry.mjs` (+39), `docs/CHANGELOG.md` (+2),
`docs/CHANGELOG.ru.md` (+6/-2).
Заход: r2 (код-ревью), блокирующих циклов израсходовано 1/4.
## Скоуп
Дельта закрывает ровно три Medium-находки `CODE-REVIEW-663-r1.md`: направление
разметки ступеней для `direction: 'backward'` (AC3), mutation-witness для
защитного AC7 (битая цель) и mutation-witness для защитного AC11
(backward-compat старого конфига + remap ссылки при полном импорте). Общий
скоуп задачи (#663) — прежний, см. `CODE-REVIEW-663-r1.md`, раздел «Скоуп»;
здесь не повторяется.
## Закрытие раунда r1
| Находка r1 | Чем закрыта | Где это видно |
|---|---|---|
| Medium-1 — разметка ступеней неверна для `direction:'backward'` | `src/stairs.ts:151-157`: якорь отсчёта и условие останова теперь зависят от `forward`; полные 30-см интервалы для `backward` считаются от `+length/2` (физический низ при развороте стрелки), остаток — у `-length/2` (физический верх) | `test/stairs.test.mjs:78-85` (новый блок в существующем тесте) — `backward.treads[0].a[0] === 595` (мимо форвардного `405`, зеркально), `backward.treads.at(-1).a[0] === 395` (мимо `605`); лично реверсировал `src/stairs.ts` к r1-версии и убедился, что этот же тест краснеет (`25 !== -25`) — тест не тавтологичен |
| Medium-2 — защитный AC7 без mutation witness | Новый `stairs-broken-targets-become-active` (`scripts/mutation-registry.mjs`), патчит `src/stairs-editor-model.ts:101-103` (`stairTargetState`) на константу `'active'`, guard `node demo/smoke_stairs.mjs` | Лично прочитан diff мутанта — find-блок совпадает с реальным кодом строка-в-строку; в CI Validate `36276064818` (job «Мутанты по диффу (6/6)») лог: `ok stairs-broken-targets-become-active: заявленный тест покраснел на мутанте` — прочитан лично |
| Medium-3 — защитный AC11 без mutation witness | Два новых мутанта: `stairs-legacy-config-materializes-empty-collection` (`src/plan-optimizer.ts`, guard — юнит `test/stairs.test.mjs` по имени) и `stairs-import-skips-target-space-remap` (`custom_components/houseplan/import_export.py`, guard — `backend-test-guard.mjs` на существующем backend-тесте) | Job «Мутанты по диффу (4/6)»: `ok stairs-legacy-config-materializes-empty-collection: …`; job «Мутанты по диффу (3/6)»: `ok stairs-import-skips-target-space-remap: …` — оба лога прочитаны лично. Юнит-тест `#663 legacy-no-stairs-config …` (`test/stairs.test.mjs:196-208`) лично пересобран и прогнан (`node --test`, 1/1 OK) |
Все три находки r1 закрыты доказательно (не только заявлением автора): для
Medium-1 я лично воспроизвёл регресс на пред-фикс версии кода, для Medium-2/3
прочитал реальные логи CI-джобов текущего SHA, где заявленный тест-свидетель
покраснел на мутанте (не поверил хендоффу на слово).
## Унаследовано из r1 без повторной проверки
Дельта не касается ни одного из следующих файлов/областей — приняты по
`CODE-REVIEW-663-r1.md` (материал `59a60d52`, вердикт жёлтый, High 0) без
повторного разбора: continuous/мебельный transform-контракт (кроме
затронутого магнитного свидетеля, см. находку ниже), backend-схема
(`validation.py`, лимит 250), repair ссылки при удалении пространства,
i18n (4 языка), 2D/2.5D через общий floor-projection слой, батчинг вычитания
площади по кадрам (`ea1f19c3`), navigation/gesture guard'ы, документация
(`STAIRS.md`, `CANVAS.md`, `UX-MODES.md` и др.).
## Как проверялось
Ссылка на подтверждённый зелёный Validate точного SHA материала:
https://github.com/Matysh/houseplan-card/actions/runs/36276064818 —
job-список проверен построчно (`gh run view … --json jobs`): зелёные —
предпролёт, классификация, **frontend (types+unit+build)**, **мутанты по
диффу (6/6 шардов)**, «доказательство выполненных проверок»; **skipped** —
hacs, hassfest, geometry_parity, backend (pytest), смоки в браузере, golden,
performance_smoke (ожидаемо: heavy-гейты не запускаются на обычном пуше).
| Гейт | Прогнано | Результат |
|---|---|---|
| `npx tsc --noEmit` / `npm run build` | зачтено по зелёному Validate (frontend job) | — |
| `npm test` | зачтено по зелёному Validate (frontend job) | лог job `108498979810`: `# tests 3143`, `# pass 3142`, `# fail 0` (1 skipped) — включает все 11 тестов `test/stairs.test.mjs`, включая новые |
| Мутанты по диффу (registry, 6/6 шардов) | зачтено по зелёному Validate + лично прочитаны логи 3 новых ID | `stairs-broken-targets-become-active`, `stairs-legacy-config-materializes-empty-collection`, `stairs-import-skips-target-space-remap` — все три «заявленный тест покраснел на мутанте» |
| `node scripts/check-docs.mjs --screenshots=warn` (режим CI на обычном пуше, сверено с `validate.yml:76-90`) | прогнано лично | `Documentation checks passed (7 files, 12 external links)`, 1 ожидаемый `WARN` про устаревший отпечаток скриншотов (routine-следствие правки `src/**`, не блокирует до кандидата беты, #479) |
| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | прогнано лично | 101 прямое совпадение, 3 зарегистрированные связи (upload-guard, к лестницам не относится) — как и в r1, широкий список из-за общих символов диффа в 18 файлах `src/**` |
| `node demo/smoke_stairs.mjs` (AC1/AC2/AC3/AC5/AC6/AC7/AC8/AC10) | прогнано лично, свежий `npm run bundle:sync`, **~13 последовательных запусков** | 10/13 — все 29 проверок `OK`; **3/13 — `wallMagnetUsesPhysicalFace: false`** (см. находку Medium-2 ниже) |
| `npm run golden:verify` (полная матрица, 179 сцен) | прогнано лично на свежем бандле этого SHA | **175 passed, 4 different**: `stairs-flat-normal-light`, `stairs-flat-hover-dark`, `stairs-flat-selected-light`, `stairs-isometric-dark` (см. находку Medium-1 ниже) |
| `python -m pytest tests_backend -q` | не прогнано | в этой ревью-среде не установлен `pytest`/`homeassistant` (как и в r1); дельта не трогает backend-продакшен-код, только добавляет mutation-registry запись поверх уже существующего и уже проверенного в r1 backend-теста |
| `npm run invariants -- --config …` | не прогнано | дельта не меняет модель/геометрию хранения, только рендер-производную математику (позиции линий ступеней) — не тот класс риска, под который заточен этот гейт |
После проверки бандл возвращён к закоммиченному состоянию (`npm run
bundle:clean`), рабочая копия чистая (`git status` — пусто).
## Находки
### Medium-1 (в скоупе задачи) — AC12: golden-эталон устарел для 4 сцен с лестницами, фикс AC3 не сопровождён обновлением baseline
`demo/golden/matrix.mjs:206-212` заводит `stairLayerFixture` с прямой малой
лестницей `direction: 'backward'` (используется в 4 сценах
`stairs-flat-normal-light`/`stairs-flat-hover-dark`/
`stairs-flat-selected-light`/`stairs-isometric-dark`, помеченных в коде как
`#663 AC3/AC10/AC12`). Фикс Medium-1 из r1 (`src/stairs.ts`, эта дельта)
корректно меняет позицию ступеней именно для `backward` — то есть *обязан*
изменить пиксели этих 4 сцен. Committed baseline (`demo/golden/baselines/**`)
не обновлён: `git diff 59a60d52..d6433822 -- demo/golden/` — пусто.
**Воспроизведение:** `npm run bundle:sync && npm run golden:verify` на этом
точном SHA → `175 passed, 4 different` (список выше). Открыл
`artifacts/golden/diff/stairs-flat-normal-light.png` — расхождение (пурпурные
линии) сосредоточено ровно на малой развёрнутой на 45° `backward`-лестнице
справа сверху; все остальные объекты сцены (прямая forward, обе спиральные,
стены, проёмы) идентичны принятому эталону пиксель-в-пиксель. Это подтверждает
точную причину: расхождение — прямое и единственное следствие фикса AC3, а не
шум окружения (иначе разошлись бы и другие 175 сцен).
Not High: сам фикс корректен (расхождение — ожидаемое улучшение, не
регрессия), продукт ничего не ломает. Но AC12 («light/dark, оба типа, оба
направления… не имеют clipping, мерцания и нечитаемой стрелки») сейчас
**не доказан** на этом SHA: заявленный в r1 «179 passed, 0 расхождений»
(тогда — на пред-фикс материале, где не было расхождения) для текущего
материала неверен, и новый baseline никем не принят. Прецедент в этой же
ветке показывает штатный путь закрытия — коммиты `93876d75`/`ac6f4bfe`
(`chore(golden): закрепить …` + `docs: обновить отпечаток …`, оба с
`Release:`/`Baseline-Reviewed-Local:` трейлерами) делали ровно это после
похожей правки рендера (`ea1f19c3`). Для этой находки такого коммита нет.
Чинится в этой же задаче: `npm run golden:capture`/`npm run docs:capture` →
визуальный осмотр 4 diff → `npm run golden:accept -- --reviewed` с
`Release:`/`Baseline-Reviewed*` трейлерами.
### Medium-2 (в скоупе задачи) — AC2 smoke-свидетель `wallMagnetUsesPhysicalFace` статистически ненадёжен из-за бага в собственной формуле сравнения углов
`demo/smoke_stairs.mjs:87`:
```js
&& closeTo(((straight.angle % 180) + 180) % 180, 0);
```
Продукт после wall-magnet snap иногда отдаёт `angle` как чистый `0`, а иногда
как `-2.2737367544323206e-13` (нормальный floating-point шум тригонометрии
snap — сам по себе не баг: обе величины физически означают «строго
горизонтально»). Формула нормализации `((x % 180) + 180) % 180` для
отрицательного `x`, близкого к нулю **снизу**, корректно заворачивает его к
`≈180`, а не к `≈0` — и тест сравнивает результат только с `0` без учёта
цикличности. `closeTo(179.999999999999773, 0, 1e-5)` — `false`, хотя физически
угол равен нулю с точностью до `2×10⁻¹³`. Правильный паттерн для «расстояние
до 0 по модулю 180» уже есть в этом же кодовом дереве и делает именно так:
`src/junction-limits.ts:200-204` (`axisDegrees`/`collinear`) использует
`Math.min(delta, 180 - delta)`, а не сырое сравнение с `0`. Баг — только в
тестовой формуле, не в продукте.
**Воспроизведение:** 13 последовательных запусков `node demo/smoke_stairs.mjs`
на этом SHA (свежий `npm run bundle:sync`) → 3 красных (`false`), 10 зелёных
(`true`). Изолировал причину инструментированной копией скрипта: на красных
прогонах `straight.angle === -2.2737367544323206e-13` (на зелёных — ровно
`0`); подставил оба значения в `node -e` — воспроизводится детерминированно
(`norm = 179.99999999999977`, `closeTo0? false`). Проверил историю: строка
существует с самого первого коммита фичи (`d03a68b8`), не тронута
тест-стабилизацией `59a60d52` из материала r1 — то есть это не новый регресс
дельты r1→r2, а ранее не пойманная (видимо, ловилась в r1 везением: 5/5 при
эмпирической частоте отказа ~23% — `0.77⁵≈0.27`, шанс не так уж мал)
нестабильность собственного AC2-свидетеля, обнаруженная сейчас при
обязательном прогоне названного в AC смока.
Not High: продуктовый код корректен (подтверждено кодом-аналогом в этом же
дереве и математикой), поведение magnet не меняется. Но это защитный
AC2-свидетель со ссылкой в самом ТЗ (`unit + smoke`), и его ненадёжность
~1 раз из 4 создаёт риск ложных возвратов в `S6-in-progress` на будущих
переприменениях этого же смока (mutation-gate, nightly, merge-candidate
Validate) — фиксируется в этой же задаче: заменить сравнение на
`Math.min(norm, 180 - norm)` либо на прямую проверку `Math.abs(angle) < eps
|| Math.abs(Math.abs(angle) - 180) < eps`.
Итого: **High: 0, Medium: 2 (обе в скоупе задачи)**. Без High — жёлтый
вердикт; находки чинятся в этой же задаче, повторный раунд по дельте,
отдельный issue не заводится (#202).
## Что проверено и корректно
- **Все три Medium-находки r1 закрыты доказательно** — см. таблицу
«Закрытие раунда r1» выше; проверено исполнением (не только чтением кода):
личный реверт+перезапуск теста для Medium-1, чтение фактических CI-логов
красного-на-мутанте для Medium-2/3.
- **Unit-контракт** — 3143 юнит-теста зелёные (3142 pass + 1 skip), включая
все 11 тестов `test/stairs.test.mjs` (было 9 до дельты).
- **Мутанты по диффу** — все 6 шардов Validate зелёные на этом точном SHA;
прочитаны логи для всех 3 новых записей реестра — каждая корректно нацелена
на реальный код (find-блок совпадает построчно) и подтверждённо краснеет
заявленный тест.
- **Docs** — `check-docs.mjs` в режиме, реально используемом Validate на
обычном пуше (`--screenshots=warn`, сверено с `validate.yml:76-90`), зелёный;
единственный warn (устаревший отпечаток скриншотов) — ожидаемое следствие
правки `src/**`, не блокирует до кандидата беты.
- **Golden — 175 из 179 сцен идентичны эталону**; 4 расхождения локализованы
и объяснены (Medium-1 выше), не свидетельствуют о постороннем регрессе.
- Рассмотрен и **отклонён** как находка кандидат: `angle: 2.2737367544323206e-13`
(положительный, а не отрицательный эпсилон) на одном из зелёных прогонов —
проверил формулу отдельно, для положительного знака `((x%180)+180)%180`
корректно даёт `≈0`, баг асимметричен и проявляется только для
отрицательного эпсилон — согласуется с наблюдаемой частотой отказа (не
каждый второй, а по факту сторона эпсилон зависит от суб-пиксельного джиттера
клика).
## Чего не проверял
- **`python -m pytest tests_backend`** — не прогнан (нет `pytest`/
`homeassistant` в этой среде, как и в r1); дельта не меняет backend-код,
только добавляет mutation-registry запись поверх уже принятого в r1
backend-теста. Риск закрыт чтением: find/replace мутанта
`stairs-import-skips-target-space-remap` совпадает с реальным кодом
`import_export.py:1144-1151` построчно, и сам мутант подтверждён красным в
логе CI (см. таблицу выше) — то есть выполнен, просто не мной лично на этой
машине.
- **`npm run invariants`** — не прогнан, как и в r1 (нет готового экспорта
конфигурации с лестницами под рукой); дельта не меняет модель хранения.
- **Полная browser-smoke матрица** (278 смоков за пределами 101 прямого
совпадения) — не гонял; `smoke_stairs` (профильный) прогнан 13 раз лично
(см. находку Medium-2), остальные направленные смоки r1
(`smoke_room_resize`, `smoke_summary_first_paint`) дельтой не задеты и не
перепрогонялись.
- **`performance_smoke`/Full Performance** — heavy-гейт skipped на материале,
дельта не трогает perf-путь (`ea1f19c3` вне дельты r1→r2), не перепрогонял.
- **HACS/hassfest/geometry_parity** — skipped на материале, дельта не трогает
манифесты/geometry-модель, не прогонял отдельно.
- **Golden-эталоны других 175 сцен** — свежий локальный прогон подтвердил их
идентичность принятому baseline, но саму пересъёмку/приёмку новых 4 не
выполнял (не моя роль — это работа автора с `--reviewed`).
---
<!-- material-anchors: заполняется конвейером (#414) -->
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `issue/663-stairs`, коммит `d64338224da8` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `9d9f9c3524f9fcf632e09f3e177754e7579a2be6`
```
git log --all --format='%H %T' | grep 9d9f9c3524f9
```
- Тело issue: `f3f0ee8408eb76ccec4f3ec50a9048ff1e29b2967259f0d4c4d333d8c68157e4`
- Вердикт конвейера: `yellow` · High 0