Code review: one definition per rule, ESLint gate, teardown fixes, docs reconciled

Full review per CODE.REVIEW.md (style, language conformity, doc/code
consistency, and shared-data redundancy).

Bugs fixed:
- ROIDrawer.destroy() now clears the baked overlay layer, so autoAttach's
  destroy-then-attach no longer leaves an orphan #drawnrois layer + stale
  svgo.layers entry (the adapter's comment claimed this already happened).
- One isTextEntry() rule (ui/dom-utils.js) for every keyboard guard: the
  edit overlay's Delete guard blocked ALL inputs, so Delete went dead
  while the Import file input held focus.
- LassoOverlay's 800 ms settle re-measure is tracked and cancelled on destroy.
- test fake's setOverlayLayer returns the contract's boolean.

Redundancy collapsed to single homes:
- Bezier segment topology (segCount/segControls/minAnchors/hasCurve in
  core/bezier.js) now feeds both samplers, both nearest-point searches,
  deleteAnchor, the edit overlay, and the adapter's SVG path writer;
  test/bezier-topology.test.js pins that they agree.
- uvPxCorrespondences()+ALL_UV_BOUNDS (viewer-adapter.js) replaces two
  copies; ndcToPixel replaces the adapter's _ndc; polygonBounds replaces
  two bbox loops; TimerSet (core/timer-set.js) replaces two _later/Set
  copies; CanvasOverlay (ui/overlay-canvas.js) is the shared overlay base;
  MODE/TOOL/asTool name the mode+tool vocabulary; ShapeSet.defaultName
  unifies prompt defaults with the import fallback; fitRingBezier,
  _promptName, _export dedupe the controller; make_viewer.py imports
  bake.bake(); test:py uses unittest discover.

Style gate: eslint flat config (recommended + repo conventions), run
first by npm test and CI; tree is lint-clean; README documents the
conventions. Docs reconciled across 13 stale claims (viewer-adapter
header, TESTING.md phantom test, ROI-only wording, the "Not here"
pointer at a private repo from this now-public one, etc.).

Tests 161 -> 174 JS + 12 Python, all green; bundle rebuilt and
smoke-tested headless (Firefox) against a real baked viewer, including
destroy + re-attach with no orphan layer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: jackgallant

.github/workflows/test.yml

@@ -24,6 +24,6 @@ jobs:
       # Reproducible install from the committed package-lock.json.
       - run: npm ci
 
-      # `npm test` runs `pretest` (build the bundle) then the JS core/pipeline/contract/bundle
-      # tests and the Python tooling tests.
+      # `npm test` lints, runs `pretest:js` (build the bundle), then the JS
+      # core/pipeline/contract/bundle tests and the Python tooling tests.
       - run: npm test

README.md

@@ -200,22 +200,29 @@ Three layers; only the adapter knows the host viewer.
 
 ```
 core/      pure JS — no DOM, no THREE, no host globals (unit-tested under node)
-  geom.js        point-in-polygon, RDP simplify, Chaikin smooth, ndc↔pixel, centroid
+  geom.js        point-in-polygon, bounds, RDP simplify, Chaikin smooth, ndc↔pixel, centroid
   selection.js   projected vertices + polygon → selected vertex set (works in px OR uv)
   outline.js     polygon → ordered boundary ring of vertices (+ label vertex)
-  bezier.js      fit an editable bezier — closed (ROI ring) or open (sulcus trace) — and edit it
+  bezier.js      fit an editable bezier — closed (ROI ring) or open (sulcus trace) — sample it,
+                 edit it; owns the segment topology (segCount/segControls/minAnchors) that the
+                 samplers, the editor and the adapter's path writer all read
   transform.js   uv↔px homography (place/grab edit knots; map a traced stroke back to uv)
-  shape-model.js the shape collection (ROIs + sulci) + the vertexset-v2 ROI export/import
+  shape-model.js the shape collection (ROIs + sulci), default names, the vertexset-v2 ROI export/import
   svg-export.js  pure writer for pycortex overlays.svg sulci markup (paths only, never labels)
-  draw-mode.js   the flat-only Draw state machine (the "reached flat" latch)
+  draw-mode.js   the flat-only Draw state machine (the "reached flat" latch) + the MODE names
+  timer-set.js   tracked setTimeouts that one destroy() cancels (controller + adapter share it)
 
 adapter/   the ViewerAdapter CONTRACT + one host implementation
-  viewer-adapter.js     documented interface the core/ui depend on
+  viewer-adapter.js     documented interface the core/ui depend on, + uvPxCorrespondences()
+                        (the one place projectVerticesInUvBounds is flattened for a homography fit)
   pycortex-adapter.js   the ONLY file that touches pycortex internals
 
 ui/        host-agnostic DOM components (talk only to core + adapter)
+  overlay-canvas.js  base class: the transparent canvas over the surface (size/position sync,
+                     event→px, teardown) that both overlays below extend
   lasso-overlay.js  bezier-edit-overlay.js  draw-panel.js  mode-toggle.js  roidraw.css
   overlay-geom.js   pure hit-testing math for the edit overlay (no DOM; unit-tested)
+  dom-utils.js      isTextEntry(): the one "is the user typing?" rule every key handler uses
 
 draw-pipeline.js  ROI: lasso → select → fit bezier → re-derive membership.
                   Sulcus: trace → px→uv via homography → fit open bezier → label vertex
@@ -240,23 +247,38 @@ viewBox coords, label `data-ptidx` convention, control-panel internals) is quara
 ## Building
 
 ```bash
-npm install        # one-time (esbuild)
+npm install        # one-time (esbuild + eslint)
 npm run build      # -> dist/roidraw.bundle.js
 ```
 
 ## Testing
 
 ```bash
-npm test           # builds the bundle, then runs the JS suite (node) + Python tooling tests
+npm test           # lints, builds the bundle, then runs the JS suite (node) + Python tooling tests
+npm run lint       # eslint alone (eslint.config.js: recommended rules + this repo's conventions)
 ```
 
 The JS suite layers property-based geometry invariants (closed *and* open curves), the Draw-mode
 state machine, the draw pipeline (driven headless against a synthetic-surface adapter), the pure
-`overlays.svg` writer, the edit-overlay hit-testing, an adapter-contract guard, a host preflight,
-and a smoke test of the built bundle. CI (`.github/workflows/test.yml`) runs it on every push. See
+`overlays.svg` writer, the edit-overlay hit-testing, the bezier segment-topology agreement between
+the samplers/editor/adapter path writer, an adapter-contract guard, a host preflight, and a smoke
+test of the built bundle. CI (`.github/workflows/test.yml`) runs it on every push. See
 [TESTING.md](TESTING.md) for what each layer guarantees — and the gaps (live-browser integration)
 it can't.
 
+### Conventions
+
+- **JS**: ES modules, `const`/`let`, 4-space indent, double quotes, semicolons; `camelCase` for
+  functions and variables, `UPPER_SNAKE` for module constants, a leading `_` for private methods
+  and for deliberately unused parameters. A file starts with a block comment saying what it owns
+  and what it must not know about. `npm run lint` enforces the mechanical part.
+- **Python** (`bake.py`, `examples/`, `test/test_*.py`): PEP 8, stdlib only, `unittest`, the same
+  4-space/snake_case conventions as pycortex itself.
+- **One definition per rule.** Anything two code paths must agree on — the bezier's segment
+  topology, the overlay's coordinate mapping, the "is the user typing?" test, default shape names,
+  the timer bookkeeping — lives in exactly one module and is imported from there (see the
+  Architecture notes above). If you find the same rule restated in two places, that is the bug.
+
 ## Requirements
 
 - **Node** ≥ 18 to build/test the JS.
@@ -265,10 +287,7 @@ it can't.
 
 ## Not here
 
-General pycortex **viewer-modernization** tooling — `reengine.py` (re-emit a static viewer's engine
-from a current pycortex checkout), `fixups.py` (correct long-standing static-viewer UI bugs),
-`add_help.py` (inject a help menu into viewers built without one), and `convert_huth.py` — used to
-live in this repo. They were moved out on 2026-07-13, because this repo is the ROI-drawing project
-and that tooling applies to any pycortex viewer. They now live in `jackgallant/pycortex-viewer-tools`.
-
-`bake.py` stayed: it injects the ROI-drawing bundle, so it *is* part of this project.
+General pycortex viewer-maintenance tooling (`reengine.py`, `fixups.py`, `add_help.py`,
+`convert_huth.py`) once lived here and was moved out in July 2026 to a separate tooling repository
+(`jackgallant/pycortex-viewer-tools`, currently private). This repo is the ROI/sulcus-drawing
+project only; `bake.py` stays because it injects the drawing bundle.

TESTING.md

@@ -1,7 +1,9 @@
 # Testing & correctness
 
-`npm test` builds the bundle (via `pretest:js`) and runs the JS suite (`node --test`) plus the Python
-tooling tests. CI runs the same on every push/PR to `main` (`.github/workflows/test.yml`).
+`npm test` lints (`eslint`), builds the bundle (via `pretest:js`), runs the JS suite (`node --test`)
+and then the Python tooling tests (`unittest discover` over `test/test_*.py`, so a new Python test
+file is picked up without editing `package.json`). CI runs the same on every push/PR to `main`
+(`.github/workflows/test.yml`).
 
 The suite is layered by how strong a guarantee each layer can give.
 
@@ -24,8 +26,27 @@ prints the seed and reproduces deterministically):
   (move/smooth-toggle/delete/split), and `isClosed` reports `false` throughout — a sulcus can never
   silently acquire a closing segment.
 
-The example-based core tests (`geom`, `selection`, `bezier`, `transform`, `shape-model`, `outline`,
-`uv-membership`) remain as targeted cases.
+The example-based core tests (`geom`, `selection`, `bezier`, `transform`, `shape-model`,
+`uv-membership`, `timer-set`) remain as targeted cases.
+
+### One definition, read everywhere — agreement tests
+A recurring class of bug here has been two code paths restating the same rule and drifting. Three
+tests pin that the restatements are gone:
+
+- `test/bezier-topology.test.js`: the closed/open segment topology (how many segments, which
+  anchors each joins, the 3/2 anchor floor) is defined once in `core/bezier.js`
+  (`segCount`/`segControls`/`minAnchors`/`hasCurve`) and read by *both* samplers, *both*
+  nearest-point searches, `deleteAnchor`, and the adapter's SVG path writer. The test checks
+  `evalBezier`/`nearestOnBezier` dispatch to the explicit forms, that the adapter emits one `C` per
+  segment and a `Z` only when closed, and that every consumer honors the same floor.
+- `test/draw-pipeline.test.js` (`uvPxCorrespondences`): the edit overlay and the sulcus trace
+  pipeline fit their homographies from the same flattening of `projectVerticesInUvBounds`.
+- `test/shape-model.test.js` (`defaultName`): the name a prompt offers and the name an unnamed
+  import gets come from one rule.
+
+`test/dom-utils.test.js` pins `isTextEntry`, the single "is the user typing?" predicate behind
+every keyboard handler (controller Shift/Esc and the editor's Delete): a file input, button,
+checkbox or slider holding focus must not swallow a gesture.
 
 ### Sulcus SVG export — unit (pure writer) + a real XML parser
 `core/svg-export.js` is the pure writer for the sulci layer of an `overlays.svg`.

adapter/pycortex-adapter.js

@@ -16,9 +16,10 @@
  *   - dat.GUI control panel (gui.__folders) + LandscapeControls (setTarget/setRadius) + viewer.animate.
  */
 import { ViewerAdapter } from "./viewer-adapter.js";
-import { chaikin } from "../core/geom.js";
-import { isClosed, segCount } from "../core/bezier.js";
+import { chaikin, ndcToPixel } from "../core/geom.js";
+import { isClosed, segCount, segControls, hasCurve } from "../core/bezier.js";
 import { exportSulciSvg, SULCI_STROKE_WIDTH, SULCI_STROKE_OPACITY } from "../core/svg-export.js";
+import { TimerSet } from "../core/timer-set.js";
 
 const SVGNS = "http://www.w3.org/2000/svg";
 const HEMIS = ["left", "right"];
@@ -110,7 +111,7 @@ export function surfaceReady(viewer) {
 }
 
 export class PycortexAdapter extends ViewerAdapter {
-    constructor(viewer, { layerName = "drawnrois", animSpeedFallback = 0.6 } = {}) {
+    constructor(viewer, { animSpeedFallback = 0.6 } = {}) {
         super();
         this.THREE = globalThis.THREE;
         this.mriview = globalThis.mriview;
@@ -124,16 +125,15 @@ export class PycortexAdapter extends ViewerAdapter {
         this.surface = findSurface(viewer);
         this.posdata = (this.surface.picker && this.surface.picker.posdata) || this._buildPosdata();
 
-        this._layerName = layerName;
         this._animSpeedFallback = animSpeedFallback;
         this._v = new this.THREE.Vector3();
         this._thickmix = DEFAULT_THICKMIX;
         this._drawn = null;          // { layerEl, labels } for the current overlay layer
         this._layerHidden = false;
         this._labelsHidden = false;
         this._uiFolderAdded = false;
-        this._timers = new Set();    // pending setTimeout ids, cancelled by destroy()
-        this._onSetData = null;      // host listener installed by applyHostDefaults()
+        this._timers = new TimerSet();   // every poll/deferred teardown; destroy() cancels them
+        this._onSetData = null;          // host listener installed by applyHostDefaults()
     }
 
     // --- surface identity -------------------------------------------------------------
@@ -215,7 +215,7 @@ export class PycortexAdapter extends ViewerAdapter {
                 const v = this._worldOf(pd, mw, i, surfmix, foy).project(cam);
                 if (v.z < -1 || v.z > 1) continue; // behind camera / outside frustum
                 out[h].idx.push(revIdx[i]);
-                out[h].px.push(this._ndc(v, W, H));
+                out[h].px.push(ndcToPixel(v, W, H));
             }
         }
         return out;
@@ -272,14 +272,12 @@ export class PycortexAdapter extends ViewerAdapter {
                 const p = this._worldOf(pd, mw, i, surfmix, foy).project(cam);
                 if (p.z < -1 || p.z > 1) continue;
                 out[h].uv.push([u, v]);
-                out[h].px.push(this._ndc(p, W, H));
+                out[h].px.push(ndcToPixel(p, W, H));
             }
         }
         return out;
     }
 
-    _ndc(v, W, H) { return [(v.x * 0.5 + 0.5) * W, (-v.y * 0.5 + 0.5) * H]; }
-
     // --- view framing primitive -------------------------------------------------------
 
     // Center of mass (world) + the camera radius that fills `fillTarget` of the viewport.
@@ -301,7 +299,7 @@ export class PycortexAdapter extends ViewerAdapter {
                 sx += w.x; sy += w.y; sz += w.z; count++;
                 const nd = w.clone().project(cam);
                 if (nd.z < -1 || nd.z > 1) continue;
-                const px = this._ndc(nd, W, H);
+                const px = ndcToPixel(nd, W, H);
                 if (px[0] < minx) minx = px[0];
                 if (px[0] > maxx) maxx = px[0];
                 if (px[1] < miny) miny = px[1];
@@ -492,7 +490,7 @@ export class PycortexAdapter extends ViewerAdapter {
         svgo.svg.appendChild(layerEl);
 
         // occlusion-aware label sprites, reusing pycortex's own Labels; degrade gracefully
-        let labels = null;
+        let labels;
         try {
             labels = new this.svgoverlay.Labels(labelsEl, svgo.posdata, !!this._labelsHidden);
             labels.shader.uniforms.depth.value = svgo.depth;
@@ -549,18 +547,16 @@ export class PycortexAdapter extends ViewerAdapter {
     // sulcus) has n-1 segments and MUST NOT close — the missing `Z` is exactly what distinguishes
     // a sulcus from an ROI on disk.
     _bezierSvgPath(bez, W, H) {
-        const { anchors, inHandles, outHandles } = bez;
-        const n = anchors.length;
-        const closed = isClosed(bez);
-        if (n < (closed ? 3 : 2)) return null;
+        if (!hasCurve(bez)) return null;
         const P = (uv) => (uv[0] * W).toFixed(2) + "," + ((1 - uv[1]) * H).toFixed(2);
-        let d = "M" + P(anchors[0]);
-        const segs = segCount(bez);   // n when closed (the wrap), n-1 when open
-        for (let i = 0; i < segs; i++) {
-            const j = closed ? (i + 1) % n : i + 1;
-            d += "C" + P(outHandles[i]) + " " + P(inHandles[j]) + " " + P(anchors[j]);
+        let d = "M" + P(bez.anchors[0]);
+        // segControls/segCount own the wrap rule (n segments closed, n-1 open) — the same walk the
+        // samplers use, so what is baked here is exactly the curve the editor shows.
+        for (let i = 0, segs = segCount(bez); i < segs; i++) {
+            const [, c1, c2, p3] = segControls(bez, i);
+            d += "C" + P(c1) + " " + P(c2) + " " + P(p3);
         }
-        return closed ? d + "Z" : d;
+        return isClosed(bez) ? d + "Z" : d;
     }
 
     /*
@@ -647,7 +643,7 @@ export class PycortexAdapter extends ViewerAdapter {
             const svg = this.surface && this.surface.svg;
             if (!svg || !svg.layers || !(svg.rois || svg.sulci)) {
                 if (tries > OVERLAY_RETRY_MAX) return;
-                this._later(() => trySetOverlays(tries + 1), OVERLAY_RETRY_MS);
+                this._timers.later(() => trySetOverlays(tries + 1), OVERLAY_RETRY_MS);
                 return;
             }
             if (svg.rois) { svg.rois.showhide(false); if (svg.rois.labels) svg.rois.labels.showhide(false); }
@@ -656,25 +652,17 @@ export class PycortexAdapter extends ViewerAdapter {
         };
         trySetOverlays(0);
         // the datasets folder is built open after data loads (post-attach); re-collapse a few times
-        COLLAPSE_SCHEDULE_MS.forEach((ms) => this._later(() => this.collapseControlPanel(false), ms));
+        COLLAPSE_SCHEDULE_MS.forEach((ms) => this._timers.later(() => this.collapseControlPanel(false), ms));
         const t0 = Date.now();
         if (this.viewer.addEventListener) {
             this._onSetData = () => { if (Date.now() - t0 < COLLAPSE_WINDOW_MS) this.collapseControlPanel(false); };
             this.viewer.addEventListener("setData", this._onSetData);
         }
     }
 
-    /* setTimeout, remembered, so destroy() can cancel it. */
-    _later(fn, ms) {
-        const id = setTimeout(() => { this._timers.delete(id); fn(); }, ms);
-        this._timers.add(id);
-        return id;
-    }
-
     // Release everything applyHostDefaults() started. The overlay layer itself is left in place:
     // the controller clears it (setOverlayLayer(name, [])) before it tears the adapter down.
     destroy() {
-        for (const id of this._timers) clearTimeout(id);
         this._timers.clear();
         if (this._onSetData && this.viewer.removeEventListener)
             this.viewer.removeEventListener("setData", this._onSetData);

adapter/viewer-adapter.js

@@ -5,8 +5,9 @@
  * never to a specific viewer's internals. To run ROI drawing on a different WebGL surface
  * viewer, implement this interface (see pycortex-adapter.js for the reference implementation).
  *
- * It is a documented duck-typed interface, not a base class — implementers provide their own
- * object with these methods. The methods below `throw` so an incomplete adapter fails loudly.
+ * It is a documented interface. The class is a convenience base (both PycortexAdapter and the test
+ * fake `extends` it, and `REQUIRED` below drives a conformance test), but any object with these
+ * methods works — the required methods `throw` here so an incomplete adapter fails loudly.
  *
  * Coordinate conventions:
  *   - "screen px": CSS pixels relative to the surface canvas's top-left (what the lasso uses).
@@ -133,3 +134,24 @@ ViewerAdapter.REQUIRED = [
     "setOverlayLayer", "setLayerVisible", "flatten",
     "setCameraTarget", "setCameraRadius", "cameraRadius", "requestRender", "onMixChange",
 ];
+
+/* Everything: the bounds that select the whole flatmap for projectVerticesInUvBounds. */
+export const ALL_UV_BOUNDS = Object.freeze({ minu: -Infinity, maxu: Infinity, minv: -Infinity, maxv: Infinity });
+
+/*
+ * uv->px correspondences for a homography fit, from the adapter's projectVerticesInUvBounds
+ * flattened across both hemispheres into parallel `src` (uv) / `dst` (px) arrays. The edit overlay
+ * (LOCAL bounds around the shape) and the sulcus trace pipeline (the whole flatmap) both fit the
+ * same kind of transform from the same kind of data — this is the one place that data is shaped.
+ * `bounds` defaults to the whole flatmap.
+ */
+export function uvPxCorrespondences(adapter, bounds = ALL_UV_BOUNDS) {
+    const proj = adapter.projectVerticesInUvBounds(bounds);
+    const src = [], dst = [];
+    for (const h of ["left", "right"]) {
+        const p = proj[h];
+        if (!p) continue;
+        for (let i = 0; i < p.uv.length; i++) { src.push(p.uv[i]); dst.push(p.px[i]); }
+    }
+    return { src, dst };
+}