From 726915134fea37ec698291e07f4cc163b882cbb6 Mon Sep 17 00:00:00 2001 From: P S Kesavan Date: Sun, 6 Sep 2026 02:16:24 +0530 Subject: [PATCH] fix(operate): make Start say which of four things it will do MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `startRun` branches on `_mode` into four different verbs. 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. single Acquire one volume acquires one volume and finishes adaptive Start timelapse library Run tactic agent Brief the agent `single` is the one that matters: it takes a single volume and stops. Calling that "Start" invited an operator to believe they had started an experiment. The markup now ships single's verb so there is no flash of the wrong one on load, and the label is restored after a run rather than reverting to "Start". ## The fallback that was exactly backwards const subs = _embryos.filter(e => e.role !== 'calibration').map(e => e.id); return subs.length ? subs : _embryos.map(e => e.id); The fallback was 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`. So it 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, and silent. Gone. `haveSubjects()` now refuses at the single point both roster-driven modes pass through, and distinguishes the two states, because "no embryos" and "no subjects among your embryos" need different fixes. ## Checks Four source assertions: every mode has a verb (a mode added to `setMode` but not to `RUN_VERB` silently falls back to "Start"), single's label does not contain "start", `subjectIds` keeps no fallback, and the guard names both states rather than emitting one message. Verified in the running app: labels track the mode with no flash on load, and all three roster states produce the right answer — every-reference refused by name, empty roster refused differently, one real subject allowed through. Audit findings 4 and 5 in docs/devices-tab-audit.md. Co-Authored-By: Claude Opus 5 (1M context) --- gently/ui/web/static/js/operate.js | 54 +++++++++++++-- gently/ui/web/templates/index.html | 5 +- tests/test_run_button_names_its_verb.py | 88 +++++++++++++++++++++++++ 3 files changed, 142 insertions(+), 5 deletions(-) create mode 100644 tests/test_run_button_names_its_verb.py 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