Skip to content

fix: preserve explicit inherited fields - #10

Merged
goduni merged 3 commits into
goduni:mainfrom
K1rL3s:fix/preserve-explicit-inherited-fields
Aug 16, 2026
Merged

fix: preserve explicit inherited fields#10
goduni merged 3 commits into
goduni:mainfrom
K1rL3s:fix/preserve-explicit-inherited-fields

Conversation

@K1rL3s

@K1rL3s K1rL3s commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Description

Inheritance mode removed properties explicitly declared by a child schema when they
repeated an inherited property. This discarded child-specific metadata and prevented
downstream profiles from recognizing the declaration.

This PR changes the contract:

  • every property explicitly declared by a child remains in the child's IR, even when
    its type, description, constraints, requiredness, and default match the base field;
  • compatible overrides render normally — including a $ref narrowed to a schema that
    inherits from the base's (companion: Pet over companion: Creature) and integer
    over number;
  • an override the base cannot admit stays faithful to the schema and receives a local
    # type: ignore[assignment, unused-ignore], plus a warning naming the schema and
    property, because a subtype that is not substitutable for its base is a defect worth
    fixing in the spec;
  • properties declared only on the base remain inherited and are not copied down.

The marker is stored on IRField.incompatible_override and turned into a suppression
comment in one place shared by the Adaptix, Pydantic, and msgspec renderers. This keeps
generated packages clean under mypy --strict without losing explicit OpenAPI
declarations.

unused-ignore rides along with assignment on purpose: the builder judges
compatibility from the IR, which is a coarser view of a type than mypy's, so wherever
the builder is the stricter of the two a bare [assignment] ignore would be unused —
which --strict reports as an error of its own.

Behaviour change (breaking for consumers)

Keeping a subtype's widening restatement changes decoding, not just typing. Where a
subtype re-declares an inherited property as nullable, the generated model now accepts
null for it, on every serializer:

payload before after
LinkButton with text: null rejected (AggregateLoadError / ValidationError) accepted, text=None

This matches what the spec declares, and sibling subtypes are unaffected — a subtype
that does not restate the property still requires it. But the annotation consumers see
widens with it: code that read button.text as a str now gets str | None and fails
mypy until it handles the None. Regenerating a client against such a spec is
therefore a breaking change for its callers.

Type of change

  • Bug fix (resolves an issue)
  • Breaking change (a fix or feature that would cause existing functionality to not
    work as expected) — see the section above
  • This change requires a documentation update

Verification

  • uv run pytest --cov src --cov-fail-under=80 --cov-report=term - 334 passed,
    100% coverage
  • uv run ruff check
  • uv run ruff format --check
  • uv run mypy
  • inheritance compile gate covers Adaptix, Pydantic, and msgspec in both file layouts,
    including the two shapes only the base chain or the numeric tower can justify
  • decoding behaviour and the consumer-visible annotation change verified against a
    generated package on both branches

Checklist

  • My code adheres to the style guidelines of this project
  • I have conducted a self-review of my own code
  • I have made the necessary changes to the documentation
  • My changes do not generate any new warnings
  • I have added tests to validate the effectiveness of my fix
  • I have ensured that type checking passes by running uv run mypy
  • The generated incompatible override is covered as a concrete code example

@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (387daab) to head (b2f715c).

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #10   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           31        31           
  Lines         2706      2726   +20     
=========================================
+ Hits          2706      2726   +20     

☔ 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.

K1rL3s and others added 2 commits August 14, 2026 20:20
Preserving every explicit child declaration made the narrowing predicate
load-bearing in a way it was not written for. It is deliberately conservative
because a false "no" used to mean only a slightly less precise inherited type;
now a false "no" emits `# type: ignore[assignment]` on a line mypy is happy
with, and `--strict` reports the unused ignore as an error of its own. Two
ordinary spec shapes hit this and made `--inheritance --check` fail:

- a subtype narrowing a `$ref` to a schema that inherits from the base's
  (`companion: Pet` over `companion: Creature`) -- nothing in either annotation
  says the two are related;
- `boolean` over `integer`, which mypy accepts via the numeric tower.

Both are now recognised as narrowings: `_is_narrowing` consults the declaration
map for `$ref` overrides and a promotion table for primitives. The suppression
comment additionally lists `unused-ignore`, so the remaining gap between the
IR's view of a type and mypy's costs a redundant comment rather than a failed
build. Both shapes join the hierarchy fixture, so the compile gate holds them
under `mypy --strict` for every serializer and layout.

The flag itself moves to `IRField.incompatible_override`: the IR records the
fact about the spec, and the serializer strategies decide what to emit for it
in one shared place instead of three copies. An incompatible override is a spec
defect -- the subtype is not substitutable for its base -- so the warning now
says that, and points at the schema and property to fix upstream.
@goduni
goduni merged commit 6eefcd0 into goduni:main Aug 16, 2026
5 checks passed
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.

2 participants