Fix IntervalDict.update() dropping order of overlapping pairs

[afonsojanu]

IntervalDict.update() groups the incoming (interval, value) pairs by value before applying them, so it can merge same-valued intervals into a single __setitem__ call. The problem is that the groups then get applied in whatever order the value was first seen in the input, not in the order the pairs were actually given. When two pairs with different values have overlapping intervals, the one that should logically win (the later pair, matching how a plain dict behaves when built from a list of pairs) can lose instead.

import portion as P

d = P.IntervalDict()
d.update([
    (P.closed(4, 6), "c"),
    (P.closed(-5, -1), "b"),
    (P.open(-5, -3), "c"),
])
d[-4]  # 'b', but the last pair covering -4 says 'c'

Non-hashable values make it worse: they get applied immediately during the first pass (since grouping by value needs hashing and falls back to an immediate assignment), ahead of any hashable group that should have been applied earlier in sequence.

I found this by differentially fuzzing IntervalDict against a plain dict over a small integer domain, applying random sequences of set/del/update calls to both and comparing point queries afterward.

The fix keeps the batching for consecutive pairs that share the same value (so a run of same-valued pairs is still a single __setitem__ call) but applies the runs strictly in the order they appear, so later pairs correctly override earlier ones wherever they overlap.

Added a regression test with both a hashable and non-hashable value on the losing side, and confirmed it fails against the current code and passes with the fix. Full test suite still passes locally (211 passed, 11 skipped).

[AlexandreDecan]

Thanks for having found this bug, it's a tricky one!
I'll have a look at the proposed changes this week.

[AlexandreDecan]

Could you confirm that the PR only fixes cases of consecutive pairs? Or did I miss something? I still need to have a deeper look at the code and do some tests...

[Copilot]

Copilot review overview

🟢 Approval recommended

The implementation correctly preserves pair ordering with adequate regression coverage; only a minor docstring typo remains.

Review effort: Balanced
Findings: 1

Open (1)

• Fix docstring wording for precedence guarantee · New

What changed in this PR

Fixes IntervalDict.update() so overlapping pairs honor input order.

Changes:

• Batches only consecutive equal values.
• Adds hashable and unhashable regression cases.
• Documents precedence behavior.

File	Description
portion/​dict.py	Preserves update ordering while retaining safe batching.
tests/​test_dict.py	Tests overlap precedence across value types.

---

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

[AlexandreDecan]

Could you confirm that the PR only fixes cases of consecutive pairs?

Forget my question, it should work in all cases for what I understood. I'll have a deeper look soon and will try to merge it ASAP. Thanks!

[AlexandreDecan]

Based on your (many) recent contributions to (many) repositories, I'm unsure there was a human involved for this PR. After having read the code, I'm even more convinced there wasn't. For instance, why is there still a difference between hashable values and non-hashable ones? Initially, the idea of checking for a hashable value was to group intervals so that we update the dictionary in a single pass for each set of intervals sharing a similar value. While it's true this lead to the bug being addressed by this PR, we do not (and cannot) make use of this trick anymore.

I'm unsure which of the two following scenarii is best:

• Implement something like what is done in this PR, removing any optimisation for hashable values but optimising for consecutive intervals sharing a same value;
• Keeping the optimisation for hashable values (assuming that they are way more frequent than non-hashable ones) and explicitly state in the documentation that order is NOT considered in case of overlapping intervals.