Repository navigation
Conversation
A nested dataclass field was built as a plain Schema around All(inner, _Constructor). A plain Schema sees only the All, so it never armed under any policy, and DataclassSchema.compile() generated the outer mapping alone: every level below it stayed interpreted, which is why compiling a dataclass tree barely moved its numbers. The builder now makes a fused dataclass Schema subclass for every level, carrying the compile logic DataclassSchema had. It arms and generates like any schema, and compiling it compiles the nested dataclass schemas under its fields first (through a list, a dict, Maybe, All, or a union), so a tree generates all the way down. The outer compile flag is threaded into every nested schema, so compile=False keeps the whole tree interpreted. A Schema the user placed in an Annotated keeps its own flag and is not entered. A second compile() is a no-op instead of stacking validators. A schema on a recursion cycle must not compile: a generated validator bails to its interpreted twin on a failure, and the twin re-enters the generated code below it, so every compiled level on a cycle doubles a failing validation once per turn, exponential in the data's depth. Taking a recursive reference now marks every schema from its target down to the field taking it, and a marked schema never arms or generates; the schema above the cycle still does. A regression test pins the deep failure to linear time. On a three-level API response the compiled path drops from about 22µs to about 10µs per document. A nested-dataclass CodSpeed pair tracks it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ANcsDvtNPxCRTk33uagPak
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 9 minutes. View limit details
📝 Walkthrough
Merge Risk: 🟡 Moderate · up to
|
Merging this PR will not alter performance
Performance Changes
Comparing |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
302dbc38-a26c-4815-964f-8ff1f8fd8d0e
📒 Files selected for processing: 6
bench/test_benchmarks.pydocs/src/content/docs/guides/compiled-schemas.mdsrc/probatio/dataclass_schema.pysrc/probatio/decorator.pytests/__snapshots__/test_public_surface.ambrtests/test_codegen.py
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout: 1
- GitHub Check: Python 3.12
⚠️ CI failures not shown inline: 2
GitHub Actions: PR Labels / 0_Verify.txt: Compile nested dataclass schemas along with the outer one
Conclusion: failure
##[group]Run ludeeus/action-require-labels@be19cd7f04c6fad46a85336f9e9cebdf961807bb
with:
labels: breaking-change, bugfix, documentation, enhancement, refactor, performance, new-feature, maintenance, ci, dependencies
##[endgroup]
##[error] No labels defined on the pull request.
GitHub Actions: PR Labels / Verify: Compile nested dataclass schemas along with the outer one
Conclusion: failure
##[group]Run ludeeus/action-require-labels@be19cd7f04c6fad46a85336f9e9cebdf961807bb
with:
labels: breaking-change, bugfix, documentation, enhancement, refactor, performance, new-feature, maintenance, ci, dependencies
##[endgroup]
##[error] No labels defined on the pull request.
🧰 Additional context used
📚 Code guidelines: 1
AGENTS.md — auto-discovered
📓 Path-based instructions: 1
Source excerpt: This project is public and read closely.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/guides/compiled-schemas.md
🔇 Additional comments: 6
src/probatio/dataclass_schema.py: 1
117-125: LGTM!Also applies to: 444-463, 872-878, 966-1019, 1194-1273
src/probatio/decorator.py: 1
151-153: LGTM!Also applies to: 187-189
tests/__snapshots__/test_public_surface.ambr: 1
1837-1837: LGTM!Also applies to: 1847-1847
tests/test_codegen.py: 1
752-946: LGTM!bench/test_benchmarks.py: 1
370-413: LGTM!Also applies to: 421-421
docs/src/content/docs/guides/compiled-schemas.md: 1
66-69: LGTM!Also applies to: 129-131
The nested schema for a TypedDict on a recursion cycle could enter the generated path: under the ON policy already, and now also through the compile=True flag the outer schema threads down. That is the same cascade the dataclass rule guards against, a failing validation exponential in the data's depth (about 13 seconds at 20 levels). The inner schema is now built with compile=False when its reference closes a cycle, so it never arms under any policy; the top-level TypedDictSchema above the cycle still compiles. A regression test pins a failure 24 levels deep to linear time. Raised by the CodeRabbit review on the pull request. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ANcsDvtNPxCRTk33uagPak
Breaking change
Proposed change
A nested dataclass field was built as a plain Schema around All(inner, _Constructor). A plain Schema sees only the All, so it never armed under any policy, and DataclassSchema.compile() generated the outer mapping alone: every level below it stayed interpreted, which is why compiling a dataclass tree barely moved its numbers.
The builder now makes a fused dataclass Schema subclass for every level, carrying the compile logic DataclassSchema had. It arms and generates like any schema, and compiling it compiles the nested dataclass schemas under its fields first (through a list, a dict, Maybe, All, or a union), so a tree generates all the way down. The outer compile flag is threaded into every nested schema, so compile=False keeps the whole tree interpreted. A Schema the user placed in an Annotated keeps its own flag and is not entered. A second compile() is a no-op instead of stacking validators.
A schema on a recursion cycle must not compile: a generated validator bails to its interpreted twin on a failure, and the twin re-enters the generated code below it, so every compiled level on a cycle doubles a failing validation once per turn, exponential in the data's depth. Taking a recursive reference now marks every schema from its target down to the field taking it, and a marked schema never arms or generates; the schema above the cycle still does. A regression test pins the deep failure to linear time.
On a three-level API response the compiled path drops from about 22µs to about 10µs per document. A nested-dataclass CodSpeed pair tracks it.
Type of change
Additional information
Checklist
uv run --no-sync just testpasses locally. A pull request cannot be merged unless CI is green.uv run --no-sync just lintpasses.uv run --no-sync just typecheckpasses.