Skip to content

feat: support sourcing declarative shadow variables from planning list variable elements - #2506

Merged
triceo merged 6 commits into
TimefoldAI:mainfrom
fodzal:feat/list-element-shadow-sources
Jul 27, 2026
Merged

feat: support sourcing declarative shadow variables from planning list variable elements#2506
triceo merged 6 commits into
TimefoldAI:mainfrom
fodzal:feat/list-element-shadow-sources

Conversation

@fodzal

@fodzal fodzal commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

What

Follow-up to #2469. An entity declaring a @PlanningListVariable can now use a
@ShadowSources path such as "visits[].endTime" to depend on a declarative shadow
variable of the list variable's elements:

@PlanningListVariable
List<Visit> visits;

@ShadowVariable(supplierName = "endTimeSupplier")
LocalDateTime endTime;

@ShadowSources("visits[].endTime")
LocalDateTime endTimeSupplier() {
    return visits.isEmpty() ? departureTime : visits.getLast().getEndTime();
}

This is the capability discussed in the #2469 review: accessing the declarative shadow
variables of entities inside the @PlanningListVariable, with the edges of the graph
created and updated as the list variable changes. The fail-fast added by #2469 now points
users to this syntax.

How

  • RootVariableSource accepts a [] segment on a genuine list variable declared on the
    root entity, producing the new ParentVariableType.LIST_ELEMENT. The path must end on a
    declarative shadow variable of the element (optionally reached via a fact).
  • Fan-in edges (element.shadowVariable -> entity.shadowVariable) are created from the
    list variable's contents when the graph is built. They are registered as initial dynamic
    edges, not fixed edges, so a dependency loop through them is treated as breakable by the
    solver instead of failing fast.
  • before/afterListVariableChanged events are forwarded to the declarative shadow
    variable session, which incrementally removes/adds the fan-in edges for the changed
    range and always marks the target variable as changed (covering removals and
    reorderings). Models without list element sources skip this entirely.
  • Thanks to the edges, the topological order guarantees the target variable is computed
    once per move, after all its elements' shadow variables have settled.
  • GraphStructure classifies models with list element sources as arbitrary, since the
    graph's edges change during solving; the grouped-updater decision is static for the same
    reason.

Testing

  • Path parsing: valid values[].shadowVar forms and the new fail-fasts
    (non-declarative element variable, list variable reached via another member) in
    RootVariableSourceTest.
  • ListElementShadowVariableTest: SolutionManager.updateShadowVariables (including
    empty lists and mutations between calls) and two FULL_ASSERT solver runs, one from an
    uninitialized solution and one from an initialized solution (exercising the initial
    edge seeding).
  • A dedicated testdomain/shadow/list_element domain is used instead of extending
    simple_list, because adding a list element source there would reclassify it away from
    SINGLE_DIRECTIONAL_PARENT and lose that structure's coverage.
  • Full core test suite passes.

…t variable elements

An entity declaring a @PlanningListVariable can now use a @Shadowsources
path such as "values[].endTime" to declare that one of its declarative
shadow variables depends on a declarative shadow variable of the list's
elements, e.g. a vehicle-level end time aggregated from its visits.

Previously, this dependency was inexpressible: "values[].endTime"
failed fast (accesses a collection via a variable) and
"lastValue.endTime" failed fast as well (declarative accessed from
declarative). The closest available spelling, a bare list variable
source such as @Shadowsources("values"), was accepted and documented
but silently broken: it was computed once when the variable reference
graph was built and never retriggered during solving.

- RootVariableSource accepts a [] segment on a genuine list variable
  declared on the root entity, producing the new
  ParentVariableType.LIST_ELEMENT. The path must end on a declarative
  shadow variable of the element (optionally reached via a fact).
- The initial fan-in edges (element.shadow -> owner.shadow) are created
  from the list's contents when the graph is built.
- List variable change events are now forwarded to the declarative
  shadow variable session, which incrementally adds/removes the fan-in
  edges for the changed range and always marks the owner variable
  changed, so the aggregate also reacts to reorders and removals.
- GraphStructure classifies LIST_ELEMENT sources as arbitrary, since
  the graph's edges change with the list's contents.
- Bare list variable sources, e.g. @Shadowsources("values"), now fail
  fast with a message pointing to the new element syntax, and the
  mention of @PlanningListVariable as a valid source was removed from
  the @ShadowVariable javadoc, as they gave stale values that even
  FULL_ASSERT could not detect.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ange

The element's declarative shadow variable depends on a genuine basic
variable and the element has no ordering shadow variables, proving the
aggregate is retriggered through the element edge alone.

@Christopher-Chianelli Christopher-Chianelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will do my own testing of this to verify correctness. Some comments.

- Remove requiresListVariableChangeEvents(); list variable events are
  forwarded unconditionally, like basic variable events.
- Rename ListElementEdgeMaintainer to ListElementSourceLocator, and the
  related field and method accordingly.
- Rename the elements parameter to elementList.
- Remove comments that stated the obvious.
- Rewrite the incremental tests with MoveTester.
- Use forEachIncludingUnassigned/forEach in the test constraint provider.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@fodzal

fodzal commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @Christopher-Chianelli, I have done the required changes.

I have tested that on a simplified model (VRP with sequential vehicles), it works well.

@triceo

triceo commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

@fodzal As a side-effect of some other changes we made, there is now a conflict in this PR. Can you please resolve it?

We'll take a look at this afterwards and provide a final review.

Upstream TimefoldAI#2525 (refactor: remove variable listeners) reworked the shadow
variable machinery this branch builds on: VariableListenerSupport was
renamed to ShadowVariableSupport and its `notificationQueuesAreEmpty` flag
to `dirty` (with inverted polarity, so `notificationQueuesAreEmpty = false`
maps to `dirty = true`).

Manual conflict resolution in ShadowVariableSupport:
- beforeListVariableChanged: `notificationQueuesAreEmpty = false` becomes
  `dirty = true`, matching the renamed field.
- afterListVariableChanged: fold the two adjacent `shadowVariableSession`
  guards (one from upstream marking the state dirty, one from this branch
  forwarding the change) into a single guard.

Behaviour is unchanged.
@fodzal

fodzal commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@triceo Merged main (incl. #2525) with 1 minor conflit resolved. I re-verified against the new ShadowVariableSupport — it works well.

@Christopher-Chianelli Christopher-Chianelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code overall looks good, will do more testing before approval. I have written code similar to this about 1 year ago in my attempt to implement this feature, but it had score corruptions back then. But the feature I was implementing was more varied (i.e. also included Collection based @InverseShadowVariable), and it is possible we fixed a bug that was causing the original implementation to fail.

@Christopher-Chianelli Christopher-Chianelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM when green.

@triceo

triceo commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

@fodzal The final review is in. Once you resolve the comments, and once CI is green, we will merge the PR.

(The native build is currently failing, but that is likely unrelated to this PR and I won't hold you to fixing that one.)

@sonarqubecloud

Copy link
Copy Markdown

@triceo
triceo merged commit 8e1948f into TimefoldAI:main Jul 27, 2026
12 of 19 checks passed
@triceo triceo added this to the v2.5.0 milestone Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants