diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 87c0e310..921e6201 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,9 +1,10 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1056, issue: 372. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1057, issue: 373. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| +| #649 | [SPEC-REVIEW-649-r1.md](SPEC-REVIEW-649-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | в скоупе задачи (возвращается автору); принято ревьюером с записью, правки не требует | `lab.js` `houseplan-card.ts` | | #647 | [SPEC-REVIEW-647-r1.md](SPEC-REVIEW-647-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 2 | П.4 ТЗ переносит на новый слот прежнее; ТЗ не упоминает и не защищает документированный | `src/styles/dialogs.styles.ts` `src/styles.ts` `smoke_glow_blending.mjs` `smoke_test_facade.mjs` `smoke_unified_wall_tool.mjs` `docs/UX-MODES.md` `docs/reviews/CODE-REVIEW-195-r1.md` `src/styles/chrome.styles.ts` | | #647 | [SPEC-REVIEW-647-r2.md](SPEC-REVIEW-647-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — | | #647 | [CODE-REVIEW-647-r1.md](CODE-REVIEW-647-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-649-r1.md b/docs/reviews/SPEC-REVIEW-649-r1.md new file mode 100644 index 00000000..75aa1511 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-649-r1.md @@ -0,0 +1,216 @@ +# SPEC-REVIEW-649-r1 + +Этап: spec (PROCESS.md §2.4). Заход r1, блокирующих циклов израсходовано 0 из 4 +(перед этим раундом). + +## Скоуп + +Issue #649 — продолжение #644: 2.5D выходит из experimental и становится +публичным режимом; в 2.5D меняются плитки маркеров/замков (объём, торец, тень +на полу), свет из окон (мягкая заливка по солнцу вместо проекции Flat-клиньев), +чинятся два бага декора/стен (толщина линий мебели, независимость цвета стен от +темы) и добавляется переключатель `settings.volumetric_view` в «Общих +настройках». Flat (2D), `houseplan-space-card` и оба редактора не меняются. + +Материал ревью — тело issue #649, раздел `## ТЗ` (редакция r1, 25.09), плюс +комментарий «Аналитика» с ответами владельца на Q1–Q3 от 25.09. ТЗ соответствует +полному треку (лёгкий трек явно отклонён в комментарии «Аналитика» с указанием +нарушенных критериев §5 — сложность 9/10, несколько поверхностей, новое +конфиг-поле с backend-валидацией, новый публичный UX-контракт, влияние на +производительность и touch). + +Работа сделана в скоупе `docs/SCOPE.md`: 2.5D-презентация канонического плана — +поименованное исключение #89; задача не вводит вторую модель, не трогает +декор-редактор и не расширяет его семантику. + +## Как проверялось + +1. Прочитаны `docs/SCOPE.md`, `docs/process/REVIEWER.md`, `AGENTS.md`, + `PROCESS.md` §2.4/§2.5/§7.1/§7.2/§2.10, тело issue #649 целиком, комментарий + «Аналитика». +2. Проверены обязательные разделы ТЗ по §7.1 (сценарий, что видно до/после, + проблема, скоуп/не-скоуп, контракт поведения, UX, модель данных и миграция, + i18n, AC1…AC13 с доказательством, план автотестов, риски, откат, + release-артефакты) — все присутствуют по существу (сценарий и «до/после» + объединены в одном абзаце, но обе продуктовые вещи в тексте есть: «что видит + администратор» и «что видит домочадец»). +3. Каждое утверждение раздела «Проблема (по коду `dev@b7079ead`)» и раздела + «Эталон и единицы» сверено с кодом на этом же SHA (рабочая копия уже на нём): + - `_renderSunRays`, `furnitureScreenScale`/`furniturePlanScreenScale`, + `openingInnerFaceOffsetFromIndex`, `windowLit`, `planSunAngle`, + `gridVisualUnits`, `ISO_WALL_HEIGHT`, `isoRaisedOverlayHalfSize`, + `wall_fill`, `houseplan_card_view_v1`, `projection-toggle`, `LABS_FLAGS` — + все существуют, `grep -rn` по `src/`; + - `src/houseplan-card.ts:8414` — `furnitureScreenScale = this._renderProjection + === 'iso' ? 1 : furniturePlanScreenScale(...)` — точное совпадение с текстом + issue и с формулировкой AC8/мутанта «`iso ? 1 : …` возвращён»; + - `src/styles/plan.styles.ts:338-368` — правила `.stage.theme-dark .iso-*`, + строка `757` — `.stage.projection-iso.theme-dark.mode-view .roomlabel` — + совпадает с заявленными диапазонами «~338–390» и «~740–760»; + - `src/labs.ts` — `LABS_FLAGS` содержит ровно один флаг `iso`; утверждение + «`hp_alpha` остаётся механизмом без экспериментов» после удаления `iso` + подтверждается — массив станет пустым, а не будет удалён весь механизм; + - `docs/SUN.md:208` — `RAY_MIN_COS = 0.05` — подтверждает решение «принято + предположительно»: порог окна `windowLit()` = 0.05, а не 0.08 лаборатории; + - `docs/ISOMETRIC.md` — камера 0°/20°, `wallHeight=84`, поворот створки + `50° × openingAmount` — подтверждает «Общие условия» (камера + 20°/0°/84/50° сохраняется, это не новое число); + - `src/houseplan-card.ts:2302-2305` (`_desiredProjection`) — iso уже сегодня + доступен только при `this._mode === 'view'`; заявление «редакторы и + `houseplan-space-card` — всегда Flat» не новое ограничение, а действующее + поведение, и `src/space-card.ts`/`src/space-render.ts` вовсе не содержат + projection-ветвления — задаче не нужно ничего менять для этого пункта; + - `src/editors/general-settings-dialog.ts:86-100` — карточка «Отображение» + сегодня содержит ровно два тумблера в порядке `gs-room-tooltip` → + `gs-radar-live` («Show live presence on the plan»); заявление «третьим + пунктом после …» в ТЗ арифметически верно; + - `src/i18n/{en,ru,de,fr}.json:766-767` — ключи `view.volumetric`/ + `view.flat` действительно существуют и будут освобождены, как написано; + - backend-прецедент: `custom_components/houseplan/validation.py:2256` + (`vol.Optional("show_room_tooltip"): bool`), `support_package.py:155-157`, + `scripts/config-schema.json:431` — ровно тот шаблон, который ТЗ предлагает + повторить для `volumetric_view`; + - `demo/performance/budgets-large-house-isometric.json`, + `budgets-isometric-stage3-dense.json` — профили из AC13 существуют. +4. Отдельно проверена внутренняя непротиворечивость числовых формул раздела 1 + (плитка/торец/тень) и раздела 2 (свет): пересчёт лабораторных единиц в базы + **D** и **H** (радиус 22/80=0.275 D, длина луча 910/218.8≈4.16 H, + 215/218.8≈0.98 H, 408/218.8≈1.865 H, blur 14/218.8≈0.064 H) — совпадает + поразрядно. +5. Не запускались никакие гейты (typecheck/test/build) — на этапе ревью ТЗ + продуктовый код ещё не пишется, гейты неприменимы (§2.5, §8). + +## Находки + +### Medium — в скоупе задачи (возвращается автору) + +**M1. Тень плитки: не определена opacity для пересечения «тёмная тема + светлый +пол», хотя это одна из четырёх утверждённых опорных комбинаций (вложение 13).** + +Раздел «1. Иконки-плитки», подраздел «Тень на полу»: + +> Opacity: белые тела 0.34, цветные тела и бейджи 0.50. **Тёмная тема — 0.40** +> (цветные 0.50). +> … +> **Светлый пол**: opacity 0.30 (цветные 0.42), размытие 0.1375 D, сдвиг +> (0.1 D, 0.425 D). + +Тема и пол — независимые переменные (пол вычисляется по яркости заливки +комнаты/бумаги плана, тема — по UI; ничто в ТЗ не привязывает одно к другому, +и пример из «Эталон и единицы» специально существует для случая «тёмная тема, +светлый пол» — вложение 13, `13-lab-sketch07-darktheme-lightfloor-approved.png`, +утверждено владельцем). Текст задаёт модификатор темы (0.40/0.50, без +привязки к полу) и модификатор пола (0.30/0.42, без привязки к теме), но не +говорит, что происходит при обеих модификациях сразу: остаётся 0.40 (тема +победила), становится 0.30 (пол победил) или это третья, ещё не названная +величина. Blur и сдвиг для этой комбинации тоже не названы отдельно — +неявно предполагается, что там побеждает правило пола (0.1375 D / сдвиг 0.425 D), +раз только у пола есть blur/сдвиг-модификатор, но для opacity такой +однозначности нет, потому что модификатор есть у обеих переменных. + +Это не гипотетическая придирка: в «Аналитика» написано «Кадры сняты во всех +четырёх сочетаниях темы и пола» — то есть у автора ТЗ физически есть +референсный кадр с точным числом для этой комбинации (из `lab.js`/ +`applyTheme()` или скриншота), просто оно не перенесено в текст. AC4 отдельно +требует «сдвиг/opacity/размытие по полу **и** теме» (обе переменные явно +названы как определяющие), а AC12 требует side-by-side сравнение именно с +вложением 13 — то есть на кодревью потребуется точное число, а разработчик, +не имея его, либо угадает, либо придумает своё правило приоритета, которое +затем не совпадёт с рефересом без объяснения (сам AC12 называет это находкой). + +**Чем закрывается:** одна строка в ТЗ, например «Тёмная тема + светлый пол: +opacity N (цветные M), размытие/сдвиг как у светлого пола» — с числом, +проверенным по `lab.js`/`applyTheme()`, а не придуманным заново. Дешёвая правка, +без пересмотра остальной части раздела. + +### Low — принято ревьюером с записью, правки не требует + +**L1. Строка кода для группы `iso-floor-scene` в разделе «Проблема» указана +неточно.** Текст: «`houseplan-card.ts` ~9870, группа `iso-floor-scene`»; на +SHA `b7079ead` фактическая строка — `10901` +(`grep -n "iso-floor-scene" src/houseplan-card.ts`). Расхождение ~1000 строк в +файле на 12882 строки — не влияет ни на один AC (это справочный указатель для +реализующего агента, не проверяемое утверждение поведения), другие +координаты того же раздела (`8414`, `338-368`, `757`) точны. Оставляю без +правки: не блокирует, автор может поправить попутно или проигнорировать. + +## Что проверено и корректно + +- Обязательные разделы ТЗ по §7.1 присутствуют по существу (см. «Как + проверялось», п. 2). +- Все технические утверждения раздела «Проблема» и «Эталон и единицы» о + текущем состоянии кода подтверждены чтением кода на SHA `b7079ead` (список + выше) — ни одно не оказалось догадкой, выданной за факт. +- Открытые продуктовые вопросы закрыты: Q1–Q3 из тела issue отвечены + владельцем в чате 25.09 и зафиксированы в комментарии «Аналитика» с прямой + ссылкой на источник; ответы корректно перенесены в текст ТЗ (виртуальные + устройства без пунктира; отдельной настройки яркости нет; переключателя на + карте нет). +- Раздел «Принято предположительно» корректно отделяет техническое решение + (можно менять свободно) от продуктового (уже решено владельцем) — ни одно + техническое решение оттуда не выглядит замаскированным продуктовым вопросом. +- Таблица AC1…AC13 — для каждого защитного AC заполнены все три столбца + (AC · чем доказан · чем краснеет), пустых «чем краснеет» нет (§2.7, #435); + для AC12 (сравнение с макетом, не защита) корректно используется другой тип + свидетеля — сравнение, а не мутация, что соответствует исключению из §2.7. +- AC13 (производительность) корректно вынесен в предрелизный гейт, а не + заявлен как проверяемый на этом ревью — совпадает с практикой AGENTS.md + («Gates», performance/golden/HA — предрелизно). +- Модель данных: поле `settings.volumetric_view` (bool, default false, + хранится только при true) повторяет уже принятый в проекте паттерн + (`show_room_tooltip`, `sun_ray_origin` — см. `docs/CONFIG-COMPATIBILITY.md`); + backend-план (`validation.py`/`support_package.py`/`config-schema.json`) + указывает точные прецедентные места. +- Явно названный откат: `volumetric_view=false` возвращает Flat без + перезагрузки, старый бандл игнорирует поле по схеме — соответствует §2.5. +- i18n: заголовок/описание даны на en/ru/de/fr; ключ `view.volumetric`/ + `view.flat` действительно существует и корректно помечен к освобождению. +- Дискрепанс дизайн-референса замечен и разрешён самим автором заранее: + вложение 11 показывает янтарную рамку наведения (правило лаборатории для + светильников), а ТЗ явно фиксирует, что нормативен цвет `#0C82F0` из пакета + иконок, вложение 11 — эталон только формы/подъёма. Без этой оговорки + ревьюер кода почти наверняка завёл бы ложную находку на кодревью. + +## Чего не проверял + +- Не открывал архив лаборатории (`first-floor-lab-2026-09-25-sketch07.zip`) и + не сверял `lab.js`/`lab.css` построчно с текстом ТЗ — кроме найденной выше + комбинации, доверился утверждению автора о полном прочтении (это его + ответственность по «Материалы прочитаны целиком»); частичная перепроверка + других чисел (радиус, длина луча, blur) по единицам D/H сошлась без + расхождений (см. «Как проверялось», п. 4), что даёт основания доверять + остальным цифрам, но не является гарантией на 100% чисел документа. +- Не проверял математически формулу параллелограмма луча + `s = n·L + t·L·(dir·t)/(dir·n)` с нуля (геометрический вывод из физики + теней) — это проверяемо только исполнением на кодревью через + `smoke_iso_sun`/`test/iso-sun*.test.mjs`, а не чтением спецификации. +- Не запускал `npx tsc --noEmit`, `npm test`, `npm run build` — на этапе + ревью ТЗ продуктовый код не меняется, гейты неприменимы. +- Не сверял восемь вложений (09–16) попиксельно с текстом ТЗ — это работа + AC12 на кодревью (side-by-side, `docs/design/649-25d-stage6/ACCEPTANCE.md`), + не задача ревью текста. +- Не проверял `scripts/config-field-registry.mjs` на предмет обязательности + регистрации нового поля `volumetric_view` — `docs/CONFIG-COMPATIBILITY.md` + прямо говорит, что реестр пока не покрывает все публичные поля, так что + отсутствие явного плана регистрации в ТЗ не расценено как находка. + +## Вердикт + +Ровно одна находка Medium в скоупе задачи, High нет. Возвращается автору для +однострочного уточнения M1; остальное ТЗ пригодно к разработке без изменений. + +Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `b7079ead801f` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `27ed2efaa7081238ee82e7c0d965016363dcba54` + ``` + git log --all --format='%H %T' | grep 27ed2efaa708 + ``` +- Тело issue: `6d3993469405770c498f080b1f6e13b0fe2c05c511c48ccd12bc62884fd3c151` +- Вердикт конвейера: `yellow` · High 0