fix(prisma): fetch_pmc via the OA Web Service instead of the bot-challenged /pdf/ guess URL (refs #657)

The /pmc/articles/<id>/pdf/ path serves an HTML challenge to scripted
clients, so the PMC tier never yielded a PDF. fetch_pmc now asks
oa.fcgi (sanctioned; resolves ~half our PMC items) and rewrites ftp://
hrefs to the HTTPS mirror. Delivery is still broken upstream — NCBI
retired both the FTP tree and the HTTPS mirror paths the OA service
links to — so yield is unchanged until the S3 open-data path lands;
full findings and the remaining plan are on #657.

src/prisma/extract.py

@@ -18,9 +18,9 @@ from typing import TYPE_CHECKING
 
 from prisma.export import load_item, to_markdown
 from prisma.ingest import apply_extraction
-from prisma.llm import LLMCall, LLMMessage, Provider
+from prisma.llm import LLMCall, LLMMessage, LLMTool, Provider
 from prisma.project import Project
-from prisma.tools import EXTRACT_TOOL
+from prisma.tools import build_extract_tool
 
 if TYPE_CHECKING:
     from zot.db import Db
@@ -41,7 +41,7 @@ extraction into `extraction_notes` (one to three sentences max).
 """
 
 
-def _build_call(project: Project, item_markdown: str) -> LLMCall:
+def _build_call(project: Project, item_markdown: str, tool: LLMTool) -> LLMCall:
     return LLMCall(
         messages=[
             LLMMessage(role="system", content=_SYSTEM),
@@ -55,8 +55,8 @@ def _build_call(project: Project, item_markdown: str) -> LLMCall:
             ),
             LLMMessage(role="user", content=item_markdown),
         ],
-        tools=[EXTRACT_TOOL],
-        force_tool=EXTRACT_TOOL.name,
+        tools=[tool],
+        force_tool=tool.name,
         max_tokens=3072,
         temperature=0.0,
     )
@@ -95,6 +95,10 @@ def run(
     progress: bool = True,
 ) -> dict[str, int]:
     """Extract structured data for every included study."""
+    # Built once, before the loop: the schema comes from the project's
+    # extraction-template note, so a broken template should fail the run
+    # outright rather than erroring on every item in turn.
+    tool = build_extract_tool(project.extraction_template, project=project.name)
     ids = _queue(db, project.name, limit)
     stats = {"extracted": 0, "errors": 0}
 
@@ -102,7 +106,7 @@ def run(
         try:
             snap = load_item(db, zot_id, storage_dir)
             md = to_markdown(snap, include_fulltext=True)
-            call = _build_call(project, md)
+            call = _build_call(project, md, tool)
             result = provider.complete(call)
             if not result.tool_calls:
                 log.warning("no tool_call for item %s — skipping", zot_id)

src/prisma/fetch.py

@@ -139,11 +139,33 @@ def fetch_unpaywall(client: httpx.Client, doi: str, email: str) -> str | None:
 
 
 def fetch_pmc(client: httpx.Client, pmcid: str) -> str | None:
-    """PMC direct PDF URL for a PMC ID."""
+    """PMC PDF URL via the OA Web Service.
+
+    The old ``/pmc/articles/<id>/pdf/`` guess URL now serves an HTML
+    bot-challenge to scripted clients (200, text/html — #657), so ask
+    the sanctioned OA service instead. Only articles in the OA subset
+    resolve; free-to-read author manuscripts return
+    ``idIsNotOpenAccess`` and yield None. FTP hrefs are rewritten to
+    the HTTPS mirror.
+    """
     if not pmcid:
         return None
     pmc = pmcid if pmcid.startswith("PMC") else f"PMC{pmcid}"
-    return f"https://www.ncbi.nlm.nih.gov/pmc/articles/{pmc}/pdf/"
+    try:
+        r = client.get(
+            "https://www.ncbi.nlm.nih.gov/pmc/utils/oa/oa.fcgi",
+            params={"id": pmc, "format": "pdf"},
+            follow_redirects=True,
+        )
+        r.raise_for_status()
+    except httpx.HTTPError:
+        return None
+    m = re.search(r'<link[^>]*href="([^"]+\.pdf)"', r.text)
+    if not m:
+        return None
+    return m.group(1).replace(
+        "ftp://ftp.ncbi.nlm.nih.gov/", "https://ftp.ncbi.nlm.nih.gov/"
+    )
 
 
 def resolve_doi_from_title(client: httpx.Client, title: str) -> str | None:

src/prisma/tools.py

@@ -10,6 +10,8 @@ them when criteria evolve.
 
 from __future__ import annotations
 
+import re
+
 from prisma.llm import LLMTool
 
 # ── Stage 2: title / abstract screening ─────────────────────────
@@ -99,87 +101,137 @@ ELIGIBILITY_TOOL = LLMTool(
 
 # ── Stage 3+: data extraction ───────────────────────────────────
 #
-# Schema mirrors the starter extraction template in project.py. Every
-# field is optional (items that don't report a value get an empty
-# string / 0) so the tool call always succeeds; downstream analysis
-# handles missingness.
+# The extraction form is per-project and lives in Zotero as the
+# ``prisma:extraction-template`` note (see project.py) so reviewers can
+# edit it without touching code. The tool schema is therefore *derived*
+# from that markdown rather than hardcoded: a single hardcoded schema
+# meant every project inherited skin-subs' wound/product fields, which
+# fit palliative-rfi not at all — the LLM was asked for
+# ``wound_type``/``product_category`` on palliative-care papers while the
+# template in front of it described ``rfi_parts`` and
+# ``eligibility_basis``.
+#
+# Template grammar (already used by both starter templates):
+#
+#     ## Section heading          <- ignored; grouping for humans
+#     - **field_name** — description
+#
+# Descriptions that read as a short closed list ("low / some concerns /
+# high") become enums; anything else stays a free string. The rule is
+# deliberately conservative — a mis-read enum would reject a legitimate
+# value at tool-call time, whereas a missed enum only loses a hint.
 
 
-_EXTRACT_FIELD = {"type": "string"}
-_ROB_FIELD = {
-    "type": "string",
-    "enum": ["low", "some-concerns", "high", ""],
-}
-
-
-EXTRACT_TOOL = LLMTool(
-    name="extract_study_data",
-    description=(
-        "Fill the skin-subs data extraction form for an included study. "
-        "Leave absent fields as empty strings; do not fabricate. "
-        "Extract numeric values as written in the paper (preserve units "
-        "where relevant)."
-    ),
-    schema={
-        "type": "object",
-        "properties": {
-            # Study metadata
-            "authors": _EXTRACT_FIELD,
-            "year": _EXTRACT_FIELD,
-            "country": _EXTRACT_FIELD,
-            "funding": {
-                "type": "string",
-                "enum": ["industry", "public", "mixed", "unreported", ""],
-            },
-            "coi_declared": {
-                "type": "string",
-                "enum": ["yes", "no", "partial", ""],
-            },
-            # Population
-            "n_total": _EXTRACT_FIELD,
-            "n_analyzed": _EXTRACT_FIELD,
-            "wound_type": {
-                "type": "string",
-                "enum": ["DFU", "VLU", "pressure", "burn", "mixed", ""],
-            },
-            "baseline_wound_area_cm2": _EXTRACT_FIELD,
-            "inclusion_notes": _EXTRACT_FIELD,
-            # Intervention
-            "product_name": _EXTRACT_FIELD,
-            "manufacturer": _EXTRACT_FIELD,
-            "product_category": {
-                "type": "string",
-                "enum": ["CTP", "synthetic", "xenograft", "amniotic", "other", ""],
-            },
-            "application_frequency": _EXTRACT_FIELD,
-            "concomitant_care": _EXTRACT_FIELD,
-            # Comparator
-            "comparator_type": {
-                "type": "string",
-                "enum": ["standard-care", "active", "placebo", ""],
-            },
-            "comparator_detail": _EXTRACT_FIELD,
-            # Outcomes
-            "primary_outcome": _EXTRACT_FIELD,
-            "primary_result_treatment": _EXTRACT_FIELD,
-            "primary_result_comparator": _EXTRACT_FIELD,
-            "effect_size": _EXTRACT_FIELD,
-            "p_value": _EXTRACT_FIELD,
-            "time_to_closure_days": _EXTRACT_FIELD,
-            "adverse_events": _EXTRACT_FIELD,
-            "cost_per_healed": _EXTRACT_FIELD,
-            # Risk of bias (Cochrane RoB-2)
-            "rob_randomization": _ROB_FIELD,
-            "rob_deviations": _ROB_FIELD,
-            "rob_missing_outcome_data": _ROB_FIELD,
-            "rob_measurement_of_outcome": _ROB_FIELD,
-            "rob_selection_of_reported_result": _ROB_FIELD,
-            "rob_overall": _ROB_FIELD,
-            # Free-form note from the LLM about the extraction
-            "extraction_notes": _EXTRACT_FIELD,
-        },
-        # required kept minimal so an LLM can always respond with a
-        # complete payload even for papers missing most fields
-        "required": ["extraction_notes"],
-    },
+_FIELD_RE = re.compile(
+    r"^\s*[-*]\s+\*\*(?P<name>[A-Za-z0-9_]+)\*\*\s*(?:[—–-]+\s*(?P<desc>.*?))?\s*$"
 )
+
+#: An enum needs at least this many members — two-part descriptions like
+#: "accuracy / discrimination as reported" are prose, not a codebook.
+_MIN_ENUM_MEMBERS = 3
+#: …and every member must be this short. Keeps "OR / RR / HR / mean diff
+#: with 95% CI" a free-text field.
+_MAX_ENUM_WORDS = 3
+
+#: Always present, always required: the model's own caveats about the
+#: extraction. ingest.apply_extraction() renders this one separately.
+NOTES_FIELD = "extraction_notes"
+
+
+def _enum_members(desc: str) -> list[str] | None:
+    """Read ``a / b / c`` as an enum, or return None for free text.
+
+    Parentheses disqualify the whole description ("direct (Medicare
+    data/population) / transferable / weak") because the slash inside
+    the parenthetical is not a separator.
+    """
+    if "/" not in desc or "(" in desc or ")" in desc:
+        return None
+    parts = [p.strip() for p in desc.split("/")]
+    if len(parts) < _MIN_ENUM_MEMBERS:
+        return None
+    if any(not p or "," in p or len(p.split()) > _MAX_ENUM_WORDS for p in parts):
+        return None
+    return parts
+
+
+def parse_extraction_fields(template: str) -> dict[str, dict[str, object]]:
+    """Turn an extraction-template note into JSON-schema properties.
+
+    Descriptions may wrap onto indented continuation lines — both
+    starter templates wrap at ~68 columns, and reading only the first
+    line would truncate a codebook mid-list ("RCT / quasi-experimental
+    / cohort / validation /") and lose the enum with it.
+
+    Later definitions of the same field win, matching how a reviewer
+    would read the note top to bottom.
+    """
+    fields: list[tuple[str, list[str]]] = []
+    current: list[str] | None = None
+    for line in template.splitlines():
+        m = _FIELD_RE.match(line)
+        if m:
+            current = [(m.group("desc") or "").strip()]
+            fields.append((m.group("name"), current))
+        elif current is not None and line.strip() and line[:1].isspace():
+            current.append(line.strip())
+        else:
+            # Blank line, heading, or unindented prose ends the field.
+            current = None
+
+    props: dict[str, dict[str, object]] = {}
+    for name, chunks in fields:
+        desc = " ".join(c for c in chunks if c).strip()
+        prop: dict[str, object] = {"type": "string"}
+        members = _enum_members(desc)
+        if members:
+            # "" so an item that reports nothing still yields a valid
+            # tool call — every field is optional by design.
+            prop["enum"] = [*members, ""]
+        if desc:
+            prop["description"] = desc
+        props[name] = prop
+    return props
+
+
+def build_extract_tool(template: str, *, project: str = "") -> LLMTool:
+    """Build the stage-3+ extraction tool for one project's template.
+
+    Raises ``ValueError`` when the template defines no fields — that is
+    a broken ``prisma:extraction-template`` note, and failing here beats
+    silently extracting nothing for every included study.
+    """
+    props = parse_extraction_fields(template)
+    if not props:
+        raise ValueError(
+            f"extraction template for project {project!r} defines no "
+            f"`- **field** — description` lines; fix the "
+            f"prisma:extraction-template note in Zotero"
+        )
+    props.setdefault(
+        NOTES_FIELD,
+        {
+            "type": "string",
+            "description": (
+                "Noteworthy observations, caveats, or uncertainty about "
+                "this extraction (one to three sentences)."
+            ),
+        },
+    )
+    label = f"the {project} " if project else "the "
+    return LLMTool(
+        name="extract_study_data",
+        description=(
+            f"Fill {label}data extraction form for an included study. "
+            "Leave absent fields as empty strings; do not fabricate. "
+            "Extract numeric values as written in the paper (preserve "
+            "units where relevant)."
+        ),
+        schema={
+            "type": "object",
+            "properties": props,
+            # Kept minimal so the model can always return a complete
+            # payload even for papers missing most fields.
+            "required": [NOTES_FIELD],
+        },
+    )

tests/prisma/test_tools.py

@@ -2,9 +2,159 @@
 
 from __future__ import annotations
 
+import pytest
+
+from prisma.project import STARTERS
+from prisma.tools import (
+    NOTES_FIELD,
+    build_extract_tool,
+    parse_extraction_fields,
+)
+
+_PALLIATIVE_TEMPLATE = STARTERS["palliative-rfi"][1]
+_SKIN_SUBS_TEMPLATE = STARTERS["skin-subs"][1]
+
 
 class TestImports:
     def test_importable(self):
         from prisma.tools import LLMTool
 
         assert LLMTool is not None
+
+
+class TestParseExtractionFields:
+    def test_reads_field_lines(self):
+        props = parse_extraction_fields(
+            "## Study metadata\n"
+            "- **authors** — lead + et al.\n"
+            "- **year** — publication year (YYYY)\n"
+        )
+        assert set(props) == {"authors", "year"}
+        assert props["authors"]["type"] == "string"
+        assert props["authors"]["description"] == "lead + et al."
+
+    def test_ignores_prose_and_headings(self):
+        props = parse_extraction_fields(
+            "# Data extraction template\n\n"
+            "The LLM will fill this form for every included study.\n\n"
+            "## Population\n"
+            "- **n_total** — total enrolled\n"
+        )
+        assert list(props) == ["n_total"]
+
+    def test_bare_field_without_description(self):
+        props = parse_extraction_fields("- **comparator_detail**\n")
+        assert props["comparator_detail"] == {"type": "string"}
+
+    def test_wrapped_description_is_joined(self):
+        props = parse_extraction_fields(
+            "- **design** — RCT / quasi-experimental / cohort /\n"
+            "  systematic review / economic\n"
+        )
+        assert props["design"]["enum"] == [
+            "RCT",
+            "quasi-experimental",
+            "cohort",
+            "systematic review",
+            "economic",
+            "",
+        ]
+
+    def test_blank_line_ends_a_field(self):
+        props = parse_extraction_fields(
+            "- **n_total** — total enrolled\n"
+            "\n"
+            "  stray indented prose after a blank line\n"
+        )
+        assert props["n_total"]["description"] == "total enrolled"
+
+    def test_heading_ends_a_field(self):
+        props = parse_extraction_fields(
+            "- **n_total** — total enrolled\n## Intervention\n"
+        )
+        assert props["n_total"]["description"] == "total enrolled"
+
+    def test_later_definition_wins(self):
+        props = parse_extraction_fields(
+            "- **year** — first\n- **year** — second\n",
+        )
+        assert props["year"]["description"] == "second"
+
+
+class TestEnumDetection:
+    def test_short_closed_list_becomes_enum(self):
+        props = parse_extraction_fields("- **overall** — low / some concerns / high\n")
+        assert props["overall"]["enum"] == ["low", "some concerns", "high", ""]
+
+    def test_empty_string_always_allowed(self):
+        props = parse_extraction_fields("- **coi_declared** — yes / no / partial\n")
+        assert "" in props["coi_declared"]["enum"]
+
+    def test_long_members_stay_free_text(self):
+        # "mean diff with 95% CI" is prose, not a code.
+        props = parse_extraction_fields(
+            "- **effect_size** — OR / RR / HR / mean diff with 95% CI\n"
+        )
+        assert "enum" not in props["effect_size"]
+
+    def test_parenthetical_slash_is_not_a_separator(self):
+        props = parse_extraction_fields(
+            "- **medicare_relevance** — direct (Medicare data/population) / "
+            "transferable / weak\n"
+        )
+        assert "enum" not in props["medicare_relevance"]
+
+    def test_two_part_description_is_prose(self):
+        props = parse_extraction_fields(
+            "- **instrument_performance** — accuracy / discrimination as reported\n"
+        )
+        assert "enum" not in props["instrument_performance"]
+
+    def test_no_slash_is_free_text(self):
+        props = parse_extraction_fields("- **n_total** — total enrolled\n")
+        assert "enum" not in props["n_total"]
+
+
+class TestBuildExtractTool:
+    def test_palliative_schema_follows_its_own_template(self):
+        """The bug: palliative-rfi used to inherit skin-subs' fields."""
+        tool = build_extract_tool(_PALLIATIVE_TEMPLATE, project="palliative-rfi")
+        props = tool.schema["properties"]
+        assert {"rfi_parts", "eligibility_basis", "fwa_findings"} <= set(props)
+        assert not {"wound_type", "product_category", "cost_per_healed"} & set(props)
+
+    def test_skin_subs_schema_still_has_its_fields(self):
+        tool = build_extract_tool(_SKIN_SUBS_TEMPLATE, project="skin-subs")
+        props = tool.schema["properties"]
+        assert {"wound_type", "product_name", "n_total"} <= set(props)
+        assert props["wound_type"]["enum"] == [
+            "DFU",
+            "VLU",
+            "pressure",
+            "burn",
+            "mixed",
+            "",
+        ]
+
+    def test_notes_field_present_and_required(self):
+        tool = build_extract_tool(_PALLIATIVE_TEMPLATE, project="palliative-rfi")
+        assert NOTES_FIELD in tool.schema["properties"]
+        assert tool.schema["required"] == [NOTES_FIELD]
+
+    def test_template_notes_field_is_not_clobbered(self):
+        tool = build_extract_tool(
+            "- **extraction_notes** — reviewer caveats only\n", project="x"
+        )
+        assert (
+            tool.schema["properties"][NOTES_FIELD]["description"]
+            == "reviewer caveats only"
+        )
+
+    def test_tool_name_is_stable(self):
+        tool = build_extract_tool(_PALLIATIVE_TEMPLATE, project="palliative-rfi")
+        assert tool.name == "extract_study_data"
+        assert "palliative-rfi" in tool.description
+
+    def test_empty_template_raises(self):
+        with pytest.raises(ValueError, match="defines no"):
+            build_extract_tool("# Nothing here\n\nJust prose.\n", project="broken")