diff --git a/README.md b/README.md index efc43d3..6cff9cd 100644 --- a/README.md +++ b/README.md @@ -200,12 +200,12 @@ What each side may rely on: `mirror_of`. - **Grouping is a proposal; pondie's verdict overrides it.** When pondie disagrees, it writes a revision: a complete `CoordinateParse` with `revision_of` set and - one `AnalysisVerdict` per original analysis (accept, relabel, split, merge, omit, each - with a reason and evidence). Accepted and relabelled analyses keep their keys. A split - or merge copies the claims and votes stored against the old keys to their replacements, - each copy noting where it came from; the revised version is kept unchanged as history - and left out of projections (`AnalysisVerdict` gives the rules). Region, - seed and target sets given only as definitions are relabelled `anchor` and become + one `AnalysisVerdict` per original analysis (accept, relabel, edit, split, merge, omit, each + with a reason and evidence). Accepted and relabelled analyses keep their keys. An edit + corrects a single analysis's points, so its key changes. An edit, split or merge copies the + claims and votes stored against the old keys to their replacements, each copy noting where it came from; the revised version is + kept unchanged as history and left out of projections (`AnalysisVerdict` gives the rules). + Region, seed and target sets given only as definitions are relabelled `anchor` and become CoordinateSets in the record, not Analyses with statistics. - **Uploads are asymmetric.** neurostore accepts a parse alone, a parse with a record, a record alone against a parse already stored (every verdict accept), or a revision with a diff --git a/neuroimaging-paper-parse/coordinates.yaml b/neuroimaging-paper-parse/coordinates.yaml index c0fb9ec..bb0be1d 100644 --- a/neuroimaging-paper-parse/coordinates.yaml +++ b/neuroimaging-paper-parse/coordinates.yaml @@ -503,6 +503,8 @@ classes: - accept, relabel: nothing moves. The key is unchanged, so everything stored against it applies to the revision as it stands. + - edit: everything is copied to the one analysis in `replaced_by`, whose points are + the corrected ones. - split: everything is copied to each analysis in `replaced_by`. - merge: everything stored against each merged analysis is copied to the one analysis they merge into. Two copies of the same field with the same value become @@ -530,7 +532,8 @@ classes: multivalued: true description: >- The keys in this revision that take the analysis's place: itself for accept and - relabel, the replacements for split and merge, none for omit. + relabel, the one corrected analysis for edit, the replacements for split and merge, + none for omit. role: any_of: - range: CoordinateRole diff --git a/neuroimaging-paper-parse/parse_enums.yaml b/neuroimaging-paper-parse/parse_enums.yaml index 08c3250..a5d084d 100644 --- a/neuroimaging-paper-parse/parse_enums.yaml +++ b/neuroimaging-paper-parse/parse_enums.yaml @@ -193,6 +193,11 @@ enums: A reviewed correction of the ingestion workflow's proposed role, which wins: the same points in a different role, such as a region of interest, a seed, a stimulation target where the proposal said result. Its key is unchanged. + edit: + description: >- + The same analysis, with its points corrected: a value misread, a sign dropped, a point + added to this analysis, removed or moved. Its key changes because its content did. + Reordering points is not an edit. split: description: The points belong to more than one analysis. The replacements have new keys. merge: diff --git a/src/study_schema/jsonschema/coordinate-parse.schema.json b/src/study_schema/jsonschema/coordinate-parse.schema.json index c58232a..b2c0431 100644 --- a/src/study_schema/jsonschema/coordinate-parse.schema.json +++ b/src/study_schema/jsonschema/coordinate-parse.schema.json @@ -25,7 +25,7 @@ }, "AnalysisVerdict": { "additionalProperties": false, - "description": "pondie's verdict on one analysis of the parse a revision revises, reached while extracting the whole record. The verdict overrides the original's proposal, grouping and role alike.\nWhat a verdict does to what is stored against the analysis -- claims, their evidence, and users' votes on them:\n- accept, relabel: nothing moves. The key is unchanged, so everything stored against\n it applies to the revision as it stands.\n- split: everything is copied to each analysis in `replaced_by`. - merge: everything stored against each merged analysis is copied to the one\n analysis they merge into. Two copies of the same field with the same value become\n one claim; with different values they stay competing claims, resolved like any\n other.\n- omit: nothing is copied. - add: nothing to copy; the analysis is new.\nEvery copy records the key and parse it was copied from, so a reviewer can tell a copied claim from one made against the revision. The revised parse and everything stored against it are kept, unchanged, as history: superseded rather than deleted, and left out of projections and studysets.", + "description": "pondie's verdict on one analysis of the parse a revision revises, reached while extracting the whole record. The verdict overrides the original's proposal, grouping and role alike.\nWhat a verdict does to what is stored against the analysis -- claims, their evidence, and users' votes on them:\n- accept, relabel: nothing moves. The key is unchanged, so everything stored against\n it applies to the revision as it stands.\n- edit: everything is copied to the one analysis in `replaced_by`, whose points are\n the corrected ones.\n- split: everything is copied to each analysis in `replaced_by`. - merge: everything stored against each merged analysis is copied to the one\n analysis they merge into. Two copies of the same field with the same value become\n one claim; with different values they stay competing claims, resolved like any\n other.\n- omit: nothing is copied. - add: nothing to copy; the analysis is new.\nEvery copy records the key and parse it was copied from, so a reviewer can tell a copied claim from one made against the revision. The revised parse and everything stored against it are kept, unchanged, as history: superseded rather than deleted, and left out of projections and studysets.", "properties": { "evidence": { "description": "The sentences of the paper the verdict rests on.", @@ -45,7 +45,7 @@ "type": "string" }, "replaced_by": { - "description": "The keys in this revision that take the analysis's place: itself for accept and relabel, the replacements for split and merge, none for omit.", + "description": "The keys in this revision that take the analysis's place: itself for accept and relabel, the one corrected analysis for edit, the replacements for split and merge, none for omit.", "items": { "type": "string" }, @@ -187,6 +187,7 @@ "enum": [ "accept", "relabel", + "edit", "split", "merge", "omit", diff --git a/src/study_schema/jsonschema/parsed-paper.schema.json b/src/study_schema/jsonschema/parsed-paper.schema.json index 93d755f..69a6bb9 100644 --- a/src/study_schema/jsonschema/parsed-paper.schema.json +++ b/src/study_schema/jsonschema/parsed-paper.schema.json @@ -25,7 +25,7 @@ }, "AnalysisVerdict": { "additionalProperties": false, - "description": "pondie's verdict on one analysis of the parse a revision revises, reached while extracting the whole record. The verdict overrides the original's proposal, grouping and role alike.\nWhat a verdict does to what is stored against the analysis -- claims, their evidence, and users' votes on them:\n- accept, relabel: nothing moves. The key is unchanged, so everything stored against\n it applies to the revision as it stands.\n- split: everything is copied to each analysis in `replaced_by`. - merge: everything stored against each merged analysis is copied to the one\n analysis they merge into. Two copies of the same field with the same value become\n one claim; with different values they stay competing claims, resolved like any\n other.\n- omit: nothing is copied. - add: nothing to copy; the analysis is new.\nEvery copy records the key and parse it was copied from, so a reviewer can tell a copied claim from one made against the revision. The revised parse and everything stored against it are kept, unchanged, as history: superseded rather than deleted, and left out of projections and studysets.", + "description": "pondie's verdict on one analysis of the parse a revision revises, reached while extracting the whole record. The verdict overrides the original's proposal, grouping and role alike.\nWhat a verdict does to what is stored against the analysis -- claims, their evidence, and users' votes on them:\n- accept, relabel: nothing moves. The key is unchanged, so everything stored against\n it applies to the revision as it stands.\n- edit: everything is copied to the one analysis in `replaced_by`, whose points are\n the corrected ones.\n- split: everything is copied to each analysis in `replaced_by`. - merge: everything stored against each merged analysis is copied to the one\n analysis they merge into. Two copies of the same field with the same value become\n one claim; with different values they stay competing claims, resolved like any\n other.\n- omit: nothing is copied. - add: nothing to copy; the analysis is new.\nEvery copy records the key and parse it was copied from, so a reviewer can tell a copied claim from one made against the revision. The revised parse and everything stored against it are kept, unchanged, as history: superseded rather than deleted, and left out of projections and studysets.", "properties": { "evidence": { "description": "The sentences of the paper the verdict rests on.", @@ -45,7 +45,7 @@ "type": "string" }, "replaced_by": { - "description": "The keys in this revision that take the analysis's place: itself for accept and relabel, the replacements for split and merge, none for omit.", + "description": "The keys in this revision that take the analysis's place: itself for accept and relabel, the one corrected analysis for edit, the replacements for split and merge, none for omit.", "items": { "type": "string" }, @@ -187,6 +187,7 @@ "enum": [ "accept", "relabel", + "edit", "split", "merge", "omit", diff --git a/src/study_schema/layouts.py b/src/study_schema/layouts.py index de41617..497421d 100644 --- a/src/study_schema/layouts.py +++ b/src/study_schema/layouts.py @@ -194,6 +194,13 @@ def check_revision(original: CoordinateParse, revision: CoordinateParse) -> list if verdict.key in judged: problems.append(f"{verdict.key} has more than one verdict") judged.add(verdict.key) + if verdict.verdict == "edit" and len(verdict.replaced_by or []) != 1: + problems.append(f"edit {verdict.key}: replaced_by must name exactly one key") + if verdict.verdict == "edit" and verdict.replaced_by == [verdict.key]: + problems.append( + f"edit {verdict.key}: replaced_by names the analysis itself; " + "unchanged content is an accept" + ) for key in verdict.replaced_by or []: if key not in after: problems.append( diff --git a/src/study_schema/models/paper_parse.py b/src/study_schema/models/paper_parse.py index 0620df0..39ef89e 100644 --- a/src/study_schema/models/paper_parse.py +++ b/src/study_schema/models/paper_parse.py @@ -296,6 +296,10 @@ class AuditVerdict(str, Enum): """ A reviewed correction of the ingestion workflow's proposed role, which wins: the same points in a different role, such as a region of interest, a seed, a stimulation target where the proposal said result. Its key is unchanged. """ + edit = "edit" + """ + The same analysis, with its points corrected: a value misread, a sign dropped, a point added to this analysis, removed or moved. Its key changes because its content did. Reordering points is not an edit. + """ split = "split" """ The points belong to more than one analysis. The replacements have new keys. @@ -1608,6 +1612,8 @@ class AnalysisVerdict(ConfiguredBaseModel): What a verdict does to what is stored against the analysis -- claims, their evidence, and users' votes on them: - accept, relabel: nothing moves. The key is unchanged, so everything stored against it applies to the revision as it stands. + - edit: everything is copied to the one analysis in `replaced_by`, whose points are + the corrected ones. - split: everything is copied to each analysis in `replaced_by`. - merge: everything stored against each merged analysis is copied to the one analysis they merge into. Two copies of the same field with the same value become one claim; with different values they stay competing claims, resolved like any @@ -1617,7 +1623,7 @@ class AnalysisVerdict(ConfiguredBaseModel): """ key: str = Field(default=..., description="""The revised parse's analysis key; for `add`, the new analysis's key in this revision.""") verdict: AuditVerdict = Field(default=...) - replaced_by: Optional[list[str]] = Field(default=None, description="""The keys in this revision that take the analysis's place: itself for accept and relabel, the replacements for split and merge, none for omit.""") + replaced_by: Optional[list[str]] = Field(default=None, description="""The keys in this revision that take the analysis's place: itself for accept and relabel, the one corrected analysis for edit, the replacements for split and merge, none for omit.""") role: Optional[Union[CoordinateRole, str]] = Field(default=None, description="""For relabel, the role the points have instead.""") reason: str = Field(default=...) evidence: Optional[list[TextSpan]] = Field(default=None, description="""The sentences of the paper the verdict rests on.""") diff --git a/tests/test_layouts.py b/tests/test_layouts.py index 976d335..e022738 100644 --- a/tests/test_layouts.py +++ b/tests/test_layouts.py @@ -84,3 +84,40 @@ def test_a_different_text_is_caught(tmp_path): (tmp_path / STUDY / "processed" / "pubget" / "text.txt").write_text("edited") paper = PaperParse.read(tmp_path / STUDY / "parse") assert any("text_sha256" in p for p in check_paper(paper, tmp_path / STUDY)) + + +def test_an_edit_names_exactly_one_replacement(): + original, revision = _pair() + [split] = [v for v in revision.verdicts if v.verdict == "split"] + split.verdict = "edit" + assert any("edit" in p and "exactly one" in p for p in check_revision(original, revision)) + + +@pytest.mark.parametrize("replaced_by", [[], None]) +def test_an_edit_with_no_replacement_is_caught(replaced_by): + original, revision = _pair() + [split] = [v for v in revision.verdicts if v.verdict == "split"] + split.verdict = "edit" + split.replaced_by = replaced_by + assert any("edit" in p and "exactly one" in p for p in check_revision(original, revision)) + + +def test_an_edit_that_names_its_one_replacement_is_accepted(): + original, revision = _pair() + [split] = [v for v in revision.verdicts if v.verdict == "split"] + split.verdict = "edit" + kept, dropped = split.replaced_by + split.replaced_by = [kept] + revision.analyses = [a for a in revision.analyses if a.key != dropped] + assert check_revision(original, revision) == [] + + +def test_an_edit_that_names_its_own_key_is_refused(): + original, revision = _pair() + [split] = [v for v in revision.verdicts if v.verdict == "split"] + split.verdict = "edit" + split.replaced_by = [split.key] + assert ( + f"edit {split.key}: replaced_by names the analysis itself; unchanged content is an accept" + in check_revision(original, revision) + ) diff --git a/tests/test_models.py b/tests/test_models.py index 6b70dce..edcae06 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -179,3 +179,24 @@ def test_names_differing_only_in_whitespace_share_a_key(): assert analysis_key("PO > Sil", "MNI", points) == analysis_key(" PO>\u00a0Sil", "MNI", points) assert analysis_key("PO > Sil", "MNI", points) != analysis_key("PO < Sil", "MNI", points) assert analysis_key("PO > Sil", "MNI", points) != analysis_key("PC > Sil", "MNI", points) + + +def _with_edit_verdict(example_paths): + data = load(example_paths["revision"]) + [split] = [v for v in data["verdicts"] if v["verdict"] == "split"] + data["verdicts"] = [ + { + "key": split["key"], + "verdict": "edit", + "replaced_by": split["replaced_by"][:1], + "reason": "A minus sign was dropped from one point of the analysis.", + } + ] + return data + + +def test_an_edit_verdict_validates(example_paths): + data = _with_edit_verdict(example_paths) + assert CoordinateParse.model_validate(data).verdicts[0].verdict == "edit" + real = pytest.importorskip("jsonschema", reason="optional: JSON Schema validation") + real.validate(data, jsonschema.load("coordinate-parse"))