From 156e51c7c3d04a0ac14f739992a3322e35b3591f Mon Sep 17 00:00:00 2001 From: fodzal Date: Mon, 6 Jul 2026 11:57:22 +0200 Subject: [PATCH 1/5] fix: retrigger declarative shadow variables sourced from a planning list variable A declarative shadow variable using a genuine planning list variable as a source, e.g. @ShadowSources("values"), was computed once when the variable reference graph was built and then never retriggered during solving: list variable change events were only delivered to variable listeners, never to the declarative shadow variable session, so the graph processor registered for the list variable never fired. The shadow variable silently kept its stale value, and the staleness was invisible even to FULL_ASSERT, whose forced re-trigger goes through the same list variable events. Forward before/afterListVariableChanged to the declarative shadow variable session, like basic variable changes already are. Co-Authored-By: Claude Fable 5 --- .../support/VariableListenerSupport.java | 9 ++ .../BareListVariableSourceStalenessTest.java | 127 ++++++++++++++++++ 2 files changed, 136 insertions(+) create mode 100644 core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/BareListVariableSourceStalenessTest.java diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java index b6b998e4c04..dc16b7f8200 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java @@ -311,6 +311,11 @@ public void beforeListVariableChanged(ListVariableDescriptor variable } notificationQueuesAreEmpty = false; } + if (shadowVariableSession != null) { + // Declarative shadow variables may use the list variable itself as a source. + shadowVariableSession.beforeVariableChanged(variableDescriptor, entity); + notificationQueuesAreEmpty = false; + } } public void afterListVariableChanged(ListVariableDescriptor variableDescriptor, Object entity, int fromIndex, @@ -326,6 +331,10 @@ public void afterListVariableChanged(ListVariableDescriptor variableD if (!cascadingUpdateShadowVarDescriptorList.isEmpty()) { // Only necessary if there is a cascade. listVariableChangedNotificationList.add(notification); } + if (shadowVariableSession != null) { + // Declarative shadow variables may use the list variable itself as a source. + shadowVariableSession.afterVariableChanged(variableDescriptor, entity); + } } public InnerScoreDirector getScoreDirector() { diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/BareListVariableSourceStalenessTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/BareListVariableSourceStalenessTest.java new file mode 100644 index 00000000000..52c446268c2 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/BareListVariableSourceStalenessTest.java @@ -0,0 +1,127 @@ +package ai.timefold.solver.core.impl.domain.variable.declarative; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.ArrayList; +import java.util.List; + +import ai.timefold.solver.core.api.domain.common.PlanningId; +import ai.timefold.solver.core.api.domain.entity.PlanningEntity; +import ai.timefold.solver.core.api.domain.solution.PlanningEntityCollectionProperty; +import ai.timefold.solver.core.api.domain.solution.PlanningScore; +import ai.timefold.solver.core.api.domain.solution.PlanningSolution; +import ai.timefold.solver.core.api.domain.solution.ProblemFactCollectionProperty; +import ai.timefold.solver.core.api.domain.valuerange.ValueRangeProvider; +import ai.timefold.solver.core.api.domain.variable.PlanningListVariable; +import ai.timefold.solver.core.api.domain.variable.ShadowSources; +import ai.timefold.solver.core.api.domain.variable.ShadowVariable; +import ai.timefold.solver.core.api.score.SimpleScore; +import ai.timefold.solver.core.api.score.calculator.EasyScoreCalculator; +import ai.timefold.solver.core.api.solver.SolverFactory; +import ai.timefold.solver.core.config.score.director.ScoreDirectorFactoryConfig; +import ai.timefold.solver.core.config.solver.EnvironmentMode; +import ai.timefold.solver.core.config.solver.SolverConfig; +import ai.timefold.solver.core.config.solver.termination.TerminationConfig; + +import org.jspecify.annotations.NonNull; +import org.junit.jupiter.api.Test; + +/** + * Demonstrates whether a declarative shadow variable with a bare planning list variable + * source, i.e. {@code @ShadowSources("values")}, is retriggered when the list variable + * changes during solving. + * The javadoc of {@code @ShadowVariable} documents a genuine {@code @PlanningListVariable} + * as a valid source. + */ +class BareListVariableSourceStalenessTest { + + @Test + void bareListVariableSourceIsRetriggeredDuringSolving() { + var problem = new Solution(); + problem.owners = List.of(new Owner("o1"), new Owner("o2")); + problem.vals = List.of(new Val("v1"), new Val("v2"), new Val("v3")); + + var solverConfig = new SolverConfig() + .withEnvironmentMode(EnvironmentMode.PHASE_ASSERT) + .withSolutionClass(Solution.class) + .withEntityClasses(Owner.class) + .withScoreDirectorFactory(new ScoreDirectorFactoryConfig() + .withEasyScoreCalculatorClass(AssertingCalculator.class)) + .withTerminationConfig(new TerminationConfig().withMoveCountLimit(100L)); + + var solution = SolverFactory. create(solverConfig).buildSolver().solve(problem); + + assertThat(solution).isNotNull(); + for (var owner : solution.owners) { + assertThat(owner.valueCount).isEqualTo(owner.values.size()); + } + } + + public static class AssertingCalculator implements EasyScoreCalculator { + @Override + public @NonNull SimpleScore calculateScore(@NonNull Solution solution) { + var total = 0; + for (var owner : solution.owners) { + if (owner.valueCount == null || owner.valueCount != owner.values.size()) { + throw new IllegalStateException( + "Stale shadow: owner (%s) has valueCount (%s) but its list has size (%d)." + .formatted(owner.id, owner.valueCount, owner.values.size())); + } + total += owner.valueCount; + } + return SimpleScore.of(total); + } + } + + @PlanningSolution + public static class Solution { + @PlanningEntityCollectionProperty + public List owners; + @ProblemFactCollectionProperty + @ValueRangeProvider + public List vals; + @PlanningScore + public SimpleScore score; + } + + @PlanningEntity + public static class Owner { + @PlanningId + public String id; + + @PlanningListVariable + public List values = new ArrayList<>(); + + @ShadowVariable(supplierName = "countSupplier") + public Integer valueCount; + + public Owner() { + } + + public Owner(String id) { + this.id = id; + } + + @ShadowSources("values") + public Integer countSupplier() { + return values.size(); + } + } + + public static class Val { + @PlanningId + public String id; + + public Val() { + } + + public Val(String id) { + this.id = id; + } + + @Override + public String toString() { + return id; + } + } +} From e1c4edd3415ce9f61a17f7884ad5ce313aebbd5a Mon Sep 17 00:00:00 2001 From: fodzal Date: Tue, 7 Jul 2026 16:21:28 +0200 Subject: [PATCH 2/5] fix: fail fast on planning list variable sources instead of retriggering them As suggested in review, retriggering a bare planning list variable source only makes suppliers reading facts or the list's structure correct; a supplier reading a declarative shadow variable of an element (e.g. visits.getLast().getEndTime()) would still be computed without graph edges, so without topological ordering, and would not be retriggered when the element's shadow variable changes without a list change. Most users would expect exactly that to work, so retriggering would trade a total freeze for intermittent corruption. Instead, fail fast when a @ShadowSources path accesses a genuine planning list variable, remove the mention of @PlanningListVariable as a valid source from the @ShadowVariable javadoc, and document the change in the migration guide. Co-Authored-By: Claude Fable 5 --- .../api/domain/variable/ShadowSources.java | 3 +- .../api/domain/variable/ShadowVariable.java | 6 +- .../declarative/RootVariableSource.java | 11 ++ .../support/VariableListenerSupport.java | 9 -- .../BareListVariableSourceStalenessTest.java | 127 ------------------ .../declarative/RootVariableSourceTest.java | 16 +++ .../TestdataInvalidDeclarativeEntity.java | 14 +- .../modeling-planning-problems.adoc | 2 +- 8 files changed, 46 insertions(+), 142 deletions(-) delete mode 100644 core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/BareListVariableSourceStalenessTest.java diff --git a/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowSources.java b/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowSources.java index 91628d00576..2f6b2687813 100644 --- a/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowSources.java +++ b/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowSources.java @@ -20,7 +20,8 @@ * *
    *
  • - * "variableName", for referring any variable on the same planning entity. + * "variableName", for referring any variable on the same planning entity, + * except a {@link PlanningListVariable}, which cannot be used as a source. *
    • *
  • * A list of names seperated by ".", such as "variableOrFact.fact.entity.variable", diff --git a/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowVariable.java b/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowVariable.java index a2fa0e0b09f..a754b0a2c93 100644 --- a/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowVariable.java +++ b/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowVariable.java @@ -11,8 +11,10 @@ /** * Specifies that a bean property (or a field) is a custom shadow variable of 1 or more source variables. - * The source variable may be a genuine {@link PlanningVariable}, {@link PlanningListVariable}, - * or another shadow variable. + * The source variable may be a genuine {@link PlanningVariable} or another shadow variable. + * A genuine {@link PlanningListVariable} cannot be a source, + * since the supplier would not be updated when the list variable + * or the shadow variables of its elements change. *

    * It is specified on a getter of a java bean property (or a field) of a {@link PlanningEntity} class. */ diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java index de226d1cdb7..d6449060538 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java @@ -10,6 +10,7 @@ import ai.timefold.solver.core.api.domain.variable.InverseRelationShadowVariable; import ai.timefold.solver.core.api.domain.variable.NextElementShadowVariable; +import ai.timefold.solver.core.api.domain.variable.PlanningListVariable; import ai.timefold.solver.core.api.domain.variable.PlanningVariable; import ai.timefold.solver.core.api.domain.variable.PreviousElementShadowVariable; import ai.timefold.solver.core.api.domain.variable.ShadowSources; @@ -126,6 +127,16 @@ public static RootVariableSource from( } var isVariable = isVariable(solutionMetaModel, memberAccessor.getDeclaringClass(), pathPart.name()); + if (isVariable + && getAnnotation(memberAccessor.getDeclaringClass(), pathPart.name(), + PlanningListVariable.class) != null) { + throw new IllegalArgumentException( + """ + The source path (%s) starting from root class (%s) accesses a planning list variable (%s), which is not supported. + A planning list variable cannot be a source, since its supplier would not be updated when the list variable or the shadow variables of its elements change, + and would therefore compute a stale value.""" + .formatted(variablePath, rootEntityClass.getSimpleName(), pathPart.name())); + } chainToVariable.add(memberAccessor); for (var chain : chainStartingFromSourceVariableList) { chain.add(memberAccessor); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java index dc16b7f8200..b6b998e4c04 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java @@ -311,11 +311,6 @@ public void beforeListVariableChanged(ListVariableDescriptor variable } notificationQueuesAreEmpty = false; } - if (shadowVariableSession != null) { - // Declarative shadow variables may use the list variable itself as a source. - shadowVariableSession.beforeVariableChanged(variableDescriptor, entity); - notificationQueuesAreEmpty = false; - } } public void afterListVariableChanged(ListVariableDescriptor variableDescriptor, Object entity, int fromIndex, @@ -331,10 +326,6 @@ public void afterListVariableChanged(ListVariableDescriptor variableD if (!cascadingUpdateShadowVarDescriptorList.isEmpty()) { // Only necessary if there is a cascade. listVariableChangedNotificationList.add(notification); } - if (shadowVariableSession != null) { - // Declarative shadow variables may use the list variable itself as a source. - shadowVariableSession.afterVariableChanged(variableDescriptor, entity); - } } public InnerScoreDirector getScoreDirector() { diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/BareListVariableSourceStalenessTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/BareListVariableSourceStalenessTest.java deleted file mode 100644 index 52c446268c2..00000000000 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/BareListVariableSourceStalenessTest.java +++ /dev/null @@ -1,127 +0,0 @@ -package ai.timefold.solver.core.impl.domain.variable.declarative; - -import static org.assertj.core.api.Assertions.assertThat; - -import java.util.ArrayList; -import java.util.List; - -import ai.timefold.solver.core.api.domain.common.PlanningId; -import ai.timefold.solver.core.api.domain.entity.PlanningEntity; -import ai.timefold.solver.core.api.domain.solution.PlanningEntityCollectionProperty; -import ai.timefold.solver.core.api.domain.solution.PlanningScore; -import ai.timefold.solver.core.api.domain.solution.PlanningSolution; -import ai.timefold.solver.core.api.domain.solution.ProblemFactCollectionProperty; -import ai.timefold.solver.core.api.domain.valuerange.ValueRangeProvider; -import ai.timefold.solver.core.api.domain.variable.PlanningListVariable; -import ai.timefold.solver.core.api.domain.variable.ShadowSources; -import ai.timefold.solver.core.api.domain.variable.ShadowVariable; -import ai.timefold.solver.core.api.score.SimpleScore; -import ai.timefold.solver.core.api.score.calculator.EasyScoreCalculator; -import ai.timefold.solver.core.api.solver.SolverFactory; -import ai.timefold.solver.core.config.score.director.ScoreDirectorFactoryConfig; -import ai.timefold.solver.core.config.solver.EnvironmentMode; -import ai.timefold.solver.core.config.solver.SolverConfig; -import ai.timefold.solver.core.config.solver.termination.TerminationConfig; - -import org.jspecify.annotations.NonNull; -import org.junit.jupiter.api.Test; - -/** - * Demonstrates whether a declarative shadow variable with a bare planning list variable - * source, i.e. {@code @ShadowSources("values")}, is retriggered when the list variable - * changes during solving. - * The javadoc of {@code @ShadowVariable} documents a genuine {@code @PlanningListVariable} - * as a valid source. - */ -class BareListVariableSourceStalenessTest { - - @Test - void bareListVariableSourceIsRetriggeredDuringSolving() { - var problem = new Solution(); - problem.owners = List.of(new Owner("o1"), new Owner("o2")); - problem.vals = List.of(new Val("v1"), new Val("v2"), new Val("v3")); - - var solverConfig = new SolverConfig() - .withEnvironmentMode(EnvironmentMode.PHASE_ASSERT) - .withSolutionClass(Solution.class) - .withEntityClasses(Owner.class) - .withScoreDirectorFactory(new ScoreDirectorFactoryConfig() - .withEasyScoreCalculatorClass(AssertingCalculator.class)) - .withTerminationConfig(new TerminationConfig().withMoveCountLimit(100L)); - - var solution = SolverFactory. create(solverConfig).buildSolver().solve(problem); - - assertThat(solution).isNotNull(); - for (var owner : solution.owners) { - assertThat(owner.valueCount).isEqualTo(owner.values.size()); - } - } - - public static class AssertingCalculator implements EasyScoreCalculator { - @Override - public @NonNull SimpleScore calculateScore(@NonNull Solution solution) { - var total = 0; - for (var owner : solution.owners) { - if (owner.valueCount == null || owner.valueCount != owner.values.size()) { - throw new IllegalStateException( - "Stale shadow: owner (%s) has valueCount (%s) but its list has size (%d)." - .formatted(owner.id, owner.valueCount, owner.values.size())); - } - total += owner.valueCount; - } - return SimpleScore.of(total); - } - } - - @PlanningSolution - public static class Solution { - @PlanningEntityCollectionProperty - public List owners; - @ProblemFactCollectionProperty - @ValueRangeProvider - public List vals; - @PlanningScore - public SimpleScore score; - } - - @PlanningEntity - public static class Owner { - @PlanningId - public String id; - - @PlanningListVariable - public List values = new ArrayList<>(); - - @ShadowVariable(supplierName = "countSupplier") - public Integer valueCount; - - public Owner() { - } - - public Owner(String id) { - this.id = id; - } - - @ShadowSources("values") - public Integer countSupplier() { - return values.size(); - } - } - - public static class Val { - @PlanningId - public String id; - - public Val() { - } - - public Val(String id) { - this.id = id; - } - - @Override - public String toString() { - return id; - } - } -} diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java index 0c64370ecc9..b3ac868ac30 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java @@ -548,6 +548,22 @@ void invalidPathUsingGroupAfterGroup() { " after another collection (group), which is not allowed."); } + @Test + void invalidPathUsingBareListVariable() { + assertThatCode(() -> RootVariableSource.from( + planningSolutionMetaModel, + TestdataInvalidDeclarativeEntity.class, + "shadow", + "values", + DEFAULT_MEMBER_ACCESSOR_FACTORY, + DEFAULT_DESCRIPTOR_POLICY)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContainingAll( + "The source path (values) starting from root class (TestdataInvalidDeclarativeEntity)" + + " accesses a planning list variable (values), which is not supported.", + "A planning list variable cannot be a source"); + } + @Test void invalidPathUsingGroupAfterVariable() { assertThatCode(() -> RootVariableSource.from( diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/shadow/invalid/TestdataInvalidDeclarativeEntity.java b/core/src/test/java/ai/timefold/solver/core/testdomain/shadow/invalid/TestdataInvalidDeclarativeEntity.java index 647abb9152a..d8f36caae75 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/shadow/invalid/TestdataInvalidDeclarativeEntity.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/shadow/invalid/TestdataInvalidDeclarativeEntity.java @@ -10,6 +10,8 @@ @PlanningEntity public class TestdataInvalidDeclarativeEntity extends TestdataObject { + TestdataInvalidDeclarativeEntity fact; + @PlanningListVariable List values; @@ -23,9 +25,17 @@ public TestdataInvalidDeclarativeEntity(String code) { super(code); } - @ShadowSources("values") + @ShadowSources("fact.shadow") public Integer shadowSupplier() { - return values.size(); + return fact == null ? 0 : fact.getShadow(); + } + + public TestdataInvalidDeclarativeEntity getFact() { + return fact; + } + + public void setFact(TestdataInvalidDeclarativeEntity fact) { + this.fact = fact; } public List getValues() { diff --git a/docs/src/modules/ROOT/pages/domain-modeling/modeling-planning-problems.adoc b/docs/src/modules/ROOT/pages/domain-modeling/modeling-planning-problems.adoc index d79b4689052..ac478029239 100644 --- a/docs/src/modules/ROOT/pages/domain-modeling/modeling-planning-problems.adoc +++ b/docs/src/modules/ROOT/pages/domain-modeling/modeling-planning-problems.adoc @@ -1800,7 +1800,7 @@ These paths must follow 1 of 3 syntactic forms: |Simple Variable Name |"variableName" -|Refers to a variable (genuine or shadow) on the same planning entity. +|Refers to a variable (genuine or shadow) on the same planning entity. A planning list variable cannot be a source and fails fast, since the supplier would not be updated when the list variable or the shadow variables of its elements change. |Chained Property Path |"a.b.c" From 65e7bf6583eb5d2c34940df0384612a83c2018ac Mon Sep 17 00:00:00 2001 From: fodzal Date: Wed, 8 Jul 2026 11:54:31 +0200 Subject: [PATCH 3/5] fix: align the fail-fast error message with existing conventions Use the "which is not allowed" ending, the "Maybe ...?" suffix, and the "the shadow variable would not be updated" phrasing, consistently across the error message, the Javadoc and the reference documentation. Co-Authored-By: Claude Fable 5 --- .../solver/core/api/domain/variable/ShadowVariable.java | 2 +- .../domain/variable/declarative/RootVariableSource.java | 9 +++++---- .../variable/declarative/RootVariableSourceTest.java | 6 ++++-- .../domain-modeling/modeling-planning-problems.adoc | 2 +- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowVariable.java b/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowVariable.java index a754b0a2c93..30cb6876578 100644 --- a/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowVariable.java +++ b/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowVariable.java @@ -13,7 +13,7 @@ * Specifies that a bean property (or a field) is a custom shadow variable of 1 or more source variables. * The source variable may be a genuine {@link PlanningVariable} or another shadow variable. * A genuine {@link PlanningListVariable} cannot be a source, - * since the supplier would not be updated when the list variable + * since the shadow variable would not be updated when the list variable * or the shadow variables of its elements change. *

    * It is specified on a getter of a java bean property (or a field) of a {@link PlanningEntity} class. diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java index d6449060538..ebe149e5e2e 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java @@ -132,10 +132,11 @@ && getAnnotation(memberAccessor.getDeclaringClass(), pathPart.name(), PlanningListVariable.class) != null) { throw new IllegalArgumentException( """ - The source path (%s) starting from root class (%s) accesses a planning list variable (%s), which is not supported. - A planning list variable cannot be a source, since its supplier would not be updated when the list variable or the shadow variables of its elements change, - and would therefore compute a stale value.""" - .formatted(variablePath, rootEntityClass.getSimpleName(), pathPart.name())); + The source path (%s) starting from root class (%s) accesses a planning list variable (%s), which is not allowed. + The shadow variable would not be updated when the list variable or the shadow variables of its elements change. + Maybe remove the source path (%s) from the @%s?""" + .formatted(variablePath, rootEntityClass.getSimpleName(), pathPart.name(), + variablePath, ShadowSources.class.getSimpleName())); } chainToVariable.add(memberAccessor); for (var chain : chainStartingFromSourceVariableList) { diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java index b3ac868ac30..e60d6fc46a1 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java @@ -560,8 +560,10 @@ void invalidPathUsingBareListVariable() { .isInstanceOf(IllegalArgumentException.class) .hasMessageContainingAll( "The source path (values) starting from root class (TestdataInvalidDeclarativeEntity)" - + " accesses a planning list variable (values), which is not supported.", - "A planning list variable cannot be a source"); + + " accesses a planning list variable (values), which is not allowed.", + "The shadow variable would not be updated when the list variable" + + " or the shadow variables of its elements change.", + "Maybe remove the source path (values) from the @ShadowSources?"); } @Test diff --git a/docs/src/modules/ROOT/pages/domain-modeling/modeling-planning-problems.adoc b/docs/src/modules/ROOT/pages/domain-modeling/modeling-planning-problems.adoc index ac478029239..382628de9d0 100644 --- a/docs/src/modules/ROOT/pages/domain-modeling/modeling-planning-problems.adoc +++ b/docs/src/modules/ROOT/pages/domain-modeling/modeling-planning-problems.adoc @@ -1800,7 +1800,7 @@ These paths must follow 1 of 3 syntactic forms: |Simple Variable Name |"variableName" -|Refers to a variable (genuine or shadow) on the same planning entity. A planning list variable cannot be a source and fails fast, since the supplier would not be updated when the list variable or the shadow variables of its elements change. +|Refers to a variable (genuine or shadow) on the same planning entity. A planning list variable cannot be a source and fails fast, since the shadow variable would not be updated when the list variable or the shadow variables of its elements change. |Chained Property Path |"a.b.c" From a16f4ef934ef606772b93e6541c2ff2fda4bf338 Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Mon, 13 Jul 2026 10:25:43 -0400 Subject: [PATCH 4/5] chore: apply suggestions from code review Co-authored-by: Christopher Chianelli --- .../core/api/domain/variable/ShadowVariable.java | 4 +--- .../variable/declarative/RootVariableSource.java | 1 - .../variable/declarative/RootVariableSourceTest.java | 10 ++++++---- .../domain-modeling/modeling-planning-problems.adoc | 2 +- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowVariable.java b/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowVariable.java index 30cb6876578..4ac06c9b5a9 100644 --- a/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowVariable.java +++ b/core/src/main/java/ai/timefold/solver/core/api/domain/variable/ShadowVariable.java @@ -12,9 +12,7 @@ /** * Specifies that a bean property (or a field) is a custom shadow variable of 1 or more source variables. * The source variable may be a genuine {@link PlanningVariable} or another shadow variable. - * A genuine {@link PlanningListVariable} cannot be a source, - * since the shadow variable would not be updated when the list variable - * or the shadow variables of its elements change. + * A genuine {@link PlanningListVariable} cannot be a source. *

    * It is specified on a getter of a java bean property (or a field) of a {@link PlanningEntity} class. */ diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java index ebe149e5e2e..9eaa2477f41 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java @@ -133,7 +133,6 @@ && getAnnotation(memberAccessor.getDeclaringClass(), pathPart.name(), throw new IllegalArgumentException( """ The source path (%s) starting from root class (%s) accesses a planning list variable (%s), which is not allowed. - The shadow variable would not be updated when the list variable or the shadow variables of its elements change. Maybe remove the source path (%s) from the @%s?""" .formatted(variablePath, rootEntityClass.getSimpleName(), pathPart.name(), variablePath, ShadowSources.class.getSimpleName())); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java index e60d6fc46a1..c02359fe71c 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java @@ -559,10 +559,12 @@ void invalidPathUsingBareListVariable() { DEFAULT_DESCRIPTOR_POLICY)) .isInstanceOf(IllegalArgumentException.class) .hasMessageContainingAll( - "The source path (values) starting from root class (TestdataInvalidDeclarativeEntity)" - + " accesses a planning list variable (values), which is not allowed.", - "The shadow variable would not be updated when the list variable" - + " or the shadow variables of its elements change.", + """ + The source path (values) starting from root class (TestdataInvalidDeclarativeEntity) \ + accesses a planning list variable (values), which is not allowed.""", + """ + The shadow variable would not be updated when the list variable \ + or the shadow variables of its elements change.""", "Maybe remove the source path (values) from the @ShadowSources?"); } diff --git a/docs/src/modules/ROOT/pages/domain-modeling/modeling-planning-problems.adoc b/docs/src/modules/ROOT/pages/domain-modeling/modeling-planning-problems.adoc index 382628de9d0..adc7ff8c69b 100644 --- a/docs/src/modules/ROOT/pages/domain-modeling/modeling-planning-problems.adoc +++ b/docs/src/modules/ROOT/pages/domain-modeling/modeling-planning-problems.adoc @@ -1800,7 +1800,7 @@ These paths must follow 1 of 3 syntactic forms: |Simple Variable Name |"variableName" -|Refers to a variable (genuine or shadow) on the same planning entity. A planning list variable cannot be a source and fails fast, since the shadow variable would not be updated when the list variable or the shadow variables of its elements change. +|Refers to a variable (genuine or shadow) on the same planning entity. A planning list variable cannot be used as a source. |Chained Property Path |"a.b.c" From a14a1bf81b47b41df55a40658770a6dad4c09cf0 Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Mon, 13 Jul 2026 10:41:47 -0400 Subject: [PATCH 5/5] fix: formatting and delete removed message from test --- .../variable/declarative/RootVariableSourceTest.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java index c02359fe71c..5379ad53114 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSourceTest.java @@ -560,11 +560,8 @@ void invalidPathUsingBareListVariable() { .isInstanceOf(IllegalArgumentException.class) .hasMessageContainingAll( """ - The source path (values) starting from root class (TestdataInvalidDeclarativeEntity) \ - accesses a planning list variable (values), which is not allowed.""", - """ - The shadow variable would not be updated when the list variable \ - or the shadow variables of its elements change.""", + The source path (values) starting from root class (TestdataInvalidDeclarativeEntity) \ + accesses a planning list variable (values), which is not allowed.""", "Maybe remove the source path (values) from the @ShadowSources?"); }