fix: move UnionMode import outside TYPE_CHECKING for Pydantic runtime…#2950
Conversation
… access The UnionMode type is used in GenerateConfig.union_mode field annotation. Pydantic needs to access this type at runtime for model validation, but it was only imported inside TYPE_CHECKING block, causing: PydanticUserError: GenerateConfig is not fully defined; you should define UnionMode, then call GenerateConfig.model_rebuild(). This follows the same pattern as other runtime-required imports in this file (Path, DataModel, DataModelFieldBase, etc.) which have noqa: TC001 comments.
|
Warning Rate limit exceeded@koxudaxi has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 25 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughUnionMode is now imported at module scope from Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ 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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes a Pydantic runtime validation error by moving the UnionMode import outside the TYPE_CHECKING block. The type annotation is used in the GenerateConfig.union_mode field and must be accessible at runtime for Pydantic model validation.
Key changes:
- Moved
UnionModeimport fromTYPE_CHECKINGblock to runtime imports - Added
noqa: TC001comment to suppress type-checking linter warning, consistent with other runtime-required imports
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Alternatively this here in pyproject :) [tool.ruff.lint.flake8-type-checking]
runtime-evaluated-base-classes = ["pydantic.BaseModel"] |
CodSpeed Performance ReportMerging this PR will degrade performance by 17.71%Comparing
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | test_perf_graphql_style_pydantic_v2 |
705 ms | 823.5 ms | -14.39% |
| ❌ | WallTime | test_perf_stripe_style_pydantic_v2 |
1.7 s | 2 s | -15.07% |
| ❌ | WallTime | test_perf_complex_refs |
1.8 s | 2 s | -13.29% |
| ❌ | WallTime | test_perf_duplicate_names |
842.7 ms | 1,000 ms | -15.73% |
| ❌ | WallTime | test_perf_aws_style_openapi_pydantic_v2 |
1.7 s | 1.9 s | -13.88% |
| ❌ | WallTime | test_perf_large_models_pydantic_v2 |
3.1 s | 3.7 s | -17.71% |
| ❌ | WallTime | test_perf_all_options_enabled |
5.8 s | 6.7 s | -13.78% |
| ❌ | WallTime | test_perf_kubernetes_style_pydantic_v2 |
2.2 s | 2.6 s | -15.06% |
| ❌ | WallTime | test_perf_deep_nested |
5.2 s | 6 s | -14.34% |
| ❌ | WallTime | test_perf_openapi_large |
2.5 s | 2.9 s | -14.18% |
| ❌ | WallTime | test_perf_multiple_files_input |
3.2 s | 3.7 s | -15.02% |
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. ↩
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2950 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 94 94
Lines 17716 17723 +7
Branches 2037 2037
=========================================
+ Hits 17716 17723 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @tests/main/test_main_general.py:
- Around line 2065-2093: The Ruff S603 false positive on the subprocess.run argv
can be silenced by adding an explicit per-line ignore; update the subprocess.run
invocation in test_generate_config_union_mode_runtime_access to append the
comment "# noqa: S603" (or "# ruff: noqa: S603") to the line with
subprocess.run(...) so the constant argv list is exempted from the S603 check
while leaving the rest of the test unchanged.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/main/test_main_general.py
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: koxudaxi
Repo: koxudaxi/datamodel-code-generator PR: 2799
File: src/datamodel_code_generator/model/pydantic/__init__.py:43-43
Timestamp: 2025-12-25T09:22:22.481Z
Learning: In datamodel-code-generator project, defensive `# noqa: PLC0415` directives should be kept on lazy imports (imports inside functions/methods) even when Ruff reports them as unused via RUF100, to prepare for potential future Ruff configuration changes that might enable the import-outside-top-level rule.
Learnt from: koxudaxi
Repo: koxudaxi/datamodel-code-generator PR: 2681
File: tests/cli_doc/test_cli_doc_coverage.py:82-82
Timestamp: 2025-12-18T13:43:16.235Z
Learning: In datamodel-code-generator project, Ruff preview mode is enabled via `lint.preview = true` in pyproject.toml. This enables preview rules like PLR6301 (no-self-use), so `noqa: PLR6301` directives are necessary and should not be removed even if RUF100 suggests they are unused.
🪛 Ruff (0.14.10)
tests/main/test_main_general.py
2079-2079: subprocess call: check for execution of untrusted input
(S603)
cba66b2 to
d58b475
Compare
Add test that verifies UnionMode is available at runtime for Pydantic model_rebuild(). This test runs in a subprocess to ensure a clean namespace without pytest's import side effects, properly detecting when UnionMode is incorrectly placed inside TYPE_CHECKING block.
d58b475 to
65e9531
Compare
Breaking Change AnalysisResult: No breaking changes detected Reasoning: This PR is a bug fix that moves the UnionMode import from inside TYPE_CHECKING block to runtime in config.py. The change was necessary because Pydantic requires access to UnionMode at runtime for model validation of GenerateConfig.union_mode. This fixes a PydanticUserError that would occur when using GenerateConfig with union_mode. The UnionMode type was already publicly exported via datamodel_code_generator.model.pydantic_v2 and datamodel_code_generator.enums - this change only affects internal imports. No generated code, API, CLI, templates, defaults, or error handling behavior changed. Users benefit from this fix as it allows GenerateConfig.union_mode to work correctly. This analysis was performed by Claude Code Action |
|
🎉 Released in 0.53.0 This PR is now available in the latest release. See the release notes for details. |
… access
The UnionMode type is used in GenerateConfig.union_mode field annotation. Pydantic needs to access this type at runtime for model validation, but it was only imported inside TYPE_CHECKING block, causing:
PydanticUserError: GenerateConfig is not fully defined; you should define UnionMode, then call GenerateConfig.model_rebuild().
This follows the same pattern as other runtime-required imports in this file (Path, DataModel, DataModelFieldBase, etc.) which have noqa: TC001 comments.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.