feat: embeddings model support #86

hahaQWQ · 2025-03-10T13:56:25Z

No description provided.

luoling8192 · 2025-03-10T16:40:19Z

apps/cli/src/commands/embed.ts

@@ -1,12 +1,11 @@
 import * as input from '@inquirer/prompts'
 import { useLogger } from '@tg-search/common'
 import { EmbeddingService } from '@tg-search/core'
-import { findMessagesByChatId, updateMessageEmbeddings } from '@tg-search/db'
+import { findMessageMissingEmbed, updateMessageEmbeddings, useEmbeddingTable } from '@tg-search/db'


Is findMissingEmbedMessage better?

luoling8192 · 2025-03-10T16:43:57Z

apps/cli/src/commands/embed.ts

-      const messages = await findMessagesByChatId(Number(chatId))
-      const messagesToEmbed = messages.items.filter(m => !m.embedding && m.content)
-      const totalMessages = messagesToEmbed.length
+      const messages = await findMessageMissingEmbed(Number(chatId), embedding.getEmbeddingConfig())


Is missingMessage better?

Just a propose, not a forcement. but can got better DX?

luoling8192 · 2025-03-10T16:47:07Z

apps/server/src/services/commands/embed.ts

@@ -48,6 +48,8 @@ export class EmbedCommandHandler {

    // Initialize embedding service
    const embedding = new EmbeddingService()
+    await useEmbeddingTable(embedding.getEmbeddingConfig())
+    logger.debug('awdadwd')


what's meaning?

OK, I will correct these mistakes. (:3 ⌒ﾞ)

luoling8192 · 2025-03-10T16:54:31Z

packages/core/src/services/embedding.ts

+    return {
+      ...this.embedding_config,
+      // 替换-为_
+      model: this.embedding_config.model.replace(/-/g, '_'),


why need convert?

because it looks good, if it's not suitable, I can make some reversions

Some embedding models, such as nomic-embed-text, have a hyphen in their names. this is not allowed in the table naming of PostgreSQL, so a conversion is needed.

PostgreSQL table naming rules

what do you think about moving the convert logic to the models layer? which is more relevant.

I will try it.

luoling8192 · 2025-03-10T16:55:43Z

packages/db/src/schema/embedding.ts

do you have regenerate drizzle migration file after you modify the schema?

Hmm... No😲

if you use a dynamic table, it is unnecessary

Copilot

PR Overview

This pull request adds support for embedding models by introducing an EmbeddingTableConfig and related helper functions. Key changes include:

Adding new schema and utility functions for creating and using embedding tables.
Extending configuration and types to include the embedding dimensions.
Updating message handling and search functionality to work with the new embedding model support.

Reviewed Changes

File	Description
packages/db/src/schema/embedding.ts	Introduces embedding table creation and use functions.
packages/db/src/schema/types.ts	Adds the EmbeddingTableConfig interface including dimensions.
packages/db/src/models/message/embedding.ts	Updates functions to use the new embedding table and config; refactors types.
packages/db/src/models/message/message.test.ts	Skips the findSimilarMessages test indicating decreased test coverage for that feature.
packages/common/src/types/config.ts	Adds the embedding dimensions to the API config.
config/config.example.yaml	Updates the example configuration with dimensions for embeddings.
packages/core/src/services/embedding.ts	Updates the embedding service to incorporate the new EmbeddingTableConfig.
apps/cli/src/commands/embed.ts	Refactors the embed command to use the new embedding config and table initialization.
packages/common/src/composable/config.ts	Adds a default dimensions value to the configuration.
apps/server/src/routes/config.ts	Updates the configuration schema to include embedding dimensions.
packages/db/src/models/message/types.ts	Adds an optional uuid field for message creation inputs.
packages/db/src/schema/index.ts	Re-exports the embedding schema.
apps/server/src/services/commands/embed.ts	Updates the server command for embedding to use new embedding table/config functions.
apps/cli/src/commands/search.ts	Updates the search command to utilize the new embedding configuration.
apps/server/src/routes/search.ts	Adjusts server search routes to use the updated embedding API.
packages/db/src/schema/message.ts	Updates the message table schema by removing the embedding field from the ORM schema.
apps/frontend/src/pages/settings.vue	Adds a settings input for configuring embedding dimensions.

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

packages/db/src/schema/message.ts:29

The embedding column is defined in the CREATE TABLE SQL block but has been removed from the ORM schema definition. Ensure that the ORM schema and SQL table definitions remain consistent.

embedding: vector('embedding'),

packages/db/src/models/message/message.test.ts:95

The findSimilarMessages test is skipped, which may leave the vector search functionality unverified. Consider re-enabling or adding tests to ensure proper coverage.

describe.skip('findSimilarMessages', () => {

Copilot · 2025-03-10T18:56:56Z

apps/cli/src/commands/embed.ts

-      const messagesToEmbed = messages.items.filter(m => !m.embedding && m.content)
-      const totalMessages = messagesToEmbed.length
+      const messages = await findMessageMissingEmbed(Number(chatId), embedding.getEmbeddingConfig())
+      logger.debug('awdadwd')


[nitpick] The debug log message 'awdadwd' appears to be a placeholder and is not descriptive. Please remove or replace it with a meaningful message.

Suggested change

logger.debug('awdadwd')

logger.debug('Successfully retrieved messages for embedding processing')

…e embedding table retrieval

…clude chatId for improved retrieval

… result handling - Added TypeScript interfaces for Command and CommandMetadata to improve type safety. - Updated the search function to return a structured SearchResponse. - Introduced a new types file for SearchResult and SearchResponse to standardize search-related data structures.

…ings components - Updated pagination logic in search.vue to ensure total is a valid number before comparison with pageSize. - Ensured apiId in settings.vue is converted to a string before saving the configuration. - Removed unused imports in search.ts to clean up the codebase. - Adjusted parameters in findSimilarMessages function to include targetChatId for better search results.

- Implemented migration for creating embedding_models table with necessary fields and triggers for automatic timestamp updates. - Added logic to create corresponding embedding tables for existing message tables, including necessary constraints and indexes. - Enhanced logging for better tracking of migration progress and errors.

luoling8192 · 2025-03-18T07:06:05Z

LGTM, Thank you very much!

This reverts commit 032cbb4.

hahaQWQ added 4 commits March 10, 2025 20:40

feat: embeddings model support

845bc86

fix: cli embed

172b96f

fix: server embedding fix

acfe646

fix: config save

ba5fc69

hahaQWQ mentioned this pull request Mar 10, 2025

Roadmap v1.0.0 #23

Closed

40 tasks

fix: type error

b8900ce

hahaQWQ added the PR label Mar 10, 2025

luoling8192 reviewed Mar 10, 2025

View reviewed changes

luoling8192 changed the title ~~embeddings model support~~ feat: embeddings model support Mar 10, 2025

luoling8192 requested a review from Copilot March 10, 2025 18:55

Copilot AI reviewed Mar 10, 2025

View reviewed changes

style: embeddings

3c5ffc7

hahaQWQ requested a review from luoling8192 March 14, 2025 13:00

hahaQWQ force-pushed the dev/embeddings-model branch from 430340f to 3c5ffc7 Compare March 14, 2025 13:07

hahaqwq added 3 commits March 15, 2025 14:05

feat: embedding_models schema

85b966b

feat: update message embedding functions to include chatId and improv…

6452ba7

…e embedding table retrieval

feat: add embedding_models table and update embedding functions to in…

da8e73e

…clude chatId for improved retrieval

hahaQWQ force-pushed the dev/embeddings-model branch from aaffcbf to da8e73e Compare March 15, 2025 07:45

hahaQWQ and others added 4 commits March 15, 2025 15:47

Merge branch 'main' into dev/embeddings-model

5d57e6b

luoling8192 mentioned this pull request Mar 18, 2025

Dev/quick jump #94

Merged

luoling8192 merged commit 032cbb4 into main Mar 18, 2025
3 checks passed

luoling8192 added a commit that referenced this pull request Mar 21, 2025

Revert "feat: embeddings model support (#86)"

1553b71

This reverts commit 032cbb4.

luoling8192 deleted the dev/embeddings-model branch May 3, 2025 05:26

	logger.debug('awdadwd')
	logger.debug('Successfully retrieved messages for embedding processing')

Uh oh!

feat: embeddings model support #86

feat: embeddings model support #86

Uh oh!

Conversation

hahaQWQ commented Mar 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

PR Overview

Reviewed Changes

Uh oh!

Copilot AI Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

luoling8192 commented Mar 18, 2025

Uh oh!

Uh oh!

Uh oh!