Skip to content

chore: fail-fast validation for invalid value ranges#2069

Merged
triceo merged 13 commits intoTimefoldAI:mainfrom
zepfred:fix/assigned
Feb 4, 2026
Merged

chore: fail-fast validation for invalid value ranges#2069
triceo merged 13 commits intoTimefoldAI:mainfrom
zepfred:fix/assigned

Conversation

@zepfred
Copy link
Contributor

@zepfred zepfred commented Feb 2, 2026

No description provided.

@zepfred zepfred changed the title chore: fail-fast if the unassigned count is inconsistent chore: fail-fast validation for invalid value ranges Feb 3, 2026
@zepfred zepfred marked this pull request as ready for review February 3, 2026 14:54
@zepfred zepfred requested a review from triceo as a code owner February 3, 2026 14:54
Copilot AI review requested due to automatic review settings February 3, 2026 14:54
@zepfred zepfred marked this pull request as draft February 3, 2026 14:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request implements fail-fast validation for invalid value ranges to catch configuration errors early when planning values are assigned outside their defined value ranges.

Changes:

  • Added value range validation that checks all assigned planning values are within their respective value ranges
  • Validation runs once at solver initialization and optionally during move execution in FULL_ASSERT mode
  • Comprehensive test coverage for basic variables, list variables, multi-variable entities, and custom phases

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java Added interface method for value range assertion
core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java Implemented value range validation logic for basic and list variables, integrated into move execution assertions
core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java Added one-time value range check during initial solution setup
core/src/main/java/ai/timefold/solver/core/impl/phase/custom/DefaultCustomPhase.java Added value range assertion for custom phases when running in FULL_ASSERT mode
core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java Added comprehensive tests for value range validation across different variable types and scenarios

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Copy link
Collaborator

@triceo triceo left a comment

Choose a reason for hiding this comment

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

Much better! IMO still room for improvement.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.

@triceo
Copy link
Collaborator

triceo commented Feb 4, 2026

Having just merged my other PR, this one now has conflicts. :-(
Also, I see some more Copilot comments.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 4, 2026

@triceo triceo merged commit f24e9a3 into TimefoldAI:main Feb 4, 2026
36 checks passed
@triceo triceo added this to the v1.31.0 milestone Feb 4, 2026
@zepfred zepfred deleted the fix/assigned branch February 4, 2026 14:52
@zepfred zepfred self-assigned this Feb 4, 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.

2 participants