Fix AIndex.entrySet() losing sorted key order via HashSet#652
Merged
brittleboye merged 1 commit intoJul 24, 2026
Merged
Conversation
AIndex documents itself as a sorted radix-tree map that provides sorted orderings for indexes, and entryAt(i) correctly walks the tree in ascending key order. But entrySet() collected those entries into a plain HashSet, whose iteration order is hash-bucket-based rather than insertion order — silently discarding the sort guarantee for any caller iterating entrySet() (or keySet(), forEach, etc.) directly instead of calling entryAt(i) in a loop. Swap HashSet for LinkedHashSet, which preserves the insertion order entryAt(i) already produces. Adds a regression test that fails on the old HashSet-based implementation and passes with LinkedHashSet. Fixes Convex-Dev#651.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #651.
Problem
AIndexdocuments itself as "a sorted radix-tree map of Blobs to Values" whose primary benefit is to "provide sorted orderings for indexes." That guarantee holds forentryAt(i), but not forentrySet():entryAt(i)walks the radix tree correctly in ascending key order, but the entries are collected into ajava.util.HashSet, whose iteration order is hash-bucket-based, not insertion order. Any caller iteratingentrySet()directly (orkeySet(),forEach, an enhanced-for loop) — a reasonable thing to do given the class's documented sort guarantee — gets entries back in effectively arbitrary order instead.Fix
Swap
HashSetforLinkedHashSet. SinceentryAt(i)already inserts in correct ascending order fori = 0..n-1, aLinkedHashSet(which preserves insertion order) is sufficient to restore the documented guarantee — no other logic changes.Testing
Added
IndexTest.testEntrySetOrder(), which builds a 50-entryIndexand assertsentrySet()'s iteration order matchesentryAt(i)for everyi.HashSetimplementation (confirmed locally: entry 0 expected, entry 49 observed at the same position)LinkedHashSetfixconvex-coresuite: 2812 tests, 0 failures, 0 errors🤖 Generated with Claude Code