feat: replace Jansi with JLine to resolve Java 21+ native access warnings#611
Conversation
📝 WalkthroughWalkthroughReplaces Jansi dependency with JLine's jansi-core, updates ANSI usage and --no-color handling, adds terminal-width detection and text-wrapping for CLI help, hardens DOM use in schemagen by snapshotting/importing Elements, and adds related tests and docs. Changes
Sequence Diagram(s)(omitted — changes are library migration, CLI formatting/wrapping, DOM snapshotting, tests and docs; no new multi-component sequential flow requiring diagramming) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.java📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (4)📚 Learning: 2025-12-24T21:22:07.082ZApplied to files:
📚 Learning: 2025-12-24T21:21:40.208ZApplied to files:
📚 Learning: 2025-12-19T04:01:37.408ZApplied to files:
📚 Learning: 2025-12-27T16:52:04.509ZApplied to files:
⏰ 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 (3)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PMD (7.19.0)schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.java[ERROR] Cannot load ruleset pmd/category/java/custom.xml: Cannot resolve rule/ruleset reference 'pmd/category/java/custom.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath. metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java[ERROR] Cannot load ruleset pmd/category/java/custom.xml: Cannot resolve rule/ruleset reference 'pmd/category/java/custom.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath. Comment |
david-waltermire
force-pushed
the
feature/603-jansi-to-jline
branch
from
1311cb5 to
74b7c76
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java (1)
62-70: Add explicit instance comparison to match test intent.The test name indicates verification that
instance(true)andinstance(false)return different singletons, but the test only checks configuration differences. Per the implementation, these should be two distinct static singleton instances (NO_LOG_EXCPTION_INSTANCEandLOG_EXCPTION_INSTANCE).🔎 Suggested enhancement to verify instance difference
@Test @DisplayName("instance(true) and instance(false) return different singletons") void testInstanceWithDifferentExceptionSettings() { LoggingValidationHandler withExceptions = LoggingValidationHandler.instance(true); LoggingValidationHandler withoutExceptions = LoggingValidationHandler.instance(false); + assertThat(withExceptions).isNotSameAs(withoutExceptions); assertThat(withExceptions.isLogExceptions()).isTrue(); assertThat(withoutExceptions.isLogExceptions()).isFalse(); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
cli-processor/pom.xmlcli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.javametaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.javametaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.javapom.xml
🚧 Files skipped from review as they are similar to previous changes (3)
- pom.xml
- metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.java
- cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java
🧰 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:
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.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:
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java
🧠 Learnings (5)
📚 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:
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.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:
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.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:
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.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:
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java
📚 Learning: 2024-11-15T20:00:59.203Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java:15-15
Timestamp: 2024-11-15T20:00:59.203Z
Learning: The `DefaultExtraArgument` class in `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java` is currently a stub for future exploration.
Applied to files:
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java
🧬 Code graph analysis (3)
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java (1)
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.java (1)
LoggingValidationHandler(37-248)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java (1)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (1)
CallingContext(46-671)
⏰ 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 (13)
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java (4)
31-60: LGTM! Factory method tests are well-structured.The tests for
instance()andinstance(boolean)properly verify singleton behavior and exception logging configuration. Descriptive method names and@DisplayNameannotations provide clear test intent without requiring Javadoc.
72-100: LGTM! Factory method tests for custom configurations are correct.Tests for
withPathFormatter()andwithSettings()properly verify handler creation with custom configurations. The assertions correctly validate both the exception logging flags and non-null handler instances.
103-122: LGTM! Configuration tests document default behavior clearly.The
ConfigurationTestsnested class appropriately focuses on verifying default configuration behavior. While there is some overlap withFactoryMethodTests, the separation provides clear documentation of configuration expectations and makes the test suite easier to navigate.
18-24: The Javadoc reference togov.nist.secauto.metaschema.cli.CLITestis correct—the class exists in the codebase.cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java (3)
8-8: LGTM! Imports correctly updated to JLine's ANSI package.The static import
org.jline.jansi.Ansi.ansiand class importorg.jline.jansi.Ansicorrectly reference JLine's bundled Jansi implementation.Also applies to: 24-24
175-177: LGTM! Simplified output stream handling avoids native library loading.Using
System.outdirectly is appropriate since modern terminals (Windows 10+, Linux, macOS) handle ANSI natively. This avoids the native terminal detection that triggered JEP 472 warnings.Note that very old Windows versions (pre-Windows 10 build 1511) or unusual terminal environments may display raw ANSI escape codes. If this is a concern, consider documenting this as a known limitation.
272-275: LGTM! Correct use of JLine's ANSI disable mechanism.
Ansi.setEnabled(false)is the appropriate API to disable ANSI escape sequences globally, causing theAnsiclass to output plain text.cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java (1)
269-360: LGTM! Comprehensive test coverage forwrapTextfunctionality.The test suite covers essential scenarios:
- Text shorter than or equal to max width (no wrapping needed)
- Word boundary wrapping
- Continuation line indentation
- Force-break for text without spaces
- Empty indentation handling
- Content preservation verification
Good use of descriptive
@DisplayNameannotations per coding guidelines.cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (4)
8-8: LGTM! Import correctly updated to JLine's ANSI package.
397-452: LGTM! Terminal-width-aware help footer formatting.The implementation correctly:
- Calculates description column width based on terminal width
- Enforces a minimum width of 20 characters for readability
- Uses
wrapTextfor long descriptions with proper continuation indentation
550-574: LGTM! Environment-based terminal width detection avoids native calls.Using the
COLUMNSenvironment variable is a good approach that avoids native terminal detection. The fallback to 80 columns is reasonable for Windows and environments whereCOLUMNSisn't set.
634-670: LGTM! Help output correctly uses terminal width and handles ANSI formatting.The implementation:
- Retrieves terminal width from environment
- Enforces minimum width of 50 for readability
- Bypasses
TextHelpAppendablefor the footer to avoid incorrect wrapping calculations with ANSI escape sequencescli-processor/pom.xml (1)
44-47: LGTM! Dependency correctly migrated from Jansi to JLine.The switch from
org.fusesource.jansi:jansitoorg.jline:jlinealigns with the PR objective of resolving Java 21+ native access warnings. JLine 3.25+ includes the Jansi codebase underorg.jline.jansi, providing equivalent ANSI functionality. By removing the direct Jansi dependency and relying on JLine's terminal provider fallback mechanism, the project avoids Jansi's JNI native access warnings on Java 21+.
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java
Show resolved
Hide resolved
david-waltermire
force-pushed
the
feature/603-jansi-to-jline
branch
from
74b7c76 to
2f81494
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java (1)
81-149: Consider consolidating into a parameterized test.The seven test methods follow an identical pattern: invoke
--version, capture output, and check for a single expected substring. While explicit and clear, this approach introduces repetition.🔎 Optional refactor using JUnit 5 parameterized tests
You can consolidate these into a single parameterized test:
+import org.junit.jupiter.api.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + @Nested @DisplayName("Version Output Tests") class VersionOutputTests { - @Test - @DisplayName("version output contains app name") - void testVersionOutputContainsAppName() { - processor.process("--version"); - - String output = outputCapture.toString(StandardCharsets.UTF_8); - assertTrue(output.contains("test-cli"), "Version output should contain app name"); - } - - @Test - @DisplayName("version output contains version number") - void testVersionOutputContainsVersion() { - processor.process("--version"); - - String output = outputCapture.toString(StandardCharsets.UTF_8); - assertTrue(output.contains("1.0.0-test"), "Version output should contain version number"); - } - - @Test - @DisplayName("version output contains build timestamp") - void testVersionOutputContainsBuildTimestamp() { - processor.process("--version"); - - String output = outputCapture.toString(StandardCharsets.UTF_8); - assertTrue(output.contains("2025-01-01"), "Version output should contain build timestamp"); - } - - @Test - @DisplayName("version output contains git branch") - void testVersionOutputContainsGitBranch() { - processor.process("--version"); - - String output = outputCapture.toString(StandardCharsets.UTF_8); - assertTrue(output.contains("test-branch"), "Version output should contain git branch"); - } - - @Test - @DisplayName("version output contains git commit") - void testVersionOutputContainsGitCommit() { - processor.process("--version"); - - String output = outputCapture.toString(StandardCharsets.UTF_8); - assertTrue(output.contains("abc1234"), "Version output should contain git commit"); - } - - @Test - @DisplayName("version output contains git origin URL") - void testVersionOutputContainsGitOriginUrl() { - processor.process("--version"); - - String output = outputCapture.toString(StandardCharsets.UTF_8); - assertTrue(output.contains("https://example.com/test.git"), - "Version output should contain git origin URL"); - } - - @Test - @DisplayName("version output contains descriptive text") - void testVersionOutputContainsDescriptiveText() { - processor.process("--version"); - - String output = outputCapture.toString(StandardCharsets.UTF_8); - assertTrue(output.contains("built at"), "Version output should contain 'built at'"); - assertTrue(output.contains("from branch"), "Version output should contain 'from branch'"); - } + @ParameterizedTest(name = "version output contains {1}") + @CsvSource({ + "test-cli, app name", + "1.0.0-test, version number", + "2025-01-01, build timestamp", + "test-branch, git branch", + "abc1234, git commit", + "https://example.com/test.git, git origin URL" + }) + void testVersionOutputContainsExpectedElement(String expectedSubstring, String description) { + processor.process("--version"); + String output = outputCapture.toString(StandardCharsets.UTF_8); + assertTrue(output.contains(expectedSubstring), + "Version output should contain " + description); + } + + @Test + @DisplayName("version output contains descriptive text") + void testVersionOutputContainsDescriptiveText() { + processor.process("--version"); + String output = outputCapture.toString(StandardCharsets.UTF_8); + assertAll( + () -> assertTrue(output.contains("built at"), "Version output should contain 'built at'"), + () -> assertTrue(output.contains("from branch"), "Version output should contain 'from branch'")); + } }This reduces duplication while maintaining clear test output and failure messages.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
README.mdcli-processor/pom.xmlcli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.javametaschema-cli/src/main/distro/README.txtmetaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.javametaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.javapom.xml
🚧 Files skipped from review as they are similar to previous changes (5)
- metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java
- metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.java
- cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java
- cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java
- cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java
🧰 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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java
🧠 Learnings (4)
📚 Learning: 2025-12-30T19:08:21.896Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 605
File: src/site/markdown/index.md.vm:3-3
Timestamp: 2025-12-30T19:08:21.896Z
Learning: In the metaschema-framework/metaschema-java repository, https://framework.metaschema.dev/ is the project's preferred and canonical documentation URL for the Metaschema framework. While https://pages.nist.gov/metaschema/ is an alternative, the project uses framework.metaschema.dev as the official reference in documentation and should not be changed to pages.nist.gov URLs.
Applied to files:
README.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java
🔇 Additional comments (5)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java (1)
151-188: LGTM! Well-structured no-color mode tests.The test suite appropriately validates integration-level behavior: option acceptance and output generation. The inline comments (lines 168-169, 180-181) helpfully document the argument order constraint for phase 1 parsing.
README.md (1)
108-119: LGTM! Clear user guidance on disabling colored output.The new documentation section effectively explains ANSI support and provides clear instructions for users who need to disable colored output. The examples and scenarios are well-described.
cli-processor/pom.xml (1)
45-46: LGTM! Dependency migration correctly implemented.The replacement of org.fusesource.jansi:jansi with org.jline:jline is correct and aligns with the PR objectives to eliminate Java 21+ native access warnings. Version management through the parent POM is appropriate.
pom.xml (1)
328-330: LGTM! DependencyManagement entry correctly updated.The dependency management entry has been properly migrated from Jansi to JLine, with correct coordinates and version property reference.
metaschema-cli/src/main/distro/README.txt (1)
54-66: LGTM! Consistent user guidance in distribution README.The new section provides clear guidance on disabling colored output, consistent with the main README.md. The placement after shell completion documentation is logical and the content is well-written.
david-waltermire
force-pushed
the
feature/603-jansi-to-jline
branch
3 times, most recently
from
277b824 to
9dcdeaa
Compare
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 (11)
.lycheeignoreREADME.mdcli-processor/pom.xmlcli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.javametaschema-cli/src/main/distro/README.txtmetaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.javametaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.javapom.xml
🚧 Files skipped from review as they are similar to previous changes (5)
- README.md
- cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java
- cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java
- metaschema-cli/src/main/distro/README.txt
- metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java
🧰 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:
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java
🧠 Learnings (5)
📚 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:
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.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:
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java
📚 Learning: 2024-11-15T20:00:59.203Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java:15-15
Timestamp: 2024-11-15T20:00:59.203Z
Learning: The `DefaultExtraArgument` class in `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java` is currently a stub for future exploration.
Applied to files:
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.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: Use SpotBugs, PMD, Checkstyle, and Jacoco for static analysis. Project enforces Checkstyle code style, PMD source analysis (fails on priority 2+ violations), SpotBugs bug detection (with spotbugs-exclude.xml exclusions), and Jacoco code coverage (target: 60% coverage)
Applied to files:
.lycheeignore
📚 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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java
🧬 Code graph analysis (1)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
⏰ 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 (9)
.lycheeignore (1)
2-2: LGTM! Appropriate addition for the JLine migration.Adding the JLine repository URL to the Lychee ignore list is consistent with the migration from Jansi to JLine.
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.java (1)
8-8: LGTM! Clean library migration.The import changes correctly migrate from
org.fusesource.jansitoorg.jline.jansi. Since JLine 3.25+ merged Jansi into theorg.jline.jansinamespace, the API remains compatible and no functional changes are required.Also applies to: 22-23
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java (1)
270-387: Excellent test coverage for the wrapText() method.The test suite comprehensively validates the text wrapping functionality with well-structured test cases covering:
- Normal wrapping scenarios (word boundaries, indentation)
- Edge cases (empty indent, single long words)
- Force-break behavior (character preservation, line length verification)
- Boundary conditions (text at/below max width)
The tests follow TDD principles and use descriptive method names, which per the coding guidelines means they don't require Javadoc.
pom.xml (2)
54-54: LGTM! Dependency property correctly updated.The property rename from
dependency.jansi.versiontodependency.jline.versionwith value3.30.6is consistent with the migration strategy and was confirmed in previous review discussions.
328-330: LGTM! Dependency declaration correctly migrated.The dependency declaration properly switches from
org.fusesource.jansi:jansitoorg.jline:jlineand correctly references the updated${dependency.jline.version}property.cli-processor/pom.xml (1)
45-46: LGTM! Module dependency correctly migrated.The dependency declaration properly switches from
org.fusesource.jansi:jansitoorg.jline:jline. The version will be inherited from the parent POM's dependency management, ensuring consistency across the project.cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (3)
8-8: LGTM: Import correctly updated for JLine migration.The static import change from
org.fusesource.jansi.Ansi.ansitoorg.jline.jansi.Ansi.ansicorrectly reflects the JLine 3.x package structure where Jansi functionality was merged.
550-574: LGTM: Terminal width detection correctly avoids native calls.The implementation successfully avoids native terminal detection that would trigger Java 21+ warnings by reading the
COLUMNSenvironment variable with a sensible 80-column fallback. Proper validation and error handling are in place.
635-671: LGTM: Help rendering correctly uses environment-based terminal width.The updated
showHelp()method properly:
- Uses
getTerminalWidth()to avoid native terminal detection- Sets a reasonable minimum width of 50 for the help appendable
- Removes the empty "Since" column per PR objectives
- Prints the footer directly to handle ANSI escape sequences correctly
The implementation aligns well with the PR objectives, though the infinite loop issue in
wrapText()should be addressed to ensure robustness when handling long command names in narrow terminals.
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java
Show resolved
Hide resolved
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java
Show resolved
Hide resolved
david-waltermire
force-pushed
the
feature/603-jansi-to-jline
branch
from
9dcdeaa to
0f701a7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (2)
589-627: Critical: Missing validation can cause infinite loop.The wrapText() method doesn't validate that
indent.length() < maxWidth. Whenindent.length() >= maxWidth:
- Line 602:
effectiveWidth = maxWidth - indent.length()becomes ≤ 0- Line 612:
lineEnd = lineStart + effectiveWidthresults inlineEnd ≤ lineStart- Line 617 or 620:
append()does nothing or fails- Line 618 or 621:
lineStartdoesn't advance- Loop continues indefinitely
This exact issue was flagged in previous reviews and remains unaddressed.
The current test suite doesn't cover this edge case (all tests use
indent.length() < maxWidth).🔎 Proposed fix: Add validation
@NonNull static String wrapText(@NonNull String text, int maxWidth, @NonNull String indent) { + if (maxWidth <= 0) { + throw new IllegalArgumentException("maxWidth must be positive, got: " + maxWidth); + } + if (indent.length() >= maxWidth) { + throw new IllegalArgumentException( + "indent length (" + indent.length() + ") must be less than maxWidth (" + maxWidth + ")"); + } + if (text.length() <= maxWidth) { return text; }
424-432: Critical: Ensure indent length is less than description width.The calculation doesn't validate that
continuationIndent.length() < descWidth. When terminal width is narrow or command names are long:Example failure scenario:
- Command name: 50 chars →
prefixWidth = 54- Terminal width: 40 →
descWidth = 20continuationIndent.length() = 54- Calling
wrapText(..., 20, " ".repeat(54))triggers the infinite loop (see previous comment)This scenario was explicitly flagged in earlier reviews.
🔎 Proposed fix: Validate and cap indent width
int commandColWidth = subCommands.stream() .mapToInt(command -> command.getName().length()) .max().orElse(0); -// Calculate description column width: terminal - 3 (leading spaces) - -// commandCol - 1 (space) -int prefixWidth = 3 + commandColWidth + 1; -int descWidth = Math.max(terminalWidth - prefixWidth, 20); -String continuationIndent = " ".repeat(prefixWidth); +// Calculate description column width: terminal - 3 (leading spaces) - +// commandCol - 1 (space) +int prefixWidth = 3 + commandColWidth + 1; +int descWidth = Math.max(terminalWidth - prefixWidth, 20); + +// Ensure indent is shorter than description width to avoid infinite loop in wrapText +int indentWidth = Math.min(prefixWidth, descWidth - 1); +String continuationIndent = " ".repeat(Math.max(0, indentWidth));
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.lycheeignoreREADME.mdcli-processor/pom.xmlcli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.javametaschema-cli/src/main/distro/README.txtmetaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.javametaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.javapom.xml
🚧 Files skipped from review as they are similar to previous changes (6)
- cli-processor/pom.xml
- README.md
- metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.java
- .lycheeignore
- metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java
- metaschema-cli/src/main/distro/README.txt
🧰 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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java
🧠 Learnings (4)
📚 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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java
📚 Learning: 2024-11-15T20:00:59.203Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java:15-15
Timestamp: 2024-11-15T20:00:59.203Z
Learning: The `DefaultExtraArgument` class in `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java` is currently a stub for future exploration.
Applied to files:
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java
🧬 Code graph analysis (3)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java (1)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (1)
CallingContext(46-672)
🪛 GitHub Actions: Build and Test
pom.xml
[error] 1-1: Failed to read artifact descriptor for org.codehaus.plexus:plexus-io:jar:3.4.1 during maven-source-plugin:jar-no-fork execution. Plugin or dependencies could not be resolved.
🔇 Additional comments (8)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java (1)
8-8: LGTM! Clean migration from Jansi to JLine.The import changes and removal of AnsiConsole-based terminal detection successfully eliminate Java 21+ native access warnings while maintaining ANSI functionality through modern terminal support.
Also applies to: 24-24, 175-177
pom.xml (2)
54-54: LGTM! JLine version is current.Version 3.30.6 is the latest stable release as confirmed in previous review discussions.
328-331: No evidence supports the claimed pipeline failure.The repository shows no evidence of a plexus-io 3.4.1 resolution failure. The commit (0f701a7) successfully migrated from org.fusesource.jansi:jansi 2.4.2 to org.jline:jline 3.30.6, with all 113 CLI tests passing. There is no maven-source-plugin configuration in the pom.xml, no plexus-io dependency declared or referenced anywhere in the codebase, and no error logs or failure documentation related to this issue.
Likely an incorrect or invalid review comment.
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java (1)
81-188: Excellent test coverage for version output and no-color mode.The new test suites comprehensively verify:
- Version output formatting (app name, version, timestamps, git metadata, descriptive text)
- No-color mode behavior across different command combinations
- Proper exit codes and output generation
The descriptive test method names eliminate the need for Javadoc per project guidelines.
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java (1)
270-387: Comprehensive test coverage for text wrapping functionality.The WrapTextTests suite thoroughly validates the wrapText() method with excellent edge case coverage:
- Boundary conditions (shorter, equal, longer than max width)
- Word-boundary wrapping with proper indentation
- Force-breaking for long words without spaces
- Content preservation and character accuracy
This follows TDD principles and uses descriptive test names per project guidelines.
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (3)
8-8: LGTM! Import updated for JLine migration.
550-574: Excellent design for terminal width detection.This implementation successfully avoids Java 21+ native access warnings by using the COLUMNS environment variable with a sensible 80-column fallback. The error handling is robust, and the Javadoc clearly explains the rationale.
637-668: Well-integrated terminal width detection and help rendering.The changes effectively:
- Use
getTerminalWidth()to set appropriate text wrapping bounds- Disable the "Since" column (line 651) for cleaner output
- Print the footer directly to properly handle ANSI escape sequences (lines 666-668)
The inline comments clearly explain the design decisions.
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java
Show resolved
Hide resolved
david-waltermire
force-pushed
the
feature/603-jansi-to-jline
branch
from
0f701a7 to
040ceba
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (2)
424-432: Essential: Validate indent length against description width to prevent infinite loop.The calculation does not ensure
continuationIndent.length() < descWidth. When command names are long relative toterminalWidth,prefixWidthcan exceeddescWidth, causingwrapText()to enter an infinite loop (sinceindent.length() >= maxWidth).Example: If the longest command name is 50 characters and
terminalWidth = 60, thenprefixWidth = 54anddescWidth = max(60 - 54, 20) = 20. The continuation indent of 54 characters exceeds the description width of 20, triggering the infinite loop.🔎 Proposed fix: Cap prefixWidth to leave room for description
int commandColWidth = subCommands.stream() .mapToInt(command -> command.getName().length()) .max().orElse(0); -// Calculate description column width: terminal - 3 (leading spaces) - -// commandCol - 1 (space) -int prefixWidth = 3 + commandColWidth + 1; +// Calculate prefix width, ensuring we leave enough room for description +int rawPrefixWidth = 3 + commandColWidth + 1; +// Reserve at least 30 chars for description to ensure indent < descWidth +int maxPrefixWidth = Math.max(terminalWidth - 30, terminalWidth / 2); +int prefixWidth = Math.min(rawPrefixWidth, maxPrefixWidth); + int descWidth = Math.max(terminalWidth - prefixWidth, 20); + +// Ensure indent is strictly shorter than description width +if (prefixWidth >= descWidth) { + prefixWidth = Math.max(1, descWidth - 1); +} String continuationIndent = " ".repeat(prefixWidth);
589-627: Critical: Infinite loop when indent length equals or exceeds maxWidth.The
wrapText()method does not validate thatindent.length() < maxWidth. On continuation lines (line 602), wheneffectiveWidth = maxWidth - indent.length()becomes ≤ 0:
lineEnd = lineStart + effectiveWidthresults inlineEnd ≤ lineStart(line 612)- The loop cannot advance because
lineStartis not incremented- Infinite loop ensues
Example scenario: If
buildHelpFootercomputesprefixWidth = 54anddescWidth = 20, thenindent.length() = 54andeffectiveWidth = 20 - 54 = -34, triggering the infinite loop.🔎 Proposed fix: Add validation at method entry
@NonNull static String wrapText(@NonNull String text, int maxWidth, @NonNull String indent) { + if (maxWidth <= 0) { + throw new IllegalArgumentException("maxWidth must be positive"); + } + if (indent.length() >= maxWidth) { + throw new IllegalArgumentException( + "indent length (" + indent.length() + ") must be less than maxWidth (" + maxWidth + ")"); + } + if (text.length() <= maxWidth) { return text; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.lycheeignoreREADME.mdcli-processor/pom.xmlcli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.javametaschema-cli/src/main/distro/README.txtmetaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.javametaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.javapom.xml
🚧 Files skipped from review as they are similar to previous changes (7)
- metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.java
- cli-processor/pom.xml
- cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java
- README.md
- cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java
- metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java
- pom.xml
🧰 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:
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java
🧠 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: Use SpotBugs, PMD, Checkstyle, and Jacoco for static analysis. Project enforces Checkstyle code style, PMD source analysis (fails on priority 2+ violations), SpotBugs bug detection (with spotbugs-exclude.xml exclusions), and Jacoco code coverage (target: 60% coverage)
Applied to files:
.lycheeignore
📚 Learning: 2024-11-15T20:00:59.203Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java:15-15
Timestamp: 2024-11-15T20:00:59.203Z
Learning: The `DefaultExtraArgument` class in `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java` is currently a stub for future exploration.
Applied to files:
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.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:
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.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:
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java
🧬 Code graph analysis (1)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
⏰ 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 (2)
.lycheeignore (1)
2-2: Correct placement and formatting of the JLine3 ignore pattern.The addition of the
https://github.com/jline/jline3/.*ignore entry is appropriate and follows the established format for similar dependency references (e.g., line 35). This change aligns with the PR's migration from Jansi to JLine and prevents Lychee from attempting to validate JLine3 repository URLs.metaschema-cli/src/main/distro/README.txt (1)
54-65: Well-documented new feature section.The "Disabling Color Output" section is clearly written, well-formatted, and consistent with the existing documentation structure. It effectively explains ANSI terminal support, common scenarios where issues might occur (legacy consoles, CI/CD environments, file redirection), and provides a practical solution with a clear usage example.
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestSuite.java (1)
1-134: Revert manual edits to generated binding class.This file is explicitly marked as generated (lines 5-6: "Generated from: ../../../../../../../../metaschema/unit-tests.yaml" and "Do not edit - changes will be lost when regenerated"). The cosmetic formatting and Javadoc changes in this file violate project conventions and will be lost when the code is regenerated.
Additionally, these changes are unrelated to PR #611's objective of replacing Jansi with JLine.
Recommended actions:
- Revert all changes to this file from the current PR
- If formatting/Javadoc improvements are needed, implement them at the code generator level (metaschema-maven-plugin) to ensure consistency across all generated bindings
Based on learnings, generated binding classes should not be manually edited; improvements must be tracked as generator enhancements.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathNamespace.java (1)
1-138: Do not manually edit generated binding classes.This file is explicitly marked as generated code (lines 5-6) with a "Do not edit - changes will be lost when regenerated" warning. The formatting changes in this file are purely cosmetic and unrelated to the PR's objective of migrating from Jansi to JLine.
Based on learnings from this repository, formatting and style improvements to generated binding classes should be implemented at the code generator level (metaschema-maven-plugin) rather than by manually editing the generated output. Manual edits will be lost the next time the generator runs.
Recommendation: Revert the formatting changes to this file, or if these improvements are desired across all generated binding classes, implement them in the metaschema-maven-plugin code generator instead.
Based on learnings, generated binding classes should be improved at the generator level, not through manual edits.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineField.java (1)
767-770: Add Javadoc for the new public toString() method.The new
toString()method is missing Javadoc documentation. Per coding guidelines, all public/protected members must have Javadoc coverage including @return tags.Additionally, if this file is truly generated, the
toString()method should be added by the code generator (metaschema-maven-plugin) to ensure consistency across all generated binding classes.As per coding guidelines, new/modified public methods require complete Javadoc.
🔎 Suggested Javadoc
+ /** + * Produces a string representation of this object using reflection. + * + * @return a multi-line string representation of this object + */ @Override public String toString() { return ObjectUtils.notNull(new ReflectionToStringBuilder(this, ToStringStyle.MULTI_LINE_STYLE).toString()); }
🧹 Nitpick comments (1)
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Property.java (1)
5-7: Manual edits to generated code should be avoided; apply formatting improvements at the generator level.The file header (lines 5-6) explicitly marks this as generated code with the warning "Do not edit - changes will be lost when regenerated." The formatting and Javadoc improvements in this file will be lost the next time the code generator runs.
Additionally, this file is unrelated to the PR's stated objective of replacing Jansi with JLine to resolve Java 21+ native access warnings.
Based on learnings, documentation and style improvements for generated binding classes should be tracked and implemented at the metaschema-maven-plugin generator level rather than through manual edits to generated sources.
Consider either:
- Reverting these changes to keep this PR focused on the Jansi→JLine migration, or
- Opening a separate issue to track these formatting improvements at the code generator level
Also applies to: 27-27, 35-35, 42-42, 49-49, 53-55, 62-64, 67-67, 92-92, 112-112, 132-132
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (57)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/MetaschemaBindings.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/MetaschemaBindingsModule.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/package-info.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Any.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyModel.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyReference.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/ConstraintLetExpression.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/ConstraintValueEnum.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Example.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FieldConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FieldReference.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagAllowedValues.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagExpect.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagIndexHasKey.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagMatches.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagReference.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagReport.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/GroupingAs.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineAssembly.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineFlag.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/JsonKey.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/JsonValueKeyFlag.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/KeyConstraintField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/METASCHEMA.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathNamespace.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaMetaConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaMetapath.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaModelModule.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaModuleConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Property.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Remarks.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedAllowedValuesConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedExpectConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexHasKeyConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedMatchesConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedReportConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/UseName.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/package-info.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/GenerateSchema.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/GenerationCase.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/Metaschema.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/MetaschemaTestSuiteModule.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestCollection.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestScenario.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestSuite.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/ValidationCase.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/package-info.java
✅ Files skipped from review due to trivial changes (32)
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyReference.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FieldReference.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedAllowedValuesConstraint.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/GroupingAs.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagMatches.java
- metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/Metaschema.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/package-info.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagReport.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedReportConstraint.java
- metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/MetaschemaTestSuiteModule.java
- metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/ValidationCase.java
- metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestCollection.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/JsonValueKeyFlag.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexHasKeyConstraint.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/MetaschemaBindings.java
- metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/GenerationCase.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/JsonKey.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyConstraints.java
- metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/package-info.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagIndexHasKey.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaMetapath.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Any.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagExpect.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedExpectConstraint.java
- metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/GenerateSchema.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineAssembly.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Remarks.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaModuleConstraints.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/ConstraintValueEnum.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FieldConstraints.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedMatchesConstraint.java
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaModelModule.java
🚧 Files skipped from review as they are similar to previous changes (2)
- cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java
- cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java
🧰 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/model/metaschema/binding/FlagReference.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/MetaschemaBindingsModule.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Example.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagAllowedValues.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Property.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaMetaConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/package-info.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestSuite.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineFlag.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/UseName.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestScenario.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathNamespace.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/ConstraintLetExpression.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyModel.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/KeyConstraintField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagConstraints.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java
🧠 Learnings (11)
📚 Learning: 2025-12-24T21:21:56.361Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:56.361Z
Learning: In metaschema-testing, generated binding classes under gov.nist.secauto.metaschema.model.testing.testsuite are produced by metaschema-maven-plugin from YAML metaschema definitions. Javadoc issues in these generated classes should not be flagged for manual fixes; improvements are tracked and handled through code generator enhancements rather than manual edits to the generated source.
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagReference.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/MetaschemaBindingsModule.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Example.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagAllowedValues.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Property.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaMetaConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/package-info.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineFlag.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/UseName.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathNamespace.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/ConstraintLetExpression.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyModel.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/KeyConstraintField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagConstraints.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/model/metaschema/binding/FlagReference.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/MetaschemaBindingsModule.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Example.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagAllowedValues.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Property.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaMetaConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/package-info.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestSuite.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineFlag.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/UseName.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestScenario.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathNamespace.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/ConstraintLetExpression.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyModel.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/KeyConstraintField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagConstraints.java
📚 Learning: 2025-12-24T21:22:07.082Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/GenerationCase.java:74-80
Timestamp: 2025-12-24T21:22:07.082Z
Learning: Files in the package gov.nist.secauto.metaschema.model.testing.testsuite in metaschema-testing are generated binding classes created from Metaschema definitions. Documentation and style improvements for these files should be made at the code generator level (metaschema-maven-plugin) rather than by manually editing the generated code.
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagReference.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/MetaschemaBindingsModule.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Example.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagAllowedValues.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Property.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaMetaConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/package-info.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineFlag.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/UseName.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathNamespace.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/ConstraintLetExpression.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyModel.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/KeyConstraintField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagConstraints.java
📚 Learning: 2025-12-30T04:19:49.192Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 605
File: databind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/typeinfo/DefaultMetaschemaClassFactory.java:480-484
Timestamp: 2025-12-30T04:19:49.192Z
Learning: In metaschema-framework/metaschema-java, when users configure custom superinterfaces via binding configuration, david-waltermire prefers both documentation (explaining how to add dependencies to the Maven plugin) and validation mechanisms (warnings when classes aren't on classpath) to prevent redundant IBoundObject interface additions during code generation.
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagReference.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/MetaschemaBindingsModule.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Example.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagAllowedValues.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Property.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaMetaConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/package-info.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineFlag.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/UseName.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathNamespace.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/ConstraintLetExpression.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyModel.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/KeyConstraintField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagConstraints.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/model/metaschema/binding/FlagReference.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/MetaschemaBindingsModule.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Example.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagAllowedValues.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Property.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaMetaConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/package-info.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestSuite.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineFlag.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/UseName.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestScenario.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathNamespace.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/ConstraintLetExpression.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyModel.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/KeyConstraintField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagConstraints.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/model/metaschema/binding/FlagReference.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/MetaschemaBindingsModule.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Example.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagAllowedValues.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Property.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaMetaConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/package-info.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestSuite.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineFlag.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/UseName.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestScenario.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathNamespace.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathContext.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/ConstraintLetExpression.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyModel.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/KeyConstraintField.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagConstraints.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 : Follow package naming convention gov.nist.secauto.metaschema.* for all Java packages
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/MetaschemaBindingsModule.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/package-info.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestSuite.java
📚 Learning: 2024-11-14T17:07:03.586Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IIPv4AddressItem.java:66-73
Timestamp: 2024-11-14T17:07:03.586Z
Learning: In the Metaschema Java codebase, differences in casting patterns across atomic type implementations are intentional and required; any differences in approach are significant and necessary.
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaMetaConstraints.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyModel.java
📚 Learning: 2025-12-24T21:21:59.692Z
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:21:59.692Z
Learning: Do not manually edit generated binding classes produced from Metaschema definitions (e.g., in metaschema-testing's testing suite). Document and style improvements should be made at the code generator level (metaschema-maven-plugin) so that generated code remains consistent and regenerable. If you need to fix issues, adjust the generator templates or generation process rather than editing the generated .java files directly.
Applied to files:
metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestSuite.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestScenario.java
📚 Learning: 2025-12-24T21:21:52.756Z
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:52.756Z
Learning: In metaschema-testing, generated binding classes under gov.nist.secauto.metaschema.model.testing.testsuite are produced by the metaschema-maven-plugin. Do not flag Javadoc issues in these generated classes for manual fixes; improvements should be tracked and implemented via code generator enhancements. Treat such issues as generator-related and avoid edits to generated source unless regenerating."
Applied to files:
metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestSuite.javametaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestScenario.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/model/metaschema/binding/MetapathNamespace.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathContext.java
🧬 Code graph analysis (15)
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagReference.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java (1)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (1)
CallingContext(46-679)
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagAllowedValues.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/model/metaschema/binding/MetaschemaMetaConstraints.java (2)
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/Let.java (1)
Let(23-156)core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestSuite.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/model/metaschema/binding/TargetedHasCardinalityConstraint.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/model/metaschema/binding/InlineDefineFlag.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestScenario.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/model/metaschema/binding/MetapathNamespace.java (4)
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyModel.java (7)
MetaschemaAssembly(41-2329)MetaschemaAssembly(143-244)MetaschemaAssembly(246-2328)MetaschemaAssembly(511-859)MetaschemaAssembly(861-1314)MetaschemaAssembly(1316-1724)MetaschemaAssembly(1726-2327)databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Example.java (1)
MetaschemaAssembly(25-177)databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FieldReference.java (1)
MetaschemaAssembly(38-500)databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagAllowedValues.java (1)
MetaschemaAssembly(35-447)
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathContext.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/model/metaschema/binding/TargetedIsUniqueConstraint.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/model/metaschema/binding/InlineDefineField.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/model/metaschema/binding/TargetedIndexConstraint.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/model/metaschema/binding/AssemblyModel.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/model/metaschema/binding/FlagConstraints.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
🔇 Additional comments (19)
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetaschemaMetaConstraints.java (1)
1-552: LGTM! Generated file with non-functional formatting changes.This is a generated binding class (as noted in lines 5-6), and all changes are purely Javadoc text rewrapping and annotation formatting adjustments with no functional or behavioral modifications. Based on learnings, documentation improvements in generated binding classes are appropriately handled through code generator enhancements rather than manual edits.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/AssemblyModel.java (2)
1-7: Generated binding class - changes appear to be from regeneration.This file is auto-generated from Metaschema definitions (as noted in lines 5-6). The changes consist of formatting adjustments and constraint updates that would result from regenerating the bindings. Based on learnings, any issues with generated code should be tracked as code generator improvements rather than manual edits.
However, this file (
databind/.../AssemblyModel.java) appears unrelated to the PR's primary objective of migrating from Jansi to JLine for CLI processing. Please verify this file was intentionally included in the PR and not accidentally staged during the build/regeneration process.
1757-1820: Constraint update:as-typefield now uses AllowedValues instead of regex pattern.The
as-typeflag constraint has been updated from a regex-based pattern to an explicit@AllowedValuesenumeration withallowOthers = true. This is a more descriptive approach that:
- Lists all supported data types with documentation links
- Includes deprecated type aliases with
deprecatedVersionannotations- Allows extensibility via
allowOthers = trueThis appears to be a schema/constraint improvement generated from updated Metaschema definitions. Since this is generated code, the change should be validated by ensuring the source Metaschema definition is correct.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineFlag.java (1)
1-548: LGTM! Generated binding class with formatting improvements.This is a generated file (per header lines 5-6 and retrieved learnings), so any style/documentation adjustments are appropriately made at the code generator level. The changes consist of:
- Whitespace and line-wrapping consistency in annotations and Javadoc
- Improved
requireNonNullcalls at lines 425, 441, 524, and 540 now include descriptive error messages ("item cannot be null")The error message additions improve debuggability without changing behavior. Based on learnings, improvements to generated classes are correctly tracked through generator enhancements.
databind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/MetaschemaBindingsModule.java (1)
7-58: Generated file changes unrelated to PR objective.This file is marked as generated code (lines 5-6) and should not be manually edited. The changes are formatting and documentation improvements, which suggests the code generator was updated and this file was regenerated.
However, this file is in the
databindmodule and appears unrelated to the PR's stated objective of replacing Jansi with JLine in the CLI processor. The changes are fine if the generator was intentionally updated, but please confirm:
- Was this file regenerated as part of routine maintenance?
- Should these changes be in a separate PR focused on generator improvements?
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagAllowedValues.java (1)
1-447: Generated file changes appear benign.This file is a generated binding class (lines 5-6) and appears to have been regenerated during the PR, resulting in formatting improvements, Javadoc text reflow, and enhanced parameter validation (lines 343, 359, 394, 410). These changes are unrelated to the PR's Jansi-to-JLine migration objectives but are acceptable.
As noted in the learnings, improvements to generated binding classes should be tracked at the code generator level rather than in manual file reviews.
Based on learnings, generated binding classes in gov.nist.secauto.metaschema.databind.model.metaschema.binding are produced by metaschema-maven-plugin, and Javadoc/style improvements should be handled through generator enhancements.
metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/TestScenario.java (1)
5-7: Generated code changes are appropriate.This file is marked as generated (lines 5-6) and the formatting/Javadoc refinements plus defensive null checks (lines 183, 199) appear to be artifacts of regenerating the binding classes with an updated code generator. Per project learnings, generated binding classes should not be manually edited, and improvements should be made at the generator level. Since this file was regenerated rather than manually edited, these changes are acceptable.
Based on learnings, this is the correct approach for maintaining generated binding code.
Also applies to: 33-33, 45-45, 54-54, 65-65, 69-71, 78-80, 83-83, 114-114, 140-140, 169-169, 177-180, 183-183, 191-195, 199-199
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagReference.java (1)
1-414: Generated binding class with improved null-safety messaging.This is a generated file (per header comment and learnings). The regenerated code includes improved input validation using
ObjectUtils.requireNonNull(item, "item cannot be null")inaddProp/removePropmethods, along with consistent formatting and Javadoc reflow. Based on learnings, any style improvements should be tracked at the code generator level rather than manual edits.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIndexConstraint.java (1)
1-456: Generated binding class with improved null-safety messaging.This regenerated file includes improved input validation with descriptive error messages in collection manipulation methods (
addProp,removeProp,addKeyField,removeKeyField). Formatting and Javadoc changes are consistent with generator output. Based on learnings, improvements are tracked at the code generator level.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/Example.java (1)
1-177: Generated binding class with formatting refinements.This regenerated file contains only formatting and Javadoc reflow changes with no behavioral modifications. The changes are consistent with the broader formatting pass observed across the databind module.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedHasCardinalityConstraint.java (1)
1-427: Generated binding class with improved null-safety messaging.This regenerated file includes improved input validation with descriptive error messages in
addPropandremovePropmethods. Formatting and Javadoc changes are consistent with generator output.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/UseName.java (1)
1-114: Generated binding class with formatting refinements.This regenerated file contains only formatting and Javadoc reflow changes with no behavioral modifications.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/package-info.java (1)
14-21: Added XML namespace configuration for binding package.The new
@XmlSchemaannotation properly configures XML namespace handling with the Metaschema namespace (http://csrc.nist.gov/ns/oscal/metaschema/1.0) and qualified element form default. This is a meaningful generator improvement that ensures correct XML serialization behavior for binding classes in this package.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/ConstraintLetExpression.java (1)
1-152: Generated binding class with formatting refinements and nullability annotation.This regenerated file includes formatting/Javadoc reflow and correctly annotates
getRemarks()with@Nullable(line 129), properly documenting that the optional remarks field may return null.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/FlagConstraints.java (1)
1-169: Generated binding class changes look correct.This is a generated binding class (per header comment). The changes include:
- Added
getMetaschemaData()getter (lines 78-81) to expose internal metaschema data- Formatting improvements to annotations and Javadoc
The new getter correctly implements the interface contract (indicated by
@Override) and requires no additional Javadoc per project standards for overridden methods. All changes appear appropriate for generated code.Note: Based on learnings, any style or documentation improvements for generated binding classes should be addressed at the code generator level (metaschema-maven-plugin) rather than in the generated source.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/KeyConstraintField.java (1)
1-150: Generated binding class changes look correct.This is a generated binding class (per header comment). The changes mirror those in
FlagConstraints.java:
- Added
getMetaschemaData()getter (lines 73-76) to expose internal metaschema data- Formatting improvements to annotations and Javadoc
The new getter correctly implements the interface contract (indicated by
@Override) and follows the same pattern as the corresponding change inFlagConstraints.java. All changes appear appropriate for generated code.Note: Based on learnings, any style or documentation improvements for generated binding classes should be addressed at the code generator level (metaschema-maven-plugin) rather than in the generated source.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/TargetedIsUniqueConstraint.java (1)
5-7: LGTM! Generated file appropriately updated.This is a generated binding class (per the header at lines 5-6), and the changes appear to be from code generator updates rather than manual edits. The modifications are formatting and documentation improvements only:
- Expanded
@AllowedValuesdescriptions (lines 55-65) now provide detailed explanations for each severity level (CRITICAL, ERROR, WARNING, INFORMATIONAL, DEBUG)- Consistent formatting improvements to
requireNonNullmessages (lines 304, 320, 355, 371)- Javadoc line-wrapping enhancements throughout
- No functional or API changes
These changes improve code quality and documentation without affecting behavior. Based on learnings, improvements to generated binding classes should come from code generator enhancements rather than manual edits—this appears to follow that pattern correctly.
Also applies to: 39-41, 55-65, 304-304, 320-320, 355-355, 371-371
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java (1)
270-428: LGTM! Previous review feedback has been addressed.The comprehensive test suite now includes all the edge-case validation tests that were suggested in the previous review:
- Exception handling for
maxWidth <= 0(lines 388-400)- Exception handling for
indent.length() >= maxWidth(lines 402-414)- Very narrow width scenarios (lines 416-427)
The tests are well-structured, use descriptive names with
@DisplayNameannotations (satisfying the Javadoc exemption for test methods), and properly validate both normal wrapping behavior and error conditions. The test coverage aligns well with the validation logic in thewrapText()implementation.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/MetapathContext.java (1)
7-250: Generated file with formatting-only changes.This file is a generated binding class (as indicated by the header comment at lines 5-6). The changes are purely formatting adjustments: Javadoc reflow, annotation spacing, and minor whitespace improvements. No functional changes to the API or behavior.
While the formatting improvements are benign, per project conventions for generated code, future style and documentation enhancements should ideally be made at the code generator level (metaschema-maven-plugin) to ensure consistency across all generated binding classes.
Based on learnings, generated binding classes should have improvements tracked at the generator level rather than through manual edits.
...in/java/gov/nist/secauto/metaschema/databind/model/metaschema/binding/InlineDefineField.java
Show resolved
Hide resolved
david-waltermire
force-pushed
the
feature/603-jansi-to-jline
branch
from
c34738d to
e273376
Compare
…ings Resolves metaschema-framework#603 by migrating from org.fusesource.jansi:jansi 2.4.2 to org.jline:jansi-core 3.30.6. JLine 3.25+ includes the Jansi codebase directly, providing the same ANSI console functionality under the org.jline.jansi package. Key Changes: - Replace Jansi dependency with JLine jansi-core (minimal ANSI module) - Remove AnsiConsole.systemInstall() - this call triggers native library loading which causes warnings on JDK 22+ (JEP 454) and JDK 24+ (JEP 472) - Use System.out directly instead of AnsiConsole.out() - modern terminals (Windows 10+, Linux, macOS) support ANSI escape codes natively - Replace AnsiPrintStream.getTerminalWidth() with COLUMNS environment variable lookup (falls back to 80 columns) - Use Ansi.setEnabled(false) for --no-color mode instead of AnsiConsole.JANSI_MODE_STRIP property - Update imports from org.fusesource.jansi to org.jline.jansi - Add input validation to wrapText() to prevent infinite loops - Add Javadoc to handleNoColor() method - Hide "Since" column in help output for cleaner display - Document --no-color flag in README files Tests: - CLIProcessorTest: Version output tests (7), No-color mode tests (3) - CallingContextTest: Text wrapping tests (17 including edge cases) - LoggingValidationHandlerTest: Factory method and configuration tests (11) - All 128 CLI tests pass This approach eliminates native access entirely, avoiding warnings on all Java versions without requiring --enable-native-access flags.
david-waltermire
force-pushed
the
feature/603-jansi-to-jline
branch
from
e273376 to
bce13da
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java (2)
81-149: Consider extracting shared version output for optional refactoring.All seven tests in this class follow an identical pattern: call
process("--version"), capture output, and assert one substring. While the current approach ensures test isolation and clarity, you could optionally reduce duplication by capturing the version output once in a@BeforeEachmethod or using a@RepeatedTestwith parameters.However, the current implementation is clear and maintains good test isolation, so this refactoring is entirely optional.
151-188: Consider verifying ANSI codes are actually absent in --no-color mode.The tests verify that
--no-coloris accepted and produces output, but they don't verify that ANSI escape codes are actually removed from the output. Consider adding assertions that check for the absence of ANSI escape sequences (e.g.,\u001B[) in the captured output when--no-coloris used, and their presence when it's not.This would provide stronger validation that the
Ansi.setEnabled(false)call inCLIProcessor.handleNoColor()is working as intended.Example assertion to verify ANSI codes are absent
// After capturing output with --no-color String output = outputCapture.toString(StandardCharsets.UTF_8); assertFalse(output.contains("\u001B["), "Output should not contain ANSI escape codes when --no-color is used");
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.lycheeignoreREADME.mdcli-processor/pom.xmlcli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javacli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.javametaschema-cli/src/main/distro/README.txtmetaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.javametaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.javapom.xmlschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.java
✅ Files skipped from review due to trivial changes (1)
- metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.java
🚧 Files skipped from review as they are similar to previous changes (6)
- pom.xml
- cli-processor/pom.xml
- cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java
- cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java
- metaschema-cli/src/main/distro/README.txt
- README.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.javametaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javametaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java
🧠 Learnings (7)
📚 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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.javametaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.javametaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.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:
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.javametaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java
📚 Learning: 2024-11-15T20:00:59.203Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java:15-15
Timestamp: 2024-11-15T20:00:59.203Z
Learning: The `DefaultExtraArgument` class in `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java` is currently a stub for future exploration.
Applied to files:
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.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:
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.javametaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.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:
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.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: Use SpotBugs, PMD, Checkstyle, and Jacoco for static analysis. Project enforces Checkstyle code style, PMD source analysis (fails on priority 2+ violations), SpotBugs bug detection (with spotbugs-exclude.xml exclusions), and Jacoco code coverage (target: 60% coverage)
Applied to files:
.lycheeignore
🧬 Code graph analysis (3)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java (1)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java (7)
Nested(64-102)Nested(104-134)Nested(136-190)Nested(192-233)Nested(235-268)Nested(270-428)DisplayName(37-429)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java (1)
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.java (1)
LoggingValidationHandler(37-248)
⏰ 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 (11)
.lycheeignore (1)
2-2: LGTM!The addition of the jline3 repository pattern aligns with the PR's migration from Jansi to JLine and follows the established convention for ignoring broken/timeout-prone GitHub repository links used elsewhere in the file (e.g., lines 35, 38, 39). The regex pattern is correct and appropriately positioned under the "broken plugin and dependency references" comment section.
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java (3)
1-25: LGTM! Well-documented test class with clear purpose.The class-level Javadoc effectively explains the test scope and clarifies the relationship between these unit tests and integration tests for ANSI behavior. The package naming and imports follow project conventions.
27-101: LGTM! Comprehensive factory method test coverage.The test suite thoroughly verifies:
- Singleton behavior and identity across calls
- Correct exception logging configuration for both
instance(boolean)variants- Custom formatter integration via
withPathFormatter- Explicit configuration via
withSettingsThe use of
assertSamefor singleton identity checks andassertThatfor boolean properties is appropriate.
103-122: LGTM! Configuration default behavior properly verified.The tests confirm that both
instance()andwithPathFormatterdefault to not logging exceptions, which is important for ensuring consistent behavior across factory methods.schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.java (1)
45-70: Good approach for addressing Xerces thread-safety issues.The design to import elements into a fresh Document for complete DOM isolation, combined with the
NodeListSnapshotfor safe iteration, effectively addresses the thread-safety issues with Xerces' internalfNodeListCache. The implementation is clean and well-documented.Also applies to: 84-101, 234-240, 249-271
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (6)
8-8: LGTM! Import updated for JLine migration.The static import correctly reflects the migration from
org.fusesource.jansitoorg.jline.jansi, aligning with the PR objective to resolve Java 21+ native access warnings.
400-405: LGTM! Method signature updated with proper documentation.The addition of the
terminalWidthparameter enables ANSI-aware text wrapping for command descriptions. Javadoc correctly documents the new parameter.
424-440: LGTM! Terminal width calculation with proper wrapping.The logic correctly:
- Computes column widths based on terminal width
- Ensures minimum description width of 20 characters (line 431)
- Wraps command descriptions with appropriate indentation
The edge case where
continuationIndent.length() >= descWidth(e.g., narrow terminal + long command names) is handled by the input validation inwrapText()(lines 593-596), which throwsIllegalArgumentExceptionwith a clear error message rather than causing an infinite loop.
550-574: LGTM! Clean terminal width detection without native access.The implementation correctly:
- Reads the
COLUMNSenvironment variable (set by most shells)- Validates the parsed width is positive
- Falls back to 80 columns on error or missing variable
- Avoids native terminal detection that triggers Java 21+ warnings
This aligns with the PR objective to eliminate native-access warnings.
597-634: LGTM! Text wrapping logic is correct and well-defended.The implementation correctly handles:
- Early return for text that fits within maxWidth (lines 597-599)
- Continuation line indentation (lines 607-610)
- Force-break when no space is found (lines 622-625): continues from
lineEndwithout skipping characters- Word-boundary wrapping when space is found (lines 626-629): skips the space
The input validation (lines 590-596) prevents the infinite loop scenario confirmed in past reviews. The logic properly addresses the off-by-one bug identified in previous review cycles.
644-676: LGTM! Help rendering updated for ANSI-aware wrapping.The changes correctly:
- Obtain terminal width via
getTerminalWidth()to avoid native detection (lines 644-646)- Remove the empty "Since" column from options tables via
setShowSince(false)(line 658)- Print the footer directly (line 675) to bypass
TextHelpAppendable's text wrapping, which doesn't account for ANSI escape sequence lengthsThese changes align with the PR objectives to improve help output with ANSI-aware text wrapping while avoiding Java 21+ native-access warnings.
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java
Show resolved
Hide resolved
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.java (1)
45-70: Missing@throwstag in constructor Javadoc.The constructor calls
importElements(line 69), which throwsIllegalStateExceptionwhen DocumentBuilder creation fails (line 101). Per coding guidelines, all Javadoc must include@throwstags for any exceptions thrown—this is a BLOCKING requirement.🔎 Proposed fix
* @param dependencies * the list of datatype names that this datatype depends on + * @throws IllegalStateException + * if the DocumentBuilder cannot be created for DOM isolation */ @SuppressFBWarnings(value = "CT_CONSTRUCTOR_THROW",As per coding guidelines, all Javadoc must include @throws tags in the correct order (BLOCKING).
🧹 Nitpick comments (1)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (1)
400-405: Approved: Text wrapping properly integrated into help footer.The terminal width is correctly passed through and used to compute column widths for command descriptions. The
wrapTextintegration ensures descriptions are properly formatted with continuation indentation.Optional enhancement: In extreme edge cases (very narrow terminal + very long command names),
continuationIndent.length()can equal or exceeddescWidth, causingwrapTextto throwIllegalArgumentExceptionand--helpto fail. For example, with a 40-character terminal and a 30-character command name:prefixWidth = 34,descWidth = max(40 - 34, 20) = 20, triggering the exception since34 >= 20.The current fail-fast behavior is valid, but if you want graceful degradation in narrow terminals, you could cap
prefixWidthbefore creatingcontinuationIndent:int prefixWidth = 3 + commandColWidth + 1; int descWidth = Math.max(terminalWidth - prefixWidth, 20); // Optional: ensure indent fits within description width if (prefixWidth >= descWidth) { prefixWidth = Math.max(descWidth / 2, descWidth - 10); } String continuationIndent = " ".repeat(prefixWidth);This is entirely optional—the validation in
wrapTextalready prevents undefined behavior.Also applies to: 424-432, 435-440
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.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:
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.java
🧠 Learnings (6)
📚 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:
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java
📚 Learning: 2024-11-15T20:00:59.203Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java:15-15
Timestamp: 2024-11-15T20:00:59.203Z
Learning: The `DefaultExtraArgument` class in `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java` is currently a stub for future exploration.
Applied to files:
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.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:
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.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:
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.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:
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.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:
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.java
🧬 Code graph analysis (1)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
⏰ 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 (8)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.java (4)
72-103: LGTM—previous Javadoc concern addressed.The
@throws IllegalStateExceptiontag (lines 82-83) is now present and correctly documents the exception behavior. The implementation correctly isolates each DOM tree to prevent thread-safety issues with Xerces' internal caches.
187-189: LGTM—thread-safety improvement.Replacing direct NodeList iteration with a snapshot correctly addresses the thread-safety issue with Xerces' internal
fNodeListCachewhen schema generation runs in parallel.
222-242: LGTM—effective snapshot mechanism.The implementation correctly captures NodeList contents at a point in time, preventing issues with live NodeList updates during parallel iteration. The empty-list optimization is a nice touch.
244-273: LGTM—clean immutable list implementation.The inner class correctly implements an immutable, random-access snapshot backed by an array. Extending
AbstractListwithout overriding mutation methods ensures any modification attempts throwUnsupportedOperationExceptionas expected.cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java (4)
8-8: LGTM: Import successfully migrated to JLine.The import path correctly reflects the migration from
org.fusesource.jansitoorg.jline.jansias outlined in the PR objectives.
550-574: Excellent approach to avoid native access warnings.The environment-variable-based terminal width detection (using
COLUMNS) successfully eliminates the need for native terminal access that triggered Java 21+ warnings. The fallback to 80 columns is reasonable, and the error handling for invalid values is appropriate.The Javadoc is complete and follows the project style guide.
576-637: LGTM: Text wrapping implementation is solid.The method includes comprehensive input validation that prevents the infinite loop scenarios flagged in previous reviews:
- ✅ Validates
maxWidth > 0to prevent non-positive width- ✅ Validates
indent.length() < maxWidthto ensure effective width is positive on continuation lines- ✅ Fixed off-by-one bug in force-break case (line 628:
lineStart = lineEndinstead oflineEnd + 1)The Javadoc is complete with all required tags (
@param,@return,@throws) in the correct order, satisfying the coding guidelines.
647-678: LGTM: Help output rendering correctly updated.The changes successfully integrate the terminal width detection and text wrapping:
- ✅ Uses environment-based width detection (line 649) to avoid native access
- ✅ Removes empty "Since" column (line 661) as noted in PR summary
- ✅ Directly prints footer (line 678) to preserve ANSI codes and use custom wrapping
The comments clearly explain the rationale for bypassing the formatter's text wrapping, which is important for maintainability.
- Add explicit isNotSameAs() assertion in LoggingValidationHandlerTest to verify instance(true) and instance(false) return different singletons - Refactor CLIProcessorTest version output tests to use parameterized tests reducing duplication while maintaining clear test output
|
@coderabbitai Addressed all nitpick suggestions in commit 3ba10a1:
|
|
Tip For best results, initiate chat on the files or code changes. @david-waltermire Thank you for addressing those nitpick suggestions! The changes sound great:
These updates enhance both the test coverage and maintainability. Great work on the refactoring! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java (2)
27-102: Consider testing thatinstance()andinstance(false)return the same singleton.The factory method tests are comprehensive and well-structured. However, there's a gap in singleton coverage: while lines 38-44 verify that
instance()returns the same singleton on repeated calls, no test verifies thatinstance()andinstance(false)return the same singleton object.Given that the implementation shows
instance()delegates toinstance(false), adding a test like this would strengthen the contract verification:@Test @DisplayName("instance() and instance(false) return the same singleton") void testInstanceAndInstanceFalseReturnSameSingleton() { LoggingValidationHandler defaultInstance = LoggingValidationHandler.instance(); LoggingValidationHandler noExceptionInstance = LoggingValidationHandler.instance(false); assertSame(defaultInstance, noExceptionInstance, "instance() and instance(false) should return the same singleton"); }
115-122: Consider removing redundant test.This test duplicates the verification already performed in lines 73-81 of the
FactoryMethodTestsclass. Both tests callwithPathFormatter(IPathFormatter.METAPATH_PATH_FORMATER)and verify thatisLogExceptions()returnsfalse.While some redundancy can clarify intent, this particular duplication doesn't add new assertions or test different scenarios. Consider removing this test to reduce maintenance overhead.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.javametaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java
🧰 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:
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.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:
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java
🧠 Learnings (5)
📚 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:
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.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:
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.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:
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.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:
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.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:
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java
🧬 Code graph analysis (1)
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java (1)
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandler.java (1)
LoggingValidationHandler(37-248)
⏰ 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 (2)
metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/util/LoggingValidationHandlerTest.java (2)
18-25: LGTM! Well-documented test class.The class-level Javadoc clearly explains the purpose and provides helpful context about the Jansi to JLine migration. The reference to integration tests in CLITest appropriately delineates the scope.
1-124: Well-structured test suite with clear intent.This test file demonstrates good testing practices:
- Organized into logical nested test groups using JUnit 5
@Nested- Descriptive test names with
@DisplayNameannotations (no method Javadoc needed per coding guidelines)- Good use of
isNotSameAs()on line 68 to verify distinct singleton instances- Tests the public factory method API thoroughly
- Safe for parallel execution (no shared mutable state)
The test suite effectively validates the
LoggingValidationHandlerAPI behavior during the Jansi to JLine migration.
- Add test verifying instance() and instance(false) return the same singleton - Add @throws IllegalStateException to DomDatatypeContent constructor Javadoc
|
@coderabbitai Addressed additional nitpick suggestions in commits 3ba10a1 and df6f0f7:
Build verified:
|
|
Tip For best results, initiate chat on the files or code changes. @david-waltermire Excellent work addressing all the nitpick suggestions! Your changes look great:
The clean build verification with all checks passing (11 tests, 0 violations across all tools, and coverage checks) confirms everything is in excellent shape. Great attention to detail throughout this PR! 🎉 |
david-waltermire
merged commit 24ce35b
into
metaschema-framework:develop
1 participant
Summary
Resolves #603 by migrating from
org.fusesource.jansi:jansi2.4.2 toorg.jline:jansi-core3.30.6 and eliminating native terminal detection entirely.JLine 3.25+ includes the Jansi codebase directly under the
org.jline.jansipackage. However, simply switching dependencies still triggered native access warnings becauseAnsiConsole.systemInstall()loads native libraries for terminal detection. This PR eliminates native access by relying on modern terminals' built-in ANSI support instead.Key Changes
Dependency Migration
org.fusesource.jansi:jansiwithorg.jline:jansi-core3.30.6 (minimal module for ANSI support, ~245KB vs ~1.8MB for full JLine)org.fusesource.jansitoorg.jline.jansiNative Access Elimination
AnsiConsole.systemInstall()- This call triggered native library loading which causes warnings on JDK 22+ (JEP 454) and errors on JDK 24+ (JEP 472)System.outdirectly - Modern terminals (Windows 10+, Linux, macOS) support ANSI escape codes natively without detectionAnsiPrintStream.getTerminalWidth()- Now usesCOLUMNSenvironment variable with 80-column fallbackAnsi.setEnabled(false)for--no-colormode instead of AnsiConsole propertiesHelp Output Improvements
Input Validation
Flaky Test Fix
fNodeListCachenull pointer errors during parallel test execution by importing DOM elements into a fresh Document for each instance, ensuring complete DOM tree isolationUser Impact
Breaking Changes
None expected. The CLI behavior should be identical.
Behavioral Differences
COLUMNSenv var or 80 defaultWARNING: A restricted method in java.lang.System has been calledTerminal Width
The CLI now reads terminal width from the
COLUMNSenvironment variable, which is set automatically by most shells (bash, zsh, fish, PowerShell). If not set, it defaults to 80 columns. This may affect help text wrapping in edge cases where:COLUMNSCOLUMNSdynamically, others don't)ANSI Color Support
The CLI now assumes the terminal supports ANSI escape codes. This is true for:
Legacy Console Support
Users on legacy consoles that do not support ANSI escape codes (e.g., older Windows cmd.exe, certain CI/CD environments, or redirected output) may see raw escape sequences in the output. To address this, use the
--no-colorflag to disable ANSI formatting:Test Plan
mvn -pl cli-processor,metaschema-cli test- All CLI tests passmvn clean install -PCI -Prelease- Full build passes (5078 tests, 0 failures, 0 errors)--no-colorBuild Verification
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.