From 3e873e28ca218a11b2f05ad9fc4164996d0d9037 Mon Sep 17 00:00:00 2001 From: James Kent Date: Thu, 15 Oct 2026 14:59:19 -0500 Subject: [PATCH] Give a point-level correction its own coordinate verdict A reviewer who fixes an analysis's points (a dropped minus sign, a misread value, a point added to, removed from or moved out of it) changes its content key, but no verdict said "the same analysis, points corrected", so it could only be written as a split with one replacement. AuditVerdict gains edit: its key changes because its content did, and everything stored against it is copied to its one replacement, as for a split. check_revision refuses an edit that names no replacement, more than one, or itself (unchanged content is an accept). Point order is not part of the key, so reordering points is not an edit. Co-Authored-By: Claude Opus 5.5 --- README.md | 12 +++--- neuroimaging-paper-parse/coordinates.yaml | 5 ++- neuroimaging-paper-parse/parse_enums.yaml | 5 +++ .../jsonschema/coordinate-parse.schema.json | 5 ++- .../jsonschema/parsed-paper.schema.json | 5 ++- src/study_schema/layouts.py | 7 ++++ src/study_schema/models/paper_parse.py | 8 +++- tests/test_layouts.py | 37 +++++++++++++++++++ tests/test_models.py | 21 +++++++++++ 9 files changed, 93 insertions(+), 12 deletions(-) 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"))