mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 20:29:00 +00:00
claude[bot]
@@ -0,0 +1,166 @@
|
||||
# CODE-REVIEW · issue #567 · заход r1
|
||||
|
||||
Материал: `03983946c561a7e8e3e27ef0c3ba40dbebca7790` (рабочая копия — на нём же,
|
||||
HEAD detached). Трек — `trivial` (light), ТЗ и AC в теле issue, бюджет циклов —
|
||||
2, израсходовано 0.
|
||||
|
||||
## Скоуп изменения
|
||||
|
||||
`keepTwoPoint` в `radarConfigFromDraft` (`src/radar-editor.ts`) сравнивал
|
||||
`sources` через `JSON.stringify(...) === JSON.stringify(...)`, то есть текстом.
|
||||
Билдер сам меняет порядок ключей (`slots`/`ranges`/`zones`/`occupancy_entity`/
|
||||
`count_entity` удаляются и дописываются заново, `availability_entity` остаётся
|
||||
на месте) — конфиг, только что записанный этим же билдером, при следующем
|
||||
сравнении оказывался «другим», и радар с `availability_entity` молча терял
|
||||
`refs`/`rms_cm` двухточечной калибровки при первом обычном сохранении.
|
||||
|
||||
Правка вводит `canonicalSources()` — каноничное по порядку ключей объектов
|
||||
(рекурсивно, включая объекты внутри массивов) и чувствительное к порядку
|
||||
*элементов* массивов сравнение через `JSON.stringify` с сортирующим
|
||||
replacer'ом, сортировка по кодовым точкам. Диапазон правки соответствует
|
||||
заявленному откату: одна функция плюс замена одной строки сравнения.
|
||||
|
||||
Изменённые файлы вне генерируемых: `src/radar-editor.ts` (+23/−1),
|
||||
`test/radar-editor.test.mjs` (+99, новые тесты AC1–AC3), `scripts/mutation-
|
||||
registry.mjs` (+12, один мутант), `docs/images/screenshots.json` (пересчёт
|
||||
`sourceFingerprint` после `docs:accept --identical`). `dist/**` и
|
||||
`custom_components/houseplan/frontend/**` — синхронный билд.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Дешёвые гейты (`tsc --noEmit`, `npm test`, `npm run build` + сверка трёх копий
|
||||
бандла) на этом SHA уже зелёные в Validate
|
||||
(https://github.com/Matysh/houseplan-card/actions/runs/34780715957) — не
|
||||
перегонял.
|
||||
|
||||
Прогнал сам, точечно, под диффом:
|
||||
|
||||
- `npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs &&
|
||||
node --test --test-name-pattern="#567" test/radar-editor.test.mjs` —
|
||||
**3/3 зелёные** (AC1, AC2, AC3).
|
||||
- **Дисциплина «тест умеет падать»**: временно откатил `canonicalSources(...)
|
||||
=== canonicalSources(...)` на `JSON.stringify(...) === JSON.stringify(...)`
|
||||
в рабочей копии и перепрогнал те же три теста — **AC1 и AC3 красные**
|
||||
(`method`: ожидали `two_point`, получили `manual`), **AC2 остался зелёным**.
|
||||
Это ровно то, что заявил автор в хендоффе (регресс ловится не ценой ложного
|
||||
прохода AC2). Файл восстановлен из бэкапа, `git status --short` после
|
||||
восстановления — пусто, дерево чистое.
|
||||
- `node scripts/mutation-registry.mjs --check` — exit 0, без нарушений.
|
||||
- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` — вывод
|
||||
**НЕОПРЕДЕЛЁННОСТЬ**, единственный незарегистрированный символ на изменённых
|
||||
строках — `canonicalSources`. Проверил `demo/smoke_*.mjs` на упоминания
|
||||
`two_point`/калибровки — ни один смок не трогает сохранение двухточечной
|
||||
калибровки при повторном открытии редактора, значит запись в
|
||||
`smoke-links.mjs` была бы ложной связью. Решение автора «браузерный смок не
|
||||
нужен» — согласен: `canonicalSources` — приватный хелпер чистой функции,
|
||||
единственный путь к нему из браузера — `radarConfigFromDraft`, целиком
|
||||
закрытый тремя unit-AC и мутантом. Смок не прогонял (по диффу и AC не
|
||||
требуется, а факт «широкой» связи не подтверждён).
|
||||
- `node scripts/check-docs.mjs` — «Documentation checks passed (7 files, 12
|
||||
external links)», зелёный на пересчитанном `sourceFingerprint`.
|
||||
- `diff -rq dist custom_components/houseplan/frontend` — пусто, деревья
|
||||
идентичны (третья копия, `demo/srv/assets`, больше не коммитится, #255).
|
||||
- `node --test test/single-source-numbers.test.mjs` — 3/3, для очистки
|
||||
совести: правка не касается ничего отображаемого, но механический гейт
|
||||
«одно число — один источник» остаётся зелёным.
|
||||
- Грепнул `src/**` на `JSON.stringify(...) === JSON.stringify(...)` — есть ещё
|
||||
четыре места (`houseplan-editor-runtime.ts:1941,2237,10019,10513`,
|
||||
`summary-panel.ts:309`), но все они сравнивают массивы/объекты, чей порядок
|
||||
ключей контролирует не билдер того же модуля симметрично тому, как здесь —
|
||||
не предмет этой задачи (откат ТЗ прямо ограничивает её одной функцией), и в
|
||||
issue не заявлены. Не заведение, а просто фиксирую, что смотрел: если один
|
||||
из них когда-то проявит тот же паттерн (класс #319, как и написано в issue),
|
||||
это отдельная задача.
|
||||
|
||||
Не прогонял: `golden:verify` (диф не меняет рендер — чистая функция
|
||||
преобразования конфига), `pytest tests_backend` (Python не тронут),
|
||||
performance-профили (не заявлены в AC, путь не хайпасный). Полный набор
|
||||
браузерных смоков — избыточен для одной внутренней функции без UI-поверхности.
|
||||
|
||||
## AC — разобраны по коду и исполнением
|
||||
|
||||
| AC | Доказательство | Вердикт |
|
||||
|---|---|---|
|
||||
| AC1 | `test/radar-editor.test.mjs` «#567 AC1», выполнил лично: `method: two_point`, `refs`, `rms_cm`, `mount`/`cell_cm`/`mirror` неизменны, `sources` содержит все три ключа по смыслу | подтверждено |
|
||||
| AC2 | «#567 AC2», выполнил лично: смена `x_entity` → `method: manual`, `refs`/`rms_cm` удалены | подтверждено |
|
||||
| AC3 | «#567 AC3», выполнил лично: перестановка полей внутри слота не мешает; добавление/удаление слота и перестановка сущностей между слотами — мешают | подтверждено |
|
||||
|
||||
Регресс-проверка (тест умеет падать) выполнена лично, не только со слов автора
|
||||
— см. раздел «Как проверялось».
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- `canonicalSources` рекурсивна: `JSON.stringify`-replacer вызывается и для
|
||||
элементов массивов, поэтому порядок полей *внутри* слота (второй слой,
|
||||
упомянутый в issue отдельно) канонизируется тем же кодом, без специального
|
||||
случая — это закрывает наблюдение issue про `slots` заодно с
|
||||
`availability_entity`.
|
||||
- Порядок элементов массива не трогается — позиция слота осталась смыслом
|
||||
(`target_N`), что и требует контракт ТЗ; проверено тестом AC3 (перестановка
|
||||
сущностей между слотами инвалидирует калибровку).
|
||||
- Единственный мутант, `radar-sources-compared-as-text`, пойман (`--check`
|
||||
зелёный). Решение не заводить второго мутанта на сортировку массивов
|
||||
задокументировано и математически верно: `!Array.isArray` регулирует только
|
||||
то, применяется ли сортировка ключей к узлу; для массива это оставило бы его
|
||||
элементы под числовыми индексами в исходном порядке (для длин, о которых
|
||||
идёт речь — единицы слотов, лексикографический порядок строковых индексов
|
||||
совпадает с числовым) — реального различия в поведении нет, ловить нечего.
|
||||
Отдельная запись в AC3-тесте фиксирует смысловое свойство вместо этого.
|
||||
- Геометрия (`project_local`, `mount`/`cell_cm`/`mirror`) не затронута —
|
||||
проверено и код-ревью, и выполнением (AC1 отдельно утверждает неизменность
|
||||
этих величин).
|
||||
- `User-Visible: no` обоснован: `method` и `rms_cm` нигде не рендерятся
|
||||
(грепом по `src/*.ts` подтверждено отсутствие обращений вне
|
||||
`radar-editor.ts`/`radar-setup.ts`/`types.ts`), геометрия не двигается —
|
||||
changelog не нужен.
|
||||
- Трейлеры коммита: `Issue: #567`, `User-Visible: no` — оба на месте, коммит
|
||||
один.
|
||||
- Классы изменений соблюдены: `src/**` и `test/**`/`scripts/**` — по одному
|
||||
issue; `dist/**` и `custom_components/houseplan/frontend/**` синхронны
|
||||
(класс D, не редактировались вручную); `docs/images/screenshots.json` —
|
||||
часть DoD правки `src/**` (обновление отпечатка через `docs:accept
|
||||
--identical`, все 11 кадров попиксельно совпали).
|
||||
|
||||
## Находки
|
||||
|
||||
Нет. High: 0, Medium: 0, Low: 0.
|
||||
|
||||
## Продуктовое рассуждение (SCOPE.md)
|
||||
|
||||
Правка не расширяет и не меняет ни один Core user job — она чинит J7-смежный
|
||||
внутренний контракт (сохранность результата двухточечной калибровки радара
|
||||
между сохранениями), не вводя нового поведения и не трогая ни один экран.
|
||||
Она укладывается в «Keep the plan true as the home evolves» (J6) косвенно:
|
||||
предотвращает молчаливую деградацию уже настроенной инфраструктуры без
|
||||
действия пользователя. Возражений по скоупу нет.
|
||||
|
||||
## Чего не проверял и почему
|
||||
|
||||
- `npx tsc --noEmit` / полный `npm test` / `npm run build` — не перегонял,
|
||||
зелёный Validate на этом же SHA уже подтверждён и назван по ссылке.
|
||||
- Браузерные смоки — не прогонял ни одного: `smoke-select` дал
|
||||
НЕОПРЕДЕЛЁННОСТЬ по единственному непокрытому символу, ручная проверка
|
||||
показала, что ни один смок не относится к сценарию двухточечной калибровки,
|
||||
прогон был бы шумом без сигнала.
|
||||
- `golden:verify`, `pytest tests_backend`, performance-профили — не по AC, не
|
||||
по diff (нет Python, нет изменений рендера).
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. Обе стороны регресса (ложное «изменилось» и ложное «не изменилось»)
|
||||
проверены исполнением, не только чтением; геометрия и достаточность отката
|
||||
подтверждены; гейты, требуемые диффом, зелёные.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/567-radar-canonical-sources`, коммит `03983946c561` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `f3a9eb9b27aa5d0ecd52c09a27483f9e8e50bc06`
|
||||
```
|
||||
git log --all --format='%H %T' | grep f3a9eb9b27aa
|
||||
```
|
||||
- Тело issue: `aa2955659fda7d26ffd448236319e735d0a5c1d9ebec765ad9c00e559e82f000`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user