mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,231 @@
|
||||
# CODE-REVIEW-580-r2
|
||||
|
||||
Issue: #580 · Этап: code · Заход: r2 · Материал: `c044e9ea3861b777c04233073a02a0a3e2da354a`
|
||||
(рабочая копия проверена на этом SHA, `git fetch`/`checkout` не выполнялись)
|
||||
|
||||
## Почему это r2, а не продолжение r1
|
||||
|
||||
Первый код-ревью (`CODE-REVIEW-580-r1.md`) вынес **зелёный** вердикт на материале
|
||||
`4cb6827c3d1ea0e8307ff7f406b425262c25649a`. Пока ревью шло, `origin/dev` продвинулся на
|
||||
один коммит, автоматическое слияние отказало (§7.2 — «это другой код»), и автор
|
||||
перебазировал ветку на новый `origin/dev` скриптом `scripts/rebase-on-dev.mjs`
|
||||
(комментарий issue от 2026-09-15T06:07:14Z). Новая вершина — `c044e9ea` — и это
|
||||
материал текущего раунда. Старые SHA r1 (`4cb6827c`, `23e66392`, `c22eaf7d`) после
|
||||
ребейза не существуют в истории (`git cat-file -t` — `Not a valid object name`),
|
||||
поэтому дельта «дословно» через `git diff <старый SHA>..HEAD` невозможна.
|
||||
|
||||
Это ребейз на ушедший вперёд `dev` — по правилам раунда такой случай **не
|
||||
сокращается до дельты** и требует полного разбора. Ниже — полный разбор, но
|
||||
методом, который эксплуатирует то, что можно проверить дёшево: побайтовое
|
||||
сравнение каждого изменённого файла в `git diff origin/dev...HEAD` с тем, что
|
||||
цитировал и проверял r1, плюс независимая проверка, что тяжёлые CI-гейты на новом
|
||||
SHA подтверждены не «переиспользованием ради переиспользования», а совпадением
|
||||
хеша содержимого с ранее проверенным деревом.
|
||||
|
||||
## Скоуп
|
||||
|
||||
`git log --oneline origin/dev..HEAD` — шесть коммитов, из них три содержательных
|
||||
(остальные три — `docs: review document for #580`, коммиты документов SPEC-REVIEW
|
||||
r1/r2 и этого самого CODE-REVIEW r1, наследие этапа spec и предыдущего раунда, не
|
||||
трогаются):
|
||||
|
||||
- `d2c7e6f7` (`fix`, User-Visible: yes) — окклюзия комнатной части внешнего луча
|
||||
внутренними откосами толстой стены.
|
||||
- `a32b280b` (`fix`, User-Visible: no) — типизация того же кода (`Geom` вместо
|
||||
`as any`).
|
||||
- `8b8260b2` (`test`, User-Visible: no, несёт `Release: v1.76.0-beta.4` и
|
||||
`Baseline-Reviewed:`) — принятие двух golden-эталонов (DPR1/DPR2).
|
||||
|
||||
`git diff origin/dev...HEAD --stat`: 57 файлов, из них по существу —
|
||||
`src/sun.ts`, `test/sun.test.mjs`, `test/golden-matrix.test.mjs`,
|
||||
`demo/smoke_sun.mjs`, `demo/golden/matrix.mjs`, `scripts/mutation-registry.mjs`,
|
||||
`docs/SUN.md`, оба `CHANGELOG`; плюс два новых PNG-эталона,
|
||||
`demo/golden/baselines/baselines-index.json` и весь бандл
|
||||
(`dist/**`, `custom_components/houseplan/frontend/**`) — бандл пересобран с новыми
|
||||
хешами файлов из-за нового `dev` в основании, это ожидаемо и не несёт логики.
|
||||
`custom_components/**/*.py` в диффе отсутствует — AC7 по touch/i18n/схеме не
|
||||
задет.
|
||||
|
||||
Трейлеры: `d2c7e6f7` — `User-Visible: yes`, оба changelog в этом же коммите
|
||||
(проверено `git show --stat`). `8b8260b2` — `Release:`/`Baseline-Reviewed:` со
|
||||
ссылкой на реальный run.
|
||||
|
||||
## Проверка того, что ребейз не изменил код по существу
|
||||
|
||||
Построчно сравнил `git diff origin/dev...HEAD` для каждого содержательного файла
|
||||
с текстом, который `CODE-REVIEW-580-r1.md` цитировал и разбирал:
|
||||
|
||||
- `src/sun.ts` — diff идентичен описанному в r1 (тот же `intersectRings`,
|
||||
та же сигнатура `rayRimEdges`/`rimSources`, те же номера механик — `intersectRings(quad,
|
||||
rayQuad(innerA, innerB, away, len), clipPoly)` и дедупликация источников по
|
||||
`eps=1e-4`).
|
||||
- `test/sun.test.mjs` — оба новых теста (`#580: an oblique outer ray...`,
|
||||
`#580: normal incidence...`) побайтово совпадают с тем, что цитировал r1,
|
||||
включая константу `328.4529946162075`.
|
||||
- `demo/smoke_sun.mjs` — тот же `pointInRing`, тот же `innerFaceX` через отражение
|
||||
экрана, те же четыре новых `check(...)`.
|
||||
- `demo/golden/matrix.mjs`, `test/golden-matrix.test.mjs` — та же пара
|
||||
сценариев `dpr{1,2}`, версия матрицы `62→63`.
|
||||
- `scripts/mutation-registry.mjs` — те же два новых мутанта
|
||||
(`sun-ray-outer-jamb-occlusion-ignored`,
|
||||
`sun-ray-outer-occluded-rim-source-ignored`), `find`-паттерны совпадают со
|
||||
строками текущего `src/sun.ts`.
|
||||
- `docs/SUN.md`, оба `CHANGELOG` — тот же текст.
|
||||
|
||||
Вывод: продуктовая дельта #580 после ребейза не изменилась ни на строку —
|
||||
изменился только бандл (новые хеши ассетов) и добавились три коммита документов
|
||||
ревью. Это подтверждает и сам автор в комментарии («продуктовая дельта #580 не
|
||||
менялась»), но здесь это установлено чтением диффа, а не принято на слово.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
### Гейты
|
||||
|
||||
Валидация на точном материале `c044e9ea` — **зелёная**, дважды: run `34935586944`
|
||||
(упомянут во вводном сообщении) и более ранний `34935525859` того же SHA — оба
|
||||
`conclusion: success`.
|
||||
|
||||
Не поверил заявлению «зелёный» буквально: посмотрел job-логи run `34935586944` и
|
||||
обнаружил, что тяжёлые джобы (browser-смоки, golden, perf-смок, geometry-parity,
|
||||
backend) в нём **skipped** через `Переиспользование: это дерево уже проверено`.
|
||||
Чтобы отличить легитимный реюз от «зелёного, потому что не запускалось», прочитал
|
||||
лог этого job'а: ключи реюза считает `scripts/gate-reuse.mjs --job=<job>` **по
|
||||
содержимому**, не по SHA коммита и не по полному git-дереву (иначе ребилд бандла
|
||||
сломал бы совпадение):
|
||||
|
||||
| job | результат |
|
||||
|---|---|
|
||||
| `smoke` | `Cache hit for: reuse-smoke-bb7d10d9…` |
|
||||
| `golden` | `Cache hit for: reuse-golden-ac206839…` |
|
||||
| `performance_smoke` | `Cache hit for: reuse-performance_smoke-c2e55df8…-glow` |
|
||||
| `geometry_parity` | `Cache not found` (ожидаемо — diff не трогает модель геометрии) |
|
||||
| `backend` | `Cache not found` (ожидаемо — `custom_components/**/*.py` не тронут) |
|
||||
|
||||
Кэш-хиты для `smoke`/`golden`/`performance_smoke` означают, что релевантное
|
||||
содержимое (то, что реально проверяют эти гейты) на `c044e9ea` побайтово
|
||||
совпало с деревом, на котором r1 уже нашёл **реально выполненный** (не
|
||||
переиспользованный) прогон — `34933022019`, зафиксированный в r1 как run с
|
||||
3/3 browser-смоками, golden и 6/6 mutation-шардами зелёными. Это закрывает
|
||||
AC4/AC5 без повторного локального прогона: реюз здесь не «дыра», а
|
||||
детерминированная функция от неизменного содержимого, что я проверил, а не
|
||||
предположил.
|
||||
|
||||
Мутационные джобы (`Мутанты по диффу 1–6/6`) на `c044e9ea` — **не** переиспользованы,
|
||||
выполнены заново и зелёные: `npx tsc --noEmit`/typecheck и юнит-тесты прошли
|
||||
свежо на этом самом SHA.
|
||||
|
||||
**Не гонял локально:** `npx tsc --noEmit`, `npm test`, `npm run build`,
|
||||
`node scripts/check-docs.mjs`, `npm run golden:verify` — покрыты зелёным Validate
|
||||
на точном SHA (см. выше), плюс отдельно проверенным содержательным реюзом.
|
||||
`npm run invariants`, `python -m pytest tests_backend` — diff не касается
|
||||
персистентной модели геометрии (рёбра/толщина/layout/marker.space/open_spans) и
|
||||
не трогает `custom_components/**/*.py`; толщина стены здесь только читается
|
||||
(`wallDepthByOpening`), не записывается.
|
||||
|
||||
### Разбор кода
|
||||
|
||||
Продуктовый код (`src/sun.ts`) не изменился относительно того, что r1 уже
|
||||
разобрал построчно (независимый вывод формулы окклюзии в базисе {нормаль,
|
||||
тангенциальная ось окна}, ручной пересчёт константы `328.4529946162075` с
|
||||
совпадением до 13 знаков, проверка что мутанты бьют именно по изменённым
|
||||
строкам, проверка что `demo/smoke_sun.mjs` — независимая, не скопированная из
|
||||
продукта реализация геометрии). Побайтовое совпадение диффа (раздел выше)
|
||||
означает, что этот разбор остаётся в силе без повторного вывода — не потому что
|
||||
я поверил документу r1, а потому что сверил текст, который он проверял, с
|
||||
текстом, который лежит в `origin/dev...HEAD` сейчас.
|
||||
|
||||
Дополнительно к r1 проверил:
|
||||
- что три «продуктовых» коммита (`d2c7e6f7`, `a32b280b`, `8b8260b2`) после
|
||||
ребейза не имеют собственных CI-прогонов (`gh run list --commit <sha>` → `[]`
|
||||
для каждого) — Validate гонялся только на итоговом SHA ветки, что ожидаемо для
|
||||
push после ребейза без промежуточных пушей;
|
||||
- что коммиты `d37b2c9f` (`Prepare v1.76.0-beta.4`) и `e54ff33c` (новый tip
|
||||
`origin/dev`, docs-ревью #581), которые вошли в основание после ребейза, не
|
||||
трогают `src/sun.ts` и вообще не пересекаются с темой солнечных лучей —
|
||||
подтверждено `git diff d37b2c9f..e54ff33c -- src/sun.ts` (пусто) и
|
||||
`git show --stat` обоих коммитов;
|
||||
- что коммиты #577 (`внешний луч`, та же тема) целиком лежат в основании
|
||||
**до** ребейза (были в `dev` ещё когда открывалась ветка #580) — новых
|
||||
пересечений с #577 не возникло, `src/sun.ts` в диапазоне `d37b2c9f..e54ff33c`
|
||||
не менялся.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| находка r1 | статус |
|
||||
|---|---|
|
||||
| Блокирующих находок не было | — |
|
||||
| Не-Medium наблюдение (неточность формулировки ТЗ «одно дополнительное пересечение») | не относится к коду; сам код и диагноз не изменились ребейзом — наблюдение остаётся зафиксированным для памяти, не требует действия |
|
||||
|
||||
Раздел «находка → чем закрыта» здесь вырожден: r1 был зелёным без замечаний в
|
||||
скоупе задачи, поэтому r2 не закрывает правки — он подтверждает, что зелёный
|
||||
вердикт остаётся верным на новом материале после механического (не
|
||||
содержательного) ребейза.
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Из `docs/reviews/CODE-REVIEW-580-r1.md` (материал `4cb6827c3d1ea0e8307ff7f406b425262c25649a`,
|
||||
дерево `eb607a98d8c5a3f34ff9545a589358b50988c20f`) принято без повторного
|
||||
самостоятельного вывода, поскольку побайтовое сравнение диффов (раздел выше)
|
||||
подтвердило отсутствие изменений в этих файлах:
|
||||
|
||||
- независимый математический вывод формулы окклюзии и ручной пересчёт константы
|
||||
`328.4529946162075` — сверено, что тест и код не изменились;
|
||||
- анализ `rayRimEdges`/`uniqueSources`/отката на `[ray.a, ray.b]` при
|
||||
`rimSources === undefined` (AC3, режим `inner`/`d<=0`) — код не изменился;
|
||||
- проверка независимости `demo/smoke_sun.mjs` (`pointInRing`, `innerFaceX` через
|
||||
отражение) от продуктовой формулы — файл не изменился;
|
||||
- проверка `demo/golden/matrix.mjs`/хешей PNG на не-пустой принятый эталон и на
|
||||
корректный DPR при рендере — сценарии не изменились, только версия матрицы;
|
||||
- вывод «единственный источник числа» (обрезанная ширина луча входит в тот же
|
||||
`SunRay.polys`, второго независимого вычисления в продукте нет) — код не
|
||||
изменился.
|
||||
|
||||
## Найдено
|
||||
|
||||
Блокирующих находок нет.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- AC1–AC3 — код идентичен r1-проверенному, юнит-тесты не изменились.
|
||||
- AC4 (browser smoke), AC5 (golden DPR1/DPR2) — реюз-кэш на точном SHA `c044e9ea`
|
||||
подтверждён как совпадение по содержимому с деревом, где эти джобы реально
|
||||
выполнялись (см. таблицу `gate-reuse` выше), а не как слепое доверие статусу
|
||||
Validate.
|
||||
- AC6 — Validate зелёный на точном материале раунда, двумя независимыми run.
|
||||
- AC7 — diff по-прежнему ограничен солнечной геометрией/тестами/документацией и
|
||||
пересборкой бандла; `custom_components/**/*.py`, UI, i18n, touch-контракт и
|
||||
схема конфигурации не тронуты.
|
||||
- Трейлеры и оба changelog — в правильном коммите (не изменилось).
|
||||
- Ребейз на новый `dev` — чисто механический: без конфликтов исходников,
|
||||
продуктовая дельта #580 не изменилась ни на строку, новое основание
|
||||
(`d37b2c9f`, `e54ff33c`) не пересекается с темой солнечных лучей.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал `npx tsc --noEmit`, `npm test`, `npm run build`,
|
||||
`node scripts/check-docs.mjs`, `npm run golden:verify` локально — переиспользовал
|
||||
зелёный Validate на этом самом SHA, дополнительно проверив легитимность
|
||||
реюза тяжёлых джобов по ключам содержимого (см. выше), а не только по статусу.
|
||||
- Не запускал `npm run invariants` — diff не касается персистентной модели
|
||||
геометрии (рёбра/толщина/layout/marker.space/open_spans).
|
||||
- Не запускал `python -m pytest tests_backend` — `custom_components/**/*.py` не
|
||||
тронут.
|
||||
- Не перевыводил независимо математику окклюзии и не пересчитывал константу
|
||||
`328.4529946162075` — унаследовано из r1, поскольку строки `src/sun.ts` и
|
||||
`test/sun.test.mjs` не изменились (подтверждено побайтовым сравнением диффа).
|
||||
|
||||
---
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/580-outer-sun-wall-occlusion`, коммит `c044e9ea3861` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `723c351607934580f0bfbbc19bef5b182273f939`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 723c35160793
|
||||
```
|
||||
- Тело issue: `558be5cfc33a8fe1f475fbc5a0210392339265c20fad0c22e82858b57c96b848`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user