diff --git a/docs/reviews/CODE-REVIEW-339-r1.md b/docs/reviews/CODE-REVIEW-339-r1.md new file mode 100644 index 00000000..9f9c6b5b --- /dev/null +++ b/docs/reviews/CODE-REVIEW-339-r1.md @@ -0,0 +1,170 @@ +# Код-ревью issue #339 — заход r1 + +Вердикт: зелёный · заход r1 · блокирующих циклов 0/2 · High: 0 · Medium: 0 + +## Скоуп + +Одна поверхность: `isDegenerateApexCorner` (`src/wall-thickness.ts`), функция, +классифицирующая острый (<15°) угол легаси-плана как вырожденный для целей +рендера контура стены (`outsetContour`/`insetContour`). Путь записи не тронут +(П1 из #329 запрещает создавать такие углы заново — правка чисто +рендер-легаси). Коммит `66bd3d69`, HEAD совпадает с веткой +`issue/339-mixed-thickness-apex`, один коммит поверх актуального `origin/dev` +(dev — предок HEAD, дерево чистое). + +Диапазон изменённых файлов по существу: `src/wall-thickness.ts` (24 строки), +`test/junction-limits.test.mjs` (+29 строк, только добавление). Остальное в +диффе — синхронизированный бандл (`dist/`, `custom_components/.../frontend`), +пересъём скриншотов и CHANGELOG (en+ru) — сопутствующие, не код. + +Трек — `trivial` (§5.1 PROCESS.md): тип bug, одна поверхность, без миграций/ +i18n/перфа/touch, ожидаемое поведение зафиксировано (§4 спеки #329 + +собственный анализ автора в S2 с точной геометрической выкладкой). Соответствие +критериям — под вопросом не было, поэтому не проверяю отдельно: сам S2-анализ +содержит доказательство (таблица смыканий), которое я перепроверил численно. + +## Что закрывает задачу продуктово + +`docs/SCOPE.md` J6 — «Keep the plan true as the home evolves»: честный рендер +геометрии для существующих (легаси) планов — это условие доверия к карточке. +Баг — визуальный дефект унаследованного плана, не новая функциональность; +подходящая строка ровно эта. + +## Как проверялось + +**Математика.** Формула в PR: `alongPrev = (halfNext + halfPrev·cosθ)/sinθ`, +`alongNext = (halfPrev + halfNext·cosθ)/sinθ`, вырожденность — обе величины +внутри своих рёбер. Перепроверил численно для кейса из S2-анализа (10°, 15+30 +см на рёбрах по 160 см): + +``` +along(edge h=7.5) = 128.916 см +along(edge h=15) = 128.260 см +``` + +Совпадает с таблицей автора (128.3/128.9 см < 160 см) с точностью до +округления. Тождество со старой формулой `h/tan(θ/2)` при `h1=h2=h` +проверил алгебраически: `(1+cosθ)/sinθ = cot(θ/2)` — верно, регресс +равнотолщинных углов исключён по построению, а не только по утверждению. + +**Тест умеет падать.** Временно вернул старую версию `src/wall-thickness.ts` +(`git show origin/dev:...`), пересобрал `test-build`, прогнал новый тест +`#339` — упал ожидаемо (`false !== true`, "смешанные толщины [prev=0.0125, +next=0.00625] обязаны быть вырождены"). Восстановил рабочее дерево, тест +снова зелёный, `git status` чист. + +**Гейты, прогнанные мной лично на `66bd3d69`:** + +| Гейт | Результат | +|---|---| +| `npx tsc --noEmit` | чисто, без вывода | +| `npm test` | 1461/1461 pass, 1 skipped, 0 fail | +| `npm run build` + сверка 3 копий бандла | сборка ок; `diff dist/houseplan-card.js custom_components/.../houseplan-card.js` — идентичны; `diff dist/houseplan-assets.json ...` — идентичны; после `npm run build` `git status` чист (рабочее дерево уже содержало актуальный синхронизированный бандл) | +| `node scripts/check-docs.mjs` (diff трогает `src/**`) | "Documentation checks passed (7 files, 10 external links)" | +| `npm run golden:verify` (диф меняет геометрию рендера) | полный прогон, все сцены `passed`, 0 `failed`, exit code 0; включая `sharp-apex-legacy-dark` (равнотолщинная контрольная сцена #329 — подтверждает отсутствие регресса) | +| Мутационная проверка нового юнита | тест падает на дооткатной версии функции, зелёный на текущей — не тавтология | + +Порядок был непустой: `demo/srv/assets` в репозитории не хранится, и первый +прогон `golden:verify` упал таймаутом на `Failed to fetch dynamically +imported module` — потребовался `npm run bundle:sync`, чтобы наполнить +рабочую копию стенда. Это состояние окружения ревью, не находка о ветке. + +**Инварианты модели (`npm run invariants -- --config ...`) — не прогонял.** +Обоснование: изменённая функция работает исключительно внутри +`outsetContour`/`insetContour` — строит точки контура для рендера, не +трогает ключи рёбер (`keyOf`), записи толщины или `WallBodyPath.key`/`cm` +(они вычисляются отдельно, до вызова `isDegenerateApexCorner`, и не зависят +от неё). Три вопроса, на которые отвечают инварианты — исчезновение записи +толщины, разрешимость ссылок, совпадение ключа записи с ключом решёточного +ребра — не про эту функцию: она ничего не пишет и не адресует по ключу, +только решает геометрическую развилку рендера. `npm test` уже прогоняет +инварианты на всех моделях проекта и прошёл (см. выше). + +**Смоки `demo/smoke_*.mjs` — не прогонял ни одного вручную.** +`node scripts/smoke-select.mjs --base origin/dev --head HEAD` вернул +НЕОПРЕДЕЛЁННОСТЬ (0 связанных символов, порог широкого символа — 39). Решение +за мной: полный `golden:verify` уже пиксельно проверяет рендер стен по всей +матрице (включая профильную сцену `sharp-apex-legacy-dark` и десяток +`junction-*`/`opening-*`/`wall-*` сцен с разными толщинами и углами) — это +более сильная и более прямая проверка именно этой функции, чем интерактивные +браузерные смоки (`real_plan_masonry`, `resize_pointer_real_plan`, +`plan_drawing_repairs`, `junction_limits`), которые автор прогнал вручную и +на которые я не полагаюсь без независимого доказательства. К тому же +интерактивные смоки идут через путь записи, а он не может создать угол <15° +(П1 из #329) — значит, воспроизвести именно легаси-конфигурацию 15+30 см +через них нельзя в принципе, они бы проверяли не тот код. Юнит-тест +считает точную геометрию напрямую, golden проверяет пиксели напрямую — этого +достаточно для узкой чистой функции. + +**`npm run golden:capture`/`accept`, `python -m pytest tests_backend`, +performance-профили** — не прогонял: diff не трогает `custom_components/**/ +*.py`, AC не называют перф, `golden:verify` (без `capture`) — правильный режим +для проверки отсутствия регрессии, а не пересъёма эталонов. + +**«Одно число — один источник»** — не применимо: диф не добавляет и не +меняет ни одной видимой пользователю величины (не показывает толщину, +площадь, длину и т.п. — это решение внутренней геометрии контура). Новый +`test/single-source-numbers.test.mjs` не задет и не должен быть. + +## Проверка AC (из S2-анализа, зафиксированного в S5-ready) + +- **AC1** — угол 10°, смежные рёбра 15+30 см в оба порядка обхода: вырождена. + Доказано юнитом `#339: вершина 10° со смежными рёбрами 15+30 см вырождена в + оба обхода` — оба порядка (`[7.5,7.5,15]` и `[15,7.5,7.5]`) дают `true`; + плюс рендер-проверка: `outsetContour` даёт ровно одну точку внешнего + контура у вершины плана (острие в `(0,0)` с точностью 1e-9). **Выполнено.** +- **AC2** — равные толщины и обычные углы: вердикты байт-прежние, существующие + §4-юниты и golden `sharp-apex-legacy-dark` зелёные без правок. Тестовый файл + не содержит правок существующих кейсов (только добавление в конец), все + 1461 юнит зелёные, `sharp-apex-legacy-dark` — `passed` в полном + `golden:verify`. Алгебраическое тождество `(1+cosθ)/sinθ = cot(θ/2)` + подтверждает, что для `halfPrev = halfNext` формулы `alongPrev`/`alongNext` + сводятся к старой `half/tan(θ/2)` без приближений. **Выполнено.** +- **AC3** — 30° со смешанными толщинами не вырождена (пара #310), нулевая + толщина не вырождена. Оба случая — отдельные `assert.equal(..., false)` в + том же юните. **Выполнено.** + +## Трейлеры и changelog + +`Issue: #339`, `User-Visible: yes` — оба присутствуют в теле коммита +`66bd3d69`. При `User-Visible: yes` правки обязаны быть в обоих changelog в +том же коммите — проверено: `docs/CHANGELOG.md` и `docs/CHANGELOG.ru.md` +получили симметричные записи в разделе `## Unreleased` в этом же коммите. +`docs/USER-GUIDE.ru.md` не тронут и не должен быть — новой терминологии +интерфейса задача не вводит (внутренняя геометрия рендера, не новый +UX-контракт), критерию `trivial`-трека это не противоречит. + +## Что проверено и корректно + +- Формула замены математически верна и алгебраически регресс-безопасна + (равные толщины ⇒ старая формула тождественно). +- Новый тест не тавтологичен: падает на предыдущей версии функции, проходит + на текущей — проверено мутацией, а не по одному прочтению кода. +- Полный прогон golden (все сцены каталога `demo/golden/matrix.mjs`) зелёный, + включая контрольную сцену именно для этого класса дефекта. +- Бандл синхронизирован побайтово между `dist` и обеими копиями, + `check-docs` зелёный несмотря на пересъём 9 скриншотов (фингерпринт по + `src/**` учтён и принят). +- Путь записи не затронут — П1 (#329) остаётся в силе, новых углов <15° по + прежнему нельзя создать; правка не расширяет и не сужает контракт записи. + +## Чего не проверял + +- `npm run invariants -- --config <...>` на конкретной конфигурации — не + нашёл предмета: функция не адресует по ключу и не пишет записи толщины + (см. обоснование выше). Общие инварианты по всем моделям прогнаны через + `npm test`. +- Интерактивные браузерные смоки (`real_plan_masonry`, + `resize_pointer_real_plan`, `plan_drawing_repairs`, `junction_limits`) — + не прогонял сам; полагаюсь на `golden:verify` (пиксельно строже) и на то, + что путь записи, который эти смоки в основном упражняют, не может + воспроизвести исследуемый легаси-кейс. +- `golden:capture`/`golden:accept`, `pytest tests_backend`, perf-профили — + вне скоупа диффа, не прогонял намеренно (см. обоснование выше). +- Ручного тестирования в браузере не было — как и предписано для стадии code + для этого цикла: доказательство — автотест плюс чтение кода, оба выполнены. + +## Находки + +Нет. High: 0, Medium: 0. Зелёный вердикт, возврата автору не требуется, +issue вне скоупа не заводится.