Conversation
Reviewer's GuideAdjusts Pydantic V2 nullable schema handling so optional fields no longer generate anyOf[type, null] but instead emit just the underlying type, adds tests to validate AsyncAPI schema generation for various optional field patterns, and simplifies the Nix shell development environment dependencies. Sequence diagram for nullable_schema override in optional field generationsequenceDiagram
participant Caller
participant CompatJsonSchemaGenerator
participant BaseJsonSchemaGenerator
Caller->>CompatJsonSchemaGenerator: nullable_schema(schema)
alt PYDANTIC_V2 is true and schema contains key schema
CompatJsonSchemaGenerator->>CompatJsonSchemaGenerator: inner_schema = schema.get(schema)
CompatJsonSchemaGenerator->>CompatJsonSchemaGenerator: generate_inner(inner_schema)
CompatJsonSchemaGenerator-->>Caller: JsonSchemaValue without null anyOf
else Fallback to base implementation
CompatJsonSchemaGenerator->>BaseJsonSchemaGenerator: nullable_schema(schema)
BaseJsonSchemaGenerator-->>CompatJsonSchemaGenerator: JsonSchemaValue (possibly anyOf[type, null])
CompatJsonSchemaGenerator-->>Caller: JsonSchemaValue
end
Class diagram for updated nullable_schema handlingclassDiagram
class BaseJsonSchemaGenerator {
+nullable_schema(schema: JsonSchemaValue) JsonSchemaValue
}
class CompatJsonSchemaGenerator {
+PYDANTIC_V2 bool
+generate_inner(schema: JsonSchemaValue) JsonSchemaValue
+nullable_schema(schema: JsonSchemaValue) JsonSchemaValue
}
BaseJsonSchemaGenerator <|-- CompatJsonSchemaGenerator
class JsonSchemaValue {
}
CompatJsonSchemaGenerator : +nullable_schema(schema) JsonSchemaValue
CompatJsonSchemaGenerator : when PYDANTIC_V2 and schema has key schema
CompatJsonSchemaGenerator : extract inner_schema
CompatJsonSchemaGenerator : return generate_inner(inner_schema)
CompatJsonSchemaGenerator : otherwise call super.nullable_schema(schema)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a Pydantic‑v2 aware override for nullable JSON schema generation that flattens Optional/Union nullable types, removes several development tool dependencies from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `natsapi/_compat.py:239-247` </location>
<code_context>
"""
return value
+
+ def nullable_schema(self, schema: JsonSchemaValue) -> JsonSchemaValue:
+ """
+ Override nullable schema generation to flatten optional types.
+ Instead of generating anyOf with [type, null], just return the type.
+ """
+ if PYDANTIC_V2:
+ # Extract the inner schema and generate it without the null variant
+ inner_schema = schema.get('schema')
+ if inner_schema:
+ return self.generate_inner(inner_schema)
+ return super().nullable_schema(schema)
</code_context>
<issue_to_address>
**suggestion:** Consider checking for the presence of the 'schema' key rather than its truthiness to avoid skipping valid but empty inner schemas.
Because `inner_schema = schema.get('schema')` is checked with `if inner_schema:`, an empty dict (or other falsy but valid value) will be treated as if there is no inner schema and will fall back to `super().nullable_schema(schema)`. If empty dicts are valid inner schemas, this creates inconsistent behavior. Using `if 'schema' in schema:` and then `self.generate_inner(schema['schema'])` will consistently apply the new behavior whenever an inner schema is present.
</issue_to_address>
### Comment 2
<location> `tests/asyncapi/test_generation.py:150-154` </location>
<code_context>
+ schema = app.asyncapi_schema
+
+ assert schema["components"]["schemas"]["User"]["properties"]["mandatory_property_1"]["type"] == "string"
+ assert schema["components"]["schemas"]["User"]["properties"]["optional_property_1"]["type"] == "string"
+ assert schema["components"]["schemas"]["User"]["properties"]["optional_property_2"]["type"] == "string"
+ assert schema["components"]["schemas"]["User"]["properties"]["optional_property_3"]["type"] == "string"
+ assert schema["components"]["schemas"]["User"]["properties"]["optional_property_4"]["type"] == "string"
+ assert schema["components"]["schemas"]["User"]["properties"]["optional_property_5"]["type"] == "string"
+ assert schema["components"]["schemas"]["User"]["properties"]["optional_property_6"]["type"] == "string"
+
</code_context>
<issue_to_address>
**suggestion (testing):** Assert explicitly that `anyOf` with a `null` schema is no longer generated for optional fields.
These assertions don’t actually verify the bug is fixed, because they would still pass if the field were wrapped in an `anyOf` that includes `"type": "string"`. Please also assert that `anyOf` is absent from the optional field schemas, e.g.:
```python
type_schema = schema["components"]["schemas"]["User"]["properties"]["optional_property_1"]
assert "anyOf" not in type_schema
assert type_schema.get("type") == "string"
```
(and similarly for a representative set of optional fields), so the test will fail if `anyOf` reappears.
```suggestion
app.include_router(user_router)
app.generate_asyncapi()
schema = app.asyncapi_schema
user_properties = schema["components"]["schemas"]["User"]["properties"]
# Mandatory field should be a plain string type
assert user_properties["mandatory_property_1"]["type"] == "string"
# Optional fields should not be wrapped in anyOf and should have a plain string type
optional_1_schema = user_properties["optional_property_1"]
assert "anyOf" not in optional_1_schema
assert optional_1_schema.get("type") == "string"
optional_2_schema = user_properties["optional_property_2"]
assert "anyOf" not in optional_2_schema
assert optional_2_schema.get("type") == "string"
optional_3_schema = user_properties["optional_property_3"]
assert "anyOf" not in optional_3_schema
assert optional_3_schema.get("type") == "string"
optional_4_schema = user_properties["optional_property_4"]
assert "anyOf" not in optional_4_schema
assert optional_4_schema.get("type") == "string"
optional_5_schema = user_properties["optional_property_5"]
assert "anyOf" not in optional_5_schema
assert optional_5_schema.get("type") == "string"
optional_6_schema = user_properties["optional_property_6"]
assert "anyOf" not in optional_6_schema
assert optional_6_schema.get("type") == "string"
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/asyncapi/test_generation.py`:
- Around line 129-136: In the User model, fix the nullable type annotations:
change optional_property_6 from "str = None" to a nullable type such as
"Optional[str] = None" or "str | None = None" so the default None matches the
type, and simplify the redundant union on optional_property_4 (currently
"Optional[str] | None = None") to just "Optional[str] = None" (or "str | None =
None") to remove redundancy; update the annotations for optional_property_4 and
optional_property_6 in the User class accordingly.
LanderMoerkerke
approved these changes
Feb 6, 2026
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.
Fix issues with 'AnyOf' type generation for optional types and just render the actual type instead.
Summary by Sourcery
Adjust AsyncAPI schema generation to flatten optional types and add coverage ensuring optional fields are rendered as simple string types rather than anyOf variants.
Enhancements:
Build:
Tests:
Summary by CodeRabbit
Bug Fixes
Chores
Tests