Repository navigation
Conversation
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
BM25 search was unusable for any project documented in a non-Latin script, for two independent reasons. bm25_build_match() treated only [a-zA-Z0-9_] as token bytes, so a Cyrillic (or Greek, or CJK) query was tokenized to nothing, reached FTS5 as an empty MATCH and returned unranked noise. Bytes >= 0x80 are now token bytes: no UTF-8 decoding is needed here, only keeping multi-byte sequences intact, since the unicode61 tokenizer already applies the real word-boundary rules on both sides. nodes_fts also indexed identifiers only. Identifiers are English in practice even when the team writes prose in another language, so the one field that could match a native-language query was the one field never indexed. A docstring column is added and populated from json_extract(properties, '$.docstring') in both backfill paths. Databases created before this carry the 4-column table, which CREATE ... IF NOT EXISTS would leave in place and every backfill would then fail on, so the new column is probed and the table dropped when missing — it is contentless, nothing is lost, the next backfill repopulates it from nodes. English search improves as a side effect: docstrings carry intent that identifiers alone do not. Fixes #1401 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
8284486 to
9ef1c23
Compare
|
Force-pushed a fix to a defect in my own patch — worth describing, because the first version would have been worse than the bug it fixes. What was wrong. The schema upgrade dropped the 4-column I hit this on my own machine right after installing the patched binary: a previously working project stopped matching even plain Fix. The upgrade is now self-contained — immediately after recreating the table it repopulates from Verified on real databases, reproducing the exact sequence that broke:
The third row is where the previous revision went empty. One behavioural note I should flag: the upgrade path runs when the schema is created, not on read. Upgrading the binary alone therefore neither breaks nor fixes an existing database — it keeps working under the old schema until the project is indexed again. That seemed preferable to migrating on every open, but say the word if you would rather it happened eagerly. Testing after the fix: full suite |
Fixes #1401
BM25 search was unusable for any project documented in a non-Latin script. Two independent causes, both in this diff.
1. The query tokenizer dropped non-ASCII bytes
bm25_build_match()accepted only[a-zA-Z0-9_]as token bytes, so a Cyrillic query was tokenized to nothing and reached FTS5 as an empty MATCH — the caller got unranked noise rather than an empty result, which is why this went unnoticed.Bytes
>= 0x80are now token bytes. No UTF-8 decoding is introduced: the only requirement here is to keep multi-byte sequences intact, since the FTS5unicode61tokenizer already applies the real word-boundary rules on both the index and the query side.2. Docstrings were never indexed
nodes_ftscoveredname, qualified_name, label, file_path— identifiers only. In real projects identifiers are English even when the team documents in another language, so the one field that could match a native-language query was the one field missing from the index.A
docstringcolumn is added and populated fromjson_extract(properties, '$.docstring')in both backfill paths (pipeline.c,pipeline_incremental.c).Schema upgrade. Databases created before this carry the 4-column table, and
CREATE VIRTUAL TABLE IF NOT EXISTSwould leave them that way — every subsequent backfill would fail on the column count. The new column is probed with a cheapSELECT docstring FROM nodes_fts LIMIT 0and the table dropped only when it is missing. The table is contentless, so nothing is lost: the next backfill repopulates it fromnodes.Result
Same project, same query, before and after:
English search improves as well rather than regressing —
validate arguments schemanow ranks_validate_argsfirst, because docstrings carry intent that identifiers alone do not. camelCase splitting is unaffected.Testing
tool_search_graph_query_matches_non_ascii_docstringintests/test_mcp.c: a node whose only Cyrillic text is its docstring must be found by a Cyrillic query, and an unrelated node must not. It fails onmainand passes here.build/c/test-runner, macOS 15.5 arm64).mcpsuite specifically is stable across runs (190 passed, 2 skipped).make -f Makefile.cbm lint-formatcould not run locally —clang-formatis not installed on this machine. The new code follows the surrounding style; happy to adjust if CI disagrees.Scope
No API surface change, no new dependency, no new pipeline pass.
json_extractis already available in the vendored SQLite (3.51.3).