Address second round of code review feedback#261
Conversation
- Clarify display_name schema comment: document that schema constraints and domain validation are separate (schema allows spaces, domain checks non-empty when trimmed) - Restructure ErrorCode sync test with pattern matching for compile-time exhaustiveness; adding new variants now causes build errors - Make assert_object_schema_has_field more defensive: handle $ref, AllOf, OneOf, AnyOf, Array with diagnostic messages - Add unit tests for UserSchema constraints (uuid format, min/max length, pattern) - Add BDD scenario for User schema validation constraints - Fix "Open API" typo to "OpenAPI Specification" - Fix "serialises" spelling to "serializes" (en-GB-oxendict) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Reviewer's GuideStrengthens OpenAPI schema verification for User and ErrorCode types by making schema inspection helpers more defensive, adding focused unit and BDD tests for UserSchema constraints, and performing minor documentation and spelling cleanups. Sequence diagram for ErrorCode schema serialization verificationsequenceDiagram
participant T as error_code_schema_matches_domain_serialization_test
participant H as expected_serialization
participant S as serde_json
T->>T: define variants = [InvalidRequest, Unauthorized,
T->>T: Forbidden, NotFound, InternalError]
loop for each code in variants
T->>S: to_string(code)
S-->>T: serialized_json_string
T->>H: expected_serialization(code)
H-->>T: expected_string
T->>T: assert serialized_json_string == "\"expected_string\""
end
note over T: Pattern matching in expected_serialization
note over T: causes compile-time failure if
note over T: a new ErrorCode variant is added without
note over T: updating the match arms
Updated class diagram for OpenAPI schema test helpers and related typesclassDiagram
class ApiDoc {
+openapi() OpenApi
}
class OpenApi {
+components ComponentsOption
}
class ComponentsOption {
+schemas Map~String,RefOr_Schema_~
}
class RefOr_Schema_ {
}
class Schema {
}
class ObjectSchema {
+properties Map~String,RefOr_Schema_~
+min_length Option~u32~
+max_length Option~u32~
+pattern Option~String~
+format Option~SchemaFormat~
}
class SchemaFormat {
<<enumeration>>
+Custom(String)
}
class UserSchema {
-id String
-display_name String
}
class ErrorCode {
<<enumeration>>
+InvalidRequest
+Unauthorized
+Forbidden
+NotFound
+InternalError
}
class TestHelpers {
+assert_object_schema_has_field(schema RefOr_Schema_, schema_name String, field String) void
+unwrap_object_schema(schema RefOr_Schema_, name String) ObjectSchema
+expected_serialization(code ErrorCode) String
}
ApiDoc --> OpenApi : produces
OpenApi --> ComponentsOption : has
ComponentsOption --> RefOr_Schema_ : contains
RefOr_Schema_ --> Schema : wraps
Schema <|-- ObjectSchema : variant
ObjectSchema --> SchemaFormat : uses
UserSchema --> ObjectSchema : generates_schema
UserSchema --> ApiDoc : included_in
ErrorCode ..> TestHelpers : passed_to
TestHelpers ..> ObjectSchema : inspects
TestHelpers ..> RefOr_Schema_ : inspects
TestHelpers ..> ErrorCode : matches_variants
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughSummarise added test-support OpenAPI helpers and richer schema assertions: introduce Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (4)**/*.{rs,ts,tsx,js,jsx,py}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,ts,tsx,js,jsx,py,md}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (1)backend/tests/openapi_schemas_bdd.rs (1)
🔍 Remote MCPThe Ref tool is not properly configured. Let me try using web search to gather relevant technical context:Based on my search of relevant documentation, here is a summary of contextual information helpful for reviewing this PR: Technical ContextUtoipa OpenAPI Schema Validation FrameworkThe utoipa crate provides auto-generated OpenAPI documentation for Rust REST APIs using a code-first approach with simple macros, and also contains Rust types of the OpenAPI spec allowing you to write the OpenAPI spec using Rust. This is the framework being enhanced in this PR. Schema Constraint Support: Utoipa's ToSchema derive macro supports validation attributes including max_length, min_length, and pattern constraints on fields. This directly relates to the PR's changes adding constraints for Schema Format Types: Utoipa's KnownFormat enum includes a Uuid variant used with String values to indicate value is in UUID format, which aligns with the PR's addition of uuid format validation for the RefOr Pattern: Utoipa uses a RefOr pattern representing either a Ref or some other type T, which is the core pattern the PR's helper functions ( Cucumber BDD Testing Framework for RustCucumber is a specification for running tests in a BDD (behavior-driven development) style workflow using Gherkin language for human-readable test syntax. Cucumber tests are more likely to take the form of integration, functional or E2E testing. World Pattern: The World object holds the state of Cucumber tests during execution, which is the pattern the PR uses with Code Quality ContextHelper Function Consolidation: The PR's proposed refactoring with Defensive Schema Handling: The enhancements to ⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (4)
Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The OpenAPI User schema traversal logic is duplicated in multiple places (e.g.
unwrap_object_schemaindoc.rstests and the BDD step definitions inopenapi_schemas_bdd.rs); consider extracting a shared helper for resolvingRefOr<Schema>toObjectto keep the assertions DRY and easier to update if the schema shape changes. assert_object_schema_has_fieldprovides detailed diagnostics for non-Objectschemas, whileunwrap_object_schemaand the BDD steps use simplerpanic!messages; aligning these helpers (or reusing one in terms of the other) would give more consistent and descriptive failures when the schema is not anObject.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The OpenAPI User schema traversal logic is duplicated in multiple places (e.g. `unwrap_object_schema` in `doc.rs` tests and the BDD step definitions in `openapi_schemas_bdd.rs`); consider extracting a shared helper for resolving `RefOr<Schema>` to `Object` to keep the assertions DRY and easier to update if the schema shape changes.
- `assert_object_schema_has_field` provides detailed diagnostics for non-`Object` schemas, while `unwrap_object_schema` and the BDD steps use simpler `panic!` messages; aligning these helpers (or reusing one in terms of the other) would give more consistent and descriptive failures when the schema is not an `Object`.
## Individual Comments
### Comment 1
<location> `backend/src/doc.rs:162-179` </location>
<code_context>
+ }
+
+ #[test]
+ fn openapi_user_id_has_uuid_format() {
+ use utoipa::openapi::schema::SchemaFormat;
+
+ let doc = ApiDoc::openapi();
+ let schemas = &doc.components.as_ref().expect("components").schemas;
+ let user_schema = schemas.get(USER_SCHEMA_NAME).expect("User schema");
+ let obj = unwrap_object_schema(user_schema, USER_SCHEMA_NAME);
+
+ let id_prop = obj.properties.get("id").expect("id property exists");
+ let id_obj = unwrap_object_schema(id_prop, "id");
+
+ // Schema format is set via #[schema(format = "uuid")] which produces Custom variant
</code_context>
<issue_to_address>
**suggestion (testing):** The UUID format assertion should likely target a string schema instead of unwrapping to an object.
Here you unwrap `id_prop` as an object and then read `id_obj.format`, but in utoipa a UUID `format` is attached to `Schema::String`, not `Object`. Instead, match on `RefOr::T(Schema::String(s))` (or use a small helper for string schemas) and assert on `s.format` so the test matches utoipa’s schema shape and avoids panics on non-object fields.
```suggestion
#[test]
fn openapi_user_id_has_uuid_format() {
use utoipa::openapi::schema::{Schema, SchemaFormat};
let doc = ApiDoc::openapi();
let schemas = &doc.components.as_ref().expect("components").schemas;
let user_schema = schemas.get(USER_SCHEMA_NAME).expect("User schema");
let obj = unwrap_object_schema(user_schema, USER_SCHEMA_NAME);
let id_prop = obj.properties.get("id").expect("id property exists");
let id_schema = match id_prop {
RefOr::T(Schema::String(s)) => s,
_ => panic!("schema 'id' is not a String"),
};
// Schema format is set via #[schema(format = "uuid")] which produces Custom variant
assert!(
matches!(&id_schema.format, Some(SchemaFormat::Custom(s)) if s == "uuid"),
"id should have format=uuid"
);
}
```
</issue_to_address>
### Comment 2
<location> `backend/src/doc.rs:182-191` </location>
<code_context>
+ }
+
+ #[test]
+ fn openapi_user_display_name_has_constraints() {
+ let doc = ApiDoc::openapi();
+ let schemas = &doc.components.as_ref().expect("components").schemas;
+ let user_schema = schemas.get(USER_SCHEMA_NAME).expect("User schema");
+ let obj = unwrap_object_schema(user_schema, USER_SCHEMA_NAME);
+
+ let display_name_prop = obj
+ .properties
+ .get("display_name")
+ .expect("display_name property exists");
+ let display_name_obj = unwrap_object_schema(display_name_prop, "display_name");
+
+ assert_eq!(
</code_context>
<issue_to_address>
**suggestion (testing):** Length and pattern checks on `display_name` should probably use the string schema variant rather than forcing an object.
Because `display_name` is a `String` with schema attributes, utoipa will typically produce a `Schema::String` with `min_length`, `max_length`, and `pattern` set directly, not an `Object`. Forcing an object here could panic if the schema shape changes. Consider either:
- a helper that matches `Schema::String` and asserts `min_length`/`max_length`/`pattern`, or
- a helper that can handle both string and object schemas.
That way the test remains robust to the actual schema representation of this field.
Suggested implementation:
```rust
let display_name_prop = obj
.properties
.get("display_name")
.expect("display_name property exists");
match display_name_prop {
utoipa::openapi::schema::Schema::String(string_schema) => {
assert_eq!(
string_schema.min_length,
Some(3),
"display_name should have min_length=3"
);
assert_eq!(
string_schema.max_length,
Some(32),
"display_name should have max_length=32"
);
assert!(
string_schema.pattern.is_some(),
"display_name should have a pattern constraint"
);
}
utoipa::openapi::schema::Schema::Object(obj_schema) => {
assert_eq!(
obj_schema.min_length,
Some(3),
"display_name should have min_length=3"
);
assert_eq!(
obj_schema.max_length,
Some(32),
"display_name should have max_length=32"
);
assert!(
obj_schema.pattern.is_some(),
"display_name should have a pattern constraint"
);
}
other => panic!(
"display_name should be represented as a string or object schema; got {:?}",
other
),
}
```
1. If there are any remaining assertions below this snippet that still reference `display_name_obj` or assume an object schema, update them to:
- either be removed (if redundant with the new checks), or
- be moved into the `match` arms, using `string_schema` / `obj_schema` as appropriate.
2. If you want to assert the exact pattern rather than just "some pattern", replace the `pattern.is_some()` assertions with `assert_eq!(..., Some("your-regex-here".to_string()), "...")` in both match arms.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: backend/tests/openapi_schemas_bdd.rs Comment on lines +157 to +193 fn user_display_name_has_length_constraints(world: &Mutex<OpenApiWorld>) {
use utoipa::openapi::schema::Schema;
use utoipa::openapi::RefOr;
let world = world.lock().expect("world lock");
let doc = world.document.as_ref().expect("document generated");
let components = doc.components.as_ref().expect("components present");
let user_schema = components
.schemas
.get(USER_SCHEMA_NAME)
.expect("User schema");
let obj = match user_schema {
RefOr::T(Schema::Object(obj)) => obj,
_ => panic!("User schema is not an Object"),
};
let display_name_prop = obj
.properties
.get("display_name")
.expect("display_name property");
let display_name_obj = match display_name_prop {
RefOr::T(Schema::Object(obj)) => obj,
_ => panic!("display_name property is not an Object"),
};
assert_eq!(
display_name_obj.min_length,
Some(3),
"User.display_name should have min_length=3"
);
assert_eq!(
display_name_obj.max_length,
Some(32),
"User.display_name should have max_length=32"
);
}❌ New issue: Code Duplication |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
leynos
force-pushed
the
domain-ports-feedback
branch
from
bf09afb to
c494182
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![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
♻️ Duplicate comments (3)
backend/tests/openapi_schemas_bdd.rs (2)
129-196: Verify that string properties are correctly handled as Object schemas.Past reviewers raised concerns that
UserSchemadefinesidanddisplay_nameasStringfields (lines 77, 90 in backend/src/inbound/http/schemas.rs), and utoipa should emitSchema::Stringfor these properties, notSchema::Object. If that's the case,unwrap_object_schemawill panic on every test run.Confirm that the tests pass and that utoipa v5.0.0 generates Object schemas (with format, min_length, max_length, pattern fields) for string properties with validation constraints. If the tests panic, refactor to match
Schema::Stringinstead.Run the following to verify test behaviour:
#!/bin/bash # Verify the new OpenAPI schema validation tests execute without panicking cd backend cargo test --test openapi_schemas_bdd -- --nocapture
151-196: Extract shared schema traversal logic.The two functions
user_display_name_has_length_constraintsanduser_display_name_has_pattern_constraintshare nearly identical structure (lock world, retrieve document, get User schema, unwrap to object, get display_name property). Extract a helper function that accepts a closure to eliminate duplication and reduce maintenance burden.Proposed refactor with helper function
Add this helper after line 56:
+/// Invoke closure with User display_name Object schema. +fn with_user_property_object_schema<F>( + world: &Mutex<OpenApiWorld>, + property_name: &str, + f: F, +) where + F: FnOnce(&utoipa::openapi::schema::Object), +{ + let world = world.lock().expect("world lock"); + let doc = world.document.as_ref().expect("document generated"); + let components = doc.components.as_ref().expect("components present"); + let user_schema = components + .schemas + .get(USER_SCHEMA_NAME) + .expect("User schema"); + let obj = unwrap_object_schema(user_schema, USER_SCHEMA_NAME); + let property_obj = unwrap_object_schema(get_property(obj, property_name), property_name); + f(property_obj); +}Refactor the three test functions:
#[then("the User id field has uuid format")] fn user_id_has_uuid_format(world: &Mutex<OpenApiWorld>) { use utoipa::openapi::schema::SchemaFormat; - - let world = world.lock().expect("world lock"); - let doc = world.document.as_ref().expect("document generated"); - let components = doc.components.as_ref().expect("components present"); - let user_schema = components - .schemas - .get(USER_SCHEMA_NAME) - .expect("User schema"); - - let obj = unwrap_object_schema(user_schema, USER_SCHEMA_NAME); - let id_obj = unwrap_object_schema(get_property(obj, "id"), "User.id"); - - // Schema format is set via #[schema(format = "uuid")] which produces Custom variant - assert!( - matches!(&id_obj.format, Some(SchemaFormat::Custom(s)) if s == "uuid"), - "User.id should have format=uuid" - ); + with_user_property_object_schema(world, "id", |id_obj| { + assert!( + matches!(&id_obj.format, Some(SchemaFormat::Custom(s)) if s == "uuid"), + "User.id should have format=uuid" + ); + }); } #[then("the User display_name field has length constraints")] fn user_display_name_has_length_constraints(world: &Mutex<OpenApiWorld>) { - let world = world.lock().expect("world lock"); - let doc = world.document.as_ref().expect("document generated"); - let components = doc.components.as_ref().expect("components present"); - let user_schema = components - .schemas - .get(USER_SCHEMA_NAME) - .expect("User schema"); - - let obj = unwrap_object_schema(user_schema, USER_SCHEMA_NAME); - let display_name_obj = - unwrap_object_schema(get_property(obj, "display_name"), "User.display_name"); - - assert_eq!( - display_name_obj.min_length, - Some(3), - "User.display_name should have min_length=3" - ); - assert_eq!( - display_name_obj.max_length, - Some(32), - "User.display_name should have max_length=32" - ); + with_user_property_object_schema(world, "display_name", |display_name_obj| { + assert_eq!( + display_name_obj.min_length, + Some(3), + "User.display_name should have min_length=3" + ); + assert_eq!( + display_name_obj.max_length, + Some(32), + "User.display_name should have max_length=32" + ); + }); } #[then("the User display_name field has pattern constraint")] fn user_display_name_has_pattern_constraint(world: &Mutex<OpenApiWorld>) { - let world = world.lock().expect("world lock"); - let doc = world.document.as_ref().expect("document generated"); - let components = doc.components.as_ref().expect("components present"); - let user_schema = components - .schemas - .get(USER_SCHEMA_NAME) - .expect("User schema"); - - let obj = unwrap_object_schema(user_schema, USER_SCHEMA_NAME); - let display_name_obj = - unwrap_object_schema(get_property(obj, "display_name"), "User.display_name"); - - assert_eq!( - display_name_obj.pattern.as_deref(), - Some("^[A-Za-z0-9_ ]+$"), - "User.display_name should have pattern constraint" - ); + with_user_property_object_schema(world, "display_name", |display_name_obj| { + assert_eq!( + display_name_obj.pattern.as_deref(), + Some("^[A-Za-z0-9_ ]+$"), + "User.display_name should have pattern constraint" + ); + }); }backend/src/doc.rs (1)
182-229: Verify that string properties are correctly handled as Object schemas.Past reviewers flagged that
UserSchemadefinesidanddisplay_nameasStringfields withvalue_type = String(backend/src/inbound/http/schemas.rs lines 73-74, 84), and utoipa should emitSchema::Stringfor these properties. If that's the case,unwrap_object_schemawill panic when called on these properties.Confirm that the tests pass and that utoipa v5.0.0 generates Object schemas (with format, min_length, max_length, pattern fields) for string properties with validation constraints. If the tests panic, refactor to match
Schema::Stringand read fromstring_schema.format,string_schema.min_length, etc.Run the following to verify test behaviour:
#!/bin/bash # Verify the new OpenAPI schema validation tests execute without panicking cd backend cargo test --lib doc::tests::openapi_user_id_has_uuid_format -- --nocapture cargo test --lib doc::tests::openapi_user_display_name_has_constraints -- --nocapture
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
backend/src/doc.rs(3 hunks)backend/src/inbound/http/schemas.rs(3 hunks)backend/tests/features/openapi_schemas.feature(1 hunks)backend/tests/openapi_schemas_bdd.rs(2 hunks)backend/tests/openapi_support.rs(1 hunks)docs/execplans/phase-0-update-modules-to-use-domain-ports.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{rs,ts,tsx,js,jsx,py,md}
📄 CodeRabbit inference engine (AGENTS.md)
Use consistent spelling and grammar with en-GB-oxendict ("-ize" / "-yse" / "-our") spelling and grammar, except for references to external APIs.
Files:
docs/execplans/phase-0-update-modules-to-use-domain-ports.mdbackend/src/inbound/http/schemas.rsbackend/tests/openapi_schemas_bdd.rsbackend/tests/openapi_support.rsbackend/src/doc.rs
docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/**/*.md: Use the markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
Proactively update documentation indocs/when new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve to ensure accuracy and currency.
Documentation must use en-GB-oxendict ("-ize" / "-yse" / "-our") spelling and grammar, with the exception of the "LICENSE" file which should remain unchanged.Follow the Documentation style guide for clear, consistent documentation conventions
Files:
docs/execplans/phase-0-update-modules-to-use-domain-ports.md
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: Validate Markdown files usingmake markdownlint.
Runmake fmtafter any documentation changes to format Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by runningmake nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns; code blocks at 120 columns; tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Files:
docs/execplans/phase-0-update-modules-to-use-domain-ports.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/execplans/phase-0-update-modules-to-use-domain-ports.md
**/*.{rs,ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,ts,tsx,js,jsx,py}: Comment why, not what—explain assumptions, edge cases, trade-offs, or complexity; do not echo the obvious.
Favour clarity over cleverness—be concise but prefer explicit over terse or obscure idioms; prioritise code that is easy to follow.
Use functions and composition to avoid repetition; prefer generators or comprehensions and declarative code to imperative repetition when readable.
Keep functions small and meaningful—single responsibility, clear purpose, and obey command/query segregation.
Name things precisely—use clear, descriptive variable and function names; for booleans, prefer names withis,has, orshould.
Structure logically—each file should encapsulate a coherent module; group related code close together.
Illustrate with clear examples in function documentation demonstrating usage and outcome; for tests, omit examples where the example only reiterates test logic.
Keep file size manageable—no single code file may exceed 400 lines; break up long switch statements and move large test data blocks to external files.
New functionality or changes in behaviour must be fully validated by relevant unittests and behavioural tests before committing.
When fixing a bug, provide a unittest demonstrating the corrected behaviour to validate the fix and guard against regression.
Files:
backend/src/inbound/http/schemas.rsbackend/tests/openapi_schemas_bdd.rsbackend/tests/openapi_support.rsbackend/src/doc.rs
**/*.{rs,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Group by feature, not layer—colocate views, logic, fixtures, and helpers related to a domain concept rather than splitting by type.
Files:
backend/src/inbound/http/schemas.rsbackend/tests/openapi_schemas_bdd.rsbackend/tests/openapi_support.rsbackend/src/doc.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Runmake fmt,make lint, andmake testbefore committing Rust code; these wrapcargo fmt,cargo clippy, andcargo test.
Clippy warnings must be disallowed; fix any warnings emitted during tests in the code itself rather than silencing them.
For excessively long functions, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
For functions with too many parameters, group related parameters in meaningfully named structs.
Where a function returns a large error, consider usingArcto reduce the amount of data returned.
Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated withcargo doc.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments, not before.
Do not usereturnin single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Lints must not be silenced except as a last resort; lint rule suppressions must be tightly scoped and include a clear reason.
Preferexpectoverallowwhen suppressing warnings.
Where a function is unused with specific features selected, use conditional compilation with#[cfg]or#[cfg_attr].
Prefer.expect()over.unwrap().
Useconcat!()to combine long string literals rather than escaping newlines with a backslash.
Prefer single-line versions of functions where appropriate (e.g.,pub fn new(id: u64) -> Self { Self(id) }).
Write unit and behavioural tests for new Rust functionality; run both before and after making any change.
Userstestfixtures for shared setup in Rust tests.
Replace duplicated tests with#[rstest(...)]parameterised cases in Rust.
Prefermockallfor mocks/stubs...
Files:
backend/src/inbound/http/schemas.rsbackend/tests/openapi_schemas_bdd.rsbackend/tests/openapi_support.rsbackend/src/doc.rs
⚙️ CodeRabbit configuration file
**/*.rs: * Seek to keep the cognitive complexity of functions no more than 9.
- Adhere to single responsibility and CQRS
- Place function attributes after doc comments.
- Do not use
returnin single-line functions.- Move conditionals with >2 branches into a predicate function.
- Avoid
unsafeunless absolutely necessary.- Every module must begin with a
//!doc comment that explains the module's purpose and utility.- Comments and docs must follow en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.- Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
- Use
rstestfixtures for shared setup and to avoid repetition between tests.- Replace duplicated tests with
#[rstest(...)]parameterised cases.- Prefer
mockallfor mocks/stubs.- Prefer
.expect()over.unwrap()in tests..expect()and.unwrap()are forbidden outside of tests. Errors must be propagated.- Ensure that any API or behavioural changes are reflected in the documentation in
docs/- Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/- Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
- Environment access (env::set_var and env::remove_var) are always unsafe in Rust 2024 and MUST be marked as such
- For testing of functionality depending upon environment variables, dependency injection and...
Files:
backend/src/inbound/http/schemas.rsbackend/tests/openapi_schemas_bdd.rsbackend/tests/openapi_support.rsbackend/src/doc.rs
🧠 Learnings (2)
📚 Learning: 2025-12-14T12:57:20.659Z
Learnt from: leynos
Repo: leynos/wildside PR: 253
File: docs/wildside-pwa-design.md:28-30
Timestamp: 2025-12-14T12:57:20.659Z
Learning: In the leynos/wildside repository, documentation should follow en-GB-oxendict spelling standards that require -ize endings for localization-related terms: use 'localization', 'localizability', and 'localizable' (not -ise variants). Apply this guideline to all Markdown documentation files under the docs/ directory and any future docs as they are added.
Applied to files:
docs/execplans/phase-0-update-modules-to-use-domain-ports.md
📚 Learning: 2025-12-14T12:57:30.806Z
Learnt from: leynos
Repo: leynos/wildside PR: 253
File: docs/wildside-pwa-data-model.md:156-158
Timestamp: 2025-12-14T12:57:30.806Z
Learning: Adopt en-GB-oxendict spelling in all Wildside documentation. Use -ize endings (e.g., localize, localization, localized) instead of -ise endings (e.g., localise) wherever the en-GB-oxendict style guide applies. Apply this consistently across all docs under the docs directory.
Applied to files:
docs/execplans/phase-0-update-modules-to-use-domain-ports.md
🧬 Code graph analysis (4)
backend/src/inbound/http/schemas.rs (2)
backend/src/domain/error.rs (1)
code(105-107)backend/src/domain/user_events.rs (1)
code(40-45)
backend/tests/openapi_schemas_bdd.rs (1)
backend/tests/openapi_support.rs (2)
get_property(42-46)unwrap_object_schema(14-37)
backend/tests/openapi_support.rs (1)
backend/src/doc.rs (1)
unwrap_object_schema(154-180)
backend/src/doc.rs (2)
backend/src/domain/user_events.rs (1)
field(34-36)backend/tests/openapi_support.rs (1)
unwrap_object_schema(14-37)
🔍 Remote MCP
Based on the search results, I've found helpful context about Utoipa and OpenAPI schema validation. Let me gather additional information specific to this codebase's testing patterns.## Summary of Key Context Found
Utoipa Framework & OpenAPI Schema Validation
Utoipa provides auto-generated OpenAPI documentation for Rust REST APIs and treats code-first approach as a first class citizen, simplifying API documentation by providing simple macros for generating documentation from code. Utoipa supports validation attributes on schema fields, allowing you to define constraints like maximum/minimum values, max_length/min_length, and pattern matching, which aligns directly with this PR's testing of UserSchema constraints.
Utoipa has specific support for uuid format—Uuid type will be presented as String with format uuid in OpenAPI spec. This is directly relevant to the PR's test openapi_user_id_has_uuid_format that validates the id property has uuid-like custom format.
BDD Testing with Cucumber-rs
Cucumber is a tool for behavior-driven development (BDD) that uses Gherkin language to specify test scenarios with syntax close to natural language, using keywords such as When and Then. This context is useful for understanding the BDD scenario added in backend/tests/features/openapi_schemas.feature. The PR's use of Cucumber tests to validate OpenAPI schema constraints follows standard BDD practices.
Exhaustive Pattern Matching & Compile-time Guarantees
Exhaustive checking via match in Rust ensures all possible variants are handled, reducing bugs, and adding a new variant to an enum will cause a compile-time error if not handled, prompting methodical updates. This directly supports the PR's refactoring of the ErrorCode serialization test to use exhaustive pattern matching over a compile-time-exhaustive variants array—a key safety improvement that ensures developers won't accidentally miss new ErrorCode variants when they're added.
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Sourcery review
- GitHub Check: build
🔇 Additional comments (7)
docs/execplans/phase-0-update-modules-to-use-domain-ports.md (1)
13-14: LGTM: Corrected terminology.The fix from "Open API Specification" to "OpenAPI Specification" aligns with official naming conventions.
backend/src/inbound/http/schemas.rs (3)
80-82: LGTM: Clarified validation layer separation.The updated documentation correctly distinguishes schema-level constraints from domain-level validation, making it clear that non-empty-when-trimmed is a domain concern separate from the 3–32 character schema constraint.
99-99: LGTM: Corrected spelling.Changed "serialises" to "serializes" per en-GB-oxendict standards.
167-198: LGTM: Exhaustive pattern matching strengthens compile-time guarantees.The refactored test now uses a helper function with exhaustive pattern matching, ensuring that adding a new
ErrorCodevariant will trigger a build error until the test is updated. This is a significant improvement over the previous static array approach.backend/tests/features/openapi_schemas.feature (1)
23-28: LGTM: Added comprehensive validation constraint scenario.The new scenario exercises User schema constraints (uuid format for id, length and pattern for display_name) via BDD steps, improving test coverage.
backend/src/doc.rs (1)
86-128: LGTM: Improved diagnostic messages for schema type mismatches.The updated
assert_object_schema_has_fieldnow handles $ref, AllOf, OneOf, AnyOf, and Array cases with clear diagnostic messages, making test failures easier to diagnose and fix.backend/tests/openapi_support.rs (1)
14-46: LGTM: Clear diagnostic helpers for schema traversal.The
unwrap_object_schemaandget_propertyhelpers provide helpful diagnostic messages for schema type mismatches. However, verify that utoipa v5.0.0 generates Object schemas (rather than String schemas) for string properties with validation constraints, as flagged in the reviews for backend/src/doc.rs and backend/tests/openapi_schemas_bdd.rs.
Replace separate openapi_support module with inline with_user_property_object_schema helper that accepts a closure. This eliminates code duplication while keeping the navigation logic self-contained in the test file. Also align unwrap_object_schema in doc.rs to have detailed diagnostics matching assert_object_schema_has_field for consistent failure messages. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
leynos
force-pushed
the
domain-ports-feedback
branch
from
c494182 to
d3de032
Compare
Creates backend/src/test_support.rs with unwrap_object_schema helper that can be used by both unit tests in doc.rs and integration tests. Eliminates duplicate implementations across test files. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
backend/src/doc.rs(4 hunks)backend/src/lib.rs(1 hunks)backend/src/test_support.rs(1 hunks)backend/tests/openapi_schemas_bdd.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{rs,ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,ts,tsx,js,jsx,py}: Comment why, not what—explain assumptions, edge cases, trade-offs, or complexity; do not echo the obvious.
Favour clarity over cleverness—be concise but prefer explicit over terse or obscure idioms; prioritise code that is easy to follow.
Use functions and composition to avoid repetition; prefer generators or comprehensions and declarative code to imperative repetition when readable.
Keep functions small and meaningful—single responsibility, clear purpose, and obey command/query segregation.
Name things precisely—use clear, descriptive variable and function names; for booleans, prefer names withis,has, orshould.
Structure logically—each file should encapsulate a coherent module; group related code close together.
Illustrate with clear examples in function documentation demonstrating usage and outcome; for tests, omit examples where the example only reiterates test logic.
Keep file size manageable—no single code file may exceed 400 lines; break up long switch statements and move large test data blocks to external files.
New functionality or changes in behaviour must be fully validated by relevant unittests and behavioural tests before committing.
When fixing a bug, provide a unittest demonstrating the corrected behaviour to validate the fix and guard against regression.
Files:
backend/src/lib.rsbackend/tests/openapi_schemas_bdd.rsbackend/src/doc.rsbackend/src/test_support.rs
**/*.{rs,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Group by feature, not layer—colocate views, logic, fixtures, and helpers related to a domain concept rather than splitting by type.
Files:
backend/src/lib.rsbackend/tests/openapi_schemas_bdd.rsbackend/src/doc.rsbackend/src/test_support.rs
**/*.{rs,ts,tsx,js,jsx,py,md}
📄 CodeRabbit inference engine (AGENTS.md)
Use consistent spelling and grammar with en-GB-oxendict ("-ize" / "-yse" / "-our") spelling and grammar, except for references to external APIs.
Files:
backend/src/lib.rsbackend/tests/openapi_schemas_bdd.rsbackend/src/doc.rsbackend/src/test_support.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Runmake fmt,make lint, andmake testbefore committing Rust code; these wrapcargo fmt,cargo clippy, andcargo test.
Clippy warnings must be disallowed; fix any warnings emitted during tests in the code itself rather than silencing them.
For excessively long functions, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
For functions with too many parameters, group related parameters in meaningfully named structs.
Where a function returns a large error, consider usingArcto reduce the amount of data returned.
Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated withcargo doc.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments, not before.
Do not usereturnin single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Lints must not be silenced except as a last resort; lint rule suppressions must be tightly scoped and include a clear reason.
Preferexpectoverallowwhen suppressing warnings.
Where a function is unused with specific features selected, use conditional compilation with#[cfg]or#[cfg_attr].
Prefer.expect()over.unwrap().
Useconcat!()to combine long string literals rather than escaping newlines with a backslash.
Prefer single-line versions of functions where appropriate (e.g.,pub fn new(id: u64) -> Self { Self(id) }).
Write unit and behavioural tests for new Rust functionality; run both before and after making any change.
Userstestfixtures for shared setup in Rust tests.
Replace duplicated tests with#[rstest(...)]parameterised cases in Rust.
Prefermockallfor mocks/stubs...
Files:
backend/src/lib.rsbackend/tests/openapi_schemas_bdd.rsbackend/src/doc.rsbackend/src/test_support.rs
⚙️ CodeRabbit configuration file
**/*.rs: * Seek to keep the cognitive complexity of functions no more than 9.
- Adhere to single responsibility and CQRS
- Place function attributes after doc comments.
- Do not use
returnin single-line functions.- Move conditionals with >2 branches into a predicate function.
- Avoid
unsafeunless absolutely necessary.- Every module must begin with a
//!doc comment that explains the module's purpose and utility.- Comments and docs must follow en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.- Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
- Use
rstestfixtures for shared setup and to avoid repetition between tests.- Replace duplicated tests with
#[rstest(...)]parameterised cases.- Prefer
mockallfor mocks/stubs.- Prefer
.expect()over.unwrap()in tests..expect()and.unwrap()are forbidden outside of tests. Errors must be propagated.- Ensure that any API or behavioural changes are reflected in the documentation in
docs/- Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/- Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
- Environment access (env::set_var and env::remove_var) are always unsafe in Rust 2024 and MUST be marked as such
- For testing of functionality depending upon environment variables, dependency injection and...
Files:
backend/src/lib.rsbackend/tests/openapi_schemas_bdd.rsbackend/src/doc.rsbackend/src/test_support.rs
🧠 Learnings (1)
📚 Learning: 2025-12-19T16:24:57.331Z
Learnt from: leynos
Repo: leynos/wildside PR: 261
File: backend/src/doc.rs:191-193
Timestamp: 2025-12-19T16:24:57.331Z
Learning: In utoipa 5.x, there is no Schema::String variant. All primitive types, including strings, are represented as Schema::Object with a schema_type field (e.g., SchemaType::String). String constraints like format, min_length, max_length, and pattern are fields on the Object struct itself. When reviewing Rust code that uses utoipa, verify that string fields are modeled as Schema::Object with schema_type = String and that constraints are applied on the Object, not on a non-existent Schema::String variant.
Applied to files:
backend/src/lib.rsbackend/src/doc.rsbackend/src/test_support.rs
🧬 Code graph analysis (2)
backend/tests/openapi_schemas_bdd.rs (1)
backend/src/test_support.rs (1)
unwrap_object_schema(19-42)
backend/src/doc.rs (1)
backend/src/test_support.rs (1)
unwrap_object_schema(19-42)
🔍 Remote MCP
Based on my research, I've found valuable context about OpenAPI schema validation patterns in Rust. Let me summarize the relevant findings for reviewing this PR:
Summary of Additional Context for PR #261 Review
OpenAPI Schema Testing and Validation Patterns
Utoipa provides auto-generated OpenAPI documentation for Rust REST APIs by treating a code-first approach as a first-class citizen and simplifying API documentation through simple macros. It also contains Rust types of the OpenAPI spec, allowing you to write the OpenAPI spec using Rust if auto generation is not appropriate.
Schema Validation Attributes in Utoipa
The PR's focus on adding validation constraints to the User schema aligns with Utoipa's built-in support for schema validation. Utoipa supports validation attributes such as max_length, min_length, and pattern constraints on string fields. For the User schema specifically:
- Display name constraints: min_length (3) and max_length (32) constraints can be applied to string fields
- ID format handling: Uuid type will be presented as String with format uuid in OpenAPI spec when uuid support is enabled
Test Helper Pattern Validation
The PR's approach of creating test-support helpers to unwrap and inspect OpenAPI schemas follows good practices for schema validation. Generating documentation from code alone doesn't guarantee correctness—despite Rust's strong type system, issues can arise such as mapping wrong request body types to endpoints or providing invalid example values, so validation of actual schema structure is needed.
Code Duplication Refactoring Context
The code review comment about duplicated navigation logic in BDD steps is valid—centralizing schema navigation patterns improves maintainability. The proposed helper function pattern with_user_property_object_schema(world, property_name, f) to extract shared traversal logic (accessing User schema → finding property → matching as Object → passing to closure) is a sound refactoring approach for BDD test code.
[::web_search::]
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Sourcery review
- GitHub Check: build
🔇 Additional comments (4)
backend/tests/openapi_schemas_bdd.rs (1)
151-188: LGTM—correct Schema::Object usage and clear assertions.These Then step functions correctly:
- Use unwrap_object_schema to extract Object schemas (no Schema::String assumption)
- Assert UUID format via SchemaFormat::Custom("uuid") per utoipa 5.x
- Validate min_length, max_length, and pattern constraints on the Object
The with_user_property_object_schema helper cleanly centralises navigation logic and the closures remain focused on assertions.
Based on learnings, utoipa 5.x represents all primitives as Schema::Object with constraints on the Object struct itself.
backend/src/doc.rs (2)
86-130: Enhanced diagnostics improve test maintainability.Adding the schema_name parameter and expanding match arms for all Schema variants (Ref, AllOf, OneOf, AnyOf, Array) provides clear, actionable panic messages. The function now guides developers towards the correct inspection strategy for each schema type.
The repetitive structure of the combinator panic messages (lines 106-123) could be factored into a helper, but for test code the explicit patterns aid clarity.
152-199: New tests correctly validate OpenAPI constraints per utoipa 5.x.Both tests appropriately:
- Use unwrap_object_schema to extract nested Object schemas
- Check constraints on the Object struct (format, min_length, max_length, pattern)
- Use matches! for SchemaFormat::Custom("uuid") validation
- Assert exact expected values with clear failure messages
This aligns with the learnings that utoipa 5.x represents all primitives as Schema::Object with a schema_type field, not as Schema::String variants.
Based on learnings, utoipa 5.x has no Schema::String variant; string constraints are fields on Schema::Object.
backend/src/test_support.rs (1)
1-52: Well-structured test utilities with clear diagnostics.This module appropriately:
- Documents its purpose and scope (unit and integration tests)
- Provides focused helpers for OpenAPI schema traversal
- Panics with actionable messages for refs, combinators, arrays, and unexpected types
- Uses correct utoipa 5.x Schema enum variants (Object, Array, OneOf, AllOf, AnyOf)
The unwrap_object_schema function correctly returns &Object for Schema::Object, matching the learnings that utoipa 5.x has no Schema::String variant. The get_property helper encapsulates property access with clear diagnostics.
Both functions use
.unwrap_or_else(|| panic!(...))for custom messages, which is appropriate for test utilities where panics indicate test-author errors rather than runtime failures.Based on learnings, the Schema enum handling is correct for utoipa 5.x (Object, Array, combinators only).
…helper - Guard test_support module with #[cfg(any(test, feature = "test-support"))] to prevent test utilities from leaking into production builds - Add test-support feature to backend/Cargo.toml - Import and use get_property helper in BDD tests to simplify property access and provide consistent panic diagnostics 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
backend/Cargo.toml(1 hunks)backend/src/lib.rs(1 hunks)backend/tests/openapi_schemas_bdd.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{rs,ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,ts,tsx,js,jsx,py}: Comment why, not what—explain assumptions, edge cases, trade-offs, or complexity; do not echo the obvious.
Favour clarity over cleverness—be concise but prefer explicit over terse or obscure idioms; prioritise code that is easy to follow.
Use functions and composition to avoid repetition; prefer generators or comprehensions and declarative code to imperative repetition when readable.
Keep functions small and meaningful—single responsibility, clear purpose, and obey command/query segregation.
Name things precisely—use clear, descriptive variable and function names; for booleans, prefer names withis,has, orshould.
Structure logically—each file should encapsulate a coherent module; group related code close together.
Illustrate with clear examples in function documentation demonstrating usage and outcome; for tests, omit examples where the example only reiterates test logic.
Keep file size manageable—no single code file may exceed 400 lines; break up long switch statements and move large test data blocks to external files.
New functionality or changes in behaviour must be fully validated by relevant unittests and behavioural tests before committing.
When fixing a bug, provide a unittest demonstrating the corrected behaviour to validate the fix and guard against regression.
Files:
backend/src/lib.rsbackend/tests/openapi_schemas_bdd.rs
**/*.{rs,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Group by feature, not layer—colocate views, logic, fixtures, and helpers related to a domain concept rather than splitting by type.
Files:
backend/src/lib.rsbackend/tests/openapi_schemas_bdd.rs
**/*.{rs,ts,tsx,js,jsx,py,md}
📄 CodeRabbit inference engine (AGENTS.md)
Use consistent spelling and grammar with en-GB-oxendict ("-ize" / "-yse" / "-our") spelling and grammar, except for references to external APIs.
Files:
backend/src/lib.rsbackend/tests/openapi_schemas_bdd.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Runmake fmt,make lint, andmake testbefore committing Rust code; these wrapcargo fmt,cargo clippy, andcargo test.
Clippy warnings must be disallowed; fix any warnings emitted during tests in the code itself rather than silencing them.
For excessively long functions, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
For functions with too many parameters, group related parameters in meaningfully named structs.
Where a function returns a large error, consider usingArcto reduce the amount of data returned.
Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated withcargo doc.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments, not before.
Do not usereturnin single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Lints must not be silenced except as a last resort; lint rule suppressions must be tightly scoped and include a clear reason.
Preferexpectoverallowwhen suppressing warnings.
Where a function is unused with specific features selected, use conditional compilation with#[cfg]or#[cfg_attr].
Prefer.expect()over.unwrap().
Useconcat!()to combine long string literals rather than escaping newlines with a backslash.
Prefer single-line versions of functions where appropriate (e.g.,pub fn new(id: u64) -> Self { Self(id) }).
Write unit and behavioural tests for new Rust functionality; run both before and after making any change.
Userstestfixtures for shared setup in Rust tests.
Replace duplicated tests with#[rstest(...)]parameterised cases in Rust.
Prefermockallfor mocks/stubs...
Files:
backend/src/lib.rsbackend/tests/openapi_schemas_bdd.rs
⚙️ CodeRabbit configuration file
**/*.rs: * Seek to keep the cognitive complexity of functions no more than 9.
- Adhere to single responsibility and CQRS
- Place function attributes after doc comments.
- Do not use
returnin single-line functions.- Move conditionals with >2 branches into a predicate function.
- Avoid
unsafeunless absolutely necessary.- Every module must begin with a
//!doc comment that explains the module's purpose and utility.- Comments and docs must follow en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.- Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
- Use
rstestfixtures for shared setup and to avoid repetition between tests.- Replace duplicated tests with
#[rstest(...)]parameterised cases.- Prefer
mockallfor mocks/stubs.- Prefer
.expect()over.unwrap()in tests..expect()and.unwrap()are forbidden outside of tests. Errors must be propagated.- Ensure that any API or behavioural changes are reflected in the documentation in
docs/- Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/- Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
- Environment access (env::set_var and env::remove_var) are always unsafe in Rust 2024 and MUST be marked as such
- For testing of functionality depending upon environment variables, dependency injection and...
Files:
backend/src/lib.rsbackend/tests/openapi_schemas_bdd.rs
🧠 Learnings (1)
📚 Learning: 2025-12-19T16:24:57.331Z
Learnt from: leynos
Repo: leynos/wildside PR: 261
File: backend/src/doc.rs:191-193
Timestamp: 2025-12-19T16:24:57.331Z
Learning: In utoipa 5.x, there is no Schema::String variant. All primitive types, including strings, are represented as Schema::Object with a schema_type field (e.g., SchemaType::String). String constraints like format, min_length, max_length, and pattern are fields on the Object struct itself. When reviewing Rust code that uses utoipa, verify that string fields are modeled as Schema::Object with schema_type = String and that constraints are applied on the Object, not on a non-existent Schema::String variant.
Applied to files:
backend/src/lib.rs
🧬 Code graph analysis (1)
backend/tests/openapi_schemas_bdd.rs (1)
backend/src/test_support.rs (2)
get_property(47-51)unwrap_object_schema(19-42)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (6)
backend/Cargo.toml (2)
53-54: LGTM: Standard feature flag pattern.The test-support feature is correctly defined for conditional compilation of test utilities.
57-58: LGTM: Correct self-referential dev-dependency pattern.Enabling the test-support feature in dev-dependencies allows integration tests to access test utilities whilst keeping them out of production builds.
backend/src/lib.rs (1)
13-14: LGTM: Correctly implements conditional compilation for test utilities.The cfg gate ensures test_support is only available during tests or when the test-support feature is explicitly enabled, preventing test utilities from leaking into production builds whilst making them accessible to integration tests.
backend/tests/openapi_schemas_bdd.rs (3)
9-9: LGTM: Correct import of test utilities.Both helpers are used by the with_user_property_object_schema function to simplify schema navigation.
146-156: LGTM: UUID format validation is correct.The test correctly validates that User.id has format=uuid using matches! with a pattern guard, aligning with utoipa 5.x schema representation.
158-183: LGTM: Length and pattern constraints correctly validated.Both test steps properly validate User.display_name schema constraints:
- min_length=3 and max_length=32 are correctly asserted
- Pattern constraint "^[A-Za-z0-9_ ]+$" is correctly validated using as_deref()
The tests align with the domain validation rules and provide clear assertion messages.
Documents the helper's purpose, parameters, usage pattern, and includes an example to guide future contributors on when and how to use it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
🤖 Generated with Claude Code
Summary by Sourcery
Strengthen OpenAPI schema validation around the User and ErrorCode types, and clarify associated documentation and comments.
New Features:
Enhancements: