fix: update every element unassigned in the same shadow variable pass

[fodzal]

Issue.
In a SINGLE_DIRECTIONAL_PARENT graph, a pass that unassigns two or more elements only updates the first of
them; the rest keep the value they held while still assigned.

updateChanged() walks each changed element's chain, and skips one that a previous walk already covered:

var key = keyFunction.apply(changedEntity);   // the element's inverse entity
var lastProcessed = keyToLastProcessedObject.get(key);
if (lastProcessed == null || topologicalOrderComparator.compare(lastProcessed, changedEntity) < 0) {
    lastProcessed = updateChanged(changedEntity);
    keyToLastProcessedObject.put(key, lastProcessed);
}

The key is null for every unassigned element, and they all compare equal too (getIndexOrElse defaults to 0) — so the first one walked claims the null key, and every other unassigned element wrongly reads that walk as already covering it, even though a walk starting at an unassigned element never reaches past itself.

[v1, v2, v3] (counts 0, 1, 2), one move unassigning both v2 and v3:

	key	guard	outcome
v2	null	nothing under that key yet	walked, recomputed to null ✓
v3	null	compare(v2, v3) == 0, not < 0	skipped, keeps its stale count of 2

Any move that unassigns 2 or more elements of the same list under one before/after bracket triggers this (for example: SubListUnassignMoveProvider).

Fix.
An unassigned element is tracked by its own identity instead of being pooled into keyToLastProcessedObject under the shared null key:

var key = keyFunction.apply(changedEntity);
if (key == null) {
    if (processedUnassignedEntitySet.add(changedEntity)) {
        updateChanged(changedEntity);
    }
} else {
    var lastProcessed = keyToLastProcessedObject.get(key);
    if (lastProcessed == null || topologicalOrderComparator.compare(lastProcessed, changedEntity) < 0) {
        lastProcessed = updateChanged(changedEntity);
        keyToLastProcessedObject.put(key, lastProcessed);
    }
}

A dedicated processedUnassignedEntitySet (identity-based, cleared every pass like keyToLastProcessedObject) dedupes the two events one unassigned element fires (entity and previous both changing), without pooling it into the map meant for chain-position lookups.

TopologicalSorter.key() was declared non-nullable even though it's getInverseSingleton; the record's Javadoc now says what each component returns for an element in no chain.

Test.
SingleDirectionalParentVariableReferenceGraphTest#elementsUnassignedInTheSamePassAreAllUpdated
It unassigns two of an entity's four elements in one pass, firing both parent-variable changes for each (as a real unassign does), and asserts both lose their count. TestdataCountingValue#countSupplier throws if called twice in the same pass, which pins that the fix doesn't trade a skipped update for a doubled one.

[Christopher-Chianelli]

I would do it a different way to avoid putting useless keys inside the processed object map.

[Christopher-Chianelli]

I wouldn't do this; I would instead do if (key == null) {updateChanged(changedEntity);} else if (...) {} below

[fodzal]

It's still useful for unassigned element to avoid updating twice an unassigned element (since its entity and previous both change and each fires its own event).
So, I have added another map processedUnassignedEntitySet.

[triceo]

@fodzal Thanks for the fix! Do you have any plan for when you expect to undraft this?
We have a release coming up and, if possible, we'd like to roll this one in.

[triceo]

Any chance we can use some sort of sentinel for the null key, and therefore avoiding the second map? Since the map is an identity hash map, we can probably use almost anything that's guaranteed not to be there already - such as some UUID.

[triceo]

This may be where I run against Chris' previous comment. If that's the case, then I'd argue the solution is worse than the problem, and the sentinel works.

[fodzal]

Yes, I guess we can keep my first version (and add a comment in the code to explain the logic)

[Christopher-Chianelli]

You both do realize changedEntities can be turned into an identity set to avoid duplicates? The hash check would either be here (inside the map) or in afterVariableChanged, and doing it in afterVariableChanged would minimize the size of changedEntities.

[triceo]

It doesn't help if we're both looking into this. My bad - I'll remove myself from the conversation.