fix: Update looped status of entity even if no variables changed

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java

@@ -118,13 +118,18 @@ private boolean updateShadowVariable(EntityVariablePair<Solution_> entityVariabl
         var entity = entityVariable.entity();
         var shadowVariableReference = entityVariable.variableReference();
         var oldValue = shadowVariableReference.memberAccessor().executeGetter(entity);
+        var loopDescriptor = shadowVariableReference.shadowVariableLoopedDescriptor();
+        if (loopDescriptor != null) {
+            var oldLooped = (boolean) loopDescriptor.getValue(entity);
+            if (oldLooped != isLooped) {
+                // Loop status change; add to affected entities
+                affectedEntities.add(entityVariable);
+            }
+        }
 
         if (isLooped) {
-            // null might be a valid value, and thus it could be the case
-            // that is was not looped and null, then turned to looped and null,
-            // which is still considered a change.
-            affectedEntities.add(entityVariable);
             if (oldValue != null) {
+                affectedEntities.add(entityVariable);
                 changeShadowVariableAndNotify(shadowVariableReference, entity, null);
             }
             return true;

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java

@@ -2,25 +2,35 @@
 
 import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor;
 import ai.timefold.solver.core.impl.domain.variable.supply.Supply;
+import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel;
 
 import org.jspecify.annotations.NullMarked;
 
 @NullMarked
-public class DefaultShadowVariableSession<Solution_> implements Supply {
-
+public final class DefaultShadowVariableSession<Solution_> implements Supply {
     final VariableReferenceGraph<Solution_> graph;
 
     public DefaultShadowVariableSession(VariableReferenceGraph<Solution_> graph) {
         this.graph = graph;
     }
 
     public void beforeVariableChanged(VariableDescriptor<Solution_> variableDescriptor, Object entity) {
-        graph.beforeVariableChanged(variableDescriptor.getVariableMetaModel(),
+        beforeVariableChanged(variableDescriptor.getVariableMetaModel(),
                 entity);
     }
 
     public void afterVariableChanged(VariableDescriptor<Solution_> variableDescriptor, Object entity) {
-        graph.afterVariableChanged(variableDescriptor.getVariableMetaModel(),
+        afterVariableChanged(variableDescriptor.getVariableMetaModel(),
+                entity);
+    }
+
+    public void beforeVariableChanged(VariableMetaModel<Solution_, ?, ?> variableMetaModel, Object entity) {
+        graph.beforeVariableChanged(variableMetaModel,
+                entity);
+    }
+
+    public void afterVariableChanged(VariableMetaModel<Solution_, ?, ?> variableMetaModel, Object entity) {
+        graph.afterVariableChanged(variableMetaModel,
                 entity);
     }
 

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java

@@ -354,6 +354,12 @@ public void triggerVariableListenersInNotificationQueues() {
         }
         if (shadowVariableSession != null) {
             shadowVariableSession.updateVariables();
+            // Some internal variable listeners (such as those used
+            // to check for solution corruption) might have a declarative
+            // shadow variable as a source and need to be triggered here.
+            for (var notifiable : notifiableRegistry.getAll()) {
+                notifiable.triggerAllNotifications();
+            }
         }
         notificationQueuesAreEmpty = true;
     }

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/violation/SolutionTracker.java

@@ -111,9 +111,15 @@ private List<String> getEntitiesMissingBeforeAfterEvents(VariableSnapshotTotal<S
         return out;
     }
 
-    public String buildScoreCorruptionMessage() {
+    public record SolutionCorruptionResult(boolean isCorrupted, String message) {
+        public static SolutionCorruptionResult untracked() {
+            return new SolutionCorruptionResult(false, "");
+        }
+    }
+
+    public SolutionCorruptionResult buildSolutionCorruptionResult() {
         if (beforeMoveSolution == null) {
-            return "";
+            return new SolutionCorruptionResult(false, "");
         }
 
         StringBuilder out = new StringBuilder();
@@ -161,11 +167,17 @@ public String buildScoreCorruptionMessage() {
                     """.formatted(formatList(missingEventsBackward)));
         }
 
-        if (out.isEmpty()) {
-            return "Genuine and shadow variables agree with from scratch calculation after the undo move and match the state prior to the move.";
+        var isCorrupted = !out.isEmpty();
+        if (isCorrupted) {
+            return new SolutionCorruptionResult(isCorrupted, out.toString());
+        } else {
+            return new SolutionCorruptionResult(false,
+                    "Genuine and shadow variables agree with from scratch calculation after the undo move and match the state prior to the move.");
         }
+    }
 
-        return out.toString();
+    public String buildScoreCorruptionMessage() {
+        return buildSolutionCorruptionResult().message;
     }
 
     static <Solution_> List<String> getVariableChangedViolations(

core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java

@@ -781,6 +781,36 @@ that could cause the scoreDifference (%s)."""
         }
     }
 
+    public SolutionTracker.SolutionCorruptionResult getSolutionCorruptionAfterUndo(Move<Solution_> move,
+            InnerScore<Score_> undoInnerScore) {
+        var trackingWorkingSolution = solutionTracker != null;
+        if (trackingWorkingSolution) {
+            solutionTracker.setAfterUndoSolution(workingSolution);
+        }
+        // Precondition: assert that there are probably no corrupted constraints
+        var undoMoveToString = "Undo(%s)".formatted(move);
+        assertWorkingScoreFromScratch(undoInnerScore, undoMoveToString);
+        // Precondition: assert that shadow variables aren't stale after doing the undoMove
+        assertShadowVariablesAreNotStale(undoInnerScore, undoMoveToString);
+        if (trackingWorkingSolution) {
+            // Recalculate all shadow variables from scratch.
+            // We cannot set all shadow variables to null, since some variable listeners
+            // may expect them to be non-null.
+            // Instead, we just simulate a change to all genuine variables.
+            variableListenerSupport.forceTriggerAllVariableListeners(workingSolution);
+            solutionTracker.setUndoFromScratchSolution(workingSolution);
+
+            // Also calculate from scratch for the before solution, since it might
+            // have been corrupted but was only detected now
+            solutionTracker.restoreBeforeSolution();
+            variableListenerSupport.forceTriggerAllVariableListeners(workingSolution);
+            solutionTracker.setBeforeFromScratchSolution(workingSolution);
+
+            return solutionTracker.buildSolutionCorruptionResult();
+        }
+        return SolutionTracker.SolutionCorruptionResult.untracked();
+    }
+
     /**
      * @param uncorruptedScoreDirector never null
      * @param predicted true if the score was predicted and might have been calculated on another thread

core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAssertScoreDirector.java

@@ -0,0 +1,47 @@
+package ai.timefold.solver.core.impl.solver;
+
+import java.util.Map;
+
+import ai.timefold.solver.core.api.score.Score;
+import ai.timefold.solver.core.api.score.constraint.ConstraintMatchTotal;
+import ai.timefold.solver.core.api.score.constraint.Indictment;
+import ai.timefold.solver.core.impl.score.constraint.ConstraintMatchPolicy;
+import ai.timefold.solver.core.impl.score.director.AbstractScoreDirector;
+import ai.timefold.solver.core.impl.score.director.InnerScore;
+
+public class MoveAssertScoreDirector<Solution_, Score_ extends Score<Score_>>
+        extends AbstractScoreDirector<Solution_, Score_, MoveAssertScoreDirectorFactory<Solution_, Score_>> {
+
+    protected MoveAssertScoreDirector(MoveAssertScoreDirectorFactory<Solution_, Score_> scoreDirectorFactory,
+            boolean lookUpEnabled,
+            ConstraintMatchPolicy constraintMatchPolicy,
+            boolean expectShadowVariablesInCorrectState) {
+        super(scoreDirectorFactory, lookUpEnabled, constraintMatchPolicy, expectShadowVariablesInCorrectState);
+    }
+
+    @Override
+    public void setWorkingSolution(Solution_ workingSolution) {
+        super.setWorkingSolution(workingSolution, ignored -> {
+        });
+    }
+
+    @Override
+    public InnerScore<Score_> calculateScore() {
+        return InnerScore.fullyAssigned(scoreDirectorFactory.getScoreDefinition().getZeroScore());
+    }
+
+    @Override
+    public Map<String, ConstraintMatchTotal<Score_>> getConstraintMatchTotalMap() {
+        return Map.of();
+    }
+
+    @Override
+    public Map<Object, Indictment<Score_>> getIndictmentMap() {
+        return Map.of();
+    }
+
+    @Override
+    public boolean requiresFlushing() {
+        return false;
+    }
+}

core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAssertScoreDirectorFactory.java

@@ -0,0 +1,38 @@
+package ai.timefold.solver.core.impl.solver;
+
+import ai.timefold.solver.core.api.score.Score;
+import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor;
+import ai.timefold.solver.core.impl.score.director.AbstractScoreDirector;
+import ai.timefold.solver.core.impl.score.director.AbstractScoreDirectorFactory;
+
+import org.jspecify.annotations.NullMarked;
+
+@NullMarked
+public class MoveAssertScoreDirectorFactory<Solution_, Score_ extends Score<Score_>>
+        extends AbstractScoreDirectorFactory<Solution_, Score_, MoveAssertScoreDirectorFactory<Solution_, Score_>> {
+    public MoveAssertScoreDirectorFactory(
+            SolutionDescriptor<Solution_> solutionDescriptor) {
+        super(solutionDescriptor);
+    }
+
+    @Override
+    public AbstractScoreDirector.AbstractScoreDirectorBuilder<Solution_, Score_, ?, ?> createScoreDirectorBuilder() {
+        return new MoveAssertScoreDirectorBuilder(this);
+    }
+
+    public class MoveAssertScoreDirectorBuilder extends
+            AbstractScoreDirector.AbstractScoreDirectorBuilder<Solution_, Score_, MoveAssertScoreDirectorFactory<Solution_, Score_>, MoveAssertScoreDirectorBuilder> {
+
+        protected MoveAssertScoreDirectorBuilder(MoveAssertScoreDirectorFactory<Solution_, Score_> scoreDirectorFactory) {
+            super(scoreDirectorFactory);
+        }
+
+        @Override
+        public MoveAssertScoreDirector<Solution_, Score_> build() {
+            return new MoveAssertScoreDirector<>(scoreDirectorFactory,
+                    lookUpEnabled,
+                    constraintMatchPolicy,
+                    expectShadowVariablesInCorrectState);
+        }
+    }
+}

core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAsserter.java

@@ -0,0 +1,37 @@
+package ai.timefold.solver.core.impl.solver;
+
+import ai.timefold.solver.core.api.score.Score;
+import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor;
+import ai.timefold.solver.core.impl.score.director.InnerScore;
+import ai.timefold.solver.core.preview.api.move.Move;
+
+public class MoveAsserter<Solution_> {
+    private final SolutionDescriptor<Solution_> solutionDescriptor;
+
+    private MoveAsserter(SolutionDescriptor<Solution_> solutionDescriptor) {
+        this.solutionDescriptor = solutionDescriptor;
+    }
+
+    public static <Solution_> MoveAsserter<Solution_> create(SolutionDescriptor<Solution_> solutionDescriptor) {
+        return new MoveAsserter<>(solutionDescriptor);
+    }
+
+    public void assertMove(Solution_ solution, Move<Solution_> move) {
+        var scoreDirectorFactory = new MoveAssertScoreDirectorFactory<>(solutionDescriptor);
+        scoreDirectorFactory.setTrackingWorkingSolution(true);
+        try (var scoreDirector = scoreDirectorFactory.createScoreDirectorBuilder()
+                .withLookUpEnabled(false)
+                .buildDerived()) {
+            var innerScore = InnerScore.fullyAssigned((Score) scoreDirector.getScoreDefinition().getZeroScore());
+            scoreDirector.setWorkingSolution(solution);
+            scoreDirector.executeTemporaryMove(move, true);
+            var corruptionResult = scoreDirector.getSolutionCorruptionAfterUndo(move, innerScore);
+            if (corruptionResult.isCorrupted()) {
+                throw new AssertionError("""
+                        Solution corruption caused by move (%s) or its undo.
+                        Analysis:
+                        %s""".formatted(move, corruptionResult.message()));
+            }
+        }
+    }
+}

core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/dependent/DependencyValuesShadowVariableTest.java

@@ -6,12 +6,17 @@
 import java.time.LocalDateTime;
 import java.time.ZoneOffset;
 import java.util.List;
+import java.util.Set;
 
 import ai.timefold.solver.core.api.solver.SolverFactory;
 import ai.timefold.solver.core.config.solver.EnvironmentMode;
 import ai.timefold.solver.core.config.solver.PreviewFeature;
 import ai.timefold.solver.core.config.solver.SolverConfig;
 import ai.timefold.solver.core.config.solver.termination.TerminationConfig;
+import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor;
+import ai.timefold.solver.core.impl.move.streams.generic.move.ListAssignMove;
+import ai.timefold.solver.core.impl.solver.MoveAsserter;
+import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningListVariableMetaModel;
 import ai.timefold.solver.core.testdomain.declarative.dependency.TestdataDependencyConstraintProvider;
 import ai.timefold.solver.core.testdomain.declarative.dependency.TestdataDependencyEntity;
 import ai.timefold.solver.core.testdomain.declarative.dependency.TestdataDependencySolution;
@@ -69,4 +74,37 @@ void testSolve() {
             }
         }
     }
+
+    @Test
+    void testLoopStatusOfEntityIsUpdatedEvenIfNoVariablesOnTheEntityChanged() {
+        var baseTime = LocalDateTime.ofEpochSecond(0, 0, ZoneOffset.UTC);
+        var entityA = new TestdataDependencyEntity(baseTime);
+        var entityB = new TestdataDependencyEntity(baseTime);
+        var entityC = new TestdataDependencyEntity(baseTime);
+
+        var valueA = new TestdataDependencyValue("A", Duration.ofHours(5), null);
+        var valueB = new TestdataDependencyValue("B", Duration.ofHours(6), null);
+        var valueC = new TestdataDependencyValue("C", Duration.ofHours(7), List.of(valueA, valueB));
+
+        var schedule = new TestdataDependencySolution(
+                List.of(entityA, entityB, entityC),
+                List.of(valueA, valueB, valueC));
+
+        entityA.getValues().add(valueB);
+        entityA.getValues().add(valueA);
+
+        var solutionDescriptor = SolutionDescriptor.buildSolutionDescriptor(Set.of(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES),
+                TestdataDependencySolution.class, TestdataDependencyEntity.class, TestdataDependencyValue.class);
+        var moveAsserter = MoveAsserter.create(solutionDescriptor);
+
+        // Tests the move [A, B] -> [C, A, B].
+        // Since C depends on A and B, this is an invalid solution,
+        // and C.startTime/C.endTime remains null and C.isLooped is true.
+        // When the move is undone, C.startTime/C.endTime remains null,
+        // and C.isLooped is false.
+        moveAsserter.assertMove(schedule, new ListAssignMove<>(
+                (PlanningListVariableMetaModel<TestdataDependencySolution, ? super TestdataDependencyEntity, ? super TestdataDependencyValue>) solutionDescriptor
+                        .getListVariableDescriptor().getVariableMetaModel(),
+                valueC, entityA, 0));
+    }
 }