From 58425dd36cc80c239d37af728959daa2cab4085d Mon Sep 17 00:00:00 2001 From: Sujal Maheshwari Date: Mon, 24 Aug 2026 01:46:34 +0530 Subject: [PATCH] Never send empty text to the embedder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pure insert has no old text and a pure delete has no new text, and deltaText was handing that empty string to the embedder. Real embedding endpoints reject it — TEI answers "413 `inputs` cannot be empty" — so every insert- or delete-shaped hunk failed the moment a real model was configured. Adding or removing a line is the common edit, so in practice the semantic diff was broken for most changes against a real endpoint. It went unnoticed because the offline LexicalEmbedder happily embeds "" and returns a zero vector. The fallback masked it completely, and the fallback is what every test until now had run under. An empty side is a total shift at that point by definition — content appeared or vanished — so it returns 1 directly and makes no embed call at all. Tests use a stub embedder that refuses empty input the way TEI does, covering insert, delete, append, delete-at-end and delete-everything. The lexical embedder cannot catch this class of bug, so the stub is the point. Measured against BAAI/bge-small-en-v1.5 via TEI, before and after: inserting a line mid-prompt and deleting a line both went from "Internal: diff failed" to a verdict. --- semdiff/semdiff.go | 14 ++++++++ semdiff/semdiff_test.go | 73 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/semdiff/semdiff.go b/semdiff/semdiff.go index bf6079a..23f18d4 100644 --- a/semdiff/semdiff.go +++ b/semdiff/semdiff.go @@ -249,6 +249,20 @@ func deltaText(emb Embedder, a, b string, cache map[string][]float32) (float64, if a == b { // identical context contributes no shift, and no embed call return 0, nil } + // One side empty means content appeared or vanished outright — a pure insert + // or a pure delete hunk. That is a total shift at this point by definition, + // and there is nothing to embed. + // + // This has to be handled here rather than passed to the embedder, because + // real embedding endpoints reject an empty input: TEI answers + // "413 `inputs` cannot be empty". Sending it made every insert- or + // delete-shaped hunk fail as soon as a real model was configured — which is + // most edits, since adding or removing a line is the common case. The + // offline lexical embedder happily embeds "" and returns a zero vector, so + // the fallback masked this completely. + if strings.TrimSpace(a) == "" || strings.TrimSpace(b) == "" { + return 1, nil + } va, err := embedOne(emb, a, cache) if err != nil { return 0, err diff --git a/semdiff/semdiff_test.go b/semdiff/semdiff_test.go index 71d09a9..00207e3 100644 --- a/semdiff/semdiff_test.go +++ b/semdiff/semdiff_test.go @@ -1,6 +1,7 @@ package semdiff import ( + "errors" "strings" "testing" ) @@ -130,3 +131,75 @@ func TestIsolatedEditStaysLocal(t *testing.T) { t.Errorf("a one-word synonym swap classified %q, want a non-structural verdict", got) } } + +// stubEmbedder mimics a real embedding endpoint: it refuses empty input, the +// way TEI does ("413 `inputs` cannot be empty"). The offline LexicalEmbedder +// accepts "" and returns a zero vector, which is why sending empty text went +// unnoticed until a real model was configured. +type stubEmbedder struct{ calls int } + +func (s *stubEmbedder) Embed(texts []string) ([][]float32, error) { + for _, t := range texts { + if strings.TrimSpace(t) == "" { + return nil, errors.New("embeddings endpoint: 413 `inputs` cannot be empty") + } + } + s.calls++ + out := make([][]float32, len(texts)) + for i, t := range texts { + // crude but deterministic: length + first byte + var b float32 + if len(t) > 0 { + b = float32(t[0]) + } + out[i] = []float32{float32(len(t)), b, 1} + } + return out, nil +} + +// A pure insert has no old text, and a pure delete has no new text. Neither may +// reach the embedder. +func TestInsertAndDeleteNeverEmbedEmptyText(t *testing.T) { + cases := map[string]struct{ old, new []string }{ + "pure insert": {lines("keep one\nkeep two"), lines("keep one\nadded line\nkeep two")}, + "pure delete": {lines("keep one\ndrop me\nkeep two"), lines("keep one\nkeep two")}, + "append at the end": {lines("only line"), lines("only line\nappended")}, + "delete at the end": {lines("only line\ntrailing"), lines("only line")}, + "delete everything": {lines("a\nb\nc"), lines("")}, + } + for name, c := range cases { + t.Run(name, func(t *testing.T) { + emb := &stubEmbedder{} + res, err := Analyze(emb, c.old, c.new) + if err != nil { + t.Fatalf("Analyze sent empty text to the embedder: %v", err) + } + if len(res) == 0 { + t.Fatal("expected at least one hunk") + } + for _, r := range res { + if r.Class == "" { + t.Error("hunk produced no classification") + } + } + }) + } +} + +// An insert is a total shift at the point of change: there was nothing there. +func TestEmptySideIsMaximumDelta(t *testing.T) { + d, err := deltaText(&stubEmbedder{}, "", "some new instruction", map[string][]float32{}) + if err != nil { + t.Fatal(err) + } + if d != 1 { + t.Errorf("insert point delta = %v, want 1", d) + } + if d, err = deltaText(&stubEmbedder{}, "gone", "", map[string][]float32{}); err != nil || d != 1 { + t.Errorf("delete point delta = %v (err %v), want 1", d, err) + } + // Whitespace-only counts as empty — it carries no meaning to embed. + if d, err = deltaText(&stubEmbedder{}, " \n ", "text", map[string][]float32{}); err != nil || d != 1 { + t.Errorf("whitespace-only side = %v (err %v), want 1", d, err) + } +}