Repository navigation
fix(search): require a three-character last word for message search - #2210
Conversation
Message typeahead prefix-matches the last word, and the relay ranks every match before it applies the limit, so a one- or two-character last word makes Postgres score most of a community's messages. block/buzz#8947 makes the relay answer such a search with no hits; send it only once the last word has three characters, operators or not, so the palette says to keep typing instead of showing an empty result. People and channel lookups keep the shared two-character minimum. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6e9b79540
ℹ️ 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".
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 LGTM. The client gate matches the relay prefix rule in block/buzz#8947 (trimmed last whitespace token, code-point count >= 3), the operator bypass is correctly removed since the relay applies the rule regardless of filters, and operator-only queries still read history. Verified at d6e9b79: vitest src/app/shell + src/shared (651 passed), tsc --noEmit, biome on changed files.
A multiword query such as `project pl` is already long enough as a whole; only its last word is short, so say so. The bundled help guide now explains the three-character last-word rule for agents answering why search waits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Salman Mohammed <smohammed@squareup.com>
…eal tsquery (#8976) ## Summary Message search is one of the slowest relay queries. Short typeahead prefixes make it worst: `a:*` matches most of a community. **Why it was slow:** the tsquery was built inside a `CROSS JOIN LATERAL` subquery, so the planner saw an InitPlan and couldn't read the `search_tsv` statistics. It fell back to the default 0.5% selectivity for `@@`: for `a:*` on 1M rows it estimated 4,497 rows when 517,065 matched. Every search therefore took every GIN match, ran `ts_rank_cd` on each one, and top-N sorted them before `LIMIT 20`. This PR: 1. **Binds the real tsquery.** `push_tsquery` is unchanged. A separate `SELECT (…)::text` round trip resolves the tsquery, and the search binds it as `$q::tsquery`. An empty tsquery returns no hits without running the search. 2. **Plans each search for its own tsquery.** The search runs with `.persistent(false)`. A cached prepared statement can switch to a generic plan after five runs, and a generic plan can't see the tsquery. Forcing a generic plan lost the gain in testing. 3. **Ranks only the newest 1,000 prefix matches.** Prefix-mode relevance search ranks the newest `PREFIX_RANK_WINDOW` (1,000) matches. The inner `ORDER BY created_at DESC, id DESC LIMIT 1000` matches each partition's primary key read backwards, so for common prefixes Postgres streams the newest rows and stops at 1,000. For rarer terms it keeps the GIN bitmap plan, now with an accurate estimate. Full-text search and profile (kind:0) typeahead still rank every match. ### Behavior change: every human message search (product decision needed) Prefix mode isn't limited to keystroke typeahead. buzz-app (`useSearchMessages.ts`) and legacy desktop (`desktop/src-tauri/src/commands/messages.rs:184`) send `search_mode: "prefix"` for **all** message search. Full-text search runs only on the WS REQ path, which agents and the CLI use. So for people: - Results are the best-ranked of the newest 1,000 matches in the search's scope, not of all matches. If more than 1,000 newer messages also match, an older, more relevant message drops out of ⌘K results. The scope includes `#channel`, `from:` and date bounds, so a narrower search has the same limit within it. People have no full-text fallback. - The bridge honors `page`, and buzz-app loads later pages at 20 per page. Every page ranks the same window, so paging stays consistent, but results end after 1,000 matches (page 50). Full-text search (agents/CLI) and profile (kind:0) typeahead still rank every match. ### Perf risk: matches the backward scan has to walk far to find The planner picks the backward scan when it expects 1,000 matches near the top of the community, and it estimates that by treating each filter as independent. Two known shapes break that assumption. **Terms common in old messages but absent from recent ones.** The planner assumes matches are spread evenly over time. For a term that was common long ago but doesn't appear recently (an old project name, a former teammate), it can pick the backward primary-key scan and walk every newer row before reaching the first match. Reviewers measured this on 1.2M synthetic rows: about 2.05–2.1 s, against 0.7–1.8 s for the old query shape and 1.2 s with the GIN plan forced. The 20 s statement timeout and the #8954 cancel still bound it. The worst case is close to the old `a:*` worst case, but it now scales with community size, not match count. The planned EXPLAIN on production-shaped data will include such a term. **Terms common in channels the searcher can't read.** Human search always adds `channel_id = ANY(<readable channels>) OR channel_id IS NULL` (`build_search_channel_scope_filter`). A term that is common in private channels or other people's DMs can get a high match estimate, take the backward scan, and walk the community's rows to find a few readable matches, where the old shape used the GIN bitmap. This hasn't been reproduced yet; the production-shaped EXPLAIN will include such a term next to the old-only one. The same timeout and cancel bound it. ### Cost - One extra round trip per search to resolve the tsquery (about 0.03 ms of execution). It runs on the same pool checkout as the search (`cancel::fetch_all` takes the caller's connection), so a busy pool queues each search once. An IMMUTABLE SQL function could remove the round trip, but that needs a migration, so it's not done here. ### Related issue - Follows #8954 (cancel abandoned searches, bound the search pool) and #8964 (closed in favor of #8954). - Builds on #8947 (3-character server-side prefix minimum), merged from main. The resolve step passes its minimum. When the trailing prefix is too short, the resolved tsquery is NULL, and the search returns no hits without running the second query. - Client-side pairing: block/buzz-app#2210 (3-character last-word minimum for ⌘K message search). ## Testing **EXPLAIN ANALYZE on a synthetic 1M-row `events` table** (monthly partitions, GIN on `search_tsv`, kinds 9/40002/40008, custom plans, warm cache): | Prefix | Matches | Before | After | Plan after | |---|---|---|---|---| | `a` | 517k | 322 ms | **0.7 ms** | backward primary-key scan per partition, stops at 1,000 | | `th` | 137k | 127 ms | **1.2 ms** | backward primary-key scan, stops at 1,000 | | `a` (community with 10% of rows) | — | ~100 ms | **1–2 ms** | backward primary-key scan | | `deploy` | 8.2k | 15 ms | 5–10 ms | GIN bitmap, accurate estimate | | `d278` | 500 | 1.1 ms | 0.5 ms | GIN bitmap | | rare word | 0 | 0.07 ms | 0.03 ms | GIN bitmap | **Postgres integration tests:** - New test `prefix_relevance_ranks_only_the_newest_matches`. An old, highly relevant message drops out of prefix results behind 1,000 newer matches but still ranks first in full-text search. Prefix pages end at page 50. An old, best-ranked profile still tops profile typeahead behind 1,000 newer profile matches. Removing the window fails the test, and so does applying it to profiles. - `abandoned_search_cancels_its_statement`: the `pg_stat_activity` matcher now looks for `ts_rank_cd(`, because the old `search_query.query` text no longer exists. - Locally, each test ran against a fresh database because `cargo-nextest` isn't installed, so the hosted lane hasn't run yet. 26 of 27 pass. `artifact_search_uses_only_live_current_head` also fails on unmodified main in that local setup, so the failure isn't from this change. **Checks:** `cargo fmt --check` and `cargo clippy -p buzz-search --all-targets -D warnings` are clean. **Not yet done:** - hosted CI; - a manual check against a local relay with ⌘K typeahead; - EXPLAIN against production-shaped data, including a term common only in old messages and a term common only in channels the searcher can't read. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Salman Mohammed <smohammed@squareup.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Problem
⌘K message search sends
search_mode: "prefix", so the relay adds:*to the last word. Postgres can't return GIN matches in rank order, so it scores every match withts_rank_cdbefore it appliesLIMIT 20. A one- or two-character prefix matches most of a community. In Wes's 2M-message benchmark (block/buzz#8964),thranked about 1.4M rows (1.29 s) andrelranked about 48k (187 ms). Message search is the top slow query in production database telemetry.#2200 set one shared two-character minimum for every relay search. That's right for people and
@completion, because some names are two characters, but too low for messages. block/buzz#8947 makes the relay return no hits for a message prefix search whose last word has fewer than three characters, with or without filters. With only #2200, the palette would send those searches and show "No matching messages" for them.Change
MESSAGE_SEARCH_MIN_CHARS = 3andmessageSearchable(text)insrc/shared/search-debounce.ts. Like the relay rule, it counts the characters of the last whitespace-delimited word, becauseproject plruns the same broadpl:*lookup aspl.useSearchMessagesuses it for any typed text, including whenfrom:orin:operators are present. This drops refactor: centralize relay search debounce and require two characters #2200's filter bypass for short text, because the relay answers those searches with no hits too. Operators without text still read newest history, as before.SEARCH_MIN_CHARS = 2.Tests
reads no message search until the last word has three characters:abandproject plsend no message search and show the too-short copy.project plasends one.an in: operator does not lower the message search minimum: replaces the test that pinned the filter bypass.in:#crew absends nothing and shows the copy.in:#crew abcsends the scoped prefix search.src/app/shell/(277 tests) andsrc/features/identity-names/consumers.test.tsxpass. biome,tsc --noEmitand the pre-push hooks pass.Not yet checked
🤖 Generated with Claude Code