Add validation for reverse connections pointing through reverse prope…

cognite/neat/_data_model/rules/dms/__init__.py

@@ -21,6 +21,7 @@
     ReverseConnectionSourceViewMissing,
     ReverseConnectionTargetMismatch,
     ReverseConnectionTargetMissing,
+    ReverseConnectionThroughPropertyIsReverse,
 )
 from ._consistency import ViewSpaceVersionInconsistentWithDataModel
 from ._containers import (
@@ -76,6 +77,7 @@
     "ReverseConnectionSourceViewMissing",
     "ReverseConnectionTargetMismatch",
     "ReverseConnectionTargetMissing",
+    "ReverseConnectionThroughPropertyIsReverse",
     "SuboptimalRequiresConstraint",
     "UnresolvableQueryPerformance",
     "ViewContainerCountIsOutOfLimits",

cognite/neat/_data_model/rules/dms/_connections.py

@@ -7,7 +7,10 @@
     ViewDirectReference,
     ViewReference,
 )
-from cognite.neat._data_model.models.dms._view_property import ViewCorePropertyRequest
+from cognite.neat._data_model.models.dms._view_property import (
+    ReverseDirectRelationProperty,
+    ViewCorePropertyRequest,
+)
 from cognite.neat._data_model.rules.dms._base import DataModelRule
 from cognite.neat._issues import ConsistencyError, Recommendation
 
@@ -254,6 +257,9 @@ def validate(self) -> list[ConsistencyError]:
 
             source_property = source_view_expanded.properties[through.identifier]
 
+            if isinstance(source_property, ReverseDirectRelationProperty):
+                continue  # Handled by ReverseConnectionThroughPropertyIsReverse
+
             if not isinstance(source_property, ViewCorePropertyRequest):
                 errors.append(
                     ConsistencyError(
@@ -538,3 +544,113 @@ def validate(self) -> list[Recommendation]:
                 )
 
         return recommendations
+
+
+class ReverseConnectionThroughPropertyIsReverse(DataModelRule):
+    """Validates that reverse connections do not point through other reverse direct relations.
+
+    ## What it does
+    Checks that the property referenced in a reverse connection's 'through' clause
+    is a direct relation property, not another reverse direct relation property.
+
+    ## Why is this bad?
+    Reverse direct relation properties do not map to a container; they rely on the
+    direct relation property they reverse. Pointing two reverse properties at each other
+    creates a cycle that never resolves to container storage, even when the containers
+    have correct direct relation properties defined.
+
+    ## Example
+    If ViewA has reverse property `items` through ViewB's reverse property `owners`,
+    and ViewB has reverse property `owners` through ViewA's reverse property `items`,
+    neither reverse connection maps down to a container direct relation.
+    """
+
+    code = f"{BASE_CODE}-REVERSE-010"
+    issue_type = ConsistencyError
+
+    def validate(self) -> list[ConsistencyError]:
+        errors: list[ConsistencyError] = []
+
+        for (target_view_ref, reverse_prop_name), (
+            source_view_ref,
+            through,
+        ) in self.validation_resources.reverse_to_direct_mapping.items():
+            through = self.validation_resources.normalize_through_reference(source_view_ref, through)
+            source_view = self.validation_resources.select_view(source_view_ref, through.identifier)
+
+            if not source_view:
+                continue  # Handled by ReverseConnectionSourceViewMissing
+
+            if not (source_view_expanded := self.validation_resources.expand_view_properties(source_view_ref)):
+                raise RuntimeError(f"{type(self).__name__}: View {source_view_ref!s} not found. This is a bug in NEAT.")
+
+            if not source_view_expanded.properties or through.identifier not in source_view_expanded.properties:
+                continue  # Handled by ReverseConnectionSourcePropertyMissing
+
+            source_property = source_view_expanded.properties[through.identifier]
+
+            if not isinstance(source_property, ReverseDirectRelationProperty):
+                continue
+
+            cycle_path = self._find_reverse_cycle_path(target_view_ref, reverse_prop_name)
+            cycle_suffix = ""
+            if cycle_path:
+                path_str = " -> ".join(f"{view!s}.{prop}" for view, prop in cycle_path)
+                cycle_suffix = f"This forms a cycle of reverse connections: {path_str}."
+
+            errors.append(
+                ConsistencyError(
+                    message=(
+                        f"Reverse connection '{reverse_prop_name}' in view {target_view_ref!s} "
+                        f"points through '{through.identifier}' in view {source_view_ref!s}, "
+                        f"but '{through.identifier}' is itself a reverse direct relation "
+                        f"(not a direct relation that maps to a container). "
+                        f"Reverse connections must reference a direct relation property. {cycle_suffix}"
+                    ),
+                    fix=(
+                        "Update the reverse connection to point through the corresponding "
+                        "direct relation property on the source view (not another reverse property)."
+                    ),
+                    code=self.code,
+                )
+            )
+
+        return errors
+
+    def _find_reverse_cycle_path(
+        self,
+        start_target_view: ViewReference,
+        start_reverse_prop: str,
+    ) -> list[tuple[ViewReference, str]] | None:
+        """Return the cycle path if reverse-through-reverse forms a cycle, else None."""
+        seen: list[tuple[ViewReference, str]] = []
+        current = (start_target_view, start_reverse_prop)
+
+        while True:
+            if current in seen:
+                return seen + [current]
+
+            mapping = self.validation_resources.reverse_to_direct_mapping.get(current)
+            if not mapping:
+                return None
+
+            source_view_ref, through = mapping
+            through = self.validation_resources.normalize_through_reference(source_view_ref, through)
+
+            source_view_expanded = self.validation_resources.expand_view_properties(source_view_ref)
+            if not source_view_expanded or not source_view_expanded.properties:
+                return None
+
+            if through.identifier not in source_view_expanded.properties:
+                return None
+
+            through_property = source_view_expanded.properties[through.identifier]
+
+            if isinstance(through_property, ViewCorePropertyRequest):
+                return None
+
+            if not isinstance(through_property, ReverseDirectRelationProperty):
+                return None
+
+            seen.append(current)
+            current = (source_view_ref, through.identifier)

docs/validation/index.md

@@ -1,4 +1,4 @@
-**Neat supports 38 validation rules** for data modeling. These rules are learned
+**Neat supports 39 validation rules** for data modeling. These rules are learned
  from best practice, knowledge of the Cognite Data Fusion data modeling service, and practical experience from
  helping customers build and maintain their data models.
 
@@ -36,6 +36,7 @@ Validators for connections in data model specifications.
 | NEAT-DMS-CONNECTIONS-REVERSE-007 | [ReverseConnectionTargetMissing](neat-dms-connections-reverse-007.md) | Validates that the direct connection in reverse connection pair have target views specified. |
 | NEAT-DMS-CONNECTIONS-REVERSE-008 | [ReverseConnectionPointsToAncestor](neat-dms-connections-reverse-008.md) | Validates that direct connections point to specific views rather than ancestors. |
 | NEAT-DMS-CONNECTIONS-REVERSE-009 | [ReverseConnectionTargetMismatch](neat-dms-connections-reverse-009.md) | Validates that direct connections point to the correct target views. |
+| NEAT-DMS-CONNECTIONS-REVERSE-010 | [ReverseConnectionThroughPropertyIsReverse](neat-dms-connections-reverse-010.md) | Validates that reverse connections do not point through other reverse direct relations. |
 
 ### Consistency (NEAT-DMS-CONSISTENCY)
 

docs/validation/neat-dms-connections-reverse-010.md

@@ -0,0 +1,16 @@
+Validates that reverse connections do not point through other reverse direct relations.
+
+## What it does
+Checks that the property referenced in a reverse connection's 'through' clause
+is a direct relation property, not another reverse direct relation property.
+
+## Why is this bad?
+Reverse direct relation properties do not map to a container; they rely on the
+direct relation property they reverse. Pointing two reverse properties at each other
+creates a cycle that never resolves to container storage, even when the containers
+have correct direct relation properties defined.
+
+## Example
+If ViewA has reverse property `items` through ViewB's reverse property `owners`,
+and ViewB has reverse property `owners` through ViewA's reverse property `items`,
+neither reverse connection maps down to a container direct relation.

tests/data/snapshots/local/bi_directional_connections/containers.yaml

@@ -95,3 +95,21 @@
       type:
         type: direct
         list: true
+- space: my_space
+  externalId: CyclicContainerA
+  usedFor: node
+  properties:
+    cyclicDirectAStorage:
+      nullable: true
+      type:
+        type: direct
+        list: false
+- space: my_space
+  externalId: CyclicContainerB
+  usedFor: node
+  properties:
+    cyclicDirectBStorage:
+      nullable: true
+      type:
+        type: direct
+        list: false

tests/data/snapshots/local/bi_directional_connections/data_model.yaml

@@ -50,3 +50,11 @@ views:
     externalId: ReverseToListDirectView
     version: v1
     type: view
+  - space: my_space
+    externalId: CyclicViewA
+    version: v1
+    type: view
+  - space: my_space
+    externalId: CyclicViewB
+    version: v1
+    type: view

tests/data/snapshots/local/bi_directional_connections/views.yaml

@@ -378,3 +378,61 @@
           version: v1
           type: view
         identifier: listDirectRelationTarget
+- space: my_space
+  externalId: CyclicViewA
+  version: v1
+  properties:
+    cyclicDirectA:
+      container:
+        space: my_space
+        externalId: CyclicContainerA
+        type: container
+      containerPropertyIdentifier: cyclicDirectAStorage
+      source:
+        space: my_space
+        externalId: CyclicViewB
+        version: v1
+        type: view
+    cyclicReverseA:
+      connectionType: single_reverse_direct_relation
+      source:
+        space: my_space
+        externalId: CyclicViewB
+        version: v1
+        type: view
+      through:
+        source:
+          space: my_space
+          externalId: CyclicViewB
+          version: v1
+          type: view
+        identifier: cyclicReverseB
+- space: my_space
+  externalId: CyclicViewB
+  version: v1
+  properties:
+    cyclicDirectB:
+      container:
+        space: my_space
+        externalId: CyclicContainerB
+        type: container
+      containerPropertyIdentifier: cyclicDirectBStorage
+      source:
+        space: my_space
+        externalId: CyclicViewA
+        version: v1
+        type: view
+    cyclicReverseB:
+      connectionType: single_reverse_direct_relation
+      source:
+        space: my_space
+        externalId: CyclicViewA
+        version: v1
+        type: view
+      through:
+        source:
+          space: my_space
+          externalId: CyclicViewA
+          version: v1
+          type: view
+        identifier: cyclicReverseA

tests/tests_unit/test_data_model/test_rules/dms/test_bidirectional_connection_validator.py

@@ -17,6 +17,7 @@
     ReverseConnectionSourceViewMissing,
     ReverseConnectionTargetMismatch,
     ReverseConnectionTargetMissing,
+    ReverseConnectionThroughPropertyIsReverse,
 )
 from tests.data import SNAPSHOT_CATALOG
 
@@ -34,6 +35,7 @@
     ReverseConnectionTargetMissing: {"reverseToAttribute", "reverseToDirectWithoutTyping"},
     ReverseConnectionPointsToAncestor: {"innerReflection"},
     ReverseConnectionTargetMismatch: {"reverseSourceToTargetViewConnection"},
+    ReverseConnectionThroughPropertyIsReverse: {"cyclicReverseA", "cyclicReverseB"},
 }
 
 
@@ -101,3 +103,42 @@ def test_validation_deep(
                     break
 
     assert found_problematic_reversals == actual_problematic_reversal
+
+
+@pytest.mark.parametrize("profile", ["deep-additive", "legacy-additive"])
+def test_cyclic_reverse_relation_validator_message(
+    profile: Literal["deep-additive", "legacy-additive"],
+) -> None:
+    config = internal_profiles()[profile]
+    mode = config.modeling.mode
+    can_run_validator = config.validation.can_run_validator
+
+    local_snapshot, cdf_snapshot = SNAPSHOT_CATALOG.load_scenario(
+        "bi_directional_connections", "for_validators", modus_operandi=mode, include_cdm=False, format="snapshots"
+    )
+    data_model = SNAPSHOT_CATALOG.snapshot_to_request_schema(local_snapshot)
+
+    orchestrator = DmsDataModelRulesOrchestrator(
+        cdf_snapshot=cdf_snapshot,
+        limits=SchemaLimits(),
+        modus_operandi=mode,
+        can_run_validator=can_run_validator,
+    )
+    orchestrator.run(data_model)
+    by_code = orchestrator.issues.by_code()
+
+    if can_run_validator(
+        ReverseConnectionThroughPropertyIsReverse.code, ReverseConnectionThroughPropertyIsReverse.issue_type
+    ):
+        cyclic_issues = by_code[ReverseConnectionThroughPropertyIsReverse.code]
+        cyclic_messages = [issue.message for issue in cyclic_issues if "cyclicReverse" in issue.message]
+
+        assert len(cyclic_messages) == 2
+        for message in cyclic_messages:
+            assert "reverse direct relation" in message
+            assert "cycle of reverse connections" in message
+            assert "cyclicReverseA" in message or "cyclicReverseB" in message
+
+        wrong_type_issues = by_code.get(ReverseConnectionSourcePropertyWrongType.code, [])
+        wrong_type_cyclic = [issue for issue in wrong_type_issues if "cyclicReverse" in issue.message]
+        assert wrong_type_cyclic == []