Conversation
📝 WalkthroughWalkthroughAdds an event feed system: DB schema and model for event-post relationships, repositories and services for tagging and similarity processing, API endpoints to list tags and fetch paginated event feeds, an hourly cron to compute similarity, and comprehensive integration tests. Changes
Sequence DiagramssequenceDiagram
actor User
participant API as EventController
participant Service as EventService
participant EPRepo as EventPostRepository
participant ETRepo as EventTagRepository
participant PostRepo as PostRepository
participant DB as Database
User->>API: GET /event/{eventTagId}/posts?page=1&limit=10&source=user
API->>Service: getEventPosts(user, eventTagId, page, limit, source)
Service->>ETRepo: findOne(eventTagId)
ETRepo->>DB: SELECT event_tag
DB-->>ETRepo: event_tag
ETRepo-->>Service: event_tag
par fetch list and count
Service->>EPRepo: getPostsForEvent(eventTagId, userId, source, skip, limit)
EPRepo->>DB: Query eventPosts join posts/users...
DB-->>EPRepo: rows
EPRepo-->>Service: posts[]
and
Service->>EPRepo: getPostCountForEvent(eventTagId, userId, source)
EPRepo->>DB: COUNT query
DB-->>EPRepo: count
EPRepo-->>Service: total
end
Service->>PostRepo: getBlockedUserIds(user)
PostRepo->>DB: SELECT blocked ids
DB-->>PostRepo: blocked ids
PostRepo-->>Service: blocked ids
Service->>Service: filter out inactive/blocked, attach source/relevance
Service-->>API: GetEventPostsResponse
API-->>User: { posts[], total, page, limit }
sequenceDiagram
participant Cron as EventSimilarityCron
participant ETRepo as EventTagRepository
participant EPSvc as EventPostService
participant EPRepo as EventPostRepository
participant PostRepo as PostRepository
participant DB as Database
Cron->>ETRepo: getAllEventTags()
ETRepo->>DB: SELECT * FROM event_tags ORDER BY name
DB-->>ETRepo: tags[]
ETRepo-->>Cron: tags[]
loop per tag
Cron->>EPSvc: processEventSimilarity(tagId)
EPSvc->>EPRepo: getUserTaggedPostsForEvent(tagId)
EPRepo->>DB: SELECT user-sourced eventPosts with post embeddings
DB-->>EPRepo: anchor posts
EPRepo-->>EPSvc: anchors[]
EPSvc->>EPSvc: computeCentroid(anchors.embeddings)
EPSvc->>PostRepo: findSimilarPostsForEvent(centroid, exclude, threshold, limit)
PostRepo->>DB: pgvector similarity query -> {id,score}[]
DB-->>PostRepo: results
PostRepo-->>EPSvc: similar posts with scores
EPSvc->>EPRepo: deleteRelationshipsBySourceForEvent(tagId,"similarity")
EPRepo->>DB: DELETE ...
DB-->>EPRepo: ok
loop per similar
EPSvc->>EPRepo: upsertRelationship(postId, tagId, "similarity", score)
EPRepo->>DB: INSERT/UPDATE
DB-->>EPRepo: row
end
EPSvc-->>Cron: processed tagId
end
Cron-->>Cron: complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
src/app.ts (1)
29-29: Consider single-run protection for event similarity cron startup.Line 203 starts the cron unconditionally on each app instance. In multi-replica deployments, this can trigger duplicate hourly jobs and unnecessary DB load. Consider gating with an env flag or leader/lock-based scheduling.
Also applies to: 203-203
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app.ts` at line 29, The cron is being started unconditionally via startEventSimilarityCron which causes duplicate jobs in multi-replica deployments; modify the startup to guard the call to startEventSimilarityCron by either (A) honoring an environment flag (e.g., ENABLE_EVENT_SIMILARITY_CRON) and only calling startEventSimilarityCron when that flag is true, or (B) acquiring a leader/distributed lock (Redis/DB advisory lock) at startup and only calling startEventSimilarityCron if the lock is obtained; update startup logic around startEventSimilarityCron to check the chosen guard and log whether the instance is running the cron or skipping it.src/repositories/EventPostRepository.ts (2)
41-48: Hardcoded string'user'should use theEventPostSourcetype.Line 46 uses a hardcoded string
'user'instead of referencing a constant. While functionally correct, using a typed constant would be more maintainable.♻️ Suggested fix
- .andWhere("epr.source = 'user'") + .andWhere("epr.source = :source", { source: 'user' as EventPostSource })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repositories/EventPostRepository.ts` around lines 41 - 48, The query in getUserTaggedPostsForEvent uses the hardcoded string 'user'; replace it with the typed constant from EventPostSource to improve maintainability: import EventPostSource and change the .andWhere clause to use a parameterized value (e.g. .andWhere("epr.source = :source", { source: EventPostSource.User })) so the code references EventPostSource instead of the literal string.
79-84: Remove redundant type annotations.ESLint correctly flags that
skip: number = 0andlimit: number = 10have types that can be trivially inferred from the default values.♻️ Proposed fix
public async getPostsForEvent( eventTagId: string, source?: EventPostSource, - skip: number = 0, - limit: number = 10, + skip = 0, + limit = 10, ): Promise<EventPostModel[]> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repositories/EventPostRepository.ts` around lines 79 - 84, The method signature for getPostsForEvent in EventPostRepository.ts has redundant type annotations on parameters skip: number = 0 and limit: number = 10; remove the explicit ": number" annotations so the types are inferred from the default values (i.e., change to skip = 0, limit = 10) while keeping the parameter names, defaults, return type Promise<EventPostModel[]>, and the rest of the method intact; locate the signature in the getPostsForEvent method and update it accordingly.src/services/EventPostService.ts (3)
87-98: Missing dimension validation incomputeCentroid.If embeddings have mismatched dimensions, the loop at line 92-94 will silently produce incorrect results (shorter embeddings won't contribute to higher indices). While the caller filters to posts with embeddings, there's no guarantee they all have the same dimension.
🛡️ Proposed defensive check
private computeCentroid(embeddings: number[][]): number[] { const dim = embeddings[0].length; + if (embeddings.some(e => e.length !== dim)) { + throw new Error(`Embedding dimension mismatch: expected ${dim}`); + } const centroid = new Array(dim).fill(0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/EventPostService.ts` around lines 87 - 98, computeCentroid lacks validation for embedding dimensionality which allows mismatched-length arrays to silently produce wrong results; update computeCentroid to first assert embeddings is non-empty, capture const dim = embeddings[0].length, then iterate embeddings and throw a descriptive Error (or return a safe failure) if any embedding.length !== dim (include the offending index and length in the message), only then proceed to sum and average so all vectors contribute correctly.
24-26: Test environment skip pattern may hide integration bugs.Skipping the entire method when
NODE_ENV === "test"prevents unit testing of this logic. The test file works around this by temporarily overridingNODE_ENV, but this pattern is fragile.Consider injecting a configuration flag or making the skip behavior configurable to allow proper testing without environment manipulation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/EventPostService.ts` around lines 24 - 26, The method processEventSimilarity in EventPostService currently returns early based on process.env.NODE_ENV === "test"; change this to use an injectable/configurable flag so tests can control the behavior without mutating NODE_ENV: add a constructor parameter (e.g., skipEventSimilarityInTest: boolean or a ConfigService boolean like config.skipEventSimilarity) stored on the EventPostService instance (or read from ConfigService), defaulting to the existing NODE_ENV check for backwards compatibility, then replace the direct process.env check in processEventSimilarity with a check against that injected flag and update callers/tests to pass the desired value.
71-78: Sequential upserts could be batched for performance.The loop issues individual
upsertRelationshipcalls sequentially. For large result sets (up toMAX_RESULTS = 100), this creates many round trips. Consider batching these operations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/EventPostService.ts` around lines 71 - 78, The loop in EventPostService that iterates over similarPosts and calls eventPostRepo.upsertRelationship sequentially (up to MAX_RESULTS) causes many round trips; change this to batch the operations by adding a repository method (e.g., eventPostRepo.upsertRelationshipsBulk or upsertRelationshipBatch) that accepts an array of {postId, eventTagId, type, score} and performs a single bulk upsert or transaction, or if DB/binding lacks bulk support, run the existing upserts in parallel with controlled concurrency (Promise.all with a p-limit) to avoid sequential awaits; update the code in the similarPosts handling block to prepare the array of relationships from similarPosts and call the new bulk/parallel mechanism instead of awaiting each eventPostRepo.upsertRelationship inside the for loop.src/tests/EventTest.test.ts (1)
479-481: Type assertion suggests a type mismatch.The
source: "similarity" as anycast indicates TypeORM'sfindoptions may not properly recognize theEventPostSourceliteral type. This works but is a code smell.Consider defining a type-safe query interface or using query builder for better type safety.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tests/EventTest.test.ts` around lines 479 - 481, The test is using a loose cast "similarity" as any when calling conn.manager.find for EventPostModel; replace the unsafe any cast with a type-safe value by using the actual EventPostSource literal/enum or a proper typed query: ensure the string "similarity" is typed as EventPostSource (or import/construct the enum/union used by EventPostModel) when calling conn.manager.find (the simRows lookup), or switch to conn.createQueryBuilder/EventPostModel with an explicit where clause to avoid the type assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/repositories/PostRepository.ts`:
- Around line 555-572: In findSimilarPostsForEvent add a guard to exclude empty
embeddings before casting by ensuring array_length(post.embedding,1) = 512 (or
>0 / =512 as appropriate) in the WHERE clauses that use CAST(... AS vector(512))
so empty arrays ([]) are filtered out; update the scoreExpr usage and the
.andWhere chain in the method to include this array_length check alongside
"post.embedding IS NOT NULL". Apply the same array_length guard to the other
vector-distance queries: PostRepository.findSimilarPosts (both places it
builds/uses the vector cast) and the second ordering clause in
PostRepository.findSimilarPostsForEvent, and to
RequestRepository.findSimilarRequests so pgvector never receives an empty array
and avoids the dimension mismatch error.
In `@src/services/EventService.ts`:
- Around line 41-68: The current code applies active/blocked filtering after
calling eventPostRepo.getPostsForEvent and getPostCountForEvent, which causes
pagination and total to include rows that should be hidden; modify the repo
calls so filtering happens in the DB layer: retrieve blocked UIDs from
userRepo.getUserWithBlockedInfo(user.firebaseUid) first, then call
eventPostRepo.getPostsForEvent(eventTagId, source, skip, limit, {excludeUids:
blockedUids, requireActiveUser: true}) and
eventPostRepo.getPostCountForEvent(eventTagId, source, {excludeUids:
blockedUids, requireActiveUser: true}) (or add equivalent parameters/overloads)
so both the SELECT and COUNT apply the same predicates; remove the post-level
isActive/blocked filtering in EventService and keep only mapping to
PostWithSource for attaching source/relevanceScore.
In `@src/services/PostService.ts`:
- Around line 553-562: When removing the manual event tags in PostService (use
removeEventTagsRequest.eventTags, removedTags,
eventPostRepository.deleteRelationship and post.eventTags), ensure you also
clear any leftover similarity rows when the last user-tagged seed for that event
is gone: after deleting each (post.id, tag.id) and updating post.eventTags,
check if there are no remaining user-tagged posts or embeddings for the event
and then call the repository method that removes similarity rows
(eventPostRepository.deleteRelationshipsBySourceForEvent or the EventPostService
equivalent) for that event/source before saving the post; this mirrors the
cleanup logic found in EventPostService.ts (deleteRelationshipsBySourceForEvent)
to prevent stale event matches.
---
Nitpick comments:
In `@src/app.ts`:
- Line 29: The cron is being started unconditionally via
startEventSimilarityCron which causes duplicate jobs in multi-replica
deployments; modify the startup to guard the call to startEventSimilarityCron by
either (A) honoring an environment flag (e.g., ENABLE_EVENT_SIMILARITY_CRON) and
only calling startEventSimilarityCron when that flag is true, or (B) acquiring a
leader/distributed lock (Redis/DB advisory lock) at startup and only calling
startEventSimilarityCron if the lock is obtained; update startup logic around
startEventSimilarityCron to check the chosen guard and log whether the instance
is running the cron or skipping it.
In `@src/repositories/EventPostRepository.ts`:
- Around line 41-48: The query in getUserTaggedPostsForEvent uses the hardcoded
string 'user'; replace it with the typed constant from EventPostSource to
improve maintainability: import EventPostSource and change the .andWhere clause
to use a parameterized value (e.g. .andWhere("epr.source = :source", { source:
EventPostSource.User })) so the code references EventPostSource instead of the
literal string.
- Around line 79-84: The method signature for getPostsForEvent in
EventPostRepository.ts has redundant type annotations on parameters skip: number
= 0 and limit: number = 10; remove the explicit ": number" annotations so the
types are inferred from the default values (i.e., change to skip = 0, limit =
10) while keeping the parameter names, defaults, return type
Promise<EventPostModel[]>, and the rest of the method intact; locate the
signature in the getPostsForEvent method and update it accordingly.
In `@src/services/EventPostService.ts`:
- Around line 87-98: computeCentroid lacks validation for embedding
dimensionality which allows mismatched-length arrays to silently produce wrong
results; update computeCentroid to first assert embeddings is non-empty, capture
const dim = embeddings[0].length, then iterate embeddings and throw a
descriptive Error (or return a safe failure) if any embedding.length !== dim
(include the offending index and length in the message), only then proceed to
sum and average so all vectors contribute correctly.
- Around line 24-26: The method processEventSimilarity in EventPostService
currently returns early based on process.env.NODE_ENV === "test"; change this to
use an injectable/configurable flag so tests can control the behavior without
mutating NODE_ENV: add a constructor parameter (e.g., skipEventSimilarityInTest:
boolean or a ConfigService boolean like config.skipEventSimilarity) stored on
the EventPostService instance (or read from ConfigService), defaulting to the
existing NODE_ENV check for backwards compatibility, then replace the direct
process.env check in processEventSimilarity with a check against that injected
flag and update callers/tests to pass the desired value.
- Around line 71-78: The loop in EventPostService that iterates over
similarPosts and calls eventPostRepo.upsertRelationship sequentially (up to
MAX_RESULTS) causes many round trips; change this to batch the operations by
adding a repository method (e.g., eventPostRepo.upsertRelationshipsBulk or
upsertRelationshipBatch) that accepts an array of {postId, eventTagId, type,
score} and performs a single bulk upsert or transaction, or if DB/binding lacks
bulk support, run the existing upserts in parallel with controlled concurrency
(Promise.all with a p-limit) to avoid sequential awaits; update the code in the
similarPosts handling block to prepare the array of relationships from
similarPosts and call the new bulk/parallel mechanism instead of awaiting each
eventPostRepo.upsertRelationship inside the for loop.
In `@src/tests/EventTest.test.ts`:
- Around line 479-481: The test is using a loose cast "similarity" as any when
calling conn.manager.find for EventPostModel; replace the unsafe any cast with a
type-safe value by using the actual EventPostSource literal/enum or a proper
typed query: ensure the string "similarity" is typed as EventPostSource (or
import/construct the enum/union used by EventPostModel) when calling
conn.manager.find (the simRows lookup), or switch to
conn.createQueryBuilder/EventPostModel with an explicit where clause to avoid
the type assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 08335a9d-5164-42c9-bf02-4aacc68f66fc
📒 Files selected for processing (22)
.gitignorejest.config.jssrc/api/controllers/EventController.tssrc/api/controllers/index.tssrc/app.tssrc/cron/eventSimilarityCron.tssrc/migrations/1769700000000-CreateEventPost.tssrc/models/EventPostModel.tssrc/models/EventTagModel.tssrc/models/index.tssrc/repositories/EventPostRepository.tssrc/repositories/EventTagRepository.tssrc/repositories/PostRepository.tssrc/repositories/index.tssrc/services/EventPostService.tssrc/services/EventService.tssrc/services/PostService.tssrc/tests/EventTest.test.tssrc/tests/controllers/ControllerFactory.tssrc/tests/data/DatabaseConnection.tssrc/types/ApiResponses.tsswagger.json
JoshD94
left a comment
There was a problem hiding this comment.
lgtm after coderabbit fixes are added
Xhether
left a comment
There was a problem hiding this comment.
Lgtm after code rabbit comments fixed
| /** | ||
| * Hourly cron that reprocesses similarity for every event tag. | ||
| * For each tag it computes a centroid from user-tagged post embeddings | ||
| * and upserts similar posts into the eventPosts table. | ||
| */ |
There was a problem hiding this comment.
Seems fine but I'm curious why a cron job is needed here
There was a problem hiding this comment.
Instead of reprocessing similarity on all posts in an event every time a post is added or edited, we just reprocess similarity every hour for every event if that makes sense?
| export type EventPostSource = "user" | "similarity" | "nlp_context"; | ||
|
|
||
| @Entity("eventPosts") | ||
| @Unique(["postId", "eventTagId"]) // Enforces one row per post per event, so ML layer can't insert another row if event already user-tagged |
There was a problem hiding this comment.
Should probably move the comment above or below this line for readability
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/repositories/RequestRepository.ts (1)
130-137:⚠️ Potential issue | 🟠 MajorSkip archived requests when generating similarity matches.
src/services/PostService.ts:90-135calls this right after a post is created. Withoutrequest.archive = false, archived requests can keep receiving new matches.Suggested fix
.where("request.embedding IS NOT NULL") .andWhere("array_length(request.embedding, 1) = 512") + .andWhere("request.archive = false") .andWhere("user.firebaseUid != :excludeUserId", { excludeUserId }) .setParameter("embedding", embeddingString)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repositories/RequestRepository.ts` around lines 130 - 137, The similarity query is currently including archived requests; update the query chain that uses request.embedding, user.firebaseUid, embeddingString, orderBy("distance"), and take(limit) to exclude archived items by adding a condition like request.archive = false (e.g., an additional .andWhere("request.archive = false") before setting parameters/order/take) so archived requests are skipped when generating similarity matches.src/repositories/PostRepository.ts (1)
543-550:⚠️ Potential issue | 🟠 MajorExclude archived and sold posts from
findSimilarPosts().
src/services/RequestService.ts:67-86uses this query to populate matches for a new request. Since it only filters on embedding shape and owner, archived or already-sold listings can still be returned as “similar” posts.Suggested fix
return await this.repository .createQueryBuilder("post") .leftJoinAndSelect("post.user", "user") .where("post.embedding IS NOT NULL") .andWhere("array_length(post.embedding, 1) = 512") + .andWhere("post.archive = false") + .andWhere("post.sold = false") .andWhere("user.firebaseUid != :excludeUserId", { excludeUserId }) .orderBy(`post.embedding::vector <-> CAST('${lit}' AS vector(512))`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repositories/PostRepository.ts` around lines 543 - 550, The query in PostRepository.findSimilarPosts returns archived or sold listings; update the query builder chain (the createQueryBuilder("post") -> leftJoinAndSelect("post.user", "user") sequence) to add filters excluding archived and sold posts by adding AND conditions such as checking a boolean archived flag (e.g., post.isArchived = false) and a sold/status check (e.g., post.status != 'SOLD' or post.isSold = false depending on your schema), ensuring these .andWhere clauses are placed before orderBy/limit so archived/sold posts are never returned as similar matches.
🧹 Nitpick comments (2)
src/repositories/EventPostRepository.ts (1)
83-84: Remove redundant type annotations.Static analysis flagged trivially inferrable types on default parameter values.
♻️ Proposed fix
public async getPostsForEvent( eventTagId: string, requestingUserId: string, source?: EventPostSource, - skip: number = 0, - limit: number = 10, + skip = 0, + limit = 10, ): Promise<EventPostModel[]> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repositories/EventPostRepository.ts` around lines 83 - 84, Remove the redundant explicit type annotations on the default parameters in EventPostRepository: drop ": number" from the skip and limit default values (e.g., change "skip: number = 0" and "limit: number = 10" to "skip = 0" and "limit = 10") so the types are inferred; update the method signature(s) in EventPostRepository where skip and limit are declared accordingly.src/services/EventService.ts (1)
28-29: Remove redundant type annotations.Static analysis flagged trivially inferrable types on default parameter values.
♻️ Proposed fix
public async getEventPosts( user: UserModel, eventTagId: string, - page: number = 1, - limit: number = 10, + page = 1, + limit = 10, source?: EventPostSource, ): Promise<GetEventPostsResponse> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/EventService.ts` around lines 28 - 29, Remove the redundant explicit type annotations on the defaulted parameters `page: number = 1` and `limit: number = 10` in EventService (replace `page: number = 1` and `limit: number = 10` with `page = 1` and `limit = 10`) so TypeScript can infer the types from the default values; locate the parameters named `page` and `limit` in the affected method(s) in src/services/EventService.ts (e.g., the event listing/pagination method) and update the function signature accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/repositories/EventPostRepository.ts`:
- Around line 11-35: The upsertRelationship method has a race between the read
(existing) and insert that causes unique constraint failures; replace the
read-modify-write with an atomic upsert using TypeORM's upsert or a raw INSERT
... ON CONFLICT to perform insert-or-update in one DB statement, and encode the
"user source takes priority" rule into the conflict update (e.g., ON CONFLICT
(postId,eventTagId) DO UPDATE SET source = EXCLUDED.source, relevanceScore =
EXCLUDED.relevanceScore WHERE NOT (existing.source = 'user' AND EXCLUDED.source
<> 'user')); use the repository.upsert or
repository.createQueryBuilder().insert().onConflict(...) targeting the same
table/entity as this.repository and ensure the method returns the persisted
EventPostModel (fetching the row after upsert if necessary).
In `@src/repositories/PostRepository.ts`:
- Around line 567-583: The hourly similarity query in PostRepository (the query
built around scoreExpr selecting post.id and post.embedding) needs an ANN index
on Post.embedding so EventPostService.processEventSimilarity doesn't full-scan;
add a migration that creates a pgvector ivfflat (or hnsw) index on the
Post.embedding column (e.g., CREATE INDEX post_embedding_idx ON "Post" USING
ivfflat (embedding vector_cosine_ops)), ensure the pgvector extension exists in
the migration and configure any ivfflat lists parameter as appropriate; after
adding the migration, confirm the index name (post_embedding_idx) and that
Post.embedding remains type vector so the existing scoreExpr queries benefit
from the index.
---
Outside diff comments:
In `@src/repositories/PostRepository.ts`:
- Around line 543-550: The query in PostRepository.findSimilarPosts returns
archived or sold listings; update the query builder chain (the
createQueryBuilder("post") -> leftJoinAndSelect("post.user", "user") sequence)
to add filters excluding archived and sold posts by adding AND conditions such
as checking a boolean archived flag (e.g., post.isArchived = false) and a
sold/status check (e.g., post.status != 'SOLD' or post.isSold = false depending
on your schema), ensuring these .andWhere clauses are placed before
orderBy/limit so archived/sold posts are never returned as similar matches.
In `@src/repositories/RequestRepository.ts`:
- Around line 130-137: The similarity query is currently including archived
requests; update the query chain that uses request.embedding, user.firebaseUid,
embeddingString, orderBy("distance"), and take(limit) to exclude archived items
by adding a condition like request.archive = false (e.g., an additional
.andWhere("request.archive = false") before setting parameters/order/take) so
archived requests are skipped when generating similarity matches.
---
Nitpick comments:
In `@src/repositories/EventPostRepository.ts`:
- Around line 83-84: Remove the redundant explicit type annotations on the
default parameters in EventPostRepository: drop ": number" from the skip and
limit default values (e.g., change "skip: number = 0" and "limit: number = 10"
to "skip = 0" and "limit = 10") so the types are inferred; update the method
signature(s) in EventPostRepository where skip and limit are declared
accordingly.
In `@src/services/EventService.ts`:
- Around line 28-29: Remove the redundant explicit type annotations on the
defaulted parameters `page: number = 1` and `limit: number = 10` in EventService
(replace `page: number = 1` and `limit: number = 10` with `page = 1` and `limit
= 10`) so TypeScript can infer the types from the default values; locate the
parameters named `page` and `limit` in the affected method(s) in
src/services/EventService.ts (e.g., the event listing/pagination method) and
update the function signature accordingly.
🪄 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: 5a1b3ea2-bd4b-41e1-a891-58edd8a68a45
📒 Files selected for processing (6)
src/models/EventPostModel.tssrc/repositories/EventPostRepository.tssrc/repositories/PostRepository.tssrc/repositories/RequestRepository.tssrc/services/EventService.tssrc/services/PostService.ts
| public async upsertRelationship( | ||
| postId: string, | ||
| eventTagId: string, | ||
| source: EventPostSource, | ||
| relevanceScore: number | null, | ||
| ): Promise<EventPostModel> { | ||
| const existing = await this.repository | ||
| .createQueryBuilder("epr") | ||
| .where("epr.postId = :postId", { postId }) | ||
| .andWhere("epr.eventTagId = :eventTagId", { eventTagId }) | ||
| .getOne(); | ||
|
|
||
| // User source takes priority (an existing user tag is never overwritten by an ML source) | ||
| if (existing) { | ||
| if (existing.source === 'user' && source !== 'user') { | ||
| return existing; | ||
| } | ||
| existing.source = source; | ||
| existing.relevanceScore = relevanceScore; | ||
| return await this.repository.save(existing); | ||
| } | ||
|
|
||
| const relationship = this.repository.create({ postId, eventTagId, source, relevanceScore }); | ||
| return await this.repository.save(relationship); | ||
| } |
There was a problem hiding this comment.
Race condition in read-modify-write pattern.
Two concurrent calls with the same (postId, eventTagId) can both read existing = null, then both attempt to insert, causing a unique constraint violation and an unhandled exception.
Use TypeORM's upsert or a raw ON CONFLICT clause to make this atomic:
🔧 Proposed fix using TypeORM upsert
public async upsertRelationship(
postId: string,
eventTagId: string,
source: EventPostSource,
relevanceScore: number | null,
): Promise<EventPostModel> {
- const existing = await this.repository
- .createQueryBuilder("epr")
- .where("epr.postId = :postId", { postId })
- .andWhere("epr.eventTagId = :eventTagId", { eventTagId })
- .getOne();
-
- // User source takes priority (an existing user tag is never overwritten by an ML source)
- if (existing) {
- if (existing.source === 'user' && source !== 'user') {
- return existing;
- }
- existing.source = source;
- existing.relevanceScore = relevanceScore;
- return await this.repository.save(existing);
- }
-
- const relationship = this.repository.create({ postId, eventTagId, source, relevanceScore });
- return await this.repository.save(relationship);
+ // Atomic upsert with source precedence: user tags are never overwritten by ML sources
+ await this.repository
+ .createQueryBuilder()
+ .insert()
+ .into(EventPostModel)
+ .values({ postId, eventTagId, source, relevanceScore })
+ .orUpdate(
+ ['source', 'relevanceScore', 'updatedAt'],
+ ['postId', 'eventTagId'],
+ { skipUpdateIfNoValuesChanged: true }
+ )
+ // Preserve user source: only update if existing source != 'user' OR incoming source = 'user'
+ .setParameter('source', source)
+ .execute();
+
+ // For user-source precedence, use raw SQL with conditional update:
+ // ON CONFLICT (postId, eventTagId) DO UPDATE SET source = :source, relevanceScore = :relevanceScore
+ // WHERE eventPosts.source != 'user' OR :source = 'user'
+
+ return await this.repository.findOneOrFail({ where: { postId, eventTagId } });
}Note: The conditional "user source takes priority" logic requires a WHERE clause in the ON CONFLICT DO UPDATE, which may need raw SQL depending on your TypeORM version.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/repositories/EventPostRepository.ts` around lines 11 - 35, The
upsertRelationship method has a race between the read (existing) and insert that
causes unique constraint failures; replace the read-modify-write with an atomic
upsert using TypeORM's upsert or a raw INSERT ... ON CONFLICT to perform
insert-or-update in one DB statement, and encode the "user source takes
priority" rule into the conflict update (e.g., ON CONFLICT (postId,eventTagId)
DO UPDATE SET source = EXCLUDED.source, relevanceScore = EXCLUDED.relevanceScore
WHERE NOT (existing.source = 'user' AND EXCLUDED.source <> 'user')); use the
repository.upsert or repository.createQueryBuilder().insert().onConflict(...)
targeting the same table/entity as this.repository and ensure the method returns
the persisted EventPostModel (fetching the row after upsert if necessary).
| // get IDs and scores with threshold filter + pagination | ||
| const qb = this.repository | ||
| .createQueryBuilder("post") | ||
| .select("post.id", "id") | ||
| .addSelect(scoreExpr, "score") | ||
| .where("post.embedding IS NOT NULL") | ||
| .andWhere("array_length(post.embedding, 1) = 512") | ||
| .andWhere("post.archive = false") | ||
| .andWhere("post.sold = false") | ||
| .andWhere(`${scoreExpr} >= :threshold`, { threshold }); | ||
|
|
||
| if (excludePostIds.length > 0) { | ||
| qb.andWhere("post.id NOT IN (:...excludePostIds)", { excludePostIds }); | ||
| } | ||
|
|
||
| qb.orderBy("score", "DESC").limit(limit); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 '@Index|CREATE INDEX|ivfflat|hnsw|vector_cosine_ops|embedding' srcRepository: cuappdev/resell-backend
Length of output: 44225
Confirm this hourly similarity query is backed by a pgvector ANN index.
EventPostService.processEventSimilarity() drives similarity scoring on the hourly cron. The Post.embedding column exists but currently lacks an ivfflat/hnsw index. Without it, the cosine-distance queries at lines 567–583 will degrade into full table scans of active posts for every event tag.
The migration creating the embedding columns (1740007049335-pgvector.ts) has the ivfflat index creation commented out as optional. Meanwhile, an ivfflat index with vector_cosine_ops was added for the searches table, but not for Post embeddings.
Expected: Create an ANN index for efficient cosine-distance search on Post.embedding (e.g., CREATE INDEX post_embedding_idx ON "Post" USING ivfflat (embedding vector_cosine_ops)).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/repositories/PostRepository.ts` around lines 567 - 583, The hourly
similarity query in PostRepository (the query built around scoreExpr selecting
post.id and post.embedding) needs an ANN index on Post.embedding so
EventPostService.processEventSimilarity doesn't full-scan; add a migration that
creates a pgvector ivfflat (or hnsw) index on the Post.embedding column (e.g.,
CREATE INDEX post_embedding_idx ON "Post" USING ivfflat (embedding
vector_cosine_ops)), ensure the pgvector extension exists in the migration and
configure any ivfflat lists parameter as appropriate; after adding the
migration, confirm the index name (post_embedding_idx) and that Post.embedding
remains type vector so the existing scoreExpr queries benefit from the index.
ashleynhs
left a comment
There was a problem hiding this comment.
lgtm. fix merge conflicts and then we can merge.
Overview
EventPostModelto track post-event relationships with source (user/similarity/nlp_context) and relevanceScoreGET /events/available-for-tagging/andGET /events/:eventId/posts/endpointsTest Coverage
EventTest.test.tscovering event feed reads, pagination, source filtering, and user blockingSummary by CodeRabbit
New Features
Documentation
Tests
Chores