Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions .claude/rules/development-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,27 @@ PR to develop

---

## Debugging Workflow
## Debugging Workflow (MANDATORY)

**ALL debugging MUST use `superpowers:systematic-debugging` skill. No exceptions.**

When debugging issues (during development or in production):

### Step 0: Invoke the Debugging Skill (REQUIRED FIRST STEP)

**BEFORE ANY investigation or fix attempts:**
1. Use `Skill` tool to invoke `superpowers:systematic-debugging`
2. Follow the four phases exactly as specified
3. Create TodoWrite todos for each phase

**Red Flags (You're Skipping the Skill):**
- "Let me just check this quickly"
- "This is a simple bug"
- "I can see the problem, let me fix it"
- "One quick fix to try first"

**Why:** Random fixes waste time and create new bugs. The skill ensures systematic root cause identification.

### Step 1: Identify Root Cause (REQUIRED)
**Do NOT attempt fixes until root cause is identified.**

Expand All @@ -244,7 +261,7 @@ Use these skills:
- Where in the code does the bug originate?
- Why does this code path produce the wrong result?

**Post Bug Reproduction comment to Jira** (see Jira Comments section below)
**Post Bug Reproduction comment to Jira** (if applicable)
Comment thread
david-waltermire marked this conversation as resolved.

### Step 2: Verify Test Coverage
Once root cause is confirmed:
Expand Down Expand Up @@ -287,7 +304,7 @@ Implement Fix (target the root cause)
verification-before-completion
PR to staging
PR to develop
```

---
Expand Down
31 changes: 31 additions & 0 deletions .claude/rules/metaschema-authoring.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,37 @@ When Metaschema modules are used for code generation but create circular depende

See `metaschema-testing/` for an example of this pattern.

### Generated Binding Class Locations

The following packages contain binding classes derived from Metaschema modules:

| Package | Source Metaschema | Bootstrap POM |
|---------|------------------|---------------|
| `databind/src/main/java/gov/nist/secauto/metaschema/databind/config/binding/` | `databind/src/main/metaschema/metaschema-bindings.yaml` | `databind/pom-bootstrap.xml` |
| `metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/` | `metaschema-testing/src/main/metaschema/unit-tests.yaml` | `metaschema-testing/pom-bootstrap.xml` |

### CRITICAL: Never Manually Edit Generated Binding Classes

**All changes to generated binding classes MUST be driven through the source Metaschema module.**

When you need to modify a generated binding class:

1. **Identify the source Metaschema** - Find the `.yaml` or `.xml` module that generates the class
2. **Modify the Metaschema module** - Update the `.yaml` or `.xml` module definition
3. **Build the project first** - Run `mvn install` to ensure the code generator is up to date
4. **Regenerate the binding classes** - Run the bootstrap POM: `mvn -f {module}/pom-bootstrap.xml generate-sources`
5. **Verify the changes** - Check that the regenerated classes contain the expected changes

**Why this matters:**
- Manual edits will be lost when classes are regenerated
- Manual edits may diverge from the Metaschema schema definition
- The Metaschema module is the authoritative source for the data model

**Red flags that you're about to make a mistake:**
- Opening a file in `.../config/binding/` or `.../testsuite/` for editing
- Adding fields, methods, or annotations directly to these classes
- Copying code patterns from these files to create new bindings manually

## Code Generation

After modifying Metaschema modules that generate Java bindings:
Expand Down
15 changes: 15 additions & 0 deletions .claude/rules/unit-testing.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
# Unit Testing Standards

## 100% Test Pass Rate (BLOCKING)

**All unit tests MUST pass before pushing code or creating PRs.**

- Run `mvn test` or `mvn -pl {module} test` to verify
- A single failing test blocks the PR
- Flaky tests must be fixed or marked as `@Disabled` with explanation
- Network-dependent tests should have appropriate timeouts and retry logic

**If CI fails due to test timeout or flakiness:**
1. Investigate the root cause
2. If it's a pre-existing issue on develop, note this in the PR
3. Re-run CI to verify it's not caused by your changes
4. Open an issue to track the flaky test if not already tracked

## Core Principles

### What NOT to Test
Expand Down
61 changes: 61 additions & 0 deletions .claude/skills/metaschema-module-authoring.md
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,67 @@ model:
</model>
```

### Code Generation: Nested vs Separate Classes

The choice between inline definitions and global definitions with references affects how Java classes are generated:

| Approach | Generated Structure | Use When |
|----------|---------------------|----------|
| Inline definitions | Nested inner classes | Single-use, tightly coupled structures |
| Global definitions + refs | Separate top-level classes | Reusable, shared across multiple parents |

#### Example: Inline produces nested classes

```yaml
# This YAML structure with inline definitions:
- object-type: assembly
name: parent
model:
instances:
- object-type: assembly
name: child
formal-name: Child
model:
instances:
- object-type: field
name: value
as-type: string
```

Generates:
```java
public class Parent {
public static class Child {
// nested inside Parent
}
}
```

#### Example: References produce separate classes

```yaml
# This YAML structure with global definitions and refs:
definitions:
- object-type: assembly
name: parent
model:
instances:
- object-type: assembly-ref
ref: child

- object-type: assembly
name: child
formal-name: Child
```

Generates:
```java
public class Parent { ... }
public class Child { ... } // separate file
```

**Key insight**: Use inline definitions when you want tightly coupled, nested class hierarchies (like binding configurations with deeply nested elements). Use global definitions with refs when the component is reused elsewhere.

## Cardinality and Grouping

### Cardinality Attributes
Expand Down
29 changes: 28 additions & 1 deletion .claude/skills/unit-test-writing.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,40 @@ description: Use when writing or expanding unit tests - provides edge case check
| Empty/Null | Empty string, null reference, empty collection | ☐ |
| Boundaries | Zero, negative, max int, min int, boundary values | ☐ |
| Invalid | Wrong type, malformed input, out of range | ☐ |
| Missing | Required field absent, partial data | ☐ |
| Missing | Required field absent, partial data, missing nested element | ☐ |
| Duplicates | Repeated values, duplicate keys | ☐ |
| Ordering | First, last, middle, unsorted | ☐ |
| Concurrency | Race conditions, thread safety (if applicable) | ☐ |
| State | Uninitialized, already processed, closed | ☐ |
| Combinations | Multiple flags, conflicting options | ☐ |

## Configuration/Parsing Edge Cases

When testing XML/JSON/YAML configuration loading, always test:

| Scenario | Test Case | Expected |
|----------|-----------|----------|
| Missing optional child | Parent element exists, optional child absent | Null or default value |
| Empty element | `<element/>` or `<element></element>` | Empty string or null |
| Unknown module | Query from module not in config | Null, not exception |
| Unknown definition | Query definition not in config | Null, not exception |
| Partial config | Some fields present, others missing | Only specified fields set |
| Multiple types | Same pattern for different parent types (assembly vs field) | Both work correctly |

**Example edge case test structure:**

```java
@Test
void testMissingOptionalElement() throws IOException {
// Load config with element that has no optional child
config.load(new File("edge-cases.xml"));

// Query should return null, not throw
IPropertyBindingConfiguration result = config.getPropertyBindingConfiguration(definition, "missing-child");
assertNull(result, "Missing optional element should return null");
}
```
Comment thread
coderabbitai[bot] marked this conversation as resolved.

## Coverage Expansion Workflow

When touching existing code:
Expand Down
103 changes: 88 additions & 15 deletions PRDs/20251224-codegen-quality/implementation-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,16 @@ This PR fixes the code generator and regenerates metaschema-testing binding clas

---

## PR 2: Collection Class Override Support
## PR 2: Collection Class Override Support

| Attribute | Value |
|-----------|-------|
| **Files Changed** | ~10 |
| **Files Changed** | ~15 |
| **Risk Level** | Low |
| **Dependencies** | PR 1 |
| **Target Branch** | develop |
| **Status** | Pending |
| **Pull Request** | |
| **Status** | Completed |
| **Pull Request** | [#584](https://github.com/metaschema-framework/metaschema-java/pull/584) |

This PR extends the binding configuration to support overriding default collection implementation classes.

Expand Down Expand Up @@ -155,14 +155,29 @@ This PR extends the binding configuration to support overriding default collecti

### Acceptance Criteria

- [ ] Binding configuration schema supports `<collection-class>` element
- [ ] `DefaultBindingConfiguration` parses collection-class from XML
- [ ] Override is applied in `getCollectionImplementationClass()`
- [ ] Type compatibility validation (List vs Map)
- [ ] Unit tests for collection-class parsing and application
- [ ] `mvn checkstyle:check` passes
- [ ] All tests pass: `mvn test`
- [ ] Build succeeds: `mvn clean install -PCI -Prelease`
- [x] Binding configuration schema supports `<collection-class>` element
- [x] `DefaultBindingConfiguration` parses collection-class from XML
- [x] Override is applied in `getCollectionImplementationClass()` (infrastructure ready)
- [ ] Type compatibility validation (List vs Map) - deferred to PR 4
- [x] Unit tests for collection-class parsing and application
- [x] `mvn checkstyle:check` passes
- [x] All tests pass: `mvn test`
- [x] Build succeeds: `mvn clean install -PCI -Prelease`

### Files Actually Modified

| File | Changes |
|------|---------|
| `databind/src/main/metaschema/metaschema-bindings.yaml` | Added `collection-class` field and fixed `object-type` keywords |
| `databind/src/main/java/.../config/binding/MetaschemaBindings.java` | Regenerated with `getCollectionClass()` method |
| `databind/src/main/java/.../config/binding/MetaschemaBindingsModule.java` | Regenerated module class |
| `databind/src/main/java/.../config/binding/package-info.java` | Regenerated package info |
| `databind/src/main/java/.../codegen/config/DefaultBindingConfiguration.java` | Added property binding parsing |
| `databind/src/main/java/.../codegen/config/IPropertyBindingConfiguration.java` | New interface for property bindings |
| `databind/src/main/java/.../codegen/config/IMutablePropertyBindingConfiguration.java` | New mutable interface |
| `databind/src/main/java/.../codegen/config/DefaultPropertyBindingConfiguration.java` | New implementation |
| `databind/pom-bootstrap.xml` | Simplified bootstrap POM for direct generation |
| `databind/src/test/resources/metaschema/binding-config-with-collection-class.xml` | Test configuration |

---

Expand Down Expand Up @@ -237,16 +252,74 @@ This PR creates the databind bootstrap infrastructure and regenerates the databi

---

## PR 4: Parser Required Field Validation

| Attribute | Value |
|-----------|-------|
| **Files Changed** | ~10 |
| **Risk Level** | Medium |
| **Dependencies** | PR 1 |
| **Target Branch** | develop |
| **Status** | Pending |
| **Pull Request** | |

This PR adds validation during parsing to emit meaningful errors when required fields are missing, and includes type compatibility validation for collection class overrides.

### Motivation

Currently, when a required field/flag is missing from input data, the generated binding classes have `@NonNull` annotations on getters but the parser doesn't actively validate and provide meaningful errors. This can result in confusing null pointer exceptions later rather than clear parse-time errors.

### Files to Modify

| File | Changes |
|------|---------|
| `databind/src/main/java/.../io/xml/DefaultXmlDeserializer.java` | Add required field validation |
| `databind/src/main/java/.../io/json/DefaultJsonDeserializer.java` | Add required field validation |
| `databind/src/main/java/.../codegen/typeinfo/AbstractModelInstanceTypeInfo.java` | Validate collection class type compatibility |

### Implementation Approach

1. **Required field validation at parse time**
- After parsing an object, check if all required fields (those with `@NonNull` getters) have values
- Emit meaningful error message with field name and source location
- Must be efficient - check once after parsing, not on every field access

2. **Collection class type compatibility**
- When collection-class override is specified, validate at code generation time:
- For List fields: class must implement `java.util.List`
- For Map fields: class must implement `java.util.Map`
- Emit clear error if incompatible type specified

3. **Performance considerations**
- Required field tracking should use bitsets or similar compact representation
- Validation should happen once per object, not per field
- No reflection at runtime - track requirements at class binding time

### Acceptance Criteria

- [ ] Parser validates required fields are present during deserialization
- [ ] Missing required field produces clear error with field name and location
- [ ] Collection class override validates type compatibility (List/Map)
- [ ] Validation is efficient (no per-field overhead)
- [ ] Unit tests for required field validation
- [ ] Unit tests for collection class type validation
- [ ] `mvn checkstyle:check` passes
- [ ] All tests pass: `mvn test`
- [ ] Build succeeds: `mvn clean install -PCI -Prelease`

---

## PR Summary Table

| PR | Description | Files | Risk | Dependencies | Status |
|----|-------------|-------|------|--------------|--------|
| 1 | Code generator improvements + metaschema-testing regeneration | 20 | Low | None | ✅ Completed ([#577](https://github.com/metaschema-framework/metaschema-java/pull/577)) |
| 2 | Collection class override support | ~10 | Low | PR 1 | Pending |
| 2 | Collection class override support | ~15 | Low | PR 1 | ✅ Completed |
| 3 | Databind bootstrap setup + regeneration | ~35 | Medium | PR 1, PR 2 | Pending |
| 4 | Parser required field validation | ~10 | Medium | PR 1 | Pending |

**Total Estimated PRs**: 3
**Total Estimated Files**: ~65
**Total Estimated PRs**: 4
**Total Estimated Files**: ~80

---

Expand Down
Loading
Loading