Conversation
|
Warning Rate limit exceeded@koxudaxi has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 15 minutes and 9 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 (1)
📝 WalkthroughWalkthroughAdds propagation of OpenAPI/JSON Schema x-* extension fields into template extra data by introducing JsonSchemaParser.set_schema_extensions and calling it at multiple parse points so templates can access schema extensions. Changes
Sequence Diagram(s)(Skipped — change is confined to parser/template data propagation and does not introduce a multi-component control flow requiring visualization.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 |
4dde7d3 to
8a19cab
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/datamodel_code_generator/parser/jsonschema.py (1)
2602-2602: Remove unused noqa directive.Static analysis correctly identifies that the
noqadirective forPLR0912andPLR0915is unused, as noted in the provided hints.🔎 Suggested cleanup
- def parse_root_type( # noqa: PLR0912, PLR0915 + def parse_root_type(tests/main/openapi/test_main_openapi.py (1)
613-635: Consider improving test isolation for cache management.The explicit cache clearing at lines 617-618 suggests potential test isolation concerns:
- Asymmetric cleanup: Caches are cleared before the test but not after, which could affect subsequent tests.
- Inconsistency: Other tests in this file don't clear these caches, which may indicate incomplete isolation.
- Test order dependency risk: Manual cache management can lead to flaky tests if execution order changes.
Recommendation: Consider using a pytest fixture for cache management to ensure consistent cleanup:
Suggested fixture approach
@pytest.fixture def clear_template_cache(): """Clear template caches before and after test to ensure isolation.""" model_base._get_environment.cache_clear() model_base._get_template_with_custom_dir.cache_clear() yield model_base._get_environment.cache_clear() model_base._get_template_with_custom_dir.cache_clear()Then use it in the test:
def test_main_openapi_schema_extensions( capsys: pytest.CaptureFixture, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, clear_template_cache ) -> None: """Test that schema extensions (x-* fields) are passed to custom templates.""" # Cache clearing handled by fixture monkeypatch.chdir(tmp_path) ...However, if this manual approach is intentional for this specific test scenario, the current implementation is functionally correct.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
tests/data/openapi/schema_extensions.yamlis excluded by!tests/data/**/*.yamland included by nonetests/data/templates_extensions/pydantic_v2/BaseModel.jinja2is excluded by none and included by none
📒 Files selected for processing (3)
src/datamodel_code_generator/parser/jsonschema.pytests/data/expected/main/openapi/schema_extensions.pytests/main/openapi/test_main_openapi.py
🧰 Additional context used
🧬 Code graph analysis (3)
tests/data/expected/main/openapi/schema_extensions.py (1)
src/datamodel_code_generator/model/base.py (1)
name(741-743)
src/datamodel_code_generator/parser/jsonschema.py (2)
src/datamodel_code_generator/reference.py (1)
reference(76-78)src/datamodel_code_generator/model/base.py (1)
path(822-824)
tests/main/openapi/test_main_openapi.py (4)
src/datamodel_code_generator/model/base.py (2)
_get_environment(405-420)_get_template_with_custom_dir(424-436)src/datamodel_code_generator/__init__.py (1)
chdir(154-164)tests/conftest.py (1)
freeze_time(285-288)tests/main/conftest.py (1)
run_main_and_assert(244-408)
🪛 Ruff (0.14.10)
src/datamodel_code_generator/parser/jsonschema.py
2602-2602: Unused noqa directive (non-enabled: PLR0912, 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). (9)
- GitHub Check: benchmarks
- GitHub Check: py312-isort6 on Ubuntu
- GitHub Check: 3.13 on Ubuntu
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.10 on macOS
- GitHub Check: py312-isort7 on Ubuntu
- GitHub Check: 3.11 on Ubuntu
- GitHub Check: 3.14 on Ubuntu
- GitHub Check: Analyze (python)
🔇 Additional comments (4)
src/datamodel_code_generator/parser/jsonschema.py (2)
1158-1163: LGTM! Well-designed helper method.The implementation correctly filters x-* extension fields from
obj.extrasand stores them inextra_template_dataonly when extensions exist. This follows the same pattern as other metadata setters in the class and avoids polluting the template data with empty dictionaries.
1010-1010: Excellent coverage of extension propagation points.The
set_schema_extensionscalls are well-placed across all major parsing flows (variant models, objects, arrays, enums, root types, and allOf combinations). The consistent pattern of calling withreference.pathafter reference creation ensures schema extensions are available for all model types.Also applies to: 1904-1904, 2056-2056, 2110-2110, 2281-2281, 2563-2563, 2664-2664, 2729-2729, 2852-2852, 2925-2925
tests/data/expected/main/openapi/schema_extensions.py (1)
1-19: LGTM! Test fixture demonstrates the feature working correctly.This expected output validates that schema extensions are successfully passed to templates and used to generate custom model attributes (
__collection__). The models are valid Pydantic v2 code and demonstrate a realistic use case for the new feature.tests/main/openapi/test_main_openapi.py (1)
32-32: LGTM! Import placement is correct.The import is properly positioned within the datamodel_code_generator imports section and uses a clear alias.
CodSpeed Performance ReportMerging #2790 will not alter performanceComparing Summary
Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2790 +/- ##
========================================
Coverage 99.47% 99.48%
========================================
Files 88 88
Lines 13256 13386 +130
Branches 1557 1566 +9
========================================
+ Hits 13187 13317 +130
Misses 36 36
Partials 33 33
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: 0
🧹 Nitpick comments (1)
docs/custom_template.md (1)
86-86: Use Markdown headings instead of bold emphasis for section labels.Lines 86, 111, 122, and 129 use
**bold**formatting for what are effectively subsection labels (file names and command descriptions). To improve document structure and follow Markdown best practices, convert these to proper###headings.🔎 Proposed fixes
-**api.yaml** +### api.yaml -**custom_templates/pydantic_v2/BaseModel.jinja2** +### custom_templates/pydantic_v2/BaseModel.jinja2 -**Run the generator** +### Run the generator -**Generated Output** +### Generated OutputThis improves document accessibility and consistency, as the earlier "Example: Database Model Configuration" section already uses proper heading levels (lines 82).
Also applies to: 111-111, 122-122, 129-129
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/custom_template.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/custom_template.md
86-86: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
111-111: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
122-122: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
129-129: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ 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). (20)
- GitHub Check: 3.14 on Windows
- GitHub Check: py312-isort5 on Ubuntu
- GitHub Check: py312-black23 on Ubuntu
- GitHub Check: py312-isort6 on Ubuntu
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.11 on Ubuntu
- GitHub Check: 3.12 on macOS
- GitHub Check: 3.11 on macOS
- GitHub Check: py312-pydantic1 on Ubuntu
- GitHub Check: 3.10 on macOS
- GitHub Check: 3.12 on Ubuntu
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.13 on macOS
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.14 on Ubuntu
- GitHub Check: 3.13 on Windows
- GitHub Check: pkg_meta
- GitHub Check: benchmarks
- GitHub Check: Analyze (python)
- GitHub Check: update-readme
🔇 Additional comments (1)
docs/custom_template.md (1)
78-159: Comprehensive and clear documentation of the schema extensions feature.The new "Schema Extensions" section effectively communicates the feature with a practical example, clear template access patterns, availability reference table, and use cases. The progression from schema definition → template code → command execution → generated output is well-structured for user understanding.
Document how x-* schema extensions are passed to custom templates via the extensions variable, with examples for database model configuration and other use cases. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
5ce0cd7 to
628eab7
Compare
Breaking Change AnalysisResult: No breaking changes detected Reasoning: This PR adds a new This analysis was performed by Claude Code Action |
* Pass schema extensions to templates * Move model_base import to top of file * Add schema extensions documentation Document how x-* schema extensions are passed to custom templates via the extensions variable, with examples for database model configuration and other use cases. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
* Add --collapse-root-models-name-strategy option * docs: update CLI reference documentation and prompt data 🤖 Generated by GitHub Actions * Add pragma no cover for defensive edge cases * Achieve 100% diff coverage for collapse-root-models-name-strategy * Use cast instead of type ignore comment * Remove line comments from collapse-root-models implementation * Add complex e2e tests for collapse-root-models-name-strategy * Update reference metadata when renaming in parent strategy * Refactor collapse-root-models tests to use parameterization for v1/v2 * Add schema path context to error messages (#2786) * Return str or dict when output=None in generate() (#2787) * Add --http-timeout CLI option (#2788) * Add --http-timeout CLI option for configurable HTTP request timeout * docs: update CLI reference documentation and prompt data 🤖 Generated by GitHub Actions --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Pass schema extensions to templates (#2790) * Pass schema extensions to templates * Move model_base import to top of file * Add schema extensions documentation Document how x-* schema extensions are passed to custom templates via the extensions variable, with examples for database model configuration and other use cases. 🤖 Generated with [Claude Code](https://claude.com/claude-code) * Add propertyNames and x-propertyNames support (#2789) * Add propertyNames and x-propertyNames support * Fix Pydantic v1 compatibility for x-propertyNames Use the model_validate utility function from util module instead of calling JsonSchemaObject.model_validate() directly, which only exists in Pydantic v2. 🤖 Generated with [Claude Code](https://claude.com/claude-code) * Add test for x-propertyNames non-dict branch coverage Test that x-propertyNames with non-dict value (e.g., boolean) is correctly ignored, achieving 100% diff coverage. 🤖 Generated with [Claude Code](https://claude.com/claude-code) * Add support for additional_imports in extra-template-data JSON (#2793) * Update zensical to 0.0.15 (#2794) * Add --use-field-description-example option (#2792) * Add --use-field-description-example option * docs: update CLI reference documentation and prompt data 🤖 Generated by GitHub Actions * Add tests for complete branch coverage of docstring property --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Fix formatting in test file --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Fixes: #2158
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.