Conversation
- 背景 / Background:ASCII-only tokenization dropped CJK queries, making lexical and rare-term ranking signals zero / ASCII-only tokenization removed CJK queries and erased relevance signals. - 改动 / Changes:新增 bounded Unicode tokenizer,Latin/数字保留原语义,Han/Hiragana/Katakana/Hangul 使用 bigram,并支持日文长音符 / Add a bounded Unicode tokenizer with CJK bigrams and Japanese prolonged-mark support while preserving Latin/digit behavior. - 文件 / Files:src/search/core/text-tokenizer.ts, lexical-alignment.ts, rare-terms.ts, related unit tests - 验证 / Verification:24 files / 236 tests passed; npm run lint passed; npm run build passed; git diff --check passed.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change adds a bounded Unicode-aware tokenizer and uses it in lexical alignment and rare-term scoring. Tests cover Latin, numeric, Japanese, Chinese, Korean, and large unsegmented inputs. ChangesCJK ranking signals
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Query
participant lexicalAlignment
participant tokenizeRankingText
participant rareTermFunctions
Query->>lexicalAlignment: tokenize query, title, and snippet
lexicalAlignment->>tokenizeRankingText: tokenize ranking text
tokenizeRankingText-->>lexicalAlignment: return Latin or CJK tokens
Query->>rareTermFunctions: build rare-term signals
rareTermFunctions->>tokenizeRankingText: tokenize query and document text
tokenizeRankingText-->>rareTermFunctions: return bounded token streams
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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 |
|
qqq |
|
Nice work — verified locally that this introduces zero new test failures (the two One thing before merge: the comments in I'll approve CI on the branch so you get the cross-platform run. |
- 背景 / Background:维护者要求将 tokenizer 中的中文注释改为英文,以保持代码库风格一致 / The maintainer requested English comments for consistency across the codebase. - 改动 / Changes:翻译 Script_Extensions、Unicode-aware tokenization 与输入上限相关注释,不改动运行逻辑 / Translated comments about Script_Extensions, Unicode-aware tokenization, and input bounds without changing runtime logic. - 文件 / Files:src/search/core/text-tokenizer.ts - 验证 / Verification:28 targeted tests passed;npm run lint passed;npm run build passed;git diff --check passed;full npm test reached 7837 passed with 22 unrelated failures / 28 targeted tests passed; lint, build, and diff check passed; the full suite completed with 7837 passes and 22 failures outside this comment-only scope.
|
Thanks for the review. I translated all Chinese comments and JSDoc in Verification:
The update is in commit |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/search/core/text-tokenizer.ts`:
- Around line 3-6: Update the CJK_RUN pattern used by TOKEN_PART_RE to use base
Script properties for Han, Hiragana, Katakana, and Hangul, explicitly allowing
only intended non-letter marks such as \u30FC; do not use Script_Extensions so
punctuation like 、, 。, and ・ cannot join CJK runs. Add regression coverage
verifying these punctuation characters are excluded from token parts and
bigrams.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4be0ee1f-1639-487f-af60-b1a123848360
📒 Files selected for processing (1)
src/search/core/text-tokenizer.ts
- 背景 / Background:CodeRabbit 指出 Script_Extensions 会把 、。・并入 CJK run,生成含标点的 bigram / CodeRabbit found that Script_Extensions allowed CJK punctuation into token runs and bigrams. - 改动 / Changes:改用基础 Script 属性并仅显式保留 U+30FC,同时增加 CJK 标点分隔回归测试 / Switched to base Script properties with explicit U+30FC support and added a punctuation-boundary regression test. - 文件 / Files:src/search/core/text-tokenizer.ts, tests/unit/search/core/text-tokenizer.test.ts - 验证 / Verification:先确认新增测试失败,再确认 29 个定向测试、npm run lint、npm run build、git diff --check 通过 / Confirmed the new test failed first, then 29 targeted tests, lint, build, and diff check passed.
|
Follow-up: CodeRabbit identified that |
|
CI follow-up: all lint/build/unit jobs, the Ubuntu full test suite, and 9 of 10 clean-machine smoke jobs passed. The only failure was |
|
Hi @w-up — thank you for this PR, and I'm sorry it's been sitting here without a proper review. I've been heads-down building some pretty big features and improvements for wigolo, and as a solo dev, shipping that while also reviewing and maintaining every issue and PR has been genuinely hard. This one isn't forgotten — it just hasn't had the attention it deserves yet. I'd really appreciate your patience here. I'm going to work through the open PRs and issues properly over the next few weeks (sooner if I can free up), and I'll follow up right here. In the meantime, the wigolo Discord is open if you'd like to follow what's being built, ask questions, or nudge me directly: https://discord.gg/BkUUgz2bNF Thanks again for contributing, and for understanding — it genuinely means a lot. 🙏 |
Summary / 摘要
Fixes #230.
The core ranker used ASCII-only tokenization for lexical alignment and rare-term phrase signals. CJK queries were reduced to an empty token set, so topically relevant results and unrelated fresh calendar/weather pages could receive the same zero lexical score.
核心 ranking 的 lexical alignment 与 rare-term phrase 使用 ASCII-only tokenizer,CJK 查询会被清空,导致主题相关结果无法通过文本相关性压过黄历、天气等无关新内容。
Changes / 改动
Script_Extensionsso Japanese shared marks such asーstay attached:AIニュース→ai,ニュ,ュー,ースlexicalAlignment()sqlite-vec,vec0,vec_distance) unchanged.Regression coverage / 回归测试
rareTermFactor()andisRareTermMiss()distinguish CJK topic hits from misses.Verification / 验证
Scope / 范围
CONTRIBUTING.mdand its CLA terms.Summary by CodeRabbit