Skip to content

Commit 6a0836a

Browse files
committed
fix: enforce read-only vector opens, batch hybrid FTS leg, MCP scope param
1 parent ed9994a commit 6a0836a

8 files changed

Lines changed: 254 additions & 31 deletions

File tree

docs/semantic-search-internals.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,12 @@ Generation activation always happens under the single writer. Search opens
289289
are unchanged on every backend.
290290
- **`scope` governs unit visibility and supersedes `include_children`.**
291291
`scope=top|all|subordinate` (default `all`) filters each leg's hits before
292-
the RRF merge and before the limit, so a scoped search still fills up to the
293-
limit from the over-fetched candidates. In semantic/hybrid modes the
292+
the RRF merge and before the limit. The hybrid FTS leg fetches additional
293+
rank-ordered batches until it holds the fusion depth `k` of surviving
294+
entries (capped at `maxHybridFTSBatches`), so scope discards and same-unit
295+
collapse do not starve it; the semantic (KNN) leg cannot page, so scoped or
296+
collapse-heavy searches can still under-fill past those caps even when more
297+
matches exist deeper in the ranking. In semantic/hybrid modes the
294298
sidebar-child session exclusion is lifted (`semanticSessionScopeSubquery`) —
295299
both hybrid legs must see the same universe for fusion to be sound — and an
296300
explicit `include_children` is accepted but superseded. Subagent/fork-typed

docs/semantic-search.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,8 +342,10 @@ MCP tool error, carrying the same remediation text.
342342
fixed minimum if that's larger), then drop hits whose session fails
343343
`--project`/`--agent`/`--date*`/etc., then truncate to the requested limit.
344344
At small corpus sizes or with a narrow filter, this can return fewer than
345-
`--limit` results even though more exist. This is a known v1 tradeoff, not a
346-
bug.
345+
`--limit` results even though more exist. A narrow `--scope` (and, in
346+
hybrid, matches concentrated in one long run) can likewise return fewer than
347+
`--limit` even when more matches exist deeper in the ranking. This is a
348+
known v1 tradeoff, not a bug.
347349
- **Legacy no-`source_uuid` rows re-embed on ordinal shifts.** Each embedded
348350
document is keyed by its first message's stable per-message UUID when the
349351
parser recorded one, or by `(session_id, ordinal)` when it didn't.

internal/db/search_content.go

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,6 +1033,14 @@ func (db *DB) hybridVectorLeg(
10331033
return leg, nil
10341034
}
10351035

1036+
// maxHybridFTSBatches caps how many k-row FTS batches hybridFTSLeg fetches.
1037+
// It bounds the worst-case work when discard dominates — many rows collapsing
1038+
// into one unit, or a narrow f.Scope dropping most rows — while letting the
1039+
// leg keep paging past discarded rows instead of under-filling after the
1040+
// first batch. The residual is documented: a leg needing survivors deeper
1041+
// than maxHybridFTSBatches x k rows can still under-fill.
1042+
const maxHybridFTSBatches = 4
1043+
10361044
// hybridFTSLeg runs a rank-ordered FTS query over the embedded universe
10371045
// (role user/assistant, is_system = 0, system-prefix excluded -- the same
10381046
// predicate ScanEmbeddableUnits uses), scoped in SQL to sessions passing
@@ -1045,9 +1053,38 @@ func (db *DB) hybridVectorLeg(
10451053
// hits in one unit collapse to the best-ranked one. A hit with no containing
10461054
// unit keeps a message-granularity key, is never subordinate-penalized, and
10471055
// so survives fusion on its own.
1056+
//
1057+
// Rows are fetched in rank-ordered batches of k with OFFSET continuation:
1058+
// collapse and scope filtering can discard most of a batch, so the leg keeps
1059+
// fetching until it holds k entries, the stream is exhausted, or
1060+
// maxHybridFTSBatches is hit. The display seen-check dedups across batches;
1061+
// earlier batches rank better, so the best-ranked hit per unit always wins.
10481062
func (db *DB) hybridFTSLeg(
10491063
ctx context.Context, f ContentSearchFilter, searcher VectorSearcher, k int,
10501064
) (hybridLeg, error) {
1065+
leg := hybridLeg{display: make(map[string]hybridDisplay, k)}
1066+
for batch := range maxHybridFTSBatches {
1067+
hits, err := db.fetchHybridFTSBatch(ctx, f, k, batch*k)
1068+
if err != nil {
1069+
return hybridLeg{}, err
1070+
}
1071+
if err := appendHybridFTSHits(ctx, searcher, f.Scope, hits, &leg); err != nil {
1072+
return hybridLeg{}, err
1073+
}
1074+
if len(hits) < k || len(leg.ranked) >= k {
1075+
break
1076+
}
1077+
}
1078+
return leg, nil
1079+
}
1080+
1081+
// fetchHybridFTSBatch fetches one rank-ordered batch of at most k FTS message
1082+
// rows for hybridFTSLeg, starting at offset. The ORDER BY carries m.id as a
1083+
// deterministic tiebreak so OFFSET continuation is stable across batches when
1084+
// ranks tie.
1085+
func (db *DB) fetchHybridFTSBatch(
1086+
ctx context.Context, f ContentSearchFilter, k, offset int,
1087+
) ([]hybridDisplay, error) {
10511088
scope, scopeArgs := semanticSessionScopeSubquery(f)
10521089
query := fmt.Sprintf(`
10531090
SELECT m.session_id, m.ordinal,
@@ -1056,45 +1093,57 @@ func (db *DB) hybridFTSLeg(
10561093
WHERE messages_fts MATCH ? AND m.role IN ('user','assistant')
10571094
AND m.is_system = 0 AND %s
10581095
AND m.%s
1059-
ORDER BY f.rank LIMIT ?`,
1096+
ORDER BY f.rank, m.id LIMIT ? OFFSET ?`,
10601097
SystemPrefixSQL("m.content", "m.role"), scope)
10611098

10621099
args := []any{PrepareFTSQuery(f.Pattern)}
10631100
args = append(args, scopeArgs...)
1064-
args = append(args, k)
1101+
args = append(args, k, offset)
10651102

10661103
rows, err := db.getReader().QueryContext(ctx, query, args...)
10671104
if err != nil {
1068-
return hybridLeg{}, classifyFTSError(fmt.Errorf("hybrid search fts leg: %w", err))
1105+
return nil, classifyFTSError(fmt.Errorf("hybrid search fts leg: %w", err))
10691106
}
10701107
defer rows.Close()
10711108

10721109
var hits []hybridDisplay
10731110
for rows.Next() {
10741111
var hit hybridDisplay
10751112
if err := rows.Scan(&hit.sessionID, &hit.ordinal, &hit.snippet); err != nil {
1076-
return hybridLeg{}, fmt.Errorf("scan hybrid fts hit: %w", err)
1113+
return nil, fmt.Errorf("scan hybrid fts hit: %w", err)
10771114
}
10781115
hits = append(hits, hit)
10791116
}
10801117
if err := rows.Err(); err != nil {
1081-
return hybridLeg{}, err
1118+
return nil, err
10821119
}
1120+
return hits, nil
1121+
}
10831122

1123+
// appendHybridFTSHits resolves one batch of FTS message hits to their
1124+
// containing units and accumulates the survivors into leg: hits outside
1125+
// scope are dropped, and a unit already seen (within or across batches)
1126+
// keeps its earlier, better-ranked entry.
1127+
func appendHybridFTSHits(
1128+
ctx context.Context, searcher VectorSearcher, scope string,
1129+
hits []hybridDisplay, leg *hybridLeg,
1130+
) error {
1131+
if len(hits) == 0 {
1132+
return nil
1133+
}
10841134
refs := make([]MessageRef, len(hits))
10851135
for i, hit := range hits {
10861136
refs[i] = MessageRef{SessionID: hit.sessionID, Ordinal: hit.ordinal}
10871137
}
10881138
units, err := searcher.ResolveMessageUnits(ctx, refs)
10891139
if err != nil {
1090-
return hybridLeg{}, fmt.Errorf("resolving fts hits to units: %w", err)
1140+
return fmt.Errorf("resolving fts hits to units: %w", err)
10911141
}
10921142
if len(units) != len(refs) {
1093-
return hybridLeg{}, fmt.Errorf(
1143+
return fmt.Errorf(
10941144
"resolving fts hits to units: got %d units for %d refs", len(units), len(refs))
10951145
}
10961146

1097-
leg := hybridLeg{display: make(map[string]hybridDisplay, len(hits))}
10981147
for i, hit := range hits {
10991148
key := messageFusionKey(hit.sessionID, hit.ordinal)
11001149
hit.ordinalStart, hit.ordinalEnd = hit.ordinal, hit.ordinal
@@ -1104,7 +1153,7 @@ func (db *DB) hybridFTSLeg(
11041153
hit.ordinalEnd = units[i].OrdinalEnd
11051154
hit.subordinate = units[i].Subordinate
11061155
}
1107-
if scopeExcludes(f.Scope, hit.subordinate) {
1156+
if scopeExcludes(scope, hit.subordinate) {
11081157
continue
11091158
}
11101159
if _, seen := leg.display[key]; seen {
@@ -1113,7 +1162,7 @@ func (db *DB) hybridFTSLeg(
11131162
leg.ranked = append(leg.ranked, unitRanked{Key: key, Subordinate: hit.subordinate})
11141163
leg.display[key] = hit
11151164
}
1116-
return leg, nil
1165+
return nil
11171166
}
11181167

11191168
// enrichHybridMatches looks up session/message metadata for the fused units
@@ -1325,7 +1374,8 @@ func (db *DB) enrichSemanticHits(
13251374
// approximate snippet text semantic/hybrid modes locate within a message's
13261375
// full content: the vector index's trailing unicode ellipsis
13271376
// (internal/vector's truncateRunes) and FTS5 snippet()'s literal "..." marker
1328-
// (used at both ends), configured as hybridFTSLeg's 5th snippet() argument.
1377+
// (used at both ends), configured as fetchHybridFTSBatch's 5th snippet()
1378+
// argument.
13291379
var snippetTruncationMarkers = []string{"...", "…"}
13301380

13311381
// approxSnippetSpan locates approx (a searcher-provided chunk/snippet or

internal/db/search_content_hybrid_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,79 @@ func TestSearchContentHybridMatchCarriesUnitRangeAndLineage(t *testing.T) {
458458
assert.True(t, m.Sidechain, "anchor message is_sidechain")
459459
}
460460

461+
// TestSearchContentHybridFTSLegCollapseRefillsFromDeeperRanks pins the
462+
// batched FTS leg against unit collapse: when more than k (the
463+
// semanticOverfetchMin=200 fusion depth) rank-ordered FTS rows all fall
464+
// inside ONE run-unit, they collapse to a single leg entry, and a match in a
465+
// different unit ranked below all of them must still be fetched and returned
466+
// rather than being cut off by the first batch's window.
467+
func TestSearchContentHybridFTSLegCollapseRefillsFromDeeperRanks(t *testing.T) {
468+
d := testDB(t)
469+
if !d.HasFTS() {
470+
t.Skip("fts5 not available")
471+
}
472+
// One run-unit spanning 205 assistant messages, each repeating the term
473+
// so bm25 ranks every one of them above the single-occurrence session.
474+
const runLen = 205
475+
runMsgs := make([][2]string, runLen)
476+
for i := range runMsgs {
477+
runMsgs[i] = [2]string{"assistant", "zebra zebra zebra zebra"}
478+
}
479+
seedSearchSession(t, d, "bigrun", "proj", runMsgs)
480+
seedSearchSession(t, d, "other", "proj", [][2]string{
481+
{"user", "zebra appears once in a much longer unrelated sentence"},
482+
})
483+
// The vector leg is empty; the resolver knows the whole run as one unit.
484+
d.SetVectorSearcher(&fakeVectorSearcher{units: []UnitRef{
485+
{DocKey: "r:bigrun:0", SessionID: "bigrun",
486+
OrdinalStart: 0, OrdinalEnd: runLen - 1},
487+
}})
488+
489+
page, err := d.SearchContent(context.Background(), ContentSearchFilter{
490+
Pattern: "zebra", Mode: "hybrid", Limit: 10,
491+
})
492+
require.NoError(t, err, "SearchContent hybrid")
493+
require.Len(t, page.Matches, 2,
494+
"the lower-ranked unit past the collapsed run must be fetched")
495+
ids := []string{page.Matches[0].SessionID, page.Matches[1].SessionID}
496+
assert.ElementsMatch(t, []string{"bigrun", "other"}, ids)
497+
}
498+
499+
// TestSearchContentHybridFTSLegScopeExcludedRowsRefill pins the batched FTS
500+
// leg against scope discard: with scope=subordinate, when the first k FTS
501+
// rows are all top-level (and so all dropped), a subordinate match ranked
502+
// below them must still be fetched and returned.
503+
func TestSearchContentHybridFTSLegScopeExcludedRowsRefill(t *testing.T) {
504+
d := testDB(t)
505+
if !d.HasFTS() {
506+
t.Skip("fts5 not available")
507+
}
508+
// 205 top-level sessions would be slow; one top-level session with 205
509+
// matching messages fills the first batch the same way, since each
510+
// message is its own unit-less (never-subordinate) row.
511+
const topLen = 205
512+
topMsgs := make([][2]string, topLen)
513+
for i := range topMsgs {
514+
topMsgs[i] = [2]string{"user", "zebra zebra zebra zebra"}
515+
}
516+
seedSearchSession(t, d, "toplots", "proj", topMsgs)
517+
seedSubagentSession(t, d, "sub", "toplots", "proj", [][2]string{
518+
{"user", "zebra appears once in a much longer subagent sentence"},
519+
})
520+
d.SetVectorSearcher(&fakeVectorSearcher{units: []UnitRef{
521+
{DocKey: "u:sub:0", SessionID: "sub",
522+
OrdinalStart: 0, OrdinalEnd: 0, Subordinate: true},
523+
}})
524+
525+
page, err := d.SearchContent(context.Background(), ContentSearchFilter{
526+
Pattern: "zebra", Mode: "hybrid", Scope: "subordinate", Limit: 10,
527+
})
528+
require.NoError(t, err, "SearchContent hybrid")
529+
require.Len(t, page.Matches, 1,
530+
"the subordinate match past the excluded top-level rows must be fetched")
531+
assert.Equal(t, "sub", page.Matches[0].SessionID)
532+
}
533+
461534
// TestSearchContentHybridVectorOnlyMatchCarriesUnitRange pins the
462535
// vector-leg display path: a unit only the semantic leg found keeps its
463536
// chunk anchor and still exposes the unit range and subordinate flag.

internal/mcp/tools.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,7 @@ func (t *toolset) getMessagesAround(
475475
type searchContentIn struct {
476476
Pattern string `json:"pattern" jsonschema:"Exact substring or regex to find across message text and tool inputs/results."`
477477
Mode string `json:"mode,omitempty" jsonschema:"substring (default), regex, semantic, or hybrid."`
478+
Scope string `json:"scope,omitempty" jsonschema:"Semantic/hybrid result scope: top, all, or subordinate (default all). Only valid with mode semantic or hybrid."`
478479
Project string `json:"project,omitempty" jsonschema:"Restrict to one project."`
479480
Agent string `json:"agent,omitempty" jsonschema:"Restrict to one agent."`
480481
DateFrom string `json:"date_from,omitempty" jsonschema:"Only sessions on or after this date (YYYY-MM-DD)."`
@@ -533,9 +534,17 @@ type searchContentOut struct {
533534
func (t *toolset) searchContent(
534535
ctx context.Context, _ *mcp.CallToolRequest, in searchContentIn,
535536
) (*mcp.CallToolResult, searchContentOut, error) {
537+
// The db layer silently ignores Scope outside semantic/hybrid, so reject
538+
// it here with the same message the HTTP transport uses
539+
// (internal/server/huma_routes_search.go).
540+
if in.Scope != "" && in.Mode != "semantic" && in.Mode != "hybrid" {
541+
return nil, searchContentOut{}, fmt.Errorf(
542+
"scope is only supported for semantic and hybrid search modes")
543+
}
536544
res, err := t.svc.SearchContent(ctx, service.ContentSearchRequest{
537545
Pattern: in.Pattern,
538546
Mode: in.Mode,
547+
Scope: in.Scope,
539548
Project: in.Project,
540549
Agent: in.Agent,
541550
DateFrom: in.DateFrom,

internal/mcp/tools_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -848,6 +848,48 @@ func TestSearchContent_SemanticUnavailableMapsToRemediationError(t *testing.T) {
848848
assert.Equal(t, "semantic", fake.lastReq.Mode)
849849
}
850850

851+
// search_content must reject scope outside semantic/hybrid with the same
852+
// message the HTTP transport uses (the db layer silently ignores Scope for
853+
// lexical modes, so the guard lives in the transport), and must not reach
854+
// the service at all on rejection.
855+
func TestSearchContent_ScopeRejectedOnLexicalModes(t *testing.T) {
856+
fake := &fakeContentSearchService{result: &service.ContentSearchResult{}}
857+
ts := &toolset{svc: fake, now: func() time.Time { return fixedNow }}
858+
859+
for _, mode := range []string{"", "substring", "regex", "fts"} {
860+
t.Run("mode="+mode, func(t *testing.T) {
861+
_, _, err := ts.searchContent(context.Background(), nil, searchContentIn{
862+
Pattern: "needle", Mode: mode, Scope: "top", IncludeActive: true,
863+
})
864+
require.Error(t, err)
865+
assert.EqualError(t, err,
866+
"scope is only supported for semantic and hybrid search modes")
867+
})
868+
}
869+
assert.Empty(t, fake.lastReq.Pattern,
870+
"a rejected request must not reach the service")
871+
}
872+
873+
// search_content must pass Scope through to the service untouched for
874+
// semantic and hybrid modes; the db layer owns scope-value validation from
875+
// there.
876+
func TestSearchContent_ScopeForwardedForSemanticModes(t *testing.T) {
877+
for _, mode := range []string{"semantic", "hybrid"} {
878+
t.Run(mode, func(t *testing.T) {
879+
fake := &fakeContentSearchService{result: &service.ContentSearchResult{}}
880+
ts := &toolset{svc: fake, now: func() time.Time { return fixedNow }}
881+
882+
_, _, err := ts.searchContent(context.Background(), nil, searchContentIn{
883+
Pattern: "retries", Mode: mode, Scope: "subordinate", IncludeActive: true,
884+
})
885+
require.NoError(t, err)
886+
assert.Equal(t, mode, fake.lastReq.Mode)
887+
assert.Equal(t, "subordinate", fake.lastReq.Scope,
888+
"scope must reach the service untouched")
889+
})
890+
}
891+
}
892+
851893
// search_content's Context parameter must reach the service, and each
852894
// match's ContextBefore/ContextAfter (full service-level db.Message) must
853895
// map to the MCP layer's truncated contextMessage shape, along with Score.

internal/vector/index.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,11 @@ type GenerationInfo struct {
134134
// concurrently rewritten by another agentsview process, and carries its own
135135
// busy timeout so a reader waits out a concurrent writer's lock instead of
136136
// failing immediately with SQLITE_BUSY.
137+
//
138+
// Both branches emit a file: URI. mattn/go-sqlite3 forwards the `_`-prefixed
139+
// pragma params either way, but it only honors mode=ro when the DSN carries
140+
// the file: scheme — a bare path silently opens read-write, so the ro
141+
// contract depends on the prefix.
137142
func vectorDSN(path string, readOnly bool) string {
138143
params := url.Values{}
139144
if readOnly {
@@ -144,7 +149,7 @@ func vectorDSN(path string, readOnly bool) string {
144149
params.Set("_busy_timeout", "5000")
145150
params.Set("_synchronous", "NORMAL")
146151
}
147-
return path + "?" + params.Encode()
152+
return "file:" + path + "?" + params.Encode()
148153
}
149154

150155
// ChunkOverlap derives the SplitOptions.Overlap rune count from

0 commit comments

Comments
 (0)