diff --git a/AGENTS.md b/AGENTS.md index 490a2ff..9fa3769 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,7 +55,7 @@ cmd/backscroll/ internal/ ├── config/ — config resolution: backscroll.toml → ~/.config → env → defaults ├── compat/ — stateless schema-shape inspection, release lineage catalog, migration plans, and canonical recovery planning -├── directsearch/ — shared direct-search predicates (IsDirectSearchCommand, IsCodexDirectSearchCall) so readers ingest path and storage replay path use the exact same strings.Fields / argv-shape acceptance +├── directsearch/ — shared direct-search predicates (IsDirectSearchCommand, IsCodexDirectSearchCall, IsSerializedDirectSearchCall): the single strict boundary for "is this a direct search call", at ingest (readers, raw input) and at query/replay (storage, serialized text) ├── input_config/ — input manifest loading, discovery, and legacy session-dirs compatibility ├── models/ — domain types: SessionRecord, MessageContent, ParsedFile, SearchResult, Stats ├── sync/ — WalkDir, SHA-256 dedup, JSONL parsing, noise filtering, content-type classification @@ -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`) — 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. +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 by construction: the serialized-text boundary is owned by exactly one strict Go predicate, `directsearch.IsSerializedDirectSearchCall` (bash, exec_command, and the Codex `shell` wrapper with JSON-decoded argv), and both IDF counting (`recallFrequency`) and requeue detection (`PendingSearchEchoPaths`) apply it in Go over a broad, provable-superset SQL prefilter (`content_type='tool' AND COALESCE(search_echo,0)=0 AND text LIKE '%backscroll%'`). Never reintroduce a SQL-side shape recognizer (GLOB/LIKE token patterns) for these rows — the accepted separator byte-sequences are unbounded for SQL, which is what caused the #86/#87/#89 divergence chain; `internal/storage/echo_parity_test.go` enforces three-way page/IDF/requeue agreement across shapes × separator alphabets. Ordinary search behavior/output must remain unchanged. ### Core Pipeline @@ -128,7 +128,7 @@ External knowledge sources are configured with active `*.inputs.toml` manifests - **Legacy `[sources]` migration boundary**: Any `[sources]` table in the global `config.toml` or local `./backscroll.toml` is a hard preflight error before executable commands perform database or file side effects. Help and version remain available. Migrate each key to an active `*.inputs.toml` manifest under `/backscroll/inputs/`: `ke` → `source = "ke"`, `decode.format = "markdown_document"`; `memories` → `source = "memory"`, `markdown_document`; `decisions` → `source = "decision"`, `markdown_sections`; `rules` → `source = "rule"`, `markdown_sections`; `specs` → `source = "spec"`, `markdown_sections`; `backlog` → `source = "backlog"`, `markdown_sections`. - **Auto-tagging**: Regex heuristics in `internal/tagging` detect session categories (debugging, refactoring, feature, testing, docs, config) during sync; stored in `session_tags` table. - **Content-type classification**: Messages classified as `text`/`code`/`tool`/`reasoning` based on message content types during sync. Tool content is indexed in separate `search_items` rows with `content_type='tool'`. Pi agent reasoning blocks are captured when `index_reasoning=true` (default off) in the input manifest and indexed with `content_type='reasoning'`. Sync writes only to `search_items`; the `session_events` table was dropped in migration v5. -- **Split FTS by retrieval semantics**: tool content (`content_type='tool'`) lives in a separate FTS5 index `tool_fts` (tokenizer `trigram`, substring/exact match for paths/commands/errors); prose content (text, code, reasoning) lives in `messages_fts` (`porter unicode61`). Migration v4 branched the triggers by content type. Migration v7 updated the triggers to route 'reasoning' alongside 'text'/'code' to `messages_fts`. `--content-type tool` queries `tool_fts`; prose queries `messages_fts`; an unfiltered query merges both via Reciprocal Rank Fusion (RRF, k=60), which fuses by rank position, not score magnitude, and is immune to incomparable cross-tokenizer BM25 scales. Before unfiltered fusion, `internal/storage/search.go` excludes canonical direct `Bash command=backscroll search ...` calls and results paired by call ID, using v15 provenance retained through sync and recovery; the Claude, Codex (`exec_command`/`cmd` and the exact `[, -c|-lc, cmd]` argv form) and OpenCode readers set that provenance from raw tool input, sharing one `isDirectSearchCommand` boundary in `internal/readers`. `PendingSearchEchoPaths` also requeues surviving sources whose tool rows were stored as `search_echo=0` with a serialized direct search call (pre-#80 Codex/OpenCode writes), so identity pairing can mark the result without guessing output shape. Both candidate streams enforce this after FTS rebuilds; explicit tool search retains the rows. See `docs/search.md#query-echo-handling` for exact boundaries and the unprovable expired-source limit. The trigram tokenizer matches substrings of ≥3 characters, so tool queries shorter than 3 characters will match zero results. +- **Split FTS by retrieval semantics**: tool content (`content_type='tool'`) lives in a separate FTS5 index `tool_fts` (tokenizer `trigram`, substring/exact match for paths/commands/errors); prose content (text, code, reasoning) lives in `messages_fts` (`porter unicode61`). Migration v4 branched the triggers by content type. Migration v7 updated the triggers to route 'reasoning' alongside 'text'/'code' to `messages_fts`. `--content-type tool` queries `tool_fts`; prose queries `messages_fts`; an unfiltered query merges both via Reciprocal Rank Fusion (RRF, k=60), which fuses by rank position, not score magnitude, and is immune to incomparable cross-tokenizer BM25 scales. Before unfiltered fusion, `internal/storage/search.go` excludes canonical direct `Bash command=backscroll search ...` calls and results paired by call ID, using v15 provenance retained through sync and recovery; the Claude, Codex (`exec_command`/`cmd` and the exact `[, -c|-lc, cmd]` argv form) and OpenCode readers set that provenance from raw tool input, sharing one `isDirectSearchCommand` boundary in `internal/readers`. `PendingSearchEchoPaths` also requeues surviving sources whose tool rows were stored as `search_echo=0` with a serialized direct search call (pre-#80 Codex/OpenCode writes), so identity pairing can mark the result without guessing output shape; the requeue candidate filter is the same broad-prefilter + `IsSerializedDirectSearchCall` split used by the page and IDF paths. Both candidate streams enforce this after FTS rebuilds; explicit tool search retains the rows. See `docs/search.md#query-echo-handling` for exact boundaries and the unprovable expired-source limit. The trigram tokenizer matches substrings of ≥3 characters, so tool queries shorter than 3 characters will match zero results. - **Pure Go SQLite**: `modernc.org/sqlite` — no CGO, trivially cross-compilable. - **Semantic schema lineage invariant (fixes #52/#58)**: Schema identity is the pair `(AppliedVersion, semantic Signature)`. Migration rows are provenance only and are never signature input. Canonical SQL is deliberately conservative: it discards comments and formatting, but preserves literals, quoted identifiers, constraints, expressions, triggers, indexes, and virtual-table configuration. Cosmetic DDL layout changes must not create a new identity, while any semantic schema change must. Every physical catalog fixture is an upgrade target; no recognized fixture may be skipped. Every new migration must pass `TestEveryCatalogFixtureReachesCurrentSemanticHead` before merge, proving all known physical lineages can reach the current semantic head. - **Startup coordination**: `github.com/gofrs/flock` via `internal/startuplock` — persistent `.startup-sync.lock` sidecar with `0600` permissions, OS-owned advisory lock, local-host-only WAL snapshot coordination, and read-only followers that never delete the sidecar. @@ -236,7 +236,7 @@ Workflows delegate to [pablontiv/crossbeam](https://github.com/pablontiv/crossbe github.com/pablontiv/backscroll/cmd/backscroll — CLI entrypoint github.com/pablontiv/backscroll/internal/config — Config structs and resolution github.com/pablontiv/backscroll/internal/compat — Stateless schema inspection, release lineage catalog, migration planning, and canonical recovery planning -github.com/pablontiv/backscroll/internal/directsearch — Shared direct-search predicates (IsDirectSearchCommand, IsCodexDirectSearchCall) used by readers and storage replay +github.com/pablontiv/backscroll/internal/directsearch — Shared direct-search predicates (IsDirectSearchCommand, IsCodexDirectSearchCall, IsSerializedDirectSearchCall): the single strict boundary used by readers at ingest and by storage at query/replay time github.com/pablontiv/backscroll/internal/input_config — Input manifest loading, discovery, and legacy session-dirs compatibility github.com/pablontiv/backscroll/internal/models — Domain types and SearchEngine interface github.com/pablontiv/backscroll/internal/sync — Session parsing and noise filtering diff --git a/cmd/backscroll/echo_shell_zero_query_gap_test.go b/cmd/backscroll/echo_shell_zero_query_gap_test.go index c6517c2..c421da7 100644 --- a/cmd/backscroll/echo_shell_zero_query_gap_test.go +++ b/cmd/backscroll/echo_shell_zero_query_gap_test.go @@ -14,9 +14,11 @@ import ( // 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 +// isDirectBackscrollSearchEcho and the IDF-counting path (the code that +// excludes a row from unfiltered result pages and --relax IDF counting // RIGHT NOW, before reparse converges it) never gained shell handling. +// Since the chokepoint refactor, both paths share +// directsearch.IsSerializedDirectSearchCall behind a broad SQL prefilter. // // 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 diff --git a/docs/research/2026-09-12-echo-chokepoint-spike.md b/docs/research/2026-09-12-echo-chokepoint-spike.md new file mode 100644 index 0000000..0739462 --- /dev/null +++ b/docs/research/2026-09-12-echo-chokepoint-spike.md @@ -0,0 +1,56 @@ +# Echo-Exclusion Chokepoint Spike + +Date: 2026-09-12 +Branch: `fm/bs-echo-chokepoint-hardening-r1` +Status: decided — proceed to implementation + +## Question + +Can the three independent recognizers of "is this stored row a direct +`backscroll search` echo" (page-exclusion Go predicate in +`internal/storage/search.go`, IDF-counting SQL GLOB in +`directBackscrollSearchEchoSQL`, requeue SQL GLOB in +`unmarkedDirectSearchCallSQL` + shell-only Go fallback) be replaced by one +strict Go predicate behind one broad SQL prefilter, without changing any +accepted boundary case — and does that structurally close the separator +alphabet divergence class (#64/#86/#87/#89) instead of point-patching the SQL +whitespace list? + +## PoC (RED) + +`internal/storage/echo_parity_test.go` generates the cross product of +serialized shape {bash, exec_command, shell} × separator alphabet {ASCII +controls, NBSP, U+2028, U+2029, U+3000, repeated/mixed runs} × +leading/trailing runs, stores each fixture as a zero-valued (`search_echo=0`) +tool row, and asserts three-way agreement: unfiltered page exclusion +(`isDirectBackscrollSearchEcho`) == unfiltered `--relax` IDF exclusion +(`recallFrequency`) == requeue detection (`PendingSearchEchoPaths`). + +Against the pre-change code it fails 48 subtests, all of them the bug class, +none a boundary dispute: + +- `bash`/`exec_command` × {NBSP, U+2028, U+2029, U+3000, mixed runs}: the Go + page predicate excludes the row (`strings.Fields` accepts every + `unicode.IsSpace` rune); the SQL GLOB separator alphabet is ASCII-only, so + IDF counting and requeue keep it. Patching NBSP into the GLOB class would + leave U+2028/U+2029/U+3000 and every future rune — confirming the point + patch is structurally insufficient. +- `shell` × leading-run: the `text LIKE 'shell %'` prefilter requires the row + to start with `shell`, so a leading whitespace run makes IDF/requeue miss + a row the Go decode excludes. A substring prefilter (`%backscroll%`) has no + anchor and no alphabet. + +All negative controls (status/searcher subcommands, absolute paths, env +wrappers, folded key case, nested `bash -lc`, prose mentions) already pass +and must keep passing unchanged. + +## Verdict + +Proceed. Every accepted echo row contains the literal lowercase substring +`backscroll` in its stored text (bash: `command=backscroll`; exec_command: +`cmd=backscroll`; shell: the JSON-encoded argv), so +`content_type='tool' AND COALESCE(search_echo,0)=0 AND text LIKE '%backscroll%'` +is a provable superset prefilter; the strict +`directsearch.IsSerializedDirectSearchCall` predicate applied in Go on the +prefiltered rows is then the single definition of the boundary for pages, +IDF, and requeue alike. No schema migration; deletes more code than it adds. diff --git a/docs/search.md b/docs/search.md index 9996b3d..7d99053 100644 --- a/docs/search.md +++ b/docs/search.md @@ -177,15 +177,20 @@ 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. 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. +IDF counting, recognizing the same serialized forms. The serialized-text +boundary has exactly one owner: `directsearch.IsSerializedDirectSearchCall` +recognizes all three stored shapes (`bash`, `exec_command`, and the Codex +`shell` wrapper, whose JSON-encoded argv is decoded and fed to the same +predicate the reader applies at ingest, with no token-count floor — an +all-escaped argv serializes to just two whitespace-separated tokens). Result +pages apply it directly in Go. IDF counting and requeue detection apply it in +Go over the rows returned by a broad SQL prefilter (`content_type='tool'`, +`search_echo` zero, text containing the literal substring `backscroll`) that +is a provable superset of every accepted shape, so no SQL-side pattern needs +to stay in sync with Go logic. Enumerating the accepted separator +byte-sequences in SQL was provably unbounded (every `unicode.IsSpace` rune +plus JSON escaping) and caused the page/IDF divergences fixed by this +structure. Subsequent source expiry, `rebuild`, and supported canonical recovery preserve proven pairing evidence. The general extraction epoch is unchanged. @@ -221,7 +226,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 ..."]`, 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. +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`, or any serialized invocation accepted by `directsearch.IsSerializedDirectSearchCall` — a `bash command=backscroll search ...` or `exec_command cmd=backscroll search ...` prefix, or a serialized Codex `shell` call whose JSON-encoded argv is exactly `[, "-c" | "-lc", "backscroll search ..."]`, all recognized in Go over a broad substring prefilter since the accepted 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. @@ -243,7 +248,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/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). +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), `internal/storage/relaxation_echo_idf_test.go` (echo-eligibility of unfiltered IDF, including echo-only DF=0 and the boundary-case corpus), and `internal/storage/echo_parity_test.go` (three-way page/IDF/requeue parity over shape × separator-alphabet × leading/trailing runs). ## Exit Codes diff --git a/internal/directsearch/directsearch.go b/internal/directsearch/directsearch.go index 66f5f35..5d36048 100644 --- a/internal/directsearch/directsearch.go +++ b/internal/directsearch/directsearch.go @@ -1,13 +1,15 @@ -// Package directsearch holds the shared predicates the Codex ingest path -// and the storage replay path both rely on to decide whether a stored tool -// call is a direct `backscroll search` invocation. Keeping a single source of -// truth here is the only way the SQL "what's pending requeue?" predicate and -// the reader's "did this row get marked?" predicate can stay in lockstep — -// every prior SQL GLOB attempt to enumerate separator byte-sequences missed at -// least one real encoding (Unicode whitespace that strings.Fields accepts but -// JSON escapes in non-uniform ways), so the replay check now decodes the JSON -// argv and re-runs the exact strings.Fields-based acceptance that ingest -// already uses. +// Package directsearch holds the shared predicates every path that must +// answer "is this a direct `backscroll search` call" relies on: the readers' +// ingest-time marking, the storage page exclusion, the --relax IDF counting, +// and the requeue detection. The serialized-text boundary has exactly one +// owner — IsSerializedDirectSearchCall — and SQL only ever applies a broad, +// provable-superset prefilter (the stored text of an accepted call always +// contains the literal substring "backscroll"); the strict decision is made +// in Go by this package. Hand-written SQL pattern equivalents were tried and +// abandoned: enumerating the separator byte-sequences strings.Fields accepts +// is unbounded (Unicode whitespace such as NBSP, U+2028/2029, U+3000; JSON +// control escapes in the shell argv), so per-path SQL recognizers always +// diverged from the Go predicate on some encoding. package directsearch import ( @@ -59,3 +61,79 @@ func IsCodexDirectSearchCall(tool, arguments string) bool { } return false } + +// IsSerializedDirectSearchCall reports whether serialized tool-input text, as +// produced by readers.SerializeToolInput, is a direct `backscroll search` +// call. It owns all three stored shapes, so the page exclusion, the --relax +// IDF counting, and the requeue detection can share one strict predicate +// behind a broad SQL prefilter instead of re-deriving the boundary per path: +// +// - bash: "Bash command=backscroll search ..." (tool-name token +// case-insensitive, key and subcommand exact) +// - exec_command: "exec_command cmd=backscroll search ..." (all tokens exact) +// - shell: "shell ... command=[\"\",\"-c\"|\"-lc\",\"\"] ..." +// (Codex wrapper; the argv is JSON-decoded and fed to the same +// IsCodexDirectSearchCall predicate the reader applies at ingest) +// +// Separators are whatever strings.Fields accepts, in every shape. The shell +// decode deliberately has no token-count floor: an argv whose separators are +// all JSON control escapes serializes to just two whitespace-separated tokens. +// +// The literal-substring guard below is what makes the SQL prefilter +// (text LIKE '%backscroll%') a provable superset of this predicate for every +// shape, not just bash/exec_command: the shell argv is stored as raw, +// un-decoded JSON bytes, so without the guard a JSON \u-escaped letter of +// "backscroll" (accepted after decoding) would be missed by the prefilter. +// The match is deliberately case-SENSITIVE: literal-lowercase-substring +// present implies LIKE matches, full stop. Folding case would go the wrong +// way — Go's ToLower folds strictly more than LIKE's ASCII-only fold +// (U+212A KELVIN SIGN lowercases to 'k' under ToLower but is invisible to +// LIKE), which would reopen exactly the divergence this guard closes. +func IsSerializedDirectSearchCall(text string) bool { + if !strings.Contains(text, "backscroll") { + return false + } + fields := strings.Fields(text) + if len(fields) == 0 { + return false + } + switch { + case strings.EqualFold(fields[0], "bash"): + return len(fields) >= 3 && fields[1] == "command=backscroll" && fields[2] == "search" + case fields[0] == "exec_command": + return len(fields) >= 3 && fields[1] == "cmd=backscroll" && fields[2] == "search" + case strings.EqualFold(fields[0], "shell"): + return serializedShellMatches(text) + } + return false +} + +// serializedShellMatches is the serialized-text half of the Codex shell +// wrapper boundary. SerializeToolInput emits the rollout's `arguments` as a +// space-joined `key=value` token list with keys sorted alphabetically. Real +// Codex shell calls carry not just `command` but also `workdir`, +// `timeout_ms`, and potentially `additional_permissions` or `sandbox` — so +// `command=` is not guaranteed to be the first key. We locate the ` command=` +// token boundary anywhere in the text and feed only what follows to a +// json.Decoder, which stops after reading one complete JSON value (the +// array). Whatever (already-serialized, non-JSON) `key=value` text follows +// the array is ignored. The decoded array is then wrapped into the object +// shape IsCodexDirectSearchCall expects and fed to the exact same predicate +// the reader uses at ingest time. +func serializedShellMatches(text string) bool { + const token = " command=" + idx := strings.Index(text, token) + if idx < 0 { + return false + } + remainder := text[idx+len(token):] + var commandArray []string + if err := json.NewDecoder(strings.NewReader(remainder)).Decode(&commandArray); err != nil { + return false + } + args, err := json.Marshal(map[string][]string{"command": commandArray}) + if err != nil { + return false + } + return IsCodexDirectSearchCall("shell", string(args)) +} diff --git a/internal/directsearch/directsearch_test.go b/internal/directsearch/directsearch_test.go index 6a36243..a37488b 100644 --- a/internal/directsearch/directsearch_test.go +++ b/internal/directsearch/directsearch_test.go @@ -174,3 +174,64 @@ func TestIsCodexDirectSearchCall_UnknownTool(t *testing.T) { t.Errorf("empty tool should not match, got true") } } + +func TestIsSerializedDirectSearchCall(t *testing.T) { + tests := []struct { + name, text string + want bool + }{ + // bash shape + {"bash_canonical", "Bash command=backscroll search --text orchard", true}, + {"bash_lowercase", "bash command=backscroll search", true}, + {"bash_uppercase", "BASH command=backscroll search", true}, + {"bash_tab_separator", "Bash command=backscroll\tsearch", true}, + {"bash_nbsp_separator", "Bash command=backscroll search", true}, + {"bash_ideographic_space", "Bash command=backscroll　search", true}, + {"bash_leading_run", " Bash command=backscroll search", true}, + {"bash_trailing_run", "Bash command=backscroll search ", true}, + {"bash_status_subcommand", "Bash command=backscroll status", false}, + {"bash_searcher", "Bash command=backscroll searcher", false}, + {"bash_absolute_path", "Bash command=/usr/local/bin/backscroll search", false}, + {"bash_env_wrapper", "Bash command=env backscroll search", false}, + {"bash_nested_lc", `Bash command=bash -lc "backscroll search"`, false}, + {"bash_folded_key", "Bash Command=backscroll search", false}, + {"bash_folded_subcommand", "Bash command=backscroll Search", false}, + {"bash_tool_name_only", "bash", false}, + // exec_command shape + {"exec_canonical", "exec_command cmd=backscroll search --text orchard", true}, + {"exec_nbsp_separator", "exec_command cmd=backscroll search", true}, + {"exec_status_subcommand", "exec_command cmd=backscroll status", false}, + {"exec_folded_tool_name", "Exec_Command cmd=backscroll search", false}, + // shell shape (Codex wrapper, argv JSON-encoded in the text) + {"shell_canonical", `shell command=["sh","-c","backscroll search"]`, true}, + {"shell_lc_flag", `shell command=["bash","-lc","backscroll search --text orchard"]`, true}, + {"shell_extra_sorted_keys", `shell additional_permissions=read command=["sh","-c","backscroll search"] timeout_ms=1000`, true}, + {"shell_json_escaped_tab", `shell command=["sh","-c","backscroll\tsearch\t--text\tquery"]`, true}, + {"shell_raw_nbsp_in_argv", "shell command=[\"sh\",\"-c\",\"backscroll search\"]", true}, + {"shell_leading_run", ` shell command=["sh","-c","backscroll search"]`, true}, + {"shell_wrong_flag", `shell command=["sh","-x","backscroll search"]`, false}, + {"shell_extra_argv_element", `shell command=["sh","-c","backscroll search","bar"]`, false}, + {"shell_non_search_call", `shell command=["sh","-c","backscroll status"]`, false}, + {"shell_env_wrapper_in_argv", `shell command=["sh","-c","env backscroll search"]`, false}, + {"shell_no_command_key", "shell workdir=/tmp", false}, + {"shell_malformed_json", "shell command=notjson", false}, + // The argv is stored as raw, un-decoded JSON bytes: a JSON \u-escaped + // letter of "backscroll" decodes to an accepted command but the stored + // text lacks the literal substring, so the broad SQL prefilter would + // miss what the Go predicate accepted. The literal-substring guard + // keeps the prefilter a provable superset of this predicate. + {"shell_json_escaped_letter", `shell command=["sh","-c","\u0062ackscroll search --text orchard"]`, false}, + // non-shapes + {"prose_mention", "run backscroll search from your shell", false}, + {"other_tool", "Bash command=rg backscroll .", false}, + {"empty", "", false}, + {"whitespace_only", " \t ", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsSerializedDirectSearchCall(tt.text); got != tt.want { + t.Errorf("IsSerializedDirectSearchCall(%q) = %v, want %v", tt.text, got, tt.want) + } + }) + } +} diff --git a/internal/storage/echo_parity_test.go b/internal/storage/echo_parity_test.go new file mode 100644 index 0000000..9d97cb1 --- /dev/null +++ b/internal/storage/echo_parity_test.go @@ -0,0 +1,198 @@ +package storage + +// Three-way echo-exclusion parity: for every serialized direct-search-call +// shape (bash, exec_command, shell) crossed with every separator alphabet +// strings.Fields accepts, the unfiltered page exclusion, the --relax IDF +// exclusion, and the requeue detection must agree. This is the structural +// regression net for the bug class behind #64/#86/#87/#89: previously each +// path recognized echoes with its own hand-written matcher (SQL GLOB with an +// ASCII-only whitespace alphabet vs Go strings.Fields), so any separator the +// SQL alphabet missed — NBSP, U+2028/2029, U+3000 — made pages and IDF +// diverge. There is now exactly one strict predicate +// (directsearch.IsSerializedDirectSearchCall) behind a broad SQL prefilter; +// this test would fail on any reintroduced per-path recognizer. + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/pablontiv/backscroll/internal/readers" +) + +// echoSeparators samples the unicode.IsSpace alphabet: ASCII controls, NBSP, +// U+2028/U+2029, U+3000, and repeated runs. Any separator here is legal in a +// real raw command string (bash/exec_command) or inside a JSON-encoded argv +// (shell, where Go's encoder emits U+2028/U+2029 as escapes and every other +// non-control rune as raw UTF-8 bytes). +var echoSeparators = []struct { + name string + sep string +}{ + {"space", " "}, + {"tab", "\t"}, + {"newline", "\n"}, + {"vertical-tab", "\v"}, + {"form-feed", "\f"}, + {"carriage-return", "\r"}, + {"nbsp", " "}, + {"line-separator", "
"}, + {"paragraph-separator", "
"}, + {"ideographic-space", "　"}, + {"nbsp-run", "  "}, + {"mixed-run", " \t "}, +} + +// echoParityFixture is one stored tool row plus its expected classification. +type echoParityFixture struct { + name string + text string + token string // unique ≥3-char token so trigram FTS can MATCH the row + wantEcho bool +} + +// echoParityCorpus generates the cross product of shape × separator × +// leading/trailing runs for direct calls, plus negative controls that must +// never classify as echoes regardless of separator. +func echoParityCorpus(t *testing.T) []echoParityFixture { + t.Helper() + var fixtures []echoParityFixture + n := 0 + next := func(prefix string) string { + n++ + // Fixed width + fixed suffix: no token is a substring of another, so + // the trigram phrase match for one fixture's token cannot hit another + // fixture's row. + return fmt.Sprintf("%s%03dzq", prefix, n) + } + + for _, s := range echoSeparators { + // bash / exec_command shapes carry the raw command string after the + // key= prefix; separators appear between the two canonical tokens and + // between the command and its arguments. + cmd := "backscroll" + s.sep + "search" + s.sep + "--text" + s.sep + for _, wrap := range []struct { + name, pre, post string + }{ + {"plain", "", ""}, + {"leading-run", s.sep + s.sep, ""}, + {"trailing-run", "", s.sep + s.sep}, + } { + tok := next("paritok") + fixtures = append(fixtures, echoParityFixture{ + name: fmt.Sprintf("bash/%s/%s", s.name, wrap.name), + text: wrap.pre + "Bash command=" + cmd + tok + wrap.post, token: tok, wantEcho: true, + }) + tok = next("paritok") + fixtures = append(fixtures, echoParityFixture{ + name: fmt.Sprintf("exec_command/%s/%s", s.name, wrap.name), + text: wrap.pre + "exec_command cmd=" + cmd + tok + wrap.post, token: tok, wantEcho: true, + }) + // shell shape: the separator lives inside the JSON-encoded argv, + // serialized through a real json.Marshal + SerializeToolInput + // round-trip so the stored text carries the encoder's escaping. + tok = next("paritok") + raw, err := json.Marshal(map[string]any{"command": []string{"sh", "-c", cmd + tok}}) + if err != nil { + t.Fatal(err) + } + fixtures = append(fixtures, echoParityFixture{ + name: fmt.Sprintf("shell/%s/%s", s.name, wrap.name), + text: wrap.pre + readers.SerializeToolInput("shell", raw) + wrap.post, + token: tok, + wantEcho: true, + }) + } + } + + // Negative controls: near-miss shapes that no path may classify as echo. + negatives := []struct{ name, text string }{ + {"bash/status", "Bash command=backscroll status"}, + {"bash/searcher", "Bash command=backscroll searcher"}, + {"bash/abs-path", "Bash command=/usr/local/bin/backscroll search"}, + {"bash/env-wrapper", "Bash command=env backscroll search"}, + {"bash/folded-command-key", "Bash Command=backscroll search"}, + {"bash/folded-search", "Bash command=backscroll Search"}, + {"bash/nested-lc", `Bash command=bash -lc "backscroll search"`}, + {"exec/status", "exec_command cmd=backscroll status"}, + {"prose-mention", "run backscroll search from your shell"}, + {"bash-other-tool", "Bash command=rg backscroll ."}, + // Shell argv stored as raw JSON bytes with a \u-escaped letter of + // "backscroll": decodes to an accepted command, but the stored text + // lacks the literal substring, so the strict predicate must reject it + // to keep the SQL prefilter a provable superset (all three paths agree + // on NOT excluding the row). + {"shell/json-escaped-letter", `shell command=["sh","-c","\u0062ackscroll search --text X"]`}, + // Same escaped-letter argv, plus a BACSCROLL near-miss + // elsewhere in the row: Go's ToLower folds U+212A KELVIN SIGN to 'k', + // turning this into the literal "backscroll", but SQLite LIKE folds + // only ASCII A-Z, so a case-folding guard would pass a row the SQL + // prefilter cannot see. The guard is case-sensitive precisely so this + // class cannot exist. + {"shell/json-escaped-letter-kelvin-near-miss", `shell command=["sh","-c","\u0062ackscroll search --text X"] note=BACKSCROLL`}, + } + for _, neg := range negatives { + tok := next("paritok") + fixtures = append(fixtures, echoParityFixture{ + name: neg.name, text: neg.text + " " + tok, token: tok, wantEcho: false, + }) + } + return fixtures +} + +// TestEchoExclusionThreeWayParity stores every fixture as a zero-valued +// (pre-#80, unmarked) tool row and asserts the three exclusion paths agree: +// the unfiltered page predicate, the --relax unfiltered IDF count, and the +// requeue detection in PendingSearchEchoPaths. +func TestEchoExclusionThreeWayParity(t *testing.T) { + db, cleanup := newTestDB(t) + t.Cleanup(cleanup) + + fixtures := echoParityCorpus(t) + paths := make(map[string]string, len(fixtures)) // token -> source_path + var files []IndexedFile + for i, fx := range fixtures { + path := fmt.Sprintf("/parity-%d.jsonl", i) + paths[fx.token] = path + files = append(files, IndexedFile{ + SourcePath: path, Source: "session", Hash: fx.token, + Messages: []IndexedMessage{{ + Ordinal: 0, UUID: fx.token, Role: "assistant", ContentType: "tool", + Text: fx.text, + }}, + }) + } + if err := db.SyncFiles(files); err != nil { + t.Fatal(err) + } + // Zero out provenance: pre-#80 writers stored search_echo=0, the state in + // which every path must fall back to the serialized-text chokepoint. + if _, err := db.db.Exec(`UPDATE search_items SET search_echo = 0 WHERE content_type = 'tool'`); err != nil { + t.Fatal(err) + } + + pending, err := db.PendingSearchEchoPaths() + if err != nil { + t.Fatal(err) + } + pendingSet := make(map[string]bool, len(pending)) + for _, p := range pending { + pendingSet[p] = true + } + + for _, fx := range fixtures { + t.Run(fx.name, func(t *testing.T) { + page := isDirectBackscrollSearchEcho(SearchResult{ContentType: "tool", Text: fx.text}) + idf, err := db.recallFrequency(recallTerm{text: fx.token}, "") + if err != nil { + t.Fatal(err) + } + idfExcluded := idf == 0 + requeued := pendingSet[paths[fx.token]] + if page != fx.wantEcho || idfExcluded != fx.wantEcho || requeued != fx.wantEcho { + t.Fatalf("three-way divergence for %q: page=%v idfExcluded=%v(idf=%d) requeued=%v, want echo=%v", + fx.text, page, idfExcluded, idf, requeued, fx.wantEcho) + } + }) + } +} diff --git a/internal/storage/queries.go b/internal/storage/queries.go index db143ed..e0adf59 100644 --- a/internal/storage/queries.go +++ b/internal/storage/queries.go @@ -3,7 +3,6 @@ package storage import ( "context" "database/sql" - "encoding/json" "fmt" "sort" "strings" @@ -992,15 +991,14 @@ func (d *Database) StalePaths(currentVersion int) ([]string, error) { // with a serialized direct search call (pre-#80 Codex/OpenCode writes), so // identity pairing can mark the paired result. Output-only rows stay unmatched. // -// The bash and exec_cmd cases are filtered in SQL — their stored text is the -// simple `name key=value` token form with no JSON encoding, so a GLOB prefix -// is faithful. The Codex shell case is filtered in Go (see -// filterShellEchoZeroPaths / pendingSearchEchoShellMatches): its arguments -// are JSON-encoded and the on-disk separator between 'backscroll' and -// 'search' can be any form strings.Fields accepts but the JSON encoder -// leaves in the wild (literal space, \t/\n/\f/\r, \uXXXX, or raw UTF-8 bytes -// for non-control whitespace). Trying to enumerate every byte sequence in -// SQL GLOB is provably unbounded; decoding the JSON argv in Go is not. +// Candidate discovery uses a broad SQL prefilter (searchEchoZeroPrefilterSQL: +// any echo-zero tool row containing the literal substring "backscroll") and +// the strict per-shape decision happens in Go via +// directsearch.IsSerializedDirectSearchCall (filterEchoZeroPaths). Enumerating +// the accepted separator byte-sequences in SQL is provably unbounded — +// strings.Fields accepts every unicode.IsSpace rune and the shell argv adds +// JSON escaping on top — so no SQL-side shape matching exists to drift out of +// sync. func (d *Database) PendingSearchEchoPaths() ([]string, error) { return d.stalePaths(0, true) } @@ -1014,16 +1012,12 @@ func (d *Database) stalePaths(currentVersion int, echoOnly bool) ([]string, erro AND ((? AND (search_items.extraction_version IS NULL OR search_items.extraction_version < ?)) OR search_items.search_echo IS NULL` if echoOnly { - // Broad filter for shell candidates (any tool row whose text starts - // with the bare `shell` tool-name token). The strict - // isCodexDirectSearchCall check happens in Go via - // pendingSearchEchoShellMatches, which locates the `command=` token - // boundary inside the sorted key=value list (it may not be the first - // key — e.g. `additional_permissions` sorts before `command`) and - // decodes just the JSON array that follows. + // Broad prefilter for echo-zero candidates: any tool row whose stored + // text contains the literal "backscroll" substring. The strict + // per-shape check happens in Go (filterEchoZeroPaths) via + // directsearch.IsSerializedDirectSearchCall. query += ` - OR (` + unmarkedDirectSearchCallSQL("search_items") + `) - OR (search_items.content_type = 'tool' AND COALESCE(search_items.search_echo, 0) = 0 AND search_items.text LIKE 'shell %')` + OR (` + searchEchoZeroPrefilterSQL("search_items") + `)` } query += `) ORDER BY indexed_files.last_indexed ASC, search_items.source_path ASC @@ -1049,7 +1043,7 @@ func (d *Database) stalePaths(currentVersion int, echoOnly bool) ([]string, erro } if echoOnly { - filtered, err := d.filterShellEchoZeroPaths(paths) + filtered, err := d.filterEchoZeroPaths(paths) if err != nil { return nil, err } @@ -1059,13 +1053,12 @@ func (d *Database) stalePaths(currentVersion int, echoOnly bool) ([]string, erro return paths, nil } -// filterShellEchoZeroPaths drops paths whose only echo-zero shell candidate -// row is rejected by directsearch.IsCodexDirectSearchCall. A path survives if -// it has a v15 NULL search_echo row (those are always kept), a non-shell -// echo-zero candidate (bash / exec_cmd) that already passed the SQL filter, -// or at least one echo-zero shell row that the strict reader predicate -// accepts. -func (d *Database) filterShellEchoZeroPaths(paths []string) ([]string, error) { +// filterEchoZeroPaths drops paths whose only echo-zero candidate rows are +// rejected by the strict serialized-text predicate. A path survives if it has +// a v15 NULL search_echo row (those are always kept), or at least one +// echo-zero row that directsearch.IsSerializedDirectSearchCall accepts — for +// any of the three stored shapes (bash, exec_command, shell). +func (d *Database) filterEchoZeroPaths(paths []string) ([]string, error) { if len(paths) == 0 { return paths, nil } @@ -1085,51 +1078,31 @@ func (d *Database) filterShellEchoZeroPaths(paths []string) ([]string, error) { keep[path] = true continue } - // Reason 2: non-shell echo-zero survivor (bash / exec_cmd GLOB). - var globHit int - err = d.db.QueryRow(` - SELECT 1 FROM search_items - WHERE source_path = ? - AND content_type = 'tool' - AND COALESCE(search_echo, 0) = 0 - AND text NOT LIKE 'shell %' - AND (`+unmarkedDirectSearchCallSQL("search_items")+`) - LIMIT 1 - `, path).Scan(&globHit) - if err != nil && err != sql.ErrNoRows { - return nil, fmt.Errorf("check non-shell echo-zero for %s: %w", path, err) - } - if globHit == 1 { - keep[path] = true - continue - } - // Reason 3: echo-zero shell candidate. Apply the strict reader - // predicate — keep the path if ANY row matches. + // Reason 2: echo-zero serialized candidate. Apply the strict + // chokepoint predicate — keep the path if ANY row matches. rows, err := d.db.Query(` SELECT text FROM search_items WHERE source_path = ? - AND content_type = 'tool' - AND COALESCE(search_echo, 0) = 0 - AND text LIKE 'shell %' + AND `+searchEchoZeroPrefilterSQL("search_items")+` `, path) if err != nil { - return nil, fmt.Errorf("query shell rows for %s: %w", path, err) + return nil, fmt.Errorf("query echo-zero rows for %s: %w", path, err) } matched := false for rows.Next() { var text string if err := rows.Scan(&text); err != nil { _ = rows.Close() - return nil, fmt.Errorf("scan shell row for %s: %w", path, err) + return nil, fmt.Errorf("scan echo-zero row for %s: %w", path, err) } - if pendingSearchEchoShellMatches(text) { + if directsearch.IsSerializedDirectSearchCall(text) { matched = true break } } _ = rows.Close() if err := rows.Err(); err != nil { - return nil, fmt.Errorf("iterate shell rows for %s: %w", path, err) + return nil, fmt.Errorf("iterate echo-zero rows for %s: %w", path, err) } if matched { keep[path] = true @@ -1144,47 +1117,6 @@ func (d *Database) filterShellEchoZeroPaths(paths []string) ([]string, error) { return out, nil } -// 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). -// 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 -// calls carry not just `command` but also `workdir`, `timeout_ms`, and -// potentially `additional_permissions` or `sandbox` — so `command=` is not -// guaranteed to be the first key. We locate the ` command=` token boundary -// anywhere in the text and feed only what follows to a json.Decoder, which -// stops after reading one complete JSON value (the array). Whatever -// (already-serialized, non-JSON) `key=value` text follows the array is -// ignored. The decoded array is then wrapped into the object shape -// directsearch.IsCodexDirectSearchCall expects and fed to the exact same -// predicate the reader uses at ingest time. -func pendingSearchEchoShellMatches(text string) bool { - const token = " command=" - idx := strings.Index(text, token) - if idx < 0 { - return false - } - remainder := text[idx+len(token):] - var commandArray []string - if err := json.NewDecoder(strings.NewReader(remainder)).Decode(&commandArray); err != nil { - return false - } - args, err := json.Marshal(map[string][]string{"command": commandArray}) - if err != nil { - return false - } - return directsearch.IsCodexDirectSearchCall("shell", string(args)) -} - // ReresolveProjects iterates all distinct source_paths where project='unknown' or project IS NULL, // calls the resolver function for each path, and updates ALL rows for that path with the returned project ID. // If resolver returns empty string or "unknown", the source_path is skipped and rows remain unchanged. diff --git a/internal/storage/relaxation.go b/internal/storage/relaxation.go index c6d0244..c8833a0 100644 --- a/internal/storage/relaxation.go +++ b/internal/storage/relaxation.go @@ -6,6 +6,7 @@ import ( "strings" "unicode" + "github.com/pablontiv/backscroll/internal/directsearch" "github.com/pablontiv/backscroll/internal/models" ) @@ -219,36 +220,36 @@ func (d *Database) recallFrequency(term recallTerm, contentType string) (int, er } var count int err := d.db.QueryRow( - "SELECT COUNT(*) FROM ("+matched+") matched JOIN search_items si ON si.id = matched.rowid WHERE NOT ("+directBackscrollSearchEchoSQL("si")+")", + "SELECT COUNT(*) FROM ("+matched+") matched JOIN search_items si ON si.id = matched.rowid WHERE NOT ("+directSearchEchoSQL("si")+")", args..., ).Scan(&count) 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 %'", + // Serialized-text fallback rows (search_echo=0) are never recognized in + // SQL: the accepted separator byte-sequences are unbounded for SQL + // pattern matching. Subtract them with a broad, provable-superset + // prefilter plus the strict Go chokepoint, so unfiltered IDF counts the + // exact row set the page exclusion keeps — for all three shapes (bash, + // exec_command, shell), not just the shell wrapper. + echoRows, err := d.db.Query( + "SELECT si.text FROM ("+matched+") matched JOIN search_items si ON si.id = matched.rowid WHERE "+searchEchoZeroPrefilterSQL("si"), args..., ) if err != nil { return 0, fmt.Errorf("measure relaxation term frequency: %w", err) } - defer func() { _ = shellRows.Close() }() - for shellRows.Next() { + defer func() { _ = echoRows.Close() }() + for echoRows.Next() { var text string - if err := shellRows.Scan(&text); err != nil { + if err := echoRows.Scan(&text); err != nil { return 0, fmt.Errorf("measure relaxation term frequency: %w", err) } - if pendingSearchEchoShellMatches(text) { + if directsearch.IsSerializedDirectSearchCall(text) { count-- } } - if err := shellRows.Err(); err != nil { + if err := echoRows.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 f7b6bb3..fc16efc 100644 --- a/internal/storage/relaxation_echo_idf_test.go +++ b/internal/storage/relaxation_echo_idf_test.go @@ -286,7 +286,13 @@ func TestRelaxationUnfilteredIDFTreatsEchoOnlyTermsAsAbsent(t *testing.T) { } } -func TestRecallFrequencySQLEchoPredicatePreservesBoundaries(t *testing.T) { +// TestEchoBoundaryCasesThreeWayParity pins the accepted/rejected boundary for +// every hand-picked lookalike and asserts the three exclusion paths agree on +// each: the page predicate, the unfiltered IDF count, and the requeue +// detection. There is deliberately no SQL-side shape predicate left to +// compare against — SQL applies only the exact search_echo check and the +// broad substring prefilter, and the strict decision is shared Go code. +func TestEchoBoundaryCasesThreeWayParity(t *testing.T) { db, cleanup := newTestDB(t) t.Cleanup(cleanup) @@ -298,12 +304,6 @@ 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. @@ -318,7 +318,6 @@ func TestRecallFrequencySQLEchoPredicatePreservesBoundaries(t *testing.T) { } 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}, @@ -338,19 +337,22 @@ 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}, + // Codex shell wrapper form: excluded by the same shared Go predicate, + // and by recallFrequency's broad-prefilter + strict-Go subtraction. + {name: "canonical shell", contentType: "tool", text: shellText(t, "bash", "-lc", "backscroll search --text boundtok20"), unique: "boundtok20", wantEcho: true}, + {name: "shell extra sorted keys", contentType: "tool", text: shellFull([]string{"sh", "-c", "backscroll search --text boundtok21"}), unique: "boundtok21", wantEcho: true}, + {name: "shell /bin/bash path", contentType: "tool", text: shellText(t, "/bin/bash", "-lc", "backscroll search --text boundtok22"), unique: "boundtok22", wantEcho: true}, + // Real NBSP (U+00A0, bytes 0xC2 0xA0) between the canonical tokens: + // Go's json encoder emits NBSP as raw UTF-8 bytes (it escapes only + // control chars and U+2028/2029), and strings.Fields splits on it. + {name: "shell NBSP separator", contentType: "tool", text: shellText(t, "sh", "-c", "backscroll\u00a0search --text boundtok23"), unique: "boundtok23", wantEcho: true}, // 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 JSON-escaped tab separators", contentType: "tool", text: shellText(t, "sh", "-c", "backscroll\tsearch\t--text\tboundtok31"), unique: "boundtok31", wantEcho: true}, + {name: "null echo shell fallback", contentType: "tool", text: shellText(t, "bash", "-lc", "backscroll search --text boundtok24"), nullEcho: true, unique: "boundtok24", wantEcho: true}, {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"}, @@ -382,15 +384,23 @@ func TestRecallFrequencySQLEchoPredicatePreservesBoundaries(t *testing.T) { } } - for _, tc := range cases { + pending, err := db.PendingSearchEchoPaths() + if err != nil { + t.Fatal(err) + } + pendingSet := make(map[string]bool, len(pending)) + for _, p := range pending { + pendingSet[p] = true + } + + for i, tc := range cases { t.Run(tc.name, func(t *testing.T) { var result SearchResult var echo int - var sqlEcho int err := db.db.QueryRow( - "SELECT si.content_type, COALESCE(si.search_echo, 0), si.text, CASE WHEN "+directBackscrollSearchEchoSQL("si")+" THEN 1 ELSE 0 END FROM search_items si WHERE si.uuid = ?", + "SELECT si.content_type, COALESCE(si.search_echo, 0), si.text FROM search_items si WHERE si.uuid = ?", tc.unique, - ).Scan(&result.ContentType, &echo, &result.Text, &sqlEcho) + ).Scan(&result.ContentType, &echo, &result.Text) if err != nil { t.Fatal(err) } @@ -399,13 +409,6 @@ 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) } - 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}, "") if err != nil { @@ -418,6 +421,15 @@ func TestRecallFrequencySQLEchoPredicatePreservesBoundaries(t *testing.T) { if got != wantDF { t.Fatalf("unfiltered IDF(%s)=%d, want %d", tc.unique, got, wantDF) } + + // Requeue detection: NULL-backlog rows are always requeued; a + // zero-valued row is requeued iff the chokepoint accepts its text; + // an already-proven echo row is classified and never requeued. + path := fmt.Sprintf("/bound-%d.jsonl", i) + wantRequeue := tc.nullEcho || (tc.wantEcho && !tc.echo) + if pendingSet[path] != wantRequeue { + t.Fatalf("requeue(%s) = %v, want %v (echo=%d nullEcho=%v text=%q)", tc.name, pendingSet[path], wantRequeue, echo, tc.nullEcho, result.Text) + } }) } } diff --git a/internal/storage/search.go b/internal/storage/search.go index 948bb29..80a15f1 100644 --- a/internal/storage/search.go +++ b/internal/storage/search.go @@ -6,6 +6,7 @@ import ( "strings" "time" + "github.com/pablontiv/backscroll/internal/directsearch" "github.com/pablontiv/backscroll/internal/hybrid" "github.com/pablontiv/backscroll/internal/models" ) @@ -381,19 +382,6 @@ func refillCandidatesWithoutDirectEchoes(opts models.SearchOptions, loadPage fun return filtered, nil } -// excludeDirectBackscrollSearchEchoes removes direct Backscroll retrieval calls -// from the tool candidates used for unfiltered recall. Explicit tool-only search -// bypasses this function and retains the commands. -func excludeDirectBackscrollSearchEchoes(results []SearchResult) []SearchResult { - filtered := make([]SearchResult, 0, len(results)) - for _, result := range results { - if !isDirectBackscrollSearchEcho(result) { - filtered = append(filtered, result) - } - } - return filtered -} - func isDirectBackscrollSearchEcho(result SearchResult) bool { if result.ContentType != "tool" { return false @@ -401,86 +389,28 @@ func isDirectBackscrollSearchEcho(result SearchResult) bool { if result.SearchEcho { 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" - } - return fields[0] == "exec_command" && fields[1] == "cmd=backscroll" && fields[2] == "search" + // The serialized-text boundary is owned by exactly one predicate, + // shared with the --relax IDF counting and the requeue detection. + return directsearch.IsSerializedDirectSearchCall(result.Text) } -// asciiWhitespaceSQL is the ASCII subset of unicode.IsSpace. SQL-side echo -// matching trims a leading run and treats one separator between tokens. -const asciiWhitespaceSQL = "char(9, 10, 11, 12, 13, 32)" - -// directBackscrollSearchEchoSQL is the SQL equivalent of -// isDirectBackscrollSearchEcho for the given search_items alias. Keep them in -// lockstep: tool rows with search_echo != 0, or a three-token prefix of -// case-insensitive "bash", exact "command=backscroll", exact "search"; or -// exact "exec_command", exact "cmd=backscroll", exact "search". GLOB is -// case-sensitive, so only the bash token uses a character class; LIKE would -// 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 + " || ']'" - bash := "'[Bb][Aa][Ss][Hh]' || " + sep + " || 'command=backscroll' || " + sep + " || 'search'" - execCmd := "'exec_command' || " + sep + " || 'cmd=backscroll' || " + sep + " || 'search'" - glob := func(prefix string) string { - return trimmed + " GLOB (" + prefix + ") OR " + trimmed + " GLOB (" + prefix + " || " + sep + " || '*')" - } - return alias + ".content_type = 'tool' AND (COALESCE(" + alias + ".search_echo, 0) != 0 OR " + - glob(bash) + " OR " + glob(execCmd) + ")" +// directSearchEchoSQL is the SQL-exact half of the echo boundary: rows with +// proven provenance. Serialized-text fallback rows are never recognized in +// SQL — see searchEchoZeroPrefilterSQL. +func directSearchEchoSQL(alias string) string { + return alias + ".content_type = 'tool' AND COALESCE(" + alias + ".search_echo, 0) != 0" } -// unmarkedDirectSearchCallSQL matches tool rows stored as search_echo=0 whose -// serialized text is a direct Backscroll search call. Pre-#80 Codex/OpenCode -// readers wrote false as 0, so those rows are not in the v15 NULL backlog. -// Requeueing the call's source file lets identity pairing mark the result; -// output shape is never matched here. -// -// Only the bash and exec_cmd cases are matched in SQL: their stored text uses -// simple `name key=value` tokens with no JSON encoding, so a GLOB prefix is -// faithful. The Codex shell case has its own predicate in Go (see -// PendingSearchEchoPaths and directsearch.IsCodexDirectSearchCall) because -// its arguments are JSON-encoded and the on-disk separator between -// 'backscroll' and 'search' can be any form strings.Fields accepts but the -// JSON encoder leaves in the wild (literal space, \t/\n/\f/\r, \uXXXX, or -// raw UTF-8 bytes for non-control whitespace). Trying to enumerate every -// byte sequence in SQL GLOB is provably unbounded; decoding the JSON argv -// in Go is not. -func unmarkedDirectSearchCallSQL(alias string) string { - trimmed := "ltrim(" + alias + ".text, " + asciiWhitespaceSQL + ")" - sep := "'[' || " + asciiWhitespaceSQL + " || ']'" - bash := "'[Bb][Aa][Ss][Hh]' || " + sep + " || 'command=backscroll' || " + sep + " || 'search'" - execCmd := "'exec_command' || " + sep + " || 'cmd=backscroll' || " + sep + " || 'search'" - glob := func(prefix string) string { - return trimmed + " GLOB (" + prefix + ") OR " + trimmed + " GLOB (" + prefix + " || " + sep + " || '*')" - } - return alias + ".content_type = 'tool' AND COALESCE(" + alias + ".search_echo, 0) = 0 AND (" + - glob(bash) + " OR " + glob(execCmd) + ")" +// searchEchoZeroPrefilterSQL is the broad SQL prefilter for rows whose +// serialized text MIGHT be a direct search call, used by recallFrequency and +// the requeue detection before the strict Go predicate +// (directsearch.IsSerializedDirectSearchCall) makes the final decision. It is +// a provable superset of every accepted shape: an accepted bash, +// exec_command, or shell row always carries the literal substring +// "backscroll" in its stored text, and substring LIKE has no separator +// alphabet, no anchor, and no token-count floor to keep in sync with Go. +func searchEchoZeroPrefilterSQL(alias string) string { + return alias + ".content_type = 'tool' AND COALESCE(" + alias + ".search_echo, 0) = 0 AND " + alias + ".text LIKE '%backscroll%'" } // mergeRRF uses Reciprocal Rank Fusion to merge two ranked lists by position, diff --git a/internal/storage/search_test.go b/internal/storage/search_test.go index 2be4e26..b8da6bf 100644 --- a/internal/storage/search_test.go +++ b/internal/storage/search_test.go @@ -103,7 +103,7 @@ func TestMergeRRF_NoOverlap(t *testing.T) { } } -func TestExcludeDirectBackscrollSearchEchoesPreservesBoundaries(t *testing.T) { +func TestRefillCandidatesWithoutDirectEchoesPreservesBoundaries(t *testing.T) { results := []SearchResult{ {ID: 1, ContentType: "tool", Text: "Bash command=backscroll search --text needle"}, {ID: 2, ContentType: "tool", Text: "bash command=backscroll search --text needle"}, @@ -118,7 +118,19 @@ func TestExcludeDirectBackscrollSearchEchoesPreservesBoundaries(t *testing.T) { {ID: 11, ContentType: "text", Text: "Bash command=backscroll search --text needle"}, } - filtered := excludeDirectBackscrollSearchEchoes(results) + filtered, err := refillCandidatesWithoutDirectEchoes(models.SearchOptions{Limit: len(results)}, func(opts models.SearchOptions) ([]SearchResult, error) { + if opts.Offset >= len(results) { + return nil, nil + } + end := opts.Offset + opts.Limit + if end > len(results) { + end = len(results) + } + return results[opts.Offset:end], nil + }) + if err != nil { + t.Fatal(err) + } var got []int for _, result := range filtered { got = append(got, result.ID)