fix(pfs): family edges — add-on/by-reference/single-target replacement/stem only; split components spanning hand families (refs #687)

Claude-Session: https://claude.ai/code/session_01Aum3pEMAM3yQVdFSdVe6Gc

src/pfs/families.py

@@ -169,21 +169,102 @@ def stem_tokens(description: str | None) -> frozenset[str]:
     )
 
 
-_LINK_RELATIONS = {
-    "addon-of",
-    "not-with",
-    "defined-by-reference-to",
-    "replaces",
-    "replaced-by",
-    "crosswalk-valued-to",
-}
-_LINK_EVENTS = {"replaces", "replaced_by", "crosswalk"}
+# Deliberately narrow (#687 over-merge incident): a family-membership edge
+# is only ever a direct, unambiguous statement that two codes belong
+# together. Dropped from the relation set: ``not-with`` (an explicit
+# *exclusion*, the opposite of membership), ``replaces``/``replaced-by``/
+# ``crosswalk-valued-to`` relation *elements* (these overlap the lineage
+# *event* signal below, which already carries year/anchor precision).
+# ``crosswalk`` events are dropped entirely — a rule's "codes ... are
+# crosswalked to X for valuation" sentence routinely lists a whole
+# family's worth of codes together and is evidence, not a merge signal.
+_LINK_RELATIONS = {"addon-of", "defined-by-reference-to"}
+#: ``replaces``/``replaced_by`` events still count, but only when the
+#: relevant field names exactly one code — a multi-code list (a blanket
+#: "these codes are replaced by ..." sentence spanning several families,
+#: see #687) is ambiguous and is ignored as a merge edge.
+_LINK_EVENTS = {"replaces", "replaced_by"}
 
 
 def _jaccard(a: frozenset[str], b: frozenset[str]) -> float:
     return len(a & b) / len(a | b) if a or b else 0.0
 
 
+def _name_group(
+    members: Sequence[str],
+    stems: Mapping[str, frozenset[str]],
+    descriptions: Mapping[str, str],
+    tenure: Any,
+) -> tuple[str, str]:
+    """The stem-derived ``(key, name)`` for a group with no hand family —
+    shared by ungrouped components and by the leftover codes a hand-family
+    split (see ``derive_families``) doesn't reach."""
+    rep = sorted(members, key=lambda c: (-tenure(c), c))[0]
+    key = (
+        "-".join(
+            t
+            for t in _RE_TOKEN.findall((descriptions.get(rep) or "").lower())
+            if t in stems[rep]
+        ).upper()
+        or rep
+    )
+    name = descriptions.get(rep) or rep
+    return key, name
+
+
+def _group_rows(
+    key: str,
+    name: str,
+    members: Sequence[str],
+    elements: Mapping[str, Sequence[Any]],
+    events: Mapping[str, Sequence[Any]],
+) -> list[Any]:
+    """``FamilyRow`` per code in *members*, labeled ``key``/``name``."""
+    from pfs.codetables import FamilyRow
+
+    out = []
+    for c in sorted(members):
+        els = elements.get(c, ())
+        evs = events.get(c, ())
+        kinds = {ev.kind for ev in evs}
+        if "replaced_by" in kinds:
+            role = "predecessor"
+        elif any(e.type == "relation" and e.value == "addon-of" for e in els):
+            role = "add-on"
+        elif "replaces" in kinds:
+            role = "successor"
+        else:
+            role = "base"
+        since = next((ev.year for ev in evs if ev.kind == "appeared"), None)
+        until = next((ev.year for ev in evs if ev.kind == "disappeared"), None)
+        anchor = next(((ev.item_key, ev.p_id) for ev in evs if ev.item_key), ("", 0))
+        out.append(FamilyRow(key, name, c, role, since, until, anchor[0], anchor[1]))
+    return out
+
+
+def _connected(nodes: Sequence[str], adj: Mapping[str, set[str]]) -> list[list[str]]:
+    """Connected components of *nodes* using only edges between members of
+    *nodes* itself (an edge to a node outside the set doesn't count)."""
+    node_set = set(nodes)
+    seen: set[str] = set()
+    out: list[list[str]] = []
+    for start in sorted(nodes):
+        if start in seen:
+            continue
+        stack = [start]
+        seen.add(start)
+        comp = []
+        while stack:
+            c = stack.pop()
+            comp.append(c)
+            for n in adj.get(c, ()):
+                if n in node_set and n not in seen:
+                    seen.add(n)
+                    stack.append(n)
+        out.append(comp)
+    return out
+
+
 def derive_families(
     elements: Mapping[str, Sequence[Any]],
     events: Mapping[str, Sequence[Any]],
@@ -196,11 +277,19 @@ def derive_families(
     predecessor (has a ``replaced_by`` event) > add-on (has an
     ``addon-of`` relation element) > successor (has a ``replaces``
     event) > base.
-    """
-    from pfs.codetables import FamilyRow
 
+    A component that reaches into two or more hand families (#687: the
+    real corpus's blanket multi-code lineage sentences used to merge ACP,
+    CCM and PCM into one blob even with the narrowed edges above still
+    occasionally bridging two families) is split: each hand family's own
+    codes seed a BFS over the component's edges, run in ``HAND_FAMILIES``
+    order, and every code the BFS reaches joins that hand family. Codes no
+    BFS reaches keep their own stem-keyed family, same as an ungrouped
+    component.
+    """
     codes = sorted(set(elements) | set(events) | set(descriptions))
     parent = {c: c for c in codes}
+    adj: dict[str, set[str]] = {c: set() for c in codes}
 
     def find(c: str) -> str:
         while parent[c] != c:
@@ -210,6 +299,8 @@ def derive_families(
 
     def union(a: str, b: str) -> None:
         if a in parent and b in parent:
+            adj[a].add(b)
+            adj[b].add(a)
             parent[find(a)] = find(b)
 
     for code, els in elements.items():
@@ -218,9 +309,12 @@ def derive_families(
                 union(code, e.detail.upper())
     for code, evs in events.items():
         for ev in evs:
-            if ev.kind in _LINK_EVENTS:
-                for other in (ev.from_codes + " " + ev.to_codes).split():
-                    union(code, other.upper())
+            if ev.kind not in _LINK_EVENTS:
+                continue
+            field = ev.from_codes if ev.kind == "replaces" else ev.to_codes
+            targets = field.split()
+            if len(targets) == 1:
+                union(code, targets[0].upper())
     stems = {c: stem_tokens(descriptions.get(c) or "") for c in codes}
     acts = {
         c: frozenset(e.value for e in elements.get(c, ()) if e.type == "activity")
@@ -260,44 +354,49 @@ def derive_families(
         )
         return until - since
 
-    rows: list[FamilyRow] = []
+    rows: list[Any] = []
     for members in groups.values():
-        hand = next(
-            (k for k, f in HAND_FAMILIES.items() if set(f.codes) & set(members)), None
-        )
+        member_set = set(members)
+        hands = [k for k in HAND_FAMILIES if set(HAND_FAMILIES[k].codes) & member_set]
+        if len(hands) >= 2:
+            _logger.warning(
+                "family component of %d codes spans hand families %s — "
+                "splitting by hand-family BFS",
+                len(members),
+                hands,
+            )
+            assigned: dict[str, str] = {}
+            for hk in hands:
+                for seed in HAND_FAMILIES[hk].codes:
+                    if seed in member_set:
+                        assigned[seed] = hk
+            for hk in hands:
+                frontier = [c for c in HAND_FAMILIES[hk].codes if c in member_set]
+                while frontier:
+                    nxt = []
+                    for c in frontier:
+                        for n in adj.get(c, ()):
+                            if n in member_set and n not in assigned:
+                                assigned[n] = hk
+                                nxt.append(n)
+                    frontier = nxt
+            for hk in hands:
+                grp = [c for c, v in assigned.items() if v == hk]
+                if grp:
+                    rows.extend(
+                        _group_rows(hk, HAND_FAMILIES[hk].name, grp, elements, events)
+                    )
+            unreached = [c for c in members if c not in assigned]
+            for sub in _connected(unreached, adj):
+                key, name = _name_group(sub, stems, descriptions, tenure)
+                rows.extend(_group_rows(key, name, sub, elements, events))
+            continue
+        hand = hands[0] if hands else None
         if hand:
             key, name = hand, HAND_FAMILIES[hand].name
         else:
-            rep = sorted(members, key=lambda c: (-tenure(c), c))[0]
-            key = (
-                "-".join(
-                    t
-                    for t in _RE_TOKEN.findall((descriptions.get(rep) or "").lower())
-                    if t in stems[rep]
-                ).upper()
-                or rep
-            )
-            name = descriptions.get(rep) or rep
-        for c in sorted(members):
-            els = elements.get(c, ())
-            evs = events.get(c, ())
-            kinds = {ev.kind for ev in evs}
-            if "replaced_by" in kinds:
-                role = "predecessor"
-            elif any(e.type == "relation" and e.value == "addon-of" for e in els):
-                role = "add-on"
-            elif "replaces" in kinds:
-                role = "successor"
-            else:
-                role = "base"
-            since = next((ev.year for ev in evs if ev.kind == "appeared"), None)
-            until = next((ev.year for ev in evs if ev.kind == "disappeared"), None)
-            anchor = next(
-                ((ev.item_key, ev.p_id) for ev in evs if ev.item_key), ("", 0)
-            )
-            rows.append(
-                FamilyRow(key, name, c, role, since, until, anchor[0], anchor[1])
-            )
+            key, name = _name_group(members, stems, descriptions, tenure)
+        rows.extend(_group_rows(key, name, members, elements, events))
     return sorted(rows, key=lambda r: (r.key, r.code))
 
 

tests/pfs/test_families.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import logging
 import time
 
 import duckdb
@@ -144,6 +145,11 @@ class TestStemTokens:
 
 class TestDerive:
     def test_ccm_reproduced_with_predecessor_and_addon_roles(self):
+        # #687 ruling 16: `not-with` is an exclusion, not a membership
+        # signal, and was dropped from _LINK_RELATIONS. The `not-with`
+        # element below is now inert as a merge edge — 99487 joins CCM
+        # through the stem-Jaccard + shared "comprehensive-care-plan"
+        # activity rule instead (its stem overlaps 99490's >= 0.5).
         elements = {
             "99490": [
                 _el("99490", "actor", "clinical-staff-directed"),
@@ -206,6 +212,59 @@ class TestDerive:
             r.name == HAND_FAMILIES[r.key].name for r in rows if r.key in HAND_FAMILIES
         )
 
+    def test_multi_code_replaced_by_does_not_merge(self):
+        # #687 ruling 16: a `replaced_by` event naming more than one
+        # to_codes is ambiguous (a blanket "these codes are replaced by
+        # ..." sentence spanning several families in the real corpus) and
+        # must not become a merge edge.
+        elements = {"11111": [], "22222": []}
+        events = {"11111": [_ev("11111", 2021, "replaced_by", to="22222 33333")]}
+        descriptions = {"11111": "Alpha widget", "22222": "Zeta gadget"}
+        rows = derive_families(elements, events, descriptions)
+        keys = {r.code: r.key for r in rows}
+        assert keys["11111"] != keys["22222"]
+
+    def test_not_with_relation_does_not_merge(self):
+        # #687 ruling 16: `not-with` is an explicit exclusion (cannot be
+        # billed together), the opposite of family membership.
+        elements = {
+            "99490": [_el("99490", "relation", "not-with", "G0556")],
+            "G0556": [],
+        }
+        descriptions = {
+            "99490": "Chrnc care mgmt staff 1st 20",
+            "G0556": "Adv prim care mgmt lvl 1",
+        }
+        rows = derive_families(elements, {}, descriptions)
+        keys = {r.code: r.key for r in rows}
+        assert keys["99490"] != keys["G0556"]
+
+    def test_bridging_addon_chain_splits_by_hand_family_and_warns(self, caplog):
+        # #687 ruling 16 guard: a component that reaches into two or more
+        # hand families is split by a BFS seeded from each hand family's
+        # own codes, run in HAND_FAMILIES order (ACP, CCM, PCM, TCM,
+        # APCM). Here "77777" is addon-of both a CCM seed (99490) and an
+        # ACP seed (99497), joining all three into one component; ACP
+        # comes first in HAND_FAMILIES, so its BFS reaches — and claims —
+        # the bridging code before CCM's BFS runs.
+        elements = {
+            "99490": [],
+            "99497": [],
+            "77777": [
+                _el("77777", "relation", "addon-of", "99490"),
+                _el("77777", "relation", "addon-of", "99497"),
+            ],
+        }
+        with caplog.at_level(logging.WARNING, logger="pfs.families"):
+            rows = derive_families(elements, {}, {})
+        by_code = {r.code: r for r in rows}
+        assert by_code["99490"].key == "CCM"
+        assert by_code["99497"].key == "ACP"
+        assert by_code["77777"].key == "ACP"
+        assert any("spans hand families" in rec.message for rec in caplog.records), (
+            caplog.text
+        )
+
     def test_unknown_family_gets_stem_key(self):
         elements = {
             "99453": [_el("99453", "activity", "device-data-review")],