mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,230 @@
|
||||
# Ревью ТЗ — issue #203, цикл r2
|
||||
|
||||
- Этап: `S4-spec-review` (PROCESS.md §2.4)
|
||||
- Артефакт ТЗ: [`docs/specs/203-hide-room-names.md`](../specs/203-hide-room-names.md),
|
||||
правка коммитом `f7b811a` поверх `009fed9` на ветке `issue/203-hide-room-names`
|
||||
- Issue: [#203](https://github.com/Matysh/houseplan-card/issues/203)
|
||||
- Ревьюер: Claude (роль «ревьюер ТЗ»)
|
||||
- Трек: обычный (не `small`), лимит цикла — 4 (§4 PROCESS.md); это второй цикл
|
||||
- Предыдущий цикл: [`SPEC-REVIEW-203-r1.md`](SPEC-REVIEW-203-r1.md) — жёлтый,
|
||||
High: 0, Medium: 1 (в скоупе)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Полный повторный разбор `docs/specs/203-hide-room-names.md` — не только
|
||||
дельта коммита `f7b811a`, а весь документ целиком, как того требует §2.4
|
||||
(«ревьюер получает issue и ТЗ, без устных пояснений автора»): обязательные
|
||||
разделы §7.1 PROCESS.md, однозначность и доказуемость AC1–AC10, отсутствие
|
||||
догадок, выданных за факт, соответствие `docs/SCOPE.md` (J4/J6),
|
||||
`docs/UX-MODES.md`, `docs/STYLING-HOOKS.md`, `docs/USER-GUIDE.ru.md`,
|
||||
`docs/TESTING.md`, `AGENTS.md` (Labs flags) и текущему коду
|
||||
(`src/houseplan-card.ts`, `src/space-render.ts`, `src/logic.ts`,
|
||||
`src/labs.ts`, `scripts/mutation-gate.mjs`). Дополнительный фокус цикла —
|
||||
устранение Medium-находки r1 (третье независимое место дефекта, forced HTML
|
||||
в hidden iso, без сопровождающего мутанта). Продуктовый код не менялся между
|
||||
r1 и r2 (`git diff 009fed9..HEAD -- src/` пуст) — верно для стадии ТЗ.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Перечитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком заново (не
|
||||
по памяти предыдущего цикла).
|
||||
2. Перечитано тело issue #203 и все четыре комментария, включая хендофф
|
||||
автора о правке r1 (`f7b811a`, «AC3 теперь требует отдельное мутационное
|
||||
доказательство hidden iso, а §10.3 фиксирует три независимых
|
||||
entries/guards»).
|
||||
3. Прочитан весь текст ТЗ построчно заново, сверен с §7.1 (обязательные
|
||||
разделы) и §2.5 (DoR-чеклист) — все присутствуют, без регресса после
|
||||
правки.
|
||||
4. Прочитан точечный diff `git show f7b811a`: правка ограничена AC3 (строка
|
||||
таблицы AC) и §10.3 (таблица мутантов); остальной текст не тронут.
|
||||
5. Проверено содержательное соответствие правки находке r1 построчным чтением
|
||||
актуального кода:
|
||||
- `src/houseplan-card.ts:10910-10919` (`_renderSvgRoomLabels`) — условие
|
||||
`this._renderProjection === 'iso' || space.bg || disp.showNames ||
|
||||
this._markup` дословно совпадает с описанием мутанта
|
||||
`hidden-room-names-full-svg-fallback`;
|
||||
- `src/space-render.ts:345-352` (`staticSvgLabels`) — условие
|
||||
`!space.bg && !disp.showNames` дословно совпадает с
|
||||
`hidden-room-names-compact-svg-fallback`;
|
||||
- `src/houseplan-card.ts:15825` — `disp.showNames || (iso && !space.bg) ||
|
||||
this._markup` дословно совпадает с новым третьим мутантом
|
||||
`hidden-room-names-iso-override`; это ровно та строка, которую r1 указал
|
||||
как непокрытую.
|
||||
6. Проверена **техническая реализуемость** нового мутанта, а не только текст
|
||||
ТЗ: каждый из трёх якорей (`grep -c`) встречается в соответствующем файле
|
||||
ровно один раз — требование `scripts/mutation-gate.mjs` («`find` обязан
|
||||
встречаться в файле ровно один раз») выполнимо для всех трёх записей, а не
|
||||
только продекларировано.
|
||||
7. Прочитан `scripts/mutation-gate.mjs` (шапка и существующие записи реестра,
|
||||
например `empty-space-cleanup-disabled` с `guard: 'node
|
||||
demo/smoke_optional_space_model.mjs'`) — подтверждено, что предложенная
|
||||
форма мутанта (id · guard = существующий/расширяемый browser-smoke ·
|
||||
патч, ломающий один анкер) соответствует принятому в проекте шаблону, а не
|
||||
изобретает новый.
|
||||
8. Перепроверены сами SVG/HTML fallback-ветки на отсутствие иных, ещё не
|
||||
учтённых мест того же дефекта: `grep -rn "rlabel" src/*.ts` даёт ровно два
|
||||
определения (`houseplan-card.ts`, `space-render.ts`) плюс CSS в
|
||||
`styles.ts`; других мест создания `.rlabel`/принудительного показа
|
||||
`.roomlabel` в источниках нет — после правки r1 все известные независимые
|
||||
места действительно покрыты, четвёртого не обнаружено.
|
||||
9. Прочитан `docs/TESTING.md` §«Правила для новых тестов» (правила 1, 3, 4) —
|
||||
подтверждено, что новая формулировка AC3 («+ отдельный iso mutant»)
|
||||
закрывает именно тот риск, который правило 4 называет («тест, охраняющий
|
||||
механизм, сопровождается мутантом»), и что DOM-присутствие/отсутствие
|
||||
само по себе (правило 1) для всех трёх AC теперь подкреплено мутантом, а
|
||||
не голым ассертом.
|
||||
10. Проверено текущее содержимое существующих iso-смоков
|
||||
(`demo/smoke_isometric_contract.mjs:28`, `demo/smoke_isometric_live_touch.mjs:25`)
|
||||
— оба сейчас жёстко ставят `show_names: true` и не покрывают `false`;
|
||||
подтверждено, что план §10.2 п.5 («повторить ключевую проверку в hidden
|
||||
iso») действительно требует нового покрытия, а не переиспользования уже
|
||||
существующего ассерта, то есть AC3 не завышает готовность.
|
||||
11. Заново прочитан `docs/UX-MODES.md`, раздел «What a space may choose not to
|
||||
draw» — таблица трёх «display-only» переключателей (`show_borders`,
|
||||
`hide_decor`, `hide_openings`) не упоминает `show_names`; см. находку Low
|
||||
ниже.
|
||||
12. Заново прочитан `docs/STYLING-HOOKS.md` §1 и §3 — подтверждена
|
||||
самообязывающая формулировка «we promise the names in §3 are stable. If
|
||||
we ever have to change one, it is a breaking change and it goes in the
|
||||
changelog»; `grep -rn "rlabel" src/*.ts` подтверждает, что строка
|
||||
`.rlabel` в §3 не имеет ни одного легитимного (не багового) источника —
|
||||
удаление строки корректно, см. находку Low ниже про формулировку
|
||||
changelog-записи.
|
||||
13. Перечитан `docs/USER-GUIDE.ru.md`, строка «Показывать названия» — текст
|
||||
не требует правки: он не обещает второго «статического» режима и
|
||||
остаётся честным после исправления.
|
||||
14. Проверено существование всех файлов, на которые ссылается план тестов:
|
||||
`demo/smoke_space_settings.mjs`, `demo/smoke_styling_hooks.mjs`,
|
||||
`demo/smoke_room_cards.mjs`, `demo/smoke_room_link.mjs`,
|
||||
`demo/smoke_isometric_contract.mjs`, `demo/smoke_isometric_live_touch.mjs`
|
||||
— все существуют.
|
||||
15. Проверено, что оба Low из r1 (устаревший Labs-флаг `iso`, несобранный в
|
||||
один блок список затронутых файлов) не требовали правки ТЗ — они были
|
||||
сняты решением ревьюера r1 без возврата автору, и в правке `f7b811a`
|
||||
корректно не тронуты.
|
||||
|
||||
Код не менялся, поэтому повторной сборки/прогона гейтов для этого цикла не
|
||||
требовалось — вопрос ревью ТЗ остаётся «выполнимо и проверяемо ли», а не
|
||||
«работает ли реализация». Точечные `grep`/чтение кода использовались только
|
||||
для проверки фактических утверждений документа.
|
||||
|
||||
## Проверено и корректно
|
||||
|
||||
- **Medium-находка r1 устранена полностью и точно**: третье независимое место
|
||||
дефекта (`iso && !space.bg` override на `houseplan-card.ts:15825`) теперь
|
||||
имеет собственный мутант `hidden-room-names-iso-override` с явным guard
|
||||
(isometric contract/live smoke из AC3), список из трёх мутантов
|
||||
зарегистрирован в §10.3 отдельными строками с явным запретом объединять их
|
||||
в один — ровно то исправление, которое просил r1, без побочных изменений
|
||||
контракта.
|
||||
- Все три анкера дефекта уникальны в исходном коде (проверено `grep -c`) —
|
||||
план мутантов не просто текстуально корректен, а технически реализуем под
|
||||
правило `scripts/mutation-gate.mjs` «`find` встречается ровно один раз».
|
||||
- Правка ограничена ровно двумя точками документа (AC3, §10.3); остальной
|
||||
текст, ранее признанный корректным в r1 (сценарий, матрица видимости,
|
||||
scope/non-scope, риски, rollback, release-артефакты, блок предположений),
|
||||
не пострадал и не требует повторной валидации по существу — но был
|
||||
перечитан целиком (см. «Как проверялось» пп.3, 11–13) и по-прежнему
|
||||
корректен.
|
||||
- Все обязательные разделы §7.1 присутствуют, AC1–AC10 однозначны и несут
|
||||
явное доказательство; в частности AC3 после правки формулирует «browser
|
||||
smoke **+** мутант» вместо одного лишь смока.
|
||||
- Диагноз дефекта (три независимых места) подтверждён самостоятельно, заново,
|
||||
чтением исходников — не принят на веру из r1 или из текста ТЗ.
|
||||
- Продуктовых вопросов владельцу по-прежнему нет и это обоснованно: ни одна
|
||||
из находок этого цикла не требует решения о том, что видит или делает
|
||||
пользователь — обе ниже являются вопросами полноты документации уже принятых
|
||||
решений.
|
||||
|
||||
## Находки
|
||||
|
||||
### Low — таблица «What a space may choose not to draw» в `docs/UX-MODES.md` не получает строку `show_names`
|
||||
|
||||
**Файл:** `docs/UX-MODES.md`, раздел «What a space may choose not to draw»
|
||||
(строки 112–124); связано с `docs/specs/203-hide-room-names.md` §13
|
||||
(Release-артефакты).
|
||||
|
||||
**Суть:** после этого исправления `show_names` начинает вести себя ровно по
|
||||
правилу, которое этот раздел документирует как канон для трёх других
|
||||
переключателей — «display only… each layer stays visible in the editor that
|
||||
owns it, because a layer you cannot see is a layer you cannot edit»
|
||||
(`show_borders`, `hide_decor`, `hide_openings`). Название комнаты — четвёртый
|
||||
такой слой (виден в Plan editor через `_markup`, скрыт во View/Devices editor/
|
||||
compact card), но таблица этого раздела его не перечисляет. `docs/specs/
|
||||
203-hide-room-names.md` §13 требует обновить `USER-GUIDE.ru.md`,
|
||||
`STYLING-HOOKS.md` и `TESTING.md`, но не называет `UX-MODES.md` — канонический
|
||||
документ подсистемы, который как раз формулирует это самое правило.
|
||||
|
||||
**Почему не блокирует:** ТЗ не изобретает новое правило — оно явно (§14, п.2)
|
||||
опирается на уже принятый прецедент этого самого раздела, и правильно его
|
||||
применяет к Plan editor. Отсутствие строки в таблице — вопрос полноты
|
||||
канонического документа, а не корректности контракта AC1–AC10; ни один AC не
|
||||
зависит от текста `UX-MODES.md`, и его отсутствие не меняет проверяемость
|
||||
задачи.
|
||||
|
||||
**Решение ревьюера:** снимается без правки ТЗ, с записью в этом документе.
|
||||
Рекомендация для реализации: одной строкой добавить `show_names` в таблицу
|
||||
`docs/UX-MODES.md` при обновлении документации в implementation-коммите —
|
||||
дёшево и закрывает дрейф канона от кода, но не стоит отдельного цикла ревью.
|
||||
|
||||
### Low — release-артефакты не называют явно, что запись в changelog обязана отметить снятие `.rlabel` как «breaking change» styling hook
|
||||
|
||||
**Файл:** `docs/specs/203-hide-room-names.md` §7, §13; связано с
|
||||
`docs/STYLING-HOOKS.md` §1 («we promise the names in §3 are stable. If we
|
||||
ever have to change one, it is a breaking change and it goes in the
|
||||
changelog»).
|
||||
|
||||
**Суть:** §7 ТЗ корректно фиксирует, что `text.rlabel` перестаёт быть
|
||||
доступным hook и что документацию `STYLING-HOOKS.md` нужно поправить, но не
|
||||
связывает это явно с собственным правилом `STYLING-HOOKS.md` о том, что
|
||||
изменение записи §3 — breaking change, требующий отдельного упоминания в
|
||||
changelog (а не только общей формулировки бага «названия комнат теперь
|
||||
скрываются полностью»). Пользователь card-mod, ранее нацеливший CSS на
|
||||
`.rlabel` (что было доступно только в багованном состоянии, но синтаксически
|
||||
валидно), теряет цель без явного предупреждения.
|
||||
|
||||
**Почему не блокирует:** §13 уже требует правки обоих changelog и
|
||||
`STYLING-HOOKS.md` в implementation-коммите; добавление одной фразы про
|
||||
удаление hook — редакционное дополнение существующего обязательного пункта,
|
||||
не новое AC и не продуктовое решение.
|
||||
|
||||
**Решение ревьюера:** снимается без правки ТЗ, с записью в этом документе.
|
||||
Рекомендация для реализации: changelog-запись (RU+EN) явно называет удаление
|
||||
`.rlabel`/`text.rlabel` как исправление ошибочного styling-контракта, а не
|
||||
только пользовательский эффект «имена корректно скрываются».
|
||||
|
||||
Других находок — Low, Medium или High — не выявлено. High: 0, Medium: 0.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Реализация не существует на этапе ТЗ; код-ревью будет отдельным циклом
|
||||
(`S7-code-review`).
|
||||
- Не прогонялись `npm run typecheck`/`npm test`/`npm run build` в этом цикле:
|
||||
продуктовый код не менялся с r1 (проверено `git diff 009fed9..HEAD -- src/`
|
||||
— пусто), гейты ТЗ их не требуют на этой стадии.
|
||||
- Не запускались браузерные смоки повторно в этом цикле — точечный прогон
|
||||
`smoke_isometric_contract.mjs` в r1 уже подтвердил механику Labs-флага;
|
||||
content смоков (строки 28/25 с `show_names: true`) проверен чтением файла,
|
||||
не исполнением, только чтобы подтвердить отсутствие уже существующего
|
||||
покрытия `false`-ветки — это не гейт, а факт-чек утверждения ТЗ.
|
||||
- Не оценивались golden-сцены §10.3 — фикстуры не существуют на этапе ТЗ.
|
||||
- Не оценивалась производительность — риск в §11 не менялся между r1 и r2 и
|
||||
был признан корректным в r1.
|
||||
- Не проверялся текст будущих записей changelog — они не написаны на этапе
|
||||
ТЗ; §13 называет оба файла обязательными, находка Low выше — рекомендация
|
||||
к их содержанию, не блокирующее требование.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Medium-находка r1 (третье независимое место дефекта — форсированная
|
||||
HTML-подпись в hidden iso, `houseplan-card.ts:15825`) устранена точно и
|
||||
полно: новый мутант `hidden-room-names-iso-override` привязан к правильному
|
||||
анкеру, анкер уникален в исходнике, guard корректно ссылается на AC3, три
|
||||
мутанта явно запрещено объединять. Самостоятельная повторная проверка не
|
||||
нашла четвёртого независимого места того же дефекта и не нашла регресса в
|
||||
остальном тексте ТЗ, ранее признанном корректным. Обе новые находки —
|
||||
Low, документационная полнота уже принятых решений, не создающая продуктовой
|
||||
неопределённости и не влияющая на проверяемость AC1–AC10 — сняты решением
|
||||
ревьюера с рекомендациями для реализации. High: 0, Medium: 0.
|
||||
|
||||
**Вердикт: зелёный · цикл r2/4 · High: 0 · Medium: 0**
|
||||
Reference in New Issue
Block a user