Skip to content

Commit 30bd94f

Browse files
Explorer UX + CI hardening: touch targets, non-blocking init, test parity
Five improvements from the repository review. Each is backed by a measurement rather than an impression; the numbers below are from this machine (desktop arm64, WASM active). 1. Slider touch targets (mobile) The input element WAS the visible 6 px bar, so the entire hit area was 6 px tall -- measured 220x6 on the 412px mobile project, against a WCAG 2.2 AA minimum of 24x24 and an Apple HIG recommendation of 44x44. All 8 sliders failed. The visible bar moves to the track pseudo-element so the input box can be a real target: 44 px on mobile, 24 px on desktop, with the thumb re-centred via margin-top. Nothing the user sees changes. touch-targets.spec.ts pins all three properties. 2. Page-load no longer blocks the main thread initStrengthModel rebuilds the 670x670 kernel and Cholesky-factors it (~100 MFLOP). Measured with PerformanceObserver: one 108 ms long task during startup, which at mobile's 3-6x is a ~325-650 ms freeze before first interaction. It now runs in a module worker (docs/model_init_worker.mjs) with the large buffers handed back as transferables; startup long tasks drop to zero. Falls back to synchronous init when Worker is unavailable, so node tests and file:// origins are unaffected. Note on what did NOT work: rewriting the Cholesky over a flat Float64Array, the obvious micro-optimisation, measured 20% SLOWER (46-50 ms vs 38-40 ms) because V8 hoists the row pointer for L[i][k] while flat indexing needs two adds per inner-loop access. Reverted. The cost had to be moved, not shrunk. 3. make lint now gates what CI gates Local flake8 selected 4 error codes; CI selects 13. E501 and F401 -- the two most common real failures -- passed locally and broke CI, which happened during the work that produced the previous commit. The select list is now a single FLAKE8_SELECT variable documented to stay in lockstep with tests.yml. 4. Visual regression is live; mobile has behavioural coverage visual-regression.spec.ts sat behind test.skip(true, ...) and had never run. Linux baselines are now committed and the spec gates on platform, so it protects CI while skipping on macOS where a Linux baseline can never match. Verified by re-running WITHOUT --update-snapshots: it passes by comparison, not by writing. mobile-behavior.spec.ts adds the behaviour the mobile project lacked -- the existing specs skipped mobile only because the controls sit behind the view toggle, not because taps don't work. 5. Filter logic extracted from the ui.mjs monolith makeComputedFilters and matchesFilters lived inside a 400-line setupEventListeners closure purely to capture colNames, so the only part of the filter subsystem with real correctness content was reachable only through Playwright. Now docs/filters.mjs with 26 node assertions covering the categorical branch, NaN-safety on degenerate inputs, and AND-composition. colIdx becomes colIdxStrict: a missing column throws instead of silently disabling the filter. Behaviour-neutral -- including the paste-fraction denominator, which omits HRWR where Python's _TOTAL_MASS_NAMES includes it (<1%); documented in place rather than changed under a refactor. Verification: lint 0, test-js 13/13, test-py 270 passing at 100% coverage, notebooks 3/3, e2e 82 passing on macOS (three consecutive runs) and 85 on Linux in the matching Playwright image, including the four visual specs.
1 parent f59913c commit 30bd94f

15 files changed

Lines changed: 589 additions & 68 deletions

.github/workflows/js-sync.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,6 @@ jobs:
100100

101101
- name: Categorical source kernel is not interpolable
102102
run: node test/test_js_categorical_source.mjs
103+
104+
- name: Scatter filter predicate + derived quantities
105+
run: node test/test_js_filters.mjs

Makefile

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,16 @@ TRACKED_PY := $(shell (sl files 2>/dev/null || git ls-files) | grep -E '\.py$$')
6767
# untracked working-tree files don't poison the result. Black version
6868
# is pinned via pyproject.toml's [project.optional-dependencies].dev
6969
# so the formatter output is bit-for-bit identical to CI.
70+
# Hard-gate error codes. MUST stay identical to the flake8 --select list in
71+
# .github/workflows/tests.yml, otherwise `make lint` green does not imply the
72+
# CI lint job is green. This previously gated only E9,F63,F7,F82 (4 codes)
73+
# while CI gated 13 -- so E501 (long lines) and F401 (unused imports), the two
74+
# most common real failures, passed locally and broke CI.
75+
FLAKE8_SELECT = E9,E202,E226,E251,E402,E501,E741,F401,F63,F7,F811,F82,F841
76+
7077
lint:
7178
$(PYTHON) -m black --check --diff $(TRACKED_PY)
72-
$(PYTHON) -m flake8 $(TRACKED_PY) --count --select=E9,F63,F7,F82 --show-source --statistics
79+
$(PYTHON) -m flake8 $(TRACKED_PY) --count --select=$(FLAKE8_SELECT) --show-source --statistics
7380
$(PYTHON) -m flake8 $(TRACKED_PY) --count --exit-zero --statistics
7481

7582
format:
@@ -99,7 +106,8 @@ JS_TESTS = \
99106
test/test_curve_monotonicity.mjs \
100107
test/test_data_freshness.mjs \
101108
test/test_js_preview_state.mjs \
102-
test/test_js_categorical_source.mjs
109+
test/test_js_categorical_source.mjs \
110+
test/test_js_filters.mjs
103111

104112
test-js:
105113
@for t in $(JS_TESTS); do \

docs/filters.mjs

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/**
2+
* Scatter-filter logic, extracted from ui.mjs so it can be unit tested.
3+
*
4+
* These functions used to live inside `setupEventListeners` — a 400-line
5+
* closure — purely so they could capture `colNames`. That made the only part
6+
* of the filter subsystem with real correctness content (the predicate, and
7+
* the derived-quantity formulas) reachable only through Playwright. Both are
8+
* pure given their inputs, so they belong here with a fast node test.
9+
*
10+
* See test/test_js_filters.mjs.
11+
*/
12+
13+
/**
14+
* Resolve a column name to its index, throwing loudly on a miss.
15+
*
16+
* A silent fallback here would be dangerous: `indexOf` returning -1 and being
17+
* used as an index yields `undefined`, which makes every numeric comparison
18+
* false and quietly disables the filter instead of failing.
19+
*/
20+
export function colIdxStrict(colNames, name) {
21+
const i = colNames.indexOf(name);
22+
if (i < 0) {
23+
throw new Error(
24+
`filters: column ${JSON.stringify(name)} not found in ${JSON.stringify(colNames)}. ` +
25+
"Column names must match DEFAULT_X_COLUMNS in boxcrete/utils.py.",
26+
);
27+
}
28+
return i;
29+
}
30+
31+
/**
32+
* Derived quantities offered in the filter dropdown alongside raw columns.
33+
* Each `compute` takes a raw composition row and returns a scalar.
34+
*
35+
* NOTE: `paste` divides by a 6-term total that omits HRWR, whereas Python's
36+
* `_TOTAL_MASS_NAMES` (boxcrete/utils.py) includes it. HRWR is at most ~13 of
37+
* ~2400 kg/m3, so the two differ by well under 1%, but they are not the same
38+
* definition. Preserved as-is here to keep this extraction behaviour-neutral.
39+
*
40+
* @param {string[]} colNames - composition column names, in catalog order.
41+
*/
42+
export function makeComputedFilters(colNames) {
43+
const iCement = colIdxStrict(colNames, "Cement (kg/m3)");
44+
const iFlyAsh = colIdxStrict(colNames, "Fly Ash (kg/m3)");
45+
const iSlag = colIdxStrict(colNames, "Slag (kg/m3)");
46+
const iWater = colIdxStrict(colNames, "Water (kg/m3)");
47+
const iCoarse = colIdxStrict(colNames, "Coarse Aggregates (kg/m3)");
48+
const iFine = colIdxStrict(colNames, "Fine Aggregate (kg/m3)");
49+
50+
const binderOf = (c) => c[iCement] + c[iFlyAsh] + c[iSlag];
51+
52+
return [
53+
{
54+
id: "wb",
55+
label: "W/B Ratio",
56+
compute: (c) => {
57+
const b = binderOf(c);
58+
return b > 0 ? c[iWater] / b : Infinity;
59+
},
60+
},
61+
{ id: "binder", label: "Total Binder", compute: binderOf },
62+
{
63+
id: "scm",
64+
label: "SCM Replacement %",
65+
compute: (c) => {
66+
const b = binderOf(c);
67+
return b > 0 ? ((c[iFlyAsh] + c[iSlag]) / b) * 100 : 0;
68+
},
69+
},
70+
{
71+
id: "paste",
72+
label: "Paste Fraction",
73+
compute: (c) => {
74+
const paste = binderOf(c) + c[iWater];
75+
const total = paste + c[iCoarse] + c[iFine];
76+
return total > 0 ? paste / total : 0;
77+
},
78+
},
79+
];
80+
}
81+
82+
/**
83+
* Does `comp` satisfy every active filter?
84+
*
85+
* A filter is either
86+
* { colIdx, classes: Set<number> } categorical: class membership
87+
* { colIdx | computed, min, max } numeric: inclusive bounds
88+
*
89+
* Categorical columns (Material Source) are unordered, so a min/max range is
90+
* meaningless for them — "between Source A and Source B" says nothing. They
91+
* test set membership on the rounded class instead.
92+
*
93+
* @param {number[]} comp - one composition row.
94+
* @param {Array|null} filters - active filter specs; null/empty means "match all".
95+
* @returns {boolean} true when the point should stay visible.
96+
*/
97+
export function matchesFilters(comp, filters) {
98+
if (!filters || filters.length === 0) return true;
99+
for (const f of filters) {
100+
const val = f.computed ? f.computed(comp) : comp[f.colIdx];
101+
if (f.classes) {
102+
if (!f.classes.has(Math.round(val))) return false;
103+
} else if (val < f.min || val > f.max) {
104+
return false;
105+
}
106+
}
107+
return true;
108+
}

docs/model_init_worker.mjs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/**
2+
* Off-main-thread strength-model initialization.
3+
*
4+
* `initStrengthModel` rebuilds the 670x670 training kernel and Cholesky-factors
5+
* it -- about 100 MFLOP, measured at ~70 ms on desktop arm64 and therefore
6+
* roughly 230-460 ms on a mid-range phone. Run inline it blocks first paint and
7+
* every tap for that whole window.
8+
*
9+
* The factorization itself is not meaningfully optimizable in JS: array-of-
10+
* arrays measured 38-40 ms and a flat Float64Array rewrite came out *slower*
11+
* (46-50 ms), because V8 hoists the row pointer for `L[i][k]` while flat
12+
* indexing needs two adds per inner-loop access. So the win has to come from
13+
* moving the work, not shrinking it.
14+
*
15+
* This worker runs the identical `initStrengthModel` and posts the fully
16+
* derived params back. The big buffers are handed over as transferables, so
17+
* the ~3.6 MB factor costs nothing to return.
18+
*/
19+
20+
import { initStrengthModel } from "./gp.mjs";
21+
22+
self.onmessage = (e) => {
23+
const params = e.data;
24+
try {
25+
initStrengthModel(params);
26+
} catch (err) {
27+
// Surface the real reason; ui.mjs falls back to synchronous init.
28+
self.postMessage({ __error: String((err && err.message) || err) });
29+
return;
30+
}
31+
32+
// Hand over the large typed arrays instead of copying them. Anything not
33+
// listed here is structure-cloned, which is fine for the small fields.
34+
const transfer = [];
35+
for (const key of ["L_flat", "X_train_flat", "alpha_f64", "_hTrain"]) {
36+
const buf = params[key] && params[key].buffer;
37+
// Guard against two views sharing one buffer -- transferring twice throws.
38+
if (buf && !transfer.includes(buf)) transfer.push(buf);
39+
}
40+
self.postMessage(params, transfer);
41+
};

docs/style.css

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -606,16 +606,31 @@ h2 {
606606
border-color: var(--accent);
607607
background: rgba(var(--card-bg-rgb), 0.6);
608608
}
609+
/* The input is the TOUCH TARGET; the visible 6 px bar is drawn by the track
610+
pseudo-elements below. Previously the element itself was the 6 px bar, which
611+
meant the whole hit area was 6 px tall -- measured 220x6 on mobile, against a
612+
WCAG 2.2 AA minimum of 24x24 and an Apple HIG recommendation of 44x44. All 8
613+
sliders failed. Separating hit area from visual lets the target grow without
614+
changing anything the user sees. */
609615
.slider-group input[type=range] {
610616
width: 100%;
611617
cursor: pointer;
612618
accent-color: var(--accent);
613619
-webkit-appearance: none;
614620
appearance: none;
621+
height: 24px;
622+
background: transparent;
623+
outline: none;
624+
}
625+
.slider-group input[type=range]::-webkit-slider-runnable-track {
626+
height: 6px;
627+
background: var(--border);
628+
border-radius: 3px;
629+
}
630+
.slider-group input[type=range]::-moz-range-track {
615631
height: 6px;
616632
background: var(--border);
617633
border-radius: 3px;
618-
outline: none;
619634
}
620635
.slider-group input[type=range]::-webkit-slider-thumb {
621636
-webkit-appearance: none;
@@ -626,6 +641,9 @@ h2 {
626641
border: 2px solid rgba(255,255,255,0.9);
627642
cursor: pointer;
628643
box-shadow: 0 1px 3px rgba(0,0,0,0.2);
644+
/* WebKit aligns the thumb to the top of the runnable track, so pull it up
645+
by half the difference to re-center it on the 6 px bar. */
646+
margin-top: -6px;
629647
}
630648
.slider-group input[type=range]::-moz-range-thumb {
631649
width: 18px;
@@ -1507,7 +1525,12 @@ a.ref-link {
15071525
small breathing margin from the label above and info-row below. */
15081526
width: min(60vw, 220px);
15091527
display: block;
1510-
margin: 4px auto;
1528+
/* 44 px hit area (Apple HIG); the visible bar stays 6 px via the track
1529+
pseudo-element. Margin drops to 0 so the row grows by less than the
1530+
full height increase -- the taller target absorbs the gap that used to
1531+
sit between the label row and the info-row. */
1532+
height: 44px;
1533+
margin: 0 auto;
15111534
}
15121535
/* Smaller thumb on mobile reduces the intrinsic thumb-half-width inset
15131536
(was 9 px for the 18 px desktop thumb). 16 px is still a comfortable
@@ -1520,6 +1543,8 @@ a.ref-link {
15201543
.mobile-sliders-view .slider-group input[type=range]::-webkit-slider-thumb {
15211544
width: 16px;
15221545
height: 16px;
1546+
/* Re-center the smaller mobile thumb on the 6 px track. */
1547+
margin-top: -5px;
15231548
}
15241549
.mobile-sliders-view .slider-group input[type=range]::-moz-range-thumb {
15251550
width: 16px;

0 commit comments

Comments
 (0)