mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -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`).
|
||||
Reference in New Issue
Block a user