mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,299 @@
|
||||
# SPEC-REVIEW-406-r2
|
||||
|
||||
- Issue: #406
|
||||
- ТЗ: `docs/specs/406-beta2-polish.md`, ветка `issue/406-beta2-polish`,
|
||||
ревизия 2, SHA `0d5b52d1` (HEAD ветки `44715dd0` добавляет поверх только
|
||||
`docs/reviews/SPEC-REVIEW-406-r1.md`, самого ТЗ не трогает)
|
||||
- Этап: spec (PROCESS.md §2.4)
|
||||
- Заход: r2 · блокирующих циклов израсходовано 1 из 4
|
||||
- Вердикт: **жёлтый**
|
||||
|
||||
## Почему разбор полный, а не по дельте
|
||||
|
||||
Владелец перебазировал ветку на `origin/dev` (`b04387ce`) между r1 и r2.
|
||||
SHA r1, названный в его собственном документе, — `8248f9e4` — на текущем
|
||||
дереве не существует (`git cat-file -t 8248f9e4` → `Not a valid object
|
||||
name`): это не «SHA не назван», SHA назван корректно, но ребейз его
|
||||
переписал. Это ровно случай PROCESS.md §2.10: «ребейз на ушедший вперёд dev
|
||||
(после ребейза это другой код)» — разбор обязан быть полным. Дополнительное
|
||||
основание для полноты: ребейз протащил на ветку чужой коммит
|
||||
(`8119c523`, issue #409), который меняет ровно тот файл и ровно то место,
|
||||
которое контракт (д) описывает как открытый дефект — см. находку №1.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Без изменений от r1: пять независимых мелочей полного трека (i18n-мусор +
|
||||
гейт, роль `hp-dialog`/`hp-confirm`, смок HA-ветки, уборка снапшотов area,
|
||||
сохранение следа приёмки скриншотов).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Прочитан `docs/SCOPE.md` (раздел «Partially covered»: «Accessibility:
|
||||
… no ARIA labelling of the plan» — задача (б)/(в) законно попадает сюда),
|
||||
`PROCESS.md` §2.4/§2.9/§2.10/§7.1/§7.2, тело issue #406 и все комментарии
|
||||
(включая S2-разбор, ссылку на ТЗ, вердикт r1, ответ владельца на r1),
|
||||
`docs/USER-GUIDE.ru.md` (терминология подтверждения/разблокировки, строки
|
||||
672-730, 824-963 — новых формулировок ТЗ не вводит).
|
||||
|
||||
Само ТЗ вычитано целиком построчно. Каждое фактическое утверждение
|
||||
перепроверено на текущем дереве (не на SHA r1 — его больше нет), а не
|
||||
принято на слово:
|
||||
|
||||
- `src/i18n/en.json|ru.json|de.json|fr.json` — подсчёт ключей и паритет
|
||||
(`node -e` подсчёт), точечная проверка литерального использования всех
|
||||
13 заявленных мёртвых ключей и представителей заявленных динамических
|
||||
семейств (`furn.cat_*`, `furn.sym_*`, `wall_model.reason.*`,
|
||||
`resize.disabled.*`, `junction.limit_*`, `decor.*`, `_help()`/`.aria`);
|
||||
- `src/hp-dialog.ts`, `src/hp-confirm.ts`, `src/danger-confirm.ts`,
|
||||
`src/houseplan-card.ts` (строки вокруг `_lockAction`) — роль диалога,
|
||||
устройство `HpConfirmKind`, место вызова `warning`-подтверждения;
|
||||
- `src/device-area-relocation.ts` целиком — `resolveDeviceAreaRelocations`,
|
||||
`applyAreaRelocationResolution`, `markerAreaSnapshotOf`, `git log` файла
|
||||
(единственный коммит с #126, после рёбейза не менялся);
|
||||
- `scripts/docs-accept.mjs` целиком, `git log -p` по нему — здесь нашлась
|
||||
находка №1;
|
||||
- `test/unified-wall-tool-source.test.mjs:32`, `demo/smoke_free_walls.mjs`
|
||||
(ha-dialog-стаб), `demo/smoke_esc_dialogs.mjs`, `demo/smoke_danger_confirm_
|
||||
branches.mjs`, `demo/smoke_danger_confirmation.mjs` — покрытие веток и
|
||||
прецедент стаба.
|
||||
|
||||
Гейты (`tsc`/`test`/`build`) не гонял: `git diff --stat origin/dev...HEAD`
|
||||
показывает только два doc-файла (`docs/specs/406-beta2-polish.md`,
|
||||
`docs/reviews/SPEC-REVIEW-406-r1.md`), продуктового кода на ветке ещё нет —
|
||||
то же основание, что в r1.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где видно |
|
||||
|---|---|---|
|
||||
| (а) Medium: инвентарь `*.help.aria` неверен (13 из 19 объявлены мёртвыми, а живы все 19) | Раздел «(а)» переписан: явно выделен абзац «Важное динамическое семейство, которое удалять нельзя» — все 19 `*.help.aria` признаны живыми через производную `${key}.aria` от литерального `_help('*.help')`; список мёртвых сокращён до 13 (7 `confirm.*` + 6 прочих); правило внесено в AC3 явно, а не подразумевается в «прочих семействах» | `docs/specs/406-beta2-polish.md:59-64,217-221`. Перепроверено заново на текущем дереве (не наследовано): все 19 `*.help.aria`-ключей действительно имеют по одному литеральному `_help('x.help')`-потребителю, 13 заявленных мёртвых ключей — без потребителей ни в одном из 4 словарей |
|
||||
| (б) Medium: AC6/AC7 не решают роль для `HpConfirmKind.warning` | Контракт (б) переписан: явная дихотомия снята, оба `kind` — `destructive` и `warning` — объявлены as `alertdialog`; `warning` назван явно как разблокировка двери с `confirm.unlock_body`; AC6 переформулирован на «оба вида» | `docs/specs/406-beta2-polish.md:100-105,227-230`. Перепроверено: `houseplan-card.ts:13041-13049` — единственный `warning`-запрос, ссылка точна |
|
||||
|
||||
Обе находки r1 закрыты по существу, не косметически. Полный повторный разбор
|
||||
(см. выше) вскрыл в разделе (б) отдельную, ранее не поднимавшуюся проблему
|
||||
феазибилити (находка №2) — она не переоткрывает находку r1, это независимый
|
||||
дефект того же раздела.
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Формально пусто: ребейз на `dev` обесценил SHA r1, поэтому по правилу §2.10
|
||||
разбор в этом раунде — полный, а не по дельте, и каждое утверждение ТЗ
|
||||
перепроверено заново на текущем дереве (см. «Как проверялось»), а не принято
|
||||
на основании документа r1. Структурная оценка §7.1 (все обязательные разделы
|
||||
присутствуют, продуктовых вопросов владельцу не вынесено, скоуп/не-скоуп
|
||||
корректны, `docs/SCOPE.md` подтверждает легитимность полного трека) не
|
||||
поменялась между r1 и r2 и не пересчитывалась заново построчно — в остальном
|
||||
результат r1 в этой части подтверждён независимым прочтением r2, а не
|
||||
переносится слепо.
|
||||
|
||||
## Находки
|
||||
|
||||
### [Medium, в скоупе] №1 — (д) контракт описывает уже исправленный дефект и расходится с тем, что реально попало в дерево ребейзом
|
||||
|
||||
**Файл**: `docs/specs/406-beta2-polish.md`, раздел «(д) Затирание следа
|
||||
приёмки» (строки 157-172) и AC12/AC13 (строки 246-251).
|
||||
|
||||
**Что не так**: ТЗ строит контракт (д) на утверждении «`scripts/docs-
|
||||
accept.mjs:145` пишет `declared: [...decision.replace]`» безусловно, и
|
||||
приводит модельный прогон: «`replace=[]` → в манифест уйдёт
|
||||
`acceptance.declared = []`». На SHA r1 (`8248f9e4`) это было верно — сам
|
||||
дефект и являлся предметом п. (д). Но ветка с тех пор перебазирована на
|
||||
`origin/dev`, и в `dev` уже есть коммит `8119c523` («fix: the screenshot
|
||||
witness floor comes from the set, not from survivors», issue #409, попал в
|
||||
дерево этим самым ребейзом), который правит именно эту строку:
|
||||
|
||||
```js
|
||||
// scripts/docs-accept.mjs (текущее дерево)
|
||||
const previous = JSON.parse(readFileSync(resolve(ROOT, 'docs/images/screenshots.json'), 'utf8'))
|
||||
.acceptance;
|
||||
const accepted = {
|
||||
...manifest,
|
||||
acceptance: decision.replace.length
|
||||
? { declared: [...decision.replace], witnesses: decision.witnesses.length, floor: decision.floor, ... }
|
||||
: { ...(previous || {}), lastWriteWasFingerprintOnly: true },
|
||||
};
|
||||
```
|
||||
|
||||
Commit message коммита `8119c523` прямым текстом: «Попутно Low из #405:
|
||||
повторная приёмка неизменённого набора затирала `acceptance.declared`
|
||||
пустым списком … Прежний след сохраняется и помечается
|
||||
`lastWriteWasFingerprintOnly`.» — то есть ровно дефект (д), заведённый в
|
||||
#406 как самостоятельный пункт, уже был отмечен как Low в #405 и уже
|
||||
устранён в #409 до того, как ТЗ #406 дошло до ревизии 2 (ревизия 2 датирована
|
||||
2026-09-01, тем же днём, что и коммит фикса, но после ребейза на `dev`,
|
||||
согласно комментарию владельца «Ветка перебазирована… ТЗ поднято до
|
||||
revision 2»).
|
||||
|
||||
Хуже того — исправление, которое реально попало в дерево, **не совпадает**
|
||||
с контрактом (д) в тексте ТЗ. ТЗ требует: «Свежие числа (`witnesses`,
|
||||
`floor`) обновляются, список `declared` … сохраняется» — то есть на пустом
|
||||
`replace` `witnesses`/`floor` должны пересчитываться заново. Фактически
|
||||
смёрженный код делает обратное: при `replace.length === 0` он **не**
|
||||
пересчитывает `witnesses`/`floor` вовсе, а замораживает весь предыдущий
|
||||
`acceptance`-блок целиком (`{...(previous || {})}`) и лишь помечает его
|
||||
флагом `lastWriteWasFingerprintOnly` — что по своему обоснованию в коммите
|
||||
специально сигнализирует «эти числа не свежие, а старые от последней
|
||||
настоящей проверки».
|
||||
|
||||
**Сценарий отказа**: если реализация будет буквально следовать тексту ТЗ
|
||||
(«свежие числа `witnesses`/`floor` обновляются»), она отменит уже
|
||||
смёрженное и обоснованное поведение `lastWriteWasFingerprintOnly` — то есть
|
||||
пункт (д) этой же задачи внесёт **регресс** в код, который #409 только что
|
||||
починил, и уберёт сигнал «эти цифры устарели», ради которого фикс и писался.
|
||||
Если же реализация просто оставит код как есть (ничего не делая, потому что
|
||||
контракт уже выполнен фактически), AC12/AC13 будут пройдены случайно, без
|
||||
теста на функцию `main()` (`docs-accept.mjs`) — единственный существующий
|
||||
тест, `test/docs-accept.test.mjs`, покрывает только `verifyDocsCandidate`,
|
||||
а не запись `acceptance` в `main()`; `test/docs-acceptance.test.mjs`
|
||||
покрывает только чистую функцию `docsAcceptancePlan`, тоже без записи файла.
|
||||
То есть заявленное AC12 «доказательство: тест на функции приёмки — `replace:
|
||||
[]` оставляет прежний список» ни разу не написано и по этой ревизии ТЗ не
|
||||
попадёт в план работы как отдельная задача, потому что автор считает пункт
|
||||
ещё непочиненным.
|
||||
|
||||
**Ожидаемо**: раздел (д) переписан с учётом текущего состояния дерева —
|
||||
либо признаётся, что дефект уже устранён коммитом `8119c523`/#409 и пункт
|
||||
(д) сокращается до «добавить недостающий тест на запись `main()` в
|
||||
`docs-accept.mjs`, поведение не менять», либо, если владелец сознательно
|
||||
хочет другое поведение (`witnesses`/`floor` обновлять даже на пустом
|
||||
`replace`, отказавшись от `lastWriteWasFingerprintOnly`), это должно быть
|
||||
явно названо как осознанное расхождение с #409, а не как продолжение
|
||||
нарратива «дефект ещё жив».
|
||||
|
||||
### [Medium, в скоупе] №2 — (б) контракт не называет механизм, которым HA-ветка `hp-dialog` получает `alertdialog`/`aria-describedby`, хотя рядом есть свидетельство, что «просто атрибут» не сработает
|
||||
|
||||
**Файл**: `docs/specs/406-beta2-polish.md`, раздел «(б)» (строки 107-115) и
|
||||
AC6 (строки 227-230).
|
||||
|
||||
**Что не так**: контракт формулирует требование как факт, а не как
|
||||
инженерную задачу: «Практически: `hp-dialog` получает семантический признак
|
||||
alert/обычный и id описания … Обе ветки — HA и запасная — объявляют роль и
|
||||
описание одинаково.» Это утверждение о поведении стороннего компонента
|
||||
`ha-dialog`, которого нет в этом репозитории (это компонент фронтенда Home
|
||||
Assistant, подключаемый в рантайме; локально существует только тестовый
|
||||
стаб, `demo/smoke_free_walls.mjs:20-31`, — фейковый `HTMLElement` с
|
||||
произвольным `shadowRoot`, написанный этим же проектом, а не поведение
|
||||
настоящего `ha-dialog`).
|
||||
|
||||
В том же файле, который цитирует контракт, уже есть прецедент именно этой
|
||||
проблемы: для связывания заголовка (`aria-labelledby`) HA-ветка не
|
||||
принимает обычный HTML-атрибут — она использует специальное свойство
|
||||
`ha-dialog`, `.ariaLabelledBy` (`src/hp-dialog.ts:449`,
|
||||
`.ariaLabelledBy=${this._titleId}`), тогда как запасная ветка использует
|
||||
обычный атрибут `aria-labelledby=${this._titleId}` (`:462`). Это разделение
|
||||
не случайно: `ha-dialog` рендерит собственную внутреннюю структуру в своём
|
||||
`shadowRoot`, и произвольный ARIA-атрибут, поставленный на внешний тег
|
||||
`<ha-dialog>`, необязательно долетает до реального узла диалога внутри —
|
||||
именно поэтому для заголовка потребовалось выделенное свойство, а не
|
||||
атрибут.
|
||||
|
||||
Контракт (б) не называет для `alertdialog`/`aria-describedby` никакого
|
||||
аналога `.ariaLabelledBy` — ни существующего свойства `ha-dialog` (в этом
|
||||
репозитории такого свойства нигде не встречается — `grep -rn
|
||||
"ariaDescribedBy|ariaLive|type=.\{0,5\}alert"` по `src/*.ts` пуст), ни
|
||||
явного признания, что такого свойства может не быть и нужно исследование
|
||||
на этапе реализации. При этом ни один существующий смок не проверяет
|
||||
роль/описание на HA-ветке вообще (`demo/smoke_esc_dialogs.mjs:34` — только
|
||||
на запасной `dialog`; ни в одном файле `demo/smoke_*.mjs` нет `getAttribute
|
||||
('role')` рядом с `ha-dialog`), то есть у проекта пока нет ни одного
|
||||
подтверждённого прецедента, что произвольный ARIA-признак на HA-ветке вообще
|
||||
долетает до реального диалога в настоящем Home Assistant — только
|
||||
локальный, полностью подконтрольный автору стаб.
|
||||
|
||||
**Сценарий отказа**: реализация ставит `role="alertdialog"` и
|
||||
`aria-describedby` атрибутами прямо на тег `<ha-dialog>`, локальный смок
|
||||
(тестирующий собственный же стаб) зеленеет по AC6/AC8, PR проходит ревью —
|
||||
а в реальном Home Assistant скринридер по-прежнему не объявляет
|
||||
`alertdialog`, потому что настоящий `ha-dialog` эти атрибуты на внешнем
|
||||
теге игнорирует так же, как проигнорировал бы `aria-labelledby`, если бы
|
||||
для него не завели `.ariaLabelledBy`. Ровно тот класс регрессии, который
|
||||
локальный тест из-за природы стаба не способен поймать.
|
||||
|
||||
**Ожидаемо**: контракт (б) явно называет способ донести
|
||||
alert-семантику/описание до реального `ha-dialog` (существующее свойство,
|
||||
если оно есть — тогда назвать его, как назван `.ariaLabelledBy`), либо
|
||||
явно помечает это как открытый технический вопрос реализации с планом
|
||||
проверки на реальной Home Assistant (не только на локальном стабе), а не
|
||||
формулирует как решённый факт «обе ветки объявляют одинаково».
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **(а)** Числа сходятся на текущем дереве независимо от r1: 1201 ключ во
|
||||
всех четырёх словарях (`en`/`ru`/`de`/`fr`), все 13 заявленных мёртвых
|
||||
ключей (7 `confirm.*` + `marker.display_hint(_icon)`, `history.
|
||||
delete_room`, `markup.delete`, `title.markup`, `history.partition_add`)
|
||||
без единого литерального потребителя ни в одном из четырёх словарей; все
|
||||
19 `*.help.aria` действительно производятся из литеральных
|
||||
`_help('*.help')` на строке `houseplan-editor-runtime.ts:1194`, номер
|
||||
строки точен. Заявленные динамические семейства AC3 (`furn.cat_*`,
|
||||
`furn.sym_*`, `wall_model.reason.*`, `resize.disabled.*`,
|
||||
`junction.limit_*`, `decor.*`) реально существуют как шаблонные литералы
|
||||
в `houseplan-editor-runtime.ts`/`houseplan-card.ts`. `52 = 13×4` верно.
|
||||
Коллизия с `test/unified-wall-tool-source.test.mjs:32` подтверждена
|
||||
дословно на текущей строке.
|
||||
- **(б) дихотомия destructive/warning** — подтверждена: ровно 7 мест
|
||||
`kind: 'destructive'` (все — удаления, `houseplan-editor-runtime.ts` и
|
||||
`houseplan-onboarding-runtime.ts`) и ровно одно `kind: 'warning'`
|
||||
(`houseplan-card.ts:13041-13049`, разблокировка, `confirm.unlock_body`).
|
||||
`hp-confirm.ts` на текущем дереве действительно не передаёт `hp-dialog`
|
||||
никакого alert-признака — контракт описывает реальный, ещё не закрытый
|
||||
разрыв, только механизм его закрытия для HA-ветки не назван (находка №2).
|
||||
- **(в) непокрытая HA-ветка** — подтверждена без изменений от r1:
|
||||
`smoke_free_walls.mjs` — единственный файл с `ha-dialog`-стабом и глушит
|
||||
`_confirmDanger`; `smoke_danger_confirmation.mjs`/`smoke_danger_
|
||||
confirm_branches.mjs` слова `ha-dialog` не содержат. Файл `demo/smoke_
|
||||
danger_confirm_branches.mjs` действительно существует (создан #402,
|
||||
подтверждено содержимым) — план расширить именно его реалистичен.
|
||||
- **(г) утечка снапшота и порядок обрезки** — файл `device-area-
|
||||
relocation.ts` не менялся с исходного коммита #126 (`633cb20e`, до
|
||||
ребейза), поэтому не задет им; перечитан целиком заново, а не наследован.
|
||||
`resolveDeviceAreaRelocations` действительно строит `decisions` только из
|
||||
цикла по `options.devices` (строка 155) — устройство, отсутствующее в
|
||||
этом списке целиком, не порождает decision и, соответственно,
|
||||
`applyAreaRelocationResolution` (строка 239) его запись не тронет никогда.
|
||||
Условие `!options.authoritative` (строка 142) — ранний `return` с пустым
|
||||
результатом, соответствует контракту «не трогать при неавторитетном
|
||||
реестре». `.slice(0, MARKER_AREA_SNAPSHOT_LIMIT)` (строка 68,
|
||||
`MARKER_AREA_SNAPSHOT_LIMIT = 20_000`, строка 9) читает по порядку
|
||||
вставки Object.entries — обрезка действительно режет самые свежие записи;
|
||||
разворот правила (AC11) реализуем без переписывания структуры.
|
||||
`removeMarkerAreaSnapshots` (строки 8326, 8448 `houseplan-editor-
|
||||
runtime.ts`) — номера строк точны.
|
||||
- **UX/терминология** — `docs/USER-GUIDE.ru.md` уже описывает
|
||||
подтверждение разблокировки (строки 729-730) и удаления (672-673,
|
||||
824-847) тем же языком, который использует ТЗ; новых терминов не введено.
|
||||
`docs/SCOPE.md`, раздел «Partially covered»: «Accessibility: … no ARIA
|
||||
labelling of the plan» — пункты (б)/(в) закрывают именно эту декларативно
|
||||
признанную продуктом брешь, скоуп легитимен.
|
||||
- **Скоуп/не-скоуп, отсутствие технических вопросов владельцу** — как и в
|
||||
r1, границы очерчены явно, пересечения с #403/#405 разведены по разным
|
||||
контрактам одного файла; ни одного технического вопроса не вынесено
|
||||
владельцу.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Гейты (`tsc`/`test`/`build`, `check-docs.mjs`) — не гонял: диапазон
|
||||
`origin/dev...HEAD` содержит только два `.md`-файла, продуктового кода
|
||||
ещё нет. То же основание, что в r1.
|
||||
- Не пересчитывал вручную полный список «48 динамических семейств» и
|
||||
«1188 использовано / 13 не используется» по всем 1201 ключам — точечно
|
||||
перепроверил категории, названные в тексте (`confirm.*`, `*.help.aria`,
|
||||
единичные ключи, шесть заявленных семейств AC3); этого достаточно, чтобы
|
||||
подтвердить или опровергнуть числа в тексте, исчерпывающий пересчёт —
|
||||
задача самого гейта (AC1), а не ревью ТЗ.
|
||||
- Не проверял (унаследовано из r1 без изменений, дельта его не касается):
|
||||
останется ли запись `marker_area_snapshot` живой, если устройство пропало
|
||||
из `_devices`, но по тому же id ещё существует маркер — открытый вопрос
|
||||
r1 остаётся открытым, контракт (г) не переформулирован в этой ревизии,
|
||||
граница AC9 та же, что и была.
|
||||
- Не проверял поведение настоящего `ha-dialog` в реальной Home Assistant
|
||||
(нет доступа к его исходнику из этого репозитория) — отсюда и находка №2:
|
||||
это ровно то, что нельзя подтвердить чтением этого репозитория, и спецификация
|
||||
обязана либо назвать механизм, либо явно пометить как неисследованное.
|
||||
- Не проверял `scripts/mutation-gate.mjs` на предмет технической
|
||||
реализуемости трёх заявленных мутантов — на этапе ТЗ это описание
|
||||
намерения, а не код.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Вердикт: жёлтый · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 2 → в задаче
|
||||
Reference in New Issue
Block a user