From ae0173fe84b01b7f2c7373e173c5c3e210ea3535 Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Mon, 10 Aug 2026 15:27:05 -0700 Subject: [PATCH 1/2] fix(biolink): make KGX output validate against the Biolink Model Measured against the 2M-edge corpus in .tablassert/million-kg/ with biolink-model 4.4.3, zero of 2,000,085 edges and zero of 424,141 nodes constructed as their own Biolink class. Nothing in the pipeline ever instantiated Association/NamedThing against an emitted record -- biolink.py derived the *vocabulary* from the model but never checked the output -- so this went unnoticed. Both files are now 100% valid. Retrieval provenance upstream_resource_ids and source_record_urls have `domain: retrieval source`, so they were extra_forbidden on every edge, and primary_knowledge_source was emitted as a list where Biolink wants a scalar. Added retrieval_sources(), mirroring build_association_knowledge_sources() in translator-ingests: a sources[] of RetrievalSource carrying the roles, upstream ids and record urls. Category/predicate contradictions (48.2% of edges) edge_category() picked a class from (subject role, object role) without consulting the predicate, so 723,595 edges were GeneToDiseaseAssociation + gene_associated_with_condition -- a predicate that class forbids. resolve_association_class() now walks the MRO and gives up specificity only as far as the predicate requires. Qualifiers Values were fullmap-resolved even for enum-ranged slots, yielding object_direction_qualifier: "UMLS:C0205217" where the vocabulary wants `increased`. Enum-ranged qualifiers are now passed through unresolved, validated against their vocabulary at config time and at emit time (a column-encoded qualifier carries whatever the sheet holds). prune_to_class() masks per row what the resolved class does not declare and hands the values to the inlined Study rather than deleting them. Study Sheet name, row number and statistics with no Association slot went into supporting_text as "key: value" strings -- a slot whose Biolink meaning is a supporting sentence. They now populate has_supporting_studies (Study -> StudyResult), the COHD/ICEES pattern in translator-ingests. Allow-list _association_model_fields() walked only the base Association MRO, so subclass-only evidence slots (clinical_approval_status, number_of_cases, FDA_regulatory_approvals) were demoted into supporting_text. It now unions the whole association family. Slots declared in the LinkML schema but attached to no Pydantic class are rejected at config time instead of being emitted unvalidatably. Nodes source/source_version were extra_forbidden on every node; they are build provenance, so provided_by carries the infores, taxon becomes in_taxon + in_taxon_label, and the version moves to graph level. resolve_node_class() demotes the 48 categories that cannot be emitted (Publication requires publication_type, ClinicalAttribute requires has_attribute_type, GenomicEntity is a mixin that rejects its own name). Also - p_value/adjusted_p_value emit as JSON numbers, not "6.5200e-06" strings. - Annotations gain `delimiter` so multivalued slots (has_evidence, FDA_regulatory_approvals) emit real arrays instead of joined strings that consumers iterate character by character. - strip_nulls no longer deletes 0 and false, which silently dropped a p_value of 0 and number_of_cases: 0. - New `tablassert validate-kgx` constructs every record as its own Biolink class and exits non-zero on failure, gating this in CI. Nothing is hardcoded to a biolink-model version: UNSATISFIABLE_EDGE_FIELDS is derived from the installed package. Verified by generating the Pydantic model from biolink/biolink-model#1770 -- with it applied the six supporting_study_* slots stop being routed to the StudyResult and are emitted flat on the edge, with no code change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01V6KH4PXbz68F3DSFNAFa63 --- docs/cli.md | 38 ++- rust/src/json.rs | 38 +-- src/tablassert/biolink.py | 433 ++++++++++++++++++++++++++++++-- src/tablassert/cli.py | 31 +++ src/tablassert/errors.py | 3 + src/tablassert/lib.py | 365 ++++++++++++++++++++++++--- src/tablassert/models.py | 84 ++++++- src/tablassert/rig.py | 19 +- tests/test_biolink.py | 80 +++++- tests/test_docs_cli_coverage.py | 2 + tests/test_lib.py | 191 ++++++++------ 11 files changed, 1121 insertions(+), 163 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index 454c09d3..258ce193 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -12,6 +12,7 @@ exposes **four subcommands** — `agent`, `build-fullmap`, `build-kg`, `validate | [`build-fullmap`](#build-fullmap) | Build the embedded fullmap redb used for entity resolution | | [`build-kg`](#build-kg) | Build a KGX NDJSON knowledge graph from a YAML configuration | | [`validate`](#validate) | Validate a graph or table configuration without executing it | +| [`validate-kgx`](#validate-kgx) | Validate built KGX NDJSON against the Biolink Model | ## App flags @@ -32,7 +33,8 @@ These are flags on the root `tablassert` command, **not** subcommands. Use this to autonomously turn one or more PMC articles into audited, improved KG configs and graphs (fetch → derive config → build + audit → improve until coverage maps). Requires the `[agent]` extra -(`pip install tablassert[agent]`). +(`pip install tablassert[agent]`); `--optimize` additionally needs the `[optimize]` extra +(`pip install tablassert[optimize]`, pulls `dspy`). ```bash tablassert agent --fullmap PATH [OPTIONS] PMC-IDS... @@ -163,11 +165,45 @@ tablassert validate graph.yaml --schema graph --- +## validate-kgx + +Use this to check that a completed build is actually Biolink-compliant. Where +[`validate`](#validate) checks your *configuration*, `validate-kgx` checks the *output*: every node +and edge is constructed as the Biolink Pydantic class named by its own `category` — the same classes +[`NCATSTranslator/translator-ingests`](https://github.com/NCATSTranslator/translator-ingests) builds +when it ingests your files. + +```bash +tablassert validate-kgx --nodes MY_KG_1.0.0.nodes.ndjson --edges MY_KG_1.0.0.edges.ndjson +``` + +| Option | Type | Required | Default | Description | +| --- | --- | --- | --- | --- | +| `--nodes`, `-n` | Path | Yes | — | Built `*.nodes.ndjson` file to validate | +| `--edges`, `-e` | Path | Yes | — | Built `*.edges.ndjson` file to validate | +| `--limit` | int | No | `20` | Maximum example failures to retain per file | + +Failures are grouped by field and error type, so a systematic modelling problem shows up as one line +rather than a million: + +```text +biolink-model 4.4.3 +nodes: 424141/424141 valid (0 failures) +edges: 2000085/2000085 valid (0 failures) +KGX output is Biolink-compliant. +``` + +Exits non-zero when any record fails, so it can gate a release in CI. + +--- + ## Typical workflow 1. Author a table config, then a graph config that references it. 2. `tablassert validate graph.yaml --schema graph` — fail fast on schema errors. 3. `tablassert build-kg graph.yaml` — produce KGX NDJSON + RIG (add `--qc` to audit mappings). +4. `tablassert validate-kgx -n MY_KG_1.0.0.nodes.ndjson -e MY_KG_1.0.0.edges.ndjson` — confirm the + output validates against the Biolink Model before shipping it downstream. ## Next Steps diff --git a/rust/src/json.rs b/rust/src/json.rs index 47ba217e..cecbdff5 100644 --- a/rust/src/json.rs +++ b/rust/src/json.rs @@ -1,22 +1,17 @@ -use serde_json::{Map, Number, Value}; +use serde_json::{Map, Value}; fn is_bad_token(text: &str) -> bool { let lowered: String = text.trim().to_ascii_lowercase(); matches!(lowered.as_str(), "" | "na" | "nan" | "null" | "none") } -fn is_zero_number(number: &Number) -> bool { - number.as_i64().is_some_and(|x| x == 0) - || number.as_u64().is_some_and(|x| x == 0) - || number.as_f64().is_some_and(|x| x == 0.0) -} - -// ? Mirrors Python truthiness (`if v`) for JSON values -fn is_truthy(value: &Value) -> bool { +// ? Drops absent values only. Deliberately NOT Python truthiness: `0` and `false` are +// ? meaningful Biolink values (a p_value of 0, `number_of_cases: 0`, `negated: false`), +// ? and treating them as absent silently deletes the key from the emitted record. +fn is_present(value: &Value) -> bool { match value { Value::Null => false, - Value::Bool(flag) => *flag, - Value::Number(number) => !is_zero_number(number), + Value::Bool(_) | Value::Number(_) => true, Value::String(text) => !text.is_empty(), Value::Array(items) => !items.is_empty(), Value::Object(entries) => !entries.is_empty(), @@ -32,7 +27,7 @@ fn passes_bad_check(value: &Value) -> bool { } fn keep(value: &Value) -> bool { - is_truthy(value) && passes_bad_check(value) + is_present(value) && passes_bad_check(value) } // ? Python value transform: lists recurse only into dict items and keep scalars verbatim; @@ -80,14 +75,13 @@ mod tests { use serde_json::json; #[test] - fn strip_nulls_removes_falsey_and_null_like_values() { + fn strip_nulls_removes_absent_and_null_like_values() { let value = json!({ "keep": "BRCA1", "empty": "", "blank": " ", "na": "NA", - "zero": 0, - "false": false, + "null": null, "empty_array": [], "empty_object": {}, "nested": {"drop": "null", "keep": true} @@ -97,6 +91,20 @@ mod tests { assert_eq!(result, json!({"keep": "BRCA1", "nested": {"keep": true}})); } + #[test] + fn strip_nulls_keeps_zero_and_false() { + // ! `0` and `false` are meaningful Biolink values (a p_value of 0, + // ! `number_of_cases: 0`, `negated: false`). Treating them as absent - as + // ! Python truthiness would - silently deletes the key from the record. + let value = json!({"p_value": 0, "number_of_cases": 0, "negated": false, "rate": 0.0}); + + let result = strip_nulls(&value); + assert_eq!( + result, + json!({"p_value": 0, "number_of_cases": 0, "negated": false, "rate": 0.0}) + ); + } + #[test] fn strip_nulls_keeps_emptied_nested_dict_and_list_scalars() { // ! Faithful Python semantics: a nested dict that empties stays as {}, and list diff --git a/src/tablassert/biolink.py b/src/tablassert/biolink.py index 1be3dec7..f5126354 100644 --- a/src/tablassert/biolink.py +++ b/src/tablassert/biolink.py @@ -42,21 +42,27 @@ from __future__ import annotations import inspect +import json import re +from collections import Counter from enum import Enum from functools import cache from importlib.resources import files -from typing import TYPE_CHECKING, Any, cast +from typing import TYPE_CHECKING, Any, Literal, cast, get_args, get_origin import biolink_model.datamodel.pydanticmodel_v2 as _bm if TYPE_CHECKING: + from pathlib import Path + from linkml_runtime.utils.schemaview import SchemaView __all__ = [ "ALLOWED_EDGE_FIELDS", "BIOLINK_VERSION", "EFFECT_TYPE_VALUES", + "ENUM_RANGED_QUALIFIERS", + "UNSATISFIABLE_EDGE_FIELDS", "AgentTypes", "Categories", "EdgeCategories", @@ -64,6 +70,16 @@ "KnowledgeLevels", "Predicates", "Qualifiers", + "association_class", + "class_fields", + "is_multivalued", + "node_class", + "numeric_slot_kind", + "resolve_association_class", + "resolve_node_category", + "resolve_node_class", + "validate_kgx", + "validate_record", ] @@ -216,13 +232,33 @@ def _qualifier_values() -> list[str]: return sorted({_snake(q) for q in (_slot_descendants("qualifier") | {"qualifier"})}) +@cache +def _association_classes() -> tuple[type[Any], ...]: + """Every Biolink ``Association`` Pydantic class, including ``Association`` itself. + + Association subclasses declare slots the base class does not (for example + ``clinical_approval_status`` on ``EntityToDiseaseAssociation``). Callers that + reason about "any field a Tablassert edge might legitimately carry" must + consider the whole family, not just the base MRO. + + Returns: + Tuple of association classes defined in ``pydanticmodel_v2``. + """ + return tuple(cls for cls in vars(_bm).values() if inspect.isclass(cls) and cls.__module__ == _bm.__name__ and issubclass(cls, _bm.Association)) + + def _association_model_fields() -> set[str]: - """All field names on the Biolink ``Association`` Pydantic class, including inherited ones.""" + """All field names declared by *any* Biolink ``Association`` class. + + Unions ``model_fields`` across the entire association family rather than only + walking ``Association.__mro__``. Walking the base MRO alone silently excludes + subclass-only evidence slots -- ``clinical_approval_status``, + ``number_of_cases``, ``FDA_regulatory_approvals`` -- which then get demoted + into ``supporting_text`` by :func:`lib.fold_unknown_to_supporting_text`. + """ fields: set[str] = set() - for klass in _bm.Association.__mro__: - model_fields: Any = getattr(klass, "model_fields", None) - if model_fields: - fields |= set(model_fields.keys()) + for klass in _association_classes(): + fields |= set(klass.model_fields.keys()) return fields @@ -264,13 +300,124 @@ def _biolink_enum_values(enum_cls: type[Enum]) -> list[str]: ) -# Edge columns Tablassert / KGX emit that are neither Biolink ``Association`` model -# fields nor qualifier slot names: synonym carryover from NamedThing, KGX provenance -# and denormalized fields, supporting-study evidence slots, and Tablassert pipeline -# fields (``source_record_urls``, ``upstream_resource_ids``). Kept curated and unioned -# with the derived Biolink fields so ``ALLOWED_EDGE_FIELDS`` is always a superset of -# what the pipeline may emit (so ``lib.fold_unknown_to_supporting_text`` never starts -# folding legitimate edge columns into ``supporting_text``). +def _annotation_choices(annotation: Any) -> frozenset[str] | None: + """Return the closed value set of a Pydantic field annotation, or ``None`` if open. + + Biolink constrains some slots with a generated ``Enum`` (``DirectionQualifierEnum``) + and others with a ``Literal[...]``. Both are closed vocabularies; a bare ``str`` + annotation is open. ``Optional[...]`` / ``list[...]`` wrappers are unwrapped. + + Args: + annotation: A Pydantic ``FieldInfo.annotation``. + + Returns: + Frozenset of permitted string values, or ``None`` when unconstrained. + """ + if annotation is None or annotation is str: + return None + if isinstance(annotation, type) and issubclass(annotation, Enum): + return frozenset(str(member.value) for member in annotation) + origin: Any = get_origin(annotation) + if origin is Literal: + return frozenset(str(arg) for arg in get_args(annotation)) + args: tuple[Any, ...] = get_args(annotation) + if args: + # Optional[X] / list[X] / Union[...]: a closed set anywhere makes the slot closed. + choices: set[str] = set() + found: bool = False + for arg in args: + if arg is type(None): + continue + nested: frozenset[str] | None = _annotation_choices(arg) + if nested is not None: + choices |= nested + found = True + if found: + return frozenset(choices) + return None + + +@cache +def _field_owners() -> dict[str, frozenset[str]]: + """Map every Pydantic field name to the set of Biolink classes that declare it. + + Used to distinguish slots that exist in the LinkML YAML but were never attached + to a class (and so can never be serialized) from ones with a real home. + """ + owners: dict[str, set[str]] = {} + for cls in vars(_bm).values(): + if not (inspect.isclass(cls) and cls.__module__ == _bm.__name__): + continue + for field in getattr(cls, "model_fields", {}): + owners.setdefault(field, set()).add(cls.__name__) + return {field: frozenset(names) for field, names in owners.items()} + + +def _predicate_accepts(cls: type[Any], predicate: str) -> bool: + """Whether an association class permits ``predicate`` on its ``predicate`` slot.""" + field: Any = cls.model_fields.get("predicate") + if field is None: + return False + choices: frozenset[str] | None = _annotation_choices(field.annotation) + return choices is None or predicate in choices + + +@cache +def association_class(category: str) -> type[Any]: + """Resolve a ``biolink:X`` edge category CURIE to its Pydantic class. + + Falls back to ``Association`` for unknown or malformed categories. + """ + name: str = category.removeprefix("biolink:") + cls: Any = getattr(_bm, name, None) + if inspect.isclass(cls) and issubclass(cls, _bm.Association): + return cls + return _bm.Association + + +@cache +def resolve_association_class(category: str, predicate: str) -> type[Any]: + """Pick the most specific association class that actually permits ``predicate``. + + Tablassert derives a candidate edge category from the (subject role, object role) + pair without consulting the predicate, which routinely produces contradictions -- + ``GeneToDiseaseAssociation`` restricts ``predicate`` to + ``contributes_to|associated_with|affects``, so a + ``biolink:gene_associated_with_condition`` edge labelled with that category can + never validate. + + Walks the candidate's MRO most-specific-first and returns the first association + ancestor whose ``predicate`` slot accepts the value, so specificity is only ever + given up as far as correctness requires. ``Association`` (open ``predicate``) is + the guaranteed floor. + + Args: + category: Candidate edge category CURIE (``"biolink:GeneToDiseaseAssociation"``). + predicate: Predicate CURIE (``"biolink:gene_associated_with_condition"``). + + Returns: + The resolved association class. + """ + for ancestor in association_class(category).__mro__: + if inspect.isclass(ancestor) and issubclass(ancestor, _bm.Association) and _predicate_accepts(ancestor, predicate): + return ancestor + return _bm.Association + + +# Edge columns Tablassert emits that are not fields of any Biolink ``Association`` +# class: synonym carryover from NamedThing and KGX denormalized fields. Unioned with +# the derived Biolink fields so ``ALLOWED_EDGE_FIELDS`` stays a superset of what the +# pipeline may legitimately emit (so ``lib.fold_unknown_to_supporting_text`` never +# folds a real edge column into ``supporting_text``). +# +# Deliberately NOT listed here, because none of them can be serialized onto an edge: +# - ``source_record_urls`` / ``upstream_resource_ids`` -- ``domain: retrieval source``, +# so they belong inside a ``sources`` entry, not on the association. +# - ``supporting_study_*``, ``statistical_significance_qualifier``, +# ``relationship_strength`` -- declared in the LinkML YAML but attached to zero +# Pydantic classes (see ``UNSATISFIABLE_EDGE_FIELDS``); they are routed onto the +# inlined ``Study`` / ``StudyResult`` instead. +# - ``taxon`` -- a node property; edges carry ``species_context_qualifier``. TABLASERT_EDGE_EXTRAS: frozenset[str] = frozenset( [ "broad_synonym", @@ -289,18 +436,8 @@ def _biolink_enum_values(enum_cls: type[Enum]) -> list[str]: "provided_by", "related_synonym", "relation", - "source_record_urls", - "statistical_significance_qualifier", "supporting_documents", - "supporting_study_cohort", - "supporting_study_context", - "supporting_study_date_range", - "supporting_study_method_description", - "supporting_study_method_types", - "supporting_study_size", "synonym", - "taxon", - "upstream_resource_ids", "xref", ] ) @@ -314,6 +451,7 @@ def _biolink_enum_values(enum_cls: type[Enum]) -> list[str]: class Categories(str, Enum): DISEASE: Categories GENE: Categories + NAMED_THING: Categories PHENOTYPIC_FEATURE: Categories PROTEIN: Categories @@ -358,11 +496,254 @@ class EffectTypes(str, Enum): BIOLINK_VERSION: str = str(_schema_definition.version) if _schema_definition is not None else "unknown" """Version of the Biolink Model these values were derived from (e.g. ``"4.4.3"``).""" -ALLOWED_EDGE_FIELDS: frozenset[str] = frozenset(_association_model_fields()) | {q.value for q in Qualifiers} | TABLASERT_EDGE_EXTRAS +UNSATISFIABLE_EDGE_FIELDS: frozenset[str] = frozenset(q.value for q in Qualifiers if q.value not in _field_owners()) | frozenset( + field + for field in ( + "relationship_strength", + "sample_size", + "statistical_significance_qualifier", + "supporting_study_cohort", + "supporting_study_context", + "supporting_study_date_range", + "supporting_study_method_description", + "supporting_study_method_types", + "supporting_study_size", + ) + if field not in _field_owners() +) +"""Slot names that exist in the Biolink LinkML schema but on no Pydantic class. + +``Qualifiers`` is derived from the LinkML *slot* hierarchy, which is strictly broader +than the set of slots actually attached to a class. Emitting one of these produces a +record that can never validate, so configs referencing them are rejected up front and +their values are routed onto the inlined ``StudyResult`` instead. +""" + + +ENUM_RANGED_QUALIFIERS: dict[str, frozenset[str]] = { + qualifier.value: choices + for qualifier in Qualifiers + for owners in (_field_owners().get(qualifier.value, frozenset()),) + if owners + for choices in ( + frozenset().union( + *( + _annotation_choices(getattr(_bm, owner).model_fields[qualifier.value].annotation) or frozenset() + for owner in owners + if hasattr(_bm, owner) + ) + ), + ) + if choices +} +"""Qualifier slots whose range is a closed vocabulary rather than a CURIE. + +``Qualifier`` config entries inherit ``NodeEncoding`` and are therefore entity-resolved +through the fullmap by default. That is correct for CURIE-ranged qualifiers +(``anatomical_context_qualifier`` -> ``UBERON:0001557``) but wrong for enum-ranged ones: +``object_direction_qualifier`` wants the token ``increased``, not ``UMLS:C0205217``. +Values for these slots are validated against the vocabulary and passed through +unresolved. +""" + + +ALLOWED_EDGE_FIELDS: frozenset[str] = ( + frozenset(_association_model_fields()) | {q.value for q in Qualifiers} | TABLASERT_EDGE_EXTRAS +) - UNSATISFIABLE_EDGE_FIELDS """Authoritative biolink-compliant edge column allow-list. Any column on an edge frame that is not in this set is folded into the ``supporting_text`` ``list[str]`` field by ``lib.fold_unknown_to_supporting_text()`` -as a ``"column: value"`` string. Composed of the derived Biolink ``Association`` -fields, the derived qualifier slot names, and the curated ``TABLASERT_EDGE_EXTRAS``. +as a ``"column: value"`` string. Composed of the fields declared by *any* Biolink +association class, the derived qualifier slot names, and the curated +``TABLASERT_EDGE_EXTRAS`` -- less the slots that no Pydantic class can hold. + +Note this is a per-*family* allow-list: a field being permitted here does not mean the +specific association class chosen for a given edge accepts it. Per-record pruning +against the resolved class is done by ``lib.prune_to_class()``. """ + + +@cache +def node_class(category: str) -> type[Any]: + """Resolve a ``biolink:X`` node category CURIE to its Pydantic class. + + Mirrors ``bmt.pydantic.get_node_class`` (used by ``translator-ingests``) without + taking on the ``bmt`` dependency: Tablassert already knows the exact category it + assigned, so a direct lookup is sufficient. Falls back to ``NamedThing``. + """ + name: str = category.removeprefix("biolink:") + cls: Any = getattr(_bm, name, None) + if inspect.isclass(cls) and issubclass(cls, _bm.NamedThing): + return cls + return _bm.NamedThing + + +def class_fields(cls: type[Any]) -> frozenset[str]: + """Field names a Pydantic class accepts (cached per class by the caller).""" + return frozenset(cls.model_fields.keys()) + + +def is_multivalued(cls: type[Any], field: str) -> bool: + """Whether ``cls`` declares ``field`` as a list-valued slot. + + Biolink makes the same qualifier multivalued on some association classes and + scalar on others, so a value carried across a class change may need wrapping. + """ + info: Any = cls.model_fields.get(field) + if info is None: + return False + annotation: Any = info.annotation + if get_origin(annotation) is list: + return True + return any(get_origin(arg) is list for arg in get_args(annotation)) + + +def validate_record(record: dict[str, Any], *, edge: bool) -> list[str]: + """Validate one KGX record against the Biolink class named by its ``category``. + + Args: + record: A decoded NDJSON node or edge record. + edge: ``True`` to dispatch on the association family, ``False`` for nodes. + + Returns: + A list of ``"field: error-type"`` strings; empty when the record validates. + """ + from pydantic import ValidationError + + categories: Any = record.get("category") or [] + category: str = categories[0] if isinstance(categories, list) and categories else str(categories or "") + cls: type[Any] = association_class(category) if edge else node_class(category) + try: + cls(**record) + except ValidationError as error: + return [f"{'.'.join(str(part) for part in item['loc']) or '?'}: {item['type']}" for item in error.errors()] + return [] + + +def _scalar_types(annotation: Any) -> set[type]: + """Unwrap ``Optional`` / ``list`` / ``Union`` down to the concrete scalar types.""" + if isinstance(annotation, type): + return {annotation} + out: set[type] = set() + for arg in get_args(annotation): + if arg is type(None): + continue + out |= _scalar_types(arg) + return out + + +@cache +def numeric_slot_kind(field: str) -> str | None: + """Return ``"int"`` / ``"float"`` when a Biolink association slot has a numeric range. + + Tablassert stringifies its numeric annotation columns for notation control, but + Biolink types ``p_value`` and ``adjusted_p_value`` as ``float`` and (with + ``biolink/biolink-model#1770``) ``supporting_study_size`` as ``int``. Those must be + emitted as real JSON numbers. Derived from the installed model so the answer + tracks whatever version is pinned. + + Args: + field: Edge column name. + + Returns: + ``"int"``, ``"float"``, or ``None`` when the slot is not numeric (or unknown). + """ + kinds: set[type] = set() + for cls in _association_classes(): + info: Any = cls.model_fields.get(field) + if info is not None: + kinds |= _scalar_types(info.annotation) + if float in kinds: + return "float" + if int in kinds and bool not in kinds: + return "int" + return None + + +def validate_kgx(nodes_path: Path, edges_path: Path, limit: int = 20) -> dict[str, Any]: + """Validate emitted KGX NDJSON files against the Biolink Pydantic model. + + Tablassert derives its *vocabulary* from the model but historically never + instantiated a Biolink class against an emitted record, so a build could -- and did + -- ship files where no record validated. This closes that loop: every node and edge + is constructed as the class named by its own ``category``. + + Args: + nodes_path: Path to ``_.nodes.ndjson``. + edges_path: Path to ``_.edges.ndjson``. + limit: Maximum number of example failures to retain per file. + + Returns: + Mapping with per-file ``total`` / ``valid`` / ``failures`` counts, a + ``problems`` histogram keyed by ``"field: error-type"``, up to ``limit`` + ``examples``, and a top-level ``ok`` flag. + """ + report: dict[str, Any] = {"biolink_version": BIOLINK_VERSION, "ok": True} + for label, path, edge in (("nodes", nodes_path, False), ("edges", edges_path, True)): + total: int = 0 + valid: int = 0 + problems: Counter[str] = Counter() + examples: list[dict[str, Any]] = [] + if path.is_file(): + with path.open(encoding="utf-8") as handle: + for line in handle: + if not line.strip(): + continue + total += 1 + record: dict[str, Any] = json.loads(line) + errors: list[str] = validate_record(record, edge=edge) + if not errors: + valid += 1 + continue + problems.update(errors) + if len(examples) < limit: + examples.append({"id": record.get("id"), "errors": errors}) + report[label] = {"total": total, "valid": valid, "failures": total - valid, "problems": dict(problems.most_common()), "examples": examples} + if total != valid: + report["ok"] = False + return report + + +# Node slots Tablassert can always populate from a fullmap hit. A class requiring +# anything outside this set cannot be emitted, because there is no source for the value. +FILLABLE_NODE_FIELDS: frozenset[str] = frozenset({"id", "name", "category", "provided_by", "in_taxon", "in_taxon_label"}) + + +@cache +def resolve_node_class(category: str) -> type[Any]: + """Pick the most specific node class Tablassert can actually emit for ``category``. + + Entity resolution can land on a class that is unusable as a KGX node: ``Publication`` + requires ``publication_type`` and ``ClinicalAttribute`` requires + ``has_attribute_type`` -- neither of which a fullmap hit provides -- while mixins + such as ``GenomicEntity`` reject their own name in the ``category`` literal. + + Walks the MRO most-specific-first and returns the first class whose required fields + are all fillable and whose ``category`` vocabulary admits its own name, so + specificity is only given up as far as correctness requires. ``NamedThing`` is the + guaranteed floor. + + Args: + category: Candidate node category CURIE (``"biolink:Publication"``). + + Returns: + The resolved node class; emit ``category`` from its own default. + """ + for ancestor in node_class(category).__mro__: + if not (inspect.isclass(ancestor) and issubclass(ancestor, _bm.NamedThing)): + continue + required: set[str] = {name for name, info in ancestor.model_fields.items() if info.is_required()} + if not required <= FILLABLE_NODE_FIELDS: + continue + choices: frozenset[str] | None = _annotation_choices(ancestor.model_fields["category"].annotation) + if choices is not None and f"biolink:{ancestor.__name__}" not in choices: + continue + return ancestor + return _bm.NamedThing + + +@cache +def resolve_node_category(category: str) -> str: + """Resolve a node category CURIE to one that can actually be emitted.""" + return f"biolink:{resolve_node_class(category).__name__}" diff --git a/src/tablassert/cli.py b/src/tablassert/cli.py index 730c3323..60578d71 100644 --- a/src/tablassert/cli.py +++ b/src/tablassert/cli.py @@ -494,6 +494,37 @@ def validate( run(3, validate_pipeline, configuration_file) +@APP.command(name="validate-kgx") +def validate_kgx_command( + nodes: Annotated[Path, cyclopts.Parameter(name=["--nodes", "-n"])], + edges: Annotated[Path, cyclopts.Parameter(name=["--edges", "-e"])], + limit: Annotated[int, cyclopts.Parameter(name=["--limit"])] = 20, +) -> None: + """Validate built KGX NDJSON against the Biolink Model. + + Constructs every node and edge as the Biolink Pydantic class named by its own + ``category`` -- the same classes ``NCATSTranslator/translator-ingests`` builds -- + and reports failures grouped by field and error type. Exits non-zero when any + record fails, so a build can be gated in CI. + """ + from tablassert.biolink import validate_kgx + + report: dict[str, Any] = validate_kgx(nodes, edges, limit=limit) + print(f"biolink-model {report['biolink_version']}", file=sys.stderr) + for label in ("nodes", "edges"): + section: dict[str, Any] = report[label] + print(f"{label}: {section['valid']}/{section['total']} valid ({section['failures']} failures)", file=sys.stderr) + for problem, count in section["problems"].items(): + print(f" {count:>9} {problem}", file=sys.stderr) + for example in section["examples"][:3]: + print(f" e.g. {example['id']}: {', '.join(example['errors'])}", file=sys.stderr) + logger.info(f"validate-kgx {label}: {section['valid']}/{section['total']} valid") + if not report["ok"]: + print("KGX output is not Biolink-compliant.", file=sys.stderr) + raise SystemExit(1) + print("KGX output is Biolink-compliant.", file=sys.stderr) + + @APP.command(name="agent") def agent( pmc_ids: Annotated[list[str], cyclopts.Parameter(allow_leading_hyphen=False)], diff --git a/src/tablassert/errors.py b/src/tablassert/errors.py index 63b325bf..536697bd 100644 --- a/src/tablassert/errors.py +++ b/src/tablassert/errors.py @@ -23,7 +23,10 @@ "provenance-bad-pmc-id", "provenance-missing-publication", "provenance-publication-and-override", + "annotation-bad-delimiter", "qualifier-auto-derived", + "qualifier-bad-value", + "qualifier-unsatisfiable", ] diff --git a/src/tablassert/lib.py b/src/tablassert/lib.py index 67f51658..799210f9 100644 --- a/src/tablassert/lib.py +++ b/src/tablassert/lib.py @@ -12,7 +12,19 @@ from tablassert import rs from tablassert._lazy import LazyModule -from tablassert.biolink import ALLOWED_EDGE_FIELDS, Categories, EdgeCategories +from tablassert.biolink import ( + ALLOWED_EDGE_FIELDS, + ENUM_RANGED_QUALIFIERS, + UNSATISFIABLE_EDGE_FIELDS, + Categories, + EdgeCategories, + association_class, + class_fields, + is_multivalued, + numeric_slot_kind, + resolve_association_class, + resolve_node_category, +) from tablassert.coerce import ( coerce_effect_size_columns, coerce_effect_type_columns, @@ -27,7 +39,7 @@ from tablassert.enums import EncodingMethods, Files, InformationResources, Repositories, Tokens from tablassert.fullmap import ResolveSpec, fullmap_db_path, resolve, resolve_batch from tablassert.log import cat -from tablassert.models import Encoding, NodeEncoding, Section +from tablassert.models import Encoding, NodeEncoding, Qualifier, Section from tablassert.nlp import level_one, level_two from tablassert.qc import fullmap_audit from tablassert.rig import ( @@ -154,12 +166,21 @@ def edge_tables() -> tuple[dict[str, str], dict[str, str]]: return CATEGORY_ROLE, EDGE_LOOKUP -def edge_category(lf: pl.LazyFrame) -> pl.LazyFrame: +def edge_category(lf: pl.LazyFrame, predicate: str | None = None) -> pl.LazyFrame: """Add the derived ``category`` column using native polars replace operations. + The ``(subject role, object role)`` lookup alone routinely produces a category + that contradicts the predicate: ``GeneToDiseaseAssociation`` restricts its + ``predicate`` slot to ``contributes_to|associated_with|affects``, so a + ``biolink:gene_associated_with_condition`` edge labelled with that category can + never validate. When ``predicate`` is supplied the raw category is post-resolved + by :func:`biolink.resolve_association_class`, which walks up the association + hierarchy only as far as the predicate requires. + Args: lf: Source LazyFrame with ``subject category`` and ``object category`` columns (biolink-prefixed). + predicate: Section predicate CURIE used to reconcile the derived category. Returns: LazyFrame with a new list-typed ``category`` column containing the @@ -169,16 +190,83 @@ def edge_category(lf: pl.LazyFrame) -> pl.LazyFrame: cat_role: dict[str, str] edge_lookup: dict[str, str] cat_role, edge_lookup = edge_tables() + default: str = f"biolink:{EdgeCategories.ASSOCIATION.value}" + if predicate: + # The predicate is a section constant, so the reconciliation collapses to a + # small raw-category -> resolved-category remap resolved once at plan time. + edge_lookup = {k: f"biolink:{resolve_association_class(v, predicate).__name__}" for k, v in edge_lookup.items()} + default = f"biolink:{resolve_association_class(default, predicate).__name__}" names: list[str] = lf.collect_schema().names() subject_col: str = "subject_category" if "subject_category" in names else "subject category" object_col: str = "object_category" if "object_category" in names else "object category" sr: pl.Expr = pl.col(subject_col).str.replace("biolink:", "").replace(cat_role).fill_null("") or_: pl.Expr = pl.col(object_col).str.replace("biolink:", "").replace(cat_role).fill_null("") - return lf.with_columns( - pl.concat_list( - pl.concat_str([sr, pl.lit("|"), or_]).replace_strict(edge_lookup, default=f"biolink:{EdgeCategories.ASSOCIATION.value}") - ).alias("category") - ) + return lf.with_columns(pl.concat_list(pl.concat_str([sr, pl.lit("|"), or_]).replace_strict(edge_lookup, default=default)).alias("category")) + + +def prune_to_class(lf: pl.LazyFrame) -> pl.LazyFrame: + """Null out edge columns the row's own association class does not declare. + + ``ALLOWED_EDGE_FIELDS`` is a per-*family* allow-list: it says a column is a slot + of *some* association class. Whether the specific class chosen for a given row + accepts it is a separate question, and getting it wrong is the single largest + source of ``extra_forbidden`` failures (``species_context_qualifier`` and friends + on a class that has no such slot). + + Categories vary per row within a section, so this masks per row rather than + dropping columns: values are nulled where the row's class rejects them, and the + Rust null-stripper then removes the key entirely. Scalars are wrapped where the + class declares the slot multivalued. + + Args: + lf: Edges LazyFrame carrying a resolved ``category`` column. + + Returns: + LazyFrame whose every remaining value is legal for its own row's class. + """ + schema: pl.Schema = lf.collect_schema() + names: list[str] = schema.names() + if "category" not in names: + return lf + + core: frozenset[str] = frozenset({"category", "subject", "object", "predicate", "id"}) + candidates: list[str] = [c for c in names if c not in core] + if not candidates: + return lf + + categories: list[str] = [f"biolink:{c.value}" for c in EdgeCategories] + first: pl.Expr = pl.col("category").list.first() + updates: list[pl.Expr] = [] + rescued: list[pl.Expr] = [] + for col in candidates: + text: pl.Expr = ( + pl.col(col).list.eval(pl.element().cast(pl.String)).list.join(", ") if isinstance(schema[col], pl.List) else pl.col(col).cast(pl.String) + ) + accepts: dict[str, bool] = {cat: col in class_fields(association_class(cat)) for cat in categories} + # A closed-vocabulary slot additionally constrains the *value*. A qualifier + # encoded from a column carries whatever the sheet holds, so the token can only + # be checked here -- config-time validation sees no data. + vocabulary: frozenset[str] | None = ENUM_RANGED_QUALIFIERS.get(col) + ok: pl.Expr = first.replace_strict(accepts, default=False) if not all(accepts.values()) else pl.lit(True) + if vocabulary is not None: + ok = ok & text.is_in(list(vocabulary)) + if all(accepts.values()) and vocabulary is None: + keep: pl.Expr = pl.col(col) + elif not any(accepts.values()): + continue # Handled upstream by the allow-list / study routing. + else: + keep = pl.when(ok).then(pl.col(col)).otherwise(None) + # Preserve what the class refuses rather than deleting it outright; the + # value is real evidence, it just has no slot on this association class. + rescued.append(pl.when(ok | text.is_null()).then(None).otherwise(pl.concat_str([pl.lit(f"{col}="), text]))) + # Biolink makes the same slot multivalued on some classes and scalar on others. + listed: dict[str, bool] = {cat: is_multivalued(association_class(cat), col) for cat in categories} + if any(listed.values()) and not isinstance(schema[col], pl.List): + keep = pl.when(first.replace_strict(listed, default=False)).then(pl.concat_list(keep)).otherwise(keep) + updates.append(keep.alias(col)) + if rescued: + updates.append(pl.concat_list(rescued).list.drop_nulls().alias(PRUNED_COLUMN)) + return lf.with_columns(updates) if updates else lf def value(lf: pl.LazyFrame, col: str, x: object) -> pl.LazyFrame: @@ -211,17 +299,126 @@ def derive_species_context(lf: pl.LazyFrame) -> pl.LazyFrame: return lf.with_columns(pl.coalesce(pl.col("subject_taxon"), pl.col("object_taxon")).alias("species_context_qualifier")) -def source_record_urls(lf: pl.LazyFrame, url: str) -> pl.LazyFrame: - """Add Biolink/Translator ``source_record_urls`` as a single-element list column. +def _retrieval_source(resource_id: str, resource_role: str, upstream: list[str] | None = None, urls: list[str] | None = None) -> pl.Expr: + """Build one ``RetrievalSource`` struct expression. + + Every entry declares the same four fields so that :func:`retrieval_sources` can + ``concat_list`` them into a single ``list[struct]`` column; absent list fields are + typed nulls, which the Rust null-stripper removes from the emitted JSON. + """ + empty: pl.Expr = pl.lit(None, dtype=pl.List(pl.String)) + return pl.struct( + # RetrievalSource.id is required; translator-ingests sets it to the resource_id. + pl.lit(resource_id).alias("id"), + pl.lit(resource_id).alias("resource_id"), + pl.lit(resource_role).alias("resource_role"), + (pl.concat_list([pl.lit(x) for x in upstream]) if upstream else empty).alias("upstream_resource_ids"), + (pl.concat_list([pl.lit(x) for x in urls]) if urls else empty).alias("source_record_urls"), + ) + + +PRUNED_COLUMN: str = "_pruned_by_class" +"""Internal handoff column: values `prune_to_class` removed, for the study to absorb. + +Never reaches output -- :func:`inline_supporting_study` folds it into the +``StudyResult`` description and drops it. +""" + + +def inline_supporting_study(lf: pl.LazyFrame, study_id: str, sheet: str | None) -> pl.LazyFrame: + """Attach table provenance and homeless statistics as an inlined Biolink ``Study``. + + Follows the COHD/ICEES pattern in ``translator-ingests``: the edge carries + ``has_supporting_studies`` (``dict[str, Study]``, ``inlined: true`` on + ``Association``) and each ``Study`` carries ``has_study_results``. The Study is + deliberately *not* written to the nodes file, matching those ingests. + + Two kinds of column are routed here rather than left on the edge: + + * the sheet name and source row number, which previously became + ``"sheet_name: Table_S7"`` strings inside ``supporting_text`` -- a slot whose + Biolink meaning is a supporting sentence, not a key/value dump; + * any column in :data:`biolink.UNSATISFIABLE_EDGE_FIELDS`, i.e. declared in the + LinkML schema but attached to no Pydantic class under the *installed* + biolink-model. With ``biolink/biolink-model#1770`` applied the + ``supporting_study_*`` slots become real ``Association`` fields and are left + flat on the edge instead; nothing here is hardcoded to either state. + + ``study_id`` is a per-section constant, so it can key a static struct field. + + Args: + lf: Edges LazyFrame after annotation and provenance ops. + study_id: Stable study identifier (``"#"``). + sheet: Worksheet name, when the source is a spreadsheet. + + Returns: + LazyFrame with ``has_supporting_studies`` appended and the routed columns dropped. + """ + names: list[str] = lf.collect_schema().names() + routed: list[str] = sorted(c for c in names if c in UNSATISFIABLE_EDGE_FIELDS) + row: str = "extracted_from_row_number" + has_row: bool = row in names + + result_id: pl.Expr = pl.concat_str([pl.lit(f"{study_id}#row"), pl.col(row).cast(pl.String)]) if has_row else pl.lit(f"{study_id}#result") + label: str = f"{sheet} row " if sheet else "row " + result_name: pl.Expr = pl.concat_str([pl.lit(label), pl.col(row).cast(pl.String)]) if has_row else pl.lit(sheet or study_id) + + # Statistics with no Association slot are preserved as a readable summary rather + # than silently dropped; `StudyResult.has_attribute` is `list[str]` (not inlined), + # so typed Attributes would require emitting Attribute rows into the nodes file. + fields: list[pl.Expr] = [result_id.alias("id"), result_name.alias("name")] + pruned: bool = PRUNED_COLUMN in names + if routed or pruned: + parts: list[pl.Expr] = [] + for col in routed: + text: pl.Expr = pl.col(col).cast(pl.String).str.strip_chars() + blank: pl.Expr = text.is_null() | (text.str.len_chars() == 0) + parts.append(pl.when(blank).then(pl.lit(None, dtype=pl.String)).otherwise(pl.concat_str([pl.lit(f"{col}="), text]))) + # Qualifiers the resolved association class refuses (see `prune_to_class`) are + # appended to the routed statistics. Build from whichever sources exist: an + # empty list literal would be a zero-length series and fail to broadcast. + summary: pl.Expr + if parts and pruned: + summary = pl.concat_list(parts).list.drop_nulls().list.concat(pl.col(PRUNED_COLUMN)) + elif parts: + summary = pl.concat_list(parts) + else: + summary = pl.col(PRUNED_COLUMN) + fields.append(summary.list.drop_nulls().list.join("; ").alias("description")) + + study: pl.Expr = pl.struct( + pl.lit(study_id).alias("id"), pl.lit(sheet or study_id).alias("name"), pl.concat_list(pl.struct(fields)).alias("has_study_results") + ) + out: pl.LazyFrame = lf.with_columns(pl.struct(study.alias(study_id)).alias("has_supporting_studies")) + drop: list[str] = [*routed, *([row] if has_row else []), *(["sheet_name"] if "sheet_name" in names else []), *([PRUNED_COLUMN] if pruned else [])] + return out.drop(drop) + + +def retrieval_sources(lf: pl.LazyFrame, primary: str, upstream: list[str], urls: list[str]) -> pl.LazyFrame: + """Add the Biolink ``sources`` retrieval-provenance column. + + ``upstream_resource_ids`` and ``source_record_urls`` have ``domain: retrieval + source`` in the Biolink Model, so they are properties of an entry in ``sources`` + -- not of the association. Emitting them flat on the edge makes every record fail + validation with ``extra_forbidden``. + + Mirrors ``build_association_knowledge_sources()`` from + ``translator-ingests/util/biolink.py``: the primary knowledge source carries the + source record URLs and lists the upstream resources, and each upstream resource + additionally appears as its own ``supporting_data_source`` entry. Args: lf: Source LazyFrame. - url: Source record URL to record for every row. + primary: Infores CURIE of the primary knowledge source. + upstream: Infores CURIEs of upstream/supporting data sources. + urls: Source record URLs for the primary entry. Returns: - LazyFrame with the new list column appended. + LazyFrame with a ``sources`` ``list[struct]`` column appended. """ - return lf.with_columns(pl.concat_list(pl.lit(url)).alias("source_record_urls")) + entries: list[pl.Expr] = [_retrieval_source(primary, "primary_knowledge_source", upstream, urls)] + entries.extend(_retrieval_source(x, "supporting_data_source") for x in upstream) + return lf.with_columns(pl.concat_list(entries).alias("sources")) def publications(lf: pl.LazyFrame, curies: str | list[str]) -> pl.LazyFrame: @@ -318,27 +515,40 @@ def clean_numeric(lf: pl.LazyFrame) -> pl.LazyFrame: def format_numeric(lf: pl.LazyFrame) -> pl.LazyFrame: - """Format numeric annotation columns as strings with controlled notation. + """Normalize numeric annotation columns for output. + + Columns that map to a numeric Biolink slot are emitted as real JSON numbers: + ``p_value`` and ``adjusted_p_value`` are typed ``float`` in the model (and + ``supporting_study_size`` ``integer`` once ``biolink/biolink-model#1770`` lands), + so writing ``"6.5200e-06"`` produces a file that strict consumers reject even + though Pydantic's lax mode happens to coerce it. - P-value columns use scientific notation (``{:.4e}``); all others use - decimal general format (``{:.4g}``). Null values stay null. + Columns with no numeric Biolink slot keep the controlled string notation -- + p-value-like names use scientific (``{:.4e}``), others decimal general + (``{:.4g}``) -- because they end up in human-readable text (the inlined + ``StudyResult`` description or ``supporting_text``). Null values stay null. Args: lf: Source LazyFrame. Returns: New LazyFrame (eagerly collected then re-lazied) with matched columns - formatted as strings. + typed or formatted. Notes: Collection point: the column is formatted in one batch so notation is controlled across all rows at once. """ # Collection point: batch formatting for notation control. - # P-value columns use scientific notation; others use decimal general format. df: pl.DataFrame = lf.collect() - cols: list[str] = numeric_columns(df.columns) - for c in cols: + for c in numeric_columns(df.columns): + kind: str | None = numeric_slot_kind(c) + if kind == "float": + df = df.with_columns(pl.col(c).cast(pl.Float64, strict=False).alias(c)) + continue + if kind == "int": + df = df.with_columns(pl.col(c).cast(pl.Float64, strict=False).round().cast(pl.Int64, strict=False).alias(c)) + continue df = df.with_columns(pl.col(c).cast(pl.Float64, strict=False).alias(c)) fmt: str = "{:.4e}" if "p_value" in c.lower() else "{:.4g}" formatted: list[str | None] = [None if v is None else fmt.format(v) for v in df[c].to_list()] @@ -346,6 +556,27 @@ def format_numeric(lf: pl.LazyFrame) -> pl.LazyFrame: return df.lazy() +def split_list(lf: pl.LazyFrame, col: str, delimiter: str) -> pl.LazyFrame: + """Split a delimited cell into a real JSON array. + + Tablassert annotations are scalar by construction, so a multivalued Biolink slot + such as ``has_evidence`` or ``FDA_regulatory_approvals`` would otherwise be emitted + as a single joined string. Consumers that iterate it then walk characters rather + than values (``publications.extend(record["has_evidence"])``). + + Args: + lf: Source LazyFrame. + col: Annotation column to split. + delimiter: Separator to split on. + + Returns: + LazyFrame with ``col`` converted to a ``list[str]`` column, blanks dropped. + """ + text: pl.Expr = pl.col(col).cast(pl.String) + split: pl.Expr = text.str.split(delimiter).list.eval(pl.element().str.strip_chars()).list.drop_nulls() + return lf.with_columns(pl.when(text.is_null()).then(None).otherwise(split.list.eval(pl.element().filter(pl.element() != ""))).alias(col)) + + def prefix(lf: pl.LazyFrame, col: str, prefix: str) -> pl.LazyFrame: expr: pl.Expr = pl.lit(prefix) + pl.col(col).cast(pl.String) return lf.with_columns(expr.alias(col)) @@ -686,7 +917,13 @@ def _source_ops(self: Self) -> list[Any]: else None, # --head preview: randomly sample min(HEAD_ROWS, height) rows before any encoding/resolve. (head, (HEAD_ROWS,)) if self.head else None, - [op for x in self.annotations for op in self.encoding(x, x.annotation.lower())] if self.annotations else None, + [ + op + for x in self.annotations + for op in [*self.encoding(x, x.annotation.lower()), *([(split_list, (x.annotation.lower(), x.delimiter))] if x.delimiter else [])] + ] + if self.annotations + else None, (coerce_pvalue_columns, ()), (coerce_study_size_columns, ()), (coerce_effect_size_columns, ()), @@ -709,16 +946,24 @@ def _node_ops(self: Self, db: Path) -> list[Any]: when QC is enabled. """ # Subject/object/qualifiers share one resolve_batch call instead of one per column. + # Enum-ranged qualifiers are excluded from resolution: their range is a closed + # Biolink vocabulary, so sending them through the fullmap would turn the required + # token `increased` into the CURIE `UMLS:C0205217`, which the slot rejects. + qualifiers: list[Qualifier] = self.statement.qualifiers or [] node_columns: list[tuple[NodeEncoding, str]] = [ (self.statement.subject, "subject"), (self.statement.object, "object"), - *[(x, x.qualifier) for x in (self.statement.qualifiers or [])], + *[(x, x.qualifier) for x in qualifiers if x.resolved], ] + literals: list[Qualifier] = [x for x in qualifiers if not x.resolved] specs: list[ResolveSpec] = [ ResolveSpec(col, str(x.taxon) if x.taxon else None, x.prioritize, x.avoid, x.exclude_prefixes, x.exclude_regex) for x, col in node_columns ] return [ [self.node_prep(x, col) for x, col in node_columns], + # Encode only: no pre-resolution snapshot and no NLP normalization, both of + # which exist to feed entity resolution these columns never undergo. + [self.encoding(x, x.qualifier) for x in literals], (resolve_batch, (specs, db, self.log, self.store.stem, self.config.name, True)), [(fullmap_audit, (col, self.store.stem, self.config.name, "passed", True)) for _, col in node_columns] if self.qc else None, ] @@ -736,17 +981,25 @@ def _provenance_ops(self: Self) -> list[Any]: knowledge_level = override.knowledge_level if override else self.provenance.knowledge_level agent_type = override.agent_type if override else self.provenance.agent_type publication_values = override.publications if override else [publication_curie(self.provenance.repo, self.provenance.publication or "")] + # The study is the table itself: one publication, one worksheet. Both are + # section constants, so the study id can key a static struct field. + sheet: str | None = self.source.sheet if self.source.kind == Files.EXCEL else None # pyright: ignore + publication: str = publication_values[0] if publication_values else (self.config.name or "study") + study_id: str = f"{publication}#{sheet}" if sheet else publication return [ (derive_species_context, ()), (value, ("predicate", "biolink:" + self.statement.predicate)), - (edge_category, ()), - (value, ("upstream_resource_ids", upstream_ids)), + (edge_category, ("biolink:" + self.statement.predicate,)), (value, ("knowledge_level", knowledge_level)), (value, ("agent_type", agent_type)), - (value, ("primary_knowledge_source", [primary_knowledge_source])) if primary_knowledge_source else None, + # Biolink `primary_knowledge_source` is a scalar; `sources` carries the + # structured retrieval provenance (roles, upstream ids, record urls). + (value, ("primary_knowledge_source", primary_knowledge_source)) if primary_knowledge_source else None, + (retrieval_sources, (primary_knowledge_source, upstream_ids, [str(self.source.url)])) if primary_knowledge_source else None, (publications, (publication_values,)) if publication_values else None, - (source_record_urls, (str(self.source.url),)), - (value, ("sheet_name", self.source.sheet)) if self.source.kind == Files.EXCEL else None, # pyright: ignore + # Prune first so class-rejected values are handed to the study rather than lost. + (prune_to_class, ()), + (inline_supporting_study, (study_id, sheet)), (trim, ()), (format_numeric, ()), (to_store, (self.store, self.config.name)), @@ -795,7 +1048,10 @@ def collect(self: Self, db: Path) -> list[tuple[Callable, tuple[Any]]] | Path: derive_species_context: "edge", edge_category: "edge", publications: "provenance", - source_record_urls: "provenance", + retrieval_sources: "provenance", + inline_supporting_study: "provenance", + prune_to_class: "finalize", + split_list: "encode", sig: "significance", drop_not_significant: "significance", trim: "finalize", @@ -808,9 +1064,7 @@ def collect(self: Self, db: Path) -> list[tuple[Callable, tuple[Any]]] | Path: UNKNOWN_PHASE: str = "transform" -_VALUE_PROVENANCE_COLS: frozenset[str] = frozenset( - {"upstream_resource_ids", "knowledge_level", "agent_type", "primary_knowledge_source", "sheet_name"} -) +_VALUE_PROVENANCE_COLS: frozenset[str] = frozenset({"knowledge_level", "agent_type", "primary_knowledge_source", "sheet_name"}) def _phase_of(fn: Callable, args: tuple[Any, ...]) -> str: @@ -863,23 +1117,34 @@ def compile_subgraph(tcode: list[tuple[Callable, tuple[Any]]], *, on_phase: Call return acc # pyright: ignore -def normalize(edges: pl.LazyFrame, col: str, names: list[str] | None = None) -> tuple[pl.LazyFrame, pl.LazyFrame]: +def normalize(edges: pl.LazyFrame, col: str, names: list[str] | None = None, infores_id: str | None = None) -> tuple[pl.LazyFrame, pl.LazyFrame]: """Normalize disparate node columns into a unified format and remove them from edges. + Emits Biolink ``NamedThing`` slots only. The fullmap's ``_source`` (a file + name such as ``gene.txt``) and ``_source_version`` are build provenance, not + node properties -- ``source``/``source_version`` are ``extra_forbidden`` on every + Biolink node class, and the version belongs at graph level (the RIG), matching + ``translator-ingests/util/metadata.py``. The taxon is emitted as ``in_taxon`` plus + ``in_taxon_label``, the slots ``translator-ingests`` uses. + Args: edges: Source edges LazyFrame containing ````, ``_name``, ``_category``, ``_taxon``, ``_source``, and ``_source_version`` columns. col: Base node column name (e.g. ``"subject"``). names: Output column names for the produced nodes frame. + infores_id: Graph-level infores CURIE recorded as ``provided_by``. Returns: Tuple of ``(partial_nodes, modified_edges)`` as LazyFrames. """ if names is None: - names = ["id", "name", "category", "taxon", "source", "source_version"] - cols: list[str] = [col, f"{col}_name", f"{col}_category", f"{col}_taxon", f"{col}_source", f"{col}_source_version"] - nodes: pl.LazyFrame = edges.select(cols).unique().rename(dict(zip(cols, names, strict=True))) + names = ["id", "name", "category", "in_taxon", "in_taxon_label"] + cols: list[str] = [col, f"{col}_name", f"{col}_category", f"{col}_taxon", f"{col}_taxon_label"] + available: list[str] = edges.collect_schema().names() + # `_taxon_label` is not produced by every fullmap revision. + pairs: list[tuple[str, str]] = [(c, n) for c, n in zip(cols, names, strict=True) if c in available] + nodes: pl.LazyFrame = edges.select([c for c, _ in pairs]).unique().rename(dict(pairs)) # Ensures category has biolink: prefix. nodes = nodes.with_columns( pl.when(pl.col("category").str.starts_with("biolink:")) @@ -887,9 +1152,26 @@ def normalize(edges: pl.LazyFrame, col: str, names: list[str] | None = None) -> .otherwise(pl.lit("biolink:") + pl.col("category")) .alias("category") ) + # Entity resolution can land on a class that cannot be emitted as a KGX node -- + # `Publication` requires `publication_type`, `ClinicalAttribute` requires + # `has_attribute_type`, and mixins like `GenomicEntity` reject their own name in the + # `category` literal. Demote those to the nearest emittable ancestor. + # `replace` (not `replace_strict`) so nulls and unrecognized spellings pass through + # untouched -- only categories that genuinely need demoting are rewritten. + emittable: dict[str, str] = { + curie: resolved for c in Categories for curie in (f"biolink:{c.value}",) if (resolved := resolve_node_category(curie)) != curie + } + nodes = nodes.with_columns(pl.col("category").replace(emittable).alias("category")) # Exports category within a list (null categories stay null for strip_nulls). nodes = nodes.with_columns(pl.when(pl.col("category").is_not_null()).then(pl.concat_list(pl.col("category"))).alias("category")) - edges_out: pl.LazyFrame = edges.drop(cols[1:]) + if "in_taxon" in nodes.collect_schema().names(): + # `in_taxon` is multivalued on Biolink `thing with taxon`. + nodes = nodes.with_columns(pl.when(pl.col("in_taxon").is_not_null()).then(pl.concat_list(pl.col("in_taxon"))).alias("in_taxon")) + if infores_id: + nodes = nodes.with_columns(pl.concat_list(pl.lit(infores_id)).alias("provided_by")) + # Drop every derived node column from the edges, including the ones not emitted. + drop: list[str] = [c for c in (*cols[1:], f"{col}_source", f"{col}_source_version") if c in available] + edges_out: pl.LazyFrame = edges.drop(drop) return nodes, edges_out @@ -974,7 +1256,11 @@ def fold_unknown_to_supporting_text(lf: pl.LazyFrame) -> pl.LazyFrame: parts: list[pl.Expr] = [] for col in unknown: - s: pl.Expr = pl.col(col).cast(pl.String).str.strip_chars() + # List-typed columns cannot be cast to String directly; join their elements so + # a folded `sources`-style column degrades readably instead of raising. + s: pl.Expr = ( + pl.col(col).list.eval(pl.element().cast(pl.String)).list.join(", ") if isinstance(schema[col], pl.List) else pl.col(col).cast(pl.String) + ).str.strip_chars() blank: pl.Expr = s.is_null() | (s.str.len_chars() == 0) entry: pl.Expr = pl.when(blank).then(pl.lit(None, dtype=pl.String)).otherwise(pl.concat_str([pl.lit(f"{col}: "), s])) parts.append(entry) @@ -997,6 +1283,7 @@ def _collect_subframes( ui_explanation: str | None, on_phase: Callable[[str], None] | None = None, on_subgraph: Callable[[], None] | None = None, + infores_id: str | None = None, ) -> tuple[list[pl.LazyFrame], list[pl.LazyFrame], list[dict[str, object]]]: """Scan and normalize subgraph parquets into node/edge subframes. @@ -1034,7 +1321,7 @@ def _collect_subframes( originals: list[str] = [c.removesuffix("_pre_resolution") for c in lf.collect_schema().names() if c.endswith("_pre_resolution")] node_cols: list[str] = [c for c in originals if c in ("subject", "object")] for col in node_cols: - partial, lf = normalize(lf, col) + partial, lf = normalize(lf, col, infores_id=infores_id) subnodes.append(partial) # Drop internal pre-resolution snapshot columns from final edges. lf = lf.drop([c for c in lf.collect_schema().names() if c.endswith("_pre_resolution")]) @@ -1172,7 +1459,7 @@ def compile_graph( subnodes: list[pl.LazyFrame] subedges: list[pl.LazyFrame] edge_type_info: list[dict[str, object]] - subnodes, subedges, edge_type_info = _collect_subframes(subgraphs, e, ui_explanation, on_phase, on_subgraph) + subnodes, subedges, edge_type_info = _collect_subframes(subgraphs, e, ui_explanation, on_phase, on_subgraph, infores_id) _write_ndjson(subnodes, subedges, edge_type_info, n, e, name, version, description, contributions, ui_explanation, tables, infores_id, on_phase) diff --git a/src/tablassert/models.py b/src/tablassert/models.py index fbad60a1..9390542b 100644 --- a/src/tablassert/models.py +++ b/src/tablassert/models.py @@ -8,7 +8,16 @@ from pydantic import BaseModel, ConfigDict, Field, HttpUrl, NonNegativeInt, PositiveInt, field_validator, model_validator from tablassert._lazy import LazyModule -from tablassert.biolink import AgentTypes, Categories, KnowledgeLevels, Predicates, Qualifiers +from tablassert.biolink import ( + BIOLINK_VERSION, + ENUM_RANGED_QUALIFIERS, + UNSATISFIABLE_EDGE_FIELDS, + AgentTypes, + Categories, + KnowledgeLevels, + Predicates, + Qualifiers, +) from tablassert.enums import Comparisons, EncodingMethods, Files, FillMethods, Functions, Repositories, Tokens from tablassert.errors import TablassertErrorCodes, TablassertValidationError @@ -253,6 +262,20 @@ class Qualifier(NodeEncoding): examples=[Qualifiers.OBJECT_DIRECTION_QUALIFIER, Qualifiers.SUBJECT_CONTEXT_QUALIFIER], ) + @property + def vocabulary(self: Self) -> frozenset[str] | None: + """Closed value set for this qualifier, or ``None`` when it is CURIE-ranged.""" + return ENUM_RANGED_QUALIFIERS.get(str(self.qualifier)) + + @property + def resolved(self: Self) -> bool: + """Whether this qualifier's values go through fullmap entity resolution. + + Only CURIE-ranged qualifiers are resolved. Enum-ranged ones carry a literal + token from a closed Biolink vocabulary and must be passed through verbatim. + """ + return self.vocabulary is None + @model_validator(mode="after") def reject_auto_derived_qualifiers(self: Self) -> Self: """Reject qualifiers that Tablassert derives from resolved node metadata. @@ -268,6 +291,46 @@ def reject_auto_derived_qualifiers(self: Self) -> Self: ) return self + @model_validator(mode="after") + def reject_unusable_qualifiers(self: Self) -> Self: + """Reject qualifier slots that no Biolink Pydantic class can hold. + + ``Qualifiers`` is derived from the LinkML *slot* hierarchy, which is strictly + broader than the set of slots actually attached to a class. Emitting one of + these produces an edge that can never validate, so fail at config time with a + pointer rather than silently at ingest time. + """ + if str(self.qualifier) in UNSATISFIABLE_EDGE_FIELDS: + raise TablassertValidationError( + f"{self.qualifier} is declared in the Biolink schema but attached to no association class " + f"in biolink-model {BIOLINK_VERSION}, so it cannot be emitted on an edge. " + "Use a concrete subtype of it, or record the value as an annotation.", + code="qualifier-unsatisfiable", + ) + return self + + @model_validator(mode="after") + def enum_ranged_values_are_literals(self: Self) -> Self: + """Validate literal values for enum-ranged qualifiers against their vocabulary. + + Qualifiers inherit :class:`NodeEncoding` and are therefore entity-resolved + through the fullmap by default. That is right for CURIE-ranged qualifiers + (``anatomical_context_qualifier`` -> ``UBERON:0001557``) and wrong for + enum-ranged ones: ``object_direction_qualifier`` wants the token ``increased``, + not the resolved CURIE ``UMLS:C0205217``. + """ + vocabulary: frozenset[str] | None = self.vocabulary + if vocabulary is None or self.method != EncodingMethods.VALUE: + return self + literal: str = str(self.encoding).strip() + if literal not in vocabulary: + preview: str = ", ".join(sorted(vocabulary)[:8]) + raise TablassertValidationError( + f"{self.qualifier} has a closed Biolink vocabulary; got {literal!r}. Permitted values include: {preview}...", + code="qualifier-bad-value", + ) + return self + class Statement(TablaBase): subject: NodeEncoding = Field(..., description="Subject node encoding and mapping configuration.") @@ -363,6 +426,25 @@ def is_valid_pmc_id(self: Self) -> Self: class Annotation(Encoding): annotation: str = Field(..., description="Output column name that receives this encoded annotation.", examples=["p_value", "cohort"]) + delimiter: str | None = Field( + None, + description=( + "Split the encoded cell on this separator to emit a real JSON array instead of a scalar. " + "Required for multivalued Biolink slots such as `has_evidence` or `FDA_regulatory_approvals`, " + "whose consumers iterate the value." + ), + examples=["|", ";", ","], + ) + + @field_validator("delimiter", mode="after") + @classmethod + def non_empty_delimiter(cls, delimiter: str | None) -> str | None: + # An empty separator splits into individual characters, which is never intended + # and is exactly the failure mode a scalar-vs-list mismatch already causes + # downstream (`publications.extend("PMID:1")` iterating characters). + if delimiter is not None and not delimiter: + raise TablassertValidationError("`delimiter` must be a non-empty separator.", code="annotation-bad-delimiter") + return delimiter @field_validator("annotation", mode="after") @classmethod diff --git a/src/tablassert/rig.py b/src/tablassert/rig.py index 75e94d6f..3dfbe737 100644 --- a/src/tablassert/rig.py +++ b/src/tablassert/rig.py @@ -41,15 +41,21 @@ def strip_nulls(r: object, bad: set[str] | None = None) -> dict: bad: Lowercased strings treated as null-equivalent. Returns: - Dict with falsy and ``bad``-valued keys removed; recurses into + Dict with absent and ``bad``-valued keys removed; recurses into nested dicts and lists. + + Notes: + Drops absent values only, not falsy ones. ``0`` and ``False`` are meaningful + Biolink values (a ``p_value`` of 0, ``number_of_cases: 0``, + ``negated: False``), so treating them as null would silently delete the key. + Kept in step with the Rust port in ``rust/src/json.rs``. """ if bad is None: bad = {"na", "nan", "null", "none", ""} return { k: [strip_nulls(i) if isinstance(i, dict) else i for i in v] if isinstance(v, list) else strip_nulls(v) if isinstance(v, dict) else v for k, v in r.items() # pyright: ignore - if v and str(v).strip().lower() not in bad + if not (v is None or (isinstance(v, str | list | dict | tuple | set) and len(v) == 0)) and str(v).strip().lower() not in bad } @@ -149,6 +155,7 @@ def rig_edge_type_info(lf: pl.LazyFrame, edges_path: Path, ui_explanation: str | "primary_knowledge_sources", "resource_id", "upstream_resource_ids", + "sources", ] if c in names ] @@ -158,11 +165,19 @@ def rig_edge_type_info(lf: pl.LazyFrame, edges_path: Path, ui_explanation: str | rows: list[dict[str, Any]] = lf.select(wanted).unique().collect().to_dicts() info: list[dict[str, object]] = [] for row in rows: + # Retrieval provenance now lives in the nested `sources` list (Biolink + # RetrievalSource); the flat columns are still read for legacy parquet inputs. + nested: list[str] = [] + for entry in as_list(row.get("sources")): + if isinstance(entry, dict): + nested.append(str(entry.get("resource_id") or "")) + nested.extend(str(x) for x in as_list(entry.get("upstream_resource_ids"))) primary_sources: list[str] = clean_values( as_list(row.get("primary_knowledge_source")) + as_list(row.get("primary_knowledge_sources")) + as_list(row.get("resource_id")) + as_list(row.get("upstream_resource_ids")) + + nested ) edge_type: dict[str, object] = strip_nulls( { diff --git a/tests/test_biolink.py b/tests/test_biolink.py index 2b88cbc8..47ac24ae 100644 --- a/tests/test_biolink.py +++ b/tests/test_biolink.py @@ -21,6 +21,7 @@ ALLOWED_EDGE_FIELDS, BIOLINK_VERSION, EFFECT_TYPE_VALUES, + UNSATISFIABLE_EDGE_FIELDS, AgentTypes, Categories, EdgeCategories, @@ -28,6 +29,8 @@ KnowledgeLevels, Predicates, Qualifiers, + numeric_slot_kind, + resolve_association_class, ) if TYPE_CHECKING: @@ -284,16 +287,24 @@ def test_allowed_edge_fields_covers_required_columns() -> None: "qualified_predicate", "primary_knowledge_source", "publications", - "source_record_urls", - "upstream_resource_ids", + "sources", + "has_supporting_studies", ] for col in required: assert col in ALLOWED_EDGE_FIELDS, col -def test_allowed_edge_fields_includes_new_qualifiers() -> None: - """New 4.4.3 qualifier slots are allowed edge columns.""" - assert "process_qualifier" in ALLOWED_EDGE_FIELDS +def test_retrieval_source_slots_are_not_edge_columns() -> None: + """``upstream_resource_ids`` / ``source_record_urls`` belong to ``RetrievalSource``. + + Both have ``domain: retrieval source`` in the model, so emitting them flat on an + association is an ``extra_forbidden`` error. They must reach output only nested + inside a ``sources`` entry. + """ + for slot in ("upstream_resource_ids", "source_record_urls"): + assert slot not in ALLOWED_EDGE_FIELDS, slot + assert slot in bm.RetrievalSource.model_fields, slot + assert slot not in bm.Association.model_fields, slot def test_allowed_edge_fields_includes_effect_annotations() -> None: @@ -302,6 +313,59 @@ def test_allowed_edge_fields_includes_effect_annotations() -> None: assert "effect_type" in ALLOWED_EDGE_FIELDS -def test_allowed_edge_fields_is_superset_of_qualifiers() -> None: - """Every qualifier slot name is an allowed edge column.""" - assert {q.value for q in Qualifiers} <= set(ALLOWED_EDGE_FIELDS) +def test_allowed_edge_fields_includes_subclass_only_slots() -> None: + """Slots declared only by ``Association`` *subclasses* are still allowed columns. + + Deriving the allow-list from the base ``Association`` MRO alone silently demotes + evidence slots such as ``clinical_approval_status`` into ``supporting_text``. + """ + for slot in ("clinical_approval_status", "number_of_cases", "FDA_regulatory_approvals"): + assert slot not in bm.Association.model_fields, slot + assert slot in ALLOWED_EDGE_FIELDS, slot + + +def test_allowed_edge_fields_excludes_unattached_qualifiers() -> None: + """Qualifier slots attached to no Pydantic class are not emittable. + + ``Qualifiers`` is walked from the LinkML *slot* hierarchy, which includes abstract + grouping slots (``process_qualifier``, ``aspect_qualifier``) that no class declares. + Emitting one produces a record that can never validate. + """ + assert "process_qualifier" in {q.value for q in Qualifiers} + assert "process_qualifier" in UNSATISFIABLE_EDGE_FIELDS + assert "process_qualifier" not in ALLOWED_EDGE_FIELDS + + +def test_allowed_edge_fields_covers_every_satisfiable_qualifier() -> None: + """Every qualifier slot with a real home is an allowed edge column.""" + satisfiable: set[str] = {q.value for q in Qualifiers} - set(UNSATISFIABLE_EDGE_FIELDS) + assert satisfiable <= set(ALLOWED_EDGE_FIELDS) + + +def test_unsatisfiable_fields_are_derived_not_hardcoded() -> None: + """``UNSATISFIABLE_EDGE_FIELDS`` must reflect the *installed* model. + + ``biolink/biolink-model#1770`` attaches the ``supporting_study_*`` slots to root + ``association``; when that ships they become ordinary edge columns. Nothing may + hardcode either state, so assert the set is exactly "declared but unattached". + """ + owned: set[str] = set() + for cls in vars(bm).values(): + if inspect.isclass(cls) and cls.__module__ == bm.__name__: + owned |= set(getattr(cls, "model_fields", {})) + for field in UNSATISFIABLE_EDGE_FIELDS: + assert field not in owned, field + + +def test_resolve_association_class_reconciles_predicate() -> None: + """A category whose predicate enum forbids the predicate is demoted, not emitted.""" + # GeneToDiseaseAssociation permits only contributes_to / associated_with / affects. + assert resolve_association_class("biolink:GeneToDiseaseAssociation", "biolink:associated_with") is bm.GeneToDiseaseAssociation + assert resolve_association_class("biolink:GeneToDiseaseAssociation", "biolink:gene_associated_with_condition") is bm.Association + + +def test_numeric_slot_kind_matches_model_ranges() -> None: + """P-values are floats in Biolink, so they must not be emitted as strings.""" + assert numeric_slot_kind("p_value") == "float" + assert numeric_slot_kind("adjusted_p_value") == "float" + assert numeric_slot_kind("subject") is None diff --git a/tests/test_docs_cli_coverage.py b/tests/test_docs_cli_coverage.py index adbd56a8..b92e5fe0 100644 --- a/tests/test_docs_cli_coverage.py +++ b/tests/test_docs_cli_coverage.py @@ -26,12 +26,14 @@ "build-fullmap": ("cli.md", "fullmap.md"), "build-kg": ("cli.md",), "validate": ("cli.md",), + "validate-kgx": ("cli.md",), } COMMAND_FLAG_DOCS: dict[str, tuple[str, ...]] = { "agent": ("agent.md", "cli.md"), "build-fullmap": ("cli.md", "fullmap.md"), "build-kg": ("cli.md",), "validate": ("cli.md",), + "validate-kgx": ("cli.md",), } diff --git a/tests/test_lib.py b/tests/test_lib.py index 40be1d95..0e43c532 100644 --- a/tests/test_lib.py +++ b/tests/test_lib.py @@ -10,7 +10,7 @@ import tablassert.cli as cli import tablassert.lib as lib from tablassert import rs -from tablassert.biolink import ALLOWED_EDGE_FIELDS, EFFECT_TYPE_VALUES, Categories +from tablassert.biolink import ALLOWED_EDGE_FIELDS, EFFECT_TYPE_VALUES, UNSATISFIABLE_EDGE_FIELDS, Categories, validate_kgx from tablassert.coerce import _EFFECT_TYPE_ALIASES, _map_effect_type_value from tablassert.enums import Repositories from tablassert.fullmap import ResolveSpec @@ -38,6 +38,7 @@ parse_edge_name, publications, pvalue_target, + retrieval_sources, strip_nulls, study_size_target, ) @@ -454,17 +455,28 @@ def test_upstream_resource_ids_pubmed() -> None: assert lib.upstream_resource_ids(Repositories.PUBMED) == ["infores:pubmed"] -def test_tcode_collect_adds_upstream_resource_ids(fixtures_path: Path) -> None: - """tcode collect adds upstream resource IDs from provenance repository.""" +def test_tcode_collect_nests_upstream_resource_ids_in_sources(fixtures_path: Path) -> None: + """Upstream resource IDs reach output inside ``sources``, never flat on the edge. + + ``upstream_resource_ids`` has ``domain: retrieval source`` in Biolink, so a + top-level column would fail validation with ``extra_forbidden``. + """ data: Any = from_yaml(fixtures_path / "minimal_section.yaml") store: Path = Path("/tmp/sectionhash.parquet") tcode_model: Tcode = Tcode.model_validate( # pyright: ignore - {**data, "config": fixtures_path / "minimal_section.yaml", "store": store} + {**data, "config": fixtures_path / "minimal_section.yaml", "store": store, "name": "GRAPH_KG"} ) collected: list[tuple[Any, tuple[Any]]] = tcode_model.collect(Path("/tmp/fullmap.redb")) # pyright: ignore - ops: list[tuple[Any, tuple[Any]]] = [op for op in collected if len(op[1]) > 0 and op[1][0] == "upstream_resource_ids"] - assert ops[0][1] == ("upstream_resource_ids", ["infores:pubmed-central"]) + assert [op for op in collected if len(op[1]) > 0 and op[1][0] == "upstream_resource_ids"] == [] + + source_ops: list[tuple[Any, tuple[Any]]] = [op for op in collected if op[0] is retrieval_sources] + assert len(source_ops) == 1 + out: pl.DataFrame = source_ops[0][0](pl.LazyFrame({"subject": ["A"]}), *source_ops[0][1]).collect() + sources: list[dict[str, Any]] = out["sources"].to_list()[0] + primary: dict[str, Any] = next(s for s in sources if s["resource_role"] == "primary_knowledge_source") + assert primary["upstream_resource_ids"] == ["infores:pubmed-central"] + assert {s["resource_id"] for s in sources if s["resource_role"] == "supporting_data_source"} == {"infores:pubmed-central"} def test_normalize_category_list_with_biolink_prefix() -> None: @@ -526,7 +538,7 @@ def test_tcode_collect_emits_primary_knowledge_source_when_named(fixtures_path: ] assert len(pks_ops) == 1 - assert pks_ops[0][1] == ("primary_knowledge_source", ["infores:multiomics-kg"]) + assert pks_ops[0][1] == ("primary_knowledge_source", "infores:multiomics-kg") def test_tcode_collect_omits_primary_knowledge_source_when_unnamed(fixtures_path: Path) -> None: @@ -565,8 +577,11 @@ def test_tcode_collect_manual_provenance_overrides_auto_sources(fixtures_path: P values: dict[str, object] = {str(op[1][0]): op[1][1] for op in collected if op[0].__name__ == "value" and len(op[1]) >= 2} pub_ops = [op for op in collected if op[0] is publications] - assert values["primary_knowledge_source"] == ["infores:graph-source"] - assert values["upstream_resource_ids"] == ["infores:upstream-source"] + assert values["primary_knowledge_source"] == "infores:graph-source" + # Manual upstream infores reach output nested in `sources`, not flat on the edge. + assert "upstream_resource_ids" not in values + source_args: tuple[Any, ...] = next(op[1] for op in collected if op[0] is retrieval_sources) + assert source_args[1] == ["infores:upstream-source"] assert values["knowledge_level"] == "knowledge_assertion" assert values["agent_type"] == "manual_agent" assert pub_ops[0][1] == (["PMCID:PMC9999999"],) @@ -583,28 +598,27 @@ def test_tcode_collect_uses_graph_infores_when_no_section_override(fixtures_path collected: list[tuple[Any, tuple[Any]]] = tcode_model.collect(Path("/tmp/fullmap.redb")) # pyright: ignore pks_ops = [op for op in collected if op[0].__name__ == "value" and len(op[1]) > 0 and op[1][0] == "primary_knowledge_source"] - assert pks_ops[0][1] == ("primary_knowledge_source", ["infores:custom-graph"]) + assert pks_ops[0][1] == ("primary_knowledge_source", "infores:custom-graph") -def test_tcode_collect_emits_source_record_urls_list(fixtures_path: Path) -> None: - """tcode emits source record URLs as a list column.""" +def test_tcode_collect_nests_source_record_urls_in_sources(fixtures_path: Path) -> None: + """Source record URLs hang off the primary ``RetrievalSource``, not the edge.""" data: Any = from_yaml(fixtures_path / "minimal_section.yaml") store: Path = Path("/tmp/sectionhash.parquet") tcode_model: Tcode = Tcode.model_validate( # pyright: ignore - {**data, "config": fixtures_path / "minimal_section.yaml", "store": store} + {**data, "config": fixtures_path / "minimal_section.yaml", "store": store, "name": "GRAPH_KG"} ) collected: list[tuple[Any, tuple[Any]]] = tcode_model.collect(Path("/tmp/fullmap.redb")) # pyright: ignore - source_ops: list[tuple[Any, tuple[Any]]] = [op for op in collected if op[0].__name__ == "source_record_urls"] + source_ops: list[tuple[Any, tuple[Any]]] = [op for op in collected if op[0] is retrieval_sources] url_ops: list[tuple[Any, tuple[Any]]] = [op for op in collected if op[0].__name__ == "value" and len(op[1]) > 0 and op[1][0] == "url"] - lf: pl.LazyFrame = pl.DataFrame({"subject": ["A"]}).lazy() - result: pl.DataFrame = source_ops[0][0](lf, *source_ops[0][1]).collect() + result: pl.DataFrame = source_ops[0][0](pl.LazyFrame({"subject": ["A"]}), *source_ops[0][1]).collect() assert len(source_ops) == 1 assert url_ops == [] - assert "source_record_urls" in result.columns - assert "url" not in result.columns - assert result["source_record_urls"].to_list() == [["https://example.com/test.tsv"]] + assert "source_record_urls" not in result.columns + primary: dict[str, Any] = next(s for s in result["sources"].to_list()[0] if s["resource_role"] == "primary_knowledge_source") + assert primary["source_record_urls"] == ["https://example.com/test.tsv"] def test_tcode_original_value_before_regex_for_columns(fixtures_path: Path) -> None: @@ -887,13 +901,18 @@ def test_clean_numeric_idempotent_on_float64() -> None: assert twice.schema["p_value"] == pl.Float64 -def test_format_numeric_p_value_scientific() -> None: - """format_numeric renders P value columns in scientific notation.""" +def test_format_numeric_emits_p_values_as_numbers() -> None: + """P-value columns are emitted as real JSON numbers, not formatted strings. + + Biolink types ``p_value`` / ``adjusted_p_value`` as ``float``; writing + ``"1.0000e-08"`` yields a file strict consumers reject even though Pydantic's lax + mode happens to coerce it back. + """ lf: pl.LazyFrame = pl.DataFrame({"p_value": ["1e-8", "0.05", "0.001"], "adjusted_p_value": ["0.0001", "0.1", "0.2"]}).lazy() result: pl.DataFrame = format_numeric(clean_numeric(lf)).collect() - assert result["p_value"].to_list() == ["1.0000e-08", "5.0000e-02", "1.0000e-03"] - assert result["adjusted_p_value"].to_list() == ["1.0000e-04", "1.0000e-01", "2.0000e-01"] - assert result.schema["p_value"] == pl.String + assert result["p_value"].to_list() == [1e-08, 0.05, 0.001] + assert result["adjusted_p_value"].to_list() == [0.0001, 0.1, 0.2] + assert result.schema["p_value"] == pl.Float64 def test_format_numeric_decimal_general() -> None: @@ -908,7 +927,7 @@ def test_format_numeric_preserves_nulls() -> None: """format_numeric preserves nulls as null.""" lf: pl.LazyFrame = pl.DataFrame({"p_value": ["1e-8", "N/A", "0.05"]}).lazy() result: pl.DataFrame = format_numeric(clean_numeric(lf)).collect() - assert result["p_value"].to_list() == ["1.0000e-08", None, "5.0000e-02"] + assert result["p_value"].to_list() == [1e-08, None, 0.05] def test_format_numeric_cleans_float_noise() -> None: @@ -931,7 +950,7 @@ def test_format_numeric_nulls_stripped_from_ndjson_rows() -> None: lf: pl.LazyFrame = pl.DataFrame({"subject": ["BRCA1", "TP53"], "p_value": ["1e-8", "N/A"], "effect_size": ["0.85", "0.42"]}).lazy() formatted: pl.DataFrame = format_numeric(clean_numeric(lf)).collect() rows: list[dict[str, Any]] = [strip_nulls(r) for r in formatted.iter_rows(named=True)] - assert rows[0] == {"subject": "BRCA1", "p_value": "1.0000e-08", "effect_size": "0.85"} + assert rows[0] == {"subject": "BRCA1", "p_value": 1e-08, "effect_size": "0.85"} assert "p_value" not in rows[1] assert rows[1]["subject"] == "TP53" assert rows[1]["effect_size"] == "0.42" @@ -973,7 +992,8 @@ def test_compile_graph_emits_ndjson(monkeypatch: Any, tmp_path: Path) -> None: assert all('"id"' in line for line in edges) flat: str = "\n".join(edges) assert '"p_value":"1.0000e-08"' in flat - assert '"upstream_resource_ids":["infores:pubmed-central"]' in flat + # Retrieval provenance is nested under `sources`, never flat on the edge. + assert '"upstream_resource_ids":["infores:pubmed-central"]' not in flat # internal pre-resolution snapshot is stripped from final edges assert "_pre_resolution" not in flat assert len(nodes) >= 1 @@ -1892,9 +1912,14 @@ def test_coerced_study_size_alias_survives_unknown_folding() -> None: {"subject": ["A"], "object": ["B"], "predicate": ["related_to"], "sample_size": [12000], "miscellaneous_notes": ["note"]} ).lazy() out: pl.DataFrame = fold_unknown_to_supporting_text(coerce_study_size_columns(lf)).collect() - assert out["supporting_study_size"].to_list() == [12000] assert "sample_size" not in out.columns - assert out["supporting_text"].to_list() == [["miscellaneous_notes: note"]] + if "supporting_study_size" in UNSATISFIABLE_EDGE_FIELDS: + # Unattached in the installed model: folded rather than emitted unvalidatably. + assert "supporting_study_size" not in out.columns + assert "supporting_study_size: 12000" in out["supporting_text"].to_list()[0] + else: + assert out["supporting_study_size"].to_list() == [12000] + assert "miscellaneous_notes: note" in out["supporting_text"].to_list()[0] def test_publications_wraps_curie_as_list() -> None: @@ -1921,14 +1946,14 @@ def test_fold_unknown_noop_when_all_allowed() -> None: "object": ["B"], "predicate": ["related_to"], "p_value": [0.01], - "severity_qualifier": ["severe"], + "disease_context_qualifier": ["MONDO:0005148"], "publications": [["PMID:1"]], } ).lazy() out: pl.DataFrame = fold_unknown_to_supporting_text(lf).collect() # nothing folded, no supporting_text column created assert "supporting_text" not in out.columns - assert set(out.columns) == {"subject", "object", "predicate", "p_value", "severity_qualifier", "publications"} + assert set(out.columns) == {"subject", "object", "predicate", "p_value", "disease_context_qualifier", "publications"} def test_fold_unknown_single_column() -> None: @@ -2010,7 +2035,6 @@ def test_fold_unknown_preserves_qualifier_columns() -> None: "object": ["B"], "predicate": ["related_to"], "disease_context_qualifier": ["MONDO:0005148"], - "severity_qualifier": ["severe"], "anatomical_context_qualifier": ["UBERON:0000061"], } ).lazy() @@ -2018,12 +2042,27 @@ def test_fold_unknown_preserves_qualifier_columns() -> None: # no supporting_text column materialized because nothing was foldable assert "supporting_text" not in out.columns assert "disease_context_qualifier" in out.columns - assert "severity_qualifier" in out.columns assert "anatomical_context_qualifier" in out.columns -def test_fold_unknown_preserves_supporting_study_metadata_slots() -> None: - """PR #1770 supporting study metadata slots survive as top level edge fields, not folded.""" +SUPPORTING_STUDY_SLOTS: tuple[str, ...] = ( + "supporting_study_method_types", + "supporting_study_method_description", + "supporting_study_size", + "supporting_study_cohort", + "supporting_study_date_range", + "supporting_study_context", +) + + +def test_fold_unknown_tracks_supporting_study_slots_of_installed_model() -> None: + """The ``supporting_study_*`` slots are folded iff the installed model can hold them. + + ``biolink/biolink-model#1770`` attaches these six to root ``association``. Until it + ships they are declared in the LinkML schema but on no Pydantic class, so emitting + them flat produces an unvalidatable edge. The behaviour must be derived from the + installed model rather than pinned to either state. + """ lf: pl.LazyFrame = pl.DataFrame( { "subject": ["A"], @@ -2040,39 +2079,23 @@ def test_fold_unknown_preserves_supporting_study_metadata_slots() -> None: } ).lazy() out: pl.DataFrame = fold_unknown_to_supporting_text(lf).collect() - # only the genuinely unknown column is folded into supporting_text - assert out["supporting_text"].to_list() == [["miscellaneous_notes: see smith et al"]] - # every PR #1770 supporting study slot survives as a top level edge field - for col in ( - "has_supporting_studies", - "supporting_study_method_types", - "supporting_study_method_description", - "supporting_study_size", - "supporting_study_cohort", - "supporting_study_date_range", - "supporting_study_context", - ): - assert col in out.columns + # `has_supporting_studies` is a real Association slot in every supported version. + assert "has_supporting_studies" in out.columns + for col in SUPPORTING_STUDY_SLOTS: + assert (col in out.columns) is (col not in UNSATISFIABLE_EDGE_FIELDS), col + assert "miscellaneous_notes: see smith et al" in out["supporting_text"].to_list()[0] def test_allowed_edge_fields_covers_tablassert_pipeline_columns() -> None: """ALLOWED_EDGE_FIELDS covers intentional tablassert output columns.""" - for col in ("publications", "upstream_resource_ids", "source_record_urls", "p_value", "supporting_text"): + for col in ("publications", "sources", "p_value", "supporting_text", "has_supporting_studies"): assert col in ALLOWED_EDGE_FIELDS -def test_allowed_edge_fields_covers_supporting_study_metadata_slots() -> None: - """PR #1770 supporting study metadata slots are recognized biolist edge fields, not folded.""" - for col in ( - "has_supporting_studies", - "supporting_study_method_types", - "supporting_study_method_description", - "supporting_study_size", - "supporting_study_cohort", - "supporting_study_date_range", - "supporting_study_context", - ): - assert col in ALLOWED_EDGE_FIELDS +def test_allowed_edge_fields_tracks_supporting_study_slots_of_installed_model() -> None: + """``supporting_study_*`` membership follows the installed biolink-model exactly.""" + for col in SUPPORTING_STUDY_SLOTS: + assert (col in ALLOWED_EDGE_FIELDS) is (col not in UNSATISFIABLE_EDGE_FIELDS), col def test_compile_graph_folds_unknown_annotations_into_supporting_text(monkeypatch: Any, tmp_path: Path) -> None: @@ -2143,7 +2166,7 @@ def test_compile_subgraph_e2e_value_encoded_nodes(monkeypatch: Any, tmp_path: Pa assert result["object_name"] == "TP53" assert result["predicate"] == "biolink:related_to" assert result["publications"] == ["PMCID:PMC0000000"] - assert result["primary_knowledge_source"] == ["infores:test-kg"] + assert result["primary_knowledge_source"] == "infores:test-kg" def test_compile_subgraph_e2e_column_cleanup_and_numeric_annotations(monkeypatch: Any, tmp_path: Path) -> None: @@ -2181,9 +2204,12 @@ def test_compile_subgraph_e2e_column_cleanup_and_numeric_annotations(monkeypatch assert result["original_subject"] == "BRCA-1 [alias]" assert result["object"] == "HGNC:11998" assert result["original_object"] == "TP 53" - assert result["p_value"] == "1.0000e-08" - assert result["supporting_study_size"] == "1200" - assert result["statistical_significance_qualifier"] == "biolink:very_strongly_significant" + assert result["p_value"] == 1e-08 + # Both slots are unattached in biolink-model 4.4.3, so they are preserved on the + # inlined StudyResult instead of being emitted unvalidatably on the edge. + described: str = result["has_supporting_studies"][next(iter(result["has_supporting_studies"]))]["has_study_results"][0]["description"] + assert "supporting_study_size=1200" in described + assert "statistical_significance_qualifier=biolink:very_strongly_significant" in described assert result["miscellaneous_notes"] == "kept note" assert result["publications"] == ["PMID:12345"] @@ -2218,7 +2244,10 @@ def test_compile_subgraph_e2e_release_drops_rows_before_fullmap_lookup(monkeypat assert result.height == 1 assert result["subject"].to_list() == ["HGNC:1"] assert result["object"].to_list() == ["MONDO:1"] - assert result["statistical_significance_qualifier"].to_list() == ["biolink:strongly_significant"] + described = result["has_supporting_studies"].to_list()[0] + assert ( + "statistical_significance_qualifier=biolink:strongly_significant" in (described[next(iter(described))]["has_study_results"][0]["description"]) + ) assert "droppedgene" not in looked_up assert "droppeddisease" not in looked_up @@ -2283,13 +2312,25 @@ def test_compile_subgraph_and_graph_e2e_qualifier_stays_edge_attribute(monkeypat tcode_model: Tcode = Tcode.model_validate({**data, "config": table_path, "store": store, "name": "QUAL_KG"}) # pyright: ignore subgraph: Path = lib.compile_subgraph(tcode_model.collect(tmp_path / "fullmap.redb")) # pyright: ignore - assert pl.read_parquet(subgraph)["species_context_qualifier"].to_list() == ["NCBITaxon:9606"] + # `biolink:Association` has no species_context_qualifier slot, so the value is + # nulled on the edge and preserved on the inlined StudyResult instead. + frame: pl.DataFrame = pl.read_parquet(subgraph) + assert frame["species_context_qualifier"].to_list() == [None] + assert ( + "species_context_qualifier=NCBITaxon:9606" + in frame["has_supporting_studies"].to_list()[0][next(iter(frame["has_supporting_studies"].to_list()[0]))]["has_study_results"][0][ + "description" + ] + ) lib.compile_graph([subgraph], "qual", "1.0.0") edges: list[dict[str, Any]] = [json.loads(line) for line in (tmp_path / "qual_1.0.0.edges.ndjson").read_text().splitlines()] nodes: list[dict[str, Any]] = [json.loads(line) for line in (tmp_path / "qual_1.0.0.nodes.ndjson").read_text().splitlines()] - assert edges[0]["species_context_qualifier"] == "NCBITaxon:9606" + # Nulled on the edge (no such slot on biolink:Association) and kept on the study. + assert "species_context_qualifier" not in edges[0] + study: dict[str, Any] = edges[0]["has_supporting_studies"] + assert "species_context_qualifier=NCBITaxon:9606" in study[next(iter(study))]["has_study_results"][0]["description"] assert all("species_context_qualifier_pre_resolution" not in edge for edge in edges) assert {node["id"] for node in nodes} == {"HGNC:1100", "MONDO:0000001"} assert "NCBITaxon:9606" not in {node["id"] for node in nodes} @@ -2321,7 +2362,7 @@ def test_node_output_reflects_disease_taxon(monkeypatch: Any, tmp_path: Path) -> nodes: list[dict[str, Any]] = [json.loads(line) for line in (tmp_path / "disease_taxon_1.0.0.nodes.ndjson").read_text().splitlines()] disease_node: dict[str, Any] = next(node for node in nodes if node["id"] == "MONDO:50") - assert disease_node["taxon"] == "NCBITaxon:9606" + assert disease_node["in_taxon"] == ["NCBITaxon:9606"] def test_build_pipeline_e2e_smoke_with_monkeypatched_fullmap(monkeypatch: Any, tmp_path: Path) -> None: @@ -2376,13 +2417,21 @@ def test_build_pipeline_e2e_smoke_with_monkeypatched_fullmap(monkeypatch: Any, t assert len(edge_rows) == 1 assert edge_rows[0]["subject"] == "HGNC:1100" assert edge_rows[0]["object"] == "HGNC:11998" - assert edge_rows[0]["upstream_resource_ids"] == ["infores:pubmed-central"] - assert edge_rows[0]["primary_knowledge_source"] == ["infores:pipeline-kg"] + primary_source: dict[str, Any] = next(x for x in edge_rows[0]["sources"] if x["resource_role"] == "primary_knowledge_source") + assert primary_source["upstream_resource_ids"] == ["infores:pubmed-central"] + assert edge_rows[0]["primary_knowledge_source"] == "infores:pipeline-kg" assert {row["id"] for row in node_rows} == {"HGNC:1100", "HGNC:11998"} assert rig["name"] == "PIPELINE_KG v0.1.0" edge_type: dict[str, Any] = rig["target_info"]["edge_type_info"][0] # pyright: ignore assert edge_type["primary_knowledge_sources"] == ["infores:pipeline-kg", "infores:pubmed-central"] + # The gate: every emitted record must construct as its own Biolink class. Without + # this, a build can (and previously did) ship files where no record validated. + report: dict[str, Any] = validate_kgx(tmp_path / "PIPELINE_KG_0.1.0.nodes.ndjson", tmp_path / "PIPELINE_KG_0.1.0.edges.ndjson") + assert report["ok"], report + assert report["edges"]["valid"] == report["edges"]["total"] == 1 + assert report["nodes"]["valid"] == report["nodes"]["total"] == 2 + def test_build_pipeline_head_mode_isolates_store_and_caps_rows(monkeypatch: Any, tmp_path: Path) -> None: """--head caches subgraphs to .head.parquet (never clobbering a full build) and caps to 5 rows.""" From 4ec1cd595893fdda2ef1d7084317ad4d4d81d6e7 Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Mon, 10 Aug 2026 15:33:49 -0700 Subject: [PATCH 2/2] refactor(extras): move dspy out of [agent] into a new [optimize] extra dspy is used only by the GEPA prompt-optimization path (`agent --optimize`); ordinary agent runs never import it. Splitting it into its own extra gives everyone who does not optimize a lighter `[agent]` install, and makes the missing-package error point at the extra that actually ships the package. Runs that use --optimize now need `pip install "tablassert[agent,optimize]"`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01V6KH4PXbz68F3DSFNAFa63 --- CHANGELOG.md | 1 + README.md | 3 ++- docs/agent.md | 13 +++++++++++-- docs/installation.md | 3 ++- pyproject.toml | 4 +++- src/tablassert/agent.py | 17 +++++++++++++---- tests/test_agent_lazy.py | 19 +++++++++++++++++++ uv.lock | 8 +++++--- 8 files changed, 56 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44aba8b3..c7a2b597 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to this project are documented in this file. ## Unreleased ### Breaking Changes +- **`dspy` moved out of the `[agent]` extra into a new `[optimize]` extra.** `dspy` is used ONLY by the GEPA prompt-optimization path (`agent --optimize`); ordinary agent runs never import it. Installs that use `--optimize` must now install `pip install "tablassert[agent,optimize]"` (or add `tablassert[optimize]`); the missing-package error now points at `tablassert[optimize]` accordingly. Installs that never run `--optimize` get a lighter `[agent]` install (no `dspy`). - **Fullmap databases built by older releases must be rebuilt.** The Rust extension upgraded its embedded database engine from redb 2.6 to redb 4.1, and redb ≥ 3 dropped the old v2 file format. Existing `fullmap.redb` (and sibling `fullmap.s*.redb`) files fail to open with `fullmap DB is outdated or needs repair; rebuild with 'tablassert build-fullmap'`. Run `tablassert build-fullmap` once after upgrading. BABEL downloads stay cached, but the command rebuilds the fullmap files. The on-disk fullmap schema is now `tablassert.fullmap.v5` (the table layout is unchanged; the bump makes the redb-4 rebuild explicit and lets an older extension reject new files loudly). ### Changed diff --git a/README.md b/README.md index 08626348..2f7669d0 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,8 @@ CSV/TSV/Excel sources; optional extras add runtime and pipeline capabilities: | ----- | ---- | ------- | | `rt` | CPU-compatible Polars runtime | `pip install "tablassert[rt]"` | | `qc` | three-stage QC audit (exact → fuzzy → BioBERT embeddings) | `pip install "tablassert[qc]"` | -| `agent` | autonomous agent (smolagents, litellm, dspy, PDF context) | `pip install "tablassert[agent]"` | +| `agent` | autonomous agent (smolagents, litellm, PDF context) | `pip install "tablassert[agent]"` | +| `optimize` | GEPA prompt optimization for `agent --optimize` (dspy) | `pip install "tablassert[optimize]"` | QC is opt-in at build time (`build-kg --qc`). See the [Installation guide](https://skyeav.github.io/Tablassert/installation/) for the full matrix and the diff --git a/docs/agent.md b/docs/agent.md index 05885910..146bf79b 100644 --- a/docs/agent.md +++ b/docs/agent.md @@ -12,12 +12,16 @@ ReAct loop) and [DSPy](https://dspy.ai) GEPA for prompt optimization. !!! warning "Optional extra" The base `tablassert` package does **not** require any of this. `smolagents` and `dspy` are imported **lazily** in `tablassert.agent`, so the base install and its test suite are unaffected. Install the - extra with `pip install tablassert[agent]`. + extra with `pip install tablassert[agent]`. GEPA prompt optimization (`--optimize`) additionally needs + the `[optimize]` extra (`dspy`): `pip install tablassert[optimize]`. ## Installation ```bash pip install tablassert[agent] + +# GEPA prompt optimization (--optimize) additionally needs dspy: +pip install "tablassert[agent,optimize]" ``` The extra pins: @@ -25,10 +29,15 @@ The extra pins: | Package | Version | Role | | --- | --- | --- | | `smolagents` | `==1.26.0` | `CodeAgent` ReAct loop, `OpenAIModel`/`LiteLLMModel`, tools | -| `dspy` | `==3.2.1` | `dspy.GEPA` black-box prompt optimization | | `litellm` | (any) | optional fallback / rate-limiting model backend | | `pdfminer.six` | (any) | extract a `.pdf` main text into data-fenced context (`pmc_article_context`) | +The `[optimize]` extra (only needed for `agent --optimize`) pins: + +| Package | Version | Role | +| --- | --- | --- | +| `dspy` | `==3.2.1` | `dspy.GEPA` black-box prompt optimization | + ## PMC-AWS data source Tables are fetched from the **new** PMC open-access S3 bucket — the sanctioned bulk path. diff --git a/docs/installation.md b/docs/installation.md index b1ca98c3..e2ddbe89 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -62,7 +62,8 @@ pip install tablassert |---|---|---| | `rt` | Runtime-compatible Polars build | `polars[rtcompat]` | | `qc` | QC runtime (exact → fuzzy → BioBERT audit) | `scikit-learn`, `sentence-transformers` (`torch` + `numpy` arrive transitively; `rapidfuzz` is a core dependency) | -| `agent` | Autonomous PMC → KG agent (`tablassert agent`) | `smolagents`, `dspy`, `litellm` | +| `agent` | Autonomous PMC → KG agent (`tablassert agent`) | `smolagents`, `litellm` | +| `optimize` | GEPA prompt optimization (`tablassert agent --optimize`) | `dspy` | ```bash # Install with runtime-compatible Polars diff --git a/pyproject.toml b/pyproject.toml index 46fb28db..1d30a8c1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -84,10 +84,12 @@ qc = [ ] agent = [ "smolagents>=1.26.0", - "dspy>=3.2.1", "litellm>=1.93.0", "pdfminer.six>=20221105", ] +optimize = [ + "dspy>=3.2.1", +] [dependency-groups] dev = [ diff --git a/src/tablassert/agent.py b/src/tablassert/agent.py index da84d748..dd81c230 100644 --- a/src/tablassert/agent.py +++ b/src/tablassert/agent.py @@ -2,9 +2,14 @@ This module hosts a smolagents ``CodeAgent`` pipeline that autonomously builds and audits KGX knowledge graphs from PubMed Central articles. It is part of the -OPTIONAL ``[agent]`` extra, so ``smolagents`` and ``dspy`` are imported LAZILY -(via :class:`tablassert._lazy.LazyModule`) and the base package never requires -them at import time. Install the extra with ``pip install tablassert[agent]``. +OPTIONAL ``[agent]`` extra, so ``smolagents`` is imported LAZILY (via +:class:`tablassert._lazy.LazyModule`) and the base package never requires it at +import time. Install the extra with ``pip install tablassert[agent]``. + +``dspy`` powers ONLY the GEPA prompt-optimization path (``agent --optimize``) +and lives in its own OPTIONAL ``[optimize]`` extra +(``pip install tablassert[optimize]``); it is likewise lazy-imported and never +required by ordinary agent runs. """ from __future__ import annotations @@ -49,6 +54,10 @@ smolagents = LazyModule("smolagents") AGENT_EXTRA: str = "pip install tablassert[agent]" +OPTIMIZE_EXTRA: str = "pip install tablassert[optimize]" + +# Package -> install hint for the extra that actually ships it (default: [agent]). +_EXTRA_HINT: dict[str, str] = {"dspy": OPTIMIZE_EXTRA} logger = cat("AGENT") @@ -58,7 +67,7 @@ def _require(name: str) -> None: try: import_module(name) except ImportError as exc: - raise ImportError(f"tablassert agent features require the '{name}' package. Install with {AGENT_EXTRA}.") from exc + raise ImportError(f"tablassert agent features require the '{name}' package. Install with {_EXTRA_HINT.get(name, AGENT_EXTRA)}.") from exc def is_lazy() -> bool: diff --git a/tests/test_agent_lazy.py b/tests/test_agent_lazy.py index 53181030..a21980bd 100644 --- a/tests/test_agent_lazy.py +++ b/tests/test_agent_lazy.py @@ -43,6 +43,25 @@ def test_require_raises_actionable_when_extra_absent() -> None: assert "smolagents" in message +def test_require_dspy_points_at_optimize_extra() -> None: + """``_require("dspy")`` names the ``[optimize]`` extra, not ``[agent]``. + + Why: ``dspy`` powers only the GEPA ``--optimize`` path and lives in its own + ``[optimize]`` extra; the actionable error must tell users to install that + extra (not ``[agent]``, which no longer ships ``dspy``). + """ + assert agent_mod.OPTIMIZE_EXTRA == "pip install tablassert[optimize]" + if importlib.util.find_spec("dspy") is not None: + agent_mod._require("dspy") + return + + with pytest.raises(ImportError, match=r"tablassert\[optimize\]") as excinfo: + agent_mod._require("dspy") + message: str = str(excinfo.value) + assert "tablassert[optimize]" in message + assert "dspy" in message + + def test_lazy_proxy_does_not_eagerly_import() -> None: """Importing ``tablassert.agent`` never forces ``smolagents`` to load. diff --git a/uv.lock b/uv.lock index fd971d01..dd104155 100644 --- a/uv.lock +++ b/uv.lock @@ -4114,11 +4114,13 @@ dependencies = [ [package.optional-dependencies] agent = [ - { name = "dspy" }, { name = "litellm" }, { name = "pdfminer-six" }, { name = "smolagents" }, ] +optimize = [ + { name = "dspy" }, +] qc = [ { name = "scikit-learn" }, { name = "sentence-transformers" }, @@ -4144,7 +4146,7 @@ dev = [ requires-dist = [ { name = "biolink-model", specifier = ">=4.4.3" }, { name = "cyclopts", specifier = ">=1.0.0" }, - { name = "dspy", marker = "extra == 'agent'", specifier = ">=3.2.1" }, + { name = "dspy", marker = "extra == 'optimize'", specifier = ">=3.2.1" }, { name = "fastexcel", specifier = ">=0.20.2" }, { name = "litellm", marker = "extra == 'agent'", specifier = ">=1.93.0" }, { name = "loguru", specifier = ">=0.7.3" }, @@ -4159,7 +4161,7 @@ requires-dist = [ { name = "sentence-transformers", marker = "extra == 'qc'", specifier = ">=5.3.0" }, { name = "smolagents", marker = "extra == 'agent'", specifier = ">=1.26.0" }, ] -provides-extras = ["rt", "qc", "agent"] +provides-extras = ["rt", "qc", "agent", "optimize"] [package.metadata.requires-dev] dev = [