mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 19:58:50 +00:00
fix: the backend judges both sides after the same migration (#329 H1)
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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
|
||||
@@ -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 '
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user