Skip to content

fix(core): resolve bare $refs before dropping schema definitions - #164

Open
LukasGold wants to merge 1 commit into
mainfrom
fix/register-schema-nested-refs
Open

fix(core): resolve bare $refs before dropping schema definitions#164
LukasGold wants to merge 1 commit into
mainfrom
fix/register-schema-nested-refs

Conversation

@LukasGold

Copy link
Copy Markdown
Contributor

Closes #101

Problem

OSW.register_schema rewrote only the $refs reachable via the $..allOf jsonpath, then deleted schema["definitions"] unconditionally.

pydantic v1 wraps a nested-model $ref in allOf only when the field carries extra metadata, e.g. Field(description=...). A plain nested field:

class Outer(OswBaseModel):
    inner: Inner          # no Field(...) metadata

emits a bare {"$ref": "#/definitions/Inner"} on the property. Those refs were never rewritten, so deleting definitions left them dangling.

Changes

  • Added a second pass over $..dollarref in register_schema that embeds the local #/definitions/... body in place, mirroring what the existing allOf loop does for wrapped refs.
  • del schema["definitions"] now runs only when every local bare ref resolved, so an unresolvable ref keeps definitions rather than producing a dangling ref.
  • The existing allOf loop is untouched; this is a strict superset of the previous behaviour.

Tests

tests/test_register_schema_nested_refs.py, two cases:

  • bare $ref (no Field(...) metadata) is resolved like an allOf-wrapped one
  • allOf-wrapped ref behaviour is unchanged

Verified the first test fails on unpatched code (AssertionError: assert not True from the dangling-ref check) and passes after the fix. Full unit suite: 170 passed, 1 skipped.

Note

Refs nested more than one level deep (a definition that itself holds a bare ref) are resolved in definitions but the already-embedded copy is not re-walked, since jsonpath.find collects matches before mutation. The pre-existing allOf loop has the same property, so this PR keeps the behaviour rather than widening scope. Worth a follow-up if such schemas occur in practice.

- register_schema only rewrote refs reachable via the $..allOf jsonpath
- a nested model field without Field() metadata emits a bare $ref
- those were left dangling once definitions was deleted
- keep definitions when a local ref cannot be resolved
- closes #101
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Release preview

Merging this PR would release v2.2.2 (current: v2.2.1).

Changelog preview (truncated)
## v2.2.2 (2026-09-03)

### Bug Fixes

- **core**: Resolve bare $refs before dropping schema definitions
  ([`9c2ad6e`](https://github.com/OpenSemanticLab/osw-python/commit/9c2ad6e89c31e9b683c039b63eb6472429cbbd76))

Preview via python-semantic-release and conventional commits.

@LukasGold LukasGold self-assigned this Sep 3, 2026
@LukasGold
LukasGold requested a review from SimonStier September 3, 2026 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enh: osw.core.OSW.register_model should be able to handle dataclasses typing attributes with other dataclasses

1 participant