feat(commerce): add hybrid search#308
Merged
Merged
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request introduces a new search feature for the commerce domain. It adds a search module with request validation, a controller, and a service that implements paginated search across products (via keyword and vector-based ranking), stores, and posts. The service normalizes inputs, handles cursor-based pagination, and returns strongly-typed results. ChangesCommerce Search Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
apps/backend/src/domains/commerce/search/search.service.ts (1)
292-296: ⚡ Quick winRun grouped searches in parallel for
type=all.These three queries are independent; sequential awaits add avoidable latency.
♻️ Suggested change
- return { - products: await this.searchProducts({ ...params, cursor: null }), - stores: await this.searchStores({ ...params, cursor: null }), - posts: await this.searchPosts({ ...params, cursor: null }), - }; + const [products, stores, posts] = await Promise.all([ + this.searchProducts({ ...params, cursor: null }), + this.searchStores({ ...params, cursor: null }), + this.searchPosts({ ...params, cursor: null }), + ]); + return { products, stores, posts };🤖 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 `@apps/backend/src/domains/commerce/search/search.service.ts` around lines 292 - 296, The current implementation performs three independent searches sequentially (searchProducts, searchStores, searchPosts) which increases latency; change the logic for the type=all branch to run these calls in parallel using Promise.all (or equivalent), await the combined promise once, and then map the returned tuple into the response object so products, stores, and posts are all filled from the parallel results.
🤖 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 `@apps/backend/src/domains/commerce/search/dto/search-query.dto.ts`:
- Around line 59-61: Rename the boolean query flag property in SearchQueryDto
from inStock to isInStock: update the property name (`@IsOptional`()
`@IsBooleanString`() inStock?: string) to isInStock and update all usages and
service contracts that accept or map this DTO (search services, controllers, and
any mappers/validators) so the DTO, method signatures, parameter names, and
downstream mappings consistently use isInStock; also update any tests and API
docs to reflect the new field name.
In `@apps/backend/src/domains/commerce/search/search.service.ts`:
- Around line 19-23: The SearchCursor interface (and other new cursor/result
interfaces) use a bare field name "id" which violates the project rule requiring
ID fields be suffixed with "Id"; rename the field in SearchCursor (and the other
introduced interfaces referenced in the comment) from id to a descriptive
<Entity>Id (e.g., productId, storeId, etc.), then update all code that
constructs, serializes/deserializes or reads these objects (e.g., any cursor
builders/parsers, search result mappers, and repository mapping functions) to
use the new <Entity>Id names; ensure tests and any JSON payload keys are updated
accordingly so callers and consumers use the new *Id names consistently.
- Around line 338-351: searchStores builds textFilter and accepts a normalized
params.location but never applies it to the Prisma query; update the where
passed to this.prisma.storeProfile.findMany (inside searchStores) to include a
location-based condition when params.location (or the normalized location
variable) is present—e.g. add a property like businessLocation / location: {
equals: params.location } or contains/match the normalized value depending on
your schema—so the location filter is combined with tier/isOpen/textFilter in
the where clause.
- Around line 705-715: The parseKobo function currently accepts arbitrarily
large BigInt values which can later fail at DB cast time; update parseKobo to
validate an upper bound before returning: after parsing to BigInt in parseKobo,
compare against a MAX_KOBO constant (e.g. 9223372036854775807n for signed
64-bit) and throw a clear error if parsed > MAX_KOBO (and keep the existing
negative check), so callers can produce a clean 400 rather than letting the DB
error; reference the parseKobo function and add a MAX_KOBO constant near it.
---
Nitpick comments:
In `@apps/backend/src/domains/commerce/search/search.service.ts`:
- Around line 292-296: The current implementation performs three independent
searches sequentially (searchProducts, searchStores, searchPosts) which
increases latency; change the logic for the type=all branch to run these calls
in parallel using Promise.all (or equivalent), await the combined promise once,
and then map the returned tuple into the response object so products, stores,
and posts are all filled from the parallel results.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b016643f-5a16-418c-a03c-25141791d735
📒 Files selected for processing (5)
apps/backend/src/domains/commerce.module.tsapps/backend/src/domains/commerce/search/dto/search-query.dto.tsapps/backend/src/domains/commerce/search/search.controller.tsapps/backend/src/domains/commerce/search/search.module.tsapps/backend/src/domains/commerce/search/search.service.ts
Comment on lines
+59
to
+61
| @IsOptional() | ||
| @IsBooleanString() | ||
| inStock?: string; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Rename boolean query flag to follow the API naming convention.
inStock should be renamed to isInStock (and propagated through service contracts) to match the backend boolean-field rule.
As per coding guidelines: "All boolean field names must be prefixed with 'is', 'has', 'can', or 'should'."
🤖 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 `@apps/backend/src/domains/commerce/search/dto/search-query.dto.ts` around
lines 59 - 61, Rename the boolean query flag property in SearchQueryDto from
inStock to isInStock: update the property name (`@IsOptional`() `@IsBooleanString`()
inStock?: string) to isInStock and update all usages and service contracts that
accept or map this DTO (search services, controllers, and any
mappers/validators) so the DTO, method signatures, parameter names, and
downstream mappings consistently use isInStock; also update any tests and API
docs to reflect the new field name.
Comment on lines
+19
to
+23
| interface SearchCursor { | ||
| type: SingleSearchType; | ||
| id: string; | ||
| score?: number; | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Use *Id names in new contracts instead of bare id.
The new cursor/result interfaces introduce multiple id fields that don’t follow the repository naming rule (...Id).
As per coding guidelines: "All ID field names must be suffixed with 'Id' (e.g. storeId, productId, orderId)."
Also applies to: 65-67, 83-84, 95-96, 99-101, 106-107, 114-115
🤖 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 `@apps/backend/src/domains/commerce/search/search.service.ts` around lines 19 -
23, The SearchCursor interface (and other new cursor/result interfaces) use a
bare field name "id" which violates the project rule requiring ID fields be
suffixed with "Id"; rename the field in SearchCursor (and the other introduced
interfaces referenced in the comment) from id to a descriptive <Entity>Id (e.g.,
productId, storeId, etc.), then update all code that constructs,
serializes/deserializes or reads these objects (e.g., any cursor
builders/parsers, search result mappers, and repository mapping functions) to
use the new <Entity>Id names; ensure tests and any JSON payload keys are updated
accordingly so callers and consumers use the new *Id names consistently.
onerandomdevv
approved these changes
May 22, 2026
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
2 participants
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.
What does this PR do?
Implements B-25 — Hybrid Search. This adds a public
GET /searchendpoint for product, store, and post search with cursor pagination, safe response mappers, Tier 0 exclusion, keyword fallback for public text search, and an internal optional pgvector path for future WhatsApp/image search callers.Task
B-25 — Hybrid Search: pgvector + tsvector + metadata
What changed
GET /searchPrisma.sqlFiles changed
apps/backend/src/domains/commerce.module.tsapps/backend/src/domains/commerce/search/dto/search-query.dto.tsapps/backend/src/domains/commerce/search/search.controller.tsapps/backend/src/domains/commerce/search/search.module.tsapps/backend/src/domains/commerce/search/search.service.tsVerification
pnpm.cmd run lint— PASSnpx.cmd tsc --noEmit— PASSpnpm.cmd run build— PASSSearch behavior
VertexClientqueryVectorpath throughSearchService.searchProducts({ queryVector })Prisma.sqlScope control
.env.localwas not staged or committedType of change
Area affected
How to test this
GET /search?q=shirt&type=products.GET /search?q=lagos&type=stores.GET /search?q=style&type=posts.GET /search?q=shirt&type=all.category,minPrice,maxPrice, andinStock.nextCursorvalues.Expected result:
Pre-commit checklist
console.logleft in production code.envfiles committedanytypes addeddb pushScreenshots
N/A — backend-only PR. No UI changes.
Notes for reviewer
Because the current
VertexClientonly supportsgenerateEmbedding(imageBuffer, text), public text search intentionally uses keyword fallback and does not generate query vectors at request time. The vector path is implemented as an internal optional service path for future WhatsApp/image search callers that already have a valid query vector.Summary by CodeRabbit