From e072219342861c31cb179ad1037c3997ebc8fd00 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 17:05:42 +0000 Subject: [PATCH 1/4] Add tools/audit_nameless_items.py to audit published manifests for nameless items Scans public S3 manifests (dandiset.jsonld and optionally assets.jsonld) of a DANDI instance for nested objects whose class has an optional `name` (Contributor/Organization, BaseType subclasses, Resource, RelatedParticipant) but lack one, and cross-checks with current validation to identify versions which would turn from valid to invalid if `name` became required (see #442). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01NdN9xPzPo1AzsnEMuU8UQs --- tools/audit_nameless_items.py | 378 ++++++++++++++++++++++++++++++++++ 1 file changed, 378 insertions(+) create mode 100755 tools/audit_nameless_items.py diff --git a/tools/audit_nameless_items.py b/tools/audit_nameless_items.py new file mode 100755 index 00000000..9d8dc11b --- /dev/null +++ b/tools/audit_nameless_items.py @@ -0,0 +1,378 @@ +#!/usr/bin/env python3 +"""Audit published DANDI manifests for nested items lacking a ``name``. + +Motivation: https://github.com/dandi/dandi-schema/issues/442 proposes making +``name`` required on ``Contributor`` (hence ``Organization``), ``BaseType`` +subclasses (``Anatomy``, ``SpeciesType``, ...) and possibly other classes whose +``name`` is currently ``Optional``. Before doing so we need to know which +already *published* (immutable) versions would turn from valid to invalid. + +The script walks the public S3 manifests of a DANDI instance +(``dandisets///dandiset.jsonld`` and, optionally, +``assets.jsonld``) without authentication, and for every nested object whose +``schemaKey`` belongs to a class with an optional ``name`` it reports: + +* ``missing`` -- ``name`` absent or ``null`` (breaks with ``name: str``) +* ``empty`` -- ``name`` is ``""`` or only whitespace (would break only if we + also add ``min_length=1`` / a non-blank pattern) + +together with whether the object carries an ``identifier`` (i.e. whether the +alternative "``name`` OR ``identifier``" rule would still accept it). + +To tell "valid -> invalid" apart from "already invalid", each Dandiset record is +also migrated to the current schema version and validated with the *installed* +``dandischema`` (``--no-validate`` to skip). Run it with ``dandischema`` from +``master`` to get the baseline; the verdict ``WOULD_BREAK`` means the record +validates today but contains at least one ``missing`` name. + +Examples +-------- +Quick look at a few Dandisets:: + + python tools/audit_nameless_items.py --dandiset 000003 --dandiset 000026 + +Full audit of published Dandiset-level metadata, cached for re-runs:: + + python tools/audit_nameless_items.py --cache-dir ~/.cache/dandi-audit \\ + --jobs 16 --output audit.jsonl + +Include asset-level metadata (downloads can be large; see ``--max-assets-mb``):: + + python tools/audit_nameless_items.py --assets --output audit.jsonl +""" + +from __future__ import annotations + +import argparse +from collections import Counter +from concurrent.futures import ThreadPoolExecutor, as_completed +from dataclasses import asdict, dataclass, field +import json +import os +from pathlib import Path +import re +import sys +from typing import Any, Dict, Iterator, List, Optional, Set, Tuple +from xml.etree import ElementTree + +import requests + +# Published records use "DANDI:" identifiers; the models pick the instance +# config up at import time, so it has to be in place before importing them. +os.environ.setdefault("DANDI_INSTANCE_NAME", "DANDI") + +from pydantic import BaseModel, ValidationError # noqa: E402 + +from dandischema import models # noqa: E402 +from dandischema.consts import DANDI_SCHEMA_VERSION # noqa: E402 +from dandischema.metadata import migrate # noqa: E402 + +DEFAULT_BUCKET_URL = "https://dandiarchive.s3.amazonaws.com" +S3_NS = {"s3": "http://s3.amazonaws.com/doc/2006-03-01/"} +VERSION_RE = re.compile(r"^\d+\.\d{6}\.\d{4}$") + +# Top-level records for which an optional ``name`` is by design (assets are +# identified by path; ``CommonModel`` is only an abstract base). +EXCLUDED_CLASSES = {"CommonModel", "BareAsset", "Asset"} + + +@dataclass +class Finding: + schema_key: str + path: str + kind: str # "missing" | "empty" + has_identifier: bool + identifier: Optional[str] = None + + +@dataclass +class VersionReport: + dandiset: str + version: str + schema_version: Optional[str] = None + valid_now: Optional[bool] = None + validation_error: Optional[str] = None + findings: List[Finding] = field(default_factory=list) + asset_findings: List[Finding] = field(default_factory=list) + assets_scanned: Optional[int] = None + assets_skipped_reason: Optional[str] = None + fetch_error: Optional[str] = None + + @property + def verdict(self) -> str: + if self.fetch_error: + return "FETCH_ERROR" + missing = any(f.kind == "missing" for f in self.findings) + if not missing: + return "OK_EMPTY_ONLY" if self.findings else "OK" + if self.valid_now is None: + return "HAS_MISSING" + return "WOULD_BREAK" if self.valid_now else "ALREADY_INVALID" + + +def optional_name_schema_keys() -> Set[str]: + """``schemaKey`` values of models whose ``name`` field is optional.""" + keys: Set[str] = set() + for obj in vars(models).values(): + if not ( + isinstance(obj, type) + and issubclass(obj, BaseModel) + and obj.__module__ == models.__name__ + and obj.__name__ not in EXCLUDED_CLASSES + ): + continue + fields = obj.model_fields + if "name" not in fields or fields["name"].is_required(): + continue + default = fields["schemaKey"].default if "schemaKey" in fields else None + keys.add(default if isinstance(default, str) else obj.__name__) + return keys + + +def scan(obj: Any, targets: Set[str], path: str = "") -> Iterator[Finding]: + """Yield findings for nested dicts with a targeted ``schemaKey``.""" + if isinstance(obj, dict): + key = obj.get("schemaKey") + if path and key in targets: + name = obj.get("name") + kind = None + if name is None: + kind = "missing" + elif isinstance(name, str) and not name.strip(): + kind = "empty" + if kind: + ident = obj.get("identifier") + yield Finding( + schema_key=key, + path=path, + kind=kind, + has_identifier=bool(ident), + identifier=str(ident) if ident else None, + ) + for k, v in obj.items(): + yield from scan(v, targets, f"{path}.{k}" if path else k) + elif isinstance(obj, list): + for i, v in enumerate(obj): + yield from scan(v, targets, f"{path}[{i}]") + + +class Fetcher: + def __init__(self, bucket_url: str, cache_dir: Optional[Path]) -> None: + self.bucket_url = bucket_url.rstrip("/") + self.cache_dir = cache_dir + self.session = requests.Session() + + def list_prefixes(self, prefix: str) -> List[str]: + out: List[str] = [] + token: Optional[str] = None + while True: + params = {"list-type": "2", "prefix": prefix, "delimiter": "/"} + if token: + params["continuation-token"] = token + r = self.session.get(self.bucket_url + "/", params=params, timeout=60) + r.raise_for_status() + root = ElementTree.fromstring(r.content) + for p in root.findall("s3:CommonPrefixes/s3:Prefix", S3_NS): + assert p.text is not None + out.append(p.text[len(prefix) :].rstrip("/")) + if root.findtext("s3:IsTruncated", namespaces=S3_NS) != "true": + return out + token = root.findtext("s3:NextContinuationToken", namespaces=S3_NS) + + def size(self, key: str) -> Optional[int]: + r = self.session.head(f"{self.bucket_url}/{key}", timeout=60) + if r.status_code != 200: + return None + return int(r.headers.get("Content-Length", 0)) + + def get_json(self, key: str) -> Any: + cached = self.cache_dir / key if self.cache_dir else None + if cached is not None and cached.exists(): + return json.loads(cached.read_bytes()) + r = self.session.get(f"{self.bucket_url}/{key}", timeout=600) + r.raise_for_status() + if cached is not None: + cached.parent.mkdir(parents=True, exist_ok=True) + cached.write_bytes(r.content) + return r.json() + + +def validate_now(meta: Dict[str, Any]) -> Tuple[bool, Optional[str]]: + """Validate a Dandiset record with the installed ``dandischema``.""" + try: + migrated = migrate(meta, to_version=DANDI_SCHEMA_VERSION, skip_validation=True) + models.Dandiset.model_validate(migrated) + except ValidationError as e: + errs = "; ".join( + f"{'.'.join(map(str, err['loc']))}: {err['msg']}" for err in e.errors() + ) + return False, errs[:2000] + except Exception as e: # migration failures etc. + return False, f"{type(e).__name__}: {e}"[:2000] + return True, None + + +def audit_version( + fetcher: Fetcher, + dandiset: str, + version: str, + targets: Set[str], + do_validate: bool, + do_assets: bool, + max_assets_mb: float, +) -> VersionReport: + rep = VersionReport(dandiset=dandiset, version=version) + base = f"dandisets/{dandiset}/{version}" + try: + meta = fetcher.get_json(f"{base}/dandiset.jsonld") + except Exception as e: + rep.fetch_error = f"{type(e).__name__}: {e}" + return rep + rep.schema_version = meta.get("schemaVersion") + rep.findings = list(scan(meta, targets)) + if do_validate: + rep.valid_now, rep.validation_error = validate_now(meta) + if do_assets: + key = f"{base}/assets.jsonld" + size = fetcher.size(key) + if size is None: + rep.assets_skipped_reason = "no assets.jsonld" + elif max_assets_mb and size > max_assets_mb * 2**20: + rep.assets_skipped_reason = f"assets.jsonld is {size / 2**20:.0f} MB" + else: + try: + assets = fetcher.get_json(key) + except Exception as e: + rep.assets_skipped_reason = f"{type(e).__name__}: {e}" + else: + rep.assets_scanned = len(assets) + for i, asset in enumerate(assets): + for f in scan(asset, targets): + f.path = f"{asset.get('path', i)}:{f.path}" + rep.asset_findings.append(f) + return rep + + +def list_versions( + fetcher: Fetcher, dandisets: List[str], include_draft: bool, jobs: int +) -> List[Tuple[str, str]]: + if not dandisets: + dandisets = [d for d in fetcher.list_prefixes("dandisets/") if d.isdigit()] + with ThreadPoolExecutor(max_workers=jobs) as pool: + listings = pool.map( + lambda ds: (ds, fetcher.list_prefixes(f"dandisets/{ds}/")), dandisets + ) + return sorted( + (ds, v) + for ds, versions in listings + for v in versions + if VERSION_RE.match(v) or (include_draft and v == "draft") + ) + + +def main(argv: Optional[List[str]] = None) -> int: + p = argparse.ArgumentParser( + description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter + ) + p.add_argument("--bucket-url", default=DEFAULT_BUCKET_URL) + p.add_argument( + "--dandiset", + action="append", + default=[], + help="Limit to this Dandiset id (repeatable). Default: all.", + ) + p.add_argument("--include-draft", action="store_true") + p.add_argument( + "--schema-key", + action="append", + default=None, + help="schemaKey to check (repeatable). Default: every class with an " + "optional `name`, derived from the installed dandischema.", + ) + p.add_argument("--no-validate", dest="validate", action="store_false") + p.add_argument("--assets", action="store_true", help="Also scan assets.jsonld") + p.add_argument( + "--max-assets-mb", + type=float, + default=200, + help="Skip assets.jsonld larger than this (0 = no limit). Default: 200", + ) + p.add_argument("--cache-dir", type=Path) + p.add_argument("--jobs", "-j", type=int, default=8) + p.add_argument("--output", "-o", type=Path, help="Write per-version JSON lines") + args = p.parse_args(argv) + + targets = set(args.schema_key) if args.schema_key else optional_name_schema_keys() + print(f"Checking schemaKeys: {', '.join(sorted(targets))}", file=sys.stderr) + + fetcher = Fetcher(args.bucket_url, args.cache_dir) + versions = list_versions(fetcher, args.dandiset, args.include_draft, args.jobs) + print(f"{len(versions)} versions to audit", file=sys.stderr) + + reports: List[VersionReport] = [] + with ThreadPoolExecutor(max_workers=args.jobs) as pool: + futs = [ + pool.submit( + audit_version, + fetcher, + ds, + v, + targets, + args.validate, + args.assets, + args.max_assets_mb, + ) + for ds, v in versions + ] + for n, fut in enumerate(as_completed(futs), 1): + rep = fut.result() + reports.append(rep) + if rep.findings or rep.asset_findings or rep.fetch_error: + print( + f"[{n}/{len(futs)}] {rep.dandiset}/{rep.version}: {rep.verdict}" + f" ({len(rep.findings)} dandiset-level," + f" {len(rep.asset_findings)} asset-level findings)", + file=sys.stderr, + ) + + reports.sort(key=lambda r: (r.dandiset, r.version)) + if args.output: + with args.output.open("w") as fh: + for rep in reports: + fh.write(json.dumps({**asdict(rep), "verdict": rep.verdict}) + "\n") + + # ---- summary ----------------------------------------------------------- + verdicts = Counter(r.verdict for r in reports) + by_key: Counter[Tuple[str, str, bool]] = Counter() + asset_by_key: Counter[Tuple[str, str, bool]] = Counter() + for r in reports: + by_key.update((f.schema_key, f.kind, f.has_identifier) for f in r.findings) + asset_by_key.update( + (f.schema_key, f.kind, f.has_identifier) for f in r.asset_findings + ) + + print(f"\n# Versions audited: {len(reports)}") + for v, c in sorted(verdicts.items()): + print(f" {v:16} {c}") + for title, counter in ( + ("Dandiset-level", by_key), + ("Asset-level", asset_by_key), + ): + if not counter: + continue + print(f"\n# {title} findings (schemaKey, kind, has identifier): count") + for (sk, kind, has_id), c in sorted(counter.items()): + print(f" {sk:26} {kind:8} {'id' if has_id else 'no-id':6} {c}") + breaking = [r for r in reports if r.verdict == "WOULD_BREAK"] + if breaking: + print("\n# Published versions that are valid now but would become invalid:") + for r in breaking: + paths = ", ".join( + f"{f.path}({f.schema_key})" for f in r.findings if f.kind == "missing" + ) + print(f" {r.dandiset}/{r.version}: {paths}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 742cc636cce1b3088186e1db8e9a1f0e19a21c13 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 17:16:41 +0000 Subject: [PATCH 2/4] Require non-empty `name` for Contributor and BaseType (schema 0.8.1) `Contributor.name` (thus also `Organization.name`) and `BaseType.name` (thus `Anatomy`, `Disorder`, `GenericType`, `SpeciesType`, ... names) were Optional, so items without any human-readable label validated and were rendered as blank cards on the Dandiset landing page (dandi/dandi-archive#2936). Make them required with `min_length=1`. This also makes the Contributor hierarchy homogeneous for the LinkML migration: `Person` no longer needs to refine `name` from optional to required (#389/#405), only to add its "Last, First" pattern. `Resource.name` stays optional: >61k asset-level and 34 dandiset-level published records rely on identifier/url only (enforced by the existing `identifier_or_url` validator). Bump DANDI_SCHEMA_VERSION to 0.8.1 (0.8.0 is already released) and keep 0.8.0 as an allowed input and (lossless) migration target. The audit script defaults now to all classes with a `name` field, since the tightened ones no longer have it optional. Closes #442 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01NdN9xPzPo1AzsnEMuU8UQs --- dandischema/consts.py | 5 ++-- dandischema/models.py | 6 ++-- dandischema/tests/test_models.py | 48 ++++++++++++++++++++++++++++++-- tools/audit_nameless_items.py | 32 +++++++++++---------- 4 files changed, 68 insertions(+), 23 deletions(-) diff --git a/dandischema/consts.py b/dandischema/consts.py index 863d7647..38310d8d 100644 --- a/dandischema/consts.py +++ b/dandischema/consts.py @@ -1,6 +1,6 @@ from packaging.version import Version as _Version -DANDI_SCHEMA_VERSION = "0.8.0" +DANDI_SCHEMA_VERSION = "0.8.1" ALLOWED_INPUT_SCHEMAS = [ "0.4.4", "0.5.1", @@ -17,12 +17,13 @@ "0.6.9", "0.6.10", "0.7.0", + "0.8.0", DANDI_SCHEMA_VERSION, ] # We establish migrations (back) to only a few recent versions. # When adding changes, please consider whether a migration path should be added. -ALLOWED_TARGET_SCHEMAS = ["0.6.10", "0.7.0", DANDI_SCHEMA_VERSION] +ALLOWED_TARGET_SCHEMAS = ["0.6.10", "0.7.0", "0.8.0", DANDI_SCHEMA_VERSION] # This allows multiple schemas for validation, whereas target schemas focus on # migration. diff --git a/dandischema/models.py b/dandischema/models.py index c91afad0..0a5cac17 100644 --- a/dandischema/models.py +++ b/dandischema/models.py @@ -736,9 +736,9 @@ class BaseType(DandiBaseModel): " supported by identifiers.org.", json_schema_extra={"nskey": "schema"}, ) - name: Optional[str] = Field( - None, + name: str = Field( description="The name of the item.", + min_length=1, max_length=150, json_schema_extra={"nskey": "schema"}, ) @@ -906,7 +906,7 @@ class Contributor(DandiBaseModel): "people or ROR (ror.org) for institutions.", json_schema_extra={"nskey": "schema"}, ) - name: Optional[str] = Field(None, json_schema_extra={"nskey": "schema"}) + name: str = Field(min_length=1, json_schema_extra={"nskey": "schema"}) email: Optional[EmailStr] = Field(None, json_schema_extra={"nskey": "schema"}) url: Optional[AnyHttpUrl] = Field(None, json_schema_extra={"nskey": "schema"}) roleName: Optional[List[RoleType]] = Field( diff --git a/dandischema/tests/test_models.py b/dandischema/tests/test_models.py index cd61fe05..024ef113 100644 --- a/dandischema/tests/test_models.py +++ b/dandischema/tests/test_models.py @@ -850,14 +850,14 @@ def test_contact_person_without_email(self, roles: List[RoleType]) -> None: with pytest.raises( pydantic.ValidationError, match="Contact person must have an email address" ): - Contributor(roleName=roles) + Contributor(name="Nemo", roleName=roles) @pytest.mark.parametrize("roles", _NON_CONTACT_PERSON_ROLES_ARGS) def test_non_contact_person_without_email(self, roles: List[RoleType]) -> None: """ Test creating a `Contributor` instance as a non-contact person without an email """ - Contributor(roleName=roles) + Contributor(name="Nemo", roleName=roles) @pytest.mark.parametrize( "roles", _NON_CONTACT_PERSON_ROLES_ARGS + _CONTACT_PERSON_ROLES_ARGS @@ -866,7 +866,49 @@ def test_with_email(self, roles: List[RoleType]) -> None: """ Test creating a `Contributor` instance with an email """ - Contributor(email="nemo@dandiarchive.org", roleName=roles) + Contributor(name="Nemo", email="nemo@dandiarchive.org", roleName=roles) + + @pytest.mark.ai_generated + @pytest.mark.parametrize("model", [Contributor, Organization, Person]) + @pytest.mark.parametrize("name_kwargs", [{}, {"name": None}, {"name": ""}]) + def test_name_required( + self, model: Type[Contributor], name_kwargs: Dict[str, Any] + ) -> None: + """ + Test that `name` is required and non-empty for `Contributor` and subclasses + + (https://github.com/dandi/dandi-schema/issues/442) + """ + with pytest.raises(pydantic.ValidationError) as exc_info: + model(**name_kwargs) + assert [e["loc"] for e in exc_info.value.errors()] == [("name",)] + + +@pytest.mark.ai_generated +@pytest.mark.parametrize( + "model", + [ + m + for m in vars(models).values() + if isclass(m) and issubclass(m, BaseType) and m.__module__ == models.__name__ + ], +) +class TestBaseTypeName: + """ + `name` is required and non-empty for `BaseType` and all its subclasses + (https://github.com/dandi/dandi-schema/issues/442) + """ + + @pytest.mark.parametrize("name_kwargs", [{}, {"name": None}, {"name": ""}]) + def test_name_required( + self, model: Type[BaseType], name_kwargs: Dict[str, Any] + ) -> None: + with pytest.raises(pydantic.ValidationError) as exc_info: + model(identifier="UBERON:0004727", **name_kwargs) + assert [e["loc"] for e in exc_info.value.errors()] == [("name",)] + + def test_with_name(self, model: Type[BaseType]) -> None: + assert model(name="cochlear nerve").name == "cochlear nerve" def _get_field_pattern( diff --git a/tools/audit_nameless_items.py b/tools/audit_nameless_items.py index 9d8dc11b..d589b29d 100755 --- a/tools/audit_nameless_items.py +++ b/tools/audit_nameless_items.py @@ -1,16 +1,17 @@ #!/usr/bin/env python3 """Audit published DANDI manifests for nested items lacking a ``name``. -Motivation: https://github.com/dandi/dandi-schema/issues/442 proposes making -``name`` required on ``Contributor`` (hence ``Organization``), ``BaseType`` -subclasses (``Anatomy``, ``SpeciesType``, ...) and possibly other classes whose -``name`` is currently ``Optional``. Before doing so we need to know which -already *published* (immutable) versions would turn from valid to invalid. +Motivation: https://github.com/dandi/dandi-schema/issues/442 made ``name`` +required on ``Contributor`` (hence ``Organization``) and ``BaseType`` subclasses +(``Anatomy``, ``SpeciesType``, ...), which were ``Optional`` in schema 0.8.0 and +earlier. This script tells which already *published* (immutable) versions +turn from valid to invalid by such a change, and can be reused for similar +tightening of other classes. The script walks the public S3 manifests of a DANDI instance (``dandisets///dandiset.jsonld`` and, optionally, ``assets.jsonld``) without authentication, and for every nested object whose -``schemaKey`` belongs to a class with an optional ``name`` it reports: +``schemaKey`` belongs to a class with a ``name`` field it reports: * ``missing`` -- ``name`` absent or ``null`` (breaks with ``name: str``) * ``empty`` -- ``name`` is ``""`` or only whitespace (would break only if we @@ -21,9 +22,10 @@ To tell "valid -> invalid" apart from "already invalid", each Dandiset record is also migrated to the current schema version and validated with the *installed* -``dandischema`` (``--no-validate`` to skip). Run it with ``dandischema`` from -``master`` to get the baseline; the verdict ``WOULD_BREAK`` means the record -validates today but contains at least one ``missing`` name. +``dandischema`` (``--no-validate`` to skip). Run it with a ``dandischema`` +version *preceding* the tightening (e.g. ``pip install dandischema==0.14.0``) +to get the baseline; the verdict ``WOULD_BREAK`` means the record validates +with it but contains at least one ``missing`` name. Examples -------- @@ -110,8 +112,8 @@ def verdict(self) -> str: return "WOULD_BREAK" if self.valid_now else "ALREADY_INVALID" -def optional_name_schema_keys() -> Set[str]: - """``schemaKey`` values of models whose ``name`` field is optional.""" +def named_schema_keys() -> Set[str]: + """``schemaKey`` values of models having a ``name`` field.""" keys: Set[str] = set() for obj in vars(models).values(): if not ( @@ -122,7 +124,7 @@ def optional_name_schema_keys() -> Set[str]: ): continue fields = obj.model_fields - if "name" not in fields or fields["name"].is_required(): + if "name" not in fields: continue default = fields["schemaKey"].default if "schemaKey" in fields else None keys.add(default if isinstance(default, str) else obj.__name__) @@ -286,8 +288,8 @@ def main(argv: Optional[List[str]] = None) -> int: "--schema-key", action="append", default=None, - help="schemaKey to check (repeatable). Default: every class with an " - "optional `name`, derived from the installed dandischema.", + help="schemaKey to check (repeatable). Default: every class with a " + "`name` field, derived from the installed dandischema.", ) p.add_argument("--no-validate", dest="validate", action="store_false") p.add_argument("--assets", action="store_true", help="Also scan assets.jsonld") @@ -302,7 +304,7 @@ def main(argv: Optional[List[str]] = None) -> int: p.add_argument("--output", "-o", type=Path, help="Write per-version JSON lines") args = p.parse_args(argv) - targets = set(args.schema_key) if args.schema_key else optional_name_schema_keys() + targets = set(args.schema_key) if args.schema_key else named_schema_keys() print(f"Checking schemaKeys: {', '.join(sorted(targets))}", file=sys.stderr) fetcher = Fetcher(args.bucket_url, args.cache_dir) From 5cd2a79b4b902c2482901015debdd28f20578827 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 17:33:34 +0000 Subject: [PATCH 3/4] audit_nameless_items: also detect records carrying nothing but schemaKey Nested objects of any class whose fields other than `schemaKey` are all absent, null, empty, or equal to their defaults are reported as `empty_record` (disable with --no-empty-records). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01NdN9xPzPo1AzsnEMuU8UQs --- tools/audit_nameless_items.py | 108 ++++++++++++++++++++++++++++++---- 1 file changed, 96 insertions(+), 12 deletions(-) diff --git a/tools/audit_nameless_items.py b/tools/audit_nameless_items.py index d589b29d..c05d7993 100755 --- a/tools/audit_nameless_items.py +++ b/tools/audit_nameless_items.py @@ -17,6 +17,12 @@ * ``empty`` -- ``name`` is ``""`` or only whitespace (would break only if we also add ``min_length=1`` / a non-blank pattern) +and, for nested objects of *any* class (``--no-empty-records`` to skip): + +* ``empty_record`` -- nothing but ``schemaKey``: every other field is absent, + ``null``, an empty string/list/dict, or equal to the field's default (e.g. + ``{"schemaKey": "Organization", "roleName": [], "includeInCitation": false}``) + together with whether the object carries an ``identifier`` (i.e. whether the alternative "``name`` OR ``identifier``" rule would still accept it). @@ -25,7 +31,7 @@ ``dandischema`` (``--no-validate`` to skip). Run it with a ``dandischema`` version *preceding* the tightening (e.g. ``pip install dandischema==0.14.0``) to get the baseline; the verdict ``WOULD_BREAK`` means the record validates -with it but contains at least one ``missing`` name. +with it but contains at least one ``missing`` name or ``empty_record``. Examples -------- @@ -77,12 +83,17 @@ # identified by path; ``CommonModel`` is only an abstract base). EXCLUDED_CLASSES = {"CommonModel", "BareAsset", "Asset"} +# Finding kinds which make a record invalid under the proposed changes +BREAKING_KINDS = {"missing", "empty_record"} + +_NO_DEFAULT = object() + @dataclass class Finding: schema_key: str path: str - kind: str # "missing" | "empty" + kind: str # "missing" | "empty" | "empty_record" has_identifier: bool identifier: Optional[str] = None @@ -104,7 +115,7 @@ class VersionReport: def verdict(self) -> str: if self.fetch_error: return "FETCH_ERROR" - missing = any(f.kind == "missing" for f in self.findings) + missing = any(f.kind in BREAKING_KINDS for f in self.findings) if not missing: return "OK_EMPTY_ONLY" if self.findings else "OK" if self.valid_now is None: @@ -131,11 +142,71 @@ def named_schema_keys() -> Set[str]: return keys -def scan(obj: Any, targets: Set[str], path: str = "") -> Iterator[Finding]: - """Yield findings for nested dicts with a targeted ``schemaKey``.""" +def field_defaults() -> Dict[str, Dict[str, Any]]: + """Map ``schemaKey`` to ``{field: default}`` of the corresponding model. + + Fields without a default map to ``_NO_DEFAULT``. + """ + out: Dict[str, Dict[str, Any]] = {} + for obj in vars(models).values(): + if not ( + isinstance(obj, type) + and issubclass(obj, BaseModel) + and obj.__module__ == models.__name__ + and "schemaKey" in obj.model_fields + ): + continue + key = obj.model_fields["schemaKey"].default + if not isinstance(key, str) or obj.__name__ != key: + continue # aliases and bases sharing a subclass' schemaKey + out[key] = { + name: (_NO_DEFAULT if f.is_required() else f.get_default()) + for name, f in obj.model_fields.items() + } + return out + + +def _is_blank(value: Any, default: Any) -> bool: + if value is None or (isinstance(value, (str, list, dict)) and not value): + return True + if isinstance(value, str) and not value.strip(): + return True + return default is not _NO_DEFAULT and value == default + + +def is_empty_record(obj: Dict[str, Any], defaults: Dict[str, Any]) -> bool: + """Whether ``obj`` carries no information besides its ``schemaKey``.""" + return all( + _is_blank(v, defaults.get(k, _NO_DEFAULT)) + for k, v in obj.items() + if k != "schemaKey" + ) + + +def scan( + obj: Any, + targets: Set[str], + defaults: Optional[Dict[str, Dict[str, Any]]] = None, + path: str = "", +) -> Iterator[Finding]: + """Yield findings for nested dicts with a ``schemaKey``. + + ``targets`` are the schemaKeys checked for a missing/empty ``name``; + if ``defaults`` (see `field_defaults`) is given, nested records of any + schemaKey are also checked for being empty (`is_empty_record`). + """ if isinstance(obj, dict): key = obj.get("schemaKey") - if path and key in targets: + if ( + path + and defaults is not None + and isinstance(key, str) + and is_empty_record(obj, defaults.get(key, {})) + ): + yield Finding( + schema_key=key, path=path, kind="empty_record", has_identifier=False + ) + elif path and key in targets: name = obj.get("name") kind = None if name is None: @@ -152,10 +223,10 @@ def scan(obj: Any, targets: Set[str], path: str = "") -> Iterator[Finding]: identifier=str(ident) if ident else None, ) for k, v in obj.items(): - yield from scan(v, targets, f"{path}.{k}" if path else k) + yield from scan(v, targets, defaults, f"{path}.{k}" if path else k) elif isinstance(obj, list): for i, v in enumerate(obj): - yield from scan(v, targets, f"{path}[{i}]") + yield from scan(v, targets, defaults, f"{path}[{i}]") class Fetcher: @@ -219,6 +290,7 @@ def audit_version( dandiset: str, version: str, targets: Set[str], + defaults: Optional[Dict[str, Dict[str, Any]]], do_validate: bool, do_assets: bool, max_assets_mb: float, @@ -231,7 +303,7 @@ def audit_version( rep.fetch_error = f"{type(e).__name__}: {e}" return rep rep.schema_version = meta.get("schemaVersion") - rep.findings = list(scan(meta, targets)) + rep.findings = list(scan(meta, targets, defaults)) if do_validate: rep.valid_now, rep.validation_error = validate_now(meta) if do_assets: @@ -249,7 +321,7 @@ def audit_version( else: rep.assets_scanned = len(assets) for i, asset in enumerate(assets): - for f in scan(asset, targets): + for f in scan(asset, targets, defaults): f.path = f"{asset.get('path', i)}:{f.path}" rep.asset_findings.append(f) return rep @@ -291,6 +363,13 @@ def main(argv: Optional[List[str]] = None) -> int: help="schemaKey to check (repeatable). Default: every class with a " "`name` field, derived from the installed dandischema.", ) + p.add_argument( + "--no-empty-records", + dest="empty_records", + action="store_false", + help="Do not check nested records of any class for carrying nothing " + "but schemaKey", + ) p.add_argument("--no-validate", dest="validate", action="store_false") p.add_argument("--assets", action="store_true", help="Also scan assets.jsonld") p.add_argument( @@ -307,6 +386,8 @@ def main(argv: Optional[List[str]] = None) -> int: targets = set(args.schema_key) if args.schema_key else named_schema_keys() print(f"Checking schemaKeys: {', '.join(sorted(targets))}", file=sys.stderr) + defaults = field_defaults() if args.empty_records else None + fetcher = Fetcher(args.bucket_url, args.cache_dir) versions = list_versions(fetcher, args.dandiset, args.include_draft, args.jobs) print(f"{len(versions)} versions to audit", file=sys.stderr) @@ -320,6 +401,7 @@ def main(argv: Optional[List[str]] = None) -> int: ds, v, targets, + defaults, args.validate, args.assets, args.max_assets_mb, @@ -364,13 +446,15 @@ def main(argv: Optional[List[str]] = None) -> int: continue print(f"\n# {title} findings (schemaKey, kind, has identifier): count") for (sk, kind, has_id), c in sorted(counter.items()): - print(f" {sk:26} {kind:8} {'id' if has_id else 'no-id':6} {c}") + print(f" {sk:26} {kind:12} {'id' if has_id else 'no-id':6} {c}") breaking = [r for r in reports if r.verdict == "WOULD_BREAK"] if breaking: print("\n# Published versions that are valid now but would become invalid:") for r in breaking: paths = ", ".join( - f"{f.path}({f.schema_key})" for f in r.findings if f.kind == "missing" + f"{f.path}({f.schema_key})" + for f in r.findings + if f.kind in BREAKING_KINDS ) print(f" {r.dandiset}/{r.version}: {paths}") return 0 From 45994e10bee7ffa5f129690ee6a7db2cd88fd318 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 04:59:35 +0000 Subject: [PATCH 4/4] audit_nameless_items: decide verdicts by validation, compared with a baseline Address review of #444: - The verdict no longer comes from the `name`/`empty_record` heuristics, which counted empty records (not rejected by #444) as breaking and `name: ""` (rejected by `min_length=1`) as not breaking. Instead every Dandiset record (migrated) and, with --assets, every asset record is validated with the installed dandischema, and --baseline compares with the output of a run made with the dandischema preceding the change: BREAKS if a record valid in the baseline is invalid now, NEW_ERRORS if an already invalid record gets new errors, UNCHANGED otherwise. The scan results remain as pointers to the offending items. - Check only the classes #444 tightens (BaseType and Contributor subclasses) for missing names by default; others via --schema-key. - Cache manifests gzip-compressed, so that draft assets.jsonld fit. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01NdN9xPzPo1AzsnEMuU8UQs --- tools/audit_nameless_items.py | 285 ++++++++++++++++++++++------------ 1 file changed, 184 insertions(+), 101 deletions(-) diff --git a/tools/audit_nameless_items.py b/tools/audit_nameless_items.py index c05d7993..5bb4288a 100755 --- a/tools/audit_nameless_items.py +++ b/tools/audit_nameless_items.py @@ -1,52 +1,62 @@ #!/usr/bin/env python3 -"""Audit published DANDI manifests for nested items lacking a ``name``. +"""Audit DANDI manifests for records which a schema change would invalidate. Motivation: https://github.com/dandi/dandi-schema/issues/442 made ``name`` required on ``Contributor`` (hence ``Organization``) and ``BaseType`` subclasses (``Anatomy``, ``SpeciesType``, ...), which were ``Optional`` in schema 0.8.0 and -earlier. This script tells which already *published* (immutable) versions -turn from valid to invalid by such a change, and can be reused for similar -tightening of other classes. +earlier. This script tells which existing versions turn from valid to invalid +by such a change, and can be reused for similar tightening of other classes. The script walks the public S3 manifests of a DANDI instance -(``dandisets///dandiset.jsonld`` and, optionally, -``assets.jsonld``) without authentication, and for every nested object whose -``schemaKey`` belongs to a class with a ``name`` field it reports: +(``dandisets///dandiset.jsonld`` and, with ``--assets``, +``assets.jsonld``) without authentication and -* ``missing`` -- ``name`` absent or ``null`` (breaks with ``name: str``) -* ``empty`` -- ``name`` is ``""`` or only whitespace (would break only if we - also add ``min_length=1`` / a non-blank pattern) +1. validates every record with the *installed* ``dandischema``: Dandiset + records are migrated to the current schema version and validated as + ``Dandiset``, asset records are validated as ``Asset`` + (``--no-validate`` to skip); +2. reports items which point at the cause, for nested objects whose + ``schemaKey`` belongs to one of the checked classes (by default the + ``BaseType`` and ``Contributor`` subclasses; see ``--schema-key``): -and, for nested objects of *any* class (``--no-empty-records`` to skip): + * ``missing`` -- ``name`` absent or ``null`` + * ``empty`` -- ``name`` is ``""`` or only whitespace -* ``empty_record`` -- nothing but ``schemaKey``: every other field is absent, - ``null``, an empty string/list/dict, or equal to the field's default (e.g. - ``{"schemaKey": "Organization", "roleName": [], "includeInCitation": false}``) + and, for nested objects of *any* class (``--no-empty-records`` to skip): -together with whether the object carries an ``identifier`` (i.e. whether the -alternative "``name`` OR ``identifier``" rule would still accept it). + * ``empty_record`` -- nothing but ``schemaKey``: every other field is + absent, ``null``, an empty string/list/dict, or equal to the field's + default (e.g. ``{"schemaKey": "ContactPoint"}``) -To tell "valid -> invalid" apart from "already invalid", each Dandiset record is -also migrated to the current schema version and validated with the *installed* -``dandischema`` (``--no-validate`` to skip). Run it with a ``dandischema`` -version *preceding* the tightening (e.g. ``pip install dandischema==0.14.0``) -to get the baseline; the verdict ``WOULD_BREAK`` means the record validates -with it but contains at least one ``missing`` name or ``empty_record``. +The verdict is decided by validation, not by these items. Run the script +twice: once with the ``dandischema`` preceding the change (the baseline) and +once with the changed one, passing the output of the first run with +``--baseline``. A version is then reported as -Examples --------- -Quick look at a few Dandisets:: +* ``BREAKS`` -- some record (the Dandiset or an asset) validates with the + baseline but not with the installed ``dandischema``; +* ``NEW_ERRORS`` -- no record turns invalid, but some already invalid record + gets new validation errors; +* ``UNCHANGED`` -- otherwise. - python tools/audit_nameless_items.py --dandiset 000003 --dandiset 000026 +Without ``--baseline`` a version is reported as ``VALID`` or ``INVALID``. -Full audit of published Dandiset-level metadata, cached for re-runs:: +Examples +-------- +Baseline with the released ``dandischema``, then the change, comparing the two +(``--cache-dir`` avoids downloading the manifests twice):: - python tools/audit_nameless_items.py --cache-dir ~/.cache/dandi-audit \\ - --jobs 16 --output audit.jsonl + pip install dandischema==0.14.0 + python tools/audit_nameless_items.py --include-draft --assets \\ + --cache-dir ~/.cache/dandi-audit -j 16 -o baseline.jsonl + pip install -e . + python tools/audit_nameless_items.py --include-draft --assets \\ + --cache-dir ~/.cache/dandi-audit -j 16 -o new.jsonl \\ + --baseline baseline.jsonl -Include asset-level metadata (downloads can be large; see ``--max-assets-mb``):: +Quick look at a few Dandisets:: - python tools/audit_nameless_items.py --assets --output audit.jsonl + python tools/audit_nameless_items.py --dandiset 000003 --dandiset 000026 """ from __future__ import annotations @@ -55,6 +65,7 @@ from collections import Counter from concurrent.futures import ThreadPoolExecutor, as_completed from dataclasses import asdict, dataclass, field +import gzip import json import os from pathlib import Path @@ -79,12 +90,9 @@ S3_NS = {"s3": "http://s3.amazonaws.com/doc/2006-03-01/"} VERSION_RE = re.compile(r"^\d+\.\d{6}\.\d{4}$") -# Top-level records for which an optional ``name`` is by design (assets are -# identified by path; ``CommonModel`` is only an abstract base). -EXCLUDED_CLASSES = {"CommonModel", "BareAsset", "Asset"} - -# Finding kinds which make a record invalid under the proposed changes -BREAKING_KINDS = {"missing", "empty_record"} +# Validation errors kept per record (the full list can be long for records +# made with old schema versions) +MAX_ERRORS = 50 _NO_DEFAULT = object() @@ -103,42 +111,33 @@ class VersionReport: dandiset: str version: str schema_version: Optional[str] = None - valid_now: Optional[bool] = None - validation_error: Optional[str] = None + # Validation errors ("loc: type") of the Dandiset record; None if not + # validated, [] if valid + errors: Optional[List[str]] = None + # Validation errors of the invalid assets, by asset path; None if not + # validated + asset_errors: Optional[Dict[str, List[str]]] = None findings: List[Finding] = field(default_factory=list) asset_findings: List[Finding] = field(default_factory=list) assets_scanned: Optional[int] = None assets_skipped_reason: Optional[str] = None fetch_error: Optional[str] = None + # Filled in when compared with a baseline + verdict: Optional[str] = None + new_errors: Dict[str, List[str]] = field(default_factory=dict) - @property - def verdict(self) -> str: - if self.fetch_error: - return "FETCH_ERROR" - missing = any(f.kind in BREAKING_KINDS for f in self.findings) - if not missing: - return "OK_EMPTY_ONLY" if self.findings else "OK" - if self.valid_now is None: - return "HAS_MISSING" - return "WOULD_BREAK" if self.valid_now else "ALREADY_INVALID" - -def named_schema_keys() -> Set[str]: - """``schemaKey`` values of models having a ``name`` field.""" +def checked_schema_keys() -> Set[str]: + """``schemaKey`` values of the ``BaseType`` and ``Contributor`` subclasses.""" keys: Set[str] = set() for obj in vars(models).values(): - if not ( + if ( isinstance(obj, type) - and issubclass(obj, BaseModel) + and issubclass(obj, (models.BaseType, models.Contributor)) and obj.__module__ == models.__name__ - and obj.__name__ not in EXCLUDED_CLASSES ): - continue - fields = obj.model_fields - if "name" not in fields: - continue - default = fields["schemaKey"].default if "schemaKey" in fields else None - keys.add(default if isinstance(default, str) else obj.__name__) + default = obj.model_fields["schemaKey"].default + keys.add(default if isinstance(default, str) else obj.__name__) return keys @@ -259,30 +258,49 @@ def size(self, key: str) -> Optional[int]: return int(r.headers.get("Content-Length", 0)) def get_json(self, key: str) -> Any: - cached = self.cache_dir / key if self.cache_dir else None - if cached is not None and cached.exists(): - return json.loads(cached.read_bytes()) + """Fetch a JSON document, caching it gzip-compressed in ``cache_dir``""" + if self.cache_dir is not None: + plain = self.cache_dir / key + gz = plain.with_name(plain.name + ".gz") + if gz.exists(): + return json.loads(gzip.decompress(gz.read_bytes())) + if plain.exists(): # cache made by earlier versions of this script + return json.loads(plain.read_bytes()) r = self.session.get(f"{self.bucket_url}/{key}", timeout=600) r.raise_for_status() - if cached is not None: - cached.parent.mkdir(parents=True, exist_ok=True) - cached.write_bytes(r.content) + if self.cache_dir is not None: + gz.parent.mkdir(parents=True, exist_ok=True) + gz.write_bytes(gzip.compress(r.content)) return r.json() -def validate_now(meta: Dict[str, Any]) -> Tuple[bool, Optional[str]]: - """Validate a Dandiset record with the installed ``dandischema``.""" +def _errors(e: Exception) -> List[str]: + if isinstance(e, ValidationError): + errs = sorted( + {f"{'.'.join(map(str, err['loc']))}: {err['type']}" for err in e.errors()} + ) + else: # migration failures etc. + errs = [f"{type(e).__name__}: {e}"[:500]] + return errs[:MAX_ERRORS] + + +def validate_dandiset(meta: Dict[str, Any]) -> List[str]: + """Validation errors of a Dandiset record with the installed ``dandischema``""" try: migrated = migrate(meta, to_version=DANDI_SCHEMA_VERSION, skip_validation=True) models.Dandiset.model_validate(migrated) - except ValidationError as e: - errs = "; ".join( - f"{'.'.join(map(str, err['loc']))}: {err['msg']}" for err in e.errors() - ) - return False, errs[:2000] - except Exception as e: # migration failures etc. - return False, f"{type(e).__name__}: {e}"[:2000] - return True, None + except Exception as e: + return _errors(e) + return [] + + +def validate_asset(meta: Dict[str, Any]) -> List[str]: + """Validation errors of an asset record with the installed ``dandischema``""" + try: + models.Asset.model_validate(meta) + except Exception as e: + return _errors(e) + return [] def audit_version( @@ -305,7 +323,7 @@ def audit_version( rep.schema_version = meta.get("schemaVersion") rep.findings = list(scan(meta, targets, defaults)) if do_validate: - rep.valid_now, rep.validation_error = validate_now(meta) + rep.errors = validate_dandiset(meta) if do_assets: key = f"{base}/assets.jsonld" size = fetcher.size(key) @@ -320,13 +338,54 @@ def audit_version( rep.assets_skipped_reason = f"{type(e).__name__}: {e}" else: rep.assets_scanned = len(assets) + if do_validate: + rep.asset_errors = {} for i, asset in enumerate(assets): + apath = str(asset.get("path", i)) for f in scan(asset, targets, defaults): - f.path = f"{asset.get('path', i)}:{f.path}" + f.path = f"{apath}:{f.path}" rep.asset_findings.append(f) + if rep.asset_errors is not None: + errs = validate_asset(asset) + if errs: + rep.asset_errors[apath] = errs return rep +def compare(rep: VersionReport, baseline: Optional[VersionReport]) -> None: + """Set ``rep.verdict`` and ``rep.new_errors`` relative to ``baseline``""" + if rep.fetch_error: + rep.verdict = "FETCH_ERROR" + return + if baseline is None: + if rep.errors is None: + rep.verdict = "NOT_VALIDATED" + else: + rep.verdict = "INVALID" if rep.errors or rep.asset_errors else "VALID" + return + breaks = False + # (record, errors now, errors in baseline); a record not validated in the + # baseline is not compared + records: List[Tuple[str, List[str], Optional[List[str]]]] = [] + if rep.errors is not None and baseline.errors is not None: + records.append(("", rep.errors, baseline.errors)) + if rep.asset_errors is not None and baseline.asset_errors is not None: + for apath, errs in rep.asset_errors.items(): + records.append((apath, errs, baseline.asset_errors.get(apath, []))) + for name, errs, base_errs in records: + assert base_errs is not None + new = sorted(set(errs) - set(base_errs)) + if new: + rep.new_errors[name] = new + breaks = breaks or not base_errs + if breaks: + rep.verdict = "BREAKS" + elif rep.new_errors: + rep.verdict = "NEW_ERRORS" + else: + rep.verdict = "UNCHANGED" + + def list_versions( fetcher: Fetcher, dandisets: List[str], include_draft: bool, jobs: int ) -> List[Tuple[str, str]]: @@ -344,6 +403,21 @@ def list_versions( ) +def load_baseline(path: Path) -> Dict[Tuple[str, str], VersionReport]: + out: Dict[Tuple[str, str], VersionReport] = {} + with path.open() as fh: + for line in fh: + d = json.loads(line) + rep = VersionReport( + dandiset=d["dandiset"], + version=d["version"], + errors=d.get("errors"), + asset_errors=d.get("asset_errors"), + ) + out[(rep.dandiset, rep.version)] = rep + return out + + def main(argv: Optional[List[str]] = None) -> int: p = argparse.ArgumentParser( description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter @@ -360,8 +434,8 @@ def main(argv: Optional[List[str]] = None) -> int: "--schema-key", action="append", default=None, - help="schemaKey to check (repeatable). Default: every class with a " - "`name` field, derived from the installed dandischema.", + help="schemaKey to check for a missing/empty `name` (repeatable). " + "Default: the BaseType and Contributor subclasses.", ) p.add_argument( "--no-empty-records", @@ -371,7 +445,13 @@ def main(argv: Optional[List[str]] = None) -> int: "but schemaKey", ) p.add_argument("--no-validate", dest="validate", action="store_false") - p.add_argument("--assets", action="store_true", help="Also scan assets.jsonld") + p.add_argument( + "--baseline", + type=Path, + help="JSON lines output of an earlier run (with the dandischema " + "preceding the change) to compare the validation results with", + ) + p.add_argument("--assets", action="store_true", help="Also audit assets.jsonld") p.add_argument( "--max-assets-mb", type=float, @@ -383,10 +463,10 @@ def main(argv: Optional[List[str]] = None) -> int: p.add_argument("--output", "-o", type=Path, help="Write per-version JSON lines") args = p.parse_args(argv) - targets = set(args.schema_key) if args.schema_key else named_schema_keys() + targets = set(args.schema_key) if args.schema_key else checked_schema_keys() print(f"Checking schemaKeys: {', '.join(sorted(targets))}", file=sys.stderr) - defaults = field_defaults() if args.empty_records else None + baseline = load_baseline(args.baseline) if args.baseline else None fetcher = Fetcher(args.bucket_url, args.cache_dir) versions = list_versions(fetcher, args.dandiset, args.include_draft, args.jobs) @@ -410,12 +490,14 @@ def main(argv: Optional[List[str]] = None) -> int: ] for n, fut in enumerate(as_completed(futs), 1): rep = fut.result() + if baseline is not None: + compare(rep, baseline.get((rep.dandiset, rep.version))) + else: + compare(rep, None) reports.append(rep) - if rep.findings or rep.asset_findings or rep.fetch_error: + if rep.verdict in ("BREAKS", "NEW_ERRORS", "FETCH_ERROR"): print( - f"[{n}/{len(futs)}] {rep.dandiset}/{rep.version}: {rep.verdict}" - f" ({len(rep.findings)} dandiset-level," - f" {len(rep.asset_findings)} asset-level findings)", + f"[{n}/{len(futs)}] {rep.dandiset}/{rep.version}: {rep.verdict}", file=sys.stderr, ) @@ -423,7 +505,7 @@ def main(argv: Optional[List[str]] = None) -> int: if args.output: with args.output.open("w") as fh: for rep in reports: - fh.write(json.dumps({**asdict(rep), "verdict": rep.verdict}) + "\n") + fh.write(json.dumps(asdict(rep)) + "\n") # ---- summary ----------------------------------------------------------- verdicts = Counter(r.verdict for r in reports) @@ -434,10 +516,14 @@ def main(argv: Optional[List[str]] = None) -> int: asset_by_key.update( (f.schema_key, f.kind, f.has_identifier) for f in r.asset_findings ) + skipped = [r for r in reports if r.assets_skipped_reason] print(f"\n# Versions audited: {len(reports)}") - for v, c in sorted(verdicts.items()): - print(f" {v:16} {c}") + for v, c in sorted(verdicts.items(), key=lambda kv: str(kv[0])): + print(f" {v!s:16} {c}") + if args.assets: + print(f" assets scanned: {sum(r.assets_scanned or 0 for r in reports)}") + print(f" assets.jsonld skipped: {len(skipped)}") for title, counter in ( ("Dandiset-level", by_key), ("Asset-level", asset_by_key), @@ -447,16 +533,13 @@ def main(argv: Optional[List[str]] = None) -> int: print(f"\n# {title} findings (schemaKey, kind, has identifier): count") for (sk, kind, has_id), c in sorted(counter.items()): print(f" {sk:26} {kind:12} {'id' if has_id else 'no-id':6} {c}") - breaking = [r for r in reports if r.verdict == "WOULD_BREAK"] - if breaking: - print("\n# Published versions that are valid now but would become invalid:") - for r in breaking: - paths = ", ".join( - f"{f.path}({f.schema_key})" - for f in r.findings - if f.kind in BREAKING_KINDS - ) - print(f" {r.dandiset}/{r.version}: {paths}") + changed = [r for r in reports if r.verdict in ("BREAKS", "NEW_ERRORS")] + if changed: + print("\n# Versions with new validation errors (record: errors):") + for r in changed: + print(f" {r.dandiset}/{r.version} [{r.verdict}]") + for name, errs in r.new_errors.items(): + print(f" {name}: {'; '.join(errs)}") return 0