docs: review document for #239

Issue: #239
User-Visible: no

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

@@ -0,0 +1,195 @@
+# SPEC-REVIEW-239-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/239
+- **ТЗ:** `docs/specs/239-grid-scale-invariance.md`
+- **Ветка / SHA:** `issue/239-grid-scale-invariance` @ `4c065f5`
+- **Этап:** spec (PROCESS.md §2.4) · трек: обычный (`small`/`trivial` явно
+  отклонены автором в аналитике issue — несколько рендереров, новый default,
+  i18n, влияние на touch/performance)
+- **Заход:** r1 · блокирующих циклов израсходовано 0/4 до этого вердикта
+- **Ревьюер:** Claude, роль «ревьюер ТЗ»
+
+## Скоуп
+
+Оценивалось только ТЗ #239 — единый визуальный контракт «масштаб сетки
+(`cell_cm`) не должен менять внешний вид плана» плюс новый default нового
+пространства (1 см метрика / 1 дюйм imperial). Код не менялся и не
+оценивался: этап S4, реализации нет, весь диапазон `origin/dev..HEAD`
+состоит из одного docs-коммита.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (первый цикл,
+   правило «по дельте» §2.10 не применяется).
+2. Прочитано тело issue #239 и оба комментария (аналитика с оценкой
+   9/9/8/8/P1/bug/обычный трек, затем хендофф «ТЗ готово»).
+3. Прочитан файл `docs/specs/239-grid-scale-invariance.md` целиком (481
+   строка) и запись в `docs/specs/README.md`.
+4. Построчно перепроверены фактические ссылки ТЗ на код (не поверено на
+   слово — большинство утверждений §3 являются "подтверждено по коду", что
+   обязывает проверить):
+   - `src/houseplan-card.ts:14088`, `:14459` — `cellCm: 5` в manual create и
+     floors-import draft — совпадает;
+   - `src/houseplan-card.ts:20591-20595` — поле `space.scale_label` /
+     `space.scale_unit` всегда рендерит сырые см без ветвления на
+     `hass.config.unit_system.length === 'mi'` — совпадает буквально
+     (заявление ТЗ §3.3, а не догадка);
+   - `src/houseplan-card.ts:295-296` — `CELL_CM_MIN = 0.1`, `CELL_CM_MAX =
+     1000` — совпадает с §5.2/§7.1 (диапазон не расширяется);
+   - `src/render/opening-symbol.ts:34-42` — `jambHalf` fallback `4`,
+     `outlineHalf = Math.max(16, jambHalf+8, gateDepth+8)`, `hitHalf =
+     Math.max(20, ...+10, ...+12)` — совпадает с заявленными «4-12 units»;
+   - `src/wall-thickness.ts:31-43` (`HATCH_REFERENCE_CELL_CM = 5`,
+     `HATCH_BASE_STEP_UNITS = 8`) — арифметика `8 × (5/5) × (шаг сетки)` даёт
+     заявленные 9.6 см из #230 и AC5 — проверено вычислением, не с чужих слов;
+   - `src/iso-projection.ts:29-30` — `ISO_WALL_HEIGHT = 64`,
+     `ISO_FLOOR_EDGE_HEIGHT = 10` — заявленные «user units» константы
+     существуют и не параметризованы `cell_cm`, совпадает с §3.2.5;
+   - `src/houseplan-card.ts:5241,5178-5254` — `_isoGeometryCache.get(source.key)`
+     подтверждает риск «тёплый remount вернёт старую высоту», если `source.key`
+     не будет включать масштаб — корректно учтено в таблице рисков §16, а не
+     пропущено;
+   - `src/space-geometry.ts:442-478` (`iconUnit`, `iconCqw`) — числитель и
+     знаменатель оба выведены из содержимого плана в тех же plan units, поэтому
+     при физически эквивалентном плане (`k = 5/cell_cm`) множитель `k`
+     сокращается сам — заявление §3.1 «эти пути уже верны, не домножать»
+     подтверждено алгеброй, а не принято на веру;
+   - `docs/USER-GUIDE.ru.md:1092,1145` — термины «дюйм»/«фут» для физических
+     размеров уже существуют в продукте, значит предлагаемая подпись «1 дюйм на
+     клетку» не изобретает новую терминологию.
+5. Сверены §5 (входит/не входит) с §6 (классификация размеров) и §8
+   (контракт по поверхностям) на отсутствие противоречий — реестр
+   `Physical/Screen/Plan-relative/Visual unit/Grid` покрывает все пункты §3.2
+   без пропусков и без двойного назначения одного размера в два класса.
+6. Сопоставлены критерии приёмки §13 (AC1–AC16) с планом автотестов §14 —
+   не как формальность, а по прямому указанию для этого этапа: «указание
+   способа доказательства». Для сверки взяты два прецедента из этого же
+   репозитория: `docs/specs/230-hatch-density-normalization.md` §13 (план
+   тестов поимённо покрывает AC1–AC12) и `docs/specs/238-opening-inner-distances.md`
+   §15 (таблица AC с отдельной графой «Доказательство» для всех 15 AC, включая
+   мета-критерии производительности и документации — AC14 «unit со
+   счётчиком/ревью кода», AC15 «provenance + ревью кода»). См. находку ниже.
+7. Проверен гейт `node scripts/check-docs.mjs` — единственный применимый на
+   docs-only диффе класса C. `npx tsc --noEmit` / `npm test` / `npm run build`
+   не запускались: диапазон изменений не содержит ни одного файла класса A/B
+   (`git diff origin/dev...HEAD --stat` — только `docs/specs/239-*.md` и
+   строка в `docs/specs/README.md`), эти гейты ничего не проверяют на чистой
+   документации и были бы тратой времени без предмета (см. «Объём гейтов
+   соразмерен задаче»).
+
+## Находки
+
+### Medium (в скоупе задачи — исправляется в этом ТЗ)
+
+**M1. §13 Acceptance criteria не указывает способ доказательства для AC13,
+AC14, AC16, и это не восстанавливается однозначно ни из §14 «План
+автотестов», ни из таблицы рисков §16.**
+
+- AC13 (инвариант `cell_cm: 5` = pre-#239 контракт) частично закрыт: в самом
+  тексте AC упомянут «current golden», так что проверяемость угадывается, но
+  §14.3 говорит лишь «Golden baseline разработчик не принимает» — не называет
+  golden прямым доказательством AC13.
+- AC14 (touch/gesture-инвариант: pan/pinch/pointercancel не получают новых
+  действий, opening action area не меньше эталонной) не назван ни в одном
+  пункте §14, а из таблицы рисков (§16, строка «Touch target станет physical»)
+  восстанавливается только половина критерия — про размер hit-area, но не про
+  «никакой новый жест ничего не запускает».
+- AC16 (EN/RU user guide и оба changelog объясняют новый контракт) не назван
+  ни в §14, ни в release-артефактах §17 как проверяемый пункт, хотя оба
+  changelog там перечислены как артефакт.
+
+Это не придирка к форме: у этой же задачи есть два прямых прецедента в этом
+репозитории, оба прошли ревью зелёными на этом самом требовании процесса.
+`docs/specs/230-hatch-density-normalization.md` §13 поимённо связывает каждый
+AC1–AC12 с тестом. `docs/specs/238-opening-inner-distances.md` §15 — тот же
+issue-поток, тот же автор, документ на день раньше #239 — держит отдельную
+графу «Доказательство» и заполняет её даже для мета-критериев вроде
+производительности (AC14: «unit со счётчиком/ревью кода») и документации
+(AC15: «provenance + ревью кода»). #239 явно откатывается от этой практики без
+объяснения, и без неё код-ревью не имеет явного контракта: чем именно должна
+быть закрыта строка «pan/pinch не получают новых действий» — тестом или
+утверждением автора. Учитывая объём и риск задачи (сложность 8/10, риск 8/10 в
+собственной оценке автора), для «not by accident» touch-инварианта и для
+одного из главных anti-regression AC (AC13) это не косметика.
+
+**Как починить:** добавить в §13 колонку «Доказательство» (как в #238 §15)
+либо явно перечислить AC13/AC14/AC16 в §14 с указанием конкретного смока,
+unit-теста или «ревью кода» — по аналогии с уже принятым для этой же задачи
+подходом «AC15 — только ревью кода, потому что perf-гейт до беты» (эта фраза
+в документе уже есть для AC15, её не хватает для соседних AC).
+
+### Low (снимается с записью, не блокирует)
+
+**L1. §10 «Данные, i18n...» не называет конкретный текст/ключ новой imperial-
+подписи внутри самого i18n-раздела** — формулировка «новые EN/RU строки нужны
+для imperial unit label» не самодостаточна. Текст фактически дан двумя
+разделами ниже, в §9.2 («дюйм на клетку» / `in per cell`), так что
+неоднозначности для реализации нет — но это расходится с конвенцией
+остальных 239 спеков репозитория (например, #157: «`opening.passage`,
+`opening.passage_binding_warning`»; #167: «label «Plan only»»; #094:
+«`tap.toggle`: «Переключить состояние»»), где i18n-раздел сам перечисляет
+строки, а не отсылает к UX-разделу. Снимаю как Low: содержательно текст уже
+в документе, вопрос чисто в консолидации. Автор может перенести/продублировать
+строку в §10 попутно с фиксом M1 либо оставить как есть.
+
+## Что проверено и корректно
+
+- **Продуктовая рамка.** Сценарий и «что человек увидит» — обе продуктовые
+  секции §7.1 присутствуют первыми, отвечают на «какая персона / какая
+  поверхность / какой момент» и не используют термины реализации. Задача
+  закрывает J4/J6 из `docs/SCOPE.md`, конфликтов с out-of-scope нет.
+- **Ни одной догадки, выданной за факт.** Каждое диагностическое утверждение
+  §3 («что уже масштабируется правильно» / «где инвариант нарушен») проверено
+  построчно по коду (см. «Как проверялось», п.4) и совпадает буквально, а не
+  на уровне пересказа. Технические решения (§19) явно помечены как «assumed,
+  change freely» и не выданы за продуктовое решение.
+- **Продуктовых вопросов владельцу нет, и это оправдано.** Единственный
+  потенциально продуктовый вопрос — «должен ли визуально измениться уже
+  существующий сохранённый план с `cell_cm ≠ 5` после фикса» — уже
+  разрешён самим текстом issue («Нужно проверить и при необходимости
+  нормализовать весь визуальный контракт»): это и есть цель бага, не
+  побочный эффект, и AC13/§17 changelog корректно фиксируют это как
+  осознанное, объявленное в changelog поведение, а не тихую перерисовку.
+- **Скоуп/не-скоуп непротиворечивы.** §5.2 явно запрещает менять
+  `GRID_N`/`GRID_PITCH`/snap-алгоритм и migration существующих `cell_cm` —
+  это ровно то, что держит задачу в границах «визуальный слой», не
+  «координатная модель», и совпадает с диапазоном 0.1…1000, подтверждённым в
+  коде.
+- **Классификация размеров (§6) внутренне непротиворечива** и предотвращает
+  ровно ту ошибку, которую называет главным риском сам автор (общий CSS/SVG
+  transform повторно умножил бы уже верные physical/plan-relative пути) —
+  проверено алгеброй на `iconCqw`/`iconUnit` (см. «Как проверялось», п.4).
+- **AC1–AC12 однозначны и алгебраически проверяемы** без реализации:
+  например AC1 (`gridVisualScale(5) === 1`, `5/cellCm` для остальных, `1` для
+  невалидных), AC5 (physical-контроль не должен домножаться второй раз — уже
+  сформулирован как мутационный тест, а не как описание), AC9 (canonical
+  `2.54` для imperial — точное значение 1 дюйма, не приближение).
+- **Откат (§18) и release-артефакты (§17) полны**: один revert коммита,
+  явное заявление «данные не мигрируют и остаются валидными и после отката»,
+  оба changelog, оба USER-GUIDE, CANVAS.md, ARCHITECTURE.md, TESTING.md и
+  пересъёмка public docs screenshot — ничего из обязательного набора не
+  забыто.
+- **Риски (§16) содержательны, не общие слова**: каждая строка называет
+  конкретный механизм провала (например, «px/unitless `calc()` в SVG
+  вычислится не в те units») и конкретную защиту, без карго-культных «будем
+  осторожны».
+- **Гейт docs зелёный**: `node scripts/check-docs.mjs` → `Documentation
+  checks passed (7 files, 10 external links)`.
+
+## Чего не проверял
+
+- `npx tsc --noEmit`, `npm test`, `npm run build` — не запускал: диапазон
+  диффа не содержит файлов класса A/B (только `docs/specs/239-*.md` и строка
+  реестра), эти гейты не имеют предмета проверки на этом этапе и их прогон на
+  чистой документации был бы потерей времени, а не тщательностью.
+- Реализацию классов Physical/Screen/Plan-relative/Visual/Grid — кода ещё
+  нет, оценивалась только логическая непротиворечивость классификации, не её
+  будущее исполнение.
+- Полный текст остальных ~470 существующих ТЗ репозитория — выбраны только
+  #230 (единственный прямой предок контракта) и #238 (непосредственно
+  предыдущая задача в очереди, тот же автор) как контрольные образцы
+  конвенции «AC → доказательство»; это не полная статистика по всем спекам,
+  а два целевых прецедента, достаточных для находки M1.
+- Golden/browser smoke/performance — не запускались и не должны: реализации
+  нет, эти гейты относятся к S6/S7 и предрелизному прогону (PROCESS.md §8),
+  не к ревью ТЗ.