Skip to content

chore: remove support for the identity map from the value range cache#2140

Merged
triceo merged 4 commits intoTimefoldAI:1.xfrom
zepfred:fix/map
Feb 23, 2026
Merged

chore: remove support for the identity map from the value range cache#2140
triceo merged 4 commits intoTimefoldAI:1.xfrom
zepfred:fix/map

Conversation

@zepfred
Copy link
Copy Markdown
Contributor

@zepfred zepfred commented Feb 23, 2026

No description provided.

@zepfred zepfred marked this pull request as ready for review February 23, 2026 17:48
@zepfred zepfred requested a review from triceo as a code owner February 23, 2026 17:48
Copilot AI review requested due to automatic review settings February 23, 2026 17:48
Copy link
Copy Markdown
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 PR removes identity-based caching/indexing behavior in value range infrastructure, standardizing lookups and deduplication on equals/hashCode semantics across value range caches and related indexing.

Changes:

  • Simplified ValueRangeCache by removing the builder/trusted-vs-user modes and introducing of(...) factory methods backed by standard hash-based collections.
  • Removed isValueImmutable plumbing from collection/composite value ranges and stopped using immutability to decide identity vs equals-based maps in ValueRangeState.
  • Updated/extended unit tests to assert contains() works for different instances that are equal by planning ID/code.

Reviewed changes

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

Show a summary per file
File Description
core/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/ValueRangeCache.java Removes identity-map support and introduces new of(...) factories for hash-based caching.
core/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/AbstractCountableValueRange.java Removes isValueImmutable() hook and its documentation.
core/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/collection/ListValueRange.java Drops immutability flag usage and builds cache via ValueRangeCache.of(...).
core/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/collection/SetValueRange.java Drops immutability flag usage and builds cache via ValueRangeCache.of(...).
core/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/composite/CompositeCountableValueRange.java Drops immutability tracking and uses ValueRangeCache.of(int) for dedup/cache creation.
core/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/AbstractFromPropertyValueRangeDescriptor.java Updates construction of SetValueRange/ListValueRange to new constructors.
core/src/main/java/ai/timefold/solver/core/impl/score/director/ValueRangeState.java Simplifies index map building to hash-based maps (no identity-map path).
core/src/test/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/collection/ListValueRangeTest.java Adds coverage asserting contains() works across equal-but-distinct instances.
core/src/test/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/collection/SetValueRangeTest.java Adds coverage asserting contains() works across equal-but-distinct instances.
core/src/test/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/composite/CompositeCountableValueRangeTest.java Adds coverage asserting contains() works across equal-but-distinct instances in composite ranges.

Copy link
Copy Markdown
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.

LGTM when comments resolved.

@triceo triceo merged commit a3ccc3a into TimefoldAI:1.x Feb 23, 2026
20 checks passed
@zepfred zepfred deleted the fix/map branch March 10, 2026 16:45
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