fix: final review minors — DSN escaping, parked-row reads, doc gaps

Author: wesm

docs/semantic-search-internals.md

@@ -139,7 +139,12 @@ exists:
   `message_vectors*` table, including vec0 tables left behind by retired or
   abandoned generations — recreates the current schema, and restamps the
   version, so the next build takes the existing first-ever full-build path.
-  `vectors.db` is disposable by design; `sessions.db` is never reset this way.
+  `embeddings activate` and `retire` also open read-write on their direct
+  (no-daemon) path (`directGenerationAction` in
+  `cmd/agentsview/embeddings.go`), so against a mismatched `vectors.db` they
+  trigger the same reset and then fail with "generation not found", the reset
+  having removed every generation. `vectors.db` is disposable by design;
+  `sessions.db` is never reset this way.
 - **Read path** (read-only `Open`: CLI reads, direct-install search): `Open`
   succeeds without touching any table, but every subsequent `Search`,
   `StaleActive`, `Generations`, or `ResolveMessageUnits` call fails closed
@@ -310,10 +315,13 @@ Generation activation always happens under the single writer. Search opens
   `internal/db` fuses rank-ordered legs with reciprocal rank fusion (rank
   constant 60) and shifts subordinate units' effective rank by +5 — a
   rank-based adjustment, not a hard tier or score multiplier, since RRF ranks
-  are the only scale comparable across legs. Semantic-only search routes its
-  single ranked list through the same merge as a one-leg fusion, so
-  `--semantic` downranks subordinate hits identically to `--hybrid` (matches
-  still carry the searcher's own cosine scores; only the order changes).
+  are the only scale comparable across legs. The merge is a local
+  implementation rather than kit's `Merge` because kit has no per-hit
+  rank-offset hook for the subordinate penalty (upstreamable later).
+  Semantic-only search routes its single ranked list through the same merge as
+  a one-leg fusion, so `--semantic` downranks subordinate hits identically to
+  `--hybrid` (matches still carry the searcher's own cosine scores; only the
+  order changes).
 - **Hybrid fuses at unit granularity, with an FTS anchor override.** The FTS leg
   stays message-granularity (exact strings, commands, filenames) over the same
   embeddable-universe predicate `ScanEmbeddableUnits` uses. Each FTS message

docs/semantic-search.md

@@ -57,6 +57,10 @@ starts out empty and every document counts as pending. For a hosted embeddings
 API that is a real cost event, so run `agentsview embeddings build` directly at
 a time of your choosing if you want to control when that initial cost lands,
 rather than letting the debounced after-sync scheduler trigger it on its own.
+The same cost event can recur on upgrade: when a new agentsview version changes
+the index's internal mirror schema or document-identity scheme, the next
+writable open resets the mirror, and with `run_after_sync = true` the next sync
+automatically re-embeds the entire archive against the configured endpoint.
 
 By default, `include_automated = false` keeps automated sessions (e.g. roborev)
 out of the embedding index entirely, mirroring session search's default

internal/db/search_content.go

@@ -872,7 +872,12 @@ type mergedUnit struct {
 // (rank+5 against a rank constant of 60). Semantic-only search routes its
 // single ranked list through this same merge as a one-leg fusion, so the
 // penalty applies identically in both modes. Ties break deterministically by
-// ascending key; limit > 0 truncates the fused list.
+// ascending key; limit > 0 truncates the fused list. Each leg's entries must
+// already be deduplicated by Key — both callers dedup via their display-map
+// seen-checks — since a repeated key within one leg would accumulate score
+// twice. This is a local merge rather than kitvec.Merge because kit's Merge
+// has no per-hit rank-offset hook for the subordinate penalty; upstreaming
+// such a hook would let this collapse onto kit's implementation later.
 func rrfMerge(legs [][]unitRanked, limit int) []mergedUnit {
 	const rankConstant = 60
 	const subordinatePenalty = 5

internal/server/search_scope_test.go

@@ -116,9 +116,10 @@ func TestSearchContentScopeFiltersSemanticResults(t *testing.T) {
 
 // TestSearchContentSemanticResponseCarriesUnitRangeAndLineage pins the HTTP
 // wire shape for run-grouped semantic hits: ordinal stays the anchor while
-// ordinal_start/ordinal_end, subordinate, and the lineage keys ride along;
-// a top-level single-message hit omits them all (omitempty), keeping its
-// JSON identical to before.
+// ordinal_start/ordinal_end, subordinate, and the lineage keys ride along.
+// The fixture's top-level single-message hit sits at ordinal 0, so all of
+// its zero-valued unit/lineage fields are omitted via omitempty; a nonzero
+// single-message hit would still emit ordinal_start/ordinal_end.
 func TestSearchContentSemanticResponseCarriesUnitRangeAndLineage(t *testing.T) {
 	te := setup(t)
 	te.seedSession(t, "top-sess", "proj", 2)

internal/vector/index.go

@@ -139,6 +139,11 @@ type GenerationInfo struct {
 // pragma params either way, but it only honors mode=ro when the DSN carries
 // the file: scheme — a bare path silently opens read-write, so the ro
 // contract depends on the prefix.
+//
+// The path component is percent-encoded (slashes kept intact): SQLite
+// percent-decodes URI paths and splits params at `?`, so a raw path
+// containing `%`, `?`, or `#` would be misparsed — e.g. a literal "%41" in a
+// directory name would silently open a different file.
 func vectorDSN(path string, readOnly bool) string {
 	params := url.Values{}
 	if readOnly {
@@ -149,7 +154,8 @@ func vectorDSN(path string, readOnly bool) string {
 		params.Set("_busy_timeout", "5000")
 		params.Set("_synchronous", "NORMAL")
 	}
-	return "file:" + path + "?" + params.Encode()
+	escaped := (&url.URL{Path: path}).EscapedPath()
+	return "file:" + escaped + "?" + params.Encode()
 }
 
 // ChunkOverlap derives the SplitOptions.Overlap rune count from

internal/vector/index_test.go

@@ -3,6 +3,7 @@ package vector
 import (
 	"context"
 	"database/sql"
+	"os"
 	"path/filepath"
 	"testing"
 
@@ -61,6 +62,44 @@ func TestOpenReadOnlyRefusesWrites(t *testing.T) {
 		"the refusal must be SQLite's readonly-database error, got: %v", err)
 }
 
+// TestOpenPathWithSpecialCharacters pins vectorDSN's path escaping: SQLite
+// percent-decodes file: URI paths and splits params at `?`, so a directory
+// name containing a space and a literal %-hex sequence ("%41") would, raw,
+// be decoded to a different path ("weArd dir") and fail to open. Both the
+// writable and read-only branches must escape the path, and read-only must
+// still refuse writes.
+func TestOpenPathWithSpecialCharacters(t *testing.T) {
+	ctx := context.Background()
+	dir := filepath.Join(t.TempDir(), "we%41rd dir")
+	require.NoError(t, os.MkdirAll(dir, 0o755))
+	path := filepath.Join(dir, "vectors.db")
+
+	rw, err := Open(ctx, path, false, 4000)
+	require.NoError(t, err, "writable Open must succeed on a path with %% and space")
+	_, err = rw.db.ExecContext(ctx,
+		`INSERT INTO vector_meta (key, value) VALUES ('probe', 'x')`)
+	require.NoError(t, err)
+	require.NoError(t, rw.Close())
+
+	_, err = os.Stat(path)
+	require.NoError(t, err, "the database file must exist at the literal path, not a decoded one")
+
+	ro, err := Open(ctx, path, true, 4000)
+	require.NoError(t, err, "read-only Open must succeed on a path with %% and space")
+	defer ro.Close()
+
+	var value string
+	require.NoError(t, ro.db.QueryRowContext(ctx,
+		`SELECT value FROM vector_meta WHERE key = 'probe'`).Scan(&value))
+	assert.Equal(t, "x", value)
+
+	_, err = ro.db.ExecContext(ctx,
+		`INSERT INTO vector_meta (key, value) VALUES ('probe2', 'y')`)
+	require.Error(t, err, "a read-only vectors.db handle must refuse writes")
+	assert.Contains(t, err.Error(), "readonly",
+		"the refusal must be SQLite's readonly-database error, got: %v", err)
+}
+
 func TestOpenSplitOptionsUse15PercentOverlap(t *testing.T) {
 	ctx := context.Background()
 	path := filepath.Join(t.TempDir(), "vectors.db")

internal/vector/search.go

@@ -18,7 +18,7 @@ const snippetMaxRunes = 200
 // Hit is one unit-level semantic search result, anchored to a specific
 // message. For a run document Ordinal is the anchor: the member message
 // whose rune span contains the matched chunk's center rune (see
-// anchorOrdinal), while OrdinalStart/OrdinalEnd span the whole run. For a
+// anchorMemberIndex), while OrdinalStart/OrdinalEnd span the whole run. For a
 // user document all three ordinals are the message's own ordinal.
 type Hit struct {
 	SessionID    string
@@ -231,7 +231,7 @@ const runMemberSeparatorRunes = 2
 
 // resolveRunHit computes a run hit's anchor ordinal and anchor-local
 // snippet: the anchor is the member whose rune span contains the matched
-// chunk's center rune (see anchorOrdinal), and the snippet is the
+// chunk's center rune (see anchorMemberIndex), and the snippet is the
 // intersection of the chunk's rune window with that member's span — always
 // a substring of the anchor message's own text, so the db layer's snippet
 // centering (semanticSnippet) can locate it inside the anchor message's
@@ -277,17 +277,6 @@ func chunkWindow(contentRunes, chunkIndex int, o kitvec.SplitOptions) (start, en
 	return start, end
 }
 
-// anchorOrdinal maps a matched chunk back to the run member whose rune span
-// contains the chunk's center rune (chunk_start + actual_chunk_runes/2).
-// Separator runes between members belong to no member's span, so a center
-// falling there resolves to the preceding member: the earlier member wins a
-// boundary tie. offsets must be non-empty (run documents only); user
-// documents pass their ordinal through without calling this.
-func anchorOrdinal(offsets []db.UnitOffset, contentRunes, chunkIndex int, o kitvec.SplitOptions) int {
-	start, end := chunkWindow(contentRunes, chunkIndex, o)
-	return offsets[anchorMemberIndex(offsets, start, end)].Ordinal
-}
-
 // anchorMemberIndex returns the offsets index of the run member whose rune
 // span contains the [start, end) chunk window's center rune, with the
 // earlier member winning when the center falls in the separator between two
@@ -310,15 +299,17 @@ func anchorMemberIndex(offsets []db.UnitOffset, start, end int) int {
 // keyed by doc_key, in maxSQLVars-sized chunks: a deep semantic overfetch
 // (large limit * over-fetch factor) can carry thousands of doc keys, well
 // past what a single IN (...) clause can bind. A key with no matching row is
-// simply absent from the result.
+// simply absent from the result. Rows parked at a negative sentinel ordinal
+// by a concurrent Refresh (see evictSlotOccupant) are excluded the same way:
+// mid-refresh state must never hydrate into a hit with a negative ordinal.
 func (ix *Index) lookupMirrorDocs(ctx context.Context, docKeys []string) (map[string]mirrorDoc, error) {
 	docs := make(map[string]mirrorDoc, len(docKeys))
 	err := chunkKeys(docKeys, func(chunk []string) error {
 		placeholders, args := inPlaceholders(chunk)
 		rows, err := ix.db.QueryContext(ctx, `
 SELECT doc_key, session_id, ordinal, ordinal_end, subordinate, offsets, content
   FROM vector_messages
- WHERE doc_key IN `+placeholders, args...)
+ WHERE ordinal >= 0 AND doc_key IN `+placeholders, args...)
 		if err != nil {
 			return fmt.Errorf("look up search hit documents: %w", err)
 		}
@@ -378,7 +369,10 @@ func truncateRunes(s string, maxRunes int) string {
 // yields a zero UnitRef. Each ref is a point lookup on the retained unique
 // (session_id, ordinal) index — greatest unit ordinal <= ref ordinal, then a
 // containment check against ordinal_end — via one prepared statement, so a
-// batch of any size never approaches SQLite's bind-variable limit.
+// batch of any size never approaches SQLite's bind-variable limit. Rows
+// parked at a negative sentinel ordinal by a concurrent Refresh (see
+// evictSlotOccupant) are skipped so a ref can never resolve into
+// mid-refresh state and surface a negative ordinal range.
 //
 // Like Search and StaleActive, it fails closed with ErrMirrorVersionMismatch
 // — before touching any table — when ix was opened read-only against a
@@ -397,7 +391,7 @@ func (ix *Index) ResolveMessageUnits(
 	stmt, err := ix.db.PrepareContext(ctx, `
 SELECT doc_key, ordinal, ordinal_end, subordinate
   FROM vector_messages
- WHERE session_id = ? AND ordinal <= ?
+ WHERE session_id = ? AND ordinal >= 0 AND ordinal <= ?
  ORDER BY ordinal DESC LIMIT 1`)
 	if err != nil {
 		return nil, fmt.Errorf("resolve message units: %w", err)

internal/vector/search_test.go

@@ -171,6 +171,16 @@ func TestStaleActiveFalseWhenNoActiveGeneration(t *testing.T) {
 	assert.False(t, stale, "no active generation means nothing to compare")
 }
 
+// anchorOrdinal is a test-only convenience over the production anchor
+// pipeline: it maps a matched chunk back to the run member ordinal whose
+// rune span contains the chunk's center rune, composing chunkWindow and
+// anchorMemberIndex exactly the way resolveRunHit does. offsets must be
+// non-empty (run documents only).
+func anchorOrdinal(offsets []db.UnitOffset, contentRunes, chunkIndex int, o kitvec.SplitOptions) int {
+	start, end := chunkWindow(contentRunes, chunkIndex, o)
+	return offsets[anchorMemberIndex(offsets, start, end)].Ordinal
+}
+
 // TestAnchorOrdinal pins the anchor policy: the anchor is the member message
 // whose rune span contains the matched chunk's center rune, computed from
 // the chunk's ACTUAL rune length (the final chunk is capped at the content's
@@ -579,6 +589,58 @@ func TestResolveMessageUnitsPointLookup(t *testing.T) {
 	}
 }
 
+// TestResolveMessageUnitsIgnoresParkedRows pins the mid-refresh read
+// contract: Refresh parks a displaced row at a negative sentinel ordinal
+// non-transactionally (evictSlotOccupant), so a concurrent resolver call can
+// see it. The point lookup's ordinal-DESC seek would otherwise land on the
+// parked row (its old ordinal_end still covers the ref) and emit a negative
+// OrdinalStart; parked rows must be invisible to readers.
+func TestResolveMessageUnitsIgnoresParkedRows(t *testing.T) {
+	ix := openTestIndex(t)
+	// Parked mid-refresh: ordinal moved to the sentinel, ordinal_end still
+	// holds its old value, so containment (2 <= 3) would pass.
+	seedUnitRow(t, ix, "r:s:parked", "s", -2, 3, false)
+	seedUnitRow(t, ix, "r:s:valid", "s", 5, 6, false)
+
+	got, err := ix.ResolveMessageUnits(context.Background(), []db.MessageRef{
+		{SessionID: "s", Ordinal: 2},
+		{SessionID: "s", Ordinal: 5},
+	})
+	require.NoError(t, err)
+	require.Len(t, got, 2)
+	assert.Equal(t, db.UnitRef{}, got[0],
+		"a ref covered only by a parked row must stay unresolved, not surface a negative ordinal")
+	assert.Equal(t, db.UnitRef{
+		DocKey: "r:s:valid", SessionID: "s", OrdinalStart: 5, OrdinalEnd: 6,
+	}, got[1], "valid rows must keep resolving alongside a parked one")
+}
+
+// TestHydrateHitsIgnoresParkedRows pins the same mid-refresh contract on the
+// hit-hydration path: a KNN hit whose doc_key points at a sentinel-parked
+// mirror row must be dropped (like a vanished doc), never hydrated into a
+// hit with a negative ordinal.
+func TestHydrateHitsIgnoresParkedRows(t *testing.T) {
+	ix := openTestIndex(t)
+	ctx := context.Background()
+	seedMirrorRow(t, ix, "u-parked", db.EmbeddableUnit{
+		SessionID: "s1", Kind: "user", Ordinal: -1, OrdinalEnd: 4,
+		Content: "parked mid-refresh",
+	})
+	seedMirrorRow(t, ix, "u-valid", db.EmbeddableUnit{
+		SessionID: "s1", Kind: "user", Ordinal: 6, OrdinalEnd: 6,
+		Content: "still visible",
+	})
+
+	hits, err := ix.hydrateHits(ctx, []kitvec.Hit[string]{
+		{Doc: "u-parked", ChunkIndex: 0, Score: 0.9},
+		{Doc: "u-valid", ChunkIndex: 0, Score: 0.8},
+	})
+	require.NoError(t, err)
+	require.Len(t, hits, 1, "the parked row's hit must be dropped")
+	assert.Equal(t, 6, hits[0].Ordinal)
+	assert.Equal(t, "still visible", hits[0].Snippet)
+}
+
 // TestResolveMessageUnitsResultParallelToRefs pins that one call over a
 // mixed batch keeps the result slice parallel to refs, with zero UnitRefs
 // holding the positions of unresolvable refs.