diff --git a/docs/reviews/SPEC-REVIEW-230-r2.md b/docs/reviews/SPEC-REVIEW-230-r2.md new file mode 100644 index 00000000..4b64da07 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-230-r2.md @@ -0,0 +1,183 @@ +# SPEC-REVIEW-230-r2 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/230 +- **ТЗ:** `docs/specs/230-hatch-density-normalization.md` +- **Ветка / SHA:** `issue/230-hatch-density-normalization` @ `44a35fd` +- **Этап:** spec (PROCESS.md §2.4) · трек: `small` (метка на issue) +- **Заход:** r2 · блокирующих циклов израсходовано 1/4 до этого вердикта +- **Ревьюер:** Claude, роль «ревьюер ТЗ» +- **Предыдущий раунд:** `SPEC-REVIEW-230-r1.md`, вердикт жёлтый на SHA `2396044` + (комментарий issue от 2026-08-21T11:46:32Z). SHA в вердикте r1 не был назван + явно в тексте — восстановлен по коммиту `docs: review document for #230` + (`6872dc4`, родитель `2396044`) и подтверждён телом комментария. + +## Скоуп раунда + +Разбор по дельте (PROCESS.md §2.10): сравнивается `2396044` (SHA r1) → +`44a35fd` (текущий HEAD). Дельта — правка того же файла ТЗ, `git diff +2396044..44a35fd -- docs/specs/230-hatch-density-normalization.md`, 42 +вставки / 12 удалений, локализована в §11 п.2, §12 (AC10, новые AC11/AC12), +§13, §14, §15. Код не менялся (реализации всё ещё нет, этап S4). Дельта +локальна и продуктовое решение владельца не меняет — полный повторный +разбор всего документа не требуется; §1–§10, §16, §17 и AC1–AC9 +наследуются из r1 (см. раздел «Унаследовано»). Отдельно проверено, не +создала ли правка новое противоречие с неизменённой частью документа — +нашла (см. Low-1 ниже). + +## Как проверялось + +1. Найден вердикт r1 и SHA, на котором он получен (см. выше). +2. Получены `git diff 2396044..44a35fd` и текущий полный текст файла ТЗ. +3. По каждой находке r1 (High-1, Medium-1, Low-1) построчно сверено, что + именно в дельте её закрывает — раздел «Закрытие раунда r1» ниже. +4. Пересчитана таблица шагов в §11 п.2 самостоятельно, по той же формуле, + что цитирует ТЗ: `inv = max(0.4, 1/max(zoom,0.4))`, шаг-сейчас = `8·inv`. + - `zoom 0.4`: `inv = max(0.4, 1/0.4) = 2.5` → `8·2.5 = 20.0` — совпадает + со строкой ТЗ. + - `zoom 2.5`: `inv = max(0.4, 1/2.5) = max(0.4,0.4) = 0.4` → `8·0.4 = 3.2` + — совпадает. + - «шаг после» для обеих сцен — `wallHatchStepUnits(5) = 8·(5/5) = 8` — + совпадает с заявленным. +5. Прочитан текущий AC10–AC12 и сверен с §13 (план тестов) и §14 (мутанты): + каждый новый AC покрыт ровно одним пунктом плана тестов и как минимум + одним мутантом-гвардом; осиротевших AC нет, осиротевших мутантов нет. + `wallHatchStepUnits(25) = 8·(5/25) = 1.6`, что отличимо от старой + константы `8` в `space-render.ts` — AC12 способен упасть, если правку + второго рендерера не сделают. +6. Прочитано тело issue #230 и все три комментария (r1-заявка автора, + вердикт r1, ответ автора на r2) — расчёт таблицы в комментарии автора + совпадает с расчётом в файле ТЗ и с пересчётом в п.4. +7. Проверено, не противоречит ли скорректированный §11/AC10–12 + неизменённым разделам документа. Нашлось: §5 («Цели»), второй пункт — + см. Low-1. +8. Код, гейты, автотесты не запускались — реализации нет, это ожидаемо на + этапе ревью ТЗ (как и в r1). + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **High-1** — риск-анализ и AC10 построены на неверной посылке «фикстуры не меняют зум» | §11 п.2 переписан: явно называет обе зумовые сцены, приводит таблицу «шаг сейчас/после», признаёт изменение ожидаемым следствием §4.2. AC10 сужен до «расходится ровно на двух сценах, третья — провал». Добавлен AC11 на осознанное переснятие через `golden:accept -- --reviewed` | `docs/specs/230-hatch-density-normalization.md:189-207` (§11 п.2), `:244-246` (AC10), `:248-250` (AC11) | +| **Medium-1** — контракт «оба рендерера» не имел собственного AC для `space-render.ts` | Добавлен AC12: `space-render.ts` при `cell_cm: 25` строит `width`/`height` по `wallHatchStepUnits(25)`, без `scale`. Смок `demo/smoke_wall_hatch_density.mjs` расширен на статический рендерер. Добавлен мутант `hatch-static-renderer-untouched` | `:252-256` (AC12), `:261-263` (§13), `:276` (§14) | +| **Low-1** — `WALL-THICKNESS.md` не входил в release-артефакты | §15 явно называет `docs/WALL-THICKNESS.md` каноническим документом подсистемы и требует правки в нём в том же коммите | `:282-285` (§15) | + +Все три находки r1 закрыты по существу, не декларативно: формулы в новой +таблице §11 п.2 пересчитаны независимо (см. «Как проверялось», п.4) и +совпадают; AC12 действительно различим от старого поведения численно (8 vs +1.6), то есть способен упасть, если правку в `space-render.ts` не внесут; +AC10 в новой формулировке — falsifiable в обе стороны (ловит и «третью» +разошедшуюся сцену, и случай, когда ожидаемые две сцены не разошлись). + +## Унаследовано из r1 + +Принято без повторной проверки, документ `SPEC-REVIEW-230-r1.md` на SHA +`2396044`; дельта r1→r2 их не касается: + +- §1–2 продуктовая рамка (сценарий/персона, видимое изменение до/после) — + отвечает на оба обязательных вопроса, персона из `docs/SCOPE.md` J4/J6. +- §3 подтверждённая причина и вывод «формула issue неверна, верный + множитель — `5/cell_cm`» — проверено алгебраическим пересчётом в r1. +- §4 продуктовые решения владельца — зафиксированы, не менялись. +- §6–7 Scope / Не входит в задачу — включая проверку по коду, что + `space-card.ts` не третий независимый потребитель паттерна. +- §8 Контракт (шаг, применение, пределы, защита от каши) и алгебраическое + доказательство AC2/AC3 (независимость числа полос от `cell_cm`, + пропорциональность толщине). +- §9–10 данные/i18n/perf. +- AC1, AC4–AC9 — однозначны, проверяемы, указывают способ доказательства; + не затронуты дельтой. +- §16 Откат, §17 Принятые предположения. +- Обязательные разделы §7.1 присутствуют по существу (полный перечень — в + r1). +- Мутационный гейт для мутантов, унаследованных из r1 + (`hatch-step-ignores-cell-cm`, `hatch-step-inverted`, + `hatch-step-unclamped`, `hatch-stroke-not-scaled`, + `hatch-zoom-compensation-back`, `hatch-density-solid-threshold-off`). + +## Находки + +### Low-1 — §5 «Цели» не обновлён вслед за исправлением §11/AC10–11 и противоречит им + +**Файл:** `docs/specs/230-hatch-density-normalization.md:92-93` (§5, второй +пункт). + +**Формулировка ТЗ:** «При `cell_cm: 5` вид не меняется — ни на карте, ни в +статическом рендерере, ни в golden-эталонах.» + +**Проблема.** Это безусловное утверждение написано до правки r2 и не +обновлено вместе с ней. После правки §11 п.2 и AC10/AC11 прямо утверждают +обратное для двух конкретных golden-эталонов: `large-house-zoom-040-dark` и +`large-house-zoom-250-dark` — обе сцены сняты на фикстуре `cell_cm: 5` +(`perf-floor-1`), и обе, по признанию самого документа, «изменятся — и это +прямое следствие решения владельца §4.2». Причина расхождения — не в +`cell_cm`-зависимой части правки (при `cell_cm: 5` шаг действительно не +меняется, `8 → 8`), а во втором, отдельном решении владельца — удалении +зумовой компенсации. §5 говорит только про `cell_cm: 5` и не делает +оговорки про зум, поэтому как написано противоречит §11/AC10/AC11 в одном и +том же документе для того же значения `cell_cm`. + +**Почему не блокирует.** ACs 10, 11 и 12 — обязывающая, точная и +самодостаточная часть контракта; они не ссылаются на §5 и не зависят от +его формулировки. Разработчик, который реализует задачу по AC, получит +верное поведение независимо от того, поправлена ли фраза в §5. Риск — +не в неверной реализации, а в том, что читатель, остановившийся на §5, +сделает неверный вывод о golden без учёта §11 — то есть это дефект +качества текста, не дефект контракта. + +**Что нужно.** Одна фраза-оговорка в §5, например: «...кроме двух +golden-сцен, снятых при зуме ≠ 1 (`large-house-zoom-040-dark`, +`large-house-zoom-250-dark`) — см. §11 п.2» — либо сузить формулировку до +«шаг паттерна не меняется» (что действительно верно при `cell_cm: 5` +безусловно), не упоминая golden-эталоны в этом пункте вовсе. + +**Решение ревьюера.** Low, не блокирует и не влияет на проверяемость +AC10–AC12 — снимаю с рекомендацией поправить одной фразой при следующей +правке файла (не обязательно отдельным циклом). + +## Что проверено и корректно + +- Все три находки r1 закрыты по существу (см. таблицу выше), включая + пересчёт числовых значений, а не просто присутствие текста. +- Новый AC10 falsifiable в обе стороны: провалится и если разойдётся + третья сцена, и если ожидаемые две не разойдутся вовсе (что покрывает + ранее открытый в r1 вопрос — попадает ли `perf-floor-1` в режим `solid` + на этих зумах; правильный исход теперь виден по факту прогона, а не + предсказывается заранее). +- AC11 и AC12 указывают способ доказательства (процесс `golden:accept -- + reviewed` с записью в отчёте; браузерный смок) и не оставляют + пространства для интерпретации. +- §13/§14 согласованы с новыми AC: ни один AC не остался без пункта плана + тестов, ни один новый мутант не остался без AC-обоснования. +- §15 теперь называет все три необходимые точки: оба CHANGELOG, + `USER-GUIDE.ru.md`, канонический `WALL-THICKNESS.md`. + +## Продуктовые вопросы владельцу + +Не требуются. Единственная находка раунда (Low-1) — текстовая +несогласованность внутри документа, решается редактурой, не требует +нового продуктового решения: владелец уже высказался (§4 п.2, комментарий +от 2026-08-21), автору дельты нужно распространить эту формулировку на §5. + +## Чего не проверял + +- Реализацию — её не существует на этом этапе (не изменилось с r1). +- Полный построчный обзор `demo/golden/matrix.mjs` (~40 сцен) на предмет + третьих сцен с `zoom ≠ 1` вне `large`/`perf-floor-1` — не требуется для + вывода этого раунда: AC10 в текущей формулировке сам ловит любую такую + сцену как провал критерия на этапе код-ревью/CI, независимо от того, + нашёл ли её ревьюер ТЗ заранее. +- Действительно ли `perf-floor-1` на этих двух зумах уже сегодня в режиме + `solid` — не пересчитывалось; не влияет на вывод, так как новая + формулировка AC10 корректно обрабатывает оба исхода (см. «Что проверено + и корректно»). +- Гейты (`tsc`, `npm test`, `npm run build`, смоки, `golden:verify`) не + прогонялись — стадия spec, кода нет; это симметрично r1 и ожидаемо. + +## Вердикт + +Все блокирующие и заявленные-в-скоупе находки r1 (High-1, Medium-1, Low-1) +закрыты по существу и проверены пересчётом, а не на слово автора. Единственная +находка этого раунда — Low, текстовая несогласованность §5 с исправленным +§11/AC10–11, не влияющая на проверяемость контракта; снимается решением +ревьюера с рекомендацией автору. High: 0, Medium: 0. Итог — зелёный: ТЗ +готово к статусу «Готово к разработке» (PROCESS.md §2.5).