Skip to content

Consolidate Asset/Dandiset models; gate publish validation on datePublished - #419

Merged
yarikoptic merged 3 commits into
masterfrom
consolidate-models
Aug 3, 2026
Merged

Consolidate Asset/Dandiset models; gate publish validation on datePublished#419
yarikoptic merged 3 commits into
masterfrom
consolidate-models

Conversation

@candleindark

@candleindark candleindark commented Jun 8, 2026

Copy link
Copy Markdown
Member

Summary

Make every model class's schemaKey value equal its class name, so that an eventual LinkML translation can use schemaKey as its type designator (designates_type). Three classes violated this (BareAsset"Asset", PublishedAsset"Asset", PublishedDandiset"Dandiset"), and fixing them takes two distinct changes:

  1. Consolidate the publication-specific variants into their base classes: PublishedDandiset merges into Dandiset and PublishedAsset into Asset. Publication requirements now fire via a datePublished-gated validator instead of separate classes.
  2. Align BareAsset.schemaKey to "BareAsset"

Stored schemaKey values in archive data are unchanged, and the old names remain as deprecated aliases so dandi-cli/dandi-archive imports keep working. Adopting this release does, however, require one lockstep dandi-cli change (see Prerequisites).

DANDI_SCHEMA_VERSION is also bumped to 0.8.0, which is due independently of the model consolidation: the schemas generated from master already differ from the published 0.7.0, and this PR changes all four of them further. A summary of the diff against the published 0.7.0, and of why none of it is breaking for existing metadata, is in #419 (comment).

Prerequisites (merge first)

What changed in dandischema/models.py
  • Merge PublishedDandisetDandiset, PublishedAssetAsset. Publication-only fields (doi, publishedBy, datePublished, releaseNotes on Dandiset; publishedBy, datePublished on Asset) become optional, and the publication requirements move into a datePublished-gated check_publication_status model validator:

    • datePublished is None (draft) → publication-only fields must be absent;
    • datePublished set (published) → enforce the former Published* rules (publishedBy/url/doi presence, stricter id/url patterns, check_filesbytes, digest_sha256check).

    dandi-archive injects datePublished before validating, so the gated checks fire exactly as the Published* classes did.

  • Per-field validation errors. The publication validator accumulates all violations and raises them via pydantic_core.ValidationError.from_exception_data(...), so each error carries its own field loc (e.g. ('assetsSummary',)) plus the usual "Value error, …" message — rather than one model-level error with an empty loc.

  • Keep BareAsset (Asset still inherits it) with schemaKey: Literal["BareAsset"]; Asset overrides it to Literal["Asset"] (a localized # type: ignore[assignment] covers the narrowed override), and ensure_schemakey pins each instance's value to its class name.

  • Remove the Publishable mixin; add PublishedDandiset/PublishedAsset as deprecated aliases.

  • Simplify ensure_schemakey to a plain schemaKey == class-name check.

  • to_datacite asserts the published precondition (datePublished set) the PublishedDandiset type used to guarantee.

What changed outside dandischema/models.py
  • dandischema/consts.py: DANDI_SCHEMA_VERSION becomes 0.8.0, and "0.7.0" joins ALLOWED_INPUT_SCHEMAS so that metadata stamped with it remains valid input for migrate().

The tools/api-with-schema.Dockerfile change that the test-dandi-cli results below depend on (adding ENV UV_NO_SYNC=1 and dropping the undeclared dev extra) was split out into #435 and merged to master, so it is no longer part of this PR's diff; this branch inherits it after being rebased onto master.

Why gate on datePublished rather than an explicit (context-triggered) validator

The publish-only checks could instead be triggered explicitly by the caller (e.g. a Pydantic validation-context flag passed by the publish flow) rather than by the presence of datePublished. We chose data-driven gating because:

  • It can be expressed in LinkML; an explicit context cannot. Gating on a real field is a conditional constraint on the data, which is exactly what a LinkML rule expresses (gen-json-schema emits it as if/then). A caller-supplied validation mode has no LinkML equivalent — LinkML describes only the data instance, not how validation is invoked — so it would have to live as hand-written Python forever, outside the source of truth, working against the migration this change is meant to enable.
  • It travels with the data. The gated checks fire through both validate() and direct construction (Dandiset(**data) / model_validate). A context flag only rides model_validate(..., context=...): the terse PublishedDandiset(**meta) form (used by to_datacite and in tests) cannot pass one, so it would silently skip the publish checks. Beyond the call-site rewrites that would force, a constructor whose enforcement quietly depends on how it is invoked is a lasting programming hazard.
  • It matches the domain. datePublished present ⟺ published is a real invariant: a draft never carries it, and the archive injects it only when building a publishable version. "Published" is a state of the record, not a mode a caller opts into.
  • Published-ness stays introspectable. With gating, "is this published?" is just obj.datePublished is not None; with a context flag, whether an object was validated as published is ephemeral to the call and recorded nowhere on the object.
Compatibility
  • Generated JSON Schema (verified by before/after diff): draft Dandiset/Asset gain only optional, readOnly publication properties — nothing newly required, no pattern tightened — so the dandi-archive Meditor is unaffected (it hides readOnly props). The published-*.json schemas become the relaxed versions, with publication strictness enforced by the gated validator.
  • published-dandiset.json and published-asset.json are now byte-identical to dandiset.json and asset.json, because publish_model_schemata generates each file from the class named in SCHEMA_MAP and PublishedDandiset/PublishedAsset are now aliases of Dandiset/Asset.
  • The publication requirements no longer appear in the generated JSON Schema at all, since they are conditional on datePublished and a flat required list cannot express that; the gated validator enforces them instead. That is an acceptable gap for now: the plan is to re-encode these requirements as LinkML rules in the LinkML translation, where gen-json-schema emits them as if/then.
  • PublishedDandiset/PublishedAsset/BareAsset imports continue to work in dandi-cli/dandi-archive.

Follow-ups (separate PRs)

Test plan

  • tox -e py3 (262 passed, 17 skipped — skips are environment-only: no DOI_PREFIX / instance name not "DANDI")
  • tox -e lint,typing
  • Generated JSON Schema before/after diff reviewed, and diffed against the published 0.7.0 release in dandi/schema (see the comment linked from the Summary)
  • Added tests for the datePublished-gated publish requirements and the coherence invariant
  • Verified against dandi-archive's own CI via the throwaway PR [DO NOT MERGE] Test CI against dandi-schema#419 consolidated models dandi-archive#2866, which pins dandischema to this branch. After this PR was rebased onto master (following the TESTS: Fix dandi cli testing #435 split), that pin was re-resolved to the current tip (227cdd01) and the run re-triggered; this box will be checked once the refreshed run passes. The earlier run, against the now-dropped pre-split tip, had the archive's publication-validation suite (backend-ci) passing against the consolidated models and both cli-integration variants (consolidated-model server against a released-dandischema client, on dandi-cli master and release) passing.

Expected test-dandi-cli failures

Bumping DANDI_SCHEMA_VERSION ahead of dandi-archive and dandi-cli leaves integration jobs red: three of them today, and two once dandi/dandi-cli#1894 merges, since the server job runs dandi-cli master and that PR drops the incidental requirement that both sides carry the same DANDI_SCHEMA_VERSION. Each is version skew between the two sides of the test rather than a regression from the model changes, and none can be fixed from this PR; the remaining two clear when both services move to this release.

Which jobs fail, and why
job client schema server schema outcome
ubuntu, client, dandi-cli master 0.8.0 0.7.0 fail
ubuntu, client, dandi-cli release 0.8.0 0.7.0 fail
ubuntu, server 0.7.0 0.8.0 fail, until dandi/dandi-cli#1894 merges
ubuntu, both (default and ember-dandi) 0.8.0 0.8.0 pass
macOS and Windows, client 0.8.0 n/a pass, no Docker so the server-backed tests are skipped
  • The two client jobs install this branch into the client only, so a 0.8.0 client meets the released 0.7.0 server. dandi-cli treats a server that is behind by a MINOR schema version as fatal (dandi/dandiapi.py:740-747), so nearly every server-backed test errors with SchemaVersionError: Server uses older incompatible schema version 0.7.0; client supports 0.8.0.
  • The server job is the mirror image: a released client against a 0.8.0 server. Only 5 of 1006 tests fail, all cli/tests/test_service_scripts.py::test_update_dandiset_from_doi, because that test builds its expected schemaVersion and @context from the client's own DANDI_SCHEMA_VERSION (test_service_scripts.py:111-118) and so expects 0.7.0 where the server emits 0.8.0. Build expected metadata with the server's DANDI schema version dandi-cli#1894 fixes that expectation, and since this job runs dandi-cli master, it goes green on a re-run once that PR merges.
  • The both jobs install this branch on both sides and pass, which is the signal that the consolidated models and the new schema version work end to end.

Before the UV_NO_SYNC=1 fix (added in #435, now in master and inherited here via rebase), the both jobs failed exactly like the client jobs, and the server job passed only because the ineffective override left both sides agreeing on 0.7.0. Its red status here is a consequence of the override finally working.

@codecov

codecov Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 37.14286% with 66 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.45%. Comparing base (e0b60a0) to head (227cdd0).

Files with missing lines Patch % Lines
dandischema/models.py 0.00% 64 Missing ⚠️
dandischema/consts.py 0.00% 1 Missing ⚠️
dandischema/datacite/__init__.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #419      +/-   ##
==========================================
- Coverage   47.48%   47.45%   -0.04%     
==========================================
  Files          19       19              
  Lines        2367     2396      +29     
==========================================
+ Hits         1124     1137      +13     
- Misses       1243     1259      +16     
Flag Coverage Δ
unittests 47.45% <37.14%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@candleindark
candleindark force-pushed the consolidate-models branch 2 times, most recently from a15e2e9 to 1298b26 Compare June 9, 2026 19:21
Comment thread dandischema/models.py
@candleindark
candleindark force-pushed the consolidate-models branch 2 times, most recently from 30cf2fd to a08719f Compare July 6, 2026 23:17
candleindark added a commit to candleindark/dandi-cli that referenced this pull request Jul 8, 2026
Temporarily point the dandischema dependency at the consolidate-models
branch (dandi/dandi-schema#419), where BareAsset.schemaKey is
"BareAsset", so CI runs the dandi-cli client against the consolidated
schema and checks that this branch still uploads schemaKey="Asset". For
checking the test outcome only; not intended to be merged.

Co-Authored-By: Claude Code 2.1.202 / Claude Opus 4.8 claude-opus-4-8 <noreply@anthropic.com>
candleindark added a commit to candleindark/dandi-cli that referenced this pull request Jul 8, 2026
… value

`prepare_metadata()` returns a `BareAsset`, whose top-level `schemaKey` is
`"Asset"` with the current dandischema but `"BareAsset"` under
dandi/dandi-schema#419 (which pins `BareAsset.schemaKey` to its class
name). To keep this test passing against either dandischema version, the
expected `schemaKey` is set to the default of the `schemaKey` field in
`BareAsset` before the comparison, and the four `metadata2asset*.json`
files now store a placeholder for the field.

The later `validate()` call checks the data against the `Asset` class, so
`data_as_dict` is promoted to an Asset (`schemaKey = "Asset"`) beforehand.
candleindark added a commit to candleindark/dandi-cli that referenced this pull request Jul 8, 2026
Temporarily point the dandischema dependency at the consolidate-models
branch (dandi/dandi-schema#419), where BareAsset.schemaKey is
"BareAsset", so CI runs the dandi-cli client against the consolidated
schema and checks that this branch still uploads schemaKey="Asset". For
checking the test outcome only; not intended to be merged.

Co-Authored-By: Claude Code 2.1.202 / Claude Opus 4.8 claude-opus-4-8 <noreply@anthropic.com>
candleindark added a commit to candleindark/dandi-cli that referenced this pull request Jul 10, 2026
Temporarily point the dandischema dependency at the consolidate-models
branch (dandi/dandi-schema#419), where BareAsset.schemaKey is
"BareAsset", so CI runs the dandi-cli client against the consolidated
schema and checks that this branch still uploads schemaKey="Asset". For
checking the test outcome only; not intended to be merged.

Co-Authored-By: Claude Code 2.1.202 / Claude Opus 4.8 claude-opus-4-8 <noreply@anthropic.com>
candleindark added a commit to candleindark/dandi-cli that referenced this pull request Jul 10, 2026
… value

`prepare_metadata()` returns a `BareAsset`, whose top-level `schemaKey` is
`"Asset"` with the current dandischema but `"BareAsset"` under
dandi/dandi-schema#419 (which pins `BareAsset.schemaKey` to its class
name). To keep this test passing against either dandischema version, the
expected `schemaKey` is set to the default of the `schemaKey` field in
`BareAsset` before the comparison, and the four `metadata2asset*.json`
files now store a placeholder for the field.

The later `validate()` call checks the data against the `Asset` class, so
`data_as_dict` is promoted to an Asset (`schemaKey = "Asset"`) beforehand.
candleindark added a commit to candleindark/dandi-cli that referenced this pull request Jul 10, 2026
Temporarily point the dandischema dependency at the consolidate-models
branch (dandi/dandi-schema#419), where BareAsset.schemaKey is
"BareAsset", so CI runs the dandi-cli client against the consolidated
schema and checks that this branch still uploads schemaKey="Asset". For
checking the test outcome only; not intended to be merged.

Co-Authored-By: Claude Code 2.1.202 / Claude Opus 4.8 claude-opus-4-8 <noreply@anthropic.com>
@candleindark
candleindark force-pushed the consolidate-models branch 3 times, most recently from 1e81506 to 18379cf Compare July 13, 2026 08:00
@candleindark
candleindark marked this pull request as ready for review July 13, 2026 17:21
@kabilar
kabilar requested review from jjnesbitt and waxlamp July 13, 2026 19:01
@candleindark
candleindark force-pushed the consolidate-models branch 2 times, most recently from bff378f to 60cc4de Compare July 16, 2026 21:23
@yarikoptic yarikoptic added the minor Increment the minor version when merged label Jul 20, 2026
@yarikoptic

Copy link
Copy Markdown
Member

note to myself,
image
is just tests of dandi-cli whenever https://github.com/dandi/dandi-archive/pull/2866/changes is of the server itself

@yarikoptic

Copy link
Copy Markdown
Member

@satra we are thinking to proceed with this transition on how we test for publisher vs draft dandiset or asset, so that we do not breed the models and rather add "rules" (which could be expressed in linkml as well). Side-effect -- jsonschema validation will be more relaxed for "PublishedDandiset" because they do not translate. After we migrate to linkml with rules -- they will be translated into jsonschema. But @candleindark that linkml rules do not translate at all or as fully to pydantic for which @candleindark yet to find or file an issue.

What do you @satra think about this transition?

@candleindark
candleindark requested a review from satra July 22, 2026 21:44
@candleindark

Copy link
Copy Markdown
Member Author

Yes, let's bump DANDI_SCHEMA_VERSION to 0.8.0. I generated the four schemas the way .github/workflows/release.yml does and diffed them against the published 0.7.0 in dandi/schema. Two independent reasons for the bump, and to your other question: nothing in this PR is breaking for existing metadata.

1. The bump is already due, independently of this PR. master alone already generates schemas that differ from the published 0.7.0: sameAs on Dandiset (#364, #370) and readOnly on Asset.access (#414). Since 0.7.0 is already published, the release workflow's "Test for unversioned changes" step fails today with DANDI_SCHEMA_VERSION left at 0.7.0, with or without #419.

2. #419 then changes all four files further, so reusing 0.7.0 would make one version string mean three different things.

Not breaking. Every required change is a removal, every added property is optional and readOnly, and both pattern changes are relaxations. No stored Dandiset or Asset metadata becomes invalid, and the publication rules that leave the JSON Schema are re-enforced in Python by the datePublished-gated validator, which metadata.validate() always runs after the JSON Schema pass. A minor bump is the right size: migrate() has no version-specific transformation past 0.6.0, so 0.7.0 -> 0.8.0 only rewrites schemaVersion.

Summary of the diff, published 0.7.0 -> this branch

Draft dandiset.json and asset.json gain schema text only, and required is unchanged in both:

file change kind
dandiset.json + doi, + publishedBy, + datePublished, + releaseNotes optional, readOnly
dandiset.json + $defs.PublishActivity pulled in by publishedBy, its only $ref
dandiset.json url.description reworded doc only
asset.json + publishedBy, + datePublished optional, readOnly
asset.json + $defs.PublishActivity as above

Since all of these are readOnly, the Meditor (which hides readOnly properties) is unaffected.

published-dandiset.json and published-asset.json become byte-identical to their draft counterparts:

file change effect
published-dandiset.json - required: datePublished, publishedBy, url relaxation
published-dandiset.json id.pattern widened to accept draft ids and lowercase instance prefixes relaxation
published-dandiset.json doi.default "" -> null; datePublished/publishedBy/url gain default: null annotation only, default is not a constraint
published-dandiset.json title "Published Dandiset" -> "Dandiset", + description cosmetic
published-asset.json - required: datePublished, publishedBy relaxation
published-asset.json id.pattern (^dandiasset:...$) dropped relaxation
published-asset.json title "Published Asset" -> "Asset", + description cosmetic

context.json drops the PublishedAsset and PublishedDandiset term mappings, since generate_context() walks the model classes and those two names are now aliases. No stored metadata carries those schemaKey values, so this is inert in practice, but it is a vocabulary removal and another reason not to reuse 0.7.0.

Also present in the diff but coming from master, not from this PR: + sameAs on both Dandiset schemas, access.readOnly on both Asset schemas, and a reordering of the LicenseType enum, which is the non-determinism tracked by #433 rather than a real change.

The one thing worth flagging: published-*.json stops expressing publication requirements

This is the change most likely to be read as breaking, so to be explicit about what it does and does not cost.

Enforcement is not lost through dandischema. validate() always runs Pydantic validation after the optional JSON Schema pass, and PublishedDandiset/PublishedAsset are aliases of Dandiset/Asset, whose check_publication_status re-checks every dropped rule once datePublished is set: publishedBy presence, url presence and pattern, doi presence, the id patterns, assetsSummary file and byte counts, and the sha2-256 digest check. dandi-archive's publish-time call sites pass schema_key='PublishedDandiset'/'PublishedAsset', so they keep the full check. Worth knowing too: validate(..., json_validation=True) uses the locally generated schema whenever schema_version == DANDI_SCHEMA_VERSION, and reads dandi/schema only for metadata stamped with an older version, fetching releases/<that version>/. Publishing 0.8.0 only adds releases/0.8.0/ and leaves every existing release directory alone, 0.7.0 included, so a record stamped 0.7.0 still gets validated against the strict releases/0.7.0/published-dandiset.json. The relaxation applies to 0.8.0-stamped metadata onward, not retroactively.

What is lost is enforcement for anyone consuming published-*.json standalone, outside dandischema: from 0.8.0 on, those files say nothing about publication. That follows from the constraints now being conditional on datePublished, which a flat required list cannot express. This is an acceptable gap for now, since the plan is to re-encode these requirements as LinkML rules in the LinkML translation, where gen-json-schema emits them as if/then and the published artifact gets the constraint back.

One direction in which the draft schemas get slightly stricter

"Additive" means added schema text, which is not quite the same as a pure relaxation. Neither asset schema sets additionalProperties at all (both Dandiset schemas set it to true), so unknown properties have always been accepted and never validated. Giving publishedBy and datePublished a declared subschema therefore type-checks documents that already carried those names:

document published 0.7.0 asset.json this branch
minimal valid asset valid valid
+ "datePublished": 12345 valid invalid, 12345 is not of type 'string'
+ "publishedBy": 12345 valid invalid, not valid under any of the given schemas
+ "someRandomKey": 12345 valid valid

Nothing is at risk in practice: draft records do not carry these fields, and published records carry values that already had to satisfy the identical subschemas in published-*.json. Noting it for completeness, since it is the only place where this PR makes a schema stricter.

candleindark added a commit that referenced this pull request Jul 27, 2026
The schemas generated from `master` already differ from the published
`0.7.0` in `dandi/schema`, and this branch changes all four of them
further, so the current version cannot be reused. Add `"0.7.0"` to
`ALLOWED_INPUT_SCHEMAS` so that metadata stamped with it stays valid
input for `migrate()`.

For the full diff against the published `0.7.0` and the reasoning behind
the bump, see
#419 (comment)

Co-Authored-By: Claude Code 2.1.220 / Claude Opus 5 <noreply@anthropic.com>
@yarikoptic

yarikoptic commented Jul 27, 2026

Copy link
Copy Markdown
Member

dandi-cli fails due to "expected"

dandi.exceptions.SchemaVersionError: Server uses older incompatible schema version 0.7.0; client supports 0.8.0.

I now - re-wondering if "both" in our matrix should mean that server should have got it updated too, and thus potentially it should have all being peachy @candleindark ?

edit, e.g. we do have one dandi-cli on linux (where we can test using docker) passing

image

@candleindark

Copy link
Copy Markdown
Member Author

dandi-cli fails due to "expected"

The failures are expected due the way the tests are set up. However, the set up of those tests are buggy.

I now - re-wondering if "both" in our matrix should mean that server should have got it updated too, and thus potentially it should have all being peachy @candleindark ?

Particularly, "both", and "server", failed to set the run environment of the dandi-archive to use this version of dandi-schema because uv re-syncs the run environment using uv.lock

@yarikoptic

Copy link
Copy Markdown
Member

I see, so we kinda broke our testing setup when jump over to use uv.lock was introduced. Do you see how to address it easily/quickly?

@candleindark

Copy link
Copy Markdown
Member Author

I see, so we kinda broke our testing setup when jump over to use uv.lock was introduced. Do you see how to address it easily/quickly?

Yes. I will push it out and see how it works.

# above. That sync must be skipped for this image to run the schema version
# under test; skipping it is safe because the base image's `uv sync` has already
# installed every other dependency.
ENV UV_NO_SYNC=1

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yarikoptic This eliminate two CI test failures.

@candleindark

Copy link
Copy Markdown
Member Author

@yarikoptic I eliminated two test failures in #419 (comment). The remaining three test failures, not including the codecov ones, can't be eliminate in this PR and are expected (see "Expected test-dandi-cli failures" in PR description for details). After dandi/dandi-cli#1894 is merged, one more CI failures can be eliminated.

This PR is good to merge. However, I want you to take a second look, particular at the changes I put in after your last approval.

Note: The PR description has been updated with the latest changes.

@candleindark
candleindark requested a review from yarikoptic July 28, 2026 01:11
@candleindark
candleindark dismissed yarikoptic’s stale review July 28, 2026 17:40

Version bump has been added. Fix regarding the failed test has been put it. There are remaining an expected failures from the CI.

Re-review is warranted.

@yarikoptic

Copy link
Copy Markdown
Member

oh -- what to add/do in https://github.com/dandi/dandi-schema/blob/master/dandischema/metadata.py#L303 migrate so we could upgrade or downgrade for the changes in this PR? (or may be forgotten for prior PRs for this schema version boost)

@candleindark

Copy link
Copy Markdown
Member Author

@yarikoptic Nothing needs to be added to migrate, as far as I can tell, neither for this PR nor for the earlier PRs that this version bump carries.

This PR neither adds nor removes a restriction on existing metadata. A draft record carries no doi / publishedBy / datePublished / releaseNotes, which is exactly what the draft branch of the gated validator requires; a published record carries all of them and satisfies the same rules the Published* classes enforced. A valid 0.7.0 instance is therefore already a valid 0.8.0 instance, and the only thing migrate has to do is restamp schemaVersion, which it already does unconditionally.

The earlier PRs contribute exactly three deltas between the published 0.7.0 and master, and none of them needs migration either:

change why no migration step
sameAs added to Dandiset (#364, #370) a new optional property, not in required, so its absence from older metadata is valid
access.readOnly: true on Asset (#414) readOnly is a JSON Schema annotation rather than a constraint, and it affects no instance. migrate handles Dandiset metadata only, so this is outside its scope regardless
LicenseType enum members reordered the same member set in a different order, an artifact of #433 rather than a schema change
Checked rather than only reasoned about: 0.7.00.8.0 round trip

For both a draft and a published Dandiset, the script below asserts the instance is valid under the published 0.7.0 schema in dandi/schema, runs migrate(), reports every field the migration changed, and then validates the result against this branch's models and JSON Schema:

[draft] valid against published 0.7.0 dandiset.json
[draft] migrate() -> schemaVersion=0.8.0
[draft] fields changed by migrate(): ['schemaVersion']
[draft] validate() against 0.8.0 models + JSON Schema: OK

[published] valid against published 0.7.0 published-dandiset.json
[published] migrate() -> schemaVersion=0.8.0
[published] fields changed by migrate(): ['schemaVersion']
[published] validate() against 0.8.0 models + JSON Schema: OK
"""Round-trip real 0.7.0-shaped Dandiset metadata through this branch's migrate()."""
import json
from copy import deepcopy
from pathlib import Path

import jsonschema

from dandischema.consts import DANDI_SCHEMA_VERSION
from dandischema.metadata import migrate, validate

PUB = Path("<clone of dandi/schema>/releases/0.7.0")
INSTANCE = "DANDI"

draft = {
    "schemaKey": "Dandiset",
    "schemaVersion": "0.7.0",
    "identifier": f"{INSTANCE}:999999",
    "id": f"{INSTANCE}:999999/draft",
    "version": "draft",
    "name": "testing dataset",
    "description": "testing",
    "contributor": [
        {
            "name": "last name, first name",
            "email": "someone@dandiarchive.org",
            "roleName": ["dcite:ContactPerson"],
            "schemaKey": "Person",
        }
    ],
    "license": ["spdx:CC-BY-4.0"],
    "citation": "Last, first (2021). Test citation.",
    "assetsSummary": {
        "schemaKey": "AssetsSummary",
        "numberOfBytes": 0,
        "numberOfFiles": 0,
        "dataStandard": [{"schemaKey": "StandardsType", "name": "NWB"}],
        "approach": [{"schemaKey": "ApproachType", "name": "electrophysiology"}],
        "measurementTechnique": [
            {"schemaKey": "MeasurementTechniqueType",
             "name": "two-photon microscopy technique"}
        ],
        "species": [{"schemaKey": "SpeciesType", "name": "Human"}],
    },
    "manifestLocation": [
        "https://api.dandiarchive.org/api/dandisets/999999/versions/draft/assets/"
    ],
    "url": "https://dandiarchive.org/dandiset/999999/draft",
}

published = deepcopy(draft)
published.update(
    {
        "id": f"{INSTANCE}:999999/0.0.0",
        "version": "0.0.0",
        "url": "https://dandiarchive.org/dandiset/999999/0.0.0",
        "doi": "10.80507/dandi.999999/0.0.0",
        "datePublished": "2021-01-01T00:00:00+00:00",
        "publishedBy": {
            "schemaKey": "PublishActivity",
            "id": "urn:uuid:08fffc59-9f1b-44d6-8e02-6729d266d1b6",
            "name": "Publish",
            "startDate": "2021-01-01T00:00:00+00:00",
            "endDate": "2021-01-01T00:00:00+00:00",
            "wasAssociatedWith": [
                {
                    "schemaKey": "Software",
                    "id": "urn:uuid:9267d2e1-4a37-463b-a2b6-3d1c1cd39c8f",
                    "identifier": "RRID:SCR_017571",
                    "name": "DANDI API",
                    "version": "0.1.0",
                }
            ],
        },
    }
)
published["assetsSummary"].update({"numberOfBytes": 1, "numberOfFiles": 1})

for label, obj, pub_file in [
    ("draft", draft, "dandiset.json"),
    ("published", published, "published-dandiset.json"),
]:
    # 1. Is it genuinely valid under the *published* 0.7.0 JSON Schema?
    schema = json.loads((PUB / pub_file).read_text())
    jsonschema.validate(obj, schema)
    print(f"[{label}] valid against published 0.7.0 {pub_file}")

    # 2. migrate() to this branch's version
    out = migrate(obj, skip_validation=True)
    print(f"[{label}] migrate() -> schemaVersion={out['schemaVersion']}")

    # 3. Does the migrated object differ from the input in anything but schemaVersion?
    delta = {k: (obj.get(k), out.get(k)) for k in set(obj) | set(out) if obj.get(k) != out.get(k)}
    print(f"[{label}] fields changed by migrate(): {sorted(delta)}")

    # 4. Does it validate under this branch (models + local JSON Schema)?
    validate(out, schema_key="Dandiset", json_validation=True)
    print(f"[{label}] validate() against {DANDI_SCHEMA_VERSION} models + JSON Schema: OK\n")

The one change migrate did need is already in the PR: "0.7.0" is added to ALLOWED_INPUT_SCHEMAS, without which migrate would reject the very metadata that is out in the wild today.

Downgrade is a separate matter. migrate rejects any target below the instance's own version, and ALLOWED_TARGET_SCHEMAS holds only DANDI_SCHEMA_VERSION, so there is no downgrade path to extend. Adding one is #343.

@candleindark

Copy link
Copy Markdown
Member Author

@yarikoptic Preferably, let's merge #434 before this PR is merged and released so that we don't have to adjust the DANDI_SCHEMA_VERSION multiple times.

candleindark and others added 3 commits August 3, 2026 09:57
…Published

Collapse the publication-specific model variants into their base classes so
that each class's schemaKey value matches its class name, which is what an
eventual LinkML translation needs to use schemaKey as a type designator
(designates_type). The three classes whose schemaKey differed from their class
name (BareAsset -> "Asset", PublishedAsset -> "Asset", PublishedDandiset ->
"Dandiset") were the blockers.

Changes (all in dandischema/models.py):

- Merge PublishedDandiset into Dandiset and PublishedAsset into Asset. The
  publication-only fields (doi, publishedBy, datePublished, releaseNotes on
  Dandiset; publishedBy, datePublished on Asset) become optional, and the
  publication requirements move into a datePublished-gated
  `check_publication_status` model validator on each class:
    - when datePublished is None (a draft), the publication-only fields must be
      absent;
    - when datePublished is set (published), enforce the former Published*
      requirements (publishedBy/url/doi presence, the stricter id/url patterns,
      check_filesbytes, digest_sha256check). All violations are reported
      together in one error.
  dandi-archive's publish flow injects datePublished before validating, so the
  gated checks fire exactly as the Published* classes did before.

- Keep BareAsset as a distinct class (Asset still inherits from it) but align
  its schemaKey to Literal["BareAsset"], so both BareAsset and Asset are
  schemaKey-aligned. The client (dandi-cli) is responsible for setting
  schemaKey to "Asset" when uploading bare metadata as an Asset.

- Remove the Publishable mixin; add PublishedDandiset/PublishedAsset as
  deprecated aliases of Dandiset/Asset for backward compatibility (dandi-cli,
  dandi-archive). These will be removed in a follow-up once consumers migrate.

- Simplify DandiBaseModel.ensure_schemakey to a plain schemaKey == class-name
  check now that no class intentionally diverges.

to_datacite now asserts the published precondition (datePublished set) that the
PublishedDandiset type used to guarantee. Tests updated for the merged models:
the published variants report the same missing fields as their base classes,
and the publication requirements are exercised on complete, datePublished
instances; new tests cover the publication-coherence invariant.

Generated JSON Schema diff: the draft schemas (Dandiset, Asset) gain only
optional, readOnly publication properties (nothing newly required, no pattern
tightened), so the dandi-archive Meditor is unaffected; the published-*.json
schemas become the relaxed versions, with publication strictness now enforced
by the gated Pydantic validator.

Co-Authored-By: Claude Code 2.1.161 / Claude Opus 4.8 claude-opus-4-8 <noreply@anthropic.com>

# Conflicts:
#	dandischema/tests/test_models.py
The `check_publication_status` model-validators on `Dandiset`/`Asset`
previously raised a single combined `ValueError`, which pydantic reports with
an empty `loc` (`()`), losing the per-field association and tripping consumers
that index `error["loc"][0]` (e.g. dandi-archive's `_encode_pydantic_error`).

Build a `list[InitErrorDetails]` instead — one entry per failing check, each
with its own `loc=(field,)`, `type="value_error"`, and the message carried in
`ctx={"error": ...}` — and raise it via
`pydantic_core.ValidationError.from_exception_data(...)`. This keeps the single
accumulating model-validator design (all violations reported together) while
giving every publication error a proper field `loc`, and preserves the exact
"Value error, <message>" text the former field-validators produced.

test_dandimeta_1 now asserts the per-field locs rather than one combined
message.

Co-Authored-By: Claude Code 2.1.198 / Claude Opus 4.8 claude-opus-4-8 <noreply@anthropic.com>
The schemas generated from `master` already differ from the published
`0.7.0` in `dandi/schema`, and this branch changes all four of them
further, so the current version cannot be reused. Add `"0.7.0"` to
`ALLOWED_INPUT_SCHEMAS` so that metadata stamped with it stays valid
input for `migrate()`.

For the full diff against the published `0.7.0` and the reasoning behind
the bump, see
#419 (comment)

Co-Authored-By: Claude Code 2.1.220 / Claude Opus 5 <noreply@anthropic.com>
@candleindark

Copy link
Copy Markdown
Member Author

Updated the PR description to reflect that the two tools/api-with-schema.Dockerfile CI changes (adding ENV UV_NO_SYNC=1 and dropping the undeclared dev extra) were split out into #435 and merged to master. They are no longer part of this PR's diff. This branch has been rebased onto the post-#435 master and inherits that Dockerfile fix from there, so the "Expected test-dandi-cli failures" explanation still holds.

@yarikoptic
yarikoptic merged commit 5a46453 into master Aug 3, 2026
79 of 83 checks passed
@yarikoptic
yarikoptic deleted the consolidate-models branch August 3, 2026 19:50
candleindark added a commit that referenced this pull request Aug 7, 2026
The notes are no longer necessary since #419.

Also handle in `find_schemakey_mismatches.py` the re-exposure of a class under
an alias.

Addresses the `MANUAL_NOTE` sub-task of #389.

Co-Authored-By: Claude Code 2.1.224 / Claude Opus 5 <noreply@anthropic.com>
candleindark added a commit that referenced this pull request Aug 7, 2026
`models.yaml` no longer defines them since #419 consolidated them into
`Dandiset` and `Asset`, so `gen-json-schema` emitted rootless schemas that
accept any object. The filename `sed` existed only for those two targets and
goes with them.

Co-Authored-By: Claude Code 2.1.224 / Claude Opus 5 <noreply@anthropic.com>
candleindark added a commit that referenced this pull request Aug 7, 2026
`publish_model_schemata` resolves `SCHEMA_MAP` keys with `getattr`, so the stub
needs the aliases #419 kept in `dandischema/models.py`. Temporary: remove them
after the #419 follow-up that drops those aliases.

Co-Authored-By: Claude Code 2.1.224 / Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor Increment the minor version when merged release Create a release when this pr is merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants