modellist: fix incorrect signal use and remove invalidate calls (#3042)

Signed-off-by: Jared Van Bortel <jared@nomic.ai>

gpt4all-chat/CHANGELOG.md

@@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 - Fix "regenerate" always forgetting the most recent message ([#3011](https://github.com/nomic-ai/gpt4all/pull/3011))
 - Fix loaded chats forgetting context when there is a system prompt ([#3015](https://github.com/nomic-ai/gpt4all/pull/3015))
 - Make it possible to downgrade and keep some chats, and avoid crash for some model types ([#3030](https://github.com/nomic-ai/gpt4all/pull/3030))
+- Fix scroll positition being reset in model view, and attempt a better fix for the clone issue ([#3042](https://github.com/nomic-ai/gpt4all/pull/3042))
 
 ## [3.3.1] - 2024-09-27 ([v3.3.y](https://github.com/nomic-ai/gpt4all/tree/v3.3.y))
 

gpt4all-chat/src/localdocsmodel.cpp

@@ -20,7 +20,6 @@ LocalDocsCollectionsModel::LocalDocsCollectionsModel(QObject *parent)
     connect(this, &LocalDocsCollectionsModel::rowsInserted, this, &LocalDocsCollectionsModel::countChanged);
     connect(this, &LocalDocsCollectionsModel::rowsRemoved, this, &LocalDocsCollectionsModel::countChanged);
     connect(this, &LocalDocsCollectionsModel::modelReset, this, &LocalDocsCollectionsModel::countChanged);
-    connect(this, &LocalDocsCollectionsModel::layoutChanged, this, &LocalDocsCollectionsModel::countChanged);
 }
 
 bool LocalDocsCollectionsModel::filterAcceptsRow(int sourceRow,
@@ -67,7 +66,6 @@ LocalDocsModel::LocalDocsModel(QObject *parent)
     connect(this, &LocalDocsModel::rowsInserted, this, &LocalDocsModel::countChanged);
     connect(this, &LocalDocsModel::rowsRemoved, this, &LocalDocsModel::countChanged);
     connect(this, &LocalDocsModel::modelReset, this, &LocalDocsModel::countChanged);
-    connect(this, &LocalDocsModel::layoutChanged, this, &LocalDocsModel::countChanged);
 }
 
 int LocalDocsModel::rowCount(const QModelIndex &parent) const

gpt4all-chat/src/modellist.cpp

@@ -398,7 +398,6 @@ InstalledModels::InstalledModels(QObject *parent, bool selectable)
     connect(this, &InstalledModels::rowsInserted, this, &InstalledModels::countChanged);
     connect(this, &InstalledModels::rowsRemoved, this, &InstalledModels::countChanged);
     connect(this, &InstalledModels::modelReset, this, &InstalledModels::countChanged);
-    connect(this, &InstalledModels::layoutChanged, this, &InstalledModels::countChanged);
 }
 
 bool InstalledModels::filterAcceptsRow(int sourceRow,
@@ -423,7 +422,6 @@ DownloadableModels::DownloadableModels(QObject *parent)
     connect(this, &DownloadableModels::rowsInserted, this, &DownloadableModels::countChanged);
     connect(this, &DownloadableModels::rowsRemoved, this, &DownloadableModels::countChanged);
     connect(this, &DownloadableModels::modelReset, this, &DownloadableModels::countChanged);
-    connect(this, &DownloadableModels::layoutChanged, this, &DownloadableModels::countChanged);
 }
 
 bool DownloadableModels::filterAcceptsRow(int sourceRow,
@@ -821,7 +819,11 @@ QVariant ModelList::data(const QModelIndex &index, int role) const
 
 void ModelList::updateData(const QString &id, const QVector<QPair<int, QVariant>> &data)
 {
+    // We only sort when one of the fields used by the sorting algorithm actually changes that
+    // is implicated or used by the sorting algorithm
+    bool shouldSort = false;
     int index;
+
     {
         QMutexLocker locker(&m_mutex);
         if (!m_modelMap.contains(id)) {
@@ -836,10 +838,6 @@ void ModelList::updateData(const QString &id, const QVector<QPair<int, QVariant>
             return;
         }
 
-        // We only sort when one of the fields used by the sorting algorithm actually changes that
-        // is implicated or used by the sorting algorithm
-        bool shouldSort = false;
-
         for (const auto &d : data) {
             const int role = d.first;
             const QVariant value = d.second;
@@ -1000,21 +998,12 @@ void ModelList::updateData(const QString &id, const QVector<QPair<int, QVariant>
             info->isEmbeddingModel = LLModel::Implementation::isEmbeddingModel(modelPath.toStdString());
             info->checkedEmbeddingModel = true;
         }
-
-        if (shouldSort) {
-            auto s = m_discoverSort;
-            auto d = m_discoverSortDirection;
-            std::stable_sort(m_models.begin(), m_models.end(), [s, d](const ModelInfo* lhs, const ModelInfo* rhs) {
-                return ModelList::lessThan(lhs, rhs, s, d);
-            });
-        }
     }
+
     emit dataChanged(createIndex(index, 0), createIndex(index, 0));
 
-    // FIXME(jared): for some reason these don't update correctly when the source model changes, so we explicitly invalidate them
+    if (shouldSort)
-    m_selectableModels->invalidate();
+        resortModel();
-    m_installedModels->invalidate();
-    m_downloadableModels->invalidate();
 
     emit selectableModelListChanged();
 }
@@ -1122,7 +1111,6 @@ void ModelList::removeClone(const ModelInfo &model)
         return;
 
     removeInternal(model);
-    emit layoutChanged();
 }
 
 void ModelList::removeInstalled(const ModelInfo &model)
@@ -1131,7 +1119,6 @@ void ModelList::removeInstalled(const ModelInfo &model)
     Q_ASSERT(!model.isClone());
     Q_ASSERT(model.isDiscovered() || model.isCompatibleApi || model.description() == "" /*indicates sideloaded*/);
     removeInternal(model);
-    emit layoutChanged();
 }
 
 void ModelList::removeInternal(const ModelInfo &model)
@@ -1928,7 +1915,6 @@ void ModelList::clearDiscoveredModels()
     }
     for (ModelInfo &info : infos)
         removeInternal(info);
-    emit layoutChanged();
 }
 
 float ModelList::discoverProgress() const
@@ -2176,7 +2162,6 @@ void ModelList::handleDiscoveryItemFinished()
     emit discoverProgressChanged();
 
     if (discoverProgress() >= 1.0) {
-        emit layoutChanged();
         m_discoverInProgress = false;
         emit discoverInProgressChanged();;
     }