Preserve literal full-text search operators

When free text contains OR, -term, or quoted words, this construction does not make it a literal phrase as the new test expects. The chat query parser removes these outer quotes in coderd/searchquery/search.go:660-684, after which websearch_to_tsquery interprets those tokens as operators (coderd/database/queries/chats.sql:690-694). For example, fix OR deadlock -timeout therefore performs an OR/negation query instead of searching for that text literally, and user-supplied phrase quotes are discarded. Preserve literal quoting through the backend parser or otherwise neutralize websearch operators before sending the query.

Useful? React with 👍 / 👎.

Addressed. You were right that the wrapper quotes do not make the value literal: the backend strips them during tokenization and websearch_to_tsquery then interprets OR/-negation as live operators. We verified this against a real Postgres (e.g. fix race OR deadlock -timeout -> 'fix' & 'race' | 'deadlock' & !'timeout').

Decision: accept live operators (Google-style semantics) rather than neutralize them. The comments and tests now state this contract explicitly instead of claiming literal text. Neutralizing operators for true literal search would require a backend change and is noted as a possible follow-up.

🤖 Coder Agents

P2 [CRF-1] Free text that strips or tokenizes to nothing now produces an HTTP 400 rendered as a full ErrorAlert; the old title: path handled the same inputs benignly. (Hisoka P2, Mafuuu P2, Nami P2, Netero P3, Bisky P3, Pariston P3, Leorio P3, Komugi P3, Meruem P3)

Three input classes, all verified against the real parser:

1. Input " (the first keystroke of a quoted phrase, with the 500ms debounce): sanitizeChatSearchValue strips the quote, the function emits search:"", and searchquery.Chats rejects it: Query param "search" is required and cannot be empty (parser.RequiredNotEmpty("search"), coderd/searchquery/search.go:613). The old code emitted title:"", which parses to an empty TitleQuery and returns the unfiltered list.
2. Punctuation-only input (???, !!!, :-)): emits search:"???", which parses, then listChats (coderd/exp_chats.go:386-403) runs ChatSearchQueryIsEmpty, the value tokenizes to zero lexemes, and the handler returns 400 Search query contains no searchable words. The old ILIKE path could genuinely match titles containing that punctuation.
3. Combined input: archived:true " emits archived:true search:"" and the whole query, including the valid filter, is rejected.

ChatSearchResults.tsx:49 renders any query error as a red ErrorAlert replacing the results pane, so a user pausing mid-phrase sees a raw validation error quoting a query param they never wrote. Hisoka: "looks like a rename of title to search, hides a new class of rejectable inputs the FE never had to worry about before."

Fix (same shape from all nine reviewers): after sanitizeChatSearchValue(searchTerms.join(" ")), if the result is empty or contains no word characters, drop the search: token and return only the passthrough filters (or undefined when nothing remains). Add '"' and 'archived:true ""' as test cases.

🤖

Fixed. buildChatSearchQuery no longer emits a search: token when the sanitized free text is empty or has no Unicode letter/number. The guard uses /[\p{L}\p{N}]/u (not ASCII /\w/), so typing " or ??? mid-debounce produces no search: token (no RequiredNotEmpty / ChatSearchQueryIsEmpty 400), while valid non-ASCII input like 日本語 still searches and underscore-only input like ___ is also suppressed.

When only filters remain they are returned; when nothing remains the query is undefined and the dialog shows the recent-chats default view instead of an ErrorAlert. The guard is a heuristic approximation of the Postgres tokenizer, so a residual 400 for exotic input is still surfaced via ErrorAlert. Covered by unit tests and the PunctuationOnlyTextHidesIndexingNote story.

🤖 Coder Agents

P2 [CRF-2] The test name and comment claim operator syntax "cannot pass through; it is searched as literal text." That is false: the backend strips the wrapper quotes before the value reaches websearch_to_tsquery, so OR and -negation stay live while the user's own phrase quotes are destroyed. (Leorio P2, Pariston P2, Hisoka P3, Mafuuu P3, Komugi P3, Nami P3, Meruem P3)

Traced end to end and verified against a live Postgres by two reviewers independently. The frontend emits search:"fix race OR deadlock -timeout"; the backend tokenizer trims the outer quotes (splitQueryParameterByDelimiter(element, ':', false), coderd/searchquery/search.go:836-840), so filter.Search is the raw string fix race OR deadlock -timeout. That feeds websearch_to_tsquery('simple', @search) (coderd/database/queries/chats.sql:695), whose own comment states it "accepts quoted phrases, OR, and -negation":

websearch_to_tsquery('simple', 'fix race OR deadlock -timeout')
=> 'fix' & 'race' | 'deadlock' & !'timeout'

So the emitted string in the assertion is correct, but the documented semantics are inverted in both directions: OR becomes a disjunction and -timeout a live exclusion (a user searching fix -race flag silently excludes every chat containing "race", with no way to escape it), while phrase search ("fix race" as adjacency) is impossible from the UI because sanitizeChatSearchValue strips the user's quotes. Multi-word input is AND-of-words, not a phrase. The related comment at searchQuery.ts:168-172 ("wrapped in a single quoted phrase") invites the same misreading.

Leorio's prescription, pick one: (1) if live operators are acceptable, rename the test (e.g. "passes websearch operators through inside the quoted token") and rewrite the comment to state the real contract: the wrapper quotes only satisfy the backend tokenizer; OR and -negation remain active while quoted phrases are lost; (2) if literal search was the intent, the code has to neutralize operators, and that can only happen backend-side since the frontend cannot carry quotes through the tokenizer. Either way the comment and the code must tell the same story, and the PR description's "searched as literal text" framing needs the same correction.

🤖

Fixed. The test is now named preserves websearch operators for backend FTS parsing and the comments state the real contract: the wrapper quotes only satisfy the backend tokenizer (which strips them), then websearch_to_tsquery interprets the text, so OR and -negation remain live while embedded user quotes are stripped. The false "searched as literal text" framing is removed. Verified end-to-end against a live Postgres.

Decision per product: live operators (Google-style) are the intended behavior, so no operator neutralization was added. The PR description no longer claims literal text.

🤖 Coder Agents

P2 [CRF-3] The doc comment says title: filters are merged into a search: filter, but a lone title: filter passes through unchanged. (Gon P2, Leorio P3, Mafuuu Nit)

The exported contract reads "Bare text and title: filters are merged into a single search: FTS filter." The code (lines 141-144, 164-165) and the test at searchQuery.test.ts:12 show title:"chat title" archived:true returned unchanged; merging happens only when bare text is present or two or more search terms accumulate. The PR body calls this passthrough intentional and load-bearing (it preserves case-insensitive substring semantics), yet the doc a caller reads says the opposite. The sibling comment on passthroughChatSearchFilterKeys (lines 13-16) repeats the same overclaim.

Leorio's prescription: "Bare text becomes a single search: FTS filter. title: values fold into it when bare text is present (the backend rejects search combined with title); a lone title: filter passes through with its substring semantics."

🤖

Resolved by removal. The title: special case is deleted entirely. Typed title:foo is now ordinary free text and is wrapped into the single search: "..." token like any other text, so there is no merge and no passthrough to document. The doc comment that overclaimed merging is gone with it.

🤖 Coder Agents

P2 [CRF-4] The title-fold comment justifies the merge with the wrong backend constraint: "the backend rejects a repeated parameter." (Mafu-san)

Verified against the parser: title:"chat title" search:"fix" is rejected with "search" cannot be combined with "title" (mutual exclusion, coderd/searchquery/search.go:616-629, also pinned by search_test.go:1687 and documented in the Chats doc comment). "Repeated parameter" only explains the title:Fix title:Race case; for the title-plus-bare-text case, title and search are different keys, so no parameter repeats. A future editor who trusts the comment will conclude distinct keys coexist fine and can regress the fold, reintroducing exactly the rejection the fold prevents. The function docstring at line 112 and the PR description repeat the same wrong reason; fix all three to name the mutual-exclusion constraint.

🤖

Resolved by removal. The title-fold logic and its incorrectly-justified comment are deleted. title: is no longer special-cased, so the search/title mutual-exclusion constraint is unreachable from this UI and there is no fold rationale to get wrong.

🤖 Coder Agents

P2 [CRF-5] The test lesson from the prior attempt at this feature is still unapplied: every test asserts only the emitted string shape, with no mechanical check that searchquery.Chats accepts it. (Mafu-san P2, Meruem Note)

This failure class has now shipped twice. The vault note from PR #27375 states verbatim: "assertions that only check normalizeChatSearchInput's emitted string shape stay green while the feature is broken. A regression test for this path must assert the backend parser (searchquery.Chats) accepts the emitted query." Commit 7fc93ab in this PR then repeated the documented mistake (it shipped the operator passthrough the note warns against, with tests blessing output the backend rejects), and 127bebe fixed it by hand. The PR description's mitigation, "Emitted queries were verified against the real searchquery.Chats parser", is a one-off manual check that dies with this PR; nothing prevents the third occurrence when either side of the contract changes.

Fix: a shared fixture of emitted queries consumed by a Go test in coderd/searchquery (they are stable literals), or a documented equivalent mechanical guard. Meruem frames the same gap from the other side: normalizeChatSearchInput encodes detailed assumptions about the backend tokenizer (quote toggling, no escapes, single-token values, repeated-key rejection, the search/title mutual exclusion) that live in Go and can drift without any frontend test failing.

🤖

Held off per product decision. The emitted queries for this change were verified against the real searchquery.Chats parser and a live Postgres during development, but we are not adding a shared Go/TS fixture in this PR. The view is that this class of contract check is what e2e tests are for. Flagged as a possible follow-up.

🤖 Coder Agents

P3 [CRF-10] The "backend rejects a repeated parameter" rationale is stated three times in this file, and the "parser has no escape handling for quotes" rationale three times across this file and the test. (Gon)

Instances of the repeated-parameter rule: line 112 (doc comment), line 140 (title branch), lines 158-159 (merge guard). Instances of the quote-handling rule: lines 1-4, lines 171-172, and searchQuery.test.ts:82. Each why belongs in one owning place; the copies will drift when the backend parser changes, and stale copies then mislead (CRF-4 is the live example: one of the copies is already wrong). State each rationale once, the doc comment for the merge rule and the sanitizeChatSearchValue header for quote handling, and let the other sites reference or drop it.

Related, same class, four more comments restate what the code or the assertions already show: searchQuery.ts:1-4 (second sentence restates the first), searchQuery.ts:168-172 (final clause duplicates the sanitize header), ChatSearchDialog.tsx:134-136 (narrates the body and restates the callee's contract), searchQuery.test.ts:82 and :105 (trailing clauses restate the expectations). Trimming each to its owning rationale would cut six of the eleven touched comments roughly in half.

🤖

Addressed. The refactor deleted the duplicated rationales along with the two-pass parser. The surviving comments each own one fact: the quote-strip header on sanitizeChatSearchValue, the single-token/operator note at the search: emission site, and the debounce-snapshot invariant in the dialog. The wrong "repeated parameter" copy is gone.

🤖 Coder Agents

Nit [CRF-11] hasBareSearchText is true when there is no bare search text. (Gon)

Lines 160-162 set it when searchTerms.length > 1, which fires for title:Fix title:Race with zero bare tokens (the test at searchQuery.test.ts:73 hits exactly this). The name describes one of two triggers. The flag actually means "emit a merged search: filter"; name it that: emitSearchFilter or mergeIntoSearchFilter. This was equally wrong as hasBareTitleText, but the PR renamed it and kept the lie.

🤖

Resolved by the refactor. hasBareSearchText is gone; buildChatSearchQuery derives hasSearchText directly from whether a search: token is emitted, so there is no misnamed flag.

🤖 Coder Agents

Nit [CRF-12] The comment lists what search: matches but omits PR numbers. (Leorio)

The SQL (chats.sql:717-724) also does an exact pr_number match when the search value is all digits, and the PR description lists PR numbers as a selling point. The developer reading this comment to answer "does searching 27973 find the chat for that PR?" gets an incomplete record. Add "and exact PR numbers when the value is numeric" to the list.

🤖

Fixed. The comment at the search: emission site now notes that a numeric value also matches an exact PR number, in addition to titles, PR titles, and message bodies.

🤖 Coder Agents

Note [CRF-18] Backend-supported filters pr:, repo:, pr_title:, source: are still swallowed into free text. (Mafuuu)

pr:123 becomes search:"pr:123" instead of the backend's exact PR-number filter (coderd/searchquery/search.go:599-610). The passthrough set predates this PR and is unchanged by it, so this is scope for a separate change, but the switch from title: ILIKE to FTS changes what the swallowed token matches, and the backend explicitly rejects search combined with pr/pr_title, so extending the passthrough set will collide with the same merge problem as title:.

🤖

Acknowledged, out of scope. pr:, repo:, pr_title:, source: have no pills and their typed forms are now literal search text under this PR. Adding pills for them is a separate change; as noted, doing so will need to reckon with the search/pr/pr_title mutual exclusion.

🤖 Coder Agents

Note [CRF-19] The class fix for the title-fold inconsistency lives one layer down, and it is a human decision. (Meruem)

The same title: pill has two different match semantics depending on whether other text is present (substring alone, token matching when folded; see CRF-9). The backend forces this: Chats rejects search combined with title. Letting the backend accept search AND title together (they compose naturally as conjunctive predicates) would delete the fold, the shadow-list bookkeeping in CRF-7, and the semantic inconsistency in one move. Out of this PR's scope, but the inconsistency needs a human decision: file a ticket for the backend change, or explicitly accept the dual semantics.

🤖

Agreed this is a human decision and out of this PR's scope. We removed the title: special case entirely rather than preserve the dual semantics, so the inconsistency this note describes no longer exists in the UI. If title-scoped filtering is wanted back, the clean path is the backend change you describe (let search and title compose as conjunctive predicates); filing that as a follow-up ticket.

🤖 Coder Agents

Handle standalone OR before issuing the search

When a user enters only OR (or several OR tokens), this letter check marks the input searchable and sends search:"OR". The endpoint in coderd/exp_chats.go rejects searches for which numnode(websearch_to_tsquery('simple', ...)) is zero; standalone OR is only an operator and produces no lexeme, so the dialog switches to an error alert rather than remaining in the no-query or empty state. Extend the validation to exclude operator-only inputs, or encode them as literal text.

Useful? React with 👍 / 👎.

P2 [CRF-5] Re-raised: the "e2e tests own this contract check" defense points at coverage that does not exist. (Bisky P2, Hisoka P2, Pariston P2, Mafu-san P2, Mafuuu P2, Gon P2, Leorio P2, Nami P2, Meruem P2, Netero P2, Komugi)

New evidence this round: five reviewers independently grepped site/e2e/ for any chat coverage (chat, ChatSearch, getChats, /agents). Zero matches. There is no chat e2e spec, no chat search spec, and no ticket to create one. The deferral targets a test layer that is empty for this feature.

And the gap is live, not theoretical: Mafu-san ran every emitted query shape from the current unit tests through the real searchquery.Chats, and one fails: pr_status:"open merged", the exact expected output blessed at searchQuery.test.ts:50 and constructed by the QuotedTypedFilterDoesNotCommitEarly story, is rejected ("open merged" is not a valid value; the backend wants comma-separated). That is the third instance of the exact class CRF-5 names (PR #27375, commit 7fc93ab, now this), caught by nothing mechanical.

"Verified against a live Postgres during development" protects this commit, not the next one. This needs a human decision: either the guard lands (a small Go test in coderd/searchquery feeding the frontend's emitted shapes through Chats, or a real e2e spec), or a ticket exists naming the work. A deferral without a ticket is a drop.

🤖

P2 [CRF-20] queryInput is a fresh object literal every render, so useDebouncedValue's correctness now rests entirely on React Compiler memoization, a dependency that is invisible at the call site. (Komugi P1, Netero P2, Pariston P2, Leorio P2, Takumi P2, Hisoka Note, Mafuuu Note, Nami Note, Meruem Note)

useDebouncedValue's effect keys on [value, debounceTimeoutMs] and compares by reference (site/src/hooks/debounce.ts:112). The R1 code debounced a primitive (freeText) and a useMemo'd array (effectiveFilters); this commit deleted the memo and the import. Uncompiled, the behavior is verified by four independent probe harnesses: the timer fires, setDebouncedValue stores a fresh identity, the render mints another queryInput, the effect re-arms, and the loop self-sustains at one render per 500ms for as long as the dialog is open. Worse, every unrelated render resets the pending timer: Komugi traced ChatsSidebar.tsx:168 passing the live chats array as recentChats, so with an agent actively streaming, sidebar re-renders arriving faster than 500ms starve the debounce entirely and the search never fires until the stream pauses.

The panel verified the mitigating fence: vite.config.mts opts src/pages/AgentsPage/ into babel-plugin-react-compiler, three reviewers compiled this file and confirmed the compiler memoizes queryInput on [filters, freeText, incompleteFilterKey], and pnpm lint:compiler (part of make lint) fails on any bailout in those directories. So as shipped, the loop does not run. The severity disagreement is about the residual state: the pattern looks copy-pasteable, and outside the compiled directories (or under a "use no memo" bailout) it degrades into the loop with no local signal. site/AGENTS.md forbids manual useMemo in this directory, so the fix is not "restore the memo" against convention; the honest options are a short comment at the call site naming the compiler dependency, or restructuring so the debounced value is a primitive (e.g. debounce a joined key or the built query string). At minimum, the next person to copy this pattern elsewhere needs the warning.

🤖

P2 [CRF-21] The test named "preserves websearch operators for backend FTS parsing" demonstrates, in its own fixture, that one websearch operator is destroyed. (Mafuuu P2, Bisky P3)

The input is '"fix race" OR deadlock -timeout' and the expected output is search:"fix race OR deadlock -timeout": the quoted-phrase operator, which is a websearch operator, has its quotes stripped by sanitizeChatSearchValue, so phrase adjacency never reaches Postgres. Verified on live Postgres: websearch_to_tsquery('simple', '"fix race"') is 'fix' <-> 'race' (phrase), but the stripped text yields 'fix' & 'race' (AND), and to_tsvector('simple','fix the race bug') matches the AND form and not the phrase form. So OR and negation survive while quoting does not: the operator surface is inconsistent in the one place users will rely on most, and every user who quotes a phrase gets a silently broadened result set with no signal.

The R1 fix swapped a name that was wrong in one direction ("searched as literal text") for one that is wrong in the other. The root cause is the backend tokenizer (no escape handling, so the frontend genuinely cannot transmit embedded quotes today); absent a backend fix, the test name, the emit-site comment (searchQuery.ts:42-46), and the PR description's "Operators stay live, Google-style" should state the limitation: OR and -negation survive, quoted phrases are flattened to AND-of-words. Whether phrase search gets a real fix (backend escaping or a dedicated parameter) is a human decision.

🤖

P3 [CRF-22] The letter/number guard is an incomplete fix for the CRF-1 class: operator-only text like or still 400s. (Leorio)

The /[\p{L}\p{N}]/u guard fixed punctuation-only input, but it checks for letters while websearch_to_tsquery checks for lexemes. Type or (say, hunting a chat about an OR mapper): it has letters, so search:"or" is emitted; websearch treats a bare or as the OR operator and yields an empty tsquery; the backend's ChatSearchQueryIsEmpty check rejects it with a 400 (coderd/exp_chats.go:394-402), and the user gets an error alert for typing an English word. Verified against the parser (search:"or" parses with Search="or"); the empty-tsquery outcome follows from websearch semantics. Sibling instance of the exact class CRF-1 targeted: frontend emits, backend rejects. The saving grace is that the backend message ("Search query contains no searchable words.") is a real diagnosis, which is why this is P3 and not P2. Consider mapping that specific validation error to the empty state instead of ErrorAlert, or accepting it explicitly.

🤖

P3 [CRF-23] Typing has_unread:false while a has_unread:true pill is active silently discards the input and keeps the opposite filter. (Hisoka P3, Meruem P3)

Probed: extractTypedFilters("has_unread:false", keys, new Set(["has_unread"])) returns {filters: [], remainingText: "", consumed: true}. The token is consumed (index added at line 130) but the filter is dropped because the key is active. Press space: the text vanishes, the pill still says true, and the results are the exact inverse of what the user just asked for, with no signal. The unit test at searchQuery.test.ts:172 blesses this by name ("consumes active duplicate keys without adding another filter"), asserting the mechanism without asking whose intent survived. The old inline code did the same, but this PR rewrote the seam and canonized the behavior in a test. Fix: when the key is active, replace the existing filter's value (a replacements field, or have the caller upsert by key), or leave the token in remainingText so the user sees it was not accepted.

🤖

P3 [CRF-26] The suite blesses pr_status:"open merged", which the backend rejects. (Mafu-san)

Verified against the real parser: searchquery.Chats('pr_status:"open merged"') returns "open merged" is not a valid value; pr_status accepts repeated or comma-separated values, so the space-separated form is invalid. The expected output at this line (and the pill the QuotedTypedFilterDoesNotCommitEarly story builds by typing pr_status:"open merged") is a query the backend refuses with a 400. The user sees the designed error alert rather than a crash, so this is a UX rough edge, but it is also the live demonstration of CRF-5: the test suite happily asserts an emitted string the backend rejects, and nothing mechanical notices. Either the pill input should guide toward comma-separated values (or validate against the known enum), or the story should stop presenting a rejected query as the happy path.

🤖

P3 [CRF-27] No test pins that a typed filter value keeps everything after the first colon, so the one filter whose values contain colons (diff_url) is unguarded. (Bisky)

Every known-key value in the extractTypedFilters suite is colon-free (true, false, "open merged"). Replace indexOf(":") with lastIndexOf(":") at searchQuery.ts:118 and all 17 tests stay green while typing diff_url:https://github.com/coder/coder/pull/1 silently stops producing a pill (the key becomes diff_url:https, unknown, and the token stays as free text). That is a live mutant the suite cannot kill, and diff_url is a shipped pill key. The test: extractTypedFilters("diff_url:https://github.com/coder/coder/pull/1", knownKeys, new Set()) asserts filters: [{ key: "diff_url", value: "https://github.com/coder/coder/pull/1" }], remainingText: "", consumed: true. Ran green against current code before reporting.

🤖

P3 [CRF-28] extractTypedFilters is an exported seam with a three-field contract and no documentation of its non-obvious behaviors. (Leorio)

The PR description calls this "the quote-aware typed-filter-to-pill seam," and the seam has subtleties a caller must know that the code never states: consumed can be true while filters is empty (a duplicate active key is swallowed, tested by name); unbalanced-quote tokens pass through untouched; and remainingText deliberately keeps a trailing space when the last token was consumed, which exists so the caller's prevented Space keystroke still separates words (the reason for the event.key === " " ? remainingText : remainingText.trimEnd() ternary at ChatSearchDialog.tsx:289-293). A future dev who "cleans up" the trailing space or the ternary reintroduces the fixauth word-gluing bug, and the only guard is a unit test whose name never says why the space is there. Prescription, on the export:

// Pulls recognized key:value tokens out of typed text so they become pills.
// Tokens with unbalanced quotes are left in place (the value is still being
// typed). A token whose key is already active is consumed without producing
// a filter, so `consumed` can be true while `filters` is empty. When the
// last token was consumed, remainingText keeps a trailing space so the
// caller's suppressed Space keystroke still separates the next word.

🤖

Note [CRF-34] const buildQuery = buildChatSearchQuery; is a single-use alias, and its comment narrates the deleted parser. (Gon P2, Nami Nit, Meruem Nit, Netero Note)

Two names for one function in one file; the reader traces the alias to learn nothing. The comment's "without parsing it again" refers to normalizeChatSearchInput, which this PR removed; a fresh reader has no antecedent for "again." Call buildChatSearchQuery directly at line 163 and delete lines 125-127.

Same class, two more new comments in this commit restate what the code already shows: ChatSearchDialog.tsx:52 (the derivation is visible on the next line) and the first sentence of ChatSearchDialog.tsx:151 ("Debounce filters and free text as one snapshot" narrates the call; only the second sentence, the why, earns its line). Minor consistency: the new code uses (previous) and (filter) at lines 284/288 where the file convention is (prev) and (f).

🤖

Note [CRF-35] Clearing free text while a filter pill is active leaves the stale search: token in the query for one debounce period; the old code zeroed it immediately. (Takumi)

Round 1's queryFreeText = incompleteFilterKey || !freeText.trim() ? "" : debouncedFreeText short-circuited cleared text past the debounce. Now the empty text rides the 500ms snapshot debounce, so with a pill active the previous text's results (and hasSearchText, which gates the indexing-lag copy) linger for up to 500ms after deletion. It converges, and symmetric debounce on deletion is defensible, so this is informational. The commit-a-filter case the snapshot comment targets is genuinely atomic and correct.

🤖