fix(ingestion): do not match on external ids by default

Reconciling against staging answered 3.2: all 38 Diver-HUB points match Ocotillo
wells by name, nothing ambiguous, nothing unmatched. The wells already exist, so
the seeding half creates none.

The same data showed external-id matching is unsafe here. thing_id_link holds
11,148 links from nine organization/relation pairs that disagree with each
other. SO-0131 carries NMBGMR "BRN-E04B (shallow)" plus an unattributed
"BRN-E04A", while SO-0132 carries NMBGMR "BRN-E04A (deep)" plus an unattributed
"BRN-E04B" -- the two sources swap which physical well is A and which is B.

Matching BRN-E04A against that returns one confident hit on SO-0131,
contradicting NMBGMR, because the parenthetical suffix stops the collision
registering as ambiguous. That is worse than the ambiguity the module was built
to escalate: a wrong answer delivered with no sign of trouble.

So the fallback is opt-in, and a test pins those exact rows. It costs nothing
today, since every point matches by name.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: jirhiker

automated_ingestion/sources/san_acacia/reconcile.py

@@ -125,16 +125,39 @@ def _normalize(value: str) -> str:
     return "".join(c for c in (value or "") if c.isalnum()).upper()
 
 
-def match_point(point: VendorPoint, candidates: Iterable[ThingCandidate]) -> Match:
-    """Decide one point against the wells it might be."""
+def match_point(
+    point: VendorPoint,
+    candidates: Iterable[ThingCandidate],
+    use_external_ids: bool = False,
+) -> Match:
+    """Decide one point against the wells it might be.
+
+    ``use_external_ids`` is off by default, for a specific reason.
+    ``thing_id_link`` holds identifiers from several organizations that disagree
+    with each other. In staging, ``SO-0131`` carries NMBGMR ``BRN-E04B
+    (shallow)`` plus an unattributed ``BRN-E04A``, while ``SO-0132`` carries
+    NMBGMR ``BRN-E04A (deep)`` plus an unattributed ``BRN-E04B`` -- the two
+    sources swap which physical well is A and which is B.
+
+    Matching ``BRN-E04A`` against that returns a single confident hit on
+    SO-0131, contradicting NMBGMR, because the parenthetical suffix stops the
+    collision registering as ambiguous. A wrong answer delivered confidently is
+    worse than no answer.
+
+    It costs nothing today: all 38 Diver-HUB points match Ocotillo wells by name.
+    """
     target = _normalize(point.name)
 
     by_name = [c for c in candidates if _normalize(c.name) == target]
-    by_external = [
-        c
-        for c in candidates
-        if any(_normalize(x) == target for x in c.external_ids) and c not in by_name
-    ]
+    by_external = (
+        [
+            c
+            for c in candidates
+            if any(_normalize(x) == target for x in c.external_ids) and c not in by_name
+        ]
+        if use_external_ids
+        else []
+    )
 
     # Name first: it is the identifier the Bureau uses, and an external id link
     # is a record of an association someone made, which may be older.
@@ -153,13 +176,17 @@ def match_point(point: VendorPoint, candidates: Iterable[ThingCandidate]) -> Mat
 
 
 def reconcile(
-    points: Iterable[VendorPoint], candidates: Iterable[ThingCandidate]
+    points: Iterable[VendorPoint],
+    candidates: Iterable[ThingCandidate],
+    use_external_ids: bool = False,
 ) -> ReconciliationReport:
     """Match every vendor point, reporting rather than resolving."""
     candidate_list = list(candidates)
     report = ReconciliationReport()
     for point in points:
-        report.matches.append(match_point(point, candidate_list))
+        report.matches.append(
+            match_point(point, candidate_list, use_external_ids=use_external_ids)
+        )
     return report
 
 

automated_ingestion/tests/test_reconcile.py

@@ -52,16 +52,47 @@ def test_adjacent_identifier_is_not_a_match():
     assert match.kind is MatchKind.UNMATCHED
 
 
-def test_external_id_match_when_the_name_differs():
+def test_external_ids_are_ignored_by_default():
     match = match_point(
         POINT,
         [ThingCandidate(thing_id=9, name="Renamed Well", external_ids=("SO-0125",))],
     )
+    assert match.kind is MatchKind.UNMATCHED
+
+
+def test_external_id_match_when_explicitly_enabled():
+    match = match_point(
+        POINT,
+        [ThingCandidate(thing_id=9, name="Renamed Well", external_ids=("SO-0125",))],
+        use_external_ids=True,
+    )
     assert match.kind is MatchKind.EXTERNAL_ID
     assert match.thing_id == 9
 
 
+def test_external_ids_can_produce_a_confident_wrong_answer():
+    """Why external id matching is off by default. Real rows from staging.
+
+    SO-0131 and SO-0132 swap which physical well is A and which is B between
+    NMBGMR and the unattributed source. Matching BRN-E04A returns SO-0131 with
+    no hint of trouble, while NMBGMR asserts SO-0132 is BRN-E04A -- the
+    parenthetical suffix stops the collision registering as ambiguous.
+    """
+    candidates = [
+        ThingCandidate(2369, "SO-0131", ("BRN-E04B (shallow)", "BRN-E04A")),
+        ThingCandidate(2373, "SO-0132", ("BRN-E04A (deep)", "BRN-E04B")),
+    ]
+    enabled = match_point(
+        VendorPoint(999, "BRN-E04A"), candidates, use_external_ids=True
+    )
+    assert enabled.thing_id == 2369  # contradicts NMBGMR, and looks certain
+
+    default = match_point(VendorPoint(999, "BRN-E04A"), candidates)
+    assert default.kind is MatchKind.UNMATCHED  # escalates instead
+
+
 def test_name_wins_over_external_id():
+    # Only relevant when external ids are enabled.
     # The name is the identifier the Bureau uses now; a link records an
     # association someone made earlier, which may be stale.
     match = match_point(
@@ -70,6 +101,7 @@ def test_name_wins_over_external_id():
             ThingCandidate(thing_id=7, name="SO-0125"),
             ThingCandidate(thing_id=9, name="Other", external_ids=("SO-0125",)),
         ],
+        use_external_ids=True,
     )
     assert match.thing_id == 7
 

docs/automated-ingestion-pipeline-plan.md

@@ -277,7 +277,12 @@ Reconciliation report built — `sources/san_acacia/reconcile.py` and `scripts/r
 - ✅ **Never picks a winner.** More than one candidate is `ambiguous` and escalates; none is `unmatched` and escalates. Ingestion does not create wells, and choosing between two plausible ones is the judgement that must not be automated.
 - ✅ `report.ready` is false unless *every* point resolved, and false for empty input. A partial load produces a series that looks complete and is not.
 - ✅ The script exits non-zero when anything needs a human, so it can gate a later step without relying on someone reading the output.
-- ⬜ Run it against staging and production and act on the result.
+- ✅ **Run against staging: all 38 points match by name. Nothing ambiguous, nothing unmatched, `ready = True`.** The wells already exist — SO-0125 is thing 2343, SO-0131 is 2369, and so on through 277 `SO-` wells in that database. So the seeding half creates no wells; it only needs the parameter, sensor, deployments and external identifiers.
+- ⬜ Run against production and confirm the same.
+
+**External-id matching is off by default, on evidence.** `thing_id_link` in staging holds 11,148 links from nine organization/relation pairs — NMBGMR (8,603), PLSS (7,052), an unattributed "Unknown" (4,825), NMOSE, USGS, NMED, TWDB — and they disagree with each other. `SO-0131` carries NMBGMR `BRN-E04B (shallow)` plus an unattributed `BRN-E04A`, while `SO-0132` carries NMBGMR `BRN-E04A (deep)` plus an unattributed `BRN-E04B`: the two sources swap which physical well is A and which is B. (`SO-0262`/`SO-0263` disagree more sharply still — NMBGMR calls them NRCS 3A/3B, the other source NRCS 2.)
+
+Matching `BRN-E04A` against that returns a single confident hit on `SO-0131`, contradicting NMBGMR, because the parenthetical suffix stops the collision registering as ambiguous. A wrong answer delivered confidently is worse than no answer, so the fallback is opt-in and a test pins the real rows.
 - ⬜ The seeding half: data migration creating missing `Location`/`Thing`, lexicon terms, DTW `Parameter`, `VanEssenDiver` `Sensor`, one `Deployment` per well, the vendor `uid` as external identifier, and `DataProvenance` for Van Essen-sourced attributes.
 
 ### 3.3 — Represent "public but provisional"