docs: review document for #331

Issue: #331
User-Visible: no

docs/reviews/SPEC-REVIEW-331-r1.md

@@ -0,0 +1,242 @@
+# SPEC-REVIEW-331-r1
+
+Issue: #331 — «Ограничения стыков (#329): пограничная точность даёт ложные
+отказы и невидимые дубли»
+ТЗ: `docs/specs/331-junction-limit-precision.md`
+Ветка/SHA материала: `issue/331-junction-limit-precision` @ `60e125e9`
+Трек: полный (не `small`; аналитик явно назвал критерий, который задача не
+проходит: «две поверхности — оба зеркала + барьер карточки», меняются
+вердикты граничных классов).
+Заход: r1 · блокирующих циклов израсходовано (до этого раунда) 0 из 4.
+
+## Скоуп
+
+ТЗ описывает шесть нормативных правок к #329 (`junction-limits.ts` /
+`junction_limits.py`): квантование ключа узла + инцидентность П4, снятие
+фильтра `degrees > EPS` в П1, итеративный обход П3 с выбором максимальной
+ветви на развилке, коллинеарность дуги к базе прогона, fail-closed на
+кандидате, сужение `except Exception` в `_migrated_spaces`. Родитель #329,
+сестринские issue вне скоупа (#333, #339) — учтены и явно исключены. Первого
+коммита с продуктовым кодом ещё нет — на ветке лежит только файл ТЗ
+(`60e125e9`, один файл, спецификация).
+
+## Как проверялось
+
+Прочитаны в порядке из инструкции: `docs/SCOPE.md`, `AGENTS.md`,
+`PROCESS.md` §2.4/§7.1, тело issue #331 и оба комментария (S2-аналитика,
+хендофф автора ТЗ), `docs/USER-GUIDE.ru.md` (раздел «Ограничения стыков
+стен»), канонический документ подсистемы — здесь это не `WALL-THICKNESS.md`
+(он про геометрию mitre/bevel, не про правила П1–П5), а собственный файл
+родителя `docs/specs/329-junction-limits.md`, использованный как эталон
+формата и терминологии. Для проверки числовых утверждений ТЗ прочитан
+текущий код: `src/junction-limits.ts`, `custom_components/houseplan/
+junction_limits.py`, `src/coordinate-canonicalization.ts` и её питон-зеркало
+`coordinate_canonicalization.py` (существующий канонический механизм
+округления координат в этом же репозитории — нужен, чтобы проверить, не
+изобретает ли ТЗ параллельный, менее надёжный способ).
+
+Гейты `typecheck`/`test`/`build` не гонялись: продуктового кода в диффе нет,
+дифф — исключительно новый файл `docs/specs/331-*.md` (класс C). Это стадия
+`spec`, а не `code`; §8/§10.2 PROCESS.md к ней не применяются.
+
+## Находки
+
+### High
+
+**H1. Ключ узла квантуется через `round()`, а не через уже принятый в этом
+репозитории cross-language-safe механизм — риск разойтись между TS и
+Python ровно на точке .5-кванта.**
+
+`docs/specs/331-junction-limit-precision.md:35` предписывает: «Ключ узла —
+координаты, квантованные к **1e-7** (`round(v·1e7)/1e7`)... формат строки
+ключа одинаков в TS и python». Формулировка не называет, какой именно
+`round` имеется в виду, и умалчивает про то, что нативные функции округления
+в TS и в Python **расходятся на .5-тиках**: JS `Math.round` всегда округляет
+дробную половину в сторону `+∞` (`Math.round(2.5) === 3`,
+`Math.round(-0.5) === -0`), а Python `round()` — банковское округление к
+чётному (`round(2.5) == 2`, `round(0.5) == 0`).
+
+Это не гипотеза, а задокументированный факт этого же репозитория:
+`src/coordinate-canonicalization.ts:47-53` (`canonicalizeNumber`) и её
+зеркало `custom_components/houseplan/coordinate_canonicalization.py:20-31`
+(`canonicalize_number`) намеренно НЕ используют `round`/`Math.round` —
+обе стороны считают `sign * floor(abs(value) * FACTOR + 0.5) / FACTOR`,
+и питоновский файл прямо комментирует причину в
+`coordinate_canonicalization.py:45`: `# JavaScript Math.round: ties go
+toward +infinity, unlike Python round().` Тот же файл содержит второй метод
+(`canonicalize_lattice_coordinate`) — оба используют идентичную формулу
+именно ради паритета на тиках.
+
+Конкретное воспроизведение расхождения для формулы ТЗ «`round(v·1e7)/1e7`»,
+если её реализовать буквально нативными функциями:
+`v = 0.00000025` (2.5e-7 в нормализованных координатах — не экзотика, а ровно
+середина между двумя соседними квантами, куда попадают, например,
+масштабированные «круглые» координаты после `cmToUnits`). `v·1e7 = 2.5`.
+`Math.round(2.5) = 3` → ключ `3e-7`. Python `round(2.5) = 2` → ключ `2e-7`.
+**Один и тот же candidate получает разные ключи узла на фронте и на
+бэкенде** — то есть ровно тот класс дефекта, который в этом же ревью
+явно назван дважды дорогим (#258, #259: ключ записи толщины не совпал с
+ключом решёточного ребра). AC1 требует паритет «через паритет-набор»,
+но паритет-набор проверяет исходы, а не формулу; конкретно эта пара тиков
+в него может не попасть, и тогда разъезд ключей останется незамеченным
+до продакшна — ровно тем способом, каким приходили #258/#259.
+
+Правка ТЗ: заменить `round(v·1e7)/1e7` на явную, одинаковую в обеих сторонах
+формулу с зафиксированным направлением округления половинки (например,
+переиспользовать уже принятый в проекте `sign * floor(abs(v)*1e7+0.5)/1e7`
+из `coordinate-canonicalization`, а не изобретать новую). Замечание в
+скоупе задачи, чинится тем же ТЗ.
+
+### Medium
+
+**M1. Собственный пример AC1 не проходит собственный порог инцидентности.**
+
+`docs/specs/331-junction-limit-precision.md:44` формулирует инцидентность
+П4 так: «пары узлов с евклидовой дистанцией **≤ 1e-7** считаются ОДНИМ
+узлом... (например `−5.1e-8` и `5.1e-8`)», и AC1 (`:98`) повторяет этот же
+пример как «пара «через границу кванта» — ноль [нарушений]».
+
+Арифметика: `|5.1e-8 − (−5.1e-8)| = 1.02e-7`, что **строго больше** 1e-7.
+По букве правила эта пара НЕ инцидентна (дистанция не ≤ 1e-7), а значит по
+П4 это два разных узла на дистанции 1.02e-7 нормализованных единиц —
+величина на четыре порядка меньше реального порога 5 см (≈4e-4), то есть
+ровно тот случай, который правило обязано отклонить как «слишком близко».
+Реализованная строго по тексту формула провалит собственный пример AC1: либо
+порог должен быть не `≤1e-7`, а как минимум `≤1.02e-7` (например `2×` кванта,
+что заодно естественно закрывает произвольную пару тиков по разные стороны
+границы округления), либо числа примера должны быть скорректированы
+(например `−4.9e-8`/`4.9e-8`, дистанция `9.8e-8 ≤ 1e-7`). Как написано,
+AC1 неоднозначен ровно на том примере, который должен его иллюстрировать.
+
+**M2. Не решено, распространяется ли fail-closed §2.6 на сторону
+`previous`, и не создаёт ли это новый способ отклонить непричастную запись.**
+
+`_migrated_spaces` (`custom_components/houseplan/junction_limits.py:300-330`)
+вызывается для ОБЕИХ сторон сравнения: `_migrated_spaces(previous)`
+(baseline) и `_migrated_spaces(config)` (candidate). ТЗ §2.6 сужает
+`except Exception` до `(WallSegmentMigrationError, ValueError)` для этой
+функции целиком, не различая вызовы.
+
+Для candidate это соответствует уже принятому в этом же ТЗ принципу
+fail-closed (§2.5). Но для `previous` ТЗ прямо в соседнем пункте (§2.5)
+формулирует другой принцип: «Baseline остаётся fail-open (недоказуемое
+наследование не повод отклонять запись) — асимметрия сознательная и
+комментируется в коде». §2.6 эту асимметрию не упоминает вовсе. Если
+`_migrated_spaces(previous)` теперь бросает TypeError/RecursionError на
+каком-то экзотическом, но уже существующем (то есть когда-то принятом)
+документе, а не глотает его — запись, вообще не касающаяся геометрии стен,
+будет честно отклонена ошибкой WS. Это именно симптом «легитимная запись
+отклонена», ради устранения которого заведена вся задача (P1, дважды
+ударивший по бетам — #316, #319), только теперь по новой причине.
+
+Может быть, это осознанный выбор (surfacing реальных багов миграции важнее
+доступности) — но тогда он должен быть назван явно, как назван для §2.5, а
+не выведен по умолчанию из общей формулировки. AC6 (`:113`) не говорит, к
+какой стороне (previous/candidate/обеим) относится тестовый документ с
+TypeError — без этого AC не проверяем однозначно.
+
+**M3. Максимальная ветвь ищется DFS-перебором вариантов; наихудшая
+сложность не оценена и не застрахована стресс-тестом, при жёстком
+требовании удержать бюджет бенча #330.**
+
+§2.3 (`:58-60`): «При развилке из нескольких коллинеарных продолжений одной
+толщины берётся максимальная ветвь (DFS по вариантам, visited на рёбрах)».
+Валентность узла ограничена П2 (≤6), но это ограничивает только
+ветвление В ОДНОМ узле, а не число узлов-развилок вдоль цепочки: документ,
+где подряд идут K развилок по 2-3 равноценные по толщине коллинеарные
+продолжения, даёт полный перебор комбинаций порядка `2^K`..`3^K`, а не
+линейную сложность. Это тот же класс входа, что уже один раз уронил П3
+(рекурсия на длинной цепочке, #331 п.3) — только вместо глубины стека
+уязвимое место сместилось на число развилок.
+
+§3 требует бюджеты бенча #330 зелёными без изменений, AC7 — то же самое, но
+план тестов (§5) и AC3 проверяют только (а) один прямой прогон на 10 000
+атомов (проверяет только стек, без развилок) и (б) «развилку двух
+коллинеарных ветвей» — судя по формулировке, единичную, не цепочку из многих
+развилок. Ни один AC/мутант не нагружает именно комбинаторику перебора.
+Это не значит, что реализация обязательно медленная — но ТЗ утверждает
+бюджет бенча гарантированным, не предъявляя для этого доказательства по
+самому изменяемому алгоритму.
+
+**M4. Release-артефакты не называют `docs/USER-GUIDE.ru.md`.**
+
+§6 «Release» перечисляет только CHANGELOG. Новый канал отказа —
+`junction.limit_check_failed`, тост при поломке САМОЙ проверки — это новое
+наблюдаемое пользователем состояние (запись отклонена без названия
+конкретного правила П1–П5). Раздел «Ограничения стыков стен» в
+`docs/USER-GUIDE.ru.md:420-444` уже документирует ровно этот класс
+информации (пороги, где виден отказ, чем отличаются каналы рисования/
+Resize/«Толщины») — тем самым методом, каким его заводил #329 (`docs/
+specs/329-junction-limits.md` §6: «раздел об ограничениях рисования»). ТЗ
+формально можно закрыть без этой правки (гейт `check-docs.mjs` её не
+поймает — там нет `src/**`-диффа для этой строки), но тогда справочник
+опишет отказы неполно. Фиксируется одной строкой в §6 ТЗ; не блокирует —
+можно решить и отклонить с запиской, если авторы считают ошибку проверки
+внутренним деталем, а не пользовательским контрактом.
+
+### Low
+
+**L1. §1 не отвечает на два обязательных продуктовых вопроса ТЗ в открытой
+форме — какая персона, на какой поверхности, и что человек видит одной
+фразой без терминов реализации (PROCESS.md §7.1, AGENTS.md).**
+
+Раздел «Сценарий и пользовательский результат» и особенно строка
+«Результат: ...вердикты честные и одинаковые в обоих зеркалах;
+переполнений стека нет; отказ проверки — это отказ записи, не пропуск»
+написаны в терминах реализации («зеркала», «ключи узлов», «отказ
+проверки» вместо «домашний админ теряет правку плана» или подобного).
+Персона (Home admin, `docs/SCOPE.md`) и поверхность (редактор плана,
+десктоп) не названы явно, хотя подразумеваются списком каналов
+(рисование/Resize/«Толщина»). Для сравнения — принятый ТЗ-предшественник
+`docs/specs/329-junction-limits.md` §1 формулирует результат как «(а)
+нарисовать такое больше нельзя... (б) уже существующие такие планы
+рендерятся прилично» — на том же уровне детализации технической
+подоплёки, но с явным «что человек увидит». Не блокирует (стиль документа
+уже был принят один раз для #329 в этом же скоупе задач), но следующая
+редакция должна дать одно предложение такого рода.
+
+## Что проверено и признано корректным
+
+- Скоуп/не-скоуп: сестринские issue #333/#339 явно исключены, границы §3
+  чёткие, пороги П1–П5 намеренно не меняются — соответствует
+  `docs/SCOPE.md` (J6, «Keep the plan true as the home evolves»).
+- §2.2 (снятие фильтра `degrees > EPS`): прослежена текущая реализация
+  `checkNodes` (`src/junction-limits.ts:76-95`) — снятие фильтра не создаёт
+  ложных срабатываний на легитимной прямой стене через узел (ровно 2 луча
+  на 180° друг от друга всегда дают на круге две дуги ~180°, а не близкую к
+  нулю, независимо от точности координат) и на T-стыке (3+ луча, но
+  вырожденная пара образуется только у геометрически совпадающих лучей).
+  AC2 однозначен и проверяем.
+- §2.4 (коллинеарность дуги к базе): числа AC4 (30×0.9°, 20 см, 0.5° излом)
+  внутренне согласованы с описанным механизмом (накопление к базе вместо
+  предыдущего атома).
+- §2.5 (fail-closed кандидата): согласован с прецедентом #278, канал тоста
+  и ключ i18n названы, AC5 проверяем смоком/юнитом.
+- Один AC на нормативный пункт §2 (1:1, AC1↔§2.1 ... AC6↔§2.6) плюс AC7 на
+  паритет/регресс — структура полная, план тестов называет способ
+  доказательства для каждого пункта (юнит/смок/бэкенд-юнит/мутант).
+- i18n: один новый ключ, en+ru — соответствует формату остальных
+  `junction.limit_*` в `src/i18n/en.json`/`ru.json`.
+- Touch: не задет, обоснованно (валидация записи, не жест).
+- Обязательство обновить сам файл #329 (§2, абзац про квантование и 0°)
+  учтено — не заводит параллельный источник истины.
+
+## Чего не проверял
+
+- Производительность фактическая (нет кода — только оценка риска в M3 по
+  описанию алгоритма).
+- Полнота списка golden-сцен/скриншотов — не требуется на этой стадии,
+  задача не меняет рендер (только вердикты записи и один новый текст тоста).
+- Backend-паритет численно (нет кода для запуска
+  `test_parity_with_the_frontend_checks`).
+- `docs/CONFIG-COMPATIBILITY.md` — не тронут задачей и не должен быть,
+  новых полей конфига нет; проверено чтением §3 (наследование данных не
+  переопределяется).
+
+## Вердикт
+
+High (H1) блокирует переход в «Готово к разработке». Medium (M1–M4) — в
+скоупе задачи, чинятся той же правкой ТЗ без отдельного issue. Low (L1) —
+на усмотрение автора, можно снять запиской при повторном заходе.
+
+`Вердикт: красный · заход r1 · блокирующих циклов 1/4 · High: 1 · Medium: 4 → в задаче · Документ: docs/reviews/SPEC-REVIEW-331-r1.md`