From bd780c60138710e717df0bfd649c18009eeecae1 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:27:40 +0000 Subject: [PATCH] docs: review document for #406 Issue: #406 User-Visible: no --- docs/reviews/SPEC-REVIEW-406-r2.md | 299 +++++++++++++++++++++++++++++ 1 file changed, 299 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-406-r2.md diff --git a/docs/reviews/SPEC-REVIEW-406-r2.md b/docs/reviews/SPEC-REVIEW-406-r2.md new file mode 100644 index 00000000..38f24395 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-406-r2.md @@ -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-атрибут, поставленный на внешний тег +``, необязательно долетает до реального узла диалога внутри — +именно поэтому для заголовка потребовалось выделенное свойство, а не +атрибут. + +Контракт (б) не называет для `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` атрибутами прямо на тег ``, локальный смок +(тестирующий собственный же стаб) зеленеет по 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 → в задаче