docs: review document for #149

Issue: #149
User-Visible: no

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

@@ -0,0 +1,264 @@
+# Ревью ТЗ — issue #149, цикл r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/149
+- **ТЗ:** `docs/specs/149-touch-view-settings-affordance.md`, коммит `2d5f0a5f0f66faf1614a9f1f367ce108011fcfbe`
+- **Этап:** spec (PROCESS.md §2.4)
+- **Трек:** обычный (не `small`/`trivial` — сам issue называет причину: новый
+  видимый элемент, таймер, снятие существующего жеста, попадание в геометрию
+  контура, затронуты все три персоны); лимит цикла — 4
+- **Вердикт:** красный · цикл r1/4 · High: 1 · Medium: 0
+
+## Скоуп ревью
+
+Прочитаны: `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1, §2.2–2.5, §3, §4,
+§7.1–7.2, §12), тело issue #149 и все три комментария владельца (аналитика с
+Q1–Q3 и defaults, решение владельца по Q1–Q3, объявление о готовом ТЗ), весь
+текст `docs/specs/149-touch-view-settings-affordance.md`, `docs/UX-MODES.md`
+(принцип режимов, политика ввода, kiosk-раздел), `docs/TOUCH-SUPPORT.md`
+(контракт, safety floor, deliberate degradation), `docs/CANVAS.md` (§1–4,
+модель content frame — чтобы не путать её с новым «physical footprint»),
+`docs/WALL-THICKNESS.md` (§1–4, модель стен/partition/column, кэширование
+структурного прохода), `docs/USER-GUIDE.ru.md` (раздел 6 «Навигация, масштаб и
+жесты», раздел 17 «Киоск-режим», полнотекстовый поиск терминов «настройки
+вида» и «киоск»), `docs/CONFIG-COMPATIBILITY.md` (чтобы подтвердить, что задача
+действительно не задевает реестр совместимости server config).
+
+Дополнительно, поскольку ТЗ описывает удаляемое/переносимое поведение как
+факт о текущем продукте, а не как предположение — прочитан сам код:
+`src/houseplan-card.ts` (`_stagePointerDown` ок. строк 5082–5104,
+`_kioskScale`/`_kioskDialog`/`LS_KIOSK` ок. строк 1551–1553, 2313–2318,
+14244–14277, применение множителя в devlayer — строка 14725,
+`_interruptViewGesture` — строка 4300), `src/i18n/ru.json`/`en.json` (ключи
+`kiosk.*`), `demo/smoke_kiosk.mjs`, `docs/CHANGELOG.md` (запись v1.41.0, где
+эта функция впервые появилась) и `docs/TESTING.md` (пункт «Kiosk mode»,
+строки 768–774).
+
+## Как проверялось
+
+1. Сверил формальные обязательные разделы ТЗ (PROCESS.md §7.1) с текстом
+   `docs/specs/149-touch-view-settings-affordance.md`: сценарий и персона/
+   поверхность/момент — §1; что человек увидит до/после — §1; скоуп и
+   не-скоуп — §4/§5; контракт поведения — §6–9; UX — §8/§10; модель данных и
+   миграция — §11; i18n — §10/§14; AC1…AC12 — §12; план автотестов — §13;
+   риски — §15; откат — §15; release-артефакты — §14. Раздела с буквальным
+   заголовком «Проблема» нет, но его содержание (почему долгий тап — плохой
+   affordance) присутствует в issue и подразумевается мотивацией §1; отмечаю
+   как Low ниже.
+2. Прогнал через код каждое фактическое утверждение ТЗ о «существующем»
+   поведении (§1, §4 п.4, §9, §11), а не поверил формулировке — именно это
+   PROCESS.md называет «догадкой, выданной за решение», и именно на это
+   ревьюер обязан ловить автора.
+3. Проверил все три owner-defaults (Q1–Q3 из комментария
+   `#issuecomment-5298450448`, подтверждённых `#issuecomment-5301106992`) на
+   соответствие тексту ТЗ §2 — совпадают буквально.
+4. Проверил геометрические термины ТЗ (§6: «physical footprint», «unbounded
+   exterior», holes/courtyard, detached porch/terrace) на отсутствие
+   конфликта с уже канонической моделью `CANVAS.md` (content frame — другая
+   сущность, используется для камеры, не для hit-теста фона) и
+   `WALL-THICKNESS.md` (thick walls, partitions, columns, кэш по structural
+   fingerprint) — конфликтов не нашёл, переиспользование корректно оговорено
+   как предположение №1 в §16.
+5. Проверил i18n-термин «Настройки вида» на предмет «терминология изобретена,
+   а не взята из USER-GUIDE» — в `docs/USER-GUIDE.ru.md` этой фразы сейчас
+   нет вообще (полнотекстовый поиск — 0 совпадений); фраза происходит из
+   заголовка/тела issue (то есть от владельца), не придумана автором ТЗ, и
+   ТЗ уже включает обновление USER-GUIDE в release-артефакты (§14) — это
+   штатный путь введения нового термина, не нарушение.
+6. Проверил каждый AC (§12, 12 пунктов) на однозначность и на то, назван ли
+   способ доказательства в §13 — не построчным сопоставлением заголовков (в
+   ТЗ они не пронумерованы 1:1 с AC), а по содержательному пересечению
+   формулировок.
+
+## Находки
+
+### High-1 — §11 утверждает как факт то, что код опровергает; §4 не включает
+необходимое для этого утверждения изменение
+
+**Что не так.** §11 «Модель данных и compatibility» пишет: «kiosk и normal
+View используют один per-screen value, **как и до изменения**» — то есть
+заявляет, что до этой задачи обычный View и kiosk уже одинаково применяют
+`_kioskScale` (масштаб иконок/текста) к рендеру. Это не так.
+
+**Воспроизведение по коду (проверено чтением, не исполнением):**
+
+- `src/houseplan-card.ts:5088` — весь блок, открывающий диалог размеров по
+  долгому тапу, обёрнут в `if (this._kiosk) { … }`. В обычном View
+  (`kiosk: false`) этот код не выполняется вовсе — то есть сегодня в обычном
+  View нет *никакого* способа, ни скрытого, ни явного, открыть этот диалог.
+- `src/houseplan-card.ts:14725` — множитель фактически применяется тоже
+  только в kiosk: `` this._kiosk ? this._kioskScale.icon : 1 `` и то же для
+  `--rl-font`. Значение читается из `localStorage` (`LS_KIOSK`,
+  строки 2313–2318) независимо от режима, но **применяется к рендеру** icon/
+  font только когда `this._kiosk === true`.
+- `docs/USER-GUIDE.ru.md:185–193` — таблица жестов документирует «Сброс
+  киоск-масштаба» и «Локальный размер киоска» строго в столбце «Сенсорный
+  экран» **киоска**, без единой строки для обычного View; это не пробел в
+  документации, это точное описание текущего продукта.
+- `docs/CHANGELOG.md:2826–2836` (v1.41.0) — функция изначально задумана и
+  описана как kiosk-only: «every tablet/TV tunes itself once». Ограничение
+  не случайно, это осознанное решение выпуска 1.41.0, а не забытый кусок кода.
+
+**Почему это блокирует, а не техническая деталь на усмотрение автора.**
+Вопрос «увидит ли пользователь эффект от сдвига слайдера в новом диалоге,
+открытом кнопкой в обычном View» — продуктовый и наблюдаемый, а не
+внутренняя реализация. Как написано сейчас, реализация по ТЗ откроет в
+обычном View **тот же диалог с теми же слайдерами**, но применение
+`this._kiosk ? … : 1` в devlayer никто не просит поменять — §4 «Скоуп» не
+называет это пунктом работы, §16 «Принятые технические предположения» не
+называет это допущением, а §11 прямо утверждает обратное факту. Результат:
+кнопка появится у домочадца и гостя на телефоне/планшете (как и требует
+owner-решение «доступны всем touch-поверхностям, отдельного поведения для
+киоска не делаем»), но движение слайдера «Размер значков устройств» ничего
+не изменит на экране — рабочий контрол, который визуально не работает. Это
+прямое попадание в `docs/TOUCH-SUPPORT.md`: «A touch-only failure in View is
+a product defect, not an accepted limitation of the editor policy» — только
+здесь дефект дня выпуска, а не более позднего открытия.
+
+Это также ровно тот класс дефекта, который PROCESS.md называет «худшим
+видом»: утверждение о поведении («используют один per-screen value, как и до
+изменения»), которого не подтверждает ни один документ и которое не помечено
+как предположение — оно читается как решённый факт и поэтому проходит ревью
+на автомате, если его не сверить с кодом.
+
+**Не эскалируется владельцу как новый вопрос.** Сам факт нужности такого
+изменения не требует решения владельца: намерение уже зафиксировано его же
+формулировкой «Настройки вида доступны всем touch-поверхностям… отдельного
+поведения для киоска не делаем» (issue, «Решения владельца»,
+подтверждено `#issuecomment-5298450448`). Контрол, открытый на всех
+поверхностях, но работающий только на части из них, — это не то, что
+владелец согласовывал; значит нужно не спрашивать, а **дописать ТЗ**: явно
+включить в §4 «Скоуп» снятие/обобщение условия `this._kiosk ?` в применении
+множителя к рендеру (или эквивалентное решение), завести под это отдельный
+AC («изменение слайдера в обычном View видимо меняет размер иконок/текста
+так же, как в kiosk») и соответствующий unit/smoke пункт в §13, и поправить
+§11, чтобы он описывал целевое, а не мнимое текущее поведение.
+
+**Серьёзность.** High — блокирует переход в `S5-ready`. Без исправления
+задача либо тихо доставит нерабочий контрол в двух из трёх персон (что
+`docs/SCOPE.md`/`TOUCH-SUPPORT.md` не допускают), либо реализация по ходу
+дела сама «дорешит» продуктовый вопрос без записи, что запрещено §7.1
+(«размытое место не додумывается, а выносится либо явно фиксируется»).
+
+## Что проверено и корректно
+
+- **Owner-решения Q1–Q3 перенесены буквально.** §2 ТЗ воспроизводит все три
+  default-а (`touch/coarse only`, 5-секундный таймер с перезапуском и
+  немедленным скрытием на pan/zoom/смену пространства, «вне плана» =
+  unbounded exterior с исключением detached porch/terrace и holes) без
+  искажений и без добавления новых решений от себя.
+- **Геометрический контракт (§6) не путает две разные модели.** «Physical
+  footprint» (архитектура: floor+walls+partitions+columns, без decor/devices/
+  Glow/sun) корректно отличается от `contentFrame` из `CANVAS.md` (камера/
+  fit, включает decor и devices) — общий пул терминов не смешан, никакой
+  скрытой ре-дефиниции существующего понятия нет.
+- **Производительность (§6.3, AC12).** Требование «топология считается
+  только при structural fingerprint change, не на pointermove/HA tick»
+  прямо аналогично уже работающей модели `WALL-THICKNESS.md` §2/§3 («one
+  cached structural pass in flat renderers; live HA state ticks do not
+  repeat the boolean topology») — это перенос уже принятого паттерна, а не
+  новая непроверенная идея.
+- **Владение жестом (§7) корректно разграничивает hit-test фона и клики по
+  интерактивным целям**, явно перечисляет device/room/opening/vacuum/header/
+  dialog как приоритетные над новым «чистым тапом», и явно требует, чтобы
+  `preventDefault()` не расширялся за пределы нового чистого тапа — это
+  прямое соответствие safety floor `TOUCH-SUPPORT.md` (`pinch`/`pointercancel`/
+  второй палец не должны читаться как клик).
+- **Права (§9).** Явно и неоднократно (§9, AC3, AC11) отделяет новый affordance
+  от административного `_settingsDialog`/«Общих настроек» и от `_canEdit` —
+  соответствует SCOPE.md (lock invariant в духе «намеренно, не по случайности»
+  распространён здесь на права: контрол для non-admin не получает доступа к
+  admin-поверхности).
+- **Accessibility (§10).** Требование «focusable button, не SVG-only»,
+  локализованные aria-label, focus trap диалога, приостановка auto-hide при
+  keyboard focus — конкретно и проверяемо, ссылается на существующий dialog
+  contract, а не изобретает новый.
+- **Совместимость (§11), помимо разобранного выше пункта.** Отсутствие
+  миграции, отсутствие новых серверных полей, отсутствие новых полей
+  `localStorage` подтверждено — задача действительно не задевает
+  `docs/CONFIG-COMPATIBILITY.md` (реестр — про server config/backend
+  validation, сюда не относится), и в «Связано» этот документ обоснованно не
+  включён.
+- **Термин «Настройки вида» не изобретён автором ТЗ** — он происходит от
+  формулировки владельца в issue и вводится в канон через release-артефакты
+  §14 (обновление `docs/USER-GUIDE.ru.md`), что штатный путь.
+- **Не-скоуп (§5) конкретен и вычёркивает реальные соседние риски**
+  (desktop mouse/hover, «Общие настройки», миграция диапазонов масштаба,
+  постоянная кнопка в header, показ по внутренним поверхностям) — не общая
+  фраза «остальное не трогаем».
+- **Откат (§15)** описан предметно: удаление временной кнопки, возврат кода
+  без миграции, совместимость `localStorage`/server config — проверяем без
+  code review.
+- **Трек и лимит цикла** выбраны верно: задача явно не `small`/`trivial` по
+  собственному разделу «Влияние» issue, лимит цикла — 4, документ лежит по
+  канону `docs/specs/149-touch-view-settings-affordance.md`.
+
+## Low — не блокируют, зафиксированы с решением ревьюера
+
+1. **Нет раздела с буквальным заголовком «Проблема».** Содержание есть
+   (в issue и в мотивации §1), но в самом файле ТЗ не выделено отдельным
+   заголовком, как того просит PROCESS.md §7.1. Снимается без правки: смысл
+   присутствует и не расходится с issue; при следующей правке (см. High-1)
+   стоит добавить подзаголовок «Проблема» из issue «Почему это стоит сделать»
+   заодно, но отдельного цикла ревью на это не требуется.
+2. **AC8, AC10, AC12 не имеют явно названного способа доказательства в §13.**
+   §13 сгруппирован по категориям (Unit / Touch smoke / Golden), а не 1:1 по
+   номеру AC, и для AC8 («desktop mouse/hover не меняется, mouse click не
+   показывает кнопку»), AC10 (accessibility/focus) и AC12 (geometry cache)
+   не нашлось однозначно соответствующего пункта ни в одном из трёх списков.
+   Не блокирует понимание контракта, но при возврате на исправление High-1
+   стоит добавить: unit-пункт на `pointerType !== 'touch'/coarse'` → кнопка
+   не показывается; smoke/unit-пункт на focus trap и aria-label; unit/perf-
+   пункт, считающий вызовы построения footprint на серию тапов без изменения
+   геометрии. Снимается автором в той же правке, без нового issue — это
+   дополнение к тесту уже принятого AC, а не отдельная находка о продукте.
+3. **Название кнопки и название диалога расходятся терминологически.**
+   Предложенный aria-label/title кнопки — «Настройки вида» / «View settings»
+   (§10), а существующий заголовок диалога в `ru.json`/`en.json`
+   (`kiosk.title`) — «Размеры на этом экране» / текущий английский эквивалент.
+   Тап по кнопке «Настройки вида» открывает диалог, который сам себя называет
+   иначе. Не мешает работе функции и не противоречит ни одному документу —
+   отмечаю, чтобы при реализации выбрали одно из двух: либо оставить как
+   мелкое, но заметное расхождение вида, либо привести название диалога и
+   aria-label к одному термину в release-артефактах (§14 это разрешает).
+
+## Чего не проверял
+
+- Реализацию — кода `src/**` под это issue ещё нет (ветка
+  `issue/149-touch-view-settings-affordance` на момент ревью содержит только
+  файл ТЗ, `git log --oneline` подтверждает единственный коммит `2d5f0a5`).
+  Проверка кода на этом этапе не требуется PROCESS.md §2.4.
+- Golden-эталоны и живые performance-профили — не существуют до реализации;
+  §13 описывает план, не результат.
+- Полный текст `docs/SUN.md`/`docs/LIGHT.md` — не читал построчно: задача не
+  трогает освещение/солнце (§5 явно перечисляет их эффекты как то, что не
+  расширяет physical footprint, но не меняет), релевантно только фактом, что
+  их эффекты не входят в hit-test, что уже подтверждено §6.1 текста ТЗ, и
+  этого достаточно для этого ревью.
+- `docs/specs/README.md` — не проверял, зарегистрирована ли запись на этот
+  документ там; при пересмотре в r2 стоит свериться, но это не продуктовая
+  находка.
+- Существующие браузерные smoke-файлы (`demo/smoke_kiosk*.mjs` и др.) прочитаны
+  только частично, ровно в объёме, нужном чтобы подтвердить/опровергнуть
+  утверждения ТЗ о текущем поведении (см. «Как проверялось» и High-1);
+  полный аудит покрытия smoke-suite не проводился — на этапе ТЗ без
+  реализации это не даёт дополнительной информации.
+
+## Вывод
+
+ТЗ методологически сильное: geometry-контракт корректно отделён от camera-
+модели `CANVAS.md`, owner-решения перенесены буквально, safety floor и права
+разобраны конкретно, откат и не-скоуп предметны. Но в основании ТЗ лежит
+фактическая ошибка о текущем продукте (§11: «kiosk и normal View используют
+один per-screen value, как и до изменения»), которую код прямо опровергает:
+сегодня масштаб иконок/текста применяется к рендеру только в kiosk, а в
+обычном View — никогда, вне зависимости от значения в `localStorage`. Раз
+owner-решение требует единого affordance на всех touch-поверхностях, эта
+задача не «делает видимым существующий скрытый вход», а частично **создаёт
+новую функцию для обычного View** — и эта часть работы не названа ни в
+скоупе, ни в AC, ни как принятое допущение. Это ровно тот guess-as-fact,
+который PROCESS.md прямо называет худшим видом дефекта, потому что он
+выглядит решением и проходит ревью, если его не сверить с исходником.
+
+Возврат в `S3-spec`. Исправление: явно добавить в §4/§12/§13 работу по
+применению множителя иконок/текста в обычном View (не только kiosk),
+поправить факт в §11, и по возможности закрыть три Low-пункта той же правкой.