From 0bb42caeff107827ddca15a7bb653c5cdc97f860 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 28 Aug 2026 00:23:54 +0300 Subject: [PATCH] fix: the backend judges both sides after the same migration (#329 H1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The limits read `wall_segments`, so a document older than the catalogue reports no walls at all — and therefore no violations, whatever its geometry. Comparing that raw baseline against a candidate the card had already migrated counted every inherited violation as new, and a legacy plan could not take an unrelated edit at all: renaming a room was refused with junction_limit_angle. Spec §3 forbids exactly this, and the frontend had already learned the same lesson in 4758767e; the backend mirror simply never got the second half. validate_junction_limits now runs both documents through commit_wall_segment_model before counting. A document that cannot be migrated is not this validator's verdict — the wall-model barrier owns that error and reports it with its own code — so it degrades to "no baseline to inherit". The regression is pinned twice: a test that asserts the legacy baseline reads clean raw and carries the apex once migrated, and the mutant junction-limit-backend-raw-baseline. Both fixtures that exercise the barrier were rebuilt as real documents (rooms plus walls), because the previous ones put walls in wall_segments with no rooms and did not survive migration. Issue: #329 User-Visible: no --- .../houseplan/junction_limits.py | 41 ++++++- docs/CONFIG-COMPATIBILITY.md | 16 ++- scripts/mutation-gate.mjs | 12 ++ tests_backend/test_junction_limits.py | 116 ++++++++++++++---- 4 files changed, 154 insertions(+), 31 deletions(-) diff --git a/custom_components/houseplan/junction_limits.py b/custom_components/houseplan/junction_limits.py index 226651e0..ab81b66b 100644 --- a/custom_components/houseplan/junction_limits.py +++ b/custom_components/houseplan/junction_limits.py @@ -18,6 +18,8 @@ from __future__ import annotations import math +from .wall_segment_model import commit_wall_segment_model + MIN_JUNCTION_ANGLE_DEG = 15.0 MAX_JUNCTION_VALENCE = 6 MIN_SEGMENT_LENGTH_CM = 20.0 @@ -254,6 +256,33 @@ def space_violations(space: dict) -> list[tuple[str, str, float, float]]: ] +def _migrated_spaces(config: dict | None) -> dict[str, dict]: + """Spaces of one document AFTER the wall-segment migration, by id. + + The limits read `wall_segments`, so a document that predates the catalogue + reports NO walls at all — a legacy space would answer "no violations" + regardless of its geometry. Comparing such a baseline against a candidate + the client already migrated counts every inherited violation as new and + refuses an unrelated edit (spec §3 forbids exactly that). Both sides are + therefore judged after the SAME migration, mirroring the frontend barrier. + + A document that cannot be migrated is not a reason to refuse the write: + the wall-model barrier owns that verdict and reports it with its own code. + Here it simply means there is no baseline to inherit from. + """ + if not isinstance(config, dict): + return {} + try: + migrated, _ = commit_wall_segment_model(config) + except Exception: # noqa: BLE001 — see the docstring: not our verdict + migrated = config + return { + str(space.get("id")): space + for space in (migrated or {}).get("spaces") or [] + if isinstance(space, dict) + } + + def validate_junction_limits(config: dict, previous: dict | None = None) -> None: """Refuse a write that ADDS a junction violation; inherit the rest. @@ -262,13 +291,13 @@ def validate_junction_limits(config: dict, previous: dict | None = None) -> None barrier and matching by it would report an inherited violation as new (the mistake that refused legitimate resizes on the frontend). Spec §3: a write may keep existing violations, it may never add one. + + Both documents go through `commit_wall_segment_model` first, so a legacy + baseline is compared in the same terms as the candidate. """ - old_spaces = { - str(space.get("id")): space - for space in (previous or {}).get("spaces") or [] - } - for space in config.get("spaces") or []: - space_id = str(space.get("id", "")) + old_spaces = _migrated_spaces(previous) + new_spaces = _migrated_spaces(config) + for space_id, space in new_spaces.items(): old_space = old_spaces.get(space_id) if old_space is None: # A brand-new space has nothing to inherit from — but neither is a diff --git a/docs/CONFIG-COMPATIBILITY.md b/docs/CONFIG-COMPATIBILITY.md index e67d02ed..d1fc5e83 100644 --- a/docs/CONFIG-COMPATIBILITY.md +++ b/docs/CONFIG-COMPATIBILITY.md @@ -442,10 +442,18 @@ transfer never run the check, and an edit that does not touch the offending element still saves. The gate compares the candidate against the pre-edit document **after both -have gone through the same `commitWallSegmentModel` migration**, and counts -violations **per rule**, not per subject id — a structural write re-keys -contour atoms, so subject identity is not stable across the barrier. Only a -rule whose violation count grows is a refusal. +have gone through the same wall-segment migration** — `commitWallSegmentModel` +on the card, `commit_wall_segment_model` in +`custom_components/houseplan/junction_limits.py` — and counts violations **per +rule**, not per subject id: a structural write re-keys contour atoms, so +subject identity is not stable across the barrier. Only a rule whose violation +count grows is a refusal. + +Migrating the baseline is not a detail. The limits read `wall_segments`, so a +document older than the catalogue reports no walls at all and therefore no +violations, whatever its geometry. Judged raw, such a baseline turns every +inherited violation of a real plan into a "new" one on the first structural +write after the card is updated, and an unrelated edit is refused. Compatibility matrix: diff --git a/scripts/mutation-gate.mjs b/scripts/mutation-gate.mjs index 85da3068..436ce2e3 100644 --- a/scripts/mutation-gate.mjs +++ b/scripts/mutation-gate.mjs @@ -537,6 +537,18 @@ export const MUTANTS = [ replace: ' // mutant: pointer capture removed\n const plan = resolution.plan;', }], }, + { + id: 'junction-limit-backend-raw-baseline', + guard: 'python3 -m pytest tests_backend/test_junction_limits.py -q', + because: 'a legacy baseline carries no wall catalogue, so judging it raw reports "no ' + + 'violations" whatever its geometry and turns every inherited one into a refusal of ' + + 'an unrelated edit (#329 §3, code review r1 H1)', + patches: [{ + file: 'custom_components/houseplan/junction_limits.py', + find: ' migrated, _ = commit_wall_segment_model(config)', + replace: ' migrated = config # mutant: judge the raw document', + }], + }, { id: 'junction-limit-angle-not-enforced', guard: 'npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs ' diff --git a/tests_backend/test_junction_limits.py b/tests_backend/test_junction_limits.py index f9d4d512..0c4fc693 100644 --- a/tests_backend/test_junction_limits.py +++ b/tests_backend/test_junction_limits.py @@ -28,9 +28,15 @@ if "custom_components.houseplan" not in sys.modules: package.__path__ = [_HOUSEPLAN_ROOT] sys.modules["custom_components.houseplan"] = package +# Loaded under its canonical package name: the module imports the migration +# mirror relatively (`from .wall_segment_model import ...`), which only resolves +# when the module knows the package it belongs to. _PATH = os.path.join(_HOUSEPLAN_ROOT, "junction_limits.py") -_spec = importlib.util.spec_from_file_location("hp_junction_limits", _PATH) +_spec = importlib.util.spec_from_file_location( + "custom_components.houseplan.junction_limits", _PATH, +) jl = importlib.util.module_from_spec(_spec) +sys.modules[_spec.name] = jl _spec.loader.exec_module(jl) CELL = 5.0 @@ -109,31 +115,99 @@ def test_distance_keeps_the_t_joint_legal(): assert "distance" not in rules(tee) +def room_space(rooms, walls, space_id="s", cell_cm=CELL, legacy=True): + """A space in the shape a real document has: rooms plus their walls. + + `legacy=True` stores the walls the pre-catalogue way (`walls`, no + `wall_segments`) — the state every plan is in before its first structural + write on a current card, and the state H1 was about. + """ + space = { + "id": space_id, "title": "L", "cell_cm": cell_cm, "view_box": [0, 0, 1, 1], + "rooms": rooms, "openings": [], "room_drafts": [], + "partitions": [], "wall_columns": [], + } + if legacy: + space["walls"] = walls + else: + space["wall_segments"] = walls + return space + + +def triangle(apex_x=0.3167, apex_y=0.24): + """The owner's spike: an apex well under 15°.""" + return [[0.30, 0.70], [apex_x, apex_y], [0.36, 0.68]] + + +def square(x=0.60, y=0.60, side=0.20): + return [[x, y], [x + side, y], [x + side, y + side], [x, y + side]] + + +def walls_of(poly, prefix, cm_value=15): + return [ + {"key": f"{prefix}{index}", "a": poly[index], + "b": poly[(index + 1) % len(poly)], "cm": cm_value} + for index in range(len(poly)) + ] + + +def test_legacy_baseline_is_judged_after_the_same_migration(): + """H1 (r1): a pre-catalogue baseline must not read as "no violations". + + `limit_segments` reads `wall_segments`, so a legacy space answers "clean" + whatever its geometry. Comparing that against a candidate the client has + already migrated counted every inherited violation as new and refused an + unrelated edit — spec §3 forbids exactly that. + """ + spike = triangle() + previous = {"spaces": [room_space( + [{"id": "r1", "name": "a", "area": None, "poly": spike}], + walls_of(spike, "w"), + )]} + # Raw, the legacy baseline claims to be clean... + assert rules(previous["spaces"][0]) == [] + # ...while the same document, migrated, carries the inherited apex. + migrated, _ = jl.commit_wall_segment_model(json.loads(json.dumps(previous))) + assert "angle" in rules(migrated["spaces"][0]) + + # An unrelated edit (renaming the room) on the migrated candidate passes. + candidate = json.loads(json.dumps(migrated)) + candidate["spaces"][0]["rooms"][0]["name"] = "b" + jl.validate_junction_limits(candidate, previous) + + def test_inherited_violation_does_not_block_an_unrelated_edit(): - broken = space([(*ray(0), 15), (*ray(9), 15)]) - previous = {"spaces": [broken]} - # The same broken space plus an unrelated, perfectly legal wall. - edited = json.loads(json.dumps(broken)) - edited["wall_segments"].append({ - "id": "unrelated", "a": [cm(1000), cm(1000)], - "b": [cm(1000), cm(1300)], "cm": 15, - }) - jl.validate_junction_limits({"spaces": [edited]}, previous) + spike = triangle() + previous = {"spaces": [room_space( + [{"id": "r1", "name": "a", "area": None, "poly": spike}], + walls_of(spike, "w"), + )]} + # Adding a well-formed room next to the broken one is a legal write. + box = square() + candidate = json.loads(json.dumps(previous)) + candidate["spaces"][0]["rooms"].append( + {"id": "r2", "name": "box", "area": None, "poly": box} + ) + candidate["spaces"][0]["walls"].extend(walls_of(box, "b")) + jl.validate_junction_limits(candidate, previous) def test_a_write_that_adds_a_violation_is_refused_with_a_stable_code(): - clean = space([ - ([0.0, 0.0], [cm(300), 0.0], 15), - ([cm(300), 0.0], [cm(300), cm(300)], 15), - ]) - previous = {"spaces": [clean]} - broken = json.loads(json.dumps(clean)) - broken["wall_segments"].append({ - "id": "spike", "a": [0.0, 0.0], - "b": [cm(300), cm(300) * 0.15], "cm": 15, - }) + box = square() + previous = {"spaces": [room_space( + [{"id": "r1", "name": "box", "area": None, "poly": box}], + walls_of(box, "b"), + )]} + jl.validate_junction_limits(json.loads(json.dumps(previous)), previous) + + spike = triangle() + candidate = json.loads(json.dumps(previous)) + candidate["spaces"][0]["rooms"].append( + {"id": "r2", "name": "spike", "area": None, "poly": spike} + ) + candidate["spaces"][0]["walls"].extend(walls_of(spike, "w")) with pytest.raises(jl.JunctionLimitError) as excinfo: - jl.validate_junction_limits({"spaces": [broken]}, previous) + jl.validate_junction_limits(candidate, previous) assert excinfo.value.code == "junction_limit_angle" assert excinfo.value.space_id == "s"