Merge pull request #7 from djarecka/fix_linkmlvalidation

Use the linkml library for schema validation and ingestion

Author: djarecka

.github/workflows/schema_submission.yml

@@ -96,37 +96,9 @@ jobs:
               sys.exit(1)
           PYEOF
 
-      - name: Validate with linkml-runtime
+      - name: Validate LinkML schema
         run: |
-          python3 - << 'PYEOF'
-          import sys
-          import yaml
-
-          name = "${{ steps.meta.outputs.schema_name }}"
-          path = f"schemas/{name}.yml"
-
-          # Use linkml-runtime's SchemaView for authoritative LinkML validation.
-          # If the schema is structurally invalid, SchemaView raises on load.
-          try:
-              from linkml_runtime.utils.schemaview import SchemaView
-              sv = SchemaView(path)
-              all_classes = sv.all_classes()
-              all_slots   = sv.all_slots()
-          except Exception as e:
-              print(f"ERROR: Invalid LinkML schema — {e}")
-              sys.exit(1)
-
-          if not all_classes:
-              print("ERROR: Schema defines no classes.")
-              sys.exit(1)
-
-          n_attrs = sum(
-              len(cls.attributes or {})
-              for cls in all_classes.values()
-          )
-          print(f"Valid LinkML schema: {len(all_classes)} classes, "
-                f"{len(all_slots)} slots, {n_attrs} inline attributes")
-          PYEOF
+          python neuro_ghost/validate_schema.py schemas/${{ steps.meta.outputs.schema_name }}.yml
 
       - name: Seed schema.org (if DB not cached)
         run: |
@@ -206,7 +178,8 @@ jobs:
               body: [
                 `❌ **Ingestion failed.**`,
                 ``,
-                `Check that your YAML is valid LinkML with \`classes:\` and \`slots:\` keys.`,
+                `Check the run log for the specific LinkML validation error (e.g. an undefined slot, ` +
+                `\`is_a\` parent, or range reference).`,
                 ``,
                 `[View run](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }})`,
               ].join("\n"),

.github/workflows/tests.yml

@@ -0,0 +1,25 @@
+name: Tests
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+
+jobs:
+  pytest:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: "3.12"
+
+      - name: Install dependencies
+        run: pip install -r requirements.txt
+
+      - name: Run pytest
+        run: pytest tests/ -v

neuro_ghost/ingest_linkml.py

@@ -68,12 +68,12 @@
 """
 
 from __future__ import annotations
-import datetime, uuid, os
+import datetime, uuid, os, re
 from pathlib import Path
 from typing import Any
 
 import click
-import yaml
+from linkml_runtime.utils.schemaview import SchemaView
 
 from db import (
     get_connection, make_base, make_uid, make_iri, now_iso,
@@ -148,20 +148,64 @@ def resolve_prefix(curie: str, prefixes: dict[str, str]) -> str:
 # LinkML parser
 # ---------------------------------------------------------------------------
 
+def _slot_to_dict(slot, prefixes: dict[str, str]) -> dict:
+    """
+    Convert a SchemaView-induced SlotDefinition into our internal slot dict.
+
+    "Induced" means inheritance (is_a), mixins, and schema-level default_range
+    have already been resolved onto this slot by SchemaView — so a slot
+    inherited from a mixin or parent class arrives here fully formed, exactly
+    as if it had been declared directly.
+    """
+    slot_uri     = slot.slot_uri or ""
+    resolved_iri = resolve_prefix(slot_uri, prefixes) if slot_uri else ""
+
+    raw_range = slot.range if isinstance(slot.range, str) and slot.range else "string"
+    if raw_range in LINKML_PRIMITIVES:
+        value_range = LINKML_PRIMITIVES[raw_range]
+    else:
+        # It's a reference to another class/type/enum — store as a resolved IRI
+        value_range = (resolve_prefix(raw_range, prefixes)
+                      if ":" in raw_range else make_iri(raw_range))
+
+    # Extract units from description if present (common in neuro schemas)
+    desc = str(slot.description or "")
+    units = ""
+    if desc and "(units:" in desc.lower():
+        m = re.search(r'\(units?:\s*([^)]+)\)', desc, re.IGNORECASE)
+        if m:
+            units = m.group(1).strip()
+
+    return {
+        "iri":         resolved_iri,
+        "definition":  desc,
+        "value_range": value_range,
+        "units":       units,
+        "multivalued": bool(slot.multivalued),
+        "required":    bool(slot.required),
+        "pattern":     slot.pattern or "",
+    }
+
+
 def parse_linkml(path: Path) -> dict[str, Any]:
     """
-    Read a LinkML YAML file and return a clean, normalised Python dict.
-
-    Input (LinkML YAML):
-      classes:
-        Person:
-          description: A person
-          class_uri: schema:Person
-          slots: [name, email]
-      slots:
-        name:
-          range: string
-          slot_uri: schema:name
+    Load a LinkML YAML file via SchemaView and return a clean, normalised dict.
+
+    Using SchemaView instead of a hand-rolled YAML walk means classes get
+    their real, induced slot set: slots inherited via is_a or mixins, slots
+    declared inline as `attributes:`, and ranges defaulted from the schema's
+    `default_range` all resolve exactly as LinkML defines them. A class that
+    lists no slots of its own but has `is_a: Device` still gets Device's
+    slots attached — the hand-rolled version silently dropped those.
+
+    IRI resolution intentionally does NOT use SchemaView's own get_uri():
+    imports (e.g. linkml:types) can declare their own "schema" prefix and,
+    depending on import-merge order, shadow a schema's own `prefixes:`
+    declaration — which would silently flip schema.org IRIs from
+    https:// to http://, breaking identity matching against the
+    https://schema.org/ IRIs seed.py uses. Resolving CURIEs ourselves from
+    the schema's own top-level `prefixes:` block (schema's own declarations
+    always win over KNOWN_PREFIXES) avoids that.
 
     Output (our internal format):
       {
@@ -186,77 +230,40 @@ def parse_linkml(path: Path) -> dict[str, Any]:
           }
         }
       }
-
-    The key normalisation step is value_range:
-      - If range is a LinkML primitive (string, integer, etc.) → XSD CURIE
-      - If range is a class name or CURIE → resolved IRI
     """
-    raw = yaml.safe_load(path.read_text(encoding="utf-8"))
+    sv = SchemaView(str(path))
 
-    prefixes = {k: v for k, v in (raw.get("prefixes") or {}).items()}
-    version  = str(raw.get("version", "1.0.0"))
+    prefixes = {k: v.prefix_reference for k, v in (sv.schema.prefixes or {}).items()}
 
     meta = {
-        "id":          raw.get("id", ""),
-        "name":        raw.get("name", path.stem),
-        "version":     version,
-        "description": raw.get("description", ""),
+        "id":          sv.schema.id or "",
+        "name":        sv.schema.name or path.stem,
+        "version":     str(sv.schema.version or "1.0.0"),
+        "description": sv.schema.description or "",
     }
 
-    # Parse slots first — classes reference them by name
+    classes: dict[str, dict] = {}
     slots: dict[str, dict] = {}
-    for slot_name, slot_def in (raw.get("slots") or {}).items():
-        slot_def   = slot_def or {}
-        raw_range  = slot_def.get("range", "string")
-        slot_uri   = slot_def.get("slot_uri", "")
-        resolved_iri = resolve_prefix(slot_uri, prefixes) if slot_uri else ""
-
-        # Normalise raw_range — can be None if the YAML slot has no range defined
-        if not raw_range or not isinstance(raw_range, str):
-            raw_range = "string"
-
-        if raw_range in LINKML_PRIMITIVES:
-            value_range = LINKML_PRIMITIVES[raw_range]
-        else:
-            # It's a reference to another class — store as a resolved IRI
-            value_range = (resolve_prefix(raw_range, prefixes)
-                          if ":" in raw_range else make_iri(raw_range))
-
-        # Extract units from description if present (common in neuro schemas)
-        desc = slot_def.get("description") or ""
-        desc = str(desc) if desc is not None else ""
-        units = ""
-        if desc and "(units:" in desc.lower():
-            import re
-            m = re.search(r'\(units?:\s*([^)]+)\)', desc, re.IGNORECASE)
-            if m:
-                units = m.group(1).strip()
-
-        slots[slot_name] = {
-            "iri":         resolved_iri,
-            "definition":  desc,
-            "value_range": value_range,
-            "units":       units,
-            "multivalued": bool(slot_def.get("multivalued", False)),
-            "required":    bool(slot_def.get("required", False)),
-            "pattern":     slot_def.get("pattern", ""),
-        }
 
-    # Parse classes
-    classes: dict[str, dict] = {}
-    for cls_name, cls_def in (raw.get("classes") or {}).items():
-        cls_def   = cls_def or {}
-        class_uri = cls_def.get("class_uri", "")
+    for cls_name in sv.all_classes():
+        cls_def       = sv.get_class(cls_name)
+        induced_slots = sv.class_induced_slots(cls_name)
+
+        class_uri    = cls_def.class_uri or ""
         resolved_iri = resolve_prefix(class_uri, prefixes) if class_uri else ""
 
         classes[cls_name] = {
             "iri":         resolved_iri,
-            "definition":  cls_def.get("description", ""),
-            "is_a":        cls_def.get("is_a", None),
-            "is_abstract": bool(cls_def.get("abstract", False)),
-            "slots":       cls_def.get("slots") or [],
+            "definition":  cls_def.description or "",
+            "is_a":        cls_def.is_a,
+            "is_abstract": bool(cls_def.abstract),
+            "slots":       [slot.name for slot in induced_slots],
         }
 
+        for slot in induced_slots:
+            if slot.name not in slots:
+                slots[slot.name] = _slot_to_dict(slot, prefixes)
+
     return {
         "meta":     meta,
         "prefixes": prefixes,

neuro_ghost/validate_schema.py

@@ -0,0 +1,100 @@
+"""
+validate_schema.py — Validate a submitted file is a well-formed LinkML schema
+==============================================================================
+
+WHY THIS FILE EXISTS
+--------------------
+The schema-submission GitHub Action used to "validate" incoming YAML by
+checking that the string "classes:" appeared in the text and that the
+parsed dict had "classes"/"slots" keys. That accepts almost anything —
+a schema with a class that lists a slot which is never defined, or an
+is_a parent that doesn't exist, sails straight through and only breaks
+later, deep inside ingest_linkml.py.
+
+This script does real LinkML validation, in two passes:
+
+  1. Metamodel validation — is this YAML shaped like a LinkML schema at
+     all? (required fields like id/name present, types match the LinkML
+     metamodel's JSON Schema). Uses linkml's own Linter.validate_schema(),
+     the same check `linkml validate schema.yml` runs.
+
+  2. Reference resolution — do all the internal references actually
+     resolve? A schema can satisfy the metamodel shape while still
+     pointing a class's slots at an undefined slot name, or is_a at an
+     undefined parent class, or a slot's range at an undefined
+     class/type/enum. SchemaView resolves every class's induced slots,
+     which forces exactly those lookups and raises a clear ValueError
+     if something doesn't resolve.
+
+USAGE
+-----
+  python validate_schema.py schemas/bbqs.yml
+  python validate_schema.py schemas/bbqs.yml --strict   # also fail on lint warnings
+"""
+
+from __future__ import annotations
+
+import sys
+
+import click
+from linkml.linter.linter import Linter
+from linkml_runtime.utils.schemaview import SchemaView
+
+
+def validate_metamodel(schema_path: str) -> list[str]:
+    """Return a list of human-readable errors from metamodel validation."""
+    return [
+        f"{problem.rule_name}: {problem.message}"
+        for problem in Linter.validate_schema(schema_path)
+    ]
+
+
+def validate_references(schema_path: str) -> list[str]:
+    """
+    Force resolution of every class's slots (and thereby their is_a
+    parents and ranges) and collect any dangling references.
+
+    Returns an empty list if the schema loads and every reference resolves.
+    """
+    errors: list[str] = []
+    try:
+        sv = SchemaView(schema_path)
+    except Exception as e:  # malformed enough that SchemaView itself chokes
+        return [f"could not load schema: {e}"]
+
+    for class_name in sv.all_classes():
+        try:
+            sv.class_induced_slots(class_name)
+        except ValueError as e:
+            errors.append(f"class '{class_name}': {e}")
+
+    return errors
+
+
+@click.command()
+@click.argument("schema_path", type=click.Path(exists=True))
+def cli(schema_path: str) -> None:
+    """Validate SCHEMA_PATH is a well-formed, internally-consistent LinkML schema."""
+    metamodel_errors = validate_metamodel(schema_path)
+    if metamodel_errors:
+        click.echo(f"FAILED metamodel validation for {schema_path}:")
+        for err in metamodel_errors:
+            click.echo(f"  - {err}")
+        sys.exit(1)
+
+    reference_errors = validate_references(schema_path)
+    if reference_errors:
+        click.echo(f"FAILED reference validation for {schema_path}:")
+        for err in reference_errors:
+            click.echo(f"  - {err}")
+        sys.exit(1)
+
+    sv = SchemaView(schema_path)
+    click.echo(
+        f"Valid LinkML schema: {schema_path} "
+        f"({len(sv.all_classes())} classes, {len(sv.all_slots())} slots)"
+    )
+
+
+if __name__ == "__main__":
+    cli()

requirements.txt

@@ -7,6 +7,7 @@ httpx>=0.27.0
 pydantic>=2.0.0
 click>=8.0.0
 pyyaml>=6.0
-linkml-runtime>=1.7.0
+linkml>=1.8.0
 # Pinned <3.0.0 to avoid torchcodec + FFmpeg system dependency
 sentence-transformers>=2.7.0,<3.0.0
+pytest>=8.0.0

tests/conftest.py

@@ -0,0 +1,12 @@
+import sys
+from pathlib import Path
+
+REPO_ROOT = Path(__file__).parent.parent
+NEURO_GHOST_DIR = REPO_ROOT / "neuro_ghost"
+
+# neuro_ghost/*.py use flat imports (e.g. `from db import ...`), the same way
+# they resolve when run directly as `python neuro_ghost/ingest_linkml.py` —
+# so neuro_ghost/ itself, not just the repo root, must be on sys.path.
+for p in (str(REPO_ROOT), str(NEURO_GHOST_DIR)):
+    if p not in sys.path:
+        sys.path.insert(0, p)