From 3019267b4eab0a0ae6875a423311be35742db21a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 8 Nov 2026 00:29:52 +0000 Subject: [PATCH 1/2] Write removed and image_count_changed records at publication Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Scma6W3LC11s3qjXHZuGDm --- CHANGELOG.md | 10 + pyproject.toml | 1 + .../run/description_file.py | 34 +- .../run/description_records.py | 229 ++++++++++++ tests/spec_gate.py | 1 + tests/test_spec_publication.py | 16 +- tests/test_spec_reconcile.py | 17 +- tests/test_spec_records.py | 343 ++++++++++++++++++ 8 files changed, 639 insertions(+), 12 deletions(-) create mode 100644 src/morphobricks_preprocessing/run/description_records.py create mode 100644 tests/test_spec_records.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 3674845a..7b177601 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ are in [docs/plan.md](docs/plan.md) §12. Upgrading from v0.3: see the ## Unreleased +- **Publication writes the `removed` and `image_count_changed` records (owner decisions RR2 and + RR1).** `publish_description` takes the user's `removals` (each with its confirmation date) and + `recorded_on`. Every unit the candidate drops must be named by a removal (else `ValueError`, + nothing written) and is appended to `morphobricks.records.jsonl` as a `removed` line with all + six keys; a condition, repeat or technical repeat whose ID the removal leaves held nowhere gets + its own line first (keys it does not name are `null`). A change of a confirmed member's + `image_count` alone writes `image_count_changed` with the old and new counts and `recorded_on` + (required then, never read from the clock). All lines go in the same single atomic append as + the re-confirmations, so the publication is whole or absent. + - **A run with a published experiment description (Stage 1, PR 11).** When the output folder holds a published `morphobricks.toml`, `run` reads it with the strict reader (a refusal is reported with its Spec code, table path and a plain message), runs the physical checks diff --git a/pyproject.toml b/pyproject.toml index 01255571..edeb9a02 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -257,6 +257,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.run.planned" = "orchestration" "morphobricks_preprocessing.run.design_file" = "io" "morphobricks_preprocessing.run.description_file" = "io" +"morphobricks_preprocessing.run.description_records" = "pure" "morphobricks_preprocessing.run.description_run" = "orchestration" "morphobricks_preprocessing.run.lock" = "io" "morphobricks_preprocessing.run.modes" = "pure" diff --git a/src/morphobricks_preprocessing/run/description_file.py b/src/morphobricks_preprocessing/run/description_file.py index bc438242..afc533c8 100644 --- a/src/morphobricks_preprocessing/run/description_file.py +++ b/src/morphobricks_preprocessing/run/description_file.py @@ -7,7 +7,9 @@ compares the candidate with the published file (:mod:`..description_transition`), and only then replaces the file atomically. Every refusal is reported as a set and writes nothing. -The user's re-confirmations that the candidate needed are appended, in the same publication, to +The user's re-confirmations that the candidate needed, a ``removed`` record for every removal +the user confirmed and an ``image_count_changed`` record for a changed image count +(:mod:`.description_records`) are appended, in the same publication, to the append-only ``morphobricks.records.jsonl`` next to the file (one JSON object per line, of one of the three Spec record types, each with its fixed key set). The records are never edited or dropped: the new file is the old bytes plus the new lines, written in full to a temporary file and @@ -40,6 +42,7 @@ read_file_bytes, sha256_hex, ) +from ..description_removal import Removal from ..description_report import DescriptionIssue, key_path from ..description_rules import Refusal from ..description_transition import ( @@ -50,6 +53,7 @@ unwaived, ) from ..description_write import canonical_bytes +from .description_records import image_count_records, removal_records from .lock import output_lock ABSENT = "absent" @@ -222,8 +226,11 @@ def _compare( published: bytes | None, candidate: Description, confirmations: tuple[Reconfirmation, ...], -) -> None: - """Refuse every ``R_*`` that applies, or a re-confirmation the candidate does not need.""" +) -> Description | None: + """Refuse every ``R_*`` that applies, or a re-confirmation the candidate does not need. + + Returns the published description (None when there was none), parsed once. + """ old = description_from_bytes(published) if published is not None else None issues = transition_issues(old, candidate) if old is not None else () left = unwaived(issues, confirmations) @@ -232,6 +239,7 @@ def _compare( if unused(issues, confirmations): names = ", ".join(f"{c.kind} {c.old_id}" for c in unused(issues, confirmations)) raise ValueError(f"a re-confirmation names a change the candidate does not make: {names}") + return old def publish_description( @@ -240,6 +248,8 @@ def publish_description( *, base_sha256: str | None, confirmations: tuple[Reconfirmation, ...] = (), + removals: tuple[Removal, ...] = (), + recorded_on: str | None = None, ) -> Publication: """Publish ``candidate`` as ``morphobricks.toml``, or refuse and write nothing. @@ -254,6 +264,14 @@ def publish_description( (a draft that recorded none) is ``R_DRAFT_BASE_MISSING``. confirmations The user's recorded re-confirmations of changed IDs; each must be needed. + removals + The user's confirmations of the removed units (each with its date). Every unit the + candidate drops must be named by one; each is written as a ``removed`` record, with the + records of the condition, repeat and technical repeat that no longer exist. + recorded_on + The date (``YYYY-MM-DD``) of an ``image_count_changed`` record: required when a + confirmed member's ``image_count`` is the only thing the candidate changes; never + defaulted and never read from the clock. Returns ------- @@ -271,7 +289,8 @@ def publish_description( DescriptionError When the candidate's canonical bytes, or the published file, is not a valid description. ValueError - When a re-confirmation is not needed by the candidate. + When a re-confirmation is not needed by the candidate, a dropped unit has no removal, a + removal names a unit the candidate keeps, or ``recorded_on`` is missing when needed. """ folder = Path(output_folder) path = folder / DESCRIPTION_FILENAME @@ -285,9 +304,14 @@ def publish_description( base = _check_base(base_sha256, published) data = canonical_bytes(candidate) description_from_bytes(data) # the exact bytes to be written pass the strict reader - _compare(published, candidate, confirmations) + old = _compare(published, candidate, confirmations) new_sha = sha256_hex(data) records = tuple(_record(c, base, new_sha) for c in sorted(confirmations, key=_order)) + if old is not None: + records += removal_records(old, candidate, removals, confirmations) + records += image_count_records(old, candidate, recorded_on) + elif removals: + raise ValueError("a removal needs a published file to remove from") write_text_atomic(path, data.decode("utf-8")) try: _append_records(folder, records) diff --git a/src/morphobricks_preprocessing/run/description_records.py b/src/morphobricks_preprocessing/run/description_records.py new file mode 100644 index 00000000..c3295a18 --- /dev/null +++ b/src/morphobricks_preprocessing/run/description_records.py @@ -0,0 +1,229 @@ +"""The ``removed`` and ``image_count_changed`` records of a publication (experiment-v1 4.6, 10). + +Both are derived by comparing the candidate with the published file it replaces, plus the user's +facts that no comparison can supply: the date of each removal confirmation +(:class:`..description_removal.Removal`) and the date the image-count change is recorded +(``recorded_on``). Nothing here reads the clock, and a date that is needed and not given is an +error, never a default. Pure: no file is touched. + +``removed`` (owner decision RR2). A published unit is removed when neither its source folders nor +its IDs (after the confirmed ID corrections) are in the candidate. Every removed unit is written +as a ``unit`` record naming all six keys. An entity whose ID the removal leaves held nowhere +(a condition, repeat or technical repeat) is written as its own record, before the unit records: +all entity records in the order condition, repeat, technical repeat (each by ID), then the unit +records by (condition ID, repeat ID, technical repeat ID). Labels are the published file's. + +``image_count_changed`` (owner decision RR1). Written for each member of a confirmed repeat whose +``image_count`` differs, only when nothing else differs between the published file and the +candidate. The members are in (condition ID, repeat ID) order. +""" + +from __future__ import annotations + +from collections.abc import Iterable +from dataclasses import replace +from datetime import date + +from ..description import Description, Member +from ..description_reconcile import PublishedUnit, UnitFolders, published_units +from ..description_removal import Removal +from ..description_transition import DATE, Reconfirmation + +_NONE = dict.fromkeys( + ( + "condition_id", + "condition_source_folder", + "repeat_id", + "source_repeat", + "technical_repeat_id", + "technical_repeat_source_folder", + ) +) + + +def _renames(confirmations: Iterable[Reconfirmation]) -> dict[str, dict[str, str]]: + """Return the confirmed ID corrections (old to new) of each kind.""" + found: dict[str, dict[str, str]] = {"conditions": {}, "repeats": {}, "technical_repeats": {}} + for item in confirmations: + if item.new_id is not None and item.kind in found: + found[item.kind][item.old_id] = item.new_id + return found + + +def _paths(description: Description) -> set[tuple[str, str, str]]: + return { + (repeat_id, condition_id, tid) + for repeat_id, repeat in description.repeats.items() + for condition_id, member in repeat.members.items() + for tid in member.technical_repeats + } + + +def _removed_units( + old: Description, new: Description, confirmations: Iterable[Reconfirmation] +) -> dict[UnitFolders, PublishedUnit]: + """Return the published units the candidate no longer holds, by source folder labels.""" + fix = _renames(confirmations) + kept_keys, kept_paths = set(published_units(new)), _paths(new) + gone = {} + for key, unit in published_units(old).items(): + path = ( + fix["repeats"].get(unit.repeat_id, unit.repeat_id), + fix["conditions"].get(unit.condition_id, unit.condition_id), + fix["technical_repeats"].get(unit.technical_id, unit.technical_id), + ) + if key not in kept_keys and path not in kept_paths: + gone[key] = unit + return gone + + +def removal_records( + old: Description, + new: Description, + removals: tuple[Removal, ...], + confirmations: tuple[Reconfirmation, ...] = (), +) -> tuple[dict, ...]: + """Return the ``removed`` records of the units and entities the candidate drops. + + Each takes the date of the first removal answer that names one of its units. + + Raises + ------ + ValueError + When a unit is dropped that no removal confirms, or a removal names a unit the + candidate keeps or the published file does not have. + """ + gone = _removed_units(old, new, confirmations) + asked = {key for removal in removals for key in removal.units} + loose = sorted(set(gone) - asked) + if loose: + names = ", ".join(f"{k.condition}/{k.source_repeat}/{k.technical}" for k in loose) + raise ValueError(f"a unit is removed and no removal confirms it: {names}") + extra = sorted(asked - set(gone)) + if extra: + names = ", ".join(f"{k.condition}/{k.source_repeat}/{k.technical}" for k in extra) + raise ValueError(f"a removal names a unit the candidate does not remove: {names}") + + def day(keys: Iterable[UnitFolders]) -> str: + held = set(keys) + return next(r.confirmed_on for r in removals if held & set(r.units)) + + fix = _renames(confirmations) + kept_tids = {tid for _, _, tid in _paths(new)} + entities: dict[tuple[int, str], dict] = {} + conditions = {u.condition_id: u for u in gone.values()} + repeats = {u.repeat_id for u in gone.values()} + tids = {u.technical_id: u for u in gone.values()} + for cid in conditions: + if fix["conditions"].get(cid, cid) not in new.conditions: + keys = [k for k, u in gone.items() if u.condition_id == cid] + entities[(0, cid)] = _row( + "condition", + day(keys), + condition_id=cid, + condition_source_folder=old.conditions[cid].source_folder, + ) + for rid in repeats: + if fix["repeats"].get(rid, rid) not in new.repeats: + keys = [k for k, u in gone.items() if u.repeat_id == rid] + entities[(1, rid)] = _row("repeat", day(keys), repeat_id=rid) + for tid, unit in tids.items(): + if fix["technical_repeats"].get(tid, tid) not in kept_tids: + keys = [k for k, u in gone.items() if u.technical_id == tid] + entities[(2, tid)] = _row( + "technical_repeat", + day(keys), + technical_repeat_id=tid, + technical_repeat_source_folder=unit.unit.source_folder, + ) + rows = [entities[key] for key in sorted(entities)] + for key, unit in sorted( + gone.items(), key=lambda kv: (kv[1].condition_id, kv[1].repeat_id, kv[1].technical_id) + ): + rows.append( + _row( + "unit", + day([key]), + condition_id=unit.condition_id, + condition_source_folder=key.condition, + repeat_id=unit.repeat_id, + source_repeat=key.source_repeat, + technical_repeat_id=unit.technical_id, + technical_repeat_source_folder=key.technical, + ) + ) + return tuple(rows) + + +def _row(entity: str, confirmed_on: str, **named: str) -> dict: + """Return a ``removed`` line: every key present, ``null`` where the entity names none.""" + return {"type": "removed", "entity": entity, **_NONE, **named, "confirmed_on": confirmed_on} + + +def _without_counts(description: Description) -> Description: + """Return the description with every member's ``image_count`` blanked, to compare the rest.""" + repeats = { + rid: replace( + repeat, + members={c: replace(m, image_count=0) for c, m in repeat.members.items()}, + ) + for rid, repeat in description.repeats.items() + } + return replace(description, repeats=repeats) + + +def image_count_records( + old: Description, new: Description, recorded_on: str | None +) -> tuple[dict, ...]: + """Return the ``image_count_changed`` records, or none when anything else differs too. + + Raises + ------ + ValueError + When a record is due and ``recorded_on`` is not a ``YYYY-MM-DD`` date (no default). + """ + if _without_counts(old) != _without_counts(new): + return () + changed = [] + for rid, repeat in old.repeats.items(): + if not repeat.confirmed: + continue + for cid, member in repeat.members.items(): + now = new.repeats[rid].members[cid].image_count + if now != member.image_count: + changed.append((cid, rid, member, now)) + if not changed: + return () + try: + if recorded_on is None or not DATE.fullmatch(recorded_on): + raise ValueError + date.fromisoformat(recorded_on) + except ValueError: + raise ValueError( + f"an image count changed: recorded_on is a real YYYY-MM-DD date, not {recorded_on!r}" + ) from None + return tuple(_count_row(old, *item, recorded_on) for item in sorted(changed, key=_by_ids)) + + +def _by_ids(item: tuple) -> tuple[str, str]: + return item[0], item[1] + + +def _count_row( + old: Description, cid: str, rid: str, member: Member, now: int, recorded_on: str +) -> dict: + """Return the ``image_count_changed`` line of one member (4.6).""" + return { + "type": "image_count_changed", + "condition_id": cid, + "condition_source_folder": old.conditions[cid].source_folder, + "repeat_id": rid, + "source_repeat": member.source_repeat, + "technical_repeats": [ + {"id": tid, "source_folder": unit.source_folder} + for tid, unit in sorted(member.technical_repeats.items()) + ], + "old_image_count": member.image_count, + "new_image_count": now, + "recorded_on": recorded_on, + } diff --git a/tests/spec_gate.py b/tests/spec_gate.py index 9efea8c9..150abb0a 100644 --- a/tests/spec_gate.py +++ b/tests/spec_gate.py @@ -38,6 +38,7 @@ "test_spec_reader.py", "test_spec_reader_phases_1_3.py", "test_spec_reconcile.py", + "test_spec_records.py", "test_spec_refval_crosscheck.py", "test_spec_rules_table.py", "test_spec_run_description.py", diff --git a/tests/test_spec_publication.py b/tests/test_spec_publication.py index 004f4c7e..bd4ad854 100644 --- a/tests/test_spec_publication.py +++ b/tests/test_spec_publication.py @@ -19,6 +19,8 @@ RECORDS_FILENAME, read_description, ) +from morphobricks_preprocessing.description_reconcile import UnitFolders +from morphobricks_preprocessing.description_removal import Removal from morphobricks_preprocessing.description_rules import Refusal from morphobricks_preprocessing.description_transition import Reconfirmation from morphobricks_preprocessing.description_write import canonical_bytes, section_digests @@ -170,10 +172,20 @@ def test_p20_removing_the_last_member_of_a_condition_needs_the_recorded_confirma lost = [("repeats", "rep1"), ("repeats", "rep2"), ("channels", "ACTIN"), ("channels", "DNA")] lost.append(("technical_repeats", "single")) removals = tuple(Reconfirmation(kind, i, None, DAY) for kind, i in lost) - done = publish_description(folder, candidate, base_sha256=base, confirmations=removals) + with pytest.raises(ValueError, match="no removal confirms"): # every removal is confirmed + publish_description(folder, candidate, base_sha256=base, confirmations=removals) + units = tuple(UnitFolders("DrugA", rep, "single") for rep in ("rep1", "rep2")) + done = publish_description( + folder, + candidate, + base_sha256=base, + confirmations=removals, + removals=(Removal(units, DAY),), + ) published = read_description(folder / DESCRIPTION_FILENAME) assert "DrugA" not in published.conditions and "DrugA" in published.used_ids["conditions"] - assert len(done.records) == 5 and {r["new_id"] for r in done.records} == {None} + assert len(done.records) == 5 + 3 # five re-confirmations, the condition, its two units + assert {r["new_id"] for r in done.records if r["type"] == "reconfirmation"} == {None} def test_p22_confirmed_on_stays_byte_for_byte_through_an_unrelated_publication(folder): diff --git a/tests/test_spec_reconcile.py b/tests/test_spec_reconcile.py index d2b56a17..f0cadb04 100644 --- a/tests/test_spec_reconcile.py +++ b/tests/test_spec_reconcile.py @@ -101,8 +101,11 @@ def test_p15_new_fovs_in_an_existing_unit_keep_the_confirmation_and_its_date(tmp } assert layout_changed(published, draft) == () output, base = published_in(tmp_path, published) - result = publish_description(output, draft, base_sha256=base) - assert result.records == () # the change is reported, and needs no re-confirmation + with pytest.raises(ValueError, match="recorded_on"): # the date is the user's, never defaulted + publish_description(output, draft, base_sha256=base) + result = publish_description(output, draft, base_sha256=base, recorded_on=DAY) + assert [r["type"] for r in result.records] == ["image_count_changed"] # no re-confirmation + assert (result.records[0]["old_image_count"], result.records[0]["new_image_count"]) == (10, 15) assert b'confirmed_on = "2026-10-30"' in (output / "morphobricks.toml").read_bytes() @@ -265,8 +268,10 @@ def test_p08_the_removal_is_one_grouped_question_and_publishes_only_when_answere found = reconfirmations(published, draft, rec, [answer]) assert found.open == () and {c.confirmed_on for c in found.accepted} == {DAY} assert len(found.accepted) == 5 - publish_description(output, draft, base_sha256=base, confirmations=found.accepted) - assert len(read_records(output)) == 5 + publish_description( + output, draft, base_sha256=base, confirmations=found.accepted, removals=[answer] + ) + assert [r["type"] for r in read_records(output)] == ["reconfirmation"] * 5 + ["removed"] assert (output / RECORDS_FILENAME).exists() @@ -287,7 +292,9 @@ def test_removing_a_condition_asks_once_for_every_lost_channel_and_technical_rep output, base = published_in(tmp_path, published) found = reconfirmations(published, draft, rec, [answer]) assert {(c.kind, c.old_id) for c in found.accepted} == {(k, i) for k, i, _ in question.needs} - publish_description(output, draft, base_sha256=base, confirmations=found.accepted) + publish_description( + output, draft, base_sha256=base, confirmations=found.accepted, removals=[answer] + ) again = replace(published, used_ids=draft.used_ids) assert Refusal.R_USED_ID_REUSED in {i.code for i in transition_issues(draft, again)} diff --git a/tests/test_spec_records.py b/tests/test_spec_records.py new file mode 100644 index 00000000..2d8ef8a0 --- /dev/null +++ b/tests/test_spec_records.py @@ -0,0 +1,343 @@ +"""The ``removed`` (RR2) and ``image_count_changed`` (RR1) records at publication (4.6, 10). + +The published file is a Spec fixture in a real output folder; the folder shows fewer units or +another count, the draft and the user's answers come from the real reconciliation, and the lines +are what ``publish_description`` appends to ``morphobricks.records.jsonl``, read back with +``read_records``. Dates are the user's: the confirmation date of a removal, ``recorded_on`` of a +count change. +""" + +from __future__ import annotations + +import json +from dataclasses import replace + +import pytest + +from morphobricks_preprocessing.description import description_from_bytes +from morphobricks_preprocessing.description_file import DESCRIPTION_FILENAME, RECORDS_FILENAME +from morphobricks_preprocessing.description_reconcile import ( + ObservedUnit, + UnitFolders, + confirm_repeat, + reconcile, +) +from morphobricks_preprocessing.description_removal import ( + Removal, + candidate, + reconfirmations, + removal_questions, +) +from morphobricks_preprocessing.run import description_file +from morphobricks_preprocessing.run.description_file import ( + ABSENT, + PublicationRefused, + publish_description, + read_records, +) +from spec_pin import fixtures + +BY_NAME = {item.name.split("_")[0]: item for item in fixtures("valid")} +DAY = "2026-11-09" +LATER = "2026-11-10" +NULLS = { + "condition_id": None, + "condition_source_folder": None, + "repeat_id": None, + "source_repeat": None, + "technical_repeat_id": None, + "technical_repeat_source_folder": None, +} + + +def model(name): + return description_from_bytes(BY_NAME[name].data) + + +def key(condition, source_repeat, technical="single"): + return UnitFolders(condition, source_repeat, technical) + + +def on_disk(description, changes=None, without=()): + seen = {} + for repeat in description.repeats.values(): + for condition_id, member in repeat.members.items(): + for unit in member.technical_repeats.values(): + folders = UnitFolders( + description.conditions[condition_id].source_folder, + member.source_repeat, + unit.source_folder, + ) + if folders not in without: + seen[folders] = ObservedUnit( + member.image_count // len(member.technical_repeats) + ) + seen.update(changes or {}) + return seen + + +def removal(published, gone): + """The draft, re-confirmations and answers when the folder loses the ``gone`` units.""" + rec = reconcile(published, on_disk(published, without=gone)) + answers = [question.answer(DAY) for question in removal_questions(published, rec)] + draft = candidate(published, rec, answers) + for repeat_id in rec.reopened: + if repeat_id in draft.repeats: # a repeat left without members is gone, not reopened + draft = confirm_repeat(draft, repeat_id, DAY) + found = reconfirmations(published, draft, rec, answers) + assert found.open == () + return draft, found.accepted, answers + + +def publish_first(folder, published): + return publish_description(folder, published, base_sha256=ABSENT).sha256 + + +def removed_lines(folder): + return [r for r in read_records(folder) if r["type"] == "removed"] + + +def snapshot(folder): + return sorted((p.name, p.read_bytes()) for p in folder.iterdir()) + + +def unit_row(condition, repeat, source_repeat, technical, folder=None, label=None, day=DAY): + return { + "type": "removed", + "entity": "unit", + "condition_id": condition, + "condition_source_folder": folder or condition, + "repeat_id": repeat, + "source_repeat": source_repeat, + "technical_repeat_id": technical, + "technical_repeat_source_folder": label or technical, + "confirmed_on": day, + } + + +# removed ------------------------------------------------------------------------------- + + +def test_a_removed_unit_writes_one_unit_record_naming_all_six_keys(tmp_path): + published = model("v03") # DrugA/experiment_23 is a member of Day1; DrugA stays in Day2 + base = publish_first(tmp_path, published) + draft, accepted, answers = removal(published, {key("DrugA", "experiment_23")}) + publish_description( + tmp_path, draft, base_sha256=base, confirmations=accepted, removals=tuple(answers) + ) + assert removed_lines(tmp_path) == [unit_row("DrugA", "Day1", "experiment_23", "single")] + kinds = [r["type"] for r in read_records(tmp_path)] + assert kinds == ["reconfirmation"] * len(accepted) + ["removed"] # one append, in this order + + +def test_a_removed_repeat_writes_the_repeat_then_each_of_its_units(tmp_path): + published = model("v01") + base = publish_first(tmp_path, published) + gone = {key("CTRL", "rep1"), key("DrugA", "rep1")} + draft, accepted, answers = removal(published, gone) + assert "rep1" not in draft.repeats and "rep1" in draft.used_ids["repeats"] + publish_description( + tmp_path, draft, base_sha256=base, confirmations=accepted, removals=tuple(answers) + ) + assert removed_lines(tmp_path) == [ + {**NULLS, "type": "removed", "entity": "repeat", "repeat_id": "rep1", "confirmed_on": DAY}, + unit_row("CTRL", "rep1", "rep1", "single"), + unit_row("DrugA", "rep1", "rep1", "single"), + ] + + +def test_a_removed_condition_writes_the_condition_then_each_of_its_units(tmp_path): + published = model("v01") + base = publish_first(tmp_path, published) + gone = {key("DrugA", "rep1"), key("DrugA", "rep2")} + draft, accepted, answers = removal(published, gone) + assert "DrugA" not in draft.conditions + publish_description( + tmp_path, draft, base_sha256=base, confirmations=accepted, removals=tuple(answers) + ) + assert removed_lines(tmp_path) == [ + { + **NULLS, + "type": "removed", + "entity": "condition", + "condition_id": "DrugA", + "condition_source_folder": "DrugA", + "confirmed_on": DAY, + }, + unit_row("DrugA", "rep1", "rep1", "single"), + unit_row("DrugA", "rep2", "rep2", "single"), + ] + + +def test_a_removed_technical_repeat_names_its_id_and_folder_then_its_unit(tmp_path): + published = model("v06") # acquisition_2 is held by one unit only + base = publish_first(tmp_path, published) + draft, accepted, answers = removal(published, {key("CTRL", "rep2", "acquisition_2")}) + publish_description( + tmp_path, draft, base_sha256=base, confirmations=accepted, removals=tuple(answers) + ) + assert removed_lines(tmp_path) == [ + { + **NULLS, + "type": "removed", + "entity": "technical_repeat", + "technical_repeat_id": "acquisition_2", + "technical_repeat_source_folder": "acquisition_2", + "confirmed_on": DAY, + }, + unit_row("CTRL", "R002", "rep2", "acquisition_2"), + ] + + +def test_the_lines_are_canonical_json_with_null_keys_and_pass_the_reader(tmp_path): + published = model("v01") + base = publish_first(tmp_path, published) + draft, accepted, answers = removal(published, {key("CTRL", "rep1"), key("DrugA", "rep1")}) + publish_description( + tmp_path, draft, base_sha256=base, confirmations=accepted, removals=tuple(answers) + ) + text = (tmp_path / RECORDS_FILENAME).read_text(encoding="utf-8") + assert text.endswith("\n") + repeat_line = next(line for line in text.split("\n") if '"entity": "repeat"' in line) + assert repeat_line == ( + '{"condition_id": null, "condition_source_folder": null, "confirmed_on": "2026-11-09", ' + '"entity": "repeat", "repeat_id": "rep1", "source_repeat": null, ' + '"technical_repeat_id": null, "technical_repeat_source_folder": null, ' + '"type": "removed"}' + ) + assert [json.loads(line)["type"] for line in text.split("\n")[:-1]] == [ + r["type"] for r in read_records(tmp_path) + ] + + +def test_the_date_of_each_record_is_the_users_confirmation_not_the_clock(tmp_path): + published = model("v03") + base = publish_first(tmp_path, published) + draft, accepted, _ = removal(published, {key("DrugA", "experiment_23")}) + answer = Removal((key("DrugA", "experiment_23"),), LATER) + accepted = tuple(replace(c, confirmed_on=LATER) for c in accepted) + publish_description( + tmp_path, draft, base_sha256=base, confirmations=accepted, removals=(answer,) + ) + assert [r["confirmed_on"] for r in read_records(tmp_path)] == [LATER] * 6 + + +def test_a_dropped_unit_without_a_removal_is_not_published(tmp_path): + published = model("v03") + base = publish_first(tmp_path, published) + draft, accepted, _ = removal(published, {key("DrugA", "experiment_23")}) + before = snapshot(tmp_path) + with pytest.raises(ValueError, match="no removal confirms"): + publish_description(tmp_path, draft, base_sha256=base, confirmations=accepted) + assert snapshot(tmp_path) == before + + +def test_a_removal_of_a_unit_the_candidate_keeps_is_not_published(tmp_path): + published = model("v03") + base = publish_first(tmp_path, published) + before = snapshot(tmp_path) + answer = Removal((key("DrugA", "experiment_23"),), DAY) + with pytest.raises(ValueError, match="does not remove"): + publish_description(tmp_path, published, base_sha256=base, removals=(answer,)) + assert snapshot(tmp_path) == before + + +def test_nothing_is_written_when_publication_is_refused(tmp_path): + published = model("v03") + base = publish_first(tmp_path, published) + draft, accepted, answers = removal(published, {key("DrugA", "experiment_23")}) + before = snapshot(tmp_path) + with pytest.raises(PublicationRefused): # the base is not the published file + publish_description(tmp_path, draft, base_sha256="0" * 64, removals=tuple(answers)) + with pytest.raises(PublicationRefused): # the changed IDs are not re-confirmed + publish_description(tmp_path, draft, base_sha256=base, removals=tuple(answers)) + assert snapshot(tmp_path) == before + assert not (tmp_path / RECORDS_FILENAME).exists() + + +def test_a_failed_records_write_puts_back_the_file_and_leaves_no_records(tmp_path, monkeypatch): + published = model("v03") + base = publish_first(tmp_path, published) + draft, accepted, answers = removal(published, {key("DrugA", "experiment_23")}) + before = snapshot(tmp_path) + + def broken(path, data): + raise OSError("disk full") + + monkeypatch.setattr(description_file, "_replace_file_durably", broken) + with pytest.raises(OSError, match="disk full"): + publish_description( + tmp_path, draft, base_sha256=base, confirmations=accepted, removals=tuple(answers) + ) + assert snapshot(tmp_path) == before + assert (tmp_path / DESCRIPTION_FILENAME).exists() and not (tmp_path / RECORDS_FILENAME).exists() + + +# image_count_changed ------------------------------------------------------------------- + + +def _counted(published, changes): + rec = reconcile(published, on_disk(published, changes)) + assert rec.reopened == {} + return candidate(published, rec) + + +def test_a_changed_image_count_writes_the_old_and_new_counts_with_recorded_on(tmp_path): + published = model("v06") # CTRL/rep2 has two technical repeats + base = publish_first(tmp_path, published) + draft = _counted(published, {key("CTRL", "rep2", "acquisition_1"): ObservedUnit(8)}) + result = publish_description(tmp_path, draft, base_sha256=base, recorded_on=LATER) + expected = { + "type": "image_count_changed", + "condition_id": "CTRL", + "condition_source_folder": "CTRL", + "repeat_id": "R002", + "source_repeat": "rep2", + "technical_repeats": [ + {"id": "acquisition_1", "source_folder": "acquisition_1"}, + {"id": "acquisition_2", "source_folder": "acquisition_2"}, + ], + "old_image_count": 9, + "new_image_count": 12, + "recorded_on": LATER, + } + assert read_records(tmp_path) == (expected,) == result.records + assert "confirmed_on" not in expected # never a confirmation + assert draft.repeats["R002"].confirmed and draft.repeats["R002"].confirmed_on == ( + published.repeats["R002"].confirmed_on + ) + + +def test_an_image_count_change_needs_a_date_and_writes_nothing_without_one(tmp_path): + published = model("v06") + base = publish_first(tmp_path, published) + draft = _counted(published, {key("CTRL", "rep2", "acquisition_1"): ObservedUnit(8)}) + before = snapshot(tmp_path) + for bad in (None, "09/11/2026", "2026-13-40"): + with pytest.raises(ValueError, match="recorded_on"): + publish_description(tmp_path, draft, base_sha256=base, recorded_on=bad) + assert snapshot(tmp_path) == before + + +def test_several_changed_members_are_written_in_condition_then_repeat_order(tmp_path): + published = model("v13") + base = publish_first(tmp_path, published) + draft = _counted( + published, + {key("DrugA", "rep2"): ObservedUnit(11), key("CTRL", "rep3"): ObservedUnit(12)}, + ) + publish_description(tmp_path, draft, base_sha256=base, recorded_on=DAY) + lines = read_records(tmp_path) + assert [(r["condition_id"], r["repeat_id"]) for r in lines] == [ + ("CTRL", "rep3"), + ("DrugA", "rep2"), + ] + assert [(r["old_image_count"], r["new_image_count"]) for r in lines] == [(10, 12), (10, 11)] + + +def test_an_unchanged_count_writes_no_record_and_needs_no_date(tmp_path): + published = model("v13") + base = publish_first(tmp_path, published) + draft = replace(published, schema_version=published.schema_version) + assert publish_description(tmp_path, draft, base_sha256=base).records == () + assert not (tmp_path / RECORDS_FILENAME).exists() From c9d7847480a26c42ea2d42d64e387a93b897259a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 8 Nov 2026 06:57:14 +0000 Subject: [PATCH 2/2] Write image_count_changed for every changed member; re-pin Spec to 2975e0e Owner decision RR3 (Spec PR 13): image_count_changed is written for every confirmed member whose image_count changed, also when the same publication changes other things. Members are matched through the confirmed ID corrections; a member the candidate no longer holds is covered by its removed records. Vendored Spec re-pinned to 2975e0e. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Scma6W3LC11s3qjXHZuGDm --- CHANGELOG.md | 12 ++++- docs/conformance.md | 2 +- .../draft_format.py | 2 +- .../run/description_file.py | 6 +-- .../run/description_records.py | 38 ++++++-------- tests/spec_fixtures/PIN.toml | 6 +-- tests/spec_fixtures/docs/experiment-v1.md | 22 +++++--- tests/spec_fixtures/fixtures/README.md | 3 +- tests/test_spec_records.py | 52 +++++++++++++++++-- 9 files changed, 99 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae580d9b..dc4badc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,14 +8,22 @@ are in [docs/plan.md](docs/plan.md) §12. Upgrading from v0.3: see the ## Unreleased +- **Re-pin the vendored Spec to 2975e0e (MorphoBricks-Spec PR 13, owner decision RR3).** Text only: the + `image_count_changed` record is written for every confirmed member whose count changed, also + beside other changes, and the order of the `removed` lines is stated; fixtures and the reference + validator are unchanged (the fixtures README alone differs). Every fixture is exact and the + cross-check against the reference validator has 0 differences. - **Publication writes the `removed` and `image_count_changed` records (owner decisions RR2 and RR1).** `publish_description` takes the user's `removals` (each with its confirmation date) and `recorded_on`. Every unit the candidate drops must be named by a removal (else `ValueError`, nothing written) and is appended to `morphobricks.records.jsonl` as a `removed` line with all six keys; a condition, repeat or technical repeat whose ID the removal leaves held nowhere gets its own line first (keys it does not name are `null`). A change of a confirmed member's - `image_count` alone writes `image_count_changed` with the old and new counts and `recorded_on` - (required then, never read from the clock). All lines go in the same single atomic append as + `image_count` writes `image_count_changed` with the old and new counts and `recorded_on` + (required then, never read from the clock), also when the same publication changes other + things (RR3). Entity lines come first, each unit line once after them; the date of an entity + is the first removal answer that touches it; with an ID correction the lines carry the + published IDs. All lines go in the same single atomic append as the re-confirmations, so the publication is whole or absent. - **`convert --project max|mean|sum [--z-range START:STOP]` (part of #176).** The command line of the recorded Z projection: no default method (`--project` alone or an unknown method is a diff --git a/docs/conformance.md b/docs/conformance.md index 16301a9c..07817735 100644 --- a/docs/conformance.md +++ b/docs/conformance.md @@ -8,7 +8,7 @@ real run; a skip is never counted as a pass. | | | |---|---| | Base commit | `68b2a4b` (`origin/main`, merge of PR 211); the run included the changes of PR 17 | -| Spec pin | the full SHA in `tests/spec_fixtures/PIN.toml` (vendored `experiment-v1.md` at `b197ac1`) | +| Spec pin | the full SHA in `tests/spec_fixtures/PIN.toml` (vendored `experiment-v1.md` at `2975e0e`) | | Command | `python tools/check.py` (Ruff, Ruff format, then `pytest -q` over the whole `tests/` folder), then `python tools/check.py --rules` | | Whole suite | 4324 passed, 4 skipped, 0 failed (301 s) | | Spec modules (`tests/test_spec_*.py`) | 1535 passed, 0 skipped, 0 failed (the Spec gate fails a skip or xfail there) | diff --git a/src/morphobricks_preprocessing/draft_format.py b/src/morphobricks_preprocessing/draft_format.py index 9b2705ef..f8dfb21b 100644 --- a/src/morphobricks_preprocessing/draft_format.py +++ b/src/morphobricks_preprocessing/draft_format.py @@ -33,7 +33,7 @@ DRAFT_FORMAT = "morphobricks-description-draft" DRAFT_VERSION = 1 -SPEC_COMMIT = "b197ac19d7a577cabe5281a81fa0fa59aef37146" +SPEC_COMMIT = "2975e0ee09d408d4a87da49d953f7988d4f9cd30" """The MorphoBricks-Spec commit the rules of this brick are pinned to (``tests/spec_fixtures``).""" BASE_MISSING = "R_DRAFT_BASE_MISSING" SHA256 = re.compile(r"[0-9a-f]{64}") diff --git a/src/morphobricks_preprocessing/run/description_file.py b/src/morphobricks_preprocessing/run/description_file.py index 48c7d654..f9158d66 100644 --- a/src/morphobricks_preprocessing/run/description_file.py +++ b/src/morphobricks_preprocessing/run/description_file.py @@ -269,8 +269,8 @@ def publish_description( candidate drops must be named by one; each is written as a ``removed`` record, with the records of the condition, repeat and technical repeat that no longer exist. recorded_on - The date (``YYYY-MM-DD``) of an ``image_count_changed`` record: required when a - confirmed member's ``image_count`` is the only thing the candidate changes; never + The date (``YYYY-MM-DD``) of the ``image_count_changed`` records: required when any + confirmed member's ``image_count`` changes, alone or with other changes; never defaulted and never read from the clock. Returns @@ -335,7 +335,7 @@ def publish_locked( records = tuple(_record(c, base, new_sha) for c in sorted(confirmations, key=_order)) if old is not None: records += removal_records(old, candidate, removals, confirmations) - records += image_count_records(old, candidate, recorded_on) + records += image_count_records(old, candidate, recorded_on, confirmations) elif removals: raise ValueError("a removal needs a published file to remove from") write_text_atomic(path, data.decode("utf-8")) diff --git a/src/morphobricks_preprocessing/run/description_records.py b/src/morphobricks_preprocessing/run/description_records.py index c3295a18..2867d9cf 100644 --- a/src/morphobricks_preprocessing/run/description_records.py +++ b/src/morphobricks_preprocessing/run/description_records.py @@ -13,15 +13,16 @@ all entity records in the order condition, repeat, technical repeat (each by ID), then the unit records by (condition ID, repeat ID, technical repeat ID). Labels are the published file's. -``image_count_changed`` (owner decision RR1). Written for each member of a confirmed repeat whose -``image_count`` differs, only when nothing else differs between the published file and the -candidate. The members are in (condition ID, repeat ID) order. +``image_count_changed`` (owner decisions RR1, RR3). Written for every member of a confirmed repeat +whose ``image_count`` differs, whether or not the publication changes anything else (a removal, a +new member, an ID correction, a reopening). Members are matched through the confirmed ID +corrections; a member the candidate no longer holds has no new count and is covered by its +``removed`` records. Lines carry the published IDs and labels, in (condition ID, repeat ID) order. """ from __future__ import annotations from collections.abc import Iterable -from dataclasses import replace from datetime import date from ..description import Description, Member @@ -160,38 +161,29 @@ def _row(entity: str, confirmed_on: str, **named: str) -> dict: return {"type": "removed", "entity": entity, **_NONE, **named, "confirmed_on": confirmed_on} -def _without_counts(description: Description) -> Description: - """Return the description with every member's ``image_count`` blanked, to compare the rest.""" - repeats = { - rid: replace( - repeat, - members={c: replace(m, image_count=0) for c, m in repeat.members.items()}, - ) - for rid, repeat in description.repeats.items() - } - return replace(description, repeats=repeats) - - def image_count_records( - old: Description, new: Description, recorded_on: str | None + old: Description, + new: Description, + recorded_on: str | None, + confirmations: Iterable[Reconfirmation] = (), ) -> tuple[dict, ...]: - """Return the ``image_count_changed`` records, or none when anything else differs too. + """Return the ``image_count_changed`` record of every confirmed member whose count changed. Raises ------ ValueError When a record is due and ``recorded_on`` is not a ``YYYY-MM-DD`` date (no default). """ - if _without_counts(old) != _without_counts(new): - return () + fix = _renames(confirmations) changed = [] for rid, repeat in old.repeats.items(): if not repeat.confirmed: continue + kept = new.repeats.get(fix["repeats"].get(rid, rid)) for cid, member in repeat.members.items(): - now = new.repeats[rid].members[cid].image_count - if now != member.image_count: - changed.append((cid, rid, member, now)) + held = kept.members.get(fix["conditions"].get(cid, cid)) if kept else None + if held is not None and held.image_count != member.image_count: + changed.append((cid, rid, member, held.image_count)) if not changed: return () try: diff --git a/tests/spec_fixtures/PIN.toml b/tests/spec_fixtures/PIN.toml index 937f2c25..f4edc117 100644 --- a/tests/spec_fixtures/PIN.toml +++ b/tests/spec_fixtures/PIN.toml @@ -1,13 +1,13 @@ # The MorphoBricks-Spec commit these files are copied from (conformance plan section 1). # Updating the pin is a reviewed change: see README.md in this folder. repository = "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/CellMigrationLab/MorphoBricks-Spec" -commit = "b197ac19d7a577cabe5281a81fa0fa59aef37146" +commit = "2975e0ee09d408d4a87da49d953f7988d4f9cd30" # `git rev-parse :fixtures`, the git tree-object ID of the fixtures/ folder. The # vendored folder must hash to it, so one changed, added or missing byte fails a test. -fixtures_tree = "5ac38b9fc91b0c461ee6585275e32b371029d4c0" +fixtures_tree = "6fafcda2a88ee57fba8683a07a215664bce1b1c3" # `git rev-parse :`, the git blob ID of each other vendored file. [files] -"docs/experiment-v1.md" = "f162c5dc9ca395838785c23e999650391b86f81f" +"docs/experiment-v1.md" = "3612d11857c655f459867c6426ffccb8d5b8c32f" "tools/audit/refval.py" = "de5a2cdb53ecb5bda7493c5b8cab2908e6e32fe6" diff --git a/tests/spec_fixtures/docs/experiment-v1.md b/tests/spec_fixtures/docs/experiment-v1.md index f162c5dc..3612d118 100644 --- a/tests/spec_fixtures/docs/experiment-v1.md +++ b/tests/spec_fixtures/docs/experiment-v1.md @@ -412,14 +412,23 @@ stays in the brick that owns it. record per imaging unit it held, and an entity that held units is followed by the `unit` records of those units, so every removed folder label appears in a record. Removing an entity never removes its ID from `used_ids` (§4.9). -- **Record of an image-count change (`image_count_changed`, owner decision RR1).** Written when - the `image_count` of a confirmed member changes and nothing else about the published file does - (§10); it needs no confirmation and reopens nothing. Keys: `type`, `condition_id`, +- **Record of an image-count change (`image_count_changed`, owner decisions RR1 and RR3).** + Written for **every** confirmed member whose `image_count` changes at a publication, including + a publication that also changes something else (a removal, a new member, an ID correction, + §10); the other changes do not suppress it and it suppresses none of their records. It needs no + confirmation and reopens nothing: a count change alone is not a reopening, and + `R_CONFIRMED_ON_CHANGED` (§4.4a) is unchanged. Keys: `type`, `condition_id`, `condition_source_folder`, `repeat_id`, `source_repeat`, `technical_repeats` (the member's imaging units as an array of objects `{"id", "source_folder"}`, sorted by `id` in code point order), `old_image_count`, `new_image_count` (integers, each at least 1) and `recorded_on` (`YYYY-MM-DD`, the date the change was recorded at publication; it is not a confirmation and is never copied into `confirmed_on`). +- **Order and content of the `removed` lines of one publication (RR2, clarified with RR3).** Each + vanished ID is one entity line; the `unit` lines of the units those entities held are written + once, after all entity lines of the publication, each unit once. An entity takes the + `confirmed_on` date of the first removal answer that touches it. When a removal is combined + with an ID correction in the same publication, the lines carry the IDs as published (before the + correction). - **A case-only correction needs a new ID (AQ2).** `ACTIN` cannot be corrected to `Actin`: the retired spelling stays in `used_ids` and the new one collides with it (`E_ID_CASE_COLLISION`, §4.9). The correction is a new, non-colliding ID (for example @@ -780,9 +789,10 @@ silently; edits go to a separate draft and are published atomically after valida channels, layout or grouping change: a new or removed unit or member, a unit's ordered channel list, or the pairing of folders into repeats. New FOVs (or removed ones) inside an existing unit whose confirmed layout still holds do **not** reopen it; they are reported and recorded. A - changed `image_count` of a confirmed member, when nothing else changes, is recorded as an - `image_count_changed` record (§4.6). A - changed `image_count` alone is therefore not a reopening: a new `confirmed_on` beside only a + changed `image_count` of a confirmed member is recorded as an + `image_count_changed` record (§4.6) whether or not anything else changes in the same + publication (RR3). A + changed `image_count` alone is not a reopening: a new `confirmed_on` beside only a changed `image_count` is `R_CONFIRMED_ON_CHANGED` (§4.4a). - **Adding a member is allowed (AQ1).** A new member folder or unit joins an existing repeat or condition without a changed-ID refusal (§4.6); it is a grouping change, so it reopens the N35 diff --git a/tests/spec_fixtures/fixtures/README.md b/tests/spec_fixtures/fixtures/README.md index 6430fa8f..27356a3f 100644 --- a/tests/spec_fixtures/fixtures/README.md +++ b/tests/spec_fixtures/fixtures/README.md @@ -215,7 +215,8 @@ This is a check of the fixtures against the text, not a conformance claim for an | AQ13 | Forbidden names apply to ID keys; NFC of channel references; one code per broken rule | `i113`-`i118` | | FR1 | Folder labels are never restricted by the reserved or forbidden names: conditions and technical repeats have a canonical ID separate from the required `source_folder`; one folder, one ID; a folder change for an existing ID is a reassignment; the derived CSV keeps the folder labels | `v33`, `v34`, `i113`, `i115`, `i120`-`i127`, `t10`-`t12`; P32, P33 | | FR2 | Derived CSV byte-identity is guaranteed only for repeat groups with a single `confirmed_on`; a legacy group with several dates gets the latest, the migration note keeps the per-row dates | `v13`; P30 | -| RR1 | Record type `image_count_changed` (a confirmed unit's image count changes and nothing else does; nothing reopens) | P34 | +| RR1 | Record type `image_count_changed` (a confirmed member's image count changes; nothing reopens) | P34 | +| RR3 | `image_count_changed` is written for every confirmed member whose count changed, also when the publication changes other things | P34 | | RR2 | Record type `removed` for every confirmed removal of a unit, technical repeat, repeat or condition | P35 | ## Valid fixtures (37) diff --git a/tests/test_spec_records.py b/tests/test_spec_records.py index 2d8ef8a0..ab5625ac 100644 --- a/tests/test_spec_records.py +++ b/tests/test_spec_records.py @@ -1,4 +1,4 @@ -"""The ``removed`` (RR2) and ``image_count_changed`` (RR1) records at publication (4.6, 10). +"""The ``removed`` (RR2) and ``image_count_changed`` (RR1, RR3) records at publication (4.6, 10). The published file is a Spec fixture in a real output folder; the folder shows fewer units or another count, the draft and the user's answers come from the real reconciliation, and the lines @@ -76,9 +76,9 @@ def on_disk(description, changes=None, without=()): return seen -def removal(published, gone): +def removal(published, gone, changes=None): """The draft, re-confirmations and answers when the folder loses the ``gone`` units.""" - rec = reconcile(published, on_disk(published, without=gone)) + rec = reconcile(published, on_disk(published, changes, without=gone)) answers = [question.answer(DAY) for question in removal_questions(published, rec)] draft = candidate(published, rec, answers) for repeat_id in rec.reopened: @@ -174,8 +174,17 @@ def test_a_removed_technical_repeat_names_its_id_and_folder_then_its_unit(tmp_pa base = publish_first(tmp_path, published) draft, accepted, answers = removal(published, {key("CTRL", "rep2", "acquisition_2")}) publish_description( - tmp_path, draft, base_sha256=base, confirmations=accepted, removals=tuple(answers) + tmp_path, + draft, + base_sha256=base, + confirmations=accepted, + removals=tuple(answers), + recorded_on=LATER, # the member's count falls with its unit: recorded too (RR3) ) + counted = [r for r in read_records(tmp_path) if r["type"] == "image_count_changed"] + assert [(r["condition_id"], r["old_image_count"], r["new_image_count"]) for r in counted] == [ + ("CTRL", 9, 4) + ] assert removed_lines(tmp_path) == [ { **NULLS, @@ -341,3 +350,38 @@ def test_an_unchanged_count_writes_no_record_and_needs_no_date(tmp_path): draft = replace(published, schema_version=published.schema_version) assert publish_description(tmp_path, draft, base_sha256=base).records == () assert not (tmp_path / RECORDS_FILENAME).exists() + + +def test_a_removal_and_a_count_change_in_one_publication_write_both_in_order(tmp_path): + """RR3: the count line is written although the same publication removes something.""" + published = model("v06") + base = publish_first(tmp_path, published) + gone = {key("CTRL", "rep2", "acquisition_2")} + draft, accepted, answers = removal( + published, gone, {key("DrugA", "experiment_23"): ObservedUnit(7)} + ) + assert draft.repeats["R001"].members["DrugA"].image_count == 7 + result = publish_description( + tmp_path, + draft, + base_sha256=base, + confirmations=accepted, + removals=tuple(answers), + recorded_on=LATER, + ) + lines = read_records(tmp_path) + assert lines == result.records + assert [(r["type"], r.get("entity")) for r in lines] == [ + *[("reconfirmation", None)] * len(accepted), + ("removed", "technical_repeat"), + ("removed", "unit"), + ("image_count_changed", None), + ("image_count_changed", None), + ] + counts = [r for r in lines if r["type"] == "image_count_changed"] + assert [(r["condition_id"], r["repeat_id"]) for r in counts] == [ + ("CTRL", "R002"), + ("DrugA", "R001"), + ] + assert [(r["old_image_count"], r["new_image_count"]) for r in counts] == [(9, 4), (5, 7)] + assert all(r["recorded_on"] == LATER and "confirmed_on" not in r for r in counts)