⚠️ Separate reduced vs full need representation for schema validation (#1652)

## Changes

- **Rename** `validate_option_fields` → `validate_fields` and
`validate_link_options` → `validate_links`
- **Add `reduce` parameter** to `get_ontology_warnings()`:
- `True` (type-specific `local`/`network` schemas): uses `reduce_need()`
— strips empty link lists and defaulted core fields. Required for
`unevaluatedProperties` (extra fields don't cause false failures) and
`required` (fields at their default are absent, so `required` enforces
explicit setting) to work correctly.
- `False` (global field/link constraint validation): uses
`NeedItem.iter_schema_items()` — returns a curated subset of fields
(core: `id`, `type`, `title`, `status`, `tags`; source: `docname`,
`is_import`, `is_external`; all extra fields; all links), filtering only
`None` values. This retains empty `[]` values for links and `tags`, so
constraints are evaluated against every need.

## Behavioral change

Previously, fields with default `[]` values (link fields like `links`,
and core fields like `tags`) were stripped before validation, silently
bypassing constraints like `minItems`, `contains`, and `minContains`.
Now with `reduce=False`, these fields are retained as `[]`, causing them
to **fail** those constraints.

This applies to both:
- **Link fields** (e.g. `links`, `implements`) — always default to `[]`
- **Core fields** with `[]` defaults (e.g. `tags`) — previously stripped
when matching the default

**Example**: A schema with `schema.minItems = 1` on `links` will now
warn for *every* need that doesn't set any links, not just those that
explicitly set links but provided too few.

## Decision needed

**Is this the correct strictness?** Two reasonable interpretations:

1. ✅ **"All needs must satisfy field/link constraints"** — the new
behavior. If you declare `minItems: 1` on a link type or `tags`, every
need must have at least one value. Users who want optional fields should
not set `minItems`.

2. ❌ **"Only needs that explicitly set a field must satisfy its
constraints"** — the old behavior. Unset/default fields are treated as
absent, not as empty arrays.

If (1) is intended, this should be noted in the changelog as a behavior
change. If (2) is preferred, the fix would be to keep `reduce_need=True`
for field/link validation, or to conditionally skip default-valued
fields when `reduce_need=False`.

### Known limitation of `reduce_need=True`

The reduction logic strips fields whose value matches the default — but
it cannot distinguish between "never set (defaulted)" and "explicitly
set to the default value." For example, if a user actively sets `:tags:`
to `[]` and the schema has `minItems: 1`, the field is stripped because
`[]` matches the default, and the violation is **silently ignored**.
This is a fundamental limitation of comparing values to defaults at
validation time, since the provenance of the value (explicit vs
implicit) is not tracked.

Author: chrisjsewell

sphinx_needs/need_item.py

@@ -669,6 +669,25 @@ def items(self) -> Iterable[tuple[str, Any]]:
             self._computed.items(),
         )
 
+    def iter_schema_items(self) -> Iterable[tuple[str, Any]]:
+        """Return the items of the need item that are relevant for schema validation."""
+        # TODO - this should be reworked to be more robust;
+        # e.g. probably all core fields should be included
+        return chain(
+            (
+                (k, v)
+                for k, v in self._core.items()
+                if k in ("id", "type", "title", "status", "tags")
+            ),
+            (
+                (k, v)
+                for k, v in self._source.dict_repr.items()
+                if k in ("docname", "is_import", "is_external")
+            ),
+            self._extras.items(),
+            self._links.items(),
+        )
+
     def iter_core_items(self) -> Iterable[tuple[str, Any]]:
         """Return the core items of the need item."""
         return chain(

sphinx_needs/schema/core.py

@@ -42,7 +42,7 @@
 """
 
 
-def validate_option_fields(
+def validate_fields(
     config: NeedsSphinxConfig,
     schema: NeedFieldsSchemaType,
     field_properties: Mapping[str, NeedFieldProperties],
@@ -56,6 +56,7 @@ def validate_option_fields(
             need,
             field_properties,
             validator,
+            reduce=False,
             fail_rule=MessageRuleEnum.field_fail,
             success_rule=MessageRuleEnum.field_success,
             schema_path=["fields", "schema"],
@@ -67,7 +68,7 @@ def validate_option_fields(
     return need_2_warnings
 
 
-def validate_link_options(
+def validate_links(
     config: NeedsSphinxConfig,
     schema: NeedFieldsSchemaType,
     field_properties: Mapping[str, NeedFieldProperties],
@@ -81,6 +82,7 @@ def validate_link_options(
             need,
             field_properties,
             validator,
+            reduce=False,
             fail_rule=MessageRuleEnum.extra_link_fail,
             success_rule=MessageRuleEnum.extra_link_success,
             schema_path=["extra_links", "schema"],
@@ -123,6 +125,7 @@ def validate_type_schema(
                 need,
                 field_properties,
                 validator,
+                reduce=False,
                 fail_rule=MessageRuleEnum.select_fail,
                 success_rule=MessageRuleEnum.select_success,
                 schema_path=[schema_name, "select"],
@@ -211,6 +214,7 @@ def recurse_validate_schemas(
             validator,
             rule_fail,
             rule_success,
+            reduce=True,
             schema_path=[*schema_path, "local"],
             need_path=need_path,
             user_message=user_message if recurse_level == 0 else None,
@@ -440,24 +444,34 @@ def reduce_need(
     schema_properties: set[str],
 ) -> dict[str, Any]:
     """
-    Reduce a need to its relevant fields for validation in a specific schema context.
+    Reduce a need to only actively-set fields for type-specific schema validation.
 
-    The reduction is required to separated actively set fields from defaults.
-    Also internal fields shall be removed, if they are not actively used in the schema.
-    This is required to make unevaluatedProperties work as expected which disallows
-    additional fields.
+    This is used for ``local`` and ``network`` schemas, where the validator needs to
+    distinguish between fields that were explicitly set and those at their defaults.
+    It strips out:
 
-    Needs can be reduced in multiple contexts as the need can be primary target of validation
-    or it can be a link target which might mean only a single field shall be checked for a
-    specific value.
+    - Extra fields that are ``None`` (not provided)
+    - Link fields that are empty (``[]``)
+    - Core fields that are ``None``, not referenced in the schema, or match their default
 
-    Fields are kept
-    - if they are extra fields and differ from their default value
-    - if they are links and the list is not empty
-    - if they are part of the user provided schema
+    This is required for:
+
+    - ``unevaluatedProperties``: without reduction, internal/unrelated fields would
+      cause false "additional property" failures.
+    - ``required``: without reduction, core fields at their default value would still
+      be present, so ``required`` would always pass even if the user never explicitly
+      set the field.
+
+    .. note::
+
+       This cannot distinguish "explicitly set to the default value" from "never set."
+       For example, if a user sets ``:tags:`` to ``[]`` and the schema has ``minItems: 1``,
+       the field is stripped (since ``[]`` matches the default) and the violation is silently
+       ignored.
 
     :param need: The need to reduce.
-    :param json_schema: The user provided and merged JSON merge.
+    :param field_properties: Mapping of field names to their schema properties (type, default).
+    :param schema_properties: Set of field names referenced in the user-provided schema.
     """
     reduced_need: dict[str, Any] = {}
 
@@ -559,15 +573,33 @@ def get_ontology_warnings(
     success_rule: MessageRuleEnum,
     schema_path: list[str],
     need_path: list[str],
+    reduce: bool,
     user_message: str | None = None,
     user_severity: SeverityEnum | None = None,
 ) -> list[OntologyWarning]:
-    reduced_need = reduce_need(need, field_properties, validator.properties)
+    """Validate a need against a compiled schema and return a list of warnings.
+
+    :param need: The need to validate.
+    :param field_properties: The properties of the need fields.
+    :param validator: The compiled schema validator to use for validation.
+    :param fail_rule: The MessageRuleEnum to use for validation failure warnings.
+    :param success_rule: The MessageRuleEnum to use for validation success warnings.
+    :param schema_path: The path to the schema in the configuration for reporting purposes.
+    :param need_path: The path to the need for reporting purposes.
+    :param reduce: Whether to reduce the need to relevant fields before validation.
+    :param user_message: An optional user message to include in the warnings.
+    :param user_severity: An optional user severity to override the default severity for the rules
+    """
+    if reduce:
+        needs_json = reduce_need(need, field_properties, validator.properties)
+    else:
+        # we always remove null values, since we currently do not allow for `{"string", "null"}` type definitions and so null values would cause validation errors
+        needs_json = {k: v for k, v in need.iter_schema_items() if v is not None}
     warnings: list[OntologyWarning] = []
     warning: OntologyWarning
     try:
         validation_errors: list[ValidationError] = list(
-            validator.compiled.iter_errors(instance=reduced_need)
+            validator.compiled.iter_errors(instance=needs_json)
         )
     except ValidationError as exc:
         warning = {
@@ -590,7 +622,7 @@ def get_ontology_warnings(
                 "severity": get_severity(fail_rule, user_severity),
                 "validation_message": err.message,
                 "need": need,
-                "reduced_need": reduced_need,
+                "reduced_need": needs_json,
                 "final_schema": validator.raw,
                 "schema_path": [*schema_path, *(str(item) for item in err.schema_path)],
                 "need_path": need_path,
@@ -605,7 +637,7 @@ def get_ontology_warnings(
             "rule": success_rule,
             "severity": get_severity(success_rule),
             "need": need,
-            "reduced_need": reduced_need,
+            "reduced_need": needs_json,
             "final_schema": validator.raw,
             "schema_path": schema_path,
             "need_path": need_path,

sphinx_needs/schema/process.py

@@ -14,8 +14,8 @@
 from sphinx_needs.schema.config import NeedFieldsSchemaType, SchemasRootType
 from sphinx_needs.schema.core import (
     NeedFieldProperties,
-    validate_link_options,
-    validate_option_fields,
+    validate_fields,
+    validate_links,
     validate_type_schema,
 )
 from sphinx_needs.schema.reporting import (
@@ -69,16 +69,12 @@ def process_schemas(app: Sphinx, builder: Builder) -> None:
     need_2_warnings: dict[str, list[OntologyWarning]] = {}
 
     if fields_schema["properties"]:
-        extra_warnings = validate_option_fields(
-            config, fields_schema, field_properties, needs
-        )
+        extra_warnings = validate_fields(config, fields_schema, field_properties, needs)
         for key, warnings in extra_warnings.items():
             need_2_warnings.setdefault(key, []).extend(warnings)
 
     if links_schema["properties"]:
-        link_warnings = validate_link_options(
-            config, links_schema, field_properties, needs
-        )
+        link_warnings = validate_links(config, links_schema, field_properties, needs)
         for key, warnings in link_warnings.items():
             need_2_warnings.setdefault(key, []).extend(warnings)