docs(flomni): add GUI test checklist and session-handoff instructions
CI for csaxs_bec / test (push) Successful in 1m31s
CI for csaxs_bec / test (push) Successful in 1m31s
TOMO_QUEUE_GUI_TESTING.md is the manual verification plan for the new command-job builder: baseline regression checks, the builder itself, the status-keyed delete/clear guards, and an end-to-end mixed-queue run, plus this iteration's known gaps (no mid-run reorder, one device per move step, no live limit validation). Nobody has clicked through the actual widget against a live session yet - it's built and smoke-tested at the logic level only, not GUI-validated. Section 8 covers continuing this work after leaving the current coding session: a Claude Project with AI_docs/ attached as project knowledge is for planning/discussion, a Claude Code session attached to this repo is what's actually needed to keep editing tomo_params.py - and a ready-to-paste prompt for starting that next session. Also updates TOMO_QUEUE_COMMAND_JOBS_PLAN.md's status line: Step 3 has a first iteration built, not the "not started" it said before.
This commit is contained in:
Mirko Holler
@@ -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
|
||||
|
||||
|
||||
@@ -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.<device>.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.<name>.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.
|
||||
Reference in New Issue
Block a user