diff --git a/internal/router/embedderid_test.go b/internal/router/embedderid_test.go index 03f1e16..6b2ecfd 100644 --- a/internal/router/embedderid_test.go +++ b/internal/router/embedderid_test.go @@ -1,10 +1,13 @@ package router -import "testing" +import ( + "strings" + "testing" +) func TestEmbedderIDFromModelPath(t *testing.T) { got := modelIDFromPath("/opt/maven/models/embedder/multilingual-e5-small.onnx") - if got != "multilingual-e5-small@384" { + if got != "multilingual-e5-small@384/tok2" { t.Fatalf("modelIDFromPath = %q", got) } // A different model file must produce a different id, even at 384 dim. @@ -12,6 +15,13 @@ func TestEmbedderIDFromModelPath(t *testing.T) { if old == got { t.Fatal("two different models share one id") } + // The tokenizer is half of what makes a vector, and it changes under a + // model file whose name never moves (V-664). An id that ignored it would + // leave stored passages in one space and every new query in another, with + // nothing to trigger the re-embed. + if !strings.Contains(got, "/tok") { + t.Fatalf("id %q does not name the tokenizer revision", got) + } } func TestEmbedderIDIncludesDim(t *testing.T) { diff --git a/internal/router/onnxembedder.go b/internal/router/onnxembedder.go index cbbf63c..005255f 100644 --- a/internal/router/onnxembedder.go +++ b/internal/router/onnxembedder.go @@ -66,12 +66,19 @@ func NewONNXEmbedder(modelPath, tokenizerPath, libPath string) (*onnxEmbedder, e func (e *onnxEmbedder) Dim() int { return embedDim } // ID names the loaded model for the DB marker (Vikunja #378): the model file's -// own name plus the dimension, so pointing the config at another model changes -// the string on its own. +// own name, the dimension, and the tokenizer revision, so pointing the config +// at another model changes the string on its own. func (e *onnxEmbedder) ID() string { return e.id } +// tokenizerRev — bumped whenever the tokenizer changes what it emits for the +// same text, because that changes every vector while the model file's name +// stays put. Rev 2 is the fix for the reversed word pieces (V-664): stored +// passages embedded under rev 1 no longer sit in the same space as a query +// embedded now, and ReembedAll rewrites them because this string moved. +const tokenizerRev = 2 + // modelIDFromPath turns /opt/.../multilingual-e5-small.onnx into -// "multilingual-e5-small@384". +// "multilingual-e5-small@384/tok2". func modelIDFromPath(modelPath string) string { name := modelPath if i := strings.LastIndexAny(name, "/\\"); i >= 0 { @@ -81,7 +88,7 @@ func modelIDFromPath(modelPath string) string { if name == "" { name = "onnx" } - return fmt.Sprintf("%s@%d", name, embedDim) + return fmt.Sprintf("%s@%d/tok%d", name, embedDim, tokenizerRev) } // Embed treats the text as a query. The classifier compares one short @@ -339,14 +346,17 @@ func (t *unigramTokenizer) encodeWord(word string) []int64 { } } + // Backtracking walks the word from its end, and prepending each piece puts + // it back in reading order. There used to be a second reverse after this + // loop, which undid it: every multi-piece word came out backwards, and + // "query: вода" tokenized to [0 12 1294 41 12489 2] where the reference + // tokenizer gives [0 41 1294 12 12489 2] (V-664). A transformer reads + // position, so the pieces of a long Russian word were being read in the + // wrong order on every turn. var result []int64 for i := n; i > 0; i = prev[i] { result = append([]int64{bestID[i]}, result...) } - // Reverse - for l, r := 0, len(result)-1; l < r; l, r = l+1, r-1 { - result[l], result[r] = result[r], result[l] - } return result }