Merge pull request #1093 from ispyisail/fix/qet-mcp-continuity-folio

Fix qet_continuity reporting a missing folio as clean

misc/qet-mcp/README.md

@@ -264,6 +264,14 @@ Python, plus the hang guard on `addConductor` and the database refresh in
 
 ## Notes and limits
 
+- **Two ways of numbering folios.** Tools that read the file —
+  `qet_project_info`, `qet_elements`, `qet_conductors`, `qet_diff` — number
+  folios from 1, as the application does. Tools that pass a folio to
+  QElectroTech's scripting API — `qet_edit` and `qet_continuity` — take an
+  index counted from 0, so the folio `qet_elements` calls 1 is `0` there.
+  `qet_continuity` refuses an index with no folio instead of reporting it
+  clean, and each of its findings carries both `folio` (the index) and
+  `folio_number` (counted from 1).
 - **The project database is reachable now, through `qet_query`.** It was
   not when this server was written, which is why every other structural
   tool here re-derives its answer from the XML. Prefer the views —

misc/qet-mcp/qet_mcp.py

@@ -1582,6 +1582,17 @@ def tool_continuity(binary: str, project: str, folio: int | None = None,
     proj = Path(project).expanduser()
     if not proj.is_file():
         raise ValueError(f"no such project: {proj}")
+    if folio is not None:
+        # qet.checkContinuity() answers an index it has no folio for with an
+        # empty list, which reads exactly like a clean folio. The index counts
+        # from 0 while qet_elements numbers folios from 1, so the likely
+        # mistake -- passing the last folio's number -- would pass silently.
+        count = len(list(_folios(_root(str(proj)))))
+        if not 0 <= folio < count:
+            raise ValueError(
+                f"folio {folio} does not exist: the project has {count} folio(s), "
+                f"indexed 0 to {count - 1} here. qet_continuity counts folios from 0; "
+                "the folio qet_elements calls N is N - 1.")
 
     folio_arg = -1 if folio is None else folio
     script = ("var out = qet.checkContinuity(%s);\n"
@@ -1611,6 +1622,12 @@ def tool_continuity(binary: str, project: str, folio: int | None = None,
         result.setdefault("hint", "no findings came back at all -- this build's "
                                   "scripting API may predate qet.checkContinuity()")
         return result
+    for f in findings:
+        # "folio" is the 0-based index qet.checkContinuity() uses; add the
+        # number qet_elements and the application show, so the two can be
+        # matched without arithmetic.
+        if isinstance(f, dict) and isinstance(f.get("folio"), int):
+            f["folio_number"] = f["folio"] + 1
     result["findings"] = findings
     result["finding_count"] = len(findings)
     result["errors"] = sum(1 for f in findings if f.get("severity") == "error")
@@ -2162,7 +2179,8 @@ TOOLS = [
             "type": "object",
             "properties": {
                 "path": {"type": "string"},
-                "folio": {"type": "integer"},
+                "folio": {"type": "integer",
+                          "description": "folio number counted from 1, as qet_elements and the application show it"},
                 "attribute": {"type": "string",
                               "description": "an XML attribute of <conductor>, e.g. cable"},
                 "non_empty": {"type": "boolean", "default": False},
@@ -2512,7 +2530,12 @@ TOOLS = [
             "properties": {
                 "binary": {"type": "string", "description": "path to the qelectrotech executable"},
                 "project": {"type": "string", "description": "the .qet to check; never modified"},
-                "folio": {"type": "integer", "description": "check one folio only; omit for the whole project"},
+                "folio": {"type": "integer", "description":
+                          "check one folio only; omit for the whole project. An index "
+                          "counted from 0, like qet_edit: the folio qet_elements calls 1 "
+                          "is 0 here. An index with no folio is refused, not reported "
+                          "clean. Each finding carries both \"folio\" (this index) and "
+                          "\"folio_number\" (counted from 1)."},
                 "elements_dir": {"type": "string"},
                 "timeout": {"type": "integer", "default": 180},
             },

misc/qet-mcp/test_qet_mcp.py

@@ -1259,6 +1259,25 @@ class QueryGuard(unittest.TestCase):
             m.tool_query("/x", "/no/such.qet", "SELECT 1")
 
 
+class ContinuityFolioGuard(unittest.TestCase):
+    """qet.checkContinuity() answers a folio index it has no folio for with an
+    empty list, which looks like a clean folio; refuse it before launch."""
+
+    def test_an_index_with_no_folio_is_refused_before_launch(self):
+        with tempfile.TemporaryDirectory() as d:
+            qet = Path(d) / "p.qet"
+            qet.write_text("<project><diagram/><diagram/></project>")
+            for folio in (2, 5, -1):
+                with self.subTest(folio=folio):
+                    with self.assertRaisesRegex(ValueError, r"2 folio\(s\), indexed 0 to 1"):
+                        m.tool_continuity("/nonexistent", str(qet), folio=folio)
+
+    def test_the_descriptions_say_how_folios_are_counted(self):
+        props = {t["name"]: t["inputSchema"]["properties"] for t in m.TOOLS}
+        self.assertIn("counted from 0", props["qet_continuity"]["folio"]["description"])
+        self.assertIn("counted from 1", props["qet_conductors"]["folio"]["description"])
+
+
 class ProjectNewValidation(unittest.TestCase):
     def test_refuses_overwrite_and_bad_arguments(self):
         with tempfile.TemporaryDirectory() as tmp:
@@ -2470,6 +2489,15 @@ class Integration(unittest.TestCase):
         self.assertGreater(both["finding_count"], 0)
         self.assertEqual(only_1["finding_count"], 0, "folio 1 has no elements at all")
 
+    def test_continuity_findings_carry_the_folio_number_qet_elements_uses(self):
+        base = self.sb.new(folios=2)
+        r = self.ok(self.sb.edit(base, [
+            {"op": "add_element", "id": "a", "folio": 1, "path": COIL, "x": 100, "y": 100}]))
+        found = m.tool_continuity(BINARY, r["output"], folio=1, elements_dir=ELEMENTS)["findings"]
+        self.assertTrue(found)
+        self.assertEqual({(f["folio"], f["folio_number"]) for f in found}, {(1, 2)})
+        self.assertEqual({e["folio"] for e in m.tool_elements(r["output"])["elements"]}, {2})
+
     def test_continuity_finds_a_report_link_colour_mismatch(self):
         """Reproduces qelectrotech/qelectrotech-source-mirror#974: a
         folio-jump conductor drawn in two different colours on either side