Skip to content

fix: default BM25 tokenizer splits CJK characters for bare-term retrieval - #12837

Merged
sjrl merged 2 commits into
deepset-ai:mainfrom
Rainmemery:fix/bm25-cjk-tokenization
Sep 28, 2026
Merged

sjrl merged 2 commits into
deepset-ai:mainfrom
Rainmemery:fix/bm25-cjk-tokenization

Conversation

@Rainmemery

Copy link
Copy Markdown
Contributor

Fixes #12836.

Problem

InMemoryDocumentStore's default bm25_tokenization_regex (r"(?u)\b\w+\b") treats every Hangul syllable as a word character, so a Korean noun never splits from an attached particle — 서울은 tokenizes as the single token 서울은 instead of ["서울", "은"]. A bare-noun query like 서울 then has zero BM25 term overlap with documents where the noun only appears with a particle attached, which is nearly always in Korean prose. The same applies to CJK ideographs and kana.

Fix

Change the default regex so that:

  • Latin/digit/underscore words tokenize exactly as before (no behaviour change for existing users), and
  • each CJK character (Hangul syllables/Jamo, CJK unified ideographs \u4e00-\u9fff, kana \u3040-\u30ff) becomes its own token — the standard char-level tokenization IR systems use for CJK text.
r"[^\W\u1100-\u11ff\u3130-\u318f\uac00-\ud7af\u3040-\u30ff\u4e00-\u9fff]+|[\u1100-\u11ff\u3130-\u318f\uac00-\ud7af\u3040-\u30ff\u4e00-\u9fff]"

Verification

  • _tokenize_bm25("서울은 대한민국의 수도") now yields 서/울/은/… (no noun+particle glued token).
  • bm25_retrieval(query="서울") returns the document whose content only contains 서울은.
  • Latin/digit behaviour unchanged: "Luna is a dog" → ["luna", "is", "a", "dog"], "Seoul 2026" → ["seoul", "2026"]. Existing single-char token tests still pass (34 BM25 tests green in the in-memory store suite).
  • Added regression test test_bm25_tokenization_splits_cjk_characters.

@Rainmemery
Rainmemery requested a review from a team as a code owner September 21, 2026 08:11
@Rainmemery
Rainmemery requested review from sjrl and removed request for a team September 21, 2026 08:11
@vercel

vercel Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

@Rainmemery is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

@sjrl

sjrl commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

@Rainmemery thanks for opening the PR! Please address the failing checks and it would be great if you could figure out why the github diff is so off since I don't think your changes are 2K lines of changes

@Rainmemery
Rainmemery force-pushed the fix/bm25-cjk-tokenization branch from 43dc32b to 057961e Compare September 21, 2026 09:19
@CLAassistant

CLAassistant commented Sep 21, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@Rainmemery

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @sjrl — I've rebuilt the branch and the diff should be clean now.

Root cause: the PR branch was initially created from an outdated snapshot of my fork's main, which is why GitHub showed ~2K lines of unrelated changes. I've rebased the branch onto the latest upstream main (b588b8b) and force-pushed it, so the PR now contains only the 3 intended files:

  • haystack/document_stores/in_memory/document_store.py (+1/-1)
  • test/document_stores/test_in_memory.py (+17)
  • releasenotes/notes/bm25-cjk-tokenization-1a2b3c4d5e6f7.yaml (+8)

Tests are re-running on the new head commit now. Let me know if anything else needs attention.

@sjrl

sjrl commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

@Rainmemery thanks for the updates! We do need you to sign the CLA agreement here #12837 (comment) for us to be able to accept your contribution. So please sign that when you can.

@Rainmemery
Rainmemery force-pushed the fix/bm25-cjk-tokenization branch 2 times, most recently from 90ad00c to 19616b6 Compare September 21, 2026 09:53
@Rainmemery

Copy link
Copy Markdown
Contributor Author

Update: the Unit suite revealed that several o_dict() serialization tests asserted the old default regex (?u)\b\w+\b. Since the default changes with this PR, I updated those expectations to the new regex value. Locally all affected tests pass (575 passed). Waiting for CI to confirm.

@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  haystack/document_stores/in_memory
  document_store.py
Project Total  

This report was generated by python-coverage-comment-action

@Rainmemery

Copy link
Copy Markdown
Contributor Author

@sjrl CLA is now signed — the author email was the issue, I rewrote the commit to the noreply address linked to my account and the CLA bot now reports "All committers have signed the CLA". Thanks for flagging it.

@github-actions github-actions Bot added the type:documentation Improvements on the docs label Sep 21, 2026
@Rainmemery
Rainmemery force-pushed the fix/bm25-cjk-tokenization branch from faaf1e7 to f3364b2 Compare September 21, 2026 21:14
@Rainmemery

Copy link
Copy Markdown
Contributor Author

@sjrl rebased the branch onto the latest main — it was 15 commits behind. The diff is unchanged (still the same 9 files / BM25 CJK tokenizer default), no conflicts. CI is re-running on the new head. Thanks!

@sjrl sjrl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three notes on the character ranges in the new default regex.

Comment thread haystack/document_stores/in_memory/document_store.py Outdated
Comment thread haystack/document_stores/in_memory/document_store.py Outdated
Comment thread haystack/document_stores/in_memory/document_store.py Outdated
Comment thread haystack/document_stores/in_memory/document_store.py Outdated
@heeoneie

Copy link
Copy Markdown

Thanks for the ping @sjrl, and thanks @Rainmemery for picking this up so quickly. I ran the PR's regex against the benchmark from the issue plus a few new ones. It fixes the reported bug, and I would like to flag one consequence before it becomes the default.

All numbers below are haystack-ai==3.1.1, same corpus and same code path, swapping only the tokenizer. The script is at the bottom.

What works

Recall goes from 0/10 to 10/10 on the issue's benchmark, with every relevant document at rank 1. Latin behaviour is unchanged ("Luna is a dog, 서울 2026!" -> ['luna', 'is', 'a', 'dog', '서', '울', '2026']), and the [^\W...] construction that achieves it is neat.

The concern: single-syllable tokens cost precision in Korean

Querying 서울 (Seoul) against a corpus where five of six documents are unrelated:

1. [wrong]  1.250  서점 앞에서 울고 있는 아이를 보았다.   ("a child crying in front of a bookstore")
2. [right]  1.106  서울은 대한민국의 수도이며 인구가 가장 많다.
3. [wrong]  1.053  울타리 너머로 개가 짖고 있었다.        ("a dog barking beyond the fence")

The unrelated document wins because it happens to contain 서 (from 서점, "bookstore") and 울 (from 울고, "crying"). Querying 인천 puts the correct document at rank 3, behind 인사말을 천천히 이어 나갔다.

This is the part I can speak to as a native speaker: a single Hangul syllable is a very weak unit of meaning. 서 on its own can be 書 / 西 / 序 and more, and 울 can be the verb stem "cry" or part of 울타리 ("fence"); neither carries the identity of the word it came from. Splitting Korean into syllables is closer to splitting English into individual letters than into words. This is why Lucene's CJK analyzer emits bigrams rather than unigrams.

Comparison

tokenizer recall@10 top-1 correct 서울 precision 1-syllable query 산 tokens per KO sentence
current default 0/10 0/10 no results 0 hits 6
this PR (unigram) 10/10 10/10 wrong doc at rank 1 3 hits 19
bigram 10/10 10/10 correct at rank 1 0 hits 13
unigram + bigram 10/10 10/10 correct at rank 1 3 hits 32

Bigrams recover the precision but lose single-syllable queries, which is exactly what Lucene's outputUnigrams option exists for; emitting both fixes that at the cost of token count.

One practical obstacle if you do want bigrams: they cannot be expressed in the current API. self.tokenizer = re.compile(bm25_tokenization_regex).findall would need a capturing lookahead for overlapping pairs, and then findall returns groups, so the Latin branch collapses to '' (with one group) or the result becomes tuples (with two). Accepting a callable alongside the regex would be needed, which is a larger change than this PR and may well be out of scope here.

Two smaller notes

  1. In a mixed-language corpus the average document length moves (8.2 -> 20.3 tokens in a 20 KO + 3 EN corpus), which also changes the absolute BM25 scores of the Latin documents (2.937 -> 3.517 for a capital query; the ordering held). Pipelines with a score threshold would need recalibrating, so it may be worth a line in the release note.
  2. The Hangul Jamo ranges (U+1100-U+11FF) split NFD text into individual jamo, which still cannot match an NFC query, so decomposed and composed spellings of the same word remain disjoint exactly as they were before. Not a regression, but the release note's "Hangul syllables/Jamo" phrasing could be read as making decomposed text work.

Where I land

The status quo is unusable for Korean, so this is a clear improvement and I would rather have it than not. But it trades a total-recall failure for a precision failure, and which of those is the better default is a call for the maintainers rather than for me. If you want the precision back, I am happy to open a follow-up PR for a callable-tokenizer API plus a CJK bigram tokenizer, or to contribute the benchmark below as a test so the tradeoff stays visible.

Benchmark script
r"""Tokenizer comparison for haystack PR #12837 (haystack-ai==3.1.1).

Swaps only the BM25 tokenizer, holding corpus, retriever and BM25 algorithm fixed:

  current  the released default,  r"(?u)\b\w+\b"
  unigram  this PR: one token per Hangul syllable / CJK ideograph / kana
  bigram   Lucene CJKBigramFilter style: overlapping 2-grams over CJK runs
  both     bigram + unigram, i.e. Lucene's outputUnigrams=true
"""

import re

from haystack import Document
from haystack.components.retrievers.in_memory import InMemoryBM25Retriever
from haystack.document_stores.in_memory import InMemoryDocumentStore

CJK = r"ᄀ-ᇿ㄰-㆏가-힯぀-ヿ一-鿿"
CURRENT = r"(?u)\b\w+\b"
UNIGRAM = rf"[^\W{CJK}]+|[{CJK}]"
SPLIT = re.compile(rf"[^\W{CJK}]+|[{CJK}]+")
CJK_RUN = re.compile(rf"[{CJK}]+")


def cjk_tokenizer(*, bigrams: bool, unigrams: bool):
    def tokenize(text: str) -> list[str]:
        out: list[str] = []
        for m in SPLIT.finditer(text):
            s = m.group(0)
            if not CJK_RUN.fullmatch(s):
                out.append(s)
                continue
            if bigrams:
                out.extend([s[i : i + 2] for i in range(len(s) - 1)] or [s])
            if unigrams:
                out.extend(list(s))
        return out

    return tokenize


def make_store(mode: str, docs: list[Document]) -> InMemoryDocumentStore:
    regex = CURRENT if mode == "current" else UNIGRAM
    store = InMemoryDocumentStore(bm25_tokenization_regex=regex)
    if mode == "bigram":
        store.tokenizer = cjk_tokenizer(bigrams=True, unigrams=False)
    elif mode == "both":
        store.tokenizer = cjk_tokenizer(bigrams=True, unigrams=True)
    store.write_documents(docs)
    return store


def search(store: InMemoryDocumentStore, query: str, top_k: int = 10) -> list[Document]:
    return InMemoryBM25Retriever(store, top_k=top_k, scale_score=False).run(query=query)["documents"]


# One natural sentence per city, mentioning it once with an attached particle.
CITIES = [
    ("서울", "서울은 대한민국의 수도이며 인구가 가장 많다."),
    ("부산", "부산은 대한민국 제2의 도시이자 최대 항구이다."),
    ("제주", "제주에는 화산 활동으로 만들어진 독특한 지형이 많다."),
    ("대구", "대구를 방문하면 근대 골목길을 걸어볼 수 있다."),
    ("인천", "인천에서 출발하는 국제선 항공편이 가장 많다."),
    ("광주", "광주의 5월은 역사적으로 중요한 의미를 가진다."),
    ("대전", "대전은 과학 연구 단지가 밀집한 도시로 알려져 있다."),
    ("울산", "울산에는 자동차와 조선 산업 단지가 모여 있다."),
    ("세종", "세종으로 여러 중앙 행정 기관이 이전하였다."),
    ("수원", "수원에는 조선 시대에 쌓은 화성이 남아 있다."),
]

# Unrelated sentences that happen to contain 서 or 울 inside other words.
DISTRACTORS = [
    "서점 앞에서 울고 있는 아이를 보았다.",
    "서류를 모두 제출해야 접수가 완료된다.",
    "울타리 너머로 개가 짖고 있었다.",
    "서열을 정하는 회의가 길어졌다.",
    "경기도는 서쪽 해안을 따라 갯벌이 넓다.",
]

SINGLE = ["산이 높고 물이 깊다.", "부산은 항구 도시이다.", "등산을 좋아한다."]

print(f"{'tokenizer':9} {'recall@10':>9} {'top-1':>7} {'서울 precision':>16} {'산 hits':>8} {'KO tokens':>10}")
for mode in ("current", "unigram", "bigram", "both"):
    store = make_store(mode, [Document(content=s, meta={"city": c}) for c, s in CITIES])
    recall = sum(1 for c, _ in CITIES if any(d.meta["city"] == c for d in search(store, c)))
    top1 = sum(1 for c, _ in CITIES if (r := search(store, c)) and r[0].meta["city"] == c)

    precision_docs = [Document(content=CITIES[0][1], meta={"relevant": True})]
    precision_docs += [Document(content=s, meta={"relevant": False}) for s in DISTRACTORS]
    ranked = search(make_store(mode, precision_docs), "서울", top_k=3)
    precision = "no results" if not ranked else ("correct@1" if ranked[0].meta["relevant"] else "WRONG@1")

    single = len(search(make_store(mode, [Document(content=s) for s in SINGLE]), "산", top_k=3))
    tokens = len(store._tokenize_bm25(CITIES[0][1]))
    print(f"{mode:9} {recall:>7}/10 {top1:>5}/10 {precision:>16} {single:>8} {tokens:>10}")

print("\nRanking for the query 서울:")
precision_docs = [Document(content=CITIES[0][1], meta={"relevant": True})]
precision_docs += [Document(content=s, meta={"relevant": False}) for s in DISTRACTORS]
for mode in ("unigram", "bigram"):
    print(f"  {mode}")
    for i, d in enumerate(search(make_store(mode, precision_docs), "서울", top_k=3)):
        print(f"    {i + 1}. [{'right' if d.meta['relevant'] else 'wrong'}] {d.score:6.3f}  {d.content}")

print("\nAverage document length, 20 Korean + 3 English documents:")
ko = [f"서울은 대한민국의 수도이며 인구가 가장 많다. 사례 {i}." for i in range(20)]
en = [
    "Seoul is the capital of South Korea and has the largest population.",
    "Busan is the second largest city in South Korea.",
    "The capital city hosts the national government.",
]
for mode in ("current", "unigram"):
    store = make_store(mode, [Document(content=c) for c in ko + en])
    avgdl = sum(len(store._tokenize_bm25(c)) for c in ko + en) / len(ko + en)
    scores = ", ".join(f"{d.score:.3f}" for d in search(store, "capital", top_k=3))
    print(f"  {mode:9} avgdl={avgdl:5.1f}  scores for the English query 'capital': {scores}")

@heeoneie

Copy link
Copy Markdown

An addendum, and a correction of my own: I wrote the comment above from the diff and the issue thread and missed that @sjrl had already left inline review threads on this file a day earlier. The NFD point I listed as my "smaller note 2" was his first — apologies for restating it as if it were new. His three technical notes all reproduce here, and two of them have a Korean angle worth adding.

NFD (his third thread). Confirmed, and it is worse than "not improved": the two spellings share no terms at all.

_tokenize_bm25("서울")                                  # ['서', '울']
_tokenize_bm25(unicodedata.normalize("NFD", "서울"))     # ['ᄉ', 'ᅥ', 'ᄋ', 'ᅮ', 'ᆯ']
# intersection: set()

Normalizing next to text.lower(), as he suggested, looks right to me. Korean NFD is not exotic: it is what macOS produces for filenames, and text scraped from files or from some Korean CMSes arrives that way.

CJK Compatibility Ideographs, U+F900–FAFF (his second thread). He noted this is the block Korean hanja uses; it is worth being precise about why, because it makes the gap asymmetric rather than merely incomplete. That block exists largely because of Korean: a hanja with several Korean readings gets one code point per reading, so 樂 is U+4E50 when read 락 and U+F914 when read 악. The same visible character therefore tokenizes two different ways:

_tokenize_bm25("樂安民")      # ['樂', '安', '民']      unified, U+4E00–9FFF
_tokenize_bm25("樂例類")      # ['樂例類']              compatibility, U+F900–FAFF

So a Korean document that uses the reading-specific forms gets no benefit from this PR at all, while the same text in unified forms does.

One block missing from his list: halfwidth Hangul jamo, U+FFA0–FFDC.

_tokenize_bm25("ハン")     # ['ハン']     halfwidth katakana, U+FF66–FF9F  (already on the list)
_tokenize_bm25("ᄆᄇᄈ")    # ['ᄆᄇᄈ']    halfwidth Hangul jamo, U+FFA0–FFDC

Same class of gap, same fix. It shows up in the same exported/legacy data that halfwidth katakana comes from.

On his first thread ((?=\w) to intersect the second alternative): for the Hangul ranges specifically it changes almost nothing — of U+3130–U+318F only U+3130 and U+318F are non-word characters, and both are unassigned — so the punctuation problem he found is kana-specific. The lookahead is harmless for Korean, so it is not a tradeoff between the two languages.

None of this changes where I landed above: the precision question is still the one I would want decided before this becomes the default.

…eval

Update the default bm25_tokenization_regex of InMemoryDocumentStore so CJK
scripts are tokenized per character, enabling single-character bare-term
retrieval for Chinese/Japanese/Korean. Also updates to_dict expectations
affected by the new default.

Refinements from review:
- Extract the default pattern into _CJK_CHAR_CLASS /
  _DEFAULT_BM25_TOKENIZATION_REGEX so the source, the to_dict tests and the
  docs reference a single definition instead of a duplicated literal.
- Narrow the kana ranges so punctuation such as the katakana middle dot
  (U+30FB), the kana iteration marks and the combining voicing marks no longer
  become standalone tokens (and no longer inflate doc_len), while keeping the
  prolonged sound mark (U+30FC).
- Cover the remaining CJK/Jamo blocks: halfwidth katakana (U+FF66-U+FF9F),
  CJK Extension A (U+3400-U+4DBF), Katakana Phonetic Extensions
  (U+31F0-U+31FF), CJK Compatibility Ideographs (U+F900-U+FAFF) and Hangul
  Jamo Extended-A/B (U+A960-U+A97F, U+D7B0-U+D7FF).
- Unicode NFC-normalize the text in _tokenize_bm25 so equivalent spellings
  (for example decomposed vs. composed Hangul) share the same terms.
- Expand the bm25_tokenization_regex docstring and update the two
  hand-maintained docs pages (documentcleaner, documentsplitter) to the new
  serialized default.
@Rainmemery
Rainmemery force-pushed the fix/bm25-cjk-tokenization branch from f3364b2 to 6c7f7ab Compare September 23, 2026 06:22
@heeoneie

Copy link
Copy Markdown

Checked the new revision against the Korean cases. Two of the three things I raised are covered, and one block is still missing.

NFC normalization — works. The two spellings now share all their tokens, where before they shared none:

_tokenize_bm25("서울은")                                   # ['서', '울', '은']
_tokenize_bm25(unicodedata.normalize("NFD", "서울은"))      # ['서', '울', '은']

CJK Compatibility Ideographs — works. _tokenize_bm25("樂例類") (U+F914/U+F9B5/U+F9D0) now gives ['樂', '例', '類'] instead of one token, so the reading-specific hanja forms Korean text uses get the same treatment as the unified ones.

Still missing: halfwidth Hangul jamo, U+FFA0–U+FFDC. The new class picks up halfwidth katakana at ヲ-゚ and stops there, so the block immediately after it is still one run:

_tokenize_bm25("ハン")       # ['ハ', 'ン']    halfwidth katakana, U+FF66–FF9F
_tokenize_bm25("ᄆᄇᄈ")      # ['ᄆᄇᄈ']       halfwidth Hangul jamo, U+FFA0–FFDC

It is the Korean counterpart of the halfwidth katakana you added, from the same legacy and exported data, and it is one more range in _CJK_CHAR_CLASS:

r"ᅠ-ᅵ"  # Halfwidth Hangul Jamo

(U+FFA0 is the halfwidth filler and U+FFA1–FFDC the jamo; the halfwidth punctuation you deliberately excluded is below at U+FF61–FF65, so this range adds no punctuation.)

Everything else in the revision reads well to me — pulling the pattern into _DEFAULT_BM25_TOKENIZATION_REGEX so the tests and docs reference one constant should keep this from drifting again.

The open question from my earlier comment is unchanged: per-character tokens still rank an unrelated document above the right one for 서울, and whether that tradeoff is the right default is the call I would want made before this lands.

@sjrl

sjrl commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

@heeoneie thanks for all of your comments. In regards to

The status quo is unusable for Korean, so this is a clear improvement and I would rather have it than not. But it trades a total-recall failure for a precision failure, and which of those is the better default is a call for the maintainers rather than for me. If you want the precision back, I am happy to open a follow-up PR for a callable-tokenizer API plus a CJK bigram tokenizer, or to contribute the benchmark below as a test so the tradeoff stays visible.

I agree that the best solution would be to move to a bigram tokenizer. And I also agree that adding such a change is should be a new issue and new PR and is probably out of scope for this PR. That being said would you say the changes in this PR still be worth adding even with its limitations?

I also wanted to ask what your use case is for using the InMemoryDocumentStore? Since I believe for proper full-language support moving to one of our integrations like OpenSearch or ElasticSearch should have better builtin Korean support and the InMemoryDocumentStore was originally intended for prototyping.

@heeoneie

Copy link
Copy Markdown

Is it worth adding as it stands? Yes. I would take this over the status quo without hesitation, for one reason above the others: the current failure is silent. A Korean query returns an empty list, with no error and no warning, and nothing in the result distinguishes "your tokenizer cannot see this word" from "no document matched". Noisy results are a problem a user can see and reason about; an empty list is one they attribute to their own data, their chunking, or their embedding model. In the benchmark on the issue, ten out of ten topics returned nothing at all, and the same corpus in English returned all ten.

The precision cost is real, but it is bounded and visible, and it can be tracked in the follow-up issue rather than blocking this.

On my use case — I should be straightforward: I am not running InMemoryDocumentStore in production. I was working through how Korean is handled across retrieval stacks, came to this default, and wrote the benchmark to check whether what I suspected from reading r"(?u)\b\w+\b" actually happened. So the issue comes from a deliberate check, not from something that broke on me.

That said, I would gently push back on "it was originally intended for prototyping" as a reason to weigh this less. Prototyping is where a silent failure does the most damage, because the person hitting it has the least basis to suspect the library: they are usually new to Haystack, and if their first Korean retrieval returns nothing they are more likely to conclude that Haystack does not work for Korean than to go read the tokenization regex. InMemoryDocumentStore is also what the tutorials and the quickstart use, so for a Korean-speaking newcomer it is not a niche component — it is the first one they touch. The OpenSearch and Elasticsearch integrations are genuinely better for full-language support, but reaching for them is a step someone takes after they know they need a Korean analyzer, and this default is what they meet before that.

On the follow-up. I would be glad to open the issue for the bigram tokenizer and write it up with the numbers I posted above: the precision comparison, the single-syllable-query tradeoff that makes pure bigrams insufficient on their own (Lucene solves it with outputUnigrams), and the API obstacle — overlapping bigrams cannot be expressed as a findall regex, so it needs a callable alongside bm25_tokenization_regex. Say the word and I will open it; I am happy to take the PR too, and to contribute the benchmark as a test so the tradeoff stays visible in CI rather than living in a comment thread.

@sjrl

sjrl commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

@heeoneie thanks for the write up! Okay lets leave this PR as-is then and please open an issue where we can discuss what changes would be needed before opening a new PR.

@heeoneie

Copy link
Copy Markdown

Opened #12867 with the numbers, the findall obstacle and the two API options (callable vs. a serializable mode string). Happy to take the PR once the direction is settled there.

Comment thread releasenotes/notes/bm25-cjk-tokenization-1a2b3c4d5e6f7.yaml Outdated
Comment thread haystack/document_stores/in_memory/document_store.py Outdated
Comment thread test/document_stores/test_in_memory.py
Comment thread test/document_stores/test_in_memory.py Outdated
- Add halfwidth Hangul Jamo (U+FFA0-U+FFDC) to the CJK char class
- Shorten the release note and drop the middle-dot claim (it was never
  a token on main)
- Stop referencing private names in the public docstring, use single
  backticks
- Assert full token lists in the CJK tokenization test
- Add a retrieval test for the bare-term CJK query the PR fixes

@heeoneie heeoneie left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All four are addressed, and I verified them against 7b9f785 rather than just reading the replies. Thanks for the quick turnaround.

Halfwidth Hangul range. U+FFA0–U+FFDC is exactly the block: U+FFA0 is HALFWIDTH HANGUL FILLER and U+FFDC is HALFWIDTH HANGUL LETTER I, U+FF9F below it is the katakana semi-voiced mark already covered, and U+FFDD above it is unassigned. There are nine unassigned code points inside the range, which is harmless in a character class. Including the filler is also consistent with U+3164 HANGUL FILLER, which the Hangul Compatibility Jamo range already picks up.

Dropping the middle-dot sentence was right. I checked both directions: U+30FB is category Po, so \w never matched it on main, and the new katakana range \u30a1-\u30fa\u30fc deliberately steps around it, so it is not emitted before or after this PR. The sentence was claiming a fix that was never a bug.

Test assertions. I ran the new regex over the four new/changed cases and they all hold, including \uffb1\uffb2\uffb3 → three tokens. _tokenize_bm25 does unicodedata.normalize("NFC", text.lower()), so the shortened docstring's "lowercased and NFC-normalized" is accurate.

Korean reads correctly. The two sentences in the new retrieval test are natural, and 서울은 / 부산은 are the right shape for the bug being pinned.

Two non-blocking notes:

  1. test_bm25_retrieval_with_cjk_bare_term_query uses top_k=1. The 부산 document shares no token with the query 서울, so it scores 0 and is dropped regardless; len(results) == 1 is therefore satisfied by construction. With top_k=2 the same test would also assert that the non-matching document stays out, which is the other half of what the fix does. Entirely optional.

  2. Worth knowing rather than fixing: NFC does not fold halfwidth or compatibility jamo into syllables (that is NFKC), so halfwidth-jamo text still only matches halfwidth-jamo queries. That is the same behavior halfwidth katakana already has here, so the PR is self-consistent; I mention it only so nobody later reads the halfwidth addition as making the forms interchangeable.

LGTM from the native-speaker side.

@sjrl sjrl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@sjrl
sjrl merged commit 6704f39 into deepset-ai:main Sep 28, 2026
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:tests type:documentation Improvements on the docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InMemoryDocumentStore's default BM25 tokenizer never splits a Korean noun from an attached particle — 0/10 recall in a same-structure EN/KO benchmark

4 participants