fix(persons): search WhatsApp participant identities#696
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the PersonService.search method to use a raw SQL query that searches across persons, platform_identities, and chat_participants to improve search results, particularly for WhatsApp contacts. It also updates the CLI command to support the new schema fields and adds corresponding unit tests. Feedback on the changes highlights two main issues: first, the use of DISTINCT ON (id) with LIMIT causes the results to be ordered arbitrarily by UUID, which can discard relevant matches; second, a portion of the UNION ALL query is dead code due to database foreign key constraints that prevent cp.person_id from referencing a non-existent person.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| SELECT DISTINCT ON (id) | ||
| id, | ||
| "displayName", | ||
| "primaryPhone", | ||
| "primaryEmail", | ||
| "avatarUrl", | ||
| metadata, | ||
| "createdAt", | ||
| "updatedAt" | ||
| FROM candidates | ||
| ORDER BY id, rank_ts DESC | ||
| LIMIT ${limit} |
There was a problem hiding this comment.
When using DISTINCT ON (id) in PostgreSQL, the query requires the first expression in ORDER BY to match the DISTINCT ON expression (which is id here).
Because of this, the final results are sorted by id (which is a UUID and effectively random) before the LIMIT is applied. This means the LIMIT ${limit} will discard highly relevant (recently active) matches in favor of arbitrary UUID ordering, and the returned list will not be sorted by rank_ts DESC.
To fix this and ensure the most relevant/recently active results are returned, you should wrap the DISTINCT ON query in a subquery or CTE, and then perform a final ORDER BY rank_ts DESC LIMIT ${limit} on the distinct results.
SELECT
id,
"displayName",
"primaryPhone",
"primaryEmail",
"avatarUrl",
metadata,
"createdAt",
"updatedAt"
FROM (
SELECT DISTINCT ON (id)
id,
"displayName",
"primaryPhone",
"primaryEmail",
"avatarUrl",
metadata,
"createdAt",
"updatedAt",
rank_ts
FROM candidates
ORDER BY id, rank_ts DESC
) distinct_candidates
ORDER BY rank_ts DESC
LIMIT ${limit}| UNION ALL | ||
|
|
||
| SELECT | ||
| cp.person_id AS id, | ||
| cp.display_name AS "displayName", | ||
| NULL::text AS "primaryPhone", | ||
| NULL::text AS "primaryEmail", | ||
| cp.avatar_url AS "avatarUrl", | ||
| jsonb_build_object( | ||
| 'source', 'chat_participants', | ||
| 'chatId', cp.chat_id, | ||
| 'platformUserId', cp.platform_user_id | ||
| ) AS metadata, | ||
| cp.created_at AS "createdAt", | ||
| cp.updated_at AS "updatedAt", | ||
| COALESCE(cp.last_seen_at, cp.updated_at, cp.created_at, 'epoch'::timestamptz) AS rank_ts | ||
| FROM chat_participants cp | ||
| LEFT JOIN persons p ON p.id = cp.person_id | ||
| WHERE | ||
| cp.person_id IS NOT NULL | ||
| AND p.id IS NULL | ||
| AND ( | ||
| cp.display_name ILIKE ${searchPattern} ESCAPE '' | ||
| OR cp.platform_user_id ILIKE ${searchPattern} ESCAPE '' | ||
| ) |
There was a problem hiding this comment.
The UNION ALL block attempts to query chat_participants where cp.person_id IS NOT NULL AND p.id IS NULL.
However, in the database schema (packages/db/src/schema.ts), chat_participants.personId has a foreign key constraint referencing persons.id with onDelete: 'set null':
personId: uuid('person_id').references(() => persons.id, { onDelete: 'set null' })
Because of this referential integrity, p.id can never be NULL when cp.person_id is not null. If a referenced Person is deleted, the database automatically sets cp.person_id to NULL. Consequently, this entire UNION ALL block is dead code and will always return zero rows. You should remove this dead UNION ALL block to simplify the query and improve performance.
namastex888
force-pushed
the
fix/persons-search-whatsapp-identities-dev
branch
from
ae5b864 to
8c95674
Compare
|
Reviewed Gemini stale comments: both made sense and are handled in the latest force-push. The contradictory |
1 participant
Summary
PersonService.search()beyond sparsepersonsrows to include platform identities and WhatsAppchat_participantsnames/ids.Cadu Cassauare discoverable.ESCAPE ''.primaryEmail/primaryPhonefrom SDK/API person objects.Dogfood / proof
tested_known_names = 835found = 835missing = 0StéfaniCezar AugustoRaphael RosaYolanda KarolinaCadu Cassau/\/\/\/Validation
bun test packages/api/src/services/__tests__/persons.test.ts→ 11 pass / 0 failbun run --filter @omni/api typecheck→ passbun run --filter @automagik/omni build→ passbun run --filter @automagik/omni build:server→ pass3971 pass / 292 skip / 0 fail