From 9ef1c23c4e6b8a06df690174cb7074d3676dd062 Mon Sep 17 00:00:00 2001 From: Andrey Shinkarenko Date: Sat, 1 Aug 2026 15:28:51 +0500 Subject: [PATCH] fix(search): index docstrings and accept non-ASCII query bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/mcp/mcp.c | 18 +++++++-- src/pipeline/pipeline.c | 20 ++++++--- src/pipeline/pipeline_incremental.c | 12 ++++-- src/store/store.c | 15 ++++++- tests/test_mcp.c | 63 +++++++++++++++++++++++++++++ 5 files changed, 114 insertions(+), 14 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index d40571b30d..d542d717b2 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -2769,6 +2769,18 @@ static sqlite3_destructor_type mcp_sqlite_transient(void) { } #define MCP_SQLITE_TRANSIENT (mcp_sqlite_transient()) +/* A byte that may sit inside a query token. ASCII alphanumerics and '_' as + * before, plus every byte >= 0x80: those are UTF-8 lead/continuation bytes, so + * treating them as separators (the previous behaviour) silently dropped every + * non-Latin word from the query — a Cyrillic, Greek or CJK search reached the + * index as an empty match and returned unranked noise. We do not need to + * decode UTF-8 here, only to keep multi-byte sequences intact: the FTS5 + * unicode61 tokenizer applies the real word-boundary rules on both sides. */ +static int bm25_is_token_byte(unsigned char c) { + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '_' || + c >= 0x80; +} + static int bm25_build_match(const char *query, char *out, size_t out_size) { if (!query || !out || out_size < BM25_MIN_BUF) { return 0; @@ -2777,16 +2789,14 @@ static int bm25_build_match(const char *query, char *out, size_t out_size) { int tokens = 0; const char *p = query; while (*p) { - while (*p && !((*p >= 'a' && *p <= 'z') || (*p >= 'A' && *p <= 'Z') || - (*p >= '0' && *p <= '9') || *p == '_')) { + while (*p && !bm25_is_token_byte((unsigned char)*p)) { p++; } if (!*p) { break; } const char *tok_start = p; - while (*p && ((*p >= 'a' && *p <= 'z') || (*p >= 'A' && *p <= 'Z') || - (*p >= '0' && *p <= '9') || *p == '_')) { + while (*p && bm25_is_token_byte((unsigned char)*p)) { p++; } size_t tok_len = (size_t)(p - tok_start); diff --git a/src/pipeline/pipeline.c b/src/pipeline/pipeline.c index 0c30419598..9a33de95d8 100644 --- a/src/pipeline/pipeline.c +++ b/src/pipeline/pipeline.c @@ -1418,16 +1418,26 @@ static int dump_and_persist_hashes(cbm_pipeline_t *p, const cbm_file_info_t *fil * Contentless FTS5 requires the special 'delete-all' command instead of * DELETE FROM to wipe prior rows (there's no underlying content table). * Falls back to plain names if cbm_camel_split is unavailable (which - * shouldn't happen because we always register it, but we stay defensive). */ + * shouldn't happen because we always register it, but we stay defensive). + * + * `docstring` is pulled out of the properties JSON so prose in comments + * is searchable, not just identifiers. It matters most for non-English + * codebases: identifiers are almost always English, so a query written + * in the team's own language matched nothing at all before. The + * unicode61 tokenizer already handles non-Latin scripts. */ CBM_PROF_START(t_fts); cbm_store_exec(hash_store, "INSERT INTO nodes_fts(nodes_fts) VALUES('delete-all');"); if (cbm_store_exec(hash_store, - "INSERT INTO nodes_fts(rowid, name, qualified_name, label, file_path) " - "SELECT id, cbm_camel_split(name), qualified_name, label, file_path " + "INSERT INTO nodes_fts(rowid, name, qualified_name, label, file_path, " + "docstring) " + "SELECT id, cbm_camel_split(name), qualified_name, label, file_path, " + "COALESCE(json_extract(properties, '$.docstring'), '') " "FROM nodes;") != CBM_STORE_OK) { cbm_store_exec(hash_store, - "INSERT INTO nodes_fts(rowid, name, qualified_name, label, file_path) " - "SELECT id, name, qualified_name, label, file_path FROM nodes;"); + "INSERT INTO nodes_fts(rowid, name, qualified_name, label, file_path, " + "docstring) " + "SELECT id, name, qualified_name, label, file_path, " + "COALESCE(json_extract(properties, '$.docstring'), '') FROM nodes;"); } CBM_PROF_END("persist", "5_fts_backfill", t_fts); diff --git a/src/pipeline/pipeline_incremental.c b/src/pipeline/pipeline_incremental.c index 21740ba647..0f32abd636 100644 --- a/src/pipeline/pipeline_incremental.c +++ b/src/pipeline/pipeline_incremental.c @@ -690,12 +690,16 @@ static int dump_and_persist(cbm_gbuf_t *gbuf, const char *db_path, const char *p rc = CBM_STORE_ERR; } if (cbm_store_exec(hash_store, - "INSERT INTO nodes_fts(rowid, name, qualified_name, label, file_path) " - "SELECT id, cbm_camel_split(name), qualified_name, label, file_path " + "INSERT INTO nodes_fts(rowid, name, qualified_name, label, file_path, " + "docstring) " + "SELECT id, cbm_camel_split(name), qualified_name, label, file_path, " + "COALESCE(json_extract(properties, '$.docstring'), '') " "FROM nodes;") != CBM_STORE_OK) { if (cbm_store_exec(hash_store, - "INSERT INTO nodes_fts(rowid, name, qualified_name, label, file_path) " - "SELECT id, name, qualified_name, label, file_path FROM nodes;") != + "INSERT INTO nodes_fts(rowid, name, qualified_name, label, file_path, " + "docstring) " + "SELECT id, name, qualified_name, label, file_path, " + "COALESCE(json_extract(properties, '$.docstring'), '') FROM nodes;") != CBM_STORE_OK) { rc = CBM_STORE_ERR; } diff --git a/src/store/store.c b/src/store/store.c index 4a032970e4..43c3415782 100644 --- a/src/store/store.c +++ b/src/store/store.c @@ -337,9 +337,22 @@ static int init_schema(cbm_store_t *s) { * Fails silently if FTS5 is not compiled in (SQLITE_ENABLE_FTS5). */ { char *fts_err = NULL; + + /* Schema upgrade (#docstring-fts): databases created before the + * `docstring` column exists carry a 4-column nodes_fts, and + * CREATE ... IF NOT EXISTS would leave them that way — the backfill + * INSERT would then fail on every index run. The table is + * contentless, so dropping it loses no source data: the next + * backfill repopulates it from `nodes`. Detect the old shape by + * probing the new column and drop only on failure. */ + if (sqlite3_exec(s->db, "SELECT docstring FROM nodes_fts LIMIT 0;", NULL, NULL, NULL) != + SQLITE_OK) { + sqlite3_exec(s->db, "DROP TABLE IF EXISTS nodes_fts;", NULL, NULL, NULL); + } + int fts_rc = sqlite3_exec(s->db, "CREATE VIRTUAL TABLE IF NOT EXISTS nodes_fts USING fts5(" - " name, qualified_name, label, file_path," + " name, qualified_name, label, file_path, docstring," " content=''," " tokenize='unicode61 remove_diacritics 2'" ");", diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 36252b1f99..63ba297719 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -2203,6 +2203,68 @@ TEST(tool_search_graph_query_honors_file_pattern_issue552) { PASS(); } +/* Identifiers are English almost everywhere, so a team that documents in its + * own language had no working full-text query: the docstring never reached the + * FTS index, and the query tokenizer dropped every non-ASCII byte on the way + * in. Both halves are exercised here — a Cyrillic query must find the node + * whose only Cyrillic text lives in its docstring. */ +TEST(tool_search_graph_query_matches_non_ascii_docstring) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *st = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(st); + + const char *proj = "non-ascii-doc"; + cbm_mcp_server_set_project(srv, proj); + cbm_store_upsert_project(st, proj, "/tmp/non-ascii-doc"); + + cbm_node_t documented = {0}; + documented.project = proj; + documented.label = "Function"; + documented.name = "handle_payment"; + documented.qualified_name = "non-ascii-doc.src.billing.handle_payment"; + documented.file_path = "src/billing.c"; + documented.start_line = 1; + documented.end_line = 9; + documented.properties_json = "{\"docstring\":\"Провести платёж по счёту\"}"; + ASSERT_GT(cbm_store_upsert_node(st, &documented), 0); + + cbm_node_t undocumented = {0}; + undocumented.project = proj; + undocumented.label = "Function"; + undocumented.name = "unrelated_helper"; + undocumented.qualified_name = "non-ascii-doc.src.util.unrelated_helper"; + undocumented.file_path = "src/util.c"; + undocumented.start_line = 1; + undocumented.end_line = 3; + ASSERT_GT(cbm_store_upsert_node(st, &undocumented), 0); + + cbm_store_exec(st, "INSERT INTO nodes_fts(nodes_fts) VALUES('delete-all');"); + ASSERT_EQ(cbm_store_exec(st, + "INSERT INTO nodes_fts(rowid, name, qualified_name, label, " + "file_path, docstring) " + "SELECT id, cbm_camel_split(name), qualified_name, label, file_path, " + "COALESCE(json_extract(properties, '$.docstring'), '') " + "FROM nodes;"), + CBM_STORE_OK); + + char *resp = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":553,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"search_graph\"," + "\"arguments\":{\"project\":\"non-ascii-doc\",\"query\":\"платёж\",\"limit\":10}}}"); + ASSERT_NOT_NULL(resp); + char *inner = extract_text_content(resp); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "search_mode: bm25")); + ASSERT_NOT_NULL(strstr(inner, "handle_payment")); + ASSERT_NULL(strstr(inner, "unrelated_helper")); + + free(inner); + free(resp); + cbm_mcp_server_free(srv); + PASS(); +} + /* Resource discovery methods this server doesn't populate must return EMPTY * lists, not -32601 Method-not-found: clients like Cline probe them on connect * and surface the errors as a failed connection (#958). */ @@ -10332,6 +10394,7 @@ SUITE(mcp) { RUN_TEST(tool_output_regression_gate); RUN_TEST(tool_output_byte_budgets); RUN_TEST(tool_search_graph_query_honors_file_pattern_issue552); + RUN_TEST(tool_search_graph_query_matches_non_ascii_docstring); RUN_TEST(mcp_resource_discovery_methods_return_empty_lists); RUN_TEST(tool_query_graph_basic); RUN_TEST(tool_index_status_no_project);