chore: move streams value range on entity for basic vars (#1707)

Also addresses issues with composite ranges which could possibly
duplicate elements.

Author: triceo

core/src/main/java/ai/timefold/solver/core/api/domain/solution/PlanningEntityCollectionProperty.java

@@ -7,6 +7,9 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
 import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.SortedSet;
 
 import ai.timefold.solver.core.api.domain.entity.PlanningEntity;
 import ai.timefold.solver.core.api.score.director.ScoreDirector;
@@ -16,6 +19,9 @@
  * <p>
  * Every element in the planning entity collection should have the {@link PlanningEntity} annotation.
  * Every element in the planning entity collection will be added to the {@link ScoreDirector}.
+ * <p>
+ * For solver reproducibility, the collection must have a deterministic, stable iteration order.
+ * It is recommended to use a {@link List}, {@link LinkedHashSet} or {@link SortedSet}.
  */
 @Target({ METHOD, FIELD })
 @Retention(RUNTIME)

core/src/main/java/ai/timefold/solver/core/api/domain/solution/ProblemFactCollectionProperty.java

@@ -7,6 +7,9 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
 import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.SortedSet;
 
 import ai.timefold.solver.core.api.domain.entity.PlanningEntity;
 import ai.timefold.solver.core.api.score.stream.ConstraintFactory;
@@ -21,7 +24,10 @@
  * <p>
  * Do not annotate {@link PlanningEntity planning entities} as problem facts:
  * they are automatically available as facts for {@link ConstraintFactory#forEach(Class)}.
- *
+ * <p>
+ * For solver reproducibility, the collection must have a deterministic, stable iteration order.
+ * It is recommended to use a {@link List}, {@link LinkedHashSet} or {@link SortedSet}.
+ * 
  * @see ProblemFactProperty
  */
 @Target({ METHOD, FIELD })

core/src/main/java/ai/timefold/solver/core/api/domain/valuerange/CountableValueRange.java

@@ -4,15 +4,20 @@
 
 import ai.timefold.solver.core.api.domain.variable.PlanningVariable;
 
-import org.jspecify.annotations.NonNull;
+import org.jspecify.annotations.NullMarked;
 import org.jspecify.annotations.Nullable;
 
 /**
  * A {@link ValueRange} that is ending. Therefore, it has a discrete (as in non-continuous) range.
- *
+ * <p>
+ * Don't implement this interface directly.
+ * If you can't use a collection to store the values,
+ * use {@link ValueRangeFactory} to get an instance of a {@link CountableValueRange}.
+ * 
  * @see ValueRangeFactory
  * @see ValueRange
  */
+@NullMarked
 public interface CountableValueRange<T> extends ValueRange<T> {
 
     /**
@@ -36,7 +41,6 @@ public interface CountableValueRange<T> extends ValueRange<T> {
     /**
      * Select the elements in original (natural) order.
      */
-    @NonNull
     Iterator<T> createOriginalIterator();
 
 }

core/src/main/java/ai/timefold/solver/core/api/domain/valuerange/ValueRange.java

@@ -6,27 +6,30 @@
 import java.util.Random;
 import java.util.Set;
 
+import ai.timefold.solver.core.api.domain.variable.PlanningListVariable;
 import ai.timefold.solver.core.api.domain.variable.PlanningVariable;
 
-import org.jspecify.annotations.NonNull;
+import org.jspecify.annotations.NullMarked;
 import org.jspecify.annotations.Nullable;
 
 /**
- * A ValueRange is a set of a values for a {@link PlanningVariable}.
+ * A ValueRange is a set of a values for a {@link PlanningVariable} or {@link PlanningListVariable}.
  * These values might be stored in memory as a {@link Collection} (usually a {@link List} or {@link Set}),
  * but if the values are numbers, they can also be stored in memory by their bounds
  * to use less memory and provide more opportunities.
  * <p>
- * ValueRange is stateful.
+ * ValueRange is stateless, and its contents must not depend on any planning variables.
  * Implementations must be immutable.
  * <p>
- * Prefer using {@link CountableValueRange}.
- * In a future version of Timefold Solver, uncountable value ranges will not be allowed,
- * and certain recently introduced features already do not support them.
+ * Don't implement this interface directly.
+ * If you can't use a collection to store the values,
+ * use {@link ValueRangeFactory} to get an instance of a {@link CountableValueRange}.
  *
- * @see ValueRangeFactory
  * @see CountableValueRange
+ * @see ValueRangeProvider
+ * @see ValueRangeFactory
  */
+@NullMarked
 public interface ValueRange<T> {
 
     /**
@@ -50,7 +53,6 @@ public interface ValueRange<T> {
      * @param workingRandom the {@link Random} to use when any random number is needed,
      *        so runs are reproducible.
      */
-    @NonNull
-    Iterator<T> createRandomIterator(@NonNull Random workingRandom);
+    Iterator<T> createRandomIterator(Random workingRandom);
 
 }

core/src/main/java/ai/timefold/solver/core/api/domain/valuerange/ValueRangeProvider.java

@@ -7,18 +7,50 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
 import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.SortedSet;
 
+import ai.timefold.solver.core.api.domain.entity.PlanningEntity;
+import ai.timefold.solver.core.api.domain.variable.PlanningListVariable;
 import ai.timefold.solver.core.api.domain.variable.PlanningVariable;
 import ai.timefold.solver.core.api.solver.SolverFactory;
+import ai.timefold.solver.core.api.solver.change.ProblemChange;
 
 import org.jspecify.annotations.NonNull;
 
 /**
  * Provides the planning values that can be used for a {@link PlanningVariable}.
+ * 
  * <p>
  * This is specified on a getter of a java bean property (or directly on a field)
  * which returns a {@link Collection} or {@link ValueRange}.
  * A {@link Collection} is implicitly converted to a {@link ValueRange}.
+ * For solver reproducibility, the collection must have a deterministic, stable iteration order.
+ * It is recommended to use a {@link List}, {@link LinkedHashSet} or {@link SortedSet}.
+ * 
+ * <p>
+ * Value ranges are not allowed to contain {@code null} values.
+ * When {@link PlanningVariable#allowsUnassigned()} or {@link PlanningListVariable#allowsUnassignedValues()} is true,
+ * the solver will handle {@code null} values on its own.
+ *
+ * <p>
+ * Value ranges are not allowed to contain multiple copies of the same object,
+ * as defined by {@code ==}.
+ * It is recommended that the value range never contains two objects
+ * that are equal according to {@link Object#equals(Object)},
+ * but this is not enforced to not depend on user-defined {@link Object#equals(Object)} implementations.
+ * Having duplicates in a value range can lead to unexpected behavior,
+ * and skews selection probabilities in random selection algorithms.
+ * 
+ * <p>
+ * Value ranges are not allowed to change during solving.
+ * This is especially important for value ranges defined on {@link PlanningEntity}-annotated classes;
+ * these must never depend on any of that entity's variables, or on any other entity's variables.
+ * If you need to change a value range defined on an entity,
+ * trigger a {@link ProblemChange} for that entity or restart the solver with an updated planning solution.
+ * If you need to change a value range defined on a planning solution,
+ * restart the solver with a new planning solution.
  */
 @Target({ METHOD, FIELD })
 @Retention(RUNTIME)

core/src/main/java/ai/timefold/solver/core/impl/bavet/uni/AbstractForEachUniNode.java

@@ -12,6 +12,7 @@
 import ai.timefold.solver.core.impl.domain.variable.supply.SupplyManager;
 
 import org.jspecify.annotations.NullMarked;
+import org.jspecify.annotations.Nullable;
 
 /**
  * Filtering nodes are expensive.
@@ -38,7 +39,7 @@ protected AbstractForEachUniNode(Class<A> forEachClass, TupleLifecycle<UniTuple<
         this.propagationQueue = new StaticPropagationQueue<>(nextNodesTupleLifecycle);
     }
 
-    public void insert(A a) {
+    public void insert(@Nullable A a) {
         var tuple = new UniTuple<>(a, outputStoreSize);
         var old = tupleMap.put(a, tuple);
         if (old != null) {
@@ -48,9 +49,9 @@ public void insert(A a) {
         propagationQueue.insert(tuple);
     }
 
-    public abstract void update(A a);
+    public abstract void update(@Nullable A a);
 
-    protected final void updateExisting(A a, UniTuple<A> tuple) {
+    protected final void updateExisting(@Nullable A a, UniTuple<A> tuple) {
         var state = tuple.state;
         if (state.isDirty()) {
             if (state == TupleState.DYING || state == TupleState.ABORTING) {
@@ -63,7 +64,7 @@ protected final void updateExisting(A a, UniTuple<A> tuple) {
         }
     }
 
-    public void retract(A a) {
+    public void retract(@Nullable A a) {
         var tuple = tupleMap.remove(a);
         if (tuple == null) {
             throw new IllegalStateException("The fact (%s) was never inserted, so it cannot retract."
@@ -72,7 +73,7 @@ public void retract(A a) {
         retractExisting(a, tuple);
     }
 
-    protected void retractExisting(A a, UniTuple<A> tuple) {
+    protected void retractExisting(@Nullable A a, UniTuple<A> tuple) {
         var state = tuple.state;
         if (state.isDirty()) {
             if (state == TupleState.DYING || state == TupleState.ABORTING) {

core/src/main/java/ai/timefold/solver/core/impl/bavet/uni/ForEachExcludingPinnedUniNode.java

@@ -31,8 +31,6 @@ public final class ForEachExcludingPinnedUniNode<Solution_, A>
      * 
      * @param entityMetaModel Expects a pinnable entity.
      *        Every other option should have already been exluded and passed to a different kind of node.
-     * @param nextNodesTupleLifecycle
-     * @param outputStoreSize
      */
     public ForEachExcludingPinnedUniNode(PlanningEntityMetaModel<Solution_, A> entityMetaModel,
             TupleLifecycle<UniTuple<A>> nextNodesTupleLifecycle, int outputStoreSize) {
@@ -57,15 +55,15 @@ private Predicate<A> buildFilter(Solution_ workingSolution, SupplyManager supply
     }
 
     @Override
-    public void insert(A a) {
+    public void insert(@Nullable A a) {
         if (!filter.test(a)) { // Skip inserting the tuple as it does not pass the filter.
             return;
         }
         super.insert(a);
     }
 
     @Override
-    public void update(A a) {
+    public void update(@Nullable A a) {
         var tuple = tupleMap.get(a);
         if (tuple == null) { // The tuple was never inserted because it did not pass the filter.
             insert(a);
@@ -77,7 +75,7 @@ public void update(A a) {
     }
 
     @Override
-    public void retract(A a) {
+    public void retract(@Nullable A a) {
         var tuple = tupleMap.remove(a);
         if (tuple == null) { // The tuple was never inserted because it did not pass the filter.
             return;

core/src/main/java/ai/timefold/solver/core/impl/bavet/uni/ForEachExcludingUnassignedUniNode.java

@@ -7,6 +7,7 @@
 import ai.timefold.solver.core.impl.bavet.common.tuple.UniTuple;
 
 import org.jspecify.annotations.NullMarked;
+import org.jspecify.annotations.Nullable;
 
 @NullMarked
 public final class ForEachExcludingUnassignedUniNode<A>
@@ -21,15 +22,15 @@ public ForEachExcludingUnassignedUniNode(Class<A> forEachClass, Predicate<A> fil
     }
 
     @Override
-    public void insert(A a) {
+    public void insert(@Nullable A a) {
         if (!filter.test(a)) { // Skip inserting the tuple as it does not pass the filter.
             return;
         }
         super.insert(a);
     }
 
     @Override
-    public void update(A a) {
+    public void update(@Nullable A a) {
         var tuple = tupleMap.get(a);
         if (tuple == null) { // The tuple was never inserted because it did not pass the filter.
             insert(a);
@@ -41,7 +42,7 @@ public void update(A a) {
     }
 
     @Override
-    public void retract(A a) {
+    public void retract(@Nullable A a) {
         var tuple = tupleMap.remove(a);
         if (tuple == null) { // The tuple was never inserted because it did not pass the filter.
             return;

core/src/main/java/ai/timefold/solver/core/impl/bavet/uni/ForEachFromSolutionUniNode.java

@@ -1,11 +1,15 @@
 package ai.timefold.solver.core.impl.bavet.uni;
 
+import java.util.Objects;
+
 import ai.timefold.solver.core.impl.bavet.common.tuple.TupleLifecycle;
 import ai.timefold.solver.core.impl.bavet.common.tuple.UniTuple;
+import ai.timefold.solver.core.impl.domain.valuerange.descriptor.ValueRangeDescriptor;
 import ai.timefold.solver.core.impl.domain.variable.supply.SupplyManager;
-import ai.timefold.solver.core.impl.move.streams.FromSolutionValueCollectingFunction;
+import ai.timefold.solver.core.impl.score.director.ValueRangeManager;
 
 import org.jspecify.annotations.NullMarked;
+import org.jspecify.annotations.Nullable;
 
 /**
  * Node that reads a property from a planning solution.
@@ -27,14 +31,19 @@ public final class ForEachFromSolutionUniNode<Solution_, A>
         extends ForEachIncludingUnassignedUniNode<A>
         implements AbstractForEachUniNode.InitializableForEachNode<Solution_> {
 
-    private final FromSolutionValueCollectingFunction<Solution_, A> valueCollectingFunction;
+    private final ValueRangeManager<Solution_> valueRangeManager;
+    private final ValueRangeDescriptor<Solution_> valueRangeDescriptor;
 
     private boolean isInitialized = false;
 
-    public ForEachFromSolutionUniNode(FromSolutionValueCollectingFunction<Solution_, A> valueCollectingFunction,
-            TupleLifecycle<UniTuple<A>> nextNodesTupleLifecycle, int outputStoreSize) {
-        super(valueCollectingFunction.declaredClass(), nextNodesTupleLifecycle, outputStoreSize);
-        this.valueCollectingFunction = valueCollectingFunction;
+    @SuppressWarnings("unchecked")
+    public ForEachFromSolutionUniNode(ValueRangeManager<Solution_> valueRangeManager,
+            ValueRangeDescriptor<Solution_> valueRangeDescriptor, TupleLifecycle<UniTuple<A>> nextNodesTupleLifecycle,
+            int outputStoreSize) {
+        super((Class<A>) valueRangeDescriptor.getVariableDescriptor().getVariablePropertyType(), nextNodesTupleLifecycle,
+                outputStoreSize);
+        this.valueRangeManager = Objects.requireNonNull(valueRangeManager);
+        this.valueRangeDescriptor = Objects.requireNonNull(valueRangeDescriptor);
     }
 
     @Override
@@ -44,7 +53,7 @@ public void initialize(Solution_ workingSolution, SupplyManager supplyManager) {
                     .formatted(this));
         } else {
             this.isInitialized = true;
-            var valueRange = valueCollectingFunction.apply(workingSolution);
+            var valueRange = valueRangeManager.<A> getFromSolution(valueRangeDescriptor, workingSolution);
             var valueIterator = valueRange.createOriginalIterator();
             while (valueIterator.hasNext()) {
                 var value = valueIterator.next();
@@ -54,13 +63,13 @@ public void initialize(Solution_ workingSolution, SupplyManager supplyManager) {
     }
 
     @Override
-    public void insert(A a) {
+    public void insert(@Nullable A a) {
         throw new UnsupportedOperationException("Impossible state: direct insert is not supported on %s."
                 .formatted(this));
     }
 
     @Override
-    public void retract(A a) {
+    public void retract(@Nullable A a) {
         throw new UnsupportedOperationException("Impossible state: direct retract is not supported on %s."
                 .formatted(this));
     }

core/src/main/java/ai/timefold/solver/core/impl/bavet/uni/ForEachIncludingUnassignedUniNode.java

@@ -4,6 +4,7 @@
 import ai.timefold.solver.core.impl.bavet.common.tuple.UniTuple;
 
 import org.jspecify.annotations.NullMarked;
+import org.jspecify.annotations.Nullable;
 
 @NullMarked
 public sealed class ForEachIncludingUnassignedUniNode<A>
@@ -16,7 +17,7 @@ public ForEachIncludingUnassignedUniNode(Class<A> forEachClass, TupleLifecycle<U
     }
 
     @Override
-    public void update(A a) {
+    public void update(@Nullable A a) {
         var tuple = tupleMap.get(a);
         if (tuple == null) {
             throw new IllegalStateException("The fact (%s) was never inserted, so it cannot update."