Tell apart two terminals at one point; qet_diff: same wire in both forms

Review of the previous commit:
- The load fallback compared a saved uuid with occurrence 0 only, so a
  wire on the second of two terminals at one point of a symbol was lost
  once the definition was replaced, and one on the first could go to
  either of the pair (Element::m_terminals is sorted, not in definition
  order). Element::parseTerminal() now records each terminal's rank among
  the terminals of the definition at the same point, derivedUuid() uses
  it, and fillMissing() starts from the same rank.
  derivedUuidFoundAfterReplacement runs on perceuse.qet and industrial.qet
  too: 154/156 and 670/671 wires without the rank, all with it.

qet-mcp: the first save of an older project now rewrites its wires from
the numbered form to the uuid form, and qet_diff keyed the two forms
differently, so an untouched resave showed every wire removed and added
(4 failures in test_qet_mcp.py). A uuid end is now resolved to the same
key as a numbered one: the terminal's definition position, moved to where
the wire docks, is the placed symbol's <terminal> record.
test_conductor_key_same_in_both_forms fails without it; 253/253 pass on
this build and on the previous stage's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

misc/qet-mcp/qet_mcp.py

@@ -126,12 +126,56 @@ def _wires(diagram: ET.Element):
 
 
 def _conductors(root: ET.Element):
+    definitions = _definition_terminals(root)
     for i, d in _folios(root):
-        index = _terminal_index(d)
+        index = _terminal_index(d, definitions)
         for c in _wires(d):
             yield i, c, index
 
 
+def _uuid_key(value: str) -> str:
+    return (value or "").strip().strip("{}").lower()
+
+
+# Orientation as a placed symbol's <terminal> record writes it (an int,
+# Qet::Orientation) or as a definition does (n/e/s/w).
+_ORIENTATIONS = {"n": 0, "e": 1, "s": 2, "w": 3, "0": 0, "1": 1, "2": 2, "3": 3}
+
+# Where QElectroTech docks a wire, relative to the terminal's position in
+# its definition (Terminal's constructor, Terminal::terminalSize = 4). A
+# placed symbol's <terminal> record is written at that point.
+_DOCK_OFFSET = {0: (0.0, 4.0), 1: (-4.0, 0.0), 2: (0.0, -4.0), 3: (4.0, 0.0)}
+
+
+def _definition_terminals(root: ET.Element) -> dict:
+    """Map each symbol stored in the project ("embed://" + its path in the
+    <collection>) to its terminals: {terminal uuid: (x, y, orientation)},
+    the position being the one in the definition."""
+    out = {}
+
+    def walk(node, path):
+        for child in node:
+            if child.tag == "category":
+                walk(child, path + [child.get("name", "")])
+            elif child.tag == "element":
+                terminals = {}
+                for t in child.findall("definition/description/terminal"):
+                    try:
+                        terminals[_uuid_key(t.get("uuid"))] = (
+                            float(t.get("x")), float(t.get("y")),
+                            _ORIENTATIONS.get((t.get("orientation") or "n")[:1], 0))
+                    except (TypeError, ValueError):
+                        continue
+                terminals.pop("", None)
+                if terminals:
+                    out["embed://" + "/".join(path + [child.get("name", "")])] = terminals
+
+    collection = root.find("collection")
+    if collection is not None:
+        walk(collection, [])
+    return out
+
+
 def _element_row(folio: int, el: ET.Element) -> dict:
     info = _element_info(el)
     etype = el.get("type", "")
@@ -147,7 +191,7 @@ def _element_row(folio: int, el: ET.Element) -> dict:
     }
 
 
-def _terminal_index(diagram: ET.Element) -> dict:
+def _terminal_index(diagram: ET.Element, definitions: dict | None = None) -> dict:
     """Map a folio's terminal ids to an identity that survives a save.
 
     A conductor names its ends with terminal1/terminal2, which are plain
@@ -170,6 +214,17 @@ def _terminal_index(diagram: ET.Element) -> dict:
     Conductors in the corpus carry no element1/element2 attribute -- 0 of
     47 in ArduinoLCD.qet, 0 of 67 in 741.qet -- so this mapping has to be
     built from the elements rather than read off the conductor.
+
+    A conductor can also name its ends by terminal uuid (element1 +
+    terminal1), and QElectroTech writes that form as soon as the terminal
+    has a uuid -- which, since a project gives every terminal one on
+    opening, is the first save of any older file. So the same untouched
+    conductor is written in the numbered form before a save and the uuid
+    form after it. With @p definitions (from _definition_terminals()), a
+    uuid end is resolved too, keyed (element uuid, terminal uuid), to the
+    very same identity as the numbered end: the terminal's definition
+    position, moved to where the wire docks, is where the placed symbol's
+    <terminal> record is.
     """
     index = {}
     for el in diagram.iter("element"):
@@ -184,12 +239,26 @@ def _terminal_index(diagram: ET.Element) -> dict:
             # marked with a "#" so the caller can see the diff is on the
             # unstable footing that file forces.
             continue
+        records = []
         for t in el.iter("terminal"):
             tid = t.get("id")
             if tid is None:
                 continue
-            index[tid] = (f"{uuid}@{t.get('x','?')},{t.get('y','?')}"
-                          f",{t.get('orientation','?')}")
+            key = (f"{uuid}@{t.get('x','?')},{t.get('y','?')}"
+                   f",{t.get('orientation','?')}")
+            index[tid] = key
+            records.append((t, key))
+        for tuuid, (x, y, o) in (definitions or {}).get(el.get("type", ""), {}).items():
+            dx, dy = _DOCK_OFFSET[o]
+            for t, key in records:
+                try:
+                    if (abs(float(t.get("x")) - (x + dx)) < 1e-6
+                            and abs(float(t.get("y")) - (y + dy)) < 1e-6
+                            and _ORIENTATIONS.get((t.get("orientation") or "")[:1]) == o):
+                        index[(_uuid_key(uuid), tuuid)] = key
+                        break
+                except (TypeError, ValueError):
+                    continue
     return index
 
 
@@ -214,7 +283,8 @@ def _conductor_key(folio: int, c: ET.Element, index: dict) -> str:
         tid = c.get(term_attr, "?")
         owner = c.get(elem_attr)
         if owner:
-            ends.append(f"{owner}/{tid or c.get(name_attr, '?')}")
+            ends.append(index.get((_uuid_key(owner), _uuid_key(tid)))
+                        or f"{owner}/{tid or c.get(name_attr, '?')}")
         else:
             # An id with no element behind it stays visible as itself
             # rather than silently collapsing conductors onto one key.

misc/qet-mcp/test_qet_mcp.py

@@ -901,6 +901,30 @@ class ReadToolContracts(unittest.TestCase):
                 r = tool(str(big))
                 self.assertEqual((r["count"], r["truncated"], len(r[key])), (201, True, 200))
 
+    def test_conductor_key_same_in_both_forms(self):
+        # A wire in the numbered form before a save and the uuid form after
+        # it (the first save of an older project) is the same wire: both
+        # ends resolve to the placed symbol's terminal record. The record is
+        # where the wire docks, 4 from the definition position.
+        root = ET.fromstring(
+            '<project><collection><category name="import"><category name="x">'
+            '<element name="S.elmt"><definition><description>'
+            '<terminal uuid="{T1}" x="0" y="0" orientation="s"/>'
+            '<terminal uuid="{T2}" x="10" y="0" orientation="e"/>'
+            '</description></definition></element>'
+            '</category></category></collection>'
+            '<diagram><elements><element uuid="{E}" type="embed://import/x/S.elmt">'
+            '<terminals><terminal id="5" x="0" y="-4" orientation="2"/>'
+            '<terminal id="6" x="6" y="0" orientation="1"/></terminals>'
+            '</element></elements><conductors>'
+            '<conductor terminal1="5" terminal2="6"/>'
+            '<conductor element1="{e}" terminal1="{t1}" element2="{E}" terminal2="{T2}"/>'
+            '</conductors></diagram></project>')
+        numbered, by_uuid = [m._conductor_row(i, c, ix)["key"]
+                             for i, c, ix in m._conductors(root)]
+        self.assertEqual(numbered, "1:{E}@0,-4,2--{E}@6,0,1")
+        self.assertEqual(by_uuid, numbered)
+
     def test_conductor_row_without_an_index(self):
         c = ET.fromstring('<conductor terminal1="7" terminal2="8"/>')
         self.assertEqual(m._conductor_row(3, c)["key"], "3:#7--#8")

sources/ElementsCollection/terminaluuids.cpp

@@ -42,6 +42,15 @@ QList<QDomElement> terminalsOf(const QDomElement &collection_element)
 	return terminals;
 }
 
+	//Place and orientation, "10" and "10.0" being the same place
+QString terminalPlace(const QDomElement &terminal)
+{
+	return QStringLiteral("%1|%2|%3")
+			.arg(QString::number(terminal.attribute(QStringLiteral("x")).toDouble()),
+				 QString::number(terminal.attribute(QStringLiteral("y")).toDouble()),
+				 terminal.attribute(QStringLiteral("orientation")));
+}
+
 	//Qet::orientationFromString(), without pulling in qet.cpp
 int orientationOf(const QDomElement &terminal)
 {
@@ -65,7 +74,11 @@ int fillDefinition(const QDomElement &collection_element)
 	}
 
 	int filled = 0;
+	QHash<QString, int> seen_at;
 	for (QDomElement t : terminals) {
+			//Same count as Terminal::setPlaceRank(): every terminal before
+			//this one at the same point, with or without a uuid
+		const int rank = seen_at[terminalPlace(t)]++;
 		if (!QUuid(t.attribute(QStringLiteral("uuid"))).isNull()) {
 			continue;
 		}
@@ -73,7 +86,7 @@ int fillDefinition(const QDomElement &collection_element)
 		const qreal y = t.attribute(QStringLiteral("y")).toDouble();
 		const int orientation = orientationOf(t);
 		QUuid uuid;
-		for (int occurrence = 0 ; uuid.isNull() || taken.contains(uuid) ; ++occurrence) {
+		for (int occurrence = rank ; uuid.isNull() || taken.contains(uuid) ; ++occurrence) {
 			uuid = TerminalUuids::derived(x, y, orientation, occurrence);
 		}
 		taken << uuid;
@@ -98,14 +111,6 @@ int fillDirectory(const QDomElement &directory)
 	return filled;
 }
 
-	//Place and orientation, "10" and "10.0" being the same place
-QString terminalPlace(const QDomElement &terminal)
-{
-	return QStringLiteral("%1|%2|%3")
-			.arg(QString::number(terminal.attribute(QStringLiteral("x")).toDouble()),
-				 QString::number(terminal.attribute(QStringLiteral("y")).toDouble()),
-				 terminal.attribute(QStringLiteral("orientation")));
-}
 }
 
 /**

sources/qetgraphicsitem/element.cpp

@@ -697,6 +697,16 @@ Terminal *Element::parseTerminal(const QDomElement &dom_element)
 	}
 
 	Terminal *new_terminal = new Terminal(data, this);
+		//Terminals are parsed in the order of the definition, and the list
+		//is not kept in that order (sort below)
+	int rank = 0;
+	for (Terminal *t : std::as_const(m_terminals)) {
+		if (t->dock_elmt_ == new_terminal->dock_elmt_
+			&& t->orientation() == new_terminal->orientation()) {
+			++rank;
+		}
+	}
+	new_terminal->setPlaceRank(rank);
 	m_terminals << new_terminal;
 
 	connect(new_terminal, &Terminal::conductorWasAdded,   this, &Element::updateConductorTexts);

sources/qetgraphicsitem/terminal.cpp

@@ -884,12 +884,28 @@ QUuid Terminal::stableUuid() const
 	needed: keying on geometry alone produces exactly the same number of
 	collisions across the example corpus, and it means renaming a terminal
 	does not change what it is.
+
+	A second terminal at the same point with the same orientation gets the
+	next occurrence (see setPlaceRank()), as TerminalUuids::fillMissing()
+	gives it, so the two terminals of such a pair are told apart.
 */
 QUuid Terminal::derivedUuid() const
 {
 	return TerminalUuids::derived(d->m_pos.x(),
 								  d->m_pos.y(),
-								  static_cast<int>(d->m_orientation));
+								  static_cast<int>(d->m_orientation),
+								  m_place_rank);
+}
+
+/**
+	@brief Terminal::setPlaceRank
+	@param rank : how many terminals of the definition, before this one in
+	document order, sit at the same point with the same orientation.
+	Set by Element::parseTerminal().
+*/
+void Terminal::setPlaceRank(int rank)
+{
+	m_place_rank = rank;
 }
 
 QString Terminal::name() const

sources/qetgraphicsitem/terminal.h

@@ -77,6 +77,7 @@ class Terminal : public QGraphicsObject
 		QUuid     uuid                () const;
 		QUuid     stableUuid          () const;
 		QUuid     derivedUuid         () const;
+		void      setPlaceRank        (int rank);
 		QString   name                () const;
 		QString   baseName            () const;
 		TerminalData::Type terminalType() const;
@@ -143,6 +144,9 @@ class Terminal : public QGraphicsObject
 		Terminal *m_previous_terminal = nullptr;
 			/// Whether the mouse pointer is hovering the terminal
 		bool m_hovered = false;
+			/// How many terminals of the definition, before this one, sit at
+			/// the same point with the same orientation; see derivedUuid()
+		int m_place_rank = 0;
 			/// Color used for the hover effect
 		QColor m_hovered_color = Terminal::neutralColor;
 

tests/qttest/tst_terminaluuids.cpp

@@ -341,9 +341,19 @@ private slots:
 	// The uuids written on opening are derived from where each terminal
 	// is: a wire saved against one still finds its terminal after the
 	// symbol's definition was replaced by one with other terminal uuids.
+	// perceuse.qet and industrial.qet have wires on the second of two
+	// terminals at one point of a symbol, which get the next occurrence.
+	void derivedUuidFoundAfterReplacement_data()
+	{
+		QTest::addColumn<QString>("project");
+		for (const char *name : {"tremie_vibrante.qet", "perceuse.qet", "industrial.qet"})
+			QTest::newRow(name) << QStringLiteral(QET_EXAMPLES_DIR "/") + QLatin1String(name);
+	}
+
 	void derivedUuidFoundAfterReplacement()
 	{
-		const QString saved = resave(QStringLiteral(QET_EXAMPLES_DIR "/tremie_vibrante.qet"));
+		QFETCH(QString, project);
+		const QString saved = resave(project);
 		QVERIFY(!saved.isEmpty());
 		QString log;
 		const int wires = loadedWires(saved, &log);
@@ -356,7 +366,7 @@ private slots:
 				terminals.at(i).toElement().setAttribute(QStringLiteral("uuid"),
 														 QUuid::createUuid().toString());
 		}
-		const QString replaced = m_dir.filePath(QStringLiteral("replaced.qet"));
+		const QString replaced = m_dir.filePath(QStringLiteral("replaced%1.qet").arg(m_run));
 		QFile out(replaced);
 		QVERIFY(out.open(QIODevice::WriteOnly));
 		out.write(doc.toByteArray());