-
Notifications
You must be signed in to change notification settings - Fork 91
feat(#6645): use AND semantics for docs site multi-word search #6799
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
base: main
Are you sure you want to change the base?
Changes from all commits
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,182 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { filterByPhrases, parseSearchQuery, textContainsPhrases } from "./searchQuery"; | ||
|
|
||
| describe("parseSearchQuery", () => { | ||
| it("returns the raw query and no phrases when there are no quotes", () => { | ||
| expect(parseSearchQuery("eval scenario")).toEqual({ | ||
| query: "eval scenario", | ||
| phrases: [], | ||
| }); | ||
| }); | ||
|
|
||
| it("extracts a single quoted phrase", () => { | ||
| expect(parseSearchQuery('"eval scenario"')).toEqual({ | ||
| query: "eval scenario", | ||
| phrases: ["eval scenario"], | ||
| }); | ||
| }); | ||
|
|
||
| it("extracts a phrase surrounded by unquoted terms", () => { | ||
| expect(parseSearchQuery('harness "eval scenario" config')).toEqual({ | ||
| query: "harness eval scenario config", | ||
| phrases: ["eval scenario"], | ||
| }); | ||
| }); | ||
|
|
||
| it("extracts multiple quoted phrases", () => { | ||
| expect(parseSearchQuery('"eval scenario" "harness config"')).toEqual({ | ||
| query: "eval scenario harness config", | ||
| phrases: ["eval scenario", "harness config"], | ||
| }); | ||
| }); | ||
|
|
||
| it("ignores empty quoted strings", () => { | ||
| expect(parseSearchQuery('foo "" bar')).toEqual({ | ||
| query: 'foo "" bar', | ||
| phrases: [], | ||
| }); | ||
| }); | ||
|
|
||
| it("treats unmatched quotes as literal characters", () => { | ||
| expect(parseSearchQuery('"eval scenario')).toEqual({ | ||
| query: '"eval scenario', | ||
| phrases: [], | ||
| }); | ||
| }); | ||
|
|
||
| it("handles a single unquoted word", () => { | ||
| expect(parseSearchQuery("harness")).toEqual({ | ||
| query: "harness", | ||
| phrases: [], | ||
| }); | ||
| }); | ||
|
|
||
| it("handles an empty string", () => { | ||
| expect(parseSearchQuery("")).toEqual({ | ||
| query: "", | ||
| phrases: [], | ||
| }); | ||
| }); | ||
|
|
||
| it("trims whitespace inside quoted phrases", () => { | ||
| expect(parseSearchQuery('" eval scenario "')).toEqual({ | ||
| query: "eval scenario", | ||
| phrases: ["eval scenario"], | ||
| }); | ||
| }); | ||
|
|
||
| it("handles a quoted single word", () => { | ||
| expect(parseSearchQuery('"harness"')).toEqual({ | ||
| query: "harness", | ||
| phrases: ["harness"], | ||
| }); | ||
| }); | ||
|
|
||
| it("separates adjacent quoted phrases without whitespace", () => { | ||
| expect(parseSearchQuery('"foo bar""baz qux"')).toEqual({ | ||
| query: "foo bar baz qux", | ||
| phrases: ["foo bar", "baz qux"], | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe("textContainsPhrases", () => { | ||
| it("returns true when there are no phrases", () => { | ||
| expect(textContainsPhrases("any text", [])).toBe(true); | ||
| }); | ||
|
|
||
| it("returns true when the phrase appears in the text", () => { | ||
| expect( | ||
| textContainsPhrases("The eval scenario runner starts here.", ["eval scenario"]), | ||
| ).toBe(true); | ||
| }); | ||
|
|
||
| it("returns false when the phrase does not appear adjacent", () => { | ||
| expect( | ||
| textContainsPhrases("The eval of each scenario is different.", ["eval scenario"]), | ||
| ).toBe(false); | ||
| }); | ||
|
|
||
| it("matches case-insensitively", () => { | ||
| expect(textContainsPhrases("The Eval Scenario runner.", ["eval scenario"])).toBe(true); | ||
| }); | ||
|
|
||
| it("requires all phrases to match", () => { | ||
| expect( | ||
| textContainsPhrases("eval scenario and harness config details", [ | ||
| "eval scenario", | ||
| "harness config", | ||
| ]), | ||
| ).toBe(true); | ||
|
|
||
| expect( | ||
| textContainsPhrases("eval scenario but no harness here", [ | ||
| "eval scenario", | ||
| "harness config", | ||
| ]), | ||
| ).toBe(false); | ||
| }); | ||
|
|
||
| it("returns true for a single-word phrase that appears in text", () => { | ||
| expect(textContainsPhrases("the harness is ready", ["harness"])).toBe(true); | ||
| }); | ||
|
|
||
| it("returns false when text is empty and phrases are not", () => { | ||
| expect(textContainsPhrases("", ["eval scenario"])).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe("filterByPhrases", () => { | ||
| const results = [ | ||
| { title: "Getting Started", titles: ["Guides"], text: "The eval scenario runner starts here." }, | ||
| { title: "Config Reference", titles: ["Guides"], text: "Harness config and eval options." }, | ||
| { title: "Eval Overview", titles: ["Concepts"], text: "Each scenario runs independently." }, | ||
| ]; | ||
|
|
||
| it("returns all results when there are no phrases", () => { | ||
| expect(filterByPhrases(results, [])).toEqual(results); | ||
| }); | ||
|
|
||
| it("keeps only results whose text contains the exact phrase", () => { | ||
| const filtered = filterByPhrases(results, ["eval scenario"]); | ||
| expect(filtered).toHaveLength(1); | ||
| expect(filtered[0].title).toBe("Getting Started"); | ||
| }); | ||
|
|
||
| it("requires all phrases to match", () => { | ||
| const filtered = filterByPhrases(results, ["eval scenario", "harness config"]); | ||
| expect(filtered).toHaveLength(0); | ||
| }); | ||
|
|
||
| it("matches phrases against titles as well as text", () => { | ||
| const filtered = filterByPhrases(results, ["eval overview"]); | ||
| expect(filtered).toHaveLength(1); | ||
| expect(filtered[0].title).toBe("Eval Overview"); | ||
| }); | ||
|
|
||
| it("matches phrases spanning title and text content", () => { | ||
| const filtered = filterByPhrases( | ||
| [{ title: "Scenario", titles: ["Eval"], text: "runner starts here" }], | ||
| ["eval scenario"], | ||
| ); | ||
| expect(filtered).toHaveLength(1); | ||
| }); | ||
|
|
||
| it("keeps results with no text content (graceful degradation)", () => { | ||
| const sparse = [{ title: "", titles: [], text: undefined as unknown as string }]; | ||
| expect(filterByPhrases(sparse, ["anything"])).toHaveLength(1); | ||
| }); | ||
|
|
||
| it("is case-insensitive", () => { | ||
| const filtered = filterByPhrases(results, ["EVAL SCENARIO"]); | ||
| expect(filtered).toHaveLength(1); | ||
| expect(filtered[0].title).toBe("Getting Started"); | ||
| }); | ||
|
|
||
| it("works end-to-end with parseSearchQuery", () => { | ||
| const { phrases } = parseSearchQuery('"eval scenario" guide'); | ||
| const filtered = filterByPhrases(results, phrases); | ||
| expect(filtered).toHaveLength(1); | ||
| expect(filtered[0].title).toBe("Getting Started"); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| export interface ParsedQuery { | ||
| /** The query string with double-quoted phrases stripped of their quotes. */ | ||
| query: string; | ||
| /** Exact phrases extracted from double-quoted substrings. */ | ||
| phrases: string[]; | ||
| } | ||
|
|
||
| /** | ||
| * Parses a search query string, extracting double-quoted phrases. | ||
| * | ||
| * Returns the cleaned query (quotes removed, words kept for AND matching) | ||
| * and an array of exact phrases that must appear adjacent in results. | ||
| * | ||
| * Examples: | ||
| * parseSearchQuery('eval scenario') | ||
| * => { query: 'eval scenario', phrases: [] } | ||
| * parseSearchQuery('"eval scenario"') | ||
| * => { query: 'eval scenario', phrases: ['eval scenario'] } | ||
| * parseSearchQuery('harness "eval scenario" config') | ||
| * => { query: 'harness eval scenario config', phrases: ['eval scenario'] } | ||
| */ | ||
| export function parseSearchQuery(raw: string): ParsedQuery { | ||
| const phrases: string[] = []; | ||
| const query = raw.replace(/"([^"]+)"/g, (_match, phrase: string) => { | ||
| const trimmed = phrase.trim(); | ||
|
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. [low] edge-case When two quoted phrases appear without whitespace between them (e.g., Suggested fix: Pad the replacement with a space: |
||
| if (trimmed) phrases.push(trimmed); | ||
| // Pad with spaces so adjacent quoted phrases don't fuse tokens | ||
| // (e.g. "foo bar""baz qux" → "foo bar baz qux", not "foo barbaz qux"). | ||
| return " " + trimmed + " "; | ||
| }); | ||
| return { query: query.replace(/\s+/g, " ").trim(), phrases }; | ||
| } | ||
|
|
||
| /** | ||
| * Returns true when every phrase appears as a case-insensitive | ||
| * substring in the given text. | ||
| */ | ||
| export function textContainsPhrases(text: string, phrases: string[]): boolean { | ||
| if (phrases.length === 0) return true; | ||
| const lower = text.toLowerCase(); | ||
| return phrases.every((p) => lower.includes(p.toLowerCase())); | ||
| } | ||
|
|
||
| /** | ||
| * Filters search results by requiring every phrase to appear in the | ||
| * result's concatenated title + text content. Results with no text | ||
| * content are kept (graceful degradation). | ||
| */ | ||
| export function filterByPhrases< | ||
| T extends { text?: string; title?: string; titles?: string[] }, | ||
| >(results: T[], phrases: string[]): T[] { | ||
| if (phrases.length === 0) return results; | ||
| return results.filter((r) => { | ||
| const content = [...(r.titles || []), r.title || "", r.text || ""].join(" "); | ||
| if (!content.trim()) return true; | ||
| return textContainsPhrases(content, phrases); | ||
| }); | ||
| } | ||
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.
[low] type-annotation-style
The variable
searchOptsis typed asRecord<string, unknown>. A narrower type like{ combineWith: string; filter?: (r: SearchResult) => boolean }would be more precise and catch typos at compile time.Suggested fix: Consider typing
searchOptsmore precisely:const searchOpts: { combineWith: string; filter?: (r: SearchResult) => boolean } = { combineWith: "AND" };