Fix type overrides for base refs#3589
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR updates model-level ChangesModel-level Override Propagation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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. Comment |
|
📚 Docs Preview: https://pr-3589.datamodel-code-generator.pages.dev |
koxudaxi
force-pushed
the
fix/type-overrides-base-refs
branch
from
2a9746a to
a0f4fe1
Compare
Merging this PR will improve performance by 53.28%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | test_perf_aws_style_openapi_pydantic_v2 |
2 s | 1.2 s | +56.91% |
| ⚡ | WallTime | test_perf_openapi_large |
3 s | 1.9 s | +55.96% |
| ⚡ | WallTime | test_perf_deep_nested |
5.3 s | 3.4 s | +55.74% |
| ⚡ | WallTime | test_perf_complex_refs |
2 s | 1.3 s | +55.64% |
| ⚡ | WallTime | test_perf_kubernetes_style_pydantic_v2 |
2.6 s | 1.7 s | +54.86% |
| ⚡ | WallTime | test_perf_graphql_style_pydantic_v2 |
808.3 ms | 522.4 ms | +54.71% |
| ⚡ | WallTime | test_perf_stripe_style_pydantic_v2 |
2 s | 1.3 s | +53.67% |
| ⚡ | WallTime | test_perf_large_models_pydantic_v2 |
3.6 s | 2.3 s | +53.5% |
| ⚡ | WallTime | test_perf_multiple_files_input |
3.5 s | 2.3 s | +53.35% |
| ⚡ | WallTime | test_perf_all_options_enabled |
5.4 s | 3.6 s | +52.55% |
| ⚡ | WallTime | test_perf_duplicate_names |
1,036.9 ms | 685.5 ms | +51.28% |
| ⚡ | WallTime | test_perf_large_models_pydantic_v2_noformat |
882.2 ms | 597.2 ms | +47.71% |
| ⚡ | WallTime | test_perf_large_models_pydantic_v2_builtin |
940.6 ms | 639.6 ms | +47.07% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix/type-overrides-base-refs (892b169) with main (be80b48)
Footnotes
-
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/cli-reference/typing-customization.md (1)
2816-2839: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument dict-key propagation too.
These updated descriptions still omit that model-level overrides are also applied recursively to dict keys. That behavior is part of the current implementation, so the docs now underdescribe the CLI contract.
♻️ Suggested doc tweak
-| `{"ModelName": "package.Type"}` | Model-level: Skip generation; replace field and inheritance refs | +| `{"ModelName": "package.Type"}` | Model-level: Skip generation; replace field, inheritance, and dict-key refs | @@ - References to that model are replaced in field annotations and `allOf` inheritance base classes. + References to that model are replaced in field annotations, `allOf` inheritance base classes, and dict keys.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/cli-reference/typing-customization.md` around lines 2816 - 2839, Update the type-overrides documentation in the CLI reference to mention that model-level overrides are applied recursively to dict keys as well as field annotations and allOf inheritance. Keep the existing explanation in the type-overrides section and the “Model-level overrides skip generation” note, but extend the wording so the behavior of dict-key propagation is explicitly described alongside the model-level override examples.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/main/test_main_general.py`:
- Around line 1614-1646: The test `test_type_overrides_model_level_base_class`
leaves the temporary `my_app` package imported, which can leak into later tests
and affect imports. Update the test setup/teardown so the `my_app` entries are
removed from `sys.modules` after `run_main_and_assert` completes, ensuring this
test does not reuse stale modules when `test_main_jsonschema` or other tests
create the same package name.
---
Nitpick comments:
In `@docs/cli-reference/typing-customization.md`:
- Around line 2816-2839: Update the type-overrides documentation in the CLI
reference to mention that model-level overrides are applied recursively to dict
keys as well as field annotations and allOf inheritance. Keep the existing
explanation in the type-overrides section and the “Model-level overrides skip
generation” note, but extend the wording so the behavior of dict-key propagation
is explicitly described alongside the model-level override examples.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 50db6942-c807-4901-b99d-b8b6a001dff5
⛔ Files ignored due to path filters (2)
tests/data/jsonschema/type_overrides_base_class.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/jsonschema/type_overrides_dict_key.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (5)
docs/cli-reference/typing-customization.mdsrc/datamodel_code_generator/parser/base.pytests/data/expected/main/type_overrides_base_class.pytests/data/expected/main/type_overrides_dict_key.pytests/main/test_main_general.py
koxudaxi
force-pushed
the
fix/type-overrides-base-refs
branch
2 times, most recently
from
c93048f to
9edf7a4
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3589 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 164 164
Lines 35243 35287 +44
Branches 4063 4075 +12
=========================================
+ Hits 35243 35287 +44
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
koxudaxi
force-pushed
the
fix/type-overrides-base-refs
branch
from
9edf7a4 to
892b169
Compare
Breaking Change AnalysisResult: No breaking changes detected Reasoning: PR #3589 fixes the existing opt-in This analysis was performed by Claude Code Action |
|
🎉 Released in 0.68.1 This PR is now available in the latest release. See the release notes for details. |
1 participant
Summary by CodeRabbit
--type-overridesmodel-level support, including replacement of referenced types in field annotations andallOf-inherited base classes.