Add release benchmark docs#3465
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new ChangesRelease Benchmark Pipeline
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant resolve as select_release_benchmark_versions.py
participant collect as collect_release_benchmarks.py
participant venv as isolated venv
participant dcg as datamodel-code-generator
participant merge as merge_release_benchmarks.py
participant build as build_release_benchmark_docs.py
participant artifacts as GitHub Artifacts
GH->>resolve: load PyPI releases and download stats
resolve->>artifacts: upload selection.json
GH->>collect: per-version matrix from selection
collect->>venv: create fresh venv + uv pip install
venv-->>collect: installed release
loop each case / formatter
collect->>dcg: warmup + measured subprocess runs
dcg-->>collect: timing stats or error
end
collect->>artifacts: upload per-version fragments
GH->>merge: download selection + fragments
merge->>merge: consolidate entries and metadata
merge->>artifacts: upload merged release-benchmarks.json
GH->>build: render Markdown and SVG from merged JSON
build->>artifacts: upload docs directory
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 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 |
Contributor
|
📚 Docs Preview: https://pr-3465.datamodel-code-generator.pages.dev |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Merging this PR will not alter performance
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3465 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 150 151 +1
Lines 32422 32484 +62
Branches 3780 3780
=========================================
+ Hits 32422 32484 +62
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
release-benchmark-docs
branch
from
356f987 to
3bf4515
Compare
koxudaxi
added
the
run-release-benchmarks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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.
Inline comments:
In @.github/workflows/release-benchmarks.yaml:
- Around line 90-94: The shell command in the collect_release_benchmarks.py
script invocation directly interpolates GitHub expressions
(steps.versions.outputs.versions and github.event_name == 'workflow_dispatch' &&
inputs.runs || '7') into single-quoted arguments, creating a potential security
vulnerability if those values contain special characters. Refactor this by
adding an env section to the step to set environment variables for these values,
then reference those environment variables in the run command using the pattern
env.VARIABLE_NAME instead of direct GitHub expression interpolation.
In `@scripts/collect_release_benchmarks.py`:
- Around line 341-344: The CLI argument parser accepts numeric arguments
(--runs, --warmups, --timeout, --install-retries) without validation, allowing
zero or negative values that cause failures in median/min/max calculations. Add
validation logic in the main() function after parsing arguments to ensure that
--runs, --warmups, --timeout, and --install-retries are all greater than zero.
If any argument fails validation, raise an appropriate error or exit with a
descriptive message to prevent execution with invalid configuration.
- Around line 208-216: The _failure_status function's "ruff" formatter case only
checks for "not found" and "invalid choice" but does not check for "unrecognized
arguments", which older Ruff releases may return. When this occurs, the code
falls through to the default case and incorrectly returns STATUS_FAILED instead
of STATUS_UNSUPPORTED. Add "unrecognized arguments" to the condition check in
the "ruff" case (alongside the existing "not found" and "invalid choice" checks)
to properly classify Ruff formatter argument incompatibility as unsupported.
- Around line 114-122: The _run_subprocess function does not handle the
TimeoutExpired exception that subprocess.run() raises when the timeout parameter
is exceeded, causing the entire script to crash on a slow subprocess instead of
gracefully recording a failed result. Wrap the subprocess.run() call inside
_run_subprocess in a try/except block that catches subprocess.TimeoutExpired and
returns a CompletedProcess object with returncode set to 124 (the standard Unix
timeout exit code), allowing the callers to handle timeouts the same way they
already handle non-zero return codes from the subprocess.
In `@tests/test_build_release_benchmark_docs_script.py`:
- Around line 73-79: The test reads generated files (docs_path and svg_path)
without first verifying that the subprocess completed successfully, which can
mask the actual CLI failure with a FileNotFoundError. Add an explicit assertion
to verify the result object's return code equals zero before calling
_completed_process_output and reading the generated files. This ensures that if
the subprocess fails, the test will report the actual CLI failure rather than a
file-not-found error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2bab4d3d-7fc1-4cd4-b0a5-b58a99bdb09e
⛔ Files ignored due to path filters (8)
docs/assets/benchmarks/release-benchmarks.svgis excluded by!**/*.svgand included by nonedocs/data/release-benchmarks.jsonis excluded by none and included by nonedocs/llms-full.txtis excluded by none and included by nonedocs/llms.txtis excluded by none and included by nonetests/data/expected/release_benchmark_docs/build_release_benchmark_docs_check.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/release_benchmark_cli_outputs.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/release_benchmark_rendered.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/release_benchmarks/sample.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (9)
.github/workflows/generated-docs-sync.yaml.github/workflows/llms-txt.yaml.github/workflows/release-benchmarks.yamldocs/performance-benchmarks.mdscripts/build_release_benchmark_docs.pyscripts/collect_release_benchmarks.pytests/test_build_release_benchmark_docs_script.pytox.inizensical.toml
koxudaxi
force-pushed
the
release-benchmark-docs
branch
2 times, most recently
from
3a6fa2f to
fa3e163
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In @.github/workflows/release-benchmarks.yaml:
- Around line 75-85: The workflow is using head_branch directly from
workflow_run events without a fallback mechanism, which is unreliable when
triggered by tag pushes. Add a new environment variable WORKFLOW_SHA to capture
the workflow run's head_sha, then modify the shell script logic in the "Select
versions" step to include a fallback that uses git describe to resolve the tag
from the commit SHA when the versions variable is empty after the workflow_run
check. This ensures that if head_branch is unavailable or blank, the script will
attempt to retrieve the tag information from the commit itself using the git
API.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 71f73fc9-0772-4fb3-a79c-662d47f06d8e
⛔ Files ignored due to path filters (17)
docs/assets/benchmarks/release-benchmarks.svgis excluded by!**/*.svgand included by nonedocs/data/release-benchmarks.jsonis excluded by none and included by nonedocs/llms-full.txtis excluded by none and included by nonedocs/llms.txtis excluded by none and included by nonetests/data/expected/release_benchmark_docs/build_release_benchmark_docs_check.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/collect_release_benchmarks_failures.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/collect_release_benchmarks_invalid_args.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/merge_release_benchmarks_cli_outputs.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/release_benchmark_cli_outputs.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/release_benchmark_rendered.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/select_release_benchmark_versions_cli_outputs.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/release_benchmarks/clickpy_versions.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/release_benchmarks/pypi_project.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/release_benchmarks/pypistats_overall.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/release_benchmarks/pypistats_recent.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/release_benchmarks/release_fragment_0641.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/release_benchmarks/sample.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (14)
.github/workflows/generated-docs-sync.yaml.github/workflows/llms-txt.yaml.github/workflows/release-benchmarks.yamlREADME.mddocs/index.mddocs/performance-benchmarks.mdscripts/build_preset_docs.pyscripts/build_release_benchmark_docs.pyscripts/collect_release_benchmarks.pyscripts/merge_release_benchmarks.pyscripts/select_release_benchmark_versions.pytests/test_build_release_benchmark_docs_script.pytox.inizensical.toml
✅ Files skipped from review due to trivial changes (4)
- zensical.toml
- README.md
- docs/performance-benchmarks.md
- docs/index.md
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/llms-txt.yaml
- .github/workflows/generated-docs-sync.yaml
- scripts/build_preset_docs.py
- tox.ini
koxudaxi
force-pushed
the
release-benchmark-docs
branch
from
fa3e163 to
2237462
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/select_release_benchmark_versions.py (1)
133-146: 💤 Low valueConsider parameterized query construction for defense-in-depth.
While the current implementation is safe because
PACKAGE_NAMEis a hardcoded constant andhistory_days/limitare validated positive integers, the f-string SQL construction pattern is generally discouraged. For defense-in-depth, consider separating the query template from variable interpolation.That said, given this is a read-only query against a public demo endpoint with fully controlled inputs, the actual risk is minimal.
🤖 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 `@scripts/select_release_benchmark_versions.py` around lines 133 - 146, The query variable uses f-string interpolation to construct the SQL query, which is discouraged for defense-in-depth. Refactor by first defining the SQL query template as a separate string with clear placeholders or comments indicating where variables will be substituted, then perform the variable substitution separately (using string formatting or concatenation) rather than embedding it directly in an f-string. This separates the query structure from the variable values and improves code readability and security posture, even though the current inputs are controlled.
🤖 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.
Nitpick comments:
In `@scripts/select_release_benchmark_versions.py`:
- Around line 133-146: The query variable uses f-string interpolation to
construct the SQL query, which is discouraged for defense-in-depth. Refactor by
first defining the SQL query template as a separate string with clear
placeholders or comments indicating where variables will be substituted, then
perform the variable substitution separately (using string formatting or
concatenation) rather than embedding it directly in an f-string. This separates
the query structure from the variable values and improves code readability and
security posture, even though the current inputs are controlled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c0bfd41-d487-4a28-9a72-09005a6dcd7f
⛔ Files ignored due to path filters (21)
docs/assets/benchmarks/release-benchmarks-large-jsonschema.svgis excluded by!**/*.svgand included by nonedocs/assets/benchmarks/release-benchmarks-large-openapi.svgis excluded by!**/*.svgand included by nonedocs/assets/benchmarks/release-benchmarks-small-openapi.svgis excluded by!**/*.svgand included by nonedocs/assets/benchmarks/release-benchmarks.svgis excluded by!**/*.svgand included by nonedocs/data/release-benchmarks.jsonis excluded by none and included by nonedocs/llms-full.txtis excluded by none and included by nonedocs/llms.txtis excluded by none and included by nonetests/data/expected/release_benchmark_docs/build_release_benchmark_docs_check.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/collect_release_benchmarks_failures.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/collect_release_benchmarks_invalid_args.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/merge_release_benchmarks_cli_outputs.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/release_benchmark_cli_outputs.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/release_benchmark_error_helpers.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/release_benchmark_rendered.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/release_benchmark_docs/select_release_benchmark_versions_cli_outputs.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/release_benchmarks/clickpy_versions.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/release_benchmarks/pypi_project.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/release_benchmarks/pypistats_overall.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/release_benchmarks/pypistats_recent.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/release_benchmarks/release_fragment_0641.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/release_benchmarks/sample.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (15)
.github/workflows/generated-docs-sync.yaml.github/workflows/llms-txt.yaml.github/workflows/release-benchmarks.yamlREADME.mddocs/index.mddocs/performance-benchmarks.mdscripts/build_preset_docs.pyscripts/build_release_benchmark_docs.pyscripts/collect_release_benchmarks.pyscripts/merge_release_benchmarks.pyscripts/release_benchmark_errors.pyscripts/select_release_benchmark_versions.pytests/test_build_release_benchmark_docs_script.pytox.inizensical.toml
✅ Files skipped from review due to trivial changes (4)
- README.md
- zensical.toml
- docs/index.md
- docs/performance-benchmarks.md
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/llms-txt.yaml
- .github/workflows/generated-docs-sync.yaml
- tox.ini
- scripts/build_preset_docs.py
koxudaxi
force-pushed
the
release-benchmark-docs
branch
from
2237462 to
3790e49
Compare
koxudaxi
force-pushed
the
release-benchmark-docs
branch
from
3790e49 to
3c83177
Compare
Contributor
Breaking Change AnalysisResult: No breaking changes detected Reasoning: PR #3465 only adds release benchmark documentation and tooling. All changed files are CI workflows, docs, benchmark scripts/data, tests for those scripts, and build config (tox.ini, zensical.toml). Nothing in the diff modifies the datamodel_code_generator package source, CLI options of the tool, generated code output, Jinja2 templates, error handling, or supported Python versions. The single deleted line is a docstring fragment in scripts/build_preset_docs.py that was extended with appended text (a new "Performance Benchmarks" link), not removed behavior. No options, parameters, classes, enum values, or functions appear as removed lines. Therefore there are no breaking changes for users of the code generator. This analysis was performed by Claude Code Action |
Contributor
|
🎉 Released in 0.65.1 This PR is now available in the latest release. See the release notes for details. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit