Files
2026-09-25 12:34:36 +00:00

19 KiB
Raw Permalink Blame History

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