Enhance article matching using TF-IDF and cosine similarity#58
Conversation
…larity for more relevant local sources
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Summary of ChangesHello, 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 enhances the local RAG pipeline by introducing TF-IDF semantic search for retrieving relevant article chunks. It improves the quality of retrieved content by matching user queries with article content semantically, using cosine similarity and a cached TF-IDF index. A fallback mechanism ensures continued functionality if semantic search fails to find relevant matches. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a TF-IDF based semantic search, which is a great improvement over the previous keyword matching. The implementation is solid, introducing new functions for building the TF-IDF index and calculating cosine similarity. My review focuses on improving the performance and maintainability of this new implementation. I've suggested pre-calculating vector norms to speed up cosine similarity, simplifying the document frequency calculation, and improving code readability by using a type alias for a complex object. I also found a small opportunity to improve relevance by including slug matches in the keyword bonus, aligning the implementation with the comment.
| function cosineSimilarity( | ||
| a: Map<string, number>, | ||
| b: Map<string, number>, | ||
| ): number { | ||
| let dot = 0; | ||
| let normA = 0; | ||
| let normB = 0; | ||
| Array.from(a.entries()).forEach(([term, val]) => { | ||
| dot += val * (b.get(term) || 0); | ||
| normA += val * val; | ||
| }); | ||
| Array.from(b.values()).forEach((val) => { | ||
| normB += val * val; | ||
| }); | ||
| const denom = Math.sqrt(normA) * Math.sqrt(normB); | ||
| return denom === 0 ? 0 : dot / denom; | ||
| } |
There was a problem hiding this comment.
The current implementation calculates the norm of vector b (the document vector) on every call. This is inefficient as document vectors are static within the built index. You can significantly improve performance by pre-calculating and caching the norm of each document vector when the TF-IDF index is built.
Here's a suggested implementation:
- Update
cachedChunkVectorstype (lines 77-87) to include anorm: numberproperty. - In
buildTfIdfIndex(lines 168-170), calculate and store this norm for each chunk vector after converting it to a TF-IDF vector.for (const entry of allChunks) { entry.vector = buildTfIdfVector(entry.vector, idfMap); let norm = 0; for (const val of entry.vector.values()) { norm += val * val; } (entry as any).norm = Math.sqrt(norm); }
- Update
cosineSimilarityto use the pre-calculated norm (as suggested below). - Update the call in
buildLocalSourcesFromDocuments(line 463) to pass the norm:const sim = cosineSimilarity(queryVector, entry.vector, entry.norm);
function cosineSimilarity(
a: Map<string, number>,
b: Map<string, number>,
normB: number
): number {
let dot = 0;
let normA = 0;
Array.from(a.entries()).forEach(([term, val]) => {
dot += val * (b.get(term) || 0);
normA += val * val;
});
const denom = Math.sqrt(normA) * normB;
return denom === 0 ? 0 : dot / denom;
}| let cachedChunkVectors: | ||
| | { | ||
| slug: string; | ||
| title: string; | ||
| url: string; | ||
| topics: string[]; | ||
| chunkIndex: number; | ||
| chunk: string; | ||
| vector: Map<string, number>; | ||
| }[] | ||
| | null = null; |
There was a problem hiding this comment.
For better readability and maintainability, consider defining a type alias or interface for this complex object shape instead of defining it inline. This makes the type reusable and the code easier to understand. You could define it near your other interface definitions at the top of the file.
For example:
interface CachedChunkVector {
slug: string;
title: string;
url: string;
topics: string[];
chunkIndex: number;
chunk: string;
vector: Map<string, number>;
}Then you can use it like this: let cachedChunkVectors: CachedChunkVector[] | null = null; and also in buildTfIdfIndex for allChunks.
| const seen = new Set<string>(); | ||
| Array.from(entry.vector.keys()).forEach((term) => { | ||
| if (!seen.has(term)) { | ||
| df.set(term, (df.get(term) || 0) + 1); | ||
| seen.add(term); | ||
| } | ||
| }); |
There was a problem hiding this comment.
The seen set is redundant here because entry.vector is a Map, and entry.vector.keys() provides an iterator of unique keys for that chunk. This loop can be simplified to directly iterate over the keys and update the document frequency count, which is more efficient and readable.
for (const term of entry.vector.keys()) {
df.set(term, (df.get(term) || 0) + 1);
}| const titleTokens = unique(tokenize(entry.title)); | ||
| const keywordBonus = overlapCount(queryTokens, titleTokens) * 0.05; |
There was a problem hiding this comment.
The comment on line 465 mentions title/slug, but the code only considers the title for the keyword bonus. To better align with the comment and potentially improve relevance for queries that match slugs, you could also include a bonus for slug matches.
const titleTokens = unique(tokenize(entry.title));
const slugTokens = unique(tokenize(entry.slug));
const keywordBonus =
(overlapCount(queryTokens, titleTokens) +
overlapCount(queryTokens, slugTokens)) *
0.05;
This pull request introduces a significant improvement to the local retrieval-augmented generation (RAG) pipeline by implementing a TF-IDF-based semantic search for retrieving relevant article chunks, replacing the previous keyword-matching approach. The new method builds and caches a TF-IDF index for local articles, computes cosine similarity between the query and article chunks, and ranks results accordingly, with a fallback to the old method if no relevant matches are found.
Key changes include:
Semantic Search with TF-IDF:
Fallback and Result Selection Logic:
These changes should result in more relevant and semantically meaningful retrieval of local article content for user queries, improving the quality of downstream responses.