fix: handle filtered updates in Neighborhoods properly

core/src/main/java/ai/timefold/solver/core/impl/neighborhood/stream/DefaultMoveStreamFactory.java

@@ -1,5 +1,7 @@
 package ai.timefold.solver.core.impl.neighborhood.stream;
 
+import static ai.timefold.solver.core.preview.api.neighborhood.stream.joiner.NeighborhoodsJoiners.filtering;
+
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
@@ -87,24 +89,60 @@ public <A> UniEnumeratingStream<Solution_, A> forEachUnfiltered(Class<A> sourceC
                 .computeIfAbsent(variableMetaModel, ignored -> new NodeSharingSupportFunctions<>(variableMetaModel));
     }
 
+    @Override
+    public <Entity_, Value_> UniEnumeratingStream<Solution_, Value_>
+            forEachAssignedValueUnfiltered(PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel) {
+        var nodeSharingSupportFunctions = getNodeSharingSupportFunctions(variableMetaModel);
+        return forEachUnfiltered(variableMetaModel.type(), false)
+                .filter(nodeSharingSupportFunctions.assignedValueFilter);
+    }
+
+    @Override
+    public <Entity_, Value_> UniEnumeratingStream<Solution_, Value_>
+            forEachAssignedValue(PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel) {
+        var nodeSharingSupportFunctions = getNodeSharingSupportFunctions(variableMetaModel);
+        return forEach(variableMetaModel.type(), false)
+                .filter(nodeSharingSupportFunctions.assignedValueFilter);
+    }
+
+    @Override
+    public <Entity_, Value_> UniEnumeratingStream<Solution_, PositionInList>
+            forEachDestination(PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel) {
+        var unpinnedEntities = forEach(variableMetaModel.entity().type(), false);
+        // Stream with unpinned values, which are assigned to any list variable;
+        // always includes null so that we can later create a position at the end of the list,
+        // i.e. with no value after it.
+        var nodeSharingSupportFunctions = getNodeSharingSupportFunctions(variableMetaModel);
+        var unpinnedValues = forEach(variableMetaModel.type(), true)
+                .filter(nodeSharingSupportFunctions.assignedValueOrNullFilter);
+        // Joins the two previous streams to create pairs of (entity, value),
+        // eliminating values which do not match that entity's value range.
+        // It maps these pairs to expected target positions in that entity's list variable.
+        return unpinnedEntities.join(unpinnedValues,
+                filtering(nodeSharingSupportFunctions.valueInRangeFilter))
+                .map(nodeSharingSupportFunctions.toPositionInListMapper)
+                .distinct();
+    }
+
+    @SuppressWarnings({ "unchecked", "rawtypes" })
     @Override
     public <Entity_, Value_> UniEnumeratingStream<Solution_, ElementPosition>
-            forEachAssignablePosition(PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel) {
-        // Stream with unpinned entities;
-        // includes null if the variable allows unassigned values.
-        var unpinnedEntities =
-                forEach(variableMetaModel.entity().type(), variableMetaModel.allowsUnassignedValues());
+            forEachDestinationIncludingUnassigned(PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel) {
+        if (!variableMetaModel.allowsUnassignedValues()) {
+            return (UniEnumeratingStream) forEachDestination(variableMetaModel);
+        }
+        var unpinnedEntities = forEach(variableMetaModel.entity().type(), true);
         // Stream with unpinned values, which are assigned to any list variable;
         // always includes null so that we can later create a position at the end of the list,
         // i.e. with no value after it.
         var nodeSharingSupportFunctions = getNodeSharingSupportFunctions(variableMetaModel);
         var unpinnedValues = forEach(variableMetaModel.type(), true)
-                .filter(nodeSharingSupportFunctions.unpinnedValueFilter);
+                .filter(nodeSharingSupportFunctions.assignedValueOrNullFilter);
         // Joins the two previous streams to create pairs of (entity, value),
         // eliminating values which do not match that entity's value range.
         // It maps these pairs to expected target positions in that entity's list variable.
         return unpinnedEntities.join(unpinnedValues,
-                NeighborhoodsJoiners.filtering(nodeSharingSupportFunctions.valueInRangeFilter))
+                filtering(nodeSharingSupportFunctions.valueInRangeFilter))
                 .map(nodeSharingSupportFunctions.toElementPositionMapper)
                 .distinct();
     }
@@ -126,6 +164,28 @@ public SolutionDescriptor<Solution_> getSolutionDescriptor() {
         return enumeratingStreamFactory.getSolutionDescriptor();
     }
 
+    @Override
+    public <Entity_, Value_> UniEnumeratingStream<Solution_, ElementPosition>
+            forEachAssignablePosition(PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel) {
+        // Stream with unpinned entities;
+        // includes null if the variable allows unassigned values.
+        var unpinnedEntities =
+                forEach(variableMetaModel.entity().type(), variableMetaModel.allowsUnassignedValues());
+        // Stream with unpinned values, which are assigned to any list variable;
+        // always includes null so that we can later create a position at the end of the list,
+        // i.e. with no value after it.
+        var nodeSharingSupportFunctions = getNodeSharingSupportFunctions(variableMetaModel);
+        var unpinnedValues = forEach(variableMetaModel.type(), true)
+                .filter(nodeSharingSupportFunctions.assignedValueOrNullFilter);
+        // Joins the two previous streams to create pairs of (entity, value),
+        // eliminating values which do not match that entity's value range.
+        // It maps these pairs to expected target positions in that entity's list variable.
+        return unpinnedEntities.join(unpinnedValues,
+                NeighborhoodsJoiners.filtering(nodeSharingSupportFunctions.valueInRangeFilter))
+                .map(nodeSharingSupportFunctions.toElementPositionMapper)
+                .distinct();
+    }
+
     public record NodeSharingSupportFunctions<Solution_, Entity_, Value_>(
             PlanningVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel,
             BiNeighborhoodsPredicate<Solution_, Entity_, Value_> differentValueFilter,
@@ -142,14 +202,19 @@ public NodeSharingSupportFunctions(PlanningVariableMetaModel<Solution_, Entity_,
     public record ListVariableNodeSharingSupportFunctions<Solution_, Entity_, Value_>(
             PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel,
             UniNeighborhoodsPredicate<Solution_, Value_> unpinnedValueFilter,
+            UniNeighborhoodsPredicate<Solution_, Value_> assignedValueOrNullFilter,
+            UniNeighborhoodsPredicate<Solution_, Value_> assignedValueFilter,
             BiNeighborhoodsPredicate<Solution_, Entity_, Value_> valueInRangeFilter,
-            BiNeighborhoodsMapper<Solution_, Entity_, Value_, ElementPosition> toElementPositionMapper) {
+            BiNeighborhoodsMapper<Solution_, Entity_, Value_, ElementPosition> toElementPositionMapper,
+            BiNeighborhoodsMapper<Solution_, Entity_, Value_, PositionInList> toPositionInListMapper) {
 
         public ListVariableNodeSharingSupportFunctions(
                 PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel) {
             this(variableMetaModel,
+                    (solutionView, value) -> value == null || !solutionView.isPinned(variableMetaModel, value),
                     (solutionView, value) -> value == null
                             || solutionView.getPositionOf(variableMetaModel, value) instanceof PositionInList,
+                    (solutionView, value) -> solutionView.getPositionOf(variableMetaModel, value) instanceof PositionInList,
                     (solutionView, entity, value) -> {
                         if (entity == null || value == null) {
                             // Necessary for the null to survive until the later stage,
@@ -169,6 +234,14 @@ public ListVariableNodeSharingSupportFunctions(
                         } else { // This will trigger assignment of the value immediately before this value.
                             return solutionView.getPositionOf(variableMetaModel, value);
                         }
+                    },
+                    (solutionView, entity, value) -> {
+                        var valueCount = solutionView.countValues(variableMetaModel, entity);
+                        if (value == null || valueCount == 0) { // This will trigger assignment of the value at the end of the list.
+                            return ElementPosition.of(entity, valueCount);
+                        } else { // This will trigger assignment of the value immediately before this value.
+                            return solutionView.getPositionOf(variableMetaModel, value).ensureAssigned();
+                        }
                     });
         }
 

core/src/main/java/ai/timefold/solver/core/impl/neighborhood/stream/enumerating/common/AbstractLeftDatasetInstance.java

@@ -6,6 +6,7 @@
 import ai.timefold.solver.core.impl.bavet.common.index.UniqueRandomIterator;
 import ai.timefold.solver.core.impl.bavet.common.tuple.Tuple;
 import ai.timefold.solver.core.impl.util.ElementAwareArrayList;
+import ai.timefold.solver.core.impl.util.ElementAwareArrayList.Entry;
 
 import org.jspecify.annotations.NullMarked;
 
@@ -28,17 +29,34 @@ public int getRightIteratorStoreIndex() {
 
     @Override
     public void insert(Tuple_ tuple) {
+        if (tuple.getStore(entryStoreIndex) != null) {
+            throw new IllegalStateException(
+                    "Impossible state: the input for the tuple (%s) was already added in the tupleStore."
+                            .formatted(tuple));
+        }
+
         tuple.setStore(entryStoreIndex, tupleList.add(tuple));
     }
 
     @Override
     public void update(Tuple_ tuple) {
-        // No need to do anything.
+        if (tuple.getStore(entryStoreIndex) == null) {
+            // No fail fast if null because we don't track which tuples made it through the filter predicate(s)
+            insert(tuple);
+        } else {
+            // No need to do anything.
+        }
     }
 
     @Override
     public void retract(Tuple_ tuple) {
-        tupleList.remove(tuple.removeStore(entryStoreIndex));
+        Entry<Tuple_> entry = tuple.removeStore(entryStoreIndex);
+        if (entry == null) {
+            // No fail fast if null because we don't track which tuples made it through the filter predicate(s)
+            return;
+        }
+
+        tupleList.remove(entry);
     }
 
     @Override

core/src/main/java/ai/timefold/solver/core/impl/neighborhood/stream/enumerating/common/AbstractRightDatasetInstance.java

@@ -30,6 +30,12 @@ protected AbstractRightDatasetInstance(AbstractDataset<Solution_> parent,
 
     @Override
     public void insert(UniTuple<Right_> tuple) {
+        if (tuple.getStore(compositeKeyStoreIndex) != null) {
+            throw new IllegalStateException(
+                    "Impossible state: the input for the tuple (%s) was already added in the tupleStore."
+                            .formatted(tuple));
+        }
+
         var compositeKey = compositeKeyExtractor.apply(tuple);
         tuple.setStore(entryStoreIndex, indexer.put(compositeKey, tuple));
         tuple.setStore(compositeKeyStoreIndex, compositeKey);
@@ -38,6 +44,12 @@ public void insert(UniTuple<Right_> tuple) {
     @Override
     public void update(UniTuple<Right_> tuple) {
         var oldCompositeKey = tuple.getStore(compositeKeyStoreIndex);
+        if (oldCompositeKey == null) {
+            // No fail fast if null because we don't track which tuples made it through the filter predicate(s)
+            insert(tuple);
+            return;
+        }
+
         var newCompositeKey = compositeKeyExtractor.apply(tuple);
         if (!Objects.equals(oldCompositeKey, newCompositeKey)) {
             indexer.remove(oldCompositeKey, tuple.getStore(entryStoreIndex));
@@ -48,7 +60,13 @@ public void update(UniTuple<Right_> tuple) {
 
     @Override
     public void retract(UniTuple<Right_> tuple) {
-        indexer.remove(tuple.removeStore(compositeKeyStoreIndex), tuple.removeStore(entryStoreIndex));
+        var compositeKey = tuple.removeStore(compositeKeyStoreIndex);
+        if (compositeKey == null) {
+            // No fail fast if null because we don't track which tuples made it through the filter predicate(s)
+            return;
+        }
+
+        indexer.remove(compositeKey, tuple.removeStore(entryStoreIndex));
     }
 
     public Iterator<UniTuple<Right_>> iterator(Object compositeKey) {

core/src/main/java/ai/timefold/solver/core/preview/api/move/builtin/ListChangeMoveProvider.java

@@ -45,7 +45,7 @@ public ListChangeMoveProvider(PlanningListVariableMetaModel<Solution_, Entity_,
 
     @Override
     public MoveStream<Solution_> build(MoveStreamFactory<Solution_> moveStreamFactory) {
-        var entityValuePairs = moveStreamFactory.forEachAssignablePosition(variableMetaModel);
+        var entityValuePairs = moveStreamFactory.forEachDestinationIncludingUnassigned(variableMetaModel);
         var availableValues = moveStreamFactory.forEach(variableMetaModel.type(), false);
         return moveStreamFactory.pick(entityValuePairs)
                 .pick(availableValues,

core/src/main/java/ai/timefold/solver/core/preview/api/neighborhood/stream/MoveStreamFactory.java

@@ -7,7 +7,9 @@
 import ai.timefold.solver.core.api.domain.variable.PlanningListVariable;
 import ai.timefold.solver.core.preview.api.domain.metamodel.ElementPosition;
 import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningListVariableMetaModel;
+import ai.timefold.solver.core.preview.api.domain.metamodel.PositionInList;
 import ai.timefold.solver.core.preview.api.domain.metamodel.UnassignedElement;
+import ai.timefold.solver.core.preview.api.move.SolutionView;
 import ai.timefold.solver.core.preview.api.neighborhood.stream.enumerating.EnumeratingStream;
 import ai.timefold.solver.core.preview.api.neighborhood.stream.enumerating.UniEnumeratingStream;
 import ai.timefold.solver.core.preview.api.neighborhood.stream.function.UniNeighborhoodsPredicate;
@@ -37,6 +39,8 @@ public interface MoveStreamFactory<Solution_> {
      * This stream returns shadow entities regardless of whether they are assigned to any genuine entity.
      * They can easily be {@link UniEnumeratingStream#filter(UniNeighborhoodsPredicate) filtered out}.
      *
+     * @param sourceClass the class of the instances to enumerate
+     * @param includeNull if true, the stream will include a single null element
      * @return A stream containing a tuple for each of the entities as described above.
      * @see PlanningPin An annotation to mark the entire entity as pinned.
      * @see PlanningPinToIndex An annotation to specify only a portion of {@link PlanningListVariable} is pinned.
@@ -55,20 +59,81 @@ public interface MoveStreamFactory<Solution_> {
      */
     <A> UniEnumeratingStream<Solution_, A> forEachUnfiltered(Class<A> sourceClass, boolean includeNull);
 
+    /**
+     * Enumerate all values assigned to any entity's {@link PlanningListVariable}.
+     * Unlike {@link #forEachAssignedValue(PlanningListVariableMetaModel)}, this will include pinned values.
+     * You can use {@link SolutionView#getPositionOf(PlanningListVariableMetaModel, Object)}
+     * later downstream to get the position of the value in an entity's list variable, if needed.
+     *
+     * @param variableMetaModel the meta model of the list variable to enumerate
+     * @return enumerating stream with all values as defined above
+     * @see PlanningPin An annotation to mark the entire entity as pinned.
+     * @see PlanningPinToIndex An annotation to specify only a portion of {@link PlanningListVariable} is pinned.
+     */
+    <Entity_, Value_> UniEnumeratingStream<Solution_, Value_>
+            forEachAssignedValueUnfiltered(PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel);
+
+    /**
+     * Enumerate all values assigned to any entity's {@link PlanningListVariable}.
+     * This will not include any pinned positions or fully pinned entities.
+     * You can use {@link SolutionView#getPositionOf(PlanningListVariableMetaModel, Object)}
+     * later downstream to get the position of the value in an entity's list variable, if needed.
+     *
+     * @param variableMetaModel the meta model of the list variable to enumerate
+     * @return enumerating stream with all values as defined above
+     * @see PlanningPin An annotation to mark the entire entity as pinned.
+     * @see PlanningPinToIndex An annotation to specify only a portion of {@link PlanningListVariable} is pinned.
+     */
+    <Entity_, Value_> UniEnumeratingStream<Solution_, Value_>
+            forEachAssignedValue(PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel);
+
+    /**
+     * Enumerate all possible positions of a list variable to which a value can be assigned.
+     * This will include one position past the current end of the list, allowing for assigning at the end of a list.
+     * It will not include any pinned positions or fully pinned entities, as well as {@link UnassignedElement}.
+     * To include {@link UnassignedElement},
+     * use {@link #forEachDestinationIncludingUnassigned(PlanningListVariableMetaModel)} instead.
+     *
+     * @param variableMetaModel the meta model of the list variable to enumerate
+     * @return enumerating stream with positions as defined above
+     * @see ElementPosition Read more about element positions.
+     * @see PlanningPin An annotation to mark the entire entity as pinned.
+     * @see PlanningPinToIndex An annotation to specify only a portion of {@link PlanningListVariable} is pinned.
+     */
+    <Entity_, Value_> UniEnumeratingStream<Solution_, PositionInList>
+            forEachDestination(PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel);
+
+    /**
+     * As defined by {@link #forEachDestination(PlanningListVariableMetaModel)},
+     * but also includes a single {@link UnassignedElement} position
+     * if the list variable allows unassigned values.
+     * If the list variable does not allow unassigned values,
+     * then this method behaves exactly the same as {@link #forEachDestination(PlanningListVariableMetaModel)}.
+     */
+    <Entity_, Value_> UniEnumeratingStream<Solution_, ElementPosition>
+            forEachDestinationIncludingUnassigned(PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel);
+
+    <A> UniSamplingStream<Solution_, A> pick(UniEnumeratingStream<Solution_, A> enumeratingStream);
+
     /**
      * Enumerate all possible positions of a list variable to which a value can be assigned.
      * This will eliminate all positions on {@link PlanningPin pinned entities},
      * as well as all {@link PlanningPinToIndex pinned indexes}.
      * If the list variable {@link PlanningListVariable#allowsUnassignedValues() allows unassigned values},
      * the resulting stream will include a single instance of {@link UnassignedElement} instance.
+     * <p>
+     * <strong>Will be removed right before this API is moved out of preview.</strong>
      *
      * @param variableMetaModel the meta model of the list variable to enumerate
      * @return enumerating stream with all assignable positions of a given list variable
      * @see ElementPosition Read more about element positions.
+     * @deprecated Use {@link #forEachDestinationIncludingUnassigned(PlanningListVariableMetaModel)} instead,
+     *             or see if {@link #forEachDestination(PlanningListVariableMetaModel)}
+     *             or {@link #forEachAssignedValue(PlanningListVariableMetaModel)}
+     *             fits your needs better.
      */
+    @Deprecated(forRemoval = true)
     <Entity_, Value_> UniEnumeratingStream<Solution_, ElementPosition>
             forEachAssignablePosition(PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel);
 
-    <A> UniSamplingStream<Solution_, A> pick(UniEnumeratingStream<Solution_, A> enumeratingStream);
-
 }