Files
houseplan-card/docs/reviews/SPEC-REVIEW-291-r3.md
2026-08-24 17:11:27 +03:00

22 KiB
Raw Permalink Blame History

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.