Skip to content

Parent-label (grouped) masks: re-pin Spec 69a58b3, resolver contract v2 - #289

Merged
guijacquemet merged 3 commits into
mainfrom
claude/parent-label-masks
Nov 12, 2026
Merged

guijacquemet merged 3 commits into
mainfrom
claude/parent-label-masks

Conversation

@guijacquemet

Copy link
Copy Markdown
Contributor

Implements MorphoBricks-Spec PR #28 (merge 69a58b3e2f89b318364c4c1252c54caf676fb6bb, ADR 0001 PL1): representation = "parent_labels" in experiment-description v1, and resolver contract version 2 (ADR 0002). Branch from main at a775ebf.

Summary (by module)

  • Spec pin (tests/spec_fixtures/, draft_format.SPEC_COMMIT): re-vendored at 69a58b3 with the Spec's own tools/vendor.py. vendor.py --check says the folder is identical. The new fixtures are v38, v39, i136 to i139, and i111 is removed. The conformance counts are now 39 valid, 138 invalid and 69 in phases 1 to 3. v38 and v39 were added to the oracle mutation set. The new Spec module tests/test_spec_parent_labels.py is registered in spec_gate.EXPECTED_MODULES. It covers v38/v39 accepted, i136/i137 E_ENUM, i138/i139 E_UNKNOWN_FIELD, and i111 retired with its number not reused.
  • Reader and answers (description_rules.REPRESENTATIONS, mask_answers): parent_labels is now an accepted value (§4.5, §4.5b). In the notebook's words it is "grouped labels". The scan's grouped and Parent_Labels are still E_ENUM.
  • Mask check (§4.5a): no code change was needed. check_representation already reads distinct values for binary only, so a parent_labels file is recorded checked once it has been read, with no other condition. Tests show this on a v38 run with three values per file. They also show that the file is A_MASK_UNCHECKED before the mask step.
  • Resolver contract v2 (run_inputs_request, run_inputs_resolve; ADR 0002 "Resolver contract version 2"):
    • RESOLVER_VERSION = 2.
    • Capabilities.parent_labels_support (bool, False by default). A value that is not a bool is a ValueError.
    • objects: A_MASK_NOT_INSTANCES with or without the capability. The message was rewritten: it no longer suggests binary_mask_policy (the old text at run_inputs_request.py:~426). It now says the mask is never split, relabelled or converted, and to use it as a region.
    • binary_mask_policy on a parent_labels mask, in either role, is a ValueError whose message says the same.
    • region without the capability: MaskSelection(representation="parent_labels", conversion="parent_labels_as_area"). The marker is exported as PARENT_LABELS_AS_AREA.
    • region with the capability: no conversion.
    • instances and binary selections are unchanged.
  • Mask scan (mask_scan.piece_counts, mask_pairs.PIECE_STRUCTURE): pieces are now counted 8-connected, with generate_binary_structure(2, 2). This is the structure of CellPatcher's BINARY_MASK_CONNECTIVITY = 2 (checked at CellPatcher 2b98fa1, object_geometry.py:40). CellPatcher is not imported because preprocessing does not depend on it. mask_relationships.json goes to report version 4: this follows the repository's precedent of bumping the version when the meaning of a count changes (2 for the matching, 3 for ties). The constant lives in mask_pairs.py so that mask_scan.py stays within the 600-line code rule.
  • Preview (preview_render): a parent_labels mask is drawn as outlines coloured by value, so all pieces of one group share a colour.
  • Testing helper (testing_described._write_mask): a parent_labels mask is two groups of two separate squares each. Other representations are unchanged.
  • Messages: the action of R_MASK_CONTRADICTS_REPRESENTATION now names parent_labels beside instances (§4.5).
  • Docs: contract (resolver, §15.1, masks row), user guide, plan question 80, a conformance record, the AGENTS map and the CHANGELOG (Breaking: API resolver v2).

Notebook files touched (kept minimal, for the claude/redesign3-* merge)

  • src/morphobricks_preprocessing/notebook_mask_question.py: a third option, "grouped labels (one value per parent, e.g. mitochondria by cell)", the caption for it, and the docstring. There is still no default.
  • src/morphobricks_preprocessing/notebook_masks_screen.py: the intro text, the purpose line and docstrings.
  • src/morphobricks_preprocessing/notebook_check_save.py: one line ("separate objects, regions or grouped labels? (Masks)").

The question stays one column with no inner scroll box. Its only disabled control (made from) already shows its reason, and a test asserts all three.

Spec citations

experiment-v1 §4.5 (value, user-confirmed, never inferred), §4.5a (checked once read), §4.5b (identity, not instances, capability, use as region), §4.5c (no conversion for parent_labels), §7 (amendment of v1), §8 (A_MASK_NOT_INSTANCES). Also ADR 0001 PL1, ADR 0002 "Resolver contract version 2", and the conformance plan §2 "Mask fields" and §4 "Parent labels" rows.

Evidence

  • Each rule's test fails on main: the new tests were run against main's code with the new pin. 19 failed. They cover the pin, v38/v39, i138/i139, the answer, the mask check (two tests), resolver v2, objects with and without the capability, the message, the policy in both roles, the region with and without the capability, the bool check, and 8-connected pieces (test_pieces_are_8_connected_and_counted_per_label: 3 pieces become 1). Five tests pass on main because they guard existing behaviour: i136/i137 were already E_ENUM, grouped/Parent_Labels were already refused as answers, and i111 is absent once re-vendored. The notebook test fails on main because the option does not exist there.
  • Nothing else changed: make_described_run was run on v07 (two instances masks), v08 (instances + binary) and v13 (no masks) with main's and this branch's code, on the same fixture bytes. Every input TIFF, plane_histograms.csv, morphobricks.toml and experiment_design.csv is byte-identical. run_manifest.json, discovery_report.* and mask_representation.json differ only in mtime_ns, written_at, the absolute folder and the manifest SHA-256 derived from them. Two runs of main differ in exactly the same files and fields. After normalising those fields, the only difference is mask_relationships.json with "report_version": "3" → "4", from the 8-connected counting. Its counts and kinds are identical for these masks. No output records the Spec pin; only the draft file's spec_commit does, and no draft is written here.

Tests

python tools/check.py (Linux, Python 3.13.18, fresh venv .[dev,notebook,zarr]): Ruff clean, 443 files formatted, pytest 5396 passed, 4 skipped, 0 failed, code rules passed, exit 0.

The 4 skips are environment skips outside the Spec modules:

  • test_cli_log.py:166, test_folder_rules.py:199 and test_folder_rules.py:230: the file system is case-sensitive.
  • test_discovery.py:665: the tests run as root, which ignores file modes.

Spec modules tests/test_spec_*.py: 1796 passed, 0 skipped (25 modules).

Notes for review

  • I added the report_version bump for the mask scan, following the repository's precedent. The task did not ask for it. Nothing reads the report back (plan Q74 checked the consumers on 3 November).
  • The Spec's docs/pins.md row for preprocessing is not updated here, because the Spec is another repository.
  • describe (CLI) asks nothing about masks, so it has nothing to pre-fill and is unchanged.
  • I checked the widgets headless only. I did not look at them in a real JupyterLab.

Follow-ups


Generated by Claude Code

MorphoBricks-Spec PR #28 (merge 69a58b3, ADR 0001 PL1; experiment-v1 4.5, 4.5a-4.5c, 7, 8;
ADR 0002 resolver contract version 2).

- Re-vendor the Spec at 69a58b3 with its tools/vendor.py (--check identical); SPEC_COMMIT
  follows. Conformance counts follow (39 valid, 138 invalid, 69 phase 1-3); v38 and v39 join
  the oracle mutations; new module test_spec_parent_labels (v38/v39, i136-i139, i111 retired).
- representation = "parent_labels": reader enum, mask answers ("grouped labels"), the overlay
  (each group in its own colour). Never pre-filled from the scan's grouped kind. The mask step
  records such a file checked once read (4.5a, no other condition).
- Resolver contract v2: Capabilities.parent_labels_support; as objects A_MASK_NOT_INSTANCES
  with or without it, its message no longer suggests binary_mask_policy; a policy on it is a
  ValueError saying it is never converted; as a region the groups with the capability, else
  MaskSelection.conversion = "parent_labels_as_area" (PARENT_LABELS_AS_AREA).
- Mask step pieces counted 8-connected (CellPatcher's BINARY_MASK_CONNECTIVITY = 2 structure,
  not imported); mask_relationships.json report version 4.
- Notebook (minimal): Your masks offers "grouped labels (one value per parent, e.g.
  mitochondria by cell)"; question texts name the third answer.
- Docs: contract, user guide, plan question 80, conformance record, AGENTS map, CHANGELOG.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
The PR read the three flags before the isinstance check, so a request whose
capabilities were None, a dict or a bool raised AttributeError instead of the
contract's ValueError ("capabilities must be Capabilities of bool"). On main the
check short-circuited. Restore the order; regression test in
tests/test_spec_parent_labels.py fails before the fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
The section heading said report version 4 but the key table still said
`report_version` is 3. A test now ties the heading, the row and
MASK_REPORT_VERSION together; it fails before the fix.

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

Copy link
Copy Markdown
Contributor Author

Independent review

Verdict: correct against Spec 69a58b3 (experiment-v1 §4.5–4.5c, §7, §8; ADR 0001 PL1; ADR 0002 resolver contract v2). I fixed two defects in my own commits, each with a regression test that fails before the fix.

Fixes

  • 8aabcf1: _check_shape regression. The three capability flags were read before the isinstance check. Because of that, Request(capabilities=None) (or a dict, or a bool) raised AttributeError instead of the contract's ValueError("capabilities must be Capabilities of bool"). On main the check short-circuited. I restored that order. Test: test_capabilities_that_are_not_capabilities_are_a_value_error (3 cases, all failed before the fix).
  • 17c4ba7: contract §15.1. The heading said report version 4, but the report_version key row still said 3. I updated the row. A new test, test_the_contract_states_the_current_report_version, checks that the heading, the row and MASK_REPORT_VERSION agree; it failed before the fix.

Checked independently

  • Spec vendoring: vendor.py --check (Spec at 69a58b3) reports the copy identical. fixtures_tree = 3705f2b.
  • Leaks: this repo builds MaskSelection in exactly one place (run_inputs_resolve._selections). There is no path for a parent_labels mask as objects:
    • without a policy, it is refused as A_MASK_NOT_INSTANCES with or without the capability;
    • with a policy, check_request raises ValueError before resolving.
  • Region handling: as a region, a parent_labels mask comes back as parent_labels_as_area without the capability and with no conversion when the capability is declared. The declared representation is kept in both cases.
  • v1 behaviour for non-parent_labels masks: with the default Capabilities(), mask_conversion returns exactly the request's policy or None, the same as v1.
  • Outputs, main vs branch: I ran make_described_run on v07, v08 and v13, writing to the same path. After masking mtime_ns, written_at and the manifest SHA, the only difference is mask_relationships.json report_version 3→4, in v07 and v08 (v13 has no masks).
  • Tests on main: the branch's changed test files were run against main's src. Result: 23 failed, plus 2 modules that cannot be collected on main: test_spec_parent_labels.py (import of PARENT_LABELS_AS_AREA) and test_spec_reader_phases_1_3.py. The PR body says "19 failed". The figure is counted differently, but the conclusion holds: the new tests do fail on main.
  • 8-connected pieces and the version bump: the CHANGELOG says that corner-touching pieces now count as one piece and that the suggested kind can change, with the bump to report v4. I agree with the bump: it follows the v2/v3 precedent when a count's meaning changes.
  • Notebook: there is no pre-selection (choice.value is None even when the scan says grouped). The disabled made from control shows its reason. The question stays one column with no inner scroll box. The only notebook changes are text and the third option.

Gate (Linux, Python 3.13, -n 2)

  • Ruff: clean.
  • Code rules: pass.
  • pytest: 5400 passed, 4 skipped (environment skips: case-sensitive file system, running as root).
  • Spec modules: 1799 passed.
  • Two test_spec_gate tests failed only because my PYTEST_ADDOPTS=-n 2 leaked into their pytest subprocess. Rerun without it: 9/9 passed.
  • No processes were killed for lack of memory.

Generated by Claude Code

@guijacquemet
guijacquemet marked this pull request as ready for review November 12, 2026 02:11
@guijacquemet
guijacquemet merged commit a685711 into main Nov 12, 2026
7 checks passed
@guijacquemet
guijacquemet deleted the claude/parent-label-masks branch November 12, 2026 02:11
guijacquemet pushed a commit that referenced this pull request Nov 12, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
guijacquemet pushed a commit that referenced this pull request Nov 12, 2026
682e14e)

Not for merging: RELEASE_REF points at main as merged (#284-#287, with #289)
so the owner can test it in Colab. Only RELEASE_REF changes; cell 3 calls
PreprocessingNotebook().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
@guijacquemet guijacquemet mentioned this pull request Nov 12, 2026
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