feat: SEARCH parity — OFFSET, SCORE THRESHOLD, LOOKUP FROM + RECOMMEND integer literal fix#37
Merged
pavanjava merged 1 commit intoMay 20, 2026
Conversation
…hreshold, and cross-collection lookup support
📝 WalkthroughWalkthroughThis PR extends QQL's ChangesOFFSET, SCORE THRESHOLD, and LOOKUP FROM Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_parser.py (1)
286-289: ⚡ Quick winAdd a boundary test for
OFFSET 0withGROUP BY.Given the parser only rejects
OFFSET > 0, add one test thatOFFSET 0 GROUP BY ...parses successfully to lock that contract.Proposed test
+ def test_search_group_by_with_zero_offset_allowed(self): + node = parse("SEARCH notes SIMILAR TO 'hi' LIMIT 5 OFFSET 0 GROUP BY author") + assert node.group_by == "author" + assert node.offset == 0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_parser.py` around lines 286 - 289, Add a new test function (e.g., test_search_group_by_with_offset_zero_parses) in tests/test_parser.py that calls parse("SEARCH notes SIMILAR TO 'hi' LIMIT 5 OFFSET 0 GROUP BY author") and asserts that it parses successfully (no QQLSyntaxError is raised) — you can simply call parse and assert a truthy result or not wrap it in pytest.raises; this complements the existing test_search_group_by_with_offset_raises which checks OFFSET > 0 triggers QQLSyntaxError.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/qql/cli.py`:
- Line 105: Update the CLI help text so it matches parser behavior: replace the
current message "OFFSET is not supported with GROUP BY." with a precise
statement that only OFFSET > 0 is disallowed with GROUP BY (e.g., "OFFSET > 0 is
not supported with GROUP BY; OFFSET 0 is allowed"). Locate and edit the help
string used when constructing the OFFSET/limit argument in src/qql/cli.py (the
parser.add_argument help text for the OFFSET option) and run the CLI tests or
smoke commands to confirm the wording now aligns with the parser behavior.
---
Nitpick comments:
In `@tests/test_parser.py`:
- Around line 286-289: Add a new test function (e.g.,
test_search_group_by_with_offset_zero_parses) in tests/test_parser.py that calls
parse("SEARCH notes SIMILAR TO 'hi' LIMIT 5 OFFSET 0 GROUP BY author") and
asserts that it parses successfully (no QQLSyntaxError is raised) — you can
simply call parse and assert a truthy result or not wrap it in pytest.raises;
this complements the existing test_search_group_by_with_offset_raises which
checks OFFSET > 0 triggers QQLSyntaxError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ebd297f8-dedf-448b-82bd-48295e5a2a36
📒 Files selected for processing (9)
docs/programmatic.mddocs/reference.mddocs/search.mdsrc/qql/ast_nodes.pysrc/qql/cli.pysrc/qql/executor.pysrc/qql/parser.pytests/test_executor.pytests/test_parser.py
| Optional: [yellow]GROUP BY[/yellow] <field> [[yellow]GROUP_SIZE[/yellow] <n>] | ||
| Group results by a payload field value (default GROUP_SIZE: 3). | ||
| Field must be keyword or integer type. RERANK and GROUP BY cannot be combined. | ||
| OFFSET is not supported with GROUP BY. |
There was a problem hiding this comment.
Align GROUP BY/OFFSET help text with actual parser behavior.
Line 105 currently states OFFSET is unsupported with GROUP BY, but parser only rejects OFFSET > 0 (so OFFSET 0 is accepted). Please make docs and behavior consistent.
Proposed doc fix
- OFFSET is not supported with GROUP BY.
+ OFFSET > 0 is not supported with GROUP BY (OFFSET 0 is treated as no offset).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| OFFSET is not supported with GROUP BY. | |
| OFFSET > 0 is not supported with GROUP BY (OFFSET 0 is treated as no offset). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/qql/cli.py` at line 105, Update the CLI help text so it matches parser
behavior: replace the current message "OFFSET is not supported with GROUP BY."
with a precise statement that only OFFSET > 0 is disallowed with GROUP BY (e.g.,
"OFFSET > 0 is not supported with GROUP BY; OFFSET 0 is allowed"). Locate and
edit the help string used when constructing the OFFSET/limit argument in
src/qql/cli.py (the parser.add_argument help text for the OFFSET option) and run
the CLI tests or smoke commands to confirm the wording now aligns with the
parser behavior.
pavanjava
approved these changes
May 20, 2026
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.
Summary
Closes ROADMAP items #2 and #3:
OFFSET,SCORE THRESHOLD, andLOOKUP FROMtoSEARCHstatements, with full support across dense, sparse, hybrid, andGROUP BYexecution paths.SCORE THRESHOLDnow accepts integer literals (e.g.,SCORE THRESHOLD 1) instead of requiring a float token.What's New
Parser
SEARCH ... LIMIT <n>:OFFSET <n>— skip first N resultsSCORE THRESHOLD <f>— drop results below this scoreLOOKUP FROM <collection> [VECTOR '<name>']— cross-collection vector lookupOFFSETraisesQQLSyntaxError(guarded in both SEARCH and RECOMMEND)OFFSET > 0combined withGROUP BYraisesQQLSyntaxError(Qdrant groups use cursors, not offset)RECOMMEND SCORE THRESHOLDnow uses_parse_number()to accept both int and float literalsExecutor
offset,score_threshold, andlookup_fromto Qdrant SDK for:SEARCHSEARCHSEARCHSEARCH ... GROUP BY(offset is intentionally omitted; score_threshold and lookup_from are passed)offset=0is translated toNonewhen calling the SDK (clean default behavior)Tests (604 passing)
test_search_forwards_offset_score_lookup— dense pathtest_search_forwards_offset_score_lookup_hybrid— hybrid pathtest_search_forwards_offset_score_lookup_sparse— sparse pathtest_search_groups_forwards_score_lookup— grouped pathtest_search_forwards_offset_0_as_none— verifies 0 → Nonetest_recommend_forwards_offset_0_as_none— verifies RECOMMEND 0 → NoneSCORE THRESHOLDon SEARCHDocs & CLI Help
docs/search.md— new examples, clause order block, GROUP BY incompatibility warningdocs/reference.md— pagination/score filtering examples, updated test countdocs/programmatic.md— updated SEARCH and RECOMMEND examplessrc/qql/cli.py— SEARCH and RECOMMEND help now list all optional clausesBug Fix
EXACTmerging logic in_parse_searchsoWITH { ... } EXACTcorrectly preserves priorwith_clausefields while settingexact=TrueFiles Changed
src/qql/ast_nodes.py— document new fields onSearchStmtandRecommendStmtsrc/qql/parser.py— parse new SEARCH clauses, guard invalid combos, fix RECOMMEND number parsingsrc/qql/executor.py— wire new params through all search pathssrc/qql/cli.py— sync help text with parser capabilitiesdocs/search.md— new examples, clause order, GROUP BY warningdocs/reference.md— new examples, updated test countdocs/programmatic.md— updated code examplestests/test_parser.py— parser coverage for new syntax and edge casestests/test_executor.py— executor coverage for all search paths and RECOMMENDChecklist
Summary by CodeRabbit
Release Notes
New Features
OFFSETclause for search queries.SCORE THRESHOLDclause for both search and recommendation queries.LOOKUP FROMclause.Documentation
OFFSETcannot be used withGROUP BY).