Skip to content

Commit e449fe0

Browse files
committed
search: actually score body matches; preserve bm25 through hybrid merge
Two interlocking bugs caused lexical-search results that should have been near-perfect (e.g. `codescan search BASH_VERSION` matching a function with BASH_VERSION literally in its body) to either disappear or rank with lex=0.0 / lex=0.4 instead of ~1.0: 1. body never made it from SQL into the scorer `tokenCoverage` and `lexicalScore` checked name/sig/doc_comment but not `body`. Worse, `body` wasn't selected in any of the 5 candidate SQL queries (browse/vector/like/comment/fts) and `readResultRow` never loaded it — so even after fixing the heuristics there'd be nothing to score against. FTS5 was happily indexing the body column and finding matches, then the post-FTS layer floored body-only matches to 0.1 "coverage". The original code even has a comment admitting it. Fix: thread body through the SQL → Result pipeline, and let both coverage and per-token weighting see it (body weight 0.4, between doc and signature). 2. hybrid merge dropped bm25 on dedup In hybrid mode, when a symbol appeared in both vector and FTS candidate lists, the dedup logic dropped the FTS Result entirely — losing its bm25 score. The kept vector row had bm25=0, so scoring fell back to lexicalScore's body-weight (0.4) instead of the bm25_norm * coverage path (which gives ~1.0 for the obvious match). Fix: when dedupping, transfer the FTS bm25 onto the existing row before discarding. On a real dotfiles index: `codescan search BASH_VERSION` now returns .profile's in_bash() at #1 with lex=1.000 (was 0.400 or buried). No reindex needed — body was already in the DB; this is purely read/score plumbing. TDD'd with 4 new tests covering both bugs in isolation and combined.
1 parent a693a29 commit e449fe0

1 file changed

Lines changed: 226 additions & 9 deletions

File tree

src/search.zig

Lines changed: 226 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,17 @@ pub fn search(
178178

179179
for (lexical) |res| {
180180
if (seen.contains(res.id)) {
181+
// Symbol already came back from vector candidates with
182+
// bm25=0. Transfer the FTS bm25 onto the existing row so
183+
// the scorer can use it; otherwise we throw away the FTS
184+
// signal and the merged result scores like a vector-only
185+
// hit.
186+
for (results.items) |*existing| {
187+
if (existing.id == res.id) {
188+
existing.bm25 = res.bm25;
189+
break;
190+
}
191+
}
181192
var tmp = res;
182193
tmp.deinit(allocator);
183194
} else {
@@ -455,7 +466,7 @@ fn browseSymbols(allocator: std.mem.Allocator, db: storage.Db, options: Options)
455466
defer sql_buf.deinit(allocator);
456467
try sql_buf.appendSlice(allocator,
457468
"SELECT id, lang, file_path, start_line, start_hash, end_line, end_hash, symbol_name, signature, doc_comment, " ++
458-
"symbol_kind, symbol_visibility, symbol_scope, symbol_arity, " ++
469+
"symbol_kind, symbol_visibility, symbol_scope, symbol_arity, body, " ++
459470
"0.0 AS distance " ++
460471
"FROM symbols"
461472
);
@@ -661,7 +672,7 @@ fn vectorCandidates(
661672
const sql = try allocPrintZ(
662673
allocator,
663674
"SELECT symbols.id, lang, file_path, start_line, start_hash, end_line, end_hash, symbol_name, signature, doc_comment, "
664-
++ "symbol_kind, symbol_visibility, symbol_scope, symbol_arity, "
675+
++ "symbol_kind, symbol_visibility, symbol_scope, symbol_arity, body, "
665676
++ "knn.distance "
666677
++ "FROM {s} AS knn JOIN symbols ON knn.rowid = symbols.id "
667678
++ "WHERE knn.embedding MATCH vec_f32(?1) AND k = ?2 "
@@ -741,7 +752,7 @@ fn likeCandidates(
741752
// ?1 = %query% (LIKE pattern), ?2 = query (exact), ?3 = query% (prefix), ?4 = limit
742753
const sql: [:0]const u8 =
743754
"SELECT id, lang, file_path, start_line, start_hash, end_line, end_hash, symbol_name, signature, doc_comment, "
744-
++ "symbol_kind, symbol_visibility, symbol_scope, symbol_arity, "
755+
++ "symbol_kind, symbol_visibility, symbol_scope, symbol_arity, body, "
745756
++ "1e999 AS distance "
746757
++ "FROM symbols "
747758
++ "WHERE symbol_name LIKE ?1 COLLATE NOCASE "
@@ -803,7 +814,7 @@ fn commentCandidates(
803814

804815
const sql: [:0]const u8 =
805816
"SELECT id, lang, file_path, start_line, start_hash, end_line, end_hash, symbol_name, signature, doc_comment, "
806-
++ "symbol_kind, symbol_visibility, symbol_scope, symbol_arity, "
817+
++ "symbol_kind, symbol_visibility, symbol_scope, symbol_arity, body, "
807818
++ "1e999 AS distance "
808819
++ "FROM symbols "
809820
++ "WHERE doc_comment IS NOT NULL "
@@ -858,7 +869,7 @@ fn ftsCandidates(
858869
allocator,
859870
"SELECT symbols.id, symbols.lang, symbols.file_path, symbols.start_line, symbols.start_hash, "
860871
++ "symbols.end_line, symbols.end_hash, symbols.symbol_name, symbols.signature, symbols.doc_comment, "
861-
++ "symbols.symbol_kind, symbols.symbol_visibility, symbols.symbol_scope, symbols.symbol_arity, "
872+
++ "symbols.symbol_kind, symbols.symbol_visibility, symbols.symbol_scope, symbols.symbol_arity, symbols.body, "
862873
++ "1e999 AS distance, "
863874
++ "bm25(symbols_fts, 10.0, 3.0, 5.0, 1.0, 0.5) AS bm25_score "
864875
++ "FROM symbols_fts JOIN symbols ON symbols_fts.rowid = symbols.id "
@@ -872,7 +883,7 @@ fn ftsCandidates(
872883
allocator,
873884
"SELECT symbols.id, symbols.lang, symbols.file_path, symbols.start_line, symbols.start_hash, "
874885
++ "symbols.end_line, symbols.end_hash, symbols.symbol_name, symbols.signature, symbols.doc_comment, "
875-
++ "symbols.symbol_kind, symbols.symbol_visibility, symbols.symbol_scope, symbols.symbol_arity, "
886+
++ "symbols.symbol_kind, symbols.symbol_visibility, symbols.symbol_scope, symbols.symbol_arity, symbols.body, "
876887
++ "1e999 AS distance, "
877888
++ "bm25(symbols_fts, 10.0, 3.0, 5.0, 1.0, 0.5) AS bm25_score "
878889
++ "FROM symbols_fts JOIN symbols ON symbols_fts.rowid = symbols.id "
@@ -900,7 +911,7 @@ fn ftsCandidates(
900911
const rc = sqlite.sqlite3_step(stmt.?);
901912
if (rc == sqlite.SQLITE_ROW) {
902913
var res = try readResultRow(allocator, stmt.?);
903-
res.bm25 = @as(f32, @floatCast(sqlite.sqlite3_column_double(stmt.?, 15)));
914+
res.bm25 = @as(f32, @floatCast(sqlite.sqlite3_column_double(stmt.?, 16)));
904915
try results.append(allocator, res);
905916
} else if (rc == sqlite.SQLITE_DONE) {
906917
break;
@@ -927,7 +938,8 @@ fn readResultRow(allocator: std.mem.Allocator, stmt: *sqlite.sqlite3_stmt) !Resu
927938
const symbol_visibility = try dupColumnTextOptional(allocator, stmt, 11);
928939
const symbol_scope = try dupColumnTextOptional(allocator, stmt, 12);
929940
const symbol_arity = readIntColumnOptional(stmt, 13);
930-
const distance = @as(f32, @floatCast(sqlite.sqlite3_column_double(stmt, 14)));
941+
const body = try dupColumnTextOptional(allocator, stmt, 14);
942+
const distance = @as(f32, @floatCast(sqlite.sqlite3_column_double(stmt, 15)));
931943

932944
return .{
933945
.id = id,
@@ -941,6 +953,7 @@ fn readResultRow(allocator: std.mem.Allocator, stmt: *sqlite.sqlite3_stmt) !Resu
941953
.symbol_visibility = symbol_visibility,
942954
.symbol_scope = symbol_scope,
943955
.symbol_arity = symbol_arity,
956+
.body = body,
944957
.start_line = start_line,
945958
.end_line = end_line,
946959
.start_hash = start_hash,
@@ -1016,7 +1029,8 @@ fn tokenCoverage(query_tokens: []const []const u8, symbol: model.Symbol) f32 {
10161029
const in_name = simd.indexOfIgnoreCase(symbol.name, tok) != null;
10171030
const in_sig = simd.indexOfIgnoreCase(symbol.signature, tok) != null;
10181031
const in_doc = if (symbol.doc_comment) |doc| simd.indexOfIgnoreCase(doc, tok) != null else false;
1019-
if (in_name or in_sig or in_doc) matched += 1;
1032+
const in_body = if (symbol.body) |b| simd.indexOfIgnoreCase(b, tok) != null else false;
1033+
if (in_name or in_sig or in_doc or in_body) matched += 1;
10201034
}
10211035
if (significant == 0) return 1.0;
10221036
return @as(f32, @floatFromInt(matched)) / @as(f32, @floatFromInt(significant));
@@ -1425,17 +1439,21 @@ fn lexicalScore(allocator: std.mem.Allocator, query_tokens: []const []const u8,
14251439
// name match → 1.0 (this symbol IS the thing)
14261440
// doc comment → 0.5 (described in docs)
14271441
// signature only → 0.3 (just referenced/called in body)
1442+
// body → 0.4 (referenced inside the implementation)
14281443
for (query_tokens) |tok| {
14291444
const in_doc = if (symbol.doc_comment) |doc| simd.indexOfIgnoreCase(doc, tok) != null else false;
14301445
if (comments_only) {
14311446
if (in_doc) weighted_score += 1.0;
14321447
} else {
14331448
const in_name = simd.indexOfIgnoreCase(symbol.name, tok) != null;
14341449
const in_sig = simd.indexOfIgnoreCase(symbol.signature, tok) != null;
1450+
const in_body = if (symbol.body) |b| simd.indexOfIgnoreCase(b, tok) != null else false;
14351451
if (in_name) {
14361452
weighted_score += 1.0;
14371453
} else if (in_doc) {
14381454
weighted_score += 0.5;
1455+
} else if (in_body) {
1456+
weighted_score += 0.4;
14391457
} else if (in_sig) {
14401458
weighted_score += 0.3;
14411459
} else {
@@ -1687,6 +1705,31 @@ test "lexicalScore comments_only ignores name and signature" {
16871705
try std.testing.expectApproxEqAbs(@as(f32, 0.0), score, 0.0001);
16881706
}
16891707

1708+
test "lexicalScore counts tokens that appear only in body" {
1709+
// Regression: a function whose body references the query terms but whose
1710+
// name/sig/doc don't should still score non-trivially. Otherwise FTS
1711+
// finds the candidate via body-column index but the post-FTS scorer
1712+
// kills its rank.
1713+
const allocator = std.testing.allocator;
1714+
var symbol = model.Symbol{
1715+
.language = try allocator.dupe(u8, "bash"),
1716+
.file_path = try allocator.dupe(u8, "bin/setup.sh"),
1717+
.name = try allocator.dupe(u8, "setup_env"),
1718+
.signature = try allocator.dupe(u8, "setup_env()"),
1719+
.doc_comment = null,
1720+
.body = try allocator.dupe(u8, "setup_env() { [ -n \"$BASH_VERSION\" ]; }"),
1721+
.start_line = 1,
1722+
.end_line = 1,
1723+
};
1724+
defer symbol.deinit(allocator);
1725+
1726+
// "bash" and "version" both appear in body (BASH_VERSION); neither in
1727+
// name/sig/doc. Score should be ~0.4 (body weight, 2/2 tokens matched).
1728+
// Stronger requirement: non-trivial, at least 0.2 to outrank pure floor.
1729+
const score = try testLexicalScore(allocator, "bash version", symbol, false);
1730+
try std.testing.expect(score >= 0.2);
1731+
}
1732+
16901733
test "search vector mode returns nearest symbol" {
16911734
const allocator = std.testing.allocator;
16921735
const db = try storage.openMemoryWithVec(allocator);
@@ -2012,6 +2055,180 @@ test "search lexical uses fts when available" {
20122055
}
20132056
}
20142057

2058+
test "tokenCoverage counts matches in body, not just name/sig/doc" {
2059+
// Regression: a function whose body references BASH_VERSION should count
2060+
// as covering "bash" and "version", even when the name/signature/doc
2061+
// don't mention either word. Without body coverage, FTS-matched results
2062+
// get hit with a 0.1 coverage floor and rank poorly.
2063+
const allocator = std.testing.allocator;
2064+
var sym = model.Symbol{
2065+
.language = try allocator.dupe(u8, "bash"),
2066+
.file_path = try allocator.dupe(u8, "bin/setup.sh"),
2067+
.name = try allocator.dupe(u8, "setup_env"),
2068+
.signature = try allocator.dupe(u8, "setup_env()"),
2069+
.doc_comment = null,
2070+
.body = try allocator.dupe(u8, "setup_env() { [ -n \"$BASH_VERSION\" ] && export FOO=1; }"),
2071+
.start_line = 1,
2072+
.end_line = 1,
2073+
};
2074+
defer sym.deinit(allocator);
2075+
2076+
const tokens = [_][]const u8{ "bash", "version" };
2077+
const cov = tokenCoverage(&tokens, sym);
2078+
// Both tokens are present (in body via BASH_VERSION); expect full coverage.
2079+
try std.testing.expectApproxEqAbs(@as(f32, 1.0), cov, 0.0001);
2080+
}
2081+
2082+
test "tokenCoverage returns 0 when neither token appears anywhere" {
2083+
const allocator = std.testing.allocator;
2084+
var sym = model.Symbol{
2085+
.language = try allocator.dupe(u8, "zig"),
2086+
.file_path = try allocator.dupe(u8, "src/a.zig"),
2087+
.name = try allocator.dupe(u8, "unrelated"),
2088+
.signature = try allocator.dupe(u8, "fn unrelated() void"),
2089+
.doc_comment = null,
2090+
.body = try allocator.dupe(u8, "fn unrelated() void { return; }"),
2091+
.start_line = 1,
2092+
.end_line = 1,
2093+
};
2094+
defer sym.deinit(allocator);
2095+
2096+
const tokens = [_][]const u8{ "bash", "version" };
2097+
const cov = tokenCoverage(&tokens, sym);
2098+
try std.testing.expectApproxEqAbs(@as(f32, 0.0), cov, 0.0001);
2099+
}
2100+
2101+
test "hybrid merge preserves bm25 when symbol is in both vector and FTS candidates" {
2102+
// Regression: in hybrid mode, when a symbol shows up in BOTH the vector
2103+
// candidate list and the FTS candidate list, the dedup logic dropped the
2104+
// FTS row entirely — losing its bm25 score. The result kept its
2105+
// vector-only bm25=0, so scoring fell back to the body-weight-only
2106+
// lexicalScore path and lex topped out at ~0.4 for body matches, instead
2107+
// of the ~1.0 that bm25_norm * coverage would have given it.
2108+
const allocator = std.testing.allocator;
2109+
const db = try storage.openMemoryWithVec(allocator);
2110+
defer storage.close(db);
2111+
2112+
_ = try storage.initSchema(allocator, db, .{ .embedding_dim = 2 });
2113+
2114+
var sym = model.Symbol{
2115+
.language = try allocator.dupe(u8, "bash"),
2116+
.file_path = try allocator.dupe(u8, ".profile"),
2117+
.name = try allocator.dupe(u8, "in_bash"),
2118+
.signature = try allocator.dupe(u8, "in_bash()"),
2119+
.doc_comment = null,
2120+
.body = try allocator.dupe(u8, "in_bash() { [ -n \"${BASH_VERSION+set}\" ]; }"),
2121+
.start_line = 1,
2122+
.end_line = 3,
2123+
};
2124+
defer sym.deinit(allocator);
2125+
2126+
const id = try storage.insertSymbol(db, sym);
2127+
// Symbol embedding matches the FakeEmbedder query embedding → vector
2128+
// will return this row as candidate #1.
2129+
try storage.insertEmbedding(db, allocator, id, &[_]f32{ 1.0, 0.0 });
2130+
2131+
const has_fts = try ftsAvailable(db);
2132+
if (!has_fts) return error.SkipZigTest;
2133+
2134+
var fake = FakeEmbedder{ .vector = &[_]f32{ 1.0, 0.0 } };
2135+
const results = (try search(allocator, db, fake.embedder(), "BASH_VERSION", .{
2136+
.top_n = 5,
2137+
.mode = .hybrid,
2138+
})).results;
2139+
defer freeResults(allocator, results);
2140+
2141+
try std.testing.expect(results.len == 1);
2142+
try std.testing.expectEqualStrings("in_bash", results[0].symbol.name);
2143+
2144+
// The FTS bm25 must survive the merge — otherwise lex stays at the
2145+
// lexicalScore body-weight (0.4) and the user sees lex=0.400 instead of
2146+
// the ~1.0 they'd expect for a literal BASH_VERSION hit.
2147+
try std.testing.expect(results[0].bm25 != 0);
2148+
try std.testing.expect(results[0].lexical > 0.5);
2149+
}
2150+
2151+
test "search surfaces body-only multi-token match (BASH_VERSION regression)" {
2152+
// User's original complaint: searching "bash version" failed to surface
2153+
// symbols whose body literally references BASH_VERSION. The bug was that
2154+
// body wasn't loaded into Result rows AND the post-FTS scorer ignored body
2155+
// for both coverage and per-token weighting, so body-only matches were
2156+
// cratered to ~0.1 (coverage floor) regardless of bm25.
2157+
//
2158+
// Minimum regression bar: the BASH_VERSION-using symbol must appear in
2159+
// results with a non-trivial lexical score, AND must outrank a symbol
2160+
// that only covers ONE of the two tokens via body (no name competitor —
2161+
// bm25's symbol_name weight of 10x makes name-match comparisons noisy).
2162+
const allocator = std.testing.allocator;
2163+
const db = try storage.openMemoryWithVec(allocator);
2164+
defer storage.close(db);
2165+
2166+
_ = try storage.initSchema(allocator, db, .{ .embedding_dim = 2 });
2167+
2168+
// Symbol A: both query tokens covered via body (BASH_VERSION).
2169+
var sym_a = model.Symbol{
2170+
.language = try allocator.dupe(u8, "bash"),
2171+
.file_path = try allocator.dupe(u8, "bin/setup.sh"),
2172+
.name = try allocator.dupe(u8, "setup_env"),
2173+
.signature = try allocator.dupe(u8, "setup_env()"),
2174+
.doc_comment = null,
2175+
.body = try allocator.dupe(u8, "setup_env() { [ -n \"$BASH_VERSION\" ] && export FOO=1; }"),
2176+
.start_line = 1,
2177+
.end_line = 1,
2178+
};
2179+
defer sym_a.deinit(allocator);
2180+
2181+
// Symbol B: only "bash" appears (in body, no name competitor).
2182+
var sym_b = model.Symbol{
2183+
.language = try allocator.dupe(u8, "bash"),
2184+
.file_path = try allocator.dupe(u8, "bin/b.sh"),
2185+
.name = try allocator.dupe(u8, "do_thing"),
2186+
.signature = try allocator.dupe(u8, "do_thing()"),
2187+
.doc_comment = null,
2188+
.body = try allocator.dupe(u8, "do_thing() { echo 'running bash'; }"),
2189+
.start_line = 1,
2190+
.end_line = 1,
2191+
};
2192+
defer sym_b.deinit(allocator);
2193+
2194+
const id_a = try storage.insertSymbol(db, sym_a);
2195+
const id_b = try storage.insertSymbol(db, sym_b);
2196+
try storage.insertEmbedding(db, allocator, id_a, &[_]f32{ 0.0, 0.0 });
2197+
try storage.insertEmbedding(db, allocator, id_b, &[_]f32{ 0.0, 0.0 });
2198+
2199+
const has_fts = try ftsAvailable(db);
2200+
if (!has_fts) return error.SkipZigTest;
2201+
2202+
var fake = FakeEmbedder{ .vector = &[_]f32{ 0.0, 0.0 } };
2203+
const results = (try search(allocator, db, fake.embedder(), "bash version", .{
2204+
.top_n = 10,
2205+
.mode = .lexical,
2206+
})).results;
2207+
defer freeResults(allocator, results);
2208+
2209+
try std.testing.expect(results.len >= 1);
2210+
2211+
var setup_env_lex: ?f32 = null;
2212+
var do_thing_lex: ?f32 = null;
2213+
for (results) |res| {
2214+
if (std.mem.eql(u8, res.symbol.name, "setup_env")) setup_env_lex = res.lexical;
2215+
if (std.mem.eql(u8, res.symbol.name, "do_thing")) do_thing_lex = res.lexical;
2216+
}
2217+
2218+
// Full-coverage body match must be present with a non-trivial score
2219+
// (regression: previously stuck at 0.1 coverage floor because body wasn't
2220+
// scored).
2221+
try std.testing.expect(setup_env_lex != null);
2222+
try std.testing.expect(setup_env_lex.? > 0.2);
2223+
2224+
// Full coverage (both tokens via body) must outrank partial coverage
2225+
// (one token via body). Without name-match noise this is the cleanest
2226+
// signal that body scoring works.
2227+
if (do_thing_lex) |dt| {
2228+
try std.testing.expect(setup_env_lex.? > dt);
2229+
}
2230+
}
2231+
20152232
test "search hybrid returns no duplicate symbol IDs" {
20162233
const allocator = std.testing.allocator;
20172234
const db = try storage.openMemoryWithVec(allocator);

0 commit comments

Comments
 (0)