From bae6afaae7bac2dd45bca52b6caf64a16d96f182 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Tue, 15 Sep 2026 06:16:08 +0000 Subject: [PATCH] docs: review document for #580 Issue: #580 User-Visible: no --- docs/reviews/CODE-REVIEW-580-r2.md | 231 +++++++++++++++++++++++++++++ 1 file changed, 231 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-580-r2.md diff --git a/docs/reviews/CODE-REVIEW-580-r2.md b/docs/reviews/CODE-REVIEW-580-r2.md new file mode 100644 index 00000000..8aaecfaf --- /dev/null +++ b/docs/reviews/CODE-REVIEW-580-r2.md @@ -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=` **по +содержимому**, не по 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 ` → `[]` + для каждого) — 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` не изменились (подтверждено побайтовым сравнением диффа). + +--- + +--- + + + +## Материал раунда + +- Ветка: `issue/580-outer-sun-wall-occlusion`, коммит `c044e9ea3861` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `723c351607934580f0bfbbc19bef5b182273f939` + ``` + git log --all --format='%H %T' | grep 723c35160793 + ``` +- Тело issue: `558be5cfc33a8fe1f475fbc5a0210392339265c20fad0c22e82858b57c96b848` +- Вердикт конвейера: `green` · High 0