DA-1319: Update haystack demo per 8.0 #73
Conversation
…ase and OpenAI - Changed tutorial path and titles to reflect the use of Couchbase Search Vector Index instead of FTS. - Updated descriptions to clarify the integration of Couchbase's vector search capabilities with OpenAI embeddings. - Removed the fts_index.json file as it is no longer needed. - Enhanced the RAG notebook to include updated content and examples using the new index types. - Adjusted tags in frontmatter to align with the new terminology.
|
Caution Notebooks or Frontmatter Files Have Been Modified
3 Notebook Files Modified:
4 Frontmatter Files Modified:
|
Summary of ChangesHello @VirajAgarwal1, 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 updates the Couchbase Haystack demo to be compatible with Haystack 8.0. It removes the old FTS-based demo and introduces a new demo leveraging query-based vector search, providing an up-to-date example of integrating Couchbase with Haystack for RAG applications. Highlights
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly refactors the Haystack demos to align with Couchbase 8.0 features, replacing the former fts and gsi based examples with new search_based and query_based notebooks. This is a great update that clarifies the technology being demonstrated.
My review of the new notebooks has uncovered a few issues, including some critical and high-severity bugs that would prevent the demos from running correctly. These include an issue with dynamic index creation in the search_based notebook, the use of unsafe eval() when processing data, and incorrect index parameters in the query_based notebook that lead to a failure during index creation. I've also noted some minor inconsistencies in markdown and comments that could be improved for clarity. Addressing these points will greatly improve the quality and user experience of these new demos.
| " \n", | ||
| " options = {\n", | ||
| " \"dimension\": 3072, # text-embedding-3-large dimension\n", | ||
| " \"description\": \"IVF1024,PQ32x8\",\n", |
There was a problem hiding this comment.
The Hyperscale index creation is configured with "IVF1024,PQ32x8", which requires at least 1024 documents for training. However, the notebook only indexes 100 documents, causing the index creation to fail as shown in the cell's output. To make the demo runnable, please change the configuration to use auto-selected centroids (e.g., "IVF,PQ32x8") or increase the number of indexed documents to be more than 1024.
"description": "IVF,PQ32x8",
| " with open('search_vector_index.json', 'r') as search_file:\n", | ||
| " search_index_definition = SearchIndex.from_json(json.load(search_file))\n", | ||
| " scope_search_manager.upsert_index(search_index_definition)\n", | ||
| " print(f\"Search Vector Index '{search_index_name}' created successfully at scope level.\")" |
There was a problem hiding this comment.
There is a bug in the index creation logic within the except block. The code re-reads the original search_vector_index.json file, which discards the dynamic updates (e.g., index name, scope, collection) applied earlier. This will result in either an incorrect index being created or a failure. The code should use the search_index_definition object that has already been modified.
scope_search_manager.upsert_index(search_index_definition)
print(f"Search Vector Index '{search_index_name}' created successfully at scope level.")
| " try:\n", | ||
| " docs_data.append({\n", | ||
| " 'id': str(row[\"id\"]),\n", | ||
| " 'content': f\"Title: {row['title']}\\nGenres: {', '.join([genre['name'] for genre in eval(row['genres'])])}\\nOverview: {row['overview']}\",\n", |
There was a problem hiding this comment.
Using eval() on data from an external source is a security vulnerability as it can execute arbitrary code. It's much safer to use ast.literal_eval() to parse literal structures from strings. Please also add import ast at the beginning of this cell.
'content': f"Title: {row['title']}\nGenres: {', '.join([genre['name'] for genre in ast.literal_eval(row['genres'])])}\nOverview: {row['overview']}",
| " news_dataset = load_dataset('RealTimeData/bbc_news_alltime', '2024-12', split=\"train\")\n", | ||
| " print(f\"Loaded the BBC News dataset with {len(news_dataset)} rows\")\n", | ||
| "except Exception as e:\n", | ||
| " raise ValueError(f\"Error loading TREC dataset: {str(e)}\")" |
There was a problem hiding this comment.
The error message in the except block is misleading. It refers to the "TREC dataset", but the code is attempting to load the "bbc_news_alltime" dataset. This should be corrected to avoid confusion.
raise ValueError(f"Error loading BBC News dataset: {str(e)}")
| "source": [ | ||
| "# Run Indexing Pipeline\n", | ||
| "\n", | ||
| "Execute the pipeline for processing and indexing BCC news documents:" |
There was a problem hiding this comment.
The markdown description for the indexing pipeline mentions using a DocumentSplitter as the first step. However, the pipeline defined in the next cell does not include a DocumentSplitter. This discrepancy should be corrected to avoid confusing users.
- Cleared all output cells to ensure a clean notebook state. - Set execution counts to null for all code cells to allow for fresh execution. - Adjusted the indexing pipeline to process a larger number of documents (from 100 to 1200). - Updated the sample query to reflect current news context.
No description provided.