From 6935115c8aa56592fb7ec9ebfb7f894c3f0c38cf Mon Sep 17 00:00:00 2001 From: wyzula-jan Date: Fri, 14 Aug 2026 14:15:31 +0200 Subject: [PATCH] feat(agents): curated AGENTS.md and CLAUDE.md files added with best practives for the project --- AGENTS.md | 207 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ CLAUDE.md | 37 ++++++++++ 2 files changed, 244 insertions(+) create mode 100644 AGENTS.md create mode 100644 CLAUDE.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..5f49f11 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,207 @@ +# Repository Guidelines — `ophyd_devices` + +`ophyd_devices` is the **hardware abstraction layer** for +[BEC (Beamline Experiment Control)](https://github.com/bec-project/bec). It extends +[ophyd](https://github.com/bluesky/ophyd) with device support for hardware that the standard EPICS +implementation does not cover — motion controllers, detectors, shutters, undulators, monochromators — +plus a full simulation framework so BEC can run end-to-end with no hardware attached. + +This file is a quick-reference for AI coding agents (and new contributors). User-facing documentation +lives at ; general ophyd concepts are documented at +. + +## Project Structure & Module Organization + +`ophyd_devices/` is the importable package: + +| Path | What goes there | +| --- | --- | +| `ophyd_devices/interfaces/base_classes/` | The classes you should inherit from: `PSIDeviceBase`, `PSIPositionerBase`, `PSIPseudoDeviceBase`, `PSIPseudoMotorBase`. Start here before writing a device. | +| `ophyd_devices/interfaces/protocols/` | `typing.Protocol` definitions (`BECDeviceProtocol`, `BECPositionerProtocol`, `BECFlyerProtocol`, …) describing what BEC expects from a device. Useful as a checklist and in `isinstance` tests. | +| `ophyd_devices/interfaces/device_config_templates/` | Templates for generating device configuration entries. | +| `ophyd_devices/devices/` | Concrete device implementations (`psi_motor.py`, `undulator.py`, `optics_shutter.py`, `dxp.py`, `areadetector/`, `panda_box/`, …), plus the generated `device_list.md`. | +| `ophyd_devices/sim/` | The simulation framework: `SimPositioner`, `SimCamera`, `SimMonitor`, `SimWaveform`, `SimFlyer`, and the `sim_data.py` data generators behind them. | +| `ophyd_devices/utils/` | Shared helpers: `bec_signals.py`, `bec_scaninfo_mixin.py`, `controller.py`, `socket.py`, `psi_device_base_utils.py` (`FileHandler`, `TaskHandler`), `static_device_test.py`. | +| `ophyd_devices/configs/` | Example device configuration YAML files, including the simulation config used for local BEC runs. | +| `ophyd_devices/npoint/`, `rt_lamni/`, `sls_devices/`, `smaract/` | Vendor- and facility-specific integrations. | +| `tests/` | The test suite (flat; one `test_.py` per area). | + +`ophyd_devices/devices/device_list.md` is **generated by CI** on pushes to `main` — do not edit it by hand. + +## Local Environment Overlay + +If a file named **`AGENTS_PERSONAL.md`** exists next to this one, read it and treat it as an extension +of this file. It carries machine-specific setup — interpreter and environment manager, local paths, +private workflow conventions — and **its instructions take precedence over the generic +"Development Environment" section below**. Everything else in this file still applies. + +That file is intentionally untracked and personal to one developer's machine. Do not commit it, do not +reference it from committed files, and do not assume it exists — if it is absent, follow this file as +written. + +## Development Environment + +Requires **Python 3.11+** (CI tests 3.11, 3.12, 3.13). + +```bash +python -m venv .venv +source .venv/bin/activate # macOS/Linux only; see "Platform Notes" +python -m pip install --upgrade pip +python -m pip install -e '.[dev]' +``` + +The `dev` extra pulls in `bec-server`, which is what the device-server-facing tests exercise. Verify the +environment resolves to this checkout: + +```bash +python -c "import ophyd_devices; print(ophyd_devices.__file__)" +``` + +No EPICS IOC or hardware is needed for development: unit tests mock connections, and the `sim/` devices +provide a full working beamline in software. + +## Writing a Device + +**Inherit from a `PSI*` base class, not from `ophyd.Device` directly.** `PSIDeviceBase` wires up the +subscription types BEC's device manager expects (`readback`, `value`, `done_moving`, `motor_is_moving`, +`progress`, `file_event`, `device_monitor_1d`, `device_monitor_2d`), gives you `scan_info` and +`device_manager`, and provides the `FileHandler` / `TaskHandler` utilities. A bare `ophyd.Device` will +appear to work locally and then misbehave inside a running BEC deployment. + +```python +from ophyd import Component as Cpt, EpicsSignal, EpicsSignalRO + +from ophyd_devices.interfaces.base_classes.psi_device_base import PSIDeviceBase + + +class MyDetector(PSIDeviceBase): + """One-line description; this text reaches the generated device list.""" + + acquire = Cpt(EpicsSignal, "ACQ", kind="omitted") + readback = Cpt(EpicsSignalRO, "VAL", kind="hinted") + + def on_stage(self) -> None: + ... # prepare for a scan + + def on_complete(self) -> None: + ... # wait for acquisition to finish + + def on_unstage(self) -> None: + ... # release resources +``` + +Rules of thumb: + +- **Where `ophyd_devices` provides a counterpart to an `ophyd` class, always import the + `ophyd_devices` one.** Several ophyd classes are subclassed here to add BEC behaviour, and the plain + ophyd version silently loses it. The status classes in `ophyd_devices/utils/psi_device_base_utils.py` + — `StatusBase`, `Status`, `DeviceStatus`, `MoveStatus`, `SubscriptionStatus`, `AndStatus` — add + timeout diagnostics that report which device and which call is stuck, and the `&` operator for + composing statuses. The same module adds BEC-only statuses with no ophyd equivalent + (`CompareStatus`, `ExceptionStatus`, `TransitionStatus`, `TaskStatus`), and signals that publish to + BEC live in `ophyd_devices/utils/bec_signals.py`. + + ```python + from ophyd_devices.utils.psi_device_base_utils import DeviceStatus, MoveStatus # yes + from ophyd.status import DeviceStatus, MoveStatus # no + ``` + + The same applies to anything re-exported from `ophyd_devices/__init__.py`. Importing straight from + `ophyd` stays correct only for what has no counterpart here — `Component`, `EpicsSignal`, `Kind`, + `PositionerBase`, and friends. +- **Never block.** Long-running work goes through `TaskHandler`, and completion is reported with a + `DeviceStatus`. A device that blocks in `stage()` or `trigger()` stalls the whole device server. +- **Set `kind` deliberately.** `hinted` signals are recorded by default; `omitted` and `config` are not. + A wrong `kind` either loses data or floods every scan file. +- **Always implement `stop()`** so the device can be interrupted mid-scan and leaves the hardware safe. +- **Emit BEC data through `ophyd_devices/utils/bec_signals.py`** rather than inventing a message shape. +- **Check your device against the protocols** in `interfaces/protocols/bec_protocols.py` — they are the + contract BEC relies on. +- **Add an example configuration** under `ophyd_devices/configs/` when adding a new device family. + +### Validating a device configuration + +`ophyd_test` statically analyses a device configuration YAML, and can optionally connect to the hardware: + +```bash +ophyd_test --config ./ophyd_devices/configs/ophyd_devices_simulation.yaml +ophyd_test --config /path/to/beamline_config.yaml --connect --timeout-per-device 30 +``` + +Reports are written to `./device_test_reports` by default. Run this before proposing a configuration +change for a real beamline. + +## Testing + +```bash +python -m pytest --random-order ./tests +``` + +`--random-order` matches CI and is how order-dependent test pollution gets caught. + +Coverage, as CI measures it: + +```bash +coverage run --source=./ophyd_devices --omit=*/ophyd_devices/tests/* -m pytest --random-order ./tests +coverage report +``` + +**Conventions:** + +- Name files `test_.py` and tests after behaviour — `test_positioner_reports_done_after_move()`, + not `test_positioner_3()`. +- Mock EPICS and sockets. Use `get_mock_scan_info` from `ophyd_devices/tests/utils.py` and the fixtures + in `tests/conftest.py` rather than constructing scan metadata by hand. +- Prefer building on the `sim/` devices when you need a working device in a test. +- Every new device class needs at least: it instantiates, it satisfies the relevant protocol, and its + `stop()` is safe to call. + +## Coding Style & Naming Conventions + +- Python 3.11+, 4-space indentation, **100-character** line limit. +- **Black** and **isort** are the source of truth (settings in `pyproject.toml`). CI fails on any diff: + + ```bash + black --line-length=100 --skip-magic-trailing-comma . + isort --line-length=100 --profile=black --multi-line=3 --trailing-comma . + ``` + +- **Pylint** runs in CI and reports a score; do not introduce new warnings. Beamline-idiomatic names + (`scanID`, `RID`, `pointID`, `*_1D`, `*_2D`) are explicitly allowed via `[tool.pylint.basic]`. +- `snake_case` for modules, functions, and test files; `PascalCase` for device classes. Device class + names should read as the hardware they represent (`SimPositioner`, `PSIMotor`, `DelayGenerator645`). +- Use f-strings and `pathlib`. +- **Docstrings are not optional on device classes** — the first line is picked up by the generated + device list and is what beamline scientists read when choosing a device. + +## Platform Notes + +Code must run on **macOS and Linux**. Windows is not supported or tested. + +## Related Repositories + +- [`bec`](https://github.com/bec-project/bec) — core library and services; `bec_lib` is a direct + dependency, and the device server here is driven by `bec_server`. +- [`bec_widgets`](https://github.com/bec-project/bec_widgets) — GUI toolkit that displays these devices. +- Beamline plugin repositories — beamline-specific devices that do not belong in this shared repository. + +A device used at exactly one beamline belongs in that beamline's plugin repository. This repository is +for hardware support that is reusable across beamlines and facilities. + +## Commit & Pull Request Guidelines + +- **Do not commit or push unless explicitly asked to.** Leave the working tree for the human to review. +- **Never open, update, or merge a pull request.** Submitting the change is the human contributor's + step. An agent's work ends at a reviewed working tree — or at a local commit on a branch, when a + commit was explicitly requested. +- Branch from `main` with a descriptive name such as `feat/panda-position-capture` or + `fix/undulator-timeout`. +- **Conventional Commits are mandatory** — `(): `, e.g. + `fix(psi_motor): report done_moving after limit hit`. Allowed types: `build`, `chore`, `ci`, `docs`, + `feat`, `fix`, `perf`, `refactor`, `style`, `test`. `feat` triggers a minor release, `fix` and `perf` a + patch release; breaking changes need `!` or a `BREAKING CHANGE:` footer. +- Commit messages are parsed by python-semantic-release and become the published `CHANGELOG.md`. Keep + them to a single clean subject line. +- The pull request itself needs a clear description, linked issues, and test evidence, and for a new + device it must state which hardware the device was tested against — or say explicitly that it has + only been tested in simulation. Put that in your summary so whoever opens the PR can carry it over. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..44fde31 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,37 @@ +# CLAUDE.md — `ophyd_devices` + +@AGENTS.md + +The guidelines above are imported from [`AGENTS.md`](AGENTS.md) (single source of +truth). The points that matter most in day-to-day work: + +- **Check for `AGENTS_PERSONAL.md` first.** If it exists, it extends `AGENTS.md` with + machine-specific environment setup and takes precedence over the generic venv/pip instructions there. + It is untracked and personal — never commit it, and never assume it exists. +- **Inherit from `PSIDeviceBase` / `PSIPositionerBase` / `PSIPseudoDeviceBase`** + (`ophyd_devices/interfaces/base_classes/`), never from `ophyd.Device` directly — the base classes wire + up the subscriptions, `scan_info`, and task/file handling that BEC's device server expects. +- **Import the `ophyd_devices` counterpart, never the plain `ophyd` one, wherever one exists.** The + status classes in `ophyd_devices/utils/psi_device_base_utils.py` (`StatusBase`, `Status`, + `DeviceStatus`, `MoveStatus`, `SubscriptionStatus`, `AndStatus`) subclass ophyd's to add timeout + diagnostics and `&` composition, and the module adds BEC-only `CompareStatus`, `ExceptionStatus`, + `TransitionStatus`, `TaskStatus`; BEC-publishing signals live in `ophyd_devices/utils/bec_signals.py`. + Importing from `ophyd` directly silently drops that behaviour. Plain `ophyd` imports are correct only + where there is no counterpart (`Component`, `EpicsSignal`, `Kind`, …). +- **Never block the device server.** Long work goes through `TaskHandler` and reports completion with a + `DeviceStatus`. Always implement a safe `stop()`. +- **Set `kind` deliberately** (`hinted` / `config` / `omitted`) — it decides what lands in the scan file. + Emit BEC data through `ophyd_devices/utils/bec_signals.py`, and check the device against the protocols + in `interfaces/protocols/bec_protocols.py`. +- **Docstring every device class** — the first line feeds the generated + `ophyd_devices/devices/device_list.md`, which is CI-generated and must not be hand-edited. +- **Tests**: `python -m pytest --random-order ./tests`. Mock EPICS and sockets; build on the `sim/` + devices; use `get_mock_scan_info` and the `tests/conftest.py` fixtures. +- **Validate configs** with `ophyd_test --config ` (add `--connect` for real hardware). +- **Format before finishing**: `black --line-length=100 --skip-magic-trailing-comma .` and + `isort --line-length=100 --profile=black --multi-line=3 --trailing-comma .`. +- **A device used at only one beamline belongs in that beamline's plugin repo**, not here. +- **Do not commit or push unless explicitly asked, and never open a pull request.** If you do commit, + write a single Conventional Commits line — it is parsed into the published changelog. Opening the PR + is the human's step; leave them the summary and test output they need for it, including whether a new + device was tested against real hardware or only in simulation.