Skip to content

chore: make vehicle routing calculation more incremental - #1218

Open
TomCools wants to merge 3 commits into
developmentfrom
chore/incrementalize-vehicle-routing
Open

TomCools wants to merge 3 commits into
developmentfrom
chore/incrementalize-vehicle-routing

Conversation

@TomCools

Copy link
Copy Markdown
Collaborator

Description of the change

The quickstart setup used foreach loops to calculate totals on the vehicle. This is bad for incremental score calculation.
Replaced that with a shadow variable on Visits, and only take the last element on the vehicle and add the home stretch and capacity.

Checklist

Development

  • The changes have been covered with tests, if necessary.
  • You have a green build, with the exception of the flaky tests.
  • UI and JS files are fully tested, the user interface works for all modules affected by your changes (e.g., solve and analyze buttons).
  • The network calls work for all modules affected by your changes (e.g., solving a problem).
  • The console messages are validated for all modules affected by your changes.

Code Review

  • This pull request includes an explanatory title and description.
  • The GitHub issue is linked.
  • At least one other engineer has approved the changes.
  • After PR is merged, inform the reporter.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved issues were identified that would block approval.

Review effort: Lite
Findings: None

What changed in this PR

Improves vehicle-routing score calculation incrementality by replacing route-wide demand and driving-time loops with shadow variables and last-visit aggregation.

Changes:

  • Adds cumulative demand and driving-time shadow data to visits.
  • Updates vehicle totals to use the final route visit.
File Description
use-cases/​vehicle-routing/​src/​main/​java/​org/​acme/​vehiclerouting/​domain/​Visit.java Updated as part of this pull request.
use-cases/​vehicle-routing/​src/​main/​java/​org/​acme/​vehiclerouting/​domain/​Vehicle.java Updated as part of this pull request.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@TomCools
TomCools marked this pull request as ready for review September 25, 2026 07:47
@TomCools
TomCools requested a review from triceo as a code owner September 25, 2026 07:47
@timefold-automations

This comment has been minimized.

@triceo triceo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM if you've seen FULL_ASSERT pass.

Copilot AI lite review requested due to automatic review settings September 30, 2026 12:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved issues were identified that would block approval.

Review effort: Lite
Findings: None

@timefold-automations

Copy link
Copy Markdown

This pull request, as it stands, would leave the Solver documentation stale in the places below; an automated check found these issues and can be wrong.

  1. Code change: TimefoldAI/timefold-quickstarts/use-cases/vehicle-routing/src/main/java/org/acme/vehiclerouting/domain/Vehicle.java:75-101 changes getTotalDemand() and getTotalDrivingTimeSeconds() from route loops to last-visit cumulative shadow values, but the copied Java sample still has the old loop-based calculations at timefold-solver/docs/src/modules/ROOT/pages/quickstart/quarkus-vehicle-routing/vehicle-routing-model.adoc:153-176. Confidence: high. Suggested edit:

    Replace the Java `Vehicle` sample's two loop-based total calculations with the cumulative-demand and cumulative-driving-time implementation from the pull request, including its null/uninitialized behavior.
  2. Code change: TimefoldAI/timefold-quickstarts/use-cases/vehicle-routing/src/main/java/org/acme/vehiclerouting/domain/Visit.java:33-34,76-151,258-284 adds cumulative shadow state, timingsSupplier(), cumulativeDemandSupplier(), cumulative-driving-time accessors, and a four-field Timings record, replacing the listener-based implementation; the copied Java sample still imports ArrivalTimeUpdatingVariableListener, declares @CascadingUpdateShadowVariable, and uses the old implementation at timefold-solver/docs/src/modules/ROOT/pages/quickstart/quarkus-vehicle-routing/vehicle-routing-model.adoc:294-384. Confidence: high. Suggested edit:

    Replace the Java `Visit` sample with the pull request's shadow-variable implementation, including `@ShadowVariable`/`@ShadowSources`, cumulative-demand and cumulative-driving-time accessors, and the four-field `Timings` record.

The documentation fix belongs in timefold-solver; because this group has no Solver pull request, add a timefold-solver pull request on the same branch name.

A new commit on the branch replaces this comment, and drift that is still there when the group merges gets a docs pull request against main.

Generated by Solver Docs Drift Review · copilot · gpt56 · 7.01 AIC · ⌖ 3.5 AIC · ⊞ 25K · ◷

This branch was successfully deployed

1 active deployment
internal — 422b91bf Deployed Sep 30, 2026 by TomCools via approval_required #1167
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