feat: add constraint processing support for TargetedReportConstraint (#592)#598
Conversation
📝 WalkthroughWalkthroughAdds end-to-end support for a new "report" constraint: new IReportConstraint and DefaultReportConstraint, visitor/validation handlers, storage and loader/binding support, databind annotations and factories, unit/integration tests, PRD and implementation-plan docs, and docs/skill updates. Changes
Sequence Diagram(s)%%{init: {"themeVariables":{"primaryColor":"#8FBBD9","secondaryColor":"#D6F0D6","tertiaryColor":"#F2E7FF"}}}%%
sequenceDiagram
autonumber
rect rgb(143,187,217)
participant Loader as BindingLoader
participant Binder as ConstraintBindingSupport
participant Factory as ConstraintFactory
end
rect rgb(214,240,214)
participant Validator as DefaultConstraintValidator
participant TargetEval as MetapathTargetEvaluator
participant Handler as ConstraintValidationHandler
end
Loader->>Binder: parse(TargetedReportConstraint)
Binder->>Factory: newReport(binding, source)
Factory-->>Loader: IReportConstraint
Loader->>Validator: register constraint(s)
Validator->>TargetEval: evaluate target expression
alt targets found
TargetEval-->>Validator: target nodes
Validator->>Validator: evaluate report test on each target
alt report test true
Validator->>Handler: handleReportViolation(constraint, node, target, context)
Handler-->>Validator: record finding / log
else report test false
note right of Validator: no finding
end
else no targets
TargetEval-->>Validator: no nodes
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PMD (7.19.0)databind/src/main/java/gov/nist/secauto/metaschema/databind/model/annotations/Report.java[ERROR] Cannot load ruleset pmd/category/java/custom.xml: Cannot resolve rule/ruleset reference 'pmd/category/java/custom.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath. 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: 0
🧹 Nitpick comments (2)
PRDs/20251228-targeted-report-constraint/PRD.md (1)
174-196: Enumerate visitor implementations to address medium/medium risk.The risk table identifies missing visitor implementations as a medium-likelihood, medium-impact risk, mitigated by "search for all IConstraintVisitor implementations." However, the PRD and implementation plan don't specify how many visitor implementations exist or provide a concrete list of files to update. This should be enumerated in the implementation plan to reduce ambiguity and ensure no implementations are missed during development.
I can help generate a shell script to enumerate all IConstraintVisitor implementations if you'd like.
PRDs/20251228-targeted-report-constraint/implementation-plan.md (1)
61-90: Include marker annotations (@NonNull/@nullable) in code snippets for clarity.The code snippet for
IReportConstraintat lines 71-85 shows@NonNullannotations, which is good. However, the code snippet for the factory methodnewReport()at lines 136-142 lacks annotations. For consistency with the CLAUDE.md Javadoc requirements and codebase conventions (using SpotBugs annotations), all public methods and parameters should include appropriate@NonNulland@Nullablemarkers. Ensure the implementation snippets reflect this expectation.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CLAUDE.mdPRDs/20251228-targeted-report-constraint/PRD.mdPRDs/20251228-targeted-report-constraint/implementation-plan.md
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: For larger initiatives requiring multiple PRs, use structured PRD (Product Requirements Document) approach. PRDs stored in PRDs/<YYYYMMDD>-<name>/ with PRD.md and implementation-plan.md. Target ≤50 files per PR, maximum 100 files per PR.
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: For larger initiatives requiring multiple PRs, use structured PRD (Product Requirements Document) approach. PRDs stored in PRDs/<YYYYMMDD>-<name>/ with PRD.md and implementation-plan.md. Target ≤50 files per PR, maximum 100 files per PR.
Applied to files:
PRDs/20251228-targeted-report-constraint/PRD.mdCLAUDE.md
📚 Learning: 2025-12-24T21:21:40.208Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/csrc/ns/metaschema/test_suite/_1_0/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:40.208Z
Learning: In metaschema-framework/metaschema-java, generated binding classes in package gov.nist.csrc.ns.metaschema.test_suite._1_0 (and similar generated binding packages) are pre-generated by metaschema-maven-plugin and checked into source control. Javadoc coverage issues in these generated classes should be tracked as code generator improvements rather than file-level issues, and improvements are deferred to generator enhancements.
Applied to files:
PRDs/20251228-targeted-report-constraint/PRD.mdPRDs/20251228-targeted-report-constraint/implementation-plan.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Website
- GitHub Check: Code
🔇 Additional comments (6)
PRDs/20251228-targeted-report-constraint/PRD.md (2)
199-213: Clarify the rationale for WARNING-level report constraints defaulting to INFORMATIONAL findings.Line 205-211 specifies a Kind mapping where WARNING-level report constraints generate INFORMATIONAL findings. This differs from expect constraints (which would likely map WARNING → WARNING or similar). While the decision is documented, the rationale isn't explained. Is this intentional to make report constraints exclusively informational by default? If so, this should be clarified in the decision rationale, as it could be surprising to implementers or future maintainers.
1-54: PRD structure is solid and well-organized.The PRD follows the established template with clear sections covering problem statement, goals, background, requirements, testing strategy, and design decisions. The distinction between report ("This MUST NOT be true") and expect ("This MUST be true") semantics is well-justified and provides clear guidance for implementation.
CLAUDE.md (1)
295-295: CLAUDE.md update correctly tracks the new PRD.The entry follows the established format and is appropriately placed in the Active PRDs section with "In Review" status.
PRDs/20251228-targeted-report-constraint/implementation-plan.md (3)
125-154: Enumerate all validation entry points for report constraint integration.Step 5 (Update Validation Pipeline) mentions integrating report validation into
validateFlag(),validateField(), andvalidateAssembly(). However, the implementation plan should enumerate all relevant validation entry points to ensure completeness. Specifically, verify whethervalidateAssemblyInstance()and other validation methods also need report constraint handling. A test case checking for missing validation calls would help catch this during TDD.
161-180: Acceptance criteria are well-structured and measurable.The 18 criteria cover interface creation, implementation, integration, testing, and documentation with clear verification points. The inclusion of full build verification (
mvn clean install -PCI -Prelease) ensures no regressions. Consider adding a specific check for visitor implementation enumeration (e.g., "All IConstraintVisitor implementations updated with visitReportConstraint()") to prevent missed implementations, though TDD tests will catch this indirectly.
198-206: Kind mapping table is clear and aligns with PRD.The mapping table properly documents the severity level to Kind translation, supporting both informational (default) and fail-level report constraints. This provides clear guidance for validation implementation.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
PRDs/20251228-targeted-report-constraint/PRD.md (1)
124-128: Improve semantic clarity in FR-4 by referencing DD-2.While FR-4 correctly specifies report constraint behavior, readers may not immediately understand the semantic inversion from expect constraints. Consider adding a parenthetical reference: "Generate findings when report test expressions evaluate to TRUE (see DD-2 for semantic distinction)."
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
PRDs/20251228-targeted-report-constraint/PRD.mdPRDs/20251228-targeted-report-constraint/implementation-plan.md
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: For larger initiatives requiring multiple PRs, use structured PRD (Product Requirements Document) approach. PRDs stored in PRDs/<YYYYMMDD>-<name>/ with PRD.md and implementation-plan.md. Target ≤50 files per PR, maximum 100 files per PR.
📚 Learning: 2025-12-24T21:21:40.208Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/csrc/ns/metaschema/test_suite/_1_0/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:40.208Z
Learning: In metaschema-framework/metaschema-java, generated binding classes in package gov.nist.csrc.ns.metaschema.test_suite._1_0 (and similar generated binding packages) are pre-generated by metaschema-maven-plugin and checked into source control. Javadoc coverage issues in these generated classes should be tracked as code generator improvements rather than file-level issues, and improvements are deferred to generator enhancements.
Applied to files:
PRDs/20251228-targeted-report-constraint/implementation-plan.md
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: For larger initiatives requiring multiple PRs, use structured PRD (Product Requirements Document) approach. PRDs stored in PRDs/<YYYYMMDD>-<name>/ with PRD.md and implementation-plan.md. Target ≤50 files per PR, maximum 100 files per PR.
Applied to files:
PRDs/20251228-targeted-report-constraint/PRD.md
📚 Learning: 2025-12-13T21:16:12.281Z
Learnt from: CR
Repo: metaschema-framework/metaschema PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T21:16:12.281Z
Learning: Constraints are documented in `website/content/specification/syntax/constraints.md`. When adding new constraint types, update the constraint type lists for `define-flag`, `define-field`, and `define-assembly`, and add a new section with syntax table and examples
Applied to files:
PRDs/20251228-targeted-report-constraint/PRD.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Code
- GitHub Check: Website
🔇 Additional comments (10)
PRDs/20251228-targeted-report-constraint/PRD.md (4)
206-237: Design decisions are well-structured and justified.All three design decisions (DD-1, DD-2, DD-3) are clearly articulated with supporting rationale and examples. The semantic distinction (DD-2) is particularly helpful for understanding the constraint's behavior. The decision to use
instanceoffor differentiation (DD-3) aligns with existing patterns as mentioned in the PR objectives.
104-150: Requirements are comprehensive and well-defined.Both functional (FR-1 through FR-6) and non-functional (NFR-1 through NFR-3) requirements are clearly specified. The inclusion of skill documentation updates (FR-6) as a formal requirement is excellent for project maintainability. All requirements map to specific acceptance criteria in the verification checklist.
196-203: Risk assessment is thorough with appropriate mitigations.The three identified risks align with the implementation complexity, and mitigations are concrete and actionable. The medium likelihood for missing visitor implementations is well-addressed by Step 3 of the implementation plan (IConstraintVisitor updates). The emphasis on comprehensive test coverage mitigates both breaking changes and missing implementations.
45-52: Success metrics and verification checklist are concrete and comprehensive.The four success metrics (Section 1.4) are measurable and tied to outcomes. The 13-item verification checklist (Section 5.2) provides thorough coverage of all implementation areas, from interface creation through build verification. All checklist items align directly with the acceptance criteria in the implementation plan.
Also applies to: 178-192
PRDs/20251228-targeted-report-constraint/implementation-plan.md (6)
61-177: Implementation approach is well-structured and TDD-focused.The seven steps provide a logical progression from core interface creation through validation integration and documentation. Each step emphasizes test-first development, which aligns with project practices. The sequencing allows parallel work on independent interfaces (Steps 2–3) while maintaining proper dependencies. Code snippets serve as helpful templates without prescribing implementation details.
38-60: File inventory and acceptance criteria are comprehensive and actionable.The 13 files identified (3 to create, 10 to modify) are appropriate in scope, and the 22-item acceptance criteria directly map to implementation deliverables. Each criterion is testable and pairs well with the TDD-driven steps. The level of detail provides sufficient guidance without over-constraining implementation choices.
Also applies to: 179-200
206-227: Design decisions section appropriately replicates PRD content.The key design decisions are clearly documented with comparison tables that will help implementers understand report vs expect semantics. However, note that the KIND mapping table here (lines 220–227) shows WARNING → INFORMATIONAL, which should be verified against the PRD's FR-4 requirement (flagged in separate review comment).
250-261: Verification commands are practical and well-targeted.The four command examples provide clear paths for both developer verification and CI/CD validation. The granular test targeting (by module and test class) will help implementers verify components incrementally during TDD development. The full CI build command aligns with project CI/CD practices.
157-171: Skill documentation updates are well-specified and appropriately conditional.Step 6 provides concrete guidance for updating both constraint authoring and Java library skills. The requirement for semantic distinction documentation in the authoring skill will help future users understand report vs expect. The conditional note about java-library.md respects the current documentation state while indicating where additions belong.
27-36: PR scope and feasibility are well-calibrated.The single-PR approach with ~13 files affected is appropriately scoped given the interdependencies between components. The Medium risk classification aligns with the need to update interfaces, implementations, loaders, and validators. The estimated file count of "~15" provides reasonable margin. The detailed breakdown of files and steps ensures implementers have clear guidance without scope ambiguity.
Also applies to: 239-247
Review Feedback AddressedCritical issue fixed (KIND mapping inconsistency in FR-4):
Other feedback addressed in earlier commits:
|
…framework#592) Add Product Requirements Document and implementation plan for adding constraint processing support for TargetedReportConstraint. Key design decisions: - Report constraints default to INFORMATIONAL level - Kind mapping: ERROR/CRITICAL -> FAIL, others -> INFORMATIONAL - Semantic: Report = "This MUST NOT be true" (opposite of expect) Closes metaschema-framework#592
Add FR-6 requiring updates to Claude Code skills: - metaschema-constraints-authoring.md - document report constraint - metaschema-java-library.md - document IReportConstraint interface Update implementation plan with Step 6 for skill documentation.
- Clarify WARNING → INFORMATIONAL rationale in design decisions - Note that no IConstraintVisitor implementations exist (lower risk) - Enumerate specific validation entry points (validateFlag, validateField, validateAssembly) - Code snippets already have @nonnull annotations
Add report constraint support as the inverse of expect constraints: - IReportConstraint interface extending IConfigurableMessageConstraint - DefaultReportConstraint implementation with builder - Default level is INFORMATIONAL (not ERROR like expect) - Add REPORT type to IConstraint.Type enum - Add visitReportConstraint() to IConstraintVisitor Report constraints generate findings when test is TRUE, semantic opposite of expect constraints which generate findings when FALSE. Implements part of metaschema-framework#592
Update constraint interface hierarchy to support IReportConstraint: - IValueConstrained: add getReportConstraints() and addConstraint() - IFeatureValueConstrained: add delegation methods - ValueConstraintSet: add thread-safe storage and retrieval - AbstractTargetedConstraints: forward report constraints in applyTo() Implements part of metaschema-framework#592
… constraints Complete the report constraint implementation with: - Add @report annotation and update @ValueConstraints to include report array - Update AnnotationGenerator to emit @report annotations in generated code - Add ConstraintFactory.newReportConstraint() for parsing @report annotations - Update ConstraintSupport to parse report constraints from annotations - Update DefaultConstraintValidator to evaluate report constraints - Add IConstraintValidationHandler.handleReportResult() callback - Update validation handlers (Logging, FindingCollecting, Abstract) - Add comprehensive tests for report constraint lifecycle - Update skill documentation with report constraint usage The report constraint is the inverse of expect: it fires when the test expression evaluates to TRUE (condition detected) rather than FALSE (assertion failed). Typical use cases include deprecation warnings, informational notices, and audit findings. Closes metaschema-framework#592
david-waltermire
force-pushed
the
feature/592-targeted-report-constraint
branch
from
5dade38 to
9b6216a
Compare
david-waltermire
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
.claude/skills/metaschema-constraints-authoring.mdCLAUDE.mdPRDs/20251228-targeted-report-constraint/PRD.mdPRDs/20251228-targeted-report-constraint/implementation-plan.mdcore/src/main/java/gov/nist/secauto/metaschema/core/model/MetaschemaModelConstants.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/AbstractConstraintValidationHandler.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/AbstractTargetedConstraints.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/DefaultConstraintValidator.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/FindingCollectingConstraintValidationHandler.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraintValidationHandler.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraintVisitor.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IFeatureValueConstrained.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IReportConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IValueConstrained.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/LoggingConstraintValidationHandler.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/ValueConstraintSet.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/impl/DefaultReportConstraint.javacore/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/ReportConstraintTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGenerator.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/annotations/Report.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/annotations/ValueConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintFactory.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintSupport.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedExpectConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexHasKeyConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedMatchesConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedReportConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/ConstraintBindingSupport.javadatabind/src/main/metaschema-bindings/metaschema-model-bindings.xmldatabind/src/test/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGeneratorTest.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/model/metaschema/BindingConstraintLoaderTest.javadatabind/src/test/resources/content/constraints/meta-constraints/meta-constraints-report.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- PRDs/20251228-targeted-report-constraint/PRD.md
- PRDs/20251228-targeted-report-constraint/implementation-plan.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.java: All code changes must follow the Javadoc style guide (docs/javadoc-style-guide.md). New code requires 100% Javadoc coverage on public/protected members. Modified code must add/update Javadoc on any members touched. All Javadoc must include @param, @return, @throws tags in the correct order (BLOCKING)
Java target version must be Java 11. Use SpotBugs annotations (@nonnull, @nullable) for null safety in code.
Follow package naming convention gov.nist.secauto.metaschema.* for all Java packages
Follow Test-Driven Development (TDD) principles: write tests first before implementing functionality, verify tests fail with current implementation, implement minimal code to pass tests, then refactor while keeping tests green
Files:
core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IFeatureValueConstrained.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedReportConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/FindingCollectingConstraintValidationHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintFactory.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraintVisitor.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintSupport.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraintValidationHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/annotations/ValueConstraints.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/DefaultConstraintValidator.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/AbstractTargetedConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/ConstraintBindingSupport.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGenerator.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexHasKeyConstraint.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGeneratorTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedExpectConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IValueConstrained.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/AbstractConstraintValidationHandler.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/model/metaschema/BindingConstraintLoaderTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedMatchesConstraint.javacore/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/ReportConstraintTest.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IReportConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/annotations/Report.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/ValueConstraintSet.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/LoggingConstraintValidationHandler.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/MetaschemaModelConstants.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/impl/DefaultReportConstraint.java
**/*Test.java
📄 CodeRabbit inference engine (CLAUDE.md)
@test methods do not require Javadoc if they use descriptive method names. Use JUnit 5 for testing with parallel execution enabled.
Files:
databind/src/test/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGeneratorTest.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/model/metaschema/BindingConstraintLoaderTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/ReportConstraintTest.java
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: For larger initiatives requiring multiple PRs, use structured PRD (Product Requirements Document) approach. PRDs stored in PRDs/<YYYYMMDD>-<name>/ with PRD.md and implementation-plan.md. Target ≤50 files per PR, maximum 100 files per PR.
📚 Learning: 2025-12-19T04:01:37.408Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 550
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/format/JsonPointerFormatter.java:56-100
Timestamp: 2025-12-19T04:01:37.408Z
Learning: When overriding Java interface methods, rely on inherited Javadoc from the interface. Do not duplicate documentation in the implementing class unless there is implementation-specific behavior that warrants additional notes beyond the interface contract.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IFeatureValueConstrained.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedReportConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/FindingCollectingConstraintValidationHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintFactory.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraintVisitor.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintSupport.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraintValidationHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/annotations/ValueConstraints.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/DefaultConstraintValidator.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/AbstractTargetedConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/ConstraintBindingSupport.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGenerator.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexHasKeyConstraint.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGeneratorTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedExpectConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IValueConstrained.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/AbstractConstraintValidationHandler.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/model/metaschema/BindingConstraintLoaderTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedMatchesConstraint.javacore/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/ReportConstraintTest.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IReportConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/annotations/Report.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/ValueConstraintSet.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/LoggingConstraintValidationHandler.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/MetaschemaModelConstants.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/impl/DefaultReportConstraint.java
📚 Learning: 2025-12-27T16:52:04.509Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 590
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/DynamicContext.java:482-492
Timestamp: 2025-12-27T16:52:04.509Z
Learning: In Java, UncheckedIOException.getCause() is declared to return IOException. In methods that declare throws IOException, you can rethrow the underlying cause with throw e.getCause() where e is an UncheckedIOException, without a cast. Ensure the surrounding method signature includes throws IOException. This does not apply to other unchecked exceptions; verify that e is actually an UncheckedIOException before using this pattern.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IFeatureValueConstrained.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedReportConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/FindingCollectingConstraintValidationHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintFactory.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraintVisitor.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintSupport.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraintValidationHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/annotations/ValueConstraints.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/DefaultConstraintValidator.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/AbstractTargetedConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/ConstraintBindingSupport.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGenerator.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexHasKeyConstraint.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGeneratorTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedExpectConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IValueConstrained.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/AbstractConstraintValidationHandler.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/model/metaschema/BindingConstraintLoaderTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedMatchesConstraint.javacore/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/ReportConstraintTest.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IReportConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/annotations/Report.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/ValueConstraintSet.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/LoggingConstraintValidationHandler.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/MetaschemaModelConstants.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/impl/DefaultReportConstraint.java
📚 Learning: 2025-12-24T21:22:07.082Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/GenerationCase.java:74-80
Timestamp: 2025-12-24T21:22:07.082Z
Learning: Files in the package gov.nist.secauto.metaschema.model.testing.testsuite in metaschema-testing are generated binding classes created from Metaschema definitions. Documentation and style improvements for these files should be made at the code generator level (metaschema-maven-plugin) rather than by manually editing the generated code.
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedReportConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintFactory.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/ConstraintBindingSupport.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGenerator.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexHasKeyConstraint.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGeneratorTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedExpectConstraint.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/model/metaschema/BindingConstraintLoaderTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedMatchesConstraint.javacore/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/ReportConstraintTest.javadatabind/src/main/metaschema-bindings/metaschema-model-bindings.xml
📚 Learning: 2025-12-24T21:21:40.208Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/csrc/ns/metaschema/test_suite/_1_0/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:40.208Z
Learning: In metaschema-framework/metaschema-java, generated binding classes in package gov.nist.csrc.ns.metaschema.test_suite._1_0 (and similar generated binding packages) are pre-generated by metaschema-maven-plugin and checked into source control. Javadoc coverage issues in these generated classes should be tracked as code generator improvements rather than file-level issues, and improvements are deferred to generator enhancements.
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedReportConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintFactory.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/DefaultConstraintValidator.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/ConstraintBindingSupport.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGenerator.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexHasKeyConstraint.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGeneratorTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedExpectConstraint.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/model/metaschema/BindingConstraintLoaderTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedMatchesConstraint.javacore/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/ReportConstraintTest.javadatabind/src/main/metaschema-bindings/metaschema-model-bindings.xml
📚 Learning: 2025-12-24T21:21:56.361Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:56.361Z
Learning: In metaschema-testing, generated binding classes under gov.nist.secauto.metaschema.model.testing.testsuite are produced by metaschema-maven-plugin from YAML metaschema definitions. Javadoc issues in these generated classes should not be flagged for manual fixes; improvements are tracked and handled through code generator enhancements rather than manual edits to the generated source.
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedReportConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintFactory.javadatabind/src/test/resources/content/constraints/meta-constraints/meta-constraints-report.yamldatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGenerator.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexHasKeyConstraint.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGeneratorTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedExpectConstraint.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/model/metaschema/BindingConstraintLoaderTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedMatchesConstraint.javacore/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/ReportConstraintTest.javadatabind/src/main/metaschema-bindings/metaschema-model-bindings.xml
📚 Learning: 2024-11-14T18:19:40.200Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IBooleanItem.java:86-104
Timestamp: 2024-11-14T18:19:40.200Z
Learning: In the file `core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IBooleanItem.java`, the 3-step approach in the `cast` method is consistent with the XPath 3.1 specification.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/FindingCollectingConstraintValidationHandler.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/LoggingConstraintValidationHandler.java
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to **/metapath/**/*.java : Metapath is an implementation of XPath 3.1. Use the XPath 3.1 specification (https://www.w3.org/TR/xpath-31/) and XPath Functions 3.1 (https://www.w3.org/TR/xpath-functions-31/) as authoritative reference when implementing new functions, fixing bugs, or understanding error handling. Raise clarification before making changes if implementation differs from spec.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/FindingCollectingConstraintValidationHandler.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/LoggingConstraintValidationHandler.java
📚 Learning: 2025-12-13T21:16:12.281Z
Learnt from: CR
Repo: metaschema-framework/metaschema PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T21:16:12.281Z
Learning: Constraints are documented in `website/content/specification/syntax/constraints.md`. When adding new constraint types, update the constraint type lists for `define-flag`, `define-field`, and `define-assembly`, and add a new section with syntax table and examples
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintFactory.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGenerator.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraint.java.claude/skills/metaschema-constraints-authoring.mdcore/src/main/java/gov/nist/secauto/metaschema/core/model/MetaschemaModelConstants.java
📚 Learning: 2024-11-14T17:07:03.586Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IIPv4AddressItem.java:66-73
Timestamp: 2024-11-14T17:07:03.586Z
Learning: In the Metaschema Java codebase, differences in casting patterns across atomic type implementations are intentional and required; any differences in approach are significant and necessary.
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javadatabind/src/main/metaschema-bindings/metaschema-model-bindings.xml
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: For larger initiatives requiring multiple PRs, use structured PRD (Product Requirements Document) approach. PRDs stored in PRDs/<YYYYMMDD>-<name>/ with PRD.md and implementation-plan.md. Target ≤50 files per PR, maximum 100 files per PR.
Applied to files:
CLAUDE.md
🧬 Code graph analysis (4)
core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/FindingCollectingConstraintValidationHandler.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/ConstraintValidationFinding.java (1)
ConstraintValidationFinding(25-312)
core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/AbstractConstraintValidationHandler.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IReportConstraint.java (2)
core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/impl/DefaultReportConstraint.java (1)
DefaultReportConstraint(39-92)core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
core/src/main/java/gov/nist/secauto/metaschema/core/model/MetaschemaModelConstants.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/MetaschemaConstants.java (1)
MetaschemaConstants(17-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Website
- GitHub Check: Code
🔇 Additional comments (46)
CLAUDE.md (1)
297-297: Administrative entry correctly documents new PRD in Active PRDs section.The entry follows the established format (date-based folder name, clear description with issue reference, appropriate "In Progress" status) and aligns with the PRD workflow documented in the same file. This properly tracks the TargetedReportConstraint PRD initiative.
core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraint.java (1)
44-46: LGTM!The new
REPORTenum value follows the established pattern for constraint types and includes appropriate documentation.core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraintVisitor.java (1)
104-115: LGTM!The new
visitReportConstraintmethod correctly extends the visitor pattern, following the established convention for other constraint visitor methods with complete Javadoc..claude/skills/metaschema-constraints-authoring.md (2)
15-16: LGTM!The constraint types table clearly distinguishes
expect(fires when FALSE) fromreport(fires when TRUE), making the semantic difference immediately apparent.
152-203: Documentation is thorough but verify default level alignment.The documentation section for
reportis comprehensive with good examples and a clear comparison table. However, line 166 states the level is "TypicallyINFORMATIONALorWARNING(notERROR)", while theReport.javaannotation defaults toLevel.ERROR(line 63 of that file).Per the PRD, report constraints should default to
INFORMATIONAL. Consider updating theReport.javadefault level toLevel.INFORMATIONALto match the documented semantics and PRD design decision.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/annotations/Report.java (1)
86-96: LGTM!The
test()element is correctly marked as required with clear Javadoc explaining the inverted semantics (fires when true, opposite of expect).databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexHasKeyConstraint.java (1)
30-41: LGTM!The addition of
ITargetedConstraintBaseimplementation aligns with the broader PR changes for targeted constraint support. As this is a generated binding class, any style or documentation improvements should be addressed at the code generator level. Based on learnings.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedMatchesConstraint.java (1)
30-41: LGTM!The
ITargetedConstraintBaseinterface addition is consistent with other targeted constraint binding classes in this PR. As this is a generated binding class, improvements should be made at the code generator level. Based on learnings.core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/AbstractTargetedConstraints.java (1)
82-89: LGTM!The addition of
getReportConstraints().forEach(definition::addConstraint)correctly extends the constraint forwarding mechanism to include report constraints, following the established pattern for other constraint types.core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/ValueConstraintSet.java (3)
45-46: LGTM!The new
reportConstraintsfield follows the established pattern for constraint storage collections.
160-169: LGTM!The
getReportConstraints()method correctly implements thread-safe read access using the read lock pattern established by other getter methods.
218-229: LGTM!The
addConstraint(IReportConstraint)method correctly implements thread-safe write access, adding to both the general constraints collection and the report-specific list, matching the pattern of other constraint type overloads.databind/src/test/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGeneratorTest.java (1)
66-67: LGTM!The addition of the
getReportConstraints()mock expectation properly extends the test to support the new report constraint API, maintaining consistency with the existing constraint mocking pattern.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedReportConstraint.java (2)
5-6: Acknowledge generated binding class.This is a generated binding class. Per project conventions, any improvements should be tracked at the code generator (metaschema-maven-plugin) level rather than through manual edits to this file.
Based on learnings, generated binding classes in similar packages are pre-generated and checked into source control.
30-30: LGTM!The addition of
ITargetedConstraintBaseto the interface list aligns with the broader pattern of updating targeted constraint bindings across the PR.Also applies to: 40-41
databind/src/test/resources/content/constraints/meta-constraints/meta-constraints-report.yaml (1)
1-16: LGTM!The YAML test resource provides a concrete example of a report constraint with appropriate structure: target path, test condition, message, and informational level. This aligns with the test coverage requirements for the new report constraint feature.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintSupport.java (1)
56-58: LGTM!The addition of report constraint parsing follows the established pattern for other constraint types (expect, matches, etc.), maintaining consistency in the codebase. The implementation properly delegates to
ConstraintFactory.newReportConstraint()and adds constraints to the set using the same streaming approach.core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IFeatureValueConstrained.java (1)
64-67: LGTM!The new default methods for report constraints follow the established delegation pattern to
getConstraintSupport(), maintaining consistency with existing constraint accessor and mutator methods.Also applies to: 89-92
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.java (2)
5-6: Acknowledge generated binding class.This is a generated binding class. Per project conventions, any improvements should be tracked at the code generator (metaschema-maven-plugin) level rather than through manual edits.
Based on learnings, generated binding classes are pre-generated and checked into source control, with improvements deferred to generator enhancements.
32-32: LGTM!The addition of
ITargetedConstraintBasemaintains consistency with the other targeted constraint binding classes updated in this PR.Also applies to: 43-44
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/annotations/ValueConstraints.java (1)
69-80: LGTM!The new
report()element is well-documented, clearly explaining the semantic difference from expect constraints (generates findings when test is true vs. false). The implementation follows the established pattern for other constraint elements in this annotation.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.java (2)
5-6: Acknowledge generated binding class.This is a generated binding class. Per project conventions, any improvements should be tracked at the code generator (metaschema-maven-plugin) level rather than through manual edits.
Based on learnings, generated binding classes are pre-generated and checked into source control, with improvements deferred to generator enhancements.
30-30: LGTM!The addition of
ITargetedConstraintBasemaintains consistency with the other targeted constraint binding classes updated in this PR.Also applies to: 40-41
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedExpectConstraint.java (1)
30-41: LGTM - Generated binding class updated correctly.The addition of
ITargetedConstraintBaseto the implements clause is consistent with the broader PR changes unifying targeted constraint interfaces. This is a generated file, so any style improvements should be addressed at the code generator level. Based on learnings, generated binding class issues are tracked as generator improvements rather than file-level issues.core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraintValidationHandler.java (1)
310-336: LGTM - Well-documented callback method for report constraint violations.The new
handleReportViolationmethod follows the established pattern of other violation handlers in this interface. The Javadoc clearly explains the semantic distinction from expect constraints: report constraints generate findings when the test evaluates totrue(opposite of expect). All required@paramand@throwstags are present.core/src/main/java/gov/nist/secauto/metaschema/core/model/MetaschemaModelConstants.java (1)
120-126: LGTM - Consistent with existing constraint QName constants.The new
REPORT_CONSTRAINT_QNAMEfollows the same pattern as other constraint QNames in this file (e.g.,EXPECT_CONSTRAINT_QNAME,MATCHES_CONSTRAINT_QNAME). Javadoc format is consistent with sibling constants.databind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/impl/AnnotationGenerator.java (3)
149-149: LGTM - Report constraints integrated into flag definition value constraints.The integration follows the same pattern as other constraint types.
172-184: LGTM - Report constraints properly integrated into model definition value constraints.The condition at line 175 correctly includes
!reports.isEmpty()to ensure ValueConstraints annotation is emitted when report constraints are present. The call toapplyReportConstraintsat line 184 follows the established pattern.
378-401: LGTM - Clean implementation mirroring the expect constraints pattern.The
applyReportConstraintsmethod correctly mirrorsapplyExpectConstraints:
- Uses
Report.classfor the annotation builder- Calls
buildConstraintfor common constraint properties- Adds
test,message, andremarksmembers appropriately- Adds to annotation with
"report"member nameThis ensures consistent handling between expect and report constraint types.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/ConstraintBindingSupport.java (3)
133-136: LGTM - TargetedReportConstraint correctly added to targeted value constraint parsing.The handling follows the same instanceof pattern as other targeted constraints.
179-182: LGTM - TargetedReportConstraint correctly added to model constraint parsing.Consistent with the targeted value constraint parsing addition.
279-300: LGTM - Well-documented factory method for report constraints.The
newReportmethod:
- Has proper Javadoc explaining the semantic difference from expect constraints (generates findings when
true)- Follows the same implementation pattern as
newExpect- Uses
IReportConstraint.builder()andapplyConfigurableCommonValuesconsistentlyThe Javadoc correctly notes that report constraints are "the opposite of expect constraints."
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintFactory.java (1)
310-339: LGTM - Factory method for report constraints follows established patterns.The
newReportConstraintmethod:
- Has comprehensive Javadoc explaining the semantic behavior
- Mirrors
newExpectConstraintstructure exactly, ensuring consistency- Properly applies all constraint properties (id, formalName, description, source, level, target, properties, message, remarks)
- Sets the test expression via
builder.test()This ensures report constraints are created consistently with other constraint types.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.java (1)
30-41: LGTM - Generated binding class updated correctly.The addition of
ITargetedConstraintBaseto the implements clause is consistent with the parallel changes to other targeted constraint binding classes (TargetedExpectConstraint, etc.). This is a generated file, so any style improvements should be addressed at the code generator level. Based on learnings, generated binding class issues are tracked as generator improvements.core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IValueConstrained.java (2)
77-84: LGTM - Getter method follows established pattern.The
getReportConstraints()method signature is consistent with other constraint getters in this interface (e.g.,getExpectConstraints()), using the covariant wildcard patternList<? extends IReportConstraint>. Javadoc follows the same format as sibling methods.
126-133: LGTM - Add method overload follows established pattern.The new
addConstraint(IReportConstraint)overload is consistent with other constraint-specific add methods in this interface. Javadoc follows the same format.core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/FindingCollectingConstraintValidationHandler.java (1)
225-237: LGTM!The
handleReportViolationmethod correctly follows the established pattern used by other violation handlers in this class (e.g.,handleExpectViolation). The implementation properly:
- Uses the constraint's level for both severity and kind mapping
- Delegates message generation to the base class's
newReportViolationMessage- Builds the finding using the consistent builder pattern
core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/LoggingConstraintValidationHandler.java (1)
300-319: LGTM!The
handleReportViolationimplementation is consistent with the existinghandleExpectViolationpattern. It correctly:
- Checks if logging is enabled for the constraint's level
- Logs the message with the appropriate severity
- Uses
newReportViolationMessagefor message generationcore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/DefaultConstraintValidator.java (1)
776-854: Well-documented and correctly implemented report constraint validation.The new
validateReportmethods properly:
- Mirror the
validateExpectpattern for consistency- Correctly invert the boolean logic (fire on TRUE instead of FALSE)
- Include clear Javadoc explaining the semantic difference from expect constraints
- Handle MetapathException errors consistently via
handleErrorThe integration at lines 191, 219, and 246 is appropriately placed alongside other constraint validations.
core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/AbstractConstraintValidationHandler.java (1)
313-345: LGTM!The
newReportViolationMessagemethod follows the established pattern of other message generators in this class. The default message appropriately uses "matched" terminology (vs. "did not match" for expect) to convey the inverted semantics clearly. The Javadoc is comprehensive and correctly documents the behavior.core/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/ReportConstraintTest.java (1)
1-652: Comprehensive test coverage for IReportConstraint.Excellent test suite that covers:
- Builder API correctness and validation (required test/source)
- Default level (INFORMATIONAL) and constraint type (REPORT)
- Visitor pattern integration
- Validation pipeline behavior for TRUE/FALSE test results
- Severity level propagation (ERROR, WARNING, CRITICAL)
- Custom message handling
- The critical semantic distinction between Report and Expect constraints
The
testReportAndExpectAreOppositestest is particularly valuable for documenting and verifying the inverse semantics.core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/impl/DefaultReportConstraint.java (1)
1-92: Well-designed immutable constraint implementation.The
DefaultReportConstraintclass follows established patterns from other constraint implementations. The class is appropriately:
- Final and immutable
- Well-documented with use cases and @SInCE tag
- Using
@SuppressWarnings("PMD.ExcessiveParameterList")appropriately for the constructorNote: The
IBooleanItemimport (line 11) is used for the Javadoc@linkreference in the class documentation, which is a valid usage.databind/src/test/java/gov/nist/secauto/metaschema/databind/model/metaschema/BindingConstraintLoaderTest.java (1)
93-540: Thorough integration tests for report constraint loading.The new tests provide excellent coverage of the constraint loading pipeline:
- End-to-end loading from YAML to bound module
ConstraintBindingSupport.parse()integration- Comparison between expect and report constraint parsing paths
- Constraint preservation through
registerModuleThe diagnostic tests (
testTraceReportConstraintLoading,testCompareExpectVsReportYamlParsing) withSystem.out.printlntracing are appropriately labeled and useful for debugging pipeline issues during development.core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IReportConstraint.java (1)
1-146: Well-designed interface with clean builder pattern.The
IReportConstraintinterface is well-structured:
- Clear documentation explaining the inverse semantics compared to
IExpectConstraintDEFAULT_LEVEL = INFORMATIONALaligns with the PRD design decision- Builder correctly tracks whether level was explicitly set to support the default level behavior
- Comprehensive Javadoc on all public members
The
@since 2.0.0tag appropriately documents API versioning.databind/src/main/metaschema-bindings/metaschema-model-bindings.xml (2)
80-121: Verify PR scope: Configuration changes in a "documentation-only" PR.The PR objectives describe this as a "documentation-only PR" that adds a PRD and implementation plan, with "implementation will be submitted in a follow-up PR." However, this file contains actual configuration changes that add the
ITargetedConstraintBaseinterface to seven targeted constraint bindings. These binding configuration changes will affect generated Java classes and represent implementation changes rather than documentation.Please clarify whether:
- These configuration changes should be part of the follow-up implementation PR instead
- The PR description should be updated to reflect that it includes binding configuration changes
- These changes were intended to be included in PR #589 (which added the TargetedReportConstraint binding class)
75-79: AddIConfigurableMessageConstraintBaseinterface totargeted-allowed-values-constraintor document the intentional design difference.
TargetedAllowedValuesConstraint(lines 75-79) is the only targeted constraint without amessagefield and without theIConfigurableMessageConstraintBaseinterface. All seven other targeted constraints (expect, has-cardinality, index, index-has-key, is-unique, matches, report) implement bothITargetedConstraintBaseandIConfigurableMessageConstraintBase. Either addIConfigurableMessageConstraintBasetotargeted-allowed-values-constraintfor consistency with the rest of the targeted constraints, or explicitly document why allowed-values constraints intentionally exclude message configuration support.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/annotations/Report.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
PRDs/20251228-targeted-report-constraint/implementation-plan.md (1)
127-128: Defensive note on visitor pattern is helpful; confirms awareness of current design state.The comment that "no implementations of IConstraintVisitor exist in the codebase" is a good defensive note that aligns with the PR review feedback. It signals that the visitor interface is prepared for future use but carries lower risk. However, consider adding a forward reference to where the interface will be used or a link to relevant documentation if one exists.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CLAUDE.mdPRDs/20251228-targeted-report-constraint/implementation-plan.md
🚧 Files skipped from review as they are similar to previous changes (1)
- CLAUDE.md
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: metaschema-framework/metaschema PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T21:16:12.281Z
Learning: Constraints are documented in `website/content/specification/syntax/constraints.md`. When adding new constraint types, update the constraint type lists for `define-flag`, `define-field`, and `define-assembly`, and add a new section with syntax table and examples
📚 Learning: 2025-12-24T21:21:40.208Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/csrc/ns/metaschema/test_suite/_1_0/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:40.208Z
Learning: In metaschema-framework/metaschema-java, generated binding classes in package gov.nist.csrc.ns.metaschema.test_suite._1_0 (and similar generated binding packages) are pre-generated by metaschema-maven-plugin and checked into source control. Javadoc coverage issues in these generated classes should be tracked as code generator improvements rather than file-level issues, and improvements are deferred to generator enhancements.
Applied to files:
PRDs/20251228-targeted-report-constraint/implementation-plan.md
🔇 Additional comments (4)
PRDs/20251228-targeted-report-constraint/implementation-plan.md (4)
1-60: Well-structured implementation plan; verify status terminology aligns with PR lifecycle.The document provides clear structure, comprehensive file lists, and detailed steps. However, the "Status: Complete" label (line 35) on a PR shown as "open" in the PR objectives may cause confusion. This file appears to be a historical artifact documenting the completed implementation plan for PR #598, so clarifying its intended purpose (reference/historical record vs. active planning) would help readers.
184-206: Acceptance criteria comprehensiveness is strong; confirm all are satisfied in PR #598.All 21 criteria are marked complete with checkmarks. Given the PR is still listed as "open" in the objectives, please verify during final review that all checked criteria are actually implemented and tested in the PR code.
To confirm the acceptance criteria are fully met, you may want to cross-reference this list against the actual code changes in PR #598 and ensure:
- All core interfaces (IReportConstraint, DefaultReportConstraint) exist and are complete
- Validation logic correctly handles report constraint semantics (find when test = TRUE)
- Kind mapping matches the table (lines 225-231)
- All referenced test classes and test cases exist
225-231: Kind mapping table aligns with PR objectives correction; validates the critical fix mentioned in review feedback.The table correctly documents the corrected kind mapping (ERROR/CRITICAL → FAIL; all others → INFORMATIONAL), which resolves the FR-4 inconsistency noted in the PR objectives review feedback (commit 5dade38). This is a good reference for the implementation.
162-177: Skill documentation updates are documented but outside typical code review scope.Lines 162–177 reference updates to
.claude/skills/metaschema-constraints-authoring.mdand.claude/skills/metaschema-java-library.md. These are configuration/documentation artifacts. Verify during code review that these files were actually updated in the PR to reflect the newreportconstraint type.
Per PRD design decision, report constraints should default to INFORMATIONAL level, which differs from expect constraints that default to ERROR. This aligns with IReportConstraint interface documentation and unit tests.
david-waltermire
merged commit 6bf7cf5
into
metaschema-framework:develop
1 participant
Summary
This PR adds full constraint processing support for
TargetedReportConstraint, completing issue #592.The report constraint is the inverse of expect: it fires when the test expression evaluates to TRUE (condition detected) rather than FALSE (assertion failed). Typical use cases include deprecation warnings, informational notices, and audit findings.
Changes
Core Module
IReportConstraintinterface extendingIConfigurableMessageConstraintDefaultReportConstraintimplementation with builder patternIValueConstrainedand related interfaces withgetReportConstraints()IConstraintVisitorwithvisitReportConstraint()methodDefaultConstraintValidatorto evaluate report constraintsIConstraintValidationHandler.handleReportResult()callbackDatabind Module
@Reportannotation for generated code@ValueConstraintsto includereportarrayAnnotationGeneratorto emit@Reportannotations in generated codeConstraintFactory.newReportConstraint()for parsing annotationsConstraintBindingSupportto load report constraints from YAML/XMLDocumentation
Test Plan
IReportConstraintandDefaultReportConstraint@ReportannotationsregisterModule()mvn clean install -PCI -PreleaseCloses #592
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.