fix: hybrid retrieval was pure vector for any question written as a sentence - #203
Merged
Conversation
…entence
`_safe_fts_query` quoted every token and left FTS5's default implicit AND in
place, so a question required every stopword — "why", "would", "a", "be", "in" —
to appear in the document. Almost nothing does. Measured on a real 5-document
KB:
"CrashLoopBackOff" FTS ranks 1, 2
"pod CrashLoopBackOff" FTS ranks 1, 2
"why would a pod be stuck in CrashLoopBackOff" FTS contributed nothing
A sentence is this product's dominant input — Chat/Consultation, Telegram, a
ticket body — so hybrid search was silently pure-vector in normal use, with no
error and no warning. Every existing test fed one or two keywords; the longest
was "VPN authentication", which is why nothing caught it.
Tokens are now OR-ed. OR costs no precision here because BM25 supplies it: a
rare token weighs far more than a stopword, and a document matching several
tokens outranks one matching a single common word. Fusion consumes *ranks*
(RRF), so what the keyword arm owes is recall of the exact tokens embeddings
miss — and under AND it owed that and delivered nothing.
Same corpus, after:
"why would a pod be stuck in CrashLoopBackOff" 1/1 2/2 3/3
"who do I page for a P1 still open after 30 minutes" 1/3 2/6 7/2
The last row is the arm doing its job: a chunk the vector side ranked 7th
arrives because FTS ranked it 2nd.
This is the failure PR-8.5 worked around by stripping [REDACTED:...] tokens
before they "crater implicit-AND recall". That removed some noise tokens;
ordinary English supplied the rest.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vicenteliu
added a commit
that referenced
this pull request
Aug 19, 2026
* fix: an exhausted tool loop answers, instead of echoing its own preamble Found by re-running the end-to-end path after #201-#205. Asked "How do I renew a TLS certificate before it expires?", deepseek-v4-flash ran `kb_search` six times — three of them the identical query — hit CHAT_MAX_TURNS, and the user got 62 characters: Let me check the knowledge base for any relevant procedures. with seven citations attached to it and 578 output tokens billed. The loop's exhaustion branch returned the last round's `resp.content`. But the last round produced a *tool call*, so that content is the preamble the model writes before reaching for a tool — never an answer. The comment said "answer with whatever the last turn produced"; the last turn produced a tool call. The cap now bounds the *tool* rounds. When they run out, one more round goes out with no tools: the results are already in `provider_msgs`, and with nothing left to call, the only move is to answer from them. Measured on the same question, same forced condition, live models: before 73 chars "Let me search the knowledge base for TLS certificate…" after 1561 chars the certbot / DigiCert renewal procedure, 5 citations The final prompt is *rebuilt*, not extended. `system_prompt` tells the model to call kb_search before answering, to call report_conflict *before it answers*, and which skills it may load — three instructions it can no longer follow, and the conflict one is a precondition it would be stuck on. PROPOSAL_HINT survives, because offering a fact to Memory is prose rather than a tool call. The instruction rides the system prompt rather than an appended user turn: `role="tool"` renders as a `tool_result` block inside a *user* message on Anthropic, so appending one more user message would stack two in a row. Verified against live claude-haiku-4-5 and deepseek-v4-flash — both return a full grounded answer through the exhaustion path. The model repeating a query it has already run is a separate, cheaper problem. It no longer costs the user an answer. behaviour-gate: 6 passed — memory injection 3/3, conflict reported 3/3, distillation keeps dead ends 3/3, proposals stay read-only 3/3, memory proposal 3/3, memory proposal restraint 3/3 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: one sample directory fed two commands, and ingest lost README.md:214 tells a new user to run opspilot ingest examples/sample_data_en/kb/ On the repo's own sample data that reported `10 succeeded · 5 failed`. The five failures were `chunks.jsonl` files raising `AdapterError: unsupported file type`, and worse, the five `doc-meta.json` sidecars were ingested *as knowledge documents* — 5 of the 17 chunks in a fresh KB were JSON metadata. Two of them came back in the top five for "why would a pod be stuck in CrashLoopBackOff". Neither half is a bug in isolation. That directory was built in 545ae7e as the sample input for `opspilot kb load-dir`, which recursively loads doc-meta.json + chunks.jsonl pairs. The README pointed a second, different command at the same tree later. One directory, two commands, both correct on their own, never run against each other. Split it: source documents stay in `kb/`, their frozen projections move to `fixtures/`. No product code changes, and no README changes either — the command it already documents is now the one that works. ingest examples/sample_data_en/kb/ 5 succeeded · 0 failed · 12 chunks kb load-dir examples/sample_data_en/fixtures/ 5 pairs, ids chk_f3a40001… The same search that used to surface metadata at ranks 3 and 5 now returns SOP prose in every position. `fixtures/README.md` says why they live apart, because the obvious tidy-up is to move them back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: retrieval joins the behaviour gate, and ROADMAP stops lying twice **The open decision is closed.** #206 left one deliberately: whether `kb/retrieval.py` belongs on the behaviour gate's protected paths. It does. #203 shipped without gate evidence, and what it changed was which chunks reach the model at all — the input every one of the five prompt-driven behaviours is judged on. The list's own rule already settled it: over-triggering costs minutes, missing the change costs the reason the gate exists. The entry carries that reasoning, because retrieval.py holds no prompt and the next reader will ask why it is there. **Two counts were stale.** #205 added the fifth behaviour and neither the CI comment nor the Makefile banner followed; CI also printed a paste-me example reading `behaviour-gate: 4/4 passed (votes 3/3, 3/3, 3/3, 3/3)`. The gate only greps for the `^behaviour-gate:` prefix, so a contributor copying that example would have landed a permanent "4/4" for a six-case run — in the one artifact the comment above it calls the whole point. **ROADMAP described two shipped things as open.** #175's silent model swap was fixed in #177 (`model_fallback` trace event, result re-labelled), and the proposed-actions preview/execute UI shipped in #190. Both still read as outstanding work. The real gap in proposed actions is elsewhere and now says so: nothing in `playbooks/` opts in, so an escalated Session returns `{"actions": []}` on every fresh install, and outside ROADMAP the key is named nowhere — not in a playbook, not in ADR-0028, which says only that playbooks opt in. **And the second run is recorded.** Ten checks against the nine fixes from #201-#205, ten passes. Three further defects, two fixed in this PR, plus one rough edge: the CLI writes as `cli:<osuser>` while the loopback API writes as `local-dev`, so `opspilot workingset status` reports nothing open while the web UI has a set open for the same person. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_safe_fts_query(kb/retrieval.py:42) quoted every token and left FTS5's default implicit AND in place. A question therefore required every stopword — "why", "would", "a", "be", "in" — to be present in the document. Almost nothing is.Measured, before
Real 5-document KB,
rankshown as vector/FTS:CrashLoopBackOff1/1 2/2 3/-pod CrashLoopBackOff1/1 2/2 3/-why would a pod be stuck in CrashLoopBackOff1/- 2/- 3/-The keyword arm contributed nothing to the third — hybrid search silently degraded to pure vector, with no error and no warning. And a sentence is the product's dominant input shape: Chat/Consultation, Telegram, a ticket body.
Why no test caught it
Every query in the suite is one or two keywords. The complete set, extracted from
tests/:The longest is two tokens.
The change
Tokens are OR-ed instead of AND-ed.
OR costs no precision here, because BM25 supplies it: a rare token weighs far more than a stopword, and a document matching several tokens outranks one matching a single common word. Fusion consumes ranks (RRF), so what the keyword arm owes is recall of the exact tokens embeddings miss — and under AND it owed that and delivered nothing.
Measured, after
CrashLoopBackOff1/1 2/2 3/-(unchanged)why would a pod be stuck in CrashLoopBackOff1/1 2/2 3/3who do I page for a P1 that is still open after 30 minutes1/3 2/6 **7/2**The last row is the arm doing its job: a chunk the vector side ranked 7th arrives because FTS ranked it 2nd.
Prior art in this repo
PR-8.5 hotfixed the same failure from the other end — stripping
[REDACTED:...]placeholders because "those tokens crater implicit-AND recall" (tests/test_orchestrator.py:906). That removed some noise tokens; ordinary English supplied the rest.Verification
9 new tests in
test_fts_sentence_recall.py: the OR shape, that a single token is unchanged, that FTS5 syntax is still neutralised (the sanitiser's original job), CJK tokenising, that a question now reaches the chunk answering it, that the old AND shape still finds nothing (kept as the record of what a regression looks like), that a bare keyword is unaffected, and that BM25 still orders the widened candidate set correctly.pytest -m "not slow and not requires_ollama and not requires_api_key"— 1320 passed. No existing test changed, which is itself the evidence for the paragraph above.One question for review
kb/retrieval.pyis not on the behaviour gate's protected list, so no gate evidence was required. This change does alter what context reaches the model. Worth deciding whether retrieval belongs on that list — I did not add it here, since expanding the gate is a decision, not a bug fix.🤖 Generated with Claude Code