From 503c8c69b5680d812d31e876a8dff9abeeea84e0 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Mon, 28 Sep 2026 13:49:36 +1300 Subject: [PATCH] qet-mcp: qet_edit addresses tables and symbol text fields by uuid set_table_position and delete_table took a table's index, and set_element_text / delete_element_text a text field's index. An index shifts when an earlier item is deleted, so a run that deletes table 0 and then moves "table 1" moved the wrong table. Each now also takes the item's uuid, resolved at run time through qet.tableIndex() and qet.elementTextIndex() (the previous commit), as texts, shapes and pictures already are through textIndex() and friends. A text field's lookup is scoped to the op's element, since copies of a symbol share their fields' uuids. An index still works, and a build without the lookups is refused with the usual missing-methods hint only when a uuid is actually used. Tests: the generated script (no binary), and end to end: delete one table then move the other, both by uuid; and edit one copy's shared field by uuid, leaving the other copy's untouched. Co-Authored-By: Claude Opus 5.5 --- misc/qet-mcp/qet_mcp.py | 35 ++++++++++++++-- misc/qet-mcp/test_qet_mcp.py | 79 ++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 4 deletions(-) diff --git a/misc/qet-mcp/qet_mcp.py b/misc/qet-mcp/qet_mcp.py index 1c557067c..8366a1b60 100755 --- a/misc/qet-mcp/qet_mcp.py +++ b/misc/qet-mcp/qet_mcp.py @@ -1128,10 +1128,10 @@ OPS = { ("source", "str"), ("value", "str"), ("x", "num"), ("y", "num")]), "set_element_text": ("setElementTextProperty", [("folio", "folio"), ("element", "elmt"), - ("index", "folio"), ("property", "str"), + ("index", "element_text"), ("property", "str"), ("value", "str")]), "delete_element_text": ("deleteElementText", [("folio", "folio"), ("element", "elmt"), - ("index", "folio")]), + ("index", "element_text")]), # Returns the uuids of the copies IN THE ORDER the elements were named, # so "$copies[0]" is the copy of the first one. Conductors between the # copied elements are copied with them; copies arrive without labels or @@ -1191,9 +1191,9 @@ OPS = { ("closed", "bool")]), "add_table": ("addTable", [("folio", "folio"), ("kind", "str"), ("name", "str"), ("query", "str")]), - "set_table_position": ("setTablePosition", [("folio", "folio"), ("table", "folio"), + "set_table_position": ("setTablePosition", [("folio", "folio"), ("table", "table"), ("x", "num"), ("y", "num")]), - "delete_table": ("deleteTable", [("folio", "folio"), ("table", "folio")]), + "delete_table": ("deleteTable", [("folio", "folio"), ("table", "table")]), } SHAPES = ["line", "rectangle", "ellipse", "polygon"] @@ -1247,6 +1247,7 @@ def _build_script(operations: list, output: str) -> str: # an index-only edit still runs on a build that predates them. uuid_methods: set[str] = set() folio_js = "0" + element_js = None # the op's element, for lookups scoped to it lines = [ "// generated by qet-mcp; do not edit", "var R = {};", # $name -> value from an earlier op @@ -1300,6 +1301,26 @@ def _build_script(operations: list, output: str) -> str: raise ValueError(f"operation {op_index}: {key!r} must be a non-empty list of " f"integer indices, got {value!r}") return _js(value) + if kind == "table": + # As for texts below: a uuid is resolved to the current index at + # run time, since deleting an earlier table shifts every index. + if isinstance(value, str) and _UUID_RE.fullmatch(value): + uuid_methods.add("tableIndex") + return f"qet.tableIndex({folio_js}, {_js(value)})" + if not isinstance(value, int) or isinstance(value, bool): + raise ValueError(f"operation {op_index}: {key!r} must be a table index " + f"or its uuid, got {value!r}") + return _js(value) + if kind == "element_text": + # A field's uuid is unique only within its element (copies keep + # them), so the lookup takes the op's element too. + if isinstance(value, str) and _UUID_RE.fullmatch(value): + uuid_methods.add("elementTextIndex") + return f"qet.elementTextIndex({folio_js}, {element_js}, {_js(value)})" + if not isinstance(value, int) or isinstance(value, bool): + raise ValueError(f"operation {op_index}: {key!r} must be a text field index " + f"or its uuid, got {value!r}") + return _js(value) if kind in ("text", "shape", "image"): # A uuid names the item for good; it is turned into the index # the call takes at run time, by the item's own folio. @@ -1399,6 +1420,7 @@ def _build_script(operations: list, output: str) -> str: if key not in op: raise ValueError(f"operation {i} ({name}) is missing {key!r}") folio_js = args[0] if args else "0" + element_js = args[1] if len(args) > 1 else None args.append(ref_or(op[key], kind, i, key)) ident = op.get("id") @@ -2415,6 +2437,11 @@ TOOLS = [ "shape's points can reorder it relative to the others -- re-list " "before addressing one by index again if more than one is being " "edited in the same run, or address it by uuid. " + "set_table_position/delete_table take a table's index or its uuid, " + "and set_element_text/delete_element_text a text field's index or " + "its uuid (the field's own, looked up within the op's element) " + "-- a uuid still names the right item after an " + "earlier one is deleted. " "Tables: add_table places a BOM/nomenclature or summary table " "(kind is \"nomenclature\" or \"summary\") built from a query " "against a project database view -- run qet.query() (the " diff --git a/misc/qet-mcp/test_qet_mcp.py b/misc/qet-mcp/test_qet_mcp.py index e19ea989f..d6f11aad7 100644 --- a/misc/qet-mcp/test_qet_mcp.py +++ b/misc/qet-mcp/test_qet_mcp.py @@ -167,6 +167,33 @@ class EditValidation(unittest.TestCase): def build(self, ops): return m._build_script(ops, "/tmp/out.qet") + def test_tables_and_text_fields_by_uuid(self): + """A table or a symbol text field named by uuid is looked up at run + time; a field's lookup is scoped to the op's element.""" + U = "{11111111-2222-4333-8444-555555555555}" + E = "{aaaaaaaa-0000-4000-8000-000000000001}" + s = self.build([{"op": "set_table_position", "folio": 2, "table": U, "x": 1, "y": 2}]) + self.assertIn(f'qet.setTablePosition(2, qet.tableIndex(2, "{U}"), 1, 2)', s) + s = self.build([{"op": "delete_table", "folio": 0, "table": 3}]) + self.assertIn("qet.deleteTable(0, 3)", s) + s = self.build([{"op": "set_element_text", "folio": 1, "element": E, "index": U, + "property": "x", "value": "5"}]) + self.assertIn(f'qet.setElementTextProperty(1, "{E}", ' + f'qet.elementTextIndex(1, "{E}", "{U}"), "x", "5")', s) + # the element may be one placed earlier in the same run + s = self.build([{"op": "add_folio", "id": "f"}, + {"op": "add_element", "id": "k", "folio": "$f", "path": "p", "x": 0, "y": 0}, + {"op": "delete_element_text", "folio": "$f", "element": "$k", "index": U}]) + self.assertIn(f'qet.deleteElementText(R["f"], R["k"], qet.elementTextIndex(R["f"], R["k"], "{U}"))', s) + # the lookups are required only when a uuid is used + self.assertIn('"tableIndex"', self.build([{"op": "delete_table", "folio": 0, "table": U}])) + self.assertNotIn('"tableIndex"', self.build([{"op": "delete_table", "folio": 0, "table": 0}])) + for op in ({"op": "delete_table", "folio": 0, "table": "second"}, + {"op": "delete_element_text", "folio": 0, "element": E, "index": "label"}): + with self.subTest(op=op["op"]): + with self.assertRaisesRegex(ValueError, "index or its uuid"): + self.build([op]) + def test_every_op_generates_a_script(self): # one minimal valid instance of every op f = {"op": "add_folio", "id": "f"} @@ -2816,6 +2843,58 @@ class UuidIndexLookups(unittest.TestCase): self.assertEqual((out["bogus"], out["not_uuid"]), (-1, -1)) self.assertEqual((out["second_after"], out["first_after"]), (0, -1)) + def test_qet_edit_deletes_then_moves_tables_by_uuid(self): + """Delete one table, then move the other, both by uuid. By index the + second op would name the wrong table: deleting table 0 shifts table 1.""" + text = (Path(EXAMPLES) / "industrial.qet").read_text(encoding="utf-8") + root = ET.fromstring(text) + folio, table = next((i, d.find("tables/graphics_table")) for i, d in enumerate(root.iter("diagram")) + if d.find("tables/graphics_table") is not None) + twin = ET.fromstring(ET.tostring(table)) + twin.set("uuid", "{11111111-2222-4333-8444-555555555555}") + twin.set("x", str(float(table.get("x")) + 900)) + # Insert into the raw text: re-serialising the whole file with + # ElementTree rewrites the embedded SVG logo's namespace, which + # QElectroTech then saves without its declaration. + start = text.index(f'uuid="{table.get("uuid")}"') + end = text.index("", start) + len("") + text = text[:end] + ET.tostring(twin, encoding="unicode") + text[end:] + with tempfile.TemporaryDirectory() as tmp: + src, out = Path(tmp) / "two.qet", Path(tmp) / "out.qet" + src.write_text(text, encoding="utf-8") + r = m.tool_edit(BINARY, str(src), [ + {"op": "delete_table", "folio": folio, "table": table.get("uuid")}, + {"op": "set_table_position", "folio": folio, "table": twin.get("uuid"), "x": 120, "y": 340}], + str(out), elements_dir=ELEMENTS or None) + self.assertTrue(r["ok"], r.get("hint")) + left = list(ET.parse(out).getroot().iter("diagram"))[folio].findall("tables/graphics_table") + self.assertEqual([(t.get("uuid"), float(t.get("x")), float(t.get("y"))) for t in left], + [(twin.get("uuid"), 120.0, 340.0)]) + + def test_qet_edit_edits_one_copys_field_by_uuid(self): + """Two copies of a symbol share a field uuid; addressing it with the + symbol changes that copy's field only.""" + root = ET.parse(Path(EXAMPLES) / "2612_ats_singlephase.qet").getroot() + owners = {} + for i, d in enumerate(root.iter("diagram")): + for el in d.iter("element"): + for t in el.findall("dynamic_texts/dynamic_elmt_text"): + if t.get("uuid"): + owners.setdefault((i, t.get("uuid")), []).append(el.get("uuid")) + (folio, field), (a, b) = next((k, v[:2]) for k, v in owners.items() if len(v) >= 2) + with tempfile.TemporaryDirectory() as tmp: + src, out = Path(tmp) / "in.qet", Path(tmp) / "out.qet" + shutil.copy(Path(EXAMPLES) / "2612_ats_singlephase.qet", src) + r = m.tool_edit(BINARY, str(src), [ + {"op": "set_element_text", "folio": folio, "element": b, "index": field, + "property": "x", "value": "77"}], str(out), elements_dir=ELEMENTS or None) + self.assertTrue(r["ok"], r.get("hint")) + x_of = lambda path, el_uuid: next( + t.get("x") for el in ET.parse(path).getroot().iter("element") if el.get("uuid") == el_uuid + for t in el.findall("dynamic_texts/dynamic_elmt_text") if t.get("uuid") == field) + self.assertEqual(float(x_of(out, b)), 77.0) + self.assertEqual(x_of(out, a), x_of(src, a)) + def test_element_text_index_needs_the_element_as_well(self): """Copies of a symbol share their text fields' uuids (2612_ats_singlephase.qet): the same field uuid resolves on each copy to that copy's own field."""