Files
2026-10-01 20:52:49 +00:00

21 KiB
Raw Permalink Blame History

SPEC-REVIEW-662-r4

  • Issue: https://github.com/Matysh/houseplan-card/issues/662
  • Этап: S4-spec-review (ревью ТЗ, PROCESS.md §2.4)
  • Трек: ask, подтверждено в r1 и не пересматривается на этом заходе.
  • Материал:
    • тело issue #662 в текущей редакции — последняя правка комментарием Matysh 2026-10-01T20:45:12Z («Правки по SPEC-REVIEW-662-r3 (жёлтый)»); более новых правок тела на момент ревью нет;
    • docs/reviews/SPEC-REVIEW-662-r1.md, -r2.md, -r3.md — вердикты и материал прошлых заходов (все три уже закоммичены в dev, последний — коммитом 5021c484, который и есть вершина ветки на этом заходе);
    • рабочая копия на 5021c484461879599ab4aad4aefdb3889282cc26 — чисто документный коммит («docs: review document for #662», публикация SPEC-REVIEW-662-r3.md + обновление docs/reviews/INDEX.md), продуктовый код не менялся со времени ветки dev@8a3bf71e, на которой сверялся r3.
  • Заход: r4 · блокирующих циклов израсходовано 2 из 4 (r1 — жёлтый, r2 — зелёный, бюджет не увеличивает, r3 — жёлтый)
  • Роль: ревьюер ТЗ (не автор)

Скоуп ревью

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

Этот заход — целиком исполнение правок, обещанных автором в ответ на r3 (жёлтый, Medium: 2): правка узкая, текстовая, не меняет геометрию, UX-контракт или модель данных. SCOPE-проверка не переоткрывается: r1 уже сверил задачу с J1/J7 и «никогда не строить»; ни одна правка r4 этого не касается.

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

Находка r3 Чем закрыта Где это видно
Medium-1 — TZ-issue-662-LED-strips.md §9 архива дизайнера («половина общего радиуса») дословно противоречит решению 12 (50 см), расхождение нигде не названо В «Визуальный эталон» добавлен абзац «Что из ТЗ дизайнера заменено»: прямо называет §9 и строку On таблицы §8 как редакцию до 01.10, заменённую решениями 12 и 13, с обоснованием («кадры показывают поле ≈ 50 см, половина общих 3 м — 150 см»); AC14 требует, чтобы docs/design/662-led-strips/README.md перечислял обе замены Тело issue, раздел «Визуальный эталон», абзац «Что из ТЗ дизайнера заменено» (после нормы «Толщина»); AC14 — фраза «…и пункты ТЗ дизайнера, заменённые решениями 12 и 13»
Medium-2 — AC14 и «Скоуп» требуют правки DEVICE-LIGHT-SETTINGS-MATRIX и docs/TESTING-DEMO.md, оба физически удалены #679 Пункт «DEVICE-LIGHT-SETTINGS-MATRIX» убран из AC14 (поглощён соседним «LIGHT… матрица настроек света»); docs/TESTING-DEMO.md заменён на demo/stand/README.md в AC14 и в «### Скоуп» Тело issue, AC14: «LIGHT («Linear sources», «Strips and walls», матрица настроек света)… demo/stand/README.md (сущность стенда)»; «### Скоуп»: «…и в seed стенда (demo/stand, demo/stand/README.md)»

Автор заодно (не как находка, а проактивно) поправил два унаследованных из r2 расхождения, замеченных при собственной сверке с dev@8a3bf71e: buildDeviceInboxRows → buildDeviceInbox (AC19) и test/i18n-parity → test/i18n.test.mjs (план автотестов). Обе правки проверены ниже.

Поиском по телу issue подтверждено отсутствие старых формулировок: grep -n "DEVICE-LIGHT-SETTINGS-MATRIX\|TESTING-DEMO\|Входящ\|buildDeviceInboxRows\|i18n-parity" — ноль совпадений.

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

Контракт C1–C13, AC1–AC13, AC15–AC18, AC20 (кроме точечных правок, разобранных в «Закрытие раунда r3» выше), UX-тексты, модель данных/миграция, i18n, риски, откат, release-артефакты, решения 1–15 и их внутренняя согласованность — приняты без повторной проверки на основании docs/reviews/SPEC-REVIEW-662-r3.md (жёлтый, High 0 · Medium 2, обе находки — только по двум узким местам, закрытым выше) и -r2.md (зелёный, High 0 · Medium 0, материал — тело issue после комментария 6, закрывшего находки r1). Правка 01.10 (решения 11–15, «Визуальный эталон», AC20) была разобрана r3 целиком, не по диффу (правило «дельта не локальна», §2.10); r4 трогает внутри неё только пограничную формулировку и список путей в AC14/«Скоуп».

Разбор по дельте r4

Сверено построчно: диф тела issue — добавленный абзац «Что из ТЗ дизайнера заменено», правка AC14 (два пункта: удалён «DEVICE-LIGHT-SETTINGS-MATRIX», docs/TESTING-DEMO.md → demo/stand/README.md), правка «### Скоуп» (тот же путь), правка AC19 (buildDeviceInbox), правка «План автотестов» (test/i18n.test.mjs).

  • Medium-1 закрыта по существу, а не косметически. Решение 12 (50 см) и решение 13 (белое ядро в «Свечении») теперь явно названы заменой §9/строки On §8 архива дизайнера, с численным обоснованием («половина общих 3 м — 150 см» против макета «≈ 50 см») — тот самый аргумент, которым решение 12 обосновывалось изначально, теперь физически привязан к конкретному месту архива, которое он опровергает. AC14 требует того же в README архива — двойная страховка (тело issue + будущий committed-документ), оба канала корректны и непротиворечивы друг другу.
  • Medium-2 закрыта полностью. DEVICE-LIGHT-SETTINGS-MATRIX как отдельный пункт правки исчез (сам контент матрицы по-прежнему покрыт соседним пунктом «LIGHT… матрица настроек света», что и предлагал вывод r3); оба упоминания docs/TESTING-DEMO.md (AC14 и «### Скоуп») заменены на существующий demo/stand/README.md. Проверено: demo/stand/README.md существует (ls demo/stand/README.md), docs/TESTING-DEMO.md и DEVICE-LIGHT-SETTINGS-MATRIX* в репозитории по-прежнему отсутствуют (находка r3 не устарела сама собой — путь действительно был нужен).
  • AC19: buildDeviceInbox. Реальное имя в src/device-inbox.ts:189 — export function buildDeviceInbox(...); buildDeviceInboxRows в коде нет ни разу (grep -rn buildDeviceInboxRows src — пусто). Правка верна.
  • План автотестов: test/i18n.test.mjs. Файл существует; тест test('i18n: every registered dictionary carries the English key set', …) на строке 121 — формулировка в точности совпадает с комментарием автора («every registered dictionary carries the English key set»). Правка верна и не расходится с AC13 («ключи i18n во всех четырёх словарях»).

Делта узкая и полностью локальна: ни одна строка контракта C1–C13, ни одна норма V1–V6, ни один AC не поменяли смысл — только устранены два указания на несуществующие/противоречивые артефакты. Полный повторный разбор всего ТЗ не требуется; раздел «Унаследовано из r3» выше формализует это согласно §2.10.

Проверено и корректно

  • DoR §7.1: все обязательные разделы на месте — сценарий, «что человек увидит до и после», проблема, скоуп и не-скоуп, контракт поведения (C1–C13), UX- тексты (i18n ×2 + пометка «de/fr — переводы»), модель данных и миграция, критерии приёмки AC1–AC20 (у каждого указан способ доказательства: unit/ smoke/golden/backend/code-review), план автотестов, риски, откат, release-артефакты. Открытых продуктовых вопросов нет — все девять вопросов 26.09 и решение по макетам 01.10 владелец закрыл явными решениями 1–15.
  • Путь docs/design/662-led-strips/ следует соглашению NNN-slug (600-settings-dialogs, 649-25d-stage6 — уже существуют в docs/design/, сам путь 662-led-strips ещё не создан — ожидаемо: материалы ложатся туда в коде, решение 15, это задача AC14/AC20, а не ревью ТЗ).
  • Идентификаторы, которые делта не трогала, но которые стоило перепроверить заодно как часть «унаследовано» (выборочно, не полный повтор r2/r3): resolveGlowAppearance, GLOW_FALLOFF, glowAlpha, GLOW_FADE_MS — существуют в src/glow-scene.ts/src/logic.ts (grep подтверждает); demo/stand/README.md, docs/LIGHT.md, docs/DEVICE-PRESENTATION.md, docs/ARCHITECTURE.md, docs/ISOMETRIC.md, docs/CONFIG-COMPATIBILITY.md, docs/TOUCH-SUPPORT.md — все существуют на HEAD.
  • Трейлеры: не применимо — этап spec, коммитов класса A/B в этом заходе нет; коммит 5021c484 — чистый class C (документ ревью), трейлеры Issue: #662 / User-Visible: no на месте и корректны для docs-коммита.
  • Одно число — один источник (§8): правка r4 текстовая, новых чисел, видимых пользователю дважды, не вводит; числа решений 12–14 (50 см, #383838, #FFFFFF) не менялись с r3, где уже сверены с кадрами.

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

  • Гейты кода не прогонялись и не могли быть прогнаны по существу: на этапе spec нет продуктового кода для этой задачи (правка r4 — комментарий к issue, не коммит в ветку; рабочая копия на 5021c484 — чистый docs-коммит публикации r3). Зависимости не установлены (node_modules отсутствует), что ожидаемо: на этапе spec зависимости и Chromium не ставятся (#696, AGENTS.md). npx tsc --noEmit, npm test, npm run build + bundle-policy --verify не запускал — предмета для них на этом SHA нет (докстрока dev уже прошла свои гейты коммитом 5021c484, который трогает только docs/reviews/**). То же относится к golden/смокам/мутантам/pytest — кода ленты ещё не существует ни в одном файле.
  • Figma-макет напрямую не открывался в этом заходе — делта не касается визуального эталона по существу (только формулировка в тексте), r3 уже сверил архив и кадры и унаследовано без повторной проверки.
  • Архив дизайнера (Issue-662-LED-light-spec-2026-10-01.zip, SHA-256/MANIFEST) не перескачивался и не пересверялся — делта r4 его не трогает; принято из r3.
  • Не проверял свежесть docs/reviews/INDEX.md как часть задачи #662 — она не в скоупе ТЗ; расхождение в ней, найденное по дороге, описано в «Находки» ниже и заведено отдельно.

Находки

Medium (вне скоупа #662, не блокирует) — scripts/reviews-index.mjs неверно индексирует SPEC-REVIEW-662-r3.md.

docs/reviews/INDEX.md показывает для SPEC-REVIEW-662-r3.md строку 🟢 зелёный | 0 | 0. Сам документ r3 заканчивается **Вердикт: жёлтый.** и High: 0 · Medium: 2 (обе в скоупе) · Low: 0. — расхождение подтверждено и прямым запуском парсера из scripts/reviews-index.mjs:

$ node --input-type=module -e "
import { parseVerdict, parseCounts } from './scripts/reviews-index.mjs';
import fs from 'fs';
const text = fs.readFileSync('docs/reviews/SPEC-REVIEW-662-r3.md', 'utf8');
console.log('verdict:', parseVerdict(text));
console.log('counts:', parseCounts(text));
"
verdict: зелёный
counts: { high: 0, medium: 0 }

Причина — раздел «Закрытие раунда r2» документа r3 пересказывает вердикт предыдущего раунда строкой Вердикт r2 — зелёный, находок не было (High: 0, Medium: 0)., которая сама формально начинается со слова «Вердикт» и укладывается в окно VERDICT_OWN_LINE_RE (≤ 60 символов до цветового слова). Комментарий в коде парсера описывает именно этот класс бага как уже чинившийся раньше (#635, «документ r2 пересказывал вердикт r1»), но защита «строка начинается с „Вердикт“» не отличает «Вердикт: <цвет>» (собственный вердикт текущего документа, формат §7.2) от «Вердикт r — <цвет>» (пересказ чужого раунда в разделе «Закрытие раунда», обязательном по §2.10 для каждого повторного захода). parseCounts по той же причине берёт High: 0, Medium: 0 из пересказанной строки раньше, чем доходит до настоящей сводки в конце документа.

Сейчас под этот паттерн (^Вердикт r\d+ — ) в docs/reviews/*.md попадает только один документ (grep -rlE "^Вердикт r[0-9]+ — " docs/reviews/*.md → только SPEC-REVIEW-662-r3.md), но сам приём — обязательный раздел «Закрытие раунда r» — стандартный для любого ревью от второго захода и будет повторяться. Риск: будущий ревьюер, который по инструкции «Повторный раунд» смотрит строки задачи в INDEX.md перед разбором подсистемы, увидит «r3 — зелёный, 0/0» и может не открыть сам документ, хотя в нём два разобранных Medium.

Это дефект инструмента индексации (scripts/reviews-index.mjs), не ТЗ #662 и не продуктового кода: чинить его в этой задаче нельзя (класс задачи #662 — A+B+C продукта, а не scripts/**), поэтому заведён отдельным issue со ссылкой на #662: #779 (bug, P3, S1-new). Не блокирует #662 — находка не про ТЗ этой задачи.

Вывод

Обе Medium-находки r3 закрыты по существу и подтверждены чтением реального текста/кода, а не на слово автора: формула радиуса из архива дизайнера теперь явно помечена как замещённая решением 12/13 в самом теле issue (и README архива это ещё раз закрепит в коде), а AC14/«Скоуп» больше не указывают на документы, которых не существует с #679. Два проактивных исправления (buildDeviceInbox, test/i18n.test.mjs) также подтверждены по коду. Делта узкая, локальная, не меняет контракт, геометрию, модель данных или AC по существу — полный повторный разбор ТЗ не требовался и не проводился за пределами того, что унаследовано как зелёное/проверенное в r2/r3.

Единственная находка этого захода — Medium вне скоупа задачи (баг индексатора ревью-документов), заведена отдельным issue #779 и не блокирует.

High: 0 · Medium: 0 (в скоупе) / 1 (вне скоупа, → #779).

Вердикт: зелёный.


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

  • Ветка: dev, коммит 5021c4844618 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 9bebfc0429aaa6e7cefc52dc4c32d775bbd69ab1
    git log --all --format='%H %T' | grep 9bebfc0429aa
    
  • Тело issue: 0faa724b50bbca9c9b1fc7d1de20627cccb81fc766a4c88776f524004bad8aa9
  • Вердикт конвейера: green · High 0 · маршрут fix