feat(search): add multilingual lexical fields - #180
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
@steipete Could you review this opt-in multilingual lexical search implementation? It keeps the default path unchanged, uses independent FTS5 fields with RRF, and includes live Kiwi/Sudachi/Jieba/Arabic E2E plus a reproducible targeted benchmark. GitHub did not allow the fork author to set a formal reviewer through |
|
Codex review: needs changes before merge. Reviewed August 25, 2026, 3:16 AM ET / 07:16 UTC. ClawSweeper reviewWhat this changesThis PR adds opt-in Korean, Japanese, Chinese, and Arabic lexical search indexes with language-specific tokenizers and reciprocal-rank fusion. Merge readinessKeep open: the prior index-rebuild blocker remains on the unchanged head, and the new persistent search/config surface also needs maintainer product direction. Priority: P2 Review scores
Verification
How this fits togetherDiscrawl stores Discord messages in SQLite and searches those local archives from its CLI. The optional lexical path tokenizes messages and queries into language-specific FTS tables, then merges those results with the default search index. flowchart LR
A[Discord message archive] --> B[Language tokenizer]
B --> C[Language-specific FTS indexes]
A --> D[Default FTS index]
E[Search query] --> F[Query tokenizer]
F --> C
F --> D
C --> G[Rank fusion]
D --> G
G --> H[CLI search results]
Decision needed
Why: This is a new configuration and distribution surface spanning native and Go helper dependencies; source review cannot determine the intended long-term product boundary. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Persist an effective per-language analyzer identity with each index, rebuild on mismatch, and land the optional helper design only if its core maintenance boundary is explicitly sponsored. Do we have a high-confidence way to reproduce the issue? Yes: create a lexical index with one helper or model setting, reopen with a different setting, then search a term whose tokenization differs. The constant stored version prevents the required rebuild. Is this the best way to solve the issue? No: disabled-to-enabled invalidation is covered, but changing an enabled analyzer's command or model is not safely handled; identity-based invalidation is the narrower durable repair. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found, but no applicable review policy affected this item. Codex review notes: model internal, reasoning high; reviewed against 5ab5d11351b9. LabelsLabel justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (11 earlier review cycles; latest 8 shown)
|
|
@clawsweeper[bot] I addressed the review in commit e581712 and would appreciate a re-review. Resolved items:
Additional live check: a clean Full |
|
@clawsweeper[bot] Korean tokenization has been reworked in commit 89ea8a1; please re-review this revision. The previous Review-relevant changes:
Live evidence:
|
Replace remaining ja/zh/ar Python workers with Go analyzers. Japanese and Chinese now run as optional Kagome and GSE helpers so the default binary stays small and CGO-free. Arabic light stemming is in-process.
|
@clawsweeper[bot] Python is now fully removed from the lexical path in 58e41fb. Please re-review. What changed:
Live proof, no Python:
Gates: lint/gosec/govulncheck, 85.0% coverage, six-target snapshot, and |
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
@clawsweeper Can you assign a reviewer? |
Summary
[search.lexical]fields for Korean, Japanese, Chinese, and Arabicgithub.com/codingpot/kiwigohelper and native Kiwi 0.23.2discrawl-jahelper (github.com/ikawaha/kagome/v2Search mode)discrawl-zhhelper (github.com/go-ego/gseCutSearch)unicode61path is unchanged when no lexical languages are configuredConfiguration
discrawl lexical installnow reports required helpers. It does not download packages or invoke pip.Live verification
Four-language live E2E and quality benchmark passed with no Python process:
Helper protocol smoke tests:
discrawl-jatokenized東京都庁に行きますto東京 都庁 都 庁 に 行き 行く ますdiscrawl-zhtokenized自然语言处理很有趣to include语言/自然/处理Verification
make checklint, gosec, govulncheck, 85.0% coverage, smoke, and six-target snapshot passedgo test -race ./...passed on rerun after an unrelatedinternal/sharegit-hardlink flakekiwipiepy,sudachipy, Jieba, or Snowball Python lexical workersLicense boundary
Kiwi/kiwigo remain LGPL-2.1 and are not linked into the default MIT binary. Kagome is MIT; GSE is Apache-2.0. Both live only in optional helper modules. Notices are in
THIRD_PARTY_NOTICES.md.