Merge pull request #36 from facebookresearch/explorer-ux-and-ci-hardening

Explorer UX + CI Improvements

Author: SebastianAment

.github/workflows/js-sync.yml

@@ -100,3 +100,6 @@ jobs:
 
       - name: Categorical source kernel is not interpolable
         run: node test/test_js_categorical_source.mjs
+
+      - name: Scatter filter predicate + derived quantities
+        run: node test/test_js_filters.mjs

Makefile

@@ -67,9 +67,16 @@ TRACKED_PY := $(shell (sl files 2>/dev/null || git ls-files) | grep -E '\.py$$')
 # untracked working-tree files don't poison the result. Black version
 # is pinned via pyproject.toml's [project.optional-dependencies].dev
 # so the formatter output is bit-for-bit identical to CI.
+# Hard-gate error codes. MUST stay identical to the flake8 --select list in
+# .github/workflows/tests.yml, otherwise `make lint` green does not imply the
+# CI lint job is green. This previously gated only E9,F63,F7,F82 (4 codes)
+# while CI gated 13 -- so E501 (long lines) and F401 (unused imports), the two
+# most common real failures, passed locally and broke CI.
+FLAKE8_SELECT = E9,E202,E226,E251,E402,E501,E741,F401,F63,F7,F811,F82,F841
+
 lint:
 	$(PYTHON) -m black --check --diff $(TRACKED_PY)
-	$(PYTHON) -m flake8 $(TRACKED_PY) --count --select=E9,F63,F7,F82 --show-source --statistics
+	$(PYTHON) -m flake8 $(TRACKED_PY) --count --select=$(FLAKE8_SELECT) --show-source --statistics
 	$(PYTHON) -m flake8 $(TRACKED_PY) --count --exit-zero --statistics
 
 format:
@@ -99,7 +106,8 @@ JS_TESTS = \
   test/test_curve_monotonicity.mjs \
   test/test_data_freshness.mjs \
   test/test_js_preview_state.mjs \
-  test/test_js_categorical_source.mjs
+  test/test_js_categorical_source.mjs \
+  test/test_js_filters.mjs
 
 test-js:
 	@for t in $(JS_TESTS); do \

docs/filters.mjs

@@ -0,0 +1,108 @@
+/**
+ * Scatter-filter logic, extracted from ui.mjs so it can be unit tested.
+ *
+ * These functions used to live inside `setupEventListeners` — a 400-line
+ * closure — purely so they could capture `colNames`. That made the only part
+ * of the filter subsystem with real correctness content (the predicate, and
+ * the derived-quantity formulas) reachable only through Playwright. Both are
+ * pure given their inputs, so they belong here with a fast node test.
+ *
+ * See test/test_js_filters.mjs.
+ */
+
+/**
+ * Resolve a column name to its index, throwing loudly on a miss.
+ *
+ * A silent fallback here would be dangerous: `indexOf` returning -1 and being
+ * used as an index yields `undefined`, which makes every numeric comparison
+ * false and quietly disables the filter instead of failing.
+ */
+export function colIdxStrict(colNames, name) {
+  const i = colNames.indexOf(name);
+  if (i < 0) {
+    throw new Error(
+      `filters: column ${JSON.stringify(name)} not found in ${JSON.stringify(colNames)}. ` +
+      "Column names must match DEFAULT_X_COLUMNS in boxcrete/utils.py.",
+    );
+  }
+  return i;
+}
+
+/**
+ * Derived quantities offered in the filter dropdown alongside raw columns.
+ * Each `compute` takes a raw composition row and returns a scalar.
+ *
+ * NOTE: `paste` divides by a 6-term total that omits HRWR, whereas Python's
+ * `_TOTAL_MASS_NAMES` (boxcrete/utils.py) includes it. HRWR is at most ~13 of
+ * ~2400 kg/m3, so the two differ by well under 1%, but they are not the same
+ * definition. Preserved as-is here to keep this extraction behaviour-neutral.
+ *
+ * @param {string[]} colNames - composition column names, in catalog order.
+ */
+export function makeComputedFilters(colNames) {
+  const iCement = colIdxStrict(colNames, "Cement (kg/m3)");
+  const iFlyAsh = colIdxStrict(colNames, "Fly Ash (kg/m3)");
+  const iSlag = colIdxStrict(colNames, "Slag (kg/m3)");
+  const iWater = colIdxStrict(colNames, "Water (kg/m3)");
+  const iCoarse = colIdxStrict(colNames, "Coarse Aggregates (kg/m3)");
+  const iFine = colIdxStrict(colNames, "Fine Aggregate (kg/m3)");
+
+  const binderOf = (c) => c[iCement] + c[iFlyAsh] + c[iSlag];
+
+  return [
+    {
+      id: "wb",
+      label: "W/B Ratio",
+      compute: (c) => {
+        const b = binderOf(c);
+        return b > 0 ? c[iWater] / b : Infinity;
+      },
+    },
+    { id: "binder", label: "Total Binder", compute: binderOf },
+    {
+      id: "scm",
+      label: "SCM Replacement %",
+      compute: (c) => {
+        const b = binderOf(c);
+        return b > 0 ? ((c[iFlyAsh] + c[iSlag]) / b) * 100 : 0;
+      },
+    },
+    {
+      id: "paste",
+      label: "Paste Fraction",
+      compute: (c) => {
+        const paste = binderOf(c) + c[iWater];
+        const total = paste + c[iCoarse] + c[iFine];
+        return total > 0 ? paste / total : 0;
+      },
+    },
+  ];
+}
+
+/**
+ * Does `comp` satisfy every active filter?
+ *
+ * A filter is either
+ *   { colIdx, classes: Set<number> }        categorical: class membership
+ *   { colIdx | computed, min, max }         numeric: inclusive bounds
+ *
+ * Categorical columns (Material Source) are unordered, so a min/max range is
+ * meaningless for them — "between Source A and Source B" says nothing. They
+ * test set membership on the rounded class instead.
+ *
+ * @param {number[]} comp - one composition row.
+ * @param {Array|null} filters - active filter specs; null/empty means "match all".
+ * @returns {boolean} true when the point should stay visible.
+ */
+export function matchesFilters(comp, filters) {
+  if (!filters || filters.length === 0) return true;
+  for (const f of filters) {
+    const val = f.computed ? f.computed(comp) : comp[f.colIdx];
+    if (f.classes) {
+      if (!f.classes.has(Math.round(val))) return false;
+    } else if (val < f.min || val > f.max) {
+      return false;
+    }
+  }
+  return true;
+}

docs/model_init_worker.mjs

@@ -0,0 +1,73 @@
+/**
+ * Off-main-thread strength-model initialization.
+ *
+ * `initStrengthModel` rebuilds the 670x670 training kernel and Cholesky-factors
+ * it -- about 100 MFLOP, measured at ~70 ms on desktop arm64 and therefore
+ * roughly 230-460 ms on a mid-range phone. Run inline it blocks first paint and
+ * every tap for that whole window.
+ *
+ * The factorization itself is not meaningfully optimizable in JS: array-of-
+ * arrays measured 38-40 ms and a flat Float64Array rewrite came out *slower*
+ * (46-50 ms), because V8 hoists the row pointer for `L[i][k]` while flat
+ * indexing needs two adds per inner-loop access. So the win has to come from
+ * moving the work, not shrinking it.
+ *
+ * This worker runs the identical `initStrengthModel` and posts the fully
+ * derived params back. The big buffers are handed over as transferables, so
+ * the ~3.6 MB factor costs nothing to return.
+ */
+
+import { initStrengthModel } from "./gp.mjs";
+
+// Refuse to run outside a dedicated worker.
+//
+// This is what makes the handler below safe without an origin check, and it is
+// enforced rather than assumed. A dedicated worker is addressable only by the
+// document that constructed it -- no cross-origin window can postMessage into
+// it -- and its message events carry no origin to verify. Measured in
+// Chromium: inside a dedicated worker `event.origin` is "" and `event.source`
+// is null, where a window handler sees the real origin.
+//
+// The hazard the check removes is real. This file is a static asset, so if it
+// were ever imported into the page instead of constructed as a Worker, `self`
+// would be the Window and `self.onmessage` would install an unguarded
+// window-level message handler that any cross-origin opener could drive --
+// precisely the CodeQL js/missing-origin-check pattern. Failing loudly means
+// that misuse can never silently become a vulnerability.
+if (
+  typeof DedicatedWorkerGlobalScope === "undefined" ||
+  !(self instanceof DedicatedWorkerGlobalScope)
+) {
+  throw new Error(
+    "model_init_worker.mjs must be constructed as a dedicated Worker, not " +
+    "imported into a document. Loading it in a window scope would install " +
+    "an unguarded message handler.",
+  );
+}
+
+self.onmessage = (e) => {
+  // Provenance gate. In a dedicated worker the only possible sender is the
+  // parent document, and such events are dispatched with an empty origin.
+  // Anything else means this is not the context asserted above, so drop the
+  // message rather than act on untrusted input.
+  if (e.origin !== "") return;
+
+  const params = e.data;
+  try {
+    initStrengthModel(params);
+  } catch (err) {
+    // Surface the real reason; ui.mjs falls back to synchronous init.
+    self.postMessage({ __error: String((err && err.message) || err) });
+    return;
+  }
+
+  // Hand over the large typed arrays instead of copying them. Anything not
+  // listed here is structure-cloned, which is fine for the small fields.
+  const transfer = [];
+  for (const key of ["L_flat", "X_train_flat", "alpha_f64", "_hTrain"]) {
+    const buf = params[key] && params[key].buffer;
+    // Guard against two views sharing one buffer -- transferring twice throws.
+    if (buf && !transfer.includes(buf)) transfer.push(buf);
+  }
+  self.postMessage(params, transfer);
+};

docs/style.css

@@ -606,16 +606,31 @@ h2 {
   border-color: var(--accent);
   background: rgba(var(--card-bg-rgb), 0.6);
 }
+/* The input is the TOUCH TARGET; the visible 6 px bar is drawn by the track
+   pseudo-elements below. Previously the element itself was the 6 px bar, which
+   meant the whole hit area was 6 px tall -- measured 220x6 on mobile, against a
+   WCAG 2.2 AA minimum of 24x24 and an Apple HIG recommendation of 44x44. All 8
+   sliders failed. Separating hit area from visual lets the target grow without
+   changing anything the user sees. */
 .slider-group input[type=range] {
   width: 100%;
   cursor: pointer;
   accent-color: var(--accent);
   -webkit-appearance: none;
   appearance: none;
+  height: 24px;
+  background: transparent;
+  outline: none;
+}
+.slider-group input[type=range]::-webkit-slider-runnable-track {
+  height: 6px;
+  background: var(--border);
+  border-radius: 3px;
+}
+.slider-group input[type=range]::-moz-range-track {
   height: 6px;
   background: var(--border);
   border-radius: 3px;
-  outline: none;
 }
 .slider-group input[type=range]::-webkit-slider-thumb {
   -webkit-appearance: none;
@@ -626,6 +641,9 @@ h2 {
   border: 2px solid rgba(255,255,255,0.9);
   cursor: pointer;
   box-shadow: 0 1px 3px rgba(0,0,0,0.2);
+  /* WebKit aligns the thumb to the top of the runnable track, so pull it up
+     by half the difference to re-center it on the 6 px bar. */
+  margin-top: -6px;
 }
 .slider-group input[type=range]::-moz-range-thumb {
   width: 18px;
@@ -1507,7 +1525,12 @@ a.ref-link {
        small breathing margin from the label above and info-row below. */
     width: min(60vw, 220px);
     display: block;
-    margin: 4px auto;
+    /* 44 px hit area (Apple HIG); the visible bar stays 6 px via the track
+       pseudo-element. Margin drops to 0 so the row grows by less than the
+       full height increase -- the taller target absorbs the gap that used to
+       sit between the label row and the info-row. */
+    height: 44px;
+    margin: 0 auto;
   }
   /* Smaller thumb on mobile reduces the intrinsic thumb-half-width inset
      (was 9 px for the 18 px desktop thumb). 16 px is still a comfortable
@@ -1520,6 +1543,8 @@ a.ref-link {
   .mobile-sliders-view .slider-group input[type=range]::-webkit-slider-thumb {
     width: 16px;
     height: 16px;
+    /* Re-center the smaller mobile thumb on the 6 px track. */
+    margin-top: -5px;
   }
   .mobile-sliders-view .slider-group input[type=range]::-moz-range-thumb {
     width: 16px;