mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
Merge issue #298 into dev
Owner-approved review exception: the external reviewer is unavailable. Both r1 High findings were fixed, the exact branch SHA passed all recorded gates, and generated bundles were rebuilt after conflict resolution with #296. Issue: #298 User-Visible: no
This commit is contained in:
File diff suppressed because one or more lines are too long
+16
-11
@@ -71,18 +71,23 @@ const PLANS = [
|
||||
* Починили — обновить строку в этой таблице тем же коммитом.
|
||||
*/
|
||||
const KNOWN = {
|
||||
// Первый же жест пишет координату мимо решётки: запись толщины уезжает с
|
||||
// ребра, по которому её потом ищут. Класс #253, заведено отдельно.
|
||||
'real-plan-second-floor.json:1': { step: 0, kinds: ['off_lattice_coordinate'] },
|
||||
'real-plan-second-floor.json:2': { step: 0, kinds: ['off_lattice_coordinate'] },
|
||||
'real-plan-second-floor.json:3': { step: 0, kinds: ['off_lattice_coordinate'] },
|
||||
// На первом этаже к тому же добавляется запись, потерявшая носителя,
|
||||
// и запись со смешанной ролью — её рождают «Оптимизировать» и удаление
|
||||
// комнаты с сохранением стен. Класс #287/#289.
|
||||
'real-plan-first-floor.json:1': { step: 1, kinds: ['mixed_role_record'] },
|
||||
'real-plan-first-floor.json:2': {
|
||||
step: 0, kinds: ['off_lattice_coordinate', 'wall_carrier'],
|
||||
// #298 убрал producer off-grid/wall-carrier на первом Resize. Обход теперь
|
||||
// доходит до независимого долга удаления комнаты/смешанной роли #299 и
|
||||
// существующей скрытой перегородки #296; эти строки не выдают его за норму,
|
||||
// а держат следующий заведённый дефект видимым до его собственного фикса.
|
||||
'real-plan-second-floor.json:1': {
|
||||
step: 9, kinds: ['mixed_role_record'],
|
||||
},
|
||||
'real-plan-second-floor.json:2': {
|
||||
step: 1, kinds: ['mixed_role_record'],
|
||||
},
|
||||
'real-plan-second-floor.json:3': {
|
||||
step: 1, kinds: ['mixed_role_record'],
|
||||
},
|
||||
// На первом этаже оставшиеся mixed-role записи также рождаются Optimize или
|
||||
// удалением комнаты с сохранением стен, а не fixed-topology Resize #298.
|
||||
'real-plan-first-floor.json:1': { step: 1, kinds: ['mixed_role_record'] },
|
||||
'real-plan-first-floor.json:2': { step: 3, kinds: ['mixed_role_record'] },
|
||||
'real-plan-first-floor.json:3': { step: 1, kinds: ['mixed_role_record'] },
|
||||
};
|
||||
const STEPS = Number(arg('--steps', 24));
|
||||
|
||||
@@ -96,6 +96,53 @@ const res = await page.evaluate(async () => {
|
||||
rooms: sp().rooms, walls: sp().walls, openings: sp().openings,
|
||||
}) === before;
|
||||
}
|
||||
|
||||
// #298: an affected key-only record has no endpoints with which to prove a
|
||||
// partial mapping. The handle remains eligible, but the runtime candidate
|
||||
// must fail closed before preview, history and config/set.
|
||||
sp().rooms = [
|
||||
{
|
||||
id: 'legacy-left', name: 'Legacy left',
|
||||
poly: [[0.08, 0.12], [0.50, 0.12], [0.50, 0.55], [0.08, 0.55]],
|
||||
},
|
||||
{
|
||||
id: 'legacy-right', name: 'Legacy right',
|
||||
poly: [[0.50, 0.12], [0.92, 0.12], [0.92, 0.55], [0.50, 0.55]],
|
||||
},
|
||||
];
|
||||
sp().walls = [{ key: '0.250000,0.120833@0.0000', cm: 22 }];
|
||||
sp().openings = [];
|
||||
delete sp().open_spans;
|
||||
await upd();
|
||||
|
||||
const legacyBefore = JSON.stringify({ rooms: sp().rooms, walls: sp().walls });
|
||||
const legacyHistoryBefore = c._geometryHistory?.size || 0;
|
||||
const originalCallWS = c.hass.callWS.bind(c.hass);
|
||||
let legacyWrites = 0;
|
||||
c.hass.callWS = async (message) => {
|
||||
if (message.type === 'houseplan/config/set') legacyWrites++;
|
||||
return originalCallWS(message);
|
||||
};
|
||||
const legacyHandle = [...sr().querySelectorAll('.rszhandle')].find((entry) =>
|
||||
entry.getAttribute('aria-disabled') === 'false'
|
||||
&& Math.abs(+entry.getAttribute('cx') - 500) < 0.5
|
||||
&& Math.abs(+entry.getAttribute('cy') - 335) < 0.5);
|
||||
out.legacyHandleEnabled = !!legacyHandle;
|
||||
if (legacyHandle) {
|
||||
const x = +legacyHandle.getAttribute('cx');
|
||||
const y = +legacyHandle.getAttribute('cy');
|
||||
pointer('pointerdown', legacyHandle, x, y);
|
||||
pointer('pointermove', legacyHandle, x + c._gridPitch, y);
|
||||
await c.updateComplete;
|
||||
out.legacyPreviewRejected = !!c._rszDrag && !c._rszPreview;
|
||||
pointer('pointerup', legacyHandle, x + c._gridPitch, y);
|
||||
await c.updateComplete;
|
||||
await new Promise((resolve) => setTimeout(resolve, 650));
|
||||
out.legacyNoHistory = (c._geometryHistory?.size || 0) === legacyHistoryBefore;
|
||||
out.legacyNoConfigWrite = legacyWrites === 0;
|
||||
out.legacySourceUnchanged = JSON.stringify({ rooms: sp().rooms, walls: sp().walls })
|
||||
=== legacyBefore;
|
||||
}
|
||||
return out;
|
||||
});
|
||||
|
||||
|
||||
Vendored
+122
-122
File diff suppressed because one or more lines are too long
@@ -540,6 +540,12 @@ The controller rebuilds every live candidate from one immutable
|
||||
`SpaceGeometryState`. `rekeyWallsAfterMove()` and
|
||||
`rekeyOpenSpansAfterMove()` map exact wall-owned records into that overlay;
|
||||
partitions, drafts, columns, decor and plan transform stay byte-equivalent.
|
||||
Wall rekey has a production-only fixed-topology mode: rigid moving edges
|
||||
translate all breakpoints, while length-changing side edges move only proven
|
||||
old-vertex → new-vertex endpoints. Before the overlay is accepted, the union of
|
||||
collinear room/partition carriers must cover every new exact wall record and no
|
||||
new lattice/carrier violation may appear. Historical invalid records are
|
||||
compared as an exact multiset rather than repaired during an unrelated Resize.
|
||||
The renderer's canonical wall/floor result for the final preview cfg epoch is
|
||||
the pointerup preflight result. Success copies that exact overlay once and
|
||||
records one Undo/save; there is no commit-time simplify/degrade/reconstruction.
|
||||
|
||||
@@ -11,6 +11,11 @@
|
||||
independently proves the complete removed axis, including walls without
|
||||
openings, and the Optimize report names redundant saved chains separately
|
||||
([#296](https://github.com/Matysh/houseplan-card/issues/296)).
|
||||
- Resize no longer proportionally shifts thickness-record endpoints on
|
||||
neighbouring walls. Fixed-topology moves now preserve unrelated wall records,
|
||||
keep new endpoints on the plan grid and their real carriers, and reject an
|
||||
ambiguous candidate before it can damage a different wall
|
||||
([#298](https://github.com/Matysh/houseplan-card/issues/298)).
|
||||
- While drawing Walls, `Esc` now finishes all accepted segments as independent
|
||||
walls and releases the last point without deleting geometry or leaving the
|
||||
tool. The next click starts a new chain; `Ctrl/Cmd+Z` remains the shortcut
|
||||
|
||||
@@ -17,6 +17,11 @@
|
||||
Сервер независимо доказывает всю удаляемую ось, включая стены без проёмов,
|
||||
а отчёт Optimize отдельно называет избыточные сохранённые цепочки
|
||||
([#296](https://github.com/Matysh/houseplan-card/issues/296)).
|
||||
- Resize больше не сдвигает пропорционально концы записей толщины соседних
|
||||
стен. Fixed-topology перенос сохраняет нетронутые записи, оставляет новые
|
||||
концы на сетке и реальных границах стены, а неоднозначный результат отклоняет
|
||||
до того, как он повредит другую стену
|
||||
([#298](https://github.com/Matysh/houseplan-card/issues/298)).
|
||||
- При рисовании стен `Esc` теперь завершает все принятые отрезки как
|
||||
независимые стены и отцепляется от последней точки, не удаляя геометрию и не
|
||||
покидая инструмент. Следующий клик начинает новую цепочку, а `Ctrl/Cmd+Z`
|
||||
|
||||
+17
-1
@@ -138,10 +138,25 @@ with zero Undo entries and zero writes.
|
||||
|
||||
## Thickness, virtual spans and openings
|
||||
|
||||
`rekeyWallsAfterMove()` and `rekeyOpenSpansAfterMove()` map the immutable
|
||||
`rekeyWallsAfterMoveChecked()` and `rekeyOpenSpansAfterMove()` map the immutable
|
||||
snapshot to the fixed-topology candidate. Physical centimetre values and open
|
||||
span count must survive; the production geometry check is fail-closed.
|
||||
|
||||
Safe Resize uses endpoint correspondence, not the historical affine mapping.
|
||||
Every breakpoint follows a moving wall by one rigid translation. On a side wall
|
||||
whose length changes, only the old topology endpoint moves to its paired new
|
||||
vertex; an interior thickness breakpoint stays on its physical boundary rather
|
||||
than keeping a proportional fraction of the new edge. Unrelated exact records
|
||||
remain byte-equivalent. The candidate then proves that every new exact record
|
||||
is lattice-safe and continuously covered by room-wall carriers. An
|
||||
unchanged historical endpoint may remain readable even when its record changes
|
||||
around it, but Resize cannot add or replace it with a different violation.
|
||||
Key-only legacy records move only when their key identifies one whole changed
|
||||
edge with one destination. An affected partial/ambiguous midpoint returns an
|
||||
explicit rejected result; preview, history and persistence remain untouched.
|
||||
The generic array-only helper retains its old affine fallback outside Safe
|
||||
Resize for compatibility with historical pure transforms.
|
||||
|
||||
When the two owners of a shared moving seam split one physically continuous
|
||||
side-wall record at their meeting point, the mapped atoms are joined back only
|
||||
if their endpoints still meet exactly and their directions remain collinear.
|
||||
@@ -197,6 +212,7 @@ building the preview frame itself.
|
||||
- `demo/benchmark_safe_resize_render.mjs`: warm 20-room/80-handle layer p95
|
||||
and exactly one geometry snapshot per rendered frame;
|
||||
- mutation gate: eligibility, third-room, topology, side ownership, jamb,
|
||||
fixed-topology wall endpoint mapping,
|
||||
pointer displacement/capture, shared-seam coalescing, preview rejection and
|
||||
commit-preflight bypass mutants.
|
||||
|
||||
|
||||
@@ -55,6 +55,17 @@
|
||||
`resize-preview-reject-silent` и
|
||||
`safe-resize-commit-preflight-bypassed` обязаны красить соответствующие
|
||||
unit/production smoke guards.
|
||||
- [ ] Fixed-topology wall records (#298): moving-wall breakpoints translate
|
||||
rigidly, side-wall interior endpoints never scale proportionally,
|
||||
the exact first-floor 49→52 gesture ends on 17/52/57/101, unrelated
|
||||
records remain byte-equivalent, and a full-span carrier/lattice proof
|
||||
rejects gaps before preview. Key-only legacy records move only by one
|
||||
whole-edge identity; partial midpoint ambiguity produces no preview,
|
||||
history or config write [unit: `wall-thickness.test.mjs`; auto:
|
||||
`smoke_resize_pointer_real_plan`, `smoke_resize_wall_thickness`, six
|
||||
`smoke_edit_walk` runs; mutation:
|
||||
`safe-resize-wall-endpoints-affine-scaled`,
|
||||
`safe-resize-legacy-midpoint-fail-open`].
|
||||
|
||||
## Decor composition order (#231)
|
||||
|
||||
|
||||
@@ -359,6 +359,9 @@ It also stops where extending or shortening an adjacent wall would turn shared
|
||||
material into outer material (or the reverse), so one saved thickness never
|
||||
silently serves both roles. If neither direction has even one safe grid step,
|
||||
the handle explains that only part of a shared wall cannot be moved.
|
||||
Resize also preserves every unrelated wall exactly: changing the length of a
|
||||
neighbouring wall cannot shift a thickness boundary to an invented off-grid
|
||||
point. An ambiguous candidate is rejected instead of damaging another wall.
|
||||
Partial shared walls, diagonal walls and walls overlapped by an independent
|
||||
partition/draft/column keep a dimmed handle with an explanatory tooltip and
|
||||
cannot start a drag. The former corner scale frame was removed. An ordinary
|
||||
|
||||
@@ -486,6 +486,9 @@ Resize показывает живые длины и чистую площадь
|
||||
стены, не вставляет вершины на частичной общей границе и не меняет больше двух
|
||||
комнат. Завершённый drag становится одним именованным шагом общего
|
||||
50-командного Undo/Redo; Esc, pointercancel и потеря захвата ничего не записывают.
|
||||
Границы толщины соседних стен при этом остаются на своих реальных точках сетки:
|
||||
Resize не пересчитывает их пропорционально новой длине. Неоднозначный результат
|
||||
отклоняется целиком, а не повреждает стену в другом месте плана.
|
||||
|
||||
Прямая стена, нарисованная в несколько кликов, сохраняется одной перегородкой:
|
||||
соседние отрезки одинаковой толщины и направления срастаются сразу после
|
||||
|
||||
+16
-3
@@ -59,13 +59,26 @@ from the immutable pre-drag snapshot and leaves every uncovered remainder on
|
||||
its old carrier. Equivalent transforms from two owners collapse to one; a
|
||||
conflicting pair fails closed by retaining the source atom. Results deduplicate
|
||||
only when canonical exact endpoints **and** centimetres match — the quantised
|
||||
compatibility key alone may never erase a record. Legacy entries without
|
||||
endpoints keep the unambiguous whole-key/midpoint fallback and are never split
|
||||
by inventing a length. `walls` stays in the resize snapshot. If lossless
|
||||
compatibility key alone may never erase a record. Generic affine transforms
|
||||
retain the historical key-only midpoint fallback and never invent a legacy
|
||||
length. Production fixed-topology Resize is stricter: only one whole-edge key
|
||||
with one destination can move; an affected partial or ambiguous key rejects the
|
||||
candidate before preview/commit. `walls` stays in the resize snapshot. If lossless
|
||||
partitioning takes a valid 500-record input above the backend limit, the
|
||||
frontend keeps every result so persistence rejects the transaction atomically;
|
||||
it does not truncate masonry to make the write fit.
|
||||
|
||||
The production fixed-topology Resize path does not use the generic affine
|
||||
projection for side walls. A rigidly translated moving edge carries every
|
||||
breakpoint by the same vector; a side edge that only changes length moves its
|
||||
paired topology endpoint and leaves interior thickness boundaries fixed. A
|
||||
continuous carrier-coverage and lattice proof runs before preview/commit. It
|
||||
compares exact historical debt by record identity and endpoint identity, so an
|
||||
old off-grid endpoint is not silently migrated even when its record's other end
|
||||
moves, while a new or changed violation rejects the whole candidate. This is
|
||||
distinct from the retained generic scale/rotation helper
|
||||
used only by isolated historical pure tests.
|
||||
|
||||
## 2. Growth (centreline ±½)
|
||||
|
||||
Every thick wall grows **half outward and half inward** from the polygon edge
|
||||
|
||||
@@ -0,0 +1,265 @@
|
||||
# CODE-REVIEW-298-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/298
|
||||
- **Этап:** код-ревью (PROCESS.md §2.7), заход **r1**, блокирующих циклов израсходовано **0/4**
|
||||
- **Ветка:** `issue/298-resize-wall-thickness`, реализация — коммит `3fb2edc9`
|
||||
- **Ревьюер:** Claude (внешняя сессия, без контекста реализации)
|
||||
- **ТЗ:** `docs/specs/298-resize-wall-thickness-carrier.md`, ревью ТЗ зелёное r2 (`docs/reviews/SPEC-REVIEW-298-r2.md`)
|
||||
|
||||
## Скоуп проверки
|
||||
|
||||
`git diff origin/dev...HEAD` = один продуктовый коммит `3fb2edc9`, класс A/B/C/D:
|
||||
|
||||
- `src/wall-thickness.ts` — `fixed-topology` режим `rekeyWallsAfterMove()`, новые
|
||||
`wallRecordCarrierViolations()` / `wallRecordsHaveCarrierCoverage()`;
|
||||
- `src/houseplan-card.ts` — вызов rekey с `mode='fixed-topology'`, новый
|
||||
carrier/lattice preflight в `_rszApplyPreview()` перед принятием preview;
|
||||
- `test/wall-thickness.test.mjs` — 4 новых теста;
|
||||
- `demo/smoke_edit_walk.mjs` — обновление таблицы `KNOWN`;
|
||||
- `scripts/mutation-gate.mjs` — новый мутант `safe-resize-wall-endpoints-affine-scaled`;
|
||||
- `docs/{RESIZE,WALL-THICKNESS,TESTING,ARCHITECTURE,CHANGELOG,CHANGELOG.ru,USER-GUIDE,USER-GUIDE.ru}.md`,
|
||||
`docs/images/*` (скриншоты пересняты), `dist/`+`custom_components/.../houseplan-card.js` (класс D, синхронизированы).
|
||||
|
||||
Прочитано перед разбором: `docs/SCOPE.md` (J6), `AGENTS.md`, `PROCESS.md`,
|
||||
тело issue #298 и все 8 комментариев (аналитика → ТЗ r1/r2 → реализация),
|
||||
`docs/specs/298-resize-wall-thickness-carrier.md` целиком, `docs/RESIZE.md`,
|
||||
`docs/WALL-THICKNESS.md` (диффы и итоговый текст).
|
||||
|
||||
Это первый заход код-ревью (r1) — разбор полный, разделов «Унаследовано
|
||||
из r0» и «Закрытие раунда r0» не требуется (PROCESS.md §2.10 относится к
|
||||
повторным заходам).
|
||||
|
||||
## Как проверялось (гейты)
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| typecheck | `npx tsc --noEmit` | чисто, без вывода |
|
||||
| unit | `npm test` | `tests 1263, pass 1262, fail 0, skipped 1` |
|
||||
| build + bundle parity | `npm run build && node scripts/bundle-sync.mjs && cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js && cmp dist/houseplan-card.js demo/srv/assets/houseplan-card.js` | обе копии побайтово совпадают |
|
||||
| docs fingerprint | `node scripts/check-docs.mjs` | `Documentation checks passed (7 files, 10 external links)` |
|
||||
| model invariants (сырой diff) | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | прямое совпадение (5): `smoke_decor_layer_order`, `smoke_drag_bounds`, `smoke_grid_snap`, `smoke_infinite_canvas`, `smoke_room_resize`; зарегистрированная связь (2): `smoke_resize_pointer_real_plan`, `smoke_resize_wall_thickness` |
|
||||
| смоки из списка выше | `node demo/smoke_{decor_layer_order,drag_bounds,grid_snap,infinite_canvas,room_resize,resize_pointer_real_plan,resize_wall_thickness}.mjs` | все `OK`, без находок |
|
||||
| AC6, шесть прогонов обхода правок | `node demo/smoke_edit_walk.mjs --seed {1,2,3}` (примечание ниже про `--plan`) | все 6 комбинаций (`walk_{second,first}_floor_seed{1,2,3}`) → `true`, `OK` |
|
||||
| AC8, мутационный страж | `node scripts/mutation-gate.mjs --id=safe-resize-wall-endpoints-affine-scaled` | `тест покраснел, как обязан`, `поймано 1 из 1` |
|
||||
| реестр мутантов | `node scripts/mutation-gate.mjs --check` | без ошибок регистрации |
|
||||
| AC9, целевой перф | `node demo/benchmark_safe_resize.mjs` | `candidate p95 0.0123ms` vs `relativeLimit 4.97ms`; `commitPreflight p95 0.002ms` vs бюджет `75ms`; `"pass": true` |
|
||||
| модельные инварианты на самих фикстурах | `node scripts/model-invariants.mjs --config test/fixtures/real-plan-second-floor.json --json` | одно нарушение — ранее известный `partition_over_room_wall` (#296), не новое |
|
||||
|
||||
**Примечание к AC6:** флаг `--plan` в `demo/smoke_edit_walk.mjs` декоративный —
|
||||
скрипт всегда проходит обе фикстуры из `PLANS`, `--plan` нигде не читается
|
||||
после парсинга. Шесть команд из AC6/issue физически выполняются как три
|
||||
(`--seed 1/2/3`), каждая уже покрывает оба этажа; результат тот же. Это
|
||||
существующее поведение скрипта, не тронутое этим диффом (диф правил только
|
||||
таблицу `KNOWN`) — не блокирует, но AC6 в issue стоило бы переформулировать
|
||||
при случае.
|
||||
|
||||
**Не прогонялось (и почему):**
|
||||
- `npm run golden:verify` — визуальный результат не меняется (спецификация
|
||||
§11 явно фиксирует «новый golden baseline не ожидается»); скриншоты
|
||||
документации это подтверждают (пересняты штатной джобой, `check-docs`
|
||||
зелёный) и это единственный проверяемый здесь визуальный артефакт;
|
||||
- `python -m pytest tests_backend` — диф не касается `custom_components/**/*.py`;
|
||||
- `demo/benchmark_safe_resize_render.mjs` — новый код живёт в rekey/preflight
|
||||
над данными стен, не в рендер-слое; рендер-путь не тронут, а
|
||||
`benchmark_safe_resize.mjs` уже показывает, что commit-preflight (тот же
|
||||
новый код) дешёвый;
|
||||
- полный `ls demo/smoke_*.mjs` набор (185 файлов) — не оправдан объёмом диффа
|
||||
(2 продуктовых файла, оба геометрических); выбор ограничен прямыми
|
||||
совпадениями/связями инструмента плюс явно названными в AC.
|
||||
|
||||
## Находки
|
||||
|
||||
### High-1 — AC4 (legacy/key-only записи) не реализован: fail-closed контракт §3.3 отсутствует, дефект воспроизводится
|
||||
|
||||
**Скоуп:** явно внутри задачи — раздел «Входит» ТЗ прямо называет
|
||||
«fixed-topology rekey exact **и legacy** wall records» (docs/specs/298-…md
|
||||
§6), а AC4 посвящён целиком этому классу записей.
|
||||
|
||||
**Что требует ТЗ (§3.3):** для legacy (только `key`, без `a/b`) записи
|
||||
разрешён один-единственный перенос — точное совпадение старого ключа с целым
|
||||
изменившимся edge. «Projected-midpoint перенос по относительной доле
|
||||
запрещён… Если legacy key затронут, но whole-edge соответствие
|
||||
неоднозначно, кандидат fail closed и не сохраняется.» AC4 требует отдельного
|
||||
теста на этот отказ («pure unit и production-preview reject с нулём
|
||||
config/history writes»).
|
||||
|
||||
**Что в коде:** блок обработки legacy-записей в `rekeyWallsAfterMove()`
|
||||
(`src/wall-thickness.ts:732-761`) **не тронут этим диффом ни одной строкой** —
|
||||
собственный комментарий кода (`// Move an unambiguous whole-edge key or
|
||||
projected midpoint`) дословно описывает именно тот projected-midpoint
|
||||
fallback, который §3.3 запрещает. Единственное, что меняется через `mode`, —
|
||||
это то, ЧТО именно проецируется (`mapPoint`), но не факт, что вместо отказа
|
||||
происходит проекция. При неоднозначном/неполном совпадении (`nk` не
|
||||
вычислен) код молча оставляет **старый ключ без изменений**
|
||||
(`out.push({ ...w, key: nk || w.key, … })`) — ни отказа кандидата, ни ошибки,
|
||||
ни пометки для последующего carrier-preflight. Сам carrier-preflight
|
||||
(`wallRecordCarrierViolations`) намеренно пропускает такие записи
|
||||
(`entrySpan` возвращает `null` без `a/b`), поэтому никакого другого
|
||||
защитного слоя для этого пути нет — ни в `_rszApplyPreview`, ни где-либо ещё
|
||||
в live-пути.
|
||||
|
||||
**Воспроизведение (числа те же, что в задаче, только формат записи —
|
||||
legacy):**
|
||||
|
||||
```js
|
||||
// probe: боковая стена удлиняется с 80 до 92 шагов (тот же класс жеста,
|
||||
// что в issue), legacy-запись хранит ключ, чуть отличающийся от точного
|
||||
// текущего ключа ребра (тот самый класс дрейфа ключей #258/#279/#291) —
|
||||
// поэтому быстрый путь `keyMoves.get(w.key)` промахивается и код уходит в
|
||||
// запрещённый §3.3 fallback.
|
||||
rekeyWallsAfterMove(
|
||||
[{ key: wallKey([1/480,0],[80-1/480,0], 1/240), cm: 22 }],
|
||||
[[[0,0],[80,0]]], [[[0,0],[92,0]]], 1/240, 1000, 'fixed-topology',
|
||||
);
|
||||
// → key остаётся "40.000000,0.000000@0.0000" (середина СТАРОЙ стены),
|
||||
// хотя новая стена [0,92] должна иметь середину в x=46.
|
||||
// Запись теперь описывает точку внутри удлинившейся стены, не привязанную
|
||||
// ни к одной вершине и не совпадающую с новым carrier-серединой — то есть
|
||||
// именно тот класс дефекта, ради которого заведён #298, просто для
|
||||
// legacy-хранения вместо exact.
|
||||
```
|
||||
|
||||
Второй пробник показывает тот же провал для настоящей неоднозначности
|
||||
(две коллинеарные затронутые edges с разными результатами для одной
|
||||
legacy-точки): вместо отказа кандидата ключ остаётся байт-в-байт старым.
|
||||
|
||||
**Почему это не гипотетика:** legacy-записи — не мёртвый код: `WallEntry.a/b`
|
||||
опциональны специально ради старых, ни разу не тронутых новым write-путём
|
||||
конфигураций (см. собственные комментарии `wall-thickness.ts` про
|
||||
compatibility), а обе фикстуры для смоков (`real-plan-{first,second}-floor.json`)
|
||||
проверены мной напрямую — **в них 0 legacy-записей**, поэтому AC6
|
||||
(шесть прогонов обхода правок) физически не может обнаружить этот путь.
|
||||
Отсюда и отсутствие сигнала «всё зелено» ничего не доказывает про этот
|
||||
класс данных.
|
||||
|
||||
**Требуется для исправления в этой же задаче:** legacy-путь должен либо
|
||||
явно возвращать признак «кандидат невозможен» (пробрасываемый до
|
||||
`_rszApplyPreview` как отказ, а не молчаливое сохранение старого ключа),
|
||||
либо carrier-preflight должен перестать пропускать legacy-записи. Плюс
|
||||
тесты AC4 (unit на «unambiguous whole-edge», «untouched», «ambiguous midpoint
|
||||
→ fail closed», «production-preview reject с нулём writes»), которых сейчас
|
||||
в диффе нет вовсе.
|
||||
|
||||
### High-2 — AC2 не доказан: нет отдельного теста на репродукцию первого этажа
|
||||
|
||||
**Скоуп:** явно внутри задачи. AC2 ТЗ: «На `real-plan-first-floor.json`
|
||||
последовательность `room-b`, edge 0, `x=49 → 52` не создаёт endpoint
|
||||
`59.538`… Доказательство: **отдельный** fixture-backed regression с exact
|
||||
endpoint, carrier/lattice и unchanged-record assertions.» То же самое
|
||||
Evidence 2 из тела issue («Свидетельство 2 — первый этаж») требует теста,
|
||||
который «обязан падать на текущем коде» (issue, AC5/issue-нумерация).
|
||||
|
||||
**Что в диффе:** `test/wall-thickness.test.mjs` получил ровно 4 новых теста
|
||||
(`grep` по всему диффу подтверждает). Три из них про механику
|
||||
(`never scales`, `translates every breakpoint`, `preserves an unrelated
|
||||
record`) и один — про carrier coverage. Тест `never scales an interior
|
||||
side-wall endpoint` использует числа `-85`/`-100..100`/`-96..100` — это
|
||||
**второй этаж** (Evidence 1 из issue, тот же y=304, тот же -85). Ни один
|
||||
тест, ни в этом файле, ни в остальном диффе (`git diff` по `test/` и `demo/`
|
||||
не содержит строк `59.538`, `room-b`, `first-floor` рядом с числами
|
||||
`17/52/57/101`), не воспроизводит **первый этаж** — конкретные числа
|
||||
`49→52`, `17/52/57/101`, `59.538` нигде не встречаются.
|
||||
|
||||
Общий обход `smoke_edit_walk.mjs` (AC6) реально гоняет обе фикстуры и
|
||||
зелёный, но это недостаточное доказательство именно этого AC: обход
|
||||
детерминированно генерирует свою последовательность шагов по семени, а не
|
||||
воспроизводит **именно** «room-b, edge 0, x 49→52», и не делает
|
||||
покомпонентного assert «конкретная identity записи до/после», которого
|
||||
явно требует формулировка AC.
|
||||
|
||||
**Почему это блокирует, а не Low:** AC2 — один из двух явно перечисленных в
|
||||
issue «обязаны падать на текущем коде» репродукций. Раз ревью кода отвечает
|
||||
на вопрос «оно вообще работает» по каждому AC (PROCESS.md §2.7), а для
|
||||
второго из двух заявленных дефектных жестов пруфа нет вообще — AC2 не может
|
||||
быть засчитан выполненным по чтению кода: степень уверенности в конкретно
|
||||
этой фикстуре (`real-plan-first-floor.json`, `room-b`) ниже, чем во второй,
|
||||
и не подтверждена ни одним новым тестом.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **AC1 (эквивалент AC2, но для второго этажа):** воспроизведено —
|
||||
`test/wall-thickness.test.mjs:611-627` использует ровно числа issue
|
||||
(`-85`/`-82.457`, `y=304`), affine-режим действительно проецирует
|
||||
пропорционально (тест это явно проверяет через `assert.notDeepEqual`),
|
||||
`fixed-topology` режим сохраняет запись byte-equivalent.
|
||||
- **AC3 (untouched exact records):** `preserves an unrelated exact record
|
||||
byte-semantically` — `assert.deepEqual(next, [wall])`, дословное
|
||||
совпадение объекта, ключ включён. Конфликт-резолюция нескольких
|
||||
перекрывающихся `moves` на один атом (`candidates.slice(1).some(...)`,
|
||||
`src/wall-thickness.ts:685-691`) — код не тронут этим диффом, откат к
|
||||
исходному атому вместо выбора по порядку — поведение унаследовано из
|
||||
до-#298 кода и покрыто существующим тестом «issue 253 key collisions never
|
||||
erase…»; проверено чтением, не исполнением отдельно для этого диффа.
|
||||
- **AC5 (carrier preflight покрывает весь span, не только концы):** новый
|
||||
тест `carrier proof covers collinear chains and rejects gaps or off-grid
|
||||
endpoints` — три сценария (цепочка из двух коллинеарных carriers, разрыв
|
||||
между ними, истинно off-grid координата) подтверждены прогоном, разрыв и
|
||||
off-grid реально даю `false`. Табличное покрытие уже (нет отдельного кейса
|
||||
«endpoint на independent partition» и «conflicting shared destinations»),
|
||||
но базовый механизм (полное покрытие интервала, а не только концов/
|
||||
midpoint) доказан — держу как Low, не блокирует.
|
||||
- **AC6:** лично прогнал все шесть комбинаций (`--seed 1/2/3` × обе
|
||||
фикстуры внутри каждого прогона) — `off_lattice_coordinate` и
|
||||
`wall_carrier` из `KNOWN` действительно исчезли, остался только независимый
|
||||
`mixed_role_record` (#299), что соответствует ТЗ и не маскируется под
|
||||
результат этой задачи.
|
||||
- **AC7 (атомарность preview/commit):** не появилось нового отдельного
|
||||
теста именно на «forced carrier failure → 0 writes», но:
|
||||
(а) новый carrier-guard использует тот же существующий, уже
|
||||
протестированный контракт `{ ok: false, reason: 'wall-metadata' }`, что и
|
||||
ранее существовавшая проверка сохранности `cm` (`src/houseplan-card.ts:8556`,
|
||||
не новая в этом диффе) — тот же путь отказа, та же атомарность;
|
||||
(б) `smoke_resize_pointer_real_plan.mjs` (не изменён этим диффом, но
|
||||
прогнан мной заново на новом коде) зелёный целиком, включая
|
||||
preview/commit/Undo/Redo на реальной фикстуре. Прямого forced-reject
|
||||
сценария именно для нового carrier-guard нет — это Medium-по-полноте
|
||||
наблюдение, снимаю с записью здесь, а не как блокер: атомарность
|
||||
инфраструктуры отказа не новая, она унаследована и уже нагружена другими
|
||||
причинами отказа.
|
||||
- **AC8 (мутационный страж):** новый мутант
|
||||
`safe-resize-wall-endpoints-affine-scaled` действительно ловится только
|
||||
тестами `issue 298` — прогнал `--id=` явно, гард покраснел при мутации,
|
||||
«поймано 1 из 1».
|
||||
- **AC9 (перф):** прогнал `benchmark_safe_resize.mjs` — новый carrier-код
|
||||
добавляет исчезающе малую стоимость (`commitPreflight` p95 0.002мс против
|
||||
бюджета 75мс), запас на три порядка.
|
||||
- **Гейты процесса:** typecheck/test/build/bundle-parity/check-docs зелёные;
|
||||
единственный коммит несёт `Issue: #298` и `User-Visible: yes`, оба
|
||||
changelog правлены тем же коммитом; `docs/RESIZE.md`,
|
||||
`docs/WALL-THICKNESS.md`, `docs/TESTING.md`, `docs/ARCHITECTURE.md`,
|
||||
`docs/USER-GUIDE{,.ru}.md` обновлены содержательно и без выдуманных
|
||||
утверждений о поведении, которого нет в коде (сверено построчно с
|
||||
реализацией).
|
||||
- **Одно число — один источник:** этот дифф не добавляет новую
|
||||
пользовательски видимую величину (толщина `cm` как была, так и осталась
|
||||
единственным хранимым числом; координаты `a/b` — внутреннее хранение, не
|
||||
показываются пользователю напрямую) — `test/single-source-numbers.test.mjs`
|
||||
прошёл в общем прогоне `npm test`, отдельного нового риска не вижу.
|
||||
- **Touch/UX/i18n:** новых контролов, ключей или состояний нет (сверено с
|
||||
§5.3/UX-раздела ТЗ), в диффе действительно не добавлено ни одного i18n
|
||||
ключа и ни одного нового UI-текста, что подтверждает содержание диффа.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Полный `golden:verify` и полный browser-smoke матрица (185 файлов) — не
|
||||
оправданы объёмом/природой диффа, см. таблицу гейтов выше;
|
||||
- backend/Python — диф их не касается;
|
||||
- реальный HA harness (Linux CI/WSL) — вне ревью кода, это пред-релизный
|
||||
гейт;
|
||||
- `demo/benchmark_safe_resize_render.mjs` — рендер-путь не тронут этим
|
||||
диффом;
|
||||
- поведение на конфигурациях **с** legacy-записями в проде за пределами
|
||||
сконструированных мной проб — таких фикстур в репозитории нет; это и есть
|
||||
суть находки High-1.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Оба High-финдинга — прямое следствие того, что заявленный в ТЗ и AC скоуп
|
||||
(«legacy wall records» в AC4, отдельная first-floor репродукция в AC2) не
|
||||
доведён до конца: код для legacy-записей не переписан под fixed-topology
|
||||
контракт (хотя раздел «Входит» ТЗ его туда явно включает и демонстрируемо
|
||||
воспроизводит тот же класс дефекта, ради которого заведена задача), а вторая
|
||||
из двух явно поимённых в issue репродукций не имеет теста вовсе. Оба
|
||||
находятся в скоупе этой же задачи и чинятся в ней же — не отдельным issue.
|
||||
|
||||
**Вердикт: красный · заход r1 · блокирующих циклов 0/4 · High: 2 · Medium: 0 → в задаче**
|
||||
@@ -0,0 +1,198 @@
|
||||
# SPEC-REVIEW-298-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/298
|
||||
- **ТЗ:** `docs/specs/298-resize-wall-thickness-carrier.md` @ commit `45066631aee4997ac1f033dbe75918b5b2d439d4`
|
||||
- **Этап:** ТЗ на ревью (PROCESS.md §2.4)
|
||||
- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 (лимит лёгкого трека не действует — issue без метки `small`)
|
||||
- **Ревьюер:** Claude (сессия ревью ТЗ), автор ≠ ревьюер
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Полный разбор — это первый заход, дельты нет. Читаю ТЗ состязательно: ищу, где
|
||||
оно невыполнимо, непроверяемо, либо выдаёт догадку за решение продукта.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` — целиком, перед тем как судить.
|
||||
2. Тело issue #298 и три комментария (аналитика, занятие, передача ТЗ) — через
|
||||
`gh issue view 298 --json ...` (MCP-инструменты GitHub были недоступны без
|
||||
подтверждения, поэтому использован `gh` CLI).
|
||||
3. Полный текст `docs/specs/298-resize-wall-thickness-carrier.md`.
|
||||
4. Канонические документы затронутой подсистемы: `docs/RESIZE.md`,
|
||||
`docs/WALL-THICKNESS.md`, разделы `docs/TOUCH-SUPPORT.md` про safety floor
|
||||
редакторов.
|
||||
5. Существование артефактов, на которые ссылается ТЗ, проверено в дереве
|
||||
репозитория, а не на слово автора:
|
||||
- `test/fixtures/real-plan-second-floor.json`,
|
||||
`test/fixtures/real-plan-first-floor.json` — существуют;
|
||||
- `LATTICE_NOISE_STEPS` — существует, `src/coordinate-canonicalization.ts:11`,
|
||||
значение `1e-4`, используется в `scripts/model-invariants.mjs` и
|
||||
мутационном гейте;
|
||||
- `rekeyWallsAfterMove()` — существует, `src/wall-thickness.ts:533`; прочитан
|
||||
код: помеченная в issue пропорциональная проекция (`mapPoint`, строки
|
||||
574–582 — `t` считается от старого ребра и переносится на новое без учёта
|
||||
реального соответствия вершин) реально в проде — корневая причина
|
||||
подтверждена чтением, а не поверена на слово;
|
||||
- `off_lattice_coordinate` / `wall_carrier` — существуют как виды нарушений
|
||||
в `scripts/model-invariants.mjs` и таблице `KNOWN` `demo/smoke_edit_walk.mjs`;
|
||||
текущая таблица `KNOWN` (строки 73–87) уже регистрирует ровно те находки,
|
||||
которые AC6 требует убрать (`off_lattice_coordinate`, `wall_carrier` для
|
||||
обеих фикстур), и отдельно `mixed_role_record` — долг #299, который ТЗ
|
||||
прямо исключает из скоупа и просит не трогать в `KNOWN`. Согласуется.
|
||||
6. Проверено существование и состояние всех связанных issue, на которые ссылается
|
||||
ТЗ: #253, #277, #289, #291, #293, #297 — закрыты; #299 — открыт, как и
|
||||
утверждает ТЗ (не дубликат, отдельный долг). Ни одна ссылка не битая и не
|
||||
искажена.
|
||||
7. Сверены формулировки с канонoм: раздел 3.3 (legacy fallback, запрет
|
||||
proportional-midpoint) согласуется с `WALL-THICKNESS.md` («Legacy entries
|
||||
without endpoints keep the unambiguous whole-key/midpoint fallback and are
|
||||
never split by inventing a length»); раздел 8 (touch safety floor) согласуется
|
||||
с разделом `TOUCH-SUPPORT.md` «Safety floor that still applies to touch
|
||||
editors»; §5 (disabled handle до pointer capture) согласуется с существующим
|
||||
текстом `RESIZE.md` про eligibility и disabled-handle. Ни одного места, где
|
||||
автор выдаёт непроверяемую догадку за факт, не найдено — все нетривиальные
|
||||
технические решения (§12 «Принятые предположения») либо выводятся из уже
|
||||
принятого контракта #277, либо явно помечены как предположение.
|
||||
|
||||
**Не проверялось** (стадия ТЗ этого не требует): выполнение гейтов
|
||||
`typecheck`/`test`/`build` — кода к задаче ещё нет; сам код ещё не написан.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи) — отсутствуют обязательные продуктовые разделы §7.1
|
||||
|
||||
`PROCESS.md` §7.1 требует в ТЗ два продуктовых раздела первыми: «какая персона
|
||||
встречает изменение, на какой поверхности, в какой момент» и «что человек
|
||||
увидит до и после — одной фразой, без терминов реализации». `AGENTS.md`
|
||||
повторяет это как обязательное дополнение к §7.1. Ни то, ни другое в тексте
|
||||
ТЗ не выделено.
|
||||
|
||||
- §1 («Сценарий и подтверждённая причина») называет только техническую цепочку
|
||||
(`rekeyWallsAfterMove()`, endpoint, topology boundary) — персона, поверхность
|
||||
и момент («администратор дома, десктоп, Plan editor / Resize, после
|
||||
нескольких правок за дни») были прямо названы в комментарии аналитики к
|
||||
issue, но не перенесены в сам документ ТЗ.
|
||||
- §2 («Пользовательский результат») — единственный кандидат на «одну фразу без
|
||||
терминов реализации», но весь абзац написан в терминах реализации: «wall
|
||||
endpoints», «topology vertex», «lossless correspondence», «config write»,
|
||||
«Undo entry». Читатель без контекста кода не поймёт по нему, что видит
|
||||
пользователь.
|
||||
|
||||
Дефект не про содержание решения — оно верное и подтверждено кодом — а про
|
||||
форму ТЗ, которая обязана доказывать, что это изменение продукта, а не работа
|
||||
над кодом (PROCESS.md §7.1: «ТЗ, которое не может ответить на эти два вопроса,
|
||||
описывает работу, а не изменение продукта»). Правка дешёвая: одна-две фразы в
|
||||
начале документа, без пересмотра контракта.
|
||||
|
||||
**Предлагаемая правка** (пример, автор волен сформулировать иначе):
|
||||
> Персона — администратор дома (`docs/SCOPE.md`), поверхность — desktop Plan
|
||||
> editor, инструмент Resize; момент — любой ресайз стены, эффект копится
|
||||
> незаметно и проявляется через дни на другой стене.
|
||||
> Видимо: сегодня после серии обычных ресайзов случайная стена в другом месте
|
||||
> плана вдруг рисуется другой толщиной или теряет рабочую ручку ресайза, хотя
|
||||
> её никто не трогал. После исправления обычный ресайз либо проходит как
|
||||
> раньше, либо (в редком неоднозначном случае) заканчивается тем же самым
|
||||
> сообщением об ошибке без изменения плана — но никогда не портит стену, которую
|
||||
> пользователь не двигал.
|
||||
|
||||
Это Medium: без High-находок вердикт жёлтый, правка делается автором в этом же
|
||||
issue, повторного полного разбора не требует — фактическая архитектура решения
|
||||
ревью не оспаривает.
|
||||
|
||||
### Low (снято ревьюером с записью) — способ доказательства не назван явно у двух AC
|
||||
|
||||
DoR-чеклист (`PROCESS.md` §2.5) требует у каждого AC явного указания, чем он
|
||||
доказывается (`unit`/`backend`/`smoke`/`golden`/«ревью кода»). У AC1, AC2, AC4,
|
||||
AC6, AC8, AC9 это явно есть (или очевидно из перечисленных команд). У **AC5**
|
||||
(carrier preflight) и **AC7** (preview/commit/Undo атомарны) отсутствует строка
|
||||
«Доказательство: …» — способ проверки не назван словом, хотя из контекста
|
||||
понятен: AC5 — чистая функция преflight из §4, доказывается unit-тестом; AC7 —
|
||||
поведение pointer-жеста, доказывается production-bundle smoke по аналогии с уже
|
||||
существующим `demo/smoke_room_resize.mjs` (см. `RESIZE.md` «Verification»).
|
||||
|
||||
Снимаю без возврата на цикл: неоднозначности в том, *что* проверяется, нет — их
|
||||
формулировки уже настолько конкретны (перечислены positive/negative кейсы,
|
||||
атомарность записи/Undo), что тип теста восстанавливается однозначно. Автору
|
||||
стоит добавить явную строку при реализации ради единообразия с остальными AC,
|
||||
но это не блокирует переход в «Готово к разработке».
|
||||
|
||||
### Low (снято ревьюером с записью) — разделы «UX» и «i18n» не выделены явно
|
||||
|
||||
`PROCESS.md` §7.1 перечисляет UX и i18n как обязательные разделы. В документе
|
||||
нет разделов с этими заголовками; содержание по факту размазано — UX-семантика
|
||||
(disabled handle, единственное существующее сообщение об ошибке, отсутствие
|
||||
нового UX) описана в §5 и подтверждена явным предположением №3 в §12; i18n
|
||||
покрыт тем же предположением №3 («новый persisted reason или новый UX в этой
|
||||
задаче не вводится» ⇒ новых ключей нет).
|
||||
|
||||
Снимаю: по существу оба вопроса закрыты (нет нового текста, нет новых ключей),
|
||||
и это прямо написано, просто не под ожидаемым заголовком. Chisto формальный
|
||||
момент, не влияющий на проверяемость AC.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Корневая причина в §1 подтверждена чтением `rekeyWallsAfterMove()` —
|
||||
реальный код делает ровно ту пропорциональную проекцию (`mapPoint`), о которой
|
||||
говорит issue и ТЗ; это не пересказ чужих слов.
|
||||
- Оба воспроизведения (AC1/AC2) содержат точные числа из issue
|
||||
(`-85 → -82.457`, `59.538`, вершины `17/52/57/101`) и совпадают с текстом
|
||||
issue буквально — не пересказаны с искажением.
|
||||
- Скоуп «не входит» (§6) корректно исключает #289/#299 (смешанные роли),
|
||||
ADR #282 (integer storage), eligibility/#277/#293 (topology) — все со ссылками
|
||||
на реально существующие issue в правильном статусе.
|
||||
- AC6 корректно ссылается на реальную структуру `KNOWN` в
|
||||
`demo/smoke_edit_walk.mjs` и требует убрать только те два вида находок, которые
|
||||
там сейчас действительно зарегистрированы для этой пары фикстур, не трогая
|
||||
независимый долг #299 (`mixed_role_record`) — проверено построчно.
|
||||
- Ни одна ссылка на связанные issue (#253, #277, #289, #291, #293, #297, #299)
|
||||
не оказалась битой, дублирующей или искажающей состояние (закрыт/открыт).
|
||||
- Технические решения раздела 3 (correspondence-таблица, конфликт нескольких
|
||||
destinations, атомарное разбиение длинной записи) и раздела 4 (carrier
|
||||
preflight по всему span, не только по концам/midpoint) не изобретают новое
|
||||
поведение — они последовательно продолжают уже принятый контракт #277 и модель
|
||||
данных `WALL-THICKNESS.md` («exact endpoints… independent of whichever room
|
||||
topology later happens to split the same straight line»), а не выдают догадку
|
||||
за факт.
|
||||
- Раздел 3.3 (legacy fallback, fail-closed при неоднозначном whole-edge
|
||||
соответствии) прямо согласован с уже задокументированным в `WALL-THICKNESS.md`
|
||||
запретом «изобретать длину» для legacy-записей без `a/b`.
|
||||
- Touch/safety floor в §8 корректно ссылается на существующий раздел
|
||||
`TOUCH-SUPPORT.md` «Safety floor that still applies to touch editors», а не
|
||||
придумывает новое правило.
|
||||
- Откат (§10) реалистичен: чистый revert коммита, миграции нет, потому что схема
|
||||
не меняется — согласуется с §8.
|
||||
- Продуктовых вопросов владельцу в этом ТЗ нет, и по факту разбора это
|
||||
оправданно: спорные места (§5 п.7 про предсказуемый конфликт до pointer
|
||||
capture) читаются как следствие уже принятого принципа fail-closed
|
||||
(`RESIZE.md`: «If neither step is safe, the handle remains… disabled»), а не
|
||||
как новое расширение eligibility — само ТЗ явно исключает изменение eligibility
|
||||
из скоупа (§6 «Не входит»), и текст §5 п.7 с этим не расходится при точном
|
||||
чтении («если конфликт можно доказать в eligibility» — то есть уже
|
||||
существующими проверками, а не новыми).
|
||||
- Ни один найденный ранее класс дефектов (#253, #258/#259, #287/#289) не
|
||||
игнорируется задним числом — ТЗ явно проговаривает отличие от каждого в
|
||||
разделе «Почему это дорого» issue и в §6/§12 ТЗ.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Само исправление (кода ещё нет — задача в `S4-spec-review`, реализация не
|
||||
началась). AC1–AC9 разбирались на выполнимость и проверяемость формулировки,
|
||||
а не на то, будущий код им действительно удовлетворит — это работа
|
||||
код-ревью.
|
||||
- Полный текст `docs/CANVAS.md` — раздел на который ссылается ТЗ, но задача не
|
||||
меняет canvas-рендер напрямую (только данные, которые он потребляет); беглая
|
||||
проверка показала, что ссылка не противоречит модели данных `walls[*]`.
|
||||
- Все шесть комбинаций `smoke_edit_walk` не прогонялись — на этой стадии нет
|
||||
кода для прогона; сверка ограничилась статическим соответствием таблицы
|
||||
`KNOWN` тому, что требует AC6.
|
||||
- Производительность (AC9) — числовые бюджеты `RESIZE.md` не пересчитывались,
|
||||
только сверено само требование «сохраняется p95 budget» с текстом канона.
|
||||
|
||||
## Вывод
|
||||
|
||||
Единственная содержательная находка — Medium, отсутствие обязательных
|
||||
продуктовых разделов §7.1 (персона/поверхность/момент + «одна фраза без
|
||||
терминов реализации»). Остальное — два Low, снятых с запиской. High-находок
|
||||
нет: контракт технически выполним, каждый AC проверяем, ни одна ссылка не
|
||||
искажена, ни одна догадка не выдана за решённый факт. Вердикт жёлтый —
|
||||
формально из-за Medium, а не из-за сомнений в самом решении.
|
||||
@@ -0,0 +1,126 @@
|
||||
# SPEC-REVIEW-298-r2
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/298
|
||||
- **ТЗ:** `docs/specs/298-resize-wall-thickness-carrier.md` @ commit `55db16df6e51027a81aaf56de6cd08bbf19c3b77`
|
||||
- **Этап:** ТЗ на ревью (PROCESS.md §2.4)
|
||||
- **Заход:** r2 · блокирующих циклов израсходовано 1 из 4 (потрачено r1, жёлтый)
|
||||
- **Ревьюер:** Claude (сессия ревью ТЗ), автор ≠ ревьюер
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Не первый заход. По PROCESS.md §2.9 / issue #214 разбор идёт по дельте:
|
||||
|
||||
- предыдущий вердикт — комментарий `claude`, 2026-08-24T19:01:20Z, в issue #298:
|
||||
жёлтый, r1, блокирующих циклов 0/4, High 0 / Medium 1 (в скоупе), документ
|
||||
`docs/reviews/SPEC-REVIEW-298-r1.md`, ТЗ проверялось на SHA
|
||||
`45066631aee4997ac1f033dbe75918b5b2d439d4` (SHA не был назван в самом
|
||||
вердикте — восстановлен из ссылки автора на коммит ТЗ в комментарии за 18:54:17Z,
|
||||
который текстуально совпадает с состоянием, разобранным в r1-документе);
|
||||
- правка автора — коммит `55db16df6e51027a81aaf56de6cd08bbf19c3b77`
|
||||
(`docs(spec): add resize user contract`, единственный файл
|
||||
`docs/specs/298-resize-wall-thickness-carrier.md`, +25/-1);
|
||||
- дельта строго локальна: добавлен раздел «Персона, поверхность, момент и
|
||||
видимый результат», добавлена подсекция «UX и i18n», добавлены две строки
|
||||
«Доказательство:» под AC5 и AC7. Ни один AC, ни контракт поведения (§§3–5),
|
||||
ни scope (§6) не менялись. Рёбейза на ушедший вперёд `dev` не было (родитель
|
||||
коммита — `8221ad9b`, публикация r1-документа, что и ожидается между
|
||||
раундами). Подсистема не сменилась, новых открытых вопросов правка не внесла.
|
||||
Объём дельты (одна секция + два предложения) несопоставим с объёмом задачи →
|
||||
разбор сокращён до дельты, остальное наследуется из r1.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. `git diff 45066631..55db16df -- docs/specs/298-resize-wall-thickness-carrier.md`
|
||||
— получен полный текст дельты, показан ниже в разделе «Закрытие раунда r1».
|
||||
2. Дельта прочитана в контексте всего текущего файла (`Read` целиком, 377
|
||||
строк), чтобы исключить рассинхронизацию новых фраз с остальным ТЗ.
|
||||
3. Новый раздел 1 сверен с:
|
||||
- `docs/SCOPE.md` — персона «Home admin (primary) … Desktop browser», job
|
||||
J6 «Keep the plan true as the home evolves … drag/resize» — совпадает
|
||||
с «администратор дома / desktop Plan editor / инструмент Resize»;
|
||||
- комментарием-аналитикой автора (18:52:14Z), где та же персона и та же
|
||||
поверхность были названы изначально, но не попадали в сам документ —
|
||||
ровно то несоответствие, которое r1 отметил как находку.
|
||||
4. Термин «ручка» и написание «Resize» сверены с `docs/USER-GUIDE.ru.md`
|
||||
(строки 441, 480–481: «Resize», «ручка объясняет…», «Ручка остаётся
|
||||
видимой, но приглушена») — терминология не изобретена, взята из канона.
|
||||
5. Новая подсекция «UX и i18n» сверена с §6 «Не входит» («изменение
|
||||
eligibility, safe range, pointer UX … не входит») и §12 п.3 («Existing
|
||||
`resize.commit_failed` достаточно … пока eligibility не меняется») —
|
||||
утверждение «новых контролов/текстов нет» не противоречит остальному
|
||||
документу.
|
||||
6. Новые строки «Доказательство:» под AC5/AC7 сверены с разделом 11
|
||||
(«Ожидаемые файлы и release artifacts»): методы («table-driven unit pure
|
||||
carrier preflight», «production-bundle Resize pointer smoke») там же
|
||||
названы как ожидаемые файлы/evidence — не расходятся.
|
||||
7. Коммит `55db16df` проверен `git show --stat`: правда только один файл,
|
||||
+25/-1, что совпадает с диффом, разобранным построчно (не поверено на
|
||||
слово автора).
|
||||
8. Трейлеры коммита: `Issue: #298`, `User-Visible: no` — для docs-only
|
||||
spec-коммита корректно (видимого поведения нет, меняется только текст ТЗ).
|
||||
|
||||
**Не проверялось повторно** (наследуется из r1, ниже отдельным разделом):
|
||||
корневая причина по коду, существование фикстур/констант/связанных issue,
|
||||
формулировки §§3–5, §6, §7 (AC1–AC9 по содержанию), §8–§12 — дельта их не
|
||||
касается, к этим утверждениям автор не притрагивался.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| **Medium (в скоупе):** ТЗ не содержит обязательных продуктовых разделов §7.1 — «персона/поверхность/момент» и «что человек увидит до/после» без терминов реализации; §1 и §2 были на языке реализации (`rekeyWallsAfterMove`, topology vertex, config write) | Добавлен раздел 1 «Персона, поверхность, момент и видимый результат»: явно назван «администратор дома», «desktop Plan editor, инструмент Resize», «обычный ресайз стены готового плана»; добавлены два предложения «До:»/«После:» без единого термина реализации (стена рисуется другой толщиной / теряет ручку → ресайз либо проходит как раньше, либо fail-closed с прежней ошибкой) | `docs/specs/298-resize-wall-thickness-carrier.md`, новые строки 16–26 (коммит `55db16df`) |
|
||||
| **Low (снят с записью, не блокировал):** AC5 и AC7 не называют явно способ доказательства | Под AC5 и AC7 добавлены явные строки «Доказательство: …» | строки 244–245 (AC5) и 272–273 (AC7) |
|
||||
| **Low (снят с записью, не блокировал):** разделы «UX» и «i18n» не выделены явными заголовками | Добавлена подсекция `### UX и i18n` внутри §5 с прямым утверждением «новых контролов, состояний и текстов нет» | строки 162–166 |
|
||||
|
||||
Все три пункта r1 закрыты правкой; новых расхождений между добавленным текстом
|
||||
и остальным документом не найдено (см. «Как проверялось», пп. 3–6).
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Без повторной проверки в этом раунде принято по документу
|
||||
`docs/reviews/SPEC-REVIEW-298-r1.md` (ТЗ на SHA `45066631aee4997ac1f033dbe75918b5b2d439d4`),
|
||||
поскольку дельта r1→r2 их не касается:
|
||||
|
||||
- корневая причина подтверждена чтением `rekeyWallsAfterMove()`
|
||||
(`src/wall-thickness.ts:533`, пропорциональная проекция `mapPoint`,
|
||||
строки 574–582) — совпадает с числами из issue;
|
||||
- обе fixture-репродукции (`real-plan-second-floor.json`,
|
||||
`real-plan-first-floor.json`) существуют и соответствуют описанным в AC1/AC2
|
||||
жестам;
|
||||
- `LATTICE_NOISE_STEPS` существует (`src/coordinate-canonicalization.ts:11`,
|
||||
`1e-4`), используется в инвариантах и мутационном гейте;
|
||||
- все связанные issue (#253, #277, #289, #291, #293, #297, #299) существуют в
|
||||
заявленном состоянии (закрыты/открыт), ссылки не искажены;
|
||||
- ссылки на канон (`WALL-THICKNESS.md`, `RESIZE.md`, `TOUCH-SUPPORT.md`)
|
||||
точны, не изобретены;
|
||||
- AC1–AC9 по содержанию проверяемы, скоуп «не входит» корректно исключает
|
||||
#299 (`mixed_role_record`);
|
||||
- §12 «Принятые технические предположения» — явные, не выданы за факт.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет. Единственная находка r1 (Medium, в скоупе) закрыта дельтой; новых находок
|
||||
дельта не создала.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Дельта r1→r2 (раздел 1, подсекция UX/i18n, две строки «Доказательство:»)
|
||||
внутренне согласована с остальным документом и с каноном (`SCOPE.md`,
|
||||
`USER-GUIDE.ru.md`).
|
||||
- Персона/поверхность/момент и фраза «до/после» присутствуют, без терминов
|
||||
реализации — оба обязательных продуктовых раздела §7.1 закрыты.
|
||||
- Коммит содержит ровно заявленное изменение, трейлеры корректны.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Повторно не перечитывал и не оспаривал §§2–12 по содержанию — дельта их не
|
||||
меняла, см. «Унаследовано из r1».
|
||||
- Гейты `typecheck`/`test`/`build`/`check-docs` не запускал — стадия ТЗ, кода к
|
||||
задаче ещё нет; это ожидается на этапе code-review.
|
||||
- Не проверял снова состояние связанных issue (#253/#277/#289/#291/#293/#297/#299)
|
||||
— наследуется из r1, ссылки дельта не меняла.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. Единственная Medium-находка r1 закрыта точечной и корректной правкой,
|
||||
новых находок нет.
|
||||
@@ -0,0 +1,376 @@
|
||||
# Issue #298 — Resize сохраняет wall records на решётке и на carrier
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/298
|
||||
- **Статус:** первая редакция для внешнего ревью; канонический статус задаётся
|
||||
метками issue
|
||||
- **Тип / приоритет:** bug / P1
|
||||
- **Оценка:** пользовательская ценность 10/10; ценность для разработки 10/10;
|
||||
сложность 7/10; риск 8/10
|
||||
- **Область:** fixed-topology Resize, exact wall records, live preview/commit,
|
||||
grid barrier, real-plan invariant smoke и mutation coverage
|
||||
- **Модель данных:** schema и model version не меняются; исправляются только
|
||||
записи, которые переносит новый Resize-жест
|
||||
- **Связано:** #253, #277, #289, #291, #293, #297,
|
||||
`docs/RESIZE.md`, `docs/WALL-THICKNESS.md`, `docs/CANVAS.md`
|
||||
|
||||
## 1. Персона, поверхность, момент и видимый результат
|
||||
|
||||
Персона — администратор дома. Поверхность — desktop Plan editor, инструмент
|
||||
Resize; момент — обычный ресайз стены готового плана. Повреждение копится
|
||||
незаметно и может проявиться лишь через несколько следующих правок или дней.
|
||||
|
||||
**До:** после серии обычных ресайзов случайная стена в другом месте плана вдруг
|
||||
рисуется другой толщиной или теряет рабочую ручку, хотя её никто не трогал.
|
||||
**После:** ресайз либо проходит внешне как раньше, либо в редком неоднозначном
|
||||
случае заканчивается прежним сообщением об ошибке без изменения плана, но
|
||||
никогда не портит другую стену.
|
||||
|
||||
### Подтверждённая причина
|
||||
|
||||
Пользователь безопасно сдвигает одну стену комнаты. Moving wall едет
|
||||
параллельно себе, а две соседние стены меняют длину. После жеста визуально всё
|
||||
может выглядеть правдоподобно, но запись толщины другой стены получает endpoint,
|
||||
которого нет ни среди вершин room polygons, ни на границе wall carrier. На
|
||||
следующем редактировании это проявляется потерей кладки, ошибкой Optimize или
|
||||
сломавшейся ручкой Resize.
|
||||
|
||||
На обеих приложенных к issue реальных fixtures причина подтверждена в
|
||||
`rekeyWallsAfterMove()`. Для точки `p` на старом ребре helper вычисляет
|
||||
относительную долю `t`, затем возвращает точку с той же долей на новом ребре.
|
||||
Когда fixed-topology Resize двигает только один endpoint бокового ребра,
|
||||
внутренний endpoint wall record тоже пропорционально уезжает. Так `-85`
|
||||
становится `-82.457`, хотя такой geometry boundary в новом polygon нет.
|
||||
|
||||
Текущий commit guard проверяет только мультимножество `cm` и число open spans.
|
||||
Он не доказывает, что все exact wall records лежат на carriers нового плана,
|
||||
поэтому повреждённая запись сохраняется.
|
||||
|
||||
## 2. Пользовательский результат
|
||||
|
||||
Resize продолжает выглядеть и управляться как после #277/#293. Отличие в
|
||||
сохранённых данных: перемещаются только wall endpoints, для которых существует
|
||||
однозначное соответствие старой и новой topology vertex. Остальные endpoints
|
||||
не интерполируются и остаются на своей физической границе. После commit все
|
||||
записи толщины остаются на grid и на room-wall carriers; последующие Resize,
|
||||
Optimize и рендер не получают скрытую повреждённую геометрию.
|
||||
|
||||
Если lossless correspondence доказать нельзя, кандидат не сохраняется. Live
|
||||
preview остаётся на последней валидной позиции, а при отсутствии валидной
|
||||
позиции жест завершается существующей локализованной ошибкой Resize без config
|
||||
write и Undo entry. Автоматического снапа настоящей off-grid координаты или
|
||||
догадки по ближайшей стене нет.
|
||||
|
||||
## 3. Fixed-topology correspondence
|
||||
|
||||
### 3.1 Источник истины
|
||||
|
||||
Для каждого затронутого room edge Resize уже имеет параллельную пару
|
||||
`old edge → new edge` из immutable pre-drag snapshot и exact candidate.
|
||||
Topology signature гарантирует одинаковое число и циклическую identity
|
||||
вершин. На этой основе строится таблица соответствия:
|
||||
|
||||
`old room vertex → new room vertex`.
|
||||
|
||||
В таблицу входят только действительно изменившиеся vertices. Одинаковая
|
||||
старая точка, принадлежащая двум room copies общей стены, обязана иметь ровно
|
||||
одну новую destination. Несколько разных destinations означают конфликт и
|
||||
отменяют кандидат; порядок rooms или wall records не выбирает победителя.
|
||||
|
||||
### 3.2 Exact wall records
|
||||
|
||||
Пара `a/b` является identity exact wall record; compatibility `key` только
|
||||
пересчитывается из итогового span.
|
||||
|
||||
Каждая исходная exact запись обрабатывается из immutable snapshot:
|
||||
|
||||
1. запись делится на атомы в endpoints перекрывающихся затронутых old edges;
|
||||
2. endpoint атома заменяется только если он равен old topology vertex из
|
||||
таблицы correspondence в пределах canonical coordinate epsilon;
|
||||
3. interior endpoint без vertex correspondence остаётся byte-equivalent —
|
||||
относительная доля длины ребра для него не вычисляется;
|
||||
4. нулевые атомы удаляются, совместимые соседние атомы с одинаковым `cm`
|
||||
склеиваются только когда их endpoints точно совпали и они коллинеарны;
|
||||
5. для каждого изменённого span заново строится compatibility key, а `cm` и
|
||||
известные совместимые поля сохраняются.
|
||||
|
||||
Длинная запись, пересекающая затронутый и незатронутый пролёты, обязана
|
||||
разделиться на границе old edge: только endpoint затронутого атома следует за
|
||||
вершиной. Нельзя affine-масштабировать целую запись или её внутреннюю точку.
|
||||
|
||||
### 3.3 Legacy key-only records
|
||||
|
||||
Legacy запись без валидных `a/b` не имеет длины и endpoints, поэтому ей нельзя
|
||||
изобретать атомы. Разрешён только однозначный whole-edge fallback: старый key
|
||||
точно соответствует целому изменённому edge и переносится на его новый key.
|
||||
Projected-midpoint перенос по относительной доле запрещён в production Safe
|
||||
Resize. Если legacy key затронут, но whole-edge соответствие неоднозначно,
|
||||
кандидат fail closed и не сохраняется. Незатронутые legacy records остаются
|
||||
byte-equivalent.
|
||||
|
||||
Исторический generic scale/rotate helper может остаться для изолированных
|
||||
pure-тестов старых преобразований, но production Safe Resize обязан вызывать
|
||||
fixed-topology API. Переиспользование proportional `t` в этом path запрещено.
|
||||
|
||||
## 4. Carrier и lattice preflight
|
||||
|
||||
После wall rekey, но до принятия live preview, product code проверяет exact
|
||||
candidate целиком.
|
||||
|
||||
Для каждой exact wall entry:
|
||||
|
||||
- `a` и `b` конечны и лежат на canonical grid либо отличаются не больше
|
||||
действующего near-node storage epsilon;
|
||||
- весь открытый span между `a` и `b` покрывается непрерывным объединением
|
||||
коллинеарных room edges; недостаточно проверить только midpoint;
|
||||
- нет зазора между carrier atoms и нет участка, принадлежащего только
|
||||
продолжению оси за пределами стены;
|
||||
- compatibility key согласован с итоговыми `a/b`;
|
||||
- `cm` остаётся в допустимом диапазоне, а мультимножество исходных физических
|
||||
значений толщины не теряется.
|
||||
|
||||
Проверка допускает одну compact record через несколько смежных коллинеарных
|
||||
room edges, но не допускает запись через разрыв. Independent partitions не
|
||||
являются carrier для `space.walls`: их толщина хранится своей partition
|
||||
geometry. Open spans по-прежнему проходят существующий отдельный rekey и
|
||||
carrier validation.
|
||||
|
||||
`LATTICE_NOISE_STEPS` из измерительного гейта отличает форматный near-node шум
|
||||
от настоящего off-grid значения. Исправление не округляет авторскую
|
||||
координату, удалённую от grid: такой candidate отклоняется. Уже сохранённые
|
||||
старые off-grid records не мигрируют при загрузке и не меняются без жеста.
|
||||
|
||||
## 5. Preview, commit и failure semantics
|
||||
|
||||
1. Preview каждый раз строится из immutable pre-drag rooms/walls/open spans,
|
||||
а не из предыдущего кадра.
|
||||
2. Fixed-topology mapping и carrier/lattice preflight являются частью одного
|
||||
candidate builder, которым пользуются preview и pointerup.
|
||||
3. `_serverCfg`, history и queued save не меняются до успешного pointerup.
|
||||
4. Commit принимает только уже показанный exact preview и повторно проверяет
|
||||
snapshot/plan signature. Отдельного второго rekey нет.
|
||||
5. Конфликт correspondence, неоднозначный legacy record либо carrier/lattice
|
||||
failure отклоняет кандидат целиком. Частичная запись запрещена.
|
||||
6. После runtime reject сохраняется последний валидный preview; если валидного
|
||||
ненулевого preview не было, config/history byte-equivalent исходному.
|
||||
7. Undo/Redo восстанавливают rooms, openings, walls и open spans целиком.
|
||||
|
||||
Предсказуемый конфликт, который можно доказать в eligibility, должен сделать
|
||||
handle disabled до pointer capture. Непредсказуемый runtime reject использует
|
||||
уже существующее сообщение `resize.commit_failed`; новый persisted reason или
|
||||
новый UX в этой задаче не вводится.
|
||||
|
||||
### UX и i18n
|
||||
|
||||
Новых контролов, состояний и текстов нет. Enabled/disabled handle, toast и
|
||||
доступные имена сохраняют существующий контракт #277/#293; новых RU/EN ключей
|
||||
не добавляется. Меняется только атомарность данных за прежним жестом.
|
||||
|
||||
## 6. Scope
|
||||
|
||||
### Входит
|
||||
|
||||
- fixed-topology rekey exact и legacy wall records;
|
||||
- endpoint correspondence без proportional interpolation;
|
||||
- product carrier/lattice preflight до preview/commit;
|
||||
- точные регрессии обеих fixtures из #298;
|
||||
- обновление `smoke_edit_walk`/`KNOWN`, mutation и performance evidence;
|
||||
- canonical Resize/wall-thickness/testing docs и оба changelog.
|
||||
|
||||
### Не входит
|
||||
|
||||
- изменение eligibility, safe range, pointer UX или topology #277/#293;
|
||||
- автоматический ремонт уже сохранённого bad plan через Optimize;
|
||||
- schema migration, integer-coordinate storage либо общий ADR #282;
|
||||
- смешанные shared/exterior роли #289/#299;
|
||||
- изменение толщины пользователем, renderer wall union или opening geometry;
|
||||
- touch parity Plan editor сверх общего safety floor.
|
||||
|
||||
## 7. Acceptance criteria
|
||||
|
||||
### AC1. Exact repro второго этажа
|
||||
|
||||
На `real-plan-second-floor.json` production Resize выполняет указанную в issue
|
||||
последовательность `room-a`, edge 2, `x=96 → 100` grid steps. Запись исходного
|
||||
горизонтального span `y=304` сохраняет endpoint на существующей topology
|
||||
boundary; значение `-85` не превращается в `-82.457` или другую
|
||||
пропорциональную точку.
|
||||
|
||||
После preview и commit нет `off_lattice_coordinate`, `wall_carrier`, wall-key,
|
||||
reference и physical-geometry нарушений. Moving/shared rooms и openings
|
||||
сохраняют контракты #277/#293.
|
||||
|
||||
**Доказательство:** fixture-backed pure/unit regression и production-bundle
|
||||
pointer smoke; assert проверяет конкретную identity записи до/после, а не
|
||||
только отсутствие исключения.
|
||||
|
||||
### AC2. Exact repro первого этажа
|
||||
|
||||
На `real-plan-first-floor.json` последовательность `room-b`, edge 0,
|
||||
`x=49 → 52` не создаёт endpoint `59.538` на wall record линии `y=155`.
|
||||
Итоговые endpoints принадлежат реальным vertices/carrier boundaries
|
||||
`17/52/57/101` согласно новому candidate.
|
||||
|
||||
**Доказательство:** отдельный fixture-backed regression с exact endpoint,
|
||||
carrier/lattice и unchanged-record assertions.
|
||||
|
||||
### AC3. Untouched records действительно не меняются
|
||||
|
||||
Table-driven unit покрывает non-shared wall, exact shared pair, длинную запись,
|
||||
пересекающую moved и untouched spans, reversed orientation и несколько
|
||||
одинаковых `cm`. Запись без overlap и без moved endpoint остаётся deep/byte
|
||||
equivalent, включая key и порядок. Затронутые записи сохраняют все значения
|
||||
`cm`; коллизия destinations отклоняет весь candidate.
|
||||
|
||||
### AC4. Legacy compatibility не угадывается
|
||||
|
||||
- unambiguous whole-edge key переезжает на новый whole-edge key;
|
||||
- untouched key-only record не меняется;
|
||||
- key-only midpoint на части изменившего длину edge не переносится
|
||||
пропорционально и приводит к fail-closed candidate;
|
||||
- exact record всегда использует `a/b`, даже если старый compatibility key
|
||||
неверен.
|
||||
|
||||
**Доказательство:** pure unit и production-preview reject с нулём config/history
|
||||
writes.
|
||||
|
||||
### AC5. Carrier preflight проверяет весь span
|
||||
|
||||
Positive cases: одна room edge и непрерывная цепочка нескольких коллинеарных
|
||||
room edges. Negative cases: оба endpoints на carriers, но между ними разрыв;
|
||||
midpoint на продолжении за пределами edge; endpoint на independent partition;
|
||||
настоящая off-grid coordinate дальше `LATTICE_NOISE_STEPS`; conflicting shared
|
||||
destinations. Каждый negative candidate отклоняется до сохранения.
|
||||
|
||||
**Доказательство:** table-driven unit pure carrier preflight плюс integration
|
||||
test отказа candidate builder до мутации preview/config.
|
||||
|
||||
### AC6. Шесть edit-walk запусков больше не несут этот долг
|
||||
|
||||
Проходят:
|
||||
|
||||
```text
|
||||
node demo/smoke_edit_walk.mjs --seed 1 --plan real-plan-second-floor.json
|
||||
node demo/smoke_edit_walk.mjs --seed 2 --plan real-plan-second-floor.json
|
||||
node demo/smoke_edit_walk.mjs --seed 3 --plan real-plan-second-floor.json
|
||||
node demo/smoke_edit_walk.mjs --seed 1 --plan real-plan-first-floor.json
|
||||
node demo/smoke_edit_walk.mjs --seed 2 --plan real-plan-first-floor.json
|
||||
node demo/smoke_edit_walk.mjs --seed 3 --plan real-plan-first-floor.json
|
||||
```
|
||||
|
||||
Для результатов Resize нет `off_lattice_coordinate` и `wall_carrier`.
|
||||
`KNOWN` обновляется в том же implementation-коммите только для исправленных
|
||||
kinds. Независимый долг `mixed_role_record` #299 не скрывается и не считается
|
||||
регрессией #298.
|
||||
|
||||
### AC7. Preview/commit/Undo атомарны
|
||||
|
||||
Valid pointer drag показывает exact candidate до release, затем создаёт ровно
|
||||
один config write и один Undo entry. Undo возвращает byte-equivalent исходные
|
||||
rooms/walls/open spans; Redo возвращает тот же exact candidate. Forced carrier
|
||||
failure даёт ноль writes/history и существующую локализованную ошибку один раз.
|
||||
|
||||
**Доказательство:** production-bundle pointer smoke с чтением DOM preview,
|
||||
persisted config и history до release, после commit, Undo/Redo и forced reject.
|
||||
|
||||
### AC8. Мутационный страж
|
||||
|
||||
Mutation возвращает proportional `t` mapping для interior endpoint либо
|
||||
отключает carrier preflight. AC1/AC2 или AC5 обязаны падать. Blanket-disable
|
||||
Resize не проходит positive pointer scenario AC7 и существующие #293 smokes.
|
||||
|
||||
### AC9. Производительность и локальные гейты
|
||||
|
||||
Mapping строится один раз на candidate из уже ограниченного набора затронутых
|
||||
edges. Нельзя добавлять глобальное pairwise сравнение всех records со всеми
|
||||
room edges на каждый `pointermove` без подготовленного carrier index. p95 budget
|
||||
Resize из `docs/RESIZE.md` сохраняется; если hot path меняется, targeted
|
||||
benchmark сравнивается с baseline.
|
||||
|
||||
Обязательны:
|
||||
|
||||
- `npm run typecheck`;
|
||||
- `npm test`;
|
||||
- `npm run build` и bundle parity;
|
||||
- `node scripts/check-docs.mjs`;
|
||||
- targeted Resize pointer smoke, шесть edit-walk запусков и mutation gate.
|
||||
|
||||
Полные golden, smoke, performance и Linux HA harness выполняются перед beta.
|
||||
|
||||
## 8. Совместимость, touch и security
|
||||
|
||||
Schema/storage/model version не меняются. Старые планы читаются без фоновой
|
||||
перезаписи; исторический off-grid долг остаётся видимым до явной правки или
|
||||
отдельного Optimize repair. Новый commit только запрещает Safe Resize создавать
|
||||
новый долг.
|
||||
|
||||
Plan editor остаётся desktop-first. Touch — best effort, но safety floor общий:
|
||||
single-pointer drag не пишет invalid candidate, pinch/pan, pointercancel и lost
|
||||
capture не создают config/history entries. Новых HA actions, сетевых запросов,
|
||||
HTML/CSS input или security boundaries нет.
|
||||
|
||||
## 9. Риски и меры
|
||||
|
||||
- **Слишком широкий vertex match** может двигать соседний interior endpoint.
|
||||
Мера: canonical epsilon, explicit correspondence и exact AC1–AC3.
|
||||
- **Слишком узкий match** потеряет толщину moved wall. Мера: positive
|
||||
non-shared/shared/long-record matrix и `checkWallRecordsPreserved`.
|
||||
- **Проверка только endpoints/midpoint** пропустит разрыв carrier. Мера: full
|
||||
interval coverage AC5.
|
||||
- **Legacy fallback снова введёт интерполяцию.** Мера: explicit fail-closed AC4
|
||||
и mutant AC8.
|
||||
- **Соседняя #299 меняет тот же `KNOWN`.** Мера: #298 удаляет только два своих
|
||||
kinds и при rebase сохраняет независимые mixed-role строки.
|
||||
- **Новый global validation замедлит pointermove.** Мера: подготовленный index
|
||||
и benchmark AC9.
|
||||
|
||||
## 10. Откат
|
||||
|
||||
Откат — полный revert implementation-коммита вместе с tests/docs/`KNOWN`.
|
||||
Миграция или восстановление schema не нужны. Конфиги, уже сохранённые новой
|
||||
версией, используют прежнюю схему и читаются старой версией.
|
||||
|
||||
## 11. Ожидаемые файлы и release artifacts
|
||||
|
||||
Product code:
|
||||
|
||||
- `src/wall-thickness.ts` — fixed-topology rekey;
|
||||
- `src/houseplan-card.ts` — candidate integration и fail-closed preflight;
|
||||
- отдельный pure carrier helper допускается, если не дублирует invariant model.
|
||||
|
||||
Tests/evidence:
|
||||
|
||||
- `test/wall-thickness.test.mjs` и/или targeted Resize unit;
|
||||
- `demo/smoke_edit_walk.mjs`, включая точное обновление `KNOWN`;
|
||||
- production-bundle Resize pointer smoke;
|
||||
- mutation registry и targeted benchmark при изменении hot path.
|
||||
|
||||
Документация:
|
||||
|
||||
- `docs/RESIZE.md`, `docs/WALL-THICKNESS.md`, `docs/TESTING.md`;
|
||||
- при изменении архитектурной границы — `docs/ARCHITECTURE.md`;
|
||||
- `docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md`.
|
||||
|
||||
Визуальный дизайн не меняется, поэтому новый golden baseline не ожидается.
|
||||
Если штатный docs screenshot всё же изменится, принимается только artifact
|
||||
Linux workflow после визуального review и bundle sync.
|
||||
|
||||
Implementation-коммит имеет terminal trailers:
|
||||
|
||||
```text
|
||||
Issue: #298
|
||||
User-Visible: yes
|
||||
```
|
||||
|
||||
Issue не закрывается вручную: она закрывается пакетно при выпуске beta.
|
||||
|
||||
## 12. Принятые технические предположения
|
||||
|
||||
1. Зафиксированные в issue gestures и tracked real-plan fixtures являются
|
||||
достаточным privacy-safe regression input; новые приватные данные не нужны.
|
||||
2. `space.walls` относится только к room-wall carriers. Independent partitions
|
||||
хранят толщину в собственном объекте и не оправдывают wall record вне room
|
||||
boundary.
|
||||
3. Existing `resize.commit_failed` достаточно для редкого runtime reject;
|
||||
новый user-facing reason не требуется, пока eligibility не меняется.
|
||||
4. Near-node storage noise из #291 отличается от настоящего repro #298;
|
||||
исправление не расширяет snap tolerance.
|
||||
@@ -78,6 +78,7 @@ GitHub Issues и GitHub Projects (v2) остаются единственным
|
||||
| [#281](https://github.com/Matysh/houseplan-card/issues/281) Честный Resize после outer-partition reconciliation | [281-resize-zero-range.md](281-resize-zero-range.md) |
|
||||
| [#293](https://github.com/Matysh/houseplan-card/issues/293) Активная рукоятка Resize выполняет pointer-жест | [293-resize-pointer-noop.md](293-resize-pointer-noop.md) |
|
||||
| [#296](https://github.com/Matysh/houseplan-card/issues/296) Optimize удаляет доказанно избыточные скрытые стены | [296-optimize-hidden-obstacles.md](296-optimize-hidden-obstacles.md) |
|
||||
| [#298](https://github.com/Matysh/houseplan-card/issues/298) Resize сохраняет wall records на решётке и на carrier | [298-resize-wall-thickness-carrier.md](298-resize-wall-thickness-carrier.md) |
|
||||
|
||||
## P2
|
||||
|
||||
|
||||
@@ -234,6 +234,33 @@ export const MUTANTS = [
|
||||
replace: " if (false && !movingAxis) return { enabled: false, reason: 'diagonal' };",
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'safe-resize-wall-endpoints-affine-scaled',
|
||||
guard: 'npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs '
|
||||
+ '&& node --test --test-name-pattern="issue 298" test/wall-thickness.test.mjs',
|
||||
because: 'fixed-topology Resize may translate a moving wall or move its topology endpoint, '
|
||||
+ 'but proportional scaling invents a wall-record coordinate with no carrier (#298)',
|
||||
patches: [{
|
||||
file: 'src/wall-thickness.ts',
|
||||
find: " if (mode === 'fixed-topology') {\n"
|
||||
+ ' const adx = move.na[0] - move.oa[0], ady = move.na[1] - move.oa[1];',
|
||||
replace: " if (false && mode === 'fixed-topology') {\n"
|
||||
+ ' const adx = move.na[0] - move.oa[0], ady = move.na[1] - move.oa[1];',
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'safe-resize-legacy-midpoint-fail-open',
|
||||
guard: 'npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs '
|
||||
+ '&& node --test --test-name-pattern="issue 298 fixed-topology legacy" '
|
||||
+ 'test/wall-thickness.test.mjs',
|
||||
because: 'Safe Resize must reject an affected key-only midpoint unless it names '
|
||||
+ 'one unambiguous whole changed edge (#298)',
|
||||
patches: [{
|
||||
file: 'src/wall-thickness.ts',
|
||||
find: " if (mode === 'fixed-topology') {\n const direct = wholeEdgeMoves.get(w.key);",
|
||||
replace: " if (false && mode === 'fixed-topology') {\n const direct = wholeEdgeMoves.get(w.key);",
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'safe-resize-third-room-cascade-enabled',
|
||||
guard: 'npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs '
|
||||
|
||||
+41
-3
@@ -60,7 +60,7 @@ import {
|
||||
cmToNorm, clampFurnSize, clampFurnCm, FURN_WALL_CELLS, type FurnitureGroup,
|
||||
} from './furniture';
|
||||
import {
|
||||
degradeWalls, rekeyWallsAfterMove,
|
||||
degradeWalls, rekeyWallsAfterMoveChecked, wallRecordCarrierViolations,
|
||||
setWallThickness, setWallThicknessForRoom, cmToField, wallCmToUnits,
|
||||
wallEdgeBodies, wallBodiesGeometry, wallBodiesUnionPath,
|
||||
floorFootprintGeometry,
|
||||
@@ -8571,9 +8571,11 @@ class HouseplanCard extends LitElement {
|
||||
if (movedOpen.length) (sp as any).open_spans = movedOpen;
|
||||
else delete (sp as any).open_spans;
|
||||
if (Array.isArray(sp.walls) && sp.walls.length) {
|
||||
sp.walls = rekeyWallsAfterMove(
|
||||
sp.walls, oldSpans, newSpans, this._wallKeyPitch, NORM_W,
|
||||
const rekeyed = rekeyWallsAfterMoveChecked(
|
||||
sp.walls, oldSpans, newSpans, this._wallKeyPitch, NORM_W, 'fixed-topology',
|
||||
);
|
||||
if (rekeyed.rejected) return { ok: false, reason: 'wall-metadata' };
|
||||
sp.walls = rekeyed.walls;
|
||||
}
|
||||
}
|
||||
// Rekeying may change coordinates/keys, never the number or physical
|
||||
@@ -8589,6 +8591,42 @@ class HouseplanCard extends LitElement {
|
||||
if ((s.open_spans || []).length !== ((sp as any).open_spans || []).length) {
|
||||
return { ok: false, reason: 'open-span-metadata' };
|
||||
}
|
||||
const wallCarriers: [number[], number[]][] = [];
|
||||
for (const room of sp.rooms || []) {
|
||||
const poly = roomPoly(room);
|
||||
if (!poly || poly.length < 2) continue;
|
||||
for (let index = 0; index < poly.length; index++) {
|
||||
wallCarriers.push([
|
||||
[poly[index][0] * NORM_W, poly[index][1] * NORM_W],
|
||||
[poly[(index + 1) % poly.length][0] * NORM_W,
|
||||
poly[(index + 1) % poly.length][1] * NORM_W],
|
||||
]);
|
||||
}
|
||||
}
|
||||
// Old plans may contain explicit historical debt (for example an authored
|
||||
// off-grid thickness breakpoint). Safe Resize must not silently repair it,
|
||||
// but it may not create a new invalid record either. Remove byte-identical
|
||||
// old records as a multiset and prove only newly written/split records.
|
||||
// That keeps pointermove bounded by the touched thickness profile instead
|
||||
// of comparing every historical record with every carrier twice per frame.
|
||||
const wallSignature = (wall: any): string => JSON.stringify([
|
||||
wall?.key, wall?.cm, wall?.a, wall?.b,
|
||||
]);
|
||||
const oldWallCounts = new Map<string, number>();
|
||||
for (const wall of s.walls || []) {
|
||||
const key = wallSignature(wall);
|
||||
oldWallCounts.set(key, (oldWallCounts.get(key) || 0) + 1);
|
||||
}
|
||||
const changedWalls: WallEntry[] = [];
|
||||
for (const wall of sp.walls || []) {
|
||||
const key = wallSignature(wall);
|
||||
const remaining = oldWallCounts.get(key) || 0;
|
||||
if (remaining) oldWallCounts.set(key, remaining - 1);
|
||||
else changedWalls.push(wall);
|
||||
}
|
||||
if (wallRecordCarrierViolations(
|
||||
changedWalls, wallCarriers, this._wallKeyPitch, NORM_W, s.walls || [],
|
||||
).length) return { ok: false, reason: 'wall-metadata' };
|
||||
if (!this._rszSpaceCandidateRenderable(this._space, sp)) {
|
||||
return { ok: false, reason: 'physical-geometry' };
|
||||
}
|
||||
|
||||
+218
-10
@@ -9,6 +9,7 @@
|
||||
import { union, difference, intersection } from 'polyclip-ts';
|
||||
import { polygonArea, roomPoly, roomEdges, sharedBoundary, paperRoomShapes } from './logic';
|
||||
import { NEAR_AXIS_MAX_DEGREES } from './near-axis';
|
||||
import { LATTICE_NOISE_STEPS } from './coordinate-canonicalization';
|
||||
|
||||
export interface WallEntry {
|
||||
key: string;
|
||||
@@ -530,15 +531,28 @@ export function wallAngleMatches(
|
||||
* so an exact whole-edge key map is insufficient: project every unmatched key
|
||||
* onto the old edge and carry that relative point onto the new one.
|
||||
*/
|
||||
export function rekeyWallsAfterMove(
|
||||
export type WallRekeyMode = 'affine' | 'fixed-topology';
|
||||
|
||||
export interface WallRekeyResult {
|
||||
walls: WallEntry[];
|
||||
/** Fixed-topology candidate is unsafe and must not reach preview/commit. */
|
||||
rejected: boolean;
|
||||
}
|
||||
|
||||
function rekeyWallsAfterMoveInternal(
|
||||
walls: WallEntry[] | null | undefined,
|
||||
oldSpans: [number[], number[]][],
|
||||
newSpans: [number[], number[]][],
|
||||
pitch: number,
|
||||
coordScale = 1,
|
||||
mode: WallRekeyMode = 'affine',
|
||||
reject?: () => void,
|
||||
): WallEntry[] {
|
||||
if (!walls?.length) return [];
|
||||
if (oldSpans.length !== newSpans.length) return walls.slice();
|
||||
if (oldSpans.length !== newSpans.length) {
|
||||
if (mode === 'fixed-topology') reject?.();
|
||||
return walls.slice();
|
||||
}
|
||||
const scale = coordScale > 0 ? coordScale : 1;
|
||||
const tol = Math.max(pitch * 0.5, 1e-9) * scale;
|
||||
const exactEps = Math.max(pitch * scale * 1e-6, 1e-9);
|
||||
@@ -548,6 +562,12 @@ export function rekeyWallsAfterMove(
|
||||
};
|
||||
const moves: Move[] = [];
|
||||
const keyMoves = new Map<string, Set<string>>();
|
||||
const wholeEdgeMoves = new Map<string, Set<string>>();
|
||||
const addKeyMove = (map: Map<string, Set<string>>, from: string, to: string): void => {
|
||||
const targets = map.get(from) || new Set<string>();
|
||||
targets.add(to);
|
||||
map.set(from, targets);
|
||||
};
|
||||
for (let i = 0; i < oldSpans.length; i++) {
|
||||
const [oa, ob] = oldSpans[i];
|
||||
const [na, nb] = newSpans[i];
|
||||
@@ -563,10 +583,14 @@ export function rekeyWallsAfterMove(
|
||||
moves.push({ oa, ob, na, nb, dx, dy, len2 });
|
||||
const ok = keyOf(oa, ob, pitch, coordScale);
|
||||
const nk = keyOf(na, nb, pitch, coordScale);
|
||||
// Some pre-normalisation configurations carry render-space legacy keys.
|
||||
// They have no endpoints with which to disambiguate storage generations,
|
||||
// so recognise only the same whole-edge identity in either historical
|
||||
// coordinate convention. Partial midpoint projection remains forbidden.
|
||||
addKeyMove(wholeEdgeMoves, ok, nk);
|
||||
addKeyMove(wholeEdgeMoves, keyOf(oa, ob, pitch, 1), keyOf(na, nb, pitch, 1));
|
||||
if (ok !== nk) {
|
||||
const targets = keyMoves.get(ok) || new Set<string>();
|
||||
targets.add(nk);
|
||||
keyMoves.set(ok, targets);
|
||||
addKeyMove(keyMoves, ok, nk);
|
||||
}
|
||||
}
|
||||
if (!moves.length) return walls.slice();
|
||||
@@ -575,13 +599,32 @@ export function rekeyWallsAfterMove(
|
||||
a[0] + (b[0] - a[0]) * t,
|
||||
a[1] + (b[1] - a[1]) * t,
|
||||
];
|
||||
const closePoint = (a: number[], b: number[]): boolean =>
|
||||
Math.hypot(a[0] - b[0], a[1] - b[1]) <= exactEps;
|
||||
const mapPoint = (p: number[], move: Move): number[] => {
|
||||
if (mode === 'fixed-topology') {
|
||||
const adx = move.na[0] - move.oa[0], ady = move.na[1] - move.oa[1];
|
||||
const bdx = move.nb[0] - move.ob[0], bdy = move.nb[1] - move.ob[1];
|
||||
// Safe Resize has only two legal transforms for one source edge:
|
||||
//
|
||||
// - the moving wall translates rigidly, so every physical breakpoint
|
||||
// follows by the same vector;
|
||||
// - a perpendicular side wall changes length, so only its topology
|
||||
// endpoint moves and interior thickness breakpoints stay put.
|
||||
//
|
||||
// Reusing the historical affine `t` mapping for the second case is the
|
||||
// producer behind #298: it creates a point that belongs to no polygon.
|
||||
if (Math.hypot(adx - bdx, ady - bdy) <= exactEps) {
|
||||
return [p[0] + adx, p[1] + ady];
|
||||
}
|
||||
if (closePoint(p, move.oa)) return [...move.na];
|
||||
if (closePoint(p, move.ob)) return [...move.nb];
|
||||
return [...p];
|
||||
}
|
||||
const t = Math.max(0, Math.min(1,
|
||||
((p[0] - move.oa[0]) * move.dx + (p[1] - move.oa[1]) * move.dy) / move.len2));
|
||||
return pointAt(move.na, move.nb, t);
|
||||
};
|
||||
const closePoint = (a: number[], b: number[]): boolean =>
|
||||
Math.hypot(a[0] - b[0], a[1] - b[1]) <= exactEps;
|
||||
const canonicalSpan = (a: number[], b: number[]): [number[], number[]] => {
|
||||
const [ux, uy] = wallDir(a, b);
|
||||
return (b[0] - a[0]) * ux + (b[1] - a[1]) * uy >= 0
|
||||
@@ -633,7 +676,12 @@ export function rekeyWallsAfterMove(
|
||||
}
|
||||
|
||||
if (!overlaps.length) {
|
||||
pushExact(wa, wb, w.cm);
|
||||
// A fixed-topology edit is not allowed to canonicalise an unrelated
|
||||
// record as a side effect: its compatibility key and endpoints are
|
||||
// observable storage identity. The generic historical transform keeps
|
||||
// its previous normalising behaviour for isolated callers/tests.
|
||||
if (mode === 'fixed-topology') out.push({ ...w });
|
||||
else pushExact(wa, wb, w.cm);
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -688,6 +736,15 @@ export function rekeyWallsAfterMove(
|
||||
coalesced.push([[...atom[0]], [...atom[1]]]);
|
||||
}
|
||||
}
|
||||
if (mode === 'fixed-topology' && coalesced.length === 1) {
|
||||
const [nextA, nextB] = coalesced[0];
|
||||
const sameSpan = (closePoint(nextA, exact[0]) && closePoint(nextB, exact[1]))
|
||||
|| (closePoint(nextA, exact[1]) && closePoint(nextB, exact[0]));
|
||||
if (sameSpan) {
|
||||
out.push({ ...w });
|
||||
continue;
|
||||
}
|
||||
}
|
||||
for (const [a, b] of coalesced) {
|
||||
pushExact(a, b, w.cm);
|
||||
}
|
||||
@@ -695,8 +752,34 @@ export function rekeyWallsAfterMove(
|
||||
}
|
||||
|
||||
// Legacy entries carry only a midpoint/direction key, so they cannot be
|
||||
// split without inventing a length. Move an unambiguous whole-edge key or
|
||||
// projected midpoint, and never deduplicate them merely by key.
|
||||
// split without inventing a length. Safe Resize permits only an exact,
|
||||
// unambiguous whole-edge identity. A partial/ambiguous affected key rejects
|
||||
// the complete candidate; an unrelated key stays byte-equivalent.
|
||||
if (mode === 'fixed-topology') {
|
||||
const direct = wholeEdgeMoves.get(w.key);
|
||||
if (direct?.size === 1) {
|
||||
const key = [...direct][0];
|
||||
out.push(key === w.key ? { ...w } : { ...w, key });
|
||||
continue;
|
||||
}
|
||||
const parsedVariants = [parseKeys([w], scale)[0]];
|
||||
if (scale !== 1) parsedVariants.push(parseKeys([w], 1)[0]);
|
||||
const touchesChangedEdge = parsedVariants.filter(Boolean).some((parsed) =>
|
||||
moves.some((move) => {
|
||||
if (!angleClose(parsed!.ang, segAngle(move.oa, move.ob))) return false;
|
||||
const t = ((parsed!.x - move.oa[0]) * move.dx
|
||||
+ (parsed!.y - move.oa[1]) * move.dy) / move.len2;
|
||||
return t >= -1e-6 && t <= 1 + 1e-6
|
||||
&& distToSeg(parsed!.x, parsed!.y,
|
||||
move.oa[0], move.oa[1], move.ob[0], move.ob[1]) <= tol;
|
||||
}));
|
||||
if ((direct?.size || 0) > 1 || touchesChangedEdge) reject?.();
|
||||
out.push({ ...w });
|
||||
continue;
|
||||
}
|
||||
|
||||
// Historical affine transformations retain their projected-midpoint
|
||||
// compatibility behaviour outside production Safe Resize.
|
||||
let nk = '';
|
||||
const direct = keyMoves.get(w.key);
|
||||
if (direct?.size === 1) nk = [...direct][0];
|
||||
@@ -728,6 +811,131 @@ export function rekeyWallsAfterMove(
|
||||
return out;
|
||||
}
|
||||
|
||||
/** Historical array-only API retained for pure affine callers. */
|
||||
export function rekeyWallsAfterMove(
|
||||
walls: WallEntry[] | null | undefined,
|
||||
oldSpans: [number[], number[]][],
|
||||
newSpans: [number[], number[]][],
|
||||
pitch: number,
|
||||
coordScale = 1,
|
||||
mode: WallRekeyMode = 'affine',
|
||||
): WallEntry[] {
|
||||
return rekeyWallsAfterMoveInternal(
|
||||
walls, oldSpans, newSpans, pitch, coordScale, mode,
|
||||
);
|
||||
}
|
||||
|
||||
/** Production result: unsafe legacy correspondence is explicit and atomic. */
|
||||
export function rekeyWallsAfterMoveChecked(
|
||||
walls: WallEntry[] | null | undefined,
|
||||
oldSpans: [number[], number[]][],
|
||||
newSpans: [number[], number[]][],
|
||||
pitch: number,
|
||||
coordScale = 1,
|
||||
mode: WallRekeyMode = 'fixed-topology',
|
||||
): WallRekeyResult {
|
||||
let rejected = false;
|
||||
const next = rekeyWallsAfterMoveInternal(
|
||||
walls, oldSpans, newSpans, pitch, coordScale, mode,
|
||||
() => { rejected = true; },
|
||||
);
|
||||
return { walls: rejected ? (walls || []).map((wall) => ({ ...wall })) : next, rejected };
|
||||
}
|
||||
|
||||
/**
|
||||
* Fail-closed carrier/lattice proof for exact wall records after Safe Resize.
|
||||
*
|
||||
* A compact record may cross several collinear room edges, so checking that
|
||||
* both endpoints touch one edge is insufficient. Project every collinear
|
||||
* room-wall carrier onto the record and require their union to cover its full
|
||||
* interval without gaps. Independent partitions are not carriers for
|
||||
* `space.walls`. Legacy key-only records have no provable extent and remain a
|
||||
* compatibility concern of `rekeyWallsAfterMoveChecked`.
|
||||
*/
|
||||
export function wallRecordCarrierViolations(
|
||||
walls: WallEntry[] | null | undefined,
|
||||
carriers: [number[], number[]][],
|
||||
pitch: number,
|
||||
coordScale = 1,
|
||||
latticeDebt: WallEntry[] | null | undefined = [],
|
||||
): string[] {
|
||||
const scale = coordScale > 0 ? coordScale : 1;
|
||||
const latticePitch = Math.abs(pitch);
|
||||
const eps = Math.max(latticePitch * scale * LATTICE_NOISE_STEPS, 1e-9);
|
||||
const onLattice = (value: number): boolean => {
|
||||
if (!(latticePitch > 0)) return true;
|
||||
const normalised = value / scale;
|
||||
const steps = normalised / latticePitch;
|
||||
return Math.abs(steps - Math.round(steps)) < LATTICE_NOISE_STEPS;
|
||||
};
|
||||
// A Resize may need to rewrite a record whose other endpoint was authored
|
||||
// off-grid historically. That coordinate is not newly produced by Resize:
|
||||
// allow it only when the exact same physical endpoint already existed in the
|
||||
// immutable source snapshot. Carrier coverage is still proved below.
|
||||
const oldEndpoints = (latticeDebt || []).flatMap((wall) => {
|
||||
const span = entrySpan(wall, scale);
|
||||
return span ? span.map((point) => [...point]) : [];
|
||||
});
|
||||
const pointIsLatticeSafe = (point: number[]): boolean => point.every((value, axis) =>
|
||||
onLattice(value) || oldEndpoints.some((old) => Math.abs(value - old[axis]) <= eps));
|
||||
|
||||
const violations: string[] = [];
|
||||
const signature = (wall: WallEntry): string => JSON.stringify([
|
||||
wall.key, wall.cm, wall.a, wall.b,
|
||||
]);
|
||||
for (const wall of walls || []) {
|
||||
const span = entrySpan(wall, scale);
|
||||
if (!span) continue;
|
||||
const [a, b] = span;
|
||||
if (![a[0], a[1], b[0], b[1]].every(Number.isFinite)
|
||||
|| !pointIsLatticeSafe(a) || !pointIsLatticeSafe(b)) {
|
||||
violations.push(signature(wall));
|
||||
continue;
|
||||
}
|
||||
const dx = b[0] - a[0], dy = b[1] - a[1];
|
||||
const length = Math.hypot(dx, dy);
|
||||
if (length <= eps) {
|
||||
violations.push(signature(wall));
|
||||
continue;
|
||||
}
|
||||
const ux = dx / length, uy = dy / length;
|
||||
const intervals: [number, number][] = [];
|
||||
for (const carrier of carriers) {
|
||||
const [ca, cb] = carrier;
|
||||
if (![ca?.[0], ca?.[1], cb?.[0], cb?.[1]].every(Number.isFinite)) continue;
|
||||
const lineDistance = (point: number[]): number =>
|
||||
Math.abs((point[0] - a[0]) * uy - (point[1] - a[1]) * ux);
|
||||
if (lineDistance(ca) > eps || lineDistance(cb) > eps) continue;
|
||||
const ta = (ca[0] - a[0]) * ux + (ca[1] - a[1]) * uy;
|
||||
const tb = (cb[0] - a[0]) * ux + (cb[1] - a[1]) * uy;
|
||||
const lo = Math.max(0, Math.min(ta, tb));
|
||||
const hi = Math.min(length, Math.max(ta, tb));
|
||||
if (hi - lo > eps) intervals.push([lo, hi]);
|
||||
}
|
||||
intervals.sort((left, right) => left[0] - right[0] || left[1] - right[1]);
|
||||
let covered = 0;
|
||||
for (const [lo, hi] of intervals) {
|
||||
if (lo > covered + eps) break;
|
||||
covered = Math.max(covered, hi);
|
||||
if (covered >= length - eps) break;
|
||||
}
|
||||
if (covered < length - eps) violations.push(signature(wall));
|
||||
}
|
||||
return violations;
|
||||
}
|
||||
|
||||
export function wallRecordsHaveCarrierCoverage(
|
||||
walls: WallEntry[] | null | undefined,
|
||||
carriers: [number[], number[]][],
|
||||
pitch: number,
|
||||
coordScale = 1,
|
||||
latticeDebt: WallEntry[] | null | undefined = [],
|
||||
): boolean {
|
||||
return wallRecordCarrierViolations(
|
||||
walls, carriers, pitch, coordScale, latticeDebt,
|
||||
).length === 0;
|
||||
}
|
||||
|
||||
/** Upsert or remove a wall entry by endpoints. */
|
||||
export function setWallThickness(
|
||||
walls: WallEntry[] | null | undefined,
|
||||
|
||||
@@ -76,6 +76,16 @@ test('#277 a lossy persistence rekey stops at the last complete preview', () =>
|
||||
assert.match(card, /g\.d = previousD/);
|
||||
});
|
||||
|
||||
test('#298 production carrier proof uses room edges, never independent partitions', () => {
|
||||
const carrierBlock = card.match(
|
||||
/const wallCarriers: \[number\[\], number\[\]\]\[\] = \[\];[\s\S]*?const wallSignature/,
|
||||
);
|
||||
assert.ok(carrierBlock, 'production wall-carrier preflight is missing');
|
||||
assert.match(carrierBlock[0], /for \(const room of sp\.rooms \|\| \[\]\)/);
|
||||
assert.doesNotMatch(carrierBlock[0], /sp\.partitions/,
|
||||
'independent partition geometry cannot carry space.walls metadata');
|
||||
});
|
||||
|
||||
test('#277 Resize render fingerprints geometry once for the whole handle layer', () => {
|
||||
assert.match(card, /const renderSnapshot = this\._rszDrag\?\.snap \|\| this\._rszSnapshot\(\)/);
|
||||
assert.match(card, /this\._rszResolution\(r\.id, i, renderSnapshot\)/);
|
||||
|
||||
@@ -4,6 +4,8 @@ import assert from 'node:assert/strict';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import {
|
||||
wallKey, lookupWall, thicknessCmAt, degradeWalls, rekeyWallsAfterMove,
|
||||
rekeyWallsAfterMoveChecked,
|
||||
wallRecordsHaveCarrierCoverage,
|
||||
setWallThickness, setWallThicknessForRoom, applyWallThicknessToNewRoom,
|
||||
drawWallPreviewD, linearWallBody, linearWallJoinPatches,
|
||||
DRAW_WALL_DEFAULT_CM, clampWallCm, cmToField, fieldToCm,
|
||||
@@ -607,6 +609,192 @@ test('issue 293 moving a shared seam keeps one continuous side-wall record', ()
|
||||
assert.equal(bent.length, 2, 'meeting atoms with different directions must remain losslessly split');
|
||||
});
|
||||
|
||||
test('issue 298 fixed-topology rekey never scales an interior side-wall endpoint', () => {
|
||||
const wall = setWallThickness([], [-85, 304], [577, 304], 20, pitch, 1000);
|
||||
const next = rekeyWallsAfterMove(
|
||||
wall,
|
||||
[[[-100, 304], [100, 304]]],
|
||||
[[[-96, 304], [100, 304]]],
|
||||
pitch,
|
||||
1000,
|
||||
'fixed-topology',
|
||||
);
|
||||
assert.equal(next.length, 1);
|
||||
assert.deepEqual([next[0].a, next[0].b], [wall[0].a, wall[0].b]);
|
||||
assert.equal(next[0].key, wall[0].key);
|
||||
|
||||
const affine = rekeyWallsAfterMove(
|
||||
wall,
|
||||
[[[-100, 304], [100, 304]]],
|
||||
[[[-96, 304], [100, 304]]],
|
||||
pitch,
|
||||
1000,
|
||||
);
|
||||
assert.notDeepEqual(affine[0].a, wall[0].a,
|
||||
'regression fixture must kill the historical proportional mapping');
|
||||
});
|
||||
|
||||
test('issue 298 fixed-topology rekey translates every breakpoint of the moving wall', () => {
|
||||
const wall = setWallThickness([], [20, 0], [80, 0], 25, pitch, 1000);
|
||||
const next = rekeyWallsAfterMove(
|
||||
wall,
|
||||
[[[0, 0], [100, 0]]],
|
||||
[[[0, 5], [100, 5]]],
|
||||
pitch,
|
||||
1000,
|
||||
'fixed-topology',
|
||||
);
|
||||
assert.deepEqual([next[0].a, next[0].b], [[0.02, 0.005], [0.08, 0.005]]);
|
||||
assert.equal(next[0].cm, 25);
|
||||
});
|
||||
|
||||
test('issue 298 fixed-topology rekey preserves an unrelated exact record byte-semantically', () => {
|
||||
const wall = {
|
||||
key: 'compatibility-key-that-must-not-change', cm: 21,
|
||||
a: [0, 0], b: [0.1, 0],
|
||||
};
|
||||
const next = rekeyWallsAfterMove(
|
||||
[wall],
|
||||
[[[0, 200], [100, 200]]],
|
||||
[[[0, 205], [100, 205]]],
|
||||
pitch,
|
||||
1000,
|
||||
'fixed-topology',
|
||||
);
|
||||
assert.deepEqual(next, [wall]);
|
||||
});
|
||||
|
||||
test('issue 298 carrier proof covers collinear chains and rejects gaps or off-grid endpoints', () => {
|
||||
const exact = setWallThickness([], [0, 0], [100, 0], 20, pitch, 1000);
|
||||
assert.equal(wallRecordsHaveCarrierCoverage(
|
||||
exact, [[[0, 0], [40, 0]], [[40, 0], [100, 0]]], pitch, 1000,
|
||||
), true);
|
||||
assert.equal(wallRecordsHaveCarrierCoverage(
|
||||
exact, [[[0, 0], [40, 0]], [[41, 0], [100, 0]]], pitch, 1000,
|
||||
), false, 'endpoint-only validation must not accept a carrier gap');
|
||||
|
||||
const offGrid = [{ ...exact[0], a: [0.00001, 0] }];
|
||||
assert.equal(wallRecordsHaveCarrierCoverage(
|
||||
offGrid, [[[0.01, 0], [100, 0]]], pitch, 1000,
|
||||
), false, 'a true off-grid coordinate is not storage noise');
|
||||
assert.equal(wallRecordsHaveCarrierCoverage(
|
||||
offGrid, [[[0.01, 0], [100, 0]]], pitch, 1000, offGrid,
|
||||
), true, 'an identical historical endpoint is debt, not a new Resize coordinate');
|
||||
assert.equal(wallRecordsHaveCarrierCoverage(
|
||||
[{ ...offGrid[0], a: [0.00002, 0] }],
|
||||
[[[0.02, 0], [100, 0]]], pitch, 1000, offGrid,
|
||||
), false, 'Resize may not replace old debt with a different off-grid endpoint');
|
||||
});
|
||||
|
||||
test('issue 298 fixed-topology legacy records move only by unambiguous whole-edge identity', () => {
|
||||
const oldEdge = [[0, 0], [80, 0]];
|
||||
const newEdge = [[0, 0], [92, 0]];
|
||||
const whole = { key: wallKey(...oldEdge, pitch), cm: 22 };
|
||||
const moved = rekeyWallsAfterMoveChecked(
|
||||
[whole], [oldEdge], [newEdge], pitch, 1, 'fixed-topology',
|
||||
);
|
||||
assert.equal(moved.rejected, false);
|
||||
assert.deepEqual(moved.walls, [{ ...whole, key: wallKey(...newEdge, pitch) }]);
|
||||
|
||||
const renderSpaceWhole = {
|
||||
key: wallKey([1 / 480, 0], [80 - 1 / 480, 0], pitch), cm: 22,
|
||||
};
|
||||
const renderMoved = rekeyWallsAfterMoveChecked(
|
||||
[renderSpaceWhole], [oldEdge], [newEdge], pitch, 1000, 'fixed-topology',
|
||||
);
|
||||
assert.equal(renderMoved.rejected, false, 'historical render-space whole-edge key is unambiguous');
|
||||
assert.equal(renderMoved.walls[0].key, wallKey(...newEdge, pitch));
|
||||
|
||||
const untouched = { key: wallKey([100, 10], [120, 10], pitch), cm: 19, future: 'kept' };
|
||||
const untouchedResult = rekeyWallsAfterMoveChecked(
|
||||
[untouched], [oldEdge], [newEdge], pitch, 1, 'fixed-topology',
|
||||
);
|
||||
assert.equal(untouchedResult.rejected, false);
|
||||
assert.deepEqual(untouchedResult.walls, [untouched]);
|
||||
|
||||
const partial = { key: wallKey([10, 0], [30, 0], pitch), cm: 21 };
|
||||
const ambiguous = rekeyWallsAfterMoveChecked(
|
||||
[partial], [oldEdge], [newEdge], pitch, 1, 'fixed-topology',
|
||||
);
|
||||
assert.equal(ambiguous.rejected, true);
|
||||
assert.deepEqual(ambiguous.walls, [partial], 'a rejected candidate cannot partially rekey storage');
|
||||
|
||||
const conflictingWhole = rekeyWallsAfterMoveChecked(
|
||||
[whole], [oldEdge, oldEdge], [newEdge, [[0, 0], [96, 0]]],
|
||||
pitch, 1, 'fixed-topology',
|
||||
);
|
||||
assert.equal(conflictingWhole.rejected, true,
|
||||
'one whole-edge key with conflicting destinations must reject atomically');
|
||||
assert.deepEqual(conflictingWhole.walls, [whole]);
|
||||
|
||||
const exactWithBadCompatibilityKey = {
|
||||
key: 'stale-compatibility-key', cm: 24, a: [0, 0], b: [0.08, 0],
|
||||
};
|
||||
const exact = rekeyWallsAfterMoveChecked(
|
||||
[exactWithBadCompatibilityKey], [oldEdge], [newEdge], pitch, 1000, 'fixed-topology',
|
||||
);
|
||||
assert.equal(exact.rejected, false);
|
||||
assert.deepEqual(exact.walls[0].b, [0.092, 0]);
|
||||
assert.equal(exact.walls[0].key, wallKey([0, 0], [0.092, 0], pitch));
|
||||
});
|
||||
|
||||
test('issue 298 first-floor fixture keeps the room-b seam on 17/52/57/101 boundaries', () => {
|
||||
const fixture = JSON.parse(readFileSync(new URL(
|
||||
'./fixtures/real-plan-first-floor.json', import.meta.url,
|
||||
), 'utf8')).space;
|
||||
const changedIds = new Set(['room-b', 'room-g']);
|
||||
const nextRooms = fixture.rooms.map((room) => {
|
||||
const copy = structuredClone(room);
|
||||
if (copy.id === 'room-b') copy.poly[0][0] = copy.poly[1][0] = 52 / 240;
|
||||
if (copy.id === 'room-g') copy.poly[2][0] = copy.poly[3][0] = 52 / 240;
|
||||
return copy;
|
||||
});
|
||||
const oldSpans = [];
|
||||
const newSpans = [];
|
||||
for (const oldRoom of fixture.rooms.filter((room) => changedIds.has(room.id))) {
|
||||
const nextRoom = nextRooms.find((room) => room.id === oldRoom.id);
|
||||
const oldPoly = oldRoom.poly.map((point) => point.map((value) => value * 1000));
|
||||
const newPoly = nextRoom.poly.map((point) => point.map((value) => value * 1000));
|
||||
for (let index = 0; index < oldPoly.length; index++) {
|
||||
oldSpans.push([oldPoly[index], oldPoly[(index + 1) % oldPoly.length]]);
|
||||
newSpans.push([newPoly[index], newPoly[(index + 1) % newPoly.length]]);
|
||||
}
|
||||
}
|
||||
|
||||
const result = rekeyWallsAfterMoveChecked(
|
||||
fixture.walls, oldSpans, newSpans, pitch, 1000, 'fixed-topology',
|
||||
);
|
||||
assert.equal(result.rejected, false);
|
||||
const next = result.walls;
|
||||
const step = (value) => Number((value * 240).toFixed(6));
|
||||
const y155 = next.filter((wall) => wall.a && wall.b
|
||||
&& Math.abs(step(wall.a[1]) - 155) < 0.001
|
||||
&& Math.abs(step(wall.b[1]) - 155) < 0.001);
|
||||
const spans = y155.map((wall) => [step(wall.a[0]), step(wall.b[0])].sort((a, b) => a - b))
|
||||
.sort((a, b) => a[0] - b[0]);
|
||||
assert.deepEqual(spans, [[17, 57], [52, 101]]);
|
||||
assert.equal(spans.flat().some((value) => Math.abs(value - 59.538461) < 0.001), false);
|
||||
|
||||
const untouched = fixture.walls.find((wall) => wall.key === '0.479167,0.050000@0.0000');
|
||||
assert.ok(next.some((wall) => JSON.stringify(wall) === JSON.stringify(untouched)),
|
||||
'an unrelated first-floor record stays byte-semantic');
|
||||
|
||||
const carriers = nextRooms.flatMap((room) => room.poly.map((point, index) => [
|
||||
point.map((value) => value * 1000),
|
||||
room.poly[(index + 1) % room.poly.length].map((value) => value * 1000),
|
||||
]));
|
||||
assert.equal(wallRecordsHaveCarrierCoverage(
|
||||
y155, carriers, pitch, 1000, fixture.walls,
|
||||
), true, 'all surviving first-floor endpoints remain lattice/carrier safe');
|
||||
|
||||
const affine = rekeyWallsAfterMove(
|
||||
fixture.walls, oldSpans, newSpans, pitch, 1000, 'affine',
|
||||
);
|
||||
assert.equal(affine.some((wall) => [wall.a, wall.b].flat()
|
||||
.some((value) => Math.abs(step(value) - 59.538461) < 0.001)), true,
|
||||
'fixture must kill the historical proportional endpoint mapping');
|
||||
});
|
||||
|
||||
test('issue 253 key collisions never erase different exact or legacy records', () => {
|
||||
const exact = [
|
||||
{ key: wallKey([-1, 0], [1, 0], pitch), cm: 20, a: [-1, 0], b: [1, 0] },
|
||||
|
||||
Reference in New Issue
Block a user