fix: paginate wildcard listing to prevent documents missing from Knowledge view - #2198
fix: paginate wildcard listing to prevent documents missing from Knowledge view#2198yogichipalkatti wants to merge 2 commits into
Conversation
github-actions
Bot
added
frontend
WalkthroughSearch requests now accept offsets. The backend propagates offsets through context into OpenSearch. Wildcard frontend searches fetch 1,000-result pages, merge chunks by file, preserve warnings, and stop at the final page or safety limit. ChangesSearch pagination
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant FrontendSearchQuery
participant SearchAPI
participant SearchService
participant OpenSearch
FrontendSearchQuery->>SearchAPI: request wildcard search with offset
SearchAPI->>SearchService: forward offset and filters
SearchService->>OpenSearch: fetch page with from=offset
OpenSearch-->>SearchService: return chunks and warnings
SearchService-->>SearchAPI: return search page
SearchAPI-->>FrontendSearchQuery: return page results
FrontendSearchQuery->>FrontendSearchQuery: merge chunks by file identity
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/search_service.py (1)
193-202: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFix
NameError:get_search_offsetis not imported.
get_search_offset()is called at Line 202, but thefrom auth_context import (...)block at Lines 194-198 imports onlyget_score_threshold,get_search_filters, andget_search_limit. Ruff confirms this as an undefined name (F821). Every call tosearch_tool— every search request — raisesNameErrorat runtime, since this line executes unconditionally before the wildcard/non-wildcard branch.🐛 Proposed fix
from auth_context import ( get_score_threshold, get_search_filters, + get_search_offset, get_search_limit, )🤖 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/services/search_service.py` around lines 193 - 202, Add get_search_offset to the auth_context import block in the search_tool flow before its existing call, preserving the current filters, limit, and offset handling.Source: Linters/SAST tools
🧹 Nitpick comments (2)
frontend/app/api/queries/useGetSearchQuery.ts (1)
122-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated file-accumulator type into a shared alias.
The
Map<string, {...}>value type is declared three times with identical shape: inmergeChunksIntoFileMap's parameter type, in the wildcard-pathfileMapdeclaration, and in the non-wildcard-pathfileMapdeclaration. A shared type alias removes the risk of the three declarations drifting apart when a field is added or renamed.♻️ Proposed refactor
+type FileAccumulator = { + filename: string; + mimetype: string; + chunks: ChunkResult[]; + totalScore: number; + source_url?: string; + owner?: string; + owner_name?: string; + owner_email?: string; + file_size?: number; + connector_type?: string; + embedding_model?: string; + embedding_dimensions?: number; + allowed_users?: string[]; + allowed_groups?: string[]; +}; + function mergeChunksIntoFileMap( chunks: ChunkResult[], - fileMap: Map< - string, - { - filename: string; - mimetype: string; - chunks: ChunkResult[]; - totalScore: number; - source_url?: string; - owner?: string; - owner_name?: string; - owner_email?: string; - file_size?: number; - connector_type?: string; - embedding_model?: string; - embedding_dimensions?: number; - allowed_users?: string[]; - allowed_groups?: string[]; - } - >, + fileMap: Map<string, FileAccumulator>, getFileIdentity: (chunk: ChunkResult) => string, ): void {Then replace both
fileMapdeclarations withnew Map<string, FileAccumulator>().Also applies to: 230-248, 325-343
🤖 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 `@frontend/app/api/queries/useGetSearchQuery.ts` around lines 122 - 142, Extract the repeated file-map value shape into a shared FileAccumulator type alias near mergeChunksIntoFileMap. Update that function’s fileMap parameter and both wildcard and non-wildcard fileMap declarations to use Map<string, FileAccumulator>, preserving all existing fields and behavior.src/services/search_service.py (1)
498-506: 🚀 Performance & Scalability | 🔵 TrivialVerify the memory/latency cost of a 10000-bucket terms aggregation on
filename.
data_sourcesnow requests up tosize: 10000buckets on thefilenamefield. Terms aggregations of this size increase per-shard memory usage and response payload size, especially on a high-cardinality field likefilename. Confirm this is bounded to an acceptable corpus size, or consider a lower ceiling with a documented rationale.🤖 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/services/search_service.py` around lines 498 - 506, Review the data_sources aggregation in the search_body construction and verify that requesting 10,000 filename buckets is acceptable for the supported corpus and cluster resource limits. If it is not, lower the terms size to an appropriate bounded ceiling and document the rationale near the configuration, preserving the other aggregations unchanged.
🤖 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/api/search.py`:
- Around line 7-12: Remove the unused get_current_user import from the
dependencies import block in the search route module; retain get_search_service,
get_session_manager, and require_permission unchanged.
In `@src/auth_context.py`:
- Line 7: Update the typing imports in auth_context.py to remove the unused Dict
and Optional names, while retaining Any for the existing type annotations.
In `@src/services/search_service.py`:
- Around line 730-734: Update the auth_context import in the search() flow to
include set_search_offset alongside set_score_threshold and set_search_limit, so
the existing set_search_offset(offset) call resolves without changing search
behavior.
- Around line 527-529: Update the wildcard pagination flow in search_service.py
around the OpenSearch request using "from" and "size", together with the
pagination logic in frontend/app/api/queries/useGetSearchQuery.ts, so requests
remain valid through the configured WILDCARD_QUERY_LIMIT of 10000. Prefer
converting wildcard listing pagination to search_after; otherwise raise the
relevant index max_result_window setting to cover the full offset-plus-size
range at both affected sites.
---
Outside diff comments:
In `@src/services/search_service.py`:
- Around line 193-202: Add get_search_offset to the auth_context import block in
the search_tool flow before its existing call, preserving the current filters,
limit, and offset handling.
---
Nitpick comments:
In `@frontend/app/api/queries/useGetSearchQuery.ts`:
- Around line 122-142: Extract the repeated file-map value shape into a shared
FileAccumulator type alias near mergeChunksIntoFileMap. Update that function’s
fileMap parameter and both wildcard and non-wildcard fileMap declarations to use
Map<string, FileAccumulator>, preserving all existing fields and behavior.
In `@src/services/search_service.py`:
- Around line 498-506: Review the data_sources aggregation in the search_body
construction and verify that requesting 10,000 filename buckets is acceptable
for the supported corpus and cluster resource limits. If it is not, lower the
terms size to an appropriate bounded ceiling and document the rationale near the
configuration, preserving the other aggregations unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 58472b6a-787d-4dfe-bd8a-8ea9fcf6dc42
📒 Files selected for processing (4)
frontend/app/api/queries/useGetSearchQuery.tssrc/api/search.pysrc/auth_context.pysrc/services/search_service.py
| from dependencies import ( | ||
| get_current_user, | ||
| get_search_service, | ||
| get_session_manager, | ||
| get_current_user, | ||
| require_permission, | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the unused get_current_user import.
Ruff flags get_current_user as unused (F401). The route authenticates via require_permission("search:use") at Line 34, not get_current_user. Remove the import to keep the pipeline green.
🧹 Proposed fix
from dependencies import (
- get_current_user,
get_search_service,
get_session_manager,
require_permission,
)📝 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.
| from dependencies import ( | |
| get_current_user, | |
| get_search_service, | |
| get_session_manager, | |
| get_current_user, | |
| require_permission, | |
| ) | |
| from dependencies import ( | |
| get_search_service, | |
| get_session_manager, | |
| require_permission, | |
| ) |
🧰 Tools
🪛 GitHub Check: Ruff and mypy on changed files
[failure] 8-8: ruff (F401)
src/api/search.py:8:5: F401 dependencies.get_current_user imported but unused
help: Remove unused import: dependencies.get_current_user
🤖 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/api/search.py` around lines 7 - 12, Remove the unused get_current_user
import from the dependencies import block in the search route module; retain
get_search_service, get_session_manager, and require_permission unchanged.
Source: Linters/SAST tools
|
|
||
| from contextvars import ContextVar | ||
| from typing import Optional, Dict, Any | ||
| from typing import Any, Dict, Optional |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove unused Dict and Optional imports.
Ruff flags Dict and Optional as unused (F401), and flags Dict as deprecated in favor of the built-in dict (UP035). The file already uses str | None and dict[str, Any] elsewhere, so Dict/Optional are dead weight.
🧹 Proposed fix
-from typing import Any, Dict, Optional
+from typing import Any📝 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.
| from typing import Any, Dict, Optional | |
| from typing import Any |
🧰 Tools
🪛 GitHub Check: Ruff and mypy on changed files
[failure] 7-7: ruff (F401)
src/auth_context.py:7:31: F401 typing.Optional imported but unused
help: Remove unused import
[failure] 7-7: ruff (F401)
src/auth_context.py:7:25: F401 typing.Dict imported but unused
help: Remove unused import
[failure] 7-7: ruff (UP035)
src/auth_context.py:7:1: UP035 typing.Dict is deprecated, use dict instead
🤖 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/auth_context.py` at line 7, Update the typing imports in auth_context.py
to remove the unused Dict and Optional names, while retaining Any for the
existing type annotations.
Source: Linters/SAST tools
| "from": offset, | ||
| "size": limit, | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate WILDCARD_QUERY_LIMIT's value and any max_result_window overrides.
rg -n "WILDCARD_QUERY_LIMIT" --type=ts -C2
rg -n "max_result_window" -C3Repository: langflow-ai/openrag
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -u
echo "Tracked files matching search/query names:"
git ls-files | rg 'search_service\.py|queries/useGetSearchQuery\.ts|searchService|search' || true
echo
echo "Find constants definitions/usages without file-type filter:"
rg -n "WILDCARD_QUERY_LIMIT|WILDCARD_PAGE_SIZE|SEARCH_CONSTANTS|from\": offset|size\": limit|max_result_window|Result window is too large" . -C 3 || true
echo
echo "Candidate search_service.py lines around pagination:"
if [ -f src/services/search_service.py ]; then
sed -n '500,545p' src/services/search_service.py | cat -n
echo "--- lines 680-720 ---"
sed -n '680,720p' src/services/search_service.py | cat -n
fi
echo
echo "Candidate frontend query lines around pagination:"
if [ -f frontend/app/api/queries.useGetSearchQuery.ts ]; then
sed -n '230,290p' frontend/app/api/queries.useGetSearchQuery.ts | cat -n
fi
if [ -f frontend/app/api/queries/useGetSearchQuery.ts ]; then
sed -n '230,290p' frontend/app/api/queries/useGetSearchQuery.ts | cat -n
fiRepository: langflow-ai/openrag
Length of output: 16480
🏁 Script executed:
#!/bin/bash
set -u
echo "List files containing useGetSearchQuery:"
git ls-files | rg 'useGetSearchQuery|queries' || true
echo
echo "Search for Wildcard constant names/usages (case insensitive):"
rg -n "WILDCARD|QUERY_LIMIT|PAGE_SIZE|max_result_window|Result window|from.*offset|size.*limit" . -C 3 || trueRepository: langflow-ai/openrag
Length of output: 50376
Handle the OpenSearch result-window boundary for wildcard pagination.
frontend/lib/constants.ts caps WILDCARD_QUERY_LIMIT at 10000, but frontend/app/api/queries/useGetSearchQuery.ts paginates with WILDCARD_PAGE_SIZE = 1000 and search_service.py passes { "from": offset, "size": limit } directly to OpenSearch. A request where offset + size exceeds the index’s index.max_result_window still fails with “Result window is too large”. Raise the index setting to support the intended pagination depth, or switch the wildcard listing to search_after so it is not bounded by from/size.
📍 Affects 2 files
src/services/search_service.py#L527-L529(this comment)frontend/app/api/queries/useGetSearchQuery.ts#L250-L276
🤖 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/services/search_service.py` around lines 527 - 529, Update the wildcard
pagination flow in search_service.py around the OpenSearch request using "from"
and "size", together with the pagination logic in
frontend/app/api/queries/useGetSearchQuery.ts, so requests remain valid through
the configured WILDCARD_QUERY_LIMIT of 10000. Prefer converting wildcard listing
pagination to search_after; otherwise raise the relevant index max_result_window
setting to cover the full offset-plus-size range at both affected sites.
| from auth_context import set_score_threshold, set_search_limit | ||
|
|
||
| set_search_limit(limit) | ||
| set_search_offset(offset) | ||
| set_score_threshold(score_threshold) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix NameError: set_search_offset is not imported.
set_search_offset(offset) is called at Line 733, but the from auth_context import set_score_threshold, set_search_limit statement at Line 730 does not import set_search_offset. Ruff confirms this as an undefined name (F821). Every call to the public search() method — the entry point used by the API route — raises NameError at runtime.
🐛 Proposed fix
- from auth_context import set_score_threshold, set_search_limit
+ from auth_context import set_score_threshold, set_search_limit, set_search_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.
| from auth_context import set_score_threshold, set_search_limit | |
| set_search_limit(limit) | |
| set_search_offset(offset) | |
| set_score_threshold(score_threshold) | |
| from auth_context import set_score_threshold, set_search_limit, set_search_offset | |
| set_search_limit(limit) | |
| set_search_offset(offset) | |
| set_score_threshold(score_threshold) |
🧰 Tools
🪛 GitHub Check: Ruff and mypy on changed files
[failure] 733-733: ruff (F821)
src/services/search_service.py:733:9: F821 Undefined name set_search_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/services/search_service.py` around lines 730 - 734, Update the
auth_context import in the search() flow to include set_search_offset alongside
set_score_threshold and set_search_limit, so the existing
set_search_offset(offset) call resolves without changing search behavior.
Source: Linters/SAST tools
1 participant
Addresses Issue #1980. Both the default listing and the filter-panel sources dropdown call POST /api/search with query: "*". The backend responds with a match_all OpenSearch query and returns results sorted by internal document order (all scores are 1.0 for a match_all). Because the limit was one hard number — 10,000 chunks — large documents that happened to be indexed later, or whose chunks landed past position 10,000 in the shard, simply never appeared in the response. Their filenames were never in the payload, so the frontend never put them in the listing. Searching "Yes_" worked because semantic ranking scores those exact files at the top and surfaces all of them within the first 100 results.
The four changes address this in two complementary ways:
src/auth_context.py + src/api/search.py + src/services/search_service.py — add an offset parameter that flows from the HTTP body all the way into OpenSearch's "from" field. This is the standard OpenSearch pagination mechanism (equivalent to SQL OFFSET). Since offset defaults to 0, every existing caller is completely unaffected.
useGetSearchQuery.ts — the wildcard path now loops: it fetches 1,000 chunks at offset=0, then 1,000 at offset=1000, and so on, merging all chunks into a single fileMap keyed by filename until a page returns fewer than 1,000 chunks (the final page), at which point the loop stops. Every filename in the index is guaranteed to pass through the merge logic. Non-wildcard searches (any real text query) are entirely unchanged.
The aggregation size fixes in search_service.py (data_sources 20→10,000, owners 10→1,000, etc.) resolve the parallel issue where the filter-panel's Document Types, Owners, and Connector Types dropdowns were also silently truncated to their first 10 entries.
Summary by CodeRabbit
New Features
Bug Fixes