fix(cliffs): port fixImpossibleCells' self-retry (#18)

docs/noise/cliffs-NOTES.md

@@ -783,9 +783,36 @@ and compared against `CLIFF_PLACED_TABLE`: **the accepted set is exactly
 Two corrections to the paragraph this replaced:
 
 - It does **not** zero the whole chunk border. The `bool` parameter gates zeroing
-  the outer edges of the four CORNER cells only (8 edges) - and
-  `crossingsForChunk` passes **`false`** (`mov w1, #0x0` at `0x10160d0c8`), so
-  that step never runs in this path at all.
+  the outer edges of the four CORNER cells only (8 edges).
+- ~~`crossingsForChunk` passes **`false`**, so that step never runs in this path
+  at all.~~ **WRONG, corrected 2026-07-30 by decompiling the function whole.**
+  The caller does pass `false`, but the `bool` is not a caller-supplied mode - it
+  is a **retry flag the function sets on itself.** On reaching a cell it cannot
+  fix, the tail does
+
+  ```
+  uVar10 = param_2 & 1;  param_2 = 1;
+  if (uVar10 != 0) { log("Unable to remove excess cliff cell edge crossings"); return; }
+  goto <top of function>;
+  ```
+
+  so it turns the flag on and **restarts the entire pass**, which now begins by
+  zeroing those eight corner edges; a second failure abandons the rest of the
+  chunk. Note the restart re-sweeps the arrays **as already mutated** by the
+  abandoned pass, not the raw crossings.
+
+  This is the error the earlier note made, in its general form: reading a
+  parameter's value at the call site says what the caller wants, not what the
+  function does with it. Ported 2026-07-30 with `test/fixImpossibleCellsRetry.spec.ts`.
+
+  **Do not read it as a fix for issue #18.** Measured over the committed
+  captures, the retry fires **once in 512 chunks** - one chunk of Vulcanus
+  `[1500,1500]`, zero across both Nauvis seeds and the other two Vulcanus regions
+  - and changes not one placed cell: `[0,0]` 335, `[1500,1500]` 1065,
+  `[-1200,800]` 375 before and after, with recall, precision and the orientation
+  count all identical. It is correctness, not progress. Because the integration
+  fixtures barely execute the branch, it has a dedicated unit spec that builds a
+  stuck corner directly; disabling the retry fails 3 of its 5 tests.
 - The binary is a **universal** Mach-O. Raw byte reads of those jump tables need
   the arm64 slice offset (115654656 here) added, or they silently return x86_64
   bytes - which is exactly what happened on the first extraction attempt and

src/noise/cliffs/cliffPlacement.ts

@@ -157,30 +157,74 @@ function cellCode(l: number, r: number, t: number, b: number): number {
  * ported. An earlier note in cliffs-NOTES.md described this pass as zeroing the
  * whole chunk border; it does not, and it does not run at all here.
  */
-function fixImpossibleCellsSweep(v: Int8Array, h: Int8Array, w: number, hh: number): void {
+export function fixImpossibleCellsSweep(v: Int8Array, h: Int8Array, w: number, hh: number): void {
   const vIndex = (cx: number, cy: number): number => cy * (w + 1) + cx;
   const hIndex = (cx: number, cy: number): number => cy * w + cx;
 
-  for (let cy = 0; cy < hh; cy++) {
-    for (let cx = 0; cx < w; cx++) {
-      const li = vIndex(cx, cy);
-      const ri = vIndex(cx + 1, cy);
-      const ti = hIndex(cx, cy);
-      const bi = hIndex(cx, cy + 1);
-
-      for (;;) {
-        const code = cellCode(v[li], v[ri], h[ti], h[bi]);
-        if (code === 0 || isCliffPlaced(code)) break;
-        if (v[li] !== 0 && cx !== 0) v[li] = 0;
-        else if (h[ti] !== 0 && cy !== 0) h[ti] = 0;
-        else if (v[ri] !== 0 && cx < w - 1) v[ri] = 0;
-        else if (h[bi] !== 0 && cy < hh - 1) h[bi] = 0;
-        // The game logs "Unable to remove excess cliff cell edge crossings" and
-        // gives up here; every remaining crossing is on the chunk boundary and
-        // is not ours to clear.
-        else break;
+  /**
+   * The `bool` parameter, and it is a **retry flag the function sets on
+   * itself** - not a caller-supplied mode, which is how it was read until
+   * 2026-07-30. `crossingsForChunk` passes `false`, and an earlier note here
+   * concluded from that alone that the corner step "never runs in this path".
+   * It does. When the sweep reaches a cell it cannot fix, the disassembly does
+   *
+   *     uVar10 = param_2 & 1;  param_2 = 1;
+   *     if (uVar10 != 0) { log(...); return; }
+   *     goto <top of function>;
+   *
+   * i.e. it turns the flag on and **restarts the whole pass**, which this time
+   * begins by zeroing the eight outer edges of the chunk's four corner cells.
+   * A second failure logs "Unable to remove excess cliff cell edge crossings"
+   * and abandons the rest of the chunk outright.
+   *
+   * Note the restart re-sweeps the arrays **as already mutated** by the
+   * abandoned pass - it is not a fresh start from the raw crossings.
+   */
+  for (let retry = 0; ; retry++) {
+    if (retry > 0) {
+      // The eight edges: the two outer edges of each corner cell. Zeroing these
+      // is what can make an otherwise unfixable corner cell legal, since its
+      // only remaining crossings were the ones the sweep is forbidden to clear.
+      v[vIndex(0, 0)] = 0;
+      h[hIndex(0, 0)] = 0;
+      v[vIndex(w, 0)] = 0;
+      h[hIndex(w - 1, 0)] = 0;
+      v[vIndex(0, hh - 1)] = 0;
+      h[hIndex(0, hh)] = 0;
+      v[vIndex(w, hh - 1)] = 0;
+      h[hIndex(w - 1, hh)] = 0;
+    }
+
+    let stuck = false;
+    for (let cy = 0; cy < hh && !stuck; cy++) {
+      for (let cx = 0; cx < w && !stuck; cx++) {
+        const li = vIndex(cx, cy);
+        const ri = vIndex(cx + 1, cy);
+        const ti = hIndex(cx, cy);
+        const bi = hIndex(cx, cy + 1);
+
+        for (;;) {
+          const code = cellCode(v[li], v[ri], h[ti], h[bi]);
+          // The engine first counts non-zero edges and only consults the table
+          // when the count is below 3. That is pure optimisation: every one of
+          // the 20 placing codes has one or two crossings, so a count of 3 or 4
+          // can never be legal. Checking the table directly is equivalent.
+          if (code === 0 || isCliffPlaced(code)) break;
+          if (v[li] !== 0 && cx !== 0) v[li] = 0;
+          else if (h[ti] !== 0 && cy !== 0) h[ti] = 0;
+          else if (v[ri] !== 0 && cx < w - 1) v[ri] = 0;
+          else if (h[bi] !== 0 && cy < hh - 1) h[bi] = 0;
+          else {
+            stuck = true;
+            break;
+          }
+        }
       }
     }
+
+    // Not stuck -> the pass completed. Stuck on the retry -> the engine logs and
+    // abandons the chunk, leaving the arrays as they are.
+    if (!stuck || retry > 0) return;
   }
 }
 

test/fixImpossibleCellsRetry.spec.ts

@@ -0,0 +1,127 @@
+import { describe, expect, it } from "vite-plus/test";
+
+import { fixImpossibleCellsSweep } from "../src/noise/cliffs/cliffPlacement";
+import { isCliffPlaced } from "../src/noise/cliffs/cliffCatalog";
+
+const W = 8;
+const H = 8;
+const vIndex = (cx: number, cy: number): number => cy * (W + 1) + cx;
+const hIndex = (cx: number, cy: number): number => cy * W + cx;
+const codeOf = (v: Int8Array, h: Int8Array, cx: number, cy: number): number =>
+  ((v[vIndex(cx, cy)] & 3) << 6) |
+  ((v[vIndex(cx + 1, cy)] & 3) << 4) |
+  ((h[hIndex(cx, cy)] & 3) << 2) |
+  (h[hIndex(cx, cy + 1)] & 3);
+
+/**
+ * `CellEdgeCliffCrossingArray::fixImpossibleCells`' **retry**, which the port did
+ * not have until 2026-07-30 (issue #18).
+ *
+ * The `bool` parameter was read as a caller-supplied mode, and since
+ * `crossingsForChunk` passes `false` an earlier note concluded the corner step
+ * "never runs in this path". It does - the function sets the flag on **itself**:
+ *
+ * ```
+ * uVar10 = param_2 & 1;  param_2 = 1;
+ * if (uVar10 != 0) { log("Unable to remove excess cliff cell edge crossings"); return; }
+ * goto <top of function>;
+ * ```
+ *
+ * So on reaching a cell it cannot fix, it restarts the whole pass, this time
+ * first zeroing the eight outer edges of the chunk's four corner cells; a second
+ * failure abandons the chunk.
+ *
+ * **This spec exists because the integration fixtures barely exercise it.**
+ * Measured over the committed captures, the retry fires **once in 512 chunks**
+ * (one chunk of Vulcanus `[1500,1500]`; zero across both Nauvis seeds and the
+ * other two Vulcanus regions) and changes no placed cell. That is a real
+ * behaviour and worth having right, but it is nowhere near issue #18's residual
+ * - do not read this as a fix for it. Without a direct test the branch would be
+ * effectively dead code.
+ */
+describe("fixImpossibleCells retry", () => {
+  /**
+   * A corner cell whose only crossings are the two chunk-boundary edges it is
+   * forbidden to clear. `L = +1`, `T = -1` gives code `0x4C`, which is not a
+   * placing code; `R` and `B` are zero, so the L/T/R/B search finds nothing
+   * clearable and the first pass is stuck.
+   */
+  const stuckCorner = (): { v: Int8Array; h: Int8Array } => {
+    const v = new Int8Array((W + 1) * H);
+    const h = new Int8Array(W * (H + 1));
+    v[vIndex(0, 0)] = 1;
+    h[hIndex(0, 0)] = -1;
+    return { v, h };
+  };
+
+  it("the constructed cell really is stuck and illegal, or this spec proves nothing", () => {
+    const { v, h } = stuckCorner();
+    const code = codeOf(v, h, 0, 0);
+    expect(code).toBe(0x4c);
+    expect(isCliffPlaced(code)).toBe(false);
+    // Both crossings are on the chunk boundary: L at cx 0, T at cy 0. Neither is
+    // clearable, and the other two edges are already zero.
+    expect(v[vIndex(1, 0)]).toBe(0);
+    expect(h[hIndex(0, 1)]).toBe(0);
+  });
+
+  it("restarts and zeroes the corner cell's outer edges, making it legal", () => {
+    const { v, h } = stuckCorner();
+    fixImpossibleCellsSweep(v, h, W, H);
+    // The retry's corner step is the only thing that can clear these two.
+    expect(v[vIndex(0, 0)]).toBe(0);
+    expect(h[hIndex(0, 0)]).toBe(0);
+    const code = codeOf(v, h, 0, 0);
+    expect(code).toBe(0);
+    expect(isCliffPlaced(code) || code === 0).toBe(true);
+  });
+
+  it("leaves every cell of the chunk legal", () => {
+    const { v, h } = stuckCorner();
+    fixImpossibleCellsSweep(v, h, W, H);
+    for (let cy = 0; cy < H; cy++)
+      for (let cx = 0; cx < W; cx++) {
+        const code = codeOf(v, h, cx, cy);
+        expect(code === 0 || isCliffPlaced(code)).toBe(true);
+      }
+  });
+
+  /**
+   * The corner step must not fire when nothing is stuck - it clears eight edges
+   * unconditionally, so running it on a healthy chunk would delete real cliffs.
+   */
+  it("does NOT touch the corner edges when the pass completes normally", () => {
+    const v = new Int8Array((W + 1) * H);
+    const h = new Int8Array(W * (H + 1));
+    // `L = +1`, `T = +1` is code 0x44, which IS a placing code, so cell (0,0) is
+    // legal as it stands and the sweep has nothing to do anywhere.
+    v[vIndex(0, 0)] = 1;
+    h[hIndex(0, 0)] = 1;
+    expect(isCliffPlaced(codeOf(v, h, 0, 0))).toBe(true);
+    fixImpossibleCellsSweep(v, h, W, H);
+    expect(v[vIndex(0, 0)]).toBe(1);
+    expect(h[hIndex(0, 0)]).toBe(1);
+  });
+
+  /**
+   * Two stuck corners at once. The restart re-sweeps the arrays **as already
+   * mutated** by the abandoned pass rather than starting from the raw crossings,
+   * and one retry clears all eight corner edges, so both are fixed in the single
+   * permitted restart - the second failure would abandon the chunk.
+   */
+  it("fixes two stuck corners in one restart", () => {
+    const v = new Int8Array((W + 1) * H);
+    const h = new Int8Array(W * (H + 1));
+    v[vIndex(0, 0)] = 1;
+    h[hIndex(0, 0)] = -1;
+    v[vIndex(W, H - 1)] = 1;
+    h[hIndex(W - 1, H)] = -1;
+    expect(isCliffPlaced(codeOf(v, h, W - 1, H - 1))).toBe(false);
+    fixImpossibleCellsSweep(v, h, W, H);
+    for (let cy = 0; cy < H; cy++)
+      for (let cx = 0; cx < W; cx++) {
+        const code = codeOf(v, h, cx, cy);
+        expect(code === 0 || isCliffPlaced(code)).toBe(true);
+      }
+  });
+});