From ccc063a19eac80e8fe2ba16e76337a99470452fb Mon Sep 17 00:00:00 2001 From: ispyisail Date: Tue, 29 Sep 2026 10:56:45 +1300 Subject: [PATCH] qet-mcp: report every wrong terminal uuid of an op, each by its argument Review of the previous commit: an add_conductor with both terminal uuids wrong logged two notes under one op index and the second replaced the first, and neither said which end it was about. Notes of one op are now joined, and a terminal note starts with its argument ("from_terminal:", "to_terminal:", "terminal:"). The integration test's both-ends case fails with the old replacing behaviour. Co-Authored-By: Claude Opus 5.5 --- misc/qet-mcp/qet_mcp.py | 11 +++++++---- misc/qet-mcp/test_qet_mcp.py | 26 ++++++++++++++++++-------- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/misc/qet-mcp/qet_mcp.py b/misc/qet-mcp/qet_mcp.py index 35cedb1d5..68d064d53 100755 --- a/misc/qet-mcp/qet_mcp.py +++ b/misc/qet-mcp/qet_mcp.py @@ -1513,10 +1513,10 @@ def _build_script(operations: list, output: str) -> str: # A terminal named by uuid is turned into the index the calls take, # on its own element; -1, with the reason logged, if that element # has no terminal with it. - "function qetMcpTerminal(index, folio, element, uuid) {", + "function qetMcpTerminal(index, key, folio, element, uuid) {", " var t = qet.terminalIndex(folio, element, uuid);", " if (t < 0) qet.log(" + _js(_MARKER) + " + JSON.stringify({kind: 'op_note', " - "index: index, note: 'no terminal ' + uuid + ' on element ' + element + " + "index: index, note: key + ': no terminal ' + uuid + ' on element ' + element + " "' (or two of its terminals carry it)'}));", " return t;", "}", @@ -1580,7 +1580,7 @@ def _build_script(operations: list, output: str) -> str: # terminals at the same point. if isinstance(value, str) and _UUID_RE.fullmatch(value): uuid_methods.add("terminalIndex") - return (f"qetMcpTerminal({op_index}, {folio_js}, {owner_js}, " + return (f"qetMcpTerminal({op_index}, {_js(key)}, {folio_js}, {owner_js}, " f"{_js(value)})") if not isinstance(value, int) or isinstance(value, bool): raise ValueError(f"operation {op_index}: {key!r} must be a terminal index " @@ -1808,7 +1808,10 @@ def _parse_script_output(text: str) -> dict: (isinstance(r, int) and not isinstance(r, bool) and r == -1)) ops.append(rec) elif rec.get("kind") == "op_note": - notes[rec.get("index")] = rec.get("note") + # An op can log more than one (add_conductor, one per end): + # keep them all rather than only the last. + idx = rec.get("index") + notes[idx] = (notes[idx] + "; " if idx in notes else "") + str(rec.get("note")) elif rec.get("kind") == "save": saved = bool(rec.get("result")) stopped = bool(rec.get("stopped_early")) diff --git a/misc/qet-mcp/test_qet_mcp.py b/misc/qet-mcp/test_qet_mcp.py index c8cda96ff..cd814eb4d 100644 --- a/misc/qet-mcp/test_qet_mcp.py +++ b/misc/qet-mcp/test_qet_mcp.py @@ -251,25 +251,25 @@ class EditValidation(unittest.TestCase): T, V = "{31111111-2222-4333-8444-555555555555}", "{41111111-2222-4333-8444-555555555555}" s = self.build([{"op": "add_conductor", "folio": 1, "from": E, "from_terminal": T, "to": F, "to_terminal": V}]) - self.assertIn(f'qet.addConductor(1, "{E}", qetMcpTerminal(0, 1, "{E}", "{T}"), ' - f'"{F}", qetMcpTerminal(0, 1, "{F}", "{V}"))', s) + self.assertIn(f'qet.addConductor(1, "{E}", qetMcpTerminal(0, "from_terminal", 1, "{E}", "{T}"), ' + f'"{F}", qetMcpTerminal(0, "to_terminal", 1, "{F}", "{V}"))', s) self.assertIn('"terminalIndex"', s) s = self.build([{"op": "set_conductor", "folio": 0, "element": E, "terminal": T, "property": "num", "value": "W1"}]) - self.assertIn(f'qet.setConductorProperty(0, "{E}", qetMcpTerminal(0, 0, "{E}", "{T}"), ' + self.assertIn(f'qet.setConductorProperty(0, "{E}", qetMcpTerminal(0, "terminal", 0, "{E}", "{T}"), ' '"num", "W1")', s) s = self.build([{"op": "delete_conductor", "folio": 0, "element": E, "terminal": T}]) - self.assertIn(f'qet.deleteConductor(0, "{E}", qetMcpTerminal(0, 0, "{E}", "{T}"))', s) + self.assertIn(f'qet.deleteConductor(0, "{E}", qetMcpTerminal(0, "terminal", 0, "{E}", "{T}"))', s) s = self.build([{"op": "move_conductor_segment", "folio": 0, "element": E, "terminal": T, "segment": 1, "dx": 5, "dy": 0}]) - self.assertIn(f'qet.moveConductorSegment(0, "{E}", qetMcpTerminal(0, 0, "{E}", "{T}"), 1, 5, 0)', s) + self.assertIn(f'qet.moveConductorSegment(0, "{E}", qetMcpTerminal(0, "terminal", 0, "{E}", "{T}"), 1, 5, 0)', s) # a $name element and a folio uuid reach the lookup resolved s = self.build([{"op": "add_folio", "id": "f"}, {"op": "add_element", "id": "a", "folio": "$f", "path": "x.elmt", "x": 0, "y": 0}, {"op": "delete_conductor", "folio": "$f", "element": "$a", "terminal": T}]) - self.assertIn(f'qet.deleteConductor(R["f"], R["a"], qetMcpTerminal(2, R["f"], R["a"], "{T}"))', s) + self.assertIn(f'qet.deleteConductor(R["f"], R["a"], qetMcpTerminal(2, "terminal", R["f"], R["a"], "{T}"))', s) s = self.build([{"op": "delete_conductor", "folio": V, "element": E, "terminal": T}]) - self.assertIn(f'qetMcpTerminal(0, qet.folioIndex("{V}"), "{E}", "{T}")', s) + self.assertIn(f'qetMcpTerminal(0, "terminal", qet.folioIndex("{V}"), "{E}", "{T}")', s) # an index is unchanged and needs no lookup s = self.build([{"op": "delete_conductor", "folio": 0, "element": E, "terminal": 2}]) self.assertIn(f'qet.deleteConductor(0, "{E}", 2)', s) @@ -2303,7 +2303,17 @@ class Integration(unittest.TestCase): "to": "$a", "to_terminal": a1["uuid"]}]) self.assertFalse(r["ok"]) self.assertTrue(r["stopped_early"]) - self.assertIn("no terminal", json.dumps(r["operations"][1])) + self.assertIn("from_terminal: no terminal", json.dumps(r["operations"][1])) + + # both ends wrong: both reported, each by its argument + r = self.sb.edit(base, [ + {"op": "add_element", "id": "a", "folio": 0, "path": COIL, "x": 100, "y": 100}, + {"op": "add_conductor", "folio": 0, "from": "$a", + "from_terminal": "{00000000-0000-4000-8000-000000000001}", + "to": "$a", "to_terminal": "{00000000-0000-4000-8000-000000000002}"}]) + note = r["operations"][1].get("note", "") + self.assertIn("from_terminal: no terminal", note) + self.assertIn("to_terminal: no terminal", note) def test_noop_edit_has_no_conductor_churn(self): """Re-saving renumbers the file's terminal ids; the diff must not