mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 20:29:00 +00:00
claude[bot]
@@ -0,0 +1,185 @@
|
||||
# SPEC-REVIEW-423-r1
|
||||
|
||||
- Issue: https://github.com/Matysh/houseplan-card/issues/423
|
||||
- Этап: ТЗ на ревью (PROCESS.md §2.4)
|
||||
- Заход: r1 · блокирующих циклов израсходовано 0 из 4
|
||||
- Материал: ветка `issue/423-v170-polish`, SHA `7102994dc669e789951f3a1906bf07152239f99d`
|
||||
(`docs: specify v1.70 support polish`)
|
||||
- ТЗ: `docs/specs/423-v170-polish.md` (420 строк, ссылка issue ↔ ТЗ на месте в обе стороны)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Полный трек, первый заход. Проверялись: полнота обязательных разделов §7.1,
|
||||
однозначность и доказуемость AC1–AC12, отсутствие догадок, выданных за факт, и
|
||||
фактическое соответствие текста ТЗ реальному состоянию кода — websocket_api.py,
|
||||
support_transport.py, houseplan-editor-runtime.ts, houseplan-card.ts, i18n,
|
||||
bundle-budget, benchmark-файлы, docs/specs/043-private-support-report.md (контракт
|
||||
Help & feedback, на который #423 ссылается), USER-GUIDE.ru.md.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Ревью — не код-ревью: продуктового кода в диффе нет (только
|
||||
`docs/specs/423-v170-polish.md` + строка в `docs/specs/README.md`,
|
||||
`User-Visible: no`). Дешёвые гейты на этом SHA не гонялись отдельно — Validate на
|
||||
`7102994d` зелёный (ссылка в задаче на ревью), а диффа в `src/**`/`custom_components/**`
|
||||
нет, значит typecheck/test/build/`check-docs.mjs` не могли измениться этим коммитом.
|
||||
`node scripts/process-gate.mjs` прогнан вручную — «гейт пройден, предупреждений 0».
|
||||
|
||||
Каждое фактическое утверждение ТЗ о текущем коде и цифрах сверено чтением
|
||||
исходников на `HEAD`, а не принято на слово:
|
||||
|
||||
| Утверждение ТЗ | Где проверено | Результат |
|
||||
|---|---|---|
|
||||
| `_support_repairs()` матчит только `broken_plan_*` | `websocket_api.py:249-256` | подтверждено |
|
||||
| `filename_token[:32]` в multipart, `token.slice(0,12)` в browser download | `support_transport.py:61`, `houseplan-editor-runtime.ts:9307` | подтверждено |
|
||||
| `_build_snapshot()` (executor, до 8 МиБ) выполняется до prune/count/limit | `websocket_api.py:2158-2202` | подтверждено; там же обнаружен смежный баг: `old_token` того же draft удаляется **до** проверки лимита (строки 2196-2202) — если проверка после этого проваливается, старый пригодный preview уже потерян. Спецификация фиксирует именно это в контракте §3 и рисках («Неудачный refresh уничтожает старый preview») |
|
||||
| строгое `_haIntegrationVersion !== CARD_VERSION` в трёх местах | `houseplan-editor-runtime.ts:9206, 9326, 9395` | подтверждено, ровно три места |
|
||||
| `config/get` не отдаёт capability-поле сейчас | `websocket_api.py:1224-1242` | подтверждено; топ-level словарь ответа не зависит от `space_id/fields/marker_fields` — добавление `support_api` как top-level поля действительно не подчиняется projection, вход AC5 корректен |
|
||||
| 43 английские строки `support.*` | `src/i18n/en.json`, `python3 -c "..."` подсчёт по факту | подтверждено, ровно 43 |
|
||||
| Initial View baseline 291046 B gzip | `dist/houseplan-assets.json` → `initialViewGzipBytes` | подтверждено побайтово; текущий бюджет `scripts/bundle-budget.mjs` — 300000, запас 8954 Б (ТЗ округляет до «9,5 КБ» — не расходится по существу) |
|
||||
| `demo/benchmark_backdrop_decode.mjs` создаёт page напрямую, без `watchPage`/`reportPageErrors` | сам файл, `chromium.launch()`+`newPage()` без импорта из `serve.mjs` | подтверждено; проверены и остальные восемь `demo/benchmark_*.mjs` — три идут через `launch()` из `serve.mjs` (уже получают `watchPage` изнутри, `serve.mjs:139`), пять не создают Playwright page вовсе. `backdrop_decode` — единственный нарушитель, объём AC9 не занижен и не завышен |
|
||||
| §7.2 ТЗ #43 уже требует «family + count» из `translation_key` | `docs/specs/043-private-support-report.md:257-272` | подтверждено; технический источник (`translation_key`, regex, длина 64) — новое техническое решение #423, корректно записанное в «Принятые предположения», не противоречит #43 |
|
||||
| §8.3 ТЗ #43 уже держит место под `{short-id}` в имени файла | `docs/specs/043-private-support-report.md:375-384` | подтверждено, #423 не меняет форму заявленного контракта, только источник short-id |
|
||||
| `.github/workflows/docs-screenshots.yml` действительно имеет разъехавшиеся строки (Capture/гейт) | сам файл, строки ~80/99 | подтверждено; #422 (`S6-in-progress`, ещё не смержен) уже владеет этим пунктом — ссылка корректна, дублирования правки нет |
|
||||
| Единственный вызывающий `async_submit_report` — `websocket_api.py:2299` | `grep` по всему backend | подтверждено; удаление параметра `filename_token` не задевает скрытых вызовов |
|
||||
|
||||
## Продуктовая рамка (§7.1, два первых раздела)
|
||||
|
||||
Сценарий и «что человек увидит до и после» присутствуют, отвечают на оба
|
||||
обязательных вопроса: персона — Home admin (единственная персона с `can_write`,
|
||||
которая видит форму поддержки, docs/SCOPE.md), поверхность — диалог «Помощь и
|
||||
обратная связь» в шапке, момент — сразу после обновления карточки/интеграции через
|
||||
HACS, когда браузер может держать старый bundle. «До»/«после» сформулированы без
|
||||
терминов реализации: форма скрывается при любом несовпадении релиза → форма
|
||||
доступна при совместимом `support_api`, независимо от номера релиза; имя файла
|
||||
перестаёт называть capability-токен. Это ровно то отличие, которое обязано увидеть
|
||||
ТЗ, а не техническая деталь.
|
||||
|
||||
## Продуктовый вопрос Q1 и его закрытие
|
||||
|
||||
Автор корректно вынес владельцу продуктовый (а не технический) вопрос: как
|
||||
трактовать несовпадение версий карточки/интеграции — по номеру релиза или по
|
||||
capability API (что человек видит: форма показана или скрыта). Вопрос задан одним
|
||||
комментарием с default и альтернативой, issue помечен `blocked` поверх `S3-spec`,
|
||||
как требует §7.1. `blocked` снят, и в финальном ТЗ зафиксирован явный выбор
|
||||
(«Владелец подтвердил default: protocol capability важнее равенства product
|
||||
versions») в разделе «Принятые предположения». Отдельного текстового ответа
|
||||
владельца отдельной репликой в этом issue не видно — но это соответствует
|
||||
установленной практике репозитория: то же самое устройство переписки видно в
|
||||
#406 («Q1–Q4 приняты по defaults из issue») и в §4 самого ТЗ #43 («Решения
|
||||
владельца» перечислены без дословной цитаты). Не поднимаю это в находку — это
|
||||
системное свойство процесса этого репозитория, а не дефект конкретного ТЗ.
|
||||
|
||||
## Критерии приёмки
|
||||
|
||||
AC1–AC12 пронумерованы, у каждого — однозначная формулировка и явный способ
|
||||
доказательства (`unit`/`backend`/`smoke`/`i18n+bundle`/`build manifest`/`review`/
|
||||
`diff`). Ни один не описывает желаемый результат без наблюдаемого признака:
|
||||
|
||||
- AC1–AC4 (repair families, filename, двухфазная quota) проверяемы backend-тестами
|
||||
на конкретных структурах данных, включая новый инвариант «старый token того же
|
||||
draft не удаляется до финальной успешной проверки» — прямое исправление
|
||||
найденного при проверке смежного бага.
|
||||
- AC5–AC6 (capability contract) единообразны: один `supportApiCompatible(value)`
|
||||
используется в трёх путях, что устраняет реальный источник несогласованности
|
||||
(сейчас — три независимые проверки, один текст).
|
||||
- AC7–AC8 (lazy i18n, bundle) привязаны к измеренному факту (291046 Б, 43 строки),
|
||||
не к общему «уменьшить» — числа проверяемы `npm run bundle:sync` +
|
||||
`dist/houseplan-assets.json`, повторный расчёт не нужен по гейт-политике.
|
||||
- AC9 (benchmark guard) — прямое расширение уже существующего в кодовой базе
|
||||
паттерна (`test/smoke-harness-contract.test.mjs`, issue #404/#407/#421,
|
||||
`demo/guard/guard_report_page_errors.mjs`); контракт и `--guard-probe`
|
||||
сформулированы так же, как уже принятый прецедент для смоков.
|
||||
- AC10 фиксирует границу с #422 отрицательным условием («workflow не меняется»),
|
||||
что проверяется тривиальным diff.
|
||||
- AC11–AC12 — стандартные гейты и документация/changelog.
|
||||
|
||||
«План отрицательной проверки» покрывает по одному конкретному способу сломать
|
||||
каждый из шести пунктов и что должно покраснеть — формально это не раздел «план
|
||||
автотестов» из §7.1, но содержательно эквивалентен и даже избыточен: у каждого AC
|
||||
есть отдельная строка «Доказательство», плюс раздел «Затрагиваемые файлы»
|
||||
перечисляет точные тестовые файлы. Тот же паттерн («План отрицательной проверки»
|
||||
вместо/вместе с «план автотестов») уже принят ревью #421 без отдельной находки —
|
||||
не поднимаю здесь вторично.
|
||||
|
||||
## Single source (числа, показанные пользователю)
|
||||
|
||||
Единственное новое/изменённое число, видимое пользователю, — short-id в имени
|
||||
скачиваемого/пересылаемого файла. Контракт §2 ТЗ явно фиксирует один источник:
|
||||
и frontend (`preview.sha256.slice(0,12)`), и backend (`attachment_sha256[:12]`)
|
||||
берут префикс одного и того же уже вычисленного и уже показанного пользователю
|
||||
SHA-256 (диалог показывает полный SHA-256 текстом) — второго независимого
|
||||
вычисления нет. Требование `docs/specs/README.md`/task про «одно число — один
|
||||
источник» выполнено предположением, а не совпадением: ТЗ прямо называет это в
|
||||
разделе Security («SHA short-id не добавляет новую информацию»).
|
||||
|
||||
## Не-скоуп и границы
|
||||
|
||||
Не-скоуп корректно исключает: смену package v1/схемы, смену лимитов 3/3 и TTL,
|
||||
передачу сырых Repair id, новый negotiation-эндпоинт, вынос немедленной локали
|
||||
целиком, правку `docs-screenshots.yml` (владеет #422). Все шесть пунктов
|
||||
проверяемого аудита (а/б/в/г/д/ж) покрыты контрактом; пункт (е) корректно и
|
||||
единственно исключён со ссылкой на #422, который на этом SHA ещё не смержен
|
||||
(`S6-in-progress`, `blocked`) — ссылка не «дублирует уже готовое», а действительно
|
||||
устраняет двойную правку одного участка файла из двух задач параллельно.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Обязательные разделы §7.1 присутствуют все, под ожидаемыми заголовками —
|
||||
сценарий, что человек увидит, проблема, скоуп/не-скоуп, контракт, UX, модель
|
||||
данных/миграция, i18n, AC, план (в форме «отрицательной проверки»), риски,
|
||||
откат, release-артефакты.
|
||||
- DoR-пункты (§2.5), проверяемые на этапе спека, закрыты явно: compatibility per
|
||||
`CONFIG-COMPATIBILITY.md` решена корректно («runtime capability, не persisted
|
||||
config, миграции нет» — поле действительно не персистится ни в одном сторе);
|
||||
touch описан («один predicate для touch/desktop/keyboard»); производительность
|
||||
названа числами, а не общими словами; откат описан по каждому из шести пунктов
|
||||
отдельно.
|
||||
- Технические решения (regex `translation_key`, источник short-id, одновременный
|
||||
перенос всех четырёх локалей) записаны явным блоком «Принятые предположения» —
|
||||
ревьюер может оспорить, но ни одно не выдано за факт о существующем поведении.
|
||||
- Все файловые пути, номера строк и цифры в ТЗ, проверенные выше, совпадают с
|
||||
фактическим деревом на `HEAD`.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет ни одной находки уровня High или Medium — ни в скоупе, ни вне скоупа.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Реализуемость AC3/AC4 «без store loads/executor build» как именно будет
|
||||
доказана backend-тестом (мокать внутреннюю функцию или считать side-effect) —
|
||||
техническая деталь реализации, не предмет ТЗ-ревью; будет видно на код-ревью.
|
||||
- Полный текст будущей русской формулировки `support.update_required` — ТЗ
|
||||
фиксирует смысл («обновите карточку и интеграцию до совместимых версий»), а не
|
||||
точный текст на четырёх языках; это нормально для ТЗ, финальный текст —
|
||||
предмет code review/i18n-тестов.
|
||||
- Тяжёлые гейты (golden, полный smoke-набор, invariants, performance_smoke,
|
||||
backend HA harness) — не запускал: диффа в `src/**`/`custom_components/**` нет,
|
||||
диффа в геометрии/config нет, AC этой задачи их не называют на этапе ТЗ. Они
|
||||
относятся к этапу код-ревью после реализации.
|
||||
|
||||
## Материал раунда
|
||||
|
||||
```
|
||||
SHA: 7102994dc669e789951f3a1906bf07152239f99d
|
||||
Дерево: docs/specs/423-v170-polish.md (blob новый в этом коммите)
|
||||
Поиск при протухании SHA:
|
||||
git log --all --find-object=$(git rev-parse 7102994d:docs/specs/423-v170-polish.md) -- docs/specs/423-v170-polish.md
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/423-v170-polish`, коммит `7102994dc669` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `69763043f0c176779743a040107b9e115862ef2d`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 69763043f0c1
|
||||
```
|
||||
- ТЗ `docs/specs/423-v170-polish.md`, блоб `3a0e7b78a9f8a01f3f54e4a6182dba04badecfae`
|
||||
```
|
||||
git log --all --find-object=3a0e7b78a9f8a01f3f54e4a6182dba04badecfae -- docs/specs/423-v170-polish.md
|
||||
```
|
||||
Reference in New Issue
Block a user