Repository navigation
Conversation
The local search index used tantivy's default tokenizer, which keeps a run of CJK characters as a single token, and matched names with prefix fuzzy search. Japanese/Chinese/Korean text could therefore only be found by its beginning, while short queries matched unrelated documents through the fuzzy distance. - Add a CJK aware tokenizer: each CJK character is a token, other alphanumeric runs stay whole words. - Build the search query with the same analyzer: CJK runs become phrase queries (substring match), other words keep the previous term/fuzzy-prefix matching. Query syntax characters no longer cause parse errors. - Store the index in a new directory and reset the indexed records once when it is created, so existing documents are indexed again. - Declare the impl_from_tantivy feature that flowy-search-pub relies on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Reviewer's GuideAdds CJK-aware indexing and analyzer-matched query construction for reliable Japanese, Chinese, and Korean substring search, while preserving existing Latin fuzzy/prefix behavior. A versioned index directory and workspace record reset ensure documents are reindexed after the tokenizer change, with expanded unit and integration coverage. Sequence diagram for CJK-aware document searchsequenceDiagram
participant User
participant DocumentTantivyState
participant Analyzer as CJKAwareAnalyzer
participant Tantivy as TantivyIndex
User->>DocumentTantivyState: search(query, limit)
DocumentTantivyState->>Analyzer: token_stream(query)
Analyzer-->>DocumentTantivyState: CJK characters and whole-word tokens
alt CJK run
DocumentTantivyState->>Tantivy: PhraseQuery consecutive CJK terms
else Non-CJK word
DocumentTantivyState->>Tantivy: TermQuery content and FuzzyTermQuery name
end
Tantivy-->>DocumentTantivyState: matching documents
DocumentTantivyState-->>User: search results
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 8 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="frontend/rust-lib/flowy-core/src/indexed_data_consumer.rs" line_range="164" />
<code_context>
+ /// once per opened index.
+ pub async fn take_needs_full_reindex(&self) -> bool {
+ match self.state.upgrade() {
+ Some(state) => std::mem::take(&mut state.write().await.created),
+ None => false,
+ }
</code_context>
<issue_to_address>
**Rebuilt index misses existing documents**
When the indexed-record reset fails during a new index rebuild, `take_needs_full_reindex` has already consumed `created`, while the runner only logs the `reset_indexed_records` error and continues; `full_index_unindexed_documents` then skips documents still marked in SQLite, and later opens do not retry, so search misses them.
Keep the reindex request pending or stop the rebuild until `reset_indexed_records` succeeds.
Also at `frontend/rust-lib/flowy-core/src/indexing_data_runner.rs:191-196`.
</issue_to_address>
### Comment 2
<location path="frontend/rust-lib/flowy-core/src/indexing_data_runner.rs" line_range="191-196" />
<code_context>
match SearchFullIndexConsumer::new(&workspace_id_cloned, user_paths.tanvity_index_path(uid)) {
Ok(consumer) => {
+ if consumer.take_needs_full_reindex().await {
+ info!("[Indexing] search index was rebuilt, reindex all documents");
+ if let Err(err) = new_full_indexed_data_writer.reset_indexed_records() {
+ error!("[Indexing] Failed to reset indexed records: {:?}", err);
+ }
+ }
new_full_indexed_data_writer
</code_context>
<issue_to_address>
**Failed documents are marked indexed**
When a document cannot be loaded or the search consumer fails while populating a rebuilt index, `full_index_unindexed_documents` logs the consumer error but still upserts the document's indexed-record row, so later runs skip the document and it remains missing from local search.
Only upsert a document's indexed-record row after its content has been successfully written to the search index.
</issue_to_address>
### Comment 3
<location path="frontend/rust-lib/flowy-core/src/indexing_data_runner.rs" line_range="191-196" />
<code_context>
match SearchFullIndexConsumer::new(&workspace_id_cloned, user_paths.tanvity_index_path(uid)) {
Ok(consumer) => {
+ if consumer.take_needs_full_reindex().await {
+ info!("[Indexing] search index was rebuilt, reindex all documents");
+ if let Err(err) = new_full_indexed_data_writer.reset_indexed_records() {
+ error!("[Indexing] Failed to reset indexed records: {:?}", err);
+ }
+ }
new_full_indexed_data_writer
</code_context>
<issue_to_address>
**Reindex overwrites newer edits**
When a document is edited while the full reindex is processing it, the full indexer can read a document snapshot, then an instant update can write newer content to the shared Tantivy state, and finally the full-index consumer can write its older snapshot over it. The SQL record is then upserted, so subsequent full indexing skips the stale document and local search returns outdated content.
Check document freshness before applying a full-index snapshot, or ensure a snapshot cannot overwrite a newer instant-index write.
</issue_to_address>
### Comment 4
<location path="frontend/rust-lib/flowy-search-pub/src/tokenizer.rs" line_range="67" />
<code_context>
+impl TokenStream for CjkAwareTokenStream<'_> {
+ fn advance(&mut self) -> bool {
+ self.token.text.clear();
+ self.token.position = self.token.position.wrapping_add(1);
+
+ while let Some((offset, c)) = self.chars.next() {
</code_context>
<issue_to_address>
**Tokenizer position test fails**
`token_stream` resets the token position, then `advance` increments it before emitting the first token, so the first token has position 1 rather than 0. `split_cjk_into_characters_test` expects position 0 and fails when the tokenizer tests run.
Initialize the stream position so the first increment yields 0, or change the implementation and expected positions to agree.
Also at `frontend/rust-lib/flowy-search-pub/src/tokenizer.rs:121-132`.
</issue_to_address>
### Comment 5
<location path="frontend/rust-lib/flowy-search-pub/src/tantivy_state.rs" line_range="508-510" />
<code_context>
+ .map(|text| Term::from_field_text(field, text))
+ .collect();
+ let query: Box<dyn Query> = if terms.len() == 1 {
+ Box::new(TermQuery::new(
+ terms[0].clone(),
+ IndexRecordOption::WithFreqs,
+ ))
+ } else {
</code_context>
<issue_to_address>
**Common CJK searches return no results**
When a common single-character CJK query has a BM25 score below 0.4, the single-character `TermQuery` produces a low BM25 score, and the 0.4 threshold filters out matching documents, so users see no results for a valid substring query.
Adjust scoring or the threshold for single-character CJK queries so valid matches are returned.
</issue_to_address>
### Comment 6
<location path="frontend/rust-lib/flowy-search-pub/src/tantivy_state.rs" line_range="529" />
<code_context>
+ // Fuzzy matching for name field (better user experience for typos)
+ clauses.push((
+ Occur::Should,
+ Box::new(FuzzyTermQuery::new_prefix(
+ Term::from_field_text(self.field_name, word),
+ 2,
</code_context>
<issue_to_address>
**Short searches return unrelated names**
When a user searches with a short non-CJK word, `build_query` applies a two-edit fuzzy-prefix query without a minimum-length guard, so unrelated document names match and appear in local search results.
Add a minimum-length guard before applying fuzzy-prefix matching.
</issue_to_address>
### Comment 7
<location path="frontend/rust-lib/flowy-search-pub/src/tokenizer.rs" line_range="67" />
<code_context>
+impl TokenStream for CjkAwareTokenStream<'_> {
+ fn advance(&mut self) -> bool {
+ self.token.text.clear();
+ self.token.position = self.token.position.wrapping_add(1);
+
+ while let Some((offset, c)) = self.chars.next() {
</code_context>
<issue_to_address>
**Punctuation creates false substring matches**
When indexed CJK characters are separated by punctuation or another character that the tokenizer treats as a separator, the tokenizer skips punctuation without adding a position gap, so the phrase query for `映画` matches adjacent indexed tokens in text such as `映,画`, even though that substring is not present. Search returns documents that do not contain the requested CJK substring.
Preserve positional gaps for separators or otherwise prevent phrase queries from matching across them.
Also at `frontend/rust-lib/flowy-search-pub/src/tantivy_state.rs:513`.
</issue_to_address>
### Comment 8
<location path="frontend/rust-lib/flowy-search-pub/src/tokenizer.rs" line_range="21-28" />
<code_context>
+/// without spaces between words, so each character is indexed as its own token.
+pub fn is_cjk(c: char) -> bool {
+ matches!(c as u32,
+ 0x1100..=0x11FF // Hangul Jamo
+ | 0x3005..=0x3007 // 々 〆 〇
+ | 0x3040..=0x30FF // Hiragana, Katakana
+ | 0x3130..=0x318F // Hangul Compatibility Jamo
+ | 0x31F0..=0x31FF // Katakana Phonetic Extensions
+ | 0x3400..=0x4DBF // CJK Unified Ideographs Extension A
+ | 0x4E00..=0x9FFF // CJK Unified Ideographs
+ | 0xAC00..=0xD7AF // Hangul Syllables
+ | 0xF900..=0xFAFF // CJK Compatibility Ideographs
+ | 0xFF66..=0xFF9F // Halfwidth Katakana
</code_context>
<issue_to_address>
**Extended Hangul stays unsplit**
When a document or query contains Hangul Extended-A or Extended-B Jamo, `is_cjk` omits Hangul Jamo Extended-A and Extended-B, so those alphanumeric characters are grouped into whole-word tokens instead of individual characters. Substring queries within those Korean text runs therefore do not match.
Add the Hangul Extended-A and Extended-B ranges to `is_cjk` and cover them with tokenizer and substring-search tests.
</issue_to_address>- Keep the reindex request until the indexed records were reset; a marker file
in the index directory makes a failed attempt retry on the next start.
- Give CJK matches a constant score of at least 1.0, so characters that appear
in most documents are not dropped by the score threshold.
- Leave a position gap at separators, so a phrase never matches across
punctuation ("映画" no longer matches "映,画").
- Allow fewer typos for short latin words in fuzzy name matching.
- Split Hangul Jamo Extended-A/B characters like other CJK characters.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
The local search index used tantivy's default tokenizer, which keeps a run of CJK characters as a single token, and matched names with prefix fuzzy search. Japanese/Chinese/Korean text could therefore only be found by its beginning, while short queries matched unrelated documents through the fuzzy distance.
Feature Preview
PR Checklist
Summary by Sourcery
Improve local document search to support accurate CJK substring matching and safely migrate existing indexes.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests: