diff --git a/misc/qet-mcp/README.md b/misc/qet-mcp/README.md index a32c453a2..6d29fe6dc 100644 --- a/misc/qet-mcp/README.md +++ b/misc/qet-mcp/README.md @@ -172,7 +172,10 @@ the answer a screenshot gave wrongly. ``` An op that creates something takes an `"id"`; later ops name it as `"$id"`. -Terminals are addressed by index — top to bottom, then left to right, **not** +A `"folio"` given as a number counts **from 0**, while `qet_elements` and +`qet_project_info` number folios from 1 as the application does: the folio +they call 1 is `"folio": 0` here. An op that fails because of this says which +index to use. Terminals are addressed by index — top to bottom, then left to right, **not** the order the `.elmt` lists them. `qet_element_info` and `qet_element_search` both report that index order. The answer carries a per-operation result *and* a `qet_diff`, because "addConductor → true" says the call was diff --git a/misc/qet-mcp/qet_mcp.py b/misc/qet-mcp/qet_mcp.py index fef21791c..453264b4f 100755 --- a/misc/qet-mcp/qet_mcp.py +++ b/misc/qet-mcp/qet_mcp.py @@ -1990,6 +1990,35 @@ def tool_project_new(binary: str, output: str, title: str = "Untitled", return result +def _wrong_folio(project: str, op: dict) -> str: + """Explain a failed op that named an element on the wrong folio. + + qet_elements and qet_project_info number folios from 1, as the UI does; + qet_edit passes "folio" straight to the scripting API, which counts from + 0. Passing the number qet_elements showed therefore addresses the next + folio, and the op fails with nothing but "false". When the element the + op names is in the project on some other folio, say which index to use. + """ + folio = op.get("folio") + spec = OPS.get(op.get("op"), (None, []))[1] + uuids = [op[key] for key, kind in spec + if kind == "elmt" and isinstance(op.get(key), str) + and not op[key].startswith("$")] + if not isinstance(folio, int) or not uuids: + return "" + try: + where = {el.get("uuid"): i for i, el in _elements(_root(project))} + except (OSError, ET.ParseError): + return "" + for uuid in uuids: + number = where.get(uuid) + if number is not None and number - 1 != folio: + return (f"Element {uuid} is on folio {number} as qet_elements numbers " + f"it, which is \"folio\": {number - 1} here: qet_edit counts " + "folios from 0.") + return "" + + def tool_edit(binary: str, project: str, operations: list, output: str, elements_dir: str | None = None, timeout: int = 180) -> dict: """Apply edits through the scripting API and report what actually changed. @@ -2060,10 +2089,14 @@ def tool_edit(binary: str, project: str, operations: list, output: str, for record in result.get("operations", []): if not record["succeeded"]: result["ok"] = False - result.setdefault("hint", - f"operation {record['index']} ({record['op']}) returned " - f"{record['result']!r}; later operations were skipped. " - "qet.log lines in stderr/stdout say why.") + hint = (f"operation {record['index']} ({record['op']}) returned " + f"{record['result']!r}; later operations were skipped. " + "qet.log lines in stderr/stdout say why.") + if 0 <= record["index"] < len(operations): + wrong = _wrong_folio(str(proj), operations[record["index"]]) + if wrong: + hint += " " + wrong + result.setdefault("hint", hint) break if result.get("saved") is False: @@ -2236,7 +2269,9 @@ TOOLS = [ "Give an op an \"id\" to name what it produced, then refer to " "it later as \"$id\" -- that is how an element placed by " "add_element gets wired by add_conductor, and how a folio made " - "by add_folio is addressed. Terminals are numbered by their " + "by add_folio is addressed. A \"folio\" given as a number is " + "an index counted from 0: the folio qet_elements and " + "qet_project_info call 1 is \"folio\": 0 here. Terminals are numbered by their " "index in the element definition; qet_element_info lists them. " "set_conductor addresses a conductor as the one on a given " "terminal and applies the change to its whole electrical " diff --git a/misc/qet-mcp/test_qet_mcp.py b/misc/qet-mcp/test_qet_mcp.py index b874a3356..1647cabb7 100644 --- a/misc/qet-mcp/test_qet_mcp.py +++ b/misc/qet-mcp/test_qet_mcp.py @@ -670,6 +670,43 @@ class ElementSearch(unittest.TestCase): self.assertEqual(m.tool_element_search(str(self.root), "fine")["total_matches"], 1) +class WrongFolioHint(unittest.TestCase): + """qet_elements numbers folios from 1, qet_edit from 0; a failed op that + used the wrong one should say which index to use.""" + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.qet = Path(self.tmp.name) / "p.qet" + self.qet.write_text( + '' + '' + '') + + def tearDown(self): + self.tmp.cleanup() + + def hint(self, op): + return m._wrong_folio(str(self.qet), op) + + def test_the_number_qet_elements_showed_gets_the_index_to_use(self): + h = self.hint({"op": "move_element", "folio": 2, "element": "{b}", "dx": 5, "dy": 0}) + self.assertIn("folio 2 as qet_elements numbers it", h) + self.assertIn('"folio": 1 here', h) + + def test_no_hint_when_the_folio_was_right_or_cannot_be_checked(self): + for op in ({"op": "move_element", "folio": 1, "element": "{b}"}, # right index + {"op": "move_element", "folio": 0, "element": "{nope}"}, # not in the file + {"op": "move_element", "folio": 0, "element": "$k1"}, # placed by the script + {"op": "move_element", "folio": "$f", "element": "{b}"}, # folio by reference + {"op": "add_folio"}): + with self.subTest(op=op): + self.assertEqual(self.hint(op), "") + + def test_the_description_says_how_folios_are_counted(self): + tool = next(t for t in m.TOOLS if t["name"] == "qet_edit") + self.assertIn("counted from 0", tool["inputSchema"]["properties"]["operations"]["description"]) + + class Diff(unittest.TestCase): def setUp(self): self.tmp = tempfile.TemporaryDirectory() @@ -2734,6 +2771,24 @@ class CorpusIntegration(unittest.TestCase): self.assertGreater(total, 3000) self.assertEqual(collisions, 0) + def test_a_folio_number_from_qet_elements_fails_with_the_index_to_use(self): + sb = Sandbox() + try: + src = shutil.copy(Path(EXAMPLES) / "grafcet.qet", sb.p("g.qet")) + first = m.tool_elements(src, folio=1)["elements"][0] + self.assertEqual(first["folio"], 1) + move = {"op": "move_element", "element": first["uuid"], "dx": 10, "dy": 0} + r = m.tool_edit(BINARY, src, [dict(move, folio=first["folio"])], sb.p("wrong.qet"), + elements_dir=ELEMENTS or None) + self.assertFalse(r["ok"]) + self.assertIn('"folio": 0 here', r["hint"]) + r = m.tool_edit(BINARY, src, [dict(move, folio=0)], sb.p("right.qet"), + elements_dir=ELEMENTS or None) + self.assertTrue(r["ok"], r.get("hint")) + self.assertEqual(r["diff"]["elements"]["distinct_move_deltas"], [[10.0, 0.0]]) + finally: + sb.close() + def test_untouched_resave_diffs_clean_on_every_small_example(self): sb = Sandbox() try: