Fix compatibility ranking and saved-search scalability#39
Conversation
|
Warning Review limit reached
More reviews will be available in 9 minutes and 22 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
Warning
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/cron/savedSearchAlert.js`:
- Around line 83-89: The current query uses the updated_searches CTE to set
saved_searches.last_alerted_at before calling enqueueNotificationsBulk, which
causes at-most-once delivery if enqueue fails; either document this guarantee or
change the flow so you first SELECT/RETURN the matched search rows (use the
matched_searches CTE or a RETURNING list), call enqueueNotificationsBulk with
that list, and only after enqueue succeeds run an UPDATE on saved_searches to
set last_alerted_at for the returned search_ids; reference the updated_searches
CTE, matched_searches, saved_searches, last_alerted_at and the
enqueueNotificationsBulk call and perform the UPDATE post-enqueue (or wrap
enqueue + UPDATE in a durable transaction/mechanism) to achieve at-least-once
delivery.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7dface08-bb50-4c67-bf42-bcc0c36e7fd5
📒 Files selected for processing (7)
migrations/010_saved_search_cap.sqlsrc/cron/savedSearchAlert.jssrc/services/listing.service.jssrc/services/savedSearch.service.jssrc/validators/listing.validators.jssrc/validators/pagination.validators.jssrc/workers/notificationQueue.js
|
@CodeRabbit review latest changes |
|
✅ Actions performedFull review triggered. |
|
@CodeRabbit review latest changes |
|
✅ Actions performedFull review triggered. |
Summary
Fixes three backend issues affecting listing ranking quality, saved-search alert scalability, and saved-search cap integrity.
Changes
cursorScoreandcursorId.cursorTimeandcursorId.Why
Previously,
sortBy=compatibilityonly reordered a recency-limited subset, so better matches could be missed. Saved-search alerts also performed one query per saved search, which would not scale. Saved-search creation used a non-atomic count-then-insert flow, allowing concurrent requests to exceed the cap.Validation
node --checkpassed for changed JS files.git diff --checkpassed.EXPLAIN.npm test -- --runInBandcould not run becausenode_modules/.bin/jestis missing.Summary by CodeRabbit
Release Notes
New Features
Improvements