move rag tests to ollama #623
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -710,6 +710,8 @@ system({ | |||
| "Function to do a search using embeddings vector similarity distance.", | |||
| }) | |||
|
|
|||
| const embeddingsModel = env.var.embeddingsModel || undefined | |||
There was a problem hiding this comment.
The variable embeddingsModel is declared but its value is never read.
generated by pr-docs-review-commit
unused_variable
| @@ -737,7 +739,7 @@ defTool( | |||
| const res = await retrieval.vectorSearch( | |||
| q, | |||
| files.map((filename) => ({ filename })) | |||
| ) | |||
| , { embeddingsModel }) | |||
There was a problem hiding this comment.
The argument label for embeddingsModel is missing in the function call.
generated by pr-docs-review-commit
missing_argument_label
| @@ -101,7 +101,7 @@ export async function resolveModelConnectionInfo( | |||
| const configuration = await host.getLanguageModelConfiguration( | |||
| model, | |||
| { | |||
There was a problem hiding this comment.
The token variable name has been changed from 'askToken' to 'withToken'. Please ensure that 'withToken' is defined and holds the correct value. 🕵️♀️
generated by pr-review-commit
token_change
| "github:text-embedding-3-small", | ||
| "github:text-embedding-3-large", | ||
| "ollama:nomic-embed-text" | ||
| > |
There was a problem hiding this comment.
The 'embeddingsModel' type has been changed and new options have been added. Please ensure that these new options are supported and handled correctly in the code. 🧐
generated by pr-review-commit
embeddingsModel_change
| @@ -72,10 +72,11 @@ class OpenAIEmbeddings implements EmbeddingsModel { | |||
| url = `${trimTrailingSlash(base)}/${model.replace(/\./g, "")}/embeddings?api-version=${AZURE_OPENAI_API_VERSION}` | |||
| delete body.model | |||
| } else { | |||
| url = `${base}/v1/embeddings` | |||
| url = `${base}/embeddings` | |||
There was a problem hiding this comment.
The URL endpoint for embeddings has been changed from '/v1/embeddings' to '/embeddings'. Please ensure that the new endpoint is correct and functional. 🌐
generated by pr-review-commit
url_change
|
The changes in the pull request contain the following:
These changes seem to be focused on expanding functionality, improving logging, and making minor adjustments to variable names. As for concerns:
Please confirm that these changes have been thoroughly tested to ensure no existing functionality is broken. Otherwise, LGTM 🚀
|
| @@ -710,6 +710,8 @@ system({ | |||
| "Function to do a search using embeddings vector similarity distance.", | |||
| }) | |||
|
|
|||
| const embeddingsModel = env.var.embeddingsModel || undefined | |||
There was a problem hiding this comment.
The variable embeddingsModel is assigned with || undefined which is redundant since an unset or nullish value defaults to undefined in JavaScript.
generated by pr-docs-review-commit
unnecessary_code
| @@ -737,7 +739,7 @@ defTool( | |||
| const res = await retrieval.vectorSearch( | |||
| q, | |||
| files.map((filename) => ({ filename })) | |||
| ) | |||
| , { embeddingsModel }) | |||
There was a problem hiding this comment.
Missing comma after the closing parenthesis of the vectorSearch function call.
generated by pr-docs-review-commit
missing_comma
| "github:text-embedding-3-small", | ||
| "github:text-embedding-3-large", | ||
| "ollama:nomic-embed-text" | ||
| > |
There was a problem hiding this comment.
The embeddingsModel options have been expanded. Please ensure that all new options are supported and correctly implemented in the code. 🧐
generated by pr-review-commit
embeddings_model_change
The GIT_DIFF reflects several changes across multiple files aiming to enhance and extend the functionality of the software.
It introduces a new variable
embeddingsModel, which is fetched from environment variables, for use in the retrieval vector search function in several files including 'system.mdx', 'system.retrieval_vector_search.genai.js' and 'rag.genai.js'. 🆕Changes in 'models.ts' replaced the term 'askToken' with 'withToken', which may affect how user authorization or access control is handled. 🔄
The most crucial changes appear to be in 'prompt_template.d.ts'. New options for embeddings models are added, expanding user choice. It means that users can now specify various text embedding models like 'openai:text-embedding-3-small', 'openai:text-embedding-3-large', 'github:text-embedding-3-small', 'github:text-embedding-3-large', 'ollama:nomic-embed-text' among others. 🚀
Another file 'rag.genai.js' had its model updated from 'openai:gpt-3.5-turbo' to 'ollama:phi3'. The selection of the embeddings model is also configurable in this file now. 🔄
In the module 'vectorsearch.ts', the endpoint url for creating embeddings has been altered slightly. Verbose logging for embedding models has also been introduced improving debugging capabilities. ✨
Changes in these files are user-facing as they primarily belong to the core functionality and directly affect how end users can interact with and use the system concerning embedding models and retrieval searches. 🧑💻
Please note that the comments and some other minor changes have been ignored as per the provided instructions.