Skip to content

fix(search): index docstrings and accept non-ASCII query bytes #1403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Metatau wants to merge 1 commit into from
Closed

fix(search): index docstrings and accept non-ASCII query bytes #1403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions src/mcp/mcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
20 changes: 15 additions & 5 deletions src/pipeline/pipeline.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
12 changes: 8 additions & 4 deletions src/pipeline/pipeline_incremental.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
15 changes: 14 additions & 1 deletion src/store/store.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
");",
Expand Down
63 changes: 63 additions & 0 deletions tests/test_mcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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). */
Expand Down Expand Up @@ -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);
Expand Down
Loading