feat: optionally ask to select AI models and use default models #40
Conversation
🦋 Changeset detectedLatest commit: cc52db7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe recent updates focus on enhancing the flexibility and customization of embedding models across the application. By introducing environment variables for embedding model selection ( Changes
Recent Review StatusConfiguration used: CodeRabbit UI Files selected for processing (12)
Files skipped from review as they are similar to previous changes (5)
Additional comments not posted (7)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (4)
- helpers/env-variables.ts (2 hunks)
- questions.ts (1 hunks)
- templates/types/streaming/express/src/controllers/engine/settings.ts (2 hunks)
- templates/types/streaming/nextjs/app/api/chat/engine/settings.ts (2 hunks)
Additional comments not posted (6)
templates/types/streaming/express/src/controllers/engine/settings.ts (3)
1-1: The import ofOpenAIEmbeddingis correctly implemented.
5-5: The declaration ofEMBEDDING_MODELusing environment variables is correctly implemented.
14-16: The initialization ofSettings.embedModelwithOpenAIEmbeddingis correctly implemented, aligning with the PR objectives.templates/types/streaming/nextjs/app/api/chat/engine/settings.ts (3)
1-1: The import ofOpenAIEmbeddingis correctly implemented.
5-5: The declaration ofEMBEDDING_MODELusing environment variables is correctly implemented.
14-16: The initialization ofSettings.embedModelwithOpenAIEmbeddingis correctly implemented, aligning with the PR objectives.
templates/types/streaming/nextjs/app/api/chat/engine/settings.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- index.ts (2 hunks)
- questions.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- questions.ts
Additional comments not posted (2)
index.ts (2)
180-185: The addition of the--use-embed-modelCLI option is correctly implemented and follows the established pattern for adding new options in Commander. The description is clear and aligns with the PR objectives.
201-203: The handling of the--use-embed-modeloption, settingprogram.useEmbedModeltotruewhen present, is correctly implemented. Ensure to verify howprogram.useEmbedModelis utilized throughout the application to fully understand its impact.Verification successful
The usage of the
--use-embed-modeloption and its correspondingprogram.useEmbedModelflag within the application, as seen in theindex.tsandquestions.tsfiles, is correctly implemented and integrated. The conditional logic inquestions.tsthat depends on this flag is consistent with an optional feature toggle, taking into account the execution environment (CI systems). This confirms the appropriate handling and integration of the--use-embed-modeloption within the application's logic.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the usage of `program.useEmbedModel` throughout the application. rg --type ts "useEmbedModel"Length of output: 171
There was a problem hiding this comment.
Actionable comments posted: 1
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (3)
- .changeset/curvy-eels-boil.md (1 hunks)
- index.ts (2 hunks)
- questions.ts (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- index.ts
- questions.ts
Additional comments not posted (1)
.changeset/curvy-eels-boil.md (1)
5-5: The CLI parameter name seems to have changed from--use-embed-modelto--change-embed-model. Please confirm this change is intentional and correctly documented across all relevant parts of the application.
marcusschiesser
force-pushed
the
feat/ask-to-use-embedding-model
branch
from
74504fa to
37a552c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (6)
- .changeset/curvy-eels-boil.md (1 hunks)
- e2e/utils.ts (2 hunks)
- index.ts (2 hunks)
- questions.ts (4 hunks)
- templates/types/streaming/express/src/controllers/engine/settings.ts (2 hunks)
- templates/types/streaming/nextjs/app/api/chat/engine/settings.ts (2 hunks)
Files skipped from review as they are similar to previous changes (4)
- index.ts
- questions.ts
- templates/types/streaming/express/src/controllers/engine/settings.ts
- templates/types/streaming/nextjs/app/api/chat/engine/settings.ts
Additional comments not posted (1)
e2e/utils.ts (1)
12-18: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-62]
The changes in
e2e/utils.tsalign with the PR objectives to simplify model configuration by removing specific constants and their usage. Ensure that the removal ofMODELandEMBEDDING_MODELconstants and their related logic does not impact other parts of the test suite or the application's functionality.
marcusschiesser
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Actionable comments outside the diff hunks (4)
templates/components/vectordbs/python/astra/generate.py (2)
Line range hint
5-11: Reorder imports to follow PEP 8 guidelines.+ import os + import logging from dotenv import load_dotenv - import os - import logging
Line range hint
32-32: Remove unnecessary f-string.- logger.info(f"Successfully created embeddings in the AstraDB") + logger.info("Successfully created embeddings in the AstraDB")templates/components/vectordbs/python/milvus/generate.py (2)
Line range hint
5-11: Reorder imports to follow PEP 8 guidelines.+ import os + import logging from dotenv import load_dotenv - import os - import logging
Line range hint
34-34: Remove unnecessary f-string.- logger.info(f"Successfully created embeddings in the Milvus") + logger.info("Successfully created embeddings in the Milvus")
Summary by CodeRabbit
--ask-models.