feat(resource): validator, builder, and tests for the resource constraint - #387
Conversation
…aint Implement the quent.resource.v1 constraint: the Resource/Capacity metadata types, a Visitor validating the eight requirements plus role placement, and a ResourceBuilder that emits the usage and bounds record types and the definition for a caller to compose the entity from. Add a focused test per requirement. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Capacities alias is private, so linking it from the public ResourceConstraint doc produced rustdoc private-intra-doc-link warnings. Refer to the capacities in plain prose; the public Capacity type stays linked. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
80f0cb4 to
1b40c07
Compare
📝 WalkthroughWalkthroughAdds a ChangesResource constraint
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
crates/resource/src/lib.rs-296-312 (1)
296-312: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMisleading diagnostic text for a usage record without an enclosing entity.
When
entityisNonehere,on_entity_refwas already confirmedtrueabove (the check that would'vecontinued otherwise). So the actual failure is "no enclosing entity", not "a non-entity reference" — the element label describes the wrong condition and will mislead anyone debugging this error.🐛 Proposed fix
None => errors.push(ResourceError::MisplacedRole { location: location.clone(), role: "usage", - element: "a non-entity reference", + element: "a record with no enclosing entity", }),🤖 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 `@crates/resource/src/lib.rs` around lines 296 - 312, Update the `None` branch of the `match entity` handling in `on_entity_ref` so `ResourceError::MisplacedRole` reports the missing enclosing entity accurately; change only the `element` diagnostic label from the misleading non-entity reference wording while preserving the existing location, role, and error flow.
🧹 Nitpick comments (2)
crates/resource/src/builder.rs (1)
159-160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider
bullet_listforBuildError::Multiplefor consistency.
ResourceError::Multiple(inlib.rs) renders its aggregated errors viabullet_list(.0); this variant instead dumps raw{0:?}. Aligning the two would give a more readable multi-error message here too.♻️ Proposed fix
- #[error("multiple resource builder errors: {0:?}")] + #[error("multiple resource builder errors:\n{}", quent_constraints::utils::bullet_list(.0))] Multiple(Vec<BuildError>),🤖 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 `@crates/resource/src/builder.rs` around lines 159 - 160, Update the BuildError::Multiple display annotation to use the existing bullet_list formatter on its contained errors, matching ResourceError::Multiple in lib.rs instead of rendering the vector with raw debug formatting.crates/resource/src/lib.rs (1)
147-387: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider splitting
finish()into per-requirement helpers.The method spans ~140 lines and interleaves four distinct validation passes (usage/capacity, FSM+reference+bounds-ownership, bounds coverage, missing-bounds). Extracting each pass into a private helper taking the relevant collections would improve readability/testability without changing behavior.
🤖 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 `@crates/resource/src/lib.rs` around lines 147 - 387, Split ResourceConstraint::finish into private helpers for the four validation passes: usage/capacity validation, reference and bounds ownership validation, bounds coverage validation, and missing-bounds validation. Have each helper receive only the relevant collections and append to the shared errors collection, while preserving the existing validation order and final error aggregation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/resource/Cargo.toml`:
- Around line 8-10: Update the resource crate’s quent-schema, quent-constraints,
and quent-fsm entries in both [dependencies] and [dev-dependencies] to use
workspace-managed declarations with workspace = true instead of direct paths.
Add matching path-based entries for these three crates to the workspace root’s
[workspace.dependencies], preserving the repository’s existing dependency
configuration style.
---
Other comments:
In `@crates/resource/src/lib.rs`:
- Around line 296-312: Update the `None` branch of the `match entity` handling
in `on_entity_ref` so `ResourceError::MisplacedRole` reports the missing
enclosing entity accurately; change only the `element` diagnostic label from the
misleading non-entity reference wording while preserving the existing location,
role, and error flow.
---
Nitpick comments:
In `@crates/resource/src/builder.rs`:
- Around line 159-160: Update the BuildError::Multiple display annotation to use
the existing bullet_list formatter on its contained errors, matching
ResourceError::Multiple in lib.rs instead of rendering the vector with raw debug
formatting.
In `@crates/resource/src/lib.rs`:
- Around line 147-387: Split ResourceConstraint::finish into private helpers for
the four validation passes: usage/capacity validation, reference and bounds
ownership validation, bounds coverage validation, and missing-bounds validation.
Have each helper receive only the relevant collections and append to the shared
errors collection, while preserving the existing validation order and final
error aggregation behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 2a834ab7-073c-4ae5-9650-afc2031957c9
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock
📒 Files selected for processing (4)
crates/resource/Cargo.tomlcrates/resource/src/builder.rscrates/resource/src/lib.rscrates/resource/tests/resource-constraint.rs
| quent-schema = { path = "../schema", features = ["serde", "visitor"] } | ||
| quent-constraints = { path = "../constraints" } | ||
| quent-fsm = { path = "../fsm" } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Route internal crate deps through [workspace.dependencies].
quent-schema, quent-constraints, and quent-fsm are declared with direct path = "..." in both [dependencies] (Lines 8-10) and [dev-dependencies] (Line 18), bypassing workspace = true used consistently by every other dependency in this file (Lines 11-15).
🔧 Proposed fix
-quent-schema = { path = "../schema", features = ["serde", "visitor"] }
-quent-constraints = { path = "../constraints" }
-quent-fsm = { path = "../fsm" }
+quent-schema = { workspace = true, features = ["serde", "visitor"] }
+quent-constraints = { workspace = true }
+quent-fsm = { workspace = true }and add corresponding entries to the workspace root's [workspace.dependencies].
As per path instructions, "Dependencies come from [workspace.dependencies] via workspace = true; no git deps."
Also applies to: 17-18
🤖 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 `@crates/resource/Cargo.toml` around lines 8 - 10, Update the resource crate’s
quent-schema, quent-constraints, and quent-fsm entries in both [dependencies]
and [dev-dependencies] to use workspace-managed declarations with workspace =
true instead of direct paths. Add matching path-based entries for these three
crates to the workspace root’s [workspace.dependencies], preserving the
repository’s existing dependency configuration style.
Source: Path instructions
|
/merge |
2 participants
Description
Implements the
quent.resource.v0.1.0constraint in the existingquent-resourcecrate.Adds validation for resource declarations, capacity claims, bounds coverage, FSM usage lifetimes, record references, and annotation placement. Refines the resource metadata types and adds a fallible
ResourceBuilderthat produces the definition, usage record, and optional bounds record.Related Issues
Solves #196
Part of #191
🤖 Generated with Claude Code
🤖 Generated by OpenAI Codex.