refactor(casework): one evidence normaliser, not two copies

casework/README.md

@@ -197,17 +197,18 @@ against a bound case and left its `evidence` rows and their notes untouched.
 ### Writing `evidence` or `entities` — read, merge, send the whole list
 
 `PATCH /evidence` **replaces the entire list.** The server deletes every existing
-row and recreates from exactly what you send (`_write_material_references`,
-`cases/api_views.py:647`). Send only your new document and you delete the press
-release and court order someone bound last month.
+row and recreates from exactly what you send (`cases.api_views._write_material_references`).
+Send only your new document and you delete the press release and court order
+someone bound last month.
 
 Never send a delta. Read the case, merge into what is already there, send it all:
 
 ```python
-from casework.bind_materials import current_evidence, merge_evidence
+from casework.common.evidence import current_evidence, merge_evidence
 
 case, etag = api.get_case_with_etag(slug)
-merged = merge_evidence(current_evidence(case), [new_iri])   # append, keep order
+# (material_iri, note) pairs. Pass "" when the stage has no note to bind yet.
+merged = merge_evidence(current_evidence(case), [(new_iri, "")])
 api.replace_list(slug, "evidence", merged, if_match=etag)
 ```
 

casework/bind_materials.py

@@ -36,6 +36,7 @@
 from dataclasses import dataclass, field
 
 from casework.common.api import CaseworkApi
+from casework.common.evidence import current_evidence, merge_evidence
 from casework.common.cli import (
     _utc_iso_now, add_common_args, basic_auth_from_env, configure_run_logging,
     log_run_footer, log_run_header, print_summary,
@@ -121,44 +122,6 @@ def n_merged(self):
         return len(self.patch_items) if self.action == "WOULD_PATCH" else self.n_current
 
 
-def current_evidence(case):
-    """Normalize the case's evidence into the {material_iri, additional_details}
-    shape the PATCH expects, preserving order.
-
-    DELIBERATELY DUPLICATED in `casework/enrich_news_articles.py`. Both stages
-    write the same destructive whole-list `PATCH /evidence`, so both must
-    normalise it identically; the copy is kept rather than shared because these
-    are standalone scripts with no shared sequencing. Change one, change the
-    other -- a divergence here silently drops evidence rows.
-    """
-    return [
-        {"material_iri": e.get("material_iri"),
-         "additional_details": e.get("additional_details") or ""}
-        for e in (case.get("evidence") or [])
-        if e.get("material_iri")
-    ]
-
-
-def merge_evidence(current, add_iris):
-    """Append each new IRI not already present, preserving existing order and
-    de-duplicating. Never reorders or drops an existing entry -- the whole-list
-    replace makes any omission destructive.
-
-    DELIBERATELY DUPLICATED as `merge_news_evidence` in
-    `casework/enrich_news_articles.py`, which differs in ONE respect: it binds
-    the Nepali evidence note at the same time instead of `additional_details:
-    ""` (its deviation 2). The append-only contract is identical and must stay
-    that way in both.
-    """
-    have = {e["material_iri"] for e in current}
-    merged = list(current)
-    for iri in add_iris:
-        if iri not in have:
-            merged.append({"material_iri": iri, "additional_details": ""})
-            have.add(iri)
-    return merged
-
-
 def missing_candidates(case, candidates):
     """Return the ``(source, ident)`` candidates NOT already bound to ``case``.
 
@@ -213,7 +176,8 @@ def plan_case(api, case, etag, candidates, required_state=REQUIRED_STATE):
                         dropped=dropped, uncertain=uncertain, probes=probes,
                         reason=f"{len(uncertain)} material(s) uncertain")
 
-    merged = merge_evidence(current, add)
+    # This stage binds no note; the news stage passes a real one.
+    merged = merge_evidence(current, [(iri, "") for iri in add])
     if merged == current:
         return BindPlan(slug=slug, action="NOOP", state=state, if_match=etag,
                         n_current=len(current), dropped=dropped, probes=probes)

casework/common/evidence.py

@@ -0,0 +1,35 @@
+"""Read and merge a case's `/evidence` list. Shared by every stage that writes it."""
+
+
+def current_evidence(case):
+    """The case's evidence in the `{material_iri, additional_details}` shape PATCH wants."""
+    return [
+        {"material_iri": e.get("material_iri"),
+         "additional_details": e.get("additional_details") or ""}
+        for e in (case.get("evidence") or [])
+        if e.get("material_iri")
+    ]
+
+
+def merge_evidence(current, additions):
+    """Append `(material_iri, note)` pairs not already present, preserving order.
+
+    Never reorders, rewrites or drops an existing entry: `PATCH /evidence` replaces
+    the whole list, so anything left out is deleted. An IRI already present is
+    skipped rather than allowed to overwrite a note a human may have edited.
+    """
+    have = {e["material_iri"] for e in current}
+    merged = list(current)
+    for addition in additions:
+        # A bare IRI is the natural mistake here, and a 2-character string would
+        # unpack into iri="a", note="b" and bind silently. Say so instead.
+        if isinstance(addition, str):
+            raise TypeError(
+                f"merge_evidence takes (material_iri, note) pairs, got the bare "
+                f"string {addition!r}. Pass [(iri, '')] if there is no note.")
+        iri, note = addition
+        if iri in have:
+            continue
+        merged.append({"material_iri": iri, "additional_details": note})
+        have.add(iri)
+    return merged

casework/enrich_news_articles.py

@@ -40,9 +40,9 @@
 byte-compatible with the 48 `/material/news/*` rows already in production; the
 IRI form is not invented here (see `news_search.news_material_ident`).
 
-DEVIATION 2 -- THE NOTE IS WRITTEN AT BIND TIME, NEVER BLANK. `bind_materials.py
-:143` appends new evidence with `additional_details: ""` and leaves a later stage
-to backfill. This stage has the article in hand and the verifier has already
+DEVIATION 2 -- THE NOTE IS WRITTEN AT BIND TIME, NEVER BLANK. `bind_materials`
+appends new evidence with `additional_details: ""` and leaves a later stage to
+backfill. This stage has the article in hand and the verifier has already
 produced the Nepali note, so binding blank would cost a second document fetch of
 a document already read and strand the entry meanwhile. The register and length
 come from the 33 news notes on the 15 IN_REVIEW cases, read from production
@@ -103,6 +103,7 @@
 from dataclasses import dataclass, field
 
 from casework.common.api import CaseworkApi
+from casework.common.evidence import current_evidence, merge_evidence
 from casework.common.cli import (
     add_common_args,
     basic_auth_from_env,
@@ -190,24 +191,6 @@ def _require_loopback(api):
 # ---------------------------------------------------------------------------
 
 
-def current_evidence(case):
-    """The case's evidence normalised to the `{material_iri, additional_details}`
-    shape the PATCH expects, order preserved.
-
-    Byte-identical to `bind_materials.current_evidence` and imported-by-copy on
-    purpose: this is the contract for what a whole-list replace must send back,
-    and the two writers of `/evidence` must not be able to disagree about it.
-    Consolidating them into `casework/common/` is a separate change that would
-    edit a module this port has no other reason to touch.
-    """
-    return [
-        {"material_iri": e.get("material_iri"),
-         "additional_details": e.get("additional_details") or ""}
-        for e in (case.get("evidence") or [])
-        if e.get("material_iri")
-    ]
-
-
 def bound_news_urls(case):
     """Every article URL already bound to this case, normalised for comparison.
 
@@ -228,27 +211,6 @@ def count_news_evidence(case):
     return len(materials_of_type(case, (NEWS_MATERIAL_TYPE,)))
 
 
-def merge_news_evidence(current, additions):
-    """Union-merge `additions` into `current`, preserving existing order.
-
-    `bind_materials.merge_evidence` with one difference: an appended entry
-    carries its note instead of `""` (deviation 2). Everything else is that
-    function's contract, and it is the load-bearing half -- an existing entry is
-    never reordered, rewritten or dropped, because the server deletes every row
-    and recreates from exactly what is sent, so any omission destroys data. An
-    addition whose IRI is already present is skipped rather than allowed to
-    overwrite the note a human may have edited.
-    """
-    have = {e["material_iri"] for e in current}
-    merged = list(current)
-    for iri, note in additions:
-        if iri in have:
-            continue
-        merged.append({"material_iri": iri, "additional_details": note})
-        have.add(iri)
-    return merged
-
-
 # ---------------------------------------------------------------------------
 # The read phase. Search -> fetch -> verify -> select. No writes.
 # ---------------------------------------------------------------------------
@@ -531,7 +493,7 @@ def plan_case(case, etag, outcome, *, client=None, save_permalinks=True):
         materials.append((iri, doc, verdict.summary, article))
         additions.append((iri, verdict.summary))
 
-    merged = merge_news_evidence(current, additions)
+    merged = merge_evidence(current, additions)
     if merged == current:
         return NewsPlan(slug=slug, action="NOOP", state=state, if_match=etag,
                         n_current=len(current), outcome=outcome,

casework/news_search.py

@@ -282,9 +282,9 @@ def is_bindable(self):
         All four conditions are load-bearing. `high` is the bar (see the module
         docstring). `event_type` must be a real lifecycle value because the
         per-event cap and the bind ordering both key on it. `summary` must be
-        SUBSTANTIAL because it IS the evidence note -- binding without one is the
-        blank-note behaviour in `bind_materials.merge_evidence` that this port
-        exists to avoid, and a one-line note is that behaviour with extra steps.
+        SUBSTANTIAL because it IS the evidence note -- an entry bound blank is
+        what this stage exists to avoid, and a one-line note is that with extra
+        steps.
 
         The length floor is not cosmetic. `salvage_json` repairs a reply truncated
         at `max_tokens` by closing the open string, so an overflowing verify call

tests/casework/test_bind_materials.py

@@ -7,9 +7,10 @@
 
 import pytest
 
+from casework.common.evidence import merge_evidence
 from casework.bind_materials import (
     BindPlan, _build_api, _ledger_status, apply_plan, candidates_from_row,
-    merge_evidence, missing_candidates, parse_source_ident, plan_case, run,
+    missing_candidates, parse_source_ident, plan_case, run,
 )
 
 HOST = "https://jawafdehi.org/material"
@@ -87,13 +88,22 @@ def test_candidates_from_row_strips_status_dedupes_and_orders():
 
 def test_merge_evidence_appends_new_preserving_order():
     current = [{"material_iri": PR, "additional_details": ""}]
-    merged = merge_evidence(current, [CO])
+    merged = merge_evidence(current, [(CO, "")])
     assert [e["material_iri"] for e in merged] == [PR, CO]
 
 
 def test_merge_evidence_is_idempotent_on_existing():
     current = [{"material_iri": PR, "additional_details": "note"}]
-    assert merge_evidence(current, [PR]) == current
+    assert merge_evidence(current, [(PR, "")]) == current
+
+
+def test_merge_evidence_refuses_bare_iris():
+    """A bare IRI is the natural mistake, and a 2-char one would unpack into
+    iri="a", note="b" and bind garbage without complaint."""
+    with pytest.raises(TypeError, match="material_iri"):
+        merge_evidence([], [PR])
+    with pytest.raises(TypeError, match="material_iri"):
+        merge_evidence([], ["ab"])
 
 
 # ---------------------------------------------------------------------------

tests/casework/test_enrich_news_articles.py

@@ -34,6 +34,7 @@
 
 import pytest
 
+from casework import bind_materials
 from casework import enrich_news_articles as en
 from casework import news_search as ns
 from tests.casework.fakes import FakeUsage
@@ -725,7 +726,7 @@ def test_the_merge_appends_and_never_disturbs_an_existing_entry():
                 "additional_details": "a human wrote this"},
                {"material_iri": "https://jawafdehi.org/material/court_order/2",
                 "additional_details": ""}]
-    merged = en.merge_news_evidence(
+    merged = en.merge_evidence(
         current, [("https://jawafdehi.org/material/news/20240101.aaaaaaaa", "नयाँ नोट")])
     assert merged[:2] == current
     assert merged[2] == {
@@ -736,12 +737,12 @@ def test_the_merge_appends_and_never_disturbs_an_existing_entry():
 def test_the_merge_never_overwrites_a_note_on_an_iri_already_present():
     iri = "https://jawafdehi.org/material/news/20240101.aaaaaaaa"
     current = [{"material_iri": iri, "additional_details": "a human edited this"}]
-    merged = en.merge_news_evidence(current, [(iri, "the model would say this")])
+    merged = en.merge_evidence(current, [(iri, "the model would say this")])
     assert merged == current
 
 
 def test_a_bound_entry_carries_its_note_rather_than_binding_blank():
-    """Deviation 2 -- `bind_materials.merge_evidence` appends `""`; not here."""
+    """The news stage passes a real note where the binder passes `""`."""
     pair = MATCHES[0]
     plan = _plan_for_pair(pair, dict(SLOPPY_MEDIUM, confidence="high"))
     appended = plan.patch_items[-1]
@@ -810,7 +811,7 @@ def test_the_write_sends_the_whole_merged_list_not_a_delta():
                                    "https://jawafdehi.org/material/news/20230101.aaaa")
     api = FakeApi(case_payload(evidence=[existing]))
     plan = _bindable_plan()
-    plan.patch_items = en.merge_news_evidence(
+    plan.patch_items = en.merge_evidence(
         en.current_evidence(api.case), [(plan.bound_iris[0], "नोट")])
     en.apply_plan(api, plan)
     items = api.replaced[0]["items"]
@@ -1747,12 +1748,15 @@ def get(self, url, kind, headers=None, expect_html=False,
     assert "ABORTED at case 1/1" in out.err
 
 
-def test_bind_materials_records_the_reciprocal_duplicate_contract():
-    """Both stages PATCH the same destructive whole-list /evidence, so both
-    normalisers must stay identical. Only one side said so."""
-    src = pathlib.Path(en.__file__).parent.joinpath("bind_materials.py").read_text(encoding="utf-8")
-    assert src.count("DELIBERATELY DUPLICATED") == 2
-    assert "enrich_news_articles" in src
+def test_both_evidence_writers_share_one_normaliser():
+    """Two copies of a whole-list-replace normaliser can diverge and silently drop
+    evidence rows. Identity, not similarity, is what makes that impossible."""
+    from casework.common import evidence as shared
+
+    assert en.current_evidence is shared.current_evidence
+    assert en.merge_evidence is shared.merge_evidence
+    assert bind_materials.current_evidence is shared.current_evidence
+    assert bind_materials.merge_evidence is shared.merge_evidence
 
 
 # ---------------------------------------------------------------------------