Files
houseplan-card/docs/reviews/SPEC-REVIEW-423-r1.md
2026-09-02 18:11:59 +00:00

18 KiB
Raw Permalink Blame History

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

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

  • Ветка: 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