fix(tune): score the detected image, not the pre-detection original

[RNBecker-bien]

Evaluator._score_one_image called scorer.score_image(image, measurements) with the untouched original image object. apply_and_measure(inplace=False) runs the candidate pipeline on a copy internally but only returns the measurement DataFrame, discarding that copy - so the original image's objmap was never populated with detected objects. SupervisedScorer read that empty objmap every trial, matched zero predicted objects against every ground-truth object, and pinned the Region cost at 1.0 (worst possible) regardless of the sampled hyperparameters.

Split apply_and_measure back into apply + measure so the processed copy can be captured and scored instead. Processed copies are now kept alive for the whole evaluate() pass (in processed_keepalive) so their id()s stay distinct - letting them get garbage-collected mid-loop let CPython reuse a freed copy's address for the next one, colliding identities that per-image scorer state (and the rung-ladder memoization test) relies on being unique.

Also gates the Linux-only renameat2 ctypes probe in _metadata_migration.py behind sys.platform.startswith("linux"): ctypes.CDLL(None) is POSIX-only and raises TypeError (not OSError) on Windows, which previously crashed the whole phenotypic import there.

[Xander-git]

Verdict

The core fix is correct and worth landing. The mechanism it introduces to keep the tests green is not, and I would ask for that to change before merge.

The fix itself is right

apply_and_measure(image, inplace=False) applies the pipeline to an internal copy and returns only the DataFrame, so the copy carrying the detection is discarded. Scoring the caller's image therefore scored an object the candidate pipeline never touched. Splitting into apply plus measure so the copy can be captured is the correct repair, and the split preserves the defaults apply_and_measure was forwarding (reset=None, include_metadata=True), so there is no behavioral drift beyond the intended one.

I confirmed the bug empirically:

num_objects on original after apply_and_measure(inplace=False): 96
num_objects on the processed copy:                             552

One correction to the PR description, since it is the record of why this code changed. The body states the original image's objmap "was never populated" and that the scorer "matched zero predicted objects". On load_synth_yeast_plate() the fixture arrives with 96 objects already in its objmap, while the candidate detects 552. The scorer was reading a stale, pipeline-independent objmap, not an empty one. The user-visible consequence described (the Region term is invariant to the sampled hyperparameters) is correct; the stated mechanism is not, for any image that arrives pre-detected.

Blocking: processed_keepalive is a production memory leak serving a test assertion

_score_one_image now appends every processed copy to a list that lives for the whole evaluate() pass, so the entire calibration set is retained in processed form (rgb, gray, detect_mat and objmap all populated) on top of the originals the caller already holds. Measured on an 18-image pass over the 600x800 synthetic plate:

	peak traced allocation
with processed_keepalive.append	132.2 MB
with that one line removed	72.8 MB

That is roughly 3.3 MB retained per image at 0.48 megapixels, and it scales linearly with pixel count. A real plate scan is one to two orders of magnitude larger, so a normal calibration set puts this in the gigabytes, per trial, in a workload that is long-running and distributed across SLURM.

The stated justification does not hold. No shipped scorer keys state on id(image): SupervisedScorer keys ground truth by image.name, QCScorer reads only the measurement frame, and the only id() uses in tune/ are _pareto.py (trials) and _composite.py (cycle detection). The actual dependency is the test suite. With the append removed, tests/unit/tune/test_rung_ladder.py::test_memoization_scores_each_image_once_across_rungs failed in 2 of 3 runs and passed in the third, because _CountingScorer counts id() of an object that is now transient and CPython recycles the address.

A concrete alternative, verified: Image.name is settable and survives apply(inplace=False). So _PerImageScorer can stamp unique names on the fixtures and map by name, and _CountingScorer plus the len(scorer._seen) == 6 assertion in test_should_prune_short_circuits_to_partial_pruned_result actually want a call count, not a distinct-identity count. That removes the retention entirely and fixes a latently flaky test at the same time.

If the retention is kept for a reason I have not seen, the comment should say it exists for the test scorers rather than for "per-image scorer state", since the comment is what the next reader will act on.

Should fix: nothing pins the bug

The diff touches two source files and zero tests. Every SupervisedScorer mask-tier test calls score_image directly, and the Evaluator-level tests use QCScorer, which reads only the measurement frame. Nothing in the suite would have caught this, and nothing will catch its reintroduction. Asserting that the object the Evaluator hands the scorer carries the candidate's detection (for the synthetic plate, 552 objects rather than the fixture's 96) would pin it in a few lines.

Minor: the Windows change is correct but out of scope and overstated

Gating the renameat2 probe behind sys.platform.startswith("linux") is a genuine fix, and _RENAMEAT2 still resolves on Linux, so behavior there is unchanged. Two notes. It is unrelated to the tune bug and is not reflected in the title. And the claim that it "crashed the whole phenotypic import" on Windows is not accurate: _metadata_migration is not loaded by import phenotypic (it is reached through the lazy module __getattr__ in phenotypic.sdk_), so the TypeError hits the migration path only. Windows is covered by the nightly run-pytest-full lane rather than the PR lane, so this cannot be verified on the PR.

CI

Red, but not from this PR. cli-packaging and measurement-refinement fail on both 3.11 and 3.12; the measurement-refinement failure is a numeric array mismatch in measurement math, and d846ca4 fix(measure): use ConvexHull.volume for 2D convex area landed on main after this PR's base. The branch is 10 commits behind. Merging main and re-running should clear it.

Locally, tests/unit/tune/ against this PR's source gave 21 failed / 927 passed; the identical 21 fail on current main (optuna engine, journal backend, distributed finalize), so they are pre-existing. ruff check is clean on both files, and mypy reports nothing for either (the 407 errors it emits are all elsewhere in the repo).

---

Generated by Claude Code

[Xander-git]

Pushed f8d9670 addressing the blocking finding from my review, at the repo owner's request.

The retention is gone. processed_keepalive is removed from Evaluator.evaluate and _score_one_image. Peak traced allocation for an 18-image pass over the 600x800 synthetic plate drops from 132.2 MB back to 78.7 MB.

The test scorers are re-keyed on image.name, which is stable, settable, and carried over by apply(inplace=False), rather than on id() of an object that is now transient. test_memoization_scores_each_image_once_across_rungs was failing in roughly two runs out of three once the retention was removed; it is now deterministic across 5 clean runs with random ordering enabled.

Two tests pin the bug this branch fixes, which previously had none:

• test_scorer_receives_the_candidates_detection_not_the_input_objmap asserts the Evaluator hands the scorer the candidate's own detection, and that the shared calibration image is left pristine. It first guards that the fixture's pre-existing object count and the candidate's differ, so it cannot pass vacuously.
• test_processed_copies_are_released_between_images asserts no earlier copy is still reachable when the next one is scored.

Both were mutation-tested: reverting to apply_and_measure() fails the first, and reintroducing the retention fails the second.

Verification: ruff clean on all three files; mypy reports nothing for _evaluator.py; tests/unit/tune/ gives 21 failed / 929 passed, the same 21 pre-existing failures as current main (optuna engine, journal backend, distributed finalize) with the two new tests added; tests/unit/gui/tune/ and tests/integration/tune/ give 144 passed, 1 skipped.

Two items from the review are untouched and still yours to judge:

1. The PR description's account of the mechanism. The fixture arrives with 96 objects already in its objmap while the candidate detects 552, so the scorer was reading a stale, candidate-independent segmentation rather than an empty one. The user-visible consequence described is right; the mechanism is not.
2. CI will still be red from the base. measurement-refinement is a numeric mismatch in measurement math that d846ca4 fixed on main after this branch's base. Merging main in should clear it, but I left that to you rather than adding a merge commit to your branch.

---

Generated by Claude Code