Repository navigation
feat(native): search follow-up questions with the previous question #80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| package team.sopa.boar.engine.answer | ||
|
|
||
| import team.sopa.boar.engine.rag.Compress | ||
| import team.sopa.boar.engine.rag.ConversationHistory | ||
| import team.sopa.boar.engine.rag.Js | ||
|
|
||
| /** | ||
| * What to search the library for when the question is a follow-up (port of src/routing/followUp.ts; golden test: | ||
| * scripts/export-native-golden-followup.mjs). | ||
| * | ||
| * Retrieval saw only the question's own words, so a follow-up found nothing: "What did he publish?" after a Darwin | ||
| * answer, "What are the side effects?" after a vaccines answer (Pixel 6a, Expo app, 2026-10-04). The model resolved "he" | ||
| * from the history and answered from memory, uncited: the weaker case for the bounty (Qwen3-4B 0.49 from memory vs 0.61 | ||
| * with passages). From the conversation's previous question: | ||
| * - a question that points back ("he", "it", "isso"…) is searched together with the previous question, and with the | ||
| * previous question alone when that finds nothing; | ||
| * - a short question with no name of its own is searched as written first, and with the previous question only when | ||
| * that finds nothing. A question that finds sources keeps them, and one with a name never borrows another topic. | ||
| */ | ||
| object FollowUp { | ||
| /** Words that only make sense with an earlier subject (English and Portuguese). */ | ||
| private val POINTS_BACK = Regex( | ||
| "(?:^|[^\\p{L}])(he|she|it|its|they|them|their|theirs|him|his|her|hers|this|that|these|those|there|ele|ela|eles|elas|dele|dela|deles|delas|isso|isto|aquilo|nele|nela|esse|essa|esses|essas)(?![\\p{L}])", | ||
| RegexOption.IGNORE_CASE, | ||
| ) | ||
|
|
||
| /** At most this many content words for a question to borrow the previous one's topic when it finds nothing. */ | ||
| const val SHORT_FOLLOW_UP_TERMS = 3 | ||
|
|
||
| /** | ||
| * [direct]: search this instead of the question (it points back); [previous]: with [direct], the previous question | ||
| * alone when the combined text finds nothing; [fallback]: search this only if the question alone found nothing. | ||
| */ | ||
| data class Plan(val direct: String? = null, val previous: String? = null, val fallback: String? = null) | ||
|
|
||
| /** The chunks found, and the text that found them (the caller compresses against the same words). */ | ||
| data class Found<T>(val chunks: List<T>, val searchedFor: String) | ||
|
|
||
| private val SPACES = Regex("${Js.WS}+") | ||
| private val OPENING_MARK = Regex("^[\"'(«“]") | ||
| private val CAPITAL_OR_DIGIT = Regex("^[\\p{Lu}0-9]") | ||
|
|
||
| /** The latest earlier question, never the current one (a history that already holds it would make it its own subject). */ | ||
| private fun previousQuestion(query: String, history: ConversationHistory?): String? { | ||
| val turns = history?.turns.orEmpty() | ||
| val current = Js.trim(query) | ||
| for (i in turns.indices.reversed()) { | ||
| val text = Js.trim(turns[i].text) | ||
| if (turns[i].role == "user" && text.isNotEmpty() && text != current) return text | ||
| } | ||
| return null | ||
| } | ||
|
|
||
| /** A capitalized word after the first one: a name the question brings itself ("Australia", "EIP-1559"). */ | ||
| private fun namesItsOwnSubject(query: String): Boolean = | ||
| Js.trim(query).split(SPACES).drop(1).any { CAPITAL_OR_DIGIT.containsMatchIn(OPENING_MARK.replaceFirst(it, "")) } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Recognize a named subject in the first word.
🤖 Prompt for AI Agents |
||
|
|
||
| fun plan(query: String, history: ConversationHistory?): Plan { | ||
| val previous = previousQuestion(query, history) ?: return Plan() | ||
| val combined = "$previous ${Js.trim(query)}" | ||
| if (POINTS_BACK.containsMatchIn(query)) return Plan(direct = combined, previous = previous) | ||
| if (Compress.tokenizeTerms(query).size <= SHORT_FOLLOW_UP_TERMS && !namesItsOwnSubject(query)) return Plan(fallback = combined) | ||
| return Plan() | ||
| } | ||
|
|
||
| /** Runs [find] (the text to search → its chunks) with the follow-up rule. */ | ||
| fun <T> search(query: String, history: ConversationHistory?, find: (String) -> List<T>): Found<T> { | ||
| val p = plan(query, history) | ||
| if (p.direct != null) { | ||
| val chunks = find(p.direct) | ||
| if (chunks.isNotEmpty() || p.previous == null) return Found(chunks, p.direct) | ||
| // Found by the earlier subject; compressed against the combined words, so the sentences closest to the | ||
| // follow-up ("publish") are the ones kept. | ||
| return Found(find(p.previous), p.direct) | ||
| } | ||
| val chunks = find(query) | ||
| if (chunks.isNotEmpty() || p.fallback == null) return Found(chunks, query) | ||
| return Found(find(p.fallback), p.fallback) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| package team.sopa.boar.engine.answer | ||
|
|
||
| import kotlinx.serialization.json.Json | ||
| import kotlinx.serialization.json.JsonElement | ||
| import kotlinx.serialization.json.JsonNull | ||
| import kotlinx.serialization.json.jsonArray | ||
| import kotlinx.serialization.json.jsonObject | ||
| import kotlinx.serialization.json.jsonPrimitive | ||
| import org.junit.Assert.assertEquals | ||
| import org.junit.Assert.assertTrue | ||
| import org.junit.Test | ||
| import team.sopa.boar.engine.rag.ConversationHistory | ||
| import team.sopa.boar.engine.rag.ConversationTurn | ||
| import java.io.File | ||
|
|
||
| /** The follow-up search rule must match src/routing/followUp.ts exactly (scripts/export-native-golden-followup.mjs). */ | ||
| class FollowUpGoldenTest { | ||
| private val g = Json.parseToJsonElement(File("src/test/resources/golden/followup.json").readText()).jsonObject | ||
| private val JsonElement.str get() = if (this is JsonNull) null else jsonPrimitive.content | ||
| private fun history(e: JsonElement?): ConversationHistory? = | ||
| if (e == null || e is JsonNull) null | ||
| else ConversationHistory(turns = e.jsonObject["turns"]!!.jsonArray.map { it.jsonObject.let { t -> ConversationTurn(t["role"]!!.str!!, t["text"]!!.str!!) } }) | ||
|
|
||
| @Test fun `search plan`() { | ||
| val cases = g["plans"]!!.jsonArray.map { it.jsonObject } | ||
| assertTrue(cases.size > 300) | ||
| for (c in cases) { | ||
| val q = c["query"]!!.str!! | ||
| val p = c["plan"]!!.jsonObject | ||
| val want = FollowUp.Plan(p["direct"]?.str, p["previous"]?.str, p["fallback"]?.str) | ||
| assertEquals("\"$q\" after ${c["history"]}", want, FollowUp.plan(q, history(c["history"]))) | ||
| } | ||
| } | ||
|
|
||
| @Test fun `searches run in the same order and keep the same words`() { | ||
| val cases = g["runs"]!!.jsonArray.map { it.jsonObject } | ||
| assertTrue(cases.size > 700) | ||
| for (c in cases) { | ||
| val q = c["query"]!!.str!! | ||
| val finds = c["finds"]!!.jsonArray.map { it.str!! }.toSet() | ||
| val calls = ArrayList<String>() | ||
| val r = FollowUp.search(q, history(c["history"])) { text -> calls += text; if (text in finds) listOf("hit:$text") else emptyList() } | ||
| val where = "\"$q\" finds $finds" | ||
| assertEquals(where, c["calls"]!!.jsonArray.map { it.str }, calls) | ||
| assertEquals(where, c["chunks"]!!.jsonArray.map { it.str }, r.chunks) | ||
| assertEquals(where, c["searchedFor"]!!.str, r.searchedFor) | ||
| } | ||
| } | ||
| } |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| // Golden outputs of the follow-up search rule (src/routing/followUp.ts) for the native port's parity test. The | ||
| // Kotlin port (FollowUp.kt) must reproduce them exactly. | ||
| // node scripts/export-native-golden-followup.mjs | ||
| import { readFileSync, writeFileSync, mkdirSync } from "node:fs"; | ||
| import { dirname, join } from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
| import { registerHooks } from "node:module"; | ||
|
|
||
| registerHooks({ | ||
| resolve(specifier, context, next) { | ||
| try { | ||
| return next(specifier, context); | ||
| } catch (e) { | ||
| if (specifier.startsWith(".") && !/\.[cm]?[jt]sx?$|\.json$/.test(specifier)) return next(`${specifier}.ts`, context); | ||
| throw e; | ||
| } | ||
| }, | ||
| }); | ||
|
|
||
| const root = join(dirname(fileURLToPath(import.meta.url)), ".."); | ||
| const out = join(root, "android-native/engine/src/test/resources/golden"); | ||
| const { followUpSearch, searchWithFollowUp } = await import(join(root, "src/routing/followUp.ts")); | ||
|
|
||
| // Real questions: every eval question the native app ships, then the conversation runs' questions (#72), then edge cases. | ||
| const evalSets = JSON.parse(readFileSync(join(root, "android-native/app/src/main/assets/boar-data/eval-set.json"), "utf8")); | ||
| const queries = []; | ||
| (function walk(v) { | ||
| if (Array.isArray(v)) v.forEach(walk); | ||
| else if (v && typeof v === "object") for (const [k, x] of Object.entries(v)) (k === "query" && typeof x === "string" ? queries.push(x) : walk(x)); | ||
| })(evalSets); | ||
| if (queries.length < 20) throw new Error(`only ${queries.length} eval questions found`); | ||
| const conversation = [ | ||
| "How do vaccines work?", "What are the side effects?", "Who proposed the theory of evolution by natural selection?", | ||
| "What did he publish?", "Compare the French Revolution and the Industrial Revolution.", "What is the capital of Australia?", | ||
| ]; | ||
| const edge = [ | ||
| "", " ", "Is it safe?", "What's its capital?", "Hehe", "theory of everything", "THEY said so", "the end", "There?", | ||
| "Tell me about Australia", "\"Darwin\" books?", "«Quem» é ele?", "E isso funciona?", "Quais são os efeitos colaterais?", | ||
| "O que ele publicou?", "Who is she?", "why", "And then?", "How about EIP-1559?", "More details", "Itself?", "it's fine", | ||
| "Him", "Write 3 examples", "what about 2024", "and the side effects", "Ela é casada?", "Ícone?", "Ozone layer", " that one ", | ||
| ]; | ||
| const all = [...new Set([...queries, ...conversation, ...edge])]; | ||
|
|
||
| // Histories: none, the previous question, a history already holding the current question, assistant-only, blank turns. | ||
| const histories = (q, i) => { | ||
| const prev = all[(i * 7 + 3) % all.length]; | ||
| return [ | ||
| undefined, | ||
| { turns: [] }, | ||
| { turns: [{ role: "user", text: prev }, { role: "assistant", text: "An answer." }] }, | ||
| { turns: [{ role: "user", text: prev }, { role: "assistant", text: "A." }, { role: "user", text: q }] }, | ||
| { turns: [{ role: "assistant", text: "Hello, ask me anything." }] }, | ||
| { turns: [{ role: "user", text: " " }, { role: "user", text: ` ${prev} ` }, { role: "assistant", text: "B." }] }, | ||
| ]; | ||
| }; | ||
|
|
||
| const plans = []; | ||
| const runs = []; | ||
| for (const [i, q] of all.entries()) { | ||
| for (const history of histories(q, i)) { | ||
| const plan = followUpSearch(q, history); | ||
| plans.push({ query: q, history: history ?? null, plan }); | ||
| // searchWithFollowUp with a stand-in search that finds something only for the texts in `finds`. | ||
| const texts = [q, plan.direct, plan.previous, plan.fallback].filter(Boolean); | ||
| for (const finds of [[], ...texts.map((t) => [t])]) { | ||
| const calls = []; | ||
| const r = await searchWithFollowUp(q, history, async (text) => { | ||
| calls.push(text); | ||
| return finds.includes(text) ? [`hit:${text}`] : []; | ||
| }); | ||
| runs.push({ query: q, history: history ?? null, finds, calls, chunks: r.chunks, searchedFor: r.searchedFor }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| mkdirSync(out, { recursive: true }); | ||
| writeFileSync(join(out, "followup.json"), JSON.stringify({ plans, runs })); | ||
| const kinds = plans.reduce((m, p) => ((m[p.plan.direct ? "direct" : p.plan.fallback ? "fallback" : "none"]++), m), { direct: 0, fallback: 0, none: 0 }); | ||
| console.log(`followup.json: ${all.length} questions, ${plans.length} plans (${JSON.stringify(kinds)}), ${runs.length} searches`); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { followUpSearch, searchWithFollowUp } from "./followUp"; | ||
|
|
||
| const after = (question: string) => ({ | ||
| turns: [ | ||
| { role: "user" as const, text: question }, | ||
| { role: "assistant" as const, text: "An answer about it." }, | ||
| ], | ||
| }); | ||
|
|
||
| describe("followUpSearch", () => { | ||
| it("searches a question that points back together with the previous question", () => { | ||
| expect(followUpSearch("What did he publish?", after("Who proposed the theory of evolution by natural selection?"))).toEqual({ | ||
| direct: "Who proposed the theory of evolution by natural selection? What did he publish?", | ||
| previous: "Who proposed the theory of evolution by natural selection?", | ||
| }); | ||
| }); | ||
|
|
||
| it("does the same in Portuguese", () => { | ||
| expect(followUpSearch("O que ele publicou?", after("Quem propôs a teoria da evolução?")).direct).toContain("Quem propôs"); | ||
| }); | ||
|
|
||
| it("borrows the previous topic for a short question only as a fallback", () => { | ||
| expect(followUpSearch("What are the side effects?", after("How do vaccines work?"))).toEqual({ | ||
| fallback: "How do vaccines work? What are the side effects?", | ||
| }); | ||
| }); | ||
|
|
||
| it("never borrows for a question that names its own subject", () => { | ||
| expect(followUpSearch("What is the capital of Australia?", after("Compare the French Revolution and the Industrial Revolution."))).toEqual({}); | ||
| }); | ||
|
|
||
| it("leaves a longer self-contained question alone", () => { | ||
| expect(followUpSearch("How does the immune system respond to a new virus infection?", after("How do vaccines work?"))).toEqual({}); | ||
| }); | ||
|
|
||
| it("never takes the current question as the previous one", () => { | ||
| const history = { | ||
| turns: [ | ||
| { role: "user" as const, text: "Who proposed the theory of evolution?" }, | ||
| { role: "assistant" as const, text: "Charles Darwin." }, | ||
| { role: "user" as const, text: "What did he publish?" }, | ||
| ], | ||
| }; | ||
| expect(followUpSearch("What did he publish?", history).direct).toBe("Who proposed the theory of evolution? What did he publish?"); | ||
| }); | ||
|
|
||
| it("does nothing without an earlier question", () => { | ||
| expect(followUpSearch("What did he publish?", { turns: [] })).toEqual({}); | ||
| expect(followUpSearch("What did he publish?")).toEqual({}); | ||
| }); | ||
|
|
||
| it("does not read 'the' or 'there' inside other words as pointing back", () => { | ||
| expect(followUpSearch("How does photosynthesis work in theory?", after("How do vaccines work?")).direct).toBeUndefined(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("searchWithFollowUp", () => { | ||
| const history = after("How do vaccines work?"); | ||
|
|
||
| it("keeps what the question alone found", async () => { | ||
| const seen: string[] = []; | ||
| const r = await searchWithFollowUp("What are the side effects?", history, async (t) => (seen.push(t), ["chunk"])); | ||
| expect(r).toEqual({ chunks: ["chunk"], searchedFor: "What are the side effects?" }); | ||
| expect(seen).toHaveLength(1); | ||
| }); | ||
|
|
||
| it("retries with the previous question when the short one finds nothing", async () => { | ||
| const seen: string[] = []; | ||
| const r = await searchWithFollowUp("What are the side effects?", history, async (t) => (seen.push(t), t.includes("vaccines") ? ["vaccine side effects"] : [])); | ||
| expect(r.chunks).toEqual(["vaccine side effects"]); | ||
| expect(r.searchedFor).toBe("How do vaccines work? What are the side effects?"); | ||
| expect(seen).toEqual(["What are the side effects?", "How do vaccines work? What are the side effects?"]); | ||
| }); | ||
|
|
||
| it("searches a question that points back combined, once, when that finds sources", async () => { | ||
| const seen: string[] = []; | ||
| const r = await searchWithFollowUp("What did he publish?", after("Who proposed evolution?"), async (t) => (seen.push(t), ["darwin"])); | ||
| expect(seen).toEqual(["Who proposed evolution? What did he publish?"]); | ||
| expect(r.searchedFor).toBe("Who proposed evolution? What did he publish?"); | ||
| }); | ||
|
|
||
| it("falls back to the previous question alone, still compressing against the combined words", async () => { | ||
| const seen: string[] = []; | ||
| const r = await searchWithFollowUp("What did he publish?", after("Who proposed evolution?"), async (t) => | ||
| (seen.push(t), t === "Who proposed evolution?" ? ["Evolution article"] : []) | ||
| ); | ||
| expect(seen).toEqual(["Who proposed evolution? What did he publish?", "Who proposed evolution?"]); | ||
| expect(r).toEqual({ chunks: ["Evolution article"], searchedFor: "Who proposed evolution? What did he publish?" }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: rferrari/boar-app
Length of output: 5286
🏁 Script executed:
Repository: rferrari/boar-app
Length of output: 4718
🏁 Script executed:
Repository: rferrari/boar-app
Length of output: 10503
Distinguish existential and locative uses of
there.POINTS_BACKmatches barethere, so “Is there an effective treatment?” enters the back-reference path. If the combined search returns no chunks,FollowUp.searchthen searches the unrelated previous question.Do not replace
therewith onlythere\?. That change would preserve “There?” and “What happened there?”, but it would stop matching the valid locative reference “Were they there last week?”. Add an existential-context distinction to the matcher. Keep the locative cases as back-references, and leavenamesItsOwnSubjectunchanged. Mirror the distinction insrc/routing/followUp.tsand add the cases to the generated parity fixture.🤖 Prompt for AI Agents