From 589958aa9361a20baa408400ffbee5090adab5b1 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 19 Sep 2026 13:22:25 +0000 Subject: [PATCH] docs: review document for #597 Issue: #597 User-Visible: no --- docs/reviews/SPEC-REVIEW-597-r1.md | 232 +++++++++++++++++++++++++++++ 1 file changed, 232 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-597-r1.md diff --git a/docs/reviews/SPEC-REVIEW-597-r1.md b/docs/reviews/SPEC-REVIEW-597-r1.md new file mode 100644 index 00000000..38dd907b --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-597-r1.md @@ -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 → в задаче** + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `83a07925465b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `bb4d62ab340508554c03325cb0b12628cc8204cd` + ``` + git log --all --format='%H %T' | grep bb4d62ab3405 + ``` +- Тело issue: `2e961eb72bcea811973fc142ce1964083b483d4d60c8ea3c39f056ff0d6f7c53` +- Вердикт конвейера: `yellow` · High 0