mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-04 05:41:34 +00:00
claude[bot]
@@ -0,0 +1,232 @@
|
||||
# SPEC-REVIEW-597-r1 — «Редактор боковой панели переезжает на общий набор контролов (шаг 2 эпика #591)»
|
||||
|
||||
Issue: [#597](https://github.com/Matysh/houseplan-card/issues/597)
|
||||
Этап: ревью ТЗ (PROCESS.md §2.4), лёгкий трек (метка `small`, лимит циклов — 2, §4)
|
||||
Заход: r1 (первый; разделы «Унаследовано из r0» и «Закрытие раунда r0» не нужны — §2.10 применяется со второго захода)
|
||||
|
||||
## Вердикт
|
||||
|
||||
**Жёлтый.** High: 0. Medium в скоупе: 2 (возвращаются автору в этой же задаче). Medium вне скоупа: 0. Low: 0.
|
||||
|
||||
## Скоуп разбора
|
||||
|
||||
Полный разбор (первый заход): тело issue #597 целиком (раздел «Аналитика» + раздел `## ТЗ`),
|
||||
метки (`P3`, `S4-spec-review`, `small`, `tech-debt`), таймлайн issue (комментариев на момент
|
||||
разбора нет — `gh issue view 597 --json comments` пуст). Прочитаны `docs/SCOPE.md`, `AGENTS.md`,
|
||||
`PROCESS.md` §2.4, §2.6, §2.7, §7.1, §4, §5. Поскольку почти все содержательные утверждения
|
||||
ТЗ — проверяемые факты о текущем коде, а не догадки о будущем, разбор включает и сам код:
|
||||
|
||||
- `src/styles/form-kit.styles.ts` — фактическая структура генератора шага 1 (#594): `sharedCss`
|
||||
(карточка-группа, обводка фокуса, выключенное состояние, опционально ряд-переключатель) +
|
||||
`extrasCss` (заголовок группы, сегмент, цвет), `formKitCss` как их композиция, параметры
|
||||
`SUMMARY_PANEL_FORM_KIT` / `CARD_DIALOG_FORM_KIT`;
|
||||
- `src/summary-panel-editor-style.ts` — точное построчное соответствие таблице ТЗ (строки
|
||||
34–41 карточка-группа, 79–85 фокус, 86 своё, 87–89 выключенное состояние, 90 своё, 91–101
|
||||
ряд-переключатель, 102–104 подпись ряда, 105+ своё) и байтовая длина извлечённого литерала
|
||||
`String.raw` шаблона;
|
||||
- `test/form-kit.test.mjs` — существующий тест `#594 AC11`, который уже хранит все пять точных
|
||||
фрагментов панели в словаре `PANEL_FRAGMENTS` и сверяет их с текущим листом панели и с выходом
|
||||
генератора;
|
||||
- `test/styles-split.test.mjs` — характер существующей проверки для `CARD_DIALOG_FORM_KIT`
|
||||
(уникальность селекторов, счётчики медиа-запросов — не байтовое сравнение текста);
|
||||
- `scripts/mutation-registry.mjs` — существующие мутанты `form-kit-writes-to-a-neighbour-key`,
|
||||
`form-kit-segment-drops-radio-semantics`, отсутствие конфликта имени с планируемым
|
||||
`form-kit-fragment-drifts-from-panel`;
|
||||
- `docs/ARCHITECTURE.md:2166-2167` — точность фразы «its sheet includes the settings-only
|
||||
composition from `summary-panel-editor-style.ts`» после предполагаемого изменения;
|
||||
- `scripts/bundle-budget.mjs` — какие именно ленивые графы реально ограничены порогом
|
||||
(`initialViewGzipBytes`, `initialPanelOnlyGzipBytes`, полоса `lazyEditorGzipBytes`), в связи с
|
||||
заявлением К4/AC7 о ленивом графе панели;
|
||||
- существование `demo/smoke_summary_panel.mjs`, `demo/smoke_summary_panel_polish.mjs`,
|
||||
`demo/smoke_room_settings.mjs`, каталога `test/fixtures/` (фикстура `summary-panel-editor.css`
|
||||
в нём пока отсутствует — ожидаемо, она создаётся при реализации).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Сверил таблицу «строки `summary-panel-editor-style.ts` / что там» из раздела «Аналитика»
|
||||
построчно с файлом: все девять строк таблицы совпадают с фактическим содержимым (небольшое
|
||||
расхождение на закрывающую скобку карточки-группы — заявлено «34–40», фактически блок
|
||||
34–41 с закрывающей `}` на 41-й; не meняет вывод и не влияет на выполнимость, не заношу как
|
||||
находку).
|
||||
2. Извлёк содержимое `String.raw` шаблона `summaryPanelEditorCss` программно и подтвердил
|
||||
заявленные в issue цифры: длина 14585 символов и отсутствие обратного слэша в тексте —
|
||||
риск №2 («`String.raw` → обычный шаблон») подтверждён как безопасный, а не как
|
||||
заявление на глаз.
|
||||
3. Прочитал `test/form-kit.test.mjs` целиком: тест `#594 AC11` уже содержит точные тексты всех
|
||||
пяти правил, помеченных в ТЗ как общие (карточка-группа, обводка фокуса, выключенное
|
||||
состояние, ряд-переключатель, подпись ряда) в словаре `PANEL_FRAGMENTS`, и сегодня проверяет
|
||||
их **присутствие** и в листе панели, и в выходе генератора (до переезда панели оба места
|
||||
идентичны по построению). Это прямое основание для AC1/AC2 нового теста — сверил, что
|
||||
переиспользовать этот словарь для проверки **отсутствия** литерала в панели после переезда
|
||||
технически возможно уже сегодняшними средствами файла.
|
||||
4. Прочитал `test/styles-split.test.mjs`: проверка для `CARD_DIALOG_FORM_KIT` — уникальность
|
||||
Set селекторов и счётчики `@media`-блоков по regexp, не полное текстовое сравнение. Значит
|
||||
заявление К3/AC3 «выход `formKitCss` для диалогов … остаётся прежним байт в байт (его
|
||||
сверяют существующие тесты набора)» не имеет сегодня буквального свидетеля — см. находку M2.
|
||||
5. Прочитал `docs/ARCHITECTURE.md` вокруг строки 2167: фраза описывает, что лист панели
|
||||
*включает* композицию из `summary-panel-editor-style.ts` — после переезда панели эта фраза
|
||||
остаётся верной (файл не переименовывается, состав вызова не меняется), значит заявление
|
||||
«Документация не меняется» в разделе Release-артефактов не создаёт устаревания.
|
||||
6. Прочитал `scripts/bundle-budget.mjs`: пороги реально проверяются (`throw`) только для
|
||||
`initialViewGzipBytes`, `initialPanelOnlyGzipBytes` и полосы `lazyEditorGzipBytes`
|
||||
(`LAZY_EDITOR_GZIP_CEILING` ± полоса). Отдельного порога для ленивого графа сводной панели
|
||||
нет — но AC7 называет именно `initialViewGzipBytes` и «ленивый редакторский» (там, где
|
||||
живёт `CARD_DIALOG_FORM_KIT` через `src/editors/form-kit.ts`), а не отдельный график панели,
|
||||
которого в бюджете и не существует. Заявление AC7 точное относительно того, что реально
|
||||
проверяется — не находка.
|
||||
7. Проверил единственного текущего импортёра `form-kit.styles.ts` — `src/editors/form-kit.ts`;
|
||||
после задачи вторым импортёром станет `summary-panel-editor-style.ts`. `form-kit.styles.ts`
|
||||
сам ничего не импортирует — циклического импорта не возникает.
|
||||
8. Проверил существование трёх названных в AC4/AC5 смоков и отсутствие конфликта имени
|
||||
планируемого мутанта в `scripts/mutation-registry.mjs`.
|
||||
9. Классы риска §2.6 в применении к тексту ТЗ (а не к коду, которого пока нет): раздел
|
||||
«Классы риска §2.6» ТЗ разбирает все шесть явно (async/данные и права/геометрия — «не
|
||||
применимо» обоснованно для чисто-CSS задачи без сети и без геометрии; визуал — назван
|
||||
главным риском и покрыт AC1; объём/perf — AC7; host/input — К5). Расхождений с кодом не
|
||||
нашёл — разметка панели (`src/summary-panel-editor.ts`) действительно не участвует ни в
|
||||
одном из названных файлов генератора/стиля.
|
||||
10. Проверил, нет ли открытого продуктового вопроса, выданного за решённый факт: сценарий
|
||||
(«администратор дома, десктоп, панель настроек, не должен заметить ничего») и «что человек
|
||||
увидит» соответствуют формату §7.1 (персона/поверхность/момент, потом факт без терминов
|
||||
реализации) и не противоречат ничему в `docs/SCOPE.md` — задача не вводит функциональность,
|
||||
а устраняет техдолг существующей, уже принятой поверхности (J4/J6), что не требует
|
||||
отдельной строки в Core user jobs.
|
||||
|
||||
## Обязательные разделы §7.1 — комплектность
|
||||
|
||||
Присутствуют все: сценарий · что человек увидит до/после · проблема · скоуп и не-скоуп ·
|
||||
контракт поведения (К1–К5) · UX («Нет») · модель данных и миграция («Нет») · i18n («Нет») ·
|
||||
критерии приёмки AC1–AC8 с указанием доказательства и «чем краснеет» · план автотестов · риски
|
||||
(3, включая явно поднятую «фикстуру, которая живёт вечно» и технический риск `String.raw`) ·
|
||||
откат · release-артефакты · блок «принято предположительно» с явным разделением технических
|
||||
решений. Комплектность разделов дефектов не имеет.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи — чинится в этой же задаче)
|
||||
|
||||
**M1. AC2 заявляет проверку «ни одно из пяти правил не лежит в листе панели литералом», но
|
||||
названный способ доказательства проверяет это только для одного из пяти фрагментов.**
|
||||
|
||||
Раздел «Критерии приёмки», строка AC2, столбец «чем доказывается»: *«тот же тест: ищет пять
|
||||
вызовов и отсутствие `min-height: 54px` в исходнике панели»*. Строка `min-height: 54px`
|
||||
встречается только внутри фрагмента «ряд-переключатель» (`.summary-editor .summary-switch`,
|
||||
`src/summary-panel-editor-style.ts:94`) — она не появляется ни в карточке-группе, ни в обводке
|
||||
фокуса, ни в выключенном состоянии, ни в подписи ряда. Значит названная проверка ловит только
|
||||
одну возможную «оставленную на всякий случай» копию из пяти, а не все пять, которые требует К2
|
||||
(«Литерал, оставленный на всякий случай, — это та же копия»).
|
||||
|
||||
Конкретный сценарий, который проходит мимо названной проверки: реализация вызывает все пять
|
||||
формКит-функций в новых местах (проверка «пять вызовов» — зелёная), но по пути оставляет,
|
||||
например, старый блок выключенного состояния как неиспользуемый закомментированный или
|
||||
недостижимый литерал где-то в файле «на будущее». Он не входит в собранный вывод — байт-в-байт
|
||||
тест AC1 не заметит (длина и содержимое строки `summaryPanelEditorCss` не меняются), а
|
||||
подстрока `min-height: 54px` в файле не появляется — AC2 в заявленном виде тоже зелёный. К2
|
||||
нарушен, ни один названный AC не покраснел.
|
||||
|
||||
Правка дешева и не требует новой инфраструктуры: `test/form-kit.test.mjs` уже хранит точный
|
||||
текст всех пяти фрагментов в словаре `PANEL_FRAGMENTS` (используется существующим тестом
|
||||
`#594 AC11` для проверки *присутствия*). Новый тест панели должен для каждого из пяти
|
||||
элементов этого словаря проверить его **отсутствие** как литерала в исходнике
|
||||
`src/summary-panel-editor-style.ts`, а не только для одной подстроки, характерной для одного
|
||||
фрагмента. Формулировку AC2 стоит поменять на «…ищет пять вызовов и отсутствие всех пяти точных
|
||||
текстов правил-фрагментов (карточка-группа/фокус/выключенное состояние/ряд-переключатель/
|
||||
подпись ряда) в исходнике панели».
|
||||
|
||||
**M2. К3/AC3 заявляют байт-в-байт неизменность выхода `formKitCss(CARD_DIALOG_FORM_KIT)`, но
|
||||
названное свидетельство («существующие тесты набора») сегодня не делает полного текстового
|
||||
сравнения.**
|
||||
|
||||
Раздел «Контракт поведения», К3: *«Выход `formKitCss` для диалогов карточки остаётся прежним
|
||||
байт в байт (его сверяют существующие тесты набора)»*; AC3, столбец «чем доказывается»:
|
||||
*«существующие тесты набора **без правок**»*. Прочитал оба файла, которые сегодня используют
|
||||
`CARD_DIALOG_FORM_KIT`: `test/form-kit.test.mjs` проверяет включение конкретных подстрок
|
||||
(`.hpf-card {`, `.hpf-form input:focus-visible`, отсутствие `summary`/`-switch` при
|
||||
`withSwitch: false`, наличие `.hpf-switch` при включённом флаге) — не полный текст;
|
||||
`test/styles-split.test.mjs` строит `Set` селекторов и считает вхождения `@media` по regexp —
|
||||
тоже не байтовое сравнение. Ни один существующий тест не зафиксирует, например, лишний или
|
||||
пропавший символ пробела/переноса строки на стыке двух фрагментов при переходе композиции
|
||||
`sharedCss`+`extrasCss` (2 функции) на пять именованных функций — а именно эта механика и есть
|
||||
предмет задачи (К3: «разрезание на функции — механическое»).
|
||||
|
||||
Последствие ниже, чем у M1 (пробел в CSS не меняет рендер, `npm run golden:verify`/AC6 поймает
|
||||
реальную потерю правила, а не разницу в пробелах), но заявленное доказательство неточно:
|
||||
K3/AC3 обещают байт-в-байт, а названный свидетель этого не проверяет. Дешёвая правка — добавить
|
||||
в `test/form-kit.test.mjs` одно сравнение `formKitCss(CARD_DIALOG_FORM_KIT)` с зафиксированным
|
||||
сегодняшним выводом (текстовая константа или маленькая фикстура, по аналогии с AC1) либо
|
||||
понизить формулировку К3/AC3 до того, что реально проверяется («набор правил и порядок
|
||||
селекторов не меняются» — как это делает `styles-split.test.mjs` сейчас).
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Таблица соответствия строк `summary-panel-editor-style.ts` пяти общим правилам — точна
|
||||
(кроме однобайтового смещения на закрывающую скобку, не влияющего на выполнимость).
|
||||
- Числа из раздела «Проверено до написания ТЗ» (длина 14585, побайтовое совпадение, отсутствие
|
||||
обратного слэша) подтверждены самостоятельным извлечением и подсчётом, а не просто процитированы.
|
||||
- AC1 (фикстура + новый тест) — метод доказательства чёткий, «чем краснеет» указан прямо
|
||||
(сам тест), не размыт.
|
||||
- AC4–AC6, AC8 — названные файлы/скрипты существуют, метод доказательства однозначен, `git diff
|
||||
--stat demo/` как критерий для AC4 проверяем механически.
|
||||
- AC7 — заявление точно относительно реально проверяемых порогов `bundle-budget.mjs`; отдельного
|
||||
несуществующего порога для ленивого графа панели AC7 не подразумевает.
|
||||
- Скоуп/не-скоуп — перечисленные файлы (`form-kit.styles.ts`, `summary-panel-editor-style.ts`,
|
||||
`form-kit.test.mjs`, новая фикстура, один мутант) совпадают с тем, что действительно нужно
|
||||
трогать; К5 (разметка панели не в диффе) проверяем механически через список файлов диффа.
|
||||
- Раздел «Классы риска §2.6» разобран по существу, а не формальной галочкой — «не применимо»
|
||||
обосновано для каждого неприменимого класса.
|
||||
- Раздел «Принято предположительно» корректно отделяет технические решения (имена фрагментов,
|
||||
путь фикстуры, `formKitCss` как композиция) — ни одно из них не является скрытым продуктовым
|
||||
решением.
|
||||
- Риск «фикстура, которая живёт вечно» явно назван и снабжён условием снятия (шаг эпика,
|
||||
легально меняющий вид панели) — не оставлен как «само собой понятно».
|
||||
- `docs/ARCHITECTURE.md` не устаревает от этого изменения — заявление «Документация не
|
||||
меняется» в Release-артефактах корректно.
|
||||
- Продуктовых вопросов, вынесенных владельцу как решённые факты без подтверждения, не нашёл —
|
||||
ни один открытый пункт не подменён догадкой (в отличие от прецедента #593 H1).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Реализацию — на этапе ревью ТЗ продуктовый код задачи ещё не написан (`git diff --stat` по
|
||||
скоупу задачи пуст на `HEAD` `83a07925465b5aa6724b2b27d454029b56ddd631`), гейты
|
||||
`typecheck`/`test`/`build`/`golden`/смоки к этому раунду неприменимы.
|
||||
- Не запускал `npm run gate:small`, `npm test` и т. п. — нет материала для запуска (см. выше);
|
||||
единственные исполненные мной проверки — извлечение строки шаблона и подсчёт длины/наличия
|
||||
обратного слэша в текущем `src/summary-panel-editor-style.ts` (см. «Как проверялось», п.2).
|
||||
- Не связывался с владельцем — открытых продуктовых вопросов в тексте не нашёл, обе находки
|
||||
этого раунда технические (полнота проверки, а не решение о видимом поведении) и решаются
|
||||
автором и ревьюером по §7.1.
|
||||
- Не проверял механику существующего мутанта-инфраструктуры (`scripts/mutation-registry.mjs`)
|
||||
дальше подтверждения отсутствия конфликта имени — сама мутация `form-kit-fragment-drifts-from-panel`
|
||||
ещё не написана, это предмет реализации и код-ревью.
|
||||
- Не искал дубликатов задачи — issue прямо ссылается на эпик #591 и долг, названный в #594; по
|
||||
содержанию совпадений с другими открытыми issue не искал целенаправленно.
|
||||
|
||||
## Вывод
|
||||
|
||||
ТЗ формально полно по §7.1, продуктовые разделы (сценарий/что человек увидит) отвечают на
|
||||
нужные вопросы без терминов реализации, все проверяемые фактические утверждения о текущем коде
|
||||
(таблица строк, длина шаблона, отсутствие обратного слэша, существующая тестовая инфраструктура)
|
||||
подтвердились чтением кода. Обе находки — про точность формулировки «чем доказывается» для
|
||||
AC2 и AC3/К3: заявленный метод проверки уже, чем заявленный контракт (K2 — все пять правил без
|
||||
литеральной копии, K3 — байт-в-байт для диалогов), и обе решаются одной-двумя строками в
|
||||
`test/form-kit.test.mjs`, переиспользуя уже существующий в этом же файле словарь
|
||||
`PANEL_FRAGMENTS`. Ни одна не блокирует план как таковой — план подстановки выполним и уже
|
||||
предварительно проверен автором эмпирически. Возврат автору для уточнения формулировок AC2/AC3
|
||||
(или соответствующей правки будущего теста, если правится текст AC, а не тест) в рамках той же
|
||||
задачи.
|
||||
|
||||
**Вердикт: жёлтый · заход r1 · блокирующих циклов 1/2 · High: 0 · Medium: 2 → в задаче**
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `83a07925465b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `bb4d62ab340508554c03325cb0b12628cc8204cd`
|
||||
```
|
||||
git log --all --format='%H %T' | grep bb4d62ab3405
|
||||
```
|
||||
- Тело issue: `2e961eb72bcea811973fc142ce1964083b483d4d60c8ea3c39f056ff0d6f7c53`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user