fix(search): keep unfiltered function pages in stable id order - #17
Merged
Conversation
The unfiltered function-list path added in #13 paged straight off SSCAN, which has no stable ordering. Consecutive pages could overlap or skip rows, and the same request could return a different page each time — the old Lua path sorted by id ascending. There is no id-ordered index to range over (save_function only does SADD on {col}:all_functions), so bound the scan instead: collect up to SORT_SCAN_CAP members, sort by id, slice the page, and report anything beyond the cap via the existing pool_truncated flag. The scan stays incremental, so it never holds Kvrocks the way the SMEMBERS-in-Lua path did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The unfiltered function-list path added in #13 paged straight off SSCAN, which has no stable ordering. Consecutive pages could overlap or skip rows, and the same request could return a different page each time — the old Lua path sorted by id ascending. Use the same shape as the unfiltered paths in search_file and search_feature: load the set, sort in Python, slice. The hang in #12 came from doing this inside Lua under the Kvrocks global EVAL lock, which #13 already removed; from Python the sibling routes have done it this way all along. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rdmmf
marked this pull request as ready for review
July 27, 2026 13:31
rdmmf
added a commit
that referenced
this pull request
Jul 27, 2026
* Avoid Lua full-set reads for unfiltered function lists * fix(search): keep unfiltered function pages in stable id order The unfiltered function-list path added in #13 paged straight off SSCAN, which has no stable ordering. Consecutive pages could overlap or skip rows, and the same request could return a different page each time — the old Lua path sorted by id ascending. There is no id-ordered index to range over (save_function only does SADD on {col}:all_functions), so bound the scan instead: collect up to SORT_SCAN_CAP members, sort by id, slice the page, and report anything beyond the cap via the existing pool_truncated flag. The scan stays incremental, so it never holds Kvrocks the way the SMEMBERS-in-Lua path did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(search): keep unfiltered function pages in stable id order The unfiltered function-list path added in #13 paged straight off SSCAN, which has no stable ordering. Consecutive pages could overlap or skip rows, and the same request could return a different page each time — the old Lua path sorted by id ascending. Use the same shape as the unfiltered paths in search_file and search_feature: load the set, sort in Python, slice. The hang in #12 came from doing this inside Lua under the Kvrocks global EVAL lock, which #13 already removed; from Python the sibling routes have done it this way all along. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: SegmondFault <izaakalfredgray@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Follow-up to #13.
#13 removed the Kvrocks hang by keeping unfiltered function lists off the Lua path, but it pages straight off
SSCAN. That has no stable ordering, so:The fix is to do what
search_file.pyandsearch_feature.pyalready do on their unfiltered paths: load the set, sort in Python, slice. The hang in #12 came from doing this inside Lua, under the Kvrocks global EVAL lock — #13 already removed that. From Python it's the house pattern, andsearch_file.py:386carries aponytail:comment naming the same ceiling.Net effect on
dev: 17 insertions, 20 deletions —_sscan_pageand its truncation bookkeeping go away.The
sort_by != "id"branch with a present zset index is unchanged, and filtered searches still go through Lua.Ceiling
all_functionsis the largest of the three sets, so if any unfiltered path needs a maintained id index first, it's this one. Noted in the docstring rather than pre-built.Testing
uv run python -m compileall bsimvis/app/routes/search_function.py🤖 Generated with Claude Code