Skip to content

fix: handle type definitions from grand(grand...) parent schemas#2861

Merged
koxudaxi merged 1 commit intokoxudaxi:mainfrom
opensemanticworld:fix-deep-inherited-item-type
Dec 30, 2025
Merged

fix: handle type definitions from grand(grand...) parent schemas#2861
koxudaxi merged 1 commit intokoxudaxi:mainfrom
opensemanticworld:fix-deep-inherited-item-type

Conversation

@simontaurus
Copy link
Copy Markdown
Contributor

@simontaurus simontaurus commented Dec 30, 2025

  • update related tests for Entity and Thing schemas

Handles the edge case where an array item schema gets annotated in a grand(-grand) child schema, see:
#2087 (comment)
#2087 (comment)

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced field type resolution for inherited properties in JSON Schema parsing, ensuring correct type determination through multi-level inheritance hierarchies.
  • Tests

    • Updated test fixtures to reflect improved handling of class inheritance and field resolution in generated code.

✏️ Tip: You can customize this high-level summary in your review settings.

update related tests for Entity and Thing schemas
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 30, 2025

📝 Walkthrough

Walkthrough

The pull request refines field type resolution in inheritance chains by improving the _get_inherited_field_type method to better handle grandparent class resolution, and updates test expectations to reflect the new behavior where a base Entity class is introduced with inherited fields.

Changes

Cohort / File(s) Summary
Parser Logic Refinement
src/datamodel_code_generator/parser/jsonschema.py
Modified _get_inherited_field_type to defer early returns until concrete types are found; introduced separate parent_result variable for recursive grandparent chain resolution; now continues searching up inheritance hierarchy if intermediate results are ambiguous (ANY or List[Any])
Test Data Update
tests/data/expected/main/openapi/allof_partial_override_array_items.py
Added new Entity(BaseModel) base class with `type_list: list[str]

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

breaking-change-analyzed, breaking-change

Poem

🐰 Through inheritance chains, we hop and we bound,
Searching for types through layers profound,
Grandparents resolved with recursive care,
Entity base classes nested with flair!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: fixing handling of type definitions from ancestor schemas at multiple inheritance levels (grandparents and deeper).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 63d8804 and 7b7f29a.

⛔ Files ignored due to path filters (1)
  • tests/data/openapi/allof_partial_override_array_items.yaml is excluded by !tests/data/**/*.yaml and included by none
📒 Files selected for processing (2)
  • src/datamodel_code_generator/parser/jsonschema.py
  • tests/data/expected/main/openapi/allof_partial_override_array_items.py
🧰 Additional context used
🧬 Code graph analysis (1)
src/datamodel_code_generator/parser/jsonschema.py (1)
src/datamodel_code_generator/types.py (1)
  • DataType (297-838)
🔇 Additional comments (2)
src/datamodel_code_generator/parser/jsonschema.py (1)

1809-1855: Verify early return behavior when grandparent search yields no concrete type.

The new logic introduces a potential early exit from the base_classes loop that could skip remaining parents in certain scenarios.

Scenario:

  1. First base class defines the property with an ambiguous type (ANY or List[Any])
  2. That base class has allOf with grandparent references
  3. Recursive grandparent search returns None
  4. Line 1853 returns the ambiguous result, exiting the function
  5. Remaining base classes are never checked, even if they might have concrete types

Question: Is this the intended behavior? Should the loop continue checking remaining base classes when:

  • Parent has ambiguous type
  • Grandparent search yields nothing
  • Other parents haven't been checked yet

The comment on line 1842 states "In case of a missing type, continue searching up the inheritance chain", which suggests continuing the search. However, line 1853's early return prevents checking sibling parents in the base_classes list.

Consider this alternative approach if the intent is to check all base classes:

# After grandparent search
if parent_result is not None:
    # Check if grandparent result is concrete
    if not (parent_result.type == ANY or self._is_list_with_any_item_type(parent_result)):
        return parent_result
    # If grandparent result is ambiguous, store it but continue searching other base classes
    if result is None:  # Only update if we don't have anything yet
        result = parent_result
# Continue to next base class instead of early return

Then return result after the loop completes, giving preference to concrete types while falling back to ambiguous types if no concrete type is found anywhere in the inheritance chain.

tests/data/expected/main/openapi/allof_partial_override_array_items.py (1)

10-16: LGTM! Test expectations correctly reflect the new inheritance structure.

The changes demonstrate the grandparent field type resolution:

  • Entity (grandparent) now defines the base type_list: list[str] | None = None
  • Thing (parent) inherits from Entity and provides a concrete default
  • This validates that the parser correctly identifies and creates the inheritance hierarchy when array item schemas receive type definitions in descendant schemas

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Dec 30, 2025

CodSpeed Performance Report

Merging #2861 will not alter performance

Comparing opensemanticworld:fix-deep-inherited-item-type (7b7f29a) with main (63d8804)

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

Summary

✅ 11 untouched
⏩ 98 skipped1

Footnotes

  1. 98 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.52%. Comparing base (7a4709c) to head (7b7f29a).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2861      +/-   ##
==========================================
+ Coverage   99.50%   99.52%   +0.02%     
==========================================
  Files          90       90              
  Lines       14869    14941      +72     
  Branches     1781     1788       +7     
==========================================
+ Hits        14795    14870      +75     
  Misses         38       38              
+ Partials       36       33       -3     
Flag Coverage Δ
unittests 99.52% <100.00%> (+0.02%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

@koxudaxi koxudaxi merged commit 8c7a9b2 into koxudaxi:main Dec 30, 2025
37 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 1, 2026

🎉 Released in 0.51.0

This PR is now available in the latest release. See the release notes for details.

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