fix: resolve model list merge issue during app update (#77)

* fix: resolve model list merge issue during app update

* chore: bump version

android/app/build.gradle

@@ -81,8 +81,8 @@ android {
         applicationId "com.pocketpalai"
         minSdkVersion rootProject.ext.minSdkVersion
         targetSdkVersion rootProject.ext.targetSdkVersion
-        versionCode 11 
-        versionName "1.4.4"
+        versionCode 12 
+        versionName "1.4.5"
         ndk {
             abiFilters "arm64-v8a", "x86_64"
         }

ios/PocketPal.xcodeproj/project.pbxproj

@@ -479,7 +479,7 @@
 			buildSettings = {
 				ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon;
 				CLANG_ENABLE_MODULES = YES;
-				CURRENT_PROJECT_VERSION = 11;
+				CURRENT_PROJECT_VERSION = 12;
 				DEVELOPMENT_TEAM = MYXGXY23Y6;
 				ENABLE_BITCODE = NO;
 				INFOPLIST_FILE = PocketPal/Info.plist;
@@ -488,7 +488,7 @@
 					"$(inherited)",
 					"@executable_path/Frameworks",
 				);
-				MARKETING_VERSION = 1.4.4;
+				MARKETING_VERSION = 1.4.5;
 				OTHER_CPLUSPLUSFLAGS = (
 					"$(OTHER_CFLAGS)",
 					"-DFOLLY_NO_CONFIG",
@@ -521,15 +521,15 @@
 			buildSettings = {
 				ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon;
 				CLANG_ENABLE_MODULES = YES;
-				CURRENT_PROJECT_VERSION = 11;
+				CURRENT_PROJECT_VERSION = 12;
 				DEVELOPMENT_TEAM = MYXGXY23Y6;
 				INFOPLIST_FILE = PocketPal/Info.plist;
 				IPHONEOS_DEPLOYMENT_TARGET = 14.0;
 				LD_RUNPATH_SEARCH_PATHS = (
 					"$(inherited)",
 					"@executable_path/Frameworks",
 				);
-				MARKETING_VERSION = 1.4.4;
+				MARKETING_VERSION = 1.4.5;
 				OTHER_CPLUSPLUSFLAGS = (
 					"$(OTHER_CFLAGS)",
 					"-DFOLLY_NO_CONFIG",

package.json

@@ -1,6 +1,6 @@
 {
   "name": "PocketPal",
-  "version": "1.4.4",
+  "version": "1.4.5",
   "private": true,
   "scripts": {
     "prepare": "husky",

src/store/ModelStore.ts

@@ -9,7 +9,7 @@ import {computed, makeAutoObservable, ObservableMap, runInAction} from 'mobx';
 import {CompletionParams, LlamaContext, initLlama} from '@pocketpalai/llama.rn';
 import AsyncStorage from '@react-native-async-storage/async-storage';
 
-import {bytesToGB, hasEnoughSpace} from '../utils';
+import {bytesToGB, deepMerge, hasEnoughSpace} from '../utils';
 import {defaultModels, MODEL_LIST_VERSION} from './defaultModels';
 
 import {chatTemplates} from '../utils/chat';
@@ -78,20 +78,24 @@ class ModelStore {
       const existingModelIndex = mergedModels.findIndex(
         m => m.id === defaultModel.id,
       );
-      // console.log('existingModelIndex: ', existingModelIndex);
 
       if (existingModelIndex !== -1) {
         // Merge existing model with new defaults
-        mergedModels[existingModelIndex] = {
-          ...defaultModel, // Defaults take precedence (for new fields or updates)
-          ...mergedModels[existingModelIndex], // User changes override defaults
-          chatTemplate:
-            mergedModels[existingModelIndex].chatTemplate ||
-            defaultModel.chatTemplate,
-          completionSettings:
-            mergedModels[existingModelIndex].completionSettings ||
-            defaultModel.completionSettings,
-        };
+        const existingModel = mergedModels[existingModelIndex];
+
+        // Deep merge chatTemplate and completionSettings
+        existingModel.chatTemplate = deepMerge(
+          existingModel.chatTemplate || {},
+          defaultModel.chatTemplate || {},
+        );
+
+        existingModel.completionSettings = deepMerge(
+          existingModel.completionSettings || {},
+          defaultModel.completionSettings || {},
+        );
+
+        // Merge other properties
+        mergedModels[existingModelIndex] = existingModel;
       } else {
         // Add new model if it doesn't exist
         mergedModels.push(defaultModel);

src/store/__tests__/ModelStore.test.ts

@@ -0,0 +1,80 @@
+jest.unmock('../ModelStore'); // This is not really needed, as only importing from store is mocked.
+import {modelStore} from '../ModelStore';
+import {runInAction} from 'mobx';
+import {defaultModels} from '../defaultModels';
+
+describe('ModelStore', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+    modelStore.models = []; // Clear models before each test
+  });
+
+  describe('mergeModelLists', () => {
+    it('should add missing default models to the existing model list', () => {
+      modelStore.models = []; // Start with no existing models
+
+      runInAction(() => {
+        modelStore.mergeModelLists();
+      });
+
+      expect(modelStore.models.length).toBeGreaterThan(0);
+      expect(modelStore.models).toEqual(expect.arrayContaining(defaultModels));
+    });
+
+    it('should merge existing models with default models, adding any that are missing', () => {
+      const notExistedModel = defaultModels[0];
+      modelStore.models = defaultModels.slice(1); // Start with all but the first model, so we can test if it's added back
+      expect(modelStore.models.length).toBe(defaultModels.length - 1);
+      expect(modelStore.models).not.toContainEqual(notExistedModel);
+
+      runInAction(() => {
+        modelStore.mergeModelLists();
+      });
+
+      expect(modelStore.models.length).toBeGreaterThan(0);
+      expect(modelStore.models).toContainEqual(notExistedModel);
+    });
+
+    it('should retain value of existing variables while merging new variables', () => {
+      const newDefaultModel = defaultModels[0];
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      const {temperature, ...completionSettingsWithoutTemperature} =
+        newDefaultModel.completionSettings; // Exclude temperature
+
+      // Apply changes to the existing model:
+      //  - chatTemplate.template: existing variable with a value different from the default
+      //  - completionSettings.n_predict: existing variable with a value different from the default
+      //  - temperature: new variable in the default model - not present in the existing model
+      const existingModel = {
+        ...newDefaultModel,
+        chatTemplate: {
+          ...newDefaultModel.chatTemplate,
+          template: 'existing',
+        },
+        completionSettings: {
+          ...completionSettingsWithoutTemperature, // Use the completionSettings without temperature - simulates new parameters
+          n_predict: 101010,
+        },
+      };
+
+      modelStore.models[0] = existingModel;
+
+      runInAction(() => {
+        modelStore.mergeModelLists();
+      });
+
+      expect(modelStore.models[0].chatTemplate).toEqual(
+        expect.objectContaining({
+          template: 'existing', // Existing value should remain
+        }),
+      );
+
+      expect(modelStore.models[0].completionSettings).toEqual(
+        expect.objectContaining({
+          n_predict: 101010, // Existing value should remain
+          temperature: newDefaultModel.completionSettings.temperature, // Non-existing value should be merged
+        }),
+      );
+    });
+  });
+});

src/store/defaultModels.ts

@@ -2,7 +2,7 @@ import {Model} from '../utils/types';
 import {chatTemplates, defaultCompletionParams} from '../utils/chat';
 import {Platform} from 'react-native';
 
-export const MODEL_LIST_VERSION = 5;
+export const MODEL_LIST_VERSION = 6;
 
 export const defaultModels: Model[] = [
   {

src/utils/__tests__/utils.test.ts

@@ -1,4 +1,4 @@
-import {formatBytes, getTextSizeInBytes, unwrap} from '..';
+import {deepMerge, formatBytes, getTextSizeInBytes, unwrap} from '..';
 
 describe('formatBytes', () => {
   it('formats bytes correctly when the size is 0', () => {
@@ -38,3 +38,76 @@ describe('unwrap', () => {
     expect(unwrap(prop)).toStrictEqual(prop);
   });
 });
+
+describe('deepMerge', () => {
+  it('should merge two flat objects', () => {
+    const target = {a: 1, b: 2};
+    const source = {b: 3, c: 4};
+    const result = deepMerge(target, source);
+    expect(result).toEqual({a: 1, b: 2, c: 4}); // b should remain 2
+  });
+
+  it('should merge nested objects', () => {
+    const target = {a: {b: 1}};
+    const source = {a: {c: 2}};
+    const result = deepMerge(target, source);
+    expect(result).toEqual({a: {b: 1, c: 2}}); // c should be added
+  });
+
+  it('should overwrite nested properties', () => {
+    const target = {a: {b: 1, c: 2}};
+    const source = {a: {b: 3}};
+    const result = deepMerge(target, source);
+    expect(result).toEqual({a: {b: 1, c: 2}}); // b should remain 1
+  });
+
+  it('should handle arrays correctly', () => {
+    const target = {a: [1, 2]};
+    const source = {a: [3, 4]};
+    const result = deepMerge(target, source);
+    expect(result).toEqual({a: [1, 2]});
+  });
+
+  it('should handle null values', () => {
+    const target = {a: null};
+    const source = {a: {b: 1}};
+    const result = deepMerge(target, source);
+    expect(result).toEqual({a: {b: 1}}); // Replaces null with the object
+  });
+
+  it('should handle flat to nested', () => {
+    const target = {a: 1, c: {d: 2, e: 3}};
+    const source = {a: {b: 1}, c: 4};
+    const result = deepMerge(target, source);
+    expect(result).toEqual({a: {b: 1}, c: 4});
+  });
+
+  it('should not modify the original objects', () => {
+    const target = {a: 1};
+    const source = {b: 2};
+    deepMerge(target, source);
+    expect(target).toEqual({a: 1, b: 2});
+    expect(source).toEqual({b: 2}); // Source should remain unchanged
+  });
+
+  it('should handle empty objects', () => {
+    const target = {};
+    const source = {a: 1};
+    const result = deepMerge(target, source);
+    expect(result).toEqual({a: 1}); // Merges from source
+  });
+
+  it('should handle deeply nested objects', () => {
+    const target = {a: {b: {c: 1}}};
+    const source = {a: {b: {d: 2}}};
+    const result = deepMerge(target, source);
+    expect(result).toEqual({a: {b: {c: 1, d: 2}}});
+  });
+
+  it('should merge multiple levels of nesting', () => {
+    const target = {a: {b: {c: {d: 1, e: 3}}}};
+    const source = {a: {b: {c: {e: 2, f: 4}}}};
+    const result = deepMerge(target, source);
+    expect(result).toEqual({a: {b: {c: {d: 1, e: 3, f: 4}}}});
+  });
+});

src/utils/index.ts

@@ -313,4 +313,32 @@ export async function hasEnoughSpace(model: Model): Promise<boolean> {
   }
 }
 
+/**
+ * Merges properties from the source object into the target object deeply.
+ * Only sets properties that do not already exist in the target or if the types differ.
+ *
+ * @param target - The target object to merge properties into.
+ * @param source - The source object from which properties are taken.
+ * @returns The updated target object after merging.
+ */
+export const deepMerge = (target: any, source: any): any => {
+  for (const key in source) {
+    if (source.hasOwnProperty(key)) {
+      if (typeof source[key] === 'object' && source[key] !== null) {
+        // If the property is an object, recursively merge.
+        // If target[key] is not an object, it means the property type is different so we will replace it.
+        target[key] =
+          target[key] && typeof target[key] === 'object' ? target[key] : {};
+        deepMerge(target[key], source[key]);
+      } else {
+        // Set the property in the target only if it doesn't exist or if the types differ
+        if (!(key in target) || typeof target[key] !== typeof source[key]) {
+          target[key] = source[key];
+        }
+      }
+    }
+  }
+  return target;
+};
+
 export const randId = () => Math.random().toString(36).substring(2, 11);