Files
houseplan-card/docs/reviews/SPEC-REVIEW-471-r1.md
2026-09-06 11:23:51 +00:00

14 KiB
Raw Permalink Blame History

SPEC-REVIEW-471-r1

  • Issue: https://github.com/Matysh/houseplan-card/issues/471
  • Этап: ревью ТЗ (PROCESS.md §2.4)
  • Заход: r1 · блокирующих циклов израсходовано 0 из 4 (лимит лёгкого трека не применяется — задача идёт полным треком)
  • Артефакт ТЗ: docs/specs/471-isometric-overlay-white-plates.md, коммит 5dbcbb81ef24f729f0febd2b836a46196faaf861 на ветке issue/471-isometric-overlay-plates (docs-only, класс C)
  • Вердикт: зелёный

Скоуп ревью

Первый заход ревью ТЗ. Разбор полный (не по дельте — предыдущего раунда нет). Проверялось:

  1. Соответствие docs/SCOPE.md — задача внутри уже утверждённого исключения #89 (2.5D presentation скрытого изометрического режима) и не расширяет его; никакой новой функциональности, только визуальный откат декоративного элемента.
  2. Правильность классификации трека — файл ТЗ существует (задача не small), что подтверждено и собственным комментарием владельца в issue (лёгкий трек: нет — меняется принятый UX-контракт #160).
  3. Полнота обязательных разделов §7.1.
  4. Однозначность и доказуемость каждого AC (AC1…AC8), в т.ч. наличие метода доказательства у каждого.
  5. Не выдана ли догадка за факт — все технические утверждения о текущем поведении сверены с реальным кодом и документами, а не приняты на слово автора.
  6. Отсутствие продуктовых вопросов, которые следовало задать владельцу, но вместо этого решены технически.

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

Читал только — правки в код и ТЗ не вносил (роль ревьюера ТЗ).

  • Прочитаны docs/SCOPE.md, AGENTS.md, PROCESS.md полностью.
  • Прочитано тело issue #471 и оба комментария (оценка владельца, занятие автором).
  • Прочитан файл ТЗ docs/specs/471-isometric-overlay-white-plates.md целиком (395 строк).
  • Технические утверждения ТЗ сверены построчно с исходным кодом:
    • src/iso-scene-render.ts — существование и сигнатуры ISO_RAISED_FOOTPRINT, isoRaisedOverlayHalfSize(); ветки для device / room-label / opening-lock; точные числа roomMetricTextCharacters (temperature: 10, humidity: 8, lqi: 6, light: 24) — совпадают с текстом issue дословно.
    • src/iso-overlays.ts — наличие resolveIsoOverlayPlacement().
    • src/styles/plan.styles.ts — точное значение заливки .iso-overlay-plate { fill: rgba(248, 249, 247, 0.9) } (свет), rgba(68, 77, 81, 0.92) (тёмная тема), forced-colors ветка с fill: Canvas, .iso-overlay-plate-texture.
    • test/isometric-contract.test.mjs — текущие assert на class="iso-overlay-plate ...", на dark/forced-colors CSS-правила — подтверждают, что контракт сегодня действительно требует видимый polygon, как и заявляет ТЗ (AC2 будет требовать обратного).
    • demo/golden/baselines/isometric-stage3-overlays-{light,dark}.png — файлы существуют.
    • docs/ISOMETRIC.md (4 упоминания plate/footprint), docs/adr/160-isometric-stage3-overlays.md (raised plate описан как видимая floor-parallel поверхность), docs/ARCHITECTURE.md (строки 719–726, «raised plate corners», «floor-parallel SVG plate») — все три документа сегодня действительно описывают plate как видимую поверхность, значит их обновление в скоупе задачи обосновано, а не придумано.
    • docs/STATUS.md — подтверждено, что Stage 3 (#160) сейчас «hidden … without public enablement or changelog entry», что оправдывает User-Visible: no в §17 ТЗ по аналогии с исходным коммитом ee7d4869 (feat: implement isometric stage 3, тоже User-Visible: no).
    • docs/TESTING.md — упоминания слова «plate» в этом файле относятся к не связанной подсистеме (working-state plate иконки устройства), не к isometric raised overlay; ТЗ корректно ограничивает правку TESTING.md фразой «только в части действующего Stage 3».
    • src/iso-scene-render.ts:1130 — hp-iso-overlay-texture паттерн привязан к root === 'overlays' и используется отдельно от wall/floor material nuance — подтверждает, что удаление overlay-текстуры (W6) не затрагивает материалы стен/пола, как и заявлено в §8 ТЗ.
  • Файлы, перечисленные в §16 «Ожидаемые файлы реализации», проверены на существование: test/iso-overlays.test.mjs, test/iso-scene-render.test.mjs, test/isometric-contract.test.mjs, demo/smoke_isometric_contract.mjs, demo/smoke_isometric_live_touch.mjs, docs/specs/160-isometric-stage3.md, docs/adr/160-isometric-stage3-overlays.md — все существуют.

Гейты (typecheck/test/build) не гонялись: изменение этого раунда — только docs (класс C), продуктовый код не тронут, гейты кода к ревью ТЗ не относятся.

Находки

Нет High. Нет Medium. Одна Low-находка, снимается ревьюером без возврата автору.

  • Low — отсутствует буквальный раздел «Проблема» (§7.1). ТЗ переходит от §2 «Что человек увидит до/после» сразу к §3 «Подтверждённая причина и заменяемый контракт», не давая отдельного заголовка «Проблема». Содержательно проблема полностью раскрыта — в issue (детальное «Проблема» и «Воспроизведение») и в §1–§3 ТЗ (сценарий, видимый эффект, причина). Снимаю без правки: дублировать содержание issue под отдельным заголовком добавило бы только повторение, не новую информацию.

Что проверено и признано корректным

  • Полнота §7.1. Все обязательные разделы присутствуют по содержанию: сценарий (§1), видимый эффект до/после (§2), проблема/причина (§3), скоуп/не-скоуп (§5, §6), контракт поведения (§4, §7, §8), UX (§10), модель данных и миграция (§11), i18n (§11, «новых строк нет»), AC1…AC8 с методом доказательства у каждого (§14), план автотестов (§15, плюс таблица red witness — избыточно хорошо для этапа ТЗ), риски (§18), откат (§19), release-артефакты (§17).
  • Ни одной догадки, выданной за факт. Каждое утверждение о текущем поведении кода и документов, которое я выборочно перепроверил, подтвердилось дословно (см. «Как проверялось»). Технические предположения на будущее корректно вынесены в явный блок §20 «Принятые технические предположения — можно менять на ревью» и не маскируются под решённые факты.
  • Продуктовых вопросов, скрытых как технические, не найдено. §20.3 (fit/home сохраняет прежний envelope) на первый взгляд похож на продуктовый вопрос о видимом framing, но по факту фиксирует нулевое изменение уже видимого поведения (защита от регресса, а не новое решение) и явно откладывает возможное последующее уточнение framing в отдельную задачу — корректно решать технически, эскалации не требовалось. §20.4 и §20.5 не новые решения, а повтор того, что уже прямо сказано в теле issue.
  • Классификация трека верна. Файл ТЗ создан (не small), что совпадает и с собственной оценкой владельца в issue-комментарии. Критерий, который задача не проходит, назван явно («меняется принятый UX-контракт #160»), как того требует AGENTS.md.
  • Согласованность с docs/SCOPE.md. Работа не расширяет ранее одобренное исключение #89 (2.5D presentation), а чинит визуальный дефект внутри него; из категорий Out-of-scope ничего не затронуто.
  • AC доказуемы и не размыты. Каждый из AC1–AC8 формулирует проверяемое условие («нет .iso-overlay-plate в SVG», «anchors/nudge/tether совпадают на fixtures до/после», «44×44 px сохраняется», и т.д.) и называет способ доказательства (unit/contract/smoke/golden/review), как того требует DoR (PROCESS.md §2.5).
  • Не-скоуп закрывает реальный риск расползания. Явно исключены смежные и правдоподобно соблазнительные правки — камера, footprint размеры, редизайн typography/shell, альтернативная текстовая подложка вместо белой plate (последнее прямо описано как «отдельное продуктовое решение»).
  • Откат и release-артефакты согласованы с прецедентом. User-Visible: no подтверждён тем же выбором в исходном коммите Stage 3 (ee7d4869) и текущим статусом docs/STATUS.md («hidden … without public enablement»).

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

  • Не читал сам продуктовый код в объёме, достаточном для код-ревью (src/iso-overlays.ts целиком, полную реализацию render... функций) — это задача этапа код-ревью, а не ревью ТЗ; здесь код читался только выборочно, чтобы верифицировать конкретные фактические утверждения ТЗ.
  • Не гонял typecheck/test/build/golden — код ещё не менялся, гейты неприменимы на этом этапе.
  • Не проверял docs/specs/160-isometric-stage3.md и docs/adr/160-isometric-stage3-overlays.md построчно целиком — только выборочно, для подтверждения текущей терминологии «plate».
  • Не оценивал визуально сам артефакт (белые прямоугольники) — ручного тестирования на этапе ревью ТЗ нет, а сам артефакт не оспаривается: автор и владелец (в оценке) уже согласны, что дефект существует и воспроизводим.

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

  • Ветка: issue/471-isometric-overlay-plates
  • SHA материала: 5dbcbb81ef24f729f0febd2b836a46196faaf861
  • ТЗ: docs/specs/471-isometric-overlay-white-plates.md (единственный коммит на дельте к dev)

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

  • Ветка: issue/471-isometric-overlay-plates, коммит 5dbcbb81ef24 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 3d667d66b10115b9300bca5ad1b406447aff97b0
    git log --all --format='%H %T' | grep 3d667d66b101
    
  • ТЗ docs/specs/471-isometric-overlay-white-plates.md, блоб 1ef8a58c9af0140089c3e20a2ded7c72cfac7402
    git log --all --find-object=1ef8a58c9af0140089c3e20a2ded7c72cfac7402 -- docs/specs/471-isometric-overlay-white-plates.md