docs: review document for #418

Issue: #418
User-Visible: no

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

@@ -0,0 +1,195 @@
+# SPEC-REVIEW-418-r1
+
+Issue: #418 · Этап: S4-spec-review (лёгкий трек, `small`) · Заход: r1 · Бюджет циклов: 0/2
+Ревьюер: Claude (роль «ревьюер ТЗ», сессия отдельна от автора)
+Материал: тело issue #418 на момент ревью (ТЗ живёт в теле issue, файла в `docs/specs/` нет —
+верно для `small`) плюс комментарии `Аналитика` и `Взял`. Код на `dev`
+(`b71bc39b`, ветка задачи ещё не заведена — стадия S3→S4, кода нет).
+
+## Скоуп задачи
+
+Баг-репорт + ТЗ по двум гонкам состояний в диалоге Help & Feedback (#43):
+
+1. Поздний ответ `houseplan/support/preview` может применить точную геометрию
+   после того, как пользователь снял «Приложить данные».
+2. Правка текста после неуспешной фактической отправки не меняет
+   `idempotency_key`, из-за чего relay (24-часовой дедуп, §9.2 #43) возвращает
+   исходный `report_id`, а исправленный текст не уходит.
+
+Задача не добавляет функциональность — восстанавливает уже опубликованные
+гарантии `docs/SUPPORT-PRIVACY.md` и §6.4/§6.5 `docs/specs/043-private-support-report.md`.
+Диалог помощи не входит буквальной строкой в J1–J7 `docs/SCOPE.md`, но он —
+существующая принятая поверхность (#43, ревью пройдено ранее), и задача её не
+расширяет, а чинит уже данное обещание приватности и доставки. Возражений по
+`docs/SCOPE.md` нет.
+
+Лёгкий трек (`small`) заявлен и подтверждён по факту: одна поверхность (диалог
+Help & Feedback, два файла одной фичи — `houseplan-editor-runtime.ts` и
+`support-feedback.ts`), нет миграции конфига, нет нового UX-контракта (правится
+поведение в рамках уже описанного #43 контракта, а не добавляется новое),
+нет влияния на perf/touch. Автор сам назвал, почему не `trivial`: два
+независимых, содержательно разных перехода состояния (не «одна строчка»).
+
+## Как проверялось
+
+Ревью ТЗ на этапе S4 не имеет диффа продуктового кода — задача ещё не в
+разработке. Материал ревью — текст ТЗ; проверка состояла в сверке каждого
+фактического утверждения ТЗ с текущим состоянием кода на `dev` и с
+каноническими документами, а не в исполнении тестов.
+
+Прочитано и сверено:
+
+- `docs/SCOPE.md`, `PROCESS.md` (§2.3–2.5, §5, §7.1), `AGENTS.md` — рамка
+  процесса и лёгкого трека.
+- `docs/SUPPORT-PRIVACY.md` — подтверждает «disabling the attachment... removes
+  the token», совпадает с формулировкой ТЗ.
+- `docs/specs/043-private-support-report.md` §6.4, §6.5, §9.2 — источник цитат
+  «снятие чекбокса немедленно скрывает raw preview и удаляет token», «один клик
+  создаёт один idempotency key», «idempotency key retained 24 h and returns the
+  original report id». Все три цитаты в ТЗ #418 дословно совпадают с текстом
+  канонического документа — не выдуманы.
+- `src/houseplan-editor-runtime.ts:9077-9349` — весь код диалога: `_supportPatch`,
+  `_updateSupportDraft`, `_buildSupportPreview`, `_setSupportAttachment`,
+  `_submitSupport`, `_scheduleSupportExpiry`, разметка `.supportpreview`
+  (:9436-9437).
+- `src/support-feedback.ts` целиком — `newSupportDialogState`, `supportDraftError`,
+  `supportCanSubmit`; `idempotencyKey` генерируется один раз при
+  `newSupportDialogState()` и нигде в рантайме не переприсваивается — подтверждает
+  находку (2) как реальный, не выдуманный дефект.
+- `test/support-feedback.test.mjs`, `demo/smoke_support_feedback.mjs` — существующее
+  покрытие; `retryKey`/`retryRequests` (:223, :243) уже проверяют «повтор без
+  правки сохраняет key», это ровно то, что AC3 требует не сломать.
+- `scripts/mutation-gate.mjs` — реестр мутаций существует и работает по
+  документированному паттерну «патч продуктового кода → тест обязан покраснеть»;
+  требование ТЗ «доказательство AC1/AC2 включает mutation» не изобретает новый
+  механизм, а использует принятый в проекте.
+
+**Гейты (tsc/test/build/smokes) не прогонялись** — они не относятся к материалу
+этого раунда: диффа продуктового кода нет, ветка `issue/418-*` не заведена,
+проверять нечего. Это решение, а не пропуск: на стадии S4-spec-review гейты
+запускаются в цикле реализации (S6) и на код-ревью (S7), не здесь.
+
+### Проверка конкретных фактических утверждений ТЗ
+
+1. **«Поздний ответ применяется как ни в чём не бывало»** — подтверждено чтением.
+   `_buildSupportPreview` (:9193) проверяет `current.attach` только **до**
+   `await this.host.hass.callWS(...)` (:9195). После await единственная проверка
+   в `_supportPatch` (:9089) — совпадение `draftId`, без повторной проверки
+   `attach`. Значит успешный поздний ответ действительно перезаписывает
+   `status:'idle', preview:null`, поставленные `_setSupportAttachment(false)`
+   (:9263-9269), на `status:'ready', preview:<geometry>` (:9237-9241). Реальный
+   дефект, не домысел.
+2. **«Блок `.supportpreview` завязан на `state.preview`, а не на `state.attach`»**
+   — подтверждено чтением, :9436: `${state.preview ? html\`<div class="supportpreview">...`.
+   Действительно не проверяет `state.attach`.
+3. **«Смягчающее обстоятельство»: сегодня недостижимо через UI, чекбокс задизейблен
+   во время building** — подтверждено чтением, :9422:
+   `?disabled=${busy || state.status === 'success'}`, где `busy` включает
+   `'building'`. Между `_setSupportAttachment(true)` и синхронной установкой
+   `status:'building'` внутри `_buildSupportPreview` нет `await`, то есть окна для
+   клика физически нет. Автор не преувеличил риск и честно снизил серьёзность
+   находки (1) указанием на недостижимость через штатный UI — это корректная,
+   проверяемая калибровка, а не самоцель для более высокого приоритета.
+4. **«`_updateSupportDraft` не блокирует правку в статусе `error`»** — подтверждено,
+   :9102: guard блокирует только `'building' | 'sending' | 'success'`, `'error'`
+   в нём нет — правка после неудачи действительно возможна через штатный UI (в
+   отличие от находки 1, эта реально достижима).
+5. **«Ключ идемпотентности создаётся один раз на открытие диалога» и «нигде не
+   меняется»** — подтверждено: `newSupportDialogState()` (`support-feedback.ts:57`)
+   присваивает `idempotencyKey` один раз; ни один вызов `_supportPatch` в
+   `houseplan-editor-runtime.ts` не содержит поля `idempotencyKey`. Дефект (2)
+   реален буквально так, как описан.
+6. **AC3 «retry неизменённого обращения с тем же key» уже покрыт существующим
+   smoke** — подтверждено, `demo/smoke_support_feedback.mjs:223,243`.
+
+Ни одно утверждение ТЗ не оказалось домыслом, выданным за факт: там, где автор
+делает техническое допущение (сравнение по trimmed-значениям, смена
+preview-token = смена payload, best-effort cleanup), оно явно вынесено в раздел
+«Принятые технические предположения» и помечено как решаемое реализацией
+свободно.
+
+## Находки
+
+### Low — нет отдельных разделов «Сценарий» и «Что человек увидит до/после» (waived)
+
+`PROCESS.md` §7.1 называет их первыми двумя обязательными разделами ТЗ.
+В теле issue #418 их нет как отдельных заголовков: «Проблема» написана в
+терминах кода (номера строк, имена функций), а не «какая персона на какой
+поверхности в какой момент».
+
+Решение ревьюера: **снимается без правки**. Причины:
+- §5 лёгкого трека прямо сужает обязательный шаблон тела issue до
+  «проблема · контракт · AC1…ACn с доказательством · откат» — сценарий и
+  «что человек увидит» в этот список не входят;
+- персона и поверхность здесь не могут быть предметом спора: диалог Help &
+  Feedback открывается только при `_canEdit` (:9078), то есть исключительно
+  Home admin в редакторе на десктопе — ровно то, что `docs/SCOPE.md` называет
+  единственной персоной редакторов;
+- момент и видимое пользователю поведение фактически описаны — раздел
+  «Ожидаемое поведение» в начале issue и honesty-репорт «смягчающее
+  обстоятельство» дают то же самое другими словами, включая точную оговорку о
+  недостижимости первой гонки через штатный UI.
+
+Не превращается в отдельный issue: находка внутри скоупа и ниже порога
+Medium.
+
+## Что проверено и корректно
+
+- Обе фактические предпосылки ТЗ (поздний preview-ответ, статичный
+  idempotency key) подтверждены чтением текущего кода, а не поверены на слово
+  автору.
+- Все три цитаты из #43/`SUPPORT-PRIVACY.md` дословны, не искажены.
+- AC1–AC3 однозначны, у каждого назван способ доказательства (`smoke` +
+  `mutation`/`unit`), и способ реалистичен: существующий harness
+  `demo/serve.mjs` + мокнутый `callWS` (`demo/smoke_support_feedback.mjs:46-76`)
+  уже даёт управляемые ответы на `houseplan/support/preview` и `.../submit`,
+  так что «browser smoke с управляемыми Promise» и «off → on ответы в обратном
+  порядке» реализуемы без нового харнесса.
+- AC3 явно защищает регресс существующего контракта #43 (fresh defaults,
+  exact bytes preview/download, success, manual recovery, disabled busy
+  controls, retry с тем же key, responsive 320×760/760×320) — сокращения
+  скоупа под видом починки нет.
+- Технические решения (где живёт generation-счётчик, имя fingerprint-хелпера)
+  корректно оставлены реализации с пометкой «assumed» там, где это
+  предположение, а не факт.
+- Откат — простой revert, без миграции; согласуется с тем, что
+  config/schema/backend protocol не меняются.
+- Лёгкий трек подтверждён по всем пяти критериям §5 одновременно, не только
+  заявлен.
+- Автор честно откалибровал приоритет находки (1): указал, что через штатный
+  UI гонка сегодня недостижима, не спрятал это в защиту серьёзности бага.
+
+## Чего не проверял
+
+- Гейты `tsc`/`test`/`build`/`smoke`/`golden`/`check-docs`/`no-new-any` — не
+  прогонял: на S4 нет диффа продуктового кода, гейты относятся к S6/S7.
+- Поведение relay (`support.houseplan.tech`) не в этом репозитории; утверждение
+  «relay вправе вернуть результат первого payload» проверено только по
+  документации (`docs/specs/043...md` §9.2), не по коду relay — она вне
+  контроля этого репозитория и вне скоупа ревью.
+- Мутанты `scripts/mutation-gate.mjs`, которые ТЗ требует добавить, ещё не
+  существуют (появятся в реализации) — не мог проверить, что они «обязаны
+  покрасить smoke», это станет предметом код-ревью (§2.10-подобная проверка
+  «тест умеет падать»).
+- Реальную доступность диалога с клавиатуры/screen reader — не относится к
+  предмету этого ТЗ (диалог существует с #43, доступность не меняется).
+
+## Материал раунда
+
+- SHA `dev`: `b71bc39b` (ветка задачи `issue/418-*` не создана — на момент
+  ревью задача в S3→S4, кода нет).
+- Материал ТЗ: тело issue #418 на момент чтения (комментарии `Аналитика`,
+  `Взял` — без изменения текста ТЗ).
+
+## Вердикт
+
+Зелёный. High: 0 · Medium: 0 (Low снят решением ревьюера, см. выше).
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Якоря снять не удалось: ветки задачи нет, материал читался по `dev`.