mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
committed by
Sergey Matyunin
parent
8b8260b2c5
commit
c044e9ea38
@@ -0,0 +1,196 @@
|
||||
# CODE-REVIEW-580-r1
|
||||
|
||||
Issue: #580 · Этап: code · Заход: r1 · Материал: `4cb6827c3d1ea0e8307ff7f406b425262c25649a`
|
||||
(рабочая копия проверена на этом SHA, `git fetch`/`checkout` не выполнялись)
|
||||
|
||||
## Скоуп
|
||||
|
||||
Диапазон `origin/dev..HEAD` — три содержательных коммита плюс два коммита
|
||||
документов ревью (`515cf057`, `a18e41e2`, унаследованы от этапа spec, не
|
||||
трогаются):
|
||||
|
||||
- `23e66392` (`fix`, User-Visible: yes) — сама правка: окклюзия комнатной
|
||||
части внешнего луча внутренними откосами толстой стены.
|
||||
- `c22eaf7d` (`fix`, User-Visible: no) — только типизация того же кода
|
||||
(`intersection`/`Geom` вместо `as any`), поведение не меняет — сверено
|
||||
построчно, дифф идентичен по логике.
|
||||
- `4cb6827c` (`test`, User-Visible: no, несёт `Release:`/`Baseline-Reviewed:`) —
|
||||
принятие двух golden-эталонов (DPR1/DPR2).
|
||||
|
||||
Класс изменений: A (`src/sun.ts`), B (`test/sun.test.mjs`,
|
||||
`demo/smoke_sun.mjs`, `demo/golden/matrix.mjs`, `test/golden-matrix.test.mjs`,
|
||||
`scripts/mutation-registry.mjs`), C (`docs/SUN.md`, оба CHANGELOG), D
|
||||
(бандл в `dist/**` и `custom_components/houseplan/frontend/**`,
|
||||
`demo/golden/baselines/**`) — соответствует AC7: UI, i18n, touch-контракт и
|
||||
схема конфигурации не тронуты (`custom_components/houseplan/validation.py` в
|
||||
диффе отсутствует).
|
||||
|
||||
Трейлеры: у `23e66392` `User-Visible: yes` и правки **обоих** changelog в
|
||||
том же коммите (проверено `git show --stat` — `docs/CHANGELOG.md` и
|
||||
`docs/CHANGELOG.ru.md` оба в списке файлов). У коммита с baseline —
|
||||
`Release: v1.76.0-beta.4` и `Baseline-Reviewed:` со ссылкой на реальный run.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
### Гейты
|
||||
|
||||
**Уже подтверждены на этом SHA (не перегонял):** `npx tsc --noEmit`,
|
||||
`npm test`, `npm run build` — по ссылке из вводного сообщения ревью
|
||||
(run `34934599122`, `completed/success`, `headSha = 4cb6827c...`).
|
||||
|
||||
**Проверил сам через `gh run view` (а не поверил заявлению автора):**
|
||||
нашёл на этом же SHA более ранний run `34933022019` (тот же `headSha`), где
|
||||
тяжёлые джобы реально выполнились, а не были пропущены реюзом:
|
||||
|
||||
| job | conclusion |
|
||||
|---|---|
|
||||
| Смоки в браузере (шард 1/2/3 из 3) | success |
|
||||
| Смоки: все шарды зелёные | success |
|
||||
| Golden-кадры против принятых эталонов | success |
|
||||
| Перф-смок: бюджет времени кадра | success |
|
||||
| Мутанты по диффу (1–6/6) | success |
|
||||
|
||||
Это закрывает AC4 (browser smoke), AC5 (golden на точном SHA, включая
|
||||
только что принятые `lighting-sun-window-outer-occluded-dpr{1,2}-dark`) и
|
||||
AC6 без повторного локального прогона. Второй run на том же SHA
|
||||
(`34934599122`) эти джобы пропустил через «Переиспользование: это дерево
|
||||
уже проверено» — ожидаемо, т.к. дерево между двумя прогонами не менялось.
|
||||
|
||||
**Не гонял и не требовалось:** `node scripts/check-docs.mjs` отдельно —
|
||||
джоб `docs`/«Предполёт» зелёный в обоих run на этом SHA, а diff не меняет
|
||||
скриншотный контракт (только `src/sun.ts`, без UI). `python -m pytest
|
||||
tests_backend` — `custom_components/**/*.py` не тронут. `npm run
|
||||
invariants` — diff не меняет рёбра комнат, записи толщины, `layout`,
|
||||
`marker.space` ни `open_spans`; толщина стены здесь читается
|
||||
(`wallDepthByOpening`), не записывается, модель геометрии не изменена.
|
||||
Полный `demo/smoke_*` набор — не требовался, три browser-shard'а уже
|
||||
покрывают изменённый `demo/smoke_sun.mjs` (см. выше).
|
||||
|
||||
### Разбор кода (чтением, не исполнением, сверх того что покрыл CI)
|
||||
|
||||
Проверил математику исправления в `src/sun.ts` независимо от тестов —
|
||||
разложил `away` в базисе {нормаль, тангенциальная ось окна}: для точки
|
||||
параллельного луча с параметром `u` на внешнем пролёте, сдвиг на внутренней
|
||||
плоскости `u' = u + s0·σ`, где `s0 = d/cos`, `σ = away·t̂` — константа,
|
||||
не зависящая от глубины проникновения в комнату. Значит
|
||||
`intersectRings(quad, rayQuad(innerA, innerB, away, len), clipPoly)`
|
||||
(`src/sun.ts:459-472`) корректно оставляет только `u`, для которых и `u`, и
|
||||
`u'` лежат в `[-half, half]` — это и есть «луч, видимый через оба пролёта».
|
||||
Туннельная часть (`intersectRings(quad, tunnel)`) не изменена относительно
|
||||
дореформенного кода.
|
||||
|
||||
Пересчитал вручную (по формуле выше) числа нового unit-теста
|
||||
`test/sun.test.mjs:495-517` (`#580: an oblique outer ray...`, окно на западной
|
||||
стене, `d=20`, `azimuth=240`, `elevation=60`, `northDeg=0`) — `cos ≈ 0.866`,
|
||||
`σ = -0.5`, `s0 ≈ 23.094`, отсюда допустимый диапазон `u ∈ [-40, 28.4529…]`,
|
||||
что даёт `min(seam)=260`, `max(seam)=300+28.4529946…=328.4529946162075` —
|
||||
совпадает с константой в тесте **до 13 знаков**, то есть значение не
|
||||
подогнано под текущий (возможно, ошибочный) вывод программы, а следствие
|
||||
независимо выведенной геометрии.
|
||||
|
||||
Rim (`rayRimEdges`, `src/sun.ts:627-636`): `rimSources = [a,b,innerA,innerB]`
|
||||
даёт кандидатов на боковые линии; `uniqueSources` дедуплицирует линии с
|
||||
одинаковым тангенциальным смещением (`eps=1e-4`) — при `d=0`/режиме `inner`
|
||||
`ray.rimSources` остаётся `undefined`, и функция откатывается на старое
|
||||
поведение `[ray.a, ray.b]` (AC3 не нарушен).
|
||||
|
||||
Проверил, что новый unit-тест умеет падать не только логически, но и
|
||||
институционально: `scripts/mutation-registry.mjs` содержит два новых
|
||||
мутанта именно на изменённых строках —
|
||||
`sun-ray-outer-jamb-occlusion-ignored` (откатывает комнатную часть на старый
|
||||
`clipToRoom(quad, clipPoly)`, guard гоняет оба теста `#580:`) и
|
||||
`sun-ray-outer-occluded-rim-source-ignored` (обрезает `rimSources` до
|
||||
`[a,b]`, guard целится в тест про `an oblique`) — `find`-паттерны обоих
|
||||
патчей побайтово совпадают со строками текущего `src/sun.ts`, оба мутанта
|
||||
из `6/6` шардов упомянутого прогона зелёные (то есть красные до фикса,
|
||||
подтверждено CI, не только моим чтением).
|
||||
|
||||
`demo/smoke_sun.mjs` (`outer_oblique_*`, строки 344-352): независимая
|
||||
третья проверка тех же величин — `innerFaceX` вычислен не копированием
|
||||
формулы из `src/sun.ts`, а отражением экрана относительно центра стены
|
||||
(`wallCenterX*2 - r.a[0]`), и `pointInRing` — самостоятельная
|
||||
point-in-polygon реализация, не переиспользующая `pointInPolygon` продукта.
|
||||
Проверил, что ассерты `innerSeamMinY > 560 && < 580` и
|
||||
`blockedPointLit === false` не могли пройти на дореформенном коде: до фикса
|
||||
`polys` для комнатной части строился без учёта внутреннего пролёта, поэтому
|
||||
шов на внутренней грани всегда покрывал бы полную высоту окна (560↔640) —
|
||||
проверка `> 560` (строгое неравенство) на этом коде проваливалась бы.
|
||||
|
||||
`demo/golden/matrix.mjs`: новая пара сценариев использует `azimuth=240`
|
||||
(предыдущий `outer-thick` — перпендикулярный `azimuth=270`), т.е. это не
|
||||
дубликат существующего кадра, а действительно новый косой угол; проверка
|
||||
через контрольный кадр без лучей (`sunRayPixels`) исключает «пустой принятый
|
||||
эталон». Хэши двух PNG (`dpr1`/`dpr2`) в `baselines-index.json` совпадают
|
||||
побайтово — проверил размеры (345×195 у обоих) и sha256; это не ошибка
|
||||
конвейера: `run.mjs` явно проверяет `devicePixelRatio` рендерера
|
||||
(`Math.abs(actualScale - expectedScale) > 0.001` кидает ошибку), так что
|
||||
второй прогон гарантированно рисовал при масштабе 2 — здесь совпадение
|
||||
результата после кропа честное, а не признак того, что DPR2 не
|
||||
перегенерировался.
|
||||
|
||||
Проверил, что `clipToRoom` не используется больше нигде за пределами
|
||||
`src/sun.ts:449` (единственный сохранившийся вызов) — рефакторинг в
|
||||
`intersectRings` не задел других потребителей.
|
||||
|
||||
## Найдено
|
||||
|
||||
Блокирующих находок нет.
|
||||
|
||||
Замечание вне блокировки (не Medium, зафиксировано для памяти): claim в ТЗ
|
||||
«одно дополнительное пересечение» неточен буквально — ветка `outer && d>0`
|
||||
раньше уже делала два вызова `clipToRoom`, сейчас тоже два (`intersectRings`
|
||||
с тремя кольцами вместо двух + один с двумя), стоимость сопоставима, не
|
||||
per-frame/per-pixel цикл. Не требует правки, т.к. контракт AC («не вводить
|
||||
новые циклы по пикселям/кадрам») формально не нарушен.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- AC1, AC2 — доказаны юнит-тестами `test/sun.test.mjs` (два новых теста),
|
||||
числа перепроверены независимым выводом формулы (см. выше).
|
||||
- AC3 — режим `inner` и `d<=0` идентичны прежней геометрии (ветка кода не
|
||||
меняется, `rimSources` остаётся `undefined`); отдельный тест `#580:
|
||||
normal incidence...` проверяет неизменность полного пролёта.
|
||||
- AC4 — `demo/smoke_sun.mjs`, зафиксировано зелёным в CI на точном SHA
|
||||
(3/3 browser-шардов), плюс независимая переформулировка геометрии внутри
|
||||
самого смока (не копия формулы продукта).
|
||||
- AC5 — два golden-сценария (DPR1/DPR2), приняты только из полного Linux CI
|
||||
артефакта (`Baseline-Reviewed:` со ссылкой), `golden:verify` зелёный на
|
||||
точном SHA.
|
||||
- AC6 — Validate зелёный на точном финальном SHA (двумя независимыми run).
|
||||
- AC7 — diff ограничен солнечной геометрией/тестами/документацией; UI,
|
||||
i18n, touch, схема конфигурации не изменены.
|
||||
- Трейлеры и оба changelog — в правильном коммите.
|
||||
- Единственный источник числа: новая величина (обрезанная ширина луча) не
|
||||
дублируется отдельно нигде во View — она входит в тот же `SunRay.polys`,
|
||||
что рендерится и что использует `rayRimEdges`; второго независимого
|
||||
вычисления той же геометрии в продукте нет.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал `npx tsc --noEmit`, `npm test`, `npm run build` локально —
|
||||
переиспользовал зелёный Validate на этом самом SHA (двойное
|
||||
подтверждение, включая тяжёлые джобы, см. таблицу выше).
|
||||
- Не запускал `node scripts/check-docs.mjs` и `npm run golden:verify`
|
||||
локально — оба покрыты зелёными джобами `docs`/`golden` в CI на этом
|
||||
SHA.
|
||||
- Не запускал `npm run invariants` — diff не касается персистентной модели
|
||||
геометрии (рёбра/толщина/layout), только рендер солнечных лучей.
|
||||
- Не запускал `python -m pytest tests_backend` — `custom_components/**/*.py`
|
||||
не тронут.
|
||||
- Не запускал `node scripts/smoke-select.mjs` вручную — все три шарда
|
||||
`demo/smoke_*` уже прогнаны и зелёные в CI на этом SHA, включая изменённый
|
||||
`demo/smoke_sun.mjs`.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/580-outer-sun-wall-occlusion`, коммит `4cb6827c3d1e` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `eb607a98d8c5a3f34ff9545a589358b50988c20f`
|
||||
```
|
||||
git log --all --format='%H %T' | grep eb607a98d8c5
|
||||
```
|
||||
- Тело issue: `558be5cfc33a8fe1f475fbc5a0210392339265c20fad0c22e82858b57c96b848`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user