feat(wellplate): one well-to-stage transform, geometry/placement split, and measured holder-rotation calibration - #611
Conversation
…tal at import
cache/sample_formats.csv was written non-atomically (open("w") + row-by-row)
while load_formats() parses it unguarded at module import time. An interrupted
write therefore left a truncated file that made `import control._def` raise
(ValueError on a blank cell, TypeError on a missing one) and the application
could not start at all. Reproduced with truncations mid-row and mid-number.
Changes:
- control/_def.py: add SAMPLE_FORMAT_CSV_FIELDNAMES as the single source of
truth for the on-disk column order, and write_sample_formats_csv() which
writes a sibling temp file, fsyncs, then os.replace()s onto the target,
cleaning up the temp on any failure.
- load_formats(): a damaged cache now logs an ERROR naming the file and the
consequence (stored plate calibration not applied) and falls back to the
shipped geometry instead of raising. A cache that parses to zero formats
(header-only or empty file - which does NOT raise today, it just fails later
and more confusingly) is treated as damaged too.
- Writer semantics tightened: a settings key missing from the schema raises
KeyError at write time (the old {**row, **settings} form silently emitted the
blank cell that bricks the reader); an extra key is dropped with a warning
naming the field (the old DictWriter used extrasaction='raise', which would
block any future schema addition at the first format save).
- WellplateFormatWidget.save_formats_to_csv reduced to a delegation so exactly
one writer exists.
Tests: tests/control/test_sample_format_cache.py (13 tests) covering the
damaged-cache fallbacks, valid-cache preference, write atomicity on failure,
round-trip, missing/extra key handling, and the widget delegation. Full test
tree verified per-file: 78 files, 1564 passed, 0 failed.
This is commits 1+2 of the sample-format refactor plan
(AI-docs/Squid/in-progress/2026-07-29-sample-format-refactor-plan.md).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three verified-dead things and one stale one, removed ahead of the wellplate rotation-calibration work so later changes have less surface to reason about: - WellplateFormatWidget.parse_csv_row: a duplicate of the CSV parsing in control._def.read_sample_formats_csv with zero callers. A second copy of the schema that could only rot. - NavigationViewer.well_spacing_mm: written twice, read nowhere in the repo. The viewer's mm<->pixel mapping never uses the pitch. The parameter stays in update_wellplate_settings (fixed 10-arg signal), it just isn't stored. - The wellplate ini keys in all 8 tracked configuration templates: wellplate_format, _wellplate_format_options, x/y_mm_384_wellplate_upperleft. Dead because the [GENERAL] loader (control/_def.py) only assigns onto names that already exist as module globals at that point in the file, and WELLPLATE_FORMAT is first defined ~130 lines after the loader; the upperleft vars appear nowhere in Python. The startup format actually comes from cache/objective_and_sample_format.txt. Note: removing _wellplate_format_options also removes a config-editor dropdown that had no runtime effect. - Well1536SelectionWidget's hardcoded geometry defaults: nine constants that had drifted from sample_formats.csv (well_size 1.5 vs 1.53, a1 11.0/7.86 vs 11.01/7.87, a1 pixels 144/108 vs 130/93) and were unconditionally overwritten on every reachable path - both construction sites (gui_hcs.py:981, :2407) pass a live wellplateFormatWidget. Geometry now always comes from the format settings, and the constructor is honestly non-optional. Tests: new tests/control/test_well1536_selection_widget.py pins that the 1536 selector's geometry equals WELLPLATE_FORMAT_SETTINGS and that its emitted navigation position matches the planner's formula exactly - the test that would have caught the drift. Regression (per-file): test_sample_format_cache 13, test_well1536_selection_widget 2, test_widgets 135, test_def 33, test_scan_coordinates 6, test_HighContentScreeningGui 5 - all passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… cancelled
One user-visible symptom - cancelling the calibration dialog left the Sample
Format dropdown stuck on "calibrate format..." - caused by two independent
defects that also fought each other:
1. WellplateFormatWidget.wellplateChanged overwrote self.wellplate_format with
"custom" BEFORE the Rejected branch read it back, so findData("custom")
re-selected the calibrate item itself. Fixed by capturing previous_format
before the overwrite; the revert now has exactly one owner.
2. WellplateCalibration.reject() carried its own competing revert that parsed
navigationViewer.sample to an int ("96 well plate" -> 96), which findData()
could never match against the string item data - it silently did nothing
(and sent custom-named formats to the glass-slide fallback). Deleted; the
dialog no longer second-guesses the widget.
Also in this commit, the same bug class one call further down:
populate_combo_box() starts with comboBox.clear(), so re-selecting a format
after calibration fired currentIndexChanged three times (-1, 0, target) - the
index-0 emission momentarily reconfigured the whole app for whatever format is
first in the dict. New WellplateFormatWidget.select_format_silently() rebuilds
the combo with signals blocked and emits the settings exactly once;
_finish_calibration and update_existing_parameters now use it.
Plus: _get_calibration_data casts the circle-fit center/radius from numpy
float64 to float before they are stored into WELLPLATE_FORMAT_SETTINGS and
written to the CSV.
Tests: tests/control/test_wellplate_format_widget.py (4) - cancel restores the
previous format (once and repeatably), select_format_silently emits exactly
once, and the spurious first-dict-entry emission never happens. All 4 fail on
the unfixed code. Regression: test_widgets 135 passed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
create_wellplate_image saved "my custom plate" as images/my_custom_plate.png (spaces -> underscores) while NavigationViewer looked up images/my custom plate.png with the raw name - so every custom format named with a space, including the dialog's own placeholder text "custom well plate", generated a plate image the navigation viewer could never find and silently fell back to the slide-carrier image. The write now preserves spaces (matching the shipped assets, e.g. "96 well plate_1509x1010.png"); the viewer tries the raw name first, then the legacy underscore variant for images generated by older builds, and the default-image fallback now logs a WARNING naming both paths it tried and the consequence. Tests: tests/control/test_custom_plate_image_lookup.py (3) - write preserves spaces; the REAL viewer lookup (run on a minimal instance) prefers raw over legacy and accepts legacy alone; total fallback warns. Regression: test_widgets 135 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…every site
Scaffolding for routing the duplicated forward formula through one transform:
pins CURRENT behaviour with exact equality (==, never approx) so the upcoming
swaps are provably no-ops where they claim to be, and the intentional
divergences are written down rather than discovered.
Covered, parameterized over WELLPLATE_OFFSET in {0, nonzero}:
- ScanCoordinates.get_selected_wells - every well of every format
- ScanCoordinatesSiLA2.get_selected_well_coordinates - both the range
(serpentine) and single-well branches, every well of every format
- MicroscopeControlServer._parse_wells - asserted OFFSET-FREE, the documented
divergence; the server-fix commit flips this expectation in the same diff
- WellSelectionWidget.onDoubleClick and Well1536SelectionWidget
.update_current_cell - corner + interior probes through the Qt signal path
(these two bind WELLPLATE_OFFSET_* into the widgets namespace via
star-import, so the tests patch control.widgets, not control._def - itself
worth pinning)
- the snapshot-vs-live offset inconsistency: ScanCoordinates snapshots the
offsets at __init__ while the SiLA2 path reads them live at call time; the
compute-time-resolution commit will flip this test to agreement
Operand order across sites (a1 + x + off vs x + a1 + off) is commutativity of
a single addition - IEEE-754 exact - so one oracle formula serves all sites
with bitwise ==. 41 tests, all green against the unmodified code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…esolver Pure module, zero call sites yet - the next commits route the duplicated forward-formula copies through it one site at a time, each guarded by the golden oracle. - PlateTransform (frozen dataclass): well_center_mm(row, col) in the exact legacy operand order so the identity path is ==-identical to today's arithmetic; per-axis pitches (column * pitch_x, row * pitch_y) scaled in the plate frame BEFORE rotation; rotation (when it lands) pivots on A1 so rotation_deg == 0 leaves a1_x/y_mm meaning untouched. Deliberately scalar, not a matrix multiply: folding a1 + offset into a translation column re-associates the sum and drifts the last bit in ~27% of cases. - well_index_at(x, y): fractional (row, col) inverse - new capability, nothing computes "which well am I in" today. Raises PlateGeometryError on pitch 0 (glass slide, format '0'); callers short-circuit those to identity. - nominal(): plate-frame copy (no rotation, no offset) for display assets - the plate-PNG renderer stays registered against the offset-free a1_x_pixel = round(a1_x_mm * scale) convention. - plate_transform_for(format_): the single resolver. Reads control._def at CALL time - never cached across calls; the snapshot-at-__init__ pattern is the bug this module exists to end. The placement sidecar and holder rotation compose here when they land. Tests (15): identity == legacy arithmetic for every well of every format under a nonzero offset; resolver reads offsets at call time; round-trip inverse to 1e-9; rotation pivots on A1; CCW direction pinned; anisotropic scale-before-rotate pinned; glass-slide inverse raises; unknown format raises; plate-frame mode ignores the legacy offset. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ough PlateTransform The load-bearing swap: ScanCoordinates.get_selected_wells, both branches of ScanCoordinatesSiLA2.get_selected_well_coordinates, WellSelectionWidget .onDoubleClick and Well1536SelectionWidget.update_current_cell now compute well centers via PlateTransform.well_center_mm instead of four inline copies of the formula. No behaviour change, asserted with exact equality: the golden oracle (41 tests, untouched by this commit) passes unchanged under both offset parameterizations. Each site keeps its exact current offset semantics, including the ones that disagree with each other: - ScanCoordinates builds the transform from its __init__-snapshot offsets; - the SiLA2 path reads control._def offsets live at call time; - the widgets read the WELLPLATE_OFFSET_* bindings in the widgets namespace; - the oracle's snapshot-vs-live disagreement test still passes, pinning that the swap did not accidentally unify them (that is a later, explicit commit). The two intentionally-divergent sites (MCP server, plate-PNG renderer) are NOT touched here - each gets its own commit so the behaviour change/decision is visible next to the diff. Regression: oracle+transform+scan_coordinates 62, test_widgets 135, selector/ format-widget/live-grid 8, microscope_control_server 19 - all passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…etry in _parse_wells BEHAVIOR CHANGE (intentional, the one site swap that is not identity): on a machine configured with a nonzero wellplate_offset_x/y_mm, wells addressed through the control server / MCP acquisition path now land at the same stage positions as the same wells addressed through the GUI. Previously this was the one copy of the forward formula that omitted the offset entirely. Also removed, per the no-arbitrary-numbers rule: the invented fallbacks a1=0 / spacing=9 for missing settings keys (an unknown format now raises via get_wellplate_settings instead of silently acquiring at fabricated positions), and the local row_to_index duplicate (control.utils.row_to_index is the one implementation; the bijective base-26 label generator is reused from ScanCoordinates). _parse_wells now takes the format name and resolves through plate_transform_for like every other site; both call sites simplified. The golden oracle's site-3 expectation flips in this same diff, from "documented divergence: offset-free" to offset-applied, plus a new unknown-format-raises test - so the change is asserted, not just described. Regression: oracle 42 + server 19 + mcp_view_settings all passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ame transform The sixth copy of the forward formula, and the one whose correct construction is the OPPOSITE of the other five: create_wellplate_image draws a plate-frame display asset registered against the offset-free a1_x_pixel = round(a1_x_mm * scale) convention, so it must never see WELLPLATE_OFFSET (or, later, rotation). Well centers and row/column label positions now come from a nominal PlateTransform (no offset, no rotation), with a comment explaining the intent so the next person does not "fix the inconsistency" by adding the offset back. New test: generating the image under offset 0 and offset 25 mm produces byte-identical files - the offset provably cannot leak into the asset. Regression: image-lookup 4 + test_widgets 135 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ith a WellplateSettings object The signal carried 10 positional args and its slots declared fewer - ScanCoordinates.update_wellplate_settings accepted 8, so Qt silently dropped rows/cols in transit; adding an 11th parameter (the placement work needs one) would have required changing every emit, connect and slot in lockstep or silently not arriving. An object cannot be truncated in transit. - WellplateSettings frozen dataclass lives in control/core/plate_transform.py (no Qt dependency), with from_format() resolving through get_wellplate_settings and glass_slide() reproducing the literal fallback values the old code emitted. - signalWellplateSettings = Signal(object); both emits in setWellplateSettings collapse to one-liners. - NavigationViewer and ScanCoordinates slots take the object; the obsolete QVariant unwrap in the viewer is gone; the gui_hcs lambda forwards settings.format. The two well-selector onWellplateChanged slots take no arguments and are unaffected. Regression: oracle + transform + widget tests 67, test_widgets 135, HCS GUI 5, scan_coordinates + control server 25 - all passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nCoordinates The only commit in the series that changes WHEN geometry is read, and it is the semantic fix the design calls out: ScanCoordinates snapshotted a1/pitch/offsets at __init__, so an in-place calibration edit (the dialog mutates WELLPLATE_FORMAT_SETTINGS directly) or an offset change was silently ignored until a full signal re-emit - while the SiLA2 path read the same values live. The same request through GUI and through MCP could plan different coordinates. get_selected_wells() now resolves through plate_transform_for(self.format) at call time. ScanCoordinates keeps only identity + display state (format, well_size_mm - both have external readers); the dead geometry snapshots (a1_x/y_mm, wellplate_offset_x/y_mm, well_spacing_mm, a1_x/y_pixel, number_of_skip) are gone, verified to have no readers anywhere. The golden oracle's snapshot-vs-live disagreement test FLIPS to agreement in this same diff (test_offset_read_time_agreement), plus a new test pinning the concrete payoff: a calibration edit applies to planning without a re-emit. Regression: tests/control/core 401 + server, HCS GUI 5, MultiPoint 20, scan-size consistency - all passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ived data The data model gains what other packages already store (useq well_spacing tuples, Micro-Manager WELL_SPACING_X/Y) without touching the frozen 10-column CSV schema: well_spacing_x/y_mm, well_size_x/y_mm and well_shape are DERIVED in memory - the scalar broadcasts to both axes (ANSI/SLAS pitch is identical in both axes for every shipped format), and well_shape reproduces the old name-based mapping (384/1536 -> rectangle, else circle) as the one code-side default instead of scattered string checks. - _with_derived_geometry() normalizes idempotently on CSV load AND in get_wellplate_settings, so runtime-added custom formats (add_custom_format inserts plain dicts) and the '0' pseudo-format get defaults too. - DERIVED_SAMPLE_FORMAT_KEYS are excluded from the writer's extra-key warning and never persisted: the cache CSV keeps the frozen schema byte-for-byte (header asserted in test). Genuinely anisotropic carriers set these fields via the user-formats YAML (next commit); no shipped value is invented. - plate_transform_for consumes well_spacing_x/y_mm (column x, row y); WellplateSettings carries the five new fields. Tests (+5): broadcast + shape mapping for every shipped format; runtime custom format gets defaults; resolver honors x != y; writer stays warning-free and 10-column. Oracle unchanged (isotropic rows -> bit-identical). Regression: test_widgets 135, def/widget/lookup 43. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mple_formats_user.yaml First half of the geometry/placement split. The shipped sample_formats.csv is byte-frozen; what a lab deliberately changes now lives in objective_and_sample_formats/sample_formats_user.yaml (next to the catalog it overlays - NOT machine_configs/, which describes the microscope hardware; gitignored, with a tracked .example): - overrides: SPARSE per-field edits of shipped formats (vendor pitch on the non-SLAS 6/12/24/48 formats, well size, number_of_skip, well_shape). Only the fields present are applied, so a future shipped correction reaches every machine except on exactly the field a lab changed - unlike the legacy cache, where one calibration froze all seven formats wholesale forever. - custom_formats: complete new formats, now expressible with per-axis spacing/ size and shape (chamber slides, slide carriers). Scalar-and-per-axis together is a validation error, not a precedence rule; a1 fields are excluded from overrides by construction (placement, not geometry). Wiring: load_formats() layers the file over the base after the CSV (identity when absent; damage is ignored loudly - import must never fail). The dialog's "Update Parameters" and "Add New Format" paths write the YAML instead of rewriting the legacy whole-table cache; "Calibrate Existing Format" (a1) still writes the cache until the placement sidecar lands in the next commit. Tests (9): absent-file identity; override touches only named fields and re-broadcasts scalars; both-set errors; anisotropic round-trip; unknown-format warn; damaged-file loud ignore; atomic save; the shipped .example parses. Regression: test_widgets 135, core+cache+def 432. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…with one-time cache migration Second half of the geometry/placement split, and the last commit of the refactor plan. Measured plate placement now lives in cache/plate_placement.yaml (per-load measured state - losing it costs one A1 touch), stored as DELTAS on the catalog's a1 so the catalog keeps sole ownership of the absolute origin and "absence of the file == identity" is literally true. - PlatePlacement model: a1_dx/dy_mm, optional rotation_deg (null => inherit; set => absolute total for this format - the per-format override the rotation design specifies), and a fit block holding RAW facts only (measured points, timestamp, note) - derived numbers are recomputed on load, never persisted. - plate_transform_for composes: catalog a1 + delta; rotation from the placement when measured; the legacy WELLPLATE_OFFSET is SUPPRESSED for any format with a placement entry, so exactly one offset is live per format and double-apply is unrepresentable. WellplateSettings.from_format carries the composed a1, so the navigation viewer and the planner agree. - _calibrate_existing_format writes the a1 delta (+ raw measured point + timestamp) to the placement file and the measured well size as a sparse geometry override; nothing rewrites the whole-table cache anymore. - One-time idempotent migration in load_formats(): a legacy cache/sample_formats.csv is split (a1 diffs -> placement deltas with provenance; spacing/size/skip diffs -> sparse overrides; unknown formats -> custom formats) and renamed to .migrated. Newer entries in the new stores are never clobbered; a cache parsing to zero formats is treated as damage (loud ERROR path) rather than migrated; migration failure keeps the legacy cache working as before. The safety property is asserted, not argued: composed coordinates equal the legacy cache's bit-for-bit (shipped + (calibrated - shipped) == calibrated), and the cache-resilience test flipped in this diff documents that a valid cache now migrates - catalog shows shipped, composition preserves the calibration. Tests: migration suite (8) + placement composition + offset suppression + flipped cache test. Regression per-file: core 386, models 13, cache+migration+ def 54, widget trio 10, server 41, test_widgets 135, HCS GUI 5, MultiPointController 9 - all passed. (Two intermediate MultiPointController failures were disk-full artifacts - the machine hit ENOSPC mid-run - and pass with space available.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The mathematical core of the rotation design (doc Step 2): fit a 4-parameter similarity in closed form, APPLY only the 3-parameter rigid part, surface the discarded scale as QC. Pure Python + numpy, no Qt, no IO. - Closed form theta = atan2(sum cross, sum dot): no SVD, structurally cannot return a reflection. The angle is rounded to 0.01 deg FIRST (SLAS moulding scatter makes more digits noise) and a1 = q_bar - R(theta_rounded) p_bar is derived from the rounded angle, so the two stored numbers are consistent. t_applied uses s = 1: the similarity translation with a rigid linear part would offset the whole plate by (s-1) R p_bar (test pins the distinction). - sigma-hat from the SIMILARITY residual (k = 4) - the only click-noise estimator unpolluted by the discarded scale; Monte Carlo test asserts unbiasedness across true scales 1.0 and 1.0006. - Predicted error per query well includes the discarded-scale bias term: sqrt(2 sigma^2/N + sigma^2 |x-p_bar|^2/S_pp + ((s-1)|x-p_bar|)^2); reported as RMS and p95 (~1.75x), never persisted - recomputed from raw points. - 6-param affine QC fit (never applied): per-axis fitted pitch - BOTH axes checked, unlike pymmcore-widgets' [0]-only bug - and the row-vs-column axis angle that reveals shear; falls back to isotropic values rather than fabricating when N or rank cannot support it. - Gates, thresholds derived not invented: spread G > 1.5 (sigma-free, from clicked indices); predicted error > half a well radius (catches one-pitch mis-clicks); mirror check via signed areas of the MEASURED points (the fitted similarity's det = s^2 >= 0 can never fire); pitch > 5% off nominal warns per axis; |rotation| > 1 deg warns with confirmation, never hard-fails. Tests (25): round trip incl. sign; skimage SimilarityTransform cross-check over 25 randomized poses; t_applied vs t_sim; mirror rejection; one-pitch mis-click rejected on all six formats; the canonical reachable corner rings pass every gate on all six formats (a gate nobody can satisfy is a bug); clustered wells rejected; wrong-plate pitch warns; large rotation warns; rounding-before-a1 consistency; MC sigma-hat unbiasedness; N=2 fits without fabricated QC; collinear rows fine for rigid; degenerate inputs raise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…olution machine_configs/plate_holder.yaml - one angle per machine, because the holder is bolted to the stage (it IS hardware) and the dominant yaw contributors (holder mount angle, stage axis non-orthogonality) are format-independent. Minimal schema per the design review: the answer (rotation_deg) plus the raw facts that make it auditable and re-fittable (measured.on/feature/points/ timestamp, optional reload_spread_deg). Derived numbers are recomputed from the points, never stored. No staleness counters - a rotation change cannot invalidate a measured A1 (rotation pivots ON A1), and re-mounting ends in the wizard, which handles overrides at write time. - Validation enforces the no-arbitrary-numbers rule: a nonzero rotation_deg without measured points is rejected, so a hand-typed or copied-from-example angle cannot validate. The tracked .example ships with rotation UNSET. - resolve_rotation_deg(format_): ONE branch, zero arithmetic between stored angles (override-with-inherit, the additive model was rejected as unidentifiable) - a format's measured placement override wins, else the holder record, else 0.0. Pitch-0 formats (glass slide, '0') always resolve to 0.0: with a 1x1 grid the only well IS the pivot. - plate_transform_for now carries the resolved rotation; an a1-only placement entry (rotation unset) still inherits the holder angle. Tests (8): provenance validation, round trip, absent==0, damaged-file loud ignore, the full inherit chain (none -> holder -> measured override, with a1-only entries still inheriting), glass-slide short-circuit, rotation pivots on A1 through the resolver, the example skeleton parses. Regression: core+models+cache+migration 453, test_widgets 135, HCS GUI 5. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e frame The background image is a NOMINAL drawing - wells on an unrotated grid - so under a measured rotation the viewer's linear mm<->pixel map would draw a planned FOV off the well it will actually image (H12 at 0.4 deg: ~10 px), and a double-click on a drawn well would navigate to the UNROTATED position. Both paths now pass through the plate frame: - forward (get_FOV_pixel_coordinates): stage -> R(-theta) about the composed a1 -> nominal anchor (a1_x_pixel). A planned well-center FOV lands on the well's drawn position. - inverse (new pixel_to_stage_mm, extracted from handle_mouse_click): pixel -> nominal plate mm -> R(+theta) -> stage. A click on a drawn well navigates to where that well actually sits. - rotation is resolved at CALL time via resolve_rotation_deg (no snapshot), once per batch in register/deregister_fovs_to_image (resolution reads the placement/holder YAMLs - per-FOV reads would be thousands per redraw); resolution failure logs loudly and draws with 0.00 deg. - The unrotated path keeps the legacy origin_pixel arithmetic VERBATIM (asserted bit-for-bit in the identity test); glass-slide samples never consult the holder. Tests (5): identity == legacy exactly; planned rotated FOVs land on drawn wells on all four 96-plate corners (and the legacy map provably misses by >5 px at H12); pixel->mm->pixel round trip under 0.4 deg; click on drawn H12 == transform's rotated center to 1e-9; glass slide ignores the holder. Regression: test_widgets 135, HCS GUI 5, core 411. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every region producer in ScanCoordinates filtered planned FOVs through
validate_coordinates and threw the failures away without a trace - a plate
seated near the travel edge, or a measured rotation pushing an edge well over
the limit, just quietly imaged fewer FOVs than the user selected. Dropping is
still correct (the stage cannot go there); silence is not.
- Every producer (add_region all three shapes + circle-center fallback,
add_flexible_region, add_flexible_region_with_step_size,
add_template_region, manual regions) now counts travel drops.
- _register_travel_drops records {region_id: dropped} in the new
ScanCoordinates.out_of_travel dict - queryable by the upcoming wizard/UI -
and logs a WARNING with the exact counts and the limits: "N of M planned
FOVs for region X fall outside the stage travel limits ... will image K".
- Entries are lifecycle-correct: re-adding a region fully in travel clears
its stale entry; remove_region and clear_regions purge. The flexible
region's out-of-bounds print() becomes a log warning.
Tests (8): counted drop on the travel-corner 3x3 (5 of 9 dropped, warning
text pinned), no entry when fully in travel, stale-entry clearing, purge on
remove/clear, flexible + step-size + template counts, manual-region warning.
Regression: core 424, live-scan-grid + scan-size, MultiPointController 9.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A saved coordinate file is a list of ABSOLUTE stage positions computed under
the placement (a1 + rotation) that was live at save time. Loading it after a
recalibration or a rotation change silently replayed stale positions - the
wells moved, the file did not. Now the file says what it was computed under,
and "Load Coordinates" says so out loud.
- control/core/coordinate_provenance.py (pure): one JSON comment line
("# squid-scan-coordinates v1 {...}") prepended to the CSV, recording
format, effective A1 (composed a1 + legacy offset), rotation_deg + source,
and the save time. write/read_scan_coordinates_csv own the file shape;
legacy unstamped files load exactly as before (no stamp -> no check).
- staleness_warning compares the stamp against the CURRENT resolution of the
stamped format: format mismatch, rotation change (> half the fit quantum),
A1 drift (> 1 um), or a vanished custom format each produce a plain-English
reason. The check WARNS and still loads - the user may know the plate has
not moved - via QMessageBox + log in both load_coordinates widgets; the
save path stamps every per-objective CSV.
Tests (8): stamped round trip (fields + no-warning on unchanged placement),
legacy unstamped passthrough, rotation change flagged with both angles, A1
drift flagged, sub-tolerance drift NOT flagged, format mismatch, vanished
custom format, garbage stamp degrades to the widget's normal validation
error. Regression: core+models 440, test_widgets 135, HCS GUI 5.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The holder-rotation mode's entire state machine as a pure-Python session (GUI design doc "Implementation mapping"): wells, touches, fit, gates, verify, save - unit-testable against the fit module, with the dialog to be a thin view on top. - Reference wells COMPUTED, never hardcoded: extreme reachable corners of the skip window under the CURRENT transform + travel limits (no clamp exists on the move path; a hardcoded AF48 on 1536 would command an out-of-limit move). Pinned against the design table: 96 -> A1/A12/H1/H12, 384 (skip=1) -> B2/B23/O2/O23, 1536 -> A1/A47/AE1/AE47. Any well re-nominatable (A1 may be empty/unreachable), with grid/travel/duplicate validation. - Per-well method follows well_shape, never asked: round -> 3 rim touches -> circumcenter (cancels touch-radius error; collinear touches rejected and dropped so the well can be re-touched); square -> ONE same-corner touch, corner chosen once and locked after the first recording (mixing corners would break the cancellation). The eyeballed-center variant is not representable here at all. - fit() recomputes fresh every call (quality numbers never cached, let alone persisted); 4 wells default, 3 the accepted fallback, 2 refused. The corner-method theorem is asserted to 1e-9: a constant same-feature offset cancels exactly out of the centered rotation estimate. - Verify: predicted_touch_mm (same-feature prediction, honest for both methods since the fit translation carries the same offset) and holdout_residual_um - which refuses wells used in the fit, keeping "the only number that is not a model" actually out-of-sample. - save(): reject-gates block with the gate copy; warn-gates require explicit confirmation; writes the minimal holder record (angle + raw fit points + feature + timestamp) and NOTHING else - the fitted translation dies here by design. Stale per-format rotation overrides are listed and cleared on request (a1 deltas survive). Tests (19): design-table reference rings, pitch-0 refusal, nomination validation incl. travel, both methods' touch counts, circumcenter recovery, collinear rejection, undo, constant-offset cancellation, round-method recovery with rim-radius QC, 3-well fallback / 2-well refusal, minimal-save (and nothing-else-written), mis-click save refusal, warn-confirm flow, override clearing with a1 preservation, hold-out zero-residual and fit-well refusal, status-line provenance chain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The one new piece of UI in the whole feature (GUI design doc): a third mode radio in WellplateCalibration, a thin view over HolderAlignmentSession - every decision the mode makes (reference ring, per-well method, gates, save rules) lives in the tested session, not in Qt code. - Entering the mode builds the session for the CURRENTLY loaded format: status card (current angle + provenance), method line driven by well_shape (square -> same-corner touch x4 with a corner picker that locks once recording starts; round -> 3 rim touches x4 with per-well fitted radius shown), the computed reference ring in editable well fields (re-nomination validated; invalid entries revert and warn). Glass slide disables the mode content with the design's copy. - Record captures the stage position into the session; the fit panel refreshes live once 3+ wells are measured: angle, scale QC, sigma, per-point residuals, predicted RMS/p95 at the named worst well, and every gate's message inline. Reject-gates disable Save (and Drive-to-test). - Verify between fit and save: Record Hold-out (measured residual at a well the fit never saw - refuses fit wells) and Drive to Test Well (moves to the predicted same-feature point at the worst well). - Save: warn-gates require TYPING the angle to confirm (a click-through dialog would defeat the gate); stale per-format rotation overrides are listed with an offer to clear; writes only the holder record. Deliberately not in this commit (needs hardware-owner input, flagged in the design doc): the backlash-controlled re-approach on Record (needs the real approach direction + backoff distance - inventing them would violate the no-arbitrary-numbers rule) and forced-brightfield-on-open. Tests (8, driving the dialog as an operator would): computed ring + square method on entry, full record->fit->save flow (holder file contents checked, status updates to "holder record"), mis-click disables Save with gate copy, nomination through the edit incl. revert-on-invalid, hold-out zero residual, drive-to-test moves to the prediction, glass-slide disabled content, round plate hides the corner picker. Regression: test_widgets 135, HCS GUI 5, widget trio 10, core+models 459. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…aphy Owner decision: the holder mode keeps old-widget parity - Record captures the current stage position exactly like Set Point always has, no motion choreography, no channel forcing. The backlash concern is closed by an instruction rather than motion code: approaching every well from the same direction makes the backlash error a CONSTANT offset across the touches, and the fit cancels constant offsets exactly (the same theorem as the one-corner-per-well method; translation is discarded in this mode regardless). The method label now says so. This buys what the re-approach choreography would have, with no invented backoff constants. Illumination likewise stays as the dialog has always behaved: inherit the live channel. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… the dialog Same action, same name: the per-well capture button is labeled "Set Point" exactly as the existing modes' buttons are (it has always behaved like them), and the hold-out capture follows as "Set Hold-out Point". Session copy about the corner lock reworded to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… review applied Reuse, simplification, efficiency, and altitude findings from four parallel review agents over e00aa69..HEAD, deduped and applied. No behavior changes except two deliberate ones flagged below. Efficiency (the big one): - control/models/yaml_store.py: ONE implementation of the guarded pydantic YAML load + tmp/fsync/replace atomic save that plate_placement.py and plate_holder.py each carried a copy of. Loads are stat-guarded ((mtime_ns, size, inode) signature): the parsed model is reused until the file changes, and callers get a deep copy so edit->save flows cannot poison the cache. This is NOT a snapshot cache - results still change the moment the file does - but it removes ~10-20 YAML parses/sec the viewer's per-tick rotation resolution was doing on the GUI thread, and the ~1500 parses that made entering holder mode on a 1536 plate a multi-second stall. - plate_transform_for no longer reads the placements file twice per call (resolve split into _resolve_rotation(placement) on the already-loaded object). - HolderAlignmentSession hoists the transform once per well scan instead of re-resolving per well; plate_fit computes each query well's lever arm once (single pass; G gate uses max of the same list). Reuse / single ownership: - rotate_deg() in plate_transform.py is now the ONE rotation leg: the transform's own rotated branches, both NavigationViewer paths, and the session all call it (theta==0 legacy paths stay verbatim; identity test still exact). predicted_touch_mm evaluates the fit through PlateTransform - a fit result IS a placement. - holder_alignment uses mosaic_utils.format_well_id; its stricter well-id parse is kept deliberately (mosaic's parser accepts interleaved "12A"). - widgets.py: holder_alignment/coordinate_provenance imported once at top (12 inline imports removed); the duplicated stamp-check block in both load_coordinates widgets is one _warn_if_coordinates_stale helper. - coordinate_provenance derives ROTATION_TOL_DEG from the fit quantum (hand-copied literal violated no-arbitrary-numbers) and gets the effective A1 from transform.well_center_mm(0,0) instead of composing it twice. Altitude (two deliberate behavior fixes): - NavigationViewer keeps self.format (catalog identity) alongside self.sample (display identity): rotation resolution no longer string-matches viewer sample aliases, and a future viewer-only alias cannot fall into the log-exception-per-redraw path. - HolderAlignmentSession.fit() queries only REACHABLE wells: worst_well doubles as the Drive-to-Test target, which must never command an out-of-limit move (the session's own docstring said so; one of two call sites forgot). - toggle_input_mode uses the three exclusive radio states (dead "and not is_holder" conjunctions removed); toggle_calibration_method can no longer unhide format-mode panels over the holder UI. - ScanCoordinates: 3 of 5 drop counters were derivable (planned - kept); all-dropped flexible regions no longer leave ghost out_of_travel entries for regions that were never created, and their warning names the cause. Tests: shared catalog_tree + design_travel_limits fixtures in conftest replace 5 copy-pasted fixture bodies; local qapp fixtures dropped for pytest-qt's. Full regression: core+models 459, test_widgets 135, HCS GUI 5, rotation/format files 98, MultiPointController 9, squid 114, MCP server 41. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The third and last hand-rolled copy of the guarded pydantic load + atomic tmp/fsync/replace save retires; load_user_sample_formats gains the stat- guarded parse cache for free (it runs at import via load_formats()). Behavior identical: absent -> None, damage -> the same loud message -> None. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WellplateCalibration.calculate_circle now delegates to the holder mode's circumcenter - previously the same dialog ran two circle-through-3-points implementations, and the retired np.linalg.solve version accepted near-collinear points and silently returned a wildly-off circle, where the shared solver rejects them with user-facing copy. calibrate()'s error path catches the solver's SessionError alongside the legacy LinAlgError and shows the same collinear-points message as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors wellplate/sample-format handling to make startup resilient to corrupt/truncated caches, introduces atomic persistence + one-time migration away from the legacy whole-table CSV cache, and lays foundational geometry/placement abstractions (including rotation) that unify well→stage coordinate computation across GUI, headless, and MCP/remote paths.
Changes:
- Make sample-format and sidecar YAML writes atomic; ensure import-time format loading never fatals on a damaged cache (falls back loudly to shipped geometry).
- Introduce split stores + migration: sparse user geometry overrides/custom formats (YAML) and measured placement deltas/rotation (YAML), replacing the legacy monolithic calibration cache.
- Add shared transform/fit/session utilities and wire them into key UI/server paths; expand tests to pin legacy equivalence and new behaviors (rotation/viewer mapping, out-of-travel surfacing, provenance stamps).
Reviewed changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| software/tests/control/test_wellplate_format_widget.py | Tests cancel→revert and single-emission format selection behavior. |
| software/tests/control/test_well1536_selection_widget.py | Pins 1536 selector geometry sourcing and planner-equivalence arithmetic. |
| software/tests/control/test_sample_format_migration.py | Tests legacy cache migration into placement + sparse overrides/custom formats. |
| software/tests/control/test_sample_format_cache.py | Tests damaged-cache fallback + atomic CSV writing + writer validation behaviors. |
| software/tests/control/test_out_of_travel_surfacing.py | Tests that out-of-travel FOV drops are recorded and warned about. |
| software/tests/control/test_navigation_viewer_rotation.py | Tests stage↔pixel mapping correctness under measured rotation and identity equivalence. |
| software/tests/control/test_holder_alignment_dialog.py | Drives holder-rotation calibration dialog flow end-to-end. |
| software/tests/control/test_custom_plate_image_lookup.py | Tests custom plate PNG naming convention and legacy underscore fallback. |
| software/tests/control/models/test_sample_format_config.py | Tests user-formats YAML schema, sparse overrides, and anisotropic custom formats. |
| software/tests/control/models/test_plate_holder.py | Tests holder rotation record validation + inheritance/override resolution chain. |
| software/tests/control/core/test_plate_transform.py | Tests transform identity, inverse, anisotropy, offset rules, and derived defaults. |
| software/tests/control/core/test_plate_geometry_oracle.py | Golden oracle pinning well→stage arithmetic across producer sites. |
| software/tests/control/core/test_plate_fit.py | Tests closed-form placement fit contract, QC gates, and cross-checks. |
| software/tests/control/core/test_holder_alignment.py | Tests pure session logic for holder-rotation measurement and persistence behavior. |
| software/tests/control/core/test_coordinate_provenance.py | Tests scan-coordinate CSV stamping, parsing, and staleness warnings. |
| software/tests/control/conftest.py | Adds fixtures for isolated catalog trees and pinned travel/offset assumptions. |
| software/objective_and_sample_formats/sample_formats_user.yaml.example | Documents sparse overrides + custom formats example schema. |
| software/machine_configs/plate_holder.yaml.example | Documents holder rotation record skeleton and provenance requirement. |
| software/control/widgets.py | Updates widgets for new settings payload, atomic persistence, migration-era flows, rotation mode UI, and provenance-stamped coordinates. |
| software/control/models/yaml_store.py | Adds shared guarded-load + atomic-save + stat-signature parse cache for YAML sidecars. |
| software/control/models/sample_format_config.py | Implements user sample-format overrides/custom formats YAML model + application. |
| software/control/models/plate_placement.py | Defines placement sidecar (A1 deltas + optional rotation override) persistence. |
| software/control/models/plate_holder.py | Defines holder rotation record persistence + provenance validation rules. |
| software/control/microscope_control_server.py | Routes MCP well parsing through shared resolver (offsets applied; unknown formats raise). |
| software/control/gui_hcs.py | Adapts connections to new WellplateSettings object signal payload. |
| software/control/core/scan_coordinates.py | Resolves geometry at compute time via resolver; adds out-of-travel accounting and loud warnings. |
| software/control/core/plate_transform.py | Introduces PlateTransform + resolver helpers + WellplateSettings payload object. |
| software/control/core/plate_fit.py | Adds similarity-fit + rigid-application placement fit with QC gates and diagnostics. |
| software/control/core/holder_alignment.py | Implements pure holder-alignment session logic with fit, gates, and save flows. |
| software/control/core/core.py | Updates NavigationViewer to resolve rotation live and map stage↔pixel through plate frame. |
| software/control/core/coordinate_provenance.py | Adds provenance stamp read/write utilities and staleness detection for CSV coordinates. |
| software/control/_def.py | Adds atomic CSV writer, derived-geometry defaults, legacy cache migration, and non-fatal import-time load behavior. |
| software/configurations/configuration_Squid+.ini | Removes dead/stale plate format keys from config template. |
| software/configurations/configuration_Squid+_Tucsen.ini | Removes dead/stale plate format keys from config template. |
| software/configurations/configuration_Squid+_Tucsen_Libra.ini | Removes dead/stale plate format keys from config template. |
| software/configurations/configuration_Squid+_Kinetix_LDI_XLight_Xeryon.ini | Removes dead/stale plate format keys from config template. |
| software/configurations/configuration_Squid+_H117_ORCA_LDI_Fluidics.ini | Removes dead/stale plate format keys from config template. |
| software/configurations/configuration_Squid+_H117_ORCA_Andor.ini | Removes dead/stale plate format keys from config template. |
| software/configurations/configuration_Squid+_Cicero_LDI.ini | Removes dead/stale plate format keys from config template. |
| software/configurations/configuration_HCS_v2.ini | Removes dead/stale plate format keys from config template. |
| .gitignore | Ignores user sample formats YAML in-repo path (local state). |
Suppressed comments (1)
software/control/widgets.py:14807
- Well1536SelectionWidget.update_current_cell() reconstructs a PlateTransform from cached widget fields and always applies WELLPLATE_OFFSET, bypassing plate_transform_for()'s resolver rules (placement deltas, per-format rotation, per-axis pitch, and offset suppression when placement exists). This can make the selector navigate to different stage coordinates than planners/viewer on calibrated/rotated/aniso setups. Use plate_transform_for(self.format) directly.
transform = PlateTransform(
a1_x_mm=self.a1_x_mm,
a1_y_mm=self.a1_y_mm,
pitch_x_mm=self.spacing_mm,
pitch_y_mm=self.spacing_mm,
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # The COMPOSED a1 (catalog + placement delta), so the navigation viewer | ||
| # and the planner agree; both compose through the same resolver rules. | ||
| placement = _placement_for(format_) | ||
| a1_x = s["a1_x_mm"] + (placement.a1_dx_mm if placement else 0.0) | ||
| a1_y = s["a1_y_mm"] + (placement.a1_dy_mm if placement else 0.0) |
There was a problem hiding this comment.
[Claude Code] Fixed in 5a82b4d - from_format now derives A1 via plate_transform_for(format_).well_center_mm(0, 0), so the signal payload obeys the full resolver rules (delta + legacy offset incl. suppression) and the viewer's registration/rotation pivot is correct on offset machines.
| self.comboBox.blockSignals(True) | ||
| try: | ||
| self.populate_combo_box() | ||
| index = self.comboBox.findData(format_id) | ||
| if index >= 0: | ||
| self.comboBox.setCurrentIndex(index) | ||
| finally: | ||
| self.comboBox.blockSignals(False) | ||
| if index >= 0: | ||
| self.wellplateChanged(index) | ||
| return index |
There was a problem hiding this comment.
[Claude Code] False positive - the block is try/finally with no except clause, so if populate_combo_box() or findData() raises, the exception propagates and the post-finally 'if index >= 0' is never reached. UnboundLocalError is unreachable.
| transform = PlateTransform( | ||
| a1_x_mm=self.a1_x_mm, | ||
| a1_y_mm=self.a1_y_mm, | ||
| pitch_x_mm=self.spacing_mm, | ||
| pitch_y_mm=self.spacing_mm, | ||
| offset_x_mm=WELLPLATE_OFFSET_X_mm, | ||
| offset_y_mm=WELLPLATE_OFFSET_Y_mm, | ||
| ) | ||
| x_mm, y_mm = transform.well_center_mm(row, col) |
There was a problem hiding this comment.
[Claude Code] Fixed in 5a82b4d - both selectors (this site and line 14803) now resolve plate_transform_for(self.format) at click time, picking up calibration deltas, measured rotation, per-axis pitch, and the offset-suppression rule. The oracle's selector pins keep their expected positions; only the offset patch target moved to control._def where click-time resolution reads.
| pitch = wellplate_settings["well_spacing_mm"] | ||
| transform = PlateTransform( | ||
| a1_x_mm=wellplate_settings["a1_x_mm"], | ||
| a1_y_mm=wellplate_settings["a1_y_mm"], | ||
| pitch_x_mm=pitch, | ||
| pitch_y_mm=pitch, | ||
| offset_x_mm=control._def.WELLPLATE_OFFSET_X_mm, | ||
| offset_y_mm=control._def.WELLPLATE_OFFSET_Y_mm, | ||
| ) |
There was a problem hiding this comment.
[Claude Code] Fixed in 5a82b4d - uses well_spacing_x_mm/well_spacing_y_mm with the scalar as fallback for caller-supplied dicts.
Four-agent review of the first cleanup's own commits (10eb406..HEAD), findings deduped and applied. The efficiency agent measured rather than guessed: the yaml_store deep-copy-per-hit costs ~70 us vs the ~3.2 ms parse it replaced and is LOAD-BEARING (override-clearing mutates loaded models before saving), so the cache design stands unchanged. Single owners (altitude): - _resolve_rotation(settings, placement) now owns the pitch-0 short-circuit; both public entry points route through it, so the provenance stamp and the planner cannot drift on when rotation applies. - circumcenter moves to plate_fit (pure geometry, PlateFitError); holder_alignment keeps a wrapper adding the wizard's rim-touch copy (session tests unchanged); the dialog catches PlateFitError - the wizard's exception type no longer reaches the A1-calibration error path, and the dead LinAlgError catch (no numpy left in that try) is gone. - _sync_method_panel() is the one owner of the method-panel visibility rule; the mode toggle and the method toggle both route through it, replacing the holder-mode early-return guard that would have needed extending per mode. - NavigationViewer._display_sample_for() owns the "4 glass slide is how an HCS machine draws a glass slide" aliasing (was written in 2 places + the inverse); pixel_to_stage_mm drops its redundant glass-slide special case - the resolver's pitch-0 rule already returns 0.0. Dead code and hoists (simplification): - core.py: unused `import math` after the rotate_deg adoption; lazy resolve_rotation_deg import folded into the existing top-level import. - plate_holder/plate_placement: dead logger plumbing (the only log call moved into yaml_store); all three model files hoist the yaml_store import to module top (no cycle - the lazy form falsely signaled one). - yaml_store: docstring describes the contract instead of narrating which files used to carry copies; cache keyed by abspath so identity is honest across chdir (tests chdir constantly; the inode signature was carrying it). - conftest hoists its fixture-body imports. Reuse: the two pre-existing sample-format fixtures (formats_tree, migration_tree) become shims over the shared catalog_tree - the two files the first pass's fixture consolidation missed. Skipped, with reasons: atomic-write skeleton consolidation into squid/ (cross-layer, touches acquisition_state's mkstemp variant - follow-up); viewer.format as a derived property (stored field keeps update_wellplate_settings authoritative); pure-rename alias fixtures; the image-space ROI rotation in core.py (not the plate frame rotate_deg owns). Regression: core+models+cache+migration+rotation files 501, test_widgets 135, HCS+widget trio 15, MultiPointController 9. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…closed Copilot review of 2026-08-16, three of four comments confirmed and fixed (the fourth - an UnboundLocalError claim in select_format_silently - is a false positive: the block is try/finally with no except, so the post-finally read is unreachable when the try raises). - WellplateSettings.from_format now derives A1 via plate_transform_for(format_).well_center_mm(0, 0): the EFFECTIVE A1 under the full resolver rules (catalog + placement delta, legacy offset incl. suppression). Composing only the delta left the navigation viewer's plate registration - and the rotation pivot - off by wellplate_offset on machines that set one. - Both well selectors (WellSelectionWidget.onDoubleClick, Well1536SelectionWidget) resolve plate_transform_for(self.format) at CLICK time instead of rebuilding a transform from cached catalog fields + always-applied offset: double-click navigation now sees calibration deltas, measured rotation, and per-axis pitch - it previously disagreed with planning on every calibrated or rotated format. The oracle's two selector pins keep their expected positions (uncalibrated formats are numerically unchanged); only the offset patch target moves to control._def, where click-time resolution actually reads. - ScanCoordinatesSiLA2.get_selected_well_coordinates uses per-axis spacing keys (scalar fallback for caller-supplied dicts): anisotropic user formats got wrong Y pitch through the SiLA2 path. Regression: core+models+cache+migration+viewer 485, test_widgets 135, server+widget trio+dialog+HCS 60. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…er format The owner's requirement - custom formats adjustable, inaccurate measurements correctable, every default editable - exposed a real gap: overrides applied BEFORE custom formats inserted, so an override naming a custom format was warned away and silently reverted on restart (the dialog's in-memory update masked it until then). - apply_user_sample_formats inserts custom formats first, then applies overrides: a user's own plate is as editable as a shipped one, from the dialog or a hand-written override. - The dialog's Update Parameters routes a custom format's edit INTO its custom_formats entry (one source of truth per custom format in the YAML) instead of shadowing it with an override; shipped formats keep the sparse override path. Test: override-on-custom applies, scalar re-broadcasts per-axis, untouched fields keep the definition. Regression: core+models+cache+migration 469, widgets 139. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner decision (2026-08-16): shipped formats are EXAMPLES. When a lab calibrates or edits a format - shipped or their own - the result is a COMPLETE definition that replaces the example wholesale. No sparse overrides, no separate placement sidecar, no deltas: one entry per format holding everything about that plate, including its measured A1. Why: users adjust their own custom formats, correct inaccurate measurements, and edit any default; A1 is partly a plate-model fact (vendors mold it differently), so it belongs with the definition. And the geometry file is the one users copy between machines - it should carry the whole plate. - sample_format_config: SampleFormatOverride + CustomSampleFormat collapse into one SampleFormat (complete definition + optional per-format rotation_deg). apply_user_sample_formats REPLACES the catalog entry; no per-field merging. Provenance is split in two - `measured` (the A1 touch) and `rotation_measured` (the multi-well fit) - because one block cannot serve two gestures without losing the angle's provenance, and rotation still refuses to exist without >= 2 points behind it. - control/models/plate_placement.py DELETED; cache/plate_placement.yaml is gone. plate_transform_for reads a1 from the settings table (the definition already replaced the example at load time) and suppresses the legacy wellplate_offset for formats whose definition carries `measured` - the same exactly-one-live-correction rule, rekeyed. - The three dialog write paths (Add New Format, Calibrate Existing Format, Update Parameters) collapse into ONE _save_format_definition(): every format is handled identically, which was the point. A translation-only touch preserves a previously measured rotation override; the holder wizard still clears stale overrides (now both the angle and its provenance). - Migration rewritten: each legacy cache row that differs from its shipped example becomes a complete definition (a1 absolute, `measured` when a1 was calibrated); rows identical to the example are dropped so they keep tracking future examples. Cache still renamed .migrated, still idempotent, still never clobbers newer entries, still leaves the cache working on failure. - The tracked .example documents the portability contract: this file is the one to copy; the holder record is what must not travel. Tests: user-formats suite rewritten around the single model (wholesale replacement, shipped-and-custom-handled-identically, is_measured edited-vs-calibrated, rotation-provenance rejection, from_settings round trip); migration suite rewritten (absolute a1 + provenance, offset suppression, geometry-edit-without-measured, identical-cache-writes-nothing); the cache test's expectation flips again - the loaded catalog now carries the calibrated a1 directly. Regression: core+models 462, widgets 135, GUI/dialog/ viewer 36, server+MPC 28. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… bugs Four-agent review of 5a82b4d..HEAD. Three findings were defects the merge introduced, not style; the rest is dedup. BUGS - Migration dropped well_shape. It hand-listed the legacy CSV's 10 columns instead of calling the SampleFormat.from_settings() added in the same commit, so a calibrated 384/1536 migrated to "circle" - and since a definition now replaces the example wholesale, the rectangle was gone for good (holder alignment would then ask for 3 rim touches instead of 1 corner). Now goes through the one dict->model converter, which also ends the shape divergence between the two writers (migration wrote scalars, the dialog per-axis). Pinned by test_migration_preserves_derived_geometry. - SiLA2 double-applied the legacy offset. Once a1 came from the settings dict, ScanCoordinatesSiLA2.get_selected_well_coordinates - which adds WELLPLATE_OFFSET unconditionally - was stacking it on top of a MEASURED a1 on every calibrated format. The suppression rule now rides in the settings dict as `a1_measured` and is applied through one helper (plate_transform.legacy_offset_for), so every site that builds a PlateTransform gets it without a second disk read. Cross-site test added to the geometry oracle, which previously never exercised a calibrated machine. - A v1 user file loaded as ZERO formats, silently. pydantic ignores unknown keys, so the intermediate schema this branch shipped would have discarded every definition without a word. load_user_sample_formats now refuses an unrecognized version through the same loud path as damage. EFFICIENCY (measured) - The rotation resolver deep-copied the whole format store to read two scalars: 97% of its cost, growing with every format a lab calibrates. yaml_store gains an opt-in copy=False read for hot read-only paths (edit-> save flows keep the defensive copy), used via load_user_sample_formats_readonly. resolve_rotation_deg with 5 calibrated formats: 140 us -> 14.3 us per call, at 10 Hz on the GUI thread. DEDUP - is_measured() had no production caller while the resolver inlined the same predicate; it is now SampleFormat.is_measured, surfaced through to_settings() as the rule's single expression. - update_existing_parameters' in-place table update was dead (the writer replaces the entry two lines later) - 11 lines and a duplicated literal. - _save_format_definition uses get_wellplate_settings (named error instead of KeyError: 'rows') and names the carried-measurement set once, so a future measurement kind is added in the model, not remembered per call site. - Stale prose from the collapse (placement sidecar/entry references in plate_transform and the migration test module docstring), the orphaned run_migration helper, unused imports. - test_migration_is_idempotent... no longer tested its name after the earlier rewrite; restored as test_migration_never_clobbers_a_newer_definition, which actually re-runs the migration over a newer definition. Skipped: merging MeasuredPoint/FormatMeasurement with the holder record's near-identical pair (cross-store model change, deserves its own commit); measurements-as-a-dict keyed by quantity (a redesign, worth doing when the third measurement kind lands); scalar/per-axis normalization in the model (the scalar is the hand-authoring ergonomic the .example advertises); test factory consolidation into conftest (~120 lines, mechanical, separate); wiring the wizard to write per-format rotation overrides (a feature - today nothing writes them, only clears them). Regression: core+models 464, widgets 135, GUI/dialog/viewer/server 55, MPC+def 42, migration+cache 22. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 40 changed files in this pull request and generated 7 comments.
Suppressed comments (1)
software/control/core/scan_coordinates.py:393
- The all-out-of-travel path has the same lifecycle hole as the overlap-based flexible planner: it does not record any drops and leaves an older region with this ID active. Clear the prior region and register all
Nx*NyFOVs as dropped so acquisition state and warning counts remain accurate.
else:
self._log.warning(f"Region {region_id!r} not added: every planned FOV is outside the stage travel limits.")
| The design rule (owner decision, 2026-08-16): **shipped formats are examples.** | ||
| The moment a lab calibrates or edits a format - shipped or their own - the | ||
| result is stored here as a COMPLETE definition that replaces the example | ||
| wholesale. There are no sparse overrides and no separate placement sidecar: | ||
| one entry per format, holding everything about that plate including its |
| well_spacing_mm: Optional[float] = Field(None, gt=0) | ||
| well_spacing_x_mm: Optional[float] = Field(None, gt=0) | ||
| well_spacing_y_mm: Optional[float] = Field(None, gt=0) |
| sim = (scale * (rot @ pt.T)).T + q_bar | ||
| ssr_sim = float(np.sum((q - sim) ** 2)) |
| class PlateHolder(BaseModel): | ||
| version: int = 1 | ||
| rotation_deg: float = 0.0 # + = CCW in the stage XY math frame; pivot = A1 |
| def parse_stamp(line: str) -> Optional[dict]: | ||
| if not line.startswith(STAMP_PREFIX): | ||
| return None | ||
| try: | ||
| stamp = json.loads(line[len(STAMP_PREFIX) :]) | ||
| return stamp if isinstance(stamp, dict) else None |
| def _as_arrays(points: Sequence[Tuple[float, float]]) -> np.ndarray: | ||
| a = np.asarray(points, dtype=float) | ||
| if a.ndim != 2 or a.shape[1] != 2: | ||
| raise PlateFitError("points must be (x, y) pairs") | ||
| return a |
| else: | ||
| self._log.info(f"Region Out of Bounds: {region_id}") | ||
| self._log.warning(f"Region {region_id!r} not added: every planned FOV is outside the stage travel limits.") |
Three conflicts, all where master's per-FOV-z work (#608, #618) touched the same lines as this branch's transform/provenance work. - scan_coordinates.py (SiLA2 well centers): master normalized every region center to a mutable [x, y(, z)] list; this branch replaced the inline arithmetic with transform.well_center_mm(). Kept both - the transform computes, and the result is stored as a list like every other center. - widgets.py load_coordinates (x2): master extracted load_coordinate_regions_from_dataframe (column validation + optional z column); this branch added stamped reads. Kept both - the stamped reader supplies the dataframe, master's shared loader owns validation and z, and the staleness warning still fires afterwards. The stamp is a leading comment line, so it passes through the z column untouched. - widgets.py save_coordinates: master rewrote it around _save_for_objective with parfocal z adjustment and coordinate_rows_for_save. Kept master's structure and swapped its bare df.to_csv for the stamped write, so saved coordinates still record the placement they were computed under. - tests/control/conftest.py: master moved the microcontroller cleanup fixture to tests/conftest.py (retiring the logging/patch imports); this branch added catalog_tree/design_travel_limits (needing os/shutil). Kept the imports each surviving fixture actually uses. Oracle expectations at the SiLA2 site now compare element-wise, since those centers are lists rather than tuples on master. Regression after merge: core+models 473, widgets 158 (master's new tests included), GUI/server/format suites 77, MPC+def+scan 57, squid 123. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The complete sample-format refactor (plan:
AI-docs/Squid/in-progress/2026-07-29-sample-format-refactor-plan.md) plus the start of the rotation feature it was built for. Each commit independently revertible. The well→stage formula that existed in six independent copies is now computed in exactly one module, and the monolithic calibration cache is split into geometry (catalog + sparse user overrides) and measured placement (deltas).The series
import control._defraised on a truncated cache).select_format_silently()kills the spurious index-0 emission that momentarily reconfigured the app for the wrong plate.==pins on every producer site, offset-parameterized, with the sites' disagreements documented as tests.PlateTransform+plate_transform_for— pure module; identity path==-identical to legacy arithmetic;well_index_atinverse;nominal()for display assets._parse_wellsnow appliesWELLPLATE_OFFSETlike every other site and stops inventinga1=0/spacing=9; unknown formats raise. Oracle expectation flipped in the same diff.nominal()— offset provably cannot leak (byte-identical PNGs under offset 0 vs 25 mm).WellplateSettingsobject replaces the 10-arg signal — slots can no longer silently truncate the payload.ScanCoordinates— a calibration edit or offset change now applies to planning without a signal re-emit; the oracle's snapshot-vs-live disagreement test flips to agreement.well_shapeas derived data — the 10-column CSV stays frozen; scalars broadcast; shape's name-based mapping becomes one code-side default.objective_and_sample_formats/sample_formats_user.yaml(sparse overrides + custom formats, incl. anisotropic carriers; tracked.example) andcache/plate_placement.yaml(a1 deltas + raw measured points; optional per-formatrotation_deg), with a one-time idempotent migration of the legacy cache. Composed coordinates equal the legacy cache bit-for-bit — asserted, not argued.Rotation (phase 2, started)
Closed-form placement fit (
control/core/plate_fit.py) — similarity fit, rigid-only application, angle rounded to 0.01° before a1 derivation, σ̂ from the similarity residual (k=4), predicted RMS/p95 including the discarded-scale bias term, per-axis pitch QC (both axes), and the gate set: spread G > 1.5, predicted error > half a well radius (catches one-pitch mis-clicks), signed-area mirror check on the measured points (the fitted similarity's det = s² can never fire), pitch > 5% warns, |rotation| > 1° warns-with-confirmation. 25 tests incl. skimage cross-check over randomized poses and Monte Carlo σ̂ unbiasedness across true scales.Holder rotation record (
machine_configs/plate_holder.yaml) — one angle per machine, minimal schema (the answer + the raw measured points; derived numbers recomputed, never stored), provenance required for a nonzero angle so a hand-typed or copied-example value cannot validate; tracked.exampleships with rotation unset.resolve_rotation_deg(): override-with-inherit in one branch, zero arithmetic between stored angles — measured placement override > holder record > 0.0; pitch-0 formats (glass slide) always resolve to 0. 8 tests covering the full inherit chain.Navigation viewer maps stage↔pixel through the plate frame — the background image is a nominal drawing, so forward (FOV boxes) applies R(−θ) about A1 and inverse (double-click) applies R(+θ); planned FOVs land on the wells they will image and clicks navigate to where wells actually sit. Unrotated path kept bit-for-bit legacy (asserted); rotation resolved at call time, once per batch redraw. 5 tests incl. pixel→mm→pixel round trip.
Out-of-travel FOVs dropped loudly — every region producer counts travel drops into
ScanCoordinates.out_of_travel(queryable by the upcoming wizard) and warns with exact counts, instead of quietly imaging fewer FOVs than selected. Lifecycle-correct (re-add/remove/clear purge). 8 tests.Provenance stamps on saved coordinate CSVs — one JSON comment line records format, effective A1, rotation + source, and save time; "Load Coordinates" warns (but still loads) when the placement has since changed, naming exactly what moved. Legacy unstamped files load as before. 8 tests.
HolderAlignmentSession— the wizard's entire state machine, pure Python: computed reference rings pinned to the design table (96 → A1/A12/H1/H12, 1536 → A1/A47/AE1/AE47 because AF48 is out of travel), method-by-well-shape (rim×3/circumcenter vs one same-corner touch), the constant-offset cancellation theorem asserted to 1e-9, hold-out verification that refuses fit wells, and a save that writes the minimal holder record and nothing else — the fitted translation dies there by design. 19 tests.Holder-rotation mode in the calibration dialog — third mode radio, a thin view over the session: live fit panel with gates inline, reject-gates disable Save, warn-gates require typing the angle, stale per-format overrides offered for clearing at save, hold-out + drive-to-test verification. 8 tests driving the dialog as an operator would.
Point capture keeps old-widget parity (owner decision): the button is labeled "Set Point" and captures the stage position exactly like the existing modes' Set Point.
Cleanup (post-review, 4-agent /simplify pass)
control/models/yaml_store.py, one shared guarded-load + atomic-save for the sidecar YAMLs with a stat-signature parse cache (re-parses only when the file changes; callers get deep copies). This removed 10–20 YAML parses/sec the viewer's per-tick rotation resolution was doing on the GUI thread, and the ~1,500 parses that stalled holder-mode entry on a 1536 plate. Also:rotate_deg()inplate_transformis now the only implementation of the rotation leg (viewer, wizard, and transform all call it; θ=0 legacy paths untouched, identity test still exact==);plate_transform_forno longer reads the placements file twice; the fit's quality query covers only reachable wells (worst_welldoubles as the Drive-to-Test target, which must never command an out-of-limit move); the viewer tracks catalog identity separately from display identity (no more sample-name string matching); out-of-travel bookkeeping is lifecycle-correct; five copy-pasted test fixtures became two shared conftest fixtures.yaml_store— the third and last hand-rolled copy of the load/save pattern retires; the import-time load gains the parse cache for free.calculate_circledelegates to the holder mode'scircumcenter; the retirednp.linalg.solveversion silently returned a wildly-off circle for near-collinear points, where the shared solver rejects them with the dialog's existing collinear-points message.Deferred to its own PR (noted in the design doc): the ScanCoordinates travel-filter chokepoint refactor — one
_finalize_regionowning filtering/counting/S-pattern/registration for all five planners, which also gives manual regions per-region drop attribution. The backlash concern is closed by an instruction, not motion code — approaching every well from the same direction makes backlash a constant offset, which the fit cancels exactly; illumination stays inherit-the-live-channel as the dialog has always behaved. Remaining: the bench work (A1 measurement, 8-reload stability experiment).User-visible changes
wellplate_offset(release-note)..migrated; positions are numerically identical.wellplate_formatdropdown that never did anything.Tests
~175 new tests across 16 files: cache resilience 13, migration 8, user-formats model 9, oracle 44 (two documented expectation flips), transform 20, plate fit 25, holder 8, viewer rotation 5, out-of-travel 8, provenance 8, session 19, dialog 8, format widget 4, 1536 selector 2, image lookup 4, MCP additions. Regression per-file: core+models 480, test_widgets 135, HCS GUI 5, rotation/format files 98, MultiPointController 9, squid 114, MCP server 41 — all passing.
Verification notes: run test files per-file — GUI-constructing files pass and then the process lingers (pre-existing teardown hang, reproduced on unmodified code). Prefix runs with
ulimit -c 0: test-kill core dumps filled the disk mid-verification (two resulting MultiPointController failures pass with space available).🤖 Generated with Claude Code