Skip to content

Conversation

@kaustubh-darekar
Copy link
Collaborator

Updating models to latest version
Updating embedding model and their dimensions parameter.
Updating integration test script
Updating other dependent codes

@kaustubh-darekar kaustubh-darekar self-assigned this Dec 5, 2025
@kaustubh-darekar kaustubh-darekar added the enhancement New feature or request label Dec 5, 2025
@kaustubh-darekar kaustubh-darekar linked an issue Dec 5, 2025 that may be closed by this pull request
4 tasks
Copilot finished reviewing on behalf of kaustubh-darekar December 5, 2025 10:48

This comment was marked as outdated.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

elif embedding_model_name == "titan":
embeddings = get_bedrock_embeddings()
dimension = 1536
dimension = 1024
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The embedding dimension for the Bedrock Titan model is changed from 1536 to 1024. However, Amazon Titan Text Embeddings v1 produces 1536-dimensional embeddings, while v2 produces 1024-dimensional embeddings. The comment on line 52 of example.env mentions amazon.titan-embed-text-v2.0, which would have 1024 dimensions. Please ensure this dimension matches the actual Titan model version being used by the get_bedrock_embeddings() function.

Copilot uses AI. Check for mistakes.
ENTITY_EMBEDDING="TRUE" # TRUE or FALSE based on whether to create embeddings for entities suitable for entity vector mode
DUPLICATE_SCORE_VALUE =0.97
DUPLICATE_TEXT_DISTANCE =3
DEFAULT_DIFFBOT_CHAT_MODEL="openai_gpt_4o" #whichever model specified here , need to add config for that model in below format)
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DEFAULT_DIFFBOT_CHAT_MODEL is set to openai_gpt_4o, which is a model that was removed in this PR. This creates an inconsistency - the default references a model that may not be configured. Consider updating this to one of the new models like openai_gpt_5_mini or another valid model that's included in the updated model list.

Suggested change
DEFAULT_DIFFBOT_CHAT_MODEL="openai_gpt_4o" #whichever model specified here , need to add config for that model in below format)
DEFAULT_DIFFBOT_CHAT_MODEL="openai_gpt_5_mini" #whichever model specified here , need to add config for that model in below format)

Copilot uses AI. Check for mistakes.
Comment on lines +12 to +13
process.env?.VITE_LLM_MODELS?.trim() != ''
? (process.env.VITE_LLM_MODELS?.split(',') as string[])
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition process.env?.VITE_LLM_MODELS?.trim() != '' is potentially incorrect. If process.env.VITE_LLM_MODELS is undefined, calling .trim() will throw an error before the comparison. The previous condition checked for truthiness first. Consider using: process.env?.VITE_LLM_MODELS?.trim() (with optional chaining continuing) or process.env.VITE_LLM_MODELS && process.env.VITE_LLM_MODELS.trim() != ''.

Suggested change
process.env?.VITE_LLM_MODELS?.trim() != ''
? (process.env.VITE_LLM_MODELS?.split(',') as string[])
process.env.VITE_LLM_MODELS && process.env.VITE_LLM_MODELS.trim() !== ''
? (process.env.VITE_LLM_MODELS.split(',') as string[])

Copilot uses AI. Check for mistakes.
return (url.trim() != '' && /(http(s)?:\/\/.)?(www\.)?[-a-zA-Z0-9@:%._~#=]{2,256}\.[a-z]{2,6}\b([-a-zA-Z0-9@:%_.~#?&//=]*)/g.test(url) != false);
return (
url.trim() != '' &&
/(http(s)?:\/\/.)?(www\.)?[-a-zA-Z0-9@:%._~#=]{2,256}\.[a-z]{2,6}\b([-a-zA-Z0-9@:%_.~#?&//=]*)/g.test(url) != false
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot uses AI. Check for mistakes.
Comment on lines +115 to +117
# is_gemini_enabled = os.environ.get("GEMINI_ENABLED", "False").lower() in ("true", "1", "yes")
# if is_gemini_enabled:
# add_routes(app,ChatVertexAI(), path="/vertexai")
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment appears to contain commented-out code.

Suggested change
# is_gemini_enabled = os.environ.get("GEMINI_ENABLED", "False").lower() in ("true", "1", "yes")
# if is_gemini_enabled:
# add_routes(app,ChatVertexAI(), path="/vertexai")

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Model Updates

2 participants