rework weights for same name signups#1298
Conversation
…lLimit to recent stats request. Update loadRecentSignUpStats function to include email normalization checks. Adjust tests to reflect new return structure.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds normalized-email duplicate detection to sign-up risk scoring: request/response types updated, server logic extended to count matching normalized emails, a composite DB index added for queries, a migration created, and a git submodule reference bumped. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR enhances the signup risk assessment system by adding same-email detection alongside the existing same-IP and similar-email-base checks. It introduces a
Confidence Score: 5/5Safe to merge — changes are additive, consistent with existing patterns, and carry no breaking changes. All changes follow established patterns (cost-capped LIMIT queries, concurrent IF NOT EXISTS index creation, null-guarding). No logic errors, security issues, or migration safety concerns were found. The test mock is correctly updated. No P1 or P0 findings. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant calculateSignUpRiskAssessment
participant loadRecentSignUpStats
participant DB as DB (replica)
Caller->>calculateSignUpRiskAssessment: context (email, ip, ...)
calculateSignUpRiskAssessment->>loadRecentSignUpStats: SignUpRiskRecentStatsRequest
par same-IP query
loadRecentSignUpStats->>DB: SELECT 1 WHERE signUpIp = ? LIMIT sameIpLimit
DB-->>loadRecentSignUpStats: sameIpRows
and same-normalized-email query (NEW)
loadRecentSignUpStats->>DB: SELECT 1 WHERE signUpEmailNormalized = ? LIMIT sameEmailLimit
DB-->>loadRecentSignUpStats: sameEmailRows
and similar-email-base query
loadRecentSignUpStats->>DB: SELECT 1 WHERE signUpEmailBase = ? LIMIT similarEmailLimit
DB-->>loadRecentSignUpStats: similarEmailRows
end
loadRecentSignUpStats-->>calculateSignUpRiskAssessment: { sameIpCount, sameEmailCount, similarEmailCount }
calculateSignUpRiskAssessment-->>Caller: SignUpRiskAssessment (scores + heuristicFacts)
Reviews (3): Last reviewed commit: "Add index for normalized sign-up email i..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Enhances the backend sign-up risk assessment plumbing by extending “recent sign-up stats” to include same-normalized-email counts/limits, and updates dependency lockfile entries likely due to a submodule/dependency refresh.
Changes:
- Extend recent stats request/response types to include
signUpEmailNormalized,sameEmailLimit, andsameEmailCount. - Add a DB query to count recent sign-ups matching the same normalized email (bounded by
sameEmailLimit). - Update the in-source Vitest stubbed dependency return shape to match the new stats structure.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pnpm-lock.yaml | Updates dependency resolution (notably Nitro now pulling in rolldown) and removes an importer entry. |
| apps/backend/src/lib/risk-scores.tsx | Adds same-normalized-email recent stats fields and query; updates test stub return shape. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/lib/risk-scores.tsx`:
- Around line 83-93: The query in apps/backend/src/lib/risk-scores.tsx that uses
request.signUpEmailNormalized (inside the prisma.$replica().$queryRaw block that
checks sameEmailLimit) needs a corresponding DB index to avoid full table scans;
add the index to migration 20260308000002_finalize_signup_fraud_protection.sql
(CREATE INDEX CONCURRENTLY IF NOT EXISTS
"ProjectUser_signUpEmailNormalized_recent_idx" ON "ProjectUser"("tenancyId",
"isAnonymous", "signUpEmailNormalized", "signedUpAt")) and add the matching
Prisma index annotation to the ProjectUser model (@@index([tenancyId,
isAnonymous, signUpEmailNormalized, signedUpAt], name:
"ProjectUser_signUpEmailNormalized_recent_idx")) so the signUpEmailNormalized
filter in the sameEmailLimit query can use the index.
In `@apps/backend/src/private/implementation`:
- Line 1: Verify the private submodule update by checking the commit range
between a93d7ea0c0a91d7a4dfbc97c0032c9c9c68ec4d6 and
576f383b69a9593a9cff8d755c64c810aeeae239: run git fetch in the private repo and
git log --oneline a93d7ea..576f383b6 to enumerate commits, then run git diff
--name-status a93d7ea0c..576f383b6 to ensure only the intended risk-score and
signup-stat files were changed (no other files or code paths), inspect each
commit message and diff for breaking/security-impacting edits, run the private
repo’s test suite and any linters/security scanners on the new commits, and if
anything else appears, revert or request the correct commit range before
merging.
🪄 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
Run ID: 1840880f-79f3-4e1f-95d1-fd9e84e37754
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
apps/backend/src/lib/risk-scores.tsxapps/backend/src/private/implementation
- Introduced a new index on the ProjectUser model for the signUpEmailNormalized field, enhancing query performance related to user sign-up activities. - Updated the migration SQL to create the corresponding index in the database, ensuring efficient data retrieval for analytics and fraud protection measures.
- Removed the existing index for signUpEmailNormalized from the finalize_signup_fraud_protection migration. - Created a new migration to add the signUpEmailNormalized index, ensuring better organization and clarity in the migration history. - This change enhances the database schema by maintaining a clear separation of index creation across migrations.
Summary by CodeRabbit
New Features
Performance