Skip unused formatter construction#3599
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 selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesParser formatting now skips default formatter construction when Formatter behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant generate
participant Parser.parse
participant CodeFormatter
Caller->>generate: generate with formatters=[]
generate->>Parser.parse: parse input schema
Parser.parse->>Parser.parse: inspect formatter configuration
Parser.parse->>CodeFormatter: build formatter when custom or overridden
Parser.parse-->>Caller: generated model output
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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-3599.datamodel-code-generator.pages.dev |
Merging this PR will degrade performance by 25.46%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | test_perf_multiple_files_input |
2.5 s | 3.6 s | -28.91% |
| ❌ | WallTime | test_perf_large_models_pydantic_v2_builtin |
661.1 ms | 901 ms | -26.63% |
| ❌ | WallTime | test_perf_deep_nested |
3.7 s | 5.1 s | -26.55% |
| ❌ | WallTime | test_perf_all_options_enabled |
3.9 s | 5.3 s | -26.32% |
| ❌ | WallTime | test_perf_graphql_style_pydantic_v2 |
576.8 ms | 779.8 ms | -26.03% |
| ❌ | WallTime | test_perf_complex_refs |
1.4 s | 1.9 s | -25.89% |
| ❌ | WallTime | test_perf_large_models_pydantic_v2 |
2.6 s | 3.5 s | -25.78% |
| ❌ | WallTime | test_perf_duplicate_names |
757.7 ms | 1,010.4 ms | -25.01% |
| ❌ | WallTime | test_perf_large_models_pydantic_v2_noformat |
634.9 ms | 844.1 ms | -24.78% |
| ❌ | WallTime | test_perf_openapi_large |
2.2 s | 2.9 s | -24.22% |
| ❌ | WallTime | test_perf_stripe_style_pydantic_v2 |
1.5 s | 1.9 s | -24.05% |
| ❌ | WallTime | test_perf_kubernetes_style_pydantic_v2 |
1.9 s | 2.5 s | -23.43% |
| ❌ | WallTime | test_perf_aws_style_openapi_pydantic_v2 |
1.4 s | 1.9 s | -23.16% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing agent/skip-unused-formatter (1a0087d) with main (01303c3)
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 #3599 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 164 164
Lines 35325 35367 +42
Branches 4078 4079 +1
=========================================
+ Hits 35325 35367 +42
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:
|
Breaking Change AnalysisResult: No breaking changes detected Reasoning: PR #3599 ("Skip unused formatter construction") is an internal performance optimization. In parse(), it skips building a code formatter only when self.formatters is an empty list AND there is no custom formatter AND _build_code_formatter has not been overridden on the instance or a subclass. In that specific case, building a CodeFormatter with an empty formatter list applies no transformations, so skipping it produces byte-identical generated output — the change avoids importing the formatter runtime (verified by the new test asserting imported_format is false). All override/escape-hatch paths (custom formatters, subclass overrides, instance-injected builders) are explicitly preserved and covered by new tests. No CLI options, API parameters, classes, enum values, or functions were removed (deleted lines are just the reformatted if-block). No default values or behavior changed for existing users. Therefore there are no breaking changes. This analysis was performed by Claude Code Action |
Summary by CodeRabbit
Performance Improvements
Bug Fixes
Tests