docs: review document for #201

Issue: #201
User-Visible: no

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

@@ -0,0 +1,202 @@
+# Ревью ТЗ — issue #201, цикл r1
+
+- Этап: `S4-spec-review` (PROCESS.md §2.4)
+- Артефакт ТЗ: [`docs/specs/201-atomic-thickness-lookup.md`](../specs/201-atomic-thickness-lookup.md),
+  коммит `7b759f3` на ветке `issue/201-atomic-thickness-lookup`
+- Issue: [#201](https://github.com/Matysh/houseplan-card/issues/201)
+- Ревьюер: Claude (роль «ревьюер ТЗ», отдельная сессия от аналитика/автора)
+- Трек: обычный (не `small`) — верно и совпадает с метками issue (`bug`, `P2`,
+  `S4-spec-review`, без `small`); сложность оценена аналитикой в 4/10, что ниже
+  порога `small` (≤3) уже само по себе, плюс задача трогает два модуля
+  (`wall-thickness.ts` и `open-spans.ts`), что дополнительно исключает лёгкий
+  трек по критерию «одна поверхность» (§5 PROCESS.md)
+
+## Скоуп ревью
+
+Оценивалось ТЗ `docs/specs/201-atomic-thickness-lookup.md` целиком: наличие
+обязательных разделов §7.1 PROCESS.md, однозначность и доказуемость AC1–AC11,
+отсутствие догадок, выданных за решённый факт, соответствие `docs/SCOPE.md`
+(job J4/J6), `docs/WALL-THICKNESS.md`, `docs/TESTING.md` и текущему коду
+(`src/wall-thickness.ts`, `src/open-spans.ts`, `test/wall-thickness.test.mjs`).
+Продуктовый код не менялся и не мог быть изменён (задача на этапе ТЗ).
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком.
+2. Прочитано тело issue #201 и оба комментария: аналитика владельца (с полным
+   подтверждением дефекта реальным исполнением на fixture #197) и хендофф
+   автора ТЗ.
+3. Прочитан весь текст ТЗ построчно, сверен с §7.1 (обязательные разделы) и
+   §2.5 (DoR-чеклист).
+4. Прочитан канонический `docs/WALL-THICKNESS.md` целиком — модель `walls`,
+   контракт «one physical stretch has one thickness», принцип «exact endpoints
+   make a thickness boundary independent of… room topology», существующий
+   testing-каталог §8.
+5. Проверена заявленная причина дефекта чтением продуктового кода:
+   `src/wall-thickness.ts:189-219` (`lookupWall`/`thicknessCmAt` — только direct
+   key либо tolerant midpoint-in-half-pitch fallback, без поиска покрывающего
+   exact span), `src/wall-thickness.ts:1008-1090` (`cmsForPoly` — уже содержит
+   ровно тот алгоритм, который ТЗ просит добавить в `thicknessCmAt`: сначала
+   узкий covering-span поиск по `entrySpan`/`distToSeg` с минимизацией
+   `spanLen - childLen`, затем by-parent fallback). Подтверждено:
+   `cmsForPoly()` действительно уже умеет то, что заявляет §3 ТЗ, и это
+   отдельный, не переиспользуемый код-путь от `thicknessCmAt()`.
+6. Проверен единственный продуктовый потребитель `thicknessCmAt()`:
+   `grep -rn "thicknessCmAt" src/` даёт ровно один вызов вне самого модуля —
+   `src/open-spans.ts:523` внутри `thicknessOnClose()`. Заявление ТЗ
+   («Единственный продуктовый consumer… — `thicknessOnClose()`») подтверждено,
+   не является догадкой.
+7. Прочитан `src/open-spans.ts:507-545` (`thicknessOnClose`/
+   `applyThicknessOnClose`) — подтверждено: при `cm === 0` от соседа функция
+   молча переходит к следующему кандидату и в итоге возвращает
+   `fallbackCm = DRAW_WALL_DEFAULT_CM`, ровно как описывает §3/§6.3 ТЗ.
+8. Проверены существующие негативные unit-тесты, которые могли бы
+   конфликтовать с новым контрактом (`test/wall-thickness.test.mjs:146,249-250,
+   1275`) — все три завязаны на отсутствие/иное происхождение записи, а не на
+   частичное покрытие exact span; новый fallback их не затрагивает.
+9. Проверено соответствие терминологии `docs/WALL-THICKNESS.md` §1 («atomic
+   collinear spans», «exact endpoints… independent of… room topology») —
+   ТЗ использует ту же терминологию, не изобретает новую модель.
+10. Проверено, что файл ТЗ и запись в `docs/specs/README.md` добавлены одним
+    коммитом `7b759f3` с трейлерами `Issue: #201` / `User-Visible: no` —
+    корректно для чистой документации без изменения поведения на этой стадии.
+
+Код не менялся, чтения было достаточно: вопрос ревью ТЗ — «выполнимо и
+проверяемо ли», а не «работает ли реализация». Дополнительно проверено, что
+предложенный контракт (§6) не является чистой гипотезой: он копирует уже
+работающий и протестированный алгоритм `cmsForPoly()`, что заметно снижает
+технический риск задачи.
+
+## Проверено и корректно
+
+- **Все обязательные разделы §7.1 присутствуют**: сценарий и персона (§1), что
+  человек увидит до/после без терминов реализации (§2), проблема и
+  подтверждённая причина (§3), scope/non-scope (§4–5), контракт поведения (§6),
+  данные/миграция (§7), UX/i18n/touch (§8), AC1–AC11 с доказательством (§9),
+  план автотестов (§10), риски (§11), rollback (§12), release-артефакты (§13),
+  явный блок принятых предположений (§14).
+- **Причина дефекта подтверждена чтением кода, а не заявлена на веру** — см.
+  «Как проверялось» пп. 5–8. Контраст «`cmsForPoly()` уже умеет находить
+  covering exact span, `thicknessCmAt()` — нет» дословно совпадает с
+  устройством кода.
+- **ТЗ корректно опровергает исходную гипотезу автора issue.** В issue
+  предполагался разрыв в общем render-path; аналитика владельца и это ТЗ верно
+  сузили дефект до одного потребителя (`thicknessOnClose`), что подтверждено
+  фактическим исполнением (аналитика: `wallIntervals()` уже возвращает 20 см
+  на fixture #197 на текущем `dev`). Разница между исходной гипотезой репорта
+  и уточнённым фактом явно проговорена в §3, а не молча подменена.
+- **Все AC пронумерованы, однозначны и несут явный способ доказательства**
+  (unit/matrix/existing regression/browser smoke/mutation gate/gates) —
+  выполняется требование DoR (§2.5 PROCESS.md). AC1–AC5 задают точную числовую
+  и структурную проверку (containment, оба scale, direction reversal,
+  permutation, legacy fallback), AC10 явно требует падающего мутанта.
+- **Не-скоуп сформулирован точно и обоснованно** (§5): explicitly исключены
+  `wallIntervals()`/`cmsForPoly()`/рендер/geometry (не трогать работающий код),
+  очистка уже сохранённых данных (#198, чужой issue), выбор ближайшего соседа
+  в `thicknessOnClose()`, default 15 см, UI Close, snapping, ownership,
+  schema/backend/migration/Optimize. Каждый пункт — реальная граница, а не
+  формальность; ни один не пересекается с AC.
+- **Защита от утечки толщины (AUD-159B6-01) явно сохранена** (§6.2) и
+  проверяется отдельным AC3 с негативной матрицей (частичный span, параллельный
+  offset, перпендикулярный сосед, malformed endpoints) — контракт не ослабляет
+  существующий инвариант, ради которого `lookupWall()` в своё время сузили.
+- **Legacy-путь не расширяется** (§6.2, AC5): key-only записи без exact
+  endpoints продолжают жить по старому direct/midpoint контракту, что
+  предотвращает недоказуемое поведение на данных, для которых покрытие нельзя
+  подтвердить.
+- **Мутационный гейт усиливает доказуемость AC1/AC6** — заявленная запись
+  `atomic-child-thickness-parent-fallback` отключает именно новый fallback и
+  обязана дать red на unit `wall-thickness` + `open-spans`, что соответствует
+  правилу 4 `docs/TESTING.md` («тест, охраняющий механизм, сопровождается
+  мутантом»).
+- **Данные/миграция/откат корректны** (§7, §12): формат `walls` не меняется,
+  миграции нет, downgrade не портит уже сохранённые планы — только ухудшает
+  наследование при следующем Close, что не является потерей данных.
+- **UX/i18n/touch раздел корректно пуст** (§8): исправление меняет только
+  внутреннее разрешение значения уже существующего действия Close, не
+  добавляет узлов, строк или новых touch-путей — это согласуется с
+  `docs/TOUCH-SUPPORT.md` (нет новой поверхности — нет нарушения контракта).
+- **Продуктовых вопросов владельцу нет, и это оправданно.** Дефект уже
+  диагностирован исполнением до постановки в ТЗ (аналитика владельца), ожидаемое
+  поведение Close «наследовать соседнюю толщину, иначе default» уже
+  зафиксировано существующим кодом/контрактом `thicknessOnClose()`
+  (`docs/WALL-THICKNESS.md` не описывает его иначе) — решать на уровне продукта
+  действительно нечего, что соответствует «принято: открытых продуктовых
+  вопросов нет» из аналитики.
+- **Технические решения корректно помечены как предположения** (§14):
+  расположение fallback внутри `thicknessCmAt()`, переиспользование текущего
+  scale-relative tolerance, конкретный файл targeted-смока. Ревьюер эти
+  предположения не оспаривает — они разумны, не влияют на AC и явно оставлены
+  свободными для реализации.
+- **Release-артефакты названы явно** (§13): оба changelog при `User-Visible:
+  yes` в implementation-коммите, `docs/TESTING.md`, unit/smoke/mutation;
+  golden/performance/migration корректно исключены с объяснением почему
+  (числовая browser-проверка уже покрывает форму, общий renderer не меняется).
+
+## Находки
+
+### Low — release-артефакты не называют обновление `docs/WALL-THICKNESS.md` §8
+
+**Файл:** `docs/specs/201-atomic-thickness-lookup.md`, раздел 13
+(«Release-артефакты»).
+
+**Суть:** канонический `docs/WALL-THICKNESS.md` §8 ведёт содержательный
+prose-каталог тестовых сценариев подсистемы и по факту обновлялся каждым
+предыдущим fix'ом этой же зоны кода — вплоть до последней строки про #197
+(«the complete #197 fixture keeps the same non-empty canonical path…»). ТЗ #201
+называет в качестве «тестового контракта» только `docs/TESTING.md` (AC9, §13),
+не упоминая `docs/WALL-THICKNESS.md` §8 вовсе, хотя новый unit/smoke-контур
+(exact-parent fallback для атомарного ребёнка, наследование в
+`thicknessOnClose()`) — ровно тот уровень детализации, который каталог §8
+исторически фиксирует.
+
+**Почему не блокирует:** `docs/TESTING.md` — процессно корректный и
+достаточный «тестовый контракт» (AC9 named и проверяем); `WALL-THICKNESS.md`
+§8 не требуется процессом как отдельный обязательный артефакт, а сам контракт
+раздела 1 этого документа («exact endpoints make a thickness boundary
+independent of… room topology») уже описывает целевое поведение в общем виде и
+не станет неточным без правки — фикс приводит `thicknessCmAt()` в соответствие
+с уже написанным инвариантом, а не меняет его. AC и корректность приёмки от
+этого не зависят.
+
+**Решение ревьюера:** снимается без правки ТЗ, с записью в этом документе
+(разрешено §2.4/§3 PROCESS.md: «Low либо правится, либо снимается решением
+ревьюера с записью»). Рекомендация для реализации: по аналогии с #197 добавить
+в `docs/WALL-THICKNESS.md` §8 одну строку про новый сценарий — дёшево и
+поддерживает историческую полноту каталога, но отдельного цикла ревью это не
+требует и не является условием зелёного вердикта.
+
+Других находок — Low, Medium или High — не выявлено.
+
+## Чего не проверял
+
+- Не проверялась реализация — на этапе ТЗ её не существует; код-ревью будет
+  отдельным циклом (`S7-code-review`) после написания кода.
+- Не запускались `npm test`/`npm run build`/смоки: код не менялся, гейты ТЗ не
+  требуют их прогона на этом этапе (документация — класс C, продуктовый код не
+  тронут).
+- Не проверялась предложенная реализация tie-break для AC4 «наиболее узкий
+  span, независимо от порядка» на конкретных крайних случаях (например,
+  два candidate exact spans с буквально идентичной длиной покрытия) — ТЗ
+  корректно требует детерминизма как свойства, а не диктует алгоритм; выбор
+  конкретного правила — техническое решение реализации и предмет код-ревью.
+- Не оценивалась реальная browser-сцена AC7/AC10 (файл смока не создан на
+  этапе ТЗ) — план автотестов (§10.2) описывает сценарий содержательно и
+  оставляет выбор конкретного файла как явное предположение (§14 п.4), этого
+  достаточно для стадии ревью ТЗ.
+- Не оценивалась производительность на реальном большом плане — риск в §11
+  обоснованно сведён к «линейный проход, вне render tick», и это утверждение
+  проверяется код-ревью, а не спецификацией.
+
+## Вердикт
+
+Все обязательные разделы на месте, все AC однозначны и снабжены способом
+доказательства, причина дефекта подтверждена чтением кода (включая то, что
+предлагаемый алгоритм уже существует и работает в `cmsForPoly()`, а значит не
+является непроверенной гипотезой), защита AUD-159B6-01 явно сохранена и
+покрыта негативным AC, non-scope точен, mutation-gate усиливает доказуемость
+ключевых AC. Открытых продуктовых вопросов нет и это оправданно — решать
+здесь действительно нечего. Единственная находка — Low, не влияющая на
+корректность контракта, снята с записью.
+
+**Вердикт: зелёный · цикл r1/4 · High: 0 · Medium: 0 → в задаче**