mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,160 @@
|
||||
# Ревью ТЗ — issue #200, цикл r1
|
||||
|
||||
- Этап: `S4-spec-review` (PROCESS.md §2.4)
|
||||
- Артефакт ТЗ: [`docs/specs/200-room-label-parity.md`](../specs/200-room-label-parity.md),
|
||||
коммит `4089c91` на ветке `issue/200-room-label-parity`
|
||||
- Issue: [#200](https://github.com/Matysh/houseplan-card/issues/200)
|
||||
- Ревьюер: Claude (роль «ревьюер ТЗ», отдельная сессия от аналитика/автора)
|
||||
- Трек: обычный (не `small`) — верно, т.к. меняется явный UX-контракт кнопки
|
||||
между View и Plan editor (§5 PROCESS.md исключает small при новом UX-контракте)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Оценивалось ТЗ `docs/specs/200-room-label-parity.md` целиком: наличие
|
||||
обязательных разделов §7.1 PROCESS.md, однозначность и доказуемость AC1–AC9,
|
||||
отсутствие догадок, выданных за решённый факт, соответствие
|
||||
`docs/SCOPE.md` (job J4/J6), `docs/UX-MODES.md`, `docs/TESTING.md`,
|
||||
`docs/USER-GUIDE.ru.md`, `docs/STYLING-HOOKS.md` и текущему коду
|
||||
(`src/houseplan-card.ts`, `src/styles.ts`, `demo/smoke_room_link.mjs`).
|
||||
Продуктовый код не менялся и не мог быть изменён (задача на этапе ТЗ).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком.
|
||||
2. Прочитано тело issue #200 и все 4 комментария: аналитика, вопросы владельцу
|
||||
Q1–Q3, принятые дефолты, хендофф автора ТЗ.
|
||||
3. Прочитан весь текст ТЗ построчно, сверен с §7.1 (обязательные разделы) и
|
||||
§2.5 (DoR-чеклист).
|
||||
4. Проверена заявленная причина дефекта чтением продуктового кода:
|
||||
`src/houseplan-card.ts:17144-17150` (`_renderRoomLabel`, условие
|
||||
`!this._markup && r.area`), `src/houseplan-card.ts:1203-1205` (геттер
|
||||
`_markup`, true только для `mode === 'plan'`), `src/houseplan-card.ts:15807-15808`
|
||||
(условие рендера самого room-label). Проверен существующий смок
|
||||
`demo/smoke_room_link.mjs` (текущий инвариант `out.noneInPlan`) и запись
|
||||
контракта в `docs/TESTING.md:932-935`.
|
||||
5. Проверен CSS `.rlgo`/`.roomlabel`/`.rlname`/`.rlmetrics` в `src/styles.ts:872-989`,
|
||||
включая комментарий в коде («the name renders in exactly the same place in
|
||||
view mode and in the plan editor (owner's request)») — он подтверждает, что
|
||||
геометрический инвариант уже задумывался раньше и сейчас нарушен именно тем
|
||||
местом, которое называет ТЗ.
|
||||
6. Проверено соответствие терминологии: `src/i18n/ru.json:526` (`room.open_area`
|
||||
→ «Открыть зону в HA»), `docs/USER-GUIDE.ru.md` (упоминаний `.rlgo`/значка
|
||||
перехода нет — новый пользовательский сценарий действительно не появляется).
|
||||
7. Проверено, что файл ТЗ и запись в `docs/specs/README.md` добавлены одним
|
||||
коммитом `4089c91` с трейлерами `Issue: #200` / `User-Visible: no` —
|
||||
корректно для чистой документации без изменения поведения.
|
||||
8. Сверено сравнение "лёгкий трек не подходит" с критериями §5 PROCESS.md.
|
||||
|
||||
Код не менялся, чтения было достаточно: вопрос ревью ТЗ — «выполнимо и
|
||||
проверяемо ли», а не «работает ли реализация».
|
||||
|
||||
## Проверено и корректно
|
||||
|
||||
- **Все обязательные разделы §7.1 присутствуют**: сценарий и персона (§1),
|
||||
что человек увидит до/после без терминов реализации (§2), проблема и
|
||||
подтверждённая причина (§3), scope/non-scope (§4–5), контракт поведения (§6),
|
||||
данные/миграция (§7), UX/i18n/touch (§8), AC1–AC9 с доказательством (§9),
|
||||
план автотестов (§10), риски (§11), rollback (§12), release-артефакты (§13).
|
||||
- **Причина дефекта подтверждена чтением кода, а не заявлена на веру.** Условие
|
||||
`!this._markup && r.area` в `_renderRoomLabel` (houseplan-card.ts:17144)
|
||||
действительно единственное место, отличающее `.rlname` между View и Plan;
|
||||
`.rlmetrics` вынесен из centering math и одинаков — ровно то, что говорит ТЗ.
|
||||
- **Продуктовые вопросы владельцу заданы по существу, не подменены техническими.**
|
||||
Q1 («что делает иконка в Plan») и Q3 («что сохраняется поверх карточки») — то,
|
||||
что видит и делает человек; Q2 («в какой системе координат мерить
|
||||
„не сдвигается ни на пиксель“») пограничен, но корректно сведён к
|
||||
наблюдаемому факту (anchor vs viewport), а не к технической реализации —
|
||||
ответ владельца определяет ожидаемое пользовательское поведение теста, а не
|
||||
структуру кода. Владелец принял все три дефолта, вопросы закрыты до начала
|
||||
написания ТЗ.
|
||||
- **Все AC пронумерованы, однозначны и несут явный способ доказательства**
|
||||
(`smoke`/`golden`/`gates`/mutation gate) — выполняется требование DoR
|
||||
(§2.5 PROCESS.md). AC4/AC5 задают числовой допуск (≤0,5 CSS px, DPR 1 и 2,
|
||||
light/dark) — не оценочная формулировка, а измеримый критерий.
|
||||
- **AC9 (mutation gate) — сильная часть ТЗ.** Оба мутанта воспроизводят ровно
|
||||
те регрессии, которых боится сценарий (иконка исчезла в Plan; иконка
|
||||
появилась, но стала кликабельной и ломает drag) и соответствуют
|
||||
установленному в проекте формату `scripts/mutation-gate.mjs` (issue #85).
|
||||
- **Не-скоуп сформулирован точно** и предотвращает соблазн заодно поправить
|
||||
соседнее: абсолютные координаты страницы, миграцию слоя `layout`, unnamed
|
||||
placeholder, resize handles, кнопку настроек, touch policy, isometric,
|
||||
performance pipeline — всё явно исключено с объяснением почему.
|
||||
- **Технические решения корректно помечены как предположения, а не факты**
|
||||
(§14 «принято предположительно, поменять свободно при ревью»): отсутствие
|
||||
title/role/tab stop у иконки в Plan, переиспользование существующих
|
||||
`.roomlabel`/styling hooks без нового wrapper, приоритет числового smoke над
|
||||
golden. Ревьюер эти предположения не оспаривает — они разумны и не влияют на
|
||||
AC.
|
||||
- **`docs/TESTING.md` и user-facing changelog корректно учтены** как
|
||||
release-артефакты (AC7, §13); существующая запись в `docs/TESTING.md:932-935`
|
||||
(«no icon in editors... [auto: smoke_room_link]») действительно требует
|
||||
правки, и ТЗ это называет прямо.
|
||||
- **`docs/USER-GUIDE.ru.md` не требует нового сценария** — проверено: гайд
|
||||
вообще не описывает иконку перехода к зоне отдельно (только общую фразу про
|
||||
карточки/названия комнат, docs/USER-GUIDE.ru.md:271), значит добавить
|
||||
противоречащую формулировку неоткуда; проверка "при реализации перепроверить
|
||||
формулировки" в ТЗ достаточна.
|
||||
- **Откат и миграция:** корректно — persisted-формат и API не меняются,
|
||||
downgrade возвращает старый визуальный сдвиг без порчи данных.
|
||||
- **Оценка "лёгкий трек не подходит" верна**: критерий §5 «нет нового
|
||||
UX-контракта» нарушается (кнопка получает новую видимую форму в Plan), значит
|
||||
обычный трек — правильный выбор, а не подстраховка.
|
||||
|
||||
## Находки
|
||||
|
||||
### Low — неточная формулировка причины дефекта в §3 ТЗ
|
||||
|
||||
**Файл:** `docs/specs/200-room-label-parity.md`, раздел 3 («Проблема и
|
||||
подтверждённая причина»).
|
||||
|
||||
**Суть:** ТЗ утверждает: «условие `!this._markup && r.area` рендерит `.rlgo`
|
||||
только в View». Это неточно: `_markup` (`houseplan-card.ts:1203`) истинен
|
||||
только при `mode === 'plan'`, то есть `.rlgo` при выполнении условия рендерится
|
||||
не только в View, но и в редакторах Devices/Background — там, где включена
|
||||
настройка «Показывать названия» (`houseplan-card.ts:15807`,
|
||||
`disp.showNames || ... || this._markup`). Иконка сегодня отсутствует только в
|
||||
Plan editor, а не «только присутствует в View».
|
||||
|
||||
**Проверено, что это не влияет на корректность AC.** Единственное изменение,
|
||||
которое требуют AC1–AC9, — снять условие `!this._markup` для Plan; поведение
|
||||
Devices/Background editors не меняется и не тестируется в этом ТЗ (что и
|
||||
верно — они вне заявленного scope «Plan editor ↔ View»). Наивная реализация
|
||||
(рендерить `.rlgo` при `r.area` независимо от режима, событийный контракт
|
||||
регулировать отдельно только для Plan) не пострадает от этой неточности.
|
||||
|
||||
**Почему не блокирует:** это неточность в описании диагноза, а не в контракте
|
||||
поведения (раздел 6) или в AC (раздел 9), которые сформулированы верно и не
|
||||
опираются на слово «только в View». Реализация и код-ревью будут сверяться с
|
||||
разделами 6 и 9, а не с формулировкой раздела 3.
|
||||
|
||||
**Решение ревьюера:** снимается без правки, с записью в этом документе
|
||||
(разрешено §2.4/§3 PROCESS.md: «Low либо правится, либо снимается решением
|
||||
ревьюера с записью»). Если автор всё же правит ТЗ по другой находке в будущем
|
||||
цикле, желательно заодно уточнить формулировку («рендерится везде, кроме Plan
|
||||
editor»), но отдельного цикла ревью это не требует.
|
||||
|
||||
Других находок — Low, Medium или High — не выявлено.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не проверялась реализация — на этапе ТЗ её не существует; код-ревью будет
|
||||
отдельным циклом (`S7-code-review`) после написания кода.
|
||||
- Не запускались `npm test`/`npm run build`/смоки: код не менялся, гейты ТЗ не
|
||||
требуют их прогона на этом этапе (документация — класс C, продуктовый код не
|
||||
тронут).
|
||||
- Не проверялась визуальная точность будущих golden-скриншотов — они появятся
|
||||
только в реализации; ТЗ описывает их корректно (focused, light/dark), этого
|
||||
достаточно для стадии ревью ТЗ.
|
||||
- Не оценивалась производительность на реальном большом плане — ТЗ обоснованно
|
||||
сводит риск к «пренебрежимо мал» (один существующий DOM-узел в ещё одном
|
||||
режиме), и это утверждение проверяется кодом-ревью, а не спецификацией.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Все обязательные разделы на месте, все AC однозначны и снабжены способом
|
||||
доказательства, причина дефекта подтверждена чтением кода, продуктовые вопросы
|
||||
владельцу закрыты до написания ТЗ, non-scope точен, mutation-gate усиливает
|
||||
доказуемость AC1/AC3. Единственная находка — Low, не влияющая на корректность
|
||||
контракта, снята с записью.
|
||||
|
||||
**Вердикт: зелёный · цикл r1/4 · High: 0 · Medium: 0 → в задаче**
|
||||
Reference in New Issue
Block a user