Align the two QueryIndex traversal workers

[brharrington]

Three changes to the Id-based traversal in QueryIndex, none of which alter what it returns. They remove waste and two divergences from the equivalent Function-based method sitting next to it. Follow-up to #1291.

The private worker is renamed to forEachMatchImpl

It was an arity overload of the public forEachMatch(Id, Consumer<T>) — the shape that caused the repeated wrapping #1291 fixed. Dropping the position argument from a recursive call bound to the public method and silently re-wrapped the consumer:

eqIdx.forEachMatch(tags, consumer);   // compiled fine, re-wrapped at every node

DedupConsumer<T> IS-A Consumer<T>, so nothing caught it. After the rename that call does not compile: the only 2-arg forEachMatchImpl takes a Function. DedupConsumer.from becomes a guard rather than the mechanism.

The has-key descent started one position early

It passed j, the position of the tag that matched this node's key, where the exact-match and other-condition descents from the same point pass j + 1. That sub-tree is built by hasKeyIdx.add(queries, j, value) from the queries after the ones for this key, so its own key sorts strictly after keyRef and the tag at j can never match it — the scan spent one iteration and one compareTagKey on it before moving on.

Safe at the boundary too: if the :has query is the last one, the sub-tree gets key == null and returns after matches.forEach, so the start position is irrelevant there.

The two methods now check the same sub-trees in the same place

otherKeysIdx/missingKeysIdx were inside the key != null guard here and outside it in the Function method, and keyPresent was scoped differently to match. The two agree today only because add() assigns key before it can create either sub-tree, so a node with no key has neither — but nothing recorded that, and the two methods read as though one of them was wrong. This takes the shape that does not depend on the invariant.

Also

DedupConsumer.from's type parameter was T, shadowing the enclosing DedupConsumer<T>, so (DedupConsumer<T>) consumer read as if it referenced the class parameter. Renamed to V, which is what the other static methods on this class use.

Tests

Three cases for the has-key descent: several keys following the :has, one of those keys absent, and the matching tag last so the sub-tree has nothing left to scan.

They pass before the change as well — the behaviour is unchanged, so they are guards rather than bug demonstrations. What makes them worth having is that they constrain the position: an off-by-one in that descent fails six tests in the class.

Worth noting the existing assertEquals helper already runs every expectation through both findMatches(Id) and findMatches(Function), so it cross-checks the two workers against each other automatically — which is what the divergence above needed.