Conversation
📝 WalkthroughWalkthroughThese changes optimize product similarity lookups by introducing a batched approach to reduce API calls. The productListingPage loader now uses the new withIsSimilarToBatched function that aggregates crossselling requests and batch-fetches product similarity data. A debug logging statement was added to fetchSafe. Changes
Sequence Diagram(s)sequenceDiagram
participant PLP as ProductListingPage
participant SIM as withIsSimilarToBatched
participant CS as CrossSelling API
participant PL as ProductList API
PLP->>SIM: products[]
activate SIM
SIM->>CS: fetch crossselling (parallel)
activate CS
CS-->>SIM: crossselling results[]
deactivate CS
SIM->>SIM: aggregate unique SKU IDs
Note over SIM: Batch productList calls<br/>(max 50 IDs per call)
loop For each batch
SIM->>PL: productList(skuIds batch)
activate PL
PL-->>SIM: products batch
deactivate PL
end
SIM->>SIM: map SKU/product for lookup<br/>distribute results to originals
SIM-->>PLP: products[] with isSimilarTo
deactivate SIM
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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
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 |
Tagging OptionsShould a new tag be published when this PR is merged?
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @vtex/utils/fetchVTEX.ts:
- Line 70: Remove the debug console.log in fetchVTEX.ts that prints "fetchSafe"
and the input; locate the call inside the fetchSafe (or fetchVTEX)
implementation and delete the console.log("fetchSafe", input) line so the
function no longer emits request details to stdout in production (optionally
replace with a guarded debug/logger call if you need non-production debugging).
🧹 Nitpick comments (1)
vtex/utils/similars.ts (1)
137-150: Unused fallback mapping by product ID.The mapping at lines 146-149 stores entries with
product:${productId}keys, but the lookup at lines 164-166 only retrieves by rawskuId. This fallback is never used.Either remove the dead code or implement the fallback lookup if it's needed.
Option 1: Remove unused fallback
// Step 4: Create a map of SKU ID -> Product for fast lookup const skuToProduct = new Map<string, Product>(); for (const product of allSimilarProducts) { // Map by SKU ID const skuId = product.sku; if (skuId) { skuToProduct.set(skuId, product); } - // Also map by product ID for fallback - const productId = product.inProductGroupWithID; - if (productId) { - skuToProduct.set(`product:${productId}`, product); - } }Option 2: Implement fallback lookup
const isSimilarTo = similarSkuIds - .map((skuId) => skuToProduct.get(skuId)) + .map((skuId) => skuToProduct.get(skuId) ?? skuToProduct.get(`product:${skuId}`)) .filter((p): p is Product => Boolean(p));
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
vtex/loaders/intelligentSearch/productListingPage.tsvtex/utils/fetchVTEX.tsvtex/utils/similars.ts
🧰 Additional context used
🧬 Code graph analysis (1)
vtex/utils/similars.ts (6)
vtex/utils/legacy.ts (1)
toSegmentParams(9-18)utils/fetch.ts (1)
STALE(61-63)vtex/utils/segment.ts (1)
withSegmentCookie(154-168)vtex/utils/transform.ts (1)
pickSku(101-114)vtex/utils/batch.ts (1)
batch(1-21)verified-reviews/loaders/productList.ts (1)
productList(10-39)
🔇 Additional comments (6)
vtex/utils/similars.ts (4)
42-70: Good optimization approach for reducing API calls.The batched approach is well-documented and should significantly reduce API calls from N to approximately ceil(totalSkuIds/50). The early return for empty products is appropriate.
72-93: Appropriate error handling for individual crossselling failures.The try/catch with fallback to empty
skuIdsensures that a failure for one product doesn't prevent others from getting their similar products. The non-null assertion on line 74 is safe given the filter at lines 64-66.
128-135: Good error logging for failed batches.Logging rejected promises while continuing with successful results is the right approach for resilience.
152-172: LGTM!The distribution logic correctly handles products without similars and avoids adding empty
isSimilarToarrays.vtex/loaders/intelligentSearch/productListingPage.ts (2)
19-19: LGTM!Import correctly updated to use the new batched function.
377-390: Clean refactoring to use batched similarity lookup.The two-step approach (transform, then optionally enrich) is clear and the inline comments explain the optimization benefit. This should reduce API calls significantly when
similarsis enabled.
| input: string | Request | URL, | ||
| init?: DecoRequestInit, | ||
| ) => { | ||
| console.log("fetchSafe", input); |
There was a problem hiding this comment.
Remove debug logging before merging.
This console.log will fire on every VTEX API request in production, generating significant log noise and potentially exposing sensitive URL parameters. Given this is a WIP PR, this appears to be debug code that should be removed before merging to main.
Suggested fix
export const fetchSafe = (
input: string | Request | URL,
init?: DecoRequestInit,
) => {
- console.log("fetchSafe", input);
return _fetchSafe(getSanitizedInput(input), init);
};🤖 Prompt for AI Agents
In @vtex/utils/fetchVTEX.ts at line 70, Remove the debug console.log in
fetchVTEX.ts that prints "fetchSafe" and the input; locate the call inside the
fetchSafe (or fetchVTEX) implementation and delete the console.log("fetchSafe",
input) line so the function no longer emits request details to stdout in
production (optionally replace with a guarded debug/logger call if you need
non-production debugging).
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.