From bf83246b7b84dc6e068f314e526c4c8ed3b73eb7 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 19 Aug 2026 13:57:42 +0000 Subject: [PATCH] docs: review document for #200 Issue: #200 User-Visible: no --- docs/reviews/SPEC-REVIEW-200-r1.md | 160 +++++++++++++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-200-r1.md diff --git a/docs/reviews/SPEC-REVIEW-200-r1.md b/docs/reviews/SPEC-REVIEW-200-r1.md new file mode 100644 index 00000000..db161966 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-200-r1.md @@ -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 → в задаче**