Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(backend): enhance config loader with embedding support and impro… #74

Merged
merged 20 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
78a3545
feat(backend): enhance config loader with embedding support and impro…
Sma1lboy Dec 16, 2024
72c6dbc
feat(backend): implement ModelStatusManager for tracking model downlo…
Sma1lboy Dec 16, 2024
1340532
chore: adding uitls ufc
Sma1lboy Dec 16, 2024
0ee6b84
Merge branch 'main' of https://github.com/Sma1lboy/codefox into feat-…
NarwhalChen Dec 25, 2024
0b412b0
feat: adding embedding provider and openai-embedding provider
NarwhalChen Dec 25, 2024
b378185
[autofix.ci] apply automated fixes
autofix-ci[bot] Dec 25, 2024
4c66d05
feat: implmenting dynamic loader for embedding and models in hugging …
NarwhalChen Dec 28, 2024
ad74c49
Merge branch 'feat-adding-embedding-config' of https://github.com/Sma…
NarwhalChen Dec 28, 2024
97cae53
Merge branch 'main' into feat-adding-embedding-config
Sma1lboy Dec 28, 2024
e579146
feat(backend): refactor model downloading and configuration handling
Sma1lboy Dec 28, 2024
8b25760
Merge branch 'feat-adding-embedding-config' of https://github.com/Sma…
NarwhalChen Dec 30, 2024
b7a8b30
to: using fastemb to implement embedding downloader
NarwhalChen Dec 30, 2024
7e68b3a
to: moving embedding provider to backend
NarwhalChen Dec 30, 2024
2c2027d
[autofix.ci] apply automated fixes
autofix-ci[bot] Dec 30, 2024
fb9be7d
feat: request llm-server with openai/some embedding provider
NarwhalChen Dec 30, 2024
0b85081
Merge branch 'feat-adding-embedding-config' of https://github.com/Sma…
NarwhalChen Dec 30, 2024
6ad8b9b
to: adding return type and comments for generateEmbResponse
NarwhalChen Dec 31, 2024
56c6ffc
fix: fixing the change of env variable
NarwhalChen Jan 6, 2025
b9c1909
fix: deleting useless embedding index.ts
NarwhalChen Jan 6, 2025
50d3bd1
Merge branch 'main' into feat-adding-embedding-config
ZHallen122 Jan 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions backend/src/config/common-path.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import * as path from 'path';
import { existsSync, mkdirSync, promises } from 'fs-extra';
import { existsSync, mkdirSync, promises, writeFileSync } from 'fs-extra';

// Constants for base directories
const APP_NAME = 'codefox';
// TODO: hack way to get the root directory of the workspace
const WORKSPACE_ROOT = path.resolve(__dirname, '../../../');
const WORKSPACE_ROOT = path.resolve(__dirname, '../../../../');
const ROOT_DIR = path.join(WORKSPACE_ROOT, `.${APP_NAME}`);

export const TEMPLATE_PATH = path.join(WORKSPACE_ROOT, 'backend/template');
Expand All @@ -27,10 +27,17 @@ const ensureDir = (dirPath: string): string => {
export const getRootDir = (): string => ensureDir(ROOT_DIR);

// Configuration Paths
export const getConfigDir = (): string =>
ensureDir(path.join(getRootDir(), 'config'));
export const getConfigPath = (configName: string): string =>
path.join(getConfigDir(), `${configName}.json`);
export const getConfigPath = (): string => {
const rootPath = ensureDir(path.join(getRootDir()));
return path.join(rootPath, 'config.json');
};

export const getModelStatusPath = (): string => {
const rootPath = ensureDir(getRootDir());
const modelStatusPath = path.join(rootPath, 'model-status.json');
writeFileSync(modelStatusPath, '{}');
return modelStatusPath;
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential overwrite of existing model-status.json
Calling getModelStatusPath always writes '{}', even if the file exists. This can wipe out existing statuses. Implement a check to write only when missing the file.

export const getModelStatusPath = (): string => {
  const rootPath = ensureDir(getRootDir());
  const modelStatusPath = path.join(rootPath, 'model-status.json');
- writeFileSync(modelStatusPath, '{}');
+ if (!existsSync(modelStatusPath)) {
+   writeFileSync(modelStatusPath, '{}');
+ }
  return modelStatusPath;
};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const getModelStatusPath = (): string => {
const rootPath = ensureDir(getRootDir());
const modelStatusPath = path.join(rootPath, 'model-status.json');
writeFileSync(modelStatusPath, '{}');
return modelStatusPath;
};
export const getModelStatusPath = (): string => {
const rootPath = ensureDir(getRootDir());
const modelStatusPath = path.join(rootPath, 'model-status.json');
if (!existsSync(modelStatusPath)) {
writeFileSync(modelStatusPath, '{}');
}
return modelStatusPath;
};


// Models Directory
export const getModelsDir = (): string =>
Expand Down
223 changes: 208 additions & 15 deletions backend/src/config/config-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import * as fs from 'fs';
import * as path from 'path';
import * as _ from 'lodash';
import { getConfigPath } from './common-path';
import { Logger } from '@nestjs/common';

export interface ChatConfig {
model: string;
endpoint?: string;
Expand All @@ -10,46 +12,237 @@ export interface ChatConfig {
task?: string;
}

export class ConfigLoader {
private chatsConfig: ChatConfig[];
export interface EmbeddingConfig {
model: string;
endpoint?: string;
token?: string;
}

export interface AppConfig {
chats?: ChatConfig[];
embeddings?: EmbeddingConfig;
}

export const exampleConfigContent = `{
// Chat models configuration
// You can configure multiple chat models
"chats": [
// Example of OpenAI GPT configuration
{
"model": "gpt-3.5-turbo",
"endpoint": "https://api.openai.com/v1",
"token": "your-openai-token", // Replace with your OpenAI token
"default": true // Set as default chat model
},

// Example of local model configuration
{
"model": "llama2",
"endpoint": "http://localhost:11434/v1",
"task": "chat"
}
],

// Embedding model configuration (optional)
"embeddings": {
"model": "text-embedding-ada-002",
"endpoint": "https://api.openai.com/v1",
"token": "your-openai-token" // Replace with your OpenAI token
}
}`;

export class ConfigLoader {
private static instance: ConfigLoader;
private config: AppConfig;
private readonly configPath: string;

constructor() {
this.configPath = getConfigPath('config');
private constructor() {
this.configPath = getConfigPath();
this.initConfigFile();
this.loadConfig();
}

private loadConfig() {
const file = fs.readFileSync(this.configPath, 'utf-8');
public static getInstance(): ConfigLoader {
if (!ConfigLoader.instance) {
ConfigLoader.instance = new ConfigLoader();
}
return ConfigLoader.instance;
}

public initConfigFile(): void {
Logger.log('Creating example config file', 'ConfigLoader');

this.chatsConfig = JSON.parse(file);
console.log('Raw file content:', this.chatsConfig);
const config = getConfigPath();
if (fs.existsSync(config)) {
return;
}

if (!fs.existsSync(config)) {
//make file
fs.writeFileSync(config, exampleConfigContent, 'utf-8');
}
Logger.log('Creating example config file', 'ConfigLoader');
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix duplicate log message and redundant existence check

The initConfigFile method has two issues:

  1. Duplicate log message at lines 73 and 84
  2. Redundant file existence check at lines 80-83
 public initConfigFile(): void {
   Logger.log('Creating example config file', 'ConfigLoader');

   const config = getConfigPath();
   if (fs.existsSync(config)) {
     return;
   }

-  if (!fs.existsSync(config)) {
-    //make file
-    fs.writeFileSync(config, exampleConfigContent, 'utf-8');
-  }
-  Logger.log('Creating example config file', 'ConfigLoader');
+  fs.writeFileSync(config, exampleConfigContent, 'utf-8');
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public initConfigFile(): void {
Logger.log('Creating example config file', 'ConfigLoader');
this.chatsConfig = JSON.parse(file);
console.log('Raw file content:', this.chatsConfig);
const config = getConfigPath();
if (fs.existsSync(config)) {
return;
}
if (!fs.existsSync(config)) {
//make file
fs.writeFileSync(config, exampleConfigContent, 'utf-8');
}
Logger.log('Creating example config file', 'ConfigLoader');
public initConfigFile(): void {
Logger.log('Creating example config file', 'ConfigLoader');
const config = getConfigPath();
if (fs.existsSync(config)) {
return;
}
fs.writeFileSync(config, exampleConfigContent, 'utf-8');
}

}

get<T>(path: string) {
public reload(): void {
this.loadConfig();
}

private loadConfig() {
try {
Logger.log(
`Loading configuration from ${this.configPath}`,
'ConfigLoader',
);
const file = fs.readFileSync(this.configPath, 'utf-8');
const jsonContent = file.replace(
/\\"|"(?:\\"|[^"])*"|(\/\/.*|\/\*[\s\S]*?\*\/)/g,
(m, g) => (g ? '' : m),
);
this.config = JSON.parse(jsonContent);
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using a library to parse JSON with comments

The custom regex used to strip comments from the JSON content may not cover all edge cases and can be error-prone. Instead of manually handling comments, consider using a library like strip-json-comments or json5 to safely parse JSON files with comments.

Apply this change using strip-json-comments:

+import stripJsonComments from 'strip-json-comments';

...

-const jsonContent = file.replace(
-  /\\"|"(?:\\"|[^"])*"|(\/\/.*|\/\*[\s\S]*?\*\/)/g,
-  (m, g) => (g ? '' : m),
-);
+const jsonContent = stripJsonComments(file);

Remember to install the package:

npm install strip-json-comments

this.validateConfig();
} catch (error) {
if (
error.code === 'ENOENT' ||
error.message.includes('Unexpected end of JSON input')
) {
this.config = {};
this.saveConfig();
} else {
throw error;
}
}
}

get<T>(path?: string): T {
if (!path) {
return this.chatsConfig as unknown as T;
return this.config as unknown as T;
}
return _.get(this.chatsConfig, path) as T;
return _.get(this.config, path) as T;
}

set(path: string, value: any) {
_.set(this.chatsConfig, path, value);
_.set(this.config, path, value);
this.saveConfig();
}

private saveConfig() {
const configDir = path.dirname(this.configPath);
if (!fs.existsSync(configDir)) {
fs.mkdirSync(configDir, { recursive: true });
}
fs.writeFileSync(
this.configPath,
JSON.stringify(this.chatsConfig, null, 4),
JSON.stringify(this.config, null, 2),
'utf-8',
);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for file write operations in saveConfig()

The saveConfig() method writes to the file system without error handling. If an error occurs during fs.writeFileSync, it could lead to application crashes or silent failures.

Consider wrapping the write operation in a try-catch block and logging any errors:

private saveConfig() {
  const configDir = path.dirname(this.configPath);
  if (!fs.existsSync(configDir)) {
    fs.mkdirSync(configDir, { recursive: true });
  }
- fs.writeFileSync(
+ try {
+   fs.writeFileSync(
      this.configPath,
      JSON.stringify(this.config, null, 2),
      'utf-8',
    );
+ } catch (error) {
+   console.error(`Failed to save configuration: ${error.message}`);
+ }
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private saveConfig() {
const configDir = path.dirname(this.configPath);
if (!fs.existsSync(configDir)) {
fs.mkdirSync(configDir, { recursive: true });
}
fs.writeFileSync(
this.configPath,
JSON.stringify(this.chatsConfig, null, 4),
JSON.stringify(this.config, null, 2),
'utf-8',
);
}
private saveConfig() {
const configDir = path.dirname(this.configPath);
if (!fs.existsSync(configDir)) {
fs.mkdirSync(configDir, { recursive: true });
}
try {
fs.writeFileSync(
this.configPath,
JSON.stringify(this.config, null, 2),
'utf-8',
);
} catch (error) {
console.error(`Failed to save configuration: ${error.message}`);
}
}


getAllChatConfigs(): ChatConfig[] {
return this.config.chats || [];
}

getChatConfig(modelName?: string): ChatConfig | null {
if (!this.config.chats || !Array.isArray(this.config.chats)) {
return null;
}

const chats = this.config.chats;

if (modelName) {
const foundChat = chats.find((chat) => chat.model === modelName);
if (foundChat) {
return foundChat;
}
}

return (
chats.find((chat) => chat.default) || (chats.length > 0 ? chats[0] : null)
);
}

addChatConfig(config: ChatConfig) {
if (!this.config.chats) {
this.config.chats = [];
}

const index = this.config.chats.findIndex(
(chat) => chat.model === config.model,
);
if (index !== -1) {
this.config.chats.splice(index, 1);
}

if (config.default) {
this.config.chats.forEach((chat) => {
chat.default = false;
});
}

this.config.chats.push(config);
this.saveConfig();
}

removeChatConfig(modelName: string): boolean {
if (!this.config.chats) {
return false;
}

const initialLength = this.config.chats.length;
this.config.chats = this.config.chats.filter(
(chat) => chat.model !== modelName,
);

if (this.config.chats.length !== initialLength) {
this.saveConfig();
return true;
}

return false;
}

getEmbeddingConfig(): EmbeddingConfig | null {
return this.config.embeddings || null;
}

validateConfig() {
if (!this.chatsConfig) {
throw new Error("Invalid configuration: 'chats' section is missing.");
if (!this.config) {
this.config = {};
}

if (typeof this.config !== 'object') {
throw new Error('Invalid configuration: Must be an object');
}

if (this.config.chats) {
if (!Array.isArray(this.config.chats)) {
throw new Error("Invalid configuration: 'chats' must be an array");
}

this.config.chats.forEach((chat, index) => {
if (!chat.model) {
throw new Error(
`Invalid chat configuration at index ${index}: 'model' is required`,
);
}
});

const defaultChats = this.config.chats.filter((chat) => chat.default);
if (defaultChats.length > 1) {
throw new Error(
'Invalid configuration: Multiple default chat configurations found',
);
}
}

if (this.config.embeddings) {
if (!this.config.embeddings.model) {
throw new Error("Invalid embedding configuration: 'model' is required");
}
}
}

getConfig(): AppConfig {
return this.config;
}
}
4 changes: 2 additions & 2 deletions backend/src/main.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { NestFactory } from '@nestjs/core';
import { AppModule } from './app.module';
import 'reflect-metadata';
import { downloadAllModels } from './model/utils';
import { checkAndDownloadAllModels } from './model/utils';

async function bootstrap() {
const app = await NestFactory.create(AppModule);
Expand All @@ -17,7 +17,7 @@ async function bootstrap() {
'Access-Control-Allow-Credentials',
],
});
await downloadAllModels();
await checkAndDownloadAllModels();
await app.listen(process.env.PORT ?? 3000);
}

Expand Down
24 changes: 0 additions & 24 deletions backend/src/model/__tests__/app.e2e-spec.ts

This file was deleted.

9 changes: 0 additions & 9 deletions backend/src/model/__tests__/jest-e2e.json

This file was deleted.

4 changes: 2 additions & 2 deletions backend/src/model/__tests__/loadAllChatsModels.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import path from 'path';
import * as fs from 'fs';
import { ConfigLoader } from '../../config/config-loader';
import { ModelDownloader } from '../model-downloader';
import { ModelDownloader } from '../downloader/downloader';
import { downloadAllModels } from '../utils';
import { getConfigDir, getConfigPath } from 'src/config/common-path';
import { getConfigPath, getConfigPath } from 'src/config/common-path';
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove duplicate import of getConfigPath
Importing getConfigPath twice is a syntax error.

-import { getConfigPath, getConfigPath } from 'src/config/common-path';
+import { getConfigPath } from 'src/config/common-path';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { getConfigPath, getConfigPath } from 'src/config/common-path';
import { getConfigPath } from 'src/config/common-path';
🧰 Tools
🪛 Biome (1.9.4)

[error] 6-6: Declarations inside of a import declaration may not have duplicates

a second declaration of getConfigPath is not allowed

getConfigPath is first declared here

(parse)


const originalIsArray = Array.isArray;

Expand Down
Loading
Loading