Conversation
|
Warning Rate limit exceeded@koxudaxi has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 33 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 ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe JSONSchema parser now creates named type aliases for inline schemas with titles (arrays, dicts with additionalProperties, enums, oneOf/anyOf) by routing them to parse_root_type; parse_root_type gained dict handling for anonymous objects. Tests and docs updated to reflect generated wrapper models and alias usage. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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-2889.datamodel-code-generator.pages.dev |
CodSpeed Performance ReportMerging #2889 will degrade performance by 16.34%Comparing
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | test_perf_stripe_style_pydantic_v2 |
1.7 s | 2 s | -14.74% |
| ❌ | WallTime | test_perf_large_models_pydantic_v2 |
3 s | 3.6 s | -16.34% |
| ❌ | WallTime | test_perf_all_options_enabled |
5.7 s | 6.6 s | -13.62% |
| ❌ | WallTime | test_perf_aws_style_openapi_pydantic_v2 |
1.7 s | 1.9 s | -13.85% |
| ❌ | WallTime | test_perf_deep_nested |
5.2 s | 6 s | -13.56% |
| ❌ | WallTime | test_perf_multiple_files_input |
3.2 s | 3.7 s | -14.15% |
| ❌ | WallTime | test_perf_graphql_style_pydantic_v2 |
702.2 ms | 816.2 ms | -13.97% |
| ❌ | WallTime | test_perf_duplicate_names |
845.5 ms | 1,001.6 ms | -15.58% |
| ❌ | WallTime | test_perf_kubernetes_style_pydantic_v2 |
2.2 s | 2.6 s | -15.39% |
| ❌ | WallTime | test_perf_complex_refs |
1.7 s | 2 s | -15.03% |
| ❌ | WallTime | test_perf_openapi_large |
2.5 s | 2.9 s | -13.56% |
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. ↩
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/main/jsonschema/test_main_jsonschema.py (1)
3053-3077: Test wiring correctly exercises the inline-title alias bugfixThe test setup (schema path, expected file, and CLI args:
--use-title-as-name, TypedDict, Python 3.13, union operator, standard collections, skip-root-model) is consistent with the behavior described in issue #2887 and the new expected snapshot. This should give good coverage for titled inline array/dict/enum/oneOf/anyOf schemas being turned into named type aliases rather than inlined types.You may want to double‑check that your formatter/black and CI configuration fully support Python 3.13 and
typestatements for this test, or gate it similarly to other 3.13‑specific tests if needed.tests/data/expected/main/jsonschema/use_title_as_name_inline_types.py (1)
1-33: Snapshot matches intended 3.13 type-alias output; verify Ruff/target versionThe expected file structure and
typealiases look correct for the 3.13‑targeted TypedDict test and align well with the PR goal of naming inline titled schemas.Static analysis is flagging the
typestatements as invalid under Python 3.10; if Ruff (or other tooling) is configured with a pre‑3.12 target, you may need to either:
- bump the target version for this part of the tree, or
- exclude these generated‑snapshot files from such checks
to avoid spurious lint failures while still testing 3.13‑only syntax.
src/datamodel_code_generator/parser/jsonschema.py (1)
2938-2991: New dict handling inparse_root_typelooks correct; remove unusednoqa
- The
elif obj.is_object and not obj.properties ... and isinstance(obj.additionalProperties, JsonSchemaObject)branch brings root‑level handling of anonymous “object with additionalProperties” in line with the inline object path, including honoringx-python-typevia_get_python_type_flags. This is needed so titled inline dict schemas can be turned into named aliases viaparse_root_type.- Ruff reports
# noqa: PLR0912, PLR0914, PLR0915onparse_root_typeas unused; you can drop the comment (or the unused codes) to satisfy RUF100.Proposed lint-only change
- def parse_root_type( # noqa: PLR0912, PLR0914, PLR0915 + def parse_root_type( self, name: str, obj: JsonSchemaObject, path: list[str], ) -> DataType:
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/data/jsonschema/use_title_as_name_inline_types.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (5)
docs/cli-reference/field-customization.mdsrc/datamodel_code_generator/parser/jsonschema.pytests/data/expected/main/jsonschema/titles_use_title_as_name.pytests/data/expected/main/jsonschema/use_title_as_name_inline_types.pytests/main/jsonschema/test_main_jsonschema.py
🧰 Additional context used
🧬 Code graph analysis (4)
tests/data/expected/main/jsonschema/use_title_as_name_inline_types.py (1)
src/datamodel_code_generator/model/typed_dict.py (1)
TypedDict(49-114)
tests/main/jsonschema/test_main_jsonschema.py (2)
tests/test_main_kr.py (1)
output_file(44-46)tests/main/conftest.py (2)
output_file(98-100)run_main_and_assert(244-408)
tests/data/expected/main/jsonschema/titles_use_title_as_name.py (2)
tests/data/expected/parser/openapi/openapi_parser_parse_modular/bar.py (1)
Field(6-7)src/datamodel_code_generator/parser/jsonschema.py (1)
parse_obj(3524-3569)
src/datamodel_code_generator/parser/jsonschema.py (1)
src/datamodel_code_generator/parser/base.py (1)
data_type(1053-1055)
🪛 Ruff (0.14.10)
tests/data/expected/main/jsonschema/use_title_as_name_inline_types.py
9-9: Cannot use type alias statement on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
12-12: Cannot use type alias statement on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
15-15: Cannot use type alias statement on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
18-18: Cannot use type alias statement on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
21-21: Cannot use type alias statement on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
24-24: Cannot use type alias statement on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
src/datamodel_code_generator/parser/jsonschema.py
2938-2938: Unused noqa directive (non-enabled: PLR0912, PLR0914, PLR0915)
Remove unused noqa directive
(RUF100)
⏰ 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). (11)
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.13 on macOS
- GitHub Check: 3.12 on macOS
- GitHub Check: py312-isort5 on Ubuntu
- GitHub Check: 3.10 on macOS
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.14 on Windows
- GitHub Check: benchmarks
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
tests/data/expected/main/jsonschema/titles_use_title_as_name.py (1)
47-50: Inline union now correctly wrapped in a titled root model
ProcessingStatusUnionTitleas a root model plus usingdefault_factory=lambda: ProcessingStatusUnionTitle.parse_obj('COMPLETED')preserves the previous default semantics while giving the union a reusable, titled type, which aligns with the--use-title-as-nameintent. The updatedprocessing_status_unionannotation and default look consistent with the schema.Also applies to: 54-57
docs/cli-reference/field-customization.md (1)
3716-3778: Docs snippet correctly updated to reflect new aliasing behaviorThe updated
--use-title-as-nameexample now showsProcessingStatusUnionTitleand theprocessing_status_unionfield using that type, matching the generator’s new behavior and the test expectations.src/datamodel_code_generator/parser/jsonschema.py (1)
2639-2676: Helper and early short‑circuit correctly centralize title‑based aliasingThe new
_should_create_type_alias_for_titleplus the early check inparse_itemcleanly route titled inline arrays, dicts (additionalPropertiesonly), oneOf/anyOf unions, and literal enums throughparse_root_typeinstead of inlining them. The logic mirrors existing enum/union handling (using_extract_const_enum_from_combinedandshould_parse_enum_as_literal) and respectsignore_enum_constraints, so behavior for non‑titled or non‑literal enums stays unchanged while fixing the “use-title-as-name ignored for inline types” bug.Also applies to: 2678-2695
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2889 +/- ##
=======================================
Coverage 99.37% 99.37%
=======================================
Files 92 92
Lines 16121 16152 +31
Branches 1898 1906 +8
=======================================
+ Hits 16020 16051 +31
Misses 52 52
Partials 49 49
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:
|
|
Thanks again for implementing it so quickly. I tried to build this PR myself, and I couldn't make the unit tests pass. It turned out the |
The extra='ignore' parameter in model_validate was added in Pydantic v2.12.5. This change filters fields manually to maintain compatibility with older Pydantic v2 versions. Fixes issue reported in #2889 comment. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The extra='ignore' parameter in model_validate was added in Pydantic v2.12.5. This change filters fields manually to maintain compatibility with older Pydantic v2 versions. Fixes issue reported in #2889 comment.
6b95f84 to
0b487bf
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/datamodel_code_generator/parser/jsonschema.py (1)
2938-2938: Remove unusednoqadirective.Static analysis indicates the
noqacodesPLR0912,PLR0914,PLR0915are not triggered for this function. Consider removing the unused directive.🔎 Proposed fix
- def parse_root_type( # noqa: PLR0912, PLR0914, PLR0915 + def parse_root_type(
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/data/jsonschema/use_title_as_name_inline_types.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (7)
docs/cli-reference/field-customization.mdsrc/datamodel_code_generator/__init__.pysrc/datamodel_code_generator/parser/jsonschema.pytests/data/expected/main/jsonschema/titles_use_title_as_name.pytests/data/expected/main/jsonschema/use_title_as_name_inline_types.pytests/data/expected/main/jsonschema/use_title_as_name_inline_types_pydantic.pytests/main/jsonschema/test_main_jsonschema.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/main/jsonschema/test_main_jsonschema.py
🧰 Additional context used
🧬 Code graph analysis (5)
src/datamodel_code_generator/__init__.py (1)
src/datamodel_code_generator/util.py (3)
is_pydantic_v2(52-57)model_dump(254-258)model_validate(261-265)
src/datamodel_code_generator/parser/jsonschema.py (3)
src/datamodel_code_generator/model/base.py (3)
name(827-829)nullable(903-905)path(908-910)src/datamodel_code_generator/parser/graphql.py (1)
should_parse_enum_as_literal(231-237)src/datamodel_code_generator/parser/base.py (1)
data_type(1053-1055)
tests/data/expected/main/jsonschema/titles_use_title_as_name.py (2)
tests/data/expected/parser/openapi/openapi_parser_parse_modular/bar.py (1)
Field(6-7)src/datamodel_code_generator/parser/jsonschema.py (1)
parse_obj(3524-3569)
tests/data/expected/main/jsonschema/use_title_as_name_inline_types_pydantic.py (3)
src/datamodel_code_generator/model/enum.py (1)
Enum(39-121)tests/data/expected/parser/openapi/openapi_parser_parse_modular/bar.py (1)
Field(6-7)tests/data/expected/main/jsonschema/use_title_as_name_inline_types.py (1)
Foo(27-33)
tests/data/expected/main/jsonschema/use_title_as_name_inline_types.py (2)
src/datamodel_code_generator/model/typed_dict.py (1)
TypedDict(49-114)tests/data/expected/main/jsonschema/use_title_as_name_inline_types_pydantic.py (7)
MyArrayName(17-18)MyObjectName(21-22)MyEnumName(25-27)MyOneOfName(30-31)MyAnyOfName(34-35)MyOneOfConstName(38-40)Foo(43-49)
🪛 Ruff (0.14.10)
src/datamodel_code_generator/parser/jsonschema.py
2938-2938: Unused noqa directive (non-enabled: PLR0912, PLR0914, PLR0915)
Remove unused noqa directive
(RUF100)
tests/data/expected/main/jsonschema/use_title_as_name_inline_types.py
9-9: Cannot use type alias statement on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
12-12: Cannot use type alias statement on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
15-15: Cannot use type alias statement on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
18-18: Cannot use type alias statement on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
21-21: Cannot use type alias statement on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
24-24: Cannot use type alias statement on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
🔇 Additional comments (9)
src/datamodel_code_generator/parser/jsonschema.py (3)
2639-2676: Well-structured helper for determining type alias creation.The logic correctly identifies the cases where a type alias should be created:
- Arrays with title
- anyOf/oneOf unions (excluding const-based enums that would be parsed as literals)
- Objects with only additionalProperties (dict types)
- Enums parsed as literals
The method properly reuses existing methods like
_extract_const_enum_from_combinedandshould_parse_enum_as_literalto maintain consistency with the parsing logic.
2693-2695: LGTM!The integration point is correctly placed after the title-based name sanitization, ensuring that titled inline types are routed to
parse_root_typeto generate proper type aliases.
2985-2991: LGTM!The new dict handling branch correctly extends
parse_root_typeto handle inline objects with onlyadditionalProperties. This enables proper type alias generation for titled dict types liketype MyObjectName = dict[str, str].tests/data/expected/main/jsonschema/use_title_as_name_inline_types_pydantic.py (1)
1-49: LGTM!The expected output correctly demonstrates the fix for issue #2887. Titled inline types now generate named type aliases:
MyArrayNameasRootModel[list[str]]MyObjectNameasRootModel[dict[str, str]]MyEnumNameasEnumMyOneOfNameandMyAnyOfNameas union RootModelsMyOneOfConstNameasStrEnumfor const-based oneOfThe
Foomodel correctly references these named types instead of using inline type definitions.tests/data/expected/main/jsonschema/use_title_as_name_inline_types.py (1)
9-24: Verify Python version target fortypealias syntax.The
typestatement syntax (lines 9, 12, 15, 18, 21, 24) is only available in Python 3.12+. This is flagged by static analysis as invalid for Python 3.10.If this test is intended for Python 3.12+ targets, this is correct. Otherwise, for broader compatibility, the traditional
TypeAliasannotation style would be needed:from typing import TypeAlias MyArrayName: TypeAlias = list[str]Please confirm this test targets Python 3.12+ specifically, or if broader compatibility is required.
src/datamodel_code_generator/__init__.py (1)
460-470: LGTM!The updated Pydantic v2 config creation logic correctly:
- Retrieves expected fields from
config_class.model_fields.keys()- Filters
generate_config.model_dump()to only include parser-relevant fields- Excludes fields already present in
additional_optionsto allow overrides- Merges with
additional_optionsusing the|operatorThis approach properly handles the field filtering without using the
extraargument inmodel_validate, which addresses the Pydantic v2.12.5 compatibility concern mentioned in the PR comments.docs/cli-reference/field-customization.md (1)
3762-3772: The example code shown is correct and doesn't require changes. Theparse_obj()method on line 3770 is the appropriate Pydantic v1 API, which is what the code generator produces by default. Thedatamodel-code-generatortool's defaultoutput_model_typeisPydanticBaseModel(Pydantic v1). When users opt into Pydantic v2 output via--output-model-type pydantic_v2.BaseModel, the code generator automatically generatesmodel_validate()instead. The example demonstrates the default behavior correctly and does not contain a Pydantic API compatibility issue.tests/data/expected/main/jsonschema/titles_use_title_as_name.py (2)
47-51: LGTM! Wrapper model follows expected pattern.The new
ProcessingStatusUnionTitlemodel correctly wraps the inline union type using the__root__pattern, consistent with other wrapper models in the codebase (e.g.,Kind,NestedCommentTitle). This achieves the PR objective of creating named type aliases for inline titled schemas.
54-57: Verify parse_obj behavior with union types and string defaults.The updated field uses a
default_factorythat callsProcessingStatusUnionTitle.parse_obj('COMPLETED'). SinceProcessingStatusUnionTitle.__root__is a union ofProcessingStatusDetail | ExtendedProcessingTask | ProcessingStatusTitle, we need to verify:
- That Pydantic v1's
parse_objcorrectly discriminates the string'COMPLETED'to theProcessingStatusTitleenum member (rather than attempting to parse it as the object types).- That this pattern is compatible with both Pydantic v1 and v2 (the PR comments mention a compatibility issue with v2.12.5 regarding
model_validateand theextraargument).Additionally, note that this change modifies the public API—consumers will now receive a
ProcessingStatusUnionTitlewrapper instead of the raw union type. Ensure this breaking change is intentional and documented.Run the following script to verify the behavior:
#!/bin/bash # Description: Test parse_obj behavior with union types containing enums # Create a test script to validate parse_obj behavior cat > /tmp/test_parse_obj.py << 'EOF' from enum import Enum from pydantic import BaseModel, Field class MyEnum(Enum): COMPLETED = 'COMPLETED' PENDING = 'PENDING' class MyObject(BaseModel): id: int class WrapperUnion(BaseModel): __root__: MyObject | MyEnum # Test 1: Can parse_obj handle string for enum in union? try: result = WrapperUnion.parse_obj('COMPLETED') print(f"✓ parse_obj('COMPLETED') succeeded: {result}") print(f" Root value: {result.__root__}") print(f" Root type: {type(result.__root__)}") except Exception as e: print(f"✗ parse_obj('COMPLETED') failed: {e}") # Test 2: Can it be used in default_factory? class Container(BaseModel): field: WrapperUnion | None = Field( default_factory=lambda: WrapperUnion.parse_obj('COMPLETED') ) try: instance = Container() print(f"\n✓ default_factory succeeded: {instance}") print(f" Field value: {instance.field}") print(f" Field root: {instance.field.__root__ if instance.field else None}") except Exception as e: print(f"\n✗ default_factory failed: {e}") EOF # Run with Python and pydantic python /tmp/test_parse_obj.py
0b487bf to
f13f7dc
Compare
…e is enabled When use_title_as_name is enabled and inline types (array, dict, enum as literal, oneOf/anyOf unions) have a title, type aliases are now created instead of using inline types directly. Also fixes Pydantic v2 compatibility issue where model_validate's extra='ignore' parameter requires Pydantic v2.12.5+. Fixes: #2887
f13f7dc to
c4ee616
Compare
|
@hgl |
Breaking Change AnalysisResult: Breaking changes detected Reasoning: This PR modifies the generated code output when using the Content for Release NotesCode Generation Changes
This analysis was performed by Claude Code Action |
|
🎉 Released in 0.52.0 This PR is now available in the latest release. See the release notes for details. |
Fixes: #2887
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.