From bd9409b33d4ee7b1e960b572a780151ab7a9e4d0 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Wed, 19 Aug 2026 09:48:18 +0000 Subject: [PATCH] docs: spec review r3 for #197 Issue: #197 User-Visible: no --- docs/reviews/SPEC-REVIEW-197-r3.md | 220 +++++++++++++++++++++++++++++ 1 file changed, 220 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-197-r3.md diff --git a/docs/reviews/SPEC-REVIEW-197-r3.md b/docs/reviews/SPEC-REVIEW-197-r3.md new file mode 100644 index 00000000..c40e6102 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-197-r3.md @@ -0,0 +1,220 @@ +# SPEC-REVIEW-197-r3 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/197 +- **ТЗ под ревью:** [`docs/specs/197-junction-patch-fail-dark.md`](https://github.com/Matysh/houseplan-card/blob/issue/197-junction-patch-fail-dark/docs/specs/197-junction-patch-fail-dark.md) + (коммит `b203e8f`, «r2» по внутренней редакции автора), обычный трек — + не `small`/`trivial` +- **Роль:** ревьюер ТЗ (не автор), этап `S4-spec-review` +- **Трек:** обычный, лимит циклов ревью ТЗ — 4 (§4 PROCESS.md) +- **Цикл:** r3/4 (эта сессия проверяет редакцию, отправленную автором в ответ + на [`SPEC-REVIEW-197-r1.md`](https://github.com/Matysh/houseplan-card/blob/issue/197-junction-patch-fail-dark/docs/reviews/SPEC-REVIEW-197-r1.md)) + +## Скоуп ревью + +ТЗ #197 предлагает исправление: один вырожденный `virtualJunctionPatches()` +patch (IEEE-754 шум в вершинах митры на виртуальном T-стыке) обнуляет весь +результат `wallBodiesGeometry()`, поскольку цикл добавления junction-патчей +(`src/wall-thickness.ts:1760-1761`) — в отличие от room-ring и wall-edge циклов +рядом — не изолирует отказ одного элемента. Решение: численная стабилизация +координат patch + транзакционный per-patch fallback, без изменения +persisted-схемы, i18n, touch-контракта, оптимизатора и без правки #198/#199. + +r1 вернул ТЗ красным с единственной High-находкой: центральное фактическое +утверждение §3 («fixture из issue → ровно один patch → `null`») не +воспроизводилось независимым исполнением того самого вызова, который ТЗ +описывало как «подтверждено исполнением» — из-за не проговорённого координатного +контракта `WallEntry.a/b` (persisted config coordinates, а не render-scaled). +Ревью этой сессии — целиком про то, снимает ли текущая редакция именно эту +находку, а не про повторный обход всего ТЗ: раздел `## 5–10, 13–16` не менялся +между `fe7b28f` и `b203e8f` (`git diff fe7b28f b203e8f -- docs/specs/197-junction-patch-fail-dark.md` +— правки ограничены новым §0, дополнениями в §3 и переформулировкой AC1/AC2), и +все находки r1 «что проверено и корректно» по этим разделам остаются в силе без +повторной проверки. + +Не в скоупе ревью: продуктовый код. На ветке `issue/197-junction-patch-fail-dark` +всё ещё нет коммитов класса A — только документация (ТЗ, документы ревью). +Гейты (`typecheck`/`test`/`build`) не прогонялись — на этапе ревью ТЗ +продуктового кода не существует, что вне скоупа этапа (PROCESS.md §2.4/§8). + +## Как проверялось + +1. Прочитан ответ автора на r1 (issue-комментарий и новый §0 ТЗ) и весь diff + `fe7b28f..b203e8f` для `docs/specs/197-junction-patch-fail-dark.md` + (`git diff fe7b28f b203e8f -- docs/specs/197-junction-patch-fail-dark.md`) — + изменения ограничены §0 (новый), дополнением к §3 (исполняемый reproducer, + разбор #201) и переформулировкой AC1/AC2; остальные 12 из 17 разделов + ТЗ побитово идентичны версии, уже прочитанной в r1. +2. **Независимое повторение исправленного reproducer.** Собран `test-build/` + (`npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs`, зелёно). + Выполнен ровно тот код из §3 ТЗ: `rooms[].poly` и `open_spans` масштабированы + через `NORM_W`, `walls[].a/b` переданы **без** предварительного умножения + (`structuredClone(fixture.walls)`), fixture взят программно из тела issue + (`gh issue view 197 --json body`, тот же JSON-блок). Результат: + `counts: [8, 25, 3]`, `nodeCm: [20, 20, 20, 22, 20]` (узел, где патч + строится, содержит `20`, как заявлено), `failed: true` — `wallBodiesGeometry()` + вернул `null` на `HEAD` (`b203e8f`, `src/` идентичен `19e92e0`). **High-1 из + r1 воспроизведён устранённым** — ровно тем способом, что описывает §3. +3. **Проверка patch-list напрямую.** Временно экспортирован (только в + сгенерированном `test-build/wall-thickness.js`, не в `src/`) + `virtualJunctionPatches()` без изменения тела и вызван с тем же + подготовленным fixture. Результат — **ровно один** patch с вершинами + `[[620.8333333333334,550],[612.5,550],[612.5000000000001,541.6666666666665], + [620.8333333333334,541.6666666666666]]` — **побайтно совпадает** с числами, + приведёнными в §3 ТЗ. +4. **Проверка отделения #201.** Прочитан `src/wall-thickness.ts:1549-1607` + (`virtualJunctionPatches`): функция строит `unique` map через + `wallIntervals(...)`, `thicknessCmAt()` не вызывается вовсе — подтверждает + заявление §0/§3 буквально. Дополнительно узел `y=550` в исполненном + `nodeCm` содержит только ненулевые толщины (`20, 20, 20, 22, 20`) — при живом + дефекте #201 (`thicknessCmAt() → 0`) он не проявляется на пути, которым + реально идёт `virtualJunctionPatches()`. +5. **Проверка дополнительных диагностических утверждений §3** — исполнено + отдельными вариациями того же скрипта: + - без `open_spans` (отключает `virtualJunctionPatches()`, т.к. функция + возвращает `[]` при пустом `openCuts`) → `wallBodiesGeometry()` **не** + `null` — подтверждает «без open cuts валидная geometry»; + - удаление 15-см wall-записи (микро-интервал из #198) при прочем неизменном + наборе → результат остаётся `null` — подтверждает «удаление + микро-интервала не лечит», то есть #198 не является причиной; + - слияние 15-см записи с соседней 22-см (сдвиг `a` соседней записи на начало + удалённой) → результат **не** `null` (валидная geometry) — **не + совпадает** с формулировкой §3 «слияние… тоже оставляет `null`» + (см. находку Low-1). Затронутый узел `y≈0.346` физически далёк от падающего + узла `y=550`; расхождение не задевает AC1/AC2 и не меняет решение по + scope/#198. + - округление вершин patch до `1e-9` render-unit перед `union()` + (сохранён неизменным весь остальной алгоритм, правка только в + сгенерированном `test-build`, не в `src/`) → `wallBodiesGeometry()` + **не** `null`; площадь результата (`geometryArea`, shoelace по всем + кольцам) — `124991.31944444453`. + - тот же fixture, но патч **пропущен** целиком (union бросает исключение — + без округления — и код просто продолжает со старым `body`, не + переприсваивая) → тоже валидная geometry, площадь **той же самой** + величины `124991.31944444453` — подтверждает буквально заявление §3 + «пропуск и успешный стабилизированный union… дают одинаковую площадь» + (то же самое, что закреплено как предположение §17.5). +6. Перечитаны §7–§17 ТЗ на предмет того, не появилась ли новая догадка, + выданная за факт, вместе с добавленным текстом §0/§3 — не появилась: новые + утверждения либо подтверждены исполнением (см. п.2–5), либо, для случая + Low-1, являются иллюстративной деталью причинного разбора вне AC. +7. Проверено, что новая ссылка `#201` в шапке ТЗ и её описание в §0 согласуются + с фактическим issue #201 (`gh issue view 201`) — тип/скоуп/статус + соответствуют тому, как их описывает ТЗ. +8. AC1/AC2 в новой формулировке прочитаны на однозначность: обе теперь прямо + отсылают к исполняемому reproducer §3 и указывают точные числа (8/25/3, + 20-см intervals, координаты patch) — в отличие от r1, где текст допускал + неоднозначную интерпретацию масштабирования `walls[].a/b`. + +## Находки + +### Low-1 — одно диагностическое утверждение §3 не воспроизводится буквально при разумной интерпретации операции «слияние» + +**Файл:** `docs/specs/197-junction-patch-fail-dark.md`, §3, абзац +«Дополнительные исполняемые проверки» — предложение «слияние микро-интервала с +соседним участком 22 см оставляет результат `null`». + +**Что заявлено.** В числе проверок, отделяющих истинную причину (float-шум +митры на узле `y=550`) от смежных находок, ТЗ утверждает, что слияние +15-см wall-записи (`key: "0.887500,0.345833@0.0000"`, микро-интервал из #198) +с соседней 22-см записью (`key: "0.933333,0.345833@0.0000"`) **не** устраняет +крах — результат остаётся `null`. + +**Что получено при независимом исполнении.** При разумной трактовке «слияния» +(удаление 15-см wall-записи, расширение соседней 22-см записи так, чтобы её +`a` совпал с началом удалённой — то есть физическая длина участка сохраняется, +запись становится одной вместо двух) `wallBodiesGeometry()` на этом входе +возвращает **не** `null`, а валидную geometry. Узел, к которому относится +слияние (`y ≈ 0.346`), не совпадает с падающим узлом (`y = 550`) — то есть +слияние в другом месте плана меняет результат в узле, который ТЗ не трогает +напрямую; это согласуется с общим тезисом ТЗ о чувствительности `union()` к +порядку и числу вызовов, но означает, что конкретная фраза «оставляет `null`» +не буквально верна для по крайней мере одного разумного прочтения «слияния». + +**Почему это не блокирует.** Утверждение не привязано к AC1–AC11: оно — +иллюстративный аргумент, показывающий, что #198 (микро-интервал) не является +причиной #197, и этот аргумент уже независимо подтверждён другим, точно +описанным тестом того же абзаца («удаление 15-см записи оставляет `null`», +воспроизведено буквально, см. «Как проверялось», п.5). Решение вынести #198 из +скоупа не опирается только на фразу про «слияние» и не меняется от её +уточнения. Ни один AC не использует формулировку «слияние» как критерий +доказательства. + +**Рекомендация.** Low, не блокирует. Снимаю запись самостоятельно: автор может +либо удалить эту конкретную фразу (аргумент о «удалении» уже достаточен и +воспроизведён точно), либо заменить её на точный код операции, если хочет +сохранить оба довода. Ни то, ни другое не меняет AC, scope или вердикт. + +## Что проверено и корректно + +- **High-1 из r1 закрыт исполнением, а не словами.** Новый §0 не просто + возражает, а вставляет ровно исполняемый reproducer (программно извлекающий + fixture из тела issue), и этот код при независимом запуске на HEAD этой + ветки даёт заявленные числа буквально: `[8, 25, 3]`, узел содержит `20`, + `failed: true`. Координатный контракт (`walls[].a/b` — persisted config + coordinates, `entrySpan()` сам умножает на `coordScale`) подтверждён чтением + `src/wall-thickness.ts:124-129` — не декларация, а факт кода. +- **Ровно один patch с точными вершинами.** Прямой вызов + `virtualJunctionPatches()` на подготовленном fixture даёт один patch, + совпадающий с приведёнными в §3 числами вплоть до последнего знака. +- **Разделение с #201 подтверждено чтением и исполнением.** + `virtualJunctionPatches()` использует `wallIntervals()`, не `thicknessCmAt()` + — единственный путь, которым #201 мог бы влиять, физически не существует в + этой функции; на исполненном узле все толщины ненулевые. +- **Техническая состоятельность предлагаемого решения.** Проверена + экспериментально (round-трюк на вершинах patch перед `union`) — устраняет + крах на этом самом fixture и даёт ту же площадь geometry, что и вариант + «пропустить упавший patch, оставить предыдущий body» (§17.5 — предположение, + подтверждено численно, не только логически). Значит контракт §7.1/§7.2 и + архитектурное решение §8 не являются недостижимой абстракцией — они реализуемы + на реальных данных этой задачи. +- **AC1/AC2 однозначны и доказуемы.** Переформулировка убирает ту двусмысленность, + которая в r1 не позволяла независимо повторить сценарий: числа, порядок + масштабирования и точный reproducer теперь в тексте ТЗ, а не только в голове + автора. +- **Оставшиеся разделы ТЗ (не тронутые r2) остаются в силе.** Полнота + обязательных разделов §7.1 PROCESS.md, продуктовый контракт со ссылкой на + `docs/specs/141-wall-junctions.md`, границы с #198/#199, трек (обычный), + технические предположения §17, i18n/touch/миграция/откат/release-артефакты — + всё это не менялось между r1 и r2 и было проверено в + `SPEC-REVIEW-197-r1.md`; повторная проверка этой сессией подтверждает, что + добавленный текст не противоречит и не меняет эти разделы. +- **Открытых продуктовых вопросов к владельцу нет.** Единственная находка этой + сессии (Low-1) — техническая деталь диагностического текста, не продуктовый + вопрос («что видит пользователь» и «объём видимых изменений» здесь не + затрагиваются). + +## Чего не проверял + +- **AC3–AC11 на уровне реального прогона.** Не запускал `npm test`/`npm run + build`/browser smoke — на этапе ревью ТЗ продуктового кода нет (ветка несёт + только документацию), прогон гейтов реализации вне скоупа этапа + (PROCESS.md §2.4/§8). Формулировки этих AC не менялись между r1 и r2 и уже + оценивались в `SPEC-REVIEW-197-r1.md` на однозначность текста. +- **Полный causal chain и корректность самого #201** — не проверялся заново; + эта сессия только подтвердила, что #201 не влияет на путь + `virtualJunctionPatches()`, а не то, чем именно вызван сам `thicknessCmAt() → 0` + (это аналитика #201, отдельная задача). +- **Матрица численных вариантов AC2 (`x`, `x ± ulp`, permutation) в полном + объёме** — воспроизведён только сам факт «один patch с этими вершинами» и + «округление устраняет крах», не вся заявленная матрица инвариантности + (перестановка записей, направление сегментов) — она относится к реализации и + будет доказана unit-тестом AC5, не спецификацией. +- **golden/визуальная сцена (AC9), performance (§13)** — не оценивались: на + этапе ТЗ артефактов не существует, и раздел не менялся относительно r1. + +## Вердикт + +Вердикт: зелёный · цикл r3/4 · High: 0 · Medium: 0 · Low: 1 (Low-1, снята +записью в этом документе, не требует issue) + +**Зелёный.** r2 закрывает единственную High-находку r1 не декларативно, а +исполняемым reproducer, который я независимо повторил и получил числа, +буквально совпадающие с заявленными в §3 и AC1/AC2 — включая точные вершины +единственного patch и byte-level совпадение результата `null`/не-`null`. +Разделение с #201 подтверждено и чтением кода, и исполнением. Техническая +состоятельность предложенного решения (численная стабилизация ⇒ тот же +результат, что и локальный skip) проверена экспериментально на этом самом +fixture. Единственная находка (Low-1) — неточность одной иллюстративной фразы +в диагностическом тексте §3, не привязанной к AC и не влияющей на scope; +снимаю её без issue, с рекомендацией автору поправить формулировку в следующей +редакции документации (не блокирует переход в `S5-ready`).