feat: wire chat search box to full-text search - #27973
Conversation
…arch The Coder Agents chat search dialog sent bare free text as a title substring filter (title:"..."). Point it at the backend full-text search filter (search:) so free text matches chat titles, PR titles, PR numbers, and message bodies. Bare free text is wrapped in a quoted phrase by default, since the backend query tokenizer requires the search value to be a single token. Websearch operators (quoted phrases, OR, -negation) still pass through when the user supplies a proper quoted phrase. The empty state notes that message content is indexed periodically.
|
@codex review |
|
/coder-agents-review model:kimi-k3 thinking:xhigh |
|
Chat: Review posted | View chat Review history
deep-review v0.9.0 | Round 6 | Last posted: Round 6, 45 findings (7 P2, 25 P3, 4 Nit, 9 Note), COMMENT. Review Finding inventoryFinding inventory, PR #27973Findings
Contested and acknowledgedCRF-5 (P2, searchQuery.test.ts:96) - No mechanical guard that the backend parser accepts emitted queries
CRF-6 (P3, searchQuery.ts:147) - User-typed
|
| Reviewer | Focus |
|---|---|
| Bisky | tests |
| Chopper | ops/errors |
| Churn-guard | change verification |
| Ging | language modernization |
| Gon | naming |
| Hisoka | edge cases |
| Killua | perf |
| Kite | change integrity |
| Knov | contracts |
| Knuckle | SQL |
| Komugi | flake/determinism |
| Kurapika | security |
| Law | decomposition |
| Leorio | docs |
| Luffy | product |
| Mafu-san | process |
| Mafuuu | contracts |
| Melody | dispatch/pairing |
| Meruem | structural |
| Nami | frontend |
| Netero | mechanical checks |
| Pariston | premise testing |
| Pen-botter | product gaps |
| Razor | verification |
| Robin | duplication |
| Ryosuke | Go arch |
| Takumi | concurrency |
| Zoro | shape |
🤖 Managed by Coder Agents.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 127bebe103
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coder-agents-review[bot]](https://avatars.githubusercontent.com/in/3454270?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
127bebe to
52fa01c
Compare
…uctured state Replace the two-pass string parser with pure helpers that build the wire query directly from pill + free-text state, so free text is never re-parsed for key:value. Typed recognized filters are extracted into pills (quote-aware, no early commit on unbalanced quotes, separators preserved mid-string). Drop the title: special case; title: input is now literal search text and never triggers the search/title 400. Also fix three interaction bugs: a Unicode-aware guard replaces an ASCII-only check so non-ASCII searches work and underscore-only input does not 400; a single atomic debounce stops a committed filter value from briefly reappearing as search text; and the empty-state indexing note only appears when a search token was actually emitted.
52fa01c to
2ee34e5
Compare
|
/coder-agents-review model:kimi-k3 thinking:xhigh |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ee34e5ed5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Address round-2 review on the chat search box: - Debounce a primitive query snapshot instead of a fresh object, so the debounce no longer depends on React Compiler memoization (CRF-20). - Document that OR and -negation stay live while quoted phrases flatten to AND-of-words; the backend tokenizer cannot carry embedded quotes (CRF-21). - Map the backend "no searchable words" 400 to the empty state instead of a raw error alert (CRF-22). - Upsert a typed filter whose key already has a pill (last-write-wins) instead of silently discarding it (CRF-23). - Refuse to commit filter pills whose value sanitizes to empty (CRF-24). - Append the separator when typed-filter extraction leaves trailing text, and only extract when the caret is at the end (CRF-25). - Emit pr_status values comma-separated, the form the backend accepts (CRF-26). - Pin diff_url first-colon extraction and document the extractTypedFilters contract (CRF-27, CRF-28). Remove a single-use alias and stale comments (CRF-34). Add contract coverage (CRF-5): a Go test asserting searchquery.Chats accepts every query shape the frontend emits, a shape test for the sidebar list emitter, and a Playwright smoke spec that searches chats end to end.
DanielleMaywood
changed the title
|
/coder-agents-review model:kimi-k3 thinking:xhigh |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86445a02b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Two review fixes: - A search whose value tokenizes to zero lexemes (operator-only input like OR, or punctuation) is not an error. listChats now returns 200 with an empty list instead of a 400, and the frontend drops the message-string special case that violated the error-handling guideline (match by status, not message text). The no-results state is reached naturally from the empty response. - pr_status splitting no longer produces empty entries from comma-plus-space input such as "open, merged"; separators are normalized without empty segments.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Address round-3 review: - Delete the ChatSearchQueryIsEmpty pre-check. A zero-lexeme search matches nothing naturally, so the extra DB round trip on every search and its querier/dbauthz/dbmock/dbmetrics surface are removed (CRF-36). - Gate pill creation on the same normalization the emitter uses, so a comma-only pr_status value cannot become an active pill that is dropped from the query (CRF-24). - Suppress a stale query error once the search becomes inactive, so clearing the input returns to the default view immediately. - Validate pill values (pr_status enum, boolean flags, diff_url scheme and host) so the UI never emits a filter the backend rejects (CRF-38). - Encode the debounced snapshot as JSON instead of an uncommented 1/0 prefix, and document why it must stay a primitive (CRF-37). - Make the caller own the extraction separator; extractTypedFilters no longer computes a dead trailing-space hint (CRF-39). - Test hygiene: real negative guard for the empty-value story, the missing unread sidebar shape in the contract table, plural typed-filter extraction, an or zero-lexeme backend case, and an honest story name (CRF-40, CRF-41, CRF-42, CRF-43, CRF-44). - Comment cleanup: move the quoting rationale to the wrapping function, rename the debounced value, share the known-keys set, and strengthen the e2e alert-absence assertion (CRF-46, CRF-53, CRF-54).
|
/coder-agents-review |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Append to the search: swagger clause that a value tokenizing to no searchable words returns an empty list, so API consumers are not surprised by the 200-with-empty-results behavior (CRF-51).
|
CRF-51 addressed in 1fcef68: the swagger
|
Docs previewCheck off each page once it's been reviewed. If a page changes in a later push, its checkbox clears automatically so it gets a fresh look. Pages not yet wired into the docs navigation aren't listed here. |
|
/coder-agents-review |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
A lone or/AND/NOT (any casing) is a lexeme under the simple FTS config, not a zero-lexeme operator. Two earlier fixes were built on the wrong premise: - Drop the OR exclusion from hasSearchText so a lone operator word gets the indexing-lag note like any other word (CRF-56). - Seed a control chat containing "or" in the backend test so search:"or" is pinned as matching it and search:"!!!" as empty, instead of asserting empty on an empty database (CRF-57). Also rename KNOWN_FILTER_KEYS to CHAT_SEARCH_KNOWN_FILTER_KEYS to match its siblings, and debouncedQueryInput to debouncedQueryResult.
Documentation CheckUpdates Needed
The regenerated Automated review via Coder Agents |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- A quote-only search (e.g. a lone ") sanitizes to empty but still emits a search token (a single space, since the backend rejects an empty value), so it shows no results rather than the unfiltered recent-chats view. - Typing pr_status:open, merged no longer splits at the space: the comma continuation is merged so the pill filters both statuses instead of emitting pr_status:open plus a full-text search for merged.
- Drop the unused knownKeys parameter from extractTypedFilters (single caller, module constant) and the stripSurroundingQuotes helper. - Remove frontend boolean and pr_status lowercasing the backend already does; keep the comma/whitespace split for pr_status, which the backend requires (CRF-26). - Debounce the built query string directly instead of a JSON-encoded snapshot, deriving the indexing-note flag from the current free text.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1846b701a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
When a user selects the PR status pill, types open,, and presses Space to continue with merged, this validator normalizes open, to open and accepts it, so the keydown handler commits the pill before the space is inserted. The following merged becomes free-text search, producing pr_status:open search:"merged" instead of pr_status:open,merged. The fresh evidence after the earlier normalization fix is that the incomplete-pill keydown path still validates and commits the trailing-comma value; allow Space to continue a PR-status list when the raw value ends in a separator.
AGENTS.md reference: site/AGENTS.md:L16-L17
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 450d239. In the incomplete pr_status pill, pressing Space with a value ending in a comma no longer commits the partial value; the space lands so the list can continue, and Enter still commits. The committed value is normalized to the comma-separated form, so a pill typed as open, merged becomes pr_status:open,merged. Added a character-by-character story asserting the pill and the emitted query are pr_status:open,merged and that no pr_status:open search:"merged" request occurs; verified against the real searchquery.Chats that pr_status:open,merged parses to statuses=[open merged].
🤖 Coder Agents
In the incomplete PR-status pill, pressing Space with a value ending in a comma no longer commits the partial value; the space lands so the user can continue the list. Enter still commits, and the committed value is normalized to the comma-separated form the backend accepts.
- Use a Set for boolean filter values instead of a regex. - Tighten the search-emission comment. - Keep the diff_url scheme/host pre-check: new URL alone accepts "https:///pull/1" as host "pull", so the regex guard is load-bearing.
new URL plus the protocol and host check is enough for the common cases
(no scheme, wrong scheme, garbage input). The rare empty-host form
("https:///pull/1") slips through to a clear, field-level backend 400,
which is acceptable feedback rather than a broken state.
2 participants
Wires the Coder Agents chat search box up to the backend full-text search filter. Bare free text previously produced a
title:"..."substring filter; it now produces asearch:filter, so free text matches chat titles, PR titles, PR numbers, and message bodies via the FTS index added in #27126.The box is for text; structured filters are pills. Free text is wrapped in a single
search:"..."token (the backend query tokenizer requires a single token and has no escape handling for embedded quotes, so embedded"are stripped first). Operators stay live, Google-style:fix OR bugdisjoins,fix -timeoutexcludes. One limitation: quoted phrases are flattened to AND-of-words, because the backend tokenizer cannot carry embedded quotes through towebsearch_to_tsquery. Typing a recognized filter code (has_unread:true,archived:true,pr_status:open,diff_url:<url>) pulls it out of the text into a removable pill. Typedtitle:is no longer special-cased; it is literal search text and never triggers thesearch/titlemutual-exclusion 400.A search whose text tokenizes to zero lexemes (operator-only or punctuation-only input) is not an error: the backend returns an empty list and the UI shows the no-results state. Pill values are validated (enum, boolean, URL) so a committed pill is always a query fragment the backend accepts.
Refs CODAGT-726
Depends on #27126
Implementation notes
searchQuery.ts: pure helpers build the wire query directly from structured state.buildChatSearchQuery(filters, freeText)returns{ query, hasSearchText }and never inspects free text forkey:value.extractTypedFiltersis the quote-aware typed-filter-to-pill seam. Per-key validators (CHAT_SEARCH_FILTER_VALIDATORS) gate pill creation.ChatSearchDialog.tsx: debounces a single primitive (a JSON{ query, hasSearchText }snapshot) so the debounce does not depend on React Compiler object memoization. Typed-filter extraction runs only when the caret is at the end, preserves separators, and upserts a typed filter whose key already has a pill (last-write-wins).coderd/exp_chats.go): the zero-lexemeChatSearchQueryIsEmptypre-check is removed; a zero-lexeme search matches nothing naturally, so the handler returns an empty list. The querier/dbauthz/dbmock/dbmetrics surface for that query is removed too (make gen).ChatSearchResults.tsx: the empty-state indexing note only appears when asearch:token was emitted (hasSearchText), and stale errors are cleared once the search becomes inactive.Contract coverage:
coderd/searchquery/search_test.goTestSearchChatsFrontendEmittedassertssearchquery.Chatsaccepts every query shape the frontend emits (cross-referenced with the frontend emitters).site/src/api/queries/chats.test.tsadds shape tests for the sidebar list emitter (getChatListQueryString).site/e2e/tests/agents/chatSearch.spec.tsis a Playwright smoke test that logs in, opens/agents, searches, and asserts thesearch:"..."request returns 200 with the no-results state and no error alert.Tests: 195 frontend unit tests, 30 Storybook interaction stories, Go contract + handler tests, and the e2e smoke test all pass.