add an in-memory LRU cache implementation and integrate it into exaSearch (cache checks before network, set on success). Expected outcome: short-lived cache keyed by request body, metrics for cache hits/misses, and eviction when capacity reached.#10
Conversation
|
@rajarshidattapy is attempting to deploy a commit to the adityap700's projects Team on Vercel. A member of the Team first needs to authorize it. |
Summary of ChangesHello @rajarshidattapy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant performance and cost optimization by integrating a short-lived, in-memory LRU cache for Exa API requests. The primary goal is to minimize redundant network calls for identical queries, thereby reducing latency and API costs. The cache is highly configurable through environment variables, allowing fine-grained control over its behavior, and includes verbose logging to provide insights into its operation. Additionally, the project's Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces an in-memory LRU cache for Exa API requests, which is a great initiative to improve performance and reduce costs. The implementation is mostly solid, but I've identified a critical bug in the cache key generation that could lead to serving incorrect data. I've provided a detailed comment and a code suggestion to fix this. Additionally, I've made a couple of medium-severity suggestions to improve error handling and encourage adding unit tests for the new caching logic to ensure its robustness. The documentation changes in the README are clear and helpful.
| function makeCacheKey(requestBody: object) { | ||
| try { | ||
| // Key by request body only (do not include API keys) | ||
| return Buffer.from(JSON.stringify(requestBody)).toString('base64') | ||
| } catch (e) { | ||
| return String(requestBody) | ||
| } | ||
| } | ||
|
|
||
| function getCache(key: string) { | ||
| if (!EXA_CACHE_ENABLED) return null | ||
| const entry = exaCache.get(key) | ||
| if (!entry) return null | ||
| if (Date.now() > entry.expiresAt) { | ||
| exaCache.delete(key) | ||
| return null | ||
| } | ||
| // LRU: move to end | ||
| exaCache.delete(key) | ||
| exaCache.set(key, entry) | ||
| return entry.value | ||
| } | ||
|
|
||
| function setCache(key: string, value: any) { | ||
| if (!EXA_CACHE_ENABLED) return | ||
| if (exaCache.size >= EXA_CACHE_CAPACITY) { | ||
| const oldest = exaCache.keys().next().value | ||
| if (oldest) exaCache.delete(oldest) | ||
| } | ||
| exaCache.set(key, { value, expiresAt: Date.now() + EXA_CACHE_TTL_MS }) | ||
| } |
There was a problem hiding this comment.
There's a critical issue in makeCacheKey that can lead to cache poisoning. The fallback return String(requestBody) will produce "[object Object]" for any object that fails JSON.stringify (e.g., due to circular references). This causes all such requests to share the same cache key, leading to incorrect cached responses being served.
To fix this, makeCacheKey should signal failure (e.g., by returning null), and getCache/setCache should be updated to handle this signal by skipping cache operations.
Here's a suggested implementation for all three functions that resolves this issue:
function makeCacheKey(requestBody: object): string | null {
try {
// Key by request body only (do not include API keys)
return Buffer.from(JSON.stringify(requestBody)).toString('base64')
} catch (e) {
console.warn('[exa] Failed to generate cache key, skipping cache for this request.', e)
return null
}
}
function getCache(key: string | null) {
if (!key || !EXA_CACHE_ENABLED) return null
const entry = exaCache.get(key)
if (!entry) return null
if (Date.now() > entry.expiresAt) {
exaCache.delete(key)
return null
}
// LRU: move to end
exaCache.delete(key)
exaCache.set(key, entry)
return entry.value
}
function setCache(key: string | null, value: any) {
if (!key || !EXA_CACHE_ENABLED) return
if (exaCache.size >= EXA_CACHE_CAPACITY) {
const oldest = exaCache.keys().next().value
if (oldest) exaCache.delete(oldest)
}
exaCache.set(key, { value, expiresAt: Date.now() + EXA_CACHE_TTL_MS })
}| // Simple in-memory cache for Exa queries (short-lived LRU with TTL) | ||
| const EXA_CACHE_ENABLED = process.env.EXA_CACHE_ENABLED !== 'false' | ||
| const EXA_CACHE_TTL_MS = Number(process.env.EXA_CACHE_TTL_MS || 60_000) // default 60s | ||
| const EXA_CACHE_CAPACITY = Number(process.env.EXA_CACHE_CAPACITY || 500) | ||
|
|
||
| interface ExaCacheEntry { value: any; expiresAt: number } | ||
| const exaCache = new Map<string, ExaCacheEntry>() |
There was a problem hiding this comment.
This new cache implementation is a great addition for performance. Since it contains non-trivial logic for TTL, LRU eviction, and key generation, it would be highly beneficial to add unit tests for it. This would help prevent regressions and verify edge cases, such as the cache capacity being reached or expired items being evicted correctly. The PR description mentions unit tests are not included, but I'd strongly recommend adding them for this critical piece of logic.
| return response.json(); | ||
| const json = await response.json(); | ||
| // store in cache for next requests | ||
| try { setCache(cacheKey, json) } catch (e) {} |
There was a problem hiding this comment.
Silently catching and ignoring errors with an empty catch block (catch (e) {}) can hide bugs and make debugging difficult. While setCache as written is unlikely to throw errors, it's a good practice to at least log any unexpected errors that might occur, especially if the function's implementation changes in the future.
| try { setCache(cacheKey, json) } catch (e) {} | |
| try { setCache(cacheKey, json) } catch (e) { console.warn('[exa] Failed to set cache value.', e) } |
name: "Exa Cache & Rate-limit"
about: Add short-lived cache and rate-limiting safeguards for Exa queries
title: "[Infra][Exa] Cache & Rate-limit"
✅ Pre-PR Checklist
Before submitting, please confirm:
npm run lint/npm run testlocallynpm installif dependencies changedDescription
Brief summary of the change being proposed:
lib/exa-service.ts, and integrated cache lookups for Exa requests to reduce redundant API calls; added verbose logging hooks for cache hits/misses.Files Changed
exora/lib/exa-service.ts— added cache helpers, cache integration, and config via env varsType of Change
Select all that apply:
Configuration / Environment Variables
Configurable env vars introduced or used:
EXA_CACHE_ENABLED(default: true) — enable/disable cacheEXA_CACHE_TTL_MS(default: 60000) — cache entry TTL in msEXA_CACHE_CAPACITY(default: 500) — max entries in in-memory LRUVERBOSE_LOGS(optional) — enable detailed logs for cache hits/missesNotes:
How Has This Been Tested?
Explain validation steps and results:
VERBOSE_LOGS=trueandEXA_CACHE_TTL_MSset low (e.g., 5000).EXA_CACHE_ENABLED=false) — second request should call Exa.Manual test commands (PowerShell example):
Then hit the UI/route that triggers Exa twice and observe logs.
Security & Privacy
Rollback Plan
EXA_CACHE_ENABLED=falseand redeploy.Linked Issue
Closes #<issue_number> (if applicable)
Additional Context