Fix allOf array property merging to preserve child $ref by koxudaxi · Pull Request #2962 · koxudaxi/datamodel-code-generator

koxudaxi · 2026-01-12T16:03:25Z

Summary by CodeRabbit

Bug Fixes
- Fixed schema property merging to correctly preserve $ref references, preventing duplicate model generation when merging child properties into parent schemas.
Tests
- Added test coverage for allOf array references with proper handling, ensuring correct code generation without creating redundant models.

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

coderabbitai · 2026-01-12T16:03:39Z

Warning

Rate limit exceeded

@koxudaxi has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 51 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between b1bb20b and e952f34.

⛔ Files ignored due to path filters (1)

tests/data/openapi/allof_array_ref_override.yaml is excluded by !tests/data/**/*.yaml and included by none

📒 Files selected for processing (2)

tests/data/expected/main/openapi/allof_array_ref_override.py
tests/main/openapi/test_main_openapi.py

📝 Walkthrough

Walkthrough

The PR fixes a schema merging bug where child schemas with $ref references were incorrectly deep-merged with parent dictionaries. The fix ensures that when a child property contains a $ref, it's preserved as-is rather than recursively merged. A test case validating the corrected behavior is added.

Changes

Cohort / File(s)	Summary
Schema Merge Logic `src/datamodel_code_generator/parser/jsonschema.py`	Modified `_merge_property_schemas` to check if child value contains a `$ref` key; if present, preserve child value unchanged instead of recursively merging dictionaries. Prevents duplicate model generation when merging allOf schemas with property overrides.
Test Expected Output `tests/data/expected/main/openapi/issue_2959.py`	New test data file containing generated Pydantic models for issue `#2959`: DataType, Metadata, CollectionWrapper, and PaginatedDataTypeList (paginated collection wrapper with specialized data field).
Test Case Update `tests/main/openapi/test_main_openapi.py`	Renamed test from `test_main_openapi_reuse_scope_tree_single_file_error` to `test_main_openapi_allof_array_ref_no_duplicate_model`; shifted from error-path testing to successful generation validation; updated CLI arguments to remove reuse-model/reuse-scope and add use-standard-collections/use-union-operator/use-schema-description.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Fix $ref handling in request-response mode for readOnly/writeOnly schemas #2942: Modifies $ref handling in jsonschema.py to update resolution logic for request/response variant models
Fix allOf multi-ref to preserve inheritance with property overrides #2838: Adjusts allOf multi-ref merging to preserve inheritance and property overrides with $ref semantics
Fix allOf with $ref to root model losing constraints #2676: Adds special handling for allOf with $ref-to-root models and constraints in schema merging

Suggested labels

breaking-change-analyzed

Poem

🐰 A fuzzy hop through merge logic's lane,
Where refs were deep-merged, causing pain,
Now children with refs stand tall and true,
No duplicate Datum in our view!
Hopping towards cleaner schemas—how grand!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title directly describes the main fix: preserving child $ref during allOf array property merging, which is the core change in the code.
Linked Issues check	✅ Passed	The PR directly fixes issue #2959 by modifying _merge_property_schemas to preserve child $ref values instead of deep-merging, preventing duplicate Datum class generation and ensuring correct DataType reference.
Out of Scope Changes check	✅ Passed	All changes align with fixing issue #2959: core logic fix in jsonschema.py parser, new expected output test file, and test function update to validate the fix.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

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

github-actions · 2026-01-12T16:04:14Z

📚 Docs Preview: https://pr-2962.datamodel-code-generator.pages.dev

codspeed-hq · 2026-01-12T16:06:08Z

CodSpeed Performance Report

Merging this PR will not alter performance

_{Comparing fix/issue-2959-allof-array-ref-override (e952f34) with main (ae89a00)}

⚠️

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 benchmarks
⏩ 98 skipped benchmarks¹

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

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In @tests/data/expected/main/openapi/issue_2959.py:
- Around line 1-3: The generated comment header contains a filename typo: change
the referenced filename string "issue_2953.yaml" in the top comment to
"issue_2959.yaml" so the header matches the actual file name (issue_2959.py / PR
#2959); update the second comment line to read '#   filename:  issue_2959.yaml'.

📜 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 ae89a00 and b1bb20b.

📒 Files selected for processing (3)

src/datamodel_code_generator/parser/jsonschema.py
tests/data/expected/main/openapi/issue_2959.py
tests/main/openapi/test_main_openapi.py

🧰 Additional context used

🧬 Code graph analysis (1)

tests/main/openapi/test_main_openapi.py (1)

tests/main/conftest.py (2)

output_file (99-101)

run_main_and_assert (245-409)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)

GitHub Check: 3.10 on macOS
GitHub Check: 3.12 on macOS
GitHub Check: 3.10 on Windows
GitHub Check: 3.14 on macOS
GitHub Check: 3.13 on Windows
GitHub Check: 3.11 on macOS
GitHub Check: 3.13 on macOS
GitHub Check: 3.14 on Windows
GitHub Check: 3.11 on Windows
GitHub Check: 3.12 on Windows
GitHub Check: benchmarks
GitHub Check: Analyze (python)

🔇 Additional comments (4)

src/datamodel_code_generator/parser/jsonschema.py (1)

1871-1879: LGTM! Correctly preserves child $ref during allOf property merging.

The fix appropriately checks for $ref presence in the child dict before deciding whether to recursively merge. When a child property schema contains a $ref, it represents a complete type reference that should override the parent rather than being deep-merged, which was causing the duplicate model generation issue reported in #2959.

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

45-48: LGTM! Validates the fix for issue #2959.

The PaginatedDataTypeList correctly inherits from CollectionWrapper and overrides the data field with the specific list[DataType] | None type. This confirms that the child $ref to DataType is properly preserved during allOf merging, rather than producing a duplicate model like the previously reported Datum class.

tests/main/openapi/test_main_openapi.py (2)

4945-4964: LGTM! Test correctly validates the allOf array $ref preservation fix.

The test structure is well-formed:

Clear docstring explaining the regression scenario (issue #2959)

Appropriate use of SKIP_PYDANTIC_V1 marker for Pydantic v2-specific behavior

CLI args align with the issue's reported configuration

Pending verification of the input file name discrepancy noted above.

4953-4953: The input file issue_2953.yaml is intentionally reused for testing the fix for issue #2959. Since only issue_2953.yaml exists and the schema is relevant for both issues, this is not a mistake and requires no changes.

codecov · 2026-01-12T16:07:42Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (ae89a00) to head (e952f34).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main     #2962   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           94        94           
  Lines        17759     17761    +2     
  Branches      2038      2039    +1     
=========================================
+ Hits         17759     17761    +2

Flag	Coverage Δ
unittests	`100.00% <100.00%> (ø)`

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.

github-actions · 2026-01-12T16:25:02Z

Breaking Change Analysis

Result: No breaking changes detected

Reasoning: This PR is a bug fix that corrects incorrect behavior in allOf array property merging. The change ensures that when a child property contains a $ref, it completely replaces the parent property instead of incorrectly merging them. Before the fix, this merging caused duplicate model generation (e.g., creating unnecessary 'Datum' classes). The new behavior produces correct output where child $ref properly overrides parent's generic type. While regenerated code will differ for affected schemas, this represents a correction of buggy behavior rather than a breaking change - users will get correct output instead of incorrect output with duplicate models. The change is internal to the parser and does not affect any public API, CLI options, or default behaviors.

This analysis was performed by Claude Code Action

github-actions · 2026-01-12T18:17:05Z

🎉 Released in 0.53.0

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

Fix allOf array property merging to preserve child $ref

b1bb20b

coderabbitai Bot reviewed Jan 12, 2026

View reviewed changes

Comment thread tests/data/expected/main/openapi/allof_array_ref_override.py

koxudaxi added 2 commits January 12, 2026 16:12

Fix filename mismatch in test fixture

7bdc5d2

Rename test fixtures to descriptive names

e952f34

koxudaxi merged commit e717208 into main Jan 12, 2026
38 checks passed

koxudaxi deleted the fix/issue-2959-allof-array-ref-override branch January 12, 2026 16:23

github-actions Bot added the breaking-change-analyzed label Jan 12, 2026

github-actions Bot mentioned this pull request Jan 12, 2026

Generation results in unexpected Datum class #2959

Closed

This was referenced Feb 7, 2026

Fix RecursionError in _merge_ref_with_schema for circular $ref #2983

Merged

$ref doesn't work with extra fields #2990

Closed

Merged result in parse_item not passed back to parse_object_fields #2995

Closed

coderabbitai Bot mentioned this pull request Feb 14, 2026

Fix merged result in parse_item not passed back to parse_object_fields #2997

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix allOf array property merging to preserve child $ref#2962

Fix allOf array property merging to preserve child $ref#2962
koxudaxi merged 3 commits intomainfrom
fix/issue-2959-allof-array-ref-override

koxudaxi commented Jan 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jan 12, 2026 •

edited

Loading

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

Uh oh!

github-actions Bot commented Jan 12, 2026 •

edited

Loading

Uh oh!

codspeed-hq Bot commented Jan 12, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

codecov Bot commented Jan 12, 2026 •

edited

Loading

Uh oh!

Uh oh!

github-actions Bot commented Jan 12, 2026

Uh oh!

github-actions Bot commented Jan 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

koxudaxi commented Jan 12, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

Uh oh!

github-actions Bot commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codspeed-hq Bot commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging this PR will not alter performance

Summary

Footnotes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

github-actions Bot commented Jan 12, 2026

Breaking Change Analysis

Uh oh!

github-actions Bot commented Jan 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

koxudaxi commented Jan 12, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jan 12, 2026 •

edited

Loading

github-actions Bot commented Jan 12, 2026 •

edited

Loading

codspeed-hq Bot commented Jan 12, 2026 •

edited

Loading

codecov Bot commented Jan 12, 2026 •

edited

Loading