diff --git a/docs/reviews/SPEC-REVIEW-266-r2.md b/docs/reviews/SPEC-REVIEW-266-r2.md new file mode 100644 index 00000000..0ca57ff2 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-266-r2.md @@ -0,0 +1,164 @@ +# SPEC-REVIEW-266-r2 + +Issue: [#266](https://github.com/Matysh/houseplan-card/issues/266) — «Рефакторинг 3/5: расщепить styles.ts» +ТЗ: `docs/specs/266-split-styles.md` (коммит `afb6e151`, ветка `issue/266-split-styles`) +Предыдущий раунд: SPEC-REVIEW-266-r1, вердикт жёлтый, ТЗ проверялось на коммите `4c93f28e` +Этап: spec (PROCESS.md §2.4) · заход r2 · блокирующих циклов израсходовано 1/4 +Вердикт: **жёлтый** + +## Скоуп ревью + +Второй заход, разбор по дельте (PROCESS.md §2.10, issue #214). Предмет — +изменения ТЗ между `4c93f28e` (на чём получен вердикт r1) и `afb6e151` +(текущий HEAD): + +``` +git diff 4c93f28e..afb6e151 -- docs/specs/266-split-styles.md +``` + +Дельта — 7 вставок / 5 удалений в одном файле: строка «Статус», §1.3.5 +(инвариант 5), новый §1.3.6 (инвариант 6), AC2, AC6 + новый AC6a, §7. Правка +только текстовая (ТЗ), кода в ветке ещё нет — соответствует масштабу этапа. +Дельта локальна: не ребейз, не смена контракта поведения, не новая подсистема, +объём несопоставим с исходной задачей → полный повторный разбор не требуется, +проверяю дельту плюс всё, до чего она дотягивается (AC2, AC6/6a, §7, и числа, +на которые они ссылаются). + +## Как проверялось + +- Прочитан диф `4c93f28e..afb6e151` целиком (выше). +- Прочитан документ `docs/reviews/SPEC-REVIEW-266-r1.md` (коммит `5c466b6c`) — + источник находок M1/L1/L2. +- Прочитаны оба комментария автора между раундами (аналитика не трогалась; + комментарий ревизии 2 от `Matysh` перечисляет фиксы по пунктам). +- Для M1 (r1): сверено текущее число `@media` блоков в `src/styles.ts` — + `grep -n "@media (forced-colors\|@media (prefers-reduced-motion)"` → + 8× `prefers-reduced-motion`, 2× `forced-colors`, совпадает с текстом нового + AC6a дословно. +- Для M1 (r1): проверено существование смоков, названных в новом §1.3.6/§7 — + `demo/smoke_plan_snap_overlay.mjs` содержит `emulateMedia({forcedColors: + 'active'})` (строка 354), `demo/smoke_preloader.mjs` содержит + `emulateMedia({reducedMotion:'reduce'})` (строка 121) — оба реальны, а не + выдуманы под текст ТЗ. +- Для L1 (r1): сверена формулировка §1.3.5 — «класс B, остаётся в + репозитории» вместо прежнего «класс C, удаляется или остаётся по решению + ревью»; таблица классов PROCESS.md §1 подтверждает `scripts/**` = класс B. +- Для L2 (r1) / нового AC2: пересчитана арифметика нового порога вручную — + находка ниже. +- `scripts/dev/styles-diff.mjs` — файла в дереве нет ни на одном коммите + ветки (`git show afb6e151:scripts/dev/styles-diff.mjs` → not found). + Ожидаемо: этап всё ещё spec, кода нет и не должно быть; текст ТЗ описывает + будущий инструмент, а не отчитывается о готовом. +- Тяжёлые гейты (typecheck/test/build/golden) не гонял — на этапе ревью ТЗ + диф состоит из одного файла документации, гонять их не по чему; то же + решение принял r1 и оно не оспаривается. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где видно | +|---|---|---| +| **M1** (Medium, в скоупе) — golden не видит `forced-colors`/переезд `@media`-обёрток, сверочный инструмент сравнивал правила без контекста вложенности, план тестов не называл смоки | §1.3.5 переписан: ключ сверки — «полный путь вложенности (`@media`/`@supports`/`@keyframes`) + селектор»; новый §1.3.6 называет `smoke_plan_snap_overlay.mjs` и `smoke_preloader.mjs` обязательными на слайсах `plan`/`base`; новый AC6a — юнит присутствия обоих `forced-colors` и всех 8 `prefers-reduced-motion` блоков в склейке; §7 явно перечисляет и смоки, и юнит | `docs/specs/266-split-styles.md` §1.3.5, §1.3.6, AC6, AC6a, §7 (все — в дифф `4c93f28e..afb6e151`) | +| **L1** (Low) — `scripts/dev/styles-diff.mjs` назван классом C, по таблице PROCESS.md §1 должен быть B | §1.3.5, последнее предложение: «класс B, остаётся в репозитории» | `docs/specs/266-split-styles.md` §1.3.5 | +| **L2** (Low, наблюдение) — грубая оценка `plan` ≈ 1300–1400 против лимита AC2 «≤ 1200» могла не сойтись | AC2 переписан на фактический замер прототипа (base 300 / chrome 447 / devices 623 / dialogs 1452 / plan 1609), пороги подняты до «каждый ≤ 1650» | `docs/specs/266-split-styles.md` AC2 — **но см. новую находку M2 ниже: сама правка вводит арифметическое противоречие** | + +## Унаследовано из r1 + +Без повторной проверки, по документу `docs/reviews/SPEC-REVIEW-266-r1.md` на +коммите `4c93f28e` — дельта r2 этих разделов не касается: + +- §7.1 обязательные разделы ТЗ присутствуют, «сценарий» и «что человек + увидит» оформлены честно для задачи без пользовательского эффекта + (прецедент `docs/specs/034-frontend-decomposition.md`). +- Числа §0/§1.1 сверены с деревом на момент r1 (`wc -l src/styles.ts` → 3690, + 129 golden-сцен, три потребителя `cardStyles`) — §0/§1.1 не менялись в r2. +- Селекторные кластеры §1.1 и распределение по файлам (`base`/`plan`/ + `devices`/`chrome`/`dialogs`) — не менялись. +- §1.2 (сборщик, внешний контракт `cardStyles`, потребители не меняются) — + не менялось. +- §1.4 (порядок слайсов: chrome → dialogs → devices → plan → base) — не + менялось. +- §2 (скоуп/не-скоуп), §3 (UX/данные/i18n/touch — не затрагиваются), §4 + (риски 1–3), §5 (release-артефакты, CHANGELOG не трогается) — не менялись. +- AC1, AC3, AC4, AC5, AC7, AC8 — текст не менялся, доказательства прежние + (юнит §1.3.3, `golden`-прогон CI, юнит непересечения селекторов, числа в + хендоффе). +- §8 (откат) — не менялось. +- Классификация задачи класс A, полный флоу — не менялась. + +## Находки (r2) + +### M2 — Medium (в скоупе) — новый AC2 противоречит сам себе: заявленный порог суммы уже нарушен собственным же измерением прототипа + +Текст AC2 (`docs/specs/266-split-styles.md`, раздел «6. AC», пункт 2): + +> Пять файлов `src/styles/*.styles.ts`; фактический замер прототипа +> расщепления: base 300, chrome 447, devices 623, dialogs 1452, plan 1609 +> строк — порог: каждый ≤ 1 650, сумма пяти файлов + сборщик ≤ исходных 3 690 +> строк + 5 % (накладные заголовки/импорты). + +Арифметика заявленного «фактического замера»: + +``` +300 + 447 + 623 + 1452 + 1609 = 4431 +3690 × 1.05 = 3874.5 +``` + +4431 > 3874.5 — сумма, которую автор приводит как уже измеренный факт +прототипа, на 557 строк (≈14 %) больше собственного же порога, и на 741 +строку (≈20 %) больше исходных 3690 без всякой надбавки. Пять строк спустя +после введения этого «факта» AC требует, чтобы итоговая сумма файлов +(практически те же пять файлов) не превышала прежний объём + 5 %. + +Формулировка «накладные заголовки/импорты» подразумевает, что запас 5 % +покрывает несколько строк `import`/шапки на файл (реалистично — единицы, +десятки строк на 5 файлов), а не рост на ~740 строк. Источник этого роста в +ТЗ не назван: ни перенос комментариев владения (§1.1: «правило, обслуживающее +две поверхности, уходит в base с комментарием»), ни форматирование, ни что- +либо ещё не объясняет разрыв в 20 %. + +Как ровно это ТЗ формулирует критерий состязательного ревью (стр. 130 этого +документа выше): «его задача — не согласиться, а найти, где ТЗ не выполнимо». +AC2 в текущей редакции не выполним: либо порог суммы нужно поднять до +согласующегося с уже измеренным прототипом значения (например, «сумма ≤ +факту прототипа + запас», либо явно назвать источник 20 %-го роста), либо +убрать sum-порог и оставить только «каждый файл ≤ 1650» (per-file порог сам +по себе непротиворечив — 1609 ≤ 1650 с запасом всего 41 строка, но это уже +отдельный, некритичный риск, не новая находка). + +**Сценарий отказа, если не поправить:** автор реализует слайсы, суммарный +результат близок к измеренному прототипу (4431+ строка, что и естественно — +прототип уже есть), AC2 формально красный на код-ревью несмотря на то, что +реализация в точности соответствует заявленному прототипу этого же ТЗ — +цикл код-ревью тратится на обсуждение порога, который был неверен ещё на +этапе ТЗ. + +Фикс дёшев (правка одной цифры/формулировки в AC2), в рамках уже +запланированного разбора — не требует нового issue. + +## Что проверено и корректно + +- Все три находки r1 (M1, L1) закрыты по существу — не декларативно, а + правками текста, дословно устраняющими описанный в r1 сценарий отказа: + scope-ключ сверки действительно покрывает потерю/смену `@media`-обёртки, + названные смоки реальны и действительно эмулируют нужные состояния, юнит + AC6a называет ровно те числа блоков, что есть в `src/styles.ts` сейчас + (8 reduced-motion, 2 forced-colors). +- L1 закрыт корректно и однозначно (класс B соответствует таблице + PROCESS.md §1). +- Остальной текст ТЗ (структура каталога, сборщик, порядок слайсов, риски, + откат, не-скоуп) не тронут дельтой и не имеет отношения к новым находкам — + наследуется из r1 без повторной проверки (раздел выше). +- Раздел «Статус: ревизия 2» корректно и по существу описывает, что именно + изменилось, со ссылкой на SPEC-REVIEW-266-r1 — трассируемость в порядке. + +## Чего не проверял + +- Тяжёлые гейты (`typecheck`/`test`/`build`/`golden`) — на этапе ревью ТЗ + диф состоит из одного файла `docs/specs/*.md`, гонять их не по чему; + соразмерно этапу, то же решение принял r1. +- Содержимое `scripts/dev/styles-diff.mjs` — файла ещё не существует ни в + одном коммите ветки; на этапе spec это ожидаемо, а не пропуск. +- Построчный состав будущих файлов `src/styles/*.styles.ts` за пределами + чисел AC2 — кода ещё нет, будет предметом код-ревью. +- Мутационный гейт — не требуется по тексту ТЗ на этом этапе (§7), не + оспаривается повторно (r1 согласился).