Files
2026-09-26 08:56:02 +00:00

27 KiB
Raw Permalink Blame History

SPEC-REVIEW-662-r2

  • Issue: https://github.com/Matysh/houseplan-card/issues/662
  • Этап: S4-spec-review (ревью ТЗ, PROCESS.md §2.4)
  • Трек: полный (подтверждено в r1: новый UX-контракт, более одной поверхности, влияние на touch и на перф — критерии §5 названы автором явно и разобраны по существу).
  • Материал: тело issue #662 в текущей редакции (после комментария 6, снявшего Medium-1/Low-1 из r1) + все 7 комментариев, включая вердикт r1 (комментарий 5) и правки автора (комментарий 6).
  • Заход: r2 · блокирующих циклов израсходовано 1 из 4 (потрачен r1; зелёный вердикт этого захода бюджет не увеличивает, PROCESS.md §4)
  • Роль: ревьюер ТЗ (не автор)

Скоуп ревью

Без изменений с r1: показ протяжённого источника света (LED-лента) как ломаной вдоль стен вместо значка в точке. Геометрия рисуется в редакторе плана инструментом «LED-лента» (контракт цепочки стен), привязывается к обычному light.* маркеру через существующий диалог «Добавить устройство»; во View/киоске/houseplan-space-card — капсульная полоса on/off/unavailable с хит-тестом по всей длине, линейный источник в fill «Свечение», поднята в 2.5D. Бэкенд: схема led_strips, инвариант «один маркер — не более одной ленты», per-space export/import, support package.

SCOPE-проверка не переоткрывается: r1 уже сверил задачу с J1/J7 и «никогда не строить» — ничего не изменилось (правки r1→r2 чисто технические, скоуп не трогают).

Закрытие раунда r1

Находка (r1) Чем закрыта Где это видно
Medium-1. «Входящие» — несуществующая вкладка каталога; текущая классификация (device-inbox.ts:219-224, else if (runtime || live) category = 'on_plan' — независимо от layout) кладёт осиротевший после отвязки/удаления ленты маркер в on_plan как фантом без отрисовки Термин «Входящие» убран из ТЗ целиком (проверено — grep -c "Входящ" body = 0). Контракт переписан под реальную модель вместо переклассификации: маркер, представленный лентой, — обычный live-маркер (on_plan/visible_explicit); после «Отвязать»/«Удалить ленту» он снова фактически отрисовывается значком — на layout, если он сохранился, либо по авторасстановке (_defaultPositions) в комнате своей области; фантома не остаётся, потому что рисовать действительно есть что Тело issue: «Решения владельца» п.8; ## ТЗ → C2 («Запись layout … не читается и не требуется, пока представлен лентой… в авторасстановке не участвует»), C4 («при привязке размещённого устройства layout не трогается»), C5 («маркер остаётся в config.markers без изменений и снова представлен значком: на сохранённой позиции… либо по авторасстановке… Никакого состояния «не размещено» не возникает»), «Модель данных и миграция», AC8 (переписан), AC19 (новый, закрывает каталог+авто-сетку явно); комментарий 6
Low-1. Раздел «Не скоуп» лежит до ## ТЗ, а не внутри ### Скоуп Строка «Не скоуп: …» продублирована внутри ### Скоуп, развёрнутый список кандидатов оставлен выше как справочный Тело issue, ## ТЗ → ### Скоуп, последний абзац: «Не скоуп: сегменты и эффекты адресных лент; вставка/удаление отдельных вершин; высота крепления…»; комментарий 6

Обе находки закрыты по существу, не только по формулировке — Medium-1 особо: автор не подогнал термин, а привёл контракт к реальному коду device-inbox.ts (проверено чтением, см. ниже).

Унаследовано из r1 (не проверялось повторно)

  • Обязательные разделы §7.1 (Сценарий, Что человек увидит, Проблема, Контракт C1–C12, UX-тексты, Модель данных, План автотестов, Риски, Откат, Release-артефакты, Принятые предположения) присутствуют и в верном порядке — SPEC-REVIEW-662-r1.md, «Как проверялось» п.3, дерево материала 8eaeacefa90fcaebfe8444496827be42e6e06012.
  • Построчная сверка технических терминов C1–C12 с реальным кодом (resolveGlowAppearance, GLOW_FALLOFF, GLOW_FADE_MS=500, visibilityPolygon/splitAtIntersections, clipCache, device-hit-owner.ts cellSize=44, MarkupTool union, MARKER_SCHEMA поля, per-space export, docs/ISOMETRIC.md 0.075 D) — SPEC-REVIEW-662-r1.md «Как проверялось» п.4; текст этих пунктов между r1 и r2 не менялся (правки — только C2/C4/C5/ модель данных/AC8, см. таблицу выше), пере-проверка не требуется.
  • Продуктовые вопросы 1–9 и решения владельца по ним — SPEC-REVIEW-662-r1.md «Как проверялось» п.6; не пересматривались.
  • Внутренняя непротиворечивость числовых порогов (толщины, хит-радиус, MAX_MARKERS/MAX_POLY_POINTS) — SPEC-REVIEW-662-r1.md п.5; новые пороги C13 (эпсилон грани = порог магнита мебели) проверены заново ниже, поскольку относятся к неразобранному материалу.

Что нового в r2 и как проверялось

Ключевой момент этого захода: решение владельца 10 (C13, AC15–AC18) в r1 не разбиралось — автор сам отметил это в комментарии 6 («Ревью r1 разбирало редакцию до решения 10»), и материал r1 подтверждает это документально («Как проверялось» п.4 не упоминает C13; «Находки» тоже). Это не точечная правка внутри уже проверенного текста, а целый непроверенный раздел контракта (стены: упор при рисовании, односторонний свет от грани, поведение внутри утолщённой стены) — по правилу §2.10 «дельта не локальна», разбираю его полностью, а не по диффу к r1.

  1. C13, тело стены при рисовании/перетаскивании (AC15, AC17). Сверено с docs/LIGHT.md «What stops light»: контурные стены с толщиной (wallBodiesGeometry), независимые перегородки/колонны и окна — опорная масонри-геометрия; проёмы door|gate|passage — разрывы в НЕЙ по геометрии, а не по состоянию двери. Это подтверждается реальным кодом: комментарий в src/houseplan-card.ts:10390-10397 («Exterior opening tunnels deliberately remain in masonryGeometry… a valid interior doorway remains transparent») — masonryGeometry/opaqueBodies, на которых строится проверка «источник внутри тела» (#92, glowSourceInOpaqueBody), действительно не зависят от текущего состояния двери; динамика (закрытая дверь = дополнительный occluder) живёт отдельно, в revision/ resolveLightBarrierRevision, и не участвует в определении «тела» для #92 и для упора при рисовании. C13 корректно называет именно эту, статичную, часть — «без учёта состояния двери» здесь не ошибка, а точное описание того, какая геометрия используется для упора пера и для проверки «точка внутри тела». furnitureWallSurfacesFor (src/furniture-wall-surface.ts:161) реально существует и возвращает физические грани — снап ленты к ним, а не к осям, для стен с толщиной технически обоснован.
  2. C13, свет от грани (AC16). Раздел отдельно и правильно вводит стилезависимость нулевых стен (Solid — барьер для лучей, кроме самопересечения точки, лежащей на нём; Dashed — барьера нет вовсе), что дословно совпадает с docs/LIGHT.md «Opaque»/«Transparent» («every zero-thickness wall when its space uses the Solid style» / «…Dashed style»). Это не противоречит первому пункту (толстые стены, двери, окна) — тело для рисования/#92 и барьер для луча описаны раздельно и оба привязаны к реально существующим механизмам (_lightBarriers, buildLightBarrierScene, resolveZeroWalls), а не изобретены. Формулировка вводного абзаца C13 («Телом стены считается то же, что для света») по первому прочтению читается как единое определение на оба подраздела; при внимательном чтении вместе с самим текстом «Свет от грани» она не противоречит реальному коду — но неопытный исполнитель без чтения кода рискует принять «без учёта состояния двери» за правило и для лучевой развёртки тоже. Это стилистический риск понимания, не фактическая ошибка контракта (реальный код и оба абзаца C13, если читать их вместе, согласованы) — не поднимаю до Medium, но фиксирую как наблюдение ниже.
  3. AC16 не содержит сценария с дверью/проёмом для лучевой развёртки самой ленты (только толстая стена и ось/пересечение нулевой стены). Поскольку развёртка ленты, по аналитике и C7, использует ту же цепочку visibilityPolygon/кэш ревизии, что и обычные лампы — то же самое поведение около двери уже покрыто существующими тестами точечных источников и не нуждается в дублирующем AC для ленты; отсутствие такого кейса в AC16 не оставляет её непроверяемой. Даю это как наблюдение (см. ниже), не как находку.
  4. AC15 / «упор в стену». Сверено построчно с описанием: сквозь толстую стену → упор на грани; вдоль грани → допустимо; сквозь нулевую стену любого стиля → допустимо; сквозь door/gate/passage → допустимо; сквозь окно → упор; точка внутри тела → не принята. Все шесть исходов совпадают с телом, определённым в C13 п.1, ни один не выдуман; прецедент упора («упирается в первую небезопасную позицию») — реальный приём, уже используемый изменением размера стен (аналитика ссылается на него по памяти, не проверял построчно код resize — не требуется: сам приём для AC15 достаточно описан текстом ТЗ, реализация — код-ревью).
  5. Мутанты C13 (strip-wall-crossing-allowed, strip-face-offset-off, strip-inside-wall-emits) целятся точно в защитные точки AC15/AC17/AC16 (снятие упора, снятие сдвига от грани, эмиссия из точки внутри тела) — план код-ревью полон, «пустого третьего столбца» по существу нет: сама ТЗ называет мутацию для каждого нетривиального инварианта C13.
  6. Каталог/авто-сетка (Medium-1 fix). src/device-inbox.ts:219-224 (else if (runtime || live) category = 'on_plan') и docs/USER-GUIDE.ru.md:1088 («Автоматически найденный маркер уже относится к «На плане», даже если для него ещё нет сохранённой записи marker») подтверждают дословно новую формулировку C2/C5: живой маркер без layout — не фантом, а обычное «На плане» с авторасстановкой. Дочитано _defaultPositions/_livePos (src/houseplan-card.ts:5188-5246): маркер без saved-записи (или чей saved.s не совпадает с текущим d.space) получает позицию из _defPos — авторасстановка реально размещает его в комнате его области, а не оставляет невидимым. Проверено чтением, не исполнением.
  7. Терминология каталога («На плане», «Доступны», «Скрытые», «Доступны снова») сверена построчно с docs/USER-GUIDE.ru.md:1081 — совпадает дословно; «Входящих» не осталось нигде (grep -c "Входящ" по телу issue = 0).

Наблюдение (не находка, не требует правки)

Вводный абзац C13 («Телом стены считается то же, что для света… без учёта состояния двери») формально относится и к упору при рисовании, и к свету от грани, хотя реально в коде это две разные вещи: статичная масонри-геометрия (без учёта состояния двери — для обоих подразделов корректно) и динамический список барьеров лучевой развёртки (occluders, зависит от состояния двери — именно он гасит свет позади закрытой двери для ЛЮБОЙ лампы, включая ленту, через общий _lightBarriers()). Текст C13 не ошибается по существу (я сверил оба чтения с кодом и они совпадают), но чтение «в лоб» способно навести исполнителя на мысль, что лента как источник света игнорирует состояние двери целиком. Автору стоит при следующей технической правке (не обязательно в этом раунде) явно развести «тело» (масонри, статично) и «барьер развёртки» (occluders, по состоянию) одним предложением — но это редакционная подстраховка, не блокер: код-ревью проверит фактическое поведение AC16 против реальных дверей, и общий механизм _lightBarriers() уже даёт правильный результат без специального кода.

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

  1. Прочитаны docs/SCOPE.md, docs/process/REVIEWER.md, AGENTS.md (уже знакомые из r1, но заново — конспект мог измениться; изменений с r1 не обнаружено).
  2. Прочитано тело issue #662 целиком в текущей редакции и все 7 комментариев, включая вердикт r1 и правки автора.
  3. Восстановлен и прочитан SPEC-REVIEW-662-r1.md из коммита 422221fe (git show 422221fe:docs/reviews/SPEC-REVIEW-662-r1.md) — материал и находки предыдущего раунда.
  4. git diff origin/dev...HEAD --stat — пусто; git status --short — чисто; git rev-parse HEAD = 422221fe9342c88d35ab389515960ba05552ea86 — точное совпадение с материалом ревью. Продуктового кода для #662 по-прежнему нет (стадия spec), гейты (tsc, test, build, смоки, golden, инварианты) неприменимы по той же причине, что в r1.
  5. Закрытие Medium-1 проверено не на слово: прочитан src/device-inbox.ts (классификация on_plan/available, строки 189-260) и src/houseplan-card.ts (_defaultPositions, _livePos, строки 5188-5246) — реальный код подтверждает, что живой маркер без layout получает авто-позицию и реально отрисовывается, а не зависает фантомом.
  6. Закрытие Low-1 проверено чтением текущего тела issue — строка «Не скоуп» присутствует внутри ### Скоуп.
  7. Полностью разобран C13/AC15-18 (см. «Что нового в r2» выше): сверено с docs/LIGHT.md «What stops light», src/houseplan-card.ts (_lightBarriers, glowSourceInOpaqueBody, комментарий про masonryGeometry и occluders), src/furniture-wall-surface.ts (furnitureWallSurfacesFor).
  8. Сверена терминология каталога с docs/USER-GUIDE.ru.md:1081,1088.
  9. grep -c "Входящ" по телу issue — 0 вхождений, подтверждает полное устранение термина.

Находки

Нет ни одной находки (High/Medium/Low). Единственное замечание — стилистическое наблюдение выше, снятое без действия: оно не влияет ни на один AC и не меняет поведение.

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

  • Обе находки r1 (Medium-1, Low-1) закрыты по существу, не косметически: контракт приведён к реальной модели device-inbox.ts/_defaultPositions, а не подогнан под старое поведение под новым именем.
  • Ранее непроверенный материал (C13, AC15–AC18, мутанты strip-wall-crossing-allowed/strip-face-offset-off/strip-inside-wall-emits) самосогласован, ссылается только на реально существующие механизмы (wallBodiesGeometry, furnitureWallSurfacesFor, _lightBarriers, glowSourceInOpaqueBody, docs/LIGHT.md) и не содержит догадок, выданных за факт.
  • AC8/AC19 однозначны и проверяемы: явно называют вкладку каталога и правило авто-сетки, смок и unit-тест из «Плана автотестов» знают, что утверждать.
  • Мутанты по-прежнему по одному на каждый нетривиальный инвариант нового раздела C13 — пустых защитных AC без «чем краснеет» не осталось.
  • Терминология видимого поведения (каталог, вкладки) берётся из docs/USER-GUIDE.ru.md, а не изобретается — выполнено по всему телу issue.

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

  • Гейты (npx tsc --noEmit, npm test, npm run build, node scripts/check-docs.mjs, смоки, golden:verify, инварианты модели) — не прогонял: диф к dev пуст, продуктового кода для #662 нет, стадия spec. Предмет код-ревью после реализации.
  • Не проверял построчно код resize-инструмента стен на предмет точного совпадения приёма «упирается в ближайшую грань» с тем, что потребуется для ленты, — сам приём достаточно описан текстом C13/AC15 для целей спек-ревью; точное соответствие реализации — код-ревью.
  • Не проверял реализуемость объединения вееров видимости из нескольких точек ломаной построчно (по-прежнему код-ревью, не спек-ревью, как и в r1).
  • Не переоценивал продуктовые решения владельца 1–10 — не оспариваю их, сверяю только соответствие контракта (как в r1).
  • Не проверял правку C4 про кросс-пространственную переустановку space у уже размещённого устройства (частный случай, не покрытый отдельным AC): по коду (_livePos) при несовпадении saved.s с новым d.space позиция корректно откатывается на авторасстановку — деградация безопасна, но нет выделенного AC для этого конкретного пути; не поднимаю до находки, так как описанное в контракте поведение («как любой маркер без layout») уже покрывает этот случай по смыслу, а не только для общего случая.

Вердикт

Обе находки r1 закрыты не косметически, а по существу — проверено чтением реального кода классификации каталога и авторасстановки, а не принято на слово. Материал, который r1 физически не успел разобрать (решение владельца 10 — стены, C13, AC15–AC18), разобран полностью в этом раунде: контракт самосогласован, ссылается только на существующие механизмы, каждый защитный AC называет мутацию. Находок нет; одно стилистическое наблюдение снято без действия, так как не влияет на проверяемость ни одного AC.

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


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

  • Ветка: dev, коммит 422221fe9342c88d35ab389515960ba05552ea86.
  • Issue: https://github.com/Matysh/houseplan-card/issues/662, тело в редакции после комментария 6 (26.09), все 7 комментариев учтены.
  • Предыдущий документ: docs/reviews/SPEC-REVIEW-662-r1.md (материал r1: дерево 8eaeacefa90fcaebfe8444496827be42e6e06012, тело — редакция до решения 10, вердикт yellow, High 0, Medium 1).

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

  • Ветка: dev, коммит 422221fe9342 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: c4e3f48a513e0468285959a11f937aa9b13c9754
    git log --all --format='%H %T' | grep c4e3f48a513e
    
  • Тело issue: f20b5d07de0349a2c8bea21863514e70645bea367de0f23797805ec10b577f7a
  • Вердикт конвейера: green · High 0