fix(codegen): isRequired() returns false for choice block properties

.claude/rules/development-workflow.md

@@ -50,6 +50,16 @@ If you think any of these, STOP and check for skills:
 - `superpowers:requesting-code-review` - Review implementation against plan
 - `superpowers:testing-anti-patterns` - Avoid common testing mistakes
 
+### Project Conventions Override Skill Defaults
+
+Managed skills (superpowers plugin) provide general templates that may specify default file paths or workflows. **Project-specific conventions in `.claude/rules/` always take precedence.**
+
+Examples:
+- Brainstorming skill outputs to `docs/plans/` by default → Use `PRDs/[date]-[name]/` per `prd-conventions.md`
+- Skills may have their own directory structures → Follow project conventions instead
+
+When in doubt, check `.claude/rules/` for project-specific guidance before following skill defaults.
+
 ### Instructions ≠ Permission to Skip Workflows
 User instructions describe WHAT to do, not HOW. "Add X" or "Fix Y" does NOT mean skip brainstorming, TDD, or verification workflows.
 

PRDs/20251231-choice-required-fix/PRD.md

@@ -0,0 +1,73 @@
+# PRD: Fix isRequired() for Choice Block Properties
+
+## Problem Statement
+
+The `isRequired()` method in the code generator type info classes returns `true` for fields with `min-occurs="1"`, but doesn't account for fields that are inside `<choice>` blocks.
+
+### Example
+
+In `metaschema-module-metaschema.xml`, the `root-name` field is defined as:
+
+```xml
+<choice>
+    <define-field name="root-name" as-type="token" min-occurs="1">
+        <formal-name>Root Name</formal-name>
+        ...
+    </define-field>
+</choice>
+```
+
+The field has `min-occurs="1"`, making `isRequired()` return `true`. However, since it's inside a `<choice>` block, it's only required when that choice branch is taken - not always.
+
+### Impact
+
+When the code generator generates getters with `@NonNull` annotations (based on `isRequired()` returning `true`), fields inside choice blocks that aren't populated can cause issues:
+- Static analysis tools expect non-null values
+- Runtime `ObjectUtils.requireNonNull()` calls would throw `NullPointerException`
+
+## Goals
+
+1. Fix `isRequired()` to return `false` for properties inside choice blocks
+2. Ensure all generated getter/setters have proper `@Nullable`/`@NonNull` annotations based on:
+   - Required properties (minOccurs >= 1, not in choice) → `@NonNull`
+   - Optional properties (minOccurs = 0 or in choice) → `@Nullable`
+   - Collection properties → `@NonNull` (lazy initialized)
+3. Ensure Javadocs on generated getters/setters document nullability behavior
+4. Document the `isRequired()` behavior in interface Javadoc
+
+## Non-Goals
+
+- Runtime validation of choice constraints (handled by validation framework)
+- Changes to flag handling (flags cannot be inside choices in Metaschema)
+
+## Solution
+
+### Approach
+
+Properties inside Metaschema choice blocks are treated as optional for null-safety purposes, regardless of their `min-occurs` value. The requirement is conditional on the choice branch being taken.
+
+### Technical Design
+
+The infrastructure for tracking choice membership already exists:
+- `AbstractNamedModelInstanceTypeInfo` has a `choiceId` field
+- `setChoiceId()` is called when processing `IChoiceInstance` in `AssemblyDefinitionTypeInfoImpl`
+- The `@BoundChoice` annotation is already generated for choice properties
+
+The fix: Update `isRequired()` to check if `getChoiceId() != null` and return `false` in that case.
+
+### Impact on Generated Code
+
+For properties inside choice blocks:
+- Getter return annotation: `@NonNull` → `@Nullable`
+- Setter parameter annotation: `@NonNull` → `@Nullable`
+
+## Success Metrics
+
+1. All existing tests pass
+2. New unit tests verify `isRequired()` behavior for choice properties
+3. Regenerated bootstrap binding classes have correct `@Nullable` annotations
+4. Build passes with `mvn clean install -PCI -Prelease`
+
+## Related
+
+- GitHub Issue: [#604](https://github.com/metaschema-framework/metaschema-java/issues/604)

PRDs/20251231-choice-required-fix/implementation-plan.md

@@ -0,0 +1,118 @@
+# Implementation Plan: Fix isRequired() for Choice Block Properties
+
+## Overview
+
+This PR fixes issue #604 where `isRequired()` incorrectly returns `true` for properties inside choice blocks.
+
+## PR Scope
+
+Single PR containing:
+- Fix to `isRequired()` logic
+- Javadoc updates
+- Unit tests
+- Regenerated bootstrap binding classes
+
+## Tasks
+
+### Task 1: Write Unit Tests (TDD - RED phase)
+
+Create tests that verify `isRequired()` behavior before implementing the fix.
+
+**File:** `databind/src/test/java/gov/nist/secauto/metaschema/databind/codegen/typeinfo/AbstractNamedModelInstanceTypeInfoTest.java`
+
+**Test cases:**
+- [x] Property outside choice with minOccurs=1, maxOccurs=1 → returns `true`
+- [x] Property outside choice with minOccurs=0 → returns `false`
+- [x] Property inside choice with minOccurs=1, maxOccurs=1 → returns `false`
+- [x] Property inside choice with minOccurs=0 → returns `false`
+- [x] Collection property (maxOccurs > 1) → returns `false`
+
+### Task 2: Implement Fix (TDD - GREEN phase)
+
+Update `isRequired()` to check for choice membership.
+
+**File:** `databind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/typeinfo/AbstractNamedModelInstanceTypeInfo.java`
+
+**Change:**
+```java
+@Override
+public boolean isRequired() {
+  // Properties inside choice blocks are never required because
+  // the requirement is conditional on the choice branch being taken
+  if (getChoiceId() != null) {
+    return false;
+  }
+
+  INSTANCE instance = getInstance();
+  return instance.getMinOccurs() >= 1 && instance.getMaxOccurs() == 1;
+}
+```
+
+### Task 3: Update Javadoc
+
+Document the choice block behavior and enhance setter Javadoc.
+
+**Files:**
+- `databind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/typeinfo/IPropertyTypeInfo.java`
+- `databind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/typeinfo/INamedModelInstanceTypeInfo.java`
+
+**Changes:**
+1. Update `isRequired()` Javadoc to document choice block behavior
+2. Enhance `buildSetterJavadoc()` to document nullability for optional parameters:
+   - Required: "the value to set"
+   - Optional: "the value to set, or {@code null} to clear"
+
+### Task 4: Regenerate Bootstrap Binding Classes
+
+Regenerate all bootstrap binding classes to pick up annotation changes.
+
+```bash
+mvn install -DskipTests
+mvn -f databind/pom-bootstrap-model.xml generate-sources
+mvn -f databind/pom-bootstrap-config.xml generate-sources
+mvn -f metaschema-testing/pom-bootstrap.xml generate-sources
+```
+
+### Task 5: Verify Build
+
+Run full CI build to ensure all tests pass.
+
+```bash
+mvn clean install -PCI -Prelease
+```
+
+## Acceptance Criteria
+
+### Core Fix
+- [x] All unit tests pass
+- [x] New tests verify `isRequired()` returns `false` for choice properties
+- [x] `IPropertyTypeInfo.isRequired()` Javadoc documents the choice block behavior
+
+### Generated Code Quality
+- [x] All generated getters have correct null-safety annotations:
+  - `@NonNull` for required properties (minOccurs >= 1, not in choice)
+  - `@NonNull` for collection properties (lazy initialized)
+  - `@Nullable` for optional properties (minOccurs = 0 or in choice)
+- [x] All generated setters have matching null-safety annotations on parameters
+- [x] Generated getter/setter Javadocs document nullability behavior
+
+### Bootstrap Bindings
+- [x] Bootstrap binding classes regenerated with correct annotations
+- [x] Verify choice properties in regenerated classes have `@Nullable` annotations
+
+### Build Verification
+- [x] Build passes: `mvn clean install -PCI -Prelease`
+
+## Files Changed Summary
+
+| File | Change Type |
+|------|-------------|
+| `databind/.../typeinfo/AbstractNamedModelInstanceTypeInfo.java` | Modified |
+| `databind/.../typeinfo/IPropertyTypeInfo.java` | Modified |
+| `databind/.../typeinfo/INamedModelInstanceTypeInfo.java` | Modified |
+| `databind/.../typeinfo/INamedInstanceTypeInfo.java` | Modified |
+| `databind/.../typeinfo/AbstractNamedModelInstanceTypeInfoTest.java` | New |
+| `.claude/rules/development-workflow.md` | Modified |
+| `databind/.../model/metaschema/binding/*.java` | Regenerated |
+| `databind/.../config/binding/*.java` | Regenerated |
+| `metaschema-testing/.../testsuite/*.java` | Regenerated |

databind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/typeinfo/AbstractNamedModelInstanceTypeInfo.java

@@ -74,6 +74,12 @@ public void setChoiceId(@Nullable String choiceId) {
 
   @Override
   public boolean isRequired() {
+    // Properties inside choice blocks are never required because
+    // the requirement is conditional on the choice branch being taken
+    if (getChoiceId() != null) {
+      return false;
+    }
+
     INSTANCE instance = getInstance();
     // A model instance is required if minOccurs >= 1 AND it's a single item (not a
     // collection)

databind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/typeinfo/INamedInstanceTypeInfo.java

@@ -101,6 +101,12 @@ default void buildSetterJavadoc(@NonNull MethodSpec.Builder builder, @NonNull St
 
     builder.addJavadoc("\n");
     builder.addJavadoc("@param $L\n", paramName);
-    builder.addJavadoc("          the $L value to set\n", propertyName);
+    // Collections and required properties require non-null values;
+    // optional properties can be set to null to clear
+    if (isRequired() || isCollectionType()) {
+      builder.addJavadoc("          the $L value to set\n", propertyName);
+    } else {
+      builder.addJavadoc("          the $L value to set, or {@code null} to clear\n", propertyName);
+    }
   }
 }

databind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/typeinfo/INamedModelInstanceTypeInfo.java

@@ -124,6 +124,12 @@ default void buildSetterJavadoc(@NonNull MethodSpec.Builder builder, @NonNull St
 
     builder.addJavadoc("\n");
     builder.addJavadoc("@param $L\n", paramName);
-    builder.addJavadoc("          the $L value to set\n", propertyName);
+    // Collections and required properties require non-null values;
+    // optional properties can be set to null to clear
+    if (isRequired() || isCollectionType()) {
+      builder.addJavadoc("          the $L value to set\n", propertyName);
+    } else {
+      builder.addJavadoc("          the $L value to set, or {@code null} to clear\n", propertyName);
+    }
   }
 }

databind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/typeinfo/IPropertyTypeInfo.java

@@ -18,14 +18,21 @@
 
 public interface IPropertyTypeInfo extends ITypeInfo {
   /**
-   * Determines if this property is required to have a value.
+   * Determines if this property is unconditionally required to have a value.
    *
    * <p>
    * For flags, this checks
    * {@link gov.nist.secauto.metaschema.core.model.IFlagInstance#isRequired()}.
    * For model instances, this is based on minimum occurrence constraints.
    *
-   * @return {@code true} if a value is required, or {@code false} otherwise
+   * <p>
+   * <strong>Choice blocks:</strong> Properties inside Metaschema choice blocks
+   * always return {@code false}, regardless of their {@code min-occurs} value.
+   * The requirement is conditional on the choice branch being taken, so for
+   * null-safety purposes they are treated as optional.
+   *
+   * @return {@code true} if a value is unconditionally required, or {@code false}
+   *         otherwise
    */
   default boolean isRequired() {
     return false;