Files
houseplan-card/docs/reviews/SPEC-REVIEW-577-r1.md
2026-09-14 20:38:41 +00:00

17 KiB
Raw Permalink Blame History

SPEC-REVIEW-577-r1

Issue: #577 «Солнечные лучи: выбирать внутренние или внешние углы окон» Этап: ТЗ на ревью (PROCESS.md §2.4), трек: полный (аналитика #577 явно называет провал критериев small: новый UX-контракт + новое compatibility-поле). Заход: r1 · блокирующих циклов израсходовано (до этого вердикта) 0 из 4.

Материал ревью

Тело issue #577, раздел ## ТЗ, на момент разбора — единственная редакция (комментарий-хендофф Сделано: аналитика и полное ТЗ добавлены в тело issue... Следующий статус: S4-spec-review, других правок после него не было). Метка на issue — S4-spec-review, P3, feature.

Скоуп задачи

Одно глобальное compatibility-поле settings.sun_ray_origin: 'inner' | 'outer', выбирающее грань окна, от которой строится источник уже существующих солнечных лучей (docs/SUN.md). Геометрия направления, длины, затухания, rim-линий, кеша, порога 3°, per-space sun_rays/north_deg не меняется — меняется только исходная грань пролёта. По docs/SCOPE.md это в рамках J1 («живой обзор дома на плане»): визуальный тюнинг уже одобренной (2026-08-03) фичи, не новая функция и не редактор.

Как проверялось

Ревью текстовое (этап spec, кода нет): тело issue #577 и его комментарии; docs/SCOPE.md, AGENTS.md, PROCESS.md §2.3/§2.4/§7.1/§2.5; канонический документ подсистемы docs/SUN.md полностью; смежный канон docs/WALL-THICKNESS.md §4–5 (Floor/fills/light/area, Sun); docs/USER-GUIDE.ru.md раздел 15 «Солнце: фон и оконные лучи» и раздел «Толстые стены»; docs/CONFIG-COMPATIBILITY.md (реестр compatibility-полей, разделы про bg_mode #146 и plan_only #167 как прецедент оформления новых compatibility-полей); src/i18n/{ru,en,de,fr}.json — проверка реальной строки gs.sun_rays/space.sun_rays, использованной в UX-разделе ТЗ, против живого кода (не против устаревшего текста гайда). Гейты (typecheck/test/build/golden) на этом этапе не гоняются — кода ещё нет, это чтение и сверка текста.

Находки

Medium (в скоупе задачи) — раздел «Release-артефакты» не называет два документа, которые эта же задача обязана обновить

Файл: тело issue #577, раздел ### Release-артефакты.

Суть: список документации к обновлению — docs/CHANGELOG.md/.ru.md, docs/SUN.md, docs/USER-GUIDE.ru.md, docs/TESTING.md, i18n, golden, docs screenshots. В нём отсутствуют два документа, которые прямо и однозначно затронуты контрактом этой задачи:

  1. docs/WALL-THICKNESS.md §5 «Sun» — действующий канон прямо и безусловно формулирует ровно то поведение, которое outer меняет: «Wedges do not draw through wall bodies. Their full source span is translated from the centreline by half the wall depth along the receiving room's inward normal... Clip to the receiving room's inner contour.» После реализации outer эта фраза станет фактически неверной для одного из двух режимов (AC4 контракта ТЗ: «луч... проходит только через физический проём в теле стены» — то есть именно draws through the wall body). docs/SUN.md в разделе «Window light wedges» сам ссылается на docs/WALL-THICKNESS.md за этой геометрией («clipped by the receiving room's inner contour when wall thickness is set... see docs/WALL-THICKNESS.md»), то есть это не побочный, а прямо цитируемый источник контракта.
  2. docs/CONFIG-COMPATIBILITY.md — реестр каждого нового additive/compatibility-поля с fail-closed default; в файле уже есть прецедентные записи именно такого рода для settings.bg_mode («Four-phase background default and transfer (#146)») и plan_only («Additive plan-only space transfer (#167)»). Новое поле settings.sun_ray_origin — тот же класс изменения (глобальное enum-поле, inner по умолчанию, read-compatibility для отсутствующего/невалидного значения), но записи о нём в списке release-артефактов нет.

Сценарий отказа: исполнитель реализует задачу строго по перечисленным в ТЗ release-артефактам (SUN.md, USER-GUIDE.ru.md, TESTING.md, changelog) и проходит код-ревью. docs/WALL-THICKNESS.md остаётся с безусловным «Wedges do not draw through wall bodies» — читатель канона (следующий автор, следующий ревьюер по смежной задаче геометрии) получит неверную информацию о текущем поведении. Новое поле не появляется в реестре docs/CONFIG-COMPATIBILITY.md — следующий аудит совместимости (npm run audit:config) не будет знать о нём как о зарегистрированном compatibility-поле, и разрыв документации с кодом (ровно то, из-за чего в docs/USER-GUIDE.ru.md уже разошлась подпись переключателя «Солнце в окнах» против кода — см. «Что проверено» ниже) повторится ещё раз, уже в другом документе.

Почему это Medium, а не High: сама реализация и AC не блокируются — AC1–AC9 доказуемы независимо от того, назван ли WALL-THICKNESS.md в списке. Это дефект полноты обязательного раздела ТЗ (DoR, PROCESS.md §2.5: «release-артефакты названы... либо явное «нет»»), правится добавлением двух строк в существующий раздел, без изменения контракта, AC или скоупа — классический Medium-в-скоупе (§2.4, #202): чинится в этой же задаче, отдельный issue не заводится.

Что проверено и корректно

  • Обязательные разделы §7.1 — сценарий, «что человек увидит до/после», проблема (вынесена в ## Проблема перед ## ТЗ, тело issue едино), скоуп/не-скоуп, контракт поведения, UX, модель данных и совместимость, i18n, критерии приёмки с доказательством, план автотестов, риски, откат, release-артефакты — присутствуют все.
  • Default и fail-closed (контракт п.2) согласован с действующим паттерном additive-поля в проекте (тот же приём, что bg_mode, CONFIG-COMPATIBILITY.md «Four-phase background default and transfer»): отсутствующее/невалидное значение → inner, без визуальной миграции существующих установок — корректно и проверяемо (AC2).
  • Геометрия inner (контракт п.3) дословно совпадает с текущим каноном docs/WALL-THICKNESS.md §5 и docs/SUN.md («translated from the centreline by half the wall depth along the receiving room's inward normal») — описание существующего поведения не искажено.
  • Терминология UX-раздела. ТЗ ссылается на переключатель «Солнце в окнах» непосредственно перед новым select. Строка «Солнце в окнах» — это ТОЧНОЕ совпадение с живым gs.sun_rays/space.sun_rays в src/i18n/ru.json, а не с текстом docs/USER-GUIDE.ru.md (там уже устаревшее «Солнечный свет через окна» — расхождение гайда с кодом, но не эта задача его создала и не в скоупе #577 его чинить; отдельный issue не заводится, находка ниже порога «завести отдельно» — тривиальная, чисто документная стилистика, не затрагивающая ничьё поведение). Термин «оконный тоннель» в AC4 тоже взят из живого текста гайда (docs/USER-GUIDE.ru.md раздел «Толстые стены»: «Солнечный луч начинается из внутренних углов оконного тоннеля») — не изобретён.
  • Технический риск наружного источника явно назван самим автором — «текущее clipping по внутреннему контуру комнаты способно молча обрезать участок от наружной до внутренней грани» в ### Риски, и он же положен в AC4 (golden + smoke) как обязанность доказать проходимость через «физический проём» — примитив уже существует в кодовой базе (opening-tunnel, data-kind: window, docs/WALL-THICKNESS.md §4: «Glow leaving through a door uses the clear rectangular opening tunnel», docs/STYLING-HOOKS.md), то есть AC4 не требует придумывать новую геометрию с нуля — она обоснованно достижима переиспользованием уже задокументированного механизма. Не находка: это техническая реализуемость, зона решения автора/код-ревью (PROCESS.md §7.1: «всё, чего пользователь не наблюдает, агенты решают сами»), а не продукт-вопрос владельцу.
  • «Принятые предположения» корректно ловят единственную реальную продуктовую неоднозначность: удлинение len от новой (внешней) исходной точки означает, что видимая длина луча ВНУТРИ комнаты у толстой стены в outer физически короче, чем в inner, при одинаковом номинальном len (AC5). Это явно зафиксировано как принятое решение, а не молча подразумевается — соответствует формату «принято предположительно, поменять свободно» (PROCESS.md §7.1).
  • Не-скоуп корректно исключает per-space override, изменение азимута/ направления/длины формулы, новые режимы освещения и правки редакторов — граница не размыта.
  • AC1–AC9 пронумерованы, у каждого указан способ доказательства (unit/backend/smoke/golden/«ревью кода»), формулировки проверяемы (конкретные геометрические и UI-утверждения, не «работает корректно»).
  • i18n — уровень детализации (русские тексты трёх строк + перечисление языков) соответствует принятой в архиве практике (сверено с docs/specs/487-room-temperature-thresholds.md §12: там тоже названы назначения ключей без итоговых английских строк) — не находка.
  • Продуктовая рамка — сценарий и «что человек увидит» отвечают на оба обязательных вопроса §7.1 одной фразой без терминов реализации; открытых продуктовых вопросов владельцу нет, и это оправдано: единственные реальные неоднозначности закрыты явным блоком предположений, который ревьюер вправе оспорить (см. предыдущий пункт) — не пустая формальность «решать нечего».

Чего не проверял

  • Не проверял scripts/config-field-registry.mjs построчно — только сверил наличие раздела-прецедента для аналогичных полей (bg_mode, plan_only) в docs/CONFIG-COMPATIBILITY.md.
  • Не проверял custom_components/houseplan/validation.py на предмет конкретного места добавления enum-валидации — это техническая деталь реализации, не предмет ревью ТЗ.
  • Не оценивал фактическую сложность реализации AC4 в src/sun.ts (доступность геометрии «физического проёма» именно в текущей сигнатуре computeSunRays()) — это код-ревью и локальные гейты следующего этапа, риск уже явно назван автором в ТЗ и покрыт golden/smoke в AC4.
  • Не запускал никаких гейтов (typecheck/test/build/golden) — на этапе ревью ТЗ кода нет, гонять нечего.
  • Английские/немецкие/французские тексты i18n не сформулированы (только предмет ключей) — это ожидаемый на этом этапе уровень детализации, не находка (см. выше).

Вердикт

Жёлтый. AC полностью выполнимы и проверяемы, продуктовая рамка и контракт поведения корректны и опираются на реальный канон, а не на догадку; единственная находка — Medium в скоупе задачи (неполный список release-артефактов, два канонических документа не названы для обновления). High-находок нет, отдельный issue не заводится.

Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче


Материал раунда

  • Ветка: issue/577-sun-ray-window-corners, коммит 1df463d7cfe5 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 30be6a8ac43a2c27e5536bfb51f1599b6396628d
    git log --all --format='%H %T' | grep 30be6a8ac43a
    
  • Тело issue: 3263b8c8be2272a314f407b5fa5713fa93150e3c1d59cf2de100dd640f46915a
  • Вердикт конвейера: yellow · High 0