mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 19:58:50 +00:00
docs: review document for #331
Проверка (CI) / Переиспользование: это дерево уже проверено (push) Successful in 53s
Проверка (CI) / Предполётные проверки: документация, провенанс, процесс (push) Failing after 1m16s
Проверка (CI) / Классификация изменённых файлов (push) Successful in 1m1s
Проверка (CI) / HACS: валидация репозитория (push) Failing after 17s
Проверка (CI) / Hassfest: манифест интеграции (push) Failing after 16s
Проверка (CI) / Фронтенд: типы, юниты, мутанты, синхрон бандла (push) Failing after 7m11s
Проверка (CI) / Смоки в браузере (шард 1 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 2 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 3 из 3) (push) Skipped
Проверка (CI) / Смоки: все шарды зелёные (push) Skipped
Проверка (CI) / Golden-кадры против принятых эталонов (push) Skipped
Проверка (CI) / Перф-смок: бюджет времени кадра (push) Skipped
Проверка (CI) / Бэкенд: pytest в Home Assistant (push) Failing after 9m5s
Проверка (CI) / Переиспользование: это дерево уже проверено (push) Successful in 53s
Проверка (CI) / Предполётные проверки: документация, провенанс, процесс (push) Failing after 1m16s
Проверка (CI) / Классификация изменённых файлов (push) Successful in 1m1s
Проверка (CI) / HACS: валидация репозитория (push) Failing after 17s
Проверка (CI) / Hassfest: манифест интеграции (push) Failing after 16s
Проверка (CI) / Фронтенд: типы, юниты, мутанты, синхрон бандла (push) Failing after 7m11s
Проверка (CI) / Смоки в браузере (шард 1 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 2 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 3 из 3) (push) Skipped
Проверка (CI) / Смоки: все шарды зелёные (push) Skipped
Проверка (CI) / Golden-кадры против принятых эталонов (push) Skipped
Проверка (CI) / Перф-смок: бюджет времени кадра (push) Skipped
Проверка (CI) / Бэкенд: pytest в Home Assistant (push) Failing after 9m5s
Issue: #331 User-Visible: no
This commit is contained in:
@@ -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=<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, не
|
||||
блокирует.
|
||||
|
||||
**Вердикт: зелёный.**
|
||||
Reference in New Issue
Block a user