From 01c7e549ef633d147191e2b06097b39227c05f7c Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 28 Aug 2026 18:05:30 +0200 Subject: [PATCH 1/3] feat(search): index prose in BM25 via a nodes_fts body column Closes #518 (Section nodes don't index body text) and #519 (META.yaml / frontmatter description values not indexed). A question asked in words could not reach the nodes that hold the words: Section carried only its heading, a config Module only its path, and both labels were filtered out of BM25 results outright. nodes_fts gains a fifth column, `body`, backfilled from each node's EXISTING properties.docstring. No new column on `nodes`, no new table, and CBM_INDEX_FORMAT_VERSION deliberately does NOT move -- nobody is forced to reindex. A legacy four-column table survives untouched under CREATE VIRTUAL TABLE IF NOT EXISTS: the backfill probes for the column before naming it, and bm25()'s surplus fifth weight is inert on a table that has no fifth column (FTS5 reads a weight only for a column an instance landed in). Such a database keeps opening and searching, just without prose. Two node kinds gain the docstring they lacked: - Markdown section bodies (#518) -- the blocks following a heading, up to the next heading or subsection, collapsed to single spaces and capped at MAX_COMMENT_LEN (500 B) with a UTF-8 backoff that never splits a multi-byte sequence. - Top-level YAML/JSON description / summary / purpose (#519), promoted onto the file's Module node. Section and Module are no longer excluded from BM25 results. Both the ranked query and the mirrored count query were changed together; removing the filter from only one desynchronises results from counts. Column weights are bm25(nodes_fts, 1.0, 1.0, 1.0, 1.0, 0.3): the four identifier columns keep parity with each other and prose sits well below them, so a prose-only hit surfaces but never outranks an identifier match. The FTS backfill is consolidated into one writer, cbm_store_fts_rebuild(), used by the wholesale rebuild AND the delta merge. That second site matters: it had its own hand-written four-column INSERT, which with a fifth column would leave body NULL for every node arriving by delta -- silently, on the warm path most users hit, while a full reindex looked perfect. There is now exactly one place the column list is decided, and a test that fails if the delta path stops writing prose. The backfill guards json_extract with json_valid: malformed properties JSON exists in pre-fix databases and would otherwise abort the whole rebuild. Tests: markdown section bodies (own body, stops at the next heading, UTF-8 safe cap, empty body stays NULL), YAML/JSON description promotion (block scalars, synonym fallback, top-level only, config-only), FTS body indexing and its column isolation, malformed-JSON tolerance, legacy four-column degrade, incremental watermark, BM25 finding Section and Module by prose, results/total consistency, identifier-outranks-prose, and the delta-merge regression. Signed-off-by: Martin Vogel Co-authored-by: ShauryaaSharma <196896785+ShauryaaSharma@users.noreply.github.com> Signed-off-by: Martin Vogel --- internal/cbm/extract_defs.c | 268 +++++++++++++++++++++++++++++++++- src/mcp/mcp.c | 54 +++++-- src/pipeline/pipeline.c | 21 +-- src/pipeline/pipeline_delta.c | 122 +++++++++------- src/store/store.c | 109 +++++++++++++- src/store/store.h | 16 ++ tests/test_extraction.c | 214 +++++++++++++++++++++++++++ tests/test_mcp.c | 242 ++++++++++++++++++++++++++++++ tests/test_pipeline.c | 72 +++++++++ tests/test_store_search.c | 172 ++++++++++++++++++++++ 10 files changed, 1200 insertions(+), 90 deletions(-) diff --git a/internal/cbm/extract_defs.c b/internal/cbm/extract_defs.c index 6982d6c130..dfef6c3f98 100644 --- a/internal/cbm/extract_defs.c +++ b/internal/cbm/extract_defs.c @@ -3861,7 +3861,132 @@ static const char *qn_safe_segment(CBMArena *a, const char *name) { return out; } -static void push_simple_class_def(CBMExtractCtx *ctx, TSNode node, char *name, const char *label) { +/* UTF-8 sequence classification: a lead byte's high bits name the sequence + * length, and every continuation byte matches 10xxxxxx. */ +enum { + UTF8_CONT_MASK = 0xC0, /* isolate the two high bits ... */ + UTF8_CONT_MARK = 0x80, /* ... which are 10 on a continuation byte */ + UTF8_LEAD2_MASK = 0xE0, /* 110xxxxx → 2-byte sequence */ + UTF8_LEAD2_MARK = 0xC0, + UTF8_LEAD3_MASK = 0xF0, /* 1110xxxx → 3-byte sequence */ + UTF8_LEAD3_MARK = 0xE0, + UTF8_LEAD4_MASK = 0xF8, /* 11110xxx → 4-byte sequence */ + UTF8_LEAD4_MARK = 0xF0, + UTF8_LEN_1 = 1, + UTF8_LEN_2 = 2, + UTF8_LEN_3 = 3, + UTF8_LEN_4 = 4, +}; + +/* Bytes the sequence starting with `lead` occupies (1 for ASCII or a byte that + * is not a valid lead). */ +static size_t utf8_sequence_len(unsigned char lead) { + if ((lead & UTF8_LEAD2_MASK) == UTF8_LEAD2_MARK) { + return UTF8_LEN_2; + } + if ((lead & UTF8_LEAD3_MASK) == UTF8_LEAD3_MARK) { + return UTF8_LEN_3; + } + if ((lead & UTF8_LEAD4_MASK) == UTF8_LEAD4_MARK) { + return UTF8_LEN_4; + } + return UTF8_LEN_1; +} + +/* Drop a trailing PARTIAL UTF-8 sequence left behind by a byte-length cut, so + * a capped prose value never ends mid-codepoint (#1017's rule, applied to the + * prose bodies below rather than to comments). */ +static void utf8_trim_partial_tail(char *text) { + size_t n = strlen(text); + if (n == 0) { + return; + } + size_t i = n; + while (i > 0 && ((unsigned char)text[i - UTF8_LEN_1] & UTF8_CONT_MASK) == UTF8_CONT_MARK) { + i--; + } + if (i == 0) { + text[0] = '\0'; /* continuation bytes only — not decodable */ + return; + } + size_t need = utf8_sequence_len((unsigned char)text[i - UTF8_LEN_1]); + if (i - UTF8_LEN_1 + need > n) { + text[i - UTF8_LEN_1] = '\0'; + } +} + +/* Collapse `len` bytes of raw prose into a single-spaced value capped at + * MAX_COMMENT_LEN (the same 500-byte ceiling docstrings already use, which + * leaves room inside the 2 KB properties buffer that carries it). + * + * The output buffer is FIXED at that cap, so a section body of any size costs a + * bounded copy rather than a copy of the whole section. Returns NULL when + * nothing but whitespace was there. */ +static char *collapse_prose(CBMArena *a, const char *src, size_t len) { + if (!src || len == 0) { + return NULL; + } + char *out = (char *)cbm_arena_alloc(a, MAX_COMMENT_LEN + NULL_TERM); + if (!out) { + return NULL; + } + size_t w = 0; + bool in_ws = true; /* start true so leading whitespace is swallowed */ + for (size_t i = 0; i < len && w < MAX_COMMENT_LEN; i++) { + unsigned char c = (unsigned char)src[i]; + if (c == ' ' || c == '\t' || c == '\n' || c == '\r') { + in_ws = true; + continue; + } + if (in_ws && w > 0) { + out[w++] = ' '; + if (w >= MAX_COMMENT_LEN) { + break; + } + } + in_ws = false; + out[w++] = (char)c; + } + out[w] = '\0'; + utf8_trim_partial_tail(out); + return out[0] ? out : NULL; +} + +static bool is_markdown_heading_kind(const char *kind) { + return strcmp(kind, "atx_heading") == 0 || strcmp(kind, "setext_heading") == 0; +} + +/* #518: a Markdown heading node is only the title line — the section's prose + * lives in the blocks that FOLLOW it. Collect that prose as the Section's + * docstring so the node carries the text a reader would search for; nodes_fts + * indexes it from there (`body`), which is the whole point of the issue. + * + * tree-sitter-markdown wraps a heading and its content in a `section` node, + * with nested subsections as further `section` children that own their own + * text. Stopping at either a `section` or another heading therefore gives each + * Section exactly its OWN body under both that shape and a flat one. The bytes + * between are contiguous in the source, so one slice beats concatenation. */ +static const char *extract_markdown_section_body(CBMArena *a, TSNode heading, const char *source) { + uint32_t start = ts_node_end_byte(heading); + uint32_t end = start; + for (TSNode sib = ts_node_next_sibling(heading); !ts_node_is_null(sib); + sib = ts_node_next_sibling(sib)) { + const char *sk = ts_node_type(sib); + if (strcmp(sk, "section") == 0 || is_markdown_heading_kind(sk)) { + break; + } + end = ts_node_end_byte(sib); + } + if (end <= start) { + return NULL; + } + return collapse_prose(a, source + start, (size_t)(end - start)); +} + +/* Push a config-language definition that carries prose. push_simple_class_def + * delegates here with no docstring. */ +static void push_simple_class_def_doc(CBMExtractCtx *ctx, TSNode node, char *name, + const char *label, const char *docstring) { CBMArena *a = ctx->arena; CBMDefinition def; memset(&def, 0, sizeof(def)); @@ -3872,9 +3997,14 @@ static void push_simple_class_def(CBMExtractCtx *ctx, TSNode node, char *name, c def.start_line = ts_node_start_point(node).row + TS_LINE_OFFSET; def.end_line = ts_node_end_point(node).row + TS_LINE_OFFSET; def.is_exported = true; + def.docstring = docstring; cbm_defs_push(&ctx->result->defs, a, def); } +static void push_simple_class_def(CBMExtractCtx *ctx, TSNode node, char *name, const char *label) { + push_simple_class_def_doc(ctx, node, name, label, NULL); +} + // Find TOML table key name from children. static char *find_toml_key_name(CBMArena *a, TSNode node, const char *source) { uint32_t nc = ts_node_child_count(node); @@ -4033,6 +4163,7 @@ static bool extract_config_class_def(CBMExtractCtx *ctx, TSNode node, const char CBMArena *a = ctx->arena; char *name = NULL; const char *label = "Class"; + const char *docstring = NULL; if (ctx->language == CBM_LANG_TOML && (strcmp(kind, "table") == 0 || strcmp(kind, "table_array_element") == 0)) { @@ -4048,6 +4179,8 @@ static bool extract_config_class_def(CBMExtractCtx *ctx, TSNode node, const char // label rather than degrade it to match a test. The markdown repro asserts // "Class"; that assertion is the inaccurate side and is flagged for review. label = "Section"; + // #518: index what the section SAYS, not just its title. + docstring = extract_markdown_section_body(a, node, ctx->source); } else if (ctx->language == CBM_LANG_HCL && strcmp(kind, "block") == 0) { name = find_hcl_block_name(a, node, ctx->source); } else { @@ -4055,7 +4188,7 @@ static bool extract_config_class_def(CBMExtractCtx *ctx, TSNode node, const char } if (name && name[0]) { - push_simple_class_def(ctx, node, name, label); + push_simple_class_def_doc(ctx, node, name, label, docstring); } return true; } @@ -6175,12 +6308,9 @@ static bool is_helm_values_file(const char *rel) { return strcmp(b, "values.yaml") == 0 || strcmp(b, "values.yml") == 0; } -// Extract ONLY top-level keys of a YAML document (no leaf explosion). Used for -// Helm values.yaml so each chart's tunables surface as a handful of structured -// Variables instead of one node per nested leaf (#338). -static void extract_yaml_toplevel_keys(CBMExtractCtx *ctx, TSNode root) { - CBMArena *a = ctx->arena; - // Descend stream -> document -> block_node down to the first block_mapping. +// Descend stream -> document -> block_node to a YAML document's top-level +// block_mapping. Returns a null node when the document has none. +static TSNode find_yaml_toplevel_mapping(TSNode root) { TSNode bm = {0}; TSNode cur = root; for (int depth = 0; depth < 6 && ts_node_is_null(bm); depth++) { @@ -6204,6 +6334,126 @@ static void extract_yaml_toplevel_keys(CBMExtractCtx *ctx, TSNode root) { } cur = next; } + return bm; +} + +// Descend to a JSON document's top-level object. Returns a null node if absent. +static TSNode find_json_toplevel_object(TSNode root) { + if (strcmp(ts_node_type(root), "object") == 0) { + return root; + } + TSNode obj = cbm_find_child_by_kind(root, "object"); + if (!ts_node_is_null(obj)) { + return obj; + } + uint32_t n = ts_node_named_child_count(root); + for (uint32_t i = 0; i < n; i++) { + TSNode inner = cbm_find_child_by_kind(ts_node_named_child(root, i), "object"); + if (!ts_node_is_null(inner)) { + return inner; + } + } + TSNode null_node = {0}; + return null_node; +} + +/* #519: a config file's own prose sits in a top-level `description` (or one of + * its usual synonyms) — META.yaml, action.yml, an OpenAPI document, + * package.json. Nothing indexed it: the VALUE is not a definition, so no node + * carried it and BM25 could not see it at all. Checked in priority order; the + * first key present wins. */ +static const char *const config_desc_keys[] = {"description", "summary", "purpose", NULL}; + +// Strip one layer of matching surrounding quotes. Operates on arena text. +static char *strip_surrounding_quotes(char *t) { + if (!t) { + return t; + } + size_t n = strlen(t); + if (n >= PAIR_CHARS && (t[0] == '"' || t[0] == '\'') && t[n - SKIP_CHAR] == t[0]) { + t[n - SKIP_CHAR] = '\0'; + return t + SKIP_CHAR; + } + return t; +} + +// Normalise a config scalar into an indexable value: drop a YAML block-scalar +// header (`|`/`>` plus its chomping and indent modifiers), collapse whitespace +// to the shared MAX_COMMENT_LEN cap, then unquote. +static const char *config_scalar_value(CBMArena *a, const char *raw) { + if (!raw) { + return NULL; + } + while (*raw == ' ' || *raw == '\t' || *raw == '\n' || *raw == '\r') { + raw++; + } + if (*raw == '|' || *raw == '>') { + while (*raw && *raw != '\n') { + raw++; + } + } + char *v = collapse_prose(a, raw, strlen(raw)); + return v ? strip_surrounding_quotes(v) : NULL; +} + +// Value of `pair` when its key is `want`, else NULL. A YAML block_mapping_pair +// and a JSON pair both expose key/value fields, so one reader serves both. +static const char *config_pair_value_if_key(CBMExtractCtx *ctx, TSNode pair, const char *want) { + TSNode key = ts_node_child_by_field_name(pair, TS_FIELD("key")); + TSNode val = ts_node_child_by_field_name(pair, TS_FIELD("value")); + if (ts_node_is_null(key) || ts_node_is_null(val)) { + return NULL; + } + char *kt = cbm_node_text(ctx->arena, key, ctx->source); + if (!kt || strcmp(strip_surrounding_quotes(kt), want) != 0) { + return NULL; + } + return config_scalar_value(ctx->arena, cbm_node_text(ctx->arena, val, ctx->source)); +} + +// First non-empty description value among `container`'s direct pairs, scanned +// in config_desc_keys priority order. +static const char *config_container_description(CBMExtractCtx *ctx, TSNode container, + const char *pair_kind) { + uint32_t n = ts_node_named_child_count(container); + for (const char *const *k = config_desc_keys; *k; k++) { + for (uint32_t i = 0; i < n; i++) { + TSNode pair = ts_node_named_child(container, i); + if (strcmp(ts_node_type(pair), pair_kind) != 0) { + continue; + } + const char *v = config_pair_value_if_key(ctx, pair, *k); + if (v && v[0]) { + return v; + } + } + } + return NULL; +} + +/* #519 entry point: the file-level description a config document declares about + * itself, promoted onto the Module node by cbm_extract_definitions so + * nodes_fts.body indexes it and the file is findable by what it SAYS it does, + * not only by its path. NULL for every other language. */ +static const char *extract_config_module_description(CBMExtractCtx *ctx) { + if (ctx->language == CBM_LANG_YAML) { + TSNode bm = find_yaml_toplevel_mapping(ctx->root); + return ts_node_is_null(bm) ? NULL + : config_container_description(ctx, bm, "block_mapping_pair"); + } + if (ctx->language == CBM_LANG_JSON) { + TSNode obj = find_json_toplevel_object(ctx->root); + return ts_node_is_null(obj) ? NULL : config_container_description(ctx, obj, "pair"); + } + return NULL; +} + +// Extract ONLY top-level keys of a YAML document (no leaf explosion). Used for +// Helm values.yaml so each chart's tunables surface as a handful of structured +// Variables instead of one node per nested leaf (#338). +static void extract_yaml_toplevel_keys(CBMExtractCtx *ctx, TSNode root) { + CBMArena *a = ctx->arena; + TSNode bm = find_yaml_toplevel_mapping(root); if (ts_node_is_null(bm)) { return; } @@ -7587,6 +7837,8 @@ void cbm_extract_definitions(CBMExtractCtx *ctx) { mod.end_line = ts_node_end_point(ctx->root).row + TS_LINE_OFFSET; mod.is_exported = true; mod.is_test = ctx->result->is_test_file; + // #519: index what a config file declares itself to be, not only its path. + mod.docstring = extract_config_module_description(ctx); cbm_defs_push(&ctx->result->defs, a, mod); cbm_extract_definitions_without_module(ctx); diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 0ce6ae7eb5..96a2e71ae6 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -2936,6 +2936,25 @@ enum { BM25_INNER_LIMIT = 2000, }; +/* Column weights for nodes_fts (name, qualified_name, label, file_path, body). + * The four identifier columns stay at parity; prose sits well below them. + * FTS5 applies these to per-column term frequency BEFORE the tf-saturation + * term, which is what makes the weighting BM25F-correct rather than a post-hoc + * rescale. 0.3 is the findability-favouring end of the field weighting the IR + * literature settles on for body text (typical title:body ratios run 3:1 to + * 10:1): a prose-only hit still surfaces, but never outranks a node whose + * IDENTIFIER matches. + * + * Defined once and used by BOTH the ranked query and the count query — they + * share an inner candidate window, so different weights would silently + * desynchronise the reported total from the rows returned. + * + * Safe against a legacy four-column nodes_fts: FTS5's bm25() reads a weight + * only when an instance actually lands in that column (`nVal > ic`), so the + * fifth weight is simply never consulted on a table that has no fifth + * column. */ +#define BM25_WEIGHTS "bm25(nodes_fts, 1.0, 1.0, 1.0, 1.0, 0.3)" + /* Module-local SQLITE_TRANSIENT wrapper to dodge performance-no-int-to-ptr. * See the matching helper in src/store/store.c for the same pattern. */ static sqlite3_destructor_type mcp_sqlite_transient(void) { @@ -3031,7 +3050,7 @@ static char *bm25_search(cbm_store_t *store, const char *project, const char *qu * matches, this causes multi-minute queries. * * The fix: let FTS5 drive the inner subquery alone. SQLite CAN early-terminate - * SELECT rowid, bm25(nodes_fts) FROM nodes_fts WHERE MATCH ? ORDER BY bm25() LIMIT N + * SELECT rowid, bm25(nodes_fts,...) FROM nodes_fts WHERE MATCH ? ORDER BY bm25() LIMIT N * because no outer predicate blocks it. We fetch BM25_INNER_LIMIT top candidates * from the FTS5 index, then join/filter/boost only those rows. bm25() returns a * NEGATIVE score (lower = more relevant). */ @@ -3046,13 +3065,18 @@ static char *bm25_search(cbm_store_t *store, const char *project, const char *qu " WHEN n.label IN (" CBM_SQL_RELATION_LABELS ") THEN 5.0 " " ELSE 0.0 END) AS rank " "FROM (" - " SELECT rowid, bm25(nodes_fts) AS base_rank" + " SELECT rowid, " BM25_WEIGHTS " AS base_rank" " FROM nodes_fts WHERE nodes_fts MATCH ?1" " ORDER BY base_rank LIMIT ?5" ") fts " "JOIN nodes n ON n.id = fts.rowid " "WHERE n.project = ?2 " - " AND n.label NOT IN ('File','Folder','Module','Section','Variable','Project') " + /* Section and Module are NO LONGER excluded (#518/#519): they are the + * labels that carry prose — a Markdown section's body, a config file's + * description — so excluding them made the body column unreachable. + * This exclusion list is MIRRORED in the count query below; the two + * must be changed together or results desynchronise from counts. */ + " AND n.label NOT IN ('File','Folder','Variable','Project') " " AND (?6 IS NULL OR n.file_path LIKE ?6) " /* rank ties are common (boosted floats) — the id tie-break makes * offset pages contractually stable across calls. */ @@ -3079,17 +3103,19 @@ static char *bm25_search(cbm_store_t *store, const char *project, const char *qu * Uses the identical subquery structure so the FTS5 early-exit applies here too. */ int total = 0; { - const char *count_sql = - "SELECT COUNT(*) FROM (" - " SELECT fts.rowid FROM (" - " SELECT rowid FROM nodes_fts WHERE nodes_fts MATCH ?1" - " ORDER BY bm25(nodes_fts) LIMIT ?3" - " ) fts " - " JOIN nodes n ON n.id = fts.rowid " - " WHERE n.project = ?2 " - " AND n.label NOT IN ('File','Folder','Module','Section','Variable','Project')" - " AND (?6 IS NULL OR n.file_path LIKE ?6)" - ")"; + const char *count_sql = "SELECT COUNT(*) FROM (" + " SELECT fts.rowid FROM (" + " SELECT rowid FROM nodes_fts WHERE nodes_fts MATCH ?1" + " ORDER BY " BM25_WEIGHTS " LIMIT ?3" + " ) fts " + " JOIN nodes n ON n.id = fts.rowid " + " WHERE n.project = ?2 " + /* MIRRORS the ranked query's filter verbatim — same weights, same + * label exclusions. Changing one alone reports a total that does + * not describe the rows returned. */ + " AND n.label NOT IN ('File','Folder','Variable','Project')" + " AND (?6 IS NULL OR n.file_path LIKE ?6)" + ")"; sqlite3_stmt *cs = NULL; if (sqlite3_prepare_v2(db, count_sql, BM25_SQL_AUTO_LEN, &cs, NULL) == SQLITE_OK) { sqlite3_bind_text(cs, BM25_BIND_QUERY, fts_query, BM25_SQL_AUTO_LEN, diff --git a/src/pipeline/pipeline.c b/src/pipeline/pipeline.c index 0c1d5eee40..9ba8182243 100644 --- a/src/pipeline/pipeline.c +++ b/src/pipeline/pipeline.c @@ -1524,22 +1524,6 @@ static void discard_generation_stage(const char *stage_path) { cbm_remove_db_sidecars(stage_path); } -static int generation_rebuild_fts(cbm_store_t *store) { - if (cbm_store_exec(store, "INSERT INTO nodes_fts(nodes_fts) VALUES('delete-all');") != - CBM_STORE_OK) { - return CBM_STORE_ERR; - } - if (cbm_store_exec(store, - "INSERT INTO nodes_fts(rowid, name, qualified_name, label, file_path) " - "SELECT id, cbm_camel_split(name), qualified_name, label, file_path " - "FROM nodes;") == CBM_STORE_OK) { - return CBM_STORE_OK; - } - return cbm_store_exec(store, - "INSERT INTO nodes_fts(rowid, name, qualified_name, label, file_path) " - "SELECT id, name, qualified_name, label, file_path FROM nodes;"); -} - typedef struct { bool quarantined; char backup_path[CBM_SZ_4K]; @@ -1809,7 +1793,10 @@ int cbm_pipeline_publish_staged(char *stage_path, const cbm_pipeline_generation_ cbm_log_info("publish.timing", "block", "coverage_replace", "elapsed_ms", itoa_buf((int)elapsed_ms(t_pub))); cbm_clock_gettime(CLOCK_MONOTONIC, &t_pub); - if (fts_wholesale && generation_rebuild_fts(store) != CBM_STORE_OK) { + /* The column list lives in cbm_store_fts_rebuild() alone — see the delta + * merge, which must index the SAME columns or prose goes missing on the + * warm path while a full reindex looks perfect. */ + if (fts_wholesale && cbm_store_fts_rebuild(store, NULL, 0) != CBM_STORE_OK) { ok = false; } cbm_log_info("publish.timing", "block", "fts", "elapsed_ms", itoa_buf((int)elapsed_ms(t_pub))); diff --git a/src/pipeline/pipeline_delta.c b/src/pipeline/pipeline_delta.c index 50fc094c26..0596eb2388 100644 --- a/src/pipeline/pipeline_delta.c +++ b/src/pipeline/pipeline_delta.c @@ -21,12 +21,17 @@ * deleted individually on existing databases; their rowids can never alias * a live node again (AUTOINCREMENT), so dead entries simply drop out of the * rowid join at query time. The patch inserts rows for exactly the new - * nodes, via the same cbm_camel_split SQL function the wholesale rebuild - * uses. + * nodes through cbm_store_fts_rebuild() — the SAME writer the wholesale + * rebuild uses, so the two paths cannot index different column sets. This + * path is the one most users actually hit: a hand-written INSERT here that + * named only the identifier columns would leave nodes_fts.body NULL for + * every node arriving by delta merge, invisibly, while a full reindex + * looked perfect (#518/#519). */ #include "foundation/constants.h" #include "pipeline/pipeline_internal.h" +#include /* intptr_t — SQLITE_TRANSIENT wrapper */ #include #include #include @@ -38,6 +43,25 @@ enum { DELTA_IN_CHUNK = 200 }; +/* Positional bind indices for the snapshot re-link statement. */ +enum { + DELTA_BIND_1 = 1, + DELTA_BIND_2 = 2, + DELTA_BIND_3 = 3, + DELTA_BIND_4 = 4, + DELTA_BIND_5 = 5, +}; + +/* Module-local SQLITE_TRANSIENT wrapper to dodge performance-no-int-to-ptr. + * Same pattern as store.c's BIND_TRANSIENT and mcp.c's MCP_SQLITE_TRANSIENT. */ +static sqlite3_destructor_type delta_sqlite_transient(void) { + static const volatile intptr_t raw = -1; + sqlite3_destructor_type dtor = NULL; + memcpy(&dtor, (const void *)&raw, sizeof(dtor)); + return dtor; +} +#define DELTA_SQLITE_TRANSIENT (delta_sqlite_transient()) + /* Assemble "?,?,...,?" for a chunked IN list. buf must hold 2*count. */ static void delta_placeholders(char *buf, int count) { int pos = 0; @@ -448,6 +472,42 @@ static void delta_patch_edge(const cbm_gbuf_edge_t *edge, void *userdata) { ctx->edges++; } +/* Re-link the snapshotted inbound edges by qualified name. A target whose QN + * no longer exists simply matches no row — full-reindex semantics for deleted + * symbols, dedup by the UNIQUE edge constraint. Returns false on failure. + * + * Extracted from cbm_delta_patch to keep that function inside the + * cognitive-complexity budget; behaviour is unchanged. */ +static bool delta_relink_snapshot(sqlite3 *db, const char *project, + const cbm_delta_saved_edge_t *snapshot, int snapshot_count) { + sqlite3_stmt *relink = NULL; + if (sqlite3_prepare_v2(db, + "INSERT OR IGNORE INTO edges (project, source_id, target_id," + " type, properties)" + " SELECT ?1, s.id, t.id, ?2, ?3 FROM nodes s, nodes t" + " WHERE s.project = ?1 AND s.qualified_name = ?4" + " AND t.project = ?1 AND t.qualified_name = ?5", + CBM_NOT_FOUND, &relink, NULL) != SQLITE_OK) { + return false; + } + bool ok = true; + for (int i = 0; i < snapshot_count && ok; i++) { + sqlite3_reset(relink); + sqlite3_bind_text(relink, DELTA_BIND_1, project, CBM_NOT_FOUND, DELTA_SQLITE_TRANSIENT); + sqlite3_bind_text(relink, DELTA_BIND_2, snapshot[i].type, CBM_NOT_FOUND, + DELTA_SQLITE_TRANSIENT); + sqlite3_bind_text(relink, DELTA_BIND_3, snapshot[i].props, CBM_NOT_FOUND, + DELTA_SQLITE_TRANSIENT); + sqlite3_bind_text(relink, DELTA_BIND_4, snapshot[i].source_qn, CBM_NOT_FOUND, + DELTA_SQLITE_TRANSIENT); + sqlite3_bind_text(relink, DELTA_BIND_5, snapshot[i].target_qn, CBM_NOT_FOUND, + DELTA_SQLITE_TRANSIENT); + ok = sqlite3_step(relink) == SQLITE_DONE; + } + sqlite3_finalize(relink); + return ok; +} + int cbm_delta_patch(cbm_store_t *store, const char *project, cbm_gbuf_t *gbuf, int64_t max_db_id, const cbm_delta_saved_edge_t *snapshot, int snapshot_count) { sqlite3 *db = cbm_store_get_db(store); @@ -489,57 +549,21 @@ int cbm_delta_patch(cbm_store_t *store, const char *project, cbm_gbuf_t *gbuf, i sqlite3_finalize(ctx.edge_stmt); sqlite3_finalize(ctx.qn_lookup); - /* Re-link the snapshotted inbound edges by qualified name. A target - * whose QN no longer exists simply matches no row — full-reindex - * semantics for deleted symbols, dedup by the UNIQUE edge constraint. */ - if (!ctx.failed && snapshot_count > 0) { - sqlite3_stmt *relink = NULL; - if (sqlite3_prepare_v2(db, - "INSERT OR IGNORE INTO edges (project, source_id, target_id," - " type, properties)" - " SELECT ?1, s.id, t.id, ?2, ?3 FROM nodes s, nodes t" - " WHERE s.project = ?1 AND s.qualified_name = ?4" - " AND t.project = ?1 AND t.qualified_name = ?5", - CBM_NOT_FOUND, &relink, NULL) != SQLITE_OK) { - ctx.failed = true; - } else { - for (int i = 0; i < snapshot_count && !ctx.failed; i++) { - sqlite3_reset(relink); - sqlite3_bind_text(relink, 1, project, CBM_NOT_FOUND, SQLITE_TRANSIENT); - sqlite3_bind_text(relink, 2, snapshot[i].type, CBM_NOT_FOUND, SQLITE_TRANSIENT); - sqlite3_bind_text(relink, 3, snapshot[i].props, CBM_NOT_FOUND, SQLITE_TRANSIENT); - sqlite3_bind_text(relink, 4, snapshot[i].source_qn, CBM_NOT_FOUND, - SQLITE_TRANSIENT); - sqlite3_bind_text(relink, 5, snapshot[i].target_qn, CBM_NOT_FOUND, - SQLITE_TRANSIENT); - if (sqlite3_step(relink) != SQLITE_DONE) { - ctx.failed = true; - } - } - sqlite3_finalize(relink); - } + if (!ctx.failed && snapshot_count > 0 && + !delta_relink_snapshot(db, project, snapshot, snapshot_count)) { + ctx.failed = true; } - /* Row-level FTS for exactly the new nodes, through the same tokenizer - * function the wholesale rebuild uses. */ + /* Row-level FTS for exactly the new nodes, through the shared writer — + * same columns, same tokenizer, same prose backfill as the wholesale + * rebuild. NOT_FOUND means the index could not be written at all (FTS5 + * compiled out); search then runs without it, matching the dump path. */ if (!ctx.failed) { - sqlite3_stmt *fts = NULL; - if (sqlite3_prepare_v2(db, - "INSERT INTO nodes_fts (rowid, name, qualified_name, label," - " file_path)" - " SELECT id, cbm_camel_split(name), qualified_name, label," - " file_path FROM nodes WHERE project = ?1 AND id > ?2", - CBM_NOT_FOUND, &fts, NULL) == SQLITE_OK) { - sqlite3_bind_text(fts, 1, project, CBM_NOT_FOUND, SQLITE_TRANSIENT); - sqlite3_bind_int64(fts, 2, max_db_id); - if (sqlite3_step(fts) != SQLITE_DONE) { - ctx.failed = true; - } - sqlite3_finalize(fts); - } else { - /* FTS5 may be compiled out; the table then never existed and - * search runs without it — matching the dump path's behavior. */ + int fts_rc = cbm_store_fts_rebuild(store, project, max_db_id); + if (fts_rc == CBM_STORE_NOT_FOUND) { cbm_log_warn("delta.fts_insert_unavailable", "project", project); + } else if (fts_rc != CBM_STORE_OK) { + ctx.failed = true; } } diff --git a/src/store/store.c b/src/store/store.c index ad0b7967b7..8e44583b32 100644 --- a/src/store/store.c +++ b/src/store/store.c @@ -356,12 +356,21 @@ static int init_schema(cbm_store_t *s) { * not a copy of the source text — required for camelCase tokenization * because we feed it `cbm_camel_split(name)` at insert time but want * queries to match against the split tokens, not the original. - * Fails silently if FTS5 is not compiled in (SQLITE_ENABLE_FTS5). */ + * Fails silently if FTS5 is not compiled in (SQLITE_ENABLE_FTS5). + * + * `body` (#518/#519) carries each node's prose — its docstring — so a + * question phrased in words rather than identifiers still finds the node. + * It is deliberately NOT an index-format change: IF NOT EXISTS leaves a + * legacy four-column table exactly as it is, cbm_store_fts_rebuild() + * probes for the column before naming it, and bm25()'s surplus weight is + * inert on a table that has no fifth column. A database written by an + * older build therefore keeps opening and searching — without prose — + * instead of forcing every user to reindex. */ { char *fts_err = 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, body," " content=''," " tokenize='unicode61 remove_diacritics 2'" ");", @@ -373,6 +382,102 @@ static int init_schema(cbm_store_t *s) { return CBM_STORE_OK; } +/* ── FTS backfill ───────────────────────────────────────────────── */ + +/* Prose source for nodes_fts.body: the docstring each node already carries in + * its properties JSON. Deriving it costs no new column on `nodes` and no new + * table, which is exactly why CBM_INDEX_FORMAT_VERSION does not move and no + * user is forced to reindex. + * - the LIKE prefilter keeps the JSON parse off the large majority of nodes + * that have no docstring at all; + * - json_valid() guards the parse because json_extract() RAISES on malformed + * properties, and pre-fix databases contain such rows (see the reverted + * is_entry_point expression index in create_user_indexes) — unguarded, one + * bad row would abort the entire backfill. */ +#define FTS_BODY_EXPR \ + "CASE WHEN properties LIKE '%\"docstring\"%' AND json_valid(properties) " \ + "THEN json_extract(properties, '$.docstring') END" + +enum { FTS_SQL_BUF = 512 }; + +/* Does nodes_fts carry the `body` column? A database created by an older + * build has only the four identifier columns, and CREATE VIRTUAL TABLE IF NOT + * EXISTS does not widen it. Probe rather than assume: naming a column that + * is not there would fail the backfill outright, and the contract for a legacy + * database is "still searchable, just without prose". */ +static bool fts_has_body_column(cbm_store_t *s) { + sqlite3_stmt *probe = NULL; + if (sqlite3_prepare_v2(s->db, "SELECT body FROM nodes_fts LIMIT 0;", CBM_NOT_FOUND, &probe, + NULL) != SQLITE_OK) { + return false; + } + sqlite3_finalize(probe); + return true; +} + +/* One backfill attempt. CBM_STORE_NOT_FOUND means the statement would not + * even prepare (missing table, column or SQL function) so the caller may retry + * with a narrower shape; CBM_STORE_ERR means the write itself failed. */ +static int fts_backfill_try(cbm_store_t *s, const char *project, int64_t after_id, bool with_body, + bool camel) { + char sql[FTS_SQL_BUF]; + int n = snprintf(sql, sizeof(sql), + "INSERT INTO nodes_fts (rowid, name, qualified_name, label, file_path%s)" + " SELECT id, %s, qualified_name, label, file_path%s FROM nodes%s;", + with_body ? ", body" : "", camel ? "cbm_camel_split(name)" : "name", + with_body ? ", " FTS_BODY_EXPR : "", + project ? " WHERE project = ?1 AND id > ?2" : ""); + if (n <= 0 || (size_t)n >= sizeof(sql)) { + return CBM_STORE_ERR; + } + sqlite3_stmt *stmt = NULL; + if (sqlite3_prepare_v2(s->db, sql, CBM_NOT_FOUND, &stmt, NULL) != SQLITE_OK) { + return CBM_STORE_NOT_FOUND; + } + if (project) { + sqlite3_bind_text(stmt, ST_COL_1, project, CBM_NOT_FOUND, BIND_TRANSIENT); + sqlite3_bind_int64(stmt, ST_COL_2, after_id); + } + int rc = sqlite3_step(stmt) == SQLITE_DONE ? CBM_STORE_OK : CBM_STORE_ERR; + if (rc != CBM_STORE_OK) { + store_set_error_sqlite(s, "fts_backfill"); + } + sqlite3_finalize(stmt); + return rc; +} + +int cbm_store_fts_rebuild(cbm_store_t *s, const char *project, int64_t after_id) { + if (!s || !s->db) { + return CBM_STORE_ERR; + } + /* Wholesale only: the incremental caller adds rows to a live index. */ + if (!project && + exec_sql(s, "INSERT INTO nodes_fts(nodes_fts) VALUES('delete-all');") != CBM_STORE_OK) { + return CBM_STORE_ERR; + } + /* Degrade one capability at a time, widest first. Dropping `body` covers a + * legacy four-column table or a build without JSON1; dropping + * cbm_camel_split covers a connection where the function is not registered + * (the pre-existing fallback). Each attempt is a single statement, so a + * failed one rolls itself back before the next runs. */ + const bool with_body = fts_has_body_column(s); + const struct { + bool body; + bool camel; + } ladder[] = {{true, true}, {true, false}, {false, true}, {false, false}}; + int rc = CBM_STORE_NOT_FOUND; + for (size_t i = 0; i < sizeof(ladder) / sizeof(ladder[0]); i++) { + if (ladder[i].body && !with_body) { + continue; + } + rc = fts_backfill_try(s, project, after_id, ladder[i].body, ladder[i].camel); + if (rc == CBM_STORE_OK) { + return rc; + } + } + return rc; +} + static int create_user_indexes(cbm_store_t *s) { const char *sql = "CREATE INDEX IF NOT EXISTS idx_nodes_label ON nodes(project, label);" diff --git a/src/store/store.h b/src/store/store.h index 45a7bb4393..71fbc911d3 100644 --- a/src/store/store.h +++ b/src/store/store.h @@ -912,4 +912,20 @@ int cbm_store_count_vectors(cbm_store_t *s, const char *project); * Returns CBM_STORE_OK on success. */ int cbm_store_exec(cbm_store_t *s, const char *sql); +/* Populate nodes_fts from the `nodes` table — the single writer for the BM25 + * index. Every backfill site routes through it so the column list is decided + * in exactly ONE place: a hand-written INSERT that names only the identifier + * columns silently leaves `body` NULL for every node it writes, which looks + * perfect after a full reindex and de-indexes prose on the warm path. + * + * project == NULL → wholesale rebuild: clears the index, then reindexes + * every node in the database. + * project != NULL → incremental: indexes only that project's nodes with + * id > after_id and leaves existing rows untouched. + * + * Returns CBM_STORE_OK on success; CBM_STORE_NOT_FOUND when nodes_fts cannot + * be written at all (FTS5 compiled out — the caller decides whether that is + * fatal); CBM_STORE_ERR on a genuine write failure. */ +int cbm_store_fts_rebuild(cbm_store_t *s, const char *project, int64_t after_id); + #endif /* CBM_STORE_H */ diff --git a/tests/test_extraction.c b/tests/test_extraction.c index 935eda07ce..b34b14024c 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -6013,6 +6013,208 @@ TEST(iris_export_xml_multi_class) { PASS(); } +/* ── #518 / #519: prose that BM25 can index ──────────────────────── + * + * A Section carried only its heading and a config Module only its path, so a + * question asked in words could not reach either. Both now carry the prose in + * `docstring`, which is what nodes_fts indexes into its `body` column. */ + +/* First Module-labelled definition, or NULL. */ +static const CBMDefinition *find_module_def(CBMFileResult *r) { + for (int i = 0; i < r->defs.count; i++) { + if (strcmp(r->defs.items[i].label, "Module") == 0) { + return &r->defs.items[i]; + } + } + return NULL; +} + +TEST(markdown_section_body_becomes_docstring_issue518) { + CBMFileResult *r = extract("# Installation\n" + "Run the bootstrap script to provision a workstation.\n" + "It installs the toolchain and seeds the cache.\n", + CBM_LANG_MARKDOWN, "t", "README.md"); + ASSERT_NOT_NULL(r); + const CBMDefinition *d = find_def_by_name(r, "Installation"); + ASSERT_NOT_NULL(d); + ASSERT_STR_EQ(d->label, "Section"); + ASSERT_NOT_NULL(d->docstring); + /* The prose — not the heading — is what makes the section findable. */ + ASSERT_NOT_NULL(strstr(d->docstring, "bootstrap script")); + ASSERT_NOT_NULL(strstr(d->docstring, "seeds the cache")); + /* Newlines collapse to single spaces so the 500-byte cap buys real words. */ + ASSERT_NULL(strchr(d->docstring, '\n')); + cbm_free_result(r); + PASS(); +} + +TEST(markdown_section_body_stops_at_next_heading_issue518) { + CBMFileResult *r = extract("# Alpha\n" + "alphatext belongs to the first section.\n" + "\n" + "# Beta\n" + "betatext belongs to the second section.\n", + CBM_LANG_MARKDOWN, "t", "README.md"); + ASSERT_NOT_NULL(r); + const CBMDefinition *alpha = find_def_by_name(r, "Alpha"); + const CBMDefinition *beta = find_def_by_name(r, "Beta"); + ASSERT_NOT_NULL(alpha); + ASSERT_NOT_NULL(beta); + ASSERT_NOT_NULL(alpha->docstring); + ASSERT_NOT_NULL(beta->docstring); + /* Each section owns ITS body: bleeding across the boundary would make every + * heading match every word in the file. */ + ASSERT_NOT_NULL(strstr(alpha->docstring, "alphatext")); + ASSERT_NULL(strstr(alpha->docstring, "betatext")); + ASSERT_NOT_NULL(strstr(beta->docstring, "betatext")); + ASSERT_NULL(strstr(beta->docstring, "alphatext")); + cbm_free_result(r); + PASS(); +} + +TEST(markdown_section_body_heading_only_has_no_docstring_issue518) { + CBMFileResult *r = extract("# Lonely\n", CBM_LANG_MARKDOWN, "t", "README.md"); + ASSERT_NOT_NULL(r); + const CBMDefinition *d = find_def_by_name(r, "Lonely"); + ASSERT_NOT_NULL(d); + /* An empty body stays NULL rather than "": append_json_string drops empty + * values, so an empty string would be a difference with no observable + * meaning — and a docstring key that promises prose it does not have. */ + ASSERT_NULL(d->docstring); + cbm_free_result(r); + PASS(); +} + +TEST(markdown_section_body_capped_utf8_safe_issue518) { + /* 400 three-byte codepoints (1200 bytes) guarantees the 500-byte cut lands + * mid-sequence unless the backoff works. */ + char src[4096]; + int pos = snprintf(src, sizeof(src), "# Unicode\n"); + for (int i = 0; i < 400; i++) { + pos += snprintf(src + pos, sizeof(src) - (size_t)pos, "\xe2\x9c\x93"); + } + snprintf(src + pos, sizeof(src) - (size_t)pos, "\n"); + + CBMFileResult *r = extract(src, CBM_LANG_MARKDOWN, "t", "README.md"); + ASSERT_NOT_NULL(r); + const CBMDefinition *d = find_def_by_name(r, "Unicode"); + ASSERT_NOT_NULL(d); + ASSERT_NOT_NULL(d->docstring); + size_t n = strlen(d->docstring); + ASSERT_LTE((int)n, 500); /* MAX_COMMENT_LEN — fits the 2 KB properties buffer */ + ASSERT_GT((int)n, 0); + /* Every byte must belong to a COMPLETE sequence: walk the string and check + * each lead byte is followed by its full continuation run. */ + for (size_t i = 0; i < n;) { + unsigned char c = (unsigned char)d->docstring[i]; + size_t need; + if ((c & 0x80) == 0) { + need = 1; + } else if ((c & 0xE0) == 0xC0) { + need = 2; + } else if ((c & 0xF0) == 0xE0) { + need = 3; + } else if ((c & 0xF8) == 0xF0) { + need = 4; + } else { + FAIL("stray UTF-8 continuation byte at a sequence start"); + } + ASSERT_LTE((int)(i + need), (int)n); /* no truncated tail sequence */ + i += need; + } + cbm_free_result(r); + PASS(); +} + +TEST(yaml_toplevel_description_promoted_to_module_issue519) { + CBMFileResult *r = extract("name: my-action\n" + "description: Provisions an ephemeral build runner.\n" + "runs:\n" + " using: node20\n", + CBM_LANG_YAML, "t", "META.yaml"); + ASSERT_NOT_NULL(r); + const CBMDefinition *mod = find_module_def(r); + ASSERT_NOT_NULL(mod); + ASSERT_NOT_NULL(mod->docstring); + ASSERT_NOT_NULL(strstr(mod->docstring, "ephemeral build runner")); + cbm_free_result(r); + PASS(); +} + +TEST(yaml_block_scalar_description_promoted_issue519) { + CBMFileResult *r = extract("name: my-action\n" + "description: |\n" + " Provisions an ephemeral build runner\n" + " and tears it down afterwards.\n", + CBM_LANG_YAML, "t", "META.yaml"); + ASSERT_NOT_NULL(r); + const CBMDefinition *mod = find_module_def(r); + ASSERT_NOT_NULL(mod); + ASSERT_NOT_NULL(mod->docstring); + /* The `|` indicator itself must not survive into the indexed text. */ + ASSERT_NOT_NULL(strstr(mod->docstring, "tears it down")); + ASSERT_NULL(strchr(mod->docstring, '|')); + cbm_free_result(r); + PASS(); +} + +TEST(yaml_summary_promoted_when_no_description_issue519) { + CBMFileResult *r = extract("name: thing\n" + "summary: Aggregates telemetry from every shard.\n", + CBM_LANG_YAML, "t", "META.yaml"); + ASSERT_NOT_NULL(r); + const CBMDefinition *mod = find_module_def(r); + ASSERT_NOT_NULL(mod); + ASSERT_NOT_NULL(mod->docstring); + ASSERT_NOT_NULL(strstr(mod->docstring, "every shard")); + cbm_free_result(r); + PASS(); +} + +TEST(json_toplevel_description_promoted_to_module_issue519) { + CBMFileResult *r = extract("{\n" + " \"name\": \"widget\",\n" + " \"description\": \"Renders dashboards from graph queries.\"\n" + "}\n", + CBM_LANG_JSON, "t", "package.json"); + ASSERT_NOT_NULL(r); + const CBMDefinition *mod = find_module_def(r); + ASSERT_NOT_NULL(mod); + ASSERT_NOT_NULL(mod->docstring); + ASSERT_NOT_NULL(strstr(mod->docstring, "Renders dashboards")); + /* The JSON string quotes are stripped — they are not part of the value. */ + ASSERT_NULL(strchr(mod->docstring, '"')); + cbm_free_result(r); + PASS(); +} + +TEST(config_description_only_at_top_level_issue519) { + /* A nested `description` describes the nested thing, not the file. */ + CBMFileResult *r = extract("name: chart\n" + "values:\n" + " description: nestedonly\n", + CBM_LANG_YAML, "t", "META.yaml"); + ASSERT_NOT_NULL(r); + const CBMDefinition *mod = find_module_def(r); + ASSERT_NOT_NULL(mod); + ASSERT(mod->docstring == NULL || strstr(mod->docstring, "nestedonly") == NULL); + cbm_free_result(r); + PASS(); +} + +TEST(non_config_language_module_has_no_promoted_description_issue519) { + /* The promotion is config-only: a Python file's Module node must not pick + * up a variable that merely happens to be called `description`. */ + CBMFileResult *r = + extract("description = 'not a config file'\n", CBM_LANG_PYTHON, "t", "conf.py"); + ASSERT_NOT_NULL(r); + const CBMDefinition *mod = find_module_def(r); + ASSERT_NOT_NULL(mod); + ASSERT_NULL(mod->docstring); + cbm_free_result(r); + PASS(); +} + SUITE(extraction) { /* Initialize extraction library */ cbm_init(); @@ -6366,5 +6568,17 @@ SUITE(extraction) { RUN_TEST(docstring_utf8_truncation_boundary_issue1017); RUN_TEST(extract_ts_decorators_survive_interleaved_comment); + /* #518/#519 — prose carried into docstring so nodes_fts can index it */ + RUN_TEST(markdown_section_body_becomes_docstring_issue518); + RUN_TEST(markdown_section_body_stops_at_next_heading_issue518); + RUN_TEST(markdown_section_body_heading_only_has_no_docstring_issue518); + RUN_TEST(markdown_section_body_capped_utf8_safe_issue518); + RUN_TEST(yaml_toplevel_description_promoted_to_module_issue519); + RUN_TEST(yaml_block_scalar_description_promoted_issue519); + RUN_TEST(yaml_summary_promoted_when_no_description_issue519); + RUN_TEST(json_toplevel_description_promoted_to_module_issue519); + RUN_TEST(config_description_only_at_top_level_issue519); + RUN_TEST(non_config_language_module_has_no_promoted_description_issue519); + cbm_shutdown(); } diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 6b7af64ed4..18bfbd3b19 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -11460,7 +11460,249 @@ TEST(index_repository_supervisor_uses_canonical_session_path) { * SUITE * ══════════════════════════════════════════════════════════════════ */ +/* ── BM25 prose search (#518 / #519) ─────────────────────────────── + * + * Section and Module used to be filtered out of BM25 results outright, which + * made the prose they carry unreachable no matter how it was indexed. These + * cover the query side: prose is findable, the excluded labels come back, and + * the ranked query and the count query still agree. */ + +static cbm_mcp_server_t *setup_prose_search_server(const char *proj) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + if (!srv) { + return NULL; + } + cbm_store_t *st = cbm_mcp_server_store(srv); + cbm_mcp_server_set_project(srv, proj); + cbm_store_upsert_project(st, proj, "/tmp/prose"); + + cbm_node_t section = {.project = proj, + .label = "Section", + .name = "Installation", + .qualified_name = "prose.README.Installation", + .file_path = "README.md", + .start_line = 1, + .end_line = 20, + .properties_json = "{\"docstring\":\"provisions an ephemeral " + "workstation runner and seeds the cache\"}"}; + cbm_store_upsert_node(st, §ion); + + cbm_node_t module = {.project = proj, + .label = "Module", + .name = "action.yml", + .qualified_name = "prose.action_yml", + .file_path = "action.yml", + .start_line = 1, + .end_line = 40, + .properties_json = + "{\"docstring\":\"aggregates telemetry from every shard\"}"}; + cbm_store_upsert_node(st, &module); + + cbm_node_t fn = {.project = proj, + .label = "Function", + .name = "telemetryCollector", + .qualified_name = "prose.src.telemetryCollector", + .file_path = "src/collect.c", + .start_line = 3, + .end_line = 9}; + cbm_store_upsert_node(st, &fn); + + cbm_store_fts_rebuild(st, NULL, 0); + return srv; +} + +static char *prose_search(cbm_mcp_server_t *srv, const char *proj, const char *query) { + char req[512]; + snprintf(req, sizeof(req), + "{\"jsonrpc\":\"2.0\",\"id\":518,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"search_graph\"," + "\"arguments\":{\"project\":\"%s\",\"query\":\"%s\",\"limit\":10}}}", + proj, query); + char *resp = cbm_mcp_server_handle(srv, req); + if (!resp) { + return NULL; + } + char *inner = extract_text_content(resp); + free(resp); + return inner; +} + +TEST(bm25_finds_section_by_its_prose_issue518) { + cbm_mcp_server_t *srv = setup_prose_search_server("prose518"); + ASSERT_NOT_NULL(srv); + + /* "ephemeral" appears NOWHERE in any identifier — only in the section's + * body. Before the body column it was unfindable. */ + char *inner = prose_search(srv, "prose518", "ephemeral"); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "search_mode: bm25")); + ASSERT_NOT_NULL(strstr(inner, "prose.README.Installation")); + ASSERT_NOT_NULL(strstr(inner, "Section")); + free(inner); + + cbm_mcp_server_free(srv); + PASS(); +} + +TEST(bm25_finds_module_by_promoted_description_issue519) { + cbm_mcp_server_t *srv = setup_prose_search_server("prose519"); + ASSERT_NOT_NULL(srv); + + /* A config file's own description, promoted onto its Module node. Module + * was one of the labels the BM25 filter used to drop unconditionally. */ + char *inner = prose_search(srv, "prose519", "shard"); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "prose.action_yml")); + ASSERT_NOT_NULL(strstr(inner, "Module")); + free(inner); + + cbm_mcp_server_free(srv); + PASS(); +} + +TEST(bm25_results_and_total_stay_consistent_issue518) { + /* The ranked query and the count query share an inner candidate window and + * MIRROR each other's filter. Changing the label exclusion (or the weights) + * in only one silently reports a total that does not describe the rows. */ + cbm_mcp_server_t *srv = setup_prose_search_server("prosecount"); + ASSERT_NOT_NULL(srv); + + char *inner = prose_search(srv, "prosecount", "telemetry"); + ASSERT_NOT_NULL(inner); + /* Both the Module (body: "aggregates telemetry...") and the Function + * (name: telemetryCollector) match, and both are now eligible. */ + const char *total = strstr(inner, "total: "); + const char *results = strstr(inner, "results: "); + ASSERT_NOT_NULL(total); + ASSERT_NOT_NULL(results); + int total_n = atoi(total + strlen("total: ")); + int results_n = atoi(results + strlen("results: ")); + ASSERT_EQ(total_n, 2); + ASSERT_EQ(results_n, total_n); + ASSERT_NOT_NULL(strstr(inner, "prose.action_yml")); + ASSERT_NOT_NULL(strstr(inner, "prose.src.telemetryCollector")); + free(inner); + + cbm_mcp_server_free(srv); + PASS(); +} + +TEST(bm25_identifier_match_outranks_prose_only_match_issue518) { + /* The 0.3 body weight is what keeps prose from drowning identifiers. Both + * candidates carry the same label boost, so the ordering here is decided by + * the column weights alone. */ + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *st = cbm_mcp_server_store(srv); + const char *proj = "prose-rank"; + cbm_mcp_server_set_project(srv, proj); + cbm_store_upsert_project(st, proj, "/tmp/prose-rank"); + + cbm_node_t by_name = {.project = proj, + .label = "Function", + .name = "reconcile", + .qualified_name = "pr.a.reconcile", + .file_path = "a.c"}; + ASSERT_TRUE(cbm_store_upsert_node(st, &by_name) > 0); + + cbm_node_t by_body = {.project = proj, + .label = "Function", + .name = "zzz", + .qualified_name = "pr.b.zzz", + .file_path = "b.c", + .properties_json = "{\"docstring\":\"reconcile the ledger\"}"}; + ASSERT_TRUE(cbm_store_upsert_node(st, &by_body) > 0); + ASSERT_EQ(cbm_store_fts_rebuild(st, NULL, 0), CBM_STORE_OK); + + char *inner = prose_search(srv, proj, "reconcile"); + ASSERT_NOT_NULL(inner); + const char *name_hit = strstr(inner, "pr.a.reconcile"); + const char *body_hit = strstr(inner, "pr.b.zzz"); + ASSERT_NOT_NULL(name_hit); /* the identifier match */ + ASSERT_NOT_NULL(body_hit); /* the prose-only match still SURFACES ... */ + ASSERT_TRUE(name_hit < body_hit); /* ... but never above the identifier */ + free(inner); + + cbm_mcp_server_free(srv); + PASS(); +} + +TEST(bm25_searches_legacy_four_column_fts_without_error_issue518) { + /* No index-format bump means a database whose nodes_fts predates the body + * column is opened by the current binary. bm25()'s fifth weight must be + * inert there, not an error: FTS5 reads a weight only for a column an + * instance actually landed in. */ + char *td = th_mktempdir("cbm_mcp_legacy_fts"); + ASSERT_NOT_NULL(td); + /* cbm_mcp_server_new(project) opens /.db, so seeding + * the legacy table THERE is what puts the server on a pre-body database — + * the same way a real upgrade finds one. */ + char saved_cache[512] = {0}; + const char *prev = getenv("CBM_CACHE_DIR"); + if (prev) { + snprintf(saved_cache, sizeof(saved_cache), "%s", prev); + } + cbm_setenv("CBM_CACHE_DIR", td, 1); + + const char *proj = "legacyfts"; + char dbpath[600]; + snprintf(dbpath, sizeof(dbpath), "%s/%s.db", td, proj); + + sqlite3 *raw = NULL; + ASSERT_EQ(sqlite3_open(dbpath, &raw), SQLITE_OK); + ASSERT_EQ(sqlite3_exec(raw, + "CREATE VIRTUAL TABLE nodes_fts USING fts5(" + " name, qualified_name, label, file_path," + " content='', tokenize='unicode61 remove_diacritics 2');", + NULL, NULL, NULL), + SQLITE_OK); + sqlite3_close(raw); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(proj); + ASSERT_NOT_NULL(srv); + cbm_store_t *st = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(st); + cbm_mcp_server_set_project(srv, proj); + cbm_store_upsert_project(st, proj, td); + + cbm_node_t fn = {.project = proj, + .label = "Function", + .name = "reconcile", + .qualified_name = "legacy.a.reconcile", + .file_path = "a.c", + .properties_json = "{\"docstring\":\"prose that cannot be indexed here\"}"}; + ASSERT_TRUE(cbm_store_upsert_node(st, &fn) > 0); + ASSERT_EQ(cbm_store_fts_rebuild(st, NULL, 0), CBM_STORE_OK); + + char *inner = prose_search(srv, proj, "reconcile"); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "search_mode: bm25")); + ASSERT_NOT_NULL(strstr(inner, "legacy.a.reconcile")); + free(inner); + + /* Prose is absent rather than broken — a degrade, not a failure. */ + inner = prose_search(srv, proj, "indexed"); + ASSERT_NOT_NULL(inner); + ASSERT_NULL(strstr(inner, "legacy.a.reconcile")); + free(inner); + + cbm_mcp_server_free(srv); + if (saved_cache[0]) { + cbm_setenv("CBM_CACHE_DIR", saved_cache, 1); + } else { + cbm_unsetenv("CBM_CACHE_DIR"); + } + th_rmtree(td); + PASS(); +} + SUITE(mcp) { + /* #518/#519 — BM25 prose search */ + RUN_TEST(bm25_finds_section_by_its_prose_issue518); + RUN_TEST(bm25_finds_module_by_promoted_description_issue519); + RUN_TEST(bm25_results_and_total_stay_consistent_issue518); + RUN_TEST(bm25_identifier_match_outranks_prose_only_match_issue518); + RUN_TEST(bm25_searches_legacy_four_column_fts_without_error_issue518); RUN_TEST(mcp_path_within_root_rejects_escape); RUN_TEST(detect_changes_rejects_option_like_base_branch); RUN_TEST(detect_changes_rejects_windows_cmd_metacharacters_in_base_branch); diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 564792d2b8..3f874bd19b 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -12457,6 +12457,77 @@ TEST(pipeline_ensemble_routing_method_scoping) { PASS(); } +/* #518/#519 item-7 regression: the DELTA merge is the warm path most users + * hit. It used to write nodes_fts with a hand-rolled four-column INSERT of + * its own; with a fifth `body` column that literal leaves prose NULL for every + * node arriving by delta, invisibly — a full reindex still looks perfect. + * Routing the write through cbm_store_fts_rebuild() is what this test binds: + * revert pipeline_delta.c to a four-column INSERT and it fails. */ +TEST(pipeline_delta_patch_indexes_docstring_into_fts_body) { + char *td = th_mktempdir("cbm_delta_fts"); + ASSERT_NOT_NULL(td); + char dbpath[512]; + snprintf(dbpath, sizeof(dbpath), "%s/delta.db", td); + + cbm_store_t *store = cbm_store_open_path(dbpath); + ASSERT_NOT_NULL(store); + const char *proj = "deltafts"; + ASSERT_EQ(cbm_store_upsert_project(store, proj, td), CBM_STORE_OK); + + /* A previous generation already on disk, so the patch runs against a real + * id watermark rather than an empty database. */ + cbm_node_t existing = {.project = proj, + .label = "Function", + .name = "alreadyThere", + .qualified_name = "deltafts.main.alreadyThere", + .file_path = "main.c"}; + ASSERT_TRUE(cbm_store_upsert_node(store, &existing) > 0); + ASSERT_EQ(cbm_store_fts_rebuild(store, NULL, 0), CBM_STORE_OK); + + /* The delta patch: preseed proxies the resident rows and sets the + * watermark, then the NEW node is minted above it. */ + cbm_gbuf_t *gb = cbm_gbuf_new(proj, td); + ASSERT_NOT_NULL(gb); + int64_t max_db_id = cbm_delta_preseed(store, proj, gb); + ASSERT_TRUE(max_db_id > 0); + int64_t new_id = cbm_gbuf_upsert_node( + gb, "Section", "Upgrading", "deltafts.README.Upgrading", "README.md", 1, 9, + "{\"docstring\":\"migrates the retention ledger before the cutover\"}"); + ASSERT_TRUE(new_id > max_db_id); + + ASSERT_EQ(cbm_delta_patch(store, proj, gb, max_db_id, NULL, 0), 0); + cbm_gbuf_free(gb); + + /* nodes_fts is contentless, so a column-filtered MATCH is the only way to + * observe WHICH column a token landed in — and the only assertion that + * distinguishes "indexed" from "indexed without its prose". */ + sqlite3_stmt *st = NULL; + ASSERT_EQ(sqlite3_prepare_v2(cbm_store_get_db(store), + "SELECT rowid FROM nodes_fts WHERE nodes_fts MATCH ?1", -1, &st, + NULL), + SQLITE_OK); + sqlite3_bind_text(st, 1, "body:retention", -1, SQLITE_TRANSIENT); + ASSERT_EQ(sqlite3_step(st), SQLITE_ROW); + ASSERT_EQ((long long)sqlite3_column_int64(st, 0), (long long)new_id); + ASSERT_EQ(sqlite3_step(st), SQLITE_DONE); + sqlite3_finalize(st); + + /* The identifier columns are still written on the same path. */ + st = NULL; + ASSERT_EQ(sqlite3_prepare_v2(cbm_store_get_db(store), + "SELECT COUNT(*) FROM nodes_fts WHERE nodes_fts MATCH ?1", -1, &st, + NULL), + SQLITE_OK); + sqlite3_bind_text(st, 1, "name:Upgrading", -1, SQLITE_TRANSIENT); + ASSERT_EQ(sqlite3_step(st), SQLITE_ROW); + ASSERT_EQ(sqlite3_column_int(st, 0), 1); + sqlite3_finalize(st); + + cbm_store_close(store); + th_rmtree(td); + PASS(); +} + SUITE(pipeline) { RUN_TEST(pipeline_lsp_surface_persisted_and_body_edit_invariant); /* Index lock */ @@ -12774,6 +12845,7 @@ SUITE(pipeline) { RUN_TEST(pipeline_ensemble_routing_attr_does_not_leak_across_items); RUN_TEST(pipeline_ensemble_routing_settings_targets); RUN_TEST(pipeline_ensemble_routing_unterminated_item_is_safe); + RUN_TEST(pipeline_delta_patch_indexes_docstring_into_fts_body); } /* Focused semantic-manifest and publication contracts. Kept separate from the diff --git a/tests/test_store_search.c b/tests/test_store_search.c index a82b15240a..814cb5fefc 100644 --- a/tests/test_store_search.c +++ b/tests/test_store_search.c @@ -8,6 +8,7 @@ #include "test_framework.h" #include "test_helpers.h" #include +#include "sqlite3.h" /* vendored/sqlite3 — raw nodes_fts MATCH probes */ #include #include #include @@ -1629,6 +1630,172 @@ TEST(store_find_nodes_rejects_null_store_without_ub) { PASS(); } +/* ── nodes_fts prose column (#518 / #519) ────────────────────────── + * + * nodes_fts is CONTENTLESS, so a column's value cannot be selected back — a + * MATCH with a column filter is the only way to prove a token landed in `body` + * rather than in one of the identifier columns. That distinction IS the test: + * a four-column INSERT still "works", it just silently indexes no prose. */ + +static int fts_match_count(cbm_store_t *s, const char *match) { + sqlite3 *db = cbm_store_get_db(s); + sqlite3_stmt *st = NULL; + if (sqlite3_prepare_v2(db, "SELECT COUNT(*) FROM nodes_fts WHERE nodes_fts MATCH ?1", -1, &st, + NULL) != SQLITE_OK) { + return -1; + } + sqlite3_bind_text(st, 1, match, -1, SQLITE_TRANSIENT); + int n = sqlite3_step(st) == SQLITE_ROW ? sqlite3_column_int(st, 0) : -1; + sqlite3_finalize(st); + return n; +} + +/* One Section carrying prose, one Function carrying none. */ +static void seed_prose_nodes(cbm_store_t *s) { + cbm_store_upsert_project(s, "p", "/tmp/p"); + cbm_node_t sec = {.project = "p", + .label = "Section", + .name = "Installation", + .qualified_name = "p.README.Installation", + .file_path = "README.md", + .properties_json = + "{\"docstring\":\"provisions an ephemeral workstation runner\"}"}; + cbm_store_upsert_node(s, &sec); + cbm_node_t fn = {.project = "p", + .label = "Function", + .name = "plainFunction", + .qualified_name = "p.main.plainFunction", + .file_path = "main.c"}; + cbm_store_upsert_node(s, &fn); +} + +static cbm_store_t *setup_prose_store(void) { + cbm_store_t *s = cbm_store_open_memory(); + seed_prose_nodes(s); + return s; +} + +TEST(store_fts_rebuild_indexes_docstring_as_body_issue518) { + cbm_store_t *s = setup_prose_store(); + ASSERT_EQ(cbm_store_fts_rebuild(s, NULL, 0), CBM_STORE_OK); + + /* The prose is in `body` — the column BM25 weights at 0.3. */ + ASSERT_EQ(fts_match_count(s, "body:ephemeral"), 1); + ASSERT_EQ(fts_match_count(s, "body:workstation"), 1); + /* ...and specifically NOT smeared into an identifier column. */ + ASSERT_EQ(fts_match_count(s, "name:ephemeral"), 0); + /* A node with no docstring contributes no body tokens. */ + ASSERT_EQ(fts_match_count(s, "body:plainFunction"), 0); + /* Identifier indexing is untouched. */ + ASSERT_EQ(fts_match_count(s, "name:plainFunction"), 1); + + cbm_store_close(s); + PASS(); +} + +TEST(store_fts_rebuild_survives_malformed_properties_json) { + /* Pre-fix databases contain rows whose properties JSON does not parse. + * json_extract() RAISES on those, so an unguarded backfill would abort + * outright and leave the whole index empty — the same trap that reverted + * the is_entry_point expression index. */ + cbm_store_t *s = setup_prose_store(); + cbm_node_t broken = {.project = "p", + .label = "Function", + .name = "brokenProps", + .qualified_name = "p.main.brokenProps", + .file_path = "main.c", + .properties_json = "{\"docstring\":\"unterminated"}; + ASSERT_TRUE(cbm_store_upsert_node(s, &broken) > 0); + + ASSERT_EQ(cbm_store_fts_rebuild(s, NULL, 0), CBM_STORE_OK); + ASSERT_EQ(fts_match_count(s, "name:brokenProps"), 1); + ASSERT_EQ(fts_match_count(s, "body:ephemeral"), 1); + + cbm_store_close(s); + PASS(); +} + +TEST(store_fts_rebuild_tolerates_legacy_four_column_table) { + /* CBM_INDEX_FORMAT_VERSION deliberately does NOT move for the body column, + * so real users open databases whose nodes_fts predates it. Reproduce that + * exactly: lay down the four-column table first, then let the current + * store open the file — CREATE VIRTUAL TABLE IF NOT EXISTS leaves it be. */ + char *td = th_mktempdir("cbm_fts_legacy"); + ASSERT_NOT_NULL(td); + char path[512]; + snprintf(path, sizeof(path), "%s/legacy.db", td); + + sqlite3 *raw = NULL; + ASSERT_EQ(sqlite3_open(path, &raw), SQLITE_OK); + ASSERT_EQ(sqlite3_exec(raw, + "CREATE VIRTUAL TABLE nodes_fts USING fts5(" + " name, qualified_name, label, file_path," + " content='', tokenize='unicode61 remove_diacritics 2');", + NULL, NULL, NULL), + SQLITE_OK); + sqlite3_close(raw); + + cbm_store_t *s = cbm_store_open_path(path); + ASSERT_NOT_NULL(s); /* a legacy database still OPENS */ + seed_prose_nodes(s); + + /* ...and still backfills, degrading to the four-column write. */ + ASSERT_EQ(cbm_store_fts_rebuild(s, NULL, 0), CBM_STORE_OK); + ASSERT_EQ(fts_match_count(s, "name:plainFunction"), 1); + ASSERT_EQ(fts_match_count(s, "qualified_name:Installation"), 1); + /* No prose, exactly as promised — the words are simply not in the index. */ + ASSERT_EQ(fts_match_count(s, "ephemeral"), 0); + + /* The ranked query passes FIVE column weights. On a four-column table the + * fifth is never consulted (FTS5 reads a weight only for a column an + * instance landed in), so the SAME expression the search uses must run + * here without error rather than needing a second, forked query. */ + sqlite3_stmt *ranked = NULL; + ASSERT_EQ(sqlite3_prepare_v2(cbm_store_get_db(s), + "SELECT rowid, bm25(nodes_fts, 1.0, 1.0, 1.0, 1.0, 0.3) AS r" + " FROM nodes_fts WHERE nodes_fts MATCH ?1 ORDER BY r LIMIT 10", + -1, &ranked, NULL), + SQLITE_OK); + sqlite3_bind_text(ranked, 1, "plainFunction", -1, SQLITE_TRANSIENT); + ASSERT_EQ(sqlite3_step(ranked), SQLITE_ROW); + ASSERT_EQ(sqlite3_step(ranked), SQLITE_DONE); /* stepped to completion: no error */ + sqlite3_finalize(ranked); + + cbm_store_close(s); + th_rmtree(td); + PASS(); +} + +TEST(store_fts_rebuild_incremental_adds_only_nodes_above_watermark) { + cbm_store_t *s = setup_prose_store(); + ASSERT_EQ(cbm_store_fts_rebuild(s, NULL, 0), CBM_STORE_OK); + + sqlite3_stmt *st = NULL; + ASSERT_EQ(sqlite3_prepare_v2(cbm_store_get_db(s), "SELECT COALESCE(MAX(id),0) FROM nodes", -1, + &st, NULL), + SQLITE_OK); + ASSERT_EQ(sqlite3_step(st), SQLITE_ROW); + int64_t watermark = sqlite3_column_int64(st, 0); + sqlite3_finalize(st); + + cbm_node_t added = {.project = "p", + .label = "Section", + .name = "Upgrading", + .qualified_name = "p.README.Upgrading", + .file_path = "README.md", + .properties_json = "{\"docstring\":\"migrates the retention ledger\"}"}; + ASSERT_TRUE(cbm_store_upsert_node(s, &added) > 0); + + ASSERT_EQ(cbm_store_fts_rebuild(s, "p", watermark), CBM_STORE_OK); + ASSERT_EQ(fts_match_count(s, "body:retention"), 1); + /* Pre-existing rows are not duplicated by the incremental pass. */ + ASSERT_EQ(fts_match_count(s, "body:ephemeral"), 1); + ASSERT_EQ(fts_match_count(s, "name:plainFunction"), 1); + + cbm_store_close(s); + PASS(); +} + SUITE(store_search) { RUN_TEST(store_search_by_label); RUN_TEST(store_search_by_name_pattern); @@ -1700,4 +1867,9 @@ SUITE(store_search) { RUN_TEST(store_risk_label_all_levels); RUN_TEST(store_impact_summary_empty); RUN_TEST(store_find_nodes_rejects_null_store_without_ub); + /* #518/#519 — nodes_fts prose column */ + RUN_TEST(store_fts_rebuild_indexes_docstring_as_body_issue518); + RUN_TEST(store_fts_rebuild_survives_malformed_properties_json); + RUN_TEST(store_fts_rebuild_tolerates_legacy_four_column_table); + RUN_TEST(store_fts_rebuild_incremental_adds_only_nodes_above_watermark); } From b493b644cf4e77f4ff9bd5b30862b01ba7ed3198 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 28 Aug 2026 18:33:48 +0200 Subject: [PATCH 2/3] test(search): end-to-end guard from source prose to a findable body match The #518/#519 tests so far each cover one layer: extraction produces the docstring, the store indexes it into nodes_fts.body, the delta merge writes the same columns, and the query returns Section and Module. Nothing yet proved the layers connect. This indexes a real temporary repository -- a README.md whose section body runs well past the 500-byte cap, plus a META.yaml with a top-level description -- through the full pipeline, then asserts both prose words are matchable in the `body` column. It is also the guard on the size budget. build_def_props drops an oversized field ATOMICALLY, so a section body that did not fit the 2 KB properties buffer would vanish without a trace and every narrower test would still pass. Signed-off-by: Martin Vogel --- tests/test_pipeline.c | 66 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 3f874bd19b..1eb10290d8 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -12528,6 +12528,71 @@ TEST(pipeline_delta_patch_indexes_docstring_into_fts_body) { PASS(); } + +/* End-to-end for #518/#519: source → docstring → properties JSON → nodes_fts + * `body` → findable. Each layer has its own test; this one proves they connect. + * It is also the guard on the size budget: build_def_props drops an oversized + * field ATOMICALLY, so a 500-byte section body that did not fit the 2 KB + * properties buffer would vanish silently and every narrower test would still + * pass. */ +TEST(pipeline_markdown_and_config_prose_reaches_fts_body) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_prose_XXXXXX"); + ASSERT_NOT_NULL(cbm_mkdtemp(tmp)); + char path[512]; + char dbpath[512]; + snprintf(dbpath, sizeof(dbpath), "%s/prose.db", tmp); + + /* A section body well past the 500-byte cap, so the truncating path — the + * one that produces the longest properties JSON — is what gets indexed. */ + snprintf(path, sizeof(path), "%s/README.md", tmp); + FILE *f = fopen(path, "w"); + ASSERT_NOT_NULL(f); + fprintf(f, "# Installation\n\nThe phlogiston bootstrap provisions a workstation.\n"); + for (int i = 0; i < 40; i++) { + fprintf(f, "Filler prose line %d that pads the section well past the cap.\n", i); + } + fclose(f); + + snprintf(path, sizeof(path), "%s/META.yaml", tmp); + f = fopen(path, "w"); + ASSERT_NOT_NULL(f); + fprintf(f, "name: widget\ndescription: Aggregates quicksilver telemetry per shard.\n"); + fclose(f); + + /* One code file so the run is a normal index rather than a docs-only edge. */ + snprintf(path, sizeof(path), "%s/main.go", tmp); + f = fopen(path, "w"); + ASSERT_NOT_NULL(f); + fprintf(f, "package main\n\nfunc main() {}\n"); + fclose(f); + + cbm_pipeline_t *p = cbm_pipeline_new(tmp, dbpath, CBM_MODE_FULL); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + cbm_pipeline_free(p); + + cbm_store_t *s = cbm_store_open_path(dbpath); + ASSERT_NOT_NULL(s); + sqlite3_stmt *st = NULL; + ASSERT_EQ(sqlite3_prepare_v2(cbm_store_get_db(s), + "SELECT COUNT(*) FROM nodes_fts WHERE nodes_fts MATCH ?1", -1, &st, + NULL), + SQLITE_OK); + const char *cases[] = {"body:phlogiston", "body:quicksilver"}; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + sqlite3_reset(st); + sqlite3_bind_text(st, 1, cases[i], -1, SQLITE_TRANSIENT); + ASSERT_EQ(sqlite3_step(st), SQLITE_ROW); + ASSERT_GT(sqlite3_column_int(st, 0), 0); + } + sqlite3_finalize(st); + cbm_store_close(s); + + th_rmtree(tmp); + PASS(); +} + SUITE(pipeline) { RUN_TEST(pipeline_lsp_surface_persisted_and_body_edit_invariant); /* Index lock */ @@ -12846,6 +12911,7 @@ SUITE(pipeline) { RUN_TEST(pipeline_ensemble_routing_settings_targets); RUN_TEST(pipeline_ensemble_routing_unterminated_item_is_safe); RUN_TEST(pipeline_delta_patch_indexes_docstring_into_fts_body); + RUN_TEST(pipeline_markdown_and_config_prose_reaches_fts_body); } /* Focused semantic-manifest and publication contracts. Kept separate from the From dc23628c6f00a818466fe09f0f180413333aee80 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 28 Aug 2026 18:47:05 +0200 Subject: [PATCH 3/3] test(search): pin that nodes_fts.body is indexed raw, not camelCase-split Only `name` goes through cbm_camel_split; qualified_name, label, file_path and now `body` are indexed verbatim. `body` is natural-language prose, so splitting it would mangle the very text the column exists to make findable, and the unicode61 tokenizer already handles prose correctly. That invariant lived only in the shape of one format string, where the splitter is substituted into the name position alone. Nothing failed if a later edit wrapped the body expression too. cbm_camel_split("getUserById") yields "getUserById get User By Id", so a split body would additionally match the fragment "User" -- seeding a camelCase identifier into the prose and asserting the fragment does NOT match turns the invariant into a test. The mirror assertion, that `name` DOES match its fragment, pins the other half so the guard cannot be satisfied by simply indexing nothing. Verified binding: wrapping the body expression in cbm_camel_split makes body:User match 1 instead of 0 and the test fails. Signed-off-by: Martin Vogel --- tests/test_store_search.c | 45 ++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/tests/test_store_search.c b/tests/test_store_search.c index 814cb5fefc..9110c420ae 100644 --- a/tests/test_store_search.c +++ b/tests/test_store_search.c @@ -1008,10 +1008,8 @@ TEST(store_bfs_reachability_is_not_trail_capped) { cbm_store_t *s = cbm_store_open_memory(); cbm_store_upsert_project(s, "test", "/tmp/test"); - cbm_node_t root = {.project = "test", - .label = "Function", - .name = "Root", - .qualified_name = "test.Root"}; + cbm_node_t root = { + .project = "test", .label = "Function", .name = "Root", .qualified_name = "test.Root"}; int64_t root_id = cbm_store_upsert_node(s, &root); enum { NODE_COUNT = 4200 }; @@ -1083,18 +1081,12 @@ TEST(store_bfs_trail_preserves_deeper_match_under_hub_budget) { cbm_store_t *s = cbm_store_open_memory(); cbm_store_upsert_project(s, "test", "/tmp/test"); - cbm_node_t root = {.project = "test", - .label = "Function", - .name = "root", - .qualified_name = "test.root"}; - cbm_node_t branch = {.project = "test", - .label = "Function", - .name = "branch", - .qualified_name = "test.branch"}; - cbm_node_t target = {.project = "test", - .label = "Function", - .name = "target", - .qualified_name = "test.target"}; + cbm_node_t root = { + .project = "test", .label = "Function", .name = "root", .qualified_name = "test.root"}; + cbm_node_t branch = { + .project = "test", .label = "Function", .name = "branch", .qualified_name = "test.branch"}; + cbm_node_t target = { + .project = "test", .label = "Function", .name = "target", .qualified_name = "test.target"}; int64_t root_id = cbm_store_upsert_node(s, &root); int64_t branch_id = cbm_store_upsert_node(s, &branch); int64_t target_id = cbm_store_upsert_node(s, &target); @@ -1112,10 +1104,8 @@ TEST(store_bfs_trail_preserves_deeper_match_under_hub_budget) { for (int i = 0; i < LEAF_COUNT; i++) { char name[32]; snprintf(name, sizeof(name), "leaf-%d", i); - cbm_node_t leaf = {.project = "test", - .label = "Function", - .name = name, - .qualified_name = name}; + cbm_node_t leaf = { + .project = "test", .label = "Function", .name = name, .qualified_name = name}; int64_t leaf_id = cbm_store_upsert_node(s, &leaf); edge.source_id = root_id; edge.target_id = leaf_id; @@ -1658,8 +1648,8 @@ static void seed_prose_nodes(cbm_store_t *s) { .name = "Installation", .qualified_name = "p.README.Installation", .file_path = "README.md", - .properties_json = - "{\"docstring\":\"provisions an ephemeral workstation runner\"}"}; + .properties_json = "{\"docstring\":\"provisions an ephemeral " + "workstation runner via getUserById\"}"}; cbm_store_upsert_node(s, &sec); cbm_node_t fn = {.project = "p", .label = "Function", @@ -1689,6 +1679,17 @@ TEST(store_fts_rebuild_indexes_docstring_as_body_issue518) { /* Identifier indexing is untouched. */ ASSERT_EQ(fts_match_count(s, "name:plainFunction"), 1); + /* `body` is PROSE and must be indexed RAW — only `name` gets the camelCase + * splitter. cbm_camel_split("getUserById") yields "getUserById get User By + * Id", so a split body would additionally match the fragment "User". + * Asserting the fragment does NOT match is what makes this test fail if + * anyone ever wraps the body expression in cbm_camel_split(). */ + ASSERT_EQ(fts_match_count(s, "body:getUserById"), 1); + ASSERT_EQ(fts_match_count(s, "body:User"), 0); + /* ...and the mirror image: `name` IS split, so its fragment DOES match. + * Together these pin both halves of the invariant. */ + ASSERT_EQ(fts_match_count(s, "name:plain"), 1); + cbm_store_close(s); PASS(); }