From 3de2a4a4b510ef312b322dea90f86ed720487182 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 28 Aug 2026 01:16:18 +0000 Subject: [PATCH] docs: review document for #331 Issue: #331 User-Visible: no --- docs/reviews/SPEC-REVIEW-331-r1.md | 242 +++++++++++++++++++++++++++++ 1 file changed, 242 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-331-r1.md diff --git a/docs/reviews/SPEC-REVIEW-331-r1.md b/docs/reviews/SPEC-REVIEW-331-r1.md new file mode 100644 index 00000000..8f792cf7 --- /dev/null +++ b/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`