From 8fd692783c31792ebc7790d3588d049b264351e2 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Mon, 28 Sep 2026 13:15:12 +1300 Subject: [PATCH] qet-mcp: key symbol text fields by symbol and field uuid; refuse shared uuids A symbol text field's uuid is unique only within its symbol: copying a symbol keeps them, so 7 of the 24 shipped examples repeat one, up to 20 times (2612_ats_singlephase.qet). Keyed on the field uuid alone, those fields merged and an edit to one copy could be reported on another. They are now keyed on (symbol uuid, field uuid). More generally, uuids are used as keys only when present on every item and unique on each side; otherwise the old position/ends matching is kept, for texts, shapes, pictures, tables, conductors and folios alike. Found by the seeded-edit invariants (an untouched symbol's field reported changed). Co-Authored-By: Claude Opus 5.5 --- misc/qet-mcp/qet_mcp.py | 23 ++++++++--- misc/qet-mcp/test_qet_mcp.py | 76 ++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 6 deletions(-) diff --git a/misc/qet-mcp/qet_mcp.py b/misc/qet-mcp/qet_mcp.py index 9cf5d64e2..dfb320bd9 100755 --- a/misc/qet-mcp/qet_mcp.py +++ b/misc/qet-mcp/qet_mcp.py @@ -409,6 +409,14 @@ def _extras(root: ET.Element) -> dict: "element_texts": element_texts, "element_text_uuids": element_text_uuids} +def _usable_ids(*sides) -> bool: + """Whether uuids can identify items: present on every item, unique on each + side. Copying a symbol keeps its text fields' uuids, so a project can + hold the same field uuid twenty times; keying on it would merge them.""" + return (any(sides) and all(all(s) for s in sides) + and all(len(set(s)) == len(s) for s in sides)) + + def _diff_keyed(a: dict, b: dict, label) -> dict: """added / removed / changed for two dicts keyed by identity.""" changed = [] @@ -432,7 +440,7 @@ def _diff_items(a: list, b: list) -> dict: and re-added. On uuid, position is part of what is compared, so a move is a change to that item. """ - by_uuid = all(r["uuid"] for r in a + b) + by_uuid = _usable_ids([r["uuid"] for r in a], [r["uuid"] for r in b]) def key(r): return r["uuid"] if by_uuid else str(r["key"]) @@ -458,7 +466,7 @@ def _diff_folios(a: dict, b: dict) -> dict: says so when the count changed. """ ua, ub = a["folio_uuids"], b["folio_uuids"] - by_uuid = bool(ua or ub) and all(ua.values()) and all(ub.values()) + by_uuid = _usable_ids(list(ua.values()), list(ub.values())) changed, reordered, added, removed = [], [], [], [] if by_uuid: pos_a = {u: n for n, u in ua.items()} @@ -499,15 +507,18 @@ def _diff_element_texts(a: dict, b: dict) -> dict: the list. A field's own text is also compared ("shows"), so relabelling an element shows up here as well as in the element's information. """ - ka, kb = a["element_text_uuids"], b["element_text_uuids"] - by_uuid = bool(ka or kb) and all(ka.values()) and all(kb.values()) + # A field's uuid is unique only within its symbol (copies keep them), so + # a field is identified by its symbol's uuid and its own. + ka = {k: (k[0], u) if k[0] and u else "" for k, u in a["element_text_uuids"].items()} + kb = {k: (k[0], u) if k[0] and u else "" for k, u in b["element_text_uuids"].items()} + by_uuid = _usable_ids(list(ka.values()), list(kb.values())) def label(k): return {"element": k[0], "source": k[1], "bound_to": k[2], "n": k[3]} if not by_uuid: out = _diff_keyed(a["element_texts"], b["element_texts"], label) out["keyed_by"] = "position" return out - labels = {u: {**label(k), "uuid": u} for side in (ka, kb) for k, u in side.items()} + labels = {u: {**label(k), "uuid": u[1]} for side in (ka, kb) for k, u in side.items()} out = _diff_keyed({ka[k]: v for k, v in a["element_texts"].items()}, {kb[k]: v for k, v in b["element_texts"].items()}, lambda u: labels[u]) @@ -588,7 +599,7 @@ def tool_diff(before: str, after: str) -> dict: # has one, so a rewired conductor is that conductor, changed ("ends"). # QElectroTech keeps a uuid only on conductors that were loaded with one # or created since, so an older file keys on its two ends instead. - co_by_uuid = bool(a_rows or b_rows) and all(r["uuid"] for r in a_rows + b_rows) + co_by_uuid = _usable_ids([r["uuid"] for r in a_rows], [r["uuid"] for r in b_rows]) co_id = (lambda r: r["uuid"]) if co_by_uuid else (lambda r: r["key"]) a_co = {co_id(r): r for r in a_rows} b_co = {co_id(r): r for r in b_rows} diff --git a/misc/qet-mcp/test_qet_mcp.py b/misc/qet-mcp/test_qet_mcp.py index a549581d4..9dc45b65b 100644 --- a/misc/qet-mcp/test_qet_mcp.py +++ b/misc/qet-mcp/test_qet_mcp.py @@ -1174,6 +1174,82 @@ class DiffContracts(unittest.TestCase): d = m.tool_diff(plain((1, "a"), (2, "b")), plain((2, "b")))["element_texts"] self.assertEqual(d["keyed_by"], "position") + def test_copied_symbols_keep_their_text_field_uuids(self): + """Copying a symbol keeps its text fields' uuids (20 copies of one in + 2612_ats_singlephase.qet), so a field is its symbol's uuid plus its own: + editing one copy's field must not be read as another copy's.""" + field = lambda x: ('t') + pair = lambda xa, xb: self.qet(self.folio(self.el(self.A, 0, 0, texts=field(xa)) + + self.el(self.B, 0, 0, texts=field(xb)))) + for before, after, which in ((pair(1, 1), pair(5, 1), self.A), (pair(1, 1), pair(1, 5), self.B)): + with self.subTest(edited=which): + d = m.tool_diff(before, after)["element_texts"] + self.assertEqual(d["keyed_by"], "uuid") + self.assertEqual([(c["item"]["element"], c["item"]["uuid"], c["changed"]) + for c in d["changed"]], [(which, "{same}", {"x": ["1", "5"]})]) + + def test_repeated_uuids_fall_back_rather_than_merge(self): + # the same field uuid twice inside one symbol + twice = lambda x: self.qet(self.folio(self.el(self.A, 0, 0, texts="".join( + f'{v}' + '' for v in (x, 9))))) + self.assertEqual(m.tool_diff(twice(1), twice(2))["element_texts"]["keyed_by"], "position") + # two shapes sharing a uuid: both must still be counted + shapes = self.qet(self.folio(extra='' + ''.join( + f'' for i in (0, 5)) + '')) + d = m.tool_diff(shapes, shapes)["shapes"] + self.assertEqual((d["keyed_by"], d["before"]), ("position", 2)) + # two conductors sharing a uuid + els = self.el(self.A, 0, 0) + self.el(self.B, 0, 0) + self.el(self.C, 0, 0) + wires = self.qet(self.folio(els, self.wire(self.A, self.B, uuid="{w}") + + self.wire(self.B, self.C, uuid="{w}"))) + c = m.tool_diff(wires, wires)["conductors"] + self.assertEqual((c["keyed_by"], c["before"]), ("ends", 2)) + + def test_uuid_matching_when_one_side_has_none_of_a_kind(self): + """No folios, fields or wires on one side is not a reason to fall back + to position: the other side's uuids are all there is to match.""" + empty = self.qet("") + full = self.qet(self.folio( + self.el(self.A, 0, 0, texts='t') + + self.el(self.B, 0, 0), self.wire(self.A, self.B, uuid="{w1}"), uuid="{f1}")) + d = m.tool_diff(empty, full) + self.assertEqual((d["folios"]["keyed_by"], d["element_texts"]["keyed_by"], + d["conductors"]["keyed_by"]), ("uuid", "uuid", "uuid")) + self.assertEqual(d["folios"]["added"], [{"folio": 1, "uuid": "{f1}", "title": ""}]) + # matched by uuid, a conductor is never on the renumbered-id footing + self.assertNotIn("unstable_keys", d["conductors"]) + + def test_a_folio_that_kept_its_place_is_not_reordered(self): + f = lambda u, t: self.folio(title=t, uuid=u) + before = self.qet(f("{f1}", "One") + f("{f2}", "Two") + f("{f3}", "Three")) + after = self.qet(f("{f1}", "One") + f("{f3}", "Three") + f("{f2}", "Two")) + self.assertEqual([r["uuid"] for r in m.tool_diff(before, after)["folios"]["reordered"]], + ["{f3}", "{f2}"]) + + def test_folio_lists_are_capped(self): + f = lambda i, t: self.folio(title=t, uuid=f"{{{i:04d}}}") + many = lambda rng, t: "".join(f(i, t) for i in rng) + d = m.tool_diff(self.qet(""), self.qet(many(range(51), "x")))["folios"] + self.assertEqual(len(d["added"]), 50) + d = m.tool_diff(self.qet(many(range(51), "x")), self.qet(""))["folios"] + self.assertEqual(len(d["removed"]), 50) + d = m.tool_diff(self.qet(many(range(52), "x")), self.qet(many(reversed(range(52)), "x")))["folios"] + self.assertEqual(len(d["reordered"]), 50) + + def test_table_fields(self): + def table(**v): + a = {"x": "0", "y": "0", "width": "100", "height": "50", "display_n_row": "10", **v} + return ('') + for attr, reported in (("y", "y"), ("width", "width"), ("height", "height")): + with self.subTest(attr=attr): + d = m.tool_diff(self.qet(self.folio(extra=table())), + self.qet(self.folio(extra=table(**{attr: "7"}))))["tables"] + self.assertEqual([list(c["changed"]) for c in d["changed"]], [[reported]]) + def test_tables(self): table = lambda x, rows: ('')