Repository navigation
fix(search): normalize profile name tiers in one place - #8334
Conversation
fd3b612 to
885dc57
Compare
dc73936 to
560b12b
Compare
885dc57 to
a2e2a24
Compare
dce8039 to
82a9cbd
Compare
958ba50 to
51320bf
Compare
51320bf to
4bc1ec0
Compare
82a9cbd to
3578814
Compare
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
APPROVE at exact head 35788145b3c84c49b1029986cfaca9ae32a6d7fa over stacked base 4bc1ec060d38920097f3ade6c399361f93faa319.
No author-actionable defect found.
The three-file patch removes the Rust/PostgreSQL case-normalization split by constructing the query and both name-tier operands from the same bound SQL normalizer (btrim(regexp_replace(lower(...), '\s+', ' ', 'g'))). Candidate matching and tenant/current-profile/name-only fences are unchanged. Ordering remains total and page-stable: tier, names-only ts_rank_cd, newest first, then event id. The API docs now describe that actual tie order.
The added PostgreSQL regression is behaviorally relevant: exact Émile must precede newer prefix Émilee, exposing the previous Rust to_lowercase versus database lower() disagreement. cargo fmt --all -- --check and git diff --check passed on a clean tree at the reviewed head. Exact-head replacement CI has Rust lint, unit tests, PostgreSQL tests, relay/backend integration, security, and Linux cross-compiles green with no active-run failure at submission.
The apparent failed fan-out belongs to cancelled superseded run 38803916833; its selector was cancelled and downstream jobs terminated within seconds. Replacement run 38803918923 is authoritative.
Author action: none.
Confidence gaps / verification owner: local Rust execution was blocked before compilation by the reviewer host's unaccepted Xcode 27 license (cc exit 69); reviewer/tooling owns that environment issue. A few unrelated exact-head CI lanes were still running at submission; CI owns merge readiness. Any head movement expires this approval.
Compare the query and the profile names after the same SQL normalization (lower, whitespace runs collapsed, trimmed). Rust to_lowercase and the database lower() disagreed for non-ASCII letters under a C collation, so an exact name such as Émile missed tier 0. Describe the real tie order: name relevance, newest first, event id. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <[email protected]>
3578814 to
1df4575
Compare
github-actions
Bot
added
the
codex-security-review-current
🔐 Codex Security Review
Review SummaryOverall Risk: NONE
FindingsNo concrete security, correctness, or reliability findings were identified. Notes
Generated by Codex Security Review | |
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: APPROVE
Reviewed: f148c0f22c35105524527dec8e4a7285c26804c6..1df457592b22961c5e95c7ec0eacf9e104c6c20d (exact head 1df457592b22961c5e95c7ec0eacf9e104c6c20d)
Risk: medium — contained server-side profile-search ranking and normalization behavior.
Behavior/contracts traced: The bound query, name, and display_name tier operands use the same PostgreSQL lowercase/whitespace normalizer in crates/buzz-search/src/query.rs:359-365,403-446. Community, profile-kind/current-profile, deletion, visibility, and non-null-name fences remain intact. Ordering stays total and page-stable: match tier, names-only ts_rank_cd, created_at DESC, then event ID. docs/profile-search.md:32-49 accurately limits relevance to name fields and does not promise accent folding or locale-independent Unicode casing. The regression at crates/buzz-search/tests/postgres_fts_integration.rs:1733-1740 exercises the production SQL seam with Émile versus newer prefix Émilee; the documented broken control reverses the intended order.
Findings: No blocking or non-blocking code defects found. The rebased commit has the same stable patch ID (15f0eb006ba123d2f4ca93697b724dbda50dab95) and byte-identical touched-path blobs as previously reviewed 35788145b3c84c49b1029986cfaca9ae32a6d7fa; relevant base objects are unchanged, while the base-only addition is an unrelated mobile accessibility script.
Author action: none.
Verification owner: exact-head CI / merge gate for jobs still running; reviewer-machine operator for local Xcode/Docker availability.
Validation at matching HEAD: clean checkout at 1df457592b22961c5e95c7ec0eacf9e104c6c20d; git diff --check f148c0f2...HEAD passed; . ./bin/activate-hermit && cargo fmt --all -- --check passed. GitHub reported the PR open, non-draft, and mergeable at the pinned base/head. Exact-head CI had 40 passing checks and no failures at review submission, including Rust lint/unit, relay artifact PostgreSQL tests, backend/relay integration, security, macOS, Linux cross-compiles, and image builds.
Manual/native evidence: not applicable to this server-side SQL ranking change. Independent local cargo test -p buzz-search/clippy execution was blocked by an unaccepted Xcode license, and Docker/PostgreSQL was unavailable on the reviewer host; these are reviewer-tooling gaps, not PR-caused failures.
Residual risk: low. Windows Rust, the PostgreSQL domain test, and three downstream Desktop integration shards were still running with no red exact-head job. Those external gates—not author rework—own final merge readiness.
…i-bindings * origin/main: feat(mobile): find people by name through the relay's name-only profile search (#8333) fix(ios): respect app minimum in generated CocoaPods targets (#8379) fix(relay): confirm requested channels in one membership query (#8378) fix(mobile): split channel history batches at the relay channel cap (#8342) fix(search): normalize profile name tiers in one place (#8334) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
… into duncan/bbpublic-nip-fi-resolver * origin/duncan/bbpublic-nip-fi-bindings: feat(mobile): find people by name through the relay's name-only profile search (#8333) fix(ios): respect app minimum in generated CocoaPods targets (#8379) fix(relay): confirm requested channels in one membership query (#8378) fix(mobile): split channel history batches at the relay channel cap (#8342) fix(search): normalize profile name tiers in one place (#8334) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
🤖
Summary
Profile name search (
POST /profiles/search, added in #8301) sorts results into tiers: an exact name first, then a name that starts with the query, then a name with a later word that starts with it. This PR stacks on #8301 and fixes two review items.Tier normalization in one place. Before this change, the query was lower-cased in Rust (
to_lowercase), and the names were lower-cased in SQL (lower()). These can disagree. Under aCcollation, PostgreSQL'slower()changes only ASCII letters, while Rust also lower-cases letters likeÉ. So a search forÉmiledid not put the profile namedÉmilein the exact tier. Now the query and both name fields go through the same SQL expression: lower-case, collapse whitespace runs to one space, then trim. The query is bound once and both sides compare equal by construction. The trim also moved after the collapse, so surrounding whitespace that the regex turns into a space is now trimmed too.Ranking wording. The doc comment and
docs/profile-search.mdsaid "ties keep the NIP-50 order". That was wrong. Ties are ordered byts_rank_cdover the two name fields only (not the whole profile, as in NIP-50), then newest first, then event id.Out of scope, not changed here: matching names that contain escaped Unicode or newlines, and any new name index.
Related issue
Review on #8301: #8301 (review)
Testing
profile_names_match_name_words_only: profilesÉmileand a newerÉmilee, queryÉmile. The exact match must come first.postgres:17-alpinedatabase created with--locale=C:profile_names_match_name_words_onlyandprofile_names_survive_unparsable_contentpass;profile_names_match_name_words_onlyfails (Émileesorts beforeÉmile).cargo fmt --check,cargo clippy -p buzz-search --all-targets -D warnings, andcargo test -p buzz-search: clean.rust-testsanddesktop-tauri-checkspassed.