mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 21:01:21 +00:00
committed by
Sergey Matyunin
parent
00b6845058
commit
4559b5cd74
@@ -0,0 +1,228 @@
|
||||
# SPEC-REVIEW-43-r1
|
||||
|
||||
- Issue: https://github.com/Matysh/houseplan-card/issues/43
|
||||
- Ревьюер: Claude (роль «Ревьюер ТЗ», PROCESS.md §2.4)
|
||||
- Материал: `docs/specs/043-private-support-report.md`, ревизия 2, на коммите
|
||||
`755fa4cf` (ветка `issue/43-help-feedback`), плюс тело issue #43 и все 7
|
||||
комментариев (аналитика 2026-08-14/2026-08-30, финальное UX-описание
|
||||
владельца 2026-09-01, повторная аналитика, Q1–Q5 и ответы владельца, хендофф
|
||||
автора «ТЗ полностью переработано»).
|
||||
- Заход: r1 (первый разбор этого ТЗ, документ прежних раундов не существует —
|
||||
предыдущая ревизия ТЗ была отклонена самим автором в аналитике 2026-09-01,
|
||||
а не ревью, поэтому раздел «Унаследовано» не применяется).
|
||||
- Трек: полный (аналитик явно назвал непройденные критерии `small`:
|
||||
больше одной поверхности, новый UX-контракт, новый внешний transport,
|
||||
privacy/security-контракт, обязательное влияние на touch-View) — лимит
|
||||
циклов ревью ТЗ 4, не 2.
|
||||
|
||||
## Скоуп
|
||||
|
||||
ТЗ описывает новый диалог «Помощь и обратная связь» в шапке карточки:
|
||||
перенос блока «О карточке», языковая ссылка на USER-GUIDE, форму
|
||||
контакт/сообщение и opt-in обезличенный support-пакет с точной геометрией,
|
||||
который уходит через backend House Plan в отдельный project-controlled
|
||||
HTTPS-relay (новый deployable сервис `support-relay/`) в закрытый
|
||||
support-mailbox. Это первая ревизия, дошедшая до ревью; предыдущая
|
||||
(clipboard-only) была отозвана автором ещё в аналитике.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (целиком, включая §1, §2.3–2.10,
|
||||
§4, §7.1–7.2) — прочитаны до разбора ТЗ.
|
||||
2. Issue #43: тело + все 7 комментариев через `gh issue view --json`
|
||||
(`mcp__github__*` были недоступны без разрешения) — восстановлена полная
|
||||
цепочка решений владельца (финальное UX-описание → Q1–Q5 → ответы).
|
||||
3. Построчное чтение `docs/specs/043-private-support-report.md` (646 строк)
|
||||
против §7.1 обязательных разделов, продуктовых вопросов из issue и
|
||||
`docs/TOUCH-SUPPORT.md`.
|
||||
4. Сверка утверждений §3 («подтверждённое текущее состояние») с реальным
|
||||
кодом: `src/houseplan-card.ts` (кнопка General Settings, `_norm && _canEdit`,
|
||||
порядок zoom→settings, ленивый `import('./houseplan-editor-runtime')`),
|
||||
`src/houseplan-editor-runtime.ts` (блок «О карточке», `gs.about_*` ключи),
|
||||
`custom_components/houseplan/{websocket_api,import_export}.py`
|
||||
(`create_export`), `repairs.py` (`broken_plan_<spaceId>`),
|
||||
`src/i18n/{ru,en,de,fr}.json` (RU/EN/DE/FR уже существуют).
|
||||
5. Сверка i18n-подписи и терминологии («О карточке», «Общие настройки»,
|
||||
GitHub/Telegram-ссылки) с `docs/USER-GUIDE.ru.md` и `src/i18n/ru.json` —
|
||||
расхождений не найдено.
|
||||
6. `docs/CONFIG-COMPATIBILITY.md` — подтверждено, что фича не трогает схему
|
||||
config/layout (ТЗ §10 корректно).
|
||||
7. `docs/TOUCH-SUPPORT.md` — сверка UX/§11 с контрактом «View dialogs and
|
||||
safe device actions: Fully supported» и правилом обязательной пометки
|
||||
`Touch editor: …`.
|
||||
8. `docs/specs/README.md` — обязательный раздел release-артефактов и
|
||||
двусторонняя ссылка issue↔ТЗ (строка 106) присутствуют.
|
||||
9. `scripts/process-gate.mjs` прочитан целиком по функции `classify()` и
|
||||
условиям `fail(1, …)`/`fail(8...)`, чтобы проверить, как гейт классифицирует
|
||||
новый путь `support-relay/**`, которого ТЗ вводит впервые.
|
||||
|
||||
### Гейты
|
||||
|
||||
| Гейт | Результат |
|
||||
|---|---|
|
||||
| `npx tsc --noEmit` | зелёный, 6.2 с |
|
||||
| `node scripts/process-gate.mjs` | «гейт пройден, предупреждений 0» (офлайн, диапазон `origin/dev..HEAD`, 1 коммит) |
|
||||
| `npm test`, `npm run build`, `check-docs.mjs`, смоки, `model-invariants`, `pytest tests_backend` | **не прогонялись** — диф коммита `755fa4cf` строго класса C: `docs/specs/043-private-support-report.md` + `docs/specs/README.md` (`git show --stat HEAD`), `src/**`/`custom_components/**` не тронуты. Эти гейты бессмысленны для чисто документного диффа на этапе ТЗ; `check-docs.mjs` условен на изменения `src/**`, которых нет |
|
||||
|
||||
Трейлеры коммита `755fa4cf`: `Issue: #43`, `User-Visible: no` — корректны для
|
||||
docs-only правки.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи) — `support-relay/**` не попадает ни в один класс изменений
|
||||
|
||||
**Файл:** `docs/specs/043-private-support-report.md`, §9.1 (строки 404–416) и
|
||||
§18 (строки 606–617).
|
||||
|
||||
**Суть:** ТЗ явно вводит новый top-level каталог `support-relay/` —
|
||||
«separately deployable service, excluded from the HACS artifact» — и явно
|
||||
кладёт «минимальный deployable relay» в объём #43 (§5.1). Ни `AGENTS.md`
|
||||
(таблица «Change classes»), ни `PROCESS.md §1` не знают такого пути: класс A
|
||||
ограничен `src/**` и `custom_components/houseplan/**/*.py`, класс B —
|
||||
конкретным списком (`test/**`, `tests_backend/**`, `demo/**`, `scripts/**`,
|
||||
`.github/**`, …), класс C — документацией. `support-relay/**` не match-ится
|
||||
ни одним regex.
|
||||
|
||||
Я прочитал `scripts/process-gate.mjs::classify()` (строки 93–99): путь, не
|
||||
попавший ни в `CLASS_D/A/B/C`, получает `'?'`. Строка 255–256 превращает это
|
||||
только в **предупреждение** (`warn(0, …)`), а не в отказ. Требование трейлера
|
||||
`Issue: #NN` (правило 1, строка 232) и проверка статуса issue через `--issues`
|
||||
(правило 8, строки 310/392/510) применяются только когда
|
||||
`c.classes.has('A') || c.classes.has('B')` — то есть коммит, который трогает
|
||||
**только** `support-relay/**`, эти проверки не запускает вовсе.
|
||||
|
||||
**Сценарий отказа:** после старта реализации коммит вида
|
||||
`feat: relay rate limiting` с diff только внутри `support-relay/` пройдёт
|
||||
`process-gate.mjs` (и локальный `pre-push`) без единого трейлера `Issue:`,
|
||||
без метки issue в `S5-ready`/`S6-in-progress`/`S7-code-review`, без ссылки на
|
||||
ТЗ — то есть ровно то, что правило №1 AGENTS.md объявляет запрещённым
|
||||
(«Изменение продуктового кода без issue запрещено»). Для обычного продукта
|
||||
это было бы досадной дырой в тулинге; здесь эта дыра открыта ровно в
|
||||
подсистеме, которая пересылает точную геометрию дома и контакт пользователя
|
||||
на внешний сервер — там же, где PROCESS.md требует наибольшей строгости.
|
||||
|
||||
**Почему это находка ТЗ, а не готового кода:** §18 «Documentation and release
|
||||
artifacts» перечисляет `docs/USER-GUIDE`, `docs/ARCHITECTURE.md`,
|
||||
`docs/SUPPORT-PRIVACY.md`, `docs/TESTING.md`, relay README — но не упоминает
|
||||
обновление таблицы классов в `AGENTS.md`/`PROCESS.md §1`. Без явного решения
|
||||
в ТЗ реализация имеет прямой путь создать неклассифицированный каталог и
|
||||
получить только warning вместо gate.
|
||||
|
||||
**Почему это не продуктовый вопрос владельцу:** это техническое решение
|
||||
(файловая раскладка / расширение таблицы классов), которое, по §7.1
|
||||
PROCESS.md, агенты решают и фиксируют сами («Всё, чего пользователь не
|
||||
наблюдает, агенты решают сами… раскладка файлов»). Автору достаточно
|
||||
добавить в ТЗ (например, в §9.1 или §18) одно из двух решений:
|
||||
- либо явно расширить класс A (или завести отдельный подкласс) на
|
||||
`support-relay/**` в `AGENTS.md`/`PROCESS.md §1` тем же коммитом, где
|
||||
каталог появляется, и включить эту правку в release-артефакты §18;
|
||||
- либо разместить relay-код внутри уже классифицированного пути (например,
|
||||
под `scripts/support-relay/` — класс B, или отдельно обсудить с владельцем
|
||||
вынос в отдельный репозиторий, если секреты не должны жить в этом дереве
|
||||
вообще).
|
||||
|
||||
Оставляю выбор автору — важно, чтобы решение было явно записано в ТЗ, а не
|
||||
осталось implicit-пробелом, который заметят только когда реальный коммит
|
||||
пройдёт гейт с одним warning.
|
||||
|
||||
### Low (снимается с записью) — отсутствует явная пометка `Touch editor: …`
|
||||
|
||||
**Файл:** `docs/specs/043-private-support-report.md`, §11 (строки 461–476).
|
||||
|
||||
`docs/TOUCH-SUPPORT.md` требует: «New editor feature specifications … must
|
||||
state one of: `Touch editor: supported`; `Touch editor: best effort /
|
||||
intentionally degraded`; `Touch editor: not exposed`.» Диалог Help/Feedback
|
||||
доступен не только в View, но и во всех трёх редакторах (Plan/Devices/
|
||||
Backdrop, §6.1), поэтому формально подпадает под это правило. §11 подробно и
|
||||
корректно описывает touch/a11y-контракт (44×44 px цель, 320 px раскладка,
|
||||
`aria-live`, фокус-менеджмент) — по содержанию это ровно `Touch editor:
|
||||
supported`, просто без канонической фразы, по которой обычно грепают такие
|
||||
декларации.
|
||||
|
||||
Снимаю как Low без возврата на цикл: содержательно контракт уже сильнее, чем
|
||||
«fully supported» из таблицы TOUCH-SUPPORT.md для View-диалогов, а сам диалог
|
||||
— не операция редактирования геометрии, а сквозной UI-элемент шапки. Автору
|
||||
имеет смысл добавить одну строку «Touch editor: supported» в §11 при
|
||||
следующей правке ТЗ ради единообразия аудита, но это не блокирует переход в
|
||||
`S5-ready`.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Обязательные разделы §7.1** — все на месте: сценарий (§1, персона Home
|
||||
admin из SCOPE.md, поверхность View+все редакторы), что человек увидит
|
||||
до/после (§2, одной фразой, без терминов реализации), проблема (§3), скоуп
|
||||
и не-скоуп (§5.1/5.2), контракт поведения (§4, §6–9), UX (§6, §11), модель
|
||||
данных и миграция (§10 — миграции нет, обоснованно: пакет строится на
|
||||
чтении, не меняет schema), i18n (§12), AC1…AC17 с доказательством (§13),
|
||||
план автотестов (§14), риски (§16), откат (§19), release-артефакты (§18).
|
||||
- **Продуктовые вопросы правильно классифицированы и закрыты владельцем.**
|
||||
Q1–Q5 в issue — все действительно продуктовые (куда уходят данные, что
|
||||
считается «бэкапом», что видно в preview, кто видит кнопку, есть ли
|
||||
контакт для ответа) — ни один не технический вопрос, вынесенный по ошибке.
|
||||
Ответы владельца дословно перенесены в §4/§6/§7/§9 ТЗ без искажений
|
||||
(сверено построчно): точная геометрия — да (Q2), preview точных байтов —
|
||||
да (Q3), только `can_write`, без kiosk (Q4), необязательный контакт вне
|
||||
support-пакета (Q5), HTTPS-relay через backend (Q1).
|
||||
- **Технические предположения промаркированы.** §20 «Принятые технические
|
||||
предположения» явно называет неочевидные архитектурные решения (email-relay
|
||||
вместо issue, package-local sequential псевдонимы вместо стабильного хэша,
|
||||
in-memory кеш preview, 30/24-часовые сроки хранения) как «assumed, change
|
||||
freely» — это именно то, что требует PROCESS.md §7.1, и защищает от
|
||||
«догадки, выданной за факт».
|
||||
- **Утверждения о текущем состоянии (§3) проверены по коду, а не приняты на
|
||||
веру.** Каждое из пяти технических утверждений подтвердилось построчно (см.
|
||||
«Как проверялось» п.4) — расхождений с реальным деревом не найдено.
|
||||
- **AC однозначны и снабжены методом доказательства.** Все 17 AC в таблице
|
||||
§13 имеют явную колонку Evidence с конкретным методом (browser smoke,
|
||||
DOM/i18n unit, backend test, adversarial security test, fake-clock test,
|
||||
performance benchmark) — ни одного «проверить вручную» или недоказуемого
|
||||
критерия не найдено.
|
||||
- **«Одно число — один источник» уже спроектировано в контракт, а не оставлено
|
||||
на волю реализации.** §6.4/§7.1/§8.1/AC5/AC10 и явный пункт mutation-gate
|
||||
«preview bytes are regenerated on submit → exact-byte test red» (§14.4)
|
||||
требуют, чтобы preview, download и отправленные байты были одним и тем же
|
||||
кешированным объектом с TTL-токеном, а не тремя независимыми вычислениями —
|
||||
ровно тот инвариант, который трижды ловил регрессии в проекте (#234, #233).
|
||||
- **Privacy allowlist/exclusion списки (§7.2/7.3) внутренне непротиворечивы**:
|
||||
каждый пункт «включено» имеет зеркальный запрет в «исключено» там, где это
|
||||
применимо (имена/ids/URL/бинарники/undo-backup/HA state исключены явно;
|
||||
точная геометрия включена явно и осознанно как продуктовое решение, а не
|
||||
побочный эффект).
|
||||
- **i18n-контракт (RU/EN/DE/FR)** соответствует уже существующим локалям
|
||||
проекта (`src/i18n/{ru,en,de,fr}.json` существуют) — фича не расширяет
|
||||
список поддерживаемых языков произвольно.
|
||||
- **Cross-links.** Issue #43 ↔ ТЗ ↔ `docs/specs/README.md` — ссылки в обе
|
||||
стороны на месте.
|
||||
- **DoR-зависимости (§17)** корректно вынесены как внешние операционные
|
||||
факты (деплой relay, mailbox, staging), а не спрятаны как «предположения»:
|
||||
ТЗ прямо говорит, что при их отсутствии после зелёного ревью issue должен
|
||||
получить `S5-ready` + `blocked`, а не мёртвую кнопку — это соответствует
|
||||
PROCESS.md §2.9 и §7.1 «риски перечислены».
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- **Полные `npm test`/`npm run build`/browser-смоки/`model-invariants`/
|
||||
`pytest tests_backend`** — не прогонялись: диф чисто документный (класс C),
|
||||
ни один из этих гейтов не имеет предмета для проверки на этом коммите.
|
||||
Они станут обязательны на этапе код-ревью, когда появится реализация.
|
||||
- **Реальная развёртываемость relay, содержимое `support-relay/`, secrets-
|
||||
handling** — предмета для проверки ещё нет (кода нет), это заявленная
|
||||
DoR-зависимость §17, а не то, что должно быть в ТЗ.
|
||||
- **Golden/скриншоты, performance-профили** — не относятся к докс-only диффу
|
||||
этого раунда.
|
||||
- **Соответствие ещё не написанных `docs/SUPPORT-PRIVACY.md`,
|
||||
`docs/ARCHITECTURE.md`-правок** — эти документы появятся вместе с кодом
|
||||
(§18), не на этапе ТЗ.
|
||||
|
||||
## Вывод
|
||||
|
||||
ТЗ методологически одно из самых тщательных, что проходили через этот
|
||||
процесс: 17 однозначных AC с доказательствами, явное разделение продуктовых
|
||||
решений владельца и технических допущений, встроенная защита от «одно число —
|
||||
два источника» и от privacy-регрессий через adversarial-фикстуру. Единственная
|
||||
блокирующая (по бюджету цикла) находка — не продуктовая, а процессная: новый
|
||||
top-level каталог `support-relay/` создаёт слепую зону в автоматическом гейте
|
||||
`process-gate.mjs`, причём именно для той части системы, где цена
|
||||
незамеченного нарушения правила «код только через issue» выше всего. Это
|
||||
Medium-находка в скоупе задачи (сам каталог заводит #43), правится одной
|
||||
явной строкой в ТЗ — второй раунд не должен требовать нового расследования.
|
||||
Reference in New Issue
Block a user