[THIS-1049] Add fix() method to fixable validators (#1573)

# Description

Add `fix()` implementations to the existing `MissingRequiresConstraint`,
`SuboptimalRequiresConstraint`, `RequiresConstraintCycle`, and
`MissingReverseDirectRelationTargetIndex` validators, enabling them to
produce `FixAction`s that can be applied by the autofix infrastructure
from part 1.

Note: This PR introduces a slight change in how recommendations are made
for `RequiresConstraintCycle` as to better align with the fix method.
This issue itself will not recommend removing "all involved suboptimal
constraints" involved in a specific cycle, but rather select the single
"most problematic" one to remove, since that is all that is required to
fix the cycle itself. In practice this mostly makes **no** difference
for the user if they are running fix, however in the very specific edge
case that they disable `MissingRequiresConstraint` and
`SuboptimalRequiresConstraint`, running the validation for
`RequiresConstraintCycle` will now only recommend removing the "one"
constraint to fix the cycle itself. I consider this to be a more
appropriately scoped recommendation/fix logic for this validator, as
opposed to the previous logic, where the recommendations for
`SuboptimalRequiresConstraint` could somewhat "leak" into this one.

Analysis/query logic shared across validators has been moved to cached
properties on `ValidationResources`.

## Bump

- [ ] Patch
- [x] Skip

---------

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: Magssch

cognite/neat/_data_model/_analysis.py

@@ -14,6 +14,7 @@
 from cognite.neat._data_model.models.dms._constraints import RequiresConstraintDefinition
 from cognite.neat._data_model.models.dms._container import ContainerPropertyDefinition, ContainerRequest
 from cognite.neat._data_model.models.dms._data_types import DirectNodeRelation
+from cognite.neat._data_model.models.dms._indexes import BtreeIndex
 from cognite.neat._data_model.models.dms._limits import SchemaLimits
 from cognite.neat._data_model.models.dms._references import (
     ContainerDirectReference,
@@ -622,10 +623,10 @@ def implements_graph(self) -> nx.DiGraph:
         return graph
 
     @cached_property
-    def implements_cycles(self) -> list[list[ViewReference]]:
+    def implements_cycles(self) -> list[tuple[ViewReference, ...]]:
         """Find all cycles in the implements graph.
         Returns:
-            List of lists, where each list contains the ordered Views involved in forming the implements cycle.
+            List of tuples, where each tuple contains the ordered Views involved in forming the implements cycle.
         """
 
         return self.graph_cycles(self.implements_graph)
@@ -731,14 +732,23 @@ def _user_intentional_constraints(self) -> set[tuple[ContainerReference, Contain
 
         A constraint is user-intentional if:
         1. The constraint identifier does NOT have '__auto' postfix
-        2. Neither src nor dst is part of a cycle (cyclic constraints are errors)
+        2. Neither src nor dst is part of a manually-created cycle
 
         These constraints are preserved even if they're not in the optimal structure, because
         they may be used for data integrity purposes.
-        We DON'T consider manual-created constraints as user-intended if they form part of a cycle,
-        because that indicates a problem with the data model where we likely can provide a better solution.
+        We DON'T consider manual-created constraints as user-intended if they form part of a cycle
+        consisting entirely of manual constraints. We ignore these because it indicates a problem with
+        the data model where we likely can provide a better solution. We don't consider cycles formed
+        only by __auto constraints.
         """
-        containers_in_cycles = {container for cycle in self.requires_constraint_cycles for container in cycle}
+        containers_in_cycles: set[ContainerReference] = set()
+        for cycle in self.requires_constraint_cycles:
+            if all(
+                self.requires_constraint_graph.edges[cycle[i], cycle[(i + 1) % len(cycle)]].get("is_auto", False)
+                for i in range(len(cycle))
+            ):
+                continue
+            containers_in_cycles.update(cycle)
 
         return {
             (src, dst)
@@ -786,17 +796,57 @@ def forms_directed_path(nodes: set[_NodeT], graph: nx.DiGraph) -> bool:
         return False
 
     @cached_property
-    def requires_constraint_cycles(self) -> list[list[ContainerReference]]:
+    def requires_constraint_cycles(self) -> list[tuple[ContainerReference, ...]]:
         """Find all cycles in the requires constraint graph.
         Returns:
-            List of lists, where each list contains the ordered containers involved in forming the requires cycle.
+            List of tuples, where each tuple contains the ordered containers involved in forming the requires cycle.
         """
         return self.graph_cycles(self.requires_constraint_graph)
 
     @staticmethod
-    def graph_cycles(graph: nx.DiGraph) -> list[list[T_Reference]]:
+    def graph_cycles(graph: nx.DiGraph) -> list[tuple[T_Reference, ...]]:
         """Returns cycles in the graph otherwise empty list"""
-        return [candidate for candidate in nx.simple_cycles(graph) if len(candidate) > 1]
+        return [tuple(candidate) for candidate in nx.simple_cycles(graph) if len(candidate) > 1]
+
+    def pick_cycle_constraint_to_remove(
+        self, cycle: tuple[ContainerReference, ...]
+    ) -> tuple[ContainerReference, ContainerReference]:
+        """Pick the single best requires constraint to remove to break a cycle.
+
+        Selects a constraint edge from the cycle using a tiered preference:
+        auto-generated constraints are preferred over user-defined ones, and
+        edges that conflict with the direction considered optimal by the global optimizer
+        are preferred over those that are only redundant and covered by other constraints.
+        """
+        suboptimal_constraints: list[tuple[ContainerReference, ContainerReference]] = []
+        for i, source_container_ref in enumerate(cycle):
+            required_container_ref = cycle[(i + 1) % len(cycle)]
+            if (source_container_ref, required_container_ref) not in self.oriented_mst_edges:
+                suboptimal_constraints.append((source_container_ref, required_container_ref))
+
+        # Tier 1: auto-generated and wrong direction from optimal solution
+        for source_ref, required_ref in suboptimal_constraints:
+            if self.requires_constraint_graph.edges[source_ref, required_ref].get("is_auto", False):
+                if (required_ref, source_ref) in self.oriented_mst_edges:
+                    return source_ref, required_ref
+
+        # Tier 2: auto-generated and redundant (covered by other constraints)
+        for source_ref, required_ref in suboptimal_constraints:
+            if self.requires_constraint_graph.edges[source_ref, required_ref].get("is_auto", False):
+                return source_ref, required_ref
+
+        # Tier 3: user-defined and wrong direction from optimal solution
+        for source_ref, required_ref in suboptimal_constraints:
+            if (required_ref, source_ref) in self.oriented_mst_edges:
+                return source_ref, required_ref
+
+        # Tier 4: user-defined and redundant (covered by other constraints)
+        if suboptimal_constraints:
+            return suboptimal_constraints[0]
+
+        raise RuntimeError(
+            f"{type(self).__name__}: No removable constraint found in cycle {cycle}. This is a bug in NEAT."
+        )
 
     @cached_property
     def optimized_requires_constraint_graph(self) -> nx.DiGraph:
@@ -1000,6 +1050,80 @@ def _transitively_reduced_edges(self) -> set[tuple[ContainerReference, Container
         # Return MST edges that survive reduction
         return {e for e in reduced.edges() if e in self.oriented_mst_edges}
 
+    @cached_property
+    def missing_requires_constraints(
+        self,
+    ) -> list[tuple[ViewReference, ContainerReference, ContainerReference]]:
+        """Views with containers that are missing requires constraints needed for optimal query performance.
+
+        Each entry is a (view, source_container, required_container) tuple indicating that
+        source_container should have a requires constraint pointing to required_container.
+        """
+        missing_requires_constraints: list[tuple[ViewReference, ContainerReference, ContainerReference]] = []
+        for view_ref in self.merged.views:
+            changes = self.get_requires_changes_for_view(view_ref)
+            if changes.status != RequiresChangeStatus.CHANGES_AVAILABLE:
+                continue
+            for source_container_ref, required_container_ref in changes.to_add:
+                missing_requires_constraints.append((view_ref, source_container_ref, required_container_ref))
+        return missing_requires_constraints
+
+    @cached_property
+    def suboptimal_requires_constraints(
+        self,
+    ) -> list[tuple[ViewReference, ContainerReference, ContainerReference]]:
+        """Views with containers that have suboptimal requires constraints that should be removed.
+
+        Each entry is a (view, source_container, required_container) tuple indicating that
+        the existing requires constraint from source_container to required_container is
+        redundant or wrongly oriented relative to the optimal structure.
+        """
+        results: list[tuple[ViewReference, ContainerReference, ContainerReference]] = []
+        for view_ref in self.merged.views:
+            changes = self.get_requires_changes_for_view(view_ref)
+            if changes.status != RequiresChangeStatus.CHANGES_AVAILABLE:
+                continue
+            for source_container_ref, required_container_ref in changes.to_remove:
+                results.append((view_ref, source_container_ref, required_container_ref))
+        return results
+
+    @cached_property
+    def missing_reverse_relation_index_targets(
+        self,
+    ) -> list[tuple[ResolvedReverseDirectRelation, tuple[str, BtreeIndex] | None]]:
+        """Reverse direct relations missing a cursorable index on the target container property.
+
+        Returns:
+            List of tuples: (resolved_relation, existing_non_cursorable_index or None)
+        """
+        targets: list[tuple[ResolvedReverseDirectRelation, tuple[str, BtreeIndex] | None]] = []
+
+        for resolved in self.resolved_reverse_direct_relations:
+            if not resolved.container or not resolved.container_property:
+                continue
+            if resolved.container_ref.space in COGNITE_SPACES:
+                continue
+            if not isinstance(resolved.container_property.type, DirectNodeRelation):
+                continue
+            if resolved.container_property.type.list:
+                continue
+
+            for index_id, index in (resolved.container.indexes or {}).items():
+                if not isinstance(index, BtreeIndex):
+                    continue
+                if len(index.properties) != 1:
+                    continue
+                if resolved.container_property_id not in index.properties:
+                    continue
+                if index.cursorable:
+                    break
+                targets.append((resolved, (index_id, index)))
+                break
+            else:
+                targets.append((resolved, None))
+
+        return targets
+
     def get_requires_changes_for_view(self, view: ViewReference) -> RequiresChangesForView:
         """Get requires constraint changes needed to optimize a view.
 

cognite/neat/_data_model/_identifiers.py

@@ -1,5 +1,8 @@
+import hashlib
+
 from pydantic import HttpUrl, RootModel, ValidationError
 
+from cognite.neat._data_model.models.dms import ContainerReference
 from cognite.neat._utils.auxiliary import local_import
 
 
@@ -59,3 +62,47 @@ def as_rdflib_namespace(self):  # type: ignore[no-untyped-def]
         from rdflib import Namespace
 
         return Namespace(self.root)
+
+
+class AutoIdentifier:
+    """Generates identifiers for auto-created CDF constraints and indexes.
+
+    CDF has a 43-character limit on constraint/index identifiers. This class
+    ensures generated IDs stay within that limit while maintaining uniqueness.
+    """
+
+    MAX_LENGTH = 43
+    SUFFIX = "__auto"
+    HASH_LENGTH = 8
+    # base_id + suffix must fit in MAX_LENGTH
+    _MAX_BASE_NO_HASH = MAX_LENGTH - len(SUFFIX)
+    # base_id + "_" + hash + suffix must fit in MAX_LENGTH
+    _MAX_BASE_WITH_HASH = _MAX_BASE_NO_HASH - HASH_LENGTH - 1
+
+    @classmethod
+    def make(cls, base_id: str) -> str:
+        """Generate an auto identifier, truncating with a hash if needed.
+
+        Args:
+            base_id: The primary identifier (e.g., external_id or property_id).
+
+        Returns:
+            For short base_ids (<=37 chars): "{base_id}__auto"
+            For long base_ids (>37 chars): "{truncated_id}_{hash}__auto"
+        """
+        if len(base_id) <= cls._MAX_BASE_NO_HASH:
+            return f"{base_id}{cls.SUFFIX}"
+
+        hash_suffix = hashlib.sha256(base_id.encode()).hexdigest()[: cls.HASH_LENGTH]
+        truncated_id = base_id[: cls._MAX_BASE_WITH_HASH]
+        return f"{truncated_id}_{hash_suffix}{cls.SUFFIX}"
+
+    @classmethod
+    def for_constraint(cls, destination: ContainerReference) -> str:
+        """Generate a constraint identifier for auto-generated requires constraints."""
+        return cls.make(destination.external_id)
+
+    @classmethod
+    def for_index(cls, property_id: str) -> str:
+        """Generate an index identifier for auto-generated indexes."""
+        return cls.make(property_id)

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

@@ -1,5 +1,8 @@
 """Validators for checking containers in the data model."""
 
+from cognite.neat._data_model._fix import FixAction
+from cognite.neat._data_model.deployer.data_classes import RemovedField, SeverityType
+from cognite.neat._data_model.models.dms._references import ContainerReference
 from cognite.neat._data_model.models.dms._view_property import ViewCorePropertyRequest
 from cognite.neat._data_model.rules.dms._base import DataModelRule
 from cognite.neat._issues import ConsistencyError
@@ -209,32 +212,62 @@ class RequiresConstraintCycle(DataModelRule):
     code = f"{BASE_CODE}-005"
     issue_type = ConsistencyError
     alpha = True  # Still in development
+    fixable = True
 
     def validate(self) -> list[ConsistencyError]:
         errors: list[ConsistencyError] = []
-        optimal_edges = self.validation_resources.oriented_mst_edges
-
         for cycle in self.validation_resources.requires_constraint_cycles:
             cycle_str = " -> ".join(str(c) for c in cycle) + f" -> {cycle[0]!s}"
-
-            # Find edges in cycle that are NOT in optimal structure (these should be removed)
-            edges_to_remove = []
-            for i, container in enumerate(cycle):
-                next_container = cycle[(i + 1) % len(cycle)]
-                edge = (container, next_container)
-                if edge not in optimal_edges:
-                    edges_to_remove.append(f"{container} -> {next_container}")
-
-            message = f"Requires constraints form a cycle: {cycle_str}"
-            if edges_to_remove:
-                message += f". Recommended removal: {', '.join(edges_to_remove)} (not in optimal structure)"
-
+            source_container_ref, required_container_ref = self.validation_resources.pick_cycle_constraint_to_remove(
+                cycle
+            )
             errors.append(
                 ConsistencyError(
-                    message=message,
-                    fix="Remove one of the requires constraints to break the cycle",
+                    message=(
+                        f"Requires constraints form a cycle: {cycle_str}. This can be fixed by removing the requires "
+                        f"constraint on {source_container_ref!s} to {required_container_ref!s}"
+                    ),
+                    fix="Remove the recommended requires constraint to break the cycle",
                     code=self.code,
                 )
             )
 
         return errors
+
+    def fix(self) -> list[FixAction]:
+        """Return fix actions to break requires constraint cycles."""
+        fix_actions: list[FixAction] = []
+        # Overlapping cycles can share the same edge to remove. Dedup here
+        # because each constraint only needs to be removed once.
+        seen: set[tuple[ContainerReference, ContainerReference]] = set()
+
+        for cycle in self.validation_resources.requires_constraint_cycles:
+            source_container_ref, required_container_ref = self.validation_resources.pick_cycle_constraint_to_remove(
+                cycle
+            )
+            if (source_container_ref, required_container_ref) in seen:
+                continue
+            seen.add((source_container_ref, required_container_ref))
+
+            container = self.validation_resources.select_container(source_container_ref)
+            if not container:
+                continue
+            for constraint_id, constraint_def in self.validation_resources.get_requires_constraints(container):
+                if constraint_def.require != required_container_ref:
+                    continue
+                fix_actions.append(
+                    FixAction(
+                        code=self.code,
+                        resource_id=source_container_ref,
+                        changes=(
+                            RemovedField(
+                                field_path=f"constraints.{constraint_id}",
+                                current_value=constraint_def,
+                                item_severity=SeverityType.WARNING,
+                            ),
+                        ),
+                        message="Removed requires constraint to break cycle",
+                    )
+                )
+
+        return fix_actions