suggest: review fixes -- a ranking in flight is dropped (worker terminated) by whatever clears the list (new part, undo, reset), a message error falls back inline, and the test ranks a torus whose answer the gap really changes

This commit is contained in:
matthewtrahan authored and Matthew Trahan committed 2026-10-14 21:32:24 -05:00
1 parent 0f9cd68cf5
commit 98c0d5a94d
3 files changed
+36 -18

No files matched your search

+3 -3
View File
@@ -104,9 +104,9 @@ first version added a squat wall 1.6 mm from the one main builds (review).
which crossed a cube's edge).
**`suggest_worker.test.js`** -- Suggest orientation's Worker (`web/suggestworker.js`) ranks
**exactly** as `suggestOrientations` does on this thread, with the default settings and
with a changed Support gap sent as tunables, and the gap doesn't leak back into this
thread's modules.
**exactly** as `suggestOrientations` does on this thread, on a torus with the default
settings and with a changed Support gap sent as tunables -- and the two rankings
differ, so the gap really reached the Worker.
**`orient.test.js`** -- the orientation/strength logic behind the left rail (pure,
no DOM), so a change to a verdict or a solver can't silently drift:
+12 -6
View File
@@ -1,8 +1,8 @@
// Suggest orientation runs in a Worker (web/suggestworker.js), with the page's
// tunables applied first. Its ranking has to be the one the page used to compute
// itself -- with the default settings and with a changed Support gap, which moves
// the finalists' fin counts -- and the Worker's module copy must not leak the
// gap into this thread's.
// itself, with the default settings and with a changed Support gap -- on a torus,
// whose finalists all take walls, so the gap really changes the answer (checked
// below, or the test would prove nothing about the tunables reaching the Worker).
import { WEB, assert, loadModel } from './_util.js';
const { suggestOrientations } = await import(`${WEB}orient.js`);
@@ -18,12 +18,13 @@ function inWorker(topo, threshold, tunables) {
});
}
const ranked = {};
for (const gap of [undefined, 0.35]) {
Deno.test(`suggest worker ranks exactly as the page did (lbracket, gap ${gap ?? 'default'})`, async () => {
const topo = loadModel('lbracket');
Deno.test(`suggest worker ranks exactly as the page did (torus, gap ${gap ?? 'default'})`, async () => {
const topo = loadModel('torus');
const before = PROP.gap;
const got = await inWorker(topo, 45, gap === undefined ? undefined : { propGap: gap });
assert(PROP.gap === before, 'the Worker\'s gap leaked into this thread');
ranked[gap ?? 'default'] = JSON.stringify(got);
try {
if (gap !== undefined) applyTunables({ propGap: gap });
const want = suggestOrientations(topo, { top: 3, threshold: 45 });
@@ -34,3 +35,8 @@ for (const gap of [undefined, 0.35]) {
}
});
}
Deno.test('the changed gap changes the torus ranking (so the tests above test the tunables)', () => {
assert(ranked.default && ranked['0.35'], 'run with the two tests above');
assert(ranked.default !== ranked['0.35'], 'gap 0.35 ranked the torus exactly like 0.2');
});
+21 -9
View File
@@ -112,6 +112,9 @@ function renderSuggestions() {
* invalidates the ranking). The disclosure chevron does NOT come through here --
* it only collapses/expands what's already there. */
export function hideSuggestions() {
// A ranking still running belongs to what is being cleared (a new part, an undo,
// a reset): drop it, so its list doesn't pop up after the action that cleared it.
cancelRanking?.();
el('suggest-list').hidden = true;
el('suggest-list').replaceChildren();
const note = el('suggest-note');
@@ -137,29 +140,36 @@ export function clearSuggestionMark() {
* can run (file://, a clone or worker failure) it ranks here, as it always did.
*/
let rankId = 0;
let cancelRanking = null; // drops the ranking in flight; it then resolves to null
function rankPoses(topo, thr) {
let cancelled = false;
const inline = () => new Promise((resolve, reject) => {
// let the button repaint before the solve blocks the thread
requestAnimationFrame(() => requestAnimationFrame(() => {
if (cancelled) { resolve(null); return; }
try { resolve(suggestOrientations(topo, { top: 3, threshold: thr })); } catch (err) { reject(err); }
}));
});
let w;
let w = null;
try {
w = new Worker(new URL('../suggestworker.js', import.meta.url), { type: 'module' });
} catch {
return inline();
}
} catch { /* no Worker here: rank inline */ }
const id = ++rankId;
return new Promise((resolve) => {
const fallBack = () => { w.terminate(); resolve(inline()); };
const cancel = () => { cancelled = true; finish(null); };
const done = () => { if (cancelRanking === cancel) cancelRanking = null; };
const finish = (v) => { done(); w?.terminate(); resolve(v); };
// inline stays cancellable until it runs: it checks `cancelled` first
const fallBack = () => { w?.terminate(); w = null; resolve(inline().finally(done)); };
cancelRanking = cancel;
if (!w) { fallBack(); return; }
w.onmessage = (e) => {
if (e.data.id !== id) return;
if (e.data.error) { console.warn('suggest worker failed; ranking inline', e.data.error); fallBack(); return; }
w.terminate();
resolve(e.data.ranked);
finish(e.data.ranked);
};
w.onerror = () => fallBack();
w.onmessageerror = () => fallBack();
// the inside grid holds a closure structured clone can't copy (finbuild.js)
const topoMsg = { ...topo };
delete topoMsg._insideGrid;
@@ -180,8 +190,10 @@ el('suggest-orient').addEventListener('click', () => {
// Walls the current pose stands inside a bore -- the baseline the "points the
// bores up" verdict measures its win against.
const curBore = (lastBuilt?.props ?? []).filter((q) => q.inBore).length;
rankPoses(topo, threshold).then(({ candidates, confidence }) => {
if (topo !== topology) return; // a different part was loaded while it ranked
rankPoses(topo, threshold).then((ranked) => {
// cancelled (hideSuggestions), or a different part was loaded while it ranked
if (!ranked || topo !== topology) return;
const { candidates, confidence } = ranked;
suggestions = candidates;
suggestCurBore = curBore;
// Fresh results always land expanded, with the collapse chevron available.