feat: tokenized item search — all terms must match, quoted phrases supported#1542
Closed
fedorlitau wants to merge 4 commits into
Closed
feat: tokenized item search — all terms must match, quoted phrases supported#1542fedorlitau wants to merge 4 commits into
fedorlitau wants to merge 4 commits into
Conversation
Splits search input on whitespace into tokens, keeping double-quoted segments as single tokens for exact-phrase matching. Groundwork for tokenized item search (sysadminsmedia#397). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Search input is now split into tokens (with double-quoted phrases kept intact); each token must match at least one searchable field, so "item blue" finds "Item, long description, blue" regardless of word order. Single-token searches behave exactly as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cyclo limit Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Summary by CodeRabbit
WalkthroughThis PR implements tokenized multi-term search for the entity repository. Instead of matching a single search string across fields with OR logic, queries are now split into tokens (with quoted phrase support), and all tokens must match at least one searchable field. A new ChangesTokenized Search for Entities
Security RecommendationsInput validation and quote handling:
Query predicate construction:
Performance considerations:
🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested Reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
docs/src/content/docs/en/user-guide/tips-tricks.mdx (3)
25-28: ⚡ Quick winConsider mentioning case-insensitivity explicitly.
The search is case-insensitive (using
ContainsFoldin the implementation), but this isn't stated in the documentation. Adding a brief note would help users understand thatdrillandDRILLare equivalent.📝 Suggested addition
The search bar splits your input into individual terms, and every term must match somewhere in the item (name, description, serial number, model number, manufacturer, or notes). Each additional term narrows the results: searching -`drill cordless` only returns items that match both `drill` and `cordless`, even if the words appear in different -fields or in a different order. +`drill cordless` only returns items that match both `drill` and `cordless`, even if the words appear in different fields +or in a different order. Searches are case-insensitive, so `drill`, `Drill`, and `DRILL` are all equivalent.🤖 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 `@docs/src/content/docs/en/user-guide/tips-tricks.mdx` around lines 25 - 28, The documentation text about how the search splits input lacks mention of case-insensitivity; update the paragraph describing search behavior to add a short sentence such as "Searches are case-insensitive (e.g., 'drill' and 'DRILL' are equivalent)" and optionally note the implementation uses ContainsFold for matching by referencing ContainsFold in the docs so readers know the matching is case-insensitive; modify the existing paragraph around the sentence that starts with "The search bar splits your input..." to include this brief note.
30-31: 💤 Low valueConsider documenting unbalanced quote behavior.
The implementation treats an unbalanced quote as part of the final token, but users might not expect this behavior. While this is an edge case, a brief note could prevent confusion.
📝 Optional addition
To match an exact phrase, wrap it in double quotes: `"blue storage box"` only matches items containing that exact -text. Quoted phrases and plain terms can be combined, e.g. `drill "18 V"`. +text. Quoted phrases and plain terms can be combined, e.g. `drill "18 V"`. If a quote is not closed, it will be +treated as part of the search term.🤖 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 `@docs/src/content/docs/en/user-guide/tips-tricks.mdx` around lines 30 - 31, Add a brief note to the existing tip about quoted phrases (the sentence using `"blue storage box"` and the `drill "18 V"` example) explaining that unbalanced quotes are treated as part of the final token by the search/tokenizer (i.e., a missing closing quote will be included in the last token rather than causing an error), so users understand the edge-case behavior; place this one-line clarification immediately after the current examples.
23-32: ⚡ Quick winClarify how the # asset ID syntax interacts with tokenized search.
The file documents the
#000-001asset ID search syntax at line 41, but the new Search Syntax section doesn't mention whether this special syntax is affected by tokenization. Users might wonder if#gets tokenized or if quotes work with asset IDs.📝 Suggested addition
Add a note after line 31:
To match an exact phrase, wrap it in double quotes: `"blue storage box"` only matches items containing that exact text. Quoted phrases and plain terms can be combined, e.g. `drill "18 V"`. + +> [!NOTE] +> The special `#` syntax for searching asset IDs (see [Managing Asset IDs](`#managing-asset-ids`)) works independently +> of the tokenized search described above.🤖 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 `@docs/src/content/docs/en/user-guide/tips-tricks.mdx` around lines 23 - 32, The Search Syntax section needs a clarifying note about the special asset ID syntax `#000-001`: explain that the `#` prefix and the full asset ID are treated as a literal identifier and are not subject to tokenized term matching, so searching `#000-001` will match that exact asset ID even if tokenization would otherwise split terms; also state that quoted phrases are optional for asset IDs (quotes may be used but are not required) and that combining an asset ID with other terms still requires all terms to match. Add this brief note after the Search Syntax paragraph referring to the `#000-001` example so users understand how asset ID searches interact with tokenization and quotes.backend/internal/data/repo/repo_items_search_test.go (1)
252-341: ⚡ Quick winWell-structured test with comprehensive coverage.
The test cases thoroughly validate the tokenized search behavior including AND semantics, case-insensitivity, multi-field matching, and quoted phrase handling.
📋 Optional: Consider adding test cases for mixed quoted/unquoted tokens
The PR description mentions that quoted and unquoted terms can be combined, but there's no test case validating this. Adding test cases like:
{ name: "mixed quoted and unquoted tokens", search: `blue "long description"`, contains: []EntityOut{blueItem, blueObject}, excludes: []EntityOut{redItem, drill}, }, { name: "unbalanced quote treated as plain token", search: `Item "blue`, contains: []EntityOut{blueItem}, excludes: []EntityOut{redItem, blueObject, drill}, },This would provide additional confidence that the tokenizer and predicate logic work correctly together for complex real-world queries.
🤖 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 `@backend/internal/data/repo/repo_items_search_test.go` around lines 252 - 341, Add two test cases to TestEntityRepository_TokenizedSearch to cover mixed quoted/unquoted tokens and unbalanced quotes: update the testCases slice used by searchIDs (which calls tRepos.Entities.QueryByGroup with EntityQuery) to include a "mixed quoted and unquoted tokens" case like `blue "long description"` expecting blueItem and blueObject to match, and an "unbalanced quote treated as plain token" case like `Item "blue` expecting blueItem only; ensure the new cases use the same contains/excludes pattern with EntityOut values so the assertions after calling searchIDs validate tokenizer+predicate behavior for these scenarios.
🤖 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.
Nitpick comments:
In `@backend/internal/data/repo/repo_items_search_test.go`:
- Around line 252-341: Add two test cases to
TestEntityRepository_TokenizedSearch to cover mixed quoted/unquoted tokens and
unbalanced quotes: update the testCases slice used by searchIDs (which calls
tRepos.Entities.QueryByGroup with EntityQuery) to include a "mixed quoted and
unquoted tokens" case like `blue "long description"` expecting blueItem and
blueObject to match, and an "unbalanced quote treated as plain token" case like
`Item "blue` expecting blueItem only; ensure the new cases use the same
contains/excludes pattern with EntityOut values so the assertions after calling
searchIDs validate tokenizer+predicate behavior for these scenarios.
In `@docs/src/content/docs/en/user-guide/tips-tricks.mdx`:
- Around line 25-28: The documentation text about how the search splits input
lacks mention of case-insensitivity; update the paragraph describing search
behavior to add a short sentence such as "Searches are case-insensitive (e.g.,
'drill' and 'DRILL' are equivalent)" and optionally note the implementation uses
ContainsFold for matching by referencing ContainsFold in the docs so readers
know the matching is case-insensitive; modify the existing paragraph around the
sentence that starts with "The search bar splits your input..." to include this
brief note.
- Around line 30-31: Add a brief note to the existing tip about quoted phrases
(the sentence using `"blue storage box"` and the `drill "18 V"` example)
explaining that unbalanced quotes are treated as part of the final token by the
search/tokenizer (i.e., a missing closing quote will be included in the last
token rather than causing an error), so users understand the edge-case behavior;
place this one-line clarification immediately after the current examples.
- Around line 23-32: The Search Syntax section needs a clarifying note about the
special asset ID syntax `#000-001`: explain that the `#` prefix and the full
asset ID are treated as a literal identifier and are not subject to tokenized
term matching, so searching `#000-001` will match that exact asset ID even if
tokenization would otherwise split terms; also state that quoted phrases are
optional for asset IDs (quotes may be used but are not required) and that
combining an asset ID with other terms still requires all terms to match. Add
this brief note after the Search Syntax paragraph referring to the `#000-001`
example so users understand how asset ID searches interact with tokenization and
quotes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 137a6328-a6e3-48c7-afa7-7e89534fd668
📒 Files selected for processing (5)
backend/internal/data/repo/repo_entities.gobackend/internal/data/repo/repo_items_search_test.gobackend/pkgs/textutils/tokenize.gobackend/pkgs/textutils/tokenize_test.godocs/src/content/docs/en/user-guide/tips-tricks.mdx
Contributor
|
Please note that this is being superseded by #1550 (which includes major search refactoring). Please consider going over that PR and checking that it implements tokenizing correctly. |
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.
What type of PR is this?
What this PR does / why we need it:
Item search currently treats the whole query as one literal substring, so
blue chaironly matches items containing that exact phrase. This PR tokenizesthe query and requires every token to match at least one of the six searched
fields (AND across tokens, OR across fields), with double-quoted phrases kept as
single tokens.
backend/pkgs/textutils/tokenize.go(new):TokenizeSearchQuery()splits onUnicode whitespace; double-quoted segments become single tokens (quotes
stripped, inner whitespace preserved); an unbalanced quote degrades gracefully
to a plain token. 13 table-driven unit tests in
tokenize_test.go.backend/internal/data/repo/repo_entities.go:QueryByGroupnow builds itssearch predicate via a new
entitySearchPredicate()helper — AND overper-token six-field
ContainsFoldORs (previously a single six-field OR overthe raw query). Extracted as a helper to stay under the gocyclo limit.
backend/internal/data/repo/repo_items_search_test.go: 8 DB-backedintegration subtests (
TestEntityRepository_TokenizedSearch).docs/src/content/docs/en/user-guide/tips-tricks.mdx: new "Search Syntax"section documenting the behavior.
Design decisions: AND semantics across tokens; tokens + quoted phrases only (no
boolean operators in this iteration). The
#<assetID>search shortcut isintentionally untouched.
Which issue(s) this PR fixes:
Implements the tokenized-search part of #438 (originally requested in #397).
Special notes for your reviewer:
QueryByGroupsits exactly at the gocyclo limit, which is why the predicatelives in a separate helper. No API surface change, so swagger/typescript-types
were not regenerated.
Testing
go test ./pkgs/textutils/... ./internal/data/repo/...— all pass (unit +DB-backed integration tests).
golangci-lint run ./...— no issues in changed files (one pre-existingstaticcheck hit in
app/api/main.gounrelated to this PR).SQLite DB.