diff --git a/README.md b/README.md index d042b64..3c344d9 100644 --- a/README.md +++ b/README.md @@ -50,12 +50,14 @@ Note: **PyPI package names use hyphens** (`datasworn-community-classic`) but **P ## Status -Everything is functional, but there are known gaps to reconcile before the first PyPI release: +Functional against Pydantic 2.13, all 10 packages tested. Two known codegen bugs from `datamodel-code-generator` are patched by `scripts/post_process_models.py` — run it after any `models.py` regeneration: -- **Pydantic pinned `<2.13`.** Pydantic 2.13 tightened validation in two ways that the generated `models.py` isn't yet reconciled with: - 1. `date` fields carry a string `pattern` constraint that 2.13 rejects (fix stripped from `models.py` for now — needs a proper generator fix so it doesn't come back on regeneration). - 2. Delve's `site_domains.*.features` / `.dangers` are typed as models but the compiled JSON emits them as lists. Delve tests are marked `xfail` until this is resolved. -- **`models.py` has not been post-processed** to convert `RootModel[str]` wrappers into plain type aliases. That's the ergonomic (`.id` vs `._id`) fix that was in a separate script on the upstream fork. +1. **String `pattern` constraint on `date` fields.** `SourceInfo.date` is typed as `datetime.date` but the schema's `pattern: "[0-9]{4}-…"` is emitted onto the `Field()`. Pydantic 2.13 rejects a string-only constraint on a non-string field. The post-processor strips just those patterns. +2. **Empty `Features` / `Dangers` / `Denizens` stubs.** Codegen emits `class Denizens(BaseModel): pass` and `class Features(BaseModel): pass`, then references them where the JSON actually contains a list of concrete items (`DelveSiteDenizen`, `DelveSiteDomainFeature`, `DelveSiteThemeFeature`, etc.). The post-processor rewrites the field types to `list[X]` and deletes the empty stubs. + +Still outstanding (nice-to-have, not blocking): + +- **`RootModel[str]` wrappers on ID types.** `.id` currently reads via `._id`; a separate post-process step in the upstream fork converted these to plain type aliases (`RulesetId: TypeAlias = str`). Not ported yet. ## Development diff --git a/packages/core/pyproject.toml b/packages/core/pyproject.toml index de10fd5..f3a46b0 100644 --- a/packages/core/pyproject.toml +++ b/packages/core/pyproject.toml @@ -8,11 +8,11 @@ authors = [ ] requires-python = ">=3.14" dependencies = [ - # Pin <2.13 until the generated models.py is reconciled with 2.13's stricter - # validation: (a) `date` fields carry a string `pattern` constraint that 2.13 - # rejects, and (b) several fields (e.g. delve site_domains.*.features) are - # typed as models but the compiled JSON emits them as lists. Bumping pydantic - # again should coincide with regenerating and post-processing models.py. + # 2.13 works because `scripts/post_process_models.py` rewrites two codegen + # bugs that 2.13 catches: string `pattern` constraints on `date` fields, and + # empty stub models where the generator should have emitted `list[X]`. + # Bumping past 2.13 requires re-running the post-processor after any + # models.py regeneration (see the script's docstring for details). "pydantic[email]>=2.13.4,<2.14", ] diff --git a/packages/core/src/datasworn/core/models.py b/packages/core/src/datasworn/core/models.py index 35ee4e2..319345d 100644 --- a/packages/core/src/datasworn/core/models.py +++ b/packages/core/src/datasworn/core/models.py @@ -376,9 +376,6 @@ class CoreTags(BaseModel): ] = None -class Denizens(BaseModel): - pass - class DelveSiteDenizenFrequency(Enum): very_common = 'very_common' @@ -410,12 +407,6 @@ class DelveSiteDenizenIdWildcard(RootModel[str]): ] -class Features(BaseModel): - pass - - -class Dangers(BaseModel): - pass class DelveSiteDomainDangerId(RootModel[str]): @@ -3444,12 +3435,7 @@ class DelveSite(BaseModel): title='MarkdownString', ), ] - denizens: Annotated[ - Denizens, - Field( - description="Represents the delve site's denizen matrix as an array of objects." - ), - ] + denizens: list[DelveSiteDenizen] type: Literal['delve_site'] @@ -3572,8 +3558,8 @@ class DelveSiteDomain(BaseModel): title='OracleRollableId', ), ] = None - features: Features - dangers: Dangers + features: list[DelveSiteDomainFeature] + dangers: list[DelveSiteDomainDanger] type: Literal['delve_site_domain'] @@ -3874,8 +3860,8 @@ class DelveSiteTheme(BaseModel): description='Optional extended description text.', title='MarkdownString' ), ] = None - features: Features - dangers: Dangers + features: list[DelveSiteThemeFeature] + dangers: list[DelveSiteThemeDanger] type: Literal['delve_site_theme'] diff --git a/scripts/post_process_models.py b/scripts/post_process_models.py new file mode 100644 index 0000000..e7cb0e9 --- /dev/null +++ b/scripts/post_process_models.py @@ -0,0 +1,196 @@ +#!/usr/bin/env python3 +"""Post-process the generated `models.py` to work around two codegen bugs. + +Datamodel-code-generator (the upstream generator that builds `models.py` from +`datasworn-source.schema.json`) has two known misbehaviors on our schema: + +1. **String `pattern` on `date` fields.** The schema declares + `SourceInfo.date` with `type: string, format: date, pattern: "[0-9]{4}-…"`. + Datamodel-code-generator preserves the pattern in the generated `Field()` + even though it also (correctly) picks `datetime.date` as the Python type. + Pydantic 2.13+ then rejects the schema at import time: a string-only + `pattern` constraint on a non-string field is a Pydantic error. + +2. **Empty `Features` / `Dangers` stubs on `DelveSiteDomain` / `DelveSiteTheme`.** + The schema types these as arrays of `DelveSiteDomainFeature` / + `DelveSiteThemeFeature` (etc.), but datamodel-code-generator emits `class + Features(BaseModel): pass` and then references it as `features: Features`, + dropping the list-of-item shape entirely. Validating a real JSON payload + (which has an array of feature objects) then fails with `model_type` errors. + +Both fixes belong in a proper generator patch upstream — until then, this +script runs against a freshly generated `models.py` and rewrites those two +patterns in place. Idempotent: running twice is a no-op on the second run. + +Usage + uv run scripts/post_process_models.py [path/to/models.py] + +Default path targets our shipped models under `packages/core/src/`. +""" + +from __future__ import annotations + +import re +import sys +from pathlib import Path + +DEFAULT_MODELS_PATH = ( + Path(__file__).resolve().parents[1] + / "packages/core/src/datasworn/core/models.py" +) + +# --- Fix 1: strip `pattern="[0-9]{4}-..."` from Field() blocks on `date_aliased` - + +# Matches: +# date: Annotated[ +# date_aliased, +# Field( +# description="...", +# pattern='[0-9]{4}-((0[0-9])|(1[0-2]))-(([0-2][0-9])|(3[0-1]))', +# ), +# ] +# +# and strips just the `pattern=...` line. Constrained to blocks that are +# already `date_aliased` so we don't accidentally strip patterns from string +# fields nearby. +_DATE_PATTERN_STRIP = re.compile( + r"(date_aliased,\s*\n\s*Field\(\n(?:.*\n)*?)" + r"(\s*pattern='\[0-9\]\{4\}[^']*',\s*\n)", + re.MULTILINE, +) + + +def _strip_date_patterns(source: str) -> tuple[str, int]: + new_source, hits = _DATE_PATTERN_STRIP.subn(r"\1", source) + return new_source, hits + + +# --- Fix 2: rewrite empty Features / Dangers stubs + their usages ----------- + +# Matches: `class (BaseModel):\n pass\n` for the stubs we replace. +_EMPTY_STUB_RE = re.compile( + r"^class (Features|Dangers|Denizens)\(BaseModel\):\n pass\n\n?", + re.MULTILINE, +) + + +# Maps (owning class name, attribute name) to the concrete list element type. +_STUB_REPLACEMENTS: dict[tuple[str, str], tuple[str, str]] = { + # (owning class, attr): (expected stub name, replacement type) + ("DelveSiteDomain", "features"): ("Features", "list[DelveSiteDomainFeature]"), + ("DelveSiteDomain", "dangers"): ("Dangers", "list[DelveSiteDomainDanger]"), + ("DelveSiteTheme", "features"): ("Features", "list[DelveSiteThemeFeature]"), + ("DelveSiteTheme", "dangers"): ("Dangers", "list[DelveSiteThemeDanger]"), + ("DelveSite", "denizens"): ("Denizens", "list[DelveSiteDenizen]"), +} + + +# Matches the field declaration at the start of an `attr: Annotated[Stub, ...]` +# block. Consumes lines up through the closing `]` of Annotated to know how +# many lines the field spans, so we can splice a bare replacement in its place. +_ANNOTATED_STUB_HEAD = re.compile( + r"^ (\w+): Annotated\[\s*\n" + r" (\w+),\s*\n", +) + + +def _rewrite_stub_fields(source: str) -> tuple[str, int]: + """Walk each `class …(BaseModel):` block and rewrite fields that reference + the empty stub types with the appropriate concrete list types. + + Handles both: + features: Features # bare + denizens: Annotated[Denizens, Field(...)] # wrapped in Annotated + """ + lines = source.splitlines(keepends=True) + current_class: str | None = None + hits = 0 + + i = 0 + while i < len(lines): + line = lines[i] + + class_match = re.match(r"^class (\w+)\(", line) + if class_match: + current_class = class_match.group(1) + i += 1 + continue + + if current_class is None: + i += 1 + continue + + # 1. Bare form: ` attr: Stub\n` + bare_match = re.match(r"^ (\w+): (\w+)\s*$", line) + if bare_match: + attr, stub_name = bare_match.group(1), bare_match.group(2) + spec = _STUB_REPLACEMENTS.get((current_class, attr)) + if spec is not None and spec[0] == stub_name: + lines[i] = f" {attr}: {spec[1]}\n" + hits += 1 + i += 1 + continue + + # 2. Annotated form: ` attr: Annotated[\n Stub,\n Field(...)...\n ]\n` + annotated_match = _ANNOTATED_STUB_HEAD.match( + "".join(lines[i:i + 2]) if i + 1 < len(lines) else line + ) + if annotated_match: + attr, stub_name = annotated_match.group(1), annotated_match.group(2) + spec = _STUB_REPLACEMENTS.get((current_class, attr)) + if spec is not None and spec[0] == stub_name: + # Find the matching closing ` ]\n` — Annotated blocks are + # rendered with 4-space indent for the closing bracket in + # datamodel-code-generator's output. + j = i + 1 + while j < len(lines) and lines[j].rstrip("\n") != " ]": + j += 1 + if j < len(lines): + lines[i:j + 1] = [f" {attr}: {spec[1]}\n"] + hits += 1 + # Don't advance i; the new line at position i is already + # our replacement, and re-checking it as a class header + # would be a no-op. + continue + i += 1 + + new_source = "".join(lines) + + # Then delete the now-orphaned stub classes. + new_source, stub_hits = _EMPTY_STUB_RE.subn("", new_source) + + return new_source, hits + stub_hits + + +# --- driver ----------------------------------------------------------------- + + +def main(argv: list[str]) -> int: + if len(argv) > 1: + path = Path(argv[1]).resolve() + else: + path = DEFAULT_MODELS_PATH + + if not path.exists(): + print(f"models.py not found: {path}", file=sys.stderr) + return 1 + + original = path.read_text(encoding="utf-8") + + after_date, date_hits = _strip_date_patterns(original) + after_features, feature_hits = _rewrite_stub_fields(after_date) + + if after_features == original: + print(f"{path}: no changes (already post-processed)") + return 0 + + path.write_text(after_features, encoding="utf-8") + print( + f"{path}: stripped {date_hits} date-field pattern(s), " + f"rewrote {feature_hits} Features/Dangers stub site(s)" + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv)) diff --git a/tests/test_load_rules_packages.py b/tests/test_load_rules_packages.py index bdb4b1d..542bea3 100644 --- a/tests/test_load_rules_packages.py +++ b/tests/test_load_rules_packages.py @@ -26,14 +26,7 @@ "starsmith", ] -# TODO: reconcile these against pydantic 2.13's stricter validation before -# unpinning. See PROVENANCE.md / package.py notes for details. -EXPECTED_FAILURES: set[str] = { - # delve: site_domains.*.features / .dangers are typed as models in the - # generated models.py but the compiled JSON emits them as lists — schema - # or generator bug we haven't tracked down yet. - "delve", -} +EXPECTED_FAILURES: set[str] = set() def _load(namespace: str, package_name: str) -> Ruleset | Expansion: @@ -61,10 +54,8 @@ def _assert_shape(rules: Ruleset | Expansion, package_name: str) -> None: @pytest.mark.parametrize("package_name", OFFICIAL_PACKAGES) def test_official(package_name: str): - if package_name in EXPECTED_FAILURES: - pytest.xfail( - f"{package_name}: known validation drift vs. generated models" - ) + if package_name in EXPECTED_FAILURES: # pragma: no cover — kept for future drift + pytest.xfail(f"{package_name}: known validation drift vs. generated models") _assert_shape(_load("datasworn", package_name), package_name)