Skip to content

PP-10d: the notebook's masks-step button (Describe the masks) - #123

Merged
guijacquemet merged 7 commits into
mainfrom
claude/pp-10d-masks-step
Nov 2, 2026
Merged

guijacquemet merged 7 commits into
mainfrom
claude/pp-10d-masks-step

Conversation

@guijacquemet

@guijacquemet guijacquemet commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

PP-10d (owner decision of 2 November 2026: right after PP-10c, before PP-11). Design v0.4 §1 (recorded inputs, lock), §2 (masks), §9 code standards; contract §15 (the mask step). Plan §10 PP-10d.

Summary (by module)

  • run/mask_step.py (new, orchestration). Commit 2f1bf1f is a move only: the pairing with the recorded inputs, the scan and the write under the lock leave cli_masks.masks_command.
    • prepare_mask_step(experiment_root, output_folder, *, exclude=, masks_folder=, mask_axes=) returns a MaskStep: the preflight, the output folder, the input changes with their effect and the blocking issues, or the experiment_not_recorded refusal. MaskStep.ready is true when nothing is refused, there are no errors and at least one mask type exists.
    • describe_masks(step, *, thresholds=, progress=) returns a MaskStepResult (the report and the path written). Review (coordinator decision 1): the output folder's lock is taken before the first mask is read and held until the write. A locked folder returns the output_locked refusal (report=None) before any mask is read. It raises ValueError for masks whose pixels cannot be used, as before.
    • Review (coordinator decision 2): recorded_masks(output_folder) replaces recorded_mask_types. It returns a RecordedMasks (output folder, experiment folder, masks folder, mask types, written_at) from a v2 manifest, or None without one.
    • All are exported from morphobricks_preprocessing.run.
  • cli_masks.py: only parses and prints. Its output is unchanged except in one case: a locked output folder is now refused before any progress line.
  • notebook_mask_step.py (new, ui): step 3's Describe the masks (optional) card, placed after the intensity checks.
    • The button is enabled when step 1's output folder holds a v2 manifest that recorded masks. That covers a run in this session and also an earlier run in a reopened notebook.
    • The help line names the run it describes: the output folder, when the run was written, the experiment folder, the masks folder and the mask types.
    • A click calls prepare_mask_step(None, output_folder), the same path as the CLI's masks --output. If the experiment or masks folder has moved, the step is refused with the existing messages.
    • A progress bar runs per image. The results show:
      • a status card with the path written, "Thresholds: the defaults." and the bold suggestions sentence;
      • the tables Mask types / Relationships found (suggestions to check), then masks of the same objects and the hierarchy.
    • The UI only renders; the rows come from mask_text.
  • mask_text.py (new, pure): mask_summary, mask_report_summary and their helpers, moved unchanged from report.py. The review added an annotation to _hierarchy_lines, which removes its # noqa: ANN001. New here: mask_type_rows, mask_relationship_rows, mask_hierarchy_lines and mask_thresholds_used.
  • notebook_ui.py / notebook_run.py: the mixin and the card. folder_step(show_output=False) hides the card and never offers the step. _start_run clears and disables it while a run runs.
  • Docs: the notebook, the user guide (§4 step 3, §5 masks, §6 lock), contract §9.2 (the masks and Lock rows, and the Notebook mask step row) and §15.1 (the lock), plan §10, and AGENTS.md.

Behaviour changes

  • New: the notebook button.
  • CLI masks: with a locked output folder, the command is refused before any mask is read: no [i/n] progress lines, the same output_locked message and exit 1. Every other scenario is byte-identical to main.
  • API: run.MaskStep, MaskStepResult, RecordedMasks, prepare_mask_step, describe_masks and recorded_masks. report.mask_summary and report.mask_report_summary moved to mask_text (they were not top-level API).
  • The embedding API is unchanged. The click handler is private, and tests/test_embedding_api.py is untouched and passes.

Evidence

  • Move only (2f1bf1f): the reviewer reran the comparison independently. 23 CLI scenarios (stdout, exit status and mask_relationships.json without versions) ran on main 94fd6ac and on 2f1bf1f.
    • The scenarios covered HPMEC (everything given, recorded, recorded plus experiment given, default output, locked) and the fixtures: recorded inputs, relative paths, thresholds, a bad threshold, mask axes, bad mask axes, no record, nothing given, other masks, exclude, an unknown exclude, no-exclude, no masks, masks inside the experiment, a missing masks folder, labels above int32, a moved experiment and a moved masks folder.
    • The results were identical except for the lock holder's PID and time.
    • At the final head, the only change is the intended one: the locked scenario prints no progress lines.
  • HPMEC (MorphoRelate/data/hpmec_perfusion) in a reopened notebook: a fresh PreprocessingNotebook on an output folder written earlier by the CLI run.
    • The button was enabled, and the help line named that run.
    • Describe took 8.9 s and found 3 mask types (cells, mito, nuclei) in 2 images.
    • Its mask_relationships.json equals the CLI's.

Tests

  • Gate at 3f3b206: own worktree and uv venv, Python 3.11, .[dev,notebook,zarr].
    • ruff check and ruff format --check are clean, and tools/check.py --rules passes.
    • pytest: 1691 passed, 1 skipped. The skip is tests/test_discovery.py:665, file modes ignored when running as root.
    • The run includes the notebook-file tests (Colab and LabConstrictor metadata) and the headless nbclient test.
  • New tests: tests/test_mask_step.py (10) and tests/test_notebook_mask_step.py (14).
    • Lock regressions: the refusal reads no mask, the lock is held during every progress call, and the CLI prints no progress line when locked. These fail on the PR's original head.
    • Reopened-notebook regressions: the report equals the CLI's, the card follows step 1's output folder, and moved experiment and masks folders are refused. These fail on the original head too; the "no run" and "embedding brick" guards pass on both.
    • Everything fails on main, where run.mask_step is missing.

§9 figures (final head)

  • Files: notebook_mask_step.py 280 lines, run/mask_step.py 259, mask_text.py 177, cli_masks.py 109, report.py 436. The largest file touched is notebook_ui.py at 480.
  • Functions:
    • The longest new function is _build_mask_step_card (38 lines).
    • The longest touched is masks_command (44).
    • Complexity is at most 11 (masks_command).
    • Parameters: at most 5 in new code (prepare_mask_step); 6 in the touched folder_step, which is unchanged.
    • Statements: at most 31 (masks_command).
  • Layers are declared: run.mask_step is orchestration, mask_text pure, and notebook_mask_step ui.
  • Ratchet: the ANN # noqa count went 55 → 54 and the baseline was lowered. There is no new exemption.

Colab hand-check list

  1. Steps 1–3 with an experiment and masks folder on Drive. Before Compute histograms, the card shows a greyed button: "Available when the output folder holds a run that recorded masks".
  2. After the run, the help line says "Describes the run in … written …, of …: 3 mask types from …". The button is enabled.
  3. Click it: a spinner, a progress bar per image, then a green bar. The tables are labelled as suggestions, and mask_relationships.json appears in the output folder.
  4. Run step 3 again: the card clears.
  5. Reopen the notebook (Runtime → restart) and choose the same folders in step 1 without running. The button is enabled, names the earlier run, and works.
  6. Move the masks folder on Drive and click: a red card says the masks folder does not exist, and nothing is written.
  7. The tables scroll inside the card, with no horizontal page scrollbar.

🤖 Generated with Claude Code

https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc

…nly)

The pairing with the recorded inputs, the scan and the write of
mask_relationships.json under the output folder's lock move from
cli_masks.masks_command into run.mask_step (orchestration):
prepare_mask_step returns a MaskStep (preflight, output folder, input
changes, blocking issues, or the experiment_not_recorded refusal) and
describe_masks scans and writes, returning the output_locked refusal
instead of raising. The CLI only prints their results; its output is
unchanged (same text and exit codes over 13 scenarios, HPMEC included).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
After a run that recorded masks, step 3's "Describe the masks (optional)"
card runs the masks step with the run's recorded inputs through
run.mask_step (the functions the CLI calls), with a progress bar, and
shows the mask types, relationships with their counts, masks of the same
objects and the hierarchy as suggestions to check, with the path of
mask_relationships.json. Refusals use the existing cards (lock card with
the stale-lock button; red cards for mask errors and unusable masks).
Default thresholds only, said on the card. Hidden with
folder_step(show_output=False); embedding API unchanged (private
handler). run.mask_step.recorded_mask_types gates the button.

The masks' texts move from report.py to mask_text.py (pure) with the new
table rows, keeping report.py within 600 lines. Docs: notebook, user
guide, contract §9.2/§15.1, plan §10, AGENTS.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
Checked on the HPMEC data rendered in a browser: the headings sat on the
table above them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
@guijacquemet
guijacquemet marked this pull request as ready for review November 2, 2026 12:45
Coordinator decision 1: describe_masks takes the lock before the first
mask is read and holds it to the write of mask_relationships.json, so the
command line's masks and the notebook's Describe the masks refuse a locked
output folder (output_locked) before reading any mask. MaskStepResult's
report is None on that refusal. Regression tests: the refusal reads no
mask, the lock is held during every progress call, and the command prints
no progress line when locked. Docs: contract §9.2 and §15.1, user guide,
plan §10, the CLI module's notes; the notebook's lock card says no mask
was read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
…ned notebook

Coordinator decision 2: the button is enabled when step 1's output folder
holds a v2 manifest that recorded masks, not only after a run in this
session. run.mask_step.recorded_masks (replacing recorded_mask_types)
returns a RecordedMasks (output folder, experiment folder, masks folder,
mask types, when written); the help line names the run it describes, and
a click runs prepare_mask_step(None, output_folder), the path the CLI's
masks --output takes. The card follows step 1's output folder (re-read
when the manifest's fingerprint changes), is never offered with
show_output=False (folder_step refreshes it), and a moved experiment or
masks folder is refused with the existing messages under a title that no
longer says the masks have errors. Regression tests: reopened notebook
(report equal to the CLI's), output folder edits, moved experiment and
masks folders, no run, embedding brick. Docs: contract §9.2, user guide,
plan §10, AGENTS.md, notebook.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
mask_text._hierarchy_lines kept its # noqa: ANN001 through the move; §9
asks moved functions to be annotated. The ANN noqa ratchet shrinks 55 -> 54.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
@guijacquemet
guijacquemet merged commit 5044992 into main Nov 2, 2026
7 checks passed
@guijacquemet
guijacquemet deleted the claude/pp-10d-masks-step branch November 2, 2026 13:05
@guijacquemet
guijacquemet restored the claude/pp-10d-masks-step branch November 2, 2026 13:05

Copy link
Copy Markdown
Contributor Author

Independent review, then merged as merge commit 5044992 (main was 94fd6ac).

Findings and fixes (on the PR branch)

  • 12b2a89 (with an import sort in 959075b), coordinator decision 1:
    • describe_masks now holds the output folder's lock from before the first mask is read until mask_relationships.json is written. A locked folder is refused (output_locked, report=None) before any mask is read, in both the CLI and the notebook.
    • Regression tests: the refusal reads no mask, the lock is held at every progress call, and the CLI prints no progress line.
    • Docs updated: contract §9.2 and §15.1, user guide §4, §5 and §6, plan §10, and the CLI notes. The notebook lock card now says "No mask was read".
  • a4b8d53, coordinator decision 2:
    • Describe the masks follows step 1's output folder. It is enabled for any v2 manifest that recorded masks, including an earlier run in a reopened notebook.
    • run.mask_step.recorded_masks (a RecordedMasks) replaces recorded_mask_types. The help line names the run: output folder, when written, experiment folder, masks folder and types.
    • A click uses prepare_mask_step(None, output), the CLI's masks --output path. Moved experiment or masks folders are refused with the existing messages; the title no longer claims "the masks have errors".
    • With show_output=False, the step is never offered (folder_step refreshes it).
    • Six new widget tests plus one unit test. Seven fail on the original head; the no-run and embedding guards pass on both.
  • 3f3b206, §9 finding: the moved mask_text._hierarchy_lines kept a # noqa: ANN001. It is now annotated, and the ANN noqa ratchet goes 55 → 54 with the baseline lowered.

Verified

  • Move-only claim for 2f1bf1f: I wrote my own script with 23 CLI scenarios, covering HPMEC (5 variants) and fixtures. The fixtures included relative paths, mask axes and bad axes, an unknown exclude, a missing masks folder, nothing given, and moved experiment and masks folders.
    • main vs 2f1bf1f: identical (stdout, exit codes, JSON without versions) apart from the lock holder's PID and time.
    • main vs final head: the only difference is the intended one, the locked case without progress lines.
  • The card computes nothing: rows come from mask_text, which is pure. The suggestions are labelled ("suggestions to check" headings and the bold sentence).
  • Same output as the CLI: the notebook's mask_relationships.json equals the CLI's in the tests and on HPMEC in a reopened notebook (3 types: cells, mito, nuclei; 8.9 s).
  • Card behaviour: refusals use the existing cards (the lock card and red status cards), a new run clears the card, and show_output=False hides it.
  • tests/test_embedding_api.py is unchanged.
  • Gate (own worktree and uv venv, Python 3.11, .[dev,notebook,zarr]):
    • ruff check and format are clean, and tools/check.py --rules passes.
    • pytest: 1691 passed, 1 skipped (test_discovery.py:665, as root). This includes the Colab/LabConstrictor notebook-file tests and headless nbclient.
  • CI is green on 3f3b206.

§9 figures (touched files)

  • Largest file: notebook_ui.py, 480 lines. New files: notebook_mask_step.py 280, run/mask_step.py 259, mask_text.py 177.
  • Longest function: masks_command, 44 lines. The longest new one is _build_mask_step_card, 38.
  • Complexity: at most 11 (masks_command).
  • Parameters: at most 5 in new code (prepare_mask_step); 6 in the unchanged folder_step.
  • Statements: at most 31 in new or moved code.
  • Layers are declared: orchestration, pure, ui.
  • Ratchet: it only shrank.

The PR branch was auto-deleted on merge and has been restored at 3f3b206.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants