Skip to content

feat: skip ninja-style ModelSchema classes instead of drawing them empty - #179

Merged
FabianClemenz merged 1 commit into
mainfrom
feat/warn-on-model-derived-schemas
Sep 14, 2026
Merged

FabianClemenz merged 1 commit into
mainfrom
feat/warn-on-model-derived-schemas

Conversation

@FabianClemenz

Copy link
Copy Markdown
Member

Summary

Part of #171 — the cheap, honest part. #171 stays open; this does not resolve it.

The bug this fixes

Naming ModelSchema via --base-classes produced this:

entity "user" as User {
  primary_key(id) : int
  column(name) : str
  column(email) : str
}

entity "user_schema" as UserSchema {
  ' (no fields)
}

An empty box beside the real table. django-ninja's ModelSchema (inner Meta) and ninja-schema's (inner Config) take their fields from a Django model, leaving the class body empty, and erdify does not resolve that reference. An empty entity is worse than no support — the diagram stops being incomplete and starts being wrong, with nothing telling the reader why.

Now

Warning: skipped UserSchema - its fields come from Meta.model (User), which erdify
  does not resolve. Drawing it would add an entity with no fields.
  See https://github.com/devsuit-berlin/erdify/issues/171

The Django model still renders; the misleading box is gone; the reader is told what happened and where to say something about it.

Detection is deliberately narrow

Only an inner Meta/Config that assigns model counts. Two things that must not trigger it, both tested:

  • an ordinary Pydantic model with a nested class Config: frozen = True
  • Django's own class Meta: db_table = "user" — which also never reaches this branch, since it classifies as django, not pydantic

A schema that declares some explicit fields alongside Meta.model is also skipped: the entity would still be missing the derived ones, so a partial box is the same problem in smaller form.

Why not implement the rest of #171

I'd hold, and the reasons are worth recording on the issue:

  • The design question is unanswered and it is the hard part. A UserSchema is a projection of User, not a table. Drawn as a second entity it duplicates fields and implies a relationship that does not exist. The only defensible shape is an opt-in view, which is a lot of machinery for something nobody has asked to see.
  • It is substantially more work than the --base-classes half. Resolving Meta.model to a Django model that is also in the scan, reusing the Django field extraction, applying fields/include/exclude/optional, two incompatible conventions (Meta + fields vs Config + include), and depth, which recurses into related schemas.
  • There is no demand yet. Support django-ninja schemas: recognise Pydantic bases defined outside the scanned files #171 has zero reactions and zero comments, and 0.14.0 shipped the first half hours ago.
  • It may be the wrong artifact. "What the API exposes vs what the database stores" is a genuinely useful diagram — but that is an API-contract diagram, not an ERD.

This warning is the thing that makes the unknown resolvable: people who hit it now have a reason to comment on #171, and what they ask for is exactly what is missing.

Type of change

  • Bug fix
  • New feature

Verification

7 new tests (348 total, was 341), coverage 93.12% against the 90% gate: both inner-class forms, a qualified model = accounts.User reference, a partial schema, an ordinary Pydantic model with a nested config, a Django class Meta, and the CLI path.

uv run pytest                 348 passed
uv run ruff check / format    clean
uv run mypy src/              Success: no issues found in 9 source files
mkdocs build --strict         passes
pre-commit run --all-files    clean

Docs: the Bases defined outside the scan note in usage/cli.md now describes the real behaviour and puts the open design question to readers; frameworks/limitations.md gains a row.

One judgement call to check

The warning prints unconditionally, including for library callers. The hint_unmatched_model_packages flag exists precisely so a discovery hint stays quiet for programmatic use — but this is not a hint, it is "I found something and deliberately did not draw it", which is closer to the unconditional parse-error warnings already in parse_all_models. Happy to gate it behind the same flag if you would rather the library stayed silent.

Checklist

  • Tests added or updated for the change
  • uv run pytest passes locally
  • uv run ruff check src/ tests/ and uv run mypy src/ pass
  • Docs/README updated if behavior changed
  • CHANGELOG.md updated under [Unreleased]

Naming `ModelSchema` via --base-classes produced an empty entity beside the
real table: django-ninja's ModelSchema (inner Meta) and ninja-schema's (inner
Config) take their fields from a Django model, leaving the class body empty,
and erdify does not resolve that reference. An empty box is worse than no
support — the diagram becomes wrong rather than incomplete.

Detect the shape and skip it, with a warning naming the referenced model and
linking #171. Detection is narrow: only an inner Meta/Config that assigns
`model` counts, so an ordinary Pydantic model with a nested config class and
Django's own `class Meta` are untouched. The Django model itself still renders.

This does not resolve #171 — whether a response schema belongs in an ERD at all
is still open — but it makes the current behaviour honest.
@FabianClemenz
FabianClemenz force-pushed the feat/warn-on-model-derived-schemas branch from c7ab109 to 1049205 Compare September 14, 2026 13:12
@FabianClemenz
FabianClemenz merged commit ef81482 into main Sep 14, 2026
12 checks passed
@FabianClemenz
FabianClemenz deleted the feat/warn-on-model-derived-schemas branch September 14, 2026 13:14
@FabianClemenz FabianClemenz mentioned this pull request Sep 14, 2026
6 tasks done
FabianClemenz added a commit that referenced this pull request Sep 14, 2026
## Summary

Patch release. `[Unreleased]` → `[0.14.1]` with the compare link,
version bump, `uv lock`, and the documented pre-commit `rev:` moved to
`v0.14.1`.

### What ships

**Fixed — ninja `ModelSchema` no longer drawn as an empty entity**
(#179). Behaviour introduced hours ago in 0.14.0: anyone passing
`--base-classes ModelSchema` got an empty box beside the real table. Now
detected, skipped, and explained on stderr.

**Fixed — `frameworks.svg` is valid XML again**, and the image footer no
longer makes the headline's "five" look like a miscount (#178).

**Changed — the README and docs home open with the five-frameworks
image** (#176–#178).

### One reclassification worth noting

The `ModelSchema` entry moved from `### Added` to `### Fixed`. It stops
erdify producing a wrong entity rather than adding a capability — and
getting that right is what makes this a **patch** rather than a minor. A
reader scanning the changelog for "do I need this?" gets a truthful
answer either way, but the version number only lines up under `Fixed`.

### Why release now rather than sit on it

Two reasons, and the second is the stronger one:

1. **It fixes output that is wrong today.** `--base-classes` shipped in
0.14.0, so the empty-entity bug is live for anyone who reaches for
`ModelSchema`.
2. **The PyPI page is a launch landing page, and it is stale.** PyPI
renders the README from the sdist at publish time. 0.14.0 was published
*before* the five-frameworks image and the comparison table landed, so
pypi.org/project/erdify currently shows neither. With Show HN and
r/Python next on the list, the page people hit from those posts should
be the current one.

Nothing in here is risky: one narrow parser guard with 7 tests, and
documentation.

## Type of change

- [x] Refactor / maintenance

## Verification

```
uv run pytest                 348 passed
uv run ruff check / mypy      clean
uv run erdify --version       erdify 0.14.1
mkdocs build --strict         passes
pre-commit run --all-files    clean
```

`uv.lock` re-synced in the same commit.

## After merge

Same flow as before: I tag `v0.14.1`, publish the GitHub release and
post the announcement Discussion. The PyPI publish then waits in the
gated `pypi` environment for your **Review deployments → Approve**.

## Checklist

- [x] Tests added or updated for the change — n/a, release mechanics
- [x] `uv run pytest` passes locally
- [x] `uv run ruff check src/ tests/` and `uv run mypy src/` pass
- [x] Docs/README updated if behavior changed
- [x] `CHANGELOG.md` updated under `[Unreleased]`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant