From 95fba6dbabe0d172ec5b7a372eae3ac723458da9 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 28 Aug 2026 02:01:06 +0000 Subject: [PATCH] docs: review document for #331 Issue: #331 User-Visible: no --- docs/reviews/CODE-REVIEW-331-r1.md | 192 +++++++++++++++++++++++++++++ 1 file changed, 192 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-331-r1.md diff --git a/docs/reviews/CODE-REVIEW-331-r1.md b/docs/reviews/CODE-REVIEW-331-r1.md new file mode 100644 index 00000000..acb5d1fb --- /dev/null +++ b/docs/reviews/CODE-REVIEW-331-r1.md @@ -0,0 +1,192 @@ +# CODE-REVIEW-331-r1 + +Issue: [#331](https://github.com/Matysh/houseplan-card/issues/331) — «Ограничения +стыков (#329): пограничная точность даёт ложные отказы и невидимые дубли». +Заход: r1 (первый код-ревью-заход для этой задачи; отдельно был полный +цикл spec-ревью r1→r3, все находки закрыты, вердикт зелёный). +Материал: `git log --oneline origin/dev..HEAD`, `git diff origin/dev...HEAD`. +SHA материала: `550be251a68b1467940cf296189c7eab31b14181` (сверено +`git rev-parse HEAD` непосредственно перед выводом итога). + +## Скоуп + +Три продуктовых коммита поверх трёх спек-документов: + +- `58f3acbb` — шесть нормативных правок §2 спеки, симметрично в + `src/junction-limits.ts` и `custom_components/houseplan/junction_limits.py`, + плюс один catch-блок в `src/houseplan-card.ts`, i18n-ключ (en+ru), + changelog (en+ru), USER-GUIDE (en+ru), `docs/specs/329-junction-limits.md` + дополнен абзацем о точности границ, 4 новых мутанта. +- `0cdf85d9` — усиление двух мутантов, которые изначально не ловились + (честно описано автором в handoff-комментарии). +- `550be251` — регистрация smoke-link для `quantizeKeyCoord`/`INCIDENT_EPS`/ + `KEY_FACTOR` в `scripts/smoke-links.mjs`. + +Изменённых файлов класса A: `src/junction-limits.ts`, `src/houseplan-card.ts`, +`custom_components/houseplan/junction_limits.py`, `src/i18n/{en,ru}.json` — +все с трейлером `Issue: #331`, `User-Visible: yes` только там, где меняется +поведение (первый коммит); остальные два — `User-Visible: no` (тесты и +регистрация в реестре), корректно. + +Диапазон не трогает геометрическую модель (рёбра комнат, `layout`, +`marker.space`, `open_spans`, записи толщины) — это проверки-валидаторы поверх +уже существующей модели wall_segments, читающие сырые координаты. Инварианты +модели (`npm run invariants`) поэтому не гоняю — diff не задевает то, что они +проверяют (ключ узла для геометрии, ссылки на неё, ключ записи толщины); +задетый здесь ключ — внутренний ключ группировки в валидаторе, не ключ +хранения. + +## Как проверялось + +| Гейт | Команда | Результат | +|---|---|---| +| Typecheck | `npx tsc --noEmit` | чисто, без вывода | +| Unit (frontend) | `npm test` | 1424 pass / 0 fail / 1 skip (1425 всего) | +| Backend (узкий) | `python -m pytest tests_backend/test_junction_limits.py -q` | 16 passed | +| Backend (полный) | `python -m pytest tests_backend -q` (после `pip install pytest pytest-asyncio pytest-homeassistant-custom-component home-assistant-frontend` — в среде их не было) | 426 passed, 1 skipped, **1 error** — `test_ha_upload.py::test_upload_ok`, `AssertionError` на `threading._DummyThread`/`waitpid-` в teardown `_run_safe_shutdown_loop`. Файл не в diff (`git diff origin/dev...HEAD -- tests_backend/test_ha_upload.py` пуст), сбой воспроизводится и на изолированном прогоне одного файла — это особенность песочницы (наименование потоков в Python 3.12 внутри `pytest-homeassistant-custom-component`), не регрессия этой задачи. Не блокирует. | +| Build | `npm run build` | собрано; `dist/houseplan-card.js` побайтово идентичен закоммиченному (`git status` чист после сборки) | +| Три копии бандла | `diff dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` до и после свежей сборки | идентичны | +| check-docs | `node scripts/check-docs.mjs` (diff трогает `src/**`) | «Documentation checks passed (7 files, 10 external links)» — фингерпринт `screenshots.json` пересчитан и совпадает | +| smoke-select | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | одна «зарегистрированная связь»: `demo/smoke_junction_limits.mjs` (символы `quantizeKeyCoord`, `INCIDENT_EPS`, `KEY_FACTOR`) — это и есть AC5-смок, других претендентов нет | +| Смок AC5 | `node demo/smoke_junction_limits.mjs` (после `npm run bundle:sync` — стендовая копия бандла не коммитится) | **OK**, все поля включая `checkFailureRefusesWrite`/`checkFailureToastShown` — true | +| Смок (доп., по заявлению автора) | `node demo/smoke_island_rooms.mjs`, `node demo/smoke_room_resize.mjs` | оба OK — держат соседние сценарии барьера #329/#330, которые эта задача не должна была задеть | +| Бенч #330 | `npm run benchmark:junction-limits` | `"pass": true`, все 5 метрик внутри бюджета без правок бюджетов (`tsFullCandidateMs` 156.8/400, `pyColdValidateMs` 2264.8/5000 и т.д.) | +| Мутанты (свои, точечно) | `node scripts/mutation-gate.mjs --id=` для всех 4 новых id | `junction-limit-zero-wedge-invisible` — 1/1 поймано; `junction-limit-key-precision-lost` — 1/1; `junction-limit-branch-dropped` — 1/1; `junction-limit-candidate-fail-open` — 1/1. Для каждого лично проверено: чистый прогон зелёный, мутация красит именно названный тест | +| Инварианты модели | не гонял | diff не меняет геометрическую модель/ключи хранения — см. «Скоуп» | +| Golden | не гонял | diff не меняет рендер/геометрию/стили/слои — только вердикты валидатора и один текстовый тост | +| Полный `mutation-gate.mjs` без `--id` | случайно запущен один раз при попытке `--help`; остановлен вручную | нашёл **несвязанный** с этой задачей провал чистого прогона `smoke_resize_pointer_real_plan.mjs` (падает ДО применения любой мутации). `git diff` не касается wall-face-graph/resize-pointer кода этой задачи — похоже на особенность песочницы (Chromium/Playwright конкретной среды), а не дефект #331. Не отношу к находкам этого ревью: не смог подтвердить, что это воспроизводится вне данной песочницы, а скоуп задачи эту область не трогает | + +## Разбор по AC (спека `docs/specs/331-junction-limit-precision.md`, ревизия 3) + +- **AC1 (ключи/инцидентность).** Доказано автотестом (`test/junction-limits.test.mjs` + «#331 AC1» ×2, `tests_backend/test_junction_limits.py::test_331_debris_is_one_node_and_duplicate_is_visible`, + плюс параметр `debris-node` в паритет-фикстурах). Тест умеет падать: лично + прогнал мутант `junction-limit-key-precision-lost`, красит именно этот тест. + Отдельно проверил чтением: `quantizeKeyCoord`/`_quantize_key_coord` + математически идентичны (`sign*floor(|v|*1e7+0.5)/1e7` в обоих зеркалах, + IEEE754 double — операции побитово совпадают в TS и Python), `-0` + нормализуется явно в обеих реализациях. `INCIDENT_EPS` поднят синхронно в + обоих местах его использования (узел↔узел и узел→стена), как того требует + §2.1. +- **AC2 (0°).** Доказано автотестом (тот же файл, «#331 AC2», + `test_331_debris_is_one_node_and_duplicate_is_visible`) — тест умеет падать, + лично прогнал мутант `junction-limit-zero-wedge-invisible`. 180°-пара и + T-стык остаются чисты — проверено тем же тестом и не задетой параллельно + паритет-фикстурой `tee`. +- **AC3 (итеративность/ветви).** Доказано автотестом («#331 AC3» на обеих + сторонах, `test_331_iterative_run_and_forks`, `collinear-fork` в паритете). + Тест умеет падать — лично прогнал `junction-limit-branch-dropped`, красит + именно узел с развилкой (не переполнение). Переполнение на 10 000 атомов + проверено прямым вызовом (JS и Python) без исключения — код читал: обход + теперь `while (frontier.length)`/`while frontier:` с `visited`-множеством, + рекурсии нет ни в одной реализации. 100 развилок — таймаут-ассерт в самом + тесте (`< 500 ms`), зелёный при штатном прогоне `npm test`. +- **AC4 (дуга).** Доказано автотестом («#331 AC4»). Проверено чтением: и в TS, + и в Python `collinear(candidate, segment)` сравнивается с ВНЕШНИМ параметром + функции (базой прогона), а не с `current`/`from` — накопление поворота по + дуге устранено симметрично в обоих зеркалах. +- **AC5 (fail-closed).** Доказано смоком (`demo/smoke_junction_limits.mjs`, + секция AC5) — лично прогнал, зелёный; асимметрия (2-й вызов ломает + кандидата, 1-й — baseline остаётся fail-open) доказана мутантом + `junction-limit-candidate-fail-open`, лично прогнан, красит смок. Проверено + чтением: `_junctionLimitLabel` строит `junction.limit_check_failed` из + `violation.rule`, ключ добавлен в `en.json`/`ru.json` без плейсхолдеров + `{actual}/{limit}` — числового дублирования нет (правило «одно число — один + источник» неприменимо, значения не показываются пользователю). +- **AC6 (except, r2 M-r2-1 — два явных случая).** Доказано автотестом + `test_331_ac6_candidate_bug_raises_previous_bug_falls_back` — читал код + теста и подтвердил, что он различает стороны: мок подменяет + `commit_wall_segment_model` на всегда бросающий `TypeError`-подкласс; + случай (а) вызывает `validate_junction_limits` с немигрированным + документом-кандидатом → `pytest.raises(Boom)` — ошибка всплывает честно; + случай (б) подсовывает уже смигрированный (`model_version >= 9`) кандидат, + так что мок не задевает сторону кандидата вовсе, а немигрированный + `previous` ловит тот же мок и падает в `except Exception`, `side != "candidate"` + → тихий фолбэк, исключение не всплывает. Тест умеет падать — если + реализация перепутает стороны или уберёт асимметрию, `raise` либо появится + там, где не должен, либо пропадёт там, где обязан. Прочитал вызовы + `_migrated_spaces`: единственные два места (`old_counts`/предыдущая — без + `side`, по умолчанию `"previous"`; `new_spaces` — `side="candidate"`) — + совпадает с нормативным текстом §2.6 дословно. +- **AC7 (паритет и регресс).** Паритет-набор расширен тремя новыми фикстурами + (`debris-node`, `duplicate-wall`, `collinear-fork`), паритет-тест реально + гоняет TS через subprocess и сверяет с python — зелёный + (`python -m pytest tests_backend -q`). Полный набор юнитов/смоков #329+#330 + зелёный (см. таблицу гейтов), бенч #330 зелёный без правок бюджетов. + +## Находки + +Нет. Ни High, ни Medium, ни Low. + +Разбор был направлен на поиск расхождений (паритет TS/Python на +плавающей арифметике — источник дефектов #258/#259 в этом же классе задач), +скрытого дублирования чисел, тихого пропуска записи и нарушения границ +скоупа (#330 бюджеты, #329 инварианты §3) — ни одного не нашёл. Всё найденное +в r1–r3 spec-ревью (H1 квантование, M1 порог инцидентности, M2/M-r2-1 +асимметрия except, M3 комбинаторика, M4 USER-GUIDE, L1/L2 формулировки) +закрыто в тексте спеки и реализация её соблюдает дословно — свёл нормативный +текст §2 с кодом построчно, расхождений нет. + +Отдельно ценю самокритичность хендоффа: автор сам обнаружил и признал два +изначально «беззубых» мутанта (`junction-limit-key-precision-lost`, +`junction-limit-branch-dropped`) и коммитом `0cdf85d9` усилил тесты и патчи +мутантов, а не просто заявил «мутанты ловятся». Я перепроверил оба лично — +после усиления оба ловят 1/1. + +## Что проверено и корректно + +- Формула квантования ключа узла и её паритет TS↔Python (включая `-0` и + `.5`-тик) — прочитано и подтверждено логически идентичным по IEEE754. +- Порог инцидентности применён синхронно в обоих местах использования + (узел↔узел, узел→стена) в обеих реализациях. +- Итеративный обход по рёбрам с `visited` — код прочитан построчно в TS и + Python, рекурсии не осталось, развилки не теряются, дуга не выдаёт себя за + стену (сравнение с базой, не с предыдущим атомом). +- Fail-closed кандидата и симметричный fail-open baseline — прочитано в коде + и доказано смоком + мутантом. +- Асимметрия except по сторонам в python-зеркале — прочитано построчно и + доказано юнитом, различающим стороны. +- i18n-ключ, changelog (en+ru) и USER-GUIDE (en+ru) — в том же коммите, что + поведение; текст USER-GUIDE и текст тоста согласованы по смыслу. +- Бандл: свежая сборка байт-в-байт совпадает с обеими закоммиченными копиями. +- Скоуп не расширен: единственная правка в `houseplan-card.ts` — тот самый + catch-блок, никаких «раз уж я здесь». + +## Чего не проверял + +- **Golden-эталоны** (`npm run golden:verify`) — diff не меняет рендер, + геометрию, стили или слои, только вердикты валидатора и текст тоста; риска + визуальной регрессии не вижу. +- **`npm run invariants`** — diff не трогает геометрическую модель, рёбра + комнат, `layout`, `marker.space`, `open_spans` или ключи хранения записей + толщины; изменённый ключ — внутренний ключ группировки валидатора, не ключ + хранения. +- **Полная матрица `demo/smoke_*.mjs`** — не прогонял всю (194 смока, задача + не задевает «всё»); прогнал названный в AC5 (`smoke_junction_limits.mjs`, + подтверждён и `smoke-select.mjs` как единственная зарегистрированная + связь) плюс два соседних по барьеру #329/#330 (`smoke_island_rooms.mjs`, + `smoke_room_resize.mjs`) — оба зелёные, потому что задача меняет общий для + них код валидатора и кэш-барьер. +- **Полный прогон `mutation-gate.mjs`** без `--id` — дорогой (сотни мутантов); + прогнал точечно только 4 новых. Случайный частичный прогон всей матрицы + (при опечатке `--help`) поймал провал `smoke_resize_pointer_real_plan.mjs` + на чистом прогоне, не связанный с этим diff — не оцениваю его как находку + этой задачи (см. таблицу гейтов), но фиксирую для истории на случай, если + это не воспроизведётся в CI: обнаружено вне скоупа проверки, вне AC #331, + относится к резалку(#330-related resize pointer capture — соседняя + функциональность), не диагностировано глубже, поскольку это заняло бы + время, не относящееся к этому issue. +- **Perf-профили** сверх названного бенчем #330 — не отдельно, AC7 их + покрывает целиком. + +## Итог + +Все AC (1–7) доказаны автотестом либо смоком, для каждого лично подтверждена +способность соответствующего теста падать (через точечный прогон мутанта). +Дешёвые гейты — typecheck/test/build/check-docs — зелёные, бенч #330 зелёный +без правок бюджетов, паритет TS/Python зелёный на новых граничных классах. +Единственная аномалия (`test_ha_upload.py` в полном backend-прогоне) не +связана с diff и воспроизводится как окружение-специфичный сбой teardown, не +блокирует. + +**Вердикт: зелёный.**