From 6f800d3eb5021da87cc81e83ec9d8065a4c7cdc9 Mon Sep 17 00:00:00 2001 From: Pablo Ontiveros Date: Sat, 12 Sep 2026 07:06:19 -0600 Subject: [PATCH 1/3] fix(storage): exclude zero-valued Codex shell echoes from unfiltered search and relax IDF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The requeue path learned the Codex shell wrapper form in PR #87, but the query-time exclusion paths never did: isDirectBackscrollSearchEcho (Go) and directBackscrollSearchEchoSQL (SQL, used by recallFrequency) recognized only the bash and exec_command three-token prefixes. A pre-#80 tool row stored as search_echo=0 with a serialized 'shell command=["","-c"|"-lc", "backscroll search ..."]' call therefore leaked into unfiltered result pages and inflated unfiltered --relax document-frequency counting until an owner sync replayed the source — the same gap class fixed for exec_command in PR #86 (bs-echo-exec-cmd-gap-r1). Following PR #87's split, the Go predicate now decodes the JSON argv via the shared pendingSearchEchoShellMatches / directsearch.IsCodexDirectSearchCall instead of token-shape matching, and recallFrequency subtracts shell echo rows with the same broad-SQL-prefilter plus strict-Go-predicate split rather than attempting an unbounded SQL GLOB enumeration. Regression coverage: E2E with a real CodexReader round-trip and forced search_echo=0 (cmd/backscroll/echo_shell_zero_query_gap_test.go), per-row Go/SQL/IDF boundary cases including wrong-command, wrong-flag, extra-argv and non-search guards, a dedicated zero-valued shell IDF test, and shell rows in the at-scale SQL-vs-Go-scan cross-check. --- .../echo_shell_zero_query_gap_test.go | 101 +++++++++++++++ docs/search.md | 10 +- internal/storage/queries.go | 5 +- internal/storage/relaxation.go | 26 ++++ internal/storage/relaxation_echo_idf_test.go | 122 +++++++++++++++++- internal/storage/search.go | 17 ++- 6 files changed, 274 insertions(+), 7 deletions(-) create mode 100644 cmd/backscroll/echo_shell_zero_query_gap_test.go diff --git a/cmd/backscroll/echo_shell_zero_query_gap_test.go b/cmd/backscroll/echo_shell_zero_query_gap_test.go new file mode 100644 index 0000000..49340d3 --- /dev/null +++ b/cmd/backscroll/echo_shell_zero_query_gap_test.go @@ -0,0 +1,101 @@ +package main + +import ( + "database/sql" + "encoding/json" + "fmt" + "path/filepath" + "strings" + "testing" + + "github.com/pablontiv/backscroll/internal/startuplock" +) + +// Spike/regression for the Codex `shell` wrapper form of the zero-valued +// search_echo query-time gap. The exec_command half of this gap was fixed in +// PR #86; the requeue side learned the shell form in PR #87, but +// isDirectBackscrollSearchEcho / directBackscrollSearchEchoSQL (the functions +// that exclude a row from unfiltered result pages and --relax IDF counting +// RIGHT NOW, before reparse converges it) never gained shell handling. +// +// The fixture is a real CodexReader.Parse round-trip: the rollout below is +// ingested by the actual Codex reader, its serialized shape is asserted, and +// only then is search_echo forced back to 0 to reproduce the pre-#80 state. +func TestZeroValuedCodexShellEchoExcludedBeforeReplay(t *testing.T) { + e := newQueryEchoE2E(t) + e.writeRecord("target.jsonl", "target", "target", 0, "violet handshake quartz marker", false) + for i := 0; i < 4; i++ { + e.writeRecord(fmt.Sprintf("noise-%d.jsonl", i), fmt.Sprintf("noise-%d", i), "noise", 1+i, "adaptation rollout distractor", false) + } + + codexRoot := e.addReaderManifest("codex", "codex") + // Real Codex shell calls carry extra keys (workdir, timeout_ms), so the + // serialized `command=` token is not the only key=value and the strict + // predicate must locate it inside the sorted token list. + for i := 0; i < 8; i++ { + id := fmt.Sprintf("codex-shell-%d", i) + args, _ := json.Marshal(map[string]any{ + "command": []string{"bash", "-lc", "backscroll search --text 'violet handshake'"}, + "workdir": "/synthetic/query-echo-e2e", + "timeout_ms": 10000, + }) + writeCodexRollout(t, filepath.Join(codexRoot, id+".jsonl"), id, 10+i, + map[string]any{"type": "function_call", "name": "shell", "call_id": id, "arguments": string(args)}) + } + // Already-fixed exec_command control in the same forced-zero state: it + // must stay excluded by both the Go and the SQL/IDF paths. + for i := 0; i < 4; i++ { + id := fmt.Sprintf("codex-exec-%d", i) + args, _ := json.Marshal(map[string]any{"cmd": "backscroll search --text 'violet handshake'"}) + writeCodexRollout(t, filepath.Join(codexRoot, id+".jsonl"), id, 20+i, + map[string]any{"type": "function_call", "name": "exec_command", "call_id": id, "arguments": string(args)}) + } + e.run("status", "--json") + + db, err := sql.Open("sqlite", e.database) + if err != nil { + t.Fatal(err) + } + var serialized string + if err := db.QueryRow(`SELECT text FROM search_items WHERE source_path LIKE ? AND content_type='tool' LIMIT 1`, "%codex-shell-0.jsonl").Scan(&serialized); err != nil { + _ = db.Close() + t.Fatal(err) + } + const wantSerialized = `shell command=["bash","-lc","backscroll search --text 'violet handshake'"] timeout_ms=10000 workdir=/synthetic/query-echo-e2e` + if serialized != wantSerialized { + _ = db.Close() + t.Fatalf("Codex shell SerializeToolInput shape=%q want %q", serialized, wantSerialized) + } + if _, err := db.Exec(`UPDATE search_items SET search_echo=0 WHERE content_type='tool' AND source_path LIKE ?`, "%codex-%.jsonl"); err != nil { + _ = db.Close() + t.Fatal(err) + } + if err := db.Close(); err != nil { + t.Fatal(err) + } + + // Hold the startup lock so the following searches run as followers on the + // last committed snapshot: the query-time fallback is what must exclude the + // zeroed rows, with no owner replay to converge them first. + lease, acquired, err := startuplock.TryAcquire(e.database) + if err != nil || !acquired { + t.Fatalf("acquire parent startup lock: acquired=%v err=%v", acquired, err) + } + defer func() { + if err := lease.Release(); err != nil { + t.Errorf("release parent startup lock: %v", err) + } + }() + + got := e.searchJSON("violet handshake", "", 20) + for _, row := range got { + if row.ContentType == "tool" && strings.HasPrefix(filepath.Base(row.FilePath), "codex-") { + t.Errorf("zero-valued Codex echo leaked into unfiltered recall: %v", queryEchoShape(got)) + } + } + + out := e.run("search", "--text", "violet handshake adaptation", "--all-projects", "--lexical-only", "--relax", "--robot", "--fields", "minimal", "--max-tokens", "200") + if !strings.Contains(out, "result_0_filepath="+filepath.Join(e.fixtures, "target.jsonl")+"\n") || !strings.Contains(out, `result_0_dropped_terms=["adaptation"]`) { + t.Errorf("zero-valued shell query echoes inverted unfiltered --relax IDF\nstdout=%s", out) + } +} diff --git a/docs/search.md b/docs/search.md index 643744b..cc633de 100644 --- a/docs/search.md +++ b/docs/search.md @@ -175,6 +175,12 @@ rows use the UUID-less per-file reload path. An index that stored those calls as `search_echo=0` before their readers marked echoes re-enters the same bounded replay while the surviving source still has a serialized direct search call; paired outputs are marked by identity on that reparse, not by output shape. +While such a source awaits replay, the query-time exclusion already keeps its +zero-valued call rows out of unfiltered result pages and unfiltered `--relax` +IDF counting, recognizing the same serialized forms: the `bash`/`exec_command` +three-token prefixes (matched in SQL) and the Codex `shell` argv form (matched +by decoding the JSON-encoded argv, since its separator byte-sequences are +unbounded for SQL pattern matching). Subsequent source expiry, `rebuild`, and supported canonical recovery preserve proven pairing evidence. The general extraction epoch is unchanged. @@ -210,7 +216,7 @@ backscroll search --text '"violet handshake" quartz marker adaptation' --relax - The deterministic sequence is: 1. Run strict AND search. Unmarked queries keep the existing sanitizer, ranking and snippets. Leading `+term` marks a term that cannot be dropped; quoted spans are protected phrase units. For queries containing these protected units, strict matching keeps every unit without dynamic stopword removal. Quotes preserve FTS phrase order; ordinary unquoted terms retain Porter stemming/prefix matching (trigram matching for tools). -2. Only if that stage has zero eligible rows, drop one **unprotected** term at a time, lowest IDF first. For a fixed corpus, this is highest document frequency first. Frequencies are counted with the actual tokenizer's MATCH expression over the applicable index(es), globally rather than within the result scope (project, path, dates, tags). Unfiltered IDF uses the same echo eligibility as unfiltered result pages: direct Backscroll retrieval-call tool rows (`search_echo=1`, a serialized `bash command=backscroll search ...` invocation, or a serialized `exec_command cmd=backscroll search ...` invocation, each matched as that three-token prefix regardless of what follows) do not inflate document frequency. Explicit `--content-type tool` keeps those rows in both the page and the IDF count. Equal frequencies drop in original query order. Zero-frequency terms have highest IDF and are not specially discarded. A term that appears only in those excluded echo rows is absent from the unfiltered corpus, so its document frequency is 0 and `--relax` drops it last — the same as any other zero-frequency extra term that can prevent recovery at the two-term floor. Each retry still requires every retained unit, bypassing dynamic stopwords so the retained core cannot silently disappear. +2. Only if that stage has zero eligible rows, drop one **unprotected** term at a time, lowest IDF first. For a fixed corpus, this is highest document frequency first. Frequencies are counted with the actual tokenizer's MATCH expression over the applicable index(es), globally rather than within the result scope (project, path, dates, tags). Unfiltered IDF uses the same echo eligibility as unfiltered result pages: direct Backscroll retrieval-call tool rows (`search_echo=1`, a serialized `bash command=backscroll search ...` or `exec_command cmd=backscroll search ...` invocation, each matched as that three-token prefix regardless of what follows, or a serialized Codex `shell` call whose JSON-encoded argv is exactly `[, "-c" | "-lc", "backscroll search ..."]`, matched by decoding the argv rather than by text shape) do not inflate document frequency. Explicit `--content-type tool` keeps those rows in both the page and the IDF count. Equal frequencies drop in original query order. Zero-frequency terms have highest IDF and are not specially discarded. A term that appears only in those excluded echo rows is absent from the unfiltered corpus, so its document frequency is 0 and `--relax` drops it last — the same as any other zero-frequency extra term that can prevent recovery at the two-term floor. Each retry still requires every retained unit, bypassing dynamic stopwords so the retained core cannot silently disappear. 3. Stop at the first stage with results, or before fewer than **two distinct unprotected terms** remain. Protected terms are additional to that floor. Case-insensitive repeated spellings count once, and a keep marker on any occurrence protects that unit. Queries with at most two unprotected terms perform strict search only. There is no single-term/empty fallback and no global OR. Stemming/phrase-expansion stages are skipped: stemming is already available and protected phrases must not weaken. Scope widening is always skipped. Project (including cwd-inferred project), source, source-path, content-type, role, dates, and tags are retained at every stage. Tool searches remain on their trigram index even when relaxation is explicitly requested. Existing unfiltered echo exclusion and cross-index RRF still apply. @@ -232,7 +238,7 @@ result_0_dropped_terms=["adaptation"] This addresses **recoverable term overload**, not vocabulary invention. In the native regression, the target contains `violet handshake`, while unrelated records make `adaptation` a common term; strict search misses and dropping that term recovers the target. In contrast, the existing synthetic `s2_conversational_paraphrase` and `s5_progressive_refinement` targets share no surviving content terms with their original queries. The refinement `violet handshake` adds new vocabulary; this feature does not promise to recover those zero-overlap cases. An absent, high-IDF extra term can also prevent recovery at the two-term floor. No production p95 or broad semantic-recall gain is claimed from the small fixture corpus. -Regression owners: `cmd/backscroll/search_relaxation_test.go` (native input-to-output recovery and unchanged strict controls), `cmd/backscroll/search_relaxation_echo_idf_e2e_test.go` (unfiltered IDF ignores query-echo rows), `cmd/backscroll/search_relaxation_output_test.go` (budgeted provenance and early validation), `internal/storage/relaxation_test.go` (IDF order, protected core, scope and paging), and `internal/storage/relaxation_echo_idf_test.go` (echo-eligibility of unfiltered IDF, including echo-only DF=0). +Regression owners: `cmd/backscroll/search_relaxation_test.go` (native input-to-output recovery and unchanged strict controls), `cmd/backscroll/search_relaxation_echo_idf_e2e_test.go` (unfiltered IDF ignores query-echo rows), `cmd/backscroll/echo_shell_zero_query_gap_test.go` (zero-valued Codex `shell` echoes excluded from unfiltered pages and IDF before replay), `cmd/backscroll/search_relaxation_output_test.go` (budgeted provenance and early validation), `internal/storage/relaxation_test.go` (IDF order, protected core, scope and paging), and `internal/storage/relaxation_echo_idf_test.go` (echo-eligibility of unfiltered IDF, including echo-only DF=0 and the Codex `shell` argv boundary cases). ## Exit Codes diff --git a/internal/storage/queries.go b/internal/storage/queries.go index 5b23653..db19534 100644 --- a/internal/storage/queries.go +++ b/internal/storage/queries.go @@ -1145,7 +1145,10 @@ func (d *Database) filterShellEchoZeroPaths(paths []string) ([]string, error) { } // pendingSearchEchoShellMatches is the Go-side check for whether a stored -// Codex shell tool row is a direct `backscroll search` call. +// Codex shell tool row is a direct `backscroll search` call. It is shared by +// the requeue path (filterShellEchoZeroPaths) and by the query-time exclusion +// paths (isDirectBackscrollSearchEcho and recallFrequency's IDF counting), so +// all three accept exactly the same shell rows. // // SerializeToolInput emits the rollout's `arguments` as a space-joined // `key=value` token list with keys sorted alphabetically. Real Codex shell diff --git a/internal/storage/relaxation.go b/internal/storage/relaxation.go index 7b461a8..c6d0244 100644 --- a/internal/storage/relaxation.go +++ b/internal/storage/relaxation.go @@ -225,5 +225,31 @@ func (d *Database) recallFrequency(term recallTerm, contentType string) (int, er if err != nil { return 0, fmt.Errorf("measure relaxation term frequency: %w", err) } + // directBackscrollSearchEchoSQL cannot recognize the Codex shell wrapper + // form: its argv is JSON-encoded and the separator byte-sequences are + // unbounded for SQL GLOB. Subtract those rows with the same + // broad-SQL-prefilter plus strict-Go-predicate split the requeue path + // uses (see pendingSearchEchoShellMatches), so unfiltered IDF counts the + // exact row set isDirectBackscrollSearchEcho keeps. + shellRows, err := d.db.Query( + "SELECT si.text FROM ("+matched+") matched JOIN search_items si ON si.id = matched.rowid WHERE si.content_type = 'tool' AND COALESCE(si.search_echo, 0) = 0 AND si.text LIKE 'shell %'", + args..., + ) + if err != nil { + return 0, fmt.Errorf("measure relaxation term frequency: %w", err) + } + defer func() { _ = shellRows.Close() }() + for shellRows.Next() { + var text string + if err := shellRows.Scan(&text); err != nil { + return 0, fmt.Errorf("measure relaxation term frequency: %w", err) + } + if pendingSearchEchoShellMatches(text) { + count-- + } + } + if err := shellRows.Err(); err != nil { + return 0, fmt.Errorf("measure relaxation term frequency: %w", err) + } return count, nil } diff --git a/internal/storage/relaxation_echo_idf_test.go b/internal/storage/relaxation_echo_idf_test.go index c230204..1795f29 100644 --- a/internal/storage/relaxation_echo_idf_test.go +++ b/internal/storage/relaxation_echo_idf_test.go @@ -1,12 +1,14 @@ package storage import ( + "encoding/json" "fmt" "strings" "testing" "time" "github.com/pablontiv/backscroll/internal/models" + "github.com/pablontiv/backscroll/internal/readers" ) func TestRelaxationUnfilteredIDFExcludesQueryEchoes(t *testing.T) { @@ -120,6 +122,78 @@ func TestRelaxationUnfilteredIDFExcludesTextFallbackEchoes(t *testing.T) { } } +func TestRelaxationUnfilteredIDFExcludesZeroValuedCodexShellEchoes(t *testing.T) { + db, cleanup := newTestDB(t) + t.Cleanup(cleanup) + + files := []IndexedFile{ + { + SourcePath: "/target.jsonl", Source: "session", Project: "alpha", Hash: "target", + Messages: []IndexedMessage{{ + Ordinal: 0, UUID: "target", Role: "assistant", ContentType: "text", + Text: "violet handshake quartz marker", Timestamp: "2026-01-01T00:00:00Z", + }}, + }, + } + for i := 0; i < 4; i++ { + files = append(files, IndexedFile{ + SourcePath: fmt.Sprintf("/noise-%d.jsonl", i), Source: "session", Project: "alpha", Hash: "noise", + Messages: []IndexedMessage{{ + Ordinal: 0, UUID: fmt.Sprintf("noise-%d", i), Role: "assistant", ContentType: "text", + Text: "adaptation rollout distractor", Timestamp: "2026-01-01T00:00:00Z", + }}, + }) + } + // Pre-#80 Codex writers stored search_echo=0 with the JSON-encoded shell + // argv as text. The pure-SQL echo predicate cannot see this form; both the + // Go predicate and recallFrequency's strict-Go subtraction must exclude it. + for i := 0; i < 8; i++ { + files = append(files, IndexedFile{ + SourcePath: fmt.Sprintf("/shell-echo-%d.jsonl", i), Source: "session", Project: "alpha", Hash: fmt.Sprintf("shell-echo-%d", i), + Messages: []IndexedMessage{{ + Ordinal: 0, UUID: fmt.Sprintf("shell-echo-%d", i), Role: "assistant", ContentType: "tool", + Text: shellText(t, "bash", "-lc", "backscroll search --text 'violet handshake'"), + Timestamp: "2026-01-01T00:00:00Z", + }}, + }) + } + if err := db.SyncFiles(files); err != nil { + t.Fatal(err) + } + if _, err := db.db.Exec(`UPDATE search_items SET search_echo=0 WHERE content_type='tool'`); err != nil { + t.Fatal(err) + } + + violet, err := db.recallFrequency(recallTerm{text: "violet"}, "") + if err != nil { + t.Fatal(err) + } + handshake, err := db.recallFrequency(recallTerm{text: "handshake"}, "") + if err != nil { + t.Fatal(err) + } + adaptation, err := db.recallFrequency(recallTerm{text: "adaptation"}, "") + if err != nil { + t.Fatal(err) + } + if violet != 1 || handshake != 1 || adaptation != 4 { + t.Fatalf("unfiltered IDF counted zero-valued shell echoes: violet=%d handshake=%d adaptation=%d", violet, handshake, adaptation) + } + + got, stages, err := db.SearchRelaxed("violet handshake adaptation", models.SearchOptions{AllProjects: true, Limit: 20}) + if err != nil { + t.Fatal(err) + } + for _, row := range got { + if isDirectBackscrollSearchEcho(row) { + t.Fatalf("shell echo leaked into unfiltered relaxed results: stages=%v row=%+v", stages, row) + } + } + if len(got) != 1 || got[0].SourcePath != "/target.jsonl" || fmt.Sprint(got[0].DroppedTerms) != "[adaptation]" { + t.Fatalf("zero-valued shell echoes inverted --relax IDF: stages=%v results=%+v", stages, got) + } +} + func TestRelaxationUnfilteredIDFStillCountsLegitimateTools(t *testing.T) { db, cleanup := newTestDB(t) t.Cleanup(cleanup) @@ -224,7 +298,27 @@ func TestRecallFrequencySQLEchoPredicatePreservesBoundaries(t *testing.T) { nullEcho bool unique string wantEcho bool + // wantSQL overrides the directBackscrollSearchEchoSQL expectation when + // it legitimately differs from the Go predicate: the Codex shell + // wrapper form is JSON-encoded and unbounded for SQL GLOB, so the SQL + // predicate cannot see it and recallFrequency excludes those rows via + // the broad prefilter + strict Go predicate instead. + wantSQL *bool + } + // Shell fixtures go through a real json.Marshal + SerializeToolInput + // round-trip so the stored text carries the encoder's actual escaping. + shellFull := func(argv []string) string { + raw, err := json.Marshal(map[string]any{ + "additional_permissions": "read", + "command": argv, + "timeout_ms": 1000, + }) + if err != nil { + t.Fatal(err) + } + return readers.SerializeToolInput("shell", raw) } + sqlMiss := false cases := []row{ {name: "canonical Bash", contentType: "tool", text: "Bash command=backscroll search --text boundtok00", unique: "boundtok00", wantEcho: true}, {name: "lowercase bash", contentType: "tool", text: "bash command=backscroll search --text boundtok01", unique: "boundtok01", wantEcho: true}, @@ -244,6 +338,19 @@ func TestRecallFrequencySQLEchoPredicatePreservesBoundaries(t *testing.T) { {name: "folded command=", contentType: "tool", text: "Bash Command=backscroll search --text boundtok15", unique: "boundtok15"}, {name: "folded Search", contentType: "tool", text: "Bash command=backscroll Search --text boundtok16", unique: "boundtok16"}, {name: "canonical exec_command", contentType: "tool", text: `exec_command cmd=backscroll search --text boundtok17 command=[["unused"]]`, unique: "boundtok17", wantEcho: true}, + // Codex shell wrapper form: excluded by the Go predicate and by + // recallFrequency, but invisible to the pure-SQL predicate. + {name: "canonical shell", contentType: "tool", text: shellText(t, "bash", "-lc", "backscroll search --text boundtok20"), unique: "boundtok20", wantEcho: true, wantSQL: &sqlMiss}, + {name: "shell extra sorted keys", contentType: "tool", text: shellFull([]string{"sh", "-c", "backscroll search --text boundtok21"}), unique: "boundtok21", wantEcho: true, wantSQL: &sqlMiss}, + {name: "shell /bin/bash path", contentType: "tool", text: shellText(t, "/bin/bash", "-lc", "backscroll search --text boundtok22"), unique: "boundtok22", wantEcho: true, wantSQL: &sqlMiss}, + {name: "shell NBSP separator", contentType: "tool", text: shellText(t, "sh", "-c", "backscroll search --text boundtok23"), unique: "boundtok23", wantEcho: true, wantSQL: &sqlMiss}, + {name: "null echo shell fallback", contentType: "tool", text: shellText(t, "bash", "-lc", "backscroll search --text boundtok24"), nullEcho: true, unique: "boundtok24", wantEcho: true, wantSQL: &sqlMiss}, + {name: "shell wrong command", contentType: "tool", text: shellText(t, "sh", "-c", "rg boundtok25 /tmp"), unique: "boundtok25"}, + {name: "shell wrong flag", contentType: "tool", text: shellText(t, "sh", "-x", "backscroll search --text boundtok26"), unique: "boundtok26"}, + {name: "shell extra argv element", contentType: "tool", text: shellTextN(t, "sh", "-c", "backscroll search --text boundtok27", "bar"), unique: "boundtok27"}, + {name: "shell non-search call", contentType: "tool", text: shellText(t, "sh", "-c", "backscroll status boundtok28"), unique: "boundtok28"}, + {name: "shell env wrapper inside argv", contentType: "tool", text: shellText(t, "sh", "-c", "env backscroll search --text boundtok29"), unique: "boundtok29"}, + {name: "shell absolute path inside argv", contentType: "tool", text: shellText(t, "sh", "-c", "/usr/local/bin/backscroll search --text boundtok30"), unique: "boundtok30"}, } var files []IndexedFile @@ -286,8 +393,12 @@ func TestRecallFrequencySQLEchoPredicatePreservesBoundaries(t *testing.T) { if gotGo != tc.wantEcho { t.Fatalf("Go predicate = %v, want %v (echo=%d text=%q)", gotGo, tc.wantEcho, echo, result.Text) } - if (sqlEcho == 1) != tc.wantEcho { - t.Fatalf("SQL predicate = %d, want echo=%v (echo=%d text=%q)", sqlEcho, tc.wantEcho, echo, result.Text) + wantSQL := tc.wantEcho + if tc.wantSQL != nil { + wantSQL = *tc.wantSQL + } + if (sqlEcho == 1) != wantSQL { + t.Fatalf("SQL predicate = %d, want echo=%v (echo=%d text=%q)", sqlEcho, wantSQL, echo, result.Text) } got, err := db.recallFrequency(recallTerm{text: tc.unique}, "") @@ -315,7 +426,12 @@ func TestRecallFrequencyUnfilteredSQLMatchesGoScan(t *testing.T) { for i := 0; i < n; i++ { text := "Bash command=rg commonterm /tmp\n" + filler echo := false - if i%10 == 0 { + if i%20 == 0 { + // Zero-valued Codex shell echo (pre-#80 state): excluded by the Go + // predicate and by recallFrequency's strict-Go subtraction, but not + // by the pure-SQL predicate — proving both paths agree at scale. + text = shellText(t, "bash", "-lc", "backscroll search --text commonterm") + "\n" + filler + } else if i%10 == 0 { text = "Bash command=backscroll search --text commonterm\n" + filler echo = true } diff --git a/internal/storage/search.go b/internal/storage/search.go index f6a05e4..3de6425 100644 --- a/internal/storage/search.go +++ b/internal/storage/search.go @@ -408,7 +408,16 @@ func isDirectBackscrollSearchEcho(result SearchResult) bool { if strings.EqualFold(fields[0], "bash") { return fields[1] == "command=backscroll" && fields[2] == "search" } - return fields[0] == "exec_command" && fields[1] == "cmd=backscroll" && fields[2] == "search" + if fields[0] == "exec_command" { + return fields[1] == "cmd=backscroll" && fields[2] == "search" + } + if fields[0] == "shell" { + // Codex shell wrapper: the argv is JSON-encoded, so token-shape + // matching is unbounded. Reuse the exact strict predicate the + // requeue path (PR #87) already applies. + return pendingSearchEchoShellMatches(result.Text) + } + return false } // asciiWhitespaceSQL is the ASCII subset of unicode.IsSpace. SQL-side echo @@ -424,6 +433,12 @@ const asciiWhitespaceSQL = "char(9, 10, 11, 12, 13, 32)" // fold the exact tokens. The patterns are prefix-only: lookalikes that do not // start with either prefix, including path collisions and "searcher", must not // match. +// +// The Codex shell wrapper form is deliberately NOT matched here: its argv is +// JSON-encoded and the separator byte-sequences strings.Fields accepts are +// unbounded for SQL GLOB. recallFrequency excludes those rows with the same +// broad-SQL-prefilter plus strict-Go-predicate split the requeue path uses +// (see pendingSearchEchoShellMatches). func directBackscrollSearchEchoSQL(alias string) string { trimmed := "ltrim(" + alias + ".text, " + asciiWhitespaceSQL + ")" sep := "'[' || " + asciiWhitespaceSQL + " || ']'" From 10ec5595a118745c0657bd32991adc4f7e343449 Mon Sep 17 00:00:00 2001 From: Pablo Ontiveros Date: Sat, 12 Sep 2026 07:07:59 -0600 Subject: [PATCH 2/3] docs: record shell-form query-echo exclusion split in AGENTS.md --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 174a053..490a2ff 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -82,7 +82,7 @@ Ten v2 CLI commands: `list [--project] [--all-projects] [--recent N] [--order ti The `SearchEngine` interface is the port; `internal/storage` is the adapter. Database opened lazily. `OpenReadOnly()` provides read-only access for external consumers. -Opt-in lexical term dropping is owned by `internal/storage/relaxation.go`; `docs/search.md#opt-in-lexical-relaxation` defines protected units, the two-unprotected-term floor, fixed scope, provenance and zero-overlap limits. Unfiltered IDF uses the same query-echo eligibility as unfiltered result pages, counted in SQL via `directBackscrollSearchEchoSQL` (keep lockstep with `isDirectBackscrollSearchEcho`; do not reintroduce a Go-side row scan). Ordinary search behavior/output must remain unchanged. +Opt-in lexical term dropping is owned by `internal/storage/relaxation.go`; `docs/search.md#opt-in-lexical-relaxation` defines protected units, the two-unprotected-term floor, fixed scope, provenance and zero-overlap limits. Unfiltered IDF uses the same query-echo eligibility as unfiltered result pages, counted in SQL via `directBackscrollSearchEchoSQL` (keep lockstep with `isDirectBackscrollSearchEcho`) — except the Codex `shell` wrapper form, whose JSON-encoded argv is unbounded for SQL GLOB: `recallFrequency` subtracts those rows with the PR #87 broad-SQL-prefilter (`text LIKE 'shell %'`) plus strict-Go-predicate split (`pendingSearchEchoShellMatches`), and `isDirectBackscrollSearchEcho` uses the same helper. Never reintroduce a general Go-side row scan or a GLOB enumeration of JSON separator byte-sequences. Ordinary search behavior/output must remain unchanged. ### Core Pipeline From 6e5b04b8f2225d67c148def374837ceecd20805b Mon Sep 17 00:00:00 2001 From: Pablo Ontiveros Date: Sat, 12 Sep 2026 07:35:08 -0600 Subject: [PATCH 3/3] fix(storage): hoist shell echo check above the three-token guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 1 on PR #89 found that the pre-existing len(fields) < 3 guard ran before the new shell branch: a Codex shell call whose argv separators are ALL JSON control escapes (\t, \n, …) serializes to only two whitespace-separated tokens, never reached the shell check, and leaked into unfiltered result pages — while recallFrequency's IDF path, which has no such floor, excluded the identical row. The fix made the two paths disagree where at base they had both been wrong. The shell check now runs on the first whitespace-delimited token before any token-count floor, case-insensitively to match the SQL LIKE prefilters. Regression coverage: boundary-table row and E2E files built through a real CodexReader round-trip with a tab-separated argv (asserted to serialize to exactly two tokens), both mutation-checked RED without the hoist; docs and the pendingSearchEchoShellMatches comment corrected to describe the page-side Go matching and the two shape gates accurately. --- .../echo_shell_zero_query_gap_test.go | 30 ++++++++++++++++++- docs/search.md | 15 ++++++---- internal/storage/queries.go | 10 +++++-- internal/storage/relaxation_echo_idf_test.go | 8 ++++- internal/storage/search.go | 24 ++++++++------- 5 files changed, 68 insertions(+), 19 deletions(-) diff --git a/cmd/backscroll/echo_shell_zero_query_gap_test.go b/cmd/backscroll/echo_shell_zero_query_gap_test.go index 49340d3..c6517c2 100644 --- a/cmd/backscroll/echo_shell_zero_query_gap_test.go +++ b/cmd/backscroll/echo_shell_zero_query_gap_test.go @@ -52,11 +52,26 @@ func TestZeroValuedCodexShellEchoExcludedBeforeReplay(t *testing.T) { } e.run("status", "--json") + // Same gap with every separator JSON-escaped (control characters like + // U+0009 must escape as two-byte \t in JSON, so the serialized text has + // no argv whitespace at all and only two strings.Fields tokens). These + // rows exercised a page/IDF disagreement: recallFrequency's shell + // prefilter has no token-count floor, but the result-page predicate did. + for i := 0; i < 2; i++ { + id := fmt.Sprintf("codex-tab-%d", i) + args, _ := json.Marshal(map[string]any{ + "command": []string{"sh", "-c", "backscroll\tsearch\t--text\tviolet\thandshake"}, + }) + writeCodexRollout(t, filepath.Join(codexRoot, id+".jsonl"), id, 30+i, + map[string]any{"type": "function_call", "name": "shell", "call_id": id, "arguments": string(args)}) + } + e.run("status", "--json") + db, err := sql.Open("sqlite", e.database) if err != nil { t.Fatal(err) } - var serialized string + var serialized, tabSerialized string if err := db.QueryRow(`SELECT text FROM search_items WHERE source_path LIKE ? AND content_type='tool' LIMIT 1`, "%codex-shell-0.jsonl").Scan(&serialized); err != nil { _ = db.Close() t.Fatal(err) @@ -66,6 +81,19 @@ func TestZeroValuedCodexShellEchoExcludedBeforeReplay(t *testing.T) { _ = db.Close() t.Fatalf("Codex shell SerializeToolInput shape=%q want %q", serialized, wantSerialized) } + if err := db.QueryRow(`SELECT text FROM search_items WHERE source_path LIKE ? AND content_type='tool' LIMIT 1`, "%codex-tab-0.jsonl").Scan(&tabSerialized); err != nil { + _ = db.Close() + t.Fatal(err) + } + const wantTabSerialized = `shell command=["sh","-c","backscroll\tsearch\t--text\tviolet\thandshake"]` + if tabSerialized != wantTabSerialized { + _ = db.Close() + t.Fatalf("Codex tab shell SerializeToolInput shape=%q want %q", tabSerialized, wantTabSerialized) + } + if got := len(strings.Fields(tabSerialized)); got != 2 { + _ = db.Close() + t.Fatalf("tab-escaped shell row must have exactly two whitespace-separated tokens to exercise the guard, got %d", got) + } if _, err := db.Exec(`UPDATE search_items SET search_echo=0 WHERE content_type='tool' AND source_path LIKE ?`, "%codex-%.jsonl"); err != nil { _ = db.Close() t.Fatal(err) diff --git a/docs/search.md b/docs/search.md index cc633de..9996b3d 100644 --- a/docs/search.md +++ b/docs/search.md @@ -177,10 +177,15 @@ replay while the surviving source still has a serialized direct search call; paired outputs are marked by identity on that reparse, not by output shape. While such a source awaits replay, the query-time exclusion already keeps its zero-valued call rows out of unfiltered result pages and unfiltered `--relax` -IDF counting, recognizing the same serialized forms: the `bash`/`exec_command` -three-token prefixes (matched in SQL) and the Codex `shell` argv form (matched -by decoding the JSON-encoded argv, since its separator byte-sequences are -unbounded for SQL pattern matching). +IDF counting, recognizing the same serialized forms. Result pages are filtered +in Go: the `bash` and `exec_command` forms by their three-token prefix shape, +and the Codex `shell` form by a `shell` first-token gate followed by decoding +the JSON-encoded argv (its separator byte-sequences are unbounded for +text-shape matching, and no token-count floor may precede the gate — an +all-escaped argv serializes to just two whitespace-separated tokens). IDF +counting evaluates the `bash`/`exec_command` prefixes in SQL and applies the +same `shell` decode in Go over a broad `text LIKE 'shell %'` prefilter, so +both paths exclude the same rows. Subsequent source expiry, `rebuild`, and supported canonical recovery preserve proven pairing evidence. The general extraction epoch is unchanged. @@ -216,7 +221,7 @@ backscroll search --text '"violet handshake" quartz marker adaptation' --relax - The deterministic sequence is: 1. Run strict AND search. Unmarked queries keep the existing sanitizer, ranking and snippets. Leading `+term` marks a term that cannot be dropped; quoted spans are protected phrase units. For queries containing these protected units, strict matching keeps every unit without dynamic stopword removal. Quotes preserve FTS phrase order; ordinary unquoted terms retain Porter stemming/prefix matching (trigram matching for tools). -2. Only if that stage has zero eligible rows, drop one **unprotected** term at a time, lowest IDF first. For a fixed corpus, this is highest document frequency first. Frequencies are counted with the actual tokenizer's MATCH expression over the applicable index(es), globally rather than within the result scope (project, path, dates, tags). Unfiltered IDF uses the same echo eligibility as unfiltered result pages: direct Backscroll retrieval-call tool rows (`search_echo=1`, a serialized `bash command=backscroll search ...` or `exec_command cmd=backscroll search ...` invocation, each matched as that three-token prefix regardless of what follows, or a serialized Codex `shell` call whose JSON-encoded argv is exactly `[, "-c" | "-lc", "backscroll search ..."]`, matched by decoding the argv rather than by text shape) do not inflate document frequency. Explicit `--content-type tool` keeps those rows in both the page and the IDF count. Equal frequencies drop in original query order. Zero-frequency terms have highest IDF and are not specially discarded. A term that appears only in those excluded echo rows is absent from the unfiltered corpus, so its document frequency is 0 and `--relax` drops it last — the same as any other zero-frequency extra term that can prevent recovery at the two-term floor. Each retry still requires every retained unit, bypassing dynamic stopwords so the retained core cannot silently disappear. +2. Only if that stage has zero eligible rows, drop one **unprotected** term at a time, lowest IDF first. For a fixed corpus, this is highest document frequency first. Frequencies are counted with the actual tokenizer's MATCH expression over the applicable index(es), globally rather than within the result scope (project, path, dates, tags). Unfiltered IDF uses the same echo eligibility as unfiltered result pages: direct Backscroll retrieval-call tool rows (`search_echo=1`, a serialized `bash command=backscroll search ...` or `exec_command cmd=backscroll search ...` invocation, each matched as that three-token prefix regardless of what follows, or a serialized Codex `shell` call whose JSON-encoded argv is exactly `[, "-c" | "-lc", "backscroll search ..."]`, selected by a `shell` text-prefix gate and then matched by decoding the argv, since the JSON separator byte-sequences are unbounded for SQL pattern matching) do not inflate document frequency. Explicit `--content-type tool` keeps those rows in both the page and the IDF count. Equal frequencies drop in original query order. Zero-frequency terms have highest IDF and are not specially discarded. A term that appears only in those excluded echo rows is absent from the unfiltered corpus, so its document frequency is 0 and `--relax` drops it last — the same as any other zero-frequency extra term that can prevent recovery at the two-term floor. Each retry still requires every retained unit, bypassing dynamic stopwords so the retained core cannot silently disappear. 3. Stop at the first stage with results, or before fewer than **two distinct unprotected terms** remain. Protected terms are additional to that floor. Case-insensitive repeated spellings count once, and a keep marker on any occurrence protects that unit. Queries with at most two unprotected terms perform strict search only. There is no single-term/empty fallback and no global OR. Stemming/phrase-expansion stages are skipped: stemming is already available and protected phrases must not weaken. Scope widening is always skipped. Project (including cwd-inferred project), source, source-path, content-type, role, dates, and tags are retained at every stage. Tool searches remain on their trigram index even when relaxation is explicitly requested. Existing unfiltered echo exclusion and cross-index RRF still apply. diff --git a/internal/storage/queries.go b/internal/storage/queries.go index db19534..db143ed 100644 --- a/internal/storage/queries.go +++ b/internal/storage/queries.go @@ -1147,8 +1147,14 @@ func (d *Database) filterShellEchoZeroPaths(paths []string) ([]string, error) { // pendingSearchEchoShellMatches is the Go-side check for whether a stored // Codex shell tool row is a direct `backscroll search` call. It is shared by // the requeue path (filterShellEchoZeroPaths) and by the query-time exclusion -// paths (isDirectBackscrollSearchEcho and recallFrequency's IDF counting), so -// all three accept exactly the same shell rows. +// paths (isDirectBackscrollSearchEcho and recallFrequency's IDF counting). +// Each call site first gates on the serialized text starting with the `shell` +// tool-name token — whitespace-delimited and case-insensitive in Go, `text +// LIKE 'shell %'` in SQL — and then applies this decode, with no token-count +// floor anywhere: an argv whose separators are all JSON control escapes +// serializes to just two whitespace-separated tokens, and a floor on only one +// path made pages and IDF disagree. Serializer-produced rows are therefore +// accepted identically by all three. // // SerializeToolInput emits the rollout's `arguments` as a space-joined // `key=value` token list with keys sorted alphabetically. Real Codex shell diff --git a/internal/storage/relaxation_echo_idf_test.go b/internal/storage/relaxation_echo_idf_test.go index 1795f29..f7b6bb3 100644 --- a/internal/storage/relaxation_echo_idf_test.go +++ b/internal/storage/relaxation_echo_idf_test.go @@ -343,7 +343,13 @@ func TestRecallFrequencySQLEchoPredicatePreservesBoundaries(t *testing.T) { {name: "canonical shell", contentType: "tool", text: shellText(t, "bash", "-lc", "backscroll search --text boundtok20"), unique: "boundtok20", wantEcho: true, wantSQL: &sqlMiss}, {name: "shell extra sorted keys", contentType: "tool", text: shellFull([]string{"sh", "-c", "backscroll search --text boundtok21"}), unique: "boundtok21", wantEcho: true, wantSQL: &sqlMiss}, {name: "shell /bin/bash path", contentType: "tool", text: shellText(t, "/bin/bash", "-lc", "backscroll search --text boundtok22"), unique: "boundtok22", wantEcho: true, wantSQL: &sqlMiss}, - {name: "shell NBSP separator", contentType: "tool", text: shellText(t, "sh", "-c", "backscroll search --text boundtok23"), unique: "boundtok23", wantEcho: true, wantSQL: &sqlMiss}, + {name: "shell NBSP separator", contentType: "tool", text: shellText(t, "sh", "-c", "backscroll search --text boundtok23"), unique: "boundtok23", wantEcho: true, wantSQL: &sqlMiss}, + // JSON control escapes (\t here) stay two-byte sequences in the + // serialized text, so with no other argv whitespace the whole row has + // only two strings.Fields tokens — it must not fall below a token-count + // floor before the shell check, or pages would keep a row the IDF path + // (which has no such floor) excludes. + {name: "shell JSON-escaped tab separators", contentType: "tool", text: shellText(t, "sh", "-c", "backscroll\tsearch\t--text\tboundtok31"), unique: "boundtok31", wantEcho: true, wantSQL: &sqlMiss}, {name: "null echo shell fallback", contentType: "tool", text: shellText(t, "bash", "-lc", "backscroll search --text boundtok24"), nullEcho: true, unique: "boundtok24", wantEcho: true, wantSQL: &sqlMiss}, {name: "shell wrong command", contentType: "tool", text: shellText(t, "sh", "-c", "rg boundtok25 /tmp"), unique: "boundtok25"}, {name: "shell wrong flag", contentType: "tool", text: shellText(t, "sh", "-x", "backscroll search --text boundtok26"), unique: "boundtok26"}, diff --git a/internal/storage/search.go b/internal/storage/search.go index 3de6425..948bb29 100644 --- a/internal/storage/search.go +++ b/internal/storage/search.go @@ -402,22 +402,26 @@ func isDirectBackscrollSearchEcho(result SearchResult) bool { return true } fields := strings.Fields(result.Text) + if len(fields) == 0 { + return false + } + // Codex shell wrapper: checked before the three-token guard because the + // argv is JSON-encoded — when every separator inside the command string + // is a JSON control escape (\t, \n, …) the serialized text has only two + // whitespace-separated fields. The SQL prefilter in recallFrequency has + // no such floor, so a guard here would make pages and IDF disagree. + // Token-shape matching on the argv itself is unbounded; reuse the exact + // strict predicate the requeue path (PR #87) already applies. + if strings.EqualFold(fields[0], "shell") { + return pendingSearchEchoShellMatches(result.Text) + } if len(fields) < 3 { return false } if strings.EqualFold(fields[0], "bash") { return fields[1] == "command=backscroll" && fields[2] == "search" } - if fields[0] == "exec_command" { - return fields[1] == "cmd=backscroll" && fields[2] == "search" - } - if fields[0] == "shell" { - // Codex shell wrapper: the argv is JSON-encoded, so token-shape - // matching is unbounded. Reuse the exact strict predicate the - // requeue path (PR #87) already applies. - return pendingSearchEchoShellMatches(result.Text) - } - return false + return fields[0] == "exec_command" && fields[1] == "cmd=backscroll" && fields[2] == "search" } // asciiWhitespaceSQL is the ASCII subset of unicode.IsSpace. SQL-side echo