[codex] harden live sync and customer identity matching - #139
Merged
Conversation
📝 WalkthroughWalkthroughAdds a centralized ChangesWorkspace Live Invalidation
Customer Identity Matching Hardening
Sequence Diagram(s)sequenceDiagram
participant DB as Supabase DB (postgres_changes)
participant Hook as useWorkspaceLiveInvalidation
participant QC as React Query Cache
participant Win as window (CustomEvent bus)
participant UI as Dashboard / Conversation Components
DB->>Hook: postgres_changes event (table, workspace_id)
Hook->>Hook: add table to pending Set, schedule 350ms debounce
Hook->>QC: invalidateQueries(workspaceKeys + globalKeys)
loop for each changed table
Hook->>Win: dispatchEvent(WORKSPACE_LIVE_INVALIDATION_EVENT, {workspaceId, table})
end
Win->>UI: onLiveChange handler fires
UI->>UI: filter by workspaceId + table name
UI->>UI: fetchActivities() / fetchInsights() / fetchTimeline() / fetchConversations()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
supabase/migrations/20260622144553_harden_customer_identity_matching.sql (2)
149-153: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoffCorrelated subquery for conversation_count may impact performance at scale.
This subquery executes once per customer row in the workspace before filtering. For workspaces with many customers, consider replacing with a pre-aggregated CTE or lateral join:
♻️ Suggested refactor using pre-aggregated CTE
+ conversation_counts as ( + select customer_id, count(*)::integer as cnt + from public.conversations + where customer_id in (select id from public.customers where workspace_id = p_workspace_id) + group by customer_id + ), scoped_customers as ( select c.id, ... - ( - select count(*)::integer - from public.conversations co - where co.customer_id = c.id - ) as conversation_count + coalesce(cc.cnt, 0) as conversation_count from public.customers c + left join conversation_counts cc on cc.customer_id = c.id where c.workspace_id = p_workspace_id ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/20260622144553_harden_customer_identity_matching.sql` around lines 149 - 153, The correlated subquery in the conversation_count field executes for every customer row, causing performance issues at scale. Replace this subquery with a pre-aggregated CTE that groups public.conversations by customer_id and calculates the count once, then left join this CTE to the customer table (aliased as c) on customer_id to retrieve the pre-calculated conversation_count for each customer. This avoids the repeated subquery execution and improves query performance.
240-248: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueThe 'weak' tier branch is unreachable.
The
WHERE s.score >= 0.55filter on line 248 excludes all rows that would fall into theelse 'weak'branch (score < 0.55). This appears intentional for hardening, but the dead branch adds slight confusion.Consider either removing the else branch or adding a comment clarifying the intentional exclusion:
case when s.score >= 0.90 then 'strong' - when s.score >= 0.55 then 'review' - else 'weak' + else 'review' -- scores < 0.55 filtered by WHERE clause end as tier,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/20260622144553_harden_customer_identity_matching.sql` around lines 240 - 248, The CASE statement in the SELECT clause contains an unreachable else branch that would assign the 'weak' tier, but the WHERE clause on line 248 filters out all rows where s.score is less than 0.55, making this branch impossible to reach. Either remove the else 'weak' branch entirely from the CASE expression since it will never execute, or if this design is intentional, add a SQL comment above the WHERE clause explaining that scores below 0.55 are intentionally excluded for customer identity hardening purposes.src/components/conversations/CustomerConversationHistory.tsx (1)
53-68: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider adding workspaceId filter for consistency.
Other consumers (
ActivityFeed,LearningInsightsWidget,InsightsWidget) checkdetail?.workspaceId === workspace?.idbefore triggering refresh. This component andCustomerTimelineskip that check. While functionally safe (the hook only emits events for the current workspace), adding the check would maintain a consistent pattern across all consumers.♻️ Suggested consistency fix
+ import { useWorkspace } from '`@/hooks/useWorkspace`'; export const CustomerConversationHistory = ({ customerId, currentConversationId, }: CustomerConversationHistoryProps) => { + const { workspace } = useWorkspace(); const [conversations, setConversations] = useState<Tables<'conversations'>[]>([]); // ... useEffect(() => { const onLiveChange = (event: Event) => { const detail = (event as CustomEvent<WorkspaceLiveInvalidationDetail>).detail; if ( - detail && + detail?.workspaceId === workspace?.id && ['conversations', 'message_events', 'customers', 'customer_identities'].includes( detail.table, ) ) { void fetchConversations(); } }; window.addEventListener(WORKSPACE_LIVE_INVALIDATION_EVENT, onLiveChange); return () => window.removeEventListener(WORKSPACE_LIVE_INVALIDATION_EVENT, onLiveChange); - }, [fetchConversations]); + }, [fetchConversations, workspace?.id]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/conversations/CustomerConversationHistory.tsx` around lines 53 - 68, In the CustomerConversationHistory component, the useEffect hook containing the onLiveChange event listener is missing a workspaceId validation check that other consumers like ActivityFeed, LearningInsightsWidget, and InsightsWidget include. Add a check for detail?.workspaceId === workspace?.id alongside the existing table check to ensure the refresh only triggers for events from the current workspace, maintaining consistency with the pattern used in other components that listen to WORKSPACE_LIVE_INVALIDATION_EVENT.src/components/conversations/CustomerTimeline.tsx (1)
172-188: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoffConsider adding workspaceId filtering for consistency.
Unlike
InsightsWidget, this listener doesn't filter bydetail.workspaceId. While the current implementation is safe (the hook is workspace-scoped at the app level), adding a workspaceId check would provide defensive filtering consistent with other components.This would require either passing
workspaceIdas a prop or deriving it from the customer data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/conversations/CustomerTimeline.tsx` around lines 172 - 188, The onLiveChange function in the useEffect hook checks the detail.table against a list of tables but does not filter by detail.workspaceId like other similar components do. Add a workspaceId check to the existing conditional that validates detail.table. You will need to either pass workspaceId as a prop to this component or derive it from the existing customer data to compare against detail.workspaceId, then include this check in the same if statement alongside the table validation to ensure the event is for the current workspace.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/migrations/20260622144553_harden_customer_identity_matching.sql`:
- Around line 40-42: The phone number normalization logic in the migration
contains a hardcoded assumption that local format numbers (starting with 0 and
10-12 digits) are UK numbers, prefixing them with +44. This will cause incorrect
normalization for workspaces in other countries. Remove or refactor the
hardcoded country code logic in the conditional block that checks `if v_digits
like '0%' and length(v_digits) >= 10 and length(v_digits) <= 12` by either
making the country code configurable through a workspace parameter or setting,
or removing this heuristic entirely to prevent false matches across different
geographical regions.
---
Nitpick comments:
In `@src/components/conversations/CustomerConversationHistory.tsx`:
- Around line 53-68: In the CustomerConversationHistory component, the useEffect
hook containing the onLiveChange event listener is missing a workspaceId
validation check that other consumers like ActivityFeed, LearningInsightsWidget,
and InsightsWidget include. Add a check for detail?.workspaceId ===
workspace?.id alongside the existing table check to ensure the refresh only
triggers for events from the current workspace, maintaining consistency with the
pattern used in other components that listen to
WORKSPACE_LIVE_INVALIDATION_EVENT.
In `@src/components/conversations/CustomerTimeline.tsx`:
- Around line 172-188: The onLiveChange function in the useEffect hook checks
the detail.table against a list of tables but does not filter by
detail.workspaceId like other similar components do. Add a workspaceId check to
the existing conditional that validates detail.table. You will need to either
pass workspaceId as a prop to this component or derive it from the existing
customer data to compare against detail.workspaceId, then include this check in
the same if statement alongside the table validation to ensure the event is for
the current workspace.
In `@supabase/migrations/20260622144553_harden_customer_identity_matching.sql`:
- Around line 149-153: The correlated subquery in the conversation_count field
executes for every customer row, causing performance issues at scale. Replace
this subquery with a pre-aggregated CTE that groups public.conversations by
customer_id and calculates the count once, then left join this CTE to the
customer table (aliased as c) on customer_id to retrieve the pre-calculated
conversation_count for each customer. This avoids the repeated subquery
execution and improves query performance.
- Around line 240-248: The CASE statement in the SELECT clause contains an
unreachable else branch that would assign the 'weak' tier, but the WHERE clause
on line 248 filters out all rows where s.score is less than 0.55, making this
branch impossible to reach. Either remove the else 'weak' branch entirely from
the CASE expression since it will never execute, or if this design is
intentional, add a SQL comment above the WHERE clause explaining that scores
below 0.55 are intentionally excluded for customer identity hardening purposes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ff6190ac-2840-414d-8743-81164093cee2
📒 Files selected for processing (10)
src/App.tsxsrc/components/conversations/CustomerConversationHistory.tsxsrc/components/conversations/CustomerTimeline.tsxsrc/components/dashboard/ActivityFeed.tsxsrc/components/dashboard/InsightsWidget.tsxsrc/components/dashboard/LearningInsightsWidget.tsxsrc/hooks/useWorkspaceLiveInvalidation.tssrc/lib/api/__tests__/edgeFunctionSendContracts.test.tssupabase/migrations/20260622144238_live_ui_realtime_publication.sqlsupabase/migrations/20260622144553_harden_customer_identity_matching.sql
Comment on lines
+40
to
+42
| if v_digits like '0%' and length(v_digits) >= 10 and length(v_digits) <= 12 then | ||
| v_digits := '+44' || substring(v_digits from 2); | ||
| end if; |
There was a problem hiding this comment.
Hardcoded UK country code assumption for local phone numbers.
Numbers starting with 0 and 10-12 digits are assumed to be UK local numbers and prefixed with +44. This will produce incorrect normalized keys for workspaces operating in other countries (e.g., a US number 0555123456 would become +44555123456).
Consider making the default country code configurable via a workspace setting or parameter, or removing this heuristic to avoid false matches.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/migrations/20260622144553_harden_customer_identity_matching.sql`
around lines 40 - 42, The phone number normalization logic in the migration
contains a hardcoded assumption that local format numbers (starting with 0 and
10-12 digits) are UK numbers, prefixing them with +44. This will cause incorrect
normalization for workspaces in other countries. Remove or refactor the
hardcoded country code logic in the conditional block that checks `if v_digits
like '0%' and length(v_digits) >= 10 and length(v_digits) <= 12` by either
making the country code configurable through a workspace parameter or setting,
or removing this heuristic entirely to prevent false matches across different
geographical regions.
This was referenced Jun 23, 2026
This was referenced Jun 30, 2026
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Safety
No deploy, no Supabase migration apply, no Supabase function deploy, no provider sends/calls, no review publishing, no billing mutation, no Cloudflare mutation, no queue destructive action, and no public signup change.
Summary by CodeRabbit
New Features
Improvements
Tests