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

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\
   .... Действительно не проверяет 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 снят решением ревьюера, см. выше).

---

Материал раунда

• Ветка: dev, коммит `` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Якоря снять не удалось: ветки задачи нет, материал читался по dev.