diff --git a/engineering/understand/diff.py b/engineering/understand/diff.py index da8bef7..71da263 100644 --- a/engineering/understand/diff.py +++ b/engineering/understand/diff.py @@ -66,6 +66,7 @@ padding are this module's declared defaults, not values from a standard. from __future__ import annotations import hashlib +import heapq import math import os from collections import defaultdict @@ -99,6 +100,10 @@ MOVE_FRACTION = 0.02 ROBUST_QUANTILE = 0.01 #: A cluster's box is padded by this share of ``move_tol``. PAD_FRACTION = 0.25 +#: The position stage keeps at most this many nearest old candidates per new +#: entity, so a dense drawing that moved as a whole (every entity left for this +#: stage, hundreds of neighbours within ``move_tol``) cannot build n x m pairs. +NEAREST_PER_ENTITY = 32 DEFAULT_LIMIT = 200 CATEGORIES = ("changed", "added", "removed") @@ -239,12 +244,14 @@ def _nearest_pairs( if b is None: continue cx, cy = math.floor(b[0] / move_tol), math.floor(b[1] / move_tol) + near: list[tuple[float, str, str, int, int]] = [] for dx in (-1, 0, 1): for dy in (-1, 0, 1): for i, a in grid.get((cx + dx, cy + dy), ()): distance = math.dist(a, b) if distance <= move_tol: - candidates.append((distance, olds[i].handle, news[j].handle, i, j)) + near.append((distance, olds[i].handle, news[j].handle, i, j)) + candidates.extend(heapq.nsmallest(NEAREST_PER_ENTITY, near)) candidates.sort() used_old: set[int] = set() used_new: set[int] = set() @@ -296,10 +303,18 @@ def _movement(o: EntityRecord, n: EntityRecord, tol: float) -> dict: return {"geometry": {"from": [list(p) for p in before], "to": [list(p) for p in after]}} dx, dy = b[0] - a[0], b[1] - a[1] half = tol / 2.0 - translated = len(before) == len(after) and all( - abs(p[0] + dx - q[0]) <= half and abs(p[1] + dy - q[1]) <= half - for p, q in zip(before, after, strict=True) - ) + + def translates(start: list, end: list) -> bool: + return len(start) == len(end) and all( + abs(p[0] + dx - q[0]) <= half and abs(p[1] + dy - q[1]) <= half + for p, q in zip(start, end, strict=True) + ) + + translated = translates(before, after) + if not translated and o.type == "LINE" and len(after) == 2: + # the signature ignores a LINE's direction, so its movement must too: + # the same line drawn the other way round and moved is a move + translated = translates(before, after[::-1]) if not translated: return {"geometry": {"from": [list(p) for p in before], "to": [list(p) for p in after]}} if abs(dx) > half or abs(dy) > half: @@ -308,12 +323,33 @@ def _movement(o: EntityRecord, n: EntityRecord, tol: float) -> dict: def _attrib_changes(o: EntityRecord, n: EntityRecord) -> list[dict]: - before, after = dict(o.attribs), dict(n.attribs) - return [ - {"tag": tag, "from": before.get(tag), "to": after.get(tag)} - for tag in sorted(set(before) | set(after)) - if before.get(tag) != after.get(tag) - ] + """One row per attribute tag whose values differ. A tag an INSERT carries + twice is compared as the list of its values, so one change cannot hide + behind the other; a tag carried once keeps plain ``from`` / ``to``.""" + + def by_tag(attribs) -> dict[str, list]: + values: dict[str, list] = defaultdict(list) + for tag, value in attribs: + values[tag].append(value) + return values + + before, after = by_tag(o.attribs), by_tag(n.attribs) + rows = [] + for tag in sorted(set(before) | set(after)): + old_values, new_values = before.get(tag, []), after.get(tag, []) + if old_values == new_values: + continue + if len(old_values) <= 1 and len(new_values) <= 1: + rows.append( + { + "tag": tag, + "from": old_values[0] if old_values else None, + "to": new_values[0] if new_values else None, + } + ) + else: + rows.append({"tag": tag, "from": old_values, "to": new_values}) + return rows def _changes(o: EntityRecord, n: EntityRecord, tol: float) -> dict: @@ -554,13 +590,16 @@ def diff_snapshots( def cap_report(result: dict, limit: int = DEFAULT_LIMIT) -> dict: - """``result`` with each list of ``CATEGORIES`` cut to ``limit`` rows and - ``truncated`` saying how many rows each cut dropped.""" + """``result`` with each list of ``CATEGORIES`` and the ``clusters`` cut to + ``limit`` rows and ``truncated`` saying how many rows each cut dropped. + + The clusters are capped in the report only: a markup still clouds every one. + """ if int(limit) < 1: raise ValueError(f"limit must be at least 1; got {limit!r}") capped = dict(result) capped["truncated"] = {} - for category in CATEGORIES: + for category in (*CATEGORIES, "clusters"): rows = list(result.get(category) or []) capped[category] = rows[: int(limit)] capped["truncated"][category] = max(0, len(rows) - int(limit)) diff --git a/tests/test_understand_diff.py b/tests/test_understand_diff.py index cfb0d1d..18b4d1b 100644 --- a/tests/test_understand_diff.py +++ b/tests/test_understand_diff.py @@ -294,10 +294,48 @@ def test_cap_report_cuts_every_list_and_says_how_much(): new = snap(*STILL, *(line(f"N{i}", 0, 1000 * (i + 1), 100, 1000 * (i + 1)) for i in range(5))) capped = cap_report(diff_snapshots(old, new, move_tol=10.0), limit=2) assert len(capped["added"]) == 2 - assert capped["truncated"] == {"changed": 0, "added": 3, "removed": 0} + # five lines 1000 apart are five clusters: the list is capped like the others + assert len(capped["clusters"]) == 2 + assert capped["truncated"] == {"changed": 0, "added": 3, "removed": 0, "clusters": 3} assert capped["summary"]["added"] == 5 +def test_a_line_drawn_back_to_front_and_moved_is_a_move(): + # the signature ignores a LINE's direction; so does its movement + old = snap(*STILL, line("D", 0, 200, 100, 200)) + new = snap(*STILL, line("D", 150, 250, 50, 250)) # reversed, then moved by (50, 50) + (row,) = diff_snapshots(old, new, move_tol=100.0)["changed"] + assert row["changes"] == {"moved_by": [pytest.approx(50.0), pytest.approx(50.0)]} + + +def test_an_attribute_tag_carried_twice_cannot_hide_a_change(): + # as a dict, both revisions read NOTE = "B" and the edit of the first NOTE vanished + before = (("NOTE", "A"), ("NOTE", "B")) + after = (("NOTE", "X"), ("NOTE", "B")) + old = snap(*STILL, rec("I", "INSERT", "TAGS", ((500, 500),), block="TB", attribs=before)) + new = snap(*STILL, rec("I", "INSERT", "TAGS", ((500, 500),), block="TB", attribs=after)) + (row,) = diff_snapshots(old, new)["changed"] + assert row["changes"] == {"attribs": [{"tag": "NOTE", "from": ["A", "B"], "to": ["X", "B"]}]} + + +def test_a_dense_drawing_moved_as_a_whole_pairs_each_entity_with_its_own_copy(): + # 200 lines 10 apart on one layer, all moved (3, 0) under new handles: no exact + # match, so no handle stage, and with move_tol 500 each line has about a hundred + # candidates. The position stage keeps only the nearest few per entity, and each + # line must still pair with its own moved copy. + old = snap(*(line(f"O{i}", 0, 10 * i, 5, 10 * i) for i in range(200))) + new = snap(*(line(f"N{i}", 3, 10 * i, 8, 10 * i) for i in range(200))) + result = diff_snapshots(old, new, move_tol=500.0) + assert result["handles_stable"] is False + assert (result["summary"]["changed"], result["summary"]["added"]) == (200, 0) + assert result["summary"]["removed"] == 0 + assert all( + row["changes"] == {"moved_by": [pytest.approx(3.0), pytest.approx(0.0)]} + and row["old_handle"][1:] == row["new_handle"][1:] + for row in result["changed"] + ) + + # -- block definitions ----------------------------------------------------------------- diff --git a/tests/test_understand_diff_tool.py b/tests/test_understand_diff_tool.py index e7c5eda..d6f6e29 100644 --- a/tests/test_understand_diff_tool.py +++ b/tests/test_understand_diff_tool.py @@ -46,7 +46,7 @@ async def test_the_current_document_against_its_older_revision(client, tmp_path) pair["insert"], } assert result["blocks"]["compared"] is True and result["blocks"]["changed"] == [] - assert result["truncated"] == {"changed": 0, "added": 0, "removed": 0} + assert result["truncated"] == {"changed": 0, "added": 0, "removed": 0, "clusters": 0} assert await _total(client) == before, "a diff without markup writes nothing"