remove fins: key review fixes -- an exact key wins (a #n twin's removal hid its first copy), older and unparseable keys match exactly only, and the net is across 0.4 / along 2.5 mm of the wall plus height within 0.35: a removed fin gone from the rebuild no longer hides the neighbour beside it (Full coverage fill walls sit 0.5 mm apart), at the cost of a few removals coming back (m4 X30 77/84)

tests/finkeys.test.js

@@ -2,7 +2,8 @@
 // moves it a little. The key used to be matched exactly and held the wall top's z
 // and height, both measured below the part by the support gap, so a gap change
 // re-keyed every fin and every removed fin came back. Pinned here:
-//   - a gap change 0.2 -> 0.3 keeps every removal (exact keys kept none);
+//   - a gap change 0.2 -> 0.3 keeps the removals (exact keys kept none);
+//   - when the removed fin is gone, its key does NOT hide a neighbour;
 //   - a key matches its own fin, and nothing in another pose;
 //   - two walls stacked at one x/y stay two fins; a reversed line is the same fin;
 //   - an older session's key (taken at the middle VERTEX) still matches.
@@ -17,6 +18,10 @@ const records = (topo, res, rot, gap) => assignKeys(
   buildFins(topo, res, rot, { mode: 'auto', bedPad: true, tunables: { propGap: gap } })
     .fins.map((f) => ({ ...f })).filter((f) => f.line?.length));
 
+// torus: one of its eight walls grows 0.9 mm taller at gap 0.3 (its mirror twin
+// doesn't) -- past the height tolerance, so that removal comes back. The safe
+// way to miss: a fin returning beats a kept fin silently hidden.
+const MISSES = { torus: 1 };
 for (const name of ['lowledge', 'tshape', 'torus']) {
   Deno.test(`fin keys: ${name} X30 -- removals hold across a gap change, and stay in their pose`, () => {
     const topo = loadModel(name), rot = rotX(30), res = analyze(topo, 45, rot);
@@ -25,7 +30,7 @@ for (const name of ['lowledge', 'tshape', 'torus']) {
     const keys = new Set(A.map((r) => r.sig));
     assert(B.filter((r) => keys.has(r.sig)).length < A.length, `${name}: exact keys already survive -- not the 045 case`);
     const m = matchRemoved(B, keys);
-    assert(m.size === A.length, `${name}: ${m.size}/${A.length} removals kept at gap 0.3`);
+    assert(m.size === A.length - (MISSES[name] ?? 0), `${name}: ${m.size}/${A.length} removals kept at gap 0.3`);
     const self = matchRemoved(A, keys);
     assert(A.every((r) => self.get(r.id) === r.sig), `${name}: a key matched another fin of its own build`);
     const up = rotX(0), other = records(topo, analyze(topo, 45, up), up, 0.2);
@@ -46,6 +51,25 @@ Deno.test('fin keys: stacked walls at one x/y stay two fins; a reversed line is
   assert(matchRemoved(turned, new Set([a.sig])).size === 0, 'a crossing wall took the removal');
 });
 
+Deno.test("fin keys: a #n twin's removal hides that twin, not its first copy", () => {
+  // one midpoint, bearing and height, different lengths: one key text, then `#1`
+  const twins = assignKeys([rec(0, [[0, 0, 5], [4, 0, 5]]), rec(1, [[-1, 0, 5], [5, 0, 5]])]);
+  assert(twins[1].sig === `${twins[0].sig}#1`, `twins keyed ${twins.map((r) => r.sig)}`);
+  const m = matchRemoved(twins, new Set([twins[1].sig]));
+  assert(m.size === 1 && m.get(1) === twins[1].sig, `the #1 removal matched ${JSON.stringify([...m])}`);
+});
+
+Deno.test('fin keys: a removed fin gone from the rebuild hides no neighbour', () => {
+  // Full coverage's short fill walls side by side, 0.5 mm apart: remove one, then
+  // a rebuild without it -- the other must stay
+  const a = rec(0, [[0, 0, 10], [2.5, 0, 10]], 3), b = rec(1, [[0, 0.5, 10], [2.5, 0.5, 10]], 3.4);
+  const [ka] = assignKeys([a, { ...b }]);
+  assert(matchRemoved(assignKeys([b]), new Set([ka.sig])).size === 0, 'a side-by-side neighbour took the removal');
+  // end to end along one line, 1.8 mm on, a mm higher
+  const c = rec(2, [[1.8, 0, 11], [4.3, 0, 11]], 4);
+  assert(matchRemoved(assignKeys([c]), new Set([ka.sig])).size === 0, 'an end-to-end neighbour took the removal');
+});
+
 Deno.test('fin keys: one key per fin, repeats and suffixes parse', () => {
   const twins = assignKeys([rec(0, [[0, 0, 5], [4, 0, 5]]), rec(1, [[0, 0, 5], [4, 0, 5]])]);
   assert(twins[0].sig !== twins[1].sig && twins[1].sig.endsWith('#1'), `twins keyed ${twins.map((r) => r.sig)}`);

web/ui/finkeys.js

@@ -5,17 +5,29 @@
  * survives a rebuild. The key used to be matched EXACTLY, and it held the wall
  * top's z and height -- both measured below the part by the support gap, so
  * changing the gap re-keyed every fin and the removed ones came back (local issue
- * 045). A rebuild mostly moves a fin up or down, a little sideways at most (gap
- * 0.2 -> 0.3: m4 bracket 92/93 fins within 0.05 mm in x/y, the rest within 1 mm),
- * so a key now matches the NEAREST fin of the same kind and bearing within a
- * tolerance. z stays in the key as a tie-break only: two walls stacked at one x/y
- * on different levels are still two fins.
+ * 045). A rebuild mostly moves a fin up or down, a little sideways at most, so a
+ * key now matches the NEAREST fin of the same kind and bearing within a tolerance.
+ *
+ * The tolerance errs toward a removed fin coming BACK, never toward a kept fin
+ * vanishing: a fin hidden that the user didn't click is support silently lost.
+ * Measured over 975 fins (real parts + stress models, Auto and Full, upright and
+ * X30, gap 0.2 -> 0.3), split along and across each wall: the same fin's middle
+ * slides ALONG the wall as its ends change (p99 1.5 mm, torus walls 1-1.4) but
+ * barely moves ACROSS it (p90 0.06), and its height changes 0.1. Neighbours of the
+ * same bearing sit side by side (>= 0.5 mm across: Full coverage's short fill
+ * walls) or end to end at another height (>= 1 mm apart) -- so a wide round net
+ * would, once the removed fin is gone from a rebuild, hide a neighbour. Hence
+ * across <= 0.4, along <= 2.5, height within 0.35. (One near-duplicate pair on a
+ * Full-coverage ramp, 0.1 across, can't be told apart.) z is a tie-break too: two
+ * walls stacked at one x/y on different levels are still two fins.
  *
  * Pure (no DOM, no three.js) so tests can run it against real builds.
  */
 
-const XY_TOL = 1.5;     // mm a fin may move sideways and still be the same fin
-const Z_TOL = 2;        // mm up or down: a gap step moves a wall top ~0.1-0.2
+const ACROSS_TOL = 0.4; // mm a fin may move across its own line and still be the same fin
+const ALONG_TOL = 2.5;  // mm its middle may slide along the line (its ends grew or shrank)
+const H_TOL = 0.35;     // mm its height may change (a gap step: 0.1)
+const Z_TOL = 1;        // mm up or down: a gap step moves a wall top ~0.1-0.2
 const BEARING_TOL = 0.15;   // rad
 
 const r1 = (v) => Math.round(v * 10) / 10;   // 0.1 mm
@@ -61,7 +73,7 @@ export function assignKeys(records) {
     const n = seen.get(base) ?? 0;
     seen.set(base, n + 1);
     r.sig = n === 0 ? base : `${base}#${n}`;
-    r.legacySig = finKey(r, true);   // matched too, so an older session's removals hold
+    r.legacySig = finKey(r, true);   // an older session's key, matched exactly
   }
   return records;
 }
@@ -70,7 +82,7 @@ export function assignKeys(records) {
  *  suffixes that keep keys unique. */
 export function parseKey(key) {
   const m = /^([^:]+):(-?[\d.]+),(-?[\d.]+),(-?[\d.]+):(-?[\d.]+):(-?[\d.]+)(?:[#~]\d+)*$/.exec(key ?? '');
-  return m ? { kind: m[1], x: +m[2], y: +m[3], z: +m[4], bearing: +m[6] } : null;
+  return m ? { kind: m[1], x: +m[2], y: +m[3], z: +m[4], h: +m[5], bearing: +m[6] } : null;
 }
 
 /** Bearing difference of two undirected lines (a rebuild may run a wall the
@@ -81,23 +93,27 @@ function bearingGap(a, b) {
 }
 
 /**
- * Match removed keys to a build's records: each key takes the nearest record of
- * its kind and bearing within the tolerances, one record per key, closest pairs
- * first. A key with no fin near it (another pose, or a fin the new build doesn't
- * make) matches nothing and is kept for later.
+ * Match removed keys to a build's records: a key that names a record exactly
+ * takes it (a `#n` twin is told apart only by its suffix); otherwise the nearest
+ * record of its kind and bearing within the tolerances, one record per key,
+ * closest pairs first. An older session's key (middle vertex), or one that
+ * doesn't parse, matches exactly only. A key with no fin near it (another pose,
+ * or a fin the new build doesn't make) matches nothing and is kept for later.
  * Returns Map record id -> the removed key it matched.
  */
 export function matchRemoved(records, removedKeys) {
-  const keys = [...removedKeys].map((k) => [k, parseKey(k)]).filter(([, p]) => p);
-  const recs = [];
-  for (const r of records) for (const s of [r.sig, r.legacySig]) { const q = parseKey(s); if (q) recs.push([r, q]); }
   const pairs = [];
-  for (const [k, p] of keys) {
+  const recs = records.map((r) => [r, parseKey(r.sig)]).filter(([, q]) => q);
+  for (const k of removedKeys) {
+    for (const r of records) if (k === r.sig || k === r.legacySig) pairs.push([-1, k, r]);
+    const p = parseKey(k);
+    if (!p) continue;
     for (const [r, q] of recs) {
       if (p.kind !== q.kind || bearingGap(p.bearing, q.bearing) > BEARING_TOL) continue;
-      const dxy = Math.hypot(p.x - q.x, p.y - q.y), dz = Math.abs(p.z - q.z);
-      if (dxy > XY_TOL || dz > Z_TOL) continue;
-      pairs.push([Math.hypot(dxy, dz), k, r]);
+      const dx = q.x - p.x, dy = q.y - p.y, c = Math.cos(p.bearing), s = Math.sin(p.bearing);
+      const along = Math.abs(dx * c + dy * s), across = Math.abs(dy * c - dx * s), dz = Math.abs(p.z - q.z);
+      if (across > ACROSS_TOL || along > ALONG_TOL || dz > Z_TOL || Math.abs(p.h - q.h) > H_TOL) continue;
+      pairs.push([Math.hypot(along, 4 * across, dz), k, r]);   // across weighs more: it's the neighbour axis
     }
   }
   pairs.sort((a, b) => a[0] - b[0]);