diff --git a/csaxs_bec/bec_ipython_client/plugins/flomni/AI_docs/TOMO_QUEUE_COMMAND_JOBS_PLAN.md b/csaxs_bec/bec_ipython_client/plugins/flomni/AI_docs/TOMO_QUEUE_COMMAND_JOBS_PLAN.md index 1c19f99..1a582da 100644 --- a/csaxs_bec/bec_ipython_client/plugins/flomni/AI_docs/TOMO_QUEUE_COMMAND_JOBS_PLAN.md +++ b/csaxs_bec/bec_ipython_client/plugins/flomni/AI_docs/TOMO_QUEUE_COMMAND_JOBS_PLAN.md @@ -8,9 +8,13 @@ registry, `move` + `optimize_idgap` (stub) actions, `tomo_queue_add_command`, blank params table. Sim-tested: `omny_e2e_tests/test_tomo_queue_command_jobs.py`, 6/6 passing (see `TOMO_QUEUE_TESTING.md` §4D for what each test covers); real device movement was exercised against `ftray` (sim-testable stand-in), not `mokev`/`idgap` -(real front-end only — see §10). **Step 3 (the `TomoQueueDialog` GUI work) is not -started** — a separate, larger PyQt change. Step 1 (executor rework) is implemented -and its sim checklist has passed (`TOMO_QUEUE_TESTING.md` §4, sections A–C). +(real front-end only — see §10). **Step 3 (the `TomoQueueDialog` GUI work) has a +first iteration built** — the command-job builder dialog (§6.5), command-job-aware +queue rendering, and the status-keyed delete/clear guards from §6.3 (partial: no +mid-run reorder yet, §6.4). **Not yet validated against a live session** — see +`TOMO_QUEUE_GUI_TESTING.md` for the manual test checklist and known first-iteration +gaps. Step 1 (executor rework) is implemented and its sim checklist has passed +(`TOMO_QUEUE_TESTING.md` §4, sections A–C). **Prerequisite (met):** the checklist in `TOMO_QUEUE_TESTING.md` has passed against the simulated flOMNI. That document is the companion to this one: it covers *what @@ -359,6 +363,8 @@ status hit `done` (stuck at `running`). Idempotent → re-runs harmlessly. Non-i The agreed interaction model. Its whole purpose: **the GUI stays usable while a tomogram is running** — you can queue more work without touching the running scan. +A first iteration against this model is now built — see `TOMO_QUEUE_GUI_TESTING.md` +for what's actually implemented vs. still open per subsection below. ### 6.1 The load-bearing rule: two separate write paths diff --git a/csaxs_bec/bec_ipython_client/plugins/flomni/AI_docs/TOMO_QUEUE_GUI_TESTING.md b/csaxs_bec/bec_ipython_client/plugins/flomni/AI_docs/TOMO_QUEUE_GUI_TESTING.md new file mode 100644 index 0000000..bff00c3 --- /dev/null +++ b/csaxs_bec/bec_ipython_client/plugins/flomni/AI_docs/TOMO_QUEUE_GUI_TESTING.md @@ -0,0 +1,246 @@ +# Tomo Queue GUI — Step 3, First Iteration: What to Test, and How to Continue + +**Read this first if you're picking this work back up**, whether that's you +(Mirko) at the beamline, or a fresh Claude Code session with no memory of +building it. Section 8 explains exactly how to hand this off to a new +session/chat. + +**Status:** First iteration of the Step 3 GUI work is **built, not yet +validated**. It compiles, imports cleanly, and its non-Qt-rendering logic +(dynamic form building, step ordering, idempotent-default computation, job +dict assembly) was exercised with a scripted fake-client smoke test — but +nobody has clicked through the actual widget against a live BEC session yet. +That's what this document is for. Companion docs: `TOMO_QUEUE_COMMAND_JOBS_PLAN.md` +section 6 (the design this implements), `TOMO_QUEUE_TESTING.md` (Steps 1 & 2, +already sim-validated). + +--- + +## 1. What was built + +All changes are in `csaxs_bec/bec_widgets/widgets/tomo_params/tomo_params.py`. +The pre-existing `TomoParamsWidget`/`TomoQueueDialog` baseline (param editing, +busy banner, basic queue add/delete/clear, 2 s polling) was already there +before this iteration — see `TOMO_QUEUE_COMMAND_JOBS_PLAN.md`'s "The GUI as +it stands" for what that baseline covers. This iteration adds command-job +support on top of it: + +1. **`CommandJobBuilderDialog`** (new class) — the structured builder from + plan section 6.5. Opened via a new **"Add command…"** button in + `TomoQueueDialog`. Reads the live `tomo_queue_actions` global var (the + registry `Flomni.__init__` publishes) so its action dropdown and + per-action fields always match whatever's actually in `flomni.py` — no + hardcoded mirror. Lets you add steps in order (action + its own fields + only — a device dropdown and target for `move`, a bounded spinbox for + `optimize_idgap`'s `search_range`), remove a step, reorder steps within + the job (move up/down), and set a label + idempotent flag (defaults to + "every step idempotent" ANDed, same as the CLI, until you touch the + checkbox yourself). On confirm it produces a job dict byte-for-byte + compatible with what `flomni.tomo_queue_add_command()` writes. +2. **Command-job-aware queue table.** `TomoQueueDialog`'s table gained a + "Details" column; a command job shows `CMD` in the Type column and its + step summary + idempotency marker in Details (e.g. + `move{'positions': {'mokev': 6.2}} > optimize_idgap{'search_range': 0.5} [idem]` + — literally the same string `flomni.tomo_queue_show()` prints). +3. **Self-describing tomo jobs.** "Add current params to queue" now stamps + `"kind": "tomo"` and a fresh `"id"` on the job it writes, matching what + the CLI's `tomo_queue_add()` has done since Step 1/2 (previously the GUI + wrote jobs with neither field — harmless, since the executor treats a + kind-less job as `"tomo"` and heals missing ids on the next + `tomo_queue_execute()`, but no longer necessary). +4. **Status-keyed delete/clear guards** (plan section 6.3, partial — see + section 7 below for what's *not* done). Deleting a selection that + includes a `"running"` job, or clearing the queue while any job is + `"running"`, now refuses with an explanatory message instead of silently + doing it. + +--- + +## 2. How to open the widget + +This is a `BECWidget` plugin (`PLUGIN = True`), so it's added to a BEC GUI +layout the same way any other BEC widget is — via the widget picker in the +running BEC GUI application, in a FlOMNI session (`bec --session flomni`). +It only renders its real UI if `fsamroy` is present in the session (the +FlOMNI discriminator); otherwise it shows the "setup not detected" message. + +If you don't already have a FlOMNI GUI session up, see +`SIMULATED_ENDSTATIONS.md` for bringing up the simulated flOMNI, or start the +real session per `docs/user/ptychography/flomni.md`'s "How to setup flOMNI +(software)" section. + +--- + +## 3. Test plan — baseline regression (should already work; confirm nothing broke) + +These exercise the pre-existing widget, not new code, but touching the same +file is exactly how a regression sneaks in. + +1. Open the widget. Confirm the params panel populates from the live global + vars (matches what `flomni.tomo_parameters()` prints in a parallel CLI). +2. Click **Edit**, change a couple of values (e.g. `tomo_countingtime`, + `fovx`), click **Submit**. Confirm the values landed + (`flomni.tomo_countingtime` in the CLI reflects the new value). +3. Click **Edit** again, change something, click **Cancel**. Confirm the + field reverts to the last-submitted value, not your edit. +4. Start a real (or fast/sim) tomo scan from the CLI + (`flomni.tomo_scan()`), then open the widget while it's running. Confirm + the busy banner appears with a reason. Try **Submit** — confirm you get + the "scan appears to be running" confirmation dialog, not a silent write. +5. Click **☰ Queue control…**. Confirm the dialog opens and the table + reflects the current `tomo_queue` (including anything you added earlier + via CLI in prior testing sessions). + +## 4. Test plan — new: command-job builder + +6. Click **"Add command…"**. Confirm the dialog opens with an **action + dropdown showing `move` and `optimize_idgap`** (or whatever's currently + in `flomni.py`'s `_TOMO_QUEUE_ACTIONS` — if you've added a third action + since this was written, it should appear automatically with no GUI + change needed; that's the point of reading the live registry). +7. With `move` selected, confirm the field area shows a **device dropdown** + (should list `mokev`, `idgap`, `ftray` per the current allow-list) and a + **numeric target field**. Pick a device, set a target, click **"Add + step"**. Confirm a line appears in the step list above, formatted like + `move{'positions': {'mokev': 6.2}}`. +8. Switch the action dropdown to `optimize_idgap`. Confirm the field area + now shows a single **search_range** spinbox pre-filled with `0.5`, + range-limited (try typing outside 0–2, confirm the widget clamps it + rather than accepting it). Click **"Add step"**. +9. Confirm the **idempotent checkbox auto-checked itself** after both adds + (both current actions default to idempotent=True). Uncheck it by hand, + then add another step — confirm it **stays unchecked** (your override + sticks; it doesn't get silently recomputed back to checked). Leave it + checked/matching your intent before continuing. +10. With two steps in the list, select the second and click **"Move up"**. + Confirm the list order swaps. Click **"Remove step"** on one entry, + confirm it's gone from the list. +11. Type a label, click **OK**. Confirm the dialog closes and a new row + appears in the queue table with **Type = `CMD`** and a **Details** + column showing the step summary + `[idem]`/`[NOT idem]` marker matching + what you set. +12. Cross-check from the CLI: `flomni.tomo_queue_show()` should print that + exact same job, in the exact same `CMD ... [idem]` format. `flomni._tomo_queue_proxy.as_list()[-1]` + should show `"kind": "command"`, a `steps` list matching what you built, + and no `"params"` key. + +## 5. Test plan — new: status-keyed guards + +13. Start a queue running from the CLI (`flomni.tomo_queue_execute()`) with + at least one job that takes a little while (or use the + `omny_e2e_tests/_queue_helpers.py`-style fast params if you're doing + this against the sim and want it quick — see `TOMO_QUEUE_TESTING.md` + section 3's "making scans short enough to test"). +14. While a job shows **status = running** in the GUI table, select that row + and click **"Delete selected"**. Confirm you get a warning dialog + refusing the delete, not a confirmation prompt that would actually do it. +15. With the same running job, click **"Clear all"**. Confirm you get a + warning refusing to clear, rather than wiping the queue out from under + the executing job. +16. Let the queue finish (or stop it / let it go `incomplete`), then confirm + you *can* now delete/clear normally. + +## 6. Test plan — mixed queue, end to end + +17. Build a small realistic sequence via the GUI only: a tomo job ("Add + current params to queue"), then a command job (e.g. a `move` on + `ftray` if you're on the sim — `mokev`/`idgap` only exist on the real + front-end, see plan section 3.1), then another tomo job. +18. From the CLI, run `flomni.tomo_queue_execute()`. Confirm: the tomo job + runs, the command job actually moves the device (check + `dev..readback.get()` before/after), and the second tomo job + runs after — all while the GUI table (polling every 2 s) shows each + job's status flip from `pending` → `running` → `done` in order, without + needing to close/reopen the dialog. + +## 7. Known limitations of this first iteration (by design, not bugs) + +Worth knowing before you file something as broken: + +- **No mid-run drag-reorder of queue rows** (plan section 6.4). You can + reorder *steps within a command job* while building it, but not reorder + *jobs* in the queue itself, running or not. Today reordering the queue + still has to go through direct global-var manipulation + (`flomni._tomo_queue_proxy`) or isn't exposed at all from the GUI. This is + the biggest deferred piece — a real drag-and-drop implementation needs the + "reject any drop above the running row" logic from the plan, which is + nontrivial enough to warrant its own iteration. +- **One device per `move` step in the builder**, even though the underlying + schema (`positions: {device: target, ...}`) technically supports moving + several devices in one step. The CLI can still do this + (`tomo_queue_add_command` with a `positions` dict of more than one + device); the GUI builder just doesn't expose it yet. Add extra `move` + steps instead if you need to move more than one device. +- **No live device-limit validation.** The plan (section 6.5) mentions + validating against `dev..limits` in addition to the schema's + min/max; this iteration only enforces the schema bounds (via the spinbox + range), not real hardware limits. The execution-time re-check in + `_queue_action_move()` is still the actual safety boundary regardless + (two-layer validation, plan section 3.3), so this is a UX gap, not a + safety gap. +- **`Done` is a soft warn, not a hard block, while a scan runs** (existing + pre-Step-3 behaviour, not something this iteration touched). The plan's + section 6.1 literally says "must be blocked"; the actual implementation + allows submitting with a confirmation dialog instead, on the reasoning + that some live params (e.g. counting time) safely take effect on the next + projection while others are already baked into the current run's + trajectory. Worth a deliberate decision (keep as documented nuance, or + tighten to match the plan literally) rather than silently "fixing" — flag + it if a real submit-while-running mistake actually happens during testing. +- **No automated widget tests.** `pytest-qt` isn't a dependency of this + repo, so there's no `qtbot`-driven test suite here — the smoke test that + validated the builder's logic (dynamic fields, step ordering, idempotent + default, job-dict shape) was a one-off script against a fake client, not + a committed test. If this GUI work continues, consider whether adding + `pytest-qt` is worth it. + +--- + +## 8. Continuing this work in a fresh Claude session + +You'll be leaving this coding session at some point. Two different things +you might want a fresh Claude session to do, and they need different setups: + +### To keep writing/fixing GUI code + +You need a **Claude Code session with access to this repository** — a new +terminal/CLI session, the desktop app, or a browser-based Claude Code +session, pointed at this same checkout (or a fresh `git clone` of this +branch after you push). A plain chat on claude.ai, without a connected +coding environment, **cannot edit files in this repo** — it can only discuss +and produce text/snippets for you to paste in yourself. + +When you start that fresh session, just point it at this file and the plan +doc — it doesn't need you to re-explain any of the above: + +> Read `csaxs_bec/bec_ipython_client/plugins/flomni/AI_docs/TOMO_QUEUE_GUI_TESTING.md` +> and `TOMO_QUEUE_COMMAND_JOBS_PLAN.md` section 6, then continue the Step 3 +> GUI work in `tomo_params.py` — pick up from section 7's known +> limitations, starting with [whichever one you want tackled next]. + +### To plan/discuss without editing code (a Claude Project) + +This is what a **Claude Project** on claude.ai is good for: attach the +`AI_docs/` files (this one, `TOMO_QUEUE_COMMAND_JOBS_PLAN.md`, +`TOMO_QUEUE_TESTING.md`, `E2E_TESTING_GUIDE.md`, `SIMULATED_ENDSTATIONS.md`) +as project knowledge, and a chat inside that project will have the full +design/status context without you re-explaining it every time — useful for +sketching out the next iteration's design, reviewing test results, or +drafting the instruction you then hand to an actual Claude Code session (the +E2E_TESTING_GUIDE.md was written for exactly this handoff pattern already; +this document and the plan doc work the same way for the GUI). + +**The concrete loop that actually moves code forward:** discuss/plan in the +Project chat if useful, then paste the resulting instruction into a real +Claude Code session attached to this repo to make the edit, then come back +here (or to a fresh Code session) to test it against a live GUI — a Project +chat by itself never touches `tomo_params.py`. + +If you run through sections 3–6 above yourself before switching sessions, +paste your findings (what passed, what didn't, exact error text if +something broke) into whichever session picks this up next — "I clicked +through it and it didn't work" is much less useful than "step 8: typing 2.5 +into search_range with the sim's flomni_sim fixture up, the widget raised +`AttributeError: ...`". Update this file's section 1 status line once +you've actually run through the checklist, so the next session doesn't +re-read "not yet validated" after you've validated it.