Answer the key that actually comes after at #nextKeyCloseTo: - #1255
Merged
Merged
Conversation
noha
force-pushed
the
next-key-close-to-contract
branch
from
September 14, 2026 14:16
918e7c1 to
9da2ac0
Compare
the page instead whenever no key on that page qualified. #nextCloseTo: then looked that earlier entry up over #at: and handed it out, so both answered something before key rather than after it. Two cases hid behind that fallback. Behind the last entry of the index there is no answer, and nil is what says so. In the gap between the last item of a page and the first item of the page after it there is an answer, just not on the page #findPageFor: stops at - #keyOrClosestAfter: only ever looks at a single page. So we walk on to the next page that holds something and answer its first key. The cursor is left on the entry that was answered, the same place a found key leaves it, and one behind the last item when the answer is nil, so that reading backwards still answers that item instead of skipping it. In the gap that is a page further on than where 7342f01 left it: reading forward now skips the entry that was answered, the way it already does when the answer sits on the page the key belongs to. SoilCursor pins that exclusion in #testReadsExcludingItsOwnKey and #testKeyGoneKeepsReadingForward, and it should not depend on which page the answer happens to live on. The three tests that pinned the old answer with "this is a bit odd, when looking with larger values we get the last one" expect nil now, and #testNextKeyCloseToBehindPageEnd expects the entry behind the gap rather than the one in front of it. made that key a byte longer and therefore larger than #'20', so the row tested the same case as #'100' instead of the one between the two entries it sits in - the space is gone. SoilPrefixRangeTest>>#readAll:from: carried a guard against the old answer and says so in its comment; nil ends that loop on its own now. SoilIndexRange>>#firstKey already describes the contract this commit establishes. Callers in apptive-base have to follow: AGListGlobalEventsOperation>>#readOlderThan: gives up on nil and would lose the page it is asked for when before sits behind the whole log. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
noha
force-pushed
the
next-key-close-to-contract
branch
from
September 14, 2026 14:25
9da2ac0 to
b063eb7
Compare
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
2 participants
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.
#nextKeyCloseTo: promised the first entry at or after key and answered the last key of the page instead whenever no key on that page qualified. #nextCloseTo: then looked that earlier entry up over #at: and handed it out as "the next one after key", so both answered something before key rather than after it.
Two cases hid behind that fallback. Behind the last entry of the index there is no answer, and nil is what says so. In the gap between the last item of a page and the first item of the page after it there is an answer, just not on the page #findPageFor: stops at - #keyOrClosestAfter: only ever looks at a single page. So we walk on to the next page that holds something and answer its first key.
The cursor is left on the entry that was answered, the same place a found key leaves it, and one behind the last item when the answer is nil, so that reading backwards still answers that item instead of skipping it. It stayed at 0 there before, which #basicNextAssociation reads as "no key on this page yet": reading went on at the beginning of the page rather than after the key.
The three tests that pinned the old answer with "this is a bit odd, when looking with larger values we get the last one" expect nil now. #testNextCloseTo of SoilIndexedDictionaryTest carried a trailing space in #'15 ' which made that key a byte longer and therefore larger than #'20', so the row tested the same case as #'100' instead of the one between the two entries it sits in - the space is gone.
Callers in apptive-base have to follow: AGListGlobalEventsOperation>>#readOlderThan: gives up on nil and would lose the page it is asked for when before sits behind the whole log.