diff --git a/gently/ui/web/static/js/operate.js b/gently/ui/web/static/js/operate.js index 9c8775ea..362b7c69 100644 --- a/gently/ui/web/static/js/operate.js +++ b/gently/ui/web/static/js/operate.js @@ -1097,6 +1097,23 @@ const OperateManager = (function () { catch (e) { toastFail(`Roles failed (${why(e)})`); } } + // What `Start` will actually do, per mode. The button used to say "Start" + // for all four, while `startRun` branched into four different verbs — one + // of which (single) acquires a volume and finishes, and is not starting an + // experiment at all. The mode selector sits in a block above, so an + // operator who chose a mode and then looked away had nothing to read. + const RUN_VERB = { + single: 'Acquire one volume', + adaptive: 'Start timelapse', + library: 'Run tactic', + agent: 'Brief the agent', + }; + + function renderRunButton() { + const b = $('op-run-start'); + if (b && !b.disabled) b.textContent = RUN_VERB[_mode] || 'Start'; + } + function setMode(m) { _mode = m; document.querySelectorAll('#op-modes [data-mode]').forEach(b => @@ -1106,6 +1123,7 @@ const OperateManager = (function () { if (p) p.hidden = k !== m; }); if (m === 'library') loadLibrary(); + renderRunButton(); renderSingle(); } @@ -1136,15 +1154,40 @@ const OperateManager = (function () { } catch (_) { host.innerHTML = '
Library unavailable
'; } } + /** + * The embryos a run should image: everything not marked as a reference. + * + * There used to be a fallback — `subs.length ? subs : all` — meant to be + * kind to a roster with no roles assigned. It could never do that. An + * embryo with no role, or role 'test', or 'unassigned', already passes the + * filter, so `subs` is empty in exactly ONE case: every embryo is marked + * `calibration`. The fallback therefore fired only when the operator had + * said "these are all references", and answered by imaging all of them as + * subjects — the precise opposite of the instruction. + * + * Empty now, and the caller says so. + */ function subjectIds() { - const subs = _embryos.filter(e => e.role !== 'calibration').map(e => e.id); - return subs.length ? subs : _embryos.map(e => e.id); + return _embryos.filter(e => e.role !== 'calibration').map(e => e.id); + } + + // Every embryo marked as a reference means there is nothing to image. Say + // so once, here, rather than at each mode — and say which state it is in, + // because "no embryos" and "no subjects among your embryos" need + // different fixes. + function haveSubjects() { + if (!_embryos.length) { toastFail('No embryos registered yet'); return false; } + if (!subjectIds().length) { + toastFail('Every embryo is marked as a reference — assign at least one subject'); + return false; + } + return true; } async function startRun() { const b = $('op-run-start'); - const done = () => { if (b) { b.disabled = false; b.textContent = 'Start'; } }; - if (b) { b.disabled = true; b.textContent = 'Starting…'; } + const done = () => { if (b) { b.disabled = false; renderRunButton(); } }; + if (b) { b.disabled = true; b.textContent = 'Working…'; } try { if (_mode === 'single') { if (!_selected) { toastFail('Select an embryo first'); return; } @@ -1166,6 +1209,7 @@ const OperateManager = (function () { return; } if (_mode === 'adaptive') { + if (!haveSubjects()) return; const interval = Math.max(1, Number(($('op-tl-interval') || {}).value) || 120); const sel = ($('op-tl-stop') || {}).value || 'manual'; const val = Math.max(1, Number(($('op-tl-condval') || {}).value) || 1); @@ -1187,6 +1231,7 @@ const OperateManager = (function () { } if (_mode === 'library') { if (!_selectedLib) { toastFail('Pick a saved tactic'); return; } + if (!haveSubjects()) return; const d = await postJSON('/api/operate/run-tactic', { library_id: _selectedLib, embryo_ids: subjectIds() }); if (d.success) { toast('Tactic started'); renderRun(); } @@ -1560,6 +1605,7 @@ const OperateManager = (function () { // surface would never get its zoom. attachImageViews(); mountPanels(); + renderRunButton(); if (_active) return; _active = true; showPaneInitial(); diff --git a/gently/ui/web/templates/index.html b/gently/ui/web/templates/index.html index 96cf0693..7f9d04ef 100644 --- a/gently/ui/web/templates/index.html +++ b/gently/ui/web/templates/index.html @@ -770,7 +770,10 @@

Device - + +
diff --git a/tests/test_run_button_names_its_verb.py b/tests/test_run_button_names_its_verb.py new file mode 100644 index 00000000..fe241524 --- /dev/null +++ b/tests/test_run_button_names_its_verb.py @@ -0,0 +1,88 @@ +"""`Start` must say which of four things it will do. + +`startRun` branches on `_mode` into four different verbs — `single` acquires +one volume and finishes, `adaptive` starts a timelapse, `library` runs a saved +tactic, `agent` hands over a prompt. The button said "Start" for all four, and +the mode selector lives in a block above it, so an operator who chose a mode +and then looked away had nothing on the button to read. + +Two invariants worth pinning, because both rot silently: + +1. Every mode has a verb. A mode added to `setMode`'s list but not to + `RUN_VERB` falls back to "Start" and quietly reintroduces the ambiguity. +2. `subjectIds()` keeps no fallback. The old `subs.length ? subs : all` could + only ever fire when EVERY embryo was marked `calibration` — an embryo with + no role, or 'test', or 'unassigned' already passes the filter. So it fired + exactly when the operator had said "these are all references" and answered + by imaging all of them as subjects. + +ponytail: source assertions, because operate.js is an IIFE with no export +surface and CI runs no JavaScript. They check the shape, not the behaviour. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +OPERATE = ( + Path(__file__).resolve().parents[1] / "gently" / "ui" / "web" / "static" / "js" / "operate.js" +) + + +def _src() -> str: + return OPERATE.read_text(encoding="utf-8") + + +def test_every_run_mode_has_a_verb() -> None: + src = _src() + + block = src[src.index("const RUN_VERB = {") : src.index("function renderRunButton")] + verbs = set(re.findall(r"(\w+):\s*'", block)) + + assert re.search(r"\['single', 'adaptive', 'library', 'agent'\]", src), ( + "setMode's mode list changed shape — update this test alongside it" + ) + modes = {"single", "adaptive", "library", "agent"} + + assert modes <= verbs, ( + f"modes with no verb in RUN_VERB: {sorted(modes - verbs)} — the button " + "falls back to 'Start' for those and the ambiguity is back" + ) + + +def test_single_mode_does_not_claim_to_start_anything() -> None: + """It acquires one volume and finishes. The label must not imply a run.""" + src = _src() + block = src[src.index("const RUN_VERB = {") : src.index("function renderRunButton")] + single = re.search(r"single:\s*'([^']+)'", block) + assert single, "single mode lost its verb" + label = single.group(1).lower() + assert "start" not in label, ( + f"single mode's label is {label!r} — it acquires one volume and stops, " + "so it must not read as starting an experiment" + ) + + +def test_subject_ids_keeps_no_fallback() -> None: + src = _src() + body = src[src.index("function subjectIds()") :] + body = body[: body.index("}")] + assert "_embryos.map" not in body, ( + "subjectIds() has a fallback again — it can only fire when every embryo " + "is a reference, and it answers by imaging all of them as subjects" + ) + + +def test_a_reference_only_roster_is_refused_rather_than_imaged() -> None: + src = _src() + assert "function haveSubjects()" in src + assert src.count("haveSubjects()") >= 3, ( + "haveSubjects is defined but not guarding both roster-driven run modes" + ) + # "no embryos" and "no subjects among your embryos" need different fixes, + # so the guard must distinguish them rather than emitting one message. + guard = src[src.index("function haveSubjects()") :] + guard = guard[: guard.index("\n }")] + assert "No embryos registered" in guard + assert "marked as a reference" in guard