fix(pfs,llm): trie alternation groups optional continuations; wide families do not expand codes; warm the replica before detection (refs #699)

src/llm/api.py

@@ -11,13 +11,28 @@ from __future__ import annotations
 
 import json
 import re
+from contextlib import asynccontextmanager
 from importlib import resources
-from typing import Iterator
+from typing import AsyncIterator, Iterator
 
 from fastapi import FastAPI, Header, HTTPException
 from fastapi.responses import HTMLResponse, StreamingResponse
 from pydantic import BaseModel
 
+
+@asynccontextmanager
+async def _lifespan(app: FastAPI) -> AsyncIterator[None]:
+    """Warm the DuckDB replica (and refresh the derived families from
+    it) at boot instead of on the chat's first code-valuation lookup —
+    refs #699 ruling B3. Never raises: ``llm.evidence.warm`` is a quiet
+    no-op when the replica file isn't there yet."""
+    from llm import config as llm_config
+    from llm.evidence import warm
+
+    warm(llm_config.load())
+    yield
+
+
 app = FastAPI(
     title="llm chat",
     description=(
@@ -25,6 +40,7 @@ app = FastAPI(
         "reference corpus."
     ),
     version="0.2.0",
+    lifespan=_lifespan,
 )
 
 _ISO_DATE = re.compile(r"^\d{4}-\d{2}-\d{2}$")

src/llm/evidence.py

@@ -149,7 +149,26 @@ def _replica_path(cfg: LlmConfig) -> str:
     return str(ROOT / p)
 
 
+def warm(cfg: LlmConfig) -> None:
+    """Open (and cache) the DuckDB replica connection ahead of the first
+    real request (refs #699 ruling B3) — called once at API startup
+    (``llm.api``'s startup hook) and again, defensively, at the top of
+    every ``valuation_evidence`` call, where it is a no-op once cached
+    (``_connect`` keys on path + mtime, so a second call in the same
+    process just returns the cached handle without reopening). A missing
+    replica file is a quiet no-op — never raises, so it can never fail
+    startup or a chat turn."""
+    path = _replica_path(cfg)
+    if not Path(path).exists():
+        return
+    try:
+        _connect(path)
+    except Exception as e:  # noqa: BLE001 — duckdb raises several types
+        log.warning("replica warm-up skipped (%s): %s", path, e)
+
+
 def valuation_evidence(question: str, cfg: LlmConfig) -> ValuationEvidence | None:
+    warm(cfg)
     det = detect_codes(question)
     if not det.codes:
         return None

src/pfs/families.py

@@ -168,7 +168,18 @@ def _trie_alternation(phrases: Sequence[str]) -> str:
             return ""  # end() only — caller makes it optional
         alts = [re.escape(ch) + _compile(node[ch]) for ch in chars]
         body = alts[0] if len(alts) == 1 else "(?:" + "|".join(alts) + ")"
-        return f"{body}?" if end else body
+        if not end:
+            return body
+        # Bug (fix round 2): a single-branch continuation's `body` can be
+        # a multi-character string ("s" from `alts[0]`, not grouped) —
+        # `f"{body}?"` then made only the *last character* optional, not
+        # the whole continuation, so any phrase that is a proper prefix
+        # of another (e.g. "cardiac catheterization" inside "cardiac
+        # catheterization for congenital heart defects") silently
+        # stopped matching. Always parenthesize before the `?`, even
+        # when `body` is already a single grouped alternation (harmless
+        # double-wrap) — this is the only case that's provably correct.
+        return f"(?:{body})?"
 
     return _compile(root)
 
@@ -254,13 +265,27 @@ class Detection:
     codes: tuple[str, ...]  # sorted unique: explicit + family-expanded
     families: tuple[str, ...]  # family keys, sorted
     explicit: tuple[str, ...]  # codes literally present in the text
+    #: Ruling B2: family keys detected (by name or member code) whose
+    #: ``len(codes) > MAX_FAMILY_EXPAND`` — still listed in ``families``,
+    #: but their member codes are *not* folded into ``codes`` (a wide
+    #: family's whole code list would crowd a valuation prompt).
+    wide: tuple[str, ...] = ()
+
+
+#: Ruling B2: a family this big is a real grouping worth naming in the
+#: answer, but not worth pricing every member code for — a 378-code
+#: heading (e.g. a broad CPT chapter-level rollup) would otherwise blow
+#: the valuation prompt budget for one mention.
+MAX_FAMILY_EXPAND = 20
 
 
 def detect_codes(text: str) -> Detection:
     """Codes a question is about: explicit codes plus every code of any
-    family named (by synonym, or by a qualifying derived name) or touched
-    (by one member code). Runs off the precompiled ``_PHRASE_RE``/
-    ``_CODE_INDEX`` — no per-family scan."""
+    *narrow* (``len(codes) <= MAX_FAMILY_EXPAND``) family named (by
+    synonym, or by a qualifying derived name) or touched (by one member
+    code); a wide family is still named in ``families``/``wide`` but its
+    codes are not expanded into ``codes``. Runs off the precompiled
+    ``_PHRASE_RE``/``_CODE_INDEX`` — no per-family scan."""
     explicit = find_codes(text)
     lowered = text.lower()
     families: set[str] = set()
@@ -269,14 +294,20 @@ def detect_codes(text: str) -> Detection:
     for code in explicit:
         families.update(_CODE_INDEX.get(code, ()))
     codes = set(explicit)
+    wide: set[str] = set()
     for key in families:
         fam = FAMILIES.get(key)
-        if fam is not None:
+        if fam is None:
+            continue
+        if len(fam.codes) <= MAX_FAMILY_EXPAND:
             codes.update(fam.codes)
+        else:
+            wide.add(key)
     return Detection(
         codes=tuple(sorted(codes)),
         families=tuple(sorted(families)),
         explicit=explicit,
+        wide=tuple(sorted(wide)),
     )
 
 

tests/llm/test_api.py

@@ -113,6 +113,22 @@ def _cfg(**kw):
     return LlmConfig(**base)
 
 
+class TestStartup:
+    """#699 ruling B3: the replica is warmed at boot, not on the chat's
+    first valuation lookup. ``with TestClient(app) as c:`` is required to
+    actually trigger FastAPI's startup event — a bare ``TestClient(app)``
+    (as ``client`` above, module-level) never sends the ASGI lifespan
+    ``startup`` message."""
+
+    @patch("llm.config.load")
+    def test_startup_event_warms_the_replica(self, mock_load):
+        mock_load.return_value = _cfg()
+        with patch("llm.evidence.warm") as mock_warm:
+            with TestClient(app):
+                pass
+        mock_warm.assert_called_once_with(mock_load.return_value)
+
+
 class TestHosts:
     @patch("llm.pool.pick_model", return_value="big")
     @patch("llm.pool.HostPool.check", return_value=["http://h2:11434"])

tests/llm/test_evidence.py

@@ -4,6 +4,7 @@ from __future__ import annotations
 
 from concurrent.futures import ThreadPoolExecutor
 from dataclasses import replace
+from types import SimpleNamespace
 from unittest.mock import MagicMock, patch
 
 import duckdb
@@ -16,6 +17,7 @@ from llm.evidence import (
     code_cited_sources,
     merge_sources,
     valuation_evidence,
+    warm,
 )
 from pfs.valuation import ValuationRow
 
@@ -228,6 +230,52 @@ class TestValuationEvidence:
         assert mock_val.call_args.kwargs == {"years": 2}
 
 
+class TestWarm:
+    """#699 ruling B3: warm the replica connection ahead of the first
+    real request — at API startup and again (a no-op once cached) at the
+    top of every ``valuation_evidence`` call."""
+
+    def test_warm_opens_once_and_is_a_noop_on_replay(self, tmp_path):
+        path = tmp_path / "aco.ro.duckdb"
+        path.touch()  # warm() only needs the file to exist, not be a real db
+        cfg = replace(CFG, duckdb_replica=str(path))
+        with patch("llm.evidence.duckdb.connect") as mock_connect:
+            mock_connect.return_value = MagicMock()
+            warm(cfg)
+            warm(cfg)
+        mock_connect.assert_called_once_with(str(path), read_only=True)
+
+    def test_warm_is_a_noop_when_the_replica_file_is_missing(self):
+        with patch("llm.evidence.duckdb.connect") as mock_connect:
+            warm(CFG)  # CFG.duckdb_replica ("/nonexistent/...") doesn't exist
+        mock_connect.assert_not_called()
+
+    def test_warm_swallows_connect_failures(self, tmp_path, caplog):
+        path = tmp_path / "aco.ro.duckdb"
+        path.touch()
+        cfg = replace(CFG, duckdb_replica=str(path))
+        with patch("llm.evidence.duckdb.connect", side_effect=OSError("boom")):
+            warm(cfg)  # must not raise
+        assert "replica warm-up skipped" in caplog.text
+
+    @patch("llm.evidence.detect_codes")
+    @patch("llm.evidence.warm")
+    def test_valuation_evidence_warms_before_detecting_codes(
+        self, mock_warm, mock_detect
+    ):
+        order: list[str] = []
+        mock_warm.side_effect = lambda cfg: order.append("warm")
+
+        def _detect(question):
+            order.append("detect")
+            return SimpleNamespace(codes=())
+
+        mock_detect.side_effect = _detect
+        assert valuation_evidence("anything", CFG) is None
+        assert order == ["warm", "detect"]
+        mock_warm.assert_called_once_with(CFG)
+
+
 class TestConnectRefreshesFamilies:
     """#699: derived families (``pfs.code_family``) live only on the
     replica — ``_connect`` is the chat's one hook to pick them up, on

tests/pfs/test_families.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import logging
+import re
 import time
 
 import duckdb
@@ -21,9 +22,11 @@ from pfs.codetables import (
 from pfs.families import (
     FAMILIES,
     HAND_FAMILIES,
+    MAX_FAMILY_EXPAND,
     Detection,
     Family,
     _cpt_edges,
+    _trie_alternation,
     cpt_groups,
     derive_families,
     detect_codes,
@@ -125,6 +128,77 @@ class TestDetectCodes:
         )
 
 
+class TestTrieAlternation:
+    """Fix round 2, item 1: ``_trie_alternation``'s optional-continuation
+    bug. When a node has exactly one continuation, ``body`` was an
+    unwrapped (possibly multi-character) string, and a trailing ``?``
+    applied to it bound to only its *last character* — so any phrase
+    that is a proper prefix of a longer registered phrase (e.g.
+    "cardiac catheterization" inside "cardiac catheterization for
+    congenital heart defects") silently stopped matching at all. The
+    task-1 report's "verified identical" claim was based on running one
+    sample question through the live registry, which happened not to
+    exercise a prefix-of-another-phrase pair — it did not prove
+    equivalence in general, and was wrong."""
+
+    #: A two-way prefix collision ("cardiac catheterization" is a strict
+    #: prefix of the "... for congenital heart defects" phrase), a
+    #: three-way share (both "... services" and "... program" continue
+    #: the same "chronic care management" prefix, so that node has an
+    #: end marker *and* two children), plus the five phrases the report
+    #: named as broken live.
+    PHRASES = (
+        "cardiac catheterization",
+        "cardiac catheterization for congenital heart defects",
+        "chronic care management",
+        "chronic care management services",
+        "chronic care management program",
+        "tricuspid valve",
+        "tricuspid valve repair",
+        "adaptive behavior assessments",
+        "repair and/or reconstruction",
+        "subcutaneous cardiac rhythm monitor",
+    )
+
+    SENTENCES = (
+        "Discuss cardiac catheterization for congenital heart defects in infants.",
+        "A cardiac catheterization was performed without complication.",
+        "Chronic care management services are billed monthly.",
+        "Our chronic care management program includes personalized outreach.",
+        "We provide chronic care management for our patients, full stop.",
+        "Tricuspid valve repair is a common cardiac procedure.",
+        "The tricuspid valve was evaluated by echo.",
+        "Adaptive behavior assessments are conducted for autism evaluations.",
+        "This code covers repair and/or reconstruction of the tendon.",
+        "A subcutaneous cardiac rhythm monitor was implanted last week.",
+        "None of these words appear in this sentence at all.",
+    ) + PHRASES  # each phrase alone too
+
+    @staticmethod
+    def _flat_re(phrases) -> re.Pattern[str]:
+        ordered = sorted(phrases, key=len, reverse=True)
+        alt = "|".join(re.escape(p) for p in ordered)
+        return re.compile(rf"\b(?:{alt})\b", re.IGNORECASE)
+
+    @staticmethod
+    def _trie_re(phrases) -> re.Pattern[str]:
+        alt = _trie_alternation(sorted(phrases))
+        return re.compile(rf"\b(?:{alt})\b", re.IGNORECASE)
+
+    def test_trie_matches_flat_alternation(self):
+        flat_re = self._flat_re(self.PHRASES)
+        trie_re = self._trie_re(self.PHRASES)
+        for s in self.SENTENCES:
+            lowered = s.lower()
+            flat = [m.group(0) for m in flat_re.finditer(lowered)]
+            trie = [m.group(0) for m in trie_re.finditer(lowered)]
+            assert trie == flat, (s, flat, trie)
+        # And directly: every prefix-collision phrase, embedded in its
+        # longer sibling, must still match on its own.
+        assert self._trie_re(self.PHRASES).search("cardiac catheterization today")
+        assert self._trie_re(self.PHRASES).search("chronic care management today")
+
+
 class TestDetectDerivedFamilies:
     """#699: the chat must see derived (``pfs.code_family``) families,
     not only the five hand families — by member code always, by name
@@ -188,6 +262,41 @@ class TestDetectDerivedFamilies:
         elapsed = time.perf_counter() - start
         assert elapsed < 0.2, elapsed
 
+    def test_wide_family_does_not_expand_codes(self, restore_families):
+        # Ruling B2: a family with more than MAX_FAMILY_EXPAND (20) codes
+        # is still named (in `families`/`wide`) but its member codes are
+        # not folded into `codes` — only explicit codes survive.
+        wide_codes = tuple(f"{20000 + i}" for i in range(378))
+        assert len(wide_codes) > MAX_FAMILY_EXPAND
+        restore_families.FAMILIES["WIDE-FAMILY"] = Family(
+            "WIDE-FAMILY",
+            "Comprehensive Ambulatory Service Bundle",
+            wide_codes,
+            (),
+            cpt=True,
+        )
+        rebuild_index()
+        d = detect_codes("How is the Comprehensive Ambulatory Service Bundle valued?")
+        assert d.explicit == ()
+        assert d.codes == d.explicit  # nothing expanded
+        assert "WIDE-FAMILY" in d.families
+        assert "WIDE-FAMILY" in d.wide
+
+    def test_narrow_family_still_expands_under_the_cap(self, restore_families):
+        codes = tuple(f"{30000 + i}" for i in range(MAX_FAMILY_EXPAND))
+        restore_families.FAMILIES["NARROW-FAMILY"] = Family(
+            "NARROW-FAMILY",
+            "Narrow Bundled Service Package",
+            codes,
+            (),
+            cpt=True,
+        )
+        rebuild_index()
+        d = detect_codes("How is the Narrow Bundled Service Package valued?")
+        assert d.codes == codes
+        assert "NARROW-FAMILY" in d.families
+        assert d.wide == ()
+
     def test_hand_families_regression(self):
         # Unchanged from TestDetectCodes — the phrase/index refactor must
         # not alter a single hand-family detection.