Generate tested docs examples#3519
Conversation
WalkthroughAdds a script to render and replace marker-delimited docs examples, wires it into tox and workflow checks, adds tests and fixtures, and regenerates the OpenAPI and model-reuse documentation examples. ChangesDocs Examples Generation Pipeline
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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-3519.datamodel-code-generator.pages.dev |
Merging this PR will not alter performance
|
![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)
docs/openapi.md (1)
374-422: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep the allOf example narrative in sync.
The regenerated schema now adds
updated_atandapi_key, but the explanatory note below still only mentionscreated_at. Please update the surrounding prose so the example and description match the new request/response split.🤖 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 `@docs/openapi.md` around lines 374 - 422, The allOf example narrative is out of sync with the regenerated schema in openapi.read-only-write-only-allof.schema, which now includes additional readOnly/writeOnly fields like updated_at and api_key. Update the surrounding prose near the User/Timestamps/Credentials example so it describes the full request/response split reflected by the current Timestamps and Credentials schemas, not just created_at.
🧹 Nitpick comments (4)
scripts/build_docs_examples.py (3)
76-89: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAvoid drift between tree listing and rendered file contents.
The directory tree (Line 82) is derived dynamically from
output_path.iterdir(), but the per-file content dump (Lines 84-88) uses a hardcoded tuple of filenames. If the fixture directory gains/loses a file, the tree and content sections will silently disagree.♻️ Proposed fix to keep tree and content in sync
def render_reuse_scope_tree_example() -> str: """Render tested reuse-scope tree output with representative files.""" output_path = EXPECTED_MAIN / "jsonschema" / "reuse_scope_tree" + files = sorted(file for file in output_path.iterdir() if file.is_file()) lines = [ "", "**Output with `--reuse-scope tree`:**", fenced("text", directory_tree(output_path, root_name="models")), ] - for file_name in ("schema_a.py", "schema_b.py", "shared.py"): + for file in files: lines.extend(( - f"**models/{file_name}:**", - fenced("python", read_python_output(output_path / file_name)), + f"**models/{file.name}:**", + fenced("python", read_python_output(file)), )) return "\n".join(lines)🤖 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/build_docs_examples.py` around lines 76 - 89, The reuse-scope example in render_reuse_scope_tree_example can drift because the directory tree is generated from the fixture directory while the rendered file blocks use a hardcoded filename list. Update the function to derive the per-file sections from the same source as directory_tree(output_path, root_name="models")—for example by iterating the fixture contents or a shared filename list—so both the tree and the file content stay in sync when files change.
190-215: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReport all out-of-date/missing-marker files in a single run.
Both the missing-marker branch (Line 199) and the
--checkout-of-date branch (Line 202-205) return immediately, so only the first problematic file is reported per invocation — the CI/dev workflow needs repeated reruns to discover every issue acrossdocs/model-reuse.mdanddocs/openapi.md.♻️ Proposed fix to accumulate issues across all paths
def update_docs_examples(*, check: bool = False) -> int: """Update or check generated documentation examples.""" changed_paths = [] + out_of_date_paths = [] + missing_markers = False for path, examples in iter_examples_by_path(): content = path.read_text(encoding="utf-8") updated_content = content for example in examples: updated_content, found = replace_example(updated_content, example) if not found: - return 1 + missing_markers = True if updated_content == content: continue if check: - print(f"Docs examples are out of date: {path}", file=sys.stderr) - print("Run 'python scripts/build_docs_examples.py' to update.", file=sys.stderr) - return 1 + out_of_date_paths.append(path) + continue path.write_text(updated_content, encoding="utf-8") changed_paths.append(path) + if out_of_date_paths: + for path in out_of_date_paths: + print(f"Docs examples are out of date: {path}", file=sys.stderr) + print("Run 'python scripts/build_docs_examples.py' to update.", file=sys.stderr) + if missing_markers or out_of_date_paths: + return 1 + if changed_paths: for path in changed_paths: print(f"Updated {path}") return 0 print("Docs examples are up to date.") return 0🤖 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/build_docs_examples.py` around lines 190 - 215, The update_docs_examples function exits too early, so it only reports the first missing-marker or out-of-date docs file instead of all affected paths. Adjust update_docs_examples to keep scanning every path returned by iter_examples_by_path(), track any files with missing markers or mismatched generated content, and only decide the final exit status after the loop. Use the existing update_docs_examples, iter_examples_by_path, and replace_example flow to collect and print all problematic paths in one run.
92-165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce boilerplate in the registry with a small builder helper.
Several entries repeat the same
render=lambda: render_file_example(language=..., path=..., strip_python_header=...)shape. Extracting a helper would shrink the registry and make adding future examples less error-prone.♻️ Proposed helper to cut duplication
+def _file_example( + example_id: str, + doc: str, + *, + language: str, + path: Path, + strip_python_header: bool = False, +) -> DocsExample: + """Build a DocsExample backed by render_file_example.""" + return DocsExample( + example_id=example_id, + path=DOCS / doc, + render=lambda: render_file_example(language=language, path=path, strip_python_header=strip_python_header), + ) + + def docs_examples() -> tuple[DocsExample, ...]: """Return all generated docs example sections.""" return ( - DocsExample( - example_id="openapi.quick-start.output", - path=DOCS / "openapi.md", - render=lambda: render_file_example( - language="python", - path=EXPECTED_MAIN / "openapi" / "general.py", - strip_python_header=True, - ), - ), + _file_example( + "openapi.quick-start.output", + "openapi.md", + language="python", + path=EXPECTED_MAIN / "openapi" / "general.py", + strip_python_header=True, + ), ...🤖 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/build_docs_examples.py` around lines 92 - 165, The docs_examples registry repeats the same render lambda pattern for many DocsExample entries, making the list verbose and harder to extend. Add a small helper around render_file_example in build_docs_examples.py and use it from docs_examples() for the repeated python/yaml/json entries, keeping the existing example_id values and strip_python_header behavior unchanged. This will centralize the builder logic and make future DocsExample additions less error-prone.tests/test_build_docs_examples_script.py (1)
22-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
update_docs_examples's write-mode and missing-marker branches.Current tests only exercise the
--checksuccess path and pure rendering functions. Neither the actual write path (check=Falsewhen content differs) nor the "markers not found" branch (replace_examplereturningFalse) is covered.🤖 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 `@tests/test_build_docs_examples_script.py` around lines 22 - 34, Add tests for the unverified branches in update_docs_examples: cover the write-mode path when check=False and generated content differs, and cover the missing-marker branch where replace_example returns False. Extend the existing test module around update_docs_examples, replace_example, and any helper used to drive file updates so the actual mutation path and failure/no-op path are both exercised, not just the render-only and --check success cases.
🤖 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 `@docs/openapi.md`:
- Around line 374-422: The allOf example narrative is out of sync with the
regenerated schema in openapi.read-only-write-only-allof.schema, which now
includes additional readOnly/writeOnly fields like updated_at and api_key.
Update the surrounding prose near the User/Timestamps/Credentials example so it
describes the full request/response split reflected by the current Timestamps
and Credentials schemas, not just created_at.
---
Nitpick comments:
In `@scripts/build_docs_examples.py`:
- Around line 76-89: The reuse-scope example in render_reuse_scope_tree_example
can drift because the directory tree is generated from the fixture directory
while the rendered file blocks use a hardcoded filename list. Update the
function to derive the per-file sections from the same source as
directory_tree(output_path, root_name="models")—for example by iterating the
fixture contents or a shared filename list—so both the tree and the file content
stay in sync when files change.
- Around line 190-215: The update_docs_examples function exits too early, so it
only reports the first missing-marker or out-of-date docs file instead of all
affected paths. Adjust update_docs_examples to keep scanning every path returned
by iter_examples_by_path(), track any files with missing markers or mismatched
generated content, and only decide the final exit status after the loop. Use the
existing update_docs_examples, iter_examples_by_path, and replace_example flow
to collect and print all problematic paths in one run.
- Around line 92-165: The docs_examples registry repeats the same render lambda
pattern for many DocsExample entries, making the list verbose and harder to
extend. Add a small helper around render_file_example in build_docs_examples.py
and use it from docs_examples() for the repeated python/yaml/json entries,
keeping the existing example_id values and strip_python_header behavior
unchanged. This will centralize the builder logic and make future DocsExample
additions less error-prone.
In `@tests/test_build_docs_examples_script.py`:
- Around line 22-34: Add tests for the unverified branches in
update_docs_examples: cover the write-mode path when check=False and generated
content differs, and cover the missing-marker branch where replace_example
returns False. Extend the existing test module around update_docs_examples,
replace_example, and any helper used to drive file updates so the actual
mutation path and failure/no-op path are both exercised, not just the
render-only and --check success cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e8fe6464-18a9-4749-bb08-2042f8c284a5
⛔ Files ignored due to path filters (3)
docs/llms-full.txtis excluded by none and included by nonetests/data/expected/docs_examples/docs_examples_registry.txtis excluded by!tests/data/**/*.txtand included by nonetests/data/expected/docs_examples/reuse_scope_tree_example.txtis excluded by!tests/data/**/*.txtand included by none
📒 Files selected for processing (6)
.github/workflows/generated-docs-sync.yamldocs/model-reuse.mddocs/openapi.mdscripts/build_docs_examples.pytests/test_build_docs_examples_script.pytox.ini
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3519 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 155 156 +1
Lines 34076 34130 +54
Branches 3992 3992
=========================================
+ Hits 34076 34130 +54
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
docs/tested-examples
branch
from
002f6ef to
a9d6e18
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/build_docs_examples.py (2)
66-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
directory_filesincludes any file type, not just generated Python output.If the fixture directory (e.g.
reuse_scope_tree) ever gains a non-code artifact (e.g..gitkeep,__pycache__cleanup miss,.pyc), it will be silently included in the rendered tree/content without validation. Low risk given fixtures are curated, but worth a defensive filter if this pipeline is reused for other directories later.🤖 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/build_docs_examples.py` around lines 66 - 68, The `directory_files` helper currently returns every file under the fixture directory, which can accidentally include non-generated artifacts. Update `directory_files` in `build_docs_examples.py` to defensively filter for the intended generated Python outputs only, and keep the existing stable sorting behavior so callers like the docs example rendering pipeline don’t pick up stray files such as `.gitkeep` or `.pyc`.
202-239: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMissing-marker failures discard already-computed valid updates.
In
update_docs_examples, whenmissing_marker_for_pathis set for any file, the loopcontinues only for that file, butmissing_markersis set globally. The function then checksif missing_markers or out_of_date_paths: return 1at Line 229 — before ever writingchanged_content_by_path(Line 232). This means: if even one file has a marker mismatch, all other files with legitimately updated content are silently never written, with no message indicating they were dropped.For a build script this may be intentional fail-fast behavior, but it's worth confirming this is the desired contract (vs. writing valid updates and reporting only the problematic file).
♻️ Possible fix: write valid updates regardless of unrelated missing markers
- for path in out_of_date_paths: - print(f"Docs examples are out of date: {path}", file=sys.stderr) - if out_of_date_paths: - print("Run 'python scripts/build_docs_examples.py' to update.", file=sys.stderr) - if missing_markers or out_of_date_paths: - return 1 - - if changed_content_by_path: - for path, updated_content in changed_content_by_path.items(): - path.write_text(updated_content, encoding="utf-8") - print(f"Updated {path}") - return 0 + for path in out_of_date_paths: + print(f"Docs examples are out of date: {path}", file=sys.stderr) + if out_of_date_paths: + print("Run 'python scripts/build_docs_examples.py' to update.", file=sys.stderr) + + if not check and changed_content_by_path: + for path, updated_content in changed_content_by_path.items(): + path.write_text(updated_content, encoding="utf-8") + print(f"Updated {path}") + + if missing_markers or out_of_date_paths: + return 1🤖 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/build_docs_examples.py` around lines 202 - 239, The `update_docs_examples` function is failing fast on any `missing_markers` before it writes `changed_content_by_path`, which drops otherwise valid updates for unrelated paths. In `scripts/build_docs_examples.py`, adjust `update_docs_examples` so the write loop for `changed_content_by_path` runs before the final error return, or otherwise ensure `missing_marker_for_path` only blocks that specific file; keep the existing `iter_examples_by_path`, `replace_example`, and `changed_content_by_path` flow intact while preserving clear reporting for paths with missing markers.
🤖 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/build_docs_examples.py`:
- Around line 66-68: The `directory_files` helper currently returns every file
under the fixture directory, which can accidentally include non-generated
artifacts. Update `directory_files` in `build_docs_examples.py` to defensively
filter for the intended generated Python outputs only, and keep the existing
stable sorting behavior so callers like the docs example rendering pipeline
don’t pick up stray files such as `.gitkeep` or `.pyc`.
- Around line 202-239: The `update_docs_examples` function is failing fast on
any `missing_markers` before it writes `changed_content_by_path`, which drops
otherwise valid updates for unrelated paths. In
`scripts/build_docs_examples.py`, adjust `update_docs_examples` so the write
loop for `changed_content_by_path` runs before the final error return, or
otherwise ensure `missing_marker_for_path` only blocks that specific file; keep
the existing `iter_examples_by_path`, `replace_example`, and
`changed_content_by_path` flow intact while preserving clear reporting for paths
with missing markers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a953eedf-8947-400c-a698-76bb9a3bfbba
📒 Files selected for processing (13)
.github/workflows/generated-docs-sync.yamldocs/llms-full.txtdocs/model-reuse.mddocs/openapi.mdscripts/build_docs_examples.pytests/data/expected/docs_examples/docs_example_render_helpers.txttests/data/expected/docs_examples/docs_examples_registry.txttests/data/expected/docs_examples/main_check_up_to_date.txttests/data/expected/docs_examples/reuse_scope_tree_example.txttests/data/expected/docs_examples/update_docs_examples_check_problems.txttests/data/expected/docs_examples/update_docs_examples_write_mode.txttests/test_build_docs_examples_script.pytox.ini
✅ Files skipped from review due to trivial changes (6)
- tests/data/expected/docs_examples/update_docs_examples_check_problems.txt
- tests/data/expected/docs_examples/docs_examples_registry.txt
- tests/data/expected/docs_examples/main_check_up_to_date.txt
- tests/data/expected/docs_examples/update_docs_examples_write_mode.txt
- docs/openapi.md
- docs/llms-full.txt
🚧 Files skipped from review as they are similar to previous changes (3)
- tox.ini
- .github/workflows/generated-docs-sync.yaml
- docs/model-reuse.md
Breaking Change AnalysisResult: No breaking changes detected Reasoning: This PR only touches documentation tooling and content. It adds a new build script (scripts/build_docs_examples.py), its tests and expected fixtures, a new tox This analysis was performed by Claude Code Action |
|
🎉 Released in 0.67.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