mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-07 23:19:14 +00:00
fix: an opening without an in-place carrier migrates unhosted (#316, review r1 H1 + r2 M2)
CODE-REVIEW-316-r1 H1: the §3.3 degraded pool picked an angle-compatible wall at ANY distance, but the backend geometry-match invariant («wall opening geometry must match its host») requires the host to agree with the opening's own x/y — the migrated document was rejected by CONFIG_SCHEMA and the write wedged again on the schema layer. The pool is removed from both migrations (TS and the Python mirror): without an in-place eligible carrier the opening goes straight to the unhosted degraded state, exactly the alternative the spec's «assumed freely changeable» section reserved; the spec is revision 6. New tests replay the reviewer's reproduction on both sides, and the frontend test is proven able to fail by restoring the pool (executed red). CODE-REVIEW-316-r2 M2: the schema-level host check is shared with #132 partition openings, so its unhosted relaxation is now pinned by a regression test — a stale writer that keeps a partition-hosted opening but silently drops its host is still rejected by validate_partition_opening_hosts. Issue: #316 User-Visible: no
This commit is contained in:
File diff suppressed because one or more lines are too long
@@ -592,15 +592,12 @@ def _host_openings(
|
||||
str(segment.get("id", "")),
|
||||
))[0]
|
||||
|
||||
# #316 §3.2 tie-break, then the §3.3 degraded pool (angle- and
|
||||
# capacity-compatible positive walls at any distance, nearest first).
|
||||
degraded = [segment for segment in segments if (
|
||||
float(segment.get("cm", 0)) > 0
|
||||
and _angle_matches(segment["a"], segment["b"], angle)
|
||||
and half >= 0
|
||||
and _length(segment["a"], segment["b"]) + EPS >= half * 2
|
||||
)]
|
||||
carrier = pick([segment for segment in segments if eligible(segment)]) or pick(degraded)
|
||||
# #316 §3.2 tie-break. No distant fallback pool (CODE-REVIEW-316-r1
|
||||
# H1): a host away from the opening's own x/y would violate the
|
||||
# geometry-match invariant of CONFIG_SCHEMA and wedge the write on the
|
||||
# schema layer; without an in-place carrier the opening goes straight
|
||||
# to the unhosted degraded state.
|
||||
carrier = pick([segment for segment in segments if eligible(segment)])
|
||||
if carrier is not None:
|
||||
materialize(carrier)
|
||||
else:
|
||||
|
||||
Vendored
+2
-2
File diff suppressed because one or more lines are too long
@@ -2,7 +2,7 @@
|
||||
"version": 1,
|
||||
"fixture": "synthetic-only",
|
||||
"chromium": "151.0.7922.34",
|
||||
"sourceFingerprint": "1f20c9d64ca59363be3909accd069595b4fe18404ee7a28a5db479dac605661a",
|
||||
"sourceFingerprint": "80bd29ef9692067b706f221eaf8d62b4030e3a51b91bc9cff8d854aaee373700",
|
||||
"captureScriptSha256": "ce2e9542fed9dade3085be87d16f69adb2ac8262893ad78ad966b1b9673f2983",
|
||||
"command": "npm run build && node demo/docs/capture.mjs",
|
||||
"scenarios": {
|
||||
@@ -14,7 +14,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "1f20c9d64ca59363be3909accd069595b4fe18404ee7a28a5db479dac605661a",
|
||||
"sourceSha256": "80bd29ef9692067b706f221eaf8d62b4030e3a51b91bc9cff8d854aaee373700",
|
||||
"imageSha256": "9190bd86a07b0cb0502019d76d3286c0542181e1b4a4f862077d9cefa120613c"
|
||||
},
|
||||
"view-touch": {
|
||||
@@ -25,7 +25,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "1f20c9d64ca59363be3909accd069595b4fe18404ee7a28a5db479dac605661a",
|
||||
"sourceSha256": "80bd29ef9692067b706f221eaf8d62b4030e3a51b91bc9cff8d854aaee373700",
|
||||
"imageSha256": "cfa2f77eb68df69ae9fc9b51ffe2f14df01f7b74444fd2b038cadc1e9dfee22b"
|
||||
},
|
||||
"space-create": {
|
||||
@@ -36,7 +36,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "1f20c9d64ca59363be3909accd069595b4fe18404ee7a28a5db479dac605661a",
|
||||
"sourceSha256": "80bd29ef9692067b706f221eaf8d62b4030e3a51b91bc9cff8d854aaee373700",
|
||||
"imageSha256": "a86d4211af24923d5048e9094d0d35f9b28129394d00cd72582a8f8c2dd7ff75"
|
||||
},
|
||||
"room-contour-close": {
|
||||
@@ -47,7 +47,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "1f20c9d64ca59363be3909accd069595b4fe18404ee7a28a5db479dac605661a",
|
||||
"sourceSha256": "80bd29ef9692067b706f221eaf8d62b4030e3a51b91bc9cff8d854aaee373700",
|
||||
"imageSha256": "c7effd4ccd19bcaff9138458583368aaab488dcb6fc7ea5ab766383cb5377ac4"
|
||||
},
|
||||
"plan-context-tray": {
|
||||
@@ -58,7 +58,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "1f20c9d64ca59363be3909accd069595b4fe18404ee7a28a5db479dac605661a",
|
||||
"sourceSha256": "80bd29ef9692067b706f221eaf8d62b4030e3a51b91bc9cff8d854aaee373700",
|
||||
"imageSha256": "af1194f073f00b03af02b949a072cffb21bfd29995aa65a067eaaaec74ef8490"
|
||||
},
|
||||
"device-editor": {
|
||||
@@ -69,7 +69,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "1f20c9d64ca59363be3909accd069595b4fe18404ee7a28a5db479dac605661a",
|
||||
"sourceSha256": "80bd29ef9692067b706f221eaf8d62b4030e3a51b91bc9cff8d854aaee373700",
|
||||
"imageSha256": "36ed21b66920ff2de9e3346cf6f27c67e64a9f81692599b9c70d8373f5eb7241"
|
||||
},
|
||||
"device-display-preview": {
|
||||
@@ -80,7 +80,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "1f20c9d64ca59363be3909accd069595b4fe18404ee7a28a5db479dac605661a",
|
||||
"sourceSha256": "80bd29ef9692067b706f221eaf8d62b4030e3a51b91bc9cff8d854aaee373700",
|
||||
"imageSha256": "7151c96dc048381f7a7070ecb85a82a890b2f4c703628f14fa472eb8dacbf3dc"
|
||||
},
|
||||
"background-editor": {
|
||||
@@ -91,7 +91,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "1f20c9d64ca59363be3909accd069595b4fe18404ee7a28a5db479dac605661a",
|
||||
"sourceSha256": "80bd29ef9692067b706f221eaf8d62b4030e3a51b91bc9cff8d854aaee373700",
|
||||
"imageSha256": "9f08f3f3711b100d68fb1a05af269e43e47d8f8d12c0cbe4f9ab557138722989"
|
||||
},
|
||||
"room-card": {
|
||||
@@ -102,7 +102,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "1f20c9d64ca59363be3909accd069595b4fe18404ee7a28a5db479dac605661a",
|
||||
"sourceSha256": "80bd29ef9692067b706f221eaf8d62b4030e3a51b91bc9cff8d854aaee373700",
|
||||
"imageSha256": "66d8b0a509909822ff6b891a483bad9d2c72b20f08e287355883aa5205c6cee0"
|
||||
},
|
||||
"device-info": {
|
||||
@@ -113,7 +113,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "1f20c9d64ca59363be3909accd069595b4fe18404ee7a28a5db479dac605661a",
|
||||
"sourceSha256": "80bd29ef9692067b706f221eaf8d62b4030e3a51b91bc9cff8d854aaee373700",
|
||||
"imageSha256": "83620033bb66edf7c804619261e4a62327049c178267f0837bd997627ffdc003"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# Issue #316 — миграция v9 сама разрешает конфликт «проём ↔ нулевая стена»
|
||||
|
||||
Статус: ревизия 4 (r1: M1/M2/Low; r2: M3/M4; r3: M5). Родитель: #306 (стены нулевой толщины), контекст: #319
|
||||
Статус: ревизия 6 (spec-ревью r1–r4; код-ревью r1: H1 — §3.3 без дальнего поиска; r2: M2 — защита #132 закреплена тестом). Родитель: #306 (стены нулевой толщины), контекст: #319
|
||||
(бэкенд-гард — отдельная задача). Решение владельца (чат, 2026-08-26):
|
||||
конфликт проёма при миграции разрешается автоматически, рисование нигде не
|
||||
блокируется; выпуск — v1.68.0-beta.4 вместе с #319.
|
||||
@@ -56,10 +56,12 @@ t·L + length/2]` пересекает интервал атома на обще
|
||||
|
||||
### 3.3 Проём без носителя
|
||||
|
||||
Если кандидатов с `cm > 0` нет вовсе: берётся ближайший по расстоянию сегмент
|
||||
пространства, удовлетворяющий угловому критерию и вместимости, без ограничения
|
||||
допуска расстояния. Если и таких нет (в пространстве нет ни одного пригодного
|
||||
сегмента) — проём сохраняется в данных **без `host`** и становится непривязанным (unhosted):
|
||||
Если подходящих на месте кандидатов (`eligible`) нет — проём сразу сохраняется
|
||||
в данных **без `host`** и становится непривязанным (unhosted). Дальний поиск
|
||||
по углу/вместимости без ограничения расстояния (вариант ревизий 1–4) отклонён
|
||||
код-ревью r1 (H1): host вдали от собственных `x/y` проёма нарушает бэкенд-
|
||||
инвариант «wall opening geometry must match its host» и клинит запись на слое
|
||||
схемы — ровно тот класс отказа, который #316 устраняет. Непривязанный проём:
|
||||
не создаёт тоннель/вырез/тело, не участвует в физике, рендерится по своим
|
||||
`x/y` как прежде. `hostRoomOpenings` при initial migration пропускает такой
|
||||
проём вместо исключения; пост-v9 запись, не меняющая этот проём, обязана его
|
||||
@@ -121,7 +123,11 @@ t·L + length/2]` пересекает интервал атома на обще
|
||||
§3.1–3.4; он переписывается в том же коммите. Раздел «Independent-wall
|
||||
opening host (#132)» (партиционные проёмы) не затрагивается.
|
||||
- `tests_backend` — кейс схемы: v9-документ с контурным проёмом без host
|
||||
принимается на чтение и запись (AC4).
|
||||
принимается на чтение и запись (AC4). Разделяемая схема-проверка host при
|
||||
этом смягчается для обоих видов проёмов; защита #132 от молчаливого сброса
|
||||
ПАРТИЦИОННОГО host остаётся на семантическом слое
|
||||
(`validate_partition_opening_hosts` → `PartitionOpeningHostError`) и
|
||||
закрепляется регрессионным тестом (код-ревью r2, M2).
|
||||
- `demo/smoke_*` — новый смок репродукции AC1.
|
||||
|
||||
### 5.2 i18n
|
||||
@@ -159,9 +165,9 @@ Perf: правило 3.1 добавляет к атомизации один п
|
||||
|
||||
- Порядок tie-break в 3.2: текущий host → расстояние → больший `cm` →
|
||||
меньший `id`.
|
||||
- Семантика деградации 3.3: ближайший пригодный сегмент без лимита допуска, и
|
||||
лишь затем непривязанное состояние; альтернатива «сразу непривязанный без
|
||||
поиска» тоже согласуется с решением владельца.
|
||||
- Семантика деградации 3.3: принята альтернатива «сразу непривязанный без
|
||||
поиска» (код-ревью r1, H1 — дальний host противоречит инварианту схемы
|
||||
host↔x/y).
|
||||
- Имя состояния 3.3 в документации: «непривязанный (unhosted)».
|
||||
|
||||
## 7. Риски
|
||||
|
||||
@@ -690,15 +690,12 @@ const migrateRoomOpeningHost = (
|
||||
|| (x.id < y.id ? -1 : x.id > y.id ? 1 : 0)
|
||||
))[0];
|
||||
};
|
||||
// §3.3 degraded pool: angle- and capacity-compatible positive walls at any
|
||||
// distance, nearest first.
|
||||
const degraded = segments.filter((segment) => (
|
||||
Number(segment.cm) > 0
|
||||
&& wallAngleMatches(segment.a, segment.b, Number(opening.angle))
|
||||
&& Number.isFinite(half) && half >= 0
|
||||
&& lengthOf(segment.a, segment.b) + EPS >= Number(opening.length)
|
||||
));
|
||||
const host = pick(eligible) ?? pick(degraded);
|
||||
// §3.3 (CODE-REVIEW-316-r1 H1): no distant fallback pool. A host away from
|
||||
// the opening's own x/y would violate the backend geometry-match invariant
|
||||
// («wall opening geometry must match its host») and wedge the write on the
|
||||
// schema layer — the very failure mode #316 removes. An opening without an
|
||||
// in-place carrier goes straight to the unhosted degraded state.
|
||||
const host = pick(eligible);
|
||||
if (!host) return null;
|
||||
return {
|
||||
kind: 'wall', id: host.id,
|
||||
|
||||
@@ -473,6 +473,22 @@ test('an opening with no usable carrier migrates unhosted and survives later wri
|
||||
assert.equal(JSON.stringify(again), JSON.stringify(out));
|
||||
});
|
||||
|
||||
test('a far same-angle wall is NOT a degraded carrier — the opening migrates unhosted (#316 H1)', () => {
|
||||
// CODE-REVIEW-316-r1 H1: a host away from the opening's own x/y would break
|
||||
// the backend geometry-match invariant and wedge the write on the schema
|
||||
// layer. The opening must therefore stay unhosted, not adopt a distant wall.
|
||||
const config = spanDoorConfig();
|
||||
config.spaces[0].openings[0] = {
|
||||
id: 'far', type: 'door', x: 0.4, y: 0.45, angle: 0, length: 0.09, cm: 90,
|
||||
};
|
||||
delete config.spaces[0].open_spans;
|
||||
const out = commitWallSegmentModel(structuredClone(config)).config;
|
||||
const opening = out.spaces[0].openings[0];
|
||||
assert.equal('host' in opening, false, 'no distant fallback host');
|
||||
const again = commitWallSegmentModel(structuredClone(out)).config;
|
||||
assert.equal(JSON.stringify(again), JSON.stringify(out));
|
||||
});
|
||||
|
||||
test('a post-v9 write that LOST its carrier keeps the fail-closed refusal (#316 AC5)', () => {
|
||||
const migrated = commitWallSegmentModel(spanDoorConfig()).config;
|
||||
const broken = structuredClone(migrated);
|
||||
|
||||
@@ -10,7 +10,9 @@ import pytest
|
||||
|
||||
from custom_components.houseplan.validation import (
|
||||
CONFIG_SCHEMA,
|
||||
PartitionOpeningHostError,
|
||||
WallModelClientOutdatedError,
|
||||
validate_partition_opening_hosts,
|
||||
validate_wall_model_transition,
|
||||
)
|
||||
from custom_components.houseplan.wall_segment_model import (
|
||||
@@ -444,3 +446,53 @@ def test_post_v9_write_that_lost_its_carrier_keeps_the_refusal() -> None:
|
||||
}]
|
||||
with pytest.raises(WallSegmentMigrationError, match="opening-host"):
|
||||
commit_wall_segment_model(base)
|
||||
|
||||
|
||||
def test_far_same_angle_wall_is_not_a_degraded_carrier() -> None:
|
||||
"""CODE-REVIEW-316-r1 H1: a distant host would break the schema invariant
|
||||
«wall opening geometry must match its host» and wedge the write again.
|
||||
The opening must migrate unhosted and the migrated document must pass
|
||||
CONFIG_SCHEMA."""
|
||||
base, _ = commit_wall_segment_model(_config({
|
||||
"id": "floor", "rooms": [_room("room")],
|
||||
}))
|
||||
space = base["spaces"][0]
|
||||
for segment in space["wall_segments"]:
|
||||
segment["cm"] = 15
|
||||
space["walls"] = [{
|
||||
"key": f"legacy-{index}", "a": copy.deepcopy(segment["a"]),
|
||||
"b": copy.deepcopy(segment["b"]), "cm": 15,
|
||||
} for index, segment in enumerate(space["wall_segments"])]
|
||||
# Same angle as the bottom wall (y=0), but far away from every wall.
|
||||
space["openings"] = [{
|
||||
"id": "far", "type": "door", "x": 0.5, "y": 0.5,
|
||||
"angle": 0, "length": 0.2,
|
||||
}]
|
||||
base["model_version"] = 8
|
||||
migrated, _ = commit_wall_segment_model(base)
|
||||
opening = migrated["spaces"][0]["openings"][0]
|
||||
assert "host" not in opening
|
||||
validated = CONFIG_SCHEMA(copy.deepcopy(migrated))
|
||||
assert "host" not in validated["spaces"][0]["openings"][0]
|
||||
assert commit_wall_segment_model(migrated)[0] == migrated
|
||||
|
||||
|
||||
def test_dropping_a_partition_host_is_still_rejected_after_the_unhosted_relaxation() -> None:
|
||||
"""CODE-REVIEW-316-r2 M2: the schema now tolerates a missing host (the
|
||||
unhosted contour state of #316 §3.3), so the #132 protection against a
|
||||
stale writer silently dropping a PARTITION host must keep holding on the
|
||||
semantic layer — pinned here so a future relaxation cannot slip through.
|
||||
"""
|
||||
base, _ = commit_wall_segment_model(_config({
|
||||
"id": "floor", "rooms": [_room("room")],
|
||||
"partitions": [{"id": "p1", "a": [0.2, 0.5], "b": [0.8, 0.5], "cm": 10}],
|
||||
"openings": [{
|
||||
"id": "pdoor", "type": "door", "x": 0.5, "y": 0.5,
|
||||
"angle": 0, "length": 0.2,
|
||||
"host": {"kind": "partition", "id": "p1", "t": 0.5},
|
||||
}],
|
||||
}))
|
||||
stale = copy.deepcopy(base)
|
||||
del stale["spaces"][0]["openings"][0]["host"]
|
||||
with pytest.raises(PartitionOpeningHostError):
|
||||
validate_partition_opening_hosts(stale, base)
|
||||
|
||||
Reference in New Issue
Block a user