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..1eb10290d8 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -12457,6 +12457,142 @@ 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(); +} + + +/* 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 */ @@ -12774,6 +12910,8 @@ 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); + RUN_TEST(pipeline_markdown_and_config_prose_reaches_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..9110c420ae 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 @@ -1007,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 }; @@ -1082,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); @@ -1111,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; @@ -1629,6 +1620,183 @@ 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 via getUserById\"}"}; + 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); + + /* `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(); +} + +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 +1868,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); }