Use builtin formatter in CI#3380
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 (10)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughAdds token-based inline-comment detection and PEP 695 placeholder parsing to the builtin formatter, and introduces pytest marker/fixture and test-helper wiring to optionally isolate and apply a builtin formatter configuration for generation tests. ChangesComment detection and test isolation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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-3380.datamodel-code-generator.pages.dev |
Merging this PR will improve performance by 12.23%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | test_perf_deep_nested |
5.5 s | 4.8 s | +13.44% |
| ⚡ | WallTime | test_perf_openapi_large |
3.1 s | 2.7 s | +12.88% |
| ⚡ | WallTime | test_perf_all_options_enabled |
5.7 s | 5 s | +12.6% |
| ⚡ | WallTime | test_perf_large_models_pydantic_v2 |
3.8 s | 3.4 s | +11.17% |
| ⚡ | WallTime | test_perf_complex_refs |
2.1 s | 1.9 s | +11.1% |
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 ci-builtin-default-formatters (089c9c0) with main (0c49f24)
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/datamodel_code_generator/_builtin_formatter.py (1)
1300-1300:⚠️ Potential issue | 🟡 MinorUse
_has_comment_tokenfor remaining inline-comment skipping checks.
_format_generated_class_definition(line 1300) and_format_generated_module_statement(line 1332) still use naive"#" in lineguards; replace them with_has_comment_token(line)for consistency with_format_generated_class_statement(line 1042) and to avoid skipping formatting when#appears inside string literals.🤖 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 `@src/datamodel_code_generator/_builtin_formatter.py` at line 1300, Replace the naive " '#' in line" guard with the helper _has_comment_token(line) in both _format_generated_class_definition and _format_generated_module_statement so they mirror _format_generated_class_statement; specifically change the conditional that currently reads like "if len(line) <= line_length or '#' in line:" to use "_has_comment_token(line)" (keeping the len(line) <= line_length check intact) so inline-comment detection uses the centralized helper.
🤖 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.
Outside diff comments:
In `@src/datamodel_code_generator/_builtin_formatter.py`:
- Line 1300: Replace the naive " '#' in line" guard with the helper
_has_comment_token(line) in both _format_generated_class_definition and
_format_generated_module_statement so they mirror
_format_generated_class_statement; specifically change the conditional that
currently reads like "if len(line) <= line_length or '#' in line:" to use
"_has_comment_token(line)" (keeping the len(line) <= line_length check intact)
so inline-comment detection uses the centralized helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c78d7e48-7916-4269-81e8-b7c7ed58b17f
📒 Files selected for processing (10)
pyproject.tomlsrc/datamodel_code_generator/_builtin_formatter.pytests/conftest.pytests/main/conftest.pytests/main/jsonschema/test_main_jsonschema.pytests/main/openapi/test_main_openapi.pytests/test_conftest_helpers.pytests/test_format.pytests/test_main_kr.pytox.ini
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3380 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 129 129
Lines 28571 28832 +261
Branches 3453 3484 +31
==========================================
+ Hits 28571 28832 +261
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
ci-builtin-default-formatters
branch
from
f7c08eb to
089c9c0
Compare
Breaking Change AnalysisResult: No breaking changes detected Reasoning: The authoritative prepared diff could not be accessed: the Read tool permission for /home/runner/work/_temp/release-draft-pr/ was not granted, all Bash routes to that path were sandbox-blocked (working directory only), the local git checkout is a shallow single root commit (d7857fb has no parent, so git show renders the whole repo as additions and cannot reconstruct the real diff), and gh/network fetch requires unavailable approval. With no visibility into any removed lines, the ground rules forbid asserting any removal of options, parameters, classes, enum values, or functions. The only reliable signal — the merged commit metadata 'Use builtin formatter in CI (#3380)' with sub-commits 'Use builtin formatter in CI', 'Handle PEP 695 aliases in builtin formatter', and 'Cover builtin formatter CI helpers' — indicates a CI-tooling/builtin-formatter-helper change rather than a user-facing API, CLI, or generated-code default change. Absent any evidence of a breaking change and unable to confirm any removed lines, I report no breaking changes, with the caveat that this rests on PR metadata, not the diff itself. This analysis was performed by Claude Code Action |
|
🎉 Released in 0.64.0 This PR is now available in the latest release. See the release notes for details. |
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests