diff --git a/.claude/skills/dandi-linkml-validation-report/SKILL.md b/.claude/skills/dandi-linkml-validation-report/SKILL.md new file mode 100644 index 00000000..a86ccbbe --- /dev/null +++ b/.claude/skills/dandi-linkml-validation-report/SKILL.md @@ -0,0 +1,105 @@ +--- +name: dandi-linkml-validation-report +description: Generate a Markdown report assessing how `dandischema/models.yaml` (the LinkML schema) validates against real DANDI Archive Dandiset metadata after migrating each instance to the latest schema version. Use when the user wants to assess schema fitness across the archive, investigate a class of validation failure across many dandisets, or compare before/after for a schema change. Covers fetching raw metadata for every dandiset (draft + every published version), migrating each instance via `dandischema.metadata.migrate`, running closed-world JSON-schema validation on successfully-migrated instances via the LinkML Python API, and aggregating per-version results into a top-level README.md bucketed by target class (Dandiset / PublishedDandiset) × schemaVersion. Versions whose metadata can't be migrated are flagged in the report; validation is skipped for them. +compatibility: Requires the `linkml-auto-converted` hatch env defined in this repo's pyproject.toml (provides linkml, linkml-runtime, dandi, typer) and network access to a DANDI Archive instance. +allowed-tools: Bash(git:*) Bash(hatch:*) Read +--- + +# DANDI LinkML validation report + +Three-stage pipeline that fetches Dandiset metadata, validates it against +`dandischema/models.yaml`, and aggregates the results into a Markdown +report. Each stage is a Typer-based script under `scripts/`; each is +idempotent and resumable. + +## When to use + +- After updating the LinkML schema (or its Pydantic source in + `dandischema.models`) — see what breaks across the archive. +- To investigate the spread of a specific validation failure across + dandisets. +- To produce a before/after diff of schema changes. + +## Prerequisites + +- The `linkml-auto-converted` hatch env exists (defined in + `pyproject.toml`). +- `dandischema/models.yaml` is present and reflects the schema you want + to validate against. Typically, you stay on the `linkml-conversion` + branch and pull the YAML from the auto-generated branch: + + ```sh + git restore --source linkml-auto-converted -- dandischema/models.yaml + ``` + +- Network access to the target DANDI instance (default: dani). + +## Workflow + +The pipeline writes everything under one flat directory: + +```sh +ROOT=linkml-validation-reports +``` + +Raw metadata is schema-independent and only fetched once; subsequent +runs reuse it. Schema-dependent files (`metadata_migrated.json`, +`validation.{json,txt}`, `SUMMARY.md`, top-level `README.md`) are +rewritten in place when the schema content changes. + +### 1. Fetch metadata + +```sh +hatch run linkml-auto-converted:python \ + .claude/skills/dandi-linkml-validation-report/scripts/fetch_metadata.py \ + $ROOT/data +``` + +Downloads `metadata.json` + `info.json` for every dandiset's draft and +every published version into `$ROOT/data///`. +Already-downloaded versions are skipped. `--refresh` is a forceful +override that re-downloads everything regardless. `--limit N` +truncates to N dandisets for smoke tests. `-i ` selects a +non-production DANDI instance. + +### 2. Migrate + validate + +```sh +hatch run linkml-auto-converted:python \ + .claude/skills/dandi-linkml-validation-report/scripts/validate_metadata.py \ + $ROOT/data --schema dandischema/models.yaml +``` + +For each version directory, runs `dandischema.metadata.migrate` on +the raw metadata first, then validates the migrated instance against +the LinkML schema (drafts → `Dandiset`, published → `PublishedDandiset`). +Writes `metadata_migrated.json` (when migration succeeds), plus +`validation.json` (structured record carrying `migration_status` and +`schema_sha256`), `validation.txt`, and `SUMMARY.md`. Versions whose +migration fails are recorded with the error and skipped for +validation. + +The resume guard is schema-aware: each `validation.json` is stamped +with the SHA-256 of the schema file's bytes, and a re-run skips a +version only when its stamp matches the current schema. So changing +`dandischema/models.yaml` (committed or uncommitted) automatically +re-runs migration and validation for every version on the next call — +no flag needed. `--refresh` is a forceful override that ignores the +stamp and re-runs everything regardless. + +### 3. Generate report + +```sh +hatch run linkml-auto-converted:python \ + .claude/skills/dandi-linkml-validation-report/scripts/generate_report.py \ + $ROOT \ + --commit-hash $(git rev-parse linkml-auto-converted) \ + --commit-date $(git show -s --format=%cI linkml-auto-converted) +``` + +Writes `$ROOT/README.md`: overall counts, then per-bucket tables +(target class × schemaVersion) with top error patterns and links to +each version's `SUMMARY.md`. Always rewritten on invocation. + +For details on the on-disk layout, JSON field shapes, and design +rationale, read the module docstrings of the three scripts directly. diff --git a/.claude/skills/dandi-linkml-validation-report/scripts/fetch_metadata.py b/.claude/skills/dandi-linkml-validation-report/scripts/fetch_metadata.py new file mode 100644 index 00000000..02a7eb49 --- /dev/null +++ b/.claude/skills/dandi-linkml-validation-report/scripts/fetch_metadata.py @@ -0,0 +1,231 @@ +#!/usr/bin/env python3 +"""Download raw ``Dandiset`` metadata from a DANDI Archive instance. + +For every dandiset on the chosen instance this script writes the raw +metadata of the draft version *and* of every published version to:: + + ///metadata.json + +Each version directory also gets an ``info.json`` with the few fields +the downstream validation and report scripts need: + + { + "dandiset_id": "000003", + "version": "0.230629.1955", # or "draft" + "is_published": true, # false for the draft version + "status": "VALID", # archive-side status + "modified": "2023-06-29T...", # ISO 8601 or null + "schema_version": "0.6.4" # raw["schemaVersion"], may be null + } + +Re-running is safe: versions whose ``metadata.json`` already exists are +skipped unless ``--refresh`` is passed. This makes it easy to resume +after a network blip or to top up a previously-fetched directory with +newly published versions. + +Example +------- +:: + + python fetch_metadata.py linkml-validation-reports/data +""" + +from __future__ import annotations + +import json +import logging +import os +from pathlib import Path + +from dandi.dandiapi import DandiAPIClient, RemoteDandiset +import typer + +logger = logging.getLogger("fetch_metadata") + +app = typer.Typer(add_completion=False, help=__doc__.splitlines()[0]) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _dump_json(data: object) -> str: + """Serialize ``data`` as pretty-printed JSON with a trailing newline. + + ``data`` is expected to be a structure of plain JSON-compatible types + (dicts, lists, strings, numbers, bools, ``None``). Datetime objects + must be converted by the caller — see how ``_fetch_version`` calls + ``.isoformat()`` before stashing values into ``info``. + """ + return json.dumps(data, indent=2) + "\n" + + +def _fetch_version( + dandiset: RemoteDandiset, + version_id: str, + *, + is_published: bool, + version_dir: Path, + refresh: bool, +) -> None: + """Fetch ``metadata.json`` and ``info.json`` for one version. + + All-or-nothing on the destination paths: this function performs the + network calls first, then writes both files to temporary paths in + ``version_dir`` and only renames them into place once both have been + written successfully. If anything fails — a raised exception or even + abrupt termination of the process — neither destination file ever + appears in a partially-written state, so the resume guard at the top + of this function can trust ``metadata.json``/``info.json`` existence + as a signal that the version was previously fetched in full. (The + leftover ``.tmp`` files are harmless cruft that the next successful + fetch overwrites.) + + Parameters + ---------- + dandiset: + The ``RemoteDandiset`` object returned by the DANDI client. + version_id: + Either the published version identifier (e.g. ``"0.230629.1955"``) + or the literal string ``"draft"``. + is_published: + ``False`` for the draft version, ``True`` for any published + version. Persisted into ``info.json`` so the validator can pick + the right target class without re-querying the archive. + version_dir: + Destination directory; created if it does not yet exist. + refresh: + If ``False`` and the destination already contains both + ``metadata.json`` and ``info.json``, do nothing. + """ + metadata_file = version_dir / "metadata.json" + info_file = version_dir / "info.json" + if metadata_file.exists() and info_file.exists() and not refresh: + logger.debug("skip %s/%s (already downloaded)", dandiset.identifier, version_id) + return + + # --- Network: gather everything before touching the filesystem. --- + # ``for_version`` returns a fresh handle bound to the requested version, + # which is what ``get_raw_metadata`` and ``get_version`` need to operate on. + ds_at_version = dandiset.for_version(version_id) + raw = ds_at_version.get_raw_metadata() + version_info = ds_at_version.get_version(version_id) + + info = { + "dandiset_id": dandiset.identifier, + "version": version_id, + "is_published": is_published, + # ``status`` is a ``VersionStatus`` enum member on the client. + "status": version_info.status.value, + # ``modified`` is a non-optional ``datetime`` per the ``Version`` + # model, so an ``isoformat()`` is always safe. + "modified": version_info.modified.isoformat(), + # The raw metadata's ``schemaVersion`` field is the dimension we + # want to group by in the top-level report, so capture it now. + "schema_version": raw.get("schemaVersion"), + } + + # --- Filesystem: write to .tmp paths then rename into place. --- + # Pre-rendering the JSON before opening any file keeps any + # serialization error from leaving stray ``.tmp`` files behind. + metadata_text = _dump_json(raw) + info_text = _dump_json(info) + + version_dir.mkdir(parents=True, exist_ok=True) + metadata_tmp = metadata_file.with_suffix(metadata_file.suffix + ".tmp") + info_tmp = info_file.with_suffix(info_file.suffix + ".tmp") + metadata_tmp.write_text(metadata_text) + info_tmp.write_text(info_text) + # ``os.replace`` performs the POSIX ``rename(2)`` syscall, which the + # kernel cannot leave half-finished: either the destination ends up + # pointing at the new content, or it stays as it was before the call + # (i.e. nonexistent on the first fetch). That guarantee is what + # makes the all-or-nothing behavior above hold even under SIGKILL, + # since simply ``write_text``-ing the final paths would leave a + # truncated file behind if the process were killed mid-write. + os.replace(metadata_tmp, metadata_file) + os.replace(info_tmp, info_file) + + logger.info("fetched %s/%s", dandiset.identifier, version_id) + + +# --------------------------------------------------------------------------- +# Typer entry point +# --------------------------------------------------------------------------- + + +@app.command() +def main( + output_dir: Path = typer.Argument( + ..., + help="Directory under which //metadata.json " + "files will be written.", + ), + dandi_instance: str = typer.Option( + "dandi", + "--dandi-instance", + "-i", + help="DANDI server instance name as understood by `DandiAPIClient." + "for_dandi_instance`", + ), + refresh: bool = typer.Option( + False, + "--refresh", + help="Re-download versions whose metadata is already on disk.", + ), + limit: int | None = typer.Option( + None, + "--limit", + help="Process at most N dandisets (useful for smoke tests).", + ), + log_level: str = typer.Option("INFO", "--log-level", "-l"), +) -> None: + """Fetch metadata for all dandisets (draft + published versions).""" + logging.basicConfig( + format="[%(asctime)s] %(levelname)s %(name)s: %(message)s", + level=getattr(logging, log_level.upper()), + ) + output_dir.mkdir(parents=True, exist_ok=True) + + with DandiAPIClient.for_dandi_instance(dandi_instance) as client: + for i, dandiset in enumerate(client.get_dandisets(draft=True, order="id")): + if limit is not None and i >= limit: + break + dandiset_id = dandiset.identifier + dandiset_dir = output_dir / dandiset_id + logger.info("processing %s", dandiset_id) + + # The draft version always exists and is what new edits land on, + # so fetch it first. + try: + _fetch_version( + dandiset, + dandiset.draft_version.identifier, + is_published=False, + version_dir=dandiset_dir / "draft", + refresh=refresh, + ) + except Exception as e: + # Never let a single dandiset blow up the whole run. + logger.error("failed draft of %s: %s", dandiset_id, e) + + # Then walk every published version (skipping the draft, which + # ``get_versions`` also yields). + for v in dandiset.get_versions(): + if v.identifier == "draft": + continue + try: + _fetch_version( + dandiset, + v.identifier, + is_published=True, + version_dir=dandiset_dir / v.identifier, + refresh=refresh, + ) + except Exception as e: + logger.error("failed %s/%s: %s", dandiset_id, v.identifier, e) + + +if __name__ == "__main__": + app() diff --git a/.claude/skills/dandi-linkml-validation-report/scripts/generate_report.py b/.claude/skills/dandi-linkml-validation-report/scripts/generate_report.py new file mode 100644 index 00000000..ab386711 --- /dev/null +++ b/.claude/skills/dandi-linkml-validation-report/scripts/generate_report.py @@ -0,0 +1,369 @@ +#!/usr/bin/env python3 +"""Generate the top-level Markdown validation report. + +The report directory is expected to look like:: + + linkml-validation-reports/ + ├── README.md <-- written by this script + └── data/ + ├── 000003/ + │ ├── draft/ + │ │ ├── metadata.json + │ │ ├── info.json + │ │ ├── metadata_migrated.json + │ │ ├── validation.json + │ │ ├── validation.txt + │ │ └── SUMMARY.md + │ └── 0.230629.1955/... + └── 000004/... + +For each ``validation.json`` produced by ``validate_metadata.py``, this +script aggregates results and writes a single ``README.md`` at the top +of the report directory. + +The report contains: + + * Header with the ``linkml-auto-converted`` commit hash and commit + date (passed via ``--commit-hash`` / ``--commit-date`` so the + script doesn't need to know which branch is in play). + + * Per-bucket summary tables, where a "bucket" is the cross product + of *target class* (``Dandiset`` for drafts, ``PublishedDandiset`` + for published versions) and *schemaVersion* of the raw metadata. + + * For every bucket, the most common error patterns (after stripping + the per-file path prefix added by ``linkml-validate``) so the + reader can spot systemic issues. + + * Per-bucket index linking each version to its per-version + ``SUMMARY.md``, so the report reads naturally on GitHub or any + static markdown viewer — no HTTP server needed. + +Example +------- +:: + + python generate_report.py linkml-validation-reports \\ + --commit-hash 54085828c72b69f3b9933dbd288114a9d074ed46 \\ + --commit-date 2026-04-20T18:47:47-07:00 +""" + +from __future__ import annotations + +from collections import Counter, defaultdict +import json +import logging +from pathlib import Path + +import typer + +logger = logging.getLogger("generate_report") + +app = typer.Typer(add_completion=False, help=__doc__.splitlines()[0]) + + +# --------------------------------------------------------------------------- +# Loading + grouping +# --------------------------------------------------------------------------- + + +def _problem_pattern(problem: dict) -> str: + """Build a grouping key for one structured problem record. + + ``validate_metadata.py`` writes each problem as a dict with at + least ``severity`` and ``message`` (and, for JSON-schema-backed + validation, a ``source.validator`` keyword). The path-prefixed + text the CLI prints carries no information not already in these + fields, so we group on ``[severity] message`` and prepend the + failing JSON-schema validator keyword when available — that lets + similar errors group across dandisets without regex scrubbing. + """ + severity = problem.get("severity", "?") + message = problem.get("message", "") + src = problem.get("source") or {} + validator = src.get("validator") + prefix = f"[{severity}]" + if validator: + prefix += f" <{validator}>" + return f"{prefix} {message}" + + +def _load_records(data_dir: Path) -> list[dict]: + """Load every ``validation.json`` under ``data_dir``. + + Each record is augmented with the relative path to its per-version + ``SUMMARY.md`` so the report can link directly to it. + """ + records: list[dict] = [] + for vj in sorted(data_dir.glob("*/*/validation.json")): + try: + rec = json.loads(vj.read_text()) + except json.JSONDecodeError: + logger.warning("skipping unreadable %s", vj) + continue + # Path to per-version SUMMARY.md, relative to README.md (which + # sits one level above ``data_dir``). + rec["_summary_link"] = ( + f"data/{vj.parent.parent.name}/{vj.parent.name}/SUMMARY.md" + ) + records.append(rec) + return records + + +def _bucket_key(rec: dict) -> tuple[str, str]: + """Return the ``(class, schema_version)`` bucket key for a record. + + ``schema_version`` may legitimately be missing from very old + metadata; we fold those into a synthetic ``""`` bucket so + they're still surfaced rather than dropped. + """ + sv = rec.get("schema_version") or "" + return rec["target_class"], sv + + +# --------------------------------------------------------------------------- +# Markdown rendering +# --------------------------------------------------------------------------- + + +def _migration_failed(rec: dict) -> bool: + """True if this record's metadata could not be migrated.""" + return rec.get("migration_status") == "failed" + + +def _render_bucket( + fh, + title: str, + records: list[dict], + *, + top_n_patterns: int, +) -> None: + """Render one bucket section to ``fh``. + + Emits: + * a one-line headline counting versions / migration-failed / + valid / failing, + * a "top error patterns" list (validation problems only — + migration-failed versions never reached the validator), + * a table indexing every version with a link to its + per-version ``SUMMARY.md``. + """ + n_total = len(records) + n_mig_failed = sum(1 for r in records if _migration_failed(r)) + n_validated = n_total - n_mig_failed + # "Valid" here means migration succeeded *and* validation found no + # problems. Migration-failed versions are excluded from both + # ``valid`` and ``with-problems`` since validation never ran. + n_valid = sum( + 1 for r in records if not _migration_failed(r) and r["problem_count"] == 0 + ) + n_with_problems = n_validated - n_valid + + fh.write(f"### {title}\n\n") + headline_parts = [f"**Versions:** {n_total}"] + if n_mig_failed: + headline_parts.append(f"**Migration failed:** {n_mig_failed}") + headline_parts += [ + f"**Valid:** {n_valid}", + f"**With problems:** {n_with_problems}", + ] + fh.write("- " + " • ".join(headline_parts) + "\n\n") + + # --- Top error patterns within this bucket. --- + pattern_counter: Counter[str] = Counter() + for r in records: + for problem in r.get("problems", []): + pattern_counter[_problem_pattern(problem)] += 1 + if pattern_counter: + fh.write(f"**Top {top_n_patterns} problem patterns:**\n\n") + for pattern, count in pattern_counter.most_common(top_n_patterns): + # Backticks + escape any stray backticks in the pattern itself. + safe = pattern.replace("`", "ʼ") + fh.write(f"- `{safe}` — {count}\n") + fh.write("\n") + + # --- Per-version index table. --- + fh.write("| Dandiset | Version | Problems | API Status | Modified |\n") + fh.write("|---|---|---:|---|---|\n") + for r in sorted(records, key=lambda x: (x["dandiset_id"], x["version"])): + if _migration_failed(r): + # Migration-failed versions don't have a problem count to + # display. Render a distinct cell so the reader can spot + # them at a glance and click through to the per-version + # SUMMARY.md for the migration error. + problems_cell = f"[migration failed]({r['_summary_link']})" + elif r["problem_count"]: + problems_cell = f"[{r['problem_count']}]({r['_summary_link']})" + else: + problems_cell = f"[OK]({r['_summary_link']})" + # ``status`` and ``modified`` come from each version's ``info.json``; + # ``_attach_info`` has already stashed them onto the record so we + # can render the table without touching the filesystem here. + status = r.get("_status", "?") + modified = r.get("_modified", "?") + fh.write( + f"| {r['dandiset_id']} | {r['version']} | {problems_cell} " + f"| {status} | {modified} |\n" + ) + fh.write("\n") + + +def _attach_info(records: list[dict], data_dir: Path) -> None: + """Stitch the matching ``info.json`` fields onto each record. + + We do this once after loading so ``_render_bucket`` can render the + per-version table without re-reading the filesystem in a loop. + """ + for r in records: + info_path = data_dir / r["dandiset_id"] / r["version"] / "info.json" + try: + info = json.loads(info_path.read_text()) + r["_status"] = info.get("status") + r["_modified"] = info.get("modified") + except (FileNotFoundError, json.JSONDecodeError): + pass + + +def _render_report( + out_path: Path, + records: list[dict], + *, + commit_hash: str, + commit_date: str, + branch: str, + schema: str, + top_n_patterns: int, +) -> None: + """Write the top-level ``README.md`` based on ``records``.""" + # Group records into the (class, schemaVersion) buckets the report + # is organized around. + buckets: dict[tuple[str, str], list[dict]] = defaultdict(list) + for r in records: + buckets[_bucket_key(r)].append(r) + + with out_path.open("w") as fh: + fh.write("# DANDI metadata — LinkML validation report\n\n") + fh.write(f"- **Branch:** `{branch}`\n") + fh.write(f"- **Commit:** `{commit_hash}`\n") + fh.write(f"- **Commit date:** {commit_date}\n") + fh.write(f"- **Schema:** `{schema}`\n") + fh.write(f"- **Total dandiset versions checked:** {len(records)}\n\n") + + n_total = len(records) + n_mig_failed = sum(1 for r in records if _migration_failed(r)) + n_valid = sum( + 1 for r in records if not _migration_failed(r) and r["problem_count"] == 0 + ) + n_with_problems = n_total - n_mig_failed - n_valid + overall_parts = [f"{n_valid} valid"] + if n_mig_failed: + overall_parts.append(f"{n_mig_failed} migration-failed") + overall_parts.append(f"{n_with_problems} with problems") + fh.write( + f"**Overall:** {' / '.join(overall_parts)} " + f"out of {n_total} versions.\n\n" + ) + + # Draft section first (target class: Dandiset), then published. + for cls, heading in [ + ("Dandiset", "Draft versions (target class: `Dandiset`)"), + ( + "PublishedDandiset", + "Published versions (target class: `PublishedDandiset`)", + ), + ]: + cls_records = [r for r in records if r["target_class"] == cls] + if not cls_records: + continue + fh.write(f"## {heading}\n\n") + fh.write(f"Total: {len(cls_records)} versions.\n\n") + + # Sub-buckets: stable order — known schema versions first + # (descending so newest tends to appear first), unknowns last. + schema_versions = sorted( + {sv for c, sv in buckets if c == cls and sv != ""}, + reverse=True, + ) + if any(sv == "" for c, sv in buckets if c == cls): + schema_versions.append("") + + for sv in schema_versions: + bucket = buckets[(cls, sv)] + _render_bucket( + fh, + f"schemaVersion {sv}", + bucket, + top_n_patterns=top_n_patterns, + ) + + +# --------------------------------------------------------------------------- +# Typer entry point +# --------------------------------------------------------------------------- + + +@app.command() +def main( + report_root: Path = typer.Argument( + ..., + help="Top-level report directory, i.e. " + "linkml-validation-reports/. " + "Must contain a `data/` subdirectory of validated versions.", + ), + commit_hash: str = typer.Option( + ..., + "--commit-hash", + help="Full commit hash of the linkml-auto-converted tip " + "(included in the report header).", + ), + commit_date: str = typer.Option( + ..., + "--commit-date", + help="ISO-8601 commit date of the linkml-auto-converted tip.", + ), + branch: str = typer.Option( + "linkml-auto-converted", + "--branch", + help="Branch name to print in the report header.", + ), + schema: str = typer.Option( + "dandischema/models.yaml", + "--schema", + help="Schema path to print in the report header.", + ), + top_n_patterns: int = typer.Option( + 10, + "--top-n-patterns", + help="How many most-common problem patterns to list per bucket.", + ), + log_level: str = typer.Option("INFO", "--log-level", "-l"), +) -> None: + """Aggregate per-version validation outputs into a top-level README.md.""" + logging.basicConfig( + format="[%(asctime)s] %(levelname)s %(name)s: %(message)s", + level=getattr(logging, log_level.upper()), + ) + + data_dir = report_root / "data" + if not data_dir.is_dir(): + raise typer.BadParameter(f"no data/ directory under {report_root}") + + records = _load_records(data_dir) + _attach_info(records, data_dir) + logger.info("loaded %d validation records", len(records)) + + out_path = report_root / "README.md" + _render_report( + out_path, + records, + commit_hash=commit_hash, + commit_date=commit_date, + branch=branch, + schema=schema, + top_n_patterns=top_n_patterns, + ) + logger.info("wrote %s", out_path) + + +if __name__ == "__main__": + app() diff --git a/.claude/skills/dandi-linkml-validation-report/scripts/validate_metadata.py b/.claude/skills/dandi-linkml-validation-report/scripts/validate_metadata.py new file mode 100644 index 00000000..7ba5f2f7 --- /dev/null +++ b/.claude/skills/dandi-linkml-validation-report/scripts/validate_metadata.py @@ -0,0 +1,450 @@ +#!/usr/bin/env python3 +"""Migrate then validate downloaded dandiset metadata. + +For every dandiset version directory produced by ``fetch_metadata.py`` this +script: + + 1. Migrates the raw metadata to the latest ``Dandiset`` / + ``PublishedDandiset`` schema using + ``dandischema.metadata.migrate(skip_validation=True)``. Migration + can fail (the source metadata may be malformed in ways the migrator + can't handle) — that's recorded on the version and validation is + skipped for it. + + 2. For successfully migrated metadata, runs LinkML validation against + ``dandischema/models.yaml`` using the LinkML ``Validator`` Python + API. + +Each version directory ends up with these sibling files: + + ``metadata_migrated.json`` — the migrated metadata, written only + when migration succeeds. Verbatim + ``metadata.json`` is preserved + untouched. + + ``validation.json`` — machine-readable record. Always + written. Carries a ``migration_status`` + field (``"success"`` or ``"failed"``). + On success, the structured LinkML + ``ValidationResult`` objects (severity, + message, JSON-pointer path, validator + keyword, etc.) are stored in the + ``problems`` array. On migration + failure, ``problems`` is empty. + + ``validation.txt`` — human-readable transcript. On success, + byte-equivalent to what ``linkml-validate`` + would have printed (same ``[severity] + [source/idx] message`` template, same + ``No issues found`` banner). On + migration failure, a one-line + ``Migration failed: …`` notice instead. + + ``SUMMARY.md`` — short markdown summary of the version's + outcome (linked from the top-level + report). + +The target class is decided from the version's ``info.json``: + + * ``Dandiset`` — for the ``draft`` version + * ``PublishedDandiset`` — for any non-``draft`` (i.e. published) version + +Re-running is schema-aware. The script stamps the SHA-256 of the +schema file's bytes into each ``validation.json`` as +``schema_sha256``. On a re-run it skips a version only when the +existing record was produced against the *same* schema content. If +the schema file has changed (committed edit, uncommitted edit, +swapped to a different file — anything that changes the byte +content), migration and validation re-run automatically and the +schema-dependent files are rewritten in place. ``metadata.json`` / +``info.json`` are never touched here; they're owned by +``fetch_metadata.py``. + +``--refresh`` is a forceful override: it ignores the resume guard +and re-runs migration, validation, and the rewrite of every +schema-dependent file regardless of stamp. You shouldn't need it for +normal "I changed the schema, re-validate" workflows — those are +already automatic. + +Example +------- +:: + + python validate_metadata.py linkml-validation-reports/data \\ + --schema dandischema/models.yaml +""" + +from __future__ import annotations + +import hashlib +import json +import logging +import os +from pathlib import Path + +from linkml.validator import Validator +from linkml.validator.plugins import JsonschemaValidationPlugin +from linkml.validator.report import Severity, ValidationResult +import typer + +from dandischema.metadata import migrate + +logger = logging.getLogger("validate_metadata") + +app = typer.Typer(add_completion=False, help=__doc__.splitlines()[0]) + + +# --------------------------------------------------------------------------- +# Result rendering +# --------------------------------------------------------------------------- + + +def _result_to_dict(r: ValidationResult) -> dict: + """Serialize one ``ValidationResult`` into a JSON-friendly dict. + + Pydantic excludes the ``source`` field from default serialization + (it's an arbitrary plugin-defined object), but for JSON-schema-based + validation it carries useful grouping signals — the failing + validator keyword (e.g. ``"required"``, ``"enum"``) and the value + that triggered the failure. We pull those out by hand so the + downstream report can group by validator without re-parsing + messages. + """ + # ``instance`` echoes the full data instance back into every result, + # which would duplicate ``metadata.json`` per-problem and bloat the + # record without adding any information the consumer doesn't already + # have. Drop it. + d = r.model_dump(mode="json", exclude={"instance"}) + src = r.source + if src is not None: + d["source"] = { + "validator": getattr(src, "validator", None), + "validator_value": getattr(src, "validator_value", None), + } + return d + + +def _format_cli_line(r: ValidationResult, source_label: str) -> str: + """Format one result the way ``linkml-validate`` prints it. + + Mirrors the f-string in ``linkml/validator/cli.py`` exactly so the + transcript stays byte-equivalent to the CLI's stdout. + """ + # Match the CLI's f-string interpolation exactly: no fallback. If + # ``instance_index`` is ``None`` (e.g. a result emitted for a non-list + # instance), the literal ``"None"`` is what the CLI would print, and + # mirroring that keeps the transcript byte-equivalent. + return f"[{r.severity.value}] [{source_label}/{r.instance_index}] {r.message}" + + +def _atomic_write_json(path: Path, data: object) -> None: + """Write ``data`` to ``path`` as pretty JSON via temp + ``os.replace``. + + Same all-or-nothing pattern as ``fetch_metadata.py``: if anything + goes wrong before the rename, no file ends up at ``path``. + """ + text = json.dumps(data, indent=2) + "\n" + tmp = path.with_suffix(path.suffix + ".tmp") + tmp.write_text(text) + os.replace(tmp, path) + + +# --------------------------------------------------------------------------- +# Per-directory migration + validation +# --------------------------------------------------------------------------- + + +def _validate_one( + validator: Validator, + version_dir: Path, + *, + schema_sha256: str, + refresh: bool, +) -> None: + """Migrate then validate one ``/`` directory. + + Reads ``info.json`` (written by ``fetch_metadata.py``) to decide the + target class, attempts a migration of ``metadata.json``, and — when + migration succeeds — validates the migrated metadata against the + LinkML schema. Writes ``metadata_migrated.json`` (on success), + ``validation.json`` / ``validation.txt`` / ``SUMMARY.md``. + + The resume guard skips a version only when its existing + ``validation.json`` was produced against a schema with the same + ``schema_sha256`` we were given. A schema-content change therefore + forces an automatic re-run without ``--refresh``. + + Logs one ``INFO`` line per version describing the outcome + (``resumed``, ``migrated, validated``, or + ``migration failed; validation skipped``). + """ + metadata_file = version_dir / "metadata.json" + info_file = version_dir / "info.json" + migrated_metadata_file = version_dir / "metadata_migrated.json" + out_text = version_dir / "validation.txt" + out_json = version_dir / "validation.json" + out_md = version_dir / "SUMMARY.md" + + info = json.loads(info_file.read_text()) + is_published = bool(info.get("is_published")) + target_class = "PublishedDandiset" if is_published else "Dandiset" + + # Resume support: if we already have a JSON record for this version + # *produced against the same schema content*, and the caller didn't + # pass --refresh, leave the directory alone. A schema-content + # change makes ``existing["schema_sha256"]`` fail the equality + # check and falls through to a fresh migration + validation. + if out_json.exists() and not refresh: + try: + existing = json.loads(out_json.read_text()) + problem_count = int(existing.get("problem_count", 0)) + except (json.JSONDecodeError, ValueError): + logger.warning( + "re-validating %s — existing validation.json is unreadable", + version_dir, + ) + else: + if existing.get("schema_sha256") == schema_sha256: + logger.info( + "%s/%s — resumed from existing record (%s, %d problems)", + version_dir.parent.name, + version_dir.name, + existing.get("migration_status", "success"), + problem_count, + ) + return + + raw = json.loads(metadata_file.read_text()) + # ``@context`` is a JSON-LD framing field that's not part of the + # ``Dandiset`` / ``PublishedDandiset`` LinkML class definitions, so a + # closed-world JSON-schema check flags it as an unexpected property + # (see linkml/linkml#3442). Strip it before migration/validation so + # we don't drown the report in noise that has nothing to do with the + # model. + raw.pop("@context", None) + + # --- Migration step. --- + # ``skip_validation=True`` keeps ``migrate`` from running its own + # internal Pydantic validation; we want to validate against the + # *LinkML* schema afterward, and we don't want a Pydantic failure to + # mask a successful structural migration. + migration_status: str + migration_error: str | None + migrated: dict | None + try: + migrated = migrate(raw, skip_validation=True) + except Exception as e: + # Migration helpers raise ``NotImplementedError`` / + # ``ValueError`` for known unsupported inputs, but ``migrate`` + # also rewires Pydantic-level traversals where any number of + # other errors can surface. Catch broadly so one bad version + # doesn't abort the run. + migrated = None + migration_status = "failed" + migration_error = repr(e) + else: + migration_status = "success" + migration_error = None + + # --- Branch on migration outcome. --- + results: list[ValidationResult] + transcript_lines: list[str] + exit_code: int | None + + if migration_status == "success": + assert migrated is not None + + # Persist the migrated metadata so the report can link to it + # and the user can inspect what was actually validated. + _atomic_write_json(migrated_metadata_file, migrated) + + report = validator.validate(migrated, target_class=target_class) + results = report.results + + # ``linkml-validate``'s exit code is 1 iff any ERROR-severity + # result is present, else 0. Replicate that for downstream + # consumers that key off ``exit_code``. + has_error = any(r.severity is Severity.ERROR for r in results) + exit_code = 1 if has_error else 0 + + # The CLI prints ``loader.source`` as the bracketed path; for a + # file-backed JsonLoader that's the file path string. Point + # readers at the migrated file since that's what was actually + # validated. + source_label = str(migrated_metadata_file) + if results: + transcript_lines = [_format_cli_line(r, source_label) for r in results] + else: + # Mirrors the CLI's success banner so byte-equivalence holds + # in the zero-results case too. + transcript_lines = ["No issues found"] + else: + # Migration failed — leave any prior ``metadata_migrated.json`` + # alone (it would belong to a previous successful run) and + # don't try to validate. + results = [] + exit_code = None + transcript_lines = [f"Migration failed: {migration_error}"] + + out_text.write_text("\n".join(transcript_lines) + "\n") + + # --- Persist the structured record. --- + record = { + "dandiset_id": info["dandiset_id"], + "version": info["version"], + "is_published": is_published, + "target_class": target_class, + "schema_version": info.get("schema_version"), + # SHA-256 of the schema file's bytes — drives the resume guard + # on the next run, so a schema-content change re-validates + # automatically without needing ``--refresh``. + "schema_sha256": schema_sha256, + "migration_status": migration_status, + "migration_error": migration_error, + "exit_code": exit_code, + "problem_count": len(results), + "problems": [_result_to_dict(r) for r in results], + } + _atomic_write_json(out_json, record) + + # --- Per-version markdown summary, linked from the top-level report. --- + md_lines = [ + f"# Validation summary — {info['dandiset_id']} @ {info['version']}", + "", + f"- **Target class:** `{target_class}`", + f"- **API status:** {info.get('status')}", + f"- **Modified:** {info.get('modified')}", + f"- **Source schemaVersion:** {info.get('schema_version')}", + f"- **Migration status:** `{migration_status}`", + ] + if migration_status == "success": + md_lines += [ + f"- **Equivalent `linkml-validate` exit code:** {exit_code}", + f"- **# problems:** {len(results)}", + "", + "## Files", + "", + "- [`metadata.json`](metadata.json) — raw metadata as fetched from the archive", + "- [`metadata_migrated.json`](metadata_migrated.json)" + " — metadata after migration to the latest schema", + "- [`validation.txt`](validation.txt) — `linkml-validate`-equivalent transcript", + "- [`validation.json`](validation.json) — structured validation record", + "", + ] + if results: + md_lines += [ + "## First 20 problems", + "", + "```", + *transcript_lines[:20], + "```", + ] + if len(results) > 20: + md_lines.append( + f"_… {len(results) - 20} more — see " + "[`validation.txt`](validation.txt)._" + ) + else: + md_lines += [ + "", + "## Migration failure", + "", + "Validation was **not** run because the metadata could not be", + "migrated to the latest schema version.", + "", + "```", + f"{migration_error}", + "```", + "", + "## Files", + "", + "- [`metadata.json`](metadata.json) — raw metadata as fetched from the archive", + "- [`validation.txt`](validation.txt) — migration-failure notice", + "- [`validation.json`](validation.json) — structured record (no validation results)", + "", + ] + out_md.write_text("\n".join(md_lines) + "\n") + + if migration_status == "success": + logger.info( + "%s/%s — migrated, validated as %s (%d problems)", + version_dir.parent.name, + version_dir.name, + target_class, + len(results), + ) + else: + logger.info( + "%s/%s — migration failed; validation skipped", + version_dir.parent.name, + version_dir.name, + ) + + +# --------------------------------------------------------------------------- +# Typer entry point +# --------------------------------------------------------------------------- + + +@app.command() +def main( + root: Path = typer.Argument( + ..., + help="Top-level directory produced by fetch_metadata.py " + "(contains //metadata.json files).", + ), + schema: Path = typer.Option( + ..., + "--schema", + help="Path to dandischema/models.yaml (the LinkML schema).", + ), + refresh: bool = typer.Option( + False, + "--refresh", + help=( + "Forceful override: re-migrate and re-validate every version, " + "ignoring the resume guard. Not needed for normal " + "schema-changed-so-revalidate workflows — those already happen " + "automatically when the schema file's content changes." + ), + ), + log_level: str = typer.Option("INFO", "--log-level", "-l"), +) -> None: + """Migrate + validate every ``//metadata.json`` under ``root``.""" + logging.basicConfig( + format="[%(asctime)s] %(levelname)s %(name)s: %(message)s", + level=getattr(logging, log_level.upper()), + ) + + # Build one ``Validator`` and reuse it across every version: parsing + # the schema is the expensive part, and the plugin configuration + # below matches the ``linkml-validate`` CLI default + # (``JsonschemaValidationPlugin`` with ``closed=True``), so the + # results we collect are the same ones the CLI would have emitted. + validator = Validator( + schema, + validation_plugins=[JsonschemaValidationPlugin(closed=True)], + ) + + # SHA-256 of the schema file's bytes — stamped into every + # ``validation.json`` so a future run can tell whether the schema + # has changed since that record was produced. Computed once here so + # we don't re-hash per version. + schema_sha256 = hashlib.sha256(schema.read_bytes()).hexdigest() + logger.info("schema sha256 = %s", schema_sha256) + + version_dirs = sorted( + p for p in root.glob("*/*") if p.is_dir() and (p / "metadata.json").is_file() + ) + logger.info("found %d version directories to process", len(version_dirs)) + + for vd in version_dirs: + try: + _validate_one(validator, vd, schema_sha256=schema_sha256, refresh=refresh) + except Exception as e: + # Never let one broken dandiset abort the whole run. + logger.exception("processing failed for %s: %s", vd, e) + + +if __name__ == "__main__": + app() diff --git a/.github/workflows/test-linkml-behavior.yml b/.github/workflows/test-linkml-behavior.yml new file mode 100644 index 00000000..f1767573 --- /dev/null +++ b/.github/workflows/test-linkml-behavior.yml @@ -0,0 +1,26 @@ +name: LinkML behavior tests + +# Runs the LinkML behavior tests under the `linkml-behavior-test` hatch +# env, which does not pin a LinkML version, so the daily schedule +# surfaces upstream LinkML regressions early. +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + schedule: + - cron: '0 6 * * *' + workflow_dispatch: + +jobs: + test: + runs-on: ubuntu-latest + steps: + - name: Check out repository + uses: actions/checkout@v6 + + - name: Install Hatch + uses: pypa/hatch@install + + - name: Run LinkML behavior tests + run: hatch run linkml-behavior-test:test diff --git a/.github/workflows/typing-linkml-behavior.yml b/.github/workflows/typing-linkml-behavior.yml new file mode 100644 index 00000000..37423f44 --- /dev/null +++ b/.github/workflows/typing-linkml-behavior.yml @@ -0,0 +1,22 @@ +name: Type-check LinkML behavior tests + +# Static type-checking for `tests/linkml_behavior/` under the +# `linkml-behavior-typing` hatch env. +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +jobs: + type-check: + runs-on: ubuntu-latest + steps: + - name: Check out repository + uses: actions/checkout@v6 + + - name: Install Hatch + uses: pypa/hatch@install + + - name: Type-check LinkML behavior tests + run: hatch run linkml-behavior-typing:check diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5de722e6..f333a1c5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,5 +1,13 @@ # See https://pre-commit.com for more information # See https://pre-commit.com/hooks.html for more hooks + +# Each migration-playbook demo exhibit keeps its source schemas and their raw +# `gen-pydantic` / `gen-json-schema` output under a `schemas/` folder, kept for +# reference. Treat that whole folder as generated/reference material — don't lint +# or reformat it (e.g. flake8 F401 on the generator's fixed import block). New +# exhibits following the `schemas/` convention are covered automatically. +exclude: 'migration_to_linkml_playbook/.*/schemas/' + repos: - repo: https://github.com/pre-commit/pre-commit-hooks rev: v6.0.0 diff --git a/dandischema/models_importstab.py b/dandischema/models_importstab.py new file mode 100644 index 00000000..7a98b607 --- /dev/null +++ b/dandischema/models_importstab.py @@ -0,0 +1,21 @@ +from .models_linkml import * # noqa: F401,F403 +from .models_orig import ( # noqa: F401 + DANDI_INSTANCE_URL_PATTERN, + DANDI_NSKEY, + get_schema_version, +) + +# TODO: temporary imports of consts etc which might need to be 'redone' +# so we do not duplicate them + +# Deprecated aliases mirroring the ones `models_orig.py` keeps for backward +# compatibility, `PublishedDandiset` and `PublishedAsset` having been consolidated +# into `Dandiset` and `Asset` respectively. `metadata.SCHEMA_MAP` still names them, +# so `publish_model_schemata` needs them to resolve. Temporary: remove these after the +# follow-up to dandi/dandi-schema#419 that drops the aliases from `models_orig.py`. +PublishedDandiset = Dandiset # noqa: F405 +PublishedAsset = Asset # noqa: F405 + + +# TODO: do the extra tune ups like linking extra validations etc, +# potentially copied from models_orig.py diff --git a/dandischema/models_merge.yaml b/dandischema/models_merge.yaml new file mode 100644 index 00000000..e0567856 --- /dev/null +++ b/dandischema/models_merge.yaml @@ -0,0 +1,18 @@ +# This file specifies a partial schema to be merged with the auto LinkML translation +# `dandischema.models`. It is a place to specify elements of the target dandischema +# LinkML schema that are not translatable from or not available in `dandischema.models` + +slots: + schemaKey: + designates_type: true + required: true + +classes: + BareAsset: + slot_usage: + wasGeneratedBy: + range: Any + Dandiset: + slot_usage: + wasGeneratedBy: + range: Project diff --git a/dandischema/models_overlay.yaml b/dandischema/models_overlay.yaml new file mode 100644 index 00000000..6bcb0144 --- /dev/null +++ b/dandischema/models_overlay.yaml @@ -0,0 +1,31 @@ +name: dandi-schema +id: https://schema.dandiarchive.org/s/dandi/v0.7 +version: 0.7.0 +status: eunal:concept-status/DRAFT + +prefixes: + dandiasset: http://dandiarchive.org/asset/ + DANDI: http://dandiarchive.org/dandiset/ + dandi: http://schema.dandiarchive.org/ + dcite: http://schema.dandiarchive.org/datacite/ + dct: http://purl.org/dc/terms/ + linkml: https://w3id.org/linkml/ + nidm: http://purl.org/nidash/nidm# + ORCID: https://orcid.org/ + owl: http://www.w3.org/2002/07/owl# + PATO: http://purl.obolibrary.org/obo/PATO_ + pav: http://purl.org/pav/ + prov: http://www.w3.org/ns/prov# + rdfa: http://www.w3.org/ns/rdfa# + rdf: http://www.w3.org/1999/02/22-rdf-syntax-ns# + rdfs: http://www.w3.org/2000/01/rdf-schema# + ROR: https://ror.org/ + RRID: "https://scicrunch.org/resolver/RRID:" + rs: http://schema.repronim.org/ + schema: http://schema.org/ + skos: http://www.w3.org/2004/02/skos/core# + spdx: http://spdx.org/licenses/ + uuid: http://uuid.repronim.org/ + xsd: http://www.w3.org/2001/XMLSchema# + +default_prefix: dandi diff --git a/docs/designs/20260411-schemaKey-mismatch-report.md b/docs/designs/20260411-schemaKey-mismatch-report.md new file mode 100644 index 00000000..5a53a342 --- /dev/null +++ b/docs/designs/20260411-schemaKey-mismatch-report.md @@ -0,0 +1,239 @@ +# schemaKey Mismatch Report + +## Related issues and PRs + +- [dandi/dandi-schema#389](https://github.com/dandi/dandi-schema/issues/389) -- + **Handle LinkML migration issue of `pydantic2linkml: Impossible to generate + slot usage entry for the`** (open). The parent issue for this investigation. + Documents 54 pydantic2linkml conversion errors, 41 of which concern + `schemaKey` slot usage. Includes the open TODO items about figuring out why + `schemaKey` differs from class name for Published/Bare models. +- [dandi/dandi-schema#388](https://github.com/dandi/dandi-schema/issues/388) -- + **Establish at least 8 issues for specific groups of problems in converted + linkml** (open). Umbrella issue cataloguing all pydantic2linkml translation + problems, grouped by category. +- [dandi/dandi-schema#385](https://github.com/dandi/dandi-schema/pull/385) -- + **Replace discriminated unions with simple unions in models** (open PR). + Removes `Field(discriminator="schemaKey")` in favor of plain `Union[...]`, + since LinkML has no discriminated union equivalent + ([dandi/pydantic2linkml#39](https://github.com/dandi/pydantic2linkml/issues/39)). + Each union member still has a distinct `schemaKey: Literal[...]`, so + Pydantic's smart union mode resolves correctly without an explicit + discriminator. Has a pending TODO to analyze effects on the Meditor. +- [dandi/dandi-schema#244](https://github.com/dandi/dandi-schema/issues/244) -- + **Use discriminated unions to improve validation errors** (closed). The + original issue that introduced discriminated unions on `schemaKey` to improve + validation error messages for `Union[Person, Organization]` contributor + fields. Now being reversed by #385 for LinkML compatibility. +- [dandi/dandi-schema#205](https://github.com/dandi/dandi-schema/issues/205) -- + **Overhaul models so that "unfinished" metadata can be represented without + cheating Pydantic** (open). Proposes separate draft vs published models so + that draft metadata does not need `model_construct()` to bypass validation. + Directly relevant to the Bare/Published model split that causes the schemaKey + mismatch. +- [dandi/dandi-schema#77](https://github.com/dandi/dandi-schema/pull/77) -- + **make schemaKey required and improve validation and migration functions** + (merged PR). Made `schemaKey` a required field in JSON Schema output and added + validation logic. +- [dandi/dandi-schema#68](https://github.com/dandi/dandi-schema/pull/68) -- + **ensure schemaKeys are set properly** (merged PR). Early work to set up + schemaKey defaults and validation across all models. +- [dandi/dandi-schema#13](https://github.com/dandi/dandi-schema/pull/13) -- + **Fix/schemakey metaclass** (merged PR). Original implementation of the + schemaKey metaclass and the `enum` -> `const` conversion in JSON Schema + export. + +## Summary + +Three pydantic model classes in `dandischema/models.py` have `schemaKey` values +that do not match their class name: + +| Class | schemaKey | Expected | +|--------------------|-------------|-------------------| +| `BareAsset` | `"Asset"` | `"BareAsset"` | +| `PublishedDandiset` | `"Dandiset"` | `"PublishedDandiset"` | +| `PublishedAsset` | `"Asset"` | `"PublishedAsset"` | + +(`Asset` inherits `"Asset"` from `BareAsset` and does match its class name, so +it is not a mismatch.) + +## Why it was done this way + +### 1. Deliberate validator logic + +The `ensure_schemakey` validator (`models.py:570-582`) has explicit special-case +logic that compensates for the mismatch: + +```python +if "Published" in cls.__name__: + tempval = "Published" + tempval # "Dandiset" -> "PublishedDandiset" +elif "BareAsset" == cls.__name__: + tempval = "Bare" + tempval # "Asset" -> "BareAsset" +if tempval != cls.__name__: + raise ValueError(...) +``` + +This proves the mismatch was intentional, not accidental. + +### 2. Conceptual collapsing in JSON Schema + +When exporting to JSON Schema (`__get_pydantic_json_schema__`, line 659-664), +`schemaKey` is emitted as a `const`: + +```python +if prop == "schemaKey": + if "enum" in value and len(value["enum"]) == 1: + value["const"] = value["enum"][0] + del value["enum"] + else: + value["const"] = value["default"] +``` + +This means: +- `BareAsset`, `Asset`, `PublishedAsset` all produce `"schemaKey": {"const": "Asset"}` +- `Dandiset`, `PublishedDandiset` both produce `"schemaKey": {"const": "Dandiset"}` + +The design intent was that from a JSON Schema / API consumer perspective, there +are only two top-level entity kinds: **Asset** and **Dandiset**. The +Bare/Published distinctions are Python model hierarchy implementation details. + +## How schemaKey is used + +### dandi-archive backend (`dandiapi/`) + +1. **Validation** (`dandiapi/api/services/metadata/__init__.py`): The archive + explicitly calls `validate(metadata, schema_key='PublishedAsset')` and + `validate(..., schema_key='PublishedDandiset')` -- passing the **class name** + as `schema_key`, not the schemaKey value. The `validate()` function in + dandischema (`metadata.py:328`) uses the `schema_key` parameter to do + `getattr(models, schema_key)` to look up the pydantic class directly. So the + archive backend **bypasses schemaKey entirely** for choosing which model to + validate against. + +2. **Schema endpoint** (`dandiapi/api/views/schema.py`): Exposes JSON schemas + for `Dandiset`, `Asset`, `PublishedDandiset`, `PublishedAsset` via + `?model=`. The mapping uses `__name__` (class name), not + `schemaKey`. So again, the model identity is carried by the Python class name, + not by `schemaKey`. + +3. **Default metadata construction** (`dandiapi/api/services/version/metadata.py`, + `dandiapi/api/models/version.py`): When constructing default metadata dicts, + the archive hardcodes `'schemaKey': 'Dandiset'` and `'schemaKey': 'Asset'`. + It never writes `'PublishedDandiset'`, `'PublishedAsset'`, or `'BareAsset'` + as schemaKey values. + +4. **Metadata stripping** (`dandiapi/api/models/version.py:196-203`): The + `strip_metadata` method explicitly strips `schemaKey` from `access` sub-objects, + treating it as a computed/server-controlled field. + +### dandi-archive frontend (Meditor) + +1. **Schema fetching** (`web/src/stores/dandiset.ts:112-122`): The Meditor + fetches the JSON Schema from the API endpoint + (`/api/schemas/?model=Dandiset`). This returns the `Dandiset` model's JSON + Schema where `schemaKey` has `"const": "Dandiset"`. + +2. **Meditor types** (`web/src/components/Meditor/types.ts:41-48`): The type + `SchemaKeyPropertiesIntersection` expects schemas to have: + ```typescript + schemaKey: { + type: 'string'; + const: string; // expects a const value + }; + ``` + The Meditor treats `schemaKey` as a read-only const field. It does **not** + use the schemaKey value to dispatch between different model types; it relies + on the JSON Schema structure itself. + +3. **Discriminator usage in Vue components** + (`web/src/components/DLP/OverviewTab.vue`, `web/src/utils/cff.ts`): The + frontend uses `schemaKey` to discriminate between `Person` and `Organization` + in contributor lists, but this is among truly distinct types -- not among + Bare/Published variants. + +4. **VJSF rendering**: The Meditor uses VJSF (Vue JSON Schema Forms) which + renders forms from JSON Schema. The `schemaKey` field's `const` constraint + means it appears as a non-editable fixed value. The Meditor's `utils.ts` + validates using Ajv against the schema, and the `const: "Dandiset"` constraint + means submitted metadata must have `schemaKey: "Dandiset"`. + +### dandi-cli + +- `dandi-cli` uses `schemaKey` as a **discriminator** for pydantic tagged unions + (e.g., `Field(discriminator="schemaKey")` at `models.py:1284` for + `Union[Person, Organization, Software, Agent]`). +- In tests, `BareAsset` instances are constructed with `schemaKey="Asset"`. +- The `schemaKey` value `"Session"` vs `"Activity"` is used to filter + `wasGeneratedBy` entries (`test_metadata.py:528`). + +### dandischema validation (`metadata.py`) + +- `SCHEMA_MAP` maps class names to JSON Schema files: `"Dandiset"` -> + `"dandiset.json"`, `"PublishedDandiset"` -> `"published-dandiset.json"`, etc. +- `validate()` accepts an explicit `schema_key` parameter (class name). If not + provided, it falls back to `obj.get("schemaKey")` -- and since stored objects + have `schemaKey: "Dandiset"` (not `"PublishedDandiset"`), this fallback would + select the wrong (base) model for Published variants. +- This is why dandi-archive always passes `schema_key='PublishedDandiset'` + explicitly. + +## Impact of making schemaKey match model name + +### What would change + +If `BareAsset.schemaKey` becomes `"BareAsset"`, `PublishedDandiset.schemaKey` +becomes `"PublishedDandiset"`, and `PublishedAsset.schemaKey` becomes +`"PublishedAsset"`: + +1. **dandischema `ensure_schemakey` validator**: The special-case prefix logic + can be removed -- each class simply checks `val == cls.__name__`. + +2. **dandischema `validate()` fallback**: The `obj.get("schemaKey")` fallback + path would now correctly resolve to the right model for Published variants. + +3. **JSON Schema output**: Each model would emit its own unique `const` value, + enabling true type discrimination in JSON Schema. + +4. **dandi-archive backend**: Hardcoded `'schemaKey': 'Dandiset'` and + `'schemaKey': 'Asset'` in metadata construction would need to stay as-is for + draft versions (which use `Dandiset`/`Asset` models) but + `PublishedDandiset`/`PublishedAsset` validation already passes `schema_key` + explicitly so no change needed there. + +5. **dandi-archive Meditor**: The schema endpoint returns the `Dandiset` model's + schema (not `PublishedDandiset`), so `const: "Dandiset"` stays the same for + the editor. No Meditor change needed. + +6. **Existing published metadata**: All published dandiset metadata in the + database has `schemaKey: "Dandiset"` and `schemaKey: "Asset"`. A migration + would be needed to update existing records if we want consistency, or the + Published models must accept both old and new values during a transition + period. + +### Benefits for LinkML + +- `schemaKey` can serve as a proper **type designator** for deserialization +- Satisfies LinkML's **monotonic slot constraint** (each class gets a unique + const, no conflicting overrides in slot_usage) +- Enables correct round-tripping: serialize -> schemaKey -> deserialize to the + right class + +### Risks + +- **Data migration**: Existing records in the archive database and published + metadata files contain `schemaKey: "Dandiset"` / `"Asset"` for what are + actually `PublishedDandiset` / `PublishedAsset` instances. Any consumer that + matches on the exact string would need updating. +- **Schema version boundary**: This is a semantic change that ideally coincides + with a schema version bump. + +## Recommendation + +Making `schemaKey` match the model name is the correct path forward for the +LinkML migration. The current mismatch exists purely as a legacy design choice +that treats Bare/Published as invisible variants. In practice, the archive +backend already works around this by passing explicit `schema_key` parameters. +The Meditor is unaffected since it only deals with draft `Dandiset` metadata. + +The change should be coordinated with a schema version bump and a data migration +plan for existing published records. diff --git a/docs/designs/migration_to_linkml_playbook/OVERVIEW.md b/docs/designs/migration_to_linkml_playbook/OVERVIEW.md new file mode 100644 index 00000000..7329209a --- /dev/null +++ b/docs/designs/migration_to_linkml_playbook/OVERVIEW.md @@ -0,0 +1,168 @@ +# Migration to LinkML — Playbook + +> **Status:** in progress. Active development branches: `linkml-conversion` (+ its patch-queue branches), `linkml-auto-converted` (translation output). +> **Entry point.** Read this file first. It links out to everything else; load deeper files only as a step needs them. +> **This playbook is self-updating.** Anyone working on the migration — human or AI assistant — must keep it current as new facts surface. See [Keeping this playbook current](#keeping-this-playbook-current) for what to update and where. + +## Problem + +Migrate `dandischema` from its current Pydantic-defined schema (`dandischema/models.py`) to a **LinkML-defined** schema, so that LinkML becomes the single source of truth and Pydantic models / JSON Schemas are *generated from* it. + +## Success criteria + +The migration is done when **all three** hold: + +1. **Behavioral parity with current Pydantic models.** Data instances accepted/rejected by today's `dandischema/models.py` are accepted/rejected the same way when validated against the LinkML schema. +2. **Generated Pydantic models are drop-in replacements** for the hand-written ones, usable by: + - this repo (`dandischema`), + - [`dandi/dandi-cli`](https://github.com/dandi/dandi-cli), + - [`dandi/dandi-archive`](https://github.com/dandi/dandi-archive). +3. **Generated JSON Schema** matches what the `Dandiset` Pydantic model currently emits, well enough to: + - validate data instances, and + - drive the dandi-archive frontend (form generation / UI). + +## How the translation is wired today + +The conversion is orchestrated by the shell script **[`tools/linkml_conversion`](../../../tools/linkml_conversion)**, which delegates the actual translation work to Hatch scripts defined in `pyproject.toml`. Mental model: + +- **Sources live on `linkml-conversion`** (and on the patch-queue branches it lists — currently `master` and `remove-discriminated-unions`). This is where you edit: + - the Pydantic source: `dandischema/models.py`, + - the LinkML-side inputs consumed by the translator: `dandischema/models_merge.yaml` (passed via `-M`: deep merge — dicts merge recursively, lists append, file wins only on scalars and type mismatches), `dandischema/models_overlay.yaml` (passed via `-O`: shallow merge — top-level keys only). See [`context/roles/linkml.md`](context/roles/linkml.md#-m-vs--o-semantics-verified-against-pydantic2linkmls-source--toolspy770809) for the full per-type breakdown. + - the import stub: `dandischema/models_importstab.py` (installed as `models.py` on the output branch — see step 4 below). +- **`./tools/linkml_conversion` runs the translation** and writes the result to the **`linkml-auto-converted`** branch (checkout flips during the script; tree must be clean before running). Order of stages: + 1. Apply the **patch queue** of branches on top of `linkml-conversion` — see [`context/patch-queue.md`](context/patch-queue.md). Order in that list matters. + 2. **Pydantic → LinkML** via `hatch run linkml-auto-converted:2linkml`. Pipeline: + ``` + pydantic2linkml -M models_merge.yaml -O models_overlay.yaml dandischema.models + | sed (scrub memory-address/line-number noise for stable output) + | tools/linkml_conversion_tools/sanitize-yaml + > dandischema/models.yaml + ``` + The translator is [`pydantic2linkml`](https://github.com/dandi/pydantic2linkml); it consumes `models_merge.yaml` (`-M`) and `models_overlay.yaml` (`-O`) directly. The `sed` filter and `sanitize-yaml` are stabilization/cleanup. + 3. **Rename dance:** `git mv dandischema/models.py dandischema/models_orig.py`, then `git mv dandischema/models_importstab.py dandischema/models.py`. On `linkml-auto-converted`, the file at `dandischema/models.py` is the import stub from [`dandischema/models_importstab.py`](../../../dandischema/models_importstab.py) on `linkml-conversion` — read it there for current content. It re-exports the generated models from `models_linkml`, currently also pulls a few constants forward from `models_orig.py`, and defines the deprecated `PublishedDandiset`/`PublishedAsset` aliases that `metadata.SCHEMA_MAP` still resolves through `getattr` (tracked for removal in [#439](https://github.com/dandi/dandi-schema/issues/439)). The stub is an evolving file; how those remaining `models_orig` imports get resolved over time is itself part of the open work. The original Pydantic source is preserved on `linkml-auto-converted` as `models_orig.py`. + 4. **LinkML → downstream artifacts** via `./tools/linkml_conversion_fromlinkml`, which orchestrates three Hatch scripts in order: + - `2pydantic` → runs LinkML's `gen-pydantic --black --template-dir tools/linkml_conversion_tools/pydantic_templates dandischema/models.yaml > dandischema/models_linkml.py`. The Pydantic Jinja templates under `tools/linkml_conversion_tools/pydantic_templates/` are the customization point for the generated Pydantic. + - `2json` → for each target class in `{Dandiset, Asset}`, runs `gen-json-schema -t dandischema/models.yaml` → `dandischema/models_linkml/.json`. Files are then lowercased (`Dandiset.json` → `dandiset.json`). `PublishedDandiset` and `PublishedAsset` were dropped from this list once #419 left the LinkML schema without such classes; asking `gen-json-schema` for a class the schema lacks exits 0 and writes a rootless schema that accepts any object, so the omission is deliberate. + - `pydantic2json` → runs `python tools/pubschemata.py` → `dandischema/models_pydantic/*.json`. The script is neutral about *which* models it serializes: it calls `publish_model_schemata`, which reaches the models via `from . import models` (`dandischema/metadata.py`). What decides the answer is **where this stage sits in `linkml_conversion`** — it runs *after* the rename dance in step 3, by which point `dandischema/models.py` is the import stub (`from .models_linkml import *`). So what lands here is dandischema's own publishing path (Pydantic's `model_json_schema` via `TransitionalGenerateJsonSchema`) applied to the **generated** models, which is the shape `dandi-archive` actually receives. Run *before* the rename dance, the same script would emit the original models' schemata instead. The giveaway in the current output is the `linkml_meta` keys, which only `gen-pydantic` emits. + 5. `pre-commit run --all` (best effort), then a commit. +- **Outputs on `linkml-auto-converted`** (all produced by the pipeline above — do not hand-edit): + - `dandischema/models.yaml` — LinkML schema + - `dandischema/models.py` — the installed import stub + - `dandischema/models_orig.py` — original Pydantic source, preserved for reference and for the constants the stub still imports from it + - `dandischema/models_linkml.py` — generated Pydantic + - `dandischema/models_linkml/*.json` — JSON Schemas derived **from the LinkML schema** (one per target class, lowercased filenames) + - `dandischema/models_pydantic/*.json` — JSON Schemas emitted by dandischema's own `publish_model_schemata` path over the **generated** models, via `tools/pubschemata.py`. The directory name is misleading; see step 4 above for why this is not an original-Pydantic baseline. Published schemata for the pre-migration models live at . +- **`tools/linkml_conversion_tools/`** is a general drawer for any tool convenient to the LinkML migration. It is **not** structurally divided into "pipeline" vs "auxiliary" — files just live here, and some of them happen to be wired into the current pipeline. New migration-related tools belong here; whether they end up wired into the pipeline is a separate decision. As of now: + - `sanitize-yaml` → wired in as the final pipe stage of `2linkml`. Internally a sub-pipeline that runs three Python helpers in order, each as a stdin→stdout filter: + 1. `remove_notes_by_pattern.py` — strips `notes:` entries matching a configured set of `Removal` rules. Each rule pairs a regex with an optional tuple of paths confining where it applies, a path being the sequence of mapping keys and sequence indices leading from the document root to a node. Three scoping modes, so a note can be suppressed in one place while staying legitimate elsewhere: + - **no paths** (`None`) — the rule applies to every `notes` in the document. + - **path ending in `"notes"`** — exact, non-recursive: that one node only, e.g. `("classes", "Dandiset", "slot_usage", "wasGeneratedBy", "notes")`. + - **any other path** — a subtree root: every `notes` at or below it, e.g. `("classes", "Dandiset")`. + + A `notes` list left empty by a removal is dropped entirely. + 2. `remove_slot_usage_schemakey.py` — strips `schemaKey` entries inside `slot_usage` blocks. + 3. `sort_license_type_permissible_values.py` — sorts `enums.LicenseType.permissible_values` alphabetically for stable output. + + All three are run inside the `linkml-auto-converted` Hatch env. To add a new sanitization step, append another pipe to `sanitize-yaml` — that's its documented extension point ("Add further sanitization steps here as additional pipes"). + - `pydantic_templates/` → wired in; consumed by `gen-pydantic --template-dir` in `2pydantic`. + - `find_schemakey_mismatches.py` → not currently called from anywhere; an on-demand tool that prints Pydantic models where the `schemaKey` default ≠ class name. Available to run by hand when a schemaKey question comes up. + + Verify a tool's current wiring from source before assuming — pipeline membership can change without renaming. +- **LinkML-behavior test envs** (separate from `tox`): `tests/linkml_behavior/` runs under its own Hatch envs: + - `hatch run linkml-behavior-test:test` — runs the behavior tests under `pytest`. + - `hatch run linkml-behavior-typing:check` — runs `mypy` against the tests. + + Both envs are **detached** (don't install `dandischema`); they exist because these tests probe LinkML itself, not our package. +- **LinkML-semantics tests:** `tests/linkml_behavior/` is **not** a parity harness for our migration. Each subdirectory pins a specific *LinkML upstream behavior* that the generated dandischema LinkML relies on. Extend these only when a new LinkML semantic our schema depends on needs a contract test; parity testing of our migration belongs elsewhere (see [Approach](#approach--repeatable-procedure)). Current topics: + - `required_refinement/` — exercises `required: False -> True` via `slot_usage`, defending against the issue tracked in [#405](https://github.com/dandi/dandi-schema/issues/405). + - `range_refinement/` — exercises respecifying the `range` of an inherited multivalued slot via `slot_usage`, both narrowing it to a subclass of the inherited range and widening it to `Any` constrained by an `any_of`. These back the `wasGeneratedBy` range overrides carried in `dandischema/models_merge.yaml`. Its cases also depend on `designates_type: true` expanding a class-valued range over the class's descendants, so a regression there surfaces as a failure here; that behavior itself is documented under the `designates_type` finding in [`findings.md`](findings.md), with a runnable exhibit at [`tools/type-designator-demo/`](tools/type-designator-demo/). + + `tests/linkml_behavior/` and each topic directory under it are **Python packages** (they carry an `__init__.py`). Those markers are **required**, not decorative: without them both pytest's default "prepend" import mode and `mypy` resolve the identically named modules in sibling topics (`_cases`, `conftest`, `test_validate`, …) to the same top-level module name and refuse to collect the second one. Import within the tree relatively (`from ._cases import ...`, `from .._generation import ...`) for the same reason. + + A topic directory holds `schema.yaml`, instance YAML files, `_cases.py` with the `(target_class, instance)` case lists, a `conftest.py`, and one `test_*.py` per validator (`linkml-validate`, `gen-json-schema` + `check-jsonschema`, `gen-pydantic` + Pydantic). The artifact generation itself lives once in [`tests/linkml_behavior/_generation.py`](../../../tests/linkml_behavior/_generation.py); each `conftest.py` is just that topic's `SCHEMA` / `CLASSES` / `INSTANCES` plus thin session-scoped fixtures delegating to those helpers. + + **Keep those fixtures topic-local — do not hoist them into a parent `conftest.py`.** A fixture defined in a parent `conftest.py` has a single `FixtureDef` shared by every topic below it, so at session scope the artifacts generated for whichever topic ran first are silently handed to all the others, even though each topic supplies its own schema. The passing cases then fail confusingly and, worse, the failing cases keep passing while asserting nothing. Narrowing the scope to `module` avoids the bug but nothing enforces it, so a later "optimization" back to `session` reintroduces it. + +## Upstream tool we own + +[`dandi/pydantic2linkml`](https://github.com/dandi/pydantic2linkml) — the package doing most of the Pydantic→LinkML translation. Because **we own it**, systematic translation bugs should usually be fixed *there*, not patched downstream in this repo's overlays. See [Suggestions → "Push fixes upstream when possible"](#suggestions-open-leads). + +## Approach — repeatable procedure + +The procedure that's known to work. Follow in order; deviations belong in `log.md`. + +1. **Make changes on `linkml-conversion`** (or the relevant patch-queue branch). Commit. Tree must be clean before step 2. +2. **Run `./tools/linkml_conversion`** from the repo root. It checks out `linkml-auto-converted` and writes the regenerated artifacts there. +3. **Inspect the diff on `linkml-auto-converted`** — both `models.yaml` and the generated `models_linkml.py` / JSON Schema files. Anything unexpected is a finding. +4. **Run the test/contract suites.** These check different things — don't conflate them: + - **Full dandischema test suite:** `tox -e py3`. + - **Lint + types:** `tox -e lint,typing`. + - **LinkML-semantics contract tests** under `tests/linkml_behavior/` (`hatch run linkml-behavior-test:test`; type-check with `hatch run linkml-behavior-typing:check`) — these defend the *LinkML upstream behaviors* our schema relies on, not Pydantic↔LinkML parity. +5. **Run a parity check.** The migration's real success criterion (criterion 3) is that the LinkML-derived JSON Schema can drive `dandi-archive`'s frontend the same way the Pydantic-derived one does. Two complementary checks: + - **Cheap structural diff:** compare the two JSON Schema sets produced on `linkml-auto-converted`: `dandischema/models_linkml/*.json` (LinkML's `gen-json-schema`) and `dandischema/models_pydantic/*.json` (Pydantic's `model_json_schema`). **Both are generated from the same LinkML schema**, so this diff compares the two *serializers*, not LinkML against the hand-written Pydantic models. It is still worth running, since `models_pydantic/` is the shape `dandi-archive` consumes. Anything beyond expected, explained differences is a finding. + - **Parity baseline against the pre-migration models:** the published schemata at are the reference for what the original Pydantic models emit — diff against those rather than against anything the pipeline produces. (`tools/pubschemata.py` run *before* the rename dance would regenerate the equivalent locally.) + - **End-to-end behavioral check (preferred when feasible):** drive the dandi-archive UI through representative flows with **Playwright MCP** and compare LinkML-derived behavior to the Pydantic-derived baseline. Two variants: + - **Local stack:** launch the dandi-archive backend + frontend locally, point them at the LinkML-derived JSON Schema. Gives full control (you can swap schemas, set breakpoints, edit on the fly). Requires local clones of `dandi-archive`, `dandi-cli`, `dandischema`, and `pydantic2linkml`. + - **Live production instance:** compare against the deployed frontend at . Faster to reach for and known-good as a baseline, but you can only *observe* — you can't swap in the LinkML-derived schema there. Useful for snapshotting "what the Pydantic-derived schema actually drives the UI to do" and as a sanity reference; not a place to *test* the LinkML side. +6. **Diagnose any divergence** — between Pydantic-validated and LinkML-validated outcomes on the same instance, between the two JSON Schemas, or in the dandi-archive frontend behavior. Record in `log.md`; promote stable conclusions into `findings.md`. +7. **Decide where to fix:** + - **In `pydantic2linkml`** if the issue is a systematic translation gap (whole class of types/constraints mishandled). + - **In `dandischema/models_merge.yaml`** (consumed by `pydantic2linkml -M`) — deep merge: dicts merge recursively, lists append, file wins only on scalars and type mismatches. Use to override a scalar nested inside generated structure, or to *add* items to a list (e.g. extra `permissible_values`, extra slots). Cannot replace or reorder list items. + - **In `dandischema/models_overlay.yaml`** (consumed by `pydantic2linkml -O`) — shallow merge of top-level keys. Use to add/replace whole top-level elements (classes, enums, prefixes), or to outright replace a top-level list that `-M` would have appended to. + + See [`context/roles/linkml.md`](context/roles/linkml.md#-m-vs--o-semantics-verified-against-pydantic2linkmls-source--toolspy770809) for the full decision matrix. + - **In `dandischema/models.py`** if the Pydantic source itself is the right place (e.g. an under-specified field). + - Default preference: upstream first. +8. **Pin the fix with a test** at the layer that caught it — no fix lands without a test that would have caught it: + - A failure that came from a LinkML semantic our schema relies on → add a contract test under `tests/linkml_behavior/`. + - A failure caught by the structural JSON Schema diff → add the comparison (or a stable subset of it) as a checked-in fixture/test. + - A failure caught only end-to-end in dandi-archive → at minimum, record the reproduction in `log.md` and link it from `findings.md`; consider whether it can be reduced to a unit/contract test at one of the earlier layers. + +## Suggestions (open leads) + +Speculative — graduate into the procedure above once confirmed, or kill into `findings.md` with reasoning. + +- **Push fixes upstream when possible.** If a class of LinkML output is wrong for many Pydantic constructs, fix `pydantic2linkml` rather than carrying growing local overrides (`models_overlay.yaml` for corrections, `models_merge.yaml` for merges) here. +- **Use the `dandi-linkml-validation-report` skill** (already present on the LinkML branches under `.claude/skills/`) as a fitness signal — running the LinkML schema against real archive metadata exposes failure modes that synthetic tests miss. +- **Re-examine the patch queue** ([`context/patch-queue.md`](context/patch-queue.md)) when behavior drifts unexpectedly — order matters, and a stale branch in the list can silently revert hunks. Use that inventory to check whether any entry has met its exit criterion and can be retired. +- **Watch out for** Pydantic v2 features that LinkML can't express natively (custom validators, discriminated unions, `Annotated` metadata). These are the most likely sources of overlay accretion. + +## Out of scope (for now) + +- Renaming or restructuring classes purely for LinkML aesthetics — preserve current public class/field names to keep downstream consumers stable. +- Migrating `dandi-cli` / `dandi-archive` consumers off the generated models before parity is proven here. + +## Keeping this playbook current + +**The playbook is a living document, not a snapshot.** Whenever working on this migration — investigating, fixing, reading code, discovering tools, hitting a wall — keep this directory in sync with what is actually true. Stale instructions are worse than missing ones, because they get followed. + +The triggers below are **illustrative, not exhaustive** — they're common shapes the update need takes, not the full set. Anything that would make this playbook a more accurate or more useful guide for the next attempt qualifies, even if it doesn't fit any bullet here. When in doubt, write it down. + +- **New fact discovered** about how the translation, the schema, the consumers, or the upstream tooling behaves → add to `log.md`; if it's stable and trustworthy, promote into `findings.md` and adjust any affected procedure step in this file. +- **Existing claim contradicted** by what you see in the code, the artifacts, or a test run → correct the claim in place (don't just append a footnote elsewhere) and note the correction in `log.md` so the history of *why it changed* is preserved. +- **New tool, script, command, or technique** found useful (whether in this repo, in `pydantic2linkml`, in LinkML's CLI, or anywhere else) → mention it where it would actually be reached for (procedure step, suggestion list, or as its own helper under `tools/`). +- **Procedure step turns out to be wrong, incomplete, or in the wrong order** → edit the [Approach](#approach--repeatable-procedure) section directly. The procedure is the part future attempts execute most literally; outdated steps cost the most. +- **Patch-queue branch changes** (added, removed, retired, exit criterion met) → update [`context/patch-queue.md`](context/patch-queue.md) in the **same** change as the edit to `tools/linkml_conversion`. +- **An open question gets answered** → remove it from the [Open questions](#open-questions--unknowns) list and fold the answer into the relevant section (or into `findings.md`). +- **A suggestion is confirmed or killed** → move it out of [Suggestions](#suggestions-open-leads) into the procedure (if confirmed) or into `findings.md` with the reasoning that retired it (if killed). +- **The success criteria, scope, or wiring change** → update the top of this file. These shape every downstream decision. + +What this looks like in practice during a working session: when something is learned, the playbook edit is part of the same unit of work as the code change or the investigation, not a separate "cleanup" pass deferred to later (which never happens). A commit that lands a fix without touching the playbook, when the playbook had something wrong or missing about that area, is incomplete. + +For an AI assistant working in this directory: treat playbook updates as a default expectation of the task, not an optional extra. If a session surfaces a fact that would have saved time at the start, that fact belongs in `findings.md` (or wherever it fits) before the session ends. + +## How to use this directory + +- **`log.md`** — append-only, dated. Raw attempts, observations, dead ends, partial wins. Don't over-curate. +- **`findings.md`** — distilled, durable conclusions promoted out of `log.md`. The thing a future attempt reads first. +- **`tools/`** — scripts/probes accumulated across attempts (diff helpers, ad-hoc validators, comparators). Each script should have a one-line header comment naming its purpose. +- **`context/`** — background material that doesn't belong in the procedure: design notes, references, deeper explanations of constraints, links to related discussions. +- **`context/roles/`** — role profiles a working agent loads when handling a slice of the migration. [`senior-developer.md`](context/roles/senior-developer.md) is a **mandatory baseline** every agent (parent or subagent) inherits; topical roles ([`vue.md`](context/roles/vue.md), [`django.md`](context/roles/django.md), [`linkml.md`](context/roles/linkml.md)) stack on top of it. See [`context/roles/README.md`](context/roles/README.md) for how this preserves coupled reasoning while still permitting subagents when isolation is genuinely beneficial. + +When working in a fresh conversation, open this `OVERVIEW.md` first, then pull only the files the current step needs. + +## Open questions / unknowns + +- Which behavior gaps (if any) are *intentional* (deliberate cleanup during migration) vs. unintentional regressions from the Pydantic baseline. +- Whether `dandi-archive`'s frontend relies on JSON Schema *extensions* (`$comment`, custom keywords) that LinkML's `gen-json-schema` doesn't currently emit. (Partly answered for the *dialect* axis: LinkML emits draft 2019-09, Pydantic and the frontend's Ajv are on 2020-12, but the frontend deletes `$schema` so the gap is mostly cosmetic except for tuple arrays — see `findings.md`. The `$comment`/custom-keyword question is still open.) +- **Frontend impact of removing discriminated unions** — the active blocker on retiring the `remove-discriminated-unions` patch-queue branch (see [`context/patch-queue.md`](context/patch-queue.md)). diff --git a/docs/designs/migration_to_linkml_playbook/context/README.md b/docs/designs/migration_to_linkml_playbook/context/README.md new file mode 100644 index 00000000..32e816fd --- /dev/null +++ b/docs/designs/migration_to_linkml_playbook/context/README.md @@ -0,0 +1,5 @@ +# context/ + +Background material that doesn't belong in the procedure: design notes, references, deeper explanations of constraints, links to related discussions (issues, PRs, upstream tickets in `pydantic2linkml` / `linkml` / `dandi-cli` / `dandi-archive`). + +Keep one topic per file; name files in kebab-case (e.g. `discriminated-unions.md`, `jsonschema-frontend-requirements.md`). diff --git a/docs/designs/migration_to_linkml_playbook/context/patch-queue.md b/docs/designs/migration_to_linkml_playbook/context/patch-queue.md new file mode 100644 index 00000000..0719d624 --- /dev/null +++ b/docs/designs/migration_to_linkml_playbook/context/patch-queue.md @@ -0,0 +1,45 @@ +# Patch queue (`tools/linkml_conversion`) + +Inventory of the branches listed in the `branches_to_merge` array of [`tools/linkml_conversion`](../../../../tools/linkml_conversion). These branches are materially **part of the translation's input**: the script applies them, in order, on top of `linkml-conversion` before writing the regenerated artifacts to `linkml-auto-converted`. Order matters (per the script's comment). + +A patch-queue entry is meant to be **temporary**. Each branch should be retired — by landing it on `master` or by removing its need (e.g. fixing the underlying issue upstream in `pydantic2linkml`) — once its exit criterion is met. This file is what lets us audit "can we drop any of these yet?" instead of treating the script's list as tribal knowledge. + +## Entry template + +``` +### `` + +- **Purpose:** what this branch changes and why it's applied during translation. +- **Rationale:** why the change isn't (or can't yet be) on `master` or fixed upstream. +- **Exit criterion:** the concrete condition under which this branch can be dropped from the queue. +- **Status:** active / blocked-on-X / ready-to-retire. +- **Ordering note:** why it sits where it sits in the list (if non-obvious). +``` + +--- + +## Current queue (in apply order) + +### `master` + +- **Purpose:** keep `linkml-conversion` up to date with `master` during each translation run, so the regenerated artifacts reflect the latest upstream Pydantic sources. +- **Rationale:** `linkml-conversion` is a long-lived branch that diverges from `master`; folding `master` in at translation time avoids carrying a manual rebase burden on `linkml-conversion` itself. +- **Exit criterion:** when `linkml-conversion` is itself merged to `master` (i.e. the migration's source-of-truth flip happens), this entry becomes unnecessary. +- **Status:** active. Expected to remain in the queue until the migration completes. +- **Ordering note:** applied first so subsequent patch branches stack on a current base. + +### `remove-discriminated-unions` + +- **Purpose:** remove the use of Pydantic discriminated unions from `dandischema/models.py` so the Pydantic sources fed to the translator don't contain a construct LinkML can't represent. The branch is a single commit on top of `master`, +6/−21 in `dandischema/models.py` only. +- **Rationale:** LinkML has no faithful equivalent to Pydantic v2 discriminated unions. Removing them on a separate branch lets the translation succeed without committing the removal to `master`, because the downstream impact on the **dandi-archive frontend** of dropping discriminated unions is not yet characterized. +- **Exit criterion:** **either** (a) the impact on dandi-archive is assessed and acceptable, at which point this branch lands on `master` and is dropped from the queue; **or** (b) `pydantic2linkml` gains a way to translate discriminated unions into a LinkML construct with equivalent validation behavior, at which point the removal is no longer needed and this branch is dropped without merging. +- **Status:** active, blocked on assessing the frontend impact (and/or on an upstream `pydantic2linkml` improvement). Track in `log.md` as that investigation progresses. +- **Ordering note:** applied after `master` so it patches the current Pydantic sources rather than a stale snapshot. + +--- + +## When changing the queue + +- Edit `tools/linkml_conversion`'s `branches_to_merge` array, and **update this file in the same change** — the script comment authorizes editing the list, but an undocumented edit makes the queue opaque again. +- A new entry without a written **exit criterion** is a smell: if there's no condition under which the branch can be retired, it's effectively a permanent patch and probably belongs on `master` (or upstream) — not in the queue. +- Removing an entry: note in `findings.md` what was learned that allowed retirement, so the rationale isn't lost. diff --git a/docs/designs/migration_to_linkml_playbook/context/roles/README.md b/docs/designs/migration_to_linkml_playbook/context/roles/README.md new file mode 100644 index 00000000..95d78559 --- /dev/null +++ b/docs/designs/migration_to_linkml_playbook/context/roles/README.md @@ -0,0 +1,42 @@ +# Roles + +A "role" is a profile of mindset, expertise, and operating habits that a working agent loads when handling a particular slice of this migration. Roles let an agent acquire the specialization a task needs by loading the relevant role files into its context. The same role files are also usable as spawn-prompt material when a subagent is invoked. + +## How roles are used + +- **Default mode — one agent, multiple roles loaded.** Migration work is deeply coupled: a frontend rendering question can implicate the JSON Schema, the LinkML schema, and the Pydantic source all at once (see [Success criteria](../../OVERVIEW.md#success-criteria), especially criterion 3). When a session touches more than one slice, the working agent loads the relevant role files into its context and stacks them — one mind, multiple specializations. +- **Subagent mode — when isolation is the actual benefit.** Subagents *may* be invoked for sub-tasks where isolation pays off: a noisy cross-repo search via `Explore`, an unbiased review of a migration PR, a verifiably-independent piece that can run in parallel for wall-clock speedup. When a subagent is invoked, **load the role(s) appropriate to its task into the spawn prompt** so it inherits the right specialization — subagents do *not* share the parent's loaded skills. + +These two modes are not in conflict. The default mode handles coupled work (most of the migration); the subagent mode is a release valve for the cases listed in the analysis above. + +## The senior-developer baseline + +[`senior-developer.md`](senior-developer.md) is a **mandatory baseline** for every agent acting in this project — the parent agent *and* every subagent it spawns. It describes operating habits (meticulousness, verifying uncertainty, reading local docs first, etc.) rather than topical expertise. Other role files build on top of it; they do not replace it. + +**Rule:** when spawning a subagent, its spawn prompt must include the contents of `senior-developer.md` (inlined, or fetched at the start of its task) so the subagent operates under the same baseline as the parent. A subagent that has not loaded the baseline is operating off-spec. + +## Roles inventory + +- [`senior-developer.md`](senior-developer.md) — mandatory baseline (operating habits, not topical expertise). Inherited by every agent. +- [`vue.md`](vue.md) — Vue / dandi-archive frontend. +- [`django.md`](django.md) — Django / dandi-archive backend. +- [`linkml.md`](linkml.md) — LinkML schema authoring + the Pydantic↔LinkML translation pipeline. + +## Ecosystem references + +The Claude Code skill/subagent ecosystem is large and uneven; treat external definitions as **starting material to lift selectively**, not gospel. As of authoring: + +- [`anthropics/skills`](https://github.com/anthropics/skills) — Anthropic's official `SKILL.md` examples; the canonical reference for the format and frontmatter. +- [`wshobson/agents`](https://github.com/wshobson/agents) — large production-leaning marketplace (~190 agents / ~155 skills) with multi-harness packaging. +- [`VoltAgent/awesome-claude-code-subagents`](https://github.com/VoltAgent/awesome-claude-code-subagents) — community catalog of ~100 specialized subagents, indexed by category and language. +- [`rohitg00/awesome-claude-code-toolkit`](https://github.com/rohitg00/awesome-claude-code-toolkit) — broader toolkit including agents, skills, commands, hooks. +- [`travisvn/awesome-claude-skills`](https://github.com/travisvn/awesome-claude-skills) — curated awesome-list. + +Quality varies across these. When borrowing, prefer lifting *specific habits or descriptions* over wholesale adoption — and credit the source in the role file's References section. + +## Adding a new role + +- One topical area per file. If you can't summarize the scope in one sentence, it's probably two roles. +- State explicitly what the role is **not** responsible for, so stacking multiple roles doesn't double-cover. +- Keep the expertise inventory tight — every loaded role takes context. Don't add a role just to populate the matrix. +- Role files are subject to the [self-updating rule](../../OVERVIEW.md#keeping-this-playbook-current): when something is learned about how this slice of the system actually behaves, update the role file in the same unit of work. diff --git a/docs/designs/migration_to_linkml_playbook/context/roles/django.md b/docs/designs/migration_to_linkml_playbook/context/roles/django.md new file mode 100644 index 00000000..5e9ee99b --- /dev/null +++ b/docs/designs/migration_to_linkml_playbook/context/roles/django.md @@ -0,0 +1,97 @@ +# Django (dandi-archive backend role) + +Topical role: the Django backend of [`dandi/dandi-archive`](https://github.com/dandi/dandi-archive). + +Stacks on top of [`senior-developer.md`](senior-developer.md) — load both together. + +## Scope + +- Django models, views, serializers, and migrations that depend on `dandischema`'s generated Pydantic models. +- Server-side validation paths that consume `dandischema` (whether via Pydantic models or JSON Schemas). +- Verifying that the generated Pydantic from the LinkML schema is a drop-in replacement for the hand-written `dandischema.models` in this consumer (success criterion 2 — see [OVERVIEW](../../OVERVIEW.md#success-criteria)). + +## Not in scope + +- Vue frontend → see [`vue.md`](vue.md). +- LinkML schema authoring → see [`linkml.md`](linkml.md). +- `pydantic2linkml` internals → see [`linkml.md`](linkml.md). +- [`dandi/dandi-cli`](https://github.com/dandi/dandi-cli) (the other major Python consumer of generated Pydantic) — its own scope; cover it with the senior-developer baseline or split out a `dandi-cli.md` role file later if it accrues distinct concerns. + +## What this role needs to know + +### Stack landscape (verified against `dandi-archive/pyproject.toml`) + +- Package: **`dandiapi`** (top-level), Python **>= 3.13**. +- **Django 5.2.x** (NOT 4.x — most external Django subagent profiles target 4+; verify before lifting patterns). +- **Django REST Framework 3.17.x** + `drf-extensions` + `drf-yasg` (OpenAPI/Swagger docs). +- Auth / permissions: `django-allauth`, `django-oauth-toolkit`, `django-guardian` (object-level perms). +- Filtering / extensions: `django-filter`, `django-extensions`, `django-cors-headers`, `django-environ`. +- **Resonant stack:** `django-resonant-settings`, `django-resonant-utils` — opinionated settings/utility layer; understand it before adding settings or storage code. +- **Celery** with multiple queues (`celery`, `calculate_sha256`, `ingest_zarr_archive`, `manifest-worker`); the backend depends on background workers for non-trivial flows. +- **PostgreSQL** (port 5432 in dev). +- Type-stub support via `django-stubs-ext`. + +### dandischema pinning observation (critical for the migration) + +- `dandi-archive` pins **`dandischema==0.12.1`** (schema version 0.7.0), exact pin — the comment in `pyproject.toml` says: *"Pin dandischema to exact version to make explicit which schema version is being used."* +- The sibling consumer `dandi-cli` pins `dandischema ~= 0.12.0` (compatible-release). +- **Implication for the migration:** bumping `dandischema` to a LinkML-derived release requires a coordinated bump in both consumers, and the generated Pydantic must remain importable and behaviorally equivalent across both pin styles (exact and compatible-release). A subtle API or runtime-validation difference can be invisible until one of these two repos breaks. + +### Where dandischema crosses into the backend + +Files importing `dandischema` (as of inspection): + +- `dandiapi/conftest.py`, `dandiapi/api/tests/factories.py`, `dandiapi/api/tests/fuzzy.py` → test infrastructure (factory generation, fuzzy comparators). +- `dandiapi/api/doi.py` → DOI metadata serialization. +- `dandiapi/api/multipart.py` → upload-related metadata. +- `dandiapi/api/tests/test_*.py` → many tests assert on `dandischema` behavior. +- `dandiapi/zarr/tests/test_ingest_zarr_archive.py` → zarr ingest path. + +These are the files most likely to fail loudly if the generated Pydantic diverges from the hand-written one. The test suite is the cheapest first signal for criterion 2 (drop-in replacement). + +### Local dev story + +From [`DEVELOPMENT.md`](https://github.com/dandi/dandi-archive/blob/master/DEVELOPMENT.md): + +- **VSCode Dev Containers** is the recommended quickstart (`Dev Containers: Reopen in Container`). +- **Docker Compose** is the alternative (`docker compose up`). +- Backend dev loop (inside container/host): + - `./manage.py migrate`, `createcachetable`, `createsuperuser --email …`, `create_dev_dandiset --owner …`. + - Three terminals: `./manage.py runserver_plus 0.0.0.0:8000`, the celery worker, and `cd web && npm run dev`. +- To exercise the parity check: swap the generated `models_linkml.py` Pydantic into where `dandischema.models` is currently imported (or install `dandischema` from a LinkML-converted branch into the dev env) and run the test suite + the local backend against the frontend. + +### Operating notes + +- The repo uses **`uv`** for dependency resolution (`uv.lock` checked in). Prefer `uv` commands over plain `pip` when touching deps. +- Resonant settings layer is non-obvious; before adding a setting, check `django-resonant-settings` to see if there's already a knob. +- Tests live alongside their app (`dandiapi/api/tests/`, `dandiapi/zarr/tests/`); idiom is `pytest-django` style. +- DRF schemas are exposed via `drf-yasg` — keep `swagger.py` in sync if URL routes change. + +### Watch-outs (LinkML-side semantic differences that can surface here) + +- The Pydantic generated by `gen-pydantic` (called from the `2pydantic` Hatch script) may differ in subtle ways from hand-written Pydantic: optional-field semantics, validator placement, `model_config` knobs, alias handling, JSON-mode serialization. Backend factories and serializers are the most likely to expose these. +- Discriminated unions (the `remove-discriminated-unions` patch-queue branch) — if the backend currently relies on `discriminator=` resolution at validation time, removing it changes the parse path. Verify with `dandiapi/api/tests/test_schema.py` and the fuzzy-comparison utilities. + +### Generic Django expertise (lifted selectively from upstream community references) + +Useful background, not dandi-archive-specific. Source: [`VoltAgent/django-developer.md`](https://github.com/VoltAgent/awesome-claude-code-subagents/blob/main/categories/02-language-specialists/django-developer.md). **Caveat:** the VoltAgent profile is written against Django 4+; this repo is Django 5.2. Don't lift patterns about `async` views, signals, or settings management without re-verifying for Django 5. + +- ORM hygiene: `select_related` / `prefetch_related` for N+1 prevention, explicit index design, careful migrations. +- DRF idioms: ViewSets, serializers, permission/throttle classes, pagination, API versioning, OpenAPI doc consistency. +- Security: CSRF/XSS, secure cookies, headers, rate limiting — but check this repo's `settings/` first; resonant-settings likely already configures most of these. +- Async views and ASGI deployment — re-verify behavior in Django 5.2 before adopting. + +When borrowing patterns from external Django subagent definitions, verify against `dandi-archive`'s actual conventions (Django 5, resonant stack, celery topology) before applying. + +## References + +External skill/agent definitions to **lift content from** (review for fit before adopting wholesale — these are community collections of varying quality): + +- [`anthropics/skills`](https://github.com/anthropics/skills) — canonical reference for the `SKILL.md` format and frontmatter conventions. +- [`VoltAgent/awesome-claude-code-subagents` → `django-developer.md`](https://github.com/VoltAgent/awesome-claude-code-subagents/blob/main/categories/02-language-specialists/django-developer.md) — Django 4+ specialist covering REST APIs, async views, ORM optimization, admin patterns. Closest off-the-shelf match. +- [`ammohq/agents`](https://github.com/ammohq/agents) — described as a "Supreme Django + DRF + ORM + Pillow expert"; useful if dandi-archive uses DRF heavily (verify before relying). +- [`wshobson/agents`](https://github.com/wshobson/agents) — broader marketplace; check for backend / Django entries. + +`dandi-archive`-specific references: + +- TODO: link the repo's backend `README.md`, `CONTRIBUTING.md`, or `DEVELOPMENT.md` once a session actually exercises the backend locally. diff --git a/docs/designs/migration_to_linkml_playbook/context/roles/linkml.md b/docs/designs/migration_to_linkml_playbook/context/roles/linkml.md new file mode 100644 index 00000000..297ca1f9 --- /dev/null +++ b/docs/designs/migration_to_linkml_playbook/context/roles/linkml.md @@ -0,0 +1,123 @@ +# LinkML (schema authoring + translation pipeline role) + +Topical role: the LinkML schema itself, and the Pydantic↔LinkML translation pipeline. Covers the core of the migration's source-of-truth flip. + +Stacks on top of [`senior-developer.md`](senior-developer.md) — load both together. + +## Scope + +- Authoring and reviewing `dandischema/models.yaml` and its inputs (`dandischema/models_overlay.yaml`, `dandischema/models_merge.yaml`). +- The [`pydantic2linkml`](https://github.com/dandi/pydantic2linkml) translator. Owned by the DANDI team; systematic translation gaps are typically fixed here. +- LinkML generators in the pipeline: `gen-pydantic` (via the `2pydantic` Hatch script) and `gen-json-schema` (via `2json`). +- The Pydantic-template customization point at `tools/linkml_conversion_tools/pydantic_templates/`. +- The contract tests under `tests/linkml_behavior/` — pinning specific LinkML upstream behaviors the generated schema relies on (see [issue #405](https://github.com/dandi/dandi-schema/issues/405) for an example). +- Choosing where a fix lands (translator vs. overlay vs. merge vs. Pydantic source) — see [procedure step 7](../../OVERVIEW.md#approach--repeatable-procedure). + +## Not in scope + +- Vue frontend behavior driven by the generated JSON Schema → see [`vue.md`](vue.md). +- Django backend behavior driven by the generated Pydantic → see [`django.md`](django.md). + +## What this role needs to know + +### `-M` vs `-O` semantics (verified against `pydantic2linkml`'s source — `tools.py:770–809`) + +This was an open question in OVERVIEW. The README's one-liner ("values from the file win on conflict") oversimplifies; the implementation is more nuanced: + +- **`-M` / `--merge-file`** = **deep merge** via [`deepmerge.always_merger`](https://deepmerge.readthedocs.io/). Per-type strategy: + - **dict** → **recursive deep merge** (descend; the file does *not* override the whole dict). + - **list** → **append** the file's list to the existing one (the file does *not* override the whole list; items accumulate). + - **set** → **union**. + - **type mismatch** (file's value has a different type than the existing) → **file wins**. + - **scalar** (int, str, bool, None) → **file wins**. + - Result is validated against the LinkML meta schema. +- **`-O` / `--overlay-file`** = **shallow merge** of the YAML file into the generated schema (top-level keys only). Result is validated against the LinkML meta schema. + +In this repo: `dandischema/models_merge.yaml` is the `-M` input, `dandischema/models_overlay.yaml` is the `-O` input. + +Picking between them: + +- **Use `-M` (deep merge)** when overriding a *scalar* deep inside the generated structure (e.g. flipping a `required: false` to `true` on a specific slot) or adding items to a list (e.g. extra `permissible_values` on an enum, extra slots on a class). Items accumulate; nested dicts merge. +- **Use `-O` (shallow merge)** when patching at the top of the document — adding/replacing whole top-level keys like extra classes, slots, enums, or prefixes — and you do *not* want the merge to descend. + +Honest gotcha: because `-M` **appends** lists rather than replacing them, you cannot remove or reorder items via `-M` alone. If you need to *replace* a list outright, either switch to `-O` (if the list is top-level) or change the source on the Pydantic side. When in doubt, run the conversion with a minimal patch and diff the resulting `models.yaml` — cheaper than guessing. + +### How `pydantic2linkml` actually works + +From `pydantic2linkml/CLAUDE.md`: it translates Pydantic v2 models by **introspecting Pydantic's internal `core_schema` objects**, not the higher-level model API. Two consequences: + +- The translation is sensitive to **how Pydantic v2 builds its `core_schema`** for a given construct. Changes to `dandischema.models` that look semantically identical may yield different `core_schema` shapes and therefore different LinkML output. +- The translation can be sensitive to **the Pydantic version itself**. `pydantic2linkml`'s test matrix runs Python 3.10–3.13 and includes `dandischema` and `aind-data-schema` as known consumers. Bumping Pydantic upstream may require a matching `pydantic2linkml` adjustment before the dandischema migration sees green again. + +### `pydantic2linkml` conventions (lifted from its `CLAUDE.md`) + +When upstreaming a fix: + +- **Hatch is the env manager.** Test invocations: `hatch run test.py3.10:pytest tests/`, type-check with `hatch run types:check`, lint/format with `ruff check . && ruff format .`, spell-check with `codespell`. +- **Document-currency rule** (mirrors this playbook's self-updating rule): "Whenever you notice that any documentation — `CLAUDE.md`, `README.md`, or any other docs — is outdated or incorrect, update it immediately." +- **Prose wraps at ~79 characters** in docs (code blocks and long URLs exempt). Match this when editing markdown there. + +### LinkML upstream conventions (lifted from `linkml/linkml/AGENTS.md`) + +When working in or proposing changes to `linkml/linkml`: + +- **`uv run`** prefix on every command (UV-workspace monorepo publishing both `linkml` and `linkml-runtime`). +- **Pytest functional style, never `unittest`-OO style.** Modern idioms: `@pytest.mark.parametrize` for combinations. +- **Doctests are first-class** — both explanatory examples and unit tests. For longer cases, write pytest tests. +- **Never mock** unless explicitly requested. *"I need to rely on tests to know if something breaks."* +- **Never weaken a failing test** to make it pass — try harder or ask. +- **Avoid `try/except` that masks bugs.** Fail fast. +- **Always use type hints; always document methods and classes.** +- For tests with external dependencies, use the `integration` pytest mark. + +### Pydantic-v2 features with no faithful LinkML equivalent (current state) + +These are the recurring trouble spots: + +- **Discriminated unions** — no LinkML equivalent; currently worked around by the `remove-discriminated-unions` patch-queue branch (see [`patch-queue.md`](../patch-queue.md)). Exit criterion is either an upstream `pydantic2linkml` improvement or an acceptable assessment of the frontend impact (see [`vue.md`](vue.md)). +- **Custom `@field_validator` / `@model_validator` decorators** — runtime-only behavior; LinkML's static schema can't carry the validator code itself. Watch for accuracy gaps where Pydantic accepts/rejects something the JSON Schema doesn't. +- **`Annotated[..., FieldInfo(...)]` and rich `Field()` metadata** — depending on which metadata is set, the translation may or may not faithfully round-trip. Verify by re-generating Pydantic from the LinkML and diffing against the source. +- **`model_config` knobs** (e.g. `populate_by_name`, JSON-serialization aliases) — easy to forget that these change Pydantic behavior in ways LinkML can't represent declaratively. + +### The two downstream consumers' Pydantic pinning + +- `dandi-archive` pins `dandischema==0.12.1` (exact; schema version 0.7.0). +- `dandi-cli` pins `dandischema ~= 0.12.0` (compatible-release). +- The generated Pydantic must remain importable and behaviorally equivalent for both. See [`django.md`](django.md) for the backend's import sites and [the migration's success criteria](../../OVERVIEW.md#success-criteria). + +### Decision matrix for "where to fix" (refines [procedure step 7](../../OVERVIEW.md#approach--repeatable-procedure)) + +| Symptom | First place to look | Likely fix layer | +|---|---|---| +| Whole class of Pydantic constructs translates wrong | `pydantic2linkml`'s `core_schema` introspection | **Upstream `pydantic2linkml`** | +| One generated class/slot has a specific wrong value | Compare `models.yaml` against `models.py` for that class | **`models_merge.yaml`** (`-M`, file-wins deep merge) | +| Need to add a top-level element (class, enum, prefix) not in the source | Inspect generated `models.yaml` structure | **`models_overlay.yaml`** (`-O`, shallow merge) | +| The Pydantic source itself is under-specified or wrong | `dandischema/models.py` | **`dandischema/models.py`** on `linkml-conversion` | +| Generated Pydantic differs subtly from intent | `tools/linkml_conversion_tools/pydantic_templates/` | **Customize the Jinja templates** consumed by `gen-pydantic` in `2pydantic` | + +Default preference, in order: upstream `pydantic2linkml` > source Pydantic > merge/overlay > template customization. Reach for the lower-leverage tool only when the higher-leverage one can't cleanly express the change. + +## References + +**First-party LinkML AI guidance (read these first):** + +- [`linkml/linkml/AGENTS.md`](https://github.com/linkml/linkml/blob/main/AGENTS.md) (symlinked from `CLAUDE.md`) — maintainer-authored "Claude Code Notes for LinkML." Covers the UV-workspace monorepo layout (`linkml` and `linkml-runtime` published from one repo), the mandatory `uv run` prefix, and Best Practices that are directly applicable here: prefer doctests + pytest functional style, never mock tests, never weaken failing tests, avoid try/except masking bugs, fail fast, always use type hints. When working inside `linkml/linkml` (or proposing changes upstream), this file's rules supersede generic instincts. +- [`linkml/linkml/.claude/skills/codecov-coverage/SKILL.md`](https://github.com/linkml/linkml/blob/main/.claude/skills/codecov-coverage/SKILL.md) — a real first-party LinkML `SKILL.md`. Useful as a structural example (frontmatter, `allowed-tools`, "When to Use" section, coverage-decrease rule) when this role file is promoted to a skill. The skill itself is about Codecov, not topical LinkML — but the *form* is exemplary. + +Primary sources: + +- [LinkML specification](https://w3id.org/linkml/specification) — the normative spec; the source of truth for what LinkML means, ahead of any tutorial-style docs. +- [LinkML official docs](https://linkml.io/linkml/) — schema syntax, generators (`gen-pydantic`, `gen-json-schema`), runtime. +- [`linkml/linkml`](https://github.com/linkml/linkml) on GitHub. +- [`linkml/linkml-runtime`](https://github.com/linkml/linkml-runtime). +- [`dandi/pydantic2linkml`](https://github.com/dandi/pydantic2linkml) — the translator we own. README, open issues, and source are the authoritative description of `-M`/`-O` semantics and current translation gaps. + +In-tree reference: + +- [`.claude/skills/dandi-linkml-validation-report/SKILL.md`](https://github.com/dandi/dandi-schema/blob/linkml-auto-converted/.claude/skills/dandi-linkml-validation-report/SKILL.md) on the `linkml-auto-converted` branch — a working `SKILL.md` already tied to this migration. Useful as a local example of how a LinkML-flavored skill is shaped. + +Format reference: + +- [`anthropics/skills`](https://github.com/anthropics/skills) — canonical `SKILL.md` format, if/when this role file is promoted to an actual skill. + +**Not yet checked:** other repos in the [`linkml` GitHub org](https://github.com/linkml) (`schema-automator`, `linkml-model`, `linkml-validator`, `linkml-store`, `linkml-project-cookiecutter`, etc.) may also carry AGENTS.md / `.claude/skills/` content. Worth spot-checking when scope expands beyond the two clones we have locally. diff --git a/docs/designs/migration_to_linkml_playbook/context/roles/senior-developer.md b/docs/designs/migration_to_linkml_playbook/context/roles/senior-developer.md new file mode 100644 index 00000000..9b1b5442 --- /dev/null +++ b/docs/designs/migration_to_linkml_playbook/context/roles/senior-developer.md @@ -0,0 +1,24 @@ +# Senior developer (mandatory baseline) + +The baseline every agent acting in this project inherits — the parent agent and every subagent. Other role files (`vue.md`, `django.md`, `linkml.md`, …) stack topical expertise *on top of* this; they do not replace it. If a role file's guidance ever appears to conflict with this baseline, the baseline wins. + +## Operating habits + +- **Meticulous.** Land changes that are correct, complete, and don't leave silent loose ends. Read what's actually there before changing it; verify edits did what you intended; don't declare done until the last open thread is closed or explicitly deferred. +- **Don't assume — verify.** When something is uncertain, prefer a quick test, a `grep`, a script run, a `git log` / `git show`, or reading the actual source over speculation. Confidence is not verification; saying "I think" or "probably" is a cue to go check. +- **Read the local map first.** Before working in any repo, look for `CLAUDE.md`, `AGENTS.md`, `README.md`, `CONTRIBUTING.md`, `DEVELOPMENT.md`, `docs/`, the repo's `pyproject.toml` / `package.json` / `Cargo.toml`, the test configuration, and any visible CI workflows. These are the fastest path to a repo's idioms, build/test commands, and gotchas. For this project specifically: `dandi-schema/CLAUDE.md` plus this playbook (`docs/designs/migration_to_linkml_playbook/OVERVIEW.md`). +- **Surface uncertainty honestly.** Distinguish "I verified X" from "I assumed X" in your output. Don't paper over gaps with confident phrasing. If a step rests on an unchecked assumption, name the assumption. +- **Push back honestly.** If a request or suggestion looks ill-advised, say so up front, with the concrete tradeoff named, *before* executing. Don't soften pushback because the request came from a user, a boss, or another agent. Don't reverse position just because someone disagreed — only if the new argument actually outweighs the original one. +- **Honor the repo's own conventions.** Style, commit-message style, branch naming, PR template, lint/format setup, language-version floor — match what the repo already does rather than imposing personal preferences. American English in code, comments, commits, and prose unless the repo says otherwise. +- **Reversibility-aware.** Local edits, branch creation, and tests are cheap. Pushes (especially force-pushes), comments on issues/PRs, sent messages, schema migrations, destructive git operations (`reset --hard`, `clean -f`, branch deletion), and anything that touches shared infrastructure are not. Confirm with the user before taking the second kind, unless durably pre-authorized. +- **Trace before you cut.** When investigating an obstacle (failing test, weird behavior, unfamiliar file), find the root cause before reaching for a workaround. Don't bypass safety checks (`--no-verify`, `--force`, deleting lock files) as a way to make a symptom go away. +- **Keep the playbook current.** This project's playbook is self-updating (see [Keeping this playbook current](../../OVERVIEW.md#keeping-this-playbook-current)). If you uncover a fact, contradict an existing claim, find a better tool, or answer an open question, update the relevant playbook file in the same unit of work — not as deferred cleanup. + +## When acting as (or spawning) a subagent + +- **As a subagent:** the spawn prompt should include this baseline (or an explicit pointer to it). If neither was provided, request it before proceeding on anything non-trivial. +- **As a parent spawning a subagent:** include this baseline in the spawn prompt. Subagents do not inherit the parent's loaded role files automatically; they only know what the prompt tells them. + +## Adding to this file + +This file is the canonical home for cross-cutting agent behaviors that apply to *every* role. New items should be **behaviors** (how to operate) rather than **knowledge** (what to know) — the latter belongs in topical role files. Keep entries short and concrete; aim for one sentence per habit with at most one sentence of clarification. diff --git a/docs/designs/migration_to_linkml_playbook/context/roles/vue.md b/docs/designs/migration_to_linkml_playbook/context/roles/vue.md new file mode 100644 index 00000000..7ca4bd8c --- /dev/null +++ b/docs/designs/migration_to_linkml_playbook/context/roles/vue.md @@ -0,0 +1,82 @@ +# Vue (dandi-archive frontend role) + +Topical role: Vue and the JSON-Schema-driven UI in [`dandi/dandi-archive`](https://github.com/dandi/dandi-archive). + +Stacks on top of [`senior-developer.md`](senior-developer.md) — load both together. + +## Scope + +- The Vue components and form-generation machinery that consume `dandischema`'s generated JSON Schemas. +- Verifying that the LinkML-derived JSON Schema drives the frontend the same way the Pydantic-derived one does (success criterion 3 — see [OVERVIEW](../../OVERVIEW.md#success-criteria)). +- Driving the UI via Playwright MCP for the end-to-end parity check in [procedure step 5](../../OVERVIEW.md#approach--repeatable-procedure). + +## Not in scope + +- Django backend → see [`django.md`](django.md). +- LinkML schema authoring → see [`linkml.md`](linkml.md). +- `pydantic2linkml` internals → see [`linkml.md`](linkml.md). + +## What this role needs to know + +### Stack landscape (verified against `dandi-archive/web/package.json`) + +- **Vue 3.5.x** with the Composition API. No Nuxt — the app is a plain Vue SPA built with Vite. +- **UI:** Vuetify 3 (Material Design); `eslint-plugin-vuetify` enforces a few Vuetify-specific rules. +- **State:** Pinia 3. +- **Router:** `vue-router` 4, with `unplugin-vue-router` for file-based route inference. +- **Build:** Vite with `vite-plugin-node-polyfills`; TypeScript via `vue-tsc`. `tsconfig.json` extends `@tsconfig/node24`. +- **Linting:** ESLint flat config (`eslint.config.js`), `eslint-plugin-vue`, `@vue/eslint-config-typescript`. +- **Error tracking:** `@sentry/vue` is wired in — be aware that errors surface to Sentry in non-dev envs. +- **Misc:** `lodash`, `moment` (legacy date handling), `marked` + `dompurify` (rendered Markdown), `axios` for HTTP. + +### The JSON-Schema → UI seam (this is criterion 3's heart) + +This is the part of the frontend the migration must not break: + +- **`@koumoul/vjsf`** (Vue JSON-Schema Form) is the form generator that consumes the JSON Schema and renders the metadata editor. Anything the LinkML-derived JSON Schema fails to support that the Pydantic-derived one supports surfaces here first. +- **`@apidevtools/json-schema-ref-parser`** resolves `$ref`s in the schema before `vjsf` sees the result. Differences in how `gen-json-schema` emits refs (e.g. `$ref` style, `$defs` location) can interact with this stage rather than with `vjsf` itself. +- **TypeScript typings from JSON Schema:** the repo's `web/src/types/schema.ts` is generated by `json-schema-to-typescript` (devDep), invoked via `npm run migrate ` (see [`web/README.md`'s Schema Migration section](https://github.com/dandi/dandi-archive/blob/master/web/README.md)). The typings are *lint-only*; they don't drive runtime behavior. Still useful as a parity probe — diffing the regenerated `schema.ts` between Pydantic-derived and LinkML-derived JSON Schemas is a quick structural signal. +- **The form-driven UI lives in `web/src/components/Meditor/`** — the metadata editor. That's the directory to load in the browser when doing the [end-to-end parity check](../../OVERVIEW.md#approach--repeatable-procedure). + +### Local dev story + +- Frontend dev server: `cd web && npm install && npm run dev` → http://localhost:8085/. +- Backend at http://localhost:8000/ (see [`django.md`](django.md) for how to launch it). The frontend assumes the backend is reachable; without it the UI loads but most flows fail. +- To exercise the parity check: launch the local stack, regenerate the JSON Schema from the LinkML side, then either (a) point the frontend at the LinkML-derived JSON via the backend's schema-serving endpoint (preferred), or (b) drop the LinkML-derived JSON into where `dandischema` would deliver it for that schema version. Drive the Meditor with Playwright MCP and compare to the Pydantic-derived baseline. + +### Operating notes + +- Composition API and `