feat(databind): improve validation error messages with location and context#597
Conversation
📝 WalkthroughWalkthroughIntroduces context-rich validation error reporting: adds ValidationContext, PathTracker, and SimpleResourceLocation; threads context through JSON/XML readers and problem handlers; updates AbstractProblemHandler to emit format-appropriate, path- and location-aware messages; adds tests, PRD, and supporting documentation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Parser as JSON/XML Parser
participant Reader as MetaschemaJsonReader / MetaschemaXmlReader
participant Path as PathTracker
participant Context as ValidationContext
participant Handler as ProblemHandler
Parser->>Reader: emit token / start element/object
Reader->>Path: push(definitionSegment)
Note right of Path `#E8F0FE`: maintain current path (e.g. /root/item)
Reader->>Context: buildValidationContext(currentLocation, Path.getCurrentPath(), Format)
Context-->>Reader: ValidationContext
Reader->>Handler: handleMissingInstances(parentDef, targetObj, unhandled, ValidationContext)
Handler->>Handler: group by type, resolve names by Format, include path/location
Handler-->>Reader: throw IOException with formatted message (source, line:col, path)
Reader->>Path: pop()
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)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-12-17T13:27:43.679ZApplied to files:
🪛 LanguageTool.claude/commands/address-pr-feedback.md[grammar] ~96-~96: Use a hyphen to join words. (QB_NEW_EN_HYPHEN) ⏰ 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)
🔇 Additional comments (1)
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 (1)
PRDs/20251228-validation-errors/implementation-plan.md (1)
190-190: Fix hyphenation in compound adjective.Line 190 uses "Backward compatible" as a descriptor. When a compound adjective modifies a following noun, it should be hyphenated.
🔎 Proposed fix
-- [ ] Backward compatible default implementation +- [ ] Backward-compatible default implementation
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CLAUDE.mdPRDs/20251228-validation-errors/PRD.mdPRDs/20251228-validation-errors/implementation-plan.md
🧰 Additional context used
🧠 Learnings (5)
📚 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-validation-errors/PRD.mdCLAUDE.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: Clone repository with submodules using `git clone --recurse-submodules` and update submodules with `git submodule update --init --recursive`
Applied to files:
CLAUDE.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-validation-errors/implementation-plan.md
📚 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:
PRDs/20251228-validation-errors/implementation-plan.md
📚 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:
PRDs/20251228-validation-errors/implementation-plan.md
🪛 LanguageTool
PRDs/20251228-validation-errors/implementation-plan.md
[uncategorized] ~190-~190: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...] New handleNullFieldValue method - [ ] Backward compatible default implementation - [ ] Javadoc co...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
⏰ 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 (4)
CLAUDE.md (2)
8-9: LGTM! Submodule initialization instructions are clear and well-positioned.The addition properly addresses a common onboarding blocker and complements the existing build commands section. The instruction is precise and matches the pattern used in the project's Git Workflow section.
298-298: LGTM! PRD entry is correctly formatted and consistent.The new PRD table entry follows the established format and status tracking approach. The reference to issues #595, #596, #205 aligns with the PR objectives.
PRDs/20251228-validation-errors/implementation-plan.md (1)
1-326: Comprehensive and well-structured implementation plan.The plan effectively decomoses the validation error improvement initiative into four logical phases with clear task breakdown, acceptance criteria, and deliverables. Key strengths:
- TDD-first approach (Phase 2) aligns with project standards from CLAUDE.md
- Granular acceptance criteria enable clear verification of each task
- Risk assessment identifies and mitigates real concerns (backward compatibility, performance)
- Specific file paths for new classes make implementation unambiguous
- Verification phase includes full build checks matching CI/CD requirements
The plan correctly identifies the three core areas: format context (Task 1.1–1.3), error formatting infrastructure (Task 3.1–3.6), and reader integration (Task 3.3–3.4). Null field value handling (Task 3.5) is appropriately scoped. The task ordering respects dependencies (foundation → tests → implementation → verification).
One consideration: Task 3.5 references "line 77-78" in IBoundDefinitionModelFieldComplex—ensure this line range remains accurate if the file changes during implementation.
PRDs/20251228-validation-errors/PRD.md (1)
1-133: Excellent PRD with clear problem framing and well-defined requirements.This PRD effectively articulates the validation error improvement initiative across three linked issues. Key strengths:
- Problem statement is specific and user-centric—identifies why current messages are inadequate
- Non-goals appropriately scope future work (i18n, array indexing, constraint validation)
- Requirements R1–R7 are concrete and testable:
- Edge cases section demonstrates foresight (choice groups, default values, special characters, missing columns)
- Success metrics are measurable and verifiable through automated testing
The requirements correctly identify
getEffectiveName()as the appropriate method for format-aware property names, andIResourceLocationas the existing interface to leverage. The error message format example (lines 102–105) is practical and implementable.One observation: R5 indicates replacing
ObjectUtils.requireNonNull()calls—the implementation plan correctly identifies this in Task 3.5, but ensure all affected call sites are discovered during implementation (validate with IDE search before claiming completeness).
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java (1)
135-136: Consider documenting the UNKNOWN_SOURCE constant.While the coding guidelines mandate Javadoc only for public/protected members, this sentinel URI is central to the null-handling strategy for location reporting. Adding a brief Javadoc comment would clarify its purpose and usage for future maintainers.
📝 Suggested Javadoc
+ /** + * A synthetic URI used as a fallback when the actual resource URI is null. + * This ensures location reporting can proceed even when the document source is unknown. + */ private static final URI UNKNOWN_SOURCE = URI.create("unknown:source");databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/DefaultJsonDeserializer.java (1)
37-37: Missing Javadoc on private static field.Per the coding guidelines, all new code requires Javadoc coverage. While this is a private field, adding documentation would clarify why this synthetic URI is used and when it's applied as a fallback.
Suggested Javadoc
+ /** + * A synthetic URI used as a fallback when the document URI is null. + * This allows parsing to proceed with location tracking even when the + * source is unknown. + */ private static final URI UNKNOWN_SOURCE = URI.create("unknown:source");databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java (1)
78-79: Missing Javadoc on new field.The new
pathTrackerfield lacks Javadoc documentation. Per the coding guidelines, modified code must add/update Javadoc on any members touched, and this is a newly added field.Suggested Javadoc
+ /** + * Tracks the current parsing path for context-aware error reporting. + */ @NonNull private final PathTracker pathTracker = new PathTracker();databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java (1)
438-460: Test logic may need adjustment for null handling verification.The current test catches
IOExceptioninsideassertDoesNotThrow, which is unusual. If anIOExceptionis thrown, the test still passes as long as its cause isn't an NPE. Consider restructuring to be more explicit:🔎 Suggested clearer test structure
@Test void testNullFieldValueDoesNotThrowNpe() throws IOException { // This test will need adjustment based on how null field values can occur // For now, test that parsing doesn't throw NPE for edge cases String xml = "<root xmlns='http://csrc.nist.gov/ns/metaschema/testing/validation-errors' required-flag='rf'>" + "<required-field>value</required-field>" + "<required-assembly id='a1'/>" + "</root>"; IBindingContext bindingContext = newBindingContext(); IDeserializer<?> deserializer = bindingContext.newDeserializer(Format.XML, rootClass); - // Should not throw NullPointerException - assertDoesNotThrow(() -> { - try { - deserializer.deserialize(new StringReader(xml), URI.create("test://example.xml")); - } catch (IOException e) { - // IOException is acceptable, NPE is not - assertFalse(e.getCause() instanceof NullPointerException, - "Should not throw NullPointerException: " + e); - } - }); + // Should parse successfully - this is a valid document with all required fields + Object result = deserializer.deserialize(new StringReader(xml), URI.create("test://example.xml")); + assertNotNull(result, "Should parse successfully with all required fields"); }databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java (4)
371-377: Unused parameter ingetParentName.The
contextparameter is declared but never used in this method. Either use it for format-specific parent naming or remove it to avoid confusion.🔎 Consider removing unused parameter
@NonNull private static String getParentName( - @NonNull IBoundDefinitionModelComplex parentDefinition, - @Nullable ValidationContext context) { + @NonNull IBoundDefinitionModelComplex parentDefinition) { // Use effective name which is format-appropriate return parentDefinition.getEffectiveName(); }Then update call sites accordingly.
486-499: Unused parameter ingetInstanceName.The
contextparameter is declared but never used. The method always usesgetEffectiveName()or falls back togetJsonName(), regardless of context. Either use the context for format-specific naming or remove the parameter.🔎 Consider removing unused parameter or using it
If the intent is to use format from context for name selection:
@NonNull private static String getInstanceName( @NonNull IBoundProperty<?> instance, @Nullable ValidationContext context) { + // For XML format, prefer XML-specific names if available + Format format = context != null ? context.getFormat() : Format.JSON; + // Check for specific instance types that have getEffectiveName() if (instance instanceof IFlagInstance) { - return ((IFlagInstance) instance).getEffectiveName(); + IFlagInstance flag = (IFlagInstance) instance; + return format == Format.XML ? flag.getXmlQName().getLocalPart() : flag.getJsonName(); } else if (instance instanceof IFieldInstance) { ... } // Fall back to JSON name for other types return instance.getJsonName(); }Otherwise, remove the unused parameter to avoid confusion.
388-405: Unused parameter inbuildInstanceToChoiceMap.The
contextparameter is declared but never used - the method usesgetEffectiveName()directly without consulting the context.Same pattern as other methods - consider removing the unused parameter or documenting why it's reserved for future use.
426-443: Unused parameter inisChoiceSatisfied.The
contextparameter is declared but never used in this method.Consider removing this unused parameter or using it for format-specific logic.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.claude/rules/error-message-terminology.mdPRDs/20251228-validation-errors/PRD.mdcore/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/IProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/PathTracker.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/DefaultJsonDeserializer.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/IXmlProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.javadatabind/src/test/resources/metaschema/validation-errors/metaschema.xml
✅ Files skipped from review due to trivial changes (1)
- databind/src/test/resources/metaschema/validation-errors/metaschema.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- PRDs/20251228-validation-errors/PRD.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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/IProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/DefaultJsonDeserializer.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/IXmlProblemHandler.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/PathTracker.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.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/io/ValidationErrorMessageTest.java
🧠 Learnings (6)
📚 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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/IProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/DefaultJsonDeserializer.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/IXmlProblemHandler.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/PathTracker.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/IProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/DefaultJsonDeserializer.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/IXmlProblemHandler.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/PathTracker.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.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/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java
📚 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/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java
📚 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/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java
🧬 Code graph analysis (6)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/AutoCloser.java (1)
AutoCloser(26-136)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/DefaultJsonDeserializer.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.java (1)
SimpleResourceLocation(18-132)
databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java (2)
core/src/main/java/gov/nist/secauto/metaschema/core/model/MetaschemaException.java (1)
MetaschemaException(11-70)core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java (4)
core/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.java (1)
SimpleResourceLocation(18-132)core/src/main/java/gov/nist/secauto/metaschema/core/model/util/JsonUtil.java (1)
JsonUtil(35-432)databind/src/main/java/gov/nist/secauto/metaschema/databind/io/PathTracker.java (1)
PathTracker(31-111)databind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.java (1)
ValidationContext(30-225)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java (3)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/BindingException.java (1)
BindingException(11-56)databind/src/main/java/gov/nist/secauto/metaschema/databind/io/PathTracker.java (1)
PathTracker(31-111)databind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.java (1)
ValidationContext(30-225)
🪛 LanguageTool
.claude/rules/error-message-terminology.md
[style] ~44-~44: Consider using a different verb for a more formal wording.
Context: ...ble**: Users can immediately locate and fix issues using familiar terminology 3. **...
(FIX_RESOLVE)
⏰ 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 (25)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java (2)
141-145: LGTM! Improved location reporting.The change to pass the document URI as the systemId parameter enhances location tracking in error messages. The comment clearly explains the intent, and the implementation correctly converts the URI to an ASCII string suitable for use as a system identifier.
163-164: LGTM! Consistent URI usage.The
effectiveResourceis correctly used in both method calls, ensuring the same URI is used for XMLEventReader creation and parsing. This consistency is important for accurate location reporting in validation errors.core/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.java (1)
1-132: LGTM! Well-documented implementation.The implementation is clean and complete:
- Comprehensive Javadoc on all public members with proper @param/@return tags
- Null-safe factory methods with documented fallback to UNKNOWN
- Clear separation between XML and JSON location conversions
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.java (1)
1-225: LGTM! Excellent implementation of validation context.The class is well-designed with:
- Complete Javadoc coverage on all public members
- Proper factory pattern with clear documentation
- Explicit null handling with @nullable annotations
- Helpful formatting methods for user-facing error messages
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/IXmlProblemHandler.java (1)
100-178: LGTM! Context-aware overloads with proper documentation.The new overloads are well-designed:
- Complete Javadoc with all required tags
- Proper delegation to parent interface methods
- Backward compatibility explicitly documented
- Consistent with the IProblemHandler pattern
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/IProblemHandler.java (1)
45-76: LGTM! Backward-compatible context-aware extension.The new overload is well-implemented:
- Complete Javadoc explaining the enhancement
- Default implementation ensures backward compatibility
- Proper @nullable annotation on context parameter
- Clear documentation of delegation behavior
.claude/rules/error-message-terminology.md (1)
1-84: LGTM! Clear and actionable documentation.The documentation effectively establishes:
- User-friendly terminology mappings for XML vs JSON/YAML
- Clear examples of correct vs incorrect messages
- Explicit implementation patterns using switch statements
- Guidance for extending to future formats
This complements the ValidationContext and format-aware error handling introduced in the PR.
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java (2)
130-143: LGTM! Clean helper for building validation context.The
buildValidationContextmethod is well-documented and properly handles null locations with fallback toSimpleResourceLocation.UNKNOWN.
491-534: LGTM! Proper path tracking with cleanup guarantee.The implementation correctly:
- Pushes the definition name before processing the element
- Uses a
finallyblock to ensure the path is popped even if exceptions occur- Maintains proper stack discipline for nested elements
This pattern ensures accurate path context in error messages regardless of parsing success or failure.
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java (5)
81-82: LGTM!The
PathTrackerfield is properly initialized inline with@NonNullannotation.
180-194: LGTM! Well-structured context builder.The method correctly:
- Handles the
JsonLocation.NAcase with a fallback toSimpleResourceLocation.UNKNOWN- Uses the appropriate factory method for JSON location conversion
- Passes
Format.JSONconsistently for JSON parsing contextThe
@SuppressWarnings("resource")is appropriate here sincegetReader()returns a non-owning reference.
623-688: LGTM! Proper path tracking with try-finally.The path tracking lifecycle is correctly implemented:
- Path is pushed before processing properties
try-finallyensures the path is always popped, even on exceptions- ValidationContext is built after processing all properties for accurate location reporting
One minor note: the path is pushed using
definition.getEffectiveName()which provides format-appropriate naming.
713-720: LGTM!The new context-aware
handleMissingInstancesoverload correctly delegates to the underlying delegate handler, maintaining proper decorator pattern.
761-768: LGTM!Consistent implementation with
GroupedInstanceProblemHandler- properly delegates the context-aware call to the wrapped delegate.databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java (6)
42-62: LGTM! Well-structured test setup.The static setup correctly:
- Uses
@BeforeAllfor one-time compilation- Compiles the test metaschema module to generate binding classes
- Stores the root class for use across all nested test classes
Per coding guidelines,
@Testmethods don't require Javadoc if they use descriptive method names - which these tests do.
67-91: LGTM!The test correctly verifies that XML errors use "attribute" terminology for missing flags. The test setup is clean and the assertion checks case-insensitively.
93-114: LGTM!Symmetric test for JSON format - verifies that JSON errors use "property" terminology for missing flags.
116-158: LGTM!Good coverage for fields and assemblies in XML format, both correctly expected to use "element" terminology.
164-257: LGTM! Comprehensive location tests.Tests verify:
- File URI presence in error messages
- Line number inclusion (with multi-line input)
- Column number inclusion
- JSON location information
The regex pattern
".*\\d+[:\\s].*"for JSON location is reasonable for detecting numeric location info.
263-330: LGTM! Path context tests.Tests verify path information is included in error messages for both XML and JSON formats, including root-level path references.
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java (5)
69-88: LGTM! Good backward compatibility pattern.The original
handleMissingInstancesmethod delegates to the new context-aware overload withnullcontext, maintaining backward compatibility while enabling enhanced functionality when context is available.
107-149: LGTM! Well-structured validation with type categorization.The validation logic correctly:
- Groups missing properties by type (flags, fields, assemblies)
- Respects choice group semantics
- Only throws when there are actual missing required properties
163-178: LGTM!The categorization logic correctly identifies instance types. The fallback to
missingFieldsfor unknown types is a reasonable default.
195-255: LGTM! Comprehensive and user-friendly error formatting.The message formatting logic:
- Provides singular/plural forms appropriately
- Groups properties by type when multiple types are missing
- Includes location and path context when available
- Uses format-appropriate terminology (attribute/element for XML, property for JSON)
Good defensive check for non-root paths (
!"/".equals(path)) before appending path info.
273-310: LGTM! Clean format-appropriate terminology mapping.The switch statements correctly map:
- XML: flags → "attribute", fields/assemblies → "element"
- JSON/YAML: all → "property"
The default case provides sensible fallback for future formats.
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/PathTracker.java
Show resolved
Hide resolved
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java
Outdated
Show resolved
Hide resolved
databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
PRDs/20251228-validation-errors/implementation-plan.md (1)
176-176: Use hyphenated compound adjectives before nouns.When "backward compatible" modifies a following noun, it should be hyphenated as "backward-compatible."
🔎 Proposed fixes
- [x] Backward compatible default implementation + [x] Backward-compatible default implementation- [x] Backward compatible default implementations + [x] Backward-compatible default implementationsAlso applies to: 236-236
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
PRDs/20251228-validation-errors/implementation-plan.md
🧰 Additional context used
🧠 Learnings (3)
📚 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-validation-errors/implementation-plan.md
📚 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:
PRDs/20251228-validation-errors/implementation-plan.md
📚 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:
PRDs/20251228-validation-errors/implementation-plan.md
🪛 LanguageTool
PRDs/20251228-validation-errors/implementation-plan.md
[uncategorized] ~176-~176: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ded method with ValidationContext - [x] Backward compatible default implementation - [x] @functiona...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~236-~236: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ed methods with ValidationContext - [x] Backward compatible default implementations - [x] Javadoc c...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🔇 Additional comments (2)
PRDs/20251228-validation-errors/implementation-plan.md (2)
242-250: Clarify JSON deserialization class reference.Task 3.6 mentions both
DefaultJsonDeserializer(line 243) andDefaultXmlDeserializer, but the AI summary listsDefaultJsonProblemHandleras modified. Verify which JSON class(es) should be updated for null URI handling and ensure consistency throughout the plan and implementation.
1-310: Well-structured implementation plan with clear phases and acceptance criteria.The plan systematically addresses all three related issues (#595, #596, #205) with a test-first approach (Phase 2 before Phase 3 implementation). The four-phase structure—Foundation, Tests First, Implementation, Verification—provides a logical flow. Risk assessment and backward-compatibility considerations (default method implementations) are thoughtful. Files Changed Summary and dependencies sections provide good visibility.
Minor note: The checklist items in Phase 4 (Task 4.3, line 281) show the PR creation itself as pending, while most acceptance criteria appear complete. Consider clarifying the status of the work relative to this plan document once the PR is fully ready.
Confirm that all acceptance criteria checkbox items ([x]) truly reflect completed work before merging.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java (1)
136-137: Document the synthetic URI constant.The
UNKNOWN_SOURCEconstant serves as a fallback for location reporting when no URI is provided. Adding Javadoc would clarify its purpose and improve maintainability.📝 Suggested Javadoc
+ /** + * Synthetic URI used as a fallback for location reporting when the actual + * document URI is not available (null). + */ private static final URI UNKNOWN_SOURCE = URI.create("unknown:source");
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/PathTracker.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java
🚧 Files skipped from review as they are similar to previous changes (1)
- databind/src/main/java/gov/nist/secauto/metaschema/databind/io/PathTracker.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java
🧠 Learnings (2)
📚 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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java
🧬 Code graph analysis (1)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/AutoCloser.java (1)
AutoCloser(26-136)
⏰ 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 (1)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java (1)
34-34: LGTM! Import addition is correct.The
@Nullableimport is properly added and matches the project's null-safety annotation package (SpotBugs annotations).
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java
Outdated
Show resolved
Hide resolved
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 (5)
PRDs/20251228-validation-errors/implementation-plan.mddatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/DefaultJsonDeserializer.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java
🚧 Files skipped from review as they are similar to previous changes (2)
- databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java
- PRDs/20251228-validation-errors/implementation-plan.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/DefaultJsonDeserializer.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java
🧠 Learnings (6)
📚 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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/DefaultJsonDeserializer.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/DefaultJsonDeserializer.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java
📚 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/io/json/MetaschemaJsonReader.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/io/json/MetaschemaJsonReader.java
📚 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/io/json/MetaschemaJsonReader.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 **/*.java : Java target version must be Java 11. Use SpotBugs annotations (NonNull, Nullable) for null safety in code.
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java
🧬 Code graph analysis (1)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/DefaultJsonDeserializer.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
🔇 Additional comments (11)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java (5)
34-34: LGTM! Correct null-safety annotation import.The import of SpotBugs
@Nullableannotation aligns with the coding guidelines for null safety in Java 11 code.
136-141: LGTM! Well-documented fallback constant.The
UNKNOWN_SOURCEconstant is well-designed with clear Javadoc explaining its purpose as a synthetic URI for location reporting when the actual resource URI is unknown. This enables graceful degradation while maintaining error context.
147-151: LGTM! Improved location reporting alignment.Using
documentUri.toASCIIString()as thesystemIdcorrectly aligns the XMLEventReader's location reporting with the document URI passed through the deserialization flow.
158-161: LGTM! Defensive null handling is appropriate.The method defensively handles a potentially null
documentUriby falling back toUNKNOWN_SOURCE, ensuring the downstreamnewDocumentNodeItemcall always receives a non-null URI. This is appropriate defensive programming even though the parameter itself is not annotated@Nullable.
164-178: Excellent work addressing past review comments!This implementation properly addresses both previous review comments:
- ✅ The
@Nullableannotation on theresourceparameter (line 171) correctly reflects the null-accepting contract.- ✅ The Javadoc (lines 164-169) documents the implementation-specific null-handling behavior as required by the coding guidelines.
The implementation correctly:
- Uses
effectiveResourceas a fallback toUNKNOWN_SOURCEwhen the resource is null- Passes the effective resource consistently to both
newXMLEventReader2andparseXmlInternal- Maintains error reporting context even when the source URI is unknown
As per coding guidelines, the modified method now has proper Javadoc documenting the implementation-specific behavior.
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/DefaultJsonDeserializer.java (1)
37-41: LGTM! Well-documented fallback for null URIs.The UNKNOWN_SOURCE constant provides a clean solution for maintaining location tracking even when the document source is unknown.
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java (5)
81-85: LGTM! Well-documented path tracker.The PathTracker field is properly initialized and documented, enabling context-aware error reporting throughout the parsing process.
183-197: LGTM! Robust validation context construction.The
buildValidationContext()method properly handles the NA location edge case and assembles all required context for rich error messages.
626-691: LGTM! Proper path tracking with cleanup.The try-finally pattern ensures the path tracker is correctly maintained even when exceptions occur. The ValidationContext is properly constructed and passed to the problem handler for context-aware error reporting.
716-723: LGTM! Correct delegation pattern.The new overload properly delegates to the underlying handler, maintaining the decorator pattern for grouped instances.
764-771: LGTM! Correct delegation pattern.The new overload maintains consistency with the existing problem handler delegation pattern.
...bind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/DefaultJsonDeserializer.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java (1)
237-238: LGTM: Formatting improvement for readability.Wrapping the parse call improves readability without changing functionality.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java
🚧 Files skipped from review as they are similar to previous changes (1)
- core/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java
🧠 Learnings (6)
📚 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/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java
📚 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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.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/io/xml/MetaschemaXmlReader.java
📚 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/io/xml/MetaschemaXmlReader.java
🧬 Code graph analysis (2)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java (3)
core/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.java (1)
SimpleResourceLocation(21-135)databind/src/main/java/gov/nist/secauto/metaschema/databind/io/PathTracker.java (1)
PathTracker(32-113)databind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.java (1)
ValidationContext(30-225)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java (5)
core/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.java (1)
SimpleResourceLocation(21-135)core/src/main/java/gov/nist/secauto/metaschema/core/model/util/XmlEventUtil.java (1)
XmlEventUtil(38-710)databind/src/main/java/gov/nist/secauto/metaschema/databind/io/BindingException.java (1)
BindingException(11-56)databind/src/main/java/gov/nist/secauto/metaschema/databind/io/PathTracker.java (1)
PathTracker(32-113)databind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.java (1)
ValidationContext(30-225)
⏰ 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 (15)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java (7)
9-10: LGTM: Imports for context-aware error reporting.The new imports support ValidationContext, PathTracker, and SimpleResourceLocation for enhanced error messaging. These are correctly organized and necessary for the changes in this file.
Also applies to: 16-18
77-81: LGTM: PathTracker field with clear documentation.The field is properly annotated with
@NonNulland includes clear Javadoc explaining its purpose for context-aware error reporting.
132-145: LGTM: Well-documented helper method for ValidationContext construction.The private helper method includes complete Javadoc with
@paramand@returntags, correctly handles nullable Location input, and constructs ValidationContext with appropriate parameters including the current path from PathTracker.
254-260: LGTM: Context-aware error handling for missing flags.The ValidationContext is correctly constructed from the start element location and includes the current parsing path. This provides rich debugging information for missing flag errors.
513-514: LGTM: Consistent use of SimpleResourceLocation.The code correctly uses
SimpleResourceLocation.fromXmlLocation(location)to convert XML Location to IMetaschemaData, handling null locations appropriately. This aligns with the pattern used in buildValidationContext and replaces the deprecated MetaschemaData inner class.
289-296: Exception handling and null location handling are correctly implemented.The
buildValidationContextmethod explicitly handles the potentially null location by defaulting toSimpleResourceLocation.UNKNOWNwhen the event location is null. Additionally, thehandleMissingModelInstancesmethod signature shows the context parameter is marked@Nullable, confirming it is designed to safely handle a ValidationContext with an unknown location. No changes required.
503-504: Path tracking is correctly paired.The
pathTracker.push()at line 504 and correspondingpop()in the finally block at line 536 are properly paired. The finally block is guaranteed to execute even with the early return statement at line 532, ensuring correct cleanup regardless of control flow. All exception paths within the try block also trigger the finally block before propagating.databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java (8)
15-16: LGTM: Imports for context-aware error reporting.The new imports support ValidationContext, PathTracker, and SimpleResourceLocation for enhanced error messaging in JSON parsing, mirroring the pattern in MetaschemaXmlReader.
Also applies to: 20-22
80-84: LGTM: PathTracker field with clear documentation.The field is properly annotated with
@NonNulland includes clear Javadoc explaining its purpose for context-aware error reporting, consistent with the XML reader implementation.
182-196: LGTM: Well-documented helper method for ValidationContext construction.The private helper method includes complete Javadoc with
@returntag, correctly handlesJsonLocation.NAas unknown location, and constructs ValidationContext with Format.JSON. The implementation is consistent with the XML reader's buildValidationContext method.
498-498: LGTM: Consistent use of SimpleResourceLocation.The code correctly uses
SimpleResourceLocation.fromJsonLocation(location)to convert JsonLocation to IMetaschemaData, handlingJsonLocation.NAappropriately. This aligns with the pattern used in buildValidationContext.
678-684: LGTM: Context-aware error handling for missing instances.The ValidationContext is correctly built from the current parser state and passed to
handleMissingInstances. This provides rich debugging information including location, path, and format for missing property errors.
715-722: LGTM: Proper delegation in new overload.The new
handleMissingInstancesoverload with ValidationContext properly delegates to the delegate handler. This maintains the decorator pattern correctly while supporting the enhanced error reporting.
763-770: LGTM: Proper delegation in new overload.The new
handleMissingInstancesoverload with ValidationContext properly delegates to the delegate handler, maintaining consistency with GroupedInstanceProblemHandler's implementation.
625-690: Path tracking is correctly paired in PropertyBodyHandler.The pathTracker.push at line 626 is properly paired with pathTracker.pop in the finally block at line 689. The try-finally structure ensures the pop executes for all code paths, including exception scenarios. ValidationContext is correctly built at line 679 before the finally block executes. There is a single pathTracker.push/pop pair in this method with no orphaned operations.
Add PRD and implementation plan for issues metaschema-framework#595, metaschema-framework#596, and metaschema-framework#205: - Format-appropriate names in validation error messages - Location and path context in error messages - Better null field value handling Also documents submodule initialization requirement in CLAUDE.md.
Add comprehensive test suite for validation error messages: - Format-appropriate names (Issue metaschema-framework#595) - Location information - file URI, line, column (Issue metaschema-framework#596) - Path context in error messages (Issue metaschema-framework#596) - Property type distinction (flag/field/assembly) - Null field value handling (Issue metaschema-framework#205) - Edge cases (multiple missing, no URI, defaults) Tests follow TDD - they fail with current implementation, verifying the need for the enhancements.
…ontext (metaschema-framework#595, metaschema-framework#596, metaschema-framework#205) Add rich contextual information to validation error messages: - Use format-appropriate terminology (XML: attribute/element, JSON: property) - Include source file URI, line:column location in error messages - Add document path context (e.g., /root/child) to locate errors - Handle null URI gracefully to prevent NPE New classes: - ValidationContext: bundles source, location, path, and format - PathTracker: lightweight path tracking during parsing - SimpleResourceLocation: IResourceLocation implementation for parsers Closes metaschema-framework#595, metaschema-framework#596, metaschema-framework#205
- Add Javadoc to UNKNOWN_SOURCE constants in XML and JSON deserializers - Add Javadoc documenting null-accepting behavior in DefaultXmlDeserializer.deserializeToValueInternal() - Fix inconsistent null URI handling in deserializeToNodeItemInternal() - Add Javadoc to pathTracker fields in MetaschemaXmlReader and MetaschemaJsonReader - Fix hyphenation: "Backward compatible" → "Backward-compatible"
Remove null URI fallback code from DefaultXmlDeserializer and DefaultJsonDeserializer to align with the @nonnull contract defined in AbstractDeserializer. - Remove UNKNOWN_SOURCE constants and null-check code - Update test to use URI.create("unknown:source") instead of null
Add section to development-workflow.md explaining how to respond to automated reviewers like CodeRabbit, including mentioning them with @coderabbitai and referencing commit hashes.
…urceLocation - Change SimpleResourceLocation to implement IMetaschemaData instead of just IResourceLocation, enabling reuse for bound object metadata - Remove duplicate private MetaschemaData inner classes from both MetaschemaJsonReader and MetaschemaXmlReader - Update factory methods to return IMetaschemaData for consistency - This avoids duplicate creation of location data during parsing
david-waltermire
force-pushed
the
feature/validation-error-improvements
branch
from
e91031f to
4f328aa
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java (1)
312-330: Minor: Consider consistent Javadoc style for private methods.The private methods hasSingleType (line 315) and formatNameList (line 354) use brief single-line comments, while other private methods in this class use full Javadoc format. While not required by guidelines for private methods, consistent documentation style improves maintainability.
🔎 Optional: Add full Javadoc to private helpers
+ /** + * Check if only one type list has entries. + * + * @param flags the list of flag instances + * @param fields the list of field instances + * @param assemblies the list of assembly instances + * @return {@code true} if exactly one list is non-empty + */ private static boolean hasSingleType(+ /** + * Format a list of property names as a comma-separated string. + * + * @param instances the list of property instances + * @param context the validation context, may be null + * @return a comma-separated list of instance names + */ @NonNull private String formatNameList(Also applies to: 350-360
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.claude/rules/development-workflow.md.claude/rules/error-message-terminology.md.claude/rules/prd-conventions.mdCLAUDE.mdPRDs/20251228-validation-errors/PRD.mdPRDs/20251228-validation-errors/implementation-plan.mdcore/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/IProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/PathTracker.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/IXmlProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.javadatabind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.javadatabind/src/test/resources/metaschema/validation-errors/metaschema.xml
🚧 Files skipped from review as they are similar to previous changes (5)
- core/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/io/PathTracker.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/IXmlProblemHandler.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlDeserializer.java
- CLAUDE.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:
databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/IProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.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/io/ValidationErrorMessageTest.java
🧠 Learnings (16)
📚 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: All changes require PR review with CODEOWNERS enforcement
Applied to files:
.claude/rules/development-workflow.md.claude/rules/prd-conventions.md
📚 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/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.javadatabind/src/test/resources/metaschema/validation-errors/metaschema.xmldatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.javaPRDs/20251228-validation-errors/implementation-plan.mddatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java
📚 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/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.javadatabind/src/test/resources/metaschema/validation-errors/metaschema.xmldatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.javaPRDs/20251228-validation-errors/implementation-plan.mddatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java
📚 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/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.javadatabind/src/test/resources/metaschema/validation-errors/metaschema.xmldatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.javaPRDs/20251228-validation-errors/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: Applies to **/*.java : Follow package naming convention gov.nist.secauto.metaschema.* for all Java packages
Applied to files:
databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java
📚 Learning: 2024-11-14T05:15:46.640Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/NonNegativeIntegerItemImpl.java:24-31
Timestamp: 2024-11-14T05:15:46.640Z
Learning: In the `NonNegativeIntegerItemImpl` class (`core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/NonNegativeIntegerItemImpl.java`), the validation for non-negative values is performed in the `INonNegativeIntegerItem.valueOf(BigInteger)` method. Therefore, it's not necessary to duplicate this validation in the constructor of `NonNegativeIntegerItemImpl`.
Applied to files:
databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java
📚 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:
databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/IProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.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:
databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/IProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.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 core/metaschema/schema/xml/** : XMLBeans code is generated from XSD schemas in core/metaschema/schema/xml during Maven build. Generated sources are placed in target/generated-sources/
Applied to files:
databind/src/test/resources/metaschema/validation-errors/metaschema.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/rules/prd-conventions.mdPRDs/20251228-validation-errors/PRD.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: All PRs must be created from a personal fork and must target the develop branch (BLOCKING - required by CONTRIBUTING.md)
Applied to files:
.claude/rules/prd-conventions.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: Squash non-relevant commits before submitting PR (BLOCKING)
Applied to files:
.claude/rules/prd-conventions.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: Applies to **/*.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)
Applied to files:
.claude/rules/prd-conventions.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: All PRs require passing CI checks before merge
Applied to files:
.claude/rules/prd-conventions.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: 100% of unit tests must pass before pushing code (BLOCKING)
Applied to files:
.claude/rules/prd-conventions.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: 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:
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java
🧬 Code graph analysis (3)
databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java (2)
core/src/main/java/gov/nist/secauto/metaschema/core/model/MetaschemaException.java (1)
MetaschemaException(11-70)core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java (3)
core/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.java (1)
SimpleResourceLocation(21-135)databind/src/main/java/gov/nist/secauto/metaschema/databind/io/PathTracker.java (1)
PathTracker(32-113)databind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.java (1)
ValidationContext(30-225)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/model/SimpleResourceLocation.java (1)
SimpleResourceLocation(21-135)
🪛 LanguageTool
PRDs/20251228-validation-errors/PRD.md
[grammar] ~81-~81: Ensure spelling is correct
Context: ...miliar to users of each format - Avoids Metaschema-specific jargon ("flag", "field", "asse...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
.claude/rules/error-message-terminology.md
[style] ~44-~44: Consider using a different verb for a more formal wording.
Context: ...ble**: Users can immediately locate and fix issues using familiar terminology 3. **...
(FIX_RESOLVE)
⏰ 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 (25)
.claude/rules/development-workflow.md (1)
289-301: LGTM! Helpful addition to workflow documentation.The new "Responding to Automated Reviewers" section provides clear, actionable guidance with a practical example. This will help maintain consistent communication with automated code review tools.
.claude/rules/error-message-terminology.md (1)
1-84: LGTM! Comprehensive terminology guide.This documentation clearly defines format-appropriate terminology for error messages and provides excellent implementation guidance. The switch-statement pattern ensures maintainability when new formats are added.
PRDs/20251228-validation-errors/PRD.md (1)
1-154: LGTM! Well-structured PRD.The PRD comprehensively documents the validation error improvements with clear requirements, success metrics, and edge cases. The format specifications (R4, R7) provide actionable guidance for implementation.
databind/src/test/resources/metaschema/validation-errors/metaschema.xml (1)
1-79: LGTM! Well-designed test metaschema.The schema effectively covers the test scenarios needed for validation error messages: required/optional properties of different types, nested structures for path context testing, and various property combinations.
.claude/rules/prd-conventions.md (2)
40-50: LGTM! Important enforcement of PRD discipline.Making PRD completion updates BLOCKING ensures documentation stays synchronized with implementation. The clear steps for moving PRDs from Active to Completed prevent documentation drift.
101-126: LGTM! Valuable guidance for maintaining PRD accuracy.The new PRD Update Requirement section provides clear guidance on when to update PRDs and explains why it matters. The table format makes it easy to identify update triggers.
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/IProblemHandler.java (1)
45-76: LGTM! Well-designed backward-compatible extension.The new overload adds ValidationContext support while maintaining backward compatibility through a default implementation. Javadoc is complete and clearly explains the relationship between the two methods.
databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java (6)
42-63: LGTM! Well-structured test class with clear documentation.The class-level Javadoc clearly outlines what is being tested and references the relevant issues. The setup method properly initializes the test environment.
67-159: LGTM! Comprehensive format-appropriate naming tests.The tests effectively verify that XML errors use "attribute"/"element" terminology while JSON errors use "property" terminology, addressing Issue #595.
164-258: LGTM! Thorough location information testing.Tests cover URI, line numbers, and column numbers in error messages across both XML and JSON formats, addressing Issue #596.
263-331: LGTM! Path context tests verify error message context.Tests ensure error messages include document path context (XPath-style for XML, JSON Pointer-style for JSON), addressing Issue #596.
336-427: LGTM! Edge case coverage is comprehensive.Tests cover multiple missing properties, unknown source handling, default values, and parent element naming. Note that line 375 now correctly uses
URI.create("unknown:source")instead of null, addressing the previous review concern.
436-462: LGTM! Null field value handling test.Test verifies that null field values don't throw NullPointerException, addressing Issue #205. The assertion on line 457-458 correctly checks that the cause is not an NPE.
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java (5)
77-81: LGTM! Well-documented PathTracker field.The PathTracker field is properly documented and will maintain parsing context for error reporting.
132-145: LGTM! Clean ValidationContext builder method.The helper method properly constructs a ValidationContext from the current parser state, handling null locations gracefully by using
SimpleResourceLocation.UNKNOWN.
254-260: LGTM! Enhanced error context for missing flags.The updated code builds and passes ValidationContext to the problem handler, enabling richer error messages with location and path information.
289-296: LGTM! Enhanced error context for missing model instances.The try-catch properly handles XMLStreamException when peeking at the reader, and ValidationContext is built with the current location for improved error reporting.
503-537: LGTM! Path tracking with proper cleanup.Path tracking is correctly implemented with push/pop in a finally block, ensuring cleanup even on exceptions. The use of
SimpleResourceLocation.fromXmlLocation()provides consistent location handling.databind/src/main/java/gov/nist/secauto/metaschema/databind/io/ValidationContext.java (1)
16-225: LGTM! Excellent documentation and implementation.The ValidationContext class is well-designed with:
- Complete Javadoc coverage on all public/protected members
- Proper @param and @return tags in correct order
- Appropriate @NonNull/@nullable annotations throughout
- Clean separation of concerns with private constructor and public factory methods
- Robust handling of null/unknown values in formatting methods
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java (3)
625-690: Proper path tracking with try-finally pattern.The implementation correctly:
- Pushes the definition's effective name onto the path tracker
- Uses try-finally to ensure pathTracker.pop() executes even if exceptions occur
- Builds ValidationContext with current location and path before error reporting
- Passes ValidationContext to problem handler for enhanced error messages
182-196: Clean ValidationContext construction.The buildValidationContext() method properly:
- Handles JsonLocation.NA as UNKNOWN location
- Uses SimpleResourceLocation.fromJsonLocation() for valid locations
- Retrieves current path from pathTracker
- Defaults to Format.JSON appropriately
715-722: Consistent delegation pattern for ValidationContext support.Both problem handler wrappers (GroupedInstanceProblemHandler and JsomValueKeyProblemHandler) properly implement the new ValidationContext-aware overload by delegating to their wrapped handlers, maintaining backward compatibility while enabling enhanced error reporting.
Based on learnings, interface method overrides rely on inherited Javadoc from the interface.
Also applies to: 763-770
PRDs/20251228-validation-errors/implementation-plan.md (1)
1-301: Comprehensive implementation plan following TDD principles.The implementation plan is well-structured and documents:
- Phased approach with clear acceptance criteria
- TDD workflow (tests first, then implementation)
- Foundation classes, test scenarios, and integration points
- Risk assessment and mitigation strategies
All technical acceptance criteria are marked complete, with only PR administrative tasks remaining.
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java (2)
90-149: Excellent context-aware error message implementation.The refactored validation logic successfully addresses the PR objectives:
- Groups missing properties by type (flags, fields, assemblies)
- Uses format-appropriate terminology (attributes/elements for XML, properties for JSON/YAML)
- Includes location and path context in error messages
- Resolves instance names using effective names for format awareness
The implementation is well-documented with comprehensive Javadoc on all key methods.
Also applies to: 196-255, 274-285, 299-310, 372-377, 486-499
78-88: No Javadoc update required for the implementation.The interface
IProblemHandlerhas complete Javadoc for the newhandleMissingInstancesoverload withValidationContextparameter, including all@paramtags (including thecontextparameter) and@throwstags. The implementation at lines 78-88 correctly uses the@Overrideannotation, which allows it to inherit documentation from the interface method per established standards.
- References pr-feedback skill for categorization and verification guidance - Requires TDD for code changes when addressing feedback - Uses reviewer's actual handle (not hardcoded) for automated reviewers - Generates summary table with status of each comment - Tracks resolution with commit references
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 (1)
.claude/commands/address-pr-feedback.md
🧰 Additional context used
🧠 Learnings (1)
📚 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: All changes require PR review with CODEOWNERS enforcement
Applied to files:
.claude/commands/address-pr-feedback.md
🪛 LanguageTool
.claude/commands/address-pr-feedback.md
[grammar] ~82-~82: Use a hyphen to join words.
Context: ... [description] - [status] - ... Batch related fixes into a single commit whe...
(QB_NEW_EN_HYPHEN)
⏰ 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 (1)
.claude/commands/address-pr-feedback.md (1)
38-38: Clarify external file references or verify availability.Lines 38 and 63 reference
.claude/skills/pr-feedback.md, which isn't present in the provided context. Confirm whether this file exists in the repository or adjust the guidance accordingly to avoid leaving readers unable to find the referenced documentation.Also applies to: 63-63
- Added fetch for general PR comments (issues/{id}/comments endpoint)
- Review comments are line-specific, issue comments are PR-level discussion
- Updated Step 2 with both comment types and explanatory table
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.claude/commands/address-pr-feedback.md (1)
33-33: Define {owner} and {repo} variables in bash commands (duplicate issue from prior review).The bash snippets use template placeholders
{owner}and{repo}without defining them. Users will not know whether to replace these literally or how to extract them from context.Add a short preamble at the beginning of each code block to define these variables, then reference them consistently:
🔎 Proposed fix for Step 2 (lines 32–40)
# Get PR number PR_NUMBER=$(gh pr list --head $(git branch --show-current) --json number --jq '.[0].number') + + # Extract owner and repo from current remote + REPO_OWNER=$(gh repo view --json owner --jq '.owner.login') + REPO_NAME=$(gh repo view --json name --jq '.name') # Fetch file-level review comments (line-specific) echo "=== File Review Comments ===" - gh api repos/{owner}/{repo}/pulls/$PR_NUMBER/comments \ + gh api repos/$REPO_OWNER/$REPO_NAME/pulls/$PR_NUMBER/comments \ --jq '.[] | {id: .id, type: "review", path: .path, line: .line, body: .body[0:300], user: .user.login, in_reply_to_id: .in_reply_to_id}' # Fetch general PR comments (not tied to specific lines) echo "=== General PR Comments ===" - gh api repos/{owner}/{repo}/issues/$PR_NUMBER/comments \ + gh api repos/$REPO_OWNER/$REPO_NAME/issues/$PR_NUMBER/comments \ --jq '.[] | {id: .id, type: "issue", body: .body[0:300], user: .user.login}'Apply the same pattern to Step 5 (lines 119–128), replacing
{owner}/{repo}with$REPO_OWNER/$REPO_NAME.Also applies to: 38-38, 109-109, 114-114, 121-121, 124-124
🧹 Nitpick comments (2)
.claude/commands/address-pr-feedback.md (2)
94-94: Fix grammar: add hyphen to "Batch-related".🔎 Proposed fix
- **Batch related fixes** into a single commit when possible. + **Batch-related fixes** into a single commit when possible.
42-46: Add blank lines around tables (markdown style).Tables at lines 42–46, 175–181, and 191–199 should be surrounded by blank lines per markdownlint (MD058).
🔎 Proposed fix for table at lines 42–46
**Comment Types:** + | Type | API Endpoint | Description | |------|--------------|-------------| | Review comments | `/pulls/{id}/comments` | Line-specific comments on files | | Issue comments | `/issues/{id}/comments` | General PR-level discussion | +Apply the same pattern (blank line before and after) to the Status Icons table (lines 175–181) and the Common Automated Reviewers table (lines 191–199).
Also applies to: 175-181, 191-199
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.claude/commands/address-pr-feedback.md
🧰 Additional context used
🧠 Learnings (1)
📚 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: All changes require PR review with CODEOWNERS enforcement
Applied to files:
.claude/commands/address-pr-feedback.md
🪛 LanguageTool
.claude/commands/address-pr-feedback.md
[grammar] ~94-~94: Use a hyphen to join words.
Context: ... [description] - [status] - ... Batch related fixes into a single commit whe...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
.claude/commands/address-pr-feedback.md
43-43: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
- Use gh repo view to extract owner/repo dynamically - Updated Step 2 and Step 5 bash examples - Addresses CodeRabbit review feedback
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.claude/commands/address-pr-feedback.md
🧰 Additional context used
🧠 Learnings (1)
📚 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: All changes require PR review with CODEOWNERS enforcement
Applied to files:
.claude/commands/address-pr-feedback.md
🪛 LanguageTool
.claude/commands/address-pr-feedback.md
[grammar] ~95-~95: Use a hyphen to join words.
Context: ... [description] - [status] - ... Batch related fixes into a single commit whe...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
.claude/commands/address-pr-feedback.md
44-44: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🔇 Additional comments (2)
.claude/commands/address-pr-feedback.md (2)
29-35: ✅ Bash variable definition fix properly applied.The prior feedback about undefined
{owner}and{repo}placeholders has been correctly addressed. Line 29-30 now dynamically extract theREPOvariable usinggh repo view --json nameWithOwner, and it is properly used at line 34. The commands are now executable as-is without manual placeholder substitution.
51-51: Verify referenced skill file exists.The document references
.claude/skills/pr-feedback.mdat lines 51 and 76 for detailed categorization and feedback handling guidance. Confirm this file exists in the repository; if it does not, either create it, update the references to an existing file, or remove the references if they are out of scope.Also applies to: 76-76
- Addresses MD058 markdownlint rule for table formatting
- Added ❌ for feedback that is technically incorrect - Clarified ⏭️ Deferred is for out-of-scope items to address later - Incorrect category in Step 3 maps to Rejected status in summary
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 (1)
.claude/commands/address-pr-feedback.md
🧰 Additional context used
🧠 Learnings (1)
📚 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: All changes require PR review with CODEOWNERS enforcement
Applied to files:
.claude/commands/address-pr-feedback.md
🪛 LanguageTool
.claude/commands/address-pr-feedback.md
[grammar] ~96-~96: Use a hyphen to join words.
Context: ... [description] - [status] - ... Batch related fixes into a single commit whe...
(QB_NEW_EN_HYPHEN)
- Changed from in_reply_to parameter to /comments/{id}/replies endpoint
- More explicit and cleaner API usage
- Both approaches work, but this is the dedicated endpoint
david-waltermire
merged commit 8b8b241
into
metaschema-framework:develop
1 participant
Summary
This PR improves validation error messages during deserialization to provide better debugging context for users.
Changes
Example Error Messages
Before:
After:
Implementation
ValidationContextclass bundles parsing context (location, path, format)PathTrackerutility for lightweight path tracking during parsingAbstractProblemHandlerwith enhanced error formattingMetaschemaJsonReader,MetaschemaXmlReader) to capture and pass contextTest Plan
mvn clean install -PCI -PreleaseRelated Issues
Closes #595
Closes #596
Closes #205
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.