mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,183 @@
|
||||
# SPEC-REVIEW-223-r2
|
||||
|
||||
- Issue: [#223](https://github.com/Matysh/houseplan-card/issues/223) «Оптимизировать планы» должна
|
||||
канонизировать координаты, а не консервировать floating-point шум
|
||||
- ТЗ: [`docs/specs/223-optimize-coordinate-canonicalization.md`](../specs/223-optimize-coordinate-canonicalization.md)
|
||||
на коммите `965711ee2005b8ec2587b9c70fb9b14735c7756e` (HEAD)
|
||||
- Раунд: r2/4
|
||||
- Вердикт: **жёлтый**
|
||||
|
||||
## Скоуп ревью (по дельте, PROCESS.md §2.10)
|
||||
|
||||
Round r1 получен на `c0fff33` (документ `docs/reviews/SPEC-REVIEW-223-r1.md`,
|
||||
вердикт жёлтый, High: 0, Medium: 2). Предмет этого раунда —
|
||||
`git diff c0fff33..965711e`: правки только в
|
||||
`docs/specs/223-optimize-coordinate-canonicalization.md`, 27 добавлений / 15
|
||||
удалений, разделы §7 (UX/запись), AC4, AC6, §10 (план реализации), §11 (риски),
|
||||
§13 (принятые предположения). Ни код, ни другие документы, ни тело issue в
|
||||
дельте не участвуют — дельта локальна текстом ТЗ, разбор полным не открывается
|
||||
(критерии §2.10 «дельта не локальна» не выполнены: нет ребейза, нет смены
|
||||
контракта, не задета новая подсистема, объём дельты — 42 строки против ~230
|
||||
строк исходного ТЗ).
|
||||
|
||||
Отдельно проверено: не расширился ли скоуп задачи и не сломан ли этой правкой
|
||||
какой-либо AC, который r1 уже принял (§2.10 п.5, регрессия по образцу #102) —
|
||||
см. «Унаследовано из r1» ниже.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Восстановлен вердикт r1 и его SHA командой `gh issue view 223 --comments` —
|
||||
`c0fff33`, назван в самом документе r1 (в этом раунде SHA назван, замечание
|
||||
предыдущего цикла о неназванном SHA сюда не относится).
|
||||
2. `git diff c0fff33..965711e` — построчный разбор всех правок (полный текст
|
||||
выше в тред-контексте инструмента).
|
||||
3. По каждой из двух находок r1 — проверено текстовое место закрытия (см.
|
||||
таблицу ниже), не только заявление автора в хендофф-комментарии.
|
||||
4. Для находки №2 (терминологическая коллизия) — проверено, не создаёт ли
|
||||
выбранная замена термина новую коллизию: `grep` по `нормализ|normali` в
|
||||
`docs/*.md` и `src/i18n/*.json` показал, что «normalised/нормализовано»
|
||||
уже плотно занято во всём проекте другим, устоявшимся значением —
|
||||
координата в диапазоне 0..1 холста (`docs/CANVAS.md:12,20,333,489,539`,
|
||||
`docs/ARCHITECTURE.md:345,356` и др.), и что канонический документ самой
|
||||
этой фичи (`docs/CANVAS.md` §9.5, строка 421) уже называет ровно ту
|
||||
операцию, которую считает счётчик `c` (rekey `open_spans`/`open_to`),
|
||||
термином **«canonicalisation»**, а не «normalisation». Разбор ниже.
|
||||
5. AC4/AC6/§10/§11/§13 в новой редакции сверены на внутреннюю
|
||||
непротиворечивость (совпадение формулировки контракта, доказательной
|
||||
матрицы и риска для каждого изменения).
|
||||
6. Комментарии issue #223 целиком, включая хендофф r1→r2, прочитаны заново на
|
||||
предмет незаявленных изменений скоупа — не найдено.
|
||||
|
||||
Код не проверялся: реализация не написана, этап `spec`.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где видно |
|
||||
|---|---|---|
|
||||
| Medium 1 — AC4 не покрывает `partitions`/`wall_columns`, наивная tracked-обёртка на входе засчитает отклонённый snap партиции | AC4 расширен явным требованием: «Принятая партиция и `wall_column` учитываются; у партиции с `hostedFit = false` вычисленный, но не записанный snap не учитывается и noisy endpoints сохраняются», доказательная матрица AC4 включает «применённой и отклонённой partition, wall column»; §10 переписан — вклад в `coordsCanonicalized` подтверждается «только в месте фактической записи значения в candidate», явно расписана ветка `snappedLength > EPS && hostedFit` и её `else`-ветвь; §11 получил строку риска «Отклонённый snap партиции попадает в отчёт» с мерой | `docs/specs/223-optimize-coordinate-canonicalization.md` §9 (AC4), §10, §11, коммит `965711e` |
|
||||
| Medium 2 — новый показатель делит корень «канонизировано» с существующим `c` в одной строке диалога | §7 переписан: `c` получил метку «нормализовано пространств» / «spaces normalized», `p` — «устранён шум координат» / «noisy coordinate values removed»; текст явно называет оба значения раздельно и утверждает «два разных показателя не используют один термин "канонизировано" в одном предложении»; AC6 и §11 обновлены синхронно; §13 добавил пункт 3 о том, что переименование `c` затрагивает только текст, не структуру отчёта | `docs/specs/223-optimize-coordinate-canonicalization.md` §7, §9 (AC6), §11, §13, коммит `965711e` |
|
||||
|
||||
Буквальная коллизия слова «канонизировано» действительно устранена. Но выбор
|
||||
замены для находки 2 создаёт новую проблему того же класса — см. находку ниже:
|
||||
закрытие частично, регресс того же типа воспроизвёлся другим словом.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи) — замена термина для находки r1 №2 меняет одну коллизию на другую: «нормализовано»/«normalized» уже устойчиво означает координатную систему 0..1, а не эту операцию
|
||||
|
||||
`docs/specs/223-optimize-coordinate-canonicalization.md` §7 (правка этого
|
||||
раунда): существующий показатель `c` (`OptimizeReport.canonicalized` — число
|
||||
пространств, где переписано JSON-представление `open_spans`/`open_to`/`walls`
|
||||
после rekey) получает пользовательскую метку RU «нормализовано пространств»,
|
||||
EN «spaces normalized».
|
||||
|
||||
Ровно это слово уже занято в проекте другим, никак не связанным значением —
|
||||
координата в диапазоне 0..1 холста, а не операция над JSON-представлением
|
||||
пространства:
|
||||
|
||||
- `docs/CANVAS.md:12` «the whole plan… as if the normalised unit square»;
|
||||
- `docs/CANVAS.md:20` «device positions are still stored normalised»;
|
||||
- `docs/CANVAS.md:333,489,539` — то же значение ещё трижды в том же файле;
|
||||
- `docs/ARCHITECTURE.md:345` «All coordinates are **normalized (0..1 of the
|
||||
canvas)**»; `:356` то же для layout v2.
|
||||
|
||||
Хуже: канонический документ именно этой фичи (`docs/CANVAS.md` §9.5, строка
|
||||
421 — «Оптимизировать планы» explicit whole-plan maintenance, ровно тот пассаж,
|
||||
который описывает проход, считаемый показателем `c`) уже называет эту
|
||||
операцию своим устоявшимся именем — **«exact open-span canonicalisation»**, не
|
||||
«normalisation». ТЗ заменяет пользовательскую метку операции с одного слова,
|
||||
конфликтовавшего с `p` в том же предложении («канонизировано» дважды), на
|
||||
слово, конфликтующее с устоявшимся термином координатной системы в том же
|
||||
каноническом документе и во всём проекте — и одновременно расходящееся с тем,
|
||||
как канонический документ называет саму эту операцию.
|
||||
|
||||
**Почему это не гипотетика, а конкретный сценарий чтения.** Строка диалога
|
||||
после правки — «нормализовано пространств: {c}; устранён шум координат: {p}» —
|
||||
это два счётчика **в одном предложении о геометрии/координатах**, один из
|
||||
которых называется словом, которое во всей остальной документации означает
|
||||
«координата приведена к 0..1». Администратор, читающий предпросмотр Optimize
|
||||
рядом с формулировкой issue («канонизировать координаты») или с `docs/CANVAS.md`
|
||||
(где то же слово стоит для другого понятия дважды на расстоянии нескольких
|
||||
строк от «canonicalisation»), может прочитать «нормализовано пространств» как
|
||||
третий, ещё один вид работы с координатами — то есть тот же класс путаницы,
|
||||
который находка r1 №2 уже описывала для слова «канонизировано», просто с новым
|
||||
словом. `docs/CANVAS.md` §9.5 и комментарий `AUD-158B1-01` в шапке
|
||||
`align-grid.ts` (процитирован в ревью r1) требуют точности этого отчёта как
|
||||
условия доверия — новый термин это условие не выполняет лучше старого.
|
||||
|
||||
**Фикс (в скоупе, решает автор):** не использовать «нормализовано»/«normalized»
|
||||
для `c` — слово занято координатной системой во всём проекте, включая
|
||||
канонический документ самой этой фичи. Канонический термин для операции,
|
||||
которую считает `c`, уже есть в `docs/CANVAS.md` §9.5 — «canonicalisation»
|
||||
(open-span); задача может либо оставить `c` на этом корне и развести его с `p`
|
||||
через уточняющее существительное (что именно канонизировано — представление
|
||||
связей пространства, а не координаты: например RU «пространств приведено к
|
||||
каноническому представлению: {c}» / EN «spaces re-canonicalised: {c}»), либо
|
||||
выбрать нейтральное слово без занятого во всём проекте значения (например
|
||||
«обновлено»/«updated», «перелинковано»/«relinked»). Любой вариант закрывает
|
||||
находку без создания новой при условии, что слово не совпадает ни с
|
||||
«канонизировано» (уже занято `p`), ни с «нормализовано»/«normalized» (уже
|
||||
занято координатной системой).
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Находка r1 №1 (AC4/`partitions`/`wall_columns`) закрыта по существу** —
|
||||
контракт §6 п.3 («результат действительно записан в candidate») теперь
|
||||
дословно отражён в AC4 и в плане реализации §10 с указанием точной ветки
|
||||
(`snappedLength > EPS && hostedFit`); риск-таблица получила отдельную строку.
|
||||
Ни один тест из новой доказательной матрицы не назван расплывчато.
|
||||
- **Идемпотентность, контракт снапа §6, исключение проёмов, границы
|
||||
scope/non-scope, release-артефакты, отсутствие продуктовых вопросов** — не
|
||||
затронуты дельтой этого раунда, проверка не повторялась (см. «Унаследовано
|
||||
из r1»).
|
||||
- **Нумерация принятых предположений в §13** — после вставки нового пункта 3
|
||||
весь список перенумерован без дублей и разрывов (1…6), проверено построчно.
|
||||
- **Формулировка §7 больше не содержит самоссылки** — прежний текст перечислял
|
||||
«счётчики миграций/**планов**/стен/виртуальных фрагментов» рядом с полем,
|
||||
которое само и есть «планов»; новая редакция убрала слово из перечисления
|
||||
(осталось «миграций/стен/виртуальных фрагментов») — мелкая, но корректная
|
||||
правка, не нёсшая отдельного вреда, если бы её не сделали.
|
||||
- **AC6 согласован с §7 построчно** — оба места одинаково называют оба
|
||||
показателя и одинаково описывают случай `moved=0`.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Код реализации — не написан, этап `spec`.
|
||||
- Гейты (`typecheck`/`test`/`build`) — не запускались; для этапа `spec` не
|
||||
требуются (правка не затрагивает класс A/B, только `docs/specs/**`).
|
||||
- Собственно текст `docs/USER-GUIDE.ru.md`/`docs/CANVAS.md` после будущей
|
||||
реализации — они пока не переписаны (AC9 это будущая работа реализации), r2
|
||||
проверял только формулировку самого ТЗ и её согласованность с уже
|
||||
существующим текстом канонических документов.
|
||||
- Все AC, не задетые дельтой (AC1, AC2, AC3, AC5, AC7, AC8, AC9, AC10) —
|
||||
унаследованы из r1 без повторной проверки, ниже.
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Принято без повторной проверки в этом раунде, по документу
|
||||
`docs/reviews/SPEC-REVIEW-223-r1.md` на SHA `c0fff33`:
|
||||
|
||||
- скоуп по `docs/SCOPE.md` (J6) и выбор обычного трека — не изменился, дельта
|
||||
трек-критерии не затрагивает;
|
||||
- обязательные разделы ТЗ по PROCESS.md §7.1 присутствуют и по форме верны;
|
||||
- подтверждённая причина и порядок величин шума (issue, §3 ТЗ) — арифметика
|
||||
`EPS`/`GRID_STEP_N` из `src/align-grid.ts:62-72`, не менялась;
|
||||
- контракт снапа §6, идемпотентность (AC5), исключение проёмов из счётчика,
|
||||
отсутствие вызовов `snapN()` вне `align-grid.ts`, отсутствие роста
|
||||
`moved`/`maxShift*` от near-node замен — код и раздел §6 не входят в дельту
|
||||
этого раунда;
|
||||
- AC1, AC2, AC3, AC5, AC7, AC8, AC9, AC10 — доказательство ни одного из них
|
||||
дельта не задевает (правки только в AC4/AC6/§7/§10/§11/§13);
|
||||
- ссылка issue ↔ ТЗ в `docs/specs/README.md`, release-артефакты §12 — не
|
||||
входят в дельту;
|
||||
- отсутствие продуктовых вопросов владельцу — подтверждено r1, дельта
|
||||
чисто техническая (формулировка отчёта и план реализации), новых
|
||||
продуктовых вопросов не порождает.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Вердикт: жёлтый · цикл r2/4 · High: 0 · Medium: 1 → в задаче
|
||||
|
||||
Находка Medium в скоупе текущего issue (правит ту же i18n-строку
|
||||
`gs.optimize_changes` и тот же §7 ТЗ, что и вся задача) — чинится в этом ТЗ без
|
||||
отдельного issue (владелец, 2026-08-19, #202).
|
||||
Reference in New Issue
Block a user