mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
committed by
Sergey Matyunin
parent
feff436942
commit
0146d5787e
@@ -0,0 +1,234 @@
|
||||
# SPEC-REVIEW-291-r3
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/291
|
||||
- **Этап:** ТЗ на ревью (PROCESS.md §2.4)
|
||||
- **Заход:** r3 · блокирующих циклов израсходовано 2 из 4 до этого вердикта
|
||||
- **Артефакт ТЗ:** `docs/specs/291-lattice-coordinate-write-barrier.md`
|
||||
- **Ветка:** `issue/291-lattice-coordinate-barrier`
|
||||
- **SHA ревью r2:** `23fb41d4` (зафиксирован в `docs/reviews/SPEC-REVIEW-291-r2.md`
|
||||
строкой «SHA этого ревью (r2)» — процессный пробел r1/r2, где SHA не был назван
|
||||
в тексте issue-комментария, на этот раз не повторяется: см. ниже).
|
||||
- **SHA этого ревью (r3):** `4ec43ffd` = текущий `HEAD`. `git merge-base
|
||||
origin/dev HEAD` = `523190d8`, что равно tip `origin/dev` на момент проверки —
|
||||
ребейза не было, полный разбор по §2.10 не требуется.
|
||||
- **Трек:** обычный (не `small`/`trivial`) — без изменений с r1/r2.
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Третий заход. Предмет — дельта `git diff 23fb41d4..HEAD`, один содержательный
|
||||
коммит плюс один инфраструктурный:
|
||||
|
||||
- `4f1ad6fe` «docs: review document for #291» — публикация в репозиторий уже
|
||||
вынесенного документа `docs/reviews/SPEC-REVIEW-291-r2.md`; на текст ТЗ не
|
||||
влияет;
|
||||
- `4ec43ffd` «docs: define lattice report translations» — правка единственной
|
||||
Medium-находки r2 (M3: не названы i18n-ключи per-space отчёта) плюс, сверх
|
||||
требуемого, синхронизация примера CLI-вывода в AC3 с реальным форматом
|
||||
`latticeReport()` (была унаследованная Low-находка L1 из r1/r2).
|
||||
|
||||
`git diff origin/dev...HEAD --stat` — три файла документации (два review-документа
|
||||
плюс сам ТЗ), класс C, кода нет. Полный разбор не требуется ни по признаку
|
||||
ребейза (его не было), ни по смене контракта (AC1–AC6, AC8–AC12 и разделы 1–6,
|
||||
8, 10, 11, 12 дельтой не тронуты — правка только в §7.1 (новая подсекция) и в
|
||||
примере AC3), ни по объёму (дельта — 17 добавленных/1 удалённая строка в файле
|
||||
ТЗ, точечно в месте, которое назвал r2).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Восстановлена цепочка раунда: комментарий-вердикт r2 (жёлтый, M3, документ
|
||||
`SPEC-REVIEW-291-r2.md`) → коммит `4ec43ffd`, закрывающий M3 → отсутствие
|
||||
нового вопроса владельцу (правка чисто техническая, что и требовал сам текст
|
||||
M3: «вопрос технический… снимается автором»).
|
||||
2. `git show 4ec43ffd -- docs/specs/291-lattice-coordinate-write-barrier.md`
|
||||
построчно сопоставлен с текстом находки M3 из r2 (таблица «Закрытие раунда
|
||||
r2» ниже) и отдельно — с описанием L1 (AC3 CLI-пример).
|
||||
3. Прочитан весь текущий файл ТЗ целиком (422 строки), не только диф — та же
|
||||
дисциплина, что в r2, ради регрессии §2.10 (прецедент #102). Нумерация
|
||||
разделов 1–15 с подсекцией 7.1 последовательна, разрывов и повторов нет,
|
||||
внутренних ссылок на номера разделов, кроме заголовков, не найдено.
|
||||
4. Новые ключи `gs.optimize_lattice_summary` / `gs.optimize_lattice_space`
|
||||
сверены построчно: (а) с текущими `src/i18n/en.json`/`ru.json` — оба имени
|
||||
отсутствуют, коллизии с уже существующими ключами (`gs.optimize_changes`,
|
||||
`gs.optimize_glow_migration` и соседние `gs.optimize_detail_*`) нет; (б) с
|
||||
§7 самого ТЗ — placeholders `{n}`/`{far}`/`{cm}` совпадают по смыслу с
|
||||
`latticeCoordinatesCanonicalized`/`latticeCoordinatesFar`/«maximum shift»;
|
||||
(в) с owner-решением §2 п.4 — компактный summary (два числа: canonicalized +
|
||||
max shift) буквально совпадает с двумя placeholders `gs.optimize_lattice_summary`,
|
||||
не больше и не меньше.
|
||||
5. Прочитан существующий код отчёта Optimize (`src/align-grid.ts:56-200`,
|
||||
`src/houseplan-card.ts:15250-15270, 16337-16420`), которого диф не касается,
|
||||
чтобы понять, на какую реальную практику ложится новое решение M3, и не
|
||||
противоречит ли заявление «не требующими отдельной pluralisation» из §7.1
|
||||
действующей конвенции. Найдено: существующий `gs.optimize_changes` и соседние
|
||||
ключи (`count.devices`, `space.delete_blocked`, `import.start`) уже сплошь
|
||||
используют числовой placeholder без грамматической плюрализации — заявление
|
||||
в §7.1 не голословно, оно описывает реальную практику файла.
|
||||
6. Найден и прочитан прецедент округления «физического сдвига» —
|
||||
`src/houseplan-card.ts:15259-15264`: `Math.ceil(maxShiftCm * 10) / 10`,
|
||||
документированная политика «round up, so the promise can never be smaller
|
||||
than the deed» для *обычного* grid-align. Это релевантно новому `{cm}` из
|
||||
§7.1 — см. находку L3 ниже.
|
||||
7. Пример AC3 (`docs/specs/291-*.md:246-249`) сверен буквально с
|
||||
`scripts/model-invariants.mjs:518-525` (`latticeReport()`), включая пробелы
|
||||
и хвост «— ближе 0.0001 шага, но не точно»; `NOISE_STEPS = 1e-4`
|
||||
(`model-invariants.mjs:308`) подтверждает буквальное «0.0001».
|
||||
8. Сверены 10 сопоставимых ТЗ на предмет формата раздела i18n (та же выборка,
|
||||
что в r2, целенаправленно на предмет структуры таблицы «ключ → EN → RU») —
|
||||
`094-universal-state-toggle.md:645-663` остаётся образцом; новый §7.1
|
||||
структурирован тем же способом (таблица ключ/EN/RU плюс условия показа).
|
||||
9. Гейты кода не прогонялись — диф не содержит класса A/B (см. «Чего не
|
||||
проверял»).
|
||||
|
||||
## Закрытие раунда r2
|
||||
|
||||
| Находка r2 | Чем закрыта | Где видно |
|
||||
|---|---|---|
|
||||
| **M3** — раздел/пункт i18n отсутствовал; новые строки per-space отчёта (принятые по Q1) не имели перечисленных ключей en/ru | Добавлена подсекция «7.1 i18n-контракт отчёта»: таблица с двумя ключами (`gs.optimize_lattice_summary`, `gs.optimize_lattice_space`), их EN/RU текстом, условиями показа (`n > 0`; только затронутые пространства) и явным решением не расширять `gs.optimize_changes` | `docs/specs/291-*.md` §7.1, коммит `4ec43ffd` |
|
||||
| **L1** (из r1, унаследована в r2 как открытая) — пример CLI-вывода в AC3 не совпадал с форматом `latticeReport()` | Пример переписан на реальный формат: ` шум у узла 0 (0.00%) — ближе 0.0001 шага, но не точно`, буквально совпадает с шаблонной строкой функции и значением `NOISE_STEPS` | `docs/specs/291-*.md` §9 AC3, коммит `4ec43ffd`; сверено с `scripts/model-invariants.mjs:518-525,308` |
|
||||
| **L2** (из r1) — тело issue #291 называет задачу «Стадия 1 из ADR #282» | Не тронуто в этой дельте | Тело issue #291, первый абзац — без изменений на момент этого ревью |
|
||||
|
||||
M3 — единственная Medium из r2, закрыта по существу (конкретные ключи, EN/RU
|
||||
текст, условия показа, явное разграничение от существующего ключа), а не
|
||||
переформулирована на словах. L1 закрыта тем же коммитом сверх required
|
||||
объёма — не входила в предмет обязательной правки M3, но исправлена заодно;
|
||||
отмечаю как факт, не как невыполненное требование.
|
||||
|
||||
## Находки этого раунда
|
||||
|
||||
### Low
|
||||
|
||||
**L3. Точность (число знаков/округление) нового placeholder `{cm}` в
|
||||
`gs.optimize_lattice_summary` не названа, и очевидный кандидат на переиспользование
|
||||
— существующая политика округления «вверх до 0.1 см» — даёт малоинформативное
|
||||
число именно для той величины, ради которой она вводится.**
|
||||
|
||||
- **Файл:** `docs/specs/291-lattice-coordinate-write-barrier.md` §7.1 (новая
|
||||
подсекция этой дельты).
|
||||
- **Что не так:** тело issue #291 само приводит порядок величины canonicalization
|
||||
shift — «максимальный сдвиг при канонизации 3,3·10⁻⁷ render units — три
|
||||
десятитысячных пикселя», и именно на этом основана owner-формулировка
|
||||
«продуктового риска нет». В кодовой базе уже есть готовый, документированный
|
||||
прецедент округления «максимального сдвига» для *обычного* grid-align:
|
||||
`src/houseplan-card.ts:15264` — `Math.ceil(r.report.maxShiftCm * 10) / 10`,
|
||||
с явным комментарием «round up, so the promise can never be smaller than the
|
||||
deed». Если автор реализации по аналогии применит эту же политику к новому
|
||||
`{cm}` (естественный шаг: имя переменной и физический смысл — оба «maximum
|
||||
shift в см» — совпадают), результат для реальных величин порядка `10⁻⁷` см
|
||||
будет **всегда** `0.1`, независимо от истинного значения: округление вверх
|
||||
превращает любое ненулевое, но исчезающе малое число в фиксированную
|
||||
«единицу видимости». Это не противоречит «round up» инварианту (число не
|
||||
меньше факта), но обесценивает саму причину, по которой в §7/AC7 эта
|
||||
величина выделена отдельной строкой, а не влита в `moved/maxShiftCm`: она
|
||||
должна явно читаться как микроскопическая, а не выглядеть как сопоставимая
|
||||
с реальным видимым перемещением grid-align (тот же порядок «0.1 см»). CLI-путь
|
||||
того же измерения решает эту задачу иначе — `latticeReport()` использует
|
||||
`toExponential(2)` для `worstNoise.steps` (`model-invariants.mjs:531-532`), то
|
||||
есть в проекте уже есть работающий прецедент представления субпиксельных
|
||||
величин без искажающего округления, но §7.1 к нему не апеллирует и вообще не
|
||||
упоминает точность.
|
||||
- **Сценарий, где это заметно:** пользователь открывает Optimize после первой
|
||||
барьерной записи, видит строку «максимальный сдвиг: 0.1 см», не отличимую по
|
||||
виду от строки grid-align с реальным перемещением на миллиметр — и не может
|
||||
понять из текста, что фактическая величина в миллион раз меньше, хотя именно
|
||||
эта разница и была причиной, по которой владелец не увидел здесь риска.
|
||||
- **Почему Low, не Medium:** сама цифра «0.1 см» не искажает решение
|
||||
Confirm/Cancel (величина в любом случае мала и находится в рамках owner-декларации
|
||||
«риска нет»), не блокирует ни один AC (AC7 не требует конкретной точности, только
|
||||
«не занижается») и не меняет контракт поведения — это вопрос читаемости числа,
|
||||
не корректности. Симметрично с прежней калибровкой этого документа: L1 в r1/r2
|
||||
был находкой того же класса (несовпадение текстового примера с форматом) и
|
||||
получил Low, а не Medium.
|
||||
- **Что сделать:** в §7.1 или §15 одной строкой зафиксировать точность/формат
|
||||
`{cm}` (например: «round up до значимой цифры, не до фиксированного знака»,
|
||||
либо явно «использовать `toExponential`/иной формат, отличный от
|
||||
`Math.ceil(x·10)/10` grid-align»), либо явно принять текущее округление как
|
||||
сознательный выбор с оговоркой о его информативности. Правится заодно со
|
||||
следующей правкой документа; не блокирует эту.
|
||||
|
||||
### Low (унаследованная, не блокирует, статус не изменился)
|
||||
|
||||
**L2** (из r1) — тело issue #291 всё ещё называет задачу «Стадия 1 из ADR
|
||||
#282»; расхождение с фактическим содержанием Stage 1 ADR (stable wall ids)
|
||||
остаётся. Не относится к файлу ТЗ, не блокирует.
|
||||
|
||||
## Что проверено и признано корректным
|
||||
|
||||
- **M3 закрыта по существу.** Таблица §7.1 называет оба ключа, их EN/RU текст
|
||||
и условия показа, разграничивает новый счётчик от `gs.optimize_changes` —
|
||||
ровно то, чего требовал §7.1 PROCESS.md («размытое место не додумывается [в
|
||||
коде]»). Формат таблицы соответствует образцу `094-*.md`, на который
|
||||
ссылался сам текст находки.
|
||||
- **Заявление «не требующими отдельной pluralisation» не голословно** — сверено
|
||||
с действующей практикой файла (`gs.optimize_changes` и соседние ключи,
|
||||
п.5 «Как проверялось»); все они используют числовой placeholder без
|
||||
грамматических форм.
|
||||
- **Новые ключи не коллидируют с существующими** и не переопределяют
|
||||
`gs.optimize_changes` — проверено прямым grep по обоим i18n-файлам и по
|
||||
`src/houseplan-card.ts`.
|
||||
- **AC3-пример теперь буквально совпадает с реализацией.** Построчная сверка с
|
||||
`latticeReport()` и `NOISE_STEPS`, включая пробелы перед «—».
|
||||
- **Перенумерация/вставка §7.1 не внесла дефектов.** Документ прочитан целиком
|
||||
(422 строки); последовательность 1–15 с подсекцией 7.1 без разрывов,
|
||||
повторов и висячих ссылок на номера разделов.
|
||||
- **Технический контракт §4–§6, §8, §9 (кроме текста AC3-примера), §10–§12 не
|
||||
затронут этой дельтой** и не противоречит новому §7.1: разделение
|
||||
«canonicalized noise shift» и «moved/maxShiftCm обычного grid alignment»,
|
||||
принятое в §7/AC7 ранее, буквально отражено двумя разными i18n-ключами, а
|
||||
не смешано в один.
|
||||
- **Ветка не отстаёт от `origin/dev`, ребейза не было** — `git merge-base
|
||||
origin/dev HEAD` совпадает с tip `origin/dev`; полный повторный разбор по
|
||||
§2.10 не требуется.
|
||||
|
||||
## Унаследовано из r2 (и, через него, из r1)
|
||||
|
||||
Принято без повторной проверки в этом раунде — документы
|
||||
`docs/reviews/SPEC-REVIEW-291-r1.md` (SHA `8b99df6e`) и
|
||||
`docs/reviews/SPEC-REVIEW-291-r2.md` (SHA `23fb41d4`):
|
||||
|
||||
- Численный контракт §4.1–4.2: `GRID_N = 240`, `1e-4` threshold, формула
|
||||
deviation — сверены в r1 с реализованным `scripts/model-invariants.mjs`;
|
||||
дельта r3 этих разделов не трогала (кроме буквы примера в AC3, сверенной
|
||||
заново в этом раунде).
|
||||
- Разграничение `canonicalizeLatticeCoordinate`/`canonicalizeScalar` и
|
||||
совместимость с nine-decimal contract #224 (§4.2) — не изменено.
|
||||
- Allow-list полей (§5) — сверен в r1 с `modelCoordinates()`
|
||||
(`scripts/model-invariants.mjs:316-358`); не изменено.
|
||||
- Write barrier §6 (frontend/backend, source/AST guard) — не изменено.
|
||||
- AC1–AC6, AC7 (кроме i18n-подсекции и примера AC3), AC8–AC12 — текст не
|
||||
изменён дельтой r3; численные диапазоны, мутанты, perf-бюджет приняты в r1,
|
||||
продуктовые разделы §2–3 приняты в r2.
|
||||
- Разделы «Риски» (§11) и «Откат» (§12) — приняты в r2 (M2), не изменены.
|
||||
- Scope/не-скоуп (§8) — не изменено.
|
||||
- §15 п.1–6 (принятые технические предположения) — приняты в r1/r2 как
|
||||
корректно технические; дельта r3 их не касалась.
|
||||
- Трек/трейлеры (обычный трек, не `small`) — не изменено.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Код — не существует и в этом раунде: `git diff origin/dev...HEAD --stat`
|
||||
показывает только документацию (три файла, класс C). Гейты `typecheck`,
|
||||
`npm test`, `npm run build`, `check-docs.mjs`, `invariants`, смоки, perf —
|
||||
неприменимы, не прогонялись. Причина — отсутствие класса A/B в диффе, а не
|
||||
пропуск.
|
||||
- Полный повторный разбор AC1–AC6, AC8–AC12 целиком и разделов 1–6, 8, 10–12 —
|
||||
дельта их не касалась текстуально (кроме буквы примера AC3, который сверен),
|
||||
приняты по наследству из r1/r2 (раздел выше), а не перепроверены заново
|
||||
построчно.
|
||||
- Существующий i18n-блок `gs.optimize_*` за пределами ключей, релевантных
|
||||
M3/L3 (`gs.optimize_changes` и его непосредственные соседи) — не проверял
|
||||
построчно весь блок; выборка ограничена тем, что нужно для этих находок.
|
||||
- Не связывался с владельцем: находка этого раунда (L3) сформулирована как
|
||||
Low с предложенным решением и не требует продуктового решения (персона,
|
||||
поверхность и объём изменений не затронуты — это вопрос точности отображения
|
||||
числа, технический по критерию §7.1).
|
||||
|
||||
## Итог
|
||||
|
||||
0 High, 0 Medium, 2 Low (L2 унаследована и не тронута; L3 — новая находка этого
|
||||
раунда, оба не блокируют). Единственная Medium-находка r2 (M3) закрыта по
|
||||
существу и подтверждена построчно, не на слово автора; попутно закрыта и
|
||||
унаследованная Low-находка r1 (L1), хотя это не требовалось для перехода в
|
||||
зелёный. Технический контракт, принятый в r1/r2, дельтой не нарушен.
|
||||
|
||||
Без High и без Medium — **зелёный вердикт**. Зелёный вердикт не расходует
|
||||
бюджет §4 (#227): счётчик остаётся 2 из 4.
|
||||
Reference in New Issue
Block a user